mirror of https://github.com/astral-sh/ruff
[`flake8-pytest-style`] Don't recommend `usefixtures` for parametrize values in `PT019` (#17650)
<!-- Thank you for contributing to Ruff! To help us out with reviewing, please consider the following: - Does this pull request include a summary of the change? (See below.) - Does this pull request include a descriptive title? - Does this pull request include references to any relevant issues? --> ## Summary Fixes #17599. ## Test Plan Snapshot tests. --------- Co-authored-by: Brent Westbrook <36778786+ntBre@users.noreply.github.com>
This commit is contained in:
parent
97d7b46936
commit
9b52ae8991
|
|
@ -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
|
||||
|
|
|
|||
|
|
@ -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;
|
||||
|
|
@ -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;
|
||||
|
|
@ -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(),
|
||||
|
|
@ -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);
|
||||
}
|
||||
}
|
||||
|
|
|
|||
|
|
@ -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
|
||||
|
|
||||
|
|
@ -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…
Reference in New Issue