PT-434: Better classes filtering for Findbugs tasks

plugin/src/main/groovy/com/novoda/staticanalysis/internal/findbugs/FindbugsConfigurator.groovy

@@ -62,12 +62,16 @@ class FindbugsConfigurator extends CodeQualityConfigurator<FindBugs, FindBugsExt
  sourceFilter.applyTo(task)
  task.conventionMapping.map("classes", {
  List<String> includes = createIncludePatterns(task.source, androidSourceDirs)
- project.fileTree(variant.javaCompile.destinationDir).include(includes)
- }.memoize());
+ getAndroidClasses(variant, includes)
+ });
  task.dependsOn variant.javaCompile
  }
  }
 
+ private FileCollection getAndroidClasses(Object variant, List<String> includes) {
+ includes.isEmpty() ? project.files() : project.fileTree(variant.javaCompile.destinationDir).include(includes)
+ }
+
  @Override
  protected void configureJavaProject() {
  project.sourceSets.each { SourceSet sourceSet ->
@@ -78,8 +82,8 @@ class FindbugsConfigurator extends CodeQualityConfigurator<FindBugs, FindBugsExt
  task.conventionMapping.map("classes", {
  List<File> sourceDirs = sourceSet.allJava.srcDirs.findAll { it.exists() }.toList()
  List<String> includes = createIncludePatterns(task.source, sourceDirs)
- createClassesTreeFrom(sourceSet).include(includes)
- }.memoize());
+ getJavaClasses(sourceSet, includes)
+ });
  }
  }
  }
@@ -100,6 +104,10 @@ class FindbugsConfigurator extends CodeQualityConfigurator<FindBugs, FindBugsExt
  .flatten()
  }
 
