Skip to content

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

Open
asya-vorobeva wants to merge 1 commit into
masterfrom
asya/fix-bean-validation-annotations-nullability-types
Open

JAVASE-241 Treat Bean Validation @NotNull as WEAK_NULLABLE to fix FPs (JAVASE-241)#5991
asya-vorobeva wants to merge 1 commit into
masterfrom
asya/fix-bean-validation-annotations-nullability-types

Conversation

@asya-vorobeva

Copy link
Copy Markdown
Contributor

javax.validation.constraints.NotNull and jakarta.validation.constraints.NotNull are runtime constraints, not static nullability guarantees. Their previous NON_NULL classification also caused an inconsistency (SONARJAVA-3803): @NotNull without arguments resolved to NON_NULL while @NotNull(groups=...) resolved to UNKNOWN. Moving both to WEAK_NULLABLE gives consistent, conservative treatment.

Rules fixed as a direct consequence:

  • S4454: @NotNull on equals() parameter no longer fires (WEAK_NULLABLE is not isNonNull())
  • S6539: @NotNull inside @NullMarked no longer flagged as redundant

Rules requiring explicit guards after reclassification:

  • S4682: added FQN-based exclusion — BV @NotNull on a primitive return type is a validation constraint, not a nullable annotation
  • S2638: added isBeanValidationAnnotation() guard in compareNullability() — when the upper-bound annotation is a BV annotation, the comparison is skipped so that BV @NotNull on a parent param or child return does not incorrectly fire

javax.validation.constraints.NotNull and jakarta.validation.constraints.NotNull
are runtime constraints, not static nullability guarantees. Their previous
NON_NULL classification also caused an inconsistency (SONARJAVA-3803): @NotNull
without arguments resolved to NON_NULL while @NotNull(groups=...) resolved to
UNKNOWN. Moving both to WEAK_NULLABLE gives consistent, conservative treatment.

Rules fixed as a direct consequence:
- S4454: @NotNull on equals() parameter no longer fires (WEAK_NULLABLE is not isNonNull())
- S6539: @NotNull inside @NullMarked no longer flagged as redundant

Rules requiring explicit guards after reclassification:
- S4682: added FQN-based exclusion — BV @NotNull on a primitive return type
  is a validation constraint, not a nullable annotation
- S2638: added isBeanValidationAnnotation() guard in compareNullability() —
  when the upper-bound annotation is a BV annotation, the comparison is skipped
  so that BV @NotNull on a parent param or child return does not incorrectly fire

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
@hashicorp-vault-sonar-prod

hashicorp-vault-sonar-prod Bot commented Aug 21, 2026

Copy link
Copy Markdown
Contributor

JAVASE-241

Comment on lines +97 to +98
"javax.validation.constraints.NotNull",
"jakarta.validation.constraints.NotNull",

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
"javax.validation.constraints.NotNull",
"jakarta.validation.constraints.NotNull",

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 👍 / 👎

@gitar-bot

gitar-bot Bot commented Aug 21, 2026

Copy link
Copy Markdown
Code Review 👍 Approved with suggestions 0 resolved / 2 findings

Reclassifies Bean Validation @NotNull annotations as WEAK_NULLABLE to resolve false positives and inconsistencies, adding necessary rule guards. Consider deduplicating the FQN set and auditing potential side-effects on symbolic execution rules.

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

📄 java-frontend/src/main/java/org/sonar/java/model/JSymbolMetadataNullabilityHelper.java:97-98 📄 java-checks/src/main/java/org/sonar/java/checks/ChangeMethodContractCheck.java:48-51 📄 java-checks/src/main/java/org/sonar/java/checks/PrimitivesMarkedNullableCheck.java:43-46

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.

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

📄 java-frontend/src/main/java/org/sonar/java/model/JSymbolMetadataNullabilityHelper.java:97-98 📄 java-frontend/src/main/java/org/sonar/java/model/JSymbolMetadataNullabilityHelper.java:124-125

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.

🤖 Prompt for agents
Code Review: Reclassifies Bean Validation @NotNull annotations as WEAK_NULLABLE to resolve false positives and inconsistencies, adding necessary rule guards. Consider deduplicating the FQN set and auditing potential side-effects on symbolic execution rules.

1. 💡 Quality: Bean Validation @NotNull FQN set duplicated across 3 files
   Files: java-frontend/src/main/java/org/sonar/java/model/JSymbolMetadataNullabilityHelper.java:97-98, java-checks/src/main/java/org/sonar/java/checks/ChangeMethodContractCheck.java:48-51, java-checks/src/main/java/org/sonar/java/checks/PrimitivesMarkedNullableCheck.java:43-46

   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.

2. 💡 Bug: Reclassifying @NotNull may silently change untested SE rules
   Files: java-frontend/src/main/java/org/sonar/java/model/JSymbolMetadataNullabilityHelper.java:97-98, java-frontend/src/main/java/org/sonar/java/model/JSymbolMetadataNullabilityHelper.java:124-125

   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.

Options

Auto-apply is off → Gitar will not commit updates to this branch.
Display: compact → Showing less information.

Comment with these commands to change the behavior for this request:

Auto-apply Compact
gitar auto-apply:on         
gitar display:verbose         

Was this helpful? React with 👍 / 👎 | Gitar

@sonarqube-next

Copy link
Copy Markdown
Contributor

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant