Improve handling of library projects in lint

This changeset improves the way lint handles library projects.

Until now, running lint on project "master" would also look up any
library projects referenced by the master project, and analyze the
library projects as well. This is necessary in order to correctly
compute unused resources for example, since it's possible for a
resource to be defined in one project and referenced in another.

However, while this behavior is desirable for users who partition
their code up into library projects, it has some serious problems for
users who are using a third party library project:

- Their lint output can be swamped with errors from the library which
  they have no control over.

- If for example the library provides translations into 60 languages,
  lint will use these 60 languages as the set of languages targeted by
  the application, and complain about all strings in the master
  project which are not translated into all the languages.

This changeset makes a key change to how library projects are
handled:

   (1) If you run lint on all projects (including the library projects),
       then there is no change from before.

   (2) If you run lint and specify just a project, then lint will
       continue to analyze the project as well as all its libraries,
       but will only report problems on the user-specified project.

The way this is done is by a new "report errors" attribute stored with
each project. All projects that are explicitly referenced on the
command line (or selected in the Eclipse UI), and all projects that
are found recursively if you specify a top level directory, all these
projects have their "report errors" flag set. Any remaining projects,
which would be those found through library project references, these
have their report errors flag cleared. And whenever lint is processing
errors, it will filter out errors for projects that are not reporting
errors.

This addresses issue
http://code.google.com/p/android/issues/detail?id=33847
as well as a number of other requests (in StackOverflow and elsewhere)
around the ability to filter errors in library projects.

Change-Id: I9d084f598c678ecf79cfe70d8ea7a84844333acc

9 files changed, 222 insertions, 29 deletions

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 08a5c19..69a44f3 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
@@ -126,6 +126,7 @@ public class LintDriver {
     private List<Detector> mRepeatingDetectors;
     private EnumSet<Scope> mRepeatScope;
     private Project[] mCurrentProjects;
+    private Project mCurrentProject;
     private boolean mAbbreviating = true;
 
     /**
@@ -228,6 +229,19 @@ public class LintDriver {
     }
 
     /**
+     * Returns the projects being analyzed
+     *
+     * @return the projects being analyzed
+     */
+    @NonNull
+    public List<Project> getProjects() {
+        if (mCurrentProjects != null) {
+            return Arrays.asList(mCurrentProjects);
+        }
+        return Collections.emptyList();
+    }
+
+    /**
      * Analyze the given file (which can point to an Android project). Issues found
      * are reported to the associated {@link LintClient}.
      *
@@ -604,6 +618,15 @@ public class LintDriver {
             roots.removeAll(project.getAllLibraries());
         }
 
+        // Report issues for all projects that are explicitly referenced. We need to
+        // do this here, since the project initialization will mark all library
+        // projects as no-report projects by default.
+        for (Project project : allProjects) {
+            // Report issues for all projects explicitly listed or found via a directory
+            // traversal -- including library projects.
+            project.setReportIssues(true);
+        }
+
         if (LintUtils.assertionsEnabled()) {
             // Make sure that all the project directories are unique. This ensures
             // that we didn't accidentally end up with different project instances
@@ -661,6 +684,8 @@ public class LintDriver {
         allProjects.addAll(allLibraries);
         mCurrentProjects = allProjects.toArray(new Project[allProjects.size()]);
 
+        mCurrentProject = project;
+
         for (Detector check : mApplicableDetectors) {
             check.beforeCheckProject(projectContext);
             if (mCanceled) {
@@ -668,6 +693,7 @@ public class LintDriver {
             }
         }
 
+        assert mCurrentProject == project;
         runFileDetectors(project, project);
 
         if (!Scope.checkSingleFile(mScope)) {
@@ -675,6 +701,7 @@ public class LintDriver {
             for (Project library : libraries) {
                 Context libraryContext = new Context(this, library, project, projectDir);
                 fireEvent(EventType.SCANNING_LIBRARY_PROJECT, libraryContext);
+                mCurrentProject = library;
 
                 for (Detector check : mApplicableDetectors) {
                     check.beforeCheckLibraryProject(libraryContext);
@@ -682,12 +709,15 @@ public class LintDriver {
                         return;
                     }
                 }
+                assert mCurrentProject == library;
 
                 runFileDetectors(library, project);
                 if (mCanceled) {
                     return;
                 }
 
+                assert mCurrentProject == library;
+
                 for (Detector check : mApplicableDetectors) {
                     check.afterCheckLibraryProject(libraryContext);
                     if (mCanceled) {
@@ -697,6 +727,8 @@ public class LintDriver {
             }
         }
 
+        mCurrentProject = project;
+
         for (Detector check : mApplicableDetectors) {
             check.afterCheckProject(projectContext);
             if (mCanceled) {
@@ -1487,6 +1519,11 @@ public class LintDriver {
                 @Nullable Location location,
                 @NonNull String message,
                 @Nullable Object data) {
+            assert mCurrentProject != null;
+            if (!mCurrentProject.getReportIssues()) {
+                return;
+            }
+
             Configuration configuration = context.getConfiguration();
             if (!configuration.isEnabled(issue)) {
                 if (issue != IssueRegistry.PARSER_ERROR && issue != IssueRegistry.LINT_ERROR) {
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 c05a1cd..7629034 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
@@ -87,6 +87,7 @@ public class Project {
     private List<File> mJavaLibraries;
     private List<Project> mDirectLibraries;
     private List<Project> mAllLibraries;
+    private boolean mReportIssues = true;
 
     /**
      * Creates a new {@link Project} for the given directory.
@@ -159,7 +160,12 @@ public class Project {
                             }
                         }
 
-                        mDirectLibraries.add(client.getProject(libraryDir, libraryReferenceDir));
+                        Project libraryPrj = client.getProject(libraryDir, libraryReferenceDir);
+                        mDirectLibraries.add(libraryPrj);
+                        // By default, we don't report issues in inferred library projects.
+                        // The driver will set report = true for those library explicitly
+                        // requested.
+                        libraryPrj.setReportIssues(false);
                     }
                 } finally {
                     Closeables.closeQuietly(is);
@@ -573,6 +579,34 @@ public class Project {
         return mName;
     }
 
+    /**
+     * Sets whether lint should report issues in this project. See
+     * {@link #getReportIssues()} for a full description of what that means.
+     *
+     * @param reportIssues whether lint should report issues in this project
+     */
+    public void setReportIssues(boolean reportIssues) {
+        mReportIssues = reportIssues;
+    }
+
+    /**
+     * Returns whether lint should report issues in this project.
+     * <p>
+     * If a user specifies a project and its library projects for analysis, then
+     * those library projects are all "included", and all errors found in all
+     * the projects are reported. But if the user is only running lint on the
+     * main project, we shouldn't report errors in any of the library projects.
+     * We still need to <b>consider</b> them for certain types of checks, such
+     * as determining whether resources found in the main project are unused, so
+     * the detectors must still get a chance to look at these projects. The
+     * {@code #getReportIssues()} attribute is used for this purpose.
+     *
+     * @return whether lint should report issues in this project
+     */
+    public boolean getReportIssues() {
+        return mReportIssues;
+    }
+
     // ---------------------------------------------------------------------------
     // Support for running lint on the AOSP source tree itself
 
diff --git a/lint/libs/lint_checks/src/com/android/tools/lint/checks/TranslationDetector.java b/lint/libs/lint_checks/src/com/android/tools/lint/checks/TranslationDetector.java
index a60e206..f89fb81 100644
--- a/lint/libs/lint_checks/src/com/android/tools/lint/checks/TranslationDetector.java
+++ b/lint/libs/lint_checks/src/com/android/tools/lint/checks/TranslationDetector.java
@@ -143,13 +143,19 @@ public class TranslationDetector extends ResourceXmlDetector {
 
         // Convention seen in various projects
         mIgnoreFile = context.file.getName().startsWith("donottranslate"); //$NON-NLS-1$
+
+        if (!context.getProject().getReportIssues()) {
+            mIgnoreFile = true;
+        }
     }
 
     @Override
     public void afterCheckFile(@NonNull Context context) {
         if (context.getPhase() == 1) {
             // Store this layout's set of ids for full project analysis in afterCheckProject
-            mFileToNames.put(context.file, mNames);
+            if (context.getProject().getReportIssues()) {
+                mFileToNames.put(context.file, mNames);
+            }
 
             mNames = null;
         }
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 a593f1f..bcbdd85 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
@@ -49,6 +49,7 @@ 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.Project;
 import com.android.tools.lint.detector.api.ResourceXmlDetector;
 import com.android.tools.lint.detector.api.Scope;
 import com.android.tools.lint.detector.api.Severity;
@@ -99,6 +100,7 @@ public class UnusedResourceDetector extends ResourceXmlDetector implements Detec
             Severity.WARNING,
             UnusedResourceDetector.class,
             EnumSet.of(Scope.MANIFEST, Scope.ALL_RESOURCE_FILES, Scope.ALL_JAVA_FILES));
+
     /** Unused id's */
     public static final Issue ISSUE_IDS = Issue.create("UnusedIds", //$NON-NLS-1$
             "Looks for unused id's",
@@ -175,14 +177,18 @@ public class UnusedResourceDetector extends ResourceXmlDetector implements Detec
                                 if (xmlContext.getDriver().isSuppressed(ISSUE, root)) {
                                     //  Also remove it from consideration such that even the
                                     // presence of this field in the R file is ignored.
-                                    if (mUnused != null) {
-                                        mUnused.remove(resource);
-                                    }
+                                    mUnused.remove(resource);
                                     return;
                                 }
                             }
                         }
 
+                        if (!context.getProject().getReportIssues()) {
+                            // If this is a library project not being analyzed, ignore it
+                            mUnused.remove(resource);
+                            return;
+                        }
+
                         recordLocation(resource, Location.create(file));
                     }
                 }
@@ -289,12 +295,34 @@ public class UnusedResourceDetector extends ResourceXmlDetector implements Detec
                 List<String> sorted = new ArrayList<String>(mUnused.keySet());
                 Collections.sort(sorted);
 
+                Boolean skippedLibraries = null;
+
                 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);
                     }
+
+                    if (location == null) {
+                        if (skippedLibraries == null) {
+                            skippedLibraries = false;
+                            for (Project project : context.getDriver().getProjects()) {
+                                if (!project.getReportIssues()) {
+                                    skippedLibraries = true;
+                                    break;
+                                }
+                            }
+                        }
+                        if (skippedLibraries) {
+                            // Skip this resource if we don't have a location, and one or
+                            // more library projects were skipped; the resource was very
+                            // probably defined in that library project and only encountered
+                            // in the main project's java R file
+                            continue;
+                        }
+                    }
+
                     String message = String.format("The resource %1$s appears to be unused",
                             resource);
                     Issue issue = getIssue(resource);
@@ -364,6 +392,10 @@ public class UnusedResourceDetector extends ResourceXmlDetector implements Detec
                                 mUnused.remove(resource);
                                 return;
                             }
+                            if (!context.getProject().getReportIssues()) {
+                                mUnused.remove(resource);
+                                return;
+                            }
                             recordLocation(resource, context.getLocation(nameAttribute));
                         }
                     }
@@ -422,6 +454,10 @@ public class UnusedResourceDetector extends ResourceXmlDetector implements Detec
                     mUnused.remove(resource);
                     return;
                 }
+                if (!context.getProject().getReportIssues()) {
+                    mUnused.remove(resource);
+                    return;
+                }
                 recordLocation(resource, context.getLocation(attribute));
                 return;
             }
diff --git a/lint/libs/lint_checks/tests/src/com/android/tools/lint/checks/AbstractCheckTest.java b/lint/libs/lint_checks/tests/src/com/android/tools/lint/checks/AbstractCheckTest.java
index 9c0904f..d05a203 100644
--- a/lint/libs/lint_checks/tests/src/com/android/tools/lint/checks/AbstractCheckTest.java
+++ b/lint/libs/lint_checks/tests/src/com/android/tools/lint/checks/AbstractCheckTest.java
@@ -77,13 +77,14 @@ public abstract class AbstractCheckTest extends TestCase {
 
     protected String lintFiles(String... relativePaths) throws Exception {
         List<File> files = new ArrayList<File>();
+        File targetDir = getTargetDir();
         for (String relativePath : relativePaths) {
-            File file = getTestfile(relativePath);
+            File file = getTestfile(targetDir, relativePath);
             assertNotNull(file);
             files.add(file);
         }
 
-        addManifestFile(getTargetDir());
+        addManifestFile(targetDir);
 
         return checkLint(files);
     }
@@ -107,7 +108,16 @@ public abstract class AbstractCheckTest extends TestCase {
             mOutput.append("No warnings.");
         }
 
-        return mOutput.toString();
+        String result = mOutput.toString();
+
+        // The output typically contains a few directory/filenames.
+        // On Windows we need to change the separators to the unix-style
+        // forward slash to make the test as OS-agnostic as possible.
+        if (File.separatorChar != '/') {
+            result = result.replace(File.separatorChar, '/');
+        }
+
+        return result;
     }
 
     /**
@@ -116,28 +126,30 @@ public abstract class AbstractCheckTest extends TestCase {
      *   separators to the unix-style forward slash.
      */
     protected String lintProject(String... relativePaths) throws Exception {
+        File projectDir = getProjectDir(null, relativePaths);
+        return checkLint(Collections.singletonList(projectDir));
+    }
+
+    /** Creates a project directory structure from the given files */
+    protected File getProjectDir(String name, String ...relativePaths) throws Exception {
         assertFalse("getTargetDir must be overridden to make a unique directory",
                 getTargetDir().equals(getTempDir()));
 
         File projectDir = getTargetDir();
+        if (name != null) {
+            projectDir = new File(projectDir, name);
+        }
+        assertTrue(projectDir.getPath(), projectDir.mkdirs());
 
         List<File> files = new ArrayList<File>();
         for (String relativePath : relativePaths) {
-            File file = getTestfile(relativePath);
+            File file = getTestfile(projectDir, relativePath);
             assertNotNull(file);
             files.add(file);
         }
 
         addManifestFile(projectDir);
-
-        String result = checkLint(Collections.singletonList(projectDir));
-        // The output typically contains a few directory/filenames.
-        // On Windows we need to change the separators to the unix-style
-        // forward slash to make the test as OS-agnostic as possible.
-        if (File.separatorChar != '/') {
-            result = result.replace(File.separatorChar, '/');
-        }
-        return result;
+        return projectDir;
     }
 
     private void addManifestFile(File projectDir) throws IOException {
@@ -185,7 +197,11 @@ public abstract class AbstractCheckTest extends TestCase {
 
     private File makeTestFile(String name, String relative,
             final InputStream contents) throws IOException {
-        File dir = getTargetDir();
+        return makeTestFile(getTargetDir(), name, relative, contents);
+    }
+
+    private File makeTestFile(File dir, String name, String relative,
+            final InputStream contents) throws IOException {
         if (relative != null) {
             dir = new File(dir, relative);
             if (!dir.exists()) {
@@ -210,7 +226,7 @@ public abstract class AbstractCheckTest extends TestCase {
         return tempFile;
     }
 
-    private File getTestfile(String relativePath) throws IOException {
+    private File getTestfile(File targetDir, String relativePath) throws IOException {
         // Support replacing filenames and paths with a => syntax, e.g.
         //   dir/file.txt=>dir2/dir3/file2.java
         // will read dir/file.txt from the test data and write it into the target
@@ -236,7 +252,7 @@ public abstract class AbstractCheckTest extends TestCase {
             relative = targetPath.substring(0, index);
         }
 
-        return makeTestFile(name, relative, stream);
+        return makeTestFile(targetDir, name, relative, stream);
     }
 
     protected boolean isEnabled(Issue issue) {
diff --git a/lint/libs/lint_checks/tests/src/com/android/tools/lint/checks/IconDetectorTest.java b/lint/libs/lint_checks/tests/src/com/android/tools/lint/checks/IconDetectorTest.java
index 6c08149..d33088f 100644
--- a/lint/libs/lint_checks/tests/src/com/android/tools/lint/checks/IconDetectorTest.java
+++ b/lint/libs/lint_checks/tests/src/com/android/tools/lint/checks/IconDetectorTest.java
@@ -121,5 +121,4 @@ public class IconDetectorTest extends AbstractCheckTest {
                     "res/drawable/states.xml=>res/drawable-hdpi/f.xml",
                     "res/drawable/states.xml=>res/drawable-xhdpi/f.xml"));
     }
-
 }
\ No newline at end of file
diff --git a/lint/libs/lint_checks/tests/src/com/android/tools/lint/checks/RegistrationDetectorTest.java b/lint/libs/lint_checks/tests/src/com/android/tools/lint/checks/RegistrationDetectorTest.java
index 75d784f..2369fad 100644
--- a/lint/libs/lint_checks/tests/src/com/android/tools/lint/checks/RegistrationDetectorTest.java
+++ b/lint/libs/lint_checks/tests/src/com/android/tools/lint/checks/RegistrationDetectorTest.java
@@ -85,4 +85,25 @@ public class RegistrationDetectorTest extends AbstractCheckTest {
                 "bytecode/TestReceiver$1.class.data=>bin/classes/test/pkg/TestReceiver$1.class"
                 ));
     }
+
+    public void testLibraryProjects() throws Exception {
+        // If a library project provides additional activities, it is not an error to
+        // not register all of those here
+        assertEquals(
+            "No warnings.",
+
+            lintProject(
+                // Master project
+                "multiproject/main-manifest.xml=>AndroidManifest.xml",
+                "multiproject/main.properties=>project.properties",
+
+                // Library project
+                "multiproject/library-manifest.xml=>../LibraryProject/AndroidManifest.xml",
+                "multiproject/library.properties=>../LibraryProject/project.properties",
+
+                "bytecode/.classpath=>../LibraryProject/.classpath",
+                "bytecode/OnClickActivity.java.txt=>../LibraryProject/src/test/pkg/OnClickActivity.java",
+                "bytecode/OnClickActivity.class.data=>../LibraryProject/bin/classes/test/pkg/OnClickActivity.class"
+                ));
+    }
 }
diff --git a/lint/libs/lint_checks/tests/src/com/android/tools/lint/checks/TranslationDetectorTest.java b/lint/libs/lint_checks/tests/src/com/android/tools/lint/checks/TranslationDetectorTest.java
index 6201bdb..6f1c2e6 100644
--- a/lint/libs/lint_checks/tests/src/com/android/tools/lint/checks/TranslationDetectorTest.java
+++ b/lint/libs/lint_checks/tests/src/com/android/tools/lint/checks/TranslationDetectorTest.java
@@ -114,4 +114,27 @@ public class TranslationDetectorTest extends AbstractCheckTest {
                         "res/values/strings3.xml=>res/values/strings.xml",
                         "res/values-fr/strings.xml=>res/values-fr/strings.xml"));
     }
+
+    public void testLibraryProjects() throws Exception {
+        // If a library project provides additional locales, that should not force
+        // the main project to include all those translations
+        assertEquals(
+            "No warnings.",
+
+             lintProject(
+                 // Master project
+                 "multiproject/main-manifest.xml=>AndroidManifest.xml",
+                 "multiproject/main.properties=>project.properties",
+                 "res/values/strings2.xml",
+
+                 // Library project
+                 "multiproject/library-manifest.xml=>../LibraryProject/AndroidManifest.xml",
+                 "multiproject/library.properties=>../LibraryProject/project.properties",
+
+                 "res/values/strings.xml=>../LibraryProject/res/values/strings.xml",
+                 "res/values-cs/strings.xml=>../LibraryProject/res/values-cs/strings.xml",
+                 "res/values-cs/strings.xml=>../LibraryProject/res/values-de/strings.xml",
+                 "res/values-cs/strings.xml=>../LibraryProject/res/values-nl/strings.xml"
+             ));
+    }
 }
diff --git a/lint/libs/lint_checks/tests/src/com/android/tools/lint/checks/UnusedResourceDetectorTest.java b/lint/libs/lint_checks/tests/src/com/android/tools/lint/checks/UnusedResourceDetectorTest.java
index d2af622..bf32e2b 100644
--- a/lint/libs/lint_checks/tests/src/com/android/tools/lint/checks/UnusedResourceDetectorTest.java
+++ b/lint/libs/lint_checks/tests/src/com/android/tools/lint/checks/UnusedResourceDetectorTest.java
@@ -19,6 +19,9 @@ package com.android.tools.lint.checks;
 import com.android.tools.lint.detector.api.Detector;
 import com.android.tools.lint.detector.api.Issue;
 
