Skip to content

Commit 584d033

Browse files
cobaltt7cobaltt7
authored
fix: Don't remove parenthesis around long dictionary values (#4377)
1 parent 6e96540 commit 584d033

File tree

5 files changed

+290
-66
lines changed

5 files changed

+290
-66
lines changed

‎CHANGES.md‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,8 @@
2424
(#4498)
2525
- Remove parentheses around sole list items (#4312)
2626
- Collapse multiple empty lines after an import into one (#4489)
27+
- Prevent `string_processing` and `wrap_long_dict_values_in_parens` from removing
28+
parentheses around long dictionary values (#4377)
2729

2830
### Configuration
2931

@@ -43,6 +45,7 @@
4345
### Performance
4446

4547
<!-- Changes that improve Black's performance. -->
48+
4649
- Speed up the `is_fstring_start` function in Black's tokenizer (#4541)
4750

4851
### Output

‎src/black/linegen.py‎

Lines changed: 48 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -973,29 +973,7 @@ def _maybe_split_omitting_optional_parens(
973973
try:
974974
# The RHSResult Omitting Optional Parens.
975975
rhs_oop = _first_right_hand_split(line, omit=omit)
976-
is_split_right_after_equal = (
977-
len(rhs.head.leaves) >= 2 and rhs.head.leaves[-2].type == token.EQUAL
978-
)
979-
rhs_head_contains_brackets = any(
980-
leaf.type in BRACKETS for leaf in rhs.head.leaves[:-1]
981-
)
982-
# the -1 is for the ending optional paren
983-
rhs_head_short_enough = is_line_short_enough(
984-
rhs.head, mode=replace(mode, line_length=mode.line_length - 1)
985-
)
986-
rhs_head_explode_blocked_by_magic_trailing_comma = (
987-
rhs.head.magic_trailing_comma is None
988-
)
989-
if (
990-
not (
991-
is_split_right_after_equal
992-
and rhs_head_contains_brackets
993-
and rhs_head_short_enough
994-
and rhs_head_explode_blocked_by_magic_trailing_comma
995-
)
996-
# the omit optional parens split is preferred by some other reason
997-
or _prefer_split_rhs_oop_over_rhs(rhs_oop, rhs, mode)
998-
):
976+
if _prefer_split_rhs_oop_over_rhs(rhs_oop, rhs, mode):
999977
yield from _maybe_split_omitting_optional_parens(
1000978
rhs_oop, line, mode, features=features, omit=omit
1001979
)
@@ -1006,8 +984,15 @@ def _maybe_split_omitting_optional_parens(
1006984
if line.is_chained_assignment:
1007985
pass
1008986

1009-
elif not can_be_split(rhs.body) and not is_line_short_enough(
1010-
rhs.body, mode=mode
987+
elif (
988+
not can_be_split(rhs.body)
989+
and not is_line_short_enough(rhs.body, mode=mode)
990+
and not (
991+
Preview.wrap_long_dict_values_in_parens
992+
and rhs.opening_bracket.parent
993+
and rhs.opening_bracket.parent.parent
994+
and rhs.opening_bracket.parent.parent.type == syms.dictsetmaker
995+
)
1011996
):
1012997
raise CannotSplit(
1013998
"Splitting failed, body is still too long and can't be split."
@@ -1038,6 +1023,44 @@ def _prefer_split_rhs_oop_over_rhs(
10381023
Returns whether we should prefer the result from a split omitting optional parens
10391024
(rhs_oop) over the original (rhs).
10401025
"""
1026+
# contains unsplittable type ignore
1027+
if (
1028+
rhs_oop.head.contains_unsplittable_type_ignore()
1029+
or rhs_oop.body.contains_unsplittable_type_ignore()
1030+
or rhs_oop.tail.contains_unsplittable_type_ignore()
1031+
):
1032+
return True
1033+
1034+
# Retain optional parens around dictionary values
1035+
if (
1036+
Preview.wrap_long_dict_values_in_parens
1037+
and rhs.opening_bracket.parent
1038+
and rhs.opening_bracket.parent.parent
1039+
and rhs.opening_bracket.parent.parent.type == syms.dictsetmaker
1040+
and rhs.body.bracket_tracker.delimiters
1041+
):
1042+
# Unless the split is inside the key
1043+
return any(leaf.type == token.COLON for leaf in rhs_oop.tail.leaves)
1044+
1045+
# the split is right after `=`
1046+
if not (len(rhs.head.leaves) >= 2 and rhs.head.leaves[-2].type == token.EQUAL):
1047+
return True
1048+
1049+
# the left side of assignment contains brackets
1050+
if not any(leaf.type in BRACKETS for leaf in rhs.head.leaves[:-1]):
1051+
return True
1052+
1053+
# the left side of assignment is short enough (the -1 is for the ending optional
1054+
# paren)
1055+
if not is_line_short_enough(
1056+
rhs.head, mode=replace(mode, line_length=mode.line_length - 1)
1057+
):
1058+
return True
1059+
1060+
# the left side of assignment won't explode further because of magic trailing comma
1061+
if rhs.head.magic_trailing_comma is not None:
1062+
return True
1063+
10411064
# If we have multiple targets, we prefer more `=`s on the head vs pushing them to
10421065
# the body
10431066
rhs_head_equal_count = [leaf.type for leaf in rhs.head.leaves].count(token.EQUAL)
@@ -1065,10 +1088,6 @@ def _prefer_split_rhs_oop_over_rhs(
10651088
# the first line is short enough
10661089
and is_line_short_enough(rhs_oop.head, mode=mode)
10671090
)
1068-
# contains unsplittable type ignore
1069-
or rhs_oop.head.contains_unsplittable_type_ignore()
1070-
or rhs_oop.body.contains_unsplittable_type_ignore()
1071-
or rhs_oop.tail.contains_unsplittable_type_ignore()
10721091
)
10731092

10741093

‎src/black/trans.py‎

Lines changed: 11 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -856,7 +856,7 @@ def _validate_msg(line: Line, string_idx: int) -> TResult[None]:
856856
):
857857
return TErr(
858858
"StringMerger does NOT merge f-strings with different quote types"
859-
"and internal quotes."
859+
" and internal quotes."
860860
)
861861

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

937-
# That LPAR should NOT be preceded by a function name or a closing
938-
# bracket (which could be a function which returns a function or a
939-
# list/dictionary that contains a function)...
938+
# That LPAR should NOT be preceded by a colon (which could be a
939+
# dictionary value), function name, or a closing bracket (which
940+
# could be a function returning a function or a list/dictionary
941+
# containing a function)...
940942
if is_valid_index(idx - 2) and (
941-
LL[idx - 2].type == token.NAME or LL[idx - 2].type in CLOSING_BRACKETS
943+
LL[idx - 2].type == token.COLON
944+
or LL[idx - 2].type == token.NAME
945+
or LL[idx - 2].type in CLOSING_BRACKETS
942946
):
943947
continue
944948

@@ -2264,12 +2268,12 @@ def do_transform(
22642268
elif right_leaves and right_leaves[-1].type == token.RPAR:
22652269
# Special case for lambda expressions as dict's value, e.g.:
22662270
# my_dict = {
2267-
# "key": lambda x: f"formatted: {x},
2271+
# "key": lambda x: f"formatted: {x}",
22682272
# }
22692273
# After wrapping the dict's value with parentheses, the string is
22702274
# followed by a RPAR but its opening bracket is lambda's, not
22712275
# the string's:
2272-
# "key": (lambda x: f"formatted: {x}),
2276+
# "key": (lambda x: f"formatted: {x}"),
22732277
opening_bracket = right_leaves[-1].opening_bracket
22742278
if opening_bracket is not None and opening_bracket in left_leaves:
22752279
index = left_leaves.index(opening_bracket)

0 commit comments

Comments
 (0)