Refactor common inject matchers to a shared InjectMatchers class

and alter the behavior of JavaxInjectOnAbstractMethod to match from the MethodTree up, rather than the AnnotationTree down. MOE_MIGRATED_REVID=130976271
google · Aug 23, 2016 · 5946d0a · 5946d0a
1 parent 5ee6c4c
commit 5946d0a
Show file tree

Hide file tree

Showing 19 changed files with 169 additions and 184 deletions.
diff --git a/.../java/com/google/errorprone/bugpatterns/inject/AssistedInjectAndInjectOnConstructors.java b/.../java/com/google/errorprone/bugpatterns/inject/AssistedInjectAndInjectOnConstructors.java
@@ -20,15 +20,18 @@
 import static com.google.errorprone.BugPattern.MaturityLevel.EXPERIMENTAL;
 import static com.google.errorprone.BugPattern.SeverityLevel.WARNING;
 import static com.google.errorprone.matchers.ChildMultiMatcher.MatchType.AT_LEAST_ONE;
+import static com.google.errorprone.matchers.InjectMatchers.ASSISTED_INJECT_ANNOTATION;
+import static com.google.errorprone.matchers.InjectMatchers.GUICE_INJECT_ANNOTATION;
+import static com.google.errorprone.matchers.InjectMatchers.JAVAX_INJECT_ANNOTATION;
 import static com.google.errorprone.matchers.Matchers.constructor;
-import static com.google.errorprone.matchers.Matchers.hasAnnotation;
 
 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.InjectMatchers;
 import com.google.errorprone.matchers.Matcher;
 import com.google.errorprone.matchers.Matchers;
 import com.google.errorprone.matchers.MultiMatcher;
@@ -56,19 +59,9 @@
 public class AssistedInjectAndInjectOnConstructors extends BugChecker
     implements AnnotationTreeMatcher {
 
-  private static final String GUICE_INJECT_ANNOTATION = "com.google.inject.Inject";
-  private static final String JAVAX_INJECT_ANNOTATION = "javax.inject.Inject";
-  private static final String ASSISTED_INJECT_ANNOTATION =
-      "com.google.inject.assistedinject.AssistedInject";
-
-  /**
-   * Matches if any constructor of a class is annotated with an @Inject annotation.
-   */
+  /** Matches if any constructor of a class is annotated with an @Inject annotation. */
   private final MultiMatcher<ClassTree, MethodTree> constructorWithInjectMatcher =
-      constructor(
-          AT_LEAST_ONE,
-          Matchers.<MethodTree>anyOf(
-              hasAnnotation(GUICE_INJECT_ANNOTATION), hasAnnotation(JAVAX_INJECT_ANNOTATION)));
+      constructor(AT_LEAST_ONE, InjectMatchers.<MethodTree>hasInjectAnnotation());
 
   /**
    * Matches if any constructor of a class is annotated with an @AssistedInject annotation.

diff --git a/...va/com/google/errorprone/bugpatterns/inject/AssistedInjectAndInjectOnSameConstructor.java b/...va/com/google/errorprone/bugpatterns/inject/AssistedInjectAndInjectOnSameConstructor.java
@@ -19,7 +19,13 @@
 import static com.google.errorprone.BugPattern.Category.INJECT;
 import static com.google.errorprone.BugPattern.MaturityLevel.EXPERIMENTAL;
 import static com.google.errorprone.BugPattern.SeverityLevel.ERROR;
+import static com.google.errorprone.matchers.InjectMatchers.ASSISTED_INJECT_ANNOTATION;
+import static com.google.errorprone.matchers.InjectMatchers.GUICE_INJECT_ANNOTATION;
+import static com.google.errorprone.matchers.InjectMatchers.JAVAX_INJECT_ANNOTATION;
+import static com.google.errorprone.matchers.InjectMatchers.hasInjectAnnotation;
+import static com.google.errorprone.matchers.Matchers.anyOf;
 import static com.google.errorprone.matchers.Matchers.hasAnnotation;
+import static com.google.errorprone.matchers.Matchers.isType;
 
 import com.google.errorprone.BugPattern;
 import com.google.errorprone.VisitorState;
@@ -28,12 +34,10 @@
 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.google.errorprone.util.ASTHelpers;
 import com.sun.source.tree.AnnotationTree;
 import com.sun.source.tree.MethodTree;
 import com.sun.source.tree.Tree;
-import com.sun.tools.javac.code.Symbol;
 
 /**
  * @author sgoldfeder@google.com (Steven Goldfeder)
@@ -50,45 +54,24 @@
 public class AssistedInjectAndInjectOnSameConstructor extends BugChecker
     implements AnnotationTreeMatcher {
 
-  private static final String GUICE_INJECT_ANNOTATION = "com.google.inject.Inject";
-  private static final String JAVAX_INJECT_ANNOTATION = "javax.inject.Inject";
-  private static final String ASSISTED_INJECT_ANNOTATION =
-      "com.google.inject.assistedinject.AssistedInject";
+  /** Matches a method/constructor that is annotated with an @AssistedInject annotation. */
+  private static final Matcher<MethodTree> HAS_ASSISTED_INJECT_MATCHER =
+      hasAnnotation(ASSISTED_INJECT_ANNOTATION);
 
-  /**
-   * Matches a method/constructor that is annotated with an @Inject annotation.
-   */
-  private final Matcher<MethodTree> constructorWithInjectMatcher =
-      Matchers.<MethodTree>anyOf(
-          hasAnnotation(GUICE_INJECT_ANNOTATION), hasAnnotation(JAVAX_INJECT_ANNOTATION));
-
-  /**
-   * Matches a method/constructor that is annotated with an @AssistedInject annotation.
-   */
-  private final Matcher<MethodTree> constructorWithAssistedInjectMatcher =
-      Matchers.<MethodTree>hasAnnotation(ASSISTED_INJECT_ANNOTATION);
-
-  /**
-   * Matches the @Inject and @Assisted inject annotations.
-   */
-  private final Matcher<AnnotationTree> injectOrAssistedInjectMatcher =
-      new Matcher<AnnotationTree>() {
-        @Override
-        public boolean matches(AnnotationTree annotationTree, VisitorState state) {
-          Symbol annotationSymbol = ASTHelpers.getSymbol(annotationTree);
-          return (annotationSymbol.equals(state.getSymbolFromString(JAVAX_INJECT_ANNOTATION))
-              || annotationSymbol.equals(state.getSymbolFromString(GUICE_INJECT_ANNOTATION))
-              || annotationSymbol.equals(state.getSymbolFromString(ASSISTED_INJECT_ANNOTATION)));
-        }
-      };
+  /** Matches the @Inject and @Assisted inject annotations. */
+  private static final Matcher<AnnotationTree> injectOrAssistedInjectMatcher =
+      anyOf(
+          isType(JAVAX_INJECT_ANNOTATION),
+          isType(GUICE_INJECT_ANNOTATION),
+          isType(ASSISTED_INJECT_ANNOTATION));
 
   @Override
   public Description matchAnnotation(AnnotationTree annotationTree, VisitorState state) {
     if (injectOrAssistedInjectMatcher.matches(annotationTree, state)) {
       Tree treeWithAnnotation = state.getPath().getParentPath().getParentPath().getLeaf();
       if (ASTHelpers.getSymbol(treeWithAnnotation).isConstructor()
-          && constructorWithInjectMatcher.matches((MethodTree) treeWithAnnotation, state)
-          && constructorWithAssistedInjectMatcher.matches((MethodTree) treeWithAnnotation, state)) {
+          && hasInjectAnnotation().matches(treeWithAnnotation, state)
+          && HAS_ASSISTED_INJECT_MATCHER.matches((MethodTree) treeWithAnnotation, state)) {
         return describeMatch(annotationTree, SuggestedFix.delete(annotationTree));
       }
     }

diff --git a/...rc/main/java/com/google/errorprone/bugpatterns/inject/InjectedConstructorAnnotations.java b/...rc/main/java/com/google/errorprone/bugpatterns/inject/InjectedConstructorAnnotations.java
@@ -19,6 +19,8 @@
 import static com.google.errorprone.BugPattern.Category.INJECT;
 import static com.google.errorprone.BugPattern.MaturityLevel.EXPERIMENTAL;
 import static com.google.errorprone.BugPattern.SeverityLevel.ERROR;
+import static com.google.errorprone.matchers.InjectMatchers.GUICE_BINDING_ANNOTATION;
+import static com.google.errorprone.matchers.InjectMatchers.GUICE_INJECT_ANNOTATION;
 import static com.google.errorprone.matchers.Matchers.allOf;
 import static com.google.errorprone.matchers.Matchers.booleanLiteral;
 import static com.google.errorprone.matchers.Matchers.hasAnnotation;
@@ -51,8 +53,6 @@
   maturity = EXPERIMENTAL
 )
 public class InjectedConstructorAnnotations extends BugChecker implements MethodTreeMatcher {
-  private static final String GUICE_INJECT_ANNOTATION = "com.google.inject.Inject";
-  private static final String GUICE_BINDING_ANNOTATION = "com.google.inject.BindingAnnotation";
 
   // A matcher of @Inject{optional=true}
   private static final Matcher<AnnotationTree> OPTIONAL_INJECTION_MATCHER =

diff --git a/...in/java/com/google/errorprone/bugpatterns/inject/InvalidTargetingOnScopingAnnotation.java b/...in/java/com/google/errorprone/bugpatterns/inject/InvalidTargetingOnScopingAnnotation.java
@@ -17,6 +17,9 @@
 import static com.google.errorprone.BugPattern.Category.INJECT;
 import static com.google.errorprone.BugPattern.MaturityLevel.EXPERIMENTAL;
 import static com.google.errorprone.BugPattern.SeverityLevel.ERROR;
+import static com.google.errorprone.matchers.InjectMatchers.GUICE_SCOPE_ANNOTATION;
+import static com.google.errorprone.matchers.InjectMatchers.JAVAX_SCOPE_ANNOTATION;
+import static com.google.errorprone.matchers.Matchers.anyOf;
 import static com.google.errorprone.matchers.Matchers.hasAnnotation;
 import static java.lang.annotation.ElementType.METHOD;
 import static java.lang.annotation.ElementType.TYPE;
@@ -28,7 +31,6 @@
 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.google.errorprone.util.ASTHelpers;
 import com.sun.source.tree.AnnotationTree;
 import com.sun.source.tree.ClassTree;
@@ -53,16 +55,11 @@
 )
 public class InvalidTargetingOnScopingAnnotation extends BugChecker implements ClassTreeMatcher {
 
-  private static final String GUICE_SCOPE_ANNOTATION = "com.google.inject.ScopeAnnotation";
-  private static final String JAVAX_SCOPE_ANNOTATION = "javax.inject.Scope";
   private static final String TARGET_ANNOTATION = "java.lang.annotation.Target";
 
-  /**
-   * Matches classes that are annotated with @Scope or @ScopeAnnotation.
-   */
+  /** Matches classes that are annotated with @Scope or @ScopeAnnotation. */
   private static final Matcher<ClassTree> SCOPE_ANNOTATION_MATCHER =
-      Matchers.<ClassTree>anyOf(
-          hasAnnotation(GUICE_SCOPE_ANNOTATION), hasAnnotation(JAVAX_SCOPE_ANNOTATION));
+      anyOf(hasAnnotation(GUICE_SCOPE_ANNOTATION), hasAnnotation(JAVAX_SCOPE_ANNOTATION));
 
   @Override
   public final Description matchClass(ClassTree classTree, VisitorState state) {

diff --git a/core/src/main/java/com/google/errorprone/bugpatterns/inject/JavaxInjectOnAbstractMethod.java b/core/src/main/java/com/google/errorprone/bugpatterns/inject/JavaxInjectOnAbstractMethod.java
@@ -19,24 +19,21 @@
 import static com.google.errorprone.BugPattern.Category.INJECT;
 import static com.google.errorprone.BugPattern.MaturityLevel.EXPERIMENTAL;
 import static com.google.errorprone.BugPattern.SeverityLevel.ERROR;
-import static com.sun.source.tree.Tree.Kind.METHOD;
+import static com.google.errorprone.fixes.SuggestedFix.delete;
+import static com.google.errorprone.matchers.InjectMatchers.IS_APPLICATION_OF_JAVAX_INJECT;
 import static javax.lang.model.element.Modifier.ABSTRACT;
 
 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.bugpatterns.BugChecker.MethodTreeMatcher;
 import com.google.errorprone.matchers.Description;
 import com.google.errorprone.matchers.Matcher;
-import com.google.errorprone.util.ASTHelpers;
+import com.google.errorprone.matchers.Matchers;
 import com.sun.source.tree.AnnotationTree;
 import com.sun.source.tree.MethodTree;
-import com.sun.source.tree.Tree;
 
-/**
- * @author sgoldfeder@google.com (Steven Goldfeder)
- */
+/** @author sgoldfeder@google.com (Steven Goldfeder) */
 @BugPattern(
   name = "JavaxInjectOnAbstractMethod",
   summary = "Abstract methods are not injectable with javax.inject.Inject.",
@@ -46,42 +43,28 @@
           + "method that was annotated with javax.inject.Inject, and the subclass method"
           + "is not annotated, the subclass method will not be injected.\n\n"
           + "See http://docs.oracle.com/javaee/6/api/javax/inject/Inject.html\n"
-          + "and https://code.google.com/p/google-guice/wiki/JSR330"
+          + "and https://github.com/google/guice/wiki/JSR330"
           + " ",
   category = INJECT,
   severity = ERROR,
   maturity = EXPERIMENTAL
 )
-public class JavaxInjectOnAbstractMethod extends BugChecker implements AnnotationTreeMatcher {
-
-  private static final String JAVAX_INJECT_ANNOTATION = "javax.inject.Inject";
+public class JavaxInjectOnAbstractMethod extends BugChecker implements MethodTreeMatcher {
 
-  final Matcher<AnnotationTree> javaxInjectAnnotationMatcher =
-      new Matcher<AnnotationTree>() {
-        @Override
-        public boolean matches(AnnotationTree annotationTree, VisitorState state) {
-          return (ASTHelpers.getSymbol(annotationTree)
-              .equals(state.getSymbolFromString(JAVAX_INJECT_ANNOTATION)));
-        }
-      };
+  private static final Matcher<MethodTree> IS_ABSTRACT = Matchers.hasModifier(ABSTRACT);
 
   @Override
-  public Description matchAnnotation(AnnotationTree annotationTree, VisitorState state) {
-    if (!javaxInjectAnnotationMatcher.matches(annotationTree, state)) {
+  public Description matchMethod(MethodTree methodTree, VisitorState state) {
+    if (!IS_ABSTRACT.matches(methodTree, state)) {
       return Description.NO_MATCH;
     }
-    Tree annotatedNode = state.getPath().getParentPath().getParentPath().getLeaf();
-    if (isMethod(annotatedNode) && isAbstract(annotatedNode)) {
-      return describeMatch(annotationTree, SuggestedFix.delete(annotationTree));
-    }
-    return Description.NO_MATCH;
-  }
 
-  private static boolean isMethod(Tree tree) {
-    return tree.getKind().equals(METHOD);
-  }
+    for (AnnotationTree annotationTree : methodTree.getModifiers().getAnnotations()) {
+      if (IS_APPLICATION_OF_JAVAX_INJECT.matches(annotationTree, state)) {
+        return describeMatch(annotationTree, delete(annotationTree));
+      }
+    }
 
-  private static boolean isAbstract(Tree tree) {
-    return ((MethodTree) tree).getModifiers().getFlags().contains(ABSTRACT);
+    return Description.NO_MATCH;
   }
 }
diff --git a/core/src/main/java/com/google/errorprone/bugpatterns/inject/JavaxInjectOnFinalField.java b/core/src/main/java/com/google/errorprone/bugpatterns/inject/JavaxInjectOnFinalField.java
@@ -19,6 +19,7 @@
 import static com.google.errorprone.BugPattern.Category.INJECT;
 import static com.google.errorprone.BugPattern.MaturityLevel.EXPERIMENTAL;
 import static com.google.errorprone.BugPattern.SeverityLevel.ERROR;
+import static com.google.errorprone.matchers.InjectMatchers.IS_APPLICATION_OF_JAVAX_INJECT;
 import static com.sun.source.tree.Tree.Kind.VARIABLE;
 import static javax.lang.model.element.Modifier.FINAL;
 
@@ -27,7 +28,6 @@
 import com.google.errorprone.bugpatterns.BugChecker;
 import com.google.errorprone.bugpatterns.BugChecker.AnnotationTreeMatcher;
 import com.google.errorprone.fixes.SuggestedFix;
-import com.google.errorprone.matchers.AnnotationType;
 import com.google.errorprone.matchers.Description;
 import com.google.errorprone.matchers.Matcher;
 import com.google.errorprone.util.ASTHelpers;
@@ -51,8 +51,6 @@
 )
 public class JavaxInjectOnFinalField extends BugChecker implements AnnotationTreeMatcher {
 
-  private static final String JAVAX_INJECT_ANNOTATION = "javax.inject.Inject";
-
   private static final Matcher<Tree> FINAL_FIELD_MATCHER =
       new Matcher<Tree>() {
         @Override
@@ -61,12 +59,9 @@ public boolean matches(Tree t, VisitorState state) {
         }
       };
 
-  private static final Matcher<AnnotationTree> JAVAX_INJECT_ANNOTATION_MATCHER =
-      new AnnotationType(JAVAX_INJECT_ANNOTATION);
-
   @Override
   public Description matchAnnotation(AnnotationTree annotationTree, VisitorState state) {
-    if (JAVAX_INJECT_ANNOTATION_MATCHER.matches(annotationTree, state)
+    if (IS_APPLICATION_OF_JAVAX_INJECT.matches(annotationTree, state)
         && FINAL_FIELD_MATCHER.matches(getAnnotatedNode(state), state)) {
       return describeMatch(annotationTree, SuggestedFix.delete(annotationTree));
     }

diff --git a/.../main/java/com/google/errorprone/bugpatterns/inject/MoreThanOneInjectableConstructor.java b/.../main/java/com/google/errorprone/bugpatterns/inject/MoreThanOneInjectableConstructor.java
@@ -19,15 +19,13 @@
 import static com.google.errorprone.BugPattern.Category.INJECT;
 import static com.google.errorprone.BugPattern.MaturityLevel.MATURE;
 import static com.google.errorprone.BugPattern.SeverityLevel.ERROR;
-import static com.google.errorprone.matchers.Matchers.hasAnnotation;
+import static com.google.errorprone.matchers.InjectMatchers.hasInjectAnnotation;
 
 import com.google.errorprone.BugPattern;
 import com.google.errorprone.VisitorState;
 import com.google.errorprone.bugpatterns.BugChecker;
 import com.google.errorprone.bugpatterns.BugChecker.ClassTreeMatcher;
 import com.google.errorprone.matchers.Description;
-import com.google.errorprone.matchers.Matcher;
-import com.google.errorprone.matchers.Matchers;
 import com.google.errorprone.util.ASTHelpers;
 import com.sun.source.tree.ClassTree;
 import com.sun.source.tree.MethodTree;
@@ -54,15 +52,11 @@
 )
 public class MoreThanOneInjectableConstructor extends BugChecker implements ClassTreeMatcher {
 
-  private static final Matcher<MethodTree> INJECTABLE_METHOD_MATCHER =
-      Matchers.anyOf(
-          hasAnnotation("com.google.inject.Inject"), hasAnnotation("javax.inject.Inject"));
-
   @Override
   public Description matchClass(ClassTree classTree, VisitorState state) {
     boolean hasInjectConstructor = false;
     for (MethodTree constructor : ASTHelpers.getConstructors(classTree)) {
-      if (INJECTABLE_METHOD_MATCHER.matches(constructor, state)) {
+      if (hasInjectAnnotation().matches(constructor, state)) {
         if (hasInjectConstructor) {
           return describeMatch(classTree);
         }

diff --git a/core/src/main/java/com/google/errorprone/bugpatterns/inject/MoreThanOneQualifier.java b/core/src/main/java/com/google/errorprone/bugpatterns/inject/MoreThanOneQualifier.java
@@ -19,6 +19,8 @@
 import static com.google.errorprone.BugPattern.Category.INJECT;
 import static com.google.errorprone.BugPattern.MaturityLevel.EXPERIMENTAL;
 import static com.google.errorprone.BugPattern.SeverityLevel.ERROR;
+import static com.google.errorprone.matchers.InjectMatchers.GUICE_BINDING_ANNOTATION;
+import static com.google.errorprone.matchers.InjectMatchers.JAVAX_QUALIFIER_ANNOTATION;
 import static com.google.errorprone.matchers.Matchers.hasAnnotation;
 
 import com.google.errorprone.BugPattern;
@@ -45,12 +47,10 @@
   maturity = EXPERIMENTAL
 )
 public class MoreThanOneQualifier extends BugChecker implements AnnotationTreeMatcher {
-  private static final String GUICE_BINDING_ANNOTATION = "com.google.inject.BindingAnnotation";
-  private static final String JAVAX_QUALIFER_ANNOTATION = "javax.inject.Qualifier";
 
   private static final Matcher<AnnotationTree> QUALIFIER_ANNOTATION_MATCHER =
-      Matchers.<AnnotationTree>anyOf(
-          hasAnnotation(GUICE_BINDING_ANNOTATION), hasAnnotation(JAVAX_QUALIFER_ANNOTATION));
+      Matchers.anyOf(
+          hasAnnotation(GUICE_BINDING_ANNOTATION), hasAnnotation(JAVAX_QUALIFIER_ANNOTATION));
 
   @Override
   public Description matchAnnotation(AnnotationTree annotationTree, VisitorState state) {

diff --git a/...main/java/com/google/errorprone/bugpatterns/inject/MoreThanOneScopeAnnotationOnClass.java b/...main/java/com/google/errorprone/bugpatterns/inject/MoreThanOneScopeAnnotationOnClass.java
@@ -19,6 +19,8 @@
 import static com.google.errorprone.BugPattern.Category.INJECT;
 import static com.google.errorprone.BugPattern.MaturityLevel.EXPERIMENTAL;
 import static com.google.errorprone.BugPattern.SeverityLevel.ERROR;
+import static com.google.errorprone.matchers.InjectMatchers.GUICE_SCOPE_ANNOTATION;
+import static com.google.errorprone.matchers.InjectMatchers.JAVAX_SCOPE_ANNOTATION;
 import static com.google.errorprone.matchers.Matchers.hasAnnotation;
 
 import com.google.errorprone.BugPattern;
@@ -52,9 +54,6 @@
 )
 public class MoreThanOneScopeAnnotationOnClass extends BugChecker implements AnnotationTreeMatcher {
 
-  private static final String GUICE_SCOPE_ANNOTATION = "com.google.inject.ScopeAnnotation";
-  private static final String JAVAX_SCOPE_ANNOTATION = "javax.inject.Scope";
-
   /**
    * Matches annotations that are themselves annotated with with
    * {@code @ScopeAnnotation(Guice)} or {@code @Scope(Javax)}.