diff --git a/README.md b/README.md index 7274762..3f9828c 100644 --- a/README.md +++ b/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 { } } ``` diff --git a/docs/supported-tools.md b/docs/supported-tools.md index 30c51b0..ae74f43 100644 --- a/docs/supported-tools.md +++ b/docs/supported-tools.md @@ -9,8 +9,8 @@ Tool | Java | Android
(Java) | Kotlin | Android
(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) --- diff --git a/docs/tools/android_lint.md b/docs/tools/android_lint.md new file mode 100644 index 0000000..97f68f2 --- /dev/null +++ b/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. diff --git a/plugin/src/main/groovy/com/novoda/staticanalysis/StaticAnalysisPlugin.groovy b/plugin/src/main/groovy/com/novoda/staticanalysis/StaticAnalysisPlugin.groovy index 13582f8..d412e2a 100644 --- a/plugin/src/main/groovy/com/novoda/staticanalysis/StaticAnalysisPlugin.groovy +++ b/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 { 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) ] } } diff --git a/plugin/src/main/groovy/com/novoda/staticanalysis/internal/lint/CollectLintViolationsTask.groovy b/plugin/src/main/groovy/com/novoda/staticanalysis/internal/lint/CollectLintViolationsTask.groovy new file mode 100644 index 0000000..8f4dccb --- /dev/null +++ b/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) + } + +} diff --git a/plugin/src/main/groovy/com/novoda/staticanalysis/internal/lint/LintConfigurator.groovy b/plugin/src/main/groovy/com/novoda/staticanalysis/internal/lint/LintConfigurator.groovy new file mode 100644 index 0000000..cf58b35 --- /dev/null +++ b/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 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 + } +} diff --git a/plugin/src/test/fixtures/reports/lint/lint-results.xml b/plugin/src/test/fixtures/reports/lint/lint-results.xml new file mode 100644 index 0000000..f6e1641 --- /dev/null +++ b/plugin/src/test/fixtures/reports/lint/lint-results.xml @@ -0,0 +1,37 @@ + + + + + + + + + + + diff --git a/plugin/src/test/fixtures/rules/lint/lint.xml b/plugin/src/test/fixtures/rules/lint/lint.xml new file mode 100644 index 0000000..4c8fa53 --- /dev/null +++ b/plugin/src/test/fixtures/rules/lint/lint.xml @@ -0,0 +1,5 @@ + + + + + diff --git a/plugin/src/test/fixtures/sources/lint/errors/MyView.java b/plugin/src/test/fixtures/sources/lint/errors/MyView.java new file mode 100644 index 0000000..5bb1daa --- /dev/null +++ b/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 + } +} diff --git a/plugin/src/test/fixtures/sources/lint/warnings/Warning.java b/plugin/src/test/fixtures/sources/lint/warnings/Warning.java new file mode 100644 index 0000000..d30f308 --- /dev/null +++ b/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; + } +} diff --git a/plugin/src/test/groovy/com/novoda/staticanalysis/internal/detekt/DetektIntegrationTest.groovy b/plugin/src/test/groovy/com/novoda/staticanalysis/internal/detekt/DetektIntegrationTest.groovy index c96a595..0378f77 100644 --- a/plugin/src/test/groovy/com/novoda/staticanalysis/internal/detekt/DetektIntegrationTest.groovy +++ b/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 rules() { return [TestProjectRule.forKotlinProject(), TestProjectRule.forAndroidKotlinProject()] diff --git a/plugin/src/test/groovy/com/novoda/staticanalysis/internal/lint/CollectLintViolationsTaskTest.groovy b/plugin/src/test/groovy/com/novoda/staticanalysis/internal/lint/CollectLintViolationsTaskTest.groovy new file mode 100644 index 0000000..c390487 --- /dev/null +++ b/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) + } +} diff --git a/plugin/src/test/groovy/com/novoda/staticanalysis/internal/lint/LintIntegrationTest.groovy b/plugin/src/test/groovy/com/novoda/staticanalysis/internal/lint/LintIntegrationTest.groovy new file mode 100644 index 0000000..ecbd129 --- /dev/null +++ b/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) + } + +} diff --git a/plugin/src/test/groovy/com/novoda/test/Fixtures.groovy b/plugin/src/test/groovy/com/novoda/test/Fixtures.groovy index 18cff92..a583ed6 100644 --- a/plugin/src/test/groovy/com/novoda/test/Fixtures.groovy +++ b/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') + } + } diff --git a/plugin/src/test/groovy/com/novoda/test/LogsSubject.groovy b/plugin/src/test/groovy/com/novoda/test/LogsSubject.groovy index ed089b9..1abeb75 100644 --- a/plugin/src/test/groovy/com/novoda/test/LogsSubject.groovy +++ b/plugin/src/test/groovy/com/novoda/test/LogsSubject.groovy @@ -19,6 +19,7 @@ class LogsSubject extends Subject { 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 FACTORY = new SubjectFactory() { @Override @@ -71,6 +72,10 @@ class LogsSubject extends Subject { 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 { 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) { diff --git a/plugin/src/test/groovy/com/novoda/test/TestAndroidProject.groovy b/plugin/src/test/groovy/com/novoda/test/TestAndroidProject.groovy index 9de01e8..2c90a82 100644 --- a/plugin/src/test/groovy/com/novoda/test/TestAndroidProject.groovy +++ b/plugin/src/test/groovy/com/novoda/test/TestAndroidProject.groovy @@ -63,11 +63,6 @@ ${formatExtension(project)} .join('\n\t\t') } - @Override - List defaultArguments() { - ['-x', 'lint'] + super.defaultArguments() - } - TestAndroidProject withAdditionalAndroidConfig(String additionalAndroidConfig) { this.additionalAndroidConfig = additionalAndroidConfig return this