diff --git a/baseline-error-prone/src/main/java/com/palantir/baseline/errorprone/MoreAbstractAsKeyOfSetOrMap.java b/baseline-error-prone/src/main/java/com/palantir/baseline/errorprone/MoreAbstractAsKeyOfSetOrMap.java index 280bea0d2..5c85ad863 100644 --- a/baseline-error-prone/src/main/java/com/palantir/baseline/errorprone/MoreAbstractAsKeyOfSetOrMap.java +++ b/baseline-error-prone/src/main/java/com/palantir/baseline/errorprone/MoreAbstractAsKeyOfSetOrMap.java @@ -78,8 +78,47 @@ abstract class MoreAbstractAsKeyOfSetOrMap extends AbstractAsKeyOfSetOrMap { .onClass("com.google.common.collect.ImmutableMap") .named("toImmutableMap"); - private static final Matcher HASH_KEYED_METHODS = - Matchers.anyOf(GUAVA_CACHE_BUILDER, CAFFEINE_CACHE_BUILDER); + private static final Matcher JAVA_UTIL_MAP_OF = + MethodMatchers.staticMethod().onClass("java.util.Map").named("of"); + + private static final Matcher JAVA_UTIL_SET_OF = + MethodMatchers.staticMethod().onClass("java.util.Set").named("of"); + + private static final Matcher IMMUTABLE_MAP_OF = MethodMatchers.staticMethod() + .onClass("com.google.common.collect.ImmutableMap") + .namedAnyOf("of", "copyOf"); + + private static final Matcher IMMUTABLE_MAP_BUILDER = MethodMatchers.instanceMethod() + .onDescendantOf("com.google.common.collect.ImmutableMap.Builder") + .named("build"); + + private static final Matcher IMMUTABLE_SET_OF = MethodMatchers.staticMethod() + .onClass("com.google.common.collect.ImmutableSet") + .namedAnyOf("of", "copyOf"); + + private static final Matcher IMMUTABLE_SET_BUILDER = MethodMatchers.instanceMethod() + .onDescendantOf("com.google.common.collect.ImmutableSet.Builder") + .named("build"); + + private static final Matcher STREAMEX_TO_MAP = MethodMatchers.instanceMethod() + .onDescendantOf("one.util.streamex.EntryStream") + .namedAnyOf("toMap", "toImmutableMap"); + + private static final Matcher STREAMEX_TO_SET = MethodMatchers.instanceMethod() + .onDescendantOf("one.util.streamex.AbstractStreamEx") + .namedAnyOf("toSet", "toImmutableSet"); + + private static final Matcher HASH_KEYED_METHODS = Matchers.anyOf( + GUAVA_CACHE_BUILDER, + CAFFEINE_CACHE_BUILDER, + JAVA_UTIL_MAP_OF, + JAVA_UTIL_SET_OF, + IMMUTABLE_MAP_OF, + IMMUTABLE_MAP_BUILDER, + IMMUTABLE_SET_OF, + IMMUTABLE_SET_BUILDER, + STREAMEX_TO_MAP, + STREAMEX_TO_SET); private static final Matcher HASH_KEYED_COLLECTOR_METHODS = Matchers.anyOf( SET_COLLECTOR, diff --git a/baseline-error-prone/src/test/java/com/palantir/baseline/errorprone/DangerousIdentityKeyTest.java b/baseline-error-prone/src/test/java/com/palantir/baseline/errorprone/DangerousIdentityKeyTest.java index 3afac343f..2d0a1830d 100644 --- a/baseline-error-prone/src/test/java/com/palantir/baseline/errorprone/DangerousIdentityKeyTest.java +++ b/baseline-error-prone/src/test/java/com/palantir/baseline/errorprone/DangerousIdentityKeyTest.java @@ -222,6 +222,24 @@ void testCollectMapInvalidKey() { .doTest(); } + @Test + void testCollectUnmodifiableMapInvalidKey() { + compilationHelper + .addSourceLines( + "Test.java", + "import java.util.*;", + "import java.util.regex.Pattern;", + "import java.util.stream.*;", + "class Test {", + " private Map test() {", + " return Stream.of(\".\").collect(", + " // BUG: Diagnostic contains: does not override", + " Collectors.toUnmodifiableMap(Pattern::compile, s -> s));", + " }", + "}") + .doTest(); + } + @Test void testCollectMapValidKey() { compilationHelper @@ -337,4 +355,166 @@ void testCaffeineCacheValidKey() { "}") .doTest(); } + + @Test + void testImmutableMapInvalidKey() { + compilationHelper + .addSourceLines( + "Test.java", + "import com.google.common.collect.*;", + "import java.util.regex.Pattern;", + "class Test {", + " private Object test() {", + " // BUG: Diagnostic contains: does not override", + " return ImmutableMap.of(Pattern.compile(\".\"), \"str\");", + " }", + "}") + .doTest(); + } + + @Test + void testImmutableMapBuilderInvalidKey() { + compilationHelper + .addSourceLines( + "Test.java", + "import com.google.common.collect.*;", + "import java.util.regex.Pattern;", + "class Test {", + " private Object test() {", + " return ImmutableMap.builder()", + " .put(Pattern.compile(\".\"), \"str\")", + " // BUG: Diagnostic contains: does not override", + " .build();", + " }", + "}") + .doTest(); + } + + @Test + void testImmutableSetInvalidKey() { + compilationHelper + .addSourceLines( + "Test.java", + "import com.google.common.collect.*;", + "import java.util.regex.Pattern;", + "class Test {", + " private Object test() {", + " // BUG: Diagnostic contains: does not override", + " return ImmutableSet.of(Pattern.compile(\".\"));", + " }", + "}") + .doTest(); + } + + @Test + void testImmutableSetBuilderInvalidKey() { + compilationHelper + .addSourceLines( + "Test.java", + "import com.google.common.collect.*;", + "import java.util.regex.Pattern;", + "class Test {", + " private ImmutableSet test() {", + " // BUG: Diagnostic contains: does not override", + " return ImmutableSet.builder().add(Pattern.compile(\".\")).build();", + " }", + "}") + .doTest(); + } + + @Test + void testJavaUtilMapInvalidKey() { + compilationHelper + .addSourceLines( + "Test.java", + "import java.util.*;", + "import java.util.regex.Pattern;", + "class Test {", + " private Object test() {", + " // BUG: Diagnostic contains: does not override", + " return Map.of(Pattern.compile(\".\"), \"str\");", + " }", + "}") + .doTest(); + } + + @Test + void testJavaUtilSetInvalidKey() { + compilationHelper + .addSourceLines( + "Test.java", + "import java.util.*;", + "import java.util.regex.Pattern;", + "class Test {", + " private Object test() {", + " // BUG: Diagnostic contains: does not override", + " return Set.of(Pattern.compile(\".\"));", + " }", + "}") + .doTest(); + } + + @Test + void testStreamExSetInvalidKey() { + compilationHelper + .addSourceLines( + "Test.java", + "import one.util.streamex.*;", + "import java.util.regex.Pattern;", + "class Test {", + " private Object test(StreamEx stream) {", + " // BUG: Diagnostic contains: does not override", + " return stream.toSet();", + " }", + "}") + .doTest(); + } + + @Test + void testStreamExImmutableSetInvalidKey() { + compilationHelper + .addSourceLines( + "Test.java", + "import one.util.streamex.*;", + "import java.util.regex.Pattern;", + "class Test {", + " private Object test(StreamEx stream) {", + " // BUG: Diagnostic contains: does not override", + " return stream.toImmutableSet();", + " }", + "}") + .doTest(); + } + + @Test + void testStreamExMapInvalidKey() { + compilationHelper + .addSourceLines( + "Test.java", + "import one.util.streamex.*;", + "import java.util.regex.Pattern;", + "class Test {", + " private Object test(EntryStream stream) {", + " // BUG: Diagnostic contains: does not override", + " return stream.toMap();", + " }", + "}") + .doTest(); + } + + @Test + void testStreamExImmutableMapInvalidKey() { + compilationHelper + .addSourceLines( + "Test.java", + "import one.util.streamex.*;", + "import java.util.regex.Pattern;", + "class Test {", + " private Object test(EntryStream stream) {", + " // BUG: Diagnostic contains: does not override", + " return stream.toImmutableMap();", + " }", + "}") + .doTest(); + } } diff --git a/changelog/@unreleased/pr-1735.v2.yml b/changelog/@unreleased/pr-1735.v2.yml new file mode 100644 index 000000000..fd2cdd08e --- /dev/null +++ b/changelog/@unreleased/pr-1735.v2.yml @@ -0,0 +1,5 @@ +type: improvement +improvement: + description: '`DangerousIdentityKey` validates additional hash-based collections' + links: + - https://github.com/palantir/gradle-baseline/pull/1735