SONARJAVA-6906 - Modify S8924 to raise only on inconsistent Mockito import usage

[NoemieBenard]

Summary

• S8924 now only raises an issue when a file mixes prefixed Mockito.xxx() calls with at least one static import of a Mockito method, instead of flagging every prefixed call unconditionally.
• Files that consistently use the Mockito. prefix throughout, without any static import of a Mockito method, are no longer flagged.
• Added MockitoStaticImportCheckNoStaticImportSample.java and a corresponding test case covering the fully-prefixed, no-static-import scenario.

---

Summary by Gitar

• Rule renaming and refactoring:
  • Renamed CompilationOrPreparationInLoopCheck to PreparedStatementInsideLoopCheck (rule S9142) and removed regex checks.
• Documentation updates:
  • Updated rule S9142 metadata, title, HTML description, and resources.

_{This will update automatically on new commits.}

[hashicorp-vault-sonar-prod]

SONARJAVA-6906

[gitar-bot]

⚠️ Bug: S8924 rule description contradicts the new mixed-usage behavior

The check now only reports when the file also contains a static import of a Mockito member (hasStaticMockitoImport gate at MockitoStaticImportCheck.java:109), but S8924.html and S8924.json were not updated: the description still states "This rule raises an issue when Mockito core methods are called with the Mockito. prefix instead of being statically imported", and its Noncompliant example (import org.mockito.Mockito; with no static import, two // Noncompliant markers) is exactly the code the new MockitoStaticImportCheckNoStaticImportSample.java asserts is Compliant. Users reading the rule page will expect issues on fully-prefixed files and see none; the documented Noncompliant example no longer raises. Update the lead paragraph, the "How to fix it" text and the Noncompliant/Compliant examples to describe the mixed-import trigger, and adjust the title in S8924.json accordingly (the branch's two "Update rule metadata" commits only touched S9142).

Replace the first paragraph of S8924.html, and make the Noncompliant example contain at least one import static org.mockito.Mockito.… so it matches the implemented trigger; update the S8924.json title to something like "Mockito core methods should be imported consistently".:

<p>This rule raises an issue when a test file mixes both styles of accessing Mockito core methods: some calls use a static import while others keep
the <code>Mockito.</code> prefix. Files that consistently use the <code>Mockito.</code> prefix everywhere are not reported.</p>

• Apply fix

_{Check the box to apply the fix or reply for a change | Was this helpful? React with 👍 / 👎}

[gitar-bot]

💡 Edge Case: S8924 gate triggers on static imports of Mockito fields, not methods

hasStaticMockitoImport is computed from any static import whose FQN starts with org.mockito.Mockito., so a static import of a Mockito constant such as import static org.mockito.Mockito.RETURNS_DEEP_STUBS; (or CALLS_REAL_METHODS, RETURNS_SELF, …) flips the gate on. A file that consistently uses the Mockito. prefix for every call but statically imports one such constant is then reported on every prefixed call — precisely the false positive this PR removes. Restrict the gate to static imports whose simple name is one of the method names the rule tracks.

Only consider a static import relevant when it imports one of the tracked Mockito methods (or a wildcard).:

private static final Set<String> MOCKITO_METHOD_NAMES =
  Set.of("doReturn", "doThrow", "mock", "never", "spy", "times", "verify", "when");

// in collectConflictingImports:
  hasStaticMockitoImport = staticImportFqns.stream()
    .filter(fqn -> fqn.startsWith(MOCKITO_IMPORT_PREFIX))
    .map(fqn -> fqn.substring(MOCKITO_IMPORT_PREFIX.length()))
    .anyMatch(name -> "*".equals(name) || MOCKITO_METHOD_NAMES.contains(name));

• Apply fix

_{Check the box to apply the fix or reply for a change | Was this helpful? React with 👍 / 👎}

[github-actions]

❌ Ruling needs updating. A fix PR has been created: #6087

Please review and merge it into your branch.

[github-actions]

Ruling Diff Summary

Detected changes in 1 rule files: 2 issues removed, 0 issues added.

S8924 (java) on sonar-server - 2 issues removed, 0 issues added

Removed src/test/java/org/sonar/server/component/ws/TreeActionTest.java (line 88)

(source file not found at this revision: src/test/java/org/sonar/server/component/ws/TreeActionTest.java)

Removed src/test/java/org/sonar/server/language/ws/LanguageWsTest.java (line 51)

(source file not found at this revision: src/test/java/org/sonar/server/language/ws/LanguageWsTest.java)

[gitar-bot]

💡 Edge Case: New gate's "static imports present but none from Mockito" branch untested

The new file-level gate hasStaticMockitoImport has two ways to be false: no static imports at all, and static imports that exist but none resolving under org.mockito.Mockito.. MockitoStaticImportCheckNoStaticImportSample.java only has plain imports (org.junit.jupiter.api.Test, org.mockito.Mockito), so staticImportFqns is empty and the anyMatch predicate is never actually evaluated against a non-Mockito FQN; the other samples all contain import static org.mockito.Mockito.*-style imports, so the false branch with a non-empty static-import list is unexercised. Add a non-Mockito static import (e.g. import static org.junit.jupiter.api.Assertions.assertEquals; used in the test body) to the new sample so the predicate is really evaluated, and mirror the repo convention by also adding a test_no_static_import_without_semantic case using .withoutSemantic().

Add a non-Mockito static import to the sample so the gate's anyMatch predicate is exercised with a non-empty static import list (use assertEquals in the test body, e.g. assertEquals(42, service.getValue());).:

import org.junit.jupiter.api.Test;
import org.mockito.Mockito;

import static org.junit.jupiter.api.Assertions.assertEquals;

class MockitoStaticImportCheckNoStaticImportSample {

• Apply fix

_{Check the box to apply the fix or reply for a change | Was this helpful? React with 👍 / 👎}

[gitar-bot]

Code Review ⚠️ Changes requested 1 resolved / 4 findings

Refines S8924 to only flag Mockito calls when a file mixes prefixed Mockito.xxx() calls with static imports of Mockito methods, but the rule description and examples were not updated to match the new mixed-usage behavior. The gate also triggers on static imports of Mockito constants (not just methods), creating a false positive for files that consistently use the prefix but statically import one constant. Additionally, S9142's description opens with non-standard phrasing, and the new test sample does not exercise the branch where static imports exist but none are from Mockito. Update rule metadata for S8924 to document the mixed-import trigger, restrict the gate to method names only, fix S9142's lead paragraph, and add a test case with non-Mockito static imports.

⚠️ Bug: S8924 rule description contradicts the new mixed-usage behavior

📄 java-checks/src/main/java/org/sonar/java/checks/tests/MockitoStaticImportCheck.java:85 📄 java-checks/src/main/java/org/sonar/java/checks/tests/MockitoStaticImportCheck.java:109 📄 java-checks-test-sources/default/src/test/java/checks/tests/MockitoStaticImportCheckNoStaticImportSample.java:14-22

The check now only reports when the file also contains a static import of a Mockito member (hasStaticMockitoImport gate at MockitoStaticImportCheck.java:109), but S8924.html and S8924.json were not updated: the description still states "This rule raises an issue when Mockito core methods are called with the Mockito. prefix instead of being statically imported", and its Noncompliant example (import org.mockito.Mockito; with no static import, two // Noncompliant markers) is exactly the code the new MockitoStaticImportCheckNoStaticImportSample.java asserts is Compliant. Users reading the rule page will expect issues on fully-prefixed files and see none; the documented Noncompliant example no longer raises. Update the lead paragraph, the "How to fix it" text and the Noncompliant/Compliant examples to describe the mixed-import trigger, and adjust the title in S8924.json accordingly (the branch's two "Update rule metadata" commits only touched S9142).

Replace the first paragraph of S8924.html, and make the Noncompliant example contain at least one `import static org.mockito.Mockito.…` so it matches the implemented trigger; update the S8924.json title to something like "Mockito core methods should be imported consistently".

<p>This rule raises an issue when a test file mixes both styles of accessing Mockito core methods: some calls use a static import while others keep
the <code>Mockito.</code> prefix. Files that consistently use the <code>Mockito.</code> prefix everywhere are not reported.</p>

💡 Edge Case: S8924 gate triggers on static imports of Mockito fields, not methods

📄 java-checks/src/main/java/org/sonar/java/checks/tests/MockitoStaticImportCheck.java:85

hasStaticMockitoImport is computed from any static import whose FQN starts with org.mockito.Mockito., so a static import of a Mockito constant such as import static org.mockito.Mockito.RETURNS_DEEP_STUBS; (or CALLS_REAL_METHODS, RETURNS_SELF, …) flips the gate on. A file that consistently uses the Mockito. prefix for every call but statically imports one such constant is then reported on every prefixed call — precisely the false positive this PR removes. Restrict the gate to static imports whose simple name is one of the method names the rule tracks.

Only consider a static import relevant when it imports one of the tracked Mockito methods (or a wildcard).

private static final Set<String> MOCKITO_METHOD_NAMES =
  Set.of("doReturn", "doThrow", "mock", "never", "spy", "times", "verify", "when");

// in collectConflictingImports:
  hasStaticMockitoImport = staticImportFqns.stream()
    .filter(fqn -> fqn.startsWith(MOCKITO_IMPORT_PREFIX))
    .map(fqn -> fqn.substring(MOCKITO_IMPORT_PREFIX.length()))
    .anyMatch(name -> "*".equals(name) || MOCKITO_METHOD_NAMES.contains(name));

💡 Edge Case: New gate's "static imports present but none from Mockito" branch untested

📄 java-checks-test-sources/default/src/test/java/checks/tests/MockitoStaticImportCheckNoStaticImportSample.java:1-15 📄 java-checks/src/main/java/org/sonar/java/checks/tests/MockitoStaticImportCheck.java:85 📄 java-checks/src/test/java/org/sonar/java/checks/tests/MockitoStaticImportCheckTest.java:51-57

The new file-level gate hasStaticMockitoImport has two ways to be false: no static imports at all, and static imports that exist but none resolving under org.mockito.Mockito.. MockitoStaticImportCheckNoStaticImportSample.java only has plain imports (org.junit.jupiter.api.Test, org.mockito.Mockito), so staticImportFqns is empty and the anyMatch predicate is never actually evaluated against a non-Mockito FQN; the other samples all contain import static org.mockito.Mockito.*-style imports, so the false branch with a non-empty static-import list is unexercised. Add a non-Mockito static import (e.g. import static org.junit.jupiter.api.Assertions.assertEquals; used in the test body) to the new sample so the predicate is really evaluated, and mirror the repo convention by also adding a test_no_static_import_without_semantic case using .withoutSemantic().

Add a non-Mockito static import to the sample so the gate's anyMatch predicate is exercised with a non-empty static import list (use assertEquals in the test body, e.g. assertEquals(42, service.getValue());).

import org.junit.jupiter.api.Test;
import org.mockito.Mockito;

import static org.junit.jupiter.api.Assertions.assertEquals;

class MockitoStaticImportCheckNoStaticImportSample {

✅ 1 resolved

✅ Quality: S9142 description opens with non-standard "This is an issue when"

📄 sonar-java-plugin/src/main/resources/org/sonar/l10n/java/rules/java/S9142.html:1-2
The added lead paragraph reads "This is an issue when a database <code>PreparedStatement</code> object is created inside a loop…", whereas every other rule page in this module (e.g. S8924.html) opens with "This rule raises an issue when …". The sentence as written has no subject for the condition and reads as broken English on the rendered rule page.

🤖 Prompt for agents

Code Review: Refines S8924 to only flag Mockito calls when a file mixes prefixed `Mockito.xxx()` calls with static imports of Mockito methods, but the rule description and examples were not updated to match the new mixed-usage behavior. The gate also triggers on static imports of Mockito constants (not just methods), creating a false positive for files that consistently use the prefix but statically import one constant. Additionally, S9142's description opens with non-standard phrasing, and the new test sample does not exercise the branch where static imports exist but none are from Mockito. Update rule metadata for S8924 to document the mixed-import trigger, restrict the gate to method names only, fix S9142's lead paragraph, and add a test case with non-Mockito static imports.

1. ⚠️ Bug: S8924 rule description contradicts the new mixed-usage behavior
   Files: java-checks/src/main/java/org/sonar/java/checks/tests/MockitoStaticImportCheck.java:85, java-checks/src/main/java/org/sonar/java/checks/tests/MockitoStaticImportCheck.java:109, java-checks-test-sources/default/src/test/java/checks/tests/MockitoStaticImportCheckNoStaticImportSample.java:14-22

   The check now only reports when the file also contains a static import of a Mockito member (`hasStaticMockitoImport` gate at MockitoStaticImportCheck.java:109), but S8924.html and S8924.json were not updated: the description still states "This rule raises an issue when Mockito core methods are called with the `Mockito.` prefix instead of being statically imported", and its Noncompliant example (`import org.mockito.Mockito;` with no static import, two `// Noncompliant` markers) is exactly the code the new `MockitoStaticImportCheckNoStaticImportSample.java` asserts is Compliant. Users reading the rule page will expect issues on fully-prefixed files and see none; the documented Noncompliant example no longer raises. Update the lead paragraph, the "How to fix it" text and the Noncompliant/Compliant examples to describe the mixed-import trigger, and adjust the title in S8924.json accordingly (the branch's two "Update rule metadata" commits only touched S9142).

   Fix (Replace the first paragraph of S8924.html, and make the Noncompliant example contain at least one `import static org.mockito.Mockito.…` so it matches the implemented trigger; update the S8924.json title to something like "Mockito core methods should be imported consistently".):
   <p>This rule raises an issue when a test file mixes both styles of accessing Mockito core methods: some calls use a static import while others keep
   the <code>Mockito.</code> prefix. Files that consistently use the <code>Mockito.</code> prefix everywhere are not reported.</p>

2. 💡 Edge Case: S8924 gate triggers on static imports of Mockito fields, not methods
   Files: java-checks/src/main/java/org/sonar/java/checks/tests/MockitoStaticImportCheck.java:85

   `hasStaticMockitoImport` is computed from any static import whose FQN starts with `org.mockito.Mockito.`, so a static import of a Mockito constant such as `import static org.mockito.Mockito.RETURNS_DEEP_STUBS;` (or `CALLS_REAL_METHODS`, `RETURNS_SELF`, …) flips the gate on. A file that consistently uses the `Mockito.` prefix for every call but statically imports one such constant is then reported on every prefixed call — precisely the false positive this PR removes. Restrict the gate to static imports whose simple name is one of the method names the rule tracks.

   Fix (Only consider a static import relevant when it imports one of the tracked Mockito methods (or a wildcard).):
   private static final Set<String> MOCKITO_METHOD_NAMES =
     Set.of("doReturn", "doThrow", "mock", "never", "spy", "times", "verify", "when");
   
   // in collectConflictingImports:
     hasStaticMockitoImport = staticImportFqns.stream()
       .filter(fqn -> fqn.startsWith(MOCKITO_IMPORT_PREFIX))
       .map(fqn -> fqn.substring(MOCKITO_IMPORT_PREFIX.length()))
       .anyMatch(name -> "*".equals(name) || MOCKITO_METHOD_NAMES.contains(name));

3. 💡 Edge Case: New gate's "static imports present but none from Mockito" branch untested
   Files: java-checks-test-sources/default/src/test/java/checks/tests/MockitoStaticImportCheckNoStaticImportSample.java:1-15, java-checks/src/main/java/org/sonar/java/checks/tests/MockitoStaticImportCheck.java:85, java-checks/src/test/java/org/sonar/java/checks/tests/MockitoStaticImportCheckTest.java:51-57

   The new file-level gate `hasStaticMockitoImport` has two ways to be false: no static imports at all, and static imports that exist but none resolving under `org.mockito.Mockito.`. `MockitoStaticImportCheckNoStaticImportSample.java` only has plain imports (`org.junit.jupiter.api.Test`, `org.mockito.Mockito`), so `staticImportFqns` is empty and the `anyMatch` predicate is never actually evaluated against a non-Mockito FQN; the other samples all contain `import static org.mockito.Mockito.*`-style imports, so the false branch with a non-empty static-import list is unexercised. Add a non-Mockito static import (e.g. `import static org.junit.jupiter.api.Assertions.assertEquals;` used in the test body) to the new sample so the predicate is really evaluated, and mirror the repo convention by also adding a `test_no_static_import_without_semantic` case using `.withoutSemantic()`.

   Fix (Add a non-Mockito static import to the sample so the gate's anyMatch predicate is exercised with a non-empty static import list (use assertEquals in the test body, e.g. assertEquals(42, service.getValue());).):
   import org.junit.jupiter.api.Test;
   import org.mockito.Mockito;
   
   import static org.junit.jupiter.api.Assertions.assertEquals;
   
   class MockitoStaticImportCheckNoStaticImportSample {

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
100.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube