Skip to content

Commit

Permalink
fix: use the sq 9.8 api to get the binaries (#706)
Browse files Browse the repository at this point in the history
- JavaResourceLocator.classFilesToAnalyze() is populated as a side
effect of the sonar-java plugin analysis. When a cached analysis is done
this will not return all .class files and this will cause the SpotBugs
analysis to fail
- Instead use the new API added in SonarQube 9.8 to get the binary
folders (main and test)
- In order to remain compatible with pre-9.8 call the new method using
reflection
- Now that we get the test binaries, updated ByteCodeResourceLocator to
also look for sources in the standard maven/java folder so we can report
issues in test sources
- Added a unit test to a sample project so the intergration test can
confirm that we report the issues
  • Loading branch information
gtoison authored Feb 9, 2023
1 parent ea4265b commit 52779de
Show file tree
Hide file tree
Showing 11 changed files with 307 additions and 44 deletions.
4 changes: 4 additions & 0 deletions pom.xml
Original file line number Diff line number Diff line change
Expand Up @@ -344,6 +344,10 @@
<argLine>${surefireArgLine}</argLine>
<disableModules>false</disableModules>
<useModulePath>false</useModulePath>
<!-- Exclude the unit tests from the sample projects -->
<excludes>
<exclude>projects/**</exclude>
</excludes>
</configuration>
</plugin>
<plugin>
Expand Down
66 changes: 53 additions & 13 deletions src/main/java/org/sonar/plugins/findbugs/FindbugsConfiguration.java
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@
import java.io.Writer;
import java.nio.charset.StandardCharsets;
import java.util.ArrayList;
import java.util.Collection;
import java.util.LinkedList;
import java.util.List;
import java.util.Optional;
Expand Down Expand Up @@ -53,6 +54,8 @@
import org.sonar.api.scan.filesystem.PathResolver;
import org.sonar.api.utils.log.Logger;
import org.sonar.api.utils.log.Loggers;
import org.sonar.plugins.findbugs.classpath.ClasspathLocator;
import org.sonar.plugins.findbugs.classpath.DefaultClasspathLocator;
import org.sonar.plugins.findbugs.rules.FbContribRulesDefinition;
import org.sonar.plugins.findbugs.rules.FindSecurityBugsRulesDefinition;
import org.sonar.plugins.findbugs.rules.FindbugsRulesDefinition;
Expand Down Expand Up @@ -94,12 +97,21 @@ public File getTargetXMLReport() {
}

public void initializeFindbugsProject(Project findbugsProject) throws IOException {
List<File> classFilesToAnalyze = buildClassFilesToAnalyze();
initializeFindbugsProject(findbugsProject, new DefaultClasspathLocator(javaResourceLocator));
}

void initializeFindbugsProject(Project findbugsProject, ClasspathLocator classpathLocator) throws IOException {
List<File> classFilesToAnalyze = buildClassFilesToAnalyze(classpathLocator);

for (File file : javaResourceLocator.classpath()) {
for (File file : classpathLocator.classpath()) {
//Auxiliary dependencies
findbugsProject.addAuxClasspathEntry(file.getCanonicalPath());
}

for (File file : classpathLocator.testClasspath()) {
//Auxiliary tests dependencies
findbugsProject.addAuxClasspathEntry(file.getCanonicalPath());
}

ClassScreener classScreener = getOnlyAnalyzeFilter();

Expand Down Expand Up @@ -243,23 +255,47 @@ File saveIncludeConfigXml() throws IOException {
return file;
}

private List<File> buildClassFilesToAnalyze() throws IOException {
private List<File> buildClassFilesToAnalyze(ClasspathLocator classpathLocator) throws IOException {
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<>(javaResourceLocator.classFilesToAnalyze());

addClassFilesFromClasspath(classFilesToAnalyze, binaryDirs);

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

addClassFilesFromClasspath(classFilesToAnalyze, classpathLocator.testBinaryDirs());

return classFilesToAnalyze;
}
}

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

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

Collection<File> classpath = javaResourceLocator.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);
addClassFilesFromClasspath(classFilesToAnalyze, classpath);
} else if (hasJspFiles) {
// Add the precompiled JSP .class files
addPrecompiledJspClasses(classFilesToAnalyze);
addPrecompiledJspClasses(classFilesToAnalyze, classpath);
} else if (classFilesToAnalyze.isEmpty()) {
// For some users javaResourceLocator.classFilesToAnalyze() seems to return an empty list, it is unclear why
addClassFilesFromClasspath(classFilesToAnalyze);
addClassFilesFromClasspath(classFilesToAnalyze, classpath);
}

return classFilesToAnalyze;
Expand All @@ -272,9 +308,13 @@ private List<File> buildClassFilesToAnalyze() throws IOException {
*
* @throws IOException In case an exception was thrown when building a file canonical path
*/
public void addPrecompiledJspClasses(List<File> classFilesToAnalyze) throws IOException {
addClassFilesFromClasspath(classFilesToAnalyze, FindbugsConfiguration::isPrecompiledJspClassFile);
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);

if (!hasPrecompiledJsp) {
Expand All @@ -289,12 +329,12 @@ public void addPrecompiledJspClasses(List<File> classFilesToAnalyze) throws IOEx
*
* @param classFilesToAnalyze The current list of class files to analyze
*/
private void addClassFilesFromClasspath(List<File> classFilesToAnalyze) {
addClassFilesFromClasspath(classFilesToAnalyze, f -> f.getName().endsWith(".class"));
private void addClassFilesFromClasspath(Collection<File> classFilesToAnalyze, Collection<File> classpath) {
addClassFilesFromClasspath(classFilesToAnalyze, classpath, f -> f.getName().endsWith(".class"));
}

private void addClassFilesFromClasspath(List<File> classFilesToAnalyze, Predicate<File> filePredicate) {
for (File file : javaResourceLocator.classpath()) {
private void addClassFilesFromClasspath(Collection<File> classFilesToAnalyze, Collection<File> classpath, Predicate<File> filePredicate) {
for (File file : classpath) {
//Will capture additional classes including precompiled JSP
if(file.isDirectory()) { // will include "/target/classes" and other non-standard folders
classFilesToAnalyze.addAll(scanForAdditionalClasses(file, filePredicate));
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,38 @@
/*
* SonarQube Findbugs Plugin
* Copyright (C) 2012 SonarSource
* [email protected]
*
* This program is free software; you can redistribute it and/or
* modify it under the terms of the GNU Lesser General Public
* License as published by the Free Software Foundation; either
* version 3 of the License, or (at your option) any later version.
*
* This program is distributed in the hope that it will be useful,
* but WITHOUT ANY WARRANTY; without even the implied warranty of
* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
* Lesser General Public License for more details.
*
* You should have received a copy of the GNU Lesser General Public
* License along with this program; if not, write to the Free Software
* Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02
*/
package org.sonar.plugins.findbugs.classpath;

import java.io.File;
import java.util.Collection;

/**
* @author gtoison
*
*/
public interface ClasspathLocator {

Collection<File> binaryDirs();

Collection<File> classpath();

Collection<File> testBinaryDirs();

Collection<File> testClasspath();
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,81 @@
/*
* SonarQube Findbugs Plugin
* Copyright (C) 2012 SonarSource
* [email protected]
*
* This program is free software; you can redistribute it and/or
* modify it under the terms of the GNU Lesser General Public
* License as published by the Free Software Foundation; either
* version 3 of the License, or (at your option) any later version.
*
* This program is distributed in the hope that it will be useful,
* but WITHOUT ANY WARRANTY; without even the implied warranty of
* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
* Lesser General Public License for more details.
*
* You should have received a copy of the GNU Lesser General Public
* License along with this program; if not, write to the Free Software
* Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02
*/
package org.sonar.plugins.findbugs.classpath;

import java.io.File;
import java.lang.reflect.InvocationTargetException;
import java.lang.reflect.Method;
import java.util.Collection;
import java.util.Collections;

import org.sonar.api.utils.log.Logger;
import org.sonar.api.utils.log.Loggers;
import org.sonar.plugins.java.api.JavaResourceLocator;

/**
* @author gtoison
*
*/
public class DefaultClasspathLocator implements ClasspathLocator {
@SuppressWarnings("rawtypes")
private static final Class[] EMPTY_CLASS_ARRAY = new Class[0];
private static final Object[] EMPTY_OBJECT_ARRAY = new Object[0];

private static final Logger LOG = Loggers.get(DefaultClasspathLocator.class);

private JavaResourceLocator javaResourceLocator;

public DefaultClasspathLocator(JavaResourceLocator javaResourceLocator) {
this.javaResourceLocator = javaResourceLocator;
}

@Override
public Collection<File> binaryDirs() {
return callNoArgMethodReturningFilesCollection("binaryDirs");
}

@Override
public Collection<File> classpath() {
return javaResourceLocator.classpath();
}

@Override
public Collection<File> testBinaryDirs() {
return callNoArgMethodReturningFilesCollection("testBinaryDirs");
}

@Override
public Collection<File> testClasspath() {
return callNoArgMethodReturningFilesCollection("testClasspath");
}

@SuppressWarnings("unchecked")
private Collection<File> callNoArgMethodReturningFilesCollection(String methodName) {
try {
Method method = JavaResourceLocator.class.getDeclaredMethod(methodName, EMPTY_CLASS_ARRAY);
return (Collection<File>) method.invoke(javaResourceLocator, EMPTY_OBJECT_ARRAY);
} catch (NoSuchMethodException | IllegalAccessException | IllegalArgumentException | InvocationTargetException e) {
LOG.info("JavaResourceLocator." + methodName + "() not available before SonarQube 9.8");
LOG.debug("Error calling JavaResourceLocator." + methodName + "()", e);

return Collections.emptySet();
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,16 @@ public class ByteCodeResourceLocator {

private static final Logger LOG = LoggerFactory.getLogger(ByteCodeResourceLocator.class);

private static final String[] SOURCE_DIRECTORIES = {"src/main/java","src/main/webapp","src/main/resources", "src", "src/java", "app", "src/main/scala"};
private static final String[] SOURCE_DIRECTORIES = {
"src/main/java",
"src/main/webapp",
"src/main/resources",
"src",
"src/java",
"app",
"src/main/scala",
"src/test/java"
};

/**
* findSourceFileKeyByClassName() is broken in SonarQube 6.3.1.. This method is fixing it.
Expand Down
Loading

0 comments on commit 52779de

Please sign in to comment.