Repository navigation
Conversation
Main sources still compile against empty trees and symbols. Tests are expected to fail until a semsitter parser fills them in.
Run one SemSitter scan (with the classpath) and its semantic dump over the batch, then parse each file with tree-sitter-java and attach symbols, resolved occurrences, graph call targets and graph facts by byte span.
…uery it Rust: a serve mode answering analyze, cst, parse and quit requests, and cst and parse commands printing the CST of files. Java: SemsitterBridge client.
❌ ErrorsYour PR has failed checks. Please review the issues below and take necessary action before merging. 🚦 9 Pipeline jobs failed
Useful? React with 👍 / 👎 This comment will be updated automatically if new data arrives.🔗 Commit SHA: 1fd63f4 | Docs | View more details | Give us feedback! |
|
/pvf |
|
⚖️ Ruling update ready for review: #6417 |
Ruling Diff SummaryDetected changes in 143 rule files: 5813 issues removed, 6054 issues added. S104 (
|
|
📊 Performance Validation complete |
|
/pvf |
![gitar-bot[bot]](https://avatars.githubusercontent.com/in/827041?s=60&v=4){kind=small}
| String bridgeName = bridgeName(platform); | ||
| Path bridgePath = targetDir.resolve(bridgeName); | ||
| if (!Files.exists(bridgePath)) { | ||
| String archiveName = archiveName(platform); | ||
| String resourcePath = "/bridge/" + archiveName; | ||
| try (InputStream stream = BridgeExtractor.class.getResourceAsStream(resourcePath)) { | ||
| if (stream == null) { | ||
| throw new IllegalStateException("Bridge resource not found"); | ||
| } | ||
| extractArchive(targetDir, bridgePath, stream, resourcePath, bridgeName); | ||
| } | ||
| } | ||
| return bridgePath; | ||
| } |
There was a problem hiding this comment.
⚠️ Security: Bridge runs from a predictable shared tmp path and is trusted if present
JParser.bridgePath() always uses ${java.io.tmpdir}/sonar-java/semsitter-bridge/bridge, and BridgeExtractor.extract skips extraction and returns that path whenever any file already exists there (if (!Files.exists(bridgePath))). On Linux/macOS, java.io.tmpdir is usually the world-writable /tmp. Another local user can create /tmp/sonar-java/semsitter-bridge/bridge first, and then every analysis by any other user executes that binary under the victim's account (CWE-377/CWE-427). The extractor never checks who owns the file, never verifies its hash against the embedded archive, and the directory is not private.
Fix: extract into a per-user, private, versioned directory (for example under the scanner working dir or user.home, created with 0700). Alternatively, verify a checksum of the existing file against the packaged resource before running it.
Was this helpful? React with 👍 / 👎
| } | ||
| String bridgeName = bridgeName(platform); | ||
| Path bridgePath = targetDir.resolve(bridgeName); | ||
| if (!Files.exists(bridgePath)) { |
There was a problem hiding this comment.
⚠️ Bug: Concurrent extraction shares one staging file; chmod runs after publish
Every JVM writes to the same staging file bridge.ext with REPLACE_EXISTING, so two concurrent analyses can interleave. For example, JVM A moves the file that JVM B is still writing into bridge, then runs a truncated binary. ATOMIC_MOVE onto an existing target also fails on Windows. Separately, Files.copy creates the file without the execute bit, and permissions are only set after the atomic move. In that window another process sees bridge exists and gets Permission denied when it runs it. If the JVM dies between the move and setPermissions, the non-executable file stays there forever, because extract never re-extracts an existing path.
Fix: use a unique temp file per process (Files.createTempFile(target, ...)), set its permissions before the move, and treat an already-existing target after the move as success.
Was this helpful? React with 👍 / 👎
| final Result parseFile(InputFile inputFile) { | ||
| PerformanceMeasure.Duration parseDuration = PerformanceMeasure.start("JParser"); | ||
| try { | ||
| if (bridge == null) { | ||
| return new Result(JParser.parse(javaVersion.effectiveJavaVersionAsString(), inputFile.filename(), inputFile.contents())); | ||
| } | ||
| action.accept(inputFile, result); | ||
| JsonObject document = bridge.cst(inputFile.path().toAbsolutePath().toString()); | ||
| JSema sema = new JSema(); | ||
| sema.ingestDocument(document); | ||
| sema.ingestTypes(bridge.types(sema.referencedTypeNames())); | ||
| return new Result(SemsitterTranslator.translate(sema, document)); |
There was a problem hiding this comment.
⚠️ Bug: Batch parsing reads files from disk and ignores InputFile contents/charset
The old JParserConfig passed inputFile.contents() and inputFile.charset() to the parser. Now parseFile sends only the path (bridge.cst(path)), and the bridge reads the bytes from disk itself (std::fs::read / String::from_utf8_lossy). As a result, sonar.sourceEncoding is ignored: an ISO-8859-1 or windows-1252 source gets U+FFFD in literal and comment token text, and its UTF-16 columns no longer match what the scanner reports. In SonarLint, where InputFile.contents() is the unsaved editor buffer, the analysis runs on stale on-disk content, so issues land on the wrong lines.
Fix: send the decoded contents with the request (or at least the charset), and have the bridge use them.
Was this helpful? React with 👍 / 👎
| firstTokenAfter(e.getExpression(), TerminalToken.TokenNameDOT), | ||
| i | ||
| ); | ||
| public static JavaTree.CompilationUnitTreeImpl parse(String version, String unitName, String source) { |
There was a problem hiding this comment.
⚠️ Cross-Repo: sonar-java-symbolic-execution test utils no longer compile against JParser
Fix here and in SonarSource/sonar-java-symbolic-execution
Breaks in: SonarSource/sonar-java-symbolic-execution - java-symbolic-execution/java-symbolic-execution-plugin/src/test/java/org/sonar/java/se/utils/JParserTestUtils.java - JParserTestUtils.parse
This PR removes the ECJ-based JParser.parse(ASTParser, String, String, String) and JParserConfig#astParser(). JParser.parse now takes (version, unitName, source) and starts a SemSitter bridge itself. In SonarSource/sonar-java-symbolic-execution, java-symbolic-execution/java-symbolic-execution-plugin/src/test/java/org/sonar/java/se/utils/JParserTestUtils.java (JParserTestUtils.parse) builds an ASTParser via JParserConfig...astParser() and passes it to the old overload. As soon as that repo builds against this java-frontend, it stops compiling, and with it every symbolic-execution test that uses the helper. The removal is deliberate (the PR drops ECJ), so the consumer has to move. The tests will also need a runnable bridge, either the packaged one or SEMSITTER_BRIDGE.
Here: Optionally keep a deprecated shim with the old arity (minus the ASTParser) so callers have a migration path, and record the API removal in the changelog/release notes so the symbolic-execution repo can be updated in step.
There: In JParserTestUtils.parse, drop the ASTParser/JParserConfig.astParser() construction and call the new three-argument JParser.parse; make sure the test environment can find the bridge (packaged resource or SEMSITTER_BRIDGE).
In SonarSource/sonar-java-symbolic-execution - java-symbolic-execution/java-symbolic-execution-plugin/src/test/java/org/sonar/java/se/utils/JParserTestUtils.java:
public static CompilationUnitTree parse(String unitName, String source, List<File> classpath) {
return JParser.parse(JParserConfig.MAXIMUM_SUPPORTED_JAVA_VERSION.effectiveJavaVersionAsString(), unitName, source);
}
Was this helpful? React with 👍 / 👎
|
|
||
| private static void ascribe(Tree tree, Type type) { | ||
| if (tree instanceof AbstractTypedTree typed && type != null) { | ||
| typed.type = type; |
There was a problem hiding this comment.
⚠️ Cross-PR: Translator's direct field assignments break against #6416's Supplier fields
Breaks when this PR and #6416 both merge into master, in either order. The translator then fails to compile against #6416's tree fields.
How
SemsitterTranslator.java:1657assignstyped.type = type. Lines 1291, 1417, 1419, 1421, 948, 1153 and 1414 assigntypeSymbol,methodSymbol,variableSymbolandsymboldirectly as eager values.- #6416 changes
AbstractTypedTree.java:26sotypeisSupplier<Type>. It also makesVariableTreeImpl.variableSymbol,ClassTreeImpl.typeSymbolandMethodTreeImpl.methodSymbolSuppliers, andIdentifierTreeImpl/ImportTreeImplnow usesymbolSupplier. - A
TypeorSymbolvalue cannot be assigned to aSupplier<...>field, soSemsitterTranslatorstops compiling. #6416'sSemanticValuesalso drops the null-field fallback that this PR'ssymbolType()/symbol()rely on.
Fix
- This PR: If #6416 lands first, rebase the translator onto its Supplier fields (e.g.
typed.type = () -> type), or route the assignments through setters so the field shape does not matter. - #6416: Agree on one tree-binding shape with this PR. Either keep eager fields or provide setters, so the SemSitter translator does not have to know the storage type.
Compared b0aba6f with #6416 at 12688a4
Was this helpful? React with 👍 / 👎
| private final Map<String, JType> rawTypes = new HashMap<>(); | ||
| final Map<String, Tree> declarations = new HashMap<>(); | ||
| final Map<String, List<IdentifierTree>> usages = new HashMap<>(); | ||
| private final Set<JProblem> undefinedTypes = new HashSet<>(); |
There was a problem hiding this comment.
💡 Cross-PR: Undefined types from SemSitter vanish if scanner reads ast.undefinedTypes() · #6416
Breaks when both PRs merge and JavaAstScanner uses #6416's ast.undefinedTypes() while the semantic model is this PR's JSema.
How
JSema.java:232records missing types inJSema.undefinedTypes, and the currentJavaAstScanner.java:169readsast.sema.undefinedTypes().- #6416 changes the scanner to read
ast.undefinedTypes()fromCompilationUnitTreeImpl(JavaTree.java). On its side, JParser fills that set from ECJ errors. - Neither
SemsitterTranslator.translatenorJParserConfig.parseFilecopies theJSemaset onto the tree, so the scanner always gets an empty set. Undefined-type logging and telemetry silently drop to zero.
Fix
- This PR: In
SemsitterTranslator.translate(or afteringestTypesinparseFile), copysema.undefinedTypes()onto the compilation unit if it adopts #6416's field. - #6416: Nothing needed beyond keeping
CompilationUnitTreeImpl.undefinedTypeswritable by other frontends.
Compared b0aba6f with #6416 at 12688a4
Was this helpful? React with 👍 / 👎
|
⚖️ Ruling update ready for review: #6417 |
|
📊 Performance Validation complete |
A new expression whose type reference is not a constructor used to leave a class or enum-field symbol on the identifier, and rules that cast it aborted the analysis.
…range Leaf primitive types and braces nested under a block were turned into tokens at the wrong place, so highlighting and issue ranges fell outside the line.
scoped_type_identifier was not a type kind, so a generic type like Map.Entry<K, V> reached type() with a null child and the translator threw.
Top-level methods were emitted as compilation-unit members, so visitors that look up the enclosing class crashed on Java 25 compact files.
The scanner drops issues whose location has no text range, which discarded every file-level finding such as S105.
|
/pvf |
|
⚖️ Ruling update ready for review: #6417 |
…error A syntax error used to discard the file and fail the analysis. The recovered tree is now checked, and a parse error is reported only when nothing is left to analyze, so the other files keep going.
|
/pvf |
|
⚖️ Ruling update ready for review: #6417 |
|
📊 Performance Validation complete Projects with unexpected PVF results:
|
A public class loaded from a jar had no modifiers, so isPublic() was false. S3242 then sorted an empty inheritance chain and aborted activiti.
|
/pvf |
|
⚖️ Ruling update ready for review: #6417 |
|
📊 Performance Validation complete Projects with unexpected PVF results:
|
lib/modules was skipped, so JDK symbols came from a table with no annotations. The classpath JDK now supplies the class, and @deprecated keeps its values.
|
/pvf |
|
⚖️ Ruling update ready for review: #6417 |
|
📊 Performance Validation complete Projects with unexpected PVF results:
|
|
/pvf |
|
⚖️ Ruling update ready for review: #6417 |
|
📊 Performance Validation complete Projects with unexpected PVF results:
|
CI failed: Two change-related integration failures in the semsitter ECJ replacement: Plugin QA shows JSP class, JavaTutorial type-error, and SonarLint parse-error mismatches, and Sanity Test runs out of Java heap while decoding large bridge responses.OverviewTwo unique error templates were analyzed from 150 logs at commit 1fd63f4. Both failures are attributed to the semsitter parser replacement and are classified as change-related. No flaky or infrastructure-only failures were identified in these templates. FailuresPlugin QA integration test mismatches (confidence: high)
Sanity Test OutOfMemoryError while decoding bridge responses (confidence: high)
Summary
Code Review
|
| Auto-apply | Compact | Unblock |
|
|
|
Was this helpful? React with 👍 / 👎 | Gitar
|
/pvf |
|
⚖️ Ruling update ready for review: #6417 |
|
📊 Performance Validation complete Projects with unexpected PVF results:
|
Part of SONARJAVA-7234
Draft, do not merge. The plugin now extracts its packaged SemSitter bridge automatically. This build archives the bridge for the build host; prebuilt archives for the other supported operating systems are still pending.
Summary
jdt-package(ECJ) dependency.semsitter-bridge, a Cargo crate pinned to semsitterd606e04b766dc4f506c20c3c37f26d1511b5f065. One scan plus the semantic dump produces a tree-sitter Java CST with symbols, resolved uses and graph facts attached by byte span.analyze,cst,types,parse,quit) and the same operations from the CLI.SemsitterBridgeis the Java client.JParser/JParserConfig. One bridge process scans the batch, then each file is translated. JDK return type parameters (List.get→ the type argument) are kept.sonar.java.semsitter.bridgeorSEMSITTER_BRIDGE.Still to come on this PR
Test plan
java-frontendmain sources compileA.java/B.javatranslate: fields,this,List.get→String, andBcallingA.runresolve/pvfonly after that build is greenSummary by Gitar
This will update automatically on new commits.