Bugfix: Ensure multiple runs invariant

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

@@ -34,26 +34,50 @@ abstract class CodeQualityConfigurator<T extends SourceTask, E extends CodeQuali
                 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) {
-                    NamedDomainObjectSet<Object> variants = project.container(Object)
-                    variants.addAll(isAndroidApp ? project.android.applicationVariants : project.android.libraryVariants)
-                    variants.addAll(project.android.testVariants)
-                    variants.addAll(project.android.unitTestVariants)
-                    configureAndroidProject(variants.matching { includeVariantsFilter(it) })
-                } else {
-                    configureJavaProject()
+            project.plugins.withId('com.android.application') {
+                project.afterEvaluate {
+                    configureAndroidProject(allApplicationVariants.matching { includeVariantsFilter(it) })
+                    configureToolTasks()
                 }
-                project.tasks.withType(taskClass) { task ->
-                    task.group = 'verification'
-                    configureReportEvaluation(task)
+            }
+            project.plugins.withId('com.android.library') {
+                project.afterEvaluate {
+                    configureAndroidProject(allLibraryVariants.matching { includeVariantsFilter(it) })
+                    configureToolTasks()
                 }
             }
+            project.plugins.withId('java') {
+                project.afterEvaluate {
+                    configureJavaProject()
+                    configureToolTasks()
+                }
+            }
+        }
+    }
+
+    protected NamedDomainObjectSet<Object> getAllApplicationVariants() {
+        getAllVariants(project.android.applicationVariants)
+    }
+
+    protected void configureToolTasks() {
+        project.tasks.withType(taskClass) { task ->
+            task.group = 'verification'
+            configureReportEvaluation(task, violations)
         }
     }
 
+    protected NamedDomainObjectSet<Object> getAllLibraryVariants() {
+        getAllVariants(project.android.libraryVariants)
+    }
+
+    private NamedDomainObjectSet<Object> getAllVariants(variants1) {
+        NamedDomainObjectSet<Object> variants = project.container(Object)
+        variants.addAll(variants1)
+        variants.addAll(project.android.testVariants)
+        variants.addAll(project.android.unitTestVariants)
+        return variants
+    }
+
     protected abstract String getToolName()
 
     protected Object getToolPlugin() {
@@ -78,6 +102,6 @@ abstract class CodeQualityConfigurator<T extends SourceTask, E extends CodeQuali
 
     protected abstract Class<T> getTaskClass()
 
-    protected abstract void configureReportEvaluation(T task)
+    protected abstract void configureReportEvaluation(T task, Violations violations)
 
 }

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

@@ -0,0 +1,37 @@
+package com.novoda.staticanalysis.internal
+
+import org.gradle.api.DefaultTask
+import org.gradle.api.tasks.TaskAction
+
+abstract class CollectViolationsTask extends DefaultTask {
+
+    private File xmlReportFile
+    private Violations violations
+
+    CollectViolationsTask() {
+        onlyIf { xmlReportFile?.exists() }
+    }
+
+    void setXmlReportFile(File xmlReportFile) {
+        this.xmlReportFile = xmlReportFile
+    }
+
+    void setViolations(Violations violations) {
+        this.violations = violations
+    }
+
+    @TaskAction
+    final void run() {
+        collectViolations(xmlReportFile, htmlReportFile, violations)
+    }
+
+    File getXmlReportFile() {
+        return xmlReportFile
+    }
+
+    File getHtmlReportFile() {
+        new File(xmlReportFile.absolutePath - '.xml' + '.html')
+    }
+
+    abstract void collectViolations(File xmlReportFile, File htmlReportFile, Violations violations)
+}

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

@@ -3,7 +3,7 @@ package com.novoda.staticanalysis.internal.checkstyle
 import com.novoda.staticanalysis.EvaluateViolationsTask
 import com.novoda.staticanalysis.internal.CodeQualityConfigurator
 import com.novoda.staticanalysis.internal.QuietLogger
