Compare commits

...

75 Commits

Author SHA1 Message Date
Gijs de Jong
3132810c47 Add support for migrating groups attribute 2022-10-26 11:28:24 +02:00
Gijs de Jong
5de2496035 Add jdk.compiler/parser module to compilation 2022-10-25 10:30:21 +02:00
Gijs de Jong
b8ab20b754 Remove redundant naming 2022-10-25 10:30:21 +02:00
Gijs de Jong
46aab0f5ba Improve AnnotationAttributeReplacement 2022-10-25 10:30:21 +02:00
Gijs de Jong
84086d9fee Handle test setup and teardown migration 2022-10-25 10:30:20 +02:00
Gijs de Jong
4451319fb8 Ignore extends clause in TestNGClassLevelTestAnnotation 2022-10-25 10:30:20 +02:00
Gijs de Jong
af46c602e7 Introduce BugCheckers TestNG -> JUnit migration
Fix `TestNGDataProviderCheckTest`

Suggestions

Suggested changes

Introduce `BugChecker`s TestNG -> JUnit migration

Fix `TestNGDataProviderCheckTest`

Suggestions

Suggested changes

Apply fixes for new bugchecks

Rename checks for `BugPatternNaming`

Only match migratable tests

Update javadoc with *legal* characters

Remove static method in inner class

Requested changes

Introduce support for 1d array dataprovider

Retain comments in data provider return tree

Self-apply EP checks

Swapped around a few methods and fix mutable issue

Suggested changes

Suggestions

Suggestions

Suggestions 2

