Skip to content

Commit

Permalink
Add a new check: ProvidesMethodOutsideOfModule. This detects when
Browse files Browse the repository at this point in the history
@provides methods are defined outside of Module classes.

MOE_MIGRATED_REVID=131638455
  • Loading branch information
nick-someone committed Aug 30, 2016
1 parent 7ff03cf commit b169efa
Show file tree
Hide file tree
Showing 6 changed files with 256 additions and 0 deletions.
6 changes: 6 additions & 0 deletions core/pom.xml
Expand Up @@ -154,6 +154,12 @@
<version>4.0-beta5</version>
<scope>test</scope>
</dependency>
<dependency>
<groupId>com.google.gwt.inject</groupId>
<artifactId>gin</artifactId>
<version>2.1.2</version>
<scope>test</scope>
</dependency>
<dependency>
<groupId>org.mockito</groupId>
<artifactId>mockito-core</artifactId>
Expand Down
@@ -0,0 +1,66 @@
/*
* Copyright 2016 Google Inc. All Rights Reserved.
*
* Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file except
* in compliance with the License. You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software distributed under the License
* is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express
* or implied. See the License for the specific language governing permissions and limitations under
* the License.
*/

package com.google.errorprone.bugpatterns.inject.guice;

import static com.google.errorprone.BugPattern.Category.GUICE;
import static com.google.errorprone.BugPattern.MaturityLevel.EXPERIMENTAL;
import static com.google.errorprone.BugPattern.SeverityLevel.ERROR;
import static com.google.errorprone.matchers.Matchers.allOf;
import static com.google.errorprone.matchers.Matchers.enclosingClass;
import static com.google.errorprone.matchers.Matchers.isSubtypeOf;
import static com.google.errorprone.matchers.Matchers.isType;
import static com.google.errorprone.matchers.Matchers.not;

import com.google.errorprone.BugPattern;
import com.google.errorprone.VisitorState;
import com.google.errorprone.bugpatterns.BugChecker;
import com.google.errorprone.bugpatterns.BugChecker.AnnotationTreeMatcher;
import com.google.errorprone.fixes.SuggestedFix;
import com.google.errorprone.matchers.Description;
import com.google.errorprone.matchers.Matcher;
import com.google.errorprone.matchers.Matchers;
import com.sun.source.tree.AnnotationTree;
import com.sun.source.tree.ClassTree;

