diff --git a/core/src/main/java/com/google/errorprone/bugpatterns/URLEqualsHashCode.java b/core/src/main/java/com/google/errorprone/bugpatterns/URLEqualsHashCode.java index c6f4668f4a2..86321362b62 100644 --- a/core/src/main/java/com/google/errorprone/bugpatterns/URLEqualsHashCode.java +++ b/core/src/main/java/com/google/errorprone/bugpatterns/URLEqualsHashCode.java @@ -18,15 +18,26 @@ import static com.google.errorprone.BugPattern.Category.JDK; import static com.google.errorprone.BugPattern.SeverityLevel.WARNING; +import static com.google.errorprone.matchers.Matchers.allOf; import static com.google.errorprone.matchers.Matchers.anyOf; +import static com.google.errorprone.matchers.method.MethodMatchers.instanceMethod; +import static com.google.errorprone.matchers.method.MethodMatchers.staticMethod; +import com.google.common.collect.HashBiMap; +import com.google.common.collect.ImmutableList; +import com.google.common.collect.ImmutableMap; +import com.google.common.collect.ImmutableSet; import com.google.errorprone.BugPattern; import com.google.errorprone.BugPattern.StandardTags; import com.google.errorprone.VisitorState; +import com.google.errorprone.bugpatterns.BugChecker.MethodInvocationTreeMatcher; import com.google.errorprone.bugpatterns.BugChecker.NewClassTreeMatcher; import com.google.errorprone.matchers.Description; import com.google.errorprone.matchers.Matcher; +import com.google.errorprone.predicates.TypePredicates; import com.google.errorprone.util.ASTHelpers; +import com.sun.source.tree.ExpressionTree; +import com.sun.source.tree.MethodInvocationTree; import com.sun.source.tree.NewClassTree; import com.sun.source.tree.Tree; import com.sun.tools.javac.code.Symbol; @@ -44,24 +55,52 @@ @BugPattern( name = "URLEqualsHashCode", summary = - "Creation of a Set/HashSet/HashMap of java.net.URL." - + " equals() and hashCode() of java.net.URL class make blocking internet connections.", + "Avoid hash-based containers of java.net.URL--the containers rely on equals() and hashCode()," + + " which cause java.net.URL to make blocking internet connections.", category = JDK, severity = WARNING, tags = StandardTags.FRAGILE_CODE ) -public class URLEqualsHashCode extends BugChecker implements NewClassTreeMatcher { +public class URLEqualsHashCode extends BugChecker + implements MethodInvocationTreeMatcher, NewClassTreeMatcher { private static final String URL_CLASS = "java.net.URL"; - private static final Matcher TYPE_MATCHER = + private static final Matcher CONTAINER_MATCHER = anyOf( new URLTypeArgumentMatcher("java.util.Set", 0), - new URLTypeArgumentMatcher("java.util.Map", 0)); + new URLTypeArgumentMatcher("java.util.Map", 0), + // BiMap is a Map, so its first type argument is already being checked + new URLTypeArgumentMatcher("com.google.common.collect.BiMap", 1)); + + private static final Matcher METHOD_INVOCATION_MATCHER = + allOf( + anyOf( + staticMethod() + .onClassAny( + ImmutableSet.class.getName(), + ImmutableMap.class.getName(), + HashBiMap.class.getName()), + instanceMethod() + .onClass( + TypePredicates.isExactTypeAny( + ImmutableList.of( + ImmutableSet.Builder.class.getName(), + ImmutableMap.Builder.class.getName()))) + .named("build")), + CONTAINER_MATCHER); + + @Override + public Description matchMethodInvocation(MethodInvocationTree tree, VisitorState state) { + if (METHOD_INVOCATION_MATCHER.matches(tree, state)) { + return describeMatch(tree); + } + return Description.NO_MATCH; + } @Override public Description matchNewClass(NewClassTree tree, VisitorState state) { - if (TYPE_MATCHER.matches(tree, state)) { + if (CONTAINER_MATCHER.matches(tree, state)) { return describeMatch(tree); } return Description.NO_MATCH; @@ -100,3 +139,4 @@ public boolean matches(Tree tree, VisitorState state) { } } } + diff --git a/core/src/test/java/com/google/errorprone/bugpatterns/testdata/URLEqualsHashCodePositiveCases.java b/core/src/test/java/com/google/errorprone/bugpatterns/testdata/URLEqualsHashCodePositiveCases.java index 404ad5ddbfc..6785f8c5da1 100644 --- a/core/src/test/java/com/google/errorprone/bugpatterns/testdata/URLEqualsHashCodePositiveCases.java +++ b/core/src/test/java/com/google/errorprone/bugpatterns/testdata/URLEqualsHashCodePositiveCases.java @@ -16,6 +16,9 @@ package com.google.errorprone.bugpatterns.testdata; +import com.google.common.collect.BiMap; +import com.google.common.collect.HashBiMap; +import com.google.common.collect.ImmutableSet; import java.net.URL; import java.util.HashMap; import java.util.HashSet; @@ -82,4 +85,28 @@ public void hashMapExtendedClass() { // BUG: Diagnostic contains: java.net.URL Map urlMap = new ExtendedMap(); } + + public void hashBiMapOfURL() { + // BUG: Diagnostic contains: java.net.URL + BiMap urlBiMap = HashBiMap.create(); + + // BUG: Diagnostic contains: java.net.URL + BiMap toUrlBiMap = HashBiMap.create(); + } + + public void hashBiMapOfCompleteURL() { + // BUG: Diagnostic contains: java.net.URL + HashBiMap urlBiMap = HashBiMap.create(); + + // BUG: Diagnostic contains: java.net.URL + HashBiMap toUrlBiMap = HashBiMap.create(); + } + + public void immutableSetOfURL() { + // BUG: Diagnostic contains: java.net.URL + ImmutableSet urlSet = ImmutableSet.of(); + + // BUG: Diagnostic contains: java.net.URL + ImmutableSet urlSet2 = ImmutableSet.builder().build(); + } }