Skip to content

Commit

Permalink
Separate StringNormalizer from StringPart
Browse files Browse the repository at this point in the history
  • Loading branch information
@dhruvmanila
dhruvmanila committed Feb 13, 2024
1 parent 1ccd835 commit a431f6a
Show file tree
Hide file tree
Showing 4 changed files with 170 additions and 117 deletions.
14 changes: 4 additions & 10 deletions crates/ruff_python_formatter/src/other/bytes_literal.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,7 @@ use ruff_python_ast::BytesLiteral;
use ruff_text_size::Ranged;

use crate::prelude::*;
use crate::preview::is_hex_codes_in_unicode_sequences_enabled;
use crate::string::{Quoting, StringPart};
use crate::string::{StringNormalizer, StringPart};

#[derive(Default)]
pub struct FormatBytesLiteral;
Expand All @@ -12,14 +11,9 @@ impl FormatNodeRule<BytesLiteral> for FormatBytesLiteral {
fn fmt_fields(&self, item: &BytesLiteral, f: &mut PyFormatter) -> FormatResult<()> {
let locator = f.context().locator();

StringPart::from_source(item.range(), &locator)
.normalize(
Quoting::CanChange,
&locator,
f.options().quote_style(),
f.context().docstring(),
is_hex_codes_in_unicode_sequences_enabled(f.context()),
)
StringNormalizer::from_context(f.context())
.with_preferred_quote_style(f.options().quote_style())
.normalize(&StringPart::from_source(item.range(), &locator), &locator)
.fmt(f)
}
}
12 changes: 5 additions & 7 deletions crates/ruff_python_formatter/src/other/f_string.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,7 @@ use ruff_python_ast::FString;
use ruff_text_size::Ranged;

use crate::prelude::*;
use crate::preview::is_hex_codes_in_unicode_sequences_enabled;
use crate::string::{Quoting, StringPart};
use crate::string::{Quoting, StringNormalizer, StringPart};

