From bb80dc6749d88d638fff3a7e9162126fab5657f1 Mon Sep 17 00:00:00 2001 From: Dhruv Manilawala Date: Thu, 21 Sep 2023 14:51:21 +0530 Subject: [PATCH 1/3] Separate `Q003` to accomodate f-string context --- .../fixtures/flake8_quotes/doubles_escaped.py | 24 ++ .../fixtures/flake8_quotes/singles_escaped.py | 23 ++ crates/ruff_linter/src/checkers/tokens.rs | 5 +- .../src/rules/flake8_quotes/mod.rs | 39 +++ .../rules/avoidable_escaped_quote.rs | 258 ++++++++++++++++++ .../rules/flake8_quotes/rules/from_tokens.rs | 200 +++----------- .../src/rules/flake8_quotes/rules/mod.rs | 2 + .../src/rules/flake8_quotes/settings.rs | 18 ++ ...quire_doubles_over_singles_escaped.py.snap | 112 ++++++++ ...re_doubles_over_singles_escaped_py311.snap | 80 ++++++ ...quire_singles_over_doubles_escaped.py.snap | 133 +++++++++ ...re_singles_over_doubles_escaped_py311.snap | 119 ++++++++ crates/ruff_linter/src/settings/types.rs | 7 + crates/ruff_python_parser/src/token.rs | 20 +- 14 files changed, 875 insertions(+), 165 deletions(-) create mode 100644 crates/ruff_linter/src/rules/flake8_quotes/rules/avoidable_escaped_quote.rs create mode 100644 crates/ruff_linter/src/rules/flake8_quotes/snapshots/ruff_linter__rules__flake8_quotes__tests__require_doubles_over_singles_escaped_py311.snap create mode 100644 crates/ruff_linter/src/rules/flake8_quotes/snapshots/ruff_linter__rules__flake8_quotes__tests__require_singles_over_doubles_escaped_py311.snap diff --git a/crates/ruff_linter/resources/test/fixtures/flake8_quotes/doubles_escaped.py b/crates/ruff_linter/resources/test/fixtures/flake8_quotes/doubles_escaped.py index c55ba00cd8fc3..459da81183993 100644 --- a/crates/ruff_linter/resources/test/fixtures/flake8_quotes/doubles_escaped.py +++ b/crates/ruff_linter/resources/test/fixtures/flake8_quotes/doubles_escaped.py @@ -9,3 +9,27 @@ 'This is a' '\'string\'' ) + +# Same as above, but with f-strings +f'This is a \'string\'' # Q003 +f'This is \\ a \\\'string\'' # Q003 +f'"This" is a \'string\'' +f"This is a 'string'" +f"\"This\" is a 'string'" +fr'This is a \'string\'' +fR'This is a \'string\'' +foo = ( + f'This is a' + f'\'string\'' # Q003 +) + +# Nested f-strings (Python 3.12+) +# +# The first one is interesting because the fix for it is valid pre 3.12: +# +# f"'foo' {'nested'}" +# +# but as the actual string itself is invalid pre 3.12, we don't catch it. +f'\'foo\' {'nested'}' # Q003 +f'\'foo\' {f'nested'}' # Q003 +f'\'foo\' {f'\'nested\''} \'\'' # Q003 diff --git a/crates/ruff_linter/resources/test/fixtures/flake8_quotes/singles_escaped.py b/crates/ruff_linter/resources/test/fixtures/flake8_quotes/singles_escaped.py index f011c5f90c5bd..38d5168e2358a 100644 --- a/crates/ruff_linter/resources/test/fixtures/flake8_quotes/singles_escaped.py +++ b/crates/ruff_linter/resources/test/fixtures/flake8_quotes/singles_escaped.py @@ -8,3 +8,26 @@ "This is a" "\"string\"" ) + +# Same as above, but with f-strings +f"This is a \"string\"" +f"'This' is a \"string\"" +f'This is a "string"' +f'\'This\' is a "string"' +fr"This is a \"string\"" +fR"This is a \"string\"" +foo = ( + f"This is a" + f"\"string\"" +) + +# Nested f-strings (Python 3.12+) +# +# The first one is interesting because the fix for it is valid pre 3.12: +# +# f'"foo" {"nested"}' +# +# but as the actual string itself is invalid pre 3.12, we don't catch it. +f"\"foo\" {"foo"}" # Q003 +f"\"foo\" {f"foo"}" # Q003 +f"\"foo\" {f"\"foo\""} \"\"" # Q003 diff --git a/crates/ruff_linter/src/checkers/tokens.rs b/crates/ruff_linter/src/checkers/tokens.rs index 08ff4c4585b49..f5525251b573b 100644 --- a/crates/ruff_linter/src/checkers/tokens.rs +++ b/crates/ruff_linter/src/checkers/tokens.rs @@ -118,11 +118,14 @@ pub(crate) fn check_tokens( ); } + if settings.rules.enabled(Rule::AvoidableEscapedQuote) && settings.flake8_quotes.avoid_escape { + flake8_quotes::rules::avoidable_escaped_quote(&mut diagnostics, tokens, locator, settings); + } + if settings.rules.any_enabled(&[ Rule::BadQuotesInlineString, Rule::BadQuotesMultilineString, Rule::BadQuotesDocstring, - Rule::AvoidableEscapedQuote, ]) { flake8_quotes::rules::from_tokens(&mut diagnostics, tokens, locator, settings); } diff --git a/crates/ruff_linter/src/rules/flake8_quotes/mod.rs b/crates/ruff_linter/src/rules/flake8_quotes/mod.rs index f3c1e86ad1046..1d178d1f1412d 100644 --- a/crates/ruff_linter/src/rules/flake8_quotes/mod.rs +++ b/crates/ruff_linter/src/rules/flake8_quotes/mod.rs @@ -11,6 +11,7 @@ mod tests { use crate::assert_messages; use crate::registry::Rule; + use crate::settings::types::PythonVersion; use crate::settings::LinterSettings; use crate::test::test_path; @@ -45,6 +46,44 @@ mod tests { Ok(()) } + #[test] + fn require_singles_over_doubles_escaped_py311() -> Result<()> { + let diagnostics = test_path( + Path::new("flake8_quotes/doubles_escaped.py"), + &LinterSettings { + flake8_quotes: super::settings::Settings { + inline_quotes: Quote::Single, + multiline_quotes: Quote::Single, + docstring_quotes: Quote::Single, + avoid_escape: true, + }, + ..LinterSettings::for_rule(Rule::AvoidableEscapedQuote) + .with_target_version(PythonVersion::Py311) + }, + )?; + assert_messages!(diagnostics); + Ok(()) + } + + #[test] + fn require_doubles_over_singles_escaped_py311() -> Result<()> { + let diagnostics = test_path( + Path::new("flake8_quotes/singles_escaped.py"), + &LinterSettings { + flake8_quotes: super::settings::Settings { + inline_quotes: Quote::Double, + multiline_quotes: Quote::Double, + docstring_quotes: Quote::Double, + avoid_escape: true, + }, + ..LinterSettings::for_rule(Rule::AvoidableEscapedQuote) + .with_target_version(PythonVersion::Py311) + }, + )?; + assert_messages!(diagnostics); + Ok(()) + } + #[test_case(Path::new("singles.py"))] #[test_case(Path::new("singles_escaped.py"))] #[test_case(Path::new("singles_implicit.py"))] 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 new file mode 100644 index 0000000000000..e6ee2ba4a0691 --- /dev/null +++ b/crates/ruff_linter/src/rules/flake8_quotes/rules/avoidable_escaped_quote.rs @@ -0,0 +1,258 @@ +use ruff_diagnostics::{AlwaysAutofixableViolation, Diagnostic, Edit, Fix}; +use ruff_macros::{derive_message_formats, violation}; +use ruff_python_ast::str::{is_triple_quote, leading_quote}; +use ruff_python_parser::lexer::LexResult; +use ruff_python_parser::Tok; +use ruff_source_file::Locator; +use ruff_text_size::TextRange; + +use crate::lex::docstring_detection::StateMachine; +use crate::registry::AsRule; +use crate::settings::LinterSettings; + +/// ## What it does +/// Checks for strings that include escaped quotes, and suggests changing +/// the quote style to avoid the need to escape them. +/// +/// ## Why is this bad? +/// It's preferable to avoid escaped quotes in strings. By changing the +/// outer quote style, you can avoid escaping inner quotes. +/// +/// ## Example +/// ```python +/// foo = 'bar\'s' +/// ``` +/// +/// Use instead: +/// ```python +/// foo = "bar's" +/// ``` +#[violation] +pub struct AvoidableEscapedQuote; + +impl AlwaysAutofixableViolation for AvoidableEscapedQuote { + #[derive_message_formats] + fn message(&self) -> String { + format!("Change outer quotes to avoid escaping inner quotes") + } + + fn autofix_title(&self) -> String { + "Change outer quotes to avoid escaping inner quotes".to_string() + } +} + +struct FStringContext { + /// Whether to check for escaped quotes in the f-string. + check_for_escaped_quote: bool, + /// The range of the f-string start token. + fstring_start_range: TextRange, + /// The ranges of the f-string middle tokens containing escaped quotes. + fstring_middle_ranges: Vec, +} + +impl FStringContext { + fn new(check_for_escaped_quote: bool, fstring_start_range: TextRange) -> Self { + Self { + check_for_escaped_quote, + fstring_start_range, + fstring_middle_ranges: vec![], + } + } + + /// Update the context to not check for escaped quotes, and clear any + /// existing reported ranges. + fn do_not_check_for_escaped_quote(&mut self) { + self.check_for_escaped_quote = false; + self.fstring_middle_ranges.clear(); + } + + fn push_fstring_middle_range(&mut self, range: TextRange) { + self.fstring_middle_ranges.push(range); + } +} + +/// Q003 +pub(crate) fn avoidable_escaped_quote( + diagnostics: &mut Vec, + lxr: &[LexResult], + locator: &Locator, + settings: &LinterSettings, +) { + let quotes_settings = &settings.flake8_quotes; + let supports_pep701 = settings.target_version.supports_pep701(); + 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; + } + + if !supports_pep701 { + // If this is a string or a start of a f-string which is inside another + // f-string, we won't check for escaped quotes for the entire f-string + // if the target version doesn't support PEP 701. For example: + // + // ```python + // f"\"foo\" {'nested'}" + // # ^^^^^^^^ + // # We're here + // ``` + // + // If we try to fix the above example, the outer and inner quote + // will be the same which is invalid pre 3.12: + // + // ```python + // f'"foo" {'nested'}" + // ``` + if matches!(tok, Tok::String { .. } | Tok::FStringStart) { + if let Some(fstring_context) = fstrings.last_mut() { + fstring_context.do_not_check_for_escaped_quote(); + continue; + } + } + } + + match tok { + Tok::String { + value: string_contents, + kind, + triple_quoted, + } => { + // Check if we're using the preferred quotation style. + let uses_preferred_quote = leading_quote(locator.slice(tok_range)) + .is_some_and(|text| text.contains(quotes_settings.inline_quotes.as_char())); + if kind.is_raw() || *triple_quoted || !uses_preferred_quote { + continue; + } + + if string_contents.contains(quotes_settings.inline_quotes.as_char()) + && !string_contents.contains(quotes_settings.inline_quotes.opposite().as_char()) + { + let mut diagnostic = Diagnostic::new(AvoidableEscapedQuote, tok_range); + if settings.rules.should_fix(diagnostic.kind.rule()) { + let fixed_contents = format!( + "{prefix}{quote}{value}{quote}", + prefix = kind.as_str(), + quote = quotes_settings.inline_quotes.opposite().as_char(), + value = unescaped_string(string_contents) + ); + diagnostic.set_fix(Fix::automatic(Edit::range_replacement( + fixed_contents, + tok_range, + ))); + } + diagnostics.push(diagnostic); + } + } + Tok::FStringStart => { + let text = locator.slice(tok_range); + // 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 = text + .contains(quotes_settings.inline_quotes.as_char()) + && !is_triple_quote(text); + fstrings.push(FStringContext::new(check_for_escaped_quote, tok_range)); + } + Tok::FStringMiddle { + value: string_contents, + is_raw, + } if !is_raw => { + let Some(context) = fstrings.last_mut() else { + continue; + }; + if !context.check_for_escaped_quote { + continue; + } + if string_contents.contains(quotes_settings.inline_quotes.as_char()) + && !string_contents.contains(quotes_settings.inline_quotes.opposite().as_char()) + { + context.push_fstring_middle_range(tok_range); + } + } + Tok::FStringEnd => { + let Some(context) = fstrings.pop() else { + continue; + }; + if context.fstring_middle_ranges.is_empty() { + // There are no `FStringMiddle` tokens containing any escaped + // quotes. + continue; + } + let mut diagnostic = Diagnostic::new( + AvoidableEscapedQuote, + TextRange::new(context.fstring_start_range.start(), tok_range.end()), + ); + if settings.rules.should_fix(diagnostic.kind.rule()) { + let fstring_start_edit = Edit::range_replacement( + // No need for `r`/`R` as we don't perform the checks + // for raw strings. + format!("f{}", quotes_settings.inline_quotes.opposite().as_char()), + context.fstring_start_range, + ); + let fstring_middle_and_end_edits = context + .fstring_middle_ranges + .iter() + .map(|&range| { + Edit::range_replacement(unescaped_string(locator.slice(range)), range) + }) + .chain(std::iter::once( + // `FStringEnd` edit + Edit::range_replacement( + quotes_settings + .inline_quotes + .opposite() + .as_char() + .to_string(), + tok_range, + ), + )); + diagnostic.set_fix(Fix::automatic_edits( + fstring_start_edit, + fstring_middle_and_end_edits, + )); + } + diagnostics.push(diagnostic); + } + _ => {} + } + } +} + +fn unescaped_string(value: &str) -> String { + let mut fixed_contents = String::with_capacity(value.len()); + + let chars: Vec = value.chars().collect(); + let mut backslash_count = 0; + for col_offset in 0..chars.len() { + let char = chars[col_offset]; + if char != '\\' { + fixed_contents.push(char); + continue; + } + backslash_count += 1; + // If the previous character was also a backslash + if col_offset > 0 && chars[col_offset - 1] == '\\' && backslash_count == 2 { + fixed_contents.push(char); + // reset to 0 + backslash_count = 0; + continue; + } + // If we're at the end of the line + if col_offset == chars.len() - 1 { + fixed_contents.push(char); + continue; + } + let next_char = chars[col_offset + 1]; + // Remove quote escape + if next_char == '\'' || next_char == '"' { + // reset to 0 + backslash_count = 0; + continue; + } + fixed_contents.push(char); + } + + fixed_contents +} diff --git a/crates/ruff_linter/src/rules/flake8_quotes/rules/from_tokens.rs b/crates/ruff_linter/src/rules/flake8_quotes/rules/from_tokens.rs index 7cb159008147a..5bcc9e52e1e55 100644 --- a/crates/ruff_linter/src/rules/flake8_quotes/rules/from_tokens.rs +++ b/crates/ruff_linter/src/rules/flake8_quotes/rules/from_tokens.rs @@ -34,22 +34,22 @@ use super::super::settings::Quote; /// - `flake8-quotes.inline-quotes` #[violation] pub struct BadQuotesInlineString { - quote: Quote, + preferred_quote: Quote, } impl AlwaysAutofixableViolation for BadQuotesInlineString { #[derive_message_formats] fn message(&self) -> String { - let BadQuotesInlineString { quote } = self; - match quote { + let BadQuotesInlineString { preferred_quote } = self; + match preferred_quote { Quote::Double => format!("Single quotes found but double quotes preferred"), Quote::Single => format!("Double quotes found but single quotes preferred"), } } fn autofix_title(&self) -> String { - let BadQuotesInlineString { quote } = self; - match quote { + let BadQuotesInlineString { preferred_quote } = self; + match preferred_quote { Quote::Double => "Replace single quotes with double quotes".to_string(), Quote::Single => "Replace double quotes with single quotes".to_string(), } @@ -83,22 +83,22 @@ impl AlwaysAutofixableViolation for BadQuotesInlineString { /// - `flake8-quotes.multiline-quotes` #[violation] pub struct BadQuotesMultilineString { - quote: Quote, + preferred_quote: Quote, } impl AlwaysAutofixableViolation for BadQuotesMultilineString { #[derive_message_formats] fn message(&self) -> String { - let BadQuotesMultilineString { quote } = self; - match quote { + let BadQuotesMultilineString { preferred_quote } = self; + match preferred_quote { Quote::Double => format!("Single quote multiline found but double quotes preferred"), Quote::Single => format!("Double quote multiline found but single quotes preferred"), } } fn autofix_title(&self) -> String { - let BadQuotesMultilineString { quote } = self; - match quote { + let BadQuotesMultilineString { preferred_quote } = self; + match preferred_quote { Quote::Double => "Replace single multiline quotes with double quotes".to_string(), Quote::Single => "Replace double multiline quotes with single quotes".to_string(), } @@ -131,73 +131,28 @@ impl AlwaysAutofixableViolation for BadQuotesMultilineString { /// - `flake8-quotes.docstring-quotes` #[violation] pub struct BadQuotesDocstring { - quote: Quote, + preferred_quote: Quote, } impl AlwaysAutofixableViolation for BadQuotesDocstring { #[derive_message_formats] fn message(&self) -> String { - let BadQuotesDocstring { quote } = self; - match quote { + let BadQuotesDocstring { preferred_quote } = self; + match preferred_quote { Quote::Double => format!("Single quote docstring found but double quotes preferred"), Quote::Single => format!("Double quote docstring found but single quotes preferred"), } } fn autofix_title(&self) -> String { - let BadQuotesDocstring { quote } = self; - match quote { + let BadQuotesDocstring { preferred_quote } = self; + match preferred_quote { Quote::Double => "Replace single quotes docstring with double quotes".to_string(), Quote::Single => "Replace double quotes docstring with single quotes".to_string(), } } } -/// ## What it does -/// Checks for strings that include escaped quotes, and suggests changing -/// the quote style to avoid the need to escape them. -/// -/// ## Why is this bad? -/// It's preferable to avoid escaped quotes in strings. By changing the -/// outer quote style, you can avoid escaping inner quotes. -/// -/// ## Example -/// ```python -/// foo = 'bar\'s' -/// ``` -/// -/// Use instead: -/// ```python -/// foo = "bar's" -/// ``` -#[violation] -pub struct AvoidableEscapedQuote; - -impl AlwaysAutofixableViolation for AvoidableEscapedQuote { - #[derive_message_formats] - fn message(&self) -> String { - format!("Change outer quotes to avoid escaping inner quotes") - } - - fn autofix_title(&self) -> String { - "Change outer quotes to avoid escaping inner quotes".to_string() - } -} - -const fn good_single(quote: Quote) -> char { - match quote { - Quote::Double => '"', - Quote::Single => '\'', - } -} - -const fn bad_single(quote: Quote) -> char { - match quote { - Quote::Double => '\'', - Quote::Single => '"', - } -} - const fn good_multiline(quote: Quote) -> &'static str { match quote { Quote::Double => "\"\"\"", @@ -219,6 +174,7 @@ const fn good_docstring(quote: Quote) -> &'static str { } } +#[derive(Debug)] struct Trivia<'a> { last_quote_char: char, prefix: &'a str, @@ -254,7 +210,7 @@ impl<'a> From<&'a str> for Trivia<'a> { } } -/// Q003 +/// Q002 fn docstring(locator: &Locator, range: TextRange, settings: &LinterSettings) -> Option { let quotes_settings = &settings.flake8_quotes; @@ -270,7 +226,7 @@ fn docstring(locator: &Locator, range: TextRange, settings: &LinterSettings) -> let mut diagnostic = Diagnostic::new( BadQuotesDocstring { - quote: quotes_settings.docstring_quotes, + preferred_quote: quotes_settings.docstring_quotes, }, range, ); @@ -292,7 +248,7 @@ fn docstring(locator: &Locator, range: TextRange, settings: &LinterSettings) -> Some(diagnostic) } -/// Q001, Q002 +/// Q000, Q001 fn strings( locator: &Locator, sequence: &[TextRange], @@ -318,12 +274,12 @@ fn strings( return false; } - if trivia.last_quote_char == good_single(quotes_settings.inline_quotes) { + if trivia.last_quote_char == quotes_settings.inline_quotes.as_char() { return false; } let string_contents = &trivia.raw_text[1..trivia.raw_text.len() - 1]; - string_contents.contains(good_single(quotes_settings.inline_quotes)) + string_contents.contains(quotes_settings.inline_quotes.as_char()) }); for (range, trivia) in sequence.iter().zip(trivia) { @@ -346,7 +302,7 @@ fn strings( let mut diagnostic = Diagnostic::new( BadQuotesMultilineString { - quote: quotes_settings.multiline_quotes, + preferred_quote: quotes_settings.multiline_quotes, }, *range, ); @@ -367,99 +323,31 @@ fn strings( ))); } diagnostics.push(diagnostic); - } else { - let string_contents = &trivia.raw_text[1..trivia.raw_text.len() - 1]; - - // If we're using the preferred quotation type, check for escapes. - if trivia.last_quote_char == good_single(quotes_settings.inline_quotes) { - if !quotes_settings.avoid_escape - || trivia.prefix.contains('r') - || trivia.prefix.contains('R') - { - continue; - } - - if string_contents.contains(good_single(quotes_settings.inline_quotes)) - && !string_contents.contains(bad_single(quotes_settings.inline_quotes)) - { - let mut diagnostic = Diagnostic::new(AvoidableEscapedQuote, *range); - if settings.rules.should_fix(Rule::AvoidableEscapedQuote) { - let quote = bad_single(quotes_settings.inline_quotes); - - let mut fixed_contents = - String::with_capacity(trivia.prefix.len() + string_contents.len() + 2); - fixed_contents.push_str(trivia.prefix); - fixed_contents.push(quote); - - let chars: Vec = string_contents.chars().collect(); - let mut backslash_count = 0; - for col_offset in 0..chars.len() { - let char = chars[col_offset]; - if char != '\\' { - fixed_contents.push(char); - continue; - } - backslash_count += 1; - // If the previous character was also a backslash - if col_offset > 0 - && chars[col_offset - 1] == '\\' - && backslash_count == 2 - { - fixed_contents.push(char); - // reset to 0 - backslash_count = 0; - continue; - } - // If we're at the end of the line - if col_offset == chars.len() - 1 { - fixed_contents.push(char); - continue; - } - let next_char = chars[col_offset + 1]; - // Remove quote escape - if next_char == '\'' || next_char == '"' { - // reset to 0 - backslash_count = 0; - continue; - } - fixed_contents.push(char); - } - - fixed_contents.push(quote); - - diagnostic.set_fix(Fix::automatic(Edit::range_replacement( - fixed_contents, - *range, - ))); - } - diagnostics.push(diagnostic); - } - continue; - } - + } else if trivia.last_quote_char != quotes_settings.inline_quotes.as_char() // If we're not using the preferred type, only allow use to avoid escapes. - if !relax_quote { - let mut diagnostic = Diagnostic::new( - BadQuotesInlineString { - quote: quotes_settings.inline_quotes, - }, + && !relax_quote + { + let mut diagnostic = Diagnostic::new( + BadQuotesInlineString { + preferred_quote: quotes_settings.inline_quotes, + }, + *range, + ); + if settings.rules.should_fix(Rule::BadQuotesInlineString) { + let quote = quotes_settings.inline_quotes.as_char(); + let string_contents = &trivia.raw_text[1..trivia.raw_text.len() - 1]; + let mut fixed_contents = + String::with_capacity(trivia.prefix.len() + string_contents.len() + 2); + fixed_contents.push_str(trivia.prefix); + fixed_contents.push(quote); + fixed_contents.push_str(string_contents); + fixed_contents.push(quote); + diagnostic.set_fix(Fix::automatic(Edit::range_replacement( + fixed_contents, *range, - ); - if settings.rules.should_fix(Rule::BadQuotesInlineString) { - let quote = good_single(quotes_settings.inline_quotes); - let mut fixed_contents = - String::with_capacity(trivia.prefix.len() + string_contents.len() + 2); - fixed_contents.push_str(trivia.prefix); - fixed_contents.push(quote); - fixed_contents.push_str(string_contents); - fixed_contents.push(quote); - diagnostic.set_fix(Fix::automatic(Edit::range_replacement( - fixed_contents, - *range, - ))); - } - diagnostics.push(diagnostic); + ))); } + diagnostics.push(diagnostic); } } 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 8ad6bad659da4..b57ddd014ba07 100644 --- a/crates/ruff_linter/src/rules/flake8_quotes/rules/mod.rs +++ b/crates/ruff_linter/src/rules/flake8_quotes/rules/mod.rs @@ -1,3 +1,5 @@ +pub(crate) use avoidable_escaped_quote::*; pub(crate) use from_tokens::*; +mod avoidable_escaped_quote; mod from_tokens; diff --git a/crates/ruff_linter/src/rules/flake8_quotes/settings.rs b/crates/ruff_linter/src/rules/flake8_quotes/settings.rs index 4a69c1da46064..620fb2e53a8b8 100644 --- a/crates/ruff_linter/src/rules/flake8_quotes/settings.rs +++ b/crates/ruff_linter/src/rules/flake8_quotes/settings.rs @@ -38,3 +38,21 @@ impl Default for Settings { } } } + +impl Quote { + #[must_use] + pub const fn opposite(self) -> Self { + match self { + Self::Double => Self::Single, + Self::Single => Self::Double, + } + } + + /// Get the character used to represent this quote. + pub const fn as_char(self) -> char { + match self { + Self::Double => '"', + Self::Single => '\'', + } + } +} diff --git a/crates/ruff_linter/src/rules/flake8_quotes/snapshots/ruff_linter__rules__flake8_quotes__tests__require_doubles_over_singles_escaped.py.snap b/crates/ruff_linter/src/rules/flake8_quotes/snapshots/ruff_linter__rules__flake8_quotes__tests__require_doubles_over_singles_escaped.py.snap index a05a28bb5a43b..96e17f47fd164 100644 --- a/crates/ruff_linter/src/rules/flake8_quotes/snapshots/ruff_linter__rules__flake8_quotes__tests__require_doubles_over_singles_escaped.py.snap +++ b/crates/ruff_linter/src/rules/flake8_quotes/snapshots/ruff_linter__rules__flake8_quotes__tests__require_doubles_over_singles_escaped.py.snap @@ -34,5 +34,117 @@ singles_escaped.py:9:5: Q003 [*] Change outer quotes to avoid escaping inner quo 9 |- "\"string\"" 9 |+ '"string"' 10 10 | ) +11 11 | +12 12 | # Same as above, but with f-strings + +singles_escaped.py:13:1: Q003 [*] Change outer quotes to avoid escaping inner quotes + | +12 | # Same as above, but with f-strings +13 | f"This is a \"string\"" + | ^^^^^^^^^^^^^^^^^^^^^^^ Q003 +14 | f"'This' is a \"string\"" +15 | f'This is a "string"' + | + = help: Change outer quotes to avoid escaping inner quotes + +ℹ Fix +10 10 | ) +11 11 | +12 12 | # Same as above, but with f-strings +13 |-f"This is a \"string\"" + 13 |+f'This is a "string"' +14 14 | f"'This' is a \"string\"" +15 15 | f'This is a "string"' +16 16 | f'\'This\' is a "string"' + +singles_escaped.py:21:5: Q003 [*] Change outer quotes to avoid escaping inner quotes + | +19 | foo = ( +20 | f"This is a" +21 | f"\"string\"" + | ^^^^^^^^^^^^^ Q003 +22 | ) + | + = help: Change outer quotes to avoid escaping inner quotes + +ℹ Fix +18 18 | fR"This is a \"string\"" +19 19 | foo = ( +20 20 | f"This is a" +21 |- f"\"string\"" + 21 |+ f'"string"' +22 22 | ) +23 23 | +24 24 | # Nested f-strings (Python 3.12+) + +singles_escaped.py:31:1: Q003 [*] Change outer quotes to avoid escaping inner quotes + | +29 | # +30 | # but as the actual string itself is invalid pre 3.12, we don't catch it. +31 | f"\"foo\" {"foo"}" # Q003 + | ^^^^^^^^^^^^^^^^^^ Q003 +32 | f"\"foo\" {f"foo"}" # Q003 +33 | f"\"foo\" {f"\"foo\""} \"\"" # Q003 + | + = help: Change outer quotes to avoid escaping inner quotes + +ℹ Fix +28 28 | # f'"foo" {"nested"}' +29 29 | # +30 30 | # but as the actual string itself is invalid pre 3.12, we don't catch it. +31 |-f"\"foo\" {"foo"}" # Q003 + 31 |+f'"foo" {"foo"}' # Q003 +32 32 | f"\"foo\" {f"foo"}" # Q003 +33 33 | f"\"foo\" {f"\"foo\""} \"\"" # Q003 + +singles_escaped.py:32:1: Q003 [*] Change outer quotes to avoid escaping inner quotes + | +30 | # but as the actual string itself is invalid pre 3.12, we don't catch it. +31 | f"\"foo\" {"foo"}" # Q003 +32 | f"\"foo\" {f"foo"}" # Q003 + | ^^^^^^^^^^^^^^^^^^^ Q003 +33 | f"\"foo\" {f"\"foo\""} \"\"" # Q003 + | + = help: Change outer quotes to avoid escaping inner quotes + +ℹ Fix +29 29 | # +30 30 | # but as the actual string itself is invalid pre 3.12, we don't catch it. +31 31 | f"\"foo\" {"foo"}" # Q003 +32 |-f"\"foo\" {f"foo"}" # Q003 + 32 |+f'"foo" {f"foo"}' # Q003 +33 33 | f"\"foo\" {f"\"foo\""} \"\"" # Q003 + +singles_escaped.py:33:1: Q003 [*] Change outer quotes to avoid escaping inner quotes + | +31 | f"\"foo\" {"foo"}" # Q003 +32 | f"\"foo\" {f"foo"}" # Q003 +33 | f"\"foo\" {f"\"foo\""} \"\"" # Q003 + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Q003 + | + = help: Change outer quotes to avoid escaping inner quotes + +ℹ Fix +30 30 | # but as the actual string itself is invalid pre 3.12, we don't catch it. +31 31 | f"\"foo\" {"foo"}" # Q003 +32 32 | f"\"foo\" {f"foo"}" # Q003 +33 |-f"\"foo\" {f"\"foo\""} \"\"" # Q003 + 33 |+f'"foo" {f"\"foo\""} ""' # Q003 + +singles_escaped.py:33:12: Q003 [*] Change outer quotes to avoid escaping inner quotes + | +31 | f"\"foo\" {"foo"}" # Q003 +32 | f"\"foo\" {f"foo"}" # Q003 +33 | f"\"foo\" {f"\"foo\""} \"\"" # Q003 + | ^^^^^^^^^^ Q003 + | + = help: Change outer quotes to avoid escaping inner quotes + +ℹ Fix +30 30 | # but as the actual string itself is invalid pre 3.12, we don't catch it. +31 31 | f"\"foo\" {"foo"}" # Q003 +32 32 | f"\"foo\" {f"foo"}" # Q003 +33 |-f"\"foo\" {f"\"foo\""} \"\"" # Q003 + 33 |+f"\"foo\" {f'"foo"'} \"\"" # Q003 diff --git a/crates/ruff_linter/src/rules/flake8_quotes/snapshots/ruff_linter__rules__flake8_quotes__tests__require_doubles_over_singles_escaped_py311.snap b/crates/ruff_linter/src/rules/flake8_quotes/snapshots/ruff_linter__rules__flake8_quotes__tests__require_doubles_over_singles_escaped_py311.snap new file mode 100644 index 0000000000000..0bc6a10ac635e --- /dev/null +++ b/crates/ruff_linter/src/rules/flake8_quotes/snapshots/ruff_linter__rules__flake8_quotes__tests__require_doubles_over_singles_escaped_py311.snap @@ -0,0 +1,80 @@ +--- +source: crates/ruff_linter/src/rules/flake8_quotes/mod.rs +--- +singles_escaped.py:1:26: Q003 [*] Change outer quotes to avoid escaping inner quotes + | +1 | this_should_raise_Q003 = "This is a \"string\"" + | ^^^^^^^^^^^^^^^^^^^^^^ Q003 +2 | this_is_fine = "'This' is a \"string\"" +3 | this_is_fine = 'This is a "string"' + | + = help: Change outer quotes to avoid escaping inner quotes + +ℹ Fix +1 |-this_should_raise_Q003 = "This is a \"string\"" + 1 |+this_should_raise_Q003 = 'This is a "string"' +2 2 | this_is_fine = "'This' is a \"string\"" +3 3 | this_is_fine = 'This is a "string"' +4 4 | this_is_fine = '\'This\' is a "string"' + +singles_escaped.py:9:5: Q003 [*] Change outer quotes to avoid escaping inner quotes + | + 7 | this_should_raise = ( + 8 | "This is a" + 9 | "\"string\"" + | ^^^^^^^^^^^^ Q003 +10 | ) + | + = help: Change outer quotes to avoid escaping inner quotes + +ℹ Fix +6 6 | this_is_fine = R"This is a \"string\"" +7 7 | this_should_raise = ( +8 8 | "This is a" +9 |- "\"string\"" + 9 |+ '"string"' +10 10 | ) +11 11 | +12 12 | # Same as above, but with f-strings + +singles_escaped.py:13:1: Q003 [*] Change outer quotes to avoid escaping inner quotes + | +12 | # Same as above, but with f-strings +13 | f"This is a \"string\"" + | ^^^^^^^^^^^^^^^^^^^^^^^ Q003 +14 | f"'This' is a \"string\"" +15 | f'This is a "string"' + | + = help: Change outer quotes to avoid escaping inner quotes + +ℹ Fix +10 10 | ) +11 11 | +12 12 | # Same as above, but with f-strings +13 |-f"This is a \"string\"" + 13 |+f'This is a "string"' +14 14 | f"'This' is a \"string\"" +15 15 | f'This is a "string"' +16 16 | f'\'This\' is a "string"' + +singles_escaped.py:21:5: Q003 [*] Change outer quotes to avoid escaping inner quotes + | +19 | foo = ( +20 | f"This is a" +21 | f"\"string\"" + | ^^^^^^^^^^^^^ Q003 +22 | ) + | + = help: Change outer quotes to avoid escaping inner quotes + +ℹ Fix +18 18 | fR"This is a \"string\"" +19 19 | foo = ( +20 20 | f"This is a" +21 |- f"\"string\"" + 21 |+ f'"string"' +22 22 | ) +23 23 | +24 24 | # Nested f-strings (Python 3.12+) + + diff --git a/crates/ruff_linter/src/rules/flake8_quotes/snapshots/ruff_linter__rules__flake8_quotes__tests__require_singles_over_doubles_escaped.py.snap b/crates/ruff_linter/src/rules/flake8_quotes/snapshots/ruff_linter__rules__flake8_quotes__tests__require_singles_over_doubles_escaped.py.snap index ccc63c71d3149..37f611e9a7df3 100644 --- a/crates/ruff_linter/src/rules/flake8_quotes/snapshots/ruff_linter__rules__flake8_quotes__tests__require_singles_over_doubles_escaped.py.snap +++ b/crates/ruff_linter/src/rules/flake8_quotes/snapshots/ruff_linter__rules__flake8_quotes__tests__require_singles_over_doubles_escaped.py.snap @@ -52,5 +52,138 @@ doubles_escaped.py:10:5: Q003 [*] Change outer quotes to avoid escaping inner qu 10 |- '\'string\'' 10 |+ "'string'" 11 11 | ) +12 12 | +13 13 | # Same as above, but with f-strings + +doubles_escaped.py:14:1: Q003 [*] Change outer quotes to avoid escaping inner quotes + | +13 | # Same as above, but with f-strings +14 | f'This is a \'string\'' # Q003 + | ^^^^^^^^^^^^^^^^^^^^^^^ Q003 +15 | f'This is \\ a \\\'string\'' # Q003 +16 | f'"This" is a \'string\'' + | + = help: Change outer quotes to avoid escaping inner quotes + +ℹ Fix +11 11 | ) +12 12 | +13 13 | # Same as above, but with f-strings +14 |-f'This is a \'string\'' # Q003 + 14 |+f"This is a 'string'" # Q003 +15 15 | f'This is \\ a \\\'string\'' # Q003 +16 16 | f'"This" is a \'string\'' +17 17 | f"This is a 'string'" + +doubles_escaped.py:15:1: Q003 [*] Change outer quotes to avoid escaping inner quotes + | +13 | # Same as above, but with f-strings +14 | f'This is a \'string\'' # Q003 +15 | f'This is \\ a \\\'string\'' # Q003 + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Q003 +16 | f'"This" is a \'string\'' +17 | f"This is a 'string'" + | + = help: Change outer quotes to avoid escaping inner quotes + +ℹ Fix +12 12 | +13 13 | # Same as above, but with f-strings +14 14 | f'This is a \'string\'' # Q003 +15 |-f'This is \\ a \\\'string\'' # Q003 + 15 |+f"This is \\ a \\'string'" # Q003 +16 16 | f'"This" is a \'string\'' +17 17 | f"This is a 'string'" +18 18 | f"\"This\" is a 'string'" + +doubles_escaped.py:23:5: Q003 [*] Change outer quotes to avoid escaping inner quotes + | +21 | foo = ( +22 | f'This is a' +23 | f'\'string\'' # Q003 + | ^^^^^^^^^^^^^ Q003 +24 | ) + | + = help: Change outer quotes to avoid escaping inner quotes + +ℹ Fix +20 20 | fR'This is a \'string\'' +21 21 | foo = ( +22 22 | f'This is a' +23 |- f'\'string\'' # Q003 + 23 |+ f"'string'" # Q003 +24 24 | ) +25 25 | +26 26 | # Nested f-strings (Python 3.12+) + +doubles_escaped.py:33:1: Q003 [*] Change outer quotes to avoid escaping inner quotes + | +31 | # +32 | # but as the actual string itself is invalid pre 3.12, we don't catch it. +33 | f'\'foo\' {'nested'}' # Q003 + | ^^^^^^^^^^^^^^^^^^^^^ Q003 +34 | f'\'foo\' {f'nested'}' # Q003 +35 | f'\'foo\' {f'\'nested\''} \'\'' # Q003 + | + = help: Change outer quotes to avoid escaping inner quotes + +ℹ Fix +30 30 | # f"'foo' {'nested'}" +31 31 | # +32 32 | # but as the actual string itself is invalid pre 3.12, we don't catch it. +33 |-f'\'foo\' {'nested'}' # Q003 + 33 |+f"'foo' {'nested'}" # Q003 +34 34 | f'\'foo\' {f'nested'}' # Q003 +35 35 | f'\'foo\' {f'\'nested\''} \'\'' # Q003 + +doubles_escaped.py:34:1: Q003 [*] Change outer quotes to avoid escaping inner quotes + | +32 | # but as the actual string itself is invalid pre 3.12, we don't catch it. +33 | f'\'foo\' {'nested'}' # Q003 +34 | f'\'foo\' {f'nested'}' # Q003 + | ^^^^^^^^^^^^^^^^^^^^^^ Q003 +35 | f'\'foo\' {f'\'nested\''} \'\'' # Q003 + | + = help: Change outer quotes to avoid escaping inner quotes + +ℹ Fix +31 31 | # +32 32 | # but as the actual string itself is invalid pre 3.12, we don't catch it. +33 33 | f'\'foo\' {'nested'}' # Q003 +34 |-f'\'foo\' {f'nested'}' # Q003 + 34 |+f"'foo' {f'nested'}" # Q003 +35 35 | f'\'foo\' {f'\'nested\''} \'\'' # Q003 + +doubles_escaped.py:35:1: Q003 [*] Change outer quotes to avoid escaping inner quotes + | +33 | f'\'foo\' {'nested'}' # Q003 +34 | f'\'foo\' {f'nested'}' # Q003 +35 | f'\'foo\' {f'\'nested\''} \'\'' # Q003 + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Q003 + | + = help: Change outer quotes to avoid escaping inner quotes + +ℹ Fix +32 32 | # but as the actual string itself is invalid pre 3.12, we don't catch it. +33 33 | f'\'foo\' {'nested'}' # Q003 +34 34 | f'\'foo\' {f'nested'}' # Q003 +35 |-f'\'foo\' {f'\'nested\''} \'\'' # Q003 + 35 |+f"'foo' {f'\'nested\''} ''" # Q003 + +doubles_escaped.py:35:12: Q003 [*] Change outer quotes to avoid escaping inner quotes + | +33 | f'\'foo\' {'nested'}' # Q003 +34 | f'\'foo\' {f'nested'}' # Q003 +35 | f'\'foo\' {f'\'nested\''} \'\'' # Q003 + | ^^^^^^^^^^^^^ Q003 + | + = help: Change outer quotes to avoid escaping inner quotes + +ℹ Fix +32 32 | # but as the actual string itself is invalid pre 3.12, we don't catch it. +33 33 | f'\'foo\' {'nested'}' # Q003 +34 34 | f'\'foo\' {f'nested'}' # Q003 +35 |-f'\'foo\' {f'\'nested\''} \'\'' # Q003 + 35 |+f'\'foo\' {f"'nested'"} \'\'' # Q003 diff --git a/crates/ruff_linter/src/rules/flake8_quotes/snapshots/ruff_linter__rules__flake8_quotes__tests__require_singles_over_doubles_escaped_py311.snap b/crates/ruff_linter/src/rules/flake8_quotes/snapshots/ruff_linter__rules__flake8_quotes__tests__require_singles_over_doubles_escaped_py311.snap new file mode 100644 index 0000000000000..32f3d219be247 --- /dev/null +++ b/crates/ruff_linter/src/rules/flake8_quotes/snapshots/ruff_linter__rules__flake8_quotes__tests__require_singles_over_doubles_escaped_py311.snap @@ -0,0 +1,119 @@ +--- +source: crates/ruff_linter/src/rules/flake8_quotes/mod.rs +--- +doubles_escaped.py:1:26: Q003 [*] Change outer quotes to avoid escaping inner quotes + | +1 | this_should_raise_Q003 = 'This is a \'string\'' + | ^^^^^^^^^^^^^^^^^^^^^^ Q003 +2 | this_should_raise_Q003 = 'This is \\ a \\\'string\'' +3 | this_is_fine = '"This" is a \'string\'' + | + = help: Change outer quotes to avoid escaping inner quotes + +ℹ Fix +1 |-this_should_raise_Q003 = 'This is a \'string\'' + 1 |+this_should_raise_Q003 = "This is a 'string'" +2 2 | this_should_raise_Q003 = 'This is \\ a \\\'string\'' +3 3 | this_is_fine = '"This" is a \'string\'' +4 4 | this_is_fine = "This is a 'string'" + +doubles_escaped.py:2:26: Q003 [*] Change outer quotes to avoid escaping inner quotes + | +1 | this_should_raise_Q003 = 'This is a \'string\'' +2 | this_should_raise_Q003 = 'This is \\ a \\\'string\'' + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^ Q003 +3 | this_is_fine = '"This" is a \'string\'' +4 | this_is_fine = "This is a 'string'" + | + = help: Change outer quotes to avoid escaping inner quotes + +ℹ Fix +1 1 | this_should_raise_Q003 = 'This is a \'string\'' +2 |-this_should_raise_Q003 = 'This is \\ a \\\'string\'' + 2 |+this_should_raise_Q003 = "This is \\ a \\'string'" +3 3 | this_is_fine = '"This" is a \'string\'' +4 4 | this_is_fine = "This is a 'string'" +5 5 | this_is_fine = "\"This\" is a 'string'" + +doubles_escaped.py:10:5: Q003 [*] Change outer quotes to avoid escaping inner quotes + | + 8 | this_should_raise = ( + 9 | 'This is a' +10 | '\'string\'' + | ^^^^^^^^^^^^ Q003 +11 | ) + | + = help: Change outer quotes to avoid escaping inner quotes + +ℹ Fix +7 7 | this_is_fine = R'This is a \'string\'' +8 8 | this_should_raise = ( +9 9 | 'This is a' +10 |- '\'string\'' + 10 |+ "'string'" +11 11 | ) +12 12 | +13 13 | # Same as above, but with f-strings + +doubles_escaped.py:14:1: Q003 [*] Change outer quotes to avoid escaping inner quotes + | +13 | # Same as above, but with f-strings +14 | f'This is a \'string\'' # Q003 + | ^^^^^^^^^^^^^^^^^^^^^^^ Q003 +15 | f'This is \\ a \\\'string\'' # Q003 +16 | f'"This" is a \'string\'' + | + = help: Change outer quotes to avoid escaping inner quotes + +ℹ Fix +11 11 | ) +12 12 | +13 13 | # Same as above, but with f-strings +14 |-f'This is a \'string\'' # Q003 + 14 |+f"This is a 'string'" # Q003 +15 15 | f'This is \\ a \\\'string\'' # Q003 +16 16 | f'"This" is a \'string\'' +17 17 | f"This is a 'string'" + +doubles_escaped.py:15:1: Q003 [*] Change outer quotes to avoid escaping inner quotes + | +13 | # Same as above, but with f-strings +14 | f'This is a \'string\'' # Q003 +15 | f'This is \\ a \\\'string\'' # Q003 + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Q003 +16 | f'"This" is a \'string\'' +17 | f"This is a 'string'" + | + = help: Change outer quotes to avoid escaping inner quotes + +ℹ Fix +12 12 | +13 13 | # Same as above, but with f-strings +14 14 | f'This is a \'string\'' # Q003 +15 |-f'This is \\ a \\\'string\'' # Q003 + 15 |+f"This is \\ a \\'string'" # Q003 +16 16 | f'"This" is a \'string\'' +17 17 | f"This is a 'string'" +18 18 | f"\"This\" is a 'string'" + +doubles_escaped.py:23:5: Q003 [*] Change outer quotes to avoid escaping inner quotes + | +21 | foo = ( +22 | f'This is a' +23 | f'\'string\'' # Q003 + | ^^^^^^^^^^^^^ Q003 +24 | ) + | + = help: Change outer quotes to avoid escaping inner quotes + +ℹ Fix +20 20 | fR'This is a \'string\'' +21 21 | foo = ( +22 22 | f'This is a' +23 |- f'\'string\'' # Q003 + 23 |+ f"'string'" # Q003 +24 24 | ) +25 25 | +26 26 | # Nested f-strings (Python 3.12+) + + diff --git a/crates/ruff_linter/src/settings/types.rs b/crates/ruff_linter/src/settings/types.rs index 1f5c073918164..dc8a1e0f4ffd1 100644 --- a/crates/ruff_linter/src/settings/types.rs +++ b/crates/ruff_linter/src/settings/types.rs @@ -90,6 +90,13 @@ impl PythonVersion { } minimum_version } + + /// Return `true` if the current version supports [PEP 701]. + /// + /// [PEP 701]: https://peps.python.org/pep-0701/ + pub fn supports_pep701(self) -> bool { + self >= Self::Py312 + } } #[derive(Clone, Copy, Debug, PartialEq, Eq, Default, CacheKey, is_macro::Is)] diff --git a/crates/ruff_python_parser/src/token.rs b/crates/ruff_python_parser/src/token.rs index b95ce5a18ebc8..ac441395fffc2 100644 --- a/crates/ruff_python_parser/src/token.rs +++ b/crates/ruff_python_parser/src/token.rs @@ -401,14 +401,7 @@ impl TryFrom<[char; 2]> for StringKind { impl fmt::Display for StringKind { fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { - use StringKind::{Bytes, RawBytes, RawString, String, Unicode}; - match self { - String => f.write_str(""), - Bytes => f.write_str("b"), - RawString => f.write_str("r"), - RawBytes => f.write_str("rb"), - Unicode => f.write_str("u"), - } + f.write_str(self.as_str()) } } @@ -442,6 +435,17 @@ impl StringKind { }; len.into() } + + pub fn as_str(&self) -> &'static str { + use StringKind::{Bytes, RawBytes, RawString, String, Unicode}; + match self { + String => "", + Bytes => "b", + RawString => "r", + RawBytes => "rb", + Unicode => "u", + } + } } // TODO move to ruff_python_parser? From 51f7ef1f3655f4784a887ee66f6f12dab71e2c7c Mon Sep 17 00:00:00 2001 From: Dhruv Manilawala Date: Wed, 27 Sep 2023 13:11:31 +0530 Subject: [PATCH 2/3] Simplify unescaping logic, rename fields --- .../rules/avoidable_escaped_quote.rs | 63 ++++++++----------- 1 file changed, 27 insertions(+), 36 deletions(-) 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 e6ee2ba4a0691..b143c56a84276 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 @@ -45,29 +45,29 @@ struct FStringContext { /// Whether to check for escaped quotes in the f-string. check_for_escaped_quote: bool, /// The range of the f-string start token. - fstring_start_range: TextRange, + start_range: TextRange, /// The ranges of the f-string middle tokens containing escaped quotes. - fstring_middle_ranges: Vec, + middle_ranges_with_escapes: Vec, } impl FStringContext { fn new(check_for_escaped_quote: bool, fstring_start_range: TextRange) -> Self { Self { check_for_escaped_quote, - fstring_start_range, - fstring_middle_ranges: vec![], + start_range: fstring_start_range, + middle_ranges_with_escapes: vec![], } } /// Update the context to not check for escaped quotes, and clear any /// existing reported ranges. - fn do_not_check_for_escaped_quote(&mut self) { + fn ignore_escaped_quotes(&mut self) { self.check_for_escaped_quote = false; - self.fstring_middle_ranges.clear(); + self.middle_ranges_with_escapes.clear(); } fn push_fstring_middle_range(&mut self, range: TextRange) { - self.fstring_middle_ranges.push(range); + self.middle_ranges_with_escapes.push(range); } } @@ -108,7 +108,7 @@ pub(crate) fn avoidable_escaped_quote( // ``` if matches!(tok, Tok::String { .. } | Tok::FStringStart) { if let Some(fstring_context) = fstrings.last_mut() { - fstring_context.do_not_check_for_escaped_quote(); + fstring_context.ignore_escaped_quotes(); continue; } } @@ -120,10 +120,14 @@ pub(crate) fn avoidable_escaped_quote( kind, triple_quoted, } => { + if kind.is_raw() || *triple_quoted { + continue; + } + // Check if we're using the preferred quotation style. - let uses_preferred_quote = leading_quote(locator.slice(tok_range)) - .is_some_and(|text| text.contains(quotes_settings.inline_quotes.as_char())); - if kind.is_raw() || *triple_quoted || !uses_preferred_quote { + if !leading_quote(locator.slice(tok_range)) + .is_some_and(|text| text.contains(quotes_settings.inline_quotes.as_char())) + { continue; } @@ -136,7 +140,7 @@ pub(crate) fn avoidable_escaped_quote( "{prefix}{quote}{value}{quote}", prefix = kind.as_str(), quote = quotes_settings.inline_quotes.opposite().as_char(), - value = unescaped_string(string_contents) + value = unescape_string(string_contents) ); diagnostic.set_fix(Fix::automatic(Edit::range_replacement( fixed_contents, @@ -175,27 +179,27 @@ pub(crate) fn avoidable_escaped_quote( let Some(context) = fstrings.pop() else { continue; }; - if context.fstring_middle_ranges.is_empty() { + if context.middle_ranges_with_escapes.is_empty() { // There are no `FStringMiddle` tokens containing any escaped // quotes. continue; } let mut diagnostic = Diagnostic::new( AvoidableEscapedQuote, - TextRange::new(context.fstring_start_range.start(), tok_range.end()), + TextRange::new(context.start_range.start(), tok_range.end()), ); if settings.rules.should_fix(diagnostic.kind.rule()) { let fstring_start_edit = Edit::range_replacement( // No need for `r`/`R` as we don't perform the checks // for raw strings. format!("f{}", quotes_settings.inline_quotes.opposite().as_char()), - context.fstring_start_range, + context.start_range, ); let fstring_middle_and_end_edits = context - .fstring_middle_ranges + .middle_ranges_with_escapes .iter() .map(|&range| { - Edit::range_replacement(unescaped_string(locator.slice(range)), range) + Edit::range_replacement(unescape_string(locator.slice(range)), range) }) .chain(std::iter::once( // `FStringEnd` edit @@ -220,35 +224,22 @@ pub(crate) fn avoidable_escaped_quote( } } -fn unescaped_string(value: &str) -> String { +fn unescape_string(value: &str) -> String { let mut fixed_contents = String::with_capacity(value.len()); - let chars: Vec = value.chars().collect(); - let mut backslash_count = 0; - for col_offset in 0..chars.len() { - let char = chars[col_offset]; + let mut chars = value.chars().peekable(); + while let Some(char) = chars.next() { if char != '\\' { fixed_contents.push(char); continue; } - backslash_count += 1; - // If the previous character was also a backslash - if col_offset > 0 && chars[col_offset - 1] == '\\' && backslash_count == 2 { - fixed_contents.push(char); - // reset to 0 - backslash_count = 0; - continue; - } // If we're at the end of the line - if col_offset == chars.len() - 1 { + let Some(next_char) = chars.peek() else { fixed_contents.push(char); continue; - } - let next_char = chars[col_offset + 1]; + }; // Remove quote escape - if next_char == '\'' || next_char == '"' { - // reset to 0 - backslash_count = 0; + if matches!(*next_char, '\'' | '"') { continue; } fixed_contents.push(char); From 9eb2dc4ee96f3d901969d902b2df47a68643a791 Mon Sep 17 00:00:00 2001 From: Dhruv Manilawala Date: Fri, 22 Sep 2023 10:18:57 +0530 Subject: [PATCH 3/3] Update `Q000-002` with the new f-string tokens --- crates/ruff_linter/src/checkers/tokens.rs | 2 +- ...{from_tokens.rs => check_string_quotes.rs} | 86 ++++++++++++++++--- .../src/rules/flake8_quotes/rules/mod.rs | 4 +- 3 files changed, 79 insertions(+), 13 deletions(-) rename crates/ruff_linter/src/rules/flake8_quotes/rules/{from_tokens.rs => check_string_quotes.rs} (82%) diff --git a/crates/ruff_linter/src/checkers/tokens.rs b/crates/ruff_linter/src/checkers/tokens.rs index f5525251b573b..8d1f5a28fd5f9 100644 --- a/crates/ruff_linter/src/checkers/tokens.rs +++ b/crates/ruff_linter/src/checkers/tokens.rs @@ -127,7 +127,7 @@ pub(crate) fn check_tokens( Rule::BadQuotesMultilineString, Rule::BadQuotesDocstring, ]) { - flake8_quotes::rules::from_tokens(&mut diagnostics, tokens, locator, settings); + flake8_quotes::rules::check_string_quotes(&mut diagnostics, tokens, locator, settings); } if settings.rules.any_enabled(&[ diff --git a/crates/ruff_linter/src/rules/flake8_quotes/rules/from_tokens.rs b/crates/ruff_linter/src/rules/flake8_quotes/rules/check_string_quotes.rs similarity index 82% rename from crates/ruff_linter/src/rules/flake8_quotes/rules/from_tokens.rs rename to crates/ruff_linter/src/rules/flake8_quotes/rules/check_string_quotes.rs index 5bcc9e52e1e55..8df2c67f3733f 100644 --- a/crates/ruff_linter/src/rules/flake8_quotes/rules/from_tokens.rs +++ b/crates/ruff_linter/src/rules/flake8_quotes/rules/check_string_quotes.rs @@ -1,6 +1,6 @@ use ruff_python_parser::lexer::LexResult; use ruff_python_parser::Tok; -use ruff_text_size::TextRange; +use ruff_text_size::{TextRange, TextSize}; use ruff_diagnostics::{AlwaysAutofixableViolation, Diagnostic, Edit, Fix}; use ruff_macros::{derive_message_formats, violation}; @@ -354,8 +354,59 @@ fn strings( diagnostics } +/// A builder for the f-string range. +/// +/// For now, this is limited to the outermost f-string and doesn't support +/// nested f-strings. +#[derive(Debug, Default)] +struct FStringRangeBuilder { + start_location: TextSize, + end_location: TextSize, + nesting: u32, +} + +impl FStringRangeBuilder { + fn visit_token(&mut self, token: &Tok, range: TextRange) { + match token { + Tok::FStringStart => { + if self.nesting == 0 { + self.start_location = range.start(); + } + self.nesting += 1; + } + Tok::FStringEnd => { + self.nesting = self.nesting.saturating_sub(1); + if self.nesting == 0 { + self.end_location = range.end(); + } + } + _ => {} + } + } + + /// Returns `true` if the lexer is currently inside of a f-string. + /// + /// It'll return `false` once the `FStringEnd` token for the outermost + /// f-string is visited. + const fn in_fstring(&self) -> bool { + self.nesting > 0 + } + + /// Returns the complete range of the previously visited f-string. + /// + /// This method should only be called once the lexer is outside of any + /// f-string otherwise it might return an invalid range. + /// + /// It doesn't consume the builder because there can be multiple f-strings + /// throughout the source code. + fn finish(&self) -> TextRange { + debug_assert!(!self.in_fstring()); + TextRange::new(self.start_location, self.end_location) + } +} + /// Generate `flake8-quote` diagnostics from a token stream. -pub(crate) fn from_tokens( +pub(crate) fn check_string_quotes( diagnostics: &mut Vec, lxr: &[LexResult], locator: &Locator, @@ -365,7 +416,13 @@ pub(crate) fn from_tokens( // concatenation, and should thus be handled as a single unit. let mut sequence = vec![]; let mut state_machine = StateMachine::default(); + let mut fstring_range_builder = FStringRangeBuilder::default(); for &(ref tok, range) in lxr.iter().flatten() { + fstring_range_builder.visit_token(tok, range); + if fstring_range_builder.in_fstring() { + continue; + } + let is_docstring = state_machine.consume(tok); // If this is a docstring, consume the existing sequence, then consume the @@ -379,14 +436,23 @@ pub(crate) fn from_tokens( diagnostics.push(diagnostic); } } else { - if tok.is_string() { - // If this is a string, add it to the sequence. - sequence.push(range); - } else if !matches!(tok, Tok::Comment(..) | Tok::NonLogicalNewline) { - // Otherwise, consume the sequence. - if !sequence.is_empty() { - diagnostics.extend(strings(locator, &sequence, settings)); - sequence.clear(); + match tok { + Tok::String { .. } => { + // If this is a string, add it to the sequence. + sequence.push(range); + } + Tok::FStringEnd => { + // If this is the end of an f-string, add the entire f-string + // range to the sequence. + sequence.push(fstring_range_builder.finish()); + } + Tok::Comment(..) | Tok::NonLogicalNewline => continue, + _ => { + // Otherwise, consume the sequence. + if !sequence.is_empty() { + diagnostics.extend(strings(locator, &sequence, settings)); + sequence.clear(); + } } } } 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 b57ddd014ba07..1f64976bf24b1 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,5 @@ pub(crate) use avoidable_escaped_quote::*; -pub(crate) use from_tokens::*; +pub(crate) use check_string_quotes::*; mod avoidable_escaped_quote; -mod from_tokens; +mod check_string_quotes;