Lint infrastructure fixes

This changeset contains various unrelated fixes to the lint
infrastructure:

(1) Tweak the way the classpaths are computed in the default lint
    client method such that rather than reading and parsing the
    .classpath file 3 times, once for each of source-path, output-path
    and library-path, it's now processing it once and storing the
    results for all 3.

(2) Override the lookup-classpath method in Eclipse to directly query
    the Eclipse APIs for obtaining the classpath info.

(3) Add in user libraries found in libs/, since these don't
    necessarily show up in the .classpath file.

(4) Fix a couple of bugs related to checking .class files: First, when
    locating the project for a .class file, lint would search upwards
    for the surrounding project, which meant looking for the nearest
    parent containing an AndroidManifest.xml file.  However, in the
    case of .class files, it will first encounter the bin/ directory,
    which can contain a manifest file, so it would compute a project
    for the bin/ folder rather than its parent, which meant the source
    paths would be wrong.

    Second, the list of class entries to be processed by lint must be
    sorted prior to processing; the code dealing with innerclasses
    depends on that.

(5) Some minor code cleanup: Move some generic utility code and some
    string literals out of specific detectors and into the generic
    utility and constant classes.

(6) Cache results of the lint-project to eclipse-project lookup method
    since that method is called repeatedly with the same (current)
    project.

Change-Id: I33603eed8381ca54314202620cb1bb033e70f775

5 files changed, 246 insertions, 39 deletions

diff --git a/lint/libs/lint_api/src/com/android/tools/lint/client/api/LintClient.java b/lint/libs/lint_api/src/com/android/tools/lint/client/api/LintClient.java
index 8d6367b..27741b1 100644
--- a/lint/libs/lint_api/src/com/android/tools/lint/client/api/LintClient.java
+++ b/lint/libs/lint_api/src/com/android/tools/lint/client/api/LintClient.java
@@ -16,15 +16,23 @@
 
 package com.android.tools.lint.client.api;
 
+import static com.android.tools.lint.detector.api.LintConstants.CLASS_FOLDER;
+import static com.android.tools.lint.detector.api.LintConstants.DOT_JAR;
+import static com.android.tools.lint.detector.api.LintConstants.GEN_FOLDER;
+import static com.android.tools.lint.detector.api.LintConstants.LIBS_FOLDER;
+import static com.android.tools.lint.detector.api.LintConstants.SRC_FOLDER;
+
 import com.android.annotations.NonNull;
 import com.android.annotations.Nullable;
 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.LintUtils;
 import com.android.tools.lint.detector.api.Location;
 import com.android.tools.lint.detector.api.Project;
 import com.android.tools.lint.detector.api.Severity;
 import com.google.common.annotations.Beta;
+import com.google.common.collect.Maps;
 
 import org.w3c.dom.Document;
 import org.w3c.dom.Element;
