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

Format type hints after comments #3362

Open
wants to merge 24 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
24 commits
Select commit Hold shift + click to select a range
08c9138
Add comma split before delimiter split
charlie572 Oct 27, 2022
42f3595
Refactor comma_split() and delimiter_split()
charlie572 Oct 28, 2022
c2eb395
Add to CHANGES.md
charlie572 Oct 28, 2022
2ec9eda
Add comma_split() to developer reference
charlie572 Oct 28, 2022
8ae3b7c
Reformat linegen.py
charlie572 Oct 28, 2022
54b511a
Handle other cases of comments in brackets
charlie572 Oct 30, 2022
a0c658d
Revert "Add comma_split() to developer reference"
charlie572 Oct 30, 2022
57315e3
Revert "Add to CHANGES.md"
charlie572 Oct 30, 2022
3742181
Revert "Refactor comma_split() and delimiter_split()"
charlie572 Oct 30, 2022
63dc442
Revert "Add comma split before delimiter split"
charlie572 Oct 30, 2022
2dd9a7e
Move changes to preview
charlie572 Nov 1, 2022
ed21fcf
Add to CHANGES.md
charlie572 Nov 1, 2022
7438ad1
Merge branch 'main' into format-type-hints-after-comments
JelleZijlstra Nov 3, 2022
ab14c70
Merge branch 'main' into format-type-hints-after-comments
JelleZijlstra Dec 10, 2022
2be15ac
Merge branch 'main' into format-type-hints-after-comments
charlie572 Jul 1, 2023
82d05f0
Edit test to make sure a delimiter split is performed
charlie572 Jul 1, 2023
618eb16
Add comment
charlie572 Jul 1, 2023
5658c39
Handle magic trailing commas with comments
charlie572 Jul 2, 2023
de4a517
Remove magic trailing comma correctly
charlie572 Jul 5, 2023
2ccc70f
Rename magic trailing comma attributes
charlie572 Jul 5, 2023
2d32616
Gate changes to preview
charlie572 Jul 7, 2023
4e5bf07
Merge branch 'main' into format-type-hints-after-comments
hauntsaninja Jul 9, 2023
cbce271
Reformat
charlie572 Jul 8, 2023
029fce9
Merge branch 'main' into format-type-hints-after-comments
hauntsaninja Oct 6, 2023
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 @@ -125,6 +125,7 @@ wheels in a future release as soon as the mypyc bug is fixed.
- Implicitly concatenated strings used as function args are no longer wrapped inside
parentheses (#3640)
- Remove blank lines between a class definition and its docstring (#3692)
- Stop wrapping unnecessarily when there are comments inside brackets (#3362).

### Configuration

Expand Down
25 changes: 22 additions & 3 deletions src/black/linegen.py
Original file line number Diff line number Diff line change
Expand Up @@ -561,7 +561,7 @@ def transform_line(
if (
not line.contains_uncollapsable_type_comments()
and not line.should_split_rhs
and not line.magic_trailing_comma
and not line.bracket_after_magic_trailing_comma
and (
is_line_short_enough(line, mode=mode, line_str=line_str)
or line.contains_unsplittable_type_ignore()
Expand Down Expand Up @@ -619,6 +619,24 @@ def _rhs(
string_paren_wrap,
rhs,
]

# If there are comments at the beginning or end of the line, call
# standalone_comment_split before delimiter_split. Else, call
# delimiter_split first. This means that when there is a comment in the
# middle of an expression or list, it will perform a delimiter split.
# But when the comment is at the top or bottom, it won't perform a
# delimiter split (if it doesn't have to).
if (
Preview.stop_some_unnecessary_wrapping_inside_brackets in mode
and line.contains_standalone_comments()
and len(line.comments) == 0
and all(
leaf.type != STANDALONE_COMMENT for leaf in line.leaves[1:-1]
)
and line.magic_trailing_comma is None
):
transformers.remove(standalone_comment_split)
transformers.insert(0, standalone_comment_split)
else:
transformers = [
string_merge,
Expand Down Expand Up @@ -800,6 +818,7 @@ def _first_right_hand_split(
body = bracket_split_build_line(
body_leaves, line, opening_bracket, component=_BracketSplitComponent.body
)
body.magic_trailing_comma = line.magic_trailing_comma
tail = bracket_split_build_line(
tail_leaves, line, opening_bracket, component=_BracketSplitComponent.tail
)
Expand Down Expand Up @@ -848,7 +867,7 @@ def _maybe_split_omitting_optional_parens(
)
# the left side of assignment won't explode further because of magic
# trailing comma
and rhs.head.magic_trailing_comma is None
and rhs.head.bracket_after_magic_trailing_comma is None
# the split by omitting optional parens isn't preferred by some other
# reason
and not _prefer_split_rhs_oop(rhs_oop, mode)
Expand Down Expand Up @@ -1526,7 +1545,7 @@ def generate_trailers_to_omit(line: Line, line_length: int) -> Iterator[Set[Leaf
"""

omit: Set[LeafID] = set()
if not line.magic_trailing_comma:
if not line.bracket_after_magic_trailing_comma:
yield omit

length = 4 * line.depth
Expand Down
33 changes: 30 additions & 3 deletions src/black/lines.py
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,7 @@ class Line:
bracket_tracker: BracketTracker = field(default_factory=BracketTracker)
inside_brackets: bool = False
should_split_rhs: bool = False
bracket_after_magic_trailing_comma: Optional[Leaf] = None
magic_trailing_comma: Optional[Leaf] = None

def append(
Expand Down Expand Up @@ -89,7 +90,8 @@ def append(
self.bracket_tracker.mark(leaf)
if self.mode.magic_trailing_comma:
if self.has_magic_trailing_comma(leaf):
self.magic_trailing_comma = leaf
self.bracket_after_magic_trailing_comma = leaf
self.magic_trailing_comma = self.get_last_non_comment_leaf()
elif self.has_magic_trailing_comma(leaf, ensure_removable=True):
self.remove_trailing_comma()
if not self.append_comment(leaf):
Expand Down Expand Up @@ -306,6 +308,13 @@ def contains_unsplittable_type_ignore(self) -> bool:
def contains_multiline_strings(self) -> bool:
return any(is_multiline_string(leaf) for leaf in self.leaves)

def get_last_non_comment_leaf(self) -> Optional[Leaf]:
for leaf in reversed(self.leaves):
if leaf.type != STANDALONE_COMMENT:
return leaf

return None

def has_magic_trailing_comma(
self, closing: Leaf, ensure_removable: bool = False
) -> bool:
Expand All @@ -317,10 +326,18 @@ def has_magic_trailing_comma(
- it's not from square bracket indexing
(specifically, single-element square bracket indexing)
"""
if Preview.stop_some_unnecessary_wrapping_inside_brackets in self.mode:
leaf_to_check = self.get_last_non_comment_leaf()
elif len(self.leaves) > 0:
leaf_to_check = self.leaves[-1]
else:
return False

if not (
closing.type in CLOSING_BRACKETS
and self.leaves
and self.leaves[-1].type == token.COMMA
and leaf_to_check is not None
and leaf_to_check.type == token.COMMA
):
return False

Expand Down Expand Up @@ -411,7 +428,16 @@ def comments_after(self, leaf: Leaf) -> List[Leaf]:

def remove_trailing_comma(self) -> None:
"""Remove the trailing comma and moves the comments attached to it."""
trailing_comma = self.leaves.pop()
if Preview.stop_some_unnecessary_wrapping_inside_brackets in self.mode:
# There might be comments after the magic trailing comma, so we must search
# backwards to find it.
for i in range(len(self.leaves) - 1, -1, -1):
if self.leaves[i].type == token.COMMA:
trailing_comma = self.leaves.pop(i)
break
else:
trailing_comma = self.leaves.pop()

trailing_comma_comments = self.comments.pop(id(trailing_comma), [])
self.comments.setdefault(id(self.leaves[-1]), []).extend(
trailing_comma_comments
Expand Down Expand Up @@ -463,6 +489,7 @@ def clone(self) -> "Line":
inside_brackets=self.inside_brackets,
should_split_rhs=self.should_split_rhs,
magic_trailing_comma=self.magic_trailing_comma,
bracket_after_magic_trailing_comma=self.bracket_after_magic_trailing_comma,
)

def __str__(self) -> str:
Expand Down
1 change: 1 addition & 0 deletions src/black/mode.py
Original file line number Diff line number Diff line change
Expand Up @@ -185,6 +185,7 @@ class Preview(Enum):
skip_magic_trailing_comma_in_subscript = auto()
wrap_long_dict_values_in_parens = auto()
wrap_multiple_context_managers_in_parens = auto()
stop_some_unnecessary_wrapping_inside_brackets = auto()
dummy_implementations = auto()
walrus_subscript = auto()

Expand Down
1 change: 1 addition & 0 deletions src/black/trans.py
Original file line number Diff line number Diff line change
Expand Up @@ -2133,6 +2133,7 @@ def do_transform(
inside_brackets=True,
should_split_rhs=line.should_split_rhs,
magic_trailing_comma=line.magic_trailing_comma,
bracket_after_magic_trailing_comma=line.bracket_after_magic_trailing_comma,
)
string_leaf = Leaf(token.STRING, string_value)
insert_str_child(string_leaf)
Expand Down
5 changes: 5 additions & 0 deletions tests/data/miscellaneous/skip_magic_trailing_comma.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
def func(
x,
# this comment shouldn't be deleted
):
pass
5 changes: 5 additions & 0 deletions tests/data/miscellaneous/trailing_comma_and_comment.pyi
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
def func(
x,
y,
# comment
): ...
48 changes: 45 additions & 3 deletions tests/data/preview/trailing_comma.py
Original file line number Diff line number Diff line change
Expand Up @@ -7,19 +7,19 @@
f = [
arg1,
arg2,
arg3, arg4
arg3, arg4, arg5, arg6, arg7, arg8, arg9, arg10, arg11, arg12, arg13, arg14, arg15, arg16, arg17, arg18
# comment
]
g = (
arg1,
arg2,
arg3, arg4
arg3, arg4, arg5, arg6, arg7, arg8, arg9, arg10, arg11, arg12, arg13, arg14, arg15, arg16, arg17, arg18
# comment
)
h = {
arg1,
arg2,
arg3, arg4
arg3, arg4, arg5, arg6, arg7, arg8, arg9, arg10, arg11, arg12, arg13, arg14, arg15, arg16, arg17, arg18
# comment
}

Expand All @@ -37,19 +37,61 @@
arg2,
arg3,
arg4,
arg5,
arg6,
arg7,
arg8,
arg9,
arg10,
arg11,
arg12,
arg13,
arg14,
arg15,
arg16,
arg17,
arg18,
# comment
]
g = (
arg1,
arg2,
arg3,
arg4,
arg5,
arg6,
arg7,
arg8,
arg9,
arg10,
arg11,
arg12,
arg13,
arg14,
arg15,
arg16,
arg17,
arg18,
# comment
)
h = {
arg1,
arg2,
arg3,
arg4,
arg5,
arg6,
arg7,
arg8,
arg9,
arg10,
arg11,
arg12,
arg13,
arg14,
arg15,
arg16,
arg17,
arg18,
# comment
}
6 changes: 6 additions & 0 deletions tests/data/preview/trailing_comma_with_comment.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
li = [
1,
2,
3,
# comment
]
19 changes: 19 additions & 0 deletions tests/data/preview/wrapping_with_comments_in_brackets.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
def get_requires_for_build_sdist(
# pylint: disable-next=unused-argument
config_settings: dict[str, str | list[str]] | None = None,
) -> list[str]:
return ["pathspec", "pyproject_metadata"]


(
a
and b
# comment
and c
and d
)

(
# comment
a and b and c and d
)
21 changes: 21 additions & 0 deletions tests/test_format.py
Original file line number Diff line number Diff line change
Expand Up @@ -218,3 +218,24 @@ def test_type_comment_syntax_error() -> None:
source, expected = read_data("type_comments", "type_comment_syntax_error")
assert_format(source, expected)
black.assert_equivalent(source, expected)


def test_skip_magic_trailing_comma() -> None:
"""
Test a case where the --skip-magic-trailing-comma option used
"""
source, expected = read_data("miscellaneous", "skip_magic_trailing_comma")
mode = replace(DEFAULT_MODE, magic_trailing_comma=False, preview=True)
assert_format(source, expected, mode)


def test_trailing_comma_and_comment_in_pyi() -> None:
source, expected = read_data("miscellaneous", "trailing_comma_and_comment.pyi")
mode = replace(
DEFAULT_MODE,
magic_trailing_comma=False,
preview=False,
line_length=130,
is_pyi=True,
)
assert_format(source, expected, mode)