[rombirli]

Summary

• Adds a benchmark that parses the same sources with the ECJ frontend and with this SemSitter frontend, then diffs the SonarJava trees and the symbols on them.
• Writes absolute mismatch counts, proportions, and one readable entry per mismatch (benchmark/out/mismatches.txt, plus summary.json and mismatches.jsonl).
• Runs from this branch. The ECJ side is a separate checkout (--ecj, default ../sonar-java). Both checkouts need java-frontend already compiled.

Test plan

• python3 benchmark/compare.py --ecj ~/Documents/sonar-java --semsitter <v2 checkout> --corpus benchmark/corpus
• Sample corpus: 63 shared nodes, 0 missing nodes, 2 span mismatches (3.2%), 7 semantic mismatches of 35 compared nodes (20.0%).
• Point --corpus at a larger tree (check samples) once both frontends are compiled in the checkouts you want to score.

---

Summary by Gitar

• Benchmark tooling:
  • Added bucket_sample.py to sample and bucket AST and semantic mismatches per file
  • Added AstSemanticDump.java to dump SonarJava trees with symbols and types for ECJ and SemSitter
• Corpus fixtures:
  • Added Sample.java and Use.java sample sources under benchmark/corpus/bench
• Output handling:
  • Per-file diffs and ast-issue-buckets.csv / semantic-issue-buckets.csv outputs; out/ ignored via .gitignore

_{This will update automatically on new commits.}

[hashicorp-vault-sonar-prod]

SONARJAVA-7236

[github-actions]

⚖️ Ruling update ready for review: #6438

[github-actions]

⚖️ Ruling update ready for review: #6438

[gitar-bot]

CI failed: Two unrelated CI test failures in the Plugin QA and Sanity Test jobs: six integration-test assertion failures (JSP, modular-JDK classpath, tutorial telemetry, SonarLint parse-error) and a Surefire forked-JVM OutOfMemoryError in SanityTest. Neither is attributed to this PR's benchmark-only changes.

Overview

Two unique error templates were analyzed from 2 of the 60 logs in the run (commit 3fe5877). Both failures are in the test stage. The analyzers found no evidence linking either to this PR, which only adds standalone benchmark tooling and corpus files under benchmark/.

Failures

Plugin QA integration-test assertion failures (confidence: high)

• Type: test
• Affected jobs: 125342191667
• Related to change: no
• Root cause: Six independent ITs failed: JspTest.should_transpile_jsp (expected JSP-generated classes missing), two JavaClasspathTest modular-JDK cases (expected S4551 issues not found), JavaTutorialTest (8 main-source type errors where 0 are expected), JavaTutorialTest.test_as_batch_mode, and SonarLintIntegrationTest.parse_error_should_report_analysis_error (actual value not shown in the log). The log does not show a shared cause.
• Suggested fix: Inspect the Surefire reports and reproduce each of the six tests individually. Check JSP transpilation output, modular-JDK classpath setup and S4551 analysis, tutorial type-error counts, and the SonarLint parse-error result.

Sanity Test OutOfMemoryError in Surefire fork (confidence: high)

• Type: test
• Affected jobs: 125342191752
• Related to change: no
• Root cause: Maven compilation succeeded, then the forked test JVM ran out of heap (java.lang.OutOfMemoryError: Java heap space) during org.sonar.plugins.java.SanityTest, so 0 tests completed. The log does not identify the allocation responsible.
• Suggested fix: Inspect the Surefire dump files and SanityTest memory use to find the allocation. If the workload is expected, raise the forked test JVM heap or reduce its memory demand, then rerun.

Summary

• Change-related failures: None. Both findings are marked related_to_pr: no.
• Infrastructure/flaky failures: None identified. The OOM may be resource-related, but it was not classified as infrastructure or flaky.
• Recommended action: Treat both as pre-existing or unrelated test failures in the plugin ITs and SanityTest. Triage them separately from this benchmark-only PR. Start with the Surefire reports and the SanityTest heap dump.

