RC-12 Implement rule S2654: JEE applications should not use threads or synchronization

[NoemieBenard]

No description provided.

[hashicorp-vault-sonar-prod]

RC-12

[gitar-bot]

💡 Bug: isEjbAnnotated path for synchronized methods is never tested

handleMethod flags synchronized methods either when JEE_SERVLET_MATCHER.matches(method) (servlet) or when isEjbAnnotated(method) is true. The test samples (JEEThreadCheckSample.java / JEEThreadCheckJakartaSample.java) only contain a synchronized method (syncMethod) inside the servlet classes, exercising the matcher branch. The EJB classes (Stateless/Stateful/Singleton/MessageDriven) only contain a synchronized block and a Runnable — none has a synchronized method. As a result the isEjbAnnotated branch in handleMethod, including the Objects.requireNonNull(method.symbol().enclosingClass()) call, is never covered by tests. Add a public synchronized void m() {} to at least one EJB sample class (and verify it is flagged Noncompliant) to cover this branch.

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

[gitar-bot]

💡 Edge Case: JEE component implementing Runnable directly is not flagged

isInJeeClass for a CLASS node starts the search at tree.parent(), so it checks only the enclosing class, never the class itself. Consequently a JEE component that directly implements Runnable (e.g. @Stateless class Foo implements Runnable { public void run(){} }) is not reported, because the search skips Foo and finds no enclosing JEE class. Similarly, a Runnable declared inside a non-JEE inner class nested within a servlet/EJB is not flagged, since only the first enclosing class is inspected. If these are intended out-of-scope cases it would be worth adding a Compliant/Noncompliant test documenting the behavior; otherwise consider checking the class itself and/or walking all enclosing classes.

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

[gitar-bot]

CI failed: The CI build encountered a transient 429 dependency resolution error and a series of end-to-end test timeouts. The Playwright UI tests failed due to resource contention or environment instability rather than the specific code changes in the PR.

Overview

Two primary failure patterns were identified across the CI runs: a transient dependency resolution error in the build phase and multiple E2E test timeouts. Overall, the build is blocked by infrastructure instability and test flakiness.

Failures

Dependency Resolution (429 Too Many Requests) (confidence: high)

• Type: dependency
• Affected jobs: 83241771577
• Related to change: no
• Root cause: The build environment hit a rate limit (HTTP 429) when fetching Micronaut artifacts from Maven Central.
• Suggested fix: Rerunning the job should resolve this transient network issue.

Playwright End-to-End Test Timeouts (confidence: high)

• Type: flaky_test
• Affected jobs: 83241790672
• Root cause: The Playwright test suite experienced multiple 60s timeouts during UI interactions (e.g., clicking 'Add additional info' and asserting element visibility), indicating environment latency or resource contention.
• Suggested fix: Triage the failing Playwright tests. If they continue to fail, increase the wait timeouts or improve the polling logic for elements that are slow to render.

Summary

• Change-related failures: 0; none of the identified errors stem from the submitted code changes.
• Infrastructure/flaky failures: 2; one transient network dependency failure and one set of flaky UI tests.
• Recommended action: Re-run the CI jobs. If the Playwright tests continue to fail, investigate potential performance regressions in the affected UI workflows.

Code Review 👍 Approved with suggestions 0 resolved / 2 findings

Implements rule S2654 to detect forbidden thread and synchronization usage in JEE applications. The implementation requires additional test coverage for synchronized methods in EJB-annotated classes and updated detection logic for JEE components implementing Runnable directly.

💡 Bug: isEjbAnnotated path for synchronized methods is never tested

📄 java-checks/src/main/java/org/sonar/java/checks/JEEThreadCheck.java:94-104 📄 java-checks-test-sources/default/src/main/java/checks/JEEThreadCheckSample.java:48-62

