SONARJAVA-6870: Fix FP in S9342 when zip stream is wrapped in custom OutputStream

[romainbrenguier]

Summary

• Add test example for S9342 where ObjectMapper.writeValue writes to a custom OutputStream wrapping a ZipOutputStream
• Fix clearTrackedSymbolsUsedAsArguments to deeply scan non-identifier arguments (e.g., new CustomOutputStream(zos)) for tracked symbol usage, preventing false positives when the zip stream is nested inside a constructor call within a method argument

Test plan

• Existing EmptyArchiveEntryCheckTest passes (both with and without semantic)
• CI passes

🤖 Generated with Claude Code

[hashicorp-vault-sonar-prod]

SONARJAVA-6870

[nathsou]

LGTM, approved with a minor observation regarding cast/parenthesis unwrapping in TrackedSymbolVisitor.

[nathsou]

While unwrapping casts and parentheses here covers direct arguments of the method invocation, TrackedSymbolVisitor (visitNewClass and visitMethodInvocation) still checks arg.is(Tree.Kind.IDENTIFIER) directly without unwrapping casts or parentheses.

Since TrackedSymbolVisitor.visitIdentifier is an intentional no-op, tracked symbols nested inside a constructor/method call argument behind a cast or parentheses (e.g., new CustomOutputStream((OutputStream) zos)) won't be detected during the AST traversal.

Extracting a shared helper (e.g. extractIdentifier(ExpressionTree)) to unwrap casts and parentheses and reusing it in both clearTrackedSymbolsUsedAsArguments and TrackedSymbolVisitor would ensure nested cases are handled consistently.

[gitar-bot]

Code Review ✅ Approved 3 resolved / 3 findings

Fixes false positives in S9342 when ObjectMapper.writeValue writes to a custom OutputStream wrapping a ZipOutputStream. Deep argument scanning now correctly detects tracked symbols behind casts and parentheses, and shared unwrapping logic eliminates duplicated visitor code.

✅ 3 resolved

✅ Edge Case: Deep argument scan misses symbols behind casts or parentheses

📄 java-checks/src/main/java/org/sonar/java/checks/EmptyArchiveEntryCheck.java:167-177 📄 java-checks/src/main/java/org/sonar/java/checks/EmptyArchiveEntryCheck.java:231-235
The new else branch delegates to TrackedSymbolVisitor, but that visitor's visitIdentifier is a deliberate no-op, so a tracked symbol is only detected when it is a direct argument of a nested method invocation or new expression. An argument such as objectMapper.writeValue((OutputStream) zos, new Object()), objectMapper.writeValue((zos), obj) or a conditional flag ? zos : other reaches arg.accept(visitor), traverses down to the zos identifier, hits the no-op visitIdentifier, and leaves zos in pendingEntries — so the following zos.closeEntry() still raises the same false positive this PR is meant to fix. Handling identifiers found while scanning an argument subtree (or unwrapping casts/parentheses via ExpressionUtils.skipParentheses/cast before the identifier test) would close the gap; a sample with a cast argument would pin the behaviour.

✅ Quality: Duplicated visitor/removal logic and per-argument visitor allocation

📄 java-checks/src/main/java/org/sonar/java/checks/EmptyArchiveEntryCheck.java:109-118 📄 java-checks/src/main/java/org/sonar/java/checks/EmptyArchiveEntryCheck.java:163-177
Lines 171-175 repeat verbatim the body of scanForTrackedSymbolUsage (lines 113-117), and a fresh TrackedSymbolVisitor is allocated for every non-identifier argument even though one instance could scan all of them. Extracting a single removeUsedSymbols(Tree, Map) helper used by both call sites keeps the two scanning paths from drifting apart and avoids the redundant allocations.

✅ Edge Case: Cast/parenthesized write receiver still yields FP in S9342

📄 java-checks/src/main/java/org/sonar/java/checks/EmptyArchiveEntryCheck.java:196-206 📄 java-checks/src/main/java/org/sonar/java/checks/EmptyArchiveEntryCheck.java:217-221 📄 java-checks/src/main/java/org/sonar/java/checks/EmptyArchiveEntryCheck.java:166-176 📄 java-checks/src/main/java/org/sonar/java/checks/EmptyArchiveEntryCheck.java:182-193
This commit teaches argument scanning to unwrap casts and parentheses via extractIdentifierSymbol, but the receiver path (getReceiverSymbol, used both from handleMethodInvocation and from TrackedSymbolVisitor.visitMethodInvocation) still requires a bare IDENTIFIER, so ((OutputStream) zos).write("data".getBytes()); or (zos).write(...) returns null; clearTrackedSymbolsUsedAsArguments then only inspects the arguments, zos stays in pendingEntries, and the following zos.closeEntry() is flagged — a false positive of exactly the family this PR fixes (verified by tracing both the top-level statement path and the nested-statement visitor path, where visitIdentifier is a no-op so the cast's inner identifier is never credited). Reuse the new helper in getReceiverSymbol so casted/parenthesized receivers are resolved too, and add a sample case such as ((OutputStream) zos).write("data".getBytes()); followed by a compliant zos.closeEntry();.

Implementation Status ◻️ 2 of 3 objectives covered

◻️ SONARJAVA-6870 - 2 of 3 objectives covered

This PR covers investigating the FP feedback, fixing false positives in S9342 when zip streams are wrapped in custom OutputStreams or use casts and parentheses, but does not add an example in rspec.

Other objectives on this issue, possibly covered elsewhere:

• ◻️ Add example in rspec with empty file

✅ 2 covered here

• ✅ Investigate FP feedback for S9342
• ✅ Fix FP in S9342 when zip stream is wrapped in custom OutputStream

Options

Auto-apply is off → Gitar will not commit updates to this branch.
Display: compact → Showing less information.

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

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

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

[sonarqube-next]

Quality Gate passed

Issues
0 New issues
0 Fixed issues
0 Accepted issues

Measures
0 Security Hotspots
0 Dependency risks
97.7% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube