From 41503079471408b712a98f841748b715848dd056 Mon Sep 17 00:00:00 2001 From: Giovanni Zotta Date: Sun, 12 Feb 2023 19:23:35 +0100 Subject: [PATCH 1/7] Introduce `StreamMapReduceIntSum` Refaster rule --- .../errorprone/refasterrules/StreamRules.java | 13 +++++++++++++ .../refasterrules/StreamRulesTestInput.java | 6 ++++++ .../refasterrules/StreamRulesTestOutput.java | 5 +++++ 3 files changed, 24 insertions(+) diff --git a/error-prone-contrib/src/main/java/tech/picnic/errorprone/refasterrules/StreamRules.java b/error-prone-contrib/src/main/java/tech/picnic/errorprone/refasterrules/StreamRules.java index 251fdea9c6..32c702449c 100644 --- a/error-prone-contrib/src/main/java/tech/picnic/errorprone/refasterrules/StreamRules.java +++ b/error-prone-contrib/src/main/java/tech/picnic/errorprone/refasterrules/StreamRules.java @@ -19,6 +19,7 @@ import java.util.Optional; import java.util.function.Function; import java.util.function.Predicate; +import java.util.function.ToIntFunction; import java.util.stream.Collector; import java.util.stream.Collectors; import java.util.stream.Stream; @@ -379,4 +380,16 @@ boolean after(Stream stream) { return stream.allMatch(e -> test(e)); } } + + static final class StreamMapReduceIntSum { + @BeforeTemplate + int before(Stream stream, Function mapper) { + return stream.map(mapper).reduce(0, Integer::sum); + } + + @AfterTemplate + int after(Stream stream, ToIntFunction mapper) { + return stream.mapToInt(mapper).sum(); + } + } } diff --git a/error-prone-contrib/src/test/resources/tech/picnic/errorprone/refasterrules/StreamRulesTestInput.java b/error-prone-contrib/src/test/resources/tech/picnic/errorprone/refasterrules/StreamRulesTestInput.java index 0f291c343e..b6b8b9cbb9 100644 --- a/error-prone-contrib/src/test/resources/tech/picnic/errorprone/refasterrules/StreamRulesTestInput.java +++ b/error-prone-contrib/src/test/resources/tech/picnic/errorprone/refasterrules/StreamRulesTestInput.java @@ -138,4 +138,10 @@ ImmutableSet testStreamAllMatch() { boolean testStreamAllMatch2() { return Stream.of("foo").noneMatch(s -> !s.isBlank()); } + + ImmutableSet testStreamMapReduceIntSum() { + return ImmutableSet.of( + Stream.of(2).map(i -> i * 2).reduce(0, Integer::sum), + Stream.of("foo").map(String::length).reduce(0, Integer::sum)); + } } diff --git a/error-prone-contrib/src/test/resources/tech/picnic/errorprone/refasterrules/StreamRulesTestOutput.java b/error-prone-contrib/src/test/resources/tech/picnic/errorprone/refasterrules/StreamRulesTestOutput.java index 19e7d9fdda..ab3ea8811f 100644 --- a/error-prone-contrib/src/test/resources/tech/picnic/errorprone/refasterrules/StreamRulesTestOutput.java +++ b/error-prone-contrib/src/test/resources/tech/picnic/errorprone/refasterrules/StreamRulesTestOutput.java @@ -137,4 +137,9 @@ ImmutableSet testStreamAllMatch() { boolean testStreamAllMatch2() { return Stream.of("foo").allMatch(s -> s.isBlank()); } + + ImmutableSet testStreamMapReduceIntSum() { + return ImmutableSet.of( + Stream.of(2).mapToInt(i -> i * 2).sum(), Stream.of("foo").mapToInt(String::length).sum()); + } } From b6137dcce97ed99edf25c762348d7b7d26dfe80b Mon Sep 17 00:00:00 2001 From: Stephan Schroevers Date: Sun, 12 Feb 2023 20:58:24 +0100 Subject: [PATCH 2/7] Suggestions --- .../errorprone/refasterrules/StreamRules.java | 31 ++++++++++++++++++- .../refasterrules/StreamRulesTestInput.java | 18 +++++++++-- .../refasterrules/StreamRulesTestOutput.java | 15 +++++++-- 3 files changed, 58 insertions(+), 6 deletions(-) diff --git a/error-prone-contrib/src/main/java/tech/picnic/errorprone/refasterrules/StreamRules.java b/error-prone-contrib/src/main/java/tech/picnic/errorprone/refasterrules/StreamRules.java index 32c702449c..5c4729b9e9 100644 --- a/error-prone-contrib/src/main/java/tech/picnic/errorprone/refasterrules/StreamRules.java +++ b/error-prone-contrib/src/main/java/tech/picnic/errorprone/refasterrules/StreamRules.java @@ -19,7 +19,9 @@ import java.util.Optional; import java.util.function.Function; import java.util.function.Predicate; +import java.util.function.ToDoubleFunction; import java.util.function.ToIntFunction; +import java.util.function.ToLongFunction; import java.util.stream.Collector; import java.util.stream.Collectors; import java.util.stream.Stream; @@ -381,7 +383,8 @@ boolean after(Stream stream) { } } - static final class StreamMapReduceIntSum { + // XXX: Introduce a `@Matches(IsLambdaExpressionOrMethodReference.class)` guard for the `mapper`. + static final class StreamMapToIntSum { @BeforeTemplate int before(Stream stream, Function mapper) { return stream.map(mapper).reduce(0, Integer::sum); @@ -392,4 +395,30 @@ int after(Stream stream, ToIntFunction mapper) { return stream.mapToInt(mapper).sum(); } } + + // XXX: Introduce a `@Matches(IsLambdaExpressionOrMethodReference.class)` guard for the `mapper`. + static final class StreamMapToDoubleSum { + @BeforeTemplate + double before(Stream stream, Function mapper) { + return stream.map(mapper).reduce(0.0, Double::sum); + } + + @AfterTemplate + double after(Stream stream, ToDoubleFunction mapper) { + return stream.mapToDouble(mapper).sum(); + } + } + + // XXX: Introduce a `@Matches(IsLambdaExpressionOrMethodReference.class)` guard for the `mapper`. + static final class StreamMapToLongSum { + @BeforeTemplate + long before(Stream stream, Function mapper) { + return stream.map(mapper).reduce(0L, Long::sum); + } + + @AfterTemplate + long after(Stream stream, ToLongFunction mapper) { + return stream.mapToLong(mapper).sum(); + } + } } diff --git a/error-prone-contrib/src/test/resources/tech/picnic/errorprone/refasterrules/StreamRulesTestInput.java b/error-prone-contrib/src/test/resources/tech/picnic/errorprone/refasterrules/StreamRulesTestInput.java index b6b8b9cbb9..aab35eb355 100644 --- a/error-prone-contrib/src/test/resources/tech/picnic/errorprone/refasterrules/StreamRulesTestInput.java +++ b/error-prone-contrib/src/test/resources/tech/picnic/errorprone/refasterrules/StreamRulesTestInput.java @@ -139,9 +139,21 @@ boolean testStreamAllMatch2() { return Stream.of("foo").noneMatch(s -> !s.isBlank()); } - ImmutableSet testStreamMapReduceIntSum() { + ImmutableSet testStreamMapToIntSum() { return ImmutableSet.of( - Stream.of(2).map(i -> i * 2).reduce(0, Integer::sum), - Stream.of("foo").map(String::length).reduce(0, Integer::sum)); + Stream.of(1).map(i -> i * 2).reduce(0, Integer::sum), + Stream.of("1").map(Integer::parseInt).reduce(0, Integer::sum)); + } + + ImmutableSet testStreamMapToDoubleSum() { + return ImmutableSet.of( + Stream.of(1).map(i -> i * 2.0).reduce(0.0, Double::sum), + Stream.of("1").map(Double::parseDouble).reduce(0.0, Double::sum)); + } + + ImmutableSet testStreamMapToLongSum() { + return ImmutableSet.of( + Stream.of(1).map(i -> i * 2L).reduce(0L, Long::sum), + Stream.of("1").map(Long::parseLong).reduce(0L, Long::sum)); } } diff --git a/error-prone-contrib/src/test/resources/tech/picnic/errorprone/refasterrules/StreamRulesTestOutput.java b/error-prone-contrib/src/test/resources/tech/picnic/errorprone/refasterrules/StreamRulesTestOutput.java index ab3ea8811f..91dc6108ee 100644 --- a/error-prone-contrib/src/test/resources/tech/picnic/errorprone/refasterrules/StreamRulesTestOutput.java +++ b/error-prone-contrib/src/test/resources/tech/picnic/errorprone/refasterrules/StreamRulesTestOutput.java @@ -138,8 +138,19 @@ boolean testStreamAllMatch2() { return Stream.of("foo").allMatch(s -> s.isBlank()); } - ImmutableSet testStreamMapReduceIntSum() { + ImmutableSet testStreamMapToIntSum() { return ImmutableSet.of( - Stream.of(2).mapToInt(i -> i * 2).sum(), Stream.of("foo").mapToInt(String::length).sum()); + Stream.of(1).mapToInt(i -> i * 2).sum(), Stream.of("1").mapToInt(Integer::parseInt).sum()); + } + + ImmutableSet testStreamMapToDoubleSum() { + return ImmutableSet.of( + Stream.of(1).mapToDouble(i -> i * 2.0).sum(), + Stream.of("1").mapToDouble(Double::parseDouble).sum()); + } + + ImmutableSet testStreamMapToLongSum() { + return ImmutableSet.of( + Stream.of(1).mapToLong(i -> i * 2L).sum(), Stream.of("1").mapToLong(Long::parseLong).sum()); } } From 63cf484958b98beb76d100de02f523aec4e91834 Mon Sep 17 00:00:00 2001 From: Giovanni Zotta Date: Mon, 13 Feb 2023 23:04:36 +0100 Subject: [PATCH 3/7] Annotate mapper with @Matches(IsLambdaExpressionOrMethodReference.class) --- .../errorprone/refasterrules/StreamRules.java | 17 +++++++---- .../refasterrules/StreamRulesTestInput.java | 13 +++++++-- .../refasterrules/StreamRulesTestOutput.java | 15 ++++++++-- .../IsLambdaExpressionOrMethodReference.java | 29 +++++++++++++++++++ 4 files changed, 62 insertions(+), 12 deletions(-) create mode 100644 refaster-support/src/main/java/tech/picnic/errorprone/refaster/matchers/IsLambdaExpressionOrMethodReference.java diff --git a/error-prone-contrib/src/main/java/tech/picnic/errorprone/refasterrules/StreamRules.java b/error-prone-contrib/src/main/java/tech/picnic/errorprone/refasterrules/StreamRules.java index 5c4729b9e9..adc1a53fcf 100644 --- a/error-prone-contrib/src/main/java/tech/picnic/errorprone/refasterrules/StreamRules.java +++ b/error-prone-contrib/src/main/java/tech/picnic/errorprone/refasterrules/StreamRules.java @@ -10,6 +10,7 @@ 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.Placeholder; import com.google.errorprone.refaster.annotation.UseImportPolicy; @@ -26,6 +27,7 @@ import java.util.stream.Collectors; import java.util.stream.Stream; import tech.picnic.errorprone.refaster.annotation.OnlineDocumentation; +import tech.picnic.errorprone.refaster.matchers.IsLambdaExpressionOrMethodReference; /** Refaster rules related to expressions dealing with {@link Stream}s. */ @OnlineDocumentation @@ -383,10 +385,11 @@ boolean after(Stream stream) { } } - // XXX: Introduce a `@Matches(IsLambdaExpressionOrMethodReference.class)` guard for the `mapper`. static final class StreamMapToIntSum { @BeforeTemplate - int before(Stream stream, Function mapper) { + int before( + Stream stream, + @Matches(IsLambdaExpressionOrMethodReference.class) Function mapper) { return stream.map(mapper).reduce(0, Integer::sum); } @@ -396,10 +399,11 @@ int after(Stream stream, ToIntFunction mapper) { } } - // XXX: Introduce a `@Matches(IsLambdaExpressionOrMethodReference.class)` guard for the `mapper`. static final class StreamMapToDoubleSum { @BeforeTemplate - double before(Stream stream, Function mapper) { + double before( + Stream stream, + @Matches(IsLambdaExpressionOrMethodReference.class) Function mapper) { return stream.map(mapper).reduce(0.0, Double::sum); } @@ -409,10 +413,11 @@ static final class StreamMapToDoubleSum { } } - // XXX: Introduce a `@Matches(IsLambdaExpressionOrMethodReference.class)` guard for the `mapper`. static final class StreamMapToLongSum { @BeforeTemplate - long before(Stream stream, Function mapper) { + long before( + Stream stream, + @Matches(IsLambdaExpressionOrMethodReference.class) Function mapper) { return stream.map(mapper).reduce(0L, Long::sum); } diff --git a/error-prone-contrib/src/test/resources/tech/picnic/errorprone/refasterrules/StreamRulesTestInput.java b/error-prone-contrib/src/test/resources/tech/picnic/errorprone/refasterrules/StreamRulesTestInput.java index aab35eb355..d76f2baeeb 100644 --- a/error-prone-contrib/src/test/resources/tech/picnic/errorprone/refasterrules/StreamRulesTestInput.java +++ b/error-prone-contrib/src/test/resources/tech/picnic/errorprone/refasterrules/StreamRulesTestInput.java @@ -9,6 +9,7 @@ import com.google.common.collect.Streams; import java.util.Objects; import java.util.Optional; +import java.util.function.Function; import java.util.function.Predicate; import java.util.stream.Stream; import tech.picnic.errorprone.refaster.test.RefasterRuleCollectionTestCase; @@ -140,20 +141,26 @@ boolean testStreamAllMatch2() { } ImmutableSet testStreamMapToIntSum() { + Function parseIntFunction = (String s) -> Integer.parseInt(s); return ImmutableSet.of( Stream.of(1).map(i -> i * 2).reduce(0, Integer::sum), - Stream.of("1").map(Integer::parseInt).reduce(0, Integer::sum)); + Stream.of("1").map(Integer::parseInt).reduce(0, Integer::sum), + Stream.of("1").map(parseIntFunction).reduce(0, Integer::sum)); } ImmutableSet testStreamMapToDoubleSum() { + Function parseDoubleFunction = (String s) -> Double.parseDouble(s); return ImmutableSet.of( Stream.of(1).map(i -> i * 2.0).reduce(0.0, Double::sum), - Stream.of("1").map(Double::parseDouble).reduce(0.0, Double::sum)); + Stream.of("1").map(Double::parseDouble).reduce(0.0, Double::sum), + Stream.of("1").map(parseDoubleFunction).reduce(0.0, Double::sum)); } ImmutableSet testStreamMapToLongSum() { + Function parseLongFunction = (String s) -> Long.parseLong(s); return ImmutableSet.of( Stream.of(1).map(i -> i * 2L).reduce(0L, Long::sum), - Stream.of("1").map(Long::parseLong).reduce(0L, Long::sum)); + Stream.of("1").map(Long::parseLong).reduce(0L, Long::sum), + Stream.of("1").map(parseLongFunction).reduce(0L, Long::sum)); } } diff --git a/error-prone-contrib/src/test/resources/tech/picnic/errorprone/refasterrules/StreamRulesTestOutput.java b/error-prone-contrib/src/test/resources/tech/picnic/errorprone/refasterrules/StreamRulesTestOutput.java index 91dc6108ee..1aaef44d80 100644 --- a/error-prone-contrib/src/test/resources/tech/picnic/errorprone/refasterrules/StreamRulesTestOutput.java +++ b/error-prone-contrib/src/test/resources/tech/picnic/errorprone/refasterrules/StreamRulesTestOutput.java @@ -11,6 +11,7 @@ import java.util.Arrays; import java.util.Objects; import java.util.Optional; +import java.util.function.Function; import java.util.function.Predicate; import java.util.stream.Stream; import tech.picnic.errorprone.refaster.test.RefasterRuleCollectionTestCase; @@ -139,18 +140,26 @@ boolean testStreamAllMatch2() { } ImmutableSet testStreamMapToIntSum() { + Function parseIntFunction = (String s) -> Integer.parseInt(s); return ImmutableSet.of( - Stream.of(1).mapToInt(i -> i * 2).sum(), Stream.of("1").mapToInt(Integer::parseInt).sum()); + Stream.of(1).mapToInt(i -> i * 2).sum(), + Stream.of("1").mapToInt(Integer::parseInt).sum(), + Stream.of("1").map(parseIntFunction).reduce(0, Integer::sum)); } ImmutableSet testStreamMapToDoubleSum() { + Function parseDoubleFunction = (String s) -> Double.parseDouble(s); return ImmutableSet.of( Stream.of(1).mapToDouble(i -> i * 2.0).sum(), - Stream.of("1").mapToDouble(Double::parseDouble).sum()); + Stream.of("1").mapToDouble(Double::parseDouble).sum(), + Stream.of("1").map(parseDoubleFunction).reduce(0.0, Double::sum)); } ImmutableSet testStreamMapToLongSum() { + Function parseLongFunction = (String s) -> Long.parseLong(s); return ImmutableSet.of( - Stream.of(1).mapToLong(i -> i * 2L).sum(), Stream.of("1").mapToLong(Long::parseLong).sum()); + Stream.of(1).mapToLong(i -> i * 2L).sum(), + Stream.of("1").mapToLong(Long::parseLong).sum(), + Stream.of("1").map(parseLongFunction).reduce(0L, Long::sum)); } } diff --git a/refaster-support/src/main/java/tech/picnic/errorprone/refaster/matchers/IsLambdaExpressionOrMethodReference.java b/refaster-support/src/main/java/tech/picnic/errorprone/refaster/matchers/IsLambdaExpressionOrMethodReference.java new file mode 100644 index 0000000000..bf11aa74c0 --- /dev/null +++ b/refaster-support/src/main/java/tech/picnic/errorprone/refaster/matchers/IsLambdaExpressionOrMethodReference.java @@ -0,0 +1,29 @@ +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.LambdaExpressionTree; +import com.sun.source.tree.MemberReferenceTree; + +/** A matcher of lambda expressions or method references. */ +public final class IsLambdaExpressionOrMethodReference implements Matcher { + private static final long serialVersionUID = 1L; + + /** Instantiates a new {@link IsLambdaExpressionOrMethodReference} instance. */ + public IsLambdaExpressionOrMethodReference() {} + + @Override + public boolean matches(ExpressionTree tree, VisitorState state) { + return isLambdaExpression(tree) || isMethodReference(tree); + } + + private static boolean isLambdaExpression(ExpressionTree tree) { + return tree instanceof LambdaExpressionTree; + } + + private static boolean isMethodReference(ExpressionTree tree) { + return tree instanceof MemberReferenceTree + && ((MemberReferenceTree) tree).getMode() == MemberReferenceTree.ReferenceMode.INVOKE; + } +} From 1d94f39ff2ec33aec657bedcfe3fae2daef726ca Mon Sep 17 00:00:00 2001 From: Giovanni Zotta Date: Sat, 18 Feb 2023 21:17:22 +0100 Subject: [PATCH 4/7] Refactor `IsLambdaExpressionOrMethodReference` with a delegate --- .../IsLambdaExpressionOrMethodReference.java | 22 +++-- ...LambdaExpressionOrMethodReferenceTest.java | 87 +++++++++++++++++++ 2 files changed, 103 insertions(+), 6 deletions(-) create mode 100644 refaster-support/src/test/java/tech/picnic/errorprone/refaster/matchers/IsLambdaExpressionOrMethodReferenceTest.java diff --git a/refaster-support/src/main/java/tech/picnic/errorprone/refaster/matchers/IsLambdaExpressionOrMethodReference.java b/refaster-support/src/main/java/tech/picnic/errorprone/refaster/matchers/IsLambdaExpressionOrMethodReference.java index bf11aa74c0..4a7c5f9585 100644 --- a/refaster-support/src/main/java/tech/picnic/errorprone/refaster/matchers/IsLambdaExpressionOrMethodReference.java +++ b/refaster-support/src/main/java/tech/picnic/errorprone/refaster/matchers/IsLambdaExpressionOrMethodReference.java @@ -1,29 +1,39 @@ package tech.picnic.errorprone.refaster.matchers; +import static com.google.errorprone.matchers.Matchers.anyOf; +import static com.sun.source.tree.MemberReferenceTree.ReferenceMode.INVOKE; + import com.google.errorprone.VisitorState; import com.google.errorprone.matchers.Matcher; import com.sun.source.tree.ExpressionTree; import com.sun.source.tree.LambdaExpressionTree; import com.sun.source.tree.MemberReferenceTree; +import com.sun.source.tree.Tree; /** A matcher of lambda expressions or method references. */ public final class IsLambdaExpressionOrMethodReference implements Matcher { private static final long serialVersionUID = 1L; + private static final Matcher DELEGATE = + anyOf(isLambdaExpression(), isMethodReference()); + /** Instantiates a new {@link IsLambdaExpressionOrMethodReference} instance. */ public IsLambdaExpressionOrMethodReference() {} @Override public boolean matches(ExpressionTree tree, VisitorState state) { - return isLambdaExpression(tree) || isMethodReference(tree); + return DELEGATE.matches(tree, state); } - private static boolean isLambdaExpression(ExpressionTree tree) { - return tree instanceof LambdaExpressionTree; + /** Returns a matcher that matches lambda expressions. */ + public static Matcher isLambdaExpression() { + return (tree, state) -> tree instanceof LambdaExpressionTree; } - private static boolean isMethodReference(ExpressionTree tree) { - return tree instanceof MemberReferenceTree - && ((MemberReferenceTree) tree).getMode() == MemberReferenceTree.ReferenceMode.INVOKE; + /** Returns a matcher that matches method references. */ + public static Matcher isMethodReference() { + return (tree, state) -> + tree instanceof MemberReferenceTree + && ((MemberReferenceTree) tree).getMode().equals(INVOKE); } } diff --git a/refaster-support/src/test/java/tech/picnic/errorprone/refaster/matchers/IsLambdaExpressionOrMethodReferenceTest.java b/refaster-support/src/test/java/tech/picnic/errorprone/refaster/matchers/IsLambdaExpressionOrMethodReferenceTest.java new file mode 100644 index 0000000000..9d9e18097d --- /dev/null +++ b/refaster-support/src/test/java/tech/picnic/errorprone/refaster/matchers/IsLambdaExpressionOrMethodReferenceTest.java @@ -0,0 +1,87 @@ +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.Disabled; +import org.junit.jupiter.api.Test; + +final class IsLambdaExpressionOrMethodReferenceTest { + + @Disabled + @Test + void matches() { + CompilationTestHelper.newInstance(MatcherTestChecker.class, getClass()) + .addSourceLines( + "A.java", + "import java.util.function.Function;", + "import java.util.stream.Stream;", + "", + "class A {", + " Integer negative1() {", + " Function parseIntFunction = (String s) -> Integer.parseInt(s);", + " return Stream.of(\"1\").map(parseIntFunction).reduce(0, Integer::sum);", + " }", + "", + " Integer negative2() {", + " Function stringLengthMethodReference = String::length;", + " return Stream.of(\"1\").map(stringLengthMethodReference).reduce(0, Integer::sum);", + " }", + "", + " Double negative3() {", + " Function parseDoubleFunction = new Function() {", + " @Override", + " public Double apply(String s) {", + " return Double.parseDouble(s);", + " }", + " };", + " return Stream.of(\"1\").map(parseDoubleFunction).reduce(0.0, Double::sum);", + " }", + "", + " Long negative4() {", + " class ParseLongFunction implements Function {", + " @Override", + " public Long apply(String s) {", + " return Long.parseLong(s);", + " }", + " }", + " return Stream.of(\"1\").map(new ParseLongFunction()).reduce(0L, Long::sum);", + " }", + "", + " Integer positive1() {", + " // BUG: Diagnostic contains:", + " return Stream.of(1).map(i -> i * 2).reduce(0, Integer::sum);", + " }", + "", + " Long positive2() {", + " // BUG: Diagnostic contains:", + " return Stream.of(1)", + " .map(", + " i -> {", + " return i * 2L;", + " })", + " .reduce(0L, Long::sum);", + " }", + "", + " Double positive3() {", + " // BUG: Diagnostic contains:", + " return Stream.of(\"1\").map(Double::parseDouble).reduce(0.0, Double::sum);", + " }", + "}") + .doTest(); + } + + /** A {@link BugChecker} that simply delegates to {@link IsLambdaExpressionOrMethodReference}. */ + @BugPattern( + summary = "Flags expressions matched by `IsLambdaExpressionOrMethodReference`", + severity = ERROR) + public static final class MatcherTestChecker extends AbstractMatcherTestChecker { + private static final long serialVersionUID = 1L; + + public MatcherTestChecker() { + super(new IsLambdaExpressionOrMethodReference()); + } + } +} From 16142148b53a33e60914c0835673ec681a4c9191 Mon Sep 17 00:00:00 2001 From: Stephan Schroevers Date: Sun, 19 Feb 2023 10:39:45 +0100 Subject: [PATCH 5/7] Suggestions --- .../refasterrules/StreamRulesTestInput.java | 6 +- .../refasterrules/StreamRulesTestOutput.java | 6 +- .../IsLambdaExpressionOrMethodReference.java | 23 +------ ...LambdaExpressionOrMethodReferenceTest.java | 68 ++++++++++++------- 4 files changed, 53 insertions(+), 50 deletions(-) diff --git a/error-prone-contrib/src/test/resources/tech/picnic/errorprone/refasterrules/StreamRulesTestInput.java b/error-prone-contrib/src/test/resources/tech/picnic/errorprone/refasterrules/StreamRulesTestInput.java index d76f2baeeb..d86a7cc420 100644 --- a/error-prone-contrib/src/test/resources/tech/picnic/errorprone/refasterrules/StreamRulesTestInput.java +++ b/error-prone-contrib/src/test/resources/tech/picnic/errorprone/refasterrules/StreamRulesTestInput.java @@ -141,7 +141,7 @@ boolean testStreamAllMatch2() { } ImmutableSet testStreamMapToIntSum() { - Function parseIntFunction = (String s) -> Integer.parseInt(s); + Function parseIntFunction = Integer::parseInt; return ImmutableSet.of( Stream.of(1).map(i -> i * 2).reduce(0, Integer::sum), Stream.of("1").map(Integer::parseInt).reduce(0, Integer::sum), @@ -149,7 +149,7 @@ ImmutableSet testStreamMapToIntSum() { } ImmutableSet testStreamMapToDoubleSum() { - Function parseDoubleFunction = (String s) -> Double.parseDouble(s); + Function parseDoubleFunction = Double::parseDouble; return ImmutableSet.of( Stream.of(1).map(i -> i * 2.0).reduce(0.0, Double::sum), Stream.of("1").map(Double::parseDouble).reduce(0.0, Double::sum), @@ -157,7 +157,7 @@ ImmutableSet testStreamMapToDoubleSum() { } ImmutableSet testStreamMapToLongSum() { - Function parseLongFunction = (String s) -> Long.parseLong(s); + Function parseLongFunction = Long::parseLong; return ImmutableSet.of( Stream.of(1).map(i -> i * 2L).reduce(0L, Long::sum), Stream.of("1").map(Long::parseLong).reduce(0L, Long::sum), diff --git a/error-prone-contrib/src/test/resources/tech/picnic/errorprone/refasterrules/StreamRulesTestOutput.java b/error-prone-contrib/src/test/resources/tech/picnic/errorprone/refasterrules/StreamRulesTestOutput.java index 1aaef44d80..ae72193868 100644 --- a/error-prone-contrib/src/test/resources/tech/picnic/errorprone/refasterrules/StreamRulesTestOutput.java +++ b/error-prone-contrib/src/test/resources/tech/picnic/errorprone/refasterrules/StreamRulesTestOutput.java @@ -140,7 +140,7 @@ boolean testStreamAllMatch2() { } ImmutableSet testStreamMapToIntSum() { - Function parseIntFunction = (String s) -> Integer.parseInt(s); + Function parseIntFunction = Integer::parseInt; return ImmutableSet.of( Stream.of(1).mapToInt(i -> i * 2).sum(), Stream.of("1").mapToInt(Integer::parseInt).sum(), @@ -148,7 +148,7 @@ ImmutableSet testStreamMapToIntSum() { } ImmutableSet testStreamMapToDoubleSum() { - Function parseDoubleFunction = (String s) -> Double.parseDouble(s); + Function parseDoubleFunction = Double::parseDouble; return ImmutableSet.of( Stream.of(1).mapToDouble(i -> i * 2.0).sum(), Stream.of("1").mapToDouble(Double::parseDouble).sum(), @@ -156,7 +156,7 @@ ImmutableSet testStreamMapToDoubleSum() { } ImmutableSet testStreamMapToLongSum() { - Function parseLongFunction = (String s) -> Long.parseLong(s); + Function parseLongFunction = Long::parseLong; return ImmutableSet.of( Stream.of(1).mapToLong(i -> i * 2L).sum(), Stream.of("1").mapToLong(Long::parseLong).sum(), diff --git a/refaster-support/src/main/java/tech/picnic/errorprone/refaster/matchers/IsLambdaExpressionOrMethodReference.java b/refaster-support/src/main/java/tech/picnic/errorprone/refaster/matchers/IsLambdaExpressionOrMethodReference.java index 4a7c5f9585..23721f73ae 100644 --- a/refaster-support/src/main/java/tech/picnic/errorprone/refaster/matchers/IsLambdaExpressionOrMethodReference.java +++ b/refaster-support/src/main/java/tech/picnic/errorprone/refaster/matchers/IsLambdaExpressionOrMethodReference.java @@ -1,39 +1,20 @@ package tech.picnic.errorprone.refaster.matchers; -import static com.google.errorprone.matchers.Matchers.anyOf; -import static com.sun.source.tree.MemberReferenceTree.ReferenceMode.INVOKE; - import com.google.errorprone.VisitorState; import com.google.errorprone.matchers.Matcher; import com.sun.source.tree.ExpressionTree; import com.sun.source.tree.LambdaExpressionTree; import com.sun.source.tree.MemberReferenceTree; -import com.sun.source.tree.Tree; -/** A matcher of lambda expressions or method references. */ +/** A matcher of lambda expressions and method references. */ public final class IsLambdaExpressionOrMethodReference implements Matcher { private static final long serialVersionUID = 1L; - private static final Matcher DELEGATE = - anyOf(isLambdaExpression(), isMethodReference()); - /** Instantiates a new {@link IsLambdaExpressionOrMethodReference} instance. */ public IsLambdaExpressionOrMethodReference() {} @Override public boolean matches(ExpressionTree tree, VisitorState state) { - return DELEGATE.matches(tree, state); - } - - /** Returns a matcher that matches lambda expressions. */ - public static Matcher isLambdaExpression() { - return (tree, state) -> tree instanceof LambdaExpressionTree; - } - - /** Returns a matcher that matches method references. */ - public static Matcher isMethodReference() { - return (tree, state) -> - tree instanceof MemberReferenceTree - && ((MemberReferenceTree) tree).getMode().equals(INVOKE); + return tree instanceof LambdaExpressionTree || tree instanceof MemberReferenceTree; } } diff --git a/refaster-support/src/test/java/tech/picnic/errorprone/refaster/matchers/IsLambdaExpressionOrMethodReferenceTest.java b/refaster-support/src/test/java/tech/picnic/errorprone/refaster/matchers/IsLambdaExpressionOrMethodReferenceTest.java index 9d9e18097d..9362597a9d 100644 --- a/refaster-support/src/test/java/tech/picnic/errorprone/refaster/matchers/IsLambdaExpressionOrMethodReferenceTest.java +++ b/refaster-support/src/test/java/tech/picnic/errorprone/refaster/matchers/IsLambdaExpressionOrMethodReferenceTest.java @@ -5,12 +5,9 @@ import com.google.errorprone.BugPattern; import com.google.errorprone.CompilationTestHelper; import com.google.errorprone.bugpatterns.BugChecker; -import org.junit.jupiter.api.Disabled; import org.junit.jupiter.api.Test; final class IsLambdaExpressionOrMethodReferenceTest { - - @Disabled @Test void matches() { CompilationTestHelper.newInstance(MatcherTestChecker.class, getClass()) @@ -21,23 +18,35 @@ void matches() { "", "class A {", " Integer negative1() {", + " // BUG: Diagnostic contains:", " Function parseIntFunction = (String s) -> Integer.parseInt(s);", - " return Stream.of(\"1\").map(parseIntFunction).reduce(0, Integer::sum);", + " return Stream.of(\"1\")", + " .map(parseIntFunction)", + " // BUG: Diagnostic contains:", + " .reduce(0, Integer::sum);", " }", "", " Integer negative2() {", + " // BUG: Diagnostic contains:", " Function stringLengthMethodReference = String::length;", - " return Stream.of(\"1\").map(stringLengthMethodReference).reduce(0, Integer::sum);", + " return Stream.of(\"1\")", + " .map(stringLengthMethodReference)", + " // BUG: Diagnostic contains:", + " .reduce(0, Integer::sum);", " }", "", " Double negative3() {", - " Function parseDoubleFunction = new Function() {", - " @Override", - " public Double apply(String s) {", - " return Double.parseDouble(s);", - " }", - " };", - " return Stream.of(\"1\").map(parseDoubleFunction).reduce(0.0, Double::sum);", + " Function parseDoubleFunction =", + " new Function() {", + " @Override", + " public Double apply(String s) {", + " return Double.parseDouble(s);", + " }", + " };", + " return Stream.of(\"1\")", + " .map(parseDoubleFunction)", + " // BUG: Diagnostic contains:", + " .reduce(0.0, Double::sum);", " }", "", " Long negative4() {", @@ -47,27 +56,37 @@ void matches() { " return Long.parseLong(s);", " }", " }", - " return Stream.of(\"1\").map(new ParseLongFunction()).reduce(0L, Long::sum);", + " return Stream.of(\"1\")", + " .map(new ParseLongFunction())", + " // BUG: Diagnostic contains:", + " .reduce(0L, Long::sum);", " }", "", " Integer positive1() {", - " // BUG: Diagnostic contains:", - " return Stream.of(1).map(i -> i * 2).reduce(0, Integer::sum);", + " return Stream.of(1)", + " // BUG: Diagnostic contains:", + " .map(i -> i * 2)", + " // BUG: Diagnostic contains:", + " .reduce(0, Integer::sum);", " }", "", " Long positive2() {", - " // BUG: Diagnostic contains:", " return Stream.of(1)", - " .map(", - " i -> {", - " return i * 2L;", - " })", - " .reduce(0L, Long::sum);", + " .map(", + " // BUG: Diagnostic contains:", + " i -> {", + " return i * 2L;", + " })", + " // BUG: Diagnostic contains:", + " .reduce(0L, Long::sum);", " }", "", " Double positive3() {", - " // BUG: Diagnostic contains:", - " return Stream.of(\"1\").map(Double::parseDouble).reduce(0.0, Double::sum);", + " return Stream.of(\"1\")", + " // BUG: Diagnostic contains:", + " .map(Double::parseDouble)", + " // BUG: Diagnostic contains:", + " .reduce(0.0, Double::sum);", " }", "}") .doTest(); @@ -80,6 +99,9 @@ void matches() { 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 IsLambdaExpressionOrMethodReference()); } From 3d73f2ec1cc319cb945b399b9e41dfe952e2a000 Mon Sep 17 00:00:00 2001 From: Stephan Schroevers Date: Sun, 19 Feb 2023 11:36:37 +0100 Subject: [PATCH 6/7] Simplify test --- ...LambdaExpressionOrMethodReferenceTest.java | 78 ++++--------------- 1 file changed, 17 insertions(+), 61 deletions(-) diff --git a/refaster-support/src/test/java/tech/picnic/errorprone/refaster/matchers/IsLambdaExpressionOrMethodReferenceTest.java b/refaster-support/src/test/java/tech/picnic/errorprone/refaster/matchers/IsLambdaExpressionOrMethodReferenceTest.java index 9362597a9d..97af67ce1b 100644 --- a/refaster-support/src/test/java/tech/picnic/errorprone/refaster/matchers/IsLambdaExpressionOrMethodReferenceTest.java +++ b/refaster-support/src/test/java/tech/picnic/errorprone/refaster/matchers/IsLambdaExpressionOrMethodReferenceTest.java @@ -13,80 +13,36 @@ void matches() { CompilationTestHelper.newInstance(MatcherTestChecker.class, getClass()) .addSourceLines( "A.java", + "import com.google.common.base.Predicates;", "import java.util.function.Function;", - "import java.util.stream.Stream;", + "import java.util.function.Predicate;", "", "class A {", - " Integer negative1() {", - " // BUG: Diagnostic contains:", - " Function parseIntFunction = (String s) -> Integer.parseInt(s);", - " return Stream.of(\"1\")", - " .map(parseIntFunction)", - " // BUG: Diagnostic contains:", - " .reduce(0, Integer::sum);", - " }", - "", - " Integer negative2() {", - " // BUG: Diagnostic contains:", - " Function stringLengthMethodReference = String::length;", - " return Stream.of(\"1\")", - " .map(stringLengthMethodReference)", - " // BUG: Diagnostic contains:", - " .reduce(0, Integer::sum);", + " boolean negative1() {", + " return true;", " }", "", - " Double negative3() {", - " Function parseDoubleFunction =", - " new Function() {", - " @Override", - " public Double apply(String s) {", - " return Double.parseDouble(s);", - " }", - " };", - " return Stream.of(\"1\")", - " .map(parseDoubleFunction)", - " // BUG: Diagnostic contains:", - " .reduce(0.0, Double::sum);", + " String negative2() {", + " return new String(new byte[0]);", " }", "", - " Long negative4() {", - " class ParseLongFunction implements Function {", - " @Override", - " public Long apply(String s) {", - " return Long.parseLong(s);", - " }", - " }", - " return Stream.of(\"1\")", - " .map(new ParseLongFunction())", - " // BUG: Diagnostic contains:", - " .reduce(0L, Long::sum);", + " Predicate negative3() {", + " return Predicates.alwaysTrue();", " }", "", - " Integer positive1() {", - " return Stream.of(1)", - " // BUG: Diagnostic contains:", - " .map(i -> i * 2)", - " // BUG: Diagnostic contains:", - " .reduce(0, Integer::sum);", + " Predicate positive1() {", + " // BUG: Diagnostic contains:", + " return str -> true;", " }", "", - " Long positive2() {", - " return Stream.of(1)", - " .map(", - " // BUG: Diagnostic contains:", - " i -> {", - " return i * 2L;", - " })", - " // BUG: Diagnostic contains:", - " .reduce(0L, Long::sum);", + " Predicate positive2() {", + " // BUG: Diagnostic contains:", + " return String::isEmpty;", " }", "", - " Double positive3() {", - " return Stream.of(\"1\")", - " // BUG: Diagnostic contains:", - " .map(Double::parseDouble)", - " // BUG: Diagnostic contains:", - " .reduce(0.0, Double::sum);", + " Function positive3() {", + " // BUG: Diagnostic contains:", + " return String::new;", " }", "}") .doTest(); From 5228e4e817022c83d527ed0106417d4fe9359d7e Mon Sep 17 00:00:00 2001 From: Rick Ossendrijver Date: Tue, 21 Feb 2023 09:11:50 +0100 Subject: [PATCH 7/7] Suggestions --- .../refasterrules/StreamRulesTestInput.java | 12 ++++++------ .../refasterrules/StreamRulesTestOutput.java | 12 ++++++------ .../IsLambdaExpressionOrMethodReferenceTest.java | 9 ++++++++- 3 files changed, 20 insertions(+), 13 deletions(-) diff --git a/error-prone-contrib/src/test/resources/tech/picnic/errorprone/refasterrules/StreamRulesTestInput.java b/error-prone-contrib/src/test/resources/tech/picnic/errorprone/refasterrules/StreamRulesTestInput.java index d86a7cc420..f11d04964a 100644 --- a/error-prone-contrib/src/test/resources/tech/picnic/errorprone/refasterrules/StreamRulesTestInput.java +++ b/error-prone-contrib/src/test/resources/tech/picnic/errorprone/refasterrules/StreamRulesTestInput.java @@ -144,23 +144,23 @@ ImmutableSet testStreamMapToIntSum() { Function parseIntFunction = Integer::parseInt; return ImmutableSet.of( Stream.of(1).map(i -> i * 2).reduce(0, Integer::sum), - Stream.of("1").map(Integer::parseInt).reduce(0, Integer::sum), - Stream.of("1").map(parseIntFunction).reduce(0, Integer::sum)); + Stream.of("2").map(Integer::parseInt).reduce(0, Integer::sum), + Stream.of("3").map(parseIntFunction).reduce(0, Integer::sum)); } ImmutableSet testStreamMapToDoubleSum() { Function parseDoubleFunction = Double::parseDouble; return ImmutableSet.of( Stream.of(1).map(i -> i * 2.0).reduce(0.0, Double::sum), - Stream.of("1").map(Double::parseDouble).reduce(0.0, Double::sum), - Stream.of("1").map(parseDoubleFunction).reduce(0.0, Double::sum)); + Stream.of("2").map(Double::parseDouble).reduce(0.0, Double::sum), + Stream.of("3").map(parseDoubleFunction).reduce(0.0, Double::sum)); } ImmutableSet testStreamMapToLongSum() { Function parseLongFunction = Long::parseLong; return ImmutableSet.of( Stream.of(1).map(i -> i * 2L).reduce(0L, Long::sum), - Stream.of("1").map(Long::parseLong).reduce(0L, Long::sum), - Stream.of("1").map(parseLongFunction).reduce(0L, Long::sum)); + Stream.of("2").map(Long::parseLong).reduce(0L, Long::sum), + Stream.of("3").map(parseLongFunction).reduce(0L, Long::sum)); } } diff --git a/error-prone-contrib/src/test/resources/tech/picnic/errorprone/refasterrules/StreamRulesTestOutput.java b/error-prone-contrib/src/test/resources/tech/picnic/errorprone/refasterrules/StreamRulesTestOutput.java index ae72193868..8c89199f0d 100644 --- a/error-prone-contrib/src/test/resources/tech/picnic/errorprone/refasterrules/StreamRulesTestOutput.java +++ b/error-prone-contrib/src/test/resources/tech/picnic/errorprone/refasterrules/StreamRulesTestOutput.java @@ -143,23 +143,23 @@ ImmutableSet testStreamMapToIntSum() { Function parseIntFunction = Integer::parseInt; return ImmutableSet.of( Stream.of(1).mapToInt(i -> i * 2).sum(), - Stream.of("1").mapToInt(Integer::parseInt).sum(), - Stream.of("1").map(parseIntFunction).reduce(0, Integer::sum)); + Stream.of("2").mapToInt(Integer::parseInt).sum(), + Stream.of("3").map(parseIntFunction).reduce(0, Integer::sum)); } ImmutableSet testStreamMapToDoubleSum() { Function parseDoubleFunction = Double::parseDouble; return ImmutableSet.of( Stream.of(1).mapToDouble(i -> i * 2.0).sum(), - Stream.of("1").mapToDouble(Double::parseDouble).sum(), - Stream.of("1").map(parseDoubleFunction).reduce(0.0, Double::sum)); + Stream.of("2").mapToDouble(Double::parseDouble).sum(), + Stream.of("3").map(parseDoubleFunction).reduce(0.0, Double::sum)); } ImmutableSet testStreamMapToLongSum() { Function parseLongFunction = Long::parseLong; return ImmutableSet.of( Stream.of(1).mapToLong(i -> i * 2L).sum(), - Stream.of("1").mapToLong(Long::parseLong).sum(), - Stream.of("1").map(parseLongFunction).reduce(0L, Long::sum)); + Stream.of("2").mapToLong(Long::parseLong).sum(), + Stream.of("3").map(parseLongFunction).reduce(0L, Long::sum)); } } diff --git a/refaster-support/src/test/java/tech/picnic/errorprone/refaster/matchers/IsLambdaExpressionOrMethodReferenceTest.java b/refaster-support/src/test/java/tech/picnic/errorprone/refaster/matchers/IsLambdaExpressionOrMethodReferenceTest.java index 97af67ce1b..3280024cda 100644 --- a/refaster-support/src/test/java/tech/picnic/errorprone/refaster/matchers/IsLambdaExpressionOrMethodReferenceTest.java +++ b/refaster-support/src/test/java/tech/picnic/errorprone/refaster/matchers/IsLambdaExpressionOrMethodReferenceTest.java @@ -37,10 +37,17 @@ void matches() { "", " Predicate positive2() {", " // BUG: Diagnostic contains:", + " return str -> {", + " return true;", + " };", + " }", + "", + " Predicate positive3() {", + " // BUG: Diagnostic contains:", " return String::isEmpty;", " }", "", - " Function positive3() {", + " Function positive4() {", " // BUG: Diagnostic contains:", " return String::new;", " }",