diff options
| author | Tor Norbye <tnorbye@google.com> | 2012-01-26 14:33:02 -0800 |
|---|---|---|
| committer | Android (Google) Code Review <android-gerrit@google.com> | 2012-01-26 14:33:02 -0800 |
| commit | 73926b1e6312781da56f29954b95e1791f4e1ee9 (patch) | |
| tree | c37a02da4ee6bbe62ef956831bfaf68b3e339a6a /lint | |
| parent | d2db24e92907f54a1e22f128e9e8a8d613516721 (diff) | |
| parent | 380bdf7838171c02e29853027354c58ab9376beb (diff) | |
| download | sdk-73926b1e6312781da56f29954b95e1791f4e1ee9.zip sdk-73926b1e6312781da56f29954b95e1791f4e1ee9.tar.gz sdk-73926b1e6312781da56f29954b95e1791f4e1ee9.tar.bz2 | |
Merge "Add support for multi-pass lint checks, and chained locations"
Diffstat (limited to 'lint')
20 files changed, 868 insertions, 234 deletions
diff --git a/lint/cli/src/com/android/tools/lint/HtmlReporter.java b/lint/cli/src/com/android/tools/lint/HtmlReporter.java index 43984f4..5abdec5 100644 --- a/lint/cli/src/com/android/tools/lint/HtmlReporter.java +++ b/lint/cli/src/com/android/tools/lint/HtmlReporter.java @@ -17,6 +17,7 @@ package com.android.tools.lint; import static com.android.tools.lint.detector.api.LintConstants.DOT_9PNG; +import static com.android.tools.lint.detector.api.LintConstants.DOT_JPG; import static com.android.tools.lint.detector.api.LintConstants.DOT_PNG; import static com.android.tools.lint.detector.api.LintUtils.endsWith; @@ -267,6 +268,7 @@ class HtmlReporter extends Reporter { if (warning.path != null) { url = writeLocation(warning.file, warning.path, warning.line); } + mWriter.write(':'); // Is the URL for a single image? If so, place it here near the top // of the error floating on the right. If there are multiple images, @@ -295,30 +297,51 @@ class HtmlReporter extends Reporter { if (warning.location != null && warning.location.getSecondary() != null) { mWriter.write("<ul>"); Location l = warning.location.getSecondary(); + boolean haveOtherLocations = false; while (l != null) { if (l.getMessage() != null && l.getMessage().length() > 0) { Position start = l.getStart(); int line = start != null ? start.getLine() : -1; - int offset = start != null ? start.getOffset() : -1; String path = mClient.getDisplayPath(warning.project, l.getFile()); writeLocation(l.getFile(), path, line); + mWriter.write(':'); mWriter.write("<span class=\"message\">"); //$NON-NLS-1$ appendEscapedText(l.getMessage()); mWriter.write("</span>"); //$NON-NLS-1$ mWriter.write("<br />"); //$NON-NLS-1$ - String s = mClient.readFile(l.getFile()); - if (s != null && s.length() > 0) { - mWriter.write("<pre class=\"errorlines\">\n"); //$NON-NLS-1$ - appendCodeBlock(s, line, offset); - mWriter.write("\n</pre>"); //$NON-NLS-1$ + String name = l.getFile().getName(); + if (!(endsWith(name, DOT_PNG) || endsWith(name, DOT_JPG))) { + String s = mClient.readFile(l.getFile()); + if (s != null && s.length() > 0) { + mWriter.write("<pre class=\"errorlines\">\n"); //$NON-NLS-1$ + int offset = start != null ? start.getOffset() : -1; + appendCodeBlock(s, line, offset); + mWriter.write("\n</pre>"); //$NON-NLS-1$ + } } + } else { + haveOtherLocations = true; } l = l.getSecondary(); } mWriter.write("</ul>"); - + if (haveOtherLocations) { + mWriter.write("Additional locations: "); + mWriter.write("<ul>\n"); //$NON-NLS-1$ + l = warning.location.getSecondary(); + while (l != null) { + Position start = l.getStart(); + int line = start != null ? start.getLine() : -1; + String path = mClient.getDisplayPath(warning.project, l.getFile()); + mWriter.write("<li> "); //$NON-NLS-1$ + writeLocation(l.getFile(), path, line); + mWriter.write("\n"); //$NON-NLS-1$ + l = l.getSecondary(); + } + mWriter.write("</ul>\n"); //$NON-NLS-1$ + } } // Place a block of images? @@ -406,10 +429,10 @@ class HtmlReporter extends Reporter { if (url != null) { mWriter.write("</a>"); //$NON-NLS-1$ } - mWriter.write(':'); if (line >= 0) { // 0-based line numbers, but display 1-based - mWriter.write(Integer.toString(line + 1) + ':'); + mWriter.write(':'); + mWriter.write(Integer.toString(line + 1)); } mWriter.write("</span>"); //$NON-NLS-1$ mWriter.write(' '); diff --git a/lint/cli/src/com/android/tools/lint/LintCliXmlParser.java b/lint/cli/src/com/android/tools/lint/LintCliXmlParser.java index 1c7b13a..9ee5ee8 100644 --- a/lint/cli/src/com/android/tools/lint/LintCliXmlParser.java +++ b/lint/cli/src/com/android/tools/lint/LintCliXmlParser.java @@ -145,6 +145,12 @@ public class LintCliXmlParser extends PositionXmlParser implements IDomParser { public void setEnd(com.android.util.PositionXmlParser.Position end) { mEnd = end; } + + @Override + public String toString() { + return "OffsetPosition [line=" + mLine + ", column=" + mColumn + ", offset=" + + mOffset + ", end=" + mEnd + "]"; + } } @Override diff --git a/lint/cli/src/com/android/tools/lint/Main.java b/lint/cli/src/com/android/tools/lint/Main.java index 493b092..d3e4c8d 100644 --- a/lint/cli/src/com/android/tools/lint/Main.java +++ b/lint/cli/src/com/android/tools/lint/Main.java @@ -65,7 +65,7 @@ import java.util.Set; * </ul> */ public class Main extends LintClient { - private static final int MAX_LINE_WIDTH = 78; + static final int MAX_LINE_WIDTH = 78; private static final String ARG_ENABLE = "--enable"; //$NON-NLS-1$ private static final String ARG_DISABLE = "--disable"; //$NON-NLS-1$ private static final String ARG_CHECK = "--check"; //$NON-NLS-1$ @@ -285,7 +285,7 @@ public class Main extends LintClient { System.exit(ERRNO_EXISTS); } try { - mReporter = new XmlReporter(output); + mReporter = new XmlReporter(this, output); } catch (IOException e) { log(e, null); System.exit(ERRNO_INVALIDARGS); @@ -912,12 +912,19 @@ public class Main extends LintClient { private class ProgressPrinter implements LintListener { @Override - public void update(EventType type, Context context) { + public void update(Lint lint, EventType type, Context context) { switch (type) { case SCANNING_PROJECT: - System.out.print(String.format( - "\nScanning %1$s: ", - context.getProject().getDir().getName())); + if (lint.getPhase() > 1) { + System.out.print(String.format( + "\nScanning %1$s (Phase %2$d): ", + context.getProject().getDir().getName(), + lint.getPhase())); + } else { + System.out.print(String.format( + "\nScanning %1$s: ", + context.getProject().getDir().getName())); + } break; case SCANNING_LIBRARY_PROJECT: System.out.print(String.format( @@ -927,6 +934,9 @@ public class Main extends LintClient { case SCANNING_FILE: System.out.print('.'); break; + case NEW_PHASE: + // Ignored for now: printing status as part of next project's status + break; case CANCELED: case COMPLETED: System.out.println(); diff --git a/lint/cli/src/com/android/tools/lint/TextReporter.java b/lint/cli/src/com/android/tools/lint/TextReporter.java index 8a71bbf..4b8f7c3 100644 --- a/lint/cli/src/com/android/tools/lint/TextReporter.java +++ b/lint/cli/src/com/android/tools/lint/TextReporter.java @@ -68,44 +68,76 @@ class TextReporter extends Reporter { output.append('\n'); if (warning.errorLine != null) { - output.append(warning.errorLine); + output.append(warning.errorLine.trim()); + output.append('\n'); } if (warning.location != null && warning.location.getSecondary() != null) { Location location = warning.location.getSecondary(); - int count = 0; while (location != null) { - if (location.getMessage() != null && location.getMessage().length() > 0) { + if (location.getMessage() != null + && location.getMessage().length() > 0) { + output.append(" "); //$NON-NLS-1$ String path = mClient.getDisplayPath(warning.project, location.getFile()); output.append(path); - output.append(':'); Position start = location.getStart(); if (start != null) { int line = start.getLine(); if (line >= 0) { - output.append(Integer.toString(line + 1)); output.append(':'); + output.append(Integer.toString(line + 1)); } } - output.append(' '); - output.append(location.getMessage()); + if (location.getMessage() != null + && location.getMessage().length() > 0) { + output.append(':'); + output.append(' '); + output.append(location.getMessage()); + } + output.append('\n'); - count++; - if (count == 5) { - output.append("..."); - output.append('\n'); + } + + location = location.getSecondary(); + } + + location = warning.location.getSecondary(); + StringBuilder sb = new StringBuilder(); + sb.append("Also affects: "); + int begin = sb.length(); + while (location != null) { + if (location.getMessage() == null + || location.getMessage().length() > 0) { + if (sb.length() > begin) { + sb.append(", "); + } + + String path = mClient.getDisplayPath(warning.project, + location.getFile()); + sb.append(path); + + Position start = location.getStart(); + if (start != null) { + int line = start.getLine(); + if (line >= 0) { + sb.append(':'); + sb.append(Integer.toString(line + 1)); + } } } location = location.getSecondary(); } + String wrapped = Main.wrap(sb.toString(), Main.MAX_LINE_WIDTH, " "); //$NON-NLS-1$ + output.append(wrapped); + } } - System.out.println(output.toString()); + System.out.print(output.toString()); System.out.println(String.format("%1$d errors, %2$d warnings", errorCount, warningCount)); diff --git a/lint/cli/src/com/android/tools/lint/XmlReporter.java b/lint/cli/src/com/android/tools/lint/XmlReporter.java index 447a335..4c92810 100644 --- a/lint/cli/src/com/android/tools/lint/XmlReporter.java +++ b/lint/cli/src/com/android/tools/lint/XmlReporter.java @@ -16,6 +16,7 @@ package com.android.tools.lint; +import com.android.tools.lint.detector.api.Location; import com.android.tools.lint.detector.api.Position; import com.google.common.base.Charsets; import com.google.common.io.Files; @@ -31,9 +32,11 @@ import java.util.List; */ class XmlReporter extends Reporter { private final File mOutput; + private final Main mClient; - XmlReporter(File output) throws IOException { + XmlReporter(Main client, File output) throws IOException { super(new BufferedWriter(Files.newWriter(output, Charsets.UTF_8))); + mClient = client; mOutput = output; } @@ -45,45 +48,66 @@ class XmlReporter extends Reporter { if (issues.size() > 0) { for (Warning warning : issues) { - mWriter.write("\n <issue"); - writeAttribute(mWriter, "id", warning.issue.getId()); //$NON-NLS-1$ - writeAttribute(mWriter, "severity", warning.severity.getDescription()); //$NON-NLS-1$ - writeAttribute(mWriter, "message", warning.message); //$NON-NLS-1$ + mWriter.write('\n'); + indent(mWriter, 1); + mWriter.write("<issue"); //$NON-NLS-1$ + writeAttribute(mWriter, 2, "id", warning.issue.getId()); //$NON-NLS-1$ + writeAttribute(mWriter, 2, "severity", warning.severity.getDescription()); //$NON-NLS-1$ + writeAttribute(mWriter, 2, "message", warning.message); //$NON-NLS-1$ + assert (warning.file != null) == (warning.location != null); + if (warning.file != null) { - writeAttribute(mWriter, "file", warning.path); //$NON-NLS-1$ - if (warning.location != null) { - Position start = warning.location.getStart(); + assert warning.location.getFile() == warning.file; + } + + Location location = warning.location; + if (location != null) { + mWriter.write(">\n"); //$NON-NLS-1$ + while (location != null) { + indent(mWriter, 2); + mWriter.write("<location"); //$NON-NLS-1$ + String path = mClient.getDisplayPath(warning.project, location.getFile()); + writeAttribute(mWriter, 3, "file", path); //$NON-NLS-1$ + Position start = location.getStart(); if (start != null) { int line = start.getLine(); int column = start.getColumn(); if (line >= 0) { // +1: Line numbers internally are 0-based, report should be // 1-based. - writeAttribute(mWriter, "line", //$NON-NLS-1$ + writeAttribute(mWriter, 3, "line", //$NON-NLS-1$ Integer.toString(line + 1)); if (column >= 0) { - writeAttribute(mWriter, "column", //$NON-NLS-1$ + writeAttribute(mWriter, 3, "column", //$NON-NLS-1$ Integer.toString(column + 1)); } } } + + mWriter.write("/>\n"); //$NON-NLS-1$ + location = location.getSecondary(); } + indent(mWriter, 1); + mWriter.write("</issue>\n"); //$NON-NLS-1$ + } else { + mWriter.write('\n'); + indent(mWriter, 1); + mWriter.write("/>\n"); //$NON-NLS-1$ } - mWriter.write("\n />\n"); } } - mWriter.write( - "\n</issues>\n"); //$NON-NLS-1$ + mWriter.write("\n</issues>\n"); //$NON-NLS-1$ mWriter.close(); String path = mOutput.getAbsolutePath(); System.out.println(String.format("Wrote HTML report to %1$s", path)); } - private static void writeAttribute(Writer writer, String name, String value) + private static void writeAttribute(Writer writer, int indent, String name, String value) throws IOException { - writer.write("\n "); //$NON-NLS-1$ + writer.write('\n'); + indent(writer, indent); writer.write(name); writer.write('='); writer.write('"'); @@ -109,4 +133,10 @@ class XmlReporter extends Reporter { } writer.write('"'); } + + private static void indent(Writer writer, int indent) throws IOException { + for (int level = 0; level < indent; level++) { + writer.write(" "); //$NON-NLS-1$ + } + } }
\ No newline at end of file diff --git a/lint/libs/lint_api/src/com/android/tools/lint/client/api/Lint.java b/lint/libs/lint_api/src/com/android/tools/lint/client/api/Lint.java index e3df54f..c6f9913 100644 --- a/lint/libs/lint_api/src/com/android/tools/lint/client/api/Lint.java +++ b/lint/libs/lint_api/src/com/android/tools/lint/client/api/Lint.java @@ -16,8 +16,11 @@ 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.DOT_CLASS; import static com.android.tools.lint.detector.api.LintConstants.DOT_JAVA; +import static com.android.tools.lint.detector.api.LintConstants.PROGUARD_CFG; +import static com.android.tools.lint.detector.api.LintConstants.RES_FOLDER; import com.android.annotations.NonNull; import com.android.annotations.Nullable; @@ -37,6 +40,8 @@ import com.android.tools.lint.detector.api.Scope; import com.android.tools.lint.detector.api.Severity; import com.android.tools.lint.detector.api.XmlContext; import com.google.common.annotations.Beta; +import com.google.common.collect.ArrayListMultimap; +import com.google.common.collect.Multimap; import com.google.common.io.Files; import org.objectweb.asm.ClassReader; @@ -64,9 +69,11 @@ import java.util.Set; */ @Beta public class Lint { - private static final String PROGUARD_CFG = "proguard.cfg"; //$NON-NLS-1$ - private static final String ANDROID_MANIFEST_XML = "AndroidManifest.xml"; //$NON-NLS-1$ - private static final String RES_FOLDER_NAME = "res"; //$NON-NLS-1$ + /** + * Max number of passes to run through the lint runner if requested by + * {@link #requestRepeat} + */ + private static final int MAX_PHASES = 3; private final LintClient mClient; private volatile boolean mCanceled; @@ -75,6 +82,9 @@ public class Lint { private List<? extends Detector> mApplicableDetectors; private Map<Scope, List<Detector>> mScopeDetectors; private List<LintListener> mListeners; + private int mPhase; + private List<Detector> mRepeatingDetectors; + private EnumSet<Scope> mRepeatScope; /** * Creates a new {@link Lint} @@ -93,6 +103,36 @@ public class Lint { } /** + * Returns the scope for the lint job + * + * @return the scope, never null + */ + @NonNull + public EnumSet<Scope> getScope() { + return mScope; + } + + /** + * Returns the lint client requesting the lint check + * + * @return the client, never null + */ + @NonNull + public LintClient getClient() { + return mClient; + } + + /** + * Returns the current phase number. The first pass is numbered 1. Only one pass + * will be performed, unless a {@link Detector} calls {@link #requestRepeat}. + * + * @return the current phase, usually 1 + */ + public int getPhase() { + return mPhase; + } + + /** * Analyze the given file (which can point to an Android project). Issues found * are reported to the associated {@link LintClient}. * @@ -126,8 +166,8 @@ public class Lint { mScope.add(Scope.RESOURCE_FILE); } else if (name.equals(PROGUARD_CFG)) { mScope.add(Scope.PROGUARD_FILE); - } else if (name.equals(RES_FOLDER_NAME) - || file.getParent().equals(RES_FOLDER_NAME)) { + } else if (name.equals(RES_FOLDER) + || file.getParent().equals(RES_FOLDER)) { mScope.add(Scope.ALL_RESOURCE_FILES); mScope.add(Scope.RESOURCE_FILE); } else if (name.endsWith(DOT_JAVA)) { @@ -147,18 +187,153 @@ public class Lint { fireEvent(EventType.STARTING, null); for (Project project : projects) { + mPhase = 1; + // The set of available detectors varies between projects computeDetectors(project); + if (mApplicableDetectors.size() == 0) { + // No detectors enabled in this project: skip it + continue; + } + checkProject(project); if (mCanceled) { break; } + + runExtraPhases(project); } fireEvent(mCanceled ? EventType.CANCELED : EventType.COMPLETED, null); } + private void runExtraPhases(Project project) { + // Did any detectors request another phase? + if (mRepeatingDetectors != null) { + // Yes. Iterate up to MAX_PHASES times. + + // During the extra phases, we might be narrowing the scope, and setting it in the + // scope field such that detectors asking about the available scope will get the + // correct result. However, we need to restore it to the original scope when this + // is done in case there are other projects that will be checked after this, since + // the repeated phases is done *per project*, not after all projects have been + // processed. + EnumSet<Scope> oldScope = mScope; + + do { + mPhase++; + fireEvent(EventType.NEW_PHASE, + new Context(this, project, null, project.getDir())); + + // Narrow the scope down to the set of scopes requested by + // the rules. + if (mRepeatScope == null) { + mRepeatScope = Scope.ALL; + } + mScope = Scope.intersect(mScope, mRepeatScope); + if (mScope.isEmpty()) { + break; + } + + // Compute the detectors to use for this pass. + // Unlike the normal computeDetectors(project) call, + // this is going to use the existing instances, and include + // those that apply for the configuration. + computeRepeatingDetectors(mRepeatingDetectors, project); + + if (mApplicableDetectors.size() == 0) { + // No detectors enabled in this project: skip it + continue; + } + + checkProject(project); + if (mCanceled) { + break; + } + } while (mPhase < MAX_PHASES && mRepeatingDetectors != null); + + mScope = oldScope; + } + } + + private void computeRepeatingDetectors(List<Detector> detectors, Project project) { + // Ensure that the current visitor is recomputed + mCurrentFolderType = null; + mCurrentVisitor = null; + + // Create map from detector class to issue such that we can + // compute applicable issues for each detector in the list of detectors + // to be repeated + List<Issue> issues = mRegistry.getIssues(); + Multimap<Class<? extends Detector>, Issue> issueMap = + ArrayListMultimap.create(issues.size(), 3); + for (Issue issue : issues) { + issueMap.put(issue.getDetectorClass(), issue); + } + + Map<Class<? extends Detector>, EnumSet<Scope>> detectorToScope = + new HashMap<Class<? extends Detector>, EnumSet<Scope>>(); + Map<Scope, List<Detector>> scopeToDetectors = + new HashMap<Scope, List<Detector>>(); + + List<Detector> detectorList = new ArrayList<Detector>(); + // Compute the list of detectors (narrowed down from mRepeatingDetectors), + // and simultaneously build up the detectorToScope map which tracks + // the scopes each detector is affected by (this is used to populate + // the mScopeDetectors map which is used during iteration). + Configuration configuration = project.getConfiguration(); + for (Detector detector : detectors) { + Class<? extends Detector> detectorClass = detector.getClass(); + Collection<Issue> detectorIssues = issueMap.get(detectorClass); + if (issues != null) { + boolean add = false; + for (Issue issue : detectorIssues) { + // The reason we have to check whether the detector is enabled + // is that this is a per-project property, so when running lint in multiple + // projects, a detector enabled only in a different project could have + // requested another phase, and we end up in this project checking whether + // the detector is enabled here. + if (!configuration.isEnabled(issue)) { + continue; + } + + add = true; // Include detector if any of its issues are enabled + + EnumSet<Scope> s = detectorToScope.get(detectorClass); + EnumSet<Scope> issueScope = issue.getScope(); + if (s == null) { + detectorToScope.put(detectorClass, issueScope); + } else if (!s.containsAll(issueScope)) { + EnumSet<Scope> union = EnumSet.copyOf(s); + union.addAll(issueScope); + detectorToScope.put(detectorClass, union); + } + } + + if (add) { + detectorList.add(detector); + EnumSet<Scope> union = detectorToScope.get(detector.getClass()); + for (Scope s : union) { + List<Detector> list = scopeToDetectors.get(s); + if (list == null) { + list = new ArrayList<Detector>(); + scopeToDetectors.put(s, list); + } + list.add(detector); + } + } + } + } + + mApplicableDetectors = detectorList; + mScopeDetectors = scopeToDetectors; + mRepeatingDetectors = null; + mRepeatScope = null; + + validateScopeList(); + } + private void computeDetectors(@NonNull Project project) { // Ensure that the current visitor is recomputed mCurrentFolderType = null; @@ -343,7 +518,7 @@ public class Lint { private void checkProject(@NonNull Project project) { File projectDir = project.getDir(); - Context projectContext = new Context(mClient, project, null, projectDir, mScope); + Context projectContext = new Context(this, project, null, projectDir); fireEvent(EventType.SCANNING_PROJECT, projectContext); for (Detector check : mApplicableDetectors) { @@ -359,7 +534,7 @@ public class Lint { if (!Scope.checkSingleFile(mScope)) { List<Project> libraries = project.getDirectLibraries(); for (Project library : libraries) { - Context libraryContext = new Context(mClient, library, project, projectDir, mScope); + Context libraryContext = new Context(this, library, project, projectDir); fireEvent(EventType.SCANNING_LIBRARY_PROJECT, libraryContext); for (Detector check : mApplicableDetectors) { @@ -406,7 +581,7 @@ public class Lint { // Look up manifest information (but not for library projects) File manifestFile = new File(project.getDir(), ANDROID_MANIFEST_XML); if (!project.isLibrary() && manifestFile.exists()) { - XmlContext context = new XmlContext(mClient, project, main, manifestFile, mScope); + XmlContext context = new XmlContext(this, project, main, manifestFile); IDomParser parser = mClient.getDomParser(); context.document = parser.parseXml(context); if (context.document != null) { @@ -441,7 +616,7 @@ public class Lint { checkIndividualResources(project, main, xmlDetectors, project.getSubset()); } else { - File res = new File(project.getDir(), RES_FOLDER_NAME); + File res = new File(project.getDir(), RES_FOLDER); if (res.exists() && xmlDetectors.size() > 0) { checkResFolder(project, main, res, xmlDetectors); } @@ -484,7 +659,7 @@ public class Lint { if (detectors != null) { File file = new File(project.getDir(), PROGUARD_CFG); if (file.exists()) { - Context context = new Context(mClient, project, main, file, mScope); + Context context = new Context(this, project, main, file); fireEvent(EventType.SCANNING_FILE, context); for (Detector detector : detectors) { if (detector.appliesTo(context, file)) { @@ -543,9 +718,8 @@ public class Lint { ClassReader reader = new ClassReader(bytes); ClassNode classNode = new ClassNode(); reader.accept(classNode, 0 /*flags*/); - ClassContext context = new ClassContext(mClient, project, main, file, - mScope, binDir, bytes, classNode); - + ClassContext context = new ClassContext(this, project, main, file, + binDir, bytes, classNode); for (Detector detector : checks) { if (detector.appliesTo(context, file)) { fireEvent(EventType.SCANNING_FILE, context); @@ -609,7 +783,7 @@ public class Lint { if (sources.size() > 0) { JavaVisitor visitor = new JavaVisitor(javaParser, checks); for (File file : sources) { - JavaContext context = new JavaContext(mClient, project, main, file, mScope); + JavaContext context = new JavaContext(this, project, main, file); fireEvent(EventType.SCANNING_FILE, context); visitor.visitFile(context, file); if (mCanceled) { @@ -709,8 +883,7 @@ public class Lint { if (visitor != null) { // if not, there are no applicable rules in this folder for (File file : xmlFiles) { if (LintUtils.isXmlFile(file)) { - XmlContext context = new XmlContext(mClient, project, main, - file, mScope); + XmlContext context = new XmlContext(this, project, main, file); fireEvent(EventType.SCANNING_FILE, context); visitor.visitFile(context, file); if (mCanceled) { @@ -732,10 +905,10 @@ public class Lint { if (file.isDirectory()) { // Is it a resource folder? ResourceFolderType type = ResourceFolderType.getFolderType(file.getName()); - if (type != null && new File(file.getParentFile(), RES_FOLDER_NAME).exists()) { + if (type != null && new File(file.getParentFile(), RES_FOLDER).exists()) { // Yes. checkResourceFolder(project, main, file, type, xmlDetectors); - } else if (file.getName().equals(RES_FOLDER_NAME)) { // Is it the res folder? + } else if (file.getName().equals(RES_FOLDER)) { // Is it the res folder? // Yes checkResFolder(project, main, file, xmlDetectors); } else { @@ -750,8 +923,7 @@ public class Lint { if (type != null) { XmlVisitor visitor = getVisitor(type, xmlDetectors); if (visitor != null) { - XmlContext context = new XmlContext(mClient, project, main, file, - mScope); + XmlContext context = new XmlContext(this, project, main, file); fireEvent(EventType.SCANNING_FILE, context); visitor.visitFile(context, file); } @@ -789,7 +961,7 @@ public class Lint { if (mListeners != null) { for (int i = 0, n = mListeners.size(); i < n; i++) { LintListener listener = mListeners.get(i); - listener.update(type, context); + listener.update(this, type, context); } } } @@ -802,7 +974,7 @@ public class Lint { * and lint clients don't have to make sure they check for ignored issues or * filtered out warnings. */ - private static class LintClientWrapper extends LintClient { + private class LintClientWrapper extends LintClient { @NonNull private final LintClient mDelegate; @@ -901,4 +1073,37 @@ public class Lint { return mDelegate.getJavaParser(); } } + + /** + * Requests another pass through the data for the given detector. This is + * typically done when a detector needs to do more expensive computation, + * but it only wants to do this once it <b>knows</b> that an error is + * present, or once it knows more specifically what to check for. + * + * @param detector the detector that should be included in the next pass. + * Note that the lint runner may refuse to run more than a couple + * of runs. + * @param scope the scope to be revisited. This must be a subset of the + * current scope ({@link #getScope()}, and it is just a performance hint; + * in particular, the detector should be prepared to be called on other + * scopes as well (since they may have been requested by other detectors). + * You can pall null to indicate "all". + */ + public void requestRepeat(@NonNull Detector detector, @Nullable EnumSet<Scope> scope) { + if (mRepeatingDetectors == null) { + mRepeatingDetectors = new ArrayList<Detector>(); + } + mRepeatingDetectors.add(detector); + + if (scope != null) { + if (mRepeatScope == null) { + mRepeatScope = scope; + } else { + mRepeatScope = EnumSet.copyOf(mRepeatScope); + mRepeatScope.addAll(scope); + } + } else { + mRepeatScope = Scope.ALL; + } + } } diff --git a/lint/libs/lint_api/src/com/android/tools/lint/client/api/LintListener.java b/lint/libs/lint_api/src/com/android/tools/lint/client/api/LintListener.java index 49c54a7..638045c 100644 --- a/lint/libs/lint_api/src/com/android/tools/lint/client/api/LintListener.java +++ b/lint/libs/lint_api/src/com/android/tools/lint/client/api/LintListener.java @@ -42,6 +42,9 @@ public interface LintListener { /** Lint is about to check the given file, see {@link Context#file} */ SCANNING_FILE, + /** A new pass was initiated */ + NEW_PHASE, + /** The lint check was canceled */ CANCELED, @@ -57,8 +60,9 @@ public interface LintListener { * {@link EventType#COMPLETED} events which are fired outside of project * contexts.) * + * @param driver the driver running through the checks * @param type the type of event that occurred * @param context the context providing additional information */ - public void update(@NonNull EventType type, @NonNull Context context); + public void update(@NonNull Lint driver, @NonNull EventType type, @NonNull Context context); } 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 09ef05c..3b2b5fd 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 @@ -21,13 +21,12 @@ import static com.android.tools.lint.detector.api.LintConstants.DOT_JAVA; import com.android.annotations.NonNull; import com.android.annotations.Nullable; -import com.android.tools.lint.client.api.LintClient; +import com.android.tools.lint.client.api.Lint; import com.google.common.annotations.Beta; import org.objectweb.asm.tree.ClassNode; import java.io.File; -import java.util.EnumSet; import java.util.List; /** @@ -53,28 +52,26 @@ public class ClassContext extends Context { /** * Construct a new {@link ClassContext} * - * @param client the client requesting a lint check + * @param driver the driver running through the checks * @param project the project containing the file being checked * @param main the main project if this project is a library project, or * null if this is not a library project. The main project is * the root project of all library projects, not necessarily the * directly including project. * @param file the file being checked - * @param scope the scope for the lint job * @param binDir the root binary directory containing this .class file. * @param bytes the bytecode raw data * @param classNode the bytecode object model */ public ClassContext( - @NonNull LintClient client, + @NonNull Lint driver, @NonNull Project project, @Nullable Project main, @NonNull File file, - @NonNull EnumSet<Scope> scope, @NonNull File binDir, @NonNull byte[] bytes, @NonNull ClassNode classNode) { - super(client, project, main, file, scope); + super(driver, project, main, file); mBinDir = binDir; mBytes = bytes; mClassNode = classNode; diff --git a/lint/libs/lint_api/src/com/android/tools/lint/detector/api/Context.java b/lint/libs/lint_api/src/com/android/tools/lint/detector/api/Context.java index 0c088d8..1d4e317 100644 --- a/lint/libs/lint_api/src/com/android/tools/lint/detector/api/Context.java +++ b/lint/libs/lint_api/src/com/android/tools/lint/detector/api/Context.java @@ -19,6 +19,7 @@ package com.android.tools.lint.detector.api; import com.android.annotations.NonNull; import com.android.annotations.Nullable; import com.android.tools.lint.client.api.Configuration; +import com.android.tools.lint.client.api.Lint; import com.android.tools.lint.client.api.LintClient; import com.android.tools.lint.client.api.SdkInfo; import com.google.common.annotations.Beta; @@ -46,8 +47,8 @@ public class Context { */ public final File file; - /** The client requesting a lint check */ - private final LintClient mClient; + /** The driver running through the checks */ + private final Lint mDriver; /** The project containing the file being checked */ @NonNull @@ -71,12 +72,6 @@ public class Context { /** The contents of the file */ private String mContents; - /** The scope of the current lint check */ - private final EnumSet<Scope> mScope; - - /** The SDK info, if any */ - private SdkInfo mSdkInfo; - /** * Whether the lint job has been canceled. * <p> @@ -92,27 +87,24 @@ public class Context { /** * Construct a new {@link Context} * - * @param client the client requesting a lint check + * @param driver the driver running through the checks * @param project the project containing the file being checked * @param main the main project if this project is a library project, or * null if this is not a library project. The main project is * the root project of all library projects, not necessarily the * directly including project. * @param file the file being checked - * @param scope the scope for the lint job */ public Context( - @NonNull LintClient client, + @NonNull Lint driver, @NonNull Project project, @Nullable Project main, - @NonNull File file, - @NonNull EnumSet<Scope> scope) { + @NonNull File file) { this.file = file; - mClient = client; + mDriver = driver; mProject = project; mMainProject = main; - mScope = scope; mConfiguration = project.getConfiguration(); } @@ -123,7 +115,7 @@ public class Context { */ @NonNull public EnumSet<Scope> getScope() { - return mScope; + return mDriver.getScope(); } /** @@ -165,7 +157,17 @@ public class Context { */ @NonNull public LintClient getClient() { - return mClient; + return mDriver.getClient(); + } + + /** + * Returns the driver running through the lint checks + * + * @return the driver + */ + @NonNull + public Lint getDriver() { + return mDriver; } /** @@ -179,7 +181,7 @@ public class Context { @Nullable public String getContents() { if (mContents == null) { - mContents = mClient.readFile(file); + mContents = mDriver.getClient().readFile(file); } return mContents; @@ -226,11 +228,7 @@ public class Context { */ @NonNull public SdkInfo getSdkInfo() { - if (mSdkInfo == null) { - mSdkInfo = mClient.getSdkInfo(mProject); - } - - return mSdkInfo; + return mProject.getSdkInfo(); } // ---- Convenience wrappers ---- (makes the detector code a bit leaner) @@ -259,7 +257,7 @@ public class Context { @Nullable Location location, @NonNull String message, @Nullable Object data) { - mClient.report(this, issue, location, message, data); + mDriver.getClient().report(this, issue, location, message, data); } /** @@ -273,7 +271,35 @@ public class Context { @Nullable Throwable exception, @Nullable String format, @Nullable Object... args) { - mClient.log(exception, format, args); + mDriver.getClient().log(exception, format, args); } + /** + * Returns the current phase number. The first pass is numbered 1. Only one pass + * will be performed, unless a {@link Detector} calls {@link #requestRepeat}. + * + * @return the current phase, usually 1 + */ + public int getPhase() { + return mDriver.getPhase(); + } + + /** + * Requests another pass through the data for the given detector. This is + * typically done when a detector needs to do more expensive computation, + * but it only wants to do this once it <b>knows</b> that an error is + * present, or once it knows more specifically what to check for. + * + * @param detector the detector that should be included in the next pass. + * Note that the lint runner may refuse to run more than a couple + * of runs. + * @param scope the scope to be revisited. This must be a subset of the + * current scope ({@link #getScope()}, and it is just a performance hint; + * in particular, the detector should be prepared to be called on other + * scopes as well (since they may have been requested by other detectors). + * You can pall null to indicate "all". + */ + public void requestRepeat(@NonNull Detector detector, @Nullable EnumSet<Scope> scope) { + mDriver.requestRepeat(detector, scope); + } } diff --git a/lint/libs/lint_api/src/com/android/tools/lint/detector/api/JavaContext.java b/lint/libs/lint_api/src/com/android/tools/lint/detector/api/JavaContext.java index b14b4ab..6aad8db 100644 --- a/lint/libs/lint_api/src/com/android/tools/lint/detector/api/JavaContext.java +++ b/lint/libs/lint_api/src/com/android/tools/lint/detector/api/JavaContext.java @@ -19,10 +19,9 @@ package com.android.tools.lint.detector.api; import com.android.annotations.NonNull; import com.android.annotations.Nullable; import com.android.tools.lint.client.api.IJavaParser; -import com.android.tools.lint.client.api.LintClient; +import com.android.tools.lint.client.api.Lint; import java.io.File; -import java.util.EnumSet; import lombok.ast.Node; @@ -43,22 +42,20 @@ public class JavaContext extends Context { * the given scope, in the given project reporting errors to the given * client. * - * @param client the client to report errors to + * @param driver the driver running through the checks * @param project the project to run lint on which contains the given file * @param main the main project if this project is a library project, or * null if this is not a library project. The main project is * the root project of all library projects, not necessarily the * directly including project. * @param file the file to be analyzed - * @param scope the scope used for analysis */ public JavaContext( - @NonNull LintClient client, + @NonNull Lint driver, @NonNull Project project, @Nullable Project main, - @NonNull File file, - @NonNull EnumSet<Scope> scope) { - super(client, project, main, file, scope); + @NonNull File file) { + super(driver, project, main, file); } /** 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 1d7f0ec..73dc853 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 @@ -205,6 +205,7 @@ public class LintConstants { // Filenames and folder names public static final String ANDROID_MANIFEST_XML = "AndroidManifest.xml"; //$NON-NLS-1$ + public static final String PROGUARD_CFG = "proguard.cfg"; //$NON-NLS-1$ public static final String RES_FOLDER = "res"; //$NON-NLS-1$ public static final String DOT_XML = ".xml"; //$NON-NLS-1$ public static final String DOT_GIF = ".gif"; //$NON-NLS-1$ @@ -249,5 +250,6 @@ public class LintConstants { public static final String R_PREFIX = "R."; //$NON-NLS-1$ public static final String R_ID_PREFIX = "R.id."; //$NON-NLS-1$ public static final String R_LAYOUT_PREFIX = "R.layout."; //$NON-NLS-1$ + public static final String R_DRAWABLE_PREFIX = "R.drawable."; //$NON-NLS-1$ public static final String R_ATTR_PREFIX = "R.attr."; //$NON-NLS-1$ } diff --git a/lint/libs/lint_api/src/com/android/tools/lint/detector/api/Location.java b/lint/libs/lint_api/src/com/android/tools/lint/detector/api/Location.java index 4cac0ff..506728c 100644 --- a/lint/libs/lint_api/src/com/android/tools/lint/detector/api/Location.java +++ b/lint/libs/lint_api/src/com/android/tools/lint/detector/api/Location.java @@ -143,6 +143,13 @@ public class Location { return mMessage; } + + @Override + public String toString() { + return "Location [file=" + mFile + ", start=" + mStart + ", end=" + mEnd + ", message=" + + mMessage + "]"; + } + /** * Creates a new location for the given file * @@ -292,6 +299,25 @@ public class Location { } /** + * Reverses the secondary location list initiated by the given location + * + * @param location the first location in the list + * @return the first location in the reversed list + */ + public static Location reverse(Location location) { + Location next = location.getSecondary(); + location.setSecondary(null); + while (next != null) { + Location nextNext = next.getSecondary(); + next.setSecondary(location); + location = next; + next = nextNext; + } + + return location; + } + + /** * A {@link Handle} is a reference to a location. The point of a location * handle is to be able to create them cheaply, and then resolve them into * actual locations later (if needed). This makes it possible to for example diff --git a/lint/libs/lint_api/src/com/android/tools/lint/detector/api/Project.java b/lint/libs/lint_api/src/com/android/tools/lint/detector/api/Project.java index 32ac071..b0d7186 100644 --- a/lint/libs/lint_api/src/com/android/tools/lint/detector/api/Project.java +++ b/lint/libs/lint_api/src/com/android/tools/lint/detector/api/Project.java @@ -30,6 +30,7 @@ import com.android.annotations.NonNull; import com.android.annotations.Nullable; import com.android.tools.lint.client.api.Configuration; import com.android.tools.lint.client.api.LintClient; +import com.android.tools.lint.client.api.SdkInfo; import com.google.common.annotations.Beta; import com.google.common.io.Closeables; @@ -65,6 +66,9 @@ public class Project { private int mTargetSdk = -1; private boolean mLibrary; + /** The SDK info, if any */ + private SdkInfo mSdkInfo; + /** * If non null, specifies a non-empty list of specific files under this * project which should be checked. @@ -459,4 +463,18 @@ public class Project { library.addLibraryProjects(collection); } } + + /** + * Gets the SDK info for the current project. + * + * @return the SDK info for the current project, never null + */ + @NonNull + public SdkInfo getSdkInfo() { + if (mSdkInfo == null) { + mSdkInfo = mClient.getSdkInfo(this); + } + + return mSdkInfo; + } } diff --git a/lint/libs/lint_api/src/com/android/tools/lint/detector/api/Scope.java b/lint/libs/lint_api/src/com/android/tools/lint/detector/api/Scope.java index 5141b16..9e5ed81 100644 --- a/lint/libs/lint_api/src/com/android/tools/lint/detector/api/Scope.java +++ b/lint/libs/lint_api/src/com/android/tools/lint/detector/api/Scope.java @@ -16,8 +16,8 @@ package com.android.tools.lint.detector.api; -import com.google.common.annotations.Beta; import com.android.annotations.NonNull; +import com.google.common.annotations.Beta; import java.util.EnumSet; @@ -97,6 +97,23 @@ public enum Scope { || scopes.contains(MANIFEST)); } + /** + * Returns the intersection of two scope sets + * + * @param scope1 the first set to intersect + * @param scope2 the second set to intersect + * @return the intersection of the two sets + */ + @NonNull + public static EnumSet<Scope> intersect( + @NonNull EnumSet<Scope> scope1, + @NonNull EnumSet<Scope> scope2) { + EnumSet<Scope> scope = EnumSet.copyOf(scope1); + scope.retainAll(scope2); + + return scope; + } + /** All scopes: running lint on a project will check these scopes */ public static final EnumSet<Scope> ALL = EnumSet.allOf(Scope.class); /** Scope-set used for detectors which are affected by a single resource file */ diff --git a/lint/libs/lint_api/src/com/android/tools/lint/detector/api/XmlContext.java b/lint/libs/lint_api/src/com/android/tools/lint/detector/api/XmlContext.java index cd2ee3a..940300d 100644 --- a/lint/libs/lint_api/src/com/android/tools/lint/detector/api/XmlContext.java +++ b/lint/libs/lint_api/src/com/android/tools/lint/detector/api/XmlContext.java @@ -19,14 +19,13 @@ package com.android.tools.lint.detector.api; import com.android.annotations.NonNull; import com.android.annotations.Nullable; import com.android.tools.lint.client.api.IDomParser; -import com.android.tools.lint.client.api.LintClient; +import com.android.tools.lint.client.api.Lint; import com.google.common.annotations.Beta; import org.w3c.dom.Document; import org.w3c.dom.Node; import java.io.File; -import java.util.EnumSet; /** * A {@link Context} used when checking XML files. @@ -44,22 +43,20 @@ public class XmlContext extends Context { /** * Construct a new {@link XmlContext} * - * @param client the client requesting a lint check + * @param driver the driver running through the checks * @param project the project containing the file being checked * @param main the main project if this project is a library project, or * null if this is not a library project. The main project is * the root project of all library projects, not necessarily the * directly including project. * @param file the file being checked - * @param scope the scope for the lint job */ public XmlContext( - @NonNull LintClient client, + @NonNull Lint driver, @NonNull Project project, @Nullable Project main, - @NonNull File file, - @NonNull EnumSet<Scope> scope) { - super(client, project, main, file, scope); + @NonNull File file) { + super(driver, project, main, file); } /** diff --git a/lint/libs/lint_checks/src/com/android/tools/lint/checks/IconDetector.java b/lint/libs/lint_checks/src/com/android/tools/lint/checks/IconDetector.java index 6118991..5b63580 100644 --- a/lint/libs/lint_checks/src/com/android/tools/lint/checks/IconDetector.java +++ b/lint/libs/lint_checks/src/com/android/tools/lint/checks/IconDetector.java @@ -925,7 +925,6 @@ public class IconDetector extends Detector implements Detector.XmlScanner { return result; } - return Collections.emptySet(); } diff --git a/lint/libs/lint_checks/src/com/android/tools/lint/checks/UnusedResourceDetector.java b/lint/libs/lint_checks/src/com/android/tools/lint/checks/UnusedResourceDetector.java index 0f378d3..73bd75d 100644 --- a/lint/libs/lint_checks/src/com/android/tools/lint/checks/UnusedResourceDetector.java +++ b/lint/libs/lint_checks/src/com/android/tools/lint/checks/UnusedResourceDetector.java @@ -19,12 +19,14 @@ package com.android.tools.lint.checks; import static com.android.tools.lint.detector.api.LintConstants.ANDROID_URI; import static com.android.tools.lint.detector.api.LintConstants.ATTR_NAME; import static com.android.tools.lint.detector.api.LintConstants.ATTR_REF_PREFIX; -import static com.android.tools.lint.detector.api.LintConstants.DOT_JAVA; +import static com.android.tools.lint.detector.api.LintConstants.DOT_GIF; +import static com.android.tools.lint.detector.api.LintConstants.DOT_JPG; import static com.android.tools.lint.detector.api.LintConstants.DOT_PNG; import static com.android.tools.lint.detector.api.LintConstants.DOT_XML; import static com.android.tools.lint.detector.api.LintConstants.RESOURCE_CLR_STYLEABLE; import static com.android.tools.lint.detector.api.LintConstants.RESOURCE_CLZ_ARRAY; import static com.android.tools.lint.detector.api.LintConstants.RESOURCE_CLZ_ID; +import static com.android.tools.lint.detector.api.LintConstants.RES_FOLDER; import static com.android.tools.lint.detector.api.LintConstants.R_ATTR_PREFIX; import static com.android.tools.lint.detector.api.LintConstants.R_ID_PREFIX; import static com.android.tools.lint.detector.api.LintConstants.R_PREFIX; @@ -33,9 +35,9 @@ import static com.android.tools.lint.detector.api.LintConstants.TAG_ITEM; import static com.android.tools.lint.detector.api.LintConstants.TAG_RESOURCES; import static com.android.tools.lint.detector.api.LintConstants.TAG_STRING_ARRAY; import static com.android.tools.lint.detector.api.LintConstants.TAG_STYLE; +import static com.android.tools.lint.detector.api.LintUtils.endsWith; import com.android.resources.ResourceType; -import com.android.tools.lint.client.api.IDomParser; import com.android.tools.lint.detector.api.Category; import com.android.tools.lint.detector.api.Context; import com.android.tools.lint.detector.api.Detector; @@ -43,7 +45,6 @@ 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.Location; -import com.android.tools.lint.detector.api.Location.Handle; import com.android.tools.lint.detector.api.ResourceXmlDetector; import com.android.tools.lint.detector.api.Scope; import com.android.tools.lint.detector.api.Severity; @@ -60,6 +61,7 @@ import java.util.ArrayList; import java.util.Arrays; import java.util.Collection; import java.util.Collections; +import java.util.Comparator; import java.util.EnumSet; import java.util.HashMap; import java.util.HashSet; @@ -118,12 +120,9 @@ public class UnusedResourceDetector extends ResourceXmlDetector implements Detec EnumSet.of(Scope.MANIFEST, Scope.ALL_RESOURCE_FILES, Scope.ALL_JAVA_FILES)) .setEnabledByDefault(false); - protected Set<String> mDeclarations; - protected Set<String> mReferences; - protected Map<String, Attr> mIdToAttr; - protected Map<Attr, Handle> mAttrToLocation; - protected Map<String, File> mDeclarationToFile; - protected Map<String, Handle> mDeclarationToHandle; + private Set<String> mDeclarations; + private Set<String> mReferences; + private Map<String, Location> mUnused; /** * Constructs a new {@link UnusedResourceDetector} @@ -138,17 +137,15 @@ public class UnusedResourceDetector extends ResourceXmlDetector implements Detec @Override public boolean appliesTo(Context context, File file) { - return LintUtils.isXmlFile(file) || LintUtils.endsWith(file.getName(), DOT_JAVA); + return true; } @Override public void beforeCheckProject(Context context) { - mIdToAttr = new HashMap<String, Attr>(300); - mAttrToLocation = new HashMap<Attr, Handle>(300); - mDeclarations = new HashSet<String>(300); - mReferences = new HashSet<String>(300); - mDeclarationToFile = new HashMap<String, File>(300); - mDeclarationToHandle = new HashMap<String, Handle>(300); + if (context.getPhase() == 1) { + mDeclarations = new HashSet<String>(300); + mReferences = new HashSet<String>(300); + } } // ---- Implements JavaScanner ---- @@ -156,8 +153,12 @@ public class UnusedResourceDetector extends ResourceXmlDetector implements Detec @Override public void beforeCheckFile(Context context) { File file = context.file; + String fileName = file.getName(); - if (LintUtils.endsWith(fileName, DOT_XML)) { + if (endsWith(fileName, DOT_XML) + || endsWith(fileName, DOT_PNG) + || endsWith(fileName, DOT_JPG) + || endsWith(fileName, DOT_GIF)) { String parentName = file.getParentFile().getName(); int dash = parentName.indexOf('-'); String typeName = parentName.substring(0, dash == -1 ? parentName.length() : dash); @@ -165,95 +166,140 @@ public class UnusedResourceDetector extends ResourceXmlDetector implements Detec if (type != null && LintUtils.isFileBasedResourceType(type)) { String baseName = fileName.substring(0, fileName.length() - DOT_XML.length()); String resource = R_PREFIX + typeName + '.' + baseName; - mDeclarations.add(resource); - mDeclarationToFile.put(resource, file); + if (context.getPhase() == 1) { + mDeclarations.add(resource); + } else { + assert context.getPhase() == 2; + if (mUnused.containsKey(resource)) { + recordLocation(resource, Location.create(file)); + } + } } } } @Override public void afterCheckProject(Context context) { - mDeclarations.removeAll(mReferences); - Set<String> unused = mDeclarations; - - // Remove styles: they may be used - List<String> styles = new ArrayList<String>(); - for (String resource : unused) { - // R.style.x, R.styleable.x - if (resource.startsWith("R.style")) { //$NON-NLS-1$ - styles.add(resource); - } - } - unused.removeAll(styles); - - // Remove id's if the user has disabled reporting issue ids - if (unused.size() > 0 && !context.isEnabled(ISSUE_IDS)) { - // Remove all R.id references - List<String> ids = new ArrayList<String>(); + if (context.getPhase() == 1) { + mDeclarations.removeAll(mReferences); + Set<String> unused = mDeclarations; + mReferences = null; + mDeclarations = null; + + // Remove styles: they may be used + List<String> styles = new ArrayList<String>(); for (String resource : unused) { - if (resource.startsWith(R_ID_PREFIX)) { - ids.add(resource); + // R.style.x, R.styleable.x + if (resource.startsWith("R.style")) { //$NON-NLS-1$ + styles.add(resource); } } - unused.removeAll(ids); - } - - List<String> sorted = new ArrayList<String>(); - for (String r : unused) { - sorted.add(r); - } - Collections.sort(sorted); - - for (String resource : sorted) { - String message = String.format("The resource %1$s appears to be unused", resource); - Location location = null; - Attr attr = mIdToAttr.get(resource); - if (attr != null) { - Handle handle = mAttrToLocation.get(attr); - if (handle != null) { - location = handle.resolve(); - } - } else { - // Try to figure out the file if it's a file based resource (such as R.layout) -- - // in that case we can figure out the filename since it has a simple mapping - // from the resource name (though the presence of qualifiers like -land etc - // makes it a little tricky if there's no base file provided) - int secondDot = resource.indexOf('.', 2); - String typeName = resource.substring(2, secondDot); // 2: Skip R. - ResourceType type = ResourceType.getEnum(typeName); - if (type != null && LintUtils.isFileBasedResourceType(type)) { - String name = resource.substring(secondDot + 1); - File file = new File(context.getProject().getDir(), - "res" + File.separator + typeName + File.separator + //$NON-NLS-1$ - name + DOT_XML); - if (file.exists()) { - location = Location.create(file); - } else if (type == ResourceType.DRAWABLE) { - file = new File(file.getParentFile(), file.getName().substring(0, - file.getName().length() - DOT_XML.length()) + DOT_PNG); - if (file.exists()) { - location = Location.create(file); - } + unused.removeAll(styles); + + // Remove id's if the user has disabled reporting issue ids + if (unused.size() > 0 && !context.isEnabled(ISSUE_IDS)) { + // Remove all R.id references + List<String> ids = new ArrayList<String>(); + for (String resource : unused) { + if (resource.startsWith(R_ID_PREFIX)) { + ids.add(resource); } } + unused.removeAll(ids); } - if (location == null) { - Handle handle = mDeclarationToHandle.get(resource); - if (handle != null) { - location = handle.resolve(); + + if (unused.size() > 0) { + mUnused = new HashMap<String, Location>(unused.size()); + for (String resource : unused) { + mUnused.put(resource, null); } + + // Request another pass, and in the second pass we'll gather location + // information for all declaration locations we've found + context.requestRepeat(this, Scope.ALL_RESOURCES_SCOPE); } - if (location == null) { - File file = mDeclarationToFile.get(resource); - if (file != null) { - location = Location.create(file); + } else { + assert context.getPhase() == 2; + + // Report any resources that we (for some reason) could not find a declaration + // location for + if (mUnused.size() > 0) { + // Fill in locations for files that we didn't encounter in other ways + for (Map.Entry<String, Location> entry : mUnused.entrySet()) { + String resource = entry.getKey(); + Location location = entry.getValue(); + if (location != null) { + continue; + } + + // Try to figure out the file if it's a file based resource (such as R.layout) -- + // in that case we can figure out the filename since it has a simple mapping + // from the resource name (though the presence of qualifiers like -land etc + // makes it a little tricky if there's no base file provided) + int secondDot = resource.indexOf('.', 2); + String typeName = resource.substring(2, secondDot); // 2: Skip R. + ResourceType type = ResourceType.getEnum(typeName); + if (type != null && LintUtils.isFileBasedResourceType(type)) { + String name = resource.substring(secondDot + 1); + + File res = new File(context.getProject().getDir(), RES_FOLDER); + File[] folders = res.listFiles(); + if (folders != null) { + // Process folders in alphabetical order such that we process + // based folders first: we want the locations in base folder + // order + Arrays.sort(folders, new Comparator<File>() { + @Override + public int compare(File file1, File file2) { + return file1.getName().compareTo(file2.getName()); + } + }); + for (File folder : folders) { + if (folder.getName().startsWith(typeName)) { + File[] files = folder.listFiles(); + if (files != null) { + for (File file : files) { + String fileName = file.getName(); + if (fileName.startsWith(name) + && fileName.startsWith(".", //$NON-NLS-1$ + name.length())) { + recordLocation(resource, Location.create(file)); + } + } + } + } + } + } + } + } + + List<String> sorted = new ArrayList<String>(mUnused.keySet()); + Collections.sort(sorted); + + for (String resource : sorted) { + Location location = mUnused.get(resource); + if (location != null) { + // We were prepending locations, but we want to prefer the base + // folders + location = Location.reverse(location); + } + String message = String.format("The resource %1$s appears to be unused", + resource); + Issue issue = resource.startsWith(R_ID_PREFIX) ? ISSUE_IDS : ISSUE; + context.report(issue, location, message, resource); } } - Issue issue = resource.startsWith(R_ID_PREFIX) ? ISSUE_IDS : ISSUE; - context.report(issue, location, message, resource); } } + private void recordLocation(String resource, Location location) { + Location oldLocation = mUnused.get(resource); + if (oldLocation != null) { + location.setSecondary(oldLocation); + } + mUnused.put(resource, location); + } + @Override public Collection<String> getApplicableAttributes() { return ALL; @@ -288,15 +334,19 @@ public class UnusedResourceDetector extends ResourceXmlDetector implements Detec type = RESOURCE_CLZ_ARRAY; } String resource = R_PREFIX + type + '.' + name; - mDeclarations.add(resource); - mDeclarationToFile.put(resource, context.file); - Handle handle = context.parser.createLocationHandle(context, item); - mDeclarationToHandle.put(resource, handle); - checkChildRefs(item); + if (context.getPhase() == 1) { + mDeclarations.add(resource); + checkChildRefs(item); + } else { + assert context.getPhase() == 2; + if (mUnused.containsKey(resource)) { + recordLocation(resource, context.getLocation(item)); + } + } } } - } else { + } else if (mReferences != null) { assert TAG_STYLE.equals(element.getTagName()) || TAG_ARRAY.equals(element.getTagName()) || TAG_STRING_ARRAY.equals(element.getTagName()); @@ -335,27 +385,30 @@ public class UnusedResourceDetector extends ResourceXmlDetector implements Detec @Override public void visitAttribute(XmlContext context, Attr attribute) { String value = attribute.getValue(); + if (value.startsWith("@+") && !value.startsWith("@+android")) { //$NON-NLS-1$ //$NON-NLS-2$ - String r = R_PREFIX + value.substring(2).replace('/', '.'); + String resource = R_PREFIX + value.substring(2).replace('/', '.'); // We already have the declarations when we scan the R file, but we're tracking // these here to get attributes for position info - mDeclarations.add(r); - mIdToAttr.put(r, attribute); - // It's important for this to be lightweight since we're storing ALL attribute - // locations even if we don't know that we're going to have any unused resources! - IDomParser parser = context.parser; - mAttrToLocation.put(attribute, parser.createLocationHandle(context, attribute)); - } else if (value.startsWith("@") //$NON-NLS-1$ - && !value.startsWith("@android:")) { //$NON-NLS-1$ - // Compute R-string, e.g. @string/foo => R.string.foo - String r = R_PREFIX + value.substring(1).replace('/', '.'); - mReferences.add(r); - } else if (value.startsWith(ATTR_REF_PREFIX)) { - mReferences.add(R_ATTR_PREFIX + value.substring(ATTR_REF_PREFIX.length())); + + if (context.getPhase() == 1) { + mDeclarations.add(resource); + } else if (mUnused.containsKey(resource)) { + recordLocation(resource, context.getLocation(attribute)); + } + } else if (mReferences != null) { + if (value.startsWith("@") //$NON-NLS-1$ + && !value.startsWith("@android:")) { //$NON-NLS-1$ + // Compute R-string, e.g. @string/foo => R.string.foo + String r = R_PREFIX + value.substring(1).replace('/', '.'); + mReferences.add(r); + } else if (value.startsWith(ATTR_REF_PREFIX)) { + mReferences.add(R_ATTR_PREFIX + value.substring(ATTR_REF_PREFIX.length())); + } } if (attribute.getNamespaceURI() != null - && !ANDROID_URI.equals(attribute.getNamespaceURI())) { + && !ANDROID_URI.equals(attribute.getNamespaceURI()) && mReferences != null) { mReferences.add(R_ATTR_PREFIX + attribute.getLocalName()); } } @@ -378,13 +431,20 @@ public class UnusedResourceDetector extends ResourceXmlDetector implements Detec @Override public void visitResourceReference(JavaContext context, AstVisitor visitor, VariableReference node, String type, String name) { - String reference = R_PREFIX + type + '.' + name; - mReferences.add(reference); + if (mReferences != null) { + String reference = R_PREFIX + type + '.' + name; + mReferences.add(reference); + } } @Override public AstVisitor createJavaVisitor(JavaContext context) { - return new UnusedResourceVisitor(); + if (mReferences != null) { + return new UnusedResourceVisitor(); + } else { + // Second pass, computing resource declaration locations: No need to look at Java + return null; + } } // Look for references and declarations @@ -432,8 +492,8 @@ public class UnusedResourceDetector extends ResourceXmlDetector implements Detec (VariableDefinition) child; String name = def.astVariables().first() .astName().astValue(); - String resource = R_PREFIX + className + '.' - + name; + String resource = R_PREFIX + className + + '.' + name; mDeclarations.add(resource); } // Else: It could be a comment node } diff --git a/lint/libs/lint_checks/tests/src/com/android/tools/lint/LintCliXmlParserTest.java b/lint/libs/lint_checks/tests/src/com/android/tools/lint/LintCliXmlParserTest.java index 7dca49e..ddc643f 100644 --- a/lint/libs/lint_checks/tests/src/com/android/tools/lint/LintCliXmlParserTest.java +++ b/lint/libs/lint_checks/tests/src/com/android/tools/lint/LintCliXmlParserTest.java @@ -16,6 +16,8 @@ package com.android.tools.lint; +import com.android.tools.lint.checks.BuiltinIssueRegistry; +import com.android.tools.lint.client.api.Lint; import com.android.tools.lint.client.api.LintClient; import com.android.tools.lint.detector.api.Context; import com.android.tools.lint.detector.api.Issue; @@ -23,7 +25,6 @@ import com.android.tools.lint.detector.api.Location; import com.android.tools.lint.detector.api.Location.Handle; import com.android.tools.lint.detector.api.Position; import com.android.tools.lint.detector.api.Project; -import com.android.tools.lint.detector.api.Scope; import com.android.tools.lint.detector.api.XmlContext; import org.w3c.dom.Attr; @@ -35,7 +36,6 @@ import java.io.BufferedWriter; import java.io.File; import java.io.FileWriter; import java.io.Writer; -import java.util.EnumSet; import junit.framework.TestCase; @@ -68,9 +68,9 @@ public class LintCliXmlParserTest extends TestCase { fw.write(xml); fw.close(); LintClient client = new TestClient(); + Lint driver = new Lint(new BuiltinIssueRegistry(), client); Project project = Project.create(client, file.getParentFile(), file.getParentFile()); - XmlContext context = new XmlContext(client, project, null, file, - EnumSet.of(Scope.RESOURCE_FILE)); + XmlContext context = new XmlContext(driver, project, null, file); Document document = parser.parseXml(context); assertNotNull(document); @@ -142,9 +142,9 @@ public class LintCliXmlParserTest extends TestCase { fw.write(xml); fw.close(); LintClient client = new TestClient(); + Lint driver = new Lint(new BuiltinIssueRegistry(), client); Project project = Project.create(client, file.getParentFile(), file.getParentFile()); - XmlContext context = new XmlContext(client, project, null, file, - EnumSet.of(Scope.RESOURCE_FILE)); + XmlContext context = new XmlContext(driver, project, null, file); Document document = parser.parseXml(context); assertNotNull(document); diff --git a/lint/libs/lint_checks/tests/src/com/android/tools/lint/detector/api/LocationTest.java b/lint/libs/lint_checks/tests/src/com/android/tools/lint/detector/api/LocationTest.java new file mode 100644 index 0000000..b9e1acd --- /dev/null +++ b/lint/libs/lint_checks/tests/src/com/android/tools/lint/detector/api/LocationTest.java @@ -0,0 +1,129 @@ +/* + * Copyright (C) 2012 The Android Open Source Project + * + * Licensed under the Eclipse Public License, Version 1.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.eclipse.org/org/documents/epl-v10.php + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package com.android.tools.lint.detector.api; + +import java.io.File; +import java.io.IOException; + +import junit.framework.TestCase; + +@SuppressWarnings("javadoc") +public class LocationTest extends TestCase { + public void testReverse() throws IOException { + File file1 = new File("parent/location1"); + File file2 = new File("parent/location2"); + File file3 = new File("parent/location3"); + File file4 = new File("parent/location4"); + + Location location1 = Location.create(file1); + Location location2 = Location.create(file2); + Location location3 = Location.create(file3); + Location location4 = Location.create(file4); + + // 1-element location list + assertSame(location1, Location.reverse(location1)); + assertFalse(containsCycle(location1)); + + // 2-element location list + location1.setSecondary(location2); + assertSame(location2, Location.reverse(location1)); + assertFalse(containsCycle(location2)); + assertSame(location1, location2.getSecondary()); + + // 3-element location list + location1.setSecondary(location2); + location2.setSecondary(location3); + assertSame(location3, Location.reverse(location1)); + assertFalse(containsCycle(location3)); + assertSame(location2, location3.getSecondary()); + assertSame(location1, location2.getSecondary()); + + // 4-element location list + location1.setSecondary(location2); + location2.setSecondary(location3); + location3.setSecondary(location4); + assertSame(location4, Location.reverse(location1)); + assertFalse(containsCycle(location4)); + assertSame(location3, location4.getSecondary()); + assertSame(location2, location3.getSecondary()); + assertSame(location1, location2.getSecondary()); + } + + public void testFaen() throws Exception { + File[] paths = new File[] { + new File("values-zh-rTW/arrays.xml"), new File("values-zh-rCN/arrays.xml"), + new File("values-vi/arrays.xml"), new File("values-uk/arrays.xml"), + new File("values-tr/arrays.xml"), new File("values-tl/arrays.xml"), + new File("values-th/arrays.xml"), new File("values-sv/arrays.xml"), + new File("values-sr/arrays.xml"), new File("values-sl/arrays.xml"), + new File("values-sk/arrays.xml"), new File("values-ru/arrays.xml"), + new File("values-ro/arrays.xml"), new File("values-rm/arrays.xml"), + new File("values-pt-rPT/arrays.xml"), new File("values-pt/arrays.xml"), + new File("values-pl/arrays.xml"), new File("values-nl/arrays.xml"), + new File("values-nb/arrays.xml"), new File("values-lv/arrays.xml"), + new File("values-lt/arrays.xml"), new File("values-ko/arrays.xml"), + new File("values-ja/arrays.xml"), new File("values-iw/arrays.xml"), + new File("values-it/arrays.xml"), new File("values-in/arrays.xml"), + new File("values-hu/arrays.xml"), new File("values-hr/arrays.xml"), + new File("values-fr/arrays.xml"), new File("values-fi/arrays.xml"), + new File("values-fa/arrays.xml"), new File("values-es-rUS/arrays.xml"), + new File("values-es/arrays.xml"), new File("values-en-rGB/arrays.xml"), + new File("values-el/arrays.xml"), new File("values-de/arrays.xml"), + new File("values-da/arrays.xml"), new File("values-cs/arrays.xml"), + new File("values-ca/arrays.xml"), new File("values-bg/arrays.xml"), + new File("values-ar/arrays.xml"), new File("values/arrays.xml") + }; + + Location last = null; + for (int i = paths.length - 1; i >= 0; i--) { + Location location = Location.create(paths[i]); + location.setSecondary(last); + last = location; + } + + assertFalse(containsCycle(last)); + Location.reverse(last); + assertFalse(containsCycle(last)); + } + + private static boolean containsCycle(Location location) { + // Make sure there's no cycle: iterate + Location a = location; + Location b = location; + + while (true) { + b = b.getSecondary(); + if (b == null) { + // OK! Found list end + return false; + } + if (b == a) { + return true; + } + b = b.getSecondary(); + if (b == null) { + // OK! Found list end + return false; + } + if (b == a) { + return true; + } + + a = a.getSecondary(); + assert a != null; + } + } +} diff --git a/lint/libs/lint_checks/tests/src/com/android/tools/lint/detector/api/ScopeTest.java b/lint/libs/lint_checks/tests/src/com/android/tools/lint/detector/api/ScopeTest.java new file mode 100644 index 0000000..c92eff9 --- /dev/null +++ b/lint/libs/lint_checks/tests/src/com/android/tools/lint/detector/api/ScopeTest.java @@ -0,0 +1,56 @@ +/* + * Copyright (C) 2012 The Android Open Source Project + * + * Licensed under the Eclipse Public License, Version 1.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.eclipse.org/org/documents/epl-v10.php + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package com.android.tools.lint.detector.api; + +import java.util.EnumSet; + +import junit.framework.TestCase; + +@SuppressWarnings("javadoc") +public class ScopeTest extends TestCase { + public void testIntersect() { + assertEquals(Scope.RESOURCE_FILE_SCOPE, + Scope.intersect(Scope.RESOURCE_FILE_SCOPE, Scope.RESOURCE_FILE_SCOPE)); + + assertEquals(EnumSet.of(Scope.RESOURCE_FILE), + Scope.intersect( + EnumSet.of(Scope.RESOURCE_FILE), + EnumSet.of(Scope.RESOURCE_FILE))); + + assertEquals(EnumSet.of(Scope.RESOURCE_FILE), + Scope.intersect( + EnumSet.of(Scope.RESOURCE_FILE, Scope.JAVA_FILE), + EnumSet.of(Scope.RESOURCE_FILE))); + + assertEquals(EnumSet.of(Scope.JAVA_FILE), + Scope.intersect( + EnumSet.of(Scope.RESOURCE_FILE, Scope.JAVA_FILE), + EnumSet.of(Scope.JAVA_FILE))); + + assertEquals(EnumSet.of(Scope.RESOURCE_FILE), + Scope.intersect( + EnumSet.of(Scope.RESOURCE_FILE), + EnumSet.of(Scope.RESOURCE_FILE, Scope.JAVA_FILE))); + + assertEquals(EnumSet.of(Scope.JAVA_FILE), + Scope.intersect( + EnumSet.of(Scope.JAVA_FILE), + EnumSet.of(Scope.RESOURCE_FILE, Scope.JAVA_FILE))); + + assertTrue(Scope.intersect( + EnumSet.of(Scope.JAVA_FILE), EnumSet.of(Scope.RESOURCE_FILE)).isEmpty()); + } +} |