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

[gitar-bot]

Code Review ✅ Approved 2 resolved / 2 findings

Fixes false positives in S9342 when a ZipOutputStream is wrapped in a custom OutputStream by deeply scanning non-identifier arguments for tracked symbols, including those behind casts or parentheses. The fix prevents the detector from missing zip stream usage nested inside constructor calls within method arguments.

✅ 2 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.

Implementation Status ◻️ 2 of 3 objectives covered

◻️ SONARJAVA-6870 - 2 of 3 objectives covered

This PR fixes the false positive in S9342 when a zip stream is wrapped in a custom OutputStream or passed via casts and parentheses.

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
100.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube