From d43e827648adb499983342afa55dbf6f40333062 Mon Sep 17 00:00:00 2001 From: Romain Brenguier Date: Mon, 26 Oct 2026 12:28:04 +0100 Subject: [PATCH 1/4] SONARJAVA-7243 Improve S8989: Do not report when all checked exceptions are thrown before any write Use the control flow graph of the method to skip the issue when every point that may throw a checked exception is executed before any write operation. Co-Authored-By: Claude Opus 5.5 (1M context) --- ...eckedExceptionCheckSampleNonCompiling.java | 27 ++++ ...onalMethodCheckedExceptionCheckSample.java | 141 ++++++++++++++++++ ...nsactionalMethodCheckedExceptionCheck.java | 98 ++++++++++++ 3 files changed, 266 insertions(+) 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 e8b14751c9e..e25c7ece2d8 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 @@ -19,4 +19,31 @@ public void knownException() throws java.io.IOException { // Noncompliant [[seco public void propagationFromMethodCall() throws java.io.IOException { // Noncompliant [[secondary=18]] // ^^^^^^^^^^^^^^^^^^^^^^^^^ } + + @Transactional + public void unknownThrownTypeAfterWrite(Repository repository) throws java.io.IOException { // Noncompliant [[secondary=24]] +// ^^^^^^^^^^^^^^^^^^^^^^^^^^^ + repository.save(null); + throw new UnknownException(); + } + + @Transactional + public void unknownMethodAfterWrite(Repository repository) throws java.io.IOException { // Noncompliant [[secondary=31]] +// ^^^^^^^^^^^^^^^^^^^^^^^ + repository.save(null); + unknownMethod(); + } + + @Transactional + public void checkedThrowBeforeWrite(Repository repository) throws java.io.IOException { // Compliant + if (repository == null) { + throw new java.io.IOException(); + } + repository.save(null); + } + + static class Repository { + void save(Object o) { + } + } } 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 0b2b5edc2ad..0b6732a51aa 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 @@ -414,4 +414,145 @@ public void directSaveAndLambda() throws IOException { // Noncompliant [[seconda Runnable r = () -> deleteEntity(null); } } + + static class Repository { + void save(Object o) { + } + + void saveChecked(Object o) throws IOException { + } + } + + static class Validator { + static void validate(Object o) throws IOException { + } + + Validator() throws SQLException { + } + } + + @Transactional + static class ThrowsBeforeWrites { + private final Repository repository = new Repository(); + + public void saveAfterThrow(Object o) throws IOException { // Compliant - all checked exceptions are thrown before any write + if (o == null) { + throw new IOException("null"); + } + repository.save(o); + } + + public void saveAfterValidation(Object o) throws IOException { // Compliant - validation throwing checked exception happens before any write + Validator.validate(o); + repository.save(o); + } + + public void saveAfterConstructor(Object o) throws SQLException { // Compliant - constructor throwing checked exception happens before any write + new Validator(); + repository.save(o); + } + + public void saveAfterThrowInLoop(java.util.List list) throws IOException { // Compliant + for (Object o : list) { + if (o == null) { + throw new IOException("null"); + } + } + list.forEach(repository::save); + list.forEach(o -> repository.save(o)); + } + + public void saveBeforeThrow(Object o) throws IOException { // Noncompliant +// ^^^^^^^^^^^^^^^ + repository.save(o); + if (o == null) { + throw new IOException("null"); + } + } + + public void saveBeforeValidation(Object o) throws IOException { // Noncompliant +// ^^^^^^^^^^^^^^^^^^^^ + repository.save(o); + Validator.validate(o); + } + + public void saveBeforeConstructor(Object o) throws SQLException { // Noncompliant +// ^^^^^^^^^^^^^^^^^^^^^ + repository.save(o); + new Validator(); + } + + public void saveThrowingChecked(Object o) throws IOException { // Noncompliant +// ^^^^^^^^^^^^^^^^^^^ + repository.saveChecked(o); + } + + public void saveInLoopWithThrow(java.util.List list) throws IOException { // Noncompliant +// ^^^^^^^^^^^^^^^^^^^ + for (Object o : list) { + if (o == null) { + throw new IOException("null"); + } + repository.save(o); + } + } + + public void saveInLambdaBeforeThrow(java.util.List list) throws IOException { // Noncompliant +// ^^^^^^^^^^^^^^^^^^^^^^^ + list.forEach(o -> repository.save(o)); + Validator.validate(list); + } + + public void saveInTryThenThrowInCatch(Object o) throws IOException { // Noncompliant +// ^^^^^^^^^^^^^^^^^^^^^^^^^ + try { + repository.save(o); + } catch (RuntimeException e) { + throw new IOException(e); + } + } + + public void saveThenRethrowUnknownType(Object o, Exception e) throws Exception { // Noncompliant +// ^^^^^^^^^^^^^^^^^^^^^^^^^^ + repository.save(o); + throw e; + } + + public void saveAfterUncheckedThrow(Object o) throws IOException { // Noncompliant +// ^^^^^^^^^^^^^^^^^^^^^^^ + if (o == null) { + throw new IllegalArgumentException(); + } + repository.save(o); + throw new IllegalStateException(); + } + } + + private final Repository repository = new Repository(); + + @Transactional + public void methodLevelThrowBeforeWrite(Object o) throws IOException { // Compliant - all checked exceptions are thrown before any write + if (o == null) { + throw new IOException("null"); + } + repository.save(o); + } + + @Transactional + public void methodLevelThrowWithoutWrite(Object o) throws IOException { // Noncompliant +// ^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + if (o == null) { + throw new IOException("null"); + } + o.toString(); + } + + @Transactional + public void methodLevelWriteBeforeThrow(Object o) throws IOException { // Noncompliant +// ^^^^^^^^^^^^^^^^^^^^^^^^^^^ + repository.save(o); + if (o == null) { + throw new IOException("null"); + } + } } 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 2af80683774..0da39d946cf 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 @@ -16,11 +16,15 @@ */ package org.sonar.java.checks.spring; +import java.util.ArrayDeque; import java.util.ArrayList; import java.util.Collections; +import java.util.Deque; +import java.util.HashSet; import java.util.List; import java.util.Locale; import java.util.Optional; +import java.util.Set; import java.util.function.Function; import java.util.stream.Collectors; import org.sonar.check.Rule; @@ -34,6 +38,8 @@ 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.cfg.ControlFlowGraph; +import org.sonar.plugins.java.api.semantic.Symbol; import org.sonar.plugins.java.api.semantic.Type; import org.sonar.plugins.java.api.tree.AnnotationTree; import org.sonar.plugins.java.api.tree.Arguments; @@ -50,6 +56,7 @@ import org.sonar.plugins.java.api.tree.NewClassTree; import org.sonar.plugins.java.api.tree.Modifier; import org.sonar.plugins.java.api.tree.SyntaxToken; +import org.sonar.plugins.java.api.tree.ThrowStatementTree; import org.sonar.plugins.java.api.tree.Tree; import org.sonar.plugins.java.api.tree.TypeTree; @@ -113,6 +120,10 @@ public void visitNode(Tree tree) { return; } + if (allCheckedExceptionsThrownBeforeWrites(method)) { + return; + } + var issueBuilder = QuickFixHelper.newIssue(context) .forRule(this) .onTree(method.simpleName()); @@ -296,6 +307,93 @@ private static boolean looksTransactional(MethodTree method) { return visitor.foundTransactionalCall; } + /** + * Uses the control flow graph of the method to determine whether every point that may throw a checked exception + * is executed before any write operation. In that case, no write can have happened when a checked exception is + * thrown, so the rollback behavior is irrelevant. + * Returns false when the method has no body, no write operation or no point throwing a checked exception. + */ + private static boolean allCheckedExceptionsThrownBeforeWrites(MethodTree method) { + ControlFlowGraph cfg = method.cfg(); + if (cfg == null) { + return false; + } + boolean hasWrite = false; + boolean hasThrowPoint = false; + for (ControlFlowGraph.Block block : cfg.blocks()) { + List items = blockItems(block); + for (int i = 0; i < items.size(); i++) { + hasThrowPoint |= isCheckedThrowPoint(items.get(i)); + if (isWriteOperation(items.get(i))) { + hasWrite = true; + if (items.subList(i, items.size()).stream().anyMatch(TransactionalMethodCheckedExceptionCheck::isCheckedThrowPoint) + || hasReachableThrowPoint(block)) { + return false; + } + } + } + } + return hasWrite && hasThrowPoint; + } + + private static List blockItems(ControlFlowGraph.Block block) { + Tree terminator = block.terminator(); + if (terminator == null) { + return block.elements(); + } + List items = new ArrayList<>(block.elements()); + items.add(terminator); + return items; + } + + private static boolean hasReachableThrowPoint(ControlFlowGraph.Block start) { + Set visited = new HashSet<>(); + Deque toVisit = new ArrayDeque<>(); + addSuccessors(start, toVisit); + while (!toVisit.isEmpty()) { + ControlFlowGraph.Block block = toVisit.pop(); + if (visited.add(block)) { + if (blockItems(block).stream().anyMatch(TransactionalMethodCheckedExceptionCheck::isCheckedThrowPoint)) { + return true; + } + addSuccessors(block, toVisit); + } + } + return false; + } + + private static void addSuccessors(ControlFlowGraph.Block block, Deque toVisit) { + toVisit.addAll(block.successors()); + toVisit.addAll(block.exceptions()); + } + + private static boolean isWriteOperation(Tree tree) { + if (!(tree instanceof MethodInvocationTree) && !(tree instanceof NewClassTree)) { + return false; + } + var visitor = new MethodInvocationVisitor(); + tree.accept(visitor); + return visitor.foundTransactionalCall; + } + + private static boolean isCheckedThrowPoint(Tree tree) { + if (tree instanceof ThrowStatementTree throwStatement) { + Type type = throwStatement.expression().symbolType(); + return type.isUnknown() || isCheckedException(type); + } + if (tree instanceof MethodInvocationTree mit) { + return mayThrowCheckedException(mit.methodSymbol()); + } + if (tree instanceof NewClassTree newClass) { + return mayThrowCheckedException(newClass.methodSymbol()); + } + return false; + } + + private static boolean mayThrowCheckedException(Symbol.MethodSymbol symbol) { + return symbol.isUnknown() || symbol.thrownTypes().stream().anyMatch(type -> type.isUnknown() || isCheckedException(type)); + } + private static boolean hasTransactionalPrefix(String name) { String lowerName = name.toLowerCase(Locale.ROOT); return TRANSACTIONAL_PREFIXES.stream().anyMatch(prefix -> { From c8d68c41c024597abb3aec23dc9d92460f600a10 Mon Sep 17 00:00:00 2001 From: Romain Brenguier Date: Tue, 27 Oct 2026 14:05:37 +0100 Subject: [PATCH 2/4] SONARJAVA-7243 Handle try-with-resources close() and persistence API writes in S8989 Co-Authored-By: Claude Opus 5.5 (1M context) --- ...onalMethodCheckedExceptionCheckSample.java | 33 ++++++ ...nsactionalMethodCheckedExceptionCheck.java | 109 ++++++++++++++++-- 2 files changed, 132 insertions(+), 10 deletions(-) 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 0b6732a51aa..84091d17ff3 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 @@ -518,6 +518,39 @@ public void saveThenRethrowUnknownType(Object o, Exception e) throws Exception { throw e; } + public void saveInTryWithResources(java.nio.file.Path path, Object o) throws IOException { // Noncompliant +// ^^^^^^^^^^^^^^^^^^^^^^ + try (java.io.BufferedReader reader = java.nio.file.Files.newBufferedReader(path)) { + repository.save(o); + } + } + + public void saveInTryWithResourcesNotThrowingOnClose(java.nio.file.Path path, java.util.List list) throws IOException { // Compliant - close() of Stream does not throw checked exceptions + try (java.util.stream.Stream lines = java.nio.file.Files.lines(path)) { + repository.save(lines); + } + } + + public void executeUpdateBeforeValidation(jakarta.persistence.EntityManager em, Object o) throws IOException { // Noncompliant +// ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + em.createQuery("UPDATE Order o SET o.status = 'DONE'").executeUpdate(); + Validator.validate(o); + repository.save(o); + } + + public void batchUpdateBeforeValidation(org.springframework.jdbc.core.JdbcTemplate jdbcTemplate, Object o) throws IOException { // Noncompliant +// ^^^^^^^^^^^^^^^^^^^^^^^^^^^ + jdbcTemplate.batchUpdate("UPDATE orders SET status = 'DONE'"); + Validator.validate(o); + repository.save(o); + } + + public void readBeforeValidation(org.springframework.jdbc.core.JdbcTemplate jdbcTemplate, Object o) throws IOException { // Compliant - only reads before the checked exception + jdbcTemplate.queryForList("SELECT * FROM orders"); + Validator.validate(o); + jdbcTemplate.execute("UPDATE orders SET status = 'DONE'"); + } + public void saveAfterUncheckedThrow(Object o) throws IOException { // Noncompliant // ^^^^^^^^^^^^^^^^^^^^^^^ if (o == null) { 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 0da39d946cf..7b72f56513e 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 @@ -26,7 +26,10 @@ import java.util.Optional; import java.util.Set; import java.util.function.Function; +import java.util.function.Predicate; import java.util.stream.Collectors; +import java.util.stream.Stream; +import javax.annotation.Nullable; import org.sonar.check.Rule; import org.sonar.java.checks.helpers.QuickFixHelper; import org.sonar.java.model.ExpressionUtils; @@ -58,7 +61,9 @@ import org.sonar.plugins.java.api.tree.SyntaxToken; import org.sonar.plugins.java.api.tree.ThrowStatementTree; import org.sonar.plugins.java.api.tree.Tree; +import org.sonar.plugins.java.api.tree.TryStatementTree; import org.sonar.plugins.java.api.tree.TypeTree; +import org.sonar.plugins.java.api.tree.VariableTree; @Rule(key = "S8989") public class TransactionalMethodCheckedExceptionCheck extends IssuableSubscriptionVisitor implements DependencyVersionAware { @@ -66,6 +71,20 @@ public class TransactionalMethodCheckedExceptionCheck extends IssuableSubscripti private static final List TRANSACTIONAL_PREFIXES = List.of( "save", "delete", "update", "persist", "merge", "flush", "insert"); + private static final List READ_PREFIXES = List.of( + "get", "find", "read", "exists", "count", "is", "has", "query", "select", "load", "create", "contains"); + + private static final List PERSISTENCE_TYPES = List.of( + "javax.persistence.EntityManager", + "jakarta.persistence.EntityManager", + "javax.persistence.Query", + "jakarta.persistence.Query", + "org.hibernate.SharedSessionContract", + "org.springframework.jdbc.core.JdbcOperations", + "org.springframework.jdbc.core.namedparam.NamedParameterJdbcOperations", + "org.springframework.data.repository.Repository", + "java.sql.Statement"); + @Override public List nodesToVisit() { return Collections.singletonList(Tree.Kind.METHOD); @@ -302,7 +321,7 @@ private static boolean looksTransactional(MethodTree method) { if (hasTransactionalPrefix(method.simpleName().name())) { return true; } - var visitor = new MethodInvocationVisitor(); + var visitor = new MethodInvocationVisitor(TransactionalMethodCheckedExceptionCheck::hasTransactionalName); method.accept(visitor); return visitor.foundTransactionalCall; } @@ -327,7 +346,8 @@ private static boolean allCheckedExceptionsThrownBeforeWrites(MethodTree method) if (isWriteOperation(items.get(i))) { hasWrite = true; if (items.subList(i, items.size()).stream().anyMatch(TransactionalMethodCheckedExceptionCheck::isCheckedThrowPoint) - || hasReachableThrowPoint(block)) { + || hasReachableThrowPoint(block) + || isInTryWithResourcesClosingWithCheckedException(items.get(i))) { return false; } } @@ -371,11 +391,74 @@ private static boolean isWriteOperation(Tree tree) { if (!(tree instanceof MethodInvocationTree) && !(tree instanceof NewClassTree)) { return false; } - var visitor = new MethodInvocationVisitor(); + var visitor = new MethodInvocationVisitor(TransactionalMethodCheckedExceptionCheck::isPotentialWrite); tree.accept(visitor); return visitor.foundTransactionalCall; } + private static boolean hasTransactionalName(MethodInvocationTree mit) { + return hasTransactionalPrefix(ExpressionUtils.methodName(mit).name()); + } + + /** + * A method invocation is considered as a potential write when its name starts with a transactional prefix, + * or when it is invoked on a persistence API (EntityManager, JdbcTemplate, repository, ...) and its name + * does not look like a read operation. + */ + private static boolean isPotentialWrite(MethodInvocationTree mit) { + if (hasTransactionalName(mit)) { + return true; + } + Symbol.MethodSymbol symbol = mit.methodSymbol(); + if (symbol.isUnknown()) { + return false; + } + Type ownerType = symbol.owner().type(); + return ownerType != null + && PERSISTENCE_TYPES.stream().anyMatch(ownerType::isSubtypeOf) + && !hasPrefix(ExpressionUtils.methodName(mit).name(), READ_PREFIXES); + } + + /** + * The control flow graph does not model the implicit close() calls of try-with-resources, which are executed + * after the body and may throw checked exceptions. + */ + private static boolean isInTryWithResourcesClosingWithCheckedException(Tree tree) { + Tree parent = tree.parent(); + while (parent != null && !(parent instanceof MethodTree)) { + if (parent instanceof TryStatementTree tryStatement + && tryStatement.resourceList().stream().anyMatch(TransactionalMethodCheckedExceptionCheck::closeMayThrowCheckedException)) { + return true; + } + parent = parent.parent(); + } + return false; + } + + private static boolean closeMayThrowCheckedException(Tree resource) { + Type type = resource instanceof VariableTree variable ? variable.symbol().type() : ((ExpressionTree) resource).symbolType(); + return findCloseMethod(type).map(TransactionalMethodCheckedExceptionCheck::mayThrowCheckedException).orElse(true); + } + + private static Optional findCloseMethod(@Nullable Type type) { + if (type == null || type.isUnknown()) { + return Optional.empty(); + } + Symbol.TypeSymbol typeSymbol = type.symbol(); + Optional close = typeSymbol.lookupSymbols("close").stream() + .filter(Symbol::isMethodSymbol) + .map(Symbol.MethodSymbol.class::cast) + .filter(m -> m.parameterTypes().isEmpty()) + .findFirst(); + if (close.isPresent()) { + return close; + } + return Stream.concat(Stream.of(typeSymbol.superClass()), typeSymbol.interfaces().stream()) + .map(TransactionalMethodCheckedExceptionCheck::findCloseMethod) + .flatMap(Optional::stream) + .findFirst(); + } + private static boolean isCheckedThrowPoint(Tree tree) { if (tree instanceof ThrowStatementTree throwStatement) { Type type = throwStatement.expression().symbolType(); @@ -395,8 +478,12 @@ private static boolean mayThrowCheckedException(Symbol.MethodSymbol symbol) { } private static boolean hasTransactionalPrefix(String name) { + return hasPrefix(name, TRANSACTIONAL_PREFIXES); + } + + private static boolean hasPrefix(String name, List prefixes) { String lowerName = name.toLowerCase(Locale.ROOT); - return TRANSACTIONAL_PREFIXES.stream().anyMatch(prefix -> { + return prefixes.stream().anyMatch(prefix -> { if (!lowerName.startsWith(prefix)) { return false; } @@ -409,16 +496,18 @@ private static boolean hasTransactionalPrefix(String name) { } private static class MethodInvocationVisitor extends BaseTreeVisitor { + private final Predicate isTransactionalCall; boolean foundTransactionalCall = false; + MethodInvocationVisitor(Predicate isTransactionalCall) { + this.isTransactionalCall = isTransactionalCall; + } + @Override public void visitMethodInvocation(MethodInvocationTree tree) { - if (!foundTransactionalCall) { - String calledMethodName = ExpressionUtils.methodName(tree).name(); - if (hasTransactionalPrefix(calledMethodName)) { - foundTransactionalCall = true; - return; - } + if (!foundTransactionalCall && isTransactionalCall.test(tree)) { + foundTransactionalCall = true; + return; } super.visitMethodInvocation(tree); } From a5d6027c04d8989e21973264a00f9fa889b320dd Mon Sep 17 00:00:00 2001 From: Romain Brenguier Date: Wed, 28 Oct 2026 11:07:01 +0100 Subject: [PATCH 3/4] SONARJAVA-7243 Handle method references and resources closed after writes in S8989 Co-Authored-By: Claude Opus 5.5 (1M context) --- ...onalMethodCheckedExceptionCheckSample.java | 25 +++++++++++ ...nsactionalMethodCheckedExceptionCheck.java | 42 +++++++++++++++---- 2 files changed, 60 insertions(+), 7 deletions(-) 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 84091d17ff3..50760808aad 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 @@ -503,6 +503,13 @@ public void saveInLambdaBeforeThrow(java.util.List list) throws IOExcept Validator.validate(list); } + public void saveInMethodReferenceBeforeThrow(java.util.List list) throws IOException { // Noncompliant +// ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + list.forEach(repository::save); + Validator.validate(list); + repository.save(list); + } + public void saveInTryThenThrowInCatch(Object o) throws IOException { // Noncompliant // ^^^^^^^^^^^^^^^^^^^^^^^^^ try { @@ -531,6 +538,24 @@ public void saveInTryWithResourcesNotThrowingOnClose(java.nio.file.Path path, ja } } + public void saveBeforeTryWithResources(Object o) throws IOException { // Noncompliant +// ^^^^^^^^^^^^^^^^^^^^^^^^^^ + if (o == null) { + throw new IOException(); + } + repository.save(o); + try (java.io.ByteArrayInputStream in = new java.io.ByteArrayInputStream(new byte[0])) { + in.available(); + } + } + + public void saveAfterTryWithResources(Object o) throws IOException { // Compliant - the resource is closed before the write + try (java.io.ByteArrayInputStream in = new java.io.ByteArrayInputStream(new byte[0])) { + in.available(); + } + repository.save(o); + } + public void executeUpdateBeforeValidation(jakarta.persistence.EntityManager em, Object o) throws IOException { // Noncompliant // ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ em.createQuery("UPDATE Order o SET o.status = 'DONE'").executeUpdate(); 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 7b72f56513e..8fbc319c556 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 @@ -55,6 +55,7 @@ import org.sonar.plugins.java.api.tree.LiteralTree; import org.sonar.plugins.java.api.tree.MemberSelectExpressionTree; import org.sonar.plugins.java.api.tree.MethodInvocationTree; +import org.sonar.plugins.java.api.tree.MethodReferenceTree; import org.sonar.plugins.java.api.tree.MethodTree; import org.sonar.plugins.java.api.tree.NewClassTree; import org.sonar.plugins.java.api.tree.Modifier; @@ -321,7 +322,7 @@ private static boolean looksTransactional(MethodTree method) { if (hasTransactionalPrefix(method.simpleName().name())) { return true; } - var visitor = new MethodInvocationVisitor(TransactionalMethodCheckedExceptionCheck::hasTransactionalName); + var visitor = new MethodInvocationVisitor(TransactionalMethodCheckedExceptionCheck::hasTransactionalName, ref -> false); method.accept(visitor); return visitor.foundTransactionalCall; } @@ -391,7 +392,9 @@ private static boolean isWriteOperation(Tree tree) { if (!(tree instanceof MethodInvocationTree) && !(tree instanceof NewClassTree)) { return false; } - var visitor = new MethodInvocationVisitor(TransactionalMethodCheckedExceptionCheck::isPotentialWrite); + var visitor = new MethodInvocationVisitor( + mit -> isPotentialWrite(mit.methodSymbol(), ExpressionUtils.methodName(mit).name()), + ref -> isPotentialWrite(ref.method().symbol(), ref.method().name())); tree.accept(visitor); return visitor.foundTransactionalCall; } @@ -405,18 +408,17 @@ private static boolean hasTransactionalName(MethodInvocationTree mit) { * or when it is invoked on a persistence API (EntityManager, JdbcTemplate, repository, ...) and its name * does not look like a read operation. */ - private static boolean isPotentialWrite(MethodInvocationTree mit) { - if (hasTransactionalName(mit)) { + private static boolean isPotentialWrite(Symbol symbol, String name) { + if (hasTransactionalPrefix(name)) { return true; } - Symbol.MethodSymbol symbol = mit.methodSymbol(); if (symbol.isUnknown()) { return false; } Type ownerType = symbol.owner().type(); return ownerType != null && PERSISTENCE_TYPES.stream().anyMatch(ownerType::isSubtypeOf) - && !hasPrefix(ExpressionUtils.methodName(mit).name(), READ_PREFIXES); + && !hasPrefix(name, READ_PREFIXES); } /** @@ -435,6 +437,18 @@ private static boolean isInTryWithResourcesClosingWithCheckedException(Tree tree return false; } + /** + * A resource of a try-with-resources is closed after the body of the try. Considering it as a throw point covers + * writes executed before the try statement. + */ + private static boolean isResourceClosingWithCheckedException(Tree tree) { + Tree parent = tree.parent(); + return parent != null + && parent.parent() instanceof TryStatementTree tryStatement + && tryStatement.resourceList() == parent + && closeMayThrowCheckedException(tree); + } + private static boolean closeMayThrowCheckedException(Tree resource) { Type type = resource instanceof VariableTree variable ? variable.symbol().type() : ((ExpressionTree) resource).symbolType(); return findCloseMethod(type).map(TransactionalMethodCheckedExceptionCheck::mayThrowCheckedException).orElse(true); @@ -460,6 +474,9 @@ private static Optional findCloseMethod(@Nullable Type type } private static boolean isCheckedThrowPoint(Tree tree) { + if (isResourceClosingWithCheckedException(tree)) { + return true; + } if (tree instanceof ThrowStatementTree throwStatement) { Type type = throwStatement.expression().symbolType(); return type.isUnknown() || isCheckedException(type); @@ -497,10 +514,21 @@ private static boolean hasPrefix(String name, List prefixes) { private static class MethodInvocationVisitor extends BaseTreeVisitor { private final Predicate isTransactionalCall; + private final Predicate isTransactionalReference; boolean foundTransactionalCall = false; - MethodInvocationVisitor(Predicate isTransactionalCall) { + MethodInvocationVisitor(Predicate isTransactionalCall, Predicate isTransactionalReference) { this.isTransactionalCall = isTransactionalCall; + this.isTransactionalReference = isTransactionalReference; + } + + @Override + public void visitMethodReference(MethodReferenceTree tree) { + if (!foundTransactionalCall && isTransactionalReference.test(tree)) { + foundTransactionalCall = true; + return; + } + super.visitMethodReference(tree); } @Override From 7efe319273dfa44c9caa0b53162157c75aa4862f Mon Sep 17 00:00:00 2001 From: Gitar Date: Wed, 28 Oct 2026 10:18:42 +0000 Subject: [PATCH 4/4] fix: Ignore catch and finally writes for resource closes Co-authored-by: Romain Brenguier <11854194+romainbrenguier@users.noreply.github.com> --- .../spring/TransactionalMethodCheckedExceptionCheck.java | 3 +++ 1 file changed, 3 insertions(+) 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 8fbc319c556..e7105026e8d 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 @@ -426,12 +426,15 @@ private static boolean isPotentialWrite(Symbol symbol, String name) { * after the body and may throw checked exceptions. */ private static boolean isInTryWithResourcesClosingWithCheckedException(Tree tree) { + Tree child = tree; Tree parent = tree.parent(); while (parent != null && !(parent instanceof MethodTree)) { if (parent instanceof TryStatementTree tryStatement + && (child == tryStatement.block() || child == tryStatement.resourceList()) && tryStatement.resourceList().stream().anyMatch(TransactionalMethodCheckedExceptionCheck::closeMayThrowCheckedException)) { return true; } + child = parent; parent = parent.parent(); } return false;