Introduce {CONTRIBUTING,LICENSE,README}.md and Error Prone Support's logo (#212)

This commit is contained in:
Jelmer Borst
2022-09-19 08:50:08 +02:00
committed by GitHub
parent b675ff680f
commit 84ba3946d9
6 changed files with 359 additions and 81 deletions

71
CONTRIBUTING.md Normal file
View 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 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) 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
View 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.

224
README.md Normal file
View File

@@ -0,0 +1,224 @@
<div align="center">
<picture>
<source media="(prefers-color-scheme: dark)" srcset="logo-dark.svg">
<source media="(prefers-color-scheme: light)" srcset="logo.svg">
<img alt="Error Prone Support logo" src="logo.svg" width="50%">
</picture>
# Error Prone Support
Error Prone Support is a Picnic-opinionated extension of Google's [Error
Prone][error-prone-orig-repo]. It aims to improve code quality, focussing on
maintainability, consistency and avoidance of common gotchas.
> Error Prone is a static analysis tool for Java that catches common
> programming mistakes at compile-time.
[![Maven Central][maven-central-badge]][maven-central-search]
[![GitHub Actions][github-actions-build-badge]][github-actions-build-master]
[![License][license-badge]][license]
[![PRs Welcome][pr-badge]][contributing]
[Getting started](#-getting-started) • [Building](#-building) •
[How it works](#-how-it-works) • [Contributing](#%EF%B8%8F-contributing)
</div>
---
## ⚡ 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 templates. -->
<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.refastertemplates.BigDecimalTemplates.BigDecimalZero]
Did you mean 'return BigDecimal.ZERO;'?
[WARNING] Example.java:[14,35] [IdentityConversion] This method invocation appears redundant; remove it or suppress this warning and add a comment explaining its purpose
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] template capable of
[rewriting][refaster-templates-bigdecimal] expressions of the form
`BigDecimal.valueOf(0)` and `new BigDecimal(0)` to `BigDecimal.ZERO`.
Be sure to check out all [bug checks][bug-checks] and [refaster
templates][refaster-templates].
## 👷 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` 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]: error-prone-contrib/src/main/java/tech/picnic/errorprone/bugpatterns/
[bug-checks-identity-conversion]: error-prone-contrib/src/main/java/tech/picnic/errorprone/bugpatterns/IdentityConversion.java
[contributing]: CONTRIBUTING.md
[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]: 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
[pitest]: https://pitest.org
[pitest-maven]: https://pitest.org/quickstart/maven
[pr-badge]: https://img.shields.io/badge/PRs-welcome-brightgreen.svg
[refaster]: https://errorprone.info/docs/refaster
[refaster-templates-bigdecimal]: error-prone-contrib/src/main/java/tech/picnic/errorprone/refastertemplates/BigDecimalTemplates.java
[refaster-templates]: error-prone-contrib/src/main/java/tech/picnic/errorprone/refastertemplates/

View File

@@ -11,77 +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 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.
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 [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 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.
@@ -99,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:
@@ -124,12 +69,13 @@ The following is a list of checks we'd like to see implemented:
code and Javadoc `@link` references.
- A check which simplifies array expressions. It would replace empty array
expressions of the form `new int[] {}` with `new int[0]`. Statements of the
form `byte[] arr = new byte[] {'c'};` would be shortened to `byte[] arr =
{'c'};`.
- A check which replaces expressions of the form `String.format("some prefix
%s", arg)` with `"some prefix " + arg`, and similar for simple suffixes. Can
perhaps be generalized further, though it's unclear how far. (Well, a
`String.format` call without arguments can certainly be simplified, too.)
form `byte[] arr = new byte[] {'c'};` would be shortened to
`byte[] arr = {'c'};`.
- A check which replaces expressions of the form
`String.format("some prefix %s", arg)` with `"some prefix " + arg`, and
similar for simple suffixes. Can perhaps be generalized further, though it's
unclear how far. (Well, a `String.format` call without arguments can
certainly be simplified, too.)
- A check which replaces single-character strings with `char`s where possible.
For example as argument to `StringBuilder.append` and in string
concatenations.
@@ -174,11 +120,11 @@ The following is a list of checks we'd like to see implemented:
- A check which flags imports from other test classes.
- A Guava-specific check which 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
IllegalArgumentException(); }` and variants to an equivalent `checkArgument`
statement. Idem for other exception types.
- 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 IllegalArgumentException(); }` and variants to
an equivalent `checkArgument` statement. Idem for other exception types.
- A Guava-specific check which replaces simple anonymous `CacheLoader` subclass
declarations with `CacheLoader.from(someLambda)`.
- A Spring-specific check which enforces that methods with the `@Scheduled`
@@ -253,6 +199,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.
@@ -265,16 +212,16 @@ Refaster's expressiveness:
- Some Refaster refactorings (e.g. when dealing with lazy evaluation) are valid
only when some free parameter is a constant, variable reference or some other
pure expression. Introduce a way to express such a constraint. For example,
rewriting `optional1.map(Optional::of).orElse(optional2)` to `optional1.or(()
-> optional2)` is not behavior preserving if evaluation of `optional2` has
side-effects.
rewriting `optional1.map(Optional::of).orElse(optional2)` to
`optional1.or(() -> optional2)` is not behavior preserving if evaluation of
`optional2` has side-effects.
- Similarly, certain refactoring operations are only valid if one of the
matches expressions is not `@Nullable`. It'd be nice to be able to express
this.
- Generalize `@Placeholder` support such that rules can reference e.g. "any
concrete unary method". This would allow refactorings such as
`Mono.just(constant).flatmap(this::someFun)` -> `Mono.defer(() ->
someFun(constant))`.
`Mono.just(constant).flatmap(this::someFun)` ->
`Mono.defer(() -> someFun(constant))`.
- Sometimes a Refaster refactoring can cause the resulting code not to compile
due to a lack of generic type information. Identify and resolve such
occurrences. For example, an `@AfterTemplate` may require the insertion of a
@@ -333,17 +280,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
[idea-288052]: https://youtrack.jetbrains.com/issue/IDEA-288052
[maven]: https://maven.apache.org
[main-contributing]: ../CONTRIBUTING.md
[main-readme]: ../README.md
[modernizer-maven-plugin]: https://github.com/gaul/modernizer-maven-plugin
[pitest]: https://pitest.org
[pitest-maven]: https://pitest.org/quickstart/maven
[sonarcloud]: https://sonarcloud.io

10
logo-dark.svg Normal file

File diff suppressed because one or more lines are too long

After

Width:  |  Height:  |  Size: 63 KiB

10
logo.svg Normal file

File diff suppressed because one or more lines are too long

After

Width:  |  Height:  |  Size: 63 KiB