diff --git a/its/autoscan/src/test/resources/autoscan/diffs/diff_S3577.json b/its/autoscan/src/test/resources/autoscan/diffs/diff_S3577.json index 0095bde9c30..6b9a0072838 100644 --- a/its/autoscan/src/test/resources/autoscan/diffs/diff_S3577.json +++ b/its/autoscan/src/test/resources/autoscan/diffs/diff_S3577.json @@ -1,6 +1,6 @@ { "ruleKey": "S3577", "hasTruePositives": true, - "falseNegatives": 48, + "falseNegatives": 49, "falsePositives": 0 } diff --git a/its/autoscan/src/test/resources/autoscan/diffs/diff_S5969.json b/its/autoscan/src/test/resources/autoscan/diffs/diff_S5969.json index a7b33df96ea..5f632d8d793 100644 --- a/its/autoscan/src/test/resources/autoscan/diffs/diff_S5969.json +++ b/its/autoscan/src/test/resources/autoscan/diffs/diff_S5969.json @@ -1,6 +1,6 @@ { "ruleKey": "S5969", "hasTruePositives": false, - "falseNegatives": 7, + "falseNegatives": 8, "falsePositives": 0 -} \ No newline at end of file +} diff --git a/its/autoscan/src/test/resources/autoscan/diffs/diff_S6068.json b/its/autoscan/src/test/resources/autoscan/diffs/diff_S6068.json index 3f3a9ada7b6..ee1691b71a0 100644 --- a/its/autoscan/src/test/resources/autoscan/diffs/diff_S6068.json +++ b/its/autoscan/src/test/resources/autoscan/diffs/diff_S6068.json @@ -1,6 +1,6 @@ { "ruleKey": "S6068", "hasTruePositives": false, - "falseNegatives": 11, + "falseNegatives": 14, "falsePositives": 0 -} \ No newline at end of file +} diff --git a/its/autoscan/src/test/resources/autoscan/diffs/diff_S8924.json b/its/autoscan/src/test/resources/autoscan/diffs/diff_S8924.json new file mode 100644 index 00000000000..0a2f84696bf --- /dev/null +++ b/its/autoscan/src/test/resources/autoscan/diffs/diff_S8924.json @@ -0,0 +1,6 @@ +{ + "ruleKey": "S8924", + "hasTruePositives": false, + "falseNegatives": 35, + "falsePositives": 0 +} diff --git a/its/ruling/src/test/resources/sonar-server/java-S8924.json b/its/ruling/src/test/resources/sonar-server/java-S8924.json new file mode 100644 index 00000000000..7998d026310 --- /dev/null +++ b/its/ruling/src/test/resources/sonar-server/java-S8924.json @@ -0,0 +1,14 @@ +{ + "org.sonarsource.sonarqube:sonar-server:src/test/java/org/sonar/ce/log/CeLoggingTest.java": [ + 64 + ], + "org.sonarsource.sonarqube:sonar-server:src/test/java/org/sonar/server/component/ws/TreeActionTest.java": [ + 88 + ], + "org.sonarsource.sonarqube:sonar-server:src/test/java/org/sonar/server/language/ws/LanguageWsTest.java": [ + 51 + ], + "org.sonarsource.sonarqube:sonar-server:src/test/java/org/sonar/server/rule/ws/ShowActionTest.java": [ + 150 + ] +} diff --git a/java-checks-test-sources/default/src/test/java/checks/tests/MockitoStaticImportCheckSample.java b/java-checks-test-sources/default/src/test/java/checks/tests/MockitoStaticImportCheckSample.java new file mode 100644 index 00000000000..162670661f4 --- /dev/null +++ b/java-checks-test-sources/default/src/test/java/checks/tests/MockitoStaticImportCheckSample.java @@ -0,0 +1,152 @@ +package checks.tests; + +import org.junit.jupiter.api.Test; +import org.mockito.Mockito; + +import static org.mockito.ArgumentMatchers.any; +import static org.mockito.ArgumentMatchers.eq; +import static org.mockito.Mockito.doReturn; +import static org.mockito.Mockito.doThrow; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.never; +import static org.mockito.Mockito.spy; +import static org.mockito.Mockito.times; +import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.when; + +class MockitoStaticImportCheckSample { + + interface MyService { + int getValue(); + String process(String input); + } + + @Test + void noncompliant_mock() { + MyService service = Mockito.mock(MyService.class); // Noncompliant {{Use a static import for "mock".}} + verify(service).getValue(); + } + + @Test + void noncompliant_when() { + MyService service = mock(MyService.class); + Mockito.when(service.getValue()).thenReturn(42); // Noncompliant {{Use a static import for "when".}} + verify(service).getValue(); + } + + @Test + void noncompliant_spy_on_instance() { + MyService service = mock(MyService.class); + MyService spied = Mockito.spy(service); // Noncompliant {{Use a static import for "spy".}} + verify(spied).getValue(); + } + + @Test + void noncompliant_spy_on_class() { + MyService spied = Mockito.spy(MyService.class); // Noncompliant {{Use a static import for "spy".}} + verify(spied).getValue(); + } + + @Test + void noncompliant_doReturn() { + MyService spied = spy(MyService.class); + Mockito.doReturn(42).when(spied).getValue(); // Noncompliant {{Use a static import for "doReturn".}} + verify(spied).getValue(); + } + + @Test + void noncompliant_doThrow() { + MyService spied = spy(MyService.class); + Mockito.doThrow(new RuntimeException()).when(spied).getValue(); // Noncompliant {{Use a static import for "doThrow".}} + verify(spied).getValue(); + } + + @Test + void noncompliant_verify() { + MyService service = mock(MyService.class); + service.getValue(); + Mockito.verify(service).getValue(); // Noncompliant {{Use a static import for "verify".}} + } + + @Test + void noncompliant_times() { + MyService service = mock(MyService.class); + service.getValue(); + service.getValue(); + verify(service, Mockito.times(2)).getValue(); // Noncompliant {{Use a static import for "times".}} + } + + @Test + void noncompliant_never() { + MyService service = mock(MyService.class); + verify(service, Mockito.never()).getValue(); // Noncompliant {{Use a static import for "never".}} + } + + + @Test + void noncompliant_verify_and_times() { + MyService service = mock(MyService.class); + service.getValue(); + service.getValue(); + Mockito.verify(service, // Noncompliant {{Use a static import for "verify".}} + Mockito.times(2)) // Noncompliant {{Use a static import for "times".}} + .getValue(); + } + + @Test + void noncompliant_verify_and_never() { + MyService service = mock(MyService.class); + Mockito.verify(service, // Noncompliant {{Use a static import for "verify".}} + Mockito.never()) // Noncompliant {{Use a static import for "never".}} + .getValue(); + } + + @Test + void noncompliant_mock_inside_when() { + Mockito.when( // Noncompliant {{Use a static import for "when".}} + Mockito.mock(MyService.class).getValue()) // Noncompliant {{Use a static import for "mock".}} + .thenReturn(42); + verify(Mockito.mock(MyService.class)).getValue(); // Noncompliant {{Use a static import for "mock".}} + } + + @Test + void compliant_all_methods() { + MyService service = mock(MyService.class); + MyService spied = spy(service); + + when(service.getValue()).thenReturn(42); + when(service.process(any())).thenReturn("result"); + when(service.process(eq("input"))).thenReturn("result"); + + doReturn(42).when(spied).getValue(); + doThrow(new RuntimeException()).when(spied).getValue(); + + service.getValue(); + service.getValue(); + verify(service, times(2)).getValue(); + verify(service, never()).process(any()); + } + + @Test + void compliant_out_of_scope_methods() { + MyService service = mock(MyService.class); + MyService spied = spy(MyService.class); + + when(service.process(Mockito.any())).thenReturn("result"); + when(service.process(Mockito.eq("input"))).thenReturn("result"); + Mockito.inOrder(service); + Mockito.reset(service); + Mockito.doCallRealMethod().when(spied).getValue(); + Mockito.doNothing().when(spied).getValue(); + Mockito.doAnswer(invocation -> 42).when(spied).getValue(); + verify(service).getValue(); + } + + @Test + void compliant_argument_matchers_prefix() { + MyService service = mock(MyService.class); + when(service.process(org.mockito.ArgumentMatchers.any())).thenReturn("result"); + when(service.process(org.mockito.ArgumentMatchers.eq("input"))).thenReturn("result"); + verify(service).getValue(); + } +} diff --git a/java-checks/src/main/java/org/sonar/java/checks/tests/MockitoStaticImportCheck.java b/java-checks/src/main/java/org/sonar/java/checks/tests/MockitoStaticImportCheck.java new file mode 100644 index 00000000000..4ab7c3e8867 --- /dev/null +++ b/java-checks/src/main/java/org/sonar/java/checks/tests/MockitoStaticImportCheck.java @@ -0,0 +1,58 @@ +/* + * SonarQube Java + * Copyright (C) SonarSource Sàrl + * mailto:info AT sonarsource DOT com + * + * You can redistribute and/or modify this program under the terms of + * the Sonar Source-Available License Version 1, as published by SonarSource Sàrl. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. + * See the Sonar Source-Available License for more details. + * + * You should have received a copy of the Sonar Source-Available License + * along with this program; if not, see https://sonarsource.com/license/ssal/ + */ +package org.sonar.java.checks.tests; + +import java.util.Collections; +import java.util.List; +import org.sonar.check.Rule; +import org.sonar.plugins.java.api.IssuableSubscriptionVisitor; +import org.sonar.plugins.java.api.semantic.MethodMatchers; +import org.sonar.plugins.java.api.tree.ExpressionTree; +import org.sonar.plugins.java.api.tree.MemberSelectExpressionTree; +import org.sonar.plugins.java.api.tree.MethodInvocationTree; +import org.sonar.plugins.java.api.tree.Tree; + +@Rule(key = "S8924") +public class MockitoStaticImportCheck extends IssuableSubscriptionVisitor { + + private static final String MOCKITO_CLASS = "org.mockito.Mockito"; + + private static final MethodMatchers MOCKITO_METHODS = MethodMatchers.create() + .ofTypes(MOCKITO_CLASS) + .names("doReturn", "doThrow", "mock", "never", "spy", "times", "verify", "when") + .withAnyParameters() + .build(); + + @Override + public List nodesToVisit() { + return Collections.singletonList(Tree.Kind.METHOD_INVOCATION); + } + + @Override + public void visitNode(Tree tree) { + MethodInvocationTree mit = (MethodInvocationTree) tree; + ExpressionTree methodSelect = mit.methodSelect(); + if (!methodSelect.is(Tree.Kind.MEMBER_SELECT)) { + return; + } + MemberSelectExpressionTree mset = (MemberSelectExpressionTree) methodSelect; + String methodName = mset.identifier().name(); + if (MOCKITO_METHODS.matches(mit.methodSymbol())) { + reportIssue(methodSelect, "Use a static import for \"%s\".".formatted(methodName)); + } + } +} diff --git a/java-checks/src/test/java/org/sonar/java/checks/tests/MockitoStaticImportCheckTest.java b/java-checks/src/test/java/org/sonar/java/checks/tests/MockitoStaticImportCheckTest.java new file mode 100644 index 00000000000..d92a9634906 --- /dev/null +++ b/java-checks/src/test/java/org/sonar/java/checks/tests/MockitoStaticImportCheckTest.java @@ -0,0 +1,42 @@ +/* + * SonarQube Java + * Copyright (C) SonarSource Sàrl + * mailto:info AT sonarsource DOT com + * + * You can redistribute and/or modify this program under the terms of + * the Sonar Source-Available License Version 1, as published by SonarSource Sàrl. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. + * See the Sonar Source-Available License for more details. + * + * You should have received a copy of the Sonar Source-Available License + * along with this program; if not, see https://sonarsource.com/license/ssal/ + */ +package org.sonar.java.checks.tests; + +import org.junit.jupiter.api.Test; +import org.sonar.java.checks.verifier.CheckVerifier; + +import static org.sonar.java.checks.verifier.TestUtils.testCodeSourcesPath; + +class MockitoStaticImportCheckTest { + + @Test + void test() { + CheckVerifier.newVerifier() + .onFile(testCodeSourcesPath("checks/tests/MockitoStaticImportCheckSample.java")) + .withCheck(new MockitoStaticImportCheck()) + .verifyIssues(); + } + + @Test + void test_without_semantic() { + CheckVerifier.newVerifier() + .onFile(testCodeSourcesPath("checks/tests/MockitoStaticImportCheckSample.java")) + .withCheck(new MockitoStaticImportCheck()) + .withoutSemantic() + .verifyNoIssues(); + } +} diff --git a/sonar-java-plugin/src/main/resources/org/sonar/l10n/java/rules/java/S8924.html b/sonar-java-plugin/src/main/resources/org/sonar/l10n/java/rules/java/S8924.html new file mode 100644 index 00000000000..8d879b42e5f --- /dev/null +++ b/sonar-java-plugin/src/main/resources/org/sonar/l10n/java/rules/java/S8924.html @@ -0,0 +1,76 @@ +

This rule raises an issue when Mockito core methods are called with the Mockito. prefix instead of being statically imported.

+

This applies to commonly used Mockito methods such as:

+ +

Why is this an issue?

+

Mockito was designed with static imports in mind to create a more readable, fluent testing DSL (Domain-Specific Language). When you use the +Mockito. prefix for method calls, the test code becomes more verbose and harder to read.

+

Compare these two examples:

+
+Mockito.when(Mockito.mock(MyService.class).getValue()).thenReturn(42);
+
+

versus:

+
+when(mock(MyService.class).getValue()).thenReturn(42);
+
+

The second version reads more naturally, almost like plain English, which is the intent of Mockito’s API design. This readability improvement +becomes even more significant in larger test files with many mock interactions.

+

What is the potential impact?

+

Using the Mockito. prefix reduces code readability and makes test files unnecessarily verbose.

+

How to fix it

+

Add a static import for Mockito methods at the top of your test class and remove the Mockito. prefix from method calls. You can import +specific methods or use a wildcard import for commonly used Mockito methods.

+

Code examples

+

Noncompliant code example

+
+import org.mockito.Mockito;
+
+public class MyServiceTest {
+    private MyService myService = Mockito.mock(MyService.class);
+
+    @Test
+    public void testBehavior() {
+        Mockito.when(myService.getValue()).thenReturn(42); // Noncompliant
+
+        int result = myService.getValue();
+
+        Mockito.verify(myService).getValue(); // Noncompliant
+        assertEquals(42, result);
+    }
+}
+
+

Compliant solution

+
+import static org.mockito.Mockito.*;
+
+public class MyServiceTest {
+    private MyService myService = mock(MyService.class);
+
+    @Test
+    public void testBehavior() {
+        when(myService.getValue()).thenReturn(42);
+
+        int result = myService.getValue();
+
+        verify(myService).getValue();
+        assertEquals(42, result);
+    }
+}
+
+

Resources

+

Documentation

+ + diff --git a/sonar-java-plugin/src/main/resources/org/sonar/l10n/java/rules/java/S8924.json b/sonar-java-plugin/src/main/resources/org/sonar/l10n/java/rules/java/S8924.json new file mode 100644 index 00000000000..18d1e1fc381 --- /dev/null +++ b/sonar-java-plugin/src/main/resources/org/sonar/l10n/java/rules/java/S8924.json @@ -0,0 +1,25 @@ +{ + "title": "Mockito core methods should be statically imported", + "type": "CODE_SMELL", + "status": "ready", + "remediation": { + "func": "Constant\/Issue", + "constantCost": "5 min" + }, + "tags": [ + "mockito", + "tests", + "convention" + ], + "defaultSeverity": "Major", + "ruleSpecification": "RSPEC-8924", + "sqKey": "S8924", + "scope": "Tests", + "quickfix": "targeted", + "code": { + "impacts": { + "MAINTAINABILITY": "MEDIUM" + }, + "attribute": "CLEAR" + } +} diff --git a/sonar-java-plugin/src/main/resources/org/sonar/l10n/java/rules/java/Sonar_way_profile.json b/sonar-java-plugin/src/main/resources/org/sonar/l10n/java/rules/java/Sonar_way_profile.json index 9c58ec16ec8..519e0c1058b 100644 --- a/sonar-java-plugin/src/main/resources/org/sonar/l10n/java/rules/java/Sonar_way_profile.json +++ b/sonar-java-plugin/src/main/resources/org/sonar/l10n/java/rules/java/Sonar_way_profile.json @@ -531,6 +531,7 @@ "S8714", "S8715", "S8745", - "S8786" + "S8786", + "S8924" ] }