From 443fd3b660b8bd03c07f4c29cf39512cdaab577a Mon Sep 17 00:00:00 2001 From: Micha Reiser Date: Sun, 3 Nov 2024 12:49:26 +0100 Subject: [PATCH] Disallow single-line implicit concatenated strings (#13928) --- .../test/fixtures/ruff/expression/string.py | 57 ++++ .../src/statement/stmt_assign.rs | 10 +- .../src/string/implicit.rs | 42 ++- ...@cases__long_strings_flag_disabled.py.snap | 1 + ...bility@cases__preview_long_strings.py.snap | 1 + ...__preview_long_strings__regression.py.snap | 1 + .../format@expression__fstring.py.snap | 1 + .../format@expression__string.py.snap | 258 ++++++++++++++++++ 8 files changed, 364 insertions(+), 7 deletions(-) diff --git a/crates/ruff_python_formatter/resources/test/fixtures/ruff/expression/string.py b/crates/ruff_python_formatter/resources/test/fixtures/ruff/expression/string.py index ce01296d5f7e7..cc559e1ab9a86 100644 --- a/crates/ruff_python_formatter/resources/test/fixtures/ruff/expression/string.py +++ b/crates/ruff_python_formatter/resources/test/fixtures/ruff/expression/string.py @@ -138,3 +138,60 @@ a = """\x1F""" a = """\\x1F""" a = """\\\x1F""" + + +############################################################################## +# Implicit concatenated strings +############################################################################## + +# In preview, don't collapse implicit concatenated strings that can't be joined into a single string +# and that are multiline in the source. + +( + r"aaaaaaaaa" + r"bbbbbbbbbbbbbbbbbbbb" +) + +( + r"aaaaaaaaa" r"bbbbbbbbbbbbbbbbbbbb" + "cccccccccccccccccccccc" +) + +( + """aaaaaaaaa""" + """bbbbbbbbbbbbbbbbbbbb""" +) + +( + f"""aaaa{ + 10}aaaaa""" + fr"""bbbbbbbbbbbbbbbbbbbb""" +) + +if ( + r"aaaaaaaaa" + r"bbbbbbbbbbbbbbbbbbbb" + ) + ["aaaaaaaaaa", "bbbbbbbbbbbbbbbb"]: ... + + + +# In preview, keep implicit concatenated strings on a single line if they fit and are not separate by line breaks in the source +( + r"aaaaaaaaa" r"bbbbbbbbbbbbbbbbbbbb" +) + +r"aaaaaaaaa" r"bbbbbbbbbbbbbbbbbbbb" + +( + f"aaaa{ + 10}aaaaa" fr"bbbbbbbbbbbbbbbbbbbb" +) + +( + r"""aaaaaaaaa""" r"""bbbbbbbbbbbbbbbbbbbb""" +) + +( + f"""aaaa{ + 10}aaaaa""" fr"""bbbbbbbbbbbbbbbbbbbb""" +) diff --git a/crates/ruff_python_formatter/src/statement/stmt_assign.rs b/crates/ruff_python_formatter/src/statement/stmt_assign.rs index f568d2a9756cb..7ad90cd025db7 100644 --- a/crates/ruff_python_formatter/src/statement/stmt_assign.rs +++ b/crates/ruff_python_formatter/src/statement/stmt_assign.rs @@ -20,6 +20,7 @@ use crate::preview::is_join_implicit_concatenated_string_enabled; use crate::statement::trailing_semicolon; use crate::string::implicit::{ FormatImplicitConcatenatedStringExpanded, FormatImplicitConcatenatedStringFlat, + ImplicitConcatenatedLayout, }; use crate::{has_skip_comment, prelude::*}; @@ -375,7 +376,13 @@ impl Format> for FormatStatementsLastExpression<'_> { let f = &mut WithNodeLevel::new(NodeLevel::Expression(Some(group_id)), f); - write!(f, [FormatImplicitConcatenatedStringExpanded::new(string)]) + write!( + f, + [FormatImplicitConcatenatedStringExpanded::new( + string, + ImplicitConcatenatedLayout::MaybeFlat + )] + ) }); // Join the implicit concatenated string if it fits on a single line @@ -686,6 +693,7 @@ impl Format> for FormatStatementsLastExpression<'_> { FormatImplicitConcatenatedStringExpanded::new( StringLike::try_from(*value).unwrap(), + ImplicitConcatenatedLayout::MaybeFlat, ) .fmt(f) }) diff --git a/crates/ruff_python_formatter/src/string/implicit.rs b/crates/ruff_python_formatter/src/string/implicit.rs index 2f057a49d8481..b103dc545a70d 100644 --- a/crates/ruff_python_formatter/src/string/implicit.rs +++ b/crates/ruff_python_formatter/src/string/implicit.rs @@ -1,5 +1,4 @@ -use std::borrow::Cow; - +use itertools::Itertools; use ruff_formatter::{format_args, write, FormatContext}; use ruff_python_ast::str::Quote; use ruff_python_ast::str_prefix::{ @@ -8,6 +7,7 @@ use ruff_python_ast::str_prefix::{ use ruff_python_ast::{AnyStringFlags, FStringElement, StringFlags, StringLike, StringLikePart}; use ruff_source_file::LineRanges; use ruff_text_size::{Ranged, TextRange}; +use std::borrow::Cow; use crate::comments::{leading_comments, trailing_comments}; use crate::expression::parentheses::in_parentheses_only_soft_line_break_or_space; @@ -41,12 +41,20 @@ impl<'a> FormatImplicitConcatenatedString<'a> { impl Format> for FormatImplicitConcatenatedString<'_> { fn fmt(&self, f: &mut PyFormatter) -> FormatResult<()> { - let expanded = FormatImplicitConcatenatedStringExpanded::new(self.string); + let flat = FormatImplicitConcatenatedStringFlat::new(self.string, f.context()); + let expanded = FormatImplicitConcatenatedStringExpanded::new( + self.string, + if flat.is_some() { + ImplicitConcatenatedLayout::MaybeFlat + } else { + ImplicitConcatenatedLayout::Multipart + }, + ); // If the string can be joined, try joining the implicit concatenated string into a single string // if it fits on the line. Otherwise, parenthesize the string parts and format each part on its // own line. - if let Some(flat) = FormatImplicitConcatenatedStringFlat::new(self.string, f.context()) { + if let Some(flat) = flat { write!( f, [if_group_fits_on_line(&flat), if_group_breaks(&expanded)] @@ -60,13 +68,14 @@ impl Format> for FormatImplicitConcatenatedString<'_> { /// Formats an implicit concatenated string where parts are separated by a space or line break. pub(crate) struct FormatImplicitConcatenatedStringExpanded<'a> { string: StringLike<'a>, + layout: ImplicitConcatenatedLayout, } impl<'a> FormatImplicitConcatenatedStringExpanded<'a> { - pub(crate) fn new(string: StringLike<'a>) -> Self { + pub(crate) fn new(string: StringLike<'a>, layout: ImplicitConcatenatedLayout) -> Self { assert!(string.is_implicit_concatenated()); - Self { string } + Self { string, layout } } } @@ -77,6 +86,19 @@ impl Format> for FormatImplicitConcatenatedStringExpanded<'_ let join_implicit_concatenated_string_enabled = is_join_implicit_concatenated_string_enabled(f.context()); + + // Keep implicit concatenated strings expanded unless they're already written on a single line. + if matches!(self.layout, ImplicitConcatenatedLayout::Multipart) + && join_implicit_concatenated_string_enabled + && self.string.parts().tuple_windows().any(|(a, b)| { + f.context() + .source() + .contains_line_break(TextRange::new(a.end(), b.start())) + }) + { + expand_parent().fmt(f)?; + } + let mut joiner = f.join_with(in_parentheses_only_soft_line_break_or_space()); for part in self.string.parts() { @@ -108,6 +130,14 @@ impl Format> for FormatImplicitConcatenatedStringExpanded<'_ } } +#[derive(Copy, Clone, Debug)] +pub(crate) enum ImplicitConcatenatedLayout { + /// The string might get joined into a single string if it fits on a single line. + MaybeFlat, + /// The string will remain a multipart string. + Multipart, +} + /// Formats an implicit concatenated string where parts are joined into a single string if possible. pub(crate) struct FormatImplicitConcatenatedStringFlat<'a> { string: StringLike<'a>, diff --git a/crates/ruff_python_formatter/tests/snapshots/black_compatibility@cases__long_strings_flag_disabled.py.snap b/crates/ruff_python_formatter/tests/snapshots/black_compatibility@cases__long_strings_flag_disabled.py.snap index 97505058f4bf0..4435d34dcb11a 100644 --- a/crates/ruff_python_formatter/tests/snapshots/black_compatibility@cases__long_strings_flag_disabled.py.snap +++ b/crates/ruff_python_formatter/tests/snapshots/black_compatibility@cases__long_strings_flag_disabled.py.snap @@ -1,6 +1,7 @@ --- source: crates/ruff_python_formatter/tests/fixtures.rs input_file: crates/ruff_python_formatter/resources/test/fixtures/black/cases/long_strings_flag_disabled.py +snapshot_kind: text --- ## Input diff --git a/crates/ruff_python_formatter/tests/snapshots/black_compatibility@cases__preview_long_strings.py.snap b/crates/ruff_python_formatter/tests/snapshots/black_compatibility@cases__preview_long_strings.py.snap index fa0173187df9c..3b7b47cb747b9 100644 --- a/crates/ruff_python_formatter/tests/snapshots/black_compatibility@cases__preview_long_strings.py.snap +++ b/crates/ruff_python_formatter/tests/snapshots/black_compatibility@cases__preview_long_strings.py.snap @@ -1,6 +1,7 @@ --- source: crates/ruff_python_formatter/tests/fixtures.rs input_file: crates/ruff_python_formatter/resources/test/fixtures/black/cases/preview_long_strings.py +snapshot_kind: text --- ## Input diff --git a/crates/ruff_python_formatter/tests/snapshots/black_compatibility@cases__preview_long_strings__regression.py.snap b/crates/ruff_python_formatter/tests/snapshots/black_compatibility@cases__preview_long_strings__regression.py.snap index fc3857c2b45b2..7f61408c3cf01 100644 --- a/crates/ruff_python_formatter/tests/snapshots/black_compatibility@cases__preview_long_strings__regression.py.snap +++ b/crates/ruff_python_formatter/tests/snapshots/black_compatibility@cases__preview_long_strings__regression.py.snap @@ -1,6 +1,7 @@ --- source: crates/ruff_python_formatter/tests/fixtures.rs input_file: crates/ruff_python_formatter/resources/test/fixtures/black/cases/preview_long_strings__regression.py +snapshot_kind: text --- ## Input diff --git a/crates/ruff_python_formatter/tests/snapshots/format@expression__fstring.py.snap b/crates/ruff_python_formatter/tests/snapshots/format@expression__fstring.py.snap index af2a324aa36a0..2d00c181189ef 100644 --- a/crates/ruff_python_formatter/tests/snapshots/format@expression__fstring.py.snap +++ b/crates/ruff_python_formatter/tests/snapshots/format@expression__fstring.py.snap @@ -1,6 +1,7 @@ --- source: crates/ruff_python_formatter/tests/fixtures.rs input_file: crates/ruff_python_formatter/resources/test/fixtures/ruff/expression/fstring.py +snapshot_kind: text --- ## Input ```python diff --git a/crates/ruff_python_formatter/tests/snapshots/format@expression__string.py.snap b/crates/ruff_python_formatter/tests/snapshots/format@expression__string.py.snap index 3303732eb9434..8ecc488b8da7a 100644 --- a/crates/ruff_python_formatter/tests/snapshots/format@expression__string.py.snap +++ b/crates/ruff_python_formatter/tests/snapshots/format@expression__string.py.snap @@ -1,6 +1,7 @@ --- source: crates/ruff_python_formatter/tests/fixtures.rs input_file: crates/ruff_python_formatter/resources/test/fixtures/ruff/expression/string.py +snapshot_kind: text --- ## Input ```python @@ -144,6 +145,63 @@ a = f"""\x1F""" a = """\x1F""" a = """\\x1F""" a = """\\\x1F""" + + +############################################################################## +# Implicit concatenated strings +############################################################################## + +# In preview, don't collapse implicit concatenated strings that can't be joined into a single string +# and that are multiline in the source. + +( + r"aaaaaaaaa" + r"bbbbbbbbbbbbbbbbbbbb" +) + +( + r"aaaaaaaaa" r"bbbbbbbbbbbbbbbbbbbb" + "cccccccccccccccccccccc" +) + +( + """aaaaaaaaa""" + """bbbbbbbbbbbbbbbbbbbb""" +) + +( + f"""aaaa{ + 10}aaaaa""" + fr"""bbbbbbbbbbbbbbbbbbbb""" +) + +if ( + r"aaaaaaaaa" + r"bbbbbbbbbbbbbbbbbbbb" + ) + ["aaaaaaaaaa", "bbbbbbbbbbbbbbbb"]: ... + + + +# In preview, keep implicit concatenated strings on a single line if they fit and are not separate by line breaks in the source +( + r"aaaaaaaaa" r"bbbbbbbbbbbbbbbbbbbb" +) + +r"aaaaaaaaa" r"bbbbbbbbbbbbbbbbbbbb" + +( + f"aaaa{ + 10}aaaaa" fr"bbbbbbbbbbbbbbbbbbbb" +) + +( + r"""aaaaaaaaa""" r"""bbbbbbbbbbbbbbbbbbbb""" +) + +( + f"""aaaa{ + 10}aaaaa""" fr"""bbbbbbbbbbbbbbbbbbbb""" +) ``` ## Outputs @@ -328,6 +386,49 @@ a = f"""\x1f""" a = """\x1f""" a = """\\x1F""" a = """\\\x1f""" + + +############################################################################## +# Implicit concatenated strings +############################################################################## + +# In preview, don't collapse implicit concatenated strings that can't be joined into a single string +# and that are multiline in the source. + +(r"aaaaaaaaa" r"bbbbbbbbbbbbbbbbbbbb") + +(r"aaaaaaaaa" r"bbbbbbbbbbbbbbbbbbbb" "cccccccccccccccccccccc") + +("""aaaaaaaaa""" """bbbbbbbbbbbbbbbbbbbb""") + +( + f"""aaaa{ + 10}aaaaa""" + rf"""bbbbbbbbbbbbbbbbbbbb""" +) + +if (r"aaaaaaaaa" r"bbbbbbbbbbbbbbbbbbbb") + ["aaaaaaaaaa", "bbbbbbbbbbbbbbbb"]: + ... + + +# In preview, keep implicit concatenated strings on a single line if they fit and are not separate by line breaks in the source +(r"aaaaaaaaa" r"bbbbbbbbbbbbbbbbbbbb") + +r"aaaaaaaaa" r"bbbbbbbbbbbbbbbbbbbb" + +( + f"aaaa{ + 10}aaaaa" + rf"bbbbbbbbbbbbbbbbbbbb" +) + +(r"""aaaaaaaaa""" r"""bbbbbbbbbbbbbbbbbbbb""") + +( + f"""aaaa{ + 10}aaaaa""" + rf"""bbbbbbbbbbbbbbbbbbbb""" +) ``` @@ -356,6 +457,63 @@ a = """\\\x1f""" # Regression test for https://github.com/astral-sh/ruff/issues/5893 +@@ -172,19 +172,31 @@ + # In preview, don't collapse implicit concatenated strings that can't be joined into a single string + # and that are multiline in the source. + +-(r"aaaaaaaaa" r"bbbbbbbbbbbbbbbbbbbb") ++( ++ r"aaaaaaaaa" ++ r"bbbbbbbbbbbbbbbbbbbb" ++) + +-(r"aaaaaaaaa" r"bbbbbbbbbbbbbbbbbbbb" "cccccccccccccccccccccc") ++( ++ r"aaaaaaaaa" ++ r"bbbbbbbbbbbbbbbbbbbb" ++ "cccccccccccccccccccccc" ++) + +-("""aaaaaaaaa""" """bbbbbbbbbbbbbbbbbbbb""") ++( ++ """aaaaaaaaa""" ++ """bbbbbbbbbbbbbbbbbbbb""" ++) + + ( +- f"""aaaa{ +- 10}aaaaa""" ++ f"""aaaa{10}aaaaa""" + rf"""bbbbbbbbbbbbbbbbbbbb""" + ) + +-if (r"aaaaaaaaa" r"bbbbbbbbbbbbbbbbbbbb") + ["aaaaaaaaaa", "bbbbbbbbbbbbbbbb"]: ++if ( ++ r"aaaaaaaaa" ++ r"bbbbbbbbbbbbbbbbbbbb" ++) + ["aaaaaaaaaa", "bbbbbbbbbbbbbbbb"]: + ... + + +@@ -193,16 +205,8 @@ + + r"aaaaaaaaa" r"bbbbbbbbbbbbbbbbbbbb" + +-( +- f"aaaa{ +- 10}aaaaa" +- rf"bbbbbbbbbbbbbbbbbbbb" +-) ++(f"aaaa{10}aaaaa" rf"bbbbbbbbbbbbbbbbbbbb") + + (r"""aaaaaaaaa""" r"""bbbbbbbbbbbbbbbbbbbb""") + +-( +- f"""aaaa{ +- 10}aaaaa""" +- rf"""bbbbbbbbbbbbbbbbbbbb""" +-) ++(f"""aaaa{10}aaaaa""" rf"""bbbbbbbbbbbbbbbbbbbb""") ``` @@ -540,6 +698,49 @@ a = f"""\x1f""" a = """\x1f""" a = """\\x1F""" a = """\\\x1f""" + + +############################################################################## +# Implicit concatenated strings +############################################################################## + +# In preview, don't collapse implicit concatenated strings that can't be joined into a single string +# and that are multiline in the source. + +(r'aaaaaaaaa' r'bbbbbbbbbbbbbbbbbbbb') + +(r'aaaaaaaaa' r'bbbbbbbbbbbbbbbbbbbb' 'cccccccccccccccccccccc') + +("""aaaaaaaaa""" """bbbbbbbbbbbbbbbbbbbb""") + +( + f"""aaaa{ + 10}aaaaa""" + rf"""bbbbbbbbbbbbbbbbbbbb""" +) + +if (r'aaaaaaaaa' r'bbbbbbbbbbbbbbbbbbbb') + ['aaaaaaaaaa', 'bbbbbbbbbbbbbbbb']: + ... + + +# In preview, keep implicit concatenated strings on a single line if they fit and are not separate by line breaks in the source +(r'aaaaaaaaa' r'bbbbbbbbbbbbbbbbbbbb') + +r'aaaaaaaaa' r'bbbbbbbbbbbbbbbbbbbb' + +( + f'aaaa{ + 10}aaaaa' + rf'bbbbbbbbbbbbbbbbbbbb' +) + +(r"""aaaaaaaaa""" r"""bbbbbbbbbbbbbbbbbbbb""") + +( + f"""aaaa{ + 10}aaaaa""" + rf"""bbbbbbbbbbbbbbbbbbbb""" +) ``` @@ -568,4 +769,61 @@ a = """\\\x1f""" # Regression test for https://github.com/astral-sh/ruff/issues/5893 +@@ -172,19 +172,31 @@ + # In preview, don't collapse implicit concatenated strings that can't be joined into a single string + # and that are multiline in the source. + +-(r'aaaaaaaaa' r'bbbbbbbbbbbbbbbbbbbb') ++( ++ r'aaaaaaaaa' ++ r'bbbbbbbbbbbbbbbbbbbb' ++) + +-(r'aaaaaaaaa' r'bbbbbbbbbbbbbbbbbbbb' 'cccccccccccccccccccccc') ++( ++ r'aaaaaaaaa' ++ r'bbbbbbbbbbbbbbbbbbbb' ++ 'cccccccccccccccccccccc' ++) + +-("""aaaaaaaaa""" """bbbbbbbbbbbbbbbbbbbb""") ++( ++ """aaaaaaaaa""" ++ """bbbbbbbbbbbbbbbbbbbb""" ++) + + ( +- f"""aaaa{ +- 10}aaaaa""" ++ f"""aaaa{10}aaaaa""" + rf"""bbbbbbbbbbbbbbbbbbbb""" + ) + +-if (r'aaaaaaaaa' r'bbbbbbbbbbbbbbbbbbbb') + ['aaaaaaaaaa', 'bbbbbbbbbbbbbbbb']: ++if ( ++ r'aaaaaaaaa' ++ r'bbbbbbbbbbbbbbbbbbbb' ++) + ['aaaaaaaaaa', 'bbbbbbbbbbbbbbbb']: + ... + + +@@ -193,16 +205,8 @@ + + r'aaaaaaaaa' r'bbbbbbbbbbbbbbbbbbbb' + +-( +- f'aaaa{ +- 10}aaaaa' +- rf'bbbbbbbbbbbbbbbbbbbb' +-) ++(f'aaaa{10}aaaaa' rf'bbbbbbbbbbbbbbbbbbbb') + + (r"""aaaaaaaaa""" r"""bbbbbbbbbbbbbbbbbbbb""") + +-( +- f"""aaaa{ +- 10}aaaaa""" +- rf"""bbbbbbbbbbbbbbbbbbbb""" +-) ++(f"""aaaa{10}aaaaa""" rf"""bbbbbbbbbbbbbbbbbbbb""") ```