Skip to content

[flake8-pytest-style] Don't recommend usefixtures for parametrize values in PT019 #17650

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 7 commits into from
May 14, 2025
Original file line number Diff line number Diff line change
Expand Up @@ -12,3 +12,43 @@ def test_xxx(_fixture): # Error arg

def test_xxx(*, _fixture): # Error kwonly
pass

# https://github.com/astral-sh/ruff/issues/17599

@pytest.mark.parametrize("_foo", [1, 2, 3])
def test_thingy(_foo): # Ok defined in parametrize
pass

@pytest.mark.parametrize(
"_test_input,_expected",
[("3+5", 8), ("2+4", 6), pytest.param("6*9", 42, marks=pytest.mark.xfail)],
)
def test_eval(_test_input, _expected): # OK defined in parametrize
pass


@pytest.mark.parametrize("_foo", [1, 2, 3])
def test_thingy2(_foo, _bar): # Error _bar is not defined in parametrize
pass

@pytest.mark.parametrize(["_foo", "_bar"], [1, 2, 3])
def test_thingy3(_foo, _bar): # OK defined in parametrize
pass

@pytest.mark.parametrize(("_foo"), [1, 2, 3])
def test_thingy4(_foo, _bar): # Error _bar is not defined in parametrize
pass

@pytest.mark.parametrize([" _foo", " _bar "], [1, 2, 3])
def test_thingy5(_foo, _bar): # OK defined in parametrize
pass

x = ["_foo", "_bar"]
@pytest.mark.parametrize(x, [1, 2, 3])
def test_thingy5(_foo, _bar): # OK defined in parametrize
pass

x = "_bar"
@pytest.mark.parametrize(["_foo", x], [1, 2, 3])
def test_thingy5(_foo, _bar): # OK defined in parametrize
pass
49 changes: 46 additions & 3 deletions crates/ruff_linter/src/rules/flake8_pytest_style/rules/fixture.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
use ruff_diagnostics::{AlwaysFixableViolation, Violation};
use ruff_diagnostics::{Diagnostic, Edit, Fix};
use ruff_macros::{derive_message_formats, ViolationMetadata};
use ruff_python_ast::helpers::map_callable;
use ruff_python_ast::name::UnqualifiedName;
use ruff_python_ast::visitor;
use ruff_python_ast::visitor::Visitor;
Expand All @@ -11,6 +12,7 @@ use ruff_python_semantic::SemanticModel;
use ruff_source_file::LineRanges;
use ruff_text_size::Ranged;
use ruff_text_size::{TextLen, TextRange};
use rustc_hash::FxHashSet;

use crate::checkers::ast::Checker;
use crate::fix::edits;
Expand Down Expand Up @@ -807,10 +809,51 @@ fn check_fixture_returns(checker: &Checker, name: &str, body: &[Stmt], returns:
}

/// PT019
fn check_test_function_args(checker: &Checker, parameters: &Parameters) {
fn check_test_function_args(checker: &Checker, parameters: &Parameters, decorators: &[Decorator]) {
let mut named_parametrize = FxHashSet::default();
for decorator in decorators.iter().filter(|decorator| {
UnqualifiedName::from_expr(map_callable(&decorator.expression))
.is_some_and(|name| matches!(name.segments(), ["pytest", "mark", "parametrize"]))
}) {
let Some(call_expr) = decorator.expression.as_call_expr() else {
continue;
};
let Some(first_arg) = call_expr.arguments.find_argument_value("argnames", 0) else {
continue;
};

match first_arg {
Expr::StringLiteral(ast::ExprStringLiteral { value, .. }) => {
named_parametrize.extend(
value
.to_str()
.split(',')
.map(str::trim)
.filter(|param| !param.is_empty() && param.starts_with('_')),
);
}

Expr::Name(_) => return,
Expr::List(ast::ExprList { elts, .. }) | Expr::Tuple(ast::ExprTuple { elts, .. })
if elts.iter().any(Expr::is_name_expr) =>
{
return
}
Expr::List(ast::ExprList { elts, .. }) | Expr::Tuple(ast::ExprTuple { elts, .. }) => {
named_parametrize.extend(
elts.iter()
.filter_map(Expr::as_string_literal_expr)
.map(|param| param.value.to_str().trim())
.filter(|param| !param.is_empty() && param.starts_with('_')),
);
}
_ => {}
}
}

for parameter in parameters.iter_non_variadic_params() {
let name = parameter.name();
if name.starts_with('_') {
if name.starts_with('_') && !named_parametrize.contains(name.as_str()) {
checker.report_diagnostic(Diagnostic::new(
PytestFixtureParamWithoutValue {
name: name.to_string(),
Expand Down Expand Up @@ -915,6 +958,6 @@ pub(crate) fn fixture(
}

if checker.enabled(Rule::PytestFixtureParamWithoutValue) && name.starts_with("test_") {
check_test_function_args(checker, parameters);
check_test_function_args(checker, parameters, decorators);
}
}
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
---
source: crates/ruff_linter/src/rules/flake8_pytest_style/mod.rs
snapshot_kind: text
---
PT019.py:9:14: PT019 Fixture `_fixture` without value is injected as parameter, use `@pytest.mark.usefixtures` instead
|
Expand All @@ -15,3 +14,19 @@ PT019.py:13:17: PT019 Fixture `_fixture` without value is injected as parameter,
| ^^^^^^^^ PT019
14 | pass
|

PT019.py:31:24: PT019 Fixture `_bar` without value is injected as parameter, use `@pytest.mark.usefixtures` instead
|
30 | @pytest.mark.parametrize("_foo", [1, 2, 3])
31 | def test_thingy2(_foo, _bar): # Error _bar is not defined in parametrize
| ^^^^ PT019
32 | pass
|

PT019.py:39:24: PT019 Fixture `_bar` without value is injected as parameter, use `@pytest.mark.usefixtures` instead
|
38 | @pytest.mark.parametrize(("_foo"), [1, 2, 3])
39 | def test_thingy4(_foo, _bar): # Error _bar is not defined in parametrize
| ^^^^ PT019
40 | pass
|
Loading