diff --git a/its/autoscan/src/test/resources/autoscan/diffs/diff_S2699.json b/its/autoscan/src/test/resources/autoscan/diffs/diff_S2699.json index 039237eaa74..7d9c0e972b0 100644 --- a/its/autoscan/src/test/resources/autoscan/diffs/diff_S2699.json +++ b/its/autoscan/src/test/resources/autoscan/diffs/diff_S2699.json @@ -1,6 +1,6 @@ { "ruleKey": "S2699", "hasTruePositives": true, - "falseNegatives": 171, + "falseNegatives": 174, "falsePositives": 1 } 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 f6f9d38e209..4163ede9e59 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": 50, + "falseNegatives": 51, "falsePositives": 0 } diff --git a/its/autoscan/src/test/resources/autoscan/diffs/diff_S9016.json b/its/autoscan/src/test/resources/autoscan/diffs/diff_S9016.json new file mode 100644 index 00000000000..fffd6893eb7 --- /dev/null +++ b/its/autoscan/src/test/resources/autoscan/diffs/diff_S9016.json @@ -0,0 +1,6 @@ +{ + "ruleKey": "S9016", + "hasTruePositives": false, + "falseNegatives": 1, + "falsePositives": 0 +} diff --git a/its/ruling/src/test/resources/sonar-server/java-S9016.json b/its/ruling/src/test/resources/sonar-server/java-S9016.json new file mode 100644 index 00000000000..95ee2606bec --- /dev/null +++ b/its/ruling/src/test/resources/sonar-server/java-S9016.json @@ -0,0 +1,21 @@ +{ +"org.sonarsource.sonarqube:sonar-server:src/test/java/org/sonar/server/app/TomcatContextsTest.java": [ +60 +], +"org.sonarsource.sonarqube:sonar-server:src/test/java/org/sonar/server/authentication/OAuth2CallbackFilterTest.java": [ +75 +], +"org.sonarsource.sonarqube:sonar-server:src/test/java/org/sonar/server/issue/ws/ComponentTagsActionTest.java": [ +93, +117 +], +"org.sonarsource.sonarqube:sonar-server:src/test/java/org/sonar/server/plugins/StaticResourcesServletTest.java": [ +145 +], +"org.sonarsource.sonarqube:sonar-server:src/test/java/org/sonar/server/rule/RegisterRulesTest.java": [ +486 +], +"org.sonarsource.sonarqube:sonar-server:src/test/java/org/sonar/server/rule/WebServerRuleFinderImplTest.java": [ +41 +] +} diff --git a/java-checks-test-sources/default/src/test/java/checks/tests/MockitoInlineMockInThenReturnCheckSample.java b/java-checks-test-sources/default/src/test/java/checks/tests/MockitoInlineMockInThenReturnCheckSample.java new file mode 100644 index 00000000000..43eb1156401 --- /dev/null +++ b/java-checks-test-sources/default/src/test/java/checks/tests/MockitoInlineMockInThenReturnCheckSample.java @@ -0,0 +1,39 @@ +package checks.tests; + +import org.junit.jupiter.api.Test; + +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.when; + +class MockitoInlineMockInThenReturnCheckSample { + + interface Foo { + String method(); + } + + class MyClass { + Foo foo() { + return null; + } + } + + @Test + void noncompliantInlineMock() { + MyClass myMock = mock(MyClass.class); + when(myMock.foo()).thenReturn(mock(Foo.class)); // Noncompliant {{Extract this mock creation to a local variable.}} + // ^^^^^^^^^^^^^^^ + } + + @Test + void compliantExtractedMock() { + MyClass myMock = mock(MyClass.class); + Foo fooMock = mock(Foo.class); + when(myMock.foo()).thenReturn(fooMock); // Compliant + } + + @Test + void compliantLiteralArg() { + MyClass myMock = mock(MyClass.class); + when(myMock.foo()).thenReturn(null); // Compliant + } +} diff --git a/java-checks/src/main/java/org/sonar/java/checks/tests/MockitoInlineMockInThenReturnCheck.java b/java-checks/src/main/java/org/sonar/java/checks/tests/MockitoInlineMockInThenReturnCheck.java new file mode 100644 index 00000000000..b3e2654b1fe --- /dev/null +++ b/java-checks/src/main/java/org/sonar/java/checks/tests/MockitoInlineMockInThenReturnCheck.java @@ -0,0 +1,67 @@ +/* + * 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.sonar.check.Rule; +import org.sonar.plugins.java.api.JavaFileScanner; +import org.sonar.plugins.java.api.JavaFileScannerContext; +import org.sonar.plugins.java.api.semantic.MethodMatchers; +import org.sonar.plugins.java.api.tree.BaseTreeVisitor; +import org.sonar.plugins.java.api.tree.ExpressionTree; +import org.sonar.plugins.java.api.tree.MethodInvocationTree; +import org.sonar.plugins.java.api.tree.Tree; + +@Rule(key = "S9016") +public class MockitoInlineMockInThenReturnCheck extends BaseTreeVisitor implements JavaFileScanner { + + private static final String MESSAGE = "Extract this mock creation to a local variable."; + + private static final MethodMatchers THEN_RETURN = MethodMatchers.create() + .ofSubTypes("org.mockito.stubbing.OngoingStubbing") + .names("thenReturn") + .withAnyParameters() + .build(); + + private static final MethodMatchers MOCKITO_MOCK = MethodMatchers.create() + .ofTypes("org.mockito.Mockito") + .names("mock") + .withAnyParameters() + .build(); + + private JavaFileScannerContext context; + + @Override + public void scanFile(JavaFileScannerContext context) { + if (context.getSemanticModel() == null) { + return; + } + this.context = context; + scan(context.getTree()); + } + + @Override + public void visitMethodInvocation(MethodInvocationTree mit) { + if (THEN_RETURN.matches(mit)) { + for (ExpressionTree arg : mit.arguments()) { + if (arg.is(Tree.Kind.METHOD_INVOCATION) && MOCKITO_MOCK.matches((MethodInvocationTree) arg)) { + context.reportIssue(this, arg, MESSAGE); + } + } + } + super.visitMethodInvocation(mit); + } +} diff --git a/java-checks/src/test/java/org/sonar/java/checks/tests/MockitoInlineMockInThenReturnCheckTest.java b/java-checks/src/test/java/org/sonar/java/checks/tests/MockitoInlineMockInThenReturnCheckTest.java new file mode 100644 index 00000000000..bd7fcc87f65 --- /dev/null +++ b/java-checks/src/test/java/org/sonar/java/checks/tests/MockitoInlineMockInThenReturnCheckTest.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 MockitoInlineMockInThenReturnCheckTest { + + @Test + void test() { + CheckVerifier.newVerifier() + .onFile(testCodeSourcesPath("checks/tests/MockitoInlineMockInThenReturnCheckSample.java")) + .withCheck(new MockitoInlineMockInThenReturnCheck()) + .verifyIssues(); + } + + @Test + void testWithoutSemantics() { + CheckVerifier.newVerifier() + .onFile(testCodeSourcesPath("checks/tests/MockitoInlineMockInThenReturnCheckSample.java")) + .withCheck(new MockitoInlineMockInThenReturnCheck()) + .withoutSemantic() + .verifyNoIssues(); + } +} diff --git a/sonar-java-plugin/src/main/resources/org/sonar/l10n/java/rules/java/S9016.html b/sonar-java-plugin/src/main/resources/org/sonar/l10n/java/rules/java/S9016.html new file mode 100644 index 00000000000..aba011b3f81 --- /dev/null +++ b/sonar-java-plugin/src/main/resources/org/sonar/l10n/java/rules/java/S9016.html @@ -0,0 +1,61 @@ +