+import java.io.File;
+import java.util.Arrays;
+
 @SuppressWarnings("javadoc")
 public class UnusedResourceDetectorTest extends AbstractCheckTest {
     private boolean mEnableIds = false;
@@ -101,12 +104,9 @@ public class UnusedResourceDetectorTest extends AbstractCheckTest {
                 "AndroidManifest.xml"));
     }
 
-    public void testMultiProject() throws Exception {
+    public void testMultiProjectIgnoreLibraries() throws Exception {
         assertEquals(
-           // string1 is defined and used in the library project
-           // string2 is defined in the library project and used in the master project
-           // string3 is defined in the library project and not used anywhere
-           "strings.xml:7: Warning: The resource R.string.string3 appears to be unused",
+           "No warnings.",
 
             lintProject(
                 // Master project
@@ -122,6 +122,29 @@ public class UnusedResourceDetectorTest extends AbstractCheckTest {
             ));
     }
 
+    public void testMultiProject() throws Exception {
+        File master = getProjectDir("MasterProject",
+                // Master project
+                "multiproject/main-manifest.xml=>AndroidManifest.xml",
+                "multiproject/main.properties=>project.properties",
+                "multiproject/MainCode.java.txt=>src/foo/main/MainCode.java"
+        );
+        File library = getProjectDir("LibraryProject",
+                // Library project
+                "multiproject/library-manifest.xml=>AndroidManifest.xml",
+                "multiproject/library.properties=>project.properties",
+                "multiproject/LibraryCode.java.txt=>src/foo/library/LibraryCode.java",
+                "multiproject/strings.xml=>res/values/strings.xml"
+        );
+        assertEquals(
+           // string1 is defined and used in the library project
+           // string2 is defined in the library project and used in the master project
+           // string3 is defined in the library project and not used anywhere
+           "strings.xml:7: Warning: The resource R.string.string3 appears to be unused",
+
+           checkLint(Arrays.asList(master, library)));
+    }
+
     public void testFqcnReference() throws Exception {
         assertEquals(
            "No warnings.",
@@ -151,6 +174,4 @@ public class UnusedResourceDetectorTest extends AbstractCheckTest {
                 "res/values/plurals.xml",
                 "AndroidManifest.xml"));
     }
-
-
 }