From 80a90bc32c85eb3df905c0eb36c97d9600322848 Mon Sep 17 00:00:00 2001 From: Tobias Heine Date: Tue, 30 Jan 2018 11:28:40 +0100 Subject: [PATCH 01/39] Add warning and error fixture for android lint --- .../test/fixtures/sources/lint/errors/MyView.java | 14 ++++++++++++++ .../fixtures/sources/lint/warnings/Warning.java | 7 +++++++ 2 files changed, 21 insertions(+) create mode 100644 plugin/src/test/fixtures/sources/lint/errors/MyView.java create mode 100644 plugin/src/test/fixtures/sources/lint/warnings/Warning.java 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; + } +} From 536d441ac7132a6090affa3564f62f12a81078c7 Mon Sep 17 00:00:00 2001 From: Tobias Heine Date: Tue, 30 Jan 2018 11:34:40 +0100 Subject: [PATCH 02/39] Remove not needed deploy test rule since we do not deploy the rules to maven in this test --- .../internal/detekt/DetektIntegrationTest.groovy | 7 ------- 1 file changed, 7 deletions(-) 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()] From ecfb0d8b4555575af239ea1fb1e7933dc42f3d30 Mon Sep 17 00:00:00 2001 From: Tobias Heine Date: Tue, 30 Jan 2018 11:43:17 +0100 Subject: [PATCH 03/39] Link lint fixtures --- plugin/src/test/groovy/com/novoda/test/Fixtures.groovy | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/plugin/src/test/groovy/com/novoda/test/Fixtures.groovy b/plugin/src/test/groovy/com/novoda/test/Fixtures.groovy index 18cff92..a7c8728 100644 --- a/plugin/src/test/groovy/com/novoda/test/Fixtures.groovy +++ b/plugin/src/test/groovy/com/novoda/test/Fixtures.groovy @@ -50,4 +50,9 @@ 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') + } + } From 431e1318158bc287e4e0af246f7e82f6ee001234 Mon Sep 17 00:00:00 2001 From: Tobias Heine Date: Tue, 30 Jan 2018 11:44:53 +0100 Subject: [PATCH 04/39] Verify the build should fail when lint warnings over threshold --- .../internal/lint/LintIntegrationTest.groovy | 55 +++++++++++++++++++ .../groovy/com/novoda/test/LogsSubject.groovy | 5 ++ 2 files changed, 60 insertions(+) create mode 100644 plugin/src/test/groovy/com/novoda/staticanalysis/internal/lint/LintIntegrationTest.groovy 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..b64e915 --- /dev/null +++ b/plugin/src/test/groovy/com/novoda/staticanalysis/internal/lint/LintIntegrationTest.groovy @@ -0,0 +1,55 @@ +package com.novoda.staticanalysis.internal.lint + +import com.novoda.test.Fixtures +import com.novoda.test.TestProject +import com.novoda.test.TestProjectRule +import org.junit.Rule +import org.junit.Test +import org.junit.runner.RunWith +import org.junit.runners.Parameterized + +import static com.novoda.test.LogsSubject.assertThat + +@RunWith(Parameterized.class) +class LintIntegrationTest { + + @Parameterized.Parameters(name = "{0}") + static Iterable rules() { + return [TestProjectRule.forAndroidProject()] + } + + @Rule + public final TestProjectRule projectRule + + LintIntegrationTest(TestProjectRule projectRule) { + this.projectRule = projectRule + } + + @Test + void shouldFailBuildWhenLintWarningsOverTheThreshold() throws Exception { + def result = createProjectWith(Fixtures.Lint.SOURCES_WITH_WARNINGS, 0, 0) + .buildAndFail('check') + + assertThat(result.logs).containsLintViolations(0, 1) + } + + private TestProject createProjectWith(File sources, int maxWarnings = 0, int maxErrors = 0) { + def testProject = projectRule.newProject() + .withSourceSet('main', sources) + .withPenalty("""{ + maxWarnings = ${maxWarnings} + maxErrors = ${maxErrors} + }""") + + testProject.withToolsConfig(lintConfiguration(testProject, sources)) + } + + private static GString lintConfiguration(TestProject testProject, File input) { + """ + lintOptions { + //lintConfig = file("${Fixtures.Detekt.RULES}") + xmlOutput = "${testProject.projectDir()}/build/reports" + } + """ + } +} diff --git a/plugin/src/test/groovy/com/novoda/test/LogsSubject.groovy b/plugin/src/test/groovy/com/novoda/test/LogsSubject.groovy index ed089b9..4d22901 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 @@ -87,6 +88,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) { From f1a44a90671c751f122f3e841ad064b055bc7f00 Mon Sep 17 00:00:00 2001 From: Tobias Heine Date: Tue, 30 Jan 2018 13:24:42 +0100 Subject: [PATCH 05/39] Remove not needed TestRule since we android lint just supports android projects. --- .../internal/lint/LintIntegrationTest.groovy | 24 ++++--------------- 1 file changed, 4 insertions(+), 20 deletions(-) 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 index b64e915..da0c619 100644 --- a/plugin/src/test/groovy/com/novoda/staticanalysis/internal/lint/LintIntegrationTest.groovy +++ b/plugin/src/test/groovy/com/novoda/staticanalysis/internal/lint/LintIntegrationTest.groovy @@ -1,40 +1,24 @@ package com.novoda.staticanalysis.internal.lint import com.novoda.test.Fixtures +import com.novoda.test.TestAndroidProject import com.novoda.test.TestProject -import com.novoda.test.TestProjectRule -import org.junit.Rule import org.junit.Test -import org.junit.runner.RunWith -import org.junit.runners.Parameterized import static com.novoda.test.LogsSubject.assertThat -@RunWith(Parameterized.class) class LintIntegrationTest { - @Parameterized.Parameters(name = "{0}") - static Iterable rules() { - return [TestProjectRule.forAndroidProject()] - } - - @Rule - public final TestProjectRule projectRule - - LintIntegrationTest(TestProjectRule projectRule) { - this.projectRule = projectRule - } - @Test void shouldFailBuildWhenLintWarningsOverTheThreshold() throws Exception { - def result = createProjectWith(Fixtures.Lint.SOURCES_WITH_WARNINGS, 0, 0) + def result = createAndroidProjectWith(Fixtures.Lint.SOURCES_WITH_WARNINGS, 0, 0) .buildAndFail('check') assertThat(result.logs).containsLintViolations(0, 1) } - private TestProject createProjectWith(File sources, int maxWarnings = 0, int maxErrors = 0) { - def testProject = projectRule.newProject() + private static TestProject createAndroidProjectWith(File sources, int maxWarnings = 0, int maxErrors = 0) { + def testProject = new TestAndroidProject() .withSourceSet('main', sources) .withPenalty("""{ maxWarnings = ${maxWarnings} From 83bb9e102b35871d03b5d33f59d270d5b80bf28b Mon Sep 17 00:00:00 2001 From: Tobias Heine Date: Tue, 30 Jan 2018 14:24:35 +0100 Subject: [PATCH 06/39] Add lint sample report fixture --- .../fixtures/reports/lint/lint-results.xml | 37 +++++++++++++++++++ .../groovy/com/novoda/test/Fixtures.groovy | 1 + 2 files changed, 38 insertions(+) create mode 100644 plugin/src/test/fixtures/reports/lint/lint-results.xml 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/groovy/com/novoda/test/Fixtures.groovy b/plugin/src/test/groovy/com/novoda/test/Fixtures.groovy index a7c8728..510f3ee 100644 --- a/plugin/src/test/groovy/com/novoda/test/Fixtures.groovy +++ b/plugin/src/test/groovy/com/novoda/test/Fixtures.groovy @@ -53,6 +53,7 @@ public final class Fixtures { 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') } } From cc50e0d6254482f5434beeae2dd496d24712976d Mon Sep 17 00:00:00 2001 From: Tobias Heine Date: Tue, 30 Jan 2018 14:41:37 +0100 Subject: [PATCH 07/39] Verify we collect lint violations from the lint sample report --- .../lint/CollectLintViolationsTask.groovy | 13 +++++++++++ .../lint/CollectLintViolationsTaskTest.groovy | 23 +++++++++++++++++++ 2 files changed, 36 insertions(+) create mode 100644 plugin/src/main/groovy/com/novoda/staticanalysis/internal/lint/CollectLintViolationsTask.groovy create mode 100644 plugin/src/test/groovy/com/novoda/staticanalysis/internal/lint/CollectLintViolationsTaskTest.groovy 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..7c29f51 --- /dev/null +++ b/plugin/src/main/groovy/com/novoda/staticanalysis/internal/lint/CollectLintViolationsTask.groovy @@ -0,0 +1,13 @@ +package com.novoda.staticanalysis.internal.lint + +import com.novoda.staticanalysis.internal.CollectViolationsTask +import com.novoda.staticanalysis.internal.Violations + +class CollectLintViolationsTask extends CollectViolationsTask { + + @Override + void collectViolations(File xmlReportFile, File htmlReportFile, Violations violations) { + //TODO: collect lint violations + } + +} 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..3509a05 --- /dev/null +++ b/plugin/src/test/groovy/com/novoda/staticanalysis/internal/lint/CollectLintViolationsTaskTest.groovy @@ -0,0 +1,23 @@ +package com.novoda.staticanalysis.internal.lint + +import com.google.common.truth.Truth +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 + +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) + + Truth.assertThat(violations.getErrors()).isEqualTo(1) + Truth.assertThat(violations.getWarnings()).isEqualTo(1) + } +} From 89db2710fd011d385b45f6b1d4e4d110d1ddc4d0 Mon Sep 17 00:00:00 2001 From: Tobias Heine Date: Tue, 30 Jan 2018 15:57:24 +0100 Subject: [PATCH 08/39] Configure lint via plugin extension --- .../StaticAnalysisPlugin.groovy | 4 +- .../internal/lint/LintConfigurator.groovy | 64 +++++++++++++++++++ 2 files changed, 67 insertions(+), 1 deletion(-) create mode 100644 plugin/src/main/groovy/com/novoda/staticanalysis/internal/lint/LintConfigurator.groovy 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/LintConfigurator.groovy b/plugin/src/main/groovy/com/novoda/staticanalysis/internal/lint/LintConfigurator.groovy new file mode 100644 index 0000000..63ff9af --- /dev/null +++ b/plugin/src/main/groovy/com/novoda/staticanalysis/internal/lint/LintConfigurator.groovy @@ -0,0 +1,64 @@ +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 + } + + project.extensions.findByName('android').lintOptions(config) + + configureToolTask() + } + } + + private void configureToolTask() { + // evaluate violations after lint + def collectViolations = createCollectViolationsTask(violations) + evaluateViolations.dependsOn collectViolations + collectViolations.dependsOn project.tasks['lint'] + } + + private CollectLintViolationsTask createCollectViolationsTask(Violations violations) { + def outputFolder = project.file(project.extensions.findByName('android').lintOptions.xmlOutput.parent) + project.tasks.create("collectLintViolations", CollectLintViolationsTask) { collectViolations -> + collectViolations.xmlReportFile = new File(outputFolder, 'lint-report.xml') + collectViolations.htmlReportFile = new File(outputFolder, 'lint-report.html') + collectViolations.violations = violations + } + } + + 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 + } +} From 6532dd6dc96b28d4ac710ae0ebead7c4a584368b Mon Sep 17 00:00:00 2001 From: Tobias Heine Date: Tue, 30 Jan 2018 16:17:50 +0100 Subject: [PATCH 09/39] Do not exclude lint for android projects --- .../test/groovy/com/novoda/test/TestAndroidProject.groovy | 5 ----- 1 file changed, 5 deletions(-) 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 From cb13b2b38c7d15d75e07714ce8482e7b66fbd00d Mon Sep 17 00:00:00 2001 From: Tobias Heine Date: Tue, 30 Jan 2018 16:18:26 +0100 Subject: [PATCH 10/39] Lint expects link to file instead of path as string --- .../staticanalysis/internal/lint/LintIntegrationTest.groovy | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) 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 index da0c619..5f14449 100644 --- a/plugin/src/test/groovy/com/novoda/staticanalysis/internal/lint/LintIntegrationTest.groovy +++ b/plugin/src/test/groovy/com/novoda/staticanalysis/internal/lint/LintIntegrationTest.groovy @@ -32,7 +32,7 @@ class LintIntegrationTest { """ lintOptions { //lintConfig = file("${Fixtures.Detekt.RULES}") - xmlOutput = "${testProject.projectDir()}/build/reports" + xmlOutput = file("${testProject.projectDir()}/build/reports") } """ } From be9727326938f6f8994c59dd549afb4a3757b220 Mon Sep 17 00:00:00 2001 From: Tobias Heine Date: Tue, 30 Jan 2018 16:22:50 +0100 Subject: [PATCH 11/39] Bump build tool version to suppress unwanted lint warning --- .../src/test/groovy/com/novoda/test/TestAndroidProject.groovy | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/plugin/src/test/groovy/com/novoda/test/TestAndroidProject.groovy b/plugin/src/test/groovy/com/novoda/test/TestAndroidProject.groovy index 2c90a82..5b29530 100644 --- a/plugin/src/test/groovy/com/novoda/test/TestAndroidProject.groovy +++ b/plugin/src/test/groovy/com/novoda/test/TestAndroidProject.groovy @@ -22,7 +22,7 @@ repositories { apply plugin: 'com.android.library' android { compileSdkVersion 27 - buildToolsVersion '27.0.0' + buildToolsVersion '27.0.3' defaultConfig { minSdkVersion 16 From b5ce20a7bc341d99477c24f4acbdd8f1f17c42ef Mon Sep 17 00:00:00 2001 From: Tobias Heine Date: Tue, 30 Jan 2018 16:24:10 +0100 Subject: [PATCH 12/39] Resolve report directory manually --- .../staticanalysis/internal/lint/LintConfigurator.groovy | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) 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 index 63ff9af..f20dcf8 100644 --- a/plugin/src/main/groovy/com/novoda/staticanalysis/internal/lint/LintConfigurator.groovy +++ b/plugin/src/main/groovy/com/novoda/staticanalysis/internal/lint/LintConfigurator.groovy @@ -48,7 +48,8 @@ class LintConfigurator implements Configurator { } private CollectLintViolationsTask createCollectViolationsTask(Violations violations) { - def outputFolder = project.file(project.extensions.findByName('android').lintOptions.xmlOutput.parent) +// def outputFolder = project.file(project.extensions.findByName('android').lintOptions.xmlOutput.parent) + def outputFolder = new File(project.projectDir, 'build/reports') project.tasks.create("collectLintViolations", CollectLintViolationsTask) { collectViolations -> collectViolations.xmlReportFile = new File(outputFolder, 'lint-report.xml') collectViolations.htmlReportFile = new File(outputFolder, 'lint-report.html') From 495897b33fb4850e6aa7f156713062017a1bce73 Mon Sep 17 00:00:00 2001 From: Tobias Heine Date: Tue, 30 Jan 2018 16:28:15 +0100 Subject: [PATCH 13/39] Apply lint test rule --- plugin/src/test/fixtures/rules/lint/lint.xml | 19 +++++++++++++++++++ .../internal/lint/LintIntegrationTest.groovy | 4 ++-- .../groovy/com/novoda/test/Fixtures.groovy | 1 + 3 files changed, 22 insertions(+), 2 deletions(-) create mode 100644 plugin/src/test/fixtures/rules/lint/lint.xml 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..471535e --- /dev/null +++ b/plugin/src/test/fixtures/rules/lint/lint.xml @@ -0,0 +1,19 @@ + + + + + + + + + + + + + + + + + + + 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 index 5f14449..5dd4d00 100644 --- a/plugin/src/test/groovy/com/novoda/staticanalysis/internal/lint/LintIntegrationTest.groovy +++ b/plugin/src/test/groovy/com/novoda/staticanalysis/internal/lint/LintIntegrationTest.groovy @@ -31,8 +31,8 @@ class LintIntegrationTest { private static GString lintConfiguration(TestProject testProject, File input) { """ lintOptions { - //lintConfig = file("${Fixtures.Detekt.RULES}") - xmlOutput = file("${testProject.projectDir()}/build/reports") + lintConfig = file("${Fixtures.Lint.RULES}") + //xmlOutput = file("${testProject.projectDir()}/build/reports") } """ } diff --git a/plugin/src/test/groovy/com/novoda/test/Fixtures.groovy b/plugin/src/test/groovy/com/novoda/test/Fixtures.groovy index 510f3ee..a583ed6 100644 --- a/plugin/src/test/groovy/com/novoda/test/Fixtures.groovy +++ b/plugin/src/test/groovy/com/novoda/test/Fixtures.groovy @@ -54,6 +54,7 @@ public final class Fixtures { 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') } } From 98d9c0490ad7f7400f36d89fa20179a5fa88cbcf Mon Sep 17 00:00:00 2001 From: Tobias Heine Date: Tue, 30 Jan 2018 16:28:25 +0100 Subject: [PATCH 14/39] Revert "Bump build tool version to suppress unwanted lint warning" This reverts commit 6a1a943ac775fa06c0bbbd7f05e2b8d0962e66cb. --- .../src/test/groovy/com/novoda/test/TestAndroidProject.groovy | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/plugin/src/test/groovy/com/novoda/test/TestAndroidProject.groovy b/plugin/src/test/groovy/com/novoda/test/TestAndroidProject.groovy index 5b29530..2c90a82 100644 --- a/plugin/src/test/groovy/com/novoda/test/TestAndroidProject.groovy +++ b/plugin/src/test/groovy/com/novoda/test/TestAndroidProject.groovy @@ -22,7 +22,7 @@ repositories { apply plugin: 'com.android.library' android { compileSdkVersion 27 - buildToolsVersion '27.0.3' + buildToolsVersion '27.0.0' defaultConfig { minSdkVersion 16 From 19ed465cbe23d19ef7a5acc2ba1ea19c0ea202ae Mon Sep 17 00:00:00 2001 From: Tobias Heine Date: Tue, 30 Jan 2018 16:31:01 +0100 Subject: [PATCH 15/39] Ignore GradleDependency warning to keep the test stable --- plugin/src/test/fixtures/rules/lint/lint.xml | 16 +--------------- .../internal/lint/LintIntegrationTest.groovy | 5 ++--- 2 files changed, 3 insertions(+), 18 deletions(-) diff --git a/plugin/src/test/fixtures/rules/lint/lint.xml b/plugin/src/test/fixtures/rules/lint/lint.xml index 471535e..4c8fa53 100644 --- a/plugin/src/test/fixtures/rules/lint/lint.xml +++ b/plugin/src/test/fixtures/rules/lint/lint.xml @@ -1,19 +1,5 @@ - - - - - - - - - - - - - - - + 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 index 5dd4d00..8792936 100644 --- a/plugin/src/test/groovy/com/novoda/staticanalysis/internal/lint/LintIntegrationTest.groovy +++ b/plugin/src/test/groovy/com/novoda/staticanalysis/internal/lint/LintIntegrationTest.groovy @@ -25,14 +25,13 @@ class LintIntegrationTest { maxErrors = ${maxErrors} }""") - testProject.withToolsConfig(lintConfiguration(testProject, sources)) + testProject.withToolsConfig(lintConfiguration()) } - private static GString lintConfiguration(TestProject testProject, File input) { + private static GString lintConfiguration() { """ lintOptions { lintConfig = file("${Fixtures.Lint.RULES}") - //xmlOutput = file("${testProject.projectDir()}/build/reports") } """ } From 0f742bdeee115d7e0efccba79a8e084648143ec9 Mon Sep 17 00:00:00 2001 From: Tobias Heine Date: Tue, 30 Jan 2018 16:33:29 +0100 Subject: [PATCH 16/39] Remove not needed comment --- .../novoda/staticanalysis/internal/lint/LintConfigurator.groovy | 1 - 1 file changed, 1 deletion(-) 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 index f20dcf8..20073fc 100644 --- a/plugin/src/main/groovy/com/novoda/staticanalysis/internal/lint/LintConfigurator.groovy +++ b/plugin/src/main/groovy/com/novoda/staticanalysis/internal/lint/LintConfigurator.groovy @@ -48,7 +48,6 @@ class LintConfigurator implements Configurator { } private CollectLintViolationsTask createCollectViolationsTask(Violations violations) { -// def outputFolder = project.file(project.extensions.findByName('android').lintOptions.xmlOutput.parent) def outputFolder = new File(project.projectDir, 'build/reports') project.tasks.create("collectLintViolations", CollectLintViolationsTask) { collectViolations -> collectViolations.xmlReportFile = new File(outputFolder, 'lint-report.xml') From f98172339449b6b980fcf0c61f785c82ca5e4cfe Mon Sep 17 00:00:00 2001 From: Tobias Heine Date: Tue, 30 Jan 2018 16:52:40 +0100 Subject: [PATCH 17/39] Fix typo --- .../staticanalysis/internal/lint/LintConfigurator.groovy | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) 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 index 20073fc..df8492e 100644 --- a/plugin/src/main/groovy/com/novoda/staticanalysis/internal/lint/LintConfigurator.groovy +++ b/plugin/src/main/groovy/com/novoda/staticanalysis/internal/lint/LintConfigurator.groovy @@ -50,8 +50,8 @@ class LintConfigurator implements Configurator { private CollectLintViolationsTask createCollectViolationsTask(Violations violations) { def outputFolder = new File(project.projectDir, 'build/reports') project.tasks.create("collectLintViolations", CollectLintViolationsTask) { collectViolations -> - collectViolations.xmlReportFile = new File(outputFolder, 'lint-report.xml') - collectViolations.htmlReportFile = new File(outputFolder, 'lint-report.html') + collectViolations.xmlReportFile = new File(outputFolder, 'lint-results.xml') + collectViolations.htmlReportFile = new File(outputFolder, 'lint-results.html') collectViolations.violations = violations } } From 85cea377c93af8810c7f19095155ef0b4b64449f Mon Sep 17 00:00:00 2001 From: Tobias Heine Date: Tue, 30 Jan 2018 17:00:18 +0100 Subject: [PATCH 18/39] Parse lint xml result and append errors and warnings to Violations --- .../internal/lint/CollectLintViolationsTask.groovy | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) 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 index 7c29f51..8f4dccb 100644 --- a/plugin/src/main/groovy/com/novoda/staticanalysis/internal/lint/CollectLintViolationsTask.groovy +++ b/plugin/src/main/groovy/com/novoda/staticanalysis/internal/lint/CollectLintViolationsTask.groovy @@ -2,12 +2,16 @@ 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) { - //TODO: collect lint 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) } } From a43b9e082e4901370cb1ace519273cafb59f0293 Mon Sep 17 00:00:00 2001 From: Tobias Heine Date: Tue, 30 Jan 2018 17:01:31 +0100 Subject: [PATCH 19/39] Add static import for Truth.assertThat --- .../internal/lint/CollectLintViolationsTaskTest.groovy | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) 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 index 3509a05..03d2d5e 100644 --- a/plugin/src/test/groovy/com/novoda/staticanalysis/internal/lint/CollectLintViolationsTaskTest.groovy +++ b/plugin/src/test/groovy/com/novoda/staticanalysis/internal/lint/CollectLintViolationsTaskTest.groovy @@ -1,12 +1,13 @@ package com.novoda.staticanalysis.internal.lint -import com.google.common.truth.Truth 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 @@ -17,7 +18,7 @@ class CollectLintViolationsTaskTest { Violations violations = new Violations("Android Lint") task.collectViolations(Fixtures.Lint.SAMPLE_REPORT, null, violations) - Truth.assertThat(violations.getErrors()).isEqualTo(1) - Truth.assertThat(violations.getWarnings()).isEqualTo(1) + assertThat(violations.getErrors()).isEqualTo(1) + assertThat(violations.getWarnings()).isEqualTo(1) } } From 9a9786494978a8f8b4914b3a63fd14ac6035c377 Mon Sep 17 00:00:00 2001 From: Tobias Heine Date: Tue, 30 Jan 2018 17:08:36 +0100 Subject: [PATCH 20/39] Verify the build passes when warnings within thresholds --- .../internal/lint/LintIntegrationTest.groovy | 8 ++++++++ plugin/src/test/groovy/com/novoda/test/LogsSubject.groovy | 4 ++++ 2 files changed, 12 insertions(+) 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 index 8792936..0cfed22 100644 --- a/plugin/src/test/groovy/com/novoda/staticanalysis/internal/lint/LintIntegrationTest.groovy +++ b/plugin/src/test/groovy/com/novoda/staticanalysis/internal/lint/LintIntegrationTest.groovy @@ -17,6 +17,14 @@ class LintIntegrationTest { assertThat(result.logs).containsLintViolations(0, 1) } + @Test + void shouldNotFailBuildWhenLintWarningsWithinTheThreshold() 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) diff --git a/plugin/src/test/groovy/com/novoda/test/LogsSubject.groovy b/plugin/src/test/groovy/com/novoda/test/LogsSubject.groovy index 4d22901..1abeb75 100644 --- a/plugin/src/test/groovy/com/novoda/test/LogsSubject.groovy +++ b/plugin/src/test/groovy/com/novoda/test/LogsSubject.groovy @@ -72,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) } From da00fed5457c8103c389b136710031936122cb06 Mon Sep 17 00:00:00 2001 From: Tobias Heine Date: Tue, 30 Jan 2018 17:11:08 +0100 Subject: [PATCH 21/39] Verify the html report is linked in the log --- .../staticanalysis/internal/lint/LintIntegrationTest.groovy | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) 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 index 0cfed22..3d8fb14 100644 --- a/plugin/src/test/groovy/com/novoda/staticanalysis/internal/lint/LintIntegrationTest.groovy +++ b/plugin/src/test/groovy/com/novoda/staticanalysis/internal/lint/LintIntegrationTest.groovy @@ -14,7 +14,7 @@ class LintIntegrationTest { def result = createAndroidProjectWith(Fixtures.Lint.SOURCES_WITH_WARNINGS, 0, 0) .buildAndFail('check') - assertThat(result.logs).containsLintViolations(0, 1) + assertThat(result.logs).containsLintViolations(0, 1, 'reports/lint-results.html') } @Test From b2c0f72a6003ec9a9f0d3b968858721ee7a5300f Mon Sep 17 00:00:00 2001 From: Tobias Heine Date: Tue, 30 Jan 2018 17:13:15 +0100 Subject: [PATCH 22/39] Verify the build fails when errors over thresholds --- .../internal/lint/LintIntegrationTest.groovy | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) 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 index 3d8fb14..8d05a42 100644 --- a/plugin/src/test/groovy/com/novoda/staticanalysis/internal/lint/LintIntegrationTest.groovy +++ b/plugin/src/test/groovy/com/novoda/staticanalysis/internal/lint/LintIntegrationTest.groovy @@ -9,6 +9,14 @@ import static com.novoda.test.LogsSubject.assertThat class LintIntegrationTest { + @Test + void shouldFailBuildWhenLintErrorsOverTheThreshold() 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 shouldFailBuildWhenLintWarningsOverTheThreshold() throws Exception { def result = createAndroidProjectWith(Fixtures.Lint.SOURCES_WITH_WARNINGS, 0, 0) @@ -38,7 +46,8 @@ class LintIntegrationTest { private static GString lintConfiguration() { """ - lintOptions { + lintOptions { + abortOnError false lintConfig = file("${Fixtures.Lint.RULES}") } """ From 92f90d9df26739eeb0e892bc95d1ecfe3675b7ec Mon Sep 17 00:00:00 2001 From: Tobias Heine Date: Tue, 30 Jan 2018 17:15:29 +0100 Subject: [PATCH 23/39] Verify the build does not fail when erros within thresholds --- .../internal/lint/LintIntegrationTest.groovy | 14 +++++++++++--- 1 file changed, 11 insertions(+), 3 deletions(-) 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 index 8d05a42..1057c40 100644 --- a/plugin/src/test/groovy/com/novoda/staticanalysis/internal/lint/LintIntegrationTest.groovy +++ b/plugin/src/test/groovy/com/novoda/staticanalysis/internal/lint/LintIntegrationTest.groovy @@ -10,7 +10,7 @@ import static com.novoda.test.LogsSubject.assertThat class LintIntegrationTest { @Test - void shouldFailBuildWhenLintErrorsOverTheThreshold() throws Exception { + void shouldFailBuildWhenLintErrorsOverTheThresholds() throws Exception { def result = createAndroidProjectWith(Fixtures.Lint.SOURCES_WITH_ERRORS, 0, 0) .buildAndFail('check') @@ -18,7 +18,15 @@ class LintIntegrationTest { } @Test - void shouldFailBuildWhenLintWarningsOverTheThreshold() throws Exception { + 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') @@ -26,7 +34,7 @@ class LintIntegrationTest { } @Test - void shouldNotFailBuildWhenLintWarningsWithinTheThreshold() throws Exception { + void shouldNotFailBuildWhenLintWarningsWithinTheThresholds() throws Exception { def result = createAndroidProjectWith(Fixtures.Lint.SOURCES_WITH_WARNINGS, 1, 0) .build('check') From f99d0bae851ce23b677c60a6f9fb1938f6a2fd0f Mon Sep 17 00:00:00 2001 From: Tobias Heine Date: Tue, 30 Jan 2018 17:53:41 +0100 Subject: [PATCH 24/39] Simplify lint configuration --- .../novoda/staticanalysis/internal/lint/LintConfigurator.groovy | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) 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 index df8492e..543b43f 100644 --- a/plugin/src/main/groovy/com/novoda/staticanalysis/internal/lint/LintConfigurator.groovy +++ b/plugin/src/main/groovy/com/novoda/staticanalysis/internal/lint/LintConfigurator.groovy @@ -34,7 +34,7 @@ class LintConfigurator implements Configurator { return } - project.extensions.findByName('android').lintOptions(config) + project.android.lintOptions(config) configureToolTask() } From 7f0e764ba4dd6c8951a8210bc1a351ad0f21f4e5 Mon Sep 17 00:00:00 2001 From: Tobias Heine Date: Tue, 30 Jan 2018 17:57:24 +0100 Subject: [PATCH 25/39] Remove empty line --- .../novoda/staticanalysis/internal/lint/LintConfigurator.groovy | 1 - 1 file changed, 1 deletion(-) 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 index 543b43f..281af9d 100644 --- a/plugin/src/main/groovy/com/novoda/staticanalysis/internal/lint/LintConfigurator.groovy +++ b/plugin/src/main/groovy/com/novoda/staticanalysis/internal/lint/LintConfigurator.groovy @@ -29,7 +29,6 @@ class LintConfigurator implements Configurator { @Override void execute() { project.extensions.findByType(StaticAnalysisExtension).ext."lintOptions" = { Closure config -> - if (!isAndroidProject(project)) { return } From 7bd712b4632776cd3ec5dbf1aa87795b5ace982d Mon Sep 17 00:00:00 2001 From: Tobias Heine Date: Tue, 30 Jan 2018 17:57:53 +0100 Subject: [PATCH 26/39] Use single quotes --- .../novoda/staticanalysis/internal/lint/LintConfigurator.groovy | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) 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 index 281af9d..8bf3a14 100644 --- a/plugin/src/main/groovy/com/novoda/staticanalysis/internal/lint/LintConfigurator.groovy +++ b/plugin/src/main/groovy/com/novoda/staticanalysis/internal/lint/LintConfigurator.groovy @@ -48,7 +48,7 @@ class LintConfigurator implements Configurator { private CollectLintViolationsTask createCollectViolationsTask(Violations violations) { def outputFolder = new File(project.projectDir, 'build/reports') - project.tasks.create("collectLintViolations", CollectLintViolationsTask) { collectViolations -> + project.tasks.create('collectLintViolations', CollectLintViolationsTask) { collectViolations -> collectViolations.xmlReportFile = new File(outputFolder, 'lint-results.xml') collectViolations.htmlReportFile = new File(outputFolder, 'lint-results.html') collectViolations.violations = violations From d10f0d3f64d240c2a4bddefa47d284d0e05909ed Mon Sep 17 00:00:00 2001 From: Tobias Heine Date: Tue, 30 Jan 2018 18:00:27 +0100 Subject: [PATCH 27/39] Use single quoted string --- .../internal/lint/CollectLintViolationsTaskTest.groovy | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) 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 index 03d2d5e..c390487 100644 --- a/plugin/src/test/groovy/com/novoda/staticanalysis/internal/lint/CollectLintViolationsTaskTest.groovy +++ b/plugin/src/test/groovy/com/novoda/staticanalysis/internal/lint/CollectLintViolationsTaskTest.groovy @@ -15,7 +15,7 @@ class CollectLintViolationsTaskTest { Project project = ProjectBuilder.builder().build() def task = project.task('collectLintViolationsTask', type: CollectLintViolationsTask) - Violations violations = new Violations("Android Lint") + Violations violations = new Violations('Android Lint') task.collectViolations(Fixtures.Lint.SAMPLE_REPORT, null, violations) assertThat(violations.getErrors()).isEqualTo(1) From 73028015e86cbdcd2ac24b18f4edf220144bb45f Mon Sep 17 00:00:00 2001 From: Tobias Heine Date: Tue, 30 Jan 2018 18:02:49 +0100 Subject: [PATCH 28/39] Extract constant for lint configuration --- .../internal/lint/LintIntegrationTest.groovy | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) 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 index 1057c40..cdf3cbe 100644 --- a/plugin/src/test/groovy/com/novoda/staticanalysis/internal/lint/LintIntegrationTest.groovy +++ b/plugin/src/test/groovy/com/novoda/staticanalysis/internal/lint/LintIntegrationTest.groovy @@ -9,6 +9,14 @@ import static com.novoda.test.LogsSubject.assertThat class LintIntegrationTest { + private static GString LINT_CONFIGURATION = + """ + lintOptions { + abortOnError false + lintConfig = file("${Fixtures.Lint.RULES}") + } + """ + @Test void shouldFailBuildWhenLintErrorsOverTheThresholds() throws Exception { def result = createAndroidProjectWith(Fixtures.Lint.SOURCES_WITH_ERRORS, 0, 0) @@ -49,15 +57,7 @@ class LintIntegrationTest { maxErrors = ${maxErrors} }""") - testProject.withToolsConfig(lintConfiguration()) + testProject.withToolsConfig(LINT_CONFIGURATION) } - private static GString lintConfiguration() { - """ - lintOptions { - abortOnError false - lintConfig = file("${Fixtures.Lint.RULES}") - } - """ - } } From db6bb78d64a842863571ce7b844036a51a8e112f Mon Sep 17 00:00:00 2001 From: Tobias Heine Date: Wed, 31 Jan 2018 10:03:57 +0100 Subject: [PATCH 29/39] Force xml report and not to fail on any error --- .../internal/lint/LintConfigurator.groovy | 12 +++++++++--- .../internal/lint/LintIntegrationTest.groovy | 1 - 2 files changed, 9 insertions(+), 4 deletions(-) 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 index 8bf3a14..63d59aa 100644 --- a/plugin/src/main/groovy/com/novoda/staticanalysis/internal/lint/LintConfigurator.groovy +++ b/plugin/src/main/groovy/com/novoda/staticanalysis/internal/lint/LintConfigurator.groovy @@ -32,13 +32,19 @@ class LintConfigurator implements Configurator { if (!isAndroidProject(project)) { return } - - project.android.lintOptions(config) - + configureLint(config) configureToolTask() } } + private void configureLint(Closure config) { + project.android.lintOptions(config) + project.android.lintOptions { + xmlReport = true + abortOnError false + } + } + private void configureToolTask() { // evaluate violations after lint def collectViolations = createCollectViolationsTask(violations) 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 index cdf3cbe..ecbd129 100644 --- a/plugin/src/test/groovy/com/novoda/staticanalysis/internal/lint/LintIntegrationTest.groovy +++ b/plugin/src/test/groovy/com/novoda/staticanalysis/internal/lint/LintIntegrationTest.groovy @@ -12,7 +12,6 @@ class LintIntegrationTest { private static GString LINT_CONFIGURATION = """ lintOptions { - abortOnError false lintConfig = file("${Fixtures.Lint.RULES}") } """ From c0846221db367ad1697063ea6bf99187f8c69fd8 Mon Sep 17 00:00:00 2001 From: Tobias Heine Date: Wed, 31 Jan 2018 10:17:57 +0100 Subject: [PATCH 30/39] Force html report since we link it --- .../novoda/staticanalysis/internal/lint/LintConfigurator.groovy | 1 + 1 file changed, 1 insertion(+) 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 index 63d59aa..01dc640 100644 --- a/plugin/src/main/groovy/com/novoda/staticanalysis/internal/lint/LintConfigurator.groovy +++ b/plugin/src/main/groovy/com/novoda/staticanalysis/internal/lint/LintConfigurator.groovy @@ -41,6 +41,7 @@ class LintConfigurator implements Configurator { project.android.lintOptions(config) project.android.lintOptions { xmlReport = true + htmlReport = true abortOnError false } } From ef87810c27e344e519d5b243c63e8a31e3c0c930 Mon Sep 17 00:00:00 2001 From: Tobias Heine Date: Wed, 31 Jan 2018 10:34:45 +0100 Subject: [PATCH 31/39] Use configured xml output file or fall back to hardcoded one --- .../internal/lint/LintConfigurator.groovy | 13 ++++++++++--- 1 file changed, 10 insertions(+), 3 deletions(-) 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 index 01dc640..250a82a 100644 --- a/plugin/src/main/groovy/com/novoda/staticanalysis/internal/lint/LintConfigurator.groovy +++ b/plugin/src/main/groovy/com/novoda/staticanalysis/internal/lint/LintConfigurator.groovy @@ -54,14 +54,21 @@ class LintConfigurator implements Configurator { } private CollectLintViolationsTask createCollectViolationsTask(Violations violations) { - def outputFolder = new File(project.projectDir, 'build/reports') project.tasks.create('collectLintViolations', CollectLintViolationsTask) { collectViolations -> - collectViolations.xmlReportFile = new File(outputFolder, 'lint-results.xml') - collectViolations.htmlReportFile = new File(outputFolder, 'lint-results.html') + collectViolations.xmlReportFile = xmlOutputFile() + collectViolations.htmlReportFile = new File(defaultOutputFolder(), 'lint-results.html') collectViolations.violations = violations } } + private File xmlOutputFile() { + project.android.lintOptions.xmlOutput ?: new File(defaultOutputFolder(), 'lint-results.xml') + } + + private File defaultOutputFolder() { + new File(project.projectDir, 'build/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') From 141164318fdf2e78fbeb4f9a0bb587ac7367573f Mon Sep 17 00:00:00 2001 From: Tobias Heine Date: Wed, 31 Jan 2018 11:30:57 +0100 Subject: [PATCH 32/39] Use a getter --- .../internal/lint/LintConfigurator.groovy | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) 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 index 250a82a..b4ad545 100644 --- a/plugin/src/main/groovy/com/novoda/staticanalysis/internal/lint/LintConfigurator.groovy +++ b/plugin/src/main/groovy/com/novoda/staticanalysis/internal/lint/LintConfigurator.groovy @@ -55,17 +55,17 @@ class LintConfigurator implements Configurator { private CollectLintViolationsTask createCollectViolationsTask(Violations violations) { project.tasks.create('collectLintViolations', CollectLintViolationsTask) { collectViolations -> - collectViolations.xmlReportFile = xmlOutputFile() - collectViolations.htmlReportFile = new File(defaultOutputFolder(), 'lint-results.html') + collectViolations.xmlReportFile = xmlOutputFile + collectViolations.htmlReportFile = new File(defaultOutputFolder, 'lint-results.html') collectViolations.violations = violations } } - private File xmlOutputFile() { - project.android.lintOptions.xmlOutput ?: new File(defaultOutputFolder(), 'lint-results.xml') + private File getXmlOutputFile() { + project.android.lintOptions.xmlOutput ?: new File(defaultOutputFolder, 'lint-results.xml') } - private File defaultOutputFolder() { + private File getDefaultOutputFolder() { new File(project.projectDir, 'build/reports') } From a3ef3195e2a052e3b4ebfdb3be1470be5dd5c7d2 Mon Sep 17 00:00:00 2001 From: Tobias Heine Date: Wed, 31 Jan 2018 11:31:32 +0100 Subject: [PATCH 33/39] Use project.buildDir rather than building it manually --- .../novoda/staticanalysis/internal/lint/LintConfigurator.groovy | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) 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 index b4ad545..cf58b35 100644 --- a/plugin/src/main/groovy/com/novoda/staticanalysis/internal/lint/LintConfigurator.groovy +++ b/plugin/src/main/groovy/com/novoda/staticanalysis/internal/lint/LintConfigurator.groovy @@ -66,7 +66,7 @@ class LintConfigurator implements Configurator { } private File getDefaultOutputFolder() { - new File(project.projectDir, 'build/reports') + new File(project.buildDir, 'reports') } private static boolean isAndroidProject(final Project project) { From 2fcb248168b19127ef2a36531ac8bc023b749d97 Mon Sep 17 00:00:00 2001 From: Tobias Heine Date: Wed, 31 Jan 2018 16:41:57 +0100 Subject: [PATCH 34/39] Add lint configuration to readme --- README.md | 5 +++-- docs/supported-tools.md | 4 ++-- docs/tools/android_lint.md | 7 +++++++ 3 files changed, 12 insertions(+), 4 deletions(-) create mode 100644 docs/tools/android_lint.md 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..1bc2486 --- /dev/null +++ b/docs/tools/android_lint.md @@ -0,0 +1,7 @@ +# Android Lint +[Android Lint](https://developer.android.com/studio/write/lint.html) is a code scanning tool with focus on Android. Besides code it also analyses resources +and configuration files. + +## 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 hardcoded so lint won't break the build by it's own and always generates reports. From d3c7246b3e2a15919f3daf33d0b23899f3118c0a Mon Sep 17 00:00:00 2001 From: Tobias Heine Date: Wed, 31 Jan 2018 17:27:59 +0100 Subject: [PATCH 35/39] Tweak android lint readme --- docs/tools/android_lint.md | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/docs/tools/android_lint.md b/docs/tools/android_lint.md index 1bc2486..6e06b3e 100644 --- a/docs/tools/android_lint.md +++ b/docs/tools/android_lint.md @@ -1,7 +1,7 @@ # Android Lint -[Android Lint](https://developer.android.com/studio/write/lint.html) is a code scanning tool with focus on Android. Besides code it also analyses resources -and configuration files. +[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. ## 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 hardcoded so lint won't break the build by it's own and always generates reports. +properties except `abortOnError`, `htmlReport` and `xmlReport`. These are hardcoded so lint won't break the build on its own and always generates reports. From 56ea31b9e167ca1b623c38ed0994127b617287c3 Mon Sep 17 00:00:00 2001 From: Tobias Heine Date: Wed, 31 Jan 2018 17:33:44 +0100 Subject: [PATCH 36/39] Add a warning regarding missing kotlin support for lint --- docs/tools/android_lint.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/docs/tools/android_lint.md b/docs/tools/android_lint.md index 6e06b3e..65b9ae2 100644 --- a/docs/tools/android_lint.md +++ b/docs/tools/android_lint.md @@ -2,6 +2,8 @@ [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.0.1 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 hardcoded so lint won't break the build on its own and always generates reports. From 0cb9763eabfc9e56b235a5791d24b0fddb46c547 Mon Sep 17 00:00:00 2001 From: Tobias Heine Date: Thu, 1 Feb 2018 08:30:32 +0100 Subject: [PATCH 37/39] Fix wrong version --- docs/tools/android_lint.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/tools/android_lint.md b/docs/tools/android_lint.md index 65b9ae2..8754625 100644 --- a/docs/tools/android_lint.md +++ b/docs/tools/android_lint.md @@ -2,7 +2,7 @@ [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.0.1 of the [Android Gradle Plugin](https://developer.android.com/studio/releases/gradle-plugin.html). +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) From 6f0bfc7552f1f4407f02b23edaa11547a4f70701 Mon Sep 17 00:00:00 2001 From: Tobias Heine Date: Thu, 1 Feb 2018 08:31:05 +0100 Subject: [PATCH 38/39] Fix typo --- docs/tools/android_lint.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/tools/android_lint.md b/docs/tools/android_lint.md index 8754625..b3ca875 100644 --- a/docs/tools/android_lint.md +++ b/docs/tools/android_lint.md @@ -6,4 +6,4 @@ Be aware that Lint just supports Kotlin since version 3.1.0 of the [Android Grad ## 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 hardcoded so lint won't break the build on its own and always generates reports. +properties except `abortOnError`, `htmlReport` and `xmlReport`. These are hardcoded so Lint won't break the build on its own and always generates reports. From 8b1d1cf58e48d029a68c17db81a251a8cfb12c74 Mon Sep 17 00:00:00 2001 From: Tobias Heine Date: Thu, 1 Feb 2018 09:50:38 +0100 Subject: [PATCH 39/39] Tweak lint documentation --- docs/tools/android_lint.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/tools/android_lint.md b/docs/tools/android_lint.md index b3ca875..97f68f2 100644 --- a/docs/tools/android_lint.md +++ b/docs/tools/android_lint.md @@ -6,4 +6,4 @@ Be aware that Lint just supports Kotlin since version 3.1.0 of the [Android Grad ## 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 hardcoded so Lint won't break the build on its own and always generates reports. +properties except `abortOnError`, `htmlReport` and `xmlReport`. These are overridden so that Lint won't break the build on its own and always generates reports.