SONARJAVA-4926 Implement new rule S8989 by romainbrenguier · Pull Request #5770 · SonarSource/sonar-java

romainbrenguier · 2026-07-10T08:06:37Z

This rule detects methods annotated with @transactional from Spring Framework that declare checked exceptions but do not explicitly configure rollback behavior using rollbackFor or noRollbackFor attributes. By default, Spring only rolls back transactions for unchecked exceptions, which can lead to silent data corruption when checked exceptions are thrown but the transaction commits anyway. The rule helps prevent this by requiring explicit rollback configuration for all transactional methods that throw checked exceptions.

RSPEC PR

hashicorp-vault-sonar-prod · 2026-07-10T08:06:57Z

SONARJAVA-4926

gitar-bot · 2026-07-10T10:04:05Z

Code Review 👍 Approved with suggestions 2 resolved / 7 findings

Implements rule S8989 to detect unchecked checked exceptions in @Transactional methods, resolving issues with duplicate reporting and dead code. Please address the missing profile registration, metadata classification, and potential compilation issues with quick fixes.

💡 Bug: Rule S8989 is "ready" but not added to Sonar_way profile

📄 sonar-java-plugin/src/main/resources/org/sonar/l10n/java/rules/java/S8989.json:3

S8989.json declares "status": "ready", but the rule key S8989 is not present in the Sonar_way_profile.json ruleKeys array. Inclusion in that profile is manual (there is no test enforcing it). As a result the rule will be shipped but inactive by default, so users will not benefit from it unless they manually enable it. If the rule is intended to be active in the default profile, add "S8989" to the ruleKeys array in Sonar_way_profile.json. (This may be intentional while the PR is still a draft.)

💡 Quality: Impact/type may misclassify a data-integrity rule as maintainability

📄 sonar-java-plugin/src/main/resources/org/sonar/l10n/java/rules/java/S8989.json:12-18

The rule description centers on silent data corruption, inconsistent database state, and business-logic violations (a reliability/correctness concern), yet the metadata sets type: CODE_SMELL with impacts.MAINTAINABILITY: HIGH. Given the stated consequences (partial commits, orphaned records, silent failures), a RELIABILITY impact (and/or BUG type) would more accurately reflect the issue and drive correct severity/quality-gate treatment. Consider aligning the metadata with the described impact, or confirm the maintainability classification is the intended taxonomy for this rule.

💡 Edge Case: Quick fix uses simple type name; may not compile if not imported

📄 java-checks/src/main/java/org/sonar/java/checks/spring/TransactionalMethodCheckedExceptionCheck.java:113-127

computeQuickFixes builds the rollbackFor value from Type::name, which returns the unqualified type name (e.g. IOException.class). If a checked exception is declared in the throws clause via a fully-qualified name without an import (e.g. throws com.acme.MyException), or is otherwise not imported into the file, applying the quick fix inserts MyException.class, which will not compile. Consider using the fully-qualified name (Type::fullyQualifiedName) for the exception classes, or verifying the simple name is resolvable in the compilation unit.

💡 Bug: Meta-annotated (composed) @Transactional no longer detected

📄 java-checks/src/main/java/org/sonar/java/checks/spring/TransactionalMethodCheckedExceptionCheck.java:88-102

