[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>
This commit is contained in:
InSync 2025-02-13 15:36:11 +07:00 committed by GitHub
parent f8093b65ea
commit 7d2e40be2d
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
9 changed files with 63 additions and 40 deletions

View file

@ -8,7 +8,6 @@ use ruff_python_ast::PySourceType;
use ruff_python_codegen::Stylist; use ruff_python_codegen::Stylist;
use ruff_python_index::Indexer; use ruff_python_index::Indexer;
use ruff_python_parser::Tokens; use ruff_python_parser::Tokens;
use ruff_text_size::Ranged;
use crate::directives::TodoComment; use crate::directives::TodoComment;
use crate::registry::{AsRule, Rule}; use crate::registry::{AsRule, Rule};
@ -88,12 +87,7 @@ pub(crate) fn check_tokens(
Rule::InvalidCharacterZeroWidthSpace, Rule::InvalidCharacterZeroWidthSpace,
]) { ]) {
for token in tokens { for token in tokens {
pylint::rules::invalid_string_characters( pylint::rules::invalid_string_characters(&mut diagnostics, token, locator);
&mut diagnostics,
token.kind(),
token.range(),
locator,
);
} }
} }

View file

@ -1,9 +1,7 @@
use ruff_diagnostics::AlwaysFixableViolation; use ruff_diagnostics::{Diagnostic, DiagnosticKind, Edit, Fix, FixAvailability, Violation};
use ruff_diagnostics::Edit;
use ruff_diagnostics::{Diagnostic, DiagnosticKind, Fix};
use ruff_macros::{derive_message_formats, ViolationMetadata}; use ruff_macros::{derive_message_formats, ViolationMetadata};
use ruff_python_parser::TokenKind; use ruff_python_parser::{Token, TokenKind};
use ruff_text_size::{TextLen, TextRange, TextSize}; use ruff_text_size::{Ranged, TextLen, TextRange, TextSize};
use crate::Locator; use crate::Locator;
@ -29,14 +27,16 @@ use crate::Locator;
#[derive(ViolationMetadata)] #[derive(ViolationMetadata)]
pub(crate) struct InvalidCharacterBackspace; pub(crate) struct InvalidCharacterBackspace;
impl AlwaysFixableViolation for InvalidCharacterBackspace { impl Violation for InvalidCharacterBackspace {
const FIX_AVAILABILITY: FixAvailability = FixAvailability::Sometimes;
#[derive_message_formats] #[derive_message_formats]
fn message(&self) -> String { fn message(&self) -> String {
"Invalid unescaped character backspace, use \"\\b\" instead".to_string() "Invalid unescaped character backspace, use \"\\b\" instead".to_string()
} }
fn fix_title(&self) -> String { fn fix_title(&self) -> Option<String> {
"Replace with escape sequence".to_string() Some("Replace with escape sequence".to_string())
} }
} }
@ -62,14 +62,16 @@ impl AlwaysFixableViolation for InvalidCharacterBackspace {
#[derive(ViolationMetadata)] #[derive(ViolationMetadata)]
pub(crate) struct InvalidCharacterSub; pub(crate) struct InvalidCharacterSub;
impl AlwaysFixableViolation for InvalidCharacterSub { impl Violation for InvalidCharacterSub {
const FIX_AVAILABILITY: FixAvailability = FixAvailability::Sometimes;
#[derive_message_formats] #[derive_message_formats]
fn message(&self) -> String { fn message(&self) -> String {
"Invalid unescaped character SUB, use \"\\x1A\" instead".to_string() "Invalid unescaped character SUB, use \"\\x1A\" instead".to_string()
} }
fn fix_title(&self) -> String { fn fix_title(&self) -> Option<String> {
"Replace with escape sequence".to_string() Some("Replace with escape sequence".to_string())
} }
} }
@ -95,14 +97,16 @@ impl AlwaysFixableViolation for InvalidCharacterSub {
#[derive(ViolationMetadata)] #[derive(ViolationMetadata)]
pub(crate) struct InvalidCharacterEsc; pub(crate) struct InvalidCharacterEsc;
impl AlwaysFixableViolation for InvalidCharacterEsc { impl Violation for InvalidCharacterEsc {
const FIX_AVAILABILITY: FixAvailability = FixAvailability::Sometimes;
#[derive_message_formats] #[derive_message_formats]
fn message(&self) -> String { fn message(&self) -> String {
"Invalid unescaped character ESC, use \"\\x1B\" instead".to_string() "Invalid unescaped character ESC, use \"\\x1B\" instead".to_string()
} }
fn fix_title(&self) -> String { fn fix_title(&self) -> Option<String> {
"Replace with escape sequence".to_string() Some("Replace with escape sequence".to_string())
} }
} }
@ -128,14 +132,16 @@ impl AlwaysFixableViolation for InvalidCharacterEsc {
#[derive(ViolationMetadata)] #[derive(ViolationMetadata)]
pub(crate) struct InvalidCharacterNul; pub(crate) struct InvalidCharacterNul;
impl AlwaysFixableViolation for InvalidCharacterNul { impl Violation for InvalidCharacterNul {
const FIX_AVAILABILITY: FixAvailability = FixAvailability::Sometimes;
#[derive_message_formats] #[derive_message_formats]
fn message(&self) -> String { fn message(&self) -> String {
"Invalid unescaped character NUL, use \"\\0\" instead".to_string() "Invalid unescaped character NUL, use \"\\0\" instead".to_string()
} }
fn fix_title(&self) -> String { fn fix_title(&self) -> Option<String> {
"Replace with escape sequence".to_string() Some("Replace with escape sequence".to_string())
} }
} }
@ -160,28 +166,29 @@ impl AlwaysFixableViolation for InvalidCharacterNul {
#[derive(ViolationMetadata)] #[derive(ViolationMetadata)]
pub(crate) struct InvalidCharacterZeroWidthSpace; pub(crate) struct InvalidCharacterZeroWidthSpace;
impl AlwaysFixableViolation for InvalidCharacterZeroWidthSpace { impl Violation for InvalidCharacterZeroWidthSpace {
const FIX_AVAILABILITY: FixAvailability = FixAvailability::Sometimes;
#[derive_message_formats] #[derive_message_formats]
fn message(&self) -> String { fn message(&self) -> String {
"Invalid unescaped character zero-width-space, use \"\\u200B\" instead".to_string() "Invalid unescaped character zero-width-space, use \"\\u200B\" instead".to_string()
} }
fn fix_title(&self) -> String { fn fix_title(&self) -> Option<String> {
"Replace with escape sequence".to_string() Some("Replace with escape sequence".to_string())
} }
} }
/// PLE2510, PLE2512, PLE2513, PLE2514, PLE2515 /// PLE2510, PLE2512, PLE2513, PLE2514, PLE2515
pub(crate) fn invalid_string_characters( pub(crate) fn invalid_string_characters(
diagnostics: &mut Vec<Diagnostic>, diagnostics: &mut Vec<Diagnostic>,
token: TokenKind, token: &Token,
range: TextRange,
locator: &Locator, 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 // 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. // 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, _ => 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()); let range = TextRange::at(location, c.text_len());
diagnostics.push(Diagnostic::new(rule, range).with_fix(Fix::safe_edit( let mut diagnostic = Diagnostic::new(rule, range);
Edit::range_replacement(replacement.to_string(), 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);
} }
} }

View file

@ -14,7 +14,7 @@ use ruff_python_ast::str::{Quote, TripleQuotes};
use ruff_python_ast::str_prefix::{ use ruff_python_ast::str_prefix::{
AnyStringPrefix, ByteStringPrefix, FStringPrefix, StringLiteralPrefix, 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}; use ruff_text_size::{Ranged, TextRange};
#[derive(Clone, Copy, PartialEq, Eq)] #[derive(Clone, Copy, PartialEq, Eq)]
@ -50,8 +50,7 @@ impl Token {
/// ///
/// If it isn't a string or any f-string tokens. /// If it isn't a string or any f-string tokens.
pub fn is_triple_quoted_string(self) -> bool { pub fn is_triple_quoted_string(self) -> bool {
assert!(self.is_any_string()); self.unwrap_string_flags().is_triple_quoted()
self.flags.is_triple_quoted()
} }
/// Returns the [`Quote`] style for the current string token of any kind. /// 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. /// If it isn't a string or any f-string tokens.
pub fn string_quote_style(self) -> Quote { pub fn string_quote_style(self) -> Quote {
assert!(self.is_any_string()); self.unwrap_string_flags().quote_style()
self.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. /// Returns `true` if this is any kind of string token.