Skip to content
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.

Sign up for GitHub

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

[flake8-pytest-style] Autofix PT014 #6698

Merged
merged 7 commits into from
Aug 21, 2023
+211 −16
Merged

[flake8-pytest-style] Autofix PT014 #6698

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
7ea6180
Autofix PT014
harupy Aug 20, 2023
a16cdad
Address comments
harupy Aug 20, 2023
0e11ed2
Test case for parentheses
harupy Aug 20, 2023
d2e8e77
Add tests
harupy Aug 20, 2023
fd74b22
Handle parentheses
harupy Aug 20, 2023
695bf90
Merge branch 'main' into autofix-PT014
charliermarsh Aug 21, 2023
66eebf3
Rename
charliermarsh Aug 21, 2023
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
29 changes: 28 additions & 1 deletion crates/ruff/resources/test/fixtures/flake8_pytest_style/PT014.py
View file
Open in desktop
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
View file
Open in desktop
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
143 changes: 134 additions & 9 deletions ...c/rules/flake8_pytest_style/snapshots/ruff__rules__flake8_pytest_style__tests__PT014.snap
View file
Open in desktop
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):