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
346 changed files with 3047 additions and 81773 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, 21.0.0 ]
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@b4ffde65f46336ab88eb53be808477a3936bae11 # v4.1.1
with:
persist-credentials: false
uses: actions/checkout@v3.1.0
- name: Set up JDK
uses: actions/setup-java@387ac29b308b003ca37ba93a6cab5eb57c8f5f93 # v4.0.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@b4ffde65f46336ab88eb53be808477a3936bae11 # v4.1.1
with:
persist-credentials: false
- name: Set up JDK
uses: actions/setup-java@387ac29b308b003ca37ba93a6cab5eb57c8f5f93 # v4.0.0
with:
java-version: 17.0.8
distribution: temurin
cache: maven
- name: Initialize CodeQL
uses: github/codeql-action/init@407ffafae6a767df3e0230c3df91b6443ae8df75 # v2.22.8
with:
languages: ${{ matrix.language }}
- name: Perform minimal build
if: matrix.language == 'java'
run: mvn -T1C clean package -DskipTests -Dverification.skip
- name: Perform CodeQL analysis
uses: github/codeql-action/analyze@407ffafae6a767df3e0230c3df91b6443ae8df75 # v2.22.8
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@b4ffde65f46336ab88eb53be808477a3936bae11 # v4.1.1
with:
persist-credentials: false
- uses: ruby/setup-ruby@8575951200e472d5f2d95c625da0c7bec8217c42 # v1.161.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@1f0c5cde4bc74cd7e1254d0cb4de8d49e9068c7d # v4.0.0
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@0252fc4ba7626f0298f0cf00902a25c6afc77fa8 # v3.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@7a9bd943aa5e5175aeb8502edcc6c1c02d398e10 # v4.0.2
uses: actions/deploy-pages@v1.2.2

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@b4ffde65f46336ab88eb53be808477a3936bae11 # v4.1.1
with:
persist-credentials: false
- name: Run OpenSSF Scorecard analysis
uses: ossf/scorecard-action@0864cf19026789058feabb7e87baa5f140aac736 # v2.3.1
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@407ffafae6a767df3e0230c3df91b6443ae8df75 # v2.22.8
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@b4ffde65f46336ab88eb53be808477a3936bae11 # v4.1.1
with:
fetch-depth: 2
persist-credentials: false
- name: Set up JDK
uses: actions/setup-java@387ac29b308b003ca37ba93a6cab5eb57c8f5f93 # v4.0.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@c7d193f32edcb7bfad88892161225aeda64e9392 # v4.0.0
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@b4ffde65f46336ab88eb53be808477a3936bae11 # v4.1.1
with:
persist-credentials: false
- name: Set up JDK
uses: actions/setup-java@387ac29b308b003ca37ba93a6cab5eb57c8f5f93 # v4.0.0
with:
java-version: 17.0.8
distribution: temurin
cache: maven
- name: Download Pitest analysis artifact
uses: dawidd6/action-download-artifact@e7466d1a7587ed14867642c2ca74b5bcc1e19a2d # v3.0.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,43 +0,0 @@
# If requested by means of a pull request comment, runs integration tests
# against the project, using the code found on the pull request branch.
# XXX: Generalize this to a matrix build of multiple integration tests,
# possibly using multiple JDK or OS versions.
# XXX: Investigate whether the comment can specify which integration tests run
# run. See this example of a dynamic build matrix:
# https://docs.github.com/en/actions/learn-github-actions/expressions#example-returning-a-json-object
name: "Integration tests"
on:
issue_comment:
types: [ created ]
permissions:
contents: read
jobs:
run-integration-tests:
name: On-demand integration test
if: |
github.event.issue.pull_request && contains(github.event.comment.body, '/integration-test')
runs-on: ubuntu-22.04
steps:
- name: Check out code
uses: actions/checkout@b4ffde65f46336ab88eb53be808477a3936bae11 # v4.1.1
with:
persist-credentials: false
ref: refs/pull/${{ github.event.issue.number }}/head
- name: Set up JDK
uses: actions/setup-java@387ac29b308b003ca37ba93a6cab5eb57c8f5f93 # v4.0.0
with:
java-version: 17.0.8
distribution: temurin
cache: maven
- name: Install project to local Maven repository
run: mvn -T1C install -DskipTests -Dverification.skip
- name: Run integration test
run: xvfb-run ./integration-tests/checkstyle-10.12.4.sh "${{ runner.temp }}/artifacts"
- name: Upload artifacts on failure
if: ${{ failure() }}
uses: actions/upload-artifact@c7d193f32edcb7bfad88892161225aeda64e9392 # v4.0.0
with:
name: integration-test-checkstyle-10.12.4
path: "${{ runner.temp }}/artifacts"
- name: Remove installed project artifacts
run: mvn build-helper:remove-project-artifact

View File

@@ -1,39 +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:
# Analysis of code in forked repositories is skipped, as such workflow runs
# do not have access to the requisite secrets.
if: github.event.pull_request.head.repo.full_name == github.repository
permissions:
contents: read
runs-on: ubuntu-22.04
steps:
- name: Check out code
uses: actions/checkout@b4ffde65f46336ab88eb53be808477a3936bae11 # v4.1.1
with:
fetch-depth: 0
persist-credentials: false
- name: Set up JDK
uses: actions/setup-java@387ac29b308b003ca37ba93a6cab5eb57c8f5f93 # v4.0.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

3
.gitignore vendored
View File

@@ -9,6 +9,3 @@
target
*.iml
*.swp
# The Git repositories checked out by the integration test framework.
integration-tests/.repos

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

129
README.md
View File

@@ -15,26 +15,13 @@ focussing on maintainability, consistency and avoidance of common pitfalls.
> Error Prone is a static analysis tool for Java that catches common
> programming mistakes at compile-time.
To learn more about Error Prone (Support), how you can start using Error Prone
in practice, and how we use it at Picnic, watch the conference talk
[_Automating away bugs with Error Prone in practice_][conference-talk]. Also
consider checking out the blog post [_Picnic loves Error Prone: producing
high-quality and consistent Java code_][picnic-blog-ep-post].
Read more on how Picnic uses Error Prone (Support) in the blog post [_Picnic
loves Error Prone: producing high-quality and consistent Java
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]
@@ -49,9 +36,7 @@ high-quality and consistent Java code_][picnic-blog-ep-post].
### Installation
This library is built on top of [Error Prone][error-prone-orig-repo]. To use
it, read the installation guide for Maven or Gradle below.
#### Maven
it:
1. First, follow Error Prone's [installation
guide][error-prone-installation-guide].
@@ -109,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:
@@ -185,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.
@@ -210,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:
@@ -258,27 +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
[conference-talk]: https://www.youtube.com/watch?v=-47WD-3wKBs
[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.yml/badge.svg
[github-actions-build-master]: https://github.com/PicnicSupermarket/error-prone-support/actions/workflows/build.yml?query=branch:master&event=push
[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%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
@@ -286,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
@@ -301,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=27/10/2025
keyVersion=1
signature=aQLVt0J//7nXDAlJuBe9ru7Dq6oApcLh17rxsgHoJqBkUCd2zvrtRxkcPm5PmF1I8gBDT9PHb+kTf6Kcfz+D1fT0wRrZv38ip56521AQPD6zjRSqak9O2Gisjo+QTPMD7oS009aSWKzQ1SMdtuZ+KIRvw4Gl+frtYzexqnGWEUQ\=
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,115 +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.service.AutoService;
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.VisitorState;
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 java.util.Optional;
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
@AutoService(Extractor.class)
@SuppressWarnings("rawtypes" /* See https://github.com/google/auto/issues/870. */)
public final class BugPatternExtractor implements Extractor<BugPatternDocumentation> {
/** Instantiates a new {@link BugPatternExtractor} instance. */
public BugPatternExtractor() {}
@Override
public String identifier() {
return "bugpattern";
}
@Override
public Optional<BugPatternDocumentation> tryExtract(ClassTree tree, VisitorState state) {
ClassSymbol symbol = ASTHelpers.getSymbol(tree);
BugPattern annotation = symbol.getAnnotation(BugPattern.class);
if (annotation == null) {
return Optional.empty();
}
return Optional.of(
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()));
}
/**
* 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,232 +0,0 @@
package tech.picnic.errorprone.documentation;
import static com.google.errorprone.matchers.Matchers.instanceMethod;
import static com.google.errorprone.matchers.method.MethodMatchers.staticMethod;
import static java.util.Objects.requireNonNull;
import static java.util.function.Predicate.not;
import com.google.auto.service.AutoService;
import com.google.auto.value.AutoValue;
import com.google.common.collect.ImmutableList;
import com.google.errorprone.VisitorState;
import com.google.errorprone.annotations.Immutable;
import com.google.errorprone.matchers.Matcher;
import com.google.errorprone.util.ASTHelpers;
import com.sun.source.tree.ClassTree;
import com.sun.source.tree.ExpressionTree;
import com.sun.source.tree.MethodInvocationTree;
import com.sun.source.util.TreeScanner;
import java.util.ArrayList;
import java.util.List;
import java.util.Objects;
import java.util.Optional;
import org.jspecify.annotations.NonNull;
import org.jspecify.annotations.Nullable;
import tech.picnic.errorprone.documentation.BugPatternTestExtractor.TestCases;
/**
* An {@link Extractor} that describes how to extract data from classes that test a {@code
* BugChecker}.
*/
// XXX: Handle other methods from `{BugCheckerRefactoring,Compilation}TestHelper`:
// - Indicate which custom arguments are specified, if any.
// - For replacement tests, indicate which `FixChooser` is used.
// - ... (We don't use all optional features; TBD what else to support.)
@Immutable
@AutoService(Extractor.class)
@SuppressWarnings("rawtypes" /* See https://github.com/google/auto/issues/870. */)
public final class BugPatternTestExtractor implements Extractor<TestCases> {
/** Instantiates a new {@link BugPatternTestExtractor} instance. */
public BugPatternTestExtractor() {}
@Override
public String identifier() {
return "bugpattern-test";
}
@Override
public Optional<TestCases> tryExtract(ClassTree tree, VisitorState state) {
BugPatternTestCollector collector = new BugPatternTestCollector();
collector.scan(tree, state);
return Optional.of(collector.getCollectedTests())
.filter(not(ImmutableList::isEmpty))
.map(
tests ->
new AutoValue_BugPatternTestExtractor_TestCases(
ASTHelpers.getSymbol(tree).className(), tests));
}
private static final class BugPatternTestCollector
extends TreeScanner<@Nullable Void, VisitorState> {
private static final Matcher<ExpressionTree> COMPILATION_HELPER_DO_TEST =
instanceMethod()
.onDescendantOf("com.google.errorprone.CompilationTestHelper")
.named("doTest");
private static final Matcher<ExpressionTree> TEST_HELPER_NEW_INSTANCE =
staticMethod()
.onDescendantOfAny(
"com.google.errorprone.CompilationTestHelper",
"com.google.errorprone.BugCheckerRefactoringTestHelper")
.named("newInstance")
.withParameters(Class.class.getCanonicalName(), Class.class.getCanonicalName());
private static final Matcher<ExpressionTree> IDENTIFICATION_SOURCE_LINES =
instanceMethod()
.onDescendantOf("com.google.errorprone.CompilationTestHelper")
.named("addSourceLines");
private static final Matcher<ExpressionTree> REPLACEMENT_DO_TEST =
instanceMethod()
.onDescendantOf("com.google.errorprone.BugCheckerRefactoringTestHelper")
.named("doTest");
private static final Matcher<ExpressionTree> REPLACEMENT_EXPECT_UNCHANGED =
instanceMethod()
.onDescendantOf("com.google.errorprone.BugCheckerRefactoringTestHelper.ExpectOutput")
.named("expectUnchanged");
private static final Matcher<ExpressionTree> REPLACEMENT_OUTPUT_SOURCE_LINES =
instanceMethod()
.onDescendantOf("com.google.errorprone.BugCheckerRefactoringTestHelper.ExpectOutput")
.namedAnyOf("addOutputLines", "expectUnchanged");
private final List<TestCase> collectedTestCases = new ArrayList<>();
private ImmutableList<TestCase> getCollectedTests() {
return ImmutableList.copyOf(collectedTestCases);
}
@Override
public @Nullable Void visitMethodInvocation(MethodInvocationTree node, VisitorState state) {
boolean isReplacementTest = REPLACEMENT_DO_TEST.matches(node, state);
if (isReplacementTest || COMPILATION_HELPER_DO_TEST.matches(node, state)) {
getClassUnderTest(node, state)
.ifPresent(
classUnderTest -> {
List<TestEntry> entries = new ArrayList<>();
if (isReplacementTest) {
extractReplacementTestCases(node, entries, state);
} else {
extractIdentificationTestCases(node, entries, state);
}
if (!entries.isEmpty()) {
collectedTestCases.add(
new AutoValue_BugPatternTestExtractor_TestCase(
classUnderTest, ImmutableList.copyOf(entries).reverse()));
}
});
}
return super.visitMethodInvocation(node, state);
}
private static Optional<String> getClassUnderTest(
MethodInvocationTree tree, VisitorState state) {
if (TEST_HELPER_NEW_INSTANCE.matches(tree, state)) {
return Optional.ofNullable(ASTHelpers.getSymbol(tree.getArguments().get(0)))
.filter(s -> !s.type.allparams().isEmpty())
.map(s -> s.type.allparams().get(0).tsym.getQualifiedName().toString());
}
ExpressionTree receiver = ASTHelpers.getReceiver(tree);
return receiver instanceof MethodInvocationTree
? getClassUnderTest((MethodInvocationTree) receiver, state)
: Optional.empty();
}
private static void extractIdentificationTestCases(
MethodInvocationTree tree, List<TestEntry> sink, VisitorState state) {
if (IDENTIFICATION_SOURCE_LINES.matches(tree, state)) {
String path = ASTHelpers.constValue(tree.getArguments().get(0), String.class);
Optional<String> sourceCode =
getSourceCode(tree).filter(s -> s.contains("// BUG: Diagnostic"));
if (path != null && sourceCode.isPresent()) {
sink.add(
new AutoValue_BugPatternTestExtractor_IdentificationTestEntry(
path, sourceCode.orElseThrow()));
}
}
ExpressionTree receiver = ASTHelpers.getReceiver(tree);
if (receiver instanceof MethodInvocationTree) {
extractIdentificationTestCases((MethodInvocationTree) receiver, sink, state);
}
}
private static void extractReplacementTestCases(
MethodInvocationTree tree, List<TestEntry> sink, VisitorState state) {
if (REPLACEMENT_OUTPUT_SOURCE_LINES.matches(tree, state)) {
/*
* Retrieve the method invocation that contains the input source code. Note that this cast
* is safe, because this code is guarded by an earlier call to `#getClassUnderTest(..)`,
* which ensures that `tree` is part of a longer method invocation chain.
*/
MethodInvocationTree inputTree = (MethodInvocationTree) requireNonNull(ASTHelpers.getReceiver(tree));
String path = ASTHelpers.constValue(inputTree.getArguments().get(0), String.class);
Optional<String> inputCode = getSourceCode(inputTree);
if (path != null && inputCode.isPresent()) {
Optional<String> outputCode =
REPLACEMENT_EXPECT_UNCHANGED.matches(tree, state) ? inputCode : getSourceCode(tree);
if (outputCode.isPresent() && !inputCode.equals(outputCode)) {
sink.add(
new AutoValue_BugPatternTestExtractor_ReplacementTestEntry(
path, inputCode.orElseThrow(), outputCode.orElseThrow()));
}
}
}
ExpressionTree receiver = ASTHelpers.getReceiver(tree);
if (receiver instanceof MethodInvocationTree) {
extractReplacementTestCases((MethodInvocationTree) receiver, sink, state);
}
}
// XXX: This logic is duplicated in `ErrorProneTestSourceFormat`. Can we do better?
private static Optional<String> getSourceCode(MethodInvocationTree tree) {
List<? extends ExpressionTree> sourceLines =
tree.getArguments().subList(1, tree.getArguments().size());
StringBuilder source = new StringBuilder();
for (ExpressionTree sourceLine : sourceLines) {
String value = ASTHelpers.constValue(sourceLine, String.class);
if (value == null) {
return Optional.empty();
}
source.append(value).append('\n');
}
return Optional.of(source.toString());
}
}
@AutoValue
abstract static class TestCases {
abstract String testClass();
abstract ImmutableList<TestCase> testCases();
}
@AutoValue
abstract static class TestCase {
abstract String classUnderTest();
abstract ImmutableList<TestEntry> entries();
}
interface TestEntry {
String path();
}
@AutoValue
abstract static class ReplacementTestEntry implements TestEntry {
abstract String input();
abstract String output();
}
@AutoValue
abstract static class IdentificationTestEntry implements TestEntry {
abstract String code();
}
}

View File

@@ -1,57 +0,0 @@
package tech.picnic.errorprone.documentation;
import static com.google.common.base.Preconditions.checkArgument;
import static java.util.Objects.requireNonNull;
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 = requireNonNull(matcher.group(1), "Path must be present");
try {
return Path.of(path);
} catch (InvalidPathException e) {
throw new IllegalArgumentException(String.format("Invalid path '%s'", path), e);
}
}
}

