Stop changing return type annotations to tuples (#2384)

This fixes a bug where a trailing comma would be added to a
parenthesized return 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 parameter & import list (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 return annotation before truly adding a trailing comma.

diff --git a/CHANGES.md b/CHANGES.md
index ed08ab3b9adb531e67d65c41ef77bb4d2fedb9b6..47e64cfa2744c6bf8cb7c433fd31de33e2fd3e8b 100644 (file)
--- a/CHANGES.md
+++ b/CHANGES.md
@@ -9,6 +9,8 @@
 - Present a more user-friendly error if .gitignore is invalid (#2414)
 - The failsafe for accidentally added backslashes in f-string expressions has been
   hardened to handle more edge cases during quote normalization (#2437)
+- Avoid changing a function return type annotation's type to a tuple by adding a
+  trailing comma (#2384)
 
 ### _Blackd_
 

diff --git a/src/black/linegen.py b/src/black/linegen.py
index 76b553a959a4b770b7f45427e325edd09c95e2ee..fafaf1032ca55e002d49a5c3b7a8789f57f522e4 100644 (file)
--- a/src/black/linegen.py
+++ b/src/black/linegen.py
@@ -7,7 +7,7 @@ from typing import Collection, Iterator, List, Optional, Set, Union
 
 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
@@ -574,6 +574,20 @@ def bracket_split_build_line(
                 original.is_def
                 and opening_bracket.value == "("
                 and not any(leaf.type == token.COMMA for leaf in leaves)
+                # In particular, don't add one within a parenthesized return annotation.
+                # Unfortunately the indicator we're in a return annotation (RARROW) may
+                # be defined directly in the parent node, the parent of the parent ...
+                # and so on depending on how complex the return annotation is.
+                # This isn't perfect and there's some false negatives but they are in
+                # contexts were a comma is actually fine.
+                and not any(
+                    node.prev_sibling.type == RARROW
+                    for node in (
+                        leaves[0].parent,
+                        getattr(leaves[0].parent, "parent", None),
+                    )
+                    if isinstance(node, Node) and isinstance(node.prev_sibling, Leaf)
+                )
             )
 
             if original.is_import or no_commas:

diff --git a/src/black/nodes.py b/src/black/nodes.py
index e0db9a424264ccf807184a28787be8a053fe23c0..8f2e15b2cc3013926bc0636c05995749645f8b20 100644 (file)
--- a/src/black/nodes.py
+++ b/src/black/nodes.py
@@ -135,6 +135,8 @@ CLOSING_BRACKETS = set(BRACKET.values())
 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 d15459cbeb5b19410e9554e1f12b01d36c8d6d52..02078219e8255f00559ac63391af8394bab63115 100644 (file)
--- a/tests/data/function_trailing_comma.py
+++ b/tests/data/function_trailing_comma.py
@@ -21,6 +21,34 @@ 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
+
+
+def func() -> (
+    also_super_long_type_annotation_that_may_cause_an_AST_related_crash_in_black(this_shouldn_t_get_a_trailing_comma_too)
+):
+    pass
+
+
+def func() -> ((also_super_long_type_annotation_that_may_cause_an_AST_related_crash_in_black(
+        this_shouldn_t_get_a_trailing_comma_too
+    ))
+):
+    pass
+
 # output
 
 def f(
@@ -85,4 +113,38 @@ 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
+
+
+def func() -> (
+    also_super_long_type_annotation_that_may_cause_an_AST_related_crash_in_black(
+        this_shouldn_t_get_a_trailing_comma_too
+    )
+):
+    pass
+
+
+def func() -> (
+    (
+        also_super_long_type_annotation_that_may_cause_an_AST_related_crash_in_black(
+            this_shouldn_t_get_a_trailing_comma_too
+        )
+    )
+):
+    pass