Move Q004 to AST based checker (#10548)

## 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
This commit is contained in:
Dhruv Manilawala
2024-03-25 09:01:31 +05:30
committed by GitHub
parent d625f55c05
commit e9115b8d8a
7 changed files with 210 additions and 204 deletions

View File

@@ -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);
}
}

View File

@@ -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,

View File

@@ -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
}

View File

@@ -1,4 +1,5 @@
//! Rules from [flake8-quotes](https://pypi.org/project/flake8-quotes/).
mod helpers;
pub(crate) mod rules;
pub mod settings;

View File

@@ -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<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,
}
}
@@ -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<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
}

View File

@@ -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;

View File

@@ -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<Diagnostic> {
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<Diagnostic> {
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()]
}