SONARJAVA-6509 Implement S9024: @InjectMocks should be preferred over manual initialization

[NoemieBenard]

---

Summary by Gitar

• New rule implementation:
  • Added MockitoInjectMocksShouldBeUsedCheck (S9024) to flag manual instantiation of test objects when @InjectMocks could be used.
  • Implemented rule metadata and localized documentation for S9024.
• Test coverage:
  • Created MockitoInjectMocksShouldBeUsedSample covering various JUnit 4/5 scenarios and Mockito configurations.
  • Added MockitoInjectMocksShouldBeUsedCheckTest to verify the detection logic.

_{This will update automatically on new commits.}

[hashicorp-vault-sonar-prod]

SONARJAVA-6509

[gitar-bot]

Code Review ✅ Approved 1 resolved / 1 findings

Implements S9024 to enforce the use of @InjectMocks, addressing the previous issue where openMocks detection incorrectly aborted scans on bodiless setup methods. No open issues remain.

✅ 1 resolved

✅ Edge Case: openMocks detection aborts scan on first bodiless setup method

📄 java-checks/src/main/java/org/sonar/java/checks/tests/MockitoInjectMocksShouldBeUsedCheck.java:184-197
In callsOpenOrInitMocksInSetup, when a setup method has a null block the code return false immediately, abandoning the scan of any remaining setup methods. If a class has multiple @Before/@beforeeach methods and an earlier one has no body (e.g. an abstract/native override), a later method actually calling openMocks/initMocks would be missed, causing the class to be wrongly treated as not Mockito-managed and issues to be lost. Use continue to skip to the next method instead of aborting the loop.

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}

[gitar-bot]

Code Review ✅ Approved 1 resolved / 1 findings

Implements S9024 to enforce @InjectMocks over manual initialization, resolving an issue where openMocks detection prematurely aborted scans.

✅ 1 resolved

✅ Edge Case: openMocks detection aborts scan on first bodiless setup method

📄 java-checks/src/main/java/org/sonar/java/checks/tests/MockitoInjectMocksShouldBeUsedCheck.java:184-197
In callsOpenOrInitMocksInSetup, when a setup method has a null block the code return false immediately, abandoning the scan of any remaining setup methods. If a class has multiple @Before/@beforeeach methods and an earlier one has no body (e.g. an abstract/native override), a later method actually calling openMocks/initMocks would be missed, causing the class to be wrongly treated as not Mockito-managed and issues to be lost. Use continue to skip to the next method instead of aborting the loop.

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}

[sonarqube-next]

Quality Gate passed

Issues
0 New issues
0 Fixed issues
0 Accepted issues

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

See analysis details on SonarQube

[rombirli]

Looks good to me overall: the implementation is sound and the rule itself makes sense. One thing we could still improve in a follow-up is covering more edge cases in the implementation.

[rombirli]

This sample is a bit narrow because every case revolves around OrderProcessor, which is an easy @InjectMocks target. It would be useful to add another constructed class with less straightforward constructors, for example one mixing mocks with non-mock/primitive parameters, to cover edge cases beyond the happy path.

[rombirli]

This is purely argument-based and does not inspect constructor injectability. A constructor taking two parameters of the same mock type still matches here, even though @InjectMocks resolution is ambiguous/unreliable in that case.

for example :

  static class SameTypeDependenciesOrderProcessor {
    SameTypeDependenciesOrderProcessor(PaymentGateway primaryGateway, PaymentGateway backupGateway) {}
  }

[rombirli]

This only checks that we are assigning a new instance to a field; it never verifies that the constructed type is actually a valid @InjectMocks target. For example, automatic @InjectMocks instantiation fails on non-static inner classes because Mockito cannot implicitly resolve the required enclosing outer instance. We could filter these out.