Skip to content

Commit

Permalink
refactor: Simplify quote selection logic (#13536)
Browse files Browse the repository at this point in the history
  • Loading branch information
MichaReiser authored Sep 27, 2024
1 parent 253f5f2 commit f3e464e
Show file tree
Hide file tree
Showing 4 changed files with 23 additions and 51 deletions.
4 changes: 1 addition & 3 deletions crates/ruff_python_formatter/src/other/bytes_literal.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,11 +8,9 @@ pub struct FormatBytesLiteral;

impl FormatNodeRule<BytesLiteral> for FormatBytesLiteral {
fn fmt_fields(&self, item: &BytesLiteral, f: &mut PyFormatter) -> FormatResult<()> {
let locator = f.context().locator();

StringNormalizer::from_context(f.context())
.with_preferred_quote_style(f.options().quote_style())
.normalize(item.into(), &locator)
.normalize(item.into())
.fmt(f)
}
}
6 changes: 2 additions & 4 deletions crates/ruff_python_formatter/src/other/f_string.rs
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ impl Format<PyFormatContext<'_>> for FormatFString<'_> {
// If f-string formatting is disabled (not in preview), then we will
// fall back to the previous behavior of normalizing the f-string.
if !is_f_string_formatting_enabled(f.context()) {
let result = normalizer.normalize(self.value.into(), &locator).fmt(f);
let result = normalizer.normalize(self.value.into()).fmt(f);
let comments = f.context().comments();
self.value.elements.iter().for_each(|value| {
comments.mark_verbatim_node_comments_formatted(value.into());
Expand All @@ -56,9 +56,7 @@ impl Format<PyFormatContext<'_>> for FormatFString<'_> {
return result;
}

let string_kind = normalizer
.choose_quotes(self.value.into(), &locator)
.flags();
let string_kind = normalizer.choose_quotes(self.value.into()).flags();

let context = FStringContext::new(
string_kind,
Expand Down
4 changes: 1 addition & 3 deletions crates/ruff_python_formatter/src/other/string_literal.rs
Original file line number Diff line number Diff line change
Expand Up @@ -46,8 +46,6 @@ impl StringLiteralKind {

impl Format<PyFormatContext<'_>> for FormatStringLiteral<'_> {
fn fmt(&self, f: &mut PyFormatter) -> FormatResult<()> {
let locator = f.context().locator();

let quote_style = f.options().quote_style();
let quote_style = if self.layout.is_docstring() && !quote_style.is_preserve() {
// Per PEP 8 and PEP 257, always prefer double quotes for docstrings,
Expand All @@ -60,7 +58,7 @@ impl Format<PyFormatContext<'_>> for FormatStringLiteral<'_> {
let normalized = StringNormalizer::from_context(f.context())
.with_quoting(self.layout.quoting())
.with_preferred_quote_style(quote_style)
.normalize(self.value.into(), &locator);
.normalize(self.value.into());

if self.layout.is_docstring() {
docstring::format(&normalized, f)
Expand Down
60 changes: 19 additions & 41 deletions crates/ruff_python_formatter/src/string/normalize.rs
Original file line number Diff line number Diff line change
@@ -1,36 +1,29 @@
use std::borrow::Cow;
use std::cmp::Ordering;
use std::iter::FusedIterator;

use ruff_formatter::FormatContext;
use ruff_python_ast::{str::Quote, AnyStringFlags, StringFlags};
use ruff_source_file::Locator;
use ruff_text_size::{Ranged, TextRange};

use crate::context::FStringState;
use crate::options::PythonVersion;
use crate::prelude::*;
use crate::preview::is_f_string_formatting_enabled;
use crate::string::{Quoting, StringPart, StringQuotes};
use crate::QuoteStyle;

pub(crate) struct StringNormalizer {
pub(crate) struct StringNormalizer<'a, 'src> {
quoting: Quoting,
preferred_quote_style: QuoteStyle,
parent_docstring_quote_char: Option<Quote>,
f_string_state: FStringState,
target_version: PythonVersion,
format_fstring: bool,
context: &'a PyFormatContext<'src>,
}

impl StringNormalizer {
pub(crate) fn from_context(context: &PyFormatContext<'_>) -> Self {
impl<'a, 'src> StringNormalizer<'a, 'src> {
pub(crate) fn from_context(context: &'a PyFormatContext<'src>) -> Self {
Self {
quoting: Quoting::default(),
preferred_quote_style: QuoteStyle::default(),
parent_docstring_quote_char: context.docstring(),
f_string_state: context.f_string_state(),
target_version: context.options().target_version(),
format_fstring: is_f_string_formatting_enabled(context),
context,
}
}

Expand All @@ -45,7 +38,7 @@ impl StringNormalizer {
}

fn quoting(&self, string: StringPart) -> Quoting {
if let FStringState::InsideExpressionElement(context) = self.f_string_state {
if let FStringState::InsideExpressionElement(context) = self.context.f_string_state() {
// If we're inside an f-string, we need to make sure to preserve the
// existing quotes unless we're inside a triple-quoted f-string and
// the inner string itself isn't triple-quoted. For example:
Expand All @@ -61,7 +54,7 @@ impl StringNormalizer {
// the original f-string is valid in terms of quoting, and we don't
// want to change that to make it invalid.
if (context.f_string().flags().is_triple_quoted() && !string.flags().is_triple_quoted())
|| self.target_version.supports_pep_701()
|| self.context.options().target_version().supports_pep_701()
{
self.quoting
} else {
Expand All @@ -73,8 +66,8 @@ impl StringNormalizer {
}

/// Computes the strings preferred quotes.
pub(crate) fn choose_quotes(&self, string: StringPart, locator: &Locator) -> QuoteSelection {
let raw_content = locator.slice(string.content_range());
pub(crate) fn choose_quotes(&self, string: StringPart) -> QuoteSelection {
let raw_content = self.context.locator().slice(string.content_range());
let first_quote_or_normalized_char_offset = raw_content
.bytes()
.position(|b| matches!(b, b'\\' | b'"' | b'\'' | b'\r' | b'{'));
Expand Down Expand Up @@ -131,7 +124,7 @@ impl StringNormalizer {
// Overall this is a bit of a corner case and just inverting the
// style from what the parent ultimately decided upon works, even
// if it doesn't have perfect alignment with PEP8.
if let Some(quote) = self.parent_docstring_quote_char {
if let Some(quote) = self.context.docstring() {
QuoteStyle::from(quote.opposite())
} else if self.preferred_quote_style.is_preserve() {
QuoteStyle::Preserve
Expand Down Expand Up @@ -175,13 +168,9 @@ impl StringNormalizer {
}

/// Computes the strings preferred quotes and normalizes its content.
pub(crate) fn normalize<'a>(
&self,
string: StringPart,
locator: &'a Locator,
) -> NormalizedString<'a> {
let raw_content = locator.slice(string.content_range());
let quote_selection = self.choose_quotes(string, locator);
pub(crate) fn normalize(&self, string: StringPart) -> NormalizedString<'src> {
let raw_content = self.context.locator().slice(string.content_range());
let quote_selection = self.choose_quotes(string);

let normalized = if let Some(first_quote_or_escape_offset) =
quote_selection.first_quote_or_normalized_char_offset
Expand All @@ -192,7 +181,7 @@ impl StringNormalizer {
quote_selection.flags,
// TODO: Remove the `b'{'` in `choose_quotes` when promoting the
// `format_fstring` preview style
self.format_fstring,
is_f_string_formatting_enabled(self.context),
)
} else {
Cow::Borrowed(raw_content)
Expand Down Expand Up @@ -405,21 +394,10 @@ fn choose_quotes_impl(
}
}

match preferred_quote {
Quote::Single => {
if single_quotes > double_quotes {
Quote::Double
} else {
Quote::Single
}
}
Quote::Double => {
if double_quotes > single_quotes {
Quote::Single
} else {
Quote::Double
}
}
match single_quotes.cmp(&double_quotes) {
Ordering::Less => Quote::Single,
Ordering::Equal => preferred_quote,
Ordering::Greater => Quote::Double,
}
};

Expand Down

0 comments on commit f3e464e

Please sign in to comment.