From 07de8f999735d944501381a9ba299453cf9eee2e Mon Sep 17 00:00:00 2001 From: Micha Reiser Date: Fri, 6 Dec 2024 11:21:39 +0100 Subject: [PATCH] Fix fstring formatting removing overlong implicit concatenated string in expression part --- crates/ruff_formatter/src/buffer.rs | 31 +++++++++++-------- .../test/fixtures/ruff/expression/fstring.py | 4 +++ .../format@expression__fstring.py.snap | 27 ++++++++++++++-- 3 files changed, 47 insertions(+), 15 deletions(-) diff --git a/crates/ruff_formatter/src/buffer.rs b/crates/ruff_formatter/src/buffer.rs index b96dc7bb08da02..b376fcf8eb1d6f 100644 --- a/crates/ruff_formatter/src/buffer.rs +++ b/crates/ruff_formatter/src/buffer.rs @@ -299,7 +299,8 @@ where /// /// - Removes [`lines`](FormatElement::Line) with the mode [`Soft`](LineMode::Soft). /// - Replaces [`lines`](FormatElement::Line) with the mode [`Soft`](LineMode::SoftOrSpace) with a [`Space`](FormatElement::Space) -/// - Removes [`if_group_breaks`](crate::builders::if_group_breaks) elements. +/// - Removes [`if_group_breaks`](crate::builders::if_group_breaks) and all its content. +/// - Unwraps the content of [`if_group_fits_on_line`](crate::builders::if_group_fits_on_line) elements (but retains it). /// /// # Examples /// @@ -387,7 +388,7 @@ fn clean_interned( .iter() .enumerate() .find_map(|(index, element)| match element { - FormatElement::Line(LineMode::Soft | LineMode::SoftOrSpace) => { + FormatElement::Line(LineMode::SoftOrSpace) => { let mut cleaned = Vec::new(); let (before, after) = interned.split_at(index); cleaned.extend_from_slice(before); @@ -427,7 +428,6 @@ fn clean_interned( } let element = match element { - FormatElement::Line(LineMode::Soft) => continue, FormatElement::Line(LineMode::SoftOrSpace) => FormatElement::Space, FormatElement::Interned(interned) => { FormatElement::Interned(clean_interned(interned, interned_cache)) @@ -458,7 +458,6 @@ impl Buffer for RemoveSoftLinesBuffer<'_, Context> { } let element = match element { - FormatElement::Line(LineMode::Soft) => return, FormatElement::Line(LineMode::SoftOrSpace) => FormatElement::Space, FormatElement::Interned(interned) => { FormatElement::Interned(self.clean_interned(&interned)) @@ -508,28 +507,34 @@ enum RemoveSoftLineBreaksState { impl RemoveSoftLineBreaksState { fn should_drop(&mut self, element: &FormatElement) -> bool { match self { - Self::Default => { - // Entered the start of an `if_group_breaks` - if let FormatElement::Tag(Tag::StartConditionalContent(condition)) = element { + Self::Default => match element { + FormatElement::Line(LineMode::Soft) => true, + + // Entered the start of an `if_group_breaks` or `if_group_fits` + // For `if_group_breaks`: Remove the start and end tag and all content in between. + // For `if_group_fits_on_line`: Unwrap the content. This is important because the enclosing group + // might still *expand* if the content exceeds the line width limit, in which case the + // `if_group_fits_on_line` content would be removed. + FormatElement::Tag(Tag::StartConditionalContent(condition)) => { if condition.mode.is_expanded() { *self = Self::InIfGroupBreaks { conditional_content_level: NonZeroUsize::new(1).unwrap(), }; - return true; } + true } - - false - } + FormatElement::Tag(Tag::EndConditionalContent) => true, + _ => false, + }, Self::InIfGroupBreaks { conditional_content_level, } => { match element { - // A nested `if_group_breaks` or `if_group_fits` + // A nested `if_group_breaks` or `if_group_fits_on_line` FormatElement::Tag(Tag::StartConditionalContent(_)) => { *conditional_content_level = conditional_content_level.saturating_add(1); } - // The end of an `if_group_breaks` or `if_group_fits`. + // The end of an `if_group_breaks` or `if_group_fits_on_line`. FormatElement::Tag(Tag::EndConditionalContent) => { if let Some(level) = NonZeroUsize::new(conditional_content_level.get() - 1) { diff --git a/crates/ruff_python_formatter/resources/test/fixtures/ruff/expression/fstring.py b/crates/ruff_python_formatter/resources/test/fixtures/ruff/expression/fstring.py index 35001e91ce9708..1b0d11ce330b9f 100644 --- a/crates/ruff_python_formatter/resources/test/fixtures/ruff/expression/fstring.py +++ b/crates/ruff_python_formatter/resources/test/fixtures/ruff/expression/fstring.py @@ -665,6 +665,10 @@ # Regression test for https://github.com/astral-sh/ruff/issues/14487 f"aaaaaaaaaaaaaaaaaaaaaaaaaa {10**27} bbbbbbbbbbbbbbbbbbbbbbbbbb ccccccccccccccccccccccccc" +# Regression test for https://github.com/astral-sh/ruff/issues/14778 +f"{'aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa' 'a' if True else ""}" +f"{'aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa' 'a' if True else ""}" + # Quotes reuse f"{'a'}" 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 11293d7dde179e..94838c61ec0045 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 @@ -671,6 +672,10 @@ _ = ( # Regression test for https://github.com/astral-sh/ruff/issues/14487 f"aaaaaaaaaaaaaaaaaaaaaaaaaa {10**27} bbbbbbbbbbbbbbbbbbbbbbbbbb ccccccccccccccccccccccccc" +# Regression test for https://github.com/astral-sh/ruff/issues/14778 +f"{'aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa' 'a' if True else ""}" +f"{'aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa' 'a' if True else ""}" + # Quotes reuse f"{'a'}" @@ -1441,6 +1446,10 @@ _ = ( # Regression test for https://github.com/astral-sh/ruff/issues/14487 f"aaaaaaaaaaaaaaaaaaaaaaaaaa {10**27} bbbbbbbbbbbbbbbbbbbbbbbbbb ccccccccccccccccccccccccc" +# Regression test for https://github.com/astral-sh/ruff/issues/14778 +f"{'aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa' if True else ''}" +f"{'aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa' if True else ''}" + # Quotes reuse f"{'a'}" @@ -2163,6 +2172,10 @@ _ = ( # Regression test for https://github.com/astral-sh/ruff/issues/14487 f"aaaaaaaaaaaaaaaaaaaaaaaaaa {10**27} bbbbbbbbbbbbbbbbbbbbbbbbbb ccccccccccccccccccccccccc" +# Regression test for https://github.com/astral-sh/ruff/issues/14778 +f"{'aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa' 'a' if True else ""}" +f"{'aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa' 'a' if True else ""}" + # Quotes reuse f"{'a'}" @@ -2993,7 +3006,7 @@ f'{1=: {'ab"cd"'}}' # It's okay if the quotes are in an expression part. ) # Indentation -@@ -632,29 +680,29 @@ +@@ -632,37 +680,37 @@ if indent2: foo = f"""hello world hello { @@ -3041,7 +3054,17 @@ f'{1=: {'ab"cd"'}}' # It's okay if the quotes are in an expression part. ) # Regression test for https://github.com/astral-sh/ruff/issues/14487 -@@ -678,18 +726,18 @@ + f"aaaaaaaaaaaaaaaaaaaaaaaaaa {10**27} bbbbbbbbbbbbbbbbbbbbbbbbbb ccccccccccccccccccccccccc" + + # Regression test for https://github.com/astral-sh/ruff/issues/14778 +-f"{'aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa' 'a' if True else ""}" +-f"{'aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa' 'a' if True else ""}" ++f"{'aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa' if True else ''}" ++f"{'aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa' if True else ''}" + + # Quotes reuse + f"{'a'}" +@@ -682,18 +730,18 @@ f'{1: hy "user"}' f'{1:hy "user"}' f'{1: abcd "{1}" }'