PT-427: Make Findbugs to honour the exclude filters #20
Conversation
| /** | ||
| * The simple "classes = sourceSet.output" may lead to non-existing resources directory | ||
| * being passed to FindBugs Ant task, resulting in an error | ||
| * */ |
There was a problem hiding this comment.
Is above comment still applicable?
There was a problem hiding this comment.
Well, we are not changing the default behaviour here, instead we stack filters upon the default implementation: this method has been copied (and groovyfied) from the original FindbugsPlugin#L189-L191. Should we reference the source?
| createIncludePatterns(includedSourceFiles, sourceDirs.collect { it.toPath() }) | ||
| } | ||
|
|
||
| private static List<String> createIncludePatterns(List<Path> includedSourceFiles, List<Path> sourceDirs) { |
There was a problem hiding this comment.
NIT: Here it's ...Include..., in the method name above ...Includes....
| } | ||
| } | ||
| pmd.mustRunAfter variant.javaCompile | ||
| sourceFilter.applyTo(task) | ||
| task.mustRunAfter variant.javaCompile |
There was a problem hiding this comment.
Is the mustRunAfter still needed here?
There was a problem hiding this comment.
Might be to ensure the compiler fails first, even before the codestyle checks run, right?
There was a problem hiding this comment.
I'm actually not sure this mustRunAfter is correct here to be honest - like we briefly discussed last week, I reckon we should consider the use of dependsOn instead.
| .withPenalty('''{ | ||
| maxErrors = 0 | ||
| maxWarnings = 10 | ||
| }''') | ||
| .withFindbugs('findbugs { exclude "HighPriorityViolator.java" }') | ||
| .withFindbugs('findbugs { exclude "com/novoda/test/HighPriorityViolator.java" }') |
Scope of the PR
Findbugs is currently ignoring the exclude filters specified as part of the plugin configuration.
The issue is related to the fact the tool is working against class files while the exclude filters are applied only to sources. To solve the problem we need to filter out the classes produced from source files excluded in the config.
Considerations and implementation
We decided to create include patterns for the
classesproperty of theFindbugstask evaluating the filtered set of source files specified viasource(and after theexcludefilters from the plugin extension have been applied), for instance:com.novoda.test.HighPriorityViolator.javacom/novoda/test/HighPriorityViolator*Note that the
*at the end will allow to catch inner classes as well.Incidentally the internals of
*Configuratorclasses have been refactored to expose a clean configuration point for Java projects viaconfigureJavaProject().Test(s) added
FindbugsIntegrationTestis now testing the case where we provide anexcludefilter to skip one specific file from the sources as opposed to the entire source set as before. The source file used in the test is also now using a proper package and contains an inner class, to make the test more significative.