From 2a0bb07d5e4640c402ac8f1357528af5e6093db6 Mon Sep 17 00:00:00 2001 From: Stephan Schroevers Date: Sat, 31 Dec 2022 15:27:07 +0100 Subject: [PATCH 1/5] StatementSwitchToExpressionSwitch: only trigger on compatible target versions This change introduces a `TargetVersion` utility class, analogous to the existing `RuntimeVersion` class. The new class tells whether the current compilation process targets a bytecode version that is generally associated with a given (or more recent) JDK version. The new utility class is used to make sure that `StatementSwitchToExpressionSwitch` flags statement switches only if the target version supports expression switches, irrespective of whether the current runtime supports such switch types. Concretely this makes the check compatible with projects that target JDK 11, yet support building with JDK 17. --- .../errorprone/util/RuntimeVersion.java | 7 +- .../google/errorprone/util/TargetVersion.java | 88 +++++++++++++++++++ .../StatementSwitchToExpressionSwitch.java | 3 +- pom.xml | 5 +- 4 files changed, 98 insertions(+), 5 deletions(-) create mode 100644 check_api/src/main/java/com/google/errorprone/util/TargetVersion.java diff --git a/check_api/src/main/java/com/google/errorprone/util/RuntimeVersion.java b/check_api/src/main/java/com/google/errorprone/util/RuntimeVersion.java index f965d3f625c..e83cc4b1af0 100644 --- a/check_api/src/main/java/com/google/errorprone/util/RuntimeVersion.java +++ b/check_api/src/main/java/com/google/errorprone/util/RuntimeVersion.java @@ -16,9 +16,12 @@ package com.google.errorprone.util; -/** JDK version string utilities. */ +/** + * JDK runtime version utilities. + * + * @see TargetVersion + */ public final class RuntimeVersion { - private static final int FEATURE = Runtime.version().feature(); /** Returns true if the current runtime is JDK 12 or newer. */ diff --git a/check_api/src/main/java/com/google/errorprone/util/TargetVersion.java b/check_api/src/main/java/com/google/errorprone/util/TargetVersion.java new file mode 100644 index 00000000000..fcc2eefb66e --- /dev/null +++ b/check_api/src/main/java/com/google/errorprone/util/TargetVersion.java @@ -0,0 +1,88 @@ +/* + * Copyright 2023 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. + */ + +package com.google.errorprone.util; + +import com.sun.tools.javac.jvm.Target; +import com.sun.tools.javac.util.Context; + +/** + * JDK target version utilities. + * + * @see RuntimeVersion + */ +public final class TargetVersion { + /** Returns true if the compiler targets JRE 11 or newer. */ + public static boolean isAtLeast11(Context context) { + return majorVersion(context) >= 55; + } + + /** Returns true if the compiler targets JRE 12 or newer. */ + public static boolean isAtLeast12(Context context) { + return majorVersion(context) >= 56; + } + + /** Returns true if the compiler targets JRE 13 or newer. */ + public static boolean isAtLeast13(Context context) { + return majorVersion(context) >= 57; + } + + /** Returns true if the compiler targets JRE 14 or newer. */ + public static boolean isAtLeast14(Context context) { + return majorVersion(context) >= 58; + } + + /** Returns true if the compiler targets JRE 15 or newer. */ + public static boolean isAtLeast15(Context context) { + return majorVersion(context) >= 59; + } + + /** Returns true if the compiler targets JRE 16 or newer. */ + public static boolean isAtLeast16(Context context) { + return majorVersion(context) >= 60; + } + + /** Returns true if the compiler targets JRE 17 or newer. */ + public static boolean isAtLeast17(Context context) { + return majorVersion(context) >= 61; + } + + /** Returns true if the compiler targets JRE 18 or newer. */ + public static boolean isAtLeast18(Context context) { + return majorVersion(context) >= 62; + } + + /** Returns true if the compiler targets JRE 19 or newer. */ + public static boolean isAtLeast19(Context context) { + return majorVersion(context) >= 63; + } + + /** Returns true if the compiler targets JRE 20 or newer. */ + public static boolean isAtLeast20(Context context) { + return majorVersion(context) >= 64; + } + + /** Returns true if the compiler targets JRE 21 or newer. */ + public static boolean isAtLeast21(Context context) { + return majorVersion(context) >= 65; + } + + private static int majorVersion(Context context) { + return Target.instance(context).majorVersion; + } + + private TargetVersion() {} +} diff --git a/core/src/main/java/com/google/errorprone/bugpatterns/StatementSwitchToExpressionSwitch.java b/core/src/main/java/com/google/errorprone/bugpatterns/StatementSwitchToExpressionSwitch.java index b885de8b298..4644f8ac5d4 100644 --- a/core/src/main/java/com/google/errorprone/bugpatterns/StatementSwitchToExpressionSwitch.java +++ b/core/src/main/java/com/google/errorprone/bugpatterns/StatementSwitchToExpressionSwitch.java @@ -39,6 +39,7 @@ import com.google.errorprone.matchers.Description; import com.google.errorprone.util.Reachability; import com.google.errorprone.util.RuntimeVersion; +import com.google.errorprone.util.TargetVersion; import com.sun.source.tree.BreakTree; import com.sun.source.tree.CaseTree; import com.sun.source.tree.ExpressionTree; @@ -86,7 +87,7 @@ public StatementSwitchToExpressionSwitch(ErrorProneFlags flags) { @Override public Description matchSwitch(SwitchTree switchTree, VisitorState state) { // Expression switches finalized in Java 14 - if (!RuntimeVersion.isAtLeast14()) { + if (!TargetVersion.isAtLeast14(state.context)) { return NO_MATCH; } diff --git a/pom.xml b/pom.xml index 515f8b7edb6..51a7df5932c 100644 --- a/pom.xml +++ b/pom.xml @@ -161,10 +161,10 @@ --add-exports=java.base/jdk.internal.javac=ALL-UNNAMED --add-exports=jdk.compiler/com.sun.tools.javac.api=ALL-UNNAMED - --add-exports=jdk.compiler/com.sun.tools.javac.file=ALL-UNNAMED --add-exports=jdk.compiler/com.sun.tools.javac.code=ALL-UNNAMED - --add-exports=jdk.compiler/com.sun.tools.javac.util=ALL-UNNAMED --add-exports=jdk.compiler/com.sun.tools.javac.comp=ALL-UNNAMED + --add-exports=jdk.compiler/com.sun.tools.javac.file=ALL-UNNAMED + --add-exports=jdk.compiler/com.sun.tools.javac.jvm=ALL-UNNAMED --add-exports=jdk.compiler/com.sun.tools.javac.main=ALL-UNNAMED --add-exports=jdk.compiler/com.sun.tools.javac.model=ALL-UNNAMED --add-exports=jdk.compiler/com.sun.tools.javac.parser=ALL-UNNAMED @@ -213,6 +213,7 @@ -Xmx1g --add-exports=jdk.compiler/com.sun.tools.javac.api=ALL-UNNAMED --add-exports=jdk.compiler/com.sun.tools.javac.file=ALL-UNNAMED + --add-exports=jdk.compiler/com.sun.tools.javac.jvm=ALL-UNNAMED --add-exports=jdk.compiler/com.sun.tools.javac.main=ALL-UNNAMED --add-exports=jdk.compiler/com.sun.tools.javac.model=ALL-UNNAMED --add-exports=jdk.compiler/com.sun.tools.javac.parser=ALL-UNNAMED From 42a036c90373f6e7c35190437cfd7d8b99646912 Mon Sep 17 00:00:00 2001 From: Stephan Schroevers Date: Sat, 31 Dec 2022 16:24:55 +0100 Subject: [PATCH 2/5] Alternative implementation --- .../errorprone/util/RuntimeVersion.java | 6 +- .../google/errorprone/util/SourceVersion.java | 58 ++++++++++++ .../google/errorprone/util/TargetVersion.java | 88 ------------------- .../StatementSwitchToExpressionSwitch.java | 5 +- pom.xml | 5 +- 5 files changed, 67 insertions(+), 95 deletions(-) create mode 100644 check_api/src/main/java/com/google/errorprone/util/SourceVersion.java delete mode 100644 check_api/src/main/java/com/google/errorprone/util/TargetVersion.java diff --git a/check_api/src/main/java/com/google/errorprone/util/RuntimeVersion.java b/check_api/src/main/java/com/google/errorprone/util/RuntimeVersion.java index e83cc4b1af0..570c0862efa 100644 --- a/check_api/src/main/java/com/google/errorprone/util/RuntimeVersion.java +++ b/check_api/src/main/java/com/google/errorprone/util/RuntimeVersion.java @@ -19,7 +19,11 @@ /** * JDK runtime version utilities. * - * @see TargetVersion + *

These methods are generally used when deciding which method to call reflectively. Bug checkers + * that rely on support for specific source code constructs should consult {@link SourceVersion} + * instead. + * + * @see RuntimeVersion */ public final class RuntimeVersion { private static final int FEATURE = Runtime.version().feature(); 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 new file mode 100644 index 00000000000..eece907c1e8 --- /dev/null +++ b/check_api/src/main/java/com/google/errorprone/util/SourceVersion.java @@ -0,0 +1,58 @@ +/* + * Copyright 2023 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. + */ + +package com.google.errorprone.util; + +import com.google.common.collect.ImmutableMap; +import com.google.common.collect.Maps; +import com.sun.tools.javac.code.Source; +import com.sun.tools.javac.code.Source.Feature; +import com.sun.tools.javac.util.Context; +import java.util.Arrays; + +/** + * JDK source version utilities. + * + * @see RuntimeVersion + */ +public final class SourceVersion { + private static final ImmutableMap KNOWN_FEATURES = + Maps.uniqueIndex(Arrays.asList(Feature.values()), Enum::name); + + /** Returns true if the compiler source version level supports switch expressions. */ + public static boolean supportsSwitchExpressions(Context context) { + return supportsFeature("SWITCH_EXPRESSION", context); + } + + /** Returns true if the compiler source version level supports text blocks. */ + public static boolean supportsTextBlocks(Context context) { + return supportsFeature("TEXT_BLOCKS", context); + } + + /** + * Returns true if the compiler source version level supports the {@link Feature} indicated by the + * specified string. + * + * @apiNote For features explicitly recognized by this class, prefer calling the associated method + * instead. + */ + public static boolean supportsFeature(String featureString, Context context) { + Feature feature = KNOWN_FEATURES.get(featureString); + return feature != null && feature.allowedInSource(Source.instance(context)); + } + + private SourceVersion() {} +} diff --git a/check_api/src/main/java/com/google/errorprone/util/TargetVersion.java b/check_api/src/main/java/com/google/errorprone/util/TargetVersion.java deleted file mode 100644 index fcc2eefb66e..00000000000 --- a/check_api/src/main/java/com/google/errorprone/util/TargetVersion.java +++ /dev/null @@ -1,88 +0,0 @@ -/* - * Copyright 2023 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. - */ - -package com.google.errorprone.util; - -import com.sun.tools.javac.jvm.Target; -import com.sun.tools.javac.util.Context; - -/** - * JDK target version utilities. - * - * @see RuntimeVersion - */ -public final class TargetVersion { - /** Returns true if the compiler targets JRE 11 or newer. */ - public static boolean isAtLeast11(Context context) { - return majorVersion(context) >= 55; - } - - /** Returns true if the compiler targets JRE 12 or newer. */ - public static boolean isAtLeast12(Context context) { - return majorVersion(context) >= 56; - } - - /** Returns true if the compiler targets JRE 13 or newer. */ - public static boolean isAtLeast13(Context context) { - return majorVersion(context) >= 57; - } - - /** Returns true if the compiler targets JRE 14 or newer. */ - public static boolean isAtLeast14(Context context) { - return majorVersion(context) >= 58; - } - - /** Returns true if the compiler targets JRE 15 or newer. */ - public static boolean isAtLeast15(Context context) { - return majorVersion(context) >= 59; - } - - /** Returns true if the compiler targets JRE 16 or newer. */ - public static boolean isAtLeast16(Context context) { - return majorVersion(context) >= 60; - } - - /** Returns true if the compiler targets JRE 17 or newer. */ - public static boolean isAtLeast17(Context context) { - return majorVersion(context) >= 61; - } - - /** Returns true if the compiler targets JRE 18 or newer. */ - public static boolean isAtLeast18(Context context) { - return majorVersion(context) >= 62; - } - - /** Returns true if the compiler targets JRE 19 or newer. */ - public static boolean isAtLeast19(Context context) { - return majorVersion(context) >= 63; - } - - /** Returns true if the compiler targets JRE 20 or newer. */ - public static boolean isAtLeast20(Context context) { - return majorVersion(context) >= 64; - } - - /** Returns true if the compiler targets JRE 21 or newer. */ - public static boolean isAtLeast21(Context context) { - return majorVersion(context) >= 65; - } - - private static int majorVersion(Context context) { - return Target.instance(context).majorVersion; - } - - private TargetVersion() {} -} diff --git a/core/src/main/java/com/google/errorprone/bugpatterns/StatementSwitchToExpressionSwitch.java b/core/src/main/java/com/google/errorprone/bugpatterns/StatementSwitchToExpressionSwitch.java index 4644f8ac5d4..d600b70b584 100644 --- a/core/src/main/java/com/google/errorprone/bugpatterns/StatementSwitchToExpressionSwitch.java +++ b/core/src/main/java/com/google/errorprone/bugpatterns/StatementSwitchToExpressionSwitch.java @@ -39,7 +39,7 @@ import com.google.errorprone.matchers.Description; import com.google.errorprone.util.Reachability; import com.google.errorprone.util.RuntimeVersion; -import com.google.errorprone.util.TargetVersion; +import com.google.errorprone.util.SourceVersion; import com.sun.source.tree.BreakTree; import com.sun.source.tree.CaseTree; import com.sun.source.tree.ExpressionTree; @@ -86,8 +86,7 @@ public StatementSwitchToExpressionSwitch(ErrorProneFlags flags) { @Override public Description matchSwitch(SwitchTree switchTree, VisitorState state) { - // Expression switches finalized in Java 14 - if (!TargetVersion.isAtLeast14(state.context)) { + if (!SourceVersion.supportsSwitchExpressions(state.context)) { return NO_MATCH; } diff --git a/pom.xml b/pom.xml index 51a7df5932c..515f8b7edb6 100644 --- a/pom.xml +++ b/pom.xml @@ -161,10 +161,10 @@ --add-exports=java.base/jdk.internal.javac=ALL-UNNAMED --add-exports=jdk.compiler/com.sun.tools.javac.api=ALL-UNNAMED + --add-exports=jdk.compiler/com.sun.tools.javac.file=ALL-UNNAMED --add-exports=jdk.compiler/com.sun.tools.javac.code=ALL-UNNAMED + --add-exports=jdk.compiler/com.sun.tools.javac.util=ALL-UNNAMED --add-exports=jdk.compiler/com.sun.tools.javac.comp=ALL-UNNAMED - --add-exports=jdk.compiler/com.sun.tools.javac.file=ALL-UNNAMED - --add-exports=jdk.compiler/com.sun.tools.javac.jvm=ALL-UNNAMED --add-exports=jdk.compiler/com.sun.tools.javac.main=ALL-UNNAMED --add-exports=jdk.compiler/com.sun.tools.javac.model=ALL-UNNAMED --add-exports=jdk.compiler/com.sun.tools.javac.parser=ALL-UNNAMED @@ -213,7 +213,6 @@ -Xmx1g --add-exports=jdk.compiler/com.sun.tools.javac.api=ALL-UNNAMED --add-exports=jdk.compiler/com.sun.tools.javac.file=ALL-UNNAMED - --add-exports=jdk.compiler/com.sun.tools.javac.jvm=ALL-UNNAMED --add-exports=jdk.compiler/com.sun.tools.javac.main=ALL-UNNAMED --add-exports=jdk.compiler/com.sun.tools.javac.model=ALL-UNNAMED --add-exports=jdk.compiler/com.sun.tools.javac.parser=ALL-UNNAMED From 3690ab1dc7498f830489109c3875b14e8b19ed72 Mon Sep 17 00:00:00 2001 From: Stephan Schroevers Date: Sat, 31 Dec 2022 16:40:32 +0100 Subject: [PATCH 3/5] Javadoc doesn't like `@apiNote` --- .../main/java/com/google/errorprone/util/SourceVersion.java | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) 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 eece907c1e8..04c0501f65e 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 @@ -46,8 +46,8 @@ public static boolean supportsTextBlocks(Context context) { * Returns true if the compiler source version level supports the {@link Feature} indicated by the * specified string. * - * @apiNote For features explicitly recognized by this class, prefer calling the associated method - * instead. + *

For features explicitly recognized by this class, prefer calling the associated method + * instead. */ public static boolean supportsFeature(String featureString, Context context) { Feature feature = KNOWN_FEATURES.get(featureString); From 78068088e23a6fc0b643c3fb4bedb1dad01922d9 Mon Sep 17 00:00:00 2001 From: Stephan Schroevers Date: Sat, 31 Dec 2022 16:59:25 +0100 Subject: [PATCH 4/5] Unit test proposal --- .../errorprone/util/SourceVersionTest.java | 66 +++++++++++++++++++ 1 file changed, 66 insertions(+) create mode 100644 check_api/src/test/java/com/google/errorprone/util/SourceVersionTest.java diff --git a/check_api/src/test/java/com/google/errorprone/util/SourceVersionTest.java b/check_api/src/test/java/com/google/errorprone/util/SourceVersionTest.java new file mode 100644 index 00000000000..c7da4fb92b9 --- /dev/null +++ b/check_api/src/test/java/com/google/errorprone/util/SourceVersionTest.java @@ -0,0 +1,66 @@ +/* + * Copyright 2023 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. + */ +package com.google.errorprone.util; + +import static com.google.common.truth.Truth.assertThat; + +import com.sun.tools.javac.main.Option; +import com.sun.tools.javac.util.Context; +import com.sun.tools.javac.util.Options; +import org.junit.Test; +import org.junit.runner.RunWith; +import org.junit.runners.JUnit4; + +@RunWith(JUnit4.class) +public class SourceVersionTest { + @Test + public void supportsSwitchExpressions_notSupported() { + Context context = contextWithSourceVersion("13"); + + assertThat(SourceVersion.supportsSwitchExpressions(context)).isFalse(); + } + + @Test + public void supportsSwitchExpressions_conditionallySupported() { + Context context = contextWithSourceVersion("14"); + + assertThat(SourceVersion.supportsSwitchExpressions(context)) + .isEqualTo(RuntimeVersion.isAtLeast14()); + } + + @Test + public void supportsTextBlocks_notSupported() { + Context context = contextWithSourceVersion("14"); + + assertThat(SourceVersion.supportsTextBlocks(context)).isFalse(); + } + + @Test + public void supportsTextBlocks_conditionallySupported() { + Context context = contextWithSourceVersion("15"); + + assertThat(SourceVersion.supportsTextBlocks(context)).isEqualTo(RuntimeVersion.isAtLeast15()); + } + + private static Context contextWithSourceVersion(String versionString) { + Context context = new Context(); + + Options options = Options.instance(context); + options.put(Option.SOURCE, versionString); + + return context; + } +} From 641c9dc5bc42ee7e3d1ca7a57e39ac83bb2b343a Mon Sep 17 00:00:00 2001 From: Stephan Schroevers Date: Sat, 31 Dec 2022 18:00:39 +0100 Subject: [PATCH 5/5] Avoid reliance on `Source.Feature` --- .../google/errorprone/util/SourceVersion.java | 24 ++++--------------- 1 file changed, 5 insertions(+), 19 deletions(-) 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 04c0501f65e..23de9c6402a 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 @@ -16,12 +16,8 @@ package com.google.errorprone.util; -import com.google.common.collect.ImmutableMap; -import com.google.common.collect.Maps; import com.sun.tools.javac.code.Source; -import com.sun.tools.javac.code.Source.Feature; import com.sun.tools.javac.util.Context; -import java.util.Arrays; /** * JDK source version utilities. @@ -29,29 +25,19 @@ * @see RuntimeVersion */ public final class SourceVersion { - private static final ImmutableMap KNOWN_FEATURES = - Maps.uniqueIndex(Arrays.asList(Feature.values()), Enum::name); - /** Returns true if the compiler source version level supports switch expressions. */ public static boolean supportsSwitchExpressions(Context context) { - return supportsFeature("SWITCH_EXPRESSION", context); + return sourceIsAtLeast(context, "14"); } /** Returns true if the compiler source version level supports text blocks. */ public static boolean supportsTextBlocks(Context context) { - return supportsFeature("TEXT_BLOCKS", context); + return sourceIsAtLeast(context, "15"); } - /** - * Returns true if the compiler source version level supports the {@link Feature} indicated by the - * specified string. - * - *

For features explicitly recognized by this class, prefer calling the associated method - * instead. - */ - public static boolean supportsFeature(String featureString, Context context) { - Feature feature = KNOWN_FEATURES.get(featureString); - return feature != null && feature.allowedInSource(Source.instance(context)); + private static boolean sourceIsAtLeast(Context context, String versionString) { + Source lowerBound = Source.lookup(versionString); + return lowerBound != null && Source.instance(context).compareTo(lowerBound) >= 0; } private SourceVersion() {}