Skip to content

SONARJAVA-6540 Implement S8954: Bean Validation constraints should not be placed on static fields#5724

Merged
NoemieBenard merged 7 commits into
masterfrom
nb/sonarjava-6540-implement-S8954
Jul 3, 2026
Merged

SONARJAVA-6540 Implement S8954: Bean Validation constraints should not be placed on static fields#5724
NoemieBenard merged 7 commits into
masterfrom
nb/sonarjava-6540-implement-S8954

Conversation

@NoemieBenard

@NoemieBenard NoemieBenard commented Jul 1, 2026

Copy link
Copy Markdown
Contributor

Summary by Gitar

  • New Rule Implementation:
    • Added BeanValidationConstraintOnStaticFieldCheck to flag constraints on static fields.
    • Supports both javax and jakarta validation packages.
  • Documentation and Metadata:
    • Created S8954.json and S8954.html with rule details and remediation guidance.
    • Enabled the rule in the Sonar_way_profile.json configuration.
  • Test Coverage:
    • Implemented unit tests in BeanValidationConstraintOnStaticFieldCheckTest using provided sample files for javax and jakarta validation.
  • Autoscan Configuration:
    • Updated diff_S1710.json and added diff_S8954.json in the autoscan test resources.

This will update automatically on new commits.

@hashicorp-vault-sonar-prod

hashicorp-vault-sonar-prod Bot commented Jul 1, 2026

Copy link
Copy Markdown
Contributor

SONARJAVA-6540

@NoemieBenard NoemieBenard marked this pull request as ready for review July 2, 2026 14:28
@gitar-bot

gitar-bot Bot commented Jul 2, 2026

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

Implements rule S8954 to flag Bean Validation constraints on static fields, with support for both javax and jakarta packages. Heuristic container detection is functional, though note that nested annotation detection may occasionally cause false positives.

💡 Edge Case: Heuristic container detection may cause false positives

📄 java-checks/src/main/java/org/sonar/java/checks/BeanValidationConstraintOnStaticFieldCheck.java:65-79

The new logic infers that an annotation is a repeatable-constraint container whenever any of its argument values is (or contains, via an array/assignment) a nested annotation that is itself a Bean Validation constraint. This is a heuristic rather than an actual check for the container/@Repeatable relationship.

As a result, any non-container annotation that legitimately accepts a nested constraint annotation as a member value (e.g. a custom or framework annotation with a member of type Pattern or @Constraint-meta-annotated element) placed on a static field would be reported, even though it is not a repeatable constraint container. This could produce false positives.

A more precise approach is to verify that the outer annotation type is the container of a repeatable constraint — e.g. its value() element returns an array of an annotation type that is meta-annotated with @Constraint and is itself marked @Repeatable with this container. If the current heuristic is intentional for simplicity, consider documenting the limitation. Given the low likelihood of a real-world non-container annotation nesting a constraint, this is minor.

✅ 1 resolved
Edge Case: Repeatable/container constraint annotations are not detected

📄 java-checks/src/main/java/org/sonar/java/checks/BeanValidationConstraintOnStaticFieldCheck.java:46-60
isBeanValidationConstraint only flags an annotation whose type is directly meta-annotated with javax.validation.Constraint/jakarta.validation.Constraint. Bean Validation container annotations for repeatable constraints (e.g. @Pattern.List({ @Pattern(...), @Pattern(...) }), @Size.List, etc.) are NOT themselves meta-annotated with @Constraint — they are @Repeatable/list container types whose value() holds an array of constraints. As a result, a static field annotated with such a container annotation would be silently ignored by this rule, producing a false negative for a real Bean Validation constraint that the framework will still ignore on the static field.

Consider also inspecting the annotation's value() members (or recognizing the standard *.List container annotations) so repeatable constraints applied to static fields are reported. At minimum, add a test case covering a repeated constraint to document the current behavior.

sources: java-checks/src/main/java/org/sonar/java/checks/BeanValidationConstraintOnStaticFieldCheck.java:53-60

🤖 Prompt for agents
Code Review: Implements rule S8954 to flag Bean Validation constraints on static fields, with support for both javax and jakarta packages. Heuristic container detection is functional, though note that nested annotation detection may occasionally cause false positives.

