Issue 34322: Fix handling of import statements

Fix the commit prefs lint check such that it correctly identifies
scenarios where the innerclass "Editor" is referenced without its
qualifying top level class, SharedPreferences.

Change-Id: I3a22738508b66ce0b3e836feff91f816b2c368e8

8 files changed, 210 insertions, 3 deletions

diff --git a/eclipse/dictionary.txt b/eclipse/dictionary.txt
index af3d835..c4d9089 100644
--- a/eclipse/dictionary.txt
+++ b/eclipse/dictionary.txt
@@ -341,6 +341,7 @@ wakelocks
 wallpaper
 webtools
 whilst
+wildcard
 workflow
 xdpi
 xhdpi
diff --git a/lint/libs/lint_api/src/com/android/tools/lint/detector/api/LintUtils.java b/lint/libs/lint_api/src/com/android/tools/lint/detector/api/LintUtils.java
index 514d91e..fe659cd 100644
--- a/lint/libs/lint_api/src/com/android/tools/lint/detector/api/LintUtils.java
+++ b/lint/libs/lint_api/src/com/android/tools/lint/detector/api/LintUtils.java
@@ -47,6 +47,8 @@ import java.io.UnsupportedEncodingException;
 import java.util.ArrayList;
 import java.util.List;
 
+import lombok.ast.ImportDeclaration;
+
 
 /**
  * Useful utility methods related to lint.
@@ -707,4 +709,47 @@ public class LintUtils {
 
          return locale;
      }
+
+    /**
+     * Returns true if the given class (specified by a fully qualified class
+     * name) name is imported in the given compilation unit either through a fully qualified
+     * import or by a wildcard import.
+     *
+     * @param compilationUnit the compilation unit
+     * @param fullyQualifiedName the fully qualified class name
+     * @return true if the given imported name refers to the given fully
+     *         qualified name
+     */
+    public static boolean isImported(
+            @NonNull lombok.ast.Node compilationUnit,
+            @NonNull String fullyQualifiedName) {
+        int dotIndex = fullyQualifiedName.lastIndexOf('.');
+        int dotLength = fullyQualifiedName.length() - dotIndex;
+
+        boolean imported = false;
+        for (lombok.ast.Node rootNode : compilationUnit.getChildren()) {
+            if (rootNode instanceof ImportDeclaration) {
+                ImportDeclaration importDeclaration = (ImportDeclaration) rootNode;
+                String fqn = importDeclaration.asFullyQualifiedName();
+                if (fqn.equals(fullyQualifiedName)) {
+                    return true;
+                } else if (fullyQualifiedName.regionMatches(dotIndex, fqn,
+                        fqn.length() - dotLength, dotLength)) {
+                    // This import is importing the class name using some other prefix, so there
+                    // fully qualified class name cannot be imported under that name
+                    return false;
+                } else if (importDeclaration.astStarImport()
+                        && fqn.regionMatches(0, fqn, 0, dotIndex + 1)) {
+                    imported = true;
+                    // but don't break -- keep searching in case there's a non-wildcard
+                    // import of the specific class name, e.g. if we're looking for
+                    // android.content.SharedPreferences.Editor, don't match on the following:
+                    //   import android.content.SharedPreferences.*;
+                    //   import foo.bar.Editor;
+                }
+            }
+        }
+
+        return imported;
+    }
 }
diff --git a/lint/libs/lint_checks/src/com/android/tools/lint/checks/SharedPrefsDetector.java b/lint/libs/lint_checks/src/com/android/tools/lint/checks/SharedPrefsDetector.java
index 685d9cf..5db301b 100644
--- a/lint/libs/lint_checks/src/com/android/tools/lint/checks/SharedPrefsDetector.java
+++ b/lint/libs/lint_checks/src/com/android/tools/lint/checks/SharedPrefsDetector.java
@@ -23,6 +23,7 @@ import com.android.tools.lint.detector.api.Context;
 import com.android.tools.lint.detector.api.Detector;
 import com.android.tools.lint.detector.api.Issue;
 import com.android.tools.lint.detector.api.JavaContext;
+import com.android.tools.lint.detector.api.LintUtils;
 import com.android.tools.lint.detector.api.Scope;
 import com.android.tools.lint.detector.api.Severity;
 
@@ -109,8 +110,12 @@ public class SharedPrefsDetector extends Detector implements Detector.JavaScanne
         }
         VariableDefinition definition = (VariableDefinition) node.getParent().getParent();
         String type = definition.astTypeReference().toString();
