mirror of
https://github.com/jlengrand/error-prone-support.git
synced 2026-03-10 15:49:33 +00:00
Compare commits
84 Commits
v0.2.0
...
pdsoels/do
| Author | SHA1 | Date | |
|---|---|---|---|
|
|
a71e90f856 | ||
|
|
af794e5fc6 | ||
|
|
6d25ff68b0 | ||
|
|
bd048d2a0e | ||
|
|
d5a78186db | ||
|
|
128e178d37 | ||
|
|
7aef2cfe51 | ||
|
|
5cec0dd508 | ||
|
|
3f1399c139 | ||
|
|
757d5b1d70 | ||
|
|
45cac6105e | ||
|
|
8f8f57fc5b | ||
|
|
902d4f7736 | ||
|
|
4ec349582c | ||
|
|
57836103ea | ||
|
|
bd73243c34 | ||
|
|
9af50d7e0b | ||
|
|
f5ae47fbac | ||
|
|
9ad8c275d3 | ||
|
|
f4e191e33b | ||
|
|
c4e476a731 | ||
|
|
5ca95eb36d | ||
|
|
9204ef0e84 | ||
|
|
ed45be4e15 | ||
|
|
0561c371de | ||
|
|
aa5ad4d25b | ||
|
|
8c0041a94e | ||
|
|
397f9c3df7 | ||
|
|
2ba7bf9f46 | ||
|
|
5b079eef84 | ||
|
|
a2ce053daf | ||
|
|
ba02bad9bf | ||
|
|
d97a20247f | ||
|
|
50970eb932 | ||
|
|
7e7318ad80 | ||
|
|
fb6fe5a96e | ||
|
|
e37da2a1ed | ||
|
|
7bef1c8e67 | ||
|
|
0160eafca0 | ||
|
|
891fecd297 | ||
|
|
7f18bd9030 | ||
|
|
1473a70de8 | ||
|
|
c0c3ce2644 | ||
|
|
100d5c86f7 | ||
|
|
e12f99975b | ||
|
|
791113669f | ||
|
|
564bc7e1d1 | ||
|
|
43bcbeaa98 | ||
|
|
d682b7d41f | ||
|
|
72a124a20e | ||
|
|
326f3328a7 | ||
|
|
ae7068f464 | ||
|
|
066931fe53 | ||
|
|
3874ca9be2 | ||
|
|
84ba3946d9 | ||
|
|
b675ff680f | ||
|
|
8e7d04a24c | ||
|
|
bfc951b61f | ||
|
|
b30562bbd8 | ||
|
|
9be85204ae | ||
|
|
6fbf1b0cb2 | ||
|
|
5a428e6e29 | ||
|
|
71163c0061 | ||
|
|
880be0dbdd | ||
|
|
62fe10f2cd | ||
|
|
f7ec28368a | ||
|
|
e34c2baf7c | ||
|
|
184ba8af51 | ||
|
|
63c3aa8259 | ||
|
|
7daabeee48 | ||
|
|
0acfd8a723 | ||
|
|
000fcefe92 | ||
|
|
1e3ad0fe32 | ||
|
|
b88a668819 | ||
|
|
4c8e125dcb | ||
|
|
4ab5dc4f32 | ||
|
|
7c667334cc | ||
|
|
b4b2afd130 | ||
|
|
4445c93f6e | ||
|
|
5a7d7ff89b | ||
|
|
7ef75e8f07 | ||
|
|
a1f2418805 | ||
|
|
39bfcc75ca | ||
|
|
753cdce29e |
51
.github/ISSUE_TEMPLATE/bug_report.md
vendored
Normal file
51
.github/ISSUE_TEMPLATE/bug_report.md
vendored
Normal 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. -->
|
||||
57
.github/ISSUE_TEMPLATE/feature_request.md
vendored
Normal file
57
.github/ISSUE_TEMPLATE/feature_request.md
vendored
Normal 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. -->
|
||||
29
.github/release.yml
vendored
Normal file
29
.github/release.yml
vendored
Normal file
@@ -0,0 +1,29 @@
|
||||
changelog:
|
||||
exclude:
|
||||
labels:
|
||||
- "ignore-changelog"
|
||||
categories:
|
||||
- title: ":rocket: New Error Prone checks and Refaster rules"
|
||||
labels:
|
||||
- "new feature"
|
||||
- title: ":sparkles: Improvements"
|
||||
labels:
|
||||
- "improvement"
|
||||
- title: ":warning: Update considerations and deprecations"
|
||||
labels:
|
||||
- "breaking change"
|
||||
- "deprecation"
|
||||
- title: ":bug: Bug fixes"
|
||||
labels:
|
||||
- "bug"
|
||||
- "bug fix"
|
||||
- title: ":books: Documentation, test and build improvements"
|
||||
labels:
|
||||
- "documentation"
|
||||
- "chore"
|
||||
- title: ":chart_with_upwards_trend: Dependency upgrades"
|
||||
labels:
|
||||
- "dependencies"
|
||||
- title: "Other changes"
|
||||
labels:
|
||||
- "*"
|
||||
13
.github/workflows/build.yaml
vendored
13
.github/workflows/build.yaml
vendored
@@ -2,8 +2,9 @@ name: Build and verify
|
||||
on:
|
||||
pull_request:
|
||||
push:
|
||||
branches:
|
||||
- 'master'
|
||||
branches: [ master ]
|
||||
permissions:
|
||||
contents: read
|
||||
jobs:
|
||||
build:
|
||||
runs-on: ubuntu-22.04
|
||||
@@ -17,17 +18,17 @@ 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.5.1
|
||||
with:
|
||||
java-version: ${{ matrix.jdk }}
|
||||
distribution: temurin
|
||||
cache: maven
|
||||
- name: Display build environment details
|
||||
run: mvn --version
|
||||
- name: Build project against vanilla Error Prone
|
||||
run: mvn -T1C install
|
||||
- name: Build project against vanilla Error Prone, compile Javadoc
|
||||
run: mvn -T1C install javadoc:jar
|
||||
- name: Build project with self-check against Error Prone fork
|
||||
run: mvn -T1C clean verify -Perror-prone-fork -Pnon-maven-central -Pself-check -s settings.xml
|
||||
- name: Remove installed project artifacts
|
||||
|
||||
48
.github/workflows/deploy-website.yaml
vendored
Normal file
48
.github/workflows/deploy-website.yaml
vendored
Normal file
@@ -0,0 +1,48 @@
|
||||
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.117.0
|
||||
with:
|
||||
working-directory: ./website
|
||||
bundler-cache: true
|
||||
- name: Configure Github Pages
|
||||
uses: actions/configure-pages@v2.1.1
|
||||
# XXX: Run website/generate-docs.rb instead.
|
||||
- name: Generate documentation
|
||||
working-directory: ./website
|
||||
run: bundle exec ruby generate-docs.rb
|
||||
- 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.1
|
||||
@@ -2,6 +2,7 @@
|
||||
-XX:SoftRefLRUPolicyMSPerMB=10
|
||||
-XX:+UseParallelGC
|
||||
--add-exports=jdk.compiler/com.sun.tools.javac.api=ALL-UNNAMED
|
||||
--add-exports=jdk.compiler/com.sun.tools.javac.code=ALL-UNNAMED
|
||||
--add-exports=jdk.compiler/com.sun.tools.javac.file=ALL-UNNAMED
|
||||
--add-exports=jdk.compiler/com.sun.tools.javac.main=ALL-UNNAMED
|
||||
--add-exports=jdk.compiler/com.sun.tools.javac.model=ALL-UNNAMED
|
||||
@@ -9,5 +10,4 @@
|
||||
--add-exports=jdk.compiler/com.sun.tools.javac.processing=ALL-UNNAMED
|
||||
--add-exports=jdk.compiler/com.sun.tools.javac.tree=ALL-UNNAMED
|
||||
--add-exports=jdk.compiler/com.sun.tools.javac.util=ALL-UNNAMED
|
||||
--add-opens=jdk.compiler/com.sun.tools.javac.code=ALL-UNNAMED
|
||||
--add-opens=jdk.compiler/com.sun.tools.javac.comp=ALL-UNNAMED
|
||||
|
||||
71
CONTRIBUTING.md
Normal file
71
CONTRIBUTING.md
Normal file
@@ -0,0 +1,71 @@
|
||||
# Contributing
|
||||
|
||||
Thank you for checking this document! This project is free software, and we
|
||||
(the maintainers) encourage and value any contribution.
|
||||
|
||||
Here are some guidelines to help you get started.
|
||||
|
||||
## 🐛 Reporting a bug
|
||||
|
||||
Like any non-trivial piece of software, this library is probably not bug-free.
|
||||
If you found a bug, feel free to [report the issue][error-prone-support-issues]
|
||||
on GitHub.
|
||||
|
||||
Before doing so, please:
|
||||
- Verify that the issue is reproducible against the latest version of the
|
||||
project.
|
||||
- Search through the existing set of issues to see whether the problem is
|
||||
already known. With some luck a solution is already in place, or a workaround
|
||||
may have been provided.
|
||||
|
||||
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
|
||||
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
|
||||
(and optionally fix) the bug.
|
||||
- Any additional information that may be relevant.
|
||||
|
||||
## 💡 Reporting an improvement
|
||||
|
||||
If you would like to see an improvement, you can file a [GitHub
|
||||
issue][error-prone-support-issues]. This is also a good idea when you're
|
||||
already working towards opening a pull request, as this allows for discussion
|
||||
around the idea.
|
||||
|
||||
## 🚀 Opening a pull request
|
||||
|
||||
All submissions, including submissions by project members, require approval by
|
||||
at least two reviewers. We use [GitHub pull
|
||||
requests][error-prone-support-pulls] for this purpose.
|
||||
|
||||
Before opening a pull request, please check whether there are any existing
|
||||
(open or closed) issues or pull requests addressing the same problem. This
|
||||
avoids double work or lots of time spent on a solution that may ultimately not
|
||||
be accepted. When in doubt, make sure to first raise an
|
||||
[issue][error-prone-support-issues] to discuss the idea.
|
||||
|
||||
To the extent possible, the pull request process guards our coding guidelines.
|
||||
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 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
|
||||
checks][error-prone-criteria]. Most guidelines described there apply to this
|
||||
project as well, except that this project _does_ focus quite heavy on style
|
||||
enforcement. But that just makes the previous point doubly important.
|
||||
- Make sure that a check's [(mutation) test
|
||||
coverage][error-prone-support-mutation-tests] is or remains about as high as
|
||||
it can be. Not only does this lead to better tests, it also points out
|
||||
opportunities to simplify the code.
|
||||
- Please restrict the scope of a pull request to a single feature or fix. Don't
|
||||
sneak in unrelated changes; instead just open more than one pull request 😉.
|
||||
|
||||
[error-prone-criteria]: https://errorprone.info/docs/criteria
|
||||
[error-prone-support-issues]: https://github.com/PicnicSupermarket/error-prone-support/issues
|
||||
[error-prone-support-mutation-tests]: https://github.com/PicnicSupermarket/error-prone-support/blob/master/run-mutation-tests.sh
|
||||
[error-prone-support-pulls]: https://github.com/PicnicSupermarket/error-prone-support/pulls
|
||||
21
LICENSE.md
Normal file
21
LICENSE.md
Normal file
@@ -0,0 +1,21 @@
|
||||
MIT License
|
||||
|
||||
Copyright (c) 2017-2022 Picnic Technologies BV
|
||||
|
||||
Permission is hereby granted, free of charge, to any person obtaining a copy
|
||||
of this software and associated documentation files (the "Software"), to deal
|
||||
in the Software without restriction, including without limitation the rights
|
||||
to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
|
||||
copies of the Software, and to permit persons to whom the Software is
|
||||
furnished to do so, subject to the following conditions:
|
||||
|
||||
The above copyright notice and this permission notice shall be included in all
|
||||
copies or substantial portions of the Software.
|
||||
|
||||
THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
|
||||
IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
|
||||
FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE
|
||||
AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
|
||||
LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
|
||||
OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE
|
||||
SOFTWARE.
|
||||
231
README.md
Normal file
231
README.md
Normal file
@@ -0,0 +1,231 @@
|
||||
<div align="center">
|
||||
<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][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) •
|
||||
[Developing Error Prone Support](#-developing-error-prone-support) •
|
||||
[How it works](#-how-it-works) • [Contributing](#%EF%B8%8F-contributing)
|
||||
|
||||
---
|
||||
|
||||
## ⚡ Getting started
|
||||
|
||||
### Installation
|
||||
|
||||
This library is built on top of [Error Prone][error-prone-orig-repo]. To use
|
||||
it:
|
||||
|
||||
1. First, follow Error Prone's [installation
|
||||
guide][error-prone-installation-guide].
|
||||
2. Next, edit your `pom.xml` file to add one or more Error Prone Support
|
||||
modules to the `annotationProcessorPaths` of the `maven-compiler-plugin`:
|
||||
|
||||
```xml
|
||||
<build>
|
||||
<pluginManagement>
|
||||
<plugins>
|
||||
<plugin>
|
||||
<groupId>org.apache.maven.plugins</groupId>
|
||||
<artifactId>maven-compiler-plugin</artifactId>
|
||||
<configuration>
|
||||
<annotationProcessorPaths>
|
||||
<!-- Error Prone itself. -->
|
||||
<path>
|
||||
<groupId>com.google.errorprone</groupId>
|
||||
<artifactId>error_prone_core</artifactId>
|
||||
<version>${error-prone.version}</version>
|
||||
</path>
|
||||
<!-- Error Prone Support's additional bug checkers. -->
|
||||
<path>
|
||||
<groupId>tech.picnic.error-prone-support</groupId>
|
||||
<artifactId>error-prone-contrib</artifactId>
|
||||
<version>${error-prone-support.version}</version>
|
||||
</path>
|
||||
<!-- Error Prone Support's Refaster rules. -->
|
||||
<path>
|
||||
<groupId>tech.picnic.error-prone-support</groupId>
|
||||
<artifactId>refaster-runner</artifactId>
|
||||
<version>${error-prone-support.version}</version>
|
||||
</path>
|
||||
</annotationProcessorPaths>
|
||||
<compilerArgs>
|
||||
<arg>
|
||||
-Xplugin:ErrorProne
|
||||
<!-- Add other Error Prone flags here. See
|
||||
https://errorprone.info/docs/flags. -->
|
||||
</arg>
|
||||
<arg>-XDcompilePolicy=simple</arg>
|
||||
</compilerArgs>
|
||||
<!-- Some checks raise warnings rather than errors. -->
|
||||
<showWarnings>true</showWarnings>
|
||||
<!-- Enable this if you'd like to fail your build upon warnings. -->
|
||||
<!-- <failOnWarning>true</failOnWarning> -->
|
||||
</configuration>
|
||||
</plugin>
|
||||
</plugins>
|
||||
</pluginManagement>
|
||||
</build>
|
||||
```
|
||||
|
||||
<!-- XXX: Reference `oss-parent`'s `pom.xml` once that project also uses Error
|
||||
Prone Support. Alternatively reference this project's `self-check` profile
|
||||
definition. -->
|
||||
|
||||
### Seeing it in action
|
||||
|
||||
Consider the following example code:
|
||||
|
||||
```java
|
||||
import com.google.common.collect.ImmutableSet;
|
||||
import java.math.BigDecimal;
|
||||
|
||||
public class Example {
|
||||
static BigDecimal getNumber() {
|
||||
return BigDecimal.valueOf(0);
|
||||
}
|
||||
|
||||
public ImmutableSet<Integer> getSet() {
|
||||
ImmutableSet<Integer> set = ImmutableSet.of(1);
|
||||
return ImmutableSet.copyOf(set);
|
||||
}
|
||||
}
|
||||
```
|
||||
|
||||
If the [installation](#installation) was successful, then building the above
|
||||
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.refasterrules.BigDecimalRules.BigDecimalZero]
|
||||
Did you mean 'return BigDecimal.ZERO;'?
|
||||
[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] -------------------------------------------------------------
|
||||
...
|
||||
```
|
||||
|
||||
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] 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
|
||||
rules][refaster-rules].
|
||||
|
||||
## 👷 Developing Error Prone Support
|
||||
|
||||
This is a [Maven][maven] project, so running `mvn clean install` performs a
|
||||
full clean build and installs the library to your local Maven repository. Some
|
||||
relevant flags:
|
||||
|
||||
- `-Dverification.warn` makes the warnings and errors emitted by various
|
||||
plugins and the Java compiler non-fatal, where possible.
|
||||
- `-Dverification.skip` disables various non-essential plugins and compiles the
|
||||
code with minimal checks (i.e. without linting, Error Prone checks, etc.).
|
||||
- `-Dversion.error-prone=some-version` runs the build using the specified
|
||||
version of Error Prone. This is useful e.g. when testing a locally built
|
||||
Error Prone SNAPSHOT.
|
||||
- `-Perror-prone-fork` runs the build using Picnic's [Error Prone
|
||||
fork][error-prone-fork-repo], hosted on [Jitpack][error-prone-fork-jitpack].
|
||||
This fork generally contains a few changes on top of the latest Error Prone
|
||||
release.
|
||||
- `-Pself-check` runs the checks defined by this project against itself.
|
||||
Pending a release of [google/error-prone#3301][error-prone-pull-3301], this
|
||||
flag must currently be used in combination with `-Perror-prone-fork`.
|
||||
|
||||
Some other commands one may find relevant:
|
||||
|
||||
- `mvn fmt:format` formats the code using
|
||||
[`google-java-format`][google-java-format].
|
||||
- `./run-mutation-tests.sh` runs mutation tests using [PIT][pitest]. The
|
||||
results can be reviewed by opening the respective
|
||||
`target/pit-reports/index.html` files. For more information check the [PIT
|
||||
Maven plugin][pitest-maven].
|
||||
- `./apply-error-prone-suggestions.sh` applies Error Prone and Error Prone
|
||||
Support code suggestions to this project. Before running this command, make
|
||||
sure to have installed the project (`mvn clean install`) and make sure that
|
||||
the current working directory does not contain unstaged or uncommited
|
||||
changes.
|
||||
|
||||
When running the project's tests in IntelliJ IDEA, you might see the following
|
||||
error:
|
||||
|
||||
```
|
||||
java: exporting a package from system module jdk.compiler is not allowed with --release
|
||||
```
|
||||
|
||||
If this happens, go to _Settings -> Build, Execution, Deployment -> Compiler ->
|
||||
Java Compiler_ and deselect the option _Use '--release' option for
|
||||
cross-compilation (Java 9 and later)_. See [IDEA-288052][idea-288052] for
|
||||
details.
|
||||
|
||||
## 💡 How it works
|
||||
|
||||
This project provides additional [`BugChecker`][error-prone-bugchecker]
|
||||
implementations.
|
||||
|
||||
<!-- XXX: Extend this section. -->
|
||||
|
||||
## ✍️ Contributing
|
||||
|
||||
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]: 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
|
||||
[error-prone-installation-guide]: https://errorprone.info/docs/installation#maven
|
||||
[error-prone-orig-repo]: https://github.com/google/error-prone
|
||||
[error-prone-pull-3301]: https://github.com/google/error-prone/pull/3301
|
||||
[github-actions-build-badge]: https://github.com/PicnicSupermarket/error-prone-support/actions/workflows/build.yaml/badge.svg
|
||||
[github-actions-build-master]: https://github.com/PicnicSupermarket/error-prone-support/actions/workflows/build.yaml?query=branch%3Amaster
|
||||
[google-java-format]: https://github.com/google/google-java-format
|
||||
[idea-288052]: https://youtrack.jetbrains.com/issue/IDEA-288052
|
||||
[license-badge]: https://img.shields.io/github/license/PicnicSupermarket/error-prone-support
|
||||
[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-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/
|
||||
53
docgen/pom.xml
Normal file
53
docgen/pom.xml
Normal file
@@ -0,0 +1,53 @@
|
||||
<?xml version="1.0" encoding="UTF-8"?>
|
||||
<project xmlns="http://maven.apache.org/POM/4.0.0" xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" xsi:schemaLocation="http://maven.apache.org/POM/4.0.0 https://maven.apache.org/xsd/maven-4.0.0.xsd">
|
||||
<modelVersion>4.0.0</modelVersion>
|
||||
|
||||
<parent>
|
||||
<groupId>tech.picnic.error-prone-support</groupId>
|
||||
<artifactId>error-prone-support</artifactId>
|
||||
<version>0.3.1-SNAPSHOT</version>
|
||||
</parent>
|
||||
|
||||
<artifactId>docgen</artifactId>
|
||||
|
||||
<name>Picnic :: Error Prone Support :: Docgen</name>
|
||||
<description>Docgen.</description>
|
||||
|
||||
<dependencies>
|
||||
<dependency>
|
||||
<groupId>${groupId.error-prone}</groupId>
|
||||
<artifactId>error_prone_annotations</artifactId>
|
||||
<scope>provided</scope>
|
||||
</dependency>
|
||||
<dependency>
|
||||
<groupId>${groupId.error-prone}</groupId>
|
||||
<artifactId>error_prone_check_api</artifactId>
|
||||
</dependency>
|
||||
<dependency>
|
||||
<groupId>${groupId.error-prone}</groupId>
|
||||
<artifactId>error_prone_core</artifactId>
|
||||
</dependency>
|
||||
<dependency>
|
||||
<groupId>com.fasterxml.jackson.core</groupId>
|
||||
<artifactId>jackson-databind</artifactId>
|
||||
</dependency>
|
||||
<dependency>
|
||||
<groupId>com.google.auto.service</groupId>
|
||||
<artifactId>auto-service-annotations</artifactId>
|
||||
<scope>provided</scope>
|
||||
</dependency>
|
||||
<dependency>
|
||||
<groupId>com.google.auto.value</groupId>
|
||||
<artifactId>auto-value-annotations</artifactId>
|
||||
</dependency>
|
||||
<dependency>
|
||||
<groupId>com.google.code.findbugs</groupId>
|
||||
<artifactId>jsr305</artifactId>
|
||||
<scope>provided</scope>
|
||||
</dependency>
|
||||
<dependency>
|
||||
<groupId>com.google.guava</groupId>
|
||||
<artifactId>guava</artifactId>
|
||||
</dependency>
|
||||
</dependencies>
|
||||
</project>
|
||||
@@ -0,0 +1,26 @@
|
||||
package tech.picnic.errorprone.plugin;
|
||||
|
||||
import com.google.common.collect.ImmutableList;
|
||||
import com.google.errorprone.BugPattern;
|
||||
import com.google.errorprone.VisitorState;
|
||||
import com.sun.source.tree.ClassTree;
|
||||
import com.sun.source.util.TaskEvent;
|
||||
import tech.picnic.errorprone.plugin.models.BugPatternData;
|
||||
|
||||
public final class BugPatternExtractor implements DocExtractor<BugPatternData> {
|
||||
@Override
|
||||
public BugPatternData extractData(ClassTree tree, TaskEvent taskEvent, VisitorState state) {
|
||||
BugPattern annotation = taskEvent.getTypeElement().getAnnotation(BugPattern.class);
|
||||
return BugPatternData.create(
|
||||
taskEvent.getTypeElement().getQualifiedName().toString(),
|
||||
taskEvent.getTypeElement().getSimpleName().toString(),
|
||||
ImmutableList.copyOf(annotation.altNames()),
|
||||
annotation.linkType(),
|
||||
annotation.link(),
|
||||
ImmutableList.copyOf(annotation.tags()),
|
||||
annotation.summary(),
|
||||
annotation.explanation(),
|
||||
annotation.severity(),
|
||||
annotation.disableable());
|
||||
}
|
||||
}
|
||||
@@ -0,0 +1,104 @@
|
||||
package tech.picnic.errorprone.plugin;
|
||||
|
||||
import static com.google.errorprone.matchers.Matchers.allOf;
|
||||
import static com.google.errorprone.matchers.Matchers.hasAnnotation;
|
||||
import static com.google.errorprone.matchers.Matchers.instanceMethod;
|
||||
|
||||
import com.google.errorprone.VisitorState;
|
||||
import com.google.errorprone.annotations.Var;
|
||||
import com.google.errorprone.matchers.Matcher;
|
||||
import com.google.errorprone.util.ASTHelpers;
|
||||
import com.sun.source.tree.ClassTree;
|
||||
import com.sun.source.tree.ExpressionTree;
|
||||
import com.sun.source.tree.MethodInvocationTree;
|
||||
import com.sun.source.tree.MethodTree;
|
||||
import com.sun.source.util.TaskEvent;
|
||||
import com.sun.source.util.TreeScanner;
|
||||
import java.util.ArrayList;
|
||||
import java.util.List;
|
||||
import tech.picnic.errorprone.plugin.models.BugPatternReplacementTestData;
|
||||
import tech.picnic.errorprone.plugin.models.BugPatternTestData;
|
||||
|
||||
/** XXX: Write this. */
|
||||
// XXX: Take into account `expectUnchanged()`.
|
||||
public final class BugPatternTestsExtractor implements DocExtractor<BugPatternTestData> {
|
||||
private static final Matcher<MethodTree> JUNIT_TEST_METHOD =
|
||||
allOf(hasAnnotation("org.junit.jupiter.api.Test"));
|
||||
|
||||
private static final Matcher<ExpressionTree> IDENTIFICATION_SOURCE_LINES =
|
||||
instanceMethod()
|
||||
.onDescendantOf("com.google.errorprone.CompilationTestHelper")
|
||||
.named("addSourceLines");
|
||||
private static final Matcher<ExpressionTree> REPLACEMENT_INPUT =
|
||||
instanceMethod()
|
||||
.onDescendantOf("com.google.errorprone.BugCheckerRefactoringTestHelper")
|
||||
.named("addInputLines");
|
||||
private static final Matcher<ExpressionTree> REPLACEMENT_OUTPUT =
|
||||
instanceMethod()
|
||||
.onDescendantOf("com.google.errorprone.BugCheckerRefactoringTestHelper.ExpectOutput")
|
||||
.named("addOutputLines");
|
||||
|
||||
@Override
|
||||
public BugPatternTestData extractData(ClassTree tree, TaskEvent taskEvent, VisitorState state) {
|
||||
String name = tree.getSimpleName().toString().replace("Test", "");
|
||||
ScanBugCheckerTestData scanner = new ScanBugCheckerTestData(state);
|
||||
|
||||
tree.getMembers().stream()
|
||||
.filter(MethodTree.class::isInstance)
|
||||
.map(MethodTree.class::cast)
|
||||
.filter(m -> JUNIT_TEST_METHOD.matches(m, state))
|
||||
.forEach(m -> scanner.scan(m, null));
|
||||
|
||||
return BugPatternTestData.create(
|
||||
name, scanner.getIdentificationTests(), scanner.getReplacementTests());
|
||||
}
|
||||
|
||||
private static final class ScanBugCheckerTestData extends TreeScanner<Void, Void> {
|
||||
private final VisitorState state;
|
||||
private final List<String> identificationTests = new ArrayList<>();
|
||||
private final List<BugPatternReplacementTestData> replacementTests = new ArrayList<>();
|
||||
|
||||
// XXX: Using this output field is a bit hacky. Come up with a better solution.
|
||||
@Var private String output;
|
||||
|
||||
ScanBugCheckerTestData(VisitorState state) {
|
||||
this.state = state;
|
||||
}
|
||||
|
||||
public List<String> getIdentificationTests() {
|
||||
return identificationTests;
|
||||
}
|
||||
|
||||
public List<BugPatternReplacementTestData> getReplacementTests() {
|
||||
return replacementTests;
|
||||
}
|
||||
|
||||
@Override
|
||||
public Void visitMethodInvocation(MethodInvocationTree node, Void unused) {
|
||||
if (IDENTIFICATION_SOURCE_LINES.matches(node, state)) {
|
||||
identificationTests.add(getSourceLines(node));
|
||||
} else if (REPLACEMENT_INPUT.matches(node, state)) {
|
||||
replacementTests.add(BugPatternReplacementTestData.create(getSourceLines(node), output));
|
||||
} else if (REPLACEMENT_OUTPUT.matches(node, state)) {
|
||||
output = getSourceLines(node);
|
||||
}
|
||||
return super.visitMethodInvocation(node, unused);
|
||||
}
|
||||
|
||||
private static String getSourceLines(MethodInvocationTree tree) {
|
||||
List<? extends ExpressionTree> sourceLines =
|
||||
tree.getArguments().subList(1, tree.getArguments().size());
|
||||
StringBuilder source = new StringBuilder();
|
||||
|
||||
for (ExpressionTree sourceLine : sourceLines) {
|
||||
Object value = ASTHelpers.constValue(sourceLine);
|
||||
if (value == null) {
|
||||
return "";
|
||||
}
|
||||
source.append(value).append('\n');
|
||||
}
|
||||
|
||||
return source.toString();
|
||||
}
|
||||
}
|
||||
}
|
||||
@@ -0,0 +1,9 @@
|
||||
package tech.picnic.errorprone.plugin;
|
||||
|
||||
import com.google.errorprone.VisitorState;
|
||||
import com.sun.source.tree.ClassTree;
|
||||
import com.sun.source.util.TaskEvent;
|
||||
|
||||
public interface DocExtractor<T> {
|
||||
T extractData(ClassTree tree, TaskEvent taskEvent, VisitorState state);
|
||||
}
|
||||
@@ -0,0 +1,25 @@
|
||||
package tech.picnic.errorprone.plugin;
|
||||
|
||||
public enum DocType {
|
||||
BUG_PATTERN("bug-pattern", new BugPatternExtractor()),
|
||||
BUG_PATTERN_TEST("bug-pattern-test", new BugPatternTestsExtractor()),
|
||||
// REFASTER("refaster", new RefasterExtractor()),
|
||||
REFASTER_TEMPLATE_TEST_INPUT("refaster-test-input", new RefasterTestExtractor()),
|
||||
REFASTER_TEMPLATE_TEST_OUTPUT("refaster-test-output", new RefasterTestExtractor());
|
||||
|
||||
private final String outputFileNamePrefix;
|
||||
private final DocExtractor<?> docExtractor;
|
||||
|
||||
DocType(String outputFileNamePrefix, DocExtractor<?> docExtractor) {
|
||||
this.outputFileNamePrefix = outputFileNamePrefix;
|
||||
this.docExtractor = docExtractor;
|
||||
}
|
||||
|
||||
public String getOutputFileNamePrefix() {
|
||||
return outputFileNamePrefix;
|
||||
}
|
||||
|
||||
public DocExtractor<?> getDocExtractor() {
|
||||
return docExtractor;
|
||||
}
|
||||
}
|
||||
@@ -0,0 +1,21 @@
|
||||
package tech.picnic.errorprone.plugin;
|
||||
|
||||
import com.google.auto.service.AutoService;
|
||||
import com.sun.source.util.JavacTask;
|
||||
import com.sun.source.util.Plugin;
|
||||
import com.sun.tools.javac.api.BasicJavacTask;
|
||||
|
||||
/** XXX: Write. */
|
||||
@AutoService(Plugin.class)
|
||||
public final class Docgen implements Plugin {
|
||||
@Override
|
||||
public String getName() {
|
||||
return getClass().getSimpleName();
|
||||
}
|
||||
|
||||
@Override
|
||||
public void init(JavacTask javacTask, String... args) {
|
||||
javacTask.addTaskListener(
|
||||
new DocgenTaskListener(((BasicJavacTask) javacTask).getContext(), args[0]));
|
||||
}
|
||||
}
|
||||
@@ -0,0 +1,123 @@
|
||||
package tech.picnic.errorprone.plugin;
|
||||
|
||||
import static tech.picnic.errorprone.plugin.DocType.BUG_PATTERN;
|
||||
import static tech.picnic.errorprone.plugin.DocType.BUG_PATTERN_TEST;
|
||||
import static tech.picnic.errorprone.plugin.DocType.REFASTER_TEMPLATE_TEST_INPUT;
|
||||
import static tech.picnic.errorprone.plugin.DocType.REFASTER_TEMPLATE_TEST_OUTPUT;
|
||||
|
||||
import com.fasterxml.jackson.annotation.JsonAutoDetect.Visibility;
|
||||
import com.fasterxml.jackson.annotation.PropertyAccessor;
|
||||
import com.fasterxml.jackson.core.JsonGenerator;
|
||||
import com.fasterxml.jackson.databind.ObjectMapper;
|
||||
import com.fasterxml.jackson.databind.SerializationFeature;
|
||||
import com.google.errorprone.BugPattern;
|
||||
import com.google.errorprone.VisitorState;
|
||||
import com.google.errorprone.util.ASTHelpers;
|
||||
import com.sun.source.tree.ClassTree;
|
||||
import com.sun.source.tree.VariableTree;
|
||||
import com.sun.source.util.TaskEvent;
|
||||
import com.sun.source.util.TaskListener;
|
||||
import com.sun.tools.javac.api.JavacTrees;
|
||||
import com.sun.tools.javac.util.Context;
|
||||
import java.io.File;
|
||||
import java.io.FileWriter;
|
||||
import java.io.IOException;
|
||||
import java.nio.file.Files;
|
||||
import java.nio.file.Paths;
|
||||
import java.util.Optional;
|
||||
import javax.tools.JavaFileObject;
|
||||
|
||||
/** XXX: Write this. */
|
||||
final class DocgenTaskListener implements TaskListener {
|
||||
private final Context context;
|
||||
|
||||
private final String basePath;
|
||||
|
||||
private final VisitorState state;
|
||||
|
||||
private final ObjectMapper mapper =
|
||||
new ObjectMapper()
|
||||
.setVisibility(PropertyAccessor.FIELD, Visibility.ANY)
|
||||
.configure(SerializationFeature.FAIL_ON_EMPTY_BEANS, false)
|
||||
.configure(JsonGenerator.Feature.AUTO_CLOSE_TARGET, false);
|
||||
|
||||
DocgenTaskListener(Context context, String path) {
|
||||
this.context = context;
|
||||
this.basePath = path.substring(path.indexOf('=') + 1) + "/docs";
|
||||
this.state = VisitorState.createForUtilityPurposes(context);
|
||||
|
||||
// XXX: Move this somewhere else?
|
||||
try {
|
||||
Files.createDirectories(Paths.get(basePath));
|
||||
} catch (IOException e) {
|
||||
throw new RuntimeException(e);
|
||||
}
|
||||
}
|
||||
|
||||
@Override
|
||||
@SuppressWarnings("SystemOut")
|
||||
public void finished(TaskEvent taskEvent) {
|
||||
ClassTree tree = JavacTrees.instance(context).getTree(taskEvent.getTypeElement());
|
||||
JavaFileObject sourceFile = taskEvent.getSourceFile();
|
||||
if (tree == null || sourceFile == null || taskEvent.getKind() != TaskEvent.Kind.ANALYZE) {
|
||||
return;
|
||||
}
|
||||
|
||||
getDocType(tree, sourceFile)
|
||||
.ifPresent(
|
||||
docType ->
|
||||
writeToFile(
|
||||
docType.getDocExtractor().extractData(tree, taskEvent, state),
|
||||
docType.getOutputFileNamePrefix(),
|
||||
getSimpleClassName(sourceFile.getName())));
|
||||
}
|
||||
|
||||
private static Optional<DocType> getDocType(ClassTree tree, JavaFileObject sourceFile) {
|
||||
if (isBugPattern(tree)) {
|
||||
return Optional.of(BUG_PATTERN);
|
||||
} else if (isBugPatternTest(tree)) {
|
||||
return Optional.of(BUG_PATTERN_TEST);
|
||||
} else if (sourceFile.getName().contains("TestInput")) {
|
||||
return Optional.of(REFASTER_TEMPLATE_TEST_INPUT);
|
||||
} else if (sourceFile.getName().contains("TestOutput")) {
|
||||
return Optional.of(REFASTER_TEMPLATE_TEST_OUTPUT);
|
||||
}
|
||||
return Optional.empty();
|
||||
}
|
||||
|
||||
private <T> void writeToFile(T data, String fileName, String name) {
|
||||
File file = new File(basePath + "/" + fileName + "-" + name + ".json");
|
||||
// XXX: Use Path instead of File.
|
||||
|
||||
try (FileWriter fileWriter = new FileWriter(file, true)) {
|
||||
mapper.writeValue(fileWriter, data);
|
||||
fileWriter.write("\n");
|
||||
} catch (IOException e) {
|
||||
e.printStackTrace();
|
||||
}
|
||||
}
|
||||
|
||||
private static boolean isBugPattern(ClassTree tree) {
|
||||
return ASTHelpers.hasDirectAnnotationWithSimpleName(tree, BugPattern.class.getSimpleName());
|
||||
}
|
||||
|
||||
private static boolean isBugPatternTest(ClassTree tree) {
|
||||
return tree.getSimpleName().toString().endsWith("Test")
|
||||
// XXX: Instead, omit files from the util directory
|
||||
&& !tree.getSimpleName().toString().equals("MethodMatcherFactoryTest")
|
||||
&& tree.getMembers().stream()
|
||||
.filter(VariableTree.class::isInstance)
|
||||
.map(VariableTree.class::cast)
|
||||
.map(vt -> vt.getType().toString())
|
||||
.anyMatch(
|
||||
vt ->
|
||||
vt.equals("BugCheckerRefactoringTestHelper")
|
||||
|| vt.equals("CompilationTestHelper"));
|
||||
}
|
||||
|
||||
private static String getSimpleClassName(String path) {
|
||||
int index = path.lastIndexOf('/');
|
||||
String fileName = path.substring(index + 1);
|
||||
return fileName.replace(".java", "");
|
||||
}
|
||||
}
|
||||
@@ -0,0 +1,35 @@
|
||||
package tech.picnic.errorprone.plugin;
|
||||
|
||||
import static com.google.common.collect.ImmutableList.toImmutableList;
|
||||
|
||||
import com.google.common.collect.ImmutableList;
|
||||
import com.google.errorprone.VisitorState;
|
||||
import com.sun.source.tree.ClassTree;
|
||||
import com.sun.source.tree.MethodTree;
|
||||
import com.sun.source.util.TaskEvent;
|
||||
import tech.picnic.errorprone.plugin.models.RefasterTemplateCollectionTestData;
|
||||
import tech.picnic.errorprone.plugin.models.RefasterTemplateTestData;
|
||||
|
||||
public final class RefasterTestExtractor
|
||||
implements DocExtractor<RefasterTemplateCollectionTestData> {
|
||||
@Override
|
||||
public RefasterTemplateCollectionTestData extractData(
|
||||
ClassTree tree, TaskEvent taskEvent, VisitorState state) {
|
||||
String templateCollectionName = tree.getSimpleName().toString().replace("Test", "");
|
||||
boolean isInput = taskEvent.getSourceFile().getName().contains("Input");
|
||||
|
||||
ImmutableList<RefasterTemplateTestData> templateTests =
|
||||
tree.getMembers().stream()
|
||||
.filter(MethodTree.class::isInstance)
|
||||
.map(MethodTree.class::cast)
|
||||
.filter(m -> m.getName().toString().startsWith("test"))
|
||||
.map(
|
||||
m ->
|
||||
RefasterTemplateTestData.create(
|
||||
m.getName().toString().replace("test", ""), m.toString()))
|
||||
.collect(toImmutableList());
|
||||
|
||||
return RefasterTemplateCollectionTestData.create(
|
||||
templateCollectionName, isInput, templateTests);
|
||||
}
|
||||
}
|
||||
@@ -0,0 +1,55 @@
|
||||
package tech.picnic.errorprone.plugin.models;
|
||||
|
||||
import com.google.auto.value.AutoValue;
|
||||
import com.google.common.collect.ImmutableList;
|
||||
import com.google.errorprone.BugPattern.LinkType;
|
||||
import com.google.errorprone.BugPattern.SeverityLevel;
|
||||
|
||||
/** XXX: Write. */
|
||||
// XXX: What about `SuppressionAnnotations` and `DocumentSuppression`?
|
||||
@AutoValue
|
||||
public abstract class BugPatternData {
|
||||
public static BugPatternData create(
|
||||
String fullyQualifiedName,
|
||||
String name,
|
||||
ImmutableList<String> altNames,
|
||||
LinkType linkType,
|
||||
String link,
|
||||
ImmutableList<String> tags,
|
||||
String summary,
|
||||
String explanation,
|
||||
SeverityLevel severityLevel,
|
||||
boolean disableable) {
|
||||
return new AutoValue_BugPatternData(
|
||||
fullyQualifiedName,
|
||||
name,
|
||||
altNames,
|
||||
linkType,
|
||||
link,
|
||||
tags,
|
||||
summary,
|
||||
explanation,
|
||||
severityLevel,
|
||||
disableable);
|
||||
}
|
||||
|
||||
abstract String fullyQualifiedName();
|
||||
|
||||
abstract String name();
|
||||
|
||||
abstract ImmutableList<String> altNames();
|
||||
|
||||
abstract LinkType linkType();
|
||||
|
||||
abstract String link();
|
||||
|
||||
abstract ImmutableList<String> tags();
|
||||
|
||||
abstract String summary();
|
||||
|
||||
abstract String explanation();
|
||||
|
||||
abstract SeverityLevel severityLevel();
|
||||
|
||||
abstract boolean disableable();
|
||||
}
|
||||
@@ -0,0 +1,17 @@
|
||||
package tech.picnic.errorprone.plugin.models;
|
||||
|
||||
import com.google.auto.value.AutoValue;
|
||||
import javax.annotation.Nullable;
|
||||
|
||||
@AutoValue
|
||||
public abstract class BugPatternReplacementTestData {
|
||||
public static BugPatternReplacementTestData create(String inputLines, String outputLines) {
|
||||
return new AutoValue_BugPatternReplacementTestData(inputLines, outputLines);
|
||||
}
|
||||
|
||||
@Nullable
|
||||
abstract String inputLines();
|
||||
|
||||
@Nullable
|
||||
abstract String outputLines();
|
||||
}
|
||||
@@ -0,0 +1,23 @@
|
||||
package tech.picnic.errorprone.plugin.models;
|
||||
|
||||
import com.google.auto.value.AutoValue;
|
||||
import java.util.List;
|
||||
import javax.annotation.Nullable;
|
||||
|
||||
@AutoValue
|
||||
public abstract class BugPatternTestData {
|
||||
public static BugPatternTestData create(
|
||||
String name,
|
||||
List<String> identificationTests,
|
||||
List<BugPatternReplacementTestData> replacementTests) {
|
||||
return new AutoValue_BugPatternTestData(name, identificationTests, replacementTests);
|
||||
}
|
||||
|
||||
abstract String name();
|
||||
|
||||
@Nullable
|
||||
abstract List<String> identificationTests();
|
||||
|
||||
@Nullable
|
||||
abstract List<BugPatternReplacementTestData> replacementTests();
|
||||
}
|
||||
@@ -0,0 +1,24 @@
|
||||
package tech.picnic.errorprone.plugin.models;
|
||||
|
||||
import com.google.auto.value.AutoValue;
|
||||
import java.util.List;
|
||||
|
||||
/**
|
||||
* Object containing all data related to a Refaster template collection. This is solely used for
|
||||
* serialization.
|
||||
*/
|
||||
@AutoValue
|
||||
public abstract class RefasterTemplateCollectionData {
|
||||
public static RefasterTemplateCollectionData create(
|
||||
String name, String description, String link, List<RefasterTemplateData> templates) {
|
||||
return new AutoValue_RefasterTemplateCollectionData(name, description, link, templates);
|
||||
}
|
||||
|
||||
abstract String name();
|
||||
|
||||
abstract String description();
|
||||
|
||||
abstract String link();
|
||||
|
||||
abstract List<RefasterTemplateData> templates();
|
||||
}
|
||||
@@ -0,0 +1,19 @@
|
||||
package tech.picnic.errorprone.plugin.models;
|
||||
|
||||
import com.google.auto.value.AutoValue;
|
||||
import java.util.List;
|
||||
|
||||
@AutoValue
|
||||
public abstract class RefasterTemplateCollectionTestData {
|
||||
public static RefasterTemplateCollectionTestData create(
|
||||
String templateCollection, boolean isInput, List<RefasterTemplateTestData> templatesTests) {
|
||||
return new AutoValue_RefasterTemplateCollectionTestData(
|
||||
templateCollection, isInput, templatesTests);
|
||||
}
|
||||
|
||||
abstract String templateCollection();
|
||||
|
||||
abstract boolean isInput();
|
||||
|
||||
abstract List<RefasterTemplateTestData> templateTests();
|
||||
}
|
||||
@@ -0,0 +1,20 @@
|
||||
package tech.picnic.errorprone.plugin.models;
|
||||
|
||||
import com.google.auto.value.AutoValue;
|
||||
import com.google.errorprone.BugPattern.SeverityLevel;
|
||||
|
||||
@AutoValue
|
||||
public abstract class RefasterTemplateData {
|
||||
public static RefasterTemplateData create(
|
||||
String name, String description, String link, SeverityLevel severityLevel) {
|
||||
return new AutoValue_RefasterTemplateData(name, description, link, severityLevel);
|
||||
}
|
||||
|
||||
abstract String name();
|
||||
|
||||
abstract String description();
|
||||
|
||||
abstract String link();
|
||||
|
||||
abstract SeverityLevel severityLevel();
|
||||
}
|
||||
@@ -0,0 +1,14 @@
|
||||
package tech.picnic.errorprone.plugin.models;
|
||||
|
||||
import com.google.auto.value.AutoValue;
|
||||
|
||||
@AutoValue
|
||||
public abstract class RefasterTemplateTestData {
|
||||
public static RefasterTemplateTestData create(String templateName, String templateTestContent) {
|
||||
return new AutoValue_RefasterTemplateTestData(templateName, templateTestContent);
|
||||
}
|
||||
|
||||
abstract String templateName();
|
||||
|
||||
abstract String templateTestContent();
|
||||
}
|
||||
@@ -0,0 +1,4 @@
|
||||
/** A Java compiler plugin that XXX: fill in. */
|
||||
@com.google.errorprone.annotations.CheckReturnValue
|
||||
@javax.annotation.ParametersAreNonnullByDefault
|
||||
package tech.picnic.errorprone.plugin;
|
||||
@@ -11,71 +11,22 @@ request.
|
||||
|
||||
### Building
|
||||
|
||||
This is a [Maven][maven] project, so running `mvn clean install` performs a
|
||||
full clean build. Some relevant flags:
|
||||
- `-Dverification.warn` makes the warnings and errors emitted by various
|
||||
plugins and the Java compiler non-fatal, where possible.
|
||||
- `-Dverification.skip` disables various non-essential plugins and compiles the
|
||||
code with minimal checks (i.e. without linting, Error Prone checks, etc.)
|
||||
- `-Dversion.error-prone=some-version` runs the build using the specified
|
||||
version of Error Prone. This is useful e.g. when testing a locally built
|
||||
Error Prone SNAPSHOT.
|
||||
- `-Perror-prone-fork` run the build using Picnic's [Error Prone
|
||||
fork][error-prone-fork-repo], hosted on [Jitpack][error-prone-fork-jitpack].
|
||||
This fork generally contains a few changes on top of the latest Error Prone
|
||||
release.
|
||||
|
||||
Two other goals that one may find relevant:
|
||||
- `mvn fmt:format` formats the code using
|
||||
[`google-java-format`][google-java-format].
|
||||
- `mvn pitest:mutationCoverage` runs mutation tests using [PIT][pitest]. The
|
||||
results can be reviewed by opening the respective
|
||||
`target/pit-reports/index.html` files. For more information check the [PIT
|
||||
Maven plugin][pitest-maven].
|
||||
|
||||
When loading the project in IntelliJ IDEA (and perhaps other IDEs) errors about
|
||||
the inaccessibility of `com.sun.tools.javac.*` classes may be reported. If this
|
||||
happens, configure your IDE to enable the `add-exports` profile.
|
||||
See the main [readme][main-readme].
|
||||
|
||||
### Contribution guidelines
|
||||
|
||||
To the extend possible, the pull request process guards our coding guidelines.
|
||||
Some pointers:
|
||||
- Checks should we _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
|
||||
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
|
||||
checks][error-prone-criteria]. Most guidelines described there apply to this
|
||||
project as well, except that this project _does_ focus quite heavy on style
|
||||
enforcement. But that just makes the previous point doubly important.
|
||||
- Make sure that a check's (mutation) coverage is or remains about as high as
|
||||
it can be. Not only does this lead to better tests, it also points out
|
||||
opportunities to simplify the code.
|
||||
- Please restrict the scope of a pull request to a single feature or fix. Don't
|
||||
sneak in unrelated changes.
|
||||
- When in doubt about whether a pull request will be accepted, please first
|
||||
file an issue to discuss it.
|
||||
See our [contributing guidelines][main-contributing].
|
||||
|
||||
### Our wishlist
|
||||
|
||||
We expect the following tasks to help improve the quality of this open source
|
||||
project:
|
||||
|
||||
- Publish the artifact to Maven Central, then document the coordinates in this
|
||||
`README.md`.
|
||||
- Document how to enable the checks.
|
||||
- Document how to apply patches.
|
||||
- Document each of the checks.
|
||||
- Add Travis CI, [SonarQube][sonarcloud] and [Codecov][codecov]
|
||||
integrations.
|
||||
- Investigate whether it makes sense to include license headers in each file.
|
||||
If so, set that up and enforce it.
|
||||
- Add [SonarQube][sonarcloud] and [Codecov][codecov] integrations.
|
||||
- Add non-Java file formatting support, like we have internally at Picnic.
|
||||
(I.e., somehow open-source that stuff.)
|
||||
- Add relevant "badges" at the top of this `README.md`.
|
||||
- Auto-generate a website listing each of the checks, just like the Error Prone
|
||||
[bug patterns page][error-prone-bug-patterns]. The [Error Prone
|
||||
repository][error-prone-repo] contains code for this.
|
||||
@@ -93,7 +44,7 @@ project:
|
||||
- Improve an existing check (see `XXX`-marked comments in the code) or write a
|
||||
new one (see the list of suggestions below).
|
||||
|
||||
### Ideas for new checks
|
||||
### BugChecker extension ideas
|
||||
|
||||
The following is a list of checks we'd like to see implemented:
|
||||
|
||||
@@ -103,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.)
|
||||
@@ -113,128 +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
|
||||
- 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 which replaces single-character strings with `char`s where possible.
|
||||
- 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 `{Immutable,}{List,Set}Multimap` would be more appropriate.
|
||||
- A Guava-specific check which rewrites `if (conditional) { throw new
|
||||
- A Guava-specific check that flags `{Immutable,}Multimap` type usages where
|
||||
`{Immutable,}{List,Set}Multimap` would be more appropriate.
|
||||
- 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 which replaces simple anonymous `CacheLoader` subclass
|
||||
- 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.
|
||||
|
||||
@@ -247,6 +198,7 @@ but on the flip side Refaster is much less expressive. While this gap can never
|
||||
be fully closed, there are some ways in which Refaster's scope of utility could
|
||||
be extended. The following is a non-exhaustive list of ideas on how to extend
|
||||
Refaster's expressiveness:
|
||||
|
||||
- Allow more control over _which_ methods are statically imported by
|
||||
`@UseImportPolicy`. Sometimes the `@AfterTemplate` contains more than one
|
||||
static method invocation, and only a subset should be statically imported.
|
||||
@@ -261,9 +213,9 @@ Refaster's expressiveness:
|
||||
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.
|
||||
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
|
||||
@@ -284,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
|
||||
@@ -327,16 +275,12 @@ Refaster's expressiveness:
|
||||
[checkstyle-external-project-tests]: https://github.com/checkstyle/checkstyle/blob/master/wercker.yml
|
||||
[codecov]: https://codecov.io
|
||||
[error-prone-bug-patterns]: https://errorprone.info/bugpatterns
|
||||
[error-prone-criteria]: https://errorprone.info/docs/criteria
|
||||
[error-prone-fork-jitpack]: https://jitpack.io/#PicnicSupermarket/error-prone
|
||||
[error-prone-fork-repo]: https://github.com/PicnicSupermarket/error-prone
|
||||
[error-prone]: https://errorprone.info
|
||||
[error-prone-repo]: https://github.com/google/error-prone
|
||||
[forbidden-apis]: https://github.com/policeman-tools/forbidden-apis
|
||||
[fossa]: https://fossa.io
|
||||
[google-java-format]: https://github.com/google/google-java-format
|
||||
[maven]: https://maven.apache.org
|
||||
[main-contributing]: ../CONTRIBUTING.md
|
||||
[main-readme]: ../README.md
|
||||
[modernizer-maven-plugin]: https://github.com/gaul/modernizer-maven-plugin
|
||||
[sonarcloud]: https://sonarcloud.io
|
||||
[pitest]: https://pitest.org
|
||||
[pitest-maven]: https://pitest.org/quickstart/maven
|
||||
|
||||
@@ -5,7 +5,7 @@
|
||||
<parent>
|
||||
<groupId>tech.picnic.error-prone-support</groupId>
|
||||
<artifactId>error-prone-support</artifactId>
|
||||
<version>0.2.0</version>
|
||||
<version>0.3.1-SNAPSHOT</version>
|
||||
</parent>
|
||||
|
||||
<artifactId>error-prone-contrib</artifactId>
|
||||
@@ -69,11 +69,6 @@
|
||||
<artifactId>jsr305</artifactId>
|
||||
<scope>provided</scope>
|
||||
</dependency>
|
||||
<dependency>
|
||||
<groupId>com.google.errorprone</groupId>
|
||||
<artifactId>javac</artifactId>
|
||||
<scope>provided</scope>
|
||||
</dependency>
|
||||
<dependency>
|
||||
<groupId>com.google.googlejavaformat</groupId>
|
||||
<artifactId>google-java-format</artifactId>
|
||||
@@ -143,6 +138,11 @@
|
||||
<artifactId>value-annotations</artifactId>
|
||||
<scope>test</scope>
|
||||
</dependency>
|
||||
<dependency>
|
||||
<groupId>org.jspecify</groupId>
|
||||
<artifactId>jspecify</artifactId>
|
||||
<scope>provided</scope>
|
||||
</dependency>
|
||||
<dependency>
|
||||
<groupId>org.junit.jupiter</groupId>
|
||||
<artifactId>junit-jupiter-api</artifactId>
|
||||
@@ -238,4 +238,33 @@
|
||||
</plugins>
|
||||
</pluginManagement>
|
||||
</build>
|
||||
|
||||
<profiles>
|
||||
<profile>
|
||||
<id>docgen</id>
|
||||
<build>
|
||||
<plugins>
|
||||
<plugin>
|
||||
<groupId>org.codehaus.mojo</groupId>
|
||||
<artifactId>build-helper-maven-plugin</artifactId>
|
||||
<version>3.3.0</version>
|
||||
<executions>
|
||||
<execution>
|
||||
<id>add-test-source</id>
|
||||
<goals>
|
||||
<goal>add-test-source</goal>
|
||||
</goals>
|
||||
<phase>generate-test-sources</phase>
|
||||
<configuration>
|
||||
<sources>
|
||||
<source>src/test/resources</source>
|
||||
</sources>
|
||||
</configuration>
|
||||
</execution>
|
||||
</executions>
|
||||
</plugin>
|
||||
</plugins>
|
||||
</build>
|
||||
</profile>
|
||||
</profiles>
|
||||
</project>
|
||||
|
||||
@@ -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 {
|
||||
|
||||
@@ -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 {
|
||||
|
||||
@@ -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 {
|
||||
|
||||
@@ -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 {
|
||||
|
||||
@@ -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 {
|
||||
|
||||
@@ -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 {
|
||||
|
||||
@@ -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
|
||||
|
||||
@@ -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 {
|
||||
@@ -84,6 +86,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));
|
||||
}
|
||||
}
|
||||
|
||||
@@ -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
|
||||
|
||||
@@ -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;
|
||||
@@ -35,8 +36,8 @@ import javax.annotation.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
|
||||
|
||||
@@ -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 {
|
||||
|
||||
@@ -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 {
|
||||
|
||||
@@ -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 {
|
||||
|
||||
@@ -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;
|
||||
@@ -42,7 +45,7 @@ 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,6 +73,12 @@ 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;
|
||||
|
||||
@@ -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,34 @@ 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
|
||||
@Override
|
||||
public Void visitIdentifier(IdentifierTree node, @Nullable Void ctx) {
|
||||
nodes.add(tokenize(node));
|
||||
return super.visitIdentifier(node, ctx);
|
||||
public 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 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 Void visitPrimitiveType(PrimitiveTypeTree node, @Nullable Void unused) {
|
||||
nodes.add(ImmutableList.of(node.getPrimitiveTypeKind().toString()));
|
||||
return super.visitPrimitiveType(node, unused);
|
||||
}
|
||||
}.scan(array, null);
|
||||
|
||||
|
||||
@@ -1,10 +1,11 @@
|
||||
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 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.collect.ImmutableList;
|
||||
@@ -31,7 +32,8 @@ 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
|
||||
|
||||
@@ -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,7 +53,8 @@ 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 {
|
||||
|
||||
@@ -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 {
|
||||
|
||||
@@ -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 {
|
||||
|
||||
@@ -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,12 +21,13 @@ 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 {
|
||||
|
||||
@@ -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()}`,
|
||||
|
||||
@@ -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 {
|
||||
|
||||
@@ -1,19 +1,25 @@
|
||||
package tech.picnic.errorprone.bugpatterns;
|
||||
|
||||
import static com.google.errorprone.BugPattern.LinkType.NONE;
|
||||
import static com.google.common.collect.ImmutableSet.toImmutableSet;
|
||||
import static com.google.errorprone.BugPattern.LinkType.CUSTOM;
|
||||
import static com.google.errorprone.BugPattern.SeverityLevel.SUGGESTION;
|
||||
import static com.google.errorprone.BugPattern.StandardTags.SIMPLIFICATION;
|
||||
import static com.google.errorprone.matchers.Matchers.allOf;
|
||||
import static com.google.errorprone.matchers.Matchers.anyMethod;
|
||||
import static com.google.errorprone.matchers.Matchers.anyOf;
|
||||
import static com.google.errorprone.matchers.Matchers.argumentCount;
|
||||
import static com.google.errorprone.matchers.Matchers.isNonNullUsingDataflow;
|
||||
import static com.google.errorprone.matchers.Matchers.isSameType;
|
||||
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;
|
||||
import com.google.common.collect.Iterables;
|
||||
import com.google.common.primitives.Primitives;
|
||||
import com.google.errorprone.BugPattern;
|
||||
import com.google.errorprone.ErrorProneFlags;
|
||||
import com.google.errorprone.VisitorState;
|
||||
@@ -24,6 +30,7 @@ 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.Suppliers;
|
||||
import com.sun.source.tree.BinaryTree;
|
||||
import com.sun.source.tree.CompoundAssignmentTree;
|
||||
import com.sun.source.tree.ExpressionTree;
|
||||
@@ -41,14 +48,16 @@ import java.util.List;
|
||||
import java.util.Locale;
|
||||
import java.util.Objects;
|
||||
import java.util.Optional;
|
||||
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
|
||||
@@ -69,49 +78,30 @@ public final class RedundantStringConversion extends BugChecker
|
||||
allOf(STRING, isNonNullUsingDataflow());
|
||||
private static final Matcher<ExpressionTree> NOT_FORMATTABLE =
|
||||
not(isSubtypeOf(Formattable.class));
|
||||
private static final Matcher<ExpressionTree> WELL_KNOWN_STRING_CONVERSION_METHODS =
|
||||
private static final Matcher<MethodInvocationTree> WELL_KNOWN_STRING_CONVERSION_METHODS =
|
||||
anyOf(
|
||||
instanceMethod().onDescendantOfAny(Object.class.getName()).named("toString"),
|
||||
staticMethod()
|
||||
.onClass(Objects.class.getName())
|
||||
instanceMethod()
|
||||
.onDescendantOfAny(Object.class.getName())
|
||||
.named("toString")
|
||||
.withParameters(Object.class.getName()),
|
||||
staticMethod()
|
||||
.onClass(String.class.getName())
|
||||
.named("valueOf")
|
||||
.withParameters(Object.class.getName()),
|
||||
staticMethod()
|
||||
.onClass(String.class.getName())
|
||||
.named("valueOf")
|
||||
.withParameters(String.class.getName()),
|
||||
staticMethod()
|
||||
.onClass(Byte.class.getName())
|
||||
.named("toString")
|
||||
.withParameters(byte.class.getName()),
|
||||
staticMethod()
|
||||
.onClass(Character.class.getName())
|
||||
.named("toString")
|
||||
.withParameters(char.class.getName()),
|
||||
staticMethod()
|
||||
.onClass(Short.class.getName())
|
||||
.named("toString")
|
||||
.withParameters(short.class.getName()),
|
||||
staticMethod()
|
||||
.onClass(Integer.class.getName())
|
||||
.named("toString")
|
||||
.withParameters(int.class.getName()),
|
||||
staticMethod()
|
||||
.onClass(Long.class.getName())
|
||||
.named("toString")
|
||||
.withParameters(long.class.getName()),
|
||||
staticMethod()
|
||||
.onClass(Float.class.getName())
|
||||
.named("toString")
|
||||
.withParameters(float.class.getName()),
|
||||
staticMethod()
|
||||
.onClass(Double.class.getName())
|
||||
.named("toString")
|
||||
.withParameters(double.class.getName()));
|
||||
.withNoParameters(),
|
||||
allOf(
|
||||
argumentCount(1),
|
||||
anyOf(
|
||||
staticMethod()
|
||||
.onClassAny(
|
||||
Stream.concat(
|
||||
Primitives.allWrapperTypes().stream(), Stream.of(Objects.class))
|
||||
.map(Class::getName)
|
||||
.collect(toImmutableSet()))
|
||||
.named("toString"),
|
||||
allOf(
|
||||
staticMethod().onClass(String.class.getName()).named("valueOf"),
|
||||
not(
|
||||
anyMethod()
|
||||
.anyClass()
|
||||
.withAnyName()
|
||||
.withParametersOfType(
|
||||
ImmutableList.of(Suppliers.arrayOf(Suppliers.CHAR_TYPE))))))));
|
||||
private static final Matcher<ExpressionTree> STRINGBUILDER_APPEND_INVOCATION =
|
||||
instanceMethod()
|
||||
.onDescendantOf(StringBuilder.class.getName())
|
||||
@@ -127,17 +117,10 @@ public final class RedundantStringConversion extends BugChecker
|
||||
staticMethod().onClass(String.class.getName()).named("format"),
|
||||
instanceMethod().onDescendantOf(Formatter.class.getName()).named("format"),
|
||||
instanceMethod()
|
||||
.onDescendantOf(PrintStream.class.getName())
|
||||
.onDescendantOfAny(PrintStream.class.getName(), PrintWriter.class.getName())
|
||||
.namedAnyOf("format", "printf"),
|
||||
instanceMethod()
|
||||
.onDescendantOf(PrintStream.class.getName())
|
||||
.namedAnyOf("print", "println")
|
||||
.withParameters(Object.class.getName()),
|
||||
instanceMethod()
|
||||
.onDescendantOf(PrintWriter.class.getName())
|
||||
.namedAnyOf("format", "printf"),
|
||||
instanceMethod()
|
||||
.onDescendantOf(PrintWriter.class.getName())
|
||||
.onDescendantOfAny(PrintStream.class.getName(), PrintWriter.class.getName())
|
||||
.namedAnyOf("print", "println")
|
||||
.withParameters(Object.class.getName()),
|
||||
staticMethod()
|
||||
@@ -156,7 +139,7 @@ public final class RedundantStringConversion extends BugChecker
|
||||
.onDescendantOf("org.slf4j.Logger")
|
||||
.namedAnyOf("trace", "debug", "info", "warn", "error");
|
||||
|
||||
private final Matcher<ExpressionTree> conversionMethodMatcher;
|
||||
private final Matcher<MethodInvocationTree> conversionMethodMatcher;
|
||||
|
||||
/** Instantiates the default {@link RedundantStringConversion}. */
|
||||
public RedundantStringConversion() {
|
||||
@@ -186,7 +169,7 @@ public final class RedundantStringConversion extends BugChecker
|
||||
|
||||
List<SuggestedFix.Builder> fixes = new ArrayList<>();
|
||||
|
||||
// XXX: Not so nice: we try to simplify the RHS twice.
|
||||
// XXX: Avoid trying to simplify the RHS twice.
|
||||
ExpressionTree preferredRhs = trySimplify(rhs, state).orElse(rhs);
|
||||
if (STRING.matches(preferredRhs, state)) {
|
||||
tryFix(lhs, state, ANY_EXPR).ifPresent(fixes::add);
|
||||
@@ -243,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(
|
||||
@@ -274,17 +257,17 @@ 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 converison or going with
|
||||
// string as the last logger argument. Suggests either dropping the conversion or going with
|
||||
// `Throwable#getMessage()` instead.
|
||||
private Optional<SuggestedFix.Builder> tryFixSlf4jLogger(
|
||||
List<? extends ExpressionTree> arguments, VisitorState state) {
|
||||
/*
|
||||
* SLF4J treats the final argument to a log statement specially if it is a `Throwabe`: it
|
||||
* SLF4J treats the final argument to a log statement specially if it is a `Throwable`: it
|
||||
* will always choose to render the associated stacktrace, even if the argument has a
|
||||
* matching `{}` placeholder. (In this case the `{}` will simply be logged verbatim.) So if
|
||||
* a log statement's final argument is the string representation of a `Throwble`, then we
|
||||
* a log statement's final argument is the string representation of a `Throwable`, then we
|
||||
* must not strip this explicit string conversion, as that would change the statement's
|
||||
* semantics.
|
||||
*/
|
||||
@@ -338,11 +321,15 @@ public final class RedundantStringConversion extends BugChecker
|
||||
}
|
||||
|
||||
private Optional<ExpressionTree> trySimplify(ExpressionTree tree, VisitorState state) {
|
||||
if (tree.getKind() != Kind.METHOD_INVOCATION || !conversionMethodMatcher.matches(tree, state)) {
|
||||
if (tree.getKind() != Kind.METHOD_INVOCATION) {
|
||||
return Optional.empty();
|
||||
}
|
||||
|
||||
MethodInvocationTree methodInvocation = (MethodInvocationTree) tree;
|
||||
if (!conversionMethodMatcher.matches(methodInvocation, state)) {
|
||||
return Optional.empty();
|
||||
}
|
||||
|
||||
switch (methodInvocation.getArguments().size()) {
|
||||
case 0:
|
||||
return trySimplifyNullaryMethod(methodInvocation, state);
|
||||
@@ -383,7 +370,8 @@ public final class RedundantStringConversion extends BugChecker
|
||||
.orElse(Description.NO_MATCH);
|
||||
}
|
||||
|
||||
private static Matcher<ExpressionTree> createConversionMethodMatcher(ErrorProneFlags flags) {
|
||||
private static Matcher<MethodInvocationTree> createConversionMethodMatcher(
|
||||
ErrorProneFlags flags) {
|
||||
// XXX: ErrorProneFlags#getList splits by comma, but method signatures may also contain commas.
|
||||
// For this class methods accepting more than one argument are not valid, but still: not nice.
|
||||
return flags
|
||||
|
||||
@@ -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 {
|
||||
|
||||
@@ -0,0 +1,113 @@
|
||||
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);
|
||||
|
||||
@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));
|
||||
}
|
||||
}
|
||||
@@ -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 {
|
||||
@@ -66,7 +68,8 @@ public final class RequestMappingAnnotation extends BugChecker implements Method
|
||||
isType(ANN_PACKAGE_PREFIX + "RequestAttribute"),
|
||||
isType(ANN_PACKAGE_PREFIX + "RequestBody"),
|
||||
isType(ANN_PACKAGE_PREFIX + "RequestHeader"),
|
||||
isType(ANN_PACKAGE_PREFIX + "RequestParam"))),
|
||||
isType(ANN_PACKAGE_PREFIX + "RequestParam"),
|
||||
isType(ANN_PACKAGE_PREFIX + "RequestPart"))),
|
||||
isSameType("java.io.InputStream"),
|
||||
isSameType("java.time.ZoneId"),
|
||||
isSameType("java.util.Locale"),
|
||||
|
||||
@@ -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 {
|
||||
|
||||
@@ -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 {
|
||||
|
||||
@@ -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 {
|
||||
|
||||
@@ -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 {
|
||||
|
||||
@@ -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 {
|
||||
|
||||
@@ -0,0 +1,182 @@
|
||||
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 javax.annotation.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);
|
||||
|
||||
@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));
|
||||
}
|
||||
}
|
||||
@@ -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 {
|
||||
|
||||
@@ -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() {}
|
||||
}
|
||||
@@ -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
|
||||
@@ -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
|
||||
@@ -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
|
||||
@@ -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
|
||||
@@ -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
|
||||
@@ -0,0 +1,94 @@
|
||||
package tech.picnic.errorprone.refasterrules;
|
||||
|
||||
import static com.google.errorprone.refaster.ImportPolicy.STATIC_IMPORT_ALWAYS;
|
||||
import static org.assertj.core.api.Assertions.assertThat;
|
||||
|
||||
import com.google.errorprone.refaster.annotation.AfterTemplate;
|
||||
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;
|
||||
|
||||
@OnlineDocumentation
|
||||
final class AssertJComparableRules {
|
||||
private AssertJComparableRules() {}
|
||||
|
||||
static final class AssertThatIsEqualByComparingTo<T extends Comparable<? super T>> {
|
||||
@BeforeTemplate
|
||||
AbstractIntegerAssert<?> before(T actual, T expected) {
|
||||
return assertThat(actual.compareTo(expected)).isEqualTo(0);
|
||||
}
|
||||
|
||||
@AfterTemplate
|
||||
@UseImportPolicy(STATIC_IMPORT_ALWAYS)
|
||||
AbstractComparableAssert<?, ?> after(T actual, T expected) {
|
||||
return assertThat(actual).isEqualByComparingTo(expected);
|
||||
}
|
||||
}
|
||||
|
||||
static final class AssertThatIsNotEqualByComparingTo<T extends Comparable<? super T>> {
|
||||
@BeforeTemplate
|
||||
AbstractIntegerAssert<?> before(T actual, T expected) {
|
||||
return assertThat(actual.compareTo(expected)).isNotEqualTo(0);
|
||||
}
|
||||
|
||||
@AfterTemplate
|
||||
@UseImportPolicy(STATIC_IMPORT_ALWAYS)
|
||||
AbstractComparableAssert<?, ?> after(T actual, T expected) {
|
||||
return assertThat(actual).isNotEqualByComparingTo(expected);
|
||||
}
|
||||
}
|
||||
|
||||
static final class AssertThatIsLessThan<T extends Comparable<? super T>> {
|
||||
@BeforeTemplate
|
||||
AbstractIntegerAssert<?> before(T actual, T expected) {
|
||||
return assertThat(actual.compareTo(expected)).isNegative();
|
||||
}
|
||||
|
||||
@AfterTemplate
|
||||
@UseImportPolicy(STATIC_IMPORT_ALWAYS)
|
||||
AbstractComparableAssert<?, ?> after(T actual, T expected) {
|
||||
return assertThat(actual).isLessThan(expected);
|
||||
}
|
||||
}
|
||||
|
||||
static final class AssertThatIsLessThanOrEqualTo<T extends Comparable<? super T>> {
|
||||
@BeforeTemplate
|
||||
AbstractIntegerAssert<?> before(T actual, T expected) {
|
||||
return assertThat(actual.compareTo(expected)).isNotPositive();
|
||||
}
|
||||
|
||||
@AfterTemplate
|
||||
@UseImportPolicy(STATIC_IMPORT_ALWAYS)
|
||||
AbstractComparableAssert<?, ?> after(T actual, T expected) {
|
||||
return assertThat(actual).isLessThanOrEqualTo(expected);
|
||||
}
|
||||
}
|
||||
|
||||
static final class AssertThatIsGreaterThan<T extends Comparable<? super T>> {
|
||||
@BeforeTemplate
|
||||
AbstractIntegerAssert<?> before(T actual, T expected) {
|
||||
return assertThat(actual.compareTo(expected)).isPositive();
|
||||
}
|
||||
|
||||
@AfterTemplate
|
||||
@UseImportPolicy(STATIC_IMPORT_ALWAYS)
|
||||
AbstractComparableAssert<?, ?> after(T actual, T expected) {
|
||||
return assertThat(actual).isGreaterThan(expected);
|
||||
}
|
||||
}
|
||||
|
||||
static final class AssertThatIsGreaterThanOrEqualTo<T extends Comparable<? super T>> {
|
||||
@BeforeTemplate
|
||||
AbstractIntegerAssert<?> before(T actual, T expected) {
|
||||
return assertThat(actual.compareTo(expected)).isNotNegative();
|
||||
}
|
||||
|
||||
@AfterTemplate
|
||||
@UseImportPolicy(STATIC_IMPORT_ALWAYS)
|
||||
AbstractComparableAssert<?, ?> after(T actual, T expected) {
|
||||
return assertThat(actual).isGreaterThanOrEqualTo(expected);
|
||||
}
|
||||
}
|
||||
}
|
||||
@@ -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
|
||||
@@ -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
|
||||
@@ -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
|
||||
@@ -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
|
||||
@@ -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
|
||||
@@ -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
|
||||
@@ -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;
|
||||
@@ -6,6 +6,7 @@ import static org.assertj.core.api.Assertions.assertThat;
|
||||
import com.google.errorprone.refaster.Refaster;
|
||||
import com.google.errorprone.refaster.annotation.AfterTemplate;
|
||||
import com.google.errorprone.refaster.annotation.BeforeTemplate;
|
||||
import com.google.errorprone.refaster.annotation.NotMatches;
|
||||
import com.google.errorprone.refaster.annotation.UseImportPolicy;
|
||||
import java.math.BigDecimal;
|
||||
import java.math.BigInteger;
|
||||
@@ -18,9 +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.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
|
||||
@@ -226,9 +230,16 @@ final class AssertJNumberTemplates {
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* Prefer {@link AbstractLongAssert#isOdd()} (and similar methods for other {@link NumberAssert}
|
||||
* subtypes) over alternatives with less informative error messages.
|
||||
*
|
||||
* <p>Note that {@link org.assertj.core.api.AbstractCharacterAssert} does not implement {@link
|
||||
* NumberAssert} and does not provide an {@code isOdd} test.
|
||||
*/
|
||||
static final class AssertThatIsOdd {
|
||||
@BeforeTemplate
|
||||
AbstractIntegerAssert<?> before(int number) {
|
||||
AbstractIntegerAssert<?> before(@NotMatches(IsCharacter.class) int number) {
|
||||
return assertThat(number % 2).isEqualTo(1);
|
||||
}
|
||||
|
||||
@@ -244,9 +255,16 @@ final class AssertJNumberTemplates {
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* Prefer {@link AbstractLongAssert#isEven()} (and similar methods for other {@link NumberAssert}
|
||||
* subtypes) over alternatives with less informative error messages.
|
||||
*
|
||||
* <p>Note that {@link org.assertj.core.api.AbstractCharacterAssert} does not implement {@link
|
||||
* NumberAssert} and does not provide an {@code isEven} test.
|
||||
*/
|
||||
static final class AssertThatIsEven {
|
||||
@BeforeTemplate
|
||||
AbstractIntegerAssert<?> before(int number) {
|
||||
AbstractIntegerAssert<?> before(@NotMatches(IsCharacter.class) int number) {
|
||||
return assertThat(number % 2).isEqualTo(0);
|
||||
}
|
||||
|
||||
@@ -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
|
||||
@@ -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
|
||||
@@ -0,0 +1,113 @@
|
||||
package tech.picnic.errorprone.refasterrules;
|
||||
|
||||
import static com.google.errorprone.refaster.ImportPolicy.STATIC_IMPORT_ALWAYS;
|
||||
import static org.assertj.core.api.Assertions.assertThat;
|
||||
|
||||
import com.google.errorprone.refaster.Refaster;
|
||||
import com.google.errorprone.refaster.annotation.AfterTemplate;
|
||||
import com.google.errorprone.refaster.annotation.BeforeTemplate;
|
||||
import com.google.errorprone.refaster.annotation.UseImportPolicy;
|
||||
import org.assertj.core.api.AbstractBooleanAssert;
|
||||
import org.assertj.core.api.AbstractDoubleAssert;
|
||||
import tech.picnic.errorprone.refaster.annotation.OnlineDocumentation;
|
||||
|
||||
@OnlineDocumentation
|
||||
final class AssertJPrimitiveRules {
|
||||
private AssertJPrimitiveRules() {}
|
||||
|
||||
static final class AssertThatIsEqualTo {
|
||||
@BeforeTemplate
|
||||
AbstractBooleanAssert<?> before(boolean actual, boolean expected) {
|
||||
return Refaster.anyOf(
|
||||
assertThat(actual == expected).isTrue(), assertThat(actual != expected).isFalse());
|
||||
}
|
||||
|
||||
@BeforeTemplate
|
||||
AbstractBooleanAssert<?> before(double actual, double expected) {
|
||||
return Refaster.anyOf(
|
||||
assertThat(actual == expected).isTrue(), assertThat(actual != expected).isFalse());
|
||||
}
|
||||
|
||||
@AfterTemplate
|
||||
@UseImportPolicy(STATIC_IMPORT_ALWAYS)
|
||||
AbstractBooleanAssert<?> after(boolean actual, boolean expected) {
|
||||
return assertThat(actual).isEqualTo(expected);
|
||||
}
|
||||
}
|
||||
|
||||
static final class AssertThatIsNotEqualTo {
|
||||
@BeforeTemplate
|
||||
AbstractBooleanAssert<?> before(boolean actual, boolean expected) {
|
||||
return Refaster.anyOf(
|
||||
assertThat(actual != expected).isTrue(), assertThat(actual == expected).isFalse());
|
||||
}
|
||||
|
||||
@BeforeTemplate
|
||||
AbstractBooleanAssert<?> before(double actual, double expected) {
|
||||
return Refaster.anyOf(
|
||||
assertThat(actual != expected).isTrue(), assertThat(actual == expected).isFalse());
|
||||
}
|
||||
|
||||
@AfterTemplate
|
||||
@UseImportPolicy(STATIC_IMPORT_ALWAYS)
|
||||
AbstractBooleanAssert<?> after(boolean actual, boolean expected) {
|
||||
return assertThat(actual).isNotEqualTo(expected);
|
||||
}
|
||||
}
|
||||
|
||||
static final class AssertThatIsLessThan {
|
||||
@BeforeTemplate
|
||||
AbstractBooleanAssert<?> before(double actual, double expected) {
|
||||
return Refaster.anyOf(
|
||||
assertThat(actual < expected).isTrue(), assertThat(actual >= expected).isFalse());
|
||||
}
|
||||
|
||||
@AfterTemplate
|
||||
@UseImportPolicy(STATIC_IMPORT_ALWAYS)
|
||||
AbstractDoubleAssert<?> after(double actual, double expected) {
|
||||
return assertThat(actual).isLessThan(expected);
|
||||
}
|
||||
}
|
||||
|
||||
static final class AssertThatIsLessThanOrEqualTo {
|
||||
@BeforeTemplate
|
||||
AbstractBooleanAssert<?> before(double actual, double expected) {
|
||||
return Refaster.anyOf(
|
||||
assertThat(actual <= expected).isTrue(), assertThat(actual > expected).isFalse());
|
||||
}
|
||||
|
||||
@AfterTemplate
|
||||
@UseImportPolicy(STATIC_IMPORT_ALWAYS)
|
||||
AbstractDoubleAssert<?> after(double actual, double expected) {
|
||||
return assertThat(actual).isLessThanOrEqualTo(expected);
|
||||
}
|
||||
}
|
||||
|
||||
static final class AssertThatIsGreaterThan {
|
||||
@BeforeTemplate
|
||||
AbstractBooleanAssert<?> before(double actual, double expected) {
|
||||
return Refaster.anyOf(
|
||||
assertThat(actual > expected).isTrue(), assertThat(actual <= expected).isFalse());
|
||||
}
|
||||
|
||||
@AfterTemplate
|
||||
@UseImportPolicy(STATIC_IMPORT_ALWAYS)
|
||||
AbstractDoubleAssert<?> after(double actual, double expected) {
|
||||
return assertThat(actual).isGreaterThan(expected);
|
||||
}
|
||||
}
|
||||
|
||||
static final class AssertThatIsGreaterThanOrEqualTo {
|
||||
@BeforeTemplate
|
||||
AbstractBooleanAssert<?> before(double actual, double expected) {
|
||||
return Refaster.anyOf(
|
||||
assertThat(actual >= expected).isTrue(), assertThat(actual < expected).isFalse());
|
||||
}
|
||||
|
||||
@AfterTemplate
|
||||
@UseImportPolicy(STATIC_IMPORT_ALWAYS)
|
||||
AbstractDoubleAssert<?> after(double actual, double expected) {
|
||||
return assertThat(actual).isGreaterThanOrEqualTo(expected);
|
||||
}
|
||||
}
|
||||
}
|
||||
@@ -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> {
|
||||
@@ -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
|
||||
@@ -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
|
||||
@@ -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(
|
||||
@@ -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;
|
||||
@@ -28,13 +28,15 @@ import java.util.Objects;
|
||||
import java.util.Set;
|
||||
import java.util.stream.Stream;
|
||||
import javax.annotation.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 {
|
||||
@@ -82,7 +84,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?
|
||||
@@ -117,8 +119,8 @@ final class AssortedTemplates {
|
||||
}
|
||||
|
||||
/** 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 +174,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) {
|
||||
@@ -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 {
|
||||
@@ -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
|
||||
@@ -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;
|
||||
}
|
||||
}
|
||||
}
|
||||
@@ -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 {
|
||||
@@ -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> {
|
||||
@@ -68,17 +70,14 @@ final class EqualityTemplates {
|
||||
* Don't negate an equality test or use the ternary operator to compare two booleans; directly
|
||||
* test for inequality instead.
|
||||
*/
|
||||
// XXX: Replacing `a ? !b : b` with `a != b` changes semantics if both `a` and `b` are boxed
|
||||
// booleans.
|
||||
static final class Negation {
|
||||
@BeforeTemplate
|
||||
boolean before(boolean a, boolean b) {
|
||||
return Refaster.anyOf(!(a == b), a ? !b : b);
|
||||
}
|
||||
|
||||
@BeforeTemplate
|
||||
boolean before(long a, long b) {
|
||||
return !(a == b);
|
||||
}
|
||||
|
||||
@BeforeTemplate
|
||||
boolean before(double a, double b) {
|
||||
return !(a == b);
|
||||
@@ -99,17 +98,14 @@ final class EqualityTemplates {
|
||||
* Don't negate an inequality test or use the ternary operator to compare two booleans; directly
|
||||
* test for equality instead.
|
||||
*/
|
||||
// XXX: Replacing `a ? b : !b` with `a == b` changes semantics if both `a` and `b` are boxed
|
||||
// booleans.
|
||||
static final class IndirectDoubleNegation {
|
||||
@BeforeTemplate
|
||||
boolean before(boolean a, boolean b) {
|
||||
return Refaster.anyOf(!(a != b), a ? b : !b);
|
||||
}
|
||||
|
||||
@BeforeTemplate
|
||||
boolean before(long a, long b) {
|
||||
return !(a != b);
|
||||
}
|
||||
|
||||
@BeforeTemplate
|
||||
boolean before(double a, double b) {
|
||||
return !(a != b);
|
||||
@@ -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)
|
||||
@@ -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
|
||||
@@ -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)))
|
||||
@@ -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() {
|
||||
@@ -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
|
||||
@@ -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
|
||||
@@ -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()`.
|
||||
@@ -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.
|
||||
@@ -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> {
|
||||
@@ -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 {
|
||||
@@ -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> {
|
||||
@@ -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 {
|
||||
@@ -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
|
||||
@@ -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
|
||||
Some files were not shown because too many files have changed in this diff Show More
Reference in New Issue
Block a user