Prefer `orElseThrow()` over `.get()`
2022-10-25 10:30:20 +02:00
Stephan Schroevers
b2e15607c1 Migrate from JSR 305 to JSpecify (#181)
JSpecify's annotations have more well-defined semantics. Its `@Nullable`
annotation is also a type-use annotation recognized by Google Java
Format, so the formatter places it after any field or method modifiers.

See https://jspecify.dev
2022-10-25 10:18:22 +02:00
Stephan Schroevers
50e730fb40 Have LexicographicalAnnotationListing sort TYPE_USE annotations last (#182)
This ensures compatibility with Error Prone's `AnnotationPosition`
check.
2022-10-25 10:01:23 +02:00
Picnic-Bot
a844b9e532 Upgrade actions/configure-pages v2.1.1 -> v2.1.2 (#312)
See:
- https://github.com/actions/configure-pages/releases/tag/v2.1.2
- https://github.com/actions/configure-pages/compare/v2.1.1...v2.1.2
2022-10-25 09:24:43 +02:00
Stephan Schroevers
671ee1eedb Don't update project.build.outputTimestamp on mvn versions:set (#310)
As we rely on the value of `git.commit.time` instead.
2022-10-24 13:31:35 +02:00
Picnic-Bot
7fe61c226a Upgrade versions-maven-plugin 2.12.0 -> 2.13.0 (#309)
See:
- https://github.com/mojohaus/versions-maven-plugin/releases/tag/2.13.0
- https://github.com/mojohaus/versions-maven-plugin/compare/versions-maven-plugin-2.12.0...2.13.0
2022-10-24 13:21:55 +02:00
Picnic-Bot
8fcc91febf Upgrade Spring Boot 2.7.4 -> 2.7.5 (#307)
See:
- https://github.com/spring-projects/spring-boot/releases/tag/v2.7.5
- https://github.com/spring-projects/spring-boot/compare/v2.7.4...v2.7.5
2022-10-24 10:14:17 +02:00
Rick Ossendrijver
e00aba12c3 Make the build JDK 18+ compatible (#304) 2022-10-23 18:15:38 +02:00
Phil Werli
0118cc6c10 Introduce Reactor ContextEmpty Refaster rule (#306) 2022-10-23 17:30:01 +02:00
Picnic-Bot
91e009cab0 Upgrade actions/setup-java v3.5.1 -> v3.6.0 (#305)
See:
- https://github.com/actions/setup-java/releases/tag/v3.6.0
- https://github.com/actions/setup-java/compare/v3.5.1...v3.6.0
2022-10-20 09:13:00 +02:00
Phil Werli
68dca3204e Introduce Guava Preconditions Refaster rules (#292) 2022-10-20 08:55:49 +02:00
Picnic-Bot
e6ccb523fd Upgrade Mockito 4.8.0 -> 4.8.1 (#303)
See:
- https://github.com/mockito/mockito/releases/tag/v4.8.1
- https://github.com/mockito/mockito/compare/v4.8.0...v4.8.1
2022-10-20 08:46:03 +02:00
Picnic-Bot
d0533f7190 Upgrade actions/deploy-pages v1.2.1 -> v1.2.2 (#302)
See:
- https://github.com/actions/deploy-pages/releases/tag/v1.2.2
- https://github.com/actions/deploy-pages/compare/v1.2.1...v1.2.2
2022-10-20 08:12:58 +02:00
Picnic-Bot
759e7ea2ff Upgrade swagger-annotations 2.2.3 -> 2.2.4 (#299)
See:
- https://github.com/swagger-api/swagger-core/releases/tag/v2.2.4
- https://github.com/swagger-api/swagger-core/compare/v2.2.3...v2.2.4
2022-10-17 14:54:27 +02:00
Picnic-Bot
2578857a37 Upgrade errorprone-slf4j 0.1.15 -> 0.1.16 (#296)
See:
- https://github.com/KengoTODA/errorprone-slf4j/releases/tag/v0.1.16
- https://github.com/KengoTODA/errorprone-slf4j/compare/v0.1.15...v0.1.16
2022-10-17 11:10:20 +02:00
Picnic-Bot
1d28512f09 Upgrade swagger-annotations 1.6.7 -> 1.6.8 (#300)
See:
- https://github.com/swagger-api/swagger-core/releases/tag/v1.6.8
- https://github.com/swagger-api/swagger-core/compare/v1.6.7...v1.6.8
2022-10-17 10:44:11 +02:00
Picnic-Bot
4efd79bfa6 Upgrade Project Reactor 2020.0.23 -> 2020.0.24 (#295)
See:
- https://github.com/reactor/reactor/releases/tag/2020.0.24
- https://github.com/reactor/reactor/compare/2020.0.23...2020.0.24
2022-10-17 10:25:01 +02:00
Picnic-Bot
d6b28733f6 Upgrade Jackson 2.13.4 -> 2.13.4.20221013 (#294)
See:
- https://github.com/FasterXML/jackson-databind/compare/jackson-databind-2.13.4...jackson-databind-2.13.4.2
- https://github.com/FasterXML/jackson-bom/compare/jackson-bom-2.13.4...jackson-bom-2.13.4.20221013
2022-10-15 17:01:00 +02:00
Picnic-Bot
b9884fa6d6 Upgrade Error Prone 2.15.0 -> 2.16 (#291)
See:
- https://github.com/google/error-prone/releases/tag/v2.16
- https://github.com/google/error-prone/compare/v2.15.0...v2.16
- https://github.com/PicnicSupermarket/error-prone/compare/v2.15.0-picnic-3...v2.16-picnic-1
2022-10-15 13:22:02 +02:00
Picnic-Bot
4fd8f57430 Upgrade ruby/setup-ruby v1.117.0 -> v1.118.0 (#298)
See:
- https://github.com/ruby/setup-ruby/releases/tag/v1.118.0
- https://github.com/ruby/setup-ruby/compare/v1.117.0...v1.118.0
2022-10-15 11:03:17 +02:00
Jelmer Borst
a2559bddda Fix and simplify documented example compiler output (#297) 2022-10-13 10:12:37 +02:00
Stephan Schroevers
7de9bede19 [maven-release-plugin] prepare for next development iteration 2022-10-13 09:20:46 +02:00
Stephan Schroevers
85cb997f1b [maven-release-plugin] prepare release v0.4.0 2022-10-13 09:20:46 +02:00
Stephan Schroevers
d5a78186db Augment Descriptions of Refaster rule matches (#255)
By emitting a website link, if available, and reporting the matching Refaster
rule in the description's message rather than the check name. Additionally, it
is now possible to provide a custom explanatory text, to assign a custom
severity to Refaster rules, and to override the severity of matches for all
Refaster rules by passing `-Xep:Refaster:[SEVERITY]`.

Violations of Refaster rules defined in this repository now include a link to
the rule's documentation, hosted on https://error-prone.picnic.tech.
2022-10-12 11:41:14 +02:00
Rick Ossendrijver
128e178d37 Consistently prefer "Refaster rule" over "Refaster template" (#286)
As the former term references a class containing one or more `@BeforeTemplate`
methods, one or more `@Placeholder` methods and an optional `@AfterTemplate`, 
while the latter term more narrowly references a single `@BeforeTemplate` or 
`@AfterTemplate` method.
2022-10-11 17:23:54 +02:00
Picnic-Bot
7aef2cfe51 Upgrade pitest-maven-plugin 1.9.7 -> 1.9.8 (#290)
See https://github.com/hcoles/pitest/compare/1.9.7...1.9.8
2022-10-11 15:43:05 +02:00
Pieter Dirk Soels
5cec0dd508 Rename Slf4JLogStatementTest to Slf4jLogStatementTest (#289) 2022-10-11 14:57:19 +02:00
Rick Ossendrijver
3f1399c139 Accommodate rapid website development on the website branch (#287)
By (a) deploying from that branch and (b) temporarily disabling external link
checking.
2022-10-11 14:43:34 +02:00
Jelmer Borst
757d5b1d70 Unify local and GitHub Actions website generation flow (#274) 2022-10-09 19:43:01 +02:00
Jelmer Borst
45cac6105e Have website use theme variables instead of custom SASS overrides (#285) 2022-10-09 19:41:32 +02:00
Picnic-Bot
8f8f57fc5b Upgrade AutoValue 1.9 -> 1.10 (#283)
See:
- https://github.com/google/auto/releases/tag/auto-value-1.10
- https://github.com/google/auto/compare/auto-value-1.9...auto-value-1.10
2022-10-09 10:03:55 +02:00
Bastien Diederichs
902d4f7736 Suggest Flux#concatMap{,Iterable} usage in more contexts (#279) 2022-10-08 11:12:41 +02:00
Sander Mak
4ec349582c Reference blog post in README (#282) 2022-10-07 21:28:08 +02:00
Bastien Diederichs
57836103ea Fix LexicographicalAnnotationAttributeListing string sorting (#281)
In particular, the empty string is now ordered first.
2022-10-07 14:34:14 +02:00
chamil-prabodha
bd73243c34 Introduce {Mono,Flux}OnErrorComplete Refaster rules (#273) 2022-10-07 11:54:50 +02:00
Stephan Schroevers
9af50d7e0b Introduce StringJoin check (#194)
This new `BugChecker` flags `String#format` invocations which can be
replaced with a `String#join` or even `String#valueOf` invocation.
2022-10-06 11:27:29 +02:00
Picnic-Bot
f5ae47fbac Upgrade pitest-maven-plugin 1.9.6 -> 1.9.7 (#280)
See https://github.com/hcoles/pitest/compare/1.9.6...1.9.7
2022-10-06 10:40:32 +02:00
Picnic-Bot
9ad8c275d3 Upgrade actions/checkout v3.0.2 -> v3.1.0 (#277)
See:
- https://github.com/actions/checkout/releases/tag/v3.1.0
- https://github.com/actions/checkout/compare/v3.0.2...v3.1.0
2022-10-05 15:57:16 +02:00
Jelmer Borst
f4e191e33b Fix default branch reference in GitHub Actions definition (#278) 2022-10-05 12:48:07 +02:00
Picnic-Bot
c4e476a731 Upgrade Checker Framework Annotations 3.25.0 -> 3.26.0 (#276)
See:
- https://github.com/typetools/checker-framework/releases/tag/checker-framework-3.26.0
- https://github.com/typetools/checker-framework/compare/checker-framework-3.25.0...checker-framework-3.26.0
2022-10-04 10:03:16 +02:00
Jelmer Borst
5ca95eb36d Set up documentation website generation and deployment (#253)
Generating the website is done by Jekyll with a theme called `just-the-docs`. 
Deployment of the website is done via GitHub Pages.

See:
- https://github.com/jekyll/jekyll
- https://github.com/just-the-docs/just-the-docs
2022-10-04 09:08:23 +02:00
Picnic-Bot
9204ef0e84 Upgrade pitest-maven-plugin 1.9.5 -> 1.9.6 (#275)
See:
- https://github.com/hcoles/pitest/releases/tag/1.9.6
- https://github.com/hcoles/pitest/compare/1.9.5...1.9.6
2022-10-04 08:53:45 +02:00
Picnic-Bot
ed45be4e15 Upgrade Forbidden APIs plugin 3.3 -> 3.4 (#272)
See:
- https://github.com/policeman-tools/forbidden-apis/wiki/Changes
- https://github.com/policeman-tools/forbidden-apis/compare/3.3...3.4
2022-10-03 07:04:08 +02:00
Rick Ossendrijver
0561c371de Emit website link along with BugChecker rewrite suggestions (#251) 2022-10-02 14:12:27 +02:00
Nadir Belarouci
aa5ad4d25b Introduce Comparators{Min,Max} Refaster templates (#270) 2022-09-29 20:35:05 +02:00
Rick Ossendrijver
8c0041a94e Fix typos and grammar in pom.xml (#268) 2022-09-29 16:10:30 +02:00
Stephan Schroevers
397f9c3df7 Suggest canonical modifier usage for Refaster template definitions (#254) 2022-09-29 13:07:56 +02:00
Stephan Schroevers
2ba7bf9f46 Have RefasterTemplateCollection verify template test class names (#233) 2022-09-29 11:53:22 +02:00
Rick Ossendrijver
5b079eef84 Rename package tech.picnic.errorprone.refaster.{util => matchers} (#267) 2022-09-29 10:29:36 +02:00
Picnic-Bot
a2ce053daf Upgrade SLF4J API 2.0.2 -> 2.0.3 (#269)
See:
- https://www.slf4j.org/news.html
- https://github.com/qos-ch/slf4j/compare/v_2.0.2...v_2.0.3
2022-09-29 08:28:00 +02:00
Picnic-Bot
ba02bad9bf Upgrade pitest-junit5-plugin 1.0.0 -> 1.1.0 (#265)
See https://github.com/pitest/pitest-junit5-plugin/compare/1.0.0...1.1.0
2022-09-28 20:13:53 +02:00
Picnic-Bot
d97a20247f Upgrade swagger-annotations 2.2.2 -> 2.2.3 (#263)
See:
- https://github.com/swagger-api/swagger-core/releases/tag/v2.2.3
- https://github.com/swagger-api/swagger-core/compare/v2.2.2...v2.2.3
2022-09-28 19:47:13 +02:00
Sander Mak
50970eb932 Apply small README improvements (#266) 2022-09-28 17:21:51 +02:00
Picnic-Bot
7e7318ad80 Upgrade swagger-annotations 1.6.6 -> 1.6.7 (#264)
See:
- https://github.com/swagger-api/swagger-core/releases/tag/v1.6.7
- https://github.com/swagger-api/swagger-core/compare/v1.6.6...v1.6.7
2022-09-28 08:26:53 +02:00
Rick Ossendrijver
fb6fe5a96e Introduce GitHub issue template for feature requests (#244) 2022-09-27 14:53:33 +02:00
Rick Ossendrijver
e37da2a1ed Drop unnecessary BugCheckerRefactoringTestHelper file path prefixes (#248) 2022-09-27 14:22:52 +02:00
Picnic-Bot
7bef1c8e67 Upgrade actions/setup-java v3.4.1 -> v3.5.1 (#262)
See:
- https://github.com/actions/setup-java/releases/tag/v3.5.0
- https://github.com/actions/setup-java/releases/tag/v3.5.1
- https://github.com/actions/setup-java/compare/v3.4.1...v3.5.1
2022-09-27 07:57:25 +02:00
Picnic-Bot
0160eafca0 Upgrade Checkstyle 10.3.3 -> 10.3.4 (#260)
See:
- https://checkstyle.sourceforge.io/releasenotes.html
- https://github.com/checkstyle/checkstyle/releases/tag/checkstyle-10.3.4
- https://github.com/checkstyle/checkstyle/compare/checkstyle-10.3.3...checkstyle-10.3.4
2022-09-26 09:01:51 +02:00
Rick Ossendrijver
891fecd297 Replace occurrences of "which" with "that" in defining clauses (#259) 2022-09-25 19:09:22 +02:00
Jelmer Borst
7f18bd9030 Introduce GitHub issue template for reporting a bug (#223) 2022-09-25 11:26:12 +02:00
Picnic-Bot
1473a70de8 Upgrade Spring Boot 2.7.3 -> 2.7.4 (#257)
See:
- https://github.com/spring-projects/spring-boot/releases/tag/v2.7.4
- https://github.com/spring-projects/spring-boot/compare/v2.7.3...v2.7.4
2022-09-23 13:55:00 +02:00
Rick Ossendrijver
c0c3ce2644 Set project home page to https://error-prone.picnic.tech (#258) 2022-09-23 13:04:46 +02:00
Picnic-Bot
100d5c86f7 Upgrade NullAway 0.10.1 -> 0.10.2 (#256)
See:
- https://github.com/uber/NullAway/blob/master/CHANGELOG.md
- https://github.com/uber/NullAway/compare/v0.10.1...v0.10.2
2022-09-22 10:32:30 +02:00
Picnic-Bot
e12f99975b Upgrade SLF4J API 1.7.36 -> 2.0.2 (#209)
See:
- https://www.slf4j.org/news.html
- https://github.com/qos-ch/slf4j/compare/v_1.7.36...v_2.0.1
2022-09-22 09:00:19 +02:00
Picnic-Bot
791113669f Upgrade JUnit Jupiter 5.9.0 -> 5.9.1 (#252)
See:
- https://junit.org/junit5/docs/current/release-notes/index.html
- https://github.com/junit-team/junit5/releases/tag/r5.9.1
- https://github.com/junit-team/junit5/compare/r5.9.0...r5.9.1
2022-09-21 16:07:03 +02:00
Stephan Schroevers
564bc7e1d1 Generate reproducible build output (#243)
By deriving `project.build.outputTimestamp` from the timestamp of the 
most recent commit, the timestamp embedded in generated JARs no longer
depends on the exact time at which the artifacts are built. As such
repeated executions of `mvn clean install` yield byte-for-byte identical
results.

This change requires replacing `buildnumber-maven-plugin` with
`git-commit-id-maven-plugin`.

See https://maven.apache.org/guides/mini/guide-reproducible-builds.html
2022-09-21 13:23:02 +02:00
Rick Ossendrijver
43bcbeaa98 Update XXX comments to reference google/error-prone#2706 (#249) 2022-09-21 08:23:09 +02:00
Svava Bjarnadóttir
d682b7d41f Fix typos and grammar in error-prone-contrib/README.md (#250)
While there, drop an obsolete TODO entry.
2022-09-21 07:55:07 +02:00
Stephan Schroevers
72a124a20e [maven-release-plugin] prepare for next development iteration 2022-09-20 13:47:18 +02:00
336 changed files with 6429 additions and 1526 deletions

51
.github/ISSUE_TEMPLATE/bug_report.md vendored Normal file
View File

@@ -0,0 +1,51 @@
---
name: 🐛 Bug report
about: Create a report to help us improve.
title: ""
labels: bug
assignees: ""
---
### Describe the bug
<!-- Provide a clear and concise description of what the bug or issue is. -->
- [ ] I have verified that the issue is reproducible against the latest version
of the project.
- [ ] I have searched through existing issues to verify that this issue is not
already known.
### Minimal Reproducible Example
<!-- Provide a clear and concise description of what happened. Please include
steps on how to reproduce the issue. -->
```java
If applicable and possible, please replace this section with the code that
triggered the isue.
```
<details>
<summary>Logs</summary>
```sh
Please replace this sentence with log output, if applicable.
```
</details>
### Expected behavior
<!-- Provide a clear and concise description of what you expected to happen. -->
### Setup
<!-- Please complete the following information: -->
- Operating system (e.g. MacOS Monterey).
- 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
<!-- Provide any other context about the problem here. -->

View File

@@ -0,0 +1,57 @@
---
name: 🚀 Request a new feature
about: Suggest a new feature.
title: ""
labels: new feature
assignees: ""
---
### Problem
<!-- Here, describe the context of the problem that you're facing, and which
you'd like to be solved through Error Prone Support. -->
### Description of the proposed new feature
<!-- Please indicate the type of improvement. -->
- [ ] Support a stylistic preference.
- [ ] Avoid a common gotcha, or potential problem.
<!--
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.
```
-->
### Considerations
<!--
Here, mention any other aspects to consider. Relevant questions:
- If applicable, is the rewrite operation a clear improvement in all cases?
- Are there special cases to consider?
- Can we further generalize the proposed solution?
- Are there alternative solutions we should consider?
-->
### Participation
<!-- Pull requests are very welcome, and we happily review contributions. Are
you up for the challenge? :D -->
- [ ] I am willing to submit a pull request to implement this improvement.
### Additional context
<!-- Provide any other context about the request here. -->

2
.github/release.yml vendored
View File

@@ -3,7 +3,7 @@ changelog:
labels:
- "ignore-changelog"
categories:
- title: ":rocket: New Error Prone checks and Refaster templates"
- title: ":rocket: New Error Prone checks and Refaster rules"
labels:
- "new feature"
- title: ":sparkles: Improvements"

View File

@@ -2,8 +2,7 @@ name: Build and verify
on:
pull_request:
push:
branches:
- 'master'
branches: [ master ]
permissions:
contents: read
jobs:
@@ -19,9 +18,9 @@ jobs:
# 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@v3.0.2
uses: actions/checkout@v3.1.0
- name: Set up JDK
uses: actions/setup-java@v3.4.1
uses: actions/setup-java@v3.6.0
with:
java-version: ${{ matrix.jdk }}
distribution: temurin

49
.github/workflows/deploy-website.yaml vendored Normal file
View File

@@ -0,0 +1,49 @@
name: Update `error-prone.picnic.tech` website content
on:
pull_request:
push:
branches: [ master, website ]
permissions:
contents: read
id-token: write
pages: write
concurrency:
group: pages
cancel-in-progress: true
jobs:
build:
runs-on: ubuntu-22.04
steps:
- name: Check out code
uses: actions/checkout@v3.1.0
- uses: ruby/setup-ruby@v1.118.0
with:
working-directory: ./website
bundler-cache: true
- name: Configure Github Pages
uses: actions/configure-pages@v2.1.2
- name: Generate documentation
run: ./generate-docs.sh
- name: Build website with Jekyll
working-directory: ./website
run: bundle exec jekyll build
- name: Validate HTML output
working-directory: ./website
# XXX: Drop `--disable_external true` once we fully adopted the
# "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@v1.0.4
with:
path: ./website/_site
deploy:
if: github.ref == 'refs/heads/website'
needs: build
runs-on: ubuntu-22.04
environment:
name: github-pages
url: ${{ steps.deployment.outputs.page_url }}
steps:
- name: Deploy to GitHub Pages
id: deployment
uses: actions/deploy-pages@v1.2.2

View File

@@ -20,7 +20,7 @@ Before doing so, please:
When filing a bug report, please include the following:
- Any relevant information about your environment. This should generally
include the output of `java -version`, as well as the version of Error Prone
include the output of `java --version`, as well as the version of Error Prone
you're using.
- A description of what is going on (e.g. logging output, stacktraces).
- A minimum reproducible example, so that other developers can try to reproduce
@@ -51,7 +51,7 @@ Some pointers:
- 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
it must not "get in the way". So for a check which addresses a relatively
it must not "get in the way". So for a check that addresses a relatively
trivial stylistic concern it is doubly important that the violations it
detects can be auto-patched.
- Make sure you have read Error Prone's [criteria for new

View File

@@ -1,30 +1,33 @@
<div align="center">
<picture>
<source media="(prefers-color-scheme: dark)" srcset="logo-dark.svg">
<source media="(prefers-color-scheme: light)" srcset="logo.svg">
<img alt="Error Prone Support logo" src="logo.svg" width="50%">
</picture>
<picture>
<source media="(prefers-color-scheme: dark)" srcset="website/assets/images/logo-dark.svg">
<source media="(prefers-color-scheme: light)" srcset="website/assets/images/logo.svg">
<img alt="Error Prone Support logo" src="website/assets/images/logo.svg" width="50%">
</picture>
</div>
# Error Prone Support
Error Prone Support is a Picnic-opinionated extension of Google's [Error
Prone][error-prone-orig-repo]. It aims to improve code quality, focussing on
maintainability, consistency and avoidance of common gotchas.
Error Prone Support is a [Picnic][picnic-blog]-opinionated extension of
Google's [Error Prone][error-prone-orig-repo]. It aims to improve code quality,
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.
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]
[![GitHub Actions][github-actions-build-badge]][github-actions-build-master]
[![License][license-badge]][license]
[![PRs Welcome][pr-badge]][contributing]
[Getting started](#-getting-started) • [Building](#-building) •
[Getting started](#-getting-started) •
[Developing Error Prone Support](#-developing-error-prone-support) •
[How it works](#-how-it-works) • [Contributing](#%EF%B8%8F-contributing)
</div>
---
## ⚡ Getting started
@@ -60,7 +63,7 @@ it:
<artifactId>error-prone-contrib</artifactId>
<version>${error-prone-support.version}</version>
</path>
<!-- Error Prone Support's Refaster templates. -->
<!-- Error Prone Support's Refaster rules. -->
<path>
<groupId>tech.picnic.error-prone-support</groupId>
<artifactId>refaster-runner</artifactId>
@@ -116,15 +119,13 @@ code with Maven should yield two compiler warnings:
```sh
$ mvn clean install
...
[INFO] -------------------------------------------------------------
[WARNING] COMPILATION WARNING :
[INFO] -------------------------------------------------------------
[WARNING] Example.java:[9,34] [tech.picnic.errorprone.refastertemplates.BigDecimalTemplates.BigDecimalZero]
[INFO] Example.java:[9,34] [Refaster Rule] BigDecimalRules.BigDecimalZero: Refactoring opportunity
(see https://error-prone.picnic.tech/refasterrules/BigDecimalRules#BigDecimalZero)
Did you mean 'return BigDecimal.ZERO;'?
[WARNING] Example.java:[14,35] [IdentityConversion] This method invocation appears redundant; remove it or suppress this warning and add a comment explaining its purpose
...
[WARNING] Example.java:[13,35] [IdentityConversion] This method invocation appears redundant; remove it or suppress this warning and add a comment explaining its purpose
(see https://error-prone.picnic.tech/bugpatterns/IdentityConversion)
Did you mean 'return set;' or '@SuppressWarnings("IdentityConversion") public ImmutableSet<Integer> getSet() {'?
[INFO] 2 warnings
[INFO] -------------------------------------------------------------
...
```
@@ -132,17 +133,18 @@ Two things are kicking in here:
1. An Error Prone [`BugChecker`][error-prone-bugchecker] that flags unnecessary
[identity conversions][bug-checks-identity-conversion].
2. A [Refaster][refaster] template capable of
[rewriting][refaster-templates-bigdecimal] expressions of the form
2. A [Refaster][refaster] rule capable of
[rewriting][refaster-rules-bigdecimal] expressions of the form
`BigDecimal.valueOf(0)` and `new BigDecimal(0)` to `BigDecimal.ZERO`.
Be sure to check out all [bug checks][bug-checks] and [refaster
templates][refaster-templates].
rules][refaster-rules].
## 👷 Building
## 👷 Developing Error Prone Support
This is a [Maven][maven] project, so running `mvn clean install`
performs a full clean build. Some relevant flags:
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. Some
relevant flags:
- `-Dverification.warn` makes the warnings and errors emitted by various
plugins and the Java compiler non-fatal, where possible.
@@ -198,9 +200,9 @@ 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].
[bug-checks]: error-prone-contrib/src/main/java/tech/picnic/errorprone/bugpatterns/
[bug-checks-identity-conversion]: error-prone-contrib/src/main/java/tech/picnic/errorprone/bugpatterns/IdentityConversion.java
[contributing]: CONTRIBUTING.md
[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
[contributing]: https://github.com/PicnicSupermarket/error-prone-support/blob/master/CONTRIBUTING.md
[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
@@ -212,13 +214,15 @@ guidelines][contributing].
[google-java-format]: https://github.com/google/google-java-format
[idea-288052]: https://youtrack.jetbrains.com/issue/IDEA-288052
[license-badge]: https://img.shields.io/github/license/PicnicSupermarket/error-prone-support
[license]: LICENSE.md
[license]: https://github.com/PicnicSupermarket/error-prone-support/blob/master/LICENSE.md
[maven-central-badge]: https://img.shields.io/maven-central/v/tech.picnic.error-prone-support/error-prone-support?color=blue
[maven-central-search]: https://search.maven.org/artifact/tech.picnic.error-prone-support/error-prone-support
[maven]: https://maven.apache.org
[picnic-blog]: https://blog.picnic.nl
[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
[refaster]: https://errorprone.info/docs/refaster
[refaster-templates-bigdecimal]: error-prone-contrib/src/main/java/tech/picnic/errorprone/refastertemplates/BigDecimalTemplates.java
[refaster-templates]: error-prone-contrib/src/main/java/tech/picnic/errorprone/refastertemplates/
[refaster-rules-bigdecimal]: https://github.com/PicnicSupermarket/error-prone-support/blob/master/error-prone-contrib/src/main/java/tech/picnic/errorprone/refasterrules/BigDecimalRules.java
[refaster-rules]: https://github.com/PicnicSupermarket/error-prone-support/blob/master/error-prone-contrib/src/main/java/tech/picnic/errorprone/refasterrules/

View File

@@ -54,7 +54,7 @@ The following is a list of checks we'd like to see implemented:
signature groups. Using Error Prone's method matchers forbidden method calls
can easily be identified. But Error Prone can go one step further by
auto-patching violations. For each violation two fixes can be proposed: a
purely behavior-preserving fix which makes the platform-dependent behavior
purely behavior-preserving fix, which makes the platform-dependent behavior
explicit, and another which replaces the platform-dependent behavior with the
preferred alternative. (Such as using `UTF-8` instead of the system default
charset.)
@@ -64,129 +64,128 @@ The following is a list of checks we'd like to see implemented:
functionality.
- A subset of the refactor operations provided by the Eclipse-specific
[AutoRefactor][autorefactor] plugin.
- A check which replaces fully qualified types with simple types in contexts
- A check that replaces fully qualified types with simple types in contexts
where this does not introduce ambiguity. Should consider both actual Java
code and Javadoc `@link` references.
- A check which simplifies array expressions. It would replace empty array
- A check that simplifies array expressions. It would replace empty array
expressions of the form `new int[] {}` with `new int[0]`. Statements of the
form `byte[] arr = new byte[] {'c'};` would be shortened to
`byte[] arr = {'c'};`.
- A check which replaces expressions of the form
`String.format("some prefix %s", arg)` with `"some prefix " + arg`, and
similar for simple suffixes. Can perhaps be generalized further, though it's
unclear how far. (Well, a `String.format` call without arguments can
certainly be simplified, too.)
- A check which replaces single-character strings with `char`s where possible.
form `byte[] arr = new byte[] {'c'};` would be shortened to `byte[] arr =
{'c'};`.
- A check that replaces expressions of the form `String.format("some prefix
%s", arg)` with `"some prefix " + arg`, and similar for simple suffixes. Can
perhaps be generalized further, though it's unclear how far. (Well, a
`String.format` call without arguments can certainly be simplified, too.)
- A check that replaces single-character strings with `char`s where possible.
For example as argument to `StringBuilder.append` and in string
concatenations.
- A check which adds or removes the first `Locale` argument to `String.format`
- A check that adds or removes the first `Locale` argument to `String.format`
and similar calls as necessary. (For example, a format string containing only
`%s` placeholders is locale-insensitive unless any of the arguments is a
`Formattable`, while `%f` placeholders _are_ locale-sensitive.)
- A check which replaces `String.replaceAll` with `String.replace` if the first
- A check that replaces `String.replaceAll` with `String.replace` if the first
argument is certainly not a regular expression. And if both arguments are
single-character strings then the `(char, char)` overload can be invoked
instead.
- A check which flags (and ideally, replaces) `try-finally` constructs with
- A check that flags (and ideally, replaces) `try-finally` constructs with
equivalent `try-with-resources` constructs.
- A check which drops exceptions declared in `throws` clauses if they are (a)
- A check that drops exceptions declared in `throws` clauses if they are (a)
not actually thrown and (b) the associated method cannot be overridden.
- A check which tries to statically import certain methods whenever used, if
- A check that tries to statically import certain methods whenever used, if
possible. The set of targeted methods should be configurable, but may default
to e.g. `java.util.Function.identity()`, the static methods exposed by
`java.util.stream.Collectors` and the various Guava collector factory
methods.
- A check which replaces `new Random().someMethod()` calls with
`ThreadLocalRandom.current().someMethod()` calls, so as to avoid unnecessary
- A check that replaces `new Random().someMethod()` calls with
`ThreadLocalRandom.current().someMethod()` calls, to avoid unnecessary
synchronization.
- A check which drops `this.` from `this.someMethod()` calls and which
- A check that drops `this.` from `this.someMethod()` calls and which
optionally does the same for fields, if no ambiguity arises.
- A check which replaces `Integer.valueOf` calls with `Integer.parseInt` or
vice versa in order to prevent auto (un)boxing, and likewise for other number
- A check that replaces `Integer.valueOf` calls with `Integer.parseInt` or vice
versa in order to prevent auto (un)boxing, and likewise for other number
types.
- A check which flags nullable collections.
- A check which flags `AutoCloseable` resources not managed by a
- A check that flags nullable collections.
- A check that flags `AutoCloseable` resources not managed by a
`try-with-resources` construct. Certain subtypes, such as jOOQ's `DSLContext`
should be excluded.
- A check which flags `java.time` methods which implicitly consult the system
- A check that flags `java.time` methods which implicitly consult the system
clock, suggesting that a passed-in `Clock` is used instead.
- A check which flags public methods on public classes which reference
- A check that flags public methods on public classes which reference
non-public types. This can cause `IllegalAccessError`s and
`BootstrapMethodError`s at runtime.
- A check which swaps the LHS and RHS in expressions of the form
- A check that swaps the LHS and RHS in expressions of the form
`nonConstant.equals(someNonNullConstant)`.
- A check which annotates methods which only throw an exception with
- A check that annotates methods which only throw an exception with
`@Deprecated` or ` @DoNotCall`.
- A check which flags imports from other test classes.
- A Guava-specific check which replaces `Joiner.join` calls with `String.join`
- A check that flags imports from other test classes.
- A Guava-specific check that replaces `Joiner.join` calls with `String.join`
calls in those cases where the latter is a proper substitute for the former.
- A Guava-specific check which flags `{Immutable,}Multimap` type usages where
- A Guava-specific check that flags `{Immutable,}Multimap` type usages where
`{Immutable,}{List,Set}Multimap` would be more appropriate.
- A Guava-specific check which rewrites
`if (conditional) { throw new IllegalArgumentException(); }` and variants to
an equivalent `checkArgument` statement. Idem for other exception types.
- A Guava-specific check which replaces simple anonymous `CacheLoader` subclass
- A Guava-specific check that rewrites `if (conditional) { throw new
IllegalArgumentException(); }` and variants to an equivalent `checkArgument`
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 which enforces that methods with the `@Scheduled`
- 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 which enforces that `@RequestMapping` annotations,
- 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 which looks for classes in which all
`@RequestMapping` annotations (and the various aliases) specify the same
`path`/`value` property and then moves that path to the class level.
- A Spring-specific check which flags `@Value("some.property")` annotations, as
- A Spring-specific check that looks for classes in which all `@RequestMapping`
annotations (and the various aliases) specify the same `path`/`value`
property and then moves that path to the class level.
- A Spring-specific check that flags `@Value("some.property")` annotations, as
these almost certainly should be `@Value("${some.property}")`.
- A Spring-specific check which drops the `required` attribute from
- A Spring-specific check that drops the `required` attribute from
`@RequestParam` annotations when the `defaultValue` attribute is also
specified.
- A Spring-specific check which rewrites a class which uses field injection to
- A Spring-specific check that rewrites a class which uses field injection to
one which uses constructor injection. This check wouldn't be strictly
behavior preserving, but could be used for a one-off code base migration.
- A Spring-specific check which disallows field injection, except in
- A Spring-specific check that disallows field injection, except in
`AbstractTestNGSpringContextTests` subclasses. (One known edge case:
self-injections so that a bean can call itself through an implicit proxy.)
- A Spring-specific check which verifies that public methods on all classes
- A Spring-specific check that verifies that public methods on all classes
whose name matches a certain pattern, e.g. `.*Service`, are annotated
`@Secured`.
- A Spring-specific check which verifies that annotations such as
- A Spring-specific check that verifies that annotations such as
`@RequestParam` are only present in `@RestController` classes.
- A Spring-specific check which disallows `@ResponseStatus` on MVC endpoint
- A Spring-specific check that disallows `@ResponseStatus` on MVC endpoint
methods, as this prevents communication of error status codes.
- A Hibernate Validator-specific check which looks for `@UnwrapValidatedValue`
- A Hibernate Validator-specific check that looks for `@UnwrapValidatedValue`
usages and migrates the associated constraint annotations to the generic type
argument to which they (are presumed to) apply.
- A TestNG-specific check which drops method-level `@Test` annotations if a
- A TestNG-specific check that drops method-level `@Test` annotations if a
matching/more specific annotation is already present at the class level.
- A TestNG-specific check which enforces that all tests are in a group.
- A TestNG-specific check which flags field assignments in
- A TestNG-specific check that enforces that all tests are in a group.
- A TestNG-specific check that flags field assignments in
`@BeforeMethod`-annotated methods unless the class is annotated
`@Test(singleThreaded = true)`.
- A TestNG-specific check which flags usages of the `expectedExceptions`
- A TestNG-specific check that flags usages of the `expectedExceptions`
attribute of the `@Test` annotation, pointing to `assertThrows`.
- A Jongo-specific check which disallows the creation of sparse indices, in
- A Jongo-specific check that disallows the creation of sparse indices, in
favour of partial indices.
- An Immutables-specific check which replaces
- An Immutables-specific check that replaces
`checkState`/`IllegalStateException` usages inside a `@Value.Check`-annotated
method with `checkArgument`/`IllegalArgument`, since the method is invoked
when a caller attempts to create an immutable instance.
- An Immutables-specific check which disallows references to collection types
- An Immutables-specific check that disallows references to collection types
other than the Guava immutable collections, including inside generic type
arguments.
- An SLF4J-specific check which drops or adds a trailing dot from log messages,
- An SLF4J-specific check that drops or adds a trailing dot from log messages,
as applicable.
- A Mockito-specific check which identifies sequences of statements which mock
a significant number of methods on a single object with "default data"; such
- A Mockito-specific check that identifies sequences of statements which mock a
significant number of methods on a single object with "default data"; such
constructions can often benefit from a different type of default answer, such
as `Answers.RETURNS_MOCKS`.
- An RxJava-specific check which flags `.toCompletable()` calls on expressions
- An RxJava-specific check that flags `.toCompletable()` calls on expressions
of type `Single<Completable>` etc., as most likely
`.flatMapCompletable(Functions.identity())` was meant instead. Idem for other
variations.
- An RxJava-specific check which flags `expr.firstOrError()` calls and suggests
- An RxJava-specific check that flags `expr.firstOrError()` calls and suggests
`expr.switchIfEmpty(Single.error(...))`, so that an application-specific
exception is thrown instead of `NoSuchElementException`.
- An RxJava-specific check which flags use of `#assertValueSet` without
- An RxJava-specific check that flags use of `#assertValueSet` without
`#assertValueCount`, as the former method doesn't do what one may intuitively
expect it to do. See ReactiveX/RxJava#6151.
@@ -212,16 +211,16 @@ Refaster's expressiveness:
- Some Refaster refactorings (e.g. when dealing with lazy evaluation) are valid
only when some free parameter is a constant, variable reference or some other
pure expression. Introduce a way to express such a constraint. For example,
rewriting `optional1.map(Optional::of).orElse(optional2)` to
`optional1.or(() -> optional2)` is not behavior preserving if evaluation of
`optional2` has side-effects.
rewriting `optional1.map(Optional::of).orElse(optional2)` to `optional1.or(()
-> optional2)` is not behavior preserving if evaluation of `optional2` has
side effects.
- Similarly, certain refactoring operations are only valid if one of the
matches expressions is not `@Nullable`. It'd be nice to be able to express
matched expressions is not `@Nullable`. It'd be nice to be able to express
this.
- Generalize `@Placeholder` support such that rules can reference e.g. "any
concrete unary method". This would allow refactorings such as
`Mono.just(constant).flatmap(this::someFun)` ->
`Mono.defer(() -> someFun(constant))`.
`Mono.just(constant).flatmap(this::someFun)` -> `Mono.defer(() ->
someFun(constant))`.
- Sometimes a Refaster refactoring can cause the resulting code not to compile
due to a lack of generic type information. Identify and resolve such
occurrences. For example, an `@AfterTemplate` may require the insertion of a
@@ -237,42 +236,38 @@ Refaster's expressiveness:
- Provide a way to express transformations of compile-time constants. This
would allow one to e.g. rewrite single-character strings to chars or vice
versa, thereby accommodating a target API. Another example would be to
replace SLF4J's `{}` place holders with `%s` or vice versa. Yet another
replace SLF4J's `{}` placeholders with `%s` or vice versa. Yet another
example would be to rewrite `BigDecimal.valueOf("<some-long-value>")` to
`BigDecimal.valueOf(theParsedLongValue)`.
- More generally, investigate ways to plug in in fully dynamic behavior, e.g.
by providing hooks using which arbitrary predicates/transformations can be
plugged in. The result would be a Refaster/`BugChecker` hybrid. A feature
such as this could form the basis for many other features listed here. (As a
concrete example, consider the ability to reference
- More generally, investigate ways to plug in fully dynamic behavior, e.g. by
providing hooks which enable plugging in arbitrary
predicates/transformations. The result would be a Refaster/`BugChecker`
hybrid. A feature like this could form the basis for many other features
listed here. (As a concrete example, consider the ability to reference
`com.google.errorprone.matchers.Matcher` implementations.)
- Provide a way to match lambda expressions and method references which match a
specified functional interface. This would allow rewrites such as
`Mono.fromCallable(this::doesNotThrowCheckException)` ->
`Mono.fromSupplier(this::doesNotThrowCheckException)`.
- Provide an extension API using which methods or expressions can be defined
based on functional properties. A motivating example is the Java Collections
- Provide an extension API that enables defining methods or expressions based
on functional properties. A motivating example is the Java Collections
framework, which allows many ways to define (im)mutable (un)ordered
collections with(out) duplicates. One could then express things like "match
any method call with collects its inputs into an immutable ordered list". An
any method call that collects its inputs into an immutable ordered list". An
enum analogous to `java.util.stream.Collector.Characteristics` could be used.
Out of the box JDK and Guava collection factory methods could be classified,
with the user having the option to extend the classification.
- Refaster currently unconditionally ignores expressions containing comments.
Provide two additional modes: (a) match and drop the comments or (b)
transport the comments to before/after the replaced expression.
- Extend Refaster to drop imports that come become unnecessary as a result of a
refactoring. This e.g. allows one to replace a statically import TestNG
- Extend Refaster to drop imports that become unnecessary as a result of a
refactoring. This e.g. allows one to replace a statically imported TestNG
`fail(...)` invocation with a statically imported equivalent AssertJ
`fail(...)` invocation. (Observe that without an impor cleanup this
`fail(...)` invocation. (Observe that without an import cleanup this
replacement would cause a compilation error.)
- Extend the `@Repeated` match semantics such that it also covers non-varargs
methods. For a motivating example see google/error-prone#568.
- When matching explicit type references, also match super types. For a
motivating example, see the two subtly difference loop definitions in
`CollectionRemoveAllFromCollectionBlock`.
motivating example, see the two subtly different loop definitions in
`CollectionRemoveAllFromCollectionExpression`.
- Figure out why Refaster sometimes doesn't match the correct generic overload.
See the `AssertThatIterableHasOneComparableElementEqualTo` template for an
See the `AssertThatIterableHasOneComparableElementEqualTo` rule for an
example.
[autorefactor]: https://autorefactor.org

View File

@@ -5,7 +5,7 @@
<parent>
<groupId>tech.picnic.error-prone-support</groupId>
<artifactId>error-prone-support</artifactId>
<version>0.3.0</version>
<version>0.4.1-SNAPSHOT</version>
</parent>
<artifactId>error-prone-contrib</artifactId>
@@ -64,11 +64,6 @@
<artifactId>auto-service-annotations</artifactId>
<scope>provided</scope>
</dependency>
<dependency>
<groupId>com.google.code.findbugs</groupId>
<artifactId>jsr305</artifactId>
<scope>provided</scope>
</dependency>
<dependency>
<groupId>com.google.googlejavaformat</groupId>
<artifactId>google-java-format</artifactId>
@@ -146,12 +141,12 @@
<dependency>
<groupId>org.junit.jupiter</groupId>
<artifactId>junit-jupiter-api</artifactId>
<scope>test</scope>
<scope>provided</scope>
</dependency>
<dependency>
<groupId>org.junit.jupiter</groupId>
<artifactId>junit-jupiter-engine</artifactId>
<scope>test</scope>
<scope>provided</scope>
</dependency>
<dependency>
<groupId>org.junit.jupiter</groupId>

View File

@@ -1,9 +1,10 @@
package tech.picnic.errorprone.bugpatterns;
import static com.google.errorprone.BugPattern.LinkType.NONE;
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.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;
@@ -22,11 +23,12 @@ import com.sun.tools.javac.code.Symbol;
import java.util.Map;
import javax.lang.model.element.AnnotationValue;
/** A {@link BugChecker} which flags ambiguous {@code @JsonCreator}s in enums. */
/** A {@link BugChecker} that flags ambiguous {@code @JsonCreator}s in enums. */
@AutoService(BugChecker.class)
@BugPattern(
summary = "`JsonCreator.Mode` should be set for single-argument creators",
linkType = NONE,
link = BUG_PATTERNS_BASE_URL + "AmbiguousJsonCreator",
linkType = CUSTOM,
severity = WARNING,
tags = LIKELY_ERROR)
public final class AmbiguousJsonCreator extends BugChecker implements AnnotationTreeMatcher {
@@ -34,6 +36,9 @@ public final class AmbiguousJsonCreator extends BugChecker implements Annotation
private static final Matcher<AnnotationTree> IS_JSON_CREATOR_ANNOTATION =
isType("com.fasterxml.jackson.annotation.JsonCreator");
/** Instantiates a new {@link AmbiguousJsonCreator} instance. */
public AmbiguousJsonCreator() {}
@Override
public Description matchAnnotation(AnnotationTree tree, VisitorState state) {
if (!IS_JSON_CREATOR_ANNOTATION.matches(tree, state)) {

View File

@@ -0,0 +1,213 @@
package tech.picnic.errorprone.bugpatterns;
import static com.google.common.collect.ImmutableList.toImmutableList;
import static com.google.errorprone.BugPattern.LinkType.NONE;
import static com.google.errorprone.BugPattern.SeverityLevel.ERROR;
import static com.google.errorprone.BugPattern.StandardTags.REFACTORING;
import com.google.auto.service.AutoService;
import com.google.common.collect.ImmutableList;
import com.google.common.collect.ImmutableMap;
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.Replacement;
import com.google.errorprone.fixes.SuggestedFix;
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.ClassTree;
import com.sun.source.tree.CompilationUnitTree;
import com.sun.source.tree.ExpressionTree;
import com.sun.source.tree.MethodTree;
import com.sun.source.tree.Tree;
import com.sun.tools.javac.tree.JCTree;
import java.util.Optional;
import java.util.Set;
import tech.picnic.errorprone.bugpatterns.util.AnnotationAttributeMatcher;
import tech.picnic.errorprone.bugpatterns.util.SourceCode;
/**
* A {@link BugChecker} that replaces a predefined list of annotation attributes with its own
* separate annotation.
*/
@AutoService(BugChecker.class)
@BugPattern(
summary = "Replace annotation attributes with an annotation",
linkType = NONE,
tags = REFACTORING,
severity = ERROR)
public final class AnnotationAttributeReplacement extends BugChecker implements MethodTreeMatcher {
private static final long serialVersionUID = 1L;
private static final ImmutableMap<AnnotationAttributeMatcher, AnnotationAttributeReplacer>
ANNOTATION_ATTRIBUTE_REPLACEMENT =
ImmutableMap.<AnnotationAttributeMatcher, AnnotationAttributeReplacer>builder()
.put(
singleArgumentMatcher("org.testng.annotations.Test#singleThreaded"),
(annotation, argument, state) ->
Optional.of(
SuggestedFix.builder()
.merge(removeAnnotationArgument(annotation, argument, state))
.merge(
SuggestedFix.prefixWith(
annotation,
"// XXX: Removed argument `singleThreaded = true`, as this cannot be migrated to JUnit!\n"))
.build()))
.put(
singleArgumentMatcher("org.testng.annotations.Test#priority"),
(annotation, argument, state) -> {
ClassTree classTree = state.findEnclosing(ClassTree.class);
if (classTree == null) {
return Optional.empty();
}
if (argument.getKind() != Tree.Kind.ASSIGNMENT) {
return Optional.empty();
}
AssignmentTree assignmentTree = (AssignmentTree) argument;
return Optional.of(
SuggestedFix.builder()
.merge(removeAnnotationArgument(annotation, argument, state))
.merge(
SuggestedFix.postfixWith(
annotation,
String.format(
"\n@org.junit.jupiter.api.Order(%s)",
SourceCode.treeToString(
assignmentTree.getExpression(), state))))
.merge(
SuggestedFix.prefixWith(
classTree,
"@TestMethodOrder(MethodOrderer.OrderAnnotation.class)\n"))
.addImport("org.junit.jupiter.api.TestMethodOrder")
.addImport("org.junit.jupiter.api.MethodOrderer")
.build());
})
.put(
singleArgumentMatcher("org.testng.annotations.Test#description"),
(annotation, argument, state) ->
Optional.of(argument)
.filter(AssignmentTree.class::isInstance)
.map(AssignmentTree.class::cast)
.map(
assignmentTree ->
SuggestedFix.builder()
.merge(removeAnnotationArgument(annotation, argument, state))
.merge(
SuggestedFix.postfixWith(
annotation,
String.format(
"\n@org.junit.jupiter.api.DisplayName(%s)",
SourceCode.treeToString(
assignmentTree.getExpression(), state))))
.build()))
.put(
singleArgumentMatcher("org.testng.annotations.Test#groups"),
(annotation, argument, state) ->
Optional.of(argument)
.filter(AssignmentTree.class::isInstance)
.map(AssignmentTree.class::cast)
.map(
assignmentTree ->
SuggestedFix.builder()
.merge(removeAnnotationArgument(annotation, argument, state))
.merge(
SuggestedFix.postfixWith(
annotation,
String.format(
"\n@org.junit.jupiter.api.Tag(%s)",
SourceCode.treeToString(
assignmentTree.getExpression(), state))))
.build()))
.build();
@Override
public Description matchMethod(MethodTree tree, VisitorState state) {
ImmutableList<AnnotationTree> annotations =
ASTHelpers.getAnnotations(tree).stream().collect(toImmutableList());
SuggestedFix.Builder builder = SuggestedFix.builder();
annotations.forEach(
annotation -> {
ANNOTATION_ATTRIBUTE_REPLACEMENT.forEach(
(matcher, fixBuilder) ->
matcher
.extractMatchingArguments(annotation)
.forEach(
argument ->
fixBuilder
.buildFix(annotation, argument, state)
.ifPresent(builder::merge)));
tryRemoveTrailingParenthesis(annotation, builder.build(), state)
.ifPresent(builder::merge);
});
return builder.isEmpty() ? Description.NO_MATCH : describeMatch(tree, builder.build());
}
private static Optional<SuggestedFix> tryRemoveTrailingParenthesis(
AnnotationTree annotation, SuggestedFix fix, VisitorState state) {
JCTree.JCCompilationUnit compileUnit =
((JCTree.JCCompilationUnit) state.findEnclosing(CompilationUnitTree.class));
if (compileUnit == null) {
return Optional.empty();
}
Set<Replacement> replacements = fix.getReplacements(compileUnit.endPositions);
String annotationSource = SourceCode.treeToString(annotation, state).replace(", ", ",");
String annotationArguments =
annotationSource.substring(
annotationSource.indexOf("(") + 1, annotationSource.length() - 1);
int argumentReplacementLength = replacements.stream().mapToInt(Replacement::length).sum();
if (argumentReplacementLength != annotationArguments.length()) {
return Optional.empty();
}
return replacements.stream()
.filter(replacement -> replacement.length() != 0)
.map(Replacement::startPosition)
.reduce(Integer::min)
.flatMap(
min ->
replacements.stream()
.filter(replacement -> replacement.length() != 0)
.map(Replacement::endPosition)
.reduce(Integer::max)
.map(
max ->
SuggestedFix.builder()
.merge(SuggestedFix.replace(min - 1, min, ""))
.merge(SuggestedFix.replace(max, max + 1, ""))
.build()));
}
private static SuggestedFix removeAnnotationArgument(
AnnotationTree annotation, ExpressionTree argument, VisitorState state) {
String annotationSource = SourceCode.treeToString(annotation, state);
String argumentSource = SourceCode.treeToString(argument, state);
int argumentSourceIndex = annotationSource.indexOf(argumentSource);
boolean endsWithComma =
annotationSource
.substring(
argumentSourceIndex + argumentSource.length(),
argumentSourceIndex + argumentSource.length() + 1)
.equals(",");
return SuggestedFix.builder().replace(argument, "", 0, endsWithComma ? 1 : 0).build();
}
private static AnnotationAttributeMatcher singleArgumentMatcher(String fullyQualifiedArgument) {
return AnnotationAttributeMatcher.create(
Optional.of(ImmutableList.of(fullyQualifiedArgument)), ImmutableList.of());
}
@FunctionalInterface
interface AnnotationAttributeReplacer {
Optional<SuggestedFix> buildFix(
AnnotationTree annotation, ExpressionTree argument, VisitorState state);
}
}

View File

@@ -1,6 +1,6 @@
package tech.picnic.errorprone.bugpatterns;
import static com.google.errorprone.BugPattern.LinkType.NONE;
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;
@@ -8,6 +8,7 @@ import static com.google.errorprone.matchers.Matchers.argument;
import static com.google.errorprone.matchers.Matchers.argumentCount;
import static com.google.errorprone.matchers.Matchers.instanceMethod;
import static com.google.errorprone.matchers.Matchers.nullLiteral;
import static tech.picnic.errorprone.bugpatterns.util.Documentation.BUG_PATTERNS_BASE_URL;
import com.google.auto.service.AutoService;
import com.google.errorprone.BugPattern;
@@ -21,9 +22,9 @@ import com.google.errorprone.matchers.Matcher;
import com.sun.source.tree.MethodInvocationTree;
/**
* A {@link BugChecker} which flags AssertJ {@code isEqualTo(null)} checks for simplification.
* A {@link BugChecker} that flags AssertJ {@code isEqualTo(null)} checks for simplification.
*
* <p>This bug checker cannot be replaced with a simple Refaster template, as the Refaster approach
* <p>This bug checker cannot be replaced with a simple Refaster rule, as the Refaster approach
* would require that all overloads of {@link org.assertj.core.api.Assert#isEqualTo(Object)} (such
* as {@link org.assertj.core.api.AbstractStringAssert#isEqualTo(String)}) are explicitly
* enumerated. This bug checker generically matches all such current and future overloads.
@@ -31,7 +32,8 @@ import com.sun.source.tree.MethodInvocationTree;
@AutoService(BugChecker.class)
@BugPattern(
summary = "Prefer `.isNull()` over `.isEqualTo(null)`",
linkType = NONE,
link = BUG_PATTERNS_BASE_URL + "AssertJIsNull",
linkType = CUSTOM,
severity = SUGGESTION,
tags = SIMPLIFICATION)
public final class AssertJIsNull extends BugChecker implements MethodInvocationTreeMatcher {
@@ -42,6 +44,9 @@ public final class AssertJIsNull extends BugChecker implements MethodInvocationT
argumentCount(1),
argument(0, nullLiteral()));
/** Instantiates a new {@link AssertJIsNull} instance. */
public AssertJIsNull() {}
@Override
public Description matchMethodInvocation(MethodInvocationTree tree, VisitorState state) {
if (!ASSERT_IS_EQUAL_TO_NULL.matches(tree, state)) {

View File

@@ -1,11 +1,12 @@
package tech.picnic.errorprone.bugpatterns;
import static com.google.errorprone.BugPattern.LinkType.NONE;
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.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;
@@ -24,11 +25,12 @@ import com.sun.source.tree.MethodTree;
import com.sun.source.tree.Tree;
import java.util.List;
/** A {@link BugChecker} which flags redundant {@code @Autowired} constructor annotations. */
/** A {@link BugChecker} that flags redundant {@code @Autowired} constructor annotations. */
@AutoService(BugChecker.class)
@BugPattern(
summary = "Omit `@Autowired` on a class' sole constructor, as it is redundant",
linkType = NONE,
link = BUG_PATTERNS_BASE_URL + "AutowiredConstructor",
linkType = CUSTOM,
severity = SUGGESTION,
tags = SIMPLIFICATION)
public final class AutowiredConstructor extends BugChecker implements ClassTreeMatcher {
@@ -36,6 +38,9 @@ public final class AutowiredConstructor extends BugChecker implements ClassTreeM
private static final MultiMatcher<Tree, AnnotationTree> AUTOWIRED_ANNOTATION =
annotations(AT_LEAST_ONE, isType("org.springframework.beans.factory.annotation.Autowired"));
/** Instantiates a new {@link AutowiredConstructor} instance. */
public AutowiredConstructor() {}
@Override
public Description matchClass(ClassTree tree, VisitorState state) {
List<MethodTree> constructors = ASTHelpers.getConstructors(tree);

View File

@@ -1,8 +1,9 @@
package tech.picnic.errorprone.bugpatterns;
import static com.google.errorprone.BugPattern.LinkType.NONE;
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 tech.picnic.errorprone.bugpatterns.util.Documentation.BUG_PATTERNS_BASE_URL;
import com.google.auto.service.AutoService;
import com.google.common.collect.ImmutableSet;
@@ -27,11 +28,12 @@ import java.util.regex.Matcher;
import java.util.regex.Pattern;
import tech.picnic.errorprone.bugpatterns.util.SourceCode;
/** A {@link BugChecker} which flags annotations that could be written more concisely. */
/** A {@link BugChecker} that flags annotations that could be written more concisely. */
@AutoService(BugChecker.class)
@BugPattern(
summary = "Omit redundant syntax from annotation declarations",
linkType = NONE,
link = BUG_PATTERNS_BASE_URL + "CanonicalAnnotationSyntax",
linkType = CUSTOM,
severity = SUGGESTION,
tags = SIMPLIFICATION)
public final class CanonicalAnnotationSyntax extends BugChecker implements AnnotationTreeMatcher {
@@ -44,6 +46,9 @@ public final class CanonicalAnnotationSyntax extends BugChecker implements Annot
CanonicalAnnotationSyntax::dropRedundantValueAttribute,
CanonicalAnnotationSyntax::dropRedundantCurlies);
/** Instantiates a new {@link CanonicalAnnotationSyntax} instance. */
public CanonicalAnnotationSyntax() {}
@Override
public Description matchAnnotation(AnnotationTree tree, VisitorState state) {
return FIX_FACTORIES.stream()

View File

@@ -1,9 +1,10 @@
package tech.picnic.errorprone.bugpatterns;
import static com.google.errorprone.BugPattern.LinkType.NONE;
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.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;
@@ -19,7 +20,7 @@ import com.sun.source.tree.MethodInvocationTree;
import java.util.stream.Collector;
/**
* A {@link BugChecker} which flags {@link Collector Collectors} that don't clearly express
* A {@link BugChecker} that flags {@link Collector Collectors} that don't clearly express
* (im)mutability.
*
* <p>Replacing such collectors with alternatives that produce immutable collections is preferred.
@@ -29,7 +30,8 @@ import java.util.stream.Collector;
@BugPattern(
summary =
"Avoid `Collectors.to{List,Map,Set}` in favour of alternatives that emphasize (im)mutability",
linkType = NONE,
link = BUG_PATTERNS_BASE_URL + "CollectorMutability",
linkType = CUSTOM,
severity = WARNING,
tags = FRAGILE_CODE)
public final class CollectorMutability extends BugChecker implements MethodInvocationTreeMatcher {
@@ -43,6 +45,9 @@ public final class CollectorMutability extends BugChecker implements MethodInvoc
private static final Matcher<ExpressionTree> SET_COLLECTOR =
staticMethod().anyClass().named("toSet");
/** Instantiates a new {@link CollectorMutability} instance. */
public CollectorMutability() {}
@Override
public Description matchMethodInvocation(MethodInvocationTree tree, VisitorState state) {
if (!COLLECTOR_METHOD.matches(tree, state)) {

View File

@@ -1,12 +1,13 @@
package tech.picnic.errorprone.bugpatterns;
import static com.google.errorprone.BugPattern.LinkType.NONE;
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 com.google.auto.service.AutoService;
import com.google.errorprone.BugPattern;
@@ -22,11 +23,12 @@ import com.sun.source.tree.MethodTree;
import com.sun.source.tree.Tree;
import java.util.Optional;
/** A {@link BugChecker} which flags empty methods that seemingly can simply be deleted. */
/** A {@link BugChecker} that flags empty methods that seemingly can simply be deleted. */
@AutoService(BugChecker.class)
@BugPattern(
summary = "Empty method can likely be deleted",
linkType = NONE,
link = BUG_PATTERNS_BASE_URL + "EmptyMethod",
linkType = CUSTOM,
severity = SUGGESTION,
tags = SIMPLIFICATION)
public final class EmptyMethod extends BugChecker implements MethodTreeMatcher {
@@ -36,6 +38,9 @@ public final class EmptyMethod extends BugChecker implements MethodTreeMatcher {
AT_LEAST_ONE,
anyOf(isType("java.lang.Override"), isType("org.aspectj.lang.annotation.Pointcut")));
/** Instantiates a new {@link EmptyMethod} instance. */
public EmptyMethod() {}
@Override
public Description matchMethod(MethodTree tree, VisitorState state) {
if (tree.getBody() == null

View File

@@ -1,11 +1,12 @@
package tech.picnic.errorprone.bugpatterns;
import static com.google.errorprone.BugPattern.LinkType.NONE;
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.Matchers.anyOf;
import static com.google.errorprone.matchers.Matchers.instanceMethod;
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.Splitter;
@@ -30,7 +31,7 @@ import java.util.List;
import java.util.Optional;
/**
* A {@link BugChecker} which flags improperly formatted Error Prone test code.
* A {@link BugChecker} that flags improperly formatted Error Prone test code.
*
* <p>All test code should be formatted in accordance with Google Java Format's {@link Formatter}
* output, and imports should be ordered according to the {@link Style#GOOGLE Google} style.
@@ -50,7 +51,8 @@ import java.util.Optional;
@AutoService(BugChecker.class)
@BugPattern(
summary = "Test code should follow the Google Java style",
linkType = NONE,
link = BUG_PATTERNS_BASE_URL + "ErrorProneTestHelperSourceFormat",
linkType = CUSTOM,
severity = SUGGESTION,
tags = STYLE)
public final class ErrorProneTestHelperSourceFormat extends BugChecker
@@ -70,6 +72,9 @@ public final class ErrorProneTestHelperSourceFormat extends BugChecker
.onDescendantOf("com.google.errorprone.BugCheckerRefactoringTestHelper.ExpectOutput")
.named("addOutputLines");
/** Instantiates a new {@link ErrorProneTestHelperSourceFormat} instance. */
public ErrorProneTestHelperSourceFormat() {}
@Override
public Description matchMethodInvocation(MethodInvocationTree tree, VisitorState state) {
boolean isOutputSource = OUTPUT_SOURCE_ACCEPTING_METHOD.matches(tree, state);

View File

@@ -2,11 +2,12 @@ package tech.picnic.errorprone.bugpatterns;
import static com.google.common.collect.ImmutableSet.toImmutableSet;
import static com.google.common.collect.ImmutableSetMultimap.toImmutableSetMultimap;
import static com.google.errorprone.BugPattern.LinkType.NONE;
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.method.MethodMatchers.staticMethod;
import static java.util.stream.Collectors.collectingAndThen;
import static tech.picnic.errorprone.bugpatterns.util.Documentation.BUG_PATTERNS_BASE_URL;
import com.google.auto.service.AutoService;
import com.google.common.collect.ImmutableSet;
@@ -30,13 +31,14 @@ import java.util.Set;
import java.util.stream.Stream;
/**
* A {@link BugChecker} which flags {@link Ordering#explicit(Object, Object[])}} invocations listing
* A {@link BugChecker} that flags {@link Ordering#explicit(Object, Object[])}} invocations listing
* a subset of an enum type's values.
*/
@AutoService(BugChecker.class)
@BugPattern(
summary = "Make sure `Ordering#explicit` lists all of an enum's values",
linkType = NONE,
link = BUG_PATTERNS_BASE_URL + "ExplicitEnumOrdering",
linkType = CUSTOM,
severity = WARNING,
tags = FRAGILE_CODE)
public final class ExplicitEnumOrdering extends BugChecker implements MethodInvocationTreeMatcher {
@@ -44,6 +46,9 @@ public final class ExplicitEnumOrdering extends BugChecker implements MethodInvo
private static final Matcher<ExpressionTree> EXPLICIT_ORDERING =
staticMethod().onClass(Ordering.class.getName()).named("explicit");
/** Instantiates a new {@link ExplicitEnumOrdering} instance. */
public ExplicitEnumOrdering() {}
@Override
public Description matchMethodInvocation(MethodInvocationTree tree, VisitorState state) {
if (!EXPLICIT_ORDERING.matches(tree, state)) {
@@ -84,6 +89,6 @@ public final class ExplicitEnumOrdering extends BugChecker implements MethodInvo
private static Stream<String> getMissingEnumValues(Type enumType, Set<String> values) {
Symbol.TypeSymbol typeSymbol = enumType.asElement();
return Sets.difference(ASTHelpers.enumValues(typeSymbol), values).stream()
.map(v -> String.format("%s.%s", typeSymbol.getSimpleName(), v));
.map(v -> String.join(".", typeSymbol.getSimpleName(), v));
}
}

View File

@@ -1,9 +1,10 @@
package tech.picnic.errorprone.bugpatterns;
import static com.google.errorprone.BugPattern.LinkType.NONE;
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.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.Iterables;
@@ -24,7 +25,7 @@ import java.util.function.Supplier;
import reactor.core.publisher.Flux;
/**
* A {@link BugChecker} which flags usages of {@link Flux#flatMap(Function)} and {@link
* A {@link BugChecker} that flags usages of {@link Flux#flatMap(Function)} and {@link
* Flux#flatMapSequential(Function)}.
*
* <p>{@link Flux#flatMap(Function)} and {@link Flux#flatMapSequential(Function)} eagerly perform up
@@ -43,7 +44,8 @@ import reactor.core.publisher.Flux;
summary =
"`Flux#flatMap` and `Flux#flatMapSequential` have subtle semantics; "
+ "please use `Flux#concatMap` or explicitly specify the desired amount of concurrency",
linkType = NONE,
link = BUG_PATTERNS_BASE_URL + "FluxFlatMapUsage",
linkType = CUSTOM,
severity = ERROR,
tags = LIKELY_ERROR)
public final class FluxFlatMapUsage extends BugChecker
@@ -56,6 +58,9 @@ public final class FluxFlatMapUsage extends BugChecker
.namedAnyOf("flatMap", "flatMapSequential")
.withParameters(Function.class.getName());
/** Instantiates a new {@link FluxFlatMapUsage} instance. */
public FluxFlatMapUsage() {}
@Override
public Description matchMethodInvocation(MethodInvocationTree tree, VisitorState state) {
if (!FLUX_FLATMAP.matches(tree, state)) {

View File

@@ -1,6 +1,6 @@
package tech.picnic.errorprone.bugpatterns;
import static com.google.errorprone.BugPattern.LinkType.NONE;
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 com.google.errorprone.matchers.Matchers.allOf;
@@ -10,6 +10,7 @@ import static com.google.errorprone.matchers.Matchers.instanceMethod;
import static com.google.errorprone.matchers.Matchers.not;
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;
import com.google.auto.service.AutoService;
import com.google.errorprone.BugPattern;
@@ -31,12 +32,12 @@ import com.sun.source.util.SimpleTreeVisitor;
import java.util.ArrayList;
import java.util.List;
import java.util.Optional;
import javax.annotation.Nullable;
import org.jspecify.nullness.Nullable;
import tech.picnic.errorprone.bugpatterns.util.SourceCode;
/**
* A {@link BugChecker} which flags string concatenations that produce a format string; in such
* cases the string concatenation should instead be deferred to the invoked method.
* A {@link BugChecker} that flags string concatenations that produce a format string; in such cases
* the string concatenation should instead be deferred to the invoked method.
*
* @implNote This checker is based on the implementation of {@link
* com.google.errorprone.bugpatterns.flogger.FloggerStringConcatenation}.
@@ -46,12 +47,13 @@ import tech.picnic.errorprone.bugpatterns.util.SourceCode;
// should introduce special handling of `Formattable` arguments, as this check would replace a
// `Formattable#toString` invocation with a `Formattable#formatTo` invocation. But likely that
// should be considered a bug fix, too.
// XXX: Introduce a separate check which adds/removes the `Locale` parameter to `String.format`
// invocations, as necessary.
// XXX: Introduce a separate check that adds/removes the `Locale` parameter to `String.format`
// invocations, as necessary. See also a comment in the `StringJoin` check.
@AutoService(BugChecker.class)
@BugPattern(
summary = "Defer string concatenation to the invoked method",
linkType = NONE,
link = BUG_PATTERNS_BASE_URL + "FormatStringConcatenation",
linkType = CUSTOM,
severity = WARNING,
tags = SIMPLIFICATION)
public final class FormatStringConcatenation extends BugChecker
@@ -127,6 +129,9 @@ public final class FormatStringConcatenation extends BugChecker
.onDescendantOf("org.slf4j.Logger")
.namedAnyOf("debug", "error", "info", "trace", "warn");
/** Instantiates a new {@link FormatStringConcatenation} instance. */
public FormatStringConcatenation() {}
@Override
public Description matchMethodInvocation(MethodInvocationTree tree, VisitorState state) {
if (hasNonConstantStringConcatenationArgument(tree, 0, state)) {
@@ -202,7 +207,7 @@ public final class FormatStringConcatenation extends BugChecker
}
private static class ReplacementArgumentsConstructor
extends SimpleTreeVisitor<Void, VisitorState> {
extends SimpleTreeVisitor<@Nullable Void, VisitorState> {
private final StringBuilder formatString = new StringBuilder();
private final List<Tree> formatArguments = new ArrayList<>();
private final String formatSpecifier;
@@ -211,9 +216,8 @@ public final class FormatStringConcatenation extends BugChecker
this.formatSpecifier = formatSpecifier;
}
@Nullable
@Override
public Void visitBinary(BinaryTree tree, VisitorState state) {
public @Nullable Void visitBinary(BinaryTree tree, VisitorState state) {
if (tree.getKind() == Kind.PLUS && isStringTyped(tree, state)) {
tree.getLeftOperand().accept(this, state);
tree.getRightOperand().accept(this, state);
@@ -224,15 +228,13 @@ public final class FormatStringConcatenation extends BugChecker
return null;
}
@Nullable
@Override
public Void visitParenthesized(ParenthesizedTree tree, VisitorState state) {
public @Nullable Void visitParenthesized(ParenthesizedTree tree, VisitorState state) {
return tree.getExpression().accept(this, state);
}
@Nullable
@Override
protected Void defaultAction(Tree tree, VisitorState state) {
protected @Nullable Void defaultAction(Tree tree, VisitorState state) {
appendExpression(tree);
return null;
}

View File

@@ -1,12 +1,13 @@
package tech.picnic.errorprone.bugpatterns;
import static com.google.common.collect.ImmutableSet.toImmutableSet;
import static com.google.errorprone.BugPattern.LinkType.NONE;
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 com.google.errorprone.matchers.Matchers.anyOf;
import static com.google.errorprone.matchers.Matchers.staticMethod;
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.primitives.Primitives;
@@ -36,7 +37,8 @@ import tech.picnic.errorprone.bugpatterns.util.SourceCode;
@AutoService(BugChecker.class)
@BugPattern(
summary = "Avoid or clarify identity conversions",
linkType = NONE,
link = BUG_PATTERNS_BASE_URL + "IdentityConversion",
linkType = CUSTOM,
severity = WARNING,
tags = SIMPLIFICATION)
public final class IdentityConversion extends BugChecker implements MethodInvocationTreeMatcher {
@@ -70,6 +72,9 @@ public final class IdentityConversion extends BugChecker implements MethodInvoca
.namedAnyOf("concat", "firstWithSignal", "from", "merge"),
staticMethod().onClass("reactor.core.publisher.Mono").namedAnyOf("from", "fromDirect"));
/** Instantiates a new {@link IdentityConversion} instance. */
public IdentityConversion() {}
@Override
public Description matchMethodInvocation(MethodInvocationTree tree, VisitorState state) {
List<? extends ExpressionTree> arguments = tree.getArguments();

View File

@@ -1,6 +1,6 @@
package tech.picnic.errorprone.bugpatterns;
import static com.google.errorprone.BugPattern.LinkType.NONE;
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.Matchers.allOf;
@@ -11,6 +11,7 @@ import static com.google.errorprone.matchers.Matchers.hasModifier;
import static com.google.errorprone.matchers.Matchers.isSubtypeOf;
import static com.google.errorprone.matchers.Matchers.methodReturns;
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.errorprone.BugPattern;
@@ -26,7 +27,7 @@ import java.util.SortedSet;
import javax.lang.model.element.Modifier;
/**
* A {@link BugChecker} which flags {@link SortedSet} property declarations inside
* A {@link BugChecker} that flags {@link SortedSet} property declarations inside
* {@code @Value.Immutable}- and {@code @Value.Modifiable}-annotated types that lack a
* {@code @Value.NaturalOrder} or {@code @Value.ReverseOrder} annotation.
*
@@ -44,7 +45,8 @@ import javax.lang.model.element.Modifier;
summary =
"`SortedSet` properties of a `@Value.Immutable` or `@Value.Modifiable` type must be "
+ "annotated with `@Value.NaturalOrder` or `@Value.ReverseOrder`",
linkType = NONE,
link = BUG_PATTERNS_BASE_URL + "ImmutablesSortedSetComparator",
linkType = CUSTOM,
severity = ERROR,
tags = LIKELY_ERROR)
public final class ImmutablesSortedSetComparator extends BugChecker implements MethodTreeMatcher {
@@ -65,6 +67,9 @@ public final class ImmutablesSortedSetComparator extends BugChecker implements M
hasAnnotation("org.immutables.value.Value.NaturalOrder"),
hasAnnotation("org.immutables.value.Value.ReverseOrder"))));
/** Instantiates a new {@link ImmutablesSortedSetComparator} instance. */
public ImmutablesSortedSetComparator() {}
@Override
public Description matchMethod(MethodTree tree, VisitorState state) {
if (!METHOD_LACKS_ANNOTATION.matches(tree, state)) {

View File

@@ -1,6 +1,6 @@
package tech.picnic.errorprone.bugpatterns;
import static com.google.errorprone.BugPattern.LinkType.NONE;
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;
@@ -11,6 +11,7 @@ import static com.google.errorprone.matchers.Matchers.enclosingClass;
import static com.google.errorprone.matchers.Matchers.hasModifier;
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.JavaKeywords.isReservedKeyword;
import com.google.auto.service.AutoService;
@@ -38,8 +39,8 @@ import javax.lang.model.element.Modifier;
import javax.lang.model.element.Name;
import tech.picnic.errorprone.bugpatterns.util.SourceCode;
/** A {@link BugChecker} which flags non-canonical JUnit method declarations. */
// XXX: Consider introducing a class-level check which enforces that test classes:
/** A {@link BugChecker} that flags non-canonical JUnit method declarations. */
// XXX: Consider introducing a class-level check that enforces that test classes:
// 1. Are named `*Test` or `Abstract*TestCase`.
// 2. If not `abstract`, are package-private and don't have public methods and subclasses.
// 3. Only have private fields.
@@ -47,7 +48,8 @@ import tech.picnic.errorprone.bugpatterns.util.SourceCode;
@AutoService(BugChecker.class)
@BugPattern(
summary = "JUnit method declaration can likely be improved",
linkType = NONE,
link = BUG_PATTERNS_BASE_URL + "JUnitMethodDeclaration",
linkType = CUSTOM,
severity = SUGGESTION,
tags = SIMPLIFICATION)
public final class JUnitMethodDeclaration extends BugChecker implements MethodTreeMatcher {
@@ -77,6 +79,9 @@ public final class JUnitMethodDeclaration extends BugChecker implements MethodTr
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 (HAS_UNMODIFIABLE_SIGNATURE.matches(tree, state)) {

View File

@@ -1,13 +1,16 @@
package tech.picnic.errorprone.bugpatterns;
import static com.google.errorprone.BugPattern.LinkType.NONE;
import static com.google.common.collect.ImmutableList.toImmutableList;
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 java.util.Comparator.comparing;
import static java.util.Comparator.naturalOrder;
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.Splitter;
import com.google.common.collect.Comparators;
import com.google.common.collect.ImmutableList;
import com.google.common.collect.ImmutableSet;
@@ -37,12 +40,12 @@ import java.util.List;
import java.util.Optional;
import java.util.Set;
import java.util.stream.Stream;
import javax.annotation.Nullable;
import org.jspecify.nullness.Nullable;
import tech.picnic.errorprone.bugpatterns.util.AnnotationAttributeMatcher;
import tech.picnic.errorprone.bugpatterns.util.SourceCode;
/**
* A {@link BugChecker} which flags annotation array listings which aren't sorted lexicographically.
* A {@link BugChecker} that flags annotation array listings which aren't sorted lexicographically.
*
* <p>The idea behind this checker is that maintaining a sorted sequence simplifies conflict
* resolution, and can even avoid it if two branches add the same entry.
@@ -50,7 +53,8 @@ import tech.picnic.errorprone.bugpatterns.util.SourceCode;
@AutoService(BugChecker.class)
@BugPattern(
summary = "Where possible, sort annotation array attributes lexicographically",
linkType = NONE,
link = BUG_PATTERNS_BASE_URL + "LexicographicalAnnotationAttributeListing",
linkType = CUSTOM,
severity = SUGGESTION,
tags = STYLE)
public final class LexicographicalAnnotationAttributeListing extends BugChecker
@@ -69,10 +73,16 @@ 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
* properly sorted by key, then value.
*/
private static final Splitter STRING_ARGUMENT_SPLITTER = Splitter.on('=');
private final AnnotationAttributeMatcher matcher;
/** Instantiates the default {@link LexicographicalAnnotationAttributeListing}. */
/** Instantiates a default {@link LexicographicalAnnotationAttributeListing} instance. */
public LexicographicalAnnotationAttributeListing() {
this(ErrorProneFlags.empty());
}
@@ -124,7 +134,7 @@ public final class LexicographicalAnnotationAttributeListing extends BugChecker
}
List<? extends ExpressionTree> actualOrdering = array.getInitializers();
ImmutableList<? extends ExpressionTree> desiredOrdering = doSort(actualOrdering, state);
ImmutableList<? extends ExpressionTree> desiredOrdering = doSort(actualOrdering);
if (actualOrdering.equals(desiredOrdering)) {
/* In the (presumably) common case the elements are already sorted. */
return Optional.empty();
@@ -154,12 +164,12 @@ public final class LexicographicalAnnotationAttributeListing extends BugChecker
}
private static ImmutableList<? extends ExpressionTree> doSort(
Iterable<? extends ExpressionTree> elements, VisitorState state) {
Iterable<? extends ExpressionTree> elements) {
// XXX: Perhaps we should use `Collator` with `.setStrength(Collator.PRIMARY)` and
// `getCollationKey`. Not clear whether that's worth the hassle at this point.
return ImmutableList.sortedCopyOf(
comparing(
e -> getStructure(e, state),
LexicographicalAnnotationAttributeListing::getStructure,
Comparators.lexicographical(
Comparators.lexicographical(
String.CASE_INSENSITIVE_ORDER.thenComparing(naturalOrder())))),
@@ -171,38 +181,31 @@ public final class LexicographicalAnnotationAttributeListing extends BugChecker
* performed. This approach disregards e.g. irrelevant whitespace. It also allows special
* structure within string literals to be respected.
*/
private static ImmutableList<ImmutableList<String>> getStructure(
ExpressionTree array, VisitorState state) {
private static ImmutableList<ImmutableList<String>> getStructure(ExpressionTree array) {
ImmutableList.Builder<ImmutableList<String>> nodes = ImmutableList.builder();
new TreeScanner<Void, Void>() {
@Nullable
new TreeScanner<@Nullable Void, @Nullable Void>() {
@Override
public Void visitIdentifier(IdentifierTree node, @Nullable Void ctx) {
nodes.add(tokenize(node));
return super.visitIdentifier(node, ctx);
public @Nullable Void visitIdentifier(IdentifierTree node, @Nullable Void unused) {
nodes.add(ImmutableList.of(node.getName().toString()));
return super.visitIdentifier(node, unused);
}
@Nullable
@Override
public Void visitLiteral(LiteralTree node, @Nullable Void ctx) {
nodes.add(tokenize(node));
return super.visitLiteral(node, ctx);
public @Nullable Void visitLiteral(LiteralTree node, @Nullable Void unused) {
Object value = ASTHelpers.constValue(node);
nodes.add(
value instanceof String
? STRING_ARGUMENT_SPLITTER.splitToStream((String) value).collect(toImmutableList())
: ImmutableList.of(String.valueOf(value)));
return super.visitLiteral(node, unused);
}
@Nullable
@Override
public Void visitPrimitiveType(PrimitiveTypeTree node, @Nullable Void ctx) {
nodes.add(tokenize(node));
return super.visitPrimitiveType(node, ctx);
}
private ImmutableList<String> tokenize(Tree node) {
/*
* Tokens are split on `=` so that e.g. inline Spring property declarations are properly
* sorted by key, then value.
*/
return ImmutableList.copyOf(SourceCode.treeToString(node, state).split("=", -1));
public @Nullable Void visitPrimitiveType(PrimitiveTypeTree node, @Nullable Void unused) {
nodes.add(ImmutableList.of(node.getPrimitiveTypeKind().toString()));
return super.visitPrimitiveType(node, unused);
}
}.scan(array, null);

View File

@@ -1,12 +1,16 @@
package tech.picnic.errorprone.bugpatterns;
import static com.google.common.collect.ImmutableList.toImmutableList;
import static com.google.errorprone.BugPattern.LinkType.NONE;
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.sun.tools.javac.code.TypeAnnotations.AnnotationType.DECLARATION;
import static com.sun.tools.javac.code.TypeAnnotations.AnnotationType.TYPE;
import static java.util.Comparator.comparing;
import static tech.picnic.errorprone.bugpatterns.util.Documentation.BUG_PATTERNS_BASE_URL;
import com.google.auto.service.AutoService;
import com.google.common.base.VerifyException;
import com.google.common.collect.ImmutableList;
import com.google.common.collect.Streams;
import com.google.errorprone.BugPattern;
@@ -16,10 +20,14 @@ import com.google.errorprone.bugpatterns.BugChecker.MethodTreeMatcher;
import com.google.errorprone.fixes.Fix;
import com.google.errorprone.fixes.SuggestedFix;
import com.google.errorprone.matchers.Description;
import com.google.errorprone.util.ASTHelpers;
import com.sun.source.tree.AnnotationTree;
import com.sun.source.tree.MethodTree;
import com.sun.tools.javac.code.Symbol;
import com.sun.tools.javac.code.TypeAnnotations.AnnotationType;
import java.util.Comparator;
import java.util.List;
import java.util.Optional;
import org.jspecify.nullness.Nullable;
import tech.picnic.errorprone.bugpatterns.util.SourceCode;
/**
@@ -31,12 +39,19 @@ import tech.picnic.errorprone.bugpatterns.util.SourceCode;
@AutoService(BugChecker.class)
@BugPattern(
summary = "Sort annotations lexicographically where possible",
linkType = NONE,
link = BUG_PATTERNS_BASE_URL + "LexicographicalAnnotationListing",
linkType = CUSTOM,
severity = SUGGESTION,
tags = STYLE)
public final class LexicographicalAnnotationListing extends BugChecker
implements MethodTreeMatcher {
private static final long serialVersionUID = 1L;
private static final Comparator<@Nullable AnnotationType> BY_ANNOTATION_TYPE =
(a, b) ->
(a == null || a == DECLARATION) && b == TYPE ? -1 : a == TYPE && b == DECLARATION ? 1 : 0;
/** Instantiates a new {@link LexicographicalAnnotationListing} instance. */
public LexicographicalAnnotationListing() {}
@Override
public Description matchMethod(MethodTree tree, VisitorState state) {
@@ -45,26 +60,29 @@ public final class LexicographicalAnnotationListing extends BugChecker
return Description.NO_MATCH;
}
ImmutableList<? extends AnnotationTree> sortedAnnotations = sort(originalOrdering, state);
ImmutableList<? extends AnnotationTree> sortedAnnotations =
sort(originalOrdering, ASTHelpers.getSymbol(tree), state);
if (originalOrdering.equals(sortedAnnotations)) {
return Description.NO_MATCH;
}
Optional<Fix> fix = tryFixOrdering(originalOrdering, sortedAnnotations, state);
Description.Builder description = buildDescription(originalOrdering.get(0));
fix.ifPresent(description::addFix);
return description.build();
return describeMatch(
originalOrdering.get(0), fixOrdering(originalOrdering, sortedAnnotations, state));
}
private static ImmutableList<? extends AnnotationTree> sort(
List<? extends AnnotationTree> annotations, VisitorState state) {
List<? extends AnnotationTree> annotations, Symbol symbol, VisitorState state) {
return annotations.stream()
.sorted(comparing(annotation -> SourceCode.treeToString(annotation, state)))
.sorted(
comparing(
(AnnotationTree annotation) ->
ASTHelpers.getAnnotationType(annotation, symbol, state),
BY_ANNOTATION_TYPE)
.thenComparing(annotation -> SourceCode.treeToString(annotation, state)))
.collect(toImmutableList());
}
private static Optional<Fix> tryFixOrdering(
private static Fix fixOrdering(
List<? extends AnnotationTree> originalAnnotations,
ImmutableList<? extends AnnotationTree> sortedAnnotations,
VisitorState state) {
@@ -75,6 +93,7 @@ public final class LexicographicalAnnotationListing extends BugChecker
SuggestedFix.builder()
.replace(original, SourceCode.treeToString(replacement, state)))
.reduce(SuggestedFix.Builder::merge)
.map(SuggestedFix.Builder::build);
.map(SuggestedFix.Builder::build)
.orElseThrow(() -> new VerifyException("No annotations were provided"));
}
}

View File

@@ -1,9 +1,10 @@
package tech.picnic.errorprone.bugpatterns;
import static com.google.common.collect.ImmutableList.toImmutableList;
import static com.google.errorprone.BugPattern.LinkType.NONE;
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.util.Documentation.BUG_PATTERNS_BASE_URL;
import com.google.auto.service.AutoService;
import com.google.common.base.VerifyException;
@@ -35,7 +36,7 @@ import java.util.Optional;
import javax.lang.model.element.Name;
/**
* A {@link BugChecker} which flags lambda expressions that can be replaced with method references.
* A {@link BugChecker} that flags lambda expressions that can be replaced with method references.
*/
// XXX: Other custom expressions we could rewrite:
// - `a -> "str" + a` to `"str"::concat`. But only if `str` is provably non-null.
@@ -52,12 +53,16 @@ import javax.lang.model.element.Name;
@AutoService(BugChecker.class)
@BugPattern(
summary = "Prefer method references over lambda expressions",
linkType = NONE,
link = BUG_PATTERNS_BASE_URL + "MethodReferenceUsage",
linkType = CUSTOM,
severity = SUGGESTION,
tags = STYLE)
public final class MethodReferenceUsage extends BugChecker implements LambdaExpressionTreeMatcher {
private static final long serialVersionUID = 1L;
/** Instantiates a new {@link MethodReferenceUsage} instance. */
public MethodReferenceUsage() {}
@Override
public Description matchLambdaExpression(LambdaExpressionTree tree, VisitorState state) {
/*
@@ -120,7 +125,7 @@ public final class MethodReferenceUsage extends BugChecker implements LambdaExpr
return Optional.empty();
}
Symbol sym = ASTHelpers.getSymbol(methodSelect);
if (!sym.isStatic()) {
if (!ASTHelpers.isStatic(sym)) {
return constructFix(lambdaExpr, "this", methodSelect);
}
return constructFix(lambdaExpr, sym.owner, methodSelect);
@@ -190,7 +195,7 @@ public final class MethodReferenceUsage extends BugChecker implements LambdaExpr
Name sName = target.getSimpleName();
Optional<SuggestedFix.Builder> fix = constructFix(lambdaExpr, sName, methodName);
if (!"java.lang".equals(target.packge().toString())) {
if (!"java.lang".equals(ASTHelpers.enclosingPackage(target).toString())) {
Name fqName = target.getQualifiedName();
if (!sName.equals(fqName)) {
return fix.map(b -> b.addImport(fqName.toString()));

View File

@@ -1,12 +1,13 @@
package tech.picnic.errorprone.bugpatterns;
import static com.google.errorprone.BugPattern.LinkType.NONE;
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;
@@ -24,8 +25,9 @@ import com.sun.source.tree.Tree;
/** A {@link BugChecker} that flags likely missing Refaster annotations. */
@AutoService(BugChecker.class)
@BugPattern(
summary = "The Refaster template contains a method without any Refaster annotations",
linkType = NONE,
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 {
@@ -38,6 +40,9 @@ public final class MissingRefasterAnnotation extends BugChecker implements Class
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 =

View File

@@ -1,9 +1,10 @@
package tech.picnic.errorprone.bugpatterns;
import static com.google.errorprone.BugPattern.LinkType.NONE;
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.staticMethod;
import static tech.picnic.errorprone.bugpatterns.util.Documentation.BUG_PATTERNS_BASE_URL;
import com.google.auto.service.AutoService;
import com.google.common.collect.Iterables;
@@ -20,13 +21,14 @@ import java.util.List;
import tech.picnic.errorprone.bugpatterns.util.SourceCode;
/**
* A {@link BugChecker} which flags method invocations for which all arguments are wrapped using
* A {@link BugChecker} that flags method invocations for which all arguments are wrapped using
* {@link org.mockito.Mockito#eq}; this is redundant.
*/
@AutoService(BugChecker.class)
@BugPattern(
summary = "Don't unnecessarily use Mockito's `eq(...)`",
linkType = NONE,
link = BUG_PATTERNS_BASE_URL + "MockitoStubbing",
linkType = CUSTOM,
severity = SUGGESTION,
tags = SIMPLIFICATION)
public final class MockitoStubbing extends BugChecker implements MethodInvocationTreeMatcher {
@@ -34,6 +36,9 @@ public final class MockitoStubbing extends BugChecker implements MethodInvocatio
private static final Matcher<ExpressionTree> MOCKITO_EQ_METHOD =
staticMethod().onClass("org.mockito.ArgumentMatchers").named("eq");
/** Instantiates a new {@link MockitoStubbing} instance. */
public MockitoStubbing() {}
@Override
public Description matchMethodInvocation(MethodInvocationTree tree, VisitorState state) {
List<? extends ExpressionTree> arguments = tree.getArguments();

View File

@@ -1,8 +1,9 @@
package tech.picnic.errorprone.bugpatterns;
import static com.google.errorprone.BugPattern.LinkType.NONE;
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 com.google.auto.service.AutoService;
import com.google.common.collect.Iterables;
@@ -20,18 +21,22 @@ import com.sun.tools.javac.code.Type;
import com.sun.tools.javac.util.List;
import java.util.Optional;
/** A {@link BugChecker} which flags nesting of {@link Optional Optionals}. */
/** A {@link BugChecker} that flags nesting of {@link Optional Optionals}. */
@AutoService(BugChecker.class)
@BugPattern(
summary =
"Avoid nesting `Optional`s inside `Optional`s; the resultant code is hard to reason about",
linkType = NONE,
link = BUG_PATTERNS_BASE_URL + "NestedOptionals",
linkType = CUSTOM,
severity = WARNING,
tags = FRAGILE_CODE)
public final class NestedOptionals extends BugChecker implements MethodInvocationTreeMatcher {
private static final long serialVersionUID = 1L;
private static final Supplier<Type> OPTIONAL = Suppliers.typeFromClass(Optional.class);
/** Instantiates a new {@link NestedOptionals} instance. */
public NestedOptionals() {}
@Override
public Description matchMethodInvocation(MethodInvocationTree tree, VisitorState state) {
return isOptionalOfOptional(tree, state) ? describeMatch(tree) : Description.NO_MATCH;

View File

@@ -1,10 +1,11 @@
package tech.picnic.errorprone.bugpatterns;
import static com.google.errorprone.BugPattern.LinkType.NONE;
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 com.google.errorprone.matchers.Matchers.anyOf;
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;
@@ -22,13 +23,14 @@ import reactor.core.publisher.Mono;
import tech.picnic.errorprone.bugpatterns.util.SourceCode;
/**
* A {@link BugChecker} which flags {@link Mono} operations that are known to be vacuous, given that
* A {@link BugChecker} that flags {@link Mono} operations that are known to be vacuous, given that
* they are invoked on a {@link Mono} that is known not to complete empty.
*/
@AutoService(BugChecker.class)
@BugPattern(
summary = "Avoid vacuous operations on known non-empty `Mono`s",
linkType = NONE,
link = BUG_PATTERNS_BASE_URL + "NonEmptyMono",
linkType = CUSTOM,
severity = WARNING,
tags = SIMPLIFICATION)
// XXX: This check does not simplify `someFlux.defaultIfEmpty(T).{defaultIfEmpty(T),hasElements()}`,
@@ -74,6 +76,9 @@ public final class NonEmptyMono extends BugChecker implements MethodInvocationTr
.onDescendantOf("reactor.core.publisher.Mono")
.namedAnyOf("defaultIfEmpty", "hasElement", "single"));
/** Instantiates a new {@link NonEmptyMono} instance. */
public NonEmptyMono() {}
@Override
public Description matchMethodInvocation(MethodInvocationTree tree, VisitorState state) {
if (!MONO_SIZE_CHECK.matches(tree, state)) {

View File

@@ -1,12 +1,13 @@
package tech.picnic.errorprone.bugpatterns;
import static com.google.errorprone.BugPattern.LinkType.NONE;
import static com.google.errorprone.BugPattern.LinkType.CUSTOM;
import static com.google.errorprone.BugPattern.SeverityLevel.WARNING;
import static com.google.errorprone.BugPattern.StandardTags.PERFORMANCE;
import static com.google.errorprone.matchers.Matchers.anyOf;
import static com.google.errorprone.matchers.method.MethodMatchers.instanceMethod;
import static com.google.errorprone.matchers.method.MethodMatchers.staticMethod;
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.VerifyException;
@@ -34,7 +35,7 @@ import java.util.stream.Stream;
import tech.picnic.errorprone.bugpatterns.util.SourceCode;
/**
* A {@link BugChecker} which flags {@code Comparator#comparing*} invocations that can be replaced
* A {@link BugChecker} that flags {@code Comparator#comparing*} invocations that can be replaced
* with an equivalent alternative so as to avoid unnecessary (un)boxing.
*/
// XXX: Add more documentation. Explain how this is useful in the face of refactoring to more
@@ -44,7 +45,8 @@ import tech.picnic.errorprone.bugpatterns.util.SourceCode;
summary =
"Ensure invocations of `Comparator#comparing{,Double,Int,Long}` match the return type"
+ " of the provided function",
linkType = NONE,
link = BUG_PATTERNS_BASE_URL + "PrimitiveComparison",
linkType = CUSTOM,
severity = WARNING,
tags = PERFORMANCE)
public final class PrimitiveComparison extends BugChecker implements MethodInvocationTreeMatcher {
@@ -68,6 +70,9 @@ public final class PrimitiveComparison extends BugChecker implements MethodInvoc
.named("thenComparing")
.withParameters(Function.class.getName()));
/** Instantiates a new {@link PrimitiveComparison} instance. */
public PrimitiveComparison() {}
@Override
public Description matchMethodInvocation(MethodInvocationTree tree, VisitorState state) {
boolean isStatic = STATIC_COMPARISON_METHOD.matches(tree, state);

View File

@@ -1,7 +1,7 @@
package tech.picnic.errorprone.bugpatterns;
import static com.google.common.collect.ImmutableSet.toImmutableSet;
import static com.google.errorprone.BugPattern.LinkType.NONE;
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;
@@ -14,6 +14,7 @@ import static com.google.errorprone.matchers.Matchers.isSubtypeOf;
import static com.google.errorprone.matchers.Matchers.not;
import static com.google.errorprone.matchers.method.MethodMatchers.instanceMethod;
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;
@@ -51,11 +52,12 @@ import java.util.stream.Stream;
import tech.picnic.errorprone.bugpatterns.util.MethodMatcherFactory;
import tech.picnic.errorprone.bugpatterns.util.SourceCode;
/** A {@link BugChecker} which flags redundant explicit string conversions. */
/** A {@link BugChecker} that flags redundant explicit string conversions. */
@AutoService(BugChecker.class)
@BugPattern(
summary = "Avoid redundant string conversions when possible",
linkType = NONE,
link = BUG_PATTERNS_BASE_URL + "RedundantStringConversion",
linkType = CUSTOM,
severity = SUGGESTION,
tags = SIMPLIFICATION)
public final class RedundantStringConversion extends BugChecker
@@ -139,7 +141,7 @@ public final class RedundantStringConversion extends BugChecker
private final Matcher<MethodInvocationTree> conversionMethodMatcher;
/** Instantiates the default {@link RedundantStringConversion}. */
/** Instantiates a default {@link RedundantStringConversion} instance. */
public RedundantStringConversion() {
this(ErrorProneFlags.empty());
}
@@ -224,7 +226,7 @@ public final class RedundantStringConversion extends BugChecker
.flatMap(args -> tryFix(args.get(index), state, ANY_EXPR));
}
// XXX: Write another check which checks that Formatter patterns don't use `{}` and have a
// XXX: Write another check that checks that Formatter patterns don't use `{}` and have a
// matching number of arguments of the appropriate type. Also flag explicit conversions from
// `Formattable` to string.
private Optional<SuggestedFix.Builder> tryFixFormatter(
@@ -255,7 +257,7 @@ public final class RedundantStringConversion extends BugChecker
return tryFixFormatterArguments(arguments, state, ANY_EXPR, ANY_EXPR);
}
// XXX: Write another check which checks that SLF4J patterns don't use `%s` and have a matching
// XXX: Write another check that checks that SLF4J patterns don't use `%s` and have a matching
// number of arguments of the appropriate type. Also flag explicit conversions from `Throwable` to
// string as the last logger argument. Suggests either dropping the conversion or going with
// `Throwable#getMessage()` instead.

View File

@@ -1,9 +1,10 @@
package tech.picnic.errorprone.bugpatterns;
import static com.google.errorprone.BugPattern.LinkType.NONE;
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.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;
@@ -19,15 +20,16 @@ import com.sun.source.tree.MethodInvocationTree;
import tech.picnic.errorprone.bugpatterns.util.SourceCode;
/**
* A {@link BugChecker} which flags unnecessary {@link Refaster#anyOf(Object[])} usages.
* A {@link BugChecker} that flags unnecessary {@link Refaster#anyOf(Object[])} usages.
*
* <p>Note that this logic can't be implemented as a Refaster template, as the {@link Refaster}
* class is treated specially.
* <p>Note that this logic can't be implemented as a Refaster rule, as the {@link Refaster} class is
* treated specially.
*/
@AutoService(BugChecker.class)
@BugPattern(
summary = "`Refaster#anyOf` should be passed at least two parameters",
linkType = NONE,
link = BUG_PATTERNS_BASE_URL + "RefasterAnyOfUsage",
linkType = CUSTOM,
severity = SUGGESTION,
tags = SIMPLIFICATION)
public final class RefasterAnyOfUsage extends BugChecker implements MethodInvocationTreeMatcher {
@@ -35,6 +37,9 @@ public final class RefasterAnyOfUsage extends BugChecker implements MethodInvoca
private static final Matcher<ExpressionTree> REFASTER_ANY_OF =
staticMethod().onClass(Refaster.class.getName()).named("anyOf");
/** Instantiates a new {@link RefasterAnyOfUsage} instance. */
public RefasterAnyOfUsage() {}
@Override
public Description matchMethodInvocation(MethodInvocationTree tree, VisitorState state) {
if (REFASTER_ANY_OF.matches(tree, state)) {

View File

@@ -0,0 +1,116 @@
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.Matchers.anyOf;
import static com.google.errorprone.matchers.Matchers.hasAnnotation;
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.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.refaster.annotation.AfterTemplate;
import com.google.errorprone.refaster.annotation.BeforeTemplate;
import com.google.errorprone.refaster.annotation.Placeholder;
import com.google.errorprone.util.ASTHelpers;
import com.sun.source.tree.ClassTree;
import com.sun.source.tree.MethodTree;
import com.sun.source.tree.Tree;
import java.util.EnumSet;
import java.util.Set;
import javax.lang.model.element.Modifier;
/**
* A {@link BugChecker} which suggests a canonical set of modifiers for Refaster class and method
* definitions.
*/
@AutoService(BugChecker.class)
@BugPattern(
summary = "Refaster class and method definitions should specify a canonical set of modifiers",
link = BUG_PATTERNS_BASE_URL + "RefasterRuleModifiers",
linkType = CUSTOM,
severity = SUGGESTION,
tags = STYLE)
public final class RefasterRuleModifiers extends BugChecker
implements ClassTreeMatcher, MethodTreeMatcher {
private static final long serialVersionUID = 1L;
private static final Matcher<Tree> BEFORE_TEMPLATE_METHOD = hasAnnotation(BeforeTemplate.class);
private static final Matcher<Tree> AFTER_TEMPLATE_METHOD = hasAnnotation(AfterTemplate.class);
private static final Matcher<Tree> PLACEHOLDER_METHOD = hasAnnotation(Placeholder.class);
private static final Matcher<Tree> REFASTER_METHOD =
anyOf(BEFORE_TEMPLATE_METHOD, AFTER_TEMPLATE_METHOD, PLACEHOLDER_METHOD);
/** Instantiates a new {@link RefasterRuleModifiers} instance. */
public RefasterRuleModifiers() {}
@Override
public Description matchClass(ClassTree tree, VisitorState state) {
if (!hasMatchingMember(tree, BEFORE_TEMPLATE_METHOD, state)) {
/* This class does not contain a Refaster template. */
return Description.NO_MATCH;
}
SuggestedFix fix = suggestCanonicalModifiers(tree, state);
return fix.isEmpty() ? Description.NO_MATCH : describeMatch(tree, fix);
}
@Override
public Description matchMethod(MethodTree tree, VisitorState state) {
if (!REFASTER_METHOD.matches(tree, state)) {
return Description.NO_MATCH;
}
return SuggestedFixes.removeModifiers(
tree,
state,
Modifier.FINAL,
Modifier.PRIVATE,
Modifier.PROTECTED,
Modifier.PUBLIC,
Modifier.STATIC,
Modifier.SYNCHRONIZED)
.map(fix -> describeMatch(tree, fix))
.orElse(Description.NO_MATCH);
}
private static SuggestedFix suggestCanonicalModifiers(ClassTree tree, VisitorState state) {
Set<Modifier> modifiersToAdd = EnumSet.noneOf(Modifier.class);
Set<Modifier> modifiersToRemove =
EnumSet.of(Modifier.PRIVATE, Modifier.PROTECTED, Modifier.PUBLIC, Modifier.SYNCHRONIZED);
if (!hasMatchingMember(tree, PLACEHOLDER_METHOD, state)) {
/*
* Rules without a `@Placeholder` method should be `final`. Note that Refaster enforces
* that `@Placeholder` methods are `abstract`, so rules _with_ such a method will
* naturally be `abstract` and non-`final`.
*/
modifiersToAdd.add(Modifier.FINAL);
modifiersToRemove.add(Modifier.ABSTRACT);
}
if (ASTHelpers.findEnclosingNode(state.getPath(), ClassTree.class) != null) {
/* Nested classes should be `static`. */
modifiersToAdd.add(Modifier.STATIC);
}
SuggestedFix.Builder fix = SuggestedFix.builder();
SuggestedFixes.addModifiers(tree, tree.getModifiers(), state, modifiersToAdd)
.ifPresent(fix::merge);
SuggestedFixes.removeModifiers(tree.getModifiers(), state, modifiersToRemove)
.ifPresent(fix::merge);
return fix.build();
}
private static boolean hasMatchingMember(
ClassTree tree, Matcher<Tree> matcher, VisitorState state) {
return tree.getMembers().stream().anyMatch(member -> matcher.matches(member, state));
}
}

View File

@@ -1,6 +1,6 @@
package tech.picnic.errorprone.bugpatterns;
import static com.google.errorprone.BugPattern.LinkType.NONE;
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.ALL;
@@ -11,6 +11,7 @@ import static com.google.errorprone.matchers.Matchers.isSameType;
import static com.google.errorprone.matchers.Matchers.isType;
import static com.google.errorprone.matchers.Matchers.methodHasParameters;
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.errorprone.BugPattern;
@@ -23,7 +24,7 @@ import com.sun.source.tree.MethodTree;
import com.sun.source.tree.Tree;
/**
* A {@link BugChecker} which flags {@code @RequestMapping} methods that have one or more parameters
* A {@link BugChecker} that flags {@code @RequestMapping} methods that have one or more parameters
* that appear to lack a relevant annotation.
*
* <p>Matched mappings are {@code @{Delete,Get,Patch,Post,Put,Request}Mapping}.
@@ -31,7 +32,8 @@ import com.sun.source.tree.Tree;
@AutoService(BugChecker.class)
@BugPattern(
summary = "Make sure all `@RequestMapping` method parameters are annotated",
linkType = NONE,
link = BUG_PATTERNS_BASE_URL + "RequestMappingAnnotation",
linkType = CUSTOM,
severity = WARNING,
tags = LIKELY_ERROR)
public final class RequestMappingAnnotation extends BugChecker implements MethodTreeMatcher {
@@ -81,6 +83,9 @@ public final class RequestMappingAnnotation extends BugChecker implements Method
isSameType("org.springframework.web.util.UriBuilder"),
isSameType("org.springframework.web.util.UriComponentsBuilder"))));
/** Instantiates a new {@link RequestMappingAnnotation} instance. */
public RequestMappingAnnotation() {}
@Override
public Description matchMethod(MethodTree tree, VisitorState state) {
// XXX: Auto-add `@RequestParam` where applicable.

View File

@@ -1,6 +1,6 @@
package tech.picnic.errorprone.bugpatterns;
import static com.google.errorprone.BugPattern.LinkType.NONE;
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;
@@ -9,6 +9,7 @@ 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 tech.picnic.errorprone.bugpatterns.util.Documentation.BUG_PATTERNS_BASE_URL;
import com.google.auto.service.AutoService;
import com.google.common.collect.ImmutableCollection;
@@ -21,11 +22,12 @@ import com.google.errorprone.matchers.Description;
import com.google.errorprone.matchers.Matcher;
import com.sun.source.tree.VariableTree;
/** A {@link BugChecker} which flags {@code @RequestParam} parameters with an unsupported type. */
/** A {@link BugChecker} that flags {@code @RequestParam} parameters with an unsupported type. */
@AutoService(BugChecker.class)
@BugPattern(
summary = "`@RequestParam` does not support `ImmutableCollection` and `ImmutableMap` subtypes",
linkType = NONE,
link = BUG_PATTERNS_BASE_URL + "RequestParamType",
linkType = CUSTOM,
severity = ERROR,
tags = LIKELY_ERROR)
public final class RequestParamType extends BugChecker implements VariableTreeMatcher {
@@ -35,6 +37,9 @@ public final class RequestParamType extends BugChecker implements VariableTreeMa
annotations(AT_LEAST_ONE, isType("org.springframework.web.bind.annotation.RequestParam")),
anyOf(isSubtypeOf(ImmutableCollection.class), isSubtypeOf(ImmutableMap.class)));
/** Instantiates a new {@link RequestParamType} instance. */
public RequestParamType() {}
@Override
public Description matchVariable(VariableTree tree, VisitorState state) {
return HAS_UNSUPPORTED_REQUEST_PARAM.matches(tree, state)

View File

@@ -1,12 +1,13 @@
package tech.picnic.errorprone.bugpatterns;
import static com.google.errorprone.BugPattern.LinkType.NONE;
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;
@@ -27,13 +28,14 @@ import com.sun.source.tree.MethodTree;
import com.sun.source.tree.Tree;
/**
* A {@link BugChecker} which flags methods with Spring's {@code @Scheduled} annotation that lack
* New Relic Agent's {@code @Trace(dispatcher = true)}.
* 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",
linkType = NONE,
link = BUG_PATTERNS_BASE_URL + "ScheduledTransactionTrace",
linkType = CUSTOM,
severity = ERROR,
tags = LIKELY_ERROR)
public final class ScheduledTransactionTrace extends BugChecker implements MethodTreeMatcher {
@@ -44,6 +46,9 @@ public final class ScheduledTransactionTrace extends BugChecker implements Metho
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 (!IS_SCHEDULED.matches(tree, state)) {

View File

@@ -1,11 +1,12 @@
package tech.picnic.errorprone.bugpatterns;
import static com.google.common.base.Verify.verify;
import static com.google.errorprone.BugPattern.LinkType.NONE;
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.Matchers.isSubtypeOf;
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.base.Splitter;
@@ -24,16 +25,18 @@ import java.util.List;
import java.util.Optional;
import tech.picnic.errorprone.bugpatterns.util.SourceCode;
/** A {@link BugChecker} which flags SLF4J usages that are likely to be in error. */
/** A {@link BugChecker} that flags SLF4J usages that are likely to be in error. */
// XXX: The special-casing of Throwable applies only to SLF4J 1.6.0+; see
// https://www.slf4j.org/faq.html#paramException. That should be documented.
// XXX: Also simplify `LOG.error(String.format("Something %s", arg), throwable)`.
// XXX: Also simplify `LOG.error(String.join("sep", arg1, arg2), throwable)`? Perhaps too obscure.
// XXX: Write a similar checker for Spring RestTemplates, String.format and friends, Guava
// preconditions, ...
@AutoService(BugChecker.class)
@BugPattern(
summary = "Make sure SLF4J log statements contain proper placeholders with matching arguments",
linkType = NONE,
link = BUG_PATTERNS_BASE_URL + "Slf4jLogStatement",
linkType = CUSTOM,
severity = WARNING,
tags = LIKELY_ERROR)
public final class Slf4jLogStatement extends BugChecker implements MethodInvocationTreeMatcher {
@@ -45,6 +48,9 @@ public final class Slf4jLogStatement extends BugChecker implements MethodInvocat
.onDescendantOf("org.slf4j.Logger")
.namedAnyOf("trace", "debug", "info", "warn", "error");
/** Instantiates a new {@link Slf4jLogStatement} instance. */
public Slf4jLogStatement() {}
@Override
public Description matchMethodInvocation(MethodInvocationTree tree, VisitorState state) {
if (!SLF4J_LOGGER_INVOCATION.matches(tree, state)) {

View File

@@ -1,11 +1,12 @@
package tech.picnic.errorprone.bugpatterns;
import static com.google.common.base.Verify.verify;
import static com.google.errorprone.BugPattern.LinkType.NONE;
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 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 com.google.auto.service.AutoService;
import com.google.common.base.VerifyException;
@@ -29,14 +30,15 @@ import tech.picnic.errorprone.bugpatterns.util.AnnotationAttributeMatcher;
import tech.picnic.errorprone.bugpatterns.util.SourceCode;
/**
* A {@link BugChecker} which flags {@code @RequestMapping} annotations that can be written more
* A {@link BugChecker} that flags {@code @RequestMapping} annotations that can be written more
* concisely.
*/
@AutoService(BugChecker.class)
@BugPattern(
summary =
"Prefer the conciseness of `@{Get,Put,Post,Delete,Patch}Mapping` over `@RequestMapping`",
linkType = NONE,
link = BUG_PATTERNS_BASE_URL + "SpringMvcAnnotation",
linkType = CUSTOM,
severity = SUGGESTION,
tags = SIMPLIFICATION)
public final class SpringMvcAnnotation extends BugChecker implements AnnotationTreeMatcher {
@@ -55,6 +57,9 @@ public final class SpringMvcAnnotation extends BugChecker implements AnnotationT
.put("PUT", "PutMapping")
.build();
/** Instantiates a new {@link SpringMvcAnnotation} instance. */
public SpringMvcAnnotation() {}
@Override
public Description matchAnnotation(AnnotationTree tree, VisitorState state) {
// XXX: We could remove the `@RequestMapping` import if not other usages remain.

View File

@@ -1,9 +1,10 @@
package tech.picnic.errorprone.bugpatterns;
import static com.google.errorprone.BugPattern.LinkType.NONE;
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 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.annotations.VisibleForTesting;
@@ -27,15 +28,14 @@ import com.sun.tools.javac.code.Type;
import java.util.Optional;
/**
* A {@link BugChecker} which flags methods and constants that can and should be statically
* imported.
* 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"}`
// XXX: Also introduce a check which disallows static imports of certain methods. Candidates:
// 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`
@@ -46,7 +46,8 @@ import java.util.Optional;
@AutoService(BugChecker.class)
@BugPattern(
summary = "Identifier should be statically imported",
linkType = NONE,
link = BUG_PATTERNS_BASE_URL + "StaticImport",
linkType = CUSTOM,
severity = SUGGESTION,
tags = SIMPLIFICATION)
public final class StaticImport extends BugChecker implements MemberSelectTreeMatcher {
@@ -189,6 +190,9 @@ public final class StaticImport extends BugChecker implements MemberSelectTreeMa
"of",
"valueOf");
/** Instantiates a new {@link StaticImport} instance. */
public StaticImport() {}
@Override
public Description matchMemberSelect(MemberSelectTree tree, VisitorState state) {
if (!isCandidateContext(state) || !isCandidate(tree)) {

View File

@@ -0,0 +1,185 @@
package tech.picnic.errorprone.bugpatterns;
import static com.google.errorprone.BugPattern.LinkType.NONE;
import static com.google.errorprone.BugPattern.SeverityLevel.SUGGESTION;
import static com.google.errorprone.BugPattern.StandardTags.SIMPLIFICATION;
import static com.google.errorprone.matchers.method.MethodMatchers.staticMethod;
import com.google.auto.service.AutoService;
import com.google.common.base.Splitter;
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.MethodInvocationTreeMatcher;
import com.google.errorprone.fixes.SuggestedFix;
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.Convert;
import java.util.Formattable;
import java.util.Iterator;
import java.util.List;
import org.jspecify.nullness.Nullable;
import tech.picnic.errorprone.bugpatterns.util.SourceCode;
/**
* A {@link BugChecker} which flags {@link String#format(String, Object...)} invocations which can
* be replaced with a {@link String#join(CharSequence, CharSequence...)} or even a {@link
* String#valueOf} invocation.
*/
// XXX: What about `v1 + "sep" + v2` and similar expressions? Do we want to rewrite those to
// `String.join`, or should some `String.join` invocations be rewritten to use the `+` operator?
// (The latter suggestion would conflict with the `FormatStringConcatenation` check.)
@AutoService(BugChecker.class)
@BugPattern(
summary = "Prefer `String#join` over `String#format`",
linkType = NONE,
severity = SUGGESTION,
tags = SIMPLIFICATION)
public final class StringJoin extends BugChecker implements MethodInvocationTreeMatcher {
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.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);
/** Instantiates a new {@link StringJoin} instance. */
public StringJoin() {}
@Override
public Description matchMethodInvocation(MethodInvocationTree tree, VisitorState state) {
if (!STRING_FORMAT_INVOCATION.matches(tree, state)) {
return Description.NO_MATCH;
}
// XXX: This check assumes that if the first argument to `String#format` is a `Locale`, that
// this argument is not vacuous, and that as a result the expression cannot be simplified using
// `#valueOf` or `#join`. Implement a separate check that identifies and drops redundant
// `Locale` arguments. See also a related comment in `FormatStringConcatenation`.
String formatString = ASTHelpers.constValue(tree.getArguments().get(0), String.class);
if (formatString == null) {
return Description.NO_MATCH;
}
List<String> separators = FORMAT_SPECIFIER_SPLITTER.splitToList(formatString);
if (separators.size() < 2) {
/* The format string does not contain `%s` format specifiers. */
return Description.NO_MATCH;
}
if (separators.size() != tree.getArguments().size()) {
/* The number of arguments does not match the number of `%s` format specifiers. */
return Description.NO_MATCH;
}
int lastIndex = separators.size() - 1;
if (!separators.get(0).isEmpty() || !separators.get(lastIndex).isEmpty()) {
/* The format string contains leading or trailing characters. */
return Description.NO_MATCH;
}
ImmutableSet<String> innerSeparators = ImmutableSet.copyOf(separators.subList(1, lastIndex));
if (innerSeparators.size() > 1) {
/* The `%s` format specifiers are not uniformly separated. */
return Description.NO_MATCH;
}
if (innerSeparators.isEmpty()) {
/*
* This `String#format` invocation performs a straightforward string conversion; use
* `String#valueOf` instead.
*/
return trySuggestExplicitStringConversion(tree, state);
}
String separator = Iterables.getOnlyElement(innerSeparators);
if (separator.indexOf('%') >= 0) {
/* The `%s` format specifiers are separated by another format specifier. */
// XXX: Strictly speaking we could support `%%` by mapping it to a literal `%`, but that
// doesn't seem worth the trouble.
return Description.NO_MATCH;
}
return trySuggestExplicitJoin(tree, separator, state);
}
/**
* If guaranteed to be behavior preserving, suggests replacing {@code String.format("%s", arg)}
* with {@code String.valueOf(arg)}.
*
* <p>If {@code arg} is already a string then the resultant conversion is vacuous. The {@link
* IdentityConversion} check will subsequently drop it.
*/
private Description trySuggestExplicitStringConversion(
MethodInvocationTree tree, VisitorState state) {
ExpressionTree argument = tree.getArguments().get(1);
if (isSubtype(ASTHelpers.getType(argument), FORMATTABLE_TYPE, state)) {
/*
* `Formattable` arguments are handled specially; `String#valueOf` is not a suitable
* alternative.
*/
return Description.NO_MATCH;
}
return buildDescription(tree)
.setMessage("Prefer `String#valueOf` over `String#format`")
.addFix(SuggestedFix.replace(tree, withStringConversionExpression(argument, state)))
.build();
}
/**
* Unless the given {@code String.format} expression includes {@link Formattable} arguments,
* suggests replacing it with a {@code String.join} expression using the specified argument
* separator.
*/
private Description trySuggestExplicitJoin(
MethodInvocationTree tree, String separator, VisitorState state) {
Iterator<? extends ExpressionTree> arguments = tree.getArguments().iterator();
SuggestedFix.Builder fix =
SuggestedFix.builder()
.replace(tree.getMethodSelect(), "String.join")
.replace(arguments.next(), String.format("\"%s\"", Convert.quote(separator)));
while (arguments.hasNext()) {
ExpressionTree argument = arguments.next();
Type argumentType = ASTHelpers.getType(argument);
if (isSubtype(argumentType, FORMATTABLE_TYPE, state)) {
/*
* `Formattable` arguments are handled specially; `String#join` is not a suitable
* alternative.
*/
return Description.NO_MATCH;
}
if (!isSubtype(argumentType, CHAR_SEQUENCE_TYPE, state)) {
/*
* The argument was previously implicitly converted to a string; now this must happen
* explicitly.
*/
fix.replace(argument, withStringConversionExpression(argument, state));
}
}
return describeMatch(tree, fix.build());
}
private static boolean isSubtype(
@Nullable Type subType, Supplier<Type> superType, VisitorState state) {
return ASTHelpers.isSubtype(subType, superType.get(state), state);
}
private static String withStringConversionExpression(
ExpressionTree argument, VisitorState state) {
return String.format("String.valueOf(%s)", SourceCode.treeToString(argument, state));
}
}

View File

@@ -0,0 +1,57 @@
package tech.picnic.errorprone.bugpatterns;
import static com.google.errorprone.BugPattern.LinkType.NONE;
import static com.google.errorprone.BugPattern.SeverityLevel.WARNING;
import static com.google.errorprone.BugPattern.StandardTags.REFACTORING;
import static com.google.errorprone.matchers.Matchers.isType;
import com.google.auto.service.AutoService;
import com.google.common.collect.ImmutableMap;
import com.google.errorprone.BugPattern;
import com.google.errorprone.VisitorState;
import com.google.errorprone.bugpatterns.BugChecker;
import com.google.errorprone.bugpatterns.BugChecker.MethodTreeMatcher;
import com.google.errorprone.fixes.SuggestedFix;
import com.google.errorprone.matchers.Description;
import com.google.errorprone.matchers.Matcher;
import com.google.errorprone.util.ASTHelpers;
import com.sun.source.tree.AnnotationTree;
import com.sun.source.tree.MethodTree;
/**
* A {@link BugChecker} that replaces TestNG annotations with their JUnit counterpart, if one exists
*/
@AutoService(BugChecker.class)
@BugPattern(
summary = "Migrate TestNG test annotations to JUnit",
linkType = NONE,
severity = WARNING,
tags = REFACTORING)
public final class TestNGAnnotation extends BugChecker implements MethodTreeMatcher {
private static final long serialVersionUID = 1L;
private static final ImmutableMap<Matcher<AnnotationTree>, String>
TESTNG_ANNOTATION_REPLACEMENTS =
ImmutableMap.<Matcher<AnnotationTree>, String>builder()
.put(isType("org.testng.annotations.AfterClass"), "@org.junit.jupiter.api.AfterAll")
.put(isType("org.testng.annotations.AfterMethod"), "@org.junit.jupiter.api.AfterEach")
.put(isType("org.testng.annotations.BeforeClass"), "@org.junit.jupiter.api.BeforeAll")
.put(
isType("org.testng.annotations.BeforeMethod"),
"@org.junit.jupiter.api.BeforeEach")
.put(isType("org.testng.annotations.Test"), "@org.junit.jupiter.api.Test")
.build();
@Override
public Description matchMethod(MethodTree tree, VisitorState state) {
SuggestedFix.Builder fix = SuggestedFix.builder();
ASTHelpers.getAnnotations(tree).stream()
.filter(annotation -> annotation.getArguments().isEmpty())
.forEach(
annotation ->
TESTNG_ANNOTATION_REPLACEMENTS.entrySet().stream()
.filter(entry -> entry.getKey().matches(annotation, state))
.forEach(entry -> fix.replace(annotation, entry.getValue())));
return fix.isEmpty() ? Description.NO_MATCH : describeMatch(tree, fix.build());
}
}

View File

@@ -0,0 +1,85 @@
package tech.picnic.errorprone.bugpatterns;
import static com.google.errorprone.BugPattern.LinkType.NONE;
import static com.google.errorprone.BugPattern.SeverityLevel.ERROR;
import static com.google.errorprone.BugPattern.StandardTags.REFACTORING;
import static com.google.errorprone.matchers.Matchers.allOf;
import static com.google.errorprone.matchers.Matchers.anyOf;
import static com.google.errorprone.matchers.Matchers.hasAnnotation;
import static com.google.errorprone.matchers.Matchers.isType;
import static com.google.errorprone.matchers.Matchers.methodHasVisibility;
import static com.google.errorprone.matchers.Matchers.not;
import static com.google.errorprone.matchers.MethodVisibility.Visibility.PUBLIC;
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.fixes.SuggestedFix;
import com.google.errorprone.matchers.Description;
import com.google.errorprone.matchers.Matcher;
import com.google.errorprone.util.ASTHelpers;
import com.sun.source.tree.AnnotationTree;
import com.sun.source.tree.ClassTree;
import com.sun.source.tree.MethodTree;
import java.util.Optional;
import java.util.function.Predicate;
import tech.picnic.errorprone.bugpatterns.util.SourceCode;
/** A {@link BugChecker} which flags class level `@Test` annotations from TestNG. */
@AutoService(BugChecker.class)
@BugPattern(
summary = "A bug pattern to migrate TestNG Test annotations to methods",
linkType = NONE,
tags = REFACTORING,
severity = ERROR)
public final class TestNGClassLevelTestAnnotation extends BugChecker implements ClassTreeMatcher {
private static final long serialVersionUID = 1L;
private static final Matcher<ClassTree> CLASS_TREE = hasAnnotation("org.testng.annotations.Test");
private static final Matcher<MethodTree> UNMIGRATED_TESTNG_TEST_METHOD =
allOf(
methodHasVisibility(PUBLIC),
not(
anyOf(
hasAnnotation("org.testng.annotations.Test"),
hasAnnotation("org.testng.annotations.AfterClass"),
hasAnnotation("org.testng.annotations.AfterMethod"),
hasAnnotation("org.testng.annotations.BeforeClass"),
hasAnnotation("org.testng.annotations.BeforeMethod"))));
private static final Matcher<AnnotationTree> TESTNG_ANNOTATION =
isType("org.testng.annotations.Test");
@Override
public Description matchClass(ClassTree tree, VisitorState state) {
if (!CLASS_TREE.matches(tree, state)) {
return Description.NO_MATCH;
}
Optional<? extends AnnotationTree> testAnnotation =
ASTHelpers.getAnnotations(tree).stream()
.filter(annotation -> TESTNG_ANNOTATION.matches(annotation, state))
.findFirst();
if (testAnnotation.isEmpty()) {
return Description.NO_MATCH;
}
SuggestedFix.Builder fix = SuggestedFix.builder();
tree.getMembers().stream()
.filter(MethodTree.class::isInstance)
.map(MethodTree.class::cast)
.filter(method -> UNMIGRATED_TESTNG_TEST_METHOD.matches(method, state))
.filter(Predicate.not(ASTHelpers::isGeneratedConstructor))
.forEach(
methodTree ->
fix.merge(
SuggestedFix.prefixWith(
methodTree,
String.format(
"%s\n", SourceCode.treeToString(testAnnotation.get(), state)))));
fix.delete(testAnnotation.get());
return describeMatch(testAnnotation.get(), fix.build());
}
}

View File

@@ -0,0 +1,210 @@
package tech.picnic.errorprone.bugpatterns;
import static com.google.errorprone.BugPattern.LinkType.NONE;
import static com.google.errorprone.BugPattern.SeverityLevel.ERROR;
import static com.google.errorprone.BugPattern.StandardTags.REFACTORING;
import static com.google.errorprone.matchers.Matchers.isType;
import static com.sun.source.tree.Tree.Kind.NEW_ARRAY;
import static java.util.stream.Collectors.joining;
import static java.util.stream.Collectors.toMap;
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.MethodTreeMatcher;
import com.google.errorprone.fixes.SuggestedFix;
import com.google.errorprone.matchers.Description;
import com.google.errorprone.matchers.Matcher;
import com.google.errorprone.util.ASTHelpers;
import com.google.errorprone.util.ErrorProneToken;
import com.sun.source.tree.AnnotationTree;
import com.sun.source.tree.ClassTree;
import com.sun.source.tree.ExpressionTree;
import com.sun.source.tree.IdentifierTree;
import com.sun.source.tree.MethodTree;
import com.sun.source.tree.NewArrayTree;
import com.sun.source.tree.ReturnTree;
import com.sun.tools.javac.code.Symbol.ClassSymbol;
import com.sun.tools.javac.parser.Tokens.Comment;
import com.sun.tools.javac.util.Name;
import java.util.HashMap;
import java.util.List;
import java.util.Map;
import java.util.Optional;
import tech.picnic.errorprone.bugpatterns.util.SourceCode;
/**
* A {@link BugChecker} which flags TestNG {@link org.testng.annotations.DataProvider} methods and
* provides an equivalent JUnit {@link org.junit.jupiter.params.ParameterizedTest} replacement.
*/
@AutoService(BugChecker.class)
@BugPattern(
summary = "Migrate TestNG DataProvider to JUnit argument streams",
linkType = NONE,
tags = REFACTORING,
severity = ERROR)
public final class TestNGDataProvider extends BugChecker implements MethodTreeMatcher {
private static final long serialVersionUID = 1L;
private static final Matcher<AnnotationTree> TESTNG_DATAPROVIDER_ANNOTATION =
isType("org.testng.annotations.DataProvider");
@Override
public Description matchMethod(MethodTree tree, VisitorState state) {
Optional<? extends AnnotationTree> dataProviderAnnotation =
ASTHelpers.getAnnotations(tree).stream()
.filter(annotation -> TESTNG_DATAPROVIDER_ANNOTATION.matches(annotation, state))
.findFirst();
if (dataProviderAnnotation.isEmpty()) {
return Description.NO_MATCH;
}
String methodName = tree.getName().toString();
Name migratedName = state.getName(methodName + "Junit");
ClassTree classTree = state.findEnclosing(ClassTree.class);
if (classTree == null
|| isMethodAlreadyMigratedInEnclosingClass(ASTHelpers.getSymbol(classTree), migratedName)) {
return Description.NO_MATCH;
}
ReturnTree returnTree = getReturnTree(tree);
Optional<NewArrayTree> returnArrayTree = getDataProviderReturnTree(returnTree);
if (returnArrayTree.isEmpty()) {
return Description.NO_MATCH;
}
return describeMatch(
dataProviderAnnotation.get(),
SuggestedFix.builder()
.addStaticImport("org.junit.jupiter.params.provider.Arguments.arguments")
.addImport("java.util.stream.Stream")
.addImport("org.junit.jupiter.params.provider.Arguments")
.merge(
SuggestedFix.postfixWith(
tree,
buildMethodSource(
classTree.getSimpleName().toString(),
migratedName.toString(),
tree,
returnTree,
returnArrayTree.orElseThrow(),
state)))
.build());
}
private static boolean isMethodAlreadyMigratedInEnclosingClass(
ClassSymbol enclosingClassSymbol, Name methodName) {
return enclosingClassSymbol.members().getSymbolsByName(methodName).iterator().hasNext();
}
private static ReturnTree getReturnTree(MethodTree methodTree) {
return methodTree.getBody().getStatements().stream()
.filter(ReturnTree.class::isInstance)
.findFirst()
.map(ReturnTree.class::cast)
.orElseThrow();
}
private static Optional<NewArrayTree> getDataProviderReturnTree(ReturnTree returnTree) {
if (returnTree.getExpression().getKind() != NEW_ARRAY
|| ((NewArrayTree) returnTree.getExpression()).getInitializers().isEmpty()) {
return Optional.empty();
}
return Optional.of((NewArrayTree) returnTree.getExpression());
}
private static String buildMethodSource(
String className,
String name,
MethodTree methodTree,
ReturnTree returnTree,
NewArrayTree newArrayTree,
VisitorState state) {
StringBuilder sourceBuilder =
new StringBuilder(
"@SuppressWarnings(\"UnusedMethod\" /* This is an intermediate state for the JUnit migration. */)\n")
.append(" private static Stream<Arguments> ")
.append(name)
.append(" () ");
if (!methodTree.getThrows().isEmpty()) {
sourceBuilder
.append(" throws ")
.append(
methodTree.getThrows().stream()
.filter(IdentifierTree.class::isInstance)
.map(IdentifierTree.class::cast)
.map(identifierTree -> identifierTree.getName().toString())
.collect(joining(", ")));
}
return sourceBuilder
.append(" {\n")
.append(extractMethodBodyWithoutReturnStatement(methodTree, returnTree, state))
.append(" return ")
.append(buildArgumentStream(className, newArrayTree, state))
.append(";\n}")
.toString();
}
private static String extractMethodBodyWithoutReturnStatement(
MethodTree methodTree, ReturnTree returnTree, VisitorState state) {
String body = SourceCode.treeToString(methodTree.getBody(), state);
return body.substring(2, body.indexOf(SourceCode.treeToString(returnTree, state)) - 1);
}
private static String buildArgumentStream(
String className, NewArrayTree newArrayTree, VisitorState state) {
StringBuilder argumentsBuilder = new StringBuilder();
int startPos = ASTHelpers.getStartPosition(newArrayTree);
int endPos = state.getEndPosition(newArrayTree);
Map<Integer, List<Comment>> comments =
state.getOffsetTokens(startPos, endPos).stream()
.collect(
toMap(ErrorProneToken::pos, ErrorProneToken::comments, (a, b) -> b, HashMap::new));
argumentsBuilder.append(
newArrayTree.getInitializers().stream()
.map(
expression ->
buildArguments(
expression,
comments.getOrDefault(
ASTHelpers.getStartPosition(expression), ImmutableList.of()),
state))
.collect(joining(",\n")));
// This regex expression replaces all instances of "this.getClass()" or "getClass()"
// with the fully qualified class name to retain functionality in static context.
return String.format("Stream.of(\n%s\n )", argumentsBuilder)
.replaceAll("((?<!\\b\\.)|(\\bthis\\.))(getClass\\(\\))", className + ".class");
}
private static String buildArguments(
ExpressionTree expressionTree, List<Comment> comments, VisitorState state) {
if (expressionTree.getKind() == NEW_ARRAY) {
return buildArgumentsFromArray(((NewArrayTree) expressionTree), comments, state);
} else {
return buildArgumentsFromExpression(expressionTree, comments, state);
}
}
private static String buildArgumentsFromExpression(
ExpressionTree expressionTree, List<Comment> comments, VisitorState state) {
return String.format(
"\t\t%s\n\t\targuments(%s)",
comments.stream().map(Comment::getText).collect(joining("\n")),
SourceCode.treeToString(expressionTree, state));
}
private static String buildArgumentsFromArray(
NewArrayTree argumentArray, List<Comment> comments, VisitorState state) {
String argSource = SourceCode.treeToString(argumentArray, state);
return String.format(
"\t\t%s\n\t\targuments(%s)",
comments.stream().map(Comment::getText).collect(joining("\n")),
argSource.substring(1, argSource.length() - 1));
}
}

View File

@@ -0,0 +1,164 @@
package tech.picnic.errorprone.bugpatterns;
import static com.google.auto.common.MoreStreams.toImmutableList;
import static com.google.errorprone.BugPattern.LinkType.NONE;
import static com.google.errorprone.BugPattern.SeverityLevel.ERROR;
import static com.google.errorprone.BugPattern.StandardTags.REFACTORING;
import static com.google.errorprone.matchers.Matchers.isType;
import static com.sun.source.tree.Tree.Kind.MEMBER_SELECT;
import static com.sun.source.tree.Tree.Kind.NEW_ARRAY;
import static java.util.stream.Collectors.joining;
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.MethodTreeMatcher;
import com.google.errorprone.fixes.SuggestedFix;
import com.google.errorprone.matchers.Description;
import com.google.errorprone.matchers.Matcher;
import com.google.errorprone.util.ASTHelpers;
import com.sun.source.tree.AnnotationTree;
import com.sun.source.tree.AssignmentTree;
import com.sun.source.tree.BlockTree;
import com.sun.source.tree.ExpressionTree;
import com.sun.source.tree.MethodTree;
import com.sun.source.tree.NewArrayTree;
import java.util.Optional;
import org.junit.jupiter.api.function.Executable;
import org.testng.annotations.Test;
import tech.picnic.errorprone.bugpatterns.util.SourceCode;
/**
* A {@link BugChecker} which flags {@link Test#expectedExceptions()} and suggests a JUnit
* equivalent replacement.
*
* <p>The method body is wrapped in a {@link org.junit.jupiter.api.Assertions#assertThrows(Class,
* Executable)} statement.
*
* <p>This {@link BugChecker} does not support migrating more than one exception and will therefore
* omit extra {@code expectedExceptions}. As this is not behavior preserving, a note with
* explanation is added in a comment.
*/
@AutoService(BugChecker.class)
@BugPattern(
summary = "Migrate TestNG expected exceptions to JUnit",
linkType = NONE,
tags = REFACTORING,
severity = ERROR)
public final class TestNGExpectedExceptions extends BugChecker implements MethodTreeMatcher {
private static final long serialVersionUID = 1L;
private static final Matcher<AnnotationTree> TESTNG_ANNOTATION =
isType("org.testng.annotations.Test");
@Override
public Description matchMethod(MethodTree tree, VisitorState state) {
Optional<? extends AnnotationTree> testAnnotation =
ASTHelpers.getAnnotations(tree).stream()
.filter(annotation -> TESTNG_ANNOTATION.matches(annotation, state))
.findFirst();
if (testAnnotation.isEmpty()) {
return Description.NO_MATCH;
}
Optional<AssignmentTree> assignmentTree =
testAnnotation.get().getArguments().stream()
.filter(AssignmentTree.class::isInstance)
.map(AssignmentTree.class::cast)
.filter(
assignment ->
SourceCode.treeToString(assignment.getVariable(), state)
.equals("expectedExceptions"))
.findFirst();
if (assignmentTree.isEmpty()) {
return Description.NO_MATCH;
}
ExpressionTree argumentExpression = assignmentTree.orElseThrow().getExpression();
if (argumentExpression == null) {
return Description.NO_MATCH;
}
Optional<String> expectedException = getExpectedException(argumentExpression, state);
if (expectedException.isEmpty()) {
return Description.NO_MATCH;
}
SuggestedFix.Builder fix =
SuggestedFix.builder()
.replace(
tree.getBody(),
buildWrappedBody(tree.getBody(), expectedException.orElseThrow(), state))
.replace(
testAnnotation.get(),
buildAnnotationReplacementSource(
testAnnotation.get(), assignmentTree.orElseThrow(), state));
ImmutableList<String> removedExceptions = getRemovedExceptions(argumentExpression, state);
if (!removedExceptions.isEmpty()) {
fix.prefixWith(
testAnnotation.get(),
String.format(
"// XXX: Removed handling of `%s` because this migration doesn't support it.\n",
String.join(", ", removedExceptions)));
}
return describeMatch(testAnnotation.get(), fix.build());
}
private static String buildAnnotationReplacementSource(
AnnotationTree annotationTree, AssignmentTree argumentToRemove, VisitorState state) {
StringBuilder replacement = new StringBuilder();
replacement.append(
String.format("@%s", SourceCode.treeToString(annotationTree.getAnnotationType(), state)));
String arguments =
annotationTree.getArguments().stream()
.filter(argument -> !argument.equals(argumentToRemove))
.map(argument -> SourceCode.treeToString(argument, state))
.collect(joining(", "));
if (!arguments.isEmpty()) {
replacement.append(String.format("(%s)", arguments));
}
return replacement.toString();
}
private static Optional<String> getExpectedException(
ExpressionTree expectedExceptions, VisitorState state) {
if (expectedExceptions.getKind() == NEW_ARRAY) {
NewArrayTree arrayTree = (NewArrayTree) expectedExceptions;
if (arrayTree.getInitializers().isEmpty()) {
return Optional.empty();
}
return Optional.of(SourceCode.treeToString(arrayTree.getInitializers().get(0), state));
} else if (expectedExceptions.getKind() == MEMBER_SELECT) {
return Optional.of(SourceCode.treeToString(expectedExceptions, state));
}
return Optional.empty();
}
private static ImmutableList<String> getRemovedExceptions(
ExpressionTree expectedExceptions, VisitorState state) {
if (expectedExceptions.getKind() != NEW_ARRAY) {
return ImmutableList.of();
}
NewArrayTree arrayTree = (NewArrayTree) expectedExceptions;
if (arrayTree.getInitializers().size() <= 1) {
return ImmutableList.of();
}
return arrayTree.getInitializers().subList(1, arrayTree.getInitializers().size()).stream()
.map(initializer -> SourceCode.treeToString(initializer, state))
.collect(toImmutableList());
}
private static String buildWrappedBody(BlockTree tree, String exception, VisitorState state) {
return String.format(
"{\norg.junit.jupiter.api.Assertions.assertThrows(%s, () -> %s);\n}",
exception, SourceCode.treeToString(tree, state));
}
}

View File

@@ -0,0 +1,110 @@
package tech.picnic.errorprone.bugpatterns;
import static com.google.errorprone.BugPattern.LinkType.NONE;
import static com.google.errorprone.BugPattern.SeverityLevel.ERROR;
import static com.google.errorprone.BugPattern.StandardTags.REFACTORING;
import static com.google.errorprone.matchers.Matchers.allOf;
import static com.google.errorprone.matchers.Matchers.hasArgumentWithValue;
import static com.google.errorprone.matchers.Matchers.isType;
import static com.google.errorprone.matchers.Matchers.stringLiteral;
import static com.sun.source.tree.Tree.Kind.STRING_LITERAL;
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.AnnotationMatcherUtils;
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.LiteralTree;
import com.sun.source.tree.MethodTree;
import java.util.Optional;
/**
* A {@link BugChecker} that will flag TestNG {@link org.testng.annotations.Test} annotations that
* can be migrated to a JUnit {@link org.junit.jupiter.params.ParameterizedTest}. These methods will
* only be flagged if a migrated version of the data provider is available, these are migrated using
* {@link TestNGDataProvider}.
*/
@AutoService(BugChecker.class)
@BugPattern(
summary = "Migrate TestNG parameterized tests to JUnit",
linkType = NONE,
tags = REFACTORING,
severity = ERROR)
public final class TestNGParameterized extends BugChecker implements MethodTreeMatcher {
private static final long serialVersionUID = 1L;
private static final Matcher<AnnotationTree> SUPPRESS_WARNINGS_ANNOTATION =
allOf(
isType("java.lang.SuppressWarnings"),
hasArgumentWithValue("value", stringLiteral("UnusedMethod")));
private static final Matcher<AnnotationTree> TESTNG_ANNOTATION =
isType("org.testng.annotations.Test");
@Override
public Description matchMethod(MethodTree tree, VisitorState state) {
Optional<? extends AnnotationTree> testAnnotation =
ASTHelpers.getAnnotations(tree).stream()
.filter(annotation -> TESTNG_ANNOTATION.matches(annotation, state))
.findFirst();
if (testAnnotation.isEmpty() || testAnnotation.get().getArguments().size() != 1) {
return Description.NO_MATCH;
}
ExpressionTree argumentExpression =
AnnotationMatcherUtils.getArgument(testAnnotation.get(), "dataProvider");
if (argumentExpression == null || argumentExpression.getKind() != STRING_LITERAL) {
return Description.NO_MATCH;
}
ClassTree classTree = state.findEnclosing(ClassTree.class);
if (classTree == null) {
return Description.NO_MATCH;
}
String providerName = ((LiteralTree) argumentExpression).getValue().toString();
Optional<MethodTree> providerMethod = findMethodInClassWithName(classTree, providerName);
Optional<MethodTree> migratedMethod =
findMethodInClassWithName(classTree, providerName + "Junit");
if (migratedMethod.isEmpty() || providerMethod.isEmpty()) {
return Description.NO_MATCH;
}
Optional<? extends AnnotationTree> suppressWarningsAnnotation =
ASTHelpers.getAnnotations(migratedMethod.orElseThrow()).stream()
.filter(annotation -> SUPPRESS_WARNINGS_ANNOTATION.matches(annotation, state))
.findFirst();
if (suppressWarningsAnnotation.isEmpty()) {
return Description.NO_MATCH;
}
return describeMatch(
testAnnotation.get(),
SuggestedFix.builder()
.addImport("org.junit.jupiter.params.ParameterizedTest")
.addImport("org.junit.jupiter.params.provider.MethodSource")
.merge(SuggestedFixes.renameMethod(migratedMethod.orElseThrow(), providerName, state))
.delete(providerMethod.orElseThrow())
.delete(suppressWarningsAnnotation.get())
.replace(
testAnnotation.get(), "@ParameterizedTest\n@MethodSource(\"" + providerName + "\")")
.build());
}
private static Optional<MethodTree> findMethodInClassWithName(ClassTree classTree, String name) {
return classTree.getMembers().stream()
.filter(MethodTree.class::isInstance)
.map(MethodTree.class::cast)
.filter(method -> method.getName().contentEquals(name))
.findFirst();
}
}

View File

@@ -1,6 +1,6 @@
package tech.picnic.errorprone.bugpatterns;
import static com.google.errorprone.BugPattern.LinkType.NONE;
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;
@@ -10,6 +10,7 @@ import static com.google.errorprone.matchers.Matchers.instanceMethod;
import static com.google.errorprone.matchers.Matchers.isSubtypeOf;
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;
@@ -26,12 +27,13 @@ import java.time.LocalDate;
import java.time.LocalDateTime;
import java.time.LocalTime;
/** A {@link BugChecker} which flags illegal time-zone related operations. */
/** A {@link BugChecker} that flags illegal time-zone related operations. */
@AutoService(BugChecker.class)
@BugPattern(
summary =
"Derive the current time from an existing `Clock` Spring bean, and don't rely on a `Clock`'s time zone",
linkType = NONE,
link = BUG_PATTERNS_BASE_URL + "TimeZoneUsage",
linkType = CUSTOM,
severity = WARNING,
tags = FRAGILE_CODE)
public final class TimeZoneUsage extends BugChecker implements MethodInvocationTreeMatcher {
@@ -60,6 +62,9 @@ public final class TimeZoneUsage extends BugChecker implements MethodInvocationT
.named("now"),
staticMethod().onClassAny(Instant.class.getName()).named("now").withNoParameters());
/** Instantiates a new {@link TimeZoneUsage} instance. */
public TimeZoneUsage() {}
@Override
public Description matchMethodInvocation(MethodInvocationTree tree, VisitorState state) {
return BANNED_TIME_METHOD.matches(tree, state)

View File

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

View File

@@ -0,0 +1,9 @@
package tech.picnic.errorprone.bugpatterns.util;
/** Utility class providing documentation-related code. */
public final class Documentation {
/** The base URL at which Error Prone Support bug patterns are hosted. */
public static final String BUG_PATTERNS_BASE_URL = "https://error-prone.picnic.tech/bugpatterns/";
private Documentation() {}
}

View File

@@ -21,6 +21,9 @@ public final class MethodMatcherFactory {
private static final Pattern METHOD_SIGNATURE =
Pattern.compile("([^\\s#(,)]+)#([^\\s#(,)]+)\\(((?:[^\\s#(,)]+(?:,[^\\s#(,)]+)*)?)\\)");
/** Instantiates a new {@link MethodMatcherFactory} instance. */
public MethodMatcherFactory() {}
/**
* Creates a {@link Matcher} of methods with any of the given signatures.
*

View File

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

View File

@@ -1,4 +1,4 @@
package tech.picnic.errorprone.refastertemplates;
package tech.picnic.errorprone.refasterrules;
import static org.assertj.core.data.Offset.offset;
import static org.assertj.core.data.Percentage.withPercentage;
@@ -9,19 +9,21 @@ import com.google.errorprone.refaster.annotation.BeforeTemplate;
import java.math.BigDecimal;
import org.assertj.core.api.AbstractBigDecimalAssert;
import org.assertj.core.api.BigDecimalAssert;
import tech.picnic.errorprone.refaster.annotation.OnlineDocumentation;
/**
* Refaster templates related to AssertJ assertions over {@link BigDecimal}s.
* Refaster rules related to AssertJ assertions over {@link BigDecimal}s.
*
* <p>Note that, contrary to collections of Refaster templates for other {@link
* org.assertj.core.api.NumberAssert} subtypes, these templates do not rewrite to/from {@link
* <p>Note that, contrary to collections of Refaster rules for other {@link
* org.assertj.core.api.NumberAssert} subtypes, these rules do not rewrite to/from {@link
* BigDecimalAssert#isEqualTo(Object)} and {@link BigDecimalAssert#isNotEqualTo(Object)}. This is
* because {@link BigDecimal#equals(Object)} considers not only the numeric value of compared
* instances, but also their scale. As a result various seemingly straightforward transformations
* would actually subtly change the assertion's semantics.
*/
final class AssertJBigDecimalTemplates {
private AssertJBigDecimalTemplates() {}
@OnlineDocumentation
final class AssertJBigDecimalRules {
private AssertJBigDecimalRules() {}
static final class AbstractBigDecimalAssertIsEqualByComparingTo {
@BeforeTemplate

View File

@@ -1,4 +1,4 @@
package tech.picnic.errorprone.refastertemplates;
package tech.picnic.errorprone.refasterrules;
import static org.assertj.core.data.Offset.offset;
import static org.assertj.core.data.Percentage.withPercentage;
@@ -8,11 +8,13 @@ import com.google.errorprone.refaster.annotation.AfterTemplate;
import com.google.errorprone.refaster.annotation.BeforeTemplate;
import java.math.BigInteger;
import org.assertj.core.api.AbstractBigIntegerAssert;
import tech.picnic.errorprone.refaster.annotation.OnlineDocumentation;
// XXX: If we add a rule which drops unnecessary `L` suffixes from literal longs, then the `0L`/`1L`
// XXX: If we add a rule that drops unnecessary `L` suffixes from literal longs, then the `0L`/`1L`
// cases below can go.
final class AssertJBigIntegerTemplates {
private AssertJBigIntegerTemplates() {}
@OnlineDocumentation
final class AssertJBigIntegerRules {
private AssertJBigIntegerRules() {}
static final class AbstractBigIntegerAssertIsEqualTo {
@BeforeTemplate

View File

@@ -1,4 +1,4 @@
package tech.picnic.errorprone.refastertemplates;
package tech.picnic.errorprone.refasterrules;
import static com.google.errorprone.refaster.ImportPolicy.STATIC_IMPORT_ALWAYS;
import static org.assertj.core.api.Assertions.assertThat;
@@ -8,9 +8,11 @@ import com.google.errorprone.refaster.annotation.AfterTemplate;
import com.google.errorprone.refaster.annotation.BeforeTemplate;
import com.google.errorprone.refaster.annotation.UseImportPolicy;
import org.assertj.core.api.AbstractBooleanAssert;
import tech.picnic.errorprone.refaster.annotation.OnlineDocumentation;
final class AssertJBooleanTemplates {
private AssertJBooleanTemplates() {}
@OnlineDocumentation
final class AssertJBooleanRules {
private AssertJBooleanRules() {}
static final class AbstractBooleanAssertIsEqualTo {
@BeforeTemplate

View File

@@ -1,4 +1,4 @@
package tech.picnic.errorprone.refastertemplates;
package tech.picnic.errorprone.refasterrules;
import static org.assertj.core.data.Offset.offset;
import static org.assertj.core.data.Percentage.withPercentage;
@@ -7,9 +7,11 @@ import com.google.errorprone.refaster.Refaster;
import com.google.errorprone.refaster.annotation.AfterTemplate;
import com.google.errorprone.refaster.annotation.BeforeTemplate;
import org.assertj.core.api.AbstractByteAssert;
import tech.picnic.errorprone.refaster.annotation.OnlineDocumentation;
final class AssertJByteTemplates {
private AssertJByteTemplates() {}
@OnlineDocumentation
final class AssertJByteRules {
private AssertJByteRules() {}
static final class AbstractByteAssertIsEqualTo {
@BeforeTemplate

View File

@@ -1,4 +1,4 @@
package tech.picnic.errorprone.refastertemplates;
package tech.picnic.errorprone.refasterrules;
import static com.google.errorprone.refaster.ImportPolicy.STATIC_IMPORT_ALWAYS;
import static org.assertj.core.api.Assertions.assertThat;
@@ -8,9 +8,11 @@ import com.google.errorprone.refaster.annotation.AfterTemplate;
import com.google.errorprone.refaster.annotation.BeforeTemplate;
import com.google.errorprone.refaster.annotation.UseImportPolicy;
import org.assertj.core.api.AbstractAssert;
import tech.picnic.errorprone.refaster.annotation.OnlineDocumentation;
final class AssertJCharSequenceTemplates {
private AssertJCharSequenceTemplates() {}
@OnlineDocumentation
final class AssertJCharSequenceRules {
private AssertJCharSequenceRules() {}
static final class AssertThatCharSequenceIsEmpty {
@BeforeTemplate

View File

@@ -1,4 +1,4 @@
package tech.picnic.errorprone.refastertemplates;
package tech.picnic.errorprone.refasterrules;
import static com.google.errorprone.refaster.ImportPolicy.STATIC_IMPORT_ALWAYS;
import static org.assertj.core.api.Assertions.assertThat;
@@ -8,9 +8,11 @@ import com.google.errorprone.refaster.annotation.BeforeTemplate;
import com.google.errorprone.refaster.annotation.UseImportPolicy;
import org.assertj.core.api.AbstractComparableAssert;
import org.assertj.core.api.AbstractIntegerAssert;
import tech.picnic.errorprone.refaster.annotation.OnlineDocumentation;
final class AssertJComparableTemplates {
private AssertJComparableTemplates() {}
@OnlineDocumentation
final class AssertJComparableRules {
private AssertJComparableRules() {}
static final class AssertThatIsEqualByComparingTo<T extends Comparable<? super T>> {
@BeforeTemplate

View File

@@ -1,4 +1,4 @@
package tech.picnic.errorprone.refastertemplates;
package tech.picnic.errorprone.refasterrules;
import static org.assertj.core.data.Offset.offset;
import static org.assertj.core.data.Percentage.withPercentage;
@@ -8,9 +8,11 @@ import com.google.errorprone.refaster.annotation.AfterTemplate;
import com.google.errorprone.refaster.annotation.BeforeTemplate;
import org.assertj.core.api.AbstractDoubleAssert;
import org.assertj.core.data.Offset;
import tech.picnic.errorprone.refaster.annotation.OnlineDocumentation;
final class AssertJDoubleTemplates {
private AssertJDoubleTemplates() {}
@OnlineDocumentation
final class AssertJDoubleRules {
private AssertJDoubleRules() {}
static final class AbstractDoubleAssertIsCloseToWithOffset {
@BeforeTemplate

View File

@@ -1,4 +1,4 @@
package tech.picnic.errorprone.refastertemplates;
package tech.picnic.errorprone.refasterrules;
import com.google.common.collect.Iterables;
import com.google.errorprone.refaster.Refaster;
@@ -6,9 +6,11 @@ import com.google.errorprone.refaster.annotation.AfterTemplate;
import com.google.errorprone.refaster.annotation.BeforeTemplate;
import java.util.Collection;
import org.assertj.core.api.EnumerableAssert;
import tech.picnic.errorprone.refaster.annotation.OnlineDocumentation;
final class AssertJEnumerableTemplates {
private AssertJEnumerableTemplates() {}
@OnlineDocumentation
final class AssertJEnumerableRules {
private AssertJEnumerableRules() {}
static final class EnumerableAssertIsEmpty<E> {
@BeforeTemplate

View File

@@ -1,4 +1,4 @@
package tech.picnic.errorprone.refastertemplates;
package tech.picnic.errorprone.refasterrules;
import static org.assertj.core.data.Offset.offset;
import static org.assertj.core.data.Percentage.withPercentage;
@@ -8,9 +8,11 @@ import com.google.errorprone.refaster.annotation.AfterTemplate;
import com.google.errorprone.refaster.annotation.BeforeTemplate;
import org.assertj.core.api.AbstractFloatAssert;
import org.assertj.core.data.Offset;
import tech.picnic.errorprone.refaster.annotation.OnlineDocumentation;
final class AssertJFloatTemplates {
private AssertJFloatTemplates() {}
@OnlineDocumentation
final class AssertJFloatRules {
private AssertJFloatRules() {}
static final class AbstractFloatAssertIsCloseToWithOffset {
@BeforeTemplate

View File

@@ -1,4 +1,4 @@
package tech.picnic.errorprone.refastertemplates;
package tech.picnic.errorprone.refasterrules;
import static org.assertj.core.data.Offset.offset;
import static org.assertj.core.data.Percentage.withPercentage;
@@ -7,9 +7,11 @@ import com.google.errorprone.refaster.Refaster;
import com.google.errorprone.refaster.annotation.AfterTemplate;
import com.google.errorprone.refaster.annotation.BeforeTemplate;
import org.assertj.core.api.AbstractIntegerAssert;
import tech.picnic.errorprone.refaster.annotation.OnlineDocumentation;
final class AssertJIntegerTemplates {
private AssertJIntegerTemplates() {}
@OnlineDocumentation
final class AssertJIntegerRules {
private AssertJIntegerRules() {}
static final class AbstractIntegerAssertIsEqualTo {
@BeforeTemplate

View File

@@ -1,4 +1,4 @@
package tech.picnic.errorprone.refastertemplates;
package tech.picnic.errorprone.refasterrules;
import static org.assertj.core.data.Offset.offset;
import static org.assertj.core.data.Percentage.withPercentage;
@@ -7,9 +7,11 @@ import com.google.errorprone.refaster.Refaster;
import com.google.errorprone.refaster.annotation.AfterTemplate;
import com.google.errorprone.refaster.annotation.BeforeTemplate;
import org.assertj.core.api.AbstractLongAssert;
import tech.picnic.errorprone.refaster.annotation.OnlineDocumentation;
final class AssertJLongTemplates {
private AssertJLongTemplates() {}
@OnlineDocumentation
final class AssertJLongRules {
private AssertJLongRules() {}
static final class AbstractLongAssertIsEqualTo {
@BeforeTemplate

View File

@@ -1,13 +1,15 @@
package tech.picnic.errorprone.refastertemplates;
package tech.picnic.errorprone.refasterrules;
import com.google.common.collect.ImmutableMap;
import com.google.errorprone.refaster.annotation.AfterTemplate;
import com.google.errorprone.refaster.annotation.BeforeTemplate;
import java.util.Map;
import org.assertj.core.api.AbstractMapAssert;
import tech.picnic.errorprone.refaster.annotation.OnlineDocumentation;
final class AssertJMapTemplates {
private AssertJMapTemplates() {}
@OnlineDocumentation
final class AssertJMapRules {
private AssertJMapRules() {}
static final class AbstractMapAssertContainsExactlyInAnyOrderEntriesOf<K, V> {
@BeforeTemplate

View File

@@ -1,4 +1,4 @@
package tech.picnic.errorprone.refastertemplates;
package tech.picnic.errorprone.refasterrules;
import static com.google.errorprone.refaster.ImportPolicy.STATIC_IMPORT_ALWAYS;
import static org.assertj.core.api.Assertions.assertThat;
@@ -19,10 +19,12 @@ import org.assertj.core.api.AbstractIntegerAssert;
import org.assertj.core.api.AbstractLongAssert;
import org.assertj.core.api.AbstractShortAssert;
import org.assertj.core.api.NumberAssert;
import tech.picnic.errorprone.refaster.util.IsCharacter;
import tech.picnic.errorprone.refaster.annotation.OnlineDocumentation;
import tech.picnic.errorprone.refaster.matchers.IsCharacter;
final class AssertJNumberTemplates {
private AssertJNumberTemplates() {}
@OnlineDocumentation
final class AssertJNumberRules {
private AssertJNumberRules() {}
static final class NumberAssertIsPositive {
@BeforeTemplate

View File

@@ -1,4 +1,4 @@
package tech.picnic.errorprone.refastertemplates;
package tech.picnic.errorprone.refasterrules;
import static com.google.errorprone.refaster.ImportPolicy.STATIC_IMPORT_ALWAYS;
import static org.assertj.core.api.Assertions.assertThat;
@@ -10,9 +10,11 @@ import com.google.errorprone.refaster.annotation.UseImportPolicy;
import org.assertj.core.api.AbstractBooleanAssert;
import org.assertj.core.api.AbstractStringAssert;
import org.assertj.core.api.ObjectAssert;
import tech.picnic.errorprone.refaster.annotation.OnlineDocumentation;
final class AssertJObjectTemplates {
private AssertJObjectTemplates() {}
@OnlineDocumentation
final class AssertJObjectRules {
private AssertJObjectRules() {}
static final class AssertThatIsInstanceOf<S, T> {
@BeforeTemplate

View File

@@ -1,4 +1,4 @@
package tech.picnic.errorprone.refastertemplates;
package tech.picnic.errorprone.refasterrules;
import static com.google.errorprone.refaster.ImportPolicy.STATIC_IMPORT_ALWAYS;
import static org.assertj.core.api.Assertions.assertThat;
@@ -14,9 +14,11 @@ import org.assertj.core.api.AbstractObjectAssert;
import org.assertj.core.api.AbstractOptionalAssert;
import org.assertj.core.api.ObjectAssert;
import org.assertj.core.api.OptionalAssert;
import tech.picnic.errorprone.refaster.annotation.OnlineDocumentation;
final class AssertJOptionalTemplates {
private AssertJOptionalTemplates() {}
@OnlineDocumentation
final class AssertJOptionalRules {
private AssertJOptionalRules() {}
static final class AssertThatOptional<T> {
@BeforeTemplate

View File

@@ -1,4 +1,4 @@
package tech.picnic.errorprone.refastertemplates;
package tech.picnic.errorprone.refasterrules;
import static com.google.errorprone.refaster.ImportPolicy.STATIC_IMPORT_ALWAYS;
import static org.assertj.core.api.Assertions.assertThat;
@@ -9,9 +9,11 @@ import com.google.errorprone.refaster.annotation.BeforeTemplate;
import com.google.errorprone.refaster.annotation.UseImportPolicy;
import org.assertj.core.api.AbstractBooleanAssert;
import org.assertj.core.api.AbstractDoubleAssert;
import tech.picnic.errorprone.refaster.annotation.OnlineDocumentation;
final class AssertJPrimitiveTemplates {
private AssertJPrimitiveTemplates() {}
@OnlineDocumentation
final class AssertJPrimitiveRules {
private AssertJPrimitiveRules() {}
static final class AssertThatIsEqualTo {
@BeforeTemplate

View File

@@ -1,4 +1,4 @@
package tech.picnic.errorprone.refastertemplates;
package tech.picnic.errorprone.refasterrules;
import static com.google.errorprone.refaster.ImportPolicy.STATIC_IMPORT_ALWAYS;
import static org.assertj.core.api.Assertions.assertThat;
@@ -53,9 +53,10 @@ import org.assertj.core.api.ObjectEnumerableAssert;
import org.assertj.core.api.OptionalDoubleAssert;
import org.assertj.core.api.OptionalIntAssert;
import org.assertj.core.api.OptionalLongAssert;
import tech.picnic.errorprone.refaster.util.IsArray;
import tech.picnic.errorprone.refaster.annotation.OnlineDocumentation;
import tech.picnic.errorprone.refaster.matchers.IsArray;
/** Refaster templates related to AssertJ expressions and statements. */
/** Refaster rules related to AssertJ expressions and statements. */
// XXX: Most `AbstractIntegerAssert` rules can also be applied for other primitive types. Generate
// these in separate files.
// XXX: Also do for BigInteger/BigDecimal?
@@ -63,7 +64,7 @@ import tech.picnic.errorprone.refaster.util.IsArray;
// ^ And variants.
// XXX: Consider splitting this class into multiple classes.
// XXX: Some of these rules may not apply given the updated TestNG rewrite rules. Review.
// XXX: For the templates which "unwrap" explicitly enumerated collections, also introduce variants
// XXX: For the rules that "unwrap" explicitly enumerated collections, also introduce variants
// with explicitly enumerated sorted collections. (Requires that the type bound is Comparable.)
// XXX: Handle `.isEqualTo(explicitlyEnumeratedCollection)`. Can be considered equivalent to
// `.containsOnly(elements)`. (This does mean the auto-generated code needs to be more advanced.
@@ -101,27 +102,28 @@ import tech.picnic.errorprone.refaster.util.IsArray;
// (etc.)
// XXX: Look into using Assertions#contentOf(URL url, Charset charset) instead of our own test
// method.
// XXX: Write Optional templates also for `OptionalInt` and variants.
// XXX: Write `Optional` rules also for `OptionalInt` and variants.
// XXX: Write plugin to flag `assertThat(compileTimeConstant)` occurrences. Also other likely
// candidates, such as `assertThat(ImmutableSet(foo, bar)).XXX`
// XXX: Write generic plugin to replace explicit array parameters with varargs (`new int[] {1, 2}`
// -> `1, 2`).
// XXX: Write plugin which drops any `.withFailMessage` which doesn't include a compile-time
// constant string? Most of these are useless.
// XXX: Write plugin which identifies `.get().propertyAccess()` and "pushes" this out. Would only
// XXX: Write plugin that drops any `.withFailMessage` that doesn't include a compile-time constant
// string? Most of these are useless.
// XXX: Write plugin that identifies `.get().propertyAccess()` and "pushes" this out. Would only
// nicely work for non-special types, though, cause after `extracting(propertyAccess)` many
// operations are not available...
// XXX: Write plugin which identifies repeated `assertThat(someProp.xxx)` calls and bundles these
// XXX: Write plugin that identifies repeated `assertThat(someProp.xxx)` calls and bundles these
// somehow.
// XXX: `abstractOptionalAssert.get().satisfies(pred)` ->
// `abstractOptionalAssert.hasValueSatisfying(pred)`.
// XXX: `assertThat(ImmutableList.sortedCopyOf(cmp, values)).somethingExactOrder` -> just compare
// "in any order".
// XXX: Turns out a lot of this is also covered by https://github.com/palantir/assertj-automation.
// See how we can combine these things. Do note that (at present) their Refaster templates don't
// See how we can combine these things. Do note that (at present) their Refaster rules don't
// show up as Error Prone checks. So we'd have to build an integration for that.
final class AssertJTemplates {
private AssertJTemplates() {}
@OnlineDocumentation
final class AssertJRules {
private AssertJRules() {}
//
// OptionalDouble
@@ -491,7 +493,7 @@ final class AssertJTemplates {
}
// XXX: This overload is here because `assertThat` has an overload for `Comparable` types.
// Unfortunately this still doesn't convince Refaster to match this template in the context of
// Unfortunately this still doesn't convince Refaster to match this rule in the context of
// Comparable types. Figure out why! Note that this also affects the `AssertThatOptional` rule.
static final class AssertThatIterableHasOneComparableElementEqualTo<
S extends Comparable<? super S>, T extends S> {

View File

@@ -1,4 +1,4 @@
package tech.picnic.errorprone.refastertemplates;
package tech.picnic.errorprone.refasterrules;
import static org.assertj.core.data.Offset.offset;
import static org.assertj.core.data.Percentage.withPercentage;
@@ -7,9 +7,11 @@ import com.google.errorprone.refaster.Refaster;
import com.google.errorprone.refaster.annotation.AfterTemplate;
import com.google.errorprone.refaster.annotation.BeforeTemplate;
import org.assertj.core.api.AbstractShortAssert;
import tech.picnic.errorprone.refaster.annotation.OnlineDocumentation;
final class AssertJShortTemplates {
private AssertJShortTemplates() {}
@OnlineDocumentation
final class AssertJShortRules {
private AssertJShortRules() {}
static final class AbstractShortAssertIsEqualTo {
@BeforeTemplate

View File

@@ -1,4 +1,4 @@
package tech.picnic.errorprone.refastertemplates;
package tech.picnic.errorprone.refasterrules;
import static com.google.errorprone.refaster.ImportPolicy.STATIC_IMPORT_ALWAYS;
import static org.assertj.core.api.Assertions.assertThat;
@@ -8,9 +8,11 @@ import com.google.errorprone.refaster.annotation.BeforeTemplate;
import com.google.errorprone.refaster.annotation.UseImportPolicy;
import org.assertj.core.api.AbstractAssert;
import org.assertj.core.api.AbstractStringAssert;
import tech.picnic.errorprone.refaster.annotation.OnlineDocumentation;
final class AssertJStringTemplates {
private AssertJStringTemplates() {}
@OnlineDocumentation
final class AssertJStringRules {
private AssertJStringRules() {}
static final class AbstractStringAssertStringIsEmpty {
@BeforeTemplate

View File

@@ -1,4 +1,4 @@
package tech.picnic.errorprone.refastertemplates;
package tech.picnic.errorprone.refasterrules;
import static com.google.errorprone.refaster.ImportPolicy.STATIC_IMPORT_ALWAYS;
import static org.assertj.core.api.Assertions.assertThatExceptionOfType;
@@ -16,18 +16,20 @@ import java.io.IOException;
import org.assertj.core.api.AbstractObjectAssert;
import org.assertj.core.api.AbstractThrowableAssert;
import org.assertj.core.api.ThrowableAssert.ThrowingCallable;
import tech.picnic.errorprone.refaster.annotation.OnlineDocumentation;
/**
* Refaster templates related to AssertJ assertions over expressions that may throw a {@link
* Throwable} subtype.
* Refaster rules related to AssertJ assertions over expressions that may throw a {@link Throwable}
* subtype.
*
* <p>For reasons of consistency we prefer {@link
* org.assertj.core.api.Assertions#assertThatThrownBy} over static methods for specific exception
* types. Note that only the most common assertion expressions are rewritten here; covering all
* cases would require the implementation of an Error Prone check instead.
*/
final class AssertJThrowingCallableTemplates {
private AssertJThrowingCallableTemplates() {}
@OnlineDocumentation
final class AssertJThrowingCallableRules {
private AssertJThrowingCallableRules() {}
static final class AssertThatThrownByIllegalArgumentException {
@BeforeTemplate
@@ -429,8 +431,8 @@ final class AssertJThrowingCallableTemplates {
}
}
// XXX: Drop this template in favour of a generic Error Prone check which flags
// `String.format(...)` arguments to a wide range of format methods.
// XXX: Drop this rule in favour of a generic Error Prone check that flags `String.format(...)`
// arguments to a wide range of format methods.
static final class AbstractThrowableAssertHasMessage {
@BeforeTemplate
AbstractThrowableAssert<?, ? extends Throwable> before(
@@ -449,8 +451,8 @@ final class AssertJThrowingCallableTemplates {
}
}
// XXX: Drop this template in favour of a generic Error Prone check which flags
// `String.format(...)` arguments to a wide range of format methods.
// XXX: Drop this rule in favour of a generic Error Prone check that flags `String.format(...)`
// arguments to a wide range of format methods.
static final class AbstractThrowableAssertWithFailMessage {
@BeforeTemplate
AbstractThrowableAssert<?, ? extends Throwable> before(

View File

@@ -1,4 +1,4 @@
package tech.picnic.errorprone.refastertemplates;
package tech.picnic.errorprone.refasterrules;
import static com.google.common.base.Preconditions.checkElementIndex;
import static com.google.common.collect.ImmutableSet.toImmutableSet;
@@ -27,14 +27,16 @@ import java.util.Map;
import java.util.Objects;
import java.util.Set;
import java.util.stream.Stream;
import javax.annotation.Nullable;
import org.jspecify.nullness.Nullable;
import tech.picnic.errorprone.refaster.annotation.OnlineDocumentation;
/**
* Assorted Refaster templates that do not (yet) belong in one of the other classes with more
* topical Refaster templates.
* Assorted Refaster rules that do not (yet) belong in one of the other classes with more topical
* Refaster rules.
*/
final class AssortedTemplates {
private AssortedTemplates() {}
@OnlineDocumentation
final class AssortedRules {
private AssortedRules() {}
/** Prefer {@link Objects#checkIndex(int, int)} over the Guava alternative. */
static final class CheckIndex {
@@ -44,11 +46,33 @@ final class AssortedTemplates {
}
@AfterTemplate
@UseImportPolicy(STATIC_IMPORT_ALWAYS)
int after(int index, int size) {
return checkIndex(index, size);
}
}
/**
* Prefer {@link Objects#checkIndex(int, int)} over less descriptive or more verbose alternatives.
*
* <p>If a custom error message is desired, consider using Guava's {@link
* com.google.common.base.Preconditions#checkElementIndex(int, int, String)}.
*/
static final class CheckIndexConditional {
@BeforeTemplate
void before(int index, int size) {
if (index < 0 || index >= size) {
throw new IndexOutOfBoundsException();
}
}
@AfterTemplate
@UseImportPolicy(STATIC_IMPORT_ALWAYS)
void after(int index, int size) {
checkIndex(index, size);
}
}
// XXX: We could add a rule for `new EnumMap(Map<K, ? extends V> m)`, but that constructor does
// not allow an empty non-EnumMap to be provided.
static final class CreateEnumMap<K extends Enum<K>, V> {
@@ -65,14 +89,12 @@ final class AssortedTemplates {
static final class MapGetOrNull<K, V, L> {
@BeforeTemplate
@Nullable
V before(Map<K, V> map, L key) {
@Nullable V before(Map<K, V> map, L key) {
return map.getOrDefault(key, null);
}
@AfterTemplate
@Nullable
V after(Map<K, V> map, L key) {
@Nullable V after(Map<K, V> map, L key) {
return map.get(key);
}
}
@@ -82,7 +104,7 @@ final class AssortedTemplates {
* ImmutableSet#toImmutableSet()} and produces a more compact object.
*
* <p><strong>Warning:</strong> this rewrite rule is not completely behavior preserving: while the
* original code produces a set which iterates over the elements in encounter order, the
* original code produces a set that iterates over the elements in encounter order, the
* replacement code iterates over the elements in enum definition order.
*/
// XXX: ^ Consider emitting a comment warning about this fact?
@@ -110,15 +132,14 @@ final class AssortedTemplates {
}
@AfterTemplate
@Nullable
T after(Iterator<T> iterator, T defaultValue) {
@Nullable T after(Iterator<T> iterator, T defaultValue) {
return Iterators.getNext(iterator, defaultValue);
}
}
/** Don't unnecessarily repeat boolean expressions. */
// XXX: This template captures only the simplest case. `@AlsoNegation` doesn't help. Consider
// contributing a Refaster patch which handles the negation in the `@BeforeTemplate` more
// XXX: This rule captures only the simplest case. `@AlsoNegation` doesn't help. Consider
// contributing a Refaster patch, which handles the negation in the `@BeforeTemplate` more
// intelligently.
static final class LogicalImplication {
@BeforeTemplate
@@ -172,8 +193,8 @@ final class AssortedTemplates {
* Collections#disjoint(Collection, Collection)}.
*/
// XXX: Other copy operations could be elided too, but these are most common after application of
// the `DisjointSets` template defined above. If we ever introduce a generic "makes a copy"
// stand-in, use it here.
// the `DisjointSets` rule defined above. If we ever introduce a generic "makes a copy" stand-in,
// use it here.
static final class DisjointCollections<T> {
@BeforeTemplate
boolean before(Collection<T> collection1, Collection<T> collection2) {

View File

@@ -1,13 +1,15 @@
package tech.picnic.errorprone.refastertemplates;
package tech.picnic.errorprone.refasterrules;
import com.google.errorprone.refaster.Refaster;
import com.google.errorprone.refaster.annotation.AfterTemplate;
import com.google.errorprone.refaster.annotation.BeforeTemplate;
import java.math.BigDecimal;
import tech.picnic.errorprone.refaster.annotation.OnlineDocumentation;
/** Refaster templates related to expressions dealing with {@link BigDecimal}s. */
final class BigDecimalTemplates {
private BigDecimalTemplates() {}
/** Refaster rules related to expressions dealing with {@link BigDecimal}s. */
@OnlineDocumentation
final class BigDecimalRules {
private BigDecimalRules() {}
/** Prefer using the constant {@link BigDecimal#ZERO} when possible. */
static final class BigDecimalZero {

View File

@@ -1,4 +1,4 @@
package tech.picnic.errorprone.refastertemplates;
package tech.picnic.errorprone.refasterrules;
import com.google.common.collect.ImmutableCollection;
import com.google.common.collect.ImmutableList;
@@ -19,12 +19,14 @@ import java.util.Set;
import java.util.SortedSet;
import java.util.function.IntFunction;
import java.util.stream.Stream;
import tech.picnic.errorprone.refaster.annotation.OnlineDocumentation;
/** Refaster templates related to expressions dealing with (arbitrary) collections. */
/** Refaster rules related to expressions dealing with (arbitrary) collections. */
// XXX: There are other Guava `Iterables` methods that should not be called if the input is known to
// be a `Collection`. Add those here.
final class CollectionTemplates {
private CollectionTemplates() {}
@OnlineDocumentation
final class CollectionRules {
private CollectionRules() {}
/**
* Prefer {@link Collection#isEmpty()} over alternatives that consult the collection's size or are

View File

@@ -1,4 +1,4 @@
package tech.picnic.errorprone.refastertemplates;
package tech.picnic.errorprone.refasterrules;
import static com.google.errorprone.refaster.ImportPolicy.STATIC_IMPORT_ALWAYS;
import static java.util.Comparator.comparing;
@@ -19,14 +19,17 @@ import com.google.errorprone.refaster.annotation.UseImportPolicy;
import java.util.Arrays;
import java.util.Collections;
import java.util.Comparator;
import java.util.function.BinaryOperator;
import java.util.function.Function;
import java.util.function.ToDoubleFunction;
import java.util.function.ToIntFunction;
import java.util.function.ToLongFunction;
import tech.picnic.errorprone.refaster.annotation.OnlineDocumentation;
/** Refaster templates related to expressions dealing with {@link Comparator}s. */
final class ComparatorTemplates {
private ComparatorTemplates() {}
/** Refaster rules related to expressions dealing with {@link Comparator}s. */
@OnlineDocumentation
final class ComparatorRules {
private ComparatorRules() {}
/** Prefer {@link Comparator#naturalOrder()} over more complicated constructs. */
static final class NaturalOrder<T extends Comparable<? super T>> {
@@ -259,4 +262,36 @@ final class ComparatorTemplates {
return Comparators.max(value1, value2, cmp);
}
}
/**
* Prefer a method reference to {@link Comparators#min(Comparable, Comparable)} over calling
* {@link BinaryOperator#minBy(Comparator)} with {@link Comparator#naturalOrder()}.
*/
static final class ComparatorsMin<T extends Comparable<? super T>> {
@BeforeTemplate
BinaryOperator<T> before() {
return BinaryOperator.minBy(naturalOrder());
}
@AfterTemplate
BinaryOperator<T> after() {
return Comparators::min;
}
}
/**
* Prefer a method reference to {@link Comparators#max(Comparable, Comparable)} over calling
* {@link BinaryOperator#minBy(Comparator)} with {@link Comparator#naturalOrder()}.
*/
static final class ComparatorsMax<T extends Comparable<? super T>> {
@BeforeTemplate
BinaryOperator<T> before() {
return BinaryOperator.maxBy(naturalOrder());
}
@AfterTemplate
BinaryOperator<T> after() {
return Comparators::max;
}
}
}

View File

@@ -1,4 +1,4 @@
package tech.picnic.errorprone.refastertemplates;
package tech.picnic.errorprone.refasterrules;
import com.google.common.collect.Streams;
import com.google.errorprone.refaster.Refaster;
@@ -12,10 +12,12 @@ import java.util.function.DoublePredicate;
import java.util.function.DoubleUnaryOperator;
import java.util.stream.DoubleStream;
import java.util.stream.Stream;
import tech.picnic.errorprone.refaster.annotation.OnlineDocumentation;
/** Refaster templates related to expressions dealing with {@link DoubleStream}s. */
final class DoubleStreamTemplates {
private DoubleStreamTemplates() {}
/** Refaster rules related to expressions dealing with {@link DoubleStream}s. */
@OnlineDocumentation
final class DoubleStreamRules {
private DoubleStreamRules() {}
/** Don't unnecessarily call {@link Streams#concat(DoubleStream...)}. */
static final class ConcatOneDoubleStream {

View File

@@ -1,4 +1,4 @@
package tech.picnic.errorprone.refastertemplates;
package tech.picnic.errorprone.refasterrules;
import com.google.errorprone.refaster.Refaster;
import com.google.errorprone.refaster.annotation.AfterTemplate;
@@ -6,10 +6,12 @@ import com.google.errorprone.refaster.annotation.AlsoNegation;
import com.google.errorprone.refaster.annotation.BeforeTemplate;
import java.util.Objects;
import java.util.function.Predicate;
import tech.picnic.errorprone.refaster.annotation.OnlineDocumentation;
/** Refaster templates related to expressions dealing with (in)equalities. */
final class EqualityTemplates {
private EqualityTemplates() {}
/** Refaster rules related to expressions dealing with (in)equalities. */
@OnlineDocumentation
final class EqualityRules {
private EqualityRules() {}
/** Prefer reference-based quality for enums. */
// Primitive value comparisons are not listed, because Error Prone flags those out of the box.
@@ -36,7 +38,7 @@ final class EqualityTemplates {
/** Prefer {@link Object#equals(Object)} over the equivalent lambda function. */
// XXX: As it stands, this rule is a special case of what `MethodReferenceUsage` tries to achieve.
// If/when `MethodReferenceUsage` becomes production ready, we should simply drop this check.
// XXX: Alternatively, the rule should be replaced with a plugin which also identifies cases where
// XXX: Alternatively, the rule should be replaced with a plugin that also identifies cases where
// the arguments are swapped but simplification is possible anyway, by virtue of `v` being
// non-null.
static final class EqualsPredicate<T> {

View File

@@ -1,4 +1,4 @@
package tech.picnic.errorprone.refastertemplates;
package tech.picnic.errorprone.refasterrules;
import static com.google.common.collect.ImmutableListMultimap.flatteningToImmutableListMultimap;
import static com.google.common.collect.ImmutableListMultimap.toImmutableListMultimap;
@@ -24,17 +24,19 @@ import java.util.Iterator;
import java.util.Map;
import java.util.function.Function;
import java.util.stream.Stream;
import tech.picnic.errorprone.refaster.annotation.OnlineDocumentation;
/** Refaster templates related to expressions dealing with {@link ImmutableListMultimap}s. */
final class ImmutableListMultimapTemplates {
private ImmutableListMultimapTemplates() {}
/** Refaster rules related to expressions dealing with {@link ImmutableListMultimap}s. */
@OnlineDocumentation
final class ImmutableListMultimapRules {
private ImmutableListMultimapRules() {}
/**
* Prefer {@link ImmutableListMultimap#builder()} over the associated constructor on constructions
* that produce a less-specific type.
*/
// XXX: This drops generic type information, sometimes leading to non-compilable code. Anything
// we can do about that?
// XXX: This drops generic type information, sometimes leading to non-compilable code. See
// https://github.com/google/error-prone/pull/2706.
static final class ImmutableListMultimapBuilder<K, V> {
@BeforeTemplate
ImmutableMultimap.Builder<K, V> before() {
@@ -70,7 +72,7 @@ final class ImmutableListMultimapTemplates {
* alternatives.
*/
// XXX: One can define variants for more than one key-value pair, but at some point the builder
// actually produces nicer code. So it's not clear we should add Refaster templates for those
// actually produces nicer code. So it's not clear we should add Refaster rules for those
// variants.
static final class PairToImmutableListMultimap<K, V> {
@BeforeTemplate
@@ -141,8 +143,8 @@ final class ImmutableListMultimapTemplates {
}
/**
* Don't map a a stream's elements to map entries, only to subsequently collect them into an
* {@link ImmutableListMultimap}. The collection can be performed directly.
* Don't map stream's elements to map entries, only to subsequently collect them into an {@link
* ImmutableListMultimap}. The collection can be performed directly.
*/
abstract static class StreamOfMapEntriesToImmutableListMultimap<E, K, V> {
@Placeholder(allowsIdentity = true)

View File

@@ -1,4 +1,4 @@
package tech.picnic.errorprone.refastertemplates;
package tech.picnic.errorprone.refasterrules;
import static com.google.common.collect.ImmutableList.toImmutableList;
import static com.google.common.collect.ImmutableSet.toImmutableSet;
@@ -20,14 +20,16 @@ import java.util.Comparator;
import java.util.Iterator;
import java.util.List;
import java.util.stream.Stream;
import tech.picnic.errorprone.refaster.annotation.OnlineDocumentation;
/** Refaster templates related to expressions dealing with {@link ImmutableList}s. */
final class ImmutableListTemplates {
private ImmutableListTemplates() {}
/** Refaster rules related to expressions dealing with {@link ImmutableList}s. */
@OnlineDocumentation
final class ImmutableListRules {
private ImmutableListRules() {}
/** Prefer {@link ImmutableList#builder()} over the associated constructor. */
// XXX: This drops generic type information, sometimes leading to non-compilable code. Anything
// we can do about that?
// XXX: This drops generic type information, sometimes leading to non-compilable code. See
// https://github.com/google/error-prone/pull/2706.
static final class ImmutableListBuilder<T> {
@BeforeTemplate
ImmutableList.Builder<T> before() {
@@ -154,7 +156,7 @@ final class ImmutableListTemplates {
* communicate the immutability of the resulting list at the type level.
*/
// XXX: The `Stream` variant may be too contrived to warrant inclusion. Review its usage if/when
// this and similar Refaster templates are replaced with an Error Prone check.
// this and similar Refaster rules are replaced with an Error Prone check.
static final class ImmutableListOf<T> {
@BeforeTemplate
List<T> before() {
@@ -194,7 +196,7 @@ final class ImmutableListTemplates {
* Prefer {@link ImmutableList#of(Object, Object)} over alternatives that don't communicate the
* immutability of the resulting list at the type level.
*/
// XXX: Consider writing an Error Prone check which also flags straightforward
// XXX: Consider writing an Error Prone check that also flags straightforward
// `ImmutableList.builder()` usages.
static final class ImmutableListOf2<T> {
@BeforeTemplate
@@ -212,7 +214,7 @@ final class ImmutableListTemplates {
* Prefer {@link ImmutableList#of(Object, Object, Object)} over alternatives that don't
* communicate the immutability of the resulting list at the type level.
*/
// XXX: Consider writing an Error Prone check which also flags straightforward
// XXX: Consider writing an Error Prone check that also flags straightforward
// `ImmutableList.builder()` usages.
static final class ImmutableListOf3<T> {
@BeforeTemplate
@@ -230,7 +232,7 @@ final class ImmutableListTemplates {
* Prefer {@link ImmutableList#of(Object, Object, Object, Object)} over alternatives that don't
* communicate the immutability of the resulting list at the type level.
*/
// XXX: Consider writing an Error Prone check which also flags straightforward
// XXX: Consider writing an Error Prone check that also flags straightforward
// `ImmutableList.builder()` usages.
static final class ImmutableListOf4<T> {
@BeforeTemplate
@@ -248,7 +250,7 @@ final class ImmutableListTemplates {
* Prefer {@link ImmutableList#of(Object, Object, Object, Object, Object)} over alternatives that
* don't communicate the immutability of the resulting list at the type level.
*/
// XXX: Consider writing an Error Prone check which also flags straightforward
// XXX: Consider writing an Error Prone check that also flags straightforward
// `ImmutableList.builder()` usages.
static final class ImmutableListOf5<T> {
@BeforeTemplate

View File

@@ -1,4 +1,4 @@
package tech.picnic.errorprone.refastertemplates;
package tech.picnic.errorprone.refasterrules;
import static com.google.common.collect.ImmutableMap.toImmutableMap;
import static com.google.errorprone.refaster.ImportPolicy.STATIC_IMPORT_ALWAYS;
@@ -21,14 +21,16 @@ import java.util.Map;
import java.util.Set;
import java.util.function.Function;
import java.util.stream.Stream;
import tech.picnic.errorprone.refaster.annotation.OnlineDocumentation;
/** Refaster templates related to expressions dealing with {@link ImmutableMap}s. */
final class ImmutableMapTemplates {
private ImmutableMapTemplates() {}
/** Refaster rules related to expressions dealing with {@link ImmutableMap}s. */
@OnlineDocumentation
final class ImmutableMapRules {
private ImmutableMapRules() {}
/** Prefer {@link ImmutableMap#builder()} over the associated constructor. */
// XXX: This drops generic type information, sometimes leading to non-compilable code. Anything
// we can do about that?
// XXX: This drops generic type information, sometimes leading to non-compilable code. See
// https://github.com/google/error-prone/pull/2706.
static final class ImmutableMapBuilder<K, V> {
@BeforeTemplate
ImmutableMap.Builder<K, V> before() {
@@ -135,7 +137,7 @@ final class ImmutableMapTemplates {
abstract V valueFunction(@MayOptionallyUse E element);
// XXX: We could add variants in which the entry is created some other way, but we have another
// rule which covers canonicalization to `Map.entry`.
// rule that covers canonicalization to `Map.entry`.
@BeforeTemplate
ImmutableMap<K, V> before(Stream<E> stream) {
return stream
@@ -313,7 +315,7 @@ final class ImmutableMapTemplates {
}
}
// XXX: Add a template for this:
// XXX: Add a rule for this:
// Maps.transformValues(streamOfEntries.collect(groupBy(fun)), ImmutableMap::copyOf)
// ->
// streamOfEntries.collect(groupBy(fun, toImmutableMap(Map.Entry::getKey, Map.Entry::getValue)))

View File

@@ -1,4 +1,4 @@
package tech.picnic.errorprone.refastertemplates;
package tech.picnic.errorprone.refasterrules;
import static com.google.common.collect.ImmutableMultiset.toImmutableMultiset;
import static com.google.errorprone.refaster.ImportPolicy.STATIC_IMPORT_ALWAYS;
@@ -13,14 +13,16 @@ import java.util.Arrays;
import java.util.Collection;
import java.util.Iterator;
import java.util.stream.Stream;
import tech.picnic.errorprone.refaster.annotation.OnlineDocumentation;
/** Refaster templates related to expressions dealing with {@link ImmutableMultiset}s. */
final class ImmutableMultisetTemplates {
private ImmutableMultisetTemplates() {}
/** Refaster rules related to expressions dealing with {@link ImmutableMultiset}s. */
@OnlineDocumentation
final class ImmutableMultisetRules {
private ImmutableMultisetRules() {}
/** Prefer {@link ImmutableMultiset#builder()} over the associated constructor. */
// XXX: This drops generic type information, sometimes leading to non-compilable code. Anything
// we can do about that?
// XXX: This drops generic type information, sometimes leading to non-compilable code. See
// https://github.com/google/error-prone/pull/2706.
static final class ImmutableMultisetBuilder<T> {
@BeforeTemplate
ImmutableMultiset.Builder<T> before() {

View File

@@ -1,4 +1,4 @@
package tech.picnic.errorprone.refastertemplates;
package tech.picnic.errorprone.refasterrules;
import static com.google.common.collect.ImmutableSetMultimap.flatteningToImmutableSetMultimap;
import static com.google.common.collect.ImmutableSetMultimap.toImmutableSetMultimap;
@@ -21,14 +21,16 @@ import java.util.Collection;
import java.util.Map;
import java.util.function.Function;
import java.util.stream.Stream;
import tech.picnic.errorprone.refaster.annotation.OnlineDocumentation;
/** Refaster templates related to expressions dealing with {@link ImmutableSetMultimap}s. */
final class ImmutableSetMultimapTemplates {
private ImmutableSetMultimapTemplates() {}
/** Refaster rules related to expressions dealing with {@link ImmutableSetMultimap}s. */
@OnlineDocumentation
final class ImmutableSetMultimapRules {
private ImmutableSetMultimapRules() {}
/** Prefer {@link ImmutableSetMultimap#builder()} over the associated constructor. */
// XXX: This drops generic type information, sometimes leading to non-compilable code. Anything
// we can do about that?
// XXX: This drops generic type information, sometimes leading to non-compilable code. See
// https://github.com/google/error-prone/pull/2706.
static final class ImmutableSetMultimapBuilder<K, V> {
@BeforeTemplate
ImmutableSetMultimap.Builder<K, V> before() {
@@ -56,7 +58,7 @@ final class ImmutableSetMultimapTemplates {
/** Prefer {@link ImmutableSetMultimap#of(Object, Object)} over more contrived alternatives. */
// XXX: One can define variants for more than one key-value pair, but at some point the builder
// actually produces nicer code. So it's not clear we should add Refaster templates for those
// actually produces nicer code. So it's not clear we should add Refaster rules for those
// variants.
static final class PairToImmutableSetMultimap<K, V> {
@BeforeTemplate

View File

@@ -1,4 +1,4 @@
package tech.picnic.errorprone.refastertemplates;
package tech.picnic.errorprone.refasterrules;
import static com.google.common.collect.ImmutableSet.toImmutableSet;
import static com.google.errorprone.refaster.ImportPolicy.STATIC_IMPORT_ALWAYS;
@@ -17,14 +17,16 @@ import java.util.Collection;
import java.util.Iterator;
import java.util.Set;
import java.util.stream.Stream;
import tech.picnic.errorprone.refaster.annotation.OnlineDocumentation;
/** Refaster templates related to expressions dealing with {@link ImmutableSet}s. */
final class ImmutableSetTemplates {
private ImmutableSetTemplates() {}
/** Refaster rules related to expressions dealing with {@link ImmutableSet}s. */
@OnlineDocumentation
final class ImmutableSetRules {
private ImmutableSetRules() {}
/** Prefer {@link ImmutableSet#builder()} over the associated constructor. */
// XXX: This drops generic type information, sometimes leading to non-compilable code. Anything
// we can do about that?
// XXX: This drops generic type information, sometimes leading to non-compilable code. See
// https://github.com/google/error-prone/pull/2706.
static final class ImmutableSetBuilder<T> {
@BeforeTemplate
ImmutableSet.Builder<T> before() {
@@ -104,7 +106,7 @@ final class ImmutableSetTemplates {
* communicate the immutability of the resulting set at the type level.
*/
// XXX: The `Stream` variant may be too contrived to warrant inclusion. Review its usage if/when
// this and similar Refaster templates are replaced with an Error Prone check.
// this and similar Refaster rules are replaced with an Error Prone check.
static final class ImmutableSetOf<T> {
@BeforeTemplate
Set<T> before() {
@@ -142,7 +144,7 @@ final class ImmutableSetTemplates {
* Prefer {@link ImmutableSet#of(Object, Object)} over alternatives that don't communicate the
* immutability of the resulting set at the type level.
*/
// XXX: Consider writing an Error Prone check which also flags straightforward
// XXX: Consider writing an Error Prone check that also flags straightforward
// `ImmutableSet.builder()` usages.
static final class ImmutableSetOf2<T> {
@BeforeTemplate
@@ -160,7 +162,7 @@ final class ImmutableSetTemplates {
* Prefer {@link ImmutableSet#of(Object, Object, Object)} over alternatives that don't communicate
* the immutability of the resulting set at the type level.
*/
// XXX: Consider writing an Error Prone check which also flags straightforward
// XXX: Consider writing an Error Prone check that also flags straightforward
// `ImmutableSet.builder()` usages.
static final class ImmutableSetOf3<T> {
@BeforeTemplate
@@ -178,7 +180,7 @@ final class ImmutableSetTemplates {
* Prefer {@link ImmutableSet#of(Object, Object, Object, Object)} over alternatives that don't
* communicate the immutability of the resulting set at the type level.
*/
// XXX: Consider writing an Error Prone check which also flags straightforward
// XXX: Consider writing an Error Prone check that also flags straightforward
// `ImmutableSet.builder()` usages.
static final class ImmutableSetOf4<T> {
@BeforeTemplate
@@ -196,7 +198,7 @@ final class ImmutableSetTemplates {
* Prefer {@link ImmutableSet#of(Object, Object, Object, Object, Object)} over alternatives that
* don't communicate the immutability of the resulting set at the type level.
*/
// XXX: Consider writing an Error Prone check which also flags straightforward
// XXX: Consider writing an Error Prone check that also flags straightforward
// `ImmutableSet.builder()` usages.
static final class ImmutableSetOf5<T> {
@BeforeTemplate

View File

@@ -1,4 +1,4 @@
package tech.picnic.errorprone.refastertemplates;
package tech.picnic.errorprone.refasterrules;
import static com.google.common.collect.ImmutableSortedMap.toImmutableSortedMap;
import static java.util.Comparator.naturalOrder;
@@ -13,10 +13,12 @@ import java.util.Collection;
import java.util.Comparator;
import java.util.Map;
import java.util.stream.Stream;
import tech.picnic.errorprone.refaster.annotation.OnlineDocumentation;
/** Refaster templates related to expressions dealing with {@link ImmutableSortedMap}s. */
final class ImmutableSortedMapTemplates {
private ImmutableSortedMapTemplates() {}
/** Refaster rules related to expressions dealing with {@link ImmutableSortedMap}s. */
@OnlineDocumentation
final class ImmutableSortedMapRules {
private ImmutableSortedMapRules() {}
/** Prefer {@link ImmutableSortedMap#orderedBy(Comparator)} over the associated constructor. */
static final class ImmutableSortedMapBuilder<K, V> {
@@ -35,8 +37,8 @@ final class ImmutableSortedMapTemplates {
* Prefer {@link ImmutableSortedMap#naturalOrder()} over the alternative that requires explicitly
* providing the {@link Comparator}.
*/
// XXX: This drops generic type information, sometimes leading to non-compilable code. Anything
// we can do about that?
// XXX: This drops generic type information, sometimes leading to non-compilable code. See
// https://github.com/google/error-prone/pull/2706.
static final class ImmutableSortedMapNaturalOrderBuilder<K extends Comparable<? super K>, V> {
@BeforeTemplate
ImmutableSortedMap.Builder<K, V> before() {
@@ -53,8 +55,8 @@ final class ImmutableSortedMapTemplates {
* Prefer {@link ImmutableSortedMap#reverseOrder()} over the alternative that requires explicitly
* providing the {@link Comparator}.
*/
// XXX: This drops generic type information, sometimes leading to non-compilable code. Anything
// we can do about that?
// XXX: This drops generic type information, sometimes leading to non-compilable code. See
// https://github.com/google/error-prone/pull/2706.
static final class ImmutableSortedMapReverseOrderBuilder<K extends Comparable<? super K>, V> {
@BeforeTemplate
ImmutableSortedMap.Builder<K, V> before() {
@@ -82,7 +84,7 @@ final class ImmutableSortedMapTemplates {
/** Prefer {@link ImmutableSortedMap#of(Object, Object)} over more contrived alternatives. */
// XXX: One can define variants for more than one key-value pair, but at some point the builder
// actually produces nicer code. So it's not clear we should add Refaster templates for those
// actually produces nicer code. So it's not clear we should add Refaster rules for those
// variants.
// XXX: We could also rewrite builders with non-natural orders, but that would affect
// `ImmutableSortedMap#comparator()`.

View File

@@ -1,4 +1,4 @@
package tech.picnic.errorprone.refastertemplates;
package tech.picnic.errorprone.refasterrules;
import static com.google.common.collect.ImmutableSortedMultiset.toImmutableSortedMultiset;
import static com.google.errorprone.refaster.ImportPolicy.STATIC_IMPORT_ALWAYS;
@@ -15,10 +15,12 @@ import java.util.Collection;
import java.util.Comparator;
import java.util.Iterator;
import java.util.stream.Stream;
import tech.picnic.errorprone.refaster.annotation.OnlineDocumentation;
/** Refaster templates related to expressions dealing with {@link ImmutableSortedMultiset}s. */
final class ImmutableSortedMultisetTemplates {
private ImmutableSortedMultisetTemplates() {}
/** Refaster rules related to expressions dealing with {@link ImmutableSortedMultiset}s. */
@OnlineDocumentation
final class ImmutableSortedMultisetRules {
private ImmutableSortedMultisetRules() {}
/**
* Prefer {@link ImmutableSortedMultiset#orderedBy(Comparator)} over the associated constructor.

View File

@@ -1,4 +1,4 @@
package tech.picnic.errorprone.refastertemplates;
package tech.picnic.errorprone.refasterrules;
import static com.google.common.collect.ImmutableSortedSet.toImmutableSortedSet;
import static com.google.errorprone.refaster.ImportPolicy.STATIC_IMPORT_ALWAYS;
@@ -15,10 +15,12 @@ import java.util.Collection;
import java.util.Comparator;
import java.util.Iterator;
import java.util.stream.Stream;
import tech.picnic.errorprone.refaster.annotation.OnlineDocumentation;
/** Refaster templates related to expressions dealing with {@link ImmutableSortedSet}s. */
final class ImmutableSortedSetTemplates {
private ImmutableSortedSetTemplates() {}
/** Refaster rules related to expressions dealing with {@link ImmutableSortedSet}s. */
@OnlineDocumentation
final class ImmutableSortedSetRules {
private ImmutableSortedSetRules() {}
/** Prefer {@link ImmutableSortedSet#orderedBy(Comparator)} over the associated constructor. */
static final class ImmutableSortedSetBuilder<T> {

View File

@@ -1,4 +1,4 @@
package tech.picnic.errorprone.refastertemplates;
package tech.picnic.errorprone.refasterrules;
import com.google.common.collect.Streams;
import com.google.errorprone.refaster.Refaster;
@@ -12,10 +12,12 @@ import java.util.function.IntPredicate;
import java.util.function.IntUnaryOperator;
import java.util.stream.IntStream;
import java.util.stream.Stream;
import tech.picnic.errorprone.refaster.annotation.OnlineDocumentation;
/** Refaster templates related to expressions dealing with {@link IntStream}s. */
final class IntStreamTemplates {
private IntStreamTemplates() {}
/** Refaster rules related to expressions dealing with {@link IntStream}s. */
@OnlineDocumentation
final class IntStreamRules {
private IntStreamRules() {}
/** Prefer {@link IntStream#range(int, int)} over the more contrived alternative. */
static final class IntStreamClosedOpenRange {

View File

@@ -1,4 +1,4 @@
package tech.picnic.errorprone.refastertemplates;
package tech.picnic.errorprone.refasterrules;
import static com.google.errorprone.refaster.ImportPolicy.STATIC_IMPORT_ALWAYS;
import static org.junit.jupiter.params.provider.Arguments.arguments;
@@ -8,10 +8,12 @@ import com.google.errorprone.refaster.annotation.BeforeTemplate;
import com.google.errorprone.refaster.annotation.Repeated;
import com.google.errorprone.refaster.annotation.UseImportPolicy;
import org.junit.jupiter.params.provider.Arguments;
import tech.picnic.errorprone.refaster.annotation.OnlineDocumentation;
/** Refaster templates related to JUnit expressions and statements. */
final class JUnitTemplates {
private JUnitTemplates() {}
/** Refaster rules related to JUnit expressions and statements. */
@OnlineDocumentation
final class JUnitRules {
private JUnitRules() {}
/** Prefer statically imported {@link Arguments#arguments} over {@link Arguments#of} calls. */
static final class ArgumentsEnumeration<T> {

View File

@@ -1,4 +1,4 @@
package tech.picnic.errorprone.refastertemplates;
package tech.picnic.errorprone.refasterrules;
import com.google.common.collect.Streams;
import com.google.errorprone.refaster.Refaster;
@@ -12,10 +12,12 @@ import java.util.function.LongPredicate;
import java.util.function.LongUnaryOperator;
import java.util.stream.LongStream;
import java.util.stream.Stream;
import tech.picnic.errorprone.refaster.annotation.OnlineDocumentation;
/** Refaster templates related to expressions dealing with {@link LongStream}s. */
final class LongStreamTemplates {
private LongStreamTemplates() {}
/** Refaster rules related to expressions dealing with {@link LongStream}s. */
@OnlineDocumentation
final class LongStreamRules {
private LongStreamRules() {}
/** Prefer {@link LongStream#range(long, long)} over the more contrived alternative. */
static final class LongStreamClosedOpenRange {

View File

@@ -1,4 +1,4 @@
package tech.picnic.errorprone.refastertemplates;
package tech.picnic.errorprone.refasterrules;
import static com.google.errorprone.refaster.ImportPolicy.STATIC_IMPORT_ALWAYS;
import static java.util.Comparator.comparing;
@@ -14,10 +14,12 @@ import com.google.errorprone.refaster.annotation.UseImportPolicy;
import java.util.AbstractMap;
import java.util.Comparator;
import java.util.Map;
import tech.picnic.errorprone.refaster.annotation.OnlineDocumentation;
/** Refaster templates related to expressions dealing with {@link Map.Entry} instances. */
final class MapEntryTemplates {
private MapEntryTemplates() {}
/** Refaster rules related to expressions dealing with {@link Map.Entry} instances. */
@OnlineDocumentation
final class MapEntryRules {
private MapEntryRules() {}
/**
* Prefer {@link Map#entry(Object, Object)} over alternative ways to create an immutable map

View File

@@ -1,4 +1,4 @@
package tech.picnic.errorprone.refastertemplates;
package tech.picnic.errorprone.refasterrules;
import static com.google.errorprone.refaster.ImportPolicy.STATIC_IMPORT_ALWAYS;
import static org.mockito.Mockito.never;
@@ -10,10 +10,12 @@ import com.google.errorprone.refaster.annotation.BeforeTemplate;
import com.google.errorprone.refaster.annotation.UseImportPolicy;
import org.mockito.Mockito;
import org.mockito.verification.VerificationMode;
import tech.picnic.errorprone.refaster.annotation.OnlineDocumentation;
/** Refaster templates related to Mockito expressions and statements. */
final class MockitoTemplates {
private MockitoTemplates() {}
/** Refaster rules related to Mockito expressions and statements. */
@OnlineDocumentation
final class MockitoRules {
private MockitoRules() {}
/**
* Prefer {@link Mockito#never()}} over explicitly specifying that the associated invocation must

View File

@@ -1,4 +1,4 @@
package tech.picnic.errorprone.refastertemplates;
package tech.picnic.errorprone.refasterrules;
import com.google.common.collect.Multimap;
import com.google.common.collect.Multimaps;
@@ -7,11 +7,13 @@ import com.google.errorprone.refaster.annotation.AfterTemplate;
import com.google.errorprone.refaster.annotation.BeforeTemplate;
import java.util.Collection;
import java.util.Set;
import javax.annotation.Nullable;
import org.jspecify.nullness.Nullable;
import tech.picnic.errorprone.refaster.annotation.OnlineDocumentation;
/** Refaster templates related to expressions dealing with {@link Multimap}s. */
final class MultimapTemplates {
private MultimapTemplates() {}
/** Refaster rules related to expressions dealing with {@link Multimap}s. */
@OnlineDocumentation
final class MultimapRules {
private MultimapRules() {}
/** Prefer {@link Multimap#keySet()} over more contrived alternatives. */
static final class MultimapKeySet<K, V> {
@@ -48,8 +50,7 @@ final class MultimapTemplates {
*/
static final class MultimapGet<K, V> {
@BeforeTemplate
@Nullable
Collection<V> before(Multimap<K, V> multimap, K key) {
@Nullable Collection<V> before(Multimap<K, V> multimap, K key) {
return Refaster.anyOf(multimap.asMap(), Multimaps.asMap(multimap)).get(key);
}

View File

@@ -1,4 +1,4 @@
package tech.picnic.errorprone.refastertemplates;
package tech.picnic.errorprone.refasterrules;
import static com.google.errorprone.refaster.ImportPolicy.STATIC_IMPORT_ALWAYS;
import static java.util.Objects.requireNonNullElse;
@@ -9,11 +9,13 @@ import com.google.errorprone.refaster.annotation.BeforeTemplate;
import com.google.errorprone.refaster.annotation.UseImportPolicy;
import java.util.Objects;
import java.util.function.Predicate;
import javax.annotation.Nullable;
import org.jspecify.nullness.Nullable;
import tech.picnic.errorprone.refaster.annotation.OnlineDocumentation;
/** Refaster templates related to expressions dealing with (possibly) null values. */
final class NullTemplates {
private NullTemplates() {}
/** Refaster rules related to expressions dealing with (possibly) null values. */
@OnlineDocumentation
final class NullRules {
private NullRules() {}
/** Prefer the {@code ==} operator over {@link Objects#isNull(Object)}. */
static final class IsNull {

View File

@@ -1,4 +1,4 @@
package tech.picnic.errorprone.refastertemplates;
package tech.picnic.errorprone.refasterrules;
import static com.google.errorprone.refaster.ImportPolicy.STATIC_IMPORT_ALWAYS;
@@ -16,11 +16,13 @@ import java.util.function.Function;
import java.util.function.Predicate;
import java.util.function.Supplier;
import java.util.stream.Stream;
import javax.annotation.Nullable;
import org.jspecify.nullness.Nullable;
import tech.picnic.errorprone.refaster.annotation.OnlineDocumentation;
/** Refaster templates related to expressions dealing with {@link Optional}s. */
final class OptionalTemplates {
private OptionalTemplates() {}
/** Refaster rules related to expressions dealing with {@link Optional}s. */
@OnlineDocumentation
final class OptionalRules {
private OptionalRules() {}
static final class OptionalOfNullable<T> {
// XXX: Refaster should be smart enough to also rewrite occurrences in which there are
@@ -78,8 +80,8 @@ final class OptionalTemplates {
}
/** Prefer {@link Optional#orElseThrow()} over the less explicit {@link Optional#get()}. */
// XXX: This template is analogous to `OptionalOrElseThrow` above. Arguably this is its
// generalization. If/when Refaster is extended to understand this, delete the template above.
// XXX: This rule is analogous to `OptionalOrElseThrow` above. Arguably this is its
// generalization. If/when Refaster is extended to understand this, delete the rule above.
static final class OptionalOrElseThrowMethodReference<T> {
@BeforeTemplate
Function<Optional<T>, T> before() {
@@ -153,7 +155,7 @@ final class OptionalTemplates {
* Prefer {@link Optional#filter(Predicate)} over {@link Optional#map(Function)} when converting
* an {@link Optional} to a boolean.
*/
abstract static class MapOptionalToBoolean<T> {
static final class MapOptionalToBoolean<T> {
@BeforeTemplate
boolean before(Optional<T> optional, Function<? super T, Boolean> predicate) {
return optional.map(predicate).orElse(Refaster.anyOf(false, Boolean.FALSE));
@@ -166,7 +168,7 @@ final class OptionalTemplates {
}
/**
* Prefer {@link Optional#map} over a {@link Optional#flatMap} which wraps the result of a
* Prefer {@link Optional#map} over a {@link Optional#flatMap} that wraps the result of a
* transformation in an {@link Optional}; the former operation transforms {@code null} to {@link
* Optional#empty()}.
*/
@@ -315,7 +317,7 @@ final class OptionalTemplates {
}
/** Prefer {@link Optional#or(Supplier)} over more verbose alternatives. */
abstract static class OptionalOrOtherOptional<T> {
static final class OptionalOrOtherOptional<T> {
@BeforeTemplate
@SuppressWarnings("NestedOptionals" /* Auto-fix for the `NestedOptionals` check. */)
Optional<T> before(Optional<T> optional1, Optional<T> optional2) {

View File

@@ -0,0 +1,176 @@
package tech.picnic.errorprone.refasterrules;
import static com.google.common.base.Preconditions.checkArgument;
import static com.google.common.base.Preconditions.checkElementIndex;
import static com.google.common.base.Preconditions.checkNotNull;
import static com.google.common.base.Preconditions.checkPositionIndex;
import static com.google.common.base.Preconditions.checkState;
import static com.google.errorprone.refaster.ImportPolicy.STATIC_IMPORT_ALWAYS;
import com.google.common.base.Preconditions;
import com.google.errorprone.refaster.annotation.AfterTemplate;
import com.google.errorprone.refaster.annotation.BeforeTemplate;
import com.google.errorprone.refaster.annotation.UseImportPolicy;
import tech.picnic.errorprone.refaster.annotation.OnlineDocumentation;
/** Refaster templates related to statements dealing with {@link Preconditions}. */
@OnlineDocumentation
final class PreconditionsRules {
private PreconditionsRules() {}
/** Prefer {@link Preconditions#checkArgument(boolean)} over more verbose alternatives. */
static final class CheckArgument {
@BeforeTemplate
void before(boolean condition) {
if (condition) {
throw new IllegalArgumentException();
}
}
@AfterTemplate
@UseImportPolicy(STATIC_IMPORT_ALWAYS)
void after(boolean condition) {
checkArgument(!condition);
}
}
/** Prefer {@link Preconditions#checkArgument(boolean, Object)} over more verbose alternatives. */
static final class CheckArgumentWithMessage {
@BeforeTemplate
void before(boolean condition, String message) {
if (condition) {
throw new IllegalArgumentException(message);
}
}
@AfterTemplate
@UseImportPolicy(STATIC_IMPORT_ALWAYS)
void after(boolean condition, String message) {
checkArgument(!condition, message);
}
}
/**
* Prefer {@link Preconditions#checkElementIndex(int, int, String)} over less descriptive or more
* verbose alternatives.
*
* <p>Note that the two-argument {@link Preconditions#checkElementIndex(int, int)} is better
* replaced with {@link java.util.Objects#checkIndex(int, int)}.
*/
static final class CheckElementIndexWithMessage {
@BeforeTemplate
void before(int index, int size, String message) {
if (index < 0 || index >= size) {
throw new IndexOutOfBoundsException(message);
}
}
@AfterTemplate
@UseImportPolicy(STATIC_IMPORT_ALWAYS)
void after(int index, int size, String message) {
checkElementIndex(index, size, message);
}
}
/** Prefer {@link Preconditions#checkNotNull(Object)} over more verbose alternatives. */
static final class CheckNotNull<T> {
@BeforeTemplate
void before(T object) {
if (object == null) {
throw new NullPointerException();
}
}
@AfterTemplate
@UseImportPolicy(STATIC_IMPORT_ALWAYS)
void after(T object) {
checkNotNull(object);
}
}
/** Prefer {@link Preconditions#checkNotNull(Object, Object)} over more verbose alternatives. */
static final class CheckNotNullWithMessage<T> {
@BeforeTemplate
void before(T object, String message) {
if (object == null) {
throw new NullPointerException(message);
}
}
@AfterTemplate
@UseImportPolicy(STATIC_IMPORT_ALWAYS)
void after(T object, String message) {
checkNotNull(object, message);
}
}
/**
* Prefer {@link Preconditions#checkPositionIndex(int, int)} over less descriptive or more verbose
* alternatives.
*/
static final class CheckPositionIndex {
@BeforeTemplate
void before(int index, int size) {
if (index < 0 || index > size) {
throw new IndexOutOfBoundsException();
}
}
@AfterTemplate
@UseImportPolicy(STATIC_IMPORT_ALWAYS)
void after(int index, int size) {
checkPositionIndex(index, size);
}
}
/**
* Prefer {@link Preconditions#checkPositionIndex(int, int, String)} over less descriptive or more
* verbose alternatives.
*/
static final class CheckPositionIndexWithMessage {
@BeforeTemplate
void before(int index, int size, String message) {
if (index < 0 || index > size) {
throw new IndexOutOfBoundsException(message);
}
}
@AfterTemplate
@UseImportPolicy(STATIC_IMPORT_ALWAYS)
void after(int index, int size, String message) {
checkPositionIndex(index, size, message);
}
}
/** Prefer {@link Preconditions#checkState(boolean)} over more verbose alternatives. */
static final class CheckState {
@BeforeTemplate
void before(boolean condition) {
if (condition) {
throw new IllegalStateException();
}
}
@AfterTemplate
@UseImportPolicy(STATIC_IMPORT_ALWAYS)
void after(boolean condition) {
checkState(!condition);
}
}
/** Prefer {@link Preconditions#checkState(boolean, Object)} over more verbose alternatives. */
static final class CheckStateWithMessage {
@BeforeTemplate
void before(boolean condition, String message) {
if (condition) {
throw new IllegalStateException(message);
}
}
@AfterTemplate
@UseImportPolicy(STATIC_IMPORT_ALWAYS)
void after(boolean condition, String message) {
checkState(!condition, message);
}
}
}

View File

@@ -1,12 +1,14 @@
package tech.picnic.errorprone.refastertemplates;
package tech.picnic.errorprone.refasterrules;
import com.google.common.primitives.Ints;
import com.google.errorprone.refaster.annotation.AfterTemplate;
import com.google.errorprone.refaster.annotation.BeforeTemplate;
import tech.picnic.errorprone.refaster.annotation.OnlineDocumentation;
/** Refaster templates related to expressions dealing with primitives. */
final class PrimitiveTemplates {
private PrimitiveTemplates() {}
/** Refaster rules related to expressions dealing with primitives. */
@OnlineDocumentation
final class PrimitiveRules {
private PrimitiveRules() {}
/** Avoid contrived ways of expressing the "less than" relationship. */
static final class LessThan {

View File

@@ -1,9 +1,11 @@
package tech.picnic.errorprone.refastertemplates;
package tech.picnic.errorprone.refasterrules;
import static com.google.common.collect.MoreCollectors.toOptional;
import static com.google.errorprone.refaster.ImportPolicy.STATIC_IMPORT_ALWAYS;
import static java.util.function.Function.identity;
import static org.assertj.core.api.Assertions.assertThat;
import com.google.common.collect.ImmutableMap;
import com.google.common.collect.MoreCollectors;
import com.google.errorprone.refaster.Refaster;
import com.google.errorprone.refaster.annotation.AfterTemplate;
@@ -13,6 +15,7 @@ import com.google.errorprone.refaster.annotation.NotMatches;
import com.google.errorprone.refaster.annotation.Placeholder;
import com.google.errorprone.refaster.annotation.UseImportPolicy;
import java.time.Duration;
import java.util.HashMap;
import java.util.Optional;
import java.util.concurrent.Callable;
import java.util.function.Consumer;
@@ -24,11 +27,14 @@ import reactor.core.publisher.Flux;
import reactor.core.publisher.Mono;
import reactor.test.StepVerifier;
import reactor.test.publisher.PublisherProbe;
import tech.picnic.errorprone.refaster.util.ThrowsCheckedException;
import reactor.util.context.Context;
import tech.picnic.errorprone.refaster.annotation.OnlineDocumentation;
import tech.picnic.errorprone.refaster.matchers.ThrowsCheckedException;
/** Refaster templates related to Reactor expressions and statements. */
final class ReactorTemplates {
private ReactorTemplates() {}
/** Refaster rules related to Reactor expressions and statements. */
@OnlineDocumentation
final class ReactorRules {
private ReactorRules() {}
/**
* Prefer {@link Mono#fromSupplier(Supplier)} over {@link Mono#fromCallable(Callable)} where
@@ -94,7 +100,7 @@ final class ReactorTemplates {
/**
* Don't unnecessarily pass {@link Mono#error(Supplier)} a method reference or lambda expression.
*/
// XXX: Drop this rule once the more general rule `AssortedTemplates#SupplierAsSupplier` works
// XXX: Drop this rule once the more general rule `AssortedRules#SupplierAsSupplier` works
// reliably.
static final class MonoErrorSupplier<T, E extends Throwable> {
@BeforeTemplate
@@ -111,7 +117,7 @@ final class ReactorTemplates {
/**
* Don't unnecessarily pass {@link Flux#error(Supplier)} a method reference or lambda expression.
*/
// XXX: Drop this rule once the more general rule `AssortedTemplates#SupplierAsSupplier` works
// XXX: Drop this rule once the more general rule `AssortedRules#SupplierAsSupplier` works
// reliably.
static final class FluxErrorSupplier<T, E extends Throwable> {
@BeforeTemplate
@@ -177,6 +183,26 @@ final class ReactorTemplates {
}
}
/** Prefer {@link Flux#concatMap(Function, int)} over more contrived alternatives. */
static final class FluxConcatMapWithPrefetch<T, S> {
@BeforeTemplate
Flux<S> before(
Flux<T> flux,
Function<? super T, ? extends Publisher<? extends S>> function,
int prefetch) {
return Refaster.anyOf(
flux.flatMap(function, 1, prefetch), flux.flatMapSequential(function, 1, prefetch));
}
@AfterTemplate
Flux<S> after(
Flux<T> flux,
Function<? super T, ? extends Publisher<? extends S>> function,
int prefetch) {
return flux.concatMap(function, prefetch);
}
}
/**
* Prefer {@link Flux#concatMapIterable(Function)} over {@link Flux#flatMapIterable(Function)}, as
* the former has equivalent semantics but a clearer name.
@@ -193,6 +219,24 @@ final class ReactorTemplates {
}
}
/**
* Prefer {@link Flux#concatMapIterable(Function, int)} over {@link Flux#flatMapIterable(Function,
* int)}, as the former has equivalent semantics but a clearer name.
*/
static final class FluxConcatMapIterableWithPrefetch<T, S> {
@BeforeTemplate
Flux<S> before(
Flux<T> flux, Function<? super T, ? extends Iterable<? extends S>> function, int prefetch) {
return flux.flatMapIterable(function, prefetch);
}
@AfterTemplate
Flux<S> after(
Flux<T> flux, Function<? super T, ? extends Iterable<? extends S>> function, int prefetch) {
return flux.concatMapIterable(function, prefetch);
}
}
/**
* Don't use {@link Mono#flatMapMany(Function)} to implicitly convert a {@link Mono} to a {@link
* Flux}.
@@ -228,7 +272,7 @@ final class ReactorTemplates {
/**
* Prefer a collection using {@link MoreCollectors#toOptional()} over more contrived alternatives.
*/
// XXX: Consider creating a plugin which flags/discourages `Mono<Optional<T>>` method return
// XXX: Consider creating a plugin that flags/discourages `Mono<Optional<T>>` method return
// types, just as we discourage nullable `Boolean`s and `Optional`s.
static final class MonoCollectToOptional<T> {
@BeforeTemplate
@@ -271,11 +315,89 @@ final class ReactorTemplates {
}
}
/**
* Prefer {@link Flux#concatMapIterable(Function)} over alternatives that require an additional
* subscription.
*/
static final class ConcatMapIterableIdentity<T> {
@BeforeTemplate
Flux<T> before(Flux<? extends Iterable<T>> flux) {
return Refaster.anyOf(
flux.concatMap(list -> Flux.fromIterable(list)), flux.concatMap(Flux::fromIterable));
}
@AfterTemplate
@UseImportPolicy(STATIC_IMPORT_ALWAYS)
Flux<T> after(Flux<? extends Iterable<T>> flux) {
return flux.concatMapIterable(identity());
}
}
/**
* Prefer {@link Flux#concatMapIterable(Function, int)} over alternatives that require an
* additional subscription.
*/
static final class ConcatMapIterableIdentityWithPrefetch<T> {
@BeforeTemplate
Flux<T> before(Flux<? extends Iterable<T>> flux, int prefetch) {
return Refaster.anyOf(
flux.concatMap(list -> Flux.fromIterable(list), prefetch),
flux.concatMap(Flux::fromIterable, prefetch));
}
@AfterTemplate
@UseImportPolicy(STATIC_IMPORT_ALWAYS)
Flux<T> after(Flux<? extends Iterable<T>> flux, int prefetch) {
return flux.concatMapIterable(identity(), prefetch);
}
}
/** Prefer {@link Mono#onErrorComplete()} over more contrived alternatives. */
static final class MonoOnErrorComplete<T> {
@BeforeTemplate
Mono<T> before(Mono<T> mono) {
return mono.onErrorResume(e -> Mono.empty());
}
@AfterTemplate
Mono<T> after(Mono<T> mono) {
return mono.onErrorComplete();
}
}
/** Prefer {@link Flux#onErrorComplete()} over more contrived alternatives. */
static final class FluxOnErrorComplete<T> {
@BeforeTemplate
Flux<T> before(Flux<T> flux) {
return flux.onErrorResume(e -> Refaster.anyOf(Mono.empty(), Flux.empty()));
}
@AfterTemplate
Flux<T> after(Flux<T> flux) {
return flux.onErrorComplete();
}
}
/** Prefer {@link reactor.util.context.Context#empty()}} over more verbose alternatives. */
// XXX: Consider introducing an `IsEmpty` matcher that identifies a wide range of guaranteed-empty
// `Collection` and `Map` expressions.
static final class ContextEmpty {
@BeforeTemplate
Context before() {
return Context.of(Refaster.anyOf(new HashMap<>(), ImmutableMap.of()));
}
@AfterTemplate
Context after() {
return Context.empty();
}
}
/** Prefer {@link PublisherProbe#empty()}} over more verbose alternatives. */
static final class PublisherProbeEmpty<T> {
@BeforeTemplate
PublisherProbe<T> before() {
return Refaster.anyOf(PublisherProbe.of(Mono.empty()), PublisherProbe.of(Flux.empty()));
return PublisherProbe.of(Refaster.anyOf(Mono.empty(), Flux.empty()));
}
@AfterTemplate

View File

@@ -1,4 +1,4 @@
package tech.picnic.errorprone.refastertemplates;
package tech.picnic.errorprone.refasterrules;
import com.google.errorprone.refaster.Refaster;
import com.google.errorprone.refaster.annotation.AfterTemplate;
@@ -13,10 +13,12 @@ import org.jspecify.nullness.Nullable;
import reactor.adapter.rxjava.RxJava2Adapter;
import reactor.core.publisher.Flux;
import reactor.core.publisher.Mono;
import tech.picnic.errorprone.refaster.annotation.OnlineDocumentation;
/** Refaster templates related to expressions dealing with {@link RxJava2Adapter}. */
final class RxJava2AdapterTemplates {
private RxJava2AdapterTemplates() {}
/** Refaster rules related to expressions dealing with {@link RxJava2Adapter}. */
@OnlineDocumentation
final class RxJava2AdapterRules {
private RxJava2AdapterRules() {}
/** Use the fluent API style when using {@link RxJava2Adapter#completableToMono}. */
static final class CompletableToMono {

View File

@@ -1,4 +1,4 @@
package tech.picnic.errorprone.refastertemplates;
package tech.picnic.errorprone.refasterrules;
import static com.google.errorprone.refaster.ImportPolicy.STATIC_IMPORT_ALWAYS;
import static java.util.Comparator.naturalOrder;
@@ -22,10 +22,12 @@ import java.util.function.Predicate;
import java.util.stream.Collector;
import java.util.stream.Collectors;
import java.util.stream.Stream;
import tech.picnic.errorprone.refaster.annotation.OnlineDocumentation;
/** Refaster templates related to expressions dealing with {@link Stream}s. */
final class StreamTemplates {
private StreamTemplates() {}
/** Refaster rules related to expressions dealing with {@link Stream}s. */
@OnlineDocumentation
final class StreamRules {
private StreamRules() {}
/**
* Prefer {@link Collectors#joining()} over {@link Collectors#joining(CharSequence)} with an empty

View File

@@ -1,4 +1,4 @@
package tech.picnic.errorprone.refastertemplates;
package tech.picnic.errorprone.refasterrules;
import static java.nio.charset.StandardCharsets.UTF_8;
import static java.util.stream.Collectors.joining;
@@ -16,12 +16,14 @@ import java.util.Collection;
import java.util.Objects;
import java.util.Optional;
import java.util.function.Function;
import javax.annotation.Nullable;
import org.jspecify.nullness.Nullable;
import tech.picnic.errorprone.refaster.annotation.OnlineDocumentation;
/** Refaster templates related to expressions dealing with {@link String}s. */
/** Refaster rules related to expressions dealing with {@link String}s. */
// XXX: Should we prefer `s -> !s.isEmpty()` or `not(String::isEmpty)`?
final class StringTemplates {
private StringTemplates() {}
@OnlineDocumentation
final class StringRules {
private StringRules() {}
/** Prefer {@link String#isEmpty()} over alternatives that consult the string's length. */
static final class StringIsEmpty {
@@ -128,8 +130,8 @@ final class StringTemplates {
* Prefer direct delegation to {@link String#valueOf(Object)} over the indirection introduced by
* {@link Objects#toString(Object)}.
*/
// XXX: This template is analogous to `StringValueOf` above. Arguably this is its generalization.
// If/when Refaster is extended to understand this, delete the template above.
// XXX: This rule is analogous to `StringValueOf` above. Arguably this is its generalization.
// If/when Refaster is extended to understand this, delete the rule above.
static final class StringValueOfMethodReference {
@BeforeTemplate
Function<Object, String> before() {

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