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

fix: Don't remove parenthesis around long dictionary values #4377

Merged
merged 23 commits into from
Jan 17, 2025
Merged
Show file tree
Hide file tree
Changes from 21 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
3 changes: 3 additions & 0 deletions CHANGES.md
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,8 @@
(#4498)
- Remove parentheses around sole list items (#4312)
- Collapse multiple empty lines after an import into one (#4489)
- Prevent `string_processing` and `wrap_long_dict_values_in_parens` from removing
parenthesis around long dictionary values (#4377)
JelleZijlstra marked this conversation as resolved.
Show resolved Hide resolved

### Configuration

Expand All @@ -43,6 +45,7 @@
### Performance

<!-- Changes that improve Black's performance. -->

- Speed up the `is_fstring_start` function in Black's tokenizer (#4541)

### Output
Expand Down
77 changes: 48 additions & 29 deletions src/black/linegen.py
Original file line number Diff line number Diff line change
Expand Up @@ -973,29 +973,7 @@ def _maybe_split_omitting_optional_parens(
try:
# The RHSResult Omitting Optional Parens.
rhs_oop = _first_right_hand_split(line, omit=omit)
is_split_right_after_equal = (
len(rhs.head.leaves) >= 2 and rhs.head.leaves[-2].type == token.EQUAL
)
rhs_head_contains_brackets = any(
leaf.type in BRACKETS for leaf in rhs.head.leaves[:-1]
)
# the -1 is for the ending optional paren
rhs_head_short_enough = is_line_short_enough(
rhs.head, mode=replace(mode, line_length=mode.line_length - 1)
)
rhs_head_explode_blocked_by_magic_trailing_comma = (
rhs.head.magic_trailing_comma is None
)
if (
not (
is_split_right_after_equal
and rhs_head_contains_brackets
and rhs_head_short_enough
and rhs_head_explode_blocked_by_magic_trailing_comma
)
# the omit optional parens split is preferred by some other reason
or _prefer_split_rhs_oop_over_rhs(rhs_oop, rhs, mode)
):
if _prefer_split_rhs_oop_over_rhs(rhs_oop, rhs, mode):
yield from _maybe_split_omitting_optional_parens(
rhs_oop, line, mode, features=features, omit=omit
)
Expand All @@ -1006,8 +984,15 @@ def _maybe_split_omitting_optional_parens(
if line.is_chained_assignment:
pass

elif not can_be_split(rhs.body) and not is_line_short_enough(
rhs.body, mode=mode
elif (
not can_be_split(rhs.body)
and not is_line_short_enough(rhs.body, mode=mode)
and not (
Preview.wrap_long_dict_values_in_parens
and rhs.opening_bracket.parent
and rhs.opening_bracket.parent.parent
and rhs.opening_bracket.parent.parent.type == syms.dictsetmaker
)
):
raise CannotSplit(
"Splitting failed, body is still too long and can't be split."
Expand Down Expand Up @@ -1038,6 +1023,44 @@ def _prefer_split_rhs_oop_over_rhs(
Returns whether we should prefer the result from a split omitting optional parens
(rhs_oop) over the original (rhs).
"""
# contains unsplittable type ignore
if (
rhs_oop.head.contains_unsplittable_type_ignore()
or rhs_oop.body.contains_unsplittable_type_ignore()
or rhs_oop.tail.contains_unsplittable_type_ignore()
):
return True

# Retain optional parens around dictionary values
if (
Preview.wrap_long_dict_values_in_parens
and rhs.opening_bracket.parent
and rhs.opening_bracket.parent.parent
and rhs.opening_bracket.parent.parent.type == syms.dictsetmaker
and rhs.body.bracket_tracker.delimiters
):
# Unless the split is inside the key
return any(leaf.type == token.COLON for leaf in rhs_oop.tail.leaves)

# the split is right after `=`
if not (len(rhs.head.leaves) >= 2 and rhs.head.leaves[-2].type == token.EQUAL):
return True

# the left side of assignment contains brackets
if not any(leaf.type in BRACKETS for leaf in rhs.head.leaves[:-1]):
return True

# the left side of assignment is short enough (the -1 is for the ending optional
# paren)
if not is_line_short_enough(
rhs.head, mode=replace(mode, line_length=mode.line_length - 1)
):
return True

# the left side of assignment won't explode further because of magic trailing comma
if rhs.head.magic_trailing_comma is not None:
return True

# If we have multiple targets, we prefer more `=`s on the head vs pushing them to
# the body
rhs_head_equal_count = [leaf.type for leaf in rhs.head.leaves].count(token.EQUAL)
Expand Down Expand Up @@ -1065,10 +1088,6 @@ def _prefer_split_rhs_oop_over_rhs(
# the first line is short enough
and is_line_short_enough(rhs_oop.head, mode=mode)
)
# contains unsplittable type ignore
or rhs_oop.head.contains_unsplittable_type_ignore()
or rhs_oop.body.contains_unsplittable_type_ignore()
or rhs_oop.tail.contains_unsplittable_type_ignore()
)


Expand Down
18 changes: 11 additions & 7 deletions src/black/trans.py
Original file line number Diff line number Diff line change
Expand Up @@ -856,7 +856,7 @@ def _validate_msg(line: Line, string_idx: int) -> TResult[None]:
):
return TErr(
"StringMerger does NOT merge f-strings with different quote types"
"and internal quotes."
" and internal quotes."
)

if id(leaf) in line.comments:
Expand Down Expand Up @@ -887,6 +887,7 @@ class StringParenStripper(StringTransformer):
The line contains a string which is surrounded by parentheses and:
- The target string is NOT the only argument to a function call.
- The target string is NOT a "pointless" string.
- The target string is NOT a dictionary value.
- If the target string contains a PERCENT, the brackets are not
preceded or followed by an operator with higher precedence than
PERCENT.
Expand Down Expand Up @@ -934,11 +935,14 @@ def do_match(self, line: Line) -> TMatchResult:
):
continue

# That LPAR should NOT be preceded by a function name or a closing
# bracket (which could be a function which returns a function or a
# list/dictionary that contains a function)...
# That LPAR should NOT be preceded by a colon (which could be a
# dictionary value), function name, or a closing bracket (which
# could be a function returning a function or a list/dictionary
# containing a function)...
if is_valid_index(idx - 2) and (
LL[idx - 2].type == token.NAME or LL[idx - 2].type in CLOSING_BRACKETS
LL[idx - 2].type == token.COLON
or LL[idx - 2].type == token.NAME
or LL[idx - 2].type in CLOSING_BRACKETS
):
continue

Expand Down Expand Up @@ -2264,12 +2268,12 @@ def do_transform(
elif right_leaves and right_leaves[-1].type == token.RPAR:
# Special case for lambda expressions as dict's value, e.g.:
# my_dict = {
# "key": lambda x: f"formatted: {x},
# "key": lambda x: f"formatted: {x}",
# }
# After wrapping the dict's value with parentheses, the string is
# followed by a RPAR but its opening bracket is lambda's, not
# the string's:
# "key": (lambda x: f"formatted: {x}),
# "key": (lambda x: f"formatted: {x}"),
opening_bracket = right_leaves[-1].opening_bracket
if opening_bracket is not None and opening_bracket in left_leaves:
index = left_leaves.index(opening_bracket)
Expand Down
Loading
Loading