Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Stop changing return type annotations to tuples #2384

Merged
merged 7 commits into from Aug 26, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
2 changes: 2 additions & 0 deletions CHANGES.md
Expand Up @@ -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_

Expand Down
16 changes: 15 additions & 1 deletion 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 @@ -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:
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
64 changes: 63 additions & 1 deletion tests/data/function_trailing_comma.py
Expand Up @@ -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(
Expand Down Expand Up @@ -85,4 +113,38 @@ 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


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