diff --git a/crates/ruff_linter/resources/test/fixtures/flake8_pytest_style/PT006.py b/crates/ruff_linter/resources/test/fixtures/flake8_pytest_style/PT006.py index f444a835efbe5..04ff399e37cb3 100644 --- a/crates/ruff_linter/resources/test/fixtures/flake8_pytest_style/PT006.py +++ b/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): + ... diff --git a/crates/ruff_linter/src/checkers/ast/analyze/expression.rs b/crates/ruff_linter/src/checkers/ast/analyze/expression.rs index df623fdc40d6d..dd03609194556 100644 --- a/crates/ruff_linter/src/checkers/ast/analyze/expression.rs +++ b/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, diff --git a/crates/ruff_linter/src/checkers/ast/analyze/statement.rs b/crates/ruff_linter/src/checkers/ast/analyze/statement.rs index 48d8734fbac7c..e9521d5f752d5 100644 --- a/crates/ruff_linter/src/checkers/ast/analyze/statement.rs +++ b/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, diff --git a/crates/ruff_linter/src/rules/flake8_pytest_style/mod.rs b/crates/ruff_linter/src/rules/flake8_pytest_style/mod.rs index db190ccaad1ca..277470a9e99ba 100644 --- a/crates/ruff_linter/src/rules/flake8_pytest_style/mod.rs +++ b/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(()) + } } diff --git a/crates/ruff_linter/src/rules/flake8_pytest_style/rules/helpers.rs b/crates/ruff_linter/src/rules/flake8_pytest_style/rules/helpers.rs index edbe837e2c1dd..d09474023eb10 100644 --- a/crates/ruff_linter/src/rules/flake8_pytest_style/rules/helpers.rs +++ b/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"]) }) diff --git a/crates/ruff_linter/src/rules/flake8_pytest_style/rules/parametrize.rs b/crates/ruff_linter/src/rules/flake8_pytest_style/rules/parametrize.rs index 3b2a923b45e3d..c5b89f9e71552 100644 --- a/crates/ruff_linter/src/rules/flake8_pytest_style/rules/parametrize.rs +++ b/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 { /// /// 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 { - 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); } } } diff --git a/crates/ruff_linter/src/rules/flake8_pytest_style/snapshots/ruff_linter__rules__flake8_pytest_style__tests__PT006_default.snap b/crates/ruff_linter/src/rules/flake8_pytest_style/snapshots/ruff_linter__rules__flake8_pytest_style__tests__PT006_default.snap index ddd4e40f51797..6b690a9e1b722 100644 --- a/crates/ruff_linter/src/rules/flake8_pytest_style/snapshots/ruff_linter__rules__flake8_pytest_style__tests__PT006_default.snap +++ b/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 | diff --git a/crates/ruff_linter/src/rules/flake8_pytest_style/snapshots/ruff_linter__rules__flake8_pytest_style__tests__PT006_list.snap b/crates/ruff_linter/src/rules/flake8_pytest_style/snapshots/ruff_linter__rules__flake8_pytest_style__tests__PT006_list.snap index ed5e5b11cf60a..c4615acb10e67 100644 --- a/crates/ruff_linter/src/rules/flake8_pytest_style/snapshots/ruff_linter__rules__flake8_pytest_style__tests__PT006_list.snap +++ b/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 | diff --git a/crates/ruff_linter/src/rules/flake8_pytest_style/snapshots/ruff_linter__rules__flake8_pytest_style__tests__preview__PT006_PT006.py.snap b/crates/ruff_linter/src/rules/flake8_pytest_style/snapshots/ruff_linter__rules__flake8_pytest_style__tests__preview__PT006_PT006.py.snap new file mode 100644 index 0000000000000..7d63849f9f24f --- /dev/null +++ b/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 | ...