Skip to content

Commit 6d50d03

Browse files
rombirligithub-actions[bot]
authored and
sonartech
committed
SONARPY-4417 Implement S9078: Parametrize decorators should not contain duplicate test cases
Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com> GitOrigin-RevId: 4f4df394abe306baef3d438dd63feab8815e2382
1 parent dbba186 commit 6d50d03

10 files changed

Lines changed: 430 additions & 12 deletions

File tree

‎python-checks/src/main/java/org/sonar/python/checks/OpenSourceCheckList.java‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -99,6 +99,7 @@
9999
import org.sonar.python.checks.tests.DedicatedAssertionCheck;
100100
import org.sonar.python.checks.tests.DedicatedExceptionAssertionCheck;
101101
import org.sonar.python.checks.tests.DeprecatedYieldFixtureCheck;
102+
import org.sonar.python.checks.tests.DuplicateParametrizeCasesCheck;
102103
import org.sonar.python.checks.tests.EmptyParametrizeValuesCheck;
103104
import org.sonar.python.checks.tests.FixtureParamDependenciesCheck;
104105
import org.sonar.python.checks.tests.GroupSimilarTestsParameterizedCheck;
@@ -208,6 +209,7 @@ public Stream<Class<?>> getChecks() {
208209
DebugModeCheck.class,
209210
DedicatedAssertionCheck.class,
210211
DedicatedExceptionAssertionCheck.class,
212+
DuplicateParametrizeCasesCheck.class,
211213
EmptyParametrizeValuesCheck.class,
212214
DefaultFactoryArgumentCheck.class,
213215
DeprecatedNumpyTypesCheck.class,

‎python-checks/src/main/java/org/sonar/python/checks/tests/DuplicateParametrizeCasesCheck.java‎

Lines changed: 78 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,78 @@
1+
/*
2+
* SonarQube Python Plugin
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.python.checks.tests;
18+
19+
import java.util.ArrayList;
20+
import java.util.List;
21+
import org.sonar.check.Rule;
22+
import org.sonar.plugins.python.api.PythonSubscriptionCheck;
23+
import org.sonar.plugins.python.api.SubscriptionContext;
24+
import org.sonar.plugins.python.api.tree.Decorator;
25+
import org.sonar.plugins.python.api.tree.Expression;
26+
import org.sonar.plugins.python.api.tree.Tree;
27+
import org.sonar.python.checks.utils.CheckUtils;
28+
import org.sonar.python.checks.utils.Expressions;
29+
import org.sonar.python.checks.utils.UnittestUtils;
30+
31+
@Rule(key = "S9078")
32+
public class DuplicateParametrizeCasesCheck extends PythonSubscriptionCheck {
33+
34+
private static final String MESSAGE = "Remove this duplicate test case.";
35+
private static final String SECONDARY_MESSAGE = "Original.";
36+
37+
@Override
38+
public void initialize(Context context) {
39+
context.registerSyntaxNodeConsumer(Tree.Kind.DECORATOR, DuplicateParametrizeCasesCheck::checkDecorator);
40+
}
41+
42+
@Override
43+
public CheckScope scope() {
44+
return CheckScope.ALL;
45+
}
46+
47+
private static void checkDecorator(SubscriptionContext ctx) {
48+
Decorator decorator = (Decorator) ctx.syntaxNode();
49+
Expression valuesExpression = UnittestUtils.parametrizeArgvaluesExpression(decorator, ctx);
50+
if (valuesExpression == null || !valuesExpression.is(Tree.Kind.LIST_LITERAL, Tree.Kind.TUPLE)) {
51+
return;
52+
}
53+
54+
raiseOnDuplicates(Expressions.expressionsFromListOrTuple(valuesExpression), ctx);
55+
}
56+
57+
private static void raiseOnDuplicates(List<Expression> cases, SubscriptionContext ctx) {
58+
List<Expression> distinctCases = new ArrayList<>();
59+
for (Expression testCase : cases) {
60+
Expression original = findEquivalent(distinctCases, testCase);
61+
if (original != null) {
62+
ctx.addIssue(testCase, MESSAGE).secondary(original, SECONDARY_MESSAGE);
63+
} else {
64+
distinctCases.add(testCase);
65+
}
66+
}
67+
}
68+
69+
private static Expression findEquivalent(List<Expression> distinctCases, Expression testCase) {
70+
Expression normalizedCase = Expressions.removeParentheses(testCase);
71+
for (Expression distinctCase : distinctCases) {
72+
if (CheckUtils.areEquivalent(Expressions.removeParentheses(distinctCase), normalizedCase)) {
73+
return distinctCase;
74+
}
75+
}
76+
return null;
77+
}
78+
}

‎python-checks/src/main/java/org/sonar/python/checks/tests/SpecificExceptionAssertionCheck.java‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -80,7 +80,7 @@ private static void checkCallExpression(SubscriptionContext ctx, CallExpression
8080
private static Expression genericExceptionArgument(CallExpression callExpression, SubscriptionContext ctx) {
8181
RegularArgument exceptionArgument = null;
8282
if (UnittestUtils.isPytestRaises(callExpression, ctx)) {
83-
if (UnittestUtils.hasPytestRaisesMatchArgument(callExpression)) {
83+
if (UnittestUtils.pytestMatchArgument(callExpression) != null) {
8484
return null;
8585
}
8686
exceptionArgument = UnittestUtils.pytestExpectedExceptionArgument(callExpression);

‎python-checks/src/main/java/org/sonar/python/checks/utils/UnittestUtils.java‎

Lines changed: 15 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -71,6 +71,7 @@ public class UnittestUtils {
7171
TypeMatchers.isType(UNITTEST_TEST_CASE_FQN_PREFIX + "assertIs"),
7272
TypeMatchers.isType(UNITTEST_TEST_CASE_FQN_PREFIX + "assertIsNot"));
7373
public static final TypeMatcher PYTEST_APPROX_MATCHER = TypeMatchers.isType("pytest.approx");
74+
private static final TypeMatcher PYTEST_PARAMETRIZE_MATCHER = TypeMatchers.isType("pytest.mark.parametrize");
7475
public static final TypeMatcher ASSERTPY_IS_EQUAL_TO_MATCHER = TypeMatchers.isType("assertpy.AssertionBuilder.is_equal_to");
7576
public static final TypeMatcher ASSERTPY_EQUALITY_ASSERTION_MATCHER = TypeMatchers.any(
7677
TypeMatchers.isType("assertpy.AssertionBuilder.is_equal_to"),
@@ -185,14 +186,6 @@ public static boolean isPytestWarns(CallExpression callExpression, SubscriptionC
185186
return PYTEST_WARNS_MATCHER.isTrueFor(callExpression.callee(), ctx);
186187
}
187188

188-
public static boolean hasPytestRaisesMatchArgument(CallExpression callExpression) {
189-
return TreeUtils.argumentByKeyword(PYTEST_MATCH, callExpression.arguments()) != null;
190-
}
191-
192-
public static boolean hasPytestWarnsMatchArgument(CallExpression callExpression) {
193-
return TreeUtils.argumentByKeyword(PYTEST_MATCH, callExpression.arguments()) != null;
194-
}
195-
196189
@Nullable
197190
public static RegularArgument pytestExpectedExceptionArgument(CallExpression callExpression) {
198191
return TreeUtils.nthArgumentOrKeyword(0, PYTEST_EXPECTED_EXCEPTION, callExpression.arguments());
@@ -208,6 +201,20 @@ public static RegularArgument pytestMatchArgument(CallExpression callExpression)
208201
return TreeUtils.argumentByKeyword(PYTEST_MATCH, callExpression.arguments());
209202
}
210203

204+
/**
205+
* Extracts the {@code argvalues} expression of a {@code @pytest.mark.parametrize(...)} decorator,
206+
* or {@code null} if the decorator is not a parametrize call or has no {@code argvalues}.
207+
*/
208+
@Nullable
209+
public static Expression parametrizeArgvaluesExpression(Decorator decorator, SubscriptionContext ctx) {
210+
Expression expression = decorator.expression();
211+
if (!(expression instanceof CallExpression callExpression) || !PYTEST_PARAMETRIZE_MATCHER.isTrueFor(callExpression.callee(), ctx)) {
212+
return null;
213+
}
214+
RegularArgument valuesArgument = TreeUtils.nthArgumentOrKeyword(1, "argvalues", callExpression.arguments());
215+
return valuesArgument == null ? null : Expressions.removeParentheses(valuesArgument.expression());
216+
}
217+
211218
public static boolean isUnittestAssertRaises(CallExpression callExpression, SubscriptionContext ctx) {
212219
return UNITTEST_ASSERT_RAISES_MATCHER.isTrueFor(callExpression.callee(), ctx);
213220
}

‎python-checks/src/main/resources/org/sonar/l10n/py/rules/python/S9078.html‎

Lines changed: 74 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,74 @@
1+
<p>This rule raises an issue when the <code>argvalues</code> collection of a <code>@pytest.mark.parametrize</code> decorator contains duplicate test
2+
cases.</p>
3+
<p>In Python with pytest, this occurs when <code>@pytest.mark.parametrize</code> is given an <code>argvalues</code> sequence that repeats the same
4+
case more than once (for example duplicate literals, names, or calls).</p>
5+
<h2>Why is this an issue?</h2>
6+
<p><code>@pytest.mark.parametrize</code> runs the decorated test once for each entry in its values collection. Duplicate cases therefore execute the
7+
same scenario more than once.</p>
8+
<p>That wastes CI time and usually signals a copy-paste mistake: an intended distinct case was never updated, so coverage looks broader than it
9+
is.</p>
10+
<p>Duplicates are detected by comparing the parametrize entries as expressions (literals, names, calls, and other AST forms), not only literal values.
11+
Accidental duplicates should be removed. Rarely, repeated identical entries are intentional when each run exercises mutable shared state; keep those
12+
only when that is deliberate.</p>
13+
<p>This rule complements {rule:python:S8998}, which flags empty parametrize value lists.</p>
14+
<h3>What is the potential impact?</h3>
15+
<p>Duplicate cases inflate suite runtime and can hide unfinished copy-paste edits. Teams may believe a scenario is covered in several ways when the
16+
same inputs are simply repeated.</p>
17+
<h2>How to fix it</h2>
18+
<p>When the duplicate is accidental, remove the repeated entries from the <code>@pytest.mark.parametrize</code> values list so each case appears only
19+
once. If the repeated entry was meant to be a different scenario, replace it with the intended distinct inputs.</p>
20+
<p>Do not remove duplicates that are intentional for stateful behavior (for example repeated invocations that observe mutable global state). In those
21+
cases, keep the repeated cases or make the shared-state dependency explicit another way.</p>
22+
<h3>Code examples</h3>
23+
<h4>Noncompliant code example</h4>
24+
<pre data-diff-id="1" data-diff-type="noncompliant">
25+
import pytest
26+
27+
@pytest.mark.parametrize("n", [1, 2, 2, 3]) # Noncompliant
28+
def test_double(n):
29+
assert double(n) == n * 2
30+
</pre>
31+
<h4>Compliant solution</h4>
32+
<pre data-diff-id="1" data-diff-type="compliant">
33+
import pytest
34+
35+
@pytest.mark.parametrize("n", [1, 2, 3])
36+
def test_double(n):
37+
assert double(n) == n * 2
38+
</pre>
39+
<h4>Noncompliant code example</h4>
40+
<pre data-diff-id="2" data-diff-type="noncompliant">
41+
import pytest
42+
43+
@pytest.mark.parametrize("operand,expected", [
44+
(1, 2),
45+
(2, 4),
46+
(1, 2), # Noncompliant
47+
(3, 6),
48+
])
49+
def test_double(operand, expected):
50+
assert double(operand) == expected
51+
</pre>
52+
<h4>Compliant solution</h4>
53+
<pre data-diff-id="2" data-diff-type="compliant">
54+
import pytest
55+
56+
@pytest.mark.parametrize("operand,expected", [
57+
(1, 2),
58+
(2, 4),
59+
(3, 6),
60+
])
61+
def test_double(operand, expected):
62+
assert double(operand) == expected
63+
</pre>
64+
<h2>Resources</h2>
65+
<h3>Documentation</h3>
66+
<ul>
67+
<li>pytest documentation - <a href="https://docs.pytest.org/en/stable/how-to/parametrize.html">How to parametrize fixtures and test
68+
functions</a></li>
69+
<li>pytest documentation - <a
70+
href="https://docs.pytest.org/en/stable/reference/reference.html#pytest-mark-parametrize">pytest.mark.parametrize</a></li>
71+
<li>Ruff PT014 - <a
72+
href="https://docs.astral.sh/ruff/rules/pytest-duplicate-parametrize-test-cases/">pytest-duplicate-parametrize-test-cases</a></li>
73+
</ul>
74+

‎python-checks/src/main/resources/org/sonar/l10n/py/rules/python/S9078.json‎

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,24 @@
1+
{
2+
"title": "Parametrize decorators should not contain duplicate test cases",
3+
"type": "CODE_SMELL",
4+
"status": "ready",
5+
"remediation": {
6+
"func": "Constant\/Issue",
7+
"constantCost": "5min"
8+
},
9+
"tags": [
10+
"pytest",
11+
"tests"
12+
],
13+
"defaultSeverity": "Major",
14+
"ruleSpecification": "RSPEC-9078",
15+
"sqKey": "S9078",
16+
"scope": "Tests",
17+
"quickfix": "unknown",
18+
"code": {
19+
"impacts": {
20+
"MAINTAINABILITY": "MEDIUM"
21+
},
22+
"attribute": "EFFICIENT"
23+
}
24+
}

‎python-checks/src/main/resources/org/sonar/l10n/py/rules/python/Sonar_way_profile.json‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -384,6 +384,7 @@
384384
"S9073",
385385
"S9074",
386386
"S9075",
387-
"S9076"
387+
"S9076",
388+
"S9078"
388389
]
389390
}

