-
Notifications
You must be signed in to change notification settings - Fork 725
SONARJAVA-6518 Prevent JAVA analysis failure when missing java binaries #5712
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.
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
base: master
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -33,6 +33,7 @@ | |
| import java.util.Arrays; | ||
| import java.util.Collection; | ||
| import java.util.Collections; | ||
| import java.util.Iterator; | ||
| import java.util.LinkedHashSet; | ||
| import java.util.List; | ||
| import java.util.Optional; | ||
|
|
@@ -42,10 +43,12 @@ | |
| import org.slf4j.Logger; | ||
| import org.slf4j.LoggerFactory; | ||
| import org.sonar.api.batch.ScannerSide; | ||
| import org.sonar.api.batch.fs.FilePredicate; | ||
| import org.sonar.api.batch.fs.FileSystem; | ||
| import org.sonar.api.batch.fs.InputFile; | ||
| import org.sonar.api.config.Configuration; | ||
| import org.sonar.java.collections.CollectionUtils; | ||
| import org.sonar.java.AnalysisWarningsWrapper; | ||
| import org.sonar.java.SonarComponents; | ||
| import org.sonarsource.api.sonarlint.SonarLintSide; | ||
|
|
||
| @ScannerSide | ||
|
|
@@ -58,26 +61,67 @@ public abstract class AbstractClasspath { | |
| protected final Configuration settings; | ||
| protected final FileSystem fs; | ||
| private final InputFile.Type fileType; | ||
| protected final String binariesProperty; | ||
| protected final String librariesProperty; | ||
| private static final Path[] STANDARD_CLASSES_DIRS = {Paths.get("target", "classes"), Paths.get("target", "test-classes")}; | ||
|
|
||
| protected final List<File> binaries; | ||
| protected final List<File> elements; | ||
| protected List<File> elements; | ||
| protected boolean validateLibraries; | ||
| protected boolean initialized; | ||
| private boolean inAndroidContext = false; | ||
| private final Set<String> classpathWarnings; | ||
| protected final AnalysisWarningsWrapper analysisWarnings; | ||
|
|
||
| protected AbstractClasspath(Configuration settings, FileSystem fs, InputFile.Type fileType) { | ||
| protected AbstractClasspath(Configuration settings, FileSystem fs, InputFile.Type fileType, String binariesProperty, String librariesProperty, | ||
| AnalysisWarningsWrapper analysisWarnings) { | ||
| this.settings = settings; | ||
| this.fs = fs; | ||
| this.fileType = fileType; | ||
| this.binariesProperty = binariesProperty; | ||
| this.librariesProperty = librariesProperty; | ||
| this.binaries = new ArrayList<>(); | ||
| this.elements = new ArrayList<>(); | ||
| this.elements = List.of(); | ||
| this.analysisWarnings = analysisWarnings; | ||
| classpathWarnings = new LinkedHashSet<>(); | ||
| initialized = false; | ||
| } | ||
|
|
||
| protected void init() { | ||
| if (!initialized) { | ||
| initialized = true; | ||
| validateLibraries = hasJavaFiles(); | ||
| validatePropertiesPresence(binariesProperty, librariesProperty); | ||
| binaries.addAll(getFilesFromProperty(binariesProperty)); | ||
| Set<File> libraries = new LinkedHashSet<>(getJdkJars()); | ||
| Set<File> extraLibraries = getFilesFromProperty(librariesProperty); | ||
| libraries.addAll(extraLibraries); | ||
| logResolvedFiles(binariesProperty, binaries); | ||
| logResolvedFiles(librariesProperty, libraries); | ||
| Set<File> all = new LinkedHashSet<>(binaries); | ||
| all.addAll(libraries); | ||
| elements = List.copyOf(all); | ||
| } | ||
| } | ||
|
|
||
| protected void validatePropertiesPresence(String binariesProperty, String librariesProperty) { | ||
| if (settings.getBoolean(SonarComponents.SONAR_AUTOSCAN).orElse(false)) { | ||
| return; | ||
| } | ||
| boolean missingBinary = !settings.hasKey(binariesProperty) && hasMoreThanOneJavaFile(); | ||
| boolean missingLibraries = !settings.hasKey(librariesProperty) && hasJavaFiles(); | ||
| if (missingBinary && missingLibraries) { | ||
| classpathWarnings.add(String.format("Missing '%s' and '%s' properties. You might end up with less precise analysis results.", binariesProperty, librariesProperty)); | ||
| } else if (missingBinary) { | ||
| classpathWarnings.add(String.format("Missing '%s' property. You might end up with less precise analysis results.", binariesProperty)); | ||
| } else if (missingLibraries) { | ||
| classpathWarnings.add(String.format("Missing '%s' property. You might end up with less precise analysis results.", librariesProperty)); | ||
| } | ||
| } | ||
|
|
||
| protected List<File> getJdkJars() { | ||
| List<File> jdkClassesRoots = settings.get(ClasspathProperties.SONAR_JAVA_JDK_HOME) | ||
| .flatMap(AbstractClasspath::existingDirectoryOrLog) | ||
| .flatMap(this::existingDirectoryOrLog) | ||
| .map(File::toPath) | ||
| .map(JavaSdkUtil::getJdkClassesRoots) | ||
| .orElse(Collections.emptyList()); | ||
|
|
@@ -87,26 +131,29 @@ protected List<File> getJdkJars() { | |
|
|
||
| static void logResolvedFiles(String property, Collection<File> files) { | ||
| if (LOG.isDebugEnabled()) { | ||
| LOG.debug(String.format("Property '%s' resolved with:%n%s", property, files.stream() | ||
| LOG.debug(String.format("Property '%s' resolved with: %s", property, files.stream() | ||
| .map(File::getAbsolutePath) | ||
| .collect(Collectors.joining("," + System.lineSeparator(), "[", "]")))); | ||
| .collect(Collectors.joining(",", "[", "]")))); | ||
| } | ||
| } | ||
|
|
||
| private static Optional<File> existingDirectoryOrLog(String path) { | ||
| private Optional<File> existingDirectoryOrLog(String path) { | ||
| LOG.debug("Property '{}' set with: {}", ClasspathProperties.SONAR_JAVA_JDK_HOME, path); | ||
| File file = new File(path); | ||
| if (!file.exists() || !file.isDirectory()) { | ||
| LOG.warn("Invalid value for '{}' property, defaulting to runtime JDK.{}Configured location does not exists: '{}'", | ||
| ClasspathProperties.SONAR_JAVA_JDK_HOME, System.lineSeparator(), file.getAbsolutePath()); | ||
| classpathWarnings.add(String.format("Invalid value '%s' for '%s' property, defaulting to runtime JDK.", file.getAbsolutePath(), ClasspathProperties.SONAR_JAVA_JDK_HOME)); | ||
| return Optional.empty(); | ||
| } | ||
| return Optional.of(file); | ||
| } | ||
|
|
||
| protected abstract void init(); | ||
|
|
||
| public abstract void logSuspiciousEmptyLibraries(); | ||
| public void logClasspathWarnings() { | ||
| for (String warning : classpathWarnings) { | ||
| LOG.warn(warning); | ||
| analysisWarnings.addUnique(warning); | ||
| } | ||
| classpathWarnings.clear(); | ||
| } | ||
|
Comment on lines
+150
to
+156
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
|
||
|
|
||
| protected Set<File> getFilesFromProperty(String property) { | ||
| Set<File> result = new LinkedHashSet<>(); | ||
|
|
@@ -120,9 +167,7 @@ protected Set<File> getFilesFromProperty(String property) { | |
| for (String pathPattern : fileNames) { | ||
| Set<File> libraryFilesForPattern = getFilesForPattern(baseDir.toPath(), pathPattern, isLibraryProperty); | ||
| if (validateLibraries && libraryFilesForPattern.isEmpty() && hasJavaSources) { | ||
| LOG.error("Invalid value for '{}' property.", property); | ||
| String message = "No files nor directories matching '" + pathPattern + "'"; | ||
| throw new IllegalStateException(message); | ||
| classpathWarnings.add(String.format("Invalid value for '%s', no files nor directories matching '%s'.", property, pathPattern)); | ||
| } | ||
| validateLibraries = validateLibs; | ||
| result.addAll(libraryFilesForPattern); | ||
|
|
@@ -135,11 +180,25 @@ protected Set<File> getFilesFromProperty(String property) { | |
| } | ||
|
|
||
| protected boolean hasJavaSources() { | ||
| return fs.hasFiles(fs.predicates().and(fs.predicates().hasLanguage("java"), fs.predicates().hasType(fileType))); | ||
| return fs.hasFiles(sourcePredicate()); | ||
| } | ||
|
|
||
| protected boolean hasMoreThanOneJavaFile() { | ||
| return CollectionUtils.size(fs.inputFiles(fs.predicates().and(fs.predicates().hasLanguage("java"), fs.predicates().hasType(fileType)))) > 1; | ||
| // No need to iterate over the entire collection, checking that there are two elements is enough | ||
| Iterator<InputFile> iterator = fs.inputFiles(sourcePredicate()).iterator(); | ||
| if (iterator.hasNext()) { | ||
| iterator.next(); | ||
| return iterator.hasNext(); | ||
| } | ||
| return false; | ||
| } | ||
|
|
||
| protected boolean hasJavaFiles() { | ||
| return fs.hasFiles(sourcePredicate()); | ||
| } | ||
|
|
||
| private FilePredicate sourcePredicate() { | ||
| return fs.predicates().and(fs.predicates().hasLanguage("java"), fs.predicates().hasType(fileType)); | ||
| } | ||
|
|
||
| private Set<File> getFilesForPattern(Path baseDir, String pathPattern, boolean libraryProperty) { | ||
|
|
||
This file was deleted.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
All classpath diagnostics are now accumulated into
classpathWarningsand only flushed byAbstractClasspath.logClasspathWarnings(). The only production call site isSonarComponents.logUndefinedTypes()(java-frontend/src/main/java/org/sonar/java/SonarComponents.java:615-624), but the twologClasspathWarnings()calls are placed after the early-return guard:Consequently, whenever an analysis produces no undefined types, every accumulated classpath warning is silently discarded — including warnings that were previously surfaced unconditionally. In particular, the invalid
sonar.java.jdkHomewarning used to be emitted immediately viaLOG.warn(...)insideexistingDirectoryOrLog(see the removed code in AbstractClasspath.java:140-148); it is now deferred intoclasspathWarningsand will not be logged at all ifproblemsToFilePathsis empty. The same applies to the new 'Missing sonar.java.binaries/libraries' and 'Invalid value for ...' warnings (AbstractClasspath.java:114-119,170,144). This is a regression in user-facing diagnostics introduced by this PR.Fix: flush the classpath warnings before the
isEmpty()early-return so they are always emitted regardless of undefined types.Move the classpath-warning flush above the early-return guard so warnings are always emitted.:
Check the box to apply the fix or reply for a change | Was this helpful? React with 👍 / 👎