From b312b53c2eafd97a084f9f2961a99fbfa257f21a Mon Sep 17 00:00:00 2001 From: Vasco Schiavo <115561717+VascoSch92@users.noreply.github.com> Date: Sun, 23 Feb 2025 22:22:14 +0100 Subject: [PATCH] [`flake8-pyi`] Mark `PYI030` fix unsafe when comments are deleted (#16322) --- .../test/fixtures/flake8_pyi/PYI030.py | 12 +++++ .../test/fixtures/flake8_pyi/PYI030.pyi | 12 +++++ .../rules/unnecessary_literal_union.rs | 31 ++++++++--- ...__flake8_pyi__tests__PYI030_PYI030.py.snap | 52 ++++++++++++++++++- ..._flake8_pyi__tests__PYI030_PYI030.pyi.snap | 52 ++++++++++++++++++- 5 files changed, 150 insertions(+), 9 deletions(-) diff --git a/crates/ruff_linter/resources/test/fixtures/flake8_pyi/PYI030.py b/crates/ruff_linter/resources/test/fixtures/flake8_pyi/PYI030.py index 59edf252dd..70e7fd956e 100644 --- a/crates/ruff_linter/resources/test/fixtures/flake8_pyi/PYI030.py +++ b/crates/ruff_linter/resources/test/fixtures/flake8_pyi/PYI030.py @@ -91,3 +91,15 @@ field25: typing.Union[typing_extensions.Literal[1], typing.Union[Literal[2], typ 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 +) diff --git a/crates/ruff_linter/resources/test/fixtures/flake8_pyi/PYI030.pyi b/crates/ruff_linter/resources/test/fixtures/flake8_pyi/PYI030.pyi index c6cd2b4537..67fbbe48b6 100644 --- a/crates/ruff_linter/resources/test/fixtures/flake8_pyi/PYI030.pyi +++ b/crates/ruff_linter/resources/test/fixtures/flake8_pyi/PYI030.pyi @@ -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 +) diff --git a/crates/ruff_linter/src/rules/flake8_pyi/rules/unnecessary_literal_union.rs b/crates/ruff_linter/src/rules/flake8_pyi/rules/unnecessary_literal_union.rs index a2fd4f27b6..10db113bb3 100644 --- a/crates/ruff_linter/src/rules/flake8_pyi/rules/unnecessary_literal_union.rs +++ b/crates/ruff_linter/src/rules/flake8_pyi/rules/unnecessary_literal_union.rs @@ -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)] @@ -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 = std::iter::once(literal) .chain(other_exprs.into_iter().cloned()) @@ -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) } }); diff --git a/crates/ruff_linter/src/rules/flake8_pyi/snapshots/ruff_linter__rules__flake8_pyi__tests__PYI030_PYI030.py.snap b/crates/ruff_linter/src/rules/flake8_pyi/snapshots/ruff_linter__rules__flake8_pyi__tests__PYI030_PYI030.py.snap index d649aeb619..4e92aa2078 100644 --- a/crates/ruff_linter/src/rules/flake8_pyi/snapshots/ruff_linter__rules__flake8_pyi__tests__PYI030_PYI030.py.snap +++ b/crates/ruff_linter/src/rules/flake8_pyi/snapshots/ruff_linter__rules__flake8_pyi__tests__PYI030_PYI030.py.snap @@ -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 @@ -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` @@ -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 | ) diff --git a/crates/ruff_linter/src/rules/flake8_pyi/snapshots/ruff_linter__rules__flake8_pyi__tests__PYI030_PYI030.pyi.snap b/crates/ruff_linter/src/rules/flake8_pyi/snapshots/ruff_linter__rules__flake8_pyi__tests__PYI030_PYI030.pyi.snap index 86f3114beb..d4788e5b03 100644 --- a/crates/ruff_linter/src/rules/flake8_pyi/snapshots/ruff_linter__rules__flake8_pyi__tests__PYI030_PYI030.pyi.snap +++ b/crates/ruff_linter/src/rules/flake8_pyi/snapshots/ruff_linter__rules__flake8_pyi__tests__PYI030_PYI030.pyi.snap @@ -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 @@ -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` @@ -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 | )