From ea560316a19300203397ec40a81d8514ecf34c7e Mon Sep 17 00:00:00 2001 From: jpy-git Date: Fri, 18 Mar 2022 17:06:21 +0000 Subject: [PATCH 1/4] Avoid magic-trailing-comma in single tuple type --- CHANGES.md | 2 ++ src/black/lines.py | 27 ++++++++++++++++++++---- src/black/mode.py | 5 ++--- src/black/nodes.py | 17 ++++++++++++++- tests/data/one_tuple_annotation.py | 34 ++++++++++++++++++++++++++++++ tests/test_format.py | 1 + 6 files changed, 78 insertions(+), 8 deletions(-) create mode 100644 tests/data/one_tuple_annotation.py diff --git a/CHANGES.md b/CHANGES.md index bb3ccb9ed9..171ece50b2 100644 --- a/CHANGES.md +++ b/CHANGES.md @@ -14,6 +14,8 @@ +- Ignore magic comma in one tuple type (#2942) + ### _Blackd_ diff --git a/src/black/lines.py b/src/black/lines.py index f35665c8e0..ee6af46b36 100644 --- a/src/black/lines.py +++ b/src/black/lines.py @@ -17,12 +17,17 @@ from blib2to3.pgen2 import token from black.brackets import BracketTracker, DOT_PRIORITY -from black.mode import Mode +from black.mode import Mode, Preview from black.nodes import STANDALONE_COMMENT, TEST_DESCENDANTS from black.nodes import BRACKETS, OPENING_BRACKETS, CLOSING_BRACKETS from black.nodes import syms, whitespace, replace_child, child_towards -from black.nodes import is_multiline_string, is_import, is_type_comment -from black.nodes import is_one_tuple_between +from black.nodes import ( + is_multiline_string, + is_import, + is_type_comment, + is_within_annotation, + is_one_tuple_between, +) # types T = TypeVar("T") @@ -253,7 +258,7 @@ def has_magic_trailing_comma( ) -> bool: """Return True if we have a magic trailing comma, that is when: - there's a trailing comma here - - it's not a one-tuple + - it's not a one-tuple (or the equivalent type hint) Additionally, if ensure_removable: - it's not from square bracket indexing """ @@ -268,6 +273,20 @@ def has_magic_trailing_comma( return True if closing.type == token.RSQB: + if ( + Preview.one_tuple_type in self.mode + and is_within_annotation(closing) + and closing.opening_bracket + and is_one_tuple_between(closing.opening_bracket, closing, self.leaves) + and closing.parent + and closing.parent.prev_sibling + and ( + list(closing.parent.prev_sibling.leaves())[-1].value + in ("tuple", "Tuple") + ) + ): + return False + if not ensure_removable: return True comma = self.leaves[-1] diff --git a/src/black/mode.py b/src/black/mode.py index 35a072c23e..787935b59d 100644 --- a/src/black/mode.py +++ b/src/black/mode.py @@ -127,6 +127,7 @@ class Preview(Enum): """Individual preview style features.""" string_processing = auto() + one_tuple_type = auto() class Deprecated(UserWarning): @@ -162,9 +163,7 @@ def __contains__(self, feature: Preview) -> bool: """ if feature is Preview.string_processing: return self.preview or self.experimental_string_processing - # TODO: Remove type ignore comment once preview contains more features - # than just ESP - return self.preview # type: ignore + return self.preview def get_cache_key(self) -> str: if self.target_versions: diff --git a/src/black/nodes.py b/src/black/nodes.py index f130bff990..484c06c722 100644 --- a/src/black/nodes.py +++ b/src/black/nodes.py @@ -561,7 +561,10 @@ def is_one_tuple(node: LN) -> bool: def is_one_tuple_between(opening: Leaf, closing: Leaf, leaves: List[Leaf]) -> bool: """Return True if content between `opening` and `closing` looks like a one-tuple.""" - if opening.type != token.LPAR and closing.type != token.RPAR: + if not ( + (opening.type == token.LPAR and closing.type == token.RPAR) + or (opening.type == token.LSQB and closing.type == token.RSQB) + ): return False depth = closing.bracket_depth + 1 @@ -597,6 +600,18 @@ def is_walrus_assignment(node: LN) -> bool: return inner is not None and inner.type == syms.namedexpr_test +def is_within_annotation(node: LN) -> bool: + """Return True iff `node` is either `annassign` or child of `annassign`""" + found_annassign = False + current_node = node + while current_node.parent: + if current_node.type == syms.annassign: + found_annassign = True + break + current_node = current_node.parent + return found_annassign + + def is_simple_decorator_trailer(node: LN, last: bool = False) -> bool: """Return True iff `node` is a trailer valid in a simple decorator""" return node.type == syms.trailer and ( diff --git a/tests/data/one_tuple_annotation.py b/tests/data/one_tuple_annotation.py new file mode 100644 index 0000000000..b05a1efd4a --- /dev/null +++ b/tests/data/one_tuple_annotation.py @@ -0,0 +1,34 @@ +import typing +from typing import List, Tuple + +# We should not treat the trailing comma +# in a single-element tuple type as a magic comma. +a: tuple[int,] +b: Tuple[int,] +c: typing.Tuple[int,] + +# The magic comma still applies to non tuple types. +d: list[int,] +e: List[int,] +f: typing.List[int,] + +# output +import typing +from typing import List, Tuple + +# We should not treat the trailing comma +# in a single-element tuple type as a magic comma. +a: tuple[int,] +b: Tuple[int,] +c: typing.Tuple[int,] + +# The magic comma still applies to non tuple types. +d: list[ + int, +] +e: List[ + int, +] +f: typing.List[ + int, +] diff --git a/tests/test_format.py b/tests/test_format.py index 269bbacd24..ac818a658e 100644 --- a/tests/test_format.py +++ b/tests/test_format.py @@ -79,6 +79,7 @@ "long_strings__edge_case", "long_strings__regression", "percent_precedence", + "one_tuple_annotation", ] SOURCES: List[str] = [ From a6b21b95eeb792f929c3bb63aca4bc5ebb9bc68f Mon Sep 17 00:00:00 2001 From: jpy-git Date: Fri, 18 Mar 2022 17:10:53 +0000 Subject: [PATCH 2/4] Update CHANGES.md --- CHANGES.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CHANGES.md b/CHANGES.md index 171ece50b2..7da42223e7 100644 --- a/CHANGES.md +++ b/CHANGES.md @@ -14,7 +14,7 @@ -- Ignore magic comma in one tuple type (#2942) +- Avoid magic-trailing-comma in single tuple type (#2942) ### _Blackd_ From b49fa5c4bb13d2b4a7b6a24bf8bf3ef1f6002a65 Mon Sep 17 00:00:00 2001 From: jpy-git Date: Fri, 18 Mar 2022 17:17:07 +0000 Subject: [PATCH 3/4] make helper func generic --- src/black/linegen.py | 11 ++++++++--- src/black/lines.py | 8 +++++--- src/black/nodes.py | 4 ++-- 3 files changed, 15 insertions(+), 8 deletions(-) diff --git a/src/black/linegen.py b/src/black/linegen.py index 4dc242a1df..5c1c2ddaa5 100644 --- a/src/black/linegen.py +++ b/src/black/linegen.py @@ -8,7 +8,12 @@ from black.nodes import WHITESPACE, RARROW, STATEMENT, STANDALONE_COMMENT from black.nodes import ASSIGNMENTS, OPENING_BRACKETS, CLOSING_BRACKETS from black.nodes import Visitor, syms, is_arith_like, ensure_visible -from black.nodes import is_docstring, is_empty_tuple, is_one_tuple, is_one_tuple_between +from black.nodes import ( + is_docstring, + is_empty_tuple, + is_one_tuple, + is_one_sequence_between, +) from black.nodes import is_name_token, is_lpar_token, is_rpar_token from black.nodes import is_walrus_assignment, is_yield, is_vararg, is_multiline_string from black.nodes import is_stub_suite, is_stub_body, is_atom_with_invisible_parens @@ -969,7 +974,7 @@ def generate_trailers_to_omit(line: Line, line_length: int) -> Iterator[Set[Leaf prev and prev.type == token.COMMA and leaf.opening_bracket is not None - and not is_one_tuple_between( + and not is_one_sequence_between( leaf.opening_bracket, leaf, line.leaves ) ): @@ -997,7 +1002,7 @@ def generate_trailers_to_omit(line: Line, line_length: int) -> Iterator[Set[Leaf prev and prev.type == token.COMMA and leaf.opening_bracket is not None - and not is_one_tuple_between(leaf.opening_bracket, leaf, line.leaves) + and not is_one_sequence_between(leaf.opening_bracket, leaf, line.leaves) ): # Never omit bracket pairs with trailing commas. # We need to explode on those. diff --git a/src/black/lines.py b/src/black/lines.py index ee6af46b36..0e36f60da5 100644 --- a/src/black/lines.py +++ b/src/black/lines.py @@ -26,7 +26,7 @@ is_import, is_type_comment, is_within_annotation, - is_one_tuple_between, + is_one_sequence_between, ) # types @@ -277,7 +277,9 @@ def has_magic_trailing_comma( Preview.one_tuple_type in self.mode and is_within_annotation(closing) and closing.opening_bracket - and is_one_tuple_between(closing.opening_bracket, closing, self.leaves) + and is_one_sequence_between( + closing.opening_bracket, closing, self.leaves + ) and closing.parent and closing.parent.prev_sibling and ( @@ -295,7 +297,7 @@ def has_magic_trailing_comma( if self.is_import: return True - if closing.opening_bracket is not None and not is_one_tuple_between( + if closing.opening_bracket is not None and not is_one_sequence_between( closing.opening_bracket, closing, self.leaves ): return True diff --git a/src/black/nodes.py b/src/black/nodes.py index 484c06c722..70255d7787 100644 --- a/src/black/nodes.py +++ b/src/black/nodes.py @@ -559,8 +559,8 @@ def is_one_tuple(node: LN) -> bool: ) -def is_one_tuple_between(opening: Leaf, closing: Leaf, leaves: List[Leaf]) -> bool: - """Return True if content between `opening` and `closing` looks like a one-tuple.""" +def is_one_sequence_between(opening: Leaf, closing: Leaf, leaves: List[Leaf]) -> bool: + """Return True if content between `opening` and `closing` is a one-sequence.""" if not ( (opening.type == token.LPAR and closing.type == token.RPAR) or (opening.type == token.LSQB and closing.type == token.RSQB) From 530257bf203c86ace23b014a371d6d1040e6e94f Mon Sep 17 00:00:00 2001 From: jpy-git Date: Fri, 18 Mar 2022 18:12:23 +0000 Subject: [PATCH 4/4] Convert PR to disable magic-trailing-commas for single-element subscripts --- CHANGES.md | 2 +- src/black/lines.py | 28 ++++++++++-------------- src/black/mode.py | 2 +- src/black/nodes.py | 25 +++++++-------------- tests/data/one_element_subscript.py | 24 ++++++++++++++++++++ tests/data/one_tuple_annotation.py | 34 ----------------------------- tests/test_format.py | 2 +- 7 files changed, 46 insertions(+), 71 deletions(-) create mode 100644 tests/data/one_element_subscript.py delete mode 100644 tests/data/one_tuple_annotation.py diff --git a/CHANGES.md b/CHANGES.md index 7da42223e7..d6690f42cf 100644 --- a/CHANGES.md +++ b/CHANGES.md @@ -14,7 +14,7 @@ -- Avoid magic-trailing-comma in single tuple type (#2942) +- Avoid magic-trailing-comma in single-element subscripts (#2942) ### _Blackd_ diff --git a/src/black/lines.py b/src/black/lines.py index 0e36f60da5..e455a50753 100644 --- a/src/black/lines.py +++ b/src/black/lines.py @@ -21,13 +21,8 @@ from black.nodes import STANDALONE_COMMENT, TEST_DESCENDANTS from black.nodes import BRACKETS, OPENING_BRACKETS, CLOSING_BRACKETS from black.nodes import syms, whitespace, replace_child, child_towards -from black.nodes import ( - is_multiline_string, - is_import, - is_type_comment, - is_within_annotation, - is_one_sequence_between, -) +from black.nodes import is_multiline_string, is_import, is_type_comment +from black.nodes import is_one_sequence_between # types T = TypeVar("T") @@ -258,7 +253,8 @@ def has_magic_trailing_comma( ) -> bool: """Return True if we have a magic trailing comma, that is when: - there's a trailing comma here - - it's not a one-tuple (or the equivalent type hint) + - it's not a one-tuple + - it's not a single-element subscript Additionally, if ensure_removable: - it's not from square bracket indexing """ @@ -274,17 +270,15 @@ def has_magic_trailing_comma( if closing.type == token.RSQB: if ( - Preview.one_tuple_type in self.mode - and is_within_annotation(closing) + Preview.one_element_subscript in self.mode + and closing.parent + and closing.parent.type == syms.trailer and closing.opening_bracket and is_one_sequence_between( - closing.opening_bracket, closing, self.leaves - ) - and closing.parent - and closing.parent.prev_sibling - and ( - list(closing.parent.prev_sibling.leaves())[-1].value - in ("tuple", "Tuple") + closing.opening_bracket, + closing, + self.leaves, + brackets=(token.LSQB, token.RSQB), ) ): return False diff --git a/src/black/mode.py b/src/black/mode.py index 787935b59d..77b1cabfcb 100644 --- a/src/black/mode.py +++ b/src/black/mode.py @@ -127,7 +127,7 @@ class Preview(Enum): """Individual preview style features.""" string_processing = auto() - one_tuple_type = auto() + one_element_subscript = auto() class Deprecated(UserWarning): diff --git a/src/black/nodes.py b/src/black/nodes.py index 70255d7787..d18d4bde87 100644 --- a/src/black/nodes.py +++ b/src/black/nodes.py @@ -9,6 +9,7 @@ List, Optional, Set, + Tuple, TypeVar, Union, ) @@ -559,12 +560,14 @@ def is_one_tuple(node: LN) -> bool: ) -def is_one_sequence_between(opening: Leaf, closing: Leaf, leaves: List[Leaf]) -> bool: +def is_one_sequence_between( + opening: Leaf, + closing: Leaf, + leaves: List[Leaf], + brackets: Tuple[int, int] = (token.LPAR, token.RPAR), +) -> bool: """Return True if content between `opening` and `closing` is a one-sequence.""" - if not ( - (opening.type == token.LPAR and closing.type == token.RPAR) - or (opening.type == token.LSQB and closing.type == token.RSQB) - ): + if (opening.type, closing.type) != brackets: return False depth = closing.bracket_depth + 1 @@ -600,18 +603,6 @@ def is_walrus_assignment(node: LN) -> bool: return inner is not None and inner.type == syms.namedexpr_test -def is_within_annotation(node: LN) -> bool: - """Return True iff `node` is either `annassign` or child of `annassign`""" - found_annassign = False - current_node = node - while current_node.parent: - if current_node.type == syms.annassign: - found_annassign = True - break - current_node = current_node.parent - return found_annassign - - def is_simple_decorator_trailer(node: LN, last: bool = False) -> bool: """Return True iff `node` is a trailer valid in a simple decorator""" return node.type == syms.trailer and ( diff --git a/tests/data/one_element_subscript.py b/tests/data/one_element_subscript.py new file mode 100644 index 0000000000..019d644185 --- /dev/null +++ b/tests/data/one_element_subscript.py @@ -0,0 +1,24 @@ +# We should not treat the trailing comma +# in a single-element subscript. +a: tuple[int,] +b = tuple[int,] + +# The magic comma still applies to multi-element subscripts. +c: tuple[int, int,] +d = tuple[int, int,] + +# output +# We should not treat the trailing comma +# in a single-element subscript. +a: tuple[int,] +b = tuple[int,] + +# The magic comma still applies to multi-element subscripts. +c: tuple[ + int, + int, +] +d = tuple[ + int, + int, +] diff --git a/tests/data/one_tuple_annotation.py b/tests/data/one_tuple_annotation.py deleted file mode 100644 index b05a1efd4a..0000000000 --- a/tests/data/one_tuple_annotation.py +++ /dev/null @@ -1,34 +0,0 @@ -import typing -from typing import List, Tuple - -# We should not treat the trailing comma -# in a single-element tuple type as a magic comma. -a: tuple[int,] -b: Tuple[int,] -c: typing.Tuple[int,] - -# The magic comma still applies to non tuple types. -d: list[int,] -e: List[int,] -f: typing.List[int,] - -# output -import typing -from typing import List, Tuple - -# We should not treat the trailing comma -# in a single-element tuple type as a magic comma. -a: tuple[int,] -b: Tuple[int,] -c: typing.Tuple[int,] - -# The magic comma still applies to non tuple types. -d: list[ - int, -] -e: List[ - int, -] -f: typing.List[ - int, -] diff --git a/tests/test_format.py b/tests/test_format.py index ac818a658e..cfabed7ce3 100644 --- a/tests/test_format.py +++ b/tests/test_format.py @@ -79,7 +79,7 @@ "long_strings__edge_case", "long_strings__regression", "percent_precedence", - "one_tuple_annotation", + "one_element_subscript", ] SOURCES: List[str] = [