SONARPY-4434 Add S9116: Pytest fixture options should be passed as keyword arguments

GitOrigin-RevId: 6282221ce406e55bd91e4b6f972245f0891ffaaa

Author: rombirli

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

@@ -108,6 +108,7 @@
 import org.sonar.python.checks.tests.NotDiscoverableTestMethodCheck;
 import org.sonar.python.checks.tests.PytestAutouseParametrizedFixtureCheck;
 import org.sonar.python.checks.tests.PytestFixtureMultipleYieldCheck;
+import org.sonar.python.checks.tests.PytestFixturePositionalArgsCheck;
 import org.sonar.python.checks.tests.PytestPluginsConftestCheck;
 import org.sonar.python.checks.tests.PytestRaisesContextManagerCheck;
 import org.sonar.python.checks.tests.SingleInvocationRuntimeExceptionCheck;
@@ -412,6 +413,7 @@ public Stream<Class<?>> getChecks() {
       PydanticSkipValidationWithConstraintsCheck.class,
       PytestAutouseParametrizedFixtureCheck.class,
       PytestFixtureMultipleYieldCheck.class,
+      PytestFixturePositionalArgsCheck.class,
       PytestPluginsConftestCheck.class,
       PytestRaisesContextManagerCheck.class,
       PyTorchDataLoaderNumWorkersCheck.class,

python-checks/src/main/java/org/sonar/python/checks/tests/PytestFixturePositionalArgsCheck.java

@@ -0,0 +1,74 @@
+/*
+ * SonarQube Python Plugin
+ * 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.python.checks.tests;
+
+import org.sonar.check.Rule;
+import org.sonar.plugins.python.api.PythonSubscriptionCheck;
+import org.sonar.plugins.python.api.SubscriptionContext;
+import org.sonar.plugins.python.api.tree.Argument;
+import org.sonar.plugins.python.api.tree.CallExpression;
+import org.sonar.plugins.python.api.tree.Decorator;
+import org.sonar.plugins.python.api.tree.RegularArgument;
+import org.sonar.plugins.python.api.tree.Tree;
+import org.sonar.plugins.python.api.tree.UnpackingExpression;
+import org.sonar.plugins.python.api.types.v2.matchers.TypeMatcher;
+import org.sonar.plugins.python.api.types.v2.matchers.TypeMatchers;
+
+@Rule(key = "S9116")
+public class PytestFixturePositionalArgsCheck extends PythonSubscriptionCheck {
+
+  private static final String MESSAGE = "Pass fixture options as keyword arguments.";
+
+  private static final TypeMatcher PYTEST_FIXTURE_MATCHER = TypeMatchers.withFQN("pytest.fixture");
+
+  @Override
+  public void initialize(Context context) {
+    context.registerSyntaxNodeConsumer(Tree.Kind.DECORATOR, PytestFixturePositionalArgsCheck::checkDecorator);
+  }
+
+  private static void checkDecorator(SubscriptionContext ctx) {
+    Decorator decorator = (Decorator) ctx.syntaxNode();
+    if (!(decorator.expression() instanceof CallExpression callExpression)) {
+      return;
+    }
+    if (!PYTEST_FIXTURE_MATCHER.isTrueFor(callExpression.callee(), ctx)) {
+      return;
+    }
+
+    for (Argument argument : callExpression.arguments()) {
+      if (isPositionalArgument(argument)) {
+        ctx.addIssue(argument, MESSAGE);
+        return;
+      }
+    }
+  }
+
+  private static boolean isPositionalArgument(Argument argument) {
+    if (argument instanceof RegularArgument regularArgument) {
+      return regularArgument.keywordArgument() == null;
+    }
+    if (argument instanceof UnpackingExpression unpackingExpression) {
+      return "*".equals(unpackingExpression.starToken().value());
+    }
+    return false;
+  }
+
+  @Override
+  public CheckScope scope() {
+    return CheckScope.ALL;
+  }
+}

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

@@ -0,0 +1,53 @@
+<p>This rule raises an issue when <code>@pytest.fixture</code> options are passed as positional arguments.</p>
+<p>In Python, this refers to <code>@pytest.fixture(…​)</code> with options passed as positional arguments, such as
+<code>@pytest.fixture("module")</code>.</p>
+<h2>Why is this an issue?</h2>
+<p>Passing options like scope as positional arguments to <code>@pytest.fixture</code> is ambiguous and has been removed in pytest 6.0 and later. Use
+keyword arguments:</p>
+<pre>
+@pytest.fixture("module")  # Removed; use keyword arguments
+def shared_state():
+    return {}
+</pre>
+<p>Prefer:</p>
+<pre>
+@pytest.fixture(scope="module")
+def shared_state():
+    return {}
+</pre>
+<h3>What is the potential impact?</h3>
+<p>Positional arguments to <code>@pytest.fixture</code> are rejected by modern pytest, so fixtures written this way fail to load or raise errors at
+collection time.</p>
+<h2>How to fix it</h2>
+<p>Pass fixture options with keywords (for example <code>scope="module"</code>).</p>
+<h3>Code examples</h3>
+<h4>Noncompliant code example</h4>
+<pre data-diff-id="1" data-diff-type="noncompliant">
+import pytest
+
+@pytest.fixture("module")  # Noncompliant
+def scoped():
+    return []
+</pre>
+<h4>Compliant solution</h4>
+<pre data-diff-id="1" data-diff-type="compliant">
+import pytest
+
+@pytest.fixture(scope="module")
+def scoped():
+    return []
+</pre>
+<h2>Resources</h2>
+<h3>Documentation</h3>
+<ul>
+  <li>pytest documentation - <a href="https://docs.pytest.org/en/stable/reference/reference.html#pytest.fixture">pytest.fixture</a></li>
+  <li>pytest documentation - <a href="https://docs.pytest.org/en/stable/deprecations.html#pytest-fixture-arguments-are-keyword-only">pytest.fixture
+  arguments are keyword only</a></li>
+  <li>Ruff - <a href="https://docs.astral.sh/ruff/rules/pytest-fixture-positional-args/">PT002 — pytest-fixture-positional-args</a></li>
+</ul>
+<h3>Related rules</h3>
+<ul>
+  <li>{rule:python:S9076} - Deprecated pytest.yield_fixture should be replaced with pytest.fixture</li>
+  <li>{rule:python:S9117} - Pytest fixtures should not declare the default scope="function"</li>
+</ul>
+

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

@@ -0,0 +1,24 @@
+{
+  "title": "Pytest fixture options should be passed as keyword arguments",
+  "type": "CODE_SMELL",
+  "status": "ready",
+  "remediation": {
+    "func": "Constant\/Issue",
+    "constantCost": "5min"
+  },
+  "tags": [
+    "pytest",
+    "tests"
+  ],
+  "defaultSeverity": "Major",
+  "ruleSpecification": "RSPEC-9116",
+  "sqKey": "S9116",
+  "scope": "Tests",
+  "quickfix": "unknown",
+  "code": {
+    "impacts": {
+      "MAINTAINABILITY": "MEDIUM"
+    },
+    "attribute": "CONVENTIONAL"
+  }
+}

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

@@ -385,6 +385,7 @@
     "S9074",
     "S9075",
     "S9076",
-    "S9078"
+    "S9078",
+    "S9116"
   ]
 }

python-checks/src/test/java/org/sonar/python/checks/tests/PytestFixturePositionalArgsCheckTest.java

@@ -0,0 +1,37 @@
+/*
+ * SonarQube Python Plugin
+ * 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.python.checks.tests;
+
+import org.junit.jupiter.api.Test;
+import org.sonar.plugins.python.api.PythonCheck;
+import org.sonar.python.checks.utils.PythonCheckVerifier;
+
+import static org.assertj.core.api.Assertions.assertThat;
+
+class PytestFixturePositionalArgsCheckTest {
+
+  @Test
+  void test() {
+    PythonCheckVerifier.verify("src/test/resources/checks/tests/pytestFixturePositionalArgs.py", new PytestFixturePositionalArgsCheck());
+  }
+
+  @Test
+  void test_scope() {
+    assertThat(new PytestFixturePositionalArgsCheck().scope()).isEqualTo(PythonCheck.CheckScope.ALL);
+  }
+
+}

python-checks/src/test/resources/checks/tests/pytestFixturePositionalArgs.py

@@ -0,0 +1,90 @@
+import pytest
+from pytest import fixture
+
+
+@pytest.fixture("module")  # Noncompliant {{Pass fixture options as keyword arguments.}}
+#               ^^^^^^^^
+def noncompliant_positional_scope():
+    return []
+
+
+@pytest.fixture(True)  # Noncompliant
+#               ^^^^
+def noncompliant_positional_autouse():
+    return []
+
+
+@pytest.fixture("module", True)  # Noncompliant
+#               ^^^^^^^^
+def noncompliant_multiple_positionals():
+    return []
+
+
+@pytest.fixture("module", autouse=True)  # Noncompliant
+#               ^^^^^^^^
+def noncompliant_mixed_positional_and_keyword():
+    return []
+
+
+OPTS = ("module",)
+@pytest.fixture(*OPTS)  # Noncompliant
+#               ^^^^^
+def noncompliant_unpacked_positional():
+    return []
+
+
+@fixture("session")  # Noncompliant
+#        ^^^^^^^^^
+def noncompliant_imported_fixture():
+    return []
+
+
+class TestSomething:
+    @pytest.fixture("class")  # Noncompliant
+    #               ^^^^^^^
+    def noncompliant_class_fixture(self):
+        return []
+
+
+@pytest.fixture(scope="module")
+def compliant_keyword_scope():
+    return []
+
+
+@pytest.fixture(autouse=True)
+def compliant_keyword_autouse():
+    return []
+
+
+@pytest.fixture(scope="module", autouse=True)
+def compliant_multiple_keywords():
+    return []
+
+
+@pytest.fixture()
+def compliant_empty_call():
+    return []
+
+
+@pytest.fixture
+def compliant_no_parentheses():
+    return []
+
+
+@pytest.fixture(**{"scope": "module"})
+def compliant_unpacked_keywords():
+    return []
+
+
+@fixture(scope="function")
+def compliant_imported_fixture():
+    return []
+
+
+def not_a_fixture(*args):
+    pass
+
+
+@not_a_fixture("module")
+def decorated_with_unrelated_call():
+    pass