IdentifierNaming: consider local classes to be private, and issue ren…

…ames for renameable classes. PiperOrigin-RevId: 719351125
google · Jan 24, 2025 · 05f9422 · 05f9422
1 parent 5db6f38
commit 05f9422
Show file tree

Hide file tree

Showing 5 changed files with 54 additions and 8 deletions.
diff --git a/check_api/src/main/java/com/google/errorprone/util/ASTHelpers.java b/check_api/src/main/java/com/google/errorprone/util/ASTHelpers.java
@@ -359,7 +359,9 @@ public static boolean canBeRemoved(ClassSymbol symbol) {
 
   /** Returns whether this symbol or any of its owners are private. */
   public static boolean isEffectivelyPrivate(Symbol symbol) {
-    return enclosingElements(symbol).anyMatch(Symbol::isPrivate);
+    return enclosingElements(symbol)
+        .anyMatch(
+            s -> s.isPrivate() || (s instanceof ClassSymbol && s.owner instanceof MethodSymbol));
   }
 
   /** Checks whether an expression requires parentheses. */

diff --git a/check_api/src/test/java/com/google/errorprone/util/ASTHelpersTest.java b/check_api/src/test/java/com/google/errorprone/util/ASTHelpersTest.java
@@ -43,6 +43,7 @@
 import com.google.errorprone.CompilationTestHelper;
 import com.google.errorprone.VisitorState;
 import com.google.errorprone.bugpatterns.BugChecker;
+import com.google.errorprone.bugpatterns.BugChecker.ClassTreeMatcher;
 import com.google.errorprone.bugpatterns.BugChecker.IdentifierTreeMatcher;
 import com.google.errorprone.bugpatterns.BugChecker.MemberReferenceTreeMatcher;
 import com.google.errorprone.bugpatterns.BugChecker.MethodInvocationTreeMatcher;
@@ -1762,7 +1763,14 @@ private void t() {
   /** Helper for testing {@link ASTHelpers#canBeRemoved}. */
   @BugPattern(summary = "", severity = WARNING)
   public static final class VisibleMembers extends BugChecker
-      implements MethodTreeMatcher, VariableTreeMatcher {
+      implements ClassTreeMatcher, MethodTreeMatcher, VariableTreeMatcher {
+
+    @Override
+    public Description matchClass(ClassTree tree, VisitorState state) {
+      return ASTHelpers.canBeRemoved(ASTHelpers.getSymbol(tree), state)
+          ? describeMatch(tree)
+          : Description.NO_MATCH;
+    }
 
     @Override
     public Description matchMethod(MethodTree tree, VisitorState state) {
@@ -1788,6 +1796,16 @@ public void visibleMembers() {
             class Test {
               // BUG: Diagnostic contains:
               private Test t;
+              public void foo() {
+                // BUG: Diagnostic contains:
+                class Foo {
+                  // BUG: Diagnostic contains:
+                  class Bar {}
+                  // BUG: Diagnostic contains:
+                  public void bar() {}
+                }
+              }
+              // BUG: Diagnostic contains:
               private class Inner {
                 // BUG: Diagnostic contains:
                 public Test t;

diff --git a/core/src/main/java/com/google/errorprone/bugpatterns/IdentifierName.java b/core/src/main/java/com/google/errorprone/bugpatterns/IdentifierName.java
@@ -21,6 +21,7 @@
 import static com.google.errorprone.BugPattern.LinkType.CUSTOM;
 import static com.google.errorprone.BugPattern.SeverityLevel.WARNING;
 import static com.google.errorprone.fixes.SuggestedFix.emptyFix;
+import static com.google.errorprone.fixes.SuggestedFixes.renameClassWithUses;
 import static com.google.errorprone.fixes.SuggestedFixes.renameMethodWithInvocations;
 import static com.google.errorprone.fixes.SuggestedFixes.renameVariable;
 import static com.google.errorprone.matchers.Description.NO_MATCH;
@@ -128,7 +129,7 @@ public Description matchClass(ClassTree tree, VisitorState state) {
             fixable
                 ? diagnostic
                 : diagnostic + String.format("; did you" + " mean '%s'?", suggested))
-        .addFix(emptyFix())
+        .addFix(fixable ? renameClassWithUses(tree, suggested, state) : emptyFix())
         .build();
   }
 

diff --git a/core/src/test/java/com/google/errorprone/bugpatterns/IdentifierNameTest.java b/core/src/test/java/com/google/errorprone/bugpatterns/IdentifierNameTest.java
@@ -154,6 +154,30 @@ int get() {
         .doTest();
   }
 
+  @Test
+  public void localClass_renamed() {
+    refactoringHelper
+        .addInputLines(
+            "Test.java",
+            """
+            class Test {
+              public void get() {
+                class MisnamedURLVisitor {}
+              }
+            }
+            """)
+        .addOutputLines(
+            "Test.java",
+            """
+            class Test {
+              public void get() {
+                class MisnamedUrlVisitor {}
+              }
+            }
+            """)
+        .doTest();
+  }
+
   @Test
   public void resourceVariable_renamed() {
     refactoringHelper

diff --git a/core/src/test/java/com/google/errorprone/bugpatterns/UnusedVariableTest.java b/core/src/test/java/com/google/errorprone/bugpatterns/UnusedVariableTest.java
@@ -1805,13 +1805,14 @@ class Test {
               public void test(List<Integer> xs) {
                 // BUG: Diagnostic contains: 'b' is never read
                 Collections.sort(xs, (a, b) -> a > a ? 1 : 0);
-                Collections.sort(xs, new Comparator<Integer>() {
-                    // BUG: Diagnostic contains: 'b' is never read
-                    @Override public int compare(Integer a, Integer b) { return a; }
-                    public void foo(int a, int b) {}
-                });
                 Collections.sort(xs, (a, unused) -> a > a ? 1 : 0);
               }
+
+              public class TestComparator implements Comparator<Integer> {
+                // BUG: Diagnostic contains: 'b' is never read
+                @Override public int compare(Integer a, Integer b) { return a; }
+                public void foo(int a, int b) {}
+              }
             }
             """)
         .doTest();