SONARJAVA-6588 - Implement S8983: Stateless session beans should not store mutable state in instance variables

[NoemieBenard]

---

Summary by Gitar

• New inspection rule:
  • Implemented S8983 to detect mutable instance fields in classes annotated with @Stateless.
  • Flagging occurs unless the field is static, final, or annotated with container-managed injection markers like @Inject, @EJB, or @PersistenceContext.
• Test infrastructure:
  • Added comprehensive test suites for standard and Jakarta EE contexts using StatelessBeanInstanceFieldCheckSample.java and StatelessBeanInstanceFieldCheckJakartaSample.java.
  • Included stubs for required Jakarta annotations to support semantic analysis during testing.
• Rule metadata:
  • Created rule specification, documentation, and Sonar_way profile entry for S8983.

_{This will update automatically on new commits.}

[hashicorp-vault-sonar-prod]

SONARJAVA-6588

[gitar-bot]

💡 Edge Case: final/static fields holding mutable objects are treated as safe

In isSafe() (StatelessBeanInstanceFieldCheck.java:78-84), any field marked static or final is considered safe and never reported. However the rule targets mutable state, and final/static only make the reference immutable, not the referenced object. A stateless bean with private final List<String> cache = new ArrayList<>(); or private static Map<...> shared = ...; still stores mutable, request-crossing state and can leak data or cause race conditions — exactly the problems the rule description warns about ("Multiple threads may access the same field concurrently"). These cases produce false negatives.

This appears partly intentional (the sample marks static final int MAX_RETRIES and static int sharedCounter as compliant), so it may match the RSPEC scope. If so, no change is needed; otherwise consider inspecting the field's declared type (e.g. mutable collections / non-final referenced objects) rather than relying solely on the static/final modifiers. At minimum, adding a test sample with a final collection field would document the intended behavior explicitly.

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

[sonarqube-next]

Quality Gate passed

Issues
0 New issues
0 Fixed issues
0 Accepted issues

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

See analysis details on SonarQube

[gitar-bot]

Code Review 👍 Approved with suggestions 0 resolved / 1 findings

Implements rule S8983 to detect mutable instance fields in @Stateless beans, ensuring compliance with container-managed state patterns. Note that the current implementation considers final or static fields containing mutable objects as safe; consider tightening this logic to avoid potential concurrency issues.

💡 Edge Case: final/static fields holding mutable objects are treated as safe

📄 java-checks/src/main/java/org/sonar/java/checks/StatelessBeanInstanceFieldCheck.java:78-84

In isSafe() (StatelessBeanInstanceFieldCheck.java:78-84), any field marked static or final is considered safe and never reported. However the rule targets mutable state, and final/static only make the reference immutable, not the referenced object. A stateless bean with private final List<String> cache = new ArrayList<>(); or private static Map<...> shared = ...; still stores mutable, request-crossing state and can leak data or cause race conditions — exactly the problems the rule description warns about ("Multiple threads may access the same field concurrently"). These cases produce false negatives.

This appears partly intentional (the sample marks static final int MAX_RETRIES and static int sharedCounter as compliant), so it may match the RSPEC scope. If so, no change is needed; otherwise consider inspecting the field's declared type (e.g. mutable collections / non-final referenced objects) rather than relying solely on the static/final modifiers. At minimum, adding a test sample with a final collection field would document the intended behavior explicitly.

🤖 Prompt for agents

Code Review: Implements rule S8983 to detect mutable instance fields in @Stateless beans, ensuring compliance with container-managed state patterns. Note that the current implementation considers final or static fields containing mutable objects as safe; consider tightening this logic to avoid potential concurrency issues.

1. 💡 Edge Case: final/static fields holding mutable objects are treated as safe
   Files: java-checks/src/main/java/org/sonar/java/checks/StatelessBeanInstanceFieldCheck.java:78-84

   In `isSafe()` (StatelessBeanInstanceFieldCheck.java:78-84), any field marked `static` or `final` is considered safe and never reported. However the rule targets *mutable state*, and `final`/`static` only make the *reference* immutable, not the referenced object. A stateless bean with `private final List<String> cache = new ArrayList<>();` or `private static Map<...> shared = ...;` still stores mutable, request-crossing state and can leak data or cause race conditions — exactly the problems the rule description warns about ("Multiple threads may access the same field concurrently"). These cases produce false negatives.
   
   This appears partly intentional (the sample marks `static final int MAX_RETRIES` and `static int sharedCounter` as compliant), so it may match the RSPEC scope. If so, no change is needed; otherwise consider inspecting the field's declared type (e.g. mutable collections / non-final referenced objects) rather than relying solely on the `static`/`final` modifiers. At minimum, adding a test sample with a `final` collection field would document the intended behavior explicitly.

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}