fix #1471: Error-prone DnsLookup check to prevent unintentional resol…

…ution (#1472)

Error-prone DnsLookup check to prevent unintentional resolution

README.md

@@ -166,6 +166,7 @@ Safe Logging can be found at [github.com/palantir/safe-logging](https://github.c
 - `StringBuilderConstantParameters`: StringBuilder with a constant number of parameters should be replaced by simple concatenation.
 - `JUnit5SuiteMisuse`: When migrating from JUnit4 -> JUnit5, classes annotated with `@RunWith(Suite.class)` are dangerous because if they reference any JUnit5 test classes, these tests will silently not run!
 - `ThrowError`: Prefer throwing a RuntimeException rather than Error.
+- `DnsLookup`: Calling `new InetSocketAddress(host, port)` results in a DNS lookup which prevents the address from following DNS changes.
 - `ReverseDnsLookup`: Calling address.getHostName may result in an unexpected DNS lookup.
 - `ReadReturnValueIgnored`: The result of a read call must be checked to know if EOF has been reached or the expected number of bytes have been consumed.
 - `FinalClass`: A class should be declared final if all of its constructors are private.

baseline-error-prone/src/main/java/com/palantir/baseline/errorprone/DnsLookup.java

@@ -0,0 +1,71 @@
+/*
+ * (c) Copyright 2019 Palantir Technologies Inc. All rights reserved.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package com.palantir.baseline.errorprone;
+
+import com.google.auto.service.AutoService;
+import com.google.errorprone.BugPattern;
+import com.google.errorprone.VisitorState;
+import com.google.errorprone.bugpatterns.BugChecker;
+import com.google.errorprone.fixes.SuggestedFix;
+import com.google.errorprone.matchers.Description;
+import com.google.errorprone.matchers.Matcher;
+import com.google.errorprone.matchers.method.MethodMatchers;
+import com.sun.source.tree.ExpressionTree;
+import com.sun.source.tree.NewClassTree;
+import com.sun.tools.javac.tree.JCTree;
+import java.net.InetSocketAddress;
+
+@AutoService(BugChecker.class)
+@BugPattern(
+        name = "DnsLookup",
+        link = "https://github.com/palantir/gradle-baseline#baseline-error-prone-checks",
+        linkType = BugPattern.LinkType.CUSTOM,
+        severity = BugPattern.SeverityLevel.WARNING,
+        summary = "Calling 'new InetSocketAddress(host, port)' results in a DNS lookup which prevents the address "
+                + "from following DNS changes in the future because it's already resolved. Additionally this is a "
+                + "potential a network request, making the invocation significantly more expensive than expected "
+                + "depending on the environment.\n"
+                + "This check is intended to be advisory - it's fine to @SuppressWarnings(\"DnsLookup\") in"
+                + " certain cases, but is usually not recommended.")
+public final class DnsLookup extends BugChecker implements BugChecker.NewClassTreeMatcher {
+
+    private static final Matcher<ExpressionTree> INET_SOCKET_ADDRESS_MATCHER = MethodMatchers.constructor()
+            .forClass(InetSocketAddress.class.getName())
+            .withParameters(String.class.getName(), int.class.getName());
+
+    @Override
+    public Description matchNewClass(NewClassTree tree, VisitorState state) {
+        if (tree.getClassBody() == null && INET_SOCKET_ADDRESS_MATCHER.matches(tree, state)) {
+            return buildDescription(tree)
+                    // Suggested fix exists to provide context when compilation fails, it shouldn't be used
+                    // as a drop in replacement because the unresolved string may not be sufficient in some
+                    // cases.
+                    .addFix(SuggestedFix.builder()
+                            .replace(
+                                    startPosition(tree),
+                                    state.getEndPosition(tree.getIdentifier()),
+                                    state.getSourceForNode(tree.getIdentifier()) + ".createUnresolved")
+                            .build())
+                    .build();
+        }
+        return Description.NO_MATCH;
+    }
+
+    private static int startPosition(ExpressionTree tree) {
+        return ((JCTree) tree).getStartPosition();
+    }
+}

baseline-error-prone/src/test/java/com/palantir/baseline/errorprone/DnsLookupTest.java

@@ -0,0 +1,47 @@
+/*
+ * (c) Copyright 2020 Palantir Technologies Inc. All rights reserved.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package com.palantir.baseline.errorprone;
+
+import org.junit.jupiter.api.Test;
+
+class DnsLookupTest {
+
+    @Test
+    void testFix() {
+        fix().addInputLines(
+                        "Test.java",
+                        "import java.net.InetSocketAddress;",
+                        "class Test {",
+                        "  InetSocketAddress f() {",
+                        "    return new InetSocketAddress(\"host\", 443);",
+                        "  }",
+                        "}")
+                .addOutputLines(
+                        "Test.java",
+                        "import java.net.InetSocketAddress;",
+                        "class Test {",
+                        "  InetSocketAddress f() {",
+                        "    return InetSocketAddress.createUnresolved(\"host\", 443);",
+                        "  }",
+                        "}")
+                .doTest();
+    }
+
+    private RefactoringValidator fix() {
+        return RefactoringValidator.of(new DnsLookup(), getClass());
+    }
+}

changelog/@unreleased/pr-1472.v2.yml

@@ -0,0 +1,5 @@
+type: improvement
+improvement:
+  description: Error-prone DnsLookup check to prevent unintentional resolution
+  links:
+  - https://github.com/palantir/gradle-baseline/pull/1472