Merge pull request #163 from novoda/develop

Release version 0.8
novoda · Dec 19, 2018 · 5010351 · 5010351
2 parents 208e818 + 88cb6e2
commit 5010351
Show file tree

Hide file tree

Showing 21 changed files with 143 additions and 77 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -1,6 +1,21 @@
 Change Log
 ==========
 
+[Version 0.8](https://github.com/novoda/gradle-static-analysis-plugin/releases/tag/v0.8)
+--------------------------
+
+- Fix integration for all versions of Ktlint plugin [PR#153](https://github.com/novoda/gradle-static-analysis-plugin/pull/153)
+- Make Findbugs Html report generation optional [PR#154](https://github.com/novoda/gradle-static-analysis-plugin/pull/154)
+```
+staticAnalysis {
+    findbugs {
+        htmlReportEnabled false
+    }
+}
+```
+- Display total number of errors and warnings [PR#159](https://github.com/novoda/gradle-static-analysis-plugin/pull/159)
+- Less verbose Findbugs output [PR#160](https://github.com/novoda/gradle-static-analysis-plugin/pull/160)
+
 [Version 0.7](https://github.com/novoda/gradle-static-analysis-plugin/releases/tag/v0.7)
 --------------------------
 

diff --git a/README.md b/README.md
@@ -41,7 +41,7 @@ buildscript {
        jcenter()
     }
     dependencies {
-        classpath 'com.novoda:gradle-static-analysis-plugin:0.7'
+        classpath 'com.novoda:gradle-static-analysis-plugin:0.8'
     }
 }
 
@@ -52,7 +52,7 @@ or from the [Gradle Plugins Repository](https://plugins.gradle.org/):
 
 ```gradle
 plugins {
-    id 'com.novoda.static-analysis' version '0.7'
+    id 'com.novoda.static-analysis' version '0.8'
 }
 
 ```

diff --git a/build.gradle b/build.gradle
@@ -29,7 +29,7 @@ subprojects {
     }
 }
 
-task wrapper(type: Wrapper) {
+wrapper {
     gradleVersion = '4.10.2'
     distributionType = Wrapper.DistributionType.ALL
 }
diff --git a/docs/tools/findbugs.md b/docs/tools/findbugs.md
@@ -17,6 +17,7 @@ findbugs {
     toolVersion // A string, most likely '3.0.1' — the latest Findbugs release (for a long time)
     exclude // A fileTree, such as project.fileTree('src/test/java') to exclude Java unit tests
     excludeFilter // A file containing the Findbugs exclusions, e.g., teamPropsFile('static-analysis/findbugs-excludes.xml')
+    htmlReportEnabled true // Control whether html report generation should be enabled. `true` by default.
     includeVariants { variant -> ... } // A closure to determine which variants (for Android) to include
 }
 ```

diff --git a/docs/tools/ktlint.md b/docs/tools/ktlint.md
@@ -5,7 +5,6 @@ this tool only makes sense when you have Kotlin sources in your project.
 ## Table of contents
  * [IMPORTANT: setup Ktlint](#important-setup-ktlint)
  * [Configure Ktlint](#configure-ktlint)
- * [Known Issues](#known-issues)
 ---
 
 ## IMPORTANT: setup Ktlint
@@ -56,9 +55,3 @@ For other configuration options and adding custom rules, refer to the
 
 **Note:** Failures and threshold detection is handled by Static Analysis plugin. That is why `ignoreFailures = true` is set by 
 the plugin. Please do not manually override `ignoreFailures` property. 
-
-## Known Issues
-
-6.1.0 and 6.2.1 versions are broken for Android projects because of [a bug in Ktlint](https://github.com/JLLeitschuh/ktlint-gradle/issues/153#issuecomment-437176852)
-
-Because of a behavior change, the `main` sourceSet is not checked. We recommend to use the version 5.1.0 and before.
diff --git a/gradle/publish.gradle b/gradle/publish.gradle
@@ -2,7 +2,7 @@ ext {
     websiteUrl = 'https://github.com/novoda/gradle-static-analysis-plugin'
 }
 
-version = '0.7'
+version = '0.8'
 String tag = "v$project.version"
 groovydoc.docTitle = 'Static Analysis Plugin'
 

diff --git a/plugin/src/main/groovy/com/novoda/staticanalysis/DefaultViolationsEvaluator.groovy b/plugin/src/main/groovy/com/novoda/staticanalysis/DefaultViolationsEvaluator.groovy
@@ -17,22 +17,21 @@ class DefaultViolationsEvaluator implements ViolationsEvaluator {
 
     @Override
     void evaluate(Set<Violations> allViolations) {
-        Map<String, Integer> total = [errors: 0, warnings: 0]
-        String fullMessage = '\n'
+        String fullMessage = ''
         allViolations.each { Violations violations ->
             if (!violations.isEmpty()) {
                 fullMessage += "> ${getViolationsMessage(violations, reportUrlRenderer)}\n"
             }
         }
-        total['errors'] = allViolations.collect { it.errors }.sum() as int
-        total['warnings'] = allViolations.collect { it.warnings }.sum() as int
+        int totalErrors = allViolations.collect { it.errors }.sum() as int
+        int totalWarnings = allViolations.collect { it.warnings }.sum() as int
 
-        int errorsDiff = Math.max(0, total['errors'] - penalty.maxErrors)
-        int warningsDiff = Math.max(0, total['warnings'] - penalty.maxWarnings)
+        int errorsDiff = Math.max(0, totalErrors - penalty.maxErrors)
+        int warningsDiff = Math.max(0, totalWarnings - penalty.maxWarnings)
         if (errorsDiff > 0 || warningsDiff > 0) {
-            throw new GradleException("Violations limit exceeded by $errorsDiff errors, $warningsDiff warnings.\n$fullMessage")
-        } else {
-            logger.warn fullMessage
+            throw new GradleException("Violations limit exceeded by $errorsDiff errors, $warningsDiff warnings.\n\n$fullMessage")
+        } else if (!fullMessage.isEmpty()) {
+            logger.warn "\nViolations found ($totalErrors errors, $totalWarnings warnings)\n\n$fullMessage"
         }
     }
 

diff --git a/plugin/src/main/groovy/com/novoda/staticanalysis/internal/CodeQualityConfigurator.groovy b/plugin/src/main/groovy/com/novoda/staticanalysis/internal/CodeQualityConfigurator.groovy
@@ -73,10 +73,8 @@ abstract class CodeQualityConfigurator<T extends SourceTask, E extends CodeQuali
     protected abstract Class<E> getExtensionClass()
 
     protected Action<E> getDefaultConfiguration() {
-        new Action<E>() {
-            void execute(E ignored) {
-                // no op
-            }
+        return { ignored ->
+            // no op
         }
     }
 

diff --git a/...c/main/groovy/com/novoda/staticanalysis/internal/checkstyle/CheckstyleConfigurator.groovy b/...c/main/groovy/com/novoda/staticanalysis/internal/checkstyle/CheckstyleConfigurator.groovy
@@ -3,7 +3,10 @@ package com.novoda.staticanalysis.internal.checkstyle
 import com.novoda.staticanalysis.Violations
 import com.novoda.staticanalysis.internal.CodeQualityConfigurator
 import com.novoda.staticanalysis.internal.QuietLogger
-import org.gradle.api.*
+import org.gradle.api.Action
+import org.gradle.api.NamedDomainObjectContainer
+import org.gradle.api.Project
+import org.gradle.api.Task
 import org.gradle.api.plugins.quality.Checkstyle
 import org.gradle.api.plugins.quality.CheckstyleExtension
 
@@ -34,11 +37,8 @@ class CheckstyleConfigurator extends CodeQualityConfigurator<Checkstyle, Checkst
 
     @Override
     protected Action<CheckstyleExtension> getDefaultConfiguration() {
-        new Action<CheckstyleExtension>() {
-            @Override
-            void execute(CheckstyleExtension checkstyleExtension) {
-                checkstyleExtension.toolVersion = '7.1.2'
-            }
+        return { extension ->
+            extension.toolVersion = '7.1.2'
         }
     }
 

diff --git a/plugin/src/main/groovy/com/novoda/staticanalysis/internal/detekt/DetektConfigurator.groovy b/plugin/src/main/groovy/com/novoda/staticanalysis/internal/detekt/DetektConfigurator.groovy
@@ -12,7 +12,7 @@ import org.gradle.api.Task
 class DetektConfigurator implements Configurator {
 
     private static final String DETEKT_PLUGIN = 'io.gitlab.arturbosch.detekt'
-    private static final String LAST_COMPATIBLE_DETEKT_VERSION = '1.0.0-RC10'
+    private static final String LAST_COMPATIBLE_DETEKT_VERSION = '1.0.0-RC12'
     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 static final String DETEKT_CONFIGURATION_ERROR = "A problem occurred while configuring Detekt. Please make sure to use a compatible version (All versions up to $LAST_COMPATIBLE_DETEKT_VERSION)"

diff --git a/...n/src/main/groovy/com/novoda/staticanalysis/internal/findbugs/FindbugsConfigurator.groovy b/...n/src/main/groovy/com/novoda/staticanalysis/internal/findbugs/FindbugsConfigurator.groovy
@@ -2,7 +2,10 @@ package com.novoda.staticanalysis.internal.findbugs
 
 import com.novoda.staticanalysis.Violations
 import com.novoda.staticanalysis.internal.CodeQualityConfigurator
-import org.gradle.api.*
+import org.gradle.api.Action
+import org.gradle.api.NamedDomainObjectContainer
+import org.gradle.api.Project
+import org.gradle.api.Task
 import org.gradle.api.file.ConfigurableFileTree
 import org.gradle.api.file.FileCollection
 import org.gradle.api.plugins.quality.FindBugs
@@ -13,6 +16,8 @@ import java.nio.file.Path
 
 class FindbugsConfigurator extends CodeQualityConfigurator<FindBugs, FindBugsExtension> {
 
+    protected boolean htmlReportEnabled = true
+
     static FindbugsConfigurator create(Project project,
                                        NamedDomainObjectContainer<Violations> violationsContainer,
                                        Task evaluateViolations) {
@@ -48,11 +53,9 @@ class FindbugsConfigurator extends CodeQualityConfigurator<FindBugs, FindBugsExt
 
     @Override
     protected Action<FindBugsExtension> getDefaultConfiguration() {
-        new Action<FindBugsExtension>() {
-            @Override
-            void execute(FindBugsExtension findBugsExtension) {
-                findBugsExtension.toolVersion = '3.0.1'
-            }
+        return { extension ->
+            extension.ext.htmlReportEnabled = { boolean enabled -> this.htmlReportEnabled = enabled }
+            extension.toolVersion = '3.0.1'
         }
     }
 
@@ -64,6 +67,7 @@ class FindbugsConfigurator extends CodeQualityConfigurator<FindBugs, FindBugsExt
             description = "Run FindBugs analysis for ${variant.name} classes"
             source = androidSourceDirs
             classpath = variant.javaCompile.classpath
+            extraArgs '-auxclasspath', androidJar
             exclude '**/*.kt'
         }
         sourceFilter.applyTo(task)
@@ -134,11 +138,15 @@ class FindbugsConfigurator extends CodeQualityConfigurator<FindBugs, FindBugsExt
         findBugs.reports.html.enabled = false
 
         def collectViolations = createViolationsCollectionTask(findBugs, violations)
-        def generateHtmlReport = createHtmlReportTask(findBugs, collectViolations.xmlReportFile, collectViolations.htmlReportFile)
-
         evaluateViolations.dependsOn collectViolations
-        collectViolations.dependsOn generateHtmlReport
-        generateHtmlReport.dependsOn findBugs
+
+        if (htmlReportEnabled) {
+            def generateHtmlReport = createHtmlReportTask(findBugs, collectViolations.xmlReportFile, collectViolations.htmlReportFile)
+            collectViolations.dependsOn generateHtmlReport
+            generateHtmlReport.dependsOn findBugs
+        } else {
+            collectViolations.dependsOn findBugs
+        }
     }
 
     private CollectFindbugsViolationsTask createViolationsCollectionTask(FindBugs findBugs, Violations violations) {
@@ -157,4 +165,7 @@ class FindbugsConfigurator extends CodeQualityConfigurator<FindBugs, FindBugsExt
         task
     }
 
+    private def getAndroidJar() {
+        "${project.android.sdkDirectory}/platforms/${project.android.compileSdkVersion}/android.jar"
+    }
 }
diff --git a/plugin/src/main/groovy/com/novoda/staticanalysis/internal/ktlint/KtlintConfigurator.groovy b/plugin/src/main/groovy/com/novoda/staticanalysis/internal/ktlint/KtlintConfigurator.groovy
@@ -84,22 +84,35 @@ class KtlintConfigurator implements Configurator {
     }
 
     private void configureAndroidWithVariants(def mainVariants) {
-        mainVariants.all { configureKtlint(it.name) }
-        variantFilter.filteredTestVariants.all { configureKtlint(it.name) }
-        variantFilter.filteredUnitTestVariants.all { configureKtlint(it.name) }
+        mainVariants.all { configureAndroidVariant(it) }
+        variantFilter.filteredTestVariants.all { configureAndroidVariant(it) }
+        variantFilter.filteredUnitTestVariants.all { configureAndroidVariant(it) }
     }
 
-    private void configureKtlint(def sourceSetName) {
+    private void configureAndroidVariant(def variant) {
+        variant.sourceSets.each { sourceSet ->
+            configureKtlint(sourceSet.name)
+        }
+    }
+
+    private void configureKtlint(String sourceSetName) {
         project.tasks.matching {
-            it.name == "ktlint${sourceSetName.capitalize()}Check"
+            isKtlintTask(it, sourceSetName.capitalize())
         }.all { Task ktlintTask ->
             def collectViolations = configureKtlintWithOutputFiles(sourceSetName, ktlintTask.reportOutputFiles)
             collectViolations.dependsOn ktlintTask
             evaluateViolations.dependsOn collectViolations
         }
     }
 
-    private def configureKtlintWithOutputFiles(def sourceSetName, Map<?, RegularFileProperty> reportOutputFiles) {
+    /**
+     * KtLint task has the following naming convention and the needed property to resolve the reportOutputFiles
+     */
+    private boolean isKtlintTask(Task task, String sourceSetName) {
+        task.name ==~ /^ktlint$sourceSetName(SourceSet)?Check$/ && task.hasProperty('reportOutputFiles')
+    }
+
+    private def configureKtlintWithOutputFiles(String sourceSetName, Map<?, RegularFileProperty> reportOutputFiles) {
         File xmlReportFile = null
         File txtReportFile = null
         reportOutputFiles.each { key, fileProp ->
@@ -120,10 +133,11 @@ class KtlintConfigurator implements Configurator {
     }
 
     private def createCollectViolationsTask(Violations violations, def sourceSetName, File xmlReportFile, File txtReportFile) {
-        project.tasks.create("collectKtlint${sourceSetName.capitalize()}Violations", CollectCheckstyleViolationsTask) { task ->
-            task.xmlReportFile = xmlReportFile
-            task.htmlReportFile = txtReportFile
-            task.violations = violations
-        }
+        CollectCheckstyleViolationsTask task =
+                project.tasks.maybeCreate("collectKtlint${sourceSetName.capitalize()}Violations", CollectCheckstyleViolationsTask)
+        task.xmlReportFile = xmlReportFile
+        task.htmlReportFile = txtReportFile
+        task.violations = violations
+        return task
     }
 }
diff --git a/plugin/src/main/groovy/com/novoda/staticanalysis/internal/pmd/PmdConfigurator.groovy b/plugin/src/main/groovy/com/novoda/staticanalysis/internal/pmd/PmdConfigurator.groovy
@@ -42,12 +42,9 @@ class PmdConfigurator extends CodeQualityConfigurator<Pmd, PmdExtension> {
 
     @Override
     protected Action<PmdExtension> getDefaultConfiguration() {
-        new Action<PmdExtension>() {
-            @Override
-            void execute(PmdExtension pmdExtension) {
-                pmdExtension.toolVersion = '5.5.1'
-                pmdExtension.rulePriority = 5
-            }
+        return { extension ->
+            extension.toolVersion = '5.5.1'
+            extension.rulePriority = 5
         }
     }
 

diff --git a/plugin/src/test/groovy/com/novoda/staticanalysis/DefaultViolationsEvaluatorTest.groovy b/plugin/src/test/groovy/com/novoda/staticanalysis/DefaultViolationsEvaluatorTest.groovy
@@ -2,6 +2,7 @@ package com.novoda.staticanalysis
 
 import org.gradle.api.GradleException
 import org.gradle.api.logging.Logger
+import org.gradle.internal.logging.ConsoleRenderer
 import org.junit.Before
 import org.junit.Rule
 import org.junit.Test
@@ -10,8 +11,7 @@ import org.mockito.ArgumentCaptor
 
 import static com.google.common.truth.Truth.assertThat
 import static org.junit.Assert.fail
-import static org.mockito.Mockito.mock
-import static org.mockito.Mockito.verify
+import static org.mockito.Mockito.*
 
 class DefaultViolationsEvaluatorTest {
 
@@ -43,7 +43,13 @@ class DefaultViolationsEvaluatorTest {
 
         evaluator.evaluate(allViolations)
 
-        assertThat(warningLog).contains("$TOOL_NAME violations found (1 errors, 0 warnings).")
+        def expected = """    
+            Violations found (1 errors, 0 warnings)
+            
+            > $TOOL_NAME violations found (1 errors, 0 warnings). See the reports at:
+            - $consoleClickableFileUrl
+            """
+        assertThat(warningLog).isEqualTo(expected.stripIndent())
     }
 
     @Test
@@ -52,7 +58,7 @@ class DefaultViolationsEvaluatorTest {
 
         evaluator.evaluate(allViolations)
 
-        assertThat(warningLog).doesNotContain("$TOOL_NAME violations found")
+        verifyZeroInteractions(logger)
     }
 
     @Test
@@ -63,7 +69,13 @@ class DefaultViolationsEvaluatorTest {
             evaluator.evaluate(allViolations)
             fail('Exception expected but not thrown')
         } catch (GradleException e) {
-            assertThat(e.message).contains('Violations limit exceeded by 0 errors, 1 warnings.')
+            def expected =
+                    """|Violations limit exceeded by 0 errors, 1 warnings.
+                       |
+                       |> $TOOL_NAME violations found (1 errors, 2 warnings). See the reports at:
+                       |- $consoleClickableFileUrl
+                       |"""
+            assertThat(e.message).isEqualTo(expected.stripMargin())
         }
     }
 
@@ -76,4 +88,8 @@ class DefaultViolationsEvaluatorTest {
         verify(logger).warn(captor.capture())
         captor.getValue()
     }
+
+    private String getConsoleClickableFileUrl() {
+        new ConsoleRenderer().asClickableFileUrl(reportFile)
+    }
 }
diff --git a/...src/test/groovy/com/novoda/staticanalysis/internal/detekt/DetektNewIntegrationTest.groovy b/...src/test/groovy/com/novoda/staticanalysis/internal/detekt/DetektNewIntegrationTest.groovy
@@ -25,6 +25,10 @@ class DetektNewIntegrationTest {
                 [TestProjectRule.forAndroidKotlinProject(), "1.0.0.RC9.2"],
                 [TestProjectRule.forKotlinProject(), "1.0.0-RC10"],
                 [TestProjectRule.forAndroidKotlinProject(), "1.0.0-RC10"],
+                [TestProjectRule.forKotlinProject(), "1.0.0-RC11"],
+                [TestProjectRule.forAndroidKotlinProject(), "1.0.0-RC11"],
+                [TestProjectRule.forKotlinProject(), "1.0.0-RC12"],
+                [TestProjectRule.forAndroidKotlinProject(), "1.0.0-RC12"],
         ]*.toArray()
     }