From 8488a4eb4f67a4bba8144f1f47c500c09e1a65e0 Mon Sep 17 00:00:00 2001 From: Brent Westbrook Date: Tue, 28 Jan 2025 13:34:59 -0500 Subject: [PATCH 01/13] remove effectively unused is_spec argument to unparse_f_string_value and document its use in unparse_f_string --- crates/ruff_python_codegen/src/generator.rs | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/crates/ruff_python_codegen/src/generator.rs b/crates/ruff_python_codegen/src/generator.rs index 308fc9c22d68dd..da71f1c0ee5756 100644 --- a/crates/ruff_python_codegen/src/generator.rs +++ b/crates/ruff_python_codegen/src/generator.rs @@ -1091,7 +1091,7 @@ impl<'a> Generator<'a> { self.p(")"); } Expr::FString(ast::ExprFString { value, .. }) => { - self.unparse_f_string_value(value, false); + self.unparse_f_string_value(value); } Expr::StringLiteral(ast::ExprStringLiteral { value, .. }) => { self.unparse_string_literal_value(value); @@ -1310,7 +1310,7 @@ impl<'a> Generator<'a> { } } - fn unparse_f_string_value(&mut self, value: &ast::FStringValue, is_spec: bool) { + fn unparse_f_string_value(&mut self, value: &ast::FStringValue) { let mut first = true; for f_string_part in value { self.p_delim(&mut first, " "); @@ -1319,7 +1319,7 @@ impl<'a> Generator<'a> { self.unparse_string_literal(string_literal); } ast::FStringPart::FString(f_string) => { - self.unparse_f_string(&f_string.elements, is_spec); + self.unparse_f_string(&f_string.elements, false); } } } @@ -1397,6 +1397,8 @@ impl<'a> Generator<'a> { self.p(&s); } + /// `is_spec` indicates whether we are inside of a format specifier (i.e. after the `:` in + /// `{var:...}`) fn unparse_f_string(&mut self, values: &[ast::FStringElement], is_spec: bool) { if is_spec { self.unparse_f_string_body(values); From 23ed586699b75750cc407196720537fa91a0250b Mon Sep 17 00:00:00 2001 From: Brent Westbrook Date: Tue, 28 Jan 2025 14:35:35 -0500 Subject: [PATCH 02/13] separate parse_f_string and parse_f_string_specifier and pass quote --- crates/ruff_python_codegen/src/generator.rs | 30 ++++++++++----------- 1 file changed, 15 insertions(+), 15 deletions(-) diff --git a/crates/ruff_python_codegen/src/generator.rs b/crates/ruff_python_codegen/src/generator.rs index da71f1c0ee5756..b564ccf42b81e3 100644 --- a/crates/ruff_python_codegen/src/generator.rs +++ b/crates/ruff_python_codegen/src/generator.rs @@ -1319,7 +1319,8 @@ impl<'a> Generator<'a> { self.unparse_string_literal(string_literal); } ast::FStringPart::FString(f_string) => { - self.unparse_f_string(&f_string.elements, false); + let quote = f_string.flags.quote_style(); + self.unparse_f_string(&f_string.elements, quote); } } } @@ -1366,7 +1367,7 @@ impl<'a> Generator<'a> { if let Some(spec) = spec { self.p(":"); - self.unparse_f_string(&spec.elements, true); + self.unparse_f_string_specifier(&spec.elements); } self.p("}"); @@ -1397,19 +1398,18 @@ impl<'a> Generator<'a> { self.p(&s); } - /// `is_spec` indicates whether we are inside of a format specifier (i.e. after the `:` in - /// `{var:...}`) - fn unparse_f_string(&mut self, values: &[ast::FStringElement], is_spec: bool) { - if is_spec { - self.unparse_f_string_body(values); - } else { - self.p("f"); - let mut generator = - Generator::new(self.indent, self.quote.opposite(), self.line_ending); - generator.unparse_f_string_body(values); - let body = &generator.buffer; - self.p_str_repr(body, self.quote); - } + fn unparse_f_string_specifier(&mut self, values: &[ast::FStringElement]) { + self.unparse_f_string_body(values); + } + + /// Unparse `values` with [`Generator::unparse_f_string_body`], using `quote` as the preferred + /// surrounding quote style. + fn unparse_f_string(&mut self, values: &[ast::FStringElement], quote: Quote) { + self.p("f"); + let mut generator = Generator::new(self.indent, self.quote.opposite(), self.line_ending); + generator.unparse_f_string_body(values); + let body = &generator.buffer; + self.p_str_repr(body, quote); } fn unparse_alias(&mut self, alias: &Alias) { From ed46f7ce2eb0482be0f5a4a8fe72186ae3bbcfa4 Mon Sep 17 00:00:00 2001 From: Brent Westbrook Date: Tue, 28 Jan 2025 14:46:17 -0500 Subject: [PATCH 03/13] avoid to_string --- crates/ruff_linter/src/rules/flynt/helpers.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/ruff_linter/src/rules/flynt/helpers.rs b/crates/ruff_linter/src/rules/flynt/helpers.rs index 640f922d6faa22..92f517397f9ecd 100644 --- a/crates/ruff_linter/src/rules/flynt/helpers.rs +++ b/crates/ruff_linter/src/rules/flynt/helpers.rs @@ -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: s.into(), range: TextRange::default(), }) } From aff34a50486ffe523796db9e86899604410e4f0f Mon Sep 17 00:00:00 2001 From: Brent Westbrook Date: Tue, 28 Jan 2025 14:46:47 -0500 Subject: [PATCH 04/13] use Checker's default quote style for new f-string in FLY002 --- .../src/rules/flynt/rules/static_join_to_fstring.rs | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/crates/ruff_linter/src/rules/flynt/rules/static_join_to_fstring.rs b/crates/ruff_linter/src/rules/flynt/rules/static_join_to_fstring.rs index 32cadc5851a398..baefd6e6fc4d10 100644 --- a/crates/ruff_linter/src/rules/flynt/rules/static_join_to_fstring.rs +++ b/crates/ruff_linter/src/rules/flynt/rules/static_join_to_fstring.rs @@ -1,10 +1,11 @@ use ast::FStringFlags; use itertools::Itertools; +use ruff_python_ast::str::Quote; use crate::fix::edits::pad; use ruff_diagnostics::{AlwaysFixableViolation, Diagnostic, Edit, Fix}; use ruff_macros::{derive_message_formats, ViolationMetadata}; -use ruff_python_ast::{self as ast, Arguments, Expr}; +use ruff_python_ast::{self as ast, Arguments, Expr, StringFlags}; use ruff_text_size::{Ranged, TextRange}; use crate::checkers::ast::Checker; @@ -60,7 +61,8 @@ fn is_static_length(elts: &[Expr]) -> bool { elts.iter().all(|e| !e.is_starred_expr()) } -fn build_fstring(joiner: &str, joinees: &[Expr]) -> Option { +/// Build an f-string consisting of `joinees` joined by `joiner` and surrounded by `quote`. +fn build_fstring(joiner: &str, joinees: &[Expr], quote: Quote) -> Option { // 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; @@ -104,7 +106,7 @@ fn build_fstring(joiner: &str, joinees: &[Expr]) -> Option { let node = ast::FString { elements: f_string_elements.into(), range: TextRange::default(), - flags: FStringFlags::default(), + flags: FStringFlags::default().with_quote_style(quote), }; Some(node.into()) } @@ -137,7 +139,8 @@ 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 quote = checker.default_string_flags().quote_style(); + let Some(new_expr) = build_fstring(joiner, joinees, quote) else { return; }; From 8a6208e66265a5f410dbfbd57d8783700d81917a Mon Sep 17 00:00:00 2001 From: Brent Westbrook Date: Tue, 28 Jan 2025 14:56:29 -0500 Subject: [PATCH 05/13] update Generator quote tests again --- crates/ruff_python_codegen/src/generator.rs | 13 ++++++------- 1 file changed, 6 insertions(+), 7 deletions(-) diff --git a/crates/ruff_python_codegen/src/generator.rs b/crates/ruff_python_codegen/src/generator.rs index b564ccf42b81e3..a55f57414a1f32 100644 --- a/crates/ruff_python_codegen/src/generator.rs +++ b/crates/ruff_python_codegen/src/generator.rs @@ -1813,19 +1813,18 @@ if True: }; } - // setting Generator::quote works for f-strings - round_trip_with!(Quote::Double, r#"f"hello""#, r#"f"hello""#); - round_trip_with!(Quote::Single, r#"f"hello""#, r"f'hello'"); - round_trip_with!(Quote::Double, r"f'hello'", r#"f"hello""#); - round_trip_with!(Quote::Single, r"f'hello'", r"f'hello'"); + // setting Generator::quote no longer works because the quote values are taken from the + // string literals themselves + round_trip_with!(Quote::Double, r#"f"hello""#); + round_trip_with!(Quote::Single, r#"f"hello""#); // no effect + round_trip_with!(Quote::Double, r"f'hello'"); // no effect + round_trip_with!(Quote::Single, r"f'hello'"); - // but not for bytestrings round_trip_with!(Quote::Double, r#"b"hello""#); round_trip_with!(Quote::Single, r#"b"hello""#); // no effect round_trip_with!(Quote::Double, r"b'hello'"); // no effect round_trip_with!(Quote::Single, r"b'hello'"); - // or for string literals, where the `Quote` is taken directly from their flags round_trip_with!(Quote::Double, r#""hello""#); round_trip_with!(Quote::Single, r#""hello""#); // no effect round_trip_with!(Quote::Double, r"'hello'"); // no effect From 25deb4fde127d9e648a8098289c69d2460fe846a Mon Sep 17 00:00:00 2001 From: Brent Westbrook Date: Tue, 28 Jan 2025 14:57:09 -0500 Subject: [PATCH 06/13] pass along the quote from Checker::default_string_flags for f-string in RUF030 --- .../rules/ruff/rules/assert_with_print_message.rs | 14 ++++++++------ 1 file changed, 8 insertions(+), 6 deletions(-) diff --git a/crates/ruff_linter/src/rules/ruff/rules/assert_with_print_message.rs b/crates/ruff_linter/src/rules/ruff/rules/assert_with_print_message.rs index 9e01401cf8022a..9420a48238d78e 100644 --- a/crates/ruff_linter/src/rules/ruff/rules/assert_with_print_message.rs +++ b/crates/ruff_linter/src/rules/ruff/rules/assert_with_print_message.rs @@ -90,9 +90,9 @@ pub(crate) fn assert_with_print_message(checker: &mut Checker, stmt: &ast::StmtA mod print_arguments { use itertools::Itertools; use ruff_python_ast::{ - Arguments, ConversionFlag, Expr, ExprFString, FString, FStringElement, FStringElements, - FStringExpressionElement, FStringFlags, FStringLiteralElement, FStringValue, StringLiteral, - StringLiteralFlags, + str::Quote, Arguments, ConversionFlag, Expr, ExprFString, FString, FStringElement, + FStringElements, FStringExpressionElement, FStringFlags, FStringLiteralElement, + FStringValue, StringFlags, StringLiteral, StringLiteralFlags, }; use ruff_text_size::TextRange; @@ -222,6 +222,7 @@ mod print_arguments { fn args_to_fstring_expr( mut args: impl ExactSizeIterator>, sep: impl ExactSizeIterator, + quote: Quote, ) -> Option { // If there are no arguments, short-circuit and return `None` let first_arg = args.next()?; @@ -236,7 +237,7 @@ mod print_arguments { Some(Expr::FString(ExprFString { value: FStringValue::single(FString { elements: FStringElements::from(fstring_elements), - flags: FStringFlags::default(), + flags: FStringFlags::default().with_quote_style(quote), range: TextRange::default(), }), range: TextRange::default(), @@ -286,7 +287,8 @@ 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(), flags).or_else(|| { + args_to_fstring_expr(args.into_iter(), sep.into_iter(), flags.quote_style()) + }) } } From 47d9ff290b63726c2492420b5a6c0c1a786d0a91 Mon Sep 17 00:00:00 2001 From: Brent Westbrook Date: Tue, 28 Jan 2025 15:06:10 -0500 Subject: [PATCH 07/13] delete now-unused Generator::quote field and related uses --- crates/ruff_linter/src/checkers/ast/mod.rs | 6 +- .../src/rules/flake8_type_checking/helpers.rs | 6 +- crates/ruff_python_codegen/src/generator.rs | 91 ++++++------------- 3 files changed, 29 insertions(+), 74 deletions(-) diff --git a/crates/ruff_linter/src/checkers/ast/mod.rs b/crates/ruff_linter/src/checkers/ast/mod.rs index 641f9575eb462d..77d178869fbf80 100644 --- a/crates/ruff_linter/src/checkers/ast/mod.rs +++ b/crates/ruff_linter/src/checkers/ast/mod.rs @@ -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 diff --git a/crates/ruff_linter/src/rules/flake8_type_checking/helpers.rs b/crates/ruff_linter/src/rules/flake8_type_checking/helpers.rs index e9193d1ae73008..40e24076632fea 100644 --- a/crates/ruff_linter/src/rules/flake8_type_checking/helpers.rs +++ b/crates/ruff_linter/src/rules/flake8_type_checking/helpers.rs @@ -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(), diff --git a/crates/ruff_python_codegen/src/generator.rs b/crates/ruff_python_codegen/src/generator.rs index a55f57414a1f32..dd15d7c2a7f0d1 100644 --- a/crates/ruff_python_codegen/src/generator.rs +++ b/crates/ruff_python_codegen/src/generator.rs @@ -65,11 +65,6 @@ mod precedence { pub struct Generator<'a> { /// The indentation style to use. indent: &'a Indentation, - /// The quote style to use for bytestring and f-string literals. For a plain - /// [`StringLiteral`](ast::StringLiteral), modify its `flags` field using - /// [`StringLiteralFlags::with_quote_style`](ast::StringLiteralFlags::with_quote_style) before - /// passing it to the [`Generator`]. - quote: Quote, /// The line ending to use. line_ending: LineEnding, buffer: String, @@ -82,7 +77,6 @@ impl<'a> From<&'a Stylist<'a>> for Generator<'a> { fn from(stylist: &'a Stylist<'a>) -> Self { Self { indent: stylist.indentation(), - quote: stylist.quote(), line_ending: stylist.line_ending(), buffer: String::new(), indent_depth: 0, @@ -93,11 +87,10 @@ impl<'a> From<&'a Stylist<'a>> for Generator<'a> { } impl<'a> Generator<'a> { - pub const fn new(indent: &'a Indentation, quote: Quote, line_ending: LineEnding) -> Self { + pub const fn new(indent: &'a Indentation, line_ending: LineEnding) -> Self { Self { // Style preferences. indent, - quote, line_ending, // Internal state. buffer: String::new(), @@ -1339,7 +1332,7 @@ impl<'a> Generator<'a> { conversion: ConversionFlag, spec: Option<&ast::FStringFormatSpec>, ) { - let mut generator = Generator::new(self.indent, self.quote, self.line_ending); + let mut generator = Generator::new(self.indent, self.line_ending); generator.unparse_expr(val, precedence::FORMATTED_VALUE); let brace = if generator.buffer.starts_with('{') { // put a space to avoid escaping the bracket @@ -1406,7 +1399,7 @@ impl<'a> Generator<'a> { /// surrounding quote style. fn unparse_f_string(&mut self, values: &[ast::FStringElement], quote: Quote) { self.p("f"); - let mut generator = Generator::new(self.indent, self.quote.opposite(), self.line_ending); + let mut generator = Generator::new(self.indent, self.line_ending); generator.unparse_f_string_body(values); let body = &generator.buffer; self.p_str_repr(body, quote); @@ -1431,7 +1424,7 @@ impl<'a> Generator<'a> { #[cfg(test)] mod tests { - use ruff_python_ast::{str::Quote, Mod, ModModule}; + use ruff_python_ast::{Mod, ModModule}; use ruff_python_parser::{self, parse_module, Mode}; use ruff_source_file::LineEnding; @@ -1441,47 +1434,28 @@ mod tests { fn round_trip(contents: &str) -> String { let indentation = Indentation::default(); - let quote = Quote::default(); let line_ending = LineEnding::default(); let module = parse_module(contents).unwrap(); - let mut generator = Generator::new(&indentation, quote, line_ending); + let mut generator = Generator::new(&indentation, line_ending); generator.unparse_suite(module.suite()); generator.generate() } - /// Like [`round_trip`] but configure the [`Generator`] with the requested `indentation`, - /// `quote`, and `line_ending` settings. - /// - /// Note that quoting styles for string literals are taken from their [`StringLiteralFlags`], - /// not from the [`Generator`] itself, so using this function on a plain string literal can give - /// surprising results. - /// - /// ```rust - /// assert_eq!( - /// round_trip_with( - /// &Indentation::default(), - /// Quote::Double, - /// LineEnding::default(), - /// r#"'hello'"# - /// ), - /// r#"'hello'"# - /// ); - /// ``` + /// Like [`round_trip`] but configure the [`Generator`] with the requested `indentation` and + /// `line_ending` settings. fn round_trip_with( indentation: &Indentation, - quote: Quote, line_ending: LineEnding, contents: &str, ) -> String { let module = parse_module(contents).unwrap(); - let mut generator = Generator::new(indentation, quote, line_ending); + let mut generator = Generator::new(indentation, line_ending); generator.unparse_suite(module.suite()); generator.generate() } fn jupyter_round_trip(contents: &str) -> String { let indentation = Indentation::default(); - let quote = Quote::default(); let line_ending = LineEnding::default(); let parsed = ruff_python_parser::parse(contents, Mode::Ipython).unwrap(); let Mod::Module(ModModule { body, .. }) = parsed.into_syntax() else { @@ -1490,7 +1464,7 @@ mod tests { let [stmt] = body.as_slice() else { panic!("Expected only one statement in source code") }; - let mut generator = Generator::new(&indentation, quote, line_ending); + let mut generator = Generator::new(&indentation, line_ending); generator.unparse_stmt(stmt); generator.generate() } @@ -1797,38 +1771,33 @@ if True: #[test] fn set_quote() { macro_rules! round_trip_with { - ($quote:expr, $start:expr, $end:expr) => { + ($start:expr, $end:expr) => { assert_eq!( - round_trip_with( - &Indentation::default(), - $quote, - LineEnding::default(), - $start - ), + round_trip_with(&Indentation::default(), LineEnding::default(), $start), $end, ); }; - ($quote:expr, $start:expr) => { - round_trip_with!($quote, $start, $start); + ($start:expr) => { + round_trip_with!($start, $start); }; } // setting Generator::quote no longer works because the quote values are taken from the // string literals themselves - round_trip_with!(Quote::Double, r#"f"hello""#); - round_trip_with!(Quote::Single, r#"f"hello""#); // no effect - round_trip_with!(Quote::Double, r"f'hello'"); // no effect - round_trip_with!(Quote::Single, r"f'hello'"); - - round_trip_with!(Quote::Double, r#"b"hello""#); - round_trip_with!(Quote::Single, r#"b"hello""#); // no effect - round_trip_with!(Quote::Double, r"b'hello'"); // no effect - round_trip_with!(Quote::Single, r"b'hello'"); - - round_trip_with!(Quote::Double, r#""hello""#); - round_trip_with!(Quote::Single, r#""hello""#); // no effect - round_trip_with!(Quote::Double, r"'hello'"); // no effect - round_trip_with!(Quote::Single, r"'hello'"); + round_trip_with!(r#"f"hello""#); + round_trip_with!(r#"f"hello""#); // no effect + round_trip_with!(r"f'hello'"); // no effect + round_trip_with!(r"f'hello'"); + + round_trip_with!(r#"b"hello""#); + round_trip_with!(r#"b"hello""#); // no effect + round_trip_with!(r"b'hello'"); // no effect + round_trip_with!(r"b'hello'"); + + round_trip_with!(r#""hello""#); + round_trip_with!(r#""hello""#); // no effect + round_trip_with!(r"'hello'"); // no effect + round_trip_with!(r"'hello'"); } #[test] @@ -1836,7 +1805,6 @@ if True: assert_eq!( round_trip_with( &Indentation::new(" ".to_string()), - Quote::default(), LineEnding::default(), r" if True: @@ -1854,7 +1822,6 @@ if True: assert_eq!( round_trip_with( &Indentation::new(" ".to_string()), - Quote::default(), LineEnding::default(), r" if True: @@ -1872,7 +1839,6 @@ if True: assert_eq!( round_trip_with( &Indentation::new("\t".to_string()), - Quote::default(), LineEnding::default(), r" if True: @@ -1894,7 +1860,6 @@ if True: assert_eq!( round_trip_with( &Indentation::default(), - Quote::default(), LineEnding::Lf, "if True:\n print(42)", ), @@ -1904,7 +1869,6 @@ if True: assert_eq!( round_trip_with( &Indentation::default(), - Quote::default(), LineEnding::CrLf, "if True:\n print(42)", ), @@ -1914,7 +1878,6 @@ if True: assert_eq!( round_trip_with( &Indentation::default(), - Quote::default(), LineEnding::Cr, "if True:\n print(42)", ), From 14426184d6c923a141d61b4ac44494b11ca84fd8 Mon Sep 17 00:00:00 2001 From: Brent Westbrook Date: Tue, 28 Jan 2025 15:09:13 -0500 Subject: [PATCH 08/13] macro is down to one case --- crates/ruff_python_codegen/src/generator.rs | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/crates/ruff_python_codegen/src/generator.rs b/crates/ruff_python_codegen/src/generator.rs index dd15d7c2a7f0d1..7a213d201c9658 100644 --- a/crates/ruff_python_codegen/src/generator.rs +++ b/crates/ruff_python_codegen/src/generator.rs @@ -1771,15 +1771,12 @@ if True: #[test] fn set_quote() { macro_rules! round_trip_with { - ($start:expr, $end:expr) => { + ($start:expr) => { assert_eq!( round_trip_with(&Indentation::default(), LineEnding::default(), $start), - $end, + $start, ); }; - ($start:expr) => { - round_trip_with!($start, $start); - }; } // setting Generator::quote no longer works because the quote values are taken from the From 9dade438ecf4ef8ef9a251c7556b9b6a86201764 Mon Sep 17 00:00:00 2001 From: Brent Westbrook Date: Tue, 28 Jan 2025 15:29:52 -0500 Subject: [PATCH 09/13] FStringFlags docs, no default, add Checker::default_fstring_flags --- crates/ruff_linter/src/checkers/ast/mod.rs | 6 ++++ .../flynt/rules/static_join_to_fstring.rs | 12 ++++---- .../ruff/rules/assert_with_print_message.rs | 28 +++++++++++-------- crates/ruff_python_ast/src/nodes.rs | 26 +++++++++++++++-- .../ruff_python_formatter/tests/normalizer.rs | 2 +- 5 files changed, 53 insertions(+), 21 deletions(-) diff --git a/crates/ruff_linter/src/checkers/ast/mod.rs b/crates/ruff_linter/src/checkers/ast/mod.rs index 77d178869fbf80..476b525638e482 100644 --- a/crates/ruff_linter/src/checkers/ast/mod.rs +++ b/crates/ruff_linter/src/checkers/ast/mod.rs @@ -315,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. /// diff --git a/crates/ruff_linter/src/rules/flynt/rules/static_join_to_fstring.rs b/crates/ruff_linter/src/rules/flynt/rules/static_join_to_fstring.rs index baefd6e6fc4d10..668559a2eac22c 100644 --- a/crates/ruff_linter/src/rules/flynt/rules/static_join_to_fstring.rs +++ b/crates/ruff_linter/src/rules/flynt/rules/static_join_to_fstring.rs @@ -1,11 +1,10 @@ use ast::FStringFlags; use itertools::Itertools; -use ruff_python_ast::str::Quote; use crate::fix::edits::pad; use ruff_diagnostics::{AlwaysFixableViolation, Diagnostic, Edit, Fix}; use ruff_macros::{derive_message_formats, ViolationMetadata}; -use ruff_python_ast::{self as ast, Arguments, Expr, StringFlags}; +use ruff_python_ast::{self as ast, Arguments, Expr, }; use ruff_text_size::{Ranged, TextRange}; use crate::checkers::ast::Checker; @@ -61,8 +60,8 @@ fn is_static_length(elts: &[Expr]) -> bool { elts.iter().all(|e| !e.is_starred_expr()) } -/// Build an f-string consisting of `joinees` joined by `joiner` and surrounded by `quote`. -fn build_fstring(joiner: &str, joinees: &[Expr], quote: Quote) -> Option { +/// Build an f-string consisting of `joinees` joined by `joiner` with `flags`. +fn build_fstring(joiner: &str, joinees: &[Expr], flags: FStringFlags) -> Option { // 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; @@ -106,7 +105,7 @@ fn build_fstring(joiner: &str, joinees: &[Expr], quote: Quote) -> Option { let node = ast::FString { elements: f_string_elements.into(), range: TextRange::default(), - flags: FStringFlags::default().with_quote_style(quote), + flags, }; Some(node.into()) } @@ -139,8 +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 quote = checker.default_string_flags().quote_style(); - let Some(new_expr) = build_fstring(joiner, joinees, quote) else { + let Some(new_expr) = build_fstring(joiner, joinees, checker.default_fstring_flags()) else { return; }; diff --git a/crates/ruff_linter/src/rules/ruff/rules/assert_with_print_message.rs b/crates/ruff_linter/src/rules/ruff/rules/assert_with_print_message.rs index 9420a48238d78e..b8b730e0ada32b 100644 --- a/crates/ruff_linter/src/rules/ruff/rules/assert_with_print_message.rs +++ b/crates/ruff_linter/src/rules/ruff/rules/assert_with_print_message.rs @@ -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, @@ -90,12 +89,14 @@ pub(crate) fn assert_with_print_message(checker: &mut Checker, stmt: &ast::StmtA mod print_arguments { use itertools::Itertools; use ruff_python_ast::{ - str::Quote, Arguments, ConversionFlag, Expr, ExprFString, FString, FStringElement, - FStringElements, FStringExpressionElement, FStringFlags, FStringLiteralElement, - FStringValue, StringFlags, StringLiteral, StringLiteralFlags, + Arguments, ConversionFlag, Expr, ExprFString, FString, FStringElement, FStringElements, + FStringExpressionElement, FStringFlags, FStringLiteralElement, FStringValue, StringLiteral, + StringLiteralFlags, }; use ruff_text_size::TextRange; + use crate::checkers::ast::Checker; + /// Converts an expression to a list of `FStringElement`s. /// /// Three cases are handled: @@ -222,7 +223,7 @@ mod print_arguments { fn args_to_fstring_expr( mut args: impl ExactSizeIterator>, sep: impl ExactSizeIterator, - quote: Quote, + flags: FStringFlags, ) -> Option { // If there are no arguments, short-circuit and return `None` let first_arg = args.next()?; @@ -237,7 +238,7 @@ mod print_arguments { Some(Expr::FString(ExprFString { value: FStringValue::single(FString { elements: FStringElements::from(fstring_elements), - flags: FStringFlags::default().with_quote_style(quote), + flags, range: TextRange::default(), }), range: TextRange::default(), @@ -257,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 { + pub(super) fn to_expr(arguments: &Arguments, checker: &Checker) -> Option { // Convert the `sep` argument into `FStringElement`s let sep = arguments .find_keyword("sep") @@ -287,8 +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(), flags.quote_style()) - }) + 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(), + ) + }) } } diff --git a/crates/ruff_python_ast/src/nodes.rs b/crates/ruff_python_ast/src/nodes.rs index 894ad649e8bf97..3727e43cd70d25 100644 --- a/crates/ruff_python_ast/src/nodes.rs +++ b/crates/ruff_python_ast/src/nodes.rs @@ -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 @@ -2229,7 +2251,7 @@ impl From 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() { diff --git a/crates/ruff_python_formatter/tests/normalizer.rs b/crates/ruff_python_formatter/tests/normalizer.rs index c10ed93720b73b..a2fac515cf33d9 100644 --- a/crates/ruff_python_formatter/tests/normalizer.rs +++ b/crates/ruff_python_formatter/tests/normalizer.rs @@ -181,7 +181,7 @@ impl Transformer for Normalizer { fstring.value = ast::FStringValue::single(ast::FString { elements: collector.elements.into(), range: fstring.range, - flags: FStringFlags::default(), + flags: FStringFlags::empty(), }); } } From 6bfebce9caea3f84e65c69c620898745017ebfc4 Mon Sep 17 00:00:00 2001 From: Brent Westbrook Date: Tue, 28 Jan 2025 15:52:49 -0500 Subject: [PATCH 10/13] tidy up --- .../src/rules/flynt/rules/static_join_to_fstring.rs | 2 +- crates/ruff_python_codegen/src/generator.rs | 3 +-- 2 files changed, 2 insertions(+), 3 deletions(-) diff --git a/crates/ruff_linter/src/rules/flynt/rules/static_join_to_fstring.rs b/crates/ruff_linter/src/rules/flynt/rules/static_join_to_fstring.rs index 668559a2eac22c..eec40c57a08d5c 100644 --- a/crates/ruff_linter/src/rules/flynt/rules/static_join_to_fstring.rs +++ b/crates/ruff_linter/src/rules/flynt/rules/static_join_to_fstring.rs @@ -4,7 +4,7 @@ use itertools::Itertools; use crate::fix::edits::pad; use ruff_diagnostics::{AlwaysFixableViolation, Diagnostic, Edit, Fix}; use ruff_macros::{derive_message_formats, ViolationMetadata}; -use ruff_python_ast::{self as ast, Arguments, Expr, }; +use ruff_python_ast::{self as ast, Arguments, Expr}; use ruff_text_size::{Ranged, TextRange}; use crate::checkers::ast::Checker; diff --git a/crates/ruff_python_codegen/src/generator.rs b/crates/ruff_python_codegen/src/generator.rs index 7a213d201c9658..4d648070a71976 100644 --- a/crates/ruff_python_codegen/src/generator.rs +++ b/crates/ruff_python_codegen/src/generator.rs @@ -1312,8 +1312,7 @@ impl<'a> Generator<'a> { self.unparse_string_literal(string_literal); } ast::FStringPart::FString(f_string) => { - let quote = f_string.flags.quote_style(); - self.unparse_f_string(&f_string.elements, quote); + self.unparse_f_string(&f_string.elements, f_string.flags.quote_style()); } } } From cb474df228d7da996df95afe9938812def751b50 Mon Sep 17 00:00:00 2001 From: Brent Westbrook Date: Tue, 28 Jan 2025 16:32:00 -0500 Subject: [PATCH 11/13] Box::from over into Co-authored-by: Alex Waygood --- crates/ruff_linter/src/rules/flynt/helpers.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/ruff_linter/src/rules/flynt/helpers.rs b/crates/ruff_linter/src/rules/flynt/helpers.rs index 92f517397f9ecd..a71b369b6987f4 100644 --- a/crates/ruff_linter/src/rules/flynt/helpers.rs +++ b/crates/ruff_linter/src/rules/flynt/helpers.rs @@ -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.into(), + value: Box::from(s), range: TextRange::default(), }) } From d9ee56cbbb31f987b788fbb6d9b4851a96fa8702 Mon Sep 17 00:00:00 2001 From: Brent Westbrook Date: Wed, 29 Jan 2025 09:36:17 -0500 Subject: [PATCH 12/13] combine deduplicated set_quote tests with quote tests --- crates/ruff_python_codegen/src/generator.rs | 32 ++------------------- 1 file changed, 3 insertions(+), 29 deletions(-) diff --git a/crates/ruff_python_codegen/src/generator.rs b/crates/ruff_python_codegen/src/generator.rs index 4d648070a71976..587e8052199086 100644 --- a/crates/ruff_python_codegen/src/generator.rs +++ b/crates/ruff_python_codegen/src/generator.rs @@ -1719,6 +1719,9 @@ class Foo: assert_round_trip!(r"u'hello'"); assert_round_trip!(r"r'hello'"); assert_round_trip!(r"b'hello'"); + assert_round_trip!(r#"b"hello""#); + assert_round_trip!(r"f'hello'"); + assert_round_trip!(r#"f"hello""#); assert_eq!(round_trip(r#"("abc" "def" "ghi")"#), r#""abc" "def" "ghi""#); assert_eq!(round_trip(r#""he\"llo""#), r#"'he"llo'"#); assert_eq!(round_trip(r#"f"abc{'def'}{1}""#), r#"f"abc{'def'}{1}""#); @@ -1767,35 +1770,6 @@ if True: ); } - #[test] - fn set_quote() { - macro_rules! round_trip_with { - ($start:expr) => { - assert_eq!( - round_trip_with(&Indentation::default(), LineEnding::default(), $start), - $start, - ); - }; - } - - // setting Generator::quote no longer works because the quote values are taken from the - // string literals themselves - round_trip_with!(r#"f"hello""#); - round_trip_with!(r#"f"hello""#); // no effect - round_trip_with!(r"f'hello'"); // no effect - round_trip_with!(r"f'hello'"); - - round_trip_with!(r#"b"hello""#); - round_trip_with!(r#"b"hello""#); // no effect - round_trip_with!(r"b'hello'"); // no effect - round_trip_with!(r"b'hello'"); - - round_trip_with!(r#""hello""#); - round_trip_with!(r#""hello""#); // no effect - round_trip_with!(r"'hello'"); // no effect - round_trip_with!(r"'hello'"); - } - #[test] fn set_indent() { assert_eq!( From ca8f5f0ef99d8409fef5576673bcf3dfe4139ed9 Mon Sep 17 00:00:00 2001 From: Brent Westbrook Date: Wed, 29 Jan 2025 09:42:41 -0500 Subject: [PATCH 13/13] add SIM108 test for preserving quotes --- .../test/fixtures/flake8_simplify/SIM108.py | 14 +++++ ...ke8_simplify__tests__SIM108_SIM108.py.snap | 52 +++++++++++++++++++ ...ify__tests__preview__SIM108_SIM108.py.snap | 52 +++++++++++++++++++ 3 files changed, 118 insertions(+) diff --git a/crates/ruff_linter/resources/test/fixtures/flake8_simplify/SIM108.py b/crates/ruff_linter/resources/test/fixtures/flake8_simplify/SIM108.py index 25991d478233d5..52da2ecd37e2bb 100644 --- a/crates/ruff_linter/resources/test/fixtures/flake8_simplify/SIM108.py +++ b/crates/ruff_linter/resources/test/fixtures/flake8_simplify/SIM108.py @@ -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}' diff --git a/crates/ruff_linter/src/rules/flake8_simplify/snapshots/ruff_linter__rules__flake8_simplify__tests__SIM108_SIM108.py.snap b/crates/ruff_linter/src/rules/flake8_simplify/snapshots/ruff_linter__rules__flake8_simplify__tests__SIM108_SIM108.py.snap index de1dcf780b6dab..3306802cce8707 100644 --- a/crates/ruff_linter/src/rules/flake8_simplify/snapshots/ruff_linter__rules__flake8_simplify__tests__SIM108_SIM108.py.snap +++ b/crates/ruff_linter/src/rules/flake8_simplify/snapshots/ruff_linter__rules__flake8_simplify__tests__SIM108_SIM108.py.snap @@ -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}' diff --git a/crates/ruff_linter/src/rules/flake8_simplify/snapshots/ruff_linter__rules__flake8_simplify__tests__preview__SIM108_SIM108.py.snap b/crates/ruff_linter/src/rules/flake8_simplify/snapshots/ruff_linter__rules__flake8_simplify__tests__preview__SIM108_SIM108.py.snap index 723cf6fa667687..6b72655b2f8027 100644 --- a/crates/ruff_linter/src/rules/flake8_simplify/snapshots/ruff_linter__rules__flake8_simplify__tests__preview__SIM108_SIM108.py.snap +++ b/crates/ruff_linter/src/rules/flake8_simplify/snapshots/ruff_linter__rules__flake8_simplify__tests__preview__SIM108_SIM108.py.snap @@ -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}'