/// Formats an f-string which is part of a larger f-string expression.
///
Expand All @@ -26,13 +25,12 @@ impl Format<PyFormatContext<'_>> for FormatFString<'_> {
fn fmt(&self, f: &mut PyFormatter) -> FormatResult<()> {
let locator = f.context().locator();

let result = StringPart::from_source(self.value.range(), &locator)
let result = StringNormalizer::from_context(f.context())
.with_quoting(self.quoting)
.with_preferred_quote_style(f.options().quote_style())
.normalize(
self.quoting,
&StringPart::from_source(self.value.range(), &locator),
&locator,
f.options().quote_style(),
f.context().docstring(),
is_hex_codes_in_unicode_sequences_enabled(f.context()),
)
.fmt(f);

Expand Down
17 changes: 8 additions & 9 deletions crates/ruff_python_formatter/src/other/string_literal.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,7 @@ use ruff_python_ast::StringLiteral;
use ruff_text_size::Ranged;

use crate::prelude::*;
use crate::preview::is_hex_codes_in_unicode_sequences_enabled;
use crate::string::{docstring, Quoting, StringPart};
use crate::string::{docstring, Quoting, StringNormalizer, StringPart};
use crate::QuoteStyle;

pub(crate) struct FormatStringLiteral<'a> {
Expand Down Expand Up @@ -59,13 +58,13 @@ impl Format<PyFormatContext<'_>> for FormatStringLiteral<'_> {
quote_style
};

let normalized = StringPart::from_source(self.value.range(), &locator).normalize(
self.layout.quoting(),
&locator,
quote_style,
f.context().docstring(),
is_hex_codes_in_unicode_sequences_enabled(f.context()),
);
let normalized = StringNormalizer::from_context(f.context())
.with_quoting(self.layout.quoting())
.with_preferred_quote_style(quote_style)
.normalize(
&StringPart::from_source(self.value.range(), &locator),
&locator,
);

if self.layout.is_docstring() {
docstring::format(&normalized, f)
Expand Down
244 changes: 153 additions & 91 deletions crates/ruff_python_formatter/src/string/mod.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ use crate::expression::parentheses::in_parentheses_only_soft_line_break_or_space
use crate::other::f_string::FormatFString;
use crate::other::string_literal::{FormatStringLiteral, StringLiteralKind};
use crate::prelude::*;
use crate::preview::is_hex_codes_in_unicode_sequences_enabled;
use crate::QuoteStyle;

pub(crate) mod docstring;
Expand Down Expand Up @@ -291,101 +292,70 @@ impl StringPart {
}
}

/// Returns the prefix of the string part.
pub(crate) const fn prefix(&self) -> StringPrefix {
self.prefix
}

/// Returns the surrounding quotes of the string part.
pub(crate) const fn quotes(&self) -> StringQuotes {
self.quotes
}

/// Returns the range of the string's content in the source (minus prefix and quotes).
pub(crate) const fn content_range(&self) -> TextRange {
self.content_range
}
}

pub(crate) struct StringNormalizer {
quoting: Quoting,
preferred_quote_style: QuoteStyle,
parent_docstring_quote_char: Option<QuoteChar>,
normalize_hex: bool,
}

impl StringNormalizer {
pub(crate) fn from_context(context: &PyFormatContext<'_>) -> Self {
Self {
quoting: Quoting::default(),
preferred_quote_style: QuoteStyle::default(),
parent_docstring_quote_char: context.docstring(),
normalize_hex: is_hex_codes_in_unicode_sequences_enabled(context),
}
}

pub(crate) fn with_preferred_quote_style(mut self, quote_style: QuoteStyle) -> Self {
self.preferred_quote_style = quote_style;
self
}

pub(crate) fn with_quoting(mut self, quoting: Quoting) -> Self {
self.quoting = quoting;
self
}

/// Computes the strings preferred quotes and normalizes its content.
///
/// The parent docstring quote style should be set when formatting a code
/// snippet within the docstring. The quote style should correspond to the
/// style of quotes used by said docstring. Normalization will ensure the
/// quoting styles don't conflict.
pub(crate) fn normalize<'a>(
self,
quoting: Quoting,
&self,
string: &StringPart,
locator: &'a Locator,
configured_style: QuoteStyle,
parent_docstring_quote_char: Option<QuoteChar>,
normalize_hex: bool,
) -> NormalizedString<'a> {
// Per PEP 8, always prefer double quotes for triple-quoted strings.
// Except when using quote-style-preserve.
let preferred_style = if self.quotes.triple {
// ... unless we're formatting a code snippet inside a docstring,
// then we specifically want to invert our quote style to avoid
// writing out invalid Python.
//
// It's worth pointing out that we can actually wind up being
// somewhat out of sync with PEP8 in this case. Consider this
// example:
//
// def foo():
// '''
// Something.
//
// >>> """tricksy"""
// '''
// pass
//
// Ideally, this would be reformatted as:
//
// def foo():
// """
// Something.
//
// >>> '''tricksy'''
// """
// pass
//
// But the logic here results in the original quoting being
// preserved. This is because the quoting style of the outer
// docstring is determined, in part, by looking at its contents. In
// this case, it notices that it contains a `"""` and thus infers
// that using `'''` would overall read better because it avoids
// the need to escape the interior `"""`. Except... in this case,
// the `"""` is actually part of a code snippet that could get
// reformatted to using a different quoting style itself.
//
// Fixing this would, I believe, require some fairly seismic
// changes to how formatting strings works. Namely, we would need
// to look for code snippets before normalizing the docstring, and
// then figure out the quoting style more holistically by looking
// at the various kinds of quotes used in the code snippets and
// what reformatting them might look like.
//
// 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) = parent_docstring_quote_char {
QuoteStyle::from(quote.invert())
} else if configured_style.is_preserve() {
QuoteStyle::Preserve
} else {
QuoteStyle::Double
}
} else {
configured_style
};

let raw_content = &locator.slice(self.content_range);

let quotes = match quoting {
Quoting::Preserve => self.quotes,
Quoting::CanChange => {
if let Some(preferred_quote) = QuoteChar::from_style(preferred_style) {
if self.prefix.is_raw_string() {
choose_quotes_raw(raw_content, self.quotes, preferred_quote)
} else {
choose_quotes(raw_content, self.quotes, preferred_quote)
}
} else {
self.quotes
}
}
};
let raw_content = locator.slice(string.content_range());

let quotes = choose_quotes(
string,
locator,
self.quoting,
self.preferred_quote_style,
self.parent_docstring_quote_char,
);

let normalized = normalize_string(raw_content, quotes, self.prefix, normalize_hex);
let normalized = normalize_string(raw_content, quotes, string.prefix(), self.normalize_hex);

