Skip to content

Commit

Permalink
[pylint] Do not offer fix for raw strings (PLE251) (#16132)
Browse files Browse the repository at this point in the history
## 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 <[email protected]>
  • Loading branch information
InSyncWithFoo and MichaReiser authored Feb 13, 2025
1 parent f8093b6 commit 7d2e40b
Show file tree
Hide file tree
Showing 9 changed files with 63 additions and 40 deletions.
Binary file not shown.
8 changes: 1 addition & 7 deletions crates/ruff_linter/src/checkers/tokens.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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};
Expand Down Expand Up @@ -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);
}
}

Expand Down
Original file line number Diff line number Diff line change
@@ -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;

Expand All @@ -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())
}
}

Expand All @@ -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())
}
}

Expand All @@ -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())
}
}

Expand All @@ -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())
}
}

Expand All @@ -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,
};

Expand All @@ -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);
}
}
Binary file not shown.
Binary file not shown.
Binary file not shown.
Binary file not shown.
Binary file not shown.
27 changes: 22 additions & 5 deletions crates/ruff_python_parser/src/token.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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)]
Expand Down Expand Up @@ -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.
Expand All @@ -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.
Expand Down

0 comments on commit 7d2e40b

Please sign in to comment.