Code Review ⚠️ Changes requested 2 closed / 9 findings

🟡 Medium risk · The benchmark harness deletes its output diffs tree before rewriting it, so choosing an existing --out path could erase user files.

Adds a benchmark comparing SonarJava AST and semantic output between the ECJ and SemSitter frontends, with per-file diffs, bucketed samples, and mismatch reports. Changes are requested because the dump misses or miscounts mismatches: sibling nodes with the same kind collide on path, binary-expression operands are never dumped, type-argument mismatches are bucketed as AST gaps, and whole-file parse failures inflate the global AST mismatch rate. Also open: all parsed trees are retained in memory until the end, array element type arguments are dropped from typeFull, and a relative --ecj path resolves the Maven classpath output against the wrong directory.

⚠️ Bug: Sibling nodes reached via scan(Tree) all get index [0], so paths collide

📄 benchmark/src/AstSemanticDump.java:161-175 📄 benchmark/compare.py:132-144

DumpVisitor.scan(Tree) always calls enter(tree, 0). BaseTreeVisitor reaches most children through separate scan(Tree) calls: visitMemberSelectExpression scans expression() and then identifier(), and visitVariable scans type() and then simpleName(). When two of those children have the same kind, they get the same path, e.g. .../MEMBER_SELECT[0]/IDENTIFIER[0] for both values and get in the sample corpus. ListTree children like class superInterfaces/permittedTypes hit the same problem, since they go through scan((Tree) listTree), and so do separate scan(List) calls under one parent. In compare.py, load() keys records by path (nodes[name][record["path"]] = record), so the later record overwrites the earlier one. Node counts come out too low (the reported 63 shared nodes included), the descendants of colliding subtrees get merged, and mismatches on the overwritten node are never seen. Fix: keep a per-parent Map<Tree.Kind,Integer> counter on the stack and assign the index in enter, no matter which scan overload reached the child.

Index children per parent instead of per scan call

private final java.util.ArrayDeque<Map<Tree.Kind, Integer>> counters = new java.util.ArrayDeque<>(List.of(new HashMap<>()));

@Override
protected void scan(@Nullable List<? extends Tree> trees) {
  if (trees != null) { for (Tree t : trees) { scan(t); } }
}

@Override
protected void scan(@Nullable Tree tree) {
  if (tree != null) {
    enter(tree, counters.peek().merge(tree.kind(), 1, Integer::sum) - 1);
  }
}
// in enter(): counters.push(new HashMap<>()) before accept, counters.pop() after

⚠️ Bug: Binary-expression operands are never emitted or compared

📄 benchmark/src/AstSemanticDump.java:176-190

BaseTreeVisitor.visitBinaryExpression calls tree.leftOperand().accept(this) and tree.rightOperand().accept(this) directly, not scan(...). Because of that, DumpVisitor.enter never runs for the operands: they get no record, so their symbols and types are never compared. Their descendants are also given paths directly under the binary node, as if the operands weren't there. In a + b, for example, neither identifier is dumped. Any AST or semantic difference on operands, which show up all over a real corpus, is invisible to the benchmark. Fix: override visitBinaryExpression in DumpVisitor to call scan(tree.leftOperand()); scan(tree.rightOperand());. Also check BaseTreeVisitor for any other direct accept(this) calls.

Route operands through scan so they are emitted

@Override
public void visitBinaryExpression(org.sonar.plugins.java.api.tree.BinaryExpressionTree tree) {
  scan(tree.leftOperand());
  scan(tree.rightOperand());
}

⚠️ Bug: type_arguments mismatches get bucketed as 'node only on SemSitter'

📄 benchmark/bucket_sample.py:191-205 📄 benchmark/bucket_sample.py:184-188 📄 benchmark/bucket_sample.py:345

