From b93b11f33ab6907b6f890b965d79460122f869e4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?No=C3=A9mie=20B=C3=A9nard?= Date: Wed, 15 Jul 2026 11:36:26 +0200 Subject: [PATCH 1/3] implement check --- ...kitoInlineMockInThenReturnCheckSample.java | 39 +++++++++++ .../MockitoInlineMockInThenReturnCheck.java | 67 +++++++++++++++++++ ...ockitoInlineMockInThenReturnCheckTest.java | 42 ++++++++++++ .../org/sonar/l10n/java/rules/java/S9016.html | 62 +++++++++++++++++ .../org/sonar/l10n/java/rules/java/S9016.json | 25 +++++++ .../main/resources/profiles/Sonar_way/S9016 | 0 6 files changed, 235 insertions(+) create mode 100644 java-checks-test-sources/default/src/test/java/checks/tests/MockitoInlineMockInThenReturnCheckSample.java create mode 100644 java-checks/src/main/java/org/sonar/java/checks/tests/MockitoInlineMockInThenReturnCheck.java create mode 100644 java-checks/src/test/java/org/sonar/java/checks/tests/MockitoInlineMockInThenReturnCheckTest.java create mode 100644 sonar-java-plugin/src/main/resources/org/sonar/l10n/java/rules/java/S9016.html create mode 100644 sonar-java-plugin/src/main/resources/org/sonar/l10n/java/rules/java/S9016.json create mode 100644 sonar-java-plugin/src/main/resources/profiles/Sonar_way/S9016 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..da5f724d300 --- /dev/null +++ b/sonar-java-plugin/src/main/resources/org/sonar/l10n/java/rules/java/S9016.html @@ -0,0 +1,62 @@ +

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 the when().thenReturn() pattern.

+

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..b70d13c4d7d --- /dev/null +++ b/sonar-java-plugin/src/main/resources/org/sonar/l10n/java/rules/java/S9016.json @@ -0,0 +1,25 @@ +{ + "title": "Mock objects should not be created inline within \"thenReturn()\"", + "type": "CODE_SMELL", + "status": "ready", + "remediation": { + "func": "Constant\/Issue", + "constantCost": "5min" + }, + "tags": [ + "mockito", + "tests", + "confusing" + ], + "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 From c0df184cc3be932682fd22fe329a4404ad42c211 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?No=C3=A9mie=20B=C3=A9nard?= Date: Wed, 15 Jul 2026 13:44:34 +0200 Subject: [PATCH 2/3] fix autoscan and ruling test --- .../resources/autoscan/diffs/diff_S2699.json | 2 +- .../resources/autoscan/diffs/diff_S3577.json | 2 +- .../resources/autoscan/diffs/diff_S9016.json | 6 ++++++ .../resources/sonar-server/java-S9016.json | 21 +++++++++++++++++++ 4 files changed, 29 insertions(+), 2 deletions(-) create mode 100644 its/autoscan/src/test/resources/autoscan/diffs/diff_S9016.json create mode 100644 its/ruling/src/test/resources/sonar-server/java-S9016.json 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 +] +} From 7b6e4c95fb4a0b845a530e0efda93738365667ab Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?No=C3=A9mie=20B=C3=A9nard?= Date: Wed, 15 Jul 2026 14:41:37 +0200 Subject: [PATCH 3/3] update rule metadata --- .../main/resources/org/sonar/l10n/java/rules/java/S9016.html | 5 ++--- .../main/resources/org/sonar/l10n/java/rules/java/S9016.json | 3 +-- 2 files changed, 3 insertions(+), 5 deletions(-) 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 index da5f724d300..aba011b3f81 100644 --- 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 @@ -1,6 +1,6 @@

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 the when().thenReturn() pattern.

+

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 @@ -20,8 +20,7 @@

What is the potential impact?

net that detects incomplete or improperly configured test doubles.

This can lead to:

    -
  • Test failures appearing in unrelated tests, making debugging time-consuming
    • -
  • Intermittent test behavior that is difficult to reproduce
    • +
  • Test failures appearing in the current or following tests, making debugging time-consuming
  • Misleading error messages that point to the wrong location
  • Reduced confidence in the test suite
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 index b70d13c4d7d..a2105563c64 100644 --- 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 @@ -8,8 +8,7 @@ }, "tags": [ "mockito", - "tests", - "confusing" + "tests" ], "defaultSeverity": "Major", "ruleSpecification": "RSPEC-9016",