From 1bc8ab30821f908b05d44ba0c34115d150fabf65 Mon Sep 17 00:00:00 2001 From: Said Tahsin Dane Date: Tue, 22 Oct 2019 23:11:34 +0200 Subject: [PATCH 01/16] Copy tests for SpotBugs plugin integration. Currently Android tests and exclude tests are ignored. --- .../spotbugs/SpotBugsIntegrationTest.groovy | 322 ++++++++++++++++++ .../groovy/com/novoda/test/LogsSubject.groovy | 9 + 2 files changed, 331 insertions(+) create mode 100644 plugin/src/test/groovy/com/novoda/staticanalysis/internal/spotbugs/SpotBugsIntegrationTest.groovy diff --git a/plugin/src/test/groovy/com/novoda/staticanalysis/internal/spotbugs/SpotBugsIntegrationTest.groovy b/plugin/src/test/groovy/com/novoda/staticanalysis/internal/spotbugs/SpotBugsIntegrationTest.groovy new file mode 100644 index 0000000..6ba833e --- /dev/null +++ b/plugin/src/test/groovy/com/novoda/staticanalysis/internal/spotbugs/SpotBugsIntegrationTest.groovy @@ -0,0 +1,322 @@ +package com.novoda.staticanalysis.internal.spotbugs + +import com.google.common.truth.Truth +import com.novoda.test.TestProject +import com.novoda.test.TestProjectRule +import org.gradle.testkit.runner.TaskOutcome +import org.junit.Ignore +import org.junit.Rule +import org.junit.Test +import org.junit.runner.RunWith +import org.junit.runners.Parameterized + +import static com.novoda.test.Fixtures.Findbugs.* +import static com.novoda.test.LogsSubject.assertThat +import static com.novoda.test.TestProjectSubject.assumeThat + +@RunWith(Parameterized.class) +class SpotBugsIntegrationTest { + + @Parameterized.Parameters(name = "{0}") + static Iterable rules() { + return [TestProjectRule.forJavaProject()/*, TestProjectRule.forAndroidProject()*/] + } + + @Rule + public final TestProjectRule projectRule + + SpotBugsIntegrationTest(TestProjectRule projectRule) { + this.projectRule = projectRule + } + + @Test + void shouldFailBuildWhenSpotBugsWarningsOverTheThreshold() { + TestProject.Result result = createProjectWith() + .withSourceSet('debug', SOURCES_WITH_LOW_VIOLATION, SOURCES_WITH_MEDIUM_VIOLATION) + .withPenalty('''{ + maxErrors = 0 + maxWarnings = 1 + }''') + .withToolsConfig('spotbugs {}') + .buildAndFail('check') + + assertThat(result.logs).containsLimitExceeded(0, 1) + assertThat(result.logs).containsSpotBugsViolations(0, 2, + result.buildFileUrl('reports/spotbugs/debug.html')) + } + + @Test + void shouldFailBuildAfterSecondRunWhenSpotBugsWarningsStillOverTheThreshold() { + def project = createProjectWith() + .withSourceSet('debug', SOURCES_WITH_LOW_VIOLATION, SOURCES_WITH_MEDIUM_VIOLATION) + .withPenalty('''{ + maxErrors = 0 + maxWarnings = 1 + }''') + .withToolsConfig('spotbugs {}') + + TestProject.Result result = project.buildAndFail('check') + + assertThat(result.logs).containsLimitExceeded(0, 1) + assertThat(result.logs).containsSpotBugsViolations(0, 2, + result.buildFileUrl('reports/spotbugs/debug.html')) + + result = project.buildAndFail('check') + + assertThat(result.logs).containsLimitExceeded(0, 1) + assertThat(result.logs).containsSpotBugsViolations(0, 2, + result.buildFileUrl('reports/spotbugs/debug.html')) + } + + @Test + void shouldDetectMoreWarningsWhenEffortIsMaxAndReportLevelIsLow() { + TestProject.Result result = createProjectWith() + .withSourceSet('debug', SOURCES_WITH_LOW_VIOLATION, SOURCES_WITH_MEDIUM_VIOLATION) + .withPenalty('''{ + maxErrors = 0 + maxWarnings = 1 + }''') + .withToolsConfig("spotbugs { effort = 'max' \n reportLevel = 'low'}") + .buildAndFail('check') + + assertThat(result.logs).containsLimitExceeded(0, 2) + assertThat(result.logs).containsSpotBugsViolations(0, 3, + result.buildFileUrl('reports/spotbugs/debug.html')) + } + + @Test + void shouldFailBuildWhenSpotBugsErrorsOverTheThreshold() { + TestProject.Result result = createProjectWith() + .withSourceSet('debug', SOURCES_WITH_HIGH_VIOLATION) + .withPenalty('''{ + maxErrors = 0 + maxWarnings = 0 + }''') + .withToolsConfig('spotbugs {}') + .buildAndFail('check') + + assertThat(result.logs).containsLimitExceeded(1, 0) + assertThat(result.logs).containsSpotBugsViolations(1, 0, + result.buildFileUrl('reports/spotbugs/debug.html')) + } + + @Test + void shouldNotFailBuildWhenNoSpotBugsWarningsOrErrorsEncounteredAndNoThresholdTrespassed() { + TestProject.Result result = createProjectWith() + .withPenalty('''{ + maxErrors = 0 + maxWarnings = 0 + }''') + .withToolsConfig('spotbugs {}') + .build('check') + + assertThat(result.logs).doesNotContainLimitExceeded() + assertThat(result.logs).doesNotContainSpotBugsViolations() + } + + @Test + void shouldNotFailBuildWhenSpotBugsWarningsAndErrorsEncounteredAndNoThresholdTrespassed() { + TestProject.Result result = createProjectWith() + .withSourceSet('debug', SOURCES_WITH_LOW_VIOLATION, SOURCES_WITH_MEDIUM_VIOLATION) + .withSourceSet('release', SOURCES_WITH_HIGH_VIOLATION) + .withPenalty('''{ + maxErrors = 10 + maxWarnings = 10 + }''') + .withToolsConfig('spotbugs {}') + .build('check') + + assertThat(result.logs).doesNotContainLimitExceeded() + assertThat(result.logs).containsSpotBugsViolations(1, 2, + result.buildFileUrl('reports/spotbugs/debug.html'), + result.buildFileUrl('reports/spotbugs/release.html')) + } + + @Test + void shouldNotFailBuildWhenSpotBugsConfiguredToNotIgnoreFailures() { + createProjectWith() + .withSourceSet('debug', SOURCES_WITH_LOW_VIOLATION, SOURCES_WITH_MEDIUM_VIOLATION) + .withSourceSet('release', SOURCES_WITH_HIGH_VIOLATION) + .withPenalty('''{ + maxErrors = 10 + maxWarnings = 10 + }''') + .withToolsConfig('spotbugs { ignoreFailures = false }') + .build('check') + } + + @Test + void shouldNotFailBuildWhenSpotBugsNotConfigured() { + createProjectWith() + .withSourceSet('debug', SOURCES_WITH_LOW_VIOLATION, SOURCES_WITH_MEDIUM_VIOLATION) + .withSourceSet('release', SOURCES_WITH_HIGH_VIOLATION) + .withPenalty('''{ + maxErrors = 0 + maxWarnings = 0 + }''') + .build('check') + } + + @Test + @Ignore + void shouldNotFailBuildWhenSpotBugsConfiguredToExcludePattern() { + TestProject.Result result = createProjectWith() + .withSourceSet('debug', SOURCES_WITH_LOW_VIOLATION) + .withSourceSet('release', SOURCES_WITH_HIGH_VIOLATION, SOURCES_WITH_MEDIUM_VIOLATION) + .withPenalty('''{ + maxErrors = 0 + maxWarnings = 10 + }''') + .withToolsConfig('spotbugs { exclude "com/novoda/test/HighPriorityViolator.java" }') + .build('check') + + assertThat(result.logs).doesNotContainLimitExceeded() + assertThat(result.logs).containsSpotBugsViolations(0, 2, + result.buildFileUrl('reports/spotbugs/debug.html')) + } + + @Test + @Ignore + void shouldNotFailBuildWhenSpotBugsConfiguredToExcludeFaultySourceFolder() { + TestProject.Result result = createProjectWith() + .withSourceSet('debug', SOURCES_WITH_LOW_VIOLATION, SOURCES_WITH_MEDIUM_VIOLATION) + .withSourceSet('release', SOURCES_WITH_HIGH_VIOLATION) + .withPenalty('''{ + maxErrors = 0 + maxWarnings = 10 + }''') + .withToolsConfig("spotbugs { exclude project.fileTree('${SOURCES_WITH_HIGH_VIOLATION}') }") + .build('check') + + assertThat(result.logs).doesNotContainLimitExceeded() + assertThat(result.logs).containsSpotBugsViolations(0, 2, + result.buildFileUrl('reports/spotbugs/debug.html')) + } + + @Test + @Ignore + void shouldNotFailBuildWhenSpotBugsConfiguredToIgnoreFaultyJavaSourceSets() { + TestProject project = createProjectWith() + assumeThat(project).isJavaProject() + + TestProject.Result result = project + .withSourceSet('debug', SOURCES_WITH_LOW_VIOLATION, SOURCES_WITH_MEDIUM_VIOLATION) + .withSourceSet('test', SOURCES_WITH_HIGH_VIOLATION) + .withPenalty('''{ + maxErrors = 0 + maxWarnings = 10 + }''') + .withToolsConfig('spotbugs { exclude project.sourceSets.test.java.srcDirs }') + .build('check') + + assertThat(result.logs).doesNotContainLimitExceeded() + assertThat(result.logs).containsSpotBugsViolations(0, 2, + result.buildFileUrl('reports/spotbugs/debug.html')) + } + + @Test + @Ignore + void shouldNotFailBuildWhenSpotBugsConfiguredToIgnoreFaultyAndroidSourceSets() { + TestProject project = createProjectWith() + assumeThat(project).isAndroidProject() + + TestProject.Result result = project + .withSourceSet('debug', SOURCES_WITH_LOW_VIOLATION) + .withSourceSet('test', SOURCES_WITH_HIGH_VIOLATION) + .withSourceSet('androidTest', SOURCES_WITH_HIGH_VIOLATION) + .withPenalty('''{ + maxErrors = 0 + maxWarnings = 1 + }''') + .withToolsConfig('''spotbugs { + exclude project.android.sourceSets.test.java.srcDirs + exclude project.android.sourceSets.androidTest.java.srcDirs + }''') + .build('check') + + assertThat(result.logs).doesNotContainLimitExceeded() + assertThat(result.logs).containsSpotBugsViolations(0, 1, + result.buildFileUrl('reports/spotbugs/debug.html')) + } + + @Test + @Ignore + void shouldSkipSpotBugsTasksForIgnoredFaultyAndroidSourceSets() { + TestProject project = createProjectWith() + assumeThat(project).isAndroidProject() + + TestProject.Result result = project + .withSourceSet('debug', SOURCES_WITH_LOW_VIOLATION) + .withSourceSet('test', SOURCES_WITH_HIGH_VIOLATION) + .withSourceSet('androidTest', SOURCES_WITH_HIGH_VIOLATION) + .withPenalty('''{ + maxErrors = 0 + maxWarnings = 1 + }''') + .withToolsConfig('''spotbugs { + exclude project.android.sourceSets.test.java.srcDirs + exclude project.android.sourceSets.androidTest.java.srcDirs + }''') + .build('check') + + Truth.assertThat(result.outcome(':spotbugsDebugAndroidTest')).isEqualTo(TaskOutcome.NO_SOURCE) + Truth.assertThat(result.outcome(':generateSpotBugsDebugAndroidTestHtmlReport')).isEqualTo(TaskOutcome.SKIPPED) + Truth.assertThat(result.outcome(':spotbugsDebug')).isEqualTo(TaskOutcome.SUCCESS) + Truth.assertThat(result.outcome(':generateSpotBugsDebugHtmlReport')).isEqualTo(TaskOutcome.SUCCESS) + Truth.assertThat(result.outcome(':spotbugsDebugUnitTest')).isEqualTo(TaskOutcome.NO_SOURCE) + Truth.assertThat(result.outcome(':generateSpotBugsDebugUnitTestHtmlReport')).isEqualTo(TaskOutcome.SKIPPED) + Truth.assertThat(result.outcome(':spotbugsRelease')).isEqualTo(TaskOutcome.NO_SOURCE) + Truth.assertThat(result.outcome(':generateSpotBugsReleaseHtmlReport')).isEqualTo(TaskOutcome.SKIPPED) + } + + @Test + void shouldNotFailBuildWhenSpotBugsIsConfiguredMultipleTimes() { + createProjectWith() + .withSourceSet('main', SOURCES_WITH_LOW_VIOLATION) + .withPenalty('none') + .withToolsConfig(""" + spotbugs { } + spotbugs { + ignoreFailures = false + } + """) + .build('check') + } + + @Test + void shouldBeUpToDateWhenCheckTaskRunsAgain() { + def project = createProjectWith() + .withSourceSet('debug', SOURCES_WITH_LOW_VIOLATION, SOURCES_WITH_MEDIUM_VIOLATION) + .withSourceSet('release', SOURCES_WITH_HIGH_VIOLATION) + .withPenalty('''{ + maxErrors = 10 + maxWarnings = 10 + }''') + .withToolsConfig('spotbugs {}') + + project.build('check') + + def result = project.build('check') + + Truth.assertThat(result.outcome(':spotbugsDebug')).isEqualTo(TaskOutcome.UP_TO_DATE) + Truth.assertThat(result.outcome(':generateSpotBugsDebugHtmlReport')).isEqualTo(TaskOutcome.UP_TO_DATE) + } + + @Test + @Ignore + void shouldNotGenerateHtmlWhenDisabled() { + def result = createProjectWith() + .withSourceSet('main', SOURCES_WITH_LOW_VIOLATION) + .withToolsConfig('''spotbugs { + htmlReportEnabled false + }''') + .build('check') + + Truth.assertThat(result.tasksPaths).doesNotContain(':generateSpotBugsDebugHtmlReport') + } + + private TestProject createProjectWith() { + projectRule.newProject() + .withPlugin('com.github.spotbugs', "2.0.0") + } +} diff --git a/plugin/src/test/groovy/com/novoda/test/LogsSubject.groovy b/plugin/src/test/groovy/com/novoda/test/LogsSubject.groovy index 35fb1e9..9f56360 100644 --- a/plugin/src/test/groovy/com/novoda/test/LogsSubject.groovy +++ b/plugin/src/test/groovy/com/novoda/test/LogsSubject.groovy @@ -17,6 +17,7 @@ class LogsSubject extends Subject { private static final String CHECKSTYLE_VIOLATIONS_FOUND = 'Checkstyle violations found' private static final String PMD_VIOLATIONS_FOUND = 'PMD violations found' private static final String FINDBUGS_VIOLATIONS_FOUND = 'Findbugs violations found' + private static final String SPOTBUGS_VIOLATIONS_FOUND = 'SpotBugs violations found' private static final String DETEKT_VIOLATIONS_FOUND = 'Detekt violations found' private static final String KTLINT_VIOLATIONS_FOUND = 'ktlint violations found' private static final String LINT_VIOLATIONS_FOUND = 'Lint violations found' @@ -68,6 +69,10 @@ class LogsSubject extends Subject { outputSubject.doesNotContain(FINDBUGS_VIOLATIONS_FOUND) } + public void doesNotContainSpotBugsViolations() { + outputSubject.doesNotContain(SPOTBUGS_VIOLATIONS_FOUND) + } + public void doesNotContainDetektViolations() { outputSubject.doesNotContain(DETEKT_VIOLATIONS_FOUND) } @@ -92,6 +97,10 @@ class LogsSubject extends Subject { containsToolViolations(FINDBUGS_VIOLATIONS_FOUND, errors, warnings, reportUrls) } + public void containsSpotBugsViolations(int errors, int warnings, String... reportUrls) { + containsToolViolations(SPOTBUGS_VIOLATIONS_FOUND, errors, warnings, reportUrls) + } + public void containsDetektViolations(int errors, int warnings, String... reportUrls) { containsToolViolations(DETEKT_VIOLATIONS_FOUND, errors, warnings, reportUrls) } From 2ad21104c8d2632dde56905e0a2ea1b1b9fbd32f Mon Sep 17 00:00:00 2001 From: Said Tahsin Dane Date: Tue, 22 Oct 2019 23:41:02 +0200 Subject: [PATCH 02/16] Add basic SpotBugs support for java-only projects --- .../StaticAnalysisPlugin.groovy | 2 + .../spotbugs/SpotBugsConfigurator.groovy | 102 ++++++++++++++++++ .../spotbugs/SpotBugsIntegrationTest.groovy | 16 +-- 3 files changed, 106 insertions(+), 14 deletions(-) create mode 100644 plugin/src/main/groovy/com/novoda/staticanalysis/internal/spotbugs/SpotBugsConfigurator.groovy diff --git a/plugin/src/main/groovy/com/novoda/staticanalysis/StaticAnalysisPlugin.groovy b/plugin/src/main/groovy/com/novoda/staticanalysis/StaticAnalysisPlugin.groovy index 2e3de10..36f1719 100644 --- a/plugin/src/main/groovy/com/novoda/staticanalysis/StaticAnalysisPlugin.groovy +++ b/plugin/src/main/groovy/com/novoda/staticanalysis/StaticAnalysisPlugin.groovy @@ -7,6 +7,7 @@ import com.novoda.staticanalysis.internal.findbugs.FindbugsConfigurator import com.novoda.staticanalysis.internal.ktlint.KtlintConfigurator import com.novoda.staticanalysis.internal.lint.LintConfigurator import com.novoda.staticanalysis.internal.pmd.PmdConfigurator +import com.novoda.staticanalysis.internal.spotbugs.SpotBugsConfigurator import org.gradle.api.NamedDomainObjectContainer import org.gradle.api.Plugin import org.gradle.api.Project @@ -42,6 +43,7 @@ class StaticAnalysisPlugin implements Plugin { CheckstyleConfigurator.create(project, violationsContainer, evaluateViolations), PmdConfigurator.create(project, violationsContainer, evaluateViolations), FindbugsConfigurator.create(project, violationsContainer, evaluateViolations), + SpotBugsConfigurator.create(project, violationsContainer, evaluateViolations), DetektConfigurator.create(project, violationsContainer, evaluateViolations), KtlintConfigurator.create(project, violationsContainer, evaluateViolations), LintConfigurator.create(project, violationsContainer, evaluateViolations) diff --git a/plugin/src/main/groovy/com/novoda/staticanalysis/internal/spotbugs/SpotBugsConfigurator.groovy b/plugin/src/main/groovy/com/novoda/staticanalysis/internal/spotbugs/SpotBugsConfigurator.groovy new file mode 100644 index 0000000..b000b3b --- /dev/null +++ b/plugin/src/main/groovy/com/novoda/staticanalysis/internal/spotbugs/SpotBugsConfigurator.groovy @@ -0,0 +1,102 @@ +package com.novoda.staticanalysis.internal.spotbugs + +import com.novoda.staticanalysis.StaticAnalysisExtension +import com.novoda.staticanalysis.Violations +import com.novoda.staticanalysis.internal.Configurator +import com.novoda.staticanalysis.internal.VariantFilter +import com.novoda.staticanalysis.internal.findbugs.CollectFindbugsViolationsTask +import org.gradle.api.GradleException +import org.gradle.api.NamedDomainObjectContainer +import org.gradle.api.Project +import org.gradle.api.Task + +import static com.novoda.staticanalysis.internal.Exceptions.handleException +import static com.novoda.staticanalysis.internal.TasksCompat.createTask + +class SpotBugsConfigurator implements Configurator { + + private static final String SPOTBUGS_PLUGIN = 'com.github.spotbugs' + private static final String SPOTBUGS_NOT_APPLIED = "The SpotBugs plugin is configured but not applied. Please apply the plugin: $SPOTBUGS_PLUGIN in your build script." + private static final String SPOTBUGS_CONFIGURATION_ERROR = "A problem occurred while configuring SpotBugs." + + private final Project project + private final Violations violations + private final Task evaluateViolations + private final VariantFilter variantFilter + protected boolean configured = false + + static SpotBugsConfigurator create(Project project, + NamedDomainObjectContainer violationsContainer, + Task evaluateViolations) { + Violations violations = violationsContainer.maybeCreate('SpotBugs') + return new SpotBugsConfigurator(project, violations, evaluateViolations) + } + + SpotBugsConfigurator(Project project, Violations violations, Task evaluateViolations) { + this.project = project + this.violations = violations + this.evaluateViolations = evaluateViolations + this.variantFilter = new VariantFilter(project) + } + + @Override + void execute() { + project.extensions.findByType(StaticAnalysisExtension).ext.spotbugs = { Closure config -> + if (!project.plugins.hasPlugin(SPOTBUGS_PLUGIN)) { + throw new GradleException(SPOTBUGS_NOT_APPLIED) + } + + configureSpotBugsExtension(config) + + project.plugins.withId('java') { + configureJavaProject() + } + } + } + + private void configureSpotBugsExtension(Closure config) { + try { + def spotbugs = project.spotbugs + spotbugs.ext.includeVariants = { Closure filter -> + variantFilter.includeVariantsFilter = filter + } + config.delegate = spotbugs + config.resolveStrategy = Closure.DELEGATE_FIRST + config() + spotbugs.ignoreFailures = true + } catch (Exception exception) { + handleException(SPOTBUGS_CONFIGURATION_ERROR, exception) + } + } + + protected void configureJavaProject() { + if (configured) return + + project.sourceSets.all { sourceSet -> + def collectViolations = createCollectViolations(getToolTaskNameFor(sourceSet), violations) + evaluateViolations.dependsOn collectViolations + } + configured = true + } + + private def createCollectViolations(String taskName, Violations violations) { +// if (htmlReportEnabled) { +// createHtmlReportTask(taskName) +// } + createTask(project, "collect${taskName.capitalize()}Violations", CollectFindbugsViolationsTask) { task -> + def spotbugs = project.tasks[taskName] + task.xmlReportFile = spotbugs.reports.xml.destination + task.violations = violations + +// if (htmlReportEnabled) { +// task.dependsOn project.tasks["generate${taskName.capitalize()}HtmlReport"] +// } else { + task.dependsOn spotbugs +// } + } + } + + protected static final String getToolTaskNameFor(named) { + "spotbugs${named.name.capitalize()}" + } +} diff --git a/plugin/src/test/groovy/com/novoda/staticanalysis/internal/spotbugs/SpotBugsIntegrationTest.groovy b/plugin/src/test/groovy/com/novoda/staticanalysis/internal/spotbugs/SpotBugsIntegrationTest.groovy index 6ba833e..6721aa9 100644 --- a/plugin/src/test/groovy/com/novoda/staticanalysis/internal/spotbugs/SpotBugsIntegrationTest.groovy +++ b/plugin/src/test/groovy/com/novoda/staticanalysis/internal/spotbugs/SpotBugsIntegrationTest.groovy @@ -145,18 +145,6 @@ class SpotBugsIntegrationTest { .build('check') } - @Test - void shouldNotFailBuildWhenSpotBugsNotConfigured() { - createProjectWith() - .withSourceSet('debug', SOURCES_WITH_LOW_VIOLATION, SOURCES_WITH_MEDIUM_VIOLATION) - .withSourceSet('release', SOURCES_WITH_HIGH_VIOLATION) - .withPenalty('''{ - maxErrors = 0 - maxWarnings = 0 - }''') - .build('check') - } - @Test @Ignore void shouldNotFailBuildWhenSpotBugsConfiguredToExcludePattern() { @@ -277,7 +265,7 @@ class SpotBugsIntegrationTest { .withToolsConfig(""" spotbugs { } spotbugs { - ignoreFailures = false + effort = "max" } """) .build('check') @@ -299,7 +287,7 @@ class SpotBugsIntegrationTest { def result = project.build('check') Truth.assertThat(result.outcome(':spotbugsDebug')).isEqualTo(TaskOutcome.UP_TO_DATE) - Truth.assertThat(result.outcome(':generateSpotBugsDebugHtmlReport')).isEqualTo(TaskOutcome.UP_TO_DATE) +// Truth.assertThat(result.outcome(':generateSpotBugsDebugHtmlReport')).isEqualTo(TaskOutcome.UP_TO_DATE) // todo } @Test From 5efa7a20ace64c11e0bf46543b1a127d0366ccbf Mon Sep 17 00:00:00 2001 From: Said Tahsin Dane Date: Tue, 22 Oct 2019 23:52:14 +0200 Subject: [PATCH 03/16] Add html generation support --- .../spotbugs/SpotBugsConfigurator.groovy | 29 ++++++++++++++----- .../spotbugs/SpotBugsIntegrationTest.groovy | 2 +- 2 files changed, 22 insertions(+), 9 deletions(-) diff --git a/plugin/src/main/groovy/com/novoda/staticanalysis/internal/spotbugs/SpotBugsConfigurator.groovy b/plugin/src/main/groovy/com/novoda/staticanalysis/internal/spotbugs/SpotBugsConfigurator.groovy index b000b3b..96818d0 100644 --- a/plugin/src/main/groovy/com/novoda/staticanalysis/internal/spotbugs/SpotBugsConfigurator.groovy +++ b/plugin/src/main/groovy/com/novoda/staticanalysis/internal/spotbugs/SpotBugsConfigurator.groovy @@ -5,6 +5,7 @@ import com.novoda.staticanalysis.Violations import com.novoda.staticanalysis.internal.Configurator import com.novoda.staticanalysis.internal.VariantFilter import com.novoda.staticanalysis.internal.findbugs.CollectFindbugsViolationsTask +import com.novoda.staticanalysis.internal.findbugs.GenerateFindBugsHtmlReport import org.gradle.api.GradleException import org.gradle.api.NamedDomainObjectContainer import org.gradle.api.Project @@ -23,6 +24,7 @@ class SpotBugsConfigurator implements Configurator { private final Violations violations private final Task evaluateViolations private final VariantFilter variantFilter + protected boolean htmlReportEnabled = true protected boolean configured = false static SpotBugsConfigurator create(Project project, @@ -60,6 +62,7 @@ class SpotBugsConfigurator implements Configurator { spotbugs.ext.includeVariants = { Closure filter -> variantFilter.includeVariantsFilter = filter } + spotbugs.ext.htmlReportEnabled = { boolean enabled -> this.htmlReportEnabled = enabled } config.delegate = spotbugs config.resolveStrategy = Closure.DELEGATE_FIRST config() @@ -80,23 +83,33 @@ class SpotBugsConfigurator implements Configurator { } private def createCollectViolations(String taskName, Violations violations) { -// if (htmlReportEnabled) { -// createHtmlReportTask(taskName) -// } + if (htmlReportEnabled) { + createHtmlReportTask(taskName) + } createTask(project, "collect${taskName.capitalize()}Violations", CollectFindbugsViolationsTask) { task -> def spotbugs = project.tasks[taskName] task.xmlReportFile = spotbugs.reports.xml.destination task.violations = violations -// if (htmlReportEnabled) { -// task.dependsOn project.tasks["generate${taskName.capitalize()}HtmlReport"] -// } else { + if (htmlReportEnabled) { + task.dependsOn project.tasks["generate${taskName.capitalize()}HtmlReport"] + } else { task.dependsOn spotbugs -// } + } + } + } + + private void createHtmlReportTask(String taskName) { + createTask(project, "generate${taskName.capitalize()}HtmlReport", GenerateFindBugsHtmlReport) { GenerateFindBugsHtmlReport task -> + def spotbugs = project.tasks[taskName] + task.xmlReportFile = spotbugs.reports.xml.destination + task.htmlReportFile = new File(task.xmlReportFile.absolutePath - '.xml' + '.html') + task.classpath = spotbugs.spotbugsClasspath + task.dependsOn spotbugs } } - protected static final String getToolTaskNameFor(named) { + private static String getToolTaskNameFor(named) { "spotbugs${named.name.capitalize()}" } } diff --git a/plugin/src/test/groovy/com/novoda/staticanalysis/internal/spotbugs/SpotBugsIntegrationTest.groovy b/plugin/src/test/groovy/com/novoda/staticanalysis/internal/spotbugs/SpotBugsIntegrationTest.groovy index 6721aa9..4de491a 100644 --- a/plugin/src/test/groovy/com/novoda/staticanalysis/internal/spotbugs/SpotBugsIntegrationTest.groovy +++ b/plugin/src/test/groovy/com/novoda/staticanalysis/internal/spotbugs/SpotBugsIntegrationTest.groovy @@ -287,7 +287,7 @@ class SpotBugsIntegrationTest { def result = project.build('check') Truth.assertThat(result.outcome(':spotbugsDebug')).isEqualTo(TaskOutcome.UP_TO_DATE) -// Truth.assertThat(result.outcome(':generateSpotBugsDebugHtmlReport')).isEqualTo(TaskOutcome.UP_TO_DATE) // todo + Truth.assertThat(result.outcome(':generateSpotbugsDebugHtmlReport')).isEqualTo(TaskOutcome.UP_TO_DATE) } @Test From 989c3a2682ff114305ea283653b552c246a9c6d7 Mon Sep 17 00:00:00 2001 From: Said Tahsin Dane Date: Wed, 23 Oct 2019 00:10:24 +0200 Subject: [PATCH 04/16] Add Spotbugs to core module in sample --- sample-multi-module/core/build.gradle | 7 +++++++ .../staticanalysisplugin/sample/SomeJavaClass.java | 14 ++++++++++++++ 2 files changed, 21 insertions(+) diff --git a/sample-multi-module/core/build.gradle b/sample-multi-module/core/build.gradle index b989299..be672b3 100644 --- a/sample-multi-module/core/build.gradle +++ b/sample-multi-module/core/build.gradle @@ -1,3 +1,6 @@ +plugins { + id 'com.github.spotbugs' version '2.0.0' +} apply plugin: 'java-library' apply plugin: 'kotlin' @@ -22,3 +25,7 @@ compileTestKotlin { } apply from: rootProject.file('team-props/static-analysis.gradle') + +staticAnalysis { + spotbugs {} +} diff --git a/sample-multi-module/core/src/main/java/com/novoda/staticanalysisplugin/sample/SomeJavaClass.java b/sample-multi-module/core/src/main/java/com/novoda/staticanalysisplugin/sample/SomeJavaClass.java index 2fd1406..28bd695 100644 --- a/sample-multi-module/core/src/main/java/com/novoda/staticanalysisplugin/sample/SomeJavaClass.java +++ b/sample-multi-module/core/src/main/java/com/novoda/staticanalysisplugin/sample/SomeJavaClass.java @@ -1,4 +1,18 @@ package com.novoda.staticanalysisplugin.sample; public class SomeJavaClass { + + private void THIS_IS_A_VERY_VERY_VERY_LONG_NAME_FOR_A_METHOD_IT_IS_IN_FACT_VERY_LONG_INDEED_NO_NEED_TO_COUNT_THE_NUMBER_OF_CHARACTERS_YOU_CAN_CLEARLY_SEE_THIS_IS_WAY_LONGER_THAN_IT_SHOULD(int duration) { + // no-op + } + + public static class Internal { + + public void impossibleCast() { + final Object doubleValue = Double.valueOf(1.0); + final Long value = (Long) doubleValue; + System.out.println(" - " + value); + } + + } } From 6e785e26a906d4e36adc254501a83b7d109c3a64 Mon Sep 17 00:00:00 2001 From: Said Tahsin Dane Date: Wed, 23 Oct 2019 00:20:05 +0200 Subject: [PATCH 05/16] Configure SpotBugs tasks directly to be consistent with other tools --- .../internal/spotbugs/SpotBugsConfigurator.groovy | 13 ++++++++++++- 1 file changed, 12 insertions(+), 1 deletion(-) diff --git a/plugin/src/main/groovy/com/novoda/staticanalysis/internal/spotbugs/SpotBugsConfigurator.groovy b/plugin/src/main/groovy/com/novoda/staticanalysis/internal/spotbugs/SpotBugsConfigurator.groovy index 96818d0..976aa79 100644 --- a/plugin/src/main/groovy/com/novoda/staticanalysis/internal/spotbugs/SpotBugsConfigurator.groovy +++ b/plugin/src/main/groovy/com/novoda/staticanalysis/internal/spotbugs/SpotBugsConfigurator.groovy @@ -3,6 +3,7 @@ package com.novoda.staticanalysis.internal.spotbugs import com.novoda.staticanalysis.StaticAnalysisExtension import com.novoda.staticanalysis.Violations import com.novoda.staticanalysis.internal.Configurator +import com.novoda.staticanalysis.internal.QuietLogger import com.novoda.staticanalysis.internal.VariantFilter import com.novoda.staticanalysis.internal.findbugs.CollectFindbugsViolationsTask import com.novoda.staticanalysis.internal.findbugs.GenerateFindBugsHtmlReport @@ -10,6 +11,7 @@ import org.gradle.api.GradleException import org.gradle.api.NamedDomainObjectContainer import org.gradle.api.Project import org.gradle.api.Task +import org.gradle.api.tasks.SourceTask import static com.novoda.staticanalysis.internal.Exceptions.handleException import static com.novoda.staticanalysis.internal.TasksCompat.createTask @@ -66,7 +68,6 @@ class SpotBugsConfigurator implements Configurator { config.delegate = spotbugs config.resolveStrategy = Closure.DELEGATE_FIRST config() - spotbugs.ignoreFailures = true } catch (Exception exception) { handleException(SPOTBUGS_CONFIGURATION_ERROR, exception) } @@ -88,6 +89,7 @@ class SpotBugsConfigurator implements Configurator { } createTask(project, "collect${taskName.capitalize()}Violations", CollectFindbugsViolationsTask) { task -> def spotbugs = project.tasks[taskName] + configureToolTask(spotbugs) task.xmlReportFile = spotbugs.reports.xml.destination task.violations = violations @@ -109,6 +111,15 @@ class SpotBugsConfigurator implements Configurator { } } + private void configureToolTask(SourceTask task) { + task.group = 'verification' + task.exclude '**/*.kt' + task.ignoreFailures = true + task.metaClass.getLogger = { QuietLogger.INSTANCE } + task.reports.xml.enabled = true + task.reports.html.enabled = false + } + private static String getToolTaskNameFor(named) { "spotbugs${named.name.capitalize()}" } From ae7e1e4e801c98c8d84f353063c52c78b5845b1f Mon Sep 17 00:00:00 2001 From: Said Tahsin Dane Date: Wed, 23 Oct 2019 09:42:26 +0200 Subject: [PATCH 06/16] Configure SpotBugs for Android projects --- .../spotbugs/SpotBugsConfigurator.groovy | 67 ++++++++++++++++--- .../spotbugs/SpotBugsIntegrationTest.groovy | 2 +- 2 files changed, 58 insertions(+), 11 deletions(-) diff --git a/plugin/src/main/groovy/com/novoda/staticanalysis/internal/spotbugs/SpotBugsConfigurator.groovy b/plugin/src/main/groovy/com/novoda/staticanalysis/internal/spotbugs/SpotBugsConfigurator.groovy index 976aa79..d5e12e3 100644 --- a/plugin/src/main/groovy/com/novoda/staticanalysis/internal/spotbugs/SpotBugsConfigurator.groovy +++ b/plugin/src/main/groovy/com/novoda/staticanalysis/internal/spotbugs/SpotBugsConfigurator.groovy @@ -7,10 +7,7 @@ import com.novoda.staticanalysis.internal.QuietLogger import com.novoda.staticanalysis.internal.VariantFilter import com.novoda.staticanalysis.internal.findbugs.CollectFindbugsViolationsTask import com.novoda.staticanalysis.internal.findbugs.GenerateFindBugsHtmlReport -import org.gradle.api.GradleException -import org.gradle.api.NamedDomainObjectContainer -import org.gradle.api.Project -import org.gradle.api.Task +import org.gradle.api.* import org.gradle.api.tasks.SourceTask import static com.novoda.staticanalysis.internal.Exceptions.handleException @@ -30,8 +27,8 @@ class SpotBugsConfigurator implements Configurator { protected boolean configured = false static SpotBugsConfigurator create(Project project, - NamedDomainObjectContainer violationsContainer, - Task evaluateViolations) { + NamedDomainObjectContainer violationsContainer, + Task evaluateViolations) { Violations violations = violationsContainer.maybeCreate('SpotBugs') return new SpotBugsConfigurator(project, violations, evaluateViolations) } @@ -42,7 +39,7 @@ class SpotBugsConfigurator implements Configurator { this.evaluateViolations = evaluateViolations this.variantFilter = new VariantFilter(project) } - + @Override void execute() { project.extensions.findByType(StaticAnalysisExtension).ext.spotbugs = { Closure config -> @@ -52,6 +49,12 @@ class SpotBugsConfigurator implements Configurator { configureSpotBugsExtension(config) + project.plugins.withId('com.android.application') { + configureAndroidWithVariants(variantFilter.filteredApplicationVariants) + } + project.plugins.withId('com.android.library') { + configureAndroidWithVariants(variantFilter.filteredLibraryVariants) + } project.plugins.withId('java') { configureJavaProject() } @@ -73,7 +76,39 @@ class SpotBugsConfigurator implements Configurator { } } - protected void configureJavaProject() { + protected void configureAndroidWithVariants(DomainObjectSet variants) { + if (configured) return + + variants.all { configureVariant(it) } + variantFilter.filteredTestVariants.all { configureVariant(it) } + variantFilter.filteredUnitTestVariants.all { configureVariant(it) } + configured = true + } + + private void configureVariant(variant) { + createToolTaskForAndroid(variant) + def collectViolations = createCollectViolations(getToolTaskNameFor(variant), violations) + evaluateViolations.dependsOn collectViolations + } + + private void createToolTaskForAndroid(variant) { + createTask(project, getToolTaskNameFor(variant), Class.forName('com.github.spotbugs.SpotBugsTask')) { task -> + def javaCompile = javaCompile(variant) + def androidSourceDirs = variant.sourceSets.collect { + it.javaDirectories + }.flatten() as List + task.description = "Run SpotBugs analysis for ${variant.name} classes" + task.source = androidSourceDirs + task.classpath = javaCompile.classpath + task.extraArgs '-auxclasspath', androidJar + task.conventionMapping.map("classes") { + project.fileTree(javaCompile.destinationDir) + } + task.dependsOn javaCompile + } + } + + private void configureJavaProject() { if (configured) return project.sourceSets.all { sourceSet -> @@ -88,7 +123,7 @@ class SpotBugsConfigurator implements Configurator { createHtmlReportTask(taskName) } createTask(project, "collect${taskName.capitalize()}Violations", CollectFindbugsViolationsTask) { task -> - def spotbugs = project.tasks[taskName] + def spotbugs = project.tasks[taskName] as SourceTask configureToolTask(spotbugs) task.xmlReportFile = spotbugs.reports.xml.destination task.violations = violations @@ -111,7 +146,7 @@ class SpotBugsConfigurator implements Configurator { } } - private void configureToolTask(SourceTask task) { + private static void configureToolTask(SourceTask task) { task.group = 'verification' task.exclude '**/*.kt' task.ignoreFailures = true @@ -123,4 +158,16 @@ class SpotBugsConfigurator implements Configurator { private static String getToolTaskNameFor(named) { "spotbugs${named.name.capitalize()}" } + + private static def javaCompile(variant) { + if (variant.hasProperty('javaCompileProvider')) { + variant.javaCompileProvider.get() + } else { + variant.javaCompile + } + } + + private def getAndroidJar() { + "${project.android.sdkDirectory}/platforms/${project.android.compileSdkVersion}/android.jar" + } } diff --git a/plugin/src/test/groovy/com/novoda/staticanalysis/internal/spotbugs/SpotBugsIntegrationTest.groovy b/plugin/src/test/groovy/com/novoda/staticanalysis/internal/spotbugs/SpotBugsIntegrationTest.groovy index 4de491a..ffcbc3a 100644 --- a/plugin/src/test/groovy/com/novoda/staticanalysis/internal/spotbugs/SpotBugsIntegrationTest.groovy +++ b/plugin/src/test/groovy/com/novoda/staticanalysis/internal/spotbugs/SpotBugsIntegrationTest.groovy @@ -19,7 +19,7 @@ class SpotBugsIntegrationTest { @Parameterized.Parameters(name = "{0}") static Iterable rules() { - return [TestProjectRule.forJavaProject()/*, TestProjectRule.forAndroidProject()*/] + return [TestProjectRule.forJavaProject(), TestProjectRule.forAndroidProject()] } @Rule From aa12c44a6f87093b356f78e9a4cc561b8c9d7969 Mon Sep 17 00:00:00 2001 From: Said Tahsin Dane Date: Wed, 23 Oct 2019 10:56:45 +0200 Subject: [PATCH 07/16] Remove exclude tests since it is not easy to have the same support. The office `excludeFilter` with an xml file can be used --- .../spotbugs/SpotBugsConfigurator.groovy | 6 +- .../spotbugs/SpotBugsIntegrationTest.groovy | 115 ------------------ 2 files changed, 3 insertions(+), 118 deletions(-) diff --git a/plugin/src/main/groovy/com/novoda/staticanalysis/internal/spotbugs/SpotBugsConfigurator.groovy b/plugin/src/main/groovy/com/novoda/staticanalysis/internal/spotbugs/SpotBugsConfigurator.groovy index d5e12e3..dab9fbe 100644 --- a/plugin/src/main/groovy/com/novoda/staticanalysis/internal/spotbugs/SpotBugsConfigurator.groovy +++ b/plugin/src/main/groovy/com/novoda/staticanalysis/internal/spotbugs/SpotBugsConfigurator.groovy @@ -92,13 +92,13 @@ class SpotBugsConfigurator implements Configurator { } private void createToolTaskForAndroid(variant) { - createTask(project, getToolTaskNameFor(variant), Class.forName('com.github.spotbugs.SpotBugsTask')) { task -> + createTask(project, getToolTaskNameFor(variant), Class.forName('com.github.spotbugs.SpotBugsTask')) { SourceTask task -> def javaCompile = javaCompile(variant) def androidSourceDirs = variant.sourceSets.collect { it.javaDirectories - }.flatten() as List + }.flatten() task.description = "Run SpotBugs analysis for ${variant.name} classes" - task.source = androidSourceDirs + task.setSource(androidSourceDirs) task.classpath = javaCompile.classpath task.extraArgs '-auxclasspath', androidJar task.conventionMapping.map("classes") { diff --git a/plugin/src/test/groovy/com/novoda/staticanalysis/internal/spotbugs/SpotBugsIntegrationTest.groovy b/plugin/src/test/groovy/com/novoda/staticanalysis/internal/spotbugs/SpotBugsIntegrationTest.groovy index ffcbc3a..b3df238 100644 --- a/plugin/src/test/groovy/com/novoda/staticanalysis/internal/spotbugs/SpotBugsIntegrationTest.groovy +++ b/plugin/src/test/groovy/com/novoda/staticanalysis/internal/spotbugs/SpotBugsIntegrationTest.groovy @@ -4,7 +4,6 @@ import com.google.common.truth.Truth import com.novoda.test.TestProject import com.novoda.test.TestProjectRule import org.gradle.testkit.runner.TaskOutcome -import org.junit.Ignore import org.junit.Rule import org.junit.Test import org.junit.runner.RunWith @@ -12,7 +11,6 @@ import org.junit.runners.Parameterized import static com.novoda.test.Fixtures.Findbugs.* import static com.novoda.test.LogsSubject.assertThat -import static com.novoda.test.TestProjectSubject.assumeThat @RunWith(Parameterized.class) class SpotBugsIntegrationTest { @@ -145,118 +143,6 @@ class SpotBugsIntegrationTest { .build('check') } - @Test - @Ignore - void shouldNotFailBuildWhenSpotBugsConfiguredToExcludePattern() { - TestProject.Result result = createProjectWith() - .withSourceSet('debug', SOURCES_WITH_LOW_VIOLATION) - .withSourceSet('release', SOURCES_WITH_HIGH_VIOLATION, SOURCES_WITH_MEDIUM_VIOLATION) - .withPenalty('''{ - maxErrors = 0 - maxWarnings = 10 - }''') - .withToolsConfig('spotbugs { exclude "com/novoda/test/HighPriorityViolator.java" }') - .build('check') - - assertThat(result.logs).doesNotContainLimitExceeded() - assertThat(result.logs).containsSpotBugsViolations(0, 2, - result.buildFileUrl('reports/spotbugs/debug.html')) - } - - @Test - @Ignore - void shouldNotFailBuildWhenSpotBugsConfiguredToExcludeFaultySourceFolder() { - TestProject.Result result = createProjectWith() - .withSourceSet('debug', SOURCES_WITH_LOW_VIOLATION, SOURCES_WITH_MEDIUM_VIOLATION) - .withSourceSet('release', SOURCES_WITH_HIGH_VIOLATION) - .withPenalty('''{ - maxErrors = 0 - maxWarnings = 10 - }''') - .withToolsConfig("spotbugs { exclude project.fileTree('${SOURCES_WITH_HIGH_VIOLATION}') }") - .build('check') - - assertThat(result.logs).doesNotContainLimitExceeded() - assertThat(result.logs).containsSpotBugsViolations(0, 2, - result.buildFileUrl('reports/spotbugs/debug.html')) - } - - @Test - @Ignore - void shouldNotFailBuildWhenSpotBugsConfiguredToIgnoreFaultyJavaSourceSets() { - TestProject project = createProjectWith() - assumeThat(project).isJavaProject() - - TestProject.Result result = project - .withSourceSet('debug', SOURCES_WITH_LOW_VIOLATION, SOURCES_WITH_MEDIUM_VIOLATION) - .withSourceSet('test', SOURCES_WITH_HIGH_VIOLATION) - .withPenalty('''{ - maxErrors = 0 - maxWarnings = 10 - }''') - .withToolsConfig('spotbugs { exclude project.sourceSets.test.java.srcDirs }') - .build('check') - - assertThat(result.logs).doesNotContainLimitExceeded() - assertThat(result.logs).containsSpotBugsViolations(0, 2, - result.buildFileUrl('reports/spotbugs/debug.html')) - } - - @Test - @Ignore - void shouldNotFailBuildWhenSpotBugsConfiguredToIgnoreFaultyAndroidSourceSets() { - TestProject project = createProjectWith() - assumeThat(project).isAndroidProject() - - TestProject.Result result = project - .withSourceSet('debug', SOURCES_WITH_LOW_VIOLATION) - .withSourceSet('test', SOURCES_WITH_HIGH_VIOLATION) - .withSourceSet('androidTest', SOURCES_WITH_HIGH_VIOLATION) - .withPenalty('''{ - maxErrors = 0 - maxWarnings = 1 - }''') - .withToolsConfig('''spotbugs { - exclude project.android.sourceSets.test.java.srcDirs - exclude project.android.sourceSets.androidTest.java.srcDirs - }''') - .build('check') - - assertThat(result.logs).doesNotContainLimitExceeded() - assertThat(result.logs).containsSpotBugsViolations(0, 1, - result.buildFileUrl('reports/spotbugs/debug.html')) - } - - @Test - @Ignore - void shouldSkipSpotBugsTasksForIgnoredFaultyAndroidSourceSets() { - TestProject project = createProjectWith() - assumeThat(project).isAndroidProject() - - TestProject.Result result = project - .withSourceSet('debug', SOURCES_WITH_LOW_VIOLATION) - .withSourceSet('test', SOURCES_WITH_HIGH_VIOLATION) - .withSourceSet('androidTest', SOURCES_WITH_HIGH_VIOLATION) - .withPenalty('''{ - maxErrors = 0 - maxWarnings = 1 - }''') - .withToolsConfig('''spotbugs { - exclude project.android.sourceSets.test.java.srcDirs - exclude project.android.sourceSets.androidTest.java.srcDirs - }''') - .build('check') - - Truth.assertThat(result.outcome(':spotbugsDebugAndroidTest')).isEqualTo(TaskOutcome.NO_SOURCE) - Truth.assertThat(result.outcome(':generateSpotBugsDebugAndroidTestHtmlReport')).isEqualTo(TaskOutcome.SKIPPED) - Truth.assertThat(result.outcome(':spotbugsDebug')).isEqualTo(TaskOutcome.SUCCESS) - Truth.assertThat(result.outcome(':generateSpotBugsDebugHtmlReport')).isEqualTo(TaskOutcome.SUCCESS) - Truth.assertThat(result.outcome(':spotbugsDebugUnitTest')).isEqualTo(TaskOutcome.NO_SOURCE) - Truth.assertThat(result.outcome(':generateSpotBugsDebugUnitTestHtmlReport')).isEqualTo(TaskOutcome.SKIPPED) - Truth.assertThat(result.outcome(':spotbugsRelease')).isEqualTo(TaskOutcome.NO_SOURCE) - Truth.assertThat(result.outcome(':generateSpotBugsReleaseHtmlReport')).isEqualTo(TaskOutcome.SKIPPED) - } - @Test void shouldNotFailBuildWhenSpotBugsIsConfiguredMultipleTimes() { createProjectWith() @@ -291,7 +177,6 @@ class SpotBugsIntegrationTest { } @Test - @Ignore void shouldNotGenerateHtmlWhenDisabled() { def result = createProjectWith() .withSourceSet('main', SOURCES_WITH_LOW_VIOLATION) From 11aa572781e755a193e4fb710dfca59ab5015e3a Mon Sep 17 00:00:00 2001 From: Said Tahsin Dane Date: Wed, 23 Oct 2019 10:58:03 +0200 Subject: [PATCH 08/16] Remove quite logger since it is already quite --- .../internal/spotbugs/SpotBugsConfigurator.groovy | 2 -- 1 file changed, 2 deletions(-) diff --git a/plugin/src/main/groovy/com/novoda/staticanalysis/internal/spotbugs/SpotBugsConfigurator.groovy b/plugin/src/main/groovy/com/novoda/staticanalysis/internal/spotbugs/SpotBugsConfigurator.groovy index dab9fbe..75b199d 100644 --- a/plugin/src/main/groovy/com/novoda/staticanalysis/internal/spotbugs/SpotBugsConfigurator.groovy +++ b/plugin/src/main/groovy/com/novoda/staticanalysis/internal/spotbugs/SpotBugsConfigurator.groovy @@ -3,7 +3,6 @@ package com.novoda.staticanalysis.internal.spotbugs import com.novoda.staticanalysis.StaticAnalysisExtension import com.novoda.staticanalysis.Violations import com.novoda.staticanalysis.internal.Configurator -import com.novoda.staticanalysis.internal.QuietLogger import com.novoda.staticanalysis.internal.VariantFilter import com.novoda.staticanalysis.internal.findbugs.CollectFindbugsViolationsTask import com.novoda.staticanalysis.internal.findbugs.GenerateFindBugsHtmlReport @@ -150,7 +149,6 @@ class SpotBugsConfigurator implements Configurator { task.group = 'verification' task.exclude '**/*.kt' task.ignoreFailures = true - task.metaClass.getLogger = { QuietLogger.INSTANCE } task.reports.xml.enabled = true task.reports.html.enabled = false } From c2a65d344fcaa1061897df2981d3ef59c618a5d2 Mon Sep 17 00:00:00 2001 From: Said Tahsin Dane Date: Thu, 24 Oct 2019 16:34:29 +0200 Subject: [PATCH 09/16] Add spotbugs into samples --- sample-multi-module/build.gradle | 1 + sample-multi-module/core/build.gradle | 7 ------- sample-multi-module/team-props/static-analysis.gradle | 6 ++++++ sample/app/build.gradle | 8 +++++++- sample/build.gradle | 1 + 5 files changed, 15 insertions(+), 8 deletions(-) diff --git a/sample-multi-module/build.gradle b/sample-multi-module/build.gradle index 73afee0..61e4532 100644 --- a/sample-multi-module/build.gradle +++ b/sample-multi-module/build.gradle @@ -11,6 +11,7 @@ buildscript { classpath 'com.novoda:gradle-static-analysis-plugin:local' classpath 'io.gitlab.arturbosch.detekt:detekt-gradle-plugin:1.0.0-RC14' classpath 'org.jlleitschuh.gradle:ktlint-gradle:9.0.0' + classpath "gradle.plugin.com.github.spotbugs:spotbugs-gradle-plugin:2.0.0" classpath "org.jetbrains.kotlin:kotlin-gradle-plugin:$kotlin_version" } } diff --git a/sample-multi-module/core/build.gradle b/sample-multi-module/core/build.gradle index be672b3..b989299 100644 --- a/sample-multi-module/core/build.gradle +++ b/sample-multi-module/core/build.gradle @@ -1,6 +1,3 @@ -plugins { - id 'com.github.spotbugs' version '2.0.0' -} apply plugin: 'java-library' apply plugin: 'kotlin' @@ -25,7 +22,3 @@ compileTestKotlin { } apply from: rootProject.file('team-props/static-analysis.gradle') - -staticAnalysis { - spotbugs {} -} diff --git a/sample-multi-module/team-props/static-analysis.gradle b/sample-multi-module/team-props/static-analysis.gradle index 3d7b7c7..aefa67f 100644 --- a/sample-multi-module/team-props/static-analysis.gradle +++ b/sample-multi-module/team-props/static-analysis.gradle @@ -1,6 +1,7 @@ apply plugin: 'com.novoda.static-analysis' apply plugin: 'io.gitlab.arturbosch.detekt' apply plugin: 'org.jlleitschuh.gradle.ktlint' +apply plugin: 'com.github.spotbugs' staticAnalysis { @@ -27,6 +28,11 @@ staticAnalysis { includeVariants { variant -> variant.name.contains('debug') } } + spotbugs { + excludeFilter rootProject.file('team-props/findbugs-excludes.xml') + includeVariants { variant -> variant.name.contains('debug') } + } + lintOptions { lintConfig rootProject.file('team-props/lint-config.xml') checkReleaseBuilds false diff --git a/sample/app/build.gradle b/sample/app/build.gradle index a49ab7f..320a155 100755 --- a/sample/app/build.gradle +++ b/sample/app/build.gradle @@ -1,6 +1,7 @@ plugins { id 'io.gitlab.arturbosch.detekt' - id "org.jlleitschuh.gradle.ktlint" + id 'org.jlleitschuh.gradle.ktlint' + id 'com.github.spotbugs' } apply plugin: 'com.android.application' @@ -51,6 +52,11 @@ staticAnalysis { includeVariants { variant -> variant.name.contains('debug') } } + spotbugs { + excludeFilter rootProject.file('team-props/findbugs-excludes.xml') + includeVariants { variant -> variant.name.contains('debug') } + } + lintOptions { lintConfig rootProject.file('team-props/lint-config.xml') checkReleaseBuilds false diff --git a/sample/build.gradle b/sample/build.gradle index a784b83..a2f6efa 100755 --- a/sample/build.gradle +++ b/sample/build.gradle @@ -11,6 +11,7 @@ buildscript { classpath 'com.novoda:gradle-static-analysis-plugin:local' classpath 'io.gitlab.arturbosch.detekt:detekt-gradle-plugin:1.0.0-RC14' classpath 'org.jlleitschuh.gradle:ktlint-gradle:9.0.0' + classpath "gradle.plugin.com.github.spotbugs:spotbugs-gradle-plugin:2.0.0" classpath "org.jetbrains.kotlin:kotlin-gradle-plugin:$kotlin_version" } } From 48d81783513f6fb761815871fb0c5550e3497789 Mon Sep 17 00:00:00 2001 From: Said Tahsin Dane Date: Thu, 24 Oct 2019 16:41:59 +0200 Subject: [PATCH 10/16] Add spotbugs into docs --- README.md | 5 +++-- docs/supported-tools.md | 3 +++ docs/tools/spotbugs.md | 29 +++++++++++++++++++++++++++++ 3 files changed, 35 insertions(+), 2 deletions(-) create mode 100644 docs/tools/spotbugs.md diff --git a/README.md b/README.md index fa05371..b718444 100644 --- a/README.md +++ b/README.md @@ -22,6 +22,7 @@ The plugin supports various static analysis tools for Java, Kotlin and Android p * [`Checkstyle`](docs/tools/checkstyle.md) * [`PMD`](docs/tools/pmd.md) * [`FindBugs`](docs/tools/findbugs.md) + * [`SpotBugs`](docs/tools/spotbugs.md) * [`Detekt`](docs/tools/detekt.md) * [`Android Lint`](docs/tools/android_lint.md) * [`KtLint`](docs/tools/ktlint.md) @@ -31,7 +32,6 @@ Please note that the tools availability depends on the project the plugin is app ### Tools in-consideration - * `Spotbugs` [#142](https://github.com/novoda/gradle-static-analysis-plugin/issues/142) * `CPD (Duplicate Code Detection) ` [#150](https://github.com/novoda/gradle-static-analysis-plugin/iss (Duplicate Code Detection) ues/150) * `error-prone` [#151](https://github.com/novoda/gradle-static-analysis-plugin/issues/151) * `Jetbrains IDEA Inspections` [#152](https://github.com/novoda/gradle-static-analysis-plugin/issues/152) @@ -80,6 +80,7 @@ staticAnalysis { checkstyle { } pmd { } findbugs { } + spotbugs { } detekt { } lintOptions { } } @@ -89,7 +90,7 @@ This will enable all the tools with their default settings. For more advanced co [advanced usage](docs/advanced-usage.md) and to the [supported tools](docs/supported-tools.md) pages. ## Sample app -There are two sample Android projects available, one consisting of a regular app - available [here](https://github.com/novoda/gradle-static-analysis-plugin/tree/master/sample) - and the other comprising a multi-module setup available [here](https://github.com/novoda/gradle-static-analysis-plugin/tree/master/sample-multi-module). Both sample projects showcase a setup featuring Checkstyle, FindBugs, PMD, Lint and Detekt. +There are two sample Android projects available, one consisting of a regular app - available [here](https://github.com/novoda/gradle-static-analysis-plugin/tree/master/sample) - and the other comprising a multi-module setup available [here](https://github.com/novoda/gradle-static-analysis-plugin/tree/master/sample-multi-module). Both sample projects showcase a setup featuring Checkstyle, FindBugs, SpotBugs, PMD, Lint, Ktlint and Detekt. ## Snapshots [![CI status](https://ci.novoda.com/buildStatus/icon?job=gradle-static-analysis-plugin-snapshot)](https://ci.novoda.com/job/gradle-static-analysis-plugin-snapshot/lastBuild/console) [![Download from Bintray](https://api.bintray.com/packages/novoda-oss/snapshots/gradle-static-analysis-plugin/images/download.svg)](https://bintray.com/novoda-oss/snapshots/gradle-static-analysis-plugin/_latestVersion) diff --git a/docs/supported-tools.md b/docs/supported-tools.md index 40905bd..8cff379 100644 --- a/docs/supported-tools.md +++ b/docs/supported-tools.md @@ -8,6 +8,7 @@ Tool | Java | Android
(Java) | Kotlin | Android
(Kotlin) [`Checkstyle`](https://checkstyle.sourceforge.net) | :white_check_mark: | :white_check_mark: | — | — [`PMD`](https://pmd.github.io) | :white_check_mark: | :white_check_mark: | — | — [`FindBugs`](http://findbugs.sourceforge.net/) | :white_check_mark: | :white_check_mark: | — | — +[`SpotBugs`](https://spotbugs.github.io/) | :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) | — | — | :white_check_mark:️ | :white_check_mark:️ @@ -22,6 +23,7 @@ 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) + * [SpotBugs](tools/spotbugs.md) * [Android Lint](tools/android_lint.md) * [KtLint](tools/ktlint.md) * [Example configurations](#example-configurations) @@ -40,6 +42,7 @@ staticAnalysis { checkstyle {} pmd {} findbugs {} + spotbugs {} lintOptions {} detekt {} ktlint {} diff --git a/docs/tools/spotbugs.md b/docs/tools/spotbugs.md new file mode 100644 index 0000000..2303074 --- /dev/null +++ b/docs/tools/spotbugs.md @@ -0,0 +1,29 @@ +# SpotBugs +[SpotBugs](https://spotbugs.github.io/) is a static analysis tool that looks for potential bugs in Java code. It does not support Kotlin. +It can be used in both pure Java, and Android Java projects. It then only makes sense to have SpotBugs enabled if you have Java code in your project. +The plugin only runs SpotBugs on projects that contain the Java or the Android plugin. + +## Table of contents + * [Configure SpotBugs](#configure-spotbugs) + * [SpotBugs in mixed-language projects](#spotbugs-in-mixed-language-projects) + +--- + +## Configure SpotBugs +Enabling and configuring SpotBugs for a project is done through the `spotbugs` closure: + +```gradle +spotbugs { + toolVersion // Optional string, the latest SpotBugs release (currently 4.0.0-beta4) + excludeFilter // A file containing the SpotBugs exclusions, e.g., teamPropsFile('static-analysis/spotbugs-excludes.xml') + htmlReportEnabled true // Control whether html report generation should be enabled. `true` by default. + includeVariants { variant -> ... } // A closure to determine which variants (only for Android) to include +} +``` + +(assuming you're using the Novoda scaffolding system, see [Example configurations](#example-configurations) for more details) + +For more information about SpotBugs rules, refer to the [official website](https://spotbugs.readthedocs.io/en/latest/bugDescriptions.html). + +## SpotBugs in mixed-language projects +If your project mixes Java and Kotlin code, you will need to exclude your Kotlin files by using `excludeFilter` From e31256953cb92b9a41d0fb71a40d99169c0778db Mon Sep 17 00:00:00 2001 From: Said Tahsin Dane Date: Fri, 8 Nov 2019 12:52:41 +0100 Subject: [PATCH 11/16] Fix AGP warning. JavaCompile is deprecated --- .../internal/findbugs/FindbugsConfigurator.groovy | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) 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 74e736b..af35106 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,6 @@ package com.novoda.staticanalysis.internal.findbugs import com.novoda.staticanalysis.Violations import com.novoda.staticanalysis.internal.CodeQualityConfigurator -import com.novoda.staticanalysis.internal.CollectViolationsTask import org.gradle.api.Action import org.gradle.api.DomainObjectSet import org.gradle.api.NamedDomainObjectContainer @@ -87,7 +86,7 @@ class FindbugsConfigurator extends CodeQualityConfigurator androidSourceDirs = variant.sourceSets.collect { it.javaDirectories }.flatten() task.description = "Run FindBugs analysis for ${variant.name} classes" task.source = androidSourceDirs - task.classpath = variant.javaCompile.classpath + task.classpath = javaCompile(variant).classpath task.extraArgs '-auxclasspath', androidJar task.conventionMapping.map("classes") { List includes = createIncludePatterns(task.source, androidSourceDirs) From 239a29e4f5ccec707cdd7efa9bcd1db0549a7e2b Mon Sep 17 00:00:00 2001 From: Said Tahsin Dane Date: Fri, 8 Nov 2019 13:13:54 +0100 Subject: [PATCH 12/16] In order to reproduce #199, added kotlin-kapt to all kotlin test projects. Added special configuration-only tests that also include kotlin projects even when the tool is Kotlin only. --- .../CheckstyleConfigurationTest.groovy | 52 +++++++++++++++++++ .../CheckstyleIntegrationTest.groovy | 16 ------ .../findbugs/FindbugsConfigurationTest.groovy | 52 +++++++++++++++++++ .../findbugs/FindbugsIntegrationTest.groovy | 16 +----- .../internal/pmd/PmdConfigurationTest.groovy | 51 ++++++++++++++++++ .../internal/pmd/PmdIntegrationTest.groovy | 16 ------ .../test/TestAndroidKotlinProject.groovy | 1 + .../com/novoda/test/TestKotlinProject.groovy | 1 + sample-multi-module/core/build.gradle | 5 +- sample/app/build.gradle | 1 + 10 files changed, 162 insertions(+), 49 deletions(-) create mode 100644 plugin/src/test/groovy/com/novoda/staticanalysis/internal/checkstyle/CheckstyleConfigurationTest.groovy create mode 100644 plugin/src/test/groovy/com/novoda/staticanalysis/internal/findbugs/FindbugsConfigurationTest.groovy create mode 100644 plugin/src/test/groovy/com/novoda/staticanalysis/internal/pmd/PmdConfigurationTest.groovy diff --git a/plugin/src/test/groovy/com/novoda/staticanalysis/internal/checkstyle/CheckstyleConfigurationTest.groovy b/plugin/src/test/groovy/com/novoda/staticanalysis/internal/checkstyle/CheckstyleConfigurationTest.groovy new file mode 100644 index 0000000..32dd029 --- /dev/null +++ b/plugin/src/test/groovy/com/novoda/staticanalysis/internal/checkstyle/CheckstyleConfigurationTest.groovy @@ -0,0 +1,52 @@ +package com.novoda.staticanalysis.internal.checkstyle + +import com.novoda.test.Fixtures +import com.novoda.test.TestProjectRule +import org.junit.Rule +import org.junit.Test +import org.junit.runner.RunWith +import org.junit.runners.Parameterized + +@RunWith(Parameterized.class) +class CheckstyleConfigurationTest { + + @Parameterized.Parameters(name = "{0}") + static Iterable rules() { + return [ + TestProjectRule.forJavaProject(), + TestProjectRule.forKotlinProject(), + TestProjectRule.forAndroidProject(), + TestProjectRule.forAndroidKotlinProject(), + ] + } + + @Rule + public final TestProjectRule projectRule + + CheckstyleConfigurationTest(TestProjectRule projectRule) { + this.projectRule = projectRule + } + + @Test + void shouldConfigureSuccessFully() { + projectRule.newProject() + .withSourceSet('main', Fixtures.Checkstyle.SOURCES_WITH_WARNINGS) + .withToolsConfig("checkstyle { }") + .build('check', '--dry-run') + } + + @Test + void shouldNotFailBuildWhenCheckstyleIsConfiguredMultipleTimes() { + projectRule.newProject() + .withSourceSet('main', Fixtures.Checkstyle.SOURCES_WITH_WARNINGS) + .withToolsConfig(""" + checkstyle { + configFile new File('${Fixtures.Checkstyle.MODULES.path}') + } + checkstyle { + ignoreFailures = false + } + """) + .build('check', '--dry-run') + } +} diff --git a/plugin/src/test/groovy/com/novoda/staticanalysis/internal/checkstyle/CheckstyleIntegrationTest.groovy b/plugin/src/test/groovy/com/novoda/staticanalysis/internal/checkstyle/CheckstyleIntegrationTest.groovy index 8a07151..1f1cdbf 100644 --- a/plugin/src/test/groovy/com/novoda/staticanalysis/internal/checkstyle/CheckstyleIntegrationTest.groovy +++ b/plugin/src/test/groovy/com/novoda/staticanalysis/internal/checkstyle/CheckstyleIntegrationTest.groovy @@ -229,22 +229,6 @@ public class CheckstyleIntegrationTest { result.buildFileUrl('reports/checkstyle/main.html')) } - @Test - void shouldNotFailBuildWhenCheckstyleIsConfiguredMultipleTimes() { - projectRule.newProject() - .withSourceSet('main', Fixtures.Checkstyle.SOURCES_WITH_WARNINGS) - .withPenalty('none') - .withToolsConfig(""" - checkstyle { - configFile new File('${Fixtures.Checkstyle.MODULES.path}') - } - checkstyle { - ignoreFailures = false - } - """) - .build('check', '--dry-run') - } - private static String checkstyle(String configFile, String... configs) { """checkstyle { ${configFile} diff --git a/plugin/src/test/groovy/com/novoda/staticanalysis/internal/findbugs/FindbugsConfigurationTest.groovy b/plugin/src/test/groovy/com/novoda/staticanalysis/internal/findbugs/FindbugsConfigurationTest.groovy new file mode 100644 index 0000000..07d4671 --- /dev/null +++ b/plugin/src/test/groovy/com/novoda/staticanalysis/internal/findbugs/FindbugsConfigurationTest.groovy @@ -0,0 +1,52 @@ +package com.novoda.staticanalysis.internal.findbugs + + +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.Fixtures.Findbugs.SOURCES_WITH_LOW_VIOLATION + +@RunWith(Parameterized.class) +class FindbugsConfigurationTest { + + @Parameterized.Parameters(name = "{0}") + static Iterable rules() { + return [ + TestProjectRule.forJavaProject(), + TestProjectRule.forKotlinProject(), + TestProjectRule.forAndroidProject(), + TestProjectRule.forAndroidKotlinProject(), + ] + } + + @Rule + public final TestProjectRule projectRule + + FindbugsConfigurationTest(TestProjectRule projectRule) { + this.projectRule = projectRule + } + + @Test + void shouldConfigureSuccessFully() { + projectRule.newProject() + .withSourceSet('main', SOURCES_WITH_LOW_VIOLATION) + .withToolsConfig("findbugs { }") + .build('check', '--dry-run') + } + + @Test + void shouldNotFailBuildWhenFindbugsIsConfiguredMultipleTimes() { + projectRule.newProject() + .withSourceSet('main', SOURCES_WITH_LOW_VIOLATION) + .withToolsConfig(""" + findbugs { } + findbugs { + ignoreFailures = false + } + """) + .build('check', '--dry-run') + } +} 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 1089d7c..42199ed 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 @@ -346,20 +346,6 @@ class FindbugsIntegrationTest { Truth.assertThat(result.outcome(':checkFindbugsClasses')).isEqualTo(TaskOutcome.SUCCESS) } - @Test - void shouldNotFailBuildWhenFindbugsIsConfiguredMultipleTimes() { - projectRule.newProject() - .withSourceSet('main', SOURCES_WITH_LOW_VIOLATION) - .withPenalty('none') - .withToolsConfig(""" - findbugs { } - findbugs { - ignoreFailures = false - } - """) - .build('check') - } - @Test void shouldBeUpToDateWhenCheckTaskRunsAgain() { def project = projectRule.newProject() @@ -395,7 +381,7 @@ class FindbugsIntegrationTest { * The custom task created in the snippet below will check whether {@code Findbugs} tasks with * empty {@code source} will have empty {@code classes} too.

*/ - private String addCheckFindbugsClassesTask() { + private static String addCheckFindbugsClassesTask() { ''' project.task('checkFindbugsClasses') { dependsOn project.tasks.findByName('evaluateViolations') diff --git a/plugin/src/test/groovy/com/novoda/staticanalysis/internal/pmd/PmdConfigurationTest.groovy b/plugin/src/test/groovy/com/novoda/staticanalysis/internal/pmd/PmdConfigurationTest.groovy new file mode 100644 index 0000000..7747b32 --- /dev/null +++ b/plugin/src/test/groovy/com/novoda/staticanalysis/internal/pmd/PmdConfigurationTest.groovy @@ -0,0 +1,51 @@ +package com.novoda.staticanalysis.internal.pmd + +import com.novoda.test.Fixtures +import com.novoda.test.TestProjectRule +import org.junit.Rule +import org.junit.Test +import org.junit.runner.RunWith +import org.junit.runners.Parameterized + +@RunWith(Parameterized.class) +class PmdConfigurationTest { + + @Parameterized.Parameters(name = "{0}") + static Iterable rules() { + return [ + TestProjectRule.forJavaProject(), + TestProjectRule.forKotlinProject(), + TestProjectRule.forAndroidProject(), + TestProjectRule.forAndroidKotlinProject(), + ] + } + + @Rule + public final TestProjectRule projectRule + + PmdConfigurationTest(TestProjectRule projectRule) { + this.projectRule = projectRule + } + + @Test + void shouldConfigureSuccessFully() { + projectRule.newProject() + .withSourceSet('main', Fixtures.Pmd.SOURCES_WITH_PRIORITY_1_VIOLATION) + .withToolsConfig("pmd { }") + .build('check', '--dry-run') + } + + @Test + void shouldNotFailBuildWhenPmdIsConfiguredMultipleTimes() { + projectRule.newProject() + .withSourceSet('main', Fixtures.Pmd.SOURCES_WITH_PRIORITY_1_VIOLATION) + .withToolsConfig(""" + pmd { + } + pmd { + ignoreFailures = false + } + """) + .build('check', '--dry-run') + } +} diff --git a/plugin/src/test/groovy/com/novoda/staticanalysis/internal/pmd/PmdIntegrationTest.groovy b/plugin/src/test/groovy/com/novoda/staticanalysis/internal/pmd/PmdIntegrationTest.groovy index 41771c5..8ce3824 100644 --- a/plugin/src/test/groovy/com/novoda/staticanalysis/internal/pmd/PmdIntegrationTest.groovy +++ b/plugin/src/test/groovy/com/novoda/staticanalysis/internal/pmd/PmdIntegrationTest.groovy @@ -213,22 +213,6 @@ public class PmdIntegrationTest { assertThat(result.logs).doesNotContainPmdViolations() } - @Test - void shouldNotFailBuildWhenPmdIsConfiguredMultipleTimes() { - projectRule.newProject() - .withSourceSet('main', Fixtures.Pmd.SOURCES_WITH_PRIORITY_1_VIOLATION) - .withPenalty('none') - .withToolsConfig(""" - pmd { - ruleSetFiles = $DEFAULT_RULES - } - pmd { - ignoreFailures = false - } - """) - .build('check') - } - private String pmd(String rules, String... configs) { """pmd { ruleSetFiles = $rules diff --git a/plugin/src/test/groovy/com/novoda/test/TestAndroidKotlinProject.groovy b/plugin/src/test/groovy/com/novoda/test/TestAndroidKotlinProject.groovy index 4c934e9..9e36a2c 100644 --- a/plugin/src/test/groovy/com/novoda/test/TestAndroidKotlinProject.groovy +++ b/plugin/src/test/groovy/com/novoda/test/TestAndroidKotlinProject.groovy @@ -23,6 +23,7 @@ repositories { } apply plugin: 'com.android.library' apply plugin: 'kotlin-android' +apply plugin: 'kotlin-kapt' // adding kapt since we face compat issues before apply plugin: 'com.novoda.static-analysis' android { diff --git a/plugin/src/test/groovy/com/novoda/test/TestKotlinProject.groovy b/plugin/src/test/groovy/com/novoda/test/TestKotlinProject.groovy index 95a1240..f14d66e 100644 --- a/plugin/src/test/groovy/com/novoda/test/TestKotlinProject.groovy +++ b/plugin/src/test/groovy/com/novoda/test/TestKotlinProject.groovy @@ -19,6 +19,7 @@ plugins { } apply plugin: 'kotlin' +apply plugin: 'kotlin-kapt' // adding kapt since we face compat issues before apply plugin: 'com.novoda.static-analysis' repositories { diff --git a/sample-multi-module/core/build.gradle b/sample-multi-module/core/build.gradle index b989299..78885a0 100644 --- a/sample-multi-module/core/build.gradle +++ b/sample-multi-module/core/build.gradle @@ -1,12 +1,13 @@ apply plugin: 'java-library' apply plugin: 'kotlin' +apply plugin: 'kotlin-kapt' repositories { mavenCentral() } -dependencies { - compile "org.jetbrains.kotlin:kotlin-stdlib-jdk8:$kotlin_version" +dependencies { + implementation "org.jetbrains.kotlin:kotlin-stdlib-jdk8:$kotlin_version" } compileKotlin { diff --git a/sample/app/build.gradle b/sample/app/build.gradle index 320a155..0476cc6 100755 --- a/sample/app/build.gradle +++ b/sample/app/build.gradle @@ -6,6 +6,7 @@ plugins { apply plugin: 'com.android.application' apply plugin: 'kotlin-android' +apply plugin: 'kotlin-kapt' apply plugin: 'kotlin-android-extensions' apply plugin: 'com.novoda.static-analysis' From 92dd64bba98708dfced11919b0d4c9e6fdce8586 Mon Sep 17 00:00:00 2001 From: Said Tahsin Dane Date: Fri, 8 Nov 2019 13:20:25 +0100 Subject: [PATCH 13/16] Use Simple FileTree/FileCollection instead of ConfigurableFileTree We were never dependant on that particular class and it seems like applying other plugins change the implementation class. Fixes #199 --- .../internal/findbugs/FindbugsConfigurator.groovy | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) 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 74e736b..5d2bb0d 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,14 +2,13 @@ package com.novoda.staticanalysis.internal.findbugs import com.novoda.staticanalysis.Violations import com.novoda.staticanalysis.internal.CodeQualityConfigurator -import com.novoda.staticanalysis.internal.CollectViolationsTask import org.gradle.api.Action import org.gradle.api.DomainObjectSet 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.file.FileTree import org.gradle.api.plugins.quality.FindBugs import org.gradle.api.plugins.quality.FindBugsExtension import org.gradle.api.tasks.SourceSet @@ -99,7 +98,7 @@ class FindbugsConfigurator extends CodeQualityConfigurator includes) { - includes.isEmpty() ? project.files() : project.fileTree(javaCompile.destinationDir).include(includes) as ConfigurableFileTree + includes.isEmpty() ? project.files() : project.fileTree(javaCompile.destinationDir).include(includes) as FileCollection } @Override @@ -140,10 +139,10 @@ class FindbugsConfigurator extends CodeQualityConfigurator includes) { - return sourceSet.output.classesDirs.inject(null) { ConfigurableFileTree cumulativeTree, File classesDir -> + return sourceSet.output.classesDirs.inject(null) { FileTree cumulativeTree, File classesDir -> def tree = project.fileTree(classesDir) .builtBy(sourceSet.output) - .include(includes) as ConfigurableFileTree + .include(includes) as FileCollection cumulativeTree?.plus(tree) ?: tree } } From 8ee769a7ea93e12e4f1e754003a18251844c3715 Mon Sep 17 00:00:00 2001 From: Said Tahsin Dane Date: Fri, 8 Nov 2019 13:23:00 +0100 Subject: [PATCH 14/16] Also add configuration test for spotbugs --- .../spotbugs/SpotBugsConfigurationTest.groovy | 55 +++++++++++++++++++ .../spotbugs/SpotBugsIntegrationTest.groovy | 14 ----- 2 files changed, 55 insertions(+), 14 deletions(-) create mode 100644 plugin/src/test/groovy/com/novoda/staticanalysis/internal/spotbugs/SpotBugsConfigurationTest.groovy diff --git a/plugin/src/test/groovy/com/novoda/staticanalysis/internal/spotbugs/SpotBugsConfigurationTest.groovy b/plugin/src/test/groovy/com/novoda/staticanalysis/internal/spotbugs/SpotBugsConfigurationTest.groovy new file mode 100644 index 0000000..c8fd9a4 --- /dev/null +++ b/plugin/src/test/groovy/com/novoda/staticanalysis/internal/spotbugs/SpotBugsConfigurationTest.groovy @@ -0,0 +1,55 @@ +package com.novoda.staticanalysis.internal.spotbugs + + +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.Fixtures.Findbugs.SOURCES_WITH_LOW_VIOLATION + +@RunWith(Parameterized.class) +class SpotBugsConfigurationTest { + + @Parameterized.Parameters(name = "{0}") + static Iterable rules() { + return [ + TestProjectRule.forJavaProject(), + TestProjectRule.forKotlinProject(), + TestProjectRule.forAndroidProject(), + TestProjectRule.forAndroidKotlinProject(), + ] + } + + @Rule + public final TestProjectRule projectRule + + SpotBugsConfigurationTest(TestProjectRule projectRule) { + this.projectRule = projectRule + } + + @Test + void shouldConfigureSuccessFully() { + projectRule.newProject() + .withPlugin('com.github.spotbugs', "2.0.0") + .withSourceSet('main', SOURCES_WITH_LOW_VIOLATION) + .withToolsConfig("spotbugs { }") + .build('check', '--dry-run') + } + + @Test + void shouldNotFailBuildWhenSpotBugsIsConfiguredMultipleTimes() { + projectRule.newProject() + .withPlugin('com.github.spotbugs', "2.0.0") + .withSourceSet('main', SOURCES_WITH_LOW_VIOLATION) + .withPenalty('none') + .withToolsConfig(""" + spotbugs { } + spotbugs { + effort = "max" + } + """) + .build('check', '--dry-run') + } +} diff --git a/plugin/src/test/groovy/com/novoda/staticanalysis/internal/spotbugs/SpotBugsIntegrationTest.groovy b/plugin/src/test/groovy/com/novoda/staticanalysis/internal/spotbugs/SpotBugsIntegrationTest.groovy index b3df238..0776d93 100644 --- a/plugin/src/test/groovy/com/novoda/staticanalysis/internal/spotbugs/SpotBugsIntegrationTest.groovy +++ b/plugin/src/test/groovy/com/novoda/staticanalysis/internal/spotbugs/SpotBugsIntegrationTest.groovy @@ -143,20 +143,6 @@ class SpotBugsIntegrationTest { .build('check') } - @Test - void shouldNotFailBuildWhenSpotBugsIsConfiguredMultipleTimes() { - createProjectWith() - .withSourceSet('main', SOURCES_WITH_LOW_VIOLATION) - .withPenalty('none') - .withToolsConfig(""" - spotbugs { } - spotbugs { - effort = "max" - } - """) - .build('check') - } - @Test void shouldBeUpToDateWhenCheckTaskRunsAgain() { def project = createProjectWith() From 9eb6d3b7e7748c539fff7092f2275e6e7cbb81d0 Mon Sep 17 00:00:00 2001 From: Said Tahsin Dane Date: Sat, 9 Nov 2019 16:15:42 +0100 Subject: [PATCH 15/16] Deprecate Findbugs support --- .../findbugs/FindbugsConfigurator.groovy | 17 ++++++++++++++++- 1 file changed, 16 insertions(+), 1 deletion(-) 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 ad8d73d..658ce08 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 @@ -12,16 +12,19 @@ import org.gradle.api.file.FileTree import org.gradle.api.plugins.quality.FindBugs import org.gradle.api.plugins.quality.FindBugsExtension import org.gradle.api.tasks.SourceSet +import org.gradle.util.DeprecationLogger import java.nio.file.Path import static com.novoda.staticanalysis.internal.TasksCompat.configureNamed import static com.novoda.staticanalysis.internal.TasksCompat.createTask +@Deprecated class FindbugsConfigurator extends CodeQualityConfigurator { protected boolean htmlReportEnabled = true + @Deprecated static FindbugsConfigurator create(Project project, NamedDomainObjectContainer violationsContainer, Task evaluateViolations) { @@ -66,6 +69,7 @@ class FindbugsConfigurator extends CodeQualityConfigurator @@ -172,7 +179,6 @@ class FindbugsConfigurator extends CodeQualityConfigurator def findbugs = project.tasks[taskName] as FindBugs @@ -186,4 +192,13 @@ class FindbugsConfigurator extends CodeQualityConfigurator Date: Sat, 9 Nov 2019 16:03:20 +0100 Subject: [PATCH 16/16] Prepare for release 1.2 --- CHANGELOG.md | 8 ++++++++ README.md | 6 +++--- docs/tools/findbugs.md | 5 ++++- gradle/publish.gradle | 2 +- 4 files changed, 16 insertions(+), 5 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 9baf757..7d2923a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,6 +1,14 @@ Change Log ========== +[Version 1.2](https://github.com/novoda/gradle-static-analysis-plugin/releases/tag/v1.2) +-------------------------- + +- Add support for SpotBugs [PR#209](https://github.com/novoda/gradle-static-analysis-plugin/pull/209) +- Fixed an issue caused by combination of Gradle 5.6 and kotlin-kapt [#199](https://github.com/novoda/gradle-static-analysis-plugin/issues/199) +- Fix: Removed usage of deprecated Android APIs [PR#212](https://github.com/novoda/gradle-static-analysis-plugin/pull/212) +- Findbugs support is deprecated [PR#216](https://github.com/novoda/gradle-static-analysis-plugin/pull/216) + [Version 1.1](https://github.com/novoda/gradle-static-analysis-plugin/releases/tag/v1.1) -------------------------- diff --git a/README.md b/README.md index 3750701..b1f978d 100644 --- a/README.md +++ b/README.md @@ -21,7 +21,7 @@ The plugin supports various static analysis tools for Java, Kotlin and Android p * [`Checkstyle`](docs/tools/checkstyle.md) * [`PMD`](docs/tools/pmd.md) - * [`FindBugs`](docs/tools/findbugs.md) + * [`FindBugs`](docs/tools/findbugs.md) [DEPRECATED] [Removed in Gradle 6.0] * [`SpotBugs`](docs/tools/spotbugs.md) * [`Detekt`](docs/tools/detekt.md) * [`Android Lint`](docs/tools/android_lint.md) @@ -52,7 +52,7 @@ buildscript { jcenter() } dependencies { - classpath 'com.novoda:gradle-static-analysis-plugin:1.1' + classpath 'com.novoda:gradle-static-analysis-plugin:1.2' } } @@ -63,7 +63,7 @@ or from the [Gradle Plugins Repository](https://plugins.gradle.org/): ```gradle plugins { - id 'com.novoda.static-analysis' version '1.1' + id 'com.novoda.static-analysis' version '1.2' } ``` diff --git a/docs/tools/findbugs.md b/docs/tools/findbugs.md index 7124886..d48c820 100644 --- a/docs/tools/findbugs.md +++ b/docs/tools/findbugs.md @@ -1,4 +1,7 @@ -# Findbugs +# Findbugs [DEPRECATED] + +**Disclaimer:** FindBugs is completely removed as of Gradle 6.0 and replaced by [`SpotBugs`](/docs/tools/spotbugs.md). FindBugs support in Static Analysis Plugin is now deprecated and will be removed in the next major version. + [Findbugs](http://findbugs.sourceforge.net/) is a static analysis tool that looks for potential bugs in Java code. It does not support Kotlin. It can be used in both pure Java, and Android Java projects. It then only makes sense to have Findbugs enabled if you have Java code in your project. The plugin only runs Findbugs on projects that contain the Java or the Android plugin. diff --git a/gradle/publish.gradle b/gradle/publish.gradle index 198cbae..491ec30 100644 --- a/gradle/publish.gradle +++ b/gradle/publish.gradle @@ -2,7 +2,7 @@ ext { websiteUrl = 'https://github.com/novoda/gradle-static-analysis-plugin' } -version = '1.1' +version = '1.2' groovydoc.docTitle = 'Static Analysis Plugin' apply plugin: 'com.novoda.build-properties'