-        if (!type.endsWith("SharedPreferences.Editor")) { //$NON-NLS-1$
-            return;
+        if (!type.endsWith("SharedPreferences.Editor")) {                   //$NON-NLS-1$
+            if (!type.equals("Editor") ||                                   //$NON-NLS-1$
+                    !LintUtils.isImported(context.compilationUnit,
+                            "android.content.SharedPreferences.Editor")) {  //$NON-NLS-1$
+                return;
+            }
         }
 
         Node method = findSurroundingMethod(node.getParent());
diff --git a/lint/libs/lint_checks/tests/src/com/android/tools/lint/checks/SharedPrefsDetectorTest.java b/lint/libs/lint_checks/tests/src/com/android/tools/lint/checks/SharedPrefsDetectorTest.java
index b77f3bd..964232c 100644
--- a/lint/libs/lint_checks/tests/src/com/android/tools/lint/checks/SharedPrefsDetectorTest.java
+++ b/lint/libs/lint_checks/tests/src/com/android/tools/lint/checks/SharedPrefsDetectorTest.java
@@ -39,4 +39,40 @@ public class SharedPrefsDetectorTest  extends AbstractCheckTest {
             lintProject("src/test/pkg/SharedPrefsTest.java.txt=>" +
                     "src/test/pkg/SharedPrefsTest.java"));
     }
+
+    public void test2() throws Exception {
+        // Regression test 1 for http://code.google.com/p/android/issues/detail?id=34322
+        assertEquals(
+            "src/test/pkg/SharedPrefsTest2.java:13: Warning: SharedPreferences.edit() without a corresponding commit() or apply() call [CommitPrefEdits]\n" +
+            "        SharedPreferences.Editor editor = preferences.edit();\n" +
+            "                                          ~~~~~~~~~~~~~~~~~~\n" +
+            "src/test/pkg/SharedPrefsTest2.java:17: Warning: SharedPreferences.edit() without a corresponding commit() or apply() call [CommitPrefEdits]\n" +
+            "        Editor editor = preferences.edit();\n" +
+            "                        ~~~~~~~~~~~~~~~~~~\n" +
+            "0 errors, 2 warnings\n",
+
+            lintProject("src/test/pkg/SharedPrefsTest2.java.txt=>" +
+                    "src/test/pkg/SharedPrefsTest2.java"));
+    }
+
+    public void test3() throws Exception {
+        // Regression test 2 for http://code.google.com/p/android/issues/detail?id=34322
+        assertEquals(
+            "src/test/pkg/SharedPrefsTest3.java:13: Warning: SharedPreferences.edit() without a corresponding commit() or apply() call [CommitPrefEdits]\n" +
+            "        Editor editor = preferences.edit();\n" +
+            "                        ~~~~~~~~~~~~~~~~~~\n" +
+            "0 errors, 1 warnings\n",
+
+            lintProject("src/test/pkg/SharedPrefsTest3.java.txt=>" +
+                    "src/test/pkg/SharedPrefsTest3.java"));
+    }
+
+    public void test4() throws Exception {
+        // Regression test 3 for http://code.google.com/p/android/issues/detail?id=34322
+        assertEquals(
+            "No warnings.",
+
+            lintProject("src/test/pkg/SharedPrefsTest4.java.txt=>" +
+                    "src/test/pkg/SharedPrefsTest4.java"));
+    }
 }
diff --git a/lint/libs/lint_checks/tests/src/com/android/tools/lint/checks/data/src/test/pkg/SharedPrefsTest2.java.txt b/lint/libs/lint_checks/tests/src/com/android/tools/lint/checks/data/src/test/pkg/SharedPrefsTest2.java.txt
new file mode 100644
index 0000000..20c3a21
--- /dev/null
+++ b/lint/libs/lint_checks/tests/src/com/android/tools/lint/checks/data/src/test/pkg/SharedPrefsTest2.java.txt
@@ -0,0 +1,19 @@
+package test.pkg;
+
+import android.annotation.SuppressLint;
+import android.app.Activity;
+import android.content.SharedPreferences;
+import android.content.SharedPreferences.Editor;
+import android.os.Bundle;
+import android.preference.PreferenceManager;
+
+@SuppressWarnings("unused")
+public class SharedPrefsTest2 extends Activity {
+    public void test1(SharedPreferences preferences) {
+        SharedPreferences.Editor editor = preferences.edit();
+    }
+
+    public void test2(SharedPreferences preferences) {
+        Editor editor = preferences.edit();
+    }
+}
diff --git a/lint/libs/lint_checks/tests/src/com/android/tools/lint/checks/data/src/test/pkg/SharedPrefsTest3.java.txt b/lint/libs/lint_checks/tests/src/com/android/tools/lint/checks/data/src/test/pkg/SharedPrefsTest3.java.txt
new file mode 100644
index 0000000..e5baa00
--- /dev/null
+++ b/lint/libs/lint_checks/tests/src/com/android/tools/lint/checks/data/src/test/pkg/SharedPrefsTest3.java.txt
@@ -0,0 +1,15 @@
+package test.pkg;
+
+import android.annotation.SuppressLint;
+import android.app.Activity;
+import android.content.SharedPreferences;
+import android.content.SharedPreferences.*;
+import android.os.Bundle;
+import android.preference.PreferenceManager;
+
+@SuppressWarnings("unused")
+public class SharedPrefsTest3 extends Activity {
+    public void test(SharedPreferences preferences) {
+        Editor editor = preferences.edit();
+    }
+}
diff --git a/lint/libs/lint_checks/tests/src/com/android/tools/lint/checks/data/src/test/pkg/SharedPrefsTest4.java.txt b/lint/libs/lint_checks/tests/src/com/android/tools/lint/checks/data/src/test/pkg/SharedPrefsTest4.java.txt
new file mode 100644
index 0000000..28de959
--- /dev/null
+++ b/lint/libs/lint_checks/tests/src/com/android/tools/lint/checks/data/src/test/pkg/SharedPrefsTest4.java.txt
@@ -0,0 +1,15 @@
+package test.pkg;
+
+import android.annotation.SuppressLint;
+import android.app.Activity;
+import android.content.SharedPreferences;
+import foo.bar.Editor;
+import android.os.Bundle;
+import android.preference.PreferenceManager;
+
+@SuppressWarnings("unused")
+public class SharedPrefsTest4 extends Activity {
+    public void test(SharedPreferences preferences) {
+        Editor editor = preferences.edit();
+    }
+}
diff --git a/lint/libs/lint_checks/tests/src/com/android/tools/lint/detector/api/LintUtilsTest.java b/lint/libs/lint_checks/tests/src/com/android/tools/lint/detector/api/LintUtilsTest.java
index 0ce3815..7dfa260 100644
--- a/lint/libs/lint_checks/tests/src/com/android/tools/lint/detector/api/LintUtilsTest.java
+++ b/lint/libs/lint_checks/tests/src/com/android/tools/lint/detector/api/LintUtilsTest.java
@@ -17,9 +17,15 @@
 package com.android.tools.lint.detector.api;
 
 import static com.android.tools.lint.detector.api.LintUtils.getLocaleAndRegion;
