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

Avoid magic-trailing-comma in single-element subscripts #2942

Merged
merged 7 commits into from
Mar 24, 2022
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.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGES.md
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
<!-- Changes that affect Black's preview style -->

- Code cell separators `#%%` are now standardised to `# %%` (#2919)
- Avoid magic-trailing-comma in single-element subscripts (#2942)

### _Blackd_

Expand Down
11 changes: 8 additions & 3 deletions src/black/linegen.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -973,7 +978,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
)
):
Expand Down Expand Up @@ -1001,7 +1006,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.
Expand Down
21 changes: 18 additions & 3 deletions src/black/lines.py
Original file line number Diff line number Diff line change
Expand Up @@ -17,12 +17,12 @@
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_one_sequence_between

# types
T = TypeVar("T")
Expand Down Expand Up @@ -254,6 +254,7 @@ def has_magic_trailing_comma(
"""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 single-element subscript
Additionally, if ensure_removable:
- it's not from square bracket indexing
"""
Expand All @@ -268,6 +269,20 @@ def has_magic_trailing_comma(
return True

if closing.type == token.RSQB:
if (
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,
brackets=(token.LSQB, token.RSQB),
)
):
return False

if not ensure_removable:
return True
comma = self.leaves[-1]
Expand All @@ -276,7 +291,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
Expand Down
5 changes: 2 additions & 3 deletions src/black/mode.py
Original file line number Diff line number Diff line change
Expand Up @@ -127,6 +127,7 @@ class Preview(Enum):
"""Individual preview style features."""

string_processing = auto()
one_element_subscript = auto()


class Deprecated(UserWarning):
Expand Down Expand Up @@ -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:
Expand Down
12 changes: 9 additions & 3 deletions src/black/nodes.py
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
List,
Optional,
Set,
Tuple,
TypeVar,
Union,
)
Expand Down Expand Up @@ -559,9 +560,14 @@ 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:
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 (opening.type, closing.type) != brackets:
return False

depth = closing.bracket_depth + 1
Expand Down
36 changes: 36 additions & 0 deletions tests/data/one_element_subscript.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,36 @@
# We should not treat the trailing comma
jpy-git marked this conversation as resolved.
Show resolved Hide resolved
# 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,]

# Magic commas still work as expected for non-subscripts.
small_list = [1,]
list_of_types = [tuple[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,
]

# Magic commas still work as expected for non-subscripts.
small_list = [
1,
]
list_of_types = [
tuple[int,],
]
1 change: 1 addition & 0 deletions tests/test_format.py
Original file line number Diff line number Diff line change
Expand Up @@ -80,6 +80,7 @@
"long_strings__edge_case",
"long_strings__regression",
"percent_precedence",
"one_element_subscript",
]

SOURCES: List[str] = [
Expand Down