View File

@@ -1,112 +0,0 @@
package tech.picnic.errorprone.documentation;
import static java.nio.charset.StandardCharsets.UTF_8;
import static java.util.Objects.requireNonNull;
import com.fasterxml.jackson.annotation.JsonAutoDetect.Visibility;
import com.fasterxml.jackson.annotation.PropertyAccessor;
import com.fasterxml.jackson.databind.ObjectMapper;
import com.google.common.collect.ImmutableList;
import com.google.errorprone.VisitorState;
import com.sun.source.tree.ClassTree;
import com.sun.source.tree.CompilationUnitTree;
import com.sun.source.util.TaskEvent;
import com.sun.source.util.TaskEvent.Kind;
import com.sun.source.util.TaskListener;
import com.sun.source.util.TreePath;
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 java.util.ServiceLoader;
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 {
@SuppressWarnings({"rawtypes", "unchecked"})
private static final ImmutableList<Extractor<?>> EXTRACTORS =
(ImmutableList)
ImmutableList.copyOf(
ServiceLoader.load(
Extractor.class, DocumentationGeneratorTaskListener.class.getClassLoader()));
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;
}
JavaFileObject sourceFile = taskEvent.getSourceFile();
CompilationUnitTree compilationUnit = taskEvent.getCompilationUnit();
ClassTree classTree = JavacTrees.instance(context).getTree(taskEvent.getTypeElement());
if (sourceFile == null || compilationUnit == null || classTree == null) {
return;
}
VisitorState state =
VisitorState.createForUtilityPurposes(context)
.withPath(new TreePath(new TreePath(compilationUnit), classTree));
for (Extractor<?> extractor : EXTRACTORS) {
extractor
.tryExtract(classTree, state)
.ifPresent(
data ->
writeToFile(
extractor.identifier(), getSimpleClassName(sourceFile.toUri()), data));
}
}
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 extends Object> 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 requireNonNull(Paths.get(path).getFileName(), "Path lacks filename")
.toString()
.replace(".java", "");
}
}

View File

@@ -1,33 +0,0 @@
package tech.picnic.errorprone.documentation;
import com.google.errorprone.VisitorState;
import com.google.errorprone.annotations.Immutable;
import com.sun.source.tree.ClassTree;
import java.util.Optional;
/**
* 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> {
/**
* Returns the unique identifier of this extractor.
*
* @return A non-{@code null} string.
*/
String identifier();
/**
* Attempts to extract an instance of type {@link T} using the provided arguments.
*
* @param tree The {@link ClassTree} to analyze and from which to extract an instance of type
* {@link T}.
* @param state A {@link VisitorState} describing the context in which the given {@link ClassTree}
* is found.
* @return An instance of type {@link T}, if possible.
*/
Optional<T> tryExtract(ClassTree tree, VisitorState state);
}

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,106 +0,0 @@
package tech.picnic.errorprone.documentation;
import static java.nio.charset.StandardCharsets.UTF_8;
import static org.assertj.core.api.Assertions.assertThat;
import com.google.common.io.Resources;
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 {}");
verifyGeneratedFileContent(outputDirectory, "MinimalBugChecker");
}
@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 {}");
verifyGeneratedFileContent(outputDirectory, "CompleteBugChecker");
}
@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 {}");
verifyGeneratedFileContent(outputDirectory, "UndocumentedSuppressionBugPattern");
}
private static void verifyGeneratedFileContent(Path outputDirectory, String testClass)
throws IOException {
String resourceName = String.format("bugpattern-%s.json", testClass);
assertThat(outputDirectory.resolve(resourceName))
.content(UTF_8)
.isEqualToIgnoringWhitespace(
getResource(
String.join("-", BugPatternExtractorTest.class.getSimpleName(), resourceName)));
}
// XXX: Once we support only JDK 15+, drop this method in favour of including the resources as
// text blocks in this class.
private static String getResource(String resourceName) throws IOException {
return Resources.toString(
Resources.getResource(BugPatternExtractorTest.class, resourceName), UTF_8);
}
}

View File

