fix exclusions not applied for spotbugs, support apt-generated source…

…s exclusion (fixes #27)

CHANGELOG.md

@@ -8,7 +8,8 @@
     * Add [RecordTypeParameterName](https://checkstyle.sourceforge.io/config_naming.html#RecordTypeParameterName)
     * Add [PatternVariableName](https://checkstyle.sourceforge.io/config_naming.html#PatternVariableName)
 * Update spotbugs 4.0.3 -> 4.1.2
-* Update spotbugs plugin 4.1.0 -> 4.5.0    
+* Update spotbugs plugin 4.1.0 -> 4.5.0   
+* Fix exclusions apply for spotbugs, support apt-generated sources exclusion (#27)   
 
 ### 4.3.0 (2020-05-13)
 * Update spotbugs plugin to 4.1.0 (#26)

src/main/groovy/ru/vyarus/gradle/plugin/quality/QualityPlugin.groovy

@@ -202,8 +202,6 @@ class QualityPlugin implements Plugin<Project> {
                     ignoreFailures = !extension.strict
                     effort = extension.spotbugsEffort
                     reportLevel = extension.spotbugsLevel
-                    // note: default excludeFilter is not set not set in extension, instead it is directly
-                    // set to all tasks. If you try to set it in extension, value will be ignored
 
                     // in gradle 5 default 1g was changed and so spotbugs fails on large projects (recover behaviour),
                     // but not if value set manually
@@ -218,9 +216,15 @@ class QualityPlugin implements Plugin<Project> {
                 }
 
                 tasks.withType(SpotBugsTask).configureEach { task ->
+                    doFirst {
+                        configLoader.resolveSpotbugsExclude()
+                        // it is not possible to substitute filter file here (due to locked Property)
+                        // but possible to update already configured file (it must be already a temp file here)
+                        SpotbugsUtils.replaceExcludeFilter(task, extension, logger)
+                    }
                     // have to use this way instead of doFirst hook, because nothing else will work (damn props!)
                     excludeFilter.set(project.provider(new SpotbugsExclusionConfigProvider(
-                            configLoader, extension, task, logger
+                            task.name, configLoader, extension
                     )))
                     reports {
                         xml {

src/main/groovy/ru/vyarus/gradle/plugin/quality/util/SpotbugsExclusionConfigProvider.groovy

@@ -1,9 +1,7 @@
 package ru.vyarus.gradle.plugin.quality.util
 
-import com.github.spotbugs.snom.SpotBugsTask
 import groovy.transform.CompileStatic
 import org.gradle.api.file.RegularFile
-import org.slf4j.Logger
 import ru.vyarus.gradle.plugin.quality.ConfigLoader
 import ru.vyarus.gradle.plugin.quality.QualityExtension
 
@@ -18,42 +16,41 @@ import java.util.concurrent.Callable
  * One side effect of this shift caused by clean task which removes temporary copied configs, so we have now to
  * always check for already generated config existence and re-create it if removed. Overall, callback tries to
  * avoid redundant actions as much as possible.
+ * <p>
+ * Temporary exclusion file is generated ahead of time (when it can't be checked for sure) because
+ * it is called too early (so rely only on specified exclusions and not on how they apply).
+ * In warse case temporary exclusion would be a copy of default exclusion file (if no actual exclusions
+ * performed).
  *
  * @author Vyacheslav Rusakov
  * @since 12.05.2020
  */
 @CompileStatic
 class SpotbugsExclusionConfigProvider implements Callable<RegularFile> {
 
+    String taskName
     ConfigLoader loader
     QualityExtension extension
-    SpotBugsTask task
-    Logger logger
 
     // required to avoid duplicate calculations (because provider would be called multiple times)
     File computed
 
-    SpotbugsExclusionConfigProvider(ConfigLoader loader, QualityExtension extension, SpotBugsTask task, Logger logger) {
+    SpotbugsExclusionConfigProvider(String taskName, ConfigLoader loader, QualityExtension extension) {
+        this.taskName = taskName
         this.loader = loader
         this.extension = extension
-        this.task = task
-        this.logger = logger
     }
 
     @Override
     RegularFile call() throws Exception {
         // exists condition required because of possible clean task which will remove prepared
         // default file and so it must be created again (damn props!!!)
         if (computed == null || !computed.exists()) {
-            File excludeFile = loader.resolveSpotbugsExclude()
-            // spotbugs does not support exclude of SourceTask, so appending excluded classes to
-            // xml exclude filter
-            // for custom rank appending extra rank exclusion rule
-            if (extension.exclude || extension.excludeSources || extension.spotbugsMaxRank < 20) {
-                excludeFile = SpotbugsUtils.replaceExcludeFilter(task, excludeFile, extension, logger)
-            }
-
-            computed = excludeFile
+            // if any exclusions configured, tmp file would be created ahead of time (most likely it would be required)
+            computed = SpotbugsUtils.excludesFile(
+                    // NOTE: here we can't check if task has pre-configured custom file because it would lock property
+                    // so have to always use default file
+                    taskName, extension, loader.resolveSpotbugsExclude(true))
         }
         return { -> computed } as RegularFile
     }

src/main/groovy/ru/vyarus/gradle/plugin/quality/util/SpotbugsUtils.groovy

@@ -6,6 +6,7 @@ import groovy.transform.TypeCheckingMode
 import groovy.xml.XmlUtil
 import org.gradle.api.Project
 import org.gradle.api.tasks.SourceSet
+import org.gradle.api.tasks.compile.JavaCompile
 import org.slf4j.Logger
 import ru.vyarus.gradle.plugin.quality.QualityExtension
 
@@ -33,34 +34,70 @@ class SpotbugsUtils {
     }
 
     /**
-     * Replace exclusion file with extended one when exclusions are required.
+     * Spotbugs task properties may be configured only once. At that time it is too early to compute exact file,
+     * but we can assume that if excludes configured then temp file would be required. So preparing temp file
+     * ahead of time. Later, it would be filled with actual exclusions (if anything matches).
+     *
+     * @param taskName spotbugs task name
+     * @param extension extension
+     * @param configured configured exclusions file (most likely default one)
+     * @return excludes file for task configuration
+     */
+    @SuppressWarnings('FileCreateTempFile')
+    static File excludesFile(String taskName, QualityExtension extension, File configured) {
+        // spotbugs does not support exclude of SourceTask, so appending excluded classes to
+        // xml exclude filter
+        // for custom rank appending extra rank exclusion rule
+        if (extension.exclude || extension.excludeSources || extension.spotbugsMaxRank < MAX_RANK) {
+            File tmp = File.createTempFile(taskName + '-extended-exclude', '.xml')
+            tmp.deleteOnExit()
+            tmp << configured.text
+            return tmp
+        }
+        return configured
+    }
+
+    /**
+     * Extend exclusions filter file with when exclusions are required. Note: it is assumed that tmp file was already
+     * created (because it is impossible to configure different file on this stage).
+     * <p>
+     * Apt sources are also counted (to be able to ignore apt sources).
      *
      * @param task spotbugs task
      * @param extension extension instance
      * @param logger project logger for error messages
      */
     @CompileStatic(TypeCheckingMode.SKIP)
-    static File replaceExcludeFilter(SpotBugsTask task, File excludeFile, QualityExtension extension, Logger logger) {
+    static void replaceExcludeFilter(SpotBugsTask task, QualityExtension extension, Logger logger) {
         // setting means max allowed rank, but filter evicts all ranks >= specified (so +1)
         Integer rank = extension.spotbugsMaxRank < MAX_RANK ? extension.spotbugsMaxRank + 1 : null
-        // NOTE no support for new classDirs property!
+
+        SourceSet set = FileUtils.findMatchingSet('spotbugs', task.name, extension.sourceSets)
+        if (!set) {
+            logger.error("[SpotBugs] Failed to find source set for task ${task.name}: exclusions " +
+                    ' will not be applied')
+            return
+        }
+        // apt is a special dir, not mentioned in sources!
+        File aptGenerated = (task.project.tasks.findByName(set.compileJavaTaskName) as JavaCompile)
+                .options.annotationProcessorGeneratedSourcesDirectory
+
         Set<File> ignored = FileUtils.resolveIgnoredFiles(task.sourceDirs.asFileTree, extension.exclude)
+        ignored.addAll(FileUtils.resolveIgnoredFiles(task.project.fileTree(aptGenerated), extension.exclude))
         if (extension.excludeSources) {
             // add directly excluded files
             ignored.addAll(extension.excludeSources.asFileTree.matching { include '**/*.java' }.files)
         }
         if (!ignored && !rank) {
             // no custom excludes required
-            return excludeFile
-        }
-        SourceSet set = FileUtils.findMatchingSet('spotbugs', task.name, extension.sourceSets)
-        if (!set) {
-            logger.error("[SpotBugs] Failed to find source set for task ${task.name}: exclusions " +
-                    ' will not be applied')
-            return excludeFile
+            return
         }
 
-        return mergeExcludes(excludeFile, ignored, set.allJava.srcDirs, rank)
+        Collection<File> sources = []
+        sources.addAll(set.allJava.srcDirs)
+        sources.add(aptGenerated)
+
+        mergeExcludes(task.excludeFilter.get().asFile, ignored, sources, rank)
     }
 
     /**
@@ -69,13 +106,13 @@ class SpotbugsUtils {
      * <p>
      * Also, rank-based filtering is only possible through exclusions file.
      *
-     * @param src original excludes file (default of user defined)
+     * @param exclusions file to be extended (already tmp file)
      * @param exclude files to exclude (may be empty)
      * @param roots source directories (to resolve class files)
      * @param rank custom rank value (optional)
      */
     @SuppressWarnings('FileCreateTempFile')
-    static File mergeExcludes(File src, Collection<File> exclude, Collection<File> roots, Integer rank = null) {
+    static void mergeExcludes(File src, Collection<File> exclude, Collection<File> roots, Integer rank = null) {
         Node xml = new XmlParser().parse(src)
 
         exclude.each {
@@ -89,10 +126,9 @@ class SpotbugsUtils {
             xml.appendNode(MATCH).appendNode('Rank', ['value': rank])
         }
 
-        File tmp = File.createTempFile('spotbugs-extended-exclude', '.xml')
-        tmp.deleteOnExit()
-        tmp.withWriter { XmlUtil.serialize(xml, it) }
-        return tmp
+        Writer writer = src.newWriter()
+        XmlUtil.serialize(xml, writer)
+        writer.flush()
     }
 
     /**