Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -12,3 +12,38 @@ 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 = "other"
@pytest.mark.parametrize(x, [1, 2, 3])
def test_thingy5(_foo, _bar): # known false negative, we don't try to resolve variables
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