1. 💡 Edge Case: Heuristic container detection may cause false positives
   Files: java-checks/src/main/java/org/sonar/java/checks/BeanValidationConstraintOnStaticFieldCheck.java:65-79

   The new logic infers that an annotation is a repeatable-constraint *container* whenever any of its argument values is (or contains, via an array/assignment) a nested annotation that is itself a Bean Validation constraint. This is a heuristic rather than an actual check for the container/`@Repeatable` relationship.
   
   As a result, any non-container annotation that legitimately accepts a nested constraint annotation as a member value (e.g. a custom or framework annotation with a member of type `Pattern` or `@Constraint`-meta-annotated element) placed on a static field would be reported, even though it is not a repeatable constraint container. This could produce false positives.
   
   A more precise approach is to verify that the outer annotation type is the container of a repeatable constraint — e.g. its `value()` element returns an array of an annotation type that is meta-annotated with `@Constraint` and is itself marked `@Repeatable` with this container. If the current heuristic is intentional for simplicity, consider documenting the limitation. Given the low likelihood of a real-world non-container annotation nesting a constraint, this is minor.

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

@NoemieBenard NoemieBenard force-pushed the nb/sonarjava-6540-implement-S8954 branch from 4f84482 to 7fb0576 Compare July 2, 2026 15:30

@romainbrenguier romainbrenguier left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

The messaging doesn't look great to me. Otherwise implementation looks good.

@sonarqube-next

sonarqube-next Bot commented Jul 3, 2026

Copy link
Copy Markdown

@NoemieBenard NoemieBenard merged commit 27a9c3e into master Jul 3, 2026
17 checks passed
@NoemieBenard NoemieBenard deleted the nb/sonarjava-6540-implement-S8954 branch July 3, 2026 14:06
@gitar-bot

gitar-bot Bot commented Jul 3, 2026

Copy link
Copy Markdown
Code Review ✅ Approved 2 resolved / 2 findings

Implements rule S8954 to flag Bean Validation constraints on static fields for both javax and jakarta namespaces. The final implementation resolves potential false positives from container heuristic detection and ensures accurate identification of constraint annotations.

✅ 2 resolved
Edge Case: Repeatable/container constraint annotations are not detected

📄 java-checks/src/main/java/org/sonar/java/checks/BeanValidationConstraintOnStaticFieldCheck.java:46-60
isBeanValidationConstraint only flags an annotation whose type is directly meta-annotated with javax.validation.Constraint/jakarta.validation.Constraint. Bean Validation container annotations for repeatable constraints (e.g. @Pattern.List({ @Pattern(...), @Pattern(...) }), @Size.List, etc.) are NOT themselves meta-annotated with @Constraint — they are @Repeatable/list container types whose value() holds an array of constraints. As a result, a static field annotated with such a container annotation would be silently ignored by this rule, producing a false negative for a real Bean Validation constraint that the framework will still ignore on the static field.

Consider also inspecting the annotation's value() members (or recognizing the standard *.List container annotations) so repeatable constraints applied to static fields are reported. At minimum, add a test case covering a repeated constraint to document the current behavior.

sources: java-checks/src/main/java/org/sonar/java/checks/BeanValidationConstraintOnStaticFieldCheck.java:53-60

Edge Case: Heuristic container detection may cause false positives

📄 java-checks/src/main/java/org/sonar/java/checks/BeanValidationConstraintOnStaticFieldCheck.java:65-79
The new logic infers that an annotation is a repeatable-constraint container whenever any of its argument values is (or contains, via an array/assignment) a nested annotation that is itself a Bean Validation constraint. This is a heuristic rather than an actual check for the container/@Repeatable relationship.

As a result, any non-container annotation that legitimately accepts a nested constraint annotation as a member value (e.g. a custom or framework annotation with a member of type Pattern or @Constraint-meta-annotated element) placed on a static field would be reported, even though it is not a repeatable constraint container. This could produce false positives.

A more precise approach is to verify that the outer annotation type is the container of a repeatable constraint — e.g. its value() element returns an array of an annotation type that is meta-annotated with @Constraint and is itself marked @Repeatable with this container. If the current heuristic is intentional for simplicity, consider documenting the limitation. Given the low likelihood of a real-world non-container annotation nesting a constraint, this is minor.

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

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.

2 participants