From 52699571026e6e5d33868109f5efe1c51514eb2b Mon Sep 17 00:00:00 2001 From: mlrprananta Date: Fri, 10 Mar 2023 15:33:20 +0100 Subject: [PATCH 1/9] Prefer Optional#orElseGet over Optional#orElse for non-compile-time input --- .../refasterrules/OptionalRules.java | 18 ++++++++++++++++++ .../refasterrules/OptionalRulesTestInput.java | 4 ++++ .../refasterrules/OptionalRulesTestOutput.java | 5 +++++ 3 files changed, 27 insertions(+) diff --git a/error-prone-contrib/src/main/java/tech/picnic/errorprone/refasterrules/OptionalRules.java b/error-prone-contrib/src/main/java/tech/picnic/errorprone/refasterrules/OptionalRules.java index 86fbe12501..6ed7721d56 100644 --- a/error-prone-contrib/src/main/java/tech/picnic/errorprone/refasterrules/OptionalRules.java +++ b/error-prone-contrib/src/main/java/tech/picnic/errorprone/refasterrules/OptionalRules.java @@ -4,10 +4,12 @@ import com.google.common.collect.Streams; import com.google.errorprone.annotations.CanIgnoreReturnValue; +import com.google.errorprone.matchers.CompileTimeConstantExpressionMatcher; 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.util.Comparator; @@ -224,6 +226,22 @@ T after(Optional o1, Optional o2) { } } + /** + * Prefer {@link Optional#orElseGet(Supplier)} over {@link Optional#orElse(Object)} if the given + * value is not a compile-time constant. + */ + abstract static class OrElseToOrElseGet { + @BeforeTemplate + T before(Optional o, @NotMatches(CompileTimeConstantExpressionMatcher.class) T value) { + return o.orElse(value); + } + + @AfterTemplate + T after(Optional o, T value) { + return o.orElseGet(() -> value); + } + } + /** * Flatten a stream of {@link Optional}s using {@link Optional#stream()}, rather than using one of * the more verbose alternatives. diff --git a/error-prone-contrib/src/test/resources/tech/picnic/errorprone/refasterrules/OptionalRulesTestInput.java b/error-prone-contrib/src/test/resources/tech/picnic/errorprone/refasterrules/OptionalRulesTestInput.java index 16c4d856c7..987d096f27 100644 --- a/error-prone-contrib/src/test/resources/tech/picnic/errorprone/refasterrules/OptionalRulesTestInput.java +++ b/error-prone-contrib/src/test/resources/tech/picnic/errorprone/refasterrules/OptionalRulesTestInput.java @@ -119,4 +119,8 @@ ImmutableSet> testOptionalFilter() { Optional testOptionalMap() { return Optional.of(1).stream().map(String::valueOf).findAny(); } + + ImmutableSet testOrElseToOrElseGet() { + return ImmutableSet.of(Optional.of("foo").orElse("bar"), Optional.of("foo").orElse(toString())); + } } diff --git a/error-prone-contrib/src/test/resources/tech/picnic/errorprone/refasterrules/OptionalRulesTestOutput.java b/error-prone-contrib/src/test/resources/tech/picnic/errorprone/refasterrules/OptionalRulesTestOutput.java index a24fb5e8f9..84be72394c 100644 --- a/error-prone-contrib/src/test/resources/tech/picnic/errorprone/refasterrules/OptionalRulesTestOutput.java +++ b/error-prone-contrib/src/test/resources/tech/picnic/errorprone/refasterrules/OptionalRulesTestOutput.java @@ -112,4 +112,9 @@ ImmutableSet> testOptionalFilter() { Optional testOptionalMap() { return Optional.of(1).map(String::valueOf); } + + ImmutableSet testOrElseToOrElseGet() { + return ImmutableSet.of( + Optional.of("foo").orElse("bar"), Optional.of("foo").orElseGet(() -> toString())); + } } From 435f1022f2719db666c5fd593f7285614fd7fde6 Mon Sep 17 00:00:00 2001 From: Stephan Schroevers Date: Sun, 12 Mar 2023 11:17:56 +0100 Subject: [PATCH 2/9] Tweak --- .../tech/picnic/errorprone/refasterrules/OptionalRules.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/error-prone-contrib/src/main/java/tech/picnic/errorprone/refasterrules/OptionalRules.java b/error-prone-contrib/src/main/java/tech/picnic/errorprone/refasterrules/OptionalRules.java index 6ed7721d56..ae403c5d28 100644 --- a/error-prone-contrib/src/main/java/tech/picnic/errorprone/refasterrules/OptionalRules.java +++ b/error-prone-contrib/src/main/java/tech/picnic/errorprone/refasterrules/OptionalRules.java @@ -230,7 +230,7 @@ T after(Optional o1, Optional o2) { * Prefer {@link Optional#orElseGet(Supplier)} over {@link Optional#orElse(Object)} if the given * value is not a compile-time constant. */ - abstract static class OrElseToOrElseGet { + static final class OrElseToOrElseGet { @BeforeTemplate T before(Optional o, @NotMatches(CompileTimeConstantExpressionMatcher.class) T value) { return o.orElse(value); From d7274f4f16a4c5df61e87946ed3cc4491f505252 Mon Sep 17 00:00:00 2001 From: mlrprananta Date: Tue, 4 Apr 2023 23:19:23 +0200 Subject: [PATCH 3/9] Suggestions --- .../refasterrules/OptionalRules.java | 8 +- .../refasterrules/OptionalRulesTestInput.java | 6 +- .../OptionalRulesTestOutput.java | 5 +- .../IsMethodInvocationWithTwoOrMoreArgs.java | 24 ++++++ ...MethodInvocationWithTwoOrMoreArgsTest.java | 73 +++++++++++++++++++ 5 files changed, 111 insertions(+), 5 deletions(-) create mode 100644 refaster-support/src/main/java/tech/picnic/errorprone/refaster/matchers/IsMethodInvocationWithTwoOrMoreArgs.java create mode 100644 refaster-support/src/test/java/tech/picnic/errorprone/refaster/matchers/IsMethodInvocationWithTwoOrMoreArgsTest.java diff --git a/error-prone-contrib/src/main/java/tech/picnic/errorprone/refasterrules/OptionalRules.java b/error-prone-contrib/src/main/java/tech/picnic/errorprone/refasterrules/OptionalRules.java index ae403c5d28..debfefaef3 100644 --- a/error-prone-contrib/src/main/java/tech/picnic/errorprone/refasterrules/OptionalRules.java +++ b/error-prone-contrib/src/main/java/tech/picnic/errorprone/refasterrules/OptionalRules.java @@ -8,8 +8,8 @@ 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.Matches; 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.util.Comparator; @@ -21,6 +21,7 @@ import java.util.stream.Stream; import org.jspecify.annotations.Nullable; import tech.picnic.errorprone.refaster.annotation.OnlineDocumentation; +import tech.picnic.errorprone.refaster.matchers.IsMethodInvocationWithTwoOrMoreArgs; /** Refaster rules related to expressions dealing with {@link Optional}s. */ @OnlineDocumentation @@ -164,7 +165,8 @@ Optional after(T input) { static final class MapOptionalToBoolean { @BeforeTemplate boolean before(Optional optional, Function predicate) { - return optional.map(predicate).orElse(Refaster.anyOf(false, Boolean.FALSE)); + return Refaster.anyOf( + optional.map(predicate).orElse(false), optional.map(predicate).orElse(Boolean.FALSE)); } @AfterTemplate @@ -232,7 +234,7 @@ T after(Optional o1, Optional o2) { */ static final class OrElseToOrElseGet { @BeforeTemplate - T before(Optional o, @NotMatches(CompileTimeConstantExpressionMatcher.class) T value) { + T before(Optional o, @Matches(IsMethodInvocationWithTwoOrMoreArgs.class) T value) { return o.orElse(value); } diff --git a/error-prone-contrib/src/test/resources/tech/picnic/errorprone/refasterrules/OptionalRulesTestInput.java b/error-prone-contrib/src/test/resources/tech/picnic/errorprone/refasterrules/OptionalRulesTestInput.java index 987d096f27..654b3f9f22 100644 --- a/error-prone-contrib/src/test/resources/tech/picnic/errorprone/refasterrules/OptionalRulesTestInput.java +++ b/error-prone-contrib/src/test/resources/tech/picnic/errorprone/refasterrules/OptionalRulesTestInput.java @@ -121,6 +121,10 @@ Optional testOptionalMap() { } ImmutableSet testOrElseToOrElseGet() { - return ImmutableSet.of(Optional.of("foo").orElse("bar"), Optional.of("foo").orElse(toString())); + return ImmutableSet.of( + Optional.of("foo").orElse("bar"), + Optional.of("foo").orElse(toString()), + Optional.of("foo").orElse(String.valueOf(true)), + Optional.of("foo").orElse(String.format("%s", "bar"))); } } diff --git a/error-prone-contrib/src/test/resources/tech/picnic/errorprone/refasterrules/OptionalRulesTestOutput.java b/error-prone-contrib/src/test/resources/tech/picnic/errorprone/refasterrules/OptionalRulesTestOutput.java index 84be72394c..f4c3d6b2bc 100644 --- a/error-prone-contrib/src/test/resources/tech/picnic/errorprone/refasterrules/OptionalRulesTestOutput.java +++ b/error-prone-contrib/src/test/resources/tech/picnic/errorprone/refasterrules/OptionalRulesTestOutput.java @@ -115,6 +115,9 @@ Optional testOptionalMap() { ImmutableSet testOrElseToOrElseGet() { return ImmutableSet.of( - Optional.of("foo").orElse("bar"), Optional.of("foo").orElseGet(() -> toString())); + Optional.of("foo").orElse("bar"), + Optional.of("foo").orElse(toString()), + Optional.of("foo").orElse(String.valueOf(true)), + Optional.of("foo").orElseGet(() -> String.format("%s", "bar"))); } } diff --git a/refaster-support/src/main/java/tech/picnic/errorprone/refaster/matchers/IsMethodInvocationWithTwoOrMoreArgs.java b/refaster-support/src/main/java/tech/picnic/errorprone/refaster/matchers/IsMethodInvocationWithTwoOrMoreArgs.java new file mode 100644 index 0000000000..99fb81e5c1 --- /dev/null +++ b/refaster-support/src/main/java/tech/picnic/errorprone/refaster/matchers/IsMethodInvocationWithTwoOrMoreArgs.java @@ -0,0 +1,24 @@ +package tech.picnic.errorprone.refaster.matchers; + +import static com.google.errorprone.matchers.Matchers.anyMethod; +import static com.sun.source.tree.Tree.Kind.METHOD_INVOCATION; + +import com.google.errorprone.VisitorState; +import com.google.errorprone.matchers.Matcher; +import com.sun.source.tree.ExpressionTree; +import com.sun.source.tree.MethodInvocationTree; + +/** A matcher for method invocations with two or more arguments. */ +public final class IsMethodInvocationWithTwoOrMoreArgs implements Matcher { + private static final long serialVersionUID = 1L; + + @Override + public boolean matches(ExpressionTree expressionTree, VisitorState state) { + if (expressionTree.getKind() == METHOD_INVOCATION) { + MethodInvocationTree methodInvocationTree = (MethodInvocationTree) expressionTree; + return anyMethod().matches(methodInvocationTree.getMethodSelect(), state) + && methodInvocationTree.getArguments().size() > 1; + } + return false; + } +} diff --git a/refaster-support/src/test/java/tech/picnic/errorprone/refaster/matchers/IsMethodInvocationWithTwoOrMoreArgsTest.java b/refaster-support/src/test/java/tech/picnic/errorprone/refaster/matchers/IsMethodInvocationWithTwoOrMoreArgsTest.java new file mode 100644 index 0000000000..2831e490a7 --- /dev/null +++ b/refaster-support/src/test/java/tech/picnic/errorprone/refaster/matchers/IsMethodInvocationWithTwoOrMoreArgsTest.java @@ -0,0 +1,73 @@ +package tech.picnic.errorprone.refaster.matchers; + +import static com.google.errorprone.BugPattern.SeverityLevel.ERROR; + +import com.google.errorprone.BugPattern; +import com.google.errorprone.CompilationTestHelper; +import com.google.errorprone.bugpatterns.BugChecker; +import org.junit.jupiter.api.Test; + +final class IsMethodInvocationWithTwoOrMoreArgsTest { + @Test + void matches() { + CompilationTestHelper.newInstance( + IsMethodInvocationWithTwoOrMoreArgsTest.MatcherTestChecker.class, getClass()) + .addSourceLines( + "A.java", + "class A {", + " String foo(String foo, String bar) {", + " return foo + bar;", + " }", + "", + " String foo(String foo) {", + " return foo;", + " }", + "", + " String foo() {", + " return \"foo\";", + " }", + "", + " Boolean negative1() {", + " return Boolean.TRUE;", + " }", + "", + " String negative2() {", + " return \"foo\";", + " }", + "", + " String negative3() {", + " return foo(\"foo\");", + " }", + "", + " String negative4() {", + " return foo();", + " }", + "", + " String positive1() {", + " // BUG: Diagnostic contains:", + " return foo(\"foo\", \"bar\");", + " }", + "", + " String positive2() {", + " // BUG: Diagnostic contains:", + " return String.format(\"%s\", \"foo\");", + " }", + "}") + .doTest(); + } + + /** A {@link BugChecker} that simply delegates to {@link IsCharacter}. */ + @BugPattern( + summary = "Flags expressions matched by `IsMethodInvocationWithTwoOrMoreArgs`", + severity = ERROR) + public static final class MatcherTestChecker extends AbstractMatcherTestChecker { + private static final long serialVersionUID = 1L; + + // XXX: This is a false positive reported by Checkstyle. See + // https://github.com/checkstyle/checkstyle/issues/10161#issuecomment-1242732120. + @SuppressWarnings("RedundantModifier") + public MatcherTestChecker() { + super(new IsMethodInvocationWithTwoOrMoreArgs()); + } + } +} From 2e7cde9e16eb73ffd795743e65b851771ebd5b79 Mon Sep 17 00:00:00 2001 From: mlrprananta Date: Wed, 5 Apr 2023 09:38:59 +0200 Subject: [PATCH 4/9] Fix --- .../refaster/matchers/IsMethodInvocationWithTwoOrMoreArgs.java | 3 +++ 1 file changed, 3 insertions(+) diff --git a/refaster-support/src/main/java/tech/picnic/errorprone/refaster/matchers/IsMethodInvocationWithTwoOrMoreArgs.java b/refaster-support/src/main/java/tech/picnic/errorprone/refaster/matchers/IsMethodInvocationWithTwoOrMoreArgs.java index 99fb81e5c1..dfee05e985 100644 --- a/refaster-support/src/main/java/tech/picnic/errorprone/refaster/matchers/IsMethodInvocationWithTwoOrMoreArgs.java +++ b/refaster-support/src/main/java/tech/picnic/errorprone/refaster/matchers/IsMethodInvocationWithTwoOrMoreArgs.java @@ -12,6 +12,9 @@ public final class IsMethodInvocationWithTwoOrMoreArgs implements Matcher { private static final long serialVersionUID = 1L; + /** Instantiates a new {@link IsMethodInvocationWithTwoOrMoreArgs} instance. */ + public IsMethodInvocationWithTwoOrMoreArgs() {} + @Override public boolean matches(ExpressionTree expressionTree, VisitorState state) { if (expressionTree.getKind() == METHOD_INVOCATION) { From 78a3c79046b454453ae38b38057c01830a28f8be Mon Sep 17 00:00:00 2001 From: mlrprananta Date: Fri, 7 Apr 2023 09:59:36 +0200 Subject: [PATCH 5/9] Suggestions --- .../tech/picnic/errorprone/refasterrules/OptionalRules.java | 3 ++- .../matchers/IsMethodInvocationWithTwoOrMoreArgsTest.java | 2 +- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/error-prone-contrib/src/main/java/tech/picnic/errorprone/refasterrules/OptionalRules.java b/error-prone-contrib/src/main/java/tech/picnic/errorprone/refasterrules/OptionalRules.java index debfefaef3..de3a7e5870 100644 --- a/error-prone-contrib/src/main/java/tech/picnic/errorprone/refasterrules/OptionalRules.java +++ b/error-prone-contrib/src/main/java/tech/picnic/errorprone/refasterrules/OptionalRules.java @@ -230,8 +230,9 @@ T after(Optional o1, Optional o2) { /** * Prefer {@link Optional#orElseGet(Supplier)} over {@link Optional#orElse(Object)} if the given - * value is not a compile-time constant. + * value is a method invocation with two or more arguments. */ + // XXX: Extend rule to all method invocations (with less than 2 arguments). static final class OrElseToOrElseGet { @BeforeTemplate T before(Optional o, @Matches(IsMethodInvocationWithTwoOrMoreArgs.class) T value) { diff --git a/refaster-support/src/test/java/tech/picnic/errorprone/refaster/matchers/IsMethodInvocationWithTwoOrMoreArgsTest.java b/refaster-support/src/test/java/tech/picnic/errorprone/refaster/matchers/IsMethodInvocationWithTwoOrMoreArgsTest.java index 2831e490a7..ba5b1e0b11 100644 --- a/refaster-support/src/test/java/tech/picnic/errorprone/refaster/matchers/IsMethodInvocationWithTwoOrMoreArgsTest.java +++ b/refaster-support/src/test/java/tech/picnic/errorprone/refaster/matchers/IsMethodInvocationWithTwoOrMoreArgsTest.java @@ -56,7 +56,7 @@ IsMethodInvocationWithTwoOrMoreArgsTest.MatcherTestChecker.class, getClass()) .doTest(); } - /** A {@link BugChecker} that simply delegates to {@link IsCharacter}. */ + /** A {@link BugChecker} that simply delegates to {@link IsMethodInvocationWithTwoOrMoreArgs}. */ @BugPattern( summary = "Flags expressions matched by `IsMethodInvocationWithTwoOrMoreArgs`", severity = ERROR) From b68d264a10e72ae3b776c06321d2dba22831dc1e Mon Sep 17 00:00:00 2001 From: mlrprananta Date: Fri, 7 Apr 2023 15:58:09 +0200 Subject: [PATCH 6/9] Suggestions --- .../tech/picnic/errorprone/refasterrules/OptionalRules.java | 2 +- .../picnic/errorprone/refasterrules/OptionalRulesTestInput.java | 2 +- .../errorprone/refasterrules/OptionalRulesTestOutput.java | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/error-prone-contrib/src/main/java/tech/picnic/errorprone/refasterrules/OptionalRules.java b/error-prone-contrib/src/main/java/tech/picnic/errorprone/refasterrules/OptionalRules.java index de3a7e5870..30675cc6b1 100644 --- a/error-prone-contrib/src/main/java/tech/picnic/errorprone/refasterrules/OptionalRules.java +++ b/error-prone-contrib/src/main/java/tech/picnic/errorprone/refasterrules/OptionalRules.java @@ -233,7 +233,7 @@ T after(Optional o1, Optional o2) { * value is a method invocation with two or more arguments. */ // XXX: Extend rule to all method invocations (with less than 2 arguments). - static final class OrElseToOrElseGet { + static final class OptionalOrElseGet { @BeforeTemplate T before(Optional o, @Matches(IsMethodInvocationWithTwoOrMoreArgs.class) T value) { return o.orElse(value); diff --git a/error-prone-contrib/src/test/resources/tech/picnic/errorprone/refasterrules/OptionalRulesTestInput.java b/error-prone-contrib/src/test/resources/tech/picnic/errorprone/refasterrules/OptionalRulesTestInput.java index 654b3f9f22..d644bdfcb0 100644 --- a/error-prone-contrib/src/test/resources/tech/picnic/errorprone/refasterrules/OptionalRulesTestInput.java +++ b/error-prone-contrib/src/test/resources/tech/picnic/errorprone/refasterrules/OptionalRulesTestInput.java @@ -120,7 +120,7 @@ Optional testOptionalMap() { return Optional.of(1).stream().map(String::valueOf).findAny(); } - ImmutableSet testOrElseToOrElseGet() { + ImmutableSet testOptionalOrElseGet() { return ImmutableSet.of( Optional.of("foo").orElse("bar"), Optional.of("foo").orElse(toString()), diff --git a/error-prone-contrib/src/test/resources/tech/picnic/errorprone/refasterrules/OptionalRulesTestOutput.java b/error-prone-contrib/src/test/resources/tech/picnic/errorprone/refasterrules/OptionalRulesTestOutput.java index f4c3d6b2bc..1dcf889d53 100644 --- a/error-prone-contrib/src/test/resources/tech/picnic/errorprone/refasterrules/OptionalRulesTestOutput.java +++ b/error-prone-contrib/src/test/resources/tech/picnic/errorprone/refasterrules/OptionalRulesTestOutput.java @@ -113,7 +113,7 @@ Optional testOptionalMap() { return Optional.of(1).map(String::valueOf); } - ImmutableSet testOrElseToOrElseGet() { + ImmutableSet testOptionalOrElseGet() { return ImmutableSet.of( Optional.of("foo").orElse("bar"), Optional.of("foo").orElse(toString()), From 7e224ada2ca1a40bbd079fdcd47b1b9705ebdc9c Mon Sep 17 00:00:00 2001 From: Rick Ossendrijver Date: Fri, 21 Apr 2023 11:50:24 +0200 Subject: [PATCH 7/9] Suggestions and tweaksg --- .../refasterrules/OptionalRules.java | 8 ++--- .../refasterrules/OptionalRulesTestInput.java | 7 +++-- .../OptionalRulesTestOutput.java | 7 +++-- .../IsMethodInvocationWithTwoOrMoreArgs.java | 11 ++----- ...MethodInvocationWithTwoOrMoreArgsTest.java | 30 ++++++++----------- 5 files changed, 27 insertions(+), 36 deletions(-) diff --git a/error-prone-contrib/src/main/java/tech/picnic/errorprone/refasterrules/OptionalRules.java b/error-prone-contrib/src/main/java/tech/picnic/errorprone/refasterrules/OptionalRules.java index 30675cc6b1..427f88b8b0 100644 --- a/error-prone-contrib/src/main/java/tech/picnic/errorprone/refasterrules/OptionalRules.java +++ b/error-prone-contrib/src/main/java/tech/picnic/errorprone/refasterrules/OptionalRules.java @@ -235,13 +235,13 @@ T after(Optional o1, Optional o2) { // XXX: Extend rule to all method invocations (with less than 2 arguments). static final class OptionalOrElseGet { @BeforeTemplate - T before(Optional o, @Matches(IsMethodInvocationWithTwoOrMoreArgs.class) T value) { - return o.orElse(value); + T before(Optional optional, @Matches(IsMethodInvocationWithTwoOrMoreArgs.class) T value) { + return optional.orElse(value); } @AfterTemplate - T after(Optional o, T value) { - return o.orElseGet(() -> value); + T after(Optional optional, T value) { + return optional.orElseGet(() -> value); } } diff --git a/error-prone-contrib/src/test/resources/tech/picnic/errorprone/refasterrules/OptionalRulesTestInput.java b/error-prone-contrib/src/test/resources/tech/picnic/errorprone/refasterrules/OptionalRulesTestInput.java index d644bdfcb0..7ed4c57924 100644 --- a/error-prone-contrib/src/test/resources/tech/picnic/errorprone/refasterrules/OptionalRulesTestInput.java +++ b/error-prone-contrib/src/test/resources/tech/picnic/errorprone/refasterrules/OptionalRulesTestInput.java @@ -123,8 +123,9 @@ Optional testOptionalMap() { ImmutableSet testOptionalOrElseGet() { return ImmutableSet.of( Optional.of("foo").orElse("bar"), - Optional.of("foo").orElse(toString()), - Optional.of("foo").orElse(String.valueOf(true)), - Optional.of("foo").orElse(String.format("%s", "bar"))); + Optional.of("baz").orElse(toString()), + Optional.of("qux").orElse(String.valueOf(true)), + Optional.of("quux").orElse(String.format("%s", "quuz")), + Optional.of("corge").orElse(String.format("%s", "grault", "garply"))); } } diff --git a/error-prone-contrib/src/test/resources/tech/picnic/errorprone/refasterrules/OptionalRulesTestOutput.java b/error-prone-contrib/src/test/resources/tech/picnic/errorprone/refasterrules/OptionalRulesTestOutput.java index 1dcf889d53..bda11bf1c4 100644 --- a/error-prone-contrib/src/test/resources/tech/picnic/errorprone/refasterrules/OptionalRulesTestOutput.java +++ b/error-prone-contrib/src/test/resources/tech/picnic/errorprone/refasterrules/OptionalRulesTestOutput.java @@ -116,8 +116,9 @@ Optional testOptionalMap() { ImmutableSet testOptionalOrElseGet() { return ImmutableSet.of( Optional.of("foo").orElse("bar"), - Optional.of("foo").orElse(toString()), - Optional.of("foo").orElse(String.valueOf(true)), - Optional.of("foo").orElseGet(() -> String.format("%s", "bar"))); + Optional.of("baz").orElse(toString()), + Optional.of("qux").orElse(String.valueOf(true)), + Optional.of("quux").orElseGet(() -> String.format("%s", "quuz")), + Optional.of("corge").orElseGet(() -> String.format("%s", "grault", "garply"))); } } diff --git a/refaster-support/src/main/java/tech/picnic/errorprone/refaster/matchers/IsMethodInvocationWithTwoOrMoreArgs.java b/refaster-support/src/main/java/tech/picnic/errorprone/refaster/matchers/IsMethodInvocationWithTwoOrMoreArgs.java index dfee05e985..023517858f 100644 --- a/refaster-support/src/main/java/tech/picnic/errorprone/refaster/matchers/IsMethodInvocationWithTwoOrMoreArgs.java +++ b/refaster-support/src/main/java/tech/picnic/errorprone/refaster/matchers/IsMethodInvocationWithTwoOrMoreArgs.java @@ -1,8 +1,5 @@ package tech.picnic.errorprone.refaster.matchers; -import static com.google.errorprone.matchers.Matchers.anyMethod; -import static com.sun.source.tree.Tree.Kind.METHOD_INVOCATION; - import com.google.errorprone.VisitorState; import com.google.errorprone.matchers.Matcher; import com.sun.source.tree.ExpressionTree; @@ -17,11 +14,7 @@ public IsMethodInvocationWithTwoOrMoreArgs() {} @Override public boolean matches(ExpressionTree expressionTree, VisitorState state) { - if (expressionTree.getKind() == METHOD_INVOCATION) { - MethodInvocationTree methodInvocationTree = (MethodInvocationTree) expressionTree; - return anyMethod().matches(methodInvocationTree.getMethodSelect(), state) - && methodInvocationTree.getArguments().size() > 1; - } - return false; + return expressionTree instanceof MethodInvocationTree + && ((MethodInvocationTree) expressionTree).getArguments().size() > 1; } } diff --git a/refaster-support/src/test/java/tech/picnic/errorprone/refaster/matchers/IsMethodInvocationWithTwoOrMoreArgsTest.java b/refaster-support/src/test/java/tech/picnic/errorprone/refaster/matchers/IsMethodInvocationWithTwoOrMoreArgsTest.java index ba5b1e0b11..33de7ecca0 100644 --- a/refaster-support/src/test/java/tech/picnic/errorprone/refaster/matchers/IsMethodInvocationWithTwoOrMoreArgsTest.java +++ b/refaster-support/src/test/java/tech/picnic/errorprone/refaster/matchers/IsMethodInvocationWithTwoOrMoreArgsTest.java @@ -10,23 +10,10 @@ final class IsMethodInvocationWithTwoOrMoreArgsTest { @Test void matches() { - CompilationTestHelper.newInstance( - IsMethodInvocationWithTwoOrMoreArgsTest.MatcherTestChecker.class, getClass()) + CompilationTestHelper.newInstance(MatcherTestChecker.class, getClass()) .addSourceLines( "A.java", "class A {", - " String foo(String foo, String bar) {", - " return foo + bar;", - " }", - "", - " String foo(String foo) {", - " return foo;", - " }", - "", - " String foo() {", - " return \"foo\";", - " }", - "", " Boolean negative1() {", " return Boolean.TRUE;", " }", @@ -36,22 +23,31 @@ IsMethodInvocationWithTwoOrMoreArgsTest.MatcherTestChecker.class, getClass()) " }", "", " String negative3() {", - " return foo(\"foo\");", + " return toString();", " }", "", " String negative4() {", - " return foo();", + " return String.valueOf(1);", " }", "", " String positive1() {", " // BUG: Diagnostic contains:", - " return foo(\"foo\", \"bar\");", + " return m1(\"foo\", \"bar\");", " }", "", " String positive2() {", " // BUG: Diagnostic contains:", " return String.format(\"%s\", \"foo\");", " }", + "", + " String positive3() {", + " // BUG: Diagnostic contains:", + " return String.format(\"%s-%s\", \"foo\", \"bar\");", + " }", + "", + " private static String m1(String foo, String bar) {", + " return foo + bar;", + " }", "}") .doTest(); } From d3f64af717c6579587caf7b92d8f0c8e5f1f7231 Mon Sep 17 00:00:00 2001 From: Stephan Schroevers Date: Sat, 20 May 2023 17:58:13 +0200 Subject: [PATCH 8/9] Suggestions --- .../refasterrules/OptionalRules.java | 28 ++-- .../refasterrules/OptionalRulesTestInput.java | 4 +- .../OptionalRulesTestOutput.java | 4 +- .../matchers/IsLikelyTrivialComputation.java | 92 +++++++++++ .../IsMethodInvocationWithTwoOrMoreArgs.java | 20 --- .../IsLikelyTrivialComputationTest.java | 147 ++++++++++++++++++ ...MethodInvocationWithTwoOrMoreArgsTest.java | 69 -------- 7 files changed, 259 insertions(+), 105 deletions(-) create mode 100644 refaster-support/src/main/java/tech/picnic/errorprone/refaster/matchers/IsLikelyTrivialComputation.java delete mode 100644 refaster-support/src/main/java/tech/picnic/errorprone/refaster/matchers/IsMethodInvocationWithTwoOrMoreArgs.java create mode 100644 refaster-support/src/test/java/tech/picnic/errorprone/refaster/matchers/IsLikelyTrivialComputationTest.java delete mode 100644 refaster-support/src/test/java/tech/picnic/errorprone/refaster/matchers/IsMethodInvocationWithTwoOrMoreArgsTest.java diff --git a/error-prone-contrib/src/main/java/tech/picnic/errorprone/refasterrules/OptionalRules.java b/error-prone-contrib/src/main/java/tech/picnic/errorprone/refasterrules/OptionalRules.java index 427f88b8b0..16a8ab4948 100644 --- a/error-prone-contrib/src/main/java/tech/picnic/errorprone/refasterrules/OptionalRules.java +++ b/error-prone-contrib/src/main/java/tech/picnic/errorprone/refasterrules/OptionalRules.java @@ -8,8 +8,8 @@ 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.Matches; 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.util.Comparator; @@ -21,7 +21,7 @@ import java.util.stream.Stream; import org.jspecify.annotations.Nullable; import tech.picnic.errorprone.refaster.annotation.OnlineDocumentation; -import tech.picnic.errorprone.refaster.matchers.IsMethodInvocationWithTwoOrMoreArgs; +import tech.picnic.errorprone.refaster.matchers.IsLikelyTrivialComputation; /** Refaster rules related to expressions dealing with {@link Optional}s. */ @OnlineDocumentation @@ -121,7 +121,7 @@ Optional after(Iterator it) { /** Prefer {@link Optional#filter(Predicate)} over usage of the ternary operator. */ // XXX: This rule may introduce a compilation error: the `test` expression may reference a // non-effectively final variable, which is not allowed in the replacement lambda expression. - // Maybe our `Refaster` checker should test `compilesWithFix`? + // Review whether a `@Matcher` can be used to avoid this. abstract static class TernaryOperatorOptionalPositiveFiltering { @Placeholder abstract boolean test(T value); @@ -141,7 +141,7 @@ Optional after(T input) { /** Prefer {@link Optional#filter(Predicate)} over usage of the ternary operator. */ // XXX: This rule may introduce a compilation error: the `test` expression may reference a // non-effectively final variable, which is not allowed in the replacement lambda expression. - // Maybe our `Refaster` checker should test `compilesWithFix`? + // Review whether a `@Matcher` can be used to avoid this. abstract static class TernaryOperatorOptionalNegativeFiltering { @Placeholder abstract boolean test(T value); @@ -164,9 +164,9 @@ Optional after(T input) { */ static final class MapOptionalToBoolean { @BeforeTemplate + @SuppressWarnings("OptionalOrElseGet" /* Rule is confused by `Refaster#anyOf` usage. */) boolean before(Optional optional, Function predicate) { - return Refaster.anyOf( - optional.map(predicate).orElse(false), optional.map(predicate).orElse(Boolean.FALSE)); + return optional.map(predicate).orElse(Refaster.anyOf(false, Boolean.FALSE)); } @AfterTemplate @@ -229,13 +229,18 @@ T after(Optional o1, Optional o2) { } /** - * Prefer {@link Optional#orElseGet(Supplier)} over {@link Optional#orElse(Object)} if the given - * value is a method invocation with two or more arguments. + * Prefer {@link Optional#orElseGet(Supplier)} over {@link Optional#orElse(Object)} if the + * fallback value is not the result of a trivial computation. */ - // XXX: Extend rule to all method invocations (with less than 2 arguments). + // XXX: This rule may introduce a compilation error: the `value` expression may reference a + // non-effectively final variable, which is not allowed in the replacement lambda expression. + // Review whether a `@Matcher` can be used to avoid this. + // XXX: Once `MethodReferenceUsage` is "production ready", replace + // `@NotMatches(IsLikelyTrivialComputation.class)` with `@Matches(RequiresComputation.class)` (and + // reimplement the matcher accordingly). static final class OptionalOrElseGet { @BeforeTemplate - T before(Optional optional, @Matches(IsMethodInvocationWithTwoOrMoreArgs.class) T value) { + T before(Optional optional, @NotMatches(IsLikelyTrivialComputation.class) T value) { return optional.orElse(value); } @@ -346,6 +351,9 @@ static final class OptionalOrOtherOptional { Optional before(Optional optional1, Optional optional2) { // XXX: Note that rewriting the first and third variant will change the code's behavior if // `optional2` has side-effects. + // XXX: Note that rewriting the first and third variant will introduce a compilation error if + // `optional2` is not effectively final. Review whether a `@Matcher` can be used to avoid + // this. return Refaster.anyOf( optional1.map(Optional::of).orElse(optional2), optional1.map(Optional::of).orElseGet(() -> optional2), diff --git a/error-prone-contrib/src/test/resources/tech/picnic/errorprone/refasterrules/OptionalRulesTestInput.java b/error-prone-contrib/src/test/resources/tech/picnic/errorprone/refasterrules/OptionalRulesTestInput.java index 7ed4c57924..979717fcfd 100644 --- a/error-prone-contrib/src/test/resources/tech/picnic/errorprone/refasterrules/OptionalRulesTestInput.java +++ b/error-prone-contrib/src/test/resources/tech/picnic/errorprone/refasterrules/OptionalRulesTestInput.java @@ -124,8 +124,6 @@ ImmutableSet testOptionalOrElseGet() { return ImmutableSet.of( Optional.of("foo").orElse("bar"), Optional.of("baz").orElse(toString()), - Optional.of("qux").orElse(String.valueOf(true)), - Optional.of("quux").orElse(String.format("%s", "quuz")), - Optional.of("corge").orElse(String.format("%s", "grault", "garply"))); + Optional.of("qux").orElse(String.valueOf(true))); } } diff --git a/error-prone-contrib/src/test/resources/tech/picnic/errorprone/refasterrules/OptionalRulesTestOutput.java b/error-prone-contrib/src/test/resources/tech/picnic/errorprone/refasterrules/OptionalRulesTestOutput.java index bda11bf1c4..cafc70cfa3 100644 --- a/error-prone-contrib/src/test/resources/tech/picnic/errorprone/refasterrules/OptionalRulesTestOutput.java +++ b/error-prone-contrib/src/test/resources/tech/picnic/errorprone/refasterrules/OptionalRulesTestOutput.java @@ -117,8 +117,6 @@ ImmutableSet testOptionalOrElseGet() { return ImmutableSet.of( Optional.of("foo").orElse("bar"), Optional.of("baz").orElse(toString()), - Optional.of("qux").orElse(String.valueOf(true)), - Optional.of("quux").orElseGet(() -> String.format("%s", "quuz")), - Optional.of("corge").orElseGet(() -> String.format("%s", "grault", "garply"))); + Optional.of("qux").orElseGet(() -> String.valueOf(true))); } } diff --git a/refaster-support/src/main/java/tech/picnic/errorprone/refaster/matchers/IsLikelyTrivialComputation.java b/refaster-support/src/main/java/tech/picnic/errorprone/refaster/matchers/IsLikelyTrivialComputation.java new file mode 100644 index 0000000000..9cfa30b511 --- /dev/null +++ b/refaster-support/src/main/java/tech/picnic/errorprone/refaster/matchers/IsLikelyTrivialComputation.java @@ -0,0 +1,92 @@ +package tech.picnic.errorprone.refaster.matchers; + +import com.google.errorprone.VisitorState; +import com.google.errorprone.matchers.Matcher; +import com.sun.source.tree.ArrayAccessTree; +import com.sun.source.tree.ExpressionTree; +import com.sun.source.tree.IdentifierTree; +import com.sun.source.tree.LambdaExpressionTree; +import com.sun.source.tree.LiteralTree; +import com.sun.source.tree.MemberReferenceTree; +import com.sun.source.tree.MemberSelectTree; +import com.sun.source.tree.MethodInvocationTree; +import com.sun.source.tree.ParenthesizedTree; +import com.sun.source.tree.TypeCastTree; +import com.sun.source.tree.UnaryTree; + +/** A matcher of expressions that likely require little to no computation. */ +public final class IsLikelyTrivialComputation implements Matcher { + private static final long serialVersionUID = 1L; + + /** Instantiates a new {@link IsLikelyTrivialComputation} instance. */ + public IsLikelyTrivialComputation() {} + + @Override + public boolean matches(ExpressionTree expressionTree, VisitorState state) { + if (expressionTree instanceof MethodInvocationTree) { + // XXX: Method invocations are generally *not* trivial computations, but we make an exception + // for nullary method invocations on the result of a trivial computation. This exception + // allows this `Matcher` to by the `OptionalOrElseGet` Refaster rule, such that it does not + // suggest the introduction of lambda expressions that are better expressed as method + // references. Once the `MethodReferenceUsage` bug checker is production-ready, this exception + // should be removed. (But at that point, instead defining a `RequiresComputation` matcher may + // be more appropriate.) + MethodInvocationTree methodInvocation = (MethodInvocationTree) expressionTree; + if (methodInvocation.getArguments().isEmpty() + && matches(methodInvocation.getMethodSelect())) { + return true; + } + } + + return matches(expressionTree); + } + + // XXX: Some `BinaryTree`s may represent what could be considered "trivial computations". + // Depending on feedback such trees may be matched in the future. + private static boolean matches(ExpressionTree expressionTree) { + if (expressionTree instanceof ArrayAccessTree) { + return matches(((ArrayAccessTree) expressionTree).getExpression()) + && matches(((ArrayAccessTree) expressionTree).getIndex()); + } + + if (expressionTree instanceof LiteralTree) { + return true; + } + + if (expressionTree instanceof LambdaExpressionTree) { + /* + * Lambda expressions encapsulate computations, but their definition does not involve + * significant computation. + */ + return true; + } + + if (expressionTree instanceof IdentifierTree) { + return true; + } + + if (expressionTree instanceof MemberReferenceTree) { + return matches(((MemberReferenceTree) expressionTree).getQualifierExpression()); + } + + if (expressionTree instanceof MemberSelectTree) { + return matches(((MemberSelectTree) expressionTree).getExpression()); + } + + if (expressionTree instanceof ParenthesizedTree) { + return matches(((ParenthesizedTree) expressionTree).getExpression()); + } + + if (expressionTree instanceof TypeCastTree) { + return matches(((TypeCastTree) expressionTree).getExpression()); + } + + if (expressionTree instanceof UnaryTree) { + // XXX: Arguably side-effectful options such as pre- and post-increment and -decrement are not + // trivial. + return matches(((UnaryTree) expressionTree).getExpression()); + } + + return false; + } +} diff --git a/refaster-support/src/main/java/tech/picnic/errorprone/refaster/matchers/IsMethodInvocationWithTwoOrMoreArgs.java b/refaster-support/src/main/java/tech/picnic/errorprone/refaster/matchers/IsMethodInvocationWithTwoOrMoreArgs.java deleted file mode 100644 index 023517858f..0000000000 --- a/refaster-support/src/main/java/tech/picnic/errorprone/refaster/matchers/IsMethodInvocationWithTwoOrMoreArgs.java +++ /dev/null @@ -1,20 +0,0 @@ -package tech.picnic.errorprone.refaster.matchers; - -import com.google.errorprone.VisitorState; -import com.google.errorprone.matchers.Matcher; -import com.sun.source.tree.ExpressionTree; -import com.sun.source.tree.MethodInvocationTree; - -/** A matcher for method invocations with two or more arguments. */ -public final class IsMethodInvocationWithTwoOrMoreArgs implements Matcher { - private static final long serialVersionUID = 1L; - - /** Instantiates a new {@link IsMethodInvocationWithTwoOrMoreArgs} instance. */ - public IsMethodInvocationWithTwoOrMoreArgs() {} - - @Override - public boolean matches(ExpressionTree expressionTree, VisitorState state) { - return expressionTree instanceof MethodInvocationTree - && ((MethodInvocationTree) expressionTree).getArguments().size() > 1; - } -} diff --git a/refaster-support/src/test/java/tech/picnic/errorprone/refaster/matchers/IsLikelyTrivialComputationTest.java b/refaster-support/src/test/java/tech/picnic/errorprone/refaster/matchers/IsLikelyTrivialComputationTest.java new file mode 100644 index 0000000000..a140e7248f --- /dev/null +++ b/refaster-support/src/test/java/tech/picnic/errorprone/refaster/matchers/IsLikelyTrivialComputationTest.java @@ -0,0 +1,147 @@ +package tech.picnic.errorprone.refaster.matchers; + +import static com.google.errorprone.BugPattern.SeverityLevel.ERROR; + +import com.google.errorprone.BugPattern; +import com.google.errorprone.CompilationTestHelper; +import com.google.errorprone.bugpatterns.BugChecker; +import com.sun.source.tree.ReturnTree; +import org.junit.jupiter.api.Test; + +final class IsLikelyTrivialComputationTest { + @Test + void matches() { + CompilationTestHelper.newInstance(MatcherTestChecker.class, getClass()) + .addSourceLines( + "A.java", + "import java.util.function.Predicate;", + "", + "class A {", + " String negative1() {", + " return String.valueOf(1);", + " }", + "", + " String negative2() {", + " return toString().toString();", + " }", + "", + " String negative3() {", + " return \"foo\" + toString();", + " }", + "", + " byte negative4() {", + " return \"foo\".getBytes()[0];", + " }", + "", + " int negative5() {", + " int[] arr = new int[0];", + " return arr[hashCode()];", + " }", + "", + " int negative6() {", + " return 1 * 2;", + " }", + "", + " Predicate negative7() {", + " return toString()::equals;", + " }", + "", + " String negative8() {", + " return (toString());", + " }", + "", + " Object negative9() {", + " return (Object) toString();", + " }", + "", + " int negative10() {", + " return -hashCode();", + " }", + "", + " String positive1() {", + " // BUG: Diagnostic contains:", + " return toString();", + " }", + "", + " String positive2() {", + " // BUG: Diagnostic contains:", + " return this.toString();", + " }", + "", + " int positive3() {", + " int[] arr = new int[0];", + " // BUG: Diagnostic contains:", + " return arr[0];", + " }", + "", + " String positive4() {", + " // BUG: Diagnostic contains:", + " return null;", + " }", + "", + " boolean positive5() {", + " // BUG: Diagnostic contains:", + " return false;", + " }", + "", + " int positive6() {", + " // BUG: Diagnostic contains:", + " return 0;", + " }", + "", + " String positive7() {", + " // BUG: Diagnostic contains:", + " return \"foo\" + \"bar\";", + " }", + "", + " Predicate positive8() {", + " // BUG: Diagnostic contains:", + " return v -> \"foo\".equals(v);", + " }", + "", + " A positive9() {", + " // BUG: Diagnostic contains:", + " return this;", + " }", + "", + " Predicate positive10() {", + " // BUG: Diagnostic contains:", + " return \"foo\"::equals;", + " }", + "", + " A positive11() {", + " // BUG: Diagnostic contains:", + " return (this);", + " }", + "", + " Object positive12() {", + " // BUG: Diagnostic contains:", + " return (Object) this;", + " }", + "", + " boolean positive13() {", + " // BUG: Diagnostic contains:", + " return !false;", + " }", + "}") + .doTest(); + } + + /** A {@link BugChecker} that simply delegates to {@link IsLikelyTrivialComputation}. */ + @BugPattern( + summary = "Flags return statement expressions matched by `IsLikelyTrivialComputation`", + severity = ERROR) + public static final class MatcherTestChecker extends AbstractMatcherTestChecker { + private static final long serialVersionUID = 1L; + + // XXX: This is a false positive reported by Checkstyle. See + // https://github.com/checkstyle/checkstyle/issues/10161#issuecomment-1242732120. + @SuppressWarnings("RedundantModifier") + public MatcherTestChecker() { + super( + (expressionTree, state) -> + state.getPath().getParentPath().getLeaf() instanceof ReturnTree + && new IsLikelyTrivialComputation().matches(expressionTree, state)); + } + } +} diff --git a/refaster-support/src/test/java/tech/picnic/errorprone/refaster/matchers/IsMethodInvocationWithTwoOrMoreArgsTest.java b/refaster-support/src/test/java/tech/picnic/errorprone/refaster/matchers/IsMethodInvocationWithTwoOrMoreArgsTest.java deleted file mode 100644 index 33de7ecca0..0000000000 --- a/refaster-support/src/test/java/tech/picnic/errorprone/refaster/matchers/IsMethodInvocationWithTwoOrMoreArgsTest.java +++ /dev/null @@ -1,69 +0,0 @@ -package tech.picnic.errorprone.refaster.matchers; - -import static com.google.errorprone.BugPattern.SeverityLevel.ERROR; - -import com.google.errorprone.BugPattern; -import com.google.errorprone.CompilationTestHelper; -import com.google.errorprone.bugpatterns.BugChecker; -import org.junit.jupiter.api.Test; - -final class IsMethodInvocationWithTwoOrMoreArgsTest { - @Test - void matches() { - CompilationTestHelper.newInstance(MatcherTestChecker.class, getClass()) - .addSourceLines( - "A.java", - "class A {", - " Boolean negative1() {", - " return Boolean.TRUE;", - " }", - "", - " String negative2() {", - " return \"foo\";", - " }", - "", - " String negative3() {", - " return toString();", - " }", - "", - " String negative4() {", - " return String.valueOf(1);", - " }", - "", - " String positive1() {", - " // BUG: Diagnostic contains:", - " return m1(\"foo\", \"bar\");", - " }", - "", - " String positive2() {", - " // BUG: Diagnostic contains:", - " return String.format(\"%s\", \"foo\");", - " }", - "", - " String positive3() {", - " // BUG: Diagnostic contains:", - " return String.format(\"%s-%s\", \"foo\", \"bar\");", - " }", - "", - " private static String m1(String foo, String bar) {", - " return foo + bar;", - " }", - "}") - .doTest(); - } - - /** A {@link BugChecker} that simply delegates to {@link IsMethodInvocationWithTwoOrMoreArgs}. */ - @BugPattern( - summary = "Flags expressions matched by `IsMethodInvocationWithTwoOrMoreArgs`", - severity = ERROR) - public static final class MatcherTestChecker extends AbstractMatcherTestChecker { - private static final long serialVersionUID = 1L; - - // XXX: This is a false positive reported by Checkstyle. See - // https://github.com/checkstyle/checkstyle/issues/10161#issuecomment-1242732120. - @SuppressWarnings("RedundantModifier") - public MatcherTestChecker() { - super(new IsMethodInvocationWithTwoOrMoreArgs()); - } - } -} From ead55db356800e96bfb89895658318d0dc741cab Mon Sep 17 00:00:00 2001 From: Rick Ossendrijver Date: Fri, 26 May 2023 08:34:42 +0200 Subject: [PATCH 9/9] Post-rebase fix --- .../java/tech/picnic/errorprone/refasterrules/OptionalRules.java | 1 - 1 file changed, 1 deletion(-) diff --git a/error-prone-contrib/src/main/java/tech/picnic/errorprone/refasterrules/OptionalRules.java b/error-prone-contrib/src/main/java/tech/picnic/errorprone/refasterrules/OptionalRules.java index 16a8ab4948..2a84eaf419 100644 --- a/error-prone-contrib/src/main/java/tech/picnic/errorprone/refasterrules/OptionalRules.java +++ b/error-prone-contrib/src/main/java/tech/picnic/errorprone/refasterrules/OptionalRules.java @@ -4,7 +4,6 @@ import com.google.common.collect.Streams; import com.google.errorprone.annotations.CanIgnoreReturnValue; -import com.google.errorprone.matchers.CompileTimeConstantExpressionMatcher; import com.google.errorprone.refaster.Refaster; import com.google.errorprone.refaster.annotation.AfterTemplate; import com.google.errorprone.refaster.annotation.BeforeTemplate;