Skip to content

Commit

Permalink
[flake8-pytest-style] Autofix PT014 (#6698)
Browse files Browse the repository at this point in the history
  • Loading branch information
harupy authored Aug 21, 2023
1 parent 1b7e4a1 commit 086e110
Show file tree
Hide file tree
Showing 3 changed files with 211 additions and 16 deletions.
29 changes: 28 additions & 1 deletion crates/ruff/resources/test/fixtures/flake8_pytest_style/PT014.py
Original file line number Diff line number Diff line change
Expand Up @@ -16,11 +16,38 @@ def test_error_expr_simple(x):
...


@pytest.mark.parametrize("x", [(a, b), (a, b), (b, c)])
@pytest.mark.parametrize(
"x",
[
(a, b),
# comment
(a, b),
(b, c),
],
)
def test_error_expr_complex(x):
...


@pytest.mark.parametrize("x", [a, b, (a), c, ((a))])
def test_error_parentheses(x):
...


@pytest.mark.parametrize(
"x",
[
a,
b,
(a),
c,
((a)),
],
)
def test_error_parentheses_trailing_comma(x):
...


@pytest.mark.parametrize("x", [1, 2])
def test_ok(x):
...
55 changes: 49 additions & 6 deletions crates/ruff/src/rules/flake8_pytest_style/rules/parametrize.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,12 +5,13 @@ use ruff_python_ast::{
self as ast, Arguments, Constant, Decorator, Expr, ExprContext, PySourceType, Ranged,
};
use ruff_python_parser::{lexer, AsMode, Tok};
use ruff_text_size::TextRange;
use ruff_text_size::{TextRange, TextSize};

use ruff_diagnostics::{AutofixKind, Diagnostic, Edit, Fix, Violation};
use ruff_macros::{derive_message_formats, violation};
use ruff_python_ast::comparable::ComparableExpr;
use ruff_python_codegen::Generator;
use ruff_python_trivia::{SimpleTokenKind, SimpleTokenizer};
use ruff_source_file::Locator;

use crate::checkers::ast::Checker;
Expand Down Expand Up @@ -215,11 +216,17 @@ pub struct PytestDuplicateParametrizeTestCases {
}

impl Violation for PytestDuplicateParametrizeTestCases {
const AUTOFIX: AutofixKind = AutofixKind::Sometimes;

#[derive_message_formats]
fn message(&self) -> String {
let PytestDuplicateParametrizeTestCases { index } = self;
format!("Duplicate of test case at index {index} in `@pytest_mark.parametrize`")
}

fn autofix_title(&self) -> Option<String> {
Some("Remove duplicate test case".to_string())
}
}

fn elts_to_csv(elts: &[Expr], generator: Generator) -> Option<String> {
Expand Down Expand Up @@ -551,6 +558,21 @@ fn check_values(checker: &mut Checker, names: &Expr, values: &Expr) {
}
}

/// Given an element in a list, return the comma that follows it:
/// ```python
/// @pytest.mark.parametrize(
/// "x",
/// [.., (elt), ..],
/// ^^^^^
/// Tokenize this range to locate the comma.
/// )
/// ```
fn trailing_comma(element: &Expr, source: &str) -> Option<TextSize> {
SimpleTokenizer::starts_at(element.end(), source)
.find(|token| token.kind == SimpleTokenKind::Comma)
.map(|token| token.start())
}

/// PT014
fn check_duplicates(checker: &mut Checker, values: &Expr) {
let (Expr::List(ast::ExprList { elts, .. }) | Expr::Tuple(ast::ExprTuple { elts, .. })) =
Expand All @@ -561,16 +583,37 @@ fn check_duplicates(checker: &mut Checker, values: &Expr) {

let mut seen: FxHashMap<ComparableExpr, usize> =
FxHashMap::with_capacity_and_hasher(elts.len(), BuildHasherDefault::default());
for (index, elt) in elts.iter().enumerate() {
let expr = ComparableExpr::from(elt);
let mut prev = None;
for (index, element) in elts.iter().enumerate() {
let expr = ComparableExpr::from(element);
seen.entry(expr)
.and_modify(|index| {
checker.diagnostics.push(Diagnostic::new(
let mut diagnostic = Diagnostic::new(
PytestDuplicateParametrizeTestCases { index: *index },
elt.range(),
));
element.range(),
);
if checker.patch(diagnostic.kind.rule()) {
if let Some(prev) = prev {
let values_end = values.range().end() - TextSize::new(1);
let previous_end = trailing_comma(prev, checker.locator().contents())
.unwrap_or(values_end);
let element_end = trailing_comma(element, checker.locator().contents())
.unwrap_or(values_end);
let deletion_range = TextRange::new(previous_end, element_end);
if !checker
.indexer()
.comment_ranges()
.intersects(deletion_range)
{
diagnostic
.set_fix(Fix::suggested(Edit::range_deletion(deletion_range)));
}
}
}
checker.diagnostics.push(diagnostic);
})
.or_insert(index);
prev = Some(element);
}
}

Expand Down
Original file line number Diff line number Diff line change
@@ -1,44 +1,169 @@
---
source: crates/ruff/src/rules/flake8_pytest_style/mod.rs
---
PT014.py:4:35: PT014 Duplicate of test case at index 0 in `@pytest_mark.parametrize`
PT014.py:4:35: PT014 [*] Duplicate of test case at index 0 in `@pytest_mark.parametrize`
|
4 | @pytest.mark.parametrize("x", [1, 1, 2])
| ^ PT014
5 | def test_error_literal(x):
6 | ...
|
= help: Remove duplicate test case

PT014.py:14:35: PT014 Duplicate of test case at index 0 in `@pytest_mark.parametrize`
Suggested fix
1 1 | import pytest
2 2 |
3 3 |
4 |-@pytest.mark.parametrize("x", [1, 1, 2])
4 |+@pytest.mark.parametrize("x", [1, 2])
5 5 | def test_error_literal(x):
6 6 | ...
7 7 |

PT014.py:14:35: PT014 [*] Duplicate of test case at index 0 in `@pytest_mark.parametrize`
|
14 | @pytest.mark.parametrize("x", [a, a, b, b, b, c])
| ^ PT014
15 | def test_error_expr_simple(x):
16 | ...
|
= help: Remove duplicate test case

Suggested fix
11 11 | c = 3
12 12 |
13 13 |
14 |-@pytest.mark.parametrize("x", [a, a, b, b, b, c])
14 |+@pytest.mark.parametrize("x", [a, b, b, b, c])
15 15 | def test_error_expr_simple(x):
16 16 | ...
17 17 |

PT014.py:14:41: PT014 Duplicate of test case at index 2 in `@pytest_mark.parametrize`
PT014.py:14:41: PT014 [*] Duplicate of test case at index 2 in `@pytest_mark.parametrize`
|
14 | @pytest.mark.parametrize("x", [a, a, b, b, b, c])
| ^ PT014
15 | def test_error_expr_simple(x):
16 | ...
|
= help: Remove duplicate test case

Suggested fix
11 11 | c = 3
12 12 |
13 13 |
14 |-@pytest.mark.parametrize("x", [a, a, b, b, b, c])
14 |+@pytest.mark.parametrize("x", [a, a, b, b, c])
15 15 | def test_error_expr_simple(x):
16 16 | ...
17 17 |

PT014.py:14:44: PT014 Duplicate of test case at index 2 in `@pytest_mark.parametrize`
PT014.py:14:44: PT014 [*] Duplicate of test case at index 2 in `@pytest_mark.parametrize`
|
14 | @pytest.mark.parametrize("x", [a, a, b, b, b, c])
| ^ PT014
15 | def test_error_expr_simple(x):
16 | ...
|
= help: Remove duplicate test case

Suggested fix
11 11 | c = 3
12 12 |
13 13 |
14 |-@pytest.mark.parametrize("x", [a, a, b, b, b, c])
14 |+@pytest.mark.parametrize("x", [a, a, b, b, c])
15 15 | def test_error_expr_simple(x):
16 16 | ...
17 17 |

PT014.py:24:9: PT014 Duplicate of test case at index 0 in `@pytest_mark.parametrize`
|
22 | (a, b),
23 | # comment
24 | (a, b),
| ^^^^^^ PT014
25 | (b, c),
26 | ],
|
= help: Remove duplicate test case

PT014.py:32:39: PT014 [*] Duplicate of test case at index 0 in `@pytest_mark.parametrize`
|
32 | @pytest.mark.parametrize("x", [a, b, (a), c, ((a))])
| ^ PT014
33 | def test_error_parentheses(x):
34 | ...
|
= help: Remove duplicate test case

PT014.py:19:40: PT014 Duplicate of test case at index 0 in `@pytest_mark.parametrize`
Suggested fix
29 29 | ...
30 30 |
31 31 |
32 |-@pytest.mark.parametrize("x", [a, b, (a), c, ((a))])
32 |+@pytest.mark.parametrize("x", [a, b, c, ((a))])
33 33 | def test_error_parentheses(x):
34 34 | ...
35 35 |

PT014.py:32:48: PT014 [*] Duplicate of test case at index 0 in `@pytest_mark.parametrize`
|
32 | @pytest.mark.parametrize("x", [a, b, (a), c, ((a))])
| ^ PT014
33 | def test_error_parentheses(x):
34 | ...
|
= help: Remove duplicate test case

Suggested fix
29 29 | ...
30 30 |
31 31 |
32 |-@pytest.mark.parametrize("x", [a, b, (a), c, ((a))])
32 |+@pytest.mark.parametrize("x", [a, b, (a), c])
33 33 | def test_error_parentheses(x):
34 34 | ...
35 35 |

PT014.py:42:10: PT014 [*] Duplicate of test case at index 0 in `@pytest_mark.parametrize`
|
19 | @pytest.mark.parametrize("x", [(a, b), (a, b), (b, c)])
| ^^^^^^ PT014
20 | def test_error_expr_complex(x):
21 | ...
40 | a,
41 | b,
42 | (a),
| ^ PT014
43 | c,
44 | ((a)),
|
= help: Remove duplicate test case

Suggested fix
39 39 | [
40 40 | a,
41 41 | b,
42 |- (a),
43 42 | c,
44 43 | ((a)),
45 44 | ],

PT014.py:44:11: PT014 [*] Duplicate of test case at index 0 in `@pytest_mark.parametrize`
|
42 | (a),
43 | c,
44 | ((a)),
| ^ PT014
45 | ],
46 | )
|
= help: Remove duplicate test case

Suggested fix
41 41 | b,
42 42 | (a),
43 43 | c,
44 |- ((a)),
45 44 | ],
46 45 | )
47 46 | def test_error_parentheses_trailing_comma(x):


0 comments on commit 086e110

Please sign in to comment.