Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Normalize implicit concatenated f-string quotes per part #13539

Merged
merged 4 commits into from
Oct 8, 2024
Merged
Show file tree
Hide file tree
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion crates/ruff_python_formatter/generate.py
Original file line number Diff line number Diff line change
Expand Up @@ -38,9 +38,10 @@ def rustfmt(code: str) -> str:
# implementation.
MichaReiser marked this conversation as resolved.
Show resolved Hide resolved
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.'
)
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(
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I created a new preview style for now in case we push out the f-string stabilization. We can merge this change with the f-string preview style if we decide to ship both changes.

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
Loading