From 0b2a0680ba7e09da75f237afef59d53cf3fb0346 Mon Sep 17 00:00:00 2001 From: Stephan Schroevers Date: Fri, 28 Apr 2023 16:26:29 +0200 Subject: [PATCH 01/10] Upgrade JDKs used by GitHub Actions builds Summary of changes: - Use JDK 11.0.19 instead of 11.0.18. - Use JDK 17.0.7 instead of 17.0.6. - Use JDK 20.0.1 instead of 19.0.2. - Drop the early access build, as Error Prone is currently not compatible with JDK 21-ea. See: - https://www.oracle.com/java/technologies/javase/11-0-19-relnotes.html - https://www.oracle.com/java/technologies/javase/17-0-7-relnotes.html - https://www.oracle.com/java/technologies/javase/20-relnote-issues.html - https://www.oracle.com/java/technologies/javase/20-0-1-relnotes.html --- .github/ISSUE_TEMPLATE/bug_report.md | 4 ++-- .github/workflows/build.yaml | 10 +++------- .github/workflows/codeql.yml | 2 +- .github/workflows/pitest-analyze-pr.yml | 2 +- .github/workflows/pitest-update-pr.yml | 2 +- .github/workflows/sonarcloud.yml | 2 +- 6 files changed, 9 insertions(+), 13 deletions(-) diff --git a/.github/ISSUE_TEMPLATE/bug_report.md b/.github/ISSUE_TEMPLATE/bug_report.md index fce8a8f1af2..f186587c591 100644 --- a/.github/ISSUE_TEMPLATE/bug_report.md +++ b/.github/ISSUE_TEMPLATE/bug_report.md @@ -42,9 +42,9 @@ Please replace this sentence with log output, if applicable. - Operating system (e.g. MacOS Monterey). -- Java version (i.e. `java --version`, e.g. `17.0.6`). +- Java version (i.e. `java --version`, e.g. `17.0.7`). - Error Prone version (e.g. `2.18.0`). -- Error Prone Support version (e.g. `0.8.0`). +- Error Prone Support version (e.g. `0.9.0`). ### Additional context diff --git a/.github/workflows/build.yaml b/.github/workflows/build.yaml index 6890219eab2..e9fd28307cd 100644 --- a/.github/workflows/build.yaml +++ b/.github/workflows/build.yaml @@ -10,22 +10,18 @@ jobs: strategy: matrix: os: [ ubuntu-22.04 ] - jdk: [ 11.0.18, 17.0.6, 19.0.2 ] + jdk: [ 11.0.19, 17.0.7, 20.0.1 ] distribution: [ temurin ] experimental: [ false ] include: - os: macos-12 - jdk: 17.0.6 + jdk: 17.0.7 distribution: temurin experimental: false - os: windows-2022 - jdk: 17.0.6 + jdk: 17.0.7 distribution: temurin experimental: false - - os: ubuntu-22.04 - jdk: 20-ea - distribution: zulu - experimental: true runs-on: ${{ matrix.os }} continue-on-error: ${{ matrix.experimental }} steps: diff --git a/.github/workflows/codeql.yml b/.github/workflows/codeql.yml index f6bcd764dd9..9456ab1700d 100644 --- a/.github/workflows/codeql.yml +++ b/.github/workflows/codeql.yml @@ -28,7 +28,7 @@ jobs: - name: Set up JDK uses: actions/setup-java@v3.8.0 with: - java-version: 17.0.6 + java-version: 17.0.7 distribution: temurin cache: maven - name: Initialize CodeQL diff --git a/.github/workflows/pitest-analyze-pr.yml b/.github/workflows/pitest-analyze-pr.yml index 918022529a5..5a14fa2ae95 100644 --- a/.github/workflows/pitest-analyze-pr.yml +++ b/.github/workflows/pitest-analyze-pr.yml @@ -19,7 +19,7 @@ jobs: - name: Set up JDK uses: actions/setup-java@v3.8.0 with: - java-version: 17.0.6 + java-version: 17.0.7 distribution: temurin cache: maven - name: Run Pitest diff --git a/.github/workflows/pitest-update-pr.yml b/.github/workflows/pitest-update-pr.yml index 913b781c109..cbaae60813d 100644 --- a/.github/workflows/pitest-update-pr.yml +++ b/.github/workflows/pitest-update-pr.yml @@ -26,7 +26,7 @@ jobs: - name: Set up JDK uses: actions/setup-java@v3.8.0 with: - java-version: 17.0.6 + java-version: 17.0.7 distribution: temurin cache: maven - name: Download Pitest analysis artifact diff --git a/.github/workflows/sonarcloud.yml b/.github/workflows/sonarcloud.yml index 3966960a4b8..5df6b84646c 100644 --- a/.github/workflows/sonarcloud.yml +++ b/.github/workflows/sonarcloud.yml @@ -23,7 +23,7 @@ jobs: - name: Set up JDK uses: actions/setup-java@v3.8.0 with: - java-version: 17.0.6 + java-version: 17.0.7 distribution: temurin cache: maven - name: Create missing `test` directory From eba50c673cbc5f32723fe4f61f0f1b60c7186df9 Mon Sep 17 00:00:00 2001 From: Stephan Schroevers Date: Wed, 26 Apr 2023 19:01:40 +0200 Subject: [PATCH 02/10] Use Jabel to support JDK >11 constructs while still targeting JDK 11 --- .github/workflows/build-jdk11.yaml | 35 +++++++++++ .github/workflows/build.yaml | 2 +- README.md | 13 ----- pom.xml | 94 +++++++++++++++++++++++++++--- 4 files changed, 122 insertions(+), 22 deletions(-) create mode 100644 .github/workflows/build-jdk11.yaml diff --git a/.github/workflows/build-jdk11.yaml b/.github/workflows/build-jdk11.yaml new file mode 100644 index 00000000000..16e69a1b5f0 --- /dev/null +++ b/.github/workflows/build-jdk11.yaml @@ -0,0 +1,35 @@ +name: Build with JDK 17 and test against JDK 11 +on: + pull_request: + push: + branches: [ master ] +permissions: + contents: read +jobs: + build: + runs-on: ubuntu-22.04 + steps: + # We run the build twice: once against the original Error Prone + # release, and once against the Picnic Error Prone fork. In both cases + # the code is compiled using JDK 17, while the tests are executed + # using JDK 11. + - name: Check out code + uses: actions/checkout@v3.1.0 + with: + persist-credentials: false + - name: Set up JDK + uses: actions/setup-java@v3.8.0 + with: + java-version: | + 11.0.19 + 17.0.7 + distribution: temurin + cache: maven + - name: Display build environment details + run: mvn --version + - name: Build project against vanilla Error Prone + run: mvn -T1C install -Dsurefire.jdk-toolchain-version=11.0.19 + - name: Build project with self-check against Error Prone fork + run: mvn -T1C clean verify -Perror-prone-fork -Dsurefire.jdk-toolchain-version=11.0.19 -s settings.xml + - name: Remove installed project artifacts + run: mvn build-helper:remove-project-artifact diff --git a/.github/workflows/build.yaml b/.github/workflows/build.yaml index e9fd28307cd..499056cf074 100644 --- a/.github/workflows/build.yaml +++ b/.github/workflows/build.yaml @@ -10,7 +10,7 @@ jobs: strategy: matrix: os: [ ubuntu-22.04 ] - jdk: [ 11.0.19, 17.0.7, 20.0.1 ] + jdk: [ 17.0.7, 20.0.1 ] distribution: [ temurin ] experimental: [ false ] include: diff --git a/README.md b/README.md index bc9aebff40c..030dc3884e9 100644 --- a/README.md +++ b/README.md @@ -227,18 +227,6 @@ Other highly relevant commands: `target/pit-reports/index.html` files. For more information check the [PIT Maven plugin][pitest-maven]. -When running the project's tests in IntelliJ IDEA, you might see the following -error: - -``` -java: exporting a package from system module jdk.compiler is not allowed with --release -``` - -If this happens, go to _Settings -> Build, Execution, Deployment -> Compiler -> -Java Compiler_ and deselect the option _Use '--release' option for -cross-compilation (Java 9 and later)_. See [IDEA-288052][idea-288052] for -details. - ## 💡 How it works This project provides additional [`BugChecker`][error-prone-bugchecker] @@ -273,7 +261,6 @@ channel; please see our [security policy][security] for details. [github-actions-build-badge]: https://github.com/PicnicSupermarket/error-prone-support/actions/workflows/build.yaml/badge.svg [github-actions-build-master]: https://github.com/PicnicSupermarket/error-prone-support/actions/workflows/build.yaml?query=branch:master&event=push [google-java-format]: https://github.com/google/google-java-format -[idea-288052]: https://youtrack.jetbrains.com/issue/IDEA-288052 [license-badge]: https://img.shields.io/github/license/PicnicSupermarket/error-prone-support [license]: https://github.com/PicnicSupermarket/error-prone-support/blob/master/LICENSE.md [maven-central-badge]: https://img.shields.io/maven-central/v/tech.picnic.error-prone-support/error-prone-support?color=blue diff --git a/pom.xml b/pom.xml index d3bd91db080..096460fe6a0 100644 --- a/pom.xml +++ b/pom.xml @@ -204,7 +204,11 @@ 2.18.0 0.1.18 1.0 - 11 + 1.0.0 + + 17 + 11 3.8.7 5.3.1 1.0.1 @@ -272,6 +276,13 @@ pom import + + + com.github.bsideup.jabel + jabel-javac-plugin + ${version.jabel} + com.google.auto auto-common @@ -550,7 +561,7 @@ `provided`, but we'd rather not do that.) --> false false - ${version.jdk} + ${version.jdk.target} @@ -875,6 +886,11 @@ error_prone_core ${version.error-prone} + + com.github.bsideup.jabel + jabel-javac-plugin + ${version.jabel} + com.google.auto.value auto-value @@ -925,8 +941,8 @@ 10000 true - ${version.jdk} - ${version.jdk} + ${version.jdk.target} + ${version.jdk.target} false @@ -991,7 +1007,7 @@ --> - ${version.jdk} + ${version.jdk.target} true @@ -999,7 +1015,7 @@ src/main/resources/**/*.properties,src/test/resources/**/*.properties - ${version.jdk} + ${version.jdk.source} ${version.maven} @@ -1099,7 +1115,7 @@ the compilation phase; no need to recheck during Javadoc generation. --> none - ${version.jdk} + ${version.jdk.source} @@ -1302,7 +1318,7 @@ patterns in their `@BeforeTemplate` methods. --> tech.picnic.errorprone.refasterrules - ${version.jdk} + ${version.jdk.target} @@ -1840,8 +1856,19 @@ sonar + + + sonar.projectKey + + true + + ${version.jdk.source} @@ -1908,5 +1935,56 @@ + + + idea + + + idea.maven.embedder.version + + + + + + + org.apache.maven.plugins + maven-compiler-plugin + + ${version.jdk.source} + + + + + + + + custom-test-runtime-version + + + surefire.jdk-toolchain-version + + + + + + + org.apache.maven.plugins + maven-surefire-plugin + + + ${surefire.jdk-toolchain-version} + + + + + + + From fd127d7966fc6ee9f6046e43acfc3b9ca3afa448 Mon Sep 17 00:00:00 2001 From: Stephan Schroevers Date: Sun, 12 Mar 2023 18:16:51 +0100 Subject: [PATCH 03/10] Use text blocks --- .../BugPatternExtractorTest.java | 82 ++++++++++++++----- .../bugpattern-documentation-complete.json | 19 ----- .../bugpattern-documentation-minimal.json | 14 ---- ...ocumentation-undocumented-suppression.json | 12 --- 4 files changed, 60 insertions(+), 67 deletions(-) delete mode 100644 documentation-support/src/test/resources/tech/picnic/errorprone/documentation/bugpattern-documentation-complete.json delete mode 100644 documentation-support/src/test/resources/tech/picnic/errorprone/documentation/bugpattern-documentation-minimal.json delete mode 100644 documentation-support/src/test/resources/tech/picnic/errorprone/documentation/bugpattern-documentation-undocumented-suppression.json diff --git a/documentation-support/src/test/java/tech/picnic/errorprone/documentation/BugPatternExtractorTest.java b/documentation-support/src/test/java/tech/picnic/errorprone/documentation/BugPatternExtractorTest.java index da6467c78de..561aba6f0ae 100644 --- a/documentation-support/src/test/java/tech/picnic/errorprone/documentation/BugPatternExtractorTest.java +++ b/documentation-support/src/test/java/tech/picnic/errorprone/documentation/BugPatternExtractorTest.java @@ -5,7 +5,6 @@ import static org.assertj.core.api.Assertions.assertThat; import static org.assertj.core.api.Assertions.assertThatThrownBy; -import com.google.common.io.Resources; import com.google.errorprone.BugPattern; import com.google.errorprone.CompilationTestHelper; import com.google.errorprone.VisitorState; @@ -13,7 +12,6 @@ import com.google.errorprone.bugpatterns.BugChecker.ClassTreeMatcher; import com.google.errorprone.matchers.Description; import com.sun.source.tree.ClassTree; -import java.io.IOException; import java.nio.file.Path; import org.junit.jupiter.api.Test; import org.junit.jupiter.api.io.TempDir; @@ -32,7 +30,7 @@ void noBugPattern(@TempDir Path outputDirectory) { } @Test - void minimalBugPattern(@TempDir Path outputDirectory) throws IOException { + void minimalBugPattern(@TempDir Path outputDirectory) { Compilation.compileWithDocumentationGenerator( outputDirectory, "MinimalBugChecker.java", @@ -45,14 +43,29 @@ void minimalBugPattern(@TempDir Path outputDirectory) throws IOException { "@BugPattern(summary = \"MinimalBugChecker summary\", severity = SeverityLevel.ERROR)", "public final class MinimalBugChecker extends BugChecker {}"); - verifyFileMatchesResource( + verifyFileContent( outputDirectory, "bugpattern-MinimalBugChecker.json", - "bugpattern-documentation-minimal.json"); + """ + { + "fullyQualifiedName": "pkg.MinimalBugChecker", + "name": "MinimalBugChecker", + "altNames": [], + "link": "", + "tags": [], + "summary": "MinimalBugChecker summary", + "explanation": "", + "severityLevel": "ERROR", + "canDisable": true, + "suppressionAnnotations": [ + "java.lang.SuppressWarnings" + ] + } + """); } @Test - void completeBugPattern(@TempDir Path outputDirectory) throws IOException { + void completeBugPattern(@TempDir Path outputDirectory) { Compilation.compileWithDocumentationGenerator( outputDirectory, "CompleteBugChecker.java", @@ -76,14 +89,34 @@ void completeBugPattern(@TempDir Path outputDirectory) throws IOException { " suppressionAnnotations = {BugPattern.class, Test.class})", "public final class CompleteBugChecker extends BugChecker {}"); - verifyFileMatchesResource( + verifyFileContent( outputDirectory, "bugpattern-CompleteBugChecker.json", - "bugpattern-documentation-complete.json"); + """ + { + "fullyQualifiedName": "pkg.CompleteBugChecker", + "name": "OtherName", + "altNames": [ + "Check" + ], + "link": "https://error-prone.picnic.tech", + "tags": [ + "Simplification" + ], + "summary": "CompleteBugChecker summary", + "explanation": "Example explanation", + "severityLevel": "SUGGESTION", + "canDisable": false, + "suppressionAnnotations": [ + "com.google.errorprone.BugPattern", + "org.junit.jupiter.api.Test" + ] + } + """); } @Test - void undocumentedSuppressionBugPattern(@TempDir Path outputDirectory) throws IOException { + void undocumentedSuppressionBugPattern(@TempDir Path outputDirectory) { Compilation.compileWithDocumentationGenerator( outputDirectory, "UndocumentedSuppressionBugPattern.java", @@ -99,10 +132,23 @@ void undocumentedSuppressionBugPattern(@TempDir Path outputDirectory) throws IOE " documentSuppression = false)", "public final class UndocumentedSuppressionBugPattern extends BugChecker {}"); - verifyFileMatchesResource( + verifyFileContent( outputDirectory, "bugpattern-UndocumentedSuppressionBugPattern.json", - "bugpattern-documentation-undocumented-suppression.json"); + """ + { + "fullyQualifiedName": "pkg.UndocumentedSuppressionBugPattern", + "name": "UndocumentedSuppressionBugPattern", + "altNames": [], + "link": "", + "tags": [], + "summary": "UndocumentedSuppressionBugPattern summary", + "explanation": "", + "severityLevel": "WARNING", + "canDisable": true, + "suppressionAnnotations": [] + } + """); } @Test @@ -117,19 +163,11 @@ void bugPatternAnnotationIsAbsent() { .doTest(); } - private static void verifyFileMatchesResource( - Path outputDirectory, String fileName, String resourceName) throws IOException { + private static void verifyFileContent( + Path outputDirectory, String fileName, String expectedContent) { assertThat(outputDirectory.resolve(fileName)) .content(UTF_8) - .isEqualToIgnoringWhitespace(getResource(resourceName)); - } - - // XXX: Once we support only JDK 15+, drop this method in favour of including the resources as - // text blocks in this class. (This also requires renaming the `verifyFileMatchesResource` - // method.) - private static String getResource(String resourceName) throws IOException { - return Resources.toString( - Resources.getResource(BugPatternExtractorTest.class, resourceName), UTF_8); + .isEqualToIgnoringWhitespace(expectedContent); } /** A {@link BugChecker} that validates the {@link BugPatternExtractor}. */ diff --git a/documentation-support/src/test/resources/tech/picnic/errorprone/documentation/bugpattern-documentation-complete.json b/documentation-support/src/test/resources/tech/picnic/errorprone/documentation/bugpattern-documentation-complete.json deleted file mode 100644 index eb29080c057..00000000000 --- a/documentation-support/src/test/resources/tech/picnic/errorprone/documentation/bugpattern-documentation-complete.json +++ /dev/null @@ -1,19 +0,0 @@ -{ - "fullyQualifiedName": "pkg.CompleteBugChecker", - "name": "OtherName", - "altNames": [ - "Check" - ], - "link": "https://error-prone.picnic.tech", - "tags": [ - "Simplification" - ], - "summary": "CompleteBugChecker summary", - "explanation": "Example explanation", - "severityLevel": "SUGGESTION", - "canDisable": false, - "suppressionAnnotations": [ - "com.google.errorprone.BugPattern", - "org.junit.jupiter.api.Test" - ] -} diff --git a/documentation-support/src/test/resources/tech/picnic/errorprone/documentation/bugpattern-documentation-minimal.json b/documentation-support/src/test/resources/tech/picnic/errorprone/documentation/bugpattern-documentation-minimal.json deleted file mode 100644 index 8e64bdc8870..00000000000 --- a/documentation-support/src/test/resources/tech/picnic/errorprone/documentation/bugpattern-documentation-minimal.json +++ /dev/null @@ -1,14 +0,0 @@ -{ - "fullyQualifiedName": "pkg.MinimalBugChecker", - "name": "MinimalBugChecker", - "altNames": [], - "link": "", - "tags": [], - "summary": "MinimalBugChecker summary", - "explanation": "", - "severityLevel": "ERROR", - "canDisable": true, - "suppressionAnnotations": [ - "java.lang.SuppressWarnings" - ] -} diff --git a/documentation-support/src/test/resources/tech/picnic/errorprone/documentation/bugpattern-documentation-undocumented-suppression.json b/documentation-support/src/test/resources/tech/picnic/errorprone/documentation/bugpattern-documentation-undocumented-suppression.json deleted file mode 100644 index b738dfd9885..00000000000 --- a/documentation-support/src/test/resources/tech/picnic/errorprone/documentation/bugpattern-documentation-undocumented-suppression.json +++ /dev/null @@ -1,12 +0,0 @@ -{ - "fullyQualifiedName": "pkg.UndocumentedSuppressionBugPattern", - "name": "UndocumentedSuppressionBugPattern", - "altNames": [], - "link": "", - "tags": [], - "summary": "UndocumentedSuppressionBugPattern summary", - "explanation": "", - "severityLevel": "WARNING", - "canDisable": true, - "suppressionAnnotations": [] -} From 33e29f2db11680b7620f0c5fc9b84ff525468412 Mon Sep 17 00:00:00 2001 From: Stephan Schroevers Date: Thu, 27 Apr 2023 13:18:06 +0200 Subject: [PATCH 04/10] Auto-start `RefasterRuleCompiler` --- error-prone-contrib/pom.xml | 1 - .../errorprone/refaster/plugin/RefasterRuleCompiler.java | 5 +++++ refaster-runner/pom.xml | 3 --- refaster-test-support/pom.xml | 3 --- 4 files changed, 5 insertions(+), 7 deletions(-) diff --git a/error-prone-contrib/pom.xml b/error-prone-contrib/pom.xml index b0128801e7d..75bdf49bca7 100644 --- a/error-prone-contrib/pom.xml +++ b/error-prone-contrib/pom.xml @@ -238,7 +238,6 @@ - -Xplugin:RefasterRuleCompiler -Xplugin:DocumentationGenerator -XoutputDirectory=${project.build.directory}/docs diff --git a/refaster-compiler/src/main/java/tech/picnic/errorprone/refaster/plugin/RefasterRuleCompiler.java b/refaster-compiler/src/main/java/tech/picnic/errorprone/refaster/plugin/RefasterRuleCompiler.java index e7423961df1..950fa7b0199 100644 --- a/refaster-compiler/src/main/java/tech/picnic/errorprone/refaster/plugin/RefasterRuleCompiler.java +++ b/refaster-compiler/src/main/java/tech/picnic/errorprone/refaster/plugin/RefasterRuleCompiler.java @@ -25,4 +25,9 @@ public void init(JavacTask javacTask, String... args) { javacTask.addTaskListener( new RefasterRuleCompilerTaskListener(((BasicJavacTask) javacTask).getContext())); } + + @Override + public boolean autoStart() { + return true; + } } diff --git a/refaster-runner/pom.xml b/refaster-runner/pom.xml index af64947f90b..490dd03e600 100644 --- a/refaster-runner/pom.xml +++ b/refaster-runner/pom.xml @@ -97,9 +97,6 @@ ${project.version} - - -Xplugin:RefasterRuleCompiler - diff --git a/refaster-test-support/pom.xml b/refaster-test-support/pom.xml index 27628d19917..2d9d4e6f226 100644 --- a/refaster-test-support/pom.xml +++ b/refaster-test-support/pom.xml @@ -80,9 +80,6 @@ ${project.version} - - -Xplugin:RefasterRuleCompiler - From 608c3c83357933823ed978f79cdd94bfa743f354 Mon Sep 17 00:00:00 2001 From: Stephan Schroevers Date: Thu, 27 Apr 2023 13:45:43 +0200 Subject: [PATCH 05/10] Use switch expressions --- .../bugpatterns/JUnitValueSource.java | 16 +++---- .../bugpatterns/MethodReferenceUsage.java | 43 +++++++++---------- .../MockitoMockClassReference.java | 22 ++++------ .../bugpatterns/PrimitiveComparison.java | 40 +++++++++-------- .../RedundantStringConversion.java | 17 +++----- .../bugpatterns/RefasterAnyOfUsage.java | 25 +++++------ .../bugpatterns/SpringMvcAnnotation.java | 13 +++--- .../errorprone/bugpatterns/StaticImport.java | 14 +++--- .../errorprone/refaster/runner/Refaster.java | 17 +++----- .../refaster/runner/RefasterTest.java | 18 +++----- 10 files changed, 98 insertions(+), 127 deletions(-) diff --git a/error-prone-contrib/src/main/java/tech/picnic/errorprone/bugpatterns/JUnitValueSource.java b/error-prone-contrib/src/main/java/tech/picnic/errorprone/bugpatterns/JUnitValueSource.java index e6fe6df8529..75fc9ee39aa 100644 --- a/error-prone-contrib/src/main/java/tech/picnic/errorprone/bugpatterns/JUnitValueSource.java +++ b/error-prone-contrib/src/main/java/tech/picnic/errorprone/bugpatterns/JUnitValueSource.java @@ -268,16 +268,12 @@ private static Optional tryExtractValueSourceAttributeValue( private static String toValueSourceAttributeName(Type type) { String typeString = type.tsym.name.toString(); - switch (typeString) { - case "Class": - return "classes"; - case "Character": - return "chars"; - case "Integer": - return "ints"; - default: - return typeString.toLowerCase(Locale.ROOT) + 's'; - } + return switch (typeString) { + case "Class" -> "classes"; + case "Character" -> "chars"; + case "Integer" -> "ints"; + default -> typeString.toLowerCase(Locale.ROOT) + 's'; + }; } private static Optional getElementIfSingleton(Collection collection) { diff --git a/error-prone-contrib/src/main/java/tech/picnic/errorprone/bugpatterns/MethodReferenceUsage.java b/error-prone-contrib/src/main/java/tech/picnic/errorprone/bugpatterns/MethodReferenceUsage.java index 6c75d67e823..2825dd6fc83 100644 --- a/error-prone-contrib/src/main/java/tech/picnic/errorprone/bugpatterns/MethodReferenceUsage.java +++ b/error-prone-contrib/src/main/java/tech/picnic/errorprone/bugpatterns/MethodReferenceUsage.java @@ -87,20 +87,16 @@ public Description matchLambdaExpression(LambdaExpressionTree tree, VisitorState private static Optional constructMethodRef( LambdaExpressionTree lambdaExpr, Tree subTree) { - switch (subTree.getKind()) { - case BLOCK: - return constructMethodRef(lambdaExpr, (BlockTree) subTree); - case EXPRESSION_STATEMENT: - return constructMethodRef(lambdaExpr, ((ExpressionStatementTree) subTree).getExpression()); - case METHOD_INVOCATION: - return constructMethodRef(lambdaExpr, (MethodInvocationTree) subTree); - case PARENTHESIZED: - return constructMethodRef(lambdaExpr, ((ParenthesizedTree) subTree).getExpression()); - case RETURN: - return constructMethodRef(lambdaExpr, ((ReturnTree) subTree).getExpression()); - default: - return Optional.empty(); - } + return switch (subTree.getKind()) { + case BLOCK -> constructMethodRef(lambdaExpr, (BlockTree) subTree); + case EXPRESSION_STATEMENT -> constructMethodRef( + lambdaExpr, ((ExpressionStatementTree) subTree).getExpression()); + case METHOD_INVOCATION -> constructMethodRef(lambdaExpr, (MethodInvocationTree) subTree); + case PARENTHESIZED -> constructMethodRef( + lambdaExpr, ((ParenthesizedTree) subTree).getExpression()); + case RETURN -> constructMethodRef(lambdaExpr, ((ReturnTree) subTree).getExpression()); + default -> Optional.empty(); + }; } private static Optional constructMethodRef( @@ -125,21 +121,22 @@ private static Optional constructMethodRef( MethodInvocationTree subTree, Optional expectedInstance) { ExpressionTree methodSelect = subTree.getMethodSelect(); - switch (methodSelect.getKind()) { - case IDENTIFIER: + return switch (methodSelect.getKind()) { + case IDENTIFIER -> { if (expectedInstance.isPresent()) { /* Direct method call; there is no matching "implicit parameter". */ - return Optional.empty(); + yield Optional.empty(); } Symbol sym = ASTHelpers.getSymbol(methodSelect); - return ASTHelpers.isStatic(sym) + yield ASTHelpers.isStatic(sym) ? constructFix(lambdaExpr, sym.owner, methodSelect) : constructFix(lambdaExpr, "this", methodSelect); - case MEMBER_SELECT: - return constructMethodRef(lambdaExpr, (MemberSelectTree) methodSelect, expectedInstance); - default: - throw new VerifyException("Unexpected type of expression: " + methodSelect.getKind()); - } + } + case MEMBER_SELECT -> constructMethodRef( + lambdaExpr, (MemberSelectTree) methodSelect, expectedInstance); + default -> throw new VerifyException( + "Unexpected type of expression: " + methodSelect.getKind()); + }; } private static Optional constructMethodRef( diff --git a/error-prone-contrib/src/main/java/tech/picnic/errorprone/bugpatterns/MockitoMockClassReference.java b/error-prone-contrib/src/main/java/tech/picnic/errorprone/bugpatterns/MockitoMockClassReference.java index af2e51d8a00..1ed193468f8 100644 --- a/error-prone-contrib/src/main/java/tech/picnic/errorprone/bugpatterns/MockitoMockClassReference.java +++ b/error-prone-contrib/src/main/java/tech/picnic/errorprone/bugpatterns/MockitoMockClassReference.java @@ -69,18 +69,14 @@ public Description matchMethodInvocation(MethodInvocationTree tree, VisitorState private static boolean isTypeDerivableFromContext(MethodInvocationTree tree, VisitorState state) { Tree parent = state.getPath().getParentPath().getLeaf(); - switch (parent.getKind()) { - case VARIABLE: - return !ASTHelpers.hasNoExplicitType((VariableTree) parent, state) - && MoreASTHelpers.areSameType(tree, parent, state); - case ASSIGNMENT: - return MoreASTHelpers.areSameType(tree, parent, state); - case RETURN: - return MoreASTHelpers.findMethodExitedOnReturn(state) - .filter(m -> MoreASTHelpers.areSameType(tree, m.getReturnType(), state)) - .isPresent(); - default: - return false; - } + return switch (parent.getKind()) { + case VARIABLE -> !ASTHelpers.hasNoExplicitType((VariableTree) parent, state) + && MoreASTHelpers.areSameType(tree, parent, state); + case ASSIGNMENT -> MoreASTHelpers.areSameType(tree, parent, state); + case RETURN -> MoreASTHelpers.findMethodExitedOnReturn(state) + .filter(m -> MoreASTHelpers.areSameType(tree, m.getReturnType(), state)) + .isPresent(); + default -> false; + }; } } diff --git a/error-prone-contrib/src/main/java/tech/picnic/errorprone/bugpatterns/PrimitiveComparison.java b/error-prone-contrib/src/main/java/tech/picnic/errorprone/bugpatterns/PrimitiveComparison.java index 9f2d2f82198..2ab00020ee8 100644 --- a/error-prone-contrib/src/main/java/tech/picnic/errorprone/bugpatterns/PrimitiveComparison.java +++ b/error-prone-contrib/src/main/java/tech/picnic/errorprone/bugpatterns/PrimitiveComparison.java @@ -147,36 +147,38 @@ private static String getPreferredMethod(Type cmpType, boolean isStatic, Visitor } private static Optional getPotentiallyBoxedReturnType(ExpressionTree tree) { - switch (tree.getKind()) { - case LAMBDA_EXPRESSION: + return switch (tree.getKind()) { + case LAMBDA_EXPRESSION -> { /* Return the lambda expression's actual return type. */ - return Optional.ofNullable(ASTHelpers.getType(((LambdaExpressionTree) tree).getBody())); - case MEMBER_REFERENCE: + yield Optional.ofNullable(ASTHelpers.getType(((LambdaExpressionTree) tree).getBody())); + } + case MEMBER_REFERENCE -> { /* Return the method's declared return type. */ // XXX: Very fragile. Do better. Type subType2 = ((JCMemberReference) tree).referentType; - return Optional.of(subType2.getReturnType()); - default: + yield Optional.of(subType2.getReturnType()); + } + default -> { /* This appears to be a genuine `{,ToInt,ToLong,ToDouble}Function`. */ - return Optional.empty(); - } + yield Optional.empty(); + } + }; } private static Fix suggestFix( MethodInvocationTree tree, String preferredMethodName, VisitorState state) { ExpressionTree expr = tree.getMethodSelect(); - switch (expr.getKind()) { - case IDENTIFIER: - return SuggestedFix.builder() - .addStaticImport(Comparator.class.getName() + '.' + preferredMethodName) - .replace(expr, preferredMethodName) - .build(); - case MEMBER_SELECT: + return switch (expr.getKind()) { + case IDENTIFIER -> SuggestedFix.builder() + .addStaticImport(Comparator.class.getName() + '.' + preferredMethodName) + .replace(expr, preferredMethodName) + .build(); + case MEMBER_SELECT -> { MemberSelectTree ms = (MemberSelectTree) tree.getMethodSelect(); - return SuggestedFix.replace( + yield SuggestedFix.replace( ms, SourceCode.treeToString(ms.getExpression(), state) + '.' + preferredMethodName); - default: - throw new VerifyException("Unexpected type of expression: " + expr.getKind()); - } + } + default -> throw new VerifyException("Unexpected type of expression: " + expr.getKind()); + }; } } diff --git a/error-prone-contrib/src/main/java/tech/picnic/errorprone/bugpatterns/RedundantStringConversion.java b/error-prone-contrib/src/main/java/tech/picnic/errorprone/bugpatterns/RedundantStringConversion.java index db4889a91dd..bd670101b8e 100644 --- a/error-prone-contrib/src/main/java/tech/picnic/errorprone/bugpatterns/RedundantStringConversion.java +++ b/error-prone-contrib/src/main/java/tech/picnic/errorprone/bugpatterns/RedundantStringConversion.java @@ -334,16 +334,13 @@ private Optional trySimplify(ExpressionTree tree, VisitorState s return Optional.empty(); } - switch (methodInvocation.getArguments().size()) { - case 0: - return trySimplifyNullaryMethod(methodInvocation, state); - case 1: - return trySimplifyUnaryMethod(methodInvocation, state); - default: - throw new IllegalStateException( - "Cannot simplify method call with two or more arguments: " - + SourceCode.treeToString(tree, state)); - } + return switch (methodInvocation.getArguments().size()) { + case 0 -> trySimplifyNullaryMethod(methodInvocation, state); + case 1 -> trySimplifyUnaryMethod(methodInvocation, state); + default -> throw new IllegalStateException( + "Cannot simplify method call with two or more arguments: " + + SourceCode.treeToString(tree, state)); + }; } private static Optional trySimplifyNullaryMethod( diff --git a/error-prone-contrib/src/main/java/tech/picnic/errorprone/bugpatterns/RefasterAnyOfUsage.java b/error-prone-contrib/src/main/java/tech/picnic/errorprone/bugpatterns/RefasterAnyOfUsage.java index 97e6814644b..9bca5372e61 100644 --- a/error-prone-contrib/src/main/java/tech/picnic/errorprone/bugpatterns/RefasterAnyOfUsage.java +++ b/error-prone-contrib/src/main/java/tech/picnic/errorprone/bugpatterns/RefasterAnyOfUsage.java @@ -42,21 +42,18 @@ public RefasterAnyOfUsage() {} @Override public Description matchMethodInvocation(MethodInvocationTree tree, VisitorState state) { - if (REFASTER_ANY_OF.matches(tree, state)) { - switch (tree.getArguments().size()) { - case 0: - // We can't safely fix this case; dropping the expression may produce non-compilable code. - return describeMatch(tree); - case 1: - return describeMatch( - tree, - SuggestedFix.replace( - tree, SourceCode.treeToString(tree.getArguments().get(0), state))); - default: - /* Handled below. */ - } + int argumentCount = tree.getArguments().size(); + if (argumentCount > 1 || !REFASTER_ANY_OF.matches(tree, state)) { + return Description.NO_MATCH; } - return Description.NO_MATCH; + if (argumentCount == 0) { + /* We can't safely fix this case; dropping the expression may produce non-compilable code. */ + return describeMatch(tree); + } + + return describeMatch( + tree, + SuggestedFix.replace(tree, SourceCode.treeToString(tree.getArguments().get(0), state))); } } diff --git a/error-prone-contrib/src/main/java/tech/picnic/errorprone/bugpatterns/SpringMvcAnnotation.java b/error-prone-contrib/src/main/java/tech/picnic/errorprone/bugpatterns/SpringMvcAnnotation.java index 97cad608058..1fa027d2632 100644 --- a/error-prone-contrib/src/main/java/tech/picnic/errorprone/bugpatterns/SpringMvcAnnotation.java +++ b/error-prone-contrib/src/main/java/tech/picnic/errorprone/bugpatterns/SpringMvcAnnotation.java @@ -96,14 +96,11 @@ private static Optional extractUniqueMethod(ExpressionTree arg, VisitorS } private static String extractMethod(ExpressionTree expr, VisitorState state) { - switch (expr.getKind()) { - case IDENTIFIER: - return SourceCode.treeToString(expr, state); - case MEMBER_SELECT: - return ((MemberSelectTree) expr).getIdentifier().toString(); - default: - throw new VerifyException("Unexpected type of expression: " + expr.getKind()); - } + return switch (expr.getKind()) { + case IDENTIFIER -> SourceCode.treeToString(expr, state); + case MEMBER_SELECT -> ((MemberSelectTree) expr).getIdentifier().toString(); + default -> throw new VerifyException("Unexpected type of expression: " + expr.getKind()); + }; } private static Fix replaceAnnotation( diff --git a/error-prone-contrib/src/main/java/tech/picnic/errorprone/bugpatterns/StaticImport.java b/error-prone-contrib/src/main/java/tech/picnic/errorprone/bugpatterns/StaticImport.java index 6063562002f..ae6a7928563 100644 --- a/error-prone-contrib/src/main/java/tech/picnic/errorprone/bugpatterns/StaticImport.java +++ b/error-prone-contrib/src/main/java/tech/picnic/errorprone/bugpatterns/StaticImport.java @@ -215,15 +215,11 @@ private static boolean isCandidateContext(VisitorState state) { Tree parentTree = requireNonNull(state.getPath().getParentPath(), "MemberSelectTree lacks enclosing node") .getLeaf(); - switch (parentTree.getKind()) { - case IMPORT: - case MEMBER_SELECT: - return false; - case METHOD_INVOCATION: - return ((MethodInvocationTree) parentTree).getTypeArguments().isEmpty(); - default: - return true; - } + return switch (parentTree.getKind()) { + case IMPORT, MEMBER_SELECT -> false; + case METHOD_INVOCATION -> ((MethodInvocationTree) parentTree).getTypeArguments().isEmpty(); + default -> true; + }; } private static boolean isCandidate(MemberSelectTree tree) { diff --git a/refaster-runner/src/main/java/tech/picnic/errorprone/refaster/runner/Refaster.java b/refaster-runner/src/main/java/tech/picnic/errorprone/refaster/runner/Refaster.java index 4973060243d..15f78b39af3 100644 --- a/refaster-runner/src/main/java/tech/picnic/errorprone/refaster/runner/Refaster.java +++ b/refaster-runner/src/main/java/tech/picnic/errorprone/refaster/runner/Refaster.java @@ -140,16 +140,13 @@ private Optional getSeverityOverride(VisitorState state) { } private static Optional toSeverityLevel(Severity severity) { - switch (severity) { - case DEFAULT: - return Optional.empty(); - case WARN: - return Optional.of(WARNING); - case ERROR: - return Optional.of(ERROR); - default: - throw new IllegalStateException(String.format("Unsupported severity='%s'", severity)); - } + return switch (severity) { + case DEFAULT -> Optional.empty(); + case WARN -> Optional.of(WARNING); + case ERROR -> Optional.of(ERROR); + default -> throw new IllegalStateException( + String.format("Unsupported severity='%s'", severity)); + }; } /** diff --git a/refaster-runner/src/test/java/tech/picnic/errorprone/refaster/runner/RefasterTest.java b/refaster-runner/src/test/java/tech/picnic/errorprone/refaster/runner/RefasterTest.java index 2fa0f548762..9dc55085200 100644 --- a/refaster-runner/src/test/java/tech/picnic/errorprone/refaster/runner/RefasterTest.java +++ b/refaster-runner/src/test/java/tech/picnic/errorprone/refaster/runner/RefasterTest.java @@ -198,17 +198,13 @@ private static ImmutableList extractRefasterSeverities( } private static SeverityLevel toSeverityLevel(String compilerDiagnosticsPrefix) { - switch (compilerDiagnosticsPrefix) { - case "Note": - return SUGGESTION; - case "warning": - return WARNING; - case "error": - return ERROR; - default: - throw new IllegalStateException( - String.format("Unrecognized diagnostics prefix '%s'", compilerDiagnosticsPrefix)); - } + return switch (compilerDiagnosticsPrefix) { + case "Note" -> SUGGESTION; + case "warning" -> WARNING; + case "error" -> ERROR; + default -> throw new IllegalStateException( + String.format("Unrecognized diagnostics prefix '%s'", compilerDiagnosticsPrefix)); + }; } @Test From 42a6db5b35341b6628367af49bf45aea8b8b4500 Mon Sep 17 00:00:00 2001 From: Stephan Schroevers Date: Thu, 27 Apr 2023 14:34:40 +0200 Subject: [PATCH 06/10] Update some comments --- .../picnic/errorprone/refasterrules/StringRules.java | 12 +++++++++--- 1 file changed, 9 insertions(+), 3 deletions(-) diff --git a/error-prone-contrib/src/main/java/tech/picnic/errorprone/refasterrules/StringRules.java b/error-prone-contrib/src/main/java/tech/picnic/errorprone/refasterrules/StringRules.java index c45c0550f57..3d43a966688 100644 --- a/error-prone-contrib/src/main/java/tech/picnic/errorprone/refasterrules/StringRules.java +++ b/error-prone-contrib/src/main/java/tech/picnic/errorprone/refasterrules/StringRules.java @@ -29,7 +29,9 @@ final class StringRules { private StringRules() {} /** Prefer {@link String#isEmpty()} over alternatives that consult the string's length. */ - // XXX: Once we target JDK 15+, generalize this rule to cover all `CharSequence` subtypes. + // XXX: Now that we build with JDK 15+, this rule can be generalized to cover all `CharSequence` + // subtypes. This does require a mechanism (perhaps an annotation, or a separate Maven module) to + // make sure that non-String expressions are rewritten only if client code also targets JDK 15+. static final class StringIsEmpty { @BeforeTemplate boolean before(String str) { @@ -44,7 +46,9 @@ boolean after(String str) { } /** Prefer a method reference to {@link String#isEmpty()} over the equivalent lambda function. */ - // XXX: Once we target JDK 15+, generalize this rule to cover all `CharSequence` subtypes. + // XXX: Now that we build with JDK 15+, this rule can be generalized to cover all `CharSequence` + // subtypes. But `CharSequence::isEmpty` isn't as nice as `String::isEmpty`, so we might want to + // introduce a rule that suggests `String::isEmpty` where possible. // XXX: As it stands, this rule is a special case of what `MethodReferenceUsage` tries to achieve. // If/when `MethodReferenceUsage` becomes production ready, we should simply drop this check. static final class StringIsEmptyPredicate { @@ -60,7 +64,9 @@ Predicate after() { } /** Prefer a method reference to {@link String#isEmpty()} over the equivalent lambda function. */ - // XXX: Once we target JDK 15+, generalize this rule to cover all `CharSequence` subtypes. + // XXX: Now that we build with JDK 15+, this rule can be generalized to cover all `CharSequence` + // subtypes. But `CharSequence::isEmpty` isn't as nice as `String::isEmpty`, so we might want to + // introduce a rule that suggests `String::isEmpty` where possible. static final class StringIsNotEmptyPredicate { @BeforeTemplate Predicate before() { From c8fdc95390c95025293b610c8071ce15e334fdda Mon Sep 17 00:00:00 2001 From: Stephan Schroevers Date: Thu, 27 Apr 2023 15:30:51 +0200 Subject: [PATCH 07/10] Use `instanceof` pattern matching --- .../bugpatterns/AmbiguousJsonCreator.java | 4 +- .../CanonicalAnnotationSyntax.java | 14 ++--- .../errorprone/bugpatterns/DirectReturn.java | 14 +++-- .../FormatStringConcatenation.java | 4 +- .../bugpatterns/IsInstanceLambdaUsage.java | 5 +- .../bugpatterns/JUnitValueSource.java | 7 ++- ...cographicalAnnotationAttributeListing.java | 15 ++---- .../bugpatterns/MethodReferenceUsage.java | 47 ++++++++-------- .../MissingRefasterAnnotation.java | 2 +- .../MockitoMockClassReference.java | 1 + .../bugpatterns/PrimitiveComparison.java | 54 ++++++++++--------- .../RedundantStringConversion.java | 10 ++-- .../bugpatterns/SpringMvcAnnotation.java | 23 ++++---- .../errorprone/bugpatterns/StaticImport.java | 10 ++-- .../util/AnnotationAttributeMatcher.java | 5 +- .../bugpatterns/util/MoreJUnitMatchers.java | 9 ++-- .../matchers/ThrowsCheckedException.java | 4 +- .../matchers/AbstractMatcherTestChecker.java | 2 +- .../refaster/test/RefasterRuleCollection.java | 4 +- 19 files changed, 109 insertions(+), 125 deletions(-) diff --git a/error-prone-contrib/src/main/java/tech/picnic/errorprone/bugpatterns/AmbiguousJsonCreator.java b/error-prone-contrib/src/main/java/tech/picnic/errorprone/bugpatterns/AmbiguousJsonCreator.java index 8eec308ef17..8958aff4073 100644 --- a/error-prone-contrib/src/main/java/tech/picnic/errorprone/bugpatterns/AmbiguousJsonCreator.java +++ b/error-prone-contrib/src/main/java/tech/picnic/errorprone/bugpatterns/AmbiguousJsonCreator.java @@ -18,7 +18,7 @@ import com.sun.source.tree.AnnotationTree; import com.sun.source.tree.ClassTree; import com.sun.source.tree.MethodTree; -import com.sun.source.tree.Tree; +import com.sun.source.tree.Tree.Kind; import com.sun.tools.javac.code.Symbol; import java.util.Map; import javax.lang.model.element.AnnotationValue; @@ -46,7 +46,7 @@ public Description matchAnnotation(AnnotationTree tree, VisitorState state) { } ClassTree clazz = state.findEnclosing(ClassTree.class); - if (clazz == null || clazz.getKind() != Tree.Kind.ENUM) { + if (clazz == null || clazz.getKind() != Kind.ENUM) { return Description.NO_MATCH; } diff --git a/error-prone-contrib/src/main/java/tech/picnic/errorprone/bugpatterns/CanonicalAnnotationSyntax.java b/error-prone-contrib/src/main/java/tech/picnic/errorprone/bugpatterns/CanonicalAnnotationSyntax.java index b2073c71674..7851df4f2da 100644 --- a/error-prone-contrib/src/main/java/tech/picnic/errorprone/bugpatterns/CanonicalAnnotationSyntax.java +++ b/error-prone-contrib/src/main/java/tech/picnic/errorprone/bugpatterns/CanonicalAnnotationSyntax.java @@ -19,7 +19,6 @@ import com.sun.source.tree.AssignmentTree; import com.sun.source.tree.ExpressionTree; import com.sun.source.tree.NewArrayTree; -import com.sun.source.tree.Tree.Kind; import java.util.ArrayList; import java.util.List; import java.util.Optional; @@ -119,7 +118,7 @@ private static Optional dropRedundantCurlies(AnnotationTree tree, VisitorSt * the expression as a whole. */ ExpressionTree value = - (arg.getKind() == Kind.ASSIGNMENT) ? ((AssignmentTree) arg).getExpression() : arg; + (arg instanceof AssignmentTree assignment) ? assignment.getExpression() : arg; /* Store a fix for each expression that was successfully simplified. */ simplifyAttributeValue(value, state) @@ -130,13 +129,10 @@ private static Optional dropRedundantCurlies(AnnotationTree tree, VisitorSt } private static Optional simplifyAttributeValue(ExpressionTree expr, VisitorState state) { - if (expr.getKind() != Kind.NEW_ARRAY) { - /* There are no curly braces or commas to be dropped here. */ - return Optional.empty(); - } - - NewArrayTree array = (NewArrayTree) expr; - return simplifySingletonArray(array, state).or(() -> dropTrailingComma(array, state)); + /* Drop curly braces or commas if possible. */ + return expr instanceof NewArrayTree newArray + ? simplifySingletonArray(newArray, state).or(() -> dropTrailingComma(newArray, state)) + : Optional.empty(); } /** Returns the expression describing the array's sole element, if any. */ diff --git a/error-prone-contrib/src/main/java/tech/picnic/errorprone/bugpatterns/DirectReturn.java b/error-prone-contrib/src/main/java/tech/picnic/errorprone/bugpatterns/DirectReturn.java index 212d0b12455..d6c40eb72ef 100644 --- a/error-prone-contrib/src/main/java/tech/picnic/errorprone/bugpatterns/DirectReturn.java +++ b/error-prone-contrib/src/main/java/tech/picnic/errorprone/bugpatterns/DirectReturn.java @@ -98,19 +98,17 @@ public Description matchBlock(BlockTree tree, VisitorState state) { } private static Optional tryMatchAssignment(Symbol targetSymbol, Tree tree) { - if (tree instanceof ExpressionStatementTree) { - return tryMatchAssignment(targetSymbol, ((ExpressionStatementTree) tree).getExpression()); + if (tree instanceof ExpressionStatementTree expressionStatement) { + return tryMatchAssignment(targetSymbol, expressionStatement.getExpression()); } - if (tree instanceof AssignmentTree) { - AssignmentTree assignment = (AssignmentTree) tree; + if (tree instanceof AssignmentTree assignment) { return targetSymbol.equals(ASTHelpers.getSymbol(assignment.getVariable())) ? Optional.of(assignment.getExpression()) : Optional.empty(); } - if (tree instanceof VariableTree) { - VariableTree declaration = (VariableTree) tree; + if (tree instanceof VariableTree declaration) { return declaration.getModifiers().getAnnotations().isEmpty() && targetSymbol.equals(ASTHelpers.getSymbol(declaration)) ? Optional.ofNullable(declaration.getInitializer()) @@ -148,11 +146,11 @@ private static boolean isIdentifierSymbolReferencedInAssociatedFinallyBlock( Streams.stream(state.getPath()).skip(1), Streams.stream(state.getPath()), (tree, child) -> { - if (!(tree instanceof TryTree)) { + if (!(tree instanceof TryTree tryTree)) { return null; } - BlockTree finallyBlock = ((TryTree) tree).getFinallyBlock(); + BlockTree finallyBlock = tryTree.getFinallyBlock(); return !child.equals(finallyBlock) ? finallyBlock : null; }) .anyMatch(finallyBlock -> referencesIdentifierSymbol(symbol, finallyBlock)); diff --git a/error-prone-contrib/src/main/java/tech/picnic/errorprone/bugpatterns/FormatStringConcatenation.java b/error-prone-contrib/src/main/java/tech/picnic/errorprone/bugpatterns/FormatStringConcatenation.java index 4c8f1fdd4ae..1e19df51575 100644 --- a/error-prone-contrib/src/main/java/tech/picnic/errorprone/bugpatterns/FormatStringConcatenation.java +++ b/error-prone-contrib/src/main/java/tech/picnic/errorprone/bugpatterns/FormatStringConcatenation.java @@ -240,8 +240,8 @@ private static class ReplacementArgumentsConstructor } private void appendExpression(Tree tree) { - if (tree instanceof LiteralTree) { - formatString.append(((LiteralTree) tree).getValue()); + if (tree instanceof LiteralTree literal) { + formatString.append(literal.getValue()); } else { formatString.append(formatSpecifier); formatArguments.add(tree); diff --git a/error-prone-contrib/src/main/java/tech/picnic/errorprone/bugpatterns/IsInstanceLambdaUsage.java b/error-prone-contrib/src/main/java/tech/picnic/errorprone/bugpatterns/IsInstanceLambdaUsage.java index 75c4e1b6309..0490ac54a5e 100644 --- a/error-prone-contrib/src/main/java/tech/picnic/errorprone/bugpatterns/IsInstanceLambdaUsage.java +++ b/error-prone-contrib/src/main/java/tech/picnic/errorprone/bugpatterns/IsInstanceLambdaUsage.java @@ -16,7 +16,6 @@ import com.google.errorprone.util.ASTHelpers; import com.sun.source.tree.InstanceOfTree; import com.sun.source.tree.LambdaExpressionTree; -import com.sun.source.tree.Tree.Kind; import com.sun.source.tree.VariableTree; import tech.picnic.errorprone.bugpatterns.util.SourceCode; @@ -42,12 +41,12 @@ public IsInstanceLambdaUsage() {} @Override public Description matchLambdaExpression(LambdaExpressionTree tree, VisitorState state) { - if (tree.getParameters().size() != 1 || tree.getBody().getKind() != Kind.INSTANCE_OF) { + if (tree.getParameters().size() != 1 + || !(tree.getBody() instanceof InstanceOfTree instanceOf)) { return Description.NO_MATCH; } VariableTree param = Iterables.getOnlyElement(tree.getParameters()); - InstanceOfTree instanceOf = (InstanceOfTree) tree.getBody(); if (!ASTHelpers.getSymbol(param).equals(ASTHelpers.getSymbol(instanceOf.getExpression()))) { return Description.NO_MATCH; } diff --git a/error-prone-contrib/src/main/java/tech/picnic/errorprone/bugpatterns/JUnitValueSource.java b/error-prone-contrib/src/main/java/tech/picnic/errorprone/bugpatterns/JUnitValueSource.java index 75fc9ee39aa..14bd20b9a1b 100644 --- a/error-prone-contrib/src/main/java/tech/picnic/errorprone/bugpatterns/JUnitValueSource.java +++ b/error-prone-contrib/src/main/java/tech/picnic/errorprone/bugpatterns/JUnitValueSource.java @@ -257,8 +257,8 @@ private static Optional tryExtractValueSourceAttributeValue( arguments.stream() .map( arg -> - arg instanceof MethodInvocationTree - ? Iterables.getOnlyElement(((MethodInvocationTree) arg).getArguments()) + arg instanceof MethodInvocationTree methodInvocation + ? Iterables.getOnlyElement(methodInvocation.getArguments()) : arg) .map(argument -> SourceCode.treeToString(argument, state)) .collect(joining(", "))) @@ -285,11 +285,10 @@ private static Optional getElementIfSingleton(Collection collection) { private static Matcher isSingleDimensionArrayCreationWithAllElementsMatching( Matcher elementMatcher) { return (tree, state) -> { - if (!(tree instanceof NewArrayTree)) { + if (!(tree instanceof NewArrayTree newArray)) { return false; } - NewArrayTree newArray = (NewArrayTree) tree; return newArray.getDimensions().isEmpty() && !newArray.getInitializers().isEmpty() && newArray.getInitializers().stream() diff --git a/error-prone-contrib/src/main/java/tech/picnic/errorprone/bugpatterns/LexicographicalAnnotationAttributeListing.java b/error-prone-contrib/src/main/java/tech/picnic/errorprone/bugpatterns/LexicographicalAnnotationAttributeListing.java index bd346140a55..46d0cd89cf0 100644 --- a/error-prone-contrib/src/main/java/tech/picnic/errorprone/bugpatterns/LexicographicalAnnotationAttributeListing.java +++ b/error-prone-contrib/src/main/java/tech/picnic/errorprone/bugpatterns/LexicographicalAnnotationAttributeListing.java @@ -31,7 +31,6 @@ import com.sun.source.tree.NewArrayTree; import com.sun.source.tree.PrimitiveTypeTree; import com.sun.source.tree.Tree; -import com.sun.source.tree.Tree.Kind; import com.sun.source.util.TreeScanner; import com.sun.tools.javac.code.Symtab; import com.sun.tools.javac.code.Type; @@ -119,13 +118,9 @@ private Optional sortArrayElements(AnnotationTree tree, VisitorState state) } private static Optional extractArray(ExpressionTree expr) { - if (expr.getKind() == Kind.ASSIGNMENT) { - return extractArray(((AssignmentTree) expr).getExpression()); - } - - return Optional.of(expr) - .filter(e -> e.getKind() == Kind.NEW_ARRAY) - .map(NewArrayTree.class::cast); + return expr instanceof AssignmentTree assignment + ? extractArray(assignment.getExpression()) + : Optional.of(expr).filter(NewArrayTree.class::isInstance).map(NewArrayTree.class::cast); } private static Optional suggestSorting( @@ -197,8 +192,8 @@ private static ImmutableList> getStructure(ExpressionTree public @Nullable Void visitLiteral(LiteralTree node, @Nullable Void unused) { Object value = ASTHelpers.constValue(node); nodes.add( - value instanceof String - ? STRING_ARGUMENT_SPLITTER.splitToStream((String) value).collect(toImmutableList()) + value instanceof String str + ? STRING_ARGUMENT_SPLITTER.splitToStream(str).collect(toImmutableList()) : ImmutableList.of(String.valueOf(value))); return super.visitLiteral(node, unused); diff --git a/error-prone-contrib/src/main/java/tech/picnic/errorprone/bugpatterns/MethodReferenceUsage.java b/error-prone-contrib/src/main/java/tech/picnic/errorprone/bugpatterns/MethodReferenceUsage.java index 2825dd6fc83..d86bc65dd44 100644 --- a/error-prone-contrib/src/main/java/tech/picnic/errorprone/bugpatterns/MethodReferenceUsage.java +++ b/error-prone-contrib/src/main/java/tech/picnic/errorprone/bugpatterns/MethodReferenceUsage.java @@ -27,7 +27,6 @@ import com.sun.source.tree.ParenthesizedTree; import com.sun.source.tree.ReturnTree; import com.sun.source.tree.Tree; -import com.sun.source.tree.Tree.Kind; import com.sun.source.tree.VariableTree; import com.sun.tools.javac.code.Symbol; import com.sun.tools.javac.code.Type; @@ -85,6 +84,7 @@ public Description matchLambdaExpression(LambdaExpressionTree tree, VisitorState .orElse(Description.NO_MATCH); } + // XXX: Use switch pattern matching once the targeted JDK supports this. private static Optional constructMethodRef( LambdaExpressionTree lambdaExpr, Tree subTree) { return switch (subTree.getKind()) { @@ -114,34 +114,35 @@ private static Optional constructMethodRef( .flatMap(expectedInstance -> constructMethodRef(lambdaExpr, subTree, expectedInstance)); } - @SuppressWarnings( - "java:S1151" /* Extracting `IDENTIFIER` case block to separate method does not improve readability. */) + // XXX: Review whether to use switch pattern matching once the targeted JDK supports this. private static Optional constructMethodRef( LambdaExpressionTree lambdaExpr, MethodInvocationTree subTree, Optional expectedInstance) { ExpressionTree methodSelect = subTree.getMethodSelect(); - return switch (methodSelect.getKind()) { - case IDENTIFIER -> { - if (expectedInstance.isPresent()) { - /* Direct method call; there is no matching "implicit parameter". */ - yield Optional.empty(); - } - Symbol sym = ASTHelpers.getSymbol(methodSelect); - yield ASTHelpers.isStatic(sym) - ? constructFix(lambdaExpr, sym.owner, methodSelect) - : constructFix(lambdaExpr, "this", methodSelect); + + if (methodSelect instanceof IdentifierTree) { + if (expectedInstance.isPresent()) { + /* Direct method call; there is no matching "implicit parameter". */ + return Optional.empty(); } - case MEMBER_SELECT -> constructMethodRef( - lambdaExpr, (MemberSelectTree) methodSelect, expectedInstance); - default -> throw new VerifyException( - "Unexpected type of expression: " + methodSelect.getKind()); - }; + + Symbol sym = ASTHelpers.getSymbol(methodSelect); + return ASTHelpers.isStatic(sym) + ? constructFix(lambdaExpr, sym.owner, methodSelect) + : constructFix(lambdaExpr, "this", methodSelect); + } + + if (methodSelect instanceof MemberSelectTree memberSelect) { + return constructMethodRef(lambdaExpr, memberSelect, expectedInstance); + } + + throw new VerifyException("Unexpected type of expression: " + methodSelect.getKind()); } private static Optional constructMethodRef( LambdaExpressionTree lambdaExpr, MemberSelectTree subTree, Optional expectedInstance) { - if (subTree.getExpression().getKind() != Kind.IDENTIFIER) { + if (!(subTree.getExpression() instanceof IdentifierTree identifier)) { // XXX: Could be parenthesized. Handle. Also in other classes. /* * Only suggest a replacement if the method select's expression provably doesn't have @@ -150,12 +151,12 @@ private static Optional constructMethodRef( return Optional.empty(); } - Name lhs = ((IdentifierTree) subTree.getExpression()).getName(); + Name lhs = identifier.getName(); if (expectedInstance.isEmpty()) { return constructFix(lambdaExpr, lhs, subTree.getIdentifier()); } - Type lhsType = ASTHelpers.getType(subTree.getExpression()); + Type lhsType = ASTHelpers.getType(identifier); if (lhsType == null || !expectedInstance.orElseThrow().equals(lhs)) { return Optional.empty(); } @@ -180,8 +181,8 @@ private static Optional> matchArguments( for (int i = 0; i < args.size(); i++) { ExpressionTree arg = args.get(i); - if (arg.getKind() != Kind.IDENTIFIER - || !((IdentifierTree) arg).getName().equals(expectedArguments.get(i + diff))) { + if (!(arg instanceof IdentifierTree identifier) + || !identifier.getName().equals(expectedArguments.get(i + diff))) { return Optional.empty(); } } diff --git a/error-prone-contrib/src/main/java/tech/picnic/errorprone/bugpatterns/MissingRefasterAnnotation.java b/error-prone-contrib/src/main/java/tech/picnic/errorprone/bugpatterns/MissingRefasterAnnotation.java index 12c71af6199..c8cdbb9eed2 100644 --- a/error-prone-contrib/src/main/java/tech/picnic/errorprone/bugpatterns/MissingRefasterAnnotation.java +++ b/error-prone-contrib/src/main/java/tech/picnic/errorprone/bugpatterns/MissingRefasterAnnotation.java @@ -47,7 +47,7 @@ public MissingRefasterAnnotation() {} public Description matchClass(ClassTree tree, VisitorState state) { long methodTypes = tree.getMembers().stream() - .filter(member -> member.getKind() == Tree.Kind.METHOD) + .filter(MethodTree.class::isInstance) .map(MethodTree.class::cast) .filter(method -> !ASTHelpers.isGeneratedConstructor(method)) .map(method -> REFASTER_ANNOTATION.matches(method, state)) diff --git a/error-prone-contrib/src/main/java/tech/picnic/errorprone/bugpatterns/MockitoMockClassReference.java b/error-prone-contrib/src/main/java/tech/picnic/errorprone/bugpatterns/MockitoMockClassReference.java index 1ed193468f8..be164b277d3 100644 --- a/error-prone-contrib/src/main/java/tech/picnic/errorprone/bugpatterns/MockitoMockClassReference.java +++ b/error-prone-contrib/src/main/java/tech/picnic/errorprone/bugpatterns/MockitoMockClassReference.java @@ -67,6 +67,7 @@ public Description matchMethodInvocation(MethodInvocationTree tree, VisitorState return describeMatch(tree, SuggestedFixes.removeElement(arguments.get(0), arguments, state)); } + // XXX: Use switch pattern matching once the targeted JDK supports this. private static boolean isTypeDerivableFromContext(MethodInvocationTree tree, VisitorState state) { Tree parent = state.getPath().getParentPath().getLeaf(); return switch (parent.getKind()) { diff --git a/error-prone-contrib/src/main/java/tech/picnic/errorprone/bugpatterns/PrimitiveComparison.java b/error-prone-contrib/src/main/java/tech/picnic/errorprone/bugpatterns/PrimitiveComparison.java index 2ab00020ee8..997a52f644e 100644 --- a/error-prone-contrib/src/main/java/tech/picnic/errorprone/bugpatterns/PrimitiveComparison.java +++ b/error-prone-contrib/src/main/java/tech/picnic/errorprone/bugpatterns/PrimitiveComparison.java @@ -21,6 +21,7 @@ import com.google.errorprone.matchers.Matcher; import com.google.errorprone.util.ASTHelpers; import com.sun.source.tree.ExpressionTree; +import com.sun.source.tree.IdentifierTree; import com.sun.source.tree.LambdaExpressionTree; import com.sun.source.tree.MemberSelectTree; import com.sun.source.tree.MethodInvocationTree; @@ -146,39 +147,42 @@ private static String getPreferredMethod(Type cmpType, boolean isStatic, Visitor return isStatic ? "comparing" : "thenComparing"; } + // XXX: Use switch pattern matching once the targeted JDK supports this. private static Optional getPotentiallyBoxedReturnType(ExpressionTree tree) { - return switch (tree.getKind()) { - case LAMBDA_EXPRESSION -> { - /* Return the lambda expression's actual return type. */ - yield Optional.ofNullable(ASTHelpers.getType(((LambdaExpressionTree) tree).getBody())); - } - case MEMBER_REFERENCE -> { - /* Return the method's declared return type. */ - // XXX: Very fragile. Do better. - Type subType2 = ((JCMemberReference) tree).referentType; - yield Optional.of(subType2.getReturnType()); - } - default -> { - /* This appears to be a genuine `{,ToInt,ToLong,ToDouble}Function`. */ - yield Optional.empty(); - } - }; + if (tree instanceof LambdaExpressionTree lambdaExpression) { + /* Return the lambda expression's actual return type. */ + return Optional.ofNullable(ASTHelpers.getType(lambdaExpression.getBody())); + } + + // XXX: The match against a concrete type and reference to one of its fields is fragile. Do + // better. + if (tree instanceof JCMemberReference memberReference) { + /* Return the method's declared return type. */ + Type subType = memberReference.referentType; + return Optional.of(subType.getReturnType()); + } + + /* This appears to be a genuine `{,ToInt,ToLong,ToDouble}Function`. */ + return Optional.empty(); } private static Fix suggestFix( MethodInvocationTree tree, String preferredMethodName, VisitorState state) { ExpressionTree expr = tree.getMethodSelect(); - return switch (expr.getKind()) { - case IDENTIFIER -> SuggestedFix.builder() + + if (expr instanceof IdentifierTree) { + return SuggestedFix.builder() .addStaticImport(Comparator.class.getName() + '.' + preferredMethodName) .replace(expr, preferredMethodName) .build(); - case MEMBER_SELECT -> { - MemberSelectTree ms = (MemberSelectTree) tree.getMethodSelect(); - yield SuggestedFix.replace( - ms, SourceCode.treeToString(ms.getExpression(), state) + '.' + preferredMethodName); - } - default -> throw new VerifyException("Unexpected type of expression: " + expr.getKind()); - }; + } + + if (expr instanceof MemberSelectTree memberSelect) { + return SuggestedFix.replace( + memberSelect, + SourceCode.treeToString(memberSelect.getExpression(), state) + '.' + preferredMethodName); + } + + throw new VerifyException("Unexpected type of expression: " + expr.getKind()); } } diff --git a/error-prone-contrib/src/main/java/tech/picnic/errorprone/bugpatterns/RedundantStringConversion.java b/error-prone-contrib/src/main/java/tech/picnic/errorprone/bugpatterns/RedundantStringConversion.java index bd670101b8e..aed928259b4 100644 --- a/error-prone-contrib/src/main/java/tech/picnic/errorprone/bugpatterns/RedundantStringConversion.java +++ b/error-prone-contrib/src/main/java/tech/picnic/errorprone/bugpatterns/RedundantStringConversion.java @@ -325,11 +325,10 @@ private Optional trySimplify( } private Optional trySimplify(ExpressionTree tree, VisitorState state) { - if (tree.getKind() != Kind.METHOD_INVOCATION) { + if (!(tree instanceof MethodInvocationTree methodInvocation)) { return Optional.empty(); } - MethodInvocationTree methodInvocation = (MethodInvocationTree) tree; if (!conversionMethodMatcher.matches(methodInvocation, state)) { return Optional.empty(); } @@ -345,13 +344,12 @@ private Optional trySimplify(ExpressionTree tree, VisitorState s private static Optional trySimplifyNullaryMethod( MethodInvocationTree methodInvocation, VisitorState state) { - if (!instanceMethod().matches(methodInvocation, state)) { + if (!instanceMethod().matches(methodInvocation, state) + || !(methodInvocation.getMethodSelect() instanceof MemberSelectTree memberSelect)) { return Optional.empty(); } - return Optional.of(methodInvocation.getMethodSelect()) - .filter(methodSelect -> methodSelect.getKind() == Kind.MEMBER_SELECT) - .map(methodSelect -> ((MemberSelectTree) methodSelect).getExpression()) + return Optional.of(memberSelect.getExpression()) .filter(expr -> !"super".equals(SourceCode.treeToString(expr, state))); } diff --git a/error-prone-contrib/src/main/java/tech/picnic/errorprone/bugpatterns/SpringMvcAnnotation.java b/error-prone-contrib/src/main/java/tech/picnic/errorprone/bugpatterns/SpringMvcAnnotation.java index 1fa027d2632..614a12e748c 100644 --- a/error-prone-contrib/src/main/java/tech/picnic/errorprone/bugpatterns/SpringMvcAnnotation.java +++ b/error-prone-contrib/src/main/java/tech/picnic/errorprone/bugpatterns/SpringMvcAnnotation.java @@ -1,6 +1,5 @@ package tech.picnic.errorprone.bugpatterns; -import static com.google.common.base.Verify.verify; import static com.google.errorprone.BugPattern.LinkType.CUSTOM; import static com.google.errorprone.BugPattern.SeverityLevel.SUGGESTION; import static com.google.errorprone.BugPattern.StandardTags.SIMPLIFICATION; @@ -24,7 +23,6 @@ import com.sun.source.tree.ExpressionTree; import com.sun.source.tree.MemberSelectTree; import com.sun.source.tree.NewArrayTree; -import com.sun.source.tree.Tree.Kind; import java.util.Optional; import tech.picnic.errorprone.bugpatterns.util.AnnotationAttributeMatcher; import tech.picnic.errorprone.bugpatterns.util.SourceCode; @@ -79,22 +77,19 @@ private static Optional trySimplification( } private static Optional extractUniqueMethod(ExpressionTree arg, VisitorState state) { - verify( - arg.getKind() == Kind.ASSIGNMENT, - "Annotation attribute is not an assignment: %s", - arg.getKind()); - - ExpressionTree expr = ((AssignmentTree) arg).getExpression(); - if (expr.getKind() != Kind.NEW_ARRAY) { - return Optional.of(extractMethod(expr, state)); + if (!(arg instanceof AssignmentTree assignment)) { + throw new VerifyException("Annotation attribute is not an assignment:" + arg.getKind()); } - NewArrayTree newArray = (NewArrayTree) expr; - return Optional.of(newArray.getInitializers()) - .filter(args -> args.size() == 1) - .map(args -> extractMethod(args.get(0), state)); + ExpressionTree expr = assignment.getExpression(); + return expr instanceof NewArrayTree newArray + ? Optional.of(newArray.getInitializers()) + .filter(args -> args.size() == 1) + .map(args -> extractMethod(args.get(0), state)) + : Optional.of(extractMethod(expr, state)); } + // XXX: Use switch pattern matching once the targeted JDK supports this. private static String extractMethod(ExpressionTree expr, VisitorState state) { return switch (expr.getKind()) { case IDENTIFIER -> SourceCode.treeToString(expr, state); diff --git a/error-prone-contrib/src/main/java/tech/picnic/errorprone/bugpatterns/StaticImport.java b/error-prone-contrib/src/main/java/tech/picnic/errorprone/bugpatterns/StaticImport.java index ae6a7928563..83225abf35a 100644 --- a/error-prone-contrib/src/main/java/tech/picnic/errorprone/bugpatterns/StaticImport.java +++ b/error-prone-contrib/src/main/java/tech/picnic/errorprone/bugpatterns/StaticImport.java @@ -24,6 +24,7 @@ import com.sun.source.tree.MemberSelectTree; import com.sun.source.tree.MethodInvocationTree; import com.sun.source.tree.Tree; +import com.sun.source.tree.Tree.Kind; import com.sun.tools.javac.code.Type; import java.util.Optional; @@ -215,11 +216,10 @@ private static boolean isCandidateContext(VisitorState state) { Tree parentTree = requireNonNull(state.getPath().getParentPath(), "MemberSelectTree lacks enclosing node") .getLeaf(); - return switch (parentTree.getKind()) { - case IMPORT, MEMBER_SELECT -> false; - case METHOD_INVOCATION -> ((MethodInvocationTree) parentTree).getTypeArguments().isEmpty(); - default -> true; - }; + + return parentTree instanceof MethodInvocationTree methodInvocation + ? methodInvocation.getTypeArguments().isEmpty() + : (parentTree.getKind() != Kind.IMPORT && parentTree.getKind() != Kind.MEMBER_SELECT); } private static boolean isCandidate(MemberSelectTree tree) { diff --git a/error-prone-contrib/src/main/java/tech/picnic/errorprone/bugpatterns/util/AnnotationAttributeMatcher.java b/error-prone-contrib/src/main/java/tech/picnic/errorprone/bugpatterns/util/AnnotationAttributeMatcher.java index fc10d03f45c..ca4bbfb741d 100644 --- a/error-prone-contrib/src/main/java/tech/picnic/errorprone/bugpatterns/util/AnnotationAttributeMatcher.java +++ b/error-prone-contrib/src/main/java/tech/picnic/errorprone/bugpatterns/util/AnnotationAttributeMatcher.java @@ -9,7 +9,6 @@ import com.sun.source.tree.AnnotationTree; import com.sun.source.tree.AssignmentTree; import com.sun.source.tree.ExpressionTree; -import com.sun.source.tree.Tree.Kind; import com.sun.tools.javac.code.Type; import java.io.Serializable; import java.util.HashSet; @@ -116,8 +115,8 @@ public Stream extractMatchingArguments(AnnotationTree tree) { } private static String extractAttributeName(ExpressionTree expr) { - return (expr.getKind() == Kind.ASSIGNMENT) - ? ASTHelpers.getSymbol(((AssignmentTree) expr).getVariable()).getSimpleName().toString() + return (expr instanceof AssignmentTree assignment) + ? ASTHelpers.getSymbol(assignment.getVariable()).getSimpleName().toString() : "value"; } diff --git a/error-prone-contrib/src/main/java/tech/picnic/errorprone/bugpatterns/util/MoreJUnitMatchers.java b/error-prone-contrib/src/main/java/tech/picnic/errorprone/bugpatterns/util/MoreJUnitMatchers.java index 04b230b61e6..6ca0a850298 100644 --- a/error-prone-contrib/src/main/java/tech/picnic/errorprone/bugpatterns/util/MoreJUnitMatchers.java +++ b/error-prone-contrib/src/main/java/tech/picnic/errorprone/bugpatterns/util/MoreJUnitMatchers.java @@ -97,14 +97,13 @@ public static ImmutableList getMethodSourceFactoryDescriptors( String methodName = method.getName().toString(); ExpressionTree value = AnnotationMatcherUtils.getArgument(methodSourceAnnotation, "value"); - if (!(value instanceof NewArrayTree)) { + if (!(value instanceof NewArrayTree newArray)) { return ImmutableList.of(toMethodSourceFactoryDescriptor(value, methodName)); } - return ((NewArrayTree) value) - .getInitializers().stream() - .map(name -> toMethodSourceFactoryDescriptor(name, methodName)) - .collect(toImmutableList()); + return newArray.getInitializers().stream() + .map(name -> toMethodSourceFactoryDescriptor(name, methodName)) + .collect(toImmutableList()); } private static String toMethodSourceFactoryDescriptor( diff --git a/refaster-support/src/main/java/tech/picnic/errorprone/refaster/matchers/ThrowsCheckedException.java b/refaster-support/src/main/java/tech/picnic/errorprone/refaster/matchers/ThrowsCheckedException.java index 15d743e6968..aa11abcdf34 100644 --- a/refaster-support/src/main/java/tech/picnic/errorprone/refaster/matchers/ThrowsCheckedException.java +++ b/refaster-support/src/main/java/tech/picnic/errorprone/refaster/matchers/ThrowsCheckedException.java @@ -29,8 +29,8 @@ public boolean matches(ExpressionTree tree, VisitorState state) { } private static Collection getThrownTypes(ExpressionTree tree, VisitorState state) { - if (tree instanceof LambdaExpressionTree) { - return ASTHelpers.getThrownExceptions(((LambdaExpressionTree) tree).getBody(), state); + if (tree instanceof LambdaExpressionTree lambdaExpression) { + return ASTHelpers.getThrownExceptions(lambdaExpression.getBody(), state); } if (tree instanceof MemberReferenceTree) { diff --git a/refaster-support/src/test/java/tech/picnic/errorprone/refaster/matchers/AbstractMatcherTestChecker.java b/refaster-support/src/test/java/tech/picnic/errorprone/refaster/matchers/AbstractMatcherTestChecker.java index c0361ab6e3e..33882a344d0 100644 --- a/refaster-support/src/test/java/tech/picnic/errorprone/refaster/matchers/AbstractMatcherTestChecker.java +++ b/refaster-support/src/test/java/tech/picnic/errorprone/refaster/matchers/AbstractMatcherTestChecker.java @@ -34,7 +34,7 @@ public Description matchCompilationUnit(CompilationUnitTree compilationUnit, Vis new TreeScanner<@Nullable Void, @Nullable Void>() { @Override public @Nullable Void scan(Tree tree, @Nullable Void unused) { - if (tree instanceof ExpressionTree && delegate.matches((ExpressionTree) tree, state)) { + if (tree instanceof ExpressionTree expression && delegate.matches(expression, state)) { state.reportMatch(describeMatch(tree)); } diff --git a/refaster-test-support/src/main/java/tech/picnic/errorprone/refaster/test/RefasterRuleCollection.java b/refaster-test-support/src/main/java/tech/picnic/errorprone/refaster/test/RefasterRuleCollection.java index 8fd5803bc4d..b5f48e0a833 100644 --- a/refaster-test-support/src/main/java/tech/picnic/errorprone/refaster/test/RefasterRuleCollection.java +++ b/refaster-test-support/src/main/java/tech/picnic/errorprone/refaster/test/RefasterRuleCollection.java @@ -152,8 +152,8 @@ private void reportIncorrectClassName(CompilationUnitTree tree, VisitorState sta String expectedClassName = ruleCollectionUnderTest + "Test"; for (Tree typeDeclaration : tree.getTypeDecls()) { - if (typeDeclaration instanceof ClassTree) { - if (!((ClassTree) typeDeclaration).getSimpleName().contentEquals(expectedClassName)) { + if (typeDeclaration instanceof ClassTree classTree) { + if (!classTree.getSimpleName().contentEquals(expectedClassName)) { state.reportMatch( describeMatch( typeDeclaration, From c52841ef2a98359fb5a318b58a3373414b4efae6 Mon Sep 17 00:00:00 2001 From: Stephan Schroevers Date: Thu, 27 Apr 2023 16:50:08 +0200 Subject: [PATCH 08/10] Some more text blocks --- .../picnic/errorprone/bugpatterns/FluxFlatMapUsage.java | 5 +++-- .../picnic/errorprone/bugpatterns/IdentityConversion.java | 5 +++-- .../errorprone/bugpatterns/RequestMappingAnnotation.java | 7 ++++--- 3 files changed, 10 insertions(+), 7 deletions(-) diff --git a/error-prone-contrib/src/main/java/tech/picnic/errorprone/bugpatterns/FluxFlatMapUsage.java b/error-prone-contrib/src/main/java/tech/picnic/errorprone/bugpatterns/FluxFlatMapUsage.java index 16e55d4cdd6..ba904566610 100644 --- a/error-prone-contrib/src/main/java/tech/picnic/errorprone/bugpatterns/FluxFlatMapUsage.java +++ b/error-prone-contrib/src/main/java/tech/picnic/errorprone/bugpatterns/FluxFlatMapUsage.java @@ -50,8 +50,9 @@ @AutoService(BugChecker.class) @BugPattern( summary = - "`Flux#flatMap` and `Flux#flatMapSequential` have subtle semantics; " - + "please use `Flux#concatMap` or explicitly specify the desired amount of concurrency", + """ + `Flux#flatMap` and `Flux#flatMapSequential` have subtle semantics; please use \ + `Flux#concatMap` or explicitly specify the desired amount of concurrency""", link = BUG_PATTERNS_BASE_URL + "FluxFlatMapUsage", linkType = CUSTOM, severity = ERROR, diff --git a/error-prone-contrib/src/main/java/tech/picnic/errorprone/bugpatterns/IdentityConversion.java b/error-prone-contrib/src/main/java/tech/picnic/errorprone/bugpatterns/IdentityConversion.java index 0d4d3f2769a..8bf2a55ecc5 100644 --- a/error-prone-contrib/src/main/java/tech/picnic/errorprone/bugpatterns/IdentityConversion.java +++ b/error-prone-contrib/src/main/java/tech/picnic/errorprone/bugpatterns/IdentityConversion.java @@ -112,8 +112,9 @@ public Description matchMethodInvocation(MethodInvocationTree tree, VisitorState return buildDescription(tree) .setMessage( - "This method invocation appears redundant; remove it or suppress this warning and " - + "add a comment explaining its purpose") + """ + This method invocation appears redundant; remove it or suppress this warning and add a \ + comment explaining its purpose""") .addFix(SuggestedFix.replace(tree, SourceCode.treeToString(sourceTree, state))) .addFix(SuggestedFixes.addSuppressWarnings(state, canonicalName())) .build(); diff --git a/error-prone-contrib/src/main/java/tech/picnic/errorprone/bugpatterns/RequestMappingAnnotation.java b/error-prone-contrib/src/main/java/tech/picnic/errorprone/bugpatterns/RequestMappingAnnotation.java index d8d6df2f80d..d06be93bf3c 100644 --- a/error-prone-contrib/src/main/java/tech/picnic/errorprone/bugpatterns/RequestMappingAnnotation.java +++ b/error-prone-contrib/src/main/java/tech/picnic/errorprone/bugpatterns/RequestMappingAnnotation.java @@ -99,9 +99,10 @@ public Description matchMethod(MethodTree tree, VisitorState state) { && LACKS_PARAMETER_ANNOTATION.matches(tree, state) ? buildDescription(tree) .setMessage( - "Not all parameters of this request mapping method are annotated; this may be a " - + "mistake. If the unannotated parameters represent query string parameters, " - + "annotate them with `@RequestParam`.") + """ + Not all parameters of this request mapping method are annotated; this may be a \ + mistake. If the unannotated parameters represent query string parameters, annotate \ + them with `@RequestParam`.""") .build() : Description.NO_MATCH; } From 9f653f5fca9da477f4673b88e80f0223753ac903 Mon Sep 17 00:00:00 2001 From: Picnic-Bot Date: Fri, 28 Apr 2023 19:11:50 +0000 Subject: [PATCH 09/10] Upgrade org.springframework:spring-framework-bom 5.3.27 -> 6.0.8 --- pom.xml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pom.xml b/pom.xml index 096460fe6a0..d85e477ca55 100644 --- a/pom.xml +++ b/pom.xml @@ -457,7 +457,7 @@ org.springframework spring-framework-bom - 5.3.27 + 6.0.8 pom import From 206cea6bcf4903fc74953ed8080b143007a2ebac Mon Sep 17 00:00:00 2001 From: Stephan Schroevers Date: Fri, 28 Apr 2023 22:29:21 +0200 Subject: [PATCH 10/10] Fix the build --- .../bugpatterns/AutowiredConstructorTest.java | 3 +++ .../ImmutablesSortedSetComparatorTest.java | 3 +++ .../bugpatterns/JUnitClassModifiersTest.java | 3 +++ ...aphicalAnnotationAttributeListingTest.java | 3 +++ .../RequestMappingAnnotationTest.java | 3 +++ .../bugpatterns/RequestParamTypeTest.java | 3 +++ .../ScheduledTransactionTraceTest.java | 3 +++ .../bugpatterns/SpringMvcAnnotationTest.java | 3 +++ .../bugpatterns/StaticImportTest.java | 4 ++++ .../refasterrules/RefasterRulesTest.java | 20 ++++++++++++++++++- pom.xml | 4 ++++ 11 files changed, 51 insertions(+), 1 deletion(-) diff --git a/error-prone-contrib/src/test/java/tech/picnic/errorprone/bugpatterns/AutowiredConstructorTest.java b/error-prone-contrib/src/test/java/tech/picnic/errorprone/bugpatterns/AutowiredConstructorTest.java index 12276bc5910..5f4f89b6e6f 100644 --- a/error-prone-contrib/src/test/java/tech/picnic/errorprone/bugpatterns/AutowiredConstructorTest.java +++ b/error-prone-contrib/src/test/java/tech/picnic/errorprone/bugpatterns/AutowiredConstructorTest.java @@ -4,7 +4,10 @@ import com.google.errorprone.BugCheckerRefactoringTestHelper.TestMode; import com.google.errorprone.CompilationTestHelper; import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.condition.DisabledForJreRange; +import org.junit.jupiter.api.condition.JRE; +@DisabledForJreRange(max = JRE.JAVA_16 /* Spring targets JDK 17. */) final class AutowiredConstructorTest { @Test void identification() { diff --git a/error-prone-contrib/src/test/java/tech/picnic/errorprone/bugpatterns/ImmutablesSortedSetComparatorTest.java b/error-prone-contrib/src/test/java/tech/picnic/errorprone/bugpatterns/ImmutablesSortedSetComparatorTest.java index b77767af845..47267ecfdc3 100644 --- a/error-prone-contrib/src/test/java/tech/picnic/errorprone/bugpatterns/ImmutablesSortedSetComparatorTest.java +++ b/error-prone-contrib/src/test/java/tech/picnic/errorprone/bugpatterns/ImmutablesSortedSetComparatorTest.java @@ -4,6 +4,8 @@ import com.google.errorprone.BugCheckerRefactoringTestHelper.TestMode; import com.google.errorprone.CompilationTestHelper; import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.condition.DisabledForJreRange; +import org.junit.jupiter.api.condition.JRE; final class ImmutablesSortedSetComparatorTest { @Test @@ -140,6 +142,7 @@ void replacement() { .doTest(TestMode.TEXT_MATCH); } + @DisabledForJreRange(max = JRE.JAVA_16 /* Spring targets JDK 17. */) @Test void replacementWithImportClash() { BugCheckerRefactoringTestHelper.newInstance(ImmutablesSortedSetComparator.class, getClass()) diff --git a/error-prone-contrib/src/test/java/tech/picnic/errorprone/bugpatterns/JUnitClassModifiersTest.java b/error-prone-contrib/src/test/java/tech/picnic/errorprone/bugpatterns/JUnitClassModifiersTest.java index 7d2ab285370..5ead6045cde 100644 --- a/error-prone-contrib/src/test/java/tech/picnic/errorprone/bugpatterns/JUnitClassModifiersTest.java +++ b/error-prone-contrib/src/test/java/tech/picnic/errorprone/bugpatterns/JUnitClassModifiersTest.java @@ -4,7 +4,10 @@ import com.google.errorprone.BugCheckerRefactoringTestHelper.TestMode; import com.google.errorprone.CompilationTestHelper; import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.condition.DisabledForJreRange; +import org.junit.jupiter.api.condition.JRE; +@DisabledForJreRange(max = JRE.JAVA_16 /* Spring targets JDK 17. */) final class JUnitClassModifiersTest { @Test void identification() { diff --git a/error-prone-contrib/src/test/java/tech/picnic/errorprone/bugpatterns/LexicographicalAnnotationAttributeListingTest.java b/error-prone-contrib/src/test/java/tech/picnic/errorprone/bugpatterns/LexicographicalAnnotationAttributeListingTest.java index 43fb54c0e20..a018aa74d21 100644 --- a/error-prone-contrib/src/test/java/tech/picnic/errorprone/bugpatterns/LexicographicalAnnotationAttributeListingTest.java +++ b/error-prone-contrib/src/test/java/tech/picnic/errorprone/bugpatterns/LexicographicalAnnotationAttributeListingTest.java @@ -5,8 +5,11 @@ import com.google.errorprone.BugCheckerRefactoringTestHelper.TestMode; import com.google.errorprone.CompilationTestHelper; import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.condition.DisabledForJreRange; +import org.junit.jupiter.api.condition.JRE; final class LexicographicalAnnotationAttributeListingTest { + @DisabledForJreRange(max = JRE.JAVA_16 /* Spring targets JDK 17. */) @Test void identification() { CompilationTestHelper.newInstance(LexicographicalAnnotationAttributeListing.class, getClass()) diff --git a/error-prone-contrib/src/test/java/tech/picnic/errorprone/bugpatterns/RequestMappingAnnotationTest.java b/error-prone-contrib/src/test/java/tech/picnic/errorprone/bugpatterns/RequestMappingAnnotationTest.java index 4943e76d1d9..e76724f82fa 100644 --- a/error-prone-contrib/src/test/java/tech/picnic/errorprone/bugpatterns/RequestMappingAnnotationTest.java +++ b/error-prone-contrib/src/test/java/tech/picnic/errorprone/bugpatterns/RequestMappingAnnotationTest.java @@ -2,7 +2,10 @@ import com.google.errorprone.CompilationTestHelper; import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.condition.DisabledForJreRange; +import org.junit.jupiter.api.condition.JRE; +@DisabledForJreRange(max = JRE.JAVA_16 /* Spring targets JDK 17. */) final class RequestMappingAnnotationTest { @Test void identification() { diff --git a/error-prone-contrib/src/test/java/tech/picnic/errorprone/bugpatterns/RequestParamTypeTest.java b/error-prone-contrib/src/test/java/tech/picnic/errorprone/bugpatterns/RequestParamTypeTest.java index 38616e4ea0f..cc8e8d55b88 100644 --- a/error-prone-contrib/src/test/java/tech/picnic/errorprone/bugpatterns/RequestParamTypeTest.java +++ b/error-prone-contrib/src/test/java/tech/picnic/errorprone/bugpatterns/RequestParamTypeTest.java @@ -2,7 +2,10 @@ import com.google.errorprone.CompilationTestHelper; import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.condition.DisabledForJreRange; +import org.junit.jupiter.api.condition.JRE; +@DisabledForJreRange(max = JRE.JAVA_16 /* Spring targets JDK 17. */) final class RequestParamTypeTest { @Test void identification() { diff --git a/error-prone-contrib/src/test/java/tech/picnic/errorprone/bugpatterns/ScheduledTransactionTraceTest.java b/error-prone-contrib/src/test/java/tech/picnic/errorprone/bugpatterns/ScheduledTransactionTraceTest.java index c61a99a8ca3..f420f2c9e3f 100644 --- a/error-prone-contrib/src/test/java/tech/picnic/errorprone/bugpatterns/ScheduledTransactionTraceTest.java +++ b/error-prone-contrib/src/test/java/tech/picnic/errorprone/bugpatterns/ScheduledTransactionTraceTest.java @@ -4,8 +4,11 @@ import com.google.errorprone.BugCheckerRefactoringTestHelper.TestMode; import com.google.errorprone.CompilationTestHelper; import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.condition.DisabledForJreRange; +import org.junit.jupiter.api.condition.JRE; import org.springframework.scheduling.annotation.Scheduled; +@DisabledForJreRange(max = JRE.JAVA_16 /* Spring targets JDK 17. */) final class ScheduledTransactionTraceTest { @Test void identification() { diff --git a/error-prone-contrib/src/test/java/tech/picnic/errorprone/bugpatterns/SpringMvcAnnotationTest.java b/error-prone-contrib/src/test/java/tech/picnic/errorprone/bugpatterns/SpringMvcAnnotationTest.java index b28562c0a4d..9751d4e4270 100644 --- a/error-prone-contrib/src/test/java/tech/picnic/errorprone/bugpatterns/SpringMvcAnnotationTest.java +++ b/error-prone-contrib/src/test/java/tech/picnic/errorprone/bugpatterns/SpringMvcAnnotationTest.java @@ -4,7 +4,10 @@ import com.google.errorprone.BugCheckerRefactoringTestHelper.TestMode; import com.google.errorprone.CompilationTestHelper; import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.condition.DisabledForJreRange; +import org.junit.jupiter.api.condition.JRE; +@DisabledForJreRange(max = JRE.JAVA_16 /* Spring targets JDK 17. */) final class SpringMvcAnnotationTest { @Test void identification() { diff --git a/error-prone-contrib/src/test/java/tech/picnic/errorprone/bugpatterns/StaticImportTest.java b/error-prone-contrib/src/test/java/tech/picnic/errorprone/bugpatterns/StaticImportTest.java index d876f410825..86884cc7d10 100644 --- a/error-prone-contrib/src/test/java/tech/picnic/errorprone/bugpatterns/StaticImportTest.java +++ b/error-prone-contrib/src/test/java/tech/picnic/errorprone/bugpatterns/StaticImportTest.java @@ -6,6 +6,8 @@ import com.google.errorprone.BugCheckerRefactoringTestHelper.TestMode; import com.google.errorprone.CompilationTestHelper; import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.condition.DisabledForJreRange; +import org.junit.jupiter.api.condition.JRE; final class StaticImportTest { @Test @@ -26,6 +28,7 @@ void exemptedMembersAreNotRedundant() { .doesNotContainAnyElementsOf(StaticImport.STATIC_IMPORT_EXEMPTED_IDENTIFIERS); } + @DisabledForJreRange(max = JRE.JAVA_16 /* Spring targets JDK 17. */) @Test void identification() { CompilationTestHelper.newInstance(StaticImport.class, getClass()) @@ -111,6 +114,7 @@ void identification() { .doTest(); } + @DisabledForJreRange(max = JRE.JAVA_16 /* Spring targets JDK 17. */) @Test void replacement() { BugCheckerRefactoringTestHelper.newInstance(StaticImport.class, getClass()) diff --git a/error-prone-contrib/src/test/java/tech/picnic/errorprone/refasterrules/RefasterRulesTest.java b/error-prone-contrib/src/test/java/tech/picnic/errorprone/refasterrules/RefasterRulesTest.java index 0dc5d220495..8b42f880fa6 100644 --- a/error-prone-contrib/src/test/java/tech/picnic/errorprone/refasterrules/RefasterRulesTest.java +++ b/error-prone-contrib/src/test/java/tech/picnic/errorprone/refasterrules/RefasterRulesTest.java @@ -4,6 +4,8 @@ import com.google.common.collect.ImmutableSet; import java.util.stream.Stream; +import org.junit.jupiter.api.condition.DisabledForJreRange; +import org.junit.jupiter.api.condition.JRE; import org.junit.jupiter.params.ParameterizedTest; import org.junit.jupiter.params.provider.Arguments; import org.junit.jupiter.params.provider.MethodSource; @@ -69,6 +71,10 @@ final class RefasterRulesTest { TestNGToAssertJRules.class, TimeRules.class, WebClientRules.class); + // XXX: Drop all logic related to this collection once we drop support for + // JDKs prior to version 17. + private static final ImmutableSet> JDK_17_PLUS_RULE_COLLECTIONS = + ImmutableSet.of(WebClientRules.class); // XXX: Create a JUnit extension to automatically discover the rule collections in a given context // to make sure the list is exhaustive. @@ -77,12 +83,24 @@ private static Stream validateRuleCollectionTestCases() { // method with `@ValueSource(classes = {...})`. return RULE_COLLECTIONS.stream() .filter(not(AssertJRules.class::equals)) + .filter(not(WebClientRules.class::equals)) .map(Arguments::arguments); } @MethodSource("validateRuleCollectionTestCases") @ParameterizedTest void validateRuleCollection(Class clazz) { - RefasterRuleCollection.validate(clazz); + if (!JDK_17_PLUS_RULE_COLLECTIONS.contains(clazz)) { + RefasterRuleCollection.validate(clazz); + } + } + + @DisabledForJreRange(max = JRE.JAVA_16 /* Spring targets JDK 17. */) + @MethodSource("validateRuleCollectionTestCases") + @ParameterizedTest + void validateRuleCollectionWithJdk17AndAbove(Class clazz) { + if (JDK_17_PLUS_RULE_COLLECTIONS.contains(clazz)) { + RefasterRuleCollection.validate(clazz); + } } } diff --git a/pom.xml b/pom.xml index d85e477ca55..55fdd1ef056 100644 --- a/pom.xml +++ b/pom.xml @@ -1007,6 +1007,10 @@ --> + + provided + test + ${version.jdk.target}