/** @author glorioso@google.com (Nick Glorioso) */
@BugPattern(
name = "ProvidesMethodOutsideOfModule",
summary = "@Provides methods need to be declared in a Module to have any effect.",
explanation =
"Guice `@Provides` methods annotate methods that are used as a means of declaring"
+ " bindings. However, this is only helpful inside of a module. Methods outside of these"
+ " modules are not used for binding declaration.",
category = GUICE,
severity = ERROR,
maturity = EXPERIMENTAL
)
public class ProvidesMethodOutsideOfModule extends BugChecker implements AnnotationTreeMatcher {
private static final Matcher<AnnotationTree> PROVIDES_ANNOTATION_ON_METHOD_OUTSIDE_OF_MODULE =
allOf(
isType("com.google.inject.Provides"),
enclosingClass(
not(
Matchers.<ClassTree>anyOf(
isSubtypeOf("com.google.inject.Module"),
isSubtypeOf("com.google.gwt.inject.client.GinModule")))));

@Override
public Description matchAnnotation(AnnotationTree annotation, VisitorState state) {
if (PROVIDES_ANNOTATION_ON_METHOD_OUTSIDE_OF_MODULE.matches(annotation, state)) {
return describeMatch(annotation, SuggestedFix.delete(annotation));
}
return Description.NO_MATCH;
}
}
Expand Up @@ -170,6 +170,7 @@
import com.google.errorprone.bugpatterns.inject.guice.InjectOnFinalField;
import com.google.errorprone.bugpatterns.inject.guice.OverridesGuiceInjectableMethod;
import com.google.errorprone.bugpatterns.inject.guice.OverridesJavaxInjectableMethod;
import com.google.errorprone.bugpatterns.inject.guice.ProvidesMethodOutsideOfModule;
import com.google.errorprone.bugpatterns.threadsafety.DoubleCheckedLocking;
import com.google.errorprone.bugpatterns.threadsafety.GuardedByChecker;
import com.google.errorprone.bugpatterns.threadsafety.GuardedByValidator;
Expand Down Expand Up @@ -368,6 +369,7 @@ public static ScannerSupplier errorChecks() {
PrimitiveArrayPassedToVarargsMethod.class,
PrivateConstructorForUtilityClass.class,
PrivateConstructorForNoninstantiableModule.class,
ProvidesMethodOutsideOfModule.class,
ProtoStringFieldReferenceEquality.class,
RedundantThrows.class,
RemoveUnusedImports.class,
Expand Down
@@ -0,0 +1,45 @@
/*
* Copyright 2016 Google Inc. All Rights Reserved.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/

package com.google.errorprone.bugpatterns.inject.guice;

import com.google.errorprone.CompilationTestHelper;
import org.junit.Before;
import org.junit.Test;
import org.junit.runner.RunWith;
import org.junit.runners.JUnit4;

/** @author glorioso@google.com (Nick Glorioso) */
@RunWith(JUnit4.class)
public class ProvidesMethodOutsideOfModuleTest {
private CompilationTestHelper compilationHelper;

@Before
public void setUp() {
compilationHelper =
CompilationTestHelper.newInstance(ProvidesMethodOutsideOfModule.class, getClass());
}

@Test
public void testPositiveCase() throws Exception {
compilationHelper.addSourceFile("ProvidesMethodOutsideOfModulePositiveCases.java").doTest();
}

@Test
public void testNegativeCase() throws Exception {
compilationHelper.addSourceFile("ProvidesMethodOutsideOfModuleNegativeCases.java").doTest();
}
}
@@ -0,0 +1,74 @@
/*
* Copyright 2016 Google Inc. All Rights Reserved.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/

package com.google.errorprone.bugpatterns.inject.guice.testdata;

import com.google.gwt.inject.client.AbstractGinModule;
import com.google.gwt.inject.client.GinModule;
import com.google.gwt.inject.client.binder.GinBinder;
import com.google.inject.AbstractModule;
import com.google.inject.Binder;
import com.google.inject.Module;
import com.google.inject.Provides;

/** Tests for {@code ProvidesMethodOutsideOfModule} */
public class ProvidesMethodOutsideOfModuleNegativeCases {

/** Regular module */
class Module1 extends AbstractModule {
@Override
protected void configure() {}

@Provides
int providesFoo() {
return 42;
}
}

/** implements the Module interface directly */
class Module2 implements Module {
@Override
public void configure(Binder binder) {}

@Provides
int providesFoo() {
return 42;
}
}

/** Regular GinModule */
class GinModule1 extends AbstractGinModule {

@Override
protected void configure() {}

@Provides
int providesFoo() {
return 42;
}
}

/** Implements the GinModule interface directly */
class GinModule2 implements GinModule {
@Override
public void configure(GinBinder binder) {}

@Provides
int providesFoo() {
return 42;
}
}
}
@@ -0,0 +1,63 @@
/*
* Copyright 2016 Google Inc. All Rights Reserved.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/

package com.google.errorprone.bugpatterns.inject.guice.testdata;

import com.google.inject.AbstractModule;
import com.google.inject.Provides;

/** Tests for {@code ProvidesMethodOutsideOfModule} */
public class ProvidesMethodOutsideOfModulePositiveCases {

/** Random class contains a provides method. */
public class TestClass1 {
// BUG: Diagnostic contains: remove
@Provides
void providesBlah() {}
}

/** Module contains an anonymous inner with a Provides method. */
public class TestModule extends AbstractModule {
@Override
protected void configure() {
Object x =
new Object() {
// BUG: Diagnostic contains: remove
@Provides
void providesBlah() {}
};
}
}

/** Class has inner module class */
public class TestClass2 {
class NestedModule extends AbstractModule {
@Override
protected void configure() {}

@Provides
int thisIsOk() {
return 42;
}
}

// BUG: Diagnostic contains: remove
@Provides
int thisIsNotOk() {
return 42;
}
}
}

0 comments on commit b169efa

Please sign in to comment.