From 39609bf269465e2a1204aa3039fbd6ea66a9b067 Mon Sep 17 00:00:00 2001 From: Liam Miller-Cushon Date: Wed, 5 Jun 2024 11:17:02 -0700 Subject: [PATCH] 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 --- annotations/src/main/java/module-info.java | 22 ----------- .../google/errorprone/util/SourceVersion.java | 5 +++ .../bugpatterns/ClassCanBeStatic.java | 10 ++++- .../bugpatterns/FieldCanBeStatic.java | 4 +- .../bugpatterns/MethodCanBeStatic.java | 8 +++- .../bugpatterns/ClassCanBeStaticTest.java | 27 ++++++++++++- .../bugpatterns/FieldCanBeStaticTest.java | 39 +++++++++++++++++++ .../bugpatterns/MethodCanBeStaticTest.java | 39 +++++++++++++++++++ 8 files changed, 127 insertions(+), 27 deletions(-) delete mode 100644 annotations/src/main/java/module-info.java diff --git a/annotations/src/main/java/module-info.java b/annotations/src/main/java/module-info.java deleted file mode 100644 index 3dccd229104..00000000000 --- a/annotations/src/main/java/module-info.java +++ /dev/null @@ -1,22 +0,0 @@ -/* - * Copyright 2015 The Error Prone Authors. - * - * 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. - */ - -open module com.google.errorprone.annotations { - requires java.compiler; - - exports com.google.errorprone.annotations; - exports com.google.errorprone.annotations.concurrent; -} 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 index bceb86a51e4..2eeddb37210 100644 --- 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 index b31e676bdbf..9cfb3e0f044 100644 --- 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 index 1232efb9745..6de6094ad0b 100644 --- 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 index 522a4bc442f..d6d1b2dc0f6 100644 --- 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 index a8335705647..71a385a15e9 100644 --- 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 index 765966bbab3..4e5a9319dbf 100644 --- 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 index 64276026d0e..2a929f7bd1c 100644 --- 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(); }