diff --git a/crates/ruff_linter/src/checkers/ast/analyze/string_like.rs b/crates/ruff_linter/src/checkers/ast/analyze/string_like.rs index 52f481a91a..3aaafe4d76 100644 --- a/crates/ruff_linter/src/checkers/ast/analyze/string_like.rs +++ b/crates/ruff_linter/src/checkers/ast/analyze/string_like.rs @@ -30,4 +30,7 @@ pub(crate) fn string_like(string_like: StringLike, checker: &mut Checker) { ]) { flake8_quotes::rules::check_string_quotes(checker, string_like); } + if checker.enabled(Rule::UnnecessaryEscapedQuote) { + flake8_quotes::rules::unnecessary_escaped_quote(checker, string_like); + } } diff --git a/crates/ruff_linter/src/checkers/tokens.rs b/crates/ruff_linter/src/checkers/tokens.rs index 431472093f..33c832edb1 100644 --- a/crates/ruff_linter/src/checkers/tokens.rs +++ b/crates/ruff_linter/src/checkers/tokens.rs @@ -126,10 +126,6 @@ pub(crate) fn check_tokens( flake8_quotes::rules::avoidable_escaped_quote(&mut diagnostics, tokens, locator, settings); } - if settings.rules.enabled(Rule::UnnecessaryEscapedQuote) { - flake8_quotes::rules::unnecessary_escaped_quote(&mut diagnostics, tokens, locator); - } - if settings.rules.any_enabled(&[ Rule::SingleLineImplicitStringConcatenation, Rule::MultiLineImplicitStringConcatenation, diff --git a/crates/ruff_linter/src/rules/flake8_quotes/helpers.rs b/crates/ruff_linter/src/rules/flake8_quotes/helpers.rs new file mode 100644 index 0000000000..4b3184e69a --- /dev/null +++ b/crates/ruff_linter/src/rules/flake8_quotes/helpers.rs @@ -0,0 +1,46 @@ +/// Return `true` if the haystack contains an escaped quote. +pub(super) fn contains_escaped_quote(haystack: &str, quote: char) -> bool { + for index in memchr::memchr_iter(quote as u8, haystack.as_bytes()) { + // If the quote is preceded by an even number of backslashes, it's not escaped. + if haystack.as_bytes()[..index] + .iter() + .rev() + .take_while(|&&c| c == b'\\') + .count() + % 2 + != 0 + { + return true; + } + } + false +} + +/// Return a modified version of the string with all quote escapes removed. +pub(super) fn unescape_string(haystack: &str, quote: char) -> String { + let mut fixed_contents = String::with_capacity(haystack.len()); + + let mut chars = haystack.chars().peekable(); + let mut backslashes = 0; + while let Some(char) = chars.next() { + if char != '\\' { + fixed_contents.push(char); + backslashes = 0; + continue; + } + // If we're at the end of the line + let Some(next_char) = chars.peek() else { + fixed_contents.push(char); + continue; + }; + // Remove quote escape + if *next_char == quote && backslashes % 2 == 0 { + backslashes = 0; + continue; + } + backslashes += 1; + fixed_contents.push(char); + } + + fixed_contents +} diff --git a/crates/ruff_linter/src/rules/flake8_quotes/mod.rs b/crates/ruff_linter/src/rules/flake8_quotes/mod.rs index 7ecfbd2d37..8a69e6c294 100644 --- a/crates/ruff_linter/src/rules/flake8_quotes/mod.rs +++ b/crates/ruff_linter/src/rules/flake8_quotes/mod.rs @@ -1,4 +1,5 @@ //! Rules from [flake8-quotes](https://pypi.org/project/flake8-quotes/). +mod helpers; pub(crate) mod rules; pub mod settings; diff --git a/crates/ruff_linter/src/rules/flake8_quotes/rules/avoidable_escaped_quote.rs b/crates/ruff_linter/src/rules/flake8_quotes/rules/avoidable_escaped_quote.rs index 7dd89e38c0..20a9ededa0 100644 --- a/crates/ruff_linter/src/rules/flake8_quotes/rules/avoidable_escaped_quote.rs +++ b/crates/ruff_linter/src/rules/flake8_quotes/rules/avoidable_escaped_quote.rs @@ -8,6 +8,7 @@ use ruff_text_size::TextRange; use crate::lex::docstring_detection::StateMachine; use crate::settings::LinterSettings; +use super::super::helpers::{contains_escaped_quote, unescape_string}; use super::super::settings::Quote; /// ## What it does @@ -48,43 +49,6 @@ impl AlwaysFixableViolation for AvoidableEscapedQuote { } } -/// ## What it does -/// Checks for strings that include unnecessarily escaped quotes. -/// -/// ## Why is this bad? -/// If a string contains an escaped quote that doesn't match the quote -/// character used for the string, it's unnecessary and can be removed. -/// -/// ## Example -/// ```python -/// foo = "bar\'s" -/// ``` -/// -/// Use instead: -/// ```python -/// foo = "bar's" -/// ``` -/// -/// ## Formatter compatibility -/// We recommend against using this rule alongside the [formatter]. The -/// formatter automatically removes unnecessary escapes, making the rule -/// redundant. -/// -/// [formatter]: https://docs.astral.sh/ruff/formatter -#[violation] -pub struct UnnecessaryEscapedQuote; - -impl AlwaysFixableViolation for UnnecessaryEscapedQuote { - #[derive_message_formats] - fn message(&self) -> String { - format!("Unnecessary escape on inner quote character") - } - - fn fix_title(&self) -> String { - "Remove backslash".to_string() - } -} - struct FStringContext { /// Whether to check for escaped quotes in the f-string. check_for_escaped_quote: bool, @@ -92,21 +56,14 @@ struct FStringContext { start_range: TextRange, /// The ranges of the f-string middle tokens containing escaped quotes. middle_ranges_with_escapes: Vec, - /// The quote style used for the f-string - quote_style: Quote, } impl FStringContext { - fn new( - check_for_escaped_quote: bool, - fstring_start_range: TextRange, - quote_style: Quote, - ) -> Self { + fn new(check_for_escaped_quote: bool, fstring_start_range: TextRange) -> Self { Self { check_for_escaped_quote, start_range: fstring_start_range, middle_ranges_with_escapes: vec![], - quote_style, } } @@ -207,11 +164,7 @@ pub(crate) fn avoidable_escaped_quote( // style and it isn't a triple-quoted f-string. let check_for_escaped_quote = !kind.is_triple_quoted() && Quote::from(kind.quote_style()) == quotes_settings.inline_quotes; - fstrings.push(FStringContext::new( - check_for_escaped_quote, - tok_range, - quotes_settings.inline_quotes, - )); + fstrings.push(FStringContext::new(check_for_escaped_quote, tok_range)); } Tok::FStringMiddle { value: string_contents, @@ -290,157 +243,7 @@ pub(crate) fn avoidable_escaped_quote( } } -/// Q004 -pub(crate) fn unnecessary_escaped_quote( - diagnostics: &mut Vec, - lxr: &[LexResult], - locator: &Locator, -) { - let mut fstrings: Vec = Vec::new(); - let mut state_machine = StateMachine::default(); - - for &(ref tok, tok_range) in lxr.iter().flatten() { - let is_docstring = state_machine.consume(tok); - if is_docstring { - continue; - } - - match tok { - Tok::String { - value: string_contents, - kind, - } => { - if kind.is_raw_string() || kind.is_triple_quoted() { - continue; - } - - let leading = kind.quote_style(); - if !contains_escaped_quote(string_contents, leading.opposite().as_char()) { - continue; - } - - let mut diagnostic = Diagnostic::new(UnnecessaryEscapedQuote, tok_range); - let fixed_contents = format!( - "{prefix}{quote}{value}{quote}", - prefix = kind.prefix(), - quote = leading.as_char(), - value = unescape_string(string_contents, leading.opposite().as_char()) - ); - diagnostic.set_fix(Fix::safe_edit(Edit::range_replacement( - fixed_contents, - tok_range, - ))); - diagnostics.push(diagnostic); - } - Tok::FStringStart(kind) => { - // Check for escaped quote only if we're using the preferred quotation - // style and it isn't a triple-quoted f-string. - let check_for_escaped_quote = !kind.is_triple_quoted(); - let quote_style = Quote::from(kind.quote_style()); - fstrings.push(FStringContext::new( - check_for_escaped_quote, - tok_range, - quote_style, - )); - } - Tok::FStringMiddle { - value: string_contents, - kind, - } if !kind.is_raw_string() => { - let Some(context) = fstrings.last_mut() else { - continue; - }; - if !context.check_for_escaped_quote { - continue; - } - if contains_escaped_quote(string_contents, context.quote_style.opposite().as_char()) - { - context.push_fstring_middle_range(tok_range); - } - } - Tok::FStringEnd => { - let Some(context) = fstrings.pop() else { - continue; - }; - let [first, rest @ ..] = context.middle_ranges_with_escapes.as_slice() else { - continue; - }; - let mut diagnostic = Diagnostic::new( - UnnecessaryEscapedQuote, - TextRange::new(context.start_range.start(), tok_range.end()), - ); - let first_edit = Edit::range_replacement( - unescape_string( - locator.slice(first), - context.quote_style.opposite().as_char(), - ), - *first, - ); - let rest_edits = rest.iter().map(|&range| { - Edit::range_replacement( - unescape_string( - locator.slice(range), - context.quote_style.opposite().as_char(), - ), - range, - ) - }); - diagnostic.set_fix(Fix::safe_edits(first_edit, rest_edits)); - diagnostics.push(diagnostic); - } - _ => {} - } - } -} - /// Return `true` if the haystack contains the quote. fn contains_quote(haystack: &str, quote: char) -> bool { memchr::memchr(quote as u8, haystack.as_bytes()).is_some() } - -/// Return `true` if the haystack contains an escaped quote. -fn contains_escaped_quote(haystack: &str, quote: char) -> bool { - for index in memchr::memchr_iter(quote as u8, haystack.as_bytes()) { - // If the quote is preceded by an even number of backslashes, it's not escaped. - if haystack.as_bytes()[..index] - .iter() - .rev() - .take_while(|&&c| c == b'\\') - .count() - % 2 - != 0 - { - return true; - } - } - false -} - -/// Return a modified version of the string with all quote escapes removed. -fn unescape_string(haystack: &str, quote: char) -> String { - let mut fixed_contents = String::with_capacity(haystack.len()); - - let mut chars = haystack.chars().peekable(); - let mut backslashes = 0; - while let Some(char) = chars.next() { - if char != '\\' { - fixed_contents.push(char); - backslashes = 0; - continue; - } - // If we're at the end of the line - let Some(next_char) = chars.peek() else { - fixed_contents.push(char); - continue; - }; - // Remove quote escape - if *next_char == quote && backslashes % 2 == 0 { - backslashes = 0; - continue; - } - backslashes += 1; - fixed_contents.push(char); - } - - fixed_contents -} diff --git a/crates/ruff_linter/src/rules/flake8_quotes/rules/mod.rs b/crates/ruff_linter/src/rules/flake8_quotes/rules/mod.rs index 1f64976bf2..1c617f92a0 100644 --- a/crates/ruff_linter/src/rules/flake8_quotes/rules/mod.rs +++ b/crates/ruff_linter/src/rules/flake8_quotes/rules/mod.rs @@ -1,5 +1,7 @@ pub(crate) use avoidable_escaped_quote::*; pub(crate) use check_string_quotes::*; +pub(crate) use unnecessary_escaped_quote::*; mod avoidable_escaped_quote; mod check_string_quotes; +mod unnecessary_escaped_quote; diff --git a/crates/ruff_linter/src/rules/flake8_quotes/rules/unnecessary_escaped_quote.rs b/crates/ruff_linter/src/rules/flake8_quotes/rules/unnecessary_escaped_quote.rs new file mode 100644 index 0000000000..a485396bc4 --- /dev/null +++ b/crates/ruff_linter/src/rules/flake8_quotes/rules/unnecessary_escaped_quote.rs @@ -0,0 +1,155 @@ +use ruff_diagnostics::{AlwaysFixableViolation, Diagnostic, Edit, Fix}; +use ruff_macros::{derive_message_formats, violation}; +use ruff_python_ast::{self as ast, AnyStringKind, StringLike}; +use ruff_source_file::Locator; +use ruff_text_size::{Ranged, TextLen, TextRange}; + +use crate::checkers::ast::Checker; + +use super::super::helpers::{contains_escaped_quote, unescape_string}; + +/// ## What it does +/// Checks for strings that include unnecessarily escaped quotes. +/// +/// ## Why is this bad? +/// If a string contains an escaped quote that doesn't match the quote +/// character used for the string, it's unnecessary and can be removed. +/// +/// ## Example +/// ```python +/// foo = "bar\'s" +/// ``` +/// +/// Use instead: +/// ```python +/// foo = "bar's" +/// ``` +/// +/// ## Formatter compatibility +/// We recommend against using this rule alongside the [formatter]. The +/// formatter automatically removes unnecessary escapes, making the rule +/// redundant. +/// +/// [formatter]: https://docs.astral.sh/ruff/formatter +#[violation] +pub struct UnnecessaryEscapedQuote; + +impl AlwaysFixableViolation for UnnecessaryEscapedQuote { + #[derive_message_formats] + fn message(&self) -> String { + format!("Unnecessary escape on inner quote character") + } + + fn fix_title(&self) -> String { + "Remove backslash".to_string() + } +} + +/// Q004 +pub(crate) fn unnecessary_escaped_quote(checker: &mut Checker, string_like: StringLike) { + let locator = checker.locator(); + + match string_like { + StringLike::String(expr) => { + for string in &expr.value { + if let Some(diagnostic) = check_string_or_bytes( + locator, + string.range(), + AnyStringKind::from(string.flags), + ) { + checker.diagnostics.push(diagnostic); + } + } + } + StringLike::Bytes(expr) => { + for bytes in &expr.value { + if let Some(diagnostic) = + check_string_or_bytes(locator, bytes.range(), AnyStringKind::from(bytes.flags)) + { + checker.diagnostics.push(diagnostic); + } + } + } + StringLike::FString(expr) => { + for part in &expr.value { + if let Some(diagnostic) = match part { + ast::FStringPart::Literal(string) => check_string_or_bytes( + locator, + string.range(), + AnyStringKind::from(string.flags), + ), + ast::FStringPart::FString(f_string) => check_f_string(locator, f_string), + } { + checker.diagnostics.push(diagnostic); + }; + } + } + } +} + +/// Checks for unnecessary escaped quotes in a string or bytes literal. +/// +/// # Panics +/// +/// If the string kind is an f-string. +fn check_string_or_bytes( + locator: &Locator, + range: TextRange, + kind: AnyStringKind, +) -> Option { + assert!(!kind.is_f_string()); + + if kind.is_triple_quoted() || kind.is_raw_string() { + return None; + } + + let contents = raw_contents(locator.slice(range), kind); + let quote = kind.quote_style(); + let opposite_quote_char = quote.opposite().as_char(); + + if !contains_escaped_quote(contents, opposite_quote_char) { + return None; + } + + let mut diagnostic = Diagnostic::new(UnnecessaryEscapedQuote, range); + diagnostic.set_fix(Fix::safe_edit(Edit::range_replacement( + kind.format_string_contents(&unescape_string(contents, opposite_quote_char)), + range, + ))); + Some(diagnostic) +} + +/// Checks for unnecessary escaped quotes in an f-string. +fn check_f_string(locator: &Locator, f_string: &ast::FString) -> Option { + let ast::FString { flags, range, .. } = f_string; + if flags.is_triple_quoted() || flags.prefix().is_raw() { + return None; + } + + let opposite_quote_char = flags.quote_style().opposite().as_char(); + + let mut edits = vec![]; + for literal in f_string.literals() { + let content = locator.slice(literal); + if !contains_escaped_quote(content, opposite_quote_char) { + continue; + } + edits.push(Edit::range_replacement( + unescape_string(content, opposite_quote_char), + literal.range(), + )); + } + + let mut edits_iter = edits.into_iter(); + let first = edits_iter.next()?; + + let mut diagnostic = Diagnostic::new(UnnecessaryEscapedQuote, *range); + diagnostic.set_fix(Fix::safe_edits(first, edits_iter)); + Some(diagnostic) +} + +/// Returns the raw contents of the string given the string's contents and kind. +/// This is a string without the prefix and quotes. +fn raw_contents(contents: &str, kind: AnyStringKind) -> &str { + &contents[kind.opener_len().to_usize()..(contents.text_len() - kind.closer_len()).to_usize()] +}