Skip to content

Commit

Permalink
Stop changing return type annotations to tuples
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
ichard26 committed Jul 19, 2021
1 parent 65abd10 commit 43ed104
Show file tree
Hide file tree
Showing 4 changed files with 53 additions and 3 deletions.
4 changes: 4 additions & 0 deletions CHANGES.md
Expand Up @@ -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

Expand Down
16 changes: 14 additions & 2 deletions src/black/linegen.py
Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down
2 changes: 2 additions & 0 deletions src/black/nodes.py
Expand Up @@ -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()`."""
Expand Down
34 changes: 33 additions & 1 deletion tests/data/function_trailing_comma.py
Expand Up @@ -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(
Expand Down Expand Up @@ -85,4 +101,20 @@ def xxxxxxxxxxxxxxxxxxxxxxxxxxxx() -> Set[
]
}
}
}
}


# 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

0 comments on commit 43ed104

Please sign in to comment.