From b4b5d67a4a4cc7193b4867dcc748c8399571eaf8 Mon Sep 17 00:00:00 2001 From: Takayuki Maeda Date: Fri, 19 Sep 2025 02:18:29 +0900 Subject: [PATCH] [`flynt`] Use triple quotes for joined raw strings with newlines (`FLY002`) (#20197) ## Summary Fixes #19887 - flynt(FLY002): When joining only string constants, upgrade raw single-quoted strings to raw triple-quoted if the resulting content contains a newline. - Choose a safe triple-quote delimiter by switching to the opposite quote style if the preferred triple appears inside the content. - Update FLY002 snapshot to include the `\n'.join([r'line1','line2'])` case. ## Test Plan I've added one test case to FLY002.py. --------- Co-authored-by: Brent Westbrook <36778786+ntBre@users.noreply.github.com> --- .../resources/test/fixtures/flynt/FLY002.py | 4 +- .../flynt/rules/static_join_to_fstring.rs | 54 ++++++++++++------- ...rules__flynt__tests__FLY002_FLY002.py.snap | 39 ++++++++++---- 3 files changed, 69 insertions(+), 28 deletions(-) diff --git a/crates/ruff_linter/resources/test/fixtures/flynt/FLY002.py b/crates/ruff_linter/resources/test/fixtures/flynt/FLY002.py index a73dfcc28a..2252d09741 100644 --- a/crates/ruff_linter/resources/test/fixtures/flynt/FLY002.py +++ b/crates/ruff_linter/resources/test/fixtures/flynt/FLY002.py @@ -16,7 +16,9 @@ nok4 = "a".join([a, a, *a]) # Not OK (not a static length) nok5 = "a".join([choice("flarp")]) # Not OK (not a simple call) nok6 = "a".join(x for x in "feefoofum") # Not OK (generator) nok7 = "a".join([f"foo{8}", "bar"]) # Not OK (contains an f-string) - +# https://github.com/astral-sh/ruff/issues/19887 +nok8 = '\n'.join([r'line1','line2']) +nok9 = '\n'.join([r"raw string", '<""">', "<'''>"]) # Not OK (both triple-quote delimiters appear; should bail) # Regression test for: https://github.com/astral-sh/ruff/issues/7197 def create_file_public_url(url, filename): 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 a385b910ec..5ddf21dc49 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 @@ -2,7 +2,7 @@ use ast::FStringFlags; use itertools::Itertools; use ruff_macros::{ViolationMetadata, derive_message_formats}; -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; @@ -72,24 +72,42 @@ fn is_static_length(elts: &[Expr]) -> bool { 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; - let node = ast::StringLiteral { - value: joinees - .iter() - .filter_map(|expr| { - if let Expr::StringLiteral(ast::ExprStringLiteral { value, .. }) = expr { - if flags.is_none() { - // take the flags from the first Expr - flags = Some(value.first_literal_flags()); - } - Some(value.to_str()) - } else { - None + let mut flags: Option = None; + let content = joinees + .iter() + .filter_map(|expr| { + if let Expr::StringLiteral(ast::ExprStringLiteral { value, .. }) = expr { + if flags.is_none() { + // Take the flags from the first Expr + flags = Some(value.first_literal_flags()); } - }) - .join(joiner) - .into_boxed_str(), - flags: flags?, + Some(value.to_str()) + } else { + None + } + }) + .join(joiner); + + let mut flags = flags?; + + // If the result is a raw string and contains a newline, use triple quotes. + if flags.prefix().is_raw() && content.contains(['\n', '\r']) { + flags = flags.with_triple_quotes(ruff_python_ast::str::TripleQuotes::Yes); + + // Prefer a delimiter that doesn't occur in the content; if both occur, bail. + if content.contains(flags.quote_str()) { + flags = flags.with_quote_style(flags.quote_style().opposite()); + if content.contains(flags.quote_str()) { + // Both "'''" and "\"\"\"" are present in content; avoid emitting + // an invalid raw triple-quoted literal (or escaping). Bail on the fix. + return None; + } + } + } + + let node = ast::StringLiteral { + value: content.into_boxed_str(), + flags, range: TextRange::default(), node_index: ruff_python_ast::AtomicNodeIndex::NONE, }; diff --git a/crates/ruff_linter/src/rules/flynt/snapshots/ruff_linter__rules__flynt__tests__FLY002_FLY002.py.snap b/crates/ruff_linter/src/rules/flynt/snapshots/ruff_linter__rules__flynt__tests__FLY002_FLY002.py.snap index e951d9e721..79d5496910 100644 --- a/crates/ruff_linter/src/rules/flynt/snapshots/ruff_linter__rules__flynt__tests__FLY002_FLY002.py.snap +++ b/crates/ruff_linter/src/rules/flynt/snapshots/ruff_linter__rules__flynt__tests__FLY002_FLY002.py.snap @@ -125,18 +125,39 @@ help: Replace with `f"{secrets.token_urlsafe()}a{secrets.token_hex()}"` 13 | nok2 = a.join(["1", "2", "3"]) # Not OK (not a static joiner) note: This is an unsafe fix and may change runtime behavior -FLY002 [*] Consider `f"{url}{filename}"` instead of string join - --> FLY002.py:23:11 +FLY002 [*] Consider f-string instead of string join + --> FLY002.py:20:8 | -21 | # Regression test for: https://github.com/astral-sh/ruff/issues/7197 -22 | def create_file_public_url(url, filename): -23 | return''.join([url, filename]) +18 | nok7 = "a".join([f"foo{8}", "bar"]) # Not OK (contains an f-string) +19 | # https://github.com/astral-sh/ruff/issues/19887 +20 | nok8 = '\n'.join([r'line1','line2']) + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ +21 | nok9 = '\n'.join([r"raw string", '<""">', "<'''>"]) # Not OK (both triple-quote delimiters appear; should bail) + | +help: Replace with f-string +17 | nok6 = "a".join(x for x in "feefoofum") # Not OK (generator) +18 | nok7 = "a".join([f"foo{8}", "bar"]) # Not OK (contains an f-string) +19 | # https://github.com/astral-sh/ruff/issues/19887 + - nok8 = '\n'.join([r'line1','line2']) +20 + nok8 = r'''line1 +21 + line2''' +22 | nok9 = '\n'.join([r"raw string", '<""">', "<'''>"]) # Not OK (both triple-quote delimiters appear; should bail) +23 | +24 | # Regression test for: https://github.com/astral-sh/ruff/issues/7197 +note: This is an unsafe fix and may change runtime behavior + +FLY002 [*] Consider `f"{url}{filename}"` instead of string join + --> FLY002.py:25:11 + | +23 | # Regression test for: https://github.com/astral-sh/ruff/issues/7197 +24 | def create_file_public_url(url, filename): +25 | return''.join([url, filename]) | ^^^^^^^^^^^^^^^^^^^^^^^^ | help: Replace with `f"{url}{filename}"` -20 | -21 | # Regression test for: https://github.com/astral-sh/ruff/issues/7197 -22 | def create_file_public_url(url, filename): +22 | +23 | # Regression test for: https://github.com/astral-sh/ruff/issues/7197 +24 | def create_file_public_url(url, filename): - return''.join([url, filename]) -23 + return f"{url}{filename}" +25 + return f"{url}{filename}" note: This is an unsafe fix and may change runtime behavior