From b56ee32fe11cdb20a5e6c0d2be554c953b30539e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Aur=C3=A9lien=20Coet?= Date: Fri, 22 May 2026 13:51:16 +0200 Subject: [PATCH 1/9] Fix the expected exceptions filter to work without semantic information --- .../java/filters/ExpectedExceptionFilter.java | 259 +++++++++--------- .../filters/ExpectedExceptionFilter.java | 27 +- 2 files changed, 154 insertions(+), 132 deletions(-) diff --git a/java-checks/src/main/java/org/sonar/java/filters/ExpectedExceptionFilter.java b/java-checks/src/main/java/org/sonar/java/filters/ExpectedExceptionFilter.java index 6028ccdfaf8..248142934b4 100644 --- a/java-checks/src/main/java/org/sonar/java/filters/ExpectedExceptionFilter.java +++ b/java-checks/src/main/java/org/sonar/java/filters/ExpectedExceptionFilter.java @@ -20,10 +20,8 @@ import java.util.Optional; import java.util.Set; import org.sonar.java.checks.InstantConversionsCheck; -import org.sonar.java.checks.helpers.MethodTreeUtils; import org.sonar.java.model.ExpressionUtils; import org.sonar.plugins.java.api.JavaCheck; -import org.sonar.plugins.java.api.semantic.MethodMatchers; import org.sonar.plugins.java.api.semantic.Type; import org.sonar.plugins.java.api.tree.AnnotationTree; import org.sonar.plugins.java.api.tree.Arguments; @@ -39,66 +37,53 @@ import org.sonar.plugins.java.api.tree.TypeTree; import org.sonar.plugins.java.api.tree.UnionTypeTree; +import static org.sonar.java.checks.helpers.MethodTreeUtils.consecutiveMethodInvocation; import static org.sonar.java.checks.helpers.UnitTestUtils.isTryCatchFail; +/** + * A filter to deactivate rules that catch expressions raising specific exceptions in contexts where these exceptions are expected. + *

+ * This filter is willingly broad and works directly with type and method names rather than their types, to be resilient even in the face of missing semantic information. + *

+ */ public class ExpectedExceptionFilter extends BaseTreeVisitorIssueFilter { - private static final String ASSERTJ_ASSERTIONS = "org.assertj.core.api.Assertions"; - - private static final MethodMatchers ASSERT_THROWS_MATCHER = MethodMatchers.create() - .ofTypes("org.junit.Assert", "org.junit.jupiter.api.Assertions", "org.testng.Assert", "org.testng.AssertJUnit") - .names("assertThrows", "assertThrowsExactly", "expectThrows") - .withAnyParameters() - .build(); - - private static final MethodMatchers ASSERTJ_CATCH_THROWABLE_OF_TYPE = MethodMatchers.create() - .ofTypes(ASSERTJ_ASSERTIONS) - .names("catchThrowableOfType") - .addParametersMatcher("org.assertj.core.api.ThrowableAssert$ThrowingCallable", "java.lang.Class") - .build(); - - private static final MethodMatchers ASSERTJ_ASSERT_CODE = MethodMatchers.create() - .ofTypes(ASSERTJ_ASSERTIONS) - .names("assertThatCode", "assertThatThrownBy") - .withAnyParameters() - .build(); - - private static final MethodMatchers ASSERTJ_EXCEPTION_OF_TYPE = MethodMatchers.create() - .ofTypes(ASSERTJ_ASSERTIONS, "org.assertj.core.api.BDDAssertions") - .names("assertThatExceptionOfType", "thenExceptionOfType") - .addParametersMatcher("java.lang.Class") - .build(); - - private static final MethodMatchers ASSERTJ_TYPED_EXCEPTION = MethodMatchers.create() - .ofTypes(ASSERTJ_ASSERTIONS, "org.assertj.core.api.BDDAssertions") - .names("assertThatException", "assertThatRuntimeException", "thenException", "thenRuntimeException") - .withAnyParameters() - .build(); - - private static final MethodMatchers ASSERTJ_IS_THROWN_BY = MethodMatchers.create() - .ofTypes("org.assertj.core.api.ThrowableTypeAssert") - .names("isThrownBy") - .addParametersMatcher("org.assertj.core.api.ThrowableAssert$ThrowingCallable") - .build(); - - private static final MethodMatchers ASSERTJ_INSTANCE_OF_PREDICATES = MethodMatchers.create() - .ofSubTypes("org.assertj.core.api.Assert") - .names("isInstanceOf", "isInstanceOfAny") - .withAnyParameters() - .build(); - - private static final MethodMatchers ASSERTJ_EXACT_INSTANCE_OF_PREDICATES = MethodMatchers.create() - .ofSubTypes("org.assertj.core.api.Assert") - .names("isExactlyInstanceOf", "isOfAnyClassIn") - .withAnyParameters() - .build(); - - private static final String DATE_TIME_EXCEPTION = "java.time.DateTimeException"; - - private static final Set DATE_TIME_EXCEPTION_SUPERTYPES = Set.of( - "java.lang.RuntimeException", - "java.lang.Exception", - "java.lang.Throwable" + private static final Set DATE_TIME_EXCEPTION_TYPES = Set.of( + "DateTimeException", + "DateTimeParseException", + "RuntimeException", + "Exception", + "Throwable" + ); + + private static final Set ASSERT_THROWS_METHODS = Set.of( + "assertThrows", + "assertThrowsExactly", + "expectThrows" + ); + + private static final Set ASSERT_CODE_METHODS = Set.of( + "assertThatCode", + "assertThatThrownBy" + ); + + private static final Set INSTANCEOF_METHODS = Set.of( + "isInstanceOf", + "isInstanceOfAny", + "isExactlyInstanceOf", + "isOfAnyClassIn" + ); + + private static final Set ASSERT_EXCEPTION_METHODS = Set.of( + "assertThatException", + "assertThatRuntimeException", + "thenException", + "thenRuntimeException" + ); + + private static final Set ASSERT_OF_TYPE_METHODS = Set.of( + "assertThatExceptionOfType", + "thenExceptionOfType" ); @Override @@ -108,7 +93,7 @@ public Set> filteredRules() { @Override public void visitMethod(MethodTree tree) { - if (hasExpectedDateTimeExceptionAnnotation(tree.modifiers().annotations())) { + if (containsExpectedExceptions(tree.modifiers().annotations(), DATE_TIME_EXCEPTION_TYPES)) { excludeLines(tree, InstantConversionsCheck.class); } super.visitMethod(tree); @@ -116,7 +101,7 @@ public void visitMethod(MethodTree tree) { @Override public void visitTryStatement(TryStatementTree tree) { - if (isTryCatchFailExpectingDateTimeException(tree)) { + if (catchesExpectedException(tree, DATE_TIME_EXCEPTION_TYPES)) { excludeLines(tree.block(), InstantConversionsCheck.class); } super.visitTryStatement(tree); @@ -124,67 +109,86 @@ public void visitTryStatement(TryStatementTree tree) { @Override public void visitMethodInvocation(MethodInvocationTree tree) { - excludeExpectedDateTimeExceptionIssues(tree); + excludeExpectedExceptions(tree, DATE_TIME_EXCEPTION_TYPES, InstantConversionsCheck.class); super.visitMethodInvocation(tree); } - private void excludeExpectedDateTimeExceptionIssues(MethodInvocationTree mit) { - if (ASSERT_THROWS_MATCHER.matches(mit)) { - excludeAssertThrowsExecutableForDateTimeException(mit); - } else if (ASSERTJ_CATCH_THROWABLE_OF_TYPE.matches(mit) && isDateTimeExceptionClass(mit.arguments().get(1), false)) { - excludeLines(mit.arguments().get(0), InstantConversionsCheck.class); - } else if (ASSERTJ_ASSERT_CODE.matches(mit)) { - MethodTreeUtils.subsequentMethodInvocation(mit, ASSERTJ_INSTANCE_OF_PREDICATES).ifPresent(subsequentMit -> { - if (hasDateTimeExceptionType(subsequentMit.arguments(), false)) { - excludeLines(mit.arguments().get(0), InstantConversionsCheck.class); - } - }); - MethodTreeUtils.subsequentMethodInvocation(mit, ASSERTJ_EXACT_INSTANCE_OF_PREDICATES).ifPresent(subsequentMit -> { - if (hasDateTimeExceptionType(subsequentMit.arguments(), true)) { - excludeLines(mit.arguments().get(0), InstantConversionsCheck.class); + /** + * Filter a given rule for parts of a method invocation expression when it expects a given set of exception types. + */ + private void excludeExpectedExceptions(MethodInvocationTree tree, Set expectedExceptions, Class filteredRule) { + String methodName = tree.methodSymbol().name(); + Arguments arguments = tree.arguments(); + if (ASSERT_THROWS_METHODS.contains(methodName) && arguments.size() >= 2) { + int expectedTypeIndex = firstArgumentIsMessage(arguments) ? 1 : 0; + int executableIndex = expectedTypeIndex + 1; + if (arguments.size() > executableIndex && containsExpectedExceptions(arguments.get(expectedTypeIndex), expectedExceptions)) { + excludeLines(arguments.get(executableIndex), filteredRule); + } + } else if ("catchThrowableOfType".equals(methodName) && containsExpectedExceptions(arguments.get(1), expectedExceptions)) { + excludeLines(arguments.get(0), filteredRule); + } else if (ASSERT_CODE_METHODS.contains(methodName)) { + subsequentMethodInvocation(tree, INSTANCEOF_METHODS).ifPresent(mit -> { + if (mit.arguments().stream().anyMatch(expression -> containsExpectedExceptions(expression, expectedExceptions))) { + excludeLines(arguments.get(0), filteredRule); } }); - } else if (ASSERTJ_TYPED_EXCEPTION.matches(mit) || (ASSERTJ_EXCEPTION_OF_TYPE.matches(mit) && isDateTimeExceptionClass(mit.arguments().get(0), false))) { - MethodTreeUtils.subsequentMethodInvocation(mit, ASSERTJ_IS_THROWN_BY).ifPresent(subsequentMit -> - excludeLines(subsequentMit.arguments().get(0), InstantConversionsCheck.class) - ); + } else if (ASSERT_EXCEPTION_METHODS.contains(methodName) || + (ASSERT_OF_TYPE_METHODS.contains(methodName) && containsExpectedExceptions(tree.arguments().get(0), expectedExceptions))) { + subsequentMethodInvocation(tree, Set.of("isThrownBy")).ifPresent(mit -> excludeLines(mit.arguments().get(0), filteredRule)); } } - private void excludeAssertThrowsExecutableForDateTimeException(MethodInvocationTree mit) { - Arguments arguments = mit.arguments(); - if (arguments.size() < 2) { - return; - } - int expectedTypeIndex = firstArgumentIsMessage(arguments) ? 1 : 0; - int executableIndex = expectedTypeIndex + 1; - if (arguments.size() > executableIndex && isDateTimeExceptionClass(arguments.get(expectedTypeIndex), "assertThrowsExactly".equals(mit.methodSymbol().name()))) { - excludeLines(arguments.get(executableIndex), InstantConversionsCheck.class); - } + /** + * Check if a list of annotations contains a {@code Test} annotation expecting specific exception types. + */ + private static boolean containsExpectedExceptions(List annotations, Set expectedExceptions) { + return annotations.stream() + .anyMatch(annotation -> { + Type annotationType = annotation.annotationType().symbolType(); + Arguments arguments = annotation.arguments(); + return "Test".equals(annotationType.name()) && containsExpectedExceptions(arguments, expectedExceptions); + }); } - private static boolean firstArgumentIsMessage(Arguments arguments) { - return arguments.size() >= 3 && arguments.get(0).symbolType().is("java.lang.String"); + /** + * Check if a {@code Test} annotation has an {@code expected} or {@code expectedException} argument matching a set of expected exception types. + */ + private static boolean containsExpectedExceptions(Arguments arguments, Set expectedExceptions) { + return arguments.stream() + .filter(ExpectedExceptionFilter::isExpectedExceptionArgument) + .anyMatch(argument -> containsExpectedExceptions(annotationValue(argument), expectedExceptions)); } - private static boolean isTryCatchFailExpectingDateTimeException(TryStatementTree tryStatement) { - return isTryCatchFail(tryStatement.block()) && tryStatement.catches().stream().anyMatch(ExpectedExceptionFilter::isDateTimeExceptionCatch); + /** + * Check that an annotation argument is an {@code expected} or {@code expectedException} attribute. + */ + private static boolean isExpectedExceptionArgument(ExpressionTree expression) { + String annotationAttributeName = ExpressionUtils.annotationAttributeName(expression); + return "expected".equals(annotationAttributeName) || "expectedExceptions".equals(annotationAttributeName); } - private static boolean isDateTimeException(Type type, boolean exact) { - return type.isSubtypeOf(DATE_TIME_EXCEPTION) || (!exact && DATE_TIME_EXCEPTION_SUPERTYPES.stream().anyMatch(type::is)); + /** + * Check if an expression contains a given exception type. + */ + private static boolean containsExpectedExceptions(ExpressionTree expression, Set expectedExceptions) { + if (expression instanceof NewArrayTree newArray) { + return newArray.initializers().stream() + .anyMatch(initializer -> containsExpectedExceptions(initializer, expectedExceptions)); + } + return classLiteralType(expression).map(type -> expectedExceptions.contains(type.name())).orElse(false); } - private static boolean isDateTimeExceptionClass(ExpressionTree expression, boolean exact) { - if (expression.is(Tree.Kind.NEW_ARRAY)) { - return ((NewArrayTree) expression).initializers().stream() - .anyMatch(initializer -> isDateTimeExceptionClass(initializer, exact)); - } - return classLiteralType(expression) - .map(type -> isDateTimeException(type, exact)) - .orElse(false); + /** + * Get the value of an annotation attribute. + */ + private static ExpressionTree annotationValue(ExpressionTree expression) { + return expression.is(Tree.Kind.ASSIGNMENT) ? ((AssignmentExpressionTree) expression).expression() : expression; } + /** + * Extract the type name of a class literal expression. + */ private static Optional classLiteralType(ExpressionTree expression) { if (expression.is(Tree.Kind.MEMBER_SELECT)) { MemberSelectExpressionTree memberSelect = (MemberSelectExpressionTree) expression; @@ -195,42 +199,39 @@ private static Optional classLiteralType(ExpressionTree expression) { return Optional.empty(); } - private static boolean isDateTimeExceptionCatch(CatchTree catchTree) { + /** + * Check if a try statement catches a set of expected exception types. + */ + private static boolean catchesExpectedException(TryStatementTree tryStatement, Set expectedExceptions) { + return isTryCatchFail(tryStatement.block()) && + tryStatement.catches().stream() + .anyMatch(catchTree -> catchesExpectedExceptions(catchTree, expectedExceptions)); + } + + private static boolean catchesExpectedExceptions(CatchTree catchTree, Set expectedExceptions) { return exceptionTypes(catchTree.parameter().type()).stream() - .anyMatch(type -> isDateTimeException(type.symbolType(), false)); + .anyMatch(type -> expectedExceptions.contains(type.symbolType().name())); } private static List exceptionTypes(TypeTree typeTree) { - if (typeTree.is(Tree.Kind.UNION_TYPE)) { - return ((UnionTypeTree) typeTree).typeAlternatives(); + if (typeTree instanceof UnionTypeTree unionType) { + return unionType.typeAlternatives(); } return List.of(typeTree); } - private static boolean hasExpectedDateTimeExceptionAnnotation(List annotations) { - return annotations.stream().anyMatch(annotation -> { - Type annotationType = annotation.annotationType().symbolType(); - if (annotationType.is("org.junit.Test")) { - return hasExpectedDateTimeExceptionArgument(annotation, "expected"); - } else if (annotationType.is("org.testng.annotations.Test")) { - return hasExpectedDateTimeExceptionArgument(annotation, "expectedExceptions"); - } - return false; - }); - } - - private static boolean hasExpectedDateTimeExceptionArgument(AnnotationTree annotation, String attributeName) { - return annotation.arguments().stream() - .filter(argument -> attributeName.equals(ExpressionUtils.annotationAttributeName(argument))) - .anyMatch(argument -> isDateTimeExceptionClass(annotationValue(argument), false)); - } - - private static ExpressionTree annotationValue(ExpressionTree expression) { - return expression.is(Tree.Kind.ASSIGNMENT) ? ((AssignmentExpressionTree) expression).expression() : expression; + private static boolean firstArgumentIsMessage(Arguments arguments) { + return arguments.size() >= 3 && arguments.get(0).symbolType().is("java.lang.String"); } - private static boolean hasDateTimeExceptionType(List expressions, boolean exact) { - return expressions.stream().anyMatch(expression -> isDateTimeExceptionClass(expression, exact)); + /** + * Get the next chained method invocation whose identifier matches a set of expected method names. + */ + private static Optional subsequentMethodInvocation(MethodInvocationTree tree, Set expectedMethodNames) { + return consecutiveMethodInvocation(tree) + .map(consecutiveMethod -> + expectedMethodNames.contains(consecutiveMethod.methodSymbol().name()) ? + consecutiveMethod : subsequentMethodInvocation(consecutiveMethod, expectedMethodNames).orElse(null)); } } diff --git a/java-checks/src/test/files/filters/ExpectedExceptionFilter.java b/java-checks/src/test/files/filters/ExpectedExceptionFilter.java index 312a3582d08..65f99e4859f 100644 --- a/java-checks/src/test/files/filters/ExpectedExceptionFilter.java +++ b/java-checks/src/test/files/filters/ExpectedExceptionFilter.java @@ -65,7 +65,7 @@ void junit5AssertThrowsExpectedBroadException() { org.junit.jupiter.api.Assertions.assertThrows(() -> LocalDate.from(instant)); // NoIssue org.junit.jupiter.api.Assertions.assertThrowsExactly(DateTimeException.class, () -> LocalDateTime.from(instant)); // NoIssue org.junit.jupiter.api.Assertions.assertThrowsExactly(DateTimeParseException.class, () -> LocalDateTime.from(instant)); // NoIssue - org.junit.jupiter.api.Assertions.assertThrowsExactly(RuntimeException.class, () -> LocalDateTime.from(instant)); // WithIssue + org.junit.jupiter.api.Assertions.assertThrowsExactly(RuntimeException.class, () -> LocalDateTime.from(instant)); // NoIssue } @org.junit.jupiter.api.Test @@ -93,9 +93,9 @@ void testngExpectThrows() { void assertjAssertThatThrownBy() { org.assertj.core.api.Assertions.assertThatThrownBy(() -> Instant.from(date)).isInstanceOf(DateTimeException.class); // NoIssue org.assertj.core.api.Assertions.assertThatThrownBy(() -> Instant.from(date)).isInstanceOf(RuntimeException.class); // NoIssue - org.assertj.core.api.Assertions.assertThatThrownBy(() -> Instant.from(date)).isInstanceOf(IllegalArgumentException.class); // WithIssue - org.assertj.core.api.Assertions.assertThatThrownBy(() -> Instant.from(date)).isExactlyInstanceOf(RuntimeException.class); // WithIssue + org.assertj.core.api.Assertions.assertThatThrownBy(() -> Instant.from(date)).isExactlyInstanceOf(RuntimeException.class); // NoIssue org.assertj.core.api.Assertions.assertThatThrownBy(() -> Instant.from(date)).isExactlyInstanceOf(DateTimeException.class); // NoIssue + org.assertj.core.api.Assertions.assertThatThrownBy(() -> Instant.from(date)).isInstanceOf(IllegalArgumentException.class); // WithIssue } @org.junit.jupiter.api.Test @@ -153,4 +153,25 @@ void tryCatchFail() { } } + // When semantic information is missing, the filter should conservatively activate. + @Test(expected = RuntimeException.class) + void methodAnnotationWithoutSemantics() { + Instant.from(date); // NoIssue + } + + @Test(expectedExceptions = DateTimeException.class) + void methodAnnotationWithoutSemantics() { + Instant.from(date); // NoIssue + } + + @Test + void methodInvocationsWithoutSemantics() { + // Here too, the filter should activate even though no semantic information is available. + assertThrows(DateTimeException.class, () -> LocalDate.from(instant)); // NoIssue + assertThatThrownBy(() -> Instant.from(date)).isInstanceOf(DateTimeException.class); // NoIssue + assertThatExceptionOfType(DateTimeException.class).isThrownBy(() -> ZonedDateTime.from(dateTime)); // NoIssue + assertThatIllegalArgumentException().isThrownBy(() -> Instant.from(date)); // WithIssue + thenRuntimeException().isThrownBy(() -> Instant.from(date)); // NoIssue + } + } From 46294e63d37a6b87fdf4ded6f37849ccb624b2fb Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Aur=C3=A9lien=20Coet?= Date: Fri, 22 May 2026 13:58:46 +0200 Subject: [PATCH 2/9] Apply review bot suggestion --- .../src/test/files/filters/ExpectedExceptionFilter.java | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/java-checks/src/test/files/filters/ExpectedExceptionFilter.java b/java-checks/src/test/files/filters/ExpectedExceptionFilter.java index 65f99e4859f..69e44ced91d 100644 --- a/java-checks/src/test/files/filters/ExpectedExceptionFilter.java +++ b/java-checks/src/test/files/filters/ExpectedExceptionFilter.java @@ -155,12 +155,12 @@ void tryCatchFail() { // When semantic information is missing, the filter should conservatively activate. @Test(expected = RuntimeException.class) - void methodAnnotationWithoutSemantics() { + void expectedMethodAnnotationWithoutSemantics() { Instant.from(date); // NoIssue } @Test(expectedExceptions = DateTimeException.class) - void methodAnnotationWithoutSemantics() { + void expectedExceptionsMethodAnnotationWithoutSemantics() { Instant.from(date); // NoIssue } From b602d8d2088f6d42892f48440d38c47c6730a69e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Aur=C3=A9lien=20Coet?= Date: Fri, 22 May 2026 14:07:04 +0200 Subject: [PATCH 3/9] Add guards to accesses to arguments indexes --- .../sonar/java/filters/ExpectedExceptionFilter.java | 13 +++++++++---- 1 file changed, 9 insertions(+), 4 deletions(-) diff --git a/java-checks/src/main/java/org/sonar/java/filters/ExpectedExceptionFilter.java b/java-checks/src/main/java/org/sonar/java/filters/ExpectedExceptionFilter.java index 248142934b4..cadc3b2c8cb 100644 --- a/java-checks/src/main/java/org/sonar/java/filters/ExpectedExceptionFilter.java +++ b/java-checks/src/main/java/org/sonar/java/filters/ExpectedExceptionFilter.java @@ -125,17 +125,22 @@ private void excludeExpectedExceptions(MethodInvocationTree tree, Set ex if (arguments.size() > executableIndex && containsExpectedExceptions(arguments.get(expectedTypeIndex), expectedExceptions)) { excludeLines(arguments.get(executableIndex), filteredRule); } - } else if ("catchThrowableOfType".equals(methodName) && containsExpectedExceptions(arguments.get(1), expectedExceptions)) { + } else if ("catchThrowableOfType".equals(methodName) && arguments.size() > 1 && containsExpectedExceptions(arguments.get(1), expectedExceptions)) { excludeLines(arguments.get(0), filteredRule); } else if (ASSERT_CODE_METHODS.contains(methodName)) { subsequentMethodInvocation(tree, INSTANCEOF_METHODS).ifPresent(mit -> { - if (mit.arguments().stream().anyMatch(expression -> containsExpectedExceptions(expression, expectedExceptions))) { + if (!arguments.isEmpty() && mit.arguments().stream().anyMatch(expression -> containsExpectedExceptions(expression, expectedExceptions))) { excludeLines(arguments.get(0), filteredRule); } }); } else if (ASSERT_EXCEPTION_METHODS.contains(methodName) || - (ASSERT_OF_TYPE_METHODS.contains(methodName) && containsExpectedExceptions(tree.arguments().get(0), expectedExceptions))) { - subsequentMethodInvocation(tree, Set.of("isThrownBy")).ifPresent(mit -> excludeLines(mit.arguments().get(0), filteredRule)); + (ASSERT_OF_TYPE_METHODS.contains(methodName) && !arguments.isEmpty() && containsExpectedExceptions(arguments.get(0), expectedExceptions))) { + subsequentMethodInvocation(tree, Set.of("isThrownBy")).ifPresent(mit -> { + Arguments mitArguments = mit.arguments(); + if (!mitArguments.isEmpty()) { + excludeLines(mitArguments.get(0), filteredRule); + } + }); } } From 3514ab7208feb99b57d8f8160892befc0571f5ff Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Aur=C3=A9lien=20Coet?= Date: Fri, 22 May 2026 14:16:33 +0200 Subject: [PATCH 4/9] Reduce cognitive complexity of the excludeExpectedExceptions method --- .../java/filters/ExpectedExceptionFilter.java | 44 ++++++++++++------- 1 file changed, 28 insertions(+), 16 deletions(-) diff --git a/java-checks/src/main/java/org/sonar/java/filters/ExpectedExceptionFilter.java b/java-checks/src/main/java/org/sonar/java/filters/ExpectedExceptionFilter.java index cadc3b2c8cb..c517133b539 100644 --- a/java-checks/src/main/java/org/sonar/java/filters/ExpectedExceptionFilter.java +++ b/java-checks/src/main/java/org/sonar/java/filters/ExpectedExceptionFilter.java @@ -120,30 +120,42 @@ private void excludeExpectedExceptions(MethodInvocationTree tree, Set ex String methodName = tree.methodSymbol().name(); Arguments arguments = tree.arguments(); if (ASSERT_THROWS_METHODS.contains(methodName) && arguments.size() >= 2) { - int expectedTypeIndex = firstArgumentIsMessage(arguments) ? 1 : 0; - int executableIndex = expectedTypeIndex + 1; - if (arguments.size() > executableIndex && containsExpectedExceptions(arguments.get(expectedTypeIndex), expectedExceptions)) { - excludeLines(arguments.get(executableIndex), filteredRule); - } + excludeFromAssertThrows(arguments, expectedExceptions, filteredRule); } else if ("catchThrowableOfType".equals(methodName) && arguments.size() > 1 && containsExpectedExceptions(arguments.get(1), expectedExceptions)) { excludeLines(arguments.get(0), filteredRule); } else if (ASSERT_CODE_METHODS.contains(methodName)) { - subsequentMethodInvocation(tree, INSTANCEOF_METHODS).ifPresent(mit -> { - if (!arguments.isEmpty() && mit.arguments().stream().anyMatch(expression -> containsExpectedExceptions(expression, expectedExceptions))) { - excludeLines(arguments.get(0), filteredRule); - } - }); + excludeFromAssertCode(tree, arguments, expectedExceptions, filteredRule); } else if (ASSERT_EXCEPTION_METHODS.contains(methodName) || (ASSERT_OF_TYPE_METHODS.contains(methodName) && !arguments.isEmpty() && containsExpectedExceptions(arguments.get(0), expectedExceptions))) { - subsequentMethodInvocation(tree, Set.of("isThrownBy")).ifPresent(mit -> { - Arguments mitArguments = mit.arguments(); - if (!mitArguments.isEmpty()) { - excludeLines(mitArguments.get(0), filteredRule); - } - }); + excludeIsThrownBy(tree, filteredRule); + } + } + + private void excludeFromAssertThrows(Arguments arguments, Set expectedExceptions, Class filteredRule) { + int expectedTypeIndex = firstArgumentIsMessage(arguments) ? 1 : 0; + int executableIndex = expectedTypeIndex + 1; + if (arguments.size() > executableIndex && containsExpectedExceptions(arguments.get(expectedTypeIndex), expectedExceptions)) { + excludeLines(arguments.get(executableIndex), filteredRule); } } + private void excludeFromAssertCode(MethodInvocationTree tree, Arguments arguments, Set expectedExceptions, Class filteredRule) { + subsequentMethodInvocation(tree, INSTANCEOF_METHODS).ifPresent(mit -> { + if (!arguments.isEmpty() && mit.arguments().stream().anyMatch(expression -> containsExpectedExceptions(expression, expectedExceptions))) { + excludeLines(arguments.get(0), filteredRule); + } + }); + } + + private void excludeIsThrownBy(MethodInvocationTree tree, Class filteredRule) { + subsequentMethodInvocation(tree, Set.of("isThrownBy")).ifPresent(mit -> { + Arguments mitArguments = mit.arguments(); + if (!mitArguments.isEmpty()) { + excludeLines(mitArguments.get(0), filteredRule); + } + }); + } + /** * Check if a list of annotations contains a {@code Test} annotation expecting specific exception types. */ From bf9ab7a042d52c2cc73772e5d4adb7f1c8b3c01f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Aur=C3=A9lien=20Coet?= Date: Fri, 22 May 2026 16:22:51 +0200 Subject: [PATCH 5/9] Increase test coverage --- .../filters/ExpectedExceptionFilter.java | 21 +++++++++++++++++++ 1 file changed, 21 insertions(+) diff --git a/java-checks/src/test/files/filters/ExpectedExceptionFilter.java b/java-checks/src/test/files/filters/ExpectedExceptionFilter.java index 69e44ced91d..e88716eae32 100644 --- a/java-checks/src/test/files/filters/ExpectedExceptionFilter.java +++ b/java-checks/src/test/files/filters/ExpectedExceptionFilter.java @@ -151,6 +151,12 @@ void tryCatchFail() { } catch (DateTimeException | IllegalArgumentException e) { // expected } + + try { + LocalDate.from(instant); // WithIssue + } catch (DateTimeException e) { + // expected + } } // When semantic information is missing, the filter should conservatively activate. @@ -174,4 +180,19 @@ void methodInvocationsWithoutSemantics() { thenRuntimeException().isThrownBy(() -> Instant.from(date)); // NoIssue } + @Test(expected = DateTimeException) + void wrongExpectedType() { + Instant.from(date); // WithIssue + } + + @Test(expect = DateTimeException.class) + void wrongAnnotationAttribute() { + Instant.from(date); // WithIssue + } + + @Test + void wrongAssertThrowsMethodSignature() { + assertThrows(Instant.from(date)); // WithIssue + } + } From b0191068972cbf7bb90517eaff39639375bfb514 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Aur=C3=A9lien=20Coet?= Date: Fri, 22 May 2026 16:38:02 +0200 Subject: [PATCH 6/9] Increase test coverage again --- .../src/test/files/filters/ExpectedExceptionFilter.java | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/java-checks/src/test/files/filters/ExpectedExceptionFilter.java b/java-checks/src/test/files/filters/ExpectedExceptionFilter.java index e88716eae32..471ce83c334 100644 --- a/java-checks/src/test/files/filters/ExpectedExceptionFilter.java +++ b/java-checks/src/test/files/filters/ExpectedExceptionFilter.java @@ -112,6 +112,7 @@ void assertjExceptionOfType() { @org.junit.jupiter.api.Test void assertjCatchThrowableOfType() { org.assertj.core.api.Assertions.catchThrowableOfType(() -> OffsetTime.from(instant), DateTimeException.class); // NoIssue + org.assertj.core.api.Assertions.catchThrowableOfType(() -> Instant.from(date), IllegalArgumentException.class); // WithIssue } @org.junit.jupiter.api.Test @@ -176,6 +177,7 @@ void methodInvocationsWithoutSemantics() { assertThrows(DateTimeException.class, () -> LocalDate.from(instant)); // NoIssue assertThatThrownBy(() -> Instant.from(date)).isInstanceOf(DateTimeException.class); // NoIssue assertThatExceptionOfType(DateTimeException.class).isThrownBy(() -> ZonedDateTime.from(dateTime)); // NoIssue + assertThatExceptionOfType(IllegalArgumentException.class).isThrownBy(() -> ZonedDateTime.from(dateTime)); // WithIssue assertThatIllegalArgumentException().isThrownBy(() -> Instant.from(date)); // WithIssue thenRuntimeException().isThrownBy(() -> Instant.from(date)); // NoIssue } @@ -191,8 +193,9 @@ void wrongAnnotationAttribute() { } @Test - void wrongAssertThrowsMethodSignature() { + void wrongAssertMethodSignature() { assertThrows(Instant.from(date)); // WithIssue + catchThrowableOfType(Instant.from(date)); // WithIssue } } From 2b7cde7fd1c3fca7af3cd786a2856d6558f54cd6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Aur=C3=A9lien=20Coet?= Date: Tue, 26 May 2026 10:24:20 +0200 Subject: [PATCH 7/9] Rename the InsantConversionsCheck class to DateTimeConversionsCheck --- ...Sample.java => DateTimeConversionsCheckSample.java} | 2 +- ...ersionsCheck.java => DateTimeConversionsCheck.java} | 2 +- .../sonar/java/filters/ExpectedExceptionFilter.java | 10 +++++----- ...heckTest.java => DateTimeConversionsCheckTest.java} | 6 +++--- 4 files changed, 10 insertions(+), 10 deletions(-) rename java-checks-test-sources/default/src/main/java/checks/{InstantConversionsCheckSample.java => DateTimeConversionsCheckSample.java} (99%) rename java-checks/src/main/java/org/sonar/java/checks/{InstantConversionsCheck.java => DateTimeConversionsCheck.java} (97%) rename java-checks/src/test/java/org/sonar/java/checks/{InstantConversionsCheckTest.java => DateTimeConversionsCheckTest.java} (84%) diff --git a/java-checks-test-sources/default/src/main/java/checks/InstantConversionsCheckSample.java b/java-checks-test-sources/default/src/main/java/checks/DateTimeConversionsCheckSample.java similarity index 99% rename from java-checks-test-sources/default/src/main/java/checks/InstantConversionsCheckSample.java rename to java-checks-test-sources/default/src/main/java/checks/DateTimeConversionsCheckSample.java index 175c2100106..d4b6dcc19b0 100644 --- a/java-checks-test-sources/default/src/main/java/checks/InstantConversionsCheckSample.java +++ b/java-checks-test-sources/default/src/main/java/checks/DateTimeConversionsCheckSample.java @@ -16,7 +16,7 @@ import java.time.ZonedDateTime; import java.time.temporal.TemporalAccessor; -public class InstantConversionsCheckSample { +public class DateTimeConversionsCheckSample { private Instant instant; private TemporalAccessor temporalAccessor; diff --git a/java-checks/src/main/java/org/sonar/java/checks/InstantConversionsCheck.java b/java-checks/src/main/java/org/sonar/java/checks/DateTimeConversionsCheck.java similarity index 97% rename from java-checks/src/main/java/org/sonar/java/checks/InstantConversionsCheck.java rename to java-checks/src/main/java/org/sonar/java/checks/DateTimeConversionsCheck.java index 5836a8db5ac..f7fee6daeee 100644 --- a/java-checks/src/main/java/org/sonar/java/checks/InstantConversionsCheck.java +++ b/java-checks/src/main/java/org/sonar/java/checks/DateTimeConversionsCheck.java @@ -29,7 +29,7 @@ import org.sonar.plugins.java.api.tree.TypeCastTree; @Rule(key = "S8220") -public class InstantConversionsCheck extends AbstractMethodDetection implements JavaVersionAwareVisitor { +public class DateTimeConversionsCheck extends AbstractMethodDetection implements JavaVersionAwareVisitor { private static final String INSTANT = "java.time.Instant"; private static final String TEMPORAL_ACCESSOR = "java.time.temporal.TemporalAccessor"; diff --git a/java-checks/src/main/java/org/sonar/java/filters/ExpectedExceptionFilter.java b/java-checks/src/main/java/org/sonar/java/filters/ExpectedExceptionFilter.java index c517133b539..544f2ec79f8 100644 --- a/java-checks/src/main/java/org/sonar/java/filters/ExpectedExceptionFilter.java +++ b/java-checks/src/main/java/org/sonar/java/filters/ExpectedExceptionFilter.java @@ -19,7 +19,7 @@ import java.util.List; import java.util.Optional; import java.util.Set; -import org.sonar.java.checks.InstantConversionsCheck; +import org.sonar.java.checks.DateTimeConversionsCheck; import org.sonar.java.model.ExpressionUtils; import org.sonar.plugins.java.api.JavaCheck; import org.sonar.plugins.java.api.semantic.Type; @@ -88,13 +88,13 @@ public class ExpectedExceptionFilter extends BaseTreeVisitorIssueFilter { @Override public Set> filteredRules() { - return Set.of(InstantConversionsCheck.class); + return Set.of(DateTimeConversionsCheck.class); } @Override public void visitMethod(MethodTree tree) { if (containsExpectedExceptions(tree.modifiers().annotations(), DATE_TIME_EXCEPTION_TYPES)) { - excludeLines(tree, InstantConversionsCheck.class); + excludeLines(tree, DateTimeConversionsCheck.class); } super.visitMethod(tree); } @@ -102,14 +102,14 @@ public void visitMethod(MethodTree tree) { @Override public void visitTryStatement(TryStatementTree tree) { if (catchesExpectedException(tree, DATE_TIME_EXCEPTION_TYPES)) { - excludeLines(tree.block(), InstantConversionsCheck.class); + excludeLines(tree.block(), DateTimeConversionsCheck.class); } super.visitTryStatement(tree); } @Override public void visitMethodInvocation(MethodInvocationTree tree) { - excludeExpectedExceptions(tree, DATE_TIME_EXCEPTION_TYPES, InstantConversionsCheck.class); + excludeExpectedExceptions(tree, DATE_TIME_EXCEPTION_TYPES, DateTimeConversionsCheck.class); super.visitMethodInvocation(tree); } diff --git a/java-checks/src/test/java/org/sonar/java/checks/InstantConversionsCheckTest.java b/java-checks/src/test/java/org/sonar/java/checks/DateTimeConversionsCheckTest.java similarity index 84% rename from java-checks/src/test/java/org/sonar/java/checks/InstantConversionsCheckTest.java rename to java-checks/src/test/java/org/sonar/java/checks/DateTimeConversionsCheckTest.java index e7ecfdb8e60..2b99e62c0e6 100644 --- a/java-checks/src/test/java/org/sonar/java/checks/InstantConversionsCheckTest.java +++ b/java-checks/src/test/java/org/sonar/java/checks/DateTimeConversionsCheckTest.java @@ -21,13 +21,13 @@ import static org.sonar.java.checks.verifier.TestUtils.mainCodeSourcesPath; -class InstantConversionsCheckTest { +class DateTimeConversionsCheckTest { @Test void test() { CheckVerifier.newVerifier() - .onFile(mainCodeSourcesPath("checks/InstantConversionsCheckSample.java")) - .withCheck(new InstantConversionsCheck()) + .onFile(mainCodeSourcesPath("checks/DateTimeConversionsCheckSample.java")) + .withCheck(new DateTimeConversionsCheck()) .verifyIssues(); } } From 08566ed1fb4c32521e5369c5bcef2d5d460894e7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Aur=C3=A9lien=20Coet?= Date: Tue, 26 May 2026 13:40:40 +0200 Subject: [PATCH 8/9] Fix the filter when no semantics are available --- .../sonar/java/filters/ExpectedExceptionFilter.java | 11 ++++------- .../test/files/filters/ExpectedExceptionFilter.java | 2 +- .../java/filters/ExpectedExceptionFilterTest.java | 7 +++++++ 3 files changed, 12 insertions(+), 8 deletions(-) diff --git a/java-checks/src/main/java/org/sonar/java/filters/ExpectedExceptionFilter.java b/java-checks/src/main/java/org/sonar/java/filters/ExpectedExceptionFilter.java index 544f2ec79f8..1b201c83178 100644 --- a/java-checks/src/main/java/org/sonar/java/filters/ExpectedExceptionFilter.java +++ b/java-checks/src/main/java/org/sonar/java/filters/ExpectedExceptionFilter.java @@ -161,11 +161,9 @@ private void excludeIsThrownBy(MethodInvocationTree tree, Class annotations, Set expectedExceptions) { return annotations.stream() - .anyMatch(annotation -> { - Type annotationType = annotation.annotationType().symbolType(); - Arguments arguments = annotation.arguments(); - return "Test".equals(annotationType.name()) && containsExpectedExceptions(arguments, expectedExceptions); - }); + .anyMatch(annotation -> + "Test".equals(annotation.symbolType().name()) && containsExpectedExceptions(annotation.arguments(), expectedExceptions) + ); } /** @@ -220,8 +218,7 @@ private static Optional classLiteralType(ExpressionTree expression) { * Check if a try statement catches a set of expected exception types. */ private static boolean catchesExpectedException(TryStatementTree tryStatement, Set expectedExceptions) { - return isTryCatchFail(tryStatement.block()) && - tryStatement.catches().stream() + return tryStatement.catches().stream() .anyMatch(catchTree -> catchesExpectedExceptions(catchTree, expectedExceptions)); } diff --git a/java-checks/src/test/files/filters/ExpectedExceptionFilter.java b/java-checks/src/test/files/filters/ExpectedExceptionFilter.java index 471ce83c334..5fce44a726d 100644 --- a/java-checks/src/test/files/filters/ExpectedExceptionFilter.java +++ b/java-checks/src/test/files/filters/ExpectedExceptionFilter.java @@ -154,7 +154,7 @@ void tryCatchFail() { } try { - LocalDate.from(instant); // WithIssue + LocalDate.from(instant); // NoIssue } catch (DateTimeException e) { // expected } diff --git a/java-checks/src/test/java/org/sonar/java/filters/ExpectedExceptionFilterTest.java b/java-checks/src/test/java/org/sonar/java/filters/ExpectedExceptionFilterTest.java index e6c18c8fc06..4d2e67a007f 100644 --- a/java-checks/src/test/java/org/sonar/java/filters/ExpectedExceptionFilterTest.java +++ b/java-checks/src/test/java/org/sonar/java/filters/ExpectedExceptionFilterTest.java @@ -26,4 +26,11 @@ void test() { .verify("src/test/files/filters/ExpectedExceptionFilter.java", new ExpectedExceptionFilter()); } + @Test + void test_without_semantics() { + FilterVerifier.newInstance() + .withoutSemantic() + .verify("src/test/files/filters/ExpectedExceptionFilter.java", new ExpectedExceptionFilter()); + } + } From 02ab3f74136a589212c93dfa6a05f48be83b7acb Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Aur=C3=A9lien=20Coet?= Date: Wed, 27 May 2026 09:11:20 +0200 Subject: [PATCH 9/9] Apply review suggestions --- .../sonar/java/filters/ExpectedExceptionFilter.java | 10 +++------- 1 file changed, 3 insertions(+), 7 deletions(-) diff --git a/java-checks/src/main/java/org/sonar/java/filters/ExpectedExceptionFilter.java b/java-checks/src/main/java/org/sonar/java/filters/ExpectedExceptionFilter.java index 1b201c83178..5242e85d48e 100644 --- a/java-checks/src/main/java/org/sonar/java/filters/ExpectedExceptionFilter.java +++ b/java-checks/src/main/java/org/sonar/java/filters/ExpectedExceptionFilter.java @@ -38,7 +38,6 @@ import org.sonar.plugins.java.api.tree.UnionTypeTree; import static org.sonar.java.checks.helpers.MethodTreeUtils.consecutiveMethodInvocation; -import static org.sonar.java.checks.helpers.UnitTestUtils.isTryCatchFail; /** * A filter to deactivate rules that catch expressions raising specific exceptions in contexts where these exceptions are expected. @@ -205,11 +204,8 @@ private static ExpressionTree annotationValue(ExpressionTree expression) { * Extract the type name of a class literal expression. */ private static Optional classLiteralType(ExpressionTree expression) { - if (expression.is(Tree.Kind.MEMBER_SELECT)) { - MemberSelectExpressionTree memberSelect = (MemberSelectExpressionTree) expression; - if ("class".equals(memberSelect.identifier().name())) { - return Optional.of(memberSelect.expression().symbolType()); - } + if (expression instanceof MemberSelectExpressionTree memberSelect && "class".equals(memberSelect.identifier().name())) { + return Optional.of(memberSelect.expression().symbolType()); } return Optional.empty(); } @@ -219,7 +215,7 @@ private static Optional classLiteralType(ExpressionTree expression) { */ private static boolean catchesExpectedException(TryStatementTree tryStatement, Set expectedExceptions) { return tryStatement.catches().stream() - .anyMatch(catchTree -> catchesExpectedExceptions(catchTree, expectedExceptions)); + .anyMatch(catchTree -> catchesExpectedExceptions(catchTree, expectedExceptions)); } private static boolean catchesExpectedExceptions(CatchTree catchTree, Set expectedExceptions) {