From 0705b0f46d8bc8b2c9e1cf8feae1c8b282880ac5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Aur=C3=A9lien=20Coet?= Date: Wed, 28 Jan 2026 15:35:23 +0100 Subject: [PATCH 1/6] Add support for Jakarta annotations in S5128 --- .../checks/MissingBeanValidationCheck.java | 13 +++++----- .../checks/MissingBeanValidationCheck.java | 25 +++++++++++++++++++ 2 files changed, 32 insertions(+), 6 deletions(-) diff --git a/java-checks/src/main/java/org/sonar/java/checks/MissingBeanValidationCheck.java b/java-checks/src/main/java/org/sonar/java/checks/MissingBeanValidationCheck.java index 873cca4b8c1..fe9c8ba0726 100644 --- a/java-checks/src/main/java/org/sonar/java/checks/MissingBeanValidationCheck.java +++ b/java-checks/src/main/java/org/sonar/java/checks/MissingBeanValidationCheck.java @@ -36,8 +36,9 @@ @Rule(key = "S5128") public class MissingBeanValidationCheck extends IssuableSubscriptionVisitor { - private static final String JAVAX_VALIDATION_VALID = "javax.validation.Valid"; - private static final String JAVAX_VALIDATION_CONSTRAINT = "javax.validation.Constraint"; + private static final List JSR380_VALID_ANNOTATIONS = List.of("javax.validation.Valid", "jakarta.validation.Valid"); + private static final List JSR380_CONSTRAINTS = List.of("javax.validation.Constraint", "jakarta.validation.Constraint"); + private static final List JSR380_CONSTRAINT_VALIDATORS = List.of("javax.validation.ConstraintValidator", "jakarta.validation.ConstraintValidator"); @Override public List nodesToVisit() { @@ -74,7 +75,7 @@ private static boolean isExcluded(MethodTree methodTree) { private static boolean isInConstraintValidator(MethodTree methodTree) { Symbol.TypeSymbol enclosingClass = methodTree.symbol().enclosingClass(); - return enclosingClass != null && enclosingClass.type().isSubtypeOf("javax.validation.ConstraintValidator"); + return enclosingClass != null && JSR380_CONSTRAINT_VALIDATORS.stream().anyMatch(type -> enclosingClass.type().isSubtypeOf(type)); } private static Optional getIssueMessage(VariableTree variable) { @@ -85,10 +86,10 @@ private static Optional getIssueMessage(VariableTree variable) { } private static boolean validationEnabled(VariableTree variable) { - if (variable.symbol().metadata().isAnnotatedWith(JAVAX_VALIDATION_VALID)) { + if (JSR380_VALID_ANNOTATIONS.stream().anyMatch(value -> variable.symbol().metadata().isAnnotatedWith(value))) { return true; } - return typeArgumentAnnotations(variable).anyMatch(annotation -> annotation.is(JAVAX_VALIDATION_VALID)); + return typeArgumentAnnotations(variable).anyMatch(annotation -> JSR380_VALID_ANNOTATIONS.contains(annotation.fullyQualifiedName())); } private static Stream typeArgumentAnnotations(VariableTree variable) { @@ -132,6 +133,6 @@ private static Stream fieldAnnotationInstance } private static boolean isConstraintAnnotation(SymbolMetadata.AnnotationInstance annotationInstance) { - return annotationInstance.symbol().metadata().isAnnotatedWith(JAVAX_VALIDATION_CONSTRAINT); + return JSR380_CONSTRAINTS.stream().anyMatch(constraint -> annotationInstance.symbol().metadata().isAnnotatedWith(constraint)); } } diff --git a/java-checks/src/test/files/checks/MissingBeanValidationCheck.java b/java-checks/src/test/files/checks/MissingBeanValidationCheck.java index d33fb1ed752..c25e5c2701f 100644 --- a/java-checks/src/test/files/checks/MissingBeanValidationCheck.java +++ b/java-checks/src/test/files/checks/MissingBeanValidationCheck.java @@ -106,3 +106,28 @@ class Building { static class Size { } } +public class Occupant { + @jakarta.validation.constraints.NotNull + private String name; +} + +public class Floor { + @jakarta.validation.constraints.NotNull + private List occupants; // Noncompliant {{Add missing "@Valid" on "occupants" to validate it with "Bean Validation".}} +// ^^^^^^^^^^^^^^ + + @jakarta.validation.Valid + @jakarta.validation.constraints.NotNull + private List validatedOccupants; // Compliant + + @jakarta.validation.constraints.NotNull + // Preferred style as of Bean Validation 2.0 + private List<@jakarta.validation.Valid User> validatedOccupants2; // Compliant + + public void ignore(Occupant occupant) { // Noncompliant {{Add missing "@Valid" on "occupant" to validate it with "Bean Validation".}} +// ^^^^^^^^ + } + + public void validate(@jakarta.validation.Valid Occupant occupant) { // Compliant + } +} From af6743b6117d4b587321e8e1c46472907dba7cc0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Aur=C3=A9lien=20Coet?= Date: Thu, 29 Jan 2026 13:34:37 +0100 Subject: [PATCH 2/6] Apply fixes suggested in the review --- .../checks/MissingBeanValidationCheck.java | 30 ++++++++++++------- 1 file changed, 19 insertions(+), 11 deletions(-) diff --git a/java-checks/src/main/java/org/sonar/java/checks/MissingBeanValidationCheck.java b/java-checks/src/main/java/org/sonar/java/checks/MissingBeanValidationCheck.java index fe9c8ba0726..a466996bdf9 100644 --- a/java-checks/src/main/java/org/sonar/java/checks/MissingBeanValidationCheck.java +++ b/java-checks/src/main/java/org/sonar/java/checks/MissingBeanValidationCheck.java @@ -20,6 +20,8 @@ import java.util.Collections; import java.util.List; import java.util.Optional; +import java.util.Set; +import java.util.stream.Collectors; import java.util.stream.Stream; import org.sonar.check.Rule; import org.sonar.plugins.java.api.IssuableSubscriptionVisitor; @@ -36,9 +38,9 @@ @Rule(key = "S5128") public class MissingBeanValidationCheck extends IssuableSubscriptionVisitor { - private static final List JSR380_VALID_ANNOTATIONS = List.of("javax.validation.Valid", "jakarta.validation.Valid"); - private static final List JSR380_CONSTRAINTS = List.of("javax.validation.Constraint", "jakarta.validation.Constraint"); - private static final List JSR380_CONSTRAINT_VALIDATORS = List.of("javax.validation.ConstraintValidator", "jakarta.validation.ConstraintValidator"); + private static final Set JSR380_VALID_ANNOTATIONS = Set.of("javax.validation.Valid", "jakarta.validation.Valid"); + private static final Set JSR380_CONSTRAINTS = Set.of("javax.validation.Constraint", "jakarta.validation.Constraint"); + private static final Set JSR380_CONSTRAINT_VALIDATORS = Set.of("javax.validation.ConstraintValidator", "jakarta.validation.ConstraintValidator"); @Override public List nodesToVisit() { @@ -75,7 +77,7 @@ private static boolean isExcluded(MethodTree methodTree) { private static boolean isInConstraintValidator(MethodTree methodTree) { Symbol.TypeSymbol enclosingClass = methodTree.symbol().enclosingClass(); - return enclosingClass != null && JSR380_CONSTRAINT_VALIDATORS.stream().anyMatch(type -> enclosingClass.type().isSubtypeOf(type)); + return enclosingClass != null && JSR380_CONSTRAINT_VALIDATORS.stream().anyMatch(enclosingClass.type()::isSubtypeOf); } private static Optional getIssueMessage(VariableTree variable) { @@ -86,14 +88,16 @@ private static Optional getIssueMessage(VariableTree variable) { } private static boolean validationEnabled(VariableTree variable) { - if (JSR380_VALID_ANNOTATIONS.stream().anyMatch(value -> variable.symbol().metadata().isAnnotatedWith(value))) { - return true; - } - return typeArgumentAnnotations(variable).anyMatch(annotation -> JSR380_VALID_ANNOTATIONS.contains(annotation.fullyQualifiedName())); + return isAnnotatedWithAny(variable.symbol().metadata(), JSR380_VALID_ANNOTATIONS) || + !Collections.disjoint(typeArgumentAnnotations(variable), JSR380_VALID_ANNOTATIONS); } - private static Stream typeArgumentAnnotations(VariableTree variable) { - return typeArgumentTypeTrees(variable).flatMap(type -> type.annotations().stream()).map(ExpressionTree::symbolType); + private static Set typeArgumentAnnotations(VariableTree variable) { + return typeArgumentTypeTrees(variable) + .flatMap(type -> type.annotations().stream()) + .map(ExpressionTree::symbolType) + .map(Type::fullyQualifiedName) + .collect(Collectors.toSet()); } private static Stream typeArgumentTypeTrees(VariableTree variable) { @@ -133,6 +137,10 @@ private static Stream fieldAnnotationInstance } private static boolean isConstraintAnnotation(SymbolMetadata.AnnotationInstance annotationInstance) { - return JSR380_CONSTRAINTS.stream().anyMatch(constraint -> annotationInstance.symbol().metadata().isAnnotatedWith(constraint)); + return isAnnotatedWithAny(annotationInstance.symbol().metadata(), JSR380_CONSTRAINTS); + } + + private static boolean isAnnotatedWithAny(SymbolMetadata metadata, Set annotations) { + return annotations.stream().anyMatch(metadata::isAnnotatedWith); } } From 73fbbf3fe4b7ba16da18a9759557e2c953cf1e1d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Aur=C3=A9lien=20Coet?= Date: Thu, 29 Jan 2026 14:44:18 +0100 Subject: [PATCH 3/6] Move and split the tests between Javax and Jakarta for S5128 --- ...ssingBeanValidationCheckJakartaSample.java | 49 +++++++++++++++++++ ...MissingBeanValidationCheckJavaxSample.java | 41 +++------------- .../MissingBeanValidationCheckTest.java | 21 ++++++-- 3 files changed, 75 insertions(+), 36 deletions(-) create mode 100644 java-checks-test-sources/default/src/main/java/checks/MissingBeanValidationCheckJakartaSample.java rename java-checks/src/test/files/checks/MissingBeanValidationCheck.java => java-checks-test-sources/default/src/main/java/checks/MissingBeanValidationCheckJavaxSample.java (66%) diff --git a/java-checks-test-sources/default/src/main/java/checks/MissingBeanValidationCheckJakartaSample.java b/java-checks-test-sources/default/src/main/java/checks/MissingBeanValidationCheckJakartaSample.java new file mode 100644 index 00000000000..cc47611e0e4 --- /dev/null +++ b/java-checks-test-sources/default/src/main/java/checks/MissingBeanValidationCheckJakartaSample.java @@ -0,0 +1,49 @@ +package checks; + +import jakarta.validation.Constraint; +import jakarta.validation.ConstraintValidator; +import jakarta.validation.ConstraintValidatorContext; +import jakarta.validation.Valid; +import java.util.List; + +@Constraint(validatedBy = ConstraintChecker.class) +@interface WellNamed { +} + +class ConstraintChecker implements ConstraintValidator { + @Override + public boolean isValid(Occupant occupant, ConstraintValidatorContext constraintValidatorContext) { + return occupant.name != null; + } +} + +@WellNamed +class Occupant { + public String name; +} + +class Floor { + private Id id; // Noncompliant {{Add missing "@Valid" on "id" to validate it with "Bean Validation".}} +// ^^ + + private List occupants; // Noncompliant {{Add missing "@Valid" on "occupants" to validate it with "Bean Validation".}} +// ^^^^^^^^^^^^^^ + + @Valid + private List validatedOccupants; // Compliant + + // Preferred style as of Bean Validation 2.0 + private List<@Valid Occupant> validatedOccupants2; // Compliant + + public void ignore(Occupant occupant) { // Noncompliant {{Add missing "@Valid" on "occupant" to validate it with "Bean Validation".}} +// ^^^^^^^^ + } + + public void validate(@Valid Occupant occupant) { // Compliant + } +} + +class Id { + @jakarta.validation.constraints.NotNull + private String name; +} diff --git a/java-checks/src/test/files/checks/MissingBeanValidationCheck.java b/java-checks-test-sources/default/src/main/java/checks/MissingBeanValidationCheckJavaxSample.java similarity index 66% rename from java-checks/src/test/files/checks/MissingBeanValidationCheck.java rename to java-checks-test-sources/default/src/main/java/checks/MissingBeanValidationCheckJavaxSample.java index c25e5c2701f..efde1da654a 100644 --- a/java-checks/src/test/files/checks/MissingBeanValidationCheck.java +++ b/java-checks-test-sources/default/src/main/java/checks/MissingBeanValidationCheckJavaxSample.java @@ -1,3 +1,5 @@ +package checks; + import java.util.List; import javax.validation.Constraint; import javax.validation.ConstraintValidator; @@ -10,7 +12,7 @@ class User { private String name; } -@Constraint +@Constraint(validatedBy = CheckMyConstraint.class) @interface MyConstraint { } @@ -19,12 +21,12 @@ class Department { private int hierarchyLevel; } -public class CheckMyConstraint implements ConstraintValidator -{ +class CheckMyConstraint implements ConstraintValidator { public void initialize(MyConstraint constraintAnnotation) { } public boolean isValid(Department bean, ConstraintValidatorContext context) { // Compliant - @Valid inside constraint validators would be nonsense + return true; } } @@ -52,16 +54,13 @@ class CompliantGroup { private List members; // Compliant } -class CompliantUserSelection { - private List<@Valid User> contents; // Compliant; preferred syntax as of Java 8 -} - class NonCompliantService { public void login(User user) { // Noncompliant {{Add missing "@Valid" on "user" to validate it with "Bean Validation".}} } public List list(Department department) { // Noncompliant {{Add missing "@Valid" on "department" to validate it with "Bean Validation".}} // ^^^^^^^^^^ + return List.of(); } } @@ -75,6 +74,7 @@ private void updateLastLoginDate(User user) { // Compliant - Bean Validation doe } public List list(@Valid Department department) { // Compliant + return List.of(); } } @@ -103,31 +103,6 @@ public void eat(Orange orange) { // Compliant } class Building { - static class Size { } -} - -public class Occupant { - @jakarta.validation.constraints.NotNull - private String name; -} - -public class Floor { - @jakarta.validation.constraints.NotNull - private List occupants; // Noncompliant {{Add missing "@Valid" on "occupants" to validate it with "Bean Validation".}} -// ^^^^^^^^^^^^^^ - - @jakarta.validation.Valid - @jakarta.validation.constraints.NotNull - private List validatedOccupants; // Compliant - - @jakarta.validation.constraints.NotNull - // Preferred style as of Bean Validation 2.0 - private List<@jakarta.validation.Valid User> validatedOccupants2; // Compliant - - public void ignore(Occupant occupant) { // Noncompliant {{Add missing "@Valid" on "occupant" to validate it with "Bean Validation".}} -// ^^^^^^^^ - } - - public void validate(@jakarta.validation.Valid Occupant occupant) { // Compliant + static class Size { } } diff --git a/java-checks/src/test/java/org/sonar/java/checks/MissingBeanValidationCheckTest.java b/java-checks/src/test/java/org/sonar/java/checks/MissingBeanValidationCheckTest.java index fb746ffa605..b00b3df02cc 100644 --- a/java-checks/src/test/java/org/sonar/java/checks/MissingBeanValidationCheckTest.java +++ b/java-checks/src/test/java/org/sonar/java/checks/MissingBeanValidationCheckTest.java @@ -18,19 +18,34 @@ import org.junit.jupiter.api.Test; import org.sonar.java.checks.verifier.CheckVerifier; +import org.sonar.java.checks.verifier.TestUtils; class MissingBeanValidationCheckTest { @Test - void test() { + void testWithJavax() { CheckVerifier.newVerifier() - .onFile("src/test/files/checks/MissingBeanValidationCheck.java") + .onFile(TestUtils.mainCodeSourcesPath("checks/MissingBeanValidationCheckJavaxSample.java")) .withCheck(new MissingBeanValidationCheck()) .verifyIssues(); CheckVerifier.newVerifier() - .onFile("src/test/files/checks/MissingBeanValidationCheck.java") + .onFile(TestUtils.mainCodeSourcesPath("checks/MissingBeanValidationCheckJavaxSample.java")) .withCheck(new MissingBeanValidationCheck()) .withoutSemantic() .verifyNoIssues(); } + + @Test + void testWithJakarta() { + CheckVerifier.newVerifier() + .onFile(TestUtils.mainCodeSourcesPath("checks/MissingBeanValidationCheckJakartaSample.java")) + .withCheck(new MissingBeanValidationCheck()) + .verifyIssues(); + CheckVerifier.newVerifier() + .onFile(TestUtils.mainCodeSourcesPath("checks/MissingBeanValidationCheckJakartaSample.java")) + .withCheck(new MissingBeanValidationCheck()) + .withoutSemantic() + .verifyNoIssues(); + } + } From 5c8c737811cd91e3d28f78056f05bf363dacc15e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Aur=C3=A9lien=20Coet?= Date: Thu, 29 Jan 2026 15:31:55 +0100 Subject: [PATCH 4/6] Update the ground truth for autoscan tests after moving S5128's unit tests --- .../src/test/resources/autoscan/diffs/diff_S1172.json | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/its/autoscan/src/test/resources/autoscan/diffs/diff_S1172.json b/its/autoscan/src/test/resources/autoscan/diffs/diff_S1172.json index eeb1f57dc1e..322daa80d40 100644 --- a/its/autoscan/src/test/resources/autoscan/diffs/diff_S1172.json +++ b/its/autoscan/src/test/resources/autoscan/diffs/diff_S1172.json @@ -1,6 +1,6 @@ { "ruleKey": "S1172", "hasTruePositives": true, - "falseNegatives": 31, + "falseNegatives": 32, "falsePositives": 0 -} \ No newline at end of file +} From 8d160d3972a98aa9fd191dbed9d500ea607350d1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Aur=C3=A9lien=20Coet?= Date: Fri, 30 Jan 2026 08:11:16 +0100 Subject: [PATCH 5/6] Use a static import for the path resolution method in tests --- .../java/checks/MissingBeanValidationCheckTest.java | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/java-checks/src/test/java/org/sonar/java/checks/MissingBeanValidationCheckTest.java b/java-checks/src/test/java/org/sonar/java/checks/MissingBeanValidationCheckTest.java index b00b3df02cc..e8dd3eaa144 100644 --- a/java-checks/src/test/java/org/sonar/java/checks/MissingBeanValidationCheckTest.java +++ b/java-checks/src/test/java/org/sonar/java/checks/MissingBeanValidationCheckTest.java @@ -18,18 +18,19 @@ import org.junit.jupiter.api.Test; import org.sonar.java.checks.verifier.CheckVerifier; -import org.sonar.java.checks.verifier.TestUtils; + +import static org.sonar.java.checks.verifier.TestUtils.mainCodeSourcesPath; class MissingBeanValidationCheckTest { @Test void testWithJavax() { CheckVerifier.newVerifier() - .onFile(TestUtils.mainCodeSourcesPath("checks/MissingBeanValidationCheckJavaxSample.java")) + .onFile(mainCodeSourcesPath("checks/MissingBeanValidationCheckJavaxSample.java")) .withCheck(new MissingBeanValidationCheck()) .verifyIssues(); CheckVerifier.newVerifier() - .onFile(TestUtils.mainCodeSourcesPath("checks/MissingBeanValidationCheckJavaxSample.java")) + .onFile(mainCodeSourcesPath("checks/MissingBeanValidationCheckJavaxSample.java")) .withCheck(new MissingBeanValidationCheck()) .withoutSemantic() .verifyNoIssues(); @@ -38,11 +39,11 @@ void testWithJavax() { @Test void testWithJakarta() { CheckVerifier.newVerifier() - .onFile(TestUtils.mainCodeSourcesPath("checks/MissingBeanValidationCheckJakartaSample.java")) + .onFile(mainCodeSourcesPath("checks/MissingBeanValidationCheckJakartaSample.java")) .withCheck(new MissingBeanValidationCheck()) .verifyIssues(); CheckVerifier.newVerifier() - .onFile(TestUtils.mainCodeSourcesPath("checks/MissingBeanValidationCheckJakartaSample.java")) + .onFile(mainCodeSourcesPath("checks/MissingBeanValidationCheckJakartaSample.java")) .withCheck(new MissingBeanValidationCheck()) .withoutSemantic() .verifyNoIssues(); From fff7db20236ca22490a785ce94c307a944a7eb89 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Aur=C3=A9lien=20Coet?= Date: Fri, 30 Jan 2026 10:01:24 +0100 Subject: [PATCH 6/6] Update rule metadata --- .../resources/org/sonar/l10n/java/rules/java/S5128.html | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/sonar-java-plugin/src/main/resources/org/sonar/l10n/java/rules/java/S5128.html b/sonar-java-plugin/src/main/resources/org/sonar/l10n/java/rules/java/S5128.html index c215e0ff8ce..378bb8b6de3 100644 --- a/sonar-java-plugin/src/main/resources/org/sonar/l10n/java/rules/java/S5128.html +++ b/sonar-java-plugin/src/main/resources/org/sonar/l10n/java/rules/java/S5128.html @@ -1,11 +1,11 @@

Why is this an issue?

-

Bean Validation as per defined by JSR 380 can be triggered programmatically or also executed by the Bean Validation +

Bean Validation as defined by JSR 380 can be triggered programmatically or also executed by the Bean Validation providers. However something should tell the Bean Validation provider that a variable must be validated otherwise no validation will -happen. This can be achieved by annotating a variable with javax.validation.Valid and unfortunally it’s easy to forget to add this -annotation on complex Beans.

+happen. This can be achieved by annotating a variable with javax.validation.Valid or jakarta.validation.Valid and is +unfortunately easy to forget on complex Beans.

Not annotating a variable with @Valid means Bean Validation will not be triggered for this variable, but readers may overlook this omission and assume the variable will be validated.

-

This rule will run by default on all Class'es and therefore can generate a lot of noise. This rule should be restricted to run only on +

This rule will run by default on all Class'es and can therefore generate a lot of noise. It should be restricted to run only on certain layers. For this reason, the "Restrict Scope of Coding Rules" feature should be used to check for missing @Valid annotations only on some packages of the application.

Noncompliant code example