From cfbe100e0e57808c506c96039c75266e495cbb46 Mon Sep 17 00:00:00 2001 From: Said Tahsin Dane <tasomaniac@gmail.com> Date: Mon, 3 Dec 2018 23:23:48 +0100 Subject: [PATCH 01/11] Fix ktlint for all versions --- .../internal/ktlint/KtlintConfigurator.groovy | 28 ++++++++++++------- .../ktlint/KtlintIntegrationTest.groovy | 10 +++---- 2 files changed, 23 insertions(+), 15 deletions(-) diff --git a/plugin/src/main/groovy/com/novoda/staticanalysis/internal/ktlint/KtlintConfigurator.groovy b/plugin/src/main/groovy/com/novoda/staticanalysis/internal/ktlint/KtlintConfigurator.groovy index 2b8d617..f8363fa 100644 --- a/plugin/src/main/groovy/com/novoda/staticanalysis/internal/ktlint/KtlintConfigurator.groovy +++ b/plugin/src/main/groovy/com/novoda/staticanalysis/internal/ktlint/KtlintConfigurator.groovy @@ -84,12 +84,19 @@ class KtlintConfigurator implements Configurator { } private void configureAndroidWithVariants(def mainVariants) { - mainVariants.all { configureKtlint(it.name) } - variantFilter.filteredTestVariants.all { configureKtlint(it.name) } - variantFilter.filteredUnitTestVariants.all { configureKtlint(it.name) } + mainVariants.all { configureAndroidVariant(it) } + variantFilter.filteredTestVariants.all { configureAndroidVariant(it) } + variantFilter.filteredUnitTestVariants.all { configureAndroidVariant(it) } } - private void configureKtlint(def sourceSetName) { + private void configureAndroidVariant(def variant) { + variant.sourceSets.each { sourceSet -> + configureKtlint(sourceSet.name) + } + configureKtlint(variant.name) + } + + private void configureKtlint(String sourceSetName) { project.tasks.matching { it.name == "ktlint${sourceSetName.capitalize()}Check" }.all { Task ktlintTask -> @@ -99,7 +106,7 @@ class KtlintConfigurator implements Configurator { } } - private def configureKtlintWithOutputFiles(def sourceSetName, Map<?, RegularFileProperty> reportOutputFiles) { + private def configureKtlintWithOutputFiles(String sourceSetName, Map<?, RegularFileProperty> reportOutputFiles) { File xmlReportFile = null File txtReportFile = null reportOutputFiles.each { key, fileProp -> @@ -120,10 +127,11 @@ class KtlintConfigurator implements Configurator { } private def createCollectViolationsTask(Violations violations, def sourceSetName, File xmlReportFile, File txtReportFile) { - project.tasks.create("collectKtlint${sourceSetName.capitalize()}Violations", CollectCheckstyleViolationsTask) { task -> - task.xmlReportFile = xmlReportFile - task.htmlReportFile = txtReportFile - task.violations = violations - } + CollectCheckstyleViolationsTask task = + project.tasks.maybeCreate("collectKtlint${sourceSetName.capitalize()}Violations", CollectCheckstyleViolationsTask) + task.xmlReportFile = xmlReportFile + task.htmlReportFile = txtReportFile + task.violations = violations + return task } } diff --git a/plugin/src/test/groovy/com/novoda/staticanalysis/internal/ktlint/KtlintIntegrationTest.groovy b/plugin/src/test/groovy/com/novoda/staticanalysis/internal/ktlint/KtlintIntegrationTest.groovy index 17e0511..111b957 100644 --- a/plugin/src/test/groovy/com/novoda/staticanalysis/internal/ktlint/KtlintIntegrationTest.groovy +++ b/plugin/src/test/groovy/com/novoda/staticanalysis/internal/ktlint/KtlintIntegrationTest.groovy @@ -35,12 +35,12 @@ class KtlintIntegrationTest { [TestProjectRule.forKotlinProject(), '5.1.0', 'ktlint-main.txt'], [TestProjectRule.forAndroidKotlinProject(), '5.1.0', 'ktlint-debug.txt'], [TestProjectRule.forKotlinProject(), '6.1.0', 'ktlintMainCheck.txt'], + // Fails for our test setup since we have custom sourceDirs. https://github.com/JLLeitschuh/ktlint-gradle/issues/153 + // [TestProjectRule.forAndroidKotlinProject(), '6.1.0', 'ktlintDebugCheck.txt'], [TestProjectRule.forKotlinProject(), '6.2.1', 'ktlintMainCheck.txt'], - /** - * Tracked in https://github.com/novoda/gradle-static-analysis-plugin/issues/146 - */ - //[TestProjectRule.forAndroidKotlinProject(), '6.1.0', 'ktlintDebugCheck.txt'], - //[TestProjectRule.forAndroidKotlinProject(), '6.2.1', 'ktlintDebugCheck.txt'], + [TestProjectRule.forAndroidKotlinProject(), '6.2.1', 'ktlintMainCheck.txt'], + [TestProjectRule.forKotlinProject(), '6.3.1', 'ktlintMainCheck.txt'], + [TestProjectRule.forAndroidKotlinProject(), '6.3.1', 'ktlintMainCheck.txt'], ]*.toArray() } From be0bf8269351081ffcd8c184d258bc61508d6d34 Mon Sep 17 00:00:00 2001 From: Said Tahsin Dane <tasomaniac@gmail.com> Date: Fri, 7 Dec 2018 18:35:40 +0100 Subject: [PATCH 02/11] Remove unnecessary line. Looking at sourcesets are enough for configuration. --- .../staticanalysis/internal/ktlint/KtlintConfigurator.groovy | 1 - 1 file changed, 1 deletion(-) diff --git a/plugin/src/main/groovy/com/novoda/staticanalysis/internal/ktlint/KtlintConfigurator.groovy b/plugin/src/main/groovy/com/novoda/staticanalysis/internal/ktlint/KtlintConfigurator.groovy index f8363fa..f4e6eef 100644 --- a/plugin/src/main/groovy/com/novoda/staticanalysis/internal/ktlint/KtlintConfigurator.groovy +++ b/plugin/src/main/groovy/com/novoda/staticanalysis/internal/ktlint/KtlintConfigurator.groovy @@ -93,7 +93,6 @@ class KtlintConfigurator implements Configurator { variant.sourceSets.each { sourceSet -> configureKtlint(sourceSet.name) } - configureKtlint(variant.name) } private void configureKtlint(String sourceSetName) { From fd4dee8b19aa7611dd74691387edfc771e3d0731 Mon Sep 17 00:00:00 2001 From: Said Tahsin Dane <tasomaniac@gmail.com> Date: Mon, 10 Dec 2018 10:14:58 +0100 Subject: [PATCH 03/11] Make Findbugs Html report optional (#154) * Make html report optional for findbugs * Put the feature in documentation * Also use Closure on other configs * Make the tests prettier * Revert the changes to the test * Update FindbugsIntegrationTest.groovy --- docs/tools/findbugs.md | 1 + .../internal/CodeQualityConfigurator.groovy | 6 ++--- .../checkstyle/CheckstyleConfigurator.groovy | 12 ++++----- .../findbugs/FindbugsConfigurator.groovy | 27 ++++++++++++------- .../internal/pmd/PmdConfigurator.groovy | 9 +++---- .../findbugs/FindbugsIntegrationTest.groovy | 16 +++++++++-- 6 files changed, 43 insertions(+), 28 deletions(-) diff --git a/docs/tools/findbugs.md b/docs/tools/findbugs.md index 2dedd64..7124886 100644 --- a/docs/tools/findbugs.md +++ b/docs/tools/findbugs.md @@ -17,6 +17,7 @@ findbugs { toolVersion // A string, most likely '3.0.1' — the latest Findbugs release (for a long time) exclude // A fileTree, such as project.fileTree('src/test/java') to exclude Java unit tests excludeFilter // A file containing the Findbugs exclusions, e.g., teamPropsFile('static-analysis/findbugs-excludes.xml') + htmlReportEnabled true // Control whether html report generation should be enabled. `true` by default. includeVariants { variant -> ... } // A closure to determine which variants (for Android) to include } ``` diff --git a/plugin/src/main/groovy/com/novoda/staticanalysis/internal/CodeQualityConfigurator.groovy b/plugin/src/main/groovy/com/novoda/staticanalysis/internal/CodeQualityConfigurator.groovy index 94b7d86..0858863 100644 --- a/plugin/src/main/groovy/com/novoda/staticanalysis/internal/CodeQualityConfigurator.groovy +++ b/plugin/src/main/groovy/com/novoda/staticanalysis/internal/CodeQualityConfigurator.groovy @@ -73,10 +73,8 @@ abstract class CodeQualityConfigurator<T extends SourceTask, E extends CodeQuali protected abstract Class<E> getExtensionClass() protected Action<E> getDefaultConfiguration() { - new Action<E>() { - void execute(E ignored) { - // no op - } + return { ignored -> + // no op } } diff --git a/plugin/src/main/groovy/com/novoda/staticanalysis/internal/checkstyle/CheckstyleConfigurator.groovy b/plugin/src/main/groovy/com/novoda/staticanalysis/internal/checkstyle/CheckstyleConfigurator.groovy index 534e0ba..ed10801 100644 --- a/plugin/src/main/groovy/com/novoda/staticanalysis/internal/checkstyle/CheckstyleConfigurator.groovy +++ b/plugin/src/main/groovy/com/novoda/staticanalysis/internal/checkstyle/CheckstyleConfigurator.groovy @@ -3,7 +3,10 @@ package com.novoda.staticanalysis.internal.checkstyle import com.novoda.staticanalysis.Violations import com.novoda.staticanalysis.internal.CodeQualityConfigurator import com.novoda.staticanalysis.internal.QuietLogger -import org.gradle.api.* +import org.gradle.api.Action +import org.gradle.api.NamedDomainObjectContainer +import org.gradle.api.Project +import org.gradle.api.Task import org.gradle.api.plugins.quality.Checkstyle import org.gradle.api.plugins.quality.CheckstyleExtension @@ -34,11 +37,8 @@ class CheckstyleConfigurator extends CodeQualityConfigurator<Checkstyle, Checkst @Override protected Action<CheckstyleExtension> getDefaultConfiguration() { - new Action<CheckstyleExtension>() { - @Override - void execute(CheckstyleExtension checkstyleExtension) { - checkstyleExtension.toolVersion = '7.1.2' - } + return { extension -> + extension.toolVersion = '7.1.2' } } diff --git a/plugin/src/main/groovy/com/novoda/staticanalysis/internal/findbugs/FindbugsConfigurator.groovy b/plugin/src/main/groovy/com/novoda/staticanalysis/internal/findbugs/FindbugsConfigurator.groovy index 38c51c8..145dd32 100644 --- a/plugin/src/main/groovy/com/novoda/staticanalysis/internal/findbugs/FindbugsConfigurator.groovy +++ b/plugin/src/main/groovy/com/novoda/staticanalysis/internal/findbugs/FindbugsConfigurator.groovy @@ -2,7 +2,10 @@ package com.novoda.staticanalysis.internal.findbugs import com.novoda.staticanalysis.Violations import com.novoda.staticanalysis.internal.CodeQualityConfigurator -import org.gradle.api.* +import org.gradle.api.Action +import org.gradle.api.NamedDomainObjectContainer +import org.gradle.api.Project +import org.gradle.api.Task import org.gradle.api.file.ConfigurableFileTree import org.gradle.api.file.FileCollection import org.gradle.api.plugins.quality.FindBugs @@ -13,6 +16,8 @@ import java.nio.file.Path class FindbugsConfigurator extends CodeQualityConfigurator<FindBugs, FindBugsExtension> { + protected boolean htmlReportEnabled = true + static FindbugsConfigurator create(Project project, NamedDomainObjectContainer<Violations> violationsContainer, Task evaluateViolations) { @@ -48,11 +53,9 @@ class FindbugsConfigurator extends CodeQualityConfigurator<FindBugs, FindBugsExt @Override protected Action<FindBugsExtension> getDefaultConfiguration() { - new Action<FindBugsExtension>() { - @Override - void execute(FindBugsExtension findBugsExtension) { - findBugsExtension.toolVersion = '3.0.1' - } + return { extension -> + extension.ext.htmlReportEnabled = { boolean enabled -> this.htmlReportEnabled = enabled } + extension.toolVersion = '3.0.1' } } @@ -134,11 +137,15 @@ class FindbugsConfigurator extends CodeQualityConfigurator<FindBugs, FindBugsExt findBugs.reports.html.enabled = false def collectViolations = createViolationsCollectionTask(findBugs, violations) - def generateHtmlReport = createHtmlReportTask(findBugs, collectViolations.xmlReportFile, collectViolations.htmlReportFile) - evaluateViolations.dependsOn collectViolations - collectViolations.dependsOn generateHtmlReport - generateHtmlReport.dependsOn findBugs + + if (htmlReportEnabled) { + def generateHtmlReport = createHtmlReportTask(findBugs, collectViolations.xmlReportFile, collectViolations.htmlReportFile) + collectViolations.dependsOn generateHtmlReport + generateHtmlReport.dependsOn findBugs + } else { + collectViolations.dependsOn findBugs + } } private CollectFindbugsViolationsTask createViolationsCollectionTask(FindBugs findBugs, Violations violations) { diff --git a/plugin/src/main/groovy/com/novoda/staticanalysis/internal/pmd/PmdConfigurator.groovy b/plugin/src/main/groovy/com/novoda/staticanalysis/internal/pmd/PmdConfigurator.groovy index d47a7eb..635c008 100644 --- a/plugin/src/main/groovy/com/novoda/staticanalysis/internal/pmd/PmdConfigurator.groovy +++ b/plugin/src/main/groovy/com/novoda/staticanalysis/internal/pmd/PmdConfigurator.groovy @@ -42,12 +42,9 @@ class PmdConfigurator extends CodeQualityConfigurator<Pmd, PmdExtension> { @Override protected Action<PmdExtension> getDefaultConfiguration() { - new Action<PmdExtension>() { - @Override - void execute(PmdExtension pmdExtension) { - pmdExtension.toolVersion = '5.5.1' - pmdExtension.rulePriority = 5 - } + return { extension -> + extension.toolVersion = '5.5.1' + extension.rulePriority = 5 } } diff --git a/plugin/src/test/groovy/com/novoda/staticanalysis/internal/findbugs/FindbugsIntegrationTest.groovy b/plugin/src/test/groovy/com/novoda/staticanalysis/internal/findbugs/FindbugsIntegrationTest.groovy index 8ce896e..1089d7c 100644 --- a/plugin/src/test/groovy/com/novoda/staticanalysis/internal/findbugs/FindbugsIntegrationTest.groovy +++ b/plugin/src/test/groovy/com/novoda/staticanalysis/internal/findbugs/FindbugsIntegrationTest.groovy @@ -351,11 +351,11 @@ class FindbugsIntegrationTest { projectRule.newProject() .withSourceSet('main', SOURCES_WITH_LOW_VIOLATION) .withPenalty('none') - .withToolsConfig(""" + .withToolsConfig(""" findbugs { } findbugs { ignoreFailures = false - } + } """) .build('check') } @@ -379,6 +379,18 @@ class FindbugsIntegrationTest { Truth.assertThat(result.outcome(':generateFindbugsDebugHtmlReport')).isEqualTo(TaskOutcome.UP_TO_DATE) } + @Test + void shouldNotGenerateHtmlWhenDisabled() { + def result = projectRule.newProject() + .withSourceSet('main', SOURCES_WITH_LOW_VIOLATION) + .withToolsConfig('''findbugs { + htmlReportEnabled false + }''') + .build('check') + + Truth.assertThat(result.tasksPaths).doesNotContain(':generateFindbugsDebugHtmlReport') + } + /** * The custom task created in the snippet below will check whether {@code Findbugs} tasks with * empty {@code source} will have empty {@code classes} too. </p> From 6dc877aafaee8cf51152c8a431391eb77f92ab25 Mon Sep 17 00:00:00 2001 From: Said Tahsin Dane <tasomaniac@gmail.com> Date: Mon, 10 Dec 2018 21:00:24 +0100 Subject: [PATCH 04/11] Change ktlint task matching to match all versions --- .../internal/ktlint/KtlintConfigurator.groovy | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/plugin/src/main/groovy/com/novoda/staticanalysis/internal/ktlint/KtlintConfigurator.groovy b/plugin/src/main/groovy/com/novoda/staticanalysis/internal/ktlint/KtlintConfigurator.groovy index f4e6eef..d39fcae 100644 --- a/plugin/src/main/groovy/com/novoda/staticanalysis/internal/ktlint/KtlintConfigurator.groovy +++ b/plugin/src/main/groovy/com/novoda/staticanalysis/internal/ktlint/KtlintConfigurator.groovy @@ -97,7 +97,7 @@ class KtlintConfigurator implements Configurator { private void configureKtlint(String sourceSetName) { project.tasks.matching { - it.name == "ktlint${sourceSetName.capitalize()}Check" + isKtlintTask(it, sourceSetName.capitalize()) }.all { Task ktlintTask -> def collectViolations = configureKtlintWithOutputFiles(sourceSetName, ktlintTask.reportOutputFiles) collectViolations.dependsOn ktlintTask @@ -105,6 +105,13 @@ class KtlintConfigurator implements Configurator { } } + /** + * KtLint task has the following naming convention and the needed property to resolve the reportOutputFiles + */ + private boolean isKtlintTask(Task task, String sourceSetName) { + task.name ==~ /^ktlint$sourceSetName(SourceSet)?Check$/ && task.hasProperty('reportOutputFiles') + } + private def configureKtlintWithOutputFiles(String sourceSetName, Map<?, RegularFileProperty> reportOutputFiles) { File xmlReportFile = null File txtReportFile = null From 5ab15b17860eaacde93f33d0828520f6d31ea691 Mon Sep 17 00:00:00 2001 From: Said Tahsin Dane <tasomaniac@gmail.com> Date: Mon, 10 Dec 2018 21:43:03 +0100 Subject: [PATCH 05/11] Support for copying folders into the repo --- .../internal/ktlint/KtlintIntegrationTest.groovy | 5 ++--- plugin/src/test/groovy/com/novoda/test/TestProject.groovy | 7 +++++++ 2 files changed, 9 insertions(+), 3 deletions(-) diff --git a/plugin/src/test/groovy/com/novoda/staticanalysis/internal/ktlint/KtlintIntegrationTest.groovy b/plugin/src/test/groovy/com/novoda/staticanalysis/internal/ktlint/KtlintIntegrationTest.groovy index 111b957..49db162 100644 --- a/plugin/src/test/groovy/com/novoda/staticanalysis/internal/ktlint/KtlintIntegrationTest.groovy +++ b/plugin/src/test/groovy/com/novoda/staticanalysis/internal/ktlint/KtlintIntegrationTest.groovy @@ -35,8 +35,7 @@ class KtlintIntegrationTest { [TestProjectRule.forKotlinProject(), '5.1.0', 'ktlint-main.txt'], [TestProjectRule.forAndroidKotlinProject(), '5.1.0', 'ktlint-debug.txt'], [TestProjectRule.forKotlinProject(), '6.1.0', 'ktlintMainCheck.txt'], - // Fails for our test setup since we have custom sourceDirs. https://github.com/JLLeitschuh/ktlint-gradle/issues/153 - // [TestProjectRule.forAndroidKotlinProject(), '6.1.0', 'ktlintDebugCheck.txt'], + [TestProjectRule.forAndroidKotlinProject(), '6.1.0', 'ktlintMainCheck.txt'], [TestProjectRule.forKotlinProject(), '6.2.1', 'ktlintMainCheck.txt'], [TestProjectRule.forAndroidKotlinProject(), '6.2.1', 'ktlintMainCheck.txt'], [TestProjectRule.forKotlinProject(), '6.3.1', 'ktlintMainCheck.txt'], @@ -116,7 +115,7 @@ class KtlintIntegrationTest { private TestProject createProjectWith(File sources, int maxErrors = 0) { projectRule.newProject() .withPlugin('org.jlleitschuh.gradle.ktlint', ktlintVersion) - .withSourceSet('main', sources) + .copyIntoSourceSet('main', sources) .withPenalty("""{ maxWarnings = 0 maxErrors = ${maxErrors} diff --git a/plugin/src/test/groovy/com/novoda/test/TestProject.groovy b/plugin/src/test/groovy/com/novoda/test/TestProject.groovy index c8e7efd..5bc0fb7 100644 --- a/plugin/src/test/groovy/com/novoda/test/TestProject.groovy +++ b/plugin/src/test/groovy/com/novoda/test/TestProject.groovy @@ -63,6 +63,13 @@ ${project.additionalConfiguration} file.text = text } + public T copyIntoSourceSet(String sourceSet, File srcDir) { + srcDir.listFiles().each { + withFile(it, "src/${sourceSet}/java/${it.name}") + } + return this + } + public T withSourceSet(String sourceSet, File... srcDirs) { sourceSets[sourceSet] = srcDirs return this From 0207e967b99b0a12ed51bd7551b9e7d3e2bc102f Mon Sep 17 00:00:00 2001 From: Said Tahsin Dane <tasomaniac@gmail.com> Date: Mon, 10 Dec 2018 23:55:49 +0100 Subject: [PATCH 06/11] Add new detekt versions in tests --- .../staticanalysis/internal/detekt/DetektConfigurator.groovy | 2 +- .../internal/detekt/DetektNewIntegrationTest.groovy | 4 ++++ 2 files changed, 5 insertions(+), 1 deletion(-) diff --git a/plugin/src/main/groovy/com/novoda/staticanalysis/internal/detekt/DetektConfigurator.groovy b/plugin/src/main/groovy/com/novoda/staticanalysis/internal/detekt/DetektConfigurator.groovy index 4e6cecf..ed4aaad 100644 --- a/plugin/src/main/groovy/com/novoda/staticanalysis/internal/detekt/DetektConfigurator.groovy +++ b/plugin/src/main/groovy/com/novoda/staticanalysis/internal/detekt/DetektConfigurator.groovy @@ -12,7 +12,7 @@ import org.gradle.api.Task class DetektConfigurator implements Configurator { private static final String DETEKT_PLUGIN = 'io.gitlab.arturbosch.detekt' - private static final String LAST_COMPATIBLE_DETEKT_VERSION = '1.0.0-RC10' + private static final String LAST_COMPATIBLE_DETEKT_VERSION = '1.0.0-RC12' private static final String DETEKT_NOT_APPLIED = 'The Detekt plugin is configured but not applied. Please apply the plugin in your build script.\nFor more information see https://github.com/arturbosch/detekt.' private static final String OUTPUT_NOT_DEFINED = 'Output not defined! To analyze the results, `output` needs to be defined in Detekt profile.' private static final String DETEKT_CONFIGURATION_ERROR = "A problem occurred while configuring Detekt. Please make sure to use a compatible version (All versions up to $LAST_COMPATIBLE_DETEKT_VERSION)" diff --git a/plugin/src/test/groovy/com/novoda/staticanalysis/internal/detekt/DetektNewIntegrationTest.groovy b/plugin/src/test/groovy/com/novoda/staticanalysis/internal/detekt/DetektNewIntegrationTest.groovy index ca65ded..89149b0 100644 --- a/plugin/src/test/groovy/com/novoda/staticanalysis/internal/detekt/DetektNewIntegrationTest.groovy +++ b/plugin/src/test/groovy/com/novoda/staticanalysis/internal/detekt/DetektNewIntegrationTest.groovy @@ -25,6 +25,10 @@ class DetektNewIntegrationTest { [TestProjectRule.forAndroidKotlinProject(), "1.0.0.RC9.2"], [TestProjectRule.forKotlinProject(), "1.0.0-RC10"], [TestProjectRule.forAndroidKotlinProject(), "1.0.0-RC10"], + [TestProjectRule.forKotlinProject(), "1.0.0-RC11"], + [TestProjectRule.forAndroidKotlinProject(), "1.0.0-RC11"], + [TestProjectRule.forKotlinProject(), "1.0.0-RC12"], + [TestProjectRule.forAndroidKotlinProject(), "1.0.0-RC12"], ]*.toArray() } From a0cba265d8d1f2a692ad85983dd107f4d864894b Mon Sep 17 00:00:00 2001 From: Said Tahsin Dane <tasomaniac@gmail.com> Date: Tue, 11 Dec 2018 22:24:46 +0100 Subject: [PATCH 07/11] Fix empty space problem with the output. Also modified the tests to test the whole message. Fixes #155 --- .../DefaultViolationsEvaluator.groovy | 8 +++---- .../DefaultViolationsEvaluatorTest.groovy | 24 +++++++++++++++---- .../groovy/com/novoda/test/LogsSubject.groovy | 4 ++++ 3 files changed, 27 insertions(+), 9 deletions(-) diff --git a/plugin/src/main/groovy/com/novoda/staticanalysis/DefaultViolationsEvaluator.groovy b/plugin/src/main/groovy/com/novoda/staticanalysis/DefaultViolationsEvaluator.groovy index d2b7761..0beec77 100644 --- a/plugin/src/main/groovy/com/novoda/staticanalysis/DefaultViolationsEvaluator.groovy +++ b/plugin/src/main/groovy/com/novoda/staticanalysis/DefaultViolationsEvaluator.groovy @@ -18,7 +18,7 @@ class DefaultViolationsEvaluator implements ViolationsEvaluator { @Override void evaluate(Set<Violations> allViolations) { Map<String, Integer> total = [errors: 0, warnings: 0] - String fullMessage = '\n' + String fullMessage = '' allViolations.each { Violations violations -> if (!violations.isEmpty()) { fullMessage += "> ${getViolationsMessage(violations, reportUrlRenderer)}\n" @@ -30,9 +30,9 @@ class DefaultViolationsEvaluator implements ViolationsEvaluator { int errorsDiff = Math.max(0, total['errors'] - penalty.maxErrors) int warningsDiff = Math.max(0, total['warnings'] - penalty.maxWarnings) if (errorsDiff > 0 || warningsDiff > 0) { - throw new GradleException("Violations limit exceeded by $errorsDiff errors, $warningsDiff warnings.\n$fullMessage") - } else { - logger.warn fullMessage + throw new GradleException("Violations limit exceeded by $errorsDiff errors, $warningsDiff warnings.\n\n$fullMessage") + } else if (!fullMessage.isEmpty()) { + logger.warn "\n$fullMessage" } } diff --git a/plugin/src/test/groovy/com/novoda/staticanalysis/DefaultViolationsEvaluatorTest.groovy b/plugin/src/test/groovy/com/novoda/staticanalysis/DefaultViolationsEvaluatorTest.groovy index 5093183..99438af 100644 --- a/plugin/src/test/groovy/com/novoda/staticanalysis/DefaultViolationsEvaluatorTest.groovy +++ b/plugin/src/test/groovy/com/novoda/staticanalysis/DefaultViolationsEvaluatorTest.groovy @@ -2,6 +2,7 @@ package com.novoda.staticanalysis import org.gradle.api.GradleException import org.gradle.api.logging.Logger +import org.gradle.internal.logging.ConsoleRenderer import org.junit.Before import org.junit.Rule import org.junit.Test @@ -10,8 +11,7 @@ import org.mockito.ArgumentCaptor import static com.google.common.truth.Truth.assertThat import static org.junit.Assert.fail -import static org.mockito.Mockito.mock -import static org.mockito.Mockito.verify +import static org.mockito.Mockito.* class DefaultViolationsEvaluatorTest { @@ -43,7 +43,11 @@ class DefaultViolationsEvaluatorTest { evaluator.evaluate(allViolations) - assertThat(warningLog).contains("$TOOL_NAME violations found (1 errors, 0 warnings).") + def expected = """ + > $TOOL_NAME violations found (1 errors, 0 warnings). See the reports at: + - $consoleClickableFileUrl + """ + assertThat(warningLog).isEqualTo(expected.stripIndent()) } @Test @@ -52,7 +56,7 @@ class DefaultViolationsEvaluatorTest { evaluator.evaluate(allViolations) - assertThat(warningLog).doesNotContain("$TOOL_NAME violations found") + verifyZeroInteractions(logger) } @Test @@ -63,7 +67,13 @@ class DefaultViolationsEvaluatorTest { evaluator.evaluate(allViolations) fail('Exception expected but not thrown') } catch (GradleException e) { - assertThat(e.message).contains('Violations limit exceeded by 0 errors, 1 warnings.') + def expected = + """|Violations limit exceeded by 0 errors, 1 warnings. + | + |> $TOOL_NAME violations found (1 errors, 2 warnings). See the reports at: + |- $consoleClickableFileUrl + |""" + assertThat(e.message).isEqualTo(expected.stripMargin()) } } @@ -76,4 +86,8 @@ class DefaultViolationsEvaluatorTest { verify(logger).warn(captor.capture()) captor.getValue() } + + private String getConsoleClickableFileUrl() { + new ConsoleRenderer().asClickableFileUrl(reportFile) + } } diff --git a/plugin/src/test/groovy/com/novoda/test/LogsSubject.groovy b/plugin/src/test/groovy/com/novoda/test/LogsSubject.groovy index 35fb1e9..005733f 100644 --- a/plugin/src/test/groovy/com/novoda/test/LogsSubject.groovy +++ b/plugin/src/test/groovy/com/novoda/test/LogsSubject.groovy @@ -44,6 +44,10 @@ class LogsSubject extends Subject<LogsSubject, Logs> { check().that(actual().output) } + public void isEmpty() { + outputSubject.isEmpty() + } + public void contains(log) { outputSubject.contains(log) } From 495041caeb85a2dad0f62c882a4fbdae89809384 Mon Sep 17 00:00:00 2001 From: Said Tahsin Dane <tasomaniac@gmail.com> Date: Tue, 11 Dec 2018 22:55:45 +0100 Subject: [PATCH 08/11] I was looking at #33 and realized that I've been seeing Android classes in the output of Findbugs. It turns out that these are called `auxclass` since they are not available to findbugs. Adding `auxclass` parameter removes these warnings and the output becomes less verbose --- .../internal/findbugs/FindbugsConfigurator.groovy | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/plugin/src/main/groovy/com/novoda/staticanalysis/internal/findbugs/FindbugsConfigurator.groovy b/plugin/src/main/groovy/com/novoda/staticanalysis/internal/findbugs/FindbugsConfigurator.groovy index 145dd32..26174f4 100644 --- a/plugin/src/main/groovy/com/novoda/staticanalysis/internal/findbugs/FindbugsConfigurator.groovy +++ b/plugin/src/main/groovy/com/novoda/staticanalysis/internal/findbugs/FindbugsConfigurator.groovy @@ -67,6 +67,7 @@ class FindbugsConfigurator extends CodeQualityConfigurator<FindBugs, FindBugsExt description = "Run FindBugs analysis for ${variant.name} classes" source = androidSourceDirs classpath = variant.javaCompile.classpath + extraArgs '-auxclasspath', androidJar exclude '**/*.kt' } sourceFilter.applyTo(task) @@ -164,4 +165,7 @@ class FindbugsConfigurator extends CodeQualityConfigurator<FindBugs, FindBugsExt task } + private def getAndroidJar() { + "${project.android.sdkDirectory}/platforms/${project.android.compileSdkVersion}/android.jar" + } } From 2064cb6a41c1bd5aa5a1bc429683d4e5ab4f9f22 Mon Sep 17 00:00:00 2001 From: Said Tahsin Dane <tasomaniac@gmail.com> Date: Tue, 11 Dec 2018 23:06:03 +0100 Subject: [PATCH 09/11] Also add total number of violations to the output Fixes #19 --- .../staticanalysis/DefaultViolationsEvaluator.groovy | 11 +++++------ .../DefaultViolationsEvaluatorTest.groovy | 4 +++- 2 files changed, 8 insertions(+), 7 deletions(-) diff --git a/plugin/src/main/groovy/com/novoda/staticanalysis/DefaultViolationsEvaluator.groovy b/plugin/src/main/groovy/com/novoda/staticanalysis/DefaultViolationsEvaluator.groovy index 0beec77..99f8315 100644 --- a/plugin/src/main/groovy/com/novoda/staticanalysis/DefaultViolationsEvaluator.groovy +++ b/plugin/src/main/groovy/com/novoda/staticanalysis/DefaultViolationsEvaluator.groovy @@ -17,22 +17,21 @@ class DefaultViolationsEvaluator implements ViolationsEvaluator { @Override void evaluate(Set<Violations> allViolations) { - Map<String, Integer> total = [errors: 0, warnings: 0] String fullMessage = '' allViolations.each { Violations violations -> if (!violations.isEmpty()) { fullMessage += "> ${getViolationsMessage(violations, reportUrlRenderer)}\n" } } - total['errors'] = allViolations.collect { it.errors }.sum() as int - total['warnings'] = allViolations.collect { it.warnings }.sum() as int + int totalErrors = allViolations.collect { it.errors }.sum() as int + int totalWarnings = allViolations.collect { it.warnings }.sum() as int - int errorsDiff = Math.max(0, total['errors'] - penalty.maxErrors) - int warningsDiff = Math.max(0, total['warnings'] - penalty.maxWarnings) + int errorsDiff = Math.max(0, totalErrors - penalty.maxErrors) + int warningsDiff = Math.max(0, totalWarnings - penalty.maxWarnings) if (errorsDiff > 0 || warningsDiff > 0) { throw new GradleException("Violations limit exceeded by $errorsDiff errors, $warningsDiff warnings.\n\n$fullMessage") } else if (!fullMessage.isEmpty()) { - logger.warn "\n$fullMessage" + logger.warn "\nViolations found ($totalErrors errors, $totalWarnings warnings)\n\n$fullMessage" } } diff --git a/plugin/src/test/groovy/com/novoda/staticanalysis/DefaultViolationsEvaluatorTest.groovy b/plugin/src/test/groovy/com/novoda/staticanalysis/DefaultViolationsEvaluatorTest.groovy index 99438af..d6d42b6 100644 --- a/plugin/src/test/groovy/com/novoda/staticanalysis/DefaultViolationsEvaluatorTest.groovy +++ b/plugin/src/test/groovy/com/novoda/staticanalysis/DefaultViolationsEvaluatorTest.groovy @@ -43,7 +43,9 @@ class DefaultViolationsEvaluatorTest { evaluator.evaluate(allViolations) - def expected = """ + def expected = """ + Violations found (1 errors, 0 warnings) + > $TOOL_NAME violations found (1 errors, 0 warnings). See the reports at: - $consoleClickableFileUrl """ From 294634336d48c98d2dff0c6c5fbea2df9a19b309 Mon Sep 17 00:00:00 2001 From: Said Tahsin Dane <tasomaniac@gmail.com> Date: Mon, 17 Dec 2018 23:03:44 +0100 Subject: [PATCH 10/11] Revert unnecessary change in tests --- plugin/src/test/groovy/com/novoda/test/LogsSubject.groovy | 4 ---- 1 file changed, 4 deletions(-) diff --git a/plugin/src/test/groovy/com/novoda/test/LogsSubject.groovy b/plugin/src/test/groovy/com/novoda/test/LogsSubject.groovy index 005733f..35fb1e9 100644 --- a/plugin/src/test/groovy/com/novoda/test/LogsSubject.groovy +++ b/plugin/src/test/groovy/com/novoda/test/LogsSubject.groovy @@ -44,10 +44,6 @@ class LogsSubject extends Subject<LogsSubject, Logs> { check().that(actual().output) } - public void isEmpty() { - outputSubject.isEmpty() - } - public void contains(log) { outputSubject.contains(log) } From 86afd16257bc317eeac26c30e71f18ac937fafde Mon Sep 17 00:00:00 2001 From: Said Tahsin Dane <tasomaniac@gmail.com> Date: Tue, 18 Dec 2018 23:28:54 +0100 Subject: [PATCH 11/11] Prepare for Release 0.8 --- CHANGELOG.md | 15 +++++++++++++++ README.md | 4 ++-- build.gradle | 2 +- docs/tools/ktlint.md | 7 ------- gradle/publish.gradle | 2 +- sample-multi-module/build.gradle | 5 ++--- sample/app/build.gradle | 2 +- sample/build.gradle | 5 +++-- 8 files changed, 25 insertions(+), 17 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 98a3d2e..695bdfd 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,6 +1,21 @@ Change Log ========== +[Version 0.8](https://github.com/novoda/gradle-static-analysis-plugin/releases/tag/v0.8) +-------------------------- + +- Fix integration for all versions of Ktlint plugin [PR#153](https://github.com/novoda/gradle-static-analysis-plugin/pull/153) +- Make Findbugs Html report generation optional [PR#154](https://github.com/novoda/gradle-static-analysis-plugin/pull/154) +``` +staticAnalysis { + findbugs { + htmlReportEnabled false + } +} +``` +- Display total number of errors and warnings [PR#159](https://github.com/novoda/gradle-static-analysis-plugin/pull/159) +- Less verbose Findbugs output [PR#160](https://github.com/novoda/gradle-static-analysis-plugin/pull/160) + [Version 0.7](https://github.com/novoda/gradle-static-analysis-plugin/releases/tag/v0.7) -------------------------- diff --git a/README.md b/README.md index be9e7de..4b98947 100644 --- a/README.md +++ b/README.md @@ -41,7 +41,7 @@ buildscript { jcenter() } dependencies { - classpath 'com.novoda:gradle-static-analysis-plugin:0.7' + classpath 'com.novoda:gradle-static-analysis-plugin:0.8' } } @@ -52,7 +52,7 @@ or from the [Gradle Plugins Repository](https://plugins.gradle.org/): ```gradle plugins { - id 'com.novoda.static-analysis' version '0.7' + id 'com.novoda.static-analysis' version '0.8' } ``` diff --git a/build.gradle b/build.gradle index 6a0f0d8..cb9d87b 100644 --- a/build.gradle +++ b/build.gradle @@ -29,7 +29,7 @@ subprojects { } } -task wrapper(type: Wrapper) { +wrapper { gradleVersion = '4.10.2' distributionType = Wrapper.DistributionType.ALL } diff --git a/docs/tools/ktlint.md b/docs/tools/ktlint.md index 9232242..d4069a0 100644 --- a/docs/tools/ktlint.md +++ b/docs/tools/ktlint.md @@ -5,7 +5,6 @@ this tool only makes sense when you have Kotlin sources in your project. ## Table of contents * [IMPORTANT: setup Ktlint](#important-setup-ktlint) * [Configure Ktlint](#configure-ktlint) - * [Known Issues](#known-issues) --- ## IMPORTANT: setup Ktlint @@ -56,9 +55,3 @@ For other configuration options and adding custom rules, refer to the **Note:** Failures and threshold detection is handled by Static Analysis plugin. That is why `ignoreFailures = true` is set by the plugin. Please do not manually override `ignoreFailures` property. - -## Known Issues - -6.1.0 and 6.2.1 versions are broken for Android projects because of [a bug in Ktlint](https://github.com/JLLeitschuh/ktlint-gradle/issues/153#issuecomment-437176852) - -Because of a behavior change, the `main` sourceSet is not checked. We recommend to use the version 5.1.0 and before. diff --git a/gradle/publish.gradle b/gradle/publish.gradle index 473f561..79ed259 100644 --- a/gradle/publish.gradle +++ b/gradle/publish.gradle @@ -2,7 +2,7 @@ ext { websiteUrl = 'https://github.com/novoda/gradle-static-analysis-plugin' } -version = '0.7' +version = '0.8' String tag = "v$project.version" groovydoc.docTitle = 'Static Analysis Plugin' diff --git a/sample-multi-module/build.gradle b/sample-multi-module/build.gradle index 70628e7..739cbf8 100644 --- a/sample-multi-module/build.gradle +++ b/sample-multi-module/build.gradle @@ -9,7 +9,7 @@ buildscript { dependencies { classpath 'com.android.tools.build:gradle:3.2.1' classpath 'com.novoda:gradle-static-analysis-plugin:local' - classpath 'io.gitlab.arturbosch.detekt:detekt-gradle-plugin:1.0.0-RC10' + classpath 'io.gitlab.arturbosch.detekt:detekt-gradle-plugin:1.0.0-RC12' classpath 'gradle.plugin.org.jlleitschuh.gradle:ktlint-gradle:5.1.0' classpath "org.jetbrains.kotlin:kotlin-gradle-plugin:$kotlin_version" } @@ -19,7 +19,6 @@ ext.ReporterType = org.jlleitschuh.gradle.ktlint.reporter.ReporterType apply from: rootProject.file('team-props/tasks.gradle') subprojects { - buildscript { repositories { jcenter() @@ -33,7 +32,7 @@ subprojects { } } -task wrapper(type: Wrapper) { +wrapper { gradleVersion = '4.10.2' distributionType = Wrapper.DistributionType.ALL } diff --git a/sample/app/build.gradle b/sample/app/build.gradle index 3743065..d50209b 100755 --- a/sample/app/build.gradle +++ b/sample/app/build.gradle @@ -1,7 +1,7 @@ import org.jlleitschuh.gradle.ktlint.reporter.ReporterType plugins { - id 'io.gitlab.arturbosch.detekt' version '1.0.0-RC10' + id 'io.gitlab.arturbosch.detekt' id "org.jlleitschuh.gradle.ktlint" } diff --git a/sample/build.gradle b/sample/build.gradle index e806ef9..b8cb80a 100755 --- a/sample/build.gradle +++ b/sample/build.gradle @@ -9,8 +9,9 @@ buildscript { dependencies { classpath 'com.android.tools.build:gradle:3.2.1' classpath 'com.novoda:gradle-static-analysis-plugin:local' - classpath "org.jetbrains.kotlin:kotlin-gradle-plugin:$kotlin_version" + classpath 'io.gitlab.arturbosch.detekt:detekt-gradle-plugin:1.0.0-RC12' classpath 'gradle.plugin.org.jlleitschuh.gradle:ktlint-gradle:5.1.0' + classpath "org.jetbrains.kotlin:kotlin-gradle-plugin:$kotlin_version" } } @@ -22,7 +23,7 @@ subprojects { } } -task wrapper(type: Wrapper) { +wrapper { gradleVersion = '4.10.2' distributionType = Wrapper.DistributionType.ALL }