-import groovy.util.slurpersupport.GPathResult
+import com.novoda.staticanalysis.internal.Violations
 import org.gradle.api.Action
 import org.gradle.api.NamedDomainObjectSet
 import org.gradle.api.Project
@@ -64,19 +64,21 @@ class CheckstyleConfigurator extends CodeQualityConfigurator<Checkstyle, Checkst
     }
 
     @Override
-    protected void configureReportEvaluation(Checkstyle checkstyle) {
+    protected void configureReportEvaluation(Checkstyle checkstyle, Violations violations) {
         checkstyle.showViolations = false
         checkstyle.ignoreFailures = true
         checkstyle.metaClass.getLogger = { QuietLogger.INSTANCE }
-        checkstyle.doLast {
-            File xmlReportFile = checkstyle.reports.xml.destination
-            File htmlReportFile = new File(xmlReportFile.absolutePath - '.xml' + '.html')
 
-            GPathResult xml = new XmlSlurper().parse(xmlReportFile)
-            int errors = xml.'**'.findAll { node -> node.name() == 'error' && node.@severity == 'error' }.size()
-            int warnings = xml.'**'.findAll { node -> node.name() == 'error' && node.@severity == 'warning' }.size()
-            violations.addViolations(errors, warnings, htmlReportFile ?: xmlReportFile)
+        def collectViolations = createCollectViolationsTask(checkstyle, violations)
+
+        evaluateViolations.dependsOn collectViolations
+        collectViolations.dependsOn checkstyle
+    }
+
+    private CollectCheckstyleViolationsTask createCollectViolationsTask(Checkstyle checkstyle, Violations violations) {
+        project.tasks.create("collect${checkstyle.name.capitalize()}Violations", CollectCheckstyleViolationsTask) { collectViolations ->
+            collectViolations.xmlReportFile = checkstyle.reports.xml.destination
+            collectViolations.violations = violations
         }
-        evaluateViolations.dependsOn checkstyle
     }
 }

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

@@ -0,0 +1,17 @@
+package com.novoda.staticanalysis.internal.checkstyle
+
+import com.novoda.staticanalysis.internal.CollectViolationsTask
+import com.novoda.staticanalysis.internal.Violations
+import groovy.util.slurpersupport.GPathResult
+
+class CollectCheckstyleViolationsTask extends CollectViolationsTask {
+
+    @Override
+    void collectViolations(File xmlReportFile, File htmlReportFile, Violations violations) {
+        GPathResult xml = new XmlSlurper().parse(xmlReportFile)
+        int errors = xml.'**'.findAll { node -> node.name() == 'error' && node.@severity == 'error' }.size()
+        int warnings = xml.'**'.findAll { node -> node.name() == 'error' && node.@severity == 'warning' }.size()
+        violations.addViolations(errors, warnings, htmlReportFile ?: xmlReportFile)
+    }
+
+}

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

@@ -0,0 +1,13 @@
+package com.novoda.staticanalysis.internal.findbugs
+
+import com.novoda.staticanalysis.internal.CollectViolationsTask
+import com.novoda.staticanalysis.internal.Violations
+
+class CollectFindbugsViolationsTask extends CollectViolationsTask {
+
+    @Override
+    void collectViolations(File xmlReportFile, File htmlReportFile, Violations violations) {
+        def evaluator = new FinbugsViolationsEvaluator(xmlReportFile)
+        violations.addViolations(evaluator.errorsCount(), evaluator.warningsCount(), htmlReportFile)
+    }
+}

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

@@ -2,6 +2,7 @@ package com.novoda.staticanalysis.internal.findbugs
 
 import com.novoda.staticanalysis.EvaluateViolationsTask
 import com.novoda.staticanalysis.internal.CodeQualityConfigurator
+import com.novoda.staticanalysis.internal.Violations
 import org.gradle.api.Action
 import org.gradle.api.NamedDomainObjectSet
 import org.gradle.api.Project
@@ -119,31 +120,32 @@ class FindbugsConfigurator extends CodeQualityConfigurator<FindBugs, FindBugsExt
     }
 
     @Override
