Improve exclude rules

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

@@ -12,20 +12,21 @@ abstract class CodeQualityConfigurator<T extends SourceTask, E extends CodeQuali
     protected final Project project
     protected final Violations violations
     protected final EvaluateViolationsTask evaluateViolations
-    protected final List<String> excludes = []
+    protected final SourceFilter filter
 
     protected CodeQualityConfigurator(Project project, Violations violations, EvaluateViolationsTask evaluateViolations) {
         this.project = project
         this.violations = violations
         this.evaluateViolations = evaluateViolations
+        this.filter = 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 = { String pattern -> excludes.add(pattern) }
+                ext.exclude = { Object rule -> filter.exclude(rule) }
                 config.delegate = it
                 config()
             }
@@ -60,6 +61,9 @@ abstract class CodeQualityConfigurator<T extends SourceTask, E extends CodeQuali
 
     protected abstract Class<T> getTaskClass()
 
-    protected abstract void configureTask(T task)
+    protected void configureTask(T task) {
+        task.group = 'verification'
+        filter.applyTo(task)
+    }
 
 }

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

@@ -0,0 +1,43 @@
+package com.novoda.staticanalysis.internal
+
+import org.gradle.api.Project
+import org.gradle.api.file.FileCollection
+import org.gradle.api.file.FileTree
+import org.gradle.api.tasks.SourceTask
+
+class SourceFilter {
+
+    private final Project project
+    private final List<Object> excludes = []
+
+    SourceFilter(Project project) {
+        this.project = project
+    }
+
+    void exclude(Object exclude) {
+        excludes.add(exclude)
+    }
+
+    void applyTo(SourceTask task) {
+        excludes.each { exclude ->
+            if (exclude instanceof File) {
+                apply(task, project.files(exclude))
+            } else if (exclude instanceof FileCollection) {
+                apply(task, exclude)
+            } else if (exclude instanceof Iterable<File>) {
+                apply(task, exclude.inject(null, accumulateIntoTree()) as FileTree)
+            } else {
+                task.exclude(exclude as String)
+            }
+        }
+    }
+
+    private void apply(SourceTask task, FileCollection excludedFiles) {
+        task.source = task.source.findAll { !excludedFiles.contains(it) }
+    }
+
+    private def accumulateIntoTree() {
+        return { tree, file -> tree?.plus(project.fileTree(file)) ?: project.fileTree(file) }
+    }
+
+}

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

@@ -65,11 +65,10 @@ class CheckstyleConfigurator extends CodeQualityConfigurator<Checkstyle, Checkst
 
     @Override
     protected void configureTask(Checkstyle checkstyle) {
-        checkstyle.group = 'verification'
+        super.configureTask(checkstyle)
         checkstyle.showViolations = false
         checkstyle.ignoreFailures = true
         checkstyle.metaClass.getLogger = { QuietLogger.INSTANCE }
-        checkstyle.exclude(excludes)
         checkstyle.doLast {
             File xmlReportFile = checkstyle.reports.xml.destination
             File htmlReportFile = new File(xmlReportFile.absolutePath - '.xml' + '.html')

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

@@ -61,10 +61,10 @@ class FindbugsConfigurator extends CodeQualityConfigurator<FindBugs, FindBugsExt
 
     @Override
     protected void configureTask(FindBugs findBugs) {
+        super.configureTask(findBugs)
         findBugs.ignoreFailures = true
         findBugs.reports.xml.enabled = true
         findBugs.reports.html.enabled = false
-        findBugs.exclude(excludes)
         File xmlReportFile = findBugs.reports.xml.destination
         File htmlReportFile = new File(xmlReportFile.absolutePath - '.xml' + '.html')
         findBugs.doLast {

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

@@ -65,10 +65,9 @@ class PmdConfigurator extends CodeQualityConfigurator<Pmd, PmdExtension> {
 
     @Override
     protected void configureTask(Pmd pmd) {
-        pmd.group = 'verification'
+        super.configureTask(pmd)
         pmd.ignoreFailures = true
         pmd.metaClass.getLogger = { QuietLogger.INSTANCE }
-        pmd.exclude(excludes)
         pmd.doLast {
             File xmlReportFile = pmd.reports.xml.destination
             File htmlReportFile = new File(xmlReportFile.absolutePath - '.xml' + '.html')

plugin/src/test/groovy/com/novoda/staticanalysis/internal/SourceFilterTest.groovy

@@ -0,0 +1,105 @@
+package com.novoda.staticanalysis.internal
+
+import org.gradle.api.Project
+import org.gradle.api.tasks.SourceTask
+import org.gradle.testfixtures.ProjectBuilder
+import org.junit.Before
+import org.junit.Rule
+import org.junit.Test
+import org.junit.rules.TemporaryFolder
+
+import static com.google.common.truth.Truth.assertThat
+import static com.novoda.test.Fixtures.Checkstyle.SOURCES_WITH_ERRORS
+import static com.novoda.test.Fixtures.Checkstyle.SOURCES_WITH_WARNINGS
+
+class SourceFilterTest {
+
+    @Rule
+    public TemporaryFolder temporaryFolder = new TemporaryFolder()
+
+    private Project project
+    private SourceFilter filter
+
+    @Before
+    public void setUp() {
+        project = ProjectBuilder.builder()
+                .withProjectDir(temporaryFolder.newFolder())
+                .build()
+        filter = new SourceFilter(project)
+    }
+
+    @Test
+    public void shouldKeepAllSourcesWhenNoExcludeFilterProvided() {
+        SourceTask task = givenTaskWith(errorsSources + warningsSources)
+
+        filter.applyTo(task)
+
+        assertThat(task.source).containsExactlyElementsIn(errorsSources + warningsSources)
+    }
+
+    @Test
+    public void shouldRemoveFilesMatchingThePatternWhenExcludePatternProvided() {
+        SourceTask task = givenTaskWith(errorsSources + warningsSources)
+        filter.exclude('**/*.java')
+
+        filter.applyTo(task)
+
+        assertThat(task.source).isEmpty()
+    }
+
+    @Test
+    public void shouldRemoveFilesInSpecifiedFileCollectionWhenExcludeFileCollectionProvided() {
+        SourceTask task = givenTaskWith(errorsSources + warningsSources)
+        filter.exclude(project.fileTree(SOURCES_WITH_ERRORS))
+
+        filter.applyTo(task)
+
+        assertThat(task.source).containsExactlyElementsIn(warningsSources)
+    }
+
+    @Test
+    public void shouldRemoveFilesInSpecifiedFileCollectionWhenExcludeSourceSetProvided() {
+        project.apply plugin: 'java'
+        project.sourceSets.main {
+            java {
+                srcDir project.file(SOURCES_WITH_ERRORS)
+            }
+        }
+        SourceTask task = givenTaskWith(errorsSources)
+        filter.exclude(project.sourceSets.main.java.srcDirs)
+
+        filter.applyTo(task)
+
+        assertThat(task.source).isEmpty()
+    }
+
+    @Test
+    public void shouldRemoveFilesInSpecifiedFileCollectionWhenSourceFileExcluded() {
+        project.apply plugin: 'java'
+        project.sourceSets.main {
+            java {
+                srcDir project.file(SOURCES_WITH_ERRORS)
+            }
+        }
+        SourceTask task = givenTaskWith(errorsSources)
+        filter.exclude(new File(SOURCES_WITH_ERRORS, 'Greeter.java'))
+
+        filter.applyTo(task)
+
+        assertThat(task.source).isEmpty()
+    }
+
+    private SourceTask givenTaskWith(Iterable<File> files) {
+        project.tasks.create('someSourceTask', SourceTask) {
+            source = project.files(files)
+        }
+    }
+
+    private Set<File> getErrorsSources() {
+        project.fileTree(SOURCES_WITH_ERRORS).files
+    }
+
+    private Set<File> getWarningsSources() {
+        project.fileTree(SOURCES_WITH_WARNINGS).files
+    }
+}

plugin/src/test/groovy/com/novoda/staticanalysis/internal/checkstyle/CheckstyleIntegrationTest.groovy

@@ -108,7 +108,7 @@ public class CheckstyleIntegrationTest {
     }
 
     @Test
-    public void shouldNotFailBuildWhenCheckstyleConfiguredIgnoreSourceSets() {
+    public void shouldNotFailBuildWhenCheckstyleConfiguredToExcludePattern() {
         TestProject.Result result = projectRule.newProject()
                 .withSourceSet('main', Fixtures.Checkstyle.SOURCES_WITH_WARNINGS)
                 .withSourceSet('test', Fixtures.Checkstyle.SOURCES_WITH_ERRORS)
@@ -125,6 +125,42 @@ public class CheckstyleIntegrationTest {
                 result.buildFile('reports/checkstyle/main.html'))
     }
 
+    @Test
+    public void shouldNotFailBuildWhenCheckstyleConfiguredToIgnoreFaultySourceFolder() {
+        TestProject.Result result = projectRule.newProject()
+                .withSourceSet('main', Fixtures.Checkstyle.SOURCES_WITH_WARNINGS)
+                .withSourceSet('test', Fixtures.Checkstyle.SOURCES_WITH_ERRORS)
+                .withFile(Fixtures.Checkstyle.MODULES, 'config/checkstyle/checkstyle.xml')
+                .withPenalty('''{
+                    maxWarnings = 1
+                    maxErrors = 0
+                }''')
+                .withCheckstyle(checkstyle(DEFAULT_CONFIG, "exclude project.fileTree('${Fixtures.Checkstyle.SOURCES_WITH_ERRORS}')"))
+                .build('check')
+
+        assertThat(result.logs).doesNotContainLimitExceeded()
+        assertThat(result.logs).containsCheckstyleViolations(0, 1,
+                result.buildFile('reports/checkstyle/main.html'))
+    }
+
+    @Test
+    public void shouldNotFailBuildWhenCheckstyleConfiguredToIgnoreFaultySourceSet() {
+        TestProject.Result result = projectRule.newProject()
+                .withSourceSet('main', Fixtures.Checkstyle.SOURCES_WITH_WARNINGS)
+                .withSourceSet('test', Fixtures.Checkstyle.SOURCES_WITH_ERRORS)
+                .withFile(Fixtures.Checkstyle.MODULES, 'config/checkstyle/checkstyle.xml')
+                .withPenalty('''{
+                    maxWarnings = 1
+                    maxErrors = 0
+                }''')
+                .withCheckstyle(checkstyle(DEFAULT_CONFIG, "exclude ${projectRule.printSourceSet('test')}.java.srcDirs"))
+                .build('check')
+
+        assertThat(result.logs).doesNotContainLimitExceeded()
+        assertThat(result.logs).containsCheckstyleViolations(0, 1,
+                result.buildFile('reports/checkstyle/main.html'))
+    }
+
     @Test
     public void shouldNotFailWhenCheckstyleNotConfigured() {
         TestProject.Result result = projectRule.newProject()
@@ -170,7 +206,6 @@ public class CheckstyleIntegrationTest {
                 result.buildFile('reports/checkstyle/main.html'))
     }
 
-
     private static String checkstyle(String configFile, String... configs) {
         """checkstyle {
             ${configFile}

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

@@ -132,7 +132,7 @@ class FindbugsIntegrationTest {
     }
 
     @Test
-    public void shouldNotFailBuildWhenFindbugsConfiguredToIgnoreFaultySourceSets() {
+    public void shouldNotFailBuildWhenFindbugsConfiguredToExcludePattern() {
         TestProject.Result result = projectRule.newProject()
                 .withSourceSet('debug', SOURCES_WITH_LOW_VIOLATION, SOURCES_WITH_MEDIUM_VIOLATION)
                 .withSourceSet('release', SOURCES_WITH_HIGH_VIOLATION)
@@ -148,6 +148,40 @@ class FindbugsIntegrationTest {
                 result.buildFile('reports/findbugs/debug.html'))
     }
 
+    @Test
+    public void shouldNotFailBuildWhenFindbugsConfiguredToIgnoreFaultySourceFolder() {
+        TestProject.Result result = projectRule.newProject()
+                .withSourceSet('debug', SOURCES_WITH_LOW_VIOLATION, SOURCES_WITH_MEDIUM_VIOLATION)
+                .withSourceSet('release', SOURCES_WITH_HIGH_VIOLATION)
+                .withPenalty('''{
+                    maxErrors = 0
+                    maxWarnings = 10
+                }''')
+                .withFindbugs("findbugs { exclude project.fileTree('${SOURCES_WITH_HIGH_VIOLATION}') }")
+                .build('check')
+
+        assertThat(result.logs).doesNotContainLimitExceeded()
+        assertThat(result.logs).containsFindbugsViolations(0, 2,
+                result.buildFile('reports/findbugs/debug.html'))
+    }
+
+    @Test
+    public void shouldNotFailBuildWhenFindbugsConfiguredToIgnoreFaultySourceSet() {
+        TestProject.Result result = projectRule.newProject()
+                .withSourceSet('debug', SOURCES_WITH_LOW_VIOLATION, SOURCES_WITH_MEDIUM_VIOLATION)
+                .withSourceSet('release', SOURCES_WITH_HIGH_VIOLATION)
+                .withPenalty('''{
+                    maxErrors = 0
+                    maxWarnings = 10
+                }''')
+                .withFindbugs("findbugs { exclude ${projectRule.printSourceSet('release')}.java.srcDirs }")
+                .build('check')
+
+        assertThat(result.logs).doesNotContainLimitExceeded()
+        assertThat(result.logs).containsFindbugsViolations(0, 2,
+                result.buildFile('reports/findbugs/debug.html'))
+    }
+
     @Test
     public void shouldCollectDuplicatedFindbugsWarningsAndErrorsAcrossAndroidVariantsForSharedSourceSets() {
         TestProject project = projectRule.newProject()

plugin/src/test/groovy/com/novoda/staticanalysis/internal/pmd/PmdIntegrationTest.groovy

@@ -130,7 +130,7 @@ public class PmdIntegrationTest {
     }
 
     @Test
-    public void shouldNotFailBuildWhenPmdConfiguredToIgnoreFaultySourceSets() {
+    public void shouldNotFailBuildWhenPmdConfiguredToExcludePatterns() {
         TestProject.Result result = projectRule.newProject()
                 .withSourceSet('main', Fixtures.Pmd.SOURCES_WITH_PRIORITY_1_VIOLATION)
                 .withSourceSet('main2', Fixtures.Pmd.SOURCES_WITH_PRIORITY_2_VIOLATION)
@@ -145,6 +145,42 @@ public class PmdIntegrationTest {
         assertThat(result.logs).doesNotContainPmdViolations()
     }
 
+    @Test
+    public void shouldNotFailBuildWhenPmdConfiguredToIgnoreFaultySourceFolders() {
+        TestProject.Result result = projectRule.newProject()
+                .withSourceSet('main', Fixtures.Pmd.SOURCES_WITH_PRIORITY_1_VIOLATION)
+                .withSourceSet('main2', Fixtures.Pmd.SOURCES_WITH_PRIORITY_2_VIOLATION)
+                .withPenalty('''{
+                    maxWarnings = 0
+                    maxErrors = 0
+                }''')
+                .withPmd(pmd("project.files('${Fixtures.Pmd.RULES.path}')",
+                "exclude project.fileTree('${Fixtures.Pmd.SOURCES_WITH_PRIORITY_1_VIOLATION}')",
+                "exclude project.fileTree('${Fixtures.Pmd.SOURCES_WITH_PRIORITY_2_VIOLATION}')"))
+                .build('check')
+
+        assertThat(result.logs).doesNotContainLimitExceeded()
+        assertThat(result.logs).doesNotContainPmdViolations()
+    }
+
+    @Test
+    public void shouldNotFailBuildWhenPmdConfiguredToIgnoreFaultySourceSets() {
+        TestProject.Result result = projectRule.newProject()
+                .withSourceSet('main', Fixtures.Pmd.SOURCES_WITH_PRIORITY_1_VIOLATION)
+                .withSourceSet('main2', Fixtures.Pmd.SOURCES_WITH_PRIORITY_2_VIOLATION)
+                .withPenalty('''{
+                    maxWarnings = 0
+                    maxErrors = 0
+                }''')
+                .withPmd(pmd("project.files('${Fixtures.Pmd.RULES.path}')",
+                "exclude ${projectRule.printSourceSet('main')}.java.srcDirs",
+                "exclude ${projectRule.printSourceSet('main2')}.java.srcDirs"))
+                .build('check')
+
+        assertThat(result.logs).doesNotContainLimitExceeded()
+        assertThat(result.logs).doesNotContainPmdViolations()
+    }
+
     @Test
     public void shouldNotFailBuildWhenPmdNotConfigured() {
         TestProject.Result result = projectRule.newProject()