JAVASE-241 FPs when Jakarta / Javax NotNull annotation is used

[asya-vorobeva]

Treat Javax / Jakarta Bean Validation @NotNull annotations as WEAK_NULLABLE.

[hashicorp-vault-sonar-prod]

JAVASE-241

[gitar-bot]

⚠️ Bug: @NotNull reclassified globally causes new FP in Spring S6816

Moving javax/jakarta.validation.constraints.NotNull from NONNULL_ANNOTATIONS to WEAK_NULLABLE_ANNOTATIONS is a global change that affects every rule reading nullabilityData(), not just the two rules patched with explicit exclusions. In S6816 (NullableInjectedFieldsHaveDefaultValueCheck), getNullableAnnotation() returns present whenever an annotation exists and isNonNull() is false; a Spring @Value("${x}") @NotNull field (a very common combination) now satisfies that condition and gets flagged "Provide a default null value for this field" — contradictory advice for a field explicitly required to be non-null. This is a new false positive introduced by the reclassification and is not covered by any test in this PR. Consider either keeping @NotNull out of both the NONNULL and NULLABLE sets (so it resolves to NO_ANNOTATION/UNKNOWN and only the intended rules change), or adding a bean-validation guard to S6816 like the ones added to S2638/S4682.

Guard S6816 against Bean Validation @NotNull (mirroring the exclusion sets added to S2638/S4682).:

private static Optional<AnnotationTree> getNullableAnnotation(VariableTree field) {
  SymbolMetadata.NullabilityData nullabilityData = field.symbol().metadata().nullabilityData(SymbolMetadata.NullabilityTarget.FIELD);
  SymbolMetadata.AnnotationInstance instance = nullabilityData.annotation();
  if (instance == null
      || nullabilityData.isNonNull(SymbolMetadata.NullabilityLevel.CLASS, false, false)
      || isBeanValidationConstraint(instance)) {
    return Optional.empty();
  }
  return Optional.ofNullable(field.symbol().metadata().findAnnotationTree(instance));
}

_{Was this helpful? React with 👍 / 👎}

[gitar-bot]

💡 Bug: Global @NotNull move silently changes S2789/S2447/S4454 behavior