bucket_sample.py samples the semantic categories {"symbol", "type", "type_arguments"} (line 345), and collect adds type_arguments items from semantic_kind. But issue_name only handles parse, symbol, type and span. Every other category falls through to ast_issue(category, node), and that returns f"node only on SemSitter: {kind}" for anything that isn't ast_only_ecj. So each type-argument mismatch shows up in semantic-issue-buckets.csv as an AST gap such as node only on SemSitter: IDENTIFIER, and fix_for then labels it semsitter. The semantic buckets give the wrong root cause and fix target for this whole category. Fix: add a type_arguments branch to issue_name (for example, one that compares symbolFull/typeFull), and send only the ast_only_* categories to ast_issue.

Handle type_arguments explicitly in issue_name

if category == "type_arguments":
    return "type arguments differ"
if category in {"ast_only_ecj", "ast_only_semsitter"}:
    node = item.get("ecj") or item.get("sem") or {}
    return ast_issue(category, node)
return category

⚠️ Performance: Every parsed tree is held in memory until all files are parsed

📄 benchmark/src/AstSemanticDump.java:95-109

In this delta, AstSemanticDump parses through JParserConfig.parse and stores every result.get() in the trees map (lines 95-104). Nothing is dumped until the whole corpus has been parsed. Each retained CompilationUnitTree keeps its full semantic model alive (with ECJ in FILE_BY_FILE mode, that includes per-file binding environments). Memory therefore grows with corpus size instead of staying at one file at a time, as it does in the rule tests and the scanner. The PR's next step is running against a larger corpus, and that run may hit OOM or slow down badly. Fix: run DumpVisitor (or write the error record) inside the callback, appending to a per-file buffer, so each tree can be released straight away. The output order is already fixed by sorting names.

💡 Bug: Relative --ecj path breaks the mvn output file location

📄 benchmark/compare.py:354-368 📄 benchmark/compare.py:43-57

args.semsitter is resolved before tool_classpath, but ecj is passed through unresolved. When --ecj ../sonar-java is given, output stays relative to Python's cwd. Maven, however, runs with cwd=checkout and resolves -Dmdep.outputFile relative to that directory (or to the module basedir), not Python's cwd. The classpath file ends up somewhere else, and output.read_text() then fails or reads a stale file. Fix: resolve ecj once (ecj = ecj.resolve()) before tool_classpath.

Resolve the ECJ checkout path up front

ecj = ecj.resolve()
corpus = args.corpus.resolve()

💡 Quality: Global AST counts include whole-file parse failures; per-file diffs don't

📄 benchmark/compare.py:391-405 📄 benchmark/compare.py:419-420 📄 benchmark/compare.py:309 📄 benchmark/compare.py:347-350 📄 benchmark/bucket_sample.py:224-225

load stores a file either as an error or as nodes, never both. So when one side fails to parse a file, compare() puts every node from the other side into only_ecj_all/only_sem_all and counts them in ast.mismatches, only_ecj and mismatch_proportion. mismatches.txt also lists COMPILATION_UNIT[0] as "AST only in ECJ/SemSitter". For the same file, file_diff sets whole_file and reports zero AST mismatches ("one side did not parse this file"), and bucket_sample.collect skips the file's AST rows. A single parse failure can therefore inflate the headline AST mismatch rate, while the per-file diffs and the buckets disagree with that number. Fix: skip whole-file parse failures in compare() the same way file_diff and collect do, and keep reporting them under parse_failures.

💡 Edge Case: Type arguments of array element types are dropped from typeFull

📄 benchmark/src/AstSemanticDump.java:350-364

withArguments erases fullyQualifiedName() and only adds arguments when type.isParameterized() is true. Array types are never parameterized (JType.isParameterized only returns true for Kind.NAMED with args), and their name is built from the element's FQN plus []. So List<String>[], and varargs such as List<T>... in method signatures, both come out as java.util.List[] in typeFull/symbolFull. A type-argument mismatch inside an array element type is never counted, which falls short of what this commit says it does ("Compare type arguments"). Fix: when the type is an ArrayType, recurse into elementType() and append [].

Recurse into the element type for arrays before checking parameterization

if (type.isArray()) {
  return withArguments(((Type.ArrayType) type).elementType(), depth) + "[]";
}
String name = erase(type.fullyQualifiedName());

✅ 2 closed

✅ Bug: Files with the same name in different dirs collapse into one InputFile

📄 benchmark/src/AstSemanticDump.java:81-92 📄 benchmark/src/AstSemanticDump.java:96-103
Each InputFile is built with new TestInputFileBuilder("bench", file.getParent().toFile(), file.toFile()), which makes the file's own directory its module base dir. Every input's relative path is therefore just the file name. DefaultInputFile equality and hash are based on that relative path, so a/Foo.java and b/Foo.java count as equal keys in the names HashMap. The second put overwrites the first, the parse callback maps both inputs to the same name, and trees.put keeps only one tree. One file disappears from both dumps without any warning. This is likely to happen on the larger corpora the test plan mentions (package-info.java, Test.java, ...). Fix: build every input with the corpus root as base dir, e.g. new TestInputFileBuilder("bench", root.toFile(), file.toFile()), or key names by identity or path.

✅ Edge Case: No analysis classpath passed; library types are unknown on both sides

📄 benchmark/compare.py:118-129 📄 benchmark/src/AstSemanticDump.java:69-79 📄 benchmark/src/AstSemanticDump.java:93
AstSemanticDump takes a --classpath option for the analysis classpath, but compare.py never passes it and offers no flag for it. Both frontends therefore run JParserConfig with an empty classpath. Any non-JDK type in the corpus resolves to unknown on both sides, and semantic_kind counts unknown == unknown as a match. On a real-project corpus, which the test plan calls for, the semantic mismatch rate will come out lower than it really is. Fix: add a --corpus-classpath argument to compare.py and forward it as --classpath in dump().

🤖 Prompt for agents

Code Review: Adds a benchmark comparing SonarJava AST and semantic output between the ECJ and SemSitter frontends, with per-file diffs, bucketed samples, and mismatch reports. Changes are requested because the dump misses or miscounts mismatches: sibling nodes with the same kind collide on path, binary-expression operands are never dumped, type-argument mismatches are bucketed as AST gaps, and whole-file parse failures inflate the global AST mismatch rate. Also open: all parsed trees are retained in memory until the end, array element type arguments are dropped from typeFull, and a relative --ecj path resolves the Maven classpath output against the wrong directory.

1. ⚠️ Bug: Sibling nodes reached via scan(Tree) all get index [0], so paths collide
   Files: benchmark/src/AstSemanticDump.java:161-175, benchmark/compare.py:132-144

   `DumpVisitor.scan(Tree)` always calls `enter(tree, 0)`. `BaseTreeVisitor` reaches most children through separate `scan(Tree)` calls: `visitMemberSelectExpression` scans `expression()` and then `identifier()`, and `visitVariable` scans `type()` and then `simpleName()`. When two of those children have the same kind, they get the same path, e.g. `.../MEMBER_SELECT[0]/IDENTIFIER[0]` for both `values` and `get` in the sample corpus. `ListTree` children like class `superInterfaces`/`permittedTypes` hit the same problem, since they go through `scan((Tree) listTree)`, and so do separate `scan(List)` calls under one parent. In `compare.py`, `load()` keys records by path (`nodes[name][record["path"]] = record`), so the later record overwrites the earlier one. Node counts come out too low (the reported 63 shared nodes included), the descendants of colliding subtrees get merged, and mismatches on the overwritten node are never seen. Fix: keep a per-parent `Map<Tree.Kind,Integer>` counter on the stack and assign the index in `enter`, no matter which `scan` overload reached the child.

   Fix (Index children per parent instead of per scan call):
   private final java.util.ArrayDeque<Map<Tree.Kind, Integer>> counters = new java.util.ArrayDeque<>(List.of(new HashMap<>()));
   
   @Override
   protected void scan(@Nullable List<? extends Tree> trees) {
     if (trees != null) { for (Tree t : trees) { scan(t); } }
   }
   
   @Override
   protected void scan(@Nullable Tree tree) {
     if (tree != null) {
       enter(tree, counters.peek().merge(tree.kind(), 1, Integer::sum) - 1);
     }
   }
   // in enter(): counters.push(new HashMap<>()) before accept, counters.pop() after

