[romainbrenguier]

Summary

Fixes a false positive of S8989 (methods with @Transactional should specify rollback behavior for checked exceptions): when all checked exceptions are thrown before any save is made, no write can be rolled back, so the rollback configuration is irrelevant.

The check now uses the method's control flow graph:

• Write operations: method invocations / constructor calls whose name (or a lambda argument's call) has a transactional prefix (save, delete, update, persist, merge, flush, insert).
• Checked throw points: throw statements of a checked (or unknown) type, and invocations/constructors that declare checked (or unknown) exceptions, or whose symbol is unknown.
• The issue is skipped only if the method has at least one write and at least one throw point, and no throw point is reachable from a write (including a write that itself throws, and exceptional edges to catch blocks).

Methods without a body, without detected writes, or without any detected throw point keep the previous behavior.

Tests

• Added compliant/noncompliant cases to TransactionalMethodCheckedExceptionCheckSample.java (throws before writes, writes before throws, loops, lambdas, try/catch rethrow, unchecked throws).
• Added unknown type / unknown method cases to the non-compiling sample.

🤖 Generated with Claude Code

Agent workflow

Tool link: https://github.com/SonarSource/languages-experimental-tooling/tree/romain/my-tickets/personal/romain-brenguier
PR created using uv run create_with_claude.py SONARJAVA-7243.txt

[hashicorp-vault-sonar-prod]

SONARJAVA-7243

[gitar-bot]

Code Review ⚠️ Changes requested 0 closed / 2 findings

🟡 Medium risk · The S8989 control-flow analysis may suppress rollback warnings incorrectly if it misidentifies write or checked-exception paths.

Skips S8989 when every checked exception is thrown before any detected write, removing a known false positive for @Transactional methods. Changes requested: the try-with-resources implicit close() is not treated as a throw point, so a write followed by a checked close() failure can be wrongly suppressed, and writes not matching the name prefixes (e.g. executeUpdate, batchUpdate) before a checked throw can cause false negatives.

⚠️ Bug: Implicit close() in try-with-resources is not counted as a throw point

📄 java-checks/src/main/java/org/sonar/java/checks/spring/TransactionalMethodCheckedExceptionCheck.java:316-330 📄 java-checks/src/main/java/org/sonar/java/checks/spring/TransactionalMethodCheckedExceptionCheck.java:349-363

The CFG does not model the implicit close() calls of try-with-resources. CFG.java builds only the resource list and says so in a comment: "FIXME only path with no failure constructed for now, (not taking try with resources into consideration)". So hasReachableThrowPoint never sees the close() that runs after the body. Example: try (BufferedReader r = Files.newBufferedReader(p)) { repository.save(o); } in a method declaring throws IOException. newBufferedReader is a checked throw point before the write, and nothing checked is found after save, so the issue is now skipped. But r.close() can throw IOException after the save, and Spring will then commit the write instead of rolling it back. Fix: if a write sits inside a try-with-resources whose resource types have a close() that declares checked exceptions, treat it as having a reachable throw point, or skip the optimisation when the method contains try-with-resources. Add a Noncompliant sample for this case.

💡 Edge Case: Writes not matching the name prefixes before a throw now cause false negatives

📄 java-checks/src/main/java/org/sonar/java/checks/spring/TransactionalMethodCheckedExceptionCheck.java:323-336 📄 java-checks/src/main/java/org/sonar/java/checks/spring/TransactionalMethodCheckedExceptionCheck.java:370-377

The issue is skipped as soon as every detected write comes after every checked throw point. Writes are detected only by method-name prefix (save, delete, update, ...). Any write that doesn't match a prefix and runs before a checked throw is invisible, and the issue is wrongly suppressed. Example: em.createQuery("UPDATE ...").executeUpdate(); Validator.validate(o); /* throws IOException */ repository.save(o);. Here executeUpdate, jdbcTemplate.batchUpdate and execute don't match a prefix, save does, and validate comes before save, so the check returns true. The executeUpdate write is committed when IOException is thrown. Before this PR the method was always reported. Fix: only suppress when no method invocation of unknown write status (e.g. any non-getter call on a repository, EntityManager or JdbcTemplate receiver) runs before the last throw point, or widen the write detection used for this suppression.

🤖 Prompt for agents

Code Review: Skips S8989 when every checked exception is thrown before any detected write, removing a known false positive for `@Transactional` methods. Changes requested: the try-with-resources implicit `close()` is not treated as a throw point, so a write followed by a checked `close()` failure can be wrongly suppressed, and writes not matching the name prefixes (e.g. `executeUpdate`, `batchUpdate`) before a checked throw can cause false negatives.

1. ⚠️ Bug: Implicit close() in try-with-resources is not counted as a throw point
   Files: java-checks/src/main/java/org/sonar/java/checks/spring/TransactionalMethodCheckedExceptionCheck.java:316-330, java-checks/src/main/java/org/sonar/java/checks/spring/TransactionalMethodCheckedExceptionCheck.java:349-363

   The CFG does not model the implicit `close()` calls of try-with-resources. `CFG.java` builds only the resource list and says so in a comment: "FIXME only path with no failure constructed for now, (not taking try with resources into consideration)". So `hasReachableThrowPoint` never sees the `close()` that runs after the body. Example: `try (BufferedReader r = Files.newBufferedReader(p)) { repository.save(o); }` in a method declaring `throws IOException`. `newBufferedReader` is a checked throw point before the write, and nothing checked is found after `save`, so the issue is now skipped. But `r.close()` can throw `IOException` after the save, and Spring will then commit the write instead of rolling it back. Fix: if a write sits inside a try-with-resources whose resource types have a `close()` that declares checked exceptions, treat it as having a reachable throw point, or skip the optimisation when the method contains try-with-resources. Add a Noncompliant sample for this case.

2. 💡 Edge Case: Writes not matching the name prefixes before a throw now cause false negatives
   Files: java-checks/src/main/java/org/sonar/java/checks/spring/TransactionalMethodCheckedExceptionCheck.java:323-336, java-checks/src/main/java/org/sonar/java/checks/spring/TransactionalMethodCheckedExceptionCheck.java:370-377

   The issue is skipped as soon as every *detected* write comes after every checked throw point. Writes are detected only by method-name prefix (`save`, `delete`, `update`, ...). Any write that doesn't match a prefix and runs before a checked throw is invisible, and the issue is wrongly suppressed. Example: `em.createQuery("UPDATE ...").executeUpdate(); Validator.validate(o); /* throws IOException */ repository.save(o);`. Here `executeUpdate`, `jdbcTemplate.batchUpdate` and `execute` don't match a prefix, `save` does, and `validate` comes before `save`, so the check returns true. The `executeUpdate` write is committed when `IOException` is thrown. Before this PR the method was always reported. Fix: only suppress when no method invocation of unknown write status (e.g. any non-getter call on a repository, EntityManager or JdbcTemplate receiver) runs before the last throw point, or widen the write detection used for this suppression.

Verification Rules 2/2 met Repos 1 inspected

Functional validation No results

Rules 2 passed 1 not applicable

✅ Internal artifact registries
✅ Repository license compliance

Broader impact Repos 1 inspected

Cross-repo 1 repository inspected 0 open impacts

SonarSource/sonarlint-vscode Inspected

Auto-approval Not enabled

Set up

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

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}

