Correctly handle var in StringSplitter.

Fixes google#1129 RELNOTES: Correctly handle `var` in StringSplitter ------------- Created by MOE: https://github.com/google/moe MOE_MIGRATED_REVID=215293803
epmjohnston · Oct 2, 2018 · 2984f02 · 2984f02
1 parent be0f2b4
commit 2984f02
Show file tree

Hide file tree

Showing 2 changed files with 49 additions and 2 deletions.
diff --git a/core/src/main/java/com/google/errorprone/bugpatterns/StringSplitter.java b/core/src/main/java/com/google/errorprone/bugpatterns/StringSplitter.java
@@ -206,11 +206,21 @@ public Boolean visitMemberSelect(MemberSelectTree tree, Void aVoid) {
         return Optional.empty();
       }
     }
+
+    // Use of `var` causes the start position of the variable type tree node to be < 0.
+    // Note that the .isImplicitlyTyped() method on JCVariableDecl returns the wrong answer after
+    // type attribution has occurred.
+    Tree varType = varTree.getType();
+    boolean isImplicitlyTyped = ((JCTree) varType).getStartPosition() < 0;
     if (needsList[0]) {
-      fix.replace((varTree).getType(), "List<String>").addImport("java.util.List");
+      if (!isImplicitlyTyped) {
+        fix.replace(varType, "List<String>").addImport("java.util.List");
+      }
       replaceWithSplitter(fix, tree, methodAndArgument, state, "splitToList", needsMutableList[0]);
     } else {
-      fix.replace((varTree).getType(), "Iterable<String>");
+      if (!isImplicitlyTyped) {
+        fix.replace(varType, "Iterable<String>");
+      }
       replaceWithSplitter(fix, tree, methodAndArgument, state, "split", needsMutableList[0]);
     }
     return Optional.of(fix.build());

diff --git a/core/src/test/java/com/google/errorprone/bugpatterns/StringSplitterTest.java b/core/src/test/java/com/google/errorprone/bugpatterns/StringSplitterTest.java
@@ -16,9 +16,12 @@
 
 package com.google.errorprone.bugpatterns;
 
+import static org.junit.Assume.assumeTrue;
+
 import com.google.errorprone.BugCheckerRefactoringTestHelper;
 import com.google.errorprone.BugCheckerRefactoringTestHelper.TestMode;
 import com.google.errorprone.CompilationTestHelper;
+import java.lang.reflect.Method;
 import org.junit.Ignore;
 import org.junit.Test;
 import org.junit.runner.RunWith;
@@ -51,6 +54,29 @@ public void positive() {
         .doTest(TestMode.TEXT_MATCH);
   }
 
+  // Regression test for issue #1124
+  @Test
+  public void positive_localVarTypeInference() {
+    assumeTrue(isJdk10OrGreater());
+    testHelper
+        .addInputLines(
+            "Test.java",
+            "class Test {",
+            "  void f() {",
+            "    var lines = \"\".split(\":\");",
+            "  }",
+            "}")
+        .addOutputLines(
+            "Test.java",
+            "import com.google.common.base.Splitter;",
+            "class Test {",
+            "  void f() {",
+            "    var lines = Splitter.on(':').split(\"\");",
+            "  }",
+            "}")
+        .doTest(TestMode.TEXT_MATCH);
+  }
+
   @Test
   public void positive_patternIsSymbol() {
     testHelper
@@ -412,4 +438,15 @@ public void noSplitterOnClassPath() {
         .setArgs("-cp", ":")
         .doTest(TestMode.TEXT_MATCH);
   }
+
+  private static boolean isJdk10OrGreater() {
+    try {
+      Method versionMethod = Runtime.class.getMethod("version");
+      Object version = versionMethod.invoke(null);
+      int majorVersion = (int) version.getClass().getMethod("major").invoke(version);
+      return majorVersion >= 10;
+    } catch (ReflectiveOperationException e) {
+      return false;
+    }
+  }
 }