mirror of
https://github.com/jlengrand/error-prone-support.git
synced 2026-03-10 08:11:25 +00:00
Compare commits
55 Commits
gdejong/do
...
v0.5.0
| Author | SHA1 | Date | |
|---|---|---|---|
|
|
068c03708b | ||
|
|
9c330981ea | ||
|
|
b780c05dc0 | ||
|
|
16955a9cfa | ||
|
|
8fa3ff3702 | ||
|
|
022a3d293e | ||
|
|
81227cdd94 | ||
|
|
04d886c031 | ||
|
|
afb2a28dcf | ||
|
|
dc0f90e981 | ||
|
|
2196bbd8f9 | ||
|
|
f3b81304b9 | ||
|
|
b0d374040a | ||
|
|
45dfc53d40 | ||
|
|
6cb10ffe2f | ||
|
|
92f2b0ab0f | ||
|
|
21388273c5 | ||
|
|
b2e15607c1 | ||
|
|
50e730fb40 | ||
|
|
a844b9e532 | ||
|
|
671ee1eedb | ||
|
|
7fe61c226a | ||
|
|
8fcc91febf | ||
|
|
e00aba12c3 | ||
|
|
0118cc6c10 | ||
|
|
91e009cab0 | ||
|
|
68dca3204e | ||
|
|
e6ccb523fd | ||
|
|
d0533f7190 | ||
|
|
759e7ea2ff | ||
|
|
2578857a37 | ||
|
|
1d28512f09 | ||
|
|
4efd79bfa6 | ||
|
|
d6b28733f6 | ||
|
|
b9884fa6d6 | ||
|
|
4fd8f57430 | ||
|
|
a2559bddda | ||
|
|
7de9bede19 | ||
|
|
85cb997f1b | ||
|
|
d5a78186db | ||
|
|
128e178d37 | ||
|
|
7aef2cfe51 | ||
|
|
5cec0dd508 | ||
|
|
3f1399c139 | ||
|
|
757d5b1d70 | ||
|
|
45cac6105e | ||
|
|
8f8f57fc5b | ||
|
|
902d4f7736 | ||
|
|
4ec349582c | ||
|
|
57836103ea | ||
|
|
bd73243c34 | ||
|
|
9af50d7e0b | ||
|
|
f5ae47fbac | ||
|
|
9ad8c275d3 | ||
|
|
f4e191e33b |
2
.github/release.yml
vendored
2
.github/release.yml
vendored
@@ -3,7 +3,7 @@ changelog:
|
||||
labels:
|
||||
- "ignore-changelog"
|
||||
categories:
|
||||
- title: ":rocket: New Error Prone checks and Refaster templates"
|
||||
- title: ":rocket: New Error Prone checks and Refaster rules"
|
||||
labels:
|
||||
- "new feature"
|
||||
- title: ":sparkles: Improvements"
|
||||
|
||||
29
.github/workflows/build.yaml
vendored
29
.github/workflows/build.yaml
vendored
@@ -2,15 +2,32 @@ name: Build and verify
|
||||
on:
|
||||
pull_request:
|
||||
push:
|
||||
branches: [$default-branch]
|
||||
branches: [ master ]
|
||||
permissions:
|
||||
contents: read
|
||||
jobs:
|
||||
build:
|
||||
runs-on: ubuntu-22.04
|
||||
strategy:
|
||||
matrix:
|
||||
jdk: [ 11.0.16, 17.0.4 ]
|
||||
os: [ ubuntu-22.04 ]
|
||||
jdk: [ 11.0.16, 17.0.4, 19 ]
|
||||
distribution: [ temurin ]
|
||||
experimental: [ false ]
|
||||
include:
|
||||
- os: macos-12
|
||||
jdk: 17.0.4
|
||||
distribution: temurin
|
||||
experimental: false
|
||||
- os: windows-2022
|
||||
jdk: 17.0.4
|
||||
distribution: temurin
|
||||
experimental: false
|
||||
- os: ubuntu-22.04
|
||||
jdk: 20-ea
|
||||
distribution: zulu
|
||||
experimental: true
|
||||
runs-on: ${{ matrix.os }}
|
||||
continue-on-error: ${{ matrix.experimental }}
|
||||
steps:
|
||||
# We run the build twice for each supported JDK: once against the
|
||||
# original Error Prone release, using only Error Prone checks available
|
||||
@@ -18,12 +35,12 @@ 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.5.1
|
||||
uses: actions/setup-java@v3.6.0
|
||||
with:
|
||||
java-version: ${{ matrix.jdk }}
|
||||
distribution: temurin
|
||||
distribution: ${{ matrix.distribution }}
|
||||
cache: maven
|
||||
- name: Display build environment details
|
||||
run: mvn --version
|
||||
|
||||
42
.github/workflows/deploy-website.yaml
vendored
42
.github/workflows/deploy-website.yaml
vendored
@@ -2,39 +2,43 @@ name: Update `error-prone.picnic.tech` website content
|
||||
on:
|
||||
pull_request:
|
||||
push:
|
||||
branches: [$default-branch]
|
||||
permissions:
|
||||
contents: read
|
||||
id-token: write
|
||||
pages: write
|
||||
branches: [ master, website ]
|
||||
concurrency:
|
||||
group: pages
|
||||
cancel-in-progress: true
|
||||
group: ${{ github.workflow }}-${{ github.ref }}
|
||||
jobs:
|
||||
build:
|
||||
permissions:
|
||||
contents: read
|
||||
runs-on: ubuntu-22.04
|
||||
steps:
|
||||
- name: Check out code
|
||||
uses: actions/checkout@v3.0.2
|
||||
uses: actions/checkout@v3.1.0
|
||||
- uses: ruby/setup-ruby@v1.120.0
|
||||
with:
|
||||
working-directory: ./website
|
||||
bundler-cache: true
|
||||
- name: Configure Github Pages
|
||||
uses: actions/configure-pages@v2.1.1
|
||||
uses: actions/configure-pages@v2.1.2
|
||||
- name: Generate documentation
|
||||
run: ./generate-docs.sh
|
||||
- name: Build website with Jekyll
|
||||
uses: actions/jekyll-build-pages@v1.0.5
|
||||
with:
|
||||
source: website/
|
||||
destination: ./_site
|
||||
working-directory: ./website
|
||||
run: bundle exec jekyll build
|
||||
- name: Validate HTML output
|
||||
uses: anishathalye/proof-html@v1.4.1
|
||||
with:
|
||||
directory: ./_site
|
||||
check_external_hash: false
|
||||
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 == format('refs/heads/{0}', github.event.repository.default_branch)
|
||||
if: github.ref == 'refs/heads/website'
|
||||
needs: build
|
||||
permissions:
|
||||
id-token: write
|
||||
pages: write
|
||||
runs-on: ubuntu-22.04
|
||||
environment:
|
||||
name: github-pages
|
||||
@@ -42,4 +46,4 @@ jobs:
|
||||
steps:
|
||||
- name: Deploy to GitHub Pages
|
||||
id: deployment
|
||||
uses: actions/deploy-pages@v1.2.1
|
||||
uses: actions/deploy-pages@v1.2.2
|
||||
|
||||
26
README.md
26
README.md
@@ -15,6 +15,10 @@ 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]
|
||||
@@ -59,7 +63,7 @@ it:
|
||||
<artifactId>error-prone-contrib</artifactId>
|
||||
<version>${error-prone-support.version}</version>
|
||||
</path>
|
||||
<!-- Error Prone Support's Refaster templates. -->
|
||||
<!-- Error Prone Support's Refaster rules. -->
|
||||
<path>
|
||||
<groupId>tech.picnic.error-prone-support</groupId>
|
||||
<artifactId>refaster-runner</artifactId>
|
||||
@@ -115,16 +119,13 @@ code with Maven should yield two compiler warnings:
|
||||
```sh
|
||||
$ mvn clean install
|
||||
...
|
||||
[INFO] -------------------------------------------------------------
|
||||
[WARNING] COMPILATION WARNING :
|
||||
[INFO] -------------------------------------------------------------
|
||||
[WARNING] Example.java:[9,34] [tech.picnic.errorprone.refastertemplates.BigDecimalTemplates.BigDecimalZero]
|
||||
[INFO] Example.java:[9,34] [Refaster Rule] BigDecimalRules.BigDecimalZero: Refactoring opportunity
|
||||
(see https://error-prone.picnic.tech/refasterrules/BigDecimalRules#BigDecimalZero)
|
||||
Did you mean 'return BigDecimal.ZERO;'?
|
||||
...
|
||||
[WARNING] Example.java:[13,35] [IdentityConversion] This method invocation appears redundant; remove it or suppress this warning and add a comment explaining its purpose
|
||||
(see https://error-prone.picnic.tech/bugpatterns/IdentityConversion)
|
||||
Did you mean 'return set;' or '@SuppressWarnings("IdentityConversion") public ImmutableSet<Integer> getSet() {'?
|
||||
[INFO] 2 warnings
|
||||
[INFO] -------------------------------------------------------------
|
||||
...
|
||||
```
|
||||
|
||||
@@ -132,12 +133,12 @@ Two things are kicking in here:
|
||||
|
||||
1. An Error Prone [`BugChecker`][error-prone-bugchecker] that flags unnecessary
|
||||
[identity conversions][bug-checks-identity-conversion].
|
||||
2. A [Refaster][refaster] template capable of
|
||||
[rewriting][refaster-templates-bigdecimal] expressions of the form
|
||||
2. A [Refaster][refaster] rule capable of
|
||||
[rewriting][refaster-rules-bigdecimal] expressions of the form
|
||||
`BigDecimal.valueOf(0)` and `new BigDecimal(0)` to `BigDecimal.ZERO`.
|
||||
|
||||
Be sure to check out all [bug checks][bug-checks] and [refaster
|
||||
templates][refaster-templates].
|
||||
rules][refaster-rules].
|
||||
|
||||
## 👷 Developing Error Prone Support
|
||||
|
||||
@@ -218,9 +219,10 @@ guidelines][contributing].
|
||||
[maven-central-search]: https://search.maven.org/artifact/tech.picnic.error-prone-support/error-prone-support
|
||||
[maven]: https://maven.apache.org
|
||||
[picnic-blog]: https://blog.picnic.nl
|
||||
[picnic-blog-ep-post]: https://blog.picnic.nl/picnic-loves-error-prone-producing-high-quality-and-consistent-java-code-b8a566be6886
|
||||
[pitest]: https://pitest.org
|
||||
[pitest-maven]: https://pitest.org/quickstart/maven
|
||||
[pr-badge]: https://img.shields.io/badge/PRs-welcome-brightgreen.svg
|
||||
[refaster]: https://errorprone.info/docs/refaster
|
||||
[refaster-templates-bigdecimal]: https://github.com/PicnicSupermarket/error-prone-support/blob/master/error-prone-contrib/src/main/java/tech/picnic/errorprone/refastertemplates/BigDecimalTemplates.java
|
||||
[refaster-templates]: https://github.com/PicnicSupermarket/error-prone-support/blob/master/error-prone-contrib/src/main/java/tech/picnic/errorprone/refastertemplates/
|
||||
[refaster-rules-bigdecimal]: https://github.com/PicnicSupermarket/error-prone-support/blob/master/error-prone-contrib/src/main/java/tech/picnic/errorprone/refasterrules/BigDecimalRules.java
|
||||
[refaster-rules]: https://github.com/PicnicSupermarket/error-prone-support/blob/master/error-prone-contrib/src/main/java/tech/picnic/errorprone/refasterrules/
|
||||
|
||||
@@ -267,7 +267,7 @@ Refaster's expressiveness:
|
||||
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
|
||||
|
||||
@@ -5,7 +5,7 @@
|
||||
<parent>
|
||||
<groupId>tech.picnic.error-prone-support</groupId>
|
||||
<artifactId>error-prone-support</artifactId>
|
||||
<version>0.3.1-SNAPSHOT</version>
|
||||
<version>0.5.0</version>
|
||||
</parent>
|
||||
|
||||
<artifactId>error-prone-contrib</artifactId>
|
||||
@@ -64,11 +64,6 @@
|
||||
<artifactId>auto-service-annotations</artifactId>
|
||||
<scope>provided</scope>
|
||||
</dependency>
|
||||
<dependency>
|
||||
<groupId>com.google.code.findbugs</groupId>
|
||||
<artifactId>jsr305</artifactId>
|
||||
<scope>provided</scope>
|
||||
</dependency>
|
||||
<dependency>
|
||||
<groupId>com.google.googlejavaformat</groupId>
|
||||
<artifactId>google-java-format</artifactId>
|
||||
@@ -98,6 +93,11 @@
|
||||
<artifactId>reactor-adapter</artifactId>
|
||||
<scope>provided</scope>
|
||||
</dependency>
|
||||
<dependency>
|
||||
<groupId>io.projectreactor.addons</groupId>
|
||||
<artifactId>reactor-extra</artifactId>
|
||||
<scope>provided</scope>
|
||||
</dependency>
|
||||
<dependency>
|
||||
<groupId>io.reactivex.rxjava2</groupId>
|
||||
<artifactId>rxjava</artifactId>
|
||||
@@ -173,6 +173,11 @@
|
||||
<artifactId>slf4j-api</artifactId>
|
||||
<scope>test</scope>
|
||||
</dependency>
|
||||
<dependency>
|
||||
<groupId>org.springframework</groupId>
|
||||
<artifactId>spring-context</artifactId>
|
||||
<scope>test</scope>
|
||||
</dependency>
|
||||
<dependency>
|
||||
<groupId>org.springframework</groupId>
|
||||
<artifactId>spring-test</artifactId>
|
||||
|
||||
@@ -36,6 +36,9 @@ public final class AmbiguousJsonCreator extends BugChecker implements Annotation
|
||||
private static final Matcher<AnnotationTree> IS_JSON_CREATOR_ANNOTATION =
|
||||
isType("com.fasterxml.jackson.annotation.JsonCreator");
|
||||
|
||||
/** Instantiates a new {@link AmbiguousJsonCreator} instance. */
|
||||
public AmbiguousJsonCreator() {}
|
||||
|
||||
@Override
|
||||
public Description matchAnnotation(AnnotationTree tree, VisitorState state) {
|
||||
if (!IS_JSON_CREATOR_ANNOTATION.matches(tree, state)) {
|
||||
|
||||
@@ -24,7 +24,7 @@ import com.sun.source.tree.MethodInvocationTree;
|
||||
/**
|
||||
* 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.
|
||||
@@ -44,6 +44,9 @@ public final class AssertJIsNull extends BugChecker implements MethodInvocationT
|
||||
argumentCount(1),
|
||||
argument(0, nullLiteral()));
|
||||
|
||||
/** Instantiates a new {@link AssertJIsNull} instance. */
|
||||
public AssertJIsNull() {}
|
||||
|
||||
@Override
|
||||
public Description matchMethodInvocation(MethodInvocationTree tree, VisitorState state) {
|
||||
if (!ASSERT_IS_EQUAL_TO_NULL.matches(tree, state)) {
|
||||
|
||||
@@ -38,6 +38,9 @@ public final class AutowiredConstructor extends BugChecker implements ClassTreeM
|
||||
private static final MultiMatcher<Tree, AnnotationTree> AUTOWIRED_ANNOTATION =
|
||||
annotations(AT_LEAST_ONE, isType("org.springframework.beans.factory.annotation.Autowired"));
|
||||
|
||||
/** Instantiates a new {@link AutowiredConstructor} instance. */
|
||||
public AutowiredConstructor() {}
|
||||
|
||||
@Override
|
||||
public Description matchClass(ClassTree tree, VisitorState state) {
|
||||
List<MethodTree> constructors = ASTHelpers.getConstructors(tree);
|
||||
|
||||
@@ -46,6 +46,9 @@ public final class CanonicalAnnotationSyntax extends BugChecker implements Annot
|
||||
CanonicalAnnotationSyntax::dropRedundantValueAttribute,
|
||||
CanonicalAnnotationSyntax::dropRedundantCurlies);
|
||||
|
||||
/** Instantiates a new {@link CanonicalAnnotationSyntax} instance. */
|
||||
public CanonicalAnnotationSyntax() {}
|
||||
|
||||
@Override
|
||||
public Description matchAnnotation(AnnotationTree tree, VisitorState state) {
|
||||
return FIX_FACTORIES.stream()
|
||||
|
||||
@@ -18,6 +18,7 @@ import com.google.errorprone.matchers.Matcher;
|
||||
import com.sun.source.tree.ExpressionTree;
|
||||
import com.sun.source.tree.MethodInvocationTree;
|
||||
import java.util.stream.Collector;
|
||||
import tech.picnic.errorprone.bugpatterns.util.ThirdPartyLibrary;
|
||||
|
||||
/**
|
||||
* A {@link BugChecker} that flags {@link Collector Collectors} that don't clearly express
|
||||
@@ -45,9 +46,13 @@ public final class CollectorMutability extends BugChecker implements MethodInvoc
|
||||
private static final Matcher<ExpressionTree> SET_COLLECTOR =
|
||||
staticMethod().anyClass().named("toSet");
|
||||
|
||||
/** Instantiates a new {@link CollectorMutability} instance. */
|
||||
public CollectorMutability() {}
|
||||
|
||||
@Override
|
||||
public Description matchMethodInvocation(MethodInvocationTree tree, VisitorState state) {
|
||||
if (!COLLECTOR_METHOD.matches(tree, state)) {
|
||||
if (!ThirdPartyLibrary.GUAVA.isIntroductionAllowed(state)
|
||||
|| !COLLECTOR_METHOD.matches(tree, state)) {
|
||||
return Description.NO_MATCH;
|
||||
}
|
||||
|
||||
|
||||
@@ -38,6 +38,9 @@ public final class EmptyMethod extends BugChecker implements MethodTreeMatcher {
|
||||
AT_LEAST_ONE,
|
||||
anyOf(isType("java.lang.Override"), isType("org.aspectj.lang.annotation.Pointcut")));
|
||||
|
||||
/** Instantiates a new {@link EmptyMethod} instance. */
|
||||
public EmptyMethod() {}
|
||||
|
||||
@Override
|
||||
public Description matchMethod(MethodTree tree, VisitorState state) {
|
||||
if (tree.getBody() == null
|
||||
|
||||
@@ -72,6 +72,9 @@ public final class ErrorProneTestHelperSourceFormat extends BugChecker
|
||||
.onDescendantOf("com.google.errorprone.BugCheckerRefactoringTestHelper.ExpectOutput")
|
||||
.named("addOutputLines");
|
||||
|
||||
/** Instantiates a new {@link ErrorProneTestHelperSourceFormat} instance. */
|
||||
public ErrorProneTestHelperSourceFormat() {}
|
||||
|
||||
@Override
|
||||
public Description matchMethodInvocation(MethodInvocationTree tree, VisitorState state) {
|
||||
boolean isOutputSource = OUTPUT_SOURCE_ACCEPTING_METHOD.matches(tree, state);
|
||||
|
||||
@@ -46,6 +46,9 @@ public final class ExplicitEnumOrdering extends BugChecker implements MethodInvo
|
||||
private static final Matcher<ExpressionTree> EXPLICIT_ORDERING =
|
||||
staticMethod().onClass(Ordering.class.getName()).named("explicit");
|
||||
|
||||
/** Instantiates a new {@link ExplicitEnumOrdering} instance. */
|
||||
public ExplicitEnumOrdering() {}
|
||||
|
||||
@Override
|
||||
public Description matchMethodInvocation(MethodInvocationTree tree, VisitorState state) {
|
||||
if (!EXPLICIT_ORDERING.matches(tree, state)) {
|
||||
@@ -86,6 +89,6 @@ public final class ExplicitEnumOrdering extends BugChecker implements MethodInvo
|
||||
private static Stream<String> getMissingEnumValues(Type enumType, Set<String> values) {
|
||||
Symbol.TypeSymbol typeSymbol = enumType.asElement();
|
||||
return Sets.difference(ASTHelpers.enumValues(typeSymbol), values).stream()
|
||||
.map(v -> String.format("%s.%s", typeSymbol.getSimpleName(), v));
|
||||
.map(v -> String.join(".", typeSymbol.getSimpleName(), v));
|
||||
}
|
||||
}
|
||||
|
||||
@@ -5,6 +5,10 @@ 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 static tech.picnic.errorprone.bugpatterns.util.MoreTypes.generic;
|
||||
import static tech.picnic.errorprone.bugpatterns.util.MoreTypes.subOf;
|
||||
import static tech.picnic.errorprone.bugpatterns.util.MoreTypes.type;
|
||||
import static tech.picnic.errorprone.bugpatterns.util.MoreTypes.unbound;
|
||||
|
||||
import com.google.auto.service.AutoService;
|
||||
import com.google.common.collect.Iterables;
|
||||
@@ -17,11 +21,14 @@ import com.google.errorprone.fixes.SuggestedFix;
|
||||
import com.google.errorprone.fixes.SuggestedFixes;
|
||||
import com.google.errorprone.matchers.Description;
|
||||
import com.google.errorprone.matchers.Matcher;
|
||||
import com.google.errorprone.suppliers.Supplier;
|
||||
import com.google.errorprone.suppliers.Suppliers;
|
||||
import com.google.errorprone.util.ASTHelpers;
|
||||
import com.sun.source.tree.ExpressionTree;
|
||||
import com.sun.source.tree.MemberReferenceTree;
|
||||
import com.sun.source.tree.MethodInvocationTree;
|
||||
import com.sun.tools.javac.code.Type;
|
||||
import java.util.function.Function;
|
||||
import java.util.function.Supplier;
|
||||
import reactor.core.publisher.Flux;
|
||||
|
||||
/**
|
||||
@@ -33,11 +40,12 @@ import reactor.core.publisher.Flux;
|
||||
* former interleaves values as they are emitted, yielding nondeterministic results. In most cases
|
||||
* {@link Flux#concatMap(Function)} should be preferred, as it produces consistent results and
|
||||
* avoids potentially saturating the thread pool on which subscription happens. If {@code
|
||||
* concatMap}'s single-subscription semantics are undesirable one should invoke a {@code flatMap} or
|
||||
* {@code flatMapSequential} overload with an explicit concurrency level.
|
||||
* concatMap}'s sequential-subscription semantics are undesirable one should invoke a {@code
|
||||
* flatMap} or {@code flatMapSequential} overload with an explicit concurrency level.
|
||||
*
|
||||
* <p>NB: The rarely-used overload {@link Flux#flatMap(Function, Function, Supplier)} is not flagged
|
||||
* by this check because there is no clear alternative to point to.
|
||||
* <p>NB: The rarely-used overload {@link Flux#flatMap(Function, Function,
|
||||
* java.util.function.Supplier)} is not flagged by this check because there is no clear alternative
|
||||
* to point to.
|
||||
*/
|
||||
@AutoService(BugChecker.class)
|
||||
@BugPattern(
|
||||
@@ -52,11 +60,19 @@ public final class FluxFlatMapUsage extends BugChecker
|
||||
implements MethodInvocationTreeMatcher, MemberReferenceTreeMatcher {
|
||||
private static final long serialVersionUID = 1L;
|
||||
private static final String MAX_CONCURRENCY_ARG_NAME = "MAX_CONCURRENCY";
|
||||
private static final Supplier<Type> FLUX =
|
||||
Suppliers.typeFromString("reactor.core.publisher.Flux");
|
||||
private static final Matcher<ExpressionTree> FLUX_FLATMAP =
|
||||
instanceMethod()
|
||||
.onDescendantOf("reactor.core.publisher.Flux")
|
||||
.onDescendantOf(FLUX)
|
||||
.namedAnyOf("flatMap", "flatMapSequential")
|
||||
.withParameters(Function.class.getName());
|
||||
private static final Supplier<Type> FLUX_OF_PUBLISHERS =
|
||||
VisitorState.memoize(
|
||||
generic(FLUX, subOf(generic(type("org.reactivestreams.Publisher"), unbound()))));
|
||||
|
||||
/** Instantiates a new {@link FluxFlatMapUsage} instance. */
|
||||
public FluxFlatMapUsage() {}
|
||||
|
||||
@Override
|
||||
public Description matchMethodInvocation(MethodInvocationTree tree, VisitorState state) {
|
||||
@@ -64,14 +80,27 @@ public final class FluxFlatMapUsage extends BugChecker
|
||||
return Description.NO_MATCH;
|
||||
}
|
||||
|
||||
return buildDescription(tree)
|
||||
.addFix(SuggestedFixes.renameMethodInvocation(tree, "concatMap", state))
|
||||
.addFix(
|
||||
SuggestedFix.builder()
|
||||
.postfixWith(
|
||||
Iterables.getOnlyElement(tree.getArguments()), ", " + MAX_CONCURRENCY_ARG_NAME)
|
||||
.build())
|
||||
.build();
|
||||
SuggestedFix serializationFix = SuggestedFixes.renameMethodInvocation(tree, "concatMap", state);
|
||||
SuggestedFix concurrencyCapFix =
|
||||
SuggestedFix.builder()
|
||||
.postfixWith(
|
||||
Iterables.getOnlyElement(tree.getArguments()), ", " + MAX_CONCURRENCY_ARG_NAME)
|
||||
.build();
|
||||
|
||||
Description.Builder description = buildDescription(tree);
|
||||
|
||||
if (state.getTypes().isSubtype(ASTHelpers.getType(tree), FLUX_OF_PUBLISHERS.get(state))) {
|
||||
/*
|
||||
* Nested publishers may need to be subscribed to eagerly in order to avoid a deadlock, e.g.
|
||||
* if they are produced by `Flux#groupBy`. In this case we suggest specifying an explicit
|
||||
* concurrently bound, in favour of sequential subscriptions using `Flux#concatMap`.
|
||||
*/
|
||||
description.addFix(concurrencyCapFix).addFix(serializationFix);
|
||||
} else {
|
||||
description.addFix(serializationFix).addFix(concurrencyCapFix);
|
||||
}
|
||||
|
||||
return description.build();
|
||||
}
|
||||
|
||||
@Override
|
||||
|
||||
@@ -32,7 +32,7 @@ import com.sun.source.util.SimpleTreeVisitor;
|
||||
import java.util.ArrayList;
|
||||
import java.util.List;
|
||||
import java.util.Optional;
|
||||
import javax.annotation.Nullable;
|
||||
import org.jspecify.nullness.Nullable;
|
||||
import tech.picnic.errorprone.bugpatterns.util.SourceCode;
|
||||
|
||||
/**
|
||||
@@ -48,7 +48,7 @@ import tech.picnic.errorprone.bugpatterns.util.SourceCode;
|
||||
// `Formattable#toString` invocation with a `Formattable#formatTo` invocation. But likely that
|
||||
// should be considered a bug fix, too.
|
||||
// XXX: Introduce a separate check that adds/removes the `Locale` parameter to `String.format`
|
||||
// invocations, as necessary.
|
||||
// invocations, as necessary. See also a comment in the `StringJoin` check.
|
||||
@AutoService(BugChecker.class)
|
||||
@BugPattern(
|
||||
summary = "Defer string concatenation to the invoked method",
|
||||
@@ -129,6 +129,9 @@ public final class FormatStringConcatenation extends BugChecker
|
||||
.onDescendantOf("org.slf4j.Logger")
|
||||
.namedAnyOf("debug", "error", "info", "trace", "warn");
|
||||
|
||||
/** Instantiates a new {@link FormatStringConcatenation} instance. */
|
||||
public FormatStringConcatenation() {}
|
||||
|
||||
@Override
|
||||
public Description matchMethodInvocation(MethodInvocationTree tree, VisitorState state) {
|
||||
if (hasNonConstantStringConcatenationArgument(tree, 0, state)) {
|
||||
@@ -204,7 +207,7 @@ public final class FormatStringConcatenation extends BugChecker
|
||||
}
|
||||
|
||||
private static class ReplacementArgumentsConstructor
|
||||
extends SimpleTreeVisitor<Void, VisitorState> {
|
||||
extends SimpleTreeVisitor<@Nullable Void, VisitorState> {
|
||||
private final StringBuilder formatString = new StringBuilder();
|
||||
private final List<Tree> formatArguments = new ArrayList<>();
|
||||
private final String formatSpecifier;
|
||||
@@ -213,9 +216,8 @@ public final class FormatStringConcatenation extends BugChecker
|
||||
this.formatSpecifier = formatSpecifier;
|
||||
}
|
||||
|
||||
@Nullable
|
||||
@Override
|
||||
public Void visitBinary(BinaryTree tree, VisitorState state) {
|
||||
public @Nullable Void visitBinary(BinaryTree tree, VisitorState state) {
|
||||
if (tree.getKind() == Kind.PLUS && isStringTyped(tree, state)) {
|
||||
tree.getLeftOperand().accept(this, state);
|
||||
tree.getRightOperand().accept(this, state);
|
||||
@@ -226,15 +228,13 @@ public final class FormatStringConcatenation extends BugChecker
|
||||
return null;
|
||||
}
|
||||
|
||||
@Nullable
|
||||
@Override
|
||||
public Void visitParenthesized(ParenthesizedTree tree, VisitorState state) {
|
||||
public @Nullable Void visitParenthesized(ParenthesizedTree tree, VisitorState state) {
|
||||
return tree.getExpression().accept(this, state);
|
||||
}
|
||||
|
||||
@Nullable
|
||||
@Override
|
||||
protected Void defaultAction(Tree tree, VisitorState state) {
|
||||
protected @Nullable Void defaultAction(Tree tree, VisitorState state) {
|
||||
appendExpression(tree);
|
||||
return null;
|
||||
}
|
||||
|
||||
@@ -72,6 +72,9 @@ public final class IdentityConversion extends BugChecker implements MethodInvoca
|
||||
.namedAnyOf("concat", "firstWithSignal", "from", "merge"),
|
||||
staticMethod().onClass("reactor.core.publisher.Mono").namedAnyOf("from", "fromDirect"));
|
||||
|
||||
/** Instantiates a new {@link IdentityConversion} instance. */
|
||||
public IdentityConversion() {}
|
||||
|
||||
@Override
|
||||
public Description matchMethodInvocation(MethodInvocationTree tree, VisitorState state) {
|
||||
List<? extends ExpressionTree> arguments = tree.getArguments();
|
||||
|
||||
@@ -67,6 +67,9 @@ public final class ImmutablesSortedSetComparator extends BugChecker implements M
|
||||
hasAnnotation("org.immutables.value.Value.NaturalOrder"),
|
||||
hasAnnotation("org.immutables.value.Value.ReverseOrder"))));
|
||||
|
||||
/** Instantiates a new {@link ImmutablesSortedSetComparator} instance. */
|
||||
public ImmutablesSortedSetComparator() {}
|
||||
|
||||
@Override
|
||||
public Description matchMethod(MethodTree tree, VisitorState state) {
|
||||
if (!METHOD_LACKS_ANNOTATION.matches(tree, state)) {
|
||||
|
||||
@@ -79,6 +79,9 @@ public final class JUnitMethodDeclaration extends BugChecker implements MethodTr
|
||||
isType("org.junit.jupiter.api.BeforeAll"),
|
||||
isType("org.junit.jupiter.api.BeforeEach")));
|
||||
|
||||
/** Instantiates a new {@link JUnitMethodDeclaration} instance. */
|
||||
public JUnitMethodDeclaration() {}
|
||||
|
||||
@Override
|
||||
public Description matchMethod(MethodTree tree, VisitorState state) {
|
||||
if (HAS_UNMODIFIABLE_SIGNATURE.matches(tree, state)) {
|
||||
|
||||
@@ -1,5 +1,6 @@
|
||||
package tech.picnic.errorprone.bugpatterns;
|
||||
|
||||
import static com.google.common.collect.ImmutableList.toImmutableList;
|
||||
import static com.google.errorprone.BugPattern.LinkType.CUSTOM;
|
||||
import static com.google.errorprone.BugPattern.SeverityLevel.SUGGESTION;
|
||||
import static com.google.errorprone.BugPattern.StandardTags.STYLE;
|
||||
@@ -9,6 +10,7 @@ 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;
|
||||
@@ -38,7 +40,7 @@ import java.util.List;
|
||||
import java.util.Optional;
|
||||
import java.util.Set;
|
||||
import java.util.stream.Stream;
|
||||
import javax.annotation.Nullable;
|
||||
import org.jspecify.nullness.Nullable;
|
||||
import tech.picnic.errorprone.bugpatterns.util.AnnotationAttributeMatcher;
|
||||
import tech.picnic.errorprone.bugpatterns.util.SourceCode;
|
||||
|
||||
@@ -71,10 +73,16 @@ public final class LexicographicalAnnotationAttributeListing extends BugChecker
|
||||
private static final String FLAG_PREFIX = "LexicographicalAnnotationAttributeListing:";
|
||||
private static final String INCLUDED_ANNOTATIONS_FLAG = FLAG_PREFIX + "Includes";
|
||||
private static final String EXCLUDED_ANNOTATIONS_FLAG = FLAG_PREFIX + "Excludes";
|
||||
/**
|
||||
* The splitter applied to string-typed annotation arguments prior to lexicographical sorting. By
|
||||
* splitting on {@code =}, strings that represent e.g. inline Spring property declarations are
|
||||
* properly sorted by key, then value.
|
||||
*/
|
||||
private static final Splitter STRING_ARGUMENT_SPLITTER = Splitter.on('=');
|
||||
|
||||
private final AnnotationAttributeMatcher matcher;
|
||||
|
||||
/** Instantiates the default {@link LexicographicalAnnotationAttributeListing}. */
|
||||
/** Instantiates a default {@link LexicographicalAnnotationAttributeListing} instance. */
|
||||
public LexicographicalAnnotationAttributeListing() {
|
||||
this(ErrorProneFlags.empty());
|
||||
}
|
||||
@@ -126,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();
|
||||
@@ -156,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())))),
|
||||
@@ -173,38 +181,31 @@ public final class LexicographicalAnnotationAttributeListing extends BugChecker
|
||||
* performed. This approach disregards e.g. irrelevant whitespace. It also allows special
|
||||
* structure within string literals to be respected.
|
||||
*/
|
||||
private static ImmutableList<ImmutableList<String>> getStructure(
|
||||
ExpressionTree array, VisitorState state) {
|
||||
private static ImmutableList<ImmutableList<String>> getStructure(ExpressionTree array) {
|
||||
ImmutableList.Builder<ImmutableList<String>> nodes = ImmutableList.builder();
|
||||
|
||||
new TreeScanner<Void, Void>() {
|
||||
@Nullable
|
||||
new TreeScanner<@Nullable Void, @Nullable Void>() {
|
||||
@Override
|
||||
public Void visitIdentifier(IdentifierTree node, @Nullable Void ctx) {
|
||||
nodes.add(tokenize(node));
|
||||
return super.visitIdentifier(node, ctx);
|
||||
public @Nullable Void visitIdentifier(IdentifierTree node, @Nullable Void unused) {
|
||||
nodes.add(ImmutableList.of(node.getName().toString()));
|
||||
return super.visitIdentifier(node, unused);
|
||||
}
|
||||
|
||||
@Nullable
|
||||
@Override
|
||||
public Void visitLiteral(LiteralTree node, @Nullable Void ctx) {
|
||||
nodes.add(tokenize(node));
|
||||
return super.visitLiteral(node, ctx);
|
||||
public @Nullable Void visitLiteral(LiteralTree node, @Nullable Void unused) {
|
||||
Object value = ASTHelpers.constValue(node);
|
||||
nodes.add(
|
||||
value instanceof String
|
||||
? STRING_ARGUMENT_SPLITTER.splitToStream((String) value).collect(toImmutableList())
|
||||
: ImmutableList.of(String.valueOf(value)));
|
||||
|
||||
return super.visitLiteral(node, unused);
|
||||
}
|
||||
|
||||
@Nullable
|
||||
@Override
|
||||
public Void visitPrimitiveType(PrimitiveTypeTree node, @Nullable Void ctx) {
|
||||
nodes.add(tokenize(node));
|
||||
return super.visitPrimitiveType(node, ctx);
|
||||
}
|
||||
|
||||
private ImmutableList<String> tokenize(Tree node) {
|
||||
/*
|
||||
* Tokens are split on `=` so that e.g. inline Spring property declarations are properly
|
||||
* sorted by key, then value.
|
||||
*/
|
||||
return ImmutableList.copyOf(SourceCode.treeToString(node, state).split("=", -1));
|
||||
public @Nullable Void visitPrimitiveType(PrimitiveTypeTree node, @Nullable Void unused) {
|
||||
nodes.add(ImmutableList.of(node.getPrimitiveTypeKind().toString()));
|
||||
return super.visitPrimitiveType(node, unused);
|
||||
}
|
||||
}.scan(array, null);
|
||||
|
||||
|
||||
@@ -4,10 +4,13 @@ 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 com.sun.tools.javac.code.TypeAnnotations.AnnotationType.DECLARATION;
|
||||
import static com.sun.tools.javac.code.TypeAnnotations.AnnotationType.TYPE;
|
||||
import static java.util.Comparator.comparing;
|
||||
import static tech.picnic.errorprone.bugpatterns.util.Documentation.BUG_PATTERNS_BASE_URL;
|
||||
|
||||
import com.google.auto.service.AutoService;
|
||||
import com.google.common.base.VerifyException;
|
||||
import com.google.common.collect.ImmutableList;
|
||||
import com.google.common.collect.Streams;
|
||||
import com.google.errorprone.BugPattern;
|
||||
@@ -17,10 +20,14 @@ import com.google.errorprone.bugpatterns.BugChecker.MethodTreeMatcher;
|
||||
import com.google.errorprone.fixes.Fix;
|
||||
import com.google.errorprone.fixes.SuggestedFix;
|
||||
import com.google.errorprone.matchers.Description;
|
||||
import com.google.errorprone.util.ASTHelpers;
|
||||
import com.sun.source.tree.AnnotationTree;
|
||||
import com.sun.source.tree.MethodTree;
|
||||
import com.sun.tools.javac.code.Symbol;
|
||||
import com.sun.tools.javac.code.TypeAnnotations.AnnotationType;
|
||||
import java.util.Comparator;
|
||||
import java.util.List;
|
||||
import java.util.Optional;
|
||||
import org.jspecify.nullness.Nullable;
|
||||
import tech.picnic.errorprone.bugpatterns.util.SourceCode;
|
||||
|
||||
/**
|
||||
@@ -39,6 +46,12 @@ import tech.picnic.errorprone.bugpatterns.util.SourceCode;
|
||||
public final class LexicographicalAnnotationListing extends BugChecker
|
||||
implements MethodTreeMatcher {
|
||||
private static final long serialVersionUID = 1L;
|
||||
private static final Comparator<@Nullable AnnotationType> BY_ANNOTATION_TYPE =
|
||||
(a, b) ->
|
||||
(a == null || a == DECLARATION) && b == TYPE ? -1 : a == TYPE && b == DECLARATION ? 1 : 0;
|
||||
|
||||
/** Instantiates a new {@link LexicographicalAnnotationListing} instance. */
|
||||
public LexicographicalAnnotationListing() {}
|
||||
|
||||
@Override
|
||||
public Description matchMethod(MethodTree tree, VisitorState state) {
|
||||
@@ -47,26 +60,29 @@ public final class LexicographicalAnnotationListing extends BugChecker
|
||||
return Description.NO_MATCH;
|
||||
}
|
||||
|
||||
ImmutableList<? extends AnnotationTree> sortedAnnotations = sort(originalOrdering, state);
|
||||
ImmutableList<? extends AnnotationTree> sortedAnnotations =
|
||||
sort(originalOrdering, ASTHelpers.getSymbol(tree), state);
|
||||
if (originalOrdering.equals(sortedAnnotations)) {
|
||||
return Description.NO_MATCH;
|
||||
}
|
||||
|
||||
Optional<Fix> fix = tryFixOrdering(originalOrdering, sortedAnnotations, state);
|
||||
|
||||
Description.Builder description = buildDescription(originalOrdering.get(0));
|
||||
fix.ifPresent(description::addFix);
|
||||
return description.build();
|
||||
return describeMatch(
|
||||
originalOrdering.get(0), fixOrdering(originalOrdering, sortedAnnotations, state));
|
||||
}
|
||||
|
||||
private static ImmutableList<? extends AnnotationTree> sort(
|
||||
List<? extends AnnotationTree> annotations, VisitorState state) {
|
||||
List<? extends AnnotationTree> annotations, Symbol symbol, VisitorState state) {
|
||||
return annotations.stream()
|
||||
.sorted(comparing(annotation -> SourceCode.treeToString(annotation, state)))
|
||||
.sorted(
|
||||
comparing(
|
||||
(AnnotationTree annotation) ->
|
||||
ASTHelpers.getAnnotationType(annotation, symbol, state),
|
||||
BY_ANNOTATION_TYPE)
|
||||
.thenComparing(annotation -> SourceCode.treeToString(annotation, state)))
|
||||
.collect(toImmutableList());
|
||||
}
|
||||
|
||||
private static Optional<Fix> tryFixOrdering(
|
||||
private static Fix fixOrdering(
|
||||
List<? extends AnnotationTree> originalAnnotations,
|
||||
ImmutableList<? extends AnnotationTree> sortedAnnotations,
|
||||
VisitorState state) {
|
||||
@@ -77,6 +93,7 @@ public final class LexicographicalAnnotationListing extends BugChecker
|
||||
SuggestedFix.builder()
|
||||
.replace(original, SourceCode.treeToString(replacement, state)))
|
||||
.reduce(SuggestedFix.Builder::merge)
|
||||
.map(SuggestedFix.Builder::build);
|
||||
.map(SuggestedFix.Builder::build)
|
||||
.orElseThrow(() -> new VerifyException("No annotations were provided"));
|
||||
}
|
||||
}
|
||||
|
||||
@@ -60,6 +60,9 @@ import javax.lang.model.element.Name;
|
||||
public final class MethodReferenceUsage extends BugChecker implements LambdaExpressionTreeMatcher {
|
||||
private static final long serialVersionUID = 1L;
|
||||
|
||||
/** Instantiates a new {@link MethodReferenceUsage} instance. */
|
||||
public MethodReferenceUsage() {}
|
||||
|
||||
@Override
|
||||
public Description matchLambdaExpression(LambdaExpressionTree tree, VisitorState state) {
|
||||
/*
|
||||
@@ -122,7 +125,7 @@ public final class MethodReferenceUsage extends BugChecker implements LambdaExpr
|
||||
return Optional.empty();
|
||||
}
|
||||
Symbol sym = ASTHelpers.getSymbol(methodSelect);
|
||||
if (!sym.isStatic()) {
|
||||
if (!ASTHelpers.isStatic(sym)) {
|
||||
return constructFix(lambdaExpr, "this", methodSelect);
|
||||
}
|
||||
return constructFix(lambdaExpr, sym.owner, methodSelect);
|
||||
@@ -192,7 +195,7 @@ public final class MethodReferenceUsage extends BugChecker implements LambdaExpr
|
||||
Name sName = target.getSimpleName();
|
||||
Optional<SuggestedFix.Builder> fix = constructFix(lambdaExpr, sName, methodName);
|
||||
|
||||
if (!"java.lang".equals(target.packge().toString())) {
|
||||
if (!"java.lang".equals(ASTHelpers.enclosingPackage(target).toString())) {
|
||||
Name fqName = target.getQualifiedName();
|
||||
if (!sName.equals(fqName)) {
|
||||
return fix.map(b -> b.addImport(fqName.toString()));
|
||||
|
||||
@@ -25,7 +25,7 @@ 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",
|
||||
summary = "The Refaster rule contains a method without any Refaster annotations",
|
||||
link = BUG_PATTERNS_BASE_URL + "MissingRefasterAnnotation",
|
||||
linkType = CUSTOM,
|
||||
severity = WARNING,
|
||||
@@ -40,6 +40,9 @@ public final class MissingRefasterAnnotation extends BugChecker implements Class
|
||||
isType("com.google.errorprone.refaster.annotation.BeforeTemplate"),
|
||||
isType("com.google.errorprone.refaster.annotation.AfterTemplate")));
|
||||
|
||||
/** Instantiates a new {@link MissingRefasterAnnotation} instance. */
|
||||
public MissingRefasterAnnotation() {}
|
||||
|
||||
@Override
|
||||
public Description matchClass(ClassTree tree, VisitorState state) {
|
||||
long methodTypes =
|
||||
|
||||
@@ -36,6 +36,9 @@ public final class MockitoStubbing extends BugChecker implements MethodInvocatio
|
||||
private static final Matcher<ExpressionTree> MOCKITO_EQ_METHOD =
|
||||
staticMethod().onClass("org.mockito.ArgumentMatchers").named("eq");
|
||||
|
||||
/** Instantiates a new {@link MockitoStubbing} instance. */
|
||||
public MockitoStubbing() {}
|
||||
|
||||
@Override
|
||||
public Description matchMethodInvocation(MethodInvocationTree tree, VisitorState state) {
|
||||
List<? extends ExpressionTree> arguments = tree.getArguments();
|
||||
|
||||
@@ -4,9 +4,11 @@ import static com.google.errorprone.BugPattern.LinkType.CUSTOM;
|
||||
import static com.google.errorprone.BugPattern.SeverityLevel.WARNING;
|
||||
import static com.google.errorprone.BugPattern.StandardTags.FRAGILE_CODE;
|
||||
import static tech.picnic.errorprone.bugpatterns.util.Documentation.BUG_PATTERNS_BASE_URL;
|
||||
import static tech.picnic.errorprone.bugpatterns.util.MoreTypes.generic;
|
||||
import static tech.picnic.errorprone.bugpatterns.util.MoreTypes.raw;
|
||||
import static tech.picnic.errorprone.bugpatterns.util.MoreTypes.subOf;
|
||||
|
||||
import com.google.auto.service.AutoService;
|
||||
import com.google.common.collect.Iterables;
|
||||
import com.google.errorprone.BugPattern;
|
||||
import com.google.errorprone.VisitorState;
|
||||
import com.google.errorprone.bugpatterns.BugChecker;
|
||||
@@ -16,9 +18,7 @@ import com.google.errorprone.suppliers.Supplier;
|
||||
import com.google.errorprone.suppliers.Suppliers;
|
||||
import com.google.errorprone.util.ASTHelpers;
|
||||
import com.sun.source.tree.MethodInvocationTree;
|
||||
import com.sun.source.tree.Tree;
|
||||
import com.sun.tools.javac.code.Type;
|
||||
import com.sun.tools.javac.util.List;
|
||||
import java.util.Optional;
|
||||
|
||||
/** A {@link BugChecker} that flags nesting of {@link Optional Optionals}. */
|
||||
@@ -33,21 +33,16 @@ import java.util.Optional;
|
||||
public final class NestedOptionals extends BugChecker implements MethodInvocationTreeMatcher {
|
||||
private static final long serialVersionUID = 1L;
|
||||
private static final Supplier<Type> OPTIONAL = Suppliers.typeFromClass(Optional.class);
|
||||
private static final Supplier<Type> OPTIONAL_OF_OPTIONAL =
|
||||
VisitorState.memoize(generic(OPTIONAL, subOf(raw(OPTIONAL))));
|
||||
|
||||
/** Instantiates a new {@link NestedOptionals} instance. */
|
||||
public NestedOptionals() {}
|
||||
|
||||
@Override
|
||||
public Description matchMethodInvocation(MethodInvocationTree tree, VisitorState state) {
|
||||
return isOptionalOfOptional(tree, state) ? describeMatch(tree) : Description.NO_MATCH;
|
||||
}
|
||||
|
||||
private static boolean isOptionalOfOptional(Tree tree, VisitorState state) {
|
||||
Type optionalType = OPTIONAL.get(state);
|
||||
Type type = ASTHelpers.getType(tree);
|
||||
if (!ASTHelpers.isSubtype(type, optionalType, state)) {
|
||||
return false;
|
||||
}
|
||||
|
||||
List<Type> typeArguments = type.getTypeArguments();
|
||||
return !typeArguments.isEmpty()
|
||||
&& ASTHelpers.isSubtype(Iterables.getOnlyElement(typeArguments), optionalType, state);
|
||||
return state.getTypes().isSubtype(ASTHelpers.getType(tree), OPTIONAL_OF_OPTIONAL.get(state))
|
||||
? describeMatch(tree)
|
||||
: Description.NO_MATCH;
|
||||
}
|
||||
}
|
||||
|
||||
@@ -76,6 +76,9 @@ public final class NonEmptyMono extends BugChecker implements MethodInvocationTr
|
||||
.onDescendantOf("reactor.core.publisher.Mono")
|
||||
.namedAnyOf("defaultIfEmpty", "hasElement", "single"));
|
||||
|
||||
/** Instantiates a new {@link NonEmptyMono} instance. */
|
||||
public NonEmptyMono() {}
|
||||
|
||||
@Override
|
||||
public Description matchMethodInvocation(MethodInvocationTree tree, VisitorState state) {
|
||||
if (!MONO_SIZE_CHECK.matches(tree, state)) {
|
||||
|
||||
@@ -70,6 +70,9 @@ public final class PrimitiveComparison extends BugChecker implements MethodInvoc
|
||||
.named("thenComparing")
|
||||
.withParameters(Function.class.getName()));
|
||||
|
||||
/** Instantiates a new {@link PrimitiveComparison} instance. */
|
||||
public PrimitiveComparison() {}
|
||||
|
||||
@Override
|
||||
public Description matchMethodInvocation(MethodInvocationTree tree, VisitorState state) {
|
||||
boolean isStatic = STATIC_COMPARISON_METHOD.matches(tree, state);
|
||||
|
||||
@@ -141,7 +141,7 @@ public final class RedundantStringConversion extends BugChecker
|
||||
|
||||
private final Matcher<MethodInvocationTree> conversionMethodMatcher;
|
||||
|
||||
/** Instantiates the default {@link RedundantStringConversion}. */
|
||||
/** Instantiates a default {@link RedundantStringConversion} instance. */
|
||||
public RedundantStringConversion() {
|
||||
this(ErrorProneFlags.empty());
|
||||
}
|
||||
|
||||
@@ -22,8 +22,8 @@ import tech.picnic.errorprone.bugpatterns.util.SourceCode;
|
||||
/**
|
||||
* 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(
|
||||
@@ -37,6 +37,9 @@ public final class RefasterAnyOfUsage extends BugChecker implements MethodInvoca
|
||||
private static final Matcher<ExpressionTree> REFASTER_ANY_OF =
|
||||
staticMethod().onClass(Refaster.class.getName()).named("anyOf");
|
||||
|
||||
/** Instantiates a new {@link RefasterAnyOfUsage} instance. */
|
||||
public RefasterAnyOfUsage() {}
|
||||
|
||||
@Override
|
||||
public Description matchMethodInvocation(MethodInvocationTree tree, VisitorState state) {
|
||||
if (REFASTER_ANY_OF.matches(tree, state)) {
|
||||
|
||||
@@ -29,17 +29,17 @@ 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
|
||||
* A {@link BugChecker} that 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 + "RefasterTemplateModifiers",
|
||||
link = BUG_PATTERNS_BASE_URL + "RefasterRuleModifiers",
|
||||
linkType = CUSTOM,
|
||||
severity = SUGGESTION,
|
||||
tags = STYLE)
|
||||
public final class RefasterTemplateModifiers extends BugChecker
|
||||
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);
|
||||
@@ -48,6 +48,9 @@ public final class RefasterTemplateModifiers extends BugChecker
|
||||
private static final Matcher<Tree> REFASTER_METHOD =
|
||||
anyOf(BEFORE_TEMPLATE_METHOD, AFTER_TEMPLATE_METHOD, PLACEHOLDER_METHOD);
|
||||
|
||||
/** Instantiates a new {@link RefasterRuleModifiers} instance. */
|
||||
public RefasterRuleModifiers() {}
|
||||
|
||||
@Override
|
||||
public Description matchClass(ClassTree tree, VisitorState state) {
|
||||
if (!hasMatchingMember(tree, BEFORE_TEMPLATE_METHOD, state)) {
|
||||
@@ -85,8 +88,8 @@ public final class RefasterTemplateModifiers extends BugChecker
|
||||
|
||||
if (!hasMatchingMember(tree, PLACEHOLDER_METHOD, state)) {
|
||||
/*
|
||||
* Templates without a `@Placeholder` method should be `final`. Note that Refaster enforces
|
||||
* that `@Placeholder` methods are `abstract`, so templates _with_ such a method will
|
||||
* 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);
|
||||
@@ -83,6 +83,9 @@ public final class RequestMappingAnnotation extends BugChecker implements Method
|
||||
isSameType("org.springframework.web.util.UriBuilder"),
|
||||
isSameType("org.springframework.web.util.UriComponentsBuilder"))));
|
||||
|
||||
/** Instantiates a new {@link RequestMappingAnnotation} instance. */
|
||||
public RequestMappingAnnotation() {}
|
||||
|
||||
@Override
|
||||
public Description matchMethod(MethodTree tree, VisitorState state) {
|
||||
// XXX: Auto-add `@RequestParam` where applicable.
|
||||
|
||||
@@ -37,6 +37,9 @@ public final class RequestParamType extends BugChecker implements VariableTreeMa
|
||||
annotations(AT_LEAST_ONE, isType("org.springframework.web.bind.annotation.RequestParam")),
|
||||
anyOf(isSubtypeOf(ImmutableCollection.class), isSubtypeOf(ImmutableMap.class)));
|
||||
|
||||
/** Instantiates a new {@link RequestParamType} instance. */
|
||||
public RequestParamType() {}
|
||||
|
||||
@Override
|
||||
public Description matchVariable(VariableTree tree, VisitorState state) {
|
||||
return HAS_UNSUPPORTED_REQUEST_PARAM.matches(tree, state)
|
||||
|
||||
@@ -26,6 +26,7 @@ import com.google.errorprone.util.ASTHelpers;
|
||||
import com.sun.source.tree.AnnotationTree;
|
||||
import com.sun.source.tree.MethodTree;
|
||||
import com.sun.source.tree.Tree;
|
||||
import tech.picnic.errorprone.bugpatterns.util.ThirdPartyLibrary;
|
||||
|
||||
/**
|
||||
* A {@link BugChecker} that flags methods with Spring's {@code @Scheduled} annotation that lack New
|
||||
@@ -46,9 +47,13 @@ public final class ScheduledTransactionTrace extends BugChecker implements Metho
|
||||
private static final MultiMatcher<Tree, AnnotationTree> TRACE_ANNOTATION =
|
||||
annotations(AT_LEAST_ONE, isType(TRACE_ANNOTATION_FQCN));
|
||||
|
||||
/** Instantiates a new {@link ScheduledTransactionTrace} instance. */
|
||||
public ScheduledTransactionTrace() {}
|
||||
|
||||
@Override
|
||||
public Description matchMethod(MethodTree tree, VisitorState state) {
|
||||
if (!IS_SCHEDULED.matches(tree, state)) {
|
||||
if (!ThirdPartyLibrary.NEW_RELIC_AGENT_API.isIntroductionAllowed(state)
|
||||
|| !IS_SCHEDULED.matches(tree, state)) {
|
||||
return Description.NO_MATCH;
|
||||
}
|
||||
|
||||
|
||||
@@ -29,6 +29,7 @@ import tech.picnic.errorprone.bugpatterns.util.SourceCode;
|
||||
// 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)
|
||||
@@ -47,6 +48,9 @@ public final class Slf4jLogStatement extends BugChecker implements MethodInvocat
|
||||
.onDescendantOf("org.slf4j.Logger")
|
||||
.namedAnyOf("trace", "debug", "info", "warn", "error");
|
||||
|
||||
/** Instantiates a new {@link Slf4jLogStatement} instance. */
|
||||
public Slf4jLogStatement() {}
|
||||
|
||||
@Override
|
||||
public Description matchMethodInvocation(MethodInvocationTree tree, VisitorState state) {
|
||||
if (!SLF4J_LOGGER_INVOCATION.matches(tree, state)) {
|
||||
|
||||
@@ -57,6 +57,9 @@ public final class SpringMvcAnnotation extends BugChecker implements AnnotationT
|
||||
.put("PUT", "PutMapping")
|
||||
.build();
|
||||
|
||||
/** Instantiates a new {@link SpringMvcAnnotation} instance. */
|
||||
public SpringMvcAnnotation() {}
|
||||
|
||||
@Override
|
||||
public Description matchAnnotation(AnnotationTree tree, VisitorState state) {
|
||||
// XXX: We could remove the `@RequestMapping` import if not other usages remain.
|
||||
|
||||
@@ -101,7 +101,8 @@ public final class StaticImport extends BugChecker implements MemberSelectTreeMa
|
||||
"org.springframework.http.HttpMethod",
|
||||
"org.springframework.http.MediaType",
|
||||
"org.testng.Assert",
|
||||
"reactor.function.TupleUtils");
|
||||
"reactor.function.TupleUtils",
|
||||
"tech.picnic.errorprone.bugpatterns.util.MoreTypes");
|
||||
|
||||
/** Type members that should be statically imported. */
|
||||
@VisibleForTesting
|
||||
@@ -190,6 +191,9 @@ public final class StaticImport extends BugChecker implements MemberSelectTreeMa
|
||||
"of",
|
||||
"valueOf");
|
||||
|
||||
/** Instantiates a new {@link StaticImport} instance. */
|
||||
public StaticImport() {}
|
||||
|
||||
@Override
|
||||
public Description matchMemberSelect(MemberSelectTree tree, VisitorState state) {
|
||||
if (!isCandidateContext(state) || !isCandidate(tree)) {
|
||||
|
||||
@@ -0,0 +1,185 @@
|
||||
package tech.picnic.errorprone.bugpatterns;
|
||||
|
||||
import static com.google.errorprone.BugPattern.LinkType.NONE;
|
||||
import static com.google.errorprone.BugPattern.SeverityLevel.SUGGESTION;
|
||||
import static com.google.errorprone.BugPattern.StandardTags.SIMPLIFICATION;
|
||||
import static com.google.errorprone.matchers.method.MethodMatchers.staticMethod;
|
||||
|
||||
import com.google.auto.service.AutoService;
|
||||
import com.google.common.base.Splitter;
|
||||
import com.google.common.collect.ImmutableSet;
|
||||
import com.google.common.collect.Iterables;
|
||||
import com.google.errorprone.BugPattern;
|
||||
import com.google.errorprone.VisitorState;
|
||||
import com.google.errorprone.bugpatterns.BugChecker;
|
||||
import com.google.errorprone.bugpatterns.BugChecker.MethodInvocationTreeMatcher;
|
||||
import com.google.errorprone.fixes.SuggestedFix;
|
||||
import com.google.errorprone.matchers.Description;
|
||||
import com.google.errorprone.matchers.Matcher;
|
||||
import com.google.errorprone.suppliers.Supplier;
|
||||
import com.google.errorprone.suppliers.Suppliers;
|
||||
import com.google.errorprone.util.ASTHelpers;
|
||||
import com.sun.source.tree.ExpressionTree;
|
||||
import com.sun.source.tree.MethodInvocationTree;
|
||||
import com.sun.tools.javac.code.Type;
|
||||
import com.sun.tools.javac.util.Convert;
|
||||
import java.util.Formattable;
|
||||
import java.util.Iterator;
|
||||
import java.util.List;
|
||||
import org.jspecify.nullness.Nullable;
|
||||
import tech.picnic.errorprone.bugpatterns.util.SourceCode;
|
||||
|
||||
/**
|
||||
* A {@link BugChecker} that flags {@link String#format(String, Object...)} invocations which can be
|
||||
* replaced with a {@link String#join(CharSequence, CharSequence...)} or even a {@link
|
||||
* String#valueOf} invocation.
|
||||
*/
|
||||
// XXX: What about `v1 + "sep" + v2` and similar expressions? Do we want to rewrite those to
|
||||
// `String.join`, or should some `String.join` invocations be rewritten to use the `+` operator?
|
||||
// (The latter suggestion would conflict with the `FormatStringConcatenation` check.)
|
||||
@AutoService(BugChecker.class)
|
||||
@BugPattern(
|
||||
summary = "Prefer `String#join` over `String#format`",
|
||||
linkType = NONE,
|
||||
severity = SUGGESTION,
|
||||
tags = SIMPLIFICATION)
|
||||
public final class StringJoin extends BugChecker implements MethodInvocationTreeMatcher {
|
||||
private static final long serialVersionUID = 1L;
|
||||
private static final Splitter FORMAT_SPECIFIER_SPLITTER = Splitter.on("%s");
|
||||
private static final Matcher<ExpressionTree> STRING_FORMAT_INVOCATION =
|
||||
staticMethod().onClass(String.class.getName()).named("format");
|
||||
private static final Supplier<Type> CHAR_SEQUENCE_TYPE =
|
||||
Suppliers.typeFromClass(CharSequence.class);
|
||||
private static final Supplier<Type> FORMATTABLE_TYPE = Suppliers.typeFromClass(Formattable.class);
|
||||
|
||||
/** Instantiates a new {@link StringJoin} instance. */
|
||||
public StringJoin() {}
|
||||
|
||||
@Override
|
||||
public Description matchMethodInvocation(MethodInvocationTree tree, VisitorState state) {
|
||||
if (!STRING_FORMAT_INVOCATION.matches(tree, state)) {
|
||||
return Description.NO_MATCH;
|
||||
}
|
||||
|
||||
// XXX: This check assumes that if the first argument to `String#format` is a `Locale`, that
|
||||
// this argument is not vacuous, and that as a result the expression cannot be simplified using
|
||||
// `#valueOf` or `#join`. Implement a separate check that identifies and drops redundant
|
||||
// `Locale` arguments. See also a related comment in `FormatStringConcatenation`.
|
||||
String formatString = ASTHelpers.constValue(tree.getArguments().get(0), String.class);
|
||||
if (formatString == null) {
|
||||
return Description.NO_MATCH;
|
||||
}
|
||||
|
||||
List<String> separators = FORMAT_SPECIFIER_SPLITTER.splitToList(formatString);
|
||||
if (separators.size() < 2) {
|
||||
/* The format string does not contain `%s` format specifiers. */
|
||||
return Description.NO_MATCH;
|
||||
}
|
||||
|
||||
if (separators.size() != tree.getArguments().size()) {
|
||||
/* The number of arguments does not match the number of `%s` format specifiers. */
|
||||
return Description.NO_MATCH;
|
||||
}
|
||||
|
||||
int lastIndex = separators.size() - 1;
|
||||
if (!separators.get(0).isEmpty() || !separators.get(lastIndex).isEmpty()) {
|
||||
/* The format string contains leading or trailing characters. */
|
||||
return Description.NO_MATCH;
|
||||
}
|
||||
|
||||
ImmutableSet<String> innerSeparators = ImmutableSet.copyOf(separators.subList(1, lastIndex));
|
||||
if (innerSeparators.size() > 1) {
|
||||
/* The `%s` format specifiers are not uniformly separated. */
|
||||
return Description.NO_MATCH;
|
||||
}
|
||||
|
||||
if (innerSeparators.isEmpty()) {
|
||||
/*
|
||||
* This `String#format` invocation performs a straightforward string conversion; use
|
||||
* `String#valueOf` instead.
|
||||
*/
|
||||
return trySuggestExplicitStringConversion(tree, state);
|
||||
}
|
||||
|
||||
String separator = Iterables.getOnlyElement(innerSeparators);
|
||||
if (separator.indexOf('%') >= 0) {
|
||||
/* The `%s` format specifiers are separated by another format specifier. */
|
||||
// XXX: Strictly speaking we could support `%%` by mapping it to a literal `%`, but that
|
||||
// doesn't seem worth the trouble.
|
||||
return Description.NO_MATCH;
|
||||
}
|
||||
|
||||
return trySuggestExplicitJoin(tree, separator, state);
|
||||
}
|
||||
|
||||
/**
|
||||
* If guaranteed to be behavior preserving, suggests replacing {@code String.format("%s", arg)}
|
||||
* with {@code String.valueOf(arg)}.
|
||||
*
|
||||
* <p>If {@code arg} is already a string then the resultant conversion is vacuous. The {@link
|
||||
* IdentityConversion} check will subsequently drop it.
|
||||
*/
|
||||
private Description trySuggestExplicitStringConversion(
|
||||
MethodInvocationTree tree, VisitorState state) {
|
||||
ExpressionTree argument = tree.getArguments().get(1);
|
||||
if (isSubtype(ASTHelpers.getType(argument), FORMATTABLE_TYPE, state)) {
|
||||
/*
|
||||
* `Formattable` arguments are handled specially; `String#valueOf` is not a suitable
|
||||
* alternative.
|
||||
*/
|
||||
return Description.NO_MATCH;
|
||||
}
|
||||
|
||||
return buildDescription(tree)
|
||||
.setMessage("Prefer `String#valueOf` over `String#format`")
|
||||
.addFix(SuggestedFix.replace(tree, withStringConversionExpression(argument, state)))
|
||||
.build();
|
||||
}
|
||||
|
||||
/**
|
||||
* Unless the given {@code String.format} expression includes {@link Formattable} arguments,
|
||||
* suggests replacing it with a {@code String.join} expression using the specified argument
|
||||
* separator.
|
||||
*/
|
||||
private Description trySuggestExplicitJoin(
|
||||
MethodInvocationTree tree, String separator, VisitorState state) {
|
||||
Iterator<? extends ExpressionTree> arguments = tree.getArguments().iterator();
|
||||
|
||||
SuggestedFix.Builder fix =
|
||||
SuggestedFix.builder()
|
||||
.replace(tree.getMethodSelect(), "String.join")
|
||||
.replace(arguments.next(), String.format("\"%s\"", Convert.quote(separator)));
|
||||
|
||||
while (arguments.hasNext()) {
|
||||
ExpressionTree argument = arguments.next();
|
||||
Type argumentType = ASTHelpers.getType(argument);
|
||||
if (isSubtype(argumentType, FORMATTABLE_TYPE, state)) {
|
||||
/*
|
||||
* `Formattable` arguments are handled specially; `String#join` is not a suitable
|
||||
* alternative.
|
||||
*/
|
||||
return Description.NO_MATCH;
|
||||
}
|
||||
|
||||
if (!isSubtype(argumentType, CHAR_SEQUENCE_TYPE, state)) {
|
||||
/*
|
||||
* The argument was previously implicitly converted to a string; now this must happen
|
||||
* explicitly.
|
||||
*/
|
||||
fix.replace(argument, withStringConversionExpression(argument, state));
|
||||
}
|
||||
}
|
||||
|
||||
return describeMatch(tree, fix.build());
|
||||
}
|
||||
|
||||
private static boolean isSubtype(
|
||||
@Nullable Type subType, Supplier<Type> superType, VisitorState state) {
|
||||
return ASTHelpers.isSubtype(subType, superType.get(state), state);
|
||||
}
|
||||
|
||||
private static String withStringConversionExpression(
|
||||
ExpressionTree argument, VisitorState state) {
|
||||
return String.format("String.valueOf(%s)", SourceCode.treeToString(argument, state));
|
||||
}
|
||||
}
|
||||
@@ -26,6 +26,9 @@ import java.time.Instant;
|
||||
import java.time.LocalDate;
|
||||
import java.time.LocalDateTime;
|
||||
import java.time.LocalTime;
|
||||
import java.time.OffsetDateTime;
|
||||
import java.time.OffsetTime;
|
||||
import java.time.ZonedDateTime;
|
||||
|
||||
/** A {@link BugChecker} that flags illegal time-zone related operations. */
|
||||
@AutoService(BugChecker.class)
|
||||
@@ -58,10 +61,16 @@ public final class TimeZoneUsage extends BugChecker implements MethodInvocationT
|
||||
.onClassAny(
|
||||
LocalDate.class.getName(),
|
||||
LocalDateTime.class.getName(),
|
||||
LocalTime.class.getName())
|
||||
LocalTime.class.getName(),
|
||||
OffsetDateTime.class.getName(),
|
||||
OffsetTime.class.getName(),
|
||||
ZonedDateTime.class.getName())
|
||||
.named("now"),
|
||||
staticMethod().onClassAny(Instant.class.getName()).named("now").withNoParameters());
|
||||
|
||||
/** Instantiates a new {@link TimeZoneUsage} instance. */
|
||||
public TimeZoneUsage() {}
|
||||
|
||||
@Override
|
||||
public Description matchMethodInvocation(MethodInvocationTree tree, VisitorState state) {
|
||||
return BANNED_TIME_METHOD.matches(tree, state)
|
||||
|
||||
@@ -1,4 +1,4 @@
|
||||
/** Picnic Error Prone Contrib checks. */
|
||||
@com.google.errorprone.annotations.CheckReturnValue
|
||||
@javax.annotation.ParametersAreNonnullByDefault
|
||||
@org.jspecify.nullness.NullMarked
|
||||
package tech.picnic.errorprone.bugpatterns;
|
||||
|
||||
@@ -21,6 +21,9 @@ public final class MethodMatcherFactory {
|
||||
private static final Pattern METHOD_SIGNATURE =
|
||||
Pattern.compile("([^\\s#(,)]+)#([^\\s#(,)]+)\\(((?:[^\\s#(,)]+(?:,[^\\s#(,)]+)*)?)\\)");
|
||||
|
||||
/** Instantiates a new {@link MethodMatcherFactory} instance. */
|
||||
public MethodMatcherFactory() {}
|
||||
|
||||
/**
|
||||
* Creates a {@link Matcher} of methods with any of the given signatures.
|
||||
*
|
||||
|
||||
@@ -0,0 +1,132 @@
|
||||
package tech.picnic.errorprone.bugpatterns.util;
|
||||
|
||||
import static java.util.stream.Collectors.toCollection;
|
||||
|
||||
import com.google.errorprone.VisitorState;
|
||||
import com.google.errorprone.suppliers.Supplier;
|
||||
import com.google.errorprone.suppliers.Suppliers;
|
||||
import com.sun.tools.javac.code.BoundKind;
|
||||
import com.sun.tools.javac.code.Type;
|
||||
import java.util.ArrayList;
|
||||
import java.util.Arrays;
|
||||
import java.util.List;
|
||||
import java.util.Objects;
|
||||
import java.util.Optional;
|
||||
import java.util.function.BiFunction;
|
||||
|
||||
/**
|
||||
* A set of helper methods which together define a DSL for defining {@link Type types}.
|
||||
*
|
||||
* <p>These methods are meant to be statically imported. Example usage:
|
||||
*
|
||||
* <pre>{@code
|
||||
* Supplier<Type> type =
|
||||
* VisitorState.memoize(
|
||||
* generic(
|
||||
* type("reactor.core.publisher.Flux"),
|
||||
* subOf(generic(type("org.reactivestreams.Publisher"), unbound()))));
|
||||
* }</pre>
|
||||
*
|
||||
* This statement produces a memoized supplier of the type {@code Flux<? extends Publisher<?>>}.
|
||||
*/
|
||||
public final class MoreTypes {
|
||||
private MoreTypes() {}
|
||||
|
||||
/**
|
||||
* Creates a supplier of the type with the given fully qualified name.
|
||||
*
|
||||
* <p>This method should only be used when building more complex types in combination with other
|
||||
* {@link MoreTypes} methods. In other cases prefer directly calling {@link
|
||||
* Suppliers#typeFromString(String)}.
|
||||
*
|
||||
* @param typeName The type of interest.
|
||||
* @return A supplier which returns the described type if available in the given state, and {@code
|
||||
* null} otherwise.
|
||||
*/
|
||||
public static Supplier<Type> type(String typeName) {
|
||||
return Suppliers.typeFromString(typeName);
|
||||
}
|
||||
|
||||
/**
|
||||
* Creates a supplier of the described generic type.
|
||||
*
|
||||
* @param type The base type of interest.
|
||||
* @param typeArgs The desired type arguments.
|
||||
* @return A supplier which returns the described type if available in the given state, and {@code
|
||||
* null} otherwise.
|
||||
*/
|
||||
// XXX: The given `type` should be a generic type, so perhaps `withParams` would be a better
|
||||
// method name. But the DSL wouldn't look as nice that way.
|
||||
@SafeVarargs
|
||||
@SuppressWarnings("varargs")
|
||||
public static Supplier<Type> generic(Supplier<Type> type, Supplier<Type>... typeArgs) {
|
||||
return propagateNull(
|
||||
type,
|
||||
(state, baseType) -> {
|
||||
List<Type> params =
|
||||
Arrays.stream(typeArgs).map(s -> s.get(state)).collect(toCollection(ArrayList::new));
|
||||
if (params.stream().anyMatch(Objects::isNull)) {
|
||||
return null;
|
||||
}
|
||||
|
||||
return state.getType(baseType, /* isArray= */ false, params);
|
||||
});
|
||||
}
|
||||
|
||||
/**
|
||||
* Creates a raw (erased, non-generic) variant of the given type.
|
||||
*
|
||||
* @param type The base type of interest.
|
||||
* @return A supplier which returns the described type if available in the given state, and {@code
|
||||
* null} otherwise.
|
||||
*/
|
||||
public static Supplier<Type> raw(Supplier<Type> type) {
|
||||
return propagateNull(type, (state, baseType) -> baseType.tsym.erasure(state.getTypes()));
|
||||
}
|
||||
|
||||
/**
|
||||
* Creates a {@code ? super T} wildcard type, with {@code T} bound to the given type.
|
||||
*
|
||||
* @param type The base type of interest.
|
||||
* @return A supplier which returns the described type if available in the given state, and {@code
|
||||
* null} otherwise.
|
||||
*/
|
||||
public static Supplier<Type> superOf(Supplier<Type> type) {
|
||||
return propagateNull(
|
||||
type,
|
||||
(state, baseType) ->
|
||||
new Type.WildcardType(baseType, BoundKind.SUPER, state.getSymtab().boundClass));
|
||||
}
|
||||
|
||||
/**
|
||||
* Creates a {@code ? extends T} wildcard type, with {@code T} bound to the given type.
|
||||
*
|
||||
* @param type The base type of interest.
|
||||
* @return A supplier which returns the described type if available in the given state, and {@code
|
||||
* null} otherwise.
|
||||
*/
|
||||
public static Supplier<Type> subOf(Supplier<Type> type) {
|
||||
return propagateNull(
|
||||
type,
|
||||
(state, baseType) ->
|
||||
new Type.WildcardType(
|
||||
type.get(state), BoundKind.EXTENDS, state.getSymtab().boundClass));
|
||||
}
|
||||
|
||||
/**
|
||||
* Creates an unbound wildcard type ({@code ?}).
|
||||
*
|
||||
* @return A supplier which returns the described type.
|
||||
*/
|
||||
public static Supplier<Type> unbound() {
|
||||
return state ->
|
||||
new Type.WildcardType(
|
||||
state.getSymtab().objectType, BoundKind.UNBOUND, state.getSymtab().boundClass);
|
||||
}
|
||||
|
||||
private static Supplier<Type> propagateNull(
|
||||
Supplier<Type> type, BiFunction<VisitorState, Type, Type> transformer) {
|
||||
return state ->
|
||||
Optional.ofNullable(type.get(state)).map(t -> transformer.apply(state, t)).orElse(null);
|
||||
}
|
||||
}
|
||||
@@ -0,0 +1,105 @@
|
||||
package tech.picnic.errorprone.bugpatterns.util;
|
||||
|
||||
import com.google.errorprone.VisitorState;
|
||||
import com.google.errorprone.bugpatterns.BugChecker;
|
||||
import com.google.errorprone.suppliers.Supplier;
|
||||
import com.sun.tools.javac.code.ClassFinder;
|
||||
import com.sun.tools.javac.code.Symbol.CompletionFailure;
|
||||
import com.sun.tools.javac.util.Name;
|
||||
|
||||
/**
|
||||
* Utility class that helps decide whether it is appropriate to introduce references to (well-known)
|
||||
* third-party libraries.
|
||||
*
|
||||
* <p>This class should be used by {@link BugChecker}s that may otherwise suggest the introduction
|
||||
* of code that depends on possibly-not-present third-party libraries.
|
||||
*/
|
||||
// XXX: Consider giving users more fine-grained control. This would be beneficial in cases where a
|
||||
// dependency is on the classpath, but new usages are undesirable.
|
||||
public enum ThirdPartyLibrary {
|
||||
/**
|
||||
* AssertJ.
|
||||
*
|
||||
* @see <a href="https://assertj.github.io/doc">AssertJ documentation</a>
|
||||
*/
|
||||
ASSERTJ("org.assertj.core.api.Assertions"),
|
||||
/**
|
||||
* Google's Guava.
|
||||
*
|
||||
* @see <a href="https://github.com/google/guava">Guava on GitHub</a>
|
||||
*/
|
||||
GUAVA("com.google.common.collect.ImmutableList"),
|
||||
/**
|
||||
* New Relic's Java agent API.
|
||||
*
|
||||
* @see <a href="https://github.com/newrelic/newrelic-java-agent/tree/main/newrelic-api">New Relic
|
||||
* Java agent API on GitHub</a>
|
||||
*/
|
||||
NEW_RELIC_AGENT_API("com.newrelic.api.agent.Agent"),
|
||||
/**
|
||||
* VMWare's Project Reactor.
|
||||
*
|
||||
* @see <a href="https://projectreactor.io">Home page</a>
|
||||
*/
|
||||
REACTOR("reactor.core.publisher.Flux");
|
||||
|
||||
private static final String IGNORE_CLASSPATH_COMPAT_FLAG =
|
||||
"ErrorProneSupport:IgnoreClasspathCompat";
|
||||
|
||||
@SuppressWarnings("ImmutableEnumChecker" /* Supplier is deterministic. */)
|
||||
private final Supplier<Boolean> canUse;
|
||||
|
||||
/**
|
||||
* Instantiates a {@link ThirdPartyLibrary} enum value.
|
||||
*
|
||||
* @param witnessFqcn The fully-qualified class name of a type that is expected to be on the
|
||||
* classpath iff the associated third-party library is on the classpath.
|
||||
*/
|
||||
ThirdPartyLibrary(String witnessFqcn) {
|
||||
this.canUse = VisitorState.memoize(state -> canIntroduceUsage(witnessFqcn, state));
|
||||
}
|
||||
|
||||
/**
|
||||
* Tells whether it is okay to introduce a dependency on this well-known third party library in
|
||||
* the given context.
|
||||
*
|
||||
* @param state The context under consideration.
|
||||
* @return {@code true} iff it is okay to assume or create a dependency on this library.
|
||||
*/
|
||||
public boolean isIntroductionAllowed(VisitorState state) {
|
||||
return canUse.get(state);
|
||||
}
|
||||
|
||||
private static boolean canIntroduceUsage(String className, VisitorState state) {
|
||||
return shouldIgnoreClasspath(state) || isKnownClass(className, state);
|
||||
}
|
||||
|
||||
/**
|
||||
* Attempts to determine whether a class with the given FQCN is on the classpath.
|
||||
*
|
||||
* <p>The {@link VisitorState}'s symbol table is consulted first. If the type has not yet been
|
||||
* loaded, then an attempt is made to do so.
|
||||
*/
|
||||
private static boolean isKnownClass(String className, VisitorState state) {
|
||||
return state.getTypeFromString(className) != null || canLoadClass(className, state);
|
||||
}
|
||||
|
||||
private static boolean canLoadClass(String className, VisitorState state) {
|
||||
ClassFinder classFinder = ClassFinder.instance(state.context);
|
||||
Name binaryName = state.binaryNameFromClassname(className);
|
||||
try {
|
||||
classFinder.loadClass(state.getSymtab().unnamedModule, binaryName);
|
||||
return true;
|
||||
} catch (CompletionFailure e) {
|
||||
return false;
|
||||
}
|
||||
}
|
||||
|
||||
private static boolean shouldIgnoreClasspath(VisitorState state) {
|
||||
return state
|
||||
.errorProneOptions()
|
||||
.getFlags()
|
||||
.getBoolean(IGNORE_CLASSPATH_COMPAT_FLAG)
|
||||
.orElse(Boolean.FALSE);
|
||||
}
|
||||
}
|
||||
@@ -1,4 +1,4 @@
|
||||
/** Auxiliary utilities for use by Error Prone checks. */
|
||||
@com.google.errorprone.annotations.CheckReturnValue
|
||||
@javax.annotation.ParametersAreNonnullByDefault
|
||||
@org.jspecify.nullness.NullMarked
|
||||
package tech.picnic.errorprone.bugpatterns.util;
|
||||
|
||||
@@ -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 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
|
||||
@@ -1,4 +1,4 @@
|
||||
package tech.picnic.errorprone.refastertemplates;
|
||||
package tech.picnic.errorprone.refasterrules;
|
||||
|
||||
import static com.google.errorprone.refaster.ImportPolicy.STATIC_IMPORT_ALWAYS;
|
||||
import static org.assertj.core.api.Assertions.assertThat;
|
||||
@@ -8,9 +8,11 @@ import com.google.errorprone.refaster.annotation.BeforeTemplate;
|
||||
import com.google.errorprone.refaster.annotation.UseImportPolicy;
|
||||
import org.assertj.core.api.AbstractComparableAssert;
|
||||
import org.assertj.core.api.AbstractIntegerAssert;
|
||||
import tech.picnic.errorprone.refaster.annotation.OnlineDocumentation;
|
||||
|
||||
final class AssertJComparableTemplates {
|
||||
private AssertJComparableTemplates() {}
|
||||
@OnlineDocumentation
|
||||
final class AssertJComparableRules {
|
||||
private AssertJComparableRules() {}
|
||||
|
||||
static final class AssertThatIsEqualByComparingTo<T extends Comparable<? super T>> {
|
||||
@BeforeTemplate
|
||||
@@ -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;
|
||||
@@ -19,10 +19,12 @@ import org.assertj.core.api.AbstractIntegerAssert;
|
||||
import org.assertj.core.api.AbstractLongAssert;
|
||||
import org.assertj.core.api.AbstractShortAssert;
|
||||
import org.assertj.core.api.NumberAssert;
|
||||
import tech.picnic.errorprone.refaster.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
|
||||
@@ -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
|
||||
@@ -1,4 +1,4 @@
|
||||
package tech.picnic.errorprone.refastertemplates;
|
||||
package tech.picnic.errorprone.refasterrules;
|
||||
|
||||
import static com.google.errorprone.refaster.ImportPolicy.STATIC_IMPORT_ALWAYS;
|
||||
import static org.assertj.core.api.Assertions.assertThat;
|
||||
@@ -9,9 +9,11 @@ import com.google.errorprone.refaster.annotation.BeforeTemplate;
|
||||
import com.google.errorprone.refaster.annotation.UseImportPolicy;
|
||||
import org.assertj.core.api.AbstractBooleanAssert;
|
||||
import org.assertj.core.api.AbstractDoubleAssert;
|
||||
import tech.picnic.errorprone.refaster.annotation.OnlineDocumentation;
|
||||
|
||||
final class AssertJPrimitiveTemplates {
|
||||
private AssertJPrimitiveTemplates() {}
|
||||
@OnlineDocumentation
|
||||
final class AssertJPrimitiveRules {
|
||||
private AssertJPrimitiveRules() {}
|
||||
|
||||
static final class AssertThatIsEqualTo {
|
||||
@BeforeTemplate
|
||||
@@ -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.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.matchers.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 that "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,7 +102,7 @@ import tech.picnic.errorprone.refaster.matchers.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}`
|
||||
@@ -118,10 +119,11 @@ import tech.picnic.errorprone.refaster.matchers.IsArray;
|
||||
// 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 that 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 that 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;
|
||||
@@ -27,14 +27,16 @@ import java.util.Map;
|
||||
import java.util.Objects;
|
||||
import java.util.Set;
|
||||
import java.util.stream.Stream;
|
||||
import javax.annotation.Nullable;
|
||||
import org.jspecify.nullness.Nullable;
|
||||
import tech.picnic.errorprone.refaster.annotation.OnlineDocumentation;
|
||||
|
||||
/**
|
||||
* Assorted Refaster templates that do not (yet) belong in one of the other classes with more
|
||||
* topical Refaster templates.
|
||||
* Assorted Refaster rules that do not (yet) belong in one of the other classes with more topical
|
||||
* Refaster rules.
|
||||
*/
|
||||
final class AssortedTemplates {
|
||||
private AssortedTemplates() {}
|
||||
@OnlineDocumentation
|
||||
final class AssortedRules {
|
||||
private AssortedRules() {}
|
||||
|
||||
/** Prefer {@link Objects#checkIndex(int, int)} over the Guava alternative. */
|
||||
static final class CheckIndex {
|
||||
@@ -44,11 +46,33 @@ final class AssortedTemplates {
|
||||
}
|
||||
|
||||
@AfterTemplate
|
||||
@UseImportPolicy(STATIC_IMPORT_ALWAYS)
|
||||
int after(int index, int size) {
|
||||
return checkIndex(index, size);
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* Prefer {@link Objects#checkIndex(int, int)} over less descriptive or more verbose alternatives.
|
||||
*
|
||||
* <p>If a custom error message is desired, consider using Guava's {@link
|
||||
* com.google.common.base.Preconditions#checkElementIndex(int, int, String)}.
|
||||
*/
|
||||
static final class CheckIndexConditional {
|
||||
@BeforeTemplate
|
||||
void before(int index, int size) {
|
||||
if (index < 0 || index >= size) {
|
||||
throw new IndexOutOfBoundsException();
|
||||
}
|
||||
}
|
||||
|
||||
@AfterTemplate
|
||||
@UseImportPolicy(STATIC_IMPORT_ALWAYS)
|
||||
void after(int index, int size) {
|
||||
checkIndex(index, size);
|
||||
}
|
||||
}
|
||||
|
||||
// XXX: We could add a rule for `new EnumMap(Map<K, ? extends V> m)`, but that constructor does
|
||||
// not allow an empty non-EnumMap to be provided.
|
||||
static final class CreateEnumMap<K extends Enum<K>, V> {
|
||||
@@ -65,14 +89,12 @@ final class AssortedTemplates {
|
||||
|
||||
static final class MapGetOrNull<K, V, L> {
|
||||
@BeforeTemplate
|
||||
@Nullable
|
||||
V before(Map<K, V> map, L key) {
|
||||
@Nullable V before(Map<K, V> map, L key) {
|
||||
return map.getOrDefault(key, null);
|
||||
}
|
||||
|
||||
@AfterTemplate
|
||||
@Nullable
|
||||
V after(Map<K, V> map, L key) {
|
||||
@Nullable V after(Map<K, V> map, L key) {
|
||||
return map.get(key);
|
||||
}
|
||||
}
|
||||
@@ -110,14 +132,13 @@ final class AssortedTemplates {
|
||||
}
|
||||
|
||||
@AfterTemplate
|
||||
@Nullable
|
||||
T after(Iterator<T> iterator, T defaultValue) {
|
||||
@Nullable T after(Iterator<T> iterator, T defaultValue) {
|
||||
return Iterators.getNext(iterator, defaultValue);
|
||||
}
|
||||
}
|
||||
|
||||
/** Don't unnecessarily repeat boolean expressions. */
|
||||
// XXX: This template captures only the simplest case. `@AlsoNegation` doesn't help. Consider
|
||||
// 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 {
|
||||
@@ -172,8 +193,8 @@ final class AssortedTemplates {
|
||||
* Collections#disjoint(Collection, Collection)}.
|
||||
*/
|
||||
// XXX: Other copy operations could be elided too, but these are most common after application of
|
||||
// the `DisjointSets` template defined above. If we ever introduce a generic "makes a copy"
|
||||
// stand-in, use it here.
|
||||
// the `DisjointSets` rule defined above. If we ever introduce a generic "makes a copy" stand-in,
|
||||
// use it here.
|
||||
static final class DisjointCollections<T> {
|
||||
@BeforeTemplate
|
||||
boolean before(Collection<T> collection1, Collection<T> collection2) {
|
||||
@@ -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;
|
||||
@@ -24,10 +24,12 @@ 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>> {
|
||||
@@ -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.
|
||||
@@ -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,10 +24,12 @@ 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
|
||||
@@ -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
|
||||
@@ -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,10 +20,12 @@ 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. See
|
||||
@@ -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() {
|
||||
@@ -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,10 +21,12 @@ 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. See
|
||||
@@ -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,10 +13,12 @@ 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. See
|
||||
@@ -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,10 +21,12 @@ 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. See
|
||||
@@ -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,10 +17,12 @@ 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. See
|
||||
@@ -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() {
|
||||
@@ -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> {
|
||||
@@ -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
|
||||
@@ -1,4 +1,4 @@
|
||||
package tech.picnic.errorprone.refastertemplates;
|
||||
package tech.picnic.errorprone.refasterrules;
|
||||
|
||||
import com.google.common.collect.Multimap;
|
||||
import com.google.common.collect.Multimaps;
|
||||
@@ -7,11 +7,13 @@ import com.google.errorprone.refaster.annotation.AfterTemplate;
|
||||
import com.google.errorprone.refaster.annotation.BeforeTemplate;
|
||||
import java.util.Collection;
|
||||
import java.util.Set;
|
||||
import javax.annotation.Nullable;
|
||||
import org.jspecify.nullness.Nullable;
|
||||
import tech.picnic.errorprone.refaster.annotation.OnlineDocumentation;
|
||||
|
||||
/** Refaster templates related to expressions dealing with {@link Multimap}s. */
|
||||
final class MultimapTemplates {
|
||||
private MultimapTemplates() {}
|
||||
/** Refaster rules related to expressions dealing with {@link Multimap}s. */
|
||||
@OnlineDocumentation
|
||||
final class MultimapRules {
|
||||
private MultimapRules() {}
|
||||
|
||||
/** Prefer {@link Multimap#keySet()} over more contrived alternatives. */
|
||||
static final class MultimapKeySet<K, V> {
|
||||
@@ -48,8 +50,7 @@ final class MultimapTemplates {
|
||||
*/
|
||||
static final class MultimapGet<K, V> {
|
||||
@BeforeTemplate
|
||||
@Nullable
|
||||
Collection<V> before(Multimap<K, V> multimap, K key) {
|
||||
@Nullable Collection<V> before(Multimap<K, V> multimap, K key) {
|
||||
return Refaster.anyOf(multimap.asMap(), Multimaps.asMap(multimap)).get(key);
|
||||
}
|
||||
|
||||
@@ -1,4 +1,4 @@
|
||||
package tech.picnic.errorprone.refastertemplates;
|
||||
package tech.picnic.errorprone.refasterrules;
|
||||
|
||||
import static com.google.errorprone.refaster.ImportPolicy.STATIC_IMPORT_ALWAYS;
|
||||
import static java.util.Objects.requireNonNullElse;
|
||||
@@ -9,11 +9,13 @@ import com.google.errorprone.refaster.annotation.BeforeTemplate;
|
||||
import com.google.errorprone.refaster.annotation.UseImportPolicy;
|
||||
import java.util.Objects;
|
||||
import java.util.function.Predicate;
|
||||
import javax.annotation.Nullable;
|
||||
import org.jspecify.nullness.Nullable;
|
||||
import tech.picnic.errorprone.refaster.annotation.OnlineDocumentation;
|
||||
|
||||
/** Refaster templates related to expressions dealing with (possibly) null values. */
|
||||
final class NullTemplates {
|
||||
private NullTemplates() {}
|
||||
/** Refaster rules related to expressions dealing with (possibly) null values. */
|
||||
@OnlineDocumentation
|
||||
final class NullRules {
|
||||
private NullRules() {}
|
||||
|
||||
/** Prefer the {@code ==} operator over {@link Objects#isNull(Object)}. */
|
||||
static final class IsNull {
|
||||
@@ -1,4 +1,4 @@
|
||||
package tech.picnic.errorprone.refastertemplates;
|
||||
package tech.picnic.errorprone.refasterrules;
|
||||
|
||||
import static com.google.errorprone.refaster.ImportPolicy.STATIC_IMPORT_ALWAYS;
|
||||
|
||||
@@ -16,11 +16,13 @@ import java.util.function.Function;
|
||||
import java.util.function.Predicate;
|
||||
import java.util.function.Supplier;
|
||||
import java.util.stream.Stream;
|
||||
import javax.annotation.Nullable;
|
||||
import org.jspecify.nullness.Nullable;
|
||||
import tech.picnic.errorprone.refaster.annotation.OnlineDocumentation;
|
||||
|
||||
/** Refaster templates related to expressions dealing with {@link Optional}s. */
|
||||
final class OptionalTemplates {
|
||||
private OptionalTemplates() {}
|
||||
/** Refaster rules related to expressions dealing with {@link Optional}s. */
|
||||
@OnlineDocumentation
|
||||
final class OptionalRules {
|
||||
private OptionalRules() {}
|
||||
|
||||
static final class OptionalOfNullable<T> {
|
||||
// XXX: Refaster should be smart enough to also rewrite occurrences in which there are
|
||||
@@ -78,8 +80,8 @@ final class OptionalTemplates {
|
||||
}
|
||||
|
||||
/** Prefer {@link Optional#orElseThrow()} over the less explicit {@link Optional#get()}. */
|
||||
// XXX: This template is analogous to `OptionalOrElseThrow` above. Arguably this is its
|
||||
// generalization. If/when Refaster is extended to understand this, delete the template above.
|
||||
// XXX: This rule is analogous to `OptionalOrElseThrow` above. Arguably this is its
|
||||
// generalization. If/when Refaster is extended to understand this, delete the rule above.
|
||||
static final class OptionalOrElseThrowMethodReference<T> {
|
||||
@BeforeTemplate
|
||||
Function<Optional<T>, T> before() {
|
||||
@@ -0,0 +1,176 @@
|
||||
package tech.picnic.errorprone.refasterrules;
|
||||
|
||||
import static com.google.common.base.Preconditions.checkArgument;
|
||||
import static com.google.common.base.Preconditions.checkElementIndex;
|
||||
import static com.google.common.base.Preconditions.checkNotNull;
|
||||
import static com.google.common.base.Preconditions.checkPositionIndex;
|
||||
import static com.google.common.base.Preconditions.checkState;
|
||||
import static com.google.errorprone.refaster.ImportPolicy.STATIC_IMPORT_ALWAYS;
|
||||
|
||||
import com.google.common.base.Preconditions;
|
||||
import com.google.errorprone.refaster.annotation.AfterTemplate;
|
||||
import com.google.errorprone.refaster.annotation.BeforeTemplate;
|
||||
import com.google.errorprone.refaster.annotation.UseImportPolicy;
|
||||
import tech.picnic.errorprone.refaster.annotation.OnlineDocumentation;
|
||||
|
||||
/** Refaster templates related to statements dealing with {@link Preconditions}. */
|
||||
@OnlineDocumentation
|
||||
final class PreconditionsRules {
|
||||
private PreconditionsRules() {}
|
||||
|
||||
/** Prefer {@link Preconditions#checkArgument(boolean)} over more verbose alternatives. */
|
||||
static final class CheckArgument {
|
||||
@BeforeTemplate
|
||||
void before(boolean condition) {
|
||||
if (condition) {
|
||||
throw new IllegalArgumentException();
|
||||
}
|
||||
}
|
||||
|
||||
@AfterTemplate
|
||||
@UseImportPolicy(STATIC_IMPORT_ALWAYS)
|
||||
void after(boolean condition) {
|
||||
checkArgument(!condition);
|
||||
}
|
||||
}
|
||||
|
||||
/** Prefer {@link Preconditions#checkArgument(boolean, Object)} over more verbose alternatives. */
|
||||
static final class CheckArgumentWithMessage {
|
||||
@BeforeTemplate
|
||||
void before(boolean condition, String message) {
|
||||
if (condition) {
|
||||
throw new IllegalArgumentException(message);
|
||||
}
|
||||
}
|
||||
|
||||
@AfterTemplate
|
||||
@UseImportPolicy(STATIC_IMPORT_ALWAYS)
|
||||
void after(boolean condition, String message) {
|
||||
checkArgument(!condition, message);
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* Prefer {@link Preconditions#checkElementIndex(int, int, String)} over less descriptive or more
|
||||
* verbose alternatives.
|
||||
*
|
||||
* <p>Note that the two-argument {@link Preconditions#checkElementIndex(int, int)} is better
|
||||
* replaced with {@link java.util.Objects#checkIndex(int, int)}.
|
||||
*/
|
||||
static final class CheckElementIndexWithMessage {
|
||||
@BeforeTemplate
|
||||
void before(int index, int size, String message) {
|
||||
if (index < 0 || index >= size) {
|
||||
throw new IndexOutOfBoundsException(message);
|
||||
}
|
||||
}
|
||||
|
||||
@AfterTemplate
|
||||
@UseImportPolicy(STATIC_IMPORT_ALWAYS)
|
||||
void after(int index, int size, String message) {
|
||||
checkElementIndex(index, size, message);
|
||||
}
|
||||
}
|
||||
|
||||
/** Prefer {@link Preconditions#checkNotNull(Object)} over more verbose alternatives. */
|
||||
static final class CheckNotNull<T> {
|
||||
@BeforeTemplate
|
||||
void before(T object) {
|
||||
if (object == null) {
|
||||
throw new NullPointerException();
|
||||
}
|
||||
}
|
||||
|
||||
@AfterTemplate
|
||||
@UseImportPolicy(STATIC_IMPORT_ALWAYS)
|
||||
void after(T object) {
|
||||
checkNotNull(object);
|
||||
}
|
||||
}
|
||||
|
||||
/** Prefer {@link Preconditions#checkNotNull(Object, Object)} over more verbose alternatives. */
|
||||
static final class CheckNotNullWithMessage<T> {
|
||||
@BeforeTemplate
|
||||
void before(T object, String message) {
|
||||
if (object == null) {
|
||||
throw new NullPointerException(message);
|
||||
}
|
||||
}
|
||||
|
||||
@AfterTemplate
|
||||
@UseImportPolicy(STATIC_IMPORT_ALWAYS)
|
||||
void after(T object, String message) {
|
||||
checkNotNull(object, message);
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* Prefer {@link Preconditions#checkPositionIndex(int, int)} over less descriptive or more verbose
|
||||
* alternatives.
|
||||
*/
|
||||
static final class CheckPositionIndex {
|
||||
@BeforeTemplate
|
||||
void before(int index, int size) {
|
||||
if (index < 0 || index > size) {
|
||||
throw new IndexOutOfBoundsException();
|
||||
}
|
||||
}
|
||||
|
||||
@AfterTemplate
|
||||
@UseImportPolicy(STATIC_IMPORT_ALWAYS)
|
||||
void after(int index, int size) {
|
||||
checkPositionIndex(index, size);
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* Prefer {@link Preconditions#checkPositionIndex(int, int, String)} over less descriptive or more
|
||||
* verbose alternatives.
|
||||
*/
|
||||
static final class CheckPositionIndexWithMessage {
|
||||
@BeforeTemplate
|
||||
void before(int index, int size, String message) {
|
||||
if (index < 0 || index > size) {
|
||||
throw new IndexOutOfBoundsException(message);
|
||||
}
|
||||
}
|
||||
|
||||
@AfterTemplate
|
||||
@UseImportPolicy(STATIC_IMPORT_ALWAYS)
|
||||
void after(int index, int size, String message) {
|
||||
checkPositionIndex(index, size, message);
|
||||
}
|
||||
}
|
||||
|
||||
/** Prefer {@link Preconditions#checkState(boolean)} over more verbose alternatives. */
|
||||
static final class CheckState {
|
||||
@BeforeTemplate
|
||||
void before(boolean condition) {
|
||||
if (condition) {
|
||||
throw new IllegalStateException();
|
||||
}
|
||||
}
|
||||
|
||||
@AfterTemplate
|
||||
@UseImportPolicy(STATIC_IMPORT_ALWAYS)
|
||||
void after(boolean condition) {
|
||||
checkState(!condition);
|
||||
}
|
||||
}
|
||||
|
||||
/** Prefer {@link Preconditions#checkState(boolean, Object)} over more verbose alternatives. */
|
||||
static final class CheckStateWithMessage {
|
||||
@BeforeTemplate
|
||||
void before(boolean condition, String message) {
|
||||
if (condition) {
|
||||
throw new IllegalStateException(message);
|
||||
}
|
||||
}
|
||||
|
||||
@AfterTemplate
|
||||
@UseImportPolicy(STATIC_IMPORT_ALWAYS)
|
||||
void after(boolean condition, String message) {
|
||||
checkState(!condition, message);
|
||||
}
|
||||
}
|
||||
}
|
||||
@@ -1,12 +1,14 @@
|
||||
package tech.picnic.errorprone.refastertemplates;
|
||||
package tech.picnic.errorprone.refasterrules;
|
||||
|
||||
import com.google.common.primitives.Ints;
|
||||
import com.google.errorprone.refaster.annotation.AfterTemplate;
|
||||
import com.google.errorprone.refaster.annotation.BeforeTemplate;
|
||||
import tech.picnic.errorprone.refaster.annotation.OnlineDocumentation;
|
||||
|
||||
/** Refaster templates related to expressions dealing with primitives. */
|
||||
final class PrimitiveTemplates {
|
||||
private PrimitiveTemplates() {}
|
||||
/** Refaster rules related to expressions dealing with primitives. */
|
||||
@OnlineDocumentation
|
||||
final class PrimitiveRules {
|
||||
private PrimitiveRules() {}
|
||||
|
||||
/** Avoid contrived ways of expressing the "less than" relationship. */
|
||||
static final class LessThan {
|
||||
File diff suppressed because it is too large
Load Diff
@@ -1,4 +1,4 @@
|
||||
package tech.picnic.errorprone.refastertemplates;
|
||||
package tech.picnic.errorprone.refasterrules;
|
||||
|
||||
import com.google.errorprone.refaster.Refaster;
|
||||
import com.google.errorprone.refaster.annotation.AfterTemplate;
|
||||
@@ -13,10 +13,12 @@ import org.jspecify.nullness.Nullable;
|
||||
import reactor.adapter.rxjava.RxJava2Adapter;
|
||||
import reactor.core.publisher.Flux;
|
||||
import reactor.core.publisher.Mono;
|
||||
import tech.picnic.errorprone.refaster.annotation.OnlineDocumentation;
|
||||
|
||||
/** Refaster templates related to expressions dealing with {@link RxJava2Adapter}. */
|
||||
final class RxJava2AdapterTemplates {
|
||||
private RxJava2AdapterTemplates() {}
|
||||
/** Refaster rules related to expressions dealing with {@link RxJava2Adapter}. */
|
||||
@OnlineDocumentation
|
||||
final class RxJava2AdapterRules {
|
||||
private RxJava2AdapterRules() {}
|
||||
|
||||
/** Use the fluent API style when using {@link RxJava2Adapter#completableToMono}. */
|
||||
static final class CompletableToMono {
|
||||
@@ -1,4 +1,4 @@
|
||||
package tech.picnic.errorprone.refastertemplates;
|
||||
package tech.picnic.errorprone.refasterrules;
|
||||
|
||||
import static com.google.errorprone.refaster.ImportPolicy.STATIC_IMPORT_ALWAYS;
|
||||
import static java.util.Comparator.naturalOrder;
|
||||
@@ -22,10 +22,12 @@ import java.util.function.Predicate;
|
||||
import java.util.stream.Collector;
|
||||
import java.util.stream.Collectors;
|
||||
import java.util.stream.Stream;
|
||||
import tech.picnic.errorprone.refaster.annotation.OnlineDocumentation;
|
||||
|
||||
/** Refaster templates related to expressions dealing with {@link Stream}s. */
|
||||
final class StreamTemplates {
|
||||
private StreamTemplates() {}
|
||||
/** Refaster rules related to expressions dealing with {@link Stream}s. */
|
||||
@OnlineDocumentation
|
||||
final class StreamRules {
|
||||
private StreamRules() {}
|
||||
|
||||
/**
|
||||
* Prefer {@link Collectors#joining()} over {@link Collectors#joining(CharSequence)} with an empty
|
||||
@@ -1,4 +1,4 @@
|
||||
package tech.picnic.errorprone.refastertemplates;
|
||||
package tech.picnic.errorprone.refasterrules;
|
||||
|
||||
import static java.nio.charset.StandardCharsets.UTF_8;
|
||||
import static java.util.stream.Collectors.joining;
|
||||
@@ -16,12 +16,14 @@ import java.util.Collection;
|
||||
import java.util.Objects;
|
||||
import java.util.Optional;
|
||||
import java.util.function.Function;
|
||||
import javax.annotation.Nullable;
|
||||
import org.jspecify.nullness.Nullable;
|
||||
import tech.picnic.errorprone.refaster.annotation.OnlineDocumentation;
|
||||
|
||||
/** Refaster templates related to expressions dealing with {@link String}s. */
|
||||
/** Refaster rules related to expressions dealing with {@link String}s. */
|
||||
// XXX: Should we prefer `s -> !s.isEmpty()` or `not(String::isEmpty)`?
|
||||
final class StringTemplates {
|
||||
private StringTemplates() {}
|
||||
@OnlineDocumentation
|
||||
final class StringRules {
|
||||
private StringRules() {}
|
||||
|
||||
/** Prefer {@link String#isEmpty()} over alternatives that consult the string's length. */
|
||||
static final class StringIsEmpty {
|
||||
@@ -128,8 +130,8 @@ final class StringTemplates {
|
||||
* Prefer direct delegation to {@link String#valueOf(Object)} over the indirection introduced by
|
||||
* {@link Objects#toString(Object)}.
|
||||
*/
|
||||
// XXX: This template is analogous to `StringValueOf` above. Arguably this is its generalization.
|
||||
// If/when Refaster is extended to understand this, delete the template above.
|
||||
// XXX: This rule is analogous to `StringValueOf` above. Arguably this is its generalization.
|
||||
// If/when Refaster is extended to understand this, delete the rule above.
|
||||
static final class StringValueOfMethodReference {
|
||||
@BeforeTemplate
|
||||
Function<Object, String> before() {
|
||||
@@ -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;
|
||||
@@ -30,7 +30,7 @@ import org.testng.Assert;
|
||||
import org.testng.Assert.ThrowingRunnable;
|
||||
|
||||
/**
|
||||
* Refaster templates that replace TestNG assertions with equivalent AssertJ assertions.
|
||||
* Refaster rules that replace TestNG assertions with equivalent AssertJ assertions.
|
||||
*
|
||||
* <p>Some of the classes below have TestNG {@code @BeforeTemplate}s that reference wildcard type
|
||||
* bounds ({@code <?>}), while the associated AssertJ {@code @AfterTemplate}s reference stricter
|
||||
@@ -69,11 +69,11 @@ import org.testng.Assert.ThrowingRunnable;
|
||||
* </ul>
|
||||
* </ul>
|
||||
*/
|
||||
// XXX: As-is these templates do not result in a complete migration:
|
||||
// XXX: As-is these rules do not result in a complete migration:
|
||||
// - Expressions containing comments are skipped due to a limitation of Refaster.
|
||||
// - Assertions inside lambda expressions are also skipped. Unclear why.
|
||||
final class TestNGToAssertJTemplates {
|
||||
private TestNGToAssertJTemplates() {}
|
||||
final class TestNGToAssertJRules {
|
||||
private TestNGToAssertJRules() {}
|
||||
|
||||
static final class Fail {
|
||||
@BeforeTemplate
|
||||
@@ -1,4 +1,4 @@
|
||||
package tech.picnic.errorprone.refastertemplates;
|
||||
package tech.picnic.errorprone.refasterrules;
|
||||
|
||||
import static java.time.ZoneOffset.UTC;
|
||||
|
||||
@@ -21,10 +21,12 @@ import java.time.chrono.ChronoLocalDateTime;
|
||||
import java.time.chrono.ChronoZonedDateTime;
|
||||
import java.time.temporal.ChronoUnit;
|
||||
import java.time.temporal.TemporalUnit;
|
||||
import tech.picnic.errorprone.refaster.annotation.OnlineDocumentation;
|
||||
|
||||
/** Refaster templates related to expressions dealing with time. */
|
||||
final class TimeTemplates {
|
||||
private TimeTemplates() {}
|
||||
/** Refaster rules related to expressions dealing with time. */
|
||||
@OnlineDocumentation
|
||||
final class TimeRules {
|
||||
private TimeRules() {}
|
||||
|
||||
/**
|
||||
* Prefer {@link Clock#instant()} over {@link Instant#now(Clock)}, as it is more concise and more
|
||||
@@ -1,4 +1,4 @@
|
||||
package tech.picnic.errorprone.refastertemplates;
|
||||
package tech.picnic.errorprone.refasterrules;
|
||||
|
||||
import static org.springframework.http.HttpMethod.GET;
|
||||
import static org.springframework.http.HttpMethod.HEAD;
|
||||
@@ -20,13 +20,15 @@ import org.springframework.web.reactive.function.client.WebClient.RequestBodySpe
|
||||
import org.springframework.web.reactive.function.client.WebClient.RequestBodyUriSpec;
|
||||
import org.springframework.web.reactive.function.client.WebClient.RequestHeadersSpec;
|
||||
import org.springframework.web.reactive.function.client.WebClient.RequestHeadersUriSpec;
|
||||
import tech.picnic.errorprone.refaster.annotation.OnlineDocumentation;
|
||||
|
||||
/**
|
||||
* Refaster templates related to expressions dealing with {@link
|
||||
* Refaster rules related to expressions dealing with {@link
|
||||
* org.springframework.web.reactive.function.client.WebClient} and related types.
|
||||
*/
|
||||
final class WebClientTemplates {
|
||||
private WebClientTemplates() {}
|
||||
@OnlineDocumentation
|
||||
final class WebClientRules {
|
||||
private WebClientRules() {}
|
||||
|
||||
/** Prefer {@link RequestBodySpec#bodyValue(Object)} over more contrived alternatives. */
|
||||
static final class BodyValue<T> {
|
||||
@@ -0,0 +1,4 @@
|
||||
/** Picnic Refaster rules. */
|
||||
@com.google.errorprone.annotations.CheckReturnValue
|
||||
@org.jspecify.nullness.NullMarked
|
||||
package tech.picnic.errorprone.refasterrules;
|
||||
@@ -1,443 +0,0 @@
|
||||
package tech.picnic.errorprone.refastertemplates;
|
||||
|
||||
import static com.google.common.collect.MoreCollectors.toOptional;
|
||||
import static com.google.errorprone.refaster.ImportPolicy.STATIC_IMPORT_ALWAYS;
|
||||
import static org.assertj.core.api.Assertions.assertThat;
|
||||
|
||||
import com.google.common.collect.MoreCollectors;
|
||||
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.MayOptionallyUse;
|
||||
import com.google.errorprone.refaster.annotation.NotMatches;
|
||||
import com.google.errorprone.refaster.annotation.Placeholder;
|
||||
import com.google.errorprone.refaster.annotation.UseImportPolicy;
|
||||
import java.time.Duration;
|
||||
import java.util.Optional;
|
||||
import java.util.concurrent.Callable;
|
||||
import java.util.function.Consumer;
|
||||
import java.util.function.Function;
|
||||
import java.util.function.Predicate;
|
||||
import java.util.function.Supplier;
|
||||
import org.reactivestreams.Publisher;
|
||||
import reactor.core.publisher.Flux;
|
||||
import reactor.core.publisher.Mono;
|
||||
import reactor.test.StepVerifier;
|
||||
import reactor.test.publisher.PublisherProbe;
|
||||
import tech.picnic.errorprone.refaster.matchers.ThrowsCheckedException;
|
||||
|
||||
/** Refaster templates related to Reactor expressions and statements. */
|
||||
final class ReactorTemplates {
|
||||
private ReactorTemplates() {}
|
||||
|
||||
/**
|
||||
* Prefer {@link Mono#fromSupplier(Supplier)} over {@link Mono#fromCallable(Callable)} where
|
||||
* feasible.
|
||||
*/
|
||||
static final class MonoFromSupplier<T> {
|
||||
@BeforeTemplate
|
||||
Mono<T> before(@NotMatches(ThrowsCheckedException.class) Callable<? extends T> supplier) {
|
||||
return Mono.fromCallable(supplier);
|
||||
}
|
||||
|
||||
@AfterTemplate
|
||||
Mono<T> after(Supplier<? extends T> supplier) {
|
||||
return Mono.fromSupplier(supplier);
|
||||
}
|
||||
}
|
||||
|
||||
/** Prefer {@link Mono#justOrEmpty(Optional)} over more verbose alternatives. */
|
||||
// XXX: If `optional` is a constant and effectively-final expression then the `Mono.defer` can be
|
||||
// dropped. Should look into Refaster support for identifying this.
|
||||
static final class MonoFromOptional<T> {
|
||||
@BeforeTemplate
|
||||
@SuppressWarnings(
|
||||
"MonoFromSupplier" /* `optional` may match a checked exception-throwing expression. */)
|
||||
Mono<T> before(Optional<T> optional) {
|
||||
return Refaster.anyOf(
|
||||
Mono.fromCallable(() -> optional.orElse(null)),
|
||||
Mono.fromSupplier(() -> optional.orElse(null)));
|
||||
}
|
||||
|
||||
@AfterTemplate
|
||||
Mono<T> after(Optional<T> optional) {
|
||||
return Mono.defer(() -> Mono.justOrEmpty(optional));
|
||||
}
|
||||
}
|
||||
|
||||
/** Don't unnecessarily defer {@link Mono#error(Throwable)}. */
|
||||
static final class MonoDeferredError<T> {
|
||||
@BeforeTemplate
|
||||
Mono<T> before(Throwable throwable) {
|
||||
return Mono.defer(() -> Mono.error(throwable));
|
||||
}
|
||||
|
||||
@AfterTemplate
|
||||
Mono<T> after(Throwable throwable) {
|
||||
return Mono.error(() -> throwable);
|
||||
}
|
||||
}
|
||||
|
||||
/** Don't unnecessarily defer {@link Flux#error(Throwable)}. */
|
||||
static final class FluxDeferredError<T> {
|
||||
@BeforeTemplate
|
||||
Flux<T> before(Throwable throwable) {
|
||||
return Flux.defer(() -> Flux.error(throwable));
|
||||
}
|
||||
|
||||
@AfterTemplate
|
||||
Flux<T> after(Throwable throwable) {
|
||||
return Flux.error(() -> throwable);
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* Don't unnecessarily pass {@link Mono#error(Supplier)} a method reference or lambda expression.
|
||||
*/
|
||||
// XXX: Drop this rule once the more general rule `AssortedTemplates#SupplierAsSupplier` works
|
||||
// reliably.
|
||||
static final class MonoErrorSupplier<T, E extends Throwable> {
|
||||
@BeforeTemplate
|
||||
Mono<T> before(Supplier<E> supplier) {
|
||||
return Mono.error(() -> supplier.get());
|
||||
}
|
||||
|
||||
@AfterTemplate
|
||||
Mono<T> after(Supplier<E> supplier) {
|
||||
return Mono.error(supplier);
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* Don't unnecessarily pass {@link Flux#error(Supplier)} a method reference or lambda expression.
|
||||
*/
|
||||
// XXX: Drop this rule once the more general rule `AssortedTemplates#SupplierAsSupplier` works
|
||||
// reliably.
|
||||
static final class FluxErrorSupplier<T, E extends Throwable> {
|
||||
@BeforeTemplate
|
||||
Flux<T> before(Supplier<E> supplier) {
|
||||
return Flux.error(() -> supplier.get());
|
||||
}
|
||||
|
||||
@AfterTemplate
|
||||
Flux<T> after(Supplier<E> supplier) {
|
||||
return Flux.error(supplier);
|
||||
}
|
||||
}
|
||||
|
||||
/** Prefer {@link Mono#thenReturn(Object)} over more verbose alternatives. */
|
||||
static final class MonoThenReturn<T, S> {
|
||||
@BeforeTemplate
|
||||
Mono<S> before(Mono<T> mono, S object) {
|
||||
return mono.then(Mono.just(object));
|
||||
}
|
||||
|
||||
@AfterTemplate
|
||||
Mono<S> after(Mono<T> mono, S object) {
|
||||
return mono.thenReturn(object);
|
||||
}
|
||||
}
|
||||
|
||||
/** Don't unnecessarily pass an empty publisher to {@link Mono#switchIfEmpty(Mono)}. */
|
||||
static final class MonoSwitchIfEmptyOfEmptyPublisher<T> {
|
||||
@BeforeTemplate
|
||||
Mono<T> before(Mono<T> mono) {
|
||||
return mono.switchIfEmpty(Mono.empty());
|
||||
}
|
||||
|
||||
@AfterTemplate
|
||||
Mono<T> after(Mono<T> mono) {
|
||||
return mono;
|
||||
}
|
||||
}
|
||||
|
||||
/** Don't unnecessarily pass an empty publisher to {@link Flux#switchIfEmpty(Publisher)}. */
|
||||
static final class FluxSwitchIfEmptyOfEmptyPublisher<T> {
|
||||
@BeforeTemplate
|
||||
Flux<T> before(Flux<T> flux) {
|
||||
return flux.switchIfEmpty(Refaster.anyOf(Mono.empty(), Flux.empty()));
|
||||
}
|
||||
|
||||
@AfterTemplate
|
||||
Flux<T> after(Flux<T> flux) {
|
||||
return flux;
|
||||
}
|
||||
}
|
||||
|
||||
/** Prefer {@link Flux#concatMap(Function)} over more contrived alternatives. */
|
||||
static final class FluxConcatMap<T, S> {
|
||||
@BeforeTemplate
|
||||
Flux<S> before(Flux<T> flux, Function<? super T, ? extends Publisher<? extends S>> function) {
|
||||
return Refaster.anyOf(flux.flatMap(function, 1), flux.flatMapSequential(function, 1));
|
||||
}
|
||||
|
||||
@AfterTemplate
|
||||
Flux<S> after(Flux<T> flux, Function<? super T, ? extends Publisher<? extends S>> function) {
|
||||
return flux.concatMap(function);
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* Prefer {@link Flux#concatMapIterable(Function)} over {@link Flux#flatMapIterable(Function)}, as
|
||||
* the former has equivalent semantics but a clearer name.
|
||||
*/
|
||||
static final class FluxConcatMapIterable<T, S> {
|
||||
@BeforeTemplate
|
||||
Flux<S> before(Flux<T> flux, Function<? super T, ? extends Iterable<? extends S>> function) {
|
||||
return flux.flatMapIterable(function);
|
||||
}
|
||||
|
||||
@AfterTemplate
|
||||
Flux<S> after(Flux<T> flux, Function<? super T, ? extends Iterable<? extends S>> function) {
|
||||
return flux.concatMapIterable(function);
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* Don't use {@link Mono#flatMapMany(Function)} to implicitly convert a {@link Mono} to a {@link
|
||||
* Flux}.
|
||||
*/
|
||||
abstract static class MonoFlatMapToFlux<T, S> {
|
||||
@Placeholder(allowsIdentity = true)
|
||||
abstract Mono<S> valueTransformation(@MayOptionallyUse T value);
|
||||
|
||||
@BeforeTemplate
|
||||
Flux<S> before(Mono<T> mono) {
|
||||
return mono.flatMapMany(v -> valueTransformation(v));
|
||||
}
|
||||
|
||||
@AfterTemplate
|
||||
Flux<S> after(Mono<T> mono) {
|
||||
return mono.flatMap(v -> valueTransformation(v)).flux();
|
||||
}
|
||||
}
|
||||
|
||||
/** Prefer {@link Mono#flux()}} over more contrived alternatives. */
|
||||
static final class MonoFlux<T> {
|
||||
@BeforeTemplate
|
||||
Flux<T> before(Mono<T> mono) {
|
||||
return Flux.concat(mono);
|
||||
}
|
||||
|
||||
@AfterTemplate
|
||||
Flux<T> after(Mono<T> mono) {
|
||||
return mono.flux();
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* Prefer a collection using {@link MoreCollectors#toOptional()} over more contrived alternatives.
|
||||
*/
|
||||
// XXX: Consider creating a plugin that flags/discourages `Mono<Optional<T>>` method return
|
||||
// types, just as we discourage nullable `Boolean`s and `Optional`s.
|
||||
static final class MonoCollectToOptional<T> {
|
||||
@BeforeTemplate
|
||||
Mono<Optional<T>> before(Mono<T> mono) {
|
||||
return Refaster.anyOf(
|
||||
mono.map(Optional::of).defaultIfEmpty(Optional.empty()),
|
||||
mono.map(Optional::of).switchIfEmpty(Mono.just(Optional.empty())));
|
||||
}
|
||||
|
||||
@AfterTemplate
|
||||
@UseImportPolicy(STATIC_IMPORT_ALWAYS)
|
||||
Mono<Optional<T>> after(Mono<T> mono) {
|
||||
return mono.flux().collect(toOptional());
|
||||
}
|
||||
}
|
||||
|
||||
/** Prefer {@link Mono#cast(Class)} over {@link Mono#map(Function)} with a cast. */
|
||||
static final class MonoCast<T, S> {
|
||||
@BeforeTemplate
|
||||
Mono<S> before(Mono<T> mono) {
|
||||
return mono.map(Refaster.<S>clazz()::cast);
|
||||
}
|
||||
|
||||
@AfterTemplate
|
||||
Mono<S> after(Mono<T> mono) {
|
||||
return mono.cast(Refaster.<S>clazz());
|
||||
}
|
||||
}
|
||||
|
||||
/** Prefer {@link Flux#cast(Class)} over {@link Flux#map(Function)} with a cast. */
|
||||
static final class FluxCast<T, S> {
|
||||
@BeforeTemplate
|
||||
Flux<S> before(Flux<T> flux) {
|
||||
return flux.map(Refaster.<S>clazz()::cast);
|
||||
}
|
||||
|
||||
@AfterTemplate
|
||||
Flux<S> after(Flux<T> flux) {
|
||||
return flux.cast(Refaster.<S>clazz());
|
||||
}
|
||||
}
|
||||
|
||||
/** Prefer {@link PublisherProbe#empty()}} over more verbose alternatives. */
|
||||
static final class PublisherProbeEmpty<T> {
|
||||
@BeforeTemplate
|
||||
PublisherProbe<T> before() {
|
||||
return Refaster.anyOf(PublisherProbe.of(Mono.empty()), PublisherProbe.of(Flux.empty()));
|
||||
}
|
||||
|
||||
@AfterTemplate
|
||||
PublisherProbe<T> after() {
|
||||
return PublisherProbe.empty();
|
||||
}
|
||||
}
|
||||
|
||||
/** Prefer {@link Mono#as(Function)} when creating a {@link StepVerifier}. */
|
||||
static final class StepVerifierFromMono<T> {
|
||||
@BeforeTemplate
|
||||
StepVerifier.FirstStep<? extends T> before(Mono<T> mono) {
|
||||
return StepVerifier.create(mono);
|
||||
}
|
||||
|
||||
@AfterTemplate
|
||||
StepVerifier.FirstStep<? extends T> after(Mono<T> mono) {
|
||||
return mono.as(StepVerifier::create);
|
||||
}
|
||||
}
|
||||
|
||||
/** Prefer {@link Flux#as(Function)} when creating a {@link StepVerifier}. */
|
||||
static final class StepVerifierFromFlux<T> {
|
||||
@BeforeTemplate
|
||||
StepVerifier.FirstStep<? extends T> before(Flux<T> flux) {
|
||||
return StepVerifier.create(flux);
|
||||
}
|
||||
|
||||
@AfterTemplate
|
||||
StepVerifier.FirstStep<? extends T> after(Flux<T> flux) {
|
||||
return flux.as(StepVerifier::create);
|
||||
}
|
||||
}
|
||||
|
||||
/** Don't unnecessarily call {@link StepVerifier.Step#expectNext(Object[])}. */
|
||||
static final class StepVerifierStepExpectNextEmpty<T> {
|
||||
@BeforeTemplate
|
||||
@SuppressWarnings("unchecked")
|
||||
StepVerifier.Step<T> before(StepVerifier.Step<T> step) {
|
||||
return step.expectNext();
|
||||
}
|
||||
|
||||
@AfterTemplate
|
||||
StepVerifier.Step<T> after(StepVerifier.Step<T> step) {
|
||||
return step;
|
||||
}
|
||||
}
|
||||
|
||||
/** Prefer {@link StepVerifier.Step#expectNext(Object)} over more verbose alternatives. */
|
||||
static final class StepVerifierStepExpectNext<T> {
|
||||
@BeforeTemplate
|
||||
StepVerifier.Step<T> before(StepVerifier.Step<T> step, T object) {
|
||||
return Refaster.anyOf(
|
||||
step.expectNextMatches(e -> e.equals(object)), step.expectNextMatches(object::equals));
|
||||
}
|
||||
|
||||
@AfterTemplate
|
||||
StepVerifier.Step<T> after(StepVerifier.Step<T> step, T object) {
|
||||
return step.expectNext(object);
|
||||
}
|
||||
}
|
||||
|
||||
/** Prefer {@link StepVerifier.LastStep#verifyComplete()} over more verbose alternatives. */
|
||||
static final class StepVerifierLastStepVerifyComplete {
|
||||
@BeforeTemplate
|
||||
Duration before(StepVerifier.LastStep step) {
|
||||
return step.expectComplete().verify();
|
||||
}
|
||||
|
||||
@AfterTemplate
|
||||
Duration after(StepVerifier.LastStep step) {
|
||||
return step.verifyComplete();
|
||||
}
|
||||
}
|
||||
|
||||
/** Prefer {@link StepVerifier.LastStep#verifyError()} over more verbose alternatives. */
|
||||
static final class StepVerifierLastStepVerifyError {
|
||||
@BeforeTemplate
|
||||
Duration before(StepVerifier.LastStep step) {
|
||||
return step.expectError().verify();
|
||||
}
|
||||
|
||||
@AfterTemplate
|
||||
Duration after(StepVerifier.LastStep step) {
|
||||
return step.verifyError();
|
||||
}
|
||||
}
|
||||
|
||||
/** Prefer {@link StepVerifier.LastStep#verifyError(Class)} over more verbose alternatives. */
|
||||
static final class StepVerifierLastStepVerifyErrorClass<T extends Throwable> {
|
||||
@BeforeTemplate
|
||||
Duration before(StepVerifier.LastStep step, Class<T> clazz) {
|
||||
return Refaster.anyOf(
|
||||
step.expectError(clazz).verify(),
|
||||
step.verifyErrorSatisfies(t -> assertThat(t).isInstanceOf(clazz)));
|
||||
}
|
||||
|
||||
@AfterTemplate
|
||||
Duration after(StepVerifier.LastStep step, Class<T> clazz) {
|
||||
return step.verifyError(clazz);
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* Prefer {@link StepVerifier.LastStep#verifyErrorMatches(Predicate)} over more verbose
|
||||
* alternatives.
|
||||
*/
|
||||
static final class StepVerifierLastStepVerifyErrorMatches {
|
||||
@BeforeTemplate
|
||||
Duration before(StepVerifier.LastStep step, Predicate<Throwable> predicate) {
|
||||
return step.expectErrorMatches(predicate).verify();
|
||||
}
|
||||
|
||||
@AfterTemplate
|
||||
Duration after(StepVerifier.LastStep step, Predicate<Throwable> predicate) {
|
||||
return step.verifyErrorMatches(predicate);
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* Prefer {@link StepVerifier.LastStep#verifyErrorSatisfies(Consumer)} over more verbose
|
||||
* alternatives.
|
||||
*/
|
||||
static final class StepVerifierLastStepVerifyErrorSatisfies {
|
||||
@BeforeTemplate
|
||||
Duration before(StepVerifier.LastStep step, Consumer<Throwable> consumer) {
|
||||
return step.expectErrorSatisfies(consumer).verify();
|
||||
}
|
||||
|
||||
@AfterTemplate
|
||||
Duration after(StepVerifier.LastStep step, Consumer<Throwable> consumer) {
|
||||
return step.verifyErrorSatisfies(consumer);
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* Prefer {@link StepVerifier.LastStep#verifyErrorMessage(String)} over more verbose alternatives.
|
||||
*/
|
||||
static final class StepVerifierLastStepVerifyErrorMessage {
|
||||
@BeforeTemplate
|
||||
Duration before(StepVerifier.LastStep step, String message) {
|
||||
return step.expectErrorMessage(message).verify();
|
||||
}
|
||||
|
||||
@AfterTemplate
|
||||
Duration after(StepVerifier.LastStep step, String message) {
|
||||
return step.verifyErrorMessage(message);
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* Prefer {@link StepVerifier.LastStep#verifyTimeout(Duration)} over more verbose alternatives.
|
||||
*/
|
||||
static final class StepVerifierLastStepVerifyTimeout {
|
||||
@BeforeTemplate
|
||||
Duration before(StepVerifier.LastStep step, Duration duration) {
|
||||
return step.expectTimeout(duration).verify();
|
||||
}
|
||||
|
||||
@AfterTemplate
|
||||
Duration after(StepVerifier.LastStep step, Duration duration) {
|
||||
return step.verifyTimeout(duration);
|
||||
}
|
||||
}
|
||||
}
|
||||
@@ -1,4 +0,0 @@
|
||||
/** Picnic Refaster templates. */
|
||||
@com.google.errorprone.annotations.CheckReturnValue
|
||||
@javax.annotation.ParametersAreNonnullByDefault
|
||||
package tech.picnic.errorprone.refastertemplates;
|
||||
@@ -65,6 +65,23 @@ final class CollectorMutabilityTest {
|
||||
.doTest();
|
||||
}
|
||||
|
||||
@Test
|
||||
void identificationWithoutGuavaOnClasspath() {
|
||||
compilationTestHelper
|
||||
.withClasspath()
|
||||
.addSourceLines(
|
||||
"A.java",
|
||||
"import java.util.stream.Collectors;",
|
||||
"import java.util.stream.Stream;",
|
||||
"",
|
||||
"class A {",
|
||||
" void m() {",
|
||||
" Stream.empty().collect(Collectors.toList());",
|
||||
" }",
|
||||
"}")
|
||||
.doTest();
|
||||
}
|
||||
|
||||
@Test
|
||||
void replacementFirstSuggestedFix() {
|
||||
refactoringTestHelper
|
||||
|
||||
@@ -4,6 +4,7 @@ import static com.google.errorprone.BugCheckerRefactoringTestHelper.newInstance;
|
||||
|
||||
import com.google.errorprone.BugCheckerRefactoringTestHelper;
|
||||
import com.google.errorprone.BugCheckerRefactoringTestHelper.FixChoosers;
|
||||
import com.google.errorprone.BugCheckerRefactoringTestHelper.TestMode;
|
||||
import com.google.errorprone.CompilationTestHelper;
|
||||
import org.junit.jupiter.api.Test;
|
||||
|
||||
@@ -33,6 +34,14 @@ final class FluxFlatMapUsageTest {
|
||||
" Flux.just(1).flatMapSequential(Flux::just);",
|
||||
" // BUG: Diagnostic contains:",
|
||||
" Flux.just(1).<String>flatMapSequential(i -> Flux.just(String.valueOf(i)));",
|
||||
" // BUG: Diagnostic contains:",
|
||||
" Flux.just(1, 2).groupBy(i -> i).flatMap(Flux::just);",
|
||||
" // BUG: Diagnostic contains:",
|
||||
" Flux.just(1, 2).groupBy(i -> i).<String>flatMap(i -> Flux.just(String.valueOf(i)));",
|
||||
" // BUG: Diagnostic contains:",
|
||||
" Flux.just(1, 2).groupBy(i -> i).flatMapSequential(Flux::just);",
|
||||
" // BUG: Diagnostic contains:",
|
||||
" Flux.just(1, 2).groupBy(i -> i).<String>flatMapSequential(i -> Flux.just(String.valueOf(i)));",
|
||||
"",
|
||||
" Mono.just(1).flatMap(Mono::just);",
|
||||
" Flux.just(1).concatMap(Flux::just);",
|
||||
@@ -71,9 +80,13 @@ final class FluxFlatMapUsageTest {
|
||||
"import reactor.core.publisher.Flux;",
|
||||
"",
|
||||
"class A {",
|
||||
" private static final int MAX_CONCURRENCY = 8;",
|
||||
"",
|
||||
" void m() {",
|
||||
" Flux.just(1).flatMap(Flux::just);",
|
||||
" Flux.just(1).flatMapSequential(Flux::just);",
|
||||
" Flux.just(1, 2).groupBy(i -> i).flatMap(Flux::just);",
|
||||
" Flux.just(1, 2).groupBy(i -> i).flatMapSequential(Flux::just);",
|
||||
" }",
|
||||
"}")
|
||||
.addOutputLines(
|
||||
@@ -81,12 +94,16 @@ final class FluxFlatMapUsageTest {
|
||||
"import reactor.core.publisher.Flux;",
|
||||
"",
|
||||
"class A {",
|
||||
" private static final int MAX_CONCURRENCY = 8;",
|
||||
"",
|
||||
" void m() {",
|
||||
" Flux.just(1).concatMap(Flux::just);",
|
||||
" Flux.just(1).concatMap(Flux::just);",
|
||||
" Flux.just(1, 2).groupBy(i -> i).flatMap(Flux::just, MAX_CONCURRENCY);",
|
||||
" Flux.just(1, 2).groupBy(i -> i).flatMapSequential(Flux::just, MAX_CONCURRENCY);",
|
||||
" }",
|
||||
"}")
|
||||
.doTest();
|
||||
.doTest(TestMode.TEXT_MATCH);
|
||||
}
|
||||
|
||||
@Test
|
||||
@@ -103,6 +120,8 @@ final class FluxFlatMapUsageTest {
|
||||
" void m() {",
|
||||
" Flux.just(1).flatMap(Flux::just);",
|
||||
" Flux.just(1).flatMapSequential(Flux::just);",
|
||||
" Flux.just(1, 2).groupBy(i -> i).flatMap(Flux::just);",
|
||||
" Flux.just(1, 2).groupBy(i -> i).flatMapSequential(Flux::just);",
|
||||
" }",
|
||||
"}")
|
||||
.addOutputLines(
|
||||
@@ -115,8 +134,10 @@ final class FluxFlatMapUsageTest {
|
||||
" void m() {",
|
||||
" Flux.just(1).flatMap(Flux::just, MAX_CONCURRENCY);",
|
||||
" Flux.just(1).flatMapSequential(Flux::just, MAX_CONCURRENCY);",
|
||||
" Flux.just(1, 2).groupBy(i -> i).concatMap(Flux::just);",
|
||||
" Flux.just(1, 2).groupBy(i -> i).concatMap(Flux::just);",
|
||||
" }",
|
||||
"}")
|
||||
.doTest();
|
||||
.doTest(TestMode.TEXT_MATCH);
|
||||
}
|
||||
}
|
||||
|
||||
Some files were not shown because too many files have changed in this diff Show More
Reference in New Issue
Block a user