From 84ba3946d97470b011b533744754dffcafc9a496 Mon Sep 17 00:00:00 2001 From: Jelmer Borst Date: Mon, 19 Sep 2022 08:50:08 +0200 Subject: [PATCH] Introduce `{CONTRIBUTING,LICENSE,README}.md` and Error Prone Support's logo (#212) --- CONTRIBUTING.md | 71 +++++++++++ LICENSE.md | 21 ++++ README.md | 224 ++++++++++++++++++++++++++++++++++ error-prone-contrib/README.md | 104 ++++------------ logo-dark.svg | 10 ++ logo.svg | 10 ++ 6 files changed, 359 insertions(+), 81 deletions(-) create mode 100644 CONTRIBUTING.md create mode 100644 LICENSE.md create mode 100644 README.md create mode 100644 logo-dark.svg create mode 100644 logo.svg diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md new file mode 100644 index 00000000..caae1842 --- /dev/null +++ b/CONTRIBUTING.md @@ -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 diff --git a/LICENSE.md b/LICENSE.md new file mode 100644 index 00000000..4b2daf19 --- /dev/null +++ b/LICENSE.md @@ -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. diff --git a/README.md b/README.md new file mode 100644 index 00000000..291f421a --- /dev/null +++ b/README.md @@ -0,0 +1,224 @@ +
+ + + + + Error Prone Support logo + + +# 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) + +
+ +--- + +## ⚡ 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 + + + + + org.apache.maven.plugins + maven-compiler-plugin + + + + + com.google.errorprone + error_prone_core + ${error-prone.version} + + + + tech.picnic.error-prone-support + error-prone-contrib + ${error-prone-support.version} + + + + tech.picnic.error-prone-support + refaster-runner + ${error-prone-support.version} + + + + + -Xplugin:ErrorProne + + + -XDcompilePolicy=simple + + + true + + + + + + + + ``` + + + +### 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 getSet() { + ImmutableSet 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 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. + + + +## ✍️ 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/ diff --git a/error-prone-contrib/README.md b/error-prone-contrib/README.md index 03a15715..a373124d 100644 --- a/error-prone-contrib/README.md +++ b/error-prone-contrib/README.md @@ -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 diff --git a/logo-dark.svg b/logo-dark.svg new file mode 100644 index 00000000..835d5aa8 --- /dev/null +++ b/logo-dark.svg @@ -0,0 +1,10 @@ + + 1 0 + + 1 1 0 + + 0 0 1 + + 1 0 0 1 + + \ No newline at end of file diff --git a/logo.svg b/logo.svg new file mode 100644 index 00000000..02ae35df --- /dev/null +++ b/logo.svg @@ -0,0 +1,10 @@ + + 1 0 + + 1 1 0 + + 0 0 1 + + 1 0 0 1 + + \ No newline at end of file