Skip to content

SONARJAVA-5931 Upgrade S5128 to support the Jakarta package #5410

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
aurelien-coet-sonarsource merged 6 commits into from
Jan 30, 2026
Merged

SONARJAVA-5931 Upgrade S5128 to support the Jakarta package #5410

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
0705b0f
Add support for Jakarta annotations in S5128
aurelien-coet-sonarsource Jan 28, 2026
af6743b
Apply fixes suggested in the review
aurelien-coet-sonarsource Jan 29, 2026
73fbbf3
Move and split the tests between Javax and Jakarta for S5128
aurelien-coet-sonarsource Jan 29, 2026
5c8c737
Update the ground truth for autoscan tests after moving S5128's unit …
aurelien-coet-sonarsource Jan 29, 2026
8d160d3
Use a static import for the path resolution method in tests
aurelien-coet-sonarsource Jan 30, 2026
fff7db2
Update rule metadata
aurelien-coet-sonarsource Jan 30, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions its/autoscan/src/test/resources/autoscan/diffs/diff_S1172.json
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
{
"ruleKey": "S1172",
"hasTruePositives": true,
"falseNegatives": 31,
"falseNegatives": 32,
"falsePositives": 0
}
}
49 changes: 49 additions & 0 deletions ...ks-test-sources/default/src/main/java/checks/MissingBeanValidationCheckJakartaSample.java
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -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<WellNamed, Occupant> {
@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<Occupant> occupants; // Noncompliant {{Add missing "@Valid" on "occupants" to validate it with "Bean Validation".}}
// ^^^^^^^^^^^^^^

@Valid
private List<Occupant> 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;
}
18 changes: 9 additions & 9 deletions ...es/checks/MissingBeanValidationCheck.java → ...issingBeanValidationCheckJavaxSample.java
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
package checks;

import java.util.List;
import javax.validation.Constraint;
import javax.validation.ConstraintValidator;
Expand All @@ -10,7 +12,7 @@ class User {
private String name;
}

@Constraint
@Constraint(validatedBy = CheckMyConstraint.class)
@interface MyConstraint {
}

Expand All @@ -19,12 +21,12 @@ class Department {
private int hierarchyLevel;
}

public class CheckMyConstraint implements ConstraintValidator<MyConstraint, Department>
{
class CheckMyConstraint implements ConstraintValidator<MyConstraint, Department> {
public void initialize(MyConstraint constraintAnnotation) {
}

public boolean isValid(Department bean, ConstraintValidatorContext context) { // Compliant - @Valid inside constraint validators would be nonsense
return true;
}
}

Expand Down Expand Up @@ -52,16 +54,13 @@ class CompliantGroup {
private List<User> 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<User> list(Department department) { // Noncompliant {{Add missing "@Valid" on "department" to validate it with "Bean Validation".}}
// ^^^^^^^^^^
return List.of();
}
}

Expand All @@ -75,6 +74,7 @@ private void updateLastLoginDate(User user) { // Compliant - Bean Validation doe
}

public List<User> list(@Valid Department department) { // Compliant
return List.of();
}
}

Expand Down Expand Up @@ -103,6 +103,6 @@ public void eat(Orange orange) { // Compliant
}

class Building {
static class Size<T> { }
static class Size<T> {
}
}

29 changes: 19 additions & 10 deletions java-checks/src/main/java/org/sonar/java/checks/MissingBeanValidationCheck.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -36,8 +38,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 Set<String> JSR380_VALID_ANNOTATIONS = Set.of("javax.validation.Valid", "jakarta.validation.Valid");
private static final Set<String> JSR380_CONSTRAINTS = Set.of("javax.validation.Constraint", "jakarta.validation.Constraint");
private static final Set<String> JSR380_CONSTRAINT_VALIDATORS = Set.of("javax.validation.ConstraintValidator", "jakarta.validation.ConstraintValidator");

@Override
public List<Tree.Kind> nodesToVisit() {
Expand Down Expand Up @@ -74,7 +77,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(enclosingClass.type()::isSubtypeOf);
}

private static Optional<String> getIssueMessage(VariableTree variable) {
Expand All @@ -85,14 +88,16 @@ private static Optional<String> getIssueMessage(VariableTree variable) {
}

private static boolean validationEnabled(VariableTree variable) {
if (variable.symbol().metadata().isAnnotatedWith(JAVAX_VALIDATION_VALID)) {
return true;
}
return typeArgumentAnnotations(variable).anyMatch(annotation -> annotation.is(JAVAX_VALIDATION_VALID));
return isAnnotatedWithAny(variable.symbol().metadata(), JSR380_VALID_ANNOTATIONS) ||
!Collections.disjoint(typeArgumentAnnotations(variable), JSR380_VALID_ANNOTATIONS);
}

private static Stream<Type> typeArgumentAnnotations(VariableTree variable) {
return typeArgumentTypeTrees(variable).flatMap(type -> type.annotations().stream()).map(ExpressionTree::symbolType);
private static Set<String> typeArgumentAnnotations(VariableTree variable) {
return typeArgumentTypeTrees(variable)
.flatMap(type -> type.annotations().stream())
.map(ExpressionTree::symbolType)
.map(Type::fullyQualifiedName)
.collect(Collectors.toSet());
}

private static Stream<TypeTree> typeArgumentTypeTrees(VariableTree variable) {
Expand Down Expand Up @@ -132,6 +137,10 @@ private static Stream<SymbolMetadata.AnnotationInstance> fieldAnnotationInstance
}

private static boolean isConstraintAnnotation(SymbolMetadata.AnnotationInstance annotationInstance) {
return annotationInstance.symbol().metadata().isAnnotatedWith(JAVAX_VALIDATION_CONSTRAINT);
return isAnnotatedWithAny(annotationInstance.symbol().metadata(), JSR380_CONSTRAINTS);
}

private static boolean isAnnotatedWithAny(SymbolMetadata metadata, Set<String> annotations) {
return annotations.stream().anyMatch(metadata::isAnnotatedWith);
}
}
22 changes: 19 additions & 3 deletions java-checks/src/test/java/org/sonar/java/checks/MissingBeanValidationCheckTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,18 +19,34 @@
import org.junit.jupiter.api.Test;
import org.sonar.java.checks.verifier.CheckVerifier;

import static org.sonar.java.checks.verifier.TestUtils.mainCodeSourcesPath;

class MissingBeanValidationCheckTest {

@Test
void test() {
void testWithJavax() {
CheckVerifier.newVerifier()
.onFile(mainCodeSourcesPath("checks/MissingBeanValidationCheckJavaxSample.java"))
.withCheck(new MissingBeanValidationCheck())
.verifyIssues();
CheckVerifier.newVerifier()
.onFile(mainCodeSourcesPath("checks/MissingBeanValidationCheckJavaxSample.java"))
.withCheck(new MissingBeanValidationCheck())
.withoutSemantic()
.verifyNoIssues();
}

@Test
void testWithJakarta() {
CheckVerifier.newVerifier()
.onFile("src/test/files/checks/MissingBeanValidationCheck.java")
.onFile(mainCodeSourcesPath("checks/MissingBeanValidationCheckJakartaSample.java"))
.withCheck(new MissingBeanValidationCheck())
.verifyIssues();
CheckVerifier.newVerifier()
.onFile("src/test/files/checks/MissingBeanValidationCheck.java")
.onFile(mainCodeSourcesPath("checks/MissingBeanValidationCheckJakartaSample.java"))
.withCheck(new MissingBeanValidationCheck())
.withoutSemantic()
.verifyNoIssues();
}

}
8 changes: 4 additions & 4 deletions sonar-java-plugin/src/main/resources/org/sonar/l10n/java/rules/java/S5128.html
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,11 +1,11 @@
<h2>Why is this an issue?</h2>
<p><code>Bean Validation</code> as per defined by JSR 380 can be triggered programmatically or also executed by the <code>Bean Validation</code>
<p><code>Bean Validation</code> as defined by JSR 380 can be triggered programmatically or also executed by the <code>Bean Validation</code>
providers. However something should tell the <code>Bean Validation</code> provider that a variable must be validated otherwise no validation will
happen. This can be achieved by annotating a variable with <code>javax.validation.Valid</code> and unfortunally it’s easy to forget to add this
annotation on complex Beans.</p>
happen. This can be achieved by annotating a variable with <code>javax.validation.Valid</code> or <code>jakarta.validation.Valid</code> and is
unfortunately easy to forget on complex Beans.</p>
<p>Not annotating a variable with <code>@Valid</code> means <code>Bean Validation</code> will not be triggered for this variable, but readers may
overlook this omission and assume the variable will be validated.</p>
<p>This rule will run by default on all <code>Class</code>'es and therefore can generate a lot of noise. This rule should be restricted to run only on
<p>This rule will run by default on all <code>Class</code>'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 <code>@Valid</code> annotations only
on some packages of the application.</p>
<h3>Noncompliant code example</h3>
Expand Down
Loading