Skip to content

Commit

Permalink
Alternate quotes in nested f-strings when preview mode is enabled and…
Browse files Browse the repository at this point in the history 
… targeting Py312+
  • Loading branch information
@MichaReiser
MichaReiser committed Oct 21, 2024
1 parent 2550e83 commit 5071965
Show file tree
Hide file tree
Showing 7 changed files with 264 additions and 141 deletions.
4 changes: 4 additions & 0 deletions crates/ruff_python_ast/src/expression.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -524,6 +524,10 @@ impl StringLikePart<'_> {
self.end() - kind.closer_len(),
)
}

pub const fn is_fstring(self) -> bool {
matches!(self, Self::FString(_))
}
}

impl<'a> From<&'a ast::StringLiteral> for StringLikePart<'a> {
Expand Down
37 changes: 12 additions & 25 deletions crates/ruff_python_formatter/src/other/f_string.rs
View file
Original file line number Diff line number Diff line change
@@ -1,12 +1,12 @@
use ruff_formatter::write;
use ruff_python_ast::{AnyStringFlags, FString, StringFlags};
use ruff_source_file::Locator;

use crate::prelude::*;
use crate::preview::{
is_f_string_formatting_enabled, is_f_string_implicit_concatenated_string_literal_quotes_enabled,
};
use crate::string::{Quoting, StringNormalizer, StringQuotes};
use ruff_formatter::write;
use ruff_python_ast::{AnyStringFlags, FString, StringFlags};
use ruff_source_file::Locator;
use ruff_text_size::Ranged;

use super::f_string_element::FormatFStringElement;

Expand Down Expand Up @@ -35,7 +35,7 @@ impl Format<PyFormatContext<'_>> for FormatFString<'_> {
// f-string instead of globally for the entire f-string expression.
let quoting =
if is_f_string_implicit_concatenated_string_literal_quotes_enabled(f.context()) {
f_string_quoting(self.value, &locator)
Quoting::CanChange
} else {
self.quoting
};
Expand Down Expand Up @@ -92,17 +92,21 @@ impl Format<PyFormatContext<'_>> for FormatFString<'_> {

#[derive(Clone, Copy, Debug)]
pub(crate) struct FStringContext {
flags: AnyStringFlags,
/// The string flags of the enclosing f-string part.
enclosing_flags: AnyStringFlags,
layout: FStringLayout,
}

impl FStringContext {
const fn new(flags: AnyStringFlags, layout: FStringLayout) -> Self {
Self { flags, layout }
Self {
enclosing_flags: flags,
layout,
}
}

pub(crate) fn flags(self) -> AnyStringFlags {
self.flags
self.enclosing_flags
}

pub(crate) const fn layout(self) -> FStringLayout {
Expand Down Expand Up @@ -149,20 +153,3 @@ impl FStringLayout {
matches!(self, FStringLayout::Multiline)
}
}

fn f_string_quoting(f_string: &FString, locator: &Locator) -> Quoting {
let triple_quoted = f_string.flags.is_triple_quoted();

if f_string.elements.expressions().any(|expression| {
let string_content = locator.slice(expression.range());
if triple_quoted {
string_content.contains(r#"""""#) || string_content.contains("'''")
} else {
string_content.contains(['"', '\''])
}
}) {
Quoting::Preserve
} else {
Quoting::CanChange
}
}
213 changes: 170 additions & 43 deletions crates/ruff_python_formatter/src/string/normalize.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ use std::cmp::Ordering;
use std::iter::FusedIterator;

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

use crate::context::FStringState;
Expand Down Expand Up @@ -37,51 +37,30 @@ impl<'a, 'src> StringNormalizer<'a, 'src> {
self
}

fn quoting(&self, string: StringLikePart) -> Quoting {
match (self.quoting, self.context.f_string_state()) {
(Quoting::Preserve, _) => Quoting::Preserve,

// 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:
//
// ```python
// f"""outer {"inner"}""" # Valid
// f"""outer {"""inner"""}""" # Invalid
// ```
//
// Or, if the target version supports PEP 701.
//
// The reason to preserve the quotes is based on the assumption that
// the original f-string is valid in terms of quoting, and we don't
// want to change that to make it invalid.
(Quoting::CanChange, FStringState::InsideExpressionElement(context)) => {
if (context.f_string().flags().is_triple_quoted()
&& !string.flags().is_triple_quoted())
|| self.context.options().target_version().supports_pep_701()
{
Quoting::CanChange
} else {
Quoting::Preserve
}
}

(Quoting::CanChange, _) => Quoting::CanChange,
}
}

/// Determines the preferred quote style for `string`.
/// The formatter should use the preferred quote style unless
/// it can't because the string contains the preferred quotes OR
/// it leads to more escaping.
pub(super) fn preferred_quote_style(&self, string: StringLikePart) -> QuoteStyle {
match self.quoting(string) {
match self.quoting {
Quoting::Preserve => QuoteStyle::Preserve,
Quoting::CanChange => {
let preferred_quote_style = self
.preferred_quote_style
.unwrap_or(self.context.options().quote_style());

if preferred_quote_style.is_preserve() {
return QuoteStyle::Preserve;
}

if let FStringState::InsideExpressionElement(parent_context) =
self.context.f_string_state()
{
return QuoteStyle::from(
parent_context.f_string().flags().quote_style().opposite(),
);
}

// Per PEP 8, always prefer double quotes for triple-quoted strings.
// Except when using quote-style-preserve.
if string.flags().is_triple_quoted() {
Expand Down Expand Up @@ -132,8 +111,6 @@ impl<'a, 'src> StringNormalizer<'a, 'src> {
// if it doesn't have perfect alignment with PEP8.
if let Some(quote) = self.context.docstring() {
QuoteStyle::from(quote.opposite())
} else if preferred_quote_style.is_preserve() {
QuoteStyle::Preserve
} else {
QuoteStyle::Double
}
Expand All @@ -146,6 +123,49 @@ impl<'a, 'src> StringNormalizer<'a, 'src> {

/// Computes the strings preferred quotes.
pub(crate) fn choose_quotes(&self, string: StringLikePart) -> QuoteSelection {
// Preserve the f-string quotes if the target version isn't newer or equal than Python 3.12
// and an f-string expression contains a debug text with a quote character
// because the formatter will emit the debug expression **exctly** the same as in the source text.
if is_f_string_formatting_enabled(self.context)
&& !self.context.options().target_version().supports_pep_701()
{
if let StringLikePart::FString(fstring) = string {
if fstring
.elements
.iter()
.filter_map(FStringElement::as_expression)
.any(|expression| {
if expression.debug_text.is_some() {
let content = self.context.locator().slice(expression.range());
match string.flags().quote_style() {
Quote::Single => {
if string.flags().is_triple_quoted() {
content.contains(r#"""""#)
} else {
content.contains('"')
}
}
Quote::Double => {
if string.flags().is_triple_quoted() {
content.contains("'''")
} else {
content.contains('\'')
}
}
}
} else {
false
}
})
{
return QuoteSelection {
flags: string.flags(),
first_quote_or_normalized_char_offset: None,
};
}
}
}

let raw_content = self.context.locator().slice(string.content_range());
let first_quote_or_normalized_char_offset = raw_content
.bytes()
Expand All @@ -163,12 +183,18 @@ impl<'a, 'src> StringNormalizer<'a, 'src> {
// The preferred quote style is single or double quotes, and the string contains a quote or
// another character that may require escaping
(Ok(preferred_quote), Some(first_quote_or_normalized_char_offset)) => {
let quote = QuoteMetadata::from_str(
&raw_content[first_quote_or_normalized_char_offset..],
string.flags(),
preferred_quote,
)
.choose(preferred_quote);
let metadata = if string.is_fstring() {
QuoteMetadata::from_part(string, self.context, preferred_quote)
} else {
QuoteMetadata::from_str(
&raw_content[first_quote_or_normalized_char_offset..],
string.flags(),
preferred_quote,
)
};

let quote = metadata.choose(preferred_quote);

string_flags.with_quote_style(quote)
}

Expand Down Expand Up @@ -235,6 +261,52 @@ pub(crate) struct QuoteMetadata {
/// Tracks information about the used quotes in a string which is used
/// to choose the quotes for a part.
impl QuoteMetadata {
pub(crate) fn from_part(
part: StringLikePart,
context: &PyFormatContext,
preferred_quote: Quote,
) -> Self {
match part {
StringLikePart::String(_) | StringLikePart::Bytes(_) => {
let text = context.locator().slice(part.content_range());

Self::from_str(text, part.flags(), preferred_quote)
}
StringLikePart::FString(fstring) => {
// TODO: Should we limit this behavior to Post 312?
if is_f_string_formatting_enabled(context) {
let mut literals = fstring.elements.iter().filter_map(FStringElement::as_literal);

let Some(first) = literals.next() else {
return QuoteMetadata::from_str("", part.flags(), preferred_quote);
};

let mut metadata = QuoteMetadata::from_str(
context.locator().slice(first.range()),
fstring.flags.into(),
preferred_quote,
);

for literal in literals {
metadata = metadata
.merge(&QuoteMetadata::from_str(
context.locator().slice(literal.range()),
fstring.flags.into(),
preferred_quote,
))
.expect("Merge to succeed because all parts have the same flags");
}

metadata
} else {
let text = context.locator().slice(part.content_range());

Self::from_str(text, part.flags(), preferred_quote)
}
}
}
}

pub(crate) fn from_str(text: &str, flags: AnyStringFlags, preferred_quote: Quote) -> Self {
let kind = if flags.is_raw_string() {
QuoteMetadataKind::raw(text, preferred_quote, flags.is_triple_quoted())
Expand Down Expand Up @@ -276,6 +348,61 @@ impl QuoteMetadata {
},
}
}

/// Merges the quotes metadata of different literals.
///
/// ## Raw and triple quoted strings
/// Merging raw and triple quoted strings is only correct if all literals are from the same part.
/// E.g. it's okay to merge triple and raw strings from a single `FString` part's literals
/// but it isn't safe to merge raw and triple quoted strings from different parts of an implicit
/// concatenated string. Where safe means, it may lead to incorrect results.
pub(super) fn merge(self, other: &QuoteMetadata) -> Option<QuoteMetadata> {
let kind = match (self.kind, other.kind) {
(
QuoteMetadataKind::Regular {
single_quotes: self_single,
double_quotes: self_double,
},
QuoteMetadataKind::Regular {
single_quotes: other_single,
double_quotes: other_double,
},
) => QuoteMetadataKind::Regular {
single_quotes: self_single + other_single,
double_quotes: self_double + other_double,
},

// Can't merge quotes from raw strings (even when both strings are raw)
(
QuoteMetadataKind::Raw {
contains_preferred: self_contains_preferred,
},
QuoteMetadataKind::Raw {
contains_preferred: other_contains_preferred,
},
) => QuoteMetadataKind::Raw {
contains_preferred: self_contains_preferred || other_contains_preferred,
},

(
QuoteMetadataKind::Triple {
contains_preferred: self_contains_preferred,
},
QuoteMetadataKind::Triple {
contains_preferred: other_contains_preferred,
},
) => QuoteMetadataKind::Triple {
contains_preferred: self_contains_preferred || other_contains_preferred,
},

(_, _) => return None,
};

Some(Self {
kind,
source_style: self.source_style,
})
}
}

#[derive(Copy, Clone, Debug)]
Expand Down
Loading

0 comments on commit 5071965

Please sign in to comment.