Beyond the two patched rules, the reclassification changes other rules that read nullability with no accompanying tests: S2789 (NullShouldNotBeUsedWithOptionalCheck) will now report an @NotNull Optional<...> method as "should not be @NotNull" (new false positive via isNullable at line 175); S2447 (BooleanMethodReturnCheck) will stop checking @NotNull Boolean methods that return null literals, since !isNullable(...) is now false (new false negative); and S4454 (EqualsParametersMarkedNonNullCheck) stops flagging @NotNull equals parameters (the PR's test change to EqualsParametersMarkedNonNullCheckSample marks this Compliant, so likely intended, but S2789/S2447 appear unintended). Add ruling/test coverage for these rules to confirm the behavior changes are acceptable, or gate the reclassification the same way the two patched rules do.

Avoid globally tagging @NotNull as nullable; scope the special-casing to the intended rules.:

// Prefer not classifying a runtime constraint as a static nullability signal at all:
// keep javax/jakarta validation @NotNull OUT of both WEAK_NULLABLE_ANNOTATIONS and
// NONNULL_ANNOTATIONS so it resolves to NO_ANNOTATION, then only S2638/S4682 need the
// explicit bean-validation handling. This avoids the S2789/S2447/S4454 side effects.

_{Was this helpful? React with 👍 / 👎}

[gitar-bot]

Code Review ⚠️ Changes requested 0 resolved / 2 findings

Treats Bean Validation @NotNull annotations as WEAK_NULLABLE to fix false positives, but global reclassification causes new false positives in Spring S6816 and silently alters behavior in other rules without tests.

⚠️ Bug: @NotNull reclassified globally causes new FP in Spring S6816

📄 java-frontend/src/main/java/org/sonar/java/model/JSymbolMetadataNullabilityHelper.java:94-98

Moving javax/jakarta.validation.constraints.NotNull from NONNULL_ANNOTATIONS to WEAK_NULLABLE_ANNOTATIONS is a global change that affects every rule reading nullabilityData(), not just the two rules patched with explicit exclusions. In S6816 (NullableInjectedFieldsHaveDefaultValueCheck), getNullableAnnotation() returns present whenever an annotation exists and isNonNull() is false; a Spring @Value("${x}") @NotNull field (a very common combination) now satisfies that condition and gets flagged "Provide a default null value for this field" — contradictory advice for a field explicitly required to be non-null. This is a new false positive introduced by the reclassification and is not covered by any test in this PR. Consider either keeping @NotNull out of both the NONNULL and NULLABLE sets (so it resolves to NO_ANNOTATION/UNKNOWN and only the intended rules change), or adding a bean-validation guard to S6816 like the ones added to S2638/S4682.

Guard S6816 against Bean Validation @NotNull (mirroring the exclusion sets added to S2638/S4682).

private static Optional<AnnotationTree> getNullableAnnotation(VariableTree field) {
  SymbolMetadata.NullabilityData nullabilityData = field.symbol().metadata().nullabilityData(SymbolMetadata.NullabilityTarget.FIELD);
  SymbolMetadata.AnnotationInstance instance = nullabilityData.annotation();
  if (instance == null
      || nullabilityData.isNonNull(SymbolMetadata.NullabilityLevel.CLASS, false, false)
      || isBeanValidationConstraint(instance)) {
    return Optional.empty();
  }
  return Optional.ofNullable(field.symbol().metadata().findAnnotationTree(instance));
}

💡 Bug: Global @NotNull move silently changes S2789/S2447/S4454 behavior

📄 java-frontend/src/main/java/org/sonar/java/model/JSymbolMetadataNullabilityHelper.java:94-98

Beyond the two patched rules, the reclassification changes other rules that read nullability with no accompanying tests: S2789 (NullShouldNotBeUsedWithOptionalCheck) will now report an @NotNull Optional<...> method as "should not be @NotNull" (new false positive via isNullable at line 175); S2447 (BooleanMethodReturnCheck) will stop checking @NotNull Boolean methods that return null literals, since !isNullable(...) is now false (new false negative); and S4454 (EqualsParametersMarkedNonNullCheck) stops flagging @NotNull equals parameters (the PR's test change to EqualsParametersMarkedNonNullCheckSample marks this Compliant, so likely intended, but S2789/S2447 appear unintended). Add ruling/test coverage for these rules to confirm the behavior changes are acceptable, or gate the reclassification the same way the two patched rules do.

Avoid globally tagging @NotNull as nullable; scope the special-casing to the intended rules.

// Prefer not classifying a runtime constraint as a static nullability signal at all:
// keep javax/jakarta validation @NotNull OUT of both WEAK_NULLABLE_ANNOTATIONS and
// NONNULL_ANNOTATIONS so it resolves to NO_ANNOTATION, then only S2638/S4682 need the
// explicit bean-validation handling. This avoids the S2789/S2447/S4454 side effects.

🤖 Prompt for agents

Code Review: Treats Bean Validation @NotNull annotations as WEAK_NULLABLE to fix false positives, but global reclassification causes new false positives in Spring S6816 and silently alters behavior in other rules without tests.

1. ⚠️ Bug: @NotNull reclassified globally causes new FP in Spring S6816
   Files: java-frontend/src/main/java/org/sonar/java/model/JSymbolMetadataNullabilityHelper.java:94-98

   Moving `javax/jakarta.validation.constraints.NotNull` from NONNULL_ANNOTATIONS to WEAK_NULLABLE_ANNOTATIONS is a global change that affects every rule reading nullabilityData(), not just the two rules patched with explicit exclusions. In S6816 (NullableInjectedFieldsHaveDefaultValueCheck), getNullableAnnotation() returns present whenever an annotation exists and isNonNull() is false; a Spring `@Value("${x}") @NotNull` field (a very common combination) now satisfies that condition and gets flagged "Provide a default null value for this field" — contradictory advice for a field explicitly required to be non-null. This is a new false positive introduced by the reclassification and is not covered by any test in this PR. Consider either keeping @NotNull out of both the NONNULL and NULLABLE sets (so it resolves to NO_ANNOTATION/UNKNOWN and only the intended rules change), or adding a bean-validation guard to S6816 like the ones added to S2638/S4682.

   Fix (Guard S6816 against Bean Validation @NotNull (mirroring the exclusion sets added to S2638/S4682).):
   private static Optional<AnnotationTree> getNullableAnnotation(VariableTree field) {
     SymbolMetadata.NullabilityData nullabilityData = field.symbol().metadata().nullabilityData(SymbolMetadata.NullabilityTarget.FIELD);
     SymbolMetadata.AnnotationInstance instance = nullabilityData.annotation();
     if (instance == null
         || nullabilityData.isNonNull(SymbolMetadata.NullabilityLevel.CLASS, false, false)
         || isBeanValidationConstraint(instance)) {
       return Optional.empty();
     }
     return Optional.ofNullable(field.symbol().metadata().findAnnotationTree(instance));
   }

2. 💡 Bug: Global @NotNull move silently changes S2789/S2447/S4454 behavior
   Files: java-frontend/src/main/java/org/sonar/java/model/JSymbolMetadataNullabilityHelper.java:94-98

   Beyond the two patched rules, the reclassification changes other rules that read nullability with no accompanying tests: S2789 (NullShouldNotBeUsedWithOptionalCheck) will now report an `@NotNull Optional<...>` method as "should not be @NotNull" (new false positive via isNullable at line 175); S2447 (BooleanMethodReturnCheck) will stop checking `@NotNull Boolean` methods that return null literals, since `!isNullable(...)` is now false (new false negative); and S4454 (EqualsParametersMarkedNonNullCheck) stops flagging `@NotNull` equals parameters (the PR's test change to EqualsParametersMarkedNonNullCheckSample marks this Compliant, so likely intended, but S2789/S2447 appear unintended). Add ruling/test coverage for these rules to confirm the behavior changes are acceptable, or gate the reclassification the same way the two patched rules do.

   Fix (Avoid globally tagging @NotNull as nullable; scope the special-casing to the intended rules.):
   // Prefer not classifying a runtime constraint as a static nullability signal at all:
   // keep javax/jakarta validation @NotNull OUT of both WEAK_NULLABLE_ANNOTATIONS and
   // NONNULL_ANNOTATIONS so it resolves to NO_ANNOTATION, then only S2638/S4682 need the
   // explicit bean-validation handling. This avoids the S2789/S2447/S4454 side effects.

Options

Auto-apply is off → Gitar will not commit updates to this branch.
Display: compact → Showing less information.
Unblock → Override a blocking verdict and allow merging.

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

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

_{Was this helpful? React with 👍 / 👎 | Gitar}

[sonarqube-next]

Quality Gate passed

Issues
0 New issues
0 Fixed issues
0 Accepted issues

Measures
0 Security Hotspots
0 Dependency risks
96.6% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube