From 44b65527debbc57892f21ca3ba458b16771e423e Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?=C3=89amonn=20McManus?= <emcmanus@google.com>
Date: Thu, 10 Aug 2023 08:41:13 -0700
Subject: [PATCH] Don't complain about literal IP addresses in
 `AddressSelection`.

A correct literal IP address will always resolve to the same `InetAddress` value so there is no point in suggesting the use of `getAllByName`.

Also fix a typo: `getAllName`.

PiperOrigin-RevId: 555506842
---
 .../bugpatterns/AddressSelection.java         | 37 +++++++++++++++----
 .../bugpatterns/AddressSelectionTest.java     | 18 +++++++++
 2 files changed, 48 insertions(+), 7 deletions(-)

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<SuggestedFix> 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