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 9d078d7b754..130880a5a89 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 @@ -3,12 +3,11 @@ import static com.google.errorprone.refaster.ImportPolicy.STATIC_IMPORT_ALWAYS; import com.google.common.collect.Streams; -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.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; @@ -20,6 +19,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 @@ -160,7 +160,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 @@ -228,7 +229,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 0f0c524fffd..060aadb7f41 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 d40efdc5adb..b7f496b1da7 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 00000000000..99fb81e5c11 --- /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 00000000000..2831e490a7e --- /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()); + } + } +}