The previous implementation used SymbolMetadata.isAnnotatedWith(TRANSACTIONAL_ANNOTATION), which resolves composed/meta-annotations (e.g. a custom @MyTransactional that is itself meta-annotated with Spring's @Transactional). The new getTransactionalAnnotation matches only annotations whose symbolType().is(TRANSACTIONAL_ANNOTATION) directly, so methods/classes annotated with a composed transactional annotation are no longer flagged — a false-negative regression. If composed annotations are intended to be out of scope this is fine, but otherwise consider also matching annotations meta-annotated with @Transactional.

💡 Quality: Redundant identical quick fixes when sole checked exception is Exception

📄 java-checks/src/main/java/org/sonar/java/checks/spring/TransactionalMethodCheckedExceptionCheck.java:113-127 📄 java-checks-test-sources/default/src/main/java/checks/spring/TransactionalMethodCheckedExceptionCheckSample.java:22-29

When the only checked exception is java.lang.Exception (e.g. importData() throws Exception), quick fix 1 (rollbackFor = Exception.class) and quick fix 2 (rollbackFor = Exception.class) produce identical edits and identical resulting code, only differing in description. The test at qf3/qf4 confirms both edits are the same. Presenting two identical fixes is confusing UX; consider de-duplicating when the computed rollbackFor list already equals Exception.class.

✅ 2 resolved

✅ Bug: Class-level @transactional reports duplicate issues at same location

📄 java-checks/src/main/java/org/sonar/java/checks/spring/TransactionalMethodCheckedExceptionCheck.java:88-102 📄 java-checks/src/main/java/org/sonar/java/checks/spring/TransactionalMethodCheckedExceptionCheck.java:80-85
In getTransactionalAnnotation, when a method has no method-level @Transactional but the enclosing class is annotated, the class-level AnnotationTree is returned. visitNode runs once per METHOD, so for a class with a class-level @Transactional and N methods that declare checked exceptions without rollback config, the check reports the issue N times on the exact same class-level annotation tree, with the same message. This produces duplicate/noisy issues at one location and loses the information about which method is actually problematic (the previous implementation reported on method.simpleName()). Additionally, the generated quick fix adds rollbackFor to the class-level annotation, which changes rollback behavior for every method in the class, not just the offending one. Consider reporting on the method (or its throws clause) while still resolving rollback config from the effective annotation, or de-duplicating class-level reports.

✅ Quality: Dead code: unused hasRollbackConfiguration(SymbolMetadata) and Symbol import

📄 java-checks/src/main/java/org/sonar/java/checks/spring/TransactionalMethodCheckedExceptionCheck.java:33-34 📄 java-checks/src/main/java/org/sonar/java/checks/spring/TransactionalMethodCheckedExceptionCheck.java:178-189
This commit replaced the metadata-based detection with getTransactionalAnnotation / hasRollbackConfiguration(AnnotationTree). As a result, hasRollbackConfiguration(SymbolMetadata) (lines 178-189) is no longer called anywhere, and the org.sonar.plugins.java.api.semantic.Symbol import (line 33) is now unused (grep confirms it appears only in the import statement). The SymbolMetadata import is only referenced by the now-dead method. Remove the dead method and unused imports to keep the check clean.

🤖 Prompt for agents

Code Review: Implements rule S8989 to detect unchecked checked exceptions in @Transactional methods, resolving issues with duplicate reporting and dead code. Please address the missing profile registration, metadata classification, and potential compilation issues with quick fixes.

1. 💡 Bug: Rule S8989 is "ready" but not added to Sonar_way profile
   Files: sonar-java-plugin/src/main/resources/org/sonar/l10n/java/rules/java/S8989.json:3

   S8989.json declares `"status": "ready"`, but the rule key `S8989` is not present in the `Sonar_way_profile.json` `ruleKeys` array. Inclusion in that profile is manual (there is no test enforcing it). As a result the rule will be shipped but inactive by default, so users will not benefit from it unless they manually enable it. If the rule is intended to be active in the default profile, add `"S8989"` to the `ruleKeys` array in `Sonar_way_profile.json`. (This may be intentional while the PR is still a draft.)

2. 💡 Quality: Impact/type may misclassify a data-integrity rule as maintainability
   Files: sonar-java-plugin/src/main/resources/org/sonar/l10n/java/rules/java/S8989.json:12-18

   The rule description centers on silent data corruption, inconsistent database state, and business-logic violations (a reliability/correctness concern), yet the metadata sets `type: CODE_SMELL` with `impacts.MAINTAINABILITY: HIGH`. Given the stated consequences (partial commits, orphaned records, silent failures), a RELIABILITY impact (and/or BUG type) would more accurately reflect the issue and drive correct severity/quality-gate treatment. Consider aligning the metadata with the described impact, or confirm the maintainability classification is the intended taxonomy for this rule.

3. 💡 Edge Case: Quick fix uses simple type name; may not compile if not imported
   Files: java-checks/src/main/java/org/sonar/java/checks/spring/TransactionalMethodCheckedExceptionCheck.java:113-127

   `computeQuickFixes` builds the `rollbackFor` value from `Type::name`, which returns the unqualified type name (e.g. `IOException.class`). If a checked exception is declared in the `throws` clause via a fully-qualified name without an import (e.g. `throws com.acme.MyException`), or is otherwise not imported into the file, applying the quick fix inserts `MyException.class`, which will not compile. Consider using the fully-qualified name (`Type::fullyQualifiedName`) for the exception classes, or verifying the simple name is resolvable in the compilation unit.

4. 💡 Bug: Meta-annotated (composed) @Transactional no longer detected
   Files: java-checks/src/main/java/org/sonar/java/checks/spring/TransactionalMethodCheckedExceptionCheck.java:88-102

   The previous implementation used `SymbolMetadata.isAnnotatedWith(TRANSACTIONAL_ANNOTATION)`, which resolves composed/meta-annotations (e.g. a custom `@MyTransactional` that is itself meta-annotated with Spring's `@Transactional`). The new `getTransactionalAnnotation` matches only annotations whose `symbolType().is(TRANSACTIONAL_ANNOTATION)` directly, so methods/classes annotated with a composed transactional annotation are no longer flagged — a false-negative regression. If composed annotations are intended to be out of scope this is fine, but otherwise consider also matching annotations meta-annotated with `@Transactional`.

5. 💡 Quality: Redundant identical quick fixes when sole checked exception is Exception
   Files: java-checks/src/main/java/org/sonar/java/checks/spring/TransactionalMethodCheckedExceptionCheck.java:113-127, java-checks-test-sources/default/src/main/java/checks/spring/TransactionalMethodCheckedExceptionCheckSample.java:22-29

   When the only checked exception is `java.lang.Exception` (e.g. `importData() throws Exception`), quick fix 1 (`rollbackFor = Exception.class`) and quick fix 2 (`rollbackFor = Exception.class`) produce identical edits and identical resulting code, only differing in description. The test at qf3/qf4 confirms both edits are the same. Presenting two identical fixes is confusing UX; consider de-duplicating when the computed `rollbackFor` list already equals `Exception.class`.

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}

gitar-bot · 2026-07-10T11:50:47Z

Code Review 👍 Approved with suggestions 4 resolved / 7 findings

Implements rule S8989 to detect missing rollback configurations in @Transactional methods, addressing issues with duplicate reporting and misclassification. Ensure the quick fix uses fully qualified names and restore support for meta-annotated annotations to prevent potential compilation and detection gaps.

💡 Edge Case: Quick fix uses simple type name; may not compile if not imported

📄 java-checks/src/main/java/org/sonar/java/checks/spring/TransactionalMethodCheckedExceptionCheck.java:113-127

computeQuickFixes builds the rollbackFor value from Type::name, which returns the unqualified type name (e.g. IOException.class). If a checked exception is declared in the throws clause via a fully-qualified name without an import (e.g. throws com.acme.MyException), or is otherwise not imported into the file, applying the quick fix inserts MyException.class, which will not compile. Consider using the fully-qualified name (Type::fullyQualifiedName) for the exception classes, or verifying the simple name is resolvable in the compilation unit.

💡 Bug: Meta-annotated (composed) @Transactional no longer detected

📄 java-checks/src/main/java/org/sonar/java/checks/spring/TransactionalMethodCheckedExceptionCheck.java:88-102

The previous implementation used SymbolMetadata.isAnnotatedWith(TRANSACTIONAL_ANNOTATION), which resolves composed/meta-annotations (e.g. a custom @MyTransactional that is itself meta-annotated with Spring's @Transactional). The new getTransactionalAnnotation matches only annotations whose symbolType().is(TRANSACTIONAL_ANNOTATION) directly, so methods/classes annotated with a composed transactional annotation are no longer flagged — a false-negative regression. If composed annotations are intended to be out of scope this is fine, but otherwise consider also matching annotations meta-annotated with @Transactional.

💡 Quality: Redundant identical quick fixes when sole checked exception is Exception

📄 java-checks/src/main/java/org/sonar/java/checks/spring/TransactionalMethodCheckedExceptionCheck.java:113-127 📄 java-checks-test-sources/default/src/main/java/checks/spring/TransactionalMethodCheckedExceptionCheckSample.java:22-29

When the only checked exception is java.lang.Exception (e.g. importData() throws Exception), quick fix 1 (rollbackFor = Exception.class) and quick fix 2 (rollbackFor = Exception.class) produce identical edits and identical resulting code, only differing in description. The test at qf3/qf4 confirms both edits are the same. Presenting two identical fixes is confusing UX; consider de-duplicating when the computed rollbackFor list already equals Exception.class.

✅ 4 resolved

✅ Bug: Class-level @transactional reports duplicate issues at same location

📄 java-checks/src/main/java/org/sonar/java/checks/spring/TransactionalMethodCheckedExceptionCheck.java:88-102 📄 java-checks/src/main/java/org/sonar/java/checks/spring/TransactionalMethodCheckedExceptionCheck.java:80-85
In getTransactionalAnnotation, when a method has no method-level @Transactional but the enclosing class is annotated, the class-level AnnotationTree is returned. visitNode runs once per METHOD, so for a class with a class-level @Transactional and N methods that declare checked exceptions without rollback config, the check reports the issue N times on the exact same class-level annotation tree, with the same message. This produces duplicate/noisy issues at one location and loses the information about which method is actually problematic (the previous implementation reported on method.simpleName()). Additionally, the generated quick fix adds rollbackFor to the class-level annotation, which changes rollback behavior for every method in the class, not just the offending one. Consider reporting on the method (or its throws clause) while still resolving rollback config from the effective annotation, or de-duplicating class-level reports.

✅ Quality: Dead code: unused hasRollbackConfiguration(SymbolMetadata) and Symbol import

📄 java-checks/src/main/java/org/sonar/java/checks/spring/TransactionalMethodCheckedExceptionCheck.java:33-34 📄 java-checks/src/main/java/org/sonar/java/checks/spring/TransactionalMethodCheckedExceptionCheck.java:178-189
This commit replaced the metadata-based detection with getTransactionalAnnotation / hasRollbackConfiguration(AnnotationTree). As a result, hasRollbackConfiguration(SymbolMetadata) (lines 178-189) is no longer called anywhere, and the org.sonar.plugins.java.api.semantic.Symbol import (line 33) is now unused (grep confirms it appears only in the import statement). The SymbolMetadata import is only referenced by the now-dead method. Remove the dead method and unused imports to keep the check clean.

✅ Bug: Rule S8989 is "ready" but not added to Sonar_way profile

📄 sonar-java-plugin/src/main/resources/org/sonar/l10n/java/rules/java/S8989.json:3
S8989.json declares "status": "ready", but the rule key S8989 is not present in the Sonar_way_profile.json ruleKeys array. Inclusion in that profile is manual (there is no test enforcing it). As a result the rule will be shipped but inactive by default, so users will not benefit from it unless they manually enable it. If the rule is intended to be active in the default profile, add "S8989" to the ruleKeys array in Sonar_way_profile.json. (This may be intentional while the PR is still a draft.)

✅ Quality: Impact/type may misclassify a data-integrity rule as maintainability

📄 sonar-java-plugin/src/main/resources/org/sonar/l10n/java/rules/java/S8989.json:12-18
The rule description centers on silent data corruption, inconsistent database state, and business-logic violations (a reliability/correctness concern), yet the metadata sets type: CODE_SMELL with impacts.MAINTAINABILITY: HIGH. Given the stated consequences (partial commits, orphaned records, silent failures), a RELIABILITY impact (and/or BUG type) would more accurately reflect the issue and drive correct severity/quality-gate treatment. Consider aligning the metadata with the described impact, or confirm the maintainability classification is the intended taxonomy for this rule.

🤖 Prompt for agents

Code Review: Implements rule S8989 to detect missing rollback configurations in @Transactional methods, addressing issues with duplicate reporting and misclassification. Ensure the quick fix uses fully qualified names and restore support for meta-annotated annotations to prevent potential compilation and detection gaps.

1. 💡 Edge Case: Quick fix uses simple type name; may not compile if not imported
   Files: java-checks/src/main/java/org/sonar/java/checks/spring/TransactionalMethodCheckedExceptionCheck.java:113-127

   `computeQuickFixes` builds the `rollbackFor` value from `Type::name`, which returns the unqualified type name (e.g. `IOException.class`). If a checked exception is declared in the `throws` clause via a fully-qualified name without an import (e.g. `throws com.acme.MyException`), or is otherwise not imported into the file, applying the quick fix inserts `MyException.class`, which will not compile. Consider using the fully-qualified name (`Type::fullyQualifiedName`) for the exception classes, or verifying the simple name is resolvable in the compilation unit.

2. 💡 Bug: Meta-annotated (composed) @Transactional no longer detected
   Files: java-checks/src/main/java/org/sonar/java/checks/spring/TransactionalMethodCheckedExceptionCheck.java:88-102

   The previous implementation used `SymbolMetadata.isAnnotatedWith(TRANSACTIONAL_ANNOTATION)`, which resolves composed/meta-annotations (e.g. a custom `@MyTransactional` that is itself meta-annotated with Spring's `@Transactional`). The new `getTransactionalAnnotation` matches only annotations whose `symbolType().is(TRANSACTIONAL_ANNOTATION)` directly, so methods/classes annotated with a composed transactional annotation are no longer flagged — a false-negative regression. If composed annotations are intended to be out of scope this is fine, but otherwise consider also matching annotations meta-annotated with `@Transactional`.

3. 💡 Quality: Redundant identical quick fixes when sole checked exception is Exception
   Files: java-checks/src/main/java/org/sonar/java/checks/spring/TransactionalMethodCheckedExceptionCheck.java:113-127, java-checks-test-sources/default/src/main/java/checks/spring/TransactionalMethodCheckedExceptionCheckSample.java:22-29

   When the only checked exception is `java.lang.Exception` (e.g. `importData() throws Exception`), quick fix 1 (`rollbackFor = Exception.class`) and quick fix 2 (`rollbackFor = Exception.class`) produce identical edits and identical resulting code, only differing in description. The test at qf3/qf4 confirms both edits are the same. Presenting two identical fixes is confusing UX; consider de-duplicating when the computed `rollbackFor` list already equals `Exception.class`.

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}

NoemieBenard

The implementation looks good, some questions/suggestions on the documentation and agent-generated files

NoemieBenard · 2026-07-13T07:21:35Z

+  "ruleSpecification": "RSPEC-8989",
+  "sqKey": "S8989",
+  "scope": "Main",
+  "quickfix": "unknown",


quickfix shoud be "covered" (or "partial" since it doesn't suggest using "noRollbackFor") based on the implementation

NoemieBenard · 2026-07-13T07:29:12Z

+  <li>Spring Framework Documentation - Transaction Management - <a
+  href="https://docs.spring.io/spring-framework/reference/data-access/transaction/declarative/annotations.html">Official Spring documentation
+  explaining the @Transactional annotation and its rollback behavior</a></li>
+  <li>Baeldung - Spring @Transactional Rollback - <a href="https://www.baeldung.com/spring-transactional-rollback">Practical guide to configuring


I get a 404 on this link

I'm replacing it in the rspec PR.

NoemieBenard · 2026-07-13T07:36:46Z

+  <li>Spring Framework Documentation - Transaction Management - <a
+  href="https://docs.spring.io/spring-framework/reference/data-access/transaction/declarative/annotations.html">Official Spring documentation
+  explaining the @Transactional annotation and its rollback behavior</a></li>


Following the guidelines, it probably should be:

Suggested change

<li>Spring Framework Documentation - Transaction Management - <a

href="https://docs.spring.io/spring-framework/reference/data-access/transaction/declarative/annotations.html">Official Spring documentation

explaining the @Transactional annotation and its rollback behavior</a></li>

<li>Spring Framework Documentation - <a

href="https://docs.spring.io/spring-framework/reference/data-access/transaction/declarative/annotations.html">Using @Transactional </a></li>

NoemieBenard · 2026-07-13T08:03:13Z

I am not sure I understand the purpose of this file

Sorry it looks like it was comited by mistake by nigel

NoemieBenard · 2026-07-13T08:12:14Z

I think this should not be part of the PR (and it is probably the same for TESTING-RULING-DIFF-ACTION.md, or at least we need to move it to an agent directory if we want to keep it as a skill)

@transactional

This rule detects methods annotated with @transactional from Spring Framework that declare checked exceptions but do not explicitly configure rollback behavior using rollbackFor or noRollbackFor attributes. By default, Spring only rolls back transactions for unchecked exceptions, which can lead to silent data corruption when checked exceptions are thrown but the transaction commits anyway. The rule helps prevent this by requiring explicit rollback configuration for all transactional methods that throw checked exceptions.

…ckSample Added caret markers under method names to specify exact issue locations for all Noncompliant cases in the sample file. Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>

@transactional

- Changed issue location from method name to @transactional annotation - Added two quick fixes: 1. Add rollbackFor with specific checked exceptions 2. Add rollbackFor = Exception.class (covers all) - Updated sample file with correct location markers on annotations Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>

- Removed unused imports (Symbol, SymbolMetadata) - Made isCheckedException static - Made hasRollbackConfiguration static - Removed unused hasRollbackConfiguration(SymbolMetadata) method Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>

Added parentheses around ternary operator and string concatenation to make the operator precedence explicit. Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>

@transactional

Added test case for @transactional(value = "txManager") to ensure hasRollbackConfiguration returns false when the annotation has arguments but none of them are rollback-related attributes. This covers the edge case where annotation arguments exist but are not rollback configuration attributes. Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>

@transactional

Added test cases to ensure complete coverage of getTransactionalAnnotation: - Line 92: Exit loop when no method-level @transactional found - Line 97: Parent traversal through nested class structures - Line 105: Exit loop when no class-level @transactional found Test cases added: - Nested class structure (InnerClassWithAnnotation) to test parent traversal - Nested class without annotation to test return null path - Interface with @transactional to test INTERFACE tree kind handling Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>

@transactional

…k fixes 1. Use fully qualified names in quick fixes - Changed Type::name to Type::fullyQualifiedName to avoid compilation errors when exception types are not imported - Ensures quick fixes always produce compilable code 2. Restore support for meta-annotated @transactional - Added isTransactionalAnnotation() helper that checks both direct and meta-annotations using SymbolMetadata.isAnnotatedWith() - Supports composed annotations like custom @MyTransactional that is itself annotated with Spring's @transactional - Prevents false-negatives with composed annotations 3. Deduplicate identical quick fixes - When the only checked exception is Exception, only show one quick fix instead of two identical ones - Improved UX by avoiding confusing duplicate quick fixes 4. Added test for meta-annotated annotation - Created @MyTransactional composed annotation - Verified detection works with meta-annotations Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>

@transactional

Line 168 - isCheckedException returns false for unknown types: - Added unknownException() test with UnresolvedCheckedException - When type.isUnknown() is true, should not raise issue as we cannot determine if it's truly a checked exception Line 187 - hasRollbackConfiguration returns false for non-ASSIGNMENT: - Added valueShorthand() test with @transactional("txManager") - Covers case where annotation has arguments but none are rollback-related configuration - Correctly raises Noncompliant as expected Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>

Created TransactionalMethodCheckedExceptionCheckSampleNonCompiling.java to test behavior with unresolved exception types that cannot be compiled. This properly tests line 168 (type.isUnknown() returns false) by having an UnresolvedCheckedException that is not defined, ensuring the check doesn't raise false positives on unknown types. Moved unknownException test from main sample to non-compiling sample to satisfy sanity checks that expect all main samples to compile. Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>

sonarqube-next · 2026-07-13T09:47:43Z

Quality Gate passed

Issues
0 New issues
0 Fixed issues
0 Accepted issues

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

See analysis details on SonarQube

gitar-bot · 2026-07-13T10:00:52Z

Code Review ✅ Approved 7 resolved / 7 findings

Implements rule S8989 to detect unchecked exceptions in @Transactional methods, resolving issues related to meta-annotation support, duplicate reporting, and quick-fix type resolution. No remaining issues found.

✅ 7 resolved

✅ Bug: Class-level @transactional reports duplicate issues at same location

📄 java-checks/src/main/java/org/sonar/java/checks/spring/TransactionalMethodCheckedExceptionCheck.java:88-102 📄 java-checks/src/main/java/org/sonar/java/checks/spring/TransactionalMethodCheckedExceptionCheck.java:80-85
In getTransactionalAnnotation, when a method has no method-level @Transactional but the enclosing class is annotated, the class-level AnnotationTree is returned. visitNode runs once per METHOD, so for a class with a class-level @Transactional and N methods that declare checked exceptions without rollback config, the check reports the issue N times on the exact same class-level annotation tree, with the same message. This produces duplicate/noisy issues at one location and loses the information about which method is actually problematic (the previous implementation reported on method.simpleName()). Additionally, the generated quick fix adds rollbackFor to the class-level annotation, which changes rollback behavior for every method in the class, not just the offending one. Consider reporting on the method (or its throws clause) while still resolving rollback config from the effective annotation, or de-duplicating class-level reports.

✅ Quality: Dead code: unused hasRollbackConfiguration(SymbolMetadata) and Symbol import

📄 java-checks/src/main/java/org/sonar/java/checks/spring/TransactionalMethodCheckedExceptionCheck.java:33-34 📄 java-checks/src/main/java/org/sonar/java/checks/spring/TransactionalMethodCheckedExceptionCheck.java:178-189
This commit replaced the metadata-based detection with getTransactionalAnnotation / hasRollbackConfiguration(AnnotationTree). As a result, hasRollbackConfiguration(SymbolMetadata) (lines 178-189) is no longer called anywhere, and the org.sonar.plugins.java.api.semantic.Symbol import (line 33) is now unused (grep confirms it appears only in the import statement). The SymbolMetadata import is only referenced by the now-dead method. Remove the dead method and unused imports to keep the check clean.

✅ Bug: Rule S8989 is "ready" but not added to Sonar_way profile

📄 sonar-java-plugin/src/main/resources/org/sonar/l10n/java/rules/java/S8989.json:3
S8989.json declares "status": "ready", but the rule key S8989 is not present in the Sonar_way_profile.json ruleKeys array. Inclusion in that profile is manual (there is no test enforcing it). As a result the rule will be shipped but inactive by default, so users will not benefit from it unless they manually enable it. If the rule is intended to be active in the default profile, add "S8989" to the ruleKeys array in Sonar_way_profile.json. (This may be intentional while the PR is still a draft.)

✅ Quality: Impact/type may misclassify a data-integrity rule as maintainability

📄 sonar-java-plugin/src/main/resources/org/sonar/l10n/java/rules/java/S8989.json:12-18
The rule description centers on silent data corruption, inconsistent database state, and business-logic violations (a reliability/correctness concern), yet the metadata sets type: CODE_SMELL with impacts.MAINTAINABILITY: HIGH. Given the stated consequences (partial commits, orphaned records, silent failures), a RELIABILITY impact (and/or BUG type) would more accurately reflect the issue and drive correct severity/quality-gate treatment. Consider aligning the metadata with the described impact, or confirm the maintainability classification is the intended taxonomy for this rule.

✅ Edge Case: Quick fix uses simple type name; may not compile if not imported

📄 java-checks/src/main/java/org/sonar/java/checks/spring/TransactionalMethodCheckedExceptionCheck.java:113-127
computeQuickFixes builds the rollbackFor value from Type::name, which returns the unqualified type name (e.g. IOException.class). If a checked exception is declared in the throws clause via a fully-qualified name without an import (e.g. throws com.acme.MyException), or is otherwise not imported into the file, applying the quick fix inserts MyException.class, which will not compile. Consider using the fully-qualified name (Type::fullyQualifiedName) for the exception classes, or verifying the simple name is resolvable in the compilation unit.

...and 2 more resolved from earlier reviews

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}

hashicorp-vault-sonar-prod Bot changed the title ~~Implement new rule S8989~~ SONARJAVA-6601 Implement new rule S8989 Jul 10, 2026

romainbrenguier changed the title ~~SONARJAVA-6601 Implement new rule S8989~~ SONARJAVA-4926 Implement new rule S8989 Jul 10, 2026

romainbrenguier commented Jul 10, 2026

View reviewed changes

Comment thread ...rces/default/src/main/java/checks/spring/TransactionalMethodCheckedExceptionCheckSample.java Outdated