2. ⚠️ Bug: Binary-expression operands are never emitted or compared
   Files: benchmark/src/AstSemanticDump.java:176-190

   `BaseTreeVisitor.visitBinaryExpression` calls `tree.leftOperand().accept(this)` and `tree.rightOperand().accept(this)` directly, not `scan(...)`. Because of that, `DumpVisitor.enter` never runs for the operands: they get no record, so their symbols and types are never compared. Their descendants are also given paths directly under the binary node, as if the operands weren't there. In `a + b`, for example, neither identifier is dumped. Any AST or semantic difference on operands, which show up all over a real corpus, is invisible to the benchmark. Fix: override `visitBinaryExpression` in `DumpVisitor` to call `scan(tree.leftOperand()); scan(tree.rightOperand());`. Also check `BaseTreeVisitor` for any other direct `accept(this)` calls.

   Fix (Route operands through scan so they are emitted):
   @Override
   public void visitBinaryExpression(org.sonar.plugins.java.api.tree.BinaryExpressionTree tree) {
     scan(tree.leftOperand());
     scan(tree.rightOperand());
   }

3. 💡 Bug: Relative --ecj path breaks the mvn output file location
   Files: benchmark/compare.py:354-368, benchmark/compare.py:43-57

   `args.semsitter` is resolved before `tool_classpath`, but `ecj` is passed through unresolved. When `--ecj ../sonar-java` is given, `output` stays relative to Python's cwd. Maven, however, runs with `cwd=checkout` and resolves `-Dmdep.outputFile` relative to that directory (or to the module basedir), not Python's cwd. The classpath file ends up somewhere else, and `output.read_text()` then fails or reads a stale file. Fix: resolve `ecj` once (`ecj = ecj.resolve()`) before `tool_classpath`.

   Fix (Resolve the ECJ checkout path up front):
   ecj = ecj.resolve()
   corpus = args.corpus.resolve()

4. ⚠️ Bug: type_arguments mismatches get bucketed as 'node only on SemSitter'
   Files: benchmark/bucket_sample.py:191-205, benchmark/bucket_sample.py:184-188, benchmark/bucket_sample.py:345

   `bucket_sample.py` samples the semantic categories `{"symbol", "type", "type_arguments"}` (line 345), and `collect` adds `type_arguments` items from `semantic_kind`. But `issue_name` only handles `parse`, `symbol`, `type` and `span`. Every other category falls through to `ast_issue(category, node)`, and that returns `f"node only on SemSitter: {kind}"` for anything that isn't `ast_only_ecj`. So each type-argument mismatch shows up in `semantic-issue-buckets.csv` as an AST gap such as `node only on SemSitter: IDENTIFIER`, and `fix_for` then labels it `semsitter`. The semantic buckets give the wrong root cause and fix target for this whole category. Fix: add a `type_arguments` branch to `issue_name` (for example, one that compares `symbolFull`/`typeFull`), and send only the `ast_only_*` categories to `ast_issue`.

   Fix (Handle type_arguments explicitly in issue_name):
   if category == "type_arguments":
       return "type arguments differ"
   if category in {"ast_only_ecj", "ast_only_semsitter"}:
       node = item.get("ecj") or item.get("sem") or {}
       return ast_issue(category, node)
   return category

