PT-427: Make Findbugs to honour the exclude filters

plugin/src/main/groovy/com/novoda/staticanalysis/internal/CodeQualityConfigurator.groovy

@@ -12,33 +12,38 @@ abstract class CodeQualityConfigurator<T extends SourceTask, E extends CodeQuali
     protected final Project project
     protected final Violations violations
     protected final EvaluateViolationsTask evaluateViolations
-    protected final SourceFilter filter
+    protected final SourceFilter sourceFilter
 
     protected CodeQualityConfigurator(Project project, Violations violations, EvaluateViolationsTask evaluateViolations) {
         this.project = project
         this.violations = violations
         this.evaluateViolations = evaluateViolations
-        this.filter = new SourceFilter(project)
+        this.sourceFilter = new SourceFilter(project)
     }
 
     void execute() {
         project.extensions.findByType(StaticAnalysisExtension).ext."$toolName" = { Closure config ->
             project.apply plugin: toolPlugin
             project.extensions.findByType(extensionClass).with {
                 defaultConfiguration.execute(it)
-                ext.exclude = { Object rule -> filter.exclude(rule) }
+                ext.exclude = { Object rule -> sourceFilter.exclude(rule) }
                 config.delegate = it
                 config()
             }
             project.afterEvaluate {
                 boolean isAndroidApp = project.plugins.hasPlugin('com.android.application')
                 boolean isAndroidLib = project.plugins.hasPlugin('com.android.library')
                 if (isAndroidApp || isAndroidLib) {
-                    configureAndroid(isAndroidApp ? project.android.applicationVariants : project.android.libraryVariants)
-                    configureAndroid(project.android.testVariants)
-                    configureAndroid(project.android.unitTestVariants)
+                    configureAndroidProject(isAndroidApp ? project.android.applicationVariants : project.android.libraryVariants)
+                    configureAndroidProject(project.android.testVariants)
+                    configureAndroidProject(project.android.unitTestVariants)
+                } else {
+                    configureJavaProject()
+                }
+                project.tasks.withType(taskClass) { task ->
+                    task.group = 'verification'
+                    configureReportEvaluation(task)
                 }
-                project.tasks.withType(taskClass) { task -> configureTask(task) }
             }
         }
     }
@@ -59,13 +64,14 @@ abstract class CodeQualityConfigurator<T extends SourceTask, E extends CodeQuali
         }
     }
 
-    protected abstract void configureAndroid(Object variants)
+    protected abstract void configureAndroidProject(Object variants)
+
+    protected void configureJavaProject() {
+        project.tasks.withType(taskClass) { task -> sourceFilter.applyTo(task) }
+    }
 
     protected abstract Class<T> getTaskClass()
 
-    protected void configureTask(T task) {
-        task.group = 'verification'
-        filter.applyTo(task)
-    }
+    protected abstract void configureReportEvaluation(T task)
 
 }

plugin/src/main/groovy/com/novoda/staticanalysis/internal/checkstyle/CheckstyleConfigurator.groovy

@@ -42,33 +42,29 @@ class CheckstyleConfigurator extends CodeQualityConfigurator<Checkstyle, Checkst
     }
 
     @Override