@@ -1,468 +0,0 @@
package tech.picnic.errorprone.documentation;
import static java.nio.charset.StandardCharsets.UTF_8;
import static org.assertj.core.api.Assertions.assertThat;
import com.google.common.io.Resources;
import java.io.IOException;
import java.nio.file.Path;
import org.junit.jupiter.api.Test;
import org.junit.jupiter.api.io.TempDir;
final class BugPatternTestExtractorTest {
@Test
void noTestClass(@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 noDoTestInvocation(@TempDir Path outputDirectory) {
Compilation.compileWithDocumentationGenerator(
outputDirectory,
"TestCheckerTest.java",
"import com.google.errorprone.BugCheckerRefactoringTestHelper;",
"import com.google.errorprone.CompilationTestHelper;",
"import com.google.errorprone.bugpatterns.BugChecker;",
"",
"final class TestCheckerTest {",
" private static class TestChecker extends BugChecker {}",
"",
" void m() {",
" CompilationTestHelper.newInstance(TestChecker.class, getClass())",
" .addSourceLines(\"A.java\", \"// BUG: Diagnostic contains:\", \"class A {}\");",
"",
" BugCheckerRefactoringTestHelper.newInstance(TestChecker.class, getClass())",
" .addInputLines(\"A.java\", \"class A {}\")",
" .addOutputLines(\"A.java\", \"class A { /* This is a change. */ }\");",
" }",
"}");
assertThat(outputDirectory.toAbsolutePath()).isEmptyDirectory();
}
@Test
void nullBugCheckerInstance(@TempDir Path outputDirectory) {
Compilation.compileWithDocumentationGenerator(
outputDirectory,
"TestCheckerTest.java",
"import com.google.errorprone.BugCheckerRefactoringTestHelper;",
"import com.google.errorprone.CompilationTestHelper;",
"import com.google.errorprone.bugpatterns.BugChecker;",
"",
"final class TestCheckerTest {",
" void m() {",
" CompilationTestHelper.newInstance((Class<BugChecker>) null, getClass())",
" .addSourceLines(\"A.java\", \"// BUG: Diagnostic contains:\", \"class A {}\")",
" .doTest();",
"",
" BugCheckerRefactoringTestHelper.newInstance((Class<BugChecker>) null, getClass())",
" .addInputLines(\"A.java\", \"class A {}\")",
" .addOutputLines(\"A.java\", \"class A { /* This is a change. */ }\")",
" .doTest();",
" }",
"}");
assertThat(outputDirectory.toAbsolutePath()).isEmptyDirectory();
}
@Test
void rawBugCheckerInstance(@TempDir Path outputDirectory) {
Compilation.compileWithDocumentationGenerator(
outputDirectory,
"TestCheckerTest.java",
"import com.google.errorprone.BugCheckerRefactoringTestHelper;",
"import com.google.errorprone.CompilationTestHelper;",
"import com.google.errorprone.bugpatterns.BugChecker;",
"",
"final class TestCheckerTest {",
" private static class TestChecker extends BugChecker {}",
"",
" @SuppressWarnings(\"unchecked\")",
" void m() {",
" @SuppressWarnings(\"rawtypes\")",
" Class bugChecker = TestChecker.class;",
"",
" CompilationTestHelper.newInstance(bugChecker, getClass())",
" .addSourceLines(\"A.java\", \"// BUG: Diagnostic contains:\", \"class A {}\")",
" .doTest();",
"",
" BugCheckerRefactoringTestHelper.newInstance(bugChecker, getClass())",
" .addInputLines(\"A.java\", \"class A {}\")",
" .addOutputLines(\"A.java\", \"class A { /* This is a change. */ }\")",
" .doTest();",
" }",
"}");
assertThat(outputDirectory.toAbsolutePath()).isEmptyDirectory();
}
@Test
void scannerSupplierInstance(@TempDir Path outputDirectory) {
Compilation.compileWithDocumentationGenerator(
outputDirectory,
"TestCheckerTest.java",
"import com.google.errorprone.BugCheckerRefactoringTestHelper;",
"import com.google.errorprone.CompilationTestHelper;",
"import com.google.errorprone.bugpatterns.BugChecker;",
"import com.google.errorprone.scanner.ScannerSupplier;",
"",
"final class TestCheckerTest {",
" private static class TestChecker extends BugChecker {}",
"",
" void m() {",
" CompilationTestHelper.newInstance(",
" ScannerSupplier.fromBugCheckerClasses(TestChecker.class), getClass())",
" .addSourceLines(\"A.java\", \"// BUG: Diagnostic contains:\", \"class A {}\")",
" .doTest();",
"",
" BugCheckerRefactoringTestHelper.newInstance(",
" ScannerSupplier.fromBugCheckerClasses(TestChecker.class), getClass())",
" .addInputLines(\"A.java\", \"class A {}\")",
" .addOutputLines(\"A.java\", \"class A { /* This is a change. */ }\")",
" .doTest();",
" }",
"}");
assertThat(outputDirectory.toAbsolutePath()).isEmptyDirectory();
}
@Test
void nonCompileTimeConstantStrings(@TempDir Path outputDirectory) {
Compilation.compileWithDocumentationGenerator(
outputDirectory,
"TestCheckerTest.java",
"import com.google.errorprone.BugCheckerRefactoringTestHelper;",
"import com.google.errorprone.CompilationTestHelper;",
"import com.google.errorprone.bugpatterns.BugChecker;",
"",
"final class TestCheckerTest {",
" private static class TestChecker extends BugChecker {}",
"",
" void m() {",
" CompilationTestHelper.newInstance(TestChecker.class, getClass())",
" .addSourceLines(toString() + \"A.java\", \"// BUG: Diagnostic contains:\", \"class A {}\")",
" .addSourceLines(\"B.java\", \"// BUG: Diagnostic contains:\", \"class B {}\", toString())",
" .doTest();",
"",
" BugCheckerRefactoringTestHelper.newInstance(TestChecker.class, getClass())",
" .addInputLines(toString() + \"A.java\", \"class A {}\")",
" .addOutputLines(\"A.java\", \"class A { /* This is a change. */ }\")",
" .addInputLines(\"B.java\", \"class B {}\", toString())",
" .addOutputLines(\"B.java\", \"class B { /* This is a change. */ }\")",
" .addInputLines(\"C.java\", \"class C {}\")",
" .addOutputLines(\"C.java\", \"class C { /* This is a change. */ }\", toString())",
" .doTest();",
" }",
"}");
assertThat(outputDirectory.toAbsolutePath()).isEmptyDirectory();
}
@Test
void nonFluentTestHelperExpressions(@TempDir Path outputDirectory) {
Compilation.compileWithDocumentationGenerator(
outputDirectory,
"TestCheckerTest.java",
"import com.google.errorprone.BugCheckerRefactoringTestHelper;",
"import com.google.errorprone.CompilationTestHelper;",
"import com.google.errorprone.bugpatterns.BugChecker;",
"",
"final class TestCheckerTest {",
" private static class TestChecker extends BugChecker {}",
"",
" void m() {",
" CompilationTestHelper testHelper =",
" CompilationTestHelper.newInstance(TestChecker.class, getClass())",
" .addSourceLines(\"A.java\", \"class A {}\");",
" testHelper.doTest();",
"",
" BugCheckerRefactoringTestHelper.ExpectOutput expectedOutput =",
" BugCheckerRefactoringTestHelper.newInstance(TestChecker.class, getClass())",
" .addInputLines(\"A.java\", \"class A {}\");",
" expectedOutput.addOutputLines(\"A.java\", \"class A {}\").doTest();",
" expectedOutput.expectUnchanged().doTest();",
" }",
"}");
assertThat(outputDirectory.toAbsolutePath()).isEmptyDirectory();
}
@Test
void noSource(@TempDir Path outputDirectory) {
Compilation.compileWithDocumentationGenerator(
outputDirectory,
"TestCheckerTest.java",
"import com.google.errorprone.BugCheckerRefactoringTestHelper;",
"import com.google.errorprone.CompilationTestHelper;",
"import com.google.errorprone.bugpatterns.BugChecker;",
"",
"final class TestCheckerTest {",
" private static class TestChecker extends BugChecker {}",
"",
" void m() {",
" CompilationTestHelper.newInstance(TestChecker.class, getClass()).doTest();",
"",
" BugCheckerRefactoringTestHelper.newInstance(TestChecker.class, getClass()).doTest();",
" }",
"}");
assertThat(outputDirectory.toAbsolutePath()).isEmptyDirectory();
}
@Test
void noDiagnostics(@TempDir Path outputDirectory) {
Compilation.compileWithDocumentationGenerator(
outputDirectory,
"TestCheckerTest.java",
"import com.google.errorprone.BugCheckerRefactoringTestHelper;",
"import com.google.errorprone.CompilationTestHelper;",
"import com.google.errorprone.bugpatterns.BugChecker;",
"",
"final class TestCheckerTest {",
" private static class TestChecker extends BugChecker {}",
"",
" void m() {",
" CompilationTestHelper.newInstance(TestChecker.class, getClass())",
" .addSourceLines(\"A.java\", \"class A {}\")",
" .doTest();",
"",
" BugCheckerRefactoringTestHelper.newInstance(TestChecker.class, getClass())",
" .addInputLines(\"A.java\", \"class A {}\")",
" .addOutputLines(\"A.java\", \"class A {}\")",
" .addInputLines(\"B.java\", \"class B {}\")",
" .expectUnchanged()",
" .doTest();",
" }",
"}");
assertThat(outputDirectory.toAbsolutePath()).isEmptyDirectory();
}
@Test
void singleFileCompilationTestHelper(@TempDir Path outputDirectory) throws IOException {
Compilation.compileWithDocumentationGenerator(
outputDirectory,
"SingleFileCompilationTestHelperTest.java",
"import com.google.errorprone.CompilationTestHelper;",
"import com.google.errorprone.bugpatterns.BugChecker;",
"",
"final class SingleFileCompilationTestHelperTest {",
" private static class TestChecker extends BugChecker {}",
"",
" void m() {",
" CompilationTestHelper.newInstance(TestChecker.class, getClass())",
" .addSourceLines(\"A.java\", \"// BUG: Diagnostic contains:\", \"class A {}\")",
" .doTest();",
" }",
"}");
verifyGeneratedFileContent(outputDirectory, "SingleFileCompilationTestHelperTest");
}
@Test
void singleFileCompilationTestHelperWithSetArgs(@TempDir Path outputDirectory)
throws IOException {
Compilation.compileWithDocumentationGenerator(
outputDirectory,
"SingleFileCompilationTestHelperWithSetArgsTest.java",
"import com.google.errorprone.CompilationTestHelper;",
"import com.google.errorprone.bugpatterns.BugChecker;",
"",
"final class SingleFileCompilationTestHelperWithSetArgsTest {",
" private static class TestChecker extends BugChecker {}",
"",
" void m() {",
" CompilationTestHelper.newInstance(TestChecker.class, getClass())",
" .setArgs(\"-XepAllSuggestionsAsWarnings\")",
" .addSourceLines(\"A.java\", \"// BUG: Diagnostic contains:\", \"class A {}\")",
" .doTest();",
" }",
"}");
verifyGeneratedFileContent(outputDirectory, "SingleFileCompilationTestHelperWithSetArgsTest");
}
@Test
void multiFileCompilationTestHelper(@TempDir Path outputDirectory) throws IOException {
Compilation.compileWithDocumentationGenerator(
outputDirectory,
"MultiFileCompilationTestHelperTest.java",
"import com.google.errorprone.CompilationTestHelper;",
"import com.google.errorprone.bugpatterns.BugChecker;",
"",
"final class MultiFileCompilationTestHelperTest {",
" private static class TestChecker extends BugChecker {}",
"",
" void m() {",
" CompilationTestHelper.newInstance(TestChecker.class, getClass())",
" .addSourceLines(\"A.java\", \"// BUG: Diagnostic contains:\", \"class A {}\")",
" .addSourceLines(\"B.java\", \"// BUG: Diagnostic contains:\", \"class B {}\")",
" .doTest();",
" }",
"}");
verifyGeneratedFileContent(outputDirectory, "MultiFileCompilationTestHelperTest");
}
@Test
void singleFileBugCheckerRefactoringTestHelper(@TempDir Path outputDirectory) throws IOException {
Compilation.compileWithDocumentationGenerator(
outputDirectory,
"SingleFileBugCheckerRefactoringTestHelperTest.java",
"import com.google.errorprone.BugCheckerRefactoringTestHelper;",
"import com.google.errorprone.bugpatterns.BugChecker;",
"",
"final class SingleFileBugCheckerRefactoringTestHelperTest {",
" private static class TestChecker extends BugChecker {}",
"",
" void m() {",
" BugCheckerRefactoringTestHelper.newInstance(TestChecker.class, getClass())",
" .addInputLines(\"A.java\", \"class A {}\")",
" .addOutputLines(\"A.java\", \"class A { /* This is a change. */ }\")",
" .doTest();",
" }",
"}");
verifyGeneratedFileContent(outputDirectory, "SingleFileBugCheckerRefactoringTestHelperTest");
}
@Test
void singleFileBugCheckerRefactoringTestHelperWithSetArgsFixChooserAndCustomTestMode(
@TempDir Path outputDirectory) throws IOException {
Compilation.compileWithDocumentationGenerator(
outputDirectory,
"SingleFileBugCheckerRefactoringTestHelperWithSetArgsFixChooserAndCustomTestModeTest.java",
"import com.google.errorprone.BugCheckerRefactoringTestHelper;",
"import com.google.errorprone.BugCheckerRefactoringTestHelper.FixChoosers;",
"import com.google.errorprone.BugCheckerRefactoringTestHelper.TestMode;",
"import com.google.errorprone.bugpatterns.BugChecker;",
"",
"final class SingleFileBugCheckerRefactoringTestHelperWithSetArgsFixChooserAndCustomTestModeTest {",
" private static class TestChecker extends BugChecker {}",
"",
" void m() {",
" BugCheckerRefactoringTestHelper.newInstance(TestChecker.class, getClass())",
" .setArgs(\"-XepAllSuggestionsAsWarnings\")",
" .setFixChooser(FixChoosers.SECOND)",
" .addInputLines(\"A.java\", \"class A {}\")",
" .addOutputLines(\"A.java\", \"class A { /* This is a change. */ }\")",
" .doTest(TestMode.TEXT_MATCH);",
" }",
"}");
verifyGeneratedFileContent(
outputDirectory,
"SingleFileBugCheckerRefactoringTestHelperWithSetArgsFixChooserAndCustomTestModeTest");
}
@Test
void multiFileBugCheckerRefactoringTestHelper(@TempDir Path outputDirectory) throws IOException {
Compilation.compileWithDocumentationGenerator(
outputDirectory,
"MultiFileBugCheckerRefactoringTestHelperTest.java",
"import com.google.errorprone.BugCheckerRefactoringTestHelper;",
"import com.google.errorprone.bugpatterns.BugChecker;",
"",
"final class MultiFileBugCheckerRefactoringTestHelperTest {",
" private static class TestChecker extends BugChecker {}",
"",
" void m() {",
" BugCheckerRefactoringTestHelper.newInstance(TestChecker.class, getClass())",
" .addInputLines(\"A.java\", \"class A {}\")",
" .addOutputLines(\"A.java\", \"class A { /* This is a change. */ }\")",
" .addInputLines(\"B.java\", \"class B {}\")",
" .addOutputLines(\"B.java\", \"class B { /* This is a change. */ }\")",
" .doTest();",
" }",
"}");
verifyGeneratedFileContent(outputDirectory, "MultiFileBugCheckerRefactoringTestHelperTest");
}
@Test
void compilationAndBugCheckerRefactoringTestHelpers(@TempDir Path outputDirectory)
throws IOException {
Compilation.compileWithDocumentationGenerator(
outputDirectory,
"CompilationAndBugCheckerRefactoringTestHelpersTest.java",
"import com.google.errorprone.BugCheckerRefactoringTestHelper;",
"import com.google.errorprone.CompilationTestHelper;",
"import com.google.errorprone.bugpatterns.BugChecker;",
"",
"final class CompilationAndBugCheckerRefactoringTestHelpersTest {",
" private static class TestChecker extends BugChecker {}",
"",
" void m() {",
" CompilationTestHelper.newInstance(TestChecker.class, getClass())",
" .addSourceLines(\"A.java\", \"// BUG: Diagnostic contains:\", \"class A {}\")",
" .doTest();",
"",
" BugCheckerRefactoringTestHelper.newInstance(TestChecker.class, getClass())",
" .addInputLines(\"A.java\", \"class A {}\")",
" .addOutputLines(\"A.java\", \"class A { /* This is a change. */ }\")",
" .doTest();",
" }",
"}");
verifyGeneratedFileContent(
outputDirectory, "CompilationAndBugCheckerRefactoringTestHelpersTest");
}
@Test
void compilationAndBugCheckerRefactoringTestHelpersWithCustomCheckerPackageAndNames(
@TempDir Path outputDirectory) throws IOException {
Compilation.compileWithDocumentationGenerator(
outputDirectory,
"CompilationAndBugCheckerRefactoringTestHelpersWithCustomCheckerPackageAndNamesTest.java",
"package pkg;",
"",
"import com.google.errorprone.BugCheckerRefactoringTestHelper;",
"import com.google.errorprone.CompilationTestHelper;",
"import com.google.errorprone.bugpatterns.BugChecker;",
"",
"final class CompilationAndBugCheckerRefactoringTestHelpersWithCustomCheckerPackageAndNamesTest {",
" private static class CustomTestChecker extends BugChecker {}",
"",
" private static class CustomTestChecker2 extends BugChecker {}",
"",
" void m() {",
" CompilationTestHelper.newInstance(CustomTestChecker.class, getClass())",
" .addSourceLines(\"A.java\", \"// BUG: Diagnostic contains:\", \"class A {}\")",
" .doTest();",
"",
" BugCheckerRefactoringTestHelper.newInstance(CustomTestChecker2.class, getClass())",
" .addInputLines(\"A.java\", \"class A {}\")",
" .addOutputLines(\"A.java\", \"class A { /* This is a change. */ }\")",
" .doTest();",
" }",
"}");
verifyGeneratedFileContent(
outputDirectory,
"CompilationAndBugCheckerRefactoringTestHelpersWithCustomCheckerPackageAndNamesTest");
}
private static void verifyGeneratedFileContent(Path outputDirectory, String testClass)
throws IOException {
String resourceName = String.format("bugpattern-test-%s.json", testClass);
assertThat(outputDirectory.resolve(resourceName))
.content(UTF_8)
.isEqualToIgnoringWhitespace(
getResource(
String.join("-", BugPatternTestExtractorTest.class.getSimpleName(), resourceName)));
}
// XXX: Once we support only JDK 15+, drop this method in favour of including the resources as
// text blocks in this class.
private static String getResource(String resourceName) throws IOException {
return Resources.toString(
Resources.getResource(BugPatternTestExtractorTest.class, resourceName), UTF_8);
}
}

View File

@@ -1,79 +0,0 @@
package tech.picnic.errorprone.documentation;
import static org.assertj.core.api.Assertions.assertThat;
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.io.IOException;
import java.io.UncheckedIOException;
import java.nio.file.Path;
import java.util.ArrayList;
import java.util.List;
import javax.tools.Diagnostic;
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: This class is supported by the `ErrorProneTestHelperSourceFormat` check, but until that
// support is covered by unit tests, make sure to update that logic if this class or its methods are
// moved/renamed.
public final class Compilation {
private Compilation() {}
public static void compileWithDocumentationGenerator(
Path outputDirectory, String path, String... lines) {
compileWithDocumentationGenerator(outputDirectory.toAbsolutePath().toString(), path, lines);
}
public static void compileWithDocumentationGenerator(
String outputDirectory, String path, String... lines) {
/*
* The compiler options specified here largely match those used by Error Prone's
* `CompilationTestHelper`. A key difference is the stricter linting configuration. When
* compiling using JDK 21+, these lint options also require that certain JDK modules are
* explicitly exported.
*/
compile(
ImmutableList.of(
"--add-exports=jdk.compiler/com.sun.tools.javac.code=ALL-UNNAMED",
"--add-exports=jdk.compiler/com.sun.tools.javac.tree=ALL-UNNAMED",
"--add-exports=jdk.compiler/com.sun.tools.javac.util=ALL-UNNAMED",
"-encoding",
"UTF-8",
"-parameters",
"-proc:none",
"-Werror",
"-Xlint:all,-serial",
"-Xplugin:DocumentationGenerator -XoutputDirectory=" + outputDirectory,
"-XDdev",
"-XDcompilePolicy=simple"),
FileObjects.forSourceLines(path, lines));
}
private static void compile(ImmutableList<String> options, JavaFileObject javaFileObject) {
try (JavacFileManager javacFileManager = FileManagers.testFileManager()) {
JavaCompiler compiler = JavacTool.create();
List<Diagnostic<?>> diagnostics = new ArrayList<>();
JavacTaskImpl task =
(JavacTaskImpl)
compiler.getTask(
null,
javacFileManager,
diagnostics::add,
options,
ImmutableList.of(),
ImmutableList.of(javaFileObject));
Boolean result = task.call();
assertThat(diagnostics).isEmpty();
assertThat(result).isTrue();
} catch (IOException e) {
throw new UncheckedIOException("Failed to close `JavaCompiler`", e);
}
}
}

View File

@@ -1,140 +0,0 @@
package tech.picnic.errorprone.documentation;
import static com.google.common.collect.ImmutableList.toImmutableList;
import static java.nio.charset.StandardCharsets.UTF_8;
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.auto.service.AutoService;
import com.google.auto.value.AutoValue;
import com.google.common.collect.ImmutableList;
import com.google.common.collect.ImmutableSet;
import com.google.common.collect.Sets;
import com.google.common.collect.Streams;
import com.google.errorprone.VisitorState;
import com.google.errorprone.annotations.Immutable;
import com.sun.source.tree.ClassTree;
import com.sun.source.tree.Tree;
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 java.util.Optional;
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");
}
@Test
void extraction(@TempDir Path outputDirectory) {
Compilation.compileWithDocumentationGenerator(
outputDirectory,
"DocumentationGeneratorTaskListenerTestClass.java",
"class DocumentationGeneratorTaskListenerTestClass {}");
// XXX: Once we support only JDK 15+, use a text block for the `expected` string.
assertThat(
outputDirectory.resolve(
"documentation-generator-task-listener-test-DocumentationGeneratorTaskListenerTestClass.json"))
.content(UTF_8)
.isEqualToIgnoringWhitespace(
"{\"className\":\"DocumentationGeneratorTaskListenerTestClass\",\"path\":[\"CLASS: DocumentationGeneratorTaskListenerTestClass\",\"COMPILATION_UNIT\"]}");
}
@Immutable
@AutoService(Extractor.class)
@SuppressWarnings("rawtypes" /* See https://github.com/google/auto/issues/870. */)
public static final class TestExtractor implements Extractor<ExtractionParameters> {
@Override
public String identifier() {
return "documentation-generator-task-listener-test";
}
@Override
public Optional<ExtractionParameters> tryExtract(ClassTree tree, VisitorState state) {
return Optional.of(tree.getSimpleName().toString())
.filter(n -> n.contains(DocumentationGeneratorTaskListenerTest.class.getSimpleName()))
.map(
className ->
new AutoValue_DocumentationGeneratorTaskListenerTest_ExtractionParameters(
className,
Streams.stream(state.getPath())
.map(TestExtractor::describeTree)
.collect(toImmutableList())));
}
private static String describeTree(Tree tree) {
return (tree instanceof ClassTree)
? String.join(": ", String.valueOf(tree.getKind()), ((ClassTree) tree).getSimpleName())
: tree.getKind().toString();
}
}
@AutoValue
abstract static class ExtractionParameters {
abstract String className();
abstract ImmutableList<String> path();
}
}

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

@@ -1,24 +0,0 @@
{
"testClass": "CompilationAndBugCheckerRefactoringTestHelpersTest",
"testCases": [
{
"classUnderTest": "CompilationAndBugCheckerRefactoringTestHelpersTest.TestChecker",
"entries": [
{
"path": "A.java",
"code": "// BUG: Diagnostic contains:\nclass A {}\n"
}
]
},
{
"classUnderTest": "CompilationAndBugCheckerRefactoringTestHelpersTest.TestChecker",
"entries": [
{
"path": "A.java",
"input": "class A {}\n",
"output": "class A { /* This is a change. */ }\n"
}
]
}
]
}

View File