5. ⚠️ Performance: Every parsed tree is held in memory until all files are parsed
   Files: benchmark/src/AstSemanticDump.java:95-109

   In this delta, `AstSemanticDump` parses through `JParserConfig.parse` and stores every `result.get()` in the `trees` map (lines 95-104). Nothing is dumped until the whole corpus has been parsed. Each retained `CompilationUnitTree` keeps its full semantic model alive (with ECJ in FILE_BY_FILE mode, that includes per-file binding environments). Memory therefore grows with corpus size instead of staying at one file at a time, as it does in the rule tests and the scanner. The PR's next step is running against a larger corpus, and that run may hit OOM or slow down badly. Fix: run `DumpVisitor` (or write the error record) inside the callback, appending to a per-file buffer, so each tree can be released straight away. The output order is already fixed by sorting `names`.

6. 💡 Quality: Global AST counts include whole-file parse failures; per-file diffs don't
   Files: benchmark/compare.py:391-405, benchmark/compare.py:419-420, benchmark/compare.py:309, benchmark/compare.py:347-350, benchmark/bucket_sample.py:224-225

   `load` stores a file either as an error or as nodes, never both. So when one side fails to parse a file, `compare()` puts every node from the other side into `only_ecj_all`/`only_sem_all` and counts them in `ast.mismatches`, `only_ecj` and `mismatch_proportion`. `mismatches.txt` also lists `COMPILATION_UNIT[0]` as "AST only in ECJ/SemSitter". For the same file, `file_diff` sets `whole_file` and reports zero AST mismatches ("one side did not parse this file"), and `bucket_sample.collect` skips the file's AST rows. A single parse failure can therefore inflate the headline AST mismatch rate, while the per-file diffs and the buckets disagree with that number. Fix: skip whole-file parse failures in `compare()` the same way `file_diff` and `collect` do, and keep reporting them under `parse_failures`.

7. 💡 Edge Case: Type arguments of array element types are dropped from typeFull
   Files: benchmark/src/AstSemanticDump.java:350-364

   `withArguments` erases `fullyQualifiedName()` and only adds arguments when `type.isParameterized()` is true. Array types are never parameterized (`JType.isParameterized` only returns true for `Kind.NAMED` with args), and their name is built from the element's FQN plus `[]`. So `List<String>[]`, and varargs such as `List<T>...` in method signatures, both come out as `java.util.List[]` in `typeFull`/`symbolFull`. A type-argument mismatch inside an array element type is never counted, which falls short of what this commit says it does ("Compare type arguments"). Fix: when the type is an `ArrayType`, recurse into `elementType()` and append `[]`.

   Fix (Recurse into the element type for arrays before checking parameterization):
   if (type.isArray()) {
     return withArguments(((Type.ArrayType) type).elementType(), depth) + "[]";
   }
   String name = erase(type.fullyQualifiedName());

Verification Rules 1/1 met PRs 2 inspected

Functional validation No results

Rules 1 passed 2 not applicable

✅ Repository license compliance

Broader impact PRs 2 inspected

Cross-PR 2 PRs inspected 0 open impacts

PR 6432: SONARJAVA-7259 Introduce a compiler-neutral frontend backend with scoped resource management Inspected
PR 6416: SONARJAVA-7252 Decouple the Java frontend from Eclipse JDT core Inspected

Auto-approval Not enabled

Set up

_{For more verification details, comment gitar display:verbose.}

Tip

Comment Gitar fix CI or enable auto-apply: gitar auto-apply:on

Options

Auto-apply is off → Gitar will not commit updates to this branch.
Display: compact → Showing compact verification results.
Unblock → Override a blocking verdict and allow merging.

Comment with these commands to change the behavior for this request:

Auto-apply	Compact	Unblock
gitar auto-apply:on	gitar display:verbose	gitar unblock

_{Was this helpful? React with 👍 / 👎 | Gitar}

[github-actions]

⚖️ Ruling update ready for review: #6438