From 4e3303fa08e030722d6fd4d7fe7b8d44ef98991c Mon Sep 17 00:00:00 2001 From: Jordan Ephron Date: Thu, 29 Dec 2022 18:13:15 -0500 Subject: [PATCH] Parenthesize conditional expressions (#2278) Co-authored-by: Jordan Ephron Co-authored-by: Richard Si <63936253+ichard26@users.noreply.github.com> Co-authored-by: Jelle Zijlstra --- CHANGES.md | 1 + src/black/__init__.py | 16 ++- src/black/linegen.py | 16 +++ src/black/mode.py | 1 + tests/data/conditional_expression.py | 160 +++++++++++++++++++++++++++ 5 files changed, 188 insertions(+), 6 deletions(-) create mode 100644 tests/data/conditional_expression.py diff --git a/CHANGES.md b/CHANGES.md index 587ca8a2a0d..2da0fb4720c 100644 --- a/CHANGES.md +++ b/CHANGES.md @@ -16,6 +16,7 @@ +- Add parentheses around `if`-`else` expressions (#2278) - Improve the performance on large expressions that contain many strings (#3467) - Fix a crash in preview style with assert + parenthesized string (#3415) - Fix crashes in preview style with walrus operators used in function return annotations diff --git a/src/black/__init__.py b/src/black/__init__.py index f00749aaed8..9f44722bfae 100644 --- a/src/black/__init__.py +++ b/src/black/__init__.py @@ -478,16 +478,20 @@ def main( # noqa: C901 ) normalized = [ - (source, source) - if source == "-" - else (normalize_path_maybe_ignore(Path(source), root), source) + ( + (source, source) + if source == "-" + else (normalize_path_maybe_ignore(Path(source), root), source) + ) for source in src ] srcs_string = ", ".join( [ - f'"{_norm}"' - if _norm - else f'\033[31m"{source} (skipping - invalid)"\033[34m' + ( + f'"{_norm}"' + if _norm + else f'\033[31m"{source} (skipping - invalid)"\033[34m' + ) for _norm, source in normalized ] ) diff --git a/src/black/linegen.py b/src/black/linegen.py index 2e75bc94506..4da75b28235 100644 --- a/src/black/linegen.py +++ b/src/black/linegen.py @@ -140,6 +140,22 @@ def visit_default(self, node: LN) -> Iterator[Line]: self.current_line.append(node) yield from super().visit_default(node) + def visit_test(self, node: Node) -> Iterator[Line]: + """Visit an `x if y else z` test""" + + if Preview.parenthesize_conditional_expressions in self.mode: + already_parenthesized = ( + node.prev_sibling and node.prev_sibling.type == token.LPAR + ) + + if not already_parenthesized: + lpar = Leaf(token.LPAR, "") + rpar = Leaf(token.RPAR, "") + node.insert_child(0, lpar) + node.append_child(rpar) + + yield from self.visit_default(node) + def visit_INDENT(self, node: Leaf) -> Iterator[Line]: """Increase indentation level, maybe yield a line.""" # In blib2to3 INDENT never holds comments. diff --git a/src/black/mode.py b/src/black/mode.py index a104d1b9862..775805ae960 100644 --- a/src/black/mode.py +++ b/src/black/mode.py @@ -161,6 +161,7 @@ class Preview(Enum): # NOTE: string_processing requires wrap_long_dict_values_in_parens # for https://github.com/psf/black/issues/3117 to be fixed. string_processing = auto() + parenthesize_conditional_expressions = auto() skip_magic_trailing_comma_in_subscript = auto() wrap_long_dict_values_in_parens = auto() diff --git a/tests/data/conditional_expression.py b/tests/data/conditional_expression.py new file mode 100644 index 00000000000..620a12dc986 --- /dev/null +++ b/tests/data/conditional_expression.py @@ -0,0 +1,160 @@ +long_kwargs_single_line = my_function( + foo="test, this is a sample value", + bar=some_long_value_name_foo_bar_baz if some_boolean_variable else some_fallback_value_foo_bar_baz, + baz="hello, this is a another value", +) + +multiline_kwargs_indented = my_function( + foo="test, this is a sample value", + bar=some_long_value_name_foo_bar_baz + if some_boolean_variable + else some_fallback_value_foo_bar_baz, + baz="hello, this is a another value", +) + +imploding_kwargs = my_function( + foo="test, this is a sample value", + bar=a + if foo + else b, + baz="hello, this is a another value", +) + +imploding_line = ( + 1 + if 1 + 1 == 2 + else 0 +) + +exploding_line = "hello this is a slightly long string" if some_long_value_name_foo_bar_baz else "this one is a little shorter" + +positional_argument_test(some_long_value_name_foo_bar_baz if some_boolean_variable else some_fallback_value_foo_bar_baz) + +def weird_default_argument(x=some_long_value_name_foo_bar_baz + if SOME_CONSTANT + else some_fallback_value_foo_bar_baz): + pass + +nested = "hello this is a slightly long string" if (some_long_value_name_foo_bar_baz if + nesting_test_expressions else some_fallback_value_foo_bar_baz) \ + else "this one is a little shorter" + +generator_expression = ( + some_long_value_name_foo_bar_baz if some_boolean_variable else some_fallback_value_foo_bar_baz for some_boolean_variable in some_iterable +) + + +def limit_offset_sql(self, low_mark, high_mark): + """Return LIMIT/OFFSET SQL clause.""" + limit, offset = self._get_limit_offset_params(low_mark, high_mark) + return " ".join( + sql + for sql in ( + "LIMIT %d" % limit if limit else None, + ("OFFSET %d" % offset) if offset else None, + ) + if sql + ) + + +def something(): + clone._iterable_class = ( + NamedValuesListIterable + if named + else FlatValuesListIterable + if flat + else ValuesListIterable + ) + +# output + +long_kwargs_single_line = my_function( + foo="test, this is a sample value", + bar=( + some_long_value_name_foo_bar_baz + if some_boolean_variable + else some_fallback_value_foo_bar_baz + ), + baz="hello, this is a another value", +) + +multiline_kwargs_indented = my_function( + foo="test, this is a sample value", + bar=( + some_long_value_name_foo_bar_baz + if some_boolean_variable + else some_fallback_value_foo_bar_baz + ), + baz="hello, this is a another value", +) + +imploding_kwargs = my_function( + foo="test, this is a sample value", + bar=a if foo else b, + baz="hello, this is a another value", +) + +imploding_line = 1 if 1 + 1 == 2 else 0 + +exploding_line = ( + "hello this is a slightly long string" + if some_long_value_name_foo_bar_baz + else "this one is a little shorter" +) + +positional_argument_test( + some_long_value_name_foo_bar_baz + if some_boolean_variable + else some_fallback_value_foo_bar_baz +) + + +def weird_default_argument( + x=( + some_long_value_name_foo_bar_baz + if SOME_CONSTANT + else some_fallback_value_foo_bar_baz + ), +): + pass + + +nested = ( + "hello this is a slightly long string" + if ( + some_long_value_name_foo_bar_baz + if nesting_test_expressions + else some_fallback_value_foo_bar_baz + ) + else "this one is a little shorter" +) + +generator_expression = ( + ( + some_long_value_name_foo_bar_baz + if some_boolean_variable + else some_fallback_value_foo_bar_baz + ) + for some_boolean_variable in some_iterable +) + + +def limit_offset_sql(self, low_mark, high_mark): + """Return LIMIT/OFFSET SQL clause.""" + limit, offset = self._get_limit_offset_params(low_mark, high_mark) + return " ".join( + sql + for sql in ( + "LIMIT %d" % limit if limit else None, + ("OFFSET %d" % offset) if offset else None, + ) + if sql + ) + + +def something(): + clone._iterable_class = ( + NamedValuesListIterable + if named + else FlatValuesListIterable if flat else ValuesListIterable + )