Skip to content

Commit

Permalink
DangerousIdentityKey validates additional hash-based collections (#…
Browse files Browse the repository at this point in the history 
…1735)

`DangerousIdentityKey` validates additional hash-based collections
  • Loading branch information
@carterkozak
carterkozak authored Apr 18, 2021
1 parent 959bd6c commit 97f12b3
Show file tree
Hide file tree
Showing 3 changed files with 226 additions and 2 deletions.
43 changes: 41 additions & 2 deletions ...ror-prone/src/main/java/com/palantir/baseline/errorprone/MoreAbstractAsKeyOfSetOrMap.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -78,8 +78,47 @@ abstract class MoreAbstractAsKeyOfSetOrMap extends AbstractAsKeyOfSetOrMap {
.onClass("com.google.common.collect.ImmutableMap")
.named("toImmutableMap");

private static final Matcher<MethodInvocationTree> HASH_KEYED_METHODS =
Matchers.anyOf(GUAVA_CACHE_BUILDER, CAFFEINE_CACHE_BUILDER);
private static final Matcher<ExpressionTree> JAVA_UTIL_MAP_OF =
MethodMatchers.staticMethod().onClass("java.util.Map").named("of");

private static final Matcher<ExpressionTree> JAVA_UTIL_SET_OF =
MethodMatchers.staticMethod().onClass("java.util.Set").named("of");

private static final Matcher<ExpressionTree> IMMUTABLE_MAP_OF = MethodMatchers.staticMethod()
.onClass("com.google.common.collect.ImmutableMap")
.namedAnyOf("of", "copyOf");

private static final Matcher<ExpressionTree> IMMUTABLE_MAP_BUILDER = MethodMatchers.instanceMethod()
.onDescendantOf("com.google.common.collect.ImmutableMap.Builder")
.named("build");

private static final Matcher<ExpressionTree> IMMUTABLE_SET_OF = MethodMatchers.staticMethod()
.onClass("com.google.common.collect.ImmutableSet")
.namedAnyOf("of", "copyOf");

private static final Matcher<ExpressionTree> IMMUTABLE_SET_BUILDER = MethodMatchers.instanceMethod()
.onDescendantOf("com.google.common.collect.ImmutableSet.Builder")
.named("build");

private static final Matcher<ExpressionTree> STREAMEX_TO_MAP = MethodMatchers.instanceMethod()
.onDescendantOf("one.util.streamex.EntryStream")
.namedAnyOf("toMap", "toImmutableMap");

private static final Matcher<ExpressionTree> STREAMEX_TO_SET = MethodMatchers.instanceMethod()
.onDescendantOf("one.util.streamex.AbstractStreamEx")
.namedAnyOf("toSet", "toImmutableSet");

private static final Matcher<MethodInvocationTree> 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<MethodInvocationTree> HASH_KEYED_COLLECTOR_METHODS = Matchers.anyOf(
SET_COLLECTOR,
Expand Down
180 changes: 180 additions & 0 deletions ...-error-prone/src/test/java/com/palantir/baseline/errorprone/DangerousIdentityKeyTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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<Pattern, String> test() {",
" return Stream.of(\".\").collect(",
" // BUG: Diagnostic contains: does not override",
" Collectors.toUnmodifiableMap(Pattern::compile, s -> s));",
" }",
"}")
.doTest();
}

@Test
void testCollectMapValidKey() {
compilationHelper
Expand Down Expand Up @@ -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.<Pattern, String>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<Pattern> test() {",
" // BUG: Diagnostic contains: does not override",
" return ImmutableSet.<Pattern>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<Pattern> 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<Pattern> 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<Pattern, String> 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<Pattern, String> stream) {",
" // BUG: Diagnostic contains: does not override",
" return stream.toImmutableMap();",
" }",
"}")
.doTest();
}
}
5 changes: 5 additions & 0 deletions changelog/@unreleased/pr-1735.v2.yml
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
type: improvement
improvement:
description: '`DangerousIdentityKey` validates additional hash-based collections'
links:
- https://github.com/palantir/gradle-baseline/pull/1735

0 comments on commit 97f12b3

Please sign in to comment.