madduck's git repository

Every one of the projects in this repository is available at the canonical URL git://git.madduck.net/madduck/pub/<projectpath> — see each project's metadata for the exact URL.

All patches and comments are welcome. Please squash your changes to logical commits before using git-format-patch and git-send-email to patches@git.madduck.net. If you'd read over the Git project's submission guidelines and adhered to them, I'd be especially grateful.

SSH access, as well as push access can be individually arranged.

If you use my repositories frequently, consider adding the following snippet to ~/.gitconfig and using the third clone URL listed for each project:

[url "git://git.madduck.net/madduck/"]
  insteadOf = madduck:

Stop changing return type annotations to tuples (#2384)
authorRichard Si <63936253+ichard26@users.noreply.github.com>
Thu, 26 Aug 2021 01:32:27 +0000 (21:32 -0400)
committerGitHub <noreply@github.com>
Thu, 26 Aug 2021 01:32:27 +0000 (18:32 -0700)
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.

CHANGES.md
src/black/linegen.py
src/black/nodes.py
tests/data/function_trailing_comma.py

index ed08ab3b9adb531e67d65c41ef77bb4d2fedb9b6..47e64cfa2744c6bf8cb7c433fd31de33e2fd3e8b 100644 (file)
@@ -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_
 
index 76b553a959a4b770b7f45427e325edd09c95e2ee..fafaf1032ca55e002d49a5c3b7a8789f57f522e4 100644 (file)
@@ -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:
index e0db9a424264ccf807184a28787be8a053fe23c0..8f2e15b2cc3013926bc0636c05995749645f8b20 100644 (file)
@@ -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()`."""
index d15459cbeb5b19410e9554e1f12b01d36c8d6d52..02078219e8255f00559ac63391af8394bab63115 100644 (file)
@@ -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