[gitar-bot]

⚠️ Bug: Implicit close() in try-with-resources is not counted as a throw point

The CFG does not model the implicit close() calls of try-with-resources. CFG.java builds only the resource list and says so in a comment: "FIXME only path with no failure constructed for now, (not taking try with resources into consideration)". So hasReachableThrowPoint never sees the close() that runs after the body. Example: try (BufferedReader r = Files.newBufferedReader(p)) { repository.save(o); } in a method declaring throws IOException. newBufferedReader is a checked throw point before the write, and nothing checked is found after save, so the issue is now skipped. But r.close() can throw IOException after the save, and Spring will then commit the write instead of rolling it back. Fix: if a write sits inside a try-with-resources whose resource types have a close() that declares checked exceptions, treat it as having a reachable throw point, or skip the optimisation when the method contains try-with-resources. Add a Noncompliant sample for this case.

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

[gitar-bot]

💡 Edge Case: Writes not matching the name prefixes before a throw now cause false negatives

The issue is skipped as soon as every detected write comes after every checked throw point. Writes are detected only by method-name prefix (save, delete, update, ...). Any write that doesn't match a prefix and runs before a checked throw is invisible, and the issue is wrongly suppressed. Example: em.createQuery("UPDATE ...").executeUpdate(); Validator.validate(o); /* throws IOException */ repository.save(o);. Here executeUpdate, jdbcTemplate.batchUpdate and execute don't match a prefix, save does, and validate comes before save, so the check returns true. The executeUpdate write is committed when IOException is thrown. Before this PR the method was always reported. Fix: only suppress when no method invocation of unknown write status (e.g. any non-getter call on a repository, EntityManager or JdbcTemplate receiver) runs before the last throw point, or widen the write detection used for this suppression.

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

[sonarqube-next]

Quality Gate passed

Issues
0 New issues
0 Fixed issues
0 Accepted issues

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

See analysis details on SonarQube