+ private FileCollection getJavaClasses(SourceSet sourceSet, List<String> includes) {
+ includes.isEmpty() ? project.files() : createClassesTreeFrom(sourceSet).include(includes)
+ }
+
  /**
  * The simple "classes = sourceSet.output" may lead to non-existing resources directory
  * being passed to FindBugs Ant task, resulting in an error
@@ -135,6 +143,7 @@ class FindbugsConfigurator extends CodeQualityConfigurator<FindBugs, FindBugsExt
  generateHtmlReport.htmlReportFile = htmlReportFile
  generateHtmlReport.classpath = findBugs.findbugsClasspath
  generateHtmlReport.dependsOn findBugs
+ generateHtmlReport.onlyIf { xmlReportFile?.exists() }
  evaluateViolations.dependsOn generateHtmlReport
  }
  }

plugin/src/main/groovy/com/novoda/staticanalysis/internal/findbugs/GenerateHtmlReport.groovy

@@ -13,11 +13,9 @@ class GenerateHtmlReport extends JavaExec {
 
  @Override
  void exec() {
- if (xmlReportFile != null && xmlReportFile.exists()) {
- main = 'edu.umd.cs.findbugs.PrintingBugReporter'
- standardOutput = new FileOutputStream(htmlReportFile)
- args '-html', xmlReportFile
- super.exec()
- }
+ main = 'edu.umd.cs.findbugs.PrintingBugReporter'
+ standardOutput = new FileOutputStream(htmlReportFile)
+ args '-html', xmlReportFile
+ super.exec()
  }
 }

plugin/src/test/groovy/com/novoda/staticanalysis/internal/findbugs/FindbugsIntegrationTest.groovy

@@ -1,7 +1,9 @@
 package com.novoda.staticanalysis.internal.findbugs
 
+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
@@ -165,23 +167,49 @@ class FindbugsIntegrationTest {
  result.buildFile('reports/findbugs/debug.html'))
  }
 
- @Test
- public void shouldNotFailBuildWhenFindbugsConfiguredToIgnoreFaultySourceSet() {
- TestProject.Result result = projectRule.newProject()
+ public void shouldNotFailBuildWhenFindbugsConfiguredToIgnoreFaultyJavaSourceSets() {
+ TestProject project = projectRule.newProject()
+ assumeThat(project).isJavaProject()
+
+ TestProject.Result result = project
  .withSourceSet('debug', SOURCES_WITH_LOW_VIOLATION, SOURCES_WITH_MEDIUM_VIOLATION)
- .withSourceSet('release', SOURCES_WITH_HIGH_VIOLATION)
+ .withSourceSet('test', SOURCES_WITH_HIGH_VIOLATION)
  .withPenalty('''{
  maxErrors = 0
  maxWarnings = 10
  }''')
- .withFindbugs("findbugs { exclude ${projectRule.printSourceSet('release')}.java.srcDirs }")
+ .withFindbugs('findbugs { exclude project.sourceSets.test.java.srcDirs }')
  .build('check')
 
  assertThat(result.logs).doesNotContainLimitExceeded()
  assertThat(result.logs).containsFindbugsViolations(0, 2,
  result.buildFile('reports/findbugs/debug.html'))
  }
 
+ @Test
+ public void shouldNotFailBuildWhenFindbugsConfiguredToIgnoreFaultyAndroidSourceSets() {
+ TestProject project = projectRule.newProject()
+ 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
+ }''')
+ .withFindbugs('''findbugs {
+ exclude project.android.sourceSets.test.java.srcDirs
+ exclude project.android.sourceSets.androidTest.java.srcDirs
+ }''')
+ .build('check')
+
+ assertThat(result.logs).doesNotContainLimitExceeded()
+ assertThat(result.logs).containsFindbugsViolations(0, 1,
+ result.buildFile('reports/findbugs/debug.html'))
+ }
+
  @Test
  public void shouldCollectDuplicatedFindbugsWarningsAndErrorsAcrossAndroidVariantsForSharedSourceSets() {
  TestProject project = projectRule.newProject()
@@ -202,4 +230,114 @@ class FindbugsIntegrationTest {
  result.buildFile('reports/findbugs/release.html'))
  }
 
+ @Test
+ public void shouldSkipFindbugsTasksForIgnoredFaultyJavaSourceSets() {
+ TestProject project = projectRule.newProject()
+ 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
+ }''')
+ .withFindbugs('findbugs { exclude project.sourceSets.test.java.srcDirs }')
+ .build('check')
+
+ Truth.assertThat(result.outcome(':findbugsDebug')).isEqualTo(TaskOutcome.SUCCESS)
+ Truth.assertThat(result.outcome(':generateFindbugsDebugHtmlReport')).isEqualTo(TaskOutcome.SUCCESS)
+ Truth.assertThat(result.outcome(':findbugsTest')).isEqualTo(TaskOutcome.UP_TO_DATE)
+ Truth.assertThat(result.outcome(':generateFindbugsTestHtmlReport')).isEqualTo(TaskOutcome.SKIPPED)
+ }
+
+ @Test
+ public void shouldSkipFindbugsTasksForIgnoredFaultyAndroidSourceSets() {
+ TestProject project = projectRule.newProject()
+ 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
+ }''')
+ .withFindbugs('''findbugs {
+ exclude project.android.sourceSets.test.java.srcDirs
+ exclude project.android.sourceSets.androidTest.java.srcDirs
+ }''')
+ .build('check')
+
+ Truth.assertThat(result.outcome(':findbugsDebugAndroidTest')).isEqualTo(TaskOutcome.UP_TO_DATE)
+ Truth.assertThat(result.outcome(':generateFindbugsDebugAndroidTestHtmlReport')).isEqualTo(TaskOutcome.SKIPPED)
+ Truth.assertThat(result.outcome(':findbugsDebug')).isEqualTo(TaskOutcome.SUCCESS)
+ Truth.assertThat(result.outcome(':generateFindbugsDebugHtmlReport')).isEqualTo(TaskOutcome.SUCCESS)
+ Truth.assertThat(result.outcome(':findbugsDebugUnitTest')).isEqualTo(TaskOutcome.UP_TO_DATE)
+ Truth.assertThat(result.outcome(':generateFindbugsDebugUnitTestHtmlReport')).isEqualTo(TaskOutcome.SKIPPED)
+ Truth.assertThat(result.outcome(':findbugsRelease')).isEqualTo(TaskOutcome.UP_TO_DATE)
+ Truth.assertThat(result.outcome(':generateFindbugsReleaseHtmlReport')).isEqualTo(TaskOutcome.SKIPPED)
+ }
+
+ @Test
+ public void shouldProvideNoClassesToFindbugsTaskWhenNoJavaSourcesToAnalyse() {
+ TestProject project = projectRule.newProject()
+ 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
+ }''')
+ .withFindbugs('findbugs { exclude project.sourceSets.test.java.srcDirs }')
+ .withAdditionalConfiguration(addCheckFindbugsClassesTask())
+ .build('checkFindbugsClasses')
+
+ Truth.assertThat(result.outcome(':checkFindbugsClasses')).isEqualTo(TaskOutcome.SUCCESS)
+ }
+
+ @Test
+ public void shouldProvideNoClassesToFindbugsTaskWhenNoAndroidSourcesToAnalyse() {
+ TestProject project = projectRule.newProject()
+ 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
+ }''')
+ .withFindbugs('''findbugs {
+ exclude project.android.sourceSets.test.java.srcDirs
+ exclude project.android.sourceSets.androidTest.java.srcDirs
+ }''')
+ .withAdditionalConfiguration(addCheckFindbugsClassesTask())
+ .build('checkFindbugsClasses')
+
+ Truth.assertThat(result.outcome(':checkFindbugsClasses')).isEqualTo(TaskOutcome.SUCCESS)
+ }
+
+ /**
+ * The custom task created in the snippet below will check whether {@code Findbugs} tasks with
+ * empty {@code source} will have empty {@code classes} too. </p>
+ */
+ private String addCheckFindbugsClassesTask() {
+ '''
+ project.task('checkFindbugsClasses') {
+ dependsOn project.tasks.findByName('evaluateViolations')
+ doLast {
+ project.tasks.withType(FindBugs).all { findbugs ->
+ if (findbugs.source.isEmpty() && !findbugs.classes.isEmpty()) {
+ throw new GradleException("${findbugs.path}.source is empty but ${findbugs.path}.classes is not: \\n${findbugs.classes.files}")
+ }
+ }
+ }
+ }'''
+ }
+
 }

plugin/src/test/groovy/com/novoda/test/TestProject.groovy

@@ -2,6 +2,9 @@ package com.novoda.test
 
 import org.gradle.testkit.runner.BuildResult
 import org.gradle.testkit.runner.GradleRunner
+import org.gradle.testkit.runner.TaskOutcome
+
+import javax.annotation.Nullable
 
 abstract class TestProject<T extends TestProject> {
  private static final Closure<String> EXTENSION_TEMPLATE = { TestProject project ->
@@ -12,12 +15,14 @@ staticAnalysis {
  ${(project.pmd ?: '').replace(' ', ' ')}
  ${(project.findbugs ?: '').replace(' ', ' ')}
 }
+${project.additionalConfiguration}
 """
  }
 
  private final File projectDir
  private final GradleRunner gradleRunner
  private final Closure<String> template
+ String additionalConfiguration = ''
  Map<String, List<File>> sourceSets = [main: []]
  String penalty
  String checkstyle
@@ -87,6 +92,11 @@ staticAnalysis {
  return this
  }
 
+ public T withAdditionalConfiguration(String additionalConfiguration) {
+ this.additionalConfiguration = additionalConfiguration
+ return this
+ }
+
  public Result build(String... arguments) {
  BuildResult buildResult = newRunner(arguments).build()
  createResult(buildResult)
@@ -135,6 +145,11 @@ staticAnalysis {
  new File(buildDir, path)
  }
 
+ @Nullable
+ TaskOutcome outcome(String taskPath) {
+ buildResult.task(taskPath).outcome
+ }
+
  public static class Logs {
  final String output
 

plugin/src/test/groovy/com/novoda/test/TestProjectSubject.groovy

@@ -27,4 +27,8 @@ class TestProjectSubject extends Subject<TestProjectSubject, TestProject> {
  public void isAndroidProject() {
  check().that(actual()).isInstanceOf(TestAndroidProject)
  }
+
+ public void isJavaProject() {
+ check().that(actual()).isInstanceOf(TestJavaProject)
+ }
 }