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

Preserve quotes in generated f-strings #15794

Merged
merged 13 commits into from
Jan 29, 2025
Merged
Original file line number Diff line number Diff line change
Expand Up @@ -194,3 +194,17 @@ def f():
z = not foo()
else:
z = other


# These two cases double as tests for f-string quote preservation. The first
# f-string should preserve its double quotes, and the second should preserve
# single quotes
if cond:
var = "str"
else:
var = f"{first}-{second}"

if cond:
var = "str"
else:
var = f'{first}-{second}'
12 changes: 7 additions & 5 deletions crates/ruff_linter/src/checkers/ast/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -294,11 +294,7 @@ impl<'a> Checker<'a> {

/// Create a [`Generator`] to generate source code based on the current AST state.
pub(crate) fn generator(&self) -> Generator {
Generator::new(
self.stylist.indentation(),
self.preferred_quote(),
self.stylist.line_ending(),
)
Generator::new(self.stylist.indentation(), self.stylist.line_ending())
}

/// Return the preferred quote for a generated `StringLiteral` node, given where we are in the
Expand All @@ -319,6 +315,12 @@ impl<'a> Checker<'a> {
ast::BytesLiteralFlags::empty().with_quote_style(self.preferred_quote())
}

/// Return the default f-string flags a generated `FString` node should use, given where we are
/// in the AST.
pub(crate) fn default_fstring_flags(&self) -> ast::FStringFlags {
ast::FStringFlags::empty().with_quote_style(self.preferred_quote())
}

/// Returns the appropriate quoting for f-string by reversing the one used outside of
/// the f-string.
///
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -301,3 +301,55 @@ SIM108.py:193:1: SIM108 [*] Use ternary operator `z = not foo() if foo() else ot
195 |-else:
196 |- z = other
193 |+z = not foo() if foo() else other
197 194 |
198 195 |
199 196 | # These two cases double as tests for f-string quote preservation. The first

SIM108.py:202:1: SIM108 [*] Use ternary operator `var = "str" if cond else f"{first}-{second}"` instead of `if`-`else`-block
|
200 | # f-string should preserve its double quotes, and the second should preserve
201 | # single quotes
202 | / if cond:
203 | | var = "str"
204 | | else:
205 | | var = f"{first}-{second}"
| |_____________________________^ SIM108
206 |
207 | if cond:
|
= help: Replace `if`-`else`-block with `var = "str" if cond else f"{first}-{second}"`

ℹ Unsafe fix
199 199 | # These two cases double as tests for f-string quote preservation. The first
200 200 | # f-string should preserve its double quotes, and the second should preserve
201 201 | # single quotes
202 |-if cond:
203 |- var = "str"
204 |-else:
205 |- var = f"{first}-{second}"
202 |+var = "str" if cond else f"{first}-{second}"
206 203 |
207 204 | if cond:
208 205 | var = "str"

SIM108.py:207:1: SIM108 [*] Use ternary operator `var = "str" if cond else f'{first}-{second}'` instead of `if`-`else`-block
|
205 | var = f"{first}-{second}"
206 |
207 | / if cond:
208 | | var = "str"
209 | | else:
210 | | var = f'{first}-{second}'
| |_____________________________^ SIM108
|
= help: Replace `if`-`else`-block with `var = "str" if cond else f'{first}-{second}'`

ℹ Unsafe fix
204 204 | else:
205 205 | var = f"{first}-{second}"
206 206 |
207 |-if cond:
208 |- var = "str"
209 |-else:
210 |- var = f'{first}-{second}'
207 |+var = "str" if cond else f'{first}-{second}'
Original file line number Diff line number Diff line change
Expand Up @@ -301,3 +301,55 @@ SIM108.py:193:1: SIM108 [*] Use ternary operator `z = not foo() if foo() else ot
195 |-else:
196 |- z = other
193 |+z = not foo() if foo() else other
197 194 |
198 195 |
199 196 | # These two cases double as tests for f-string quote preservation. The first

SIM108.py:202:1: SIM108 [*] Use ternary operator `var = "str" if cond else f"{first}-{second}"` instead of `if`-`else`-block
|
200 | # f-string should preserve its double quotes, and the second should preserve
201 | # single quotes
202 | / if cond:
203 | | var = "str"
204 | | else:
205 | | var = f"{first}-{second}"
| |_____________________________^ SIM108
206 |
207 | if cond:
|
= help: Replace `if`-`else`-block with `var = "str" if cond else f"{first}-{second}"`

ℹ Unsafe fix
199 199 | # These two cases double as tests for f-string quote preservation. The first
200 200 | # f-string should preserve its double quotes, and the second should preserve
201 201 | # single quotes
202 |-if cond:
203 |- var = "str"
204 |-else:
205 |- var = f"{first}-{second}"
202 |+var = "str" if cond else f"{first}-{second}"
206 203 |
207 204 | if cond:
208 205 | var = "str"

SIM108.py:207:1: SIM108 [*] Use ternary operator `var = "str" if cond else f'{first}-{second}'` instead of `if`-`else`-block
|
205 | var = f"{first}-{second}"
206 |
207 | / if cond:
208 | | var = "str"
209 | | else:
210 | | var = f'{first}-{second}'
| |_____________________________^ SIM108
|
= help: Replace `if`-`else`-block with `var = "str" if cond else f'{first}-{second}'`

ℹ Unsafe fix
204 204 | else:
205 205 | var = f"{first}-{second}"
206 206 |
207 |-if cond:
208 |- var = "str"
209 |-else:
210 |- var = f'{first}-{second}'
207 |+var = "str" if cond else f'{first}-{second}'
6 changes: 1 addition & 5 deletions crates/ruff_linter/src/rules/flake8_type_checking/helpers.rs
Original file line number Diff line number Diff line change
Expand Up @@ -363,11 +363,7 @@ impl<'a> QuoteAnnotator<'a> {
let generator = Generator::from(self.stylist);
// we first generate the annotation with the inverse quote, so we can
// generate the string literal with the preferred quote
let subgenerator = Generator::new(
self.stylist.indentation(),
self.stylist.quote().opposite(),
self.stylist.line_ending(),
);
let subgenerator = Generator::new(self.stylist.indentation(), self.stylist.line_ending());
let annotation = subgenerator.expr(&expr_without_forward_references);
generator.expr(&Expr::from(ast::StringLiteral {
range: TextRange::default(),
Expand Down
2 changes: 1 addition & 1 deletion crates/ruff_linter/src/rules/flynt/helpers.rs
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ fn to_f_string_expression_element(inner: &Expr) -> ast::FStringElement {
/// Convert a string to a [`ast::FStringElement::Literal`].
pub(super) fn to_f_string_literal_element(s: &str) -> ast::FStringElement {
ast::FStringElement::Literal(ast::FStringLiteralElement {
value: s.to_string().into_boxed_str(),
value: Box::from(s),
range: TextRange::default(),
})
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,8 @@ fn is_static_length(elts: &[Expr]) -> bool {
elts.iter().all(|e| !e.is_starred_expr())
}

fn build_fstring(joiner: &str, joinees: &[Expr]) -> Option<Expr> {
/// Build an f-string consisting of `joinees` joined by `joiner` with `flags`.
fn build_fstring(joiner: &str, joinees: &[Expr], flags: FStringFlags) -> Option<Expr> {
// If all elements are string constants, join them into a single string.
if joinees.iter().all(Expr::is_string_literal_expr) {
let mut flags = None;
Expand Down Expand Up @@ -104,7 +105,7 @@ fn build_fstring(joiner: &str, joinees: &[Expr]) -> Option<Expr> {
let node = ast::FString {
elements: f_string_elements.into(),
range: TextRange::default(),
flags: FStringFlags::default(),
flags,
};
Some(node.into())
}
Expand Down Expand Up @@ -137,7 +138,7 @@ pub(crate) fn static_join_to_fstring(checker: &mut Checker, expr: &Expr, joiner:

// Try to build the fstring (internally checks whether e.g. the elements are
// convertible to f-string elements).
let Some(new_expr) = build_fstring(joiner, joinees) else {
let Some(new_expr) = build_fstring(joiner, joinees, checker.default_fstring_flags()) else {
return;
};

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -66,8 +66,7 @@ pub(crate) fn assert_with_print_message(checker: &mut Checker, stmt: &ast::StmtA
diagnostic.set_fix(Fix::unsafe_edit(Edit::range_replacement(
checker.generator().stmt(&Stmt::Assert(ast::StmtAssert {
test: stmt.test.clone(),
msg: print_arguments::to_expr(&call.arguments, checker.default_string_flags())
.map(Box::new),
msg: print_arguments::to_expr(&call.arguments, checker).map(Box::new),
range: TextRange::default(),
})),
// We have to replace the entire statement,
Expand Down Expand Up @@ -96,6 +95,8 @@ mod print_arguments {
};
use ruff_text_size::TextRange;

use crate::checkers::ast::Checker;

/// Converts an expression to a list of `FStringElement`s.
///
/// Three cases are handled:
Expand Down Expand Up @@ -222,6 +223,7 @@ mod print_arguments {
fn args_to_fstring_expr(
mut args: impl ExactSizeIterator<Item = Vec<FStringElement>>,
sep: impl ExactSizeIterator<Item = FStringElement>,
flags: FStringFlags,
) -> Option<Expr> {
// If there are no arguments, short-circuit and return `None`
let first_arg = args.next()?;
Expand All @@ -236,7 +238,7 @@ mod print_arguments {
Some(Expr::FString(ExprFString {
value: FStringValue::single(FString {
elements: FStringElements::from(fstring_elements),
flags: FStringFlags::default(),
flags,
range: TextRange::default(),
}),
range: TextRange::default(),
Expand All @@ -256,7 +258,7 @@ mod print_arguments {
/// - [`Some`]<[`Expr::StringLiteral`]> if all arguments including `sep` are string literals.
/// - [`Some`]<[`Expr::FString`]> if any of the arguments are not string literals.
/// - [`None`] if the `print` contains no positional arguments at all.
pub(super) fn to_expr(arguments: &Arguments, flags: StringLiteralFlags) -> Option<Expr> {
pub(super) fn to_expr(arguments: &Arguments, checker: &Checker) -> Option<Expr> {
// Convert the `sep` argument into `FStringElement`s
let sep = arguments
.find_keyword("sep")
Expand Down Expand Up @@ -286,7 +288,13 @@ mod print_arguments {

// Attempt to convert the `sep` and `args` arguments to a string literal,
// falling back to an f-string if the arguments are not all string literals.
args_to_string_literal_expr(args.iter(), sep.iter(), flags)
.or_else(|| args_to_fstring_expr(args.into_iter(), sep.into_iter()))
args_to_string_literal_expr(args.iter(), sep.iter(), checker.default_string_flags())
.or_else(|| {
args_to_fstring_expr(
args.into_iter(),
sep.into_iter(),
checker.default_fstring_flags(),
)
})
}
}
26 changes: 24 additions & 2 deletions crates/ruff_python_ast/src/nodes.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1063,10 +1063,32 @@ bitflags! {

/// Flags that can be queried to obtain information
/// regarding the prefixes and quotes used for an f-string.
#[derive(Default, Copy, Clone, Eq, PartialEq, Hash)]
///
/// ## Notes on usage
///
/// If you're using a `Generator` from the `ruff_python_codegen` crate to generate a lint-rule fix
/// from an existing f-string literal, consider passing along the [`FString::flags`] field. If you
/// don't have an existing literal but have a `Checker` from the `ruff_linter` crate available,
/// consider using `Checker::default_fstring_flags` to create instances of this struct; this method
/// will properly handle nested f-strings. For usage that doesn't fit into one of these categories,
/// the public constructor [`FStringFlags::empty`] can be used.
#[derive(Copy, Clone, Eq, PartialEq, Hash)]
pub struct FStringFlags(FStringFlagsInner);

impl FStringFlags {
/// Construct a new [`FStringFlags`] with **no flags set**.
///
/// See [`FStringFlags::with_quote_style`], [`FStringFlags::with_triple_quotes`], and
/// [`FStringFlags::with_prefix`] for ways of setting the quote style (single or double),
/// enabling triple quotes, and adding prefixes (such as `r`), respectively.
///
/// See the documentation for [`FStringFlags`] for additional caveats on this constructor, and
/// situations in which alternative ways to construct this struct should be used, especially
/// when writing lint rules.
pub fn empty() -> Self {
Self(FStringFlagsInner::empty())
}

#[must_use]
pub fn with_quote_style(mut self, quote_style: Quote) -> Self {
self.0
Expand Down Expand Up @@ -2229,7 +2251,7 @@ impl From<AnyStringFlags> for FStringFlags {
value.prefix()
)
};
let new = FStringFlags::default()
let new = FStringFlags::empty()
.with_quote_style(value.quote_style())
.with_prefix(fstring_prefix);
if value.is_triple_quoted() {
Expand Down
Loading
Loading