From 51fd2f8b2567df8aea0a70f05c0018c0cc5cbf04 Mon Sep 17 00:00:00 2001 From: Aarni Koskela Date: Mon, 22 Dec 2025 11:16:05 +0200 Subject: [PATCH] Add PT101: "pytest parametrize bool without ids" Co-authored-by: Claude --- .../fixtures/flake8_pytest_style/PT101.py | 71 +++++++++++++++ .../src/checkers/ast/analyze/expression.rs | 1 + crates/ruff_linter/src/codes.rs | 1 + .../src/rules/flake8_pytest_style/mod.rs | 6 ++ .../flake8_pytest_style/rules/parametrize.rs | 86 +++++++++++++++++++ ...es__flake8_pytest_style__tests__PT101.snap | 59 +++++++++++++ ruff.schema.json | 3 + 7 files changed, 227 insertions(+) create mode 100644 crates/ruff_linter/resources/test/fixtures/flake8_pytest_style/PT101.py create mode 100644 crates/ruff_linter/src/rules/flake8_pytest_style/snapshots/ruff_linter__rules__flake8_pytest_style__tests__PT101.snap diff --git a/crates/ruff_linter/resources/test/fixtures/flake8_pytest_style/PT101.py b/crates/ruff_linter/resources/test/fixtures/flake8_pytest_style/PT101.py new file mode 100644 index 0000000000..4922f568ff --- /dev/null +++ b/crates/ruff_linter/resources/test/fixtures/flake8_pytest_style/PT101.py @@ -0,0 +1,71 @@ +import pytest + + +# Error: boolean value without ids +@pytest.mark.parametrize("flag", [True, False]) +def test_error_simple(flag): + ... + + +# Error: boolean values in tuples without ids +@pytest.mark.parametrize(("x", "flag"), [(1, True), (2, False)]) +def test_error_multi_param(x, flag): + ... + + +# Error: boolean value in list without ids +@pytest.mark.parametrize("enabled", [True]) +def test_error_single_bool(enabled): + ... + + +# Error: multiple boolean values without ids +@pytest.mark.parametrize("x", [True, True, False]) +def test_error_multiple_bools(x): + ... + + +# Error: nested booleans without ids +@pytest.mark.parametrize( + ("a", "b", "c"), + [ + (1, True, "foo"), + (2, False, "bar"), + ], +) +def test_error_nested(a, b, c): + ... + + +# OK: has ids keyword argument +@pytest.mark.parametrize("flag", [True, False], ids=["enabled", "disabled"]) +def test_ok_with_ids(flag): + ... + + +# OK: no boolean values +@pytest.mark.parametrize("x", [1, 2, 3]) +def test_ok_no_bools(x): + ... + + +# OK: strings, not booleans +@pytest.mark.parametrize("x", ["True", "False"]) +def test_ok_strings(x): + ... + + +# OK: multiple params, no booleans +@pytest.mark.parametrize(("x", "y"), [(1, 2), (3, 4)]) +def test_ok_multi_no_bools(x, y): + ... + + +# OK: has ids even with nested booleans +@pytest.mark.parametrize( + ("x", "flag"), + [(1, True), (2, False)], + ids=["case1", "case2"], +) +def test_ok_nested_with_ids(x, flag): + ... diff --git a/crates/ruff_linter/src/checkers/ast/analyze/expression.rs b/crates/ruff_linter/src/checkers/ast/analyze/expression.rs index 0957aee346..c3dd264372 100644 --- a/crates/ruff_linter/src/checkers/ast/analyze/expression.rs +++ b/crates/ruff_linter/src/checkers/ast/analyze/expression.rs @@ -996,6 +996,7 @@ pub(crate) fn expression(expr: &Expr, checker: &Checker) { Rule::PytestParametrizeNamesWrongType, Rule::PytestParametrizeValuesWrongType, Rule::PytestDuplicateParametrizeTestCases, + Rule::PytestParametrizeBoolWithoutIds, ]) { flake8_pytest_style::rules::parametrize(checker, call); } diff --git a/crates/ruff_linter/src/codes.rs b/crates/ruff_linter/src/codes.rs index 74cb38bddb..023bced8e7 100644 --- a/crates/ruff_linter/src/codes.rs +++ b/crates/ruff_linter/src/codes.rs @@ -877,6 +877,7 @@ pub fn code_to_rule(linter: Linter, code: &str) -> Option<(RuleGroup, Rule)> { (Flake8PytestStyle, "029") => rules::flake8_pytest_style::rules::PytestWarnsWithoutWarning, (Flake8PytestStyle, "030") => rules::flake8_pytest_style::rules::PytestWarnsTooBroad, (Flake8PytestStyle, "031") => rules::flake8_pytest_style::rules::PytestWarnsWithMultipleStatements, + (Flake8PytestStyle, "101") => rules::flake8_pytest_style::rules::PytestParametrizeBoolWithoutIds, // flake8-pie (Flake8Pie, "790") => rules::flake8_pie::rules::UnnecessaryPlaceholder, 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 5923d978ec..a6f4825da5 100644 --- a/crates/ruff_linter/src/rules/flake8_pytest_style/mod.rs +++ b/crates/ruff_linter/src/rules/flake8_pytest_style/mod.rs @@ -195,6 +195,12 @@ mod tests { Settings::default(), "PT015" )] + #[test_case( + Rule::PytestParametrizeBoolWithoutIds, + Path::new("PT101.py"), + Settings::default(), + "PT101" + )] #[test_case( Rule::PytestFailWithoutMessage, Path::new("PT016.py"), 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 904ca1c494..d0e09e6d8b 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 @@ -284,6 +284,59 @@ impl Violation for PytestDuplicateParametrizeTestCases { } } +/// ## What it does +/// Checks for `pytest.mark.parametrize` calls where the parameter values contain +/// booleans but no `ids` keyword argument is provided. +/// +/// ## Why is this bad? +/// When using boolean values in parametrized tests, the test names can be unclear +/// without explicit IDs. For example, `test_foo[True]` and `test_foo[False]` don't +/// convey what the boolean represents. +/// +/// Providing explicit `ids` makes test output more readable and helps identify +/// which test case failed. +/// +/// ## Example +/// +/// ```python +/// import pytest +/// +/// +/// @pytest.mark.parametrize("flag", [True, False]) +/// def test_foo(flag): ... +/// +/// +/// @pytest.mark.parametrize(("x", "flag"), [(1, True), (2, False)]) +/// def test_bar(x, flag): ... +/// ``` +/// +/// Use instead: +/// +/// ```python +/// import pytest +/// +/// +/// @pytest.mark.parametrize("flag", [True, False], ids=["enabled", "disabled"]) +/// def test_foo(flag): ... +/// +/// +/// @pytest.mark.parametrize(("x", "flag"), [(1, True), (2, False)], ids=["case1", "case2"]) +/// def test_bar(x, flag): ... +/// ``` +/// +/// ## References +/// - [`pytest` documentation: How to parametrize fixtures and test functions](https://docs.pytest.org/en/latest/how-to/parametrize.html#pytest-mark-parametrize) +#[derive(ViolationMetadata)] +#[violation_metadata(stable_since = "v0.0.0")] +pub(crate) struct PytestParametrizeBoolWithoutIds; + +impl Violation for PytestParametrizeBoolWithoutIds { + #[derive_message_formats] + fn message(&self) -> String { + "Boolean value in `pytest.mark.parametrize` without `ids` argument".to_string() + } +} + fn elts_to_csv(elts: &[Expr], generator: Generator, flags: StringLiteralFlags) -> Option { if !elts.iter().all(Expr::is_string_literal_expr) { return None; @@ -848,6 +901,36 @@ fn handle_value_rows( } } +fn expr_contains_boolean(expr: &Expr) -> bool { + match expr { + Expr::BooleanLiteral(_) => true, + Expr::Tuple(ast::ExprTuple { elts, .. }) | Expr::List(ast::ExprList { elts, .. }) => { + elts.iter().any(expr_contains_boolean) + } + _ => false, + } +} + +/// PT101 +fn check_boolean_without_ids(checker: &Checker, call: &ExprCall) { + let Some(values) = call.arguments.find_argument_value("argvalues", 1) else { + // No argvalues? Nothing to check. + return; + }; + + if call.arguments.find_keyword("ids").is_some() { + // Have `ids`, this is fine. + return; + } + + if !expr_contains_boolean(values) { + // No boolean values, this is fine. + return; + } + + checker.report_diagnostic(PytestParametrizeBoolWithoutIds, call.range()); +} + pub(crate) fn parametrize(checker: &Checker, call: &ExprCall) { if !is_pytest_parametrize(call, checker.semantic()) { return; @@ -874,4 +957,7 @@ pub(crate) fn parametrize(checker: &Checker, call: &ExprCall) { check_duplicates(checker, values); } } + if checker.is_rule_enabled(Rule::PytestParametrizeBoolWithoutIds) { + check_boolean_without_ids(checker, call); + } } diff --git a/crates/ruff_linter/src/rules/flake8_pytest_style/snapshots/ruff_linter__rules__flake8_pytest_style__tests__PT101.snap b/crates/ruff_linter/src/rules/flake8_pytest_style/snapshots/ruff_linter__rules__flake8_pytest_style__tests__PT101.snap new file mode 100644 index 0000000000..af7fc23d3c --- /dev/null +++ b/crates/ruff_linter/src/rules/flake8_pytest_style/snapshots/ruff_linter__rules__flake8_pytest_style__tests__PT101.snap @@ -0,0 +1,59 @@ +--- +source: crates/ruff_linter/src/rules/flake8_pytest_style/mod.rs +--- +PT101 Boolean value in `pytest.mark.parametrize` without `ids` argument + --> PT101.py:5:2 + | +4 | # Error: boolean value without ids +5 | @pytest.mark.parametrize("flag", [True, False]) + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ +6 | def test_error_simple(flag): +7 | ... + | + +PT101 Boolean value in `pytest.mark.parametrize` without `ids` argument + --> PT101.py:11:2 + | +10 | # Error: boolean values in tuples without ids +11 | @pytest.mark.parametrize(("x", "flag"), [(1, True), (2, False)]) + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ +12 | def test_error_multi_param(x, flag): +13 | ... + | + +PT101 Boolean value in `pytest.mark.parametrize` without `ids` argument + --> PT101.py:17:2 + | +16 | # Error: boolean value in list without ids +17 | @pytest.mark.parametrize("enabled", [True]) + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ +18 | def test_error_single_bool(enabled): +19 | ... + | + +PT101 Boolean value in `pytest.mark.parametrize` without `ids` argument + --> PT101.py:23:2 + | +22 | # Error: multiple boolean values without ids +23 | @pytest.mark.parametrize("x", [True, True, False]) + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ +24 | def test_error_multiple_bools(x): +25 | ... + | + +PT101 Boolean value in `pytest.mark.parametrize` without `ids` argument + --> PT101.py:29:2 + | +28 | # Error: nested booleans without ids +29 | @pytest.mark.parametrize( + | __^ +30 | | ("a", "b", "c"), +31 | | [ +32 | | (1, True, "foo"), +33 | | (2, False, "bar"), +34 | | ], +35 | | ) + | |_^ +36 | def test_error_nested(a, b, c): +37 | ... + | diff --git a/ruff.schema.json b/ruff.schema.json index 5af8837779..c7b7d1cbca 100644 --- a/ruff.schema.json +++ b/ruff.schema.json @@ -3850,6 +3850,9 @@ "PT03", "PT030", "PT031", + "PT1", + "PT10", + "PT101", "PTH", "PTH1", "PTH10",