From 37dc06f8d535e84ec923b3d4ab9a5f9766ac6582 Mon Sep 17 00:00:00 2001 From: Micha Reiser Date: Wed, 10 Jul 2024 22:47:31 +0200 Subject: [PATCH] Use `space` separator between `if` and `(` in comprehensions --- .../fixtures/ruff/expression/list_comp.py | 63 +++++++ .../src/other/comprehension.rs | 48 +++++- crates/ruff_python_formatter/src/preview.rs | 7 + .../format@expression__list_comp.py.snap | 162 +++++++++++++++++- 4 files changed, 274 insertions(+), 6 deletions(-) diff --git a/crates/ruff_python_formatter/resources/test/fixtures/ruff/expression/list_comp.py b/crates/ruff_python_formatter/resources/test/fixtures/ruff/expression/list_comp.py index 85038eb74e045..0f3f4af199626 100644 --- a/crates/ruff_python_formatter/resources/test/fixtures/ruff/expression/list_comp.py +++ b/crates/ruff_python_formatter/resources/test/fixtures/ruff/expression/list_comp.py @@ -105,3 +105,66 @@ # Parenthesized targets and iterators. [x for (x) in y] [x for x in (y)] + + +# Leading expression comments: +y = [ + a + for ( + # comment + a + ) in ( + # comment + x + ) + if ( + # asdasd + "askldaklsdnmklasmdlkasmdlkasmdlkasmdasd" + != "as,mdnaskldmlkasdmlaksdmlkasdlkasdm" + and "zxcm,.nzxclm,zxnckmnzxckmnzxczxc" != "zxcasdasdlmnasdlknaslkdnmlaskdm" + ) + if ( + # comment + x + ) +] + +# Tuple target: +y = [ + a + for + # comment + a, b + in x + if True +] + + +y = [ + a + for ( + # comment + a, b + ) + in x + if True +] + + +y = [ + a + for + # comment + a + in + # comment + x + if + # asdasd + "askldaklsdnmklasmdlkasmdlkasmdlkasmdasd" + != "as,mdnaskldmlkasdmlaksdmlkasdlkasdm" + and "zxcm,.nzxclm,zxnckmnzxckmnzxczxc" != "zxcasdasdlmnasdlknaslkdnmlaskdm" + if + # comment + x +] diff --git a/crates/ruff_python_formatter/src/other/comprehension.rs b/crates/ruff_python_formatter/src/other/comprehension.rs index 2ae4be783b517..106cf1957cc6c 100644 --- a/crates/ruff_python_formatter/src/other/comprehension.rs +++ b/crates/ruff_python_formatter/src/other/comprehension.rs @@ -5,18 +5,47 @@ use ruff_text_size::{Ranged, TextRange}; use crate::comments::{leading_comments, trailing_comments}; use crate::expression::expr_tuple::TupleParentheses; +use crate::expression::parentheses::is_expression_parenthesized; use crate::prelude::*; +use crate::preview::is_comprehension_leading_expression_comments_same_line_enabled; #[derive(Default)] pub struct FormatComprehension; impl FormatNodeRule for FormatComprehension { fn fmt_fields(&self, item: &Comprehension, f: &mut PyFormatter) -> FormatResult<()> { - struct Spacer<'a>(&'a Expr); + struct Spacer<'a> { + expression: &'a Expr, + preserve_parentheses: bool, + } impl Format> for Spacer<'_> { fn fmt(&self, f: &mut PyFormatter) -> FormatResult<()> { - if f.context().comments().has_leading(self.0) { + let has_leading_comments = f.context().comments().has_leading(self.expression); + + // Don't add a soft line break for parenthesized expressions with a leading comment. + // The comments are rendered **inside** the parentheses and adding a softline break + // unnecessarily forces the parentheses to be on their own line. + // ```python + // y = [ + // ... + // if + // ( + // # See how the `(` gets forced on its own line? We don't want that. + // ... + // ) + // ] + // ``` + let will_be_parenthesized = + is_comprehension_leading_expression_comments_same_line_enabled(f.context()) + && self.preserve_parentheses + && is_expression_parenthesized( + self.expression.into(), + f.context().comments().ranges(), + f.context().source(), + ); + + if has_leading_comments && !will_be_parenthesized { soft_line_break_or_space().fmt(f) } else { space().fmt(f) @@ -68,13 +97,19 @@ impl FormatNodeRule for FormatComprehension { [ token("for"), trailing_comments(before_target_comments), - Spacer(target), + Spacer { + expression: target, + preserve_parentheses: !target.is_tuple_expr() + }, ExprTupleWithoutParentheses(target), in_spacer, leading_comments(before_in_comments), token("in"), trailing_comments(trailing_in_comments), - Spacer(iter), + Spacer { + expression: iter, + preserve_parentheses: true + }, iter.format(), ] )?; @@ -99,7 +134,10 @@ impl FormatNodeRule for FormatComprehension { leading_comments(own_line_if_comments), token("if"), trailing_comments(end_of_line_if_comments), - Spacer(if_case), + Spacer { + expression: if_case, + preserve_parentheses: true + }, if_case.format(), )); diff --git a/crates/ruff_python_formatter/src/preview.rs b/crates/ruff_python_formatter/src/preview.rs index a403e4a8011d4..261906e2b61b5 100644 --- a/crates/ruff_python_formatter/src/preview.rs +++ b/crates/ruff_python_formatter/src/preview.rs @@ -22,3 +22,10 @@ pub(crate) fn is_f_string_formatting_enabled(context: &PyFormatContext) -> bool pub(crate) fn is_with_single_item_pre_39_enabled(context: &PyFormatContext) -> bool { context.is_preview() } + +/// See [#12282](https://github.com/astral-sh/ruff/pull/12282). +pub(crate) fn is_comprehension_leading_expression_comments_same_line_enabled( + context: &PyFormatContext, +) -> bool { + context.is_preview() +} diff --git a/crates/ruff_python_formatter/tests/snapshots/format@expression__list_comp.py.snap b/crates/ruff_python_formatter/tests/snapshots/format@expression__list_comp.py.snap index 316630a83f9d4..ca2f9d45d2b34 100644 --- a/crates/ruff_python_formatter/tests/snapshots/format@expression__list_comp.py.snap +++ b/crates/ruff_python_formatter/tests/snapshots/format@expression__list_comp.py.snap @@ -111,6 +111,69 @@ aaaaaaaaaaaaaaaaaaaaa = [ # Parenthesized targets and iterators. [x for (x) in y] [x for x in (y)] + + +# Leading expression comments: +y = [ + a + for ( + # comment + a + ) in ( + # comment + x + ) + if ( + # asdasd + "askldaklsdnmklasmdlkasmdlkasmdlkasmdasd" + != "as,mdnaskldmlkasdmlaksdmlkasdlkasdm" + and "zxcm,.nzxclm,zxnckmnzxckmnzxczxc" != "zxcasdasdlmnasdlknaslkdnmlaskdm" + ) + if ( + # comment + x + ) +] + +# Tuple target: +y = [ + a + for + # comment + a, b + in x + if True +] + + +y = [ + a + for ( + # comment + a, b + ) + in x + if True +] + + +y = [ + a + for + # comment + a + in + # comment + x + if + # asdasd + "askldaklsdnmklasmdlkasmdlkasmdlkasmdasd" + != "as,mdnaskldmlkasdmlaksdmlkasdlkasdm" + and "zxcm,.nzxclm,zxnckmnzxckmnzxczxc" != "zxcasdasdlmnasdlknaslkdnmlaskdm" + if + # comment + x +] ``` ## Output @@ -254,7 +317,104 @@ aaaaaaaaaaaaaaaaaaaaa = [ # Parenthesized targets and iterators. [x for (x) in y] [x for x in (y)] -``` +# Leading expression comments: +y = [ + a + for + ( + # comment + a + ) in + ( + # comment + x + ) + if + ( + # asdasd + "askldaklsdnmklasmdlkasmdlkasmdlkasmdasd" + != "as,mdnaskldmlkasdmlaksdmlkasdlkasdm" + and "zxcm,.nzxclm,zxnckmnzxckmnzxczxc" != "zxcasdasdlmnasdlknaslkdnmlaskdm" + ) + if + ( + # comment + x + ) +] + +# Tuple target: +y = [ + a + for + # comment + a, b in x + if True +] + + +y = [ + a + for ( + # comment + a, + b, + ) in x + if True +] + +y = [ + a + for + # comment + a in + # comment + x + if + # asdasd + "askldaklsdnmklasmdlkasmdlkasmdlkasmdasd" != "as,mdnaskldmlkasdmlaksdmlkasdlkasdm" + and "zxcm,.nzxclm,zxnckmnzxckmnzxczxc" != "zxcasdasdlmnasdlknaslkdnmlaskdm" + if + # comment + x +] +``` + + +## Preview changes +```diff +--- Stable ++++ Preview +@@ -142,24 +142,20 @@ + # Leading expression comments: + y = [ + a +- for +- ( ++ for ( + # comment + a +- ) in +- ( ++ ) in ( + # comment + x + ) +- if +- ( ++ if ( + # asdasd + "askldaklsdnmklasmdlkasmdlkasmdlkasmdasd" + != "as,mdnaskldmlkasdmlaksdmlkasdlkasdm" + and "zxcm,.nzxclm,zxnckmnzxckmnzxczxc" != "zxcasdasdlmnasdlknaslkdnmlaskdm" + ) +- if +- ( ++ if ( + # comment + x + ) +```