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 4c0073b..b1f978d 100644 --- a/README.md +++ b/README.md @@ -21,7 +21,8 @@ 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) * [`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/issues/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) @@ -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' } ``` @@ -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 and create `evaluateV [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/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/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` 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' 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/findbugs/FindbugsConfigurator.groovy b/plugin/src/main/groovy/com/novoda/staticanalysis/internal/findbugs/FindbugsConfigurator.groovy index 74e736b..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 @@ -2,27 +2,29 @@ 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 +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) { @@ -67,6 +69,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) @@ -99,11 +102,14 @@ 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 protected void configureJavaProject() { + if (!configured) { + logDeprecatedMessage() + } super.configureJavaProject() project.afterEvaluate { project.sourceSets.each { SourceSet sourceSet -> @@ -140,10 +146,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 } } @@ -173,7 +179,6 @@ class FindbugsConfigurator extends CodeQualityConfigurator def findbugs = project.tasks[taskName] as FindBugs @@ -187,4 +192,13 @@ class FindbugsConfigurator extends CodeQualityConfigurator 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('com.android.application') { + configureAndroidWithVariants(variantFilter.filteredApplicationVariants) + } + project.plugins.withId('com.android.library') { + configureAndroidWithVariants(variantFilter.filteredLibraryVariants) + } + project.plugins.withId('java') { + configureJavaProject() + } + } + } + + private void configureSpotBugsExtension(Closure config) { + try { + def spotbugs = project.spotbugs + 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() + } catch (Exception exception) { + handleException(SPOTBUGS_CONFIGURATION_ERROR, exception) + } + } + + 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')) { SourceTask task -> + def javaCompile = javaCompile(variant) + def androidSourceDirs = variant.sourceSets.collect { + it.javaDirectories + }.flatten() + task.description = "Run SpotBugs analysis for ${variant.name} classes" + task.setSource(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 -> + 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] as SourceTask + configureToolTask(spotbugs) + task.xmlReportFile = spotbugs.reports.xml.destination + task.violations = violations + + 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 + } + } + + private static void configureToolTask(SourceTask task) { + task.group = 'verification' + task.exclude '**/*.kt' + task.ignoreFailures = true + task.reports.xml.enabled = true + task.reports.html.enabled = false + } + + 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/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/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 new file mode 100644 index 0000000..0776d93 --- /dev/null +++ b/plugin/src/test/groovy/com/novoda/staticanalysis/internal/spotbugs/SpotBugsIntegrationTest.groovy @@ -0,0 +1,181 @@ +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.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 + +@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 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 + 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) } 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/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 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-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); + } + + } } 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..0476cc6 100755 --- a/sample/app/build.gradle +++ b/sample/app/build.gradle @@ -1,10 +1,12 @@ 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' apply plugin: 'kotlin-android' +apply plugin: 'kotlin-kapt' apply plugin: 'kotlin-android-extensions' apply plugin: 'com.novoda.static-analysis' @@ -51,6 +53,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" } }