diff --git a/its/autoscan/src/test/resources/autoscan/autoscan-diff-by-rules.json b/its/autoscan/src/test/resources/autoscan/autoscan-diff-by-rules.json index 3a77963e001..038a1fc413b 100644 --- a/its/autoscan/src/test/resources/autoscan/autoscan-diff-by-rules.json +++ b/its/autoscan/src/test/resources/autoscan/autoscan-diff-by-rules.json @@ -1076,7 +1076,7 @@ { "ruleKey": "S2230", "hasTruePositives": false, - "falseNegatives": 22, + "falseNegatives": 24, "falsePositives": 0 }, { @@ -3196,5 +3196,11 @@ "hasTruePositives": false, "falseNegatives": 8, "falsePositives": 0 + }, + { + "ruleKey": "S8989", + "hasTruePositives": false, + "falseNegatives": 13, + "falsePositives": 0 } ] diff --git a/its/autoscan/src/test/resources/autoscan/diffs/diff_S2230.json b/its/autoscan/src/test/resources/autoscan/diffs/diff_S2230.json index 2b93c0bf9ff..354535c3df9 100644 --- a/its/autoscan/src/test/resources/autoscan/diffs/diff_S2230.json +++ b/its/autoscan/src/test/resources/autoscan/diffs/diff_S2230.json @@ -1,6 +1,6 @@ { "ruleKey": "S2230", "hasTruePositives": false, - "falseNegatives": 22, + "falseNegatives": 26, "falsePositives": 0 -} +} \ No newline at end of file diff --git a/its/autoscan/src/test/resources/autoscan/diffs/diff_S8989.json b/its/autoscan/src/test/resources/autoscan/diffs/diff_S8989.json index 4019d11648a..819989347c6 100644 --- a/its/autoscan/src/test/resources/autoscan/diffs/diff_S8989.json +++ b/its/autoscan/src/test/resources/autoscan/diffs/diff_S8989.json @@ -1,6 +1,6 @@ { "ruleKey": "S8989", "hasTruePositives": false, - "falseNegatives": 12, + "falseNegatives": 13, "falsePositives": 0 } 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 94501e6bb7c..aa1e53b4865 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 @@ -169,4 +169,37 @@ public void valueShorthand() throws IOException { // Noncompliant [[secondary=16 // ^^^^^^^^^^^^^^ // Has value attribute but no rollback configuration } + + // @Transactional has no effect on non-public methods (Spring proxy-based AOP only intercepts public methods) + @Transactional + private void privateMethod() throws IOException { // Compliant - private methods are not proxied by Spring + } + + @Transactional + private void privateMethodMultipleExceptions() throws IOException, SQLException { // Compliant + } + + @Transactional + protected void protectedMethod() throws IOException { // Compliant - protected methods are not proxied by Spring + } + + @Transactional + void packagePrivateMethod() throws IOException { // Compliant - package-private methods are not proxied by Spring + } + + @Transactional + static class ClassLevelWithNonPublicMethods { + private void privateInClassLevel() throws IOException { // Compliant - private methods are not proxied + } + + protected void protectedInClassLevel() throws IOException { // Compliant - protected methods are not proxied + } + + void packagePrivateInClassLevel() throws IOException { // Compliant - package-private methods are not proxied + } + + public void publicInClassLevel() throws IOException { // Noncompliant [[secondary=190]] +// ^^^^^^^^^^^^^^^^^^ + } + } } 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 d44f2ef38e7..85c2cfce102 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 @@ -25,6 +25,7 @@ import org.sonar.check.Rule; import org.sonar.java.checks.helpers.QuickFixHelper; import org.sonar.java.checks.helpers.SpringUtils; +import org.sonar.java.model.ModifiersUtils; import org.sonar.java.reporting.JavaQuickFix; import org.sonar.java.reporting.JavaTextEdit; import org.sonar.plugins.java.api.DependencyVersionAware; @@ -36,6 +37,7 @@ import org.sonar.plugins.java.api.tree.Arguments; import org.sonar.plugins.java.api.tree.ClassTree; import org.sonar.plugins.java.api.tree.MethodTree; +import org.sonar.plugins.java.api.tree.Modifier; import org.sonar.plugins.java.api.tree.SyntaxToken; import org.sonar.plugins.java.api.tree.Tree; import org.sonar.plugins.java.api.tree.TypeTree; @@ -52,6 +54,11 @@ public List nodesToVisit() { public void visitNode(Tree tree) { MethodTree method = (MethodTree) tree; + // @Transactional has no effect on non-public methods (Spring proxy-based AOP only intercepts public methods) + if (isNonPublicMethod(method)) { + return; + } + List throwsClauses = method.throwsClauses(); if (throwsClauses.isEmpty()) { return; @@ -213,6 +220,19 @@ private static boolean hasRollbackConfiguration(AnnotationTree annotation) { }); } + private static boolean isNonPublicMethod(MethodTree method) { + if (ModifiersUtils.hasModifier(method.modifiers(), Modifier.PRIVATE) + || ModifiersUtils.hasModifier(method.modifiers(), Modifier.PROTECTED)) { + return true; + } + // Methods without an explicit access modifier are package-private in classes but implicitly public in interfaces + if (!ModifiersUtils.hasModifier(method.modifiers(), Modifier.PUBLIC)) { + Tree parent = method.parent(); + return parent == null || !parent.is(Tree.Kind.INTERFACE); + } + return false; + } + @Override public boolean isCompatibleWithDependencies(Function> dependencyFinder) { Optional springContextVersion = dependencyFinder.apply("spring-context");