[flake8-pyi] Mark PYI030 fix unsafe when comments are deleted (astral-sh#16322)

Author: VascoSch92

crates/ruff_linter/resources/test/fixtures/flake8_pyi/PYI030.py

@@ -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
+)

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
+)

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<Expr> = 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)
         }
     });
 

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 | )

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 | )