SONARJAVA-6518 Prevent JAVA analysis failure when missing java binaries#5712
SONARJAVA-6518 Prevent JAVA analysis failure when missing java binaries#5712alban-auzeill wants to merge 1 commit into
Conversation
![gitar-bot[bot]](https://avatars.githubusercontent.com/in/827041?s=60&v=4){kind=small}
| 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); | ||
| } |
There was a problem hiding this comment.
⚠️ Bug: Classpath warnings dropped when no undefined types are reported
All classpath diagnostics are now accumulated into classpathWarnings and only flushed by AbstractClasspath.logClasspathWarnings(). The only production call site is SonarComponents.logUndefinedTypes() (java-frontend/src/main/java/org/sonar/java/SonarComponents.java:615-624), but the two logClasspathWarnings() calls are placed after the early-return guard:
public void logUndefinedTypes() {
if (problemsToFilePaths.isEmpty()) {
return; // <-- returns before warnings are flushed
}
javaClasspath.logClasspathWarnings();
...
}
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.jdkHome warning used to be emitted immediately via LOG.warn(...) inside existingDirectoryOrLog (see the removed code in AbstractClasspath.java:140-148); it is now deferred into classpathWarnings and will not be logged at all if problemsToFilePaths is 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.:
public void logUndefinedTypes() {
javaClasspath.logClasspathWarnings();
if (!isAutoScan()) {
// In autoscan, test + main code are analyzed in the same batch ...
javaTestClasspath.logClasspathWarnings();
}
if (problemsToFilePaths.isEmpty()) {
return;
}
logUndefinedTypes(LOGGED_MAX_NUMBER_UNDEFINED_TYPES);
// clear the set so only new undefined types will be logged
problemsToFilePaths.clear();
}
- Apply fix
Check the box to apply the fix or reply for a change | Was this helpful? React with 👍 / 👎
alban-auzeill
force-pushed
the
alban/SONARJAVA-6518
branch
from
4ae2138 to
4baea4c
Compare
| public void logClasspathWarnings() { | ||
| for (String warning : classpathWarnings) { | ||
| LOG.warn(warning); | ||
| analysisWarnings.addUnique(warning); | ||
| } | ||
| classpathWarnings.clear(); | ||
| } |
There was a problem hiding this comment.
⚠️ Bug: Classpath warnings dropped when no undefined types collected
This PR converts fatal classpath errors (missing/invalid sonar.java.binaries, sonar.java.libraries, invalid sonar.java.jdkHome, and the new "Missing properties" notices) into warnings accumulated in AbstractClasspath.classpathWarnings and flushed by logClasspathWarnings().
However, the only production caller of logClasspathWarnings() is SonarComponents.logUndefinedTypes() (SonarComponents.java:619, 623), which begins with an early return:
public void logUndefinedTypes() {
if (problemsToFilePaths.isEmpty()) {
return;
}
javaClasspath.logClasspathWarnings();
...
}logUndefinedTypes() is invoked once at the end of analysis from JavaAstScanner.endOfAnalysis() (JavaAstScanner.java:143). When no undefined types were collected during the scan, the method returns before reaching logClasspathWarnings(), so every accumulated classpath warning is silently discarded.
This defeats the intent of the fix: a project that supplies an invalid sonar.java.binaries directory (or omits the property) but whose types all happen to resolve (e.g. from the JDK) will get no warning at all — neither in the logs nor in the SonarQube analysis warnings. Previously such cases failed loudly; now they can be silently ignored.
Suggested fix: flush the classpath warnings unconditionally, before the early return based on problemsToFilePaths. Since logClasspathWarnings() clears its own state, calling it unconditionally is safe.
Flush classpath warnings before the empty-problems early return so they are always emitted.:
public void logUndefinedTypes() {
javaClasspath.logClasspathWarnings();
if (!isAutoScan()) {
javaTestClasspath.logClasspathWarnings();
}
if (problemsToFilePaths.isEmpty()) {
return;
}
logUndefinedTypes(LOGGED_MAX_NUMBER_UNDEFINED_TYPES);
// clear the set so only new undefined types will be logged
problemsToFilePaths.clear();
}
- Apply fix
Check the box to apply the fix or reply for a change | Was this helpful? React with 👍 / 👎
alban-auzeill
force-pushed
the
alban/SONARJAVA-6518
branch
from
4baea4c to
8fd25a0
Compare
alban-auzeill
force-pushed
the
alban/SONARJAVA-6518
branch
from
8fd25a0 to
64f0718
Compare
Code Review
|
| Auto-apply | Compact | Unblock |
|
|
|
Was this helpful? React with 👍 / 👎 | Gitar
|
| return false; | ||
| } | ||
|
|
||
| protected boolean hasJavaFiles() { |
There was a problem hiding this comment.
This method is exactly the same as the hasJavaSources method line 182
| } | ||
| } | ||
|
|
||
| protected void validatePropertiesPresence(String binariesProperty, String librariesProperty) { |
There was a problem hiding this comment.
The two parameters are already class attributes, we should be able to drop them, right?
2 participants
Note
sonar-java classpath propertiesis used bellow to referencesonar.java.binaries,sonar.java.libraries,sonar.java.test.binaries,sonar.java.test.libraries.Technical decision part of this PR:
All the logic about properties "sonar.binaries" and "sonar.libraries" has been removed from the Java analyzer code.
An invalid element in one of the
sonar-java classpath properties:No files nor directories matching 'lib/*.jar'Invalid value for 'sonar.java.libraries', no files nor directories matching 'lib/*.jar'.Empty or missing sonar.java.libraries sonar.java.test.libraries:
Dependencies/libraries were not provided for analysis of SOURCE files. The 'sonar.java.libraries' property is empty. Verify your configuration, as you might end up with less precise results.Missing 'sonar.java.libraries' property. You might end up with less precise analysis results.Empty or missing sonar.java.binaries:
Your project contains .java files, please provide compiled classes with sonar.java.binaries property, or exclude them from the analysis with sonar.exclusions property.Missing 'sonar.java.binaries' property. You might end up with less precise analysis results.The logic in
logUndefinedTypes()has not changed. Warning related tosonar-java classpath propertiesare only logged/sent ifproblemsToFilePathsis not empty. So if there were no unresolved imports/types during the analysis, then there are no warnings.There is no longer a need to have a dedicated logic for SonarLint; the class ClasspathForMainForSonarLint has been removed.
Summary by Gitar
AbstractClasspath, introducing a unifiedinit()method andlogClasspathWarnings()to handle configuration validation.IllegalStateExceptionwith non-blocking analysis warnings whenbinariesorlibrariesproperties are missing or invalid.ClasspathForMainForSonarLintand streamlinedClasspathForMain/ClasspathForTestconfigurations.This will update automatically on new commits.