+import static com.android.tools.lint.detector.api.LintUtils.isImported;
 import static com.android.tools.lint.detector.api.LintUtils.splitPath;
 
+import com.android.annotations.Nullable;
+import com.android.tools.lint.LombokParser;
 import com.android.tools.lint.Main;
+import com.android.tools.lint.checks.BuiltinIssueRegistry;
+import com.android.tools.lint.client.api.IJavaParser;
+import com.android.tools.lint.client.api.LintDriver;
 import com.google.common.collect.Iterables;
 
 import java.io.BufferedOutputStream;
@@ -29,6 +35,7 @@ import java.io.OutputStreamWriter;
 import java.util.Arrays;
 
 import junit.framework.TestCase;
+import lombok.ast.Node;
 
 @SuppressWarnings("javadoc")
 public class LintUtilsTest extends TestCase {
@@ -285,4 +292,68 @@ public class LintUtilsTest extends TestCase {
         assertEquals("zh-rCN", getLocaleAndRegion("values-zh-rCN-keyshidden"));
         assertEquals("ms", getLocaleAndRegion("values-ms-keyshidden"));
     }
-}
\ No newline at end of file
+
+    public void testIsImported() throws Exception {
+        assertFalse(isImported(getCompilationUnit(
+                "package foo.bar;\n" +
+                "class Foo {\n" +
+                "}\n"),
+                "android.app.Activity"));
+
+        assertTrue(isImported(getCompilationUnit(
+                "package foo.bar;\n" +
+                "import foo.bar.*;\n" +
+                "import android.app.Activity;\n" +
+                "import foo.bar.Baz;\n" +
+                "class Foo {\n" +
+                "}\n"),
+                "android.app.Activity"));
+
+        assertTrue(isImported(getCompilationUnit(
+                "package foo.bar;\n" +
+                "import android.app.Activity;\n" +
+                "class Foo {\n" +
+                "}\n"),
+                "android.app.Activity"));
+
+        assertTrue(isImported(getCompilationUnit(
+                "package foo.bar;\n" +
+                "import android.app.*;\n" +
+                "class Foo {\n" +
+                "}\n"),
+                "android.app.Activity"));
+
+        assertFalse(isImported(getCompilationUnit(
+                "package foo.bar;\n" +
+                "import android.app.*;\n" +
+                "import foo.bar.Activity;\n" +
+                "class Foo {\n" +
+                "}\n"),
+                "android.app.Activity"));
+    }
+
+    private Node getCompilationUnit(String javaSource) {
+        IJavaParser parser = new LombokParser();
+        TestContext context = new TestContext(javaSource, new File("test"));
+        Node compilationUnit = parser.parseJava(context);
+        assertNotNull(javaSource, compilationUnit);
+        return compilationUnit;
+    }
+
+    private class TestContext extends JavaContext {
+        private final String mJavaSource;
+        public TestContext(String javaSource, File file) {
+            super(new LintDriver(new BuiltinIssueRegistry(),
+                    new Main()), new Main().getProject(new File("dummy"), new File("dummy")),
+                    null, file);
+
+            mJavaSource = javaSource;
+        }
+
+        @Override
+        @Nullable
+        public String getContents() {
+            return mJavaSource;
+        }
+    }
+}