Skip to content

Commit

Permalink
Move Q004 to AST based checker (#10548)
Browse files Browse the repository at this point in the history
## Summary

Continuing with #7595, this PR moves the `Q004` rule to the AST checker.

## Test Plan

- [x] Existing test cases should pass
- [x] No ecosystem updates
  • Loading branch information
dhruvmanila authored Mar 25, 2024
1 parent d625f55 commit e9115b8
Show file tree
Hide file tree
Showing 7 changed files with 210 additions and 204 deletions.
3 changes: 3 additions & 0 deletions crates/ruff_linter/src/checkers/ast/analyze/string_like.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
}
4 changes: 0 additions & 4 deletions crates/ruff_linter/src/checkers/tokens.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
46 changes: 46 additions & 0 deletions crates/ruff_linter/src/rules/flake8_quotes/helpers.rs
Original file line number Diff line number Diff line change
@@ -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
}
1 change: 1 addition & 0 deletions crates/ruff_linter/src/rules/flake8_quotes/mod.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
//! Rules from [flake8-quotes](https://pypi.org/project/flake8-quotes/).
mod helpers;
pub(crate) mod rules;
pub mod settings;

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -48,65 +49,21 @@ 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,
/// The range of the f-string start token.
start_range: TextRange,
/// The ranges of the f-string middle tokens containing escaped quotes.
middle_ranges_with_escapes: Vec<TextRange>,
/// 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,
}
}

Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -290,157 +243,7 @@ pub(crate) fn avoidable_escaped_quote(
}
}

/// Q004
pub(crate) fn unnecessary_escaped_quote(
diagnostics: &mut Vec<Diagnostic>,
lxr: &[LexResult],
locator: &Locator,
) {
let mut fstrings: Vec<FStringContext> = 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
}
2 changes: 2 additions & 0 deletions crates/ruff_linter/src/rules/flake8_quotes/rules/mod.rs
Original file line number Diff line number Diff line change
@@ -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;
Loading

0 comments on commit e9115b8

Please sign in to comment.