From de90651d740737517682497c0a4b422733c1148d Mon Sep 17 00:00:00 2001 From: eaftan Date: Mon, 1 Oct 2018 15:41:26 -0700 Subject: [PATCH] Correctly handle `var` in StringSplitter. Fixes #1129 RELNOTES: Correctly handle `var` in StringSplitter ------------- Created by MOE: https://github.com/google/moe MOE_MIGRATED_REVID=215293803 --- .../bugpatterns/StringSplitter.java | 14 ++++++- .../bugpatterns/StringSplitterTest.java | 37 +++++++++++++++++++ 2 files changed, 49 insertions(+), 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 index b15f4a33345..b1367bd70d6 100644 --- 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").addImport("java.util.List"); + if (!isImplicitlyTyped) { + fix.replace(varType, "List").addImport("java.util.List"); + } replaceWithSplitter(fix, tree, methodAndArgument, state, "splitToList", needsMutableList[0]); } else { - fix.replace((varTree).getType(), "Iterable"); + if (!isImplicitlyTyped) { + fix.replace(varType, "Iterable"); + } 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 index 622fd5a08d6..836318475ac 100644 --- 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; + } + } }