Skip to content

Commit

Permalink
Remove ClasspathLocator.classFilesToAnalyze() (#1035)
Browse files Browse the repository at this point in the history
  • Loading branch information
@gtoison
gtoison authored Oct 22, 2024
1 parent 26126d8 commit 1b8e05c
Show file tree
Hide file tree
Showing 5 changed files with 44 additions and 182 deletions.
62 changes: 8 additions & 54 deletions src/main/java/org/sonar/plugins/findbugs/FindbugsConfiguration.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -164,10 +164,6 @@ public IllegalStateException buildMissingCompiledCodeException() {
if (classpathLocator.classpath().isEmpty()) {
message.append("\nSonar JavaResourceLocator.classpath was empty");
}

if (classpathLocator.classFilesToAnalyze().isEmpty()) {
message.append("\nSonar JavaResourceLocator.classFilesToAnalyze was empty");
}

return new IllegalStateException(message.toString());
}
Expand Down Expand Up @@ -262,66 +258,24 @@ File saveIncludeConfigXml() throws IOException {
return file;
}

private List<File> buildClassFilesToAnalyze(ClasspathLocator classpathLocator) throws IOException {
private List<File> buildClassFilesToAnalyze(ClasspathLocator classpathLocator) {
Collection<File> binaryDirs = classpathLocator.binaryDirs();

if (binaryDirs.isEmpty()) {
return buildClassFilesToAnalyzePre98();
} else {
// It's probably redundant to use javaResourceLocator.classFilesToAnalyze() here, we'll get all the binaries later
List<File> classFilesToAnalyze = new ArrayList<>(classpathLocator.classFilesToAnalyze());

addClassFilesFromClasspath(classFilesToAnalyze, binaryDirs);

boolean hasJspFiles = fileSystem.hasFiles(fileSystem.predicates().hasLanguage("jsp"));
if (hasJspFiles) {
checkForMissingPrecompiledJsp(classFilesToAnalyze);
}
List<File> classFilesToAnalyze = new ArrayList<>();

if (isAnalyzeTests()) {
addClassFilesFromClasspath(classFilesToAnalyze, classpathLocator.testBinaryDirs());
}
addClassFilesFromClasspath(classFilesToAnalyze, binaryDirs);

return classFilesToAnalyze;
boolean hasJspFiles = fileSystem.hasFiles(fileSystem.predicates().hasLanguage("jsp"));
if (hasJspFiles) {
checkForMissingPrecompiledJsp(classFilesToAnalyze);
}
}

private List<File> buildClassFilesToAnalyzePre98() throws IOException {
List<File> classFilesToAnalyze = new ArrayList<>(classpathLocator.classFilesToAnalyze());

boolean hasScalaOrKotlinFiles = fileSystem.hasFiles(fileSystem.predicates().hasLanguages("scala", "kotlin"));
boolean hasJspFiles = fileSystem.hasFiles(fileSystem.predicates().hasLanguage("jsp"));

Collection<File> classpath = classpathLocator.classpath();

// javaResourceLocator.classFilesToAnalyze() only contains .class files from Java sources
if (hasScalaOrKotlinFiles) {
// Add all the .class files from the classpath
// For Gradle multi-module projects this will unfortunately include compiled .class files from dependency modules
addClassFilesFromClasspath(classFilesToAnalyze, classpath);
} else if (hasJspFiles) {
// Add the precompiled JSP .class files
addPrecompiledJspClasses(classFilesToAnalyze, classpath);
} else if (classFilesToAnalyze.isEmpty()) {
// For some users javaResourceLocator.classFilesToAnalyze() seems to return an empty list, it is unclear why
addClassFilesFromClasspath(classFilesToAnalyze, classpath);
if (isAnalyzeTests()) {
addClassFilesFromClasspath(classFilesToAnalyze, classpathLocator.testBinaryDirs());
}

return classFilesToAnalyze;
}

/**
* Updates the class files list by adding precompiled JSP classes
*
* @param classFilesToAnalyze The current list of class files to analyze, by default SonarQube does not include precompiled JSP classes
*
* @throws IOException In case an exception was thrown when building a file canonical path
*/
public void addPrecompiledJspClasses(List<File> classFilesToAnalyze, Collection<File> classpath) throws IOException {
addClassFilesFromClasspath(classFilesToAnalyze, classpath, FindbugsConfiguration::isPrecompiledJspClassFile);

checkForMissingPrecompiledJsp(classFilesToAnalyze);
}

public void checkForMissingPrecompiledJsp(List<File> classFilesToAnalyze) {
boolean hasPrecompiledJsp = hasPrecompiledJsp(classFilesToAnalyze);
Expand Down
2 changes: 0 additions & 2 deletions src/main/java/org/sonar/plugins/findbugs/classpath/ClasspathLocator.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,4 @@ public interface ClasspathLocator {
Collection<File> testBinaryDirs();

Collection<File> testClasspath();

Collection<File> classFilesToAnalyze();
}
59 changes: 2 additions & 57 deletions src/main/java/org/sonar/plugins/findbugs/classpath/DefaultClasspathLocator.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -20,18 +20,8 @@
package org.sonar.plugins.findbugs.classpath;

import java.io.File;
import java.io.IOException;
import java.nio.file.FileVisitResult;
import java.nio.file.FileVisitor;
import java.nio.file.Files;
import java.nio.file.Path;
import java.nio.file.PathMatcher;
import java.nio.file.attribute.BasicFileAttributes;
import java.util.ArrayList;
import java.util.Collection;

import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
import org.sonar.api.batch.ScannerSide;
import org.sonar.api.batch.fs.FileSystem;
import org.sonar.api.config.Configuration;
Expand All @@ -44,10 +34,8 @@
*/
@ScannerSide
public class DefaultClasspathLocator implements ClasspathLocator {
private static final Logger LOG = LoggerFactory.getLogger(DefaultClasspathLocator.class);

private ClasspathForMain classpathForMain;
private ClasspathForTest classpathForTest;
private final ClasspathForMain classpathForMain;
private final ClasspathForTest classpathForTest;

public DefaultClasspathLocator(Configuration configuration, FileSystem fileSystem) {
classpathForMain = new ClasspathForMain(configuration, fileSystem);
Expand All @@ -73,47 +61,4 @@ public Collection<File> testBinaryDirs() {
public Collection<File> testClasspath() {
return classpathForTest.getElements();
}

@Override
public Collection<File> classFilesToAnalyze() {
ClassFileVisitor visitor = new ClassFileVisitor();
for (File binaryDir : binaryDirs()) {
try {
Files.walkFileTree(binaryDir.toPath(), visitor);
} catch (IOException e) {
LOG.error("Error listing class files to analyze", e);
}
}

return visitor.matchedFiles;
}

private static class ClassFileVisitor implements FileVisitor<Path> {
private PathMatcher classFileMatcher = p -> p.toString().endsWith(".class");
private Collection<File> matchedFiles = new ArrayList<>();

@Override
public FileVisitResult preVisitDirectory(Path dir, BasicFileAttributes attrs) throws IOException {
return FileVisitResult.CONTINUE;
}

@Override
public FileVisitResult visitFile(Path file, BasicFileAttributes attrs) throws IOException {
if (classFileMatcher.matches(file)) {
matchedFiles.add(file.toFile().getCanonicalFile());
}

return FileVisitResult.CONTINUE;
}

@Override
public FileVisitResult visitFileFailed(Path file, IOException exc) throws IOException {
throw exc;
}

@Override
public FileVisitResult postVisitDirectory(Path dir, IOException exc) throws IOException {
return FileVisitResult.CONTINUE;
}
}
}
88 changes: 34 additions & 54 deletions src/test/java/org/sonar/plugins/findbugs/FindbugsConfigurationTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -76,7 +76,7 @@ class FindbugsConfigurationTest {
private ClasspathLocator classpathLocator;

@BeforeEach
public void setUp() throws Exception {
public void setUp() {
baseDir = new File(temp, "findbugs");
workDir = new File(temp, "findbugs");

Expand Down Expand Up @@ -136,8 +136,13 @@ void should_return_confidence_level() {

@Test
void should_set_class_files() throws IOException {
File file = new File(temp, "MyClass.class");
when(classpathLocator.classFilesToAnalyze()).thenReturn(ImmutableList.of(file));
File classesDir = new File(temp, "xyz");
File file = new File(classesDir, "MyClass.class");

Files.createDirectories(classesDir.toPath());
Files.createFile(file.toPath());

when(classpathLocator.binaryDirs()).thenReturn(ImmutableList.of(classesDir));
try (Project findbugsProject = new Project()) {
conf.initializeFindbugsProject(findbugsProject);

Expand All @@ -159,7 +164,7 @@ void should_set_class_path() throws IOException {
}

@Test
void should_copy_lib_in_working_dir() throws IOException {
void should_copy_lib_in_working_dir() {
String jsr305 = "findbugs/jsr305.jar";
String annotations = "findbugs/annotations.jar";

Expand All @@ -184,13 +189,13 @@ void should_copy_lib_in_working_dir() throws IOException {
}

@Test
void should_get_fbcontrib() throws IOException {
void should_get_fbcontrib() {
conf.copyLibs();
assertThat(conf.getFbContribJar()).isFile();
}

@Test
void should_get_findSecBugs() throws IOException {
void should_get_findSecBugs() {
conf.copyLibs();
assertThat(conf.getFindSecBugsJar()).isFile();
}
Expand Down Expand Up @@ -233,62 +238,48 @@ void should_get_only_analyze_filter() {
}

@Test
void scanEmptyFolderForAdditionalClasses() throws IOException {
void scanEmptyFolderForAdditionalClasses() {
List<File> classes = FindbugsConfiguration.scanForAdditionalClasses(temp, f -> true);

assertThat(classes).isEmpty();
}

@ParameterizedTest
@CsvSource({
"true,true",
"true,false",
"false,true",
"false,false",
"true",
"false",
})
void should_warn_of_missing_precompiled_jsp(boolean withSq98Api, boolean analyzeTests) throws IOException {
setupSampleProject(false, true, false, withSq98Api, analyzeTests);
void should_warn_of_missing_precompiled_jsp(boolean analyzeTests) throws IOException {
setupSampleProject(false, true, false, analyzeTests);

try (Project project = new Project()) {
conf.initializeFindbugsProject(project, classpathLocator);
}

// With the pre SonarQube 9.8 we There should be two warnings:
// - There are JSP but they are not precompiled
// - Findbugs needs sources to be compiled
// With the SonarQube 9.8+ API we get the Test.class so only one warning
if (withSq98Api) {
assertThat(logTester.getLogs(Level.WARN)).hasSize(1);
} else {
assertThat(logTester.getLogs(Level.WARN)).hasSize(2);
}
assertThat(logTester.getLogs(Level.WARN)).hasSize(1);
}

@ParameterizedTest
@CsvSource({
"true,true",
"true,false",
"false,true",
"false,false",
"true",
"false",
})
void should_analyze_precompiled_jsp(boolean withSq98Api, boolean analyzeTests) throws IOException {
setupSampleProject(true, true, false, withSq98Api, analyzeTests);
void should_analyze_precompiled_jsp(boolean analyzeTests) throws IOException {
setupSampleProject(true, true, false, analyzeTests);

try (Project project = new Project()) {
conf.initializeFindbugsProject(project, classpathLocator);

if (withSq98Api && analyzeTests) {
if (analyzeTests) {
// we should also capture the .class that are not from JSP sources and also the unit tests
assertThat(project.getFileCount()).isEqualTo(5);

verify(classpathLocator, times(1)).testClasspath();
} else if (withSq98Api) {
} else {
// we should also capture the .class that are not from JSP sources but not the unit tests
assertThat(project.getFileCount()).isEqualTo(4);

verify(classpathLocator, never()).testClasspath();
} else {
assertThat(project.getFileCount()).isEqualTo(3);
}
}

Expand All @@ -297,32 +288,27 @@ void should_analyze_precompiled_jsp(boolean withSq98Api, boolean analyzeTests) t

@ParameterizedTest
@CsvSource({
"true,true",
"true,false",
"false,true",
"false,false",
"true",
"false",
})
void scala_project(boolean withSq98Api, boolean analyzeTests) throws IOException {
setupSampleProject(false, false, true, withSq98Api, analyzeTests);
void scala_project(boolean analyzeTests) throws IOException {
setupSampleProject(false, false, true, analyzeTests);

try (Project project = new Project()) {
conf.initializeFindbugsProject(project, classpathLocator);

if (withSq98Api && analyzeTests) {
if (analyzeTests) {
assertThat(project.getFileCount()).isEqualTo(2);
assertThat(project.getFile(0)).endsWith("Test.class");
assertThat(project.getFile(1)).endsWith("UnitTest.class");

verify(classpathLocator, times(1)).testClasspath();
} else if (withSq98Api) {
} else {
assertThat(project.getFileCount()).isEqualTo(1);
// Even though it is named "Test" it is in the "main" folder so it should be analyzed
assertThat(project.getFile(0)).endsWith("Test.class");

verify(classpathLocator, never()).testClasspath();
} else {
assertThat(project.getFileCount()).isEqualTo(1);
assertThat(project.getFile(0)).endsWith("Test.class");
}
}

Expand All @@ -332,7 +318,6 @@ void scala_project(boolean withSq98Api, boolean analyzeTests) throws IOException
private void setupSampleProject(boolean withPrecompiledJsp,
boolean withJspFiles,
boolean withScalaFiles,
boolean withSq98Api,
boolean analyzeTests) throws IOException {
configuration.setProperty(FindbugsConstants.ANALYZE_TESTS, Boolean.toString(analyzeTests));

Expand Down Expand Up @@ -396,13 +381,11 @@ private void setupSampleProject(boolean withPrecompiledJsp,
when(classpathLocator.classpath()).thenReturn(classpathFiles);
when(classpathLocator.testClasspath()).thenReturn(testClasspathFiles);

if (withSq98Api) {
List<File> binaryDirs = Arrays.asList(classesFolder, jspServletFolder);
List<File> testBinaryDirs = Collections.singletonList(testClassesFolder);

when(classpathLocator.binaryDirs()).thenReturn(binaryDirs);
when(classpathLocator.testBinaryDirs()).thenReturn(testBinaryDirs);
}
List<File> binaryDirs = Arrays.asList(classesFolder, jspServletFolder);
List<File> testBinaryDirs = Collections.singletonList(testClassesFolder);

when(classpathLocator.binaryDirs()).thenReturn(binaryDirs);
when(classpathLocator.testBinaryDirs()).thenReturn(testBinaryDirs);
}

private void mockHasLanguagePredicate(boolean predicateReturn, String language) {
Expand Down Expand Up @@ -434,18 +417,15 @@ private void mockHasLanguagesPredicate(boolean predicateReturn, String ... langu

@Test
void buildMissingCompiledCodeException() {
when(classpathLocator.classFilesToAnalyze()).thenReturn(Collections.emptyList());
when(classpathLocator.classpath()).thenReturn(Collections.emptyList());

assertThat(conf.buildMissingCompiledCodeException().getMessage()).contains("Property sonar.java.binaries was not set");
assertThat(conf.buildMissingCompiledCodeException().getMessage()).contains("Sonar JavaResourceLocator.classpath was empty");
assertThat(conf.buildMissingCompiledCodeException().getMessage()).contains("Sonar JavaResourceLocator.classFilesToAnalyze was empty");

configuration.setProperty(FindbugsConfiguration.SONAR_JAVA_BINARIES, "foo/bar");

assertThat(conf.buildMissingCompiledCodeException().getMessage()).contains("sonar.java.binaries was set to");

when(classpathLocator.classFilesToAnalyze()).thenReturn(Collections.singletonList(baseDir));
when(classpathLocator.classpath()).thenReturn(Collections.singletonList(baseDir));

assertThat(conf.buildMissingCompiledCodeException().getMessage()).doesNotContain("Sonar JavaResourceLocator.classpath was empty");
Expand Down
Loading

0 comments on commit 1b8e05c

Please sign in to comment.