Compare commits

...

17 Commits

Author SHA1 Message Date
Stephan Schroevers
fbef169556 FAILED experiment
I sense that we can do better, but it won't be trivial.
2022-08-20 16:55:50 +02:00
Cernat Catalin Stefan
b8e22ffef0 Introduce NonEmptyMono check (#200) 2022-08-20 12:47:56 +02:00
Picnic-Bot
17035a1623 Upgrade Spring Boot 2.7.2 -> 2.7.3 (#207)
See:
- https://github.com/spring-projects/spring-boot/releases/tag/v2.7.3
- https://github.com/spring-projects/spring-boot/compare/v2.7.2...v2.7.3
2022-08-20 09:54:57 +02:00
Picnic-Bot
e7dacd19d7 Upgrade Maven API 3.6.3 -> 3.8.6 (#184)
See:
- https://maven.apache.org/release-notes-all.html
- https://github.com/apache/maven/releases/tag/maven-3.8.4
- https://github.com/apache/maven/releases/tag/maven-3.8.5
- https://github.com/apache/maven/releases/tag/maven-3.8.6
- https://github.com/apache/maven/compare/maven-3.6.3...maven-3.8.6
2022-08-19 19:05:18 +02:00
Bastien Diederichs
e64af1dde0 Don't enforce sorting of Spring resource locations (#204)
Because their order matters.
2022-08-19 15:29:21 +02:00
Rick Ossendrijver
a58630bccf Improve StreamMapToOptionalGet Refaster template documentation (#203) 2022-08-19 13:33:23 +02:00
Gijs de Jong
9ab5bbe042 Require static import of com.google.errorprone.matchers.Matchers methods (#201) 2022-08-18 15:21:16 +02:00
Stephan Schroevers
4ca75c6cf6 Enable Error Prone's VoidMissingNullable check (#180) 2022-08-18 14:28:01 +02:00
Ferdinand Swoboda
7883b31eb6 Introduce ImmutablesSortedSetComparator check (#102) 2022-08-18 11:33:20 +02:00
Stephan Schroevers
ef751ce785 Drop various vacuous null checks (#191)
Recent versions of Error Prone guarantee that `ASTHelpers#getSymbol`
never returns `null`.
2022-08-18 11:25:35 +02:00
Vincent Koeman
130c3d0bc3 Introduce NestedOptionals check (#202) 2022-08-17 22:14:31 +02:00
Picnic-Bot
c89e3905bf Upgrade Mockito 4.6.1 -> 4.7.0 (#196)
See:
- https://github.com/mockito/mockito/releases/tag/v4.7.0
- https://github.com/mockito/mockito/compare/v4.6.1...v4.7.0
2022-08-17 16:37:54 +02:00
Picnic-Bot
21421ce753 Upgrade maven-javadoc-plugin 3.4.0 -> 3.4.1 (#195)
See:
- https://github.com/apache/maven-javadoc-plugin/releases/tag/maven-javadoc-plugin-3.4.1
- https://github.com/apache/maven-javadoc-plugin/compare/maven-javadoc-plugin-3.4.0...maven-javadoc-plugin-3.4.1
2022-08-17 16:00:15 +02:00
Ivan Fedorov
c39d1251d2 Rewrite another ThrowableAssertAlternative#withMessage(String, Object...) expression (#190) 2022-08-17 07:38:22 +02:00
Dario Deya Diaz
9bc732b4fe Have RequestMappingAnnotation recognize @RequestAttribute parameters (#189) 2022-08-12 20:56:28 +02:00
Picnic-Bot
74100b6c41 Upgrade Project Reactor 2020.0.21 -> 2020.0.22 (#187)
See:
- https://github.com/reactor/reactor/releases/tag/2020.0.22
- https://github.com/reactor/reactor/compare/2020.0.21...2020.0.22
2022-08-11 08:21:06 +02:00
Stephan Schroevers
624f2ce753 [maven-release-plugin] prepare for next development iteration 2022-08-10 15:06:32 +02:00
28 changed files with 759 additions and 58 deletions

View File

@@ -5,7 +5,7 @@
<parent>
<groupId>tech.picnic.error-prone-support</groupId>
<artifactId>error-prone-support</artifactId>
<version>0.1.0</version>
<version>0.1.1-SNAPSHOT</version>
</parent>
<artifactId>error-prone-contrib</artifactId>
@@ -143,6 +143,11 @@
<artifactId>assertj-core</artifactId>
<scope>provided</scope>
</dependency>
<dependency>
<groupId>org.immutables</groupId>
<artifactId>value-annotations</artifactId>
<scope>test</scope>
</dependency>
<dependency>
<groupId>org.junit.jupiter</groupId>
<artifactId>junit-jupiter-api</artifactId>

View File

@@ -20,7 +20,6 @@ import com.google.errorprone.util.ASTHelpers;
import com.sun.source.tree.ClassTree;
import com.sun.source.tree.MethodTree;
import com.sun.source.tree.Tree;
import com.sun.tools.javac.code.Symbol.MethodSymbol;
import java.util.Optional;
/** A {@link BugChecker} which flags empty methods that seemingly can simply be deleted. */
@@ -47,8 +46,7 @@ public final class EmptyMethod extends BugChecker implements MethodTreeMatcher {
return Description.NO_MATCH;
}
MethodSymbol sym = ASTHelpers.getSymbol(tree);
if (sym == null || ASTHelpers.methodCanBeOverridden(sym)) {
if (ASTHelpers.methodCanBeOverridden(ASTHelpers.getSymbol(tree))) {
return Description.NO_MATCH;
}

View File

@@ -0,0 +1,81 @@
package tech.picnic.errorprone.bugpatterns;
import static com.google.errorprone.BugPattern.LinkType.NONE;
import static com.google.errorprone.BugPattern.SeverityLevel.ERROR;
import static com.google.errorprone.BugPattern.StandardTags.LIKELY_ERROR;
import static com.google.errorprone.matchers.Matchers.allOf;
import static com.google.errorprone.matchers.Matchers.anyOf;
import static com.google.errorprone.matchers.Matchers.enclosingClass;
import static com.google.errorprone.matchers.Matchers.hasAnnotation;
import static com.google.errorprone.matchers.Matchers.hasModifier;
import static com.google.errorprone.matchers.Matchers.isSubtypeOf;
import static com.google.errorprone.matchers.Matchers.methodReturns;
import static com.google.errorprone.matchers.Matchers.not;
import com.google.auto.service.AutoService;
import com.google.errorprone.BugPattern;
import com.google.errorprone.VisitorState;
import com.google.errorprone.bugpatterns.BugChecker;
import com.google.errorprone.bugpatterns.BugChecker.MethodTreeMatcher;
import com.google.errorprone.fixes.SuggestedFix;
import com.google.errorprone.fixes.SuggestedFixes;
import com.google.errorprone.matchers.Description;
import com.google.errorprone.matchers.Matcher;
import com.sun.source.tree.MethodTree;
import java.util.SortedSet;
import javax.lang.model.element.Modifier;
/**
* A {@link BugChecker} which 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.
*
* <p>Without such an annotation:
*
* <ul>
* <li>deserialization of the enclosing type requires that the associated JSON property is
* present, contrary to the way in which Immutables handles other collection properties; and
* <li>different instances may use different comparator implementations (e.g. deserialization
* would default to natural order sorting), potentially leading to subtle bugs.
* </ul>
*/
@AutoService(BugChecker.class)
@BugPattern(
summary =
"`SortedSet` properties of a `@Value.Immutable` or `@Value.Modifiable` type must be "
+ "annotated with `@Value.NaturalOrder` or `@Value.ReverseOrder`",
linkType = NONE,
severity = ERROR,
tags = LIKELY_ERROR)
public final class ImmutablesSortedSetComparator extends BugChecker implements MethodTreeMatcher {
private static final long serialVersionUID = 1L;
private static final Matcher<MethodTree> METHOD_LACKS_ANNOTATION =
allOf(
methodReturns(isSubtypeOf(SortedSet.class)),
anyOf(
allOf(
hasModifier(Modifier.ABSTRACT),
enclosingClass(
anyOf(
hasAnnotation("org.immutables.value.Value.Immutable"),
hasAnnotation("org.immutables.value.Value.Modifiable")))),
hasAnnotation("org.immutables.value.Value.Default")),
not(
anyOf(
hasAnnotation("org.immutables.value.Value.NaturalOrder"),
hasAnnotation("org.immutables.value.Value.ReverseOrder"))));
@Override
public Description matchMethod(MethodTree tree, VisitorState state) {
if (!METHOD_LACKS_ANNOTATION.matches(tree, state)) {
return Description.NO_MATCH;
}
SuggestedFix.Builder builder = SuggestedFix.builder();
String valueTypeIdentifier =
SuggestedFixes.qualifyType(state, builder, "org.immutables.value.Value");
return describeMatch(
tree,
builder.prefixWith(tree, String.format("@%s.NaturalOrder ", valueTypeIdentifier)).build());
}
}

View File

@@ -176,8 +176,7 @@ public final class JUnitMethodDeclaration extends BugChecker implements MethodTr
}
private static Optional<String> tryCanonicalizeMethodName(MethodTree tree) {
return Optional.ofNullable(ASTHelpers.getSymbol(tree))
.map(sym -> sym.getQualifiedName().toString())
return Optional.of(ASTHelpers.getSymbol(tree).getQualifiedName().toString())
.filter(name -> name.startsWith(TEST_PREFIX))
.map(name -> name.substring(TEST_PREFIX.length()))
.filter(not(String::isEmpty))

View File

@@ -62,7 +62,10 @@ public final class LexicographicalAnnotationAttributeListing extends BugChecker
"com.fasterxml.jackson.annotation.JsonPropertyOrder#value",
"io.swagger.annotations.ApiImplicitParams#value",
"io.swagger.v3.oas.annotations.Parameters#value",
"javax.xml.bind.annotation.XmlType#propOrder");
"javax.xml.bind.annotation.XmlType#propOrder",
"org.springframework.context.annotation.PropertySource#value",
"org.springframework.test.context.TestPropertySource#locations",
"org.springframework.test.context.TestPropertySource#value");
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";
@@ -175,21 +178,21 @@ public final class LexicographicalAnnotationAttributeListing extends BugChecker
new TreeScanner<Void, Void>() {
@Nullable
@Override
public Void visitIdentifier(IdentifierTree node, Void ctx) {
public Void visitIdentifier(IdentifierTree node, @Nullable Void ctx) {
nodes.add(tokenize(node));
return super.visitIdentifier(node, ctx);
}
@Nullable
@Override
public Void visitLiteral(LiteralTree node, Void ctx) {
public Void visitLiteral(LiteralTree node, @Nullable Void ctx) {
nodes.add(tokenize(node));
return super.visitLiteral(node, ctx);
}
@Nullable
@Override
public Void visitPrimitiveType(PrimitiveTypeTree node, Void ctx) {
public Void visitPrimitiveType(PrimitiveTypeTree node, @Nullable Void ctx) {
nodes.add(tokenize(node));
return super.visitPrimitiveType(node, ctx);
}

View File

@@ -100,6 +100,8 @@ public final class MethodReferenceUsage extends BugChecker implements LambdaExpr
.flatMap(statements -> constructMethodRef(lambdaExpr, statements.get(0)));
}
// XXX: Replace nested `Optional` usage.
@SuppressWarnings("NestedOptionals")
private static Optional<SuggestedFix.Builder> constructMethodRef(
LambdaExpressionTree lambdaExpr, MethodInvocationTree subTree) {
return matchArguments(lambdaExpr, subTree)
@@ -156,6 +158,8 @@ public final class MethodReferenceUsage extends BugChecker implements LambdaExpr
return constructFix(lambdaExpr, lhsType.tsym, subTree.getIdentifier());
}
// XXX: Refactor or replace inner `Optional` with a custom type.
@SuppressWarnings("NestedOptionals")
private static Optional<Optional<Name>> matchArguments(
LambdaExpressionTree lambdaExpr, MethodInvocationTree subTree) {
ImmutableList<Name> expectedArguments = getVariables(lambdaExpr);

View File

@@ -0,0 +1,51 @@
package tech.picnic.errorprone.bugpatterns;
import static com.google.errorprone.BugPattern.LinkType.NONE;
import static com.google.errorprone.BugPattern.SeverityLevel.WARNING;
import static com.google.errorprone.BugPattern.StandardTags.FRAGILE_CODE;
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;
import com.google.errorprone.bugpatterns.BugChecker.MethodInvocationTreeMatcher;
import com.google.errorprone.matchers.Description;
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} which flags nesting of {@link Optional Optionals}. */
@AutoService(BugChecker.class)
@BugPattern(
summary =
"Avoid nesting `Optional`s inside `Optional`s; the resultant code is hard to reason about",
linkType = NONE,
severity = WARNING,
tags = FRAGILE_CODE)
public final class NestedOptionals extends BugChecker implements MethodInvocationTreeMatcher {
private static final long serialVersionUID = 1L;
private static final Supplier<Type> OPTIONAL = Suppliers.typeFromClass(Optional.class);
@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);
}
}

View File

@@ -0,0 +1,91 @@
package tech.picnic.errorprone.bugpatterns;
import static com.google.errorprone.BugPattern.LinkType.NONE;
import static com.google.errorprone.BugPattern.SeverityLevel.WARNING;
import static com.google.errorprone.BugPattern.StandardTags.SIMPLIFICATION;
import static com.google.errorprone.matchers.Matchers.anyOf;
import static com.google.errorprone.matchers.Matchers.instanceMethod;
import com.google.auto.service.AutoService;
import com.google.errorprone.BugPattern;
import com.google.errorprone.VisitorState;
import com.google.errorprone.bugpatterns.BugChecker;
import com.google.errorprone.bugpatterns.BugChecker.MethodInvocationTreeMatcher;
import com.google.errorprone.fixes.SuggestedFix;
import com.google.errorprone.matchers.Description;
import com.google.errorprone.matchers.Matcher;
import com.google.errorprone.util.ASTHelpers;
import com.sun.source.tree.ExpressionTree;
import com.sun.source.tree.MethodInvocationTree;
import java.util.function.BiFunction;
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
* they are invoked on a {@link Mono} that is known not to complete empty.
*/
@AutoService(BugChecker.class)
@BugPattern(
summary = "Avoid vacuous operations on known non-empty `Mono`s",
linkType = NONE,
severity = WARNING,
tags = SIMPLIFICATION)
// XXX: This check does not simplify `someFlux.defaultIfEmpty(T).{defaultIfEmpty(T),hasElements()}`,
// as `someFlux.defaultIfEmpty(T)` yields a `Flux` rather than a `Mono`. Consider adding support for
// these cases.
// XXX: Given more advanced analysis many more expressions could be flagged. Consider
// `Mono.just(someValue)`, `Flux.just(someNonEmptySequence)`,
// `someMono.switchIfEmpty(someProvablyNonEmptyMono)` and many other variants.
// XXX: Consider implementing a similar check for `Publisher`s that are known to complete without
// emitting a value (e.g. `Mono.empty()`, `someFlux.then()`, ...), or known not to complete normally
// (`Mono.never()`, `someFlux.repeat()`, `Mono.error(...)`, ...). The latter category could
// potentially be split out further.
public final class NonEmptyMono extends BugChecker implements MethodInvocationTreeMatcher {
private static final long serialVersionUID = 1L;
private static final Matcher<ExpressionTree> MONO_SIZE_CHECK =
instanceMethod()
.onDescendantOf("reactor.core.publisher.Mono")
.namedAnyOf("defaultIfEmpty", "single", "switchIfEmpty");
private static final Matcher<ExpressionTree> NON_EMPTY_MONO =
anyOf(
instanceMethod()
.onDescendantOf("reactor.core.publisher.Flux")
.namedAnyOf(
"all",
"any",
"collect",
"collectList",
"collectMap",
"collectMultimap",
"collectSortedList",
"count",
"elementAt",
"hasElement",
"hasElements",
"last",
"reduceWith",
"single"),
instanceMethod()
.onDescendantOf("reactor.core.publisher.Flux")
.named("reduce")
.withParameters(Object.class.getName(), BiFunction.class.getName()),
instanceMethod()
.onDescendantOf("reactor.core.publisher.Mono")
.namedAnyOf("defaultIfEmpty", "hasElement", "single"));
@Override
public Description matchMethodInvocation(MethodInvocationTree tree, VisitorState state) {
if (!MONO_SIZE_CHECK.matches(tree, state)) {
return Description.NO_MATCH;
}
ExpressionTree receiver = ASTHelpers.getReceiver(tree);
if (!NON_EMPTY_MONO.matches(receiver, state)) {
return Description.NO_MATCH;
}
return describeMatch(
tree, SuggestedFix.replace(tree, SourceCode.treeToString(receiver, state)));
}
}

View File

@@ -6,7 +6,6 @@ import static com.google.errorprone.BugPattern.StandardTags.PERFORMANCE;
import static com.google.errorprone.matchers.Matchers.anyOf;
import static com.google.errorprone.matchers.method.MethodMatchers.instanceMethod;
import static com.google.errorprone.matchers.method.MethodMatchers.staticMethod;
import static java.util.function.Predicate.not;
import static java.util.stream.Collectors.joining;
import com.google.auto.service.AutoService;
@@ -84,16 +83,15 @@ public final class PrimitiveComparison extends BugChecker implements MethodInvoc
private static Optional<Fix> attemptMethodInvocationReplacement(
MethodInvocationTree tree, Type cmpType, boolean isStatic, VisitorState state) {
return Optional.ofNullable(ASTHelpers.getSymbol(tree))
.map(methodSymbol -> methodSymbol.getSimpleName().toString())
.flatMap(
actualMethodName ->
Optional.of(getPreferredMethod(cmpType, isStatic, state))
.filter(not(actualMethodName::equals)))
.map(
preferredMethodName ->
prefixTypeArgumentsIfRelevant(preferredMethodName, tree, cmpType, state))
.map(preferredMethodName -> suggestFix(tree, preferredMethodName, state));
String actualMethodName = ASTHelpers.getSymbol(tree).getSimpleName().toString();
String preferredMethodName = getPreferredMethod(cmpType, isStatic, state);
if (actualMethodName.equals(preferredMethodName)) {
return Optional.empty();
}
return Optional.of(
suggestFix(
tree, prefixTypeArgumentsIfRelevant(preferredMethodName, tree, cmpType, state), state));
}
/**

View File

@@ -63,6 +63,7 @@ public final class RequestMappingAnnotation extends BugChecker implements Method
AT_LEAST_ONE,
anyOf(
isType(ANN_PACKAGE_PREFIX + "PathVariable"),
isType(ANN_PACKAGE_PREFIX + "RequestAttribute"),
isType(ANN_PACKAGE_PREFIX + "RequestBody"),
isType(ANN_PACKAGE_PREFIX + "RequestHeader"),
isType(ANN_PACKAGE_PREFIX + "RequestParam"))),

View File

@@ -66,6 +66,7 @@ public final class StaticImport extends BugChecker implements MemberSelectTreeMa
"com.google.errorprone.BugPattern.LinkType",
"com.google.errorprone.BugPattern.SeverityLevel",
"com.google.errorprone.BugPattern.StandardTags",
"com.google.errorprone.matchers.Matchers",
"com.google.errorprone.refaster.ImportPolicy",
"com.mongodb.client.model.Accumulators",
"com.mongodb.client.model.Aggregates",

View File

@@ -403,6 +403,32 @@ final class AssertJThrowingCallableTemplates {
}
}
static final class AssertThatThrownByHasMessageParameters {
@BeforeTemplate
@SuppressWarnings("AssertThatThrownBy" /* Matches strictly more specific expressions. */)
AbstractObjectAssert<?, ?> before(
Class<? extends Throwable> exceptionType,
ThrowingCallable throwingCallable,
String message,
@Repeated Object parameters) {
return assertThatExceptionOfType(exceptionType)
.isThrownBy(throwingCallable)
.withMessage(message, parameters);
}
@AfterTemplate
@UseImportPolicy(STATIC_IMPORT_ALWAYS)
AbstractObjectAssert<?, ?> after(
Class<? extends Throwable> exceptionType,
ThrowingCallable throwingCallable,
String message,
@Repeated Object parameters) {
return assertThatThrownBy(throwingCallable)
.isInstanceOf(exceptionType)
.hasMessage(message, parameters);
}
}
// XXX: Drop this template in favour of a generic Error Prone check which flags
// `String.format(...)` arguments to a wide range of format methods.
static final class AbstractThrowableAssertHasMessage {

View File

@@ -239,9 +239,16 @@ final class OptionalTemplates {
}
}
/** Within a stream's map operation unconditional {@link Optional#get()} calls can be avoided. */
// XXX: An alternative approach is to `.flatMap(Optional::stream)`. That may be a bit longer, but
// yield nicer code. Think about it.
/**
* Within a stream's map operation unconditional {@link Optional#orElseThrow()} calls can be
* avoided.
*
* <p><strong>Warning:</strong> this rewrite rule is not completely behavior preserving. The
* original code throws an exception if the mapping operation does not produce a value, while the
* replacement does not.
*/
// XXX: An alternative approach is to use `.flatMap(Optional::stream)`. That may be a bit longer,
// but yields nicer code. Think about it.
abstract static class StreamMapToOptionalGet<T, S> {
@Placeholder
abstract Optional<S> toOptionalFunction(@MayOptionallyUse T element);
@@ -311,6 +318,7 @@ final class OptionalTemplates {
/** Prefer {@link Optional#or(Supplier)} over more verbose alternatives. */
abstract static class OptionalOrOtherOptional<T> {
@BeforeTemplate
@SuppressWarnings("NestedOptionals" /* Auto-fix for the `NestedOptionals` check. */)
Optional<T> before(Optional<T> optional1, Optional<T> optional2) {
// XXX: Note that rewriting the first and third variant will change the code's behavior if
// `optional2` has side-effects.

View File

@@ -0,0 +1,182 @@
package tech.picnic.errorprone.bugpatterns;
import com.google.errorprone.BugCheckerRefactoringTestHelper;
import com.google.errorprone.BugCheckerRefactoringTestHelper.TestMode;
import com.google.errorprone.CompilationTestHelper;
import org.junit.jupiter.api.Test;
final class ImmutablesSortedSetComparatorTest {
private final CompilationTestHelper compilationTestHelper =
CompilationTestHelper.newInstance(ImmutablesSortedSetComparator.class, getClass());
private final BugCheckerRefactoringTestHelper refactoringTestHelper =
BugCheckerRefactoringTestHelper.newInstance(ImmutablesSortedSetComparator.class, getClass());
@Test
void identification() {
compilationTestHelper
.addSourceLines(
"A.java",
"import com.google.common.collect.ContiguousSet;",
"import com.google.common.collect.ImmutableSet;",
"import com.google.common.collect.ImmutableSortedSet;",
"import java.util.NavigableSet;",
"import java.util.Set;",
"import java.util.SortedSet;",
"import java.util.TreeSet;",
"import org.immutables.value.Value;",
"",
"interface A {",
" @Value.Immutable",
" interface ImmutableInterface {",
" Set<String> set();",
"",
" // BUG: Diagnostic contains:",
" SortedSet<String> sortedSet();",
"",
" @Value.NaturalOrder",
" SortedSet<String> sortedSet2();",
" }",
"",
" @Value.Modifiable",
" interface ModifiableInterfaceWithDefaults {",
" @Value.Default",
" default Set<Integer> set() {",
" return new TreeSet<>();",
" }",
"",
" @Value.Default",
" // BUG: Diagnostic contains:",
" default NavigableSet<Integer> navigableSet() {",
" return new TreeSet<>();",
" }",
"",
" @Value.Default",
" @Value.ReverseOrder",
" default NavigableSet<Integer> navigableSet2() {",
" return new TreeSet<>();",
" }",
"",
" default NavigableSet<Integer> nonPropertyNavigableSet() {",
" return new TreeSet<>();",
" }",
" }",
"",
" interface NonImmutablesInterface {",
" SortedSet<String> sortedSet();",
" }",
"",
" @Value.Immutable",
" abstract class AbstractImmutableWithDefaults {",
" @Value.Default",
" ImmutableSet<Integer> immutableSet() {",
" return ImmutableSet.of();",
" }",
"",
" @Value.Default",
" // BUG: Diagnostic contains:",
" ImmutableSortedSet<String> immutableSortedSet() {",
" return ImmutableSortedSet.of();",
" }",
"",
" @Value.Default",
" @Value.NaturalOrder",
" ImmutableSortedSet<String> immutableSortedSet2() {",
" return ImmutableSortedSet.of();",
" }",
"",
" ImmutableSortedSet<String> nonPropertyImmutableSortedSet() {",
" return ImmutableSortedSet.of();",
" }",
" }",
"",
" @Value.Modifiable",
" abstract class AbstractModifiable {",
" abstract ImmutableSet<Integer> immutableSet();",
"",
" // BUG: Diagnostic contains:",
" abstract ContiguousSet<Integer> contiguousSet();",
"",
" @Value.ReverseOrder",
" abstract ContiguousSet<Integer> contiguousSet2();",
" }",
"",
" abstract class AbstractNonImmutables {",
" abstract SortedSet<Integer> sortedSet();",
" }",
"}")
.doTest();
}
@Test
void replacement() {
refactoringTestHelper
.addInputLines(
"A.java",
"import com.google.common.collect.ImmutableSortedSet;",
"import java.util.SortedSet;",
"import org.immutables.value.Value;",
"",
"@Value.Immutable",
"abstract class A {",
" abstract ImmutableSortedSet<String> sortedSet();",
"",
" @Value.Modifiable",
" interface B {",
" SortedSet<String> sortedSet();",
" }",
"}")
.addOutputLines(
"A.java",
"import com.google.common.collect.ImmutableSortedSet;",
"import java.util.SortedSet;",
"import org.immutables.value.Value;",
"",
"@Value.Immutable",
"abstract class A {",
" @Value.NaturalOrder",
" abstract ImmutableSortedSet<String> sortedSet();",
"",
" @Value.Modifiable",
" interface B {",
" @Value.NaturalOrder",
" SortedSet<String> sortedSet();",
" }",
"}")
.doTest(TestMode.TEXT_MATCH);
}
@Test
void replacementWithImportClash() {
refactoringTestHelper
.addInputLines(
"MySpringService.java",
"import com.google.common.collect.ImmutableSortedSet;",
"import org.springframework.beans.factory.annotation.Value;",
"",
"class MySpringService {",
" MySpringService(@Value(\"${someProperty}\") String prop) {}",
" ;",
"",
" @org.immutables.value.Value.Immutable",
" interface A {",
" ImmutableSortedSet<String> sortedSet();",
" }",
"}")
.addOutputLines(
"MySpringService.java",
"import com.google.common.collect.ImmutableSortedSet;",
"import org.springframework.beans.factory.annotation.Value;",
"",
"class MySpringService {",
" MySpringService(@Value(\"${someProperty}\") String prop) {}",
" ;",
"",
" @org.immutables.value.Value.Immutable",
" interface A {",
" @org.immutables.value.Value.NaturalOrder",
" ImmutableSortedSet<String> sortedSet();",
" }",
"}")
.doTest(TestMode.TEXT_MATCH);
}
}

View File

@@ -35,6 +35,8 @@ final class LexicographicalAnnotationAttributeListingTest {
"import io.swagger.v3.oas.annotations.Parameters;",
"import java.math.RoundingMode;",
"import javax.xml.bind.annotation.XmlType;",
"import org.springframework.context.annotation.PropertySource;",
"import org.springframework.test.context.TestPropertySource;",
"",
"interface A {",
" @interface Foo {",
@@ -144,7 +146,16 @@ final class LexicographicalAnnotationAttributeListingTest {
" A secondEndpoint();",
"",
" @XmlType(propOrder = {\"field2\", \"field1\"})",
" class Dummy {}",
" class XmlTypeDummy {}",
"",
" @PropertySource({\"field2\", \"field1\"})",
" class PropertySourceDummy {}",
"",
" @TestPropertySource(locations = {\"field2\", \"field1\"})",
" class FirstTestPropertySourceDummy {}",
"",
" @TestPropertySource({\"field2\", \"field1\"})",
" class SecondTestPropertySourceDummy {}",
"}")
.doTest();
}

View File

@@ -0,0 +1,42 @@
package tech.picnic.errorprone.bugpatterns;
import com.google.errorprone.CompilationTestHelper;
import org.junit.jupiter.api.Test;
final class NestedOptionalsTest {
private final CompilationTestHelper compilationTestHelper =
CompilationTestHelper.newInstance(NestedOptionals.class, getClass());
@Test
void identification() {
compilationTestHelper
.addSourceLines(
"A.java",
"import java.util.Optional;",
"import java.util.stream.Stream;",
"",
"class A {",
" void m() {",
" Optional.empty();",
" Optional.of(1);",
" // BUG: Diagnostic contains:",
" Optional.of(Optional.empty());",
" // BUG: Diagnostic contains:",
" Optional.of(Optional.of(1));",
"",
" Optional.ofNullable(null);",
" // BUG: Diagnostic contains:",
" Optional.ofNullable((Optional) null);",
"",
" Optional.of(\"foo\").map(String::length);",
" // BUG: Diagnostic contains:",
" Optional.of(\"foo\").map(Optional::of);",
"",
" Stream.of(\"foo\").findFirst();",
" // BUG: Diagnostic contains:",
" Stream.of(\"foo\").map(Optional::of).findFirst();",
" }",
"}")
.doTest();
}
}

View File

@@ -0,0 +1,155 @@
package tech.picnic.errorprone.bugpatterns;
import static com.google.errorprone.BugCheckerRefactoringTestHelper.TestMode.TEXT_MATCH;
import com.google.errorprone.BugCheckerRefactoringTestHelper;
import com.google.errorprone.CompilationTestHelper;
import org.junit.jupiter.api.Test;
final class NonEmptyMonoTest {
private final CompilationTestHelper compilationTestHelper =
CompilationTestHelper.newInstance(NonEmptyMono.class, getClass());
private final BugCheckerRefactoringTestHelper refactoringTestHelper =
BugCheckerRefactoringTestHelper.newInstance(NonEmptyMono.class, getClass());
@Test
void identification() {
compilationTestHelper
.addSourceLines(
"A.java",
"import static com.google.common.collect.ImmutableList.toImmutableList;",
"import static java.util.function.Function.identity;",
"",
"import com.google.common.collect.ImmutableList;",
"import com.google.common.collect.ImmutableMap;",
"import java.util.ArrayList;",
"import java.util.HashMap;",
"import java.util.List;",
"import reactor.core.publisher.Flux;",
"import reactor.core.publisher.Mono;",
"",
"class A {",
" void m() {",
" Mono.just(1).defaultIfEmpty(2);",
" Mono.just(1).single();",
" Mono.just(1).switchIfEmpty(Mono.just(2));",
"",
" // BUG: Diagnostic contains:",
" Flux.just(1).all(x -> true).defaultIfEmpty(true);",
"",
" // BUG: Diagnostic contains:",
" Flux.just(1).any(x -> true).single();",
"",
" // BUG: Diagnostic contains:",
" Flux.just(1).collect(toImmutableList()).switchIfEmpty(Mono.just(ImmutableList.of()));",
" // BUG: Diagnostic contains:",
" Flux.just(1).collect(ArrayList::new, List::add).defaultIfEmpty(new ArrayList<>());",
"",
" // BUG: Diagnostic contains:",
" Flux.just(1).collectList().single();",
"",
" // BUG: Diagnostic contains:",
" Flux.just(1).collectMap(identity()).switchIfEmpty(Mono.just(ImmutableMap.of()));",
" // BUG: Diagnostic contains:",
" Flux.just(1).collectMap(identity(), identity()).defaultIfEmpty(ImmutableMap.of());",
" // BUG: Diagnostic contains:",
" Flux.just(1).collectMap(identity(), identity(), HashMap::new).single();",
"",
" // BUG: Diagnostic contains:",
" Flux.just(1).collectMultimap(identity()).switchIfEmpty(Mono.just(ImmutableMap.of()));",
" // BUG: Diagnostic contains:",
" Flux.just(1).collectMultimap(identity(), identity()).defaultIfEmpty(ImmutableMap.of());",
" // BUG: Diagnostic contains:",
" Flux.just(1).collectMultimap(identity(), identity(), HashMap::new).single();",
"",
" // BUG: Diagnostic contains:",
" Flux.just(1).collectSortedList().defaultIfEmpty(ImmutableList.of());",
" // BUG: Diagnostic contains:",
" Flux.just(1).collectSortedList((o1, o2) -> 0).single();",
"",
" // BUG: Diagnostic contains:",
" Flux.just(1).count().switchIfEmpty(Mono.just(2L));",
"",
" // BUG: Diagnostic contains:",
" Flux.just(1).elementAt(0).defaultIfEmpty(1);",
" // BUG: Diagnostic contains:",
" Flux.just(1).elementAt(0, 2).single();",
"",
" // BUG: Diagnostic contains:",
" Flux.just(1).hasElement(2).switchIfEmpty(Mono.just(true));",
"",
" // BUG: Diagnostic contains:",
" Flux.just(1).hasElements().defaultIfEmpty(true);",
"",
" // BUG: Diagnostic contains:",
" Flux.just(1).last().single();",
" // BUG: Diagnostic contains:",
" Flux.just(1).last(2).switchIfEmpty(Mono.just(3));",
"",
" // BUG: Diagnostic contains:",
" Flux.just(1).reduceWith(() -> 0, Integer::sum).defaultIfEmpty(2);",
"",
" // BUG: Diagnostic contains:",
" Flux.just(1).single().single();",
" // BUG: Diagnostic contains:",
" Flux.just(1).single(2).switchIfEmpty(Mono.just(3));",
"",
" Flux.just(1).reduce(Integer::sum).defaultIfEmpty(2);",
" // BUG: Diagnostic contains:",
" Flux.just(1).reduce(2, Integer::sum).single();",
"",
" // BUG: Diagnostic contains:",
" Mono.just(1).defaultIfEmpty(1).switchIfEmpty(Mono.just(2));",
" // BUG: Diagnostic contains:",
" Mono.just(1).hasElement().defaultIfEmpty(true);",
" // BUG: Diagnostic contains:",
" Mono.just(1).single().single();",
" }",
"}")
.doTest();
}
@Test
void replacement() {
refactoringTestHelper
.addInputLines(
"A.java",
"import static com.google.common.collect.ImmutableList.toImmutableList;",
"",
"import com.google.common.collect.ImmutableList;",
"import reactor.core.publisher.Flux;",
"import reactor.core.publisher.Mono;",
"",
"class A {",
" void m() {",
" Flux.just(1).collect(toImmutableList()).single();",
" Flux.just(1).collect(toImmutableList()).defaultIfEmpty(ImmutableList.of());",
" Flux.just(1).collect(toImmutableList()).switchIfEmpty(Mono.just(ImmutableList.of()));",
"",
" Mono.just(2).hasElement().single();",
" Mono.just(2).hasElement().defaultIfEmpty(true);",
" Mono.just(2).hasElement().switchIfEmpty(Mono.just(true));",
" }",
"}")
.addOutputLines(
"A.java",
"import static com.google.common.collect.ImmutableList.toImmutableList;",
"",
"import com.google.common.collect.ImmutableList;",
"import reactor.core.publisher.Flux;",
"import reactor.core.publisher.Mono;",
"",
"class A {",
" void m() {",
" Flux.just(1).collect(toImmutableList());",
" Flux.just(1).collect(toImmutableList());",
" Flux.just(1).collect(toImmutableList());",
"",
" Mono.just(2).hasElement();",
" Mono.just(2).hasElement();",
" Mono.just(2).hasElement();",
" }",
"}")
.doTest(TEXT_MATCH);
}
}

View File

@@ -792,4 +792,39 @@ final class PrimitiveComparisonTest {
"}")
.doTest(TestMode.TEXT_MATCH);
}
// XXX: This still fails: all replacements start at the same place...
@Test
void testReplacementOfMultipleSubexpressions() {
refactoringTestHelper
.addInputLines(
"in/A.java",
"import java.util.Comparator;",
"",
"interface A extends Comparable<A> {",
" Comparator<A> cmp = Comparator.<A, A>comparing(o -> o)",
" .thenComparingInt(o -> Byte.valueOf((byte) 0))",
" .thenComparingInt(o -> Character.valueOf((char) 0))",
" .thenComparingInt(o -> Short.valueOf((short) 0))",
" .thenComparingInt(o -> Integer.valueOf(0))",
" .thenComparingLong(o -> Long.valueOf(0))",
" .thenComparingDouble(o -> Float.valueOf(0))",
" .thenComparingDouble(o -> Double.valueOf(0));",
"}")
.addOutputLines(
"out/A.java",
"import java.util.Comparator;",
"",
"interface A extends Comparable<A> {",
" Comparator<A> cmp = Comparator.<A, A>comparing(o -> o) ",
" .thenComparing(o -> Byte.valueOf((byte) 0))",
" .thenComparing(o -> Character.valueOf((char) 0))",
" .thenComparing(o -> Short.valueOf((short) 0))",
" .thenComparing(o -> Integer.valueOf(0))",
" .thenComparing(o -> Long.valueOf(0))",
" .thenComparing(o -> Float.valueOf(0))",
" .thenComparing(o -> Double.valueOf(0));",
"}")
.doTest(TestMode.TEXT_MATCH);
}
}

View File

@@ -25,6 +25,7 @@ final class RequestMappingAnnotationTest {
"import org.springframework.web.bind.annotation.PathVariable;",
"import org.springframework.web.bind.annotation.PostMapping;",
"import org.springframework.web.bind.annotation.PutMapping;",
"import org.springframework.web.bind.annotation.RequestAttribute;",
"import org.springframework.web.bind.annotation.RequestBody;",
"import org.springframework.web.bind.annotation.RequestHeader;",
"import org.springframework.web.bind.annotation.RequestMapping;",
@@ -47,12 +48,15 @@ final class RequestMappingAnnotationTest {
" A properPathVariable(@PathVariable String param);",
"",
" @PatchMapping",
" A properRequestBody(@RequestBody String body);",
" A properRequestAttribute(@RequestAttribute String attribute);",
"",
" @PostMapping",
" A properRequestHeader(@RequestHeader String header);",
" A properRequestBody(@RequestBody String body);",
"",
" @PutMapping",
" A properRequestHeader(@RequestHeader String header);",
"",
" @RequestMapping",
" A properRequestParam(@RequestParam String param);",
"",
" @RequestMapping",

View File

@@ -99,7 +99,6 @@ final class AssertJThrowingCallableTemplatesTest implements RefasterTemplateTest
return assertThatIOException().isThrownBy(() -> {}).withMessage("foo");
}
@SuppressWarnings("AssertThatThrownByIOException")
AbstractObjectAssert<?, ?> testAssertThatThrownByIOExceptionHasMessageParameters() {
return assertThatIOException().isThrownBy(() -> {}).withMessage("foo %s", "bar");
}
@@ -114,6 +113,12 @@ final class AssertJThrowingCallableTemplatesTest implements RefasterTemplateTest
.withMessage("foo");
}
AbstractObjectAssert<?, ?> testAssertThatThrownByHasMessageParameters() {
return assertThatExceptionOfType(IllegalArgumentException.class)
.isThrownBy(() -> {})
.withMessage("foo %s", "bar");
}
ImmutableSet<AbstractThrowableAssert<?, ? extends Throwable>>
testAbstractThrowableAssertHasMessage() {
return ImmutableSet.of(

View File

@@ -120,7 +120,6 @@ final class AssertJThrowingCallableTemplatesTest implements RefasterTemplateTest
return assertThatThrownBy(() -> {}).isInstanceOf(IOException.class).hasMessage("foo");
}
@SuppressWarnings("AssertThatThrownByIOException")
AbstractObjectAssert<?, ?> testAssertThatThrownByIOExceptionHasMessageParameters() {
return assertThatThrownBy(() -> {}).isInstanceOf(IOException.class).hasMessage("foo %s", "bar");
}
@@ -135,6 +134,12 @@ final class AssertJThrowingCallableTemplatesTest implements RefasterTemplateTest
.hasMessage("foo");
}
AbstractObjectAssert<?, ?> testAssertThatThrownByHasMessageParameters() {
return assertThatThrownBy(() -> {})
.isInstanceOf(IllegalArgumentException.class)
.hasMessage("foo %s", "bar");
}
ImmutableSet<AbstractThrowableAssert<?, ? extends Throwable>>
testAbstractThrowableAssertHasMessage() {
return ImmutableSet.of(

22
pom.xml
View File

@@ -4,7 +4,7 @@
<groupId>tech.picnic.error-prone-support</groupId>
<artifactId>error-prone-support</artifactId>
<version>0.1.0</version>
<version>0.1.1-SNAPSHOT</version>
<packaging>pom</packaging>
<name>Picnic :: Error Prone Support</name>
@@ -48,7 +48,7 @@
<scm>
<developerConnection>scm:git:git@github.com:PicnicSupermarket/error-prone-support.git</developerConnection>
<tag>v0.1.0</tag>
<tag>HEAD</tag>
<url>https://github.com/PicnicSupermarket/error-prone-support</url>
</scm>
<issueManagement>
@@ -150,8 +150,8 @@
<version.findbugs-format-string>3.0.0</version.findbugs-format-string>
<version.guava-beta-checker>1.0</version.guava-beta-checker>
<version.jdk>11</version.jdk>
<version.maven>3.6.3</version.maven>
<version.mockito>4.6.1</version.mockito>
<version.maven>3.8.6</version.maven>
<version.mockito>4.7.0</version.mockito>
<version.nopen-checker>1.0.1</version.nopen-checker>
<version.nullaway>0.9.9</version.nullaway>
<!-- XXX: Two other dependencies are potentially of interest:
@@ -317,7 +317,7 @@
<dependency>
<groupId>io.projectreactor</groupId>
<artifactId>reactor-bom</artifactId>
<version>2020.0.21</version>
<version>2020.0.22</version>
<type>pom</type>
<scope>import</scope>
</dependency>
@@ -386,6 +386,11 @@
<artifactId>hamcrest-core</artifactId>
<version>2.2</version>
</dependency>
<dependency>
<groupId>org.immutables</groupId>
<artifactId>value-annotations</artifactId>
<version>2.9.0</version>
</dependency>
<dependency>
<groupId>org.junit</groupId>
<artifactId>junit-bom</artifactId>
@@ -420,7 +425,7 @@
<dependency>
<groupId>org.springframework.boot</groupId>
<artifactId>spring-boot-test</artifactId>
<version>2.7.2</version>
<version>2.7.3</version>
</dependency>
<dependency>
<groupId>org.testng</groupId>
@@ -982,7 +987,7 @@
<plugin>
<groupId>org.apache.maven.plugins</groupId>
<artifactId>maven-javadoc-plugin</artifactId>
<version>3.4.0</version>
<version>3.4.1</version>
<configuration>
<!-- All relevant doclint checks are performed during
the compilation phase; no need to recheck during
@@ -1536,9 +1541,6 @@
-Xep:StaticOrDefaultInterfaceMethod:OFF
<!-- We generally discourage `var` use. -->
-Xep:Varifier:OFF
<!-- XXX: This check flags false positives.
See https://github.com/google/error-prone/issues/2679. -->
-Xep:VoidMissingNullable:OFF
-XepOpt:CheckReturnValue:CheckAllConstructors=true
<!-- XXX: Enable once there are fewer
false-positives.

View File

@@ -5,7 +5,7 @@
<parent>
<groupId>tech.picnic.error-prone-support</groupId>
<artifactId>error-prone-support</artifactId>
<version>0.1.0</version>
<version>0.1.1-SNAPSHOT</version>
</parent>
<artifactId>refaster-compiler</artifactId>

View File

@@ -28,7 +28,6 @@ import java.io.ObjectOutputStream;
import java.io.UncheckedIOException;
import java.util.List;
import java.util.Map;
import java.util.Optional;
import javax.annotation.Nullable;
import javax.tools.FileObject;
import javax.tools.JavaFileManager;
@@ -73,7 +72,7 @@ final class RefasterRuleCompilerTaskListener implements TaskListener {
return Boolean.TRUE.equals(
new TreeScanner<Boolean, Void>() {
@Override
public Boolean visitAnnotation(AnnotationTree node, Void v) {
public Boolean visitAnnotation(AnnotationTree node, @Nullable Void v) {
Symbol sym = ASTHelpers.getSymbol(node);
return (sym != null
&& sym.getQualifiedName()
@@ -94,7 +93,7 @@ final class RefasterRuleCompilerTaskListener implements TaskListener {
new TreeScanner<Void, Void>() {
@Nullable
@Override
public Void visitClass(ClassTree node, Void v) {
public Void visitClass(ClassTree node, @Nullable Void v) {
rules.putAll(node, RefasterRuleBuilderScanner.extractRules(node, context));
return super.visitClass(node, null);
}
@@ -103,16 +102,10 @@ final class RefasterRuleCompilerTaskListener implements TaskListener {
}
private FileObject getOutputFile(TaskEvent taskEvent, ClassTree tree) throws IOException {
String packageName =
Optional.ofNullable(ASTHelpers.getSymbol(tree))
.map(ASTHelpers::enclosingPackage)
.map(PackageSymbol::toString)
.orElse("");
CharSequence className =
Optional.ofNullable(ASTHelpers.getSymbol(tree))
.map(RefasterRuleCompilerTaskListener::toSimpleFlatName)
.orElseGet(tree::getSimpleName);
String relativeName = className + ".refaster";
ClassSymbol symbol = ASTHelpers.getSymbol(tree);
PackageSymbol enclosingPackage = ASTHelpers.enclosingPackage(symbol);
String packageName = enclosingPackage == null ? "" : enclosingPackage.toString();
String relativeName = toSimpleFlatName(symbol) + ".refaster";
JavaFileManager fileManager = context.get(JavaFileManager.class);
return fileManager.getFileForOutput(

View File

@@ -5,7 +5,7 @@
<parent>
<groupId>tech.picnic.error-prone-support</groupId>
<artifactId>error-prone-support</artifactId>
<version>0.1.0</version>
<version>0.1.1-SNAPSHOT</version>
</parent>
<artifactId>refaster-runner</artifactId>

View File

@@ -5,7 +5,7 @@
<parent>
<groupId>tech.picnic.error-prone-support</groupId>
<artifactId>error-prone-support</artifactId>
<version>0.1.0</version>
<version>0.1.1-SNAPSHOT</version>
</parent>
<artifactId>refaster-support</artifactId>

View File

@@ -1,14 +1,15 @@
package tech.picnic.errorprone.refaster.util;
import static com.google.errorprone.matchers.Matchers.isArrayType;
import com.google.errorprone.VisitorState;
import com.google.errorprone.matchers.Matcher;
import com.google.errorprone.matchers.Matchers;
import com.sun.source.tree.ExpressionTree;
/** A matcher of array-typed expressions, for use with Refaster's {@code @Matches} annotation. */
public final class IsArray implements Matcher<ExpressionTree> {
private static final long serialVersionUID = 1L;
private static final Matcher<ExpressionTree> DELEGATE = Matchers.isArrayType();
private static final Matcher<ExpressionTree> DELEGATE = isArrayType();
@Override
public boolean matches(ExpressionTree tree, VisitorState state) {

View File

@@ -5,7 +5,7 @@
<parent>
<groupId>tech.picnic.error-prone-support</groupId>
<artifactId>error-prone-support</artifactId>
<version>0.1.0</version>
<version>0.1.1-SNAPSHOT</version>
</parent>
<artifactId>refaster-test-support</artifactId>