Skip to content

Commit fd627f6

Browse files
NoemieBenardNoemieBenard
authored
SONARJAVA-5708 Implement rule S8924: Mockito core methods should be imported statically (#5689)
1 parent c45edfb commit fd627f6

11 files changed

Lines changed: 380 additions & 6 deletions

File tree

‎its/autoscan/src/test/resources/autoscan/diffs/diff_S3577.json‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
{
22
"ruleKey": "S3577",
33
"hasTruePositives": true,
4-
"falseNegatives": 48,
4+
"falseNegatives": 49,
55
"falsePositives": 0
66
}

‎its/autoscan/src/test/resources/autoscan/diffs/diff_S5969.json‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
{
22
"ruleKey": "S5969",
33
"hasTruePositives": false,
4-
"falseNegatives": 7,
4+
"falseNegatives": 8,
55
"falsePositives": 0
6-
}
6+
}

‎its/autoscan/src/test/resources/autoscan/diffs/diff_S6068.json‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
{
22
"ruleKey": "S6068",
33
"hasTruePositives": false,
4-
"falseNegatives": 11,
4+
"falseNegatives": 14,
55
"falsePositives": 0
6-
}
6+
}

‎its/autoscan/src/test/resources/autoscan/diffs/diff_S8924.json‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,6 @@
1+
{
2+
"ruleKey": "S8924",
3+
"hasTruePositives": false,
4+
"falseNegatives": 35,
5+
"falsePositives": 0
6+
}

‎its/ruling/src/test/resources/sonar-server/java-S8924.json‎

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,14 @@
1+
{
2+
"org.sonarsource.sonarqube:sonar-server:src/test/java/org/sonar/ce/log/CeLoggingTest.java": [
3+
64
4+
],
5+
"org.sonarsource.sonarqube:sonar-server:src/test/java/org/sonar/server/component/ws/TreeActionTest.java": [
6+
88
7+
],
8+
"org.sonarsource.sonarqube:sonar-server:src/test/java/org/sonar/server/language/ws/LanguageWsTest.java": [
9+
51
10+
],
11+
"org.sonarsource.sonarqube:sonar-server:src/test/java/org/sonar/server/rule/ws/ShowActionTest.java": [
12+
150
13+
]
14+
}

‎java-checks-test-sources/default/src/test/java/checks/tests/MockitoStaticImportCheckSample.java‎

