From 4ae3297a857c6a0c84511b4c5e727360f6958c89 Mon Sep 17 00:00:00 2001 From: Tor Norbye Date: Tue, 29 May 2012 12:04:03 -0700 Subject: 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 --- .../src/com/android/ide/eclipse/adt/AdtUtils.java | 17 ++ .../adt/internal/lint/EclipseLintClient.java | 137 +++++++++++++++- .../android/tools/lint/client/api/LintClient.java | 172 ++++++++++++++++----- .../android/tools/lint/client/api/LintDriver.java | 43 +++++- .../tools/lint/detector/api/ClassContext.java | 30 ++++ .../tools/lint/detector/api/LintConstants.java | 11 ++ .../android/tools/lint/detector/api/LintUtils.java | 29 ++++ .../com/android/tools/lint/checks/ApiDetector.java | 5 +- .../android/tools/lint/checks/HandlerDetector.java | 50 ++---- .../tools/lint/checks/ViewConstructorDetector.java | 4 +- .../tools/lint/checks/HandlerDetectorTest.java | 4 +- 11 files changed, 414 insertions(+), 88 deletions(-) diff --git a/eclipse/plugins/com.android.ide.eclipse.adt/src/com/android/ide/eclipse/adt/AdtUtils.java b/eclipse/plugins/com.android.ide.eclipse.adt/src/com/android/ide/eclipse/adt/AdtUtils.java index 09092e7..47e650d 100644 --- a/eclipse/plugins/com.android.ide.eclipse.adt/src/com/android/ide/eclipse/adt/AdtUtils.java +++ b/eclipse/plugins/com.android.ide.eclipse.adt/src/com/android/ide/eclipse/adt/AdtUtils.java @@ -444,6 +444,23 @@ public class AdtUtils { } /** + * Converts a workspace-relative path to an absolute file path + * + * @param path the workspace-relative path to convert + * @return the corresponding absolute file in the file system + */ + @NonNull + public static File workspacePathToFile(@NonNull IPath path) { + IWorkspaceRoot root = ResourcesPlugin.getWorkspace().getRoot(); + IResource res = root.findMember(path); + if (res != null) { + return res.getLocation().toFile(); + } + + return path.toFile(); + } + + /** * Converts a {@link File} to an {@link IFile}, if possible. * * @param file a file to be converted diff --git a/eclipse/plugins/com.android.ide.eclipse.adt/src/com/android/ide/eclipse/adt/internal/lint/EclipseLintClient.java b/eclipse/plugins/com.android.ide.eclipse.adt/src/com/android/ide/eclipse/adt/internal/lint/EclipseLintClient.java index 703be80..b5810a4 100644 --- a/eclipse/plugins/com.android.ide.eclipse.adt/src/com/android/ide/eclipse/adt/internal/lint/EclipseLintClient.java +++ b/eclipse/plugins/com.android.ide.eclipse.adt/src/com/android/ide/eclipse/adt/internal/lint/EclipseLintClient.java @@ -15,8 +15,11 @@ */ package com.android.ide.eclipse.adt.internal.lint; +import static com.android.ide.eclipse.adt.AdtConstants.DOT_JAR; import static com.android.ide.eclipse.adt.AdtConstants.DOT_XML; import static com.android.ide.eclipse.adt.AdtConstants.MARKER_LINT; +import static com.android.ide.eclipse.adt.AdtUtils.workspacePathToFile; +import static com.android.sdklib.SdkConstants.FD_NATIVE_LIBS; import com.android.annotations.NonNull; import com.android.annotations.Nullable; @@ -45,6 +48,7 @@ import com.android.tools.lint.detector.api.Project; import com.android.tools.lint.detector.api.Severity; import com.android.tools.lint.detector.api.XmlContext; import com.android.util.Pair; +import com.google.common.collect.Maps; import org.eclipse.core.resources.IFile; import org.eclipse.core.resources.IMarker; @@ -52,7 +56,9 @@ import org.eclipse.core.resources.IProject; import org.eclipse.core.resources.IResource; import org.eclipse.core.runtime.CoreException; import org.eclipse.core.runtime.IStatus; +import org.eclipse.jdt.core.IClasspathEntry; import org.eclipse.jdt.core.IJavaProject; +import org.eclipse.jdt.core.JavaCore; import org.eclipse.jdt.core.compiler.CategorizedProblem; import org.eclipse.jdt.internal.compiler.CompilationResult; import org.eclipse.jdt.internal.compiler.DefaultErrorHandlingPolicies; @@ -85,6 +91,7 @@ import org.w3c.dom.Node; import java.io.File; import java.io.IOException; +import java.util.ArrayList; import java.util.Collection; import java.util.Collections; import java.util.List; @@ -233,19 +240,38 @@ public class EclipseLintClient extends LintClient implements IDomParser { return null; } + // Cache for {@link getProject} + private IProject mLastEclipseProject; + private Project mLastLintProject; + private IProject getProject(Project project) { + if (project == mLastLintProject) { + return mLastEclipseProject; + } + + mLastLintProject = project; + mLastEclipseProject = null; + if (mResources != null) { if (mResources.size() == 1) { - return mResources.get(0).getProject(); + IProject p = mResources.get(0).getProject(); + mLastEclipseProject = p; + return p; } + IProject last = null; for (IResource resource : mResources) { IProject p = resource.getProject(); - if (project.getDir().equals(AdtUtils.getAbsolutePath(p).toFile())) { - return p; + if (p != last) { + if (project.getDir().equals(AdtUtils.getAbsolutePath(p).toFile())) { + mLastEclipseProject = p; + return p; + } + last = p; } } } + return null; } @@ -669,6 +695,111 @@ public class EclipseLintClient extends LintClient implements IDomParser { return new LazyLocation(context.file, model.getStructuredDocument(), (IndexedRegion) node); } + private Map mProjectInfo; + + @Override + @NonNull + 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 sources = null; + List classes = null; + List libraries = null; + + IProject p = getProject(project); + if (p != null) { + try { + IJavaProject javaProject = BaseProjectHelper.getJavaProject(p); + + // Output path + File file = workspacePathToFile(javaProject.getOutputLocation()); + classes = Collections.singletonList(file); + + // Source path + IClasspathEntry[] entries = javaProject.getRawClasspath(); + sources = new ArrayList(entries.length); + libraries = new ArrayList(entries.length); + for (int i = 0; i < entries.length; i++) { + IClasspathEntry entry = entries[i]; + int kind = entry.getEntryKind(); + + if (kind == IClasspathEntry.CPE_VARIABLE) { + entry = JavaCore.getResolvedClasspathEntry(entry); + kind = entry.getEntryKind(); + } + + if (kind == IClasspathEntry.CPE_SOURCE) { + sources.add(workspacePathToFile(entry.getPath())); + } else if (kind == IClasspathEntry.CPE_LIBRARY) { + libraries.add(entry.getPath().toFile()); + } + // Note that we ignore IClasspathEntry.CPE_CONTAINER: + // Normal Android Eclipse projects supply both + // AdtConstants.CONTAINER_FRAMEWORK + // and + // AdtConstants.CONTAINER_LIBRARIES + // here. We ignore the framework classes for obvious reasons, + // but we also ignore the library container because lint will + // process the libraries differently. When Eclipse builds a + // project, it gets the .jar output of the library projects + // from this container, which means it doesn't have to process + // the library sources. Lint on the other hand wants to process + // the source code, so instead it actually looks at the + // project.properties file to find the libraries, and then it + // iterates over all the library projects in turn and analyzes + // those separately (but passing the main project for context, + // such that the including project's manifest declarations + // are used for data like minSdkVersion level). + // + // Note that this container will also contain *other* + // libraries (Java libraries, not library projects) that we + // *should* include. However, we can't distinguish these + // class path entries from the library project jars, + // so instead of looking at these, we simply listFiles() in + // the libs/ folder after processing the classpath info + } + + // Add in libraries + File libs = new File(project.getDir(), FD_NATIVE_LIBS); + if (libs.isDirectory()) { + File[] jars = libs.listFiles(); + if (jars != null) { + for (File jar : jars) { + if (AdtUtils.endsWith(jar.getPath(), DOT_JAR)) { + libraries.add(jar); + } + } + } + } + } catch (CoreException e) { + AdtPlugin.log(e, null); + } + } + + if (sources == null) { + sources = super.getClassPath(project).getSourceFolders(); + } + if (classes == null) { + classes = super.getClassPath(project).getClassFolders(); + } + if (libraries == null) { + libraries = super.getClassPath(project).getLibraries(); + } + + info = new ClassPathInfo(sources, classes, libraries); + mProjectInfo.put(project, info); + } + + return info; + } + /** * Returns the registry of issues to check from within Eclipse. * 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 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 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 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 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 mClassFolders; + private final List mSourceFolders; + private final List mLibraries; + + public ClassPathInfo( + @NonNull List sourceFolders, + @NonNull List classFolders, + @NonNull List libraries) { + mSourceFolders = sourceFolders; + mClassFolders = classFolders; + mLibraries = libraries; + } + + @NonNull + public List getSourceFolders() { + return mSourceFolders; + } + + @NonNull + public List getClassFolders() { + return mClassFolders; + } + + @NonNull + public List 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. + *

+ * 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 getEclipseClasspath(@NonNull Project project, @NonNull String attribute, - @NonNull String... fallbackPaths) { - List folders = new ArrayList(); - 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 sources = new ArrayList(2); + List classes = new ArrayList(1); + List libraries = new ArrayList(); + + 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 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 union( @Nullable List 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 *

@@ -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 = ""; //$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; + } } diff --git a/lint/libs/lint_checks/src/com/android/tools/lint/checks/ApiDetector.java b/lint/libs/lint_checks/src/com/android/tools/lint/checks/ApiDetector.java index 43bd7a8..4f6df4f 100644 --- a/lint/libs/lint_checks/src/com/android/tools/lint/checks/ApiDetector.java +++ b/lint/libs/lint_checks/src/com/android/tools/lint/checks/ApiDetector.java @@ -17,6 +17,7 @@ package com.android.tools.lint.checks; import static com.android.tools.lint.detector.api.LintConstants.ANDROID_RESOURCE_PREFIX; +import static com.android.tools.lint.detector.api.LintConstants.CONSTRUCTOR_NAME; import static com.android.tools.lint.detector.api.LintConstants.TARGET_API; import com.android.annotations.NonNull; @@ -205,7 +206,7 @@ public class ApiDetector extends ResourceXmlDetector implements Detector.ClassSc } // TODO: Consider other widgets outside of android.widget.* int api = mApiDatabase.getCallVersion("android/widget/" + tag, //$NON-NLS-1$ - "", //$NON-NLS-1$ + CONSTRUCTOR_NAME, // Not all views provided this constructor right away, for example, // LinearLayout added it in API 11 yet LinearLayout is much older: // "(Landroid/content/Context;Landroid/util/AttributeSet;I)V"); //$NON-NLS-1$ @@ -463,7 +464,7 @@ public class ApiDetector extends ResourceXmlDetector implements Detector.ClassSc // If looking for a constructor, the string we'll see in the source is not the // method name () but the class name - if (patternStart != null && patternStart.equals("") //$NON-NLS-1$ + if (patternStart != null && patternStart.equals(CONSTRUCTOR_NAME) && node instanceof MethodInsnNode) { String owner = ((MethodInsnNode) node).owner; patternStart = owner.substring(owner.lastIndexOf('/') + 1); diff --git a/lint/libs/lint_checks/src/com/android/tools/lint/checks/HandlerDetector.java b/lint/libs/lint_checks/src/com/android/tools/lint/checks/HandlerDetector.java index 03a611f..425edac 100644 --- a/lint/libs/lint_checks/src/com/android/tools/lint/checks/HandlerDetector.java +++ b/lint/libs/lint_checks/src/com/android/tools/lint/checks/HandlerDetector.java @@ -16,26 +16,21 @@ package com.android.tools.lint.checks; -import com.android.annotations.NonNull; -import com.android.tools.lint.client.api.LintDriver; import com.android.tools.lint.detector.api.Category; import com.android.tools.lint.detector.api.ClassContext; import com.android.tools.lint.detector.api.Context; import com.android.tools.lint.detector.api.Detector; import com.android.tools.lint.detector.api.Detector.ClassScanner; 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.Scope; import com.android.tools.lint.detector.api.Severity; import com.android.tools.lint.detector.api.Speed; -import org.objectweb.asm.Opcodes; import org.objectweb.asm.tree.ClassNode; -import org.objectweb.asm.tree.FieldNode; import java.io.File; -import java.util.EnumSet; -import java.util.List; /** * Checks that Handler implementations are top level classes or static. @@ -58,7 +53,7 @@ public class HandlerDetector extends Detector implements ClassScanner { 4, Severity.WARNING, HandlerDetector.class, - EnumSet.of(Scope.CLASS_FILE)); + Scope.CLASS_FILE_SCOPE); /** Constructs a new {@link HandlerDetector} */ public HandlerDetector() { @@ -82,40 +77,13 @@ public class HandlerDetector extends Detector implements ClassScanner { return; } - // 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. - - LintDriver driver = context.getDriver(); - String name = classNode.name; - while (name != null) { - if (name.equals("android/os/Handler")) { //$NON-NLS-1$ - if (isStaticInnerClass(classNode)) { - return; - } - - Location location = context.getLocation(classNode); - context.report(ISSUE, location, String.format( - "This Handler class should be static or leaks might occur (%1$s)", - ClassContext.createSignature(classNode.name, null, null)), - null); - return; - } - name = driver.getSuperClass(name); + if (context.getDriver().isSubclassOf(classNode, "android/os/Handler") //$NON-NLS-1$ + && !LintUtils.isStaticInnerClass(classNode)) { + Location location = context.getLocation(classNode); + context.report(ISSUE, location, String.format( + "This Handler class should be static or leaks might occur (%1$s)", + ClassContext.createSignature(classNode.name, null, null)), + null); } } - - private boolean isStaticInnerClass(@NonNull ClassNode classNode) { - @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; - } } diff --git a/lint/libs/lint_checks/src/com/android/tools/lint/checks/ViewConstructorDetector.java b/lint/libs/lint_checks/src/com/android/tools/lint/checks/ViewConstructorDetector.java index 6c3b42a..72aaa77 100644 --- a/lint/libs/lint_checks/src/com/android/tools/lint/checks/ViewConstructorDetector.java +++ b/lint/libs/lint_checks/src/com/android/tools/lint/checks/ViewConstructorDetector.java @@ -16,6 +16,8 @@ package com.android.tools.lint.checks; +import static com.android.tools.lint.detector.api.LintConstants.CONSTRUCTOR_NAME; + import com.android.tools.lint.detector.api.Category; import com.android.tools.lint.detector.api.ClassContext; import com.android.tools.lint.detector.api.Context; @@ -122,7 +124,7 @@ public class ViewConstructorDetector extends Detector implements Detector.ClassS List methods = classNode.methods; for (Object methodObject : methods) { MethodNode method = (MethodNode) methodObject; - if (method.name.equals("")) { //$NON-NLS-1$ + if (method.name.equals(CONSTRUCTOR_NAME)) { String desc = method.desc; if (desc.equals(SIG1) || desc.equals(SIG2) || desc.equals(SIG3)) { return; diff --git a/lint/libs/lint_checks/tests/src/com/android/tools/lint/checks/HandlerDetectorTest.java b/lint/libs/lint_checks/tests/src/com/android/tools/lint/checks/HandlerDetectorTest.java index 1631a23..979a7b0 100644 --- a/lint/libs/lint_checks/tests/src/com/android/tools/lint/checks/HandlerDetectorTest.java +++ b/lint/libs/lint_checks/tests/src/com/android/tools/lint/checks/HandlerDetectorTest.java @@ -27,8 +27,8 @@ public class HandlerDetectorTest extends AbstractCheckTest { public void testRegistered() throws Exception { assertEquals( - "HandlerTest$1.class: Warning: This Handler class should be static or leaks might occur (test.pkg.HandlerTest.1)\n" + - "HandlerTest$Inner.class: Warning: This Handler class should be static or leaks might occur (test.pkg.HandlerTest.Inner)", + "HandlerTest.java:14: Warning: This Handler class should be static or leaks might occur (test.pkg.HandlerTest.Inner)\n" + + "HandlerTest.java:20: Warning: This Handler class should be static or leaks might occur (test.pkg.HandlerTest.1)", lintProject( "bytecode/HandlerTest.java.txt=>src/test/pkg/HandlerTest.java", -- cgit v1.1