From 7d2e40be2d0a49909331b067fcc530ea8e045649 Mon Sep 17 00:00:00 2001
From: InSync <insyncwithfoo@gmail.com>
Date: Thu, 13 Feb 2025 15:36:11 +0700
Subject: [PATCH] [`pylint`] Do not offer fix for raw strings (`PLE251`)
 (#16132)

## Summary

Resolves #13294, follow-up to #13882.

At #13882, it was concluded that a fix should not be offered for raw
strings. This change implements that. The five rules in question are now
no longer always fixable.

## Test Plan

`cargo nextest run` and `cargo insta test`.

---------

Co-authored-by: Micha Reiser <micha@reiser.io>
---
 .../fixtures/pylint/invalid_characters.py     | Bin 1639 -> 1762 bytes
 crates/ruff_linter/src/checkers/tokens.rs     |   8 +--
 .../pylint/rules/invalid_string_characters.rs |  68 ++++++++++--------
 ..._tests__PLE2510_invalid_characters.py.snap | Bin 1891 -> 2698 bytes
 ..._tests__PLE2512_invalid_characters.py.snap | Bin 2398 -> 3205 bytes
 ..._tests__PLE2513_invalid_characters.py.snap | Bin 2338 -> 3211 bytes
 ..._tests__PLE2514_invalid_characters.py.snap | Bin 808 -> 1603 bytes
 ..._tests__PLE2515_invalid_characters.py.snap | Bin 6766 -> 7613 bytes
 crates/ruff_python_parser/src/token.rs        |  27 +++++--
 9 files changed, 63 insertions(+), 40 deletions(-)

diff --git a/crates/ruff_linter/resources/test/fixtures/pylint/invalid_characters.py b/crates/ruff_linter/resources/test/fixtures/pylint/invalid_characters.py
index 79c9307695a9153b9945e2976c45335259630bfa..10c8b4202f0fdee64087d36b7fe3d45c58591d28 100644
GIT binary patch
delta 92
zcmaFP^N4ptIvW!g*JM669bZFZBTEymf}+g45{)7yB_$3iX@*A)-CRJXCR8{Lhj>sL
Gx_SV9k{8_o

delta 7
OcmaFF`<!P(IvW5E8Up|T

diff --git a/crates/ruff_linter/src/checkers/tokens.rs b/crates/ruff_linter/src/checkers/tokens.rs
index cb0eecfad1..dc157ba3e0 100644
--- a/crates/ruff_linter/src/checkers/tokens.rs
+++ b/crates/ruff_linter/src/checkers/tokens.rs
@@ -8,7 +8,6 @@ use ruff_python_ast::PySourceType;
 use ruff_python_codegen::Stylist;
 use ruff_python_index::Indexer;
 use ruff_python_parser::Tokens;
-use ruff_text_size::Ranged;
 
 use crate::directives::TodoComment;
 use crate::registry::{AsRule, Rule};
@@ -88,12 +87,7 @@ pub(crate) fn check_tokens(
         Rule::InvalidCharacterZeroWidthSpace,
     ]) {
         for token in tokens {
-            pylint::rules::invalid_string_characters(
-                &mut diagnostics,
-                token.kind(),
-                token.range(),
-                locator,
-            );
+            pylint::rules::invalid_string_characters(&mut diagnostics, token, locator);
         }
     }
 
diff --git a/crates/ruff_linter/src/rules/pylint/rules/invalid_string_characters.rs b/crates/ruff_linter/src/rules/pylint/rules/invalid_string_characters.rs
index efa6219a5d..7a30ee0763 100644
--- a/crates/ruff_linter/src/rules/pylint/rules/invalid_string_characters.rs
+++ b/crates/ruff_linter/src/rules/pylint/rules/invalid_string_characters.rs
@@ -1,9 +1,7 @@
-use ruff_diagnostics::AlwaysFixableViolation;
-use ruff_diagnostics::Edit;
-use ruff_diagnostics::{Diagnostic, DiagnosticKind, Fix};
+use ruff_diagnostics::{Diagnostic, DiagnosticKind, Edit, Fix, FixAvailability, Violation};
 use ruff_macros::{derive_message_formats, ViolationMetadata};
-use ruff_python_parser::TokenKind;
-use ruff_text_size::{TextLen, TextRange, TextSize};
+use ruff_python_parser::{Token, TokenKind};
+use ruff_text_size::{Ranged, TextLen, TextRange, TextSize};
 
 use crate::Locator;
 
@@ -29,14 +27,16 @@ use crate::Locator;
 #[derive(ViolationMetadata)]
 pub(crate) struct InvalidCharacterBackspace;
 
-impl AlwaysFixableViolation for InvalidCharacterBackspace {
+impl Violation for InvalidCharacterBackspace {
+    const FIX_AVAILABILITY: FixAvailability = FixAvailability::Sometimes;
+
     #[derive_message_formats]
     fn message(&self) -> String {
         "Invalid unescaped character backspace, use \"\\b\" instead".to_string()
     }
 
-    fn fix_title(&self) -> String {
-        "Replace with escape sequence".to_string()
+    fn fix_title(&self) -> Option<String> {
+        Some("Replace with escape sequence".to_string())
     }
 }
 
@@ -62,14 +62,16 @@ impl AlwaysFixableViolation for InvalidCharacterBackspace {
 #[derive(ViolationMetadata)]
 pub(crate) struct InvalidCharacterSub;
 
-impl AlwaysFixableViolation for InvalidCharacterSub {
+impl Violation for InvalidCharacterSub {
+    const FIX_AVAILABILITY: FixAvailability = FixAvailability::Sometimes;
+
     #[derive_message_formats]
     fn message(&self) -> String {
         "Invalid unescaped character SUB, use \"\\x1A\" instead".to_string()
     }
 
-    fn fix_title(&self) -> String {
-        "Replace with escape sequence".to_string()
+    fn fix_title(&self) -> Option<String> {
+        Some("Replace with escape sequence".to_string())
     }
 }
 
@@ -95,14 +97,16 @@ impl AlwaysFixableViolation for InvalidCharacterSub {
 #[derive(ViolationMetadata)]
 pub(crate) struct InvalidCharacterEsc;
 
-impl AlwaysFixableViolation for InvalidCharacterEsc {
+impl Violation for InvalidCharacterEsc {
+    const FIX_AVAILABILITY: FixAvailability = FixAvailability::Sometimes;
+
     #[derive_message_formats]
     fn message(&self) -> String {
         "Invalid unescaped character ESC, use \"\\x1B\" instead".to_string()
     }
 
-    fn fix_title(&self) -> String {
-        "Replace with escape sequence".to_string()
+    fn fix_title(&self) -> Option<String> {
+        Some("Replace with escape sequence".to_string())
     }
 }
 
@@ -128,14 +132,16 @@ impl AlwaysFixableViolation for InvalidCharacterEsc {
 #[derive(ViolationMetadata)]
 pub(crate) struct InvalidCharacterNul;
 
-impl AlwaysFixableViolation for InvalidCharacterNul {
+impl Violation for InvalidCharacterNul {
+    const FIX_AVAILABILITY: FixAvailability = FixAvailability::Sometimes;
+
     #[derive_message_formats]
     fn message(&self) -> String {
         "Invalid unescaped character NUL, use \"\\0\" instead".to_string()
     }
 
-    fn fix_title(&self) -> String {
-        "Replace with escape sequence".to_string()
+    fn fix_title(&self) -> Option<String> {
+        Some("Replace with escape sequence".to_string())
     }
 }
 
@@ -160,28 +166,29 @@ impl AlwaysFixableViolation for InvalidCharacterNul {
 #[derive(ViolationMetadata)]
 pub(crate) struct InvalidCharacterZeroWidthSpace;
 
-impl AlwaysFixableViolation for InvalidCharacterZeroWidthSpace {
+impl Violation for InvalidCharacterZeroWidthSpace {
+    const FIX_AVAILABILITY: FixAvailability = FixAvailability::Sometimes;
+
     #[derive_message_formats]
     fn message(&self) -> String {
         "Invalid unescaped character zero-width-space, use \"\\u200B\" instead".to_string()
     }
 
-    fn fix_title(&self) -> String {
-        "Replace with escape sequence".to_string()
+    fn fix_title(&self) -> Option<String> {
+        Some("Replace with escape sequence".to_string())
     }
 }
 
 /// PLE2510, PLE2512, PLE2513, PLE2514, PLE2515
 pub(crate) fn invalid_string_characters(
     diagnostics: &mut Vec<Diagnostic>,
-    token: TokenKind,
-    range: TextRange,
+    token: &Token,
     locator: &Locator,
 ) {
-    let text = match token {
+    let text = match token.kind() {
         // We can't use the `value` field since it's decoded and e.g. for f-strings removed a curly
         // brace that escaped another curly brace, which would gives us wrong column information.
-        TokenKind::String | TokenKind::FStringMiddle => locator.slice(range),
+        TokenKind::String | TokenKind::FStringMiddle => locator.slice(token),
         _ => return,
     };
 
@@ -198,11 +205,16 @@ pub(crate) fn invalid_string_characters(
             }
         };
 
-        let location = range.start() + TextSize::try_from(column).unwrap();
+        let location = token.start() + TextSize::try_from(column).unwrap();
         let range = TextRange::at(location, c.text_len());
 
-        diagnostics.push(Diagnostic::new(rule, range).with_fix(Fix::safe_edit(
-            Edit::range_replacement(replacement.to_string(), range),
-        )));
+        let mut diagnostic = Diagnostic::new(rule, range);
+
+        if !token.unwrap_string_flags().is_raw_string() {
+            let edit = Edit::range_replacement(replacement.to_string(), range);
+            diagnostic.set_fix(Fix::safe_edit(edit));
+        }
+
+        diagnostics.push(diagnostic);
     }
 }
diff --git a/crates/ruff_linter/src/rules/pylint/snapshots/ruff_linter__rules__pylint__tests__PLE2510_invalid_characters.py.snap b/crates/ruff_linter/src/rules/pylint/snapshots/ruff_linter__rules__pylint__tests__PLE2510_invalid_characters.py.snap
index b6f94b3dbf93fc373de656603c596573875ca7c6..46b9cb6a3fcc51cd380e08960dd32ac972ad28c5 100644
GIT binary patch
delta 443
zcmaFN*Co0knO&7DGp{T$Co?5JIU}(sF}WnQs93L{(#p)l%FxhCA;8Dg$kfnaavhT$
zlbP}4|7?2khQ>ygCR}DF3N;D^MVWae8bwM<N{=RVNIjY`o8eJIH<y9}PznL!V1{s+
znF1w%iZ!{+%;37y(6mq1WYkx|;U+UHLn9P7x#DpKQMQ2{J6VxcOx4UB?l=p$<ATyq
d{f5tR7U+(1!{bXXqO60vj?>H%>N=nc^Z=xrjGO=f

delta 7
OcmeAYeayEZnH>NN&jPCe

diff --git a/crates/ruff_linter/src/rules/pylint/snapshots/ruff_linter__rules__pylint__tests__PLE2512_invalid_characters.py.snap b/crates/ruff_linter/src/rules/pylint/snapshots/ruff_linter__rules__pylint__tests__PLE2512_invalid_characters.py.snap
index d90e38a14cc21944adda7ac288f240b8d822c2ff..88630ccb7a5a9f399caa6c67079dbf5708a872b7 100644
GIT binary patch
delta 381
zcmca7)GE0lj+4X8PyvW)6egczl;ATnu`)EWQV8&IH8M3cntY$lipk7)axaUXs-dxw
zr3sgr2~cH0QD$C=Mv;<|(xV9-QjaFgW_Z-l&848AP&2uJMNHMq6sQI$smWz#23MVi
zrhIZQlm6rjjADFdW>$vAC^ov{v4kkIAx_C>5mz-chdadr?v$W3RF@;1Vu9`yH#|Pz
SBFb>MTR6=up>6@%rw0Hpjd(u*

delta 7
OcmZpbyeG6FjuQY2vjV07

diff --git a/crates/ruff_linter/src/rules/pylint/snapshots/ruff_linter__rules__pylint__tests__PLE2513_invalid_characters.py.snap b/crates/ruff_linter/src/rules/pylint/snapshots/ruff_linter__rules__pylint__tests__PLE2513_invalid_characters.py.snap
index 72e7b93e0765fc61cbae9d7b81ab2dafc87a4992..eefd991446f13a4076b3572854890058b4337440 100644
GIT binary patch
delta 403
zcmZ1^)GfI|iBsInPyvW)6u8Wc6o3fCFjg=#p1goVkJZrF$kJr;MrH{<GZQOAV=ILK
zA6Fw&L*vPEOjb+~H40!gTxKRfodrdic_kV}N=izPCUi(WnlPK;QA79SJM7Y`W~M-C
zptvTNnHgMR8oEjjM*Yc~nZ@|b%&ZJeP%Lx9Z9W%KW^*Yh0PUUpkX=mG%pC3(3%FZ?
g($HKDcZ&tOTio$@fr}`^5su+BvxGWkG6$m`0I=nGod5s;

delta 7
OcmeB{TqLwXi4y<{IRZ2Q

diff --git a/crates/ruff_linter/src/rules/pylint/snapshots/ruff_linter__rules__pylint__tests__PLE2514_invalid_characters.py.snap b/crates/ruff_linter/src/rules/pylint/snapshots/ruff_linter__rules__pylint__tests__PLE2514_invalid_characters.py.snap
index 49b80e52e968e879ab7851fce926d64001e83047..a06d6f645f0905583da6091e65bc4dbae2121ad3 100644
GIT binary patch
literal 1603
zcmb`Hzi!(w5XLj}5i%XrK}lL!mL)T`fQ%gq1VNA>Q{5OeO;th_StX_X0W@^z5FktT
zJV{^2Ptv0p+p#Sdh8u$(NZg6v_q#hfjziR>lw5?sB`bt-Wf?`oG|37fT_rj0Q#{Qd
z^pSfr8MmY&4yGm9ElZQ}kjG3ij?t=>KZO2q*bU+Q^mC`{1@QIg8+<WwP-a4LmWwgW
zGvMs%^b?dyfHi1akYuV5Y)k;4B0hx*vNDAeux*?8$FJKxz0(o+2e_VHMf~18KB^P?
zs|h;m1Y(iqAzX+&Wn92rQp8|%ff7GUk#RxD^!FdQWRZX<xhH-AxGEhroczEqME2m`
z8=<5w;Hp4NT~~{jRgTs_6RaJ4)CYrht*-|SlKmysK^t)44Vz&1-wXB<-3DEz6f-NU
z?6pV|cm}wEWTQ({$Q`LSF<ou~l-ZzkojW3Z!+p*a^)^TjI)JOTJFhl6*-!u)gkG>{
z_Z?fE`gq|7h>Iduq3eFfo-9W#KDlw3DkMuC71vYgCQ6k!i=N-<2ZZXT%q33cfwU}Z
z`uw;*eSX=S{(4%B2g0a-i!q_PV=?@YP(5HKKWh5G_T$q05>Og?Z=Ql(*=%;Xy_Fe;
pH5j_>4GjHy7#J5(1I_=2(R&vd|0o&V+RH{7`t7M{U_`aG<G%=w)ZYLA

delta 7
OcmX@ivx04d1~UK)X98IO

diff --git a/crates/ruff_linter/src/rules/pylint/snapshots/ruff_linter__rules__pylint__tests__PLE2515_invalid_characters.py.snap b/crates/ruff_linter/src/rules/pylint/snapshots/ruff_linter__rules__pylint__tests__PLE2515_invalid_characters.py.snap
index 6e29b04cba64817955aa91bafccd05297dd63d3c..5b50a43e0589e0a5e7ffc43a4c5d55acabc371e3 100644
GIT binary patch
delta 407
zcmaE7ve$Y;uGHifF-bl%6DvbgD}?|bS0htH)5#C{JOj*(6>1ce6*5Xn3W}}t_0uy;
zGD?&5lJj%*6N^iV5_5EmGxUo})6(=ai;GKBi}ekSjVw*L%uIl~3W_rGN;Ha;l$0J#
z=#Y9eVK&2~hVF^?WmL^ffwDkhO)fJtxVkhng_GkY^e4{~65}&7vobV8ag-}=<GF}3
yn@d4Kp=PonkA$k3Iov50aHj;Np}HC26bp2xxZ&{u7g2`8-NI>R33Ut5K0N@Ic7-AU

delta 9
QcmdmM{mx`Vt`s8|02RIiT>t<8

diff --git a/crates/ruff_python_parser/src/token.rs b/crates/ruff_python_parser/src/token.rs
index 436651fab6..9574e4c23c 100644
--- a/crates/ruff_python_parser/src/token.rs
+++ b/crates/ruff_python_parser/src/token.rs
@@ -14,7 +14,7 @@ use ruff_python_ast::str::{Quote, TripleQuotes};
 use ruff_python_ast::str_prefix::{
     AnyStringPrefix, ByteStringPrefix, FStringPrefix, StringLiteralPrefix,
 };
-use ruff_python_ast::{BoolOp, Int, IpyEscapeKind, Operator, StringFlags, UnaryOp};
+use ruff_python_ast::{AnyStringFlags, BoolOp, Int, IpyEscapeKind, Operator, StringFlags, UnaryOp};
 use ruff_text_size::{Ranged, TextRange};
 
 #[derive(Clone, Copy, PartialEq, Eq)]
@@ -50,8 +50,7 @@ impl Token {
     ///
     /// If it isn't a string or any f-string tokens.
     pub fn is_triple_quoted_string(self) -> bool {
-        assert!(self.is_any_string());
-        self.flags.is_triple_quoted()
+        self.unwrap_string_flags().is_triple_quoted()
     }
 
     /// Returns the [`Quote`] style for the current string token of any kind.
@@ -60,8 +59,26 @@ impl Token {
     ///
     /// If it isn't a string or any f-string tokens.
     pub fn string_quote_style(self) -> Quote {
-        assert!(self.is_any_string());
-        self.flags.quote_style()
+        self.unwrap_string_flags().quote_style()
+    }
+
+    /// Returns the [`AnyStringFlags`] style for the current string token of any kind.
+    ///
+    /// # Panics
+    ///
+    /// If it isn't a string or any f-string tokens.
+    pub fn unwrap_string_flags(self) -> AnyStringFlags {
+        self.string_flags()
+            .unwrap_or_else(|| panic!("token to be a string"))
+    }
+
+    /// Returns true if the current token is a string and it is raw.
+    pub fn string_flags(self) -> Option<AnyStringFlags> {
+        if self.is_any_string() {
+            Some(self.flags.as_any_string_flags())
+        } else {
+            None
+        }
     }
 
     /// Returns `true` if this is any kind of string token.