Compare commits

..

15 Commits

Author SHA1 Message Date
Stephan Schroevers
8ced020ba6 Post rebase fixes 2022-11-05 17:21:46 +01:00
Stephan Schroevers
389928ce43 Tweaks 2022-11-05 16:53:45 +01:00
Rick Ossendrijver
d88f226b30 Remove the TEXT_MATCH param from doTest 2022-11-05 16:52:02 +01:00
Rick Ossendrijver
ba91c6bed7 Suggestions 2022-11-05 16:52:02 +01:00
Nathan Kooij
85f402b089 Simplify by only working with longs 2022-11-05 16:52:02 +01:00
Nathan Kooij
ad8c0a472c Fix most warnings 2022-11-05 16:52:02 +01:00
Nathan Kooij
0af127652e Use a sorted set to derive the ceiling 2022-11-05 16:52:02 +01:00
Nathan Kooij
cede5e451b Assorted cleanup 2022-11-05 16:52:02 +01:00
Nathan Kooij
134895090f Support banned fields 2022-11-05 16:52:02 +01:00
Nathan Kooij
226bfd0cee Handle implicit value attribute case 2022-11-05 16:52:02 +01:00
Nathan Kooij
89a3c605fe Support multiple attributes per annotation 2022-11-05 16:52:02 +01:00
Nathan Kooij
63273a2609 Some cleanup of simplifying code 2022-11-05 16:52:02 +01:00
Nathan Kooij
79768a2428 Notes and random things 2022-11-05 16:52:02 +01:00
Nathan Kooij
dd8d094b5a Suggest a fix 2022-11-05 16:52:02 +01:00
Nathan Kooij
4830b5b2cd Extract TimeUnit, Number value, and determine a simplification 2022-11-05 16:52:02 +01:00
331 changed files with 2580 additions and 209922 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.3`).
- Error Prone version (e.g. `2.15.0`).
- Error Prone Support version (e.g. `0.3.0`).
### Additional context

View File

@@ -17,24 +17,23 @@ you'd like to be solved through Error Prone Support. -->
- [ ] Support a stylistic preference.
- [ ] Avoid a common gotcha, or potential problem.
- [ ] Improve performance.
<!--
Here, provide a clear and concise description of the desired change.
If possible, provide a simple and minimal example using the following format:
I would like to rewrite the following code:
```java
// XXX: Write the code to match here.
```
to:
```java
// XXX: Write the desired code here.
```
-->
I would like to rewrite the following code:
```java
// XXX: Write the code to match here.
```
to:
```java
// XXX: Write the desired code here.
```
### Considerations
<!--

View File

@@ -10,32 +10,34 @@ jobs:
strategy:
matrix:
os: [ ubuntu-22.04 ]
jdk: [ 11.0.20, 17.0.8, 20.0.2 ]
jdk: [ 11.0.16, 17.0.4, 19 ]
distribution: [ temurin ]
experimental: [ false ]
include:
- os: macos-12
jdk: 17.0.8
jdk: 17.0.4
distribution: temurin
experimental: false
- os: windows-2022
jdk: 17.0.8
jdk: 17.0.4
distribution: temurin
experimental: false
- os: ubuntu-22.04
jdk: 20-ea
distribution: zulu
experimental: true
runs-on: ${{ matrix.os }}
continue-on-error: ${{ matrix.experimental }}
steps:
# We run the build twice for each supported JDK: once against the
# original Error Prone release, using only Error Prone checks available
# on Maven Central, and once against the Picnic Error Prone fork,
# additionally enabling all checks defined in this project and any Error
# Prone checks available only from other artifact repositories.
# We run the build twice for each supported JDK: once against the
# original Error Prone release, using only Error Prone checks available
# on Maven Central, and once against the Picnic Error Prone fork,
# additionally enabling all checks defined in this project and any
# Error Prone checks available only from other artifact repositories.
- name: Check out code
uses: actions/checkout@8ade135a41bc03ea155e62e844d188df1ea18608 # v4.1.0
with:
persist-credentials: false
uses: actions/checkout@v3.1.0
- name: Set up JDK
uses: actions/setup-java@0ab4596768b603586c0de567f2430c30f5b0d2b0 # v3.13.0
uses: actions/setup-java@v3.6.0
with:
java-version: ${{ matrix.jdk }}
distribution: ${{ matrix.distribution }}
@@ -50,3 +52,4 @@ jobs:
run: mvn build-helper:remove-project-artifact
# XXX: Enable Codecov once we "go public".
# XXX: Enable SonarCloud once we "go public".

View File

@@ -1,44 +0,0 @@
# Analyzes the code using GitHub's default CodeQL query database.
# Identified issues are registered with GitHub's code scanning dashboard. When
# a pull request is analyzed, any offending lines are annotated. See
# https://codeql.github.com for details.
name: CodeQL analysis
on:
pull_request:
push:
branches: [ master ]
schedule:
- cron: '0 4 * * 1'
permissions:
contents: read
jobs:
analyze:
strategy:
matrix:
language: [ java, ruby ]
permissions:
contents: read
security-events: write
runs-on: ubuntu-22.04
steps:
- name: Check out code
uses: actions/checkout@8ade135a41bc03ea155e62e844d188df1ea18608 # v4.1.0
with:
persist-credentials: false
- name: Set up JDK
uses: actions/setup-java@0ab4596768b603586c0de567f2430c30f5b0d2b0 # v3.13.0
with:
java-version: 17.0.8
distribution: temurin
cache: maven
- name: Initialize CodeQL
uses: github/codeql-action/init@2cb752a87e96af96708ab57187ab6372ee1973ab # v2.22.0
with:
languages: ${{ matrix.language }}
- name: Perform minimal build
if: matrix.language == 'java'
run: mvn -T1C clean install -DskipTests -Dverification.skip
- name: Perform CodeQL analysis
uses: github/codeql-action/analyze@2cb752a87e96af96708ab57187ab6372ee1973ab # v2.22.0
with:
category: /language:${{ matrix.language }}

View File

@@ -3,24 +3,22 @@ on:
pull_request:
push:
branches: [ master, website ]
permissions:
contents: read
concurrency:
group: ${{ github.workflow }}-${{ github.ref }}
jobs:
build:
permissions:
contents: read
runs-on: ubuntu-22.04
steps:
- name: Check out code
uses: actions/checkout@8ade135a41bc03ea155e62e844d188df1ea18608 # v4.1.0
with:
persist-credentials: false
- uses: ruby/setup-ruby@d37167af451eb51448db3354e1057b75c4b268f7 # v1.155.0
uses: actions/checkout@v3.1.0
- uses: ruby/setup-ruby@v1.120.0
with:
working-directory: ./website
bundler-cache: true
- name: Configure Github Pages
uses: actions/configure-pages@f156874f8191504dae5b037505266ed5dda6c382 # v3.0.6
uses: actions/configure-pages@v2.1.2
- name: Generate documentation
run: ./generate-docs.sh
- name: Build website with Jekyll
@@ -32,7 +30,7 @@ jobs:
# "Refaster rules" terminology on our website and in the code.
run: bundle exec htmlproofer --disable_external true --check-external-hash false ./_site
- name: Upload website as artifact
uses: actions/upload-pages-artifact@a753861a5debcf57bf8b404356158c8e1e33150c # v2.0.0
uses: actions/upload-pages-artifact@v1.0.4
with:
path: ./website/_site
deploy:
@@ -48,4 +46,4 @@ jobs:
steps:
- name: Deploy to GitHub Pages
id: deployment
uses: actions/deploy-pages@9dbe3824824f8a1377b8e298bafde1a50ede43e5 # v2.0.4
uses: actions/deploy-pages@v1.2.2

View File

@@ -1,36 +0,0 @@
name: On-demand integration test
on:
# run action every time a new comment is created
issue_comment:
types: [ created ]
permissions:
contents: read
jobs:
on_demand_integration_test:
name: On-demand integration test
# Only run the integration test if the created comment is on a pull request
# and contains the string `/integration-test`
if: |
github.event.issue.pull_request &&
contains(github.event.comment.body, '/integration-test')
# XXX TODO: Device matrix?
runs-on: ubuntu-latest
steps:
- name: Check out code
uses: actions/checkout@8ade135a41bc03ea155e62e844d188df1ea18608 # v4.1
with:
submodules: 'recursive'
persist-credentials: false
- name: Set up JDK
uses: actions/setup-java@0ab4596768b603586c0de567f2430c30f5b0d2b0 # v3.13.0
with:
# XXX TODO: java version matrix?
java-version: 17.0.8
distribution: temurin
cache: maven
- name: Display build environment details
run: mvn --version
- name: Install error-prone-support snapshot
run: mvn -T1C install -DskipTests -Dverification.skip
- name: Run integration test
run: echo "running integration test"

View File

@@ -1,36 +0,0 @@
# Analyzes the code base and GitHub project configuration for adherence to
# security best practices for open source software. Identified issues are
# registered with GitHub's code scanning dashboard. When a pull request is
# analyzed, any offending lines are annotated. See
# https://securityscorecards.dev for details.
name: OpenSSF Scorecard update
on:
pull_request:
push:
branches: [ master ]
schedule:
- cron: '0 4 * * 1'
permissions:
contents: read
jobs:
analyze:
permissions:
contents: read
security-events: write
id-token: write
runs-on: ubuntu-22.04
steps:
- name: Check out code
uses: actions/checkout@8ade135a41bc03ea155e62e844d188df1ea18608 # v4.1.0
with:
persist-credentials: false
- name: Run OpenSSF Scorecard analysis
uses: ossf/scorecard-action@483ef80eb98fb506c348f7d62e28055e49fe2398 # v2.3.0
with:
results_file: results.sarif
results_format: sarif
publish_results: ${{ github.ref == 'refs/heads/master' }}
- name: Update GitHub's code scanning dashboard
uses: github/codeql-action/upload-sarif@2cb752a87e96af96708ab57187ab6372ee1973ab # v2.22.0
with:
sarif_file: results.sarif

View File

@@ -1,38 +0,0 @@
# Performs mutation testing analysis on the files changed by a pull request and
# uploads the results. The associated PR is subsequently updated by the
# `pitest-update-pr.yml` workflow. See https://blog.pitest.org/oss-pitest-pr/
# for details.
name: "Mutation testing"
on:
pull_request:
permissions:
contents: read
jobs:
analyze-pr:
runs-on: ubuntu-22.04
steps:
- name: Check out code
uses: actions/checkout@8ade135a41bc03ea155e62e844d188df1ea18608 # v4.1.0
with:
fetch-depth: 2
persist-credentials: false
- name: Set up JDK
uses: actions/setup-java@0ab4596768b603586c0de567f2430c30f5b0d2b0 # v3.13.0
with:
java-version: 17.0.8
distribution: temurin
cache: maven
- name: Run Pitest
# By running with features `+GIT(from[HEAD~1]), +gitci`, Pitest only
# analyzes lines changed in the associated pull request, as GitHub
# exposes the changes unique to the PR as a single commit on top of the
# target branch. See https://blog.pitest.org/pitest-pr-setup for
# details.
run: mvn test pitest:mutationCoverage -DargLine.xmx=2048m -Dverification.skip -Dfeatures="+GIT(from[HEAD~1]), +gitci"
- name: Aggregate Pitest reports
run: mvn pitest-git:aggregate -DkilledEmoji=":tada:" -DmutantEmoji=":zombie:" -DtrailingText="Mutation testing report by [Pitest](https://pitest.org/). Review any surviving mutants by inspecting the line comments under [_Files changed_](${{ github.event.number }}/files)."
- name: Upload Pitest reports as artifact
uses: actions/upload-artifact@a8a3f3ad30e3422c9c7b888a15615d19a852ae32 # v3.1.3
with:
name: pitest-reports
path: ./target/pit-reports-ci

View File

@@ -1,39 +0,0 @@
# Updates a pull request based on the corresponding mutation testing analysis
# performed by the `pitest-analyze-pr.yml` workflow. See
# https://blog.pitest.org/oss-pitest-pr/ for details.
name: "Mutation testing: post results"
on:
workflow_run:
workflows: ["Mutation testing"]
types:
- completed
permissions:
actions: read
jobs:
update-pr:
if: ${{ github.event.workflow_run.conclusion == 'success' }}
permissions:
actions: read
checks: write
contents: read
pull-requests: write
runs-on: ubuntu-22.04
steps:
- name: Check out code
uses: actions/checkout@8ade135a41bc03ea155e62e844d188df1ea18608 # v4.1.0
with:
persist-credentials: false
- name: Set up JDK
uses: actions/setup-java@0ab4596768b603586c0de567f2430c30f5b0d2b0 # v3.13.0
with:
java-version: 17.0.8
distribution: temurin
cache: maven
- name: Download Pitest analysis artifact
uses: dawidd6/action-download-artifact@268677152d06ba59fcec7a7f0b5d961b6ccd7e1e # v2.28.0
with:
workflow: ${{ github.event.workflow_run.workflow_id }}
name: pitest-reports
path: ./target/pit-reports-ci
- name: Update PR
run: mvn -DrepoToken="${{ secrets.GITHUB_TOKEN }}" pitest-github:updatePR

View File

@@ -1,36 +0,0 @@
# Analyzes the code base using SonarCloud. See
# https://sonarcloud.io/project/overview?id=PicnicSupermarket_error-prone-support.
name: SonarCloud analysis
on:
pull_request:
push:
branches: [ master ]
schedule:
- cron: '0 4 * * 1'
permissions:
contents: read
jobs:
analyze:
permissions:
contents: read
runs-on: ubuntu-22.04
steps:
- name: Check out code
uses: actions/checkout@8ade135a41bc03ea155e62e844d188df1ea18608 # v4.1.0
with:
fetch-depth: 0
persist-credentials: false
- name: Set up JDK
uses: actions/setup-java@0ab4596768b603586c0de567f2430c30f5b0d2b0 # v3.13.0
with:
java-version: 17.0.8
distribution: temurin
cache: maven
- name: Create missing `test` directory
# XXX: Drop this step in favour of actually having a test.
run: mkdir refaster-compiler/src/test
- name: Perform SonarCloud analysis
env:
GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }}
SONAR_TOKEN: ${{ secrets.SONAR_TOKEN }}
run: mvn -T1C jacoco:prepare-agent verify jacoco:report sonar:sonar -Dverification.skip -Dsonar.projectKey=PicnicSupermarket_error-prone-support

15
.gitmodules vendored
View File

@@ -1,15 +0,0 @@
[submodule "integration-tests/checkstyle"]
path = integration-tests/checkstyle
url = git@github.com:checkstyle/checkstyle.git
[submodule "integration-tests/guava"]
path = integration-tests/guava
url = https://github.com/google/guava
[submodule "integration-tests/metrics"]
path = integration-tests/metrics
url = https://github.com/dropwizard/metrics
[submodule "integration-tests/druid"]
path = integration-tests/druid
url = https://github.com/apache/druid
[submodule "integration-tests/calcite"]
path = integration-tests/calcite
url = https://github.com/apache/calcite

View File

@@ -1,3 +1 @@
--batch-mode
--errors
--strict-checksums
--batch-mode --errors --strict-checksums

View File

@@ -1,8 +1,5 @@
{
"$schema": "https://docs.renovatebot.com/renovate-schema.json",
"extends": [
"helpers:pinGitHubActionDigests"
],
"packageRules": [
{
"matchPackagePatterns": [
@@ -12,11 +9,10 @@
"separateMinorPatch": true
},
{
"matchDepNames": [
"github/codeql-action",
"ruby/setup-ruby"
"matchPackagePatterns": [
"^com\\.palantir\\.baseline:baseline-error-prone$"
],
"schedule": "every 4 weeks on Monday"
"schedule": "* * 1 * *"
}
],
"reviewers": [

View File

@@ -48,17 +48,6 @@ be accepted. When in doubt, make sure to first raise an
To the extent possible, the pull request process guards our coding guidelines.
Some pointers:
- Try to make sure that the
[`./run-full-build.sh`][error-prone-support-full-build] script completes
successfully, ideally before opening a pull request. See the [development
instructions][error-prone-support-developing] for details on how to
efficiently resolve many of the errors and warnings that may be reported. (In
particular, make sure to run `mvn fmt:format` and
[`./apply-error-prone-suggestions.sh`][error-prone-support-patch].) That
said, if you feel that the build fails for invalid or debatable reasons, or
if you're unsure how to best resolve an issue, don't let that discourage you
from opening a PR with a failing build; we can have a look at the issue
together!
- Checks should be _topical_: ideally they address a single concern.
- Where possible checks should provide _fixes_, and ideally these are
completely behavior-preserving. In order for a check to be adopted by users
@@ -77,9 +66,6 @@ Some pointers:
sneak in unrelated changes; instead just open more than one pull request 😉.
[error-prone-criteria]: https://errorprone.info/docs/criteria
[error-prone-support-developing]: https://github.com/PicnicSupermarket/error-prone-support/tree/master#-developing-error-prone-support
[error-prone-support-full-build]: https://github.com/PicnicSupermarket/error-prone-support/blob/master/run-full-build.sh
[error-prone-support-issues]: https://github.com/PicnicSupermarket/error-prone-support/issues
[error-prone-support-mutation-tests]: https://github.com/PicnicSupermarket/error-prone-support/blob/master/run-branch-mutation-tests.sh
[error-prone-support-patch]: https://github.com/PicnicSupermarket/error-prone-support/blob/master/apply-error-prone-suggestions.sh
[error-prone-support-mutation-tests]: https://github.com/PicnicSupermarket/error-prone-support/blob/master/run-mutation-tests.sh
[error-prone-support-pulls]: https://github.com/PicnicSupermarket/error-prone-support/pulls

View File

@@ -1,6 +1,6 @@
MIT License
Copyright (c) 2017-2023 Picnic Technologies BV
Copyright (c) 2017-2022 Picnic Technologies BV
Permission is hereby granted, free of charge, to any person obtaining a copy
of this software and associated documentation files (the "Software"), to deal

118
README.md
View File

@@ -21,18 +21,7 @@ code_][picnic-blog-ep-post].
[![Maven Central][maven-central-badge]][maven-central-search]
[![Reproducible Builds][reproducible-builds-badge]][reproducible-builds-report]
[![OpenSSF Best Practices][openssf-best-practices-badge]][openssf-best-practices-checklist]
[![OpenSSF Scorecard][openssf-scorecard-badge]][openssf-scorecard-report]
[![CodeQL Analysis][codeql-badge]][codeql-master]
[![GitHub Actions][github-actions-build-badge]][github-actions-build-master]
[![Mutation tested with PIT][pitest-badge]][pitest]
[![Quality Gate Status][sonarcloud-quality-badge]][sonarcloud-quality-master]
[![Maintainability Rating][sonarcloud-maintainability-badge]][sonarcloud-maintainability-master]
[![Reliability Rating][sonarcloud-reliability-badge]][sonarcloud-reliability-master]
[![Security Rating][sonarcloud-security-badge]][sonarcloud-security-master]
[![Coverage][sonarcloud-coverage-badge]][sonarcloud-coverage-master]
[![Duplicated Lines (%)][sonarcloud-duplication-badge]][sonarcloud-duplication-master]
[![Technical Debt][sonarcloud-technical-debt-badge]][sonarcloud-technical-debt-master]
[![License][license-badge]][license]
[![PRs Welcome][pr-badge]][contributing]
@@ -47,9 +36,7 @@ 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.
#### Maven
it:
1. First, follow Error Prone's [installation
guide][error-prone-installation-guide].
@@ -107,32 +94,6 @@ it, read the installation guide for Maven or Gradle below.
Prone Support. Alternatively reference this project's `self-check` profile
definition. -->
#### Gradle
1. First, follow the [installation
guide][error-prone-gradle-installation-guide] of the
`gradle-errorprone-plugin`.
2. Next, edit your `build.gradle` file to add one or more Error Prone Support
modules:
```groovy
dependencies {
// Error Prone itself.
errorprone("com.google.errorprone:error_prone_core:${errorProneVersion}")
// Error Prone Support's additional bug checkers.
errorprone("tech.picnic.error-prone-support:error-prone-contrib:${errorProneSupportVersion}")
// Error Prone Support's Refaster rules.
errorprone("tech.picnic.error-prone-support:refaster-runner:${errorProneSupportVersion}")
}
tasks.withType(JavaCompile).configureEach {
options.errorprone.disableWarningsInGeneratedCode = true
// Add other Error Prone flags here. See:
// - https://github.com/tbroyer/gradle-errorprone-plugin#configuration
// - https://errorprone.info/docs/flags
}
```
### Seeing it in action
Consider the following example code:
@@ -183,15 +144,8 @@ rules][refaster-rules].
## 👷 Developing Error Prone Support
This is a [Maven][maven] project, so running `mvn clean install` performs a
full clean build and installs the library to your local Maven repository.
Once you've made changes, the build may fail due to a warning or error emitted
by static code analysis. The flags and commands listed below allow you to
suppress or (in a large subset of cases) automatically fix such cases. Make
sure to carefully check the available options, as this can save you significant
amounts of development time!
Relevant Maven build parameters:
full clean build and installs the library to your local Maven repository. Some
relevant flags:
- `-Dverification.warn` makes the warnings and errors emitted by various
plugins and the Java compiler non-fatal, where possible.
@@ -208,28 +162,19 @@ Relevant Maven build parameters:
Pending a release of [google/error-prone#3301][error-prone-pull-3301], this
flag must currently be used in combination with `-Perror-prone-fork`.
Other highly relevant commands:
Some other commands one may find relevant:
- `mvn fmt:format` formats the code using
[`google-java-format`][google-java-format].
- [`./run-full-build.sh`][script-run-full-build] builds the project twice,
where the second pass validates compatbility with Picnic's [Error Prone
fork][error-prone-fork-repo] and compliance of the code with any rules
defined within this project. (Consider running this before [opening a pull
request][contributing-pull-request], as the PR checks also perform this
validation.)
- [`./apply-error-prone-suggestions.sh`][script-apply-error-prone-suggestions]
applies Error Prone and Error Prone Support code suggestions to this project.
Before running this command, make sure to have installed the project (`mvn
clean install`) and make sure that the current working directory does not
contain unstaged or uncommited changes.
- [`./run-branch-mutation-tests.sh`][script-run-branch-mutation-tests] uses
[Pitest][pitest] to run mutation tests against code that is modified relative
to the upstream default branch. The results can be reviewed by opening the
respective `target/pit-reports/index.html` files. One can use
[`./run-mutation-tests.sh`][script-run-mutation-tests] to run mutation tests
against _all_ code in the current working directory. For more information
check the [PIT Maven plugin][pitest-maven].
- `./run-mutation-tests.sh` runs mutation tests using [PIT][pitest]. The
results can be reviewed by opening the respective
`target/pit-reports/index.html` files. For more information check the [PIT
Maven plugin][pitest-maven].
- `./apply-error-prone-suggestions.sh` applies Error Prone and Error Prone
Support code suggestions to this project. Before running this command, make
sure to have installed the project (`mvn clean install`) and make sure that
the current working directory does not contain unstaged or uncommited
changes.
When running the project's tests in IntelliJ IDEA, you might see the following
error:
@@ -256,26 +201,17 @@ Want to report or fix a bug, suggest or add a new feature, or improve the
documentation? That's awesome! Please read our [contribution
guidelines][contributing].
### Security
If you want to report a security vulnerability, please do so through a private
channel; please see our [security policy][security] for details.
[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
[codeql-master]: https://github.com/PicnicSupermarket/error-prone-support/actions/workflows/codeql.yml?query=branch:master+event:push
[contributing]: https://github.com/PicnicSupermarket/error-prone-support/blob/master/CONTRIBUTING.md
[contributing-pull-request]: https://github.com/PicnicSupermarket/error-prone-support/blob/master/CONTRIBUTING.md#-opening-a-pull-request
[error-prone-bugchecker]: https://github.com/google/error-prone/blob/master/check_api/src/main/java/com/google/errorprone/bugpatterns/BugChecker.java
[error-prone-fork-jitpack]: https://jitpack.io/#PicnicSupermarket/error-prone
[error-prone-fork-repo]: https://github.com/PicnicSupermarket/error-prone
[error-prone-gradle-installation-guide]: https://github.com/tbroyer/gradle-errorprone-plugin
[error-prone-installation-guide]: https://errorprone.info/docs/installation#maven
[error-prone-orig-repo]: https://github.com/google/error-prone
[error-prone-pull-3301]: https://github.com/google/error-prone/pull/3301
[github-actions-build-badge]: https://github.com/PicnicSupermarket/error-prone-support/actions/workflows/build.yaml/badge.svg
[github-actions-build-master]: https://github.com/PicnicSupermarket/error-prone-support/actions/workflows/build.yaml?query=branch:master&event=push
[github-actions-build-master]: https://github.com/PicnicSupermarket/error-prone-support/actions/workflows/build.yaml?query=branch%3Amaster
[google-java-format]: https://github.com/google/google-java-format
[idea-288052]: https://youtrack.jetbrains.com/issue/IDEA-288052
[license-badge]: https://img.shields.io/github/license/PicnicSupermarket/error-prone-support
@@ -283,13 +219,8 @@ channel; please see our [security policy][security] for details.
[maven-central-badge]: https://img.shields.io/maven-central/v/tech.picnic.error-prone-support/error-prone-support?color=blue
[maven-central-search]: https://search.maven.org/artifact/tech.picnic.error-prone-support/error-prone-support
[maven]: https://maven.apache.org
[openssf-best-practices-badge]: https://bestpractices.coreinfrastructure.org/projects/7199/badge
[openssf-best-practices-checklist]: https://bestpractices.coreinfrastructure.org/projects/7199
[openssf-scorecard-badge]: https://img.shields.io/ossf-scorecard/github.com/PicnicSupermarket/error-prone-support?label=openssf%20scorecard
[openssf-scorecard-report]: https://securityscorecards.dev/viewer/?uri=github.com/PicnicSupermarket/error-prone-support
[picnic-blog-ep-post]: https://blog.picnic.nl/picnic-loves-error-prone-producing-high-quality-and-consistent-java-code-b8a566be6886
[picnic-blog]: https://blog.picnic.nl
[pitest-badge]: https://img.shields.io/badge/-Mutation%20tested%20with%20PIT-blue.svg
[picnic-blog-ep-post]: https://blog.picnic.nl/picnic-loves-error-prone-producing-high-quality-and-consistent-java-code-b8a566be6886
[pitest]: https://pitest.org
[pitest-maven]: https://pitest.org/quickstart/maven
[pr-badge]: https://img.shields.io/badge/PRs-welcome-brightgreen.svg
@@ -298,22 +229,3 @@ channel; please see our [security policy][security] for details.
[refaster-rules]: https://github.com/PicnicSupermarket/error-prone-support/blob/master/error-prone-contrib/src/main/java/tech/picnic/errorprone/refasterrules/
[reproducible-builds-badge]: https://img.shields.io/badge/Reproducible_Builds-ok-success?labelColor=1e5b96
[reproducible-builds-report]: https://github.com/jvm-repo-rebuild/reproducible-central/blob/master/content/tech/picnic/error-prone-support/error-prone-support/README.md
[script-apply-error-prone-suggestions]: https://github.com/PicnicSupermarket/error-prone-support/blob/master/apply-error-prone-suggestions.sh
[script-run-branch-mutation-tests]: https://github.com/PicnicSupermarket/error-prone-support/blob/master/run-branch-mutation-tests.sh
[script-run-full-build]: https://github.com/PicnicSupermarket/error-prone-support/blob/master/run-full-build.sh
[script-run-mutation-tests]: https://github.com/PicnicSupermarket/error-prone-support/blob/master/run-mutation-tests.sh
[security]: https://github.com/PicnicSupermarket/error-prone-support/blob/master/SECURITY.md
[sonarcloud-coverage-badge]: https://sonarcloud.io/api/project_badges/measure?project=PicnicSupermarket_error-prone-support&metric=coverage
[sonarcloud-coverage-master]: https://sonarcloud.io/component_measures?id=PicnicSupermarket_error-prone-support&metric=coverage
[sonarcloud-duplication-badge]: https://sonarcloud.io/api/project_badges/measure?project=PicnicSupermarket_error-prone-support&metric=duplicated_lines_density
[sonarcloud-duplication-master]: https://sonarcloud.io/component_measures?id=PicnicSupermarket_error-prone-support&metric=duplicated_lines_density
[sonarcloud-maintainability-badge]: https://sonarcloud.io/api/project_badges/measure?project=PicnicSupermarket_error-prone-support&metric=sqale_rating
[sonarcloud-maintainability-master]: https://sonarcloud.io/component_measures?id=PicnicSupermarket_error-prone-support&metric=sqale_rating
[sonarcloud-quality-badge]: https://sonarcloud.io/api/project_badges/measure?project=PicnicSupermarket_error-prone-support&metric=alert_status
[sonarcloud-quality-master]: https://sonarcloud.io/summary/new_code?id=PicnicSupermarket_error-prone-support
[sonarcloud-reliability-badge]: https://sonarcloud.io/api/project_badges/measure?project=PicnicSupermarket_error-prone-support&metric=reliability_rating
[sonarcloud-reliability-master]: https://sonarcloud.io/component_measures?id=PicnicSupermarket_error-prone-support&metric=reliability_rating
[sonarcloud-security-badge]: https://sonarcloud.io/api/project_badges/measure?project=PicnicSupermarket_error-prone-support&metric=security_rating
[sonarcloud-security-master]: https://sonarcloud.io/component_measures?id=PicnicSupermarket_error-prone-support&metric=security_rating
[sonarcloud-technical-debt-badge]: https://sonarcloud.io/api/project_badges/measure?project=PicnicSupermarket_error-prone-support&metric=sqale_index
[sonarcloud-technical-debt-master]: https://sonarcloud.io/component_measures?id=PicnicSupermarket_error-prone-support&metric=sqale_index

View File

@@ -1,23 +0,0 @@
# Security policy
We take security seriously. We are mindful of Error Prone Support's place in
the software supply chain, and the risks and responsibilities that come with
this.
## Supported versions
This project uses [semantic versioning][semantic-versioning]. In general, only
the latest version of this software is supported. That said, if users have a
compelling reason to ask for patch release of an older major release, then we
will seriously consider such a request. We do urge users to stay up-to-date and
use the latest release where feasible.
## Reporting a vulnerability
To report a vulnerability, please visit the [security
advisories][security-advisories] page and click _Report a vulnerability_. We
will take such reports seriously and work with you to resolve the issue in a
timely manner.
[security-advisories]: https://github.com/PicnicSupermarket/error-prone-support/security/advisories
[semantic-versioning]: https://semver.org

View File

@@ -9,14 +9,13 @@
set -e -u -o pipefail
if [ "${#}" -gt 1 ]; then
echo "Usage: ${0} [PatchChecks]"
echo "Usage: ./$(basename "${0}") [PatchChecks]"
exit 1
fi
patchChecks=${1:-}
mvn clean test-compile fmt:format \
-s "$(dirname "${0}")/settings.xml" \
-T 1.0C \
-Perror-prone \
-Perror-prone-fork \

View File

@@ -1,7 +0,0 @@
# Arcmutate license for Error Prone Support, requested by sending an email to
# support@arcmutate.com.
expires=07/11/2023
keyVersion=1
signature=MhZxMbnO6UovNfllM0JuVWkZyvRT3/G5o/uT0Mm36c7200VpZNVu03gTAGivnl9W5RzvZhfpIHccuQ5ctjQkrqhsFSrl4fyqPqu3y5V2fsHIdFXP/G72EGj6Kay9ndLpaEHalqE0bEwxdnHMzEYq5y3O9vUPv8MhUl57xk+rvBo\=
packages=tech.picnic.errorprone.*
type=OSSS

View File

@@ -1,87 +0,0 @@
<?xml version="1.0" encoding="UTF-8"?>
<project xmlns="http://maven.apache.org/POM/4.0.0" xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" xsi:schemaLocation="http://maven.apache.org/POM/4.0.0 https://maven.apache.org/xsd/maven-4.0.0.xsd">
<modelVersion>4.0.0</modelVersion>
<parent>
<groupId>tech.picnic.error-prone-support</groupId>
<artifactId>error-prone-support</artifactId>
<version>0.14.1-SNAPSHOT</version>
</parent>
<artifactId>documentation-support</artifactId>
<name>Picnic :: Error Prone Support :: Documentation Support</name>
<description>Data extraction support for the purpose of documentation generation.</description>
<dependencies>
<dependency>
<groupId>${groupId.error-prone}</groupId>
<artifactId>error_prone_annotation</artifactId>
</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>
</dependency>
<dependency>
<groupId>${groupId.error-prone}</groupId>
<artifactId>error_prone_test_helpers</artifactId>
<scope>test</scope>
</dependency>
<dependency>
<groupId>com.fasterxml.jackson.core</groupId>
<artifactId>jackson-annotations</artifactId>
</dependency>
<dependency>
<groupId>com.fasterxml.jackson.core</groupId>
<artifactId>jackson-databind</artifactId>
</dependency>
<dependency>
<groupId>com.google.auto</groupId>
<artifactId>auto-common</artifactId>
</dependency>
<dependency>
<groupId>com.google.auto.service</groupId>
<artifactId>auto-service-annotations</artifactId>
<scope>provided</scope>
</dependency>
<dependency>
<groupId>com.google.auto.value</groupId>
<artifactId>auto-value-annotations</artifactId>
<scope>provided</scope>
</dependency>
<dependency>
<groupId>com.google.guava</groupId>
<artifactId>guava</artifactId>
</dependency>
<dependency>
<groupId>org.assertj</groupId>
<artifactId>assertj-core</artifactId>
<scope>test</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>
<scope>test</scope>
</dependency>
<dependency>
<groupId>org.junit.jupiter</groupId>
<artifactId>junit-jupiter-engine</artifactId>
<scope>test</scope>
</dependency>
<dependency>
<groupId>org.junit.jupiter</groupId>
<artifactId>junit-jupiter-params</artifactId>
<scope>test</scope>
</dependency>
</dependencies>
</project>

View File

@@ -1,103 +0,0 @@
package tech.picnic.errorprone.documentation;
import static com.google.common.base.Verify.verify;
import static com.google.common.collect.ImmutableList.toImmutableList;
import static java.util.Objects.requireNonNull;
import com.google.auto.common.AnnotationMirrors;
import com.google.auto.value.AutoValue;
import com.google.common.collect.ImmutableList;
import com.google.errorprone.BugPattern;
import com.google.errorprone.BugPattern.SeverityLevel;
import com.google.errorprone.annotations.Immutable;
import com.google.errorprone.util.ASTHelpers;
import com.sun.source.tree.AnnotationTree;
import com.sun.source.tree.ClassTree;
import com.sun.tools.javac.code.Attribute;
import com.sun.tools.javac.code.Symbol.ClassSymbol;
import com.sun.tools.javac.util.Context;
import javax.lang.model.element.AnnotationValue;
import tech.picnic.errorprone.documentation.BugPatternExtractor.BugPatternDocumentation;
/**
* An {@link Extractor} that describes how to extract data from a {@code @BugPattern} annotation.
*/
@Immutable
final class BugPatternExtractor implements Extractor<BugPatternDocumentation> {
@Override
public BugPatternDocumentation extract(ClassTree tree, Context context) {
ClassSymbol symbol = ASTHelpers.getSymbol(tree);
BugPattern annotation = symbol.getAnnotation(BugPattern.class);
requireNonNull(annotation, "BugPattern annotation must be present");
return new AutoValue_BugPatternExtractor_BugPatternDocumentation(
symbol.getQualifiedName().toString(),
annotation.name().isEmpty() ? tree.getSimpleName().toString() : annotation.name(),
ImmutableList.copyOf(annotation.altNames()),
annotation.link(),
ImmutableList.copyOf(annotation.tags()),
annotation.summary(),
annotation.explanation(),
annotation.severity(),
annotation.disableable(),
annotation.documentSuppression() ? getSuppressionAnnotations(tree) : ImmutableList.of());
}
@Override
public boolean canExtract(ClassTree tree) {
return ASTHelpers.hasDirectAnnotationWithSimpleName(tree, BugPattern.class.getSimpleName());
}
/**
* Returns the fully-qualified class names of suppression annotations specified by the {@link
* BugPattern} annotation located on the given tree.
*
* @implNote This method cannot simply invoke {@link BugPattern#suppressionAnnotations()}, as that
* will yield an "Attempt to access Class objects for TypeMirrors" exception.
*/
private static ImmutableList<String> getSuppressionAnnotations(ClassTree tree) {
AnnotationTree annotationTree =
ASTHelpers.getAnnotationWithSimpleName(
ASTHelpers.getAnnotations(tree), BugPattern.class.getSimpleName());
requireNonNull(annotationTree, "BugPattern annotation must be present");
Attribute.Array types =
doCast(
AnnotationMirrors.getAnnotationValue(
ASTHelpers.getAnnotationMirror(annotationTree), "suppressionAnnotations"),
Attribute.Array.class);
return types.getValue().stream()
.map(v -> doCast(v, Attribute.Class.class).classType.toString())
.collect(toImmutableList());
}
@SuppressWarnings("unchecked")
private static <T extends AnnotationValue> T doCast(AnnotationValue value, Class<T> target) {
verify(target.isInstance(value), "Value '%s' is not of type '%s'", value, target);
return (T) value;
}
@AutoValue
abstract static class BugPatternDocumentation {
abstract String fullyQualifiedName();
abstract String name();
abstract ImmutableList<String> altNames();
abstract String link();
abstract ImmutableList<String> tags();
abstract String summary();
abstract String explanation();
abstract SeverityLevel severityLevel();
abstract boolean canDisable();
abstract ImmutableList<String> suppressionAnnotations();
}
}

View File

@@ -1,56 +0,0 @@
package tech.picnic.errorprone.documentation;
import static com.google.common.base.Preconditions.checkArgument;
import com.google.auto.service.AutoService;
import com.google.common.annotations.VisibleForTesting;
import com.sun.source.util.JavacTask;
import com.sun.source.util.Plugin;
import com.sun.tools.javac.api.BasicJavacTask;
import java.nio.file.InvalidPathException;
import java.nio.file.Path;
import java.util.regex.Matcher;
import java.util.regex.Pattern;
/**
* A compiler {@link Plugin} that analyzes and extracts relevant information for documentation
* purposes from processed files.
*/
// XXX: Find a better name for this class; it doesn't generate documentation per se.
@AutoService(Plugin.class)
public final class DocumentationGenerator implements Plugin {
@VisibleForTesting static final String OUTPUT_DIRECTORY_FLAG = "-XoutputDirectory";
private static final Pattern OUTPUT_DIRECTORY_FLAG_PATTERN =
Pattern.compile(Pattern.quote(OUTPUT_DIRECTORY_FLAG) + "=(.*)");
/** Instantiates a new {@link DocumentationGenerator} instance. */
public DocumentationGenerator() {}
@Override
public String getName() {
return getClass().getSimpleName();
}
@Override
public void init(JavacTask javacTask, String... args) {
checkArgument(args.length == 1, "Precisely one path must be provided");
javacTask.addTaskListener(
new DocumentationGeneratorTaskListener(
((BasicJavacTask) javacTask).getContext(), getOutputPath(args[0])));
}
@VisibleForTesting
static Path getOutputPath(String pathArg) {
Matcher matcher = OUTPUT_DIRECTORY_FLAG_PATTERN.matcher(pathArg);
checkArgument(
matcher.matches(), "'%s' must be of the form '%s=<value>'", pathArg, OUTPUT_DIRECTORY_FLAG);
String path = matcher.group(1);
try {
return Path.of(path);
} catch (InvalidPathException e) {
throw new IllegalArgumentException(String.format("Invalid path '%s'", path), e);
}
}
}

View File

@@ -1,91 +0,0 @@
package tech.picnic.errorprone.documentation;
import static java.nio.charset.StandardCharsets.UTF_8;
import com.fasterxml.jackson.annotation.JsonAutoDetect.Visibility;
import com.fasterxml.jackson.annotation.PropertyAccessor;
import com.fasterxml.jackson.databind.ObjectMapper;
import com.sun.source.tree.ClassTree;
import com.sun.source.util.TaskEvent;
import com.sun.source.util.TaskEvent.Kind;
import com.sun.source.util.TaskListener;
import com.sun.tools.javac.api.JavacTrees;
import com.sun.tools.javac.util.Context;
import java.io.File;
import java.io.FileWriter;
import java.io.IOException;
import java.io.UncheckedIOException;
import java.net.URI;
import java.nio.file.Files;
import java.nio.file.Path;
import java.nio.file.Paths;
import javax.tools.JavaFileObject;
/**
* A {@link TaskListener} that identifies and extracts relevant content for documentation generation
* and writes it to disk.
*/
// XXX: Find a better name for this class; it doesn't generate documentation per se.
final class DocumentationGeneratorTaskListener implements TaskListener {
private static final ObjectMapper OBJECT_MAPPER =
new ObjectMapper().setVisibility(PropertyAccessor.FIELD, Visibility.ANY);
private final Context context;
private final Path docsPath;
DocumentationGeneratorTaskListener(Context context, Path path) {
this.context = context;
this.docsPath = path;
}
@Override
public void started(TaskEvent taskEvent) {
if (taskEvent.getKind() == Kind.ANALYZE) {
createDocsDirectory();
}
}
@Override
public void finished(TaskEvent taskEvent) {
if (taskEvent.getKind() != Kind.ANALYZE) {
return;
}
ClassTree classTree = JavacTrees.instance(context).getTree(taskEvent.getTypeElement());
JavaFileObject sourceFile = taskEvent.getSourceFile();
if (classTree == null || sourceFile == null) {
return;
}
ExtractorType.findMatchingType(classTree)
.ifPresent(
extractorType ->
writeToFile(
extractorType.getIdentifier(),
getSimpleClassName(sourceFile.toUri()),
extractorType.getExtractor().extract(classTree, context)));
}
private void createDocsDirectory() {
try {
Files.createDirectories(docsPath);
} catch (IOException e) {
throw new IllegalStateException(
String.format("Error while creating directory with path '%s'", docsPath), e);
}
}
private <T> void writeToFile(String identifier, String className, T data) {
File file = docsPath.resolve(String.format("%s-%s.json", identifier, className)).toFile();
try (FileWriter fileWriter = new FileWriter(file, UTF_8)) {
OBJECT_MAPPER.writeValue(fileWriter, data);
} catch (IOException e) {
throw new UncheckedIOException(String.format("Cannot write to file '%s'", file.getPath()), e);
}
}
private static String getSimpleClassName(URI path) {
return Paths.get(path).getFileName().toString().replace(".java", "");
}
}

View File

@@ -1,33 +0,0 @@
package tech.picnic.errorprone.documentation;
import com.google.errorprone.annotations.Immutable;
import com.sun.source.tree.ClassTree;
import com.sun.tools.javac.util.Context;
/**
* Interface implemented by classes that define how to extract data of some type {@link T} from a
* given {@link ClassTree}.
*
* @param <T> The type of data that is extracted.
*/
@Immutable
interface Extractor<T> {
/**
* Extracts and returns an instance of {@link T} using the provided arguments.
*
* @param tree The {@link ClassTree} to analyze and from which to extract instances of {@link T}.
* @param context The {@link Context} in which the current compilation takes place.
* @return A non-null instance of {@link T}.
*/
// XXX: Drop `Context` parameter unless used.
T extract(ClassTree tree, Context context);
/**
* Tells whether this {@link Extractor} can extract documentation content from the given {@link
* ClassTree}.
*
* @param tree The {@link ClassTree} of interest.
* @return {@code true} iff data extraction is supported.
*/
boolean canExtract(ClassTree tree);
}

View File

@@ -1,36 +0,0 @@
package tech.picnic.errorprone.documentation;
import com.google.common.collect.ImmutableSet;
import com.google.common.collect.Sets;
import com.sun.source.tree.ClassTree;
import java.util.EnumSet;
import java.util.Optional;
/** An enumeration of {@link Extractor} types. */
enum ExtractorType {
BUG_PATTERN("bugpattern", new BugPatternExtractor());
private static final ImmutableSet<ExtractorType> TYPES =
Sets.immutableEnumSet(EnumSet.allOf(ExtractorType.class));
private final String identifier;
private final Extractor<?> extractor;
ExtractorType(String identifier, Extractor<?> extractor) {
this.identifier = identifier;
this.extractor = extractor;
}
String getIdentifier() {
return identifier;
}
@SuppressWarnings("java:S1452" /* The extractor returns data of an unspecified type. */)
Extractor<?> getExtractor() {
return extractor;
}
static Optional<ExtractorType> findMatchingType(ClassTree tree) {
return TYPES.stream().filter(type -> type.getExtractor().canExtract(tree)).findFirst();
}
}

View File

@@ -1,7 +0,0 @@
/**
* A Java compiler plugin that extracts data from compiled classes, in support of the Error Prone
* Support documentation.
*/
@com.google.errorprone.annotations.CheckReturnValue
@org.jspecify.annotations.NullMarked
package tech.picnic.errorprone.documentation;

View File

@@ -1,153 +0,0 @@
package tech.picnic.errorprone.documentation;
import static com.google.errorprone.BugPattern.SeverityLevel.ERROR;
import static java.nio.charset.StandardCharsets.UTF_8;
import static org.assertj.core.api.Assertions.assertThat;
import static org.assertj.core.api.Assertions.assertThatThrownBy;
import com.google.common.io.Resources;
import com.google.errorprone.BugPattern;
import com.google.errorprone.CompilationTestHelper;
import com.google.errorprone.VisitorState;
import com.google.errorprone.bugpatterns.BugChecker;
import com.google.errorprone.bugpatterns.BugChecker.ClassTreeMatcher;
import com.google.errorprone.matchers.Description;
import com.sun.source.tree.ClassTree;
import java.io.IOException;
import java.nio.file.Path;
import org.junit.jupiter.api.Test;
import org.junit.jupiter.api.io.TempDir;
final class BugPatternExtractorTest {
@Test
void noBugPattern(@TempDir Path outputDirectory) {
Compilation.compileWithDocumentationGenerator(
outputDirectory,
"TestCheckerWithoutAnnotation.java",
"import com.google.errorprone.bugpatterns.BugChecker;",
"",
"public final class TestCheckerWithoutAnnotation extends BugChecker {}");
assertThat(outputDirectory.toAbsolutePath()).isEmptyDirectory();
}
@Test
void minimalBugPattern(@TempDir Path outputDirectory) throws IOException {
Compilation.compileWithDocumentationGenerator(
outputDirectory,
"MinimalBugChecker.java",
"package pkg;",
"",
"import com.google.errorprone.BugPattern;",
"import com.google.errorprone.BugPattern.SeverityLevel;",
"import com.google.errorprone.bugpatterns.BugChecker;",
"",
"@BugPattern(summary = \"MinimalBugChecker summary\", severity = SeverityLevel.ERROR)",
"public final class MinimalBugChecker extends BugChecker {}");
verifyFileMatchesResource(
outputDirectory,
"bugpattern-MinimalBugChecker.json",
"bugpattern-documentation-minimal.json");
}
@Test
void completeBugPattern(@TempDir Path outputDirectory) throws IOException {
Compilation.compileWithDocumentationGenerator(
outputDirectory,
"CompleteBugChecker.java",
"package pkg;",
"",
"import com.google.errorprone.BugPattern;",
"import com.google.errorprone.BugPattern.SeverityLevel;",
"import com.google.errorprone.bugpatterns.BugChecker;",
"import org.junit.jupiter.api.Test;",
"",
"@BugPattern(",
" name = \"OtherName\",",
" summary = \"CompleteBugChecker summary\",",
" linkType = BugPattern.LinkType.CUSTOM,",
" link = \"https://error-prone.picnic.tech\",",
" explanation = \"Example explanation\",",
" severity = SeverityLevel.SUGGESTION,",
" altNames = \"Check\",",
" tags = BugPattern.StandardTags.SIMPLIFICATION,",
" disableable = false,",
" suppressionAnnotations = {BugPattern.class, Test.class})",
"public final class CompleteBugChecker extends BugChecker {}");
verifyFileMatchesResource(
outputDirectory,
"bugpattern-CompleteBugChecker.json",
"bugpattern-documentation-complete.json");
}
@Test
void undocumentedSuppressionBugPattern(@TempDir Path outputDirectory) throws IOException {
Compilation.compileWithDocumentationGenerator(
outputDirectory,
"UndocumentedSuppressionBugPattern.java",
"package pkg;",
"",
"import com.google.errorprone.BugPattern;",
"import com.google.errorprone.BugPattern.SeverityLevel;",
"import com.google.errorprone.bugpatterns.BugChecker;",
"",
"@BugPattern(",
" summary = \"UndocumentedSuppressionBugPattern summary\",",
" severity = SeverityLevel.WARNING,",
" documentSuppression = false)",
"public final class UndocumentedSuppressionBugPattern extends BugChecker {}");
verifyFileMatchesResource(
outputDirectory,
"bugpattern-UndocumentedSuppressionBugPattern.json",
"bugpattern-documentation-undocumented-suppression.json");
}
@Test
void bugPatternAnnotationIsAbsent() {
CompilationTestHelper.newInstance(TestChecker.class, getClass())
.addSourceLines(
"TestChecker.java",
"import com.google.errorprone.bugpatterns.BugChecker;",
"",
"// BUG: Diagnostic contains: Can extract: false",
"public final class TestChecker extends BugChecker {}")
.doTest();
}
private static void verifyFileMatchesResource(
Path outputDirectory, String fileName, String resourceName) throws IOException {
assertThat(outputDirectory.resolve(fileName))
.content(UTF_8)
.isEqualToIgnoringWhitespace(getResource(resourceName));
}
// XXX: Once we support only JDK 15+, drop this method in favour of including the resources as
// text blocks in this class. (This also requires renaming the `verifyFileMatchesResource`
// method.)
private static String getResource(String resourceName) throws IOException {
return Resources.toString(
Resources.getResource(BugPatternExtractorTest.class, resourceName), UTF_8);
}
/** A {@link BugChecker} that validates the {@link BugPatternExtractor}. */
@BugPattern(summary = "Validates `BugPatternExtractor` extraction", severity = ERROR)
public static final class TestChecker extends BugChecker implements ClassTreeMatcher {
private static final long serialVersionUID = 1L;
@Override
public Description matchClass(ClassTree tree, VisitorState state) {
BugPatternExtractor extractor = new BugPatternExtractor();
assertThatThrownBy(() -> extractor.extract(tree, state.context))
.isInstanceOf(NullPointerException.class)
.hasMessage("BugPattern annotation must be present");
return buildDescription(tree)
.setMessage(String.format("Can extract: %s", extractor.canExtract(tree)))
.build();
}
}
}

View File

@@ -1,45 +0,0 @@
package tech.picnic.errorprone.documentation;
import com.google.common.collect.ImmutableList;
import com.google.errorprone.FileManagers;
import com.google.errorprone.FileObjects;
import com.sun.tools.javac.api.JavacTaskImpl;
import com.sun.tools.javac.api.JavacTool;
import com.sun.tools.javac.file.JavacFileManager;
import java.nio.file.Path;
import javax.tools.JavaCompiler;
import javax.tools.JavaFileObject;
// XXX: Generalize and move this class so that it can also be used by `refaster-compiler`.
// XXX: Add support for this class to the `ErrorProneTestHelperSourceFormat` check.
public final class Compilation {
private Compilation() {}
public static void compileWithDocumentationGenerator(
Path outputDirectory, String fileName, String... lines) {
compileWithDocumentationGenerator(outputDirectory.toAbsolutePath().toString(), fileName, lines);
}
public static void compileWithDocumentationGenerator(
String outputDirectory, String fileName, String... lines) {
compile(
ImmutableList.of("-Xplugin:DocumentationGenerator -XoutputDirectory=" + outputDirectory),
FileObjects.forSourceLines(fileName, lines));
}
private static void compile(ImmutableList<String> options, JavaFileObject javaFileObject) {
JavacFileManager javacFileManager = FileManagers.testFileManager();
JavaCompiler compiler = JavacTool.create();
JavacTaskImpl task =
(JavacTaskImpl)
compiler.getTask(
null,
javacFileManager,
null,
options,
ImmutableList.of(),
ImmutableList.of(javaFileObject));
task.call();
}
}

View File

@@ -1,78 +0,0 @@
package tech.picnic.errorprone.documentation;
import static com.google.common.collect.ImmutableList.toImmutableList;
import static java.nio.file.attribute.AclEntryPermission.ADD_SUBDIRECTORY;
import static org.assertj.core.api.Assertions.assertThat;
import static org.assertj.core.api.Assertions.assertThatThrownBy;
import static org.junit.jupiter.api.condition.OS.WINDOWS;
import com.google.common.collect.ImmutableSet;
import com.google.common.collect.Sets;
import java.io.IOException;
import java.nio.file.FileSystemException;
import java.nio.file.Files;
import java.nio.file.Path;
import java.nio.file.attribute.AclEntry;
import java.nio.file.attribute.AclFileAttributeView;
import org.junit.jupiter.api.Test;
import org.junit.jupiter.api.condition.DisabledOnOs;
import org.junit.jupiter.api.condition.EnabledOnOs;
import org.junit.jupiter.api.io.TempDir;
final class DocumentationGeneratorTaskListenerTest {
@EnabledOnOs(WINDOWS)
@Test
void readOnlyFileSystemWindows(@TempDir Path outputDirectory) throws IOException {
AclFileAttributeView view =
Files.getFileAttributeView(outputDirectory, AclFileAttributeView.class);
view.setAcl(
view.getAcl().stream()
.map(
entry ->
AclEntry.newBuilder(entry)
.setPermissions(
Sets.difference(entry.permissions(), ImmutableSet.of(ADD_SUBDIRECTORY)))
.build())
.collect(toImmutableList()));
readOnlyFileSystemFailsToWrite(outputDirectory.resolve("nonexistent"));
}
@DisabledOnOs(WINDOWS)
@Test
void readOnlyFileSystemNonWindows(@TempDir Path outputDirectory) {
assertThat(outputDirectory.toFile().setWritable(false))
.describedAs("Failed to make test directory unwritable")
.isTrue();
readOnlyFileSystemFailsToWrite(outputDirectory.resolve("nonexistent"));
}
private static void readOnlyFileSystemFailsToWrite(Path outputDirectory) {
assertThatThrownBy(
() ->
Compilation.compileWithDocumentationGenerator(
outputDirectory, "A.java", "class A {}"))
.hasRootCauseInstanceOf(FileSystemException.class)
.hasCauseInstanceOf(IllegalStateException.class)
.hasMessageEndingWith("Error while creating directory with path '%s'", outputDirectory);
}
@Test
void noClassNoOutput(@TempDir Path outputDirectory) {
Compilation.compileWithDocumentationGenerator(outputDirectory, "A.java", "package pkg;");
assertThat(outputDirectory).isEmptyDirectory();
}
@Test
void excessArguments(@TempDir Path outputDirectory) {
String actualOutputDirectory = outputDirectory.toAbsolutePath() + " extra-arg";
assertThatThrownBy(
() ->
Compilation.compileWithDocumentationGenerator(
actualOutputDirectory, "A.java", "package pkg;"))
.isInstanceOf(IllegalArgumentException.class)
.hasMessage("Precisely one path must be provided");
}
}

View File

@@ -1,38 +0,0 @@
package tech.picnic.errorprone.documentation;
import static org.assertj.core.api.Assertions.assertThat;
import static org.assertj.core.api.Assertions.assertThatThrownBy;
import static tech.picnic.errorprone.documentation.DocumentationGenerator.OUTPUT_DIRECTORY_FLAG;
import java.nio.file.InvalidPathException;
import java.nio.file.Path;
import org.junit.jupiter.api.Test;
import org.junit.jupiter.params.ParameterizedTest;
import org.junit.jupiter.params.provider.ValueSource;
final class DocumentationGeneratorTest {
@ParameterizedTest
@ValueSource(strings = {"bar", "foo"})
void getOutputPath(String path) {
assertThat(DocumentationGenerator.getOutputPath(OUTPUT_DIRECTORY_FLAG + '=' + path))
.isEqualTo(Path.of(path));
}
@ParameterizedTest
@ValueSource(strings = {"", "-XoutputDirectory", "invalidOption=Test", "nothing"})
void getOutputPathWithInvalidArgument(String pathArg) {
assertThatThrownBy(() -> DocumentationGenerator.getOutputPath(pathArg))
.isInstanceOf(IllegalArgumentException.class)
.hasMessage("'%s' must be of the form '%s=<value>'", pathArg, OUTPUT_DIRECTORY_FLAG);
}
@Test
void getOutputPathWithInvalidPath() {
String basePath = "path-with-null-char-\0";
assertThatThrownBy(
() -> DocumentationGenerator.getOutputPath(OUTPUT_DIRECTORY_FLAG + '=' + basePath))
.isInstanceOf(IllegalArgumentException.class)
.hasCauseInstanceOf(InvalidPathException.class)
.hasMessageEndingWith("Invalid path '%s'", basePath);
}
}

View File

@@ -1,19 +0,0 @@
{
"fullyQualifiedName": "pkg.CompleteBugChecker",
"name": "OtherName",
"altNames": [
"Check"
],
"link": "https://error-prone.picnic.tech",
"tags": [
"Simplification"
],
"summary": "CompleteBugChecker summary",
"explanation": "Example explanation",
"severityLevel": "SUGGESTION",
"canDisable": false,
"suppressionAnnotations": [
"com.google.errorprone.BugPattern",
"org.junit.jupiter.api.Test"
]
}

View File

@@ -1,14 +0,0 @@
{
"fullyQualifiedName": "pkg.MinimalBugChecker",
"name": "MinimalBugChecker",
"altNames": [],
"link": "",
"tags": [],
"summary": "MinimalBugChecker summary",
"explanation": "",
"severityLevel": "ERROR",
"canDisable": true,
"suppressionAnnotations": [
"java.lang.SuppressWarnings"
]
}

View File

@@ -1,12 +0,0 @@
{
"fullyQualifiedName": "pkg.UndocumentedSuppressionBugPattern",
"name": "UndocumentedSuppressionBugPattern",
"altNames": [],
"link": "",
"tags": [],
"summary": "UndocumentedSuppressionBugPattern summary",
"explanation": "",
"severityLevel": "WARNING",
"canDisable": true,
"suppressionAnnotations": []
}

View File

@@ -24,6 +24,7 @@ project:
- Document how to apply patches.
- Document each of the checks.
- Add [SonarQube][sonarcloud] and [Codecov][codecov] integrations.
- Add non-Java file formatting support, like we have internally at Picnic.
(I.e., somehow open-source that stuff.)
- Auto-generate a website listing each of the checks, just like the Error Prone
@@ -125,6 +126,9 @@ The following is a list of checks we'd like to see implemented:
statement. Idem for other exception types.
- A Guava-specific check that replaces simple anonymous `CacheLoader` subclass
declarations with `CacheLoader.from(someLambda)`.
- A Spring-specific check that enforces that methods with the `@Scheduled`
annotation are also annotated with New Relic's `@Trace` annotation. Such
methods should ideally not also represent Spring MVC endpoints.
- A Spring-specific check that enforces that `@RequestMapping` annotations,
when applied to a method, explicitly specify one or more target HTTP methods.
- A Spring-specific check that looks for classes in which all `@RequestMapping`
@@ -269,6 +273,7 @@ Refaster's expressiveness:
[autorefactor]: https://autorefactor.org
[bettercodehub]: https://bettercodehub.com
[checkstyle-external-project-tests]: https://github.com/checkstyle/checkstyle/blob/master/wercker.yml
[codecov]: https://codecov.io
[error-prone-bug-patterns]: https://errorprone.info/bugpatterns
[error-prone]: https://errorprone.info
[error-prone-repo]: https://github.com/google/error-prone
@@ -278,3 +283,4 @@ Refaster's expressiveness:
[main-contributing]: ../CONTRIBUTING.md
[main-readme]: ../README.md
[modernizer-maven-plugin]: https://github.com/gaul/modernizer-maven-plugin
[sonarcloud]: https://sonarcloud.io

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.5.1-SNAPSHOT</version>
</parent>
<artifactId>error-prone-contrib</artifactId>
@@ -37,15 +37,7 @@
<dependency>
<groupId>${groupId.error-prone}</groupId>
<artifactId>error_prone_test_helpers</artifactId>
<scope>provided</scope>
</dependency>
<dependency>
<groupId>${project.groupId}</groupId>
<artifactId>documentation-support</artifactId>
<!-- This dependency is declared only as a hint to Maven that
compilation depends on it; see the `maven-compiler-plugin`'s
`annotationProcessorPaths` configuration below. -->
<scope>provided</scope>
<scope>test</scope>
</dependency>
<dependency>
<groupId>${project.groupId}</groupId>
@@ -81,6 +73,11 @@
<artifactId>guava</artifactId>
<scope>provided</scope>
</dependency>
<dependency>
<groupId>com.newrelic.agent.java</groupId>
<artifactId>newrelic-api</artifactId>
<scope>test</scope>
</dependency>
<dependency>
<groupId>io.projectreactor</groupId>
<artifactId>reactor-core</artifactId>
@@ -121,11 +118,6 @@
<artifactId>jakarta.servlet-api</artifactId>
<scope>test</scope>
</dependency>
<dependency>
<groupId>javax.inject</groupId>
<artifactId>javax.inject</artifactId>
<scope>provided</scope>
</dependency>
<dependency>
<groupId>javax.xml.bind</groupId>
<artifactId>jaxb-api</artifactId>
@@ -154,7 +146,7 @@
<dependency>
<groupId>org.junit.jupiter</groupId>
<artifactId>junit-jupiter-api</artifactId>
<scope>provided</scope>
<scope>test</scope>
</dependency>
<dependency>
<groupId>org.junit.jupiter</groupId>
@@ -171,11 +163,6 @@
<artifactId>mockito-core</artifactId>
<scope>provided</scope>
</dependency>
<dependency>
<groupId>org.mongodb</groupId>
<artifactId>mongodb-driver-core</artifactId>
<scope>test</scope>
</dependency>
<dependency>
<groupId>org.reactivestreams</groupId>
<artifactId>reactive-streams</artifactId>
@@ -226,11 +213,6 @@
<artifactId>maven-compiler-plugin</artifactId>
<configuration>
<annotationProcessorPaths combine.children="append">
<path>
<groupId>${project.groupId}</groupId>
<artifactId>documentation-support</artifactId>
<version>${project.version}</version>
</path>
<path>
<groupId>${project.groupId}</groupId>
<artifactId>refaster-compiler</artifactId>
@@ -244,7 +226,6 @@
</annotationProcessorPaths>
<compilerArgs combine.children="append">
<arg>-Xplugin:RefasterRuleCompiler</arg>
<arg>-Xplugin:DocumentationGenerator -XoutputDirectory=${project.build.directory}/docs</arg>
</compilerArgs>
</configuration>
</plugin>

View File

@@ -1,103 +0,0 @@
package tech.picnic.errorprone.bugpatterns;
import static com.google.errorprone.BugPattern.LinkType.CUSTOM;
import static com.google.errorprone.BugPattern.SeverityLevel.SUGGESTION;
import static com.google.errorprone.BugPattern.StandardTags.SIMPLIFICATION;
import static com.google.errorprone.matchers.Matchers.allOf;
import static com.google.errorprone.matchers.Matchers.not;
import static com.google.errorprone.matchers.Matchers.staticMethod;
import static com.google.errorprone.matchers.Matchers.toType;
import static tech.picnic.errorprone.bugpatterns.util.Documentation.BUG_PATTERNS_BASE_URL;
import com.google.auto.service.AutoService;
import com.google.common.collect.ImmutableSet;
import com.google.errorprone.BugPattern;
import com.google.errorprone.VisitorState;
import com.google.errorprone.bugpatterns.BugChecker;
import com.google.errorprone.bugpatterns.BugChecker.MethodInvocationTreeMatcher;
import com.google.errorprone.fixes.SuggestedFix;
import com.google.errorprone.fixes.SuggestedFixes;
import com.google.errorprone.matchers.Description;
import com.google.errorprone.matchers.Matcher;
import com.google.errorprone.matchers.Matchers;
import com.google.errorprone.refaster.Refaster;
import com.google.errorprone.suppliers.Supplier;
import com.google.errorprone.suppliers.Suppliers;
import com.google.errorprone.util.ASTHelpers;
import com.sun.source.tree.ExpressionTree;
import com.sun.source.tree.MethodInvocationTree;
import com.sun.tools.javac.code.Type;
import tech.picnic.errorprone.bugpatterns.util.SourceCode;
/**
* A {@link BugChecker} that flags unnecessarily nested usage of methods that implement an
* associative operation.
*
* <p>The arguments to such methods can be flattened without affecting semantics, while making the
* code more readable.
*/
@AutoService(BugChecker.class)
@BugPattern(
summary =
"This method implements an associative operation, so the list of operands can be flattened",
link = BUG_PATTERNS_BASE_URL + "AssociativeMethodInvocation",
linkType = CUSTOM,
severity = SUGGESTION,
tags = SIMPLIFICATION)
public final class AssociativeMethodInvocation extends BugChecker
implements MethodInvocationTreeMatcher {
private static final long serialVersionUID = 1L;
private static final Supplier<Type> ITERABLE = Suppliers.typeFromClass(Iterable.class);
private static final ImmutableSet<Matcher<ExpressionTree>> ASSOCIATIVE_OPERATIONS =
ImmutableSet.of(
allOf(
staticMethod().onClass(Suppliers.typeFromClass(Matchers.class)).named("allOf"),
toType(MethodInvocationTree.class, not(hasArgumentOfType(ITERABLE)))),
allOf(
staticMethod().onClass(Suppliers.typeFromClass(Matchers.class)).named("anyOf"),
toType(MethodInvocationTree.class, not(hasArgumentOfType(ITERABLE)))),
staticMethod().onClass(Suppliers.typeFromClass(Refaster.class)).named("anyOf"));
/** Instantiates a new {@link AssociativeMethodInvocation} instance. */
public AssociativeMethodInvocation() {}
@Override
public Description matchMethodInvocation(MethodInvocationTree tree, VisitorState state) {
if (tree.getArguments().isEmpty()) {
/* Absent any arguments, there is nothing to simplify. */
return Description.NO_MATCH;
}
for (Matcher<ExpressionTree> matcher : ASSOCIATIVE_OPERATIONS) {
if (matcher.matches(tree, state)) {
SuggestedFix fix = processMatchingArguments(tree, matcher, state);
return fix.isEmpty() ? Description.NO_MATCH : describeMatch(tree, fix);
}
}
return Description.NO_MATCH;
}
private static SuggestedFix processMatchingArguments(
MethodInvocationTree tree, Matcher<ExpressionTree> matcher, VisitorState state) {
SuggestedFix.Builder fix = SuggestedFix.builder();
for (ExpressionTree arg : tree.getArguments()) {
if (matcher.matches(arg, state)) {
MethodInvocationTree invocation = (MethodInvocationTree) arg;
fix.merge(
invocation.getArguments().isEmpty()
? SuggestedFixes.removeElement(invocation, tree.getArguments(), state)
: SourceCode.unwrapMethodInvocation(invocation, state));
}
}
return fix.build();
}
private static Matcher<MethodInvocationTree> hasArgumentOfType(Supplier<Type> type) {
return (tree, state) ->
tree.getArguments().stream()
.anyMatch(arg -> ASTHelpers.isSubtype(ASTHelpers.getType(arg), type.get(state), state));
}
}

View File

@@ -15,6 +15,7 @@ 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.matchers.Description;
import com.google.errorprone.matchers.MultiMatcher;
import com.google.errorprone.util.ASTHelpers;
@@ -23,7 +24,6 @@ import com.sun.source.tree.ClassTree;
import com.sun.source.tree.MethodTree;
import com.sun.source.tree.Tree;
import java.util.List;
import tech.picnic.errorprone.bugpatterns.util.SourceCode;
/** A {@link BugChecker} that flags redundant {@code @Autowired} constructor annotations. */
@AutoService(BugChecker.class)
@@ -58,10 +58,10 @@ public final class AutowiredConstructor extends BugChecker implements ClassTreeM
/*
* This is the only `@Autowired` constructor: suggest that it be removed. Note that this likely
* means that the associated import can be removed as well. Rather than adding code for this
* case we leave flagging the unused import to Error Prone's `RemoveUnusedImports` check.
* means that the associated import can be removed as well. Rather than adding code for this case we
* leave flagging the unused import to Error Prone's `RemoveUnusedImports` check.
*/
AnnotationTree annotation = Iterables.getOnlyElement(annotations);
return describeMatch(annotation, SourceCode.deleteWithTrailingWhitespace(annotation, state));
return describeMatch(annotation, SuggestedFix.delete(annotation));
}
}

View File

@@ -91,8 +91,8 @@ public final class CanonicalAnnotationSyntax extends BugChecker implements Annot
ExpressionTree arg = args.get(0);
if (state.getSourceForNode(arg) == null) {
/*
* The annotation argument doesn't have a source representation, e.g. because `value` isn't
* assigned explicitly.
* The annotation argument isn't doesn't have a source representation, e.g. because `value`
* isn't assigned to explicitly.
*/
return Optional.empty();
}

View File

@@ -30,7 +30,7 @@ import tech.picnic.errorprone.bugpatterns.util.ThirdPartyLibrary;
@AutoService(BugChecker.class)
@BugPattern(
summary =
"Avoid `Collectors.to{List,Map,Set}` in favor of collectors that emphasize (im)mutability",
"Avoid `Collectors.to{List,Map,Set}` in favour of alternatives that emphasize (im)mutability",
link = BUG_PATTERNS_BASE_URL + "CollectorMutability",
linkType = CUSTOM,
severity = WARNING,

View File

@@ -1,175 +0,0 @@
package tech.picnic.errorprone.bugpatterns;
import static com.google.errorprone.BugPattern.LinkType.CUSTOM;
import static com.google.errorprone.BugPattern.SeverityLevel.SUGGESTION;
import static com.google.errorprone.BugPattern.StandardTags.SIMPLIFICATION;
import static com.google.errorprone.matchers.Matchers.allOf;
import static com.google.errorprone.matchers.Matchers.argument;
import static com.google.errorprone.matchers.Matchers.isSameType;
import static com.google.errorprone.matchers.Matchers.isVariable;
import static com.google.errorprone.matchers.Matchers.not;
import static com.google.errorprone.matchers.Matchers.returnStatement;
import static com.google.errorprone.matchers.Matchers.staticMethod;
import static com.google.errorprone.matchers.Matchers.toType;
import static tech.picnic.errorprone.bugpatterns.util.Documentation.BUG_PATTERNS_BASE_URL;
import com.google.auto.service.AutoService;
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.BlockTreeMatcher;
import com.google.errorprone.fixes.SuggestedFix;
import com.google.errorprone.matchers.Description;
import com.google.errorprone.matchers.Matcher;
import com.google.errorprone.util.ASTHelpers;
import com.sun.source.tree.AssignmentTree;
import com.sun.source.tree.BlockTree;
import com.sun.source.tree.ExpressionStatementTree;
import com.sun.source.tree.ExpressionTree;
import com.sun.source.tree.IdentifierTree;
import com.sun.source.tree.MethodInvocationTree;
import com.sun.source.tree.ReturnTree;
import com.sun.source.tree.StatementTree;
import com.sun.source.tree.Tree;
import com.sun.source.tree.TryTree;
import com.sun.source.tree.VariableTree;
import com.sun.source.util.TreeScanner;
import com.sun.tools.javac.code.Symbol;
import java.util.List;
import java.util.Optional;
import org.jspecify.annotations.Nullable;
import tech.picnic.errorprone.bugpatterns.util.MoreASTHelpers;
import tech.picnic.errorprone.bugpatterns.util.SourceCode;
/**
* A {@link BugChecker} that flags unnecessary local variable assignments preceding a return
* statement.
*/
@AutoService(BugChecker.class)
@BugPattern(
summary = "Variable assignment is redundant; value can be returned directly",
link = BUG_PATTERNS_BASE_URL + "DirectReturn",
linkType = CUSTOM,
severity = SUGGESTION,
tags = SIMPLIFICATION)
public final class DirectReturn extends BugChecker implements BlockTreeMatcher {
private static final long serialVersionUID = 1L;
private static final Matcher<StatementTree> VARIABLE_RETURN = returnStatement(isVariable());
private static final Matcher<ExpressionTree> MOCKITO_MOCK_OR_SPY_WITH_IMPLICIT_TYPE =
allOf(
not(toType(MethodInvocationTree.class, argument(0, isSameType(Class.class.getName())))),
staticMethod().onClass("org.mockito.Mockito").namedAnyOf("mock", "spy"));
/** Instantiates a new {@link DirectReturn} instance. */
public DirectReturn() {}
@Override
public Description matchBlock(BlockTree tree, VisitorState state) {
List<? extends StatementTree> statements = tree.getStatements();
if (statements.size() < 2) {
return Description.NO_MATCH;
}
StatementTree finalStatement = statements.get(statements.size() - 1);
if (!VARIABLE_RETURN.matches(finalStatement, state)) {
return Description.NO_MATCH;
}
Symbol variableSymbol = ASTHelpers.getSymbol(((ReturnTree) finalStatement).getExpression());
StatementTree precedingStatement = statements.get(statements.size() - 2);
return tryMatchAssignment(variableSymbol, precedingStatement)
.filter(
resultExpr ->
canInlineToReturnStatement(resultExpr, state)
&& !isIdentifierSymbolReferencedInAssociatedFinallyBlock(variableSymbol, state))
.map(
resultExpr ->
describeMatch(
precedingStatement,
SuggestedFix.builder()
.replace(
precedingStatement,
String.format("return %s;", SourceCode.treeToString(resultExpr, state)))
.delete(finalStatement)
.build()))
.orElse(Description.NO_MATCH);
}
private static Optional<ExpressionTree> tryMatchAssignment(Symbol targetSymbol, Tree tree) {
if (tree instanceof ExpressionStatementTree) {
return tryMatchAssignment(targetSymbol, ((ExpressionStatementTree) tree).getExpression());
}
if (tree instanceof AssignmentTree) {
AssignmentTree assignment = (AssignmentTree) tree;
return targetSymbol.equals(ASTHelpers.getSymbol(assignment.getVariable()))
? Optional.of(assignment.getExpression())
: Optional.empty();
}
if (tree instanceof VariableTree) {
VariableTree declaration = (VariableTree) tree;
return declaration.getModifiers().getAnnotations().isEmpty()
&& targetSymbol.equals(ASTHelpers.getSymbol(declaration))
? Optional.ofNullable(declaration.getInitializer())
: Optional.empty();
}
return Optional.empty();
}
/**
* Tells whether inlining the given expression to the associated return statement can likely be
* done without changing the expression's return type.
*
* <p>Inlining an expression generally does not change its return type, but in rare cases the
* operation may have a functional impact. The sole case considered here is the inlining of a
* Mockito mock or spy construction without an explicit type. In such a case the type created
* depends on context, such as the method's return type.
*/
private static boolean canInlineToReturnStatement(
ExpressionTree expressionTree, VisitorState state) {
return !MOCKITO_MOCK_OR_SPY_WITH_IMPLICIT_TYPE.matches(expressionTree, state)
|| MoreASTHelpers.findMethodExitedOnReturn(state)
.filter(m -> MoreASTHelpers.areSameType(expressionTree, m.getReturnType(), state))
.isPresent();
}
/**
* Tells whether the given identifier {@link Symbol} is referenced in a {@code finally} block that
* is executed <em>after</em> control flow returns from the {@link VisitorState#getPath() current
* location}.
*/
private static boolean isIdentifierSymbolReferencedInAssociatedFinallyBlock(
Symbol symbol, VisitorState state) {
return Streams.zip(
Streams.stream(state.getPath()).skip(1),
Streams.stream(state.getPath()),
(tree, child) -> {
if (!(tree instanceof TryTree)) {
return null;
}
BlockTree finallyBlock = ((TryTree) tree).getFinallyBlock();
return !child.equals(finallyBlock) ? finallyBlock : null;
})
.anyMatch(finallyBlock -> referencesIdentifierSymbol(symbol, finallyBlock));
}
private static boolean referencesIdentifierSymbol(Symbol symbol, @Nullable BlockTree tree) {
return Boolean.TRUE.equals(
new TreeScanner<Boolean, @Nullable Void>() {
@Override
public Boolean visitIdentifier(IdentifierTree node, @Nullable Void unused) {
return symbol.equals(ASTHelpers.getSymbol(node));
}
@Override
public Boolean reduce(Boolean r1, Boolean r2) {
return Boolean.TRUE.equals(r1) || Boolean.TRUE.equals(r2);
}
}.scan(tree, null));
}
}

View File

@@ -14,6 +14,7 @@ import com.google.errorprone.BugPattern;
import com.google.errorprone.VisitorState;
import com.google.errorprone.bugpatterns.BugChecker;
import com.google.errorprone.bugpatterns.BugChecker.MethodTreeMatcher;
import com.google.errorprone.fixes.SuggestedFix;
import com.google.errorprone.matchers.Description;
import com.google.errorprone.matchers.Matcher;
import com.google.errorprone.util.ASTHelpers;
@@ -21,7 +22,6 @@ import com.sun.source.tree.ClassTree;
import com.sun.source.tree.MethodTree;
import com.sun.source.tree.Tree;
import java.util.Optional;
import tech.picnic.errorprone.bugpatterns.util.SourceCode;
/** A {@link BugChecker} that flags empty methods that seemingly can simply be deleted. */
@AutoService(BugChecker.class)
@@ -42,7 +42,6 @@ public final class EmptyMethod extends BugChecker implements MethodTreeMatcher {
public EmptyMethod() {}
@Override
@SuppressWarnings("java:S1067" /* Chaining disjunctions like this does not impact readability. */)
public Description matchMethod(MethodTree tree, VisitorState state) {
if (tree.getBody() == null
|| !tree.getBody().getStatements().isEmpty()
@@ -56,7 +55,7 @@ public final class EmptyMethod extends BugChecker implements MethodTreeMatcher {
return Description.NO_MATCH;
}
return describeMatch(tree, SourceCode.deleteWithTrailingWhitespace(tree, state));
return describeMatch(tree, SuggestedFix.delete(tree));
}
private static boolean isInPossibleTestHelperClass(VisitorState state) {

View File

@@ -104,9 +104,7 @@ public final class ErrorProneTestHelperSourceFormat extends BugChecker
String formatted;
try {
formatted = formatSourceCode(source, retainUnusedImports).trim();
} catch (
@SuppressWarnings("java:S1166" /* Stack trace not relevant. */)
FormatterException e) {
} catch (FormatterException e) {
return buildDescription(methodInvocation)
.setMessage(String.format("Source code is malformed: %s", e.getMessage()))
.build();
@@ -133,7 +131,7 @@ public final class ErrorProneTestHelperSourceFormat extends BugChecker
SuggestedFix.replace(
startPos,
endPos,
Splitter.on(System.lineSeparator())
Splitter.on('\n')
.splitToStream(formatted)
.map(state::getConstantExpression)
.collect(joining(", "))));
@@ -159,7 +157,7 @@ public final class ErrorProneTestHelperSourceFormat extends BugChecker
return Optional.empty();
}
source.append(value).append(System.lineSeparator());
source.append(value).append('\n');
}
return Optional.of(source.toString());

View File

@@ -82,8 +82,10 @@ public final class FluxFlatMapUsage extends BugChecker
SuggestedFix serializationFix = SuggestedFixes.renameMethodInvocation(tree, "concatMap", state);
SuggestedFix concurrencyCapFix =
SuggestedFix.postfixWith(
Iterables.getOnlyElement(tree.getArguments()), ", " + MAX_CONCURRENCY_ARG_NAME);
SuggestedFix.builder()
.postfixWith(
Iterables.getOnlyElement(tree.getArguments()), ", " + MAX_CONCURRENCY_ARG_NAME)
.build();
Description.Builder description = buildDescription(tree);

View File

@@ -1,100 +0,0 @@
package tech.picnic.errorprone.bugpatterns;
import static com.google.common.base.Preconditions.checkState;
import static com.google.errorprone.BugPattern.LinkType.CUSTOM;
import static com.google.errorprone.BugPattern.SeverityLevel.WARNING;
import static com.google.errorprone.BugPattern.StandardTags.CONCURRENCY;
import static com.google.errorprone.BugPattern.StandardTags.PERFORMANCE;
import static com.google.errorprone.matchers.method.MethodMatchers.instanceMethod;
import static tech.picnic.errorprone.bugpatterns.util.Documentation.BUG_PATTERNS_BASE_URL;
import com.google.auto.service.AutoService;
import com.google.errorprone.BugPattern;
import com.google.errorprone.VisitorState;
import com.google.errorprone.bugpatterns.BugChecker;
import com.google.errorprone.bugpatterns.BugChecker.MethodInvocationTreeMatcher;
import com.google.errorprone.fixes.SuggestedFix;
import com.google.errorprone.fixes.SuggestedFixes;
import com.google.errorprone.matchers.Description;
import com.google.errorprone.matchers.Matcher;
import com.google.errorprone.suppliers.Supplier;
import com.google.errorprone.suppliers.Suppliers;
import com.google.errorprone.util.ASTHelpers;
import com.sun.source.tree.ExpressionTree;
import com.sun.source.tree.MethodInvocationTree;
import com.sun.tools.javac.code.Type;
import com.sun.tools.javac.util.Position;
import java.util.stream.Stream;
import tech.picnic.errorprone.bugpatterns.util.ThirdPartyLibrary;
/**
* A {@link BugChecker} that flags {@link reactor.core.publisher.Flux} operator usages that may
* implicitly cause the calling thread to be blocked.
*
* <p>Note that the methods flagged here are not themselves blocking, but iterating over the
* resulting {@link Iterable} or {@link Stream} may be.
*/
@AutoService(BugChecker.class)
@BugPattern(
summary = "Avoid iterating over `Flux`es in an implicitly blocking manner",
link = BUG_PATTERNS_BASE_URL + "FluxImplicitBlock",
linkType = CUSTOM,
severity = WARNING,
tags = {CONCURRENCY, PERFORMANCE})
public final class FluxImplicitBlock extends BugChecker implements MethodInvocationTreeMatcher {
private static final long serialVersionUID = 1L;
private static final Matcher<ExpressionTree> FLUX_WITH_IMPLICIT_BLOCK =
instanceMethod()
.onDescendantOf("reactor.core.publisher.Flux")
.namedAnyOf("toIterable", "toStream")
.withNoParameters();
private static final Supplier<Type> STREAM = Suppliers.typeFromString(Stream.class.getName());
/** Instantiates a new {@link FluxImplicitBlock} instance. */
public FluxImplicitBlock() {}
@Override
public Description matchMethodInvocation(MethodInvocationTree tree, VisitorState state) {
if (!FLUX_WITH_IMPLICIT_BLOCK.matches(tree, state)) {
return Description.NO_MATCH;
}
Description.Builder description =
buildDescription(tree).addFix(SuggestedFixes.addSuppressWarnings(state, canonicalName()));
if (ThirdPartyLibrary.GUAVA.isIntroductionAllowed(state)) {
description.addFix(
suggestBlockingElementCollection(
tree, "com.google.common.collect.ImmutableList.toImmutableList", state));
}
description.addFix(
suggestBlockingElementCollection(tree, "java.util.stream.Collectors.toList", state));
return description.build();
}
private static SuggestedFix suggestBlockingElementCollection(
MethodInvocationTree tree, String fullyQualifiedCollectorMethod, VisitorState state) {
SuggestedFix.Builder importSuggestion = SuggestedFix.builder();
String replacementMethodInvocation =
SuggestedFixes.qualifyStaticImport(fullyQualifiedCollectorMethod, importSuggestion, state);
boolean isStream =
ASTHelpers.isSubtype(ASTHelpers.getResultType(tree), STREAM.get(state), state);
String replacement =
String.format(
".collect(%s()).block()%s", replacementMethodInvocation, isStream ? ".stream()" : "");
return importSuggestion.merge(replaceMethodInvocation(tree, replacement, state)).build();
}
private static SuggestedFix.Builder replaceMethodInvocation(
MethodInvocationTree tree, String replacement, VisitorState state) {
int startPosition = state.getEndPosition(ASTHelpers.getReceiver(tree));
int endPosition = state.getEndPosition(tree);
checkState(
startPosition != Position.NOPOS && endPosition != Position.NOPOS,
"Cannot locate method to be replaced in source code");
return SuggestedFix.builder().replace(startPosition, endPosition, replacement);
}
}

View File

@@ -32,7 +32,7 @@ import com.sun.source.util.SimpleTreeVisitor;
import java.util.ArrayList;
import java.util.List;
import java.util.Optional;
import org.jspecify.annotations.Nullable;
import org.jspecify.nullness.Nullable;
import tech.picnic.errorprone.bugpatterns.util.SourceCode;
/**
@@ -59,7 +59,6 @@ import tech.picnic.errorprone.bugpatterns.util.SourceCode;
public final class FormatStringConcatenation extends BugChecker
implements MethodInvocationTreeMatcher {
private static final long serialVersionUID = 1L;
/**
* AssertJ exposes varargs {@code fail} methods with a {@link Throwable}-accepting overload, the
* latter of which should not be flagged.
@@ -69,7 +68,6 @@ public final class FormatStringConcatenation extends BugChecker
.anyClass()
.withAnyName()
.withParameters(String.class.getName(), Throwable.class.getName());
// XXX: Drop some of these methods if we use Refaster to replace some with others.
private static final Matcher<ExpressionTree> ASSERTJ_FORMAT_METHOD =
anyOf(

View File

@@ -23,7 +23,6 @@ import com.google.errorprone.matchers.Matcher;
import com.google.errorprone.util.ASTHelpers;
import com.google.errorprone.util.ASTHelpers.TargetType;
import com.sun.source.tree.ExpressionTree;
import com.sun.source.tree.MemberSelectTree;
import com.sun.source.tree.MethodInvocationTree;
import com.sun.tools.javac.code.Type;
import com.sun.tools.javac.code.Types;
@@ -33,10 +32,8 @@ import tech.picnic.errorprone.bugpatterns.util.SourceCode;
/** A {@link BugChecker} that flags redundant identity conversions. */
// XXX: Consider detecting cases where a flagged expression is passed to a method, and where removal
// of the identity conversion would cause a different method overload to be selected. Depending on
// of the identify conversion would cause a different method overload to be selected. Depending on
// the target method such a modification may change the code's semantics or performance.
// XXX: Also flag `Stream#map`, `Mono#map` and `Flux#map` invocations where the given transformation
// is effectively the identity operation.
@AutoService(BugChecker.class)
@BugPattern(
summary = "Avoid or clarify identity conversions",
@@ -48,13 +45,6 @@ public final class IdentityConversion extends BugChecker implements MethodInvoca
private static final long serialVersionUID = 1L;
private static final Matcher<ExpressionTree> IS_CONVERSION_METHOD =
anyOf(
staticMethod()
.onClassAny(
Primitives.allWrapperTypes().stream()
.map(Class::getName)
.collect(toImmutableSet()))
.named("valueOf"),
staticMethod().onClass(String.class.getName()).named("valueOf"),
staticMethod()
.onClassAny(
"com.google.common.collect.ImmutableBiMap",
@@ -70,8 +60,12 @@ public final class IdentityConversion extends BugChecker implements MethodInvoca
"com.google.common.collect.ImmutableTable")
.named("copyOf"),
staticMethod()
.onClass("com.google.errorprone.matchers.Matchers")
.namedAnyOf("allOf", "anyOf"),
.onClassAny(
Primitives.allWrapperTypes().stream()
.map(Class::getName)
.collect(toImmutableSet()))
.named("valueOf"),
staticMethod().onClass(String.class.getName()).named("valueOf"),
staticMethod().onClass("reactor.adapter.rxjava.RxJava2Adapter"),
staticMethod()
.onClass("reactor.core.publisher.Flux")
@@ -101,15 +95,6 @@ public final class IdentityConversion extends BugChecker implements MethodInvoca
return Description.NO_MATCH;
}
if (sourceType.isPrimitive()
&& state.getPath().getParentPath().getLeaf() instanceof MemberSelectTree) {
/*
* The result of the conversion method is dereferenced, while the source type is a primitive:
* dropping the conversion would yield uncompilable code.
*/
return Description.NO_MATCH;
}
return buildDescription(tree)
.setMessage(
"This method invocation appears redundant; remove it or suppress this warning and "

View File

@@ -6,18 +6,15 @@ import static com.google.errorprone.BugPattern.StandardTags.SIMPLIFICATION;
import static tech.picnic.errorprone.bugpatterns.util.Documentation.BUG_PATTERNS_BASE_URL;
import com.google.auto.service.AutoService;
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.LambdaExpressionTreeMatcher;
import com.google.errorprone.fixes.SuggestedFix;
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.bugpatterns.util.SourceCode;
/**
@@ -42,19 +39,15 @@ 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) {
return Description.NO_MATCH;
}
VariableTree param = Iterables.getOnlyElement(tree.getParameters());
InstanceOfTree instanceOf = (InstanceOfTree) tree.getBody();
if (!ASTHelpers.getSymbol(param).equals(ASTHelpers.getSymbol(instanceOf.getExpression()))) {
if (tree.getKind() != Kind.LAMBDA_EXPRESSION || tree.getBody().getKind() != Kind.INSTANCE_OF) {
return Description.NO_MATCH;
}
return describeMatch(
tree,
SuggestedFix.replace(
tree, SourceCode.treeToString(instanceOf.getType(), state) + ".class::isInstance"));
tree,
SourceCode.treeToString(((InstanceOfTree) tree.getBody()).getType(), state)
+ ".class::isInstance"));
}
}

View File

@@ -1,82 +0,0 @@
package tech.picnic.errorprone.bugpatterns;
import static com.google.errorprone.BugPattern.LinkType.CUSTOM;
import static com.google.errorprone.BugPattern.SeverityLevel.SUGGESTION;
import static com.google.errorprone.BugPattern.StandardTags.STYLE;
import static com.google.errorprone.matchers.ChildMultiMatcher.MatchType.AT_LEAST_ONE;
import static com.google.errorprone.matchers.Matchers.allOf;
import static com.google.errorprone.matchers.Matchers.annotations;
import static com.google.errorprone.matchers.Matchers.anyOf;
import static com.google.errorprone.matchers.Matchers.hasMethod;
import static com.google.errorprone.matchers.Matchers.hasModifier;
import static com.google.errorprone.matchers.Matchers.isType;
import static com.google.errorprone.matchers.Matchers.not;
import static tech.picnic.errorprone.bugpatterns.util.Documentation.BUG_PATTERNS_BASE_URL;
import static tech.picnic.errorprone.bugpatterns.util.MoreJUnitMatchers.TEST_METHOD;
import static tech.picnic.errorprone.bugpatterns.util.MoreMatchers.hasMetaAnnotation;
import com.google.auto.service.AutoService;
import com.google.common.collect.ImmutableSet;
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.Description;
import com.google.errorprone.matchers.Matcher;
import com.sun.source.tree.ClassTree;
import javax.lang.model.element.Modifier;
/**
* A {@link BugChecker} that flags non-final and non package-private JUnit test class declarations,
* unless abstract.
*/
@AutoService(BugChecker.class)
@BugPattern(
summary = "Non-abstract JUnit test classes should be declared package-private and final",
linkType = CUSTOM,
link = BUG_PATTERNS_BASE_URL + "JUnitClassModifiers",
severity = SUGGESTION,
tags = STYLE)
public final class JUnitClassModifiers extends BugChecker implements ClassTreeMatcher {
private static final long serialVersionUID = 1L;
private static final Matcher<ClassTree> HAS_SPRING_CONFIGURATION_ANNOTATION =
annotations(
AT_LEAST_ONE,
anyOf(
isType("org.springframework.context.annotation.Configuration"),
hasMetaAnnotation("org.springframework.context.annotation.Configuration")));
private static final Matcher<ClassTree> TEST_CLASS_WITH_INCORRECT_MODIFIERS =
allOf(
hasMethod(TEST_METHOD),
not(hasModifier(Modifier.ABSTRACT)),
anyOf(
hasModifier(Modifier.PRIVATE),
hasModifier(Modifier.PROTECTED),
hasModifier(Modifier.PUBLIC),
allOf(not(hasModifier(Modifier.FINAL)), not(HAS_SPRING_CONFIGURATION_ANNOTATION))));
/** Instantiates a new {@link JUnitClassModifiers} instance. */
public JUnitClassModifiers() {}
@Override
public Description matchClass(ClassTree tree, VisitorState state) {
if (!TEST_CLASS_WITH_INCORRECT_MODIFIERS.matches(tree, state)) {
return Description.NO_MATCH;
}
SuggestedFix.Builder fixBuilder = SuggestedFix.builder();
SuggestedFixes.removeModifiers(
tree.getModifiers(),
state,
ImmutableSet.of(Modifier.PRIVATE, Modifier.PROTECTED, Modifier.PUBLIC))
.ifPresent(fixBuilder::merge);
if (!HAS_SPRING_CONFIGURATION_ANNOTATION.matches(tree, state)) {
SuggestedFixes.addModifiers(tree, state, Modifier.FINAL).ifPresent(fixBuilder::merge);
}
return describeMatch(tree, fixBuilder.build());
}
}

View File

@@ -3,18 +3,19 @@ package tech.picnic.errorprone.bugpatterns;
import static com.google.errorprone.BugPattern.LinkType.CUSTOM;
import static com.google.errorprone.BugPattern.SeverityLevel.SUGGESTION;
import static com.google.errorprone.BugPattern.StandardTags.SIMPLIFICATION;
import static com.google.errorprone.matchers.ChildMultiMatcher.MatchType.AT_LEAST_ONE;
import static com.google.errorprone.matchers.Matchers.allOf;
import static com.google.errorprone.matchers.Matchers.annotations;
import static com.google.errorprone.matchers.Matchers.anyOf;
import static com.google.errorprone.matchers.Matchers.enclosingClass;
import static com.google.errorprone.matchers.Matchers.hasModifier;
import static com.google.errorprone.matchers.Matchers.not;
import static com.google.errorprone.matchers.Matchers.isType;
import static java.util.function.Predicate.not;
import static tech.picnic.errorprone.bugpatterns.util.Documentation.BUG_PATTERNS_BASE_URL;
import static tech.picnic.errorprone.bugpatterns.util.MoreJUnitMatchers.SETUP_OR_TEARDOWN_METHOD;
import static tech.picnic.errorprone.bugpatterns.util.MoreJUnitMatchers.TEST_METHOD;
import static tech.picnic.errorprone.bugpatterns.util.JavaKeywords.isReservedKeyword;
import com.google.auto.service.AutoService;
import com.google.common.collect.ImmutableSet;
import com.google.common.collect.Sets;
import com.google.errorprone.BugPattern;
import com.google.errorprone.VisitorState;
import com.google.errorprone.bugpatterns.BugChecker;
@@ -23,12 +24,20 @@ import com.google.errorprone.fixes.SuggestedFix;
import com.google.errorprone.fixes.SuggestedFixes;
import com.google.errorprone.matchers.Description;
import com.google.errorprone.matchers.Matcher;
import com.google.errorprone.matchers.Matchers;
import com.google.errorprone.matchers.MultiMatcher;
import com.google.errorprone.predicates.TypePredicate;
import com.google.errorprone.util.ASTHelpers;
import com.sun.source.tree.AnnotationTree;
import com.sun.source.tree.ClassTree;
import com.sun.source.tree.ImportTree;
import com.sun.source.tree.MethodTree;
import com.sun.tools.javac.code.Symbol.MethodSymbol;
import com.sun.source.tree.Tree;
import com.sun.tools.javac.code.Symbol;
import java.util.Optional;
import javax.lang.model.element.Modifier;
import tech.picnic.errorprone.bugpatterns.util.ConflictDetection;
import javax.lang.model.element.Name;
import tech.picnic.errorprone.bugpatterns.util.SourceCode;
/** A {@link BugChecker} that flags non-canonical JUnit method declarations. */
// XXX: Consider introducing a class-level check that enforces that test classes:
@@ -47,19 +56,35 @@ public final class JUnitMethodDeclaration extends BugChecker implements MethodTr
private static final long serialVersionUID = 1L;
private static final String TEST_PREFIX = "test";
private static final ImmutableSet<Modifier> ILLEGAL_MODIFIERS =
Sets.immutableEnumSet(Modifier.PRIVATE, Modifier.PROTECTED, Modifier.PUBLIC);
private static final Matcher<MethodTree> IS_LIKELY_OVERRIDDEN =
allOf(
not(hasModifier(Modifier.FINAL)),
not(hasModifier(Modifier.PRIVATE)),
enclosingClass(hasModifier(Modifier.ABSTRACT)));
ImmutableSet.of(Modifier.PRIVATE, Modifier.PROTECTED, Modifier.PUBLIC);
private static final Matcher<MethodTree> HAS_UNMODIFIABLE_SIGNATURE =
anyOf(
annotations(AT_LEAST_ONE, isType("java.lang.Override")),
allOf(
Matchers.not(hasModifier(Modifier.FINAL)),
Matchers.not(hasModifier(Modifier.PRIVATE)),
enclosingClass(hasModifier(Modifier.ABSTRACT))));
private static final MultiMatcher<MethodTree, AnnotationTree> TEST_METHOD =
annotations(
AT_LEAST_ONE,
anyOf(
isType("org.junit.jupiter.api.Test"),
hasMetaAnnotation("org.junit.jupiter.api.TestTemplate")));
private static final MultiMatcher<MethodTree, AnnotationTree> SETUP_OR_TEARDOWN_METHOD =
annotations(
AT_LEAST_ONE,
anyOf(
isType("org.junit.jupiter.api.AfterAll"),
isType("org.junit.jupiter.api.AfterEach"),
isType("org.junit.jupiter.api.BeforeAll"),
isType("org.junit.jupiter.api.BeforeEach")));
/** Instantiates a new {@link JUnitMethodDeclaration} instance. */
public JUnitMethodDeclaration() {}
@Override
public Description matchMethod(MethodTree tree, VisitorState state) {
if (IS_LIKELY_OVERRIDDEN.matches(tree, state) || isOverride(tree, state)) {
if (HAS_UNMODIFIABLE_SIGNATURE.matches(tree, state)) {
return Description.NO_MATCH;
}
@@ -81,11 +106,10 @@ public final class JUnitMethodDeclaration extends BugChecker implements MethodTr
private void suggestTestMethodRenameIfApplicable(
MethodTree tree, SuggestedFix.Builder fixBuilder, VisitorState state) {
MethodSymbol symbol = ASTHelpers.getSymbol(tree);
tryCanonicalizeMethodName(symbol)
tryCanonicalizeMethodName(tree)
.ifPresent(
newName ->
ConflictDetection.findMethodRenameBlocker(symbol, newName, state)
findMethodRenameBlocker(newName, state)
.ifPresentOrElse(
blocker -> reportMethodRenameBlocker(tree, blocker, state),
() -> fixBuilder.merge(SuggestedFixes.renameMethod(tree, newName, state))));
@@ -101,8 +125,63 @@ public final class JUnitMethodDeclaration extends BugChecker implements MethodTr
.build());
}
private static Optional<String> tryCanonicalizeMethodName(MethodSymbol symbol) {
return Optional.of(symbol.getQualifiedName().toString())
/**
* If applicable, returns a human-readable argument against assigning the given name to an
* existing method.
*
* <p>This method implements imperfect heuristics. Things it currently does not consider include
* the following:
*
* <ul>
* <li>Whether the rename would merely introduce a method overload, rather than clashing with an
* existing method declaration.
* <li>Whether the rename would cause a method in a superclass to be overridden.
* <li>Whether the rename would in fact clash with a static import. (It could be that a static
* import of the same name is only referenced from lexical scopes in which the method under
* consideration cannot be referenced directly.)
* </ul>
*/
private static Optional<String> findMethodRenameBlocker(String methodName, VisitorState state) {
if (isMethodInEnclosingClass(methodName, state)) {
return Optional.of(
String.format("a method named `%s` already exists in this class", methodName));
}
if (isSimpleNameStaticallyImported(methodName, state)) {
return Optional.of(String.format("`%s` is already statically imported", methodName));
}
if (isReservedKeyword(methodName)) {
return Optional.of(String.format("`%s` is a reserved keyword", methodName));
}
return Optional.empty();
}
private static boolean isMethodInEnclosingClass(String methodName, VisitorState state) {
return state.findEnclosing(ClassTree.class).getMembers().stream()
.filter(MethodTree.class::isInstance)
.map(MethodTree.class::cast)
.map(MethodTree::getName)
.map(Name::toString)
.anyMatch(methodName::equals);
}
private static boolean isSimpleNameStaticallyImported(String simpleName, VisitorState state) {
return state.getPath().getCompilationUnit().getImports().stream()
.filter(ImportTree::isStatic)
.map(ImportTree::getQualifiedIdentifier)
.map(tree -> getStaticImportSimpleName(tree, state))
.anyMatch(simpleName::contentEquals);
}
private static CharSequence getStaticImportSimpleName(Tree tree, VisitorState state) {
String source = SourceCode.treeToString(tree, state);
return source.subSequence(source.lastIndexOf('.') + 1, source.length());
}
private static Optional<String> tryCanonicalizeMethodName(MethodTree tree) {
return Optional.of(ASTHelpers.getSymbol(tree).getQualifiedName().toString())
.filter(name -> name.startsWith(TEST_PREFIX))
.map(name -> name.substring(TEST_PREFIX.length()))
.filter(not(String::isEmpty))
@@ -110,9 +189,17 @@ public final class JUnitMethodDeclaration extends BugChecker implements MethodTr
.filter(name -> !Character.isDigit(name.charAt(0)));
}
private static boolean isOverride(MethodTree tree, VisitorState state) {
return ASTHelpers.streamSuperMethods(ASTHelpers.getSymbol(tree), state.getTypes())
.findAny()
.isPresent();
// XXX: Move to a `MoreMatchers` utility class.
private static Matcher<AnnotationTree> hasMetaAnnotation(String annotationClassName) {
TypePredicate typePredicate = hasAnnotation(annotationClassName);
return (tree, state) -> {
Symbol sym = ASTHelpers.getSymbol(tree);
return sym != null && typePredicate.apply(sym.type, state);
};
}
// XXX: Move to a `MoreTypePredicates` utility class.
private static TypePredicate hasAnnotation(String annotationClassName) {
return (type, state) -> ASTHelpers.hasAnnotation(type.tsym, annotationClassName, state);
}
}

View File

@@ -1,307 +0,0 @@
package tech.picnic.errorprone.bugpatterns;
import static com.google.errorprone.BugPattern.LinkType.CUSTOM;
import static com.google.errorprone.BugPattern.SeverityLevel.SUGGESTION;
import static com.google.errorprone.BugPattern.StandardTags.SIMPLIFICATION;
import static com.google.errorprone.matchers.ChildMultiMatcher.MatchType.ALL;
import static com.google.errorprone.matchers.ChildMultiMatcher.MatchType.AT_LEAST_ONE;
import static com.google.errorprone.matchers.Matchers.allOf;
import static com.google.errorprone.matchers.Matchers.anyOf;
import static com.google.errorprone.matchers.Matchers.anything;
import static com.google.errorprone.matchers.Matchers.argument;
import static com.google.errorprone.matchers.Matchers.argumentCount;
import static com.google.errorprone.matchers.Matchers.classLiteral;
import static com.google.errorprone.matchers.Matchers.hasArguments;
import static com.google.errorprone.matchers.Matchers.isPrimitiveOrBoxedPrimitiveType;
import static com.google.errorprone.matchers.Matchers.isSameType;
import static com.google.errorprone.matchers.Matchers.methodHasParameters;
import static com.google.errorprone.matchers.Matchers.staticMethod;
import static com.google.errorprone.matchers.Matchers.toType;
import static java.util.function.Predicate.not;
import static java.util.stream.Collectors.joining;
import static tech.picnic.errorprone.bugpatterns.util.Documentation.BUG_PATTERNS_BASE_URL;
import static tech.picnic.errorprone.bugpatterns.util.MoreJUnitMatchers.HAS_METHOD_SOURCE;
import static tech.picnic.errorprone.bugpatterns.util.MoreJUnitMatchers.getMethodSourceFactoryNames;
import com.google.auto.service.AutoService;
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.MethodTreeMatcher;
import com.google.errorprone.fixes.SuggestedFix;
import com.google.errorprone.matchers.Description;
import com.google.errorprone.matchers.Matcher;
import com.google.errorprone.util.ASTHelpers;
import com.sun.source.tree.AnnotationTree;
import com.sun.source.tree.ClassTree;
import com.sun.source.tree.ExpressionTree;
import com.sun.source.tree.LambdaExpressionTree;
import com.sun.source.tree.MethodInvocationTree;
import com.sun.source.tree.MethodTree;
import com.sun.source.tree.NewArrayTree;
import com.sun.source.tree.ReturnTree;
import com.sun.source.util.TreeScanner;
import com.sun.tools.javac.code.Type;
import java.util.ArrayList;
import java.util.Collection;
import java.util.List;
import java.util.Locale;
import java.util.Optional;
import java.util.Set;
import java.util.function.Predicate;
import java.util.stream.DoubleStream;
import java.util.stream.IntStream;
import java.util.stream.LongStream;
import java.util.stream.Stream;
import org.jspecify.annotations.Nullable;
import tech.picnic.errorprone.bugpatterns.util.SourceCode;
/**
* A {@link BugChecker} that flags JUnit tests with a {@link
* org.junit.jupiter.params.provider.MethodSource} annotation that can be replaced with an
* equivalent {@link org.junit.jupiter.params.provider.ValueSource} annotation.
*/
// XXX: Where applicable, also flag `@MethodSource` annotations that reference multiple value
// factory methods (or that repeat the same value factory method multiple times).
// XXX: Support inlining of overloaded value factory methods.
// XXX: Support inlining of value factory methods referenced by multiple `@MethodSource`
// annotations.
// XXX: Support value factory return expressions of the form `Stream.of(a, b,
// c).map(Arguments::argument)`.
// XXX: Support simplification of test methods that accept additional injected parameters such as
// `TestInfo`; such parameters should be ignored for the purpose of this check.
@AutoService(BugChecker.class)
@BugPattern(
summary = "Prefer `@ValueSource` over a `@MethodSource` where possible and reasonable",
linkType = CUSTOM,
link = BUG_PATTERNS_BASE_URL + "JUnitValueSource",
severity = SUGGESTION,
tags = SIMPLIFICATION)
public final class JUnitValueSource extends BugChecker implements MethodTreeMatcher {
private static final long serialVersionUID = 1L;
private static final Matcher<ExpressionTree> SUPPORTED_VALUE_FACTORY_VALUES =
anyOf(
isArrayArgumentValueCandidate(),
toType(
MethodInvocationTree.class,
allOf(
staticMethod()
.onClass("org.junit.jupiter.params.provider.Arguments")
.namedAnyOf("arguments", "of"),
argumentCount(1),
argument(0, isArrayArgumentValueCandidate()))));
private static final Matcher<ExpressionTree> ARRAY_OF_SUPPORTED_SINGLE_VALUE_ARGUMENTS =
isSingleDimensionArrayCreationWithAllElementsMatching(SUPPORTED_VALUE_FACTORY_VALUES);
private static final Matcher<ExpressionTree> ENUMERATION_OF_SUPPORTED_SINGLE_VALUE_ARGUMENTS =
toType(
MethodInvocationTree.class,
allOf(
staticMethod()
.onClassAny(
Stream.class.getName(),
IntStream.class.getName(),
LongStream.class.getName(),
DoubleStream.class.getName(),
List.class.getName(),
Set.class.getName(),
"com.google.common.collect.ImmutableList",
"com.google.common.collect.ImmutableSet")
.named("of"),
hasArguments(AT_LEAST_ONE, anything()),
hasArguments(ALL, SUPPORTED_VALUE_FACTORY_VALUES)));
private static final Matcher<MethodTree> IS_UNARY_METHOD_WITH_SUPPORTED_PARAMETER =
methodHasParameters(
anyOf(
isPrimitiveOrBoxedPrimitiveType(),
isSameType(String.class),
isSameType(state -> state.getSymtab().classType)));
/** Instantiates a new {@link JUnitValueSource} instance. */
public JUnitValueSource() {}
@Override
public Description matchMethod(MethodTree tree, VisitorState state) {
if (!IS_UNARY_METHOD_WITH_SUPPORTED_PARAMETER.matches(tree, state)) {
return Description.NO_MATCH;
}
Type parameterType = ASTHelpers.getType(Iterables.getOnlyElement(tree.getParameters()));
return findMethodSourceAnnotation(tree, state)
.flatMap(
methodSourceAnnotation ->
getSoleLocalFactoryName(methodSourceAnnotation, tree)
.filter(factory -> !hasSiblingReferencingValueFactory(tree, factory, state))
.flatMap(factory -> findSiblingWithName(tree, factory, state))
.flatMap(
factoryMethod ->
tryConstructValueSourceFix(
parameterType, methodSourceAnnotation, factoryMethod, state))
.map(fix -> describeMatch(methodSourceAnnotation, fix)))
.orElse(Description.NO_MATCH);
}
/**
* Returns the name of the value factory method pointed to by the given {@code @MethodSource}
* annotation, if it (a) is the only one and (b) is a method in the same class as the annotated
* method.
*/
private static Optional<String> getSoleLocalFactoryName(
AnnotationTree methodSourceAnnotation, MethodTree method) {
return getElementIfSingleton(getMethodSourceFactoryNames(methodSourceAnnotation, method))
.filter(name -> name.indexOf('#') < 0);
}
/**
* Tells whether the given method has a sibling method in the same class that depends on the
* specified value factory method.
*/
private static boolean hasSiblingReferencingValueFactory(
MethodTree tree, String valueFactory, VisitorState state) {
return findMatchingSibling(tree, m -> hasValueFactory(m, valueFactory, state), state)
.isPresent();
}
private static Optional<MethodTree> findSiblingWithName(
MethodTree tree, String methodName, VisitorState state) {
return findMatchingSibling(tree, m -> m.getName().contentEquals(methodName), state);
}
private static Optional<MethodTree> findMatchingSibling(
MethodTree tree, Predicate<? super MethodTree> predicate, VisitorState state) {
return state.findEnclosing(ClassTree.class).getMembers().stream()
.filter(MethodTree.class::isInstance)
.map(MethodTree.class::cast)
.filter(not(tree::equals))
.filter(predicate)
.findFirst();
}
private static boolean hasValueFactory(
MethodTree tree, String valueFactoryMethodName, VisitorState state) {
return findMethodSourceAnnotation(tree, state).stream()
.anyMatch(
annotation ->
getMethodSourceFactoryNames(annotation, tree).contains(valueFactoryMethodName));
}
private static Optional<AnnotationTree> findMethodSourceAnnotation(
MethodTree tree, VisitorState state) {
return HAS_METHOD_SOURCE.multiMatchResult(tree, state).matchingNodes().stream().findFirst();
}
private static Optional<SuggestedFix> tryConstructValueSourceFix(
Type parameterType,
AnnotationTree methodSourceAnnotation,
MethodTree valueFactoryMethod,
VisitorState state) {
return getSingleReturnExpression(valueFactoryMethod)
.flatMap(expression -> tryExtractValueSourceAttributeValue(expression, state))
.map(
valueSourceAttributeValue ->
SuggestedFix.builder()
.addImport("org.junit.jupiter.params.provider.ValueSource")
.replace(
methodSourceAnnotation,
String.format(
"@ValueSource(%s = %s)",
toValueSourceAttributeName(parameterType), valueSourceAttributeValue))
.delete(valueFactoryMethod)
.build());
}
// XXX: This pattern also occurs a few times inside Error Prone; contribute upstream.
private static Optional<ExpressionTree> getSingleReturnExpression(MethodTree methodTree) {
List<ExpressionTree> returnExpressions = new ArrayList<>();
new TreeScanner<@Nullable Void, @Nullable Void>() {
@Override
public @Nullable Void visitClass(ClassTree node, @Nullable Void unused) {
/* Ignore `return` statements inside anonymous/local classes. */
return null;
}
@Override
public @Nullable Void visitReturn(ReturnTree node, @Nullable Void unused) {
returnExpressions.add(node.getExpression());
return super.visitReturn(node, unused);
}
@Override
public @Nullable Void visitLambdaExpression(
LambdaExpressionTree node, @Nullable Void unused) {
/* Ignore `return` statements inside lambda expressions. */
return null;
}
}.scan(methodTree, null);
return getElementIfSingleton(returnExpressions);
}
private static Optional<String> tryExtractValueSourceAttributeValue(
ExpressionTree tree, VisitorState state) {
List<? extends ExpressionTree> arguments;
if (ENUMERATION_OF_SUPPORTED_SINGLE_VALUE_ARGUMENTS.matches(tree, state)) {
arguments = ((MethodInvocationTree) tree).getArguments();
} else if (ARRAY_OF_SUPPORTED_SINGLE_VALUE_ARGUMENTS.matches(tree, state)) {
arguments = ((NewArrayTree) tree).getInitializers();
} else {
return Optional.empty();
}
/*
* Join the values into a comma-separated string, unwrapping `Arguments` factory method
* invocations if applicable.
*/
return Optional.of(
arguments.stream()
.map(
arg ->
arg instanceof MethodInvocationTree
? Iterables.getOnlyElement(((MethodInvocationTree) arg).getArguments())
: arg)
.map(argument -> SourceCode.treeToString(argument, state))
.collect(joining(", ")))
.map(value -> arguments.size() > 1 ? String.format("{%s}", value) : value);
}
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';
}
}
private static <T> Optional<T> getElementIfSingleton(Collection<T> collection) {
return Optional.of(collection)
.filter(elements -> elements.size() == 1)
.map(Iterables::getOnlyElement);
}
private static Matcher<ExpressionTree> isSingleDimensionArrayCreationWithAllElementsMatching(
Matcher<? super ExpressionTree> elementMatcher) {
return (tree, state) -> {
if (!(tree instanceof NewArrayTree)) {
return false;
}
NewArrayTree newArray = (NewArrayTree) tree;
return newArray.getDimensions().isEmpty()
&& !newArray.getInitializers().isEmpty()
&& newArray.getInitializers().stream()
.allMatch(element -> elementMatcher.matches(element, state));
};
}
private static Matcher<ExpressionTree> isArrayArgumentValueCandidate() {
return anyOf(classLiteral(anything()), (tree, state) -> ASTHelpers.constValue(tree) != null);
}
}

View File

@@ -40,10 +40,8 @@ import java.util.List;
import java.util.Optional;
import java.util.Set;
import java.util.stream.Stream;
import javax.inject.Inject;
import org.jspecify.annotations.Nullable;
import org.jspecify.nullness.Nullable;
import tech.picnic.errorprone.bugpatterns.util.AnnotationAttributeMatcher;
import tech.picnic.errorprone.bugpatterns.util.Flags;
import tech.picnic.errorprone.bugpatterns.util.SourceCode;
/**
@@ -59,7 +57,6 @@ import tech.picnic.errorprone.bugpatterns.util.SourceCode;
linkType = CUSTOM,
severity = SUGGESTION,
tags = STYLE)
@SuppressWarnings("java:S2160" /* Super class equality definition suffices. */)
public final class LexicographicalAnnotationAttributeListing extends BugChecker
implements AnnotationTreeMatcher {
private static final long serialVersionUID = 1L;
@@ -76,7 +73,6 @@ public final class LexicographicalAnnotationAttributeListing extends BugChecker
private static final String FLAG_PREFIX = "LexicographicalAnnotationAttributeListing:";
private static final String INCLUDED_ANNOTATIONS_FLAG = FLAG_PREFIX + "Includes";
private static final String EXCLUDED_ANNOTATIONS_FLAG = FLAG_PREFIX + "Excludes";
/**
* The splitter applied to string-typed annotation arguments prior to lexicographical sorting. By
* splitting on {@code =}, strings that represent e.g. inline Spring property declarations are
@@ -96,8 +92,7 @@ public final class LexicographicalAnnotationAttributeListing extends BugChecker
*
* @param flags Any provided command line flags.
*/
@Inject
LexicographicalAnnotationAttributeListing(ErrorProneFlags flags) {
public LexicographicalAnnotationAttributeListing(ErrorProneFlags flags) {
matcher = createAnnotationAttributeMatcher(flags);
}
@@ -220,15 +215,12 @@ public final class LexicographicalAnnotationAttributeListing extends BugChecker
private static AnnotationAttributeMatcher createAnnotationAttributeMatcher(
ErrorProneFlags flags) {
return AnnotationAttributeMatcher.create(
flags.get(INCLUDED_ANNOTATIONS_FLAG).isPresent()
? Optional.of(flags.getListOrEmpty(INCLUDED_ANNOTATIONS_FLAG))
: Optional.empty(),
excludedAnnotations(flags));
flags.getList(INCLUDED_ANNOTATIONS_FLAG), excludedAnnotations(flags));
}
private static ImmutableList<String> excludedAnnotations(ErrorProneFlags flags) {
Set<String> exclusions = new HashSet<>();
exclusions.addAll(Flags.getList(flags, EXCLUDED_ANNOTATIONS_FLAG));
flags.getList(EXCLUDED_ANNOTATIONS_FLAG).ifPresent(exclusions::addAll);
exclusions.addAll(BLACKLISTED_ANNOTATIONS);
return ImmutableList.copyOf(exclusions);
}

View File

@@ -27,7 +27,7 @@ import com.sun.tools.javac.code.Symbol;
import com.sun.tools.javac.code.TypeAnnotations.AnnotationType;
import java.util.Comparator;
import java.util.List;
import org.jspecify.annotations.Nullable;
import org.jspecify.nullness.Nullable;
import tech.picnic.errorprone.bugpatterns.util.SourceCode;
/**
@@ -46,20 +46,9 @@ import tech.picnic.errorprone.bugpatterns.util.SourceCode;
public final class LexicographicalAnnotationListing extends BugChecker
implements MethodTreeMatcher {
private static final long serialVersionUID = 1L;
/**
* A comparator that minimally reorders {@link AnnotationType}s, such that declaration annotations
* are placed before type annotations.
*/
@SuppressWarnings({
"java:S1067",
"java:S3358"
} /* Avoiding the nested ternary operator hurts readability. */)
private static final Comparator<@Nullable AnnotationType> BY_ANNOTATION_TYPE =
(a, b) ->
(a == null || a == DECLARATION) && b == TYPE
? -1
: (a == TYPE && b == DECLARATION ? 1 : 0);
(a == null || a == DECLARATION) && b == TYPE ? -1 : a == TYPE && b == DECLARATION ? 1 : 0;
/** Instantiates a new {@link LexicographicalAnnotationListing} instance. */
public LexicographicalAnnotationListing() {}

View File

@@ -118,8 +118,6 @@ 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. */)
private static Optional<SuggestedFix.Builder> constructMethodRef(
LambdaExpressionTree lambdaExpr,
MethodInvocationTree subTree,
@@ -132,9 +130,10 @@ public final class MethodReferenceUsage extends BugChecker implements LambdaExpr
return Optional.empty();
}
Symbol sym = ASTHelpers.getSymbol(methodSelect);
return ASTHelpers.isStatic(sym)
? constructFix(lambdaExpr, sym.owner, methodSelect)
: constructFix(lambdaExpr, "this", methodSelect);
if (!ASTHelpers.isStatic(sym)) {
return constructFix(lambdaExpr, "this", methodSelect);
}
return constructFix(lambdaExpr, sym.owner, methodSelect);
case MEMBER_SELECT:
return constructMethodRef(lambdaExpr, (MemberSelectTree) methodSelect, expectedInstance);
default:

View File

@@ -0,0 +1,59 @@
package tech.picnic.errorprone.bugpatterns;
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.anyOf;
import static com.google.errorprone.matchers.Matchers.isType;
import static tech.picnic.errorprone.bugpatterns.util.Documentation.BUG_PATTERNS_BASE_URL;
import com.google.auto.service.AutoService;
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.matchers.Description;
import com.google.errorprone.matchers.MultiMatcher;
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;
/** A {@link BugChecker} that flags likely missing Refaster annotations. */
@AutoService(BugChecker.class)
@BugPattern(
summary = "The Refaster rule contains a method without any Refaster annotations",
link = BUG_PATTERNS_BASE_URL + "MissingRefasterAnnotation",
linkType = CUSTOM,
severity = WARNING,
tags = LIKELY_ERROR)
public final class MissingRefasterAnnotation extends BugChecker implements ClassTreeMatcher {
private static final long serialVersionUID = 1L;
private static final MultiMatcher<Tree, AnnotationTree> REFASTER_ANNOTATION =
annotations(
AT_LEAST_ONE,
anyOf(
isType("com.google.errorprone.refaster.annotation.Placeholder"),
isType("com.google.errorprone.refaster.annotation.BeforeTemplate"),
isType("com.google.errorprone.refaster.annotation.AfterTemplate")));
/** Instantiates a new {@link MissingRefasterAnnotation} instance. */
public MissingRefasterAnnotation() {}
@Override
public Description matchClass(ClassTree tree, VisitorState state) {
long methodTypes =
tree.getMembers().stream()
.filter(member -> member.getKind() == Tree.Kind.METHOD)
.map(MethodTree.class::cast)
.filter(method -> !ASTHelpers.isGeneratedConstructor(method))
.map(method -> REFASTER_ANNOTATION.matches(method, state))
.distinct()
.count();
return methodTypes < 2 ? Description.NO_MATCH : buildDescription(tree).build();
}
}

View File

@@ -1,86 +0,0 @@
package tech.picnic.errorprone.bugpatterns;
import static com.google.errorprone.BugPattern.LinkType.CUSTOM;
import static com.google.errorprone.BugPattern.SeverityLevel.SUGGESTION;
import static com.google.errorprone.BugPattern.StandardTags.SIMPLIFICATION;
import static com.google.errorprone.matchers.Matchers.allOf;
import static com.google.errorprone.matchers.Matchers.argument;
import static com.google.errorprone.matchers.Matchers.isSameType;
import static com.google.errorprone.matchers.Matchers.isVariable;
import static com.google.errorprone.matchers.Matchers.not;
import static com.google.errorprone.matchers.Matchers.staticMethod;
import static tech.picnic.errorprone.bugpatterns.util.Documentation.BUG_PATTERNS_BASE_URL;
import com.google.auto.service.AutoService;
import com.google.errorprone.BugPattern;
import com.google.errorprone.VisitorState;
import com.google.errorprone.bugpatterns.BugChecker;
import com.google.errorprone.bugpatterns.BugChecker.MethodInvocationTreeMatcher;
import com.google.errorprone.fixes.SuggestedFixes;
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.MethodInvocationTree;
import com.sun.source.tree.Tree;
import com.sun.source.tree.VariableTree;
import java.util.List;
import tech.picnic.errorprone.bugpatterns.util.MoreASTHelpers;
/**
* A {@link BugChecker} that flags the use of {@link org.mockito.Mockito#mock(Class)} and {@link
* org.mockito.Mockito#spy(Class)} where instead the type to be mocked or spied can be derived from
* context.
*/
// XXX: This check currently does not flag method invocation arguments. When adding support for
// this, consider that in some cases the type to be mocked or spied must be specified explicitly so
// as to disambiguate between method overloads.
// XXX: This check currently does not flag (implicit or explicit) lambda return expressions.
// XXX: This check currently does not drop suppressions that become obsolete after the
// suggested fix is applied; consider adding support for this.
@AutoService(BugChecker.class)
@BugPattern(
summary = "Don't unnecessarily pass a type to Mockito's `mock(Class)` and `spy(Class)` methods",
link = BUG_PATTERNS_BASE_URL + "MockitoMockClassReference",
linkType = CUSTOM,
severity = SUGGESTION,
tags = SIMPLIFICATION)
public final class MockitoMockClassReference extends BugChecker
implements MethodInvocationTreeMatcher {
private static final long serialVersionUID = 1L;
private static final Matcher<MethodInvocationTree> MOCKITO_MOCK_OR_SPY_WITH_HARDCODED_TYPE =
allOf(
argument(0, allOf(isSameType(Class.class.getName()), not(isVariable()))),
staticMethod().onClass("org.mockito.Mockito").namedAnyOf("mock", "spy"));
/** Instantiates a new {@link MockitoMockClassReference} instance. */
public MockitoMockClassReference() {}
@Override
public Description matchMethodInvocation(MethodInvocationTree tree, VisitorState state) {
if (!MOCKITO_MOCK_OR_SPY_WITH_HARDCODED_TYPE.matches(tree, state)
|| !isTypeDerivableFromContext(tree, state)) {
return Description.NO_MATCH;
}
List<? extends ExpressionTree> arguments = tree.getArguments();
return describeMatch(tree, SuggestedFixes.removeElement(arguments.get(0), arguments, state));
}
private static boolean isTypeDerivableFromContext(MethodInvocationTree tree, VisitorState state) {
Tree parent = state.getPath().getParentPath().getLeaf();
switch (parent.getKind()) {
case VARIABLE:
return !ASTHelpers.hasNoExplicitType((VariableTree) parent, state)
&& MoreASTHelpers.areSameType(tree, parent, state);
case ASSIGNMENT:
return MoreASTHelpers.areSameType(tree, parent, state);
case RETURN:
return MoreASTHelpers.findMethodExitedOnReturn(state)
.filter(m -> MoreASTHelpers.areSameType(tree, m.getReturnType(), state))
.isPresent();
default:
return false;
}
}
}

View File

@@ -1,47 +0,0 @@
package tech.picnic.errorprone.bugpatterns;
import static com.google.errorprone.BugPattern.LinkType.CUSTOM;
import static com.google.errorprone.BugPattern.SeverityLevel.SUGGESTION;
import static com.google.errorprone.BugPattern.StandardTags.PERFORMANCE;
import static com.google.errorprone.matchers.method.MethodMatchers.staticMethod;
import static tech.picnic.errorprone.bugpatterns.util.Documentation.BUG_PATTERNS_BASE_URL;
import com.google.auto.service.AutoService;
import com.google.errorprone.BugPattern;
import com.google.errorprone.VisitorState;
import com.google.errorprone.bugpatterns.BugChecker;
import com.google.errorprone.bugpatterns.BugChecker.MethodInvocationTreeMatcher;
import com.google.errorprone.matchers.Description;
import com.google.errorprone.matchers.Matcher;
import com.sun.source.tree.ExpressionTree;
import com.sun.source.tree.MethodInvocationTree;
/**
* A {@link BugChecker} that flags usages of MongoDB {@code $text} filter usages.
*
* @see <a href="https://www.mongodb.com/docs/manual/text-search/">MongoDB Text Search</a>
*/
@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",
link = BUG_PATTERNS_BASE_URL + "MongoDBTextFilterUsage",
linkType = CUSTOM,
severity = SUGGESTION,
tags = PERFORMANCE)
public final class MongoDBTextFilterUsage extends BugChecker
implements MethodInvocationTreeMatcher {
private static final long serialVersionUID = 1L;
private static final Matcher<ExpressionTree> MONGO_FILTERS_TEXT_METHOD =
staticMethod().onClass("com.mongodb.client.model.Filters").named("text");
/** Instantiates a new {@link MongoDBTextFilterUsage} instance. */
public MongoDBTextFilterUsage() {}
@Override
public Description matchMethodInvocation(MethodInvocationTree tree, VisitorState state) {
return MONGO_FILTERS_TEXT_METHOD.matches(tree, state)
? describeMatch(tree)
: Description.NO_MATCH;
}
}

