Fix pytest.mark.parametrize rules to check calls instead of decorators (#14515)

Co-authored-by: Micha Reiser <micha@reiser.io>

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(&[
+            // 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,
-            ]) {
+                ])
-                flake8_pytest_style::rules::parametrize(checker, decorator_list);
+            {
+                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,
     )
-    })
 }
 
 /// 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]) {
+pub(crate) fn parametrize(checker: &mut Checker, call: &ExprCall) {
-    for decorator in decorators {
+    if !is_pytest_parametrize(call, checker.semantic()) {
-        if is_pytest_parametrize(decorator, checker.semantic()) {
+        return;
-            if let Expr::Call(ast::ExprCall {
+    }
-                arguments: Arguments { args, .. },
+
-                ..
-            }) = &decorator.expression
-            {
     if checker.enabled(Rule::PytestParametrizeNamesWrongType) {
-                    if let [names, ..] = &**args {
+        if let Some(names) = if checker.settings.preview.is_enabled() {
-                        check_names(checker, decorator, names);
+            call.arguments.find_argument("argnames", 0)
+        } else {
+            call.arguments.find_positional(0)
+        } {
+            check_names(checker, call, names);
         }
     }
     if checker.enabled(Rule::PytestParametrizeValuesWrongType) {
-                    if let [names, values, ..] = &**args {
+        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 [_, values, ..] = &**args {
+        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 |

crates/ruff_linter/src/rules/flake8_pytest_style/snapshots/ruff_linter__rules__flake8_pytest_style__tests__preview__PT006_PT006.py.snap

@@ -0,0 +1,268 @@
+---
+source: crates/ruff_linter/src/rules/flake8_pytest_style/mod.rs
+snapshot_kind: text
+---
+PT006.py:9:26: PT006 [*] Wrong type passed to first argument of `@pytest.mark.parametrize`; expected `tuple`
+   |
+ 9 | @pytest.mark.parametrize("param1,param2", [(1, 2), (3, 4)])
+   |                          ^^^^^^^^^^^^^^^ PT006
+10 | def test_csv(param1, param2):
+11 |     ...
+   |
+   = help: Use a `tuple` for the first argument
+
+ℹ Unsafe fix
+6  6  |     ...
+7  7  | 
+8  8  | 
+9     |-@pytest.mark.parametrize("param1,param2", [(1, 2), (3, 4)])
+   9  |+@pytest.mark.parametrize(("param1", "param2"), [(1, 2), (3, 4)])
+10 10 | def test_csv(param1, param2):
+11 11 |     ...
+12 12 | 
+
+PT006.py:14:26: PT006 [*] Wrong type passed to first argument of `@pytest.mark.parametrize`; expected `tuple`
+   |
+14 | @pytest.mark.parametrize("   param1,   ,    param2   , ", [(1, 2), (3, 4)])
+   |                          ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ PT006
+15 | def test_csv_with_whitespace(param1, param2):
+16 |     ...
+   |
+   = help: Use a `tuple` for the first argument
+
+ℹ Unsafe fix
+11 11 |     ...
+12 12 | 
+13 13 | 
+14    |-@pytest.mark.parametrize("   param1,   ,    param2   , ", [(1, 2), (3, 4)])
+   14 |+@pytest.mark.parametrize(("param1", "param2"), [(1, 2), (3, 4)])
+15 15 | def test_csv_with_whitespace(param1, param2):
+16 16 |     ...
+17 17 | 
+
+PT006.py:19:26: PT006 [*] Wrong type passed to first argument of `@pytest.mark.parametrize`; expected `tuple`
+   |
+19 | @pytest.mark.parametrize("param1,param2", [(1, 2), (3, 4)])
+   |                          ^^^^^^^^^^^^^^^ PT006
+20 | def test_csv_bad_quotes(param1, param2):
+21 |     ...
+   |
+   = help: Use a `tuple` for the first argument
+
+ℹ Unsafe fix
+16 16 |     ...
+17 17 | 
+18 18 | 
+19    |-@pytest.mark.parametrize("param1,param2", [(1, 2), (3, 4)])
+   19 |+@pytest.mark.parametrize(("param1", "param2"), [(1, 2), (3, 4)])
+20 20 | def test_csv_bad_quotes(param1, param2):
+21 21 |     ...
+22 22 | 
+
+PT006.py:29:26: PT006 [*] Wrong type passed to first argument of `@pytest.mark.parametrize`; expected `str`
+   |
+29 | @pytest.mark.parametrize(("param1",), [1, 2, 3])
+   |                          ^^^^^^^^^^^ PT006
+30 | def test_tuple_one_elem(param1, param2):
+31 |     ...
+   |
+   = help: Use a string for the first argument
+
+ℹ Safe fix
+26 26 |     ...
+27 27 | 
+28 28 | 
+29    |-@pytest.mark.parametrize(("param1",), [1, 2, 3])
+   29 |+@pytest.mark.parametrize("param1", [1, 2, 3])
+30 30 | def test_tuple_one_elem(param1, param2):
+31 31 |     ...
+32 32 | 
+
+PT006.py:34:26: PT006 [*] Wrong type passed to first argument of `@pytest.mark.parametrize`; expected `tuple`
+   |
+34 | @pytest.mark.parametrize(["param1", "param2"], [(1, 2), (3, 4)])
+   |                          ^^^^^^^^^^^^^^^^^^^^ PT006
+35 | def test_list(param1, param2):
+36 |     ...
+   |
+   = help: Use a `tuple` for the first argument
+
+ℹ Unsafe fix
+31 31 |     ...
+32 32 | 
+33 33 | 
+34    |-@pytest.mark.parametrize(["param1", "param2"], [(1, 2), (3, 4)])
+   34 |+@pytest.mark.parametrize(("param1", "param2"), [(1, 2), (3, 4)])
+35 35 | def test_list(param1, param2):
+36 36 |     ...
+37 37 | 
+
+PT006.py:39:26: PT006 [*] Wrong type passed to first argument of `@pytest.mark.parametrize`; expected `str`
+   |
+39 | @pytest.mark.parametrize(["param1"], [1, 2, 3])
+   |                          ^^^^^^^^^^ PT006
+40 | def test_list_one_elem(param1, param2):
+41 |     ...
+   |
+   = help: Use a string for the first argument
+
+ℹ Safe fix
+36 36 |     ...
+37 37 | 
+38 38 | 
+39    |-@pytest.mark.parametrize(["param1"], [1, 2, 3])
+   39 |+@pytest.mark.parametrize("param1", [1, 2, 3])
+40 40 | def test_list_one_elem(param1, param2):
+41 41 |     ...
+42 42 | 
+
+PT006.py:44:26: PT006 [*] Wrong type passed to first argument of `@pytest.mark.parametrize`; expected `tuple`
+   |
+44 | @pytest.mark.parametrize([some_expr, another_expr], [1, 2, 3])
+   |                          ^^^^^^^^^^^^^^^^^^^^^^^^^ PT006
+45 | def test_list_expressions(param1, param2):
+46 |     ...
+   |
+   = help: Use a `tuple` for the first argument
+
+ℹ Unsafe fix
+41 41 |     ...
+42 42 | 
+43 43 | 
+44    |-@pytest.mark.parametrize([some_expr, another_expr], [1, 2, 3])
+   44 |+@pytest.mark.parametrize((some_expr, another_expr), [1, 2, 3])
+45 45 | def test_list_expressions(param1, param2):
+46 46 |     ...
+47 47 | 
+
+PT006.py:49:26: PT006 [*] Wrong type passed to first argument of `@pytest.mark.parametrize`; expected `tuple`
+   |
+49 | @pytest.mark.parametrize([some_expr, "param2"], [1, 2, 3])
+   |                          ^^^^^^^^^^^^^^^^^^^^^ PT006
+50 | def test_list_mixed_expr_literal(param1, param2):
+51 |     ...
+   |
+   = help: Use a `tuple` for the first argument
+
+ℹ Unsafe fix
+46 46 |     ...
+47 47 | 
+48 48 | 
+49    |-@pytest.mark.parametrize([some_expr, "param2"], [1, 2, 3])
+   49 |+@pytest.mark.parametrize((some_expr, "param2"), [1, 2, 3])
+50 50 | def test_list_mixed_expr_literal(param1, param2):
+51 51 |     ...
+52 52 | 
+
+PT006.py:54:26: PT006 [*] Wrong type passed to first argument of `@pytest.mark.parametrize`; expected `tuple`
+   |
+54 | @pytest.mark.parametrize(("param1, " "param2, " "param3"), [(1, 2, 3), (4, 5, 6)])
+   |                          ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ PT006
+55 | def test_implicit_str_concat_with_parens(param1, param2, param3):
+56 |     ...
+   |
+   = help: Use a `tuple` for the first argument
+
+ℹ Unsafe fix
+51 51 |     ...
+52 52 | 
+53 53 | 
+54    |-@pytest.mark.parametrize(("param1, " "param2, " "param3"), [(1, 2, 3), (4, 5, 6)])
+   54 |+@pytest.mark.parametrize(("param1", "param2", "param3"), [(1, 2, 3), (4, 5, 6)])
+55 55 | def test_implicit_str_concat_with_parens(param1, param2, param3):
+56 56 |     ...
+57 57 | 
+
+PT006.py:59:26: PT006 [*] Wrong type passed to first argument of `@pytest.mark.parametrize`; expected `tuple`
+   |
+59 | @pytest.mark.parametrize("param1, " "param2, " "param3", [(1, 2, 3), (4, 5, 6)])
+   |                          ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ PT006
+60 | def test_implicit_str_concat_no_parens(param1, param2, param3):
+61 |     ...
+   |
+   = help: Use a `tuple` for the first argument
+
+ℹ Unsafe fix
+56 56 |     ...
+57 57 | 
+58 58 | 
+59    |-@pytest.mark.parametrize("param1, " "param2, " "param3", [(1, 2, 3), (4, 5, 6)])
+   59 |+@pytest.mark.parametrize(("param1", "param2", "param3"), [(1, 2, 3), (4, 5, 6)])
+60 60 | def test_implicit_str_concat_no_parens(param1, param2, param3):
+61 61 |     ...
+62 62 | 
+
+PT006.py:64:26: PT006 [*] Wrong type passed to first argument of `@pytest.mark.parametrize`; expected `tuple`
+   |
+64 | @pytest.mark.parametrize((("param1, " "param2, " "param3")), [(1, 2, 3), (4, 5, 6)])
+   |                          ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ PT006
+65 | def test_implicit_str_concat_with_multi_parens(param1, param2, param3):
+66 |     ...
+   |
+   = help: Use a `tuple` for the first argument
+
+ℹ Unsafe fix
+61 61 |     ...
+62 62 | 
+63 63 | 
+64    |-@pytest.mark.parametrize((("param1, " "param2, " "param3")), [(1, 2, 3), (4, 5, 6)])
+   64 |+@pytest.mark.parametrize(("param1", "param2", "param3"), [(1, 2, 3), (4, 5, 6)])
+65 65 | def test_implicit_str_concat_with_multi_parens(param1, param2, param3):
+66 66 |     ...
+67 67 | 
+
+PT006.py:69:26: PT006 [*] Wrong type passed to first argument of `@pytest.mark.parametrize`; expected `tuple`
+   |
+69 | @pytest.mark.parametrize(("param1,param2"), [(1, 2), (3, 4)])
+   |                          ^^^^^^^^^^^^^^^^^ PT006
+70 | def test_csv_with_parens(param1, param2):
+71 |     ...
+   |
+   = help: Use a `tuple` for the first argument
+
+ℹ Unsafe fix
+66 66 |     ...
+67 67 | 
+68 68 | 
+69    |-@pytest.mark.parametrize(("param1,param2"), [(1, 2), (3, 4)])
+   69 |+@pytest.mark.parametrize(("param1", "param2"), [(1, 2), (3, 4)])
+70 70 | def test_csv_with_parens(param1, param2):
+71 71 |     ...
+72 72 | 
+
+PT006.py:74:39: PT006 [*] Wrong type passed to first argument of `@pytest.mark.parametrize`; expected `tuple`
+   |
+74 | parametrize = pytest.mark.parametrize(("param1,param2"), [(1, 2), (3, 4)])
+   |                                       ^^^^^^^^^^^^^^^^^ PT006
+75 | 
+76 | @parametrize
+   |
+   = help: Use a `tuple` for the first argument
+
+ℹ Unsafe fix
+71 71 |     ...
+72 72 | 
+73 73 | 
+74    |-parametrize = pytest.mark.parametrize(("param1,param2"), [(1, 2), (3, 4)])
+   74 |+parametrize = pytest.mark.parametrize(("param1", "param2"), [(1, 2), (3, 4)])
+75 75 | 
+76 76 | @parametrize
+77 77 | def test_not_decorator(param1, param2):
+
+PT006.py:81:35: PT006 [*] Wrong type passed to first argument of `@pytest.mark.parametrize`; expected `tuple`
+   |
+81 | @pytest.mark.parametrize(argnames=("param1,param2"), argvalues=[(1, 2), (3, 4)])
+   |                                   ^^^^^^^^^^^^^^^^^ PT006
+82 | def test_keyword_arguments(param1, param2):
+83 |     ...
+   |
+   = help: Use a `tuple` for the first argument
+
+ℹ Unsafe fix
+78 78 |     ...
+79 79 | 
+80 80 | 
+81    |-@pytest.mark.parametrize(argnames=("param1,param2"), argvalues=[(1, 2), (3, 4)])
+   81 |+@pytest.mark.parametrize(argnames=("param1", "param2"), argvalues=[(1, 2), (3, 4)])
+82 82 | def test_keyword_arguments(param1, param2):
+83 83 |     ...