From 7045b191cda6d198866f19fb0e1a7ead70aaedfb Mon Sep 17 00:00:00 2001 From: Micha Reiser Date: Wed, 18 Sep 2024 18:43:29 +0200 Subject: [PATCH] Fix preview gating --- .../src/expression/mod.rs | 27 ++- ...ity@cases__function_trailing_comma.py.snap | 19 +-- ...@cases__return_annotation_brackets.py.snap | 58 +++++-- ...ormat@statement__return_annotation.py.snap | 79 +++++++-- ...atement__return_type_no_parameters.py.snap | 156 ++++++++++++++---- 5 files changed, 256 insertions(+), 83 deletions(-) diff --git a/crates/ruff_python_formatter/src/expression/mod.rs b/crates/ruff_python_formatter/src/expression/mod.rs index 3318d1ac85c390..1cc060ec114234 100644 --- a/crates/ruff_python_formatter/src/expression/mod.rs +++ b/crates/ruff_python_formatter/src/expression/mod.rs @@ -388,12 +388,18 @@ impl Format> for MaybeParenthesizeExpression<'_> { // is parenthesized. Unless, it's the `Parenthesize::IfBreaksParenthesizedNested` layout // where parenthesizing nested `maybe_parenthesized_expression` is explicitly desired. _ if f.context().node_level().is_parenthesized() => { - return if let Parenthesize::IfBreaksParenthesizedNested = parenthesize { - parenthesize_if_expands(&expression.format().with_options(Parentheses::Never)) - .with_indent(!is_expression_huggable(expression, f.context())) - .fmt(f) + if !is_empty_parameters_no_unnecessary_parentheses_around_return_value_enabled( + f.context(), + ) { + OptionalParentheses::Never + } else if matches!(parenthesize, Parenthesize::IfBreaksParenthesizedNested) { + return parenthesize_if_expands( + &expression.format().with_options(Parentheses::Never), + ) + .with_indent(!is_expression_huggable(expression, f.context())) + .fmt(f); } else { - expression.format().with_options(Parentheses::Never).fmt(f) + return expression.format().with_options(Parentheses::Never).fmt(f); } } needs_parentheses => needs_parentheses, @@ -446,11 +452,16 @@ impl Format> for MaybeParenthesizeExpression<'_> { } } }, - OptionalParentheses::Never => { - + OptionalParentheses::Never => match parenthesize { + Parenthesize::IfBreaksParenthesized | Parenthesize::IfBreaksParenthesizedNested if !is_empty_parameters_no_unnecessary_parentheses_around_return_value_enabled(f.context()) => { + parenthesize_if_expands(&expression.format().with_options(Parentheses::Never)) + .with_indent(!is_expression_huggable(expression, f.context())) + .fmt(f) + } + Parenthesize::Optional | Parenthesize::IfBreaks | Parenthesize::IfRequired | Parenthesize::IfBreaksParenthesized | Parenthesize::IfBreaksParenthesizedNested => { expression.format().with_options(Parentheses::Never).fmt(f) - + } }, OptionalParentheses::Always => { diff --git a/crates/ruff_python_formatter/tests/snapshots/black_compatibility@cases__function_trailing_comma.py.snap b/crates/ruff_python_formatter/tests/snapshots/black_compatibility@cases__function_trailing_comma.py.snap index 3021e7dfa5adf7..3bdabdf35a4da9 100644 --- a/crates/ruff_python_formatter/tests/snapshots/black_compatibility@cases__function_trailing_comma.py.snap +++ b/crates/ruff_python_formatter/tests/snapshots/black_compatibility@cases__function_trailing_comma.py.snap @@ -131,19 +131,6 @@ variable: ( ```diff --- Black +++ Ruff -@@ -52,9 +52,9 @@ - pass - - --def xxxxxxxxxxxxxxxxxxxxxxxxxxxx() -> ( -- Set["xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx"] --): -+def xxxxxxxxxxxxxxxxxxxxxxxxxxxx() -> Set[ -+ "xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx" -+]: - json = { - "k": { - "k2": { @@ -130,9 +130,7 @@ def foo() -> ( @@ -261,9 +248,9 @@ def f( pass -def xxxxxxxxxxxxxxxxxxxxxxxxxxxx() -> Set[ - "xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx" -]: +def xxxxxxxxxxxxxxxxxxxxxxxxxxxx() -> ( + Set["xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx"] +): json = { "k": { "k2": { diff --git a/crates/ruff_python_formatter/tests/snapshots/black_compatibility@cases__return_annotation_brackets.py.snap b/crates/ruff_python_formatter/tests/snapshots/black_compatibility@cases__return_annotation_brackets.py.snap index 1794c2b8bc2490..660fd0b731079e 100644 --- a/crates/ruff_python_formatter/tests/snapshots/black_compatibility@cases__return_annotation_brackets.py.snap +++ b/crates/ruff_python_formatter/tests/snapshots/black_compatibility@cases__return_annotation_brackets.py.snap @@ -126,7 +126,39 @@ def foo(a,b) -> tuple[int, int, int,]: return 2 * a -@@ -117,7 +125,9 @@ +@@ -99,25 +107,31 @@ + return 2 + + +-def foo() -> tuple[ +- loooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooong, +- loooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooong, +- loooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooong, +-]: ++def foo() -> ( ++ tuple[ ++ loooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooong, ++ loooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooong, ++ loooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooong, ++ ] ++): + return 2 + + + # Magic trailing comma example +-def foo() -> tuple[ +- int, +- int, +- int, +-]: ++def foo() -> ( ++ tuple[ ++ int, ++ int, ++ int, ++ ] ++): + return 2 # Magic trailing comma example, with params @@ -251,20 +283,24 @@ def foo() -> tuple[int, int, int]: return 2 -def foo() -> tuple[ - loooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooong, - loooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooong, - loooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooong, -]: +def foo() -> ( + tuple[ + loooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooong, + loooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooong, + loooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooong, + ] +): return 2 # Magic trailing comma example -def foo() -> tuple[ - int, - int, - int, -]: +def foo() -> ( + tuple[ + int, + int, + int, + ] +): return 2 diff --git a/crates/ruff_python_formatter/tests/snapshots/format@statement__return_annotation.py.snap b/crates/ruff_python_formatter/tests/snapshots/format@statement__return_annotation.py.snap index 2ce46801ee4b50..86f834791af1f2 100644 --- a/crates/ruff_python_formatter/tests/snapshots/format@statement__return_annotation.py.snap +++ b/crates/ruff_python_formatter/tests/snapshots/format@statement__return_annotation.py.snap @@ -335,24 +335,32 @@ def f[aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa]( # Breaking return type annotations. Black adds parentheses if the parameters are # empty; otherwise, it leverages the expressions own parentheses if possible. -def xxxxxxxxxxxxxxxxxxxxxxxxxxxx() -> Set[ - "xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx" -]: ... +def xxxxxxxxxxxxxxxxxxxxxxxxxxxx() -> ( + Set[ + "xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx" + ] +): ... -def xxxxxxxxxxxxxxxxxxxxxxxxxxxx() -> Set[ - "xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx" -]: ... +def xxxxxxxxxxxxxxxxxxxxxxxxxxxx() -> ( + Set[ + "xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx" + ] +): ... -def xxxxxxxxxxxxxxxxxxxxxxxxxxxx() -> Set[ - "xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx" -]: ... +def xxxxxxxxxxxxxxxxxxxxxxxxxxxx() -> ( + Set[ + "xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx" + ] +): ... -def xxxxxxxxxxxxxxxxxxxxxxxxxxxx() -> Set[ - "xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx" -]: ... +def xxxxxxxxxxxxxxxxxxxxxxxxxxxx() -> ( + Set[ + "xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx" + ] +): ... def xxxxxxxxxxxxxxxxxxxxxxxxxxxx( @@ -517,7 +525,52 @@ def process_board_action( ```diff --- Stable +++ Preview -@@ -249,11 +249,8 @@ +@@ -131,32 +131,24 @@ + + # Breaking return type annotations. Black adds parentheses if the parameters are + # empty; otherwise, it leverages the expressions own parentheses if possible. +-def xxxxxxxxxxxxxxxxxxxxxxxxxxxx() -> ( +- Set[ +- "xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx" +- ] +-): ... ++def xxxxxxxxxxxxxxxxxxxxxxxxxxxx() -> Set[ ++ "xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx" ++]: ... + + +-def xxxxxxxxxxxxxxxxxxxxxxxxxxxx() -> ( +- Set[ +- "xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx" +- ] +-): ... ++def xxxxxxxxxxxxxxxxxxxxxxxxxxxx() -> Set[ ++ "xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx" ++]: ... + + +-def xxxxxxxxxxxxxxxxxxxxxxxxxxxx() -> ( +- Set[ +- "xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx" +- ] +-): ... ++def xxxxxxxxxxxxxxxxxxxxxxxxxxxx() -> Set[ ++ "xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx" ++]: ... + + +-def xxxxxxxxxxxxxxxxxxxxxxxxxxxx() -> ( +- Set[ +- "xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx" +- ] +-): ... ++def xxxxxxxxxxxxxxxxxxxxxxxxxxxx() -> Set[ ++ "xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx" ++]: ... + + + def xxxxxxxxxxxxxxxxxxxxxxxxxxxx( +@@ -257,11 +249,8 @@ ): ... diff --git a/crates/ruff_python_formatter/tests/snapshots/format@statement__return_type_no_parameters.py.snap b/crates/ruff_python_formatter/tests/snapshots/format@statement__return_type_no_parameters.py.snap index be4b08dadd767e..2032db23087010 100644 --- a/crates/ruff_python_formatter/tests/snapshots/format@statement__return_type_no_parameters.py.snap +++ b/crates/ruff_python_formatter/tests/snapshots/format@statement__return_type_no_parameters.py.snap @@ -244,9 +244,11 @@ def test_return_union_with_elements_exceeding_length() -> ( ######################################################################################### -def test_return_multiline_string_type_annotation() -> """str +def test_return_multiline_string_type_annotation() -> ( + """str | list[str] -""": +""" +): pass @@ -292,9 +294,9 @@ def no_parameters_subscript_return_type() -> list[str]: # 1. Black tries to keep the list flat by parenthesizing the list as shown below even when the `list` identifier # fits on the header line. IMO, this adds unnecessary parentheses that can be avoided # and supporting it requires extra complexity (best_fitting! layout) -def no_parameters_overlong_subscript_return_type_with_single_element() -> list[ - xxxxxxxxxxxxxxxxxxxxx -]: +def no_parameters_overlong_subscript_return_type_with_single_element() -> ( + list[xxxxxxxxxxxxxxxxxxxxx] +): pass @@ -303,26 +305,34 @@ def no_parameters_overlong_subscript_return_type_with_single_element() -> list[ # `list[`. It is also inconsistent with how subscripts are normally formatted where it first tries to fit the entire subscript, # then splits after `list[` but keeps all elements on a single line, and finally, splits after each element. # IMO: Splitting after the `list[` and trying to keep the elements together when possible seems more consistent. -def no_parameters_subscript_return_type_multiple_elements() -> list[ - xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx, xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx -]: +def no_parameters_subscript_return_type_multiple_elements() -> ( + list[ + xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx, + xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx, + ] +): pass # Black removes the parentheses even the elements exceed the configured line width. # So does Ruff. -def no_parameters_subscript_return_type_multiple_overlong_elements() -> list[ - xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx, - xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx, -]: +def no_parameters_subscript_return_type_multiple_overlong_elements() -> ( + list[ + xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx, + xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx, + ] +): pass # Black parenthesizes the subscript if its name doesn't fit on the header line. # So does Ruff -def no_parameters_subscriptreturn_type_with_overlong_value_() -> liiiiiiiiiiiiiiiiiiiiist[ - xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx, xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx -]: +def no_parameters_subscriptreturn_type_with_overlong_value_() -> ( + liiiiiiiiiiiiiiiiiiiiist[ + xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx, + xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx, + ] +): pass @@ -330,9 +340,11 @@ def no_parameters_subscriptreturn_type_with_overlong_value_() -> liiiiiiiiiiiiii # `no_parameters_subscript_return_type_multiple_overlong_elements` shows. However, it doesn't # when the subscript contains a single element. Black then keeps the parentheses. # Ruff removes the parentheses in this case for consistency. -def no_parameters_overlong_subscript_return_type_with_overlong_single_element() -> list[ - xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx -]: +def no_parameters_overlong_subscript_return_type_with_overlong_single_element() -> ( + list[ + xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx + ] +): pass @@ -357,10 +369,12 @@ def test_binary_expression_return_type_annotation() -> ( # Don't paranthesize lists -def f() -> [ - a, - b, -]: +def f() -> ( + [ + a, + b, + ] +): pass ``` @@ -369,23 +383,80 @@ def f() -> [ ```diff --- Stable +++ Preview -@@ -134,9 +134,12 @@ +@@ -58,11 +58,9 @@ + ######################################################################################### + + +-def test_return_multiline_string_type_annotation() -> ( +- """str ++def test_return_multiline_string_type_annotation() -> """str + | list[str] +-""" +-): ++""": + pass - # Black parenthesizes the subscript if its name doesn't fit on the header line. - # So does Ruff --def no_parameters_subscriptreturn_type_with_overlong_value_() -> liiiiiiiiiiiiiiiiiiiiist[ -- xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx, xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx --]: -+def no_parameters_subscriptreturn_type_with_overlong_value_() -> ( -+ liiiiiiiiiiiiiiiiiiiiist[ -+ xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx, -+ xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx, -+ ] -+): + +@@ -108,9 +106,9 @@ + # 1. Black tries to keep the list flat by parenthesizing the list as shown below even when the `list` identifier + # fits on the header line. IMO, this adds unnecessary parentheses that can be avoided + # and supporting it requires extra complexity (best_fitting! layout) +-def no_parameters_overlong_subscript_return_type_with_single_element() -> ( +- list[xxxxxxxxxxxxxxxxxxxxx] +-): ++def no_parameters_overlong_subscript_return_type_with_single_element() -> list[ ++ xxxxxxxxxxxxxxxxxxxxx ++]: + pass + + +@@ -119,23 +117,18 @@ + # `list[`. It is also inconsistent with how subscripts are normally formatted where it first tries to fit the entire subscript, + # then splits after `list[` but keeps all elements on a single line, and finally, splits after each element. + # IMO: Splitting after the `list[` and trying to keep the elements together when possible seems more consistent. +-def no_parameters_subscript_return_type_multiple_elements() -> ( +- list[ +- xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx, +- xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx, +- ] +-): ++def no_parameters_subscript_return_type_multiple_elements() -> list[ ++ xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx, xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx ++]: pass -@@ -155,13 +158,10 @@ + # Black removes the parentheses even the elements exceed the configured line width. + # So does Ruff. +-def no_parameters_subscript_return_type_multiple_overlong_elements() -> ( +- list[ +- xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx, +- xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx, +- ] +-): ++def no_parameters_subscript_return_type_multiple_overlong_elements() -> list[ ++ xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx, ++ xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx, ++]: + pass + + +@@ -154,11 +147,9 @@ + # `no_parameters_subscript_return_type_multiple_overlong_elements` shows. However, it doesn't + # when the subscript contains a single element. Black then keeps the parentheses. + # Ruff removes the parentheses in this case for consistency. +-def no_parameters_overlong_subscript_return_type_with_overlong_single_element() -> ( +- list[ +- xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx +- ] +-): ++def no_parameters_overlong_subscript_return_type_with_overlong_single_element() -> list[ ++ xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx ++]: + pass + + +@@ -167,13 +158,10 @@ ######################################################################################### @@ -403,4 +474,19 @@ def f() -> [ pass +@@ -183,10 +171,8 @@ + + + # Don't paranthesize lists +-def f() -> ( +- [ +- a, +- b, +- ] +-): ++def f() -> [ ++ a, ++ b, ++]: + pass ```