View File

@@ -4,7 +4,6 @@ import static com.google.errorprone.BugPattern.LinkType.CUSTOM;
import static com.google.errorprone.BugPattern.SeverityLevel.WARNING;
import static com.google.errorprone.BugPattern.StandardTags.FRAGILE_CODE;
import static tech.picnic.errorprone.bugpatterns.util.Documentation.BUG_PATTERNS_BASE_URL;
import static tech.picnic.errorprone.bugpatterns.util.MoreMatchers.isSubTypeOf;
import static tech.picnic.errorprone.bugpatterns.util.MoreTypes.generic;
import static tech.picnic.errorprone.bugpatterns.util.MoreTypes.raw;
import static tech.picnic.errorprone.bugpatterns.util.MoreTypes.subOf;
@@ -15,19 +14,14 @@ import com.google.errorprone.VisitorState;
import com.google.errorprone.bugpatterns.BugChecker;
import com.google.errorprone.bugpatterns.BugChecker.MethodInvocationTreeMatcher;
import com.google.errorprone.matchers.Description;
import com.google.errorprone.matchers.Matcher;
import com.google.errorprone.suppliers.Supplier;
import com.google.errorprone.suppliers.Suppliers;
import com.google.errorprone.util.ASTHelpers;
import com.sun.source.tree.MethodInvocationTree;
import com.sun.source.tree.Tree;
import com.sun.tools.javac.code.Type;
import java.util.Optional;
/** A {@link BugChecker} that flags nesting of {@link Optional Optionals}. */
// XXX: Extend this checker to also flag method return types and variable/field types.
// XXX: Consider generalizing this checker and `NestedPublishers` to a single `NestedMonad` check,
// which e.g. also flags nested `Stream`s. Alternatively, combine these and other checkers into an
// even more generic `ConfusingType` checker.
@AutoService(BugChecker.class)
@BugPattern(
summary =
@@ -39,15 +33,15 @@ import java.util.Optional;
public final class NestedOptionals extends BugChecker implements MethodInvocationTreeMatcher {
private static final long serialVersionUID = 1L;
private static final Supplier<Type> OPTIONAL = Suppliers.typeFromClass(Optional.class);
private static final Matcher<Tree> IS_OPTIONAL_OF_OPTIONAL =
isSubTypeOf(VisitorState.memoize(generic(OPTIONAL, subOf(raw(OPTIONAL)))));
private static final Supplier<Type> OPTIONAL_OF_OPTIONAL =
VisitorState.memoize(generic(OPTIONAL, subOf(raw(OPTIONAL))));
/** Instantiates a new {@link NestedOptionals} instance. */
public NestedOptionals() {}
@Override
public Description matchMethodInvocation(MethodInvocationTree tree, VisitorState state) {
return IS_OPTIONAL_OF_OPTIONAL.matches(tree, state)
return state.getTypes().isSubtype(ASTHelpers.getType(tree), OPTIONAL_OF_OPTIONAL.get(state))
? describeMatch(tree)
: Description.NO_MATCH;
}

View File

@@ -1,63 +0,0 @@
package tech.picnic.errorprone.bugpatterns;
import static com.google.errorprone.BugPattern.LinkType.CUSTOM;
import static com.google.errorprone.BugPattern.SeverityLevel.WARNING;
import static com.google.errorprone.BugPattern.StandardTags.FRAGILE_CODE;
import static com.google.errorprone.matchers.Matchers.typePredicateMatcher;
import static com.google.errorprone.predicates.TypePredicates.allOf;
import static com.google.errorprone.predicates.TypePredicates.not;
import static tech.picnic.errorprone.bugpatterns.util.Documentation.BUG_PATTERNS_BASE_URL;
import static tech.picnic.errorprone.bugpatterns.util.MoreTypePredicates.hasTypeParameter;
import static tech.picnic.errorprone.bugpatterns.util.MoreTypePredicates.isSubTypeOf;
import static tech.picnic.errorprone.bugpatterns.util.MoreTypes.generic;
import static tech.picnic.errorprone.bugpatterns.util.MoreTypes.raw;
import static tech.picnic.errorprone.bugpatterns.util.MoreTypes.subOf;
import static tech.picnic.errorprone.bugpatterns.util.MoreTypes.type;
import com.google.auto.service.AutoService;
import com.google.errorprone.BugPattern;
import com.google.errorprone.VisitorState;
import com.google.errorprone.bugpatterns.BugChecker;
import com.google.errorprone.bugpatterns.BugChecker.MethodInvocationTreeMatcher;
import com.google.errorprone.matchers.Description;
import com.google.errorprone.matchers.Matcher;
import com.google.errorprone.suppliers.Supplier;
import com.sun.source.tree.ExpressionTree;
import com.sun.source.tree.MethodInvocationTree;
import com.sun.tools.javac.code.Type;
/**
* A {@link BugChecker} that flags {@code Publisher<? extends Publisher>} instances, unless the
* nested {@link org.reactivestreams.Publisher} is a {@link reactor.core.publisher.GroupedFlux}.
*/
// XXX: See the `NestedOptionals` check for some ideas on how to generalize this kind of checker.
@AutoService(BugChecker.class)
@BugPattern(
summary =
"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,
tags = FRAGILE_CODE)
public final class NestedPublishers extends BugChecker implements MethodInvocationTreeMatcher {
private static final long serialVersionUID = 1L;
private static final Supplier<Type> PUBLISHER = type("org.reactivestreams.Publisher");
private static final Matcher<ExpressionTree> IS_NON_GROUPED_PUBLISHER_OF_PUBLISHERS =
typePredicateMatcher(
allOf(
isSubTypeOf(generic(PUBLISHER, subOf(raw(PUBLISHER)))),
not(
hasTypeParameter(
0, isSubTypeOf(raw(type("reactor.core.publisher.GroupedFlux")))))));
/** Instantiates a new {@link NestedPublishers} instance. */
public NestedPublishers() {}
@Override
public Description matchMethodInvocation(MethodInvocationTree tree, VisitorState state) {
return IS_NON_GROUPED_PUBLISHER_OF_PUBLISHERS.matches(tree, state)
? describeMatch(tree)
: Description.NO_MATCH;
}
}

View File

@@ -43,7 +43,6 @@ import tech.picnic.errorprone.bugpatterns.util.SourceCode;
// emitting a value (e.g. `Mono.empty()`, `someFlux.then()`, ...), or known not to complete normally
// (`Mono.never()`, `someFlux.repeat()`, `Mono.error(...)`, ...). The latter category could
// potentially be split out further.
@SuppressWarnings("java:S1192" /* Factoring out repeated method names impacts readability. */)
public final class NonEmptyMono extends BugChecker implements MethodInvocationTreeMatcher {
private static final long serialVersionUID = 1L;
private static final Matcher<ExpressionTree> MONO_SIZE_CHECK =

View File

@@ -49,7 +49,6 @@ import tech.picnic.errorprone.bugpatterns.util.SourceCode;
linkType = CUSTOM,
severity = WARNING,
tags = PERFORMANCE)
@SuppressWarnings("java:S1192" /* Factoring out repeated method names impacts readability. */)
public final class PrimitiveComparison extends BugChecker implements MethodInvocationTreeMatcher {
private static final long serialVersionUID = 1L;
private static final Matcher<ExpressionTree> STATIC_COMPARISON_METHOD =

View File

@@ -7,7 +7,6 @@ import static com.google.errorprone.BugPattern.StandardTags.SIMPLIFICATION;
import static com.google.errorprone.matchers.Matchers.allOf;
import static com.google.errorprone.matchers.Matchers.anyMethod;
import static com.google.errorprone.matchers.Matchers.anyOf;
import static com.google.errorprone.matchers.Matchers.anything;
import static com.google.errorprone.matchers.Matchers.argumentCount;
import static com.google.errorprone.matchers.Matchers.isNonNullUsingDataflow;
import static com.google.errorprone.matchers.Matchers.isSameType;
@@ -50,8 +49,6 @@ import java.util.Locale;
import java.util.Objects;
import java.util.Optional;
import java.util.stream.Stream;
import javax.inject.Inject;
import tech.picnic.errorprone.bugpatterns.util.Flags;
import tech.picnic.errorprone.bugpatterns.util.MethodMatcherFactory;
import tech.picnic.errorprone.bugpatterns.util.SourceCode;
@@ -63,18 +60,16 @@ import tech.picnic.errorprone.bugpatterns.util.SourceCode;
linkType = CUSTOM,
severity = SUGGESTION,
tags = SIMPLIFICATION)
@SuppressWarnings({
"java:S1192" /* Factoring out repeated method names impacts readability. */,
"java:S2160" /* Super class equality definition suffices. */,
"key-to-resolve-AnnotationUseStyle-and-TrailingComment-check-conflict"
})
public final class RedundantStringConversion extends BugChecker
implements BinaryTreeMatcher, CompoundAssignmentTreeMatcher, MethodInvocationTreeMatcher {
private static final long serialVersionUID = 1L;
private static final String FLAG_PREFIX = "RedundantStringConversion:";
private static final String EXTRA_STRING_CONVERSION_METHODS_FLAG =
"RedundantStringConversion:ExtraConversionMethods";
FLAG_PREFIX + "ExtraConversionMethods";
@SuppressWarnings("UnnecessaryLambda")
private static final Matcher<ExpressionTree> ANY_EXPR = (t, s) -> true;
private static final Matcher<ExpressionTree> ANY_EXPR = anything();
private static final Matcher<ExpressionTree> LOCALE = isSameType(Locale.class);
private static final Matcher<ExpressionTree> MARKER = isSubtypeOf("org.slf4j.Marker");
private static final Matcher<ExpressionTree> STRING = isSameType(String.class);
@@ -156,8 +151,7 @@ public final class RedundantStringConversion extends BugChecker
*
* @param flags Any provided command line flags.
*/
@Inject
RedundantStringConversion(ErrorProneFlags flags) {
public RedundantStringConversion(ErrorProneFlags flags) {
conversionMethodMatcher = createConversionMethodMatcher(flags);
}
@@ -170,7 +164,7 @@ public final class RedundantStringConversion extends BugChecker
ExpressionTree lhs = tree.getLeftOperand();
ExpressionTree rhs = tree.getRightOperand();
if (!STRING.matches(lhs, state)) {
return createDescription(tree, tryFix(rhs, state, STRING));
return finalize(tree, tryFix(rhs, state, STRING));
}
List<SuggestedFix.Builder> fixes = new ArrayList<>();
@@ -184,7 +178,7 @@ public final class RedundantStringConversion extends BugChecker
}
tryFix(rhs, state, ANY_EXPR).ifPresent(fixes::add);
return createDescription(tree, fixes.stream().reduce(SuggestedFix.Builder::merge));
return finalize(tree, fixes.stream().reduce(SuggestedFix.Builder::merge));
}
@Override
@@ -193,36 +187,36 @@ public final class RedundantStringConversion extends BugChecker
return Description.NO_MATCH;
}
return createDescription(tree, tryFix(tree.getExpression(), state, ANY_EXPR));
return finalize(tree, tryFix(tree.getExpression(), state, ANY_EXPR));
}
@Override
public Description matchMethodInvocation(MethodInvocationTree tree, VisitorState state) {
if (STRINGBUILDER_APPEND_INVOCATION.matches(tree, state)) {
return createDescription(tree, tryFixPositionalConverter(tree.getArguments(), state, 0));
return finalize(tree, tryFixPositionalConverter(tree.getArguments(), state, 0));
}
if (STRINGBUILDER_INSERT_INVOCATION.matches(tree, state)) {
return createDescription(tree, tryFixPositionalConverter(tree.getArguments(), state, 1));
return finalize(tree, tryFixPositionalConverter(tree.getArguments(), state, 1));
}
if (FORMATTER_INVOCATION.matches(tree, state)) {
return createDescription(tree, tryFixFormatter(tree.getArguments(), state));
return finalize(tree, tryFixFormatter(tree.getArguments(), state));
}
if (GUAVA_GUARD_INVOCATION.matches(tree, state)) {
return createDescription(tree, tryFixGuavaGuard(tree.getArguments(), state));
return finalize(tree, tryFixGuavaGuard(tree.getArguments(), state));
}
if (SLF4J_LOGGER_INVOCATION.matches(tree, state)) {
return createDescription(tree, tryFixSlf4jLogger(tree.getArguments(), state));
return finalize(tree, tryFixSlf4jLogger(tree.getArguments(), state));
}
if (instanceMethod().matches(tree, state)) {
return createDescription(tree, tryFix(tree, state, STRING));
return finalize(tree, tryFix(tree, state, STRING));
}
return createDescription(tree, tryFix(tree, state, NON_NULL_STRING));
return finalize(tree, tryFix(tree, state, NON_NULL_STRING));
}
private Optional<SuggestedFix.Builder> tryFixPositionalConverter(
@@ -305,7 +299,7 @@ public final class RedundantStringConversion extends BugChecker
/* Simplify the values to be plugged into the format pattern, if possible. */
return arguments.stream()
.skip(patternIndex + 1L)
.skip(patternIndex + 1)
.map(arg -> tryFix(arg, state, remainingArgFilter))
.flatMap(Optional::stream)
.reduce(SuggestedFix.Builder::merge);
@@ -369,7 +363,7 @@ public final class RedundantStringConversion extends BugChecker
return Optional.of(Iterables.getOnlyElement(methodInvocation.getArguments()));
}
private Description createDescription(Tree tree, Optional<SuggestedFix.Builder> fixes) {
private Description finalize(Tree tree, Optional<SuggestedFix.Builder> fixes) {
return fixes
.map(SuggestedFix.Builder::build)
.map(fix -> describeMatch(tree, fix))
@@ -380,9 +374,10 @@ public final class RedundantStringConversion extends BugChecker
ErrorProneFlags flags) {
// XXX: ErrorProneFlags#getList splits by comma, but method signatures may also contain commas.
// For this class methods accepting more than one argument are not valid, but still: not nice.
return anyOf(
WELL_KNOWN_STRING_CONVERSION_METHODS,
new MethodMatcherFactory()
.create(Flags.getList(flags, EXTRA_STRING_CONVERSION_METHODS_FLAG)));
return flags
.getList(EXTRA_STRING_CONVERSION_METHODS_FLAG)
.map(new MethodMatcherFactory()::create)
.map(m -> anyOf(WELL_KNOWN_STRING_CONVERSION_METHODS, m))
.orElse(WELL_KNOWN_STRING_CONVERSION_METHODS);
}
}

View File

@@ -74,13 +74,9 @@ public final class RequestMappingAnnotation extends BugChecker implements Method
isSameType("java.time.ZoneId"),
isSameType("java.util.Locale"),
isSameType("java.util.TimeZone"),
isSameType("jakarta.servlet.http.HttpServletRequest"),
isSameType("jakarta.servlet.http.HttpServletResponse"),
isSameType("javax.servlet.http.HttpServletRequest"),
isSameType("javax.servlet.http.HttpServletResponse"),
isSameType("org.springframework.http.HttpMethod"),
isSameType("org.springframework.ui.Model"),
isSameType("org.springframework.validation.BindingResult"),
isSameType("org.springframework.web.context.request.NativeWebRequest"),
isSameType("org.springframework.web.context.request.WebRequest"),
isSameType("org.springframework.web.server.ServerWebExchange"),
@@ -99,9 +95,8 @@ 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

@@ -1,6 +1,5 @@
package tech.picnic.errorprone.bugpatterns;
import static com.google.common.collect.ImmutableList.toImmutableList;
import static com.google.errorprone.BugPattern.LinkType.CUSTOM;
import static com.google.errorprone.BugPattern.SeverityLevel.ERROR;
import static com.google.errorprone.BugPattern.StandardTags.LIKELY_ERROR;
@@ -10,75 +9,41 @@ import static com.google.errorprone.matchers.Matchers.annotations;
import static com.google.errorprone.matchers.Matchers.anyOf;
import static com.google.errorprone.matchers.Matchers.isSubtypeOf;
import static com.google.errorprone.matchers.Matchers.isType;
import static com.google.errorprone.matchers.Matchers.not;
import static tech.picnic.errorprone.bugpatterns.util.Documentation.BUG_PATTERNS_BASE_URL;
import com.google.auto.service.AutoService;
import com.google.common.collect.ImmutableCollection;
import com.google.common.collect.ImmutableList;
import com.google.common.collect.ImmutableMap;
import com.google.errorprone.BugPattern;
import com.google.errorprone.ErrorProneFlags;
import com.google.errorprone.VisitorState;
import com.google.errorprone.bugpatterns.BugChecker;
import com.google.errorprone.bugpatterns.BugChecker.VariableTreeMatcher;
import com.google.errorprone.matchers.Description;
import com.google.errorprone.matchers.Matcher;
import com.google.errorprone.suppliers.Suppliers;
import com.sun.source.tree.Tree;
import com.sun.source.tree.VariableTree;
import javax.inject.Inject;
import tech.picnic.errorprone.bugpatterns.util.Flags;
/** A {@link BugChecker} that flags {@code @RequestParam} parameters with an unsupported type. */
@AutoService(BugChecker.class)
@BugPattern(
summary =
"By default, `@RequestParam` does not support `ImmutableCollection` and `ImmutableMap` subtypes",
summary = "`@RequestParam` does not support `ImmutableCollection` and `ImmutableMap` subtypes",
link = BUG_PATTERNS_BASE_URL + "RequestParamType",
linkType = CUSTOM,
severity = ERROR,
tags = LIKELY_ERROR)
@SuppressWarnings("java:S2160" /* Super class equality definition suffices. */)
public final class RequestParamType extends BugChecker implements VariableTreeMatcher {
private static final long serialVersionUID = 1L;
private static final String SUPPORTED_CUSTOM_TYPES_FLAG = "RequestParamType:SupportedCustomTypes";
private static final Matcher<VariableTree> HAS_UNSUPPORTED_REQUEST_PARAM =
allOf(
annotations(AT_LEAST_ONE, isType("org.springframework.web.bind.annotation.RequestParam")),
anyOf(isSubtypeOf(ImmutableCollection.class), isSubtypeOf(ImmutableMap.class)));
private final Matcher<VariableTree> hasUnsupportedRequestParamType;
/** Instantiates a default {@link RequestParamType} instance. */
public RequestParamType() {
this(ErrorProneFlags.empty());
}
/**
* Instantiates a customized {@link RequestParamType} instance.
*
* @param flags Any provided command line flags.
*/
@Inject
RequestParamType(ErrorProneFlags flags) {
hasUnsupportedRequestParamType = hasUnsupportedRequestParamType(flags);
}
/** Instantiates a new {@link RequestParamType} instance. */
public RequestParamType() {}
@Override
public Description matchVariable(VariableTree tree, VisitorState state) {
return hasUnsupportedRequestParamType.matches(tree, state)
return HAS_UNSUPPORTED_REQUEST_PARAM.matches(tree, state)
? describeMatch(tree)
: Description.NO_MATCH;
}
private static Matcher<VariableTree> hasUnsupportedRequestParamType(ErrorProneFlags flags) {
return allOf(
annotations(AT_LEAST_ONE, isType("org.springframework.web.bind.annotation.RequestParam")),
anyOf(isSubtypeOf(ImmutableCollection.class), isSubtypeOf(ImmutableMap.class)),
not(isSubtypeOfAny(Flags.getList(flags, SUPPORTED_CUSTOM_TYPES_FLAG))));
}
private static Matcher<Tree> isSubtypeOfAny(ImmutableList<String> inclusions) {
return anyOf(
inclusions.stream()
.map(inclusion -> isSubtypeOf(Suppliers.typeFromString(inclusion)))
.collect(toImmutableList()));
}
}

