Skip to content

Commit

Permalink
Normalize implicit concatenated f-string quotes per part (#13539)
Browse files Browse the repository at this point in the history
  • Loading branch information
MichaReiser authored Oct 8, 2024
1 parent 42fcbef commit fc661e1
Show file tree
Hide file tree
Showing 11 changed files with 171 additions and 62 deletions.
7 changes: 4 additions & 3 deletions crates/ruff_python_formatter/generate.py
Original file line number Diff line number Diff line change
Expand Up @@ -33,14 +33,15 @@ def rustfmt(code: str) -> str:
nodes = []
for node_line in node_lines:
node = node_line.split("(")[1].split(")")[0].split("::")[-1].split("<")[0]
# `FString` and `StringLiteral` has a custom implementation while the formatting for
# `FStringLiteralElement` and `FStringExpressionElement` are handled by the `FString`
# `FString` has a custom implementation while the formatting for
# `FStringLiteralElement`, `FStringFormatSpec` and `FStringExpressionElement` are handled by the `FString`
# implementation.
if node in (
"FString",
"StringLiteral",
"FStringLiteralElement",
"FStringExpressionElement",
"FStringFormatSpec",
"Identifier",
):
continue
nodes.append(node)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -307,3 +307,11 @@
]
} --------
"""


# Implicit concatenated f-string containing quotes
_ = (
'This string should change its quotes to double quotes'
f'This string uses double quotes in an expression {"woah"}'
f'This f-string does not use any quotes.'
)
30 changes: 4 additions & 26 deletions crates/ruff_python_formatter/src/expression/expr_string_literal.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,37 +4,17 @@ use ruff_python_ast::{AnyNodeRef, ExprStringLiteral};
use crate::expression::parentheses::{
in_parentheses_only_group, NeedsParentheses, OptionalParentheses,
};
use crate::other::string_literal::{FormatStringLiteral, StringLiteralKind};
use crate::other::string_literal::StringLiteralKind;
use crate::prelude::*;
use crate::string::{AnyString, FormatImplicitConcatenatedString};

#[derive(Default)]
pub struct FormatExprStringLiteral {
kind: ExprStringLiteralKind,
}

#[derive(Default, Copy, Clone, Debug)]
pub enum ExprStringLiteralKind {
#[default]
String,
Docstring,
}

impl ExprStringLiteralKind {
const fn string_literal_kind(self) -> StringLiteralKind {
match self {
ExprStringLiteralKind::String => StringLiteralKind::String,
ExprStringLiteralKind::Docstring => StringLiteralKind::Docstring,
}
}

const fn is_docstring(self) -> bool {
matches!(self, ExprStringLiteralKind::Docstring)
}
kind: StringLiteralKind,
}

impl FormatRuleWithOptions<ExprStringLiteral, PyFormatContext<'_>> for FormatExprStringLiteral {
type Options = ExprStringLiteralKind;
type Options = StringLiteralKind;

fn with_options(mut self, options: Self::Options) -> Self {
self.kind = options;
Expand All @@ -47,9 +27,7 @@ impl FormatNodeRule<ExprStringLiteral> for FormatExprStringLiteral {
let ExprStringLiteral { value, .. } = item;

match value.as_slice() {
[string_literal] => {
FormatStringLiteral::new(string_literal, self.kind.string_literal_kind()).fmt(f)
}
[string_literal] => string_literal.format().with_options(self.kind).fmt(f),
_ => {
// This is just a sanity check because [`DocstringStmt::try_from_statement`]
// ensures that the docstring is a *single* string literal.
Expand Down
36 changes: 36 additions & 0 deletions crates/ruff_python_formatter/src/generated.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2935,6 +2935,42 @@ impl<'ast> IntoFormat<PyFormatContext<'ast>> for ast::TypeParamParamSpec {
}
}

impl FormatRule<ast::StringLiteral, PyFormatContext<'_>>
for crate::other::string_literal::FormatStringLiteral
{
#[inline]
fn fmt(&self, node: &ast::StringLiteral, f: &mut PyFormatter) -> FormatResult<()> {
FormatNodeRule::<ast::StringLiteral>::fmt(self, node, f)
}
}
impl<'ast> AsFormat<PyFormatContext<'ast>> for ast::StringLiteral {
type Format<'a> = FormatRefWithRule<
'a,
ast::StringLiteral,
crate::other::string_literal::FormatStringLiteral,
PyFormatContext<'ast>,
>;
fn format(&self) -> Self::Format<'_> {
FormatRefWithRule::new(
self,
crate::other::string_literal::FormatStringLiteral::default(),
)
}
}
impl<'ast> IntoFormat<PyFormatContext<'ast>> for ast::StringLiteral {
type Format = FormatOwnedWithRule<
ast::StringLiteral,
crate::other::string_literal::FormatStringLiteral,
PyFormatContext<'ast>,
>;
fn into_format(self) -> Self::Format {
FormatOwnedWithRule::new(
self,
crate::other::string_literal::FormatStringLiteral::default(),
)
}
}

impl FormatRule<ast::BytesLiteral, PyFormatContext<'_>>
for crate::other::bytes_literal::FormatBytesLiteral
{
Expand Down
38 changes: 33 additions & 5 deletions crates/ruff_python_formatter/src/other/f_string.rs
Original file line number Diff line number Diff line change
@@ -1,10 +1,12 @@
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 crate::prelude::*;
use crate::preview::is_f_string_formatting_enabled;
use crate::string::{Quoting, StringNormalizer, StringQuotes};
use ruff_text_size::Ranged;

use super::f_string_element::FormatFStringElement;

Expand All @@ -29,8 +31,17 @@ impl Format<PyFormatContext<'_>> for FormatFString<'_> {
fn fmt(&self, f: &mut PyFormatter) -> FormatResult<()> {
let locator = f.context().locator();

// If the preview style is enabled, make the decision on what quotes to use locally for each
// 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)
} else {
self.quoting
};

let normalizer = StringNormalizer::from_context(f.context())
.with_quoting(self.quoting)
.with_quoting(quoting)
.with_preferred_quote_style(f.options().quote_style());

// If f-string formatting is disabled (not in preview), then we will
Expand Down Expand Up @@ -140,3 +151,20 @@ 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
}
}
17 changes: 8 additions & 9 deletions crates/ruff_python_formatter/src/other/f_string_part.rs
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
use ruff_python_ast::FStringPart;

use crate::other::f_string::FormatFString;
use crate::other::string_literal::{FormatStringLiteral, StringLiteralKind};
use crate::other::string_literal::StringLiteralKind;
use crate::prelude::*;
use crate::string::Quoting;

Expand All @@ -25,14 +25,13 @@ impl<'a> FormatFStringPart<'a> {
impl Format<PyFormatContext<'_>> for FormatFStringPart<'_> {
fn fmt(&self, f: &mut PyFormatter) -> FormatResult<()> {
match self.part {
FStringPart::Literal(string_literal) => FormatStringLiteral::new(
string_literal,
// If an f-string part is a string literal, the f-string is always
// implicitly concatenated e.g., `"foo" f"bar {x}"`. A standalone
// string literal would be a string expression, not an f-string.
StringLiteralKind::InImplicitlyConcatenatedFString(self.quoting),
)
.fmt(f),
#[allow(deprecated)]
FStringPart::Literal(string_literal) => string_literal
.format()
.with_options(StringLiteralKind::InImplicitlyConcatenatedFString(
self.quoting,
))
.fmt(f),
FStringPart::FString(f_string) => FormatFString::new(f_string, self.quoting).fmt(f),
}
}
Expand Down
43 changes: 31 additions & 12 deletions crates/ruff_python_formatter/src/other/string_literal.rs
Original file line number Diff line number Diff line change
@@ -1,23 +1,28 @@
use ruff_formatter::FormatRuleWithOptions;
use ruff_python_ast::StringLiteral;

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

pub(crate) struct FormatStringLiteral<'a> {
value: &'a StringLiteral,
#[derive(Default)]
pub struct FormatStringLiteral {
layout: StringLiteralKind,
}

impl<'a> FormatStringLiteral<'a> {
pub(crate) fn new(value: &'a StringLiteral, layout: StringLiteralKind) -> Self {
Self { value, layout }
impl FormatRuleWithOptions<StringLiteral, PyFormatContext<'_>> for FormatStringLiteral {
type Options = StringLiteralKind;

fn with_options(mut self, layout: StringLiteralKind) -> Self {
self.layout = layout;
self
}
}

/// The kind of a string literal.
#[derive(Copy, Clone, Debug, Default)]
pub(crate) enum StringLiteralKind {
pub enum StringLiteralKind {
/// A normal string literal e.g., `"foo"`.
#[default]
String,
Expand All @@ -26,6 +31,8 @@ pub(crate) enum StringLiteralKind {
/// A string literal that is implicitly concatenated with an f-string. This
/// makes the overall expression an f-string whose quoting detection comes
/// from the parent node (f-string expression).
#[deprecated]
#[allow(private_interfaces)]
InImplicitlyConcatenatedFString(Quoting),
}

Expand All @@ -36,16 +43,28 @@ impl StringLiteralKind {
}

/// Returns the quoting to be used for this string literal.
fn quoting(self) -> Quoting {
fn quoting(self, context: &PyFormatContext) -> Quoting {
match self {
StringLiteralKind::String | StringLiteralKind::Docstring => Quoting::CanChange,
StringLiteralKind::InImplicitlyConcatenatedFString(quoting) => quoting,
#[allow(deprecated)]
StringLiteralKind::InImplicitlyConcatenatedFString(quoting) => {
// Allow string literals to pick the "optimal" quote character
// even if any other fstring in the implicit concatenation uses an expression
// containing a quote character.
// TODO: Remove StringLiteralKind::InImplicitlyConcatenatedFString when promoting
// this style to stable and remove the layout from `AnyStringPart::String`.
if is_f_string_implicit_concatenated_string_literal_quotes_enabled(context) {
Quoting::CanChange
} else {
quoting
}
}
}
}
}

impl Format<PyFormatContext<'_>> for FormatStringLiteral<'_> {
fn fmt(&self, f: &mut PyFormatter) -> FormatResult<()> {
impl FormatNodeRule<StringLiteral> for FormatStringLiteral {
fn fmt_fields(&self, item: &StringLiteral, f: &mut PyFormatter) -> FormatResult<()> {
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 @@ -56,9 +75,9 @@ impl Format<PyFormatContext<'_>> for FormatStringLiteral<'_> {
};

let normalized = StringNormalizer::from_context(f.context())
.with_quoting(self.layout.quoting())
.with_quoting(self.layout.quoting(f.context()))
.with_preferred_quote_style(quote_style)
.normalize(self.value.into());
.normalize(item.into());

if self.layout.is_docstring() {
docstring::format(&normalized, f)
Expand Down
7 changes: 7 additions & 0 deletions crates/ruff_python_formatter/src/preview.rs
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,13 @@ pub(crate) fn is_f_string_formatting_enabled(context: &PyFormatContext) -> bool
context.is_preview()
}

/// See [#13539](https://github.com/astral-sh/ruff/pull/13539)
pub(crate) fn is_f_string_implicit_concatenated_string_literal_quotes_enabled(
context: &PyFormatContext,
) -> bool {
context.is_preview()
}

pub(crate) fn is_with_single_item_pre_39_enabled(context: &PyFormatContext) -> bool {
context.is_preview()
}
Expand Down
4 changes: 2 additions & 2 deletions crates/ruff_python_formatter/src/statement/suite.rs
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ use crate::comments::{
leading_comments, trailing_comments, Comments, LeadingDanglingTrailingComments,
};
use crate::context::{NodeLevel, TopLevelStatementPosition, WithIndentLevel, WithNodeLevel};
use crate::expression::expr_string_literal::ExprStringLiteralKind;
use crate::other::string_literal::StringLiteralKind;
use crate::prelude::*;
use crate::statement::stmt_expr::FormatStmtExpr;
use crate::verbatim::{
Expand Down Expand Up @@ -850,7 +850,7 @@ impl Format<PyFormatContext<'_>> for DocstringStmt<'_> {
.then_some(source_position(self.docstring.start())),
string_literal
.format()
.with_options(ExprStringLiteralKind::Docstring),
.with_options(StringLiteralKind::Docstring),
f.options()
.source_map_generation()
.is_enabled()
Expand Down
7 changes: 3 additions & 4 deletions crates/ruff_python_formatter/src/string/any.rs
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ use ruff_text_size::{Ranged, TextRange};

use crate::expression::expr_f_string::f_string_quoting;
use crate::other::f_string::FormatFString;
use crate::other::string_literal::{FormatStringLiteral, StringLiteralKind};
use crate::other::string_literal::StringLiteralKind;
use crate::prelude::*;
use crate::string::Quoting;

Expand Down Expand Up @@ -160,6 +160,7 @@ impl<'a> Iterator for AnyStringPartsIter<'a> {
match part {
ast::FStringPart::Literal(string_literal) => AnyStringPart::String {
part: string_literal,
#[allow(deprecated)]
layout: StringLiteralKind::InImplicitlyConcatenatedFString(*quoting),
},
ast::FStringPart::FString(f_string) => AnyStringPart::FString {
Expand Down Expand Up @@ -226,9 +227,7 @@ impl Ranged for AnyStringPart<'_> {
impl Format<PyFormatContext<'_>> for AnyStringPart<'_> {
fn fmt(&self, f: &mut PyFormatter) -> FormatResult<()> {
match self {
AnyStringPart::String { part, layout } => {
FormatStringLiteral::new(part, *layout).fmt(f)
}
AnyStringPart::String { part, layout } => part.format().with_options(*layout).fmt(f),
AnyStringPart::Bytes(bytes_literal) => bytes_literal.format().fmt(f),
AnyStringPart::FString { part, quoting } => FormatFString::new(part, *quoting).fmt(f),
}
Expand Down
Loading

0 comments on commit fc661e1

Please sign in to comment.