Skip to content

JAVASE-241 Treat Bean Validation @NotNull as WEAK_NULLABLE to fix FPs (JAVASE-241) #5991

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

Open
asya-vorobeva wants to merge 1 commit into
base: master
Choose a base branch
from
Open

JAVASE-241 Treat Bean Validation @NotNull as WEAK_NULLABLE to fix FPs (JAVASE-241) #5991

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
13 changes: 13 additions & 0 deletions ...es/default/src/main/java/annotations/nullability/no_default/NullabilityAtMethodLevel.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -71,6 +71,19 @@ public Object id2019_type_NO_ANNOTATION_level_PACKAGE(
return new Object();
}

// ============== Bean Validation @NotNull is treated as WEAK_NULLABLE, not NON_NULL ==============
@javax.validation.constraints.NotNull
public Object id2025_type_WEAK_NULLABLE_level_METHOD(
@javax.validation.constraints.NotNull Object id2026_type_WEAK_NULLABLE_level_VARIABLE) {
return new Object();
}

@jakarta.validation.constraints.NotNull
public Object id2027_type_WEAK_NULLABLE_level_METHOD(
@jakarta.validation.constraints.NotNull Object id2028_type_WEAK_NULLABLE_level_VARIABLE) {
return new Object();
}

}

abstract class NullabilityAtMethodLevelParent {
Expand Down
4 changes: 3 additions & 1 deletion .../default/src/main/java/annotations/nullability/no_default/NullabilityAtVariableLevel.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -105,7 +105,9 @@ public class NullabilityAtVariableLevel {
@javax.annotation.Nonnull
Object id1032_type_NON_NULL_level_VARIABLE;
@javax.validation.constraints.NotNull
Object id1033_type_NON_NULL_level_VARIABLE;
Object id1033_type_WEAK_NULLABLE_level_VARIABLE;
@jakarta.validation.constraints.NotNull
Object id1090_type_WEAK_NULLABLE_level_VARIABLE;
@lombok.NonNull
Object id1034_type_NON_NULL_level_VARIABLE;
@org.checkerframework.checker.nullness.compatqual.NonNullDecl
Expand Down
5 changes: 1 addition & 4 deletions ...s-test-sources/default/src/main/java/checks/EqualsParametersMarkedNonNullCheckSample.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -43,11 +43,8 @@ public boolean equals(@Nonnull C c) { // Compliant

static class F {
public boolean equals(
@javax.validation.constraints.NotNull // Noncompliant {{"equals" method parameters should not be marked "@NotNull".}} [[quickfixes=qf2]]
// ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
@javax.validation.constraints.NotNull // Compliant: exceptional annotation
java.lang.Object object) {
// fix@qf2 {{Remove "@NotNull"}}
// edit@qf2 [[sc=7;ec=7;el=+2]] {{}}
return false;
}
}
Expand Down
7 changes: 7 additions & 0 deletions ...checks-test-sources/default/src/main/java/checks/PrimitivesMarkedNullableCheckSample.java
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
package checks;

import jakarta.validation.constraints.NotNull;
import javax.annotation.CheckForNull;
import javax.annotation.Nonnull;
import javax.annotation.Nullable;
Expand Down Expand Up @@ -51,6 +52,12 @@ abstract class PrimitivesMarkedNullableCheckSample {
@Nonnull
public double getDouble2_2() { return 0.0; } // Compliant, Nonnull is useless, but is accepted as it can be added for consistency

@javax.validation.constraints.NotNull
public int getIntWithJavaxNotNull() { return 0; } // Compliant, Bean Validation @NotNull is a runtime constraint, not a nullable annotation

@NotNull
public int getIntWithJakartaNotNull() { return 0; } // Compliant, Bean Validation @NotNull is a runtime constraint, not a nullable annotation

@javax.annotation.Nullable
public Double getDouble3() { return 0.0; }

Expand Down
46 changes: 37 additions & 9 deletions .../java/checks/S2638_ChangeMethodContractCheck/noPackageInfo/ChangeMethodContractCheck.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -187,35 +187,63 @@ void argAnnotatedDirectlyNullable(@MyNonnullMetaAnnotation Object a) { } // Nonc
}

/**
* Not null with arguments is inconsistently supported. See SONARJAVA-3803.
* Javax and Jakarta validation NotNull annotations are treated as weakly nullable.
*/
class ChangeMethodContractCheck_NonnullWithArguments {
class ChangeMethodContractCheck_JavaxAndJakartaValidation {

class Parent {
@javax.validation.constraints.NotNull(groups = { ChangeMethodContractCheck.class })
String annotatedNotNullWithArg(Object a) { return "null"; }
String annotatedJavaxNotNullWithArg(Object a) { return "null"; }

@javax.validation.constraints.NotNull
String annotatedNotNullWithoutArg(Object a) { return "null"; }
String annotatedJavaxNotNullWithoutArg(Object a) { return "null"; }

@jakarta.validation.constraints.NotNull(groups = { ChangeMethodContractCheck.class })
String annotatedJakartaNotNullWithArg(Object a) { return "null"; }

@jakarta.validation.constraints.NotNull
String annotatedJakartaNotNullWithoutArg(Object a) { return "null"; }

void argAnnotatedNoNullWithArg(@javax.validation.constraints.NotNull(groups = { ChangeMethodContractCheck.class }) Object a) { }
void argAnnotatedNoNullWithoutArg(@javax.validation.constraints.NotNull Object a) { }
void argAnnotatedJavaxNotNull(@javax.validation.constraints.NotNull Object a) { }
void argAnnotatedJakartaNotNull(@jakarta.validation.constraints.NotNull Object a) { }

@javax.validation.constraints.NotNull
String methodNonnullJavaxBvReturn(Object a) { return ""; }
@jakarta.validation.constraints.NotNull
String methodNonnullJakartaBvReturn(Object a) { return ""; }
}

class Child extends Parent {
// Parent is not strictly not null (NotNull with arguments).
// Parent is weakly nullable.
@Override
@javax.annotation.CheckForNull
String annotatedJavaxNotNullWithArg(Object a) { return null; }

@Override
@javax.annotation.CheckForNull
String annotatedJavaxNotNullWithoutArg(Object a) { return null; }

@Override
@javax.annotation.CheckForNull
String annotatedNotNullWithArg(Object a) { return null; }
String annotatedJakartaNotNullWithArg(Object a) { return null; }

@Override
// This one is a TP though.
@javax.annotation.CheckForNull
String annotatedNotNullWithoutArg(Object a) { return null; } // Noncompliant {{Fix the incompatibility of the annotation @CheckForNull to honor @NotNull of the overridden method.}}
String annotatedJakartaNotNullWithoutArg(Object a) { return null; }

// It works correctly for arguments though.
// It works correctly also for arguments.
void argAnnotatedNoNullWithArg(@javax.annotation.CheckForNull Object a) { }
void argAnnotatedNoNullWithoutArg(@javax.annotation.CheckForNull Object a) { }
// Bean Validation @NotNull is a runtime constraint: strengthening to @Nonnull in child is not a contract violation.
void argAnnotatedJavaxNotNull(@javax.annotation.Nonnull Object a) { } // Compliant
void argAnnotatedJakartaNotNull(@javax.annotation.Nonnull Object a) { } // Compliant
// It works correctly also for return values: BV @NotNull to @Nonnull is strengthening.
@javax.annotation.Nonnull
String methodNonnullJavaxBvReturn(Object a) { return ""; } // Compliant
@javax.annotation.Nonnull
String methodNonnullJakartaBvReturn(Object a) { return ""; } // Compliant
}
}

Expand Down
2 changes: 1 addition & 1 deletion ...ces/default/src/main/java/checks/jspecify/RedundantNullabilityAnnotationsCheckSample.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ public void methodNonNullParam(@javax.annotation.Nonnull(when= When.ALWAYS) Obje
// ...
}

@NotNull // Noncompliant {{Remove redundant annotation @NotNull as inside scope annotation @NullMarked at class level.}}
@NotNull // Compliant
public Integer methodJXNonNullReturn(Object o) {
return 0;
}
Expand Down
25 changes: 22 additions & 3 deletions java-checks/src/main/java/org/sonar/java/checks/ChangeMethodContractCheck.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@
import java.util.Collections;
import java.util.List;
import java.util.Optional;
import java.util.Set;
import org.sonar.check.Rule;
import org.sonar.java.checks.helpers.MethodTreeUtils;
import org.sonar.java.model.JUtils;
Expand All @@ -34,12 +35,21 @@
import org.sonar.plugins.java.api.tree.TypeTree;
import org.sonar.plugins.java.api.tree.VariableTree;

import org.sonarsource.analyzer.commons.collections.SetUtils;

import static org.sonar.java.checks.helpers.NullabilityDataUtils.nullabilityAsString;
import static org.sonar.plugins.java.api.semantic.SymbolMetadata.NullabilityLevel.PACKAGE;

@Rule(key = "S2638")
public class ChangeMethodContractCheck extends IssuableSubscriptionVisitor {

// Bean Validation @NotNull is a runtime constraint, not a static nullability guarantee.
// When the parent parameter is annotated with it, strengthening it to @Nonnull in the child is not a contract violation.
private static final Set<String> BEAN_VALIDATION_ANNOTATIONS = SetUtils.immutableSetOf(
"javax.validation.constraints.NotNull",
"jakarta.validation.constraints.NotNull"
);

@Override
public List<Tree.Kind> nodesToVisit() {
return Collections.singletonList(Tree.Kind.METHOD);
Expand Down Expand Up @@ -83,9 +93,12 @@ private void checkContractChange(MethodTree methodTree, Symbol.MethodSymbol over

private void compareNullability(TypeTree tree, SymbolMetadata upperBound, SymbolMetadata lowerBound, boolean overriddenIsLowerBound) {
// Check current level
if (upperBound.nullabilityData().isNullable(PACKAGE, false, false)
&& lowerBound.nullabilityData().isNonNull(PACKAGE, false, false)) {
reportIssue(tree, lowerBound.nullabilityData(), upperBound.nullabilityData(), overriddenIsLowerBound);
NullabilityData upperData = upperBound.nullabilityData();
NullabilityData lowerData = lowerBound.nullabilityData();
if (!isBeanValidationAnnotation(upperData)
&& upperData.isNullable(PACKAGE, false, false)
&& lowerData.isNonNull(PACKAGE, false, false)) {
reportIssue(tree, lowerData, upperData, overriddenIsLowerBound);
}

// Check type parameters
Expand Down Expand Up @@ -114,6 +127,12 @@ private void checkParameter(VariableTree parameter, SymbolMetadata overrideePara
compareNullability(parameter.type(), overrideeParam, parameter.symbol().metadata(), false);
}

private static boolean isBeanValidationAnnotation(NullabilityData data) {
SymbolMetadata.AnnotationInstance annotation = data.annotation();
return annotation != null
&& BEAN_VALIDATION_ANNOTATIONS.contains(annotation.symbol().type().fullyQualifiedName());
}

private void reportIssue(Tree reportLocation, NullabilityData upperBound, NullabilityData lowerBound, boolean overriddenIsLowerBound) {
NullabilityData otherNullability = overriddenIsLowerBound ? lowerBound : upperBound;
NullabilityData overrideeNullability = overriddenIsLowerBound ? upperBound : lowerBound;
Expand Down
13 changes: 12 additions & 1 deletion java-checks/src/main/java/org/sonar/java/checks/PrimitivesMarkedNullableCheck.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@

import java.util.Collections;
import java.util.List;
import java.util.Set;
import org.sonar.check.Rule;
import org.sonar.java.checks.helpers.QuickFixHelper;
import org.sonar.java.reporting.JavaQuickFix;
Expand All @@ -28,13 +29,22 @@
import org.sonar.plugins.java.api.tree.MethodTree;
import org.sonar.plugins.java.api.tree.Tree;
import org.sonar.plugins.java.api.tree.TypeTree;
import org.sonarsource.analyzer.commons.collections.SetUtils;

import static org.sonar.java.reporting.AnalyzerMessage.textSpanBetween;
import static org.sonar.plugins.java.api.semantic.SymbolMetadata.NullabilityLevel.METHOD;

@Rule(key = "S4682")
public final class PrimitivesMarkedNullableCheck extends IssuableSubscriptionVisitor {

// Bean Validation @NotNull is a runtime constraint, not a nullability annotation.
// Primitives can never be null, so this constraint is meaningless on a primitive return type,
// but it is a different concern from what this rule targets (nullable annotations on primitives).
private static final Set<String> CONSTRAINT_ANNOTATIONS_NOT_FLAGGED = SetUtils.immutableSetOf(
"javax.validation.constraints.NotNull",
"jakarta.validation.constraints.NotNull"
);

@Override
public List<Tree.Kind> nodesToVisit() {
return Collections.singletonList(Tree.Kind.METHOD);
Expand All @@ -50,7 +60,8 @@ public void visitNode(Tree tree) {
SymbolMetadata.AnnotationInstance annotation = nullabilityData.annotation();
Tree annotationTree = nullabilityData.declaration();
// Both "annotation" and "declaration" should never be null, as we only target directly annotated methods. We keep the check for defensive programming.
if (annotation != null && annotationTree != null) {
if (annotation != null && annotationTree != null
&& !CONSTRAINT_ANNOTATIONS_NOT_FLAGGED.contains(annotation.symbol().type().fullyQualifiedName())) {
String annotationName = annotation.symbol().name();
QuickFixHelper.newIssue(context)
.forRule(this)
Expand Down
7 changes: 5 additions & 2 deletions java-frontend/src/main/java/org/sonar/java/model/JSymbolMetadataNullabilityHelper.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -91,6 +91,11 @@ private JSymbolMetadataNullabilityHelper() {
"io.reactivex.rxjava3.annotations.Nullable",
"javax.annotation.Nullable",
"jakarta.annotation.Nullable",
// Bean Validation @NotNull is a runtime constraint, not a static nullability guarantee.
// It is placed here rather than NONNULL_ANNOTATIONS because it cannot serve as a reliable
// static analysis signal (especially when groups= is used), so it is treated conservatively.
"javax.validation.constraints.NotNull",
"jakarta.validation.constraints.NotNull",
Comment on lines +97 to +98

@gitar-bot gitar-bot Bot Aug 21, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Quality: Bean Validation @NotNull FQN set duplicated across 3 files

The two FQNs javax.validation.constraints.NotNull and jakarta.validation.constraints.NotNull are now hard-coded in three independent places: WEAK_NULLABLE_ANNOTATIONS in JSymbolMetadataNullabilityHelper, BEAN_VALIDATION_ANNOTATIONS in ChangeMethodContractCheck, and CONSTRAINT_ANNOTATIONS_NOT_FLAGGED in PrimitivesMarkedNullableCheck. A future change (e.g. adding another BV constraint or a new package) risks being applied inconsistently. Consider exposing a single shared constant/helper (e.g. an isBeanValidationNotNull(String fqn) in a shared helper) and referencing it from all three sites.

Was this helpful? React with 👍 / 👎

Comment on lines +97 to +98

@gitar-bot gitar-bot Bot Aug 21, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Bug: Reclassifying @NotNull may silently change untested SE rules

Moving @NotNull from NONNULL_ANNOTATIONS to WEAK_NULLABLE_ANNOTATIONS is a global change that affects every consumer of isNonNull(), including rules in the java-symbolic-execution module (e.g. NonNullSetToNullCheck / S2637 and related NPE checks) that are not part of this repo module and receive no test updates in this PR. Those rules will stop treating @NotNull as a non-null guarantee, which may turn previously-reported issues into false negatives. Verify the intended behavior of the SE-based rules and add/adjust their test coverage in the symbolic-execution module accordingly.

Was this helpful? React with 👍 / 👎

"org.checkerframework.checker.nullness.compatqual.NullableDecl",
"org.checkerframework.checker.nullness.compatqual.NullableType",
"org.checkerframework.checker.nullness.qual.Nullable",
Expand Down Expand Up @@ -119,8 +124,6 @@ private JSymbolMetadataNullabilityHelper() {
"edu.umd.cs.findbugs.annotations.NonNull",
"io.reactivex.annotations.NonNull",
"io.reactivex.rxjava3.annotations.NonNull",
"javax.validation.constraints.NotNull",
"jakarta.validation.constraints.NotNull",
"lombok.NonNull",
"org.checkerframework.checker.nullness.compatqual.NonNullDecl",
"org.checkerframework.checker.nullness.compatqual.NonNullType",
Expand Down
Loading