Lines changed: 152 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,152 @@
1+
package checks.tests;
2+
3+
import org.junit.jupiter.api.Test;
4+
import org.mockito.Mockito;
5+
6+
import static org.mockito.ArgumentMatchers.any;
7+
import static org.mockito.ArgumentMatchers.eq;
8+
import static org.mockito.Mockito.doReturn;
9+
import static org.mockito.Mockito.doThrow;
10+
import static org.mockito.Mockito.mock;
11+
import static org.mockito.Mockito.never;
12+
import static org.mockito.Mockito.spy;
13+
import static org.mockito.Mockito.times;
14+
import static org.mockito.Mockito.verify;
15+
import static org.mockito.Mockito.when;
16+
17+
class MockitoStaticImportCheckSample {
18+
19+
interface MyService {
20+
int getValue();
21+
String process(String input);
22+
}
23+
24+
@Test
25+
void noncompliant_mock() {
26+
MyService service = Mockito.mock(MyService.class); // Noncompliant {{Use a static import for "mock".}}
27+
verify(service).getValue();
28+
}
29+
30+
@Test
31+
void noncompliant_when() {
32+
MyService service = mock(MyService.class);
33+
Mockito.when(service.getValue()).thenReturn(42); // Noncompliant {{Use a static import for "when".}}
34+
verify(service).getValue();
35+
}
36+
37+
@Test
38+
void noncompliant_spy_on_instance() {
39+
MyService service = mock(MyService.class);
40+
MyService spied = Mockito.spy(service); // Noncompliant {{Use a static import for "spy".}}
41+
verify(spied).getValue();
42+
}
43+
44+
@Test
45+
void noncompliant_spy_on_class() {
46+
MyService spied = Mockito.spy(MyService.class); // Noncompliant {{Use a static import for "spy".}}
47+
verify(spied).getValue();
48+
}
49+
50+
@Test
51+
void noncompliant_doReturn() {
52+
MyService spied = spy(MyService.class);
53+
Mockito.doReturn(42).when(spied).getValue(); // Noncompliant {{Use a static import for "doReturn".}}
54+
verify(spied).getValue();
55+
}
56+
57+
@Test
58+
void noncompliant_doThrow() {
59+
MyService spied = spy(MyService.class);
60+
Mockito.doThrow(new RuntimeException()).when(spied).getValue(); // Noncompliant {{Use a static import for "doThrow".}}
61+
verify(spied).getValue();
62+
}
63+
64+
@Test
65+
void noncompliant_verify() {
66+
MyService service = mock(MyService.class);
67+
service.getValue();
68+
Mockito.verify(service).getValue(); // Noncompliant {{Use a static import for "verify".}}
69+
}
70+
71+
@Test
72+
void noncompliant_times() {
73+
MyService service = mock(MyService.class);
74+
service.getValue();
75+
service.getValue();
76+
verify(service, Mockito.times(2)).getValue(); // Noncompliant {{Use a static import for "times".}}
77+
}
78+
79+
@Test
80+
void noncompliant_never() {
81+
MyService service = mock(MyService.class);
82+
verify(service, Mockito.never()).getValue(); // Noncompliant {{Use a static import for "never".}}
83+
}
84+
85+
86+
@Test
87+
void noncompliant_verify_and_times() {
88+
MyService service = mock(MyService.class);
89+
service.getValue();
90+
service.getValue();
91+
Mockito.verify(service, // Noncompliant {{Use a static import for "verify".}}
92+
Mockito.times(2)) // Noncompliant {{Use a static import for "times".}}
93+
.getValue();
94+
}
95+
96+
@Test
97+
void noncompliant_verify_and_never() {
98+
MyService service = mock(MyService.class);
99+
Mockito.verify(service, // Noncompliant {{Use a static import for "verify".}}
100+
Mockito.never()) // Noncompliant {{Use a static import for "never".}}
101+
.getValue();
102+
}
103+
104+
@Test
105+
void noncompliant_mock_inside_when() {
106+
Mockito.when( // Noncompliant {{Use a static import for "when".}}
107+
Mockito.mock(MyService.class).getValue()) // Noncompliant {{Use a static import for "mock".}}
108+
.thenReturn(42);
109+
verify(Mockito.mock(MyService.class)).getValue(); // Noncompliant {{Use a static import for "mock".}}
110+
}
111+
112+
@Test
113+
void compliant_all_methods() {
114+
MyService service = mock(MyService.class);
115+
MyService spied = spy(service);
116+
117+
when(service.getValue()).thenReturn(42);
118+
when(service.process(any())).thenReturn("result");
119+
when(service.process(eq("input"))).thenReturn("result");
120+
121+
doReturn(42).when(spied).getValue();
122+
doThrow(new RuntimeException()).when(spied).getValue();
123+
124+
service.getValue();
125+
service.getValue();
126+
verify(service, times(2)).getValue();
127+
verify(service, never()).process(any());
128+
}
129+
130+
@Test
131+
void compliant_out_of_scope_methods() {
132+
MyService service = mock(MyService.class);
133+
MyService spied = spy(MyService.class);
134+
135+
when(service.process(Mockito.any())).thenReturn("result");
136+
when(service.process(Mockito.eq("input"))).thenReturn("result");
137+
Mockito.inOrder(service);
138+
Mockito.reset(service);
139+
Mockito.doCallRealMethod().when(spied).getValue();
140+
Mockito.doNothing().when(spied).getValue();
141+
Mockito.doAnswer(invocation -> 42).when(spied).getValue();
142+
verify(service).getValue();
143+
}
144+
145+
@Test
146+
void compliant_argument_matchers_prefix() {
147+
MyService service = mock(MyService.class);
148+
when(service.process(org.mockito.ArgumentMatchers.any())).thenReturn("result");
149+
when(service.process(org.mockito.ArgumentMatchers.eq("input"))).thenReturn("result");
150+
verify(service).getValue();
151+
}
152+
}

