SONARJAVA-6660 Fix S2229 FP for self-invocations inside TransactionTemplate callbacks

[asya-vorobeva]

Self-invocations inside TransactionTemplate.execute/executeWithoutResult callbacks are suppressed because the template guarantees an active transaction at the call site. Resolving the template's configured propagation statically would require data-flow or symbolic-execution techniques, so all self-invocations within such callbacks are treated as compliant.

[hashicorp-vault-sonar-prod]

SONARJAVA-6660

[gitar-bot]

Code Review ✅ Approved 2 resolved / 2 findings

Fixes false positives for rule S2229 by suppressing self-invocations inside TransactionTemplate.execute and executeWithoutResult callbacks, where the template guarantees an active transaction. The fix treats all such invocations as compliant since resolving the template's propagation statically would require advanced data-flow analysis.

✅ 2 resolved

✅ Edge Case: Matcher misses TransactionOperations/subclass receivers, FP persists

📄 java-checks/src/main/java/org/sonar/java/checks/spring/SpringIncompatibleTransactionalCheck.java:63-67
TRANSACTION_TEMPLATE_EXECUTE uses .ofTypes(...TransactionTemplate), and MethodMatchersBuilder.isSearchedMethod tests the call-site type with exact equality (type.is(name), MethodMatchersBuilder.java:102/236/251). So a field declared with the interface type — TransactionOperations ops; ops.executeWithoutResult(status -> retry(id)); — or with a custom class MyTemplate extends TransactionTemplate receiver does not match, and the S2229 false positive this PR targets still fires for those shapes (both execute and executeWithoutResult are declared on TransactionOperations). Match on the interface with ofSubTypes so all template flavours are covered.

✅ Bug: Suppression covers receiver and eager arguments, not just callback

📄 java-checks/src/main/java/org/sonar/java/checks/spring/SpringIncompatibleTransactionalCheck.java:105-119
The flag is set before super.visitMethodInvocation, which scans methodSelect() then arguments() (BaseTreeVisitor.java:259), so every self-invocation anywhere in the execute(...) subtree is suppressed — including expressions evaluated before the transaction starts, e.g. transactionTemplate.execute(buildCallback(retryNew(id))) or templateFor(retry(id)).execute(status -> ...). Those calls run outside the template's transaction, so the incompatible-propagation issue is silently lost (false negative). Restrict the flag to the lambda / anonymous-class arguments that actually form the callback body.

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

[lijun-chen-sonarsource]

LGTM!

I've seen your counterarguments to Gitar's comments. I think they make sense.
Do you think it'd make sense to add test cases that make explicit that these two cases are FN and FP that we intentionally accept?

[asya-vorobeva]

LGTM!

I've seen your counterarguments to Gitar's comments. I think they make sense. Do you think it'd make sense to add test cases that make explicit that these two cases are FN and FP that we intentionally accept?

I think no. These cases are super-low, there's no need to document them.