Allow all lint tasks to be up-to-date and cacheable

jsotuyod · jsotuyod · commit 371d1fdd6a8e · 2018-09-29T05:01:39.000-03:00
- Also test against AGP 3.2.0
 - Acknowledge newer lint tasks actualy define input files
diff --git a/src/integTest/groovy/com/monits/gradle/sca/AndroidLintIntegTest.groovy b/src/integTest/groovy/com/monits/gradle/sca/AndroidLintIntegTest.groovy
@@ -35,7 +35,7 @@ import static org.junit.Assert.assertThat
 class AndroidLintIntegTest extends AbstractIntegTestFixture {
 
     static final List<String> ANDROID_PLUGIN_VERSIONS = (['1.1.3', '1.2.3', '1.3.1', '1.5.0', '2.0.0', '2.1.3'] +
-        (Jvm.current.java8Compatible ? ['2.2.3', '2.3.3', '3.0.1'] : [])).asImmutable()
+        (Jvm.current.java8Compatible ? ['2.2.3', '2.3.3', '3.0.1', '3.2.0'] : [])).asImmutable()
 
     @SuppressWarnings('MethodName')
     @Unroll('AndroidLint should run when using gradle #version')
@@ -113,6 +113,8 @@ class AndroidLintIntegTest extends AbstractIntegTestFixture {
         BuildResult secondRun = gradleRunner.build()
 
         then:
+        file('build/outputs/').list().each { println it }
+
         firstRun.task(taskName()).outcome == SUCCESS
         secondRun.task(taskName()).outcome == UP_TO_DATE
 
@@ -124,6 +126,37 @@ class AndroidLintIntegTest extends AbstractIntegTestFixture {
         gradleVersion = gradleVersionForAndroid(androidVersion)
     }
 
+    @SuppressWarnings('MethodName')
+    @Unroll('AndroidLint per variant re-run is up-to-date when using plugin version #androidVersion')
+    void 'rerun is up-to-date for variants'() {
+        given:
+        writeAndroidBuildFile(androidVersion)
+        useSimpleAndroidLintConfig()
+        writeAndroidManifest()
+        goodCode()
+
+        when:
+        GradleRunner gradleRunner = gradleRunner()
+            .withGradleVersion(gradleVersion)
+            // plugin version 1.1.x failed to compile tests if assemble was not called beforehand
+            .withArguments('assemble', 'lintDebug', 'lintRelease', '--stacktrace')
+        BuildResult firstRun = gradleRunner.build()
+        BuildResult secondRun = gradleRunner.build()
+
+        then:
+        file('build/outputs/').list().each { println it }
+
+        firstRun.task(':lintDebug').outcome == SUCCESS
+        secondRun.task(':lintDebug').outcome == UP_TO_DATE
+
+        firstRun.task(':lintRelease').outcome == SUCCESS
+        secondRun.task(':lintRelease').outcome == UP_TO_DATE
+
+        where:
+        androidVersion << ANDROID_PLUGIN_VERSIONS
+        gradleVersion = gradleVersionForAndroid(androidVersion)
+    }
+
     @SuppressWarnings('MethodName')
     @Unroll('AndroidLint is skipped when disabled and using plugin version #androidVersion')
     void 'task is skipped if disabled'() {
diff --git a/src/integTest/groovy/com/monits/gradle/sca/fixture/AbstractIntegTestFixture.groovy b/src/integTest/groovy/com/monits/gradle/sca/fixture/AbstractIntegTestFixture.groovy
@@ -144,6 +144,9 @@ abstract class AbstractIntegTestFixture extends Specification {
     }
 
     TestFile writeAndroidBuildFile(Map<String, Object> toolsConfig) {
+        String extraRepository = toolsConfig.get(ANDROID_VERSION, DEFAULT_ANDROID_VERSION).startsWith('3') ? 'google()'
+            : ''
+
         buildScriptFile() << """
             |buildscript {
             |    dependencies {
@@ -154,12 +157,13 @@ abstract class AbstractIntegTestFixture extends Specification {
             |
             |    repositories {
             |        jcenter()
-            |        ${toolsConfig.get(ANDROID_VERSION, DEFAULT_ANDROID_VERSION).startsWith('3') ? 'google()' : ''}
+            |        ${extraRepository}
             |    }
             |}
             |
             |repositories {
-            |    mavenCentral()
+            |    jcenter()
+            |    ${extraRepository}
             |}
             |
             |apply plugin: 'com.android.library'
diff --git a/src/main/groovy/com/monits/gradle/sca/AndroidHelper.groovy b/src/main/groovy/com/monits/gradle/sca/AndroidHelper.groovy
@@ -27,16 +27,22 @@ final class AndroidHelper {
     private static final String ANDROID_ENABLE_CACHE_PROPERTY = 'android.enableBuildCache'
     private static final String ANDROID_CACHE_LOCATION = 'android.buildCacheDir'
     private static final String ANDROID_DEPENDENCY_PATTERN = /com\.android\.tools\.build\/gradle\/([^\/]+)/
-    private static final VersionNumber BUILD_CACHE_ANDROID_GRADLE_VERSION = VersionNumber.parse('2.3.0')
-    private static final VersionNumber REPORT_PER_VARIANT_ANDROID_GRADLE_VERSION = VersionNumber.parse('2.0.0')
+    private static final String VERSION_2_3_0 = '2.3.0'
+
+    private static final VersionNumber BUILD_CACHE_ANDROID_GRADLE_VERSION = VersionNumber.parse(VERSION_2_3_0)
+    private static final VersionNumber REPORT_PER_VARIANT_ANDROID_GRADLE_VERSION_MIN = VersionNumber.parse('2.0.0')
+    private static final VersionNumber REPORT_PER_VARIANT_ANDROID_GRADLE_VERSION_MAX = VersionNumber.parse('2.2.9')
+    private static final VersionNumber LINT_HAS_VARIANT_INFO = VersionNumber.parse('1.5.0')
+    private static final VersionNumber USES_REPORTS_DIR = VersionNumber.parse(VERSION_2_3_0)
 
     /**
-     * Checks if the current Android Plugin produces a report per variant or not.
+     * Checks if the current Android Plugin produces a global report that matches a debuggable variant or not.
      * @param project The project to analyze.
      * @return True if a report per variant is expected, false otherwise
      */
-    static boolean lintReportPerVariant(final Project project) {
-        getCurrentVersion(project) >= REPORT_PER_VARIANT_ANDROID_GRADLE_VERSION
+    static boolean globalLintIsVariant(final Project project) {
+        getCurrentVersion(project) >= REPORT_PER_VARIANT_ANDROID_GRADLE_VERSION_MIN &&
+            getCurrentVersion(project) <= REPORT_PER_VARIANT_ANDROID_GRADLE_VERSION_MAX
     }
 
     /**
@@ -53,6 +59,30 @@ final class AndroidHelper {
                 project.property(ANDROID_ENABLE_CACHE_PROPERTY) == 'true')
     }
 
+    /**
+     * Checks if the current Android build has variant info on the lint task.
+     *
+     * @param project The project to analyze
+     * @return True if the AGP version in use provides variant info on the lint task, false otherwise
+     */
+    static boolean lintTaskHasVariantInfo(final Project project) {
+        getCurrentVersion(project) >= LINT_HAS_VARIANT_INFO
+    }
+
+    /**
+     * Retrieves the location were lint reports are output by AGP.
+     *
+     * @param project The project to analyze
+     * @return The directory in which AGP outputs lint reports
+     */
+    static String getLintReportDir(final Project project) {
+        if (getCurrentVersion(project) >= USES_REPORTS_DIR) {
+            return "${project.buildDir}/reports/"
+        }
+
+        "${project.buildDir}/outputs/"
+    }
+
     /**
      * Retrieves the location of Android's build-cache directory.
      * @param project The project to analyze.
diff --git a/src/main/groovy/com/monits/gradle/sca/config/AndroidLintConfigurator.groovy b/src/main/groovy/com/monits/gradle/sca/config/AndroidLintConfigurator.groovy
@@ -23,6 +23,7 @@ import org.gradle.api.DomainObjectSet
 import org.gradle.api.Project
 import org.gradle.api.Task
 import org.gradle.api.specs.Specs
+import org.gradle.api.tasks.Copy
 import org.gradle.util.GradleVersion
 
 /**
@@ -35,6 +36,9 @@ class AndroidLintConfigurator implements AnalysisConfigurator {
     private static final String JACK_OPTIONS_PROPERTY_NAME = 'jackOptions'
     private static final String ANDROID = 'android'
     private static final String LINT_OPTIONS = 'lintOptions'
+    private static final String GLOBAL_LINT_TASK_NAME = 'lint'
+    private static final String XML = 'xml'
+    private static final String HTML = 'html'
 
     private final RemoteConfigLocator configLocator = new RemoteConfigLocator(ANDROID)
 
@@ -45,17 +49,38 @@ class AndroidLintConfigurator implements AnalysisConfigurator {
 
     @Override
     void applyAndroidConfig(final Project project, final StaticCodeAnalysisExtension extension) {
-        project.tasks.matching { Task it -> it.name == 'lint' } .all { Task t ->
+        Class<? extends Task> lintTask = getLintTaskClass(project)
+        project.tasks.withType(lintTask) { Task t ->
             setupTasks(t, project, extension)
 
             configureLintTask(project, extension, t)
         }
     }
 
+    private static Class<? extends Task> getLintTaskClass(final Project project) {
+        try {
+            // AGP 3.0+
+            return getClass().classLoader
+                .loadClass('com.android.build.gradle.tasks.LintBaseTask') as Class<? extends Task>
+        } catch (ClassNotFoundException ignored) {
+            try {
+                // Older versions
+                return getClass().classLoader
+                    .loadClass('com.android.build.gradle.tasks.Lint') as Class<? extends Task>
+            } catch (ClassNotFoundException e) {
+                // Something went wrong!
+                warnUnexpectedException(project, 'Encountered an error trying to configure Android Lint tasks.', e)
+
+                // Best effort, get the global lint task class (needs to be configured!)
+                project.tasks.getByName(GLOBAL_LINT_TASK_NAME).class
+            }
+        }
+    }
+
     private static void setupTasks(final Task lintTask, final Project project,
                                    final StaticCodeAnalysisExtension extension) {
-        Task resolveTask = project.tasks.create('resolveAndroidLint', ResolveAndroidLintTask)
-        Task cleanupTask = project.tasks.create('cleanupAndroidLint', CleanupAndroidLintTask)
+        Task resolveTask = project.tasks.maybeCreate('resolveAndroidLint', ResolveAndroidLintTask)
+        Task cleanupTask = project.tasks.maybeCreate('cleanupAndroidLint', CleanupAndroidLintTask)
 
         lintTask.dependsOn resolveTask
         lintTask.finalizedBy cleanupTask
@@ -71,14 +96,12 @@ class AndroidLintConfigurator implements AnalysisConfigurator {
                                              final File configSource, final Task lintTask) {
         def lintOptions = project[ANDROID][LINT_OPTIONS]
 
+        // update global config
         lintOptions.with { it ->
             // TODO : This won't fail on warnings, just like Checkstyle.
             // See https://issues.gradle.org/browse/GRADLE-2888
             it['abortOnError'] = !extension.ignoreErrors
 
-            // Change output location for consistency with other plugins
-            it['xmlOutput'] = project.file("${project.buildDir}/reports/android/lint-results.xml")
-
             // Update global config
             it['lintConfig'] = configSource
         }
@@ -94,6 +117,15 @@ class AndroidLintConfigurator implements AnalysisConfigurator {
 
         configureLintOptions(project, extension, config, lintTask)
 
+        if (lintTask.name == GLOBAL_LINT_TASK_NAME) {
+            // Change output location for consistency with other plugins
+            // we copy as to not tamper with other lint tasks
+            lintTask.finalizedBy(project.tasks.create('copyLintReport', Copy) { Copy it ->
+                it.from lintTask.outputs.files.filter { File f -> f.name.endsWith('.xml') }
+                it.into project.file("${project.buildDir}/reports/android/lint-results.xml")
+            })
+        }
+
         try {
             configureLintInputsAndOutputs(project, lintTask)
 
@@ -103,9 +135,8 @@ class AndroidLintConfigurator implements AnalysisConfigurator {
             }
         } catch (Throwable e) {
             // Something went wrong!
-            project.logger.warn('Encountered an error trying to set inputs and outputs for Android Lint ' +
-                    'tasks, it will be disabled. Please, report this incident in ' +
-                    'https://github.com/monits/static-code-analysis-plugin/issues', e)
+            warnUnexpectedException(project, 'Encountered an error trying to set inputs and outputs for Android Lint ' +
+                    'tasks, it will be disabled.', e)
 
             // disable up-to-date caching
             lintTask.outputs.upToDateWhen {
@@ -114,6 +145,11 @@ class AndroidLintConfigurator implements AnalysisConfigurator {
         }
     }
 
+    private static void warnUnexpectedException(final Project project, final String message, final Throwable e) {
+        project.logger.warn(message + ' Please, report this incident at ' +
+            'https://github.com/monits/static-code-analysis-plugin/issues', e)
+    }
+
     private File obtainLintRules(final Project project, final StaticCodeAnalysisExtension config,
                                     final Task lintTask) {
         boolean remoteLocation = RemoteConfigLocator.isRemoteLocation(config.androidLintConfig)
@@ -150,27 +186,41 @@ class AndroidLintConfigurator implements AnalysisConfigurator {
 
         DomainObjectSet<?> variants = getVariants(project)
 
-        String defaultReportVariant = null
-        variants.all {
-            def configuration = it.variantData.variantConfiguration
-            String variantName = it.name
-            String variantDirName = configuration.dirName
-
-            lintTask.inputs.with {
-                dir("${project.buildDir}/intermediates/classes/${variantDirName}/")
-                dir("${project.buildDir}/intermediates/assets/${variantDirName}/")
-                dir("${project.buildDir}/intermediates/manifests/full/${variantDirName}/")
-                dir("${project.buildDir}/intermediates/res/merged/${variantDirName}/")
-                dir("${project.buildDir}/intermediates/shaders/${variantDirName}/")
-                dir("${project.buildDir}/intermediates/rs/${variantDirName}/")
+        String variantName = AndroidHelper.lintTaskHasVariantInfo(project) ? lintTask.variantName
+            : (lintTask.name.toLowerCase() - GLOBAL_LINT_TASK_NAME)
+
+        // Older plugins didn't setup input files, so up-to-date checks were futile
+        if (lintTask.inputs.files.isEmpty()) {
+            variants.matching { it.name == variantName || variantName == null || variantName.empty }.all {
+                def configuration = it.variantData.variantConfiguration
+                String variantDirName = configuration.dirName
+
+                lintTask.inputs.with {
+                    dir("${project.buildDir}/intermediates/classes/${variantDirName}/")
+                    dir("${project.buildDir}/intermediates/assets/${variantDirName}/")
+                    dir("${project.buildDir}/intermediates/manifests/full/${variantDirName}/")
+                    dir("${project.buildDir}/intermediates/res/merged/${variantDirName}/")
+                    dir("${project.buildDir}/intermediates/shaders/${variantDirName}/")
+                    dir("${project.buildDir}/intermediates/rs/${variantDirName}/")
+                }
             }
+        }
 
-            if (!defaultReportVariant && configuration.buildType.isDebuggable() && !usesJack(configuration)) {
-                defaultReportVariant = variantName
+        // And none up to this date setup outputs for up-to-date checks and cache
+        if ((variantName == null || variantName.empty) && AndroidHelper.globalLintIsVariant(project)) {
+            boolean configFound = false
+            variants.all {
+                def configuration = it.variantData.variantConfiguration
+                if (!configFound && configuration.buildType.isDebuggable() && !usesJack(configuration)) {
+                    configFound = true
 
-                addReportAsOutput(lintTask, project, xmlEnabled, xmlOutput, defaultReportVariant, 'xml')
-                addReportAsOutput(lintTask, project, htmlEnabled, htmlOutput, defaultReportVariant, 'html')
+                    addReportAsOutput(lintTask, project, xmlEnabled, xmlOutput, it.name, XML)
+                    addReportAsOutput(lintTask, project, htmlEnabled, htmlOutput, it.name, HTML)
+                }
             }
+        } else {
+            addReportAsOutput(lintTask, project, xmlEnabled, xmlOutput, variantName, XML)
+            addReportAsOutput(lintTask, project, htmlEnabled, htmlOutput, variantName, HTML)
         }
     }
 
@@ -231,11 +281,11 @@ class AndroidLintConfigurator implements AnalysisConfigurator {
             File definiteOutput = output
             if (!output) {
                 // Convention naming changed along the way
-                if (AndroidHelper.lintReportPerVariant(project)) {
+                if (variantName) {
                     definiteOutput = project.file(
-                            "${project.buildDir}/outputs/lint-results-${variantName}.${extension}")
+                        AndroidHelper.getLintReportDir(project) + "lint-results-${variantName}.${extension}")
                 } else {
-                    definiteOutput = project.file("${project.buildDir}/outputs/lint-results.${extension}")
+                    definiteOutput = project.file(AndroidHelper.getLintReportDir(project) + "lint-results.${extension}")
                 }
             }
             task.outputs.file definiteOutput

Original file line number	Diff line number	Diff line change
`@@ -144,6 +144,9 @@ abstract class AbstractIntegTestFixture extends Specification {`
`144`	`144`	`}`
`145`	`145`
`146`	`146`	`TestFile writeAndroidBuildFile(Map<String, Object> toolsConfig) {`
	`147`	`+ String extraRepository = toolsConfig.get(ANDROID_VERSION, DEFAULT_ANDROID_VERSION).startsWith('3') ? 'google()'`
	`148`	`+ : ''`
	`149`	`+`
`147`	`150`	`buildScriptFile() << """`
`148`	`151`	`\|buildscript {`
`149`	`152`	`\| dependencies {`
`@@ -154,12 +157,13 @@ abstract class AbstractIntegTestFixture extends Specification {`
`154`	`157`	`\|`
`155`	`158`	`\| repositories {`
`156`	`159`	`\| jcenter()`
`157`		`- \| ${toolsConfig.get(ANDROID_VERSION, DEFAULT_ANDROID_VERSION).startsWith('3') ? 'google()' : ''}`
	`160`	`+ \| ${extraRepository}`
`158`	`161`	`\| }`
`159`	`162`	`\|}`
`160`	`163`	`\|`
`161`	`164`	`\|repositories {`
`162`		`- \| mavenCentral()`
	`165`	`+ \| jcenter()`
	`166`	`+ \| ${extraRepository}`
`163`	`167`	`\|}`
`164`	`168`	`\|`
`165`	`169`	`\|apply plugin: 'com.android.library'`