handleMethod flags synchronized methods either when JEE_SERVLET_MATCHER.matches(method) (servlet) or when isEjbAnnotated(method) is true. The test samples (JEEThreadCheckSample.java / JEEThreadCheckJakartaSample.java) only contain a synchronized method (syncMethod) inside the servlet classes, exercising the matcher branch. The EJB classes (Stateless/Stateful/Singleton/MessageDriven) only contain a synchronized block and a Runnable — none has a synchronized method. As a result the isEjbAnnotated branch in handleMethod, including the Objects.requireNonNull(method.symbol().enclosingClass()) call, is never covered by tests. Add a public synchronized void m() {} to at least one EJB sample class (and verify it is flagged Noncompliant) to cover this branch.

💡 Edge Case: JEE component implementing Runnable directly is not flagged

📄 java-checks/src/main/java/org/sonar/java/checks/JEEThreadCheck.java:106-115 📄 java-checks/src/main/java/org/sonar/java/checks/JEEThreadCheck.java:71-80

isInJeeClass for a CLASS node starts the search at tree.parent(), so it checks only the enclosing class, never the class itself. Consequently a JEE component that directly implements Runnable (e.g. @Stateless class Foo implements Runnable { public void run(){} }) is not reported, because the search skips Foo and finds no enclosing JEE class. Similarly, a Runnable declared inside a non-JEE inner class nested within a servlet/EJB is not flagged, since only the first enclosing class is inspected. If these are intended out-of-scope cases it would be worth adding a Compliant/Noncompliant test documenting the behavior; otherwise consider checking the class itself and/or walking all enclosing classes.

🤖 Prompt for agents

Code Review: Implements rule S2654 to detect forbidden thread and synchronization usage in JEE applications. The implementation requires additional test coverage for synchronized methods in EJB-annotated classes and updated detection logic for JEE components implementing Runnable directly.

1. 💡 Bug: isEjbAnnotated path for synchronized methods is never tested
   Files: java-checks/src/main/java/org/sonar/java/checks/JEEThreadCheck.java:94-104, java-checks-test-sources/default/src/main/java/checks/JEEThreadCheckSample.java:48-62

   `handleMethod` flags synchronized methods either when `JEE_SERVLET_MATCHER.matches(method)` (servlet) or when `isEjbAnnotated(method)` is true. The test samples (JEEThreadCheckSample.java / JEEThreadCheckJakartaSample.java) only contain a synchronized *method* (`syncMethod`) inside the servlet classes, exercising the matcher branch. The EJB classes (Stateless/Stateful/Singleton/MessageDriven) only contain a `synchronized` block and a `Runnable` — none has a `synchronized` method. As a result the `isEjbAnnotated` branch in `handleMethod`, including the `Objects.requireNonNull(method.symbol().enclosingClass())` call, is never covered by tests. Add a `public synchronized void m() {}` to at least one EJB sample class (and verify it is flagged Noncompliant) to cover this branch.

2. 💡 Edge Case: JEE component implementing Runnable directly is not flagged
   Files: java-checks/src/main/java/org/sonar/java/checks/JEEThreadCheck.java:106-115, java-checks/src/main/java/org/sonar/java/checks/JEEThreadCheck.java:71-80

   `isInJeeClass` for a CLASS node starts the search at `tree.parent()`, so it checks only the *enclosing* class, never the class itself. Consequently a JEE component that directly implements Runnable (e.g. `@Stateless class Foo implements Runnable { public void run(){} }`) is not reported, because the search skips `Foo` and finds no enclosing JEE class. Similarly, a Runnable declared inside a non-JEE inner class nested within a servlet/EJB is not flagged, since only the first enclosing class is inspected. If these are intended out-of-scope cases it would be worth adding a Compliant/Noncompliant test documenting the behavior; otherwise consider checking the class itself and/or walking all enclosing classes.

Tip

Comment Gitar fix CI or enable auto-apply: gitar auto-apply:on

Options

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

Comment with these commands to change:

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

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

[sonarqube-next]

Quality Gate failed

Failed conditions
1 New issue

See analysis details on SonarQube

Catch issues before they fail your Quality Gate with our IDE extension SonarQube for IDE