Merge pull request #90 from novoda/70/verify-detekt-config

Verify detekt config

plugin/src/main/groovy/com/novoda/staticanalysis/internal/detekt/DetektConfigurator.groovy

@@ -12,6 +12,7 @@ class DetektConfigurator implements Configurator {
 
     private static final String DETEKT_PLUGIN = 'io.gitlab.arturbosch.detekt'
     private static final String DETEKT_NOT_APPLIED = 'The Detekt plugin is configured but not applied. Please apply the plugin in your build script.\nFor more information see https://github.com/arturbosch/detekt.'
+    private static final String OUTPUT_NOT_DEFINED = 'Output not defined! To analyze the results, `output` needs to be defined in Detekt profile.'
 
     private final Project project
     private final Violations violations
@@ -32,7 +33,7 @@ class DetektConfigurator implements Configurator {
 
     @Override
     void execute() {
-        project.extensions.findByType(StaticAnalysisExtension).ext.'detekt' = { Closure config ->
+        project.extensions.findByType(StaticAnalysisExtension).ext.detekt = { Closure config ->
             if (!isKotlinProject(project)) {
                 return
             }
@@ -41,34 +42,35 @@ class DetektConfigurator implements Configurator {
                 throw new GradleException(DETEKT_NOT_APPLIED)
             }
 
-            project.extensions.findByName('detekt').with {
-                // apply configuration closure to detekt extension
-                config.delegate = it
-                config()
-            }
-
-            configureToolTask()
+            def detekt = project.extensions.findByName('detekt')
+            config.delegate = detekt
+            config()
+            configureToolTask(detekt)
         }
     }
 
-    private void configureToolTask() {
+    private void configureToolTask(detekt) {
         def detektTask = project.tasks['detektCheck']
+        detektTask.group = 'verification'
+
         // run detekt as part of check
         project.tasks['check'].dependsOn(detektTask)
 
         // evaluate violations after detekt
-        detektTask.group = 'verification'
-        def collectViolations = createCollectViolationsTask(violations)
+        def output = detekt.systemOrDefaultProfile().output
+        if (!output) {
+            throw new IllegalArgumentException(OUTPUT_NOT_DEFINED)
+        }
+        def collectViolations = createCollectViolationsTask(violations, project.file(output))
         evaluateViolations.dependsOn collectViolations
         collectViolations.dependsOn detektTask
     }
 
-    private CollectDetektViolationsTask createCollectViolationsTask(Violations violations) {
-        def outputFolder = project.file(project.extensions.findByName('detekt').systemOrDefaultProfile().output)
-        project.tasks.create('collectDetektViolations', CollectDetektViolationsTask) { collectViolations ->
-            collectViolations.xmlReportFile = new File(outputFolder, 'detekt-checkstyle.xml')
-            collectViolations.htmlReportFile = new File(outputFolder, 'detekt-report.html')
-            collectViolations.violations = violations
+    private CollectDetektViolationsTask createCollectViolationsTask(Violations violations, File outputFolder) {
+        project.tasks.create('collectDetektViolations', CollectDetektViolationsTask) { task ->
+            task.xmlReportFile = new File(outputFolder, 'detekt-checkstyle.xml')
+            task.htmlReportFile = new File(outputFolder, 'detekt-report.html')
+            task.violations = violations
         }
     }
 

plugin/src/test/groovy/com/novoda/staticanalysis/internal/detekt/DetektIntegrationTest.groovy

@@ -13,6 +13,9 @@ import static com.novoda.test.LogsSubject.assertThat
 @RunWith(Parameterized.class)
 class DetektIntegrationTest {
 
+    private static final String DETEKT_NOT_APPLIED = 'The Detekt plugin is configured but not applied. Please apply the plugin in your build script.'
+    private static final String OUTPUT_NOT_DEFINED = 'Output not defined! To analyze the results, `output` needs to be defined in Detekt profile.'
+
     @Parameterized.Parameters(name = "{0}")
     static Iterable<TestProjectRule> rules() {
         return [TestProjectRule.forKotlinProject(), TestProjectRule.forAndroidKotlinProject()]
@@ -25,6 +28,26 @@ class DetektIntegrationTest {
         this.projectRule = projectRule
     }
 
+    @Test
+    void shouldFailBuildOnConfigurationWhenNoOutputNotDefined() {
+        def emptyConfiguration = detektWith("")
+
+        def result = createProjectWithZeroThreshold(Fixtures.Detekt.SOURCES_WITH_WARNINGS)
+                .withToolsConfig(emptyConfiguration)
+                .buildAndFail('check')
+
+        assertThat(result.logs).contains(OUTPUT_NOT_DEFINED)
+    }
+
+    @Test
+    void shouldFailBuildOnConfigurationWhenDetektConfiguredButNotApplied() {
+        def result = projectRule.newProject()
+                .withToolsConfig(detektConfiguration(Fixtures.Detekt.SOURCES_WITH_ERRORS))
+                .buildAndFail('check')
+
+        assertThat(result.logs).contains(DETEKT_NOT_APPLIED)
+    }
+
     @Test
     void shouldFailBuildWhenDetektWarningsOverTheThreshold() {
         def result = createProjectWithZeroThreshold(Fixtures.Detekt.SOURCES_WITH_WARNINGS)
@@ -79,7 +102,7 @@ class DetektIntegrationTest {
                     maxWarnings = 0
                     maxErrors = 0
                 }''')
-        testProject = testProject.withToolsConfig(detektConfigurationWithoutInput(testProject))
+                .withToolsConfig(detektConfigurationWithoutInput())
 
         TestProject.Result result = testProject
                 .build('check')
@@ -88,36 +111,19 @@ class DetektIntegrationTest {
         assertThat(result.logs).doesNotContainDetektViolations()
     }
 
-    @Test
-    void shouldFailBuildWhenDetektConfiguredButNotApplied() {
-        def testProject = projectRule.newProject()
-                .withPenalty('''{
-                    maxWarnings = 0
-                    maxErrors = 0
-                }''')
-
-        testProject = testProject.withToolsConfig(detektConfiguration(testProject, Fixtures.Detekt.SOURCES_WITH_ERRORS))
-
-        TestProject.Result result = testProject
-                .buildAndFail('check')
-
-        assertThat(result.logs).containsDetektNotApplied()
-    }
-
     private TestProject createProjectWithZeroThreshold(File sources) {
         createProjectWith(sources)
     }
 
     private TestProject createProjectWith(File sources, int maxWarnings = 0, int maxErrors = 0) {
-        def testProject = projectRule.newProject()
+        projectRule.newProject()
                 .withPlugin("io.gitlab.arturbosch.detekt", "1.0.0.RC6-2")
                 .withSourceSet('main', sources)
                 .withPenalty("""{
                     maxWarnings = ${maxWarnings}
                     maxErrors = ${maxErrors}
                 }""")
-
-        testProject.withToolsConfig(detektConfiguration(testProject, sources))
+                .withToolsConfig(detektConfiguration(sources))
     }
 
     private TestProject createProjectWithoutDetekt() {
@@ -130,27 +136,29 @@ class DetektIntegrationTest {
                 }''')
     }
 
-    private static GString detektConfiguration(TestProject testProject, File input) {
+    private static String detektConfiguration(File input) {
+        detektWith """
+            config = '${Fixtures.Detekt.RULES}' 
+            output = "\$buildDir/reports"
+            // The input just needs to be configured for the tests. 
+            // Probably detekt doesn't pick up the changed source sets. 
+            // In a example project it was not needed.
+            input = "${input}"
         """
-        detekt { 
-            profile('main') { 
-                config = "${Fixtures.Detekt.RULES}" 
-                output = "${testProject.projectDir()}/build/reports"
-                // The input just needs to be configured for the tests. 
-                // Probably detekt doesn't pick up the changed source sets. 
-                // In a example project it was not needed.
-                input = "${input}"
-            }
-        }
+    }
+
+    private static String detektConfigurationWithoutInput() {
+        detektWith """
+            config = '${Fixtures.Detekt.RULES}' 
+            output = "\$buildDir/reports"
         """
     }
 
-    private static GString detektConfigurationWithoutInput(TestProject testProject) {
+    private static String detektWith(String mainProfile) {
         """
         detekt { 
             profile('main') { 
-                config = "${Fixtures.Detekt.RULES}" 
-                output = "${testProject.projectDir()}/build/reports"
+                ${mainProfile.stripIndent()}
             }
         }
         """

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

@@ -14,7 +14,6 @@ import static com.novoda.test.TestProject.Result.Logs;
 class LogsSubject extends Subject<LogsSubject, Logs> {
 
     private static final String VIOLATIONS_LIMIT_EXCEEDED = 'Violations limit exceeded'
-    private static final String DETEKT_NOT_APPLIED = 'The Detekt plugin is configured but not applied. Please apply the plugin in your build script.'
     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'
@@ -44,8 +43,8 @@ class LogsSubject extends Subject<LogsSubject, Logs> {
         check().that(actual().output)
     }
 
-    public void containsDetektNotApplied() {
-        outputSubject.contains(DETEKT_NOT_APPLIED)
+    public void contains(log) {
+        outputSubject.contains(log)
     }
 
     public void doesNotContainLimitExceeded() {