Skip to content

Commit

Permalink
fix: Don't remove parenthesis around long dictionary values (#4377)
Browse files Browse the repository at this point in the history
  • Loading branch information
cobaltt7 authored Jan 17, 2025
1 parent 6e96540 commit 584d033
Show file tree
Hide file tree
Showing 5 changed files with 290 additions and 66 deletions.
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
parentheses around long dictionary values (#4377)

### 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

0 comments on commit 584d033

Please sign in to comment.