‎python-checks/src/test/java/org/sonar/python/checks/tests/DuplicateParametrizeCasesCheckTest.java‎

Lines changed: 36 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,36 @@
1+
/*
2+
* SonarQube Python Plugin
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.python.checks.tests;
18+
19+
import org.junit.jupiter.api.Test;
20+
import org.sonar.plugins.python.api.PythonCheck;
21+
import org.sonar.python.checks.utils.PythonCheckVerifier;
22+
23+
import static org.assertj.core.api.Assertions.assertThat;
24+
25+
class DuplicateParametrizeCasesCheckTest {
26+
27+
@Test
28+
void test() {
29+
PythonCheckVerifier.verify("src/test/resources/checks/tests/duplicateParametrizeCases.py", new DuplicateParametrizeCasesCheck());
30+
}
31+
32+
@Test
33+
void test_scope() {
34+
assertThat(new DuplicateParametrizeCasesCheck().scope()).isEqualTo(PythonCheck.CheckScope.ALL);
35+
}
36+
}

‎python-checks/src/test/java/org/sonar/python/checks/utils/UnittestUtilsTest.java‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -256,7 +256,7 @@ void test_pytest_raises_helpers() {
256256
raises(ValueError)
257257
""", (ctx, callExpression) -> {
258258
isPytestRaises.add(UnittestUtils.isPytestRaises(callExpression, ctx));
259-
hasMatchArgument.add(UnittestUtils.hasPytestRaisesMatchArgument(callExpression));
259+
hasMatchArgument.add(UnittestUtils.pytestMatchArgument(callExpression) != null);
260260
exceptionArguments.add(UnittestUtils.pytestExpectedExceptionArgument(callExpression).expression().firstToken().value());
261261
});
262262

@@ -279,7 +279,7 @@ void test_pytest_warns_helpers() {
279279
warns(UserWarning)
280280
""", (ctx, callExpression) -> {
281281
isPytestWarns.add(UnittestUtils.isPytestWarns(callExpression, ctx));
282-
hasMatchArgument.add(UnittestUtils.hasPytestWarnsMatchArgument(callExpression));
282+
hasMatchArgument.add(UnittestUtils.pytestMatchArgument(callExpression) != null);
283283
warningArguments.add(UnittestUtils.pytestExpectedWarningArgument(callExpression).expression().firstToken().value());
284284
RegularArgument matchArgument = UnittestUtils.pytestMatchArgument(callExpression);
285285
matchArguments.add(matchArgument == null ? null : matchArgument.expression().firstToken().value());

0 commit comments

Comments
 (0)