View File

@@ -0,0 +1,94 @@
package tech.picnic.errorprone.bugpatterns;
import static com.google.errorprone.BugPattern.LinkType.CUSTOM;
import static com.google.errorprone.BugPattern.SeverityLevel.ERROR;
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.hasAnnotation;
import static com.google.errorprone.matchers.Matchers.isType;
import static tech.picnic.errorprone.bugpatterns.util.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.Iterables;
import com.google.errorprone.BugPattern;
import com.google.errorprone.VisitorState;
import com.google.errorprone.bugpatterns.BugChecker;
import com.google.errorprone.bugpatterns.BugChecker.MethodTreeMatcher;
import com.google.errorprone.fixes.SuggestedFix;
import com.google.errorprone.fixes.SuggestedFixes;
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.MethodTree;
import com.sun.source.tree.Tree;
import tech.picnic.errorprone.bugpatterns.util.ThirdPartyLibrary;
/**
* A {@link BugChecker} that flags methods with Spring's {@code @Scheduled} annotation that lack New
* Relic Agent's {@code @Trace(dispatcher = true)}.
*/
@AutoService(BugChecker.class)
@BugPattern(
summary = "Scheduled operation must start a new New Relic transaction",
link = BUG_PATTERNS_BASE_URL + "ScheduledTransactionTrace",
linkType = CUSTOM,
severity = ERROR,
tags = LIKELY_ERROR)
public final class ScheduledTransactionTrace extends BugChecker implements MethodTreeMatcher {
private static final long serialVersionUID = 1L;
private static final String TRACE_ANNOTATION_FQCN = "com.newrelic.api.agent.Trace";
private static final Matcher<Tree> IS_SCHEDULED =
hasAnnotation("org.springframework.scheduling.annotation.Scheduled");
private static final MultiMatcher<Tree, AnnotationTree> TRACE_ANNOTATION =
annotations(AT_LEAST_ONE, isType(TRACE_ANNOTATION_FQCN));
/** Instantiates a new {@link ScheduledTransactionTrace} instance. */
public ScheduledTransactionTrace() {}
@Override
public Description matchMethod(MethodTree tree, VisitorState state) {
if (!ThirdPartyLibrary.NEW_RELIC_AGENT_API.isIntroductionAllowed(state)
|| !IS_SCHEDULED.matches(tree, state)) {
return Description.NO_MATCH;
}
ImmutableList<AnnotationTree> traceAnnotations =
TRACE_ANNOTATION.multiMatchResult(tree, state).matchingNodes();
if (traceAnnotations.isEmpty()) {
/* This method completely lacks the `@Trace` annotation; add it. */
return describeMatch(
tree,
SuggestedFix.builder()
.addImport(TRACE_ANNOTATION_FQCN)
.prefixWith(tree, "@Trace(dispatcher = true)")
.build());
}
AnnotationTree traceAnnotation = Iterables.getOnlyElement(traceAnnotations);
if (isCorrectAnnotation(traceAnnotation)) {
return Description.NO_MATCH;
}
/*
* The `@Trace` annotation is present but does not specify `dispatcher = true`. Add or update
* the `dispatcher` annotation element.
*/
return describeMatch(
traceAnnotation,
SuggestedFixes.updateAnnotationArgumentValues(
traceAnnotation, state, "dispatcher", ImmutableList.of("true"))
.build());
}
private static boolean isCorrectAnnotation(AnnotationTree traceAnnotation) {
return Boolean.TRUE.equals(
AnnotationMirrors.getAnnotationValue(
ASTHelpers.getAnnotationMirror(traceAnnotation), "dispatcher")
.getValue());
}
}

View File

@@ -0,0 +1,350 @@
package tech.picnic.errorprone.bugpatterns;
import static com.google.common.base.Preconditions.checkArgument;
import static com.google.common.collect.ImmutableList.toImmutableList;
import static com.google.common.collect.ImmutableMap.toImmutableMap;
import static com.google.errorprone.BugPattern.LinkType.CUSTOM;
import static com.google.errorprone.BugPattern.SeverityLevel.WARNING;
import static com.google.errorprone.BugPattern.StandardTags.SIMPLIFICATION;
import static java.util.Objects.requireNonNull;
import static tech.picnic.errorprone.bugpatterns.util.Documentation.BUG_PATTERNS_BASE_URL;
import com.google.auto.service.AutoService;
import com.google.common.collect.ImmutableList;
import com.google.common.collect.ImmutableMap;
import com.google.common.collect.ImmutableSet;
import com.google.common.collect.ImmutableSortedSet;
import com.google.common.collect.Iterables;
import com.google.common.collect.Maps;
import com.google.errorprone.BugPattern;
import com.google.errorprone.VisitorState;
import com.google.errorprone.bugpatterns.BugChecker;
import com.google.errorprone.bugpatterns.BugChecker.AnnotationTreeMatcher;
import com.google.errorprone.fixes.Fix;
import com.google.errorprone.fixes.SuggestedFix;
import com.google.errorprone.fixes.SuggestedFixes;
import com.google.errorprone.matchers.Description;
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;
import com.sun.tools.javac.code.Scope;
import com.sun.tools.javac.code.Symbol.MethodSymbol;
import com.sun.tools.javac.code.Symbol.VarSymbol;
import java.util.Arrays;
import java.util.Map;
import java.util.Optional;
import java.util.concurrent.TimeUnit;
import java.util.stream.Stream;
import tech.picnic.errorprone.bugpatterns.util.AnnotationAttributeMatcher;
import tech.picnic.errorprone.bugpatterns.util.SourceCode;
/**
* A {@link BugChecker} which flags annotations with time attributes that can be written more
* concisely.
*/
@AutoService(BugChecker.class)
@BugPattern(
summary = "Simplifies annotations which express an amount of time using a `TimeUnit`",
link = BUG_PATTERNS_BASE_URL + "SimplifyTimeAnnotation",
linkType = CUSTOM,
severity = WARNING,
tags = SIMPLIFICATION)
public final class SimplifyTimeAnnotationCheck extends BugChecker implements AnnotationTreeMatcher {
private static final long serialVersionUID = 1L;
private static final AnnotationAttributeMatcher ARGUMENT_SELECTOR =
createAnnotationAttributeMatcher();
/** Instantiates a new {@link SimplifyTimeAnnotationCheck} instance. */
public SimplifyTimeAnnotationCheck() {}
@Override
public Description matchAnnotation(AnnotationTree annotationTree, VisitorState state) {
ImmutableList<ExpressionTree> arguments =
ARGUMENT_SELECTOR.extractMatchingArguments(annotationTree).collect(toImmutableList());
if (arguments.isEmpty()) {
return Description.NO_MATCH;
}
return trySimplification(annotationTree, arguments, state)
.map(fix -> describeMatch(annotationTree, fix))
.orElse(Description.NO_MATCH);
}
private static Optional<Fix> trySimplification(
AnnotationTree annotation, ImmutableList<ExpressionTree> arguments, VisitorState state) {
checkArgument(!arguments.isEmpty());
AnnotationDescriptor annotationDescriptor =
AnnotationDescriptor.from(getAnnotationFqcn(annotation));
if (containsAnyAttributeOf(annotation, annotationDescriptor.bannedFields)) {
return Optional.empty();
}
ImmutableMap<String, ExpressionTree> indexedAttributes =
Maps.uniqueIndex(
arguments,
expr ->
ASTHelpers.getSymbol(((AssignmentTree) expr).getVariable())
.getSimpleName()
.toString());
TimeUnit currentTimeUnit =
getTimeUnit(annotation, annotationDescriptor.timeUnitField, indexedAttributes);
ImmutableMap<String, Number> timeValues =
annotationDescriptor.timeFields.stream()
.map(field -> Map.entry(field, getValue(field, indexedAttributes)))
.filter(entry -> entry.getValue().isPresent())
.collect(toImmutableMap(Map.Entry::getKey, entry -> entry.getValue().orElseThrow()));
Map<String, TimeSimplifier.Simplification> simplifications =
Maps.transformValues(
Maps.filterValues(
Maps.transformEntries(
timeValues, (field, value) -> trySimplify(value, currentTimeUnit)),
Optional::isPresent),
Optional::orElseThrow);
// Some could not be simplified, and since the unit is shared, the others can't either.
if (simplifications.size() != timeValues.size()) {
return Optional.empty();
}
// The annotation is of the form `@Annotation(v)` or `@Annotation(value = v)`. For the former we
// must synthesize the entire annotation, but this is OK for the latter, too.
if (indexedAttributes.size() == 1 && simplifications.containsKey("value")) {
TimeSimplifier.Simplification simplification = simplifications.get("value");
return Optional.of(
getImplicitValueAttributeFix(
annotation,
simplification.value,
annotationDescriptor.timeUnitField,
simplification.timeUnit,
state));
}
// Since each might have a different simplification possible, check the common unit.
// Since we only get simplifications iff it's possible, and we check that all can be simplified,
// we don't need to check if this equals `currentTimeUnit`.
TimeUnit commonUnit =
findCommonUnit(
ImmutableSet.copyOf(
Maps.transformValues(simplifications, simplification -> simplification.timeUnit)
.values()));
return getExplicitAttributesFix(
annotation, simplifications, annotationDescriptor.timeUnitField, commonUnit, state);
}
private static boolean containsAnyAttributeOf(
AnnotationTree annotation, ImmutableSet<String> attributes) {
return annotation.getArguments().stream()
.map(
expr ->
expr.getKind() == Tree.Kind.ASSIGNMENT
? ASTHelpers.getSymbol(((AssignmentTree) expr).getVariable())
.getSimpleName()
.toString()
: "value")
.anyMatch(attributes::contains);
}
private static Fix getImplicitValueAttributeFix(
AnnotationTree annotation,
long newValue,
String timeUnitField,
TimeUnit newTimeUnit,
VisitorState state) {
String synthesizedAnnotation =
SourceCode.treeToString(annotation, state)
.replaceFirst(
"\\(.+\\)",
String.format("(value=%s, %s=%s)", newValue, timeUnitField, newTimeUnit.name()));
return SuggestedFix.builder()
.replace(annotation, synthesizedAnnotation)
.addStaticImport(TimeUnit.class.getName() + '.' + newTimeUnit.name())
.build();
}
private static Optional<Fix> getExplicitAttributesFix(
AnnotationTree annotation,
Map<String, TimeSimplifier.Simplification> simplifications,
String timeUnitField,
TimeUnit newUnit,
VisitorState state) {
return simplifications.entrySet().stream()
.map(
simplificationEntry ->
SuggestedFixes.updateAnnotationArgumentValues(
annotation, state, timeUnitField, ImmutableList.of(newUnit.name()))
.merge(
SuggestedFixes.updateAnnotationArgumentValues(
annotation,
state,
simplificationEntry.getKey(),
ImmutableList.of(
String.valueOf(simplificationEntry.getValue().toUnit(newUnit))))))
.reduce(SuggestedFix.Builder::merge)
.map(builder -> builder.addStaticImport(TimeUnit.class.getName() + '.' + newUnit.name()))
.map(SuggestedFix.Builder::build);
}
private static String getAnnotationFqcn(AnnotationTree annotation) {
return ASTHelpers.getSymbol(annotation).getQualifiedName().toString();
}
private static Optional<Number> getValue(
String field, ImmutableMap<String, ExpressionTree> indexedArguments) {
return Optional.ofNullable(indexedArguments.get(field))
.filter(AssignmentTree.class::isInstance)
.map(AssignmentTree.class::cast)
.map(AssignmentTree::getExpression)
.map(expr -> ASTHelpers.constValue(expr, Number.class));
}
private static TimeUnit getTimeUnit(
AnnotationTree annotation,
String field,
ImmutableMap<String, ExpressionTree> indexedArguments) {
VarSymbol symbol =
Optional.ofNullable(indexedArguments.get(field))
.map(
argumentTree ->
(VarSymbol)
ASTHelpers.getSymbol(((AssignmentTree) argumentTree).getExpression()))
.orElseGet(() -> getDefaultTimeUnit(annotation, field));
return TimeUnit.valueOf(symbol.getQualifiedName().toString());
}
private static VarSymbol getDefaultTimeUnit(AnnotationTree annotation, String argument) {
Scope scope = ASTHelpers.getSymbol(annotation).members();
MethodSymbol argumentSymbol =
(MethodSymbol)
Iterables.getOnlyElement(
ASTHelpers.scope(scope)
.getSymbols(symbol -> symbol.getQualifiedName().contentEquals(argument)));
return (VarSymbol)
requireNonNull(argumentSymbol.getDefaultValue(), "Default value missing").getValue();
}
private static AnnotationAttributeMatcher createAnnotationAttributeMatcher() {
ImmutableList<String> toMatch =
Arrays.stream(AnnotationDescriptor.values())
.flatMap(
annotation ->
annotation.timeFields.stream().map(field -> annotation.fqcn + '#' + field))
.collect(toImmutableList());
return AnnotationAttributeMatcher.create(Optional.of(toMatch), ImmutableList.of());
}
private static Optional<TimeSimplifier.Simplification> trySimplify(Number value, TimeUnit unit) {
checkArgument(
value instanceof Integer || value instanceof Long,
"Only time expressed as an integer or long can be simplified");
return TimeSimplifier.simplify(value.longValue(), unit);
}
private static TimeUnit findCommonUnit(ImmutableSet<TimeUnit> units) {
return ImmutableSortedSet.copyOf(units).first();
}
private enum AnnotationDescriptor {
JUNIT_TIMEOUT("org.junit.jupiter.api.Timeout", ImmutableSet.of("value"), "unit"),
SPRING_SCHEDULED(
"org.springframework.scheduling.annotation.Scheduled",
ImmutableSet.of("fixedDelay", "fixedRate", "initialDelay"),
"timeUnit",
ImmutableSet.of("fixedDelayString", "fixedRateString", "initialDelayString"));
/** The fully-qualified class name of the annotation to simplify. */
private final String fqcn;
/** The attributes containing a value of time. */
private final ImmutableSet<String> timeFields;
/** The attribute containing the time unit. */
private final String timeUnitField;
/** The set of attributes that cause the check to back off. */
private final ImmutableSet<String> bannedFields;
AnnotationDescriptor(String fqcn, ImmutableSet<String> timeFields, String timeUnitField) {
this(fqcn, timeFields, timeUnitField, ImmutableSet.of());
}
AnnotationDescriptor(
String fqcn,
ImmutableSet<String> timeFields,
String timeUnitField,
ImmutableSet<String> bannedFields) {
this.fqcn = fqcn;
this.timeFields = timeFields;
this.timeUnitField = timeUnitField;
this.bannedFields = bannedFields;
}
public static AnnotationDescriptor from(String fqcn) {
return Arrays.stream(values())
.filter(annotation -> annotation.fqcn.equals(fqcn))
.findFirst()
.orElseThrow(
() ->
new IllegalArgumentException(
String.format(
"Unknown enum constant: %s.%s",
AnnotationDescriptor.class.getName(), fqcn)));
}
}
/** Utility class to help simplify time expressions. */
private static final class TimeSimplifier {
private static final ImmutableSortedSet<TimeUnit> TIME_UNITS =
ImmutableSortedSet.copyOf(TimeUnit.values());
/**
* Returns a {@link Simplification} (iff possible) that describes how the {@code originalValue}
* and {@code originalUnit} can be simplified using a larger {@link TimeUnit}.
*/
static Optional<Simplification> simplify(long originalValue, TimeUnit originalUnit) {
return descendingLargerUnits(originalUnit).stream()
.flatMap(unit -> trySimplify(originalValue, originalUnit, unit))
.findFirst();
}
private static Stream<Simplification> trySimplify(
long originalValue, TimeUnit originalUnit, TimeUnit unit) {
long converted = unit.convert(originalValue, originalUnit);
// Check whether we lose any precision by checking whether we can convert back.
return originalValue == originalUnit.convert(converted, unit)
? Stream.of(new Simplification(converted, unit))
: Stream.empty();
}
/**
* Returns all time units that represent a larger amount of time than {@code unit}, in
* descending order.
*/
private static ImmutableSortedSet<TimeUnit> descendingLargerUnits(TimeUnit unit) {
return TIME_UNITS.tailSet(unit, /* inclusive= */ false).descendingSet();
}
/** Represents a simplification in terms of the new value and new unit. */
private static final class Simplification {
private final long value;
private final TimeUnit timeUnit;
Simplification(long value, TimeUnit timeUnit) {
this.value = value;
this.timeUnit = timeUnit;
}
/**
* Converts the value with the unit represented by this simplification to an equivalent value
* in the given {@code unit}.
*/
public long toUnit(TimeUnit unit) {
return unit.convert(value, timeUnit);
}
}
}
}

View File

