Skip to content

Commit

Permalink
[flake8-pyi] Mark fix as unsafe when type annotation contains comme…
Browse files Browse the repository at this point in the history 
…nts for `duplicate-literal-member` (`PYI062`) (astral-sh#14268)
  • Loading branch information
@sbrugman
sbrugman authored Nov 11, 2024
1 parent 9180635 commit f8aae9b
Show file tree
Hide file tree
Showing 5 changed files with 181 additions and 107 deletions.
6 changes: 6 additions & 0 deletions crates/ruff_linter/resources/test/fixtures/flake8_pyi/PYI062.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,12 @@
Literal[1, Literal[2], Literal[2]] # once
t.Literal[1, t.Literal[2, t.Literal[1]]] # once
typing_extensions.Literal[1, 1, 1] # twice
Literal[
1, # comment
Literal[ # another comment
1
]
] # once

# Ensure issue is only raised once, even on nested literals
MyType = Literal["foo", Literal[True, False, True], "bar"] # PYI062
Expand Down
12 changes: 9 additions & 3 deletions crates/ruff_linter/resources/test/fixtures/flake8_pyi/PYI062.pyi
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2,18 +2,24 @@ from typing import Literal
import typing as t
import typing_extensions

x: Literal[True, False, True, False] # PY062 twice here
x: Literal[True, False, True, False] # PYI062 twice here

y: Literal[1, print("hello"), 3, Literal[4, 1]] # PY062 on the last 1
y: Literal[1, print("hello"), 3, Literal[4, 1]] # PYI062 on the last 1

z: Literal[{1, 3, 5}, "foobar", {1,3,5}] # PY062 on the set literal
z: Literal[{1, 3, 5}, "foobar", {1,3,5}] # PYI062 on the set literal

Literal[1, Literal[1]] # once
Literal[1, 2, Literal[1, 2]] # twice
Literal[1, Literal[1], Literal[1]] # twice
Literal[1, Literal[2], Literal[2]] # once
t.Literal[1, t.Literal[2, t.Literal[1]]] # once
typing_extensions.Literal[1, 1, 1] # twice
Literal[
1, # comment
Literal[ # another comment
1
]
] # once

# Ensure issue is only raised once, even on nested literals
MyType = Literal["foo", Literal[True, False, True], "bar"] # PYI062
Expand Down
64 changes: 36 additions & 28 deletions crates/ruff_linter/src/rules/flake8_pyi/rules/duplicate_literal_member.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ use std::collections::HashSet;

use rustc_hash::FxHashSet;

use ruff_diagnostics::{AlwaysFixableViolation, Diagnostic, Edit, Fix};
use ruff_diagnostics::{AlwaysFixableViolation, Applicability, Diagnostic, Edit, Fix};
use ruff_macros::{derive_message_formats, violation};
use ruff_python_ast::comparable::ComparableExpr;
use ruff_python_ast::{self as ast, Expr, ExprContext};
Expand All @@ -28,8 +28,10 @@ use crate::checkers::ast::Checker;
/// ```
///
/// ## Fix safety
/// This rule's fix is marked as safe; however, the fix will flatten nested
/// literals into a single top-level literal.
/// This rule's fix is marked as safe, unless the type annotation contains comments.
///
/// Note that the fix will flatten nested literals into a single top-level
/// literal.
///
/// ## References
/// - [Python documentation: `typing.Literal`](https://docs.python.org/3/library/typing.html#typing.Literal)
Expand Down Expand Up @@ -73,33 +75,39 @@ pub(crate) fn duplicate_literal_member<'a>(checker: &mut Checker, expr: &'a Expr
// Traverse the literal, collect all diagnostic members.
traverse_literal(&mut check_for_duplicate_members, checker.semantic(), expr);

if diagnostics.is_empty() {
return;
}

// If there's at least one diagnostic, create a fix to remove the duplicate members.
if !diagnostics.is_empty() {
if let Expr::Subscript(subscript) = expr {
let subscript = Expr::Subscript(ast::ExprSubscript {
slice: Box::new(if let [elt] = unique_nodes.as_slice() {
(*elt).clone()
} else {
Expr::Tuple(ast::ExprTuple {
elts: unique_nodes.into_iter().cloned().collect(),
range: TextRange::default(),
ctx: ExprContext::Load,
parenthesized: false,
})
}),
value: subscript.value.clone(),
range: TextRange::default(),
ctx: ExprContext::Load,
});
let fix = Fix::safe_edit(Edit::range_replacement(
checker.generator().expr(&subscript),
expr.range(),
));
for diagnostic in &mut diagnostics {
diagnostic.set_fix(fix.clone());
}
if let Expr::Subscript(subscript) = expr {
let subscript = Expr::Subscript(ast::ExprSubscript {
slice: Box::new(if let [elt] = unique_nodes.as_slice() {
(*elt).clone()
} else {
Expr::Tuple(ast::ExprTuple {
elts: unique_nodes.into_iter().cloned().collect(),
range: TextRange::default(),
ctx: ExprContext::Load,
parenthesized: false,
})
}),
value: subscript.value.clone(),
range: TextRange::default(),
ctx: ExprContext::Load,
});
let fix = Fix::applicable_edit(
Edit::range_replacement(checker.generator().expr(&subscript), expr.range()),
if checker.comment_ranges().intersects(expr.range()) {
Applicability::Unsafe
} else {
Applicability::Safe
},
);
for diagnostic in &mut diagnostics {
diagnostic.set_fix(fix.clone());
}
}
};

checker.diagnostics.append(&mut diagnostics);
}
77 changes: 52 additions & 25 deletions ...c/rules/flake8_pyi/snapshots/ruff_linter__rules__flake8_pyi__tests__PYI062_PYI062.py.snap
View file
Original file line number Diff line number Diff line change
Expand Up @@ -207,7 +207,7 @@ PYI062.py:14:32: PYI062 [*] Duplicate literal member `2`
14 |+Literal[1, 2] # once
15 15 | t.Literal[1, t.Literal[2, t.Literal[1]]] # once
16 16 | typing_extensions.Literal[1, 1, 1] # twice
17 17 |
17 17 | Literal[

PYI062.py:15:37: PYI062 [*] Duplicate literal member `1`
|
Expand All @@ -216,6 +216,7 @@ PYI062.py:15:37: PYI062 [*] Duplicate literal member `1`
15 | t.Literal[1, t.Literal[2, t.Literal[1]]] # once
| ^ PYI062
16 | typing_extensions.Literal[1, 1, 1] # twice
17 | Literal[
|
= help: Remove duplicates

Expand All @@ -226,17 +227,17 @@ PYI062.py:15:37: PYI062 [*] Duplicate literal member `1`
15 |-t.Literal[1, t.Literal[2, t.Literal[1]]] # once
15 |+t.Literal[1, 2] # once
16 16 | typing_extensions.Literal[1, 1, 1] # twice
17 17 |
18 18 | # Ensure issue is only raised once, even on nested literals
17 17 | Literal[
18 18 | 1, # comment

PYI062.py:16:30: PYI062 [*] Duplicate literal member `1`
|
14 | Literal[1, Literal[2], Literal[2]] # once
15 | t.Literal[1, t.Literal[2, t.Literal[1]]] # once
16 | typing_extensions.Literal[1, 1, 1] # twice
| ^ PYI062
17 |
18 | # Ensure issue is only raised once, even on nested literals
17 | Literal[
18 | 1, # comment
|
= help: Remove duplicates

Expand All @@ -246,18 +247,18 @@ PYI062.py:16:30: PYI062 [*] Duplicate literal member `1`
15 15 | t.Literal[1, t.Literal[2, t.Literal[1]]] # once
16 |-typing_extensions.Literal[1, 1, 1] # twice
16 |+typing_extensions.Literal[1] # twice
17 17 |
18 18 | # Ensure issue is only raised once, even on nested literals
19 19 | MyType = Literal["foo", Literal[True, False, True], "bar"] # PYI062
17 17 | Literal[
18 18 | 1, # comment
19 19 | Literal[ # another comment

PYI062.py:16:33: PYI062 [*] Duplicate literal member `1`
|
14 | Literal[1, Literal[2], Literal[2]] # once
15 | t.Literal[1, t.Literal[2, t.Literal[1]]] # once
16 | typing_extensions.Literal[1, 1, 1] # twice
| ^ PYI062
17 |
18 | # Ensure issue is only raised once, even on nested literals
17 | Literal[
18 | 1, # comment
|
= help: Remove duplicates

Expand All @@ -267,25 +268,51 @@ PYI062.py:16:33: PYI062 [*] Duplicate literal member `1`
15 15 | t.Literal[1, t.Literal[2, t.Literal[1]]] # once
16 |-typing_extensions.Literal[1, 1, 1] # twice
16 |+typing_extensions.Literal[1] # twice
17 17 |
18 18 | # Ensure issue is only raised once, even on nested literals
19 19 | MyType = Literal["foo", Literal[True, False, True], "bar"] # PYI062
17 17 | Literal[
18 18 | 1, # comment
19 19 | Literal[ # another comment

PYI062.py:19:46: PYI062 [*] Duplicate literal member `True`
PYI062.py:20:9: PYI062 [*] Duplicate literal member `1`
|
18 | # Ensure issue is only raised once, even on nested literals
19 | MyType = Literal["foo", Literal[True, False, True], "bar"] # PYI062
18 | 1, # comment
19 | Literal[ # another comment
20 | 1
| ^ PYI062
21 | ]
22 | ] # once
|
= help: Remove duplicates

ℹ Unsafe fix
14 14 | Literal[1, Literal[2], Literal[2]] # once
15 15 | t.Literal[1, t.Literal[2, t.Literal[1]]] # once
16 16 | typing_extensions.Literal[1, 1, 1] # twice
17 |-Literal[
18 |- 1, # comment
19 |- Literal[ # another comment
20 |- 1
21 |- ]
22 |-] # once
17 |+Literal[1] # once
23 18 |
24 19 | # Ensure issue is only raised once, even on nested literals
25 20 | MyType = Literal["foo", Literal[True, False, True], "bar"] # PYI062

PYI062.py:25:46: PYI062 [*] Duplicate literal member `True`
|
24 | # Ensure issue is only raised once, even on nested literals
25 | MyType = Literal["foo", Literal[True, False, True], "bar"] # PYI062
| ^^^^ PYI062
20 |
21 | n: Literal["No", "duplicates", "here", 1, "1"]
26 |
27 | n: Literal["No", "duplicates", "here", 1, "1"]
|
= help: Remove duplicates

ℹ Safe fix
16 16 | typing_extensions.Literal[1, 1, 1] # twice
17 17 |
18 18 | # Ensure issue is only raised once, even on nested literals
19 |-MyType = Literal["foo", Literal[True, False, True], "bar"] # PYI062
19 |+MyType = Literal["foo", True, False, "bar"] # PYI062
20 20 |
21 21 | n: Literal["No", "duplicates", "here", 1, "1"]
22 22 | ] # once
23 23 |
24 24 | # Ensure issue is only raised once, even on nested literals
25 |-MyType = Literal["foo", Literal[True, False, True], "bar"] # PYI062
25 |+MyType = Literal["foo", True, False, "bar"] # PYI062
26 26 |
27 27 | n: Literal["No", "duplicates", "here", 1, "1"]
Loading

0 comments on commit f8aae9b

Please sign in to comment.