This is an issue when using mocking framework stubbing syntax to specify return values and creating a mock object directly as an argument to the +return value specification method.

+

In Java with Mockito, this specifically refers to inlining the mock creation inside the thenReturn() method.

+

Why is this an issue?

+

Mocking frameworks use an internal state machine to track stubbing operations and validate that they are completed correctly. When you create a +mock object inline within a return value specification, the framework registers the mock creation as a new event before it finishes recording the +stubbing chain.

+

This sequence confuses the framework’s validation mechanism. The framework cannot properly detect whether the previous stubbing operation was +finished or left incomplete. This breaks one of the framework’s key safety features: detecting unfinished stubbing.

+

Unfinished stubbing detection helps catch common mistakes like:

+ +

When inline mock creation prevents this detection, you may encounter confusing error messages in subsequent tests rather than in the test +that contains the actual problem. This makes debugging significantly harder because the error location and the root cause are separated.

+

What is the potential impact?

+

When this pattern is used, the testing framework’s validation system is compromised. The immediate consequence is that developers lose the safety +net that detects incomplete or improperly configured test doubles.

+

This can lead to:

+ +

In Mockito, this pattern specifically breaks the framework’s ability to detect unfinished stubbing, which is one of its key safety features.

+

How to fix it

+

Extract the mock object creation to a local variable, then pass that variable to thenReturn(). This ensures Mockito can properly track +the stubbing sequence and validate that it is complete.

+

Code examples

+

Noncompliant code example

+
+import static org.mockito.Mockito.*;
+
+class MyTest {
+    void testMethod() {
+        MyClass mock = mock(MyClass.class);
+        when(mock.foo()).thenReturn(mock(Foo.class)); // Noncompliant
+    }
+}
+
+

Compliant solution

+
+import static org.mockito.Mockito.*;
+
+class MyTest {
+    void testMethod() {
+        MyClass mock = mock(MyClass.class);
+        Foo foo = mock(Foo.class);
+        when(mock.foo()).thenReturn(foo);
+    }
+}
+
+

Resources

+

Documentation

+ + diff --git a/sonar-java-plugin/src/main/resources/org/sonar/l10n/java/rules/java/S9016.json b/sonar-java-plugin/src/main/resources/org/sonar/l10n/java/rules/java/S9016.json new file mode 100644 index 00000000000..a2105563c64 --- /dev/null +++ b/sonar-java-plugin/src/main/resources/org/sonar/l10n/java/rules/java/S9016.json @@ -0,0 +1,24 @@ +{ + "title": "Mock objects should not be created inline within \"thenReturn()\"", + "type": "CODE_SMELL", + "status": "ready", + "remediation": { + "func": "Constant\/Issue", + "constantCost": "5min" + }, + "tags": [ + "mockito", + "tests" + ], + "defaultSeverity": "Major", + "ruleSpecification": "RSPEC-9016", + "sqKey": "S9016", + "scope": "Tests", + "quickfix": "unknown", + "code": { + "impacts": { + "MAINTAINABILITY": "MEDIUM" + }, + "attribute": "CLEAR" + } +} diff --git a/sonar-java-plugin/src/main/resources/profiles/Sonar_way/S9016 b/sonar-java-plugin/src/main/resources/profiles/Sonar_way/S9016 new file mode 100644 index 00000000000..e69de29bb2d