From 43ed104b45365ca414f75b6c577e3b0e00fc3dd9 Mon Sep 17 00:00:00 2001 From: Richard Si <63936253+ichard26@users.noreply.github.com> Date: Mon, 19 Jul 2021 18:13:47 -0400 Subject: [PATCH] Stop changing return type annotations to tuples This fixes a bug where a trailing comma would be added to a parenthesized return type annotation changing its type to a tuple. Here's one case where this bug shows up: ``` def spam() -> ( this_is_a_long_type_annotation_which_should_NOT_get_a_trailing_comma ): pass ``` The root problem was that the type annotation was treated as if it was a collection (is_body=True to linegen::bracket_split_build_line) where a trailing comma is usually fine. Now there's another check in the aforementioned function to make sure the body it's operating on isn't a parenthesized return type annotation before truly adding a trailing comma. --- CHANGES.md | 4 ++++ src/black/linegen.py | 16 +++++++++++-- src/black/nodes.py | 2 ++ tests/data/function_trailing_comma.py | 34 ++++++++++++++++++++++++++- 4 files changed, 53 insertions(+), 3 deletions(-) diff --git a/CHANGES.md b/CHANGES.md index 6714a9d9eb2..68fed4b1d13 100644 --- a/CHANGES.md +++ b/CHANGES.md @@ -2,7 +2,11 @@ ## Unreleased +### _Black_ + - Moved from `appdirs` dependency to `platformdirs` (#2375) +- Avoid changing a function return type annotation's type to a tuple by adding a + trailing comma (#2384) ## 21.7b0 diff --git a/src/black/linegen.py b/src/black/linegen.py index 76b553a959a..922b8096322 100644 --- a/src/black/linegen.py +++ b/src/black/linegen.py @@ -7,7 +7,7 @@ from dataclasses import dataclass, field -from black.nodes import WHITESPACE, STATEMENT, STANDALONE_COMMENT +from black.nodes import WHITESPACE, RARROW, STATEMENT, STANDALONE_COMMENT from black.nodes import ASSIGNMENTS, OPENING_BRACKETS, CLOSING_BRACKETS from black.nodes import Visitor, syms, first_child_is_arith, ensure_visible from black.nodes import is_docstring, is_empty_tuple, is_one_tuple, is_one_tuple_between @@ -575,8 +575,20 @@ def bracket_split_build_line( and opening_bracket.value == "(" and not any(leaf.type == token.COMMA for leaf in leaves) ) + # Enables us to avoid putting a comma after a parenthesesized (return) type + # annotation (and changing its type to a tuple) in a case like this one: + # + # def spam() -> ( + # this_is_a_long_type_annotation_which_should_NOT_get_a_trailing_comma + # ): + # pass + type_annotation = ( + isinstance(leaves[0].parent, Node) + and leaves[0].parent.prev_sibling + and leaves[0].parent.prev_sibling.type == RARROW + ) - if original.is_import or no_commas: + if (original.is_import or no_commas) and not type_annotation: for i in range(len(leaves) - 1, -1, -1): if leaves[i].type == STANDALONE_COMMENT: continue diff --git a/src/black/nodes.py b/src/black/nodes.py index e0db9a42426..8f2e15b2cc3 100644 --- a/src/black/nodes.py +++ b/src/black/nodes.py @@ -135,6 +135,8 @@ BRACKETS = OPENING_BRACKETS | CLOSING_BRACKETS ALWAYS_NO_SPACE = CLOSING_BRACKETS | {token.COMMA, STANDALONE_COMMENT} +RARROW = 55 + class Visitor(Generic[T]): """Basic lib2to3 visitor that yields things of type `T` on `visit()`.""" diff --git a/tests/data/function_trailing_comma.py b/tests/data/function_trailing_comma.py index d15459cbeb5..29e90e44924 100644 --- a/tests/data/function_trailing_comma.py +++ b/tests/data/function_trailing_comma.py @@ -21,6 +21,22 @@ def xxxxxxxxxxxxxxxxxxxxxxxxxxxx() -> Set[ ]: json = {"k": {"k2": {"k3": [1,]}}} + + +# The type annotation shouldn't get a trailing comma since that would change its type. +# Relevant bug report: https://github.com/psf/black/issues/2381. +def some_function_with_a_really_long_name() -> ( + returning_a_deeply_nested_import_of_a_type_i_suppose +): + pass + + +def some_method_with_a_really_long_name(very_long_parameter_so_yeah: str, another_long_parameter: int) -> ( + another_case_of_returning_a_deeply_nested_import_of_a_type_i_suppose_cause_why_not +): + pass + + # output def f( @@ -85,4 +101,20 @@ def xxxxxxxxxxxxxxxxxxxxxxxxxxxx() -> Set[ ] } } - } \ No newline at end of file + } + + +# The type annotation shouldn't get a trailing comma since that would change its type. +# Relevant bug report: https://github.com/psf/black/issues/2381. +def some_function_with_a_really_long_name() -> ( + returning_a_deeply_nested_import_of_a_type_i_suppose +): + pass + + +def some_method_with_a_really_long_name( + very_long_parameter_so_yeah: str, another_long_parameter: int +) -> ( + another_case_of_returning_a_deeply_nested_import_of_a_type_i_suppose_cause_why_not +): + pass