NormalizedString {
prefix: self.prefix,
content_range: self.content_range,
prefix: string.prefix(),
content_range: string.content_range(),
text: normalized,
quotes,
}
Expand Down Expand Up @@ -507,12 +477,100 @@ impl Format<PyFormatContext<'_>> for StringPrefix {
}
}

/// Computes the strings preferred quotes.
///
/// The parent docstring quote style should be set when formatting a code
/// snippet within the docstring. The quote style should correspond to the
/// style of quotes used by said docstring.
pub(crate) fn choose_quotes(
string: &StringPart,
locator: &Locator,
quoting: Quoting,
configured_style: QuoteStyle,
parent_docstring_quote_char: Option<QuoteChar>,
) -> StringQuotes {
// Per PEP 8, always prefer double quotes for triple-quoted strings.
// Except when using quote-style-preserve.
let preferred_style = if string.quotes().triple {
// ... unless we're formatting a code snippet inside a docstring,
// then we specifically want to invert our quote style to avoid
// writing out invalid Python.
//
// It's worth pointing out that we can actually wind up being
// somewhat out of sync with PEP8 in this case. Consider this
// example:
//
// def foo():
// '''
// Something.
//
// >>> """tricksy"""
// '''
// pass
//
// Ideally, this would be reformatted as:
//
// def foo():
// """
// Something.
//
// >>> '''tricksy'''
// """
// pass
//
// But the logic here results in the original quoting being
// preserved. This is because the quoting style of the outer
// docstring is determined, in part, by looking at its contents. In
// this case, it notices that it contains a `"""` and thus infers
// that using `'''` would overall read better because it avoids
// the need to escape the interior `"""`. Except... in this case,
// the `"""` is actually part of a code snippet that could get
// reformatted to using a different quoting style itself.
//
// Fixing this would, I believe, require some fairly seismic
// changes to how formatting strings works. Namely, we would need
// to look for code snippets before normalizing the docstring, and
// then figure out the quoting style more holistically by looking
// at the various kinds of quotes used in the code snippets and
// what reformatting them might look like.
//
// 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) = parent_docstring_quote_char {
QuoteStyle::from(quote.invert())
} else if configured_style.is_preserve() {
QuoteStyle::Preserve
} else {
QuoteStyle::Double
}
} else {
configured_style
};

match quoting {
Quoting::Preserve => string.quotes(),
Quoting::CanChange => {
if let Some(preferred_quote) = QuoteChar::from_style(preferred_style) {
let raw_content = locator.slice(string.content_range());
if string.prefix().is_raw_string() {
choose_quotes_for_raw_string(raw_content, string.quotes(), preferred_quote)
} else {
choose_quotes_impl(raw_content, string.quotes(), preferred_quote)
}
} else {
string.quotes()
}
}
}
}

/// Choose the appropriate quote style for a raw string.
///
/// The preferred quote style is chosen unless the string contains unescaped quotes of the
/// preferred style. For example, `r"foo"` is chosen over `r'foo'` if the preferred quote
/// style is double quotes.
fn choose_quotes_raw(
fn choose_quotes_for_raw_string(
input: &str,
quotes: StringQuotes,
preferred_quote: QuoteChar,
Expand Down Expand Up @@ -571,7 +629,11 @@ fn choose_quotes_raw(
/// For triple quoted strings, the preferred quote style is always used, unless the string contains
/// a triplet of the quote character (e.g., if double quotes are preferred, double quotes will be
/// used unless the string contains `"""`).
fn choose_quotes(input: &str, quotes: StringQuotes, preferred_quote: QuoteChar) -> StringQuotes {
fn choose_quotes_impl(
input: &str,
quotes: StringQuotes,
preferred_quote: QuoteChar,
) -> StringQuotes {
let quote = if quotes.triple {
// True if the string contains a triple quote sequence of the configured quote style.
let mut uses_triple_quotes = false;
Expand Down Expand Up @@ -780,7 +842,7 @@ impl TryFrom<char> for QuoteChar {
/// with the provided [`StringQuotes`] style.
///
/// Returns the normalized string and whether it contains new lines.
fn normalize_string(
pub(crate) fn normalize_string(
input: &str,
quotes: StringQuotes,
prefix: StringPrefix,
Expand Down

0 comments on commit a431f6a

Please sign in to comment.