Fix pytest.mark.parametrize rules to check calls instead of decorat…

…ors (#14515)

Co-authored-by: Micha Reiser <[email protected]>

crates/ruff_linter/resources/test/fixtures/flake8_pytest_style/PT006.py

@@ -69,3 +69,15 @@ def test_implicit_str_concat_with_multi_parens(param1, param2, param3):
 @pytest.mark.parametrize(("param1,param2"), [(1, 2), (3, 4)])
 def test_csv_with_parens(param1, param2):
     ...
+
+
+parametrize = pytest.mark.parametrize(("param1,param2"), [(1, 2), (3, 4)])
+
+@parametrize
+def test_not_decorator(param1, param2):
+    ...
+
+
+@pytest.mark.parametrize(argnames=("param1,param2"), argvalues=[(1, 2), (3, 4)])
+def test_keyword_arguments(param1, param2):
+    ...

crates/ruff_linter/src/checkers/ast/analyze/expression.rs

@@ -856,6 +856,15 @@ pub(crate) fn expression(expr: &Expr, checker: &mut Checker) {
                     checker.diagnostics.push(diagnostic);
                 }
             }
+            if checker.settings.preview.is_enabled()
+                && checker.any_enabled(&[
+                    Rule::PytestParametrizeNamesWrongType,
+                    Rule::PytestParametrizeValuesWrongType,
+                    Rule::PytestDuplicateParametrizeTestCases,
+                ])
+            {
+                flake8_pytest_style::rules::parametrize(checker, call);
+            }
             if checker.enabled(Rule::PytestUnittestAssertion) {
                 if let Some(diagnostic) = flake8_pytest_style::rules::unittest_assertion(
                     checker, expr, func, args, keywords,

crates/ruff_linter/src/checkers/ast/analyze/statement.rs

@@ -309,12 +309,20 @@ pub(crate) fn statement(stmt: &Stmt, checker: &mut Checker) {
                     body,
                 );
             }
-            if checker.any_enabled(&[
-                Rule::PytestParametrizeNamesWrongType,
-                Rule::PytestParametrizeValuesWrongType,
-                Rule::PytestDuplicateParametrizeTestCases,
-            ]) {
-                flake8_pytest_style::rules::parametrize(checker, decorator_list);
+            // In preview mode, calls are analyzed. To avoid duplicate diagnostics,
+            // skip analyzing the decorators.
+            if !checker.settings.preview.is_enabled()
+                && checker.any_enabled(&[
+                    Rule::PytestParametrizeNamesWrongType,
+                    Rule::PytestParametrizeValuesWrongType,
+                    Rule::PytestDuplicateParametrizeTestCases,
+                ])
+            {
+                for decorator in decorator_list {
+                    if let Some(call) = decorator.expression.as_call_expr() {
+                        flake8_pytest_style::rules::parametrize(checker, call);
+                    }
+                }
             }
             if checker.any_enabled(&[
                 Rule::PytestIncorrectMarkParenthesesStyle,

crates/ruff_linter/src/rules/flake8_pytest_style/mod.rs

@@ -12,6 +12,7 @@ mod tests {
 
     use crate::registry::Rule;
     use crate::settings::types::IdentifierPattern;
+    use crate::settings::types::PreviewMode;
     use crate::test::test_path;
     use crate::{assert_messages, settings};
 
@@ -291,4 +292,22 @@ mod tests {
         assert_messages!(name, diagnostics);
         Ok(())
     }
+
+    #[test_case(Rule::PytestParametrizeNamesWrongType, Path::new("PT006.py"))]
+    fn test_pytest_style_preview(rule_code: Rule, path: &Path) -> Result<()> {
+        let snapshot = format!(
+            "preview__{}_{}",
+            rule_code.noqa_code(),
+            path.to_string_lossy()
+        );
+        let diagnostics = test_path(
+            Path::new("flake8_pytest_style").join(path).as_path(),
+            &settings::LinterSettings {
+                preview: PreviewMode::Enabled,
+                ..settings::LinterSettings::for_rule(rule_code)
+            },
+        )?;
+        assert_messages!(snapshot, diagnostics);
+        Ok(())
+    }
 }

crates/ruff_linter/src/rules/flake8_pytest_style/rules/helpers.rs

@@ -2,7 +2,7 @@ use std::fmt;
 
 use ruff_python_ast::helpers::map_callable;
 use ruff_python_ast::name::UnqualifiedName;
-use ruff_python_ast::{self as ast, Decorator, Expr, Keyword};
+use ruff_python_ast::{self as ast, Decorator, Expr, ExprCall, Keyword};
 use ruff_python_semantic::SemanticModel;
 use ruff_python_trivia::PythonWhitespace;
 
@@ -38,9 +38,9 @@ pub(super) fn is_pytest_yield_fixture(decorator: &Decorator, semantic: &Semantic
         })
 }
 
-pub(super) fn is_pytest_parametrize(decorator: &Decorator, semantic: &SemanticModel) -> bool {
+pub(super) fn is_pytest_parametrize(call: &ExprCall, semantic: &SemanticModel) -> bool {
     semantic
-        .resolve_qualified_name(map_callable(&decorator.expression))
+        .resolve_qualified_name(&call.func)
         .is_some_and(|qualified_name| {
             matches!(qualified_name.segments(), ["pytest", "mark", "parametrize"])
         })

crates/ruff_linter/src/rules/flake8_pytest_style/rules/parametrize.rs

@@ -5,7 +5,7 @@ use ruff_macros::{derive_message_formats, violation};
 use ruff_python_ast::comparable::ComparableExpr;
 use ruff_python_ast::parenthesize::parenthesized_range;
 use ruff_python_ast::AstNode;
-use ruff_python_ast::{self as ast, Arguments, Decorator, Expr, ExprContext};
+use ruff_python_ast::{self as ast, Expr, ExprCall, ExprContext};
 use ruff_python_codegen::Generator;
 use ruff_python_trivia::CommentRanges;
 use ruff_python_trivia::{SimpleTokenKind, SimpleTokenizer};
@@ -317,23 +317,21 @@ fn elts_to_csv(elts: &[Expr], generator: Generator) -> Option<String> {
 ///
 /// This method assumes that the first argument is a string.
 fn get_parametrize_name_range(
-    decorator: &Decorator,
+    call: &ExprCall,
     expr: &Expr,
     comment_ranges: &CommentRanges,
     source: &str,
 ) -> Option<TextRange> {
-    decorator.expression.as_call_expr().and_then(|call| {
-        parenthesized_range(
-            expr.into(),
-            call.arguments.as_any_node_ref(),
-            comment_ranges,
-            source,
-        )
-    })
+    parenthesized_range(
+        expr.into(),
+        call.arguments.as_any_node_ref(),
+        comment_ranges,
+        source,
+    )
 }
 
 /// PT006
-fn check_names(checker: &mut Checker, decorator: &Decorator, expr: &Expr) {
+fn check_names(checker: &mut Checker, call: &ExprCall, expr: &Expr) {
     let names_type = checker.settings.flake8_pytest_style.parametrize_names_type;
 
     match expr {
@@ -343,7 +341,7 @@ fn check_names(checker: &mut Checker, decorator: &Decorator, expr: &Expr) {
                 match names_type {
                     types::ParametrizeNameType::Tuple => {
                         let name_range = get_parametrize_name_range(
-                            decorator,
+                            call,
                             expr,
                             checker.comment_ranges(),
                             checker.locator().contents(),
@@ -378,7 +376,7 @@ fn check_names(checker: &mut Checker, decorator: &Decorator, expr: &Expr) {
                     }
                     types::ParametrizeNameType::List => {
                         let name_range = get_parametrize_name_range(
-                            decorator,
+                            call,
                             expr,
                             checker.comment_ranges(),
                             checker.locator().contents(),
@@ -797,30 +795,42 @@ fn handle_value_rows(
     }
 }
 
-pub(crate) fn parametrize(checker: &mut Checker, decorators: &[Decorator]) {
-    for decorator in decorators {
-        if is_pytest_parametrize(decorator, checker.semantic()) {
-            if let Expr::Call(ast::ExprCall {
-                arguments: Arguments { args, .. },
-                ..
-            }) = &decorator.expression
-            {
-                if checker.enabled(Rule::PytestParametrizeNamesWrongType) {
-                    if let [names, ..] = &**args {
-                        check_names(checker, decorator, names);
-                    }
-                }
-                if checker.enabled(Rule::PytestParametrizeValuesWrongType) {
-                    if let [names, values, ..] = &**args {
-                        check_values(checker, names, values);
-                    }
-                }
-                if checker.enabled(Rule::PytestDuplicateParametrizeTestCases) {
-                    if let [_, values, ..] = &**args {
-                        check_duplicates(checker, values);
-                    }
-                }
-            }
+pub(crate) fn parametrize(checker: &mut Checker, call: &ExprCall) {
+    if !is_pytest_parametrize(call, checker.semantic()) {
+        return;
+    }
+
+    if checker.enabled(Rule::PytestParametrizeNamesWrongType) {
+        if let Some(names) = if checker.settings.preview.is_enabled() {
+            call.arguments.find_argument("argnames", 0)
+        } else {
+            call.arguments.find_positional(0)
+        } {
+            check_names(checker, call, names);
+        }
+    }
+    if checker.enabled(Rule::PytestParametrizeValuesWrongType) {
+        let names = if checker.settings.preview.is_enabled() {
+            call.arguments.find_argument("argnames", 0)
+        } else {
+            call.arguments.find_positional(0)
+        };
+        let values = if checker.settings.preview.is_enabled() {
+            call.arguments.find_argument("argvalues", 1)
+        } else {
+            call.arguments.find_positional(1)
+        };
+        if let (Some(names), Some(values)) = (names, values) {
+            check_values(checker, names, values);
+        }
+    }
+    if checker.enabled(Rule::PytestDuplicateParametrizeTestCases) {
+        if let Some(values) = if checker.settings.preview.is_enabled() {
+            call.arguments.find_argument("argvalues", 1)
+        } else {
+            call.arguments.find_positional(1)
+        } {
+            check_duplicates(checker, values);
         }
     }
 }

crates/ruff_linter/src/rules/flake8_pytest_style/snapshots/ruff_linter__rules__flake8_pytest_style__tests__PT006_default.snap

@@ -228,3 +228,4 @@ PT006.py:69:26: PT006 [*] Wrong type passed to first argument of `@pytest.mark.p
    69 |+@pytest.mark.parametrize(("param1", "param2"), [(1, 2), (3, 4)])
 70 70 | def test_csv_with_parens(param1, param2):
 71 71 |     ...
+72 72 |

crates/ruff_linter/src/rules/flake8_pytest_style/snapshots/ruff_linter__rules__flake8_pytest_style__tests__PT006_list.snap

@@ -190,3 +190,4 @@ PT006.py:69:26: PT006 [*] Wrong type passed to first argument of `@pytest.mark.p
    69 |+@pytest.mark.parametrize(["param1", "param2"], [(1, 2), (3, 4)])
 70 70 | def test_csv_with_parens(param1, param2):
 71 71 |     ...
+72 72 |