@@ -1,24 +0,0 @@
{
"testClass": "pkg.CompilationAndBugCheckerRefactoringTestHelpersWithCustomCheckerPackageAndNamesTest",
"testCases": [
{
"classUnderTest": "pkg.CompilationAndBugCheckerRefactoringTestHelpersWithCustomCheckerPackageAndNamesTest.CustomTestChecker",
"entries": [
{
"path": "A.java",
"code": "// BUG: Diagnostic contains:\nclass A {}\n"
}
]
},
{
"classUnderTest": "pkg.CompilationAndBugCheckerRefactoringTestHelpersWithCustomCheckerPackageAndNamesTest.CustomTestChecker2",
"entries": [
{
"path": "A.java",
"input": "class A {}\n",
"output": "class A { /* This is a change. */ }\n"
}
]
}
]
}

View File

@@ -1,20 +0,0 @@
{
"testClass": "MultiFileBugCheckerRefactoringTestHelperTest",
"testCases": [
{
"classUnderTest": "MultiFileBugCheckerRefactoringTestHelperTest.TestChecker",
"entries": [
{
"path": "A.java",
"input": "class A {}\n",
"output": "class A { /* This is a change. */ }\n"
},
{
"path": "B.java",
"input": "class B {}\n",
"output": "class B { /* This is a change. */ }\n"
}
]
}
]
}

View File

@@ -1,18 +0,0 @@
{
"testClass": "MultiFileCompilationTestHelperTest",
"testCases": [
{
"classUnderTest": "MultiFileCompilationTestHelperTest.TestChecker",
"entries": [
{
"path": "A.java",
"code": "// BUG: Diagnostic contains:\nclass A {}\n"
},
{
"path": "B.java",
"code": "// BUG: Diagnostic contains:\nclass B {}\n"
}
]
}
]
}

View File

@@ -1,15 +0,0 @@
{
"testClass": "SingleFileBugCheckerRefactoringTestHelperTest",
"testCases": [
{
"classUnderTest": "SingleFileBugCheckerRefactoringTestHelperTest.TestChecker",
"entries": [
{
"path": "A.java",
"input": "class A {}\n",
"output": "class A { /* This is a change. */ }\n"
}
]
}
]
}

View File

@@ -1,15 +0,0 @@
{
"testClass": "SingleFileBugCheckerRefactoringTestHelperWithSetArgsFixChooserAndCustomTestModeTest",
"testCases": [
{
"classUnderTest": "SingleFileBugCheckerRefactoringTestHelperWithSetArgsFixChooserAndCustomTestModeTest.TestChecker",
"entries": [
{
"path": "A.java",
"input": "class A {}\n",
"output": "class A { /* This is a change. */ }\n"
}
]
}
]
}

View File

@@ -1,14 +0,0 @@
{
"testClass": "SingleFileCompilationTestHelperTest",
"testCases": [
{
"classUnderTest": "SingleFileCompilationTestHelperTest.TestChecker",
"entries": [
{
"path": "A.java",
"code": "// BUG: Diagnostic contains:\nclass A {}\n"
}
]
}
]
}

View File

@@ -1,14 +0,0 @@
{
"testClass": "SingleFileCompilationTestHelperWithSetArgsTest",
"testCases": [
{
"classUnderTest": "SingleFileCompilationTestHelperWithSetArgsTest.TestChecker",
"entries": [
{
"path": "A.java",
"code": "// BUG: Diagnostic contains:\nclass A {}\n"
}
]
}
]
}

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>
@@ -72,11 +64,6 @@
<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.googlejavaformat</groupId>
<artifactId>google-java-format</artifactId>
@@ -86,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>
@@ -126,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>
@@ -146,10 +133,6 @@
<artifactId>assertj-core</artifactId>
<scope>provided</scope>
</dependency>
<dependency>
<groupId>org.checkerframework</groupId>
<artifactId>checker-util</artifactId>
</dependency>
<dependency>
<groupId>org.immutables</groupId>
<artifactId>value-annotations</artifactId>
@@ -163,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>
@@ -180,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>
@@ -235,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>
@@ -253,10 +226,20 @@
</annotationProcessorPaths>
<compilerArgs combine.children="append">
<arg>-Xplugin:RefasterRuleCompiler</arg>
<arg>-Xplugin:DocumentationGenerator -XoutputDirectory=${project.build.directory}/docs</arg>
</compilerArgs>
</configuration>
</plugin>
<plugin>
<groupId>org.apache.maven.plugins</groupId>
<artifactId>maven-dependency-plugin</artifactId>
<configuration>
<ignoredUnusedDeclaredDependencies>
<!-- XXX: Figure out why the plugin thinks this
dependency is unused. -->
<ignoredUnusedDeclaredDependency>${project.groupId}:refaster-support</ignoredUnusedDeclaredDependency>
</ignoredUnusedDeclaredDependencies>
</configuration>
</plugin>
</plugins>
</pluginManagement>
</build>

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

