Skip to content

SONARJAVA-4926: Use secondary locations for class-level @Transactional issues #5786

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
romainbrenguier wants to merge 3 commits into
base: master
Choose a base branch
from
Open

SONARJAVA-4926: Use secondary locations for class-level @Transactional issues #5786

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
b9e71a9
SONARJAVA-4926 Use secondary locations for class-level @Transactional…
romainbrenguier Jul 14, 2026
7543849
Add secondary locations for method-level annotations as well
romainbrenguier Jul 14, 2026
2e65d04
Fix test sample file format for secondary locations with custom messages
romainbrenguier Jul 14, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 3 additions & 2 deletions ...n-compiling/checks/spring/TransactionalMethodCheckedExceptionCheckSampleNonCompiling.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,8 @@ class TransactionalMethodCheckedExceptionCheckSampleNonCompiling {
public void unknownException() throws UnresolvedCheckedException { // No issue - type is unknown
}

@Transactional // Noncompliant
public void knownException() throws java.io.IOException {
@Transactional
public void knownException() throws java.io.IOException { // Noncompliant [[secondary=12]]
// ^^^^^^^^^^^^^^
}
}
90 changes: 45 additions & 45 deletions ...s/default/src/main/java/checks/spring/TransactionalMethodCheckedExceptionCheckSample.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,43 +18,43 @@ class CustomCheckedException extends Exception {}

public class TransactionalMethodCheckedExceptionCheckSample {

@Transactional // Noncompliant {{Specify rollback behavior for checked exceptions using "rollbackFor" or "noRollbackFor" attributes.}} [[quickfixes=qf1,qf2]]
//^^^^^^^^^^^^^^
@Transactional
public void processOrder(Order order) throws IOException, SQLException { // Noncompliant [[secondary=21;quickfixes=qf1,qf2]]
// ^^^^^^^^^^^^
// fix@qf1 {{Add rollbackFor attribute}}
// edit@qf1 [[sc=3;ec=17]] {{@Transactional(rollbackFor = {java.io.IOException.class, java.sql.SQLException.class})}}
// edit@qf1 [[sl=-1;sc=3;el=-1;ec=17]] {{@Transactional(rollbackFor = {java.io.IOException.class, java.sql.SQLException.class})}}
// fix@qf2 {{Add rollbackFor = Exception.class}}
// edit@qf2 [[sc=3;ec=17]] {{@Transactional(rollbackFor = java.lang.Exception.class)}}
public void processOrder(Order order) throws IOException, SQLException {
// edit@qf2 [[sl=-1;sc=3;el=-1;ec=17]] {{@Transactional(rollbackFor = java.lang.Exception.class)}}
}

@Transactional // Noncompliant [[quickfixes=qf3]]
//^^^^^^^^^^^^^^
@Transactional
public void importData() throws Exception { // Noncompliant [[secondary=30;quickfixes=qf3]]
// ^^^^^^^^^^
// fix@qf3 {{Add rollbackFor = Exception.class}}
// edit@qf3 [[sc=3;ec=17]] {{@Transactional(rollbackFor = java.lang.Exception.class)}}
public void importData() throws Exception {
// edit@qf3 [[sl=-1;sc=3;el=-1;ec=17]] {{@Transactional(rollbackFor = java.lang.Exception.class)}}
}

@Transactional(timeout = 30) // Noncompliant
//^^^^^^^^^^^^^^^^^^^^^^^^^^^^
public void withOtherAttributes() throws SQLException {
@Transactional(timeout = 30)
public void withOtherAttributes() throws SQLException { // Noncompliant [[secondary=37]]
// ^^^^^^^^^^^^^^^^^^^
}

@Transactional // Noncompliant [[quickfixes=qf7,qf8]]
//^^^^^^^^^^^^^^
@Transactional
public void customException() throws CustomCheckedException { // Noncompliant [[secondary=42;quickfixes=qf7,qf8]]
// ^^^^^^^^^^^^^^^
// fix@qf7 {{Add rollbackFor attribute}}
// edit@qf7 [[sc=3;ec=17]] {{@Transactional(rollbackFor = checks.spring.CustomCheckedException.class)}}
// edit@qf7 [[sl=-1;sc=3;el=-1;ec=17]] {{@Transactional(rollbackFor = checks.spring.CustomCheckedException.class)}}
// fix@qf8 {{Add rollbackFor = Exception.class}}
// edit@qf8 [[sc=3;ec=17]] {{@Transactional(rollbackFor = java.lang.Exception.class)}}
public void customException() throws CustomCheckedException {
// edit@qf8 [[sl=-1;sc=3;el=-1;ec=17]] {{@Transactional(rollbackFor = java.lang.Exception.class)}}
}

@Transactional // Noncompliant [[quickfixes=qf9,qf10]]
//^^^^^^^^^^^^^^
@Transactional
public void mixedExceptions() throws IOException, RuntimeException { // Noncompliant [[secondary=51;quickfixes=qf9,qf10]]
// ^^^^^^^^^^^^^^^
// fix@qf9 {{Add rollbackFor attribute}}
// edit@qf9 [[sc=3;ec=17]] {{@Transactional(rollbackFor = java.io.IOException.class)}}
// edit@qf9 [[sl=-1;sc=3;el=-1;ec=17]] {{@Transactional(rollbackFor = java.io.IOException.class)}}
// fix@qf10 {{Add rollbackFor = Exception.class}}
// edit@qf10 [[sc=3;ec=17]] {{@Transactional(rollbackFor = java.lang.Exception.class)}}
public void mixedExceptions() throws IOException, RuntimeException {
// edit@qf10 [[sl=-1;sc=3;el=-1;ec=17]] {{@Transactional(rollbackFor = java.lang.Exception.class)}}
}

@Transactional(rollbackFor = IOException.class)
Expand Down Expand Up @@ -102,14 +102,10 @@ public void inherited() throws IOException {
}
}

@Transactional // Noncompliant [[quickfixes=qf11,qf12]]
//^^^^^^^^^^^^^^
// fix@qf11 {{Add rollbackFor attribute}}
// edit@qf11 [[sc=3;ec=17]] {{@Transactional(rollbackFor = java.io.IOException.class)}}
// fix@qf12 {{Add rollbackFor = Exception.class}}
// edit@qf12 [[sc=3;ec=17]] {{@Transactional(rollbackFor = java.lang.Exception.class)}}
@Transactional
static class ClassLevelNoConfig {
public void noConfig() throws IOException {
public void noConfig() throws IOException { // Noncompliant [[secondary=105]]
// ^^^^^^^^ {{Specify rollback behavior for checked exceptions using "rollbackFor" or "noRollbackFor" attributes on the class-level @Transactional.}}
}

@Transactional(rollbackFor = IOException.class)
Expand All @@ -121,29 +117,30 @@ public void methodOverrides() throws IOException {
public void errorNotChecked() throws Error {
}

@org.springframework.transaction.annotation.Transactional // Noncompliant [[quickfixes=qf13,qf14]]
//>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>^^^^^^^^^^^^^^^^^^^^^^^^
@org.springframework.transaction.annotation.Transactional
public void fullyQualified() throws IOException { // Noncompliant [[secondary=120;quickfixes=qf13,qf14]]
// ^^^^^^^^^^^^^^
// fix@qf13 {{Add rollbackFor attribute}}
// edit@qf13 [[sc=3;ec=60]] {{@org.springframework.transaction.annotation.Transactional(rollbackFor = java.io.IOException.class)}}
// edit@qf13 [[sl=-1;sc=3;el=-1;ec=60]] {{@org.springframework.transaction.annotation.Transactional(rollbackFor = java.io.IOException.class)}}
// fix@qf14 {{Add rollbackFor = Exception.class}}
// edit@qf14 [[sc=3;ec=60]] {{@org.springframework.transaction.annotation.Transactional(rollbackFor = java.lang.Exception.class)}}
public void fullyQualified() throws IOException {
// edit@qf14 [[sl=-1;sc=3;el=-1;ec=60]] {{@org.springframework.transaction.annotation.Transactional(rollbackFor = java.lang.Exception.class)}}
}

@Transactional(rollbackFor = IOException.class)
public void partialConfig() throws SQLException {
}

@Transactional(value = "txManager") // Noncompliant
//^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
public void withValueAttribute() throws IOException {
@Transactional(value = "txManager")
public void withValueAttribute() throws IOException { // Noncompliant [[secondary=133]]
// ^^^^^^^^^^^^^^^^^^
}

// Test nested structure to ensure parent traversal works
static class OuterClass {
@Transactional // Noncompliant
@Transactional
static class InnerClassWithAnnotation {
public void nestedMethod() throws IOException {
public void nestedMethod() throws IOException { // Noncompliant [[secondary=140]]
// ^^^^^^^^^^^^ {{Specify rollback behavior for checked exceptions using "rollbackFor" or "noRollbackFor" attributes on the class-level @Transactional.}}
}
}

Expand All @@ -154,19 +151,22 @@ public void nestedMethodNoAnnotation() throws IOException {
}
}

@Transactional // Noncompliant
@Transactional
interface TransactionalInterface {
void interfaceMethod() throws IOException;
void interfaceMethod() throws IOException; // Noncompliant [[secondary=154]]
// ^^^^^^^^^^^^^^^ {{Specify rollback behavior for checked exceptions using "rollbackFor" or "noRollbackFor" attributes on the class-level @Transactional.}}
}

// Test meta-annotated (composed) annotation
@MyTransactional // Noncompliant
public void metaAnnotated() throws IOException {
@MyTransactional
public void metaAnnotated() throws IOException { // Noncompliant [[secondary=161]]
// ^^^^^^^^^^^^^
}

// Test annotation with value attribute (transaction manager name)
@Transactional("txManager") // Noncompliant
public void valueShorthand() throws IOException {
@Transactional("txManager")
public void valueShorthand() throws IOException { // Noncompliant [[secondary=167]]
// ^^^^^^^^^^^^^^
// Has value attribute but no rollback configuration
}
}
35 changes: 30 additions & 5 deletions .../src/main/java/org/sonar/java/checks/spring/TransactionalMethodCheckedExceptionCheck.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@
import org.sonar.java.reporting.JavaTextEdit;
import org.sonar.plugins.java.api.DependencyVersionAware;
import org.sonar.plugins.java.api.IssuableSubscriptionVisitor;
import org.sonar.plugins.java.api.JavaFileScannerContext;
import org.sonar.plugins.java.api.Version;
import org.sonar.plugins.java.api.semantic.Type;
import org.sonar.plugins.java.api.tree.AnnotationTree;
Expand Down Expand Up @@ -75,12 +76,25 @@ public void visitNode(Tree tree) {
return;
}

QuickFixHelper.newIssue(context)
boolean isClassLevel = isClassLevelAnnotation(method, transactionalAnnotation);

var issueBuilder = QuickFixHelper.newIssue(context)
.forRule(this)
.onTree(transactionalAnnotation)
.withMessage("Specify rollback behavior for checked exceptions using \"rollbackFor\" or \"noRollbackFor\" attributes.")
.withQuickFixes(() -> computeQuickFixes(transactionalAnnotation, checkedExceptions))
.report();
.onTree(method.simpleName());

if (isClassLevel) {
issueBuilder
.withMessage("Specify rollback behavior for checked exceptions using \"rollbackFor\" or \"noRollbackFor\" attributes on the class-level @Transactional.")
.withSecondaries(new JavaFileScannerContext.Location("Class-level @Transactional annotation", transactionalAnnotation))
.withQuickFixes(() -> computeQuickFixes(transactionalAnnotation, checkedExceptions));
} else {
issueBuilder
.withMessage("Specify rollback behavior for checked exceptions using \"rollbackFor\" or \"noRollbackFor\" attributes.")
.withSecondaries(new JavaFileScannerContext.Location("@Transactional annotation", transactionalAnnotation))
.withQuickFixes(() -> computeQuickFixes(transactionalAnnotation, checkedExceptions));
}

issueBuilder.report();
}

private static AnnotationTree getTransactionalAnnotation(MethodTree method) {
Expand Down Expand Up @@ -108,6 +122,17 @@ private static AnnotationTree getTransactionalAnnotation(MethodTree method) {
return null;
}

private static boolean isClassLevelAnnotation(MethodTree method, AnnotationTree annotation) {
// Check if the annotation is on the method itself
for (AnnotationTree methodAnnotation : method.modifiers().annotations()) {
if (methodAnnotation == annotation) {
return false;
}
}
// If not on the method, it must be class-level
return true;
}

private static boolean isTransactionalAnnotation(AnnotationTree annotation) {
// Check if the annotation itself is @Transactional
if (annotation.symbolType().is(SpringUtils.TRANSACTIONAL_ANNOTATION)) {
Expand Down
Loading