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. 🚦 8 Pipeline jobs failed
Useful? React with 👍 / 👎 This comment will be updated automatically if new data arrives.🔗 Commit SHA: 40f655e | 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.
…vered binding does
…s compiler warnings
…m their initializer and declared type
…s of a class of the file
…on, field, return and creation types
…ssigned as effectively final
…ookup into smaller methods
|
/pvf |
|
⚖️ Ruling update ready for review: #6417 |
|
📊 Performance Validation complete Projects with unexpected PVF results:
|
rombirli
changed the title
Methods declared in a local or anonymous class were owned by a field symbol, so owner() could not be cast to a TypeSymbol and enclosingClass() returned null. This crashed S2061 on guava and S5961 on wicket, which aborts the whole analysis under fail-fast.
Only a parameter that was itself a type variable was substituted, so a parameter such as ContextConsumer<A> stayed raw and left a lambda with no type arguments. This crashed S2699 on activiti, initializr and netflix-conductor. The receiver is now also matched against the declaring type through its supertypes, so a method inherited from a generic base class resolves its arguments.
…d parameters Arguments were typed from the declared parameters, so a lambda passed to a generic method kept a raw target type when the receiver supplied the type arguments.
Picks up the generic parameter types emitted for classpath members.
|
/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.
CI failed: Two PR-related failures: the Custom Rules and License Check job failed because the license plugin rejects org.tukaani:xz:1.10 (0BSD license), and the SonarQube 6.5 ruling test reported 3,475 issue differences versus expected results after the ECJ-to-semsitter parser swap. JaCoCo instrumentation warnings were non-fatal.OverviewTwo distinct failure patterns across 2 unique error templates (16 logs analyzed of 123 total; other templates not included in these findings). Both failures are linked to this PR's changes, and the analysis points to a dependency/license policy issue and a ruling-regression that needs investigation. FailuresLicense check rejects XZ for Java dependency (confidence: high)
SonarQube 6.5 ruling differences (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.