diff --git a/core/src/main/java/com/google/errorprone/bugpatterns/AddressSelection.java b/core/src/main/java/com/google/errorprone/bugpatterns/AddressSelection.java index 3e2be15e29a..5159374333d 100644 --- a/core/src/main/java/com/google/errorprone/bugpatterns/AddressSelection.java +++ b/core/src/main/java/com/google/errorprone/bugpatterns/AddressSelection.java @@ -25,6 +25,7 @@ import static com.google.errorprone.matchers.method.MethodMatchers.staticMethod; import static com.google.errorprone.util.ASTHelpers.constValue; +import com.google.common.base.CharMatcher; import com.google.common.collect.ImmutableSet; import com.google.errorprone.BugPattern; import com.google.errorprone.VisitorState; @@ -37,13 +38,12 @@ import com.sun.source.tree.ExpressionTree; import com.sun.source.tree.MethodInvocationTree; import com.sun.source.tree.NewClassTree; -import java.util.Objects; import java.util.function.Supplier; /** A {@link BugChecker}; see the associated {@link BugPattern} annotation for details. */ @BugPattern( summary = - "Prefer InetAddress.getAllName to APIs that convert a hostname to a single IP address", + "Prefer InetAddress.getAllByName to APIs that convert a hostname to a single IP address", severity = WARNING) public final class AddressSelection extends BugChecker implements NewClassTreeMatcher, MethodInvocationTreeMatcher { @@ -98,13 +98,36 @@ public Description matchMethodInvocation(MethodInvocationTree tree, VisitorState private Description handleMatch( ExpressionTree argument, ExpressionTree replacement, Supplier fix) { String value = constValue(argument, String.class); - if (Objects.equals(value, "localhost")) { + // If it's a numeric loopback address, suggest using the method for that. + if (LOOPBACK.contains(value)) { + return describeMatch(replacement, fix.get()); + } + // If it isn't a constant, or it's "localhost", or it looks like a numeric IP address, then + // we don't say anything. + if (value == null || value.equals("localhost") || isNumericIp(value)) { return NO_MATCH; } - Description.Builder description = buildDescription(replacement); - if (LOOPBACK.contains(value)) { - description.addFix(fix.get()); + // Otherwise flag it but don't suggest a fix. + return describeMatch(replacement); + } + + /** + * Returns true if this string looks like it might be a numeric IP address. The matching here is + * very approximate. We want every numeric IP address to return true, but it's OK if some strings + * return true even though they are not actually valid numeric IP addresses. Actually parsing + * numeric IPv6 addresses in all their glory is more than we need here. + */ + private static boolean isNumericIp(String value) { + if (value.isEmpty()) { + return false; + } + if (value.contains(":")) { + return true; // Every numeric IPv6 address contains a colon and no non-numeric hostname does. } - return description.build(); + return ASCII_DIGIT.matches(value.charAt(0)) + && ASCII_DIGIT.matches(value.charAt(value.length() - 1)); + // Every numeric IPv4 address begins and ends with an ASCII digit. } + + private static final CharMatcher ASCII_DIGIT = CharMatcher.inRange('0', '9'); } diff --git a/core/src/test/java/com/google/errorprone/bugpatterns/AddressSelectionTest.java b/core/src/test/java/com/google/errorprone/bugpatterns/AddressSelectionTest.java index 0d1537570ac..b2b48638c07 100644 --- a/core/src/test/java/com/google/errorprone/bugpatterns/AddressSelectionTest.java +++ b/core/src/test/java/com/google/errorprone/bugpatterns/AddressSelectionTest.java @@ -88,6 +88,24 @@ public void negativeLocalhost() throws Exception { .doTest(); } + @Test + public void negativeNumeric() throws Exception { + compilationHelper + .addSourceLines( + "Test.java", + "import java.net.InetAddress;", + "import java.net.InetSocketAddress;", + "import java.net.Socket;", + "class Test {", + " void f() throws Exception {", + " new Socket(\"1.2.3.4\", 80);", + " InetAddress.getByName(\"2001:db8:85a3:8d3:1319:8a2e:370:7348\");", + " new InetSocketAddress(\"::ffff:192.0.2.128\", 80);", + " }", + "}") + .doTest(); + } + @Test public void refactor() throws Exception { refactoringTestHelper