Compare commits

..

28 Commits

Author SHA1 Message Date
Stephan Schroevers
cf80180103 Why doesn't this match? 2024-02-21 18:46:07 +01:00
Picnic-Bot
194a828c67 Upgrade Immutables Annotations 2.10.0 -> 2.10.1 (#1045)
See:
- https://github.com/immutables/immutables/releases/tag/2.10.1
- https://github.com/immutables/immutables/compare/2.10.0...2.10.1
2024-02-20 10:25:35 +01:00
Picnic-Bot
1f83eada44 Upgrade dawidd6/action-download-artifact v3.1.0 -> v3.1.1 (#1039)
See:
- https://github.com/dawidd6/action-download-artifact/releases/tag/v3.1.1
2024-02-19 09:27:12 +01:00
Picnic-Bot
34b57b76bc Upgrade Spring Security 6.2.1 -> 6.2.2 (#1038)
See:
- https://github.com/spring-projects/spring-security/releases/tag/6.2.2
- https://github.com/spring-projects/spring-security/compare/6.2.1...6.2.2
2024-02-19 08:25:18 +01:00
Picnic-Bot
cd3c2aab5d Upgrade Truth 1.4.0 -> 1.4.1 (#1041)
See:
- https://github.com/google/truth/releases/tag/v1.4.1
- https://github.com/google/truth/compare/v1.4.0...v1.4.1
2024-02-19 08:14:31 +01:00
Stephan Schroevers
b39e322a67 Upgrade JDKs used by GitHub Actions builds (#1043)
Summary of changes:
- Use JDK 17.0.10 instead of 17.0.8.
- Use JDK 21.0.2 instead of 21.0.0.
- Have GitHub issue template reference more recent version numbers.

See:
- https://adoptium.net/temurin/release-notes/?version=jdk-17.0.9+9
- https://adoptium.net/temurin/release-notes/?version=jdk-17.0.10+7
- https://adoptium.net/temurin/release-notes/?version=jdk-21.0.1+12
- https://adoptium.net/temurin/release-notes/?version=jdk-21.0.2+13
2024-02-18 16:51:54 +01:00
Picnic-Bot
ad9d2dd534 Upgrade Byte Buddy 1.14.11 -> 1.14.12 (#1040)
See:
- https://github.com/raphw/byte-buddy/releases/tag/byte-buddy-1.14.12
- https://github.com/raphw/byte-buddy/compare/byte-buddy-1.14.11...byte-buddy-1.14.12
2024-02-18 16:32:40 +01:00
Picnic-Bot
d3307645cb Upgrade AspectJ 1.9.21 -> 1.9.21.1 (#1033)
See:
- https://github.com/eclipse-aspectj/aspectj/releases/tag/V1_9_21_1
- https://github.com/eclipse/org.aspectj/compare/V1_9_21...V1_9_21_1
2024-02-17 11:14:19 +01:00
Picnic-Bot
2185a0397a Upgrade NullAway 0.10.22 -> 0.10.23 (#1035)
See:
- https://github.com/uber/NullAway/blob/master/CHANGELOG.md
- https://github.com/uber/NullAway/releases/tag/v0.10.23
- https://github.com/uber/NullAway/compare/v0.10.22...v0.10.23
2024-02-16 10:17:56 +01:00
Picnic-Bot
c4a9f6fab7 Upgrade Spring 6.1.3 -> 6.1.4 (#1036)
See:
- https://github.com/spring-projects/spring-framework/releases/tag/v6.1.4
- https://github.com/spring-projects/spring-framework/compare/v6.1.3...v6.1.4
2024-02-16 09:54:33 +01:00
Picnic-Bot
98b6b7ec0c Upgrade dawidd6/action-download-artifact v3.0.0 -> v3.1.0 (#1037)
See:
- https://github.com/dawidd6/action-download-artifact/releases/tag/v3.1.0
2024-02-16 09:42:32 +01:00
Picnic-Bot
cc6211d560 Upgrade Project Reactor 2023.0.2 -> 2023.0.3 (#1034)
See:
- https://github.com/reactor/reactor/releases/tag/2023.0.3
- https://github.com/reactor/reactor/compare/2023.0.2...2023.0.3
2024-02-16 09:31:15 +01:00
Giovanni Zotta
8855ba33a0 Extend CollectionIsEmpty Refaster rule (#1027) 2024-02-15 08:42:15 +01:00
Picnic-Bot
e87b02cfe3 Upgrade pitest-maven-plugin 1.15.7 -> 1.15.8 (#1032)
See:
- https://github.com/hcoles/pitest/releases/tag/1.15.8
- https://github.com/hcoles/pitest/compare/1.15.7...1.15.8
2024-02-14 09:05:56 +01:00
Picnic-Bot
21a16e8803 Upgrade Spring Boot 2.7.18 -> 3.2.2 (#680)
See:
- https://github.com/spring-projects/spring-boot/releases/tag/v3.0.0-M1
- https://github.com/spring-projects/spring-boot/releases/tag/v3.0.0-M2
- https://github.com/spring-projects/spring-boot/releases/tag/v3.0.0-M3
- https://github.com/spring-projects/spring-boot/releases/tag/v3.0.0-M4
- https://github.com/spring-projects/spring-boot/releases/tag/v3.0.0-M5
- https://github.com/spring-projects/spring-boot/releases/tag/v3.0.0-RC1
- https://github.com/spring-projects/spring-boot/releases/tag/v3.0.0-RC2
- https://github.com/spring-projects/spring-boot/releases/tag/v3.0.0
- https://github.com/spring-projects/spring-boot/releases/tag/v3.0.1
- https://github.com/spring-projects/spring-boot/releases/tag/v3.0.2
- https://github.com/spring-projects/spring-boot/releases/tag/v3.0.3
- https://github.com/spring-projects/spring-boot/releases/tag/v3.0.4
- https://github.com/spring-projects/spring-boot/releases/tag/v3.0.5
- https://github.com/spring-projects/spring-boot/releases/tag/v3.0.6
- https://github.com/spring-projects/spring-boot/releases/tag/v3.0.7
- https://github.com/spring-projects/spring-boot/releases/tag/v3.0.8
- https://github.com/spring-projects/spring-boot/releases/tag/v3.0.9
- https://github.com/spring-projects/spring-boot/releases/tag/v3.0.10
- https://github.com/spring-projects/spring-boot/releases/tag/v3.0.11
- https://github.com/spring-projects/spring-boot/releases/tag/v3.0.12
- https://github.com/spring-projects/spring-boot/releases/tag/v3.0.13
- https://github.com/spring-projects/spring-boot/releases/tag/v3.1.0-M1
- https://github.com/spring-projects/spring-boot/releases/tag/v3.1.0-M2
- https://github.com/spring-projects/spring-boot/releases/tag/v3.1.0-RC1
- https://github.com/spring-projects/spring-boot/releases/tag/v3.1.0-RC2
- https://github.com/spring-projects/spring-boot/releases/tag/v3.1.0
- https://github.com/spring-projects/spring-boot/releases/tag/v3.1.1
- https://github.com/spring-projects/spring-boot/releases/tag/v3.1.2
- https://github.com/spring-projects/spring-boot/releases/tag/v3.1.3
- https://github.com/spring-projects/spring-boot/releases/tag/v3.1.4
- https://github.com/spring-projects/spring-boot/releases/tag/v3.1.5
- https://github.com/spring-projects/spring-boot/releases/tag/v3.1.6
- https://github.com/spring-projects/spring-boot/releases/tag/v3.1.7
- https://github.com/spring-projects/spring-boot/releases/tag/v3.1.8
- https://github.com/spring-projects/spring-boot/releases/tag/v3.2.0-M1
- https://github.com/spring-projects/spring-boot/releases/tag/v3.2.0-M2
- https://github.com/spring-projects/spring-boot/releases/tag/v3.2.0-M3
- https://github.com/spring-projects/spring-boot/releases/tag/v3.2.0-RC1
- https://github.com/spring-projects/spring-boot/releases/tag/v3.2.0-RC2
- https://github.com/spring-projects/spring-boot/releases/tag/v3.2.0
- https://github.com/spring-projects/spring-boot/releases/tag/v3.2.1
- https://github.com/spring-projects/spring-boot/releases/tag/v3.2.2
- https://github.com/spring-projects/spring-boot/compare/v2.7.18...v3.2.2
2024-02-14 08:02:22 +01:00
Picnic-Bot
d3cc77ed93 Upgrade Spring Security 5.8.9 -> 6.2.1 (#1010)
See:
- https://github.com/spring-projects/spring-security/releases/tag/6.0.0-M1
- https://github.com/spring-projects/spring-security/releases/tag/6.0.0-M2
- https://github.com/spring-projects/spring-security/releases/tag/6.0.0-M3
- https://github.com/spring-projects/spring-security/releases/tag/6.0.0-M4
- https://github.com/spring-projects/spring-security/releases/tag/6.0.0-M5
- https://github.com/spring-projects/spring-security/releases/tag/6.0.0-M6
- https://github.com/spring-projects/spring-security/releases/tag/6.0.0-M7
- https://github.com/spring-projects/spring-security/releases/tag/6.0.0-RC1
- https://github.com/spring-projects/spring-security/releases/tag/6.0.0-RC2
- https://github.com/spring-projects/spring-security/releases/tag/6.0.0
- https://github.com/spring-projects/spring-security/releases/tag/6.0.1
- https://github.com/spring-projects/spring-security/releases/tag/6.0.2
- https://github.com/spring-projects/spring-security/releases/tag/6.0.3
- https://github.com/spring-projects/spring-security/releases/tag/6.0.4
- https://github.com/spring-projects/spring-security/releases/tag/6.0.5
- https://github.com/spring-projects/spring-security/releases/tag/6.0.6
- https://github.com/spring-projects/spring-security/releases/tag/6.0.7
- https://github.com/spring-projects/spring-security/releases/tag/6.0.8
- https://github.com/spring-projects/spring-security/releases/tag/6.1.0-M1
- https://github.com/spring-projects/spring-security/releases/tag/6.1.0-M2
- https://github.com/spring-projects/spring-security/releases/tag/6.1.0-RC1
- https://github.com/spring-projects/spring-security/releases/tag/6.1.0
- https://github.com/spring-projects/spring-security/releases/tag/6.1.1
- https://github.com/spring-projects/spring-security/releases/tag/6.1.2
- https://github.com/spring-projects/spring-security/releases/tag/6.1.3
- https://github.com/spring-projects/spring-security/releases/tag/6.1.4
- https://github.com/spring-projects/spring-security/releases/tag/6.1.5
- https://github.com/spring-projects/spring-security/releases/tag/6.1.6
- https://github.com/spring-projects/spring-security/releases/tag/6.2.0-M1
- https://github.com/spring-projects/spring-security/releases/tag/6.2.0-M2
- https://github.com/spring-projects/spring-security/releases/tag/6.2.0-M3
- https://github.com/spring-projects/spring-security/releases/tag/6.2.0-RC1
- https://github.com/spring-projects/spring-security/releases/tag/6.2.0-RC2
- https://github.com/spring-projects/spring-security/releases/tag/6.2.0
- https://github.com/spring-projects/spring-security/releases/tag/6.2.1
- https://github.com/spring-projects/spring-security/compare/5.8.9...6.2.1
2024-02-13 16:14:36 +01:00
Stephan Schroevers
c2365c01c3 Introduce InputStream{Read,Skip}NBytes Refaster rules (#993) 2024-02-13 09:47:18 +01:00
Picnic-Bot
1d0d1d6cae Upgrade Spring 5.3.31 -> 6.1.3 (#679)
See:
- https://github.com/spring-projects/spring-framework/releases/tag/v6.0.0-M1
- https://github.com/spring-projects/spring-framework/releases/tag/v6.0.0-M2
- https://github.com/spring-projects/spring-framework/releases/tag/v6.0.0-M3
- https://github.com/spring-projects/spring-framework/releases/tag/v6.0.0-M4
- https://github.com/spring-projects/spring-framework/releases/tag/v6.0.0-M5
- https://github.com/spring-projects/spring-framework/releases/tag/v6.0.0-M6
- https://github.com/spring-projects/spring-framework/releases/tag/v6.0.0-RC1
- https://github.com/spring-projects/spring-framework/releases/tag/v6.0.0-RC2
- https://github.com/spring-projects/spring-framework/releases/tag/v6.0.0-RC3
- https://github.com/spring-projects/spring-framework/releases/tag/v6.0.0-RC4
- https://github.com/spring-projects/spring-framework/releases/tag/v6.0.0
- https://github.com/spring-projects/spring-framework/releases/tag/v6.0.1
- https://github.com/spring-projects/spring-framework/releases/tag/v6.0.2
- https://github.com/spring-projects/spring-framework/releases/tag/v6.0.3
- https://github.com/spring-projects/spring-framework/releases/tag/v6.0.4
- https://github.com/spring-projects/spring-framework/releases/tag/v6.0.5
- https://github.com/spring-projects/spring-framework/releases/tag/v6.0.6
- https://github.com/spring-projects/spring-framework/releases/tag/v6.0.7
- https://github.com/spring-projects/spring-framework/releases/tag/v6.0.8
- https://github.com/spring-projects/spring-framework/releases/tag/v6.0.9
- https://github.com/spring-projects/spring-framework/releases/tag/v6.0.10
- https://github.com/spring-projects/spring-framework/releases/tag/v6.0.11
- https://github.com/spring-projects/spring-framework/releases/tag/v6.0.12
- https://github.com/spring-projects/spring-framework/releases/tag/v6.0.13
- https://github.com/spring-projects/spring-framework/releases/tag/v6.0.14
- https://github.com/spring-projects/spring-framework/releases/tag/v6.0.15
- https://github.com/spring-projects/spring-framework/releases/tag/v6.0.16
- https://github.com/spring-projects/spring-framework/releases/tag/v6.1.0-M1
- https://github.com/spring-projects/spring-framework/releases/tag/v6.1.0-M2
- https://github.com/spring-projects/spring-framework/releases/tag/v6.1.0-M3
- https://github.com/spring-projects/spring-framework/releases/tag/v6.1.0-M4
- https://github.com/spring-projects/spring-framework/releases/tag/v6.1.0-M5
- https://github.com/spring-projects/spring-framework/releases/tag/v6.1.0-RC1
- https://github.com/spring-projects/spring-framework/releases/tag/v6.1.0-RC2
- https://github.com/spring-projects/spring-framework/releases/tag/v6.1.0
- https://github.com/spring-projects/spring-framework/releases/tag/v6.1.1
- https://github.com/spring-projects/spring-framework/releases/tag/v6.1.2
- https://github.com/spring-projects/spring-framework/releases/tag/v6.1.3
- https://github.com/spring-projects/spring-framework/compare/v5.3.31...v6.1.3
2024-02-13 08:37:56 +01:00
Picnic-Bot
cce897ed4a Upgrade s4u/setup-maven-action v1.11.0 -> v1.12.0 (#1030)
See:
- https://github.com/s4u/setup-maven-action/releases/tag/v1.12.0
2024-02-13 08:08:36 +01:00
Stephan Schroevers
28bb4f6895 Introduce BugPatternLink check (#1026)
This check validates that checks defined by Error Prone Support
reference the public website, unless they explicitly specify that their
diagnostics output must not include a link.
2024-02-12 09:03:25 +01:00
Stephan Schroevers
1fe67677b4 Re-enable SonarCloud analysis on default branch (#1029)
This analysis was accidentally disabled by
ff3be8ae3f.
2024-02-12 08:46:45 +01:00
Stephan Schroevers
433b8b90c0 Require JDK 17 rather than JDK 11 (#603)
By raising this baseline the project can now use Java 17 language features such
as text blocks, switch expressions and `instanceof` pattern matching. The code
has been updated to make use of these constructs.

Note that the project can still be used by builds that target an older version
of Java, as long as those builds are executed using JDK 17+.
2024-02-11 16:57:13 +01:00
Stephan Schroevers
1f50772433 [maven-release-plugin] prepare for next development iteration 2024-02-11 14:31:59 +01:00
Stephan Schroevers
382b79989c [maven-release-plugin] prepare release v0.15.0 2024-02-11 14:31:59 +01:00
Stephan Schroevers
1cc792c615 Introduce ExhaustiveRefasterTypeMigration check (#770)
The new `@TypeMigration` annotation can be placed on Refaster rule
collections to indicate that they migrate most or all public methods of
an indicated type. The new check validates the claim made by the
annotation.
2024-02-11 12:42:39 +01:00
Benura Abeywardena
b5ace6e044 Introduce NewStringFromCharArray{,SubSequence} Refaster rules (#1012)
Resolves #1001.
2024-02-11 12:10:10 +01:00
Giovanni Zotta
1f71ccccf7 Extend StreamIsEmpty Refaster rule (#1025) 2024-02-11 12:01:33 +01:00
Stephan Schroevers
57fa6ae2b8 Introduce package-info.java for tech.picnic.errorprone.guidelines.bugpatterns (#1023) 2024-02-10 18:22:26 +01:00
81 changed files with 1753 additions and 512 deletions

View File

@@ -42,9 +42,9 @@ Please replace this sentence with log output, if applicable.
<!-- Please complete the following information: -->
- Operating system (e.g. MacOS Monterey).
- Java version (i.e. `java --version`, e.g. `17.0.8`).
- Error Prone version (e.g. `2.18.0`).
- Error Prone Support version (e.g. `0.9.0`).
- Java version (i.e. `java --version`, e.g. `17.0.10`).
- Error Prone version (e.g. `2.25.0`).
- Error Prone Support version (e.g. `0.15.0`).
### Additional context

View File

@@ -10,16 +10,16 @@ jobs:
strategy:
matrix:
os: [ ubuntu-22.04 ]
jdk: [ 11.0.20, 17.0.8, 21.0.0 ]
jdk: [ 17.0.10, 21.0.2 ]
distribution: [ temurin ]
experimental: [ false ]
include:
- os: macos-14
jdk: 17.0.8
jdk: 17.0.10
distribution: temurin
experimental: false
- os: windows-2022
jdk: 17.0.8
jdk: 17.0.10
distribution: temurin
experimental: false
runs-on: ${{ matrix.os }}
@@ -31,7 +31,7 @@ jobs:
# additionally enabling all checks defined in this project and any Error
# Prone checks available only from other artifact repositories.
- name: Check out code and set up JDK and Maven
uses: s4u/setup-maven-action@fa2c7e4517ed008b1f73e7e0195a9eecf5582cd4 # v1.11.0
uses: s4u/setup-maven-action@6d44c18d67d9e1549907b8815efa5e4dada1801b # v1.12.0
with:
java-version: ${{ matrix.jdk }}
java-distribution: ${{ matrix.distribution }}

View File

@@ -22,9 +22,9 @@ jobs:
runs-on: ubuntu-22.04
steps:
- name: Check out code and set up JDK and Maven
uses: s4u/setup-maven-action@fa2c7e4517ed008b1f73e7e0195a9eecf5582cd4 # v1.11.0
uses: s4u/setup-maven-action@6d44c18d67d9e1549907b8815efa5e4dada1801b # v1.12.0
with:
java-version: 17.0.8
java-version: 17.0.10
java-distribution: temurin
maven-version: 3.9.6
- name: Initialize CodeQL

View File

@@ -12,10 +12,10 @@ jobs:
runs-on: ubuntu-22.04
steps:
- name: Check out code and set up JDK and Maven
uses: s4u/setup-maven-action@fa2c7e4517ed008b1f73e7e0195a9eecf5582cd4 # v1.11.0
uses: s4u/setup-maven-action@6d44c18d67d9e1549907b8815efa5e4dada1801b # v1.12.0
with:
checkout-fetch-depth: 2
java-version: 17.0.8
java-version: 17.0.10
java-distribution: temurin
maven-version: 3.9.6
- name: Run Pitest

View File

@@ -20,13 +20,13 @@ jobs:
runs-on: ubuntu-22.04
steps:
- name: Check out code and set up JDK and Maven
uses: s4u/setup-maven-action@fa2c7e4517ed008b1f73e7e0195a9eecf5582cd4 # v1.11.0
uses: s4u/setup-maven-action@6d44c18d67d9e1549907b8815efa5e4dada1801b # v1.12.0
with:
java-version: 17.0.8
java-version: 17.0.10
java-distribution: temurin
maven-version: 3.9.6
- name: Download Pitest analysis artifact
uses: dawidd6/action-download-artifact@e7466d1a7587ed14867642c2ca74b5bcc1e19a2d # v3.0.0
uses: dawidd6/action-download-artifact@72aaadce3bc708349fc665eee3785cbb1b6e51d0 # v3.1.1
with:
workflow: ${{ github.event.workflow_run.workflow_id }}
name: pitest-reports

View File

@@ -19,10 +19,10 @@ jobs:
runs-on: ubuntu-22.04
steps:
- name: Check out code and set up JDK and Maven
uses: s4u/setup-maven-action@fa2c7e4517ed008b1f73e7e0195a9eecf5582cd4 # v1.11.0
uses: s4u/setup-maven-action@6d44c18d67d9e1549907b8815efa5e4dada1801b # v1.12.0
with:
checkout-ref: "refs/pull/${{ github.event.issue.number }}/head"
java-version: 17.0.8
java-version: 17.0.10
java-distribution: temurin
maven-version: 3.9.6
- name: Install project to local Maven repository

View File

@@ -13,16 +13,16 @@ jobs:
analyze:
# Analysis of code in forked repositories is skipped, as such workflow runs
# do not have access to the requisite secrets.
if: github.event.pull_request.head.repo.full_name == github.repository
if: github.event_name != 'pull_request' || github.event.pull_request.head.repo.full_name == github.repository
permissions:
contents: read
runs-on: ubuntu-22.04
steps:
- name: Check out code and set up JDK and Maven
uses: s4u/setup-maven-action@fa2c7e4517ed008b1f73e7e0195a9eecf5582cd4 # v1.11.0
uses: s4u/setup-maven-action@6d44c18d67d9e1549907b8815efa5e4dada1801b # v1.12.0
with:
checkout-fetch-depth: 0
java-version: 17.0.8
java-version: 17.0.10
java-distribution: temurin
maven-version: 3.9.6
- name: Create missing `test` directory

View File

@@ -49,7 +49,9 @@ high-quality and consistent Java code_][picnic-blog-ep-post].
### Installation
This library is built on top of [Error Prone][error-prone-orig-repo]. To use
it, read the installation guide for Maven or Gradle below.
it, read the installation guide for Maven or Gradle below. The library requires
that your build is executed using JDK 17 or above, but supports builds that
[target][baeldung-java-source-target-options] older versions of Java.
#### Maven
@@ -263,6 +265,7 @@ guidelines][contributing].
If you want to report a security vulnerability, please do so through a private
channel; please see our [security policy][security] for details.
[baeldung-java-source-target-options]: https://www.baeldung.com/java-source-target-options
[bug-checks]: https://github.com/PicnicSupermarket/error-prone-support/blob/master/error-prone-contrib/src/main/java/tech/picnic/errorprone/bugpatterns/
[bug-checks-identity-conversion]: https://github.com/PicnicSupermarket/error-prone-support/blob/master/error-prone-contrib/src/main/java/tech/picnic/errorprone/bugpatterns/IdentityConversion.java
[codeql-badge]: https://github.com/PicnicSupermarket/error-prone-support/actions/workflows/codeql.yml/badge.svg?branch=master&event=push

View File

@@ -5,7 +5,7 @@
<parent>
<groupId>tech.picnic.error-prone-support</groupId>
<artifactId>error-prone-support</artifactId>
<version>0.14.1-SNAPSHOT</version>
<version>0.15.1-SNAPSHOT</version>
</parent>
<artifactId>documentation-support</artifactId>

View File

@@ -135,8 +135,8 @@ public final class BugPatternTestExtractor implements Extractor<TestCases> {
}
ExpressionTree receiver = ASTHelpers.getReceiver(tree);
return receiver instanceof MethodInvocationTree
? getClassUnderTest((MethodInvocationTree) receiver, state)
return receiver instanceof MethodInvocationTree methodInvocation
? getClassUnderTest(methodInvocation, state)
: Optional.empty();
}
@@ -154,8 +154,8 @@ public final class BugPatternTestExtractor implements Extractor<TestCases> {
}
ExpressionTree receiver = ASTHelpers.getReceiver(tree);
if (receiver instanceof MethodInvocationTree) {
extractIdentificationTestCases((MethodInvocationTree) receiver, sink, state);
if (receiver instanceof MethodInvocationTree methodInvocation) {
extractIdentificationTestCases(methodInvocation, sink, state);
}
}
@@ -184,8 +184,8 @@ public final class BugPatternTestExtractor implements Extractor<TestCases> {
}
ExpressionTree receiver = ASTHelpers.getReceiver(tree);
if (receiver instanceof MethodInvocationTree) {
extractReplacementTestCases((MethodInvocationTree) receiver, sink, state);
if (receiver instanceof MethodInvocationTree methodInvocation) {
extractReplacementTestCases(methodInvocation, sink, state);
}
}

View File

@@ -93,13 +93,20 @@ final class DocumentationGeneratorTaskListenerTest {
"DocumentationGeneratorTaskListenerTestClass.java",
"class DocumentationGeneratorTaskListenerTestClass {}");
// XXX: Once we support only JDK 15+, use a text block for the `expected` string.
assertThat(
outputDirectory.resolve(
"documentation-generator-task-listener-test-DocumentationGeneratorTaskListenerTestClass.json"))
.content(UTF_8)
.isEqualToIgnoringWhitespace(
"{\"className\":\"DocumentationGeneratorTaskListenerTestClass\",\"path\":[\"CLASS: DocumentationGeneratorTaskListenerTestClass\",\"COMPILATION_UNIT\"]}");
"""
{
"className": "DocumentationGeneratorTaskListenerTestClass",
"path": [
"CLASS: DocumentationGeneratorTaskListenerTestClass",
"COMPILATION_UNIT"
]
}
""");
}
@Immutable
@@ -125,8 +132,8 @@ final class DocumentationGeneratorTaskListenerTest {
}
private static String describeTree(Tree tree) {
return (tree instanceof ClassTree)
? String.join(": ", String.valueOf(tree.getKind()), ((ClassTree) tree).getSimpleName())
return (tree instanceof ClassTree clazz)
? String.join(": ", String.valueOf(tree.getKind()), clazz.getSimpleName())
: tree.getKind().toString();
}
}

View File

@@ -5,7 +5,7 @@
<parent>
<groupId>tech.picnic.error-prone-support</groupId>
<artifactId>error-prone-support</artifactId>
<version>0.14.1-SNAPSHOT</version>
<version>0.15.1-SNAPSHOT</version>
</parent>
<artifactId>error-prone-contrib</artifactId>
@@ -279,7 +279,6 @@
</path>
</annotationProcessorPaths>
<compilerArgs combine.children="append">
<arg>-Xplugin:RefasterRuleCompiler</arg>
<arg>-Xplugin:DocumentationGenerator -XoutputDirectory=${project.build.directory}/docs</arg>
</compilerArgs>
</configuration>

View File

@@ -18,7 +18,7 @@ import com.google.errorprone.util.ASTHelpers;
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 final class AmbiguousJsonCreator extends BugChecker implements Annotation
}
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;
}

View File

@@ -19,7 +19,6 @@ import com.sun.source.tree.AnnotationTree;
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 @@ public final class CanonicalAnnotationSyntax extends BugChecker implements Annot
* 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 @@ public final class CanonicalAnnotationSyntax extends BugChecker implements Annot
}
private static Optional<String> 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. */

View File

@@ -81,9 +81,8 @@ public final class CanonicalClassNameUsage extends BugChecker
path = path.getParentPath();
}
return path.getLeaf() instanceof MethodInvocationTree
&& isOwnedByCanonicalNameUsingType(
ASTHelpers.getSymbol((MethodInvocationTree) path.getLeaf()));
return path.getLeaf() instanceof MethodInvocationTree methodInvocation
&& isOwnedByCanonicalNameUsingType(ASTHelpers.getSymbol(methodInvocation));
}
private static boolean isOwnedByCanonicalNameUsingType(MethodSymbol symbol) {

View File

@@ -101,19 +101,17 @@ public final class DirectReturn extends BugChecker implements BlockTreeMatcher {
}
private static Optional<ExpressionTree> 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())
@@ -151,11 +149,11 @@ public final class DirectReturn extends BugChecker implements BlockTreeMatcher {
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));

View File

@@ -50,8 +50,9 @@ import reactor.core.publisher.Flux;
@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,

View File

@@ -245,8 +245,8 @@ public final class FormatStringConcatenation extends BugChecker
}
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);

View File

@@ -124,8 +124,9 @@ public final class IdentityConversion extends BugChecker implements MethodInvoca
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();

View File

@@ -43,8 +43,9 @@ import javax.lang.model.element.Modifier;
@AutoService(BugChecker.class)
@BugPattern(
summary =
"`SortedSet` properties of a `@Value.Immutable` or `@Value.Modifiable` type must be "
+ "annotated with `@Value.NaturalOrder` or `@Value.ReverseOrder`",
"""
`SortedSet` properties of a `@Value.Immutable` or `@Value.Modifiable` type must be \
annotated with `@Value.NaturalOrder` or `@Value.ReverseOrder`""",
link = BUG_PATTERNS_BASE_URL + "ImmutablesSortedSetComparator",
linkType = CUSTOM,
severity = ERROR,

View File

@@ -16,7 +16,6 @@ import com.google.errorprone.matchers.Description;
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.utils.SourceCode;
@@ -41,12 +40,12 @@ public final class IsInstanceLambdaUsage extends BugChecker implements LambdaExp
@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;
}

View File

@@ -265,8 +265,8 @@ public final class JUnitValueSource extends BugChecker implements MethodTreeMatc
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(", ")))
@@ -276,16 +276,12 @@ public final class JUnitValueSource extends BugChecker implements MethodTreeMatc
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 <T> Optional<T> getElementIfSingleton(Collection<T> collection) {
@@ -297,11 +293,10 @@ public final class JUnitValueSource extends BugChecker implements MethodTreeMatc
private static Matcher<ExpressionTree> isSingleDimensionArrayCreationWithAllElementsMatching(
Matcher<? super ExpressionTree> 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()

View File

@@ -31,7 +31,6 @@ import com.sun.source.tree.LiteralTree;
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;
@@ -52,6 +51,9 @@ import tech.picnic.errorprone.utils.SourceCode;
* <p>The idea behind this checker is that maintaining a sorted sequence simplifies conflict
* resolution, and can even avoid it if two branches add the same entry.
*/
// XXX: In some places we declare a `@SuppressWarnings` annotation with a final value of
// `key-to-resolve-AnnotationUseStyle-and-TrailingComment-check-conflict`. That entry must stay
// last. Consider adding (generic?) support for such cases.
@AutoService(BugChecker.class)
@BugPattern(
summary = "Where possible, sort annotation array attributes lexicographically",
@@ -122,13 +124,9 @@ public final class LexicographicalAnnotationAttributeListing extends BugChecker
}
private static Optional<NewArrayTree> 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<SuggestedFix.Builder> suggestSorting(
@@ -200,8 +198,8 @@ public final class LexicographicalAnnotationAttributeListing extends BugChecker
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);

View File

@@ -36,6 +36,10 @@ import tech.picnic.errorprone.utils.SourceCode;
* <p>The idea behind this checker is that maintaining a sorted sequence simplifies conflict
* resolution, and can even avoid it if two branches add the same annotation.
*/
// XXX: Currently this checker only flags method-level annotations. It should likely also flag
// type-, field- and parameter-level annotations.
// XXX: Duplicate entries are often a mistake. Consider introducing a similar `BugChecker` that
// flags duplicates.
@AutoService(BugChecker.class)
@BugPattern(
summary = "Sort annotations lexicographically where possible",

View File

@@ -67,20 +67,19 @@ public final class MockitoMockClassReference extends BugChecker
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();
switch (parent.getKind()) {
case VARIABLE:
return !ASTHelpers.hasImplicitType((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.hasImplicitType((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;
};
}
}

View File

@@ -24,7 +24,9 @@ import com.sun.source.tree.MethodInvocationTree;
@AutoService(BugChecker.class)
@BugPattern(
summary =
"Avoid MongoDB's `$text` filter operator, as it can trigger heavy queries and even cause the server to run out of memory",
"""
Avoid MongoDB's `$text` filter operator, as it can trigger heavy queries and even cause \
the server to run out of memory""",
link = BUG_PATTERNS_BASE_URL + "MongoDBTextFilterUsage",
linkType = CUSTOM,
severity = SUGGESTION,

View File

@@ -34,8 +34,9 @@ import com.sun.tools.javac.code.Type;
@AutoService(BugChecker.class)
@BugPattern(
summary =
"Avoid `Publisher`s that emit other `Publishers`s; "
+ "the resultant code is hard to reason about",
"""
Avoid `Publisher`s that emit other `Publishers`s; the resultant code is hard to reason \
about""",
link = BUG_PATTERNS_BASE_URL + "NestedPublishers",
linkType = CUSTOM,
severity = WARNING,

View File

@@ -169,10 +169,9 @@ public final class NonStaticImport extends BugChecker implements CompilationUnit
ImmutableTable.builder();
for (ImportTree importTree : tree.getImports()) {
Tree qualifiedIdentifier = importTree.getQualifiedIdentifier();
if (importTree.isStatic() && qualifiedIdentifier instanceof MemberSelectTree) {
MemberSelectTree memberSelectTree = (MemberSelectTree) qualifiedIdentifier;
String type = SourceCode.treeToString(memberSelectTree.getExpression(), state);
String member = memberSelectTree.getIdentifier().toString();
if (importTree.isStatic() && qualifiedIdentifier instanceof MemberSelectTree memberSelect) {
String type = SourceCode.treeToString(memberSelect.getExpression(), state);
String member = memberSelect.getIdentifier().toString();
if (shouldNotBeStaticallyImported(type, member)) {
imports.put(
type,

View File

@@ -22,6 +22,7 @@ import com.google.errorprone.matchers.Description;
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;
@@ -44,8 +45,9 @@ import tech.picnic.errorprone.utils.SourceCode;
@AutoService(BugChecker.class)
@BugPattern(
summary =
"Ensure invocations of `Comparator#comparing{,Double,Int,Long}` match the return type"
+ " of the provided function",
"""
Ensure invocations of `Comparator#comparing{,Double,Int,Long}` match the return type of \
the provided function""",
link = BUG_PATTERNS_BASE_URL + "PrimitiveComparison",
linkType = CUSTOM,
severity = WARNING,
@@ -147,38 +149,44 @@ public final class PrimitiveComparison extends BugChecker implements MethodInvoc
return isStatic ? "comparing" : "thenComparing";
}
// XXX: Use switch pattern matching once the targeted JDK supports this.
private static Optional<Type> getPotentiallyBoxedReturnType(ExpressionTree tree) {
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:
/* Return the method's declared return type. */
// XXX: Very fragile. Do better.
Type subType2 = ((JCMemberReference) tree).referentType;
return Optional.of(subType2.getReturnType());
default:
/* This appears to be a genuine `{,ToInt,ToLong,ToDouble}Function`. */
return 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();
}
// XXX: Use switch pattern matching once the targeted JDK supports this.
private static Fix suggestFix(
MethodInvocationTree tree, String preferredMethodName, VisitorState state) {
ExpressionTree expr = tree.getMethodSelect();
switch (expr.getKind()) {
case IDENTIFIER:
SuggestedFix.Builder fix = SuggestedFix.builder();
String replacement =
SuggestedFixes.qualifyStaticImport(
Comparator.class.getCanonicalName() + '.' + preferredMethodName, fix, state);
return fix.replace(expr, replacement).build();
case MEMBER_SELECT:
MemberSelectTree ms = (MemberSelectTree) tree.getMethodSelect();
return SuggestedFix.replace(
ms, SourceCode.treeToString(ms.getExpression(), state) + '.' + preferredMethodName);
default:
throw new VerifyException("Unexpected type of expression: " + expr.getKind());
if (expr instanceof IdentifierTree) {
SuggestedFix.Builder fix = SuggestedFix.builder();
String replacement =
SuggestedFixes.qualifyStaticImport(
Comparator.class.getCanonicalName() + '.' + preferredMethodName, fix, state);
return fix.replace(expr, replacement).build();
}
if (expr instanceof MemberSelectTree memberSelect) {
return SuggestedFix.replace(
memberSelect,
SourceCode.treeToString(memberSelect.getExpression(), state) + '.' + preferredMethodName);
}
throw new VerifyException("Unexpected type of expression: " + expr.getKind());
}
}

View File

@@ -331,36 +331,32 @@ public final class RedundantStringConversion extends BugChecker
}
private Optional<ExpressionTree> 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();
}
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<ExpressionTree> 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)));
}

View File

@@ -105,9 +105,10 @@ public final class RequestMappingAnnotation extends BugChecker implements Method
&& 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;
}

View File

@@ -34,7 +34,9 @@ import tech.picnic.errorprone.utils.Flags;
@AutoService(BugChecker.class)
@BugPattern(
summary =
"By default, `@RequestParam` does not support `ImmutableCollection` and `ImmutableMap` subtypes",
"""
By default, `@RequestParam` does not support `ImmutableCollection` and `ImmutableMap` \
subtypes""",
link = BUG_PATTERNS_BASE_URL + "RequestParamType",
linkType = CUSTOM,
severity = ERROR,

View File

@@ -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;
@@ -25,7 +24,6 @@ import com.sun.source.tree.AssignmentTree;
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.utils.AnnotationAttributeMatcher;
import tech.picnic.errorprone.utils.SourceCode;
@@ -80,31 +78,25 @@ public final class SpringMvcAnnotation extends BugChecker implements AnnotationT
}
private static Optional<String> 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) {
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(

View File

@@ -44,6 +44,7 @@ import com.google.errorprone.util.ASTHelpers;
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.nio.charset.StandardCharsets;
import java.time.ZoneOffset;
@@ -209,15 +210,10 @@ public final class StaticImport extends BugChecker implements MemberSelectTreeMa
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 parentTree instanceof MethodInvocationTree methodInvocation
? methodInvocation.getTypeArguments().isEmpty()
: (parentTree.getKind() != Kind.IMPORT && parentTree.getKind() != Kind.MEMBER_SELECT);
}
private static boolean isCandidate(MemberSelectTree tree) {

View File

@@ -34,7 +34,9 @@ import java.time.ZonedDateTime;
@AutoService(BugChecker.class)
@BugPattern(
summary =
"Derive the current time from an existing `Clock` Spring bean, and don't rely on a `Clock`'s time zone",
"""
Derive the current time from an existing `Clock` Spring bean, and don't rely on a \
`Clock`'s time zone""",
link = BUG_PATTERNS_BASE_URL + "TimeZoneUsage",
linkType = CUSTOM,
severity = WARNING,

View File

@@ -23,6 +23,7 @@ import java.util.HashSet;
import java.util.Iterator;
import java.util.Objects;
import java.util.Set;
import java.util.function.Supplier;
import java.util.stream.Stream;
import org.jspecify.annotations.Nullable;
import tech.picnic.errorprone.refaster.annotation.OnlineDocumentation;
@@ -35,6 +36,18 @@ import tech.picnic.errorprone.refaster.annotation.OnlineDocumentation;
final class AssortedRules {
private AssortedRules() {}
final class IllegalStateExceptionSupplier {
@BeforeTemplate
Supplier<IllegalStateException> before(String message) {
return () -> new IllegalStateException(message);
}
@AfterTemplate
Supplier<IllegalStateException> after(String message) {
return () -> new IllegalStateException("XXX");
}
}
/** Prefer {@link Objects#checkIndex(int, int)} over the Guava alternative. */
static final class CheckIndex {
@BeforeTemplate

View File

@@ -35,13 +35,21 @@ final class CollectionRules {
*/
static final class CollectionIsEmpty<T> {
@BeforeTemplate
@SuppressWarnings("java:S1155" /* This violation will be rewritten. */)
@SuppressWarnings({
"java:S1155" /* This violation will be rewritten. */,
"LexicographicalAnnotationAttributeListing" /* `key-*` entry must remain last. */,
"OptionalFirstCollectionElement" /* This is a more specific template. */,
"StreamIsEmpty" /* This is a more specific template. */,
"key-to-resolve-AnnotationUseStyle-and-TrailingComment-check-conflict"
})
boolean before(Collection<T> collection) {
return Refaster.anyOf(
collection.size() == 0,
collection.size() <= 0,
collection.size() < 1,
Iterables.isEmpty(collection));
Iterables.isEmpty(collection),
collection.stream().findAny().isEmpty(),
collection.stream().findFirst().isEmpty());
}
@BeforeTemplate
@@ -337,7 +345,9 @@ final class CollectionRules {
/**
* Don't use the ternary operator to extract the first element of a possibly-empty {@link
* Collection} as an {@link Optional}.
* Collection} as an {@link Optional}, and (when applicable) prefer {@link Stream#findFirst()}
* over {@link Stream#findAny()} to communicate that the collection's first element (if any,
* according to iteration order) will be returned.
*/
static final class OptionalFirstCollectionElement<T> {
@BeforeTemplate

View File

@@ -9,8 +9,6 @@ import java.io.OutputStream;
import tech.picnic.errorprone.refaster.annotation.OnlineDocumentation;
/** Refaster rules related to expressions dealing with {@link InputStream}s. */
// XXX: Add a rule for `ByteStreams.skipFully(in, n)` -> `in.skipNBytes(n)` once we have a way to
// target JDK 12+ APIs.
@OnlineDocumentation
final class InputStreamRules {
private InputStreamRules() {}
@@ -38,4 +36,28 @@ final class InputStreamRules {
return in.readAllBytes();
}
}
static final class InputStreamReadNBytes {
@BeforeTemplate
byte[] before(InputStream in, int n) throws IOException {
return ByteStreams.limit(in, n).readAllBytes();
}
@AfterTemplate
byte[] after(InputStream in, int n) throws IOException {
return in.readNBytes(n);
}
}
static final class InputStreamSkipNBytes {
@BeforeTemplate
void before(InputStream in, long n) throws IOException {
ByteStreams.skipFully(in, n);
}
@AfterTemplate
void after(InputStream in, long n) throws IOException {
in.skipNBytes(n);
}
}
}

View File

@@ -26,6 +26,7 @@ import org.junit.jupiter.api.Assertions;
import org.junit.jupiter.api.function.Executable;
import org.junit.jupiter.api.function.ThrowingSupplier;
import tech.picnic.errorprone.refaster.annotation.OnlineDocumentation;
import tech.picnic.errorprone.refaster.annotation.TypeMigration;
/**
* Refaster rules to replace JUnit assertions with AssertJ equivalents.
@@ -40,6 +41,264 @@ import tech.picnic.errorprone.refaster.annotation.OnlineDocumentation;
// `() -> toString()` match both `ThrowingSupplier` and `ThrowingCallable`, but `() -> "constant"`
// is only compatible with the former.
@OnlineDocumentation
@TypeMigration(
of = Assertions.class,
unmigratedMethods = {
"assertAll(Collection<Executable>)",
"assertAll(Executable[])",
"assertAll(Stream<Executable>)",
"assertAll(String, Collection<Executable>)",
"assertAll(String, Executable[])",
"assertAll(String, Stream<Executable>)",
"assertArrayEquals(boolean[], boolean[])",
"assertArrayEquals(boolean[], boolean[], String)",
"assertArrayEquals(boolean[], boolean[], Supplier<String>)",
"assertArrayEquals(byte[], byte[])",
"assertArrayEquals(byte[], byte[], String)",
"assertArrayEquals(byte[], byte[], Supplier<String>)",
"assertArrayEquals(char[], char[])",
"assertArrayEquals(char[], char[], String)",
"assertArrayEquals(char[], char[], Supplier<String>)",
"assertArrayEquals(double[], double[])",
"assertArrayEquals(double[], double[], double)",
"assertArrayEquals(double[], double[], double, String)",
"assertArrayEquals(double[], double[], double, Supplier<String>)",
"assertArrayEquals(double[], double[], String)",
"assertArrayEquals(double[], double[], Supplier<String>)",
"assertArrayEquals(float[], float[])",
"assertArrayEquals(float[], float[], float)",
"assertArrayEquals(float[], float[], float, String)",
"assertArrayEquals(float[], float[], float, Supplier<String>)",
"assertArrayEquals(float[], float[], String)",
"assertArrayEquals(float[], float[], Supplier<String>)",
"assertArrayEquals(int[], int[])",
"assertArrayEquals(int[], int[], String)",
"assertArrayEquals(int[], int[], Supplier<String>)",
"assertArrayEquals(long[], long[])",
"assertArrayEquals(long[], long[], String)",
"assertArrayEquals(long[], long[], Supplier<String>)",
"assertArrayEquals(Object[], Object[])",
"assertArrayEquals(Object[], Object[], String)",
"assertArrayEquals(Object[], Object[], Supplier<String>)",
"assertArrayEquals(short[], short[])",
"assertArrayEquals(short[], short[], String)",
"assertArrayEquals(short[], short[], Supplier<String>)",
"assertEquals(Byte, Byte)",
"assertEquals(Byte, byte)",
"assertEquals(byte, Byte)",
"assertEquals(byte, byte)",
"assertEquals(Byte, Byte, String)",
"assertEquals(Byte, byte, String)",
"assertEquals(byte, Byte, String)",
"assertEquals(byte, byte, String)",
"assertEquals(Byte, Byte, Supplier<String>)",
"assertEquals(Byte, byte, Supplier<String>)",
"assertEquals(byte, Byte, Supplier<String>)",
"assertEquals(byte, byte, Supplier<String>)",
"assertEquals(char, char)",
"assertEquals(char, char, String)",
"assertEquals(char, char, Supplier<String>)",
"assertEquals(char, Character)",
"assertEquals(char, Character, String)",
"assertEquals(char, Character, Supplier<String>)",
"assertEquals(Character, char)",
"assertEquals(Character, char, String)",
"assertEquals(Character, char, Supplier<String>)",
"assertEquals(Character, Character)",
"assertEquals(Character, Character, String)",
"assertEquals(Character, Character, Supplier<String>)",
"assertEquals(Double, Double)",
"assertEquals(Double, double)",
"assertEquals(double, Double)",
"assertEquals(double, double)",
"assertEquals(double, double, double)",
"assertEquals(double, double, double, String)",
"assertEquals(double, double, double, Supplier<String>)",
"assertEquals(Double, Double, String)",
"assertEquals(Double, double, String)",
"assertEquals(double, Double, String)",
"assertEquals(double, double, String)",
"assertEquals(Double, Double, Supplier<String>)",
"assertEquals(Double, double, Supplier<String>)",
"assertEquals(double, Double, Supplier<String>)",
"assertEquals(double, double, Supplier<String>)",
"assertEquals(Float, Float)",
"assertEquals(Float, float)",
"assertEquals(float, Float)",
"assertEquals(float, float)",
"assertEquals(float, float, float)",
"assertEquals(float, float, float, String)",
"assertEquals(float, float, float, Supplier<String>)",
"assertEquals(Float, Float, String)",
"assertEquals(Float, float, String)",
"assertEquals(float, Float, String)",
"assertEquals(float, float, String)",
"assertEquals(Float, Float, Supplier<String>)",
"assertEquals(Float, float, Supplier<String>)",
"assertEquals(float, Float, Supplier<String>)",
"assertEquals(float, float, Supplier<String>)",
"assertEquals(int, int)",
"assertEquals(int, int, String)",
"assertEquals(int, int, Supplier<String>)",
"assertEquals(int, Integer)",
"assertEquals(int, Integer, String)",
"assertEquals(int, Integer, Supplier<String>)",
"assertEquals(Integer, int)",
"assertEquals(Integer, int, String)",
"assertEquals(Integer, int, Supplier<String>)",
"assertEquals(Integer, Integer)",
"assertEquals(Integer, Integer, String)",
"assertEquals(Integer, Integer, Supplier<String>)",
"assertEquals(Long, Long)",
"assertEquals(Long, long)",
"assertEquals(long, Long)",
"assertEquals(long, long)",
"assertEquals(Long, Long, String)",
"assertEquals(Long, long, String)",
"assertEquals(long, Long, String)",
"assertEquals(long, long, String)",
"assertEquals(Long, Long, Supplier<String>)",
"assertEquals(Long, long, Supplier<String>)",
"assertEquals(long, Long, Supplier<String>)",
"assertEquals(long, long, Supplier<String>)",
"assertEquals(Object, Object)",
"assertEquals(Object, Object, String)",
"assertEquals(Object, Object, Supplier<String>)",
"assertEquals(Short, Short)",
"assertEquals(Short, short)",
"assertEquals(short, Short)",
"assertEquals(short, short)",
"assertEquals(Short, Short, String)",
"assertEquals(Short, short, String)",
"assertEquals(short, Short, String)",
"assertEquals(short, short, String)",
"assertEquals(Short, Short, Supplier<String>)",
"assertEquals(Short, short, Supplier<String>)",
"assertEquals(short, Short, Supplier<String>)",
"assertEquals(short, short, Supplier<String>)",
"assertFalse(BooleanSupplier)",
"assertFalse(BooleanSupplier, String)",
"assertFalse(BooleanSupplier, Supplier<String>)",
"assertIterableEquals(Iterable<?>, Iterable<?>)",
"assertIterableEquals(Iterable<?>, Iterable<?>, String)",
"assertIterableEquals(Iterable<?>, Iterable<?>, Supplier<String>)",
"assertLinesMatch(List<String>, List<String>)",
"assertLinesMatch(List<String>, List<String>, String)",
"assertLinesMatch(List<String>, List<String>, Supplier<String>)",
"assertLinesMatch(Stream<String>, Stream<String>)",
"assertLinesMatch(Stream<String>, Stream<String>, String)",
"assertLinesMatch(Stream<String>, Stream<String>, Supplier<String>)",
"assertNotEquals(Byte, Byte)",
"assertNotEquals(Byte, byte)",
"assertNotEquals(byte, Byte)",
"assertNotEquals(byte, byte)",
"assertNotEquals(Byte, Byte, String)",
"assertNotEquals(Byte, byte, String)",
"assertNotEquals(byte, Byte, String)",
"assertNotEquals(byte, byte, String)",
"assertNotEquals(Byte, Byte, Supplier<String>)",
"assertNotEquals(Byte, byte, Supplier<String>)",
"assertNotEquals(byte, Byte, Supplier<String>)",
"assertNotEquals(byte, byte, Supplier<String>)",
"assertNotEquals(char, char)",
"assertNotEquals(char, char, String)",
"assertNotEquals(char, char, Supplier<String>)",
"assertNotEquals(char, Character)",
"assertNotEquals(char, Character, String)",
"assertNotEquals(char, Character, Supplier<String>)",
"assertNotEquals(Character, char)",
"assertNotEquals(Character, char, String)",
"assertNotEquals(Character, char, Supplier<String>)",
"assertNotEquals(Character, Character)",
"assertNotEquals(Character, Character, String)",
"assertNotEquals(Character, Character, Supplier<String>)",
"assertNotEquals(Double, Double)",
"assertNotEquals(Double, double)",
"assertNotEquals(double, Double)",
"assertNotEquals(double, double)",
"assertNotEquals(double, double, double)",
"assertNotEquals(double, double, double, String)",
"assertNotEquals(double, double, double, Supplier<String>)",
"assertNotEquals(Double, Double, String)",
"assertNotEquals(Double, double, String)",
"assertNotEquals(double, Double, String)",
"assertNotEquals(double, double, String)",
"assertNotEquals(Double, Double, Supplier<String>)",
"assertNotEquals(Double, double, Supplier<String>)",
"assertNotEquals(double, Double, Supplier<String>)",
"assertNotEquals(double, double, Supplier<String>)",
"assertNotEquals(Float, Float)",
"assertNotEquals(Float, float)",
"assertNotEquals(float, Float)",
"assertNotEquals(float, float)",
"assertNotEquals(float, float, float)",
"assertNotEquals(float, float, float, String)",
"assertNotEquals(float, float, float, Supplier<String>)",
"assertNotEquals(Float, Float, String)",
"assertNotEquals(Float, float, String)",
"assertNotEquals(float, Float, String)",
"assertNotEquals(float, float, String)",
"assertNotEquals(Float, Float, Supplier<String>)",
"assertNotEquals(Float, float, Supplier<String>)",
"assertNotEquals(float, Float, Supplier<String>)",
"assertNotEquals(float, float, Supplier<String>)",
"assertNotEquals(int, int)",
"assertNotEquals(int, int, String)",
"assertNotEquals(int, int, Supplier<String>)",
"assertNotEquals(int, Integer)",
"assertNotEquals(int, Integer, String)",
"assertNotEquals(int, Integer, Supplier<String>)",
"assertNotEquals(Integer, int)",
"assertNotEquals(Integer, int, String)",
"assertNotEquals(Integer, int, Supplier<String>)",
"assertNotEquals(Integer, Integer)",
"assertNotEquals(Integer, Integer, String)",
"assertNotEquals(Integer, Integer, Supplier<String>)",
"assertNotEquals(Long, Long)",
"assertNotEquals(Long, long)",
"assertNotEquals(long, Long)",
"assertNotEquals(long, long)",
"assertNotEquals(Long, Long, String)",
"assertNotEquals(Long, long, String)",
"assertNotEquals(long, Long, String)",
"assertNotEquals(long, long, String)",
"assertNotEquals(Long, Long, Supplier<String>)",
"assertNotEquals(Long, long, Supplier<String>)",
"assertNotEquals(long, Long, Supplier<String>)",
"assertNotEquals(long, long, Supplier<String>)",
"assertNotEquals(Object, Object)",
"assertNotEquals(Object, Object, String)",
"assertNotEquals(Object, Object, Supplier<String>)",
"assertNotEquals(Short, Short)",
"assertNotEquals(Short, short)",
"assertNotEquals(short, Short)",
"assertNotEquals(short, short)",
"assertNotEquals(Short, Short, String)",
"assertNotEquals(Short, short, String)",
"assertNotEquals(short, Short, String)",
"assertNotEquals(short, short, String)",
"assertNotEquals(Short, Short, Supplier<String>)",
"assertNotEquals(Short, short, Supplier<String>)",
"assertNotEquals(short, Short, Supplier<String>)",
"assertNotEquals(short, short, Supplier<String>)",
"assertTimeout(Duration, Executable)",
"assertTimeout(Duration, Executable, String)",
"assertTimeout(Duration, Executable, Supplier<String>)",
"assertTimeout(Duration, ThrowingSupplier<T>)",
"assertTimeout(Duration, ThrowingSupplier<T>, String)",
"assertTimeout(Duration, ThrowingSupplier<T>, Supplier<String>)",
"assertTimeoutPreemptively(Duration, Executable)",
"assertTimeoutPreemptively(Duration, Executable, String)",
"assertTimeoutPreemptively(Duration, Executable, Supplier<String>)",
"assertTimeoutPreemptively(Duration, ThrowingSupplier<T>)",
"assertTimeoutPreemptively(Duration, ThrowingSupplier<T>, String)",
"assertTimeoutPreemptively(Duration, ThrowingSupplier<T>, Supplier<String>)",
"assertTimeoutPreemptively(Duration, ThrowingSupplier<T>, Supplier<String>, TimeoutFailureFactory<E>)",
"assertTrue(BooleanSupplier)",
"assertTrue(BooleanSupplier, String)",
"assertTrue(BooleanSupplier, Supplier<String>)",
"fail(Supplier<String>)"
})
final class JUnitToAssertJRules {
private JUnitToAssertJRules() {}

View File

@@ -4,6 +4,7 @@ import static com.google.errorprone.refaster.ImportPolicy.STATIC_IMPORT_ALWAYS;
import static java.util.Comparator.naturalOrder;
import static java.util.Comparator.reverseOrder;
import static java.util.function.Predicate.not;
import static java.util.stream.Collectors.collectingAndThen;
import static java.util.stream.Collectors.counting;
import static java.util.stream.Collectors.filtering;
import static java.util.stream.Collectors.flatMapping;
@@ -37,6 +38,7 @@ import java.util.Comparator;
import java.util.DoubleSummaryStatistics;
import java.util.IntSummaryStatistics;
import java.util.LongSummaryStatistics;
import java.util.Map;
import java.util.Objects;
import java.util.Optional;
import java.util.function.BinaryOperator;
@@ -254,15 +256,21 @@ final class StreamRules {
// XXX: This rule assumes that any matched `Collector` does not perform any filtering.
// (Perhaps we could add a `@Matches` guard that validates that the collector expression does not
// contain a `Collectors#filtering` call. That'd still not be 100% accurate, though.)
static final class StreamIsEmpty<T> {
static final class StreamIsEmpty<T, K, V, C extends Collection<K>, M extends Map<K, V>> {
@BeforeTemplate
boolean before(Stream<T> stream, Collector<? super T, ?, ? extends Collection<?>> collector) {
boolean before(Stream<T> stream, Collector<? super T, ?, ? extends C> collector) {
return Refaster.anyOf(
stream.count() == 0,
stream.count() <= 0,
stream.count() < 1,
stream.findFirst().isEmpty(),
stream.collect(collector).isEmpty());
stream.collect(collector).isEmpty(),
stream.collect(collectingAndThen(collector, C::isEmpty)));
}
@BeforeTemplate
boolean before2(Stream<T> stream, Collector<? super T, ?, ? extends M> collector) {
return stream.collect(collectingAndThen(collector, M::isEmpty));
}
@AfterTemplate

View File

@@ -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 @@ final class StringRules {
}
/** 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. However, `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 @@ final class StringRules {
}
/** 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. However, `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<String> before() {
@@ -162,6 +168,39 @@ final class StringRules {
}
}
/**
* Prefer direct invocation of {@link String#String(char[], int, int)} over the indirection
* introduced by alternatives.
*/
static final class NewStringFromCharArraySubSequence {
@BeforeTemplate
String before(char[] data, int offset, int count) {
return Refaster.anyOf(
String.valueOf(data, offset, count), String.copyValueOf(data, offset, count));
}
@AfterTemplate
String after(char[] data, int offset, int count) {
return new String(data, offset, count);
}
}
/**
* Prefer direct invocation of {@link String#String(char[])} over the indirection introduced by
* alternatives.
*/
static final class NewStringFromCharArray {
@BeforeTemplate
String before(char[] data) {
return Refaster.anyOf(String.valueOf(data), new String(data, 0, data.length));
}
@AfterTemplate
String after(char[] data) {
return new String(data);
}
}
/**
* Prefer direct delegation to {@link String#valueOf(Object)} over the indirection introduced by
* {@link Objects#toString(Object)}.

View File

@@ -29,6 +29,7 @@ import org.assertj.core.api.ThrowableAssert.ThrowingCallable;
import org.testng.Assert;
import org.testng.Assert.ThrowingRunnable;
import tech.picnic.errorprone.refaster.annotation.OnlineDocumentation;
import tech.picnic.errorprone.refaster.annotation.TypeMigration;
/**
* Refaster rules that replace TestNG assertions with equivalent AssertJ assertions.
@@ -48,32 +49,107 @@ import tech.picnic.errorprone.refaster.annotation.OnlineDocumentation;
* List<Map<String, Object>> myMaps = new ArrayList<>();
* assertEquals(myMaps, ImmutableList.of(ImmutableMap.of()));
* }</pre>
*
* <p>A few {@link Assert} methods are not rewritten:
*
* <ul>
* <li>These methods cannot (easily) be expressed using AssertJ because they mix regular equality
* and array equality:
* <ul>
* <li>{@link Assert#assertEqualsDeep(Map, Map)}
* <li>{@link Assert#assertEqualsDeep(Map, Map, String)}
* <li>{@link Assert#assertEqualsDeep(Set, Set, String)}
* <li>{@link Assert#assertNotEqualsDeep(Map, Map)}
* <li>{@link Assert#assertNotEqualsDeep(Map, Map, String)}
* <li>{@link Assert#assertNotEqualsDeep(Set, Set)}
* <li>{@link Assert#assertNotEqualsDeep(Set, Set, String)}
* </ul>
* <li>This method returns the caught exception; there is no direct counterpart for this in
* AssertJ:
* <ul>
* <li>{@link Assert#expectThrows(Class, ThrowingRunnable)}
* </ul>
* </ul>
*/
// XXX: As-is these rules do not result in a complete migration:
// - Expressions containing comments are skipped due to a limitation of Refaster.
// - Assertions inside lambda expressions are also skipped. Unclear why.
// XXX: The `assertEquals` tests for this class generally use the same expression for `expected` and
// `actual`, which makes the validation weaker than necessary; fix this. (And investigate whether we
// can introduce validation for this.)
@OnlineDocumentation
@TypeMigration(
of = Assert.class,
unmigratedMethods = {
// XXX: Add migrations for the methods below.
"assertEquals(Boolean, Boolean)",
"assertEquals(Boolean, boolean)",
"assertEquals(boolean, Boolean)",
"assertEquals(Boolean, Boolean, String)",
"assertEquals(Boolean, boolean, String)",
"assertEquals(boolean, Boolean, String)",
"assertEquals(Byte, Byte)",
"assertEquals(Byte, byte)",
"assertEquals(byte, Byte)",
"assertEquals(Byte, Byte, String)",
"assertEquals(Byte, byte, String)",
"assertEquals(byte, Byte, String)",
"assertEquals(char, Character)",
"assertEquals(char, Character, String)",
"assertEquals(Character, char)",
"assertEquals(Character, char, String)",
"assertEquals(Character, Character)",
"assertEquals(Character, Character, String)",
"assertEquals(Double, Double)",
"assertEquals(Double, double)",
"assertEquals(double, Double)",
"assertEquals(Double, Double, String)",
"assertEquals(Double, double, String)",
"assertEquals(double, Double, String)",
"assertEquals(double[], double[], double)",
"assertEquals(double[], double[], double, String)",
"assertEquals(Float, Float)",
"assertEquals(Float, float)",
"assertEquals(float, Float)",
"assertEquals(Float, Float, String)",
"assertEquals(Float, float, String)",
"assertEquals(float, Float, String)",
"assertEquals(float[], float[], float)",
"assertEquals(float[], float[], float, String)",
"assertEquals(int, Integer)",
"assertEquals(int, Integer, String)",
"assertEquals(Integer, int)",
"assertEquals(Integer, int, String)",
"assertEquals(Integer, Integer)",
"assertEquals(Integer, Integer, String)",
"assertEquals(Long, Long)",
"assertEquals(Long, long)",
"assertEquals(long, Long)",
"assertEquals(Long, Long, String)",
"assertEquals(Long, long, String)",
"assertEquals(Short, Short)",
"assertEquals(Short, short)",
"assertEquals(short, Short)",
"assertEquals(Short, Short, String)",
"assertEquals(Short, short, String)",
"assertEquals(short, Short, String)",
/*
* These `assertEqualsDeep` methods cannot (easily) be expressed using AssertJ because they
* mix regular equality and array equality:
*/
"assertEqualsDeep(Map<?, ?>, Map<?, ?>)",
"assertEqualsDeep(Map<?, ?>, Map<?, ?>, String)",
"assertEqualsDeep(Set<?>, Set<?>, String)",
// XXX: Add migrations for the methods below.
"assertEqualsNoOrder(Collection<?>, Collection<?>)",
"assertEqualsNoOrder(Collection<?>, Collection<?>, String)",
"assertEqualsNoOrder(Iterator<?>, Iterator<?>)",
"assertEqualsNoOrder(Iterator<?>, Iterator<?>, String)",
"assertListContains(List<T>, Predicate<T>, String)",
"assertListContainsObject(List<T>, T, String)",
"assertListNotContains(List<T>, Predicate<T>, String)",
"assertListNotContainsObject(List<T>, T, String)",
"assertNotEquals(Collection<?>, Collection<?>)",
"assertNotEquals(Collection<?>, Collection<?>, String)",
"assertNotEquals(Iterator<?>, Iterator<?>)",
"assertNotEquals(Iterator<?>, Iterator<?>, String)",
"assertNotEquals(Object[], Object[], String)",
/*
* These `assertNotEqualsDeep` methods cannot (easily) be expressed using AssertJ because they
* mix regular equality and array equality:
*/
"assertNotEqualsDeep(Map<?, ?>, Map<?, ?>)",
"assertNotEqualsDeep(Map<?, ?>, Map<?, ?>, String)",
"assertNotEqualsDeep(Set<?>, Set<?>)",
"assertNotEqualsDeep(Set<?>, Set<?>, String)",
// XXX: Add a migration for this `assertThrows` method.
"assertThrows(String, Class<T>, ThrowingRunnable)",
/*
* These `expectThrows` methods return the caught exception; there is no direct counterpart
* for this in AssertJ.
*/
"expectThrows(Class<T>, ThrowingRunnable)",
"expectThrows(String, Class<T>, ThrowingRunnable)"
})
final class TestNGToAssertJRules {
private TestNGToAssertJRules() {}

View File

@@ -13,6 +13,7 @@ import com.google.common.collect.Streams;
import java.util.Collections;
import java.util.HashSet;
import java.util.stream.Stream;
import reactor.core.publisher.Mono;
import tech.picnic.errorprone.refaster.test.RefasterRuleCollectionTestCase;
final class AssortedRulesTest implements RefasterRuleCollectionTestCase {
@@ -28,6 +29,10 @@ final class AssortedRulesTest implements RefasterRuleCollectionTestCase {
toImmutableSet());
}
Mono<Void> testIllegalStateExceptionSupplier() {
return Mono.error(() -> new IllegalStateException("ISE"));
}
int testCheckIndex() {
return Preconditions.checkElementIndex(0, 1);
}

View File

@@ -29,7 +29,9 @@ final class CollectionRulesTest implements RefasterRuleCollectionTestCase {
ImmutableSet.of(5).size() > 0,
ImmutableSet.of(6).size() >= 1,
Iterables.isEmpty(ImmutableSet.of(7)),
ImmutableSet.of(8).asList().isEmpty());
ImmutableSet.of(8).stream().findAny().isEmpty(),
ImmutableSet.of(9).stream().findFirst().isEmpty(),
ImmutableSet.of(10).asList().isEmpty());
}
ImmutableSet<Integer> testCollectionSize() {

View File

@@ -29,7 +29,9 @@ final class CollectionRulesTest implements RefasterRuleCollectionTestCase {
!ImmutableSet.of(5).isEmpty(),
!ImmutableSet.of(6).isEmpty(),
ImmutableSet.of(7).isEmpty(),
ImmutableSet.of(8).isEmpty());
ImmutableSet.of(8).isEmpty(),
ImmutableSet.of(9).isEmpty(),
ImmutableSet.of(10).isEmpty());
}
ImmutableSet<Integer> testCollectionSize() {

View File

@@ -20,4 +20,12 @@ final class InputStreamRulesTest implements RefasterRuleCollectionTestCase {
byte[] testInputStreamReadAllBytes() throws IOException {
return ByteStreams.toByteArray(new ByteArrayInputStream(new byte[0]));
}
byte[] testInputStreamReadNBytes() throws IOException {
return ByteStreams.limit(new ByteArrayInputStream(new byte[0]), 0).readAllBytes();
}
void testInputStreamSkipNBytes() throws IOException {
ByteStreams.skipFully(new ByteArrayInputStream(new byte[0]), 0);
}
}

View File

@@ -20,4 +20,12 @@ final class InputStreamRulesTest implements RefasterRuleCollectionTestCase {
byte[] testInputStreamReadAllBytes() throws IOException {
return new ByteArrayInputStream(new byte[0]).readAllBytes();
}
byte[] testInputStreamReadNBytes() throws IOException {
return new ByteArrayInputStream(new byte[0]).readNBytes(0);
}
void testInputStreamSkipNBytes() throws IOException {
new ByteArrayInputStream(new byte[0]).skipNBytes(0);
}
}

View File

@@ -1,10 +1,13 @@
package tech.picnic.errorprone.refasterrules;
import static com.google.common.collect.ImmutableList.toImmutableList;
import static com.google.common.collect.ImmutableMap.toImmutableMap;
import static com.google.common.collect.ImmutableSet.toImmutableSet;
import static java.util.Comparator.comparingInt;
import static java.util.Comparator.reverseOrder;
import static java.util.function.Function.identity;
import static java.util.function.Predicate.not;
import static java.util.stream.Collectors.collectingAndThen;
import static java.util.stream.Collectors.counting;
import static java.util.stream.Collectors.filtering;
import static java.util.stream.Collectors.flatMapping;
@@ -21,11 +24,14 @@ import static java.util.stream.Collectors.summingInt;
import static java.util.stream.Collectors.summingLong;
import com.google.common.collect.ImmutableList;
import com.google.common.collect.ImmutableMap;
import com.google.common.collect.ImmutableSet;
import com.google.common.collect.Streams;
import java.util.DoubleSummaryStatistics;
import java.util.IntSummaryStatistics;
import java.util.List;
import java.util.LongSummaryStatistics;
import java.util.Map;
import java.util.Objects;
import java.util.Optional;
import java.util.function.Function;
@@ -38,8 +44,12 @@ final class StreamRulesTest implements RefasterRuleCollectionTestCase {
public ImmutableSet<Object> elidedTypesAndStaticImports() {
return ImmutableSet.of(
ImmutableList.class,
ImmutableMap.class,
List.class,
Map.class,
Objects.class,
Streams.class,
collectingAndThen(null, null),
counting(),
filtering(null, null),
flatMapping(null, null),
@@ -54,7 +64,9 @@ final class StreamRulesTest implements RefasterRuleCollectionTestCase {
summarizingLong(null),
summingDouble(null),
summingInt(null),
summingLong(null));
summingLong(null),
toImmutableList(),
toImmutableMap(null, null));
}
String testJoining() {
@@ -114,7 +126,12 @@ final class StreamRulesTest implements RefasterRuleCollectionTestCase {
Stream.of(2).count() <= 0,
Stream.of(3).count() < 1,
Stream.of(4).findFirst().isEmpty(),
Stream.of(5).collect(toImmutableSet()).isEmpty());
Stream.of(5).collect(toImmutableSet()).isEmpty(),
Stream.of(6).collect(collectingAndThen(toImmutableList(), List::isEmpty)),
Stream.of(7).collect(collectingAndThen(toImmutableList(), ImmutableList::isEmpty)),
Stream.of(8).collect(collectingAndThen(toImmutableMap(k -> k, v -> v), Map::isEmpty)),
Stream.of(9)
.collect(collectingAndThen(toImmutableMap(k -> k, v -> v), ImmutableMap::isEmpty)));
}
ImmutableSet<Boolean> testStreamIsNotEmpty() {

View File

@@ -1,11 +1,14 @@
package tech.picnic.errorprone.refasterrules;
import static com.google.common.collect.ImmutableList.toImmutableList;
import static com.google.common.collect.ImmutableMap.toImmutableMap;
import static com.google.common.collect.ImmutableSet.toImmutableSet;
import static java.util.Comparator.comparingInt;
import static java.util.Comparator.naturalOrder;
import static java.util.Comparator.reverseOrder;
import static java.util.function.Function.identity;
import static java.util.function.Predicate.not;
import static java.util.stream.Collectors.collectingAndThen;
import static java.util.stream.Collectors.counting;
import static java.util.stream.Collectors.filtering;
import static java.util.stream.Collectors.flatMapping;
@@ -22,12 +25,15 @@ import static java.util.stream.Collectors.summingInt;
import static java.util.stream.Collectors.summingLong;
import com.google.common.collect.ImmutableList;
import com.google.common.collect.ImmutableMap;
import com.google.common.collect.ImmutableSet;
import com.google.common.collect.Streams;
import java.util.Arrays;
import java.util.DoubleSummaryStatistics;
import java.util.IntSummaryStatistics;
import java.util.List;
import java.util.LongSummaryStatistics;
import java.util.Map;
import java.util.Objects;
import java.util.Optional;
import java.util.function.Function;
@@ -40,8 +46,12 @@ final class StreamRulesTest implements RefasterRuleCollectionTestCase {
public ImmutableSet<Object> elidedTypesAndStaticImports() {
return ImmutableSet.of(
ImmutableList.class,
ImmutableMap.class,
List.class,
Map.class,
Objects.class,
Streams.class,
collectingAndThen(null, null),
counting(),
filtering(null, null),
flatMapping(null, null),
@@ -56,7 +66,9 @@ final class StreamRulesTest implements RefasterRuleCollectionTestCase {
summarizingLong(null),
summingDouble(null),
summingInt(null),
summingLong(null));
summingLong(null),
toImmutableList(),
toImmutableMap(null, null));
}
String testJoining() {
@@ -115,7 +127,11 @@ final class StreamRulesTest implements RefasterRuleCollectionTestCase {
Stream.of(2).findAny().isEmpty(),
Stream.of(3).findAny().isEmpty(),
Stream.of(4).findAny().isEmpty(),
Stream.of(5).findAny().isEmpty());
Stream.of(5).findAny().isEmpty(),
Stream.of(6).findAny().isEmpty(),
Stream.of(7).findAny().isEmpty(),
Stream.of(8).findAny().isEmpty(),
Stream.of(9).findAny().isEmpty());
}
ImmutableSet<Boolean> testStreamIsNotEmpty() {

View File

@@ -73,6 +73,18 @@ final class StringRulesTest implements RefasterRuleCollectionTestCase {
return Objects.toString("foo");
}
ImmutableSet<String> testNewStringFromCharArraySubSequence() {
return ImmutableSet.of(
String.valueOf(new char[] {'f', 'o', 'o'}, 0, 1),
String.copyValueOf(new char[] {'b', 'a', 'r'}, 2, 3));
}
ImmutableSet<String> testNewStringFromCharArray() {
return ImmutableSet.of(
String.valueOf(new char[] {'f', 'o', 'o'}),
new String(new char[] {'b', 'a', 'r'}, 0, new char[] {'b', 'a', 'r'}.length));
}
Function<Object, String> testStringValueOfMethodReference() {
return Objects::toString;
}

View File

@@ -75,6 +75,16 @@ final class StringRulesTest implements RefasterRuleCollectionTestCase {
return String.valueOf("foo");
}
ImmutableSet<String> testNewStringFromCharArraySubSequence() {
return ImmutableSet.of(
new String(new char[] {'f', 'o', 'o'}, 0, 1), new String(new char[] {'b', 'a', 'r'}, 2, 3));
}
ImmutableSet<String> testNewStringFromCharArray() {
return ImmutableSet.of(
new String(new char[] {'f', 'o', 'o'}), new String(new char[] {'b', 'a', 'r'}));
}
Function<Object, String> testStringValueOfMethodReference() {
return String::valueOf;
}

View File

@@ -5,7 +5,7 @@
<parent>
<groupId>tech.picnic.error-prone-support</groupId>
<artifactId>error-prone-support</artifactId>
<version>0.14.1-SNAPSHOT</version>
<version>0.15.1-SNAPSHOT</version>
</parent>
<artifactId>error-prone-experimental</artifactId>
@@ -20,6 +20,11 @@
<artifactId>error_prone_annotation</artifactId>
<scope>provided</scope>
</dependency>
<dependency>
<groupId>${groupId.error-prone}</groupId>
<artifactId>error_prone_annotations</artifactId>
<scope>provided</scope>
</dependency>
<dependency>
<groupId>${groupId.error-prone}</groupId>
<artifactId>error_prone_check_api</artifactId>
@@ -45,6 +50,11 @@
<artifactId>guava</artifactId>
<scope>provided</scope>
</dependency>
<dependency>
<groupId>org.jspecify</groupId>
<artifactId>jspecify</artifactId>
<scope>provided</scope>
</dependency>
<dependency>
<groupId>org.junit.jupiter</groupId>
<artifactId>junit-jupiter-api</artifactId>

View File

@@ -27,7 +27,6 @@ import com.sun.source.tree.MethodInvocationTree;
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;
@@ -84,22 +83,19 @@ public final class MethodReferenceUsage extends BugChecker implements LambdaExpr
.orElse(Description.NO_MATCH);
}
// XXX: Use switch pattern matching once the targeted JDK supports this.
private static Optional<SuggestedFix.Builder> 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<SuggestedFix.Builder> constructMethodRef(
@@ -117,33 +113,35 @@ public final class MethodReferenceUsage extends BugChecker implements LambdaExpr
.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<SuggestedFix.Builder> constructMethodRef(
LambdaExpressionTree lambdaExpr,
MethodInvocationTree subTree,
Optional<Name> expectedInstance) {
ExpressionTree methodSelect = subTree.getMethodSelect();
switch (methodSelect.getKind()) {
case IDENTIFIER:
if (expectedInstance.isPresent()) {
/* Direct method call; there is no matching "implicit parameter". */
return Optional.empty();
}
Symbol sym = ASTHelpers.getSymbol(methodSelect);
return 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());
if (methodSelect instanceof IdentifierTree) {
if (expectedInstance.isPresent()) {
/* Direct method call; there is no matching "implicit parameter". */
return Optional.empty();
}
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<SuggestedFix.Builder> constructMethodRef(
LambdaExpressionTree lambdaExpr, MemberSelectTree subTree, Optional<Name> 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
@@ -152,12 +150,12 @@ public final class MethodReferenceUsage extends BugChecker implements LambdaExpr
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();
}
@@ -182,8 +180,8 @@ public final class MethodReferenceUsage extends BugChecker implements LambdaExpr
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();
}
}

View File

@@ -0,0 +1,4 @@
/** Experimental Error Prone checks. */
@com.google.errorprone.annotations.CheckReturnValue
@org.jspecify.annotations.NullMarked
package tech.picnic.errorprone.experimental.bugpatterns;

View File

@@ -5,7 +5,7 @@
<parent>
<groupId>tech.picnic.error-prone-support</groupId>
<artifactId>error-prone-support</artifactId>
<version>0.14.1-SNAPSHOT</version>
<version>0.15.1-SNAPSHOT</version>
</parent>
<artifactId>error-prone-guidelines</artifactId>
@@ -53,6 +53,16 @@
<artifactId>error-prone-utils</artifactId>
<scope>provided</scope>
</dependency>
<dependency>
<groupId>${project.groupId}</groupId>
<artifactId>refaster-support</artifactId>
<scope>provided</scope>
</dependency>
<dependency>
<groupId>com.google.auto</groupId>
<artifactId>auto-common</artifactId>
<scope>provided</scope>
</dependency>
<dependency>
<groupId>com.google.auto.service</groupId>
<artifactId>auto-service-annotations</artifactId>

View File

@@ -0,0 +1,140 @@
package tech.picnic.errorprone.guidelines.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.LIKELY_ERROR;
import static com.google.errorprone.matchers.ChildMultiMatcher.MatchType.AT_LEAST_ONE;
import static com.google.errorprone.matchers.FieldMatchers.staticField;
import static com.google.errorprone.matchers.Matchers.annotations;
import static com.google.errorprone.matchers.Matchers.isType;
import static com.google.errorprone.matchers.Matchers.packageStartsWith;
import static tech.picnic.errorprone.utils.Documentation.BUG_PATTERNS_BASE_URL;
import com.google.auto.service.AutoService;
import com.google.common.collect.ImmutableList;
import com.google.common.collect.Iterables;
import com.google.errorprone.BugPattern;
import com.google.errorprone.VisitorState;
import com.google.errorprone.bugpatterns.BugChecker;
import com.google.errorprone.bugpatterns.BugChecker.ClassTreeMatcher;
import com.google.errorprone.fixes.SuggestedFix;
import com.google.errorprone.fixes.SuggestedFixes;
import com.google.errorprone.matchers.AnnotationMatcherUtils;
import com.google.errorprone.matchers.Description;
import com.google.errorprone.matchers.Matcher;
import com.google.errorprone.matchers.MultiMatcher;
import com.google.errorprone.util.ASTHelpers;
import com.sun.source.tree.AnnotationTree;
import com.sun.source.tree.BinaryTree;
import com.sun.source.tree.ClassTree;
import com.sun.source.tree.ExpressionTree;
import com.sun.source.tree.Tree.Kind;
import com.sun.tools.javac.util.Constants;
import javax.lang.model.element.Name;
/**
* A {@link BugChecker} that flags {@link BugChecker} declarations inside {@code
* tech.picnic.errorprone.*} packages that do not reference the Error Prone Support website.
*/
// XXX: Introduce a similar check to enforce the Refaster `@OnlineDocumentation` annotation. (Or
// update the website generation to document Refaster collections by default, and provide an
// exclusion annotation instead. This may make more sense.)
@AutoService(BugChecker.class)
@BugPattern(
summary = "Error Prone Support checks must reference their online documentation",
link = BUG_PATTERNS_BASE_URL + "BugPatternLink",
linkType = CUSTOM,
severity = SUGGESTION,
tags = LIKELY_ERROR)
public final class BugPatternLink extends BugChecker implements ClassTreeMatcher {
private static final long serialVersionUID = 1L;
private static final Matcher<ClassTree> IS_ERROR_PRONE_SUPPORT_CLASS =
packageStartsWith("tech.picnic.errorprone");
private static final Matcher<ExpressionTree> IS_LINK_TYPE_NONE =
staticField(BugPattern.LinkType.class.getCanonicalName(), "NONE");
private static final Matcher<ExpressionTree> IS_BUG_PATTERNS_BASE_URL =
staticField("tech.picnic.errorprone.utils.Documentation", "BUG_PATTERNS_BASE_URL");
private static final MultiMatcher<ClassTree, AnnotationTree> HAS_BUG_PATTERN_ANNOTATION =
annotations(AT_LEAST_ONE, isType(BugPattern.class.getCanonicalName()));
/** Instantiates a new {@link BugPatternLink} instance. */
public BugPatternLink() {}
@Override
public Description matchClass(ClassTree tree, VisitorState state) {
if (ASTHelpers.findEnclosingNode(state.getPath(), ClassTree.class) != null) {
/*
* This is a nested class; even if it's bug checker, then it's likely declared within a test
* class.
*/
return Description.NO_MATCH;
}
if (!IS_ERROR_PRONE_SUPPORT_CLASS.matches(tree, state)) {
/*
* Bug checkers defined elsewhere are unlikely to be documented on the Error Prone Support
* website.
*/
return Description.NO_MATCH;
}
ImmutableList<AnnotationTree> bugPatternAnnotations =
HAS_BUG_PATTERN_ANNOTATION.multiMatchResult(tree, state).matchingNodes();
if (bugPatternAnnotations.isEmpty()) {
/* This isn't a bug checker. */
return Description.NO_MATCH;
}
AnnotationTree annotation = Iterables.getOnlyElement(bugPatternAnnotations);
if (isCompliant(annotation, tree.getSimpleName(), state)) {
/* The bug checker is correctly configured. */
return Description.NO_MATCH;
}
return describeMatch(annotation, suggestFix(tree, state, annotation));
}
private static boolean isCompliant(
AnnotationTree annotation, Name className, VisitorState state) {
ExpressionTree linkType = AnnotationMatcherUtils.getArgument(annotation, "linkType");
if (IS_LINK_TYPE_NONE.matches(linkType, state)) {
/* This bug checker explicitly declares that there is no link. */
return true;
}
ExpressionTree link = AnnotationMatcherUtils.getArgument(annotation, "link");
if (!(link instanceof BinaryTree binary)) {
return false;
}
verify(binary.getKind() == Kind.PLUS, "Unexpected binary operator");
return IS_BUG_PATTERNS_BASE_URL.matches(binary.getLeftOperand(), state)
&& className.contentEquals(ASTHelpers.constValue(binary.getRightOperand(), String.class));
}
private static SuggestedFix suggestFix(
ClassTree tree, VisitorState state, AnnotationTree annotation) {
SuggestedFix.Builder fix = SuggestedFix.builder();
String linkPrefix =
SuggestedFixes.qualifyStaticImport(
"tech.picnic.errorprone.utils.Documentation.BUG_PATTERNS_BASE_URL", fix, state);
fix.merge(
SuggestedFixes.updateAnnotationArgumentValues(
annotation,
state,
"link",
ImmutableList.of(
linkPrefix + " + " + Constants.format(tree.getSimpleName().toString()))));
String linkType =
SuggestedFixes.qualifyStaticImport(
BugPattern.LinkType.class.getCanonicalName() + ".CUSTOM", fix, state);
fix.merge(
SuggestedFixes.updateAnnotationArgumentValues(
annotation, state, "linkType", ImmutableList.of(linkType)));
return fix.build();
}
}

View File

@@ -50,8 +50,9 @@ import tech.picnic.errorprone.utils.ThirdPartyLibrary;
@AutoService(BugChecker.class)
@BugPattern(
summary =
"Prefer `Class#getCanonicalName()` over an equivalent string literal if and only if the "
+ "type will be on the runtime classpath",
"""
Prefer `Class#getCanonicalName()` over an equivalent string literal if and only if the \
type will be on the runtime classpath""",
link = BUG_PATTERNS_BASE_URL + "ErrorProneRuntimeClasspath",
linkType = CUSTOM,
severity = SUGGESTION,

View File

@@ -0,0 +1,232 @@
package tech.picnic.errorprone.guidelines.bugpatterns;
import static com.google.common.base.Verify.verify;
import static com.google.common.collect.ImmutableList.toImmutableList;
import static com.google.errorprone.BugPattern.LinkType.CUSTOM;
import static com.google.errorprone.BugPattern.SeverityLevel.WARNING;
import static com.google.errorprone.BugPattern.StandardTags.LIKELY_ERROR;
import static com.google.errorprone.matchers.ChildMultiMatcher.MatchType.AT_LEAST_ONE;
import static com.google.errorprone.matchers.Matchers.annotations;
import static com.google.errorprone.matchers.Matchers.isType;
import static java.util.Comparator.comparing;
import static java.util.stream.Collectors.toCollection;
import static tech.picnic.errorprone.utils.Documentation.BUG_PATTERNS_BASE_URL;
import com.google.auto.common.AnnotationMirrors;
import com.google.auto.service.AutoService;
import com.google.common.collect.ImmutableList;
import com.google.common.collect.Streams;
import com.google.errorprone.BugPattern;
import com.google.errorprone.VisitorState;
import com.google.errorprone.bugpatterns.BugChecker;
import com.google.errorprone.bugpatterns.BugChecker.ClassTreeMatcher;
import com.google.errorprone.fixes.SuggestedFixes;
import com.google.errorprone.matchers.Description;
import com.google.errorprone.matchers.MultiMatcher;
import com.google.errorprone.matchers.MultiMatcher.MultiMatchResult;
import com.google.errorprone.refaster.annotation.BeforeTemplate;
import com.google.errorprone.util.ASTHelpers;
import com.google.errorprone.util.Signatures;
import com.sun.source.tree.AnnotationTree;
import com.sun.source.tree.ClassTree;
import com.sun.source.tree.MethodInvocationTree;
import com.sun.source.tree.MethodTree;
import com.sun.source.tree.NewClassTree;
import com.sun.source.tree.Tree;
import com.sun.source.util.TreeScanner;
import com.sun.tools.javac.code.Attribute;
import com.sun.tools.javac.code.Symbol.ClassSymbol;
import com.sun.tools.javac.code.Symbol.MethodSymbol;
import com.sun.tools.javac.code.Symbol.TypeSymbol;
import com.sun.tools.javac.util.Constants;
import java.util.Comparator;
import java.util.HashMap;
import java.util.HashSet;
import java.util.Map;
import java.util.Set;
import java.util.function.Consumer;
import javax.lang.model.element.AnnotationMirror;
import javax.lang.model.element.AnnotationValue;
import javax.lang.model.element.Modifier;
import org.jspecify.annotations.Nullable;
/**
* A {@link BugChecker} that validates the claim made by {@link
* tech.picnic.errorprone.refaster.annotation.TypeMigration} annotations.
*/
// XXX: As-is this checker assumes that a method is fully migrated if it is invoked inside at least
// one `@BeforeTemplate` method. A stronger check would be to additionally verify that:
// 1. Such invocations are not conditionally matched. That is, there should be no constraint on
// their context (i.e. any surrounding code), and their parameters must be `@BeforeTemplate`
// method parameters with types that are not more restrictive than those of the method itself.
// Additionally, the result of non-void methods should be "returned" by the `@BeforeTemplate`
// method, so that Refaster will match any expression, rather than just statements. (One caveat
// with this "context-independent migrations only" approach is that APIs often expose methods
// that are only useful in combination with other methods of the API; insisting that such methods
// are migrated in isolation is unreasonable.)
// 2. Where relevant, method references should also be migrated. (TBD what "relevant" means in this
// case, and whether in fact method reference matchers can be _derived_ from the associated
// method invocation matchers.)
// XXX: This checker currently does no concern itself with public fields. Consider adding support
// for those.
@AutoService(BugChecker.class)
@BugPattern(
summary =
"""
The set of unmigrated methods listed by the `@TypeMigration` annotation must be minimal \
yet exhaustive""",
link = BUG_PATTERNS_BASE_URL + "ExhaustiveRefasterTypeMigration",
linkType = CUSTOM,
severity = WARNING,
tags = LIKELY_ERROR)
public final class ExhaustiveRefasterTypeMigration extends BugChecker implements ClassTreeMatcher {
private static final long serialVersionUID = 1L;
private static final MultiMatcher<Tree, AnnotationTree> IS_TYPE_MIGRATION =
annotations(AT_LEAST_ONE, isType("tech.picnic.errorprone.refaster.annotation.TypeMigration"));
private static final MultiMatcher<Tree, AnnotationTree> HAS_BEFORE_TEMPLATE =
annotations(AT_LEAST_ONE, isType(BeforeTemplate.class.getCanonicalName()));
private static final String TYPE_MIGRATION_TYPE_ELEMENT = "of";
private static final String TYPE_MIGRATION_UNMIGRATED_METHODS_ELEMENT = "unmigratedMethods";
/** Instantiates a new {@link ExhaustiveRefasterTypeMigration} instance. */
public ExhaustiveRefasterTypeMigration() {}
@Override
public Description matchClass(ClassTree tree, VisitorState state) {
MultiMatchResult<AnnotationTree> migrationAnnotations =
IS_TYPE_MIGRATION.multiMatchResult(tree, state);
if (!migrationAnnotations.matches()) {
return Description.NO_MATCH;
}
AnnotationTree migrationAnnotation = migrationAnnotations.onlyMatchingNode();
AnnotationMirror annotationMirror = ASTHelpers.getAnnotationMirror(migrationAnnotation);
TypeSymbol migratedType = getMigratedType(annotationMirror);
if (migratedType.asType().isPrimitive() || !(migratedType instanceof ClassSymbol)) {
return buildDescription(migrationAnnotation)
.setMessage(String.format("Migration of type '%s' is unsupported", migratedType))
.build();
}
ImmutableList<String> methodsClaimedUnmigrated = getMethodsClaimedUnmigrated(annotationMirror);
ImmutableList<String> unmigratedMethods =
getMethodsDefinitelyUnmigrated(
tree, (ClassSymbol) migratedType, signatureOrder(methodsClaimedUnmigrated), state);
if (unmigratedMethods.equals(methodsClaimedUnmigrated)) {
return Description.NO_MATCH;
}
/*
* The `@TypeMigration` annotation lists a different set of unmigrated methods than the one
* produced by our analysis; suggest a replacement.
*/
// XXX: `updateAnnotationArgumentValues` will prepend the new attribute argument if it is not
// already present. It would be nicer if it _appended_ the new attribute.
return describeMatch(
migrationAnnotation,
SuggestedFixes.updateAnnotationArgumentValues(
migrationAnnotation,
state,
TYPE_MIGRATION_UNMIGRATED_METHODS_ELEMENT,
unmigratedMethods.stream().map(Constants::format).collect(toImmutableList()))
.build());
}
private static TypeSymbol getMigratedType(AnnotationMirror migrationAnnotation) {
AnnotationValue value =
AnnotationMirrors.getAnnotationValue(migrationAnnotation, TYPE_MIGRATION_TYPE_ELEMENT);
verify(
value instanceof Attribute.Class,
"Value of annotation element `%s` is '%s' rather than a class",
TYPE_MIGRATION_TYPE_ELEMENT,
value);
return ((Attribute.Class) value).classType.tsym;
}
private static ImmutableList<String> getMethodsClaimedUnmigrated(
AnnotationMirror migrationAnnotation) {
AnnotationValue value =
AnnotationMirrors.getAnnotationValue(
migrationAnnotation, TYPE_MIGRATION_UNMIGRATED_METHODS_ELEMENT);
verify(
value instanceof Attribute.Array,
"Value of annotation element `%s` is '%s' rather than an array",
TYPE_MIGRATION_UNMIGRATED_METHODS_ELEMENT,
value);
return ((Attribute.Array) value)
.getValue().stream().map(a -> a.getValue().toString()).collect(toImmutableList());
}
// XXX: Once only JDK 14 and above are supported, change the
// `m.getModifiers().contains(Modifier.PUBLIC)` check to just `m.isPublic()`.
private static ImmutableList<String> getMethodsDefinitelyUnmigrated(
ClassTree tree, ClassSymbol migratedType, Comparator<String> comparator, VisitorState state) {
Set<MethodSymbol> publicMethods =
Streams.stream(
ASTHelpers.scope(migratedType.members())
.getSymbols(
m ->
m.getModifiers().contains(Modifier.PUBLIC)
&& m instanceof MethodSymbol))
.map(MethodSymbol.class::cast)
.collect(toCollection(HashSet::new));
/* Remove methods that *appear* to be migrated. Note that this is an imperfect heuristic. */
removeMethodsInvokedInBeforeTemplateMethods(tree, publicMethods, state);
return publicMethods.stream()
.map(m -> Signatures.prettyMethodSignature(migratedType, m))
.sorted(comparator)
.collect(toImmutableList());
}
/**
* Creates a {@link Comparator} that orders method signatures to match the given list of
* signatures, with any signatures not listed ordered first, lexicographically.
*
* @implNote This method does not use {@code comparing(list::indexOf)}, as that would make each
* comparison a linear, rather than constant-time operation.
*/
private static Comparator<String> signatureOrder(ImmutableList<String> existingOrder) {
Map<String, Integer> knownEntries = new HashMap<>();
for (int i = 0; i < existingOrder.size(); i++) {
knownEntries.putIfAbsent(existingOrder.get(i), i);
}
// XXX: The lexicographical order applied to unknown entries aims to match the order applied by
// the `LexicographicalAnnotationAttributeListing` check; consider deduplicating this logic.
return comparing((String v) -> knownEntries.getOrDefault(v, -1))
.thenComparing(String.CASE_INSENSITIVE_ORDER);
}
/**
* Removes from the given set of {@link MethodSymbol}s the ones that refer to a method that is
* invoked inside a {@link com.google.errorprone.refaster.annotation.BeforeTemplate} method inside
* the specified {@link ClassTree}.
*/
private static void removeMethodsInvokedInBeforeTemplateMethods(
ClassTree tree, Set<MethodSymbol> candidates, VisitorState state) {
new TreeScanner<@Nullable Void, Consumer<MethodSymbol>>() {
@Override
public @Nullable Void visitMethod(MethodTree tree, Consumer<MethodSymbol> sink) {
return HAS_BEFORE_TEMPLATE.matches(tree, state)
? super.visitMethod(tree, candidates::remove)
: null;
}
@Override
public @Nullable Void visitNewClass(NewClassTree tree, Consumer<MethodSymbol> sink) {
sink.accept(ASTHelpers.getSymbol(tree));
return super.visitNewClass(tree, sink);
}
@Override
public @Nullable Void visitMethodInvocation(
MethodInvocationTree tree, Consumer<MethodSymbol> sink) {
sink.accept(ASTHelpers.getSymbol(tree));
return super.visitMethodInvocation(tree, sink);
}
}.scan(tree, s -> {});
}
}

View File

@@ -42,21 +42,18 @@ public final class RefasterAnyOfUsage extends BugChecker implements MethodInvoca
@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)));
}
}

View File

@@ -45,16 +45,14 @@ public final class UnqualifiedSuggestedFixImport extends BugChecker
return Description.NO_MATCH;
}
switch (ASTHelpers.getSymbol(tree).getSimpleName().toString()) {
case "addImport":
return createDescription(
tree, "SuggestedFix.Builder#addImport", "SuggestedFixes#qualifyType");
case "addStaticImport":
return createDescription(
tree, "SuggestedFix.Builder#addStaticImport", "SuggestedFixes#qualifyStaticImport");
default:
return Description.NO_MATCH;
}
return switch (ASTHelpers.getSymbol(tree).getSimpleName().toString()) {
case "addImport" ->
createDescription(tree, "SuggestedFix.Builder#addImport", "SuggestedFixes#qualifyType");
case "addStaticImport" ->
createDescription(
tree, "SuggestedFix.Builder#addStaticImport", "SuggestedFixes#qualifyStaticImport");
default -> Description.NO_MATCH;
};
}
private Description createDescription(

View File

@@ -0,0 +1,193 @@
package tech.picnic.errorprone.guidelines.bugpatterns;
import com.google.errorprone.BugCheckerRefactoringTestHelper;
import com.google.errorprone.BugCheckerRefactoringTestHelper.TestMode;
import com.google.errorprone.CompilationTestHelper;
import org.junit.jupiter.api.Test;
final class BugPatternLinkTest {
@Test
void identification() {
CompilationTestHelper.newInstance(BugPatternLink.class, getClass())
.addSourceLines(
"A.java",
"import com.google.errorprone.BugPattern;",
"",
"@BugPattern(summary = \"Class in default package\", severity = BugPattern.SeverityLevel.ERROR)",
"class A {}")
.addSourceLines(
"com/example/B.java",
"package com.example;",
"",
"import com.google.errorprone.BugPattern;",
"",
"@BugPattern(summary = \"Class in custom package\", severity = BugPattern.SeverityLevel.ERROR)",
"class B {}")
.addSourceLines(
"tech/picnic/errorprone/C.java",
"package tech.picnic.errorprone;",
"",
"import com.google.errorprone.BugPattern;",
"",
"@BugPattern(",
" summary = \"Class explicitly without link\",",
" linkType = BugPattern.LinkType.NONE,",
" severity = BugPattern.SeverityLevel.ERROR)",
"class C {}")
.addSourceLines(
"tech/picnic/errorprone/subpackage/D.java",
"package tech.picnic.errorprone.subpackage;",
"",
"import com.google.errorprone.BugPattern;",
"import tech.picnic.errorprone.utils.Documentation;",
"",
"@BugPattern(",
" summary = \"Error Prone Support class in subpackage with proper link\",",
" link = Documentation.BUG_PATTERNS_BASE_URL + \"D\",",
" linkType = BugPattern.LinkType.CUSTOM,",
" severity = BugPattern.SeverityLevel.ERROR)",
"class D {}")
.addSourceLines(
"tech/picnic/errorprone/E.java",
"package tech.picnic.errorprone;",
"",
"import static com.google.errorprone.BugPattern.LinkType.CUSTOM;",
"import static com.google.errorprone.BugPattern.SeverityLevel.ERROR;",
"import static tech.picnic.errorprone.utils.Documentation.BUG_PATTERNS_BASE_URL;",
"",
"import com.google.errorprone.BugPattern;",
"",
"@BugPattern(",
" summary = \"Error Prone Support class with proper link and static imports\",",
" link = BUG_PATTERNS_BASE_URL + \"E\",",
" linkType = CUSTOM,",
" severity = ERROR)",
"class E {}")
.addSourceLines(
"tech/picnic/errorprone/F.java",
"package tech.picnic.errorprone;",
"",
"import com.google.errorprone.BugPattern;",
"",
"class F {",
" @BugPattern(",
" summary = \"Nested Error Prone Support class\",",
" severity = BugPattern.SeverityLevel.ERROR)",
" class Inner {}",
"}")
.addSourceLines(
"tech/picnic/errorprone/G.java",
"package tech.picnic.errorprone;",
"",
"import com.google.errorprone.BugPattern;",
"",
"// BUG: Diagnostic contains:",
"@BugPattern(",
" summary = \"Error Prone Support class lacking link\",",
" severity = BugPattern.SeverityLevel.ERROR)",
"class G {}")
.addSourceLines(
"tech/picnic/errorprone/H.java",
"package tech.picnic.errorprone;",
"",
"import com.google.errorprone.BugPattern;",
"import tech.picnic.errorprone.utils.Documentation;",
"",
"// BUG: Diagnostic contains:",
"@BugPattern(",
" summary = \"Error Prone Support class with incorrect link\",",
" link = Documentation.BUG_PATTERNS_BASE_URL + \"NotH\",",
" linkType = BugPattern.LinkType.CUSTOM,",
" severity = BugPattern.SeverityLevel.ERROR)",
"class H {}")
.addSourceLines(
"tech/picnic/errorprone/I.java",
"package tech.picnic.errorprone;",
"",
"import com.google.errorprone.BugPattern;",
"",
"// BUG: Diagnostic contains:",
"@BugPattern(",
" summary = \"Error Prone Support class with non-canonical link\",",
" link = \"https://error-prone.picnic.tech/bugpatterns/I\",",
" linkType = BugPattern.LinkType.CUSTOM,",
" severity = BugPattern.SeverityLevel.ERROR)",
"class I {}")
.addSourceLines(
"tech/picnic/errorprone/J.java",
"package tech.picnic.errorprone;",
"",
"import com.google.errorprone.BugPattern;",
"",
"// BUG: Diagnostic contains:",
"@BugPattern(",
" summary = \"Error Prone Support class in with non-canonical link\",",
" link = \"https://error-prone.picnic.tech/bugpatterns/\" + \"J\",",
" linkType = BugPattern.LinkType.CUSTOM,",
" severity = BugPattern.SeverityLevel.ERROR)",
"class J {}")
.doTest();
}
@Test
void replacement() {
BugCheckerRefactoringTestHelper.newInstance(BugPatternLink.class, getClass())
.addInputLines(
"tech/picnic/errorprone/A.java",
"package tech.picnic.errorprone;",
"",
"import com.google.errorprone.BugPattern;",
"",
"@BugPattern(",
" summary = \"Error Prone Support class lacking link\",",
" severity = BugPattern.SeverityLevel.ERROR)",
"class A {}")
.addOutputLines(
"tech/picnic/errorprone/A.java",
"package tech.picnic.errorprone;",
"",
"import static com.google.errorprone.BugPattern.LinkType.CUSTOM;",
"import static tech.picnic.errorprone.utils.Documentation.BUG_PATTERNS_BASE_URL;",
"",
"import com.google.errorprone.BugPattern;",
"",
"@BugPattern(",
" link = BUG_PATTERNS_BASE_URL + \"A\",",
" linkType = CUSTOM,",
" summary = \"Error Prone Support class lacking link\",",
" severity = BugPattern.SeverityLevel.ERROR)",
"class A {}")
.addInputLines(
"tech/picnic/errorprone/B.java",
"package tech.picnic.errorprone;",
"",
"import static com.google.errorprone.BugPattern.LinkType.CUSTOM;",
"import static com.google.errorprone.BugPattern.SeverityLevel.ERROR;",
"",
"import com.google.errorprone.BugPattern;",
"",
"@BugPattern(",
" summary = \"Error Prone Support class with incorrect link\",",
" link = \"Not the right link\",",
" linkType = CUSTOM,",
" severity = ERROR)",
"class B {}")
.addOutputLines(
"tech/picnic/errorprone/B.java",
"package tech.picnic.errorprone;",
"",
"import static com.google.errorprone.BugPattern.LinkType.CUSTOM;",
"import static com.google.errorprone.BugPattern.SeverityLevel.ERROR;",
"import static tech.picnic.errorprone.utils.Documentation.BUG_PATTERNS_BASE_URL;",
"",
"import com.google.errorprone.BugPattern;",
"",
"@BugPattern(",
" summary = \"Error Prone Support class with incorrect link\",",
" link = BUG_PATTERNS_BASE_URL + \"B\",",
" linkType = CUSTOM,",
" severity = ERROR)",
"class B {}")
.doTest(TestMode.TEXT_MATCH);
}
}

View File

@@ -0,0 +1,263 @@
package tech.picnic.errorprone.guidelines.bugpatterns;
import com.google.errorprone.BugCheckerRefactoringTestHelper;
import com.google.errorprone.BugCheckerRefactoringTestHelper.TestMode;
import com.google.errorprone.CompilationTestHelper;
import org.junit.jupiter.api.Test;
final class ExhaustiveRefasterTypeMigrationTest {
@Test
void identification() {
CompilationTestHelper.newInstance(ExhaustiveRefasterTypeMigration.class, getClass())
.addSourceLines(
"Util.java",
"class Util {",
" public static int CONSTANT = 42;",
"",
" public static void publicStaticVoidMethod() {}",
"",
" static void packagePrivateStaticVoidMethod() {}",
"",
" protected static void protectedStaticVoidMethod() {}",
"",
" private static void privateStaticVoidMethod() {}",
"",
" public static int publicStaticIntMethod2() {",
" return 0;",
" }",
"",
" public String publicStringMethodWithArg(int arg) {",
" return String.valueOf(arg);",
" }",
"}")
.addSourceLines(
"A.java",
"import com.google.errorprone.refaster.annotation.AfterTemplate;",
"import com.google.errorprone.refaster.annotation.BeforeTemplate;",
"import tech.picnic.errorprone.refaster.annotation.TypeMigration;",
"",
"class A {",
" class UnannotatedEmptyClass {}",
"",
" // BUG: Diagnostic contains: Migration of type 'int' is unsupported",
" @TypeMigration(of = int.class)",
" class AnnotatedWithPrimitive {}",
"",
" @TypeMigration(",
" of = Util.class,",
" unmigratedMethods = {",
" \"publicStaticIntMethod2()\",",
" \"publicStringMethodWithArg(int)\",",
" \"publicStaticVoidMethod()\"",
" })",
" class AnnotatedEmptyClass {}",
"",
" @TypeMigration(",
" of = Util.class,",
" unmigratedMethods = {",
" \"publicStaticVoidMethod()\",",
" \"publicStringMethodWithArg(int)\",",
" \"publicStaticIntMethod2()\"",
" })",
" class AnnotatedEmptyClassWithUnsortedMethodListing {}",
"",
" class UnannotatedTemplate {",
" @BeforeTemplate",
" void before(int value) {",
" Util.publicStaticVoidMethod();",
" Util.publicStaticIntMethod2();",
" new Util().publicStringMethodWithArg(value);",
" }",
" }",
"",
" @TypeMigration(",
" of = Util.class,",
" unmigratedMethods = {",
" \"publicStaticIntMethod2()\",",
" \"publicStringMethodWithArg(int)\",",
" \"publicStaticVoidMethod()\"",
" })",
" class AnnotatedWithoutBeforeTemplate {",
" {",
" Util.publicStaticIntMethod2();",
" }",
"",
" @AfterTemplate",
" void after(int value) {",
" Util.publicStaticVoidMethod();",
" new Util().publicStringMethodWithArg(value);",
" }",
" }",
"",
" @TypeMigration(of = Util.class)",
" class AnnotatedFullyMigrated {",
" @BeforeTemplate",
" void before() {",
" new Util().publicStringMethodWithArg(Util.publicStaticIntMethod2());",
" }",
"",
" @BeforeTemplate",
" void before2() {",
" Util.publicStaticVoidMethod();",
" }",
" }",
"",
" @TypeMigration(of = Util.class, unmigratedMethods = \"publicStringMethodWithArg(int)\")",
" class AnnotatedPartiallyMigrated {",
" @BeforeTemplate",
" void before() {",
" Util.publicStaticVoidMethod();",
" Util.publicStaticIntMethod2();",
" }",
" }",
"",
" // BUG: Diagnostic contains: The set of unmigrated methods listed by the `@TypeMigration`",
" // annotation must be minimal yet exhaustive",
" @TypeMigration(of = Util.class, unmigratedMethods = \"publicStringMethodWithArg(int)\")",
" class AnnotatedWithIncompleteMethodListing {",
" @BeforeTemplate",
" void before() {",
" Util.publicStaticIntMethod2();",
" }",
" }",
"",
" // BUG: Diagnostic contains: The set of unmigrated methods listed by the `@TypeMigration`",
" // annotation must be minimal yet exhaustive",
" @TypeMigration(",
" of = Util.class,",
" unmigratedMethods = {\"publicStaticIntMethod2()\", \"publicStringMethodWithArg(int)\"})",
" class AnnotatedWithMigratedMethodReference {",
" @BeforeTemplate",
" void before() {",
" Util.publicStaticVoidMethod();",
" Util.publicStaticIntMethod2();",
" }",
" }",
"",
" // BUG: Diagnostic contains: The set of unmigrated methods listed by the `@TypeMigration`",
" // annotation must be minimal yet exhaustive",
" @TypeMigration(",
" of = Util.class,",
" unmigratedMethods = {\"extra\", \"publicStringMethodWithArg(int)\"})",
" class AnnotatedWithUnknownMethodReference {",
" @BeforeTemplate",
" void before() {",
" Util.publicStaticVoidMethod();",
" Util.publicStaticIntMethod2();",
" }",
" }",
"}")
.doTest();
}
@Test
void replacement() {
BugCheckerRefactoringTestHelper.newInstance(ExhaustiveRefasterTypeMigration.class, getClass())
.addInputLines(
"Util.java",
"public final class Util {",
" public static void publicStaticVoidMethod() {}",
"",
" public static int publicStaticIntMethod2() {",
" return 0;",
" }",
"",
" public String publicStringMethodWithArg(int arg) {",
" return String.valueOf(arg);",
" }",
"",
" public String publicStringMethodWithArg(String arg) {",
" return arg;",
" }",
"}")
.expectUnchanged()
.addInputLines(
"A.java",
"import com.google.errorprone.refaster.annotation.BeforeTemplate;",
"import tech.picnic.errorprone.refaster.annotation.TypeMigration;",
"",
"class A {",
" @TypeMigration(of = Util.class)",
" class AnnotatedWithoutMethodListing {",
" {",
" new Util().publicStringMethodWithArg(1);",
" }",
"",
" @BeforeTemplate",
" void before() {",
" Util.publicStaticIntMethod2();",
" }",
" }",
"",
" @TypeMigration(",
" of = Util.class,",
" unmigratedMethods = {\"publicStaticIntMethod2()\", \"extra\", \"publicStringMethodWithArg(int)\"})",
" class AnnotatedWithIncorrectMethodReference {",
" @BeforeTemplate",
" void before() {",
" new Util().publicStringMethodWithArg(\"1\");",
" Util.publicStaticVoidMethod();",
" Util.publicStaticIntMethod2();",
" }",
" }",
"",
" @TypeMigration(",
" of = Util.class,",
" unmigratedMethods = {\"publicStaticVoidMethod()\", \"publicStaticVoidMethod()\"})",
" class AnnotatedWithDuplicateMethodReference {",
" @BeforeTemplate",
" void before() {",
" new Util().publicStringMethodWithArg(1);",
" new Util().publicStringMethodWithArg(\"1\");",
" Util.publicStaticIntMethod2();",
" }",
" }",
"}")
.addOutputLines(
"A.java",
"import com.google.errorprone.refaster.annotation.BeforeTemplate;",
"import tech.picnic.errorprone.refaster.annotation.TypeMigration;",
"",
"class A {",
" @TypeMigration(",
" unmigratedMethods = {",
" \"publicStaticVoidMethod()\",",
" \"publicStringMethodWithArg(int)\",",
" \"publicStringMethodWithArg(String)\",",
" \"Util()\"",
" },",
" of = Util.class)",
" class AnnotatedWithoutMethodListing {",
" {",
" new Util().publicStringMethodWithArg(1);",
" }",
"",
" @BeforeTemplate",
" void before() {",
" Util.publicStaticIntMethod2();",
" }",
" }",
"",
" @TypeMigration(of = Util.class, unmigratedMethods = \"publicStringMethodWithArg(int)\")",
" class AnnotatedWithIncorrectMethodReference {",
" @BeforeTemplate",
" void before() {",
" new Util().publicStringMethodWithArg(\"1\");",
" Util.publicStaticVoidMethod();",
" Util.publicStaticIntMethod2();",
" }",
" }",
"",
" @TypeMigration(of = Util.class, unmigratedMethods = \"publicStaticVoidMethod()\")",
" class AnnotatedWithDuplicateMethodReference {",
" @BeforeTemplate",
" void before() {",
" new Util().publicStringMethodWithArg(1);",
" new Util().publicStringMethodWithArg(\"1\");",
" Util.publicStaticIntMethod2();",
" }",
" }",
"}")
.doTest(TestMode.TEXT_MATCH);
}
}

View File

@@ -5,7 +5,7 @@
<parent>
<groupId>tech.picnic.error-prone-support</groupId>
<artifactId>error-prone-support</artifactId>
<version>0.14.1-SNAPSHOT</version>
<version>0.15.1-SNAPSHOT</version>
</parent>
<artifactId>error-prone-utils</artifactId>

View File

@@ -9,7 +9,6 @@ import com.google.errorprone.util.ASTHelpers;
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 final class AnnotationAttributeMatcher implements Serializable {
}
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";
}

View File

@@ -99,14 +99,13 @@ public final class MoreJUnitMatchers {
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(

39
pom.xml
View File

@@ -4,7 +4,7 @@
<groupId>tech.picnic.error-prone-support</groupId>
<artifactId>error-prone-support</artifactId>
<version>0.14.1-SNAPSHOT</version>
<version>0.15.1-SNAPSHOT</version>
<packaging>pom</packaging>
<name>Picnic :: Error Prone Support</name>
@@ -148,7 +148,7 @@
<groupId.error-prone>com.google.errorprone</groupId.error-prone>
<!-- The build timestamp is derived from the most recent commit
timestamp in support of reproducible builds. -->
<project.build.outputTimestamp>${git.commit.time}</project.build.outputTimestamp>
<project.build.outputTimestamp>2024-02-11T13:31:59Z</project.build.outputTimestamp>
<project.build.sourceEncoding>UTF-8</project.build.sourceEncoding>
<!-- Glob pattern identifying Refaster rule definition files. These
Java classes don't contain "regular" code, and thus require special
@@ -211,11 +211,11 @@
<version.error-prone-orig>2.24.1</version.error-prone-orig>
<version.error-prone-slf4j>0.1.22</version.error-prone-slf4j>
<version.guava-beta-checker>1.0</version.guava-beta-checker>
<version.jdk>11</version.jdk>
<version.jdk>17</version.jdk>
<version.maven>3.9.5</version.maven>
<version.mockito>5.10.0</version.mockito>
<version.nopen-checker>1.0.1</version.nopen-checker>
<version.nullaway>0.10.22</version.nullaway>
<version.nullaway>0.10.23</version.nullaway>
<version.pitest-git>1.1.4</version.pitest-git>
<version.rewrite-templating>1.5.0</version.rewrite-templating>
<version.surefire>3.2.3</version.surefire>
@@ -345,7 +345,7 @@
<dependency>
<groupId>com.google.truth</groupId>
<artifactId>truth</artifactId>
<version>1.4.0</version>
<version>1.4.1</version>
</dependency>
<dependency>
<groupId>com.jakewharton.nopen</groupId>
@@ -360,7 +360,7 @@
<dependency>
<groupId>io.projectreactor</groupId>
<artifactId>reactor-bom</artifactId>
<version>2023.0.2</version>
<version>2023.0.3</version>
<type>pom</type>
<scope>import</scope>
</dependency>
@@ -407,7 +407,7 @@
<dependency>
<groupId>net.bytebuddy</groupId>
<artifactId>byte-buddy</artifactId>
<version>1.14.11</version>
<version>1.14.12</version>
</dependency>
<!-- Specified so that Renovate will file Maven upgrade PRs, which
subsequently will cause `maven-enforcer-plugin` to require that
@@ -420,7 +420,7 @@
<dependency>
<groupId>org.aspectj</groupId>
<artifactId>aspectjweaver</artifactId>
<version>1.9.21</version>
<version>1.9.21.1</version>
</dependency>
<dependency>
<groupId>org.assertj</groupId>
@@ -442,7 +442,7 @@
<dependency>
<groupId>org.immutables</groupId>
<artifactId>value-annotations</artifactId>
<version>2.10.0</version>
<version>2.10.1</version>
</dependency>
<dependency>
<groupId>org.jspecify</groupId>
@@ -467,14 +467,6 @@
<groupId>org.mongodb</groupId>
<artifactId>mongodb-driver-core</artifactId>
<version>4.11.1</version>
<!-- XXX: Drop this exclusion once we forgo enforcement of JDK
11 bytecode version compatibility. -->
<exclusions>
<exclusion>
<groupId>org.mongodb</groupId>
<artifactId>bson-record-codec</artifactId>
</exclusion>
</exclusions>
</dependency>
<dependency>
<groupId>org.openrewrite</groupId>
@@ -498,19 +490,19 @@
<dependency>
<groupId>org.springframework</groupId>
<artifactId>spring-framework-bom</artifactId>
<version>5.3.31</version>
<version>6.1.4</version>
<type>pom</type>
<scope>import</scope>
</dependency>
<dependency>
<groupId>org.springframework.boot</groupId>
<artifactId>spring-boot-test</artifactId>
<version>2.7.18</version>
<version>3.2.2</version>
</dependency>
<dependency>
<groupId>org.springframework.security</groupId>
<artifactId>spring-security-bom</artifactId>
<version>5.8.9</version>
<version>6.2.2</version>
<type>pom</type>
<scope>import</scope>
</dependency>
@@ -1420,7 +1412,7 @@
<plugin>
<groupId>org.pitest</groupId>
<artifactId>pitest-maven</artifactId>
<version>1.15.7</version>
<version>1.15.8</version>
<configuration>
<excludedClasses>
<!-- AutoValue generated classes. -->
@@ -2016,11 +2008,6 @@
</profile>
<profile>
<id>sonar</id>
<activation>
<property>
<name>sonar.projectKey</name>
</property>
</activation>
<properties>
<maven.test.failure.ignore>true</maven.test.failure.ignore>
</properties>

View File

@@ -5,7 +5,7 @@
<parent>
<groupId>tech.picnic.error-prone-support</groupId>
<artifactId>error-prone-support</artifactId>
<version>0.14.1-SNAPSHOT</version>
<version>0.15.1-SNAPSHOT</version>
</parent>
<artifactId>refaster-compiler</artifactId>

View File

@@ -25,4 +25,9 @@ public final class RefasterRuleCompiler implements Plugin {
javacTask.addTaskListener(
new RefasterRuleCompilerTaskListener(((BasicJavacTask) javacTask).getContext()));
}
@Override
public boolean autoStart() {
return true;
}
}

View File

@@ -5,7 +5,7 @@
<parent>
<groupId>tech.picnic.error-prone-support</groupId>
<artifactId>error-prone-support</artifactId>
<version>0.14.1-SNAPSHOT</version>
<version>0.15.1-SNAPSHOT</version>
</parent>
<artifactId>refaster-runner</artifactId>
@@ -124,9 +124,6 @@
<version>${project.version}</version>
</path>
</annotationProcessorPaths>
<compilerArgs combine.children="append">
<arg>-Xplugin:RefasterRuleCompiler</arg>
</compilerArgs>
</configuration>
</plugin>
</plugins>

View File

@@ -136,16 +136,13 @@ public final class Refaster extends BugChecker implements CompilationUnitTreeMat
}
private static Optional<SeverityLevel> 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));
};
}
/**

View File

@@ -191,17 +191,14 @@ final class RefasterTest {
}
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

View File

@@ -5,7 +5,7 @@
<parent>
<groupId>tech.picnic.error-prone-support</groupId>
<artifactId>error-prone-support</artifactId>
<version>0.14.1-SNAPSHOT</version>
<version>0.15.1-SNAPSHOT</version>
</parent>
<artifactId>refaster-support</artifactId>

View File

@@ -0,0 +1,37 @@
package tech.picnic.errorprone.refaster.annotation;
import java.lang.annotation.ElementType;
import java.lang.annotation.Retention;
import java.lang.annotation.RetentionPolicy;
import java.lang.annotation.Target;
/**
* Indicates that a Refaster rule or group of Refaster rules is intended to migrate away from the
* indicated type.
*/
// XXX: Add support for `#unmigratedFields()`.
// XXX: Consider making this annotation `@Repeatable`, for cases where a single Refaster rule
// collection migrates away from multiple types.
@Retention(RetentionPolicy.SOURCE)
@Target(ElementType.TYPE)
public @interface TypeMigration {
/**
* The type migrated away from.
*
* @return The type generally used in the {@link
* com.google.errorprone.refaster.annotation.BeforeTemplate} methods of annotated Refaster
* rule(s).
*/
Class<?> of();
/**
* The signatures of public methods and constructors that are not (yet) migrated by the annotated
* Refaster rule(s).
*
* @return A possibly empty enumeration of method and constructor signatures, formatted according
* to {@link
* com.google.errorprone.util.Signatures#prettyMethodSignature(com.sun.tools.javac.code.Symbol.ClassSymbol,
* com.sun.tools.javac.code.Symbol.MethodSymbol)}.
*/
String[] unmigratedMethods() default {};
}

View File

@@ -140,11 +140,10 @@ public final class IsEmpty implements Matcher<ExpressionTree> {
}
private static boolean isEmptyArrayCreation(ExpressionTree tree) {
if (!(tree instanceof NewArrayTree)) {
if (!(tree instanceof NewArrayTree newArray)) {
return false;
}
NewArrayTree newArray = (NewArrayTree) tree;
return (!newArray.getDimensions().isEmpty()
&& ZERO.equals(ASTHelpers.constValue(newArray.getDimensions().get(0), Integer.class)))
|| (newArray.getInitializers() != null && newArray.getInitializers().isEmpty());

View File

@@ -23,7 +23,7 @@ public final class IsLikelyTrivialComputation implements Matcher<ExpressionTree>
@Override
public boolean matches(ExpressionTree expressionTree, VisitorState state) {
if (expressionTree instanceof MethodInvocationTree) {
if (expressionTree instanceof MethodInvocationTree methodInvocation) {
// XXX: Method invocations are generally *not* trivial computations, but we make an exception
// for nullary method invocations on the result of a trivial computation. This exception
// allows this `Matcher` to by the `OptionalOrElseGet` Refaster rule, such that it does not
@@ -31,7 +31,6 @@ public final class IsLikelyTrivialComputation implements Matcher<ExpressionTree>
// references. Once the `MethodReferenceUsage` bug checker is production-ready, this exception
// should be removed. (But at that point, instead defining a `RequiresComputation` matcher may
// be more appropriate.)
MethodInvocationTree methodInvocation = (MethodInvocationTree) expressionTree;
if (methodInvocation.getArguments().isEmpty()
&& matches(methodInvocation.getMethodSelect())) {
return true;
@@ -44,9 +43,8 @@ public final class IsLikelyTrivialComputation implements Matcher<ExpressionTree>
// XXX: Some `BinaryTree`s may represent what could be considered "trivial computations".
// Depending on feedback such trees may be matched in the future.
private static boolean matches(ExpressionTree expressionTree) {
if (expressionTree instanceof ArrayAccessTree) {
return matches(((ArrayAccessTree) expressionTree).getExpression())
&& matches(((ArrayAccessTree) expressionTree).getIndex());
if (expressionTree instanceof ArrayAccessTree arrayAccess) {
return matches(arrayAccess.getExpression()) && matches(arrayAccess.getIndex());
}
if (expressionTree instanceof LiteralTree) {
@@ -65,26 +63,26 @@ public final class IsLikelyTrivialComputation implements Matcher<ExpressionTree>
return true;
}
if (expressionTree instanceof MemberReferenceTree) {
return matches(((MemberReferenceTree) expressionTree).getQualifierExpression());
if (expressionTree instanceof MemberReferenceTree memberReference) {
return matches(memberReference.getQualifierExpression());
}
if (expressionTree instanceof MemberSelectTree) {
return matches(((MemberSelectTree) expressionTree).getExpression());
if (expressionTree instanceof MemberSelectTree memberSelect) {
return matches(memberSelect.getExpression());
}
if (expressionTree instanceof ParenthesizedTree) {
return matches(((ParenthesizedTree) expressionTree).getExpression());
if (expressionTree instanceof ParenthesizedTree parenthesized) {
return matches(parenthesized.getExpression());
}
if (expressionTree instanceof TypeCastTree) {
return matches(((TypeCastTree) expressionTree).getExpression());
if (expressionTree instanceof TypeCastTree typeCast) {
return matches(typeCast.getExpression());
}
if (expressionTree instanceof UnaryTree) {
if (expressionTree instanceof UnaryTree unary) {
// XXX: Arguably side-effectful options such as pre- and post-increment and -decrement are not
// trivial.
return matches(((UnaryTree) expressionTree).getExpression());
return matches(unary.getExpression());
}
return false;

View File

@@ -1,42 +0,0 @@
package tech.picnic.errorprone.refaster.matchers;
import static com.google.errorprone.matchers.Matchers.anyOf;
import static com.google.errorprone.matchers.Matchers.kindIs;
import static com.google.errorprone.matchers.Matchers.parentNode;
import com.google.errorprone.VisitorState;
import com.google.errorprone.matchers.Matcher;
import com.google.errorprone.util.ASTHelpers;
import com.sun.source.tree.ExpressionTree;
import com.sun.source.tree.LambdaExpressionTree;
import com.sun.source.tree.Tree;
import javax.lang.model.type.TypeKind;
/**
* A matcher of expressions of which the result (if any) is unused, for use with Refaster's
* {@code @Matches} annotation.
*/
// XXX: Review whether other parts of Error Prone's `AbstractReturnValueIgnored` should be ported to
// this class.
public final class ReturnValueUnused implements Matcher<ExpressionTree> {
private static final long serialVersionUID = 1L;
private static final Matcher<ExpressionTree> DELEGATE =
parentNode(
anyOf(
ReturnValueUnused::isVoidReturningLambdaExpression,
kindIs(Tree.Kind.EXPRESSION_STATEMENT)));
/** Instantiates a new {@link ReturnValueUnused} instance. */
public ReturnValueUnused() {}
@Override
public boolean matches(ExpressionTree tree, VisitorState state) {
return DELEGATE.matches(tree, state);
}
private static boolean isVoidReturningLambdaExpression(Tree tree, VisitorState state) {
return tree instanceof LambdaExpressionTree
&& state.getTypes().findDescriptorType(ASTHelpers.getType(tree)).getReturnType().getKind()
== TypeKind.VOID;
}
}

View File

@@ -23,12 +23,12 @@ public final class ThrowsCheckedException implements Matcher<ExpressionTree> {
@Override
public boolean matches(ExpressionTree tree, VisitorState state) {
if (tree instanceof LambdaExpressionTree) {
return throwsCheckedException((LambdaExpressionTree) tree, state);
if (tree instanceof LambdaExpressionTree lambdaExpression) {
return throwsCheckedException(lambdaExpression, state);
}
if (tree instanceof MemberReferenceTree) {
return throwsCheckedException((MemberReferenceTree) tree, state);
if (tree instanceof MemberReferenceTree memberReference) {
return throwsCheckedException(memberReference, state);
}
Type type = ASTHelpers.getType(tree);

View File

@@ -37,11 +37,9 @@ abstract class AbstractMatcherTestChecker extends BugChecker implements Compilat
new TreePathScanner<@Nullable Void, @Nullable Void>() {
@Override
public @Nullable Void scan(@Nullable Tree tree, @Nullable Void unused) {
if (tree instanceof ExpressionTree) {
if (tree instanceof ExpressionTree expressionTree) {
TreePath path = new TreePath(getCurrentPath(), tree);
ExpressionTree expressionTree = (ExpressionTree) tree;
if (!isMethodSelect(expressionTree, path)
&& state.getSourceForNode(expressionTree) != null
&& delegate.matches(expressionTree, state.withPath(path))) {
state.reportMatch(describeMatch(tree));
}
@@ -95,7 +93,7 @@ abstract class AbstractMatcherTestChecker extends BugChecker implements Compilat
}
Tree parentTree = parentPath.getLeaf();
return parentTree instanceof MethodInvocationTree
&& ((MethodInvocationTree) parentTree).getMethodSelect().equals(tree);
return parentTree instanceof MethodInvocationTree methodInvocation
&& methodInvocation.getMethodSelect().equals(tree);
}
}

View File

@@ -1,81 +0,0 @@
package tech.picnic.errorprone.refaster.matchers;
import static com.google.errorprone.BugPattern.SeverityLevel.ERROR;
import com.google.errorprone.BugPattern;
import com.google.errorprone.CompilationTestHelper;
import com.google.errorprone.bugpatterns.BugChecker;
import org.junit.jupiter.api.Test;
final class ReturnValueUnusedTest {
@Test
void matches() {
CompilationTestHelper.newInstance(MatcherTestChecker.class, getClass())
.addSourceLines(
"A.java",
"import java.util.function.Consumer;",
"",
"class A {",
" String negative1() {",
" return toString();",
" }",
"",
" void negative2() {",
" String s = toString();",
" }",
"",
" String negative3() {",
" return toString().toString();",
" }",
"",
// XXX: The `valueOf` result is ignored, but `String::valueOf` itself isn't. Review.
" Object negative4() {",
" return sink(String::valueOf);",
" }",
"",
" void positive1() {",
" // BUG: Diagnostic contains:",
" toString();",
" }",
"",
" void positive2() {",
" // BUG: Diagnostic contains:",
" toString().toString();",
" }",
"",
" void positive3() {",
" // BUG: Diagnostic contains:",
" new Object();",
" }",
"",
" Object positive4() {",
" // BUG: Diagnostic contains:",
" return sink(v -> toString());",
" }",
"",
" Object positive5() {",
" // BUG: Diagnostic contains:",
" return sink(v -> new Object());",
" }",
"",
" private <T, S> S sink(Consumer<T> consumer) {",
" return null;",
" }",
"}")
.doTest();
}
/** A {@link BugChecker} that simply delegates to {@link ReturnValueUnused}. */
@BugPattern(summary = "Flags expressions matched by `ReturnValueUnused`", severity = ERROR)
@SuppressWarnings({"RedundantModifier", "serial"})
public static final class MatcherTestChecker extends AbstractMatcherTestChecker {
private static final long serialVersionUID = 1L;
// XXX: This is a false positive reported by Checkstyle. See
// https://github.com/checkstyle/checkstyle/issues/10161#issuecomment-1242732120.
@SuppressWarnings("RedundantModifier")
public MatcherTestChecker() {
super(new ReturnValueUnused());
}
}
}

View File

@@ -5,7 +5,7 @@
<parent>
<groupId>tech.picnic.error-prone-support</groupId>
<artifactId>error-prone-support</artifactId>
<version>0.14.1-SNAPSHOT</version>
<version>0.15.1-SNAPSHOT</version>
</parent>
<artifactId>refaster-test-support</artifactId>
@@ -106,9 +106,6 @@
<version>${project.version}</version>
</path>
</annotationProcessorPaths>
<compilerArgs combine.children="append">
<arg>-Xplugin:RefasterRuleCompiler</arg>
</compilerArgs>
</configuration>
</plugin>
</plugins>

View File

@@ -4,6 +4,7 @@ import static com.google.common.base.Preconditions.checkState;
import static com.google.common.collect.ImmutableListMultimap.toImmutableListMultimap;
import static com.google.common.collect.ImmutableSet.toImmutableSet;
import static com.google.common.collect.ImmutableSortedSet.toImmutableSortedSet;
import static com.google.errorprone.BugPattern.LinkType.NONE;
import static com.google.errorprone.BugPattern.SeverityLevel.ERROR;
import static java.util.Comparator.naturalOrder;
import static tech.picnic.errorprone.refaster.runner.Refaster.INCLUDED_RULES_PATTERN_FLAG;
@@ -60,7 +61,7 @@ import tech.picnic.errorprone.refaster.runner.Refaster;
// XXX: This check currently only validates that one `Refaster.anyOf` branch in one
// `@BeforeTemplate` method is covered by a test. Review how we can make sure that _all_
// `@BeforeTemplate` methods and `Refaster.anyOf` branches are covered.
@BugPattern(summary = "Exercises a Refaster rule collection", severity = ERROR)
@BugPattern(summary = "Exercises a Refaster rule collection", linkType = NONE, severity = ERROR)
@SuppressWarnings("java:S2160" /* Super class equality definition suffices. */)
public final class RefasterRuleCollection extends BugChecker implements CompilationUnitTreeMatcher {
private static final long serialVersionUID = 1L;
@@ -154,8 +155,8 @@ public final class RefasterRuleCollection extends BugChecker implements Compilat
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,
@@ -276,9 +277,10 @@ public final class RefasterRuleCollection extends BugChecker implements Compilat
unexpectedMatchesByLineNumber.entries().stream()
.map(
e ->
String.format(
"Rule `%s` matches on line %s, while it should match in a method named `test%s`.",
e.getValue(), e.getKey(), e.getValue()))
"""
Rule `%s` matches on line %s, while it should match in a method named \
`test%s`."""
.formatted(e.getValue(), e.getKey(), e.getValue()))
.collect(toImmutableSet()),
state);
}