From a75d50543b2fbeb3837e41dac2b53feaebe03b6a Mon Sep 17 00:00:00 2001 From: Dan Fox Date: Fri, 5 Mar 2021 20:34:46 +0000 Subject: [PATCH 01/13] Pull in test dep on streamex --- baseline-error-prone/build.gradle | 1 + versions.props | 1 + 2 files changed, 2 insertions(+) diff --git a/baseline-error-prone/build.gradle b/baseline-error-prone/build.gradle index dd39a11f7..3fb93fc4f 100644 --- a/baseline-error-prone/build.gradle +++ b/baseline-error-prone/build.gradle @@ -26,6 +26,7 @@ dependencies { testCompileOnly 'org.immutables:value::annotations' testImplementation 'org.junit.jupiter:junit-jupiter' testRuntimeOnly 'org.junit.jupiter:junit-jupiter-migrationsupport' + testRuntimeOnly 'one.util:streamex' annotationProcessor 'com.google.auto.service:auto-service' annotationProcessor 'org.immutables:value' diff --git a/versions.props b/versions.props index b3e26fdd8..814f4f487 100644 --- a/versions.props +++ b/versions.props @@ -32,6 +32,7 @@ org.assertj:assertj-core = 3.19.0 org.hamcrest:hamcrest-core = 2.2 org.junit.jupiter:* = 5.7.1 org.mockito:* = 3.8.0 +one.util:streamex = 0.7.3 # dependency-upgrader:OFF # Don't upgrade, we will remove this in a future release. From fc3b6383b7df406e1331ce48b3ee38a58d96bcb3 Mon Sep 17 00:00:00 2001 From: Dan Fox Date: Fri, 5 Mar 2021 20:36:55 +0000 Subject: [PATCH 02/13] StreamEx toImmutableMap --- .../StreamExPreferImmutableOutput.java | 70 +++++++++++++++++++ .../StreamExPreferImmutableOutputTest.java | 47 +++++++++++++ 2 files changed, 117 insertions(+) create mode 100644 baseline-error-prone/src/main/java/com/palantir/baseline/errorprone/StreamExPreferImmutableOutput.java create mode 100644 baseline-error-prone/src/test/java/com/palantir/baseline/errorprone/StreamExPreferImmutableOutputTest.java diff --git a/baseline-error-prone/src/main/java/com/palantir/baseline/errorprone/StreamExPreferImmutableOutput.java b/baseline-error-prone/src/main/java/com/palantir/baseline/errorprone/StreamExPreferImmutableOutput.java new file mode 100644 index 000000000..7f0b91512 --- /dev/null +++ b/baseline-error-prone/src/main/java/com/palantir/baseline/errorprone/StreamExPreferImmutableOutput.java @@ -0,0 +1,70 @@ +/* + * (c) Copyright 2019 Palantir Technologies Inc. All rights reserved. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package com.palantir.baseline.errorprone; + +import com.google.auto.service.AutoService; +import com.google.errorprone.BugPattern; +import com.google.errorprone.BugPattern.SeverityLevel; +import com.google.errorprone.VisitorState; +import com.google.errorprone.bugpatterns.BugChecker; +import com.google.errorprone.fixes.SuggestedFix; +import com.google.errorprone.matchers.Description; +import com.google.errorprone.matchers.Matcher; +import com.google.errorprone.matchers.method.MethodMatchers; +import com.google.errorprone.util.ASTHelpers; +import com.sun.source.tree.ExpressionTree; +import com.sun.source.tree.MethodInvocationTree; +import java.util.Collections; + +@AutoService(BugChecker.class) +@BugPattern( + name = "StreamExPreferImmutableStreamOutputs", + link = "https://github.com/palantir/gradle-baseline#baseline-error-prone-checks", + linkType = BugPattern.LinkType.CUSTOM, + providesFix = BugPattern.ProvidesFix.REQUIRES_HUMAN_ATTENTION, + severity = SeverityLevel.WARNING, + summary = "Prefer immutable/unmodifable collections wherever possible because they are innately threadsafe, " + + "and easier to reason about when passed between different functions." + + " If you really want a mutable output then explicitly suppress this check.") +public final class StreamExPreferImmutableOutput extends BugChecker implements BugChecker.MethodInvocationTreeMatcher { + + private static final long serialVersionUID = 1L; + + private static final String ERROR_MESSAGE = "Prefer toImmutableMap()"; + + private static final Matcher TO_MAP = MethodMatchers.instanceMethod() + .onExactClass("one.util.streamex.EntryStream") + .named("toMap") + .withParameters(Collections.emptyList()); + + @Override + public Description matchMethodInvocation(MethodInvocationTree tree, VisitorState state) { + if (TO_MAP.matches(tree, state)) { + SuggestedFix.Builder fix = SuggestedFix.builder(); + + String base = state.getSourceForNode(ASTHelpers.getReceiver(tree.getMethodSelect())); + + return buildDescription(tree) + .setMessage(ERROR_MESSAGE) + .addFix(fix.replace(tree.getMethodSelect(), base + ".toImmutableMap") + .build()) + .build(); + } + + return Description.NO_MATCH; + } +} diff --git a/baseline-error-prone/src/test/java/com/palantir/baseline/errorprone/StreamExPreferImmutableOutputTest.java b/baseline-error-prone/src/test/java/com/palantir/baseline/errorprone/StreamExPreferImmutableOutputTest.java new file mode 100644 index 000000000..7e23b02d3 --- /dev/null +++ b/baseline-error-prone/src/test/java/com/palantir/baseline/errorprone/StreamExPreferImmutableOutputTest.java @@ -0,0 +1,47 @@ +/* + * (c) Copyright 2021 Palantir Technologies Inc. All rights reserved. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package com.palantir.baseline.errorprone; + +import com.google.errorprone.BugCheckerRefactoringTestHelper; +import org.junit.jupiter.api.Test; + +public class StreamExPreferImmutableOutputTest { + + @Test + void replace_EntryStream_toMap() { + fix().addInputLines( + "Test.java", + "import one.util.streamex.EntryStream;", + "import java.util.Map;", + "public class Test {", + " Map entryStream = EntryStream.of(Map.of(\"hello\", \"world\")).toMap();", + "}") + .addOutputLines( + "Test.java", + "import one.util.streamex.EntryStream;", + "import java.util.Map;", + "public class Test {", + " Map entryStream = EntryStream.of(Map.of(\"hello\", \"world\"))" + + ".toImmutableMap();", + "}") + .doTest(BugCheckerRefactoringTestHelper.TestMode.TEXT_MATCH); + } + + private RefactoringValidator fix() { + return RefactoringValidator.of(new StreamExPreferImmutableOutput(), getClass()); + } +} From a32e71ad8c7641c99c09a7dd34446ff4407ab8b9 Mon Sep 17 00:00:00 2001 From: Dan Fox Date: Fri, 5 Mar 2021 20:40:59 +0000 Subject: [PATCH 03/13] toImmutableSet --- .../StreamExPreferImmutableOutput.java | 21 ++++++++++++------- .../StreamExPreferImmutableOutputTest.java | 21 ++++++++++++++++++- 2 files changed, 34 insertions(+), 8 deletions(-) diff --git a/baseline-error-prone/src/main/java/com/palantir/baseline/errorprone/StreamExPreferImmutableOutput.java b/baseline-error-prone/src/main/java/com/palantir/baseline/errorprone/StreamExPreferImmutableOutput.java index 7f0b91512..7ea92cfef 100644 --- a/baseline-error-prone/src/main/java/com/palantir/baseline/errorprone/StreamExPreferImmutableOutput.java +++ b/baseline-error-prone/src/main/java/com/palantir/baseline/errorprone/StreamExPreferImmutableOutput.java @@ -44,24 +44,31 @@ public final class StreamExPreferImmutableOutput extends BugChecker implements B private static final long serialVersionUID = 1L; - private static final String ERROR_MESSAGE = "Prefer toImmutableMap()"; - private static final Matcher TO_MAP = MethodMatchers.instanceMethod() .onExactClass("one.util.streamex.EntryStream") .named("toMap") .withParameters(Collections.emptyList()); + private static final Matcher TO_SET = MethodMatchers.instanceMethod() + .onDescendantOf("one.util.streamex.AbstractStreamEx") + .named("toSet") + .withParameters(Collections.emptyList()); + @Override public Description matchMethodInvocation(MethodInvocationTree tree, VisitorState state) { if (TO_MAP.matches(tree, state)) { - SuggestedFix.Builder fix = SuggestedFix.builder(); - String base = state.getSourceForNode(ASTHelpers.getReceiver(tree.getMethodSelect())); + return buildDescription(tree) + .setMessage("Prefer .toImmutableMap()") + .addFix(SuggestedFix.replace(tree.getMethodSelect(), base + ".toImmutableMap")) + .build(); + } + if (TO_SET.matches(tree, state)) { + String base = state.getSourceForNode(ASTHelpers.getReceiver(tree.getMethodSelect())); return buildDescription(tree) - .setMessage(ERROR_MESSAGE) - .addFix(fix.replace(tree.getMethodSelect(), base + ".toImmutableMap") - .build()) + .setMessage("Prefer .toImmutableSet()") + .addFix(SuggestedFix.replace(tree.getMethodSelect(), base + ".toImmutableSet")) .build(); } diff --git a/baseline-error-prone/src/test/java/com/palantir/baseline/errorprone/StreamExPreferImmutableOutputTest.java b/baseline-error-prone/src/test/java/com/palantir/baseline/errorprone/StreamExPreferImmutableOutputTest.java index 7e23b02d3..60c50c125 100644 --- a/baseline-error-prone/src/test/java/com/palantir/baseline/errorprone/StreamExPreferImmutableOutputTest.java +++ b/baseline-error-prone/src/test/java/com/palantir/baseline/errorprone/StreamExPreferImmutableOutputTest.java @@ -22,7 +22,7 @@ public class StreamExPreferImmutableOutputTest { @Test - void replace_EntryStream_toMap() { + void toMap() { fix().addInputLines( "Test.java", "import one.util.streamex.EntryStream;", @@ -41,6 +41,25 @@ void replace_EntryStream_toMap() { .doTest(BugCheckerRefactoringTestHelper.TestMode.TEXT_MATCH); } + @Test + void toSet() { + fix().addInputLines( + "Test.java", + "import one.util.streamex.StreamEx;", + "import java.util.Set;", + "public class Test {", + " Set s = StreamEx.of(\"Hello\").toSet();", + "}") + .addOutputLines( + "Test.java", + "import one.util.streamex.StreamEx;", + "import java.util.Set;", + "public class Test {", + " Set s = StreamEx.of(\"Hello\").toImmutableSet();", + "}") + .doTest(BugCheckerRefactoringTestHelper.TestMode.TEXT_MATCH); + } + private RefactoringValidator fix() { return RefactoringValidator.of(new StreamExPreferImmutableOutput(), getClass()); } From 8a47a1fd3cb7709bea953dc09ef6782f489e9f5d Mon Sep 17 00:00:00 2001 From: Dan Fox Date: Fri, 5 Mar 2021 20:43:12 +0000 Subject: [PATCH 04/13] Rename to PreferImmutableStreamExCollections --- ...> PreferImmutableStreamExCollections.java} | 18 +++++++++++++-- ...eferImmutableStreamExCollectionsTest.java} | 23 +++++++++++++++++-- 2 files changed, 37 insertions(+), 4 deletions(-) rename baseline-error-prone/src/main/java/com/palantir/baseline/errorprone/{StreamExPreferImmutableOutput.java => PreferImmutableStreamExCollections.java} (80%) rename baseline-error-prone/src/test/java/com/palantir/baseline/errorprone/{StreamExPreferImmutableOutputTest.java => PreferImmutableStreamExCollectionsTest.java} (72%) diff --git a/baseline-error-prone/src/main/java/com/palantir/baseline/errorprone/StreamExPreferImmutableOutput.java b/baseline-error-prone/src/main/java/com/palantir/baseline/errorprone/PreferImmutableStreamExCollections.java similarity index 80% rename from baseline-error-prone/src/main/java/com/palantir/baseline/errorprone/StreamExPreferImmutableOutput.java rename to baseline-error-prone/src/main/java/com/palantir/baseline/errorprone/PreferImmutableStreamExCollections.java index 7ea92cfef..000ce60cf 100644 --- a/baseline-error-prone/src/main/java/com/palantir/baseline/errorprone/StreamExPreferImmutableOutput.java +++ b/baseline-error-prone/src/main/java/com/palantir/baseline/errorprone/PreferImmutableStreamExCollections.java @@ -32,7 +32,7 @@ @AutoService(BugChecker.class) @BugPattern( - name = "StreamExPreferImmutableStreamOutputs", + name = "PreferImmutableStreamExCollections", link = "https://github.com/palantir/gradle-baseline#baseline-error-prone-checks", linkType = BugPattern.LinkType.CUSTOM, providesFix = BugPattern.ProvidesFix.REQUIRES_HUMAN_ATTENTION, @@ -40,7 +40,8 @@ summary = "Prefer immutable/unmodifable collections wherever possible because they are innately threadsafe, " + "and easier to reason about when passed between different functions." + " If you really want a mutable output then explicitly suppress this check.") -public final class StreamExPreferImmutableOutput extends BugChecker implements BugChecker.MethodInvocationTreeMatcher { +public final class PreferImmutableStreamExCollections extends BugChecker + implements BugChecker.MethodInvocationTreeMatcher { private static final long serialVersionUID = 1L; @@ -54,6 +55,11 @@ public final class StreamExPreferImmutableOutput extends BugChecker implements B .named("toSet") .withParameters(Collections.emptyList()); + private static final Matcher TO_LIST = MethodMatchers.instanceMethod() + .onDescendantOf("one.util.streamex.AbstractStreamEx") + .named("toList") + .withParameters(Collections.emptyList()); + @Override public Description matchMethodInvocation(MethodInvocationTree tree, VisitorState state) { if (TO_MAP.matches(tree, state)) { @@ -72,6 +78,14 @@ public Description matchMethodInvocation(MethodInvocationTree tree, VisitorState .build(); } + if (TO_LIST.matches(tree, state)) { + String base = state.getSourceForNode(ASTHelpers.getReceiver(tree.getMethodSelect())); + return buildDescription(tree) + .setMessage("Prefer .toImmutableList()") + .addFix(SuggestedFix.replace(tree.getMethodSelect(), base + ".toImmutableList")) + .build(); + } + return Description.NO_MATCH; } } diff --git a/baseline-error-prone/src/test/java/com/palantir/baseline/errorprone/StreamExPreferImmutableOutputTest.java b/baseline-error-prone/src/test/java/com/palantir/baseline/errorprone/PreferImmutableStreamExCollectionsTest.java similarity index 72% rename from baseline-error-prone/src/test/java/com/palantir/baseline/errorprone/StreamExPreferImmutableOutputTest.java rename to baseline-error-prone/src/test/java/com/palantir/baseline/errorprone/PreferImmutableStreamExCollectionsTest.java index 60c50c125..5c0d27e54 100644 --- a/baseline-error-prone/src/test/java/com/palantir/baseline/errorprone/StreamExPreferImmutableOutputTest.java +++ b/baseline-error-prone/src/test/java/com/palantir/baseline/errorprone/PreferImmutableStreamExCollectionsTest.java @@ -19,7 +19,7 @@ import com.google.errorprone.BugCheckerRefactoringTestHelper; import org.junit.jupiter.api.Test; -public class StreamExPreferImmutableOutputTest { +public class PreferImmutableStreamExCollectionsTest { @Test void toMap() { @@ -60,7 +60,26 @@ void toSet() { .doTest(BugCheckerRefactoringTestHelper.TestMode.TEXT_MATCH); } + @Test + void toList() { + fix().addInputLines( + "Test.java", + "import one.util.streamex.StreamEx;", + "import java.util.List;", + "public class Test {", + " List s = StreamEx.of(\"Hello\").toList();", + "}") + .addOutputLines( + "Test.java", + "import one.util.streamex.StreamEx;", + "import java.util.List;", + "public class Test {", + " List s = StreamEx.of(\"Hello\").toImmutableList();", + "}") + .doTest(BugCheckerRefactoringTestHelper.TestMode.TEXT_MATCH); + } + private RefactoringValidator fix() { - return RefactoringValidator.of(new StreamExPreferImmutableOutput(), getClass()); + return RefactoringValidator.of(new PreferImmutableStreamExCollections(), getClass()); } } From ff8203d9fe56f9ee41608c8cd21a3265d1156fe3 Mon Sep 17 00:00:00 2001 From: Dan Fox Date: Fri, 5 Mar 2021 20:45:59 +0000 Subject: [PATCH 05/13] README --- README.md | 1 + 1 file changed, 1 insertion(+) diff --git a/README.md b/README.md index a552ed18f..7bb59065a 100644 --- a/README.md +++ b/README.md @@ -203,6 +203,7 @@ Safe Logging can be found at [github.com/palantir/safe-logging](https://github.c - `CompileTimeConstantViolatesLiskovSubstitution`: Requires consistent application of the `@CompileTimeConstant` annotation to resolve inconsistent validation based on the reference type on which the met is invoked. - `ClassInitializationDeadlock`: Detect type structures which can cause deadlocks initializing classes. - `ConsistentLoggerName`: Ensure Loggers are named consistently. +- `PreferImmutableStreamExCollections`: It's common to use toMap/toSet/toList() as the terminal operation on a stream, but would be extremely surprising to rely on the mutability of these collections. Prefer `toImmutableMap`, `toImmutableSet` and `toImmutableList`. (If the performance overhead of a stream is already acceptable, then the `UnmodifiableFoo` wrapper is likely tolerable). ### Programmatic Application From dcfa6133dbc4399a837632362885445d99cd1cef Mon Sep 17 00:00:00 2001 From: Dan Fox Date: Fri, 5 Mar 2021 20:46:59 +0000 Subject: [PATCH 06/13] auto-fix --- .../baseline/errorprone/PreferImmutableStreamExCollections.java | 1 - .../baseline/extensions/BaselineErrorProneExtension.java | 1 + 2 files changed, 1 insertion(+), 1 deletion(-) diff --git a/baseline-error-prone/src/main/java/com/palantir/baseline/errorprone/PreferImmutableStreamExCollections.java b/baseline-error-prone/src/main/java/com/palantir/baseline/errorprone/PreferImmutableStreamExCollections.java index 000ce60cf..c96572a24 100644 --- a/baseline-error-prone/src/main/java/com/palantir/baseline/errorprone/PreferImmutableStreamExCollections.java +++ b/baseline-error-prone/src/main/java/com/palantir/baseline/errorprone/PreferImmutableStreamExCollections.java @@ -35,7 +35,6 @@ name = "PreferImmutableStreamExCollections", link = "https://github.com/palantir/gradle-baseline#baseline-error-prone-checks", linkType = BugPattern.LinkType.CUSTOM, - providesFix = BugPattern.ProvidesFix.REQUIRES_HUMAN_ATTENTION, severity = SeverityLevel.WARNING, summary = "Prefer immutable/unmodifable collections wherever possible because they are innately threadsafe, " + "and easier to reason about when passed between different functions." diff --git a/gradle-baseline-java/src/main/groovy/com/palantir/baseline/extensions/BaselineErrorProneExtension.java b/gradle-baseline-java/src/main/groovy/com/palantir/baseline/extensions/BaselineErrorProneExtension.java index 17959a41d..57abea4cd 100644 --- a/gradle-baseline-java/src/main/groovy/com/palantir/baseline/extensions/BaselineErrorProneExtension.java +++ b/gradle-baseline-java/src/main/groovy/com/palantir/baseline/extensions/BaselineErrorProneExtension.java @@ -71,6 +71,7 @@ public class BaselineErrorProneExtension { // TODO(ckozak): re-enable pending scala check // "ThrowSpecificity", "UnsafeGaugeRegistration", + "PreferImmutableStreamExCollections", // Built-in checks "ArrayEquals", From 886cc1710e33c365b5e0697b84c951c6cdd3ec6f Mon Sep 17 00:00:00 2001 From: Dan Fox Date: Fri, 5 Mar 2021 20:51:22 +0000 Subject: [PATCH 07/13] One extra test case --- .../PreferImmutableStreamExCollectionsTest.java | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/baseline-error-prone/src/test/java/com/palantir/baseline/errorprone/PreferImmutableStreamExCollectionsTest.java b/baseline-error-prone/src/test/java/com/palantir/baseline/errorprone/PreferImmutableStreamExCollectionsTest.java index 5c0d27e54..ab6be5504 100644 --- a/baseline-error-prone/src/test/java/com/palantir/baseline/errorprone/PreferImmutableStreamExCollectionsTest.java +++ b/baseline-error-prone/src/test/java/com/palantir/baseline/errorprone/PreferImmutableStreamExCollectionsTest.java @@ -28,15 +28,18 @@ void toMap() { "import one.util.streamex.EntryStream;", "import java.util.Map;", "public class Test {", - " Map entryStream = EntryStream.of(Map.of(\"hello\", \"world\")).toMap();", + " Map map = EntryStream.of(Map.of(\"hello\", \"world\")).toMap();", + " EntryStream entryStream = EntryStream.of(Map.of(\"hello\", \"world\"));", + " Map entryStream2 = entryStream.toMap();", "}") .addOutputLines( "Test.java", "import one.util.streamex.EntryStream;", "import java.util.Map;", "public class Test {", - " Map entryStream = EntryStream.of(Map.of(\"hello\", \"world\"))" - + ".toImmutableMap();", + " Map map = EntryStream.of(Map.of(\"hello\", \"world\")).toImmutableMap();", + " EntryStream entryStream = EntryStream.of(Map.of(\"hello\", \"world\"));", + " Map entryStream2 = entryStream.toImmutableMap();", "}") .doTest(BugCheckerRefactoringTestHelper.TestMode.TEXT_MATCH); } From 733a0e2cddae44df58c6917a81c7049cafbf70d0 Mon Sep 17 00:00:00 2001 From: Dan Fox Date: Fri, 5 Mar 2021 20:53:52 +0000 Subject: [PATCH 08/13] --write-locks --- versions.lock | 1 + 1 file changed, 1 insertion(+) diff --git a/versions.lock b/versions.lock index fd8c17e9a..215bcd699 100644 --- a/versions.lock +++ b/versions.lock @@ -105,6 +105,7 @@ javax.activation:javax.activation-api:1.2.0 (1 constraints: 450a28bf) javax.xml.bind:jaxb-api:2.3.1 (1 constraints: c0069559) junit:junit-dep:4.11 (1 constraints: ba1063b3) net.lingala.zip4j:zip4j:1.3.2 (1 constraints: 0805fb35) +one.util:streamex:0.7.3 (1 constraints: 0c050336) org.apiguardian:apiguardian-api:1.1.0 (6 constraints: 18697c5a) org.jooq:jooq:3.14.8 (1 constraints: 4205493b) org.junit:junit-bom:5.7.1 (7 constraints: 6377a0d6) From ad864eff50c72c233799ff183c696c2058631bad Mon Sep 17 00:00:00 2001 From: Dan Fox Date: Fri, 5 Mar 2021 20:53:52 +0000 Subject: [PATCH 09/13] Add generated changelog entries --- changelog/@unreleased/pr-1670.v2.yml | 5 +++++ 1 file changed, 5 insertions(+) create mode 100644 changelog/@unreleased/pr-1670.v2.yml diff --git a/changelog/@unreleased/pr-1670.v2.yml b/changelog/@unreleased/pr-1670.v2.yml new file mode 100644 index 000000000..6c5b30117 --- /dev/null +++ b/changelog/@unreleased/pr-1670.v2.yml @@ -0,0 +1,5 @@ +type: improvement +improvement: + description: A new error-prone rule `PreferImmutableStreamExCollections` converts the StreamEx `toMap()` -> `toImmutableMap()`, `toImmutableList()` and `toImmutableSet()` + links: + - https://github.com/palantir/gradle-baseline/pull/1670 From 1623a36e1edcaccbfe39c371b3f15707d8dc57c8 Mon Sep 17 00:00:00 2001 From: Dan Fox Date: Fri, 5 Mar 2021 21:02:33 +0000 Subject: [PATCH 10/13] nice --- .../PreferImmutableStreamExCollections.java | 21 +++++++++---------- 1 file changed, 10 insertions(+), 11 deletions(-) diff --git a/baseline-error-prone/src/main/java/com/palantir/baseline/errorprone/PreferImmutableStreamExCollections.java b/baseline-error-prone/src/main/java/com/palantir/baseline/errorprone/PreferImmutableStreamExCollections.java index c96572a24..01fcdce1e 100644 --- a/baseline-error-prone/src/main/java/com/palantir/baseline/errorprone/PreferImmutableStreamExCollections.java +++ b/baseline-error-prone/src/main/java/com/palantir/baseline/errorprone/PreferImmutableStreamExCollections.java @@ -21,11 +21,10 @@ import com.google.errorprone.BugPattern.SeverityLevel; import com.google.errorprone.VisitorState; import com.google.errorprone.bugpatterns.BugChecker; -import com.google.errorprone.fixes.SuggestedFix; +import com.google.errorprone.fixes.SuggestedFixes; import com.google.errorprone.matchers.Description; import com.google.errorprone.matchers.Matcher; import com.google.errorprone.matchers.method.MethodMatchers; -import com.google.errorprone.util.ASTHelpers; import com.sun.source.tree.ExpressionTree; import com.sun.source.tree.MethodInvocationTree; import java.util.Collections; @@ -62,26 +61,26 @@ public final class PreferImmutableStreamExCollections extends BugChecker @Override public Description matchMethodInvocation(MethodInvocationTree tree, VisitorState state) { if (TO_MAP.matches(tree, state)) { - String base = state.getSourceForNode(ASTHelpers.getReceiver(tree.getMethodSelect())); return buildDescription(tree) - .setMessage("Prefer .toImmutableMap()") - .addFix(SuggestedFix.replace(tree.getMethodSelect(), base + ".toImmutableMap")) + .setMessage("Prefer .toImmutableMap() because immutable collections are inherently threadsafe and" + + " easier to reason about when passed between different methods.") + .addFix(SuggestedFixes.renameMethodInvocation(tree, "toImmutableMap", state)) .build(); } if (TO_SET.matches(tree, state)) { - String base = state.getSourceForNode(ASTHelpers.getReceiver(tree.getMethodSelect())); return buildDescription(tree) - .setMessage("Prefer .toImmutableSet()") - .addFix(SuggestedFix.replace(tree.getMethodSelect(), base + ".toImmutableSet")) + .setMessage("Prefer .toImmutableSet() because immutable collections are inherently threadsafe and" + + " easier to reason about when passed between different methods.") + .addFix(SuggestedFixes.renameMethodInvocation(tree, "toImmutableSet", state)) .build(); } if (TO_LIST.matches(tree, state)) { - String base = state.getSourceForNode(ASTHelpers.getReceiver(tree.getMethodSelect())); return buildDescription(tree) - .setMessage("Prefer .toImmutableList()") - .addFix(SuggestedFix.replace(tree.getMethodSelect(), base + ".toImmutableList")) + .setMessage("Prefer .toImmutableList() because immutable collections are inherently threadsafe and" + + " easier to reason about when passed between different methods.") + .addFix(SuggestedFixes.renameMethodInvocation(tree, "toImmutableList", state)) .build(); } From 01b47938f49883309d8d4a7f441c2ebaadf056df Mon Sep 17 00:00:00 2001 From: Dan Fox Date: Fri, 5 Mar 2021 21:09:44 +0000 Subject: [PATCH 11/13] even nicer --- .../errorprone/PreferImmutableStreamExCollections.java | 8 +------- 1 file changed, 1 insertion(+), 7 deletions(-) diff --git a/baseline-error-prone/src/main/java/com/palantir/baseline/errorprone/PreferImmutableStreamExCollections.java b/baseline-error-prone/src/main/java/com/palantir/baseline/errorprone/PreferImmutableStreamExCollections.java index 01fcdce1e..434e55011 100644 --- a/baseline-error-prone/src/main/java/com/palantir/baseline/errorprone/PreferImmutableStreamExCollections.java +++ b/baseline-error-prone/src/main/java/com/palantir/baseline/errorprone/PreferImmutableStreamExCollections.java @@ -35,7 +35,7 @@ link = "https://github.com/palantir/gradle-baseline#baseline-error-prone-checks", linkType = BugPattern.LinkType.CUSTOM, severity = SeverityLevel.WARNING, - summary = "Prefer immutable/unmodifable collections wherever possible because they are innately threadsafe, " + summary = "Prefer immutable/unmodifable collections wherever possible because they are inherently threadsafe " + "and easier to reason about when passed between different functions." + " If you really want a mutable output then explicitly suppress this check.") public final class PreferImmutableStreamExCollections extends BugChecker @@ -62,24 +62,18 @@ public final class PreferImmutableStreamExCollections extends BugChecker public Description matchMethodInvocation(MethodInvocationTree tree, VisitorState state) { if (TO_MAP.matches(tree, state)) { return buildDescription(tree) - .setMessage("Prefer .toImmutableMap() because immutable collections are inherently threadsafe and" - + " easier to reason about when passed between different methods.") .addFix(SuggestedFixes.renameMethodInvocation(tree, "toImmutableMap", state)) .build(); } if (TO_SET.matches(tree, state)) { return buildDescription(tree) - .setMessage("Prefer .toImmutableSet() because immutable collections are inherently threadsafe and" - + " easier to reason about when passed between different methods.") .addFix(SuggestedFixes.renameMethodInvocation(tree, "toImmutableSet", state)) .build(); } if (TO_LIST.matches(tree, state)) { return buildDescription(tree) - .setMessage("Prefer .toImmutableList() because immutable collections are inherently threadsafe and" - + " easier to reason about when passed between different methods.") .addFix(SuggestedFixes.renameMethodInvocation(tree, "toImmutableList", state)) .build(); } From 7fcca303be8e09e3ba556ceb5f5dbeabb8e53316 Mon Sep 17 00:00:00 2001 From: Dan Fox Date: Fri, 5 Mar 2021 22:09:14 +0000 Subject: [PATCH 12/13] Handle .collect(Collectors.toList()) and toSet() too --- .../PreferImmutableStreamExCollections.java | 49 ++++++++++++++++--- ...referImmutableStreamExCollectionsTest.java | 8 +++ 2 files changed, 51 insertions(+), 6 deletions(-) diff --git a/baseline-error-prone/src/main/java/com/palantir/baseline/errorprone/PreferImmutableStreamExCollections.java b/baseline-error-prone/src/main/java/com/palantir/baseline/errorprone/PreferImmutableStreamExCollections.java index 434e55011..e8635f6d5 100644 --- a/baseline-error-prone/src/main/java/com/palantir/baseline/errorprone/PreferImmutableStreamExCollections.java +++ b/baseline-error-prone/src/main/java/com/palantir/baseline/errorprone/PreferImmutableStreamExCollections.java @@ -21,6 +21,7 @@ import com.google.errorprone.BugPattern.SeverityLevel; import com.google.errorprone.VisitorState; import com.google.errorprone.bugpatterns.BugChecker; +import com.google.errorprone.fixes.SuggestedFix; import com.google.errorprone.fixes.SuggestedFixes; import com.google.errorprone.matchers.Description; import com.google.errorprone.matchers.Matcher; @@ -43,41 +44,77 @@ public final class PreferImmutableStreamExCollections extends BugChecker private static final long serialVersionUID = 1L; - private static final Matcher TO_MAP = MethodMatchers.instanceMethod() + private static final Matcher STREAMEX_TO_MAP = MethodMatchers.instanceMethod() .onExactClass("one.util.streamex.EntryStream") .named("toMap") .withParameters(Collections.emptyList()); - private static final Matcher TO_SET = MethodMatchers.instanceMethod() + private static final Matcher STREAMEX_TO_SET = MethodMatchers.instanceMethod() .onDescendantOf("one.util.streamex.AbstractStreamEx") .named("toSet") .withParameters(Collections.emptyList()); - private static final Matcher TO_LIST = MethodMatchers.instanceMethod() + private static final Matcher STREAMEX_TO_LIST = MethodMatchers.instanceMethod() .onDescendantOf("one.util.streamex.AbstractStreamEx") .named("toList") .withParameters(Collections.emptyList()); + private static final Matcher STREAMEX_COLLECT = MethodMatchers.instanceMethod() + .onDescendantOf("one.util.streamex.AbstractStreamEx") + .named("collect"); + + private static final Matcher COLLECT_TO_SET = MethodMatchers.staticMethod() + .onClass("java.util.stream.Collectors") + .named("toSet") + .withParameters(Collections.emptyList()); + + private static final Matcher COLLECT_TO_LIST = MethodMatchers.staticMethod() + .onClass("java.util.stream.Collectors") + .named("toList") + .withParameters(Collections.emptyList()); + @Override public Description matchMethodInvocation(MethodInvocationTree tree, VisitorState state) { - if (TO_MAP.matches(tree, state)) { + if (STREAMEX_TO_MAP.matches(tree, state)) { return buildDescription(tree) .addFix(SuggestedFixes.renameMethodInvocation(tree, "toImmutableMap", state)) .build(); } - if (TO_SET.matches(tree, state)) { + if (STREAMEX_TO_SET.matches(tree, state)) { return buildDescription(tree) .addFix(SuggestedFixes.renameMethodInvocation(tree, "toImmutableSet", state)) .build(); } - if (TO_LIST.matches(tree, state)) { + if (STREAMEX_TO_LIST.matches(tree, state)) { return buildDescription(tree) .addFix(SuggestedFixes.renameMethodInvocation(tree, "toImmutableList", state)) .build(); } + if (STREAMEX_COLLECT.matches(tree, state) && tree.getArguments().size() == 1) { + ExpressionTree argument = tree.getArguments().get(0); + + if (COLLECT_TO_SET.matches(argument, state)) { + return buildDescription(tree) + .addFix(SuggestedFix.builder() + .merge(SuggestedFix.delete(argument)) + .merge(SuggestedFixes.renameMethodInvocation(tree, "toImmutableSet", state)) + .build()) + .build(); + } + + if (COLLECT_TO_LIST.matches(argument, state)) { + return buildDescription(tree) + .addFix(SuggestedFix.builder() + .merge(SuggestedFix.delete(argument)) + .merge(SuggestedFixes.renameMethodInvocation(tree, "toImmutableList", state)) + .build()) + .build(); + } + } + return Description.NO_MATCH; } } diff --git a/baseline-error-prone/src/test/java/com/palantir/baseline/errorprone/PreferImmutableStreamExCollectionsTest.java b/baseline-error-prone/src/test/java/com/palantir/baseline/errorprone/PreferImmutableStreamExCollectionsTest.java index ab6be5504..e1af0c0fb 100644 --- a/baseline-error-prone/src/test/java/com/palantir/baseline/errorprone/PreferImmutableStreamExCollectionsTest.java +++ b/baseline-error-prone/src/test/java/com/palantir/baseline/errorprone/PreferImmutableStreamExCollectionsTest.java @@ -49,16 +49,20 @@ void toSet() { fix().addInputLines( "Test.java", "import one.util.streamex.StreamEx;", + "import java.util.stream.Collectors;", "import java.util.Set;", "public class Test {", " Set s = StreamEx.of(\"Hello\").toSet();", + " Set s2 = StreamEx.of(\"Hello\").collect(Collectors.toSet());", "}") .addOutputLines( "Test.java", "import one.util.streamex.StreamEx;", + "import java.util.stream.Collectors;", "import java.util.Set;", "public class Test {", " Set s = StreamEx.of(\"Hello\").toImmutableSet();", + " Set s2 = StreamEx.of(\"Hello\").toImmutableSet();", "}") .doTest(BugCheckerRefactoringTestHelper.TestMode.TEXT_MATCH); } @@ -69,15 +73,19 @@ void toList() { "Test.java", "import one.util.streamex.StreamEx;", "import java.util.List;", + "import static java.util.stream.Collectors.toList;", "public class Test {", " List s = StreamEx.of(\"Hello\").toList();", + " List s2 = StreamEx.of(\"Hello\").collect(toList());", "}") .addOutputLines( "Test.java", "import one.util.streamex.StreamEx;", "import java.util.List;", + "import static java.util.stream.Collectors.toList;", "public class Test {", " List s = StreamEx.of(\"Hello\").toImmutableList();", + " List s2 = StreamEx.of(\"Hello\").toImmutableList();", "}") .doTest(BugCheckerRefactoringTestHelper.TestMode.TEXT_MATCH); } From 235e27cc71f23ab1880c6007c8df2d248f63f941 Mon Sep 17 00:00:00 2001 From: Dan Fox Date: Fri, 5 Mar 2021 22:11:44 +0000 Subject: [PATCH 13/13] More concise --- .../errorprone/PreferImmutableStreamExCollections.java | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/baseline-error-prone/src/main/java/com/palantir/baseline/errorprone/PreferImmutableStreamExCollections.java b/baseline-error-prone/src/main/java/com/palantir/baseline/errorprone/PreferImmutableStreamExCollections.java index e8635f6d5..4734f532e 100644 --- a/baseline-error-prone/src/main/java/com/palantir/baseline/errorprone/PreferImmutableStreamExCollections.java +++ b/baseline-error-prone/src/main/java/com/palantir/baseline/errorprone/PreferImmutableStreamExCollections.java @@ -99,7 +99,7 @@ public Description matchMethodInvocation(MethodInvocationTree tree, VisitorState if (COLLECT_TO_SET.matches(argument, state)) { return buildDescription(tree) .addFix(SuggestedFix.builder() - .merge(SuggestedFix.delete(argument)) + .delete(argument) .merge(SuggestedFixes.renameMethodInvocation(tree, "toImmutableSet", state)) .build()) .build(); @@ -108,7 +108,7 @@ public Description matchMethodInvocation(MethodInvocationTree tree, VisitorState if (COLLECT_TO_LIST.matches(argument, state)) { return buildDescription(tree) .addFix(SuggestedFix.builder() - .merge(SuggestedFix.delete(argument)) + .delete(argument) .merge(SuggestedFixes.renameMethodInvocation(tree, "toImmutableList", state)) .build()) .build();