Skip to content

Commit

Permalink
[flake8-pyi] Mark PYI030 fix unsafe when comments are deleted (#1…
Browse files Browse the repository at this point in the history 
…6322)
  • Loading branch information
@VascoSch92
VascoSch92 authored Feb 23, 2025
1 parent c814745 commit b312b53
Show file tree
Hide file tree
Showing 5 changed files with 150 additions and 9 deletions.
12 changes: 12 additions & 0 deletions crates/ruff_linter/resources/test/fixtures/flake8_pyi/PYI030.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -91,3 +91,15 @@ def func2() -> Literal[1] | Literal[2]: # Error
from typing import IO, Literal

InlineOption = Literal["a"] | Literal["b"] | IO[str]

# Should use unsafe fix when comments are deleted
field26: (
# First comment
Literal["a", "b"]
# Second comment
| Literal["c", "d"]
)
field27: (
Literal["a", "b"] # First comment
| Literal["c", "d"] # Second comment
)
12 changes: 12 additions & 0 deletions crates/ruff_linter/resources/test/fixtures/flake8_pyi/PYI030.pyi
View file
Original file line number Diff line number Diff line change
Expand Up @@ -87,3 +87,15 @@ field24: typing.Union[Literal[1], typing.Union[Literal[2], typing.Union[Literal[

# Should use the first literal subscript attribute when fixing
field25: typing.Union[typing_extensions.Literal[1], typing.Union[Literal[2], typing.Union[Literal[3], Literal[4]]], str] # Error

# Should use unsafe fix when comments are deleted
field26: (
# First comment
Literal["a", "b"]
# Second comment
| Literal["c", "d"]
)
field27: (
Literal["a", "b"] # First comment
| Literal["c", "d"] # Second comment
)
31 changes: 24 additions & 7 deletions crates/ruff_linter/src/rules/flake8_pyi/rules/unnecessary_literal_union.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,22 @@ use crate::checkers::ast::Checker;
/// field: Literal[1, 2] | str
/// ```
///
/// ## Fix safety
/// This fix is marked unsafe if it would delete any comments within the replacement range.
///
/// An example to illustrate where comments are preserved and where they are not:
///
/// ```pyi
/// from typing import Literal
///
/// field: (
/// # deleted comment
/// Literal["a", "b"] # deleted comment
/// # deleted comment
/// | Literal["c", "d"] # preserved comment
/// )
/// ```
///
/// ## References
/// - [Python documentation: `typing.Literal`](https://docs.python.org/3/library/typing.html#typing.Literal)
#[derive(ViolationMetadata)]
Expand Down Expand Up @@ -131,12 +147,8 @@ pub(crate) fn unnecessary_literal_union<'a>(checker: &Checker, expr: &'a Expr) {
ctx: ExprContext::Load,
});

if other_exprs.is_empty() {
// if the union is only literals, we just replace the whole thing with a single literal
Fix::safe_edit(Edit::range_replacement(
checker.generator().expr(&literal),
expr.range(),
))
let edit = if other_exprs.is_empty() {
Edit::range_replacement(checker.generator().expr(&literal), expr.range())
} else {
let elts: Vec<Expr> = std::iter::once(literal)
.chain(other_exprs.into_iter().cloned())
Expand All @@ -159,8 +171,13 @@ pub(crate) fn unnecessary_literal_union<'a>(checker: &Checker, expr: &'a Expr) {
} else {
checker.generator().expr(&pep_604_union(&elts))
};
Edit::range_replacement(content, expr.range())
};

Fix::safe_edit(Edit::range_replacement(content, expr.range()))
if checker.comment_ranges().intersects(expr.range()) {
Fix::unsafe_edit(edit)
} else {
Fix::safe_edit(edit)
}
});

Expand Down
52 changes: 51 additions & 1 deletion ...c/rules/flake8_pyi/snapshots/ruff_linter__rules__flake8_pyi__tests__PYI030_PYI030.py.snap
View file
Original file line number Diff line number Diff line change
Expand Up @@ -377,7 +377,7 @@ PYI030.py:63:10: PYI030 [*] Multiple literal members in a union. Use a single li
|
= help: Replace with a single `Literal`

ℹ Safe fix
ℹ Unsafe fix
60 60 | field19: typing.Literal[1] | typing.Literal[2] # Error
61 61 |
62 62 | # Should emit in cases with newlines
Expand Down Expand Up @@ -538,6 +538,8 @@ PYI030.py:93:16: PYI030 [*] Multiple literal members in a union. Use a single li
92 |
93 | InlineOption = Literal["a"] | Literal["b"] | IO[str]
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ PYI030
94 |
95 | # Should use unsafe fix when comments are deleted
|
= help: Replace with a single `Literal`

Expand All @@ -547,3 +549,51 @@ PYI030.py:93:16: PYI030 [*] Multiple literal members in a union. Use a single li
92 92 |
93 |-InlineOption = Literal["a"] | Literal["b"] | IO[str]
93 |+InlineOption = Literal["a", "b"] | IO[str]
94 94 |
95 95 | # Should use unsafe fix when comments are deleted
96 96 | field26: (

PYI030.py:98:5: PYI030 [*] Multiple literal members in a union. Use a single literal, e.g. `Literal["a", "b", "c", "d"]`
|
96 | field26: (
97 | # First comment
98 | / Literal["a", "b"]
99 | | # Second comment
100 | | | Literal["c", "d"]
| |_______________________^ PYI030
101 | )
102 | field27: (
|
= help: Replace with a single `Literal`

ℹ Unsafe fix
95 95 | # Should use unsafe fix when comments are deleted
96 96 | field26: (
97 97 | # First comment
98 |- Literal["a", "b"]
99 |- # Second comment
100 |- | Literal["c", "d"]
98 |+ Literal["a", "b", "c", "d"]
101 99 | )
102 100 | field27: (
103 101 | Literal["a", "b"] # First comment

PYI030.py:103:5: PYI030 [*] Multiple literal members in a union. Use a single literal, e.g. `Literal["a", "b", "c", "d"]`
|
101 | )
102 | field27: (
103 | / Literal["a", "b"] # First comment
104 | | | Literal["c", "d"] # Second comment
| |_______________________^ PYI030
105 | )
|
= help: Replace with a single `Literal`

ℹ Unsafe fix
100 100 | | Literal["c", "d"]
101 101 | )
102 102 | field27: (
103 |- Literal["a", "b"] # First comment
104 |- | Literal["c", "d"] # Second comment
103 |+ Literal["a", "b", "c", "d"] # Second comment
105 104 | )
52 changes: 51 additions & 1 deletion .../rules/flake8_pyi/snapshots/ruff_linter__rules__flake8_pyi__tests__PYI030_PYI030.pyi.snap
View file
Original file line number Diff line number Diff line change
Expand Up @@ -377,7 +377,7 @@ PYI030.pyi:63:10: PYI030 [*] Multiple literal members in a union. Use a single l
|
= help: Replace with a single `Literal`

ℹ Safe fix
ℹ Unsafe fix
60 60 | field19: typing.Literal[1] | typing.Literal[2] # Error
61 61 |
62 62 | # Should emit in cases with newlines
Expand Down Expand Up @@ -517,6 +517,8 @@ PYI030.pyi:89:10: PYI030 [*] Multiple literal members in a union. Use a single l
88 | # Should use the first literal subscript attribute when fixing
89 | field25: typing.Union[typing_extensions.Literal[1], typing.Union[Literal[2], typing.Union[Literal[3], Literal[4]]], str] # Error
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ PYI030
90 |
91 | # Should use unsafe fix when comments are deleted
|
= help: Replace with a single `Literal`

Expand All @@ -526,3 +528,51 @@ PYI030.pyi:89:10: PYI030 [*] Multiple literal members in a union. Use a single l
88 88 | # Should use the first literal subscript attribute when fixing
89 |-field25: typing.Union[typing_extensions.Literal[1], typing.Union[Literal[2], typing.Union[Literal[3], Literal[4]]], str] # Error
89 |+field25: typing.Union[typing_extensions.Literal[1, 2, 3, 4], str] # Error
90 90 |
91 91 | # Should use unsafe fix when comments are deleted
92 92 | field26: (

PYI030.pyi:94:5: PYI030 [*] Multiple literal members in a union. Use a single literal, e.g. `Literal["a", "b", "c", "d"]`
|
92 | field26: (
93 | # First comment
94 | / Literal["a", "b"]
95 | | # Second comment
96 | | | Literal["c", "d"]
| |_______________________^ PYI030
97 | )
98 | field27: (
|
= help: Replace with a single `Literal`

ℹ Unsafe fix
91 91 | # Should use unsafe fix when comments are deleted
92 92 | field26: (
93 93 | # First comment
94 |- Literal["a", "b"]
95 |- # Second comment
96 |- | Literal["c", "d"]
94 |+ Literal["a", "b", "c", "d"]
97 95 | )
98 96 | field27: (
99 97 | Literal["a", "b"] # First comment

PYI030.pyi:99:5: PYI030 [*] Multiple literal members in a union. Use a single literal, e.g. `Literal["a", "b", "c", "d"]`
|
97 | )
98 | field27: (
99 | / Literal["a", "b"] # First comment
100 | | | Literal["c", "d"] # Second comment
| |_______________________^ PYI030
101 | )
|
= help: Replace with a single `Literal`

ℹ Unsafe fix
96 96 | | Literal["c", "d"]
97 97 | )
98 98 | field27: (
99 |- Literal["a", "b"] # First comment
100 |- | Literal["c", "d"] # Second comment
99 |+ Literal["a", "b", "c", "d"] # Second comment
101 100 | )

0 comments on commit b312b53

Please sign in to comment.