@@ -155,7 +155,6 @@ public final class StaticImport extends BugChecker implements MemberSelectTreeMa
@VisibleForTesting
static final ImmutableSetMultimap<String, String> STATIC_IMPORT_EXEMPTED_MEMBERS =
ImmutableSetMultimap.<String, String>builder()
.put("com.google.common.base.Predicates", "contains")
.put("com.mongodb.client.model.Filters", "empty")
.putAll(
"java.util.Collections",

View File

@@ -27,7 +27,7 @@ import com.sun.tools.javac.util.Convert;
import java.util.Formattable;
import java.util.Iterator;
import java.util.List;
import org.jspecify.annotations.Nullable;
import org.jspecify.nullness.Nullable;
import tech.picnic.errorprone.bugpatterns.util.SourceCode;
/**

View File

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

View File

@@ -103,7 +103,7 @@ public final class AnnotationAttributeMatcher implements Serializable {
* @param tree The annotation AST node to be inspected.
* @return Any matching annotation arguments.
*/
public Stream<ExpressionTree> extractMatchingArguments(AnnotationTree tree) {
public Stream<? extends ExpressionTree> extractMatchingArguments(AnnotationTree tree) {
Type type = ASTHelpers.getType(tree.getAnnotationType());
if (type == null) {
return Stream.empty();
@@ -111,7 +111,6 @@ public final class AnnotationAttributeMatcher implements Serializable {
String annotationType = type.toString();
return tree.getArguments().stream()
.map(ExpressionTree.class::cast)
.filter(a -> matches(annotationType, extractAttributeName(a)));
}

View File

@@ -1,75 +0,0 @@
package tech.picnic.errorprone.bugpatterns.util;
import static tech.picnic.errorprone.bugpatterns.util.JavaKeywords.isValidIdentifier;
import com.google.errorprone.VisitorState;
import com.google.errorprone.util.ASTHelpers;
import com.sun.source.tree.ImportTree;
import com.sun.source.tree.Tree;
import com.sun.tools.javac.code.Symbol.MethodSymbol;
import com.sun.tools.javac.code.Type;
import java.util.Optional;
/** A set of helper methods for detecting conflicts that would be caused when applying fixes. */
public final class ConflictDetection {
private ConflictDetection() {}
/**
* If applicable, returns a human-readable argument against assigning the given name to an
* existing method.
*
* <p>This method implements imperfect heuristics. Things it currently does not consider include
* the following:
*
* <ul>
* <li>Whether the rename would merely introduce a method overload, rather than clashing with an
* existing method declaration in its class or a supertype.
* <li>Whether the rename would in fact clash with a static import. (It could be that a static
* import of the same name is only referenced from lexical scopes in which the method under
* consideration cannot be referenced directly.)
* </ul>
*
* @param method The method considered for renaming.
* @param newName The newly proposed name for the method.
* @param state The {@link VisitorState} to use when searching for blockers.
* @return A human-readable argument against assigning the proposed name to the given method, or
* {@link Optional#empty()} if no blocker was found.
*/
public static Optional<String> findMethodRenameBlocker(
MethodSymbol method, String newName, VisitorState state) {
if (isExistingMethodName(method.owner.type, newName, state)) {
return Optional.of(
String.format(
"a method named `%s` is already defined in this class or a supertype", newName));
}
if (isSimpleNameStaticallyImported(newName, state)) {
return Optional.of(String.format("`%s` is already statically imported", newName));
}
if (!isValidIdentifier(newName)) {
return Optional.of(String.format("`%s` is not a valid identifier", newName));
}
return Optional.empty();
}
private static boolean isExistingMethodName(Type clazz, String name, VisitorState state) {
return ASTHelpers.matchingMethods(state.getName(name), method -> true, clazz, state.getTypes())
.findAny()
.isPresent();
}
private static boolean isSimpleNameStaticallyImported(String simpleName, VisitorState state) {
return state.getPath().getCompilationUnit().getImports().stream()
.filter(ImportTree::isStatic)
.map(ImportTree::getQualifiedIdentifier)
.map(tree -> getStaticImportSimpleName(tree, state))
.anyMatch(simpleName::contentEquals);
}
private static CharSequence getStaticImportSimpleName(Tree tree, VisitorState state) {
String source = SourceCode.treeToString(tree, state);
return source.subSequence(source.lastIndexOf('.') + 1, source.length());
}
}

View File

@@ -1,22 +0,0 @@
package tech.picnic.errorprone.bugpatterns.util;
import com.google.common.collect.ImmutableList;
import com.google.errorprone.ErrorProneFlags;
/** Helper methods for working with {@link ErrorProneFlags}. */
public final class Flags {
private Flags() {}
/**
* Returns the list of (comma-separated) arguments passed using the given Error Prone flag.
*
* @param errorProneFlags The full set of flags provided.
* @param name The name of the flag of interest.
* @return A non-{@code null} list of provided arguments; this list is empty if the flag was not
* provided, or if the flag's value is the empty string.
*/
public static ImmutableList<String> getList(ErrorProneFlags errorProneFlags, String name) {
ImmutableList<String> list = errorProneFlags.getListOrEmpty(name);
return list.equals(ImmutableList.of("")) ? ImmutableList.of() : list;
}
}

View File

@@ -4,20 +4,7 @@ import com.google.common.collect.ImmutableSet;
import com.google.common.collect.Sets;
/** Utility class that can be used to identify reserved keywords of the Java language. */
// XXX: This class is no longer only about keywords. Consider changing its name and class-level
// documentation.
public final class JavaKeywords {
/**
* Enumeration of boolean and null literals.
*
* @see <a href="https://docs.oracle.com/javase/specs/jls/se17/html/jls-3.html#jls-3.10.3">JDK 17
* JLS section 3.10.3: Boolean Literals</a>
* @see <a href="https://docs.oracle.com/javase/specs/jls/se17/html/jls-3.html#jls-3.10.8">JDK 17
* JLS section 3.10.8: The Null Literal</a>
*/
private static final ImmutableSet<String> BOOLEAN_AND_NULL_LITERALS =
ImmutableSet.of("true", "false", "null");
/**
* List of all reserved keywords in the Java language.
*
@@ -109,23 +96,6 @@ public final class JavaKeywords {
private JavaKeywords() {}
/**
* Tells whether the given string is a valid identifier in the Java language.
*
* @param str The string of interest.
* @return {@code true} if the given string is a valid identifier in the Java language.
* @see <a href="https://docs.oracle.com/javase/specs/jls/se17/html/jls-3.html#jls-3.8">JDK 17 JLS
* section 3.8: Identifiers</a>
*/
@SuppressWarnings("java:S1067" /* Chaining conjunctions like this does not impact readability. */)
public static boolean isValidIdentifier(String str) {
return !str.isEmpty()
&& !isReservedKeyword(str)
&& !BOOLEAN_AND_NULL_LITERALS.contains(str)
&& Character.isJavaIdentifierStart(str.codePointAt(0))
&& str.codePoints().skip(1).allMatch(Character::isUnicodeIdentifierPart);
}
/**
* Tells whether the given string is a reserved keyword in the Java language.
*

View File

@@ -18,10 +18,6 @@ import java.util.regex.Pattern;
public final class MethodMatcherFactory {
private static final Splitter ARGUMENT_TYPE_SPLITTER =
Splitter.on(',').trimResults().omitEmptyStrings();
// XXX: Check whether we can use a parser for "standard" Java signatures here. Maybe
// `sun.reflect.generics.parser.SignatureParser`?
@SuppressWarnings("java:S5998" /* In practice there will be only modest recursion. */)
private static final Pattern METHOD_SIGNATURE =
Pattern.compile("([^\\s#(,)]+)#([^\\s#(,)]+)\\(((?:[^\\s#(,)]+(?:,[^\\s#(,)]+)*)?)\\)");

View File

@@ -1,82 +0,0 @@
package tech.picnic.errorprone.bugpatterns.util;
import static com.google.common.base.Preconditions.checkArgument;
import static com.google.common.collect.ImmutableList.toImmutableList;
import com.google.common.collect.ImmutableList;
import com.google.errorprone.VisitorState;
import com.google.errorprone.util.ASTHelpers;
import com.sun.source.tree.ClassTree;
import com.sun.source.tree.LambdaExpressionTree;
import com.sun.source.tree.MethodTree;
import com.sun.source.tree.Tree;
import java.util.Optional;
/**
* A collection of helper methods for working with the AST.
*
* <p>These methods are additions to the ones found in {@link
* com.google.errorprone.util.ASTHelpers}.
*/
public final class MoreASTHelpers {
private MoreASTHelpers() {}
/**
* Finds methods with the specified name in given the {@link VisitorState}'s current enclosing
* class.
*
* @param methodName The method name to search for.
* @param state The {@link VisitorState} from which to derive the enclosing class of interest.
* @return The {@link MethodTree}s of the methods with the given name in the enclosing class.
*/
public static ImmutableList<MethodTree> findMethods(CharSequence methodName, VisitorState state) {
ClassTree clazz = state.findEnclosing(ClassTree.class);
checkArgument(clazz != null, "Visited node is not enclosed by a class");
return clazz.getMembers().stream()
.filter(MethodTree.class::isInstance)
.map(MethodTree.class::cast)
.filter(method -> method.getName().contentEquals(methodName))
.collect(toImmutableList());
}
/**
* Determines whether there are any methods with the specified name in given the {@link
* VisitorState}'s current enclosing class.
*
* @param methodName The method name to search for.
* @param state The {@link VisitorState} from which to derive the enclosing class of interest.
* @return Whether there are any methods with the given name in the enclosing class.
*/
public static boolean methodExistsInEnclosingClass(CharSequence methodName, VisitorState state) {
return !findMethods(methodName, state).isEmpty();
}
/**
* Returns the {@link MethodTree} from which control flow would exit if there would be a {@code
* return} statement at the given {@link VisitorState}'s current {@link VisitorState#getPath()
* path}.
*
* @param state The {@link VisitorState} from which to derive the AST location of interest.
* @return A {@link MethodTree}, unless the {@link VisitorState}'s path does not point to an AST
* node located inside a method, or if the (hypothetical) {@code return} statement would exit
* a lambda expression instead.
*/
public static Optional<MethodTree> findMethodExitedOnReturn(VisitorState state) {
return Optional.ofNullable(state.findEnclosing(LambdaExpressionTree.class, MethodTree.class))
.filter(MethodTree.class::isInstance)
.map(MethodTree.class::cast);
}
/**
* Tells whether the given trees are of the same type, after type erasure.
*
* @param treeA The first tree of interest.
* @param treeB The second tree of interest.
* @param state The {@link VisitorState} describing the context in which the given trees were
* found.
* @return Whether the specified trees have the same erased types.
*/
public static boolean areSameType(Tree treeA, Tree treeB, VisitorState state) {
return ASTHelpers.isSameType(ASTHelpers.getType(treeA), ASTHelpers.getType(treeB), state);
}
}

View File

@@ -1,117 +0,0 @@
package tech.picnic.errorprone.bugpatterns.util;
import static com.google.common.collect.ImmutableList.toImmutableList;
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.anyOf;
import static com.google.errorprone.matchers.Matchers.isType;
import static java.util.Objects.requireNonNullElse;
import static tech.picnic.errorprone.bugpatterns.util.MoreMatchers.hasMetaAnnotation;
import com.google.common.base.Strings;
import com.google.common.collect.ImmutableList;
import com.google.errorprone.matchers.AnnotationMatcherUtils;
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.ExpressionTree;
import com.sun.source.tree.MethodTree;
import com.sun.source.tree.NewArrayTree;
import org.jspecify.annotations.Nullable;
/**
* A collection of JUnit-specific helper methods and {@link Matcher}s.
*
* <p>These constants and methods are additions to the ones found in {@link
* com.google.errorprone.matchers.JUnitMatchers}.
*/
public final class MoreJUnitMatchers {
/** Matches JUnit Jupiter test methods. */
public static final MultiMatcher<MethodTree, AnnotationTree> TEST_METHOD =
annotations(
AT_LEAST_ONE,
anyOf(
isType("org.junit.jupiter.api.Test"),
hasMetaAnnotation("org.junit.jupiter.api.TestTemplate")));
/** Matches JUnit Jupiter setup and teardown methods. */
public static final MultiMatcher<MethodTree, AnnotationTree> SETUP_OR_TEARDOWN_METHOD =
annotations(
AT_LEAST_ONE,
anyOf(
isType("org.junit.jupiter.api.AfterAll"),
isType("org.junit.jupiter.api.AfterEach"),
isType("org.junit.jupiter.api.BeforeAll"),
isType("org.junit.jupiter.api.BeforeEach")));
/**
* Matches methods that have a {@link org.junit.jupiter.params.provider.MethodSource} annotation.
*/
public static final MultiMatcher<MethodTree, AnnotationTree> HAS_METHOD_SOURCE =
annotations(AT_LEAST_ONE, isType("org.junit.jupiter.params.provider.MethodSource"));
private MoreJUnitMatchers() {}
/**
* Returns the names of the JUnit value factory methods specified by the given {@link
* org.junit.jupiter.params.provider.MethodSource} annotation.
*
* <p>This method differs from {@link #getMethodSourceFactoryDescriptors(AnnotationTree,
* MethodTree)} in that it drops any parenthesized method parameter type enumerations. That is,
* method descriptors such as {@code factoryMethod()} and {@code factoryMethod(java.lang.String)}
* are both simplified to just {@code factoryMethod}. This also means that the returned method
* names may not unambiguously reference a single value factory method; in such a case JUnit will
* throw an error at runtime.
*
* @param methodSourceAnnotation The annotation from which to extract value factory method names.
* @param method The method on which the annotation is located.
* @return One or more value factory descriptors, in the order defined.
* @see #getMethodSourceFactoryDescriptors(AnnotationTree, MethodTree)
*/
// XXX: Drop this method in favour of `#getMethodSourceFactoryDescriptors`. That will require
// callers to either explicitly drop information, or perform a more advanced analysis.
public static ImmutableList<String> getMethodSourceFactoryNames(
AnnotationTree methodSourceAnnotation, MethodTree method) {
return getMethodSourceFactoryDescriptors(methodSourceAnnotation, method).stream()
.map(
descriptor -> {
int index = descriptor.indexOf('(');
return index < 0 ? descriptor : descriptor.substring(0, index);
})
.collect(toImmutableList());
}
/**
* Returns the descriptors of the JUnit value factory methods specified by the given {@link
* org.junit.jupiter.params.provider.MethodSource} annotation.
*
* @param methodSourceAnnotation The annotation from which to extract value factory method
* descriptors.
* @param method The method on which the annotation is located.
* @return One or more value factory descriptors, in the order defined.
* @see #getMethodSourceFactoryNames(AnnotationTree, MethodTree)
*/
// XXX: Rather than strings, have this method return instances of a value type capable of
// resolving the value factory method pointed to.
public static ImmutableList<String> getMethodSourceFactoryDescriptors(
AnnotationTree methodSourceAnnotation, MethodTree method) {
String methodName = method.getName().toString();
ExpressionTree value = AnnotationMatcherUtils.getArgument(methodSourceAnnotation, "value");
if (!(value instanceof NewArrayTree)) {
return ImmutableList.of(toMethodSourceFactoryDescriptor(value, methodName));
}
return ((NewArrayTree) value)
.getInitializers().stream()
.map(name -> toMethodSourceFactoryDescriptor(name, methodName))
.collect(toImmutableList());
}
private static String toMethodSourceFactoryDescriptor(
@Nullable ExpressionTree tree, String annotatedMethodName) {
return requireNonNullElse(
Strings.emptyToNull(ASTHelpers.constValue(tree, String.class)), annotatedMethodName);
}
}

View File

@@ -1,48 +0,0 @@
package tech.picnic.errorprone.bugpatterns.util;
import static com.google.errorprone.matchers.Matchers.typePredicateMatcher;
import static tech.picnic.errorprone.bugpatterns.util.MoreTypePredicates.hasAnnotation;
import com.google.errorprone.matchers.Matcher;
import com.google.errorprone.matchers.Matchers;
import com.google.errorprone.suppliers.Supplier;
import com.sun.source.tree.AnnotationTree;
import com.sun.source.tree.Tree;
import com.sun.tools.javac.code.Type;
/**
* A collection of general-purpose {@link Matcher}s.
*
* <p>These methods are additions to the ones found in {@link Matchers}.
*/
public final class MoreMatchers {
private MoreMatchers() {}
/**
* Returns a {@link Matcher} that determines whether a given {@link AnnotationTree} has a
* meta-annotation of the specified type.
*
* @param <T> The type of tree to match against.
* @param annotationType The binary type name of the annotation (e.g.
* "org.jspecify.annotations.Nullable", or "some.package.OuterClassName$InnerClassName")
* @return A {@link Matcher} that matches trees with the specified meta-annotation.
*/
public static <T extends AnnotationTree> Matcher<T> hasMetaAnnotation(String annotationType) {
return typePredicateMatcher(hasAnnotation(annotationType));
}
/**
* Returns a {@link Matcher} that determines whether the type of a given {@link Tree} is a subtype
* of the type returned by the specified {@link Supplier}.
*
* <p>This method differs from {@link Matchers#isSubtypeOf(Supplier)} in that it does not perform
* type erasure.
*
* @param <T> The type of tree to match against.
* @param type The {@link Supplier} that returns the type to match against.
* @return A {@link Matcher} that matches trees with the specified type.
*/
public static <T extends Tree> Matcher<T> isSubTypeOf(Supplier<Type> type) {
return typePredicateMatcher(MoreTypePredicates.isSubTypeOf(type));
}
}

View File

@@ -1,63 +0,0 @@
package tech.picnic.errorprone.bugpatterns.util;
import com.google.errorprone.VisitorState;
import com.google.errorprone.predicates.TypePredicate;
import com.google.errorprone.predicates.TypePredicates;
import com.google.errorprone.suppliers.Supplier;
import com.google.errorprone.util.ASTHelpers;
import com.sun.tools.javac.code.Type;
import com.sun.tools.javac.util.List;
/**
* A collection of general-purpose {@link TypePredicate}s.
*
* <p>These methods are additions to the ones found in {@link TypePredicates}.
*/
// XXX: The methods in this class are tested only indirectly. Consider adding a dedicated test
// class, or make sure that each method is explicitly covered by a tested analog in `MoreMatchers`.
public final class MoreTypePredicates {
private MoreTypePredicates() {}
/**
* Returns a {@link TypePredicate} that matches types that are annotated with the indicated
* annotation type.
*
* @param annotationType The fully-qualified name of the annotation type.
* @return A {@link TypePredicate} that matches appropriate types.
*/
public static TypePredicate hasAnnotation(String annotationType) {
return (type, state) -> ASTHelpers.hasAnnotation(type.tsym, annotationType, state);
}
/**
* Returns a {@link TypePredicate} that matches subtypes of the type returned by the specified
* {@link Supplier}.
*
* @implNote This method does not use {@link ASTHelpers#isSubtype(Type, Type, VisitorState)}, as
* that method performs type erasure.
* @param bound The {@link Supplier} that returns the type to match against.
* @return A {@link TypePredicate} that matches appropriate subtypes.
*/
public static TypePredicate isSubTypeOf(Supplier<Type> bound) {
Supplier<Type> memoizedType = VisitorState.memoize(bound);
return (type, state) -> {
Type boundType = memoizedType.get(state);
return boundType != null && state.getTypes().isSubtype(type, boundType);
};
}
/**
* Returns a {@link TypePredicate} that matches generic types with a type parameter that matches
* the specified {@link TypePredicate} at the indicated index.
*
* @param index The index of the type parameter to match against.
* @param predicate The {@link TypePredicate} to match against the type parameter.
* @return A {@link TypePredicate} that matches appropriate generic types.
*/
public static TypePredicate hasTypeParameter(int index, TypePredicate predicate) {
return (type, state) -> {
List<Type> typeArguments = type.getTypeArguments();
return typeArguments.size() > index && predicate.apply(typeArguments.get(index), state);
};
}
}

View File

@@ -1,31 +1,14 @@
package tech.picnic.errorprone.bugpatterns.util;
import static com.sun.tools.javac.parser.Tokens.TokenKind.RPAREN;
import static com.sun.tools.javac.util.Position.NOPOS;
import static java.util.stream.Collectors.joining;
import com.google.common.annotations.VisibleForTesting;
import com.google.common.base.CharMatcher;
import com.google.common.collect.ImmutableList;
import com.google.common.collect.Streams;
import com.google.errorprone.VisitorState;
import com.google.errorprone.fixes.SuggestedFix;
import com.google.errorprone.util.ErrorProneToken;
import com.google.errorprone.util.ErrorProneTokens;
import com.sun.source.tree.MethodInvocationTree;
import com.sun.source.tree.Tree;
import com.sun.tools.javac.util.JCDiagnostic.DiagnosticPosition;
import com.sun.tools.javac.util.Position;
import java.util.Optional;
/**
* A collection of Error Prone utility methods for dealing with the source code representation of
* AST nodes.
*/
// XXX: Can we locate this code in a better place? Maybe contribute it upstream?
public final class SourceCode {
/** The complement of {@link CharMatcher#whitespace()}. */
private static final CharMatcher NON_WHITESPACE_MATCHER = CharMatcher.whitespace().negate();
private SourceCode() {}
/**
@@ -41,85 +24,4 @@ public final class SourceCode {
String src = state.getSourceForNode(tree);
return src != null ? src : tree.toString();
}
/**
* Creates a {@link SuggestedFix} for the deletion of the given {@link Tree}, including any
* whitespace that follows it.
*
* <p>Removing trailing whitespace may prevent the introduction of an empty line at the start of a
* code block; such empty lines are not removed when formatting the code using Google Java Format.
*
* @param tree The AST node of interest.
* @param state A {@link VisitorState} describing the context in which the given {@link Tree} is
* found.
* @return A non-{@code null} {@link SuggestedFix} similar to one produced by {@link
* SuggestedFix#delete(Tree)}.
*/
public static SuggestedFix deleteWithTrailingWhitespace(Tree tree, VisitorState state) {
CharSequence sourceCode = state.getSourceCode();
int endPos = state.getEndPosition(tree);
if (sourceCode == null || endPos == NOPOS) {
/* We can't identify the trailing whitespace; delete just the tree. */
return SuggestedFix.delete(tree);
}
int whitespaceEndPos = NON_WHITESPACE_MATCHER.indexIn(sourceCode, endPos);
return SuggestedFix.replace(
((DiagnosticPosition) tree).getStartPosition(),
whitespaceEndPos == -1 ? sourceCode.length() : whitespaceEndPos,
"");
}
/**
* Creates a {@link SuggestedFix} for the replacement of the given {@link MethodInvocationTree}
* with just the arguments to the method invocation, effectively "unwrapping" the method
* invocation.
*
* <p>For example, given the method invocation {@code foo.bar(1, 2, 3)}, this method will return a
* {@link SuggestedFix} that replaces the method invocation with {@code 1, 2, 3}.
*
* <p>This method aims to preserve the original formatting of the method invocation, including
* whitespace and comments.
*
* @param tree The AST node to be unwrapped.
* @param state A {@link VisitorState} describing the context in which the given {@link
* MethodInvocationTree} is found.
* @return A non-{@code null} {@link SuggestedFix}.
*/
public static SuggestedFix unwrapMethodInvocation(MethodInvocationTree tree, VisitorState state) {
CharSequence sourceCode = state.getSourceCode();
int startPosition = state.getEndPosition(tree.getMethodSelect());
int endPosition = state.getEndPosition(tree);
if (sourceCode == null || startPosition == Position.NOPOS || endPosition == Position.NOPOS) {
return unwrapMethodInvocationDroppingWhitespaceAndComments(tree, state);
}
ImmutableList<ErrorProneToken> tokens =
ErrorProneTokens.getTokens(
sourceCode.subSequence(startPosition, endPosition).toString(), state.context);
Optional<Integer> leftParenPosition =
tokens.stream().findFirst().map(t -> startPosition + t.endPos());
Optional<Integer> rightParenPosition =
Streams.findLast(tokens.stream().filter(t -> t.kind() == RPAREN))
.map(t -> startPosition + t.pos());
if (leftParenPosition.isEmpty() || rightParenPosition.isEmpty()) {
return unwrapMethodInvocationDroppingWhitespaceAndComments(tree, state);
}
return SuggestedFix.replace(
tree,
sourceCode
.subSequence(leftParenPosition.orElseThrow(), rightParenPosition.orElseThrow())
.toString());
}
@VisibleForTesting
static SuggestedFix unwrapMethodInvocationDroppingWhitespaceAndComments(
MethodInvocationTree tree, VisitorState state) {
return SuggestedFix.replace(
tree,
tree.getArguments().stream().map(arg -> treeToString(arg, state)).collect(joining(", ")));
}
}

View File

@@ -4,10 +4,7 @@ import com.google.errorprone.VisitorState;
import com.google.errorprone.bugpatterns.BugChecker;
import com.google.errorprone.suppliers.Supplier;
import com.sun.tools.javac.code.ClassFinder;
import com.sun.tools.javac.code.Source;
import com.sun.tools.javac.code.Symbol.CompletionFailure;
import com.sun.tools.javac.code.Symbol.ModuleSymbol;
import com.sun.tools.javac.code.Symtab;
import com.sun.tools.javac.util.Name;
/**
@@ -32,6 +29,13 @@ public enum ThirdPartyLibrary {
* @see <a href="https://github.com/google/guava">Guava on GitHub</a>
*/
GUAVA("com.google.common.collect.ImmutableList"),
/**
* New Relic's Java agent API.
*
* @see <a href="https://github.com/newrelic/newrelic-java-agent/tree/main/newrelic-api">New Relic
* Java agent API on GitHub</a>
*/
NEW_RELIC_AGENT_API("com.newrelic.api.agent.Agent"),
/**
* VMWare's Project Reactor.
*
@@ -82,19 +86,11 @@ public enum ThirdPartyLibrary {
private static boolean canLoadClass(String className, VisitorState state) {
ClassFinder classFinder = ClassFinder.instance(state.context);
Symtab symtab = state.getSymtab();
// XXX: Drop support for targeting Java 8 once the oldest supported JDK drops such support.
ModuleSymbol module =
Source.instance(state.context).compareTo(Source.JDK9) < 0
? symtab.noModule
: symtab.unnamedModule;
Name binaryName = state.binaryNameFromClassname(className);
try {
classFinder.loadClass(module, binaryName);
classFinder.loadClass(state.getSymtab().unnamedModule, binaryName);
return true;
} catch (
@SuppressWarnings("java:S1166" /* Not exceptional. */)
CompletionFailure e) {
} catch (CompletionFailure e) {
return false;
}
}

View File

@@ -1,4 +1,4 @@
/** Auxiliary utilities for use by Error Prone checks. */
@com.google.errorprone.annotations.CheckReturnValue
@org.jspecify.annotations.NullMarked
@org.jspecify.nullness.NullMarked
package tech.picnic.errorprone.bugpatterns.util;

View File

@@ -1,151 +1,16 @@
package tech.picnic.errorprone.refasterrules;
import static com.google.errorprone.refaster.ImportPolicy.STATIC_IMPORT_ALWAYS;
import static org.assertj.core.api.Assertions.assertThat;
import com.google.common.collect.ImmutableBiMap;
import com.google.common.collect.ImmutableList;
import com.google.common.collect.ImmutableMap;
import com.google.common.collect.ImmutableMultiset;
import com.google.common.collect.ImmutableSet;
import com.google.common.collect.ImmutableSortedMap;
import com.google.common.collect.ImmutableSortedMultiset;
import com.google.common.collect.ImmutableSortedSet;
import com.google.errorprone.refaster.Refaster;
import com.google.errorprone.refaster.annotation.AfterTemplate;
import com.google.errorprone.refaster.annotation.BeforeTemplate;
import com.google.errorprone.refaster.annotation.UseImportPolicy;
import java.util.ArrayList;
import java.util.Collection;
import java.util.HashMap;
import java.util.HashSet;
import java.util.LinkedHashMap;
import java.util.LinkedHashSet;
import java.util.Map;
import java.util.Set;
import java.util.TreeMap;
import java.util.TreeSet;
import org.assertj.core.api.AbstractAssert;
import org.assertj.core.api.AbstractBooleanAssert;
import org.assertj.core.api.AbstractCollectionAssert;
import org.assertj.core.api.AbstractMapAssert;
import org.assertj.core.api.MapAssert;
import tech.picnic.errorprone.refaster.annotation.OnlineDocumentation;
@OnlineDocumentation
final class AssertJMapRules {
private AssertJMapRules() {}
// XXX: Reduce boilerplate using a `Matcher` that identifies "empty" instances.
static final class AbstractMapAssertIsEmpty<K, V> {
@BeforeTemplate
@SuppressWarnings("unchecked")
void before(AbstractMapAssert<?, ?, K, V> mapAssert) {
Refaster.anyOf(
mapAssert.containsExactlyEntriesOf(
Refaster.anyOf(
ImmutableMap.of(),
ImmutableBiMap.of(),
ImmutableSortedMap.of(),
new HashMap<>(),
new LinkedHashMap<>(),
new TreeMap<>())),
mapAssert.hasSameSizeAs(
Refaster.anyOf(
ImmutableMap.of(),
ImmutableBiMap.of(),
ImmutableSortedMap.of(),
new HashMap<>(),
new LinkedHashMap<>(),
new TreeMap<>())),
mapAssert.isEqualTo(
Refaster.anyOf(
ImmutableMap.of(),
ImmutableBiMap.of(),
ImmutableSortedMap.of(),
new HashMap<>(),
new LinkedHashMap<>(),
new TreeMap<>())),
mapAssert.containsOnlyKeys(
Refaster.anyOf(
ImmutableList.of(),
new ArrayList<>(),
ImmutableSet.of(),
new HashSet<>(),
new LinkedHashSet<>(),
ImmutableSortedSet.of(),
new TreeSet<>(),
ImmutableMultiset.of(),
ImmutableSortedMultiset.of())),
mapAssert.containsExactly(),
mapAssert.containsOnly(),
mapAssert.containsOnlyKeys());
}
@AfterTemplate
void after(AbstractMapAssert<?, ?, K, V> mapAssert) {
mapAssert.isEmpty();
}
}
static final class AssertThatMapIsEmpty<K, V> {
@BeforeTemplate
void before(Map<K, V> map) {
Refaster.anyOf(
assertThat(map).hasSize(0),
assertThat(map.isEmpty()).isTrue(),
assertThat(map.size()).isEqualTo(0L),
assertThat(map.size()).isNotPositive());
}
@BeforeTemplate
void before2(Map<K, V> map) {
assertThat(Refaster.anyOf(map.keySet(), map.values(), map.entrySet())).isEmpty();
}
@AfterTemplate
@UseImportPolicy(STATIC_IMPORT_ALWAYS)
void after(Map<K, V> map) {
assertThat(map).isEmpty();
}
}
static final class AbstractMapAssertIsNotEmpty<K, V> {
@BeforeTemplate
AbstractMapAssert<?, ?, K, V> before(AbstractMapAssert<?, ?, K, V> mapAssert) {
return mapAssert.isNotEqualTo(
Refaster.anyOf(
ImmutableMap.of(),
ImmutableBiMap.of(),
ImmutableSortedMap.of(),
new HashMap<>(),
new LinkedHashMap<>(),
new TreeMap<>()));
}
@AfterTemplate
AbstractMapAssert<?, ?, K, V> after(AbstractMapAssert<?, ?, K, V> mapAssert) {
return mapAssert.isNotEmpty();
}
}
static final class AssertThatMapIsNotEmpty<K, V> {
@BeforeTemplate
AbstractAssert<?, ?> before(Map<K, V> map) {
return Refaster.anyOf(
assertThat(map.isEmpty()).isFalse(),
assertThat(map.size()).isNotEqualTo(0),
assertThat(map.size()).isPositive(),
assertThat(Refaster.anyOf(map.keySet(), map.values(), map.entrySet())).isNotEmpty());
}
@AfterTemplate
@UseImportPolicy(STATIC_IMPORT_ALWAYS)
MapAssert<K, V> after(Map<K, V> map) {
return assertThat(map).isNotEmpty();
}
}
static final class AbstractMapAssertContainsExactlyInAnyOrderEntriesOf<K, V> {
@BeforeTemplate
AbstractMapAssert<?, ?, K, V> before(AbstractMapAssert<?, ?, K, V> mapAssert, Map<K, V> map) {
@@ -169,96 +34,4 @@ final class AssertJMapRules {
return mapAssert.containsExactlyEntriesOf(ImmutableMap.of(key, value));
}
}
static final class AssertThatMapHasSize<K, V> {
@BeforeTemplate
AbstractAssert<?, ?> before(Map<K, V> map, int length) {
return Refaster.anyOf(
assertThat(map.size()).isEqualTo(length),
assertThat(Refaster.anyOf(map.keySet(), map.values(), map.entrySet())).hasSize(length));
}
@AfterTemplate
@UseImportPolicy(STATIC_IMPORT_ALWAYS)
MapAssert<K, V> after(Map<K, V> map, int length) {
return assertThat(map).hasSize(length);
}
}
static final class AbstractMapAssertHasSameSizeAs<K, V> {
@BeforeTemplate
AbstractMapAssert<?, ?, K, V> before(AbstractMapAssert<?, ?, K, V> mapAssert, Map<K, V> map) {
return mapAssert.hasSize(map.size());
}
@AfterTemplate
AbstractMapAssert<?, ?, K, V> after(AbstractMapAssert<?, ?, K, V> mapAssert, Map<K, V> map) {
return mapAssert.hasSameSizeAs(map);
}
}
static final class AssertThatMapContainsKey<K, V> {
@BeforeTemplate
AbstractBooleanAssert<?> before(Map<K, V> map, K key) {
return assertThat(map.containsKey(key)).isTrue();
}
@AfterTemplate
@UseImportPolicy(STATIC_IMPORT_ALWAYS)
MapAssert<K, V> after(Map<K, V> map, K key) {
return assertThat(map).containsKey(key);
}
}
static final class AssertThatMapDoesNotContainKey<K, V> {
@BeforeTemplate
AbstractBooleanAssert<?> before(Map<K, V> map, K key) {
return assertThat(map.containsKey(key)).isFalse();
}
@AfterTemplate
@UseImportPolicy(STATIC_IMPORT_ALWAYS)
MapAssert<K, V> after(Map<K, V> map, K key) {
return assertThat(map).doesNotContainKey(key);
}
}
static final class AssertThatMapContainsOnlyKeys<K, V> {
@BeforeTemplate
AbstractCollectionAssert<?, Collection<? extends K>, K, ?> before(Map<K, V> map, Set<K> keys) {
return assertThat(map.keySet()).hasSameElementsAs(keys);
}
@AfterTemplate
@UseImportPolicy(STATIC_IMPORT_ALWAYS)
MapAssert<K, V> after(Map<K, V> map, Set<K> keys) {
return assertThat(map).containsOnlyKeys(keys);
}
}
static final class AssertThatMapContainsValue<K, V> {
@BeforeTemplate
AbstractBooleanAssert<?> before(Map<K, V> map, V value) {
return assertThat(map.containsValue(value)).isTrue();
}
@AfterTemplate
@UseImportPolicy(STATIC_IMPORT_ALWAYS)
MapAssert<K, V> after(Map<K, V> map, V value) {
return assertThat(map).containsValue(value);
}
}
static final class AssertThatMapDoesNotContainValue<K, V> {
@BeforeTemplate
AbstractBooleanAssert<?> before(Map<K, V> map, V value) {
return assertThat(map.containsValue(value)).isFalse();
}
@AfterTemplate
@UseImportPolicy(STATIC_IMPORT_ALWAYS)
MapAssert<K, V> after(Map<K, V> map, V value) {
return assertThat(map).doesNotContainValue(value);
}
}
}

View File

@@ -22,6 +22,7 @@ final class AssertJOptionalRules {
static final class AssertThatOptional<T> {
@BeforeTemplate
@SuppressWarnings("NullAway")
ObjectAssert<T> before(Optional<T> optional) {
return assertThat(optional.orElseThrow());
}

View File

@@ -23,8 +23,6 @@ final class AssertJPrimitiveRules {
}
@BeforeTemplate
@SuppressWarnings(
"java:S1244" /* The (in)equality checks are fragile, but may be seen in the wild. */)
AbstractBooleanAssert<?> before(double actual, double expected) {
return Refaster.anyOf(
assertThat(actual == expected).isTrue(), assertThat(actual != expected).isFalse());
@@ -45,8 +43,6 @@ final class AssertJPrimitiveRules {
}
@BeforeTemplate
@SuppressWarnings(
"java:S1244" /* The (in)equality checks are fragile, but may be seen in the wild. */)
AbstractBooleanAssert<?> before(double actual, double expected) {
return Refaster.anyOf(
assertThat(actual != expected).isTrue(), assertThat(actual == expected).isFalse());

View File

@@ -3,9 +3,12 @@ package tech.picnic.errorprone.refasterrules;
import static com.google.errorprone.refaster.ImportPolicy.STATIC_IMPORT_ALWAYS;
import static org.assertj.core.api.Assertions.assertThat;
import com.google.common.collect.ImmutableBiMap;
import com.google.common.collect.ImmutableList;
import com.google.common.collect.ImmutableMap;
import com.google.common.collect.ImmutableMultiset;
import com.google.common.collect.ImmutableSet;
import com.google.common.collect.ImmutableSortedMap;
import com.google.common.collect.ImmutableSortedMultiset;
import com.google.common.collect.ImmutableSortedSet;
import com.google.common.collect.Iterables;
@@ -19,7 +22,9 @@ import com.google.errorprone.refaster.annotation.UseImportPolicy;
import java.util.ArrayList;
import java.util.Arrays;
import java.util.Collection;
import java.util.HashMap;
import java.util.HashSet;
import java.util.LinkedHashMap;
import java.util.LinkedHashSet;
import java.util.List;
import java.util.Map;
@@ -27,16 +32,19 @@ import java.util.OptionalDouble;
import java.util.OptionalInt;
import java.util.OptionalLong;
import java.util.Set;
import java.util.TreeMap;
import java.util.TreeSet;
import java.util.function.Predicate;
import java.util.stream.Collector;
import java.util.stream.Stream;
import org.assertj.core.api.AbstractAssert;
import org.assertj.core.api.AbstractBooleanAssert;
import org.assertj.core.api.AbstractCollectionAssert;
import org.assertj.core.api.AbstractComparableAssert;
import org.assertj.core.api.AbstractDoubleAssert;
import org.assertj.core.api.AbstractIntegerAssert;
import org.assertj.core.api.AbstractLongAssert;
import org.assertj.core.api.AbstractMapAssert;
import org.assertj.core.api.IterableAssert;
import org.assertj.core.api.ListAssert;
import org.assertj.core.api.MapAssert;
@@ -558,8 +566,173 @@ final class AssertJRules {
// Map
//
// XXX: To match in all cases there'll need to be a `@BeforeTemplate` variant for each
// `assertThat` overload. Consider defining a `BugChecker` instead.
static final class AssertThatMapIsEmpty<K, V> {
@BeforeTemplate
@SuppressWarnings("unchecked")
void before(AbstractMapAssert<?, ?, K, V> mapAssert) {
Refaster.anyOf(
mapAssert.containsExactlyEntriesOf(
Refaster.anyOf(
ImmutableMap.of(),
ImmutableBiMap.of(),
ImmutableSortedMap.of(),
new HashMap<>(),
new LinkedHashMap<>(),
new TreeMap<>())),
mapAssert.hasSameSizeAs(
Refaster.anyOf(
ImmutableMap.of(),
ImmutableBiMap.of(),
ImmutableSortedMap.of(),
new HashMap<>(),
new LinkedHashMap<>(),
new TreeMap<>())),
mapAssert.isEqualTo(
Refaster.anyOf(
ImmutableMap.of(),
ImmutableBiMap.of(),
ImmutableSortedMap.of(),
new HashMap<>(),
new LinkedHashMap<>(),
new TreeMap<>())),
mapAssert.containsOnlyKeys(
Refaster.anyOf(
ImmutableList.of(),
new ArrayList<>(),
ImmutableSet.of(),
new HashSet<>(),
new LinkedHashSet<>(),
ImmutableSortedSet.of(),
new TreeSet<>(),
ImmutableMultiset.of(),
ImmutableSortedMultiset.of())),
mapAssert.containsExactly(),
mapAssert.containsOnly(),
mapAssert.containsOnlyKeys());
}
@AfterTemplate
void after(AbstractMapAssert<?, ?, K, V> mapAssert) {
mapAssert.isEmpty();
}
}
// XXX: Find a better name.
static final class AssertThatMapIsEmpty2<K, V> {
@BeforeTemplate
void before(Map<K, V> map) {
Refaster.anyOf(
assertThat(map).hasSize(0),
assertThat(map.isEmpty()).isTrue(),
assertThat(map.size()).isEqualTo(0L),
assertThat(map.size()).isNotPositive());
}
@BeforeTemplate
void before2(Map<K, V> map) {
assertThat(Refaster.anyOf(map.keySet(), map.values())).isEmpty();
}
@AfterTemplate
@UseImportPolicy(STATIC_IMPORT_ALWAYS)
void after(Map<K, V> map) {
assertThat(map).isEmpty();
}
}
static final class AssertThatMapIsNotEmpty<K, V> {
@BeforeTemplate
AbstractMapAssert<?, ?, K, V> before(AbstractMapAssert<?, ?, K, V> mapAssert) {
return mapAssert.isNotEqualTo(
Refaster.anyOf(
ImmutableMap.of(),
ImmutableBiMap.of(),
ImmutableSortedMap.of(),
new HashMap<>(),
new LinkedHashMap<>(),
new TreeMap<>()));
}
@AfterTemplate
AbstractMapAssert<?, ?, K, V> after(AbstractMapAssert<?, ?, K, V> mapAssert) {
return mapAssert.isNotEmpty();
}
}
// XXX: Find a better name.
static final class AssertThatMapIsNotEmpty2<K, V> {
@BeforeTemplate
AbstractAssert<?, ?> before(Map<K, V> map) {
return Refaster.anyOf(
assertThat(map.isEmpty()).isFalse(),
assertThat(map.size()).isNotEqualTo(0),
assertThat(map.size()).isPositive(),
assertThat(Refaster.anyOf(map.keySet(), map.values())).isNotEmpty());
}
@AfterTemplate
@UseImportPolicy(STATIC_IMPORT_ALWAYS)
MapAssert<K, V> after(Map<K, V> map) {
return assertThat(map).isNotEmpty();
}
}
static final class AssertThatMapHasSize<K, V> {
@BeforeTemplate
AbstractAssert<?, ?> before(Map<K, V> map, int length) {
return Refaster.anyOf(
assertThat(map.size()).isEqualTo(length),
assertThat(Refaster.anyOf(map.keySet(), map.values())).hasSize(length));
}
@AfterTemplate
@UseImportPolicy(STATIC_IMPORT_ALWAYS)
MapAssert<K, V> after(Map<K, V> map, int length) {
return assertThat(map).hasSize(length);
}
}
static final class AssertThatMapsHaveSameSize<K, V> {
@BeforeTemplate
AbstractAssert<?, ?> before(Map<K, V> map1, Map<K, V> map2) {
return assertThat(map1)
.hasSize(Refaster.anyOf(map2.size(), map2.keySet().size(), map2.values().size()));
}
@AfterTemplate
@UseImportPolicy(STATIC_IMPORT_ALWAYS)
MapAssert<K, V> after(Map<K, V> map1, Map<K, V> map2) {
return assertThat(map1).hasSameSizeAs(map2);
}
}
// XXX: Should also add a rule (elsewhere) to simplify `map.keySet().contains(key)`.
static final class AssertThatMapContainsKey<K, V> {
@BeforeTemplate
AbstractBooleanAssert<?> before(Map<K, V> map, K key) {
return assertThat(map.containsKey(key)).isTrue();
}
@AfterTemplate
@UseImportPolicy(STATIC_IMPORT_ALWAYS)
MapAssert<K, V> after(Map<K, V> map, K key) {
return assertThat(map).containsKey(key);
}
}
static final class AssertThatMapDoesNotContainKey<K, V> {
@BeforeTemplate
AbstractBooleanAssert<?> before(Map<K, V> map, K key) {
return assertThat(map.containsKey(key)).isFalse();
}
@AfterTemplate
@UseImportPolicy(STATIC_IMPORT_ALWAYS)
MapAssert<K, V> after(Map<K, V> map, K key) {
return assertThat(map).doesNotContainKey(key);
}
}
static final class AssertThatMapContainsEntry<K, V> {
@BeforeTemplate
ObjectAssert<?> before(Map<K, V> map, K key, V value) {
@@ -2024,8 +2197,8 @@ final class AssertJRules {
////////////////////////////////////////////////////////////////////////////
// Above: Generated code.
////////////////////////////////////////////////////////////////////////////
////////////////////////////////////////////////////////////////////////////
///////////////////////////////////////////////////////////////////////////////////////////////////////
///////////////////////////////////////////////////////////////////////////////////////////////////////
// Organize the code below.
// XXX: Do the "single Comparable" match shown below.

View File

@@ -1,16 +1,11 @@
package tech.picnic.errorprone.refasterrules;
import static com.google.errorprone.refaster.ImportPolicy.STATIC_IMPORT_ALWAYS;
import static java.nio.charset.StandardCharsets.UTF_8;
import static org.assertj.core.api.Assertions.assertThat;
import com.google.errorprone.refaster.annotation.AfterTemplate;
import com.google.errorprone.refaster.annotation.BeforeTemplate;
import com.google.errorprone.refaster.annotation.UseImportPolicy;
import java.io.IOException;
import java.nio.charset.Charset;
import java.nio.file.Files;
import java.nio.file.Path;
import org.assertj.core.api.AbstractAssert;
import org.assertj.core.api.AbstractStringAssert;
import tech.picnic.errorprone.refaster.annotation.OnlineDocumentation;
@@ -94,30 +89,4 @@ final class AssertJStringRules {
return assertThat(string).doesNotMatch(regex);
}
}
static final class AssertThatPathContent {
@BeforeTemplate
AbstractStringAssert<?> before(Path path, Charset charset) throws IOException {
return assertThat(Files.readString(path, charset));
}
@AfterTemplate
@UseImportPolicy(STATIC_IMPORT_ALWAYS)
AbstractStringAssert<?> after(Path path, Charset charset) {
return assertThat(path).content(charset);
}
}
static final class AssertThatPathContentUtf8 {
@BeforeTemplate
AbstractStringAssert<?> before(Path path) throws IOException {
return assertThat(Files.readString(path));
}
@AfterTemplate
@UseImportPolicy(STATIC_IMPORT_ALWAYS)
AbstractStringAssert<?> after(Path path) {
return assertThat(path).content(UTF_8);
}
}
}

View File

@@ -47,7 +47,7 @@ final class AssertJThrowingCallableRules {
static final class AssertThatThrownByIllegalArgumentExceptionHasMessage {
@BeforeTemplate
@SuppressWarnings(
"AssertThatThrownByIllegalArgumentException" /* This is a more specific template. */)
"AssertThatThrownByIllegalArgumentException" /* Matches strictly more specific expressions. */)
AbstractObjectAssert<?, ?> before(ThrowingCallable throwingCallable, String message) {
return assertThatIllegalArgumentException().isThrownBy(throwingCallable).withMessage(message);
}
@@ -64,7 +64,7 @@ final class AssertJThrowingCallableRules {
static final class AssertThatThrownByIllegalArgumentExceptionHasMessageParameters {
@BeforeTemplate
@SuppressWarnings(
"AssertThatThrownByIllegalArgumentException" /* This is a more specific template. */)
"AssertThatThrownByIllegalArgumentException" /* Matches strictly more specific expressions. */)
AbstractObjectAssert<?, ?> before(
ThrowingCallable throwingCallable, String message, @Repeated Object parameters) {
return assertThatIllegalArgumentException()
@@ -85,7 +85,7 @@ final class AssertJThrowingCallableRules {
static final class AssertThatThrownByIllegalArgumentExceptionHasMessageStartingWith {
@BeforeTemplate
@SuppressWarnings(
"AssertThatThrownByIllegalArgumentException" /* This is a more specific template. */)
"AssertThatThrownByIllegalArgumentException" /* Matches strictly more specific expressions. */)
AbstractObjectAssert<?, ?> before(ThrowingCallable throwingCallable, String message) {
return assertThatIllegalArgumentException()
.isThrownBy(throwingCallable)
@@ -104,7 +104,7 @@ final class AssertJThrowingCallableRules {
static final class AssertThatThrownByIllegalArgumentExceptionHasMessageContaining {
@BeforeTemplate
@SuppressWarnings(
"AssertThatThrownByIllegalArgumentException" /* This is a more specific template. */)
"AssertThatThrownByIllegalArgumentException" /* Matches strictly more specific expressions. */)
AbstractObjectAssert<?, ?> before(ThrowingCallable throwingCallable, String message) {
return assertThatIllegalArgumentException()
.isThrownBy(throwingCallable)
@@ -123,7 +123,7 @@ final class AssertJThrowingCallableRules {
static final class AssertThatThrownByIllegalArgumentExceptionHasMessageNotContainingAny {
@BeforeTemplate
@SuppressWarnings(
"AssertThatThrownByIllegalArgumentException" /* This is a more specific template. */)
"AssertThatThrownByIllegalArgumentException" /* Matches strictly more specific expressions. */)
AbstractObjectAssert<?, ?> before(
ThrowingCallable throwingCallable, @Repeated CharSequence values) {
return assertThatIllegalArgumentException()
@@ -157,7 +157,7 @@ final class AssertJThrowingCallableRules {
static final class AssertThatThrownByIllegalStateExceptionHasMessage {
@BeforeTemplate
@SuppressWarnings(
"AssertThatThrownByIllegalStateException" /* This is a more specific template. */)
"AssertThatThrownByIllegalStateException" /* Matches strictly more specific expressions. */)
AbstractObjectAssert<?, ?> before(ThrowingCallable throwingCallable, String message) {
return assertThatIllegalStateException().isThrownBy(throwingCallable).withMessage(message);
}
@@ -174,7 +174,7 @@ final class AssertJThrowingCallableRules {
static final class AssertThatThrownByIllegalStateExceptionHasMessageParameters {
@BeforeTemplate
@SuppressWarnings(
"AssertThatThrownByIllegalStateException" /* This is a more specific template. */)
"AssertThatThrownByIllegalStateException" /* Matches strictly more specific expressions. */)
AbstractObjectAssert<?, ?> before(
ThrowingCallable throwingCallable, String message, @Repeated Object parameters) {
return assertThatIllegalStateException()
@@ -195,7 +195,7 @@ final class AssertJThrowingCallableRules {
static final class AssertThatThrownByIllegalStateExceptionHasMessageStartingWith {
@BeforeTemplate
@SuppressWarnings(
"AssertThatThrownByIllegalStateException" /* This is a more specific template. */)
"AssertThatThrownByIllegalStateException" /* Matches strictly more specific expressions. */)
AbstractObjectAssert<?, ?> before(ThrowingCallable throwingCallable, String message) {
return assertThatIllegalStateException()
.isThrownBy(throwingCallable)
@@ -214,7 +214,7 @@ final class AssertJThrowingCallableRules {
static final class AssertThatThrownByIllegalStateExceptionHasMessageContaining {
@BeforeTemplate
@SuppressWarnings(
"AssertThatThrownByIllegalStateException" /* This is a more specific template. */)
"AssertThatThrownByIllegalStateException" /* Matches strictly more specific expressions. */)
AbstractObjectAssert<?, ?> before(ThrowingCallable throwingCallable, String message) {
return assertThatIllegalStateException()
.isThrownBy(throwingCallable)
@@ -233,7 +233,7 @@ final class AssertJThrowingCallableRules {
static final class AssertThatThrownByIllegalStateExceptionHasMessageNotContaining {
@BeforeTemplate
@SuppressWarnings(
"AssertThatThrownByIllegalStateException" /* This is a more specific template. */)
"AssertThatThrownByIllegalStateException" /* Matches strictly more specific expressions. */)
AbstractObjectAssert<?, ?> before(ThrowingCallable throwingCallable, String message) {
return assertThatIllegalStateException()
.isThrownBy(throwingCallable)
@@ -265,7 +265,7 @@ final class AssertJThrowingCallableRules {
static final class AssertThatThrownByNullPointerExceptionHasMessage {
@BeforeTemplate
@SuppressWarnings(
"AssertThatThrownByNullPointerException" /* This is a more specific template. */)
"AssertThatThrownByNullPointerException" /* Matches strictly more specific expressions. */)
AbstractObjectAssert<?, ?> before(ThrowingCallable throwingCallable, String message) {
return assertThatNullPointerException().isThrownBy(throwingCallable).withMessage(message);
}
@@ -282,7 +282,7 @@ final class AssertJThrowingCallableRules {
static final class AssertThatThrownByNullPointerExceptionHasMessageParameters {
@BeforeTemplate
@SuppressWarnings(
"AssertThatThrownByNullPointerException" /* This is a more specific template. */)
"AssertThatThrownByNullPointerException" /* Matches strictly more specific expressions. */)
AbstractObjectAssert<?, ?> before(
ThrowingCallable throwingCallable, String message, @Repeated Object parameters) {
return assertThatNullPointerException()
@@ -303,7 +303,7 @@ final class AssertJThrowingCallableRules {
static final class AssertThatThrownByNullPointerExceptionHasMessageStartingWith {
@BeforeTemplate
@SuppressWarnings(
"AssertThatThrownByNullPointerException" /* This is a more specific template. */)
"AssertThatThrownByNullPointerException" /* Matches strictly more specific expressions. */)
AbstractObjectAssert<?, ?> before(ThrowingCallable throwingCallable, String message) {
return assertThatNullPointerException()
.isThrownBy(throwingCallable)
@@ -319,44 +319,6 @@ final class AssertJThrowingCallableRules {
}
}
static final class AssertThatThrownByNullPointerExceptionHasMessageContaining {
@BeforeTemplate
@SuppressWarnings(
"AssertThatThrownByNullPointerException" /* This is a more specific template. */)
AbstractObjectAssert<?, ?> before(ThrowingCallable throwingCallable, String message) {
return assertThatNullPointerException()
.isThrownBy(throwingCallable)
.withMessageContaining(message);
}
@AfterTemplate
@UseImportPolicy(STATIC_IMPORT_ALWAYS)
AbstractObjectAssert<?, ?> after(ThrowingCallable throwingCallable, String message) {
return assertThatThrownBy(throwingCallable)
.isInstanceOf(NullPointerException.class)
.hasMessageContaining(message);
}
}
static final class AssertThatThrownByNullPointerExceptionHasMessageNotContaining {
@BeforeTemplate
@SuppressWarnings(
"AssertThatThrownByNullPointerException" /* This is a more specific template. */)
AbstractObjectAssert<?, ?> before(ThrowingCallable throwingCallable, String message) {
return assertThatNullPointerException()
.isThrownBy(throwingCallable)
.withMessageNotContaining(message);
}
@AfterTemplate
@UseImportPolicy(STATIC_IMPORT_ALWAYS)
AbstractObjectAssert<?, ?> after(ThrowingCallable throwingCallable, String message) {
return assertThatThrownBy(throwingCallable)
.isInstanceOf(NullPointerException.class)
.hasMessageNotContaining(message);
}
}
static final class AssertThatThrownByIOException {
@BeforeTemplate
AbstractObjectAssert<?, ?> before(ThrowingCallable throwingCallable) {
@@ -372,7 +334,8 @@ final class AssertJThrowingCallableRules {
static final class AssertThatThrownByIOExceptionHasMessage {
@BeforeTemplate
@SuppressWarnings("AssertThatThrownByIOException" /* This is a more specific template. */)
@SuppressWarnings(
"AssertThatThrownByIOException" /* Matches strictly more specific expressions. */)
AbstractObjectAssert<?, ?> before(ThrowingCallable throwingCallable, String message) {
return assertThatIOException().isThrownBy(throwingCallable).withMessage(message);
}
@@ -388,7 +351,8 @@ final class AssertJThrowingCallableRules {
static final class AssertThatThrownByIOExceptionHasMessageParameters {
@BeforeTemplate
@SuppressWarnings("AssertThatThrownByIOException" /* This is a more specific template. */)
@SuppressWarnings(
"AssertThatThrownByIOException" /* Matches strictly more specific expressions. */)
AbstractObjectAssert<?, ?> before(
ThrowingCallable throwingCallable, String message, @Repeated Object parameters) {
return assertThatIOException().isThrownBy(throwingCallable).withMessage(message, parameters);
@@ -404,54 +368,6 @@ final class AssertJThrowingCallableRules {
}
}
static final class AssertThatThrownByIOExceptionHasMessageStartingWith {
@BeforeTemplate
@SuppressWarnings("AssertThatThrownByIOException" /* This is a more specific template. */)
AbstractObjectAssert<?, ?> before(ThrowingCallable throwingCallable, String message) {
return assertThatIOException().isThrownBy(throwingCallable).withMessageStartingWith(message);
}
@AfterTemplate
@UseImportPolicy(STATIC_IMPORT_ALWAYS)
AbstractObjectAssert<?, ?> after(ThrowingCallable throwingCallable, String message) {
return assertThatThrownBy(throwingCallable)
.isInstanceOf(IOException.class)
.hasMessageStartingWith(message);
}
}
static final class AssertThatThrownByIOExceptionHasMessageContaining {
@BeforeTemplate
@SuppressWarnings("AssertThatThrownByIOException" /* This is a more specific template. */)
AbstractObjectAssert<?, ?> before(ThrowingCallable throwingCallable, String message) {
return assertThatIOException().isThrownBy(throwingCallable).withMessageContaining(message);
}
@AfterTemplate
@UseImportPolicy(STATIC_IMPORT_ALWAYS)
AbstractObjectAssert<?, ?> after(ThrowingCallable throwingCallable, String message) {
return assertThatThrownBy(throwingCallable)
.isInstanceOf(IOException.class)
.hasMessageContaining(message);
}
}
static final class AssertThatThrownByIOExceptionHasMessageNotContaining {
@BeforeTemplate
@SuppressWarnings("AssertThatThrownByIOException" /* This is a more specific template. */)
AbstractObjectAssert<?, ?> before(ThrowingCallable throwingCallable, String message) {
return assertThatIOException().isThrownBy(throwingCallable).withMessageNotContaining(message);
}
@AfterTemplate
@UseImportPolicy(STATIC_IMPORT_ALWAYS)
AbstractObjectAssert<?, ?> after(ThrowingCallable throwingCallable, String message) {
return assertThatThrownBy(throwingCallable)
.isInstanceOf(IOException.class)
.hasMessageNotContaining(message);
}
}
static final class AssertThatThrownBy {
@BeforeTemplate
AbstractObjectAssert<?, ?> before(
@@ -469,7 +385,7 @@ final class AssertJThrowingCallableRules {
static final class AssertThatThrownByHasMessage {
@BeforeTemplate
@SuppressWarnings("AssertThatThrownBy" /* This is a more specific template. */)
@SuppressWarnings("AssertThatThrownBy" /* Matches strictly more specific expressions. */)
AbstractObjectAssert<?, ?> before(
Class<? extends Throwable> exceptionType,
ThrowingCallable throwingCallable,
@@ -491,7 +407,7 @@ final class AssertJThrowingCallableRules {
static final class AssertThatThrownByHasMessageParameters {
@BeforeTemplate
@SuppressWarnings("AssertThatThrownBy" /* This is a more specific template. */)
@SuppressWarnings("AssertThatThrownBy" /* Matches strictly more specific expressions. */)
AbstractObjectAssert<?, ?> before(
Class<? extends Throwable> exceptionType,
ThrowingCallable throwingCallable,
@@ -515,78 +431,6 @@ final class AssertJThrowingCallableRules {
}
}
static final class AssertThatThrownByHasMessageStartingWith {
@BeforeTemplate
@SuppressWarnings("AssertThatThrownBy" /* This is a more specific template. */)
AbstractObjectAssert<?, ?> before(
Class<? extends Throwable> exceptionType,
ThrowingCallable throwingCallable,
String message) {
return assertThatExceptionOfType(exceptionType)
.isThrownBy(throwingCallable)
.withMessageStartingWith(message);
}
@AfterTemplate
@UseImportPolicy(STATIC_IMPORT_ALWAYS)
AbstractObjectAssert<?, ?> after(
Class<? extends Throwable> exceptionType,
ThrowingCallable throwingCallable,
String message) {
return assertThatThrownBy(throwingCallable)
.isInstanceOf(exceptionType)
.hasMessageStartingWith(message);
}
}
static final class AssertThatThrownByHasMessageContaining {
@BeforeTemplate
@SuppressWarnings("AssertThatThrownBy" /* This is a more specific template. */)
AbstractObjectAssert<?, ?> before(
Class<? extends Throwable> exceptionType,
ThrowingCallable throwingCallable,
String message) {
return assertThatExceptionOfType(exceptionType)
.isThrownBy(throwingCallable)
.withMessageContaining(message);
}
@AfterTemplate
@UseImportPolicy(STATIC_IMPORT_ALWAYS)
AbstractObjectAssert<?, ?> after(
Class<? extends Throwable> exceptionType,
ThrowingCallable throwingCallable,
String message) {
return assertThatThrownBy(throwingCallable)
.isInstanceOf(exceptionType)
.hasMessageContaining(message);
}
}
static final class AssertThatThrownByHasMessageNotContaining {
@BeforeTemplate
@SuppressWarnings("AssertThatThrownBy" /* This is a more specific template. */)
AbstractObjectAssert<?, ?> before(
Class<? extends Throwable> exceptionType,
ThrowingCallable throwingCallable,
String message) {
return assertThatExceptionOfType(exceptionType)
.isThrownBy(throwingCallable)
.withMessageNotContaining(message);
}
@AfterTemplate
@UseImportPolicy(STATIC_IMPORT_ALWAYS)
AbstractObjectAssert<?, ?> after(
Class<? extends Throwable> exceptionType,
ThrowingCallable throwingCallable,
String message) {
return assertThatThrownBy(throwingCallable)
.isInstanceOf(exceptionType)
.hasMessageNotContaining(message);
}
}
// XXX: Drop this rule in favour of a generic Error Prone check that flags `String.format(...)`
// arguments to a wide range of format methods.
static final class AbstractThrowableAssertHasMessage {

View File

@@ -19,12 +19,15 @@ import com.google.errorprone.refaster.annotation.BeforeTemplate;
import com.google.errorprone.refaster.annotation.UseImportPolicy;
import java.util.Collection;
import java.util.Collections;
import java.util.EnumMap;
import java.util.HashMap;
import java.util.HashSet;
import java.util.Iterator;
import java.util.Map;
import java.util.Objects;
import java.util.Set;
import java.util.stream.Stream;
import org.jspecify.annotations.Nullable;
import org.jspecify.nullness.Nullable;
import tech.picnic.errorprone.refaster.annotation.OnlineDocumentation;
/**
@@ -70,6 +73,32 @@ final class AssortedRules {
}
}
// XXX: We could add a rule for `new EnumMap(Map<K, ? extends V> m)`, but that constructor does
// not allow an empty non-EnumMap to be provided.
static final class CreateEnumMap<K extends Enum<K>, V> {
@BeforeTemplate
Map<K, V> before() {
return new HashMap<>();
}
@AfterTemplate
Map<K, V> after() {
return new EnumMap<>(Refaster.<K>clazz());
}
}
static final class MapGetOrNull<K, V, L> {
@BeforeTemplate
@Nullable V before(Map<K, V> map, L key) {
return map.getOrDefault(key, null);
}
@AfterTemplate
@Nullable V after(Map<K, V> map, L key) {
return map.get(key);
}
}
/**
* Use {@link Sets#toImmutableEnumSet()} when possible, as it is more efficient than {@link
* ImmutableSet#toImmutableSet()} and produces a more compact object.
@@ -114,7 +143,6 @@ final class AssortedRules {
// intelligently.
static final class LogicalImplication {
@BeforeTemplate
@SuppressWarnings("java:S2589" /* This violation will be rewritten. */)
boolean before(boolean firstTest, boolean secondTest) {
return firstTest || (!firstTest && secondTest);
}
@@ -196,6 +224,32 @@ final class AssortedRules {
}
}
/** Don't unnecessarily use {@link Map#entrySet()}. */
static final class MapKeyStream<K, V> {
@BeforeTemplate
Stream<K> before(Map<K, V> map) {
return map.entrySet().stream().map(Map.Entry::getKey);
}
@AfterTemplate
Stream<K> after(Map<K, V> map) {
return map.keySet().stream();
}
}
/** Don't unnecessarily use {@link Map#entrySet()}. */
static final class MapValueStream<K, V> {
@BeforeTemplate
Stream<V> before(Map<K, V> map) {
return map.entrySet().stream().map(Map.Entry::getValue);
}
@AfterTemplate
Stream<V> after(Map<K, V> map) {
return map.values().stream();
}
}
/** Prefer {@link Splitter#splitToStream(CharSequence)} over less efficient alternatives. */
static final class SplitToStream {
@BeforeTemplate

View File

@@ -50,19 +50,17 @@ final class BigDecimalRules {
}
}
/** Prefer {@link BigDecimal#valueOf(double)} over the associated constructor. */
// XXX: Ideally we also rewrite `new BigDecimal("<some-integer-value>")` in cases where the
// specified number can be represented as an `int` or `long`, but that requires a custom
// `BugChecker`.
static final class BigDecimalValueOf {
/** Prefer {@link BigDecimal#valueOf(long)} over the associated constructor. */
// XXX: Ideally we'd also rewrite `BigDecimal.valueOf("<some-integer-value>")`, but it doesn't
// appear that's currently possible with Error Prone.
static final class BigDecimalFactoryMethod {
@BeforeTemplate
@SuppressWarnings("java:S2111" /* This violation will be rewritten. */)
BigDecimal before(double value) {
BigDecimal before(long value) {
return new BigDecimal(value);
}
@AfterTemplate
BigDecimal after(double value) {
BigDecimal after(long value) {
return BigDecimal.valueOf(value);
}
}

View File

@@ -1,55 +0,0 @@
package tech.picnic.errorprone.refasterrules;
import com.google.errorprone.BugCheckerRefactoringTestHelper;
import com.google.errorprone.BugCheckerRefactoringTestHelper.FixChooser;
import com.google.errorprone.BugCheckerRefactoringTestHelper.FixChoosers;
import com.google.errorprone.annotations.CanIgnoreReturnValue;
import com.google.errorprone.refaster.Refaster;
import com.google.errorprone.refaster.annotation.AfterTemplate;
import com.google.errorprone.refaster.annotation.BeforeTemplate;
import tech.picnic.errorprone.refaster.annotation.OnlineDocumentation;
/** Refaster rules related to {@link com.google.errorprone.bugpatterns.BugChecker} classes. */
@OnlineDocumentation
final class BugCheckerRules {
private BugCheckerRules() {}
/**
* Avoid calling {@link BugCheckerRefactoringTestHelper#setFixChooser(FixChooser)} or {@link
* BugCheckerRefactoringTestHelper#setImportOrder(String)} with their respective default values.
*/
static final class BugCheckerRefactoringTestHelperIdentity {
@BeforeTemplate
BugCheckerRefactoringTestHelper before(BugCheckerRefactoringTestHelper helper) {
return Refaster.anyOf(
helper.setFixChooser(FixChoosers.FIRST), helper.setImportOrder("static-first"));
}
@AfterTemplate
@CanIgnoreReturnValue
BugCheckerRefactoringTestHelper after(BugCheckerRefactoringTestHelper helper) {
return helper;
}
}
/**
* Prefer {@link BugCheckerRefactoringTestHelper.ExpectOutput#expectUnchanged()} over repeating
* the input.
*/
// XXX: This rule assumes that the full source code is specified as a single string, e.g. using a
// text block. Support for multi-line source code input would require a `BugChecker`
// implementation instead.
static final class BugCheckerRefactoringTestHelperAddInputLinesExpectUnchanged {
@BeforeTemplate
BugCheckerRefactoringTestHelper before(
BugCheckerRefactoringTestHelper helper, String path, String source) {
return helper.addInputLines(path, source).addOutputLines(path, source);
}
@AfterTemplate
BugCheckerRefactoringTestHelper after(
BugCheckerRefactoringTestHelper helper, String path, String source) {
return helper.addInputLines(path, source).expectUnchanged();
}
}
}

View File

@@ -1,65 +0,0 @@
package tech.picnic.errorprone.refasterrules;
import com.google.errorprone.refaster.Refaster;
import com.google.errorprone.refaster.annotation.AfterTemplate;
import com.google.errorprone.refaster.annotation.BeforeTemplate;
import java.util.function.Predicate;
import tech.picnic.errorprone.refaster.annotation.OnlineDocumentation;
/** Refaster rules related to expressions dealing with classes. */
@OnlineDocumentation
final class ClassRules {
private ClassRules() {}
/** Prefer {@link Class#isInstance(Object)} over more contrived alternatives. */
static final class ClassIsInstance<T, S> {
@BeforeTemplate
boolean before(Class<T> clazz, S object) {
return clazz.isAssignableFrom(object.getClass());
}
@AfterTemplate
boolean after(Class<T> clazz, S object) {
return clazz.isInstance(object);
}
}
/** Prefer using the {@code instanceof} keyword over less idiomatic alternatives. */
static final class Instanceof<T, S> {
@BeforeTemplate
boolean before(S object) {
return Refaster.<T>clazz().isInstance(object);
}
@AfterTemplate
boolean after(S object) {
return Refaster.<T>isInstance(object);
}
}
/** Prefer {@link Class#isInstance(Object)} method references over more verbose alternatives. */
static final class ClassLiteralIsInstancePredicate<T, S> {
@BeforeTemplate
Predicate<S> before() {
return o -> Refaster.<T>isInstance(o);
}
@AfterTemplate
Predicate<S> after() {
return Refaster.<T>clazz()::isInstance;
}
}
/** Prefer {@link Class#isInstance(Object)} method references over more verbose alternatives. */
static final class ClassReferenceIsInstancePredicate<T, S> {
@BeforeTemplate
Predicate<S> before(Class<T> clazz) {
return o -> clazz.isInstance(o);
}
@AfterTemplate
Predicate<S> after(Class<T> clazz) {
return clazz::isInstance;
}
}
}

View File

@@ -17,7 +17,6 @@ import java.util.Optional;
import java.util.Queue;
import java.util.Set;
import java.util.SortedSet;
import java.util.function.Consumer;
import java.util.function.IntFunction;
import java.util.stream.Stream;
import tech.picnic.errorprone.refaster.annotation.OnlineDocumentation;
@@ -35,7 +34,6 @@ final class CollectionRules {
*/
static final class CollectionIsEmpty<T> {
@BeforeTemplate
@SuppressWarnings("java:S1155" /* This violation will be rewritten. */)
boolean before(Collection<T> collection) {
return Refaster.anyOf(
collection.size() == 0,
@@ -74,19 +72,6 @@ final class CollectionRules {
}
}
/** Prefer {@link Collection#contains(Object)} over more contrived alternatives. */
static final class CollectionContains<T, S> {
@BeforeTemplate
boolean before(Collection<T> collection, S value) {
return collection.stream().anyMatch(value::equals);
}
@AfterTemplate
boolean after(Collection<T> collection, S value) {
return collection.contains(value);
}
}
/**
* Don't call {@link Iterables#addAll(Collection, Iterable)} when the elements to be added are
* already part of a {@link Collection}.
@@ -177,8 +162,6 @@ final class CollectionRules {
}
/** Prefer {@link ArrayList#ArrayList(Collection)} over the Guava alternative. */
@SuppressWarnings(
"NonApiType" /* Matching against `List` would unnecessarily constrain the rule. */)
static final class NewArrayListFromCollection<T> {
@BeforeTemplate
ArrayList<T> before(Collection<T> collection) {
@@ -421,19 +404,6 @@ final class CollectionRules {
}
}
/** Prefer {@link Collection#forEach(Consumer)} over more contrived alternatives. */
static final class CollectionForEach<T> {
@BeforeTemplate
void before(Collection<T> collection, Consumer<? super T> consumer) {
collection.stream().forEach(consumer);
}
@AfterTemplate
void after(Collection<T> collection, Consumer<? super T> consumer) {
collection.forEach(consumer);
}
}
// XXX: collection.stream().noneMatch(e -> e.equals(other))
// ^ This is !collection.contains(other). Do we already rewrite variations on this?
}

View File

@@ -12,11 +12,9 @@ import static java.util.function.Function.identity;
import com.google.common.collect.Comparators;
import com.google.common.collect.ImmutableList;
import com.google.common.collect.ImmutableSet;
import com.google.errorprone.annotations.CanIgnoreReturnValue;
import com.google.errorprone.refaster.Refaster;
import com.google.errorprone.refaster.annotation.AfterTemplate;
import com.google.errorprone.refaster.annotation.BeforeTemplate;
import com.google.errorprone.refaster.annotation.Repeated;
import com.google.errorprone.refaster.annotation.UseImportPolicy;
import java.util.Arrays;
import java.util.Collections;
@@ -26,7 +24,6 @@ import java.util.function.Function;
import java.util.function.ToDoubleFunction;
import java.util.function.ToIntFunction;
import java.util.function.ToLongFunction;
import java.util.stream.Stream;
import tech.picnic.errorprone.refaster.annotation.OnlineDocumentation;
/** Refaster rules related to expressions dealing with {@link Comparator}s. */
@@ -40,10 +37,7 @@ final class ComparatorRules {
@BeforeTemplate
Comparator<T> before() {
return Refaster.anyOf(
T::compareTo,
comparing(Refaster.anyOf(identity(), v -> v)),
Collections.<T>reverseOrder(reverseOrder()),
Comparator.<T>reverseOrder().reversed());
comparing(Refaster.anyOf(identity(), v -> v)), Comparator.<T>reverseOrder().reversed());
}
@AfterTemplate
@@ -57,15 +51,11 @@ final class ComparatorRules {
static final class ReverseOrder<T extends Comparable<? super T>> {
@BeforeTemplate
Comparator<T> before() {
return Refaster.anyOf(
Collections.reverseOrder(),
Collections.<T>reverseOrder(naturalOrder()),
Comparator.<T>naturalOrder().reversed());
return Comparator.<T>naturalOrder().reversed();
}
// XXX: Add `@UseImportPolicy(STATIC_IMPORT_ALWAYS)` if/when
// https://github.com/google/error-prone/pull/3584 is merged and released.
@AfterTemplate
@UseImportPolicy(STATIC_IMPORT_ALWAYS)
Comparator<T> after() {
return reverseOrder();
}
@@ -79,7 +69,6 @@ final class ComparatorRules {
}
@AfterTemplate
@CanIgnoreReturnValue
@UseImportPolicy(STATIC_IMPORT_ALWAYS)
Comparator<T> after(Comparator<T> cmp) {
return cmp;
@@ -200,54 +189,15 @@ final class ComparatorRules {
}
}
/** Prefer {@link Comparable#compareTo(Object)}} over more verbose alternatives. */
static final class CompareTo<T extends Comparable<? super T>> {
@BeforeTemplate
int before(T value1, T value2) {
return Refaster.anyOf(
Comparator.<T>naturalOrder().compare(value1, value2),
Comparator.<T>reverseOrder().compare(value2, value1));
}
@AfterTemplate
int after(T value1, T value2) {
return value1.compareTo(value2);
}
}
/**
* Avoid unnecessary creation of a {@link Stream} to determine the minimum of a known collection
* of values.
*/
static final class MinOfVarargs<T> {
@BeforeTemplate
T before(@Repeated T value, Comparator<T> cmp) {
return Stream.of(Refaster.asVarargs(value)).min(cmp).orElseThrow();
}
@AfterTemplate
T after(@Repeated T value, Comparator<T> cmp) {
return Collections.min(Arrays.asList(value), cmp);
}
}
/** Prefer {@link Comparators#min(Comparable, Comparable)}} over more verbose alternatives. */
static final class MinOfPairNaturalOrder<T extends Comparable<? super T>> {
@BeforeTemplate
@SuppressWarnings("java:S1067" /* The conditional operators are independent. */)
T before(T value1, T value2) {
return Refaster.anyOf(
value1.compareTo(value2) <= 0 ? value1 : value2,
value1.compareTo(value2) > 0 ? value2 : value1,
value2.compareTo(value1) < 0 ? value2 : value1,
value2.compareTo(value1) >= 0 ? value1 : value2,
Comparators.min(value1, value2, naturalOrder()),
Comparators.max(value1, value2, reverseOrder()),
Collections.min(
Refaster.anyOf(
Arrays.asList(value1, value2),
ImmutableList.of(value1, value2),
ImmutableSet.of(value1, value2))));
return Collections.min(
Refaster.anyOf(
Arrays.asList(value1, value2),
ImmutableList.of(value1, value2),
ImmutableSet.of(value1, value2)));
}
@AfterTemplate
@@ -261,19 +211,13 @@ final class ComparatorRules {
*/
static final class MinOfPairCustomOrder<T> {
@BeforeTemplate
@SuppressWarnings("java:S1067" /* The conditional operators are independent. */)
T before(T value1, T value2, Comparator<T> cmp) {
return Refaster.anyOf(
cmp.compare(value1, value2) <= 0 ? value1 : value2,
cmp.compare(value1, value2) > 0 ? value2 : value1,
cmp.compare(value2, value1) < 0 ? value2 : value1,
cmp.compare(value2, value1) >= 0 ? value1 : value2,
Collections.min(
Refaster.anyOf(
Arrays.asList(value1, value2),
ImmutableList.of(value1, value2),
ImmutableSet.of(value1, value2)),
cmp));
return Collections.min(
Refaster.anyOf(
Arrays.asList(value1, value2),
ImmutableList.of(value1, value2),
ImmutableSet.of(value1, value2)),
cmp);
}
@AfterTemplate
@@ -282,39 +226,15 @@ final class ComparatorRules {
}
}
/**
* Avoid unnecessary creation of a {@link Stream} to determine the maximum of a known collection
* of values.
*/
static final class MaxOfVarargs<T> {
@BeforeTemplate
T before(@Repeated T value, Comparator<T> cmp) {
return Stream.of(Refaster.asVarargs(value)).max(cmp).orElseThrow();
}
@AfterTemplate
T after(@Repeated T value, Comparator<T> cmp) {
return Collections.max(Arrays.asList(value), cmp);
}
}
/** Prefer {@link Comparators#max(Comparable, Comparable)}} over more verbose alternatives. */
static final class MaxOfPairNaturalOrder<T extends Comparable<? super T>> {
@BeforeTemplate
@SuppressWarnings("java:S1067" /* The conditional operators are independent. */)
T before(T value1, T value2) {
return Refaster.anyOf(
value1.compareTo(value2) >= 0 ? value1 : value2,
value1.compareTo(value2) < 0 ? value2 : value1,
value2.compareTo(value1) > 0 ? value2 : value1,
value2.compareTo(value1) <= 0 ? value1 : value2,
Comparators.max(value1, value2, naturalOrder()),
Comparators.min(value1, value2, reverseOrder()),
Collections.max(
Refaster.anyOf(
Arrays.asList(value1, value2),
ImmutableList.of(value1, value2),
ImmutableSet.of(value1, value2))));
return Collections.max(
Refaster.anyOf(
Arrays.asList(value1, value2),
ImmutableList.of(value1, value2),
ImmutableSet.of(value1, value2)));
}
@AfterTemplate
@@ -328,19 +248,13 @@ final class ComparatorRules {
*/
static final class MaxOfPairCustomOrder<T> {
@BeforeTemplate
@SuppressWarnings("java:S1067" /* The conditional operators are independent. */)
T before(T value1, T value2, Comparator<T> cmp) {
return Refaster.anyOf(
cmp.compare(value1, value2) >= 0 ? value1 : value2,
cmp.compare(value1, value2) < 0 ? value2 : value1,
cmp.compare(value2, value1) > 0 ? value2 : value1,
cmp.compare(value2, value1) <= 0 ? value1 : value2,
Collections.max(
Refaster.anyOf(
Arrays.asList(value1, value2),
ImmutableList.of(value1, value2),
ImmutableSet.of(value1, value2)),
cmp));
return Collections.max(
Refaster.anyOf(
Arrays.asList(value1, value2),
ImmutableList.of(value1, value2),
ImmutableSet.of(value1, value2)),
cmp);
}
@AfterTemplate

View File

@@ -1,7 +1,6 @@
package tech.picnic.errorprone.refasterrules;
import com.google.common.collect.Streams;
import com.google.errorprone.annotations.CanIgnoreReturnValue;
import com.google.errorprone.refaster.Refaster;
import com.google.errorprone.refaster.annotation.AfterTemplate;
import com.google.errorprone.refaster.annotation.BeforeTemplate;
@@ -28,7 +27,6 @@ final class DoubleStreamRules {
}
@AfterTemplate
@CanIgnoreReturnValue
DoubleStream after(DoubleStream stream) {
return stream;
}
@@ -143,22 +141,6 @@ final class DoubleStreamRules {
}
}
/**
* Apply {@link DoubleStream#filter(DoublePredicate)} before {@link DoubleStream#sorted()} to
* reduce the number of elements to sort.
*/
static final class DoubleStreamFilterSorted {
@BeforeTemplate
DoubleStream before(DoubleStream stream, DoublePredicate predicate) {
return stream.sorted().filter(predicate);
}
@AfterTemplate
DoubleStream after(DoubleStream stream, DoublePredicate predicate) {
return stream.filter(predicate).sorted();
}
}
/** In order to test whether a stream has any element, simply try to find one. */
static final class DoubleStreamIsEmpty {
@BeforeTemplate
@@ -239,7 +221,6 @@ final class DoubleStreamRules {
/** Prefer {@link DoubleStream#anyMatch(DoublePredicate)} over more contrived alternatives. */
static final class DoubleStreamAnyMatch {
@BeforeTemplate
@SuppressWarnings("java:S4034" /* This violation will be rewritten. */)
boolean before(DoubleStream stream, DoublePredicate predicate) {
return Refaster.anyOf(
!stream.noneMatch(predicate), stream.filter(predicate).findAny().isPresent());
@@ -277,16 +258,4 @@ final class DoubleStreamRules {
return stream.allMatch(e -> test(e));
}
}
static final class DoubleStreamTakeWhile {
@BeforeTemplate
DoubleStream before(DoubleStream stream, DoublePredicate predicate) {
return stream.takeWhile(predicate).filter(predicate);
}
@AfterTemplate
DoubleStream after(DoubleStream stream, DoublePredicate predicate) {
return stream.takeWhile(predicate);
}
}
}

View File

@@ -1,16 +1,10 @@
package tech.picnic.errorprone.refasterrules;
import static java.util.function.Predicate.not;
import com.google.errorprone.annotations.CanIgnoreReturnValue;
import com.google.errorprone.refaster.Refaster;
import com.google.errorprone.refaster.annotation.AfterTemplate;
import com.google.errorprone.refaster.annotation.AlsoNegation;
import com.google.errorprone.refaster.annotation.BeforeTemplate;
import com.google.errorprone.refaster.annotation.MayOptionallyUse;
import com.google.errorprone.refaster.annotation.Placeholder;
import java.util.Objects;
import java.util.Optional;
import java.util.function.Predicate;
import tech.picnic.errorprone.refaster.annotation.OnlineDocumentation;
@@ -36,7 +30,6 @@ final class EqualityRules {
@AfterTemplate
@AlsoNegation
@SuppressWarnings("java:S1698" /* Reference comparison is valid for enums. */)
boolean after(T a, T b) {
return a == b;
}
@@ -63,13 +56,11 @@ final class EqualityRules {
/** Avoid double negations; this is not Javascript. */
static final class DoubleNegation {
@BeforeTemplate
@SuppressWarnings("java:S2761" /* This violation will be rewritten. */)
boolean before(boolean b) {
return !!b;
}
@AfterTemplate
@CanIgnoreReturnValue
boolean after(boolean b) {
return b;
}
@@ -81,7 +72,6 @@ final class EqualityRules {
*/
// XXX: Replacing `a ? !b : b` with `a != b` changes semantics if both `a` and `b` are boxed
// booleans.
@SuppressWarnings("java:S1940" /* This violation will be rewritten. */)
static final class Negation {
@BeforeTemplate
boolean before(boolean a, boolean b) {
@@ -89,8 +79,6 @@ final class EqualityRules {
}
@BeforeTemplate
@SuppressWarnings(
"java:S1244" /* The equality check is fragile, but may be seen in the wild. */)
boolean before(double a, double b) {
return !(a == b);
}
@@ -112,7 +100,6 @@ final class EqualityRules {
*/
// XXX: Replacing `a ? b : !b` with `a == b` changes semantics if both `a` and `b` are boxed
// booleans.
@SuppressWarnings("java:S1940" /* This violation will be rewritten. */)
static final class IndirectDoubleNegation {
@BeforeTemplate
boolean before(boolean a, boolean b) {
@@ -120,8 +107,6 @@ final class EqualityRules {
}
@BeforeTemplate
@SuppressWarnings(
"java:S1244" /* The inequality check is fragile, but may be seen in the wild. */)
boolean before(double a, double b) {
return !(a != b);
}
@@ -136,62 +121,4 @@ final class EqualityRules {
return a == b;
}
}
/**
* Don't pass a lambda expression to {@link Predicate#not(Predicate)}; instead push the negation
* into the lambda expression.
*/
abstract static class PredicateLambda<T> {
@Placeholder(allowsIdentity = true)
abstract boolean predicate(@MayOptionallyUse T value);
@BeforeTemplate
Predicate<T> before() {
return not(v -> predicate(v));
}
@AfterTemplate
Predicate<T> after() {
return v -> !predicate(v);
}
}
/** Avoid contrived ways of handling {@code null} values during equality testing. */
static final class EqualsLhsNullable<T, S> {
@BeforeTemplate
boolean before(T value1, S value2) {
return Optional.ofNullable(value1).equals(Optional.of(value2));
}
@AfterTemplate
boolean after(T value1, S value2) {
return value2.equals(value1);
}
}
/** Avoid contrived ways of handling {@code null} values during equality testing. */
static final class EqualsRhsNullable<T, S> {
@BeforeTemplate
boolean before(T value1, S value2) {
return Optional.of(value1).equals(Optional.ofNullable(value2));
}
@AfterTemplate
boolean after(T value1, S value2) {
return value1.equals(value2);
}
}
/** Avoid contrived ways of handling {@code null} values during equality testing. */
static final class EqualsLhsAndRhsNullable<T, S> {
@BeforeTemplate
boolean before(T value1, S value2) {
return Optional.ofNullable(value1).equals(Optional.ofNullable(value2));
}
@AfterTemplate
boolean after(T value1, S value2) {
return Objects.equals(value1, value2);
}
}
}

View File

@@ -1,43 +0,0 @@
package tech.picnic.errorprone.refasterrules;
import static java.nio.charset.StandardCharsets.UTF_8;
import com.google.errorprone.refaster.annotation.AfterTemplate;
import com.google.errorprone.refaster.annotation.BeforeTemplate;
import java.io.IOException;
import java.nio.charset.Charset;
import java.nio.file.Files;
import java.nio.file.Path;
import tech.picnic.errorprone.refaster.annotation.OnlineDocumentation;
/** Refaster rules related to expressions dealing with files. */
@OnlineDocumentation
final class FileRules {
private FileRules() {}
/** Prefer {@link Files#readString(Path, Charset)} over more contrived alternatives. */
static final class FilesReadStringWithCharset {
@BeforeTemplate
String before(Path path, Charset charset) throws IOException {
return new String(Files.readAllBytes(path), charset);
}
@AfterTemplate
String after(Path path, Charset charset) throws IOException {
return Files.readString(path, charset);
}
}
/** Prefer {@link Files#readString(Path)} over more verbose alternatives. */
static final class FilesReadString {
@BeforeTemplate
String before(Path path) throws IOException {
return Files.readString(path, UTF_8);
}
@AfterTemplate
String after(Path path) throws IOException {
return Files.readString(path);
}
}
}

View File

@@ -6,7 +6,6 @@ import static java.util.Collections.emptyMap;
import static java.util.Collections.singletonMap;
import static java.util.function.Function.identity;
import com.google.common.base.Predicate;
import com.google.common.collect.ImmutableMap;
import com.google.common.collect.Maps;
import com.google.common.collect.Streams;
@@ -127,8 +126,8 @@ final class ImmutableMapRules {
}
/**
* Don't map a stream's elements to map entries, only to subsequently collect them into an {@link
* ImmutableMap}. The collection can be performed directly.
* Don't map a a stream's elements to map entries, only to subsequently collect them into an
* {@link ImmutableMap}. The collection can be performed directly.
*/
abstract static class StreamOfMapEntriesToImmutableMap<E, K, V> {
@Placeholder(allowsIdentity = true)
@@ -250,8 +249,7 @@ final class ImmutableMapRules {
* Prefer {@link ImmutableMap#of(Object, Object, Object, Object)} over alternatives that don't
* communicate the immutability of the resulting map at the type level.
*/
// XXX: Consider introducing a `BugChecker` to replace these `ImmutableMapOfX` rules. That will
// also make it easier to rewrite various `ImmutableMap.builder()` variants.
// XXX: Also rewrite the `ImmutableMap.builder()` variant?
static final class ImmutableMapOf2<K, V> {
@BeforeTemplate
Map<K, V> before(K k1, V v1, K k2, V v2) {
@@ -268,8 +266,7 @@ final class ImmutableMapRules {
* Prefer {@link ImmutableMap#of(Object, Object, Object, Object, Object, Object)} over
* alternatives that don't communicate the immutability of the resulting map at the type level.
*/
// XXX: Consider introducing a `BugChecker` to replace these `ImmutableMapOfX` rules. That will
// also make it easier to rewrite various `ImmutableMap.builder()` variants.
// XXX: Also rewrite the `ImmutableMap.builder()` variant?
static final class ImmutableMapOf3<K, V> {
@BeforeTemplate
Map<K, V> before(K k1, V v1, K k2, V v2, K k3, V v3) {
@@ -287,9 +284,7 @@ final class ImmutableMapRules {
* over alternatives that don't communicate the immutability of the resulting map at the type
* level.
*/
// XXX: Consider introducing a `BugChecker` to replace these `ImmutableMapOfX` rules. That will
// also make it easier to rewrite various `ImmutableMap.builder()` variants.
@SuppressWarnings("java:S107" /* Can't avoid many method parameters here. */)
// XXX: Also rewrite the `ImmutableMap.builder()` variant?
static final class ImmutableMapOf4<K, V> {
@BeforeTemplate
Map<K, V> before(K k1, V v1, K k2, V v2, K k3, V v3, K k4, V v4) {
@@ -307,9 +302,7 @@ final class ImmutableMapRules {
* Object, Object)} over alternatives that don't communicate the immutability of the resulting map
* at the type level.
*/
// XXX: Consider introducing a `BugChecker` to replace these `ImmutableMapOfX` rules. That will
// also make it easier to rewrite various `ImmutableMap.builder()` variants.
@SuppressWarnings("java:S107" /* Can't avoid many method parameters here. */)
// XXX: Also rewrite the `ImmutableMap.builder()` variant?
static final class ImmutableMapOf5<K, V> {
@BeforeTemplate
Map<K, V> before(K k1, V v1, K k2, V v2, K k3, V v3, K k4, V v4, K k5, V v5) {
@@ -322,48 +315,6 @@ final class ImmutableMapRules {
}
}
/**
* Prefer creation of an immutable submap using {@link Maps#filterKeys(Map, Predicate)} over more
* contrived alternatives.
*/
abstract static class ImmutableMapCopyOfMapsFilterKeys<K, V> {
@Placeholder(allowsIdentity = true)
abstract boolean keyFilter(@MayOptionallyUse K key);
@BeforeTemplate
ImmutableMap<K, V> before(ImmutableMap<K, V> map) {
return map.entrySet().stream()
.filter(e -> keyFilter(e.getKey()))
.collect(toImmutableMap(Map.Entry::getKey, Map.Entry::getValue));
}
@AfterTemplate
ImmutableMap<K, V> after(ImmutableMap<K, V> map) {
return ImmutableMap.copyOf(Maps.filterKeys(map, k -> keyFilter(k)));
}
}
/**
* Prefer creation of an immutable submap using {@link Maps#filterValues(Map, Predicate)} over
* more contrived alternatives.
*/
abstract static class ImmutableMapCopyOfMapsFilterValues<K, V> {
@Placeholder(allowsIdentity = true)
abstract boolean valueFilter(@MayOptionallyUse V value);
@BeforeTemplate
ImmutableMap<K, V> before(ImmutableMap<K, V> map) {
return map.entrySet().stream()
.filter(e -> valueFilter(e.getValue()))
.collect(toImmutableMap(Map.Entry::getKey, Map.Entry::getValue));
}
@AfterTemplate
ImmutableMap<K, V> after(ImmutableMap<K, V> map) {
return ImmutableMap.copyOf(Maps.filterValues(map, v -> valueFilter(v)));
}
}
// XXX: Add a rule for this:
// Maps.transformValues(streamOfEntries.collect(groupBy(fun)), ImmutableMap::copyOf)
// ->
@@ -372,4 +323,9 @@ final class ImmutableMapRules {
// map.entrySet().stream().filter(keyPred).forEach(mapBuilder::put)
// ->
// mapBuilder.putAll(Maps.filterKeys(map, pred))
//
// map.entrySet().stream().filter(entry ->
// pred(entry.getKey())).collect(toImmutableMap(Map.Entry::getKey, Map.Entry::getValue))
// ->
// ImmutableMap.copyOf(Maps.filterKeys(map, pred))
}

View File

@@ -4,11 +4,8 @@ import static com.google.common.collect.ImmutableSet.toImmutableSet;
import static com.google.errorprone.refaster.ImportPolicy.STATIC_IMPORT_ALWAYS;
import static java.util.Collections.emptySet;
import static java.util.Collections.singleton;
import static java.util.function.Predicate.not;
import com.google.common.collect.ImmutableSet;
import com.google.common.collect.Multimap;
import com.google.common.collect.Sets;
import com.google.common.collect.Sets.SetView;
import com.google.common.collect.Streams;
import com.google.errorprone.refaster.Refaster;
@@ -18,7 +15,6 @@ import com.google.errorprone.refaster.annotation.UseImportPolicy;
import java.util.Arrays;
import java.util.Collection;
import java.util.Iterator;
import java.util.Map;
import java.util.Set;
import java.util.stream.Stream;
import tech.picnic.errorprone.refaster.annotation.OnlineDocumentation;
@@ -215,116 +211,4 @@ final class ImmutableSetRules {
return ImmutableSet.of(e1, e2, e3, e4, e5);
}
}
/**
* Prefer an immutable copy of {@link Sets#difference(Set, Set)} over more contrived alternatives.
*/
static final class SetsDifference<S, T> {
@BeforeTemplate
ImmutableSet<S> before(Set<S> set1, Set<T> set2) {
return set1.stream()
.filter(Refaster.anyOf(not(set2::contains), e -> !set2.contains(e)))
.collect(toImmutableSet());
}
@AfterTemplate
ImmutableSet<S> after(Set<S> set1, Set<T> set2) {
return Sets.difference(set1, set2).immutableCopy();
}
}
/**
* Prefer an immutable copy of {@link Sets#difference(Set, Set)} over more contrived alternatives.
*/
static final class SetsDifferenceMap<T, K, V> {
@BeforeTemplate
ImmutableSet<T> before(Set<T> set, Map<K, V> map) {
return set.stream()
.filter(Refaster.anyOf(not(map::containsKey), e -> !map.containsKey(e)))
.collect(toImmutableSet());
}
@AfterTemplate
ImmutableSet<K> after(Set<K> set, Map<K, V> map) {
return Sets.difference(set, map.keySet()).immutableCopy();
}
}
/**
* Prefer an immutable copy of {@link Sets#difference(Set, Set)} over more contrived alternatives.
*/
static final class SetsDifferenceMultimap<T, K, V> {
@BeforeTemplate
ImmutableSet<T> before(Set<T> set, Multimap<K, V> multimap) {
return set.stream()
.filter(Refaster.anyOf(not(multimap::containsKey), e -> !multimap.containsKey(e)))
.collect(toImmutableSet());
}
@AfterTemplate
ImmutableSet<T> after(Set<T> set, Multimap<K, V> multimap) {
return Sets.difference(set, multimap.keySet()).immutableCopy();
}
}
/**
* Prefer an immutable copy of {@link Sets#intersection(Set, Set)} over more contrived
* alternatives.
*/
static final class SetsIntersection<S, T> {
@BeforeTemplate
ImmutableSet<S> before(Set<S> set1, Set<T> set2) {
return set1.stream().filter(set2::contains).collect(toImmutableSet());
}
@AfterTemplate
ImmutableSet<S> after(Set<S> set1, Set<T> set2) {
return Sets.intersection(set1, set2).immutableCopy();
}
}
/**
* Prefer an immutable copy of {@link Sets#intersection(Set, Set)} over more contrived
* alternatives.
*/
static final class SetsIntersectionMap<T, K, V> {
@BeforeTemplate
ImmutableSet<T> before(Set<T> set, Map<K, V> map) {
return set.stream().filter(map::containsKey).collect(toImmutableSet());
}
@AfterTemplate
ImmutableSet<T> after(Set<T> set, Map<K, V> map) {
return Sets.intersection(set, map.keySet()).immutableCopy();
}
}
/**
* Prefer an immutable copy of {@link Sets#intersection(Set, Set)} over more contrived
* alternatives.
*/
static final class SetsIntersectionMultimap<T, K, V> {
@BeforeTemplate
ImmutableSet<T> before(Set<T> set, Multimap<K, V> multimap) {
return set.stream().filter(multimap::containsKey).collect(toImmutableSet());
}
@AfterTemplate
ImmutableSet<T> after(Set<T> set, Multimap<K, V> multimap) {
return Sets.intersection(set, multimap.keySet()).immutableCopy();
}
}
/** Prefer an immutable copy of {@link Sets#union(Set, Set)} over more contrived alternatives. */
static final class SetsUnion<S, T extends S, U extends S> {
@BeforeTemplate
ImmutableSet<S> before(Set<T> set1, Set<U> set2) {
return Stream.concat(set1.stream(), set2.stream()).collect(toImmutableSet());
}
@AfterTemplate
ImmutableSet<S> after(Set<T> set1, Set<U> set2) {
return Sets.union(set1, set2).immutableCopy();
}
}
}

View File

@@ -1,7 +1,6 @@
package tech.picnic.errorprone.refasterrules;
import com.google.common.collect.Streams;
import com.google.errorprone.annotations.CanIgnoreReturnValue;
import com.google.errorprone.refaster.Refaster;
import com.google.errorprone.refaster.annotation.AfterTemplate;
import com.google.errorprone.refaster.annotation.BeforeTemplate;
@@ -41,7 +40,6 @@ final class IntStreamRules {
}
@AfterTemplate
@CanIgnoreReturnValue
IntStream after(IntStream stream) {
return stream;
}
@@ -156,22 +154,6 @@ final class IntStreamRules {
}
}
/**
* Apply {@link IntStream#filter(IntPredicate)} before {@link IntStream#sorted()} to reduce the
* number of elements to sort.
*/
static final class IntStreamFilterSorted {
@BeforeTemplate
IntStream before(IntStream stream, IntPredicate predicate) {
return stream.sorted().filter(predicate);
}
@AfterTemplate
IntStream after(IntStream stream, IntPredicate predicate) {
return stream.filter(predicate).sorted();
}
}
/** In order to test whether a stream has any element, simply try to find one. */
static final class IntStreamIsEmpty {
@BeforeTemplate
@@ -252,7 +234,6 @@ final class IntStreamRules {
/** Prefer {@link IntStream#anyMatch(IntPredicate)} over more contrived alternatives. */
static final class IntStreamAnyMatch {
@BeforeTemplate
@SuppressWarnings("java:S4034" /* This violation will be rewritten. */)
boolean before(IntStream stream, IntPredicate predicate) {
return Refaster.anyOf(
!stream.noneMatch(predicate), stream.filter(predicate).findAny().isPresent());
@@ -290,16 +271,4 @@ final class IntStreamRules {
return stream.allMatch(e -> test(e));
}
}
static final class IntStreamTakeWhile {
@BeforeTemplate
IntStream before(IntStream stream, IntPredicate predicate) {
return stream.takeWhile(predicate).filter(predicate);
}
@AfterTemplate
IntStream after(IntStream stream, IntPredicate predicate) {
return stream.takeWhile(predicate);
}
}
}

View File

@@ -1,520 +0,0 @@
package tech.picnic.errorprone.refasterrules;
import static com.google.errorprone.refaster.ImportPolicy.STATIC_IMPORT_ALWAYS;
import static org.assertj.core.api.Assertions.assertThat;
import static org.assertj.core.api.Assertions.assertThatCode;
import static org.assertj.core.api.Assertions.assertThatThrownBy;
import static org.assertj.core.api.Assertions.fail;
import static org.junit.jupiter.api.Assertions.assertDoesNotThrow;
import static org.junit.jupiter.api.Assertions.assertFalse;
import static org.junit.jupiter.api.Assertions.assertInstanceOf;
import static org.junit.jupiter.api.Assertions.assertNotNull;
import static org.junit.jupiter.api.Assertions.assertNotSame;
import static org.junit.jupiter.api.Assertions.assertNull;
import static org.junit.jupiter.api.Assertions.assertSame;
import static org.junit.jupiter.api.Assertions.assertThrows;
import static org.junit.jupiter.api.Assertions.assertThrowsExactly;
import static org.junit.jupiter.api.Assertions.assertTrue;
import com.google.common.collect.ImmutableSet;
import com.google.errorprone.annotations.DoNotCall;
import com.google.errorprone.refaster.annotation.AfterTemplate;
import com.google.errorprone.refaster.annotation.BeforeTemplate;
import com.google.errorprone.refaster.annotation.UseImportPolicy;
import java.util.function.Supplier;
import org.assertj.core.api.ThrowableAssert.ThrowingCallable;
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;
/**
* Refaster rules to replace JUnit assertions with AssertJ equivalents.
*
* <p>Note that, while both libraries throw an {@link AssertionError} in case of an assertion
* failure, the exact subtype used generally differs.
*/
// XXX: Not all JUnit `Assertions` methods have an associated Refaster rule yet; expand this class.
// XXX: Introduce a `@Matcher` on `Executable` and `ThrowingSupplier` expressions, such that they
// are only matched if they are also compatible with the `ThrowingCallable` functional interface.
// When implementing such a matcher, note that expressions with a non-void return type such as
// `() -> toString()` match both `ThrowingSupplier` and `ThrowingCallable`, but `() -> "constant"`
// is only compatible with the former.
@OnlineDocumentation
final class JUnitToAssertJRules {
private JUnitToAssertJRules() {}
public ImmutableSet<Object> elidedTypesAndStaticImports() {
return ImmutableSet.of(
Assertions.class,
assertDoesNotThrow(() -> null),
assertInstanceOf(null, null),
assertThrows(null, null),
assertThrowsExactly(null, null),
(Runnable) () -> assertFalse(true),
(Runnable) () -> assertNotNull(null),
(Runnable) () -> assertNotSame(null, null),
(Runnable) () -> assertNull(null),
(Runnable) () -> assertSame(null, null),
(Runnable) () -> assertTrue(true));
}
static final class ThrowNewAssertionError {
@BeforeTemplate
void before() {
Assertions.fail();
}
@AfterTemplate
@DoNotCall
void after() {
throw new AssertionError();
}
}
static final class FailWithMessage<T> {
@BeforeTemplate
T before(String message) {
return Assertions.fail(message);
}
@AfterTemplate
@UseImportPolicy(STATIC_IMPORT_ALWAYS)
T after(String message) {
return fail(message);
}
}
static final class FailWithMessageAndThrowable<T> {
@BeforeTemplate
T before(String message, Throwable throwable) {
return Assertions.fail(message, throwable);
}
@AfterTemplate
@UseImportPolicy(STATIC_IMPORT_ALWAYS)
T after(String message, Throwable throwable) {
return fail(message, throwable);
}
}
static final class FailWithThrowable {
@BeforeTemplate
void before(Throwable throwable) {
Assertions.fail(throwable);
}
@AfterTemplate
@DoNotCall
void after(Throwable throwable) {
throw new AssertionError(throwable);
}
}
static final class AssertThatIsTrue {
@BeforeTemplate
void before(boolean actual) {
assertTrue(actual);
}
@AfterTemplate
@UseImportPolicy(STATIC_IMPORT_ALWAYS)
void after(boolean actual) {
assertThat(actual).isTrue();
}
}
static final class AssertThatWithFailMessageStringIsTrue {
@BeforeTemplate
void before(boolean actual, String message) {
assertTrue(actual, message);
}
@AfterTemplate
@UseImportPolicy(STATIC_IMPORT_ALWAYS)
void after(boolean actual, String message) {
assertThat(actual).withFailMessage(message).isTrue();
}
}
static final class AssertThatWithFailMessageSupplierIsTrue {
@BeforeTemplate
void before(boolean actual, Supplier<String> supplier) {
assertTrue(actual, supplier);
}
@AfterTemplate
@UseImportPolicy(STATIC_IMPORT_ALWAYS)
void after(boolean actual, Supplier<String> supplier) {
assertThat(actual).withFailMessage(supplier).isTrue();
}
}
static final class AssertThatIsFalse {
@BeforeTemplate
void before(boolean actual) {
assertFalse(actual);
}
@AfterTemplate
@UseImportPolicy(STATIC_IMPORT_ALWAYS)
void after(boolean actual) {
assertThat(actual).isFalse();
}
}
static final class AssertThatWithFailMessageStringIsFalse {
@BeforeTemplate
void before(boolean actual, String message) {
assertFalse(actual, message);
}
@AfterTemplate
@UseImportPolicy(STATIC_IMPORT_ALWAYS)
void after(boolean actual, String message) {
assertThat(actual).withFailMessage(message).isFalse();
}
}
static final class AssertThatWithFailMessageSupplierIsFalse {
@BeforeTemplate
void before(boolean actual, Supplier<String> supplier) {
assertFalse(actual, supplier);
}
@AfterTemplate
@UseImportPolicy(STATIC_IMPORT_ALWAYS)
void after(boolean actual, Supplier<String> supplier) {
assertThat(actual).withFailMessage(supplier).isFalse();
}
}
static final class AssertThatIsNull {
@BeforeTemplate
void before(Object actual) {
assertNull(actual);
}
@AfterTemplate
@UseImportPolicy(STATIC_IMPORT_ALWAYS)
void after(Object actual) {
assertThat(actual).isNull();
}
}
static final class AssertThatWithFailMessageStringIsNull {
@BeforeTemplate
void before(Object actual, String message) {
assertNull(actual, message);
}
@AfterTemplate
@UseImportPolicy(STATIC_IMPORT_ALWAYS)
void after(Object actual, String message) {
assertThat(actual).withFailMessage(message).isNull();
}
}
static final class AssertThatWithFailMessageSupplierIsNull {
@BeforeTemplate
void before(Object actual, Supplier<String> supplier) {
assertNull(actual, supplier);
}
@AfterTemplate
@UseImportPolicy(STATIC_IMPORT_ALWAYS)
void after(Object actual, Supplier<String> supplier) {
assertThat(actual).withFailMessage(supplier).isNull();
}
}
static final class AssertThatIsNotNull {
@BeforeTemplate
void before(Object actual) {
assertNotNull(actual);
}
@AfterTemplate
@UseImportPolicy(STATIC_IMPORT_ALWAYS)
void after(Object actual) {
assertThat(actual).isNotNull();
}
}
static final class AssertThatWithFailMessageStringIsNotNull {
@BeforeTemplate
void before(Object actual, String message) {
assertNotNull(actual, message);
}
@AfterTemplate
@UseImportPolicy(STATIC_IMPORT_ALWAYS)
void after(Object actual, String message) {
assertThat(actual).withFailMessage(message).isNotNull();
}
}
static final class AssertThatWithFailMessageSupplierIsNotNull {
@BeforeTemplate
void before(Object actual, Supplier<String> supplier) {
assertNotNull(actual, supplier);
}
@AfterTemplate
@UseImportPolicy(STATIC_IMPORT_ALWAYS)
void after(Object actual, Supplier<String> supplier) {
assertThat(actual).withFailMessage(supplier).isNotNull();
}
}
static final class AssertThatIsSameAs {
@BeforeTemplate
void before(Object actual, Object expected) {
assertSame(expected, actual);
}
@AfterTemplate
@UseImportPolicy(STATIC_IMPORT_ALWAYS)
void after(Object actual, Object expected) {
assertThat(actual).isSameAs(expected);
}
}
static final class AssertThatWithFailMessageStringIsSameAs {
@BeforeTemplate
void before(Object actual, Object expected, String message) {
assertSame(expected, actual, message);
}
@AfterTemplate
@UseImportPolicy(STATIC_IMPORT_ALWAYS)
void after(Object actual, Object expected, String message) {
assertThat(actual).withFailMessage(message).isSameAs(expected);
}
}
static final class AssertThatWithFailMessageSupplierIsSameAs {
@BeforeTemplate
void before(Object actual, Object expected, Supplier<String> supplier) {
assertSame(expected, actual, supplier);
}
@AfterTemplate
@UseImportPolicy(STATIC_IMPORT_ALWAYS)
void after(Object actual, Object expected, Supplier<String> supplier) {
assertThat(actual).withFailMessage(supplier).isSameAs(expected);
}
}
static final class AssertThatIsNotSameAs {
@BeforeTemplate
void before(Object actual, Object expected) {
assertNotSame(expected, actual);
}
@AfterTemplate
@UseImportPolicy(STATIC_IMPORT_ALWAYS)
void after(Object actual, Object expected) {
assertThat(actual).isNotSameAs(expected);
}
}
static final class AssertThatWithFailMessageStringIsNotSameAs {
@BeforeTemplate
void before(Object actual, Object expected, String message) {
assertNotSame(expected, actual, message);
}
@AfterTemplate
@UseImportPolicy(STATIC_IMPORT_ALWAYS)
void after(Object actual, Object expected, String message) {
assertThat(actual).withFailMessage(message).isNotSameAs(expected);
}
}
static final class AssertThatWithFailMessageSupplierIsNotSameAs {
@BeforeTemplate
void before(Object actual, Object expected, Supplier<String> supplier) {
assertNotSame(expected, actual, supplier);
}
@AfterTemplate
@UseImportPolicy(STATIC_IMPORT_ALWAYS)
void after(Object actual, Object expected, Supplier<String> supplier) {
assertThat(actual).withFailMessage(supplier).isNotSameAs(expected);
}
}
static final class AssertThatThrownByIsExactlyInstanceOf<T extends Throwable> {
@BeforeTemplate
void before(Executable throwingCallable, Class<T> clazz) {
assertThrowsExactly(clazz, throwingCallable);
}
@AfterTemplate
@UseImportPolicy(STATIC_IMPORT_ALWAYS)
void after(ThrowingCallable throwingCallable, Class<T> clazz) {
assertThatThrownBy(throwingCallable).isExactlyInstanceOf(clazz);
}
}
static final class AssertThatThrownByWithFailMessageStringIsExactlyInstanceOf<
T extends Throwable> {
@BeforeTemplate
void before(Executable throwingCallable, Class<T> clazz, String message) {
assertThrowsExactly(clazz, throwingCallable, message);
}
@AfterTemplate
@UseImportPolicy(STATIC_IMPORT_ALWAYS)
void after(ThrowingCallable throwingCallable, Class<T> clazz, String message) {
assertThatThrownBy(throwingCallable).withFailMessage(message).isExactlyInstanceOf(clazz);
}
}
static final class AssertThatThrownByWithFailMessageSupplierIsExactlyInstanceOf<
T extends Throwable> {
@BeforeTemplate
void before(Executable throwingCallable, Class<T> clazz, Supplier<String> supplier) {
assertThrowsExactly(clazz, throwingCallable, supplier);
}
@AfterTemplate
@UseImportPolicy(STATIC_IMPORT_ALWAYS)
void after(ThrowingCallable throwingCallable, Class<T> clazz, Supplier<String> supplier) {
assertThatThrownBy(throwingCallable).withFailMessage(supplier).isExactlyInstanceOf(clazz);
}
}
static final class AssertThatThrownByIsInstanceOf<T extends Throwable> {
@BeforeTemplate
void before(Executable throwingCallable, Class<T> clazz) {
assertThrows(clazz, throwingCallable);
}
@AfterTemplate
@UseImportPolicy(STATIC_IMPORT_ALWAYS)
void after(ThrowingCallable throwingCallable, Class<T> clazz) {
assertThatThrownBy(throwingCallable).isInstanceOf(clazz);
}
}
static final class AssertThatThrownByWithFailMessageStringIsInstanceOf<T extends Throwable> {
@BeforeTemplate
void before(Executable throwingCallable, Class<T> clazz, String message) {
assertThrows(clazz, throwingCallable, message);
}
@AfterTemplate
@UseImportPolicy(STATIC_IMPORT_ALWAYS)
void after(ThrowingCallable throwingCallable, Class<T> clazz, String message) {
assertThatThrownBy(throwingCallable).withFailMessage(message).isInstanceOf(clazz);
}
}
static final class AssertThatThrownByWithFailMessageSupplierIsInstanceOf<T extends Throwable> {
@BeforeTemplate
void before(Executable throwingCallable, Class<T> clazz, Supplier<String> supplier) {
assertThrows(clazz, throwingCallable, supplier);
}
@AfterTemplate
@UseImportPolicy(STATIC_IMPORT_ALWAYS)
void after(ThrowingCallable throwingCallable, Class<T> clazz, Supplier<String> supplier) {
assertThatThrownBy(throwingCallable).withFailMessage(supplier).isInstanceOf(clazz);
}
}
static final class AssertThatCodeDoesNotThrowAnyException {
@BeforeTemplate
void before(Executable throwingCallable) {
assertDoesNotThrow(throwingCallable);
}
@BeforeTemplate
void before(ThrowingSupplier<?> throwingCallable) {
assertDoesNotThrow(throwingCallable);
}
@AfterTemplate
@UseImportPolicy(STATIC_IMPORT_ALWAYS)
void after(ThrowingCallable throwingCallable) {
assertThatCode(throwingCallable).doesNotThrowAnyException();
}
}
static final class AssertThatCodeWithFailMessageStringDoesNotThrowAnyException {
@BeforeTemplate
void before(Executable throwingCallable, String message) {
assertDoesNotThrow(throwingCallable, message);
}
@BeforeTemplate
void before(ThrowingSupplier<?> throwingCallable, String message) {
assertDoesNotThrow(throwingCallable, message);
}
@AfterTemplate
@UseImportPolicy(STATIC_IMPORT_ALWAYS)
void after(ThrowingCallable throwingCallable, String message) {
assertThatCode(throwingCallable).withFailMessage(message).doesNotThrowAnyException();
}
}
static final class AssertThatCodeWithFailMessageSupplierDoesNotThrowAnyException {
@BeforeTemplate
void before(Executable throwingCallable, Supplier<String> supplier) {
assertDoesNotThrow(throwingCallable, supplier);
}
@BeforeTemplate
void before(ThrowingSupplier<?> throwingCallable, Supplier<String> supplier) {
assertDoesNotThrow(throwingCallable, supplier);
}
@AfterTemplate
@UseImportPolicy(STATIC_IMPORT_ALWAYS)
void after(ThrowingCallable throwingCallable, Supplier<String> supplier) {
assertThatCode(throwingCallable).withFailMessage(supplier).doesNotThrowAnyException();
}
}
static final class AssertThatIsInstanceOf<T> {
@BeforeTemplate
void before(Object actual, Class<T> clazz) {
assertInstanceOf(clazz, actual);
}
@AfterTemplate
@UseImportPolicy(STATIC_IMPORT_ALWAYS)
void after(Object actual, Class<T> clazz) {
assertThat(actual).isInstanceOf(clazz);
}
}
static final class AssertThatWithFailMessageStringIsInstanceOf<T> {
@BeforeTemplate
void before(Object actual, Class<T> clazz, String message) {
assertInstanceOf(clazz, actual, message);
}
@AfterTemplate
@UseImportPolicy(STATIC_IMPORT_ALWAYS)
void after(Object actual, Class<T> clazz, String message) {
assertThat(actual).withFailMessage(message).isInstanceOf(clazz);
}
}
static final class AssertThatWithFailMessageSupplierIsInstanceOf<T> {
@BeforeTemplate
void before(Object actual, Class<T> clazz, Supplier<String> supplier) {
assertInstanceOf(clazz, actual, supplier);
}
@AfterTemplate
@UseImportPolicy(STATIC_IMPORT_ALWAYS)
void after(Object actual, Class<T> clazz, Supplier<String> supplier) {
assertThat(actual).withFailMessage(supplier).isInstanceOf(clazz);
}
}
}

View File

@@ -1,7 +1,6 @@
package tech.picnic.errorprone.refasterrules;
import com.google.common.collect.Streams;
import com.google.errorprone.annotations.CanIgnoreReturnValue;
import com.google.errorprone.refaster.Refaster;
import com.google.errorprone.refaster.annotation.AfterTemplate;
import com.google.errorprone.refaster.annotation.BeforeTemplate;
@@ -41,7 +40,6 @@ final class LongStreamRules {
}
@AfterTemplate
@CanIgnoreReturnValue
LongStream after(LongStream stream) {
return stream;
}
@@ -156,22 +154,6 @@ final class LongStreamRules {
}
}
/**
* Apply {@link LongStream#filter(LongPredicate)} before {@link LongStream#sorted()} to reduce the
* number of elements to sort.
*/
static final class LongStreamFilterSorted {
@BeforeTemplate
LongStream before(LongStream stream, LongPredicate predicate) {
return stream.sorted().filter(predicate);
}
@AfterTemplate
LongStream after(LongStream stream, LongPredicate predicate) {
return stream.filter(predicate).sorted();
}
}
/** In order to test whether a stream has any element, simply try to find one. */
static final class LongStreamIsEmpty {
@BeforeTemplate
@@ -252,7 +234,6 @@ final class LongStreamRules {
/** Prefer {@link LongStream#anyMatch(LongPredicate)} over more contrived alternatives. */
static final class LongStreamAnyMatch {
@BeforeTemplate
@SuppressWarnings("java:S4034" /* This violation will be rewritten. */)
boolean before(LongStream stream, LongPredicate predicate) {
return Refaster.anyOf(
!stream.noneMatch(predicate), stream.filter(predicate).findAny().isPresent());
@@ -290,16 +271,4 @@ final class LongStreamRules {
return stream.allMatch(e -> test(e));
}
}
static final class LongStreamTakeWhile {
@BeforeTemplate
LongStream before(LongStream stream, LongPredicate predicate) {
return stream.takeWhile(predicate).filter(predicate);
}
@AfterTemplate
LongStream after(LongStream stream, LongPredicate predicate) {
return stream.takeWhile(predicate);
}
}
}

Some files were not shown because too many files have changed in this diff Show More