Replace occurrences of "which" with "that" in defining clauses (#259)

This commit is contained in:
Rick Ossendrijver
2022-09-25 19:09:22 +02:00
committed by GitHub
parent 7f18bd9030
commit 891fecd297
50 changed files with 137 additions and 139 deletions

View File

@@ -51,7 +51,7 @@ Some pointers:
- Checks should be _topical_: ideally they address a single concern.
- Where possible checks should provide _fixes_, and ideally these are
completely behavior-preserving. In order for a check to be adopted by users
it must not "get in the way". So for a check which addresses a relatively
it must not "get in the way". So for a check that addresses a relatively
trivial stylistic concern it is doubly important that the violations it
detects can be auto-patched.
- Make sure you have read Error Prone's [criteria for new

View File

@@ -54,7 +54,7 @@ The following is a list of checks we'd like to see implemented:
signature groups. Using Error Prone's method matchers forbidden method calls
can easily be identified. But Error Prone can go one step further by
auto-patching violations. For each violation two fixes can be proposed: a
purely behavior-preserving fix which makes the platform-dependent behavior
purely behavior-preserving fix, which makes the platform-dependent behavior
explicit, and another which replaces the platform-dependent behavior with the
preferred alternative. (Such as using `UTF-8` instead of the system default
charset.)
@@ -64,129 +64,128 @@ The following is a list of checks we'd like to see implemented:
functionality.
- A subset of the refactor operations provided by the Eclipse-specific
[AutoRefactor][autorefactor] plugin.
- A check which replaces fully qualified types with simple types in contexts
- A check that replaces fully qualified types with simple types in contexts
where this does not introduce ambiguity. Should consider both actual Java
code and Javadoc `@link` references.
- A check which simplifies array expressions. It would replace empty array
- A check that simplifies array expressions. It would replace empty array
expressions of the form `new int[] {}` with `new int[0]`. Statements of the
form `byte[] arr = new byte[] {'c'};` would be shortened to
`byte[] arr = {'c'};`.
- A check which replaces expressions of the form
`String.format("some prefix %s", arg)` with `"some prefix " + arg`, and
similar for simple suffixes. Can perhaps be generalized further, though it's
unclear how far. (Well, a `String.format` call without arguments can
certainly be simplified, too.)
- A check which replaces single-character strings with `char`s where possible.
form `byte[] arr = new byte[] {'c'};` would be shortened to `byte[] arr =
{'c'};`.
- A check that replaces expressions of the form `String.format("some prefix
%s", arg)` with `"some prefix " + arg`, and similar for simple suffixes. Can
perhaps be generalized further, though it's unclear how far. (Well, a
`String.format` call without arguments can certainly be simplified, too.)
- A check that replaces single-character strings with `char`s where possible.
For example as argument to `StringBuilder.append` and in string
concatenations.
- A check which adds or removes the first `Locale` argument to `String.format`
- A check that adds or removes the first `Locale` argument to `String.format`
and similar calls as necessary. (For example, a format string containing only
`%s` placeholders is locale-insensitive unless any of the arguments is a
`Formattable`, while `%f` placeholders _are_ locale-sensitive.)
- A check which replaces `String.replaceAll` with `String.replace` if the first
- A check that replaces `String.replaceAll` with `String.replace` if the first
argument is certainly not a regular expression. And if both arguments are
single-character strings then the `(char, char)` overload can be invoked
instead.
- A check which flags (and ideally, replaces) `try-finally` constructs with
- A check that flags (and ideally, replaces) `try-finally` constructs with
equivalent `try-with-resources` constructs.
- A check which drops exceptions declared in `throws` clauses if they are (a)
- A check that drops exceptions declared in `throws` clauses if they are (a)
not actually thrown and (b) the associated method cannot be overridden.
- A check which tries to statically import certain methods whenever used, if
- A check that tries to statically import certain methods whenever used, if
possible. The set of targeted methods should be configurable, but may default
to e.g. `java.util.Function.identity()`, the static methods exposed by
`java.util.stream.Collectors` and the various Guava collector factory
methods.
- A check which replaces `new Random().someMethod()` calls with
- A check that replaces `new Random().someMethod()` calls with
`ThreadLocalRandom.current().someMethod()` calls, to avoid unnecessary
synchronization.
- A check which drops `this.` from `this.someMethod()` calls and which
- A check that drops `this.` from `this.someMethod()` calls and which
optionally does the same for fields, if no ambiguity arises.
- A check which replaces `Integer.valueOf` calls with `Integer.parseInt` or
vice versa in order to prevent auto (un)boxing, and likewise for other number
- A check that replaces `Integer.valueOf` calls with `Integer.parseInt` or vice
versa in order to prevent auto (un)boxing, and likewise for other number
types.
- A check which flags nullable collections.
- A check which flags `AutoCloseable` resources not managed by a
- A check that flags nullable collections.
- A check that flags `AutoCloseable` resources not managed by a
`try-with-resources` construct. Certain subtypes, such as jOOQ's `DSLContext`
should be excluded.
- A check which flags `java.time` methods which implicitly consult the system
- A check that flags `java.time` methods which implicitly consult the system
clock, suggesting that a passed-in `Clock` is used instead.
- A check which flags public methods on public classes which reference
- A check that flags public methods on public classes which reference
non-public types. This can cause `IllegalAccessError`s and
`BootstrapMethodError`s at runtime.
- A check which swaps the LHS and RHS in expressions of the form
- A check that swaps the LHS and RHS in expressions of the form
`nonConstant.equals(someNonNullConstant)`.
- A check which annotates methods which only throw an exception with
- A check that annotates methods which only throw an exception with
`@Deprecated` or ` @DoNotCall`.
- A check which flags imports from other test classes.
- A Guava-specific check which replaces `Joiner.join` calls with `String.join`
- A check that flags imports from other test classes.
- A Guava-specific check that replaces `Joiner.join` calls with `String.join`
calls in those cases where the latter is a proper substitute for the former.
- A Guava-specific check which flags `{Immutable,}Multimap` type usages where
- A Guava-specific check that flags `{Immutable,}Multimap` type usages where
`{Immutable,}{List,Set}Multimap` would be more appropriate.
- A Guava-specific check which rewrites
`if (conditional) { throw new IllegalArgumentException(); }` and variants to
an equivalent `checkArgument` statement. Idem for other exception types.
- A Guava-specific check which replaces simple anonymous `CacheLoader` subclass
- A Guava-specific check that rewrites `if (conditional) { throw new
IllegalArgumentException(); }` and variants to an equivalent `checkArgument`
statement. Idem for other exception types.
- A Guava-specific check that replaces simple anonymous `CacheLoader` subclass
declarations with `CacheLoader.from(someLambda)`.
- A Spring-specific check which enforces that methods with the `@Scheduled`
- A Spring-specific check that enforces that methods with the `@Scheduled`
annotation are also annotated with New Relic's `@Trace` annotation. Such
methods should ideally not also represent Spring MVC endpoints.
- A Spring-specific check which enforces that `@RequestMapping` annotations,
- A Spring-specific check that enforces that `@RequestMapping` annotations,
when applied to a method, explicitly specify one or more target HTTP methods.
- A Spring-specific check which looks for classes in which all
`@RequestMapping` annotations (and the various aliases) specify the same
`path`/`value` property and then moves that path to the class level.
- A Spring-specific check which flags `@Value("some.property")` annotations, as
- A Spring-specific check that looks for classes in which all `@RequestMapping`
annotations (and the various aliases) specify the same `path`/`value`
property and then moves that path to the class level.
- A Spring-specific check that flags `@Value("some.property")` annotations, as
these almost certainly should be `@Value("${some.property}")`.
- A Spring-specific check which drops the `required` attribute from
- A Spring-specific check that drops the `required` attribute from
`@RequestParam` annotations when the `defaultValue` attribute is also
specified.
- A Spring-specific check which rewrites a class which uses field injection to
- A Spring-specific check that rewrites a class which uses field injection to
one which uses constructor injection. This check wouldn't be strictly
behavior preserving, but could be used for a one-off code base migration.
- A Spring-specific check which disallows field injection, except in
- A Spring-specific check that disallows field injection, except in
`AbstractTestNGSpringContextTests` subclasses. (One known edge case:
self-injections so that a bean can call itself through an implicit proxy.)
- A Spring-specific check which verifies that public methods on all classes
- A Spring-specific check that verifies that public methods on all classes
whose name matches a certain pattern, e.g. `.*Service`, are annotated
`@Secured`.
- A Spring-specific check which verifies that annotations such as
- A Spring-specific check that verifies that annotations such as
`@RequestParam` are only present in `@RestController` classes.
- A Spring-specific check which disallows `@ResponseStatus` on MVC endpoint
- A Spring-specific check that disallows `@ResponseStatus` on MVC endpoint
methods, as this prevents communication of error status codes.
- A Hibernate Validator-specific check which looks for `@UnwrapValidatedValue`
- A Hibernate Validator-specific check that looks for `@UnwrapValidatedValue`
usages and migrates the associated constraint annotations to the generic type
argument to which they (are presumed to) apply.
- A TestNG-specific check which drops method-level `@Test` annotations if a
- A TestNG-specific check that drops method-level `@Test` annotations if a
matching/more specific annotation is already present at the class level.
- A TestNG-specific check which enforces that all tests are in a group.
- A TestNG-specific check which flags field assignments in
- A TestNG-specific check that enforces that all tests are in a group.
- A TestNG-specific check that flags field assignments in
`@BeforeMethod`-annotated methods unless the class is annotated
`@Test(singleThreaded = true)`.
- A TestNG-specific check which flags usages of the `expectedExceptions`
- A TestNG-specific check that flags usages of the `expectedExceptions`
attribute of the `@Test` annotation, pointing to `assertThrows`.
- A Jongo-specific check which disallows the creation of sparse indices, in
- A Jongo-specific check that disallows the creation of sparse indices, in
favour of partial indices.
- An Immutables-specific check which replaces
- An Immutables-specific check that replaces
`checkState`/`IllegalStateException` usages inside a `@Value.Check`-annotated
method with `checkArgument`/`IllegalArgument`, since the method is invoked
when a caller attempts to create an immutable instance.
- An Immutables-specific check which disallows references to collection types
- An Immutables-specific check that disallows references to collection types
other than the Guava immutable collections, including inside generic type
arguments.
- An SLF4J-specific check which drops or adds a trailing dot from log messages,
- An SLF4J-specific check that drops or adds a trailing dot from log messages,
as applicable.
- A Mockito-specific check which identifies sequences of statements which mock
a significant number of methods on a single object with "default data"; such
- A Mockito-specific check that identifies sequences of statements which mock a
significant number of methods on a single object with "default data"; such
constructions can often benefit from a different type of default answer, such
as `Answers.RETURNS_MOCKS`.
- An RxJava-specific check which flags `.toCompletable()` calls on expressions
- An RxJava-specific check that flags `.toCompletable()` calls on expressions
of type `Single<Completable>` etc., as most likely
`.flatMapCompletable(Functions.identity())` was meant instead. Idem for other
variations.
- An RxJava-specific check which flags `expr.firstOrError()` calls and suggests
- An RxJava-specific check that flags `expr.firstOrError()` calls and suggests
`expr.switchIfEmpty(Single.error(...))`, so that an application-specific
exception is thrown instead of `NoSuchElementException`.
- An RxJava-specific check which flags use of `#assertValueSet` without
- An RxJava-specific check that flags use of `#assertValueSet` without
`#assertValueCount`, as the former method doesn't do what one may intuitively
expect it to do. See ReactiveX/RxJava#6151.
@@ -212,16 +211,16 @@ Refaster's expressiveness:
- Some Refaster refactorings (e.g. when dealing with lazy evaluation) are valid
only when some free parameter is a constant, variable reference or some other
pure expression. Introduce a way to express such a constraint. For example,
rewriting `optional1.map(Optional::of).orElse(optional2)` to
`optional1.or(() -> optional2)` is not behavior preserving if evaluation of
`optional2` has side effects.
rewriting `optional1.map(Optional::of).orElse(optional2)` to `optional1.or(()
-> optional2)` is not behavior preserving if evaluation of `optional2` has
side effects.
- Similarly, certain refactoring operations are only valid if one of the
matched expressions is not `@Nullable`. It'd be nice to be able to express
this.
- Generalize `@Placeholder` support such that rules can reference e.g. "any
concrete unary method". This would allow refactorings such as
`Mono.just(constant).flatmap(this::someFun)` ->
`Mono.defer(() -> someFun(constant))`.
`Mono.just(constant).flatmap(this::someFun)` -> `Mono.defer(() ->
someFun(constant))`.
- Sometimes a Refaster refactoring can cause the resulting code not to compile
due to a lack of generic type information. Identify and resolve such
occurrences. For example, an `@AfterTemplate` may require the insertion of a

View File

@@ -22,7 +22,7 @@ import com.sun.tools.javac.code.Symbol;
import java.util.Map;
import javax.lang.model.element.AnnotationValue;
/** A {@link BugChecker} which flags ambiguous {@code @JsonCreator}s in enums. */
/** A {@link BugChecker} that flags ambiguous {@code @JsonCreator}s in enums. */
@AutoService(BugChecker.class)
@BugPattern(
summary = "`JsonCreator.Mode` should be set for single-argument creators",

View File

@@ -21,7 +21,7 @@ import com.google.errorprone.matchers.Matcher;
import com.sun.source.tree.MethodInvocationTree;
/**
* A {@link BugChecker} which flags AssertJ {@code isEqualTo(null)} checks for simplification.
* A {@link BugChecker} that flags AssertJ {@code isEqualTo(null)} checks for simplification.
*
* <p>This bug checker cannot be replaced with a simple Refaster template, as the Refaster approach
* would require that all overloads of {@link org.assertj.core.api.Assert#isEqualTo(Object)} (such

View File

@@ -24,7 +24,7 @@ import com.sun.source.tree.MethodTree;
import com.sun.source.tree.Tree;
import java.util.List;
/** A {@link BugChecker} which flags redundant {@code @Autowired} constructor annotations. */
/** A {@link BugChecker} that flags redundant {@code @Autowired} constructor annotations. */
@AutoService(BugChecker.class)
@BugPattern(
summary = "Omit `@Autowired` on a class' sole constructor, as it is redundant",

View File

@@ -27,7 +27,7 @@ import java.util.regex.Matcher;
import java.util.regex.Pattern;
import tech.picnic.errorprone.bugpatterns.util.SourceCode;
/** A {@link BugChecker} which flags annotations that could be written more concisely. */
/** A {@link BugChecker} that flags annotations that could be written more concisely. */
@AutoService(BugChecker.class)
@BugPattern(
summary = "Omit redundant syntax from annotation declarations",

View File

@@ -19,7 +19,7 @@ import com.sun.source.tree.MethodInvocationTree;
import java.util.stream.Collector;
/**
* A {@link BugChecker} which flags {@link Collector Collectors} that don't clearly express
* A {@link BugChecker} that flags {@link Collector Collectors} that don't clearly express
* (im)mutability.
*
* <p>Replacing such collectors with alternatives that produce immutable collections is preferred.

View File

@@ -22,7 +22,7 @@ import com.sun.source.tree.MethodTree;
import com.sun.source.tree.Tree;
import java.util.Optional;
/** A {@link BugChecker} which flags empty methods that seemingly can simply be deleted. */
/** A {@link BugChecker} that flags empty methods that seemingly can simply be deleted. */
@AutoService(BugChecker.class)
@BugPattern(
summary = "Empty method can likely be deleted",

View File

@@ -30,7 +30,7 @@ import java.util.List;
import java.util.Optional;
/**
* A {@link BugChecker} which flags improperly formatted Error Prone test code.
* A {@link BugChecker} that flags improperly formatted Error Prone test code.
*
* <p>All test code should be formatted in accordance with Google Java Format's {@link Formatter}
* output, and imports should be ordered according to the {@link Style#GOOGLE Google} style.

View File

@@ -30,7 +30,7 @@ import java.util.Set;
import java.util.stream.Stream;
/**
* A {@link BugChecker} which flags {@link Ordering#explicit(Object, Object[])}} invocations listing
* A {@link BugChecker} that flags {@link Ordering#explicit(Object, Object[])}} invocations listing
* a subset of an enum type's values.
*/
@AutoService(BugChecker.class)

View File

@@ -24,7 +24,7 @@ import java.util.function.Supplier;
import reactor.core.publisher.Flux;
/**
* A {@link BugChecker} which flags usages of {@link Flux#flatMap(Function)} and {@link
* A {@link BugChecker} that flags usages of {@link Flux#flatMap(Function)} and {@link
* Flux#flatMapSequential(Function)}.
*
* <p>{@link Flux#flatMap(Function)} and {@link Flux#flatMapSequential(Function)} eagerly perform up

View File

@@ -35,8 +35,8 @@ import javax.annotation.Nullable;
import tech.picnic.errorprone.bugpatterns.util.SourceCode;
/**
* A {@link BugChecker} which flags string concatenations that produce a format string; in such
* cases the string concatenation should instead be deferred to the invoked method.
* A {@link BugChecker} that flags string concatenations that produce a format string; in such cases
* the string concatenation should instead be deferred to the invoked method.
*
* @implNote This checker is based on the implementation of {@link
* com.google.errorprone.bugpatterns.flogger.FloggerStringConcatenation}.
@@ -46,7 +46,7 @@ import tech.picnic.errorprone.bugpatterns.util.SourceCode;
// should introduce special handling of `Formattable` arguments, as this check would replace a
// `Formattable#toString` invocation with a `Formattable#formatTo` invocation. But likely that
// should be considered a bug fix, too.
// XXX: Introduce a separate check which adds/removes the `Locale` parameter to `String.format`
// XXX: Introduce a separate check that adds/removes the `Locale` parameter to `String.format`
// invocations, as necessary.
@AutoService(BugChecker.class)
@BugPattern(

View File

@@ -26,7 +26,7 @@ import java.util.SortedSet;
import javax.lang.model.element.Modifier;
/**
* A {@link BugChecker} which flags {@link SortedSet} property declarations inside
* A {@link BugChecker} that flags {@link SortedSet} property declarations inside
* {@code @Value.Immutable}- and {@code @Value.Modifiable}-annotated types that lack a
* {@code @Value.NaturalOrder} or {@code @Value.ReverseOrder} annotation.
*

View File

@@ -38,8 +38,8 @@ import javax.lang.model.element.Modifier;
import javax.lang.model.element.Name;
import tech.picnic.errorprone.bugpatterns.util.SourceCode;
/** A {@link BugChecker} which flags non-canonical JUnit method declarations. */
// XXX: Consider introducing a class-level check which enforces that test classes:
/** A {@link BugChecker} that flags non-canonical JUnit method declarations. */
// XXX: Consider introducing a class-level check that enforces that test classes:
// 1. Are named `*Test` or `Abstract*TestCase`.
// 2. If not `abstract`, are package-private and don't have public methods and subclasses.
// 3. Only have private fields.

View File

@@ -42,7 +42,7 @@ import tech.picnic.errorprone.bugpatterns.util.AnnotationAttributeMatcher;
import tech.picnic.errorprone.bugpatterns.util.SourceCode;
/**
* A {@link BugChecker} which flags annotation array listings which aren't sorted lexicographically.
* A {@link BugChecker} that flags annotation array listings which aren't sorted lexicographically.
*
* <p>The idea behind this checker is that maintaining a sorted sequence simplifies conflict
* resolution, and can even avoid it if two branches add the same entry.

View File

@@ -35,7 +35,7 @@ import java.util.Optional;
import javax.lang.model.element.Name;
/**
* A {@link BugChecker} which flags lambda expressions that can be replaced with method references.
* A {@link BugChecker} that flags lambda expressions that can be replaced with method references.
*/
// XXX: Other custom expressions we could rewrite:
// - `a -> "str" + a` to `"str"::concat`. But only if `str` is provably non-null.

View File

@@ -20,7 +20,7 @@ import java.util.List;
import tech.picnic.errorprone.bugpatterns.util.SourceCode;
/**
* A {@link BugChecker} which flags method invocations for which all arguments are wrapped using
* A {@link BugChecker} that flags method invocations for which all arguments are wrapped using
* {@link org.mockito.Mockito#eq}; this is redundant.
*/
@AutoService(BugChecker.class)

View File

@@ -20,7 +20,7 @@ import com.sun.tools.javac.code.Type;
import com.sun.tools.javac.util.List;
import java.util.Optional;
/** A {@link BugChecker} which flags nesting of {@link Optional Optionals}. */
/** A {@link BugChecker} that flags nesting of {@link Optional Optionals}. */
@AutoService(BugChecker.class)
@BugPattern(
summary =

View File

@@ -22,7 +22,7 @@ import reactor.core.publisher.Mono;
import tech.picnic.errorprone.bugpatterns.util.SourceCode;
/**
* A {@link BugChecker} which flags {@link Mono} operations that are known to be vacuous, given that
* A {@link BugChecker} that flags {@link Mono} operations that are known to be vacuous, given that
* they are invoked on a {@link Mono} that is known not to complete empty.
*/
@AutoService(BugChecker.class)

View File

@@ -34,7 +34,7 @@ import java.util.stream.Stream;
import tech.picnic.errorprone.bugpatterns.util.SourceCode;
/**
* A {@link BugChecker} which flags {@code Comparator#comparing*} invocations that can be replaced
* A {@link BugChecker} that flags {@code Comparator#comparing*} invocations that can be replaced
* with an equivalent alternative so as to avoid unnecessary (un)boxing.
*/
// XXX: Add more documentation. Explain how this is useful in the face of refactoring to more

View File

@@ -51,7 +51,7 @@ import java.util.stream.Stream;
import tech.picnic.errorprone.bugpatterns.util.MethodMatcherFactory;
import tech.picnic.errorprone.bugpatterns.util.SourceCode;
/** A {@link BugChecker} which flags redundant explicit string conversions. */
/** A {@link BugChecker} that flags redundant explicit string conversions. */
@AutoService(BugChecker.class)
@BugPattern(
summary = "Avoid redundant string conversions when possible",
@@ -224,7 +224,7 @@ public final class RedundantStringConversion extends BugChecker
.flatMap(args -> tryFix(args.get(index), state, ANY_EXPR));
}
// XXX: Write another check which checks that Formatter patterns don't use `{}` and have a
// XXX: Write another check that checks that Formatter patterns don't use `{}` and have a
// matching number of arguments of the appropriate type. Also flag explicit conversions from
// `Formattable` to string.
private Optional<SuggestedFix.Builder> tryFixFormatter(
@@ -255,7 +255,7 @@ public final class RedundantStringConversion extends BugChecker
return tryFixFormatterArguments(arguments, state, ANY_EXPR, ANY_EXPR);
}
// XXX: Write another check which checks that SLF4J patterns don't use `%s` and have a matching
// XXX: Write another check that checks that SLF4J patterns don't use `%s` and have a matching
// number of arguments of the appropriate type. Also flag explicit conversions from `Throwable` to
// string as the last logger argument. Suggests either dropping the conversion or going with
// `Throwable#getMessage()` instead.

View File

@@ -19,7 +19,7 @@ import com.sun.source.tree.MethodInvocationTree;
import tech.picnic.errorprone.bugpatterns.util.SourceCode;
/**
* A {@link BugChecker} which flags unnecessary {@link Refaster#anyOf(Object[])} usages.
* A {@link BugChecker} that flags unnecessary {@link Refaster#anyOf(Object[])} usages.
*
* <p>Note that this logic can't be implemented as a Refaster template, as the {@link Refaster}
* class is treated specially.

View File

@@ -23,7 +23,7 @@ import com.sun.source.tree.MethodTree;
import com.sun.source.tree.Tree;
/**
* A {@link BugChecker} which flags {@code @RequestMapping} methods that have one or more parameters
* A {@link BugChecker} that flags {@code @RequestMapping} methods that have one or more parameters
* that appear to lack a relevant annotation.
*
* <p>Matched mappings are {@code @{Delete,Get,Patch,Post,Put,Request}Mapping}.

View File

@@ -21,7 +21,7 @@ import com.google.errorprone.matchers.Description;
import com.google.errorprone.matchers.Matcher;
import com.sun.source.tree.VariableTree;
/** A {@link BugChecker} which flags {@code @RequestParam} parameters with an unsupported type. */
/** A {@link BugChecker} that flags {@code @RequestParam} parameters with an unsupported type. */
@AutoService(BugChecker.class)
@BugPattern(
summary = "`@RequestParam` does not support `ImmutableCollection` and `ImmutableMap` subtypes",

View File

@@ -27,8 +27,8 @@ import com.sun.source.tree.MethodTree;
import com.sun.source.tree.Tree;
/**
* A {@link BugChecker} which flags methods with Spring's {@code @Scheduled} annotation that lack
* New Relic Agent's {@code @Trace(dispatcher = true)}.
* A {@link BugChecker} that flags methods with Spring's {@code @Scheduled} annotation that lack New
* Relic Agent's {@code @Trace(dispatcher = true)}.
*/
@AutoService(BugChecker.class)
@BugPattern(

View File

@@ -24,7 +24,7 @@ import java.util.List;
import java.util.Optional;
import tech.picnic.errorprone.bugpatterns.util.SourceCode;
/** A {@link BugChecker} which flags SLF4J usages that are likely to be in error. */
/** A {@link BugChecker} that flags SLF4J usages that are likely to be in error. */
// XXX: The special-casing of Throwable applies only to SLF4J 1.6.0+; see
// https://www.slf4j.org/faq.html#paramException. That should be documented.
// XXX: Also simplify `LOG.error(String.format("Something %s", arg), throwable)`.

View File

@@ -29,7 +29,7 @@ import tech.picnic.errorprone.bugpatterns.util.AnnotationAttributeMatcher;
import tech.picnic.errorprone.bugpatterns.util.SourceCode;
/**
* A {@link BugChecker} which flags {@code @RequestMapping} annotations that can be written more
* A {@link BugChecker} that flags {@code @RequestMapping} annotations that can be written more
* concisely.
*/
@AutoService(BugChecker.class)

View File

@@ -27,15 +27,14 @@ import com.sun.tools.javac.code.Type;
import java.util.Optional;
/**
* A {@link BugChecker} which flags methods and constants that can and should be statically
* imported.
* A {@link BugChecker} that flags methods and constants that can and should be statically imported.
*/
// XXX: Tricky cases:
// - `org.springframework.http.HttpStatus` (not always an improvement, and `valueOf` must
// certainly be excluded)
// - `com.google.common.collect.Tables`
// - `ch.qos.logback.classic.Level.{DEBUG, ERROR, INFO, TRACE, WARN"}`
// XXX: Also introduce a check which disallows static imports of certain methods. Candidates:
// XXX: Also introduce a check that disallows static imports of certain methods. Candidates:
// - `com.google.common.base.Strings`
// - `java.util.Optional.empty`
// - `java.util.Locale.ROOT`

View File

@@ -26,7 +26,7 @@ import java.time.LocalDate;
import java.time.LocalDateTime;
import java.time.LocalTime;
/** A {@link BugChecker} which flags illegal time-zone related operations. */
/** A {@link BugChecker} that flags illegal time-zone related operations. */
@AutoService(BugChecker.class)
@BugPattern(
summary =

View File

@@ -9,7 +9,7 @@ import com.google.errorprone.refaster.annotation.BeforeTemplate;
import java.math.BigInteger;
import org.assertj.core.api.AbstractBigIntegerAssert;
// XXX: If we add a rule which drops unnecessary `L` suffixes from literal longs, then the `0L`/`1L`
// XXX: If we add a rule that drops unnecessary `L` suffixes from literal longs, then the `0L`/`1L`
// cases below can go.
final class AssertJBigIntegerTemplates {
private AssertJBigIntegerTemplates() {}

View File

@@ -63,7 +63,7 @@ import tech.picnic.errorprone.refaster.util.IsArray;
// ^ And variants.
// XXX: Consider splitting this class into multiple classes.
// XXX: Some of these rules may not apply given the updated TestNG rewrite rules. Review.
// XXX: For the templates which "unwrap" explicitly enumerated collections, also introduce variants
// XXX: For the templates 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.
@@ -106,12 +106,12 @@ import tech.picnic.errorprone.refaster.util.IsArray;
// candidates, such as `assertThat(ImmutableSet(foo, bar)).XXX`
// XXX: Write generic plugin to replace explicit array parameters with varargs (`new int[] {1, 2}`
// -> `1, 2`).
// XXX: Write plugin which drops any `.withFailMessage` which doesn't include a compile-time
// constant string? Most of these are useless.
// XXX: Write plugin which identifies `.get().propertyAccess()` and "pushes" this out. Would only
// XXX: Write plugin that drops any `.withFailMessage` that doesn't include a compile-time constant
// string? Most of these are useless.
// XXX: Write plugin that identifies `.get().propertyAccess()` and "pushes" this out. Would only
// nicely work for non-special types, though, cause after `extracting(propertyAccess)` many
// operations are not available...
// XXX: Write plugin which identifies repeated `assertThat(someProp.xxx)` calls and bundles these
// XXX: Write plugin that identifies repeated `assertThat(someProp.xxx)` calls and bundles these
// somehow.
// XXX: `abstractOptionalAssert.get().satisfies(pred)` ->
// `abstractOptionalAssert.hasValueSatisfying(pred)`.

View File

@@ -429,7 +429,7 @@ final class AssertJThrowingCallableTemplates {
}
}
// XXX: Drop this template in favour of a generic Error Prone check which flags
// XXX: Drop this template 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
@@ -449,7 +449,7 @@ final class AssertJThrowingCallableTemplates {
}
}
// XXX: Drop this template in favour of a generic Error Prone check which flags
// XXX: Drop this template 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

View File

@@ -82,7 +82,7 @@ final class AssortedTemplates {
* ImmutableSet#toImmutableSet()} and produces a more compact object.
*
* <p><strong>Warning:</strong> this rewrite rule is not completely behavior preserving: while the
* original code produces a set which iterates over the elements in encounter order, the
* original code produces a set that iterates over the elements in encounter order, the
* replacement code iterates over the elements in enum definition order.
*/
// XXX: ^ Consider emitting a comment warning about this fact?
@@ -118,7 +118,7 @@ final class AssortedTemplates {
/** Don't unnecessarily repeat boolean expressions. */
// XXX: This template captures only the simplest case. `@AlsoNegation` doesn't help. Consider
// contributing a Refaster patch which handles the negation in the `@BeforeTemplate` more
// contributing a Refaster patch, which handles the negation in the `@BeforeTemplate` more
// intelligently.
static final class LogicalImplication {
@BeforeTemplate

View File

@@ -36,7 +36,7 @@ final class EqualityTemplates {
/** Prefer {@link Object#equals(Object)} over the equivalent lambda function. */
// XXX: As it stands, this rule is a special case of what `MethodReferenceUsage` tries to achieve.
// If/when `MethodReferenceUsage` becomes production ready, we should simply drop this check.
// XXX: Alternatively, the rule should be replaced with a plugin which also identifies cases where
// XXX: Alternatively, the rule should be replaced with a plugin that also identifies cases where
// the arguments are swapped but simplification is possible anyway, by virtue of `v` being
// non-null.
static final class EqualsPredicate<T> {

View File

@@ -194,7 +194,7 @@ final class ImmutableListTemplates {
* Prefer {@link ImmutableList#of(Object, Object)} over alternatives that don't communicate the
* immutability of the resulting list at the type level.
*/
// XXX: Consider writing an Error Prone check which also flags straightforward
// XXX: Consider writing an Error Prone check that also flags straightforward
// `ImmutableList.builder()` usages.
static final class ImmutableListOf2<T> {
@BeforeTemplate
@@ -212,7 +212,7 @@ final class ImmutableListTemplates {
* Prefer {@link ImmutableList#of(Object, Object, Object)} over alternatives that don't
* communicate the immutability of the resulting list at the type level.
*/
// XXX: Consider writing an Error Prone check which also flags straightforward
// XXX: Consider writing an Error Prone check that also flags straightforward
// `ImmutableList.builder()` usages.
static final class ImmutableListOf3<T> {
@BeforeTemplate
@@ -230,7 +230,7 @@ final class ImmutableListTemplates {
* Prefer {@link ImmutableList#of(Object, Object, Object, Object)} over alternatives that don't
* communicate the immutability of the resulting list at the type level.
*/
// XXX: Consider writing an Error Prone check which also flags straightforward
// XXX: Consider writing an Error Prone check that also flags straightforward
// `ImmutableList.builder()` usages.
static final class ImmutableListOf4<T> {
@BeforeTemplate
@@ -248,7 +248,7 @@ final class ImmutableListTemplates {
* Prefer {@link ImmutableList#of(Object, Object, Object, Object, Object)} over alternatives that
* don't communicate the immutability of the resulting list at the type level.
*/
// XXX: Consider writing an Error Prone check which also flags straightforward
// XXX: Consider writing an Error Prone check that also flags straightforward
// `ImmutableList.builder()` usages.
static final class ImmutableListOf5<T> {
@BeforeTemplate

View File

@@ -135,7 +135,7 @@ final class ImmutableMapTemplates {
abstract V valueFunction(@MayOptionallyUse E element);
// XXX: We could add variants in which the entry is created some other way, but we have another
// rule which covers canonicalization to `Map.entry`.
// rule that covers canonicalization to `Map.entry`.
@BeforeTemplate
ImmutableMap<K, V> before(Stream<E> stream) {
return stream

View File

@@ -142,7 +142,7 @@ final class ImmutableSetTemplates {
* Prefer {@link ImmutableSet#of(Object, Object)} over alternatives that don't communicate the
* immutability of the resulting set at the type level.
*/
// XXX: Consider writing an Error Prone check which also flags straightforward
// XXX: Consider writing an Error Prone check that also flags straightforward
// `ImmutableSet.builder()` usages.
static final class ImmutableSetOf2<T> {
@BeforeTemplate
@@ -160,7 +160,7 @@ final class ImmutableSetTemplates {
* Prefer {@link ImmutableSet#of(Object, Object, Object)} over alternatives that don't communicate
* the immutability of the resulting set at the type level.
*/
// XXX: Consider writing an Error Prone check which also flags straightforward
// XXX: Consider writing an Error Prone check that also flags straightforward
// `ImmutableSet.builder()` usages.
static final class ImmutableSetOf3<T> {
@BeforeTemplate
@@ -178,7 +178,7 @@ final class ImmutableSetTemplates {
* Prefer {@link ImmutableSet#of(Object, Object, Object, Object)} over alternatives that don't
* communicate the immutability of the resulting set at the type level.
*/
// XXX: Consider writing an Error Prone check which also flags straightforward
// XXX: Consider writing an Error Prone check that also flags straightforward
// `ImmutableSet.builder()` usages.
static final class ImmutableSetOf4<T> {
@BeforeTemplate
@@ -196,7 +196,7 @@ final class ImmutableSetTemplates {
* Prefer {@link ImmutableSet#of(Object, Object, Object, Object, Object)} over alternatives that
* don't communicate the immutability of the resulting set at the type level.
*/
// XXX: Consider writing an Error Prone check which also flags straightforward
// XXX: Consider writing an Error Prone check that also flags straightforward
// `ImmutableSet.builder()` usages.
static final class ImmutableSetOf5<T> {
@BeforeTemplate

View File

@@ -166,7 +166,7 @@ final class OptionalTemplates {
}
/**
* Prefer {@link Optional#map} over a {@link Optional#flatMap} which wraps the result of a
* Prefer {@link Optional#map} over a {@link Optional#flatMap} that wraps the result of a
* transformation in an {@link Optional}; the former operation transforms {@code null} to {@link
* Optional#empty()}.
*/

View File

@@ -228,7 +228,7 @@ final class ReactorTemplates {
/**
* Prefer a collection using {@link MoreCollectors#toOptional()} over more contrived alternatives.
*/
// XXX: Consider creating a plugin which flags/discourages `Mono<Optional<T>>` method return
// XXX: Consider creating a plugin that flags/discourages `Mono<Optional<T>>` method return
// types, just as we discourage nullable `Boolean`s and `Optional`s.
static final class MonoCollectToOptional<T> {
@BeforeTemplate

View File

@@ -30,7 +30,7 @@ import org.testng.Assert;
import org.testng.Assert.ThrowingRunnable;
/**
* Refaster templates which replace TestNG assertions with equivalent AssertJ assertions.
* Refaster templates 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
@@ -39,9 +39,9 @@ import org.testng.Assert.ThrowingRunnable;
* the appropriate (more specific) types _will_ be inferred properly when plugged into AssertJ's
* API.
*
* <p>The following is an example of a TestNG statement which would not be rewritten if it weren't
* <p>The following is an example of a TestNG statement, which would not be rewritten if it weren't
* for the wildcard matching (note that the type parameters of the map on the right-hand side will
* be inferred to be {@code <Object, Object>} rather than {@code <String, Object>}.)
* be inferred to be {@code <Object, Object>} rather than {@code <String, Object>}).
*
* <pre>{@code
* List<Map<String, Object>> myMaps = new ArrayList<>();

View File

@@ -16,7 +16,7 @@ import com.sun.source.tree.MethodInvocationTree;
import org.junit.jupiter.api.Test;
final class MethodMatcherFactoryTest {
/** A {@link BugChecker} which flags method invocations matched by {@link #TEST_MATCHER}. */
/** A {@link BugChecker} that flags method invocations matched by {@link #TEST_MATCHER}. */
@BugPattern(severity = SUGGESTION, summary = "Flags methods matched by the test matcher.")
public static final class MatchedMethodsFlagger extends BugChecker
implements MethodInvocationTreeMatcher {

View File

@@ -159,7 +159,7 @@
<!-- XXX: Two other dependencies are potentially of interest:
`com.palantir.assertj-automation:assertj-refaster-rules` and
`com.palantir.baseline:baseline-refaster-rules` contain Refaster rules
which aren't currently applied. We should use
that aren't currently applied. We should use
`RefasterRuleBuilderScanner` to convert those to `.refaster` files so
that we can pick them up. (But in case of `baseline-refaster-rules`
perhaps we can simply incorporate all of them.) -->
@@ -1320,7 +1320,7 @@
</dependencyManagement>
</profile>
<profile>
<!-- Error Prone checks which are not availabe from Maven Central;
<!-- Error Prone checks that are not availabe from Maven Central;
these are therefore not enabled by default. -->
<id>non-maven-central</id>
<properties>
@@ -1507,7 +1507,7 @@
</build>
</profile>
<profile>
<!-- An extension of the `build-checks` profile which configures
<!-- An extension of the `build-checks` profile that configures
Error Prone. This configuration is defined in a separate profile so
that it can be combined with the `patch` profile below, without
introducing the overhead of other build checks. -->

View File

@@ -11,7 +11,7 @@
<artifactId>refaster-compiler</artifactId>
<name>Picnic :: Error Prone Support :: Refaster Compiler</name>
<description>Java compiler plugin which identifies and compiles Refaster templates, storing them as resource files on the classpath.</description>
<description>A Java compiler plugin that identifies and compiles Refaster templates, storing them as resource files on the classpath.</description>
<dependencies>
<dependency>

View File

@@ -6,7 +6,7 @@ import com.sun.source.util.Plugin;
import com.sun.tools.javac.api.BasicJavacTask;
/**
* A variant of {@code com.google.errorprone.refaster.RefasterRuleCompiler} which outputs a {@code
* A variant of {@code com.google.errorprone.refaster.RefasterRuleCompiler} that outputs a {@code
* fully/qualified/Class.refaster} file for each compiled {@code fully.qualified.Class} that
* contains a Refaster template.
*/

View File

@@ -34,7 +34,7 @@ import javax.tools.JavaFileManager;
import javax.tools.StandardLocation;
/**
* A variant of {@code com.google.errorprone.refaster.RefasterRuleCompilerAnalyzer} which stores
* A variant of {@code com.google.errorprone.refaster.RefasterRuleCompilerAnalyzer} that stores
* compiled Refaster rules in a {@code .refaster} file next to the compiled {@code .class} file,
* rather than at a fixed location.
*

View File

@@ -1,5 +1,5 @@
/**
* A Java compiler plugin which identifies and compiles Refaster templates, storing them as resource
* A Java compiler plugin that identifies and compiles Refaster templates, storing them as resource
* files on the classpath.
*/
@com.google.errorprone.annotations.CheckReturnValue

View File

@@ -37,7 +37,7 @@ import java.util.regex.Pattern;
import java.util.stream.Stream;
/**
* A {@link BugChecker} which flags code which can be simplified using Refaster templates located on
* A {@link BugChecker} that flags code that can be simplified using Refaster templates located on
* the classpath.
*
* <p>This checker locates all {@code *.refaster} classpath resources and assumes they contain a
@@ -98,9 +98,9 @@ public final class Refaster extends BugChecker implements CompilationUnitTreeMat
* Reports a subset of the given matches, such that no two reported matches suggest a replacement
* of the same part of the source code.
*
* <p>In the common case all matches will be reported. In case of overlap the match which replaces
* <p>In the common case all matches will be reported. In case of overlap the match that replaces
* the largest piece of source code is preferred. In case two matches wish to replace exactly the
* same piece of code, preference is given to the match which suggests the shortest replacement.
* same piece of code, preference is given to the match that suggests the shortest replacement.
*/
// XXX: This selection logic solves an issue described in
// https://github.com/google/error-prone/issues/559. Consider contributing it back upstream.

View File

@@ -48,7 +48,7 @@ final class IsArrayTest {
.doTest();
}
/** A {@link BugChecker} which simply delegates to {@link IsArray}. */
/** A {@link BugChecker} that simply delegates to {@link IsArray}. */
@BugPattern(summary = "Flags expressions matched by `IsArray`", severity = ERROR)
public static final class MatcherTestChecker extends AbstractMatcherTestChecker {
private static final long serialVersionUID = 1L;

View File

@@ -48,7 +48,7 @@ final class IsCharacterTest {
.doTest();
}
/** A {@link BugChecker} which simply delegates to {@link IsCharacter}. */
/** A {@link BugChecker} that simply delegates to {@link IsCharacter}. */
@BugPattern(summary = "Flags expressions matched by `IsCharacter`", severity = ERROR)
public static final class MatcherTestChecker extends AbstractMatcherTestChecker {
private static final long serialVersionUID = 1L;

View File

@@ -79,7 +79,7 @@ final class ThrowsCheckedExceptionTest {
.doTest();
}
/** A {@link BugChecker} which simply delegates to {@link ThrowsCheckedException}. */
/** A {@link BugChecker} that simply delegates to {@link ThrowsCheckedException}. */
@BugPattern(summary = "Flags expressions matched by `ThrowsCheckedException`", severity = ERROR)
public static final class MatcherTestChecker extends AbstractMatcherTestChecker {
private static final long serialVersionUID = 1L;