-    protected void configureReportEvaluation(FindBugs findBugs) {
+    protected void configureReportEvaluation(FindBugs findBugs, Violations violations) {
         findBugs.ignoreFailures = true
         findBugs.reports.xml.enabled = true
         findBugs.reports.html.enabled = false
-        File xmlReportFile = findBugs.reports.xml.destination
-        File htmlReportFile = new File(xmlReportFile.absolutePath - '.xml' + '.html')
-        findBugs.doLast {
-            evaluateReports(xmlReportFile, htmlReportFile)
-        }
-        createHtmlReportTask(findBugs, xmlReportFile, htmlReportFile)
+
+        def collectViolations = createViolationsCollectionTask(findBugs, violations)
+        def generateHtmlReport = createHtmlReportTask(findBugs, collectViolations.xmlReportFile, collectViolations.htmlReportFile)
+
+        evaluateViolations.dependsOn collectViolations
+        collectViolations.dependsOn generateHtmlReport
+        generateHtmlReport.dependsOn findBugs
     }
 
-    private void evaluateReports(File xmlReportFile, File htmlReportFile) {
-        def evaluator = new FinbugsViolationsEvaluator(xmlReportFile)
-        violations.addViolations(evaluator.errorsCount(), evaluator.warningsCount(), htmlReportFile)
+    private CollectFindbugsViolationsTask createViolationsCollectionTask(FindBugs findBugs, Violations violations) {
+        project.tasks.create("collect${findBugs.name.capitalize()}Violations", CollectFindbugsViolationsTask) { collectViolations ->
+            collectViolations.xmlReportFile = findBugs.reports.xml.destination
+            collectViolations.violations = violations
+        }
     }
 
-    private GenerateHtmlReport createHtmlReportTask(FindBugs findBugs, File xmlReportFile, File htmlReportFile) {
-        project.tasks.create("generate${findBugs.name.capitalize()}HtmlReport", GenerateHtmlReport) { GenerateHtmlReport generateHtmlReport ->
+    private GenerateFindBugsHtmlReport createHtmlReportTask(FindBugs findBugs, File xmlReportFile, File htmlReportFile) {
+        project.tasks.create("generate${findBugs.name.capitalize()}HtmlReport", GenerateFindBugsHtmlReport) { generateHtmlReport ->
             generateHtmlReport.xmlReportFile = xmlReportFile
             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/GenerateFindBugsHtmlReport.groovy

@@ -0,0 +1,19 @@
+package com.novoda.staticanalysis.internal.findbugs
+
+import org.gradle.api.tasks.JavaExec
+
+class GenerateFindBugsHtmlReport extends JavaExec {
+
+    File xmlReportFile
+    File htmlReportFile
+
+    @Override
+    void exec() {
+        if (xmlReportFile?.exists()) {
+            main = 'edu.umd.cs.findbugs.PrintingBugReporter'
+            standardOutput = new FileOutputStream(htmlReportFile)
+            args '-html', xmlReportFile
+            super.exec()
+        }
+    }
+}

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

@@ -0,0 +1,23 @@
+package com.novoda.staticanalysis.internal.pmd
+
+import com.novoda.staticanalysis.internal.CollectViolationsTask
+import com.novoda.staticanalysis.internal.Violations
+
+class CollectPmdViolationsTask extends CollectViolationsTask {
+
+    @Override
+    void collectViolations(File xmlReportFile, File htmlReportFile, Violations violations) {
+        PmdViolationsEvaluator evaluator = new PmdViolationsEvaluator(xmlReportFile)
+        int errors = 0
+        int warnings = 0
+        evaluator.collectViolations().each { PmdViolationsEvaluator.PmdViolation violation ->
+            if (violation.isError()) {
+                errors += 1
+            } else {
+                warnings += 1
+            }
+        }
+        violations.addViolations(errors, warnings, htmlReportFile ?: xmlReportFile)
+    }
+
+}

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

@@ -64,27 +64,21 @@ class PmdConfigurator extends CodeQualityConfigurator<Pmd, PmdExtension> {
     }
 
     @Override
-    protected void configureReportEvaluation(Pmd pmd) {
+    protected void configureReportEvaluation(Pmd pmd, Violations violations) {
         pmd.ignoreFailures = true
         pmd.metaClass.getLogger = { QuietLogger.INSTANCE }
-        pmd.doLast {
-            File xmlReportFile = pmd.reports.xml.destination
-            File htmlReportFile = new File(xmlReportFile.absolutePath - '.xml' + '.html')
-            evaluateReports(xmlReportFile, htmlReportFile, violations)
-        }
-        evaluateViolations.dependsOn pmd
+
+        def collectViolations = createViolationsCollectionTask(pmd, violations)
+
+        evaluateViolations.dependsOn collectViolations
+        collectViolations.dependsOn pmd
     }
 
-    private static void evaluateReports(File xmlReportFile, File htmlReportFile, Violations violations) {
-        PmdViolationsEvaluator evaluator = new PmdViolationsEvaluator(xmlReportFile)
-        int errors = 0, warnings = 0
-        evaluator.collectViolations().each { PmdViolationsEvaluator.PmdViolation violation ->
-            if (violation.isError()) {
-                errors += 1
-            } else {
-                warnings += 1
-            }
+    private CollectPmdViolationsTask createViolationsCollectionTask(Pmd pmd, Violations violations) {
+        project.tasks.create("collect${pmd.name.capitalize()}Violations", CollectPmdViolationsTask) { collectViolations ->
+            collectViolations.xmlReportFile = pmd.reports.xml.destination
+            collectViolations.violations = violations
         }
-        violations.addViolations(errors, warnings, htmlReportFile ?: xmlReportFile)
     }
+
 }

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

@@ -15,7 +15,7 @@ public class CheckstyleIntegrationTest {
 
     public static final String DEFAULT_CONFIG = "configFile new File('${Fixtures.Checkstyle.MODULES.path}')"
 
-    @Parameterized.Parameters(name =  "{0}")
+    @Parameterized.Parameters(name = "{0}")
     public static Iterable<TestProjectRule> rules() {
         return [TestProjectRule.forJavaProject(), TestProjectRule.forAndroidProject()]
     }
@@ -43,6 +43,29 @@ public class CheckstyleIntegrationTest {
                 result.buildFileUrl('reports/checkstyle/main.html'))
     }
 
+    @Test
+    public void shouldFailBuildAfterSecondRunWhenCheckstyleWarningsStillOverTheThreshold() {
+        def project = projectRule.newProject()
+                .withSourceSet('main', Fixtures.Checkstyle.SOURCES_WITH_WARNINGS)
+                .withPenalty('''{
+                    maxWarnings = 0
+                    maxErrors = 0
+                }''')
+                .withToolsConfig(checkstyle(DEFAULT_CONFIG))
+
+        TestProject.Result result = project.buildAndFail('check')
+
+        assertThat(result.logs).containsLimitExceeded(0, 1)
+        assertThat(result.logs).containsCheckstyleViolations(0, 1,
+                result.buildFileUrl('reports/checkstyle/main.html'))
+
+        result = project.buildAndFail('check')
+
+        assertThat(result.logs).containsLimitExceeded(0, 1)
+        assertThat(result.logs).containsCheckstyleViolations(0, 1,
+                result.buildFileUrl('reports/checkstyle/main.html'))
+    }
+
     @Test
     public void shouldFailBuildWhenCheckstyleErrorsOverTheThreshold() {
         TestProject.Result result = projectRule.newProject()