@@ -53,7 +61,6 @@ import javax.xml.parsers.DocumentBuilderFactory;
  */
 @Beta
 public abstract class LintClient {
-
     private static final String PROP_BIN_DIR  = "com.android.tools.lint.bindir";  //$NON-NLS-1$
 
     /**
@@ -177,7 +184,7 @@ public abstract class LintClient {
      */
     @NonNull
     public List<File> getJavaSourceFolders(@NonNull Project project) {
-        return getEclipseClasspath(project, "src", "src", "gen"); //$NON-NLS-1$ //$NON-NLS-2$
+        return getClassPath(project).getSourceFolders();
     }
 
     /**
@@ -188,7 +195,8 @@ public abstract class LintClient {
      */
     @NonNull
     public List<File> getJavaClassFolders(@NonNull Project project) {
-        return getEclipseClasspath(project, "output", "bin"); //$NON-NLS-1$ //$NON-NLS-2$
+        return getClassPath(project).getClassFolders();
+
     }
 
     /**
@@ -199,7 +207,7 @@ public abstract class LintClient {
      */
     @NonNull
     public List<File> getJavaLibraries(@NonNull Project project) {
-        return getEclipseClasspath(project, "lib"); //$NON-NLS-1$
+        return getClassPath(project).getLibraries();
     }
 
     /**
@@ -286,53 +294,141 @@ public abstract class LintClient {
         }
     }
 
+    private Map<Project, ClassPathInfo> mProjectInfo;
+
     /**
-     * Considers the given directory as an Eclipse project and returns either
-     * its source or its output folders depending on the {@code attribute} parameter.
+     * Information about class paths (sources, class files and libraries)
+     * usually associated with a project.
+     */
+    protected static class ClassPathInfo {
+        private final List<File> mClassFolders;
+        private final List<File> mSourceFolders;
+        private final List<File> mLibraries;
+
+        public ClassPathInfo(
+                @NonNull List<File> sourceFolders,
+                @NonNull List<File> classFolders,
+                @NonNull List<File> libraries) {
+            mSourceFolders = sourceFolders;
+            mClassFolders = classFolders;
+            mLibraries = libraries;
+        }
+
+        @NonNull
+        public List<File> getSourceFolders() {
+            return mSourceFolders;
+        }
+
+        @NonNull
+        public List<File> getClassFolders() {
+            return mClassFolders;
+        }
+
+        @NonNull
+        public List<File> getLibraries() {
+            return mLibraries;
+        }
+    }
+
+    /**
+     * Considers the given project as an Eclipse project and returns class path
+     * information for the project - the source folder(s), the output folder and
+     * any libraries.
+     * <p>
+     * Callers will not cache calls to this method, so if it's expensive to compute
+     * the classpath info, this method should perform its own caching.
+     *
+     * @param project the project to look up class path info for
+     * @return a class path info object, never null
      */
     @NonNull
-    private List<File> getEclipseClasspath(@NonNull Project project, @NonNull String attribute,
-            @NonNull String... fallbackPaths) {
-        List<File> folders = new ArrayList<File>();
-        File projectDir = project.getDir();
-        File classpathFile = new File(projectDir, ".classpath"); //$NON-NLS-1$
-        if (classpathFile.exists()) {
-            String classpathXml = readFile(classpathFile);
-            DocumentBuilderFactory factory = DocumentBuilderFactory.newInstance();
-            InputSource is = new InputSource(new StringReader(classpathXml));
-            factory.setNamespaceAware(false);
-            factory.setValidating(false);
-            try {
-                DocumentBuilder builder = factory.newDocumentBuilder();
-                Document document = builder.parse(is);
-                NodeList tags = document.getElementsByTagName("classpathentry"); //$NON-NLS-1$
-                for (int i = 0, n = tags.getLength(); i < n; i++) {
-                    Element element = (Element) tags.item(i);
-                    String kind = element.getAttribute("kind"); //$NON-NLS-1$
-                    if (kind.equals(attribute)) {
-                        String path = element.getAttribute("path"); //$NON-NLS-1$
-                        File sourceFolder = new File(projectDir, path);
-                        if (sourceFolder.exists()) {
-                            folders.add(sourceFolder);
+    protected ClassPathInfo getClassPath(@NonNull Project project) {
+        ClassPathInfo info;
+        if (mProjectInfo == null) {
+            mProjectInfo = Maps.newHashMap();
+            info = null;
+        } else {
+            info = mProjectInfo.get(project);
+        }
+
+        if (info == null) {
+            List<File> sources = new ArrayList<File>(2);
+            List<File> classes = new ArrayList<File>(1);
+            List<File> libraries = new ArrayList<File>();
+
+            File projectDir = project.getDir();
+            File classpathFile = new File(projectDir, ".classpath"); //$NON-NLS-1$
+            if (classpathFile.exists()) {
+                String classpathXml = readFile(classpathFile);
+                DocumentBuilderFactory factory = DocumentBuilderFactory.newInstance();
+                InputSource is = new InputSource(new StringReader(classpathXml));
+                factory.setNamespaceAware(false);
+                factory.setValidating(false);
+                try {
+                    DocumentBuilder builder = factory.newDocumentBuilder();
+                    Document document = builder.parse(is);
+                    NodeList tags = document.getElementsByTagName("classpathentry"); //$NON-NLS-1$
+                    for (int i = 0, n = tags.getLength(); i < n; i++) {
+                        Element element = (Element) tags.item(i);
+                        String kind = element.getAttribute("kind"); //$NON-NLS-1$
+                        List<File> addTo = null;
+                        if (kind.equals("src")) {            //$NON-NLS-1$
+                            addTo = sources;
+                        } else if (kind.equals("output")) {  //$NON-NLS-1$
+                            addTo = classes;
+                        } else if (kind.equals("lib")) {     //$NON-NLS-1$
+                            addTo = libraries;
+                        }
+                        if (addTo != null) {
+                            String path = element.getAttribute("path"); //$NON-NLS-1$
+                            File folder = new File(projectDir, path);
+                            if (folder.exists()) {
+                                addTo.add(folder);
+                            }
                         }
                     }
+                } catch (Exception e) {
+                    log(null, null);
                 }
-            } catch (Exception e) {
-                log(null, null);
             }
-        }
 
-        // Fallback?
-        if (folders.size() == 0) {
-            for (String fallbackPath : fallbackPaths) {
-                File folder = new File(projectDir, fallbackPath);
+            // Add in libraries that aren't specified in the .classpath file
+            File libs = new File(project.getDir(), LIBS_FOLDER);
+            if (libs.isDirectory()) {
+                File[] jars = libs.listFiles();
+                if (jars != null) {
+                    for (File jar : jars) {
+                        if (LintUtils.endsWith(jar.getPath(), DOT_JAR)
+                                && !libraries.contains(jar)) {
+                            libraries.add(jar);
+                        }
+                    }
+                }
+            }
+
+            // Fallback, in case there is no Eclipse project metadata here
+            if (sources.size() == 0) {
+                File src = new File(projectDir, SRC_FOLDER);
+                if (src.exists()) {
+                    sources.add(src);
+                }
+                File gen = new File(projectDir, GEN_FOLDER);
+                if (gen.exists()) {
+                    sources.add(gen);
+                }
+            }
+            if (classes.size() == 0) {
+                File folder = new File(projectDir, CLASS_FOLDER);
                 if (folder.exists()) {
-                    folders.add(folder);
+                    classes.add(folder);
                 }
             }
+
+            info = new ClassPathInfo(sources, classes, libraries);
+            mProjectInfo.put(project, info);
         }
 
-        return folders;
+        return info;
     }
 
     /**
diff --git a/lint/libs/lint_api/src/com/android/tools/lint/client/api/LintDriver.java b/lint/libs/lint_api/src/com/android/tools/lint/client/api/LintDriver.java
index 0f985f0..f0bbae6 100644
--- a/lint/libs/lint_api/src/com/android/tools/lint/client/api/LintDriver.java
+++ b/lint/libs/lint_api/src/com/android/tools/lint/client/api/LintDriver.java
@@ -18,6 +18,7 @@ package com.android.tools.lint.client.api;
 
 import static com.android.tools.lint.detector.api.LintConstants.ANDROID_MANIFEST_XML;
 import static com.android.tools.lint.detector.api.LintConstants.ATTR_IGNORE;
+import static com.android.tools.lint.detector.api.LintConstants.BIN_FOLDER;
 import static com.android.tools.lint.detector.api.LintConstants.DOT_CLASS;
 import static com.android.tools.lint.detector.api.LintConstants.DOT_JAR;
 import static com.android.tools.lint.detector.api.LintConstants.DOT_JAVA;
@@ -638,7 +639,22 @@ public class LintDriver {
     }
 
     private boolean isProjectDir(@NonNull File dir) {
-        return new File(dir, ANDROID_MANIFEST_XML).exists();
+        boolean hasManifest = new File(dir, ANDROID_MANIFEST_XML).exists();
+        if (hasManifest) {
+            // Special case: the bin/ folder can also contain a copy of the
+            // manifest file, but this is *not* a project directory
+            if (dir.getName().equals(BIN_FOLDER)) {
+                // ...unless of course it just *happens* to be a project named bin, in
+                // which case we peek at its parent to see if this is the case
+                dir = dir.getParentFile();
+                if (dir != null && isProjectDir(dir)) {
+                    // Yes, it's a bin/ directory inside a real project: ignore this dir
+                    return false;
+                }
+            }
+        }
+
+        return hasManifest;
     }
 
     private void checkProject(@NonNull Project project) {
@@ -865,6 +881,30 @@ public class LintDriver {
         return mSuperClassMap.get(name);
     }
 
+    /**
+     * Returns true if the given class is a subclass of the given super class.
+     *
+     * @param classNode the class to check whether it is a subclass of the given
+     *            super class name
+     * @param superClassName the fully qualified super class name (in VM format,
+     *            e.g. java/lang/Integer, not java.lang.Integer.
+     * @return true if the given class is a subclass of the given super class
+     */
+    public boolean isSubclassOf(@NonNull ClassNode classNode, @NonNull String superClassName) {
+        if (superClassName.equals(classNode.superName)) {
+            return true;
+        }
+
+        String className = classNode.name;
+        while (className != null) {
+            if (className.equals(superClassName)) {
+                return true;
+            }
+            className = getSuperClass(className);
+        }
+
+        return false;
+    }
     @Nullable
     private static List<Detector> union(
             @Nullable List<Detector> list1,
@@ -977,6 +1017,7 @@ public class LintDriver {
             }
 
             if (entries.size() > 0) {
+                Collections.sort(entries);
                 // No superclass info available on individual lint runs
                 mSuperClassMap = Collections.emptyMap();
                 runClassDetectors(Scope.CLASS_FILE, entries, project, main);
diff --git a/lint/libs/lint_api/src/com/android/tools/lint/detector/api/ClassContext.java b/lint/libs/lint_api/src/com/android/tools/lint/detector/api/ClassContext.java
index 44d5392..7b3cffd 100644
--- a/lint/libs/lint_api/src/com/android/tools/lint/detector/api/ClassContext.java
+++ b/lint/libs/lint_api/src/com/android/tools/lint/detector/api/ClassContext.java
@@ -16,6 +16,7 @@
 
 package com.android.tools.lint.detector.api;
 
+import static com.android.tools.lint.detector.api.LintConstants.CONSTRUCTOR_NAME;
 import static com.android.tools.lint.detector.api.LintConstants.DOT_CLASS;
 import static com.android.tools.lint.detector.api.LintConstants.DOT_JAVA;
 
@@ -444,6 +445,35 @@ public class ClassContext extends Context {
         return null;
     }
 
+    /**
+     * Returns a location for the given {@link MethodNode}.
+     *
+     * @param methodNode the class in the current context
+     * @param classNode the class containing the method
+     * @return a location pointing to the class declaration, or as close to it
+     *         as possible
+     */
+    @NonNull
+    public Location getLocation(@NonNull MethodNode methodNode,
+            @NonNull ClassNode classNode) {
+        // Attempt to find a proper location for this class. This is tricky
+        // since classes do not have line number entries in the class file; we need
+        // to find a method, look up the corresponding line number then search
+        // around it for a suitable tag, such as the class name.
+        String pattern;
+        if (methodNode.name.equals(CONSTRUCTOR_NAME)) {
+            if (isAnonymousClass(classNode.name)) {
+                pattern = classNode.superName.substring(classNode.superName.lastIndexOf('/') + 1);
+            } else {
+                pattern = classNode.name.substring(classNode.name.lastIndexOf('$') + 1);
+            }
+        } else {
+            pattern = methodNode.name;
+        }
+
+        return getLocationForLine(findLineNumber(methodNode), pattern, null);
+    }
+
     private static boolean isAnonymousClass(@NonNull String fqcn) {
         int lastIndex = fqcn.lastIndexOf('$');
         if (lastIndex != -1 && lastIndex < fqcn.length() - 1) {
diff --git a/lint/libs/lint_api/src/com/android/tools/lint/detector/api/LintConstants.java b/lint/libs/lint_api/src/com/android/tools/lint/detector/api/LintConstants.java
index a2757cb..12dffc5 100644
--- a/lint/libs/lint_api/src/com/android/tools/lint/detector/api/LintConstants.java
+++ b/lint/libs/lint_api/src/com/android/tools/lint/detector/api/LintConstants.java
@@ -18,6 +18,8 @@ package com.android.tools.lint.detector.api;
 
 import com.google.common.annotations.Beta;
 
+import java.io.File;
+
 /**
  * Constants used by the various detectors, defined in one place
  * <p>
@@ -254,6 +256,12 @@ public class LintConstants {
     public static final String ANDROID_MANIFEST_XML = "AndroidManifest.xml"; //$NON-NLS-1$
     public static final String OLD_PROGUARD_FILE = "proguard.cfg";     //$NON-NLS-1$
     public static final String PROGUARD_FILE = "proguard-project.txt"; //$NON-NLS-1$
+    public static final String CLASS_FOLDER =
+            "bin" + File.separator + "classes";                        //$NON-NLS-1$ //$NON-NLS-2$
+    public static final String GEN_FOLDER = "gen";                     //$NON-NLS-1$
+    public static final String SRC_FOLDER = "src";                     //$NON-NLS-1$
+    public static final String LIBS_FOLDER = "libs";                   //$NON-NLS-1$
+    public static final String BIN_FOLDER = "bin";                     //$NON-NLS-1$
 
     public static final String RES_FOLDER = "res";                     //$NON-NLS-1$
     public static final String DOT_XML = ".xml";                       //$NON-NLS-1$
@@ -316,4 +324,7 @@ public class LintConstants {
     public static final String TARGET_API = "TargetApi";                 //$NON-NLS-1$
     public static final String FQCN_SUPPRESS_LINT = "android.annotation." + SUPPRESS_LINT; //$NON-NLS-1$
     public static final String FQCN_TARGET_API = "android.annotation." + TARGET_API; //$NON-NLS-1$
+
+    // Class Names
+    public static final String CONSTRUCTOR_NAME = "<init>";                          //$NON-NLS-1$
 }
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 eef6d48..154ba67 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
@@ -16,6 +16,7 @@
 
 package com.android.tools.lint.detector.api;
 
+import static com.android.tools.lint.detector.api.LintConstants.CONSTRUCTOR_NAME;
 import static com.android.tools.lint.detector.api.LintConstants.DOT_XML;
 import static com.android.tools.lint.detector.api.LintConstants.ID_RESOURCE_PREFIX;
 import static com.android.tools.lint.detector.api.LintConstants.NEW_ID_RESOURCE_PREFIX;
@@ -29,6 +30,10 @@ import com.android.util.PositionXmlParser;
 import com.google.common.annotations.Beta;
 import com.google.common.io.Files;
 
+import org.objectweb.asm.Opcodes;
+import org.objectweb.asm.tree.ClassNode;
+import org.objectweb.asm.tree.FieldNode;
+import org.objectweb.asm.tree.MethodNode;
 import org.w3c.dom.Element;
 import org.w3c.dom.Node;
 import org.w3c.dom.NodeList;
@@ -489,4 +494,28 @@ public class LintUtils {
         }
         return text;
     }
+
+    /**
+     * Returns true if the given class node represents a static inner class.
+     *
+     * @param classNode the inner class to be checked
+     * @return true if the class node represents an inner class that is static
+     */
+    public static boolean isStaticInnerClass(@NonNull ClassNode classNode) {
+        // Note: We can't just filter out static inner classes like this:
+        //     (classNode.access & Opcodes.ACC_STATIC) != 0
+        // because the static flag only appears on methods and fields in the class
+        // file. Instead, look for the synthetic this pointer.
+
+        @SuppressWarnings("rawtypes") // ASM API
+        List fieldList = classNode.fields;
+        for (Object f : fieldList) {
+            FieldNode field = (FieldNode) f;
+            if (field.name.startsWith("this$") && (field.access & Opcodes.ACC_SYNTHETIC) != 0) {
+                return false;
+            }
+        }
+
+        return true;
+    }
 }