-    protected void configureAndroid(Object variants) {
+    protected void configureAndroidProject(Object variants) {
         project.with {
             variants.all { variant ->
                 variant.sourceSets.each { sourceSet ->
                     def taskName = "checkstyle${sourceSet.name.capitalize()}"
-                    Checkstyle checkstyle = tasks.findByName(taskName)
-                    if (checkstyle == null) {
-                        checkstyle = tasks.create(taskName, Checkstyle)
-                        def sourceDirs = sourceSet.java.srcDirs
-                        def notEmptyDirs = sourceDirs.findAll { it.list()?.length > 0 }
-                        if (!notEmptyDirs.empty) {
-                            checkstyle.with {
-                                description = "Run Checkstyle analysis for ${sourceSet.name} classes"
-                                source = sourceSet.java.srcDirs
-                                classpath = files("$buildDir/intermediates/classes/")
-                            }
+                    Checkstyle task = tasks.findByName(taskName)
+                    if (task == null) {
+                        task = tasks.create(taskName, Checkstyle)
+                        task.with {
+                            description = "Run Checkstyle analysis for ${sourceSet.name} classes"
+                            source = sourceSet.java.srcDirs
+                            classpath = files("$buildDir/intermediates/classes/")
                         }
                     }
-                    checkstyle.mustRunAfter variant.javaCompile
+                    sourceFilter.applyTo(task)
+                    task.mustRunAfter variant.javaCompile
                 }
             }
         }
     }
 
     @Override
-    protected void configureTask(Checkstyle checkstyle) {
-        super.configureTask(checkstyle)
+    protected void configureReportEvaluation(Checkstyle checkstyle) {
         checkstyle.showViolations = false
         checkstyle.ignoreFailures = true
         checkstyle.metaClass.getLogger = { QuietLogger.INSTANCE }

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

@@ -4,10 +4,15 @@ import com.novoda.staticanalysis.EvaluateViolationsTask
 import com.novoda.staticanalysis.internal.CodeQualityConfigurator
 import org.gradle.api.Action
 import org.gradle.api.Project
+import org.gradle.api.file.ConfigurableFileTree
+import org.gradle.api.file.FileCollection
 import org.gradle.api.plugins.quality.FindBugs
 import org.gradle.api.plugins.quality.FindBugsExtension
+import org.gradle.api.tasks.SourceSet
 import org.gradle.internal.logging.ConsoleRenderer
 
+import java.nio.file.Path
+
 class FindbugsConfigurator extends CodeQualityConfigurator<FindBugs, FindBugsExtension> {
 
     FindbugsConfigurator(Project project, EvaluateViolationsTask evaluateViolations) {
@@ -45,23 +50,68 @@ class FindbugsConfigurator extends CodeQualityConfigurator<FindBugs, FindBugsExt
     }
 
     @Override
-    protected void configureAndroid(Object variants) {
+    protected void configureAndroidProject(Object variants) {
         variants.all { variant ->
             FindBugs task = project.tasks.create("findbugs${variant.name.capitalize()}", QuietFindbugsPlugin.Task)
+            List<File> androidSourceDirs = variant.sourceSets.collect { it.javaDirectories }.flatten()
             task.with {
-                group = "verification"
                 description = "Run FindBugs analysis for ${variant.name} classes"
-                source = variant.sourceSets.java.srcDirs.collect { it.path }.flatten()
-                classes = project.fileTree(variant.javaCompile.destinationDir)
+                source = androidSourceDirs
                 classpath = variant.javaCompile.classpath
-                dependsOn variant.javaCompile
             }
+            sourceFilter.applyTo(task)
+            task.conventionMapping.map("classes", {
+                List<String> includes = createIncludePatterns(task.source, androidSourceDirs)
+                project.fileTree(variant.javaCompile.destinationDir).include(includes)
+            }.memoize());
+            task.dependsOn variant.javaCompile
         }
     }
 
     @Override
-    protected void configureTask(FindBugs findBugs) {
-        super.configureTask(findBugs)
+    protected void configureJavaProject() {
+        project.sourceSets.each { SourceSet sourceSet ->
+            String taskName = sourceSet.getTaskName(toolName, null)
+            FindBugs task = project.tasks.findByName(taskName)
+            if (task != null) {
+                sourceFilter.applyTo(task)
+                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());
+            }
+        }
+    }
+
+    private static List<String> createIncludePatterns(FileCollection sourceFiles, List<File> sourceDirs) {
+        List<Path> includedSourceFilesPaths = sourceFiles.matching { '**/*.java' }.files.collect { it.toPath() }
+        List<Path> sourceDirsPaths = sourceDirs.collect { it.toPath() }
+        createRelativePaths(includedSourceFilesPaths, sourceDirsPaths)
+                .collect { Path relativePath -> (relativePath as String) - '.java' + '*' }
+    }
+
+    private static List<Path> createRelativePaths(List<Path> includedSourceFiles, List<Path> sourceDirs) {
+        includedSourceFiles.collect { Path sourceFile ->
+            sourceDirs
+                    .findAll { Path sourceDir -> sourceFile.startsWith(sourceDir) }
+                    .collect { Path sourceDir -> sourceDir.relativize(sourceFile) }
+        }
+        .flatten()
+    }
+
+    /**
+     * The simple "classes = sourceSet.output" may lead to non-existing resources directory
+     * being passed to FindBugs Ant task, resulting in an error
+     * */
+    private ConfigurableFileTree createClassesTreeFrom(SourceSet sourceSet) {
+        project.fileTree(sourceSet.output.classesDir, {
+            it.builtBy(sourceSet.output)
+        })
+    }
+
+    @Override
+    protected void configureReportEvaluation(FindBugs findBugs) {
         findBugs.ignoreFailures = true
         findBugs.reports.xml.enabled = true
         findBugs.reports.html.enabled = false

plugin/src/main/groovy/com/novoda/staticanalysis/internal/pmd/PmdConfigurator.groovy

@@ -43,32 +43,28 @@ class PmdConfigurator extends CodeQualityConfigurator<Pmd, PmdExtension> {
     }
 
     @Override
-    protected void configureAndroid(Object variants) {
+    protected void configureAndroidProject(Object variants) {
         project.with {
             variants.all { variant ->
                 variant.sourceSets.each { sourceSet ->
                     def taskName = "pmd${sourceSet.name.capitalize()}"
-                    Pmd pmd = tasks.findByName(taskName)
-                    if (pmd == null) {
-                        pmd = tasks.create(taskName, Pmd)
-                        def sourceDirs = sourceSet.java.srcDirs
-                        def notEmptyDirs = sourceDirs.findAll { it.list()?.length > 0 }
-                        if (!notEmptyDirs.empty) {
-                            pmd.with {
-                                description = "Run PMD analysis for ${sourceSet.name} classes"
-                                source = sourceSet.java.srcDirs
-                            }
+                    Pmd task = tasks.findByName(taskName)
+                    if (task == null) {
+                        task = tasks.create(taskName, Pmd)
+                        task.with {
+                            description = "Run PMD analysis for ${sourceSet.name} classes"
+                            source = sourceSet.java.srcDirs
                         }
                     }
-                    pmd.mustRunAfter variant.javaCompile
+                    sourceFilter.applyTo(task)
+                    task.mustRunAfter variant.javaCompile
                 }
             }
         }
     }
 
     @Override
-    protected void configureTask(Pmd pmd) {
-        super.configureTask(pmd)
+    protected void configureReportEvaluation(Pmd pmd) {
         pmd.ignoreFailures = true
         pmd.metaClass.getLogger = { QuietLogger.INSTANCE }
         pmd.doLast {

plugin/src/test/fixtures/findbugs/high/com/novoda/test/HighPriorityViolator.java

@@ -0,0 +1,15 @@
+package com.novoda.test;
+
+public class HighPriorityViolator {
+
+    public static class Internal {
+
+        public void impossibleCast() {
+            final Object doubleValue = Double.valueOf(1.0);
+            final Long value = (Long) doubleValue;
+            System.out.println("   - " + value);
+        }
+
+    }
+
+}

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

@@ -134,13 +134,13 @@ class FindbugsIntegrationTest {
     @Test
     public void shouldNotFailBuildWhenFindbugsConfiguredToExcludePattern() {
         TestProject.Result result = projectRule.newProject()
-                .withSourceSet('debug', SOURCES_WITH_LOW_VIOLATION, SOURCES_WITH_MEDIUM_VIOLATION)
-                .withSourceSet('release', SOURCES_WITH_HIGH_VIOLATION)
+                .withSourceSet('debug', SOURCES_WITH_LOW_VIOLATION)
+                .withSourceSet('release', SOURCES_WITH_HIGH_VIOLATION, SOURCES_WITH_MEDIUM_VIOLATION)
                 .withPenalty('''{
                     maxErrors = 0
                     maxWarnings = 10
                 }''')
-                .withFindbugs('findbugs { exclude "HighPriorityViolator.java" }')
+                .withFindbugs('findbugs { exclude "com/novoda/test/HighPriorityViolator.java" }')
                 .build('check')
 
         assertThat(result.logs).doesNotContainLimitExceeded()