Handle static members in inner classes in *CanBeStatic checks

In Java 16 and newer, static members can be declared in inner classes: https://bugs.openjdk.org/browse/JDK-8254321 PiperOrigin-RevId: 640587003
google · Jun 5, 2024 · 39609bf · 39609bf
1 parent 3675b47
commit 39609bf
Show file tree

Hide file tree

Showing 8 changed files with 127 additions and 27 deletions.
diff --git a/annotations/src/main/java/module-info.java b/annotations/src/main/java/module-info.java
diff --git a/check_api/src/main/java/com/google/errorprone/util/SourceVersion.java b/check_api/src/main/java/com/google/errorprone/util/SourceVersion.java
@@ -45,6 +45,11 @@ public static boolean supportsPatternMatchingInstanceof(Context context) {
     return sourceIsAtLeast(context, 21);
   }
 
+  /** Returns true if the compiler source version level supports static inner classes. */
+  public static boolean supportsStaticInnerClass(Context context) {
+    return sourceIsAtLeast(context, 16);
+  }
+
   private static boolean sourceIsAtLeast(Context context, int version) {
     Source lowerBound = Source.lookup(Integer.toString(version));
     return lowerBound != null && Source.instance(context).compareTo(lowerBound) >= 0;

diff --git a/core/src/main/java/com/google/errorprone/bugpatterns/ClassCanBeStatic.java b/core/src/main/java/com/google/errorprone/bugpatterns/ClassCanBeStatic.java
@@ -28,6 +28,7 @@
 import com.google.errorprone.fixes.SuggestedFixes;
 import com.google.errorprone.matchers.Description;
 import com.google.errorprone.util.ASTHelpers;
+import com.google.errorprone.util.SourceVersion;
 import com.sun.source.tree.ClassTree;
 import com.sun.tools.javac.code.Symbol.ClassSymbol;
 import javax.lang.model.element.Modifier;
@@ -60,12 +61,17 @@ public Description matchClass(ClassTree tree, VisitorState state) {
       case TOP_LEVEL:
         break;
       case MEMBER:
-        // class is nested inside an inner class, so it can't be static
-        if (currentClass.owner.enclClass().hasOuterInstance()) {
+        if (!SourceVersion.supportsStaticInnerClass(state.context)
+            && currentClass.owner.enclClass().hasOuterInstance()) {
+          // class is nested inside an inner class, so it can't be static
           return NO_MATCH;
         }
         break;
       case LOCAL:
+        if (!SourceVersion.supportsStaticInnerClass(state.context)) {
+          return NO_MATCH;
+        }
+        break;
       case ANONYMOUS:
         // members of local and anonymous classes can't be static
         return NO_MATCH;

diff --git a/core/src/main/java/com/google/errorprone/bugpatterns/FieldCanBeStatic.java b/core/src/main/java/com/google/errorprone/bugpatterns/FieldCanBeStatic.java
@@ -45,6 +45,7 @@
 import com.google.errorprone.matchers.Description;
 import com.google.errorprone.suppliers.Supplier;
 import com.google.errorprone.util.ASTHelpers;
+import com.google.errorprone.util.SourceVersion;
 import com.sun.source.tree.ExpressionTree;
 import com.sun.source.tree.IdentifierTree;
 import com.sun.source.tree.MemberSelectTree;
@@ -102,7 +103,8 @@ public Description matchVariable(VariableTree tree, VisitorState state) {
     if (enclClass == null) {
       return NO_MATCH;
     }
-    if (!enclClass.getNestingKind().equals(NestingKind.TOP_LEVEL)
+    if (!SourceVersion.supportsStaticInnerClass(state.context)
+        && !enclClass.getNestingKind().equals(NestingKind.TOP_LEVEL)
         && !enclClass.isStatic()
         && symbol.getConstantValue() == null) {
       // JLS 8.1.3: inner classes cannot declare static members, unless the member is a constant

diff --git a/core/src/main/java/com/google/errorprone/bugpatterns/MethodCanBeStatic.java b/core/src/main/java/com/google/errorprone/bugpatterns/MethodCanBeStatic.java
@@ -38,6 +38,7 @@
 import com.google.errorprone.fixes.SuggestedFix;
 import com.google.errorprone.matchers.Description;
 import com.google.errorprone.util.ASTHelpers;
+import com.google.errorprone.util.SourceVersion;
 import com.sun.source.tree.ClassTree;
 import com.sun.source.tree.CompilationUnitTree;
 import com.sun.source.tree.ExpressionTree;
@@ -242,11 +243,16 @@ private static boolean isExcluded(MethodTree tree, VisitorState state) {
       case TOP_LEVEL:
         break;
       case MEMBER:
-        if (sym.owner.enclClass().hasOuterInstance()) {
+        if (!SourceVersion.supportsStaticInnerClass(state.context)
+            && sym.owner.enclClass().hasOuterInstance()) {
           return true;
         }
         break;
       case LOCAL:
+        if (!SourceVersion.supportsStaticInnerClass(state.context)) {
+          return true;
+        }
+        break;
       case ANONYMOUS:
         return true;
     }

diff --git a/core/src/test/java/com/google/errorprone/bugpatterns/ClassCanBeStaticTest.java b/core/src/test/java/com/google/errorprone/bugpatterns/ClassCanBeStaticTest.java
@@ -16,7 +16,10 @@
 
 package com.google.errorprone.bugpatterns;
 
+import static org.junit.Assume.assumeTrue;
+
 import com.google.errorprone.CompilationTestHelper;
+import com.google.errorprone.util.RuntimeVersion;
 import org.junit.Test;
 import org.junit.runner.RunWith;
 import org.junit.runners.JUnit4;
@@ -30,7 +33,10 @@ public class ClassCanBeStaticTest {
 
   @Test
   public void negativeCase() {
-    compilationHelper.addSourceFile("ClassCanBeStaticNegativeCases.java").doTest();
+    compilationHelper
+        .addSourceFile("ClassCanBeStaticNegativeCases.java")
+        .setArgs("--release", "11")
+        .doTest();
   }
 
   @Test
@@ -334,6 +340,25 @@ public void nestedInLocal() {
             "    }",
             "  }",
             "}")
+        .setArgs("--release", "11")
+        .doTest();
+  }
+
+  @Test
+  public void nestedInLocal_static() {
+    assumeTrue(RuntimeVersion.isAtLeast16());
+    compilationHelper
+        .addSourceLines(
+            "A.java", //
+            "public class A {",
+            "  static void f() {",
+            "    class Outer {",
+            "      // BUG: Diagnostic contains:",
+            "      class Inner {",
+            "      }",
+            "    }",
+            "  }",
+            "}")
         .doTest();
   }
 

diff --git a/core/src/test/java/com/google/errorprone/bugpatterns/FieldCanBeStaticTest.java b/core/src/test/java/com/google/errorprone/bugpatterns/FieldCanBeStaticTest.java
@@ -285,6 +285,45 @@ public void inner() {
             "    };",
             "  }",
             "}")
+        .setArgs("--release", "11")
+        .doTest();
+  }
+
+  @Test
+  public void inner_static() {
+    assumeTrue(RuntimeVersion.isAtLeast16());
+    compilationHelper
+        .addSourceLines(
+            "Test.java",
+            "import java.time.Duration;",
+            "class Test {",
+            "  class I {",
+            "    // BUG: Diagnostic contains: can be static",
+            "    private final Duration D = Duration.ofMillis(1);",
+            "    // BUG: Diagnostic contains: can be static",
+            "    private final int I = 42;",
+            "  }",
+            "  static class S {",
+            "    // BUG: Diagnostic contains: can be static",
+            "    private final Duration D = Duration.ofMillis(1);",
+            "    // BUG: Diagnostic contains: can be static",
+            "    private final int I = 42;",
+            "  }",
+            "  void f() {",
+            "    class L {",
+            "      // BUG: Diagnostic contains: can be static",
+            "      private final Duration D = Duration.ofMillis(1);",
+            "      // BUG: Diagnostic contains: can be static",
+            "      private final int I = 42;",
+            "    }",
+            "    new Object() {",
+            "      // BUG: Diagnostic contains: can be static",
+            "      private final Duration D = Duration.ofMillis(1);",
+            "      // BUG: Diagnostic contains: can be static",
+            "      private final int I = 42;",
+            "    };",
+            "  }",
+            "}")
         .doTest();
   }
 

diff --git a/core/src/test/java/com/google/errorprone/bugpatterns/MethodCanBeStaticTest.java b/core/src/test/java/com/google/errorprone/bugpatterns/MethodCanBeStaticTest.java
@@ -16,9 +16,12 @@
 
 package com.google.errorprone.bugpatterns;
 
+import static org.junit.Assume.assumeTrue;
+
 import com.google.common.collect.ImmutableList;
 import com.google.errorprone.BugCheckerRefactoringTestHelper;
 import com.google.errorprone.CompilationTestHelper;
+import com.google.errorprone.util.RuntimeVersion;
 import org.junit.Test;
 import org.junit.runner.RunWith;
 import org.junit.runners.JUnit4;
@@ -327,6 +330,24 @@ public void innerClass() {
             "    }",
             "  }",
             "}")
+        .setArgs("--release", "11")
+        .doTest();
+  }
+
+  @Test
+  public void innerClass_static() {
+    assumeTrue(RuntimeVersion.isAtLeast16());
+    testHelper
+        .addSourceLines(
+            "Test.java", //
+            "class Test {",
+            "  class Inner {",
+            "    // BUG: Diagnostic contains: static",
+            "    private int incr(int x) {",
+            "      return x + 1;",
+            "    }",
+            "  }",
+            "}")
         .doTest();
   }
 
@@ -370,6 +391,24 @@ public void negativeLocal() {
             "    }",
             "  }",
             "}")
+        .setArgs("--release", "11")
+        .doTest();
+  }
+
+  @Test
+  public void positiveLocal() {
+    assumeTrue(RuntimeVersion.isAtLeast16());
+    testHelper
+        .addSourceLines(
+            "Test.java", //
+            "class Test {",
+            "  static void foo() {",
+            "    class Local {",
+            "      // BUG: Diagnostic contains: static",
+            "      private void foo() {}",
+            "    }",
+            "  }",
+            "}")
         .doTest();
   }