Skip to content

Do not resolve classes dirs as a file tree #2102

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 3 commits into from
May 24, 2017
Merged

Do not resolve classes dirs as a file tree #2102

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
b46d68e
Do not resolve classes dirs as a file tree
big-guy May 22, 2017
1da7e68
Reduce visibility of getCandidateClassFiles()
big-guy May 22, 2017
9f84acb
Clarify why we have a new method
big-guy May 23, 2017
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Next Next commit
Do not resolve classes dirs as a file tree 
This causes us to include resources in a way that causes FindBugs to
produce an annoying error message
  • Loading branch information
@big-guy
big-guy committed May 22, 2017
commit b46d68ea0bcd532195804d35a0d525bd98fa0b8d
21 changes: 21 additions & 0 deletions ...egTest/groovy/org/gradle/api/plugins/quality/AbstractFindBugsPluginIntegrationTest.groovy
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -510,6 +510,27 @@ abstract class AbstractFindBugsPluginIntegrationTest extends AbstractIntegration
result.assertTaskNotSkipped(":findbugsMain")
}

def "does not fail if resources are generated into classes"() {
given:
goodCode()
buildFile << """
compileJava {
doLast {
def manifest = new File(destinationDir, "META-INF/MANIFEST.MF")
manifest.parentFile.mkdirs()
manifest.text = "manifest"
def properties = new File(destinationDir, "com/example/service.properties")
properties.parentFile.mkdirs()
properties.text = "someProp=value"
}
}
"""
when:
succeeds("check")
then:
!result.error.contains("Wrong magic bytes")
}

private static boolean containsXmlMessages(File xmlReportFile) {
new XmlSlurper().parseText(xmlReportFile.text).BugInstance.children().collect { it.name() }.containsAll(['ShortMessage', 'LongMessage'])
}
Expand Down
17 changes: 16 additions & 1 deletion subprojects/code-quality/src/main/groovy/org/gradle/api/plugins/quality/FindBugs.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,7 @@
import org.gradle.api.tasks.SourceTask;
import org.gradle.api.tasks.TaskAction;
import org.gradle.api.tasks.VerificationTask;
import org.gradle.api.tasks.util.PatternFilterable;
import org.gradle.internal.logging.ConsoleRenderer;
import org.gradle.internal.reflect.Instantiator;
import org.gradle.process.internal.worker.WorkerProcessFactory;
Expand Down Expand Up @@ -328,12 +329,26 @@ public FileTree getSource() {
@SkipWhenEmpty
@PathSensitive(PathSensitivity.RELATIVE)
@InputFiles
public FileCollection getCandidateClasses() {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A javadoc would be nice here.
We have getCandidateClassFiles() on the Test task. Should we use the same name here?
Should we make this method protected so we somewhat hide it for consumers of the class?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The javadoc is hidden in the diff.

But on second thought about this, I think filtering by .class files might be wrong here. I think nothing stops someone from doing something like:

findbugsMain {
   classes = files("someJar.jar")
}

And this would work. I took your advice and renamed the method and made it less visible.

return getClasses().getAsFileTree().matching(new Action<PatternFilterable>() {
@Override
public void execute(PatternFilterable patternFilterable) {
patternFilterable.include("**/*.class");
}
});
}

/**
* The class directories to be analyzed.
*/
@Internal
public FileCollection getClasses() {
return classes;
}


/**
* The classes to be analyzed.
* The class directories to be analyzed.
*/
public void setClasses(FileCollection classes) {
this.classes = classes;
Expand Down
2 changes: 1 addition & 1 deletion subprojects/code-quality/src/main/groovy/org/gradle/api/plugins/quality/FindBugsPlugin.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -186,7 +186,7 @@ protected void configureForSourceSet(final SourceSet sourceSet, FindBugs task) {
taskMapping.map("classes", new Callable<FileCollection>() {
@Override
public FileCollection call() {
return sourceSet.getOutput().getClassesDirs().getAsFileTree();
return sourceSet.getOutput().getClassesDirs();
}
});
taskMapping.map("classpath", new Callable<FileCollection>() {
Expand Down
2 changes: 1 addition & 1 deletion ...cts/code-quality/src/test/groovy/org/gradle/api/plugins/quality/FindBugsPluginTest.groovy
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -84,7 +84,7 @@ class FindBugsPluginTest extends AbstractProjectBuilderSpec {
description == "Run FindBugs analysis for ${sourceSet.name} classes"
source as List == sourceSet.allJava as List
findbugsClasspath == project.configurations.findbugs
classes.empty // no classes to analyze
candidateClasses.empty // no classes to analyze
reports.xml.destination == project.file("build/reports/findbugs/${sourceSet.name}.xml")
!ignoreFailures
effort == null
Expand Down
2 changes: 1 addition & 1 deletion subprojects/docs/src/docs/dsl/org.gradle.api.plugins.quality.FindBugs.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@
</thead>
<tr>
<td>classes</td>
<td><literal><replaceable>sourceSet</replaceable>.output</literal></td>
<td><literal><replaceable>sourceSet</replaceable>.output.classesDirs</literal></td>
</tr>
<tr>
<td>classpath</td>
Expand Down