diff --git a/java-checks-test-sources/default/src/main/files/non-compiling/checks/spring/TransactionalMethodCheckedExceptionCheckSampleNonCompiling.java b/java-checks-test-sources/default/src/main/files/non-compiling/checks/spring/TransactionalMethodCheckedExceptionCheckSampleNonCompiling.java index 114af2f93ea..1a2e8bb8f96 100644 --- a/java-checks-test-sources/default/src/main/files/non-compiling/checks/spring/TransactionalMethodCheckedExceptionCheckSampleNonCompiling.java +++ b/java-checks-test-sources/default/src/main/files/non-compiling/checks/spring/TransactionalMethodCheckedExceptionCheckSampleNonCompiling.java @@ -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]] +// ^^^^^^^^^^^^^^ } } diff --git a/java-checks-test-sources/default/src/main/java/checks/spring/TransactionalMethodCheckedExceptionCheckSample.java b/java-checks-test-sources/default/src/main/java/checks/spring/TransactionalMethodCheckedExceptionCheckSample.java index 27b7c44d3f5..94501e6bb7c 100644 --- a/java-checks-test-sources/default/src/main/java/checks/spring/TransactionalMethodCheckedExceptionCheckSample.java +++ b/java-checks-test-sources/default/src/main/java/checks/spring/TransactionalMethodCheckedExceptionCheckSample.java @@ -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) @@ -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) @@ -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.}} } } @@ -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 } } diff --git a/java-checks/src/main/java/org/sonar/java/checks/spring/TransactionalMethodCheckedExceptionCheck.java b/java-checks/src/main/java/org/sonar/java/checks/spring/TransactionalMethodCheckedExceptionCheck.java index 2326574ffb8..d44f2ef38e7 100644 --- a/java-checks/src/main/java/org/sonar/java/checks/spring/TransactionalMethodCheckedExceptionCheck.java +++ b/java-checks/src/main/java/org/sonar/java/checks/spring/TransactionalMethodCheckedExceptionCheck.java @@ -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; @@ -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) { @@ -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)) {