ASTHelpers: pull isMethodCanBeOverridden out of MutableMethodReturnTy…

…pe. Handle some things like enums, constructors a bit better.

RELNOTES: Add isMethodCanBeOverridden to ASTHelpers.

-------------
Created by MOE: https://github.com/google/moe
MOE_MIGRATED_REVID=206321209

check_api/src/main/java/com/google/errorprone/util/ASTHelpers.java

@@ -126,6 +126,7 @@
 import javax.lang.model.element.AnnotationMirror;
 import javax.lang.model.element.AnnotationValue;
 import javax.lang.model.element.ElementKind;
+import javax.lang.model.element.Modifier;
 import javax.lang.model.type.TypeKind;
 import javax.lang.model.util.SimpleAnnotationValueVisitor8;
 
@@ -595,6 +596,31 @@ public static Set<MethodSymbol> findMatchingMethods(
     return matchingMethods;
   }
 
+  /**
+   * Determines whether a method can be overridden.
+   *
+   * @return true if the method can be overridden.
+   */
+  public static boolean methodCanBeOverridden(MethodSymbol methodSymbol) {
+    if (methodSymbol.getModifiers().contains(Modifier.ABSTRACT)) {
+      return true;
+    }
+
+    if (methodSymbol.isStatic()
+        || methodSymbol.isPrivate()
+        || isFinal(methodSymbol)
+        || methodSymbol.isConstructor()) {
+      return false;
+    }
+
+    ClassSymbol classSymbol = (ClassSymbol) methodSymbol.owner;
+    return !isFinal(classSymbol) && !classSymbol.isAnonymous();
+  }
+
+  private static boolean isFinal(Symbol symbol) {
+    return (symbol.flags() & Flags.FINAL) == Flags.FINAL;
+  }
+
   /**
    * Determines whether a symbol has an annotation of the given type. This includes annotations
    * inherited from superclasses due to {@code @Inherited}.

core/src/main/java/com/google/errorprone/bugpatterns/MutableMethodReturnType.java

@@ -33,21 +33,18 @@
 import com.google.errorprone.matchers.Description;
 import com.google.errorprone.matchers.Matcher;
 import com.google.errorprone.util.ASTHelpers;
-import com.sun.source.tree.ClassTree;
 import com.sun.source.tree.IdentifierTree;
 import com.sun.source.tree.MethodTree;
 import com.sun.source.tree.ParameterizedTypeTree;
 import com.sun.source.tree.ReturnTree;
 import com.sun.source.tree.Tree;
 import com.sun.source.util.SimpleTreeVisitor;
 import com.sun.source.util.TreeScanner;
-import com.sun.tools.javac.code.Flags;
 import com.sun.tools.javac.code.Symbol.MethodSymbol;
 import com.sun.tools.javac.code.Type;
 import com.sun.tools.javac.code.Type.ClassType;
 import java.util.Optional;
 import java.util.function.Predicate;
-import javax.lang.model.element.Modifier;
 
 /** @author dorir@google.com (Dori Reuveni) */
 @BugPattern(
@@ -71,7 +68,7 @@ public Description matchMethod(MethodTree methodTree, VisitorState state) {
       return Description.NO_MATCH;
     }
 
-    if (isMethodCanBeOverridden(methodSymbol, state)) {
+    if (ASTHelpers.methodCanBeOverridden(methodSymbol)) {
       return Description.NO_MATCH;
     }
 
@@ -114,28 +111,6 @@ public Description matchMethod(MethodTree methodTree, VisitorState state) {
     return describeMatch(methodTree.getReturnType(), fix);
   }
 
-  private static boolean isMethodCanBeOverridden(MethodSymbol methodSymbol, VisitorState state) {
-    if (methodSymbol.isStatic() || methodSymbol.isPrivate() || isFinalMethod(methodSymbol)) {
-      return false;
-    }
-
-    ClassTree enclosingClassTree = ASTHelpers.findEnclosingNode(state.getPath(), ClassTree.class);
-    boolean isEnclosingClassFinal = isFinalClass(enclosingClassTree);
-    if (isEnclosingClassFinal) {
-      return false;
-    }
-
-    return true;
-  }
-
-  private static boolean isFinalClass(ClassTree classTree) {
-    return classTree.getModifiers().getFlags().contains(Modifier.FINAL);
-  }
-
-  private static boolean isFinalMethod(MethodSymbol methodSymbol) {
-    return (methodSymbol.flags() & Flags.FINAL) == Flags.FINAL;
-  }
-
   private static Optional<String> getCommonImmutableTypeForAllReturnStatementsTypes(
       ImmutableSet<ClassType> returnStatementsTypes) {
     checkState(!returnStatementsTypes.isEmpty());

core/src/test/java/com/google/errorprone/util/ASTHelpersTest.java

@@ -34,13 +34,16 @@
 import com.google.common.io.ByteStreams;
 import com.google.errorprone.BugPattern;
 import com.google.errorprone.BugPattern.Category;
+import com.google.errorprone.BugPattern.ProvidesFix;
 import com.google.errorprone.BugPattern.SeverityLevel;
 import com.google.errorprone.CompilationTestHelper;
 import com.google.errorprone.VisitorState;
 import com.google.errorprone.bugpatterns.BugChecker;
 import com.google.errorprone.bugpatterns.BugChecker.MethodInvocationTreeMatcher;
+import com.google.errorprone.bugpatterns.BugChecker.MethodTreeMatcher;
 import com.google.errorprone.bugpatterns.BugChecker.ParameterizedTypeTreeMatcher;
 import com.google.errorprone.bugpatterns.BugChecker.VariableTreeMatcher;
+import com.google.errorprone.fixes.SuggestedFix;
 import com.google.errorprone.matchers.CompilerBasedAbstractTest;
 import com.google.errorprone.matchers.Description;
 import com.google.errorprone.matchers.Matcher;
@@ -1035,4 +1038,99 @@ public void getDeclarationAndTypeAttributesTest() {
             "@com.google.errorprone.util.ASTHelpersTest.B(\"four\")",
             "@com.google.errorprone.util.ASTHelpersTest.A(10)");
   }
+
+  /** A {@link BugChecker} that prints if the method can be overridden. */
+  @BugPattern(
+      name = "MethodCanBeOverriddenChecker",
+      category = Category.ONE_OFF,
+      severity = SeverityLevel.ERROR,
+      summary = "Prints whether the method can be overridden.",
+      providesFix = ProvidesFix.REQUIRES_HUMAN_ATTENTION)
+  public static class MethodCanBeOverriddenChecker extends BugChecker implements MethodTreeMatcher {
+    @Override
+    public Description matchMethod(MethodTree tree, VisitorState state) {
+      boolean methodCanBeOverridden = ASTHelpers.methodCanBeOverridden(ASTHelpers.getSymbol(tree));
+      String description = methodCanBeOverridden ? "Can be overridden" : "Cannot be overridden";
+      return describeMatch(tree, SuggestedFix.prefixWith(tree, "/* " + description + " */\n"));
+    }
+  }
+
+  @Test
+  public void methodCanBeOverridden_class() {
+    CompilationTestHelper.newInstance(MethodCanBeOverriddenChecker.class, getClass())
+        .addSourceLines(
+            "Test.java",
+            "class Test {",
+            "  // BUG: Diagnostic contains: Cannot be overridden",
+            "  Test() {}",
+            "",
+            "  // BUG: Diagnostic contains: Can be overridden",
+            "  void canBeOverridden() {}",
+            "",
+            "  // BUG: Diagnostic contains: Cannot be overridden",
+            "  final void cannotBeOverriddenBecauseFinal() {}",
+            "",
+            "  // BUG: Diagnostic contains: Cannot be overridden",
+            "  static void cannotBeOverriddenBecauseStatic() {}",
+            "",
+            "  // BUG: Diagnostic contains: Cannot be overridden",
+            "  private void cannotBeOverriddenBecausePrivate() {}",
+            "}")
+        .doTest();
+  }
+
+  @Test
+  public void methodCanBeOverridden_interface() {
+    CompilationTestHelper.newInstance(MethodCanBeOverriddenChecker.class, getClass())
+        .addSourceLines(
+            "Test.java",
+            "interface Test {",
+            "  // BUG: Diagnostic contains: Can be overridden",
+            "  void canBeOverridden();",
+            "",
+            "  // BUG: Diagnostic contains: Can be overridden",
+            "  default void defaultCanBeOverridden() {}",
+            "",
+            "  // BUG: Diagnostic contains: Cannot be overridden",
+            "  static void cannotBeOverriddenBecauseStatic() {}",
+            "}")
+        .doTest();
+  }
+
+  @Test
+  public void methodCanBeOverridden_enum() {
+    CompilationTestHelper.newInstance(MethodCanBeOverriddenChecker.class, getClass())
+        .addSourceLines(
+            "Test.java",
+            "enum Test {",
+            "  VALUE {",
+            "    // BUG: Diagnostic contains: Cannot be overridden",
+            "    @Override void abstractCanBeOverridden() {}",
+            "",
+            "    // BUG: Diagnostic contains: Cannot be overridden",
+            "    void declaredOnlyInValue() {}",
+            "  };",
+            "",
+            "  // BUG: Diagnostic contains: Can be overridden",
+            "  abstract void abstractCanBeOverridden();",
+            "",
+            "  // BUG: Diagnostic contains: Can be overridden",
+            "  void canBeOverridden() {}",
+            "}")
+        .doTest();
+  }
+
+  @Test
+  public void methodCanBeOverridden_anonymous() {
+    CompilationTestHelper.newInstance(MethodCanBeOverriddenChecker.class, getClass())
+        .addSourceLines(
+            "Test.java",
+            "class Test {",
+            "  Object obj = new Object() {",
+            "    // BUG: Diagnostic contains: Cannot be overridden",
+            "    void inAnonymousClass() {}",
+            "  };",
+            "}")
+        .doTest();
+  }
 }