‎java-checks/src/main/java/org/sonar/java/checks/tests/MockitoStaticImportCheck.java‎

Lines changed: 58 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,58 @@
1+
/*
2+
* SonarQube Java
3+
* Copyright (C) SonarSource Sàrl
4+
* mailto:info AT sonarsource DOT com
5+
*
6+
* You can redistribute and/or modify this program under the terms of
7+
* the Sonar Source-Available License Version 1, as published by SonarSource Sàrl.
8+
*
9+
* This program is distributed in the hope that it will be useful,
10+
* but WITHOUT ANY WARRANTY; without even the implied warranty of
11+
* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.
12+
* See the Sonar Source-Available License for more details.
13+
*
14+
* You should have received a copy of the Sonar Source-Available License
15+
* along with this program; if not, see https://sonarsource.com/license/ssal/
16+
*/
17+
package org.sonar.java.checks.tests;
18+
19+
import java.util.Collections;
20+
import java.util.List;
21+
import org.sonar.check.Rule;
22+
import org.sonar.plugins.java.api.IssuableSubscriptionVisitor;
23+
import org.sonar.plugins.java.api.semantic.MethodMatchers;
24+
import org.sonar.plugins.java.api.tree.ExpressionTree;
25+
import org.sonar.plugins.java.api.tree.MemberSelectExpressionTree;
26+
import org.sonar.plugins.java.api.tree.MethodInvocationTree;
27+
import org.sonar.plugins.java.api.tree.Tree;
28+
29+
@Rule(key = "S8924")
30+
public class MockitoStaticImportCheck extends IssuableSubscriptionVisitor {
31+
32+
private static final String MOCKITO_CLASS = "org.mockito.Mockito";
33+
34+
private static final MethodMatchers MOCKITO_METHODS = MethodMatchers.create()
35+
.ofTypes(MOCKITO_CLASS)
36+
.names("doReturn", "doThrow", "mock", "never", "spy", "times", "verify", "when")
37+
.withAnyParameters()
38+
.build();
39+
40+
@Override
41+
public List<Tree.Kind> nodesToVisit() {
42+
return Collections.singletonList(Tree.Kind.METHOD_INVOCATION);
43+
}
44+
45+
@Override
46+
public void visitNode(Tree tree) {
47+
MethodInvocationTree mit = (MethodInvocationTree) tree;
48+
ExpressionTree methodSelect = mit.methodSelect();
49+
if (!methodSelect.is(Tree.Kind.MEMBER_SELECT)) {
50+
return;
51+
}
52+
MemberSelectExpressionTree mset = (MemberSelectExpressionTree) methodSelect;
53+
String methodName = mset.identifier().name();
54+
if (MOCKITO_METHODS.matches(mit.methodSymbol())) {
55+
reportIssue(methodSelect, "Use a static import for \"%s\".".formatted(methodName));
56+
}
57+
}
58+
}

‎java-checks/src/test/java/org/sonar/java/checks/tests/MockitoStaticImportCheckTest.java‎

Lines changed: 42 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,42 @@
1+
/*
2+
* SonarQube Java
3+
* Copyright (C) SonarSource Sàrl
4+
* mailto:info AT sonarsource DOT com
5+
*
6+
* You can redistribute and/or modify this program under the terms of
7+
* the Sonar Source-Available License Version 1, as published by SonarSource Sàrl.
8+
*
9+
* This program is distributed in the hope that it will be useful,
10+
* but WITHOUT ANY WARRANTY; without even the implied warranty of
11+
* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.
12+
* See the Sonar Source-Available License for more details.
13+
*
14+
* You should have received a copy of the Sonar Source-Available License
15+
* along with this program; if not, see https://sonarsource.com/license/ssal/
16+
*/
17+
package org.sonar.java.checks.tests;
18+
19+
import org.junit.jupiter.api.Test;
20+
import org.sonar.java.checks.verifier.CheckVerifier;
21+
22+
import static org.sonar.java.checks.verifier.TestUtils.testCodeSourcesPath;
23+
24+
class MockitoStaticImportCheckTest {
25+
26+
@Test
27+
void test() {
28+
CheckVerifier.newVerifier()
29+
.onFile(testCodeSourcesPath("checks/tests/MockitoStaticImportCheckSample.java"))
30+
.withCheck(new MockitoStaticImportCheck())
31+
.verifyIssues();
32+
}
33+
34+
@Test
35+
void test_without_semantic() {
36+
CheckVerifier.newVerifier()
37+
.onFile(testCodeSourcesPath("checks/tests/MockitoStaticImportCheckSample.java"))
38+
.withCheck(new MockitoStaticImportCheck())
39+
.withoutSemantic()
40+
.verifyNoIssues();
41+
}
42+
}

