Add Iterator impl for StringLike parts (#11399)

## Summary

This PR adds support to iterate over each part of a string-like
expression.

This similar to the one in the formatter:


128414cd95/crates/ruff_python_formatter/src/string/any.rs (L121-L125)

Although I don't think it's a 1-1 replacement in the formatter because
the one implemented in the formatter has another information for certain
variants (as can be seen for `FString`).

The main motivation for this is to avoid duplication for rules which
work only on the parts of the string and doesn't require any information
from the parent node. Here, the parent node being the expression node
which could be an implicitly concatenated string.

This PR also updates certain rule implementation to make use of this and
avoids logic duplication.
This commit is contained in:
Dhruv Manilawala 2024-05-13 21:22:03 +05:30 committed by GitHub
parent 10b85a0f07
commit 0dc130e841
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
5 changed files with 129 additions and 81 deletions

View file

@ -64,28 +64,15 @@ pub(crate) fn avoidable_escaped_quote(checker: &mut Checker, string_like: String
let mut rule_checker = AvoidableEscapedQuoteChecker::new(checker.locator(), checker.settings); let mut rule_checker = AvoidableEscapedQuoteChecker::new(checker.locator(), checker.settings);
match string_like { for part in string_like.parts() {
StringLike::String(expr) => { match part {
for string_literal in &expr.value { ast::StringLikePart::String(string_literal) => {
rule_checker.visit_string_literal(string_literal); rule_checker.visit_string_literal(string_literal);
} }
} ast::StringLikePart::Bytes(bytes_literal) => {
StringLike::Bytes(expr) => {
for bytes_literal in &expr.value {
rule_checker.visit_bytes_literal(bytes_literal); rule_checker.visit_bytes_literal(bytes_literal);
} }
} ast::StringLikePart::FString(f_string) => rule_checker.visit_f_string(f_string),
StringLike::FString(expr) => {
for part in &expr.value {
match part {
ast::FStringPart::Literal(string_literal) => {
rule_checker.visit_string_literal(string_literal);
}
ast::FStringPart::FString(f_string) => {
rule_checker.visit_f_string(f_string);
}
}
}
} }
} }

View file

@ -454,11 +454,7 @@ pub(crate) fn check_string_quotes(checker: &mut Checker, string_like: StringLike
return; return;
} }
let ranges: Vec<TextRange> = match string_like { let ranges: Vec<_> = string_like.parts().map(|part| part.range()).collect();
StringLike::String(node) => node.value.iter().map(Ranged::range).collect(),
StringLike::Bytes(node) => node.value.iter().map(Ranged::range).collect(),
StringLike::FString(node) => node.value.iter().map(Ranged::range).collect(),
};
if checker.semantic().in_pep_257_docstring() { if checker.semantic().in_pep_257_docstring() {
if checker.enabled(Rule::BadQuotesDocstring) { if checker.enabled(Rule::BadQuotesDocstring) {

View file

@ -53,39 +53,30 @@ pub(crate) fn unnecessary_escaped_quote(checker: &mut Checker, string_like: Stri
let locator = checker.locator(); let locator = checker.locator();
match string_like { for part in string_like.parts() {
StringLike::String(expr) => { match part {
for string in &expr.value { ast::StringLikePart::String(string_literal) => {
if let Some(diagnostic) = check_string_or_bytes( if let Some(diagnostic) = check_string_or_bytes(
locator, locator,
string.range(), string_literal.range(),
AnyStringFlags::from(string.flags), AnyStringFlags::from(string_literal.flags),
) { ) {
checker.diagnostics.push(diagnostic); checker.diagnostics.push(diagnostic);
} }
} }
} ast::StringLikePart::Bytes(bytes_literal) => {
StringLike::Bytes(expr) => { if let Some(diagnostic) = check_string_or_bytes(
for bytes in &expr.value {
if let Some(diagnostic) =
check_string_or_bytes(locator, bytes.range(), AnyStringFlags::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, locator,
string.range(), bytes_literal.range(),
AnyStringFlags::from(string.flags), AnyStringFlags::from(bytes_literal.flags),
), ) {
ast::FStringPart::FString(f_string) => check_f_string(locator, f_string),
} {
checker.diagnostics.push(diagnostic); checker.diagnostics.push(diagnostic);
}; }
}
ast::StringLikePart::FString(f_string) => {
if let Some(diagnostic) = check_f_string(locator, f_string) {
checker.diagnostics.push(diagnostic);
}
} }
} }
} }

View file

@ -192,33 +192,20 @@ pub(crate) fn ambiguous_unicode_character_string(checker: &mut Checker, string_l
Context::String Context::String
}; };
match string_like { for part in string_like.parts() {
StringLike::String(node) => {
for literal in &node.value {
let text = checker.locator().slice(literal);
ambiguous_unicode_character(
&mut checker.diagnostics,
text,
literal.range(),
context,
checker.settings,
);
}
}
StringLike::FString(node) => {
for part in &node.value {
match part { match part {
ast::FStringPart::Literal(literal) => { ast::StringLikePart::String(string_literal) => {
let text = checker.locator().slice(literal); let text = checker.locator().slice(string_literal);
ambiguous_unicode_character( ambiguous_unicode_character(
&mut checker.diagnostics, &mut checker.diagnostics,
text, text,
literal.range(), string_literal.range(),
context, context,
checker.settings, checker.settings,
); );
} }
ast::FStringPart::FString(f_string) => { ast::StringLikePart::Bytes(_) => {}
ast::StringLikePart::FString(f_string) => {
for literal in f_string.literals() { for literal in f_string.literals() {
let text = checker.locator().slice(literal); let text = checker.locator().slice(literal);
ambiguous_unicode_character( ambiguous_unicode_character(
@ -232,9 +219,6 @@ pub(crate) fn ambiguous_unicode_character_string(checker: &mut Checker, string_l
} }
} }
} }
}
StringLike::Bytes(_) => (),
}
} }
fn ambiguous_unicode_character( fn ambiguous_unicode_character(

View file

@ -1,3 +1,5 @@
use std::iter::FusedIterator;
use ruff_text_size::{Ranged, TextRange}; use ruff_text_size::{Ranged, TextRange};
use crate::AnyNodeRef; use crate::AnyNodeRef;
@ -394,9 +396,8 @@ impl LiteralExpressionRef<'_> {
} }
} }
/// An enum that holds a reference to a string-like literal from the AST. /// An enum that holds a reference to a string-like expression from the AST. This includes string
/// This includes string literals, bytes literals, and the literal parts of /// literals, bytes literals, and f-strings.
/// f-strings.
#[derive(Copy, Clone, Debug, PartialEq)] #[derive(Copy, Clone, Debug, PartialEq)]
pub enum StringLike<'a> { pub enum StringLike<'a> {
String(&'a ast::ExprStringLiteral), String(&'a ast::ExprStringLiteral),
@ -404,6 +405,17 @@ pub enum StringLike<'a> {
FString(&'a ast::ExprFString), FString(&'a ast::ExprFString),
} }
impl<'a> StringLike<'a> {
/// Returns an iterator over the [`StringLikePart`] contained in this string-like expression.
pub fn parts(&self) -> StringLikePartIter<'_> {
match self {
StringLike::String(expr) => StringLikePartIter::String(expr.value.iter()),
StringLike::Bytes(expr) => StringLikePartIter::Bytes(expr.value.iter()),
StringLike::FString(expr) => StringLikePartIter::FString(expr.value.iter()),
}
}
}
impl<'a> From<&'a ast::ExprStringLiteral> for StringLike<'a> { impl<'a> From<&'a ast::ExprStringLiteral> for StringLike<'a> {
fn from(value: &'a ast::ExprStringLiteral) -> Self { fn from(value: &'a ast::ExprStringLiteral) -> Self {
StringLike::String(value) StringLike::String(value)
@ -431,3 +443,81 @@ impl Ranged for StringLike<'_> {
} }
} }
} }
/// An enum that holds a reference to an individual part of a string-like expression.
#[derive(Copy, Clone, Debug, PartialEq)]
pub enum StringLikePart<'a> {
String(&'a ast::StringLiteral),
Bytes(&'a ast::BytesLiteral),
FString(&'a ast::FString),
}
impl<'a> From<&'a ast::StringLiteral> for StringLikePart<'a> {
fn from(value: &'a ast::StringLiteral) -> Self {
StringLikePart::String(value)
}
}
impl<'a> From<&'a ast::BytesLiteral> for StringLikePart<'a> {
fn from(value: &'a ast::BytesLiteral) -> Self {
StringLikePart::Bytes(value)
}
}
impl<'a> From<&'a ast::FString> for StringLikePart<'a> {
fn from(value: &'a ast::FString) -> Self {
StringLikePart::FString(value)
}
}
impl Ranged for StringLikePart<'_> {
fn range(&self) -> TextRange {
match self {
StringLikePart::String(part) => part.range(),
StringLikePart::Bytes(part) => part.range(),
StringLikePart::FString(part) => part.range(),
}
}
}
/// An iterator over all the [`StringLikePart`] of a string-like expression.
///
/// This is created by the [`StringLike::parts`] method.
pub enum StringLikePartIter<'a> {
String(std::slice::Iter<'a, ast::StringLiteral>),
Bytes(std::slice::Iter<'a, ast::BytesLiteral>),
FString(std::slice::Iter<'a, ast::FStringPart>),
}
impl<'a> Iterator for StringLikePartIter<'a> {
type Item = StringLikePart<'a>;
fn next(&mut self) -> Option<Self::Item> {
let part = match self {
StringLikePartIter::String(inner) => StringLikePart::String(inner.next()?),
StringLikePartIter::Bytes(inner) => StringLikePart::Bytes(inner.next()?),
StringLikePartIter::FString(inner) => {
let part = inner.next()?;
match part {
ast::FStringPart::Literal(string_literal) => {
StringLikePart::String(string_literal)
}
ast::FStringPart::FString(f_string) => StringLikePart::FString(f_string),
}
}
};
Some(part)
}
fn size_hint(&self) -> (usize, Option<usize>) {
match self {
StringLikePartIter::String(inner) => inner.size_hint(),
StringLikePartIter::Bytes(inner) => inner.size_hint(),
StringLikePartIter::FString(inner) => inner.size_hint(),
}
}
}
impl FusedIterator for StringLikePartIter<'_> {}
impl ExactSizeIterator for StringLikePartIter<'_> {}