[PT-282] Integrate Android Lint

README.md

@@ -21,12 +21,12 @@ The plugin supports various static analysis tools for Java, Kotlin and Android p
  * [`PMD`](https://pmd.github.io)
  * [`FindBugs`](http://findbugs.sourceforge.net/)
  * [`Detekt`](https://github.com/arturbosch/detekt)
+ * [`Android Lint`](https://developer.android.com/studio/write/lint.html)
 
 Support for additional tools is planned but not available yet:
 
  * [`KtLint`](https://github.com/shyiko/ktlint)
- * [`Android Lint`](https://developer.android.com/studio/write/lint.html)
-
+
 Please note that the tools availability depends on the project the plugin is applied to. For more details please refer to the
 [supported tools](docs/supported-tools.md) page.
 
@@ -68,6 +68,7 @@ staticAnalysis {
     pmd { }
     findbugs { }
     detekt { }
+    lintOptions { }
 }
 ```
 

docs/supported-tools.md

@@ -9,8 +9,8 @@ Tool | Java | Android<br/>(Java) | Kotlin | Android<br/>(Kotlin)
 [`PMD`](https://pmd.github.io) | :white_check_mark: | :white_check_mark: | — | —
 [`FindBugs`](http://findbugs.sourceforge.net/) | :white_check_mark: | :white_check_mark: | — | —
 [`Detekt`](https://github.com/arturbosch/detekt) | — | — | :white_check_mark: | :white_check_mark:
+[`Android Lint`](https://developer.android.com/studio/write/lint.html) | — | :white_check_mark:️ | — | :white_check_mark:️
 [`KtLint`\*](https://github.com/shyiko/ktlint) | — | — | ✖️ | ✖️
-[`Android Lint`\*](https://developer.android.com/studio/write/lint.html) | — | ✖️ | — | ✖️
 
 _\* Not supported [yet](https://github.com/novoda/gradle-static-analysis-plugin/issues?q=is%3Aopen+is%3Aissue+label%3A%22new+tool%22)_
 
@@ -24,8 +24,8 @@ For additional informations and tips on how to obtain advanced behaviours with t
    * [Checkstyle](tools/checkstyle.md)
    * [PMD](tools/pmd.md)
    * [Findbugs](tools/findbugs.md)
+   * [Android Lint](tools/android_lint.md)
    * KtLint — _COMING SOON_
-   * Android Lint — _COMING SOON_
  * [Example configurations](#example-configurations)
 
 ---

docs/tools/android_lint.md

@@ -0,0 +1,9 @@
+# Android Lint
+[Android Lint](https://developer.android.com/studio/write/lint.html) is a linter and static analysis tool for Android projects which can detect bugs
+and potential issues in code, resources and configuration files.
+
+Be aware that Lint just supports Kotlin since version 3.1.0 of the [Android Gradle Plugin](https://developer.android.com/studio/releases/gradle-plugin.html).
+
+## Configure Android Lint
+Lint is configured through the `lintOptions` closure. It supports all [official](https://google.github.io/android-gradle-dsl/current/com.android.build.gradle.internal.dsl.LintOptions.html)
+properties except `abortOnError`, `htmlReport` and `xmlReport`. These are overridden so that Lint won't break the build on its own and always generates reports.

plugin/src/main/groovy/com/novoda/staticanalysis/StaticAnalysisPlugin.groovy

@@ -5,6 +5,7 @@ import com.novoda.staticanalysis.internal.Violations
 import com.novoda.staticanalysis.internal.checkstyle.CheckstyleConfigurator
 import com.novoda.staticanalysis.internal.detekt.DetektConfigurator
 import com.novoda.staticanalysis.internal.findbugs.FindbugsConfigurator
+import com.novoda.staticanalysis.internal.lint.LintConfigurator
 import com.novoda.staticanalysis.internal.pmd.PmdConfigurator
 import org.gradle.api.NamedDomainObjectContainer
 import org.gradle.api.Plugin
@@ -39,7 +40,8 @@ class StaticAnalysisPlugin implements Plugin<Project> {
                 CheckstyleConfigurator.create(project, violationsContainer, evaluateViolations),
                 PmdConfigurator.create(project, violationsContainer, evaluateViolations),
                 FindbugsConfigurator.create(project, violationsContainer, evaluateViolations),
-                DetektConfigurator.create(project, violationsContainer, evaluateViolations)
+                DetektConfigurator.create(project, violationsContainer, evaluateViolations),
+                LintConfigurator.create(project, violationsContainer, evaluateViolations)
         ]
     }
 }

plugin/src/main/groovy/com/novoda/staticanalysis/internal/lint/CollectLintViolationsTask.groovy

@@ -0,0 +1,17 @@
+package com.novoda.staticanalysis.internal.lint
+
+import com.novoda.staticanalysis.internal.CollectViolationsTask
+import com.novoda.staticanalysis.internal.Violations
+import groovy.util.slurpersupport.GPathResult
+
+class CollectLintViolationsTask extends CollectViolationsTask {
+
+    @Override
+    void collectViolations(File xmlReportFile, File htmlReportFile, Violations violations) {
+        GPathResult xml = new XmlSlurper().parse(xmlReportFile)
+        int errors = xml.'**'.findAll { node -> node.name() == 'issue' && node.@severity == 'Error' }.size()
+        int warnings = xml.'**'.findAll { node -> node.name() == 'issue' && node.@severity == 'Warning' }.size()
+        violations.addViolations(errors, warnings, htmlReportFile ?: xmlReportFile)
+    }
+
+}

plugin/src/main/groovy/com/novoda/staticanalysis/internal/lint/LintConfigurator.groovy

@@ -0,0 +1,77 @@
+package com.novoda.staticanalysis.internal.lint
+
+import com.novoda.staticanalysis.StaticAnalysisExtension
+import com.novoda.staticanalysis.internal.Configurator
+import com.novoda.staticanalysis.internal.Violations
+import org.gradle.api.NamedDomainObjectContainer
+import org.gradle.api.Project
+import org.gradle.api.Task
+
+class LintConfigurator implements Configurator {
+
+    private final Project project
+    private final Violations violations
+    private final Task evaluateViolations
+
+    static LintConfigurator create(Project project,
+                                   NamedDomainObjectContainer<Violations> violationsContainer,
+                                   Task evaluateViolations) {
+        Violations violations = violationsContainer.maybeCreate('Lint')
+        return new LintConfigurator(project, violations, evaluateViolations)
+    }
+
+    private LintConfigurator(Project project, Violations violations, Task evaluateViolations) {
+        this.project = project
+        this.violations = violations
+        this.evaluateViolations = evaluateViolations
+    }
+
+    @Override
+    void execute() {
+        project.extensions.findByType(StaticAnalysisExtension).ext."lintOptions" = { Closure config ->
+            if (!isAndroidProject(project)) {
+                return
+            }
+            configureLint(config)
+            configureToolTask()
+        }
+    }
+
+    private void configureLint(Closure config) {
+        project.android.lintOptions(config)
+        project.android.lintOptions {
+            xmlReport = true
+            htmlReport = true
+            abortOnError false
+        }
+    }
+
+    private void configureToolTask() {
+        // evaluate violations after lint
+        def collectViolations = createCollectViolationsTask(violations)
+        evaluateViolations.dependsOn collectViolations
+        collectViolations.dependsOn project.tasks['lint']
+    }
+
+    private CollectLintViolationsTask createCollectViolationsTask(Violations violations) {
+        project.tasks.create('collectLintViolations', CollectLintViolationsTask) { collectViolations ->
+            collectViolations.xmlReportFile = xmlOutputFile
+            collectViolations.htmlReportFile = new File(defaultOutputFolder, 'lint-results.html')
+            collectViolations.violations = violations
+        }
+    }
+
+    private File getXmlOutputFile() {
+        project.android.lintOptions.xmlOutput ?: new File(defaultOutputFolder, 'lint-results.xml')
+    }
+
+    private File getDefaultOutputFolder() {
+        new File(project.buildDir, 'reports')
+    }
+
+    private static boolean isAndroidProject(final Project project) {
+        final boolean isAndroidApplication = project.plugins.hasPlugin('com.android.application')
+        final boolean isAndroidLibrary = project.plugins.hasPlugin('com.android.library')
+        return isAndroidApplication || isAndroidLibrary
+    }
+}

plugin/src/test/fixtures/reports/lint/lint-results.xml

@@ -0,0 +1,37 @@
+<?xml version="1.0" encoding="UTF-8"?>
+<issues format="4" by="lint 3.0.1">
+
+    <issue
+        id="MissingSuperCall"
+        severity="Error"
+        message="Overriding method should call `super.onDetachedFromWindow`"
+        category="Correctness"
+        priority="9"
+        summary="Missing Super Call"
+        explanation="Some methods, such as `View#onDetachedFromWindow`, require that you also call the super&#xA;implementation as part of your method."
+        errorLine1="    protected void onDetachedFromWindow() {"
+        errorLine2="                   ~~~~~~~~~~~~~~~~~~~~">
+        <location
+            file="/Users/tobiasheine/Documents/repos/gradle-static-analysis-plugin/plugin/src/test/fixtures/sources/lint/errors/MyView.java"
+            line="13"
+            column="20"/>
+    </issue>
+
+    <issue
+        id="Assert"
+        severity="Warning"
+        message="Assertions are unreliable in Dalvik and unimplemented in ART. Use `BuildConfig.DEBUG` conditional checks instead."
+        category="Correctness"
+        priority="6"
+        summary="Assertions"
+        explanation="Assertions are not checked at runtime. There are ways to request that they be used by Dalvik \&#xA;(`adb shell setprop debug.assert 1`), but note that this is not implemented in ART (the newer \&#xA;runtime), and even in Dalvik the property is ignored in many places and can not be relied upon. \&#xA;Instead, perform conditional checking inside `if (BuildConfig.DEBUG) { }` blocks. That constant \&#xA;is a static final boolean which is true in debug builds and false in release builds, and the \&#xA;Java compiler completely removes all code inside the if-body from the app.&#xA;&#xA;For example, you can replace `assert speed > 0` with `if (BuildConfig.DEBUG &amp;&amp; !(speed > 0)) { \&#xA;throw new AssertionError() }`.&#xA;&#xA;(Note: This lint check does not flag assertions purely asserting nullness or non-nullness; these \&#xA;are typically more intended for tools usage than runtime checks.)"
+        url="https://code.google.com/p/android/issues/detail?id=65183"
+        urls="https://code.google.com/p/android/issues/detail?id=65183"
+        errorLine1="        assert false;"
+        errorLine2="        ~~~~~~">
+        <location
+            file="/Users/tobiasheine/Documents/repos/gradle-static-analysis-plugin/plugin/src/test/fixtures/sources/lint/warnings/Warning.java"
+            line="6"
+            column="9"/>
+    </issue>
+</issues>

plugin/src/test/fixtures/rules/lint/lint.xml

@@ -0,0 +1,5 @@
+<?xml version="1.0" encoding="UTF-8"?>
+<lint>
+  <!-- Disable the given check in this project -->
+  <issue id="GradleDependency" severity="ignore" />
+</lint>

plugin/src/test/fixtures/sources/lint/errors/MyView.java

@@ -0,0 +1,14 @@
+import android.content.Context;
+import android.view.View;
+
+public class MyView extends View {
+
+    public MyView(Context context) {
+        super(context);
+    }
+
+    @Override
+    protected void onDetachedFromWindow() {
+        // missing super call
+    }
+}

plugin/src/test/fixtures/sources/lint/warnings/Warning.java

@@ -0,0 +1,7 @@
+public class Warning {
+
+    public Warning() {
+        // assertions are ignored at runtime
+        assert false;
+    }
+}

plugin/src/test/groovy/com/novoda/staticanalysis/internal/detekt/DetektIntegrationTest.groovy

@@ -1,10 +1,8 @@
 package com.novoda.staticanalysis.internal.detekt
 
-import com.novoda.test.DeployRulesTestRule
 import com.novoda.test.Fixtures
 import com.novoda.test.TestProject
 import com.novoda.test.TestProjectRule
-import org.junit.ClassRule
 import org.junit.Rule
 import org.junit.Test
 import org.junit.runner.RunWith
@@ -15,11 +13,6 @@ import static com.novoda.test.LogsSubject.assertThat
 @RunWith(Parameterized.class)
 class DetektIntegrationTest {
 
-    @ClassRule
-    public static final DeployRulesTestRule DEPLOY_RULES = new DeployRulesTestRule(
-            resourceDirs: [Fixtures.RULES_DIR],
-            repoDir: new File(Fixtures.BUILD_DIR, 'rules'))
-
     @Parameterized.Parameters(name = "{0}")
     static Iterable<TestProjectRule> rules() {
         return [TestProjectRule.forKotlinProject(), TestProjectRule.forAndroidKotlinProject()]

plugin/src/test/groovy/com/novoda/staticanalysis/internal/lint/CollectLintViolationsTaskTest.groovy

@@ -0,0 +1,24 @@
+package com.novoda.staticanalysis.internal.lint
+
+import com.novoda.staticanalysis.internal.Violations
+import com.novoda.test.Fixtures
+import org.gradle.api.Project
+import org.gradle.testfixtures.ProjectBuilder
+import org.junit.Test
+
+import static com.google.common.truth.Truth.assertThat
+
+class CollectLintViolationsTaskTest {
+
+    @Test
+    void shouldAddResultsToViolations() throws Exception {
+        Project project = ProjectBuilder.builder().build()
+        def task = project.task('collectLintViolationsTask', type: CollectLintViolationsTask)
+
+        Violations violations = new Violations('Android Lint')
+        task.collectViolations(Fixtures.Lint.SAMPLE_REPORT, null, violations)
+
+        assertThat(violations.getErrors()).isEqualTo(1)
+        assertThat(violations.getWarnings()).isEqualTo(1)
+    }
+}

plugin/src/test/groovy/com/novoda/staticanalysis/internal/lint/LintIntegrationTest.groovy

@@ -0,0 +1,62 @@
+package com.novoda.staticanalysis.internal.lint
+
+import com.novoda.test.Fixtures
+import com.novoda.test.TestAndroidProject
+import com.novoda.test.TestProject
+import org.junit.Test
+
+import static com.novoda.test.LogsSubject.assertThat
+
+class LintIntegrationTest {
+
+    private static GString LINT_CONFIGURATION =
+            """
+        lintOptions {              
+            lintConfig = file("${Fixtures.Lint.RULES}") 
+        }
+        """
+
+    @Test
+    void shouldFailBuildWhenLintErrorsOverTheThresholds() throws Exception {
+        def result = createAndroidProjectWith(Fixtures.Lint.SOURCES_WITH_ERRORS, 0, 0)
+                .buildAndFail('check')
+
+        assertThat(result.logs).containsLintViolations(1, 0, 'reports/lint-results.html')
+    }
+
+    @Test
+    void shouldNotFailBuildWhenLintErrorsWithinTheThresholds() throws Exception {
+        def result = createAndroidProjectWith(Fixtures.Lint.SOURCES_WITH_ERRORS, 0, 1)
+                .build('check')
+
+        assertThat(result.logs).doesNotContainLimitExceeded()
+    }
+
+    @Test
+    void shouldFailBuildWhenLintWarningsOverTheThresholds() throws Exception {
+        def result = createAndroidProjectWith(Fixtures.Lint.SOURCES_WITH_WARNINGS, 0, 0)
+                .buildAndFail('check')
+
+        assertThat(result.logs).containsLintViolations(0, 1, 'reports/lint-results.html')
+    }
+
+    @Test
+    void shouldNotFailBuildWhenLintWarningsWithinTheThresholds() throws Exception {
+        def result = createAndroidProjectWith(Fixtures.Lint.SOURCES_WITH_WARNINGS, 1, 0)
+                .build('check')
+
+        assertThat(result.logs).doesNotContainLimitExceeded()
+    }
+
+    private static TestProject createAndroidProjectWith(File sources, int maxWarnings = 0, int maxErrors = 0) {
+        def testProject = new TestAndroidProject()
+                .withSourceSet('main', sources)
+                .withPenalty("""{
+                    maxWarnings = ${maxWarnings}
+                    maxErrors = ${maxErrors}
+                }""")
+
+        testProject.withToolsConfig(LINT_CONFIGURATION)
+    }
+
+}

plugin/src/test/groovy/com/novoda/test/Fixtures.groovy

@@ -50,4 +50,11 @@ public final class Fixtures {
         public static final File RULES = new File(RULES_DIR, 'detekt/detekt.yml')
     }
 
+    final static class Lint {
+        public static final File SOURCES_WITH_WARNINGS = new File(SOURCES_DIR, 'lint/warnings')
+        public static final File SOURCES_WITH_ERRORS = new File(SOURCES_DIR, 'lint/errors')
+        public static final File SAMPLE_REPORT = new File(REPORTS_DIR, 'lint/lint-results.xml')
+        public static final File RULES = new File(RULES_DIR, 'lint/lint.xml')
+    }
+
 }

plugin/src/test/groovy/com/novoda/test/LogsSubject.groovy

@@ -19,6 +19,7 @@ class LogsSubject extends Subject<LogsSubject, Logs> {
     private static final String PMD_VIOLATIONS_FOUND = 'PMD violations found'
     private static final String FINDBUGS_VIOLATIONS_FOUND = 'Findbugs violations found'
     private static final String DETEKT_VIOLATIONS_FOUND = 'Detekt violations found'
+    private static final String LINT_VIOLATIONS_FOUND = 'Lint violations found'
 
     private static final SubjectFactory<LogsSubject, Logs> FACTORY = new SubjectFactory<LogsSubject, Logs>() {
         @Override
@@ -71,6 +72,10 @@ class LogsSubject extends Subject<LogsSubject, Logs> {
         outputSubject.doesNotContain(DETEKT_VIOLATIONS_FOUND)
     }
 
+    public void doesNotContainLintViolations() {
+        outputSubject.doesNotContain(LINT_VIOLATIONS_FOUND)
+    }
+
     public void containsCheckstyleViolations(int errors, int warnings, String... reportUrls) {
         containsToolViolations(CHECKSTYLE_VIOLATIONS_FOUND, errors, warnings, reportUrls)
     }
@@ -87,6 +92,10 @@ class LogsSubject extends Subject<LogsSubject, Logs> {
         containsToolViolations(DETEKT_VIOLATIONS_FOUND, errors, warnings, reportUrls)
     }
 
+    public void containsLintViolations(int errors, int warnings, String... reportUrls) {
+        containsToolViolations(LINT_VIOLATIONS_FOUND, errors, warnings, reportUrls)
+    }
+
     private void containsToolViolations(String template, int errors, int warnings, String... reportUrls) {
         outputSubject.contains("$template ($errors errors, $warnings warnings). See the reports at:\n")
         for (String reportUrl : reportUrls) {

plugin/src/test/groovy/com/novoda/test/TestAndroidProject.groovy

@@ -63,11 +63,6 @@ ${formatExtension(project)}
         .join('\n\t\t')
     }
 
-    @Override
-    List<String> defaultArguments() {
-        ['-x', 'lint'] + super.defaultArguments()
-    }
-
     TestAndroidProject withAdditionalAndroidConfig(String additionalAndroidConfig) {
         this.additionalAndroidConfig = additionalAndroidConfig
         return this