‎sonar-java-plugin/src/main/resources/org/sonar/l10n/java/rules/java/S8924.html‎

Lines changed: 76 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,76 @@
1+
<p>This rule raises an issue when Mockito core methods are called with the <code>Mockito.</code> prefix instead of being statically imported.</p>
2+
<p>This applies to commonly used Mockito methods such as:</p>
3+
<ul>
4+
<li><code>mock()</code></li>
5+
<li><code>when()</code></li>
6+
<li><code>verify()</code></li>
7+
<li><code>spy()</code></li>
8+
<li><code>doReturn()</code></li>
9+
<li><code>doThrow()</code></li>
10+
<li><code>times()</code></li>
11+
<li><code>never()</code></li>
12+
<li><code>any()</code></li>
13+
<li><code>eq()</code></li>
14+
</ul>
15+
<h2>Why is this an issue?</h2>
16+
<p>Mockito was designed with static imports in mind to create a more readable, fluent testing DSL (Domain-Specific Language). When you use the
17+
<code>Mockito.</code> prefix for method calls, the test code becomes more verbose and harder to read.</p>
18+
<p>Compare these two examples:</p>
19+
<pre>
20+
Mockito.when(Mockito.mock(MyService.class).getValue()).thenReturn(42);
21+
</pre>
22+
<p>versus:</p>
23+
<pre>
24+
when(mock(MyService.class).getValue()).thenReturn(42);
25+
</pre>
26+
<p>The second version reads more naturally, almost like plain English, which is the intent of Mockito’s API design. This readability improvement
27+
becomes even more significant in larger test files with many mock interactions.</p>
28+
<h3>What is the potential impact?</h3>
29+
<p>Using the <code>Mockito.</code> prefix reduces code readability and makes test files unnecessarily verbose.</p>
30+
<h2>How to fix it</h2>
31+
<p>Add a static import for Mockito methods at the top of your test class and remove the <code>Mockito.</code> prefix from method calls. You can import
32+
specific methods or use a wildcard import for commonly used Mockito methods.</p>
33+
<h3>Code examples</h3>
34+
<h4>Noncompliant code example</h4>
35+
<pre data-diff-id="1" data-diff-type="noncompliant">
36+
import org.mockito.Mockito;
37+
38+
public class MyServiceTest {
39+
private MyService myService = Mockito.mock(MyService.class);
40+
41+
@Test
42+
public void testBehavior() {
43+
Mockito.when(myService.getValue()).thenReturn(42); // Noncompliant
44+
45+
int result = myService.getValue();
46+
47+
Mockito.verify(myService).getValue(); // Noncompliant
48+
assertEquals(42, result);
49+
}
50+
}
51+
</pre>
52+
<h4>Compliant solution</h4>
53+
<pre data-diff-id="1" data-diff-type="compliant">
54+
import static org.mockito.Mockito.*;
55+
56+
public class MyServiceTest {
57+
private MyService myService = mock(MyService.class);
58+
59+
@Test
60+
public void testBehavior() {
61+
when(myService.getValue()).thenReturn(42);
62+
63+
int result = myService.getValue();
64+
65+
verify(myService).getValue();
66+
assertEquals(42, result);
67+
}
68+
}
69+
</pre>
70+
<h2>Resources</h2>
71+
<h3>Documentation</h3>
72+
<ul>
73+
<li>Mockito Documentation - How do I import Mockito? - <a
74+
href="https://javadoc.io/doc/org.mockito/mockito-core/latest/org/mockito/Mockito.html">Official Mockito documentation</a></li>
75+
</ul>
76+

0 commit comments

Comments
 (0)