@@ -9,21 +9,21 @@ 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.common.collect.ImmutableList;
import com.google.common.collect.Iterables;
import com.google.errorprone.BugPattern;
import com.google.errorprone.VisitorState;
import com.google.errorprone.bugpatterns.BugChecker;
import com.google.errorprone.bugpatterns.BugChecker.ClassTreeMatcher;
import com.google.errorprone.fixes.SuggestedFix;
import com.google.errorprone.matchers.Description;
import com.google.errorprone.matchers.MultiMatcher;
import com.google.errorprone.matchers.MultiMatcher.MultiMatchResult;
import com.google.errorprone.util.ASTHelpers;
import com.sun.source.tree.AnnotationTree;
import com.sun.source.tree.ClassTree;
import com.sun.source.tree.MethodTree;
import com.sun.source.tree.Tree;
import java.util.List;
import tech.picnic.errorprone.bugpatterns.util.SourceCode;
/** A {@link BugChecker} that flags redundant {@code @Autowired} constructor annotations. */
@AutoService(BugChecker.class)
@@ -48,18 +48,20 @@ public final class AutowiredConstructor extends BugChecker implements ClassTreeM
return Description.NO_MATCH;
}
MultiMatchResult<AnnotationTree> hasAutowiredAnnotation =
AUTOWIRED_ANNOTATION.multiMatchResult(Iterables.getOnlyElement(constructors), state);
if (!hasAutowiredAnnotation.matches()) {
ImmutableList<AnnotationTree> annotations =
AUTOWIRED_ANNOTATION
.multiMatchResult(Iterables.getOnlyElement(constructors), state)
.matchingNodes();
if (annotations.size() != 1) {
return Description.NO_MATCH;
}
/*
* 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 = hasAutowiredAnnotation.onlyMatchingNode();
return describeMatch(annotation, SourceCode.deleteWithTrailingWhitespace(annotation, state));
AnnotationTree annotation = Iterables.getOnlyElement(annotations);
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

@@ -1,92 +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.allOf;
import static com.google.errorprone.matchers.Matchers.anything;
import static com.google.errorprone.matchers.Matchers.classLiteral;
import static com.google.errorprone.matchers.Matchers.instanceMethod;
import static com.google.errorprone.matchers.Matchers.receiverOfInvocation;
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.errorprone.BugPattern;
import com.google.errorprone.VisitorState;
import com.google.errorprone.annotations.Var;
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.BinaryTree;
import com.sun.source.tree.ExpressionTree;
import com.sun.source.tree.MethodInvocationTree;
import com.sun.source.util.TreePath;
import com.sun.tools.javac.code.Symbol.MethodSymbol;
import java.util.regex.Pattern;
/**
* A {@link BugChecker} that flags invocations of {@link Class#getName()} where {@link
* Class#getCanonicalName()} was likely meant.
*
* <p>For top-level types these two methods generally return the same result, but for nested types
* the former separates identifiers using a dollar sign ({@code $}) rather than a dot ({@code .}).
*
* @implNote This check currently only flags {@link Class#getName()} invocations on class literals,
* and doesn't flag method references. This avoids false positives, such as suggesting use of
* {@link Class#getCanonicalName()} in contexts where the canonical name is {@code null}.
*/
@AutoService(BugChecker.class)
@BugPattern(
summary = "This code should likely use the type's canonical name",
link = BUG_PATTERNS_BASE_URL + "CanonicalClassNameUsage",
linkType = CUSTOM,
severity = WARNING,
tags = FRAGILE_CODE)
public final class CanonicalClassNameUsage extends BugChecker
implements MethodInvocationTreeMatcher {
private static final long serialVersionUID = 1L;
private static final Matcher<ExpressionTree> GET_NAME_INVOCATION =
toType(
MethodInvocationTree.class,
allOf(
receiverOfInvocation(classLiteral(anything())),
instanceMethod().onExactClass(Class.class.getCanonicalName()).named("getName")));
private static final Pattern CANONICAL_NAME_USING_TYPES =
Pattern.compile("(com\\.google\\.errorprone|tech\\.picnic\\.errorprone)\\..*");
/** Instantiates a new {@link CanonicalClassNameUsage} instance. */
public CanonicalClassNameUsage() {}
@Override
public Description matchMethodInvocation(MethodInvocationTree tree, VisitorState state) {
if (!GET_NAME_INVOCATION.matches(tree, state) || !isPassedToCanonicalNameUsingType(state)) {
/*
* This is not a `class.getName()` invocation of which the result is passed to another method
* known to accept canonical type names.
*/
return Description.NO_MATCH;
}
return describeMatch(
tree, SuggestedFixes.renameMethodInvocation(tree, "getCanonicalName", state));
}
private static boolean isPassedToCanonicalNameUsingType(VisitorState state) {
@Var TreePath path = state.getPath().getParentPath();
while (path.getLeaf() instanceof BinaryTree) {
path = path.getParentPath();
}
return path.getLeaf() instanceof MethodInvocationTree
&& isOwnedByCanonicalNameUsingType(
ASTHelpers.getSymbol((MethodInvocationTree) path.getLeaf()));
}
private static boolean isOwnedByCanonicalNameUsingType(MethodSymbol symbol) {
return CANONICAL_NAME_USING_TYPES.matcher(symbol.owner.getQualifiedName()).matches();
}
}

View File

@@ -7,9 +7,6 @@ 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.common.collect.ImmutableList;
import com.google.common.collect.ImmutableMap;
import com.google.common.collect.ImmutableSet;
import com.google.errorprone.BugPattern;
import com.google.errorprone.VisitorState;
import com.google.errorprone.bugpatterns.BugChecker;
@@ -20,11 +17,7 @@ import com.google.errorprone.matchers.Description;
import com.google.errorprone.matchers.Matcher;
import com.sun.source.tree.ExpressionTree;
import com.sun.source.tree.MethodInvocationTree;
import java.util.ArrayList;
import java.util.HashMap;
import java.util.HashSet;
import java.util.stream.Collector;
import java.util.stream.Collectors;
import tech.picnic.errorprone.bugpatterns.util.ThirdPartyLibrary;
/**
@@ -37,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,
@@ -45,7 +38,7 @@ import tech.picnic.errorprone.bugpatterns.util.ThirdPartyLibrary;
public final class CollectorMutability extends BugChecker implements MethodInvocationTreeMatcher {
private static final long serialVersionUID = 1L;
private static final Matcher<ExpressionTree> COLLECTOR_METHOD =
staticMethod().onClass(Collectors.class.getCanonicalName());
staticMethod().onClass("java.util.stream.Collectors");
private static final Matcher<ExpressionTree> LIST_COLLECTOR =
staticMethod().anyClass().named("toList");
private static final Matcher<ExpressionTree> MAP_COLLECTOR =
@@ -65,10 +58,7 @@ public final class CollectorMutability extends BugChecker implements MethodInvoc
if (LIST_COLLECTOR.matches(tree, state)) {
return suggestToCollectionAlternatives(
tree,
ImmutableList.class.getCanonicalName() + ".toImmutableList",
ArrayList.class.getCanonicalName(),
state);
tree, "com.google.common.collect.ImmutableList.toImmutableList", "ArrayList", state);
}
if (MAP_COLLECTOR.matches(tree, state)) {
@@ -77,10 +67,7 @@ public final class CollectorMutability extends BugChecker implements MethodInvoc
if (SET_COLLECTOR.matches(tree, state)) {
return suggestToCollectionAlternatives(
tree,
ImmutableSet.class.getCanonicalName() + ".toImmutableSet",
HashSet.class.getCanonicalName(),
state);
tree, "com.google.common.collect.ImmutableSet.toImmutableSet", "HashSet", state);
}
return Description.NO_MATCH;
@@ -88,20 +75,20 @@ public final class CollectorMutability extends BugChecker implements MethodInvoc
private Description suggestToCollectionAlternatives(
MethodInvocationTree tree,
String immutableReplacement,
String fullyQualifiedImmutableReplacement,
String mutableReplacement,
VisitorState state) {
SuggestedFix.Builder mutableFix = SuggestedFix.builder();
String toCollectionSelect =
SuggestedFixes.qualifyStaticImport(
Collectors.class.getCanonicalName() + ".toCollection", mutableFix, state);
String mutableCollection = SuggestedFixes.qualifyType(state, mutableFix, mutableReplacement);
"java.util.stream.Collectors.toCollection", mutableFix, state);
return buildDescription(tree)
.addFix(replaceMethodInvocation(tree, immutableReplacement, state))
.addFix(replaceMethodInvocation(tree, fullyQualifiedImmutableReplacement, state))
.addFix(
mutableFix
.replace(tree, String.format("%s(%s::new)", toCollectionSelect, mutableCollection))
.addImport(String.format("java.util.%s", mutableReplacement))
.replace(tree, String.format("%s(%s::new)", toCollectionSelect, mutableReplacement))
.build())
.build();
}
@@ -112,20 +99,17 @@ public final class CollectorMutability extends BugChecker implements MethodInvoc
return Description.NO_MATCH;
}
SuggestedFix.Builder mutableFix = SuggestedFix.builder();
String hashMap =
SuggestedFixes.qualifyType(state, mutableFix, HashMap.class.getCanonicalName());
return buildDescription(tree)
.addFix(
replaceMethodInvocation(
tree, ImmutableMap.class.getCanonicalName() + ".toImmutableMap", state))
tree, "com.google.common.collect.ImmutableMap.toImmutableMap", state))
.addFix(
mutableFix
SuggestedFix.builder()
.addImport("java.util.HashMap")
.postfixWith(
tree.getArguments().get(argCount - 1),
(argCount == 2 ? ", (a, b) -> { throw new IllegalStateException(); }" : "")
+ String.format(", %s::new", hashMap))
+ ", HashMap::new")
.build())
.build();
}

View File

@@ -1,178 +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.getCanonicalName())))),
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)
@@ -36,15 +36,12 @@ public final class EmptyMethod extends BugChecker implements MethodTreeMatcher {
private static final Matcher<Tree> PERMITTED_ANNOTATION =
annotations(
AT_LEAST_ONE,
anyOf(
isType(Override.class.getCanonicalName()),
isType("org.aspectj.lang.annotation.Pointcut")));
anyOf(isType("java.lang.Override"), isType("org.aspectj.lang.annotation.Pointcut")));
/** Instantiates a new {@link EmptyMethod} instance. */
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()
@@ -58,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

@@ -1,165 +0,0 @@
package tech.picnic.errorprone.bugpatterns;
import static com.google.common.collect.ImmutableSet.toImmutableSet;
import static com.google.errorprone.BugPattern.LinkType.CUSTOM;
import static com.google.errorprone.BugPattern.SeverityLevel.SUGGESTION;
import static com.google.errorprone.BugPattern.StandardTags.FRAGILE_CODE;
import static com.google.errorprone.matchers.Matchers.instanceMethod;
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.common.primitives.Primitives;
import com.google.errorprone.BugPattern;
import com.google.errorprone.VisitorState;
import com.google.errorprone.bugpatterns.BugChecker;
import com.google.errorprone.bugpatterns.BugChecker.LiteralTreeMatcher;
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.util.ASTHelpers;
import com.sun.source.tree.AnnotationTree;
import com.sun.source.tree.ExpressionTree;
import com.sun.source.tree.LiteralTree;
import com.sun.source.tree.MethodInvocationTree;
import com.sun.tools.javac.code.Symbol;
import com.sun.tools.javac.code.Symbol.ClassSymbol;
import com.sun.tools.javac.util.Constants;
import java.util.regex.Pattern;
import tech.picnic.errorprone.bugpatterns.util.ThirdPartyLibrary;
/**
* A {@link BugChecker} that flags literal strings in Error Prone Support code that represent the
* fully qualified class name of a type that's on Error Prone's classpath.
*
* <p>Deriving such strings from the associated {@link Class} instance makes for more maintainable
* code.
*/
// XXX: Consider generalizing this to a `RuntimeClasspath` check with a configurable
// `CLASSPATH_TYPES` regex. Such a check would likely be a no-op by default, with Error Prone
// Support's parent specifying the regex that is currently hard-coded.
// XXX: As-is this check may suggest usage of a string literal even if the relevant type will be on
// the runtime classpath. Review how we can further reduce false-positives.
// XXX: Slightly "out there", but we could try to derive the subset of classpath types likely to be
// available at runtime from the `compile`-scoped Maven dependencies in the `pom.xml` file of
// the module being compiled.
// XXX: Right now this check does not rewrite primitive string references such as `"int"` to
// `int.class.getCanonicalName()`. Review whether that's worth it.
@AutoService(BugChecker.class)
@BugPattern(
summary =
"Prefer `Class#getCanonicalName()` over an equivalent string literal if and only if the "
+ "type will be on the runtime classpath",
link = BUG_PATTERNS_BASE_URL + "ErrorProneRuntimeClasspath",
linkType = CUSTOM,
severity = SUGGESTION,
tags = FRAGILE_CODE)
public final class ErrorProneRuntimeClasspath extends BugChecker
implements LiteralTreeMatcher, MethodInvocationTreeMatcher {
private static final long serialVersionUID = 1L;
private static final Matcher<ExpressionTree> GET_CANONICAL_NAME_INVOCATION =
instanceMethod().onExactClass(Class.class.getCanonicalName()).named("getCanonicalName");
private static final ImmutableSet<String> PRIMITIVE_TYPES =
Primitives.allPrimitiveTypes().stream()
.map(Class::getCanonicalName)
.collect(toImmutableSet());
/**
* A pattern that matches fully qualified type names that are expected to be runtime dependencies
* of Error Prone, and that are thus presumed to be unconditionally present on a bug checker's
* runtime classpath.
*/
private static final Pattern CLASSPATH_TYPES =
Pattern.compile(
"com\\.google\\.common\\..*|com\\.google\\.errorprone\\.([^.]+(?<!TestHelper)(\\..*)?)|java\\..*");
/** Instantiates a new {@link ErrorProneRuntimeClasspath} instance. */
public ErrorProneRuntimeClasspath() {}
@Override
public Description matchLiteral(LiteralTree tree, VisitorState state) {
String value = ASTHelpers.constValue(tree, String.class);
if (value == null
|| !CLASSPATH_TYPES.matcher(value).matches()
|| state.findEnclosing(AnnotationTree.class) != null) {
return Description.NO_MATCH;
}
SuggestedFix fix = trySuggestClassReference(tree, value, state);
if (fix.isEmpty()) {
return Description.NO_MATCH;
}
return buildDescription(tree)
.setMessage(
"This type will be on the runtime classpath; use `Class#getCanonicalName()` instead")
.addFix(fix)
.build();
}
@Override
public Description matchMethodInvocation(MethodInvocationTree tree, VisitorState state) {
if (!GET_CANONICAL_NAME_INVOCATION.matches(tree, state)) {
return Description.NO_MATCH;
}
Symbol receiver = ASTHelpers.getSymbol(ASTHelpers.getReceiver(tree));
if (receiver == null
|| !(receiver.owner instanceof ClassSymbol)
|| PRIMITIVE_TYPES.contains(receiver.owner.getQualifiedName().toString())
|| CLASSPATH_TYPES.matcher(receiver.owner.getQualifiedName()).matches()) {
return Description.NO_MATCH;
}
/*
* This class reference may not be safe; suggest using a string literal instead. (Note that
* dropping the type reference may make the associated import statement (if any) obsolete.
* Dropping such imports is left to Error Prone's `RemoveUnusedImports` check.)
*/
return buildDescription(tree)
.setMessage("This type may not be on the runtime classpath; use a string literal instead")
.addFix(
SuggestedFix.replace(
tree, Constants.format(receiver.owner.getQualifiedName().toString())))
.build();
}
private static SuggestedFix trySuggestClassReference(
LiteralTree tree, String value, VisitorState state) {
if (isTypeOnClasspath(value, state)) {
return suggestClassReference(tree, value, "", state);
}
int lastDot = value.lastIndexOf('.');
String type = value.substring(0, lastDot);
if (isTypeOnClasspath(type, state)) {
return suggestClassReference(tree, type, value.substring(lastDot), state);
}
return SuggestedFix.emptyFix();
}
private static SuggestedFix suggestClassReference(
LiteralTree original, String type, String suffix, VisitorState state) {
SuggestedFix.Builder fix = SuggestedFix.builder();
String identifier = SuggestedFixes.qualifyType(state, fix, type);
return fix.replace(
original,
identifier
+ ".class.getCanonicalName()"
+ (suffix.isEmpty() ? "" : (" + " + Constants.format(suffix))))
.build();
}
private static boolean isTypeOnClasspath(String type, VisitorState state) {
try {
return ThirdPartyLibrary.canIntroduceUsage(type, state);
} catch (
@SuppressWarnings("java:S1166" /* Not exceptional. */)
IllegalArgumentException e) {
return false;
}
}
}

View File

@@ -5,7 +5,6 @@ import static com.google.errorprone.BugPattern.SeverityLevel.SUGGESTION;
import static com.google.errorprone.BugPattern.StandardTags.STYLE;
import static com.google.errorprone.matchers.Matchers.anyOf;
import static com.google.errorprone.matchers.Matchers.instanceMethod;
import static com.google.errorprone.matchers.Matchers.staticMethod;
import static java.util.stream.Collectors.joining;
import static tech.picnic.errorprone.bugpatterns.util.Documentation.BUG_PATTERNS_BASE_URL;
@@ -67,12 +66,7 @@ public final class ErrorProneTestHelperSourceFormat extends BugChecker
.named("addSourceLines"),
instanceMethod()
.onDescendantOf("com.google.errorprone.BugCheckerRefactoringTestHelper")
.named("addInputLines"),
// XXX: Add tests for `Compilation.compileWithDocumentationGenerator`. Until done, make
// sure to update this matcher if that method's class or name is changed/moved.
staticMethod()
.onClass("tech.picnic.errorprone.documentation.Compilation")
.named("compileWithDocumentationGenerator"));
.named("addInputLines"));
private static final Matcher<ExpressionTree> OUTPUT_SOURCE_ACCEPTING_METHOD =
instanceMethod()
.onDescendantOf("com.google.errorprone.BugCheckerRefactoringTestHelper.ExpectOutput")
@@ -89,8 +83,7 @@ public final class ErrorProneTestHelperSourceFormat extends BugChecker
}
List<? extends ExpressionTree> sourceLines =
tree.getArguments()
.subList(ASTHelpers.getSymbol(tree).params().size() - 1, tree.getArguments().size());
tree.getArguments().subList(1, tree.getArguments().size());
if (sourceLines.isEmpty()) {
return buildDescription(tree).setMessage("No source code provided").build();
}
@@ -111,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();
@@ -140,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(", "))));
@@ -156,18 +147,17 @@ public final class ErrorProneTestHelperSourceFormat extends BugChecker
return FORMATTER.formatSource(withOptionallyRemovedImports);
}
// XXX: This logic is duplicated in `BugPatternTestExtractor`. Can we do better?
private static Optional<String> getConstantSourceCode(
List<? extends ExpressionTree> sourceLines) {
StringBuilder source = new StringBuilder();
for (ExpressionTree sourceLine : sourceLines) {
String value = ASTHelpers.constValue(sourceLine, String.class);
Object value = ASTHelpers.constValue(sourceLine);
if (value == null) {
return Optional.empty();
}
source.append(value).append(System.lineSeparator());
source.append(value).append('\n');
}
return Optional.of(source.toString());

View File

@@ -44,7 +44,7 @@ import java.util.stream.Stream;
public final class ExplicitEnumOrdering extends BugChecker implements MethodInvocationTreeMatcher {
private static final long serialVersionUID = 1L;
private static final Matcher<ExpressionTree> EXPLICIT_ORDERING =
staticMethod().onClass(Ordering.class.getCanonicalName()).named("explicit");
staticMethod().onClass(Ordering.class.getName()).named("explicit");
/** Instantiates a new {@link ExplicitEnumOrdering} instance. */
public ExplicitEnumOrdering() {}

View File

@@ -66,7 +66,7 @@ public final class FluxFlatMapUsage extends BugChecker
instanceMethod()
.onDescendantOf(FLUX)
.namedAnyOf("flatMap", "flatMapSequential")
.withParameters(Function.class.getCanonicalName());
.withParameters(Function.class.getName());
private static final Supplier<Type> FLUX_OF_PUBLISHERS =
VisitorState.memoize(
generic(FLUX, subOf(generic(type("org.reactivestreams.Publisher"), unbound()))));
@@ -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,104 +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.common.collect.ImmutableList;
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.Collectors;
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.getCanonicalName());
/** 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, ImmutableList.class.getCanonicalName() + ".toImmutableList", state));
}
description.addFix(
suggestBlockingElementCollection(
tree, Collectors.class.getCanonicalName() + ".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

@@ -13,8 +13,6 @@ import static java.util.stream.Collectors.joining;
import static tech.picnic.errorprone.bugpatterns.util.Documentation.BUG_PATTERNS_BASE_URL;
import com.google.auto.service.AutoService;
import com.google.common.base.Preconditions;
import com.google.common.base.Verify;
import com.google.errorprone.BugPattern;
import com.google.errorprone.VisitorState;
import com.google.errorprone.bugpatterns.BugChecker;
@@ -31,12 +29,10 @@ import com.sun.source.tree.ParenthesizedTree;
import com.sun.source.tree.Tree;
import com.sun.source.tree.Tree.Kind;
import com.sun.source.util.SimpleTreeVisitor;
import com.sun.tools.javac.code.Type;
import java.util.ArrayList;
import java.util.Formatter;
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;
/**
@@ -63,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.
@@ -72,8 +67,7 @@ public final class FormatStringConcatenation extends BugChecker
anyMethod()
.anyClass()
.withAnyName()
.withParameters(String.class.getCanonicalName(), Throwable.class.getCanonicalName());
.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(
@@ -122,14 +116,14 @@ public final class FormatStringConcatenation extends BugChecker
private static final Matcher<ExpressionTree> GUAVA_FORMAT_METHOD =
anyOf(
staticMethod()
.onClass(Preconditions.class.getCanonicalName())
.onClass("com.google.common.base.Preconditions")
.namedAnyOf("checkArgument", "checkNotNull", "checkState"),
staticMethod().onClass(Verify.class.getCanonicalName()).named("verify"));
staticMethod().onClass("com.google.common.base.Verify").named("verify"));
// XXX: Add `PrintWriter`, maybe others.
private static final Matcher<ExpressionTree> JDK_FORMAT_METHOD =
anyOf(
staticMethod().onClass(String.class.getCanonicalName()).named("format"),
instanceMethod().onExactClass(Formatter.class.getCanonicalName()).named("format"));
staticMethod().onClass("java.lang.String").named("format"),
instanceMethod().onExactClass("java.util.Formatter").named("format"));
private static final Matcher<ExpressionTree> SLF4J_FORMAT_METHOD =
instanceMethod()
.onDescendantOf("org.slf4j.Logger")
@@ -209,9 +203,7 @@ public final class FormatStringConcatenation extends BugChecker
}
private static boolean isStringTyped(ExpressionTree tree, VisitorState state) {
// XXX: Open Error Prone PR to improve the `@Nullable` annotations on `ASTHelpers`.
Type type = ASTHelpers.getType(tree);
return type != null && ASTHelpers.isSameType(type, state.getSymtab().stringType, state);
return ASTHelpers.isSameType(ASTHelpers.getType(tree), state.getSymtab().stringType, state);
}
private static class ReplacementArgumentsConstructor

View File

@@ -10,17 +10,6 @@ import static com.google.errorprone.suppliers.Suppliers.OBJECT_TYPE;
import static tech.picnic.errorprone.bugpatterns.util.Documentation.BUG_PATTERNS_BASE_URL;
import com.google.auto.service.AutoService;
import com.google.common.collect.ImmutableBiMap;
import com.google.common.collect.ImmutableList;
import com.google.common.collect.ImmutableListMultimap;
import com.google.common.collect.ImmutableMap;
import com.google.common.collect.ImmutableMultimap;
import com.google.common.collect.ImmutableMultiset;
import com.google.common.collect.ImmutableRangeMap;
import com.google.common.collect.ImmutableRangeSet;
import com.google.common.collect.ImmutableSet;
import com.google.common.collect.ImmutableSetMultimap;
import com.google.common.collect.ImmutableTable;
import com.google.common.primitives.Primitives;
import com.google.errorprone.BugPattern;
import com.google.errorprone.VisitorState;
@@ -31,11 +20,9 @@ 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.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;
@@ -45,12 +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.
// XXX: Also flag nullary instance method invocations that represent an identity conversion, such as
// `Boolean#booleanValue()`, `Byte#byteValue()` and friends.
@AutoService(BugChecker.class)
@BugPattern(
summary = "Avoid or clarify identity conversions",
@@ -62,28 +45,27 @@ 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(
"com.google.common.collect.ImmutableBiMap",
"com.google.common.collect.ImmutableList",
"com.google.common.collect.ImmutableListMultimap",
"com.google.common.collect.ImmutableMap",
"com.google.common.collect.ImmutableMultimap",
"com.google.common.collect.ImmutableMultiset",
"com.google.common.collect.ImmutableRangeMap",
"com.google.common.collect.ImmutableRangeSet",
"com.google.common.collect.ImmutableSet",
"com.google.common.collect.ImmutableSetMultimap",
"com.google.common.collect.ImmutableTable")
.named("copyOf"),
staticMethod()
.onClassAny(
Primitives.allWrapperTypes().stream()
.map(Class::getName)
.collect(toImmutableSet()))
.named("valueOf"),
staticMethod().onClass(String.class.getCanonicalName()).named("valueOf"),
staticMethod()
.onClassAny(
ImmutableBiMap.class.getCanonicalName(),
ImmutableList.class.getCanonicalName(),
ImmutableListMultimap.class.getCanonicalName(),
ImmutableMap.class.getCanonicalName(),
ImmutableMultimap.class.getCanonicalName(),
ImmutableMultiset.class.getCanonicalName(),
ImmutableRangeMap.class.getCanonicalName(),
ImmutableRangeSet.class.getCanonicalName(),
ImmutableSet.class.getCanonicalName(),
ImmutableSetMultimap.class.getCanonicalName(),
ImmutableTable.class.getCanonicalName())
.named("copyOf"),
staticMethod().onClass(Matchers.class.getCanonicalName()).namedAnyOf("allOf", "anyOf"),
staticMethod().onClass(String.class.getName()).named("valueOf"),
staticMethod().onClass("reactor.adapter.rxjava.RxJava2Adapter"),
staticMethod()
.onClass("reactor.core.publisher.Flux")
@@ -113,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,91 +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.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 static tech.picnic.errorprone.bugpatterns.util.MoreMatchers.hasMetaAnnotation;
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.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.matchers.MultiMatcher.MultiMatchResult;
import com.sun.source.tree.AnnotationTree;
import com.sun.source.tree.MethodTree;
import tech.picnic.errorprone.bugpatterns.util.SourceCode;
/**
* A {@link BugChecker} that flags nullary {@link
* org.junit.jupiter.params.ParameterizedTest @ParameterizedTest} test methods.
*
* <p>Such tests are unnecessarily executed more than necessary. This checker suggests annotating
* the method with {@link org.junit.jupiter.api.Test @Test}, and to drop all declared {@link
* org.junit.jupiter.params.provider.ArgumentsSource argument sources}.
*/
@AutoService(BugChecker.class)
@BugPattern(
summary = "Nullary JUnit test methods should not be parameterized",
link = BUG_PATTERNS_BASE_URL + "JUnitNullaryParameterizedTestDeclaration",
linkType = CUSTOM,
severity = SUGGESTION,
tags = SIMPLIFICATION)
public final class JUnitNullaryParameterizedTestDeclaration extends BugChecker
implements MethodTreeMatcher {
private static final long serialVersionUID = 1L;
private static final MultiMatcher<MethodTree, AnnotationTree> IS_PARAMETERIZED_TEST =
annotations(AT_LEAST_ONE, isType("org.junit.jupiter.params.ParameterizedTest"));
private static final Matcher<AnnotationTree> IS_ARGUMENT_SOURCE =
anyOf(
isType("org.junit.jupiter.params.provider.ArgumentsSource"),
isType("org.junit.jupiter.params.provider.ArgumentsSources"),
hasMetaAnnotation("org.junit.jupiter.params.provider.ArgumentsSource"));
/** Instantiates a new {@link JUnitNullaryParameterizedTestDeclaration} instance. */
public JUnitNullaryParameterizedTestDeclaration() {}
@Override
public Description matchMethod(MethodTree tree, VisitorState state) {
if (!tree.getParameters().isEmpty()) {
return Description.NO_MATCH;
}
MultiMatchResult<AnnotationTree> isParameterizedTest =
IS_PARAMETERIZED_TEST.multiMatchResult(tree, state);
if (!isParameterizedTest.matches()) {
return Description.NO_MATCH;
}
/*
* This method is vacuously parameterized. Suggest replacing `@ParameterizedTest` with `@Test`.
* (As each method is checked independently, we cannot in general determine whether this
* suggestion makes a `ParameterizedTest` type import obsolete; that task is left to Error
* Prone's `RemoveUnusedImports` check.)
*/
SuggestedFix.Builder fix = SuggestedFix.builder();
fix.merge(
SuggestedFix.replace(
isParameterizedTest.onlyMatchingNode(),
'@' + SuggestedFixes.qualifyType(state, fix, "org.junit.jupiter.api.Test")));
/*
* Also suggest dropping all (explicit and implicit) `@ArgumentsSource`s. No attempt is made to
* assess whether a dropped `@MethodSource` also makes the referenced factory method(s) unused.
*/
tree.getModifiers().getAnnotations().stream()
.filter(a -> IS_ARGUMENT_SOURCE.matches(a, state))
.forEach(a -> fix.merge(SourceCode.deleteWithTrailingWhitespace(a, state)));
return describeMatch(tree, fix.build());
}
}

View File

@@ -1,318 +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.ImmutableList;
import com.google.common.collect.ImmutableSet;
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.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.getCanonicalName(),
IntStream.class.getCanonicalName(),
LongStream.class.getCanonicalName(),
DoubleStream.class.getCanonicalName(),
List.class.getCanonicalName(),
Set.class.getCanonicalName(),
ImmutableList.class.getCanonicalName(),
ImmutableSet.class.getCanonicalName())
.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);
}
// XXX: File ticket for the `@SuppressWarnings`: the checker incorrectly claims that the predicate
// only accepts `@InternedDistinct @SignatureBottom MethodTree` values (IIUC).
@SuppressWarnings({"interning:argument", "signature:argument"})
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 fix = SuggestedFix.builder();
String valueSource =
SuggestedFixes.qualifyType(
state, fix, "org.junit.jupiter.params.provider.ValueSource");
return fix.replace(
methodSourceAnnotation,
String.format(
"@%s(%s = %s)",
valueSource,
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

@@ -34,7 +34,6 @@ import com.sun.tools.javac.code.Type;
import java.util.List;
import java.util.Optional;
import javax.lang.model.element.Name;
import org.checkerframework.checker.nullness.qual.NonNull;
/**
* A {@link BugChecker} that flags lambda expressions that can be replaced with method references.
@@ -119,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,
@@ -133,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:
@@ -190,20 +188,13 @@ public final class MethodReferenceUsage extends BugChecker implements LambdaExpr
}
}
// XXX: These type hints shouldn't be necessary; see
// https://github.com/typetools/checker-framework/issues/4007.
return Optional.of(
diff == 0
? Optional.<@NonNull Name>empty()
: Optional.<@NonNull Name>of(expectedArguments.get(0)));
return Optional.of(diff == 0 ? Optional.empty() : Optional.of(expectedArguments.get(0)));
}
private static ImmutableList<Name> getVariables(LambdaExpressionTree tree) {
return tree.getParameters().stream().map(VariableTree::getName).collect(toImmutableList());
}
// XXX: Resolve this suppression.
@SuppressWarnings("UnqualifiedSuggestedFixImport")
private static Optional<SuggestedFix.Builder> constructFix(
LambdaExpressionTree lambdaExpr, Symbol target, Object methodName) {
Name sName = target.getSimpleName();

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.getCanonicalName()), 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.hasImplicitType((VariableTree) parent, state)
&& MoreASTHelpers.areSameType(tree, parent, state);
case ASSIGNMENT:
return MoreASTHelpers.areSameType(tree, parent, state);
case RETURN:
return MoreASTHelpers.findMethodExitedOnReturn(state)
.filter(m -> MoreASTHelpers.areSameType(tree, m.getReturnType(), state))
.isPresent();
default:
return false;
}
}
}

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 =
@@ -72,7 +71,7 @@ public final class NonEmptyMono extends BugChecker implements MethodInvocationTr
instanceMethod()
.onDescendantOf("reactor.core.publisher.Flux")
.named("reduce")
.withParameters(Object.class.getCanonicalName(), BiFunction.class.getCanonicalName()),
.withParameters(Object.class.getName(), BiFunction.class.getName()),
instanceMethod()
.onDescendantOf("reactor.core.publisher.Mono")
.namedAnyOf("defaultIfEmpty", "hasElement", "single"));

View File

@@ -1,224 +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 tech.picnic.errorprone.bugpatterns.StaticImport.STATIC_IMPORT_CANDIDATE_MEMBERS;
import static tech.picnic.errorprone.bugpatterns.util.Documentation.BUG_PATTERNS_BASE_URL;
import com.google.auto.service.AutoService;
import com.google.auto.value.AutoValue;
import com.google.common.annotations.VisibleForTesting;
import com.google.common.base.Predicates;
import com.google.common.base.Strings;
import com.google.common.collect.ImmutableSet;
import com.google.common.collect.ImmutableSetMultimap;
import com.google.common.collect.ImmutableTable;
import com.google.errorprone.BugPattern;
import com.google.errorprone.VisitorState;
import com.google.errorprone.bugpatterns.BugChecker;
import com.google.errorprone.bugpatterns.BugChecker.CompilationUnitTreeMatcher;
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.CompilationUnitTree;
import com.sun.source.tree.IdentifierTree;
import com.sun.source.tree.ImportTree;
import com.sun.source.tree.MemberSelectTree;
import com.sun.source.tree.Tree;
import com.sun.source.util.TreeScanner;
import com.sun.tools.javac.code.Symbol;
import java.time.Clock;
import java.time.ZoneOffset;
import java.util.Collections;
import java.util.Locale;
import java.util.regex.Pattern;
import org.jspecify.annotations.Nullable;
import tech.picnic.errorprone.bugpatterns.util.SourceCode;
/**
* A {@link BugChecker} that flags static imports of type members that should *not* be statically
* imported.
*/
// XXX: This check is closely linked to `StaticImport`. Consider merging the two.
// XXX: Add suppression support. If qualification of one more more identifiers is suppressed, then
// the associated static import should *not* be removed.
// XXX: Also introduce logic that disallows statically importing `ZoneOffset.ofHours` and other
// `ofXXX`-style methods.
@AutoService(BugChecker.class)
@BugPattern(
summary = "Member should not be statically imported",
link = BUG_PATTERNS_BASE_URL + "NonStaticImport",
linkType = CUSTOM,
severity = SUGGESTION,
tags = STYLE)
public final class NonStaticImport extends BugChecker implements CompilationUnitTreeMatcher {
private static final long serialVersionUID = 1L;
/**
* Types whose members should not be statically imported, unless exempted by {@link
* StaticImport#STATIC_IMPORT_CANDIDATE_MEMBERS}.
*
* <p>Types listed here should be mutually exclusive with {@link
* StaticImport#STATIC_IMPORT_CANDIDATE_TYPES}.
*/
@VisibleForTesting
static final ImmutableSet<String> NON_STATIC_IMPORT_CANDIDATE_TYPES =
ImmutableSet.of(
ASTHelpers.class.getCanonicalName(),
Clock.class.getCanonicalName(),
Strings.class.getCanonicalName(),
VisitorState.class.getCanonicalName(),
ZoneOffset.class.getCanonicalName(),
"com.google.errorprone.BugCheckerRefactoringTestHelper.TestMode");
/**
* Type members that should never be statically imported.
*
* <p>Please note that:
*
* <ul>
* <li>Types listed by {@link #NON_STATIC_IMPORT_CANDIDATE_TYPES} and members listed by {@link
* #NON_STATIC_IMPORT_CANDIDATE_IDENTIFIERS} should be omitted from this collection.
* <li>This collection should be mutually exclusive with {@link
* StaticImport#STATIC_IMPORT_CANDIDATE_MEMBERS}.
* </ul>
*/
// XXX: Perhaps the set of exempted `java.util.Collections` methods is too strict. For now any
// method name that could be considered "too vague" or could conceivably mean something else in a
// specific context is left out.
static final ImmutableSetMultimap<String, String> NON_STATIC_IMPORT_CANDIDATE_MEMBERS =
ImmutableSetMultimap.<String, String>builder()
.putAll(
Collections.class.getCanonicalName(),
"addAll",
"copy",
"fill",
"list",
"max",
"min",
"nCopies",
"rotate",
"sort",
"swap")
.put(Locale.class.getCanonicalName(), "ROOT")
.putAll(Pattern.class.getCanonicalName(), "compile", "matches", "quote")
.put(Predicates.class.getCanonicalName(), "contains")
.put("org.springframework.http.MediaType", "ALL")
.build();
/**
* Identifiers that should never be statically imported.
*
* <p>Please note that:
*
* <ul>
* <li>Identifiers listed by {@link StaticImport#STATIC_IMPORT_CANDIDATE_MEMBERS} should be
* mutually exclusive with identifiers listed here.
* <li>This list should contain a superset of the identifiers flagged by {@link
* com.google.errorprone.bugpatterns.BadImport}.
* </ul>
*/
static final ImmutableSet<String> NON_STATIC_IMPORT_CANDIDATE_IDENTIFIERS =
ImmutableSet.of(
"builder",
"copyOf",
"create",
"empty",
"from",
"getDefaultInstance",
"INSTANCE",
"MAX",
"MAX_VALUE",
"MIN",
"MIN_VALUE",
"newBuilder",
"newInstance",
"of",
"ONE",
"parse",
"valueOf",
"ZERO");
/** Instantiates a new {@link NonStaticImport} instance. */
public NonStaticImport() {}
@Override
public Description matchCompilationUnit(CompilationUnitTree tree, VisitorState state) {
ImmutableTable<String, String, UndesiredStaticImport> undesiredStaticImports =
getUndesiredStaticImports(tree, state);
if (!undesiredStaticImports.isEmpty()) {
replaceUndesiredStaticImportUsages(tree, undesiredStaticImports, state);
for (UndesiredStaticImport staticImport : undesiredStaticImports.values()) {
state.reportMatch(
describeMatch(staticImport.importTree(), staticImport.fixBuilder().build()));
}
}
/* Any violations have been flagged against the offending static import statement. */
return Description.NO_MATCH;
}
private static ImmutableTable<String, String, UndesiredStaticImport> getUndesiredStaticImports(
CompilationUnitTree tree, VisitorState state) {
ImmutableTable.Builder<String, String, UndesiredStaticImport> imports =
ImmutableTable.builder();
for (ImportTree importTree : tree.getImports()) {
Tree qualifiedIdentifier = importTree.getQualifiedIdentifier();
if (importTree.isStatic() && qualifiedIdentifier instanceof MemberSelectTree) {
MemberSelectTree memberSelectTree = (MemberSelectTree) qualifiedIdentifier;
String type = SourceCode.treeToString(memberSelectTree.getExpression(), state);
String member = memberSelectTree.getIdentifier().toString();
if (shouldNotBeStaticallyImported(type, member)) {
imports.put(
type,
member,
new AutoValue_NonStaticImport_UndesiredStaticImport(
importTree, SuggestedFix.builder().removeStaticImport(type + '.' + member)));
}
}
}
return imports.build();
}
private static boolean shouldNotBeStaticallyImported(String type, String member) {
return (NON_STATIC_IMPORT_CANDIDATE_TYPES.contains(type)
&& !STATIC_IMPORT_CANDIDATE_MEMBERS.containsEntry(type, member))
|| NON_STATIC_IMPORT_CANDIDATE_MEMBERS.containsEntry(type, member)
|| NON_STATIC_IMPORT_CANDIDATE_IDENTIFIERS.contains(member);
}
private static void replaceUndesiredStaticImportUsages(
CompilationUnitTree tree,
ImmutableTable<String, String, UndesiredStaticImport> undesiredStaticImports,
VisitorState state) {
new TreeScanner<@Nullable Void, @Nullable Void>() {
@Override
public @Nullable Void visitIdentifier(IdentifierTree node, @Nullable Void unused) {
Symbol symbol = ASTHelpers.getSymbol(node);
if (symbol != null) {
UndesiredStaticImport staticImport =
undesiredStaticImports.get(
symbol.owner.getQualifiedName().toString(), symbol.name.toString());
if (staticImport != null) {
SuggestedFix.Builder fix = staticImport.fixBuilder();
fix.prefixWith(node, SuggestedFixes.qualifyType(state, fix, symbol.owner) + '.');
}
}
return super.visitIdentifier(node, unused);
}
}.scan(tree, null);
}
@AutoValue
abstract static class UndesiredStaticImport {
abstract ImportTree importTree();
abstract SuggestedFix.Builder fixBuilder();
}
}

View File

@@ -17,7 +17,6 @@ import com.google.errorprone.bugpatterns.BugChecker;
import com.google.errorprone.bugpatterns.BugChecker.MethodInvocationTreeMatcher;
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.matchers.Matcher;
import com.google.errorprone.util.ASTHelpers;
@@ -50,27 +49,26 @@ 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 =
anyOf(
staticMethod()
.onClass(Comparator.class.getCanonicalName())
.onClass(Comparator.class.getName())
.namedAnyOf("comparingInt", "comparingLong", "comparingDouble"),
staticMethod()
.onClass(Comparator.class.getCanonicalName())
.onClass(Comparator.class.getName())
.named("comparing")
.withParameters(Function.class.getCanonicalName()));
.withParameters(Function.class.getName()));
private static final Matcher<ExpressionTree> INSTANCE_COMPARISON_METHOD =
anyOf(
instanceMethod()
.onDescendantOf(Comparator.class.getCanonicalName())
.onDescendantOf(Comparator.class.getName())
.namedAnyOf("thenComparingInt", "thenComparingLong", "thenComparingDouble"),
instanceMethod()
.onDescendantOf(Comparator.class.getCanonicalName())
.onDescendantOf(Comparator.class.getName())
.named("thenComparing")
.withParameters(Function.class.getCanonicalName()));
.withParameters(Function.class.getName()));
/** Instantiates a new {@link PrimitiveComparison} instance. */
public PrimitiveComparison() {}
@@ -168,11 +166,10 @@ public final class PrimitiveComparison extends BugChecker implements MethodInvoc
ExpressionTree expr = tree.getMethodSelect();
switch (expr.getKind()) {
case IDENTIFIER:
SuggestedFix.Builder fix = SuggestedFix.builder();
String replacement =
SuggestedFixes.qualifyStaticImport(
Comparator.class.getCanonicalName() + '.' + preferredMethodName, fix, state);
return fix.replace(expr, replacement).build();
return SuggestedFix.builder()
.addStaticImport(Comparator.class.getName() + '.' + preferredMethodName)
.replace(expr, preferredMethodName)
.build();
case MEMBER_SELECT:
MemberSelectTree ms = (MemberSelectTree) tree.getMethodSelect();
return SuggestedFix.replace(

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;
@@ -18,8 +17,6 @@ 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.common.base.Preconditions;
import com.google.common.base.Verify;
import com.google.common.collect.ImmutableList;
import com.google.common.collect.Iterables;
import com.google.common.primitives.Primitives;
@@ -52,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;
@@ -65,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);
@@ -88,7 +81,7 @@ public final class RedundantStringConversion extends BugChecker
private static final Matcher<MethodInvocationTree> WELL_KNOWN_STRING_CONVERSION_METHODS =
anyOf(
instanceMethod()
.onDescendantOfAny(Object.class.getCanonicalName())
.onDescendantOfAny(Object.class.getName())
.named("toString")
.withNoParameters(),
allOf(
@@ -102,7 +95,7 @@ public final class RedundantStringConversion extends BugChecker
.collect(toImmutableSet()))
.named("toString"),
allOf(
staticMethod().onClass(String.class.getCanonicalName()).named("valueOf"),
staticMethod().onClass(String.class.getName()).named("valueOf"),
not(
anyMethod()
.anyClass()
@@ -111,37 +104,35 @@ public final class RedundantStringConversion extends BugChecker
ImmutableList.of(Suppliers.arrayOf(Suppliers.CHAR_TYPE))))))));
private static final Matcher<ExpressionTree> STRINGBUILDER_APPEND_INVOCATION =
instanceMethod()
.onDescendantOf(StringBuilder.class.getCanonicalName())
.onDescendantOf(StringBuilder.class.getName())
.named("append")
.withParameters(String.class.getCanonicalName());
.withParameters(String.class.getName());
private static final Matcher<ExpressionTree> STRINGBUILDER_INSERT_INVOCATION =
instanceMethod()
.onDescendantOf(StringBuilder.class.getCanonicalName())
.onDescendantOf(StringBuilder.class.getName())
.named("insert")
.withParameters(int.class.getCanonicalName(), String.class.getCanonicalName());
.withParameters(int.class.getName(), String.class.getName());
private static final Matcher<ExpressionTree> FORMATTER_INVOCATION =
anyOf(
staticMethod().onClass(String.class.getCanonicalName()).named("format"),
instanceMethod().onDescendantOf(Formatter.class.getCanonicalName()).named("format"),
staticMethod().onClass(String.class.getName()).named("format"),
instanceMethod().onDescendantOf(Formatter.class.getName()).named("format"),
instanceMethod()
.onDescendantOfAny(
PrintStream.class.getCanonicalName(), PrintWriter.class.getCanonicalName())
.onDescendantOfAny(PrintStream.class.getName(), PrintWriter.class.getName())
.namedAnyOf("format", "printf"),
instanceMethod()
.onDescendantOfAny(
PrintStream.class.getCanonicalName(), PrintWriter.class.getCanonicalName())
.onDescendantOfAny(PrintStream.class.getName(), PrintWriter.class.getName())
.namedAnyOf("print", "println")
.withParameters(Object.class.getCanonicalName()),
.withParameters(Object.class.getName()),
staticMethod()
.onClass(Console.class.getCanonicalName())
.onClass(Console.class.getName())
.namedAnyOf("format", "printf", "readline", "readPassword"));
private static final Matcher<ExpressionTree> GUAVA_GUARD_INVOCATION =
anyOf(
staticMethod()
.onClass(Preconditions.class.getCanonicalName())
.onClass("com.google.common.base.Preconditions")
.namedAnyOf("checkArgument", "checkState", "checkNotNull"),
staticMethod()
.onClass(Verify.class.getCanonicalName())
.onClass("com.google.common.base.Verify")
.namedAnyOf("verify", "verifyNotNull"));
private static final Matcher<ExpressionTree> SLF4J_LOGGER_INVOCATION =
instanceMethod()
@@ -160,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);
}
@@ -174,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<>();
@@ -188,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
@@ -197,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(
@@ -309,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);
@@ -373,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))
@@ -384,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

@@ -35,7 +35,7 @@ import tech.picnic.errorprone.bugpatterns.util.SourceCode;
public final class RefasterAnyOfUsage extends BugChecker implements MethodInvocationTreeMatcher {
private static final long serialVersionUID = 1L;
private static final Matcher<ExpressionTree> REFASTER_ANY_OF =
staticMethod().onClass(Refaster.class.getCanonicalName()).named("anyOf");
staticMethod().onClass(Refaster.class.getName()).named("anyOf");
/** Instantiates a new {@link RefasterAnyOfUsage} instance. */
public RefasterAnyOfUsage() {}

View File

@@ -22,10 +22,6 @@ import com.google.errorprone.matchers.Description;
import com.google.errorprone.matchers.Matcher;
import com.sun.source.tree.MethodTree;
import com.sun.source.tree.Tree;
import java.io.InputStream;
import java.time.ZoneId;
import java.util.Locale;
import java.util.TimeZone;
/**
* A {@link BugChecker} that flags {@code @RequestMapping} methods that have one or more parameters
@@ -74,17 +70,13 @@ public final class RequestMappingAnnotation extends BugChecker implements Method
isType(ANN_PACKAGE_PREFIX + "RequestHeader"),
isType(ANN_PACKAGE_PREFIX + "RequestParam"),
isType(ANN_PACKAGE_PREFIX + "RequestPart"))),
isSameType(InputStream.class.getCanonicalName()),
isSameType(Locale.class.getCanonicalName()),
isSameType(TimeZone.class.getCanonicalName()),
isSameType(ZoneId.class.getCanonicalName()),
isSameType("jakarta.servlet.http.HttpServletRequest"),
isSameType("jakarta.servlet.http.HttpServletResponse"),
isSameType("java.io.InputStream"),
isSameType("java.time.ZoneId"),
isSameType("java.util.Locale"),
isSameType("java.util.TimeZone"),
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"),
@@ -103,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

@@ -18,7 +18,6 @@ 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.sun.source.tree.AnnotationTree;
import com.sun.source.tree.AssignmentTree;
@@ -115,8 +114,9 @@ public final class SpringMvcAnnotation extends BugChecker implements AnnotationT
.map(arg -> SourceCode.treeToString(arg, state))
.collect(joining(", "));
SuggestedFix.Builder fix = SuggestedFix.builder();
String annotation = SuggestedFixes.qualifyType(state, fix, ANN_PACKAGE_PREFIX + newAnnotation);
return fix.replace(tree, String.format("@%s(%s)", annotation, newArguments)).build();
return SuggestedFix.builder()
.addImport(ANN_PACKAGE_PREFIX + newAnnotation)
.replace(tree, String.format("@%s(%s)", newAnnotation, newArguments))
.build();
}
}

View File

@@ -2,32 +2,14 @@ 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.BugPattern.StandardTags.SIMPLIFICATION;
import static java.util.Objects.requireNonNull;
import static tech.picnic.errorprone.bugpatterns.NonStaticImport.NON_STATIC_IMPORT_CANDIDATE_IDENTIFIERS;
import static tech.picnic.errorprone.bugpatterns.NonStaticImport.NON_STATIC_IMPORT_CANDIDATE_MEMBERS;
import static tech.picnic.errorprone.bugpatterns.util.Documentation.BUG_PATTERNS_BASE_URL;
import com.google.auto.service.AutoService;
import com.google.common.annotations.VisibleForTesting;
import com.google.common.base.Functions;
import com.google.common.base.Preconditions;
import com.google.common.base.Predicates;
import com.google.common.base.Verify;
import com.google.common.collect.Comparators;
import com.google.common.collect.ImmutableList;
import com.google.common.collect.ImmutableListMultimap;
import com.google.common.collect.ImmutableMap;
import com.google.common.collect.ImmutableMultiset;
import com.google.common.collect.ImmutableRangeSet;
import com.google.common.collect.ImmutableSet;
import com.google.common.collect.ImmutableSetMultimap;
import com.google.common.collect.ImmutableSortedMap;
import com.google.common.collect.ImmutableSortedMultiset;
import com.google.common.collect.ImmutableSortedSet;
import com.google.common.collect.ImmutableTable;
import com.google.common.collect.MoreCollectors;
import com.google.common.collect.Sets;
import com.google.errorprone.BugPattern;
import com.google.errorprone.VisitorState;
import com.google.errorprone.bugpatterns.BugChecker;
@@ -38,73 +20,55 @@ 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.matchers.Matchers;
import com.google.errorprone.refaster.ImportPolicy;
import com.google.errorprone.util.ASTHelpers;
import com.sun.source.tree.MemberSelectTree;
import com.sun.source.tree.MethodInvocationTree;
import com.sun.source.tree.Tree;
import com.sun.tools.javac.code.Type;
import java.nio.charset.StandardCharsets;
import java.time.ZoneOffset;
import java.util.Collections;
import java.util.Comparator;
import java.util.Map;
import java.util.Objects;
import java.util.Optional;
import java.util.UUID;
import java.util.function.Function;
import java.util.function.Predicate;
import java.util.regex.Pattern;
import java.util.stream.Collectors;
/** A {@link BugChecker} that flags type members that can and should be statically imported. */
// XXX: This check is closely linked to `NonStaticImport`. Consider merging the two.
/**
* A {@link BugChecker} that flags methods and constants that can and should be statically imported.
*/
// XXX: Tricky cases:
// - `org.springframework.http.HttpStatus` (not always an improvement, and `valueOf` must
// certainly be excluded)
// - `com.google.common.collect.Tables`
// - `ch.qos.logback.classic.Level.{DEBUG, ERROR, INFO, TRACE, WARN}`
// - `ch.qos.logback.classic.Level.{DEBUG, ERROR, INFO, TRACE, WARN"}`
// XXX: Also introduce a check that disallows static imports of certain methods. Candidates:
// - `com.google.common.base.Strings`
// - `java.util.Optional.empty`
// - `java.util.Locale.ROOT`
// - `ZoneOffset.ofHours` and other `ofXXX`-style methods.
// - `java.time.Clock`.
// - Several other `java.time` classes.
// - Likely any of `*.{ZERO, ONE, MIX, MAX, MIN_VALUE, MAX_VALUE}`.
@AutoService(BugChecker.class)
@BugPattern(
summary = "Identifier should be statically imported",
link = BUG_PATTERNS_BASE_URL + "StaticImport",
linkType = CUSTOM,
severity = SUGGESTION,
tags = STYLE)
tags = SIMPLIFICATION)
public final class StaticImport extends BugChecker implements MemberSelectTreeMatcher {
private static final long serialVersionUID = 1L;
/**
* Types whose members should be statically imported, unless exempted by {@link
* NonStaticImport#NON_STATIC_IMPORT_CANDIDATE_MEMBERS} or {@link
* NonStaticImport#NON_STATIC_IMPORT_CANDIDATE_IDENTIFIERS}.
*
* <p>Types listed here should be mutually exclusive with {@link
* NonStaticImport#NON_STATIC_IMPORT_CANDIDATE_TYPES}.
* #STATIC_IMPORT_EXEMPTED_MEMBERS} or {@link #STATIC_IMPORT_EXEMPTED_IDENTIFIERS}.
*/
@VisibleForTesting
static final ImmutableSet<String> STATIC_IMPORT_CANDIDATE_TYPES =
ImmutableSet.of(
BugPattern.LinkType.class.getCanonicalName(),
BugPattern.SeverityLevel.class.getCanonicalName(),
BugPattern.StandardTags.class.getCanonicalName(),
Collections.class.getCanonicalName(),
Collectors.class.getCanonicalName(),
Comparator.class.getCanonicalName(),
ImportPolicy.class.getCanonicalName(),
Map.Entry.class.getCanonicalName(),
Matchers.class.getCanonicalName(),
MoreCollectors.class.getCanonicalName(),
Pattern.class.getCanonicalName(),
Preconditions.class.getCanonicalName(),
Predicates.class.getCanonicalName(),
StandardCharsets.class.getCanonicalName(),
Verify.class.getCanonicalName(),
"com.fasterxml.jackson.annotation.JsonCreator.Mode",
"com.fasterxml.jackson.annotation.JsonFormat.Shape",
"com.fasterxml.jackson.annotation.JsonInclude.Include",
"com.fasterxml.jackson.annotation.JsonProperty.Access",
"com.google.common.base.Preconditions",
"com.google.common.base.Predicates",
"com.google.common.base.Verify",
"com.google.common.collect.MoreCollectors",
"com.google.errorprone.BugPattern.LinkType",
"com.google.errorprone.BugPattern.SeverityLevel",
"com.google.errorprone.BugPattern.StandardTags",
"com.google.errorprone.matchers.Matchers",
"com.google.errorprone.refaster.ImportPolicy",
"com.mongodb.client.model.Accumulators",
"com.mongodb.client.model.Aggregates",
"com.mongodb.client.model.Filters",
@@ -112,6 +76,12 @@ public final class StaticImport extends BugChecker implements MemberSelectTreeMa
"com.mongodb.client.model.Projections",
"com.mongodb.client.model.Sorts",
"com.mongodb.client.model.Updates",
"java.nio.charset.StandardCharsets",
"java.util.Collections",
"java.util.Comparator",
"java.util.Map.Entry",
"java.util.regex.Pattern",
"java.util.stream.Collectors",
"org.assertj.core.api.Assertions",
"org.assertj.core.api.InstanceOfAssertFactories",
"org.assertj.core.api.SoftAssertions",
@@ -134,57 +104,93 @@ public final class StaticImport extends BugChecker implements MemberSelectTreeMa
"reactor.function.TupleUtils",
"tech.picnic.errorprone.bugpatterns.util.MoreTypes");
/**
* Type members that should be statically imported.
*
* <p>Please note that:
*
* <ul>
* <li>Types listed by {@link #STATIC_IMPORT_CANDIDATE_TYPES} should be omitted from this
* collection.
* <li>This collection should be mutually exclusive with {@link
* NonStaticImport#NON_STATIC_IMPORT_CANDIDATE_MEMBERS}.
* <li>This collection should not list members contained in {@link
* NonStaticImport#NON_STATIC_IMPORT_CANDIDATE_IDENTIFIERS}.
* </ul>
*/
/** Type members that should be statically imported. */
@VisibleForTesting
static final ImmutableSetMultimap<String, String> STATIC_IMPORT_CANDIDATE_MEMBERS =
ImmutableSetMultimap.<String, String>builder()
.putAll(Comparators.class.getCanonicalName(), "emptiesFirst", "emptiesLast")
.put(Function.class.getCanonicalName(), "identity")
.put(Functions.class.getCanonicalName(), "identity")
.put(ImmutableList.class.getCanonicalName(), "toImmutableList")
.putAll(
ImmutableListMultimap.class.getCanonicalName(),
"com.google.common.collect.ImmutableListMultimap",
"flatteningToImmutableListMultimap",
"toImmutableListMultimap")
.put(ImmutableMap.class.getCanonicalName(), "toImmutableMap")
.put(ImmutableMultiset.class.getCanonicalName(), "toImmutableMultiset")
.put(ImmutableRangeSet.class.getCanonicalName(), "toImmutableRangeSet")
.put(ImmutableSet.class.getCanonicalName(), "toImmutableSet")
.put("com.google.common.collect.ImmutableList", "toImmutableList")
.put("com.google.common.collect.ImmutableMap", "toImmutableMap")
.put("com.google.common.collect.ImmutableMultiset", "toImmutableMultiset")
.put("com.google.common.collect.ImmutableRangeSet", "toImmutableRangeSet")
.putAll(
ImmutableSetMultimap.class.getCanonicalName(),
"com.google.common.collect.ImmutableSetMultimap",
"flatteningToImmutableSetMultimap",
"toImmutableSetMultimap")
.put(ImmutableSortedMap.class.getCanonicalName(), "toImmutableSortedMap")
.put(ImmutableSortedMultiset.class.getCanonicalName(), "toImmutableSortedMultiset")
.put(ImmutableSortedSet.class.getCanonicalName(), "toImmutableSortedSet")
.put(ImmutableTable.class.getCanonicalName(), "toImmutableTable")
.put("com.google.common.collect.ImmutableSet", "toImmutableSet")
.put("com.google.common.collect.ImmutableSortedMap", "toImmutableSortedMap")
.put("com.google.common.collect.ImmutableSortedMultiset", "toImmutableSortedMultiset")
.put("com.google.common.collect.ImmutableSortedSet", "toImmutableSortedSet")
.put("com.google.common.collect.ImmutableTable", "toImmutableTable")
.put("com.google.common.collect.Sets", "toImmutableEnumSet")
.put("com.google.common.base.Functions", "identity")
.put("java.time.ZoneOffset", "UTC")
.put("java.util.function.Function", "identity")
.put("java.util.function.Predicate", "not")
.put("java.util.UUID", "randomUUID")
.put("org.junit.jupiter.params.provider.Arguments", "arguments")
.putAll(
Objects.class.getCanonicalName(),
"java.util.Objects",
"checkIndex",
"checkFromIndexSize",
"checkFromToIndex",
"requireNonNull",
"requireNonNullElse",
"requireNonNullElseGet")
.put(Predicate.class.getCanonicalName(), "not")
.put(Sets.class.getCanonicalName(), "toImmutableEnumSet")
.put(UUID.class.getCanonicalName(), "randomUUID")
.put(ZoneOffset.class.getCanonicalName(), "UTC")
.put("org.junit.jupiter.params.provider.Arguments", "arguments")
.putAll("com.google.common.collect.Comparators", "emptiesFirst", "emptiesLast")
.build();
/**
* Type members that should never be statically imported.
*
* <p>Identifiers listed by {@link #STATIC_IMPORT_EXEMPTED_IDENTIFIERS} should be omitted from
* this collection.
*/
// XXX: Perhaps the set of exempted `java.util.Collections` methods is too strict. For now any
// method name that could be considered "too vague" or could conceivably mean something else in a
// specific context is left out.
@VisibleForTesting
static final ImmutableSetMultimap<String, String> STATIC_IMPORT_EXEMPTED_MEMBERS =
ImmutableSetMultimap.<String, String>builder()
.put("com.mongodb.client.model.Filters", "empty")
.putAll(
"java.util.Collections",
"addAll",
"copy",
"fill",
"list",
"max",
"min",
"nCopies",
"rotate",
"sort",
"swap")
.putAll("java.util.regex.Pattern", "compile", "matches", "quote")
.put("org.springframework.http.MediaType", "ALL")
.build();
/**
* Identifiers that should never be statically imported.
*
* <p>This should be a superset of the identifiers flagged by {@link
* com.google.errorprone.bugpatterns.BadImport}.
*/
@VisibleForTesting
static final ImmutableSet<String> STATIC_IMPORT_EXEMPTED_IDENTIFIERS =
ImmutableSet.of(
"builder",
"create",
"copyOf",
"from",
"getDefaultInstance",
"INSTANCE",
"newBuilder",
"of",
"valueOf");
/** Instantiates a new {@link StaticImport} instance. */
public StaticImport() {}
@@ -222,13 +228,13 @@ public final class StaticImport extends BugChecker implements MemberSelectTreeMa
private static boolean isCandidate(MemberSelectTree tree) {
String identifier = tree.getIdentifier().toString();
if (NON_STATIC_IMPORT_CANDIDATE_IDENTIFIERS.contains(identifier)) {
if (STATIC_IMPORT_EXEMPTED_IDENTIFIERS.contains(identifier)) {
return false;
}
Type type = ASTHelpers.getType(tree.getExpression());
return type != null
&& !NON_STATIC_IMPORT_CANDIDATE_MEMBERS.containsEntry(type.toString(), identifier);
&& !STATIC_IMPORT_EXEMPTED_MEMBERS.containsEntry(type.toString(), identifier);
}
private static Optional<String> getCandidateSimpleName(StaticImportInfo importInfo) {

View File

@@ -23,11 +23,11 @@ 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.Constants;
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;
/**
@@ -49,7 +49,7 @@ public final class StringJoin extends BugChecker implements MethodInvocationTree
private static final long serialVersionUID = 1L;
private static final Splitter FORMAT_SPECIFIER_SPLITTER = Splitter.on("%s");
private static final Matcher<ExpressionTree> STRING_FORMAT_INVOCATION =
staticMethod().onClass(String.class.getCanonicalName()).named("format");
staticMethod().onClass(String.class.getName()).named("format");
private static final Supplier<Type> CHAR_SEQUENCE_TYPE =
Suppliers.typeFromClass(CharSequence.class);
private static final Supplier<Type> FORMATTABLE_TYPE = Suppliers.typeFromClass(Formattable.class);
@@ -150,7 +150,7 @@ public final class StringJoin extends BugChecker implements MethodInvocationTree
SuggestedFix.Builder fix =
SuggestedFix.builder()
.replace(tree.getMethodSelect(), "String.join")
.replace(arguments.next(), Constants.format(separator));
.replace(arguments.next(), String.format("\"%s\"", Convert.quote(separator)));
while (arguments.hasNext()) {
ExpressionTree argument = arguments.next();

View File

@@ -45,11 +45,11 @@ public final class TimeZoneUsage extends BugChecker implements MethodInvocationT
anyOf(
allOf(
instanceMethod()
.onDescendantOf(Clock.class.getCanonicalName())
.onDescendantOf(Clock.class.getName())
.namedAnyOf("getZone", "withZone"),
not(enclosingClass(isSubtypeOf(Clock.class)))),
staticMethod()
.onClass(Clock.class.getCanonicalName())
.onClass(Clock.class.getName())
.namedAnyOf(
"system",
"systemDefaultZone",
@@ -59,17 +59,14 @@ public final class TimeZoneUsage extends BugChecker implements MethodInvocationT
"tickSeconds"),
staticMethod()
.onClassAny(
LocalDate.class.getCanonicalName(),
LocalDateTime.class.getCanonicalName(),
LocalTime.class.getCanonicalName(),
OffsetDateTime.class.getCanonicalName(),
OffsetTime.class.getCanonicalName(),
ZonedDateTime.class.getCanonicalName())
LocalDate.class.getName(),
LocalDateTime.class.getName(),
LocalTime.class.getName(),
OffsetDateTime.class.getName(),
OffsetTime.class.getName(),
ZonedDateTime.class.getName())
.named("now"),
staticMethod()
.onClassAny(Instant.class.getCanonicalName())
.named("now")
.withNoParameters());
staticMethod().onClassAny(Instant.class.getName()).named("now").withNoParameters());
/** Instantiates a new {@link TimeZoneUsage} instance. */
public TimeZoneUsage() {}

View File

@@ -1,67 +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.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.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;
/**
* A {@link BugChecker} that flags suggested fixes that involve unconditional imports.
*
* <p>Such unconditional imports may clash with other imports of the same identifier.
*/
@AutoService(BugChecker.class)
@BugPattern(
summary = "Avoid direct invocation of `SuggestedFix#add{,Static}Import`",
link = BUG_PATTERNS_BASE_URL + "UnqualifiedSuggestedFixImport",
linkType = CUSTOM,
severity = WARNING,
tags = FRAGILE_CODE)
public final class UnqualifiedSuggestedFixImport extends BugChecker
implements MethodInvocationTreeMatcher {
private static final long serialVersionUID = 1L;
private static final Matcher<ExpressionTree> FIX_BUILDER_METHOD =
instanceMethod().onDescendantOf(SuggestedFix.Builder.class.getCanonicalName());
/** Instantiates a new {@link UnqualifiedSuggestedFixImport} instance. */
public UnqualifiedSuggestedFixImport() {}
@Override
public Description matchMethodInvocation(MethodInvocationTree tree, VisitorState state) {
if (!FIX_BUILDER_METHOD.matches(tree, state)) {
return Description.NO_MATCH;
}
switch (ASTHelpers.getSymbol(tree).getSimpleName().toString()) {
case "addImport":
return createDescription(
tree, "SuggestedFix.Builder#addImport", "SuggestedFixes#qualifyType");
case "addStaticImport":
return createDescription(
tree, "SuggestedFix.Builder#addStaticImport", "SuggestedFixes#qualifyStaticImport");
default:
return Description.NO_MATCH;
}
}
private Description createDescription(
MethodInvocationTree tree, String method, String alternative) {
return buildDescription(tree)
.setMessage(
String.format("Prefer `%s` over direct invocation of `%s`", alternative, method))
.build();
}
}

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#(,)]+)*)?)\\)");
@@ -45,9 +41,6 @@ public final class MethodMatcherFactory {
// XXX: It seems parse errors are silently swallowed. Double-check; if true, file a ticket.
// XXX: This (probably) doesn't work for methods with array type arguments; if true, implement a
// fix.
// XXX: The `nullness` warning suppression shouldn't be necessary. See
// https://github.com/typetools/checker-framework/issues/4006.
@SuppressWarnings("nullness:argument")
private static Matcher<ExpressionTree> createMethodMatcher(CharSequence signature) {
java.util.regex.Matcher m = METHOD_SIGNATURE.matcher(signature);
checkArgument(m.matches(), "Not a valid method signature: %s", signature);

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

@@ -1,14 +1,10 @@
package tech.picnic.errorprone.bugpatterns.util;
import com.google.common.collect.ImmutableList;
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,7 +28,14 @@ public enum ThirdPartyLibrary {
*
* @see <a href="https://github.com/google/guava">Guava on GitHub</a>
*/
GUAVA(ImmutableList.class.getCanonicalName()),
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.
*
@@ -67,14 +70,7 @@ public enum ThirdPartyLibrary {
return canUse.get(state);
}
/**
* Tells whether the given fully qualified type is available on the current class path.
*
* @param className The type of interest.
* @param state The context under consideration.
* @return {@code true} iff it is okay to assume or create a dependency on this type.
*/
public static boolean canIntroduceUsage(String className, VisitorState state) {
private static boolean canIntroduceUsage(String className, VisitorState state) {
return shouldIgnoreClasspath(state) || isKnownClass(className, state);
}
@@ -90,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,115 +1,24 @@
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.ImmutableMap;
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.Matches;
import com.google.errorprone.refaster.annotation.UseImportPolicy;
import java.util.Collection;
import java.util.Map;
import java.util.Set;
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;
import tech.picnic.errorprone.refaster.matchers.IsEmpty;
@OnlineDocumentation
final class AssertJMapRules {
private AssertJMapRules() {}
static final class AbstractMapAssertIsEmpty<K, V> {
@BeforeTemplate
void before(
AbstractMapAssert<?, ?, K, V> mapAssert,
@Matches(IsEmpty.class) Map<? extends K, ? extends V> wellTypedMap,
@Matches(IsEmpty.class) Map<?, ?> arbitrarilyTypedMap,
@Matches(IsEmpty.class) Iterable<? extends K> keys) {
Refaster.anyOf(
mapAssert.containsExactlyEntriesOf(wellTypedMap),
mapAssert.containsExactlyInAnyOrderEntriesOf(wellTypedMap),
mapAssert.hasSameSizeAs(arbitrarilyTypedMap),
mapAssert.isEqualTo(arbitrarilyTypedMap),
mapAssert.containsOnlyKeys(keys),
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, @Matches(IsEmpty.class) Map<?, ?> map) {
return mapAssert.isNotEqualTo(map);
}
@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<? extends K, ? extends V> map) {
AbstractMapAssert<?, ?, K, V> before(AbstractMapAssert<?, ?, K, V> mapAssert, Map<K, V> map) {
return mapAssert.isEqualTo(map);
}
@AfterTemplate
AbstractMapAssert<?, ?, K, V> after(
AbstractMapAssert<?, ?, K, V> mapAssert, Map<? extends K, ? extends V> map) {
AbstractMapAssert<?, ?, K, V> after(AbstractMapAssert<?, ?, K, V> mapAssert, Map<K, V> map) {
return mapAssert.containsExactlyInAnyOrderEntriesOf(map);
}
}
@@ -125,97 +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<?, ?> map) {
return mapAssert.hasSize(map.size());
}
@AfterTemplate
AbstractMapAssert<?, ?, K, V> after(AbstractMapAssert<?, ?, K, V> mapAssert, Map<?, ?> 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<? extends K> keys) {
return assertThat(map.keySet()).hasSameElementsAs(keys);
}
@AfterTemplate
@UseImportPolicy(STATIC_IMPORT_ALWAYS)
MapAssert<K, V> after(Map<K, V> map, Set<? extends 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());
}

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