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: bug where the doublestar operation had inconsistent formatting. #4154

Merged
merged 15 commits into from Feb 5, 2024
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.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
2 changes: 2 additions & 0 deletions CHANGES.md
Expand Up @@ -16,6 +16,8 @@

- Move the `hug_parens_with_braces_and_square_brackets` feature to the unstable style
due to an outstanding crash and proposed formatting tweaks (#4198)
- Fixed a bug where base expressions caused inconsistent formatting of \*\* in tenary
expression (#4154)
- Checking for newline before adding one on docstring that is almost at the line limit
(#4185)

Expand Down
2 changes: 2 additions & 0 deletions docs/the_black_code_style/future_style.md
Expand Up @@ -28,6 +28,8 @@ Currently, the following features are included in the preview style:
longer normalized
- `typed_params_trailing_comma`: consistently add trailing commas to typed function
parameters
- `is_simple_lookup_for_doublestar_expression`: fix line length computation for certain
expressions that involve the power operator
- `docstring_check_for_newline`: checks if there is a newline before the terminating
quotes of a docstring

Expand Down
1 change: 1 addition & 0 deletions src/black/mode.py
Expand Up @@ -177,6 +177,7 @@ class Preview(Enum):
wrap_long_dict_values_in_parens = auto()
multiline_string_handling = auto()
typed_params_trailing_comma = auto()
is_simple_lookup_for_doublestar_expression = auto()
wannieman98 marked this conversation as resolved.
Show resolved Hide resolved
docstring_check_for_newline = auto()


Expand Down
1 change: 1 addition & 0 deletions src/black/resources/black.schema.json
Expand Up @@ -86,6 +86,7 @@
"wrap_long_dict_values_in_parens",
"multiline_string_handling",
"typed_params_trailing_comma",
"is_simple_lookup_for_doublestar_expression",
"docstring_check_for_newline"
]
},
Expand Down
138 changes: 112 additions & 26 deletions src/black/trans.py
Expand Up @@ -29,7 +29,7 @@

from black.comments import contains_pragma_comment
from black.lines import Line, append_leaves
from black.mode import Feature, Mode
from black.mode import Feature, Mode, Preview
from black.nodes import (
CLOSING_BRACKETS,
OPENING_BRACKETS,
Expand Down Expand Up @@ -94,43 +94,36 @@ def hug_power_op(
else:
raise CannotTransform("No doublestar token was found in the line.")

def is_simple_lookup(index: int, step: Literal[1, -1]) -> bool:
def is_simple_lookup(index: int, kind: Literal[1, -1]) -> bool:
# Brackets and parentheses indicate calls, subscripts, etc. ...
# basically stuff that doesn't count as "simple". Only a NAME lookup
# or dotted lookup (eg. NAME.NAME) is OK.
if step == -1:
disallowed = {token.RPAR, token.RSQB}
else:
disallowed = {token.LPAR, token.LSQB}

while 0 <= index < len(line.leaves):
current = line.leaves[index]
if current.type in disallowed:
return False
if current.type not in {token.NAME, token.DOT} or current.value == "for":
# If the current token isn't disallowed, we'll assume this is simple as
# only the disallowed tokens are semantically attached to this lookup
# expression we're checking. Also, stop early if we hit the 'for' bit
# of a comprehension.
return True
if Preview.is_simple_lookup_for_doublestar_expression not in mode:
return original_is_simple_lookup_func(line, index, kind)

index += step

return True
else:
if kind == -1:
return handle_is_simple_look_up_prev(
line, index, {token.RPAR, token.RSQB}
)
else:
return handle_is_simple_lookup_forward(
line, index, {token.LPAR, token.LSQB}
)

def is_simple_operand(index: int, kind: Literal["base", "exponent"]) -> bool:
def is_simple_operand(index: int, kind: Literal[1, -1]) -> bool:
# An operand is considered "simple" if's a NAME, a numeric CONSTANT, a simple
# lookup (see above), with or without a preceding unary operator.
start = line.leaves[index]
if start.type in {token.NAME, token.NUMBER}:
return is_simple_lookup(index, step=(1 if kind == "exponent" else -1))
return is_simple_lookup(index, kind)

if start.type in {token.PLUS, token.MINUS, token.TILDE}:
if line.leaves[index + 1].type in {token.NAME, token.NUMBER}:
# step is always one as bases with a preceding unary op will be checked
# kind is always one as bases with a preceding unary op will be checked
# for simplicity starting from the next token (so it'll hit the check
# above).
return is_simple_lookup(index + 1, step=1)
return is_simple_lookup(index + 1, kind=1)

return False

Expand All @@ -145,9 +138,9 @@ def is_simple_operand(index: int, kind: Literal["base", "exponent"]) -> bool:
should_hug = (
(0 < idx < len(line.leaves) - 1)
and leaf.type == token.DOUBLESTAR
and is_simple_operand(idx - 1, kind="base")
and is_simple_operand(idx - 1, kind=-1)
and line.leaves[idx - 1].value != "lambda"
and is_simple_operand(idx + 1, kind="exponent")
and is_simple_operand(idx + 1, kind=1)
)
if should_hug:
new_leaf.prefix = ""
Expand All @@ -162,6 +155,99 @@ def is_simple_operand(index: int, kind: Literal["base", "exponent"]) -> bool:
yield new_line


def original_is_simple_lookup_func(
line: Line, index: int, step: Literal[1, -1]
) -> bool:
if step == -1:
disallowed = {token.RPAR, token.RSQB}
else:
disallowed = {token.LPAR, token.LSQB}

while 0 <= index < len(line.leaves):
current = line.leaves[index]
if current.type in disallowed:
return False
if current.type not in {token.NAME, token.DOT} or current.value == "for":
# If the current token isn't disallowed, we'll assume this is
# simple as only the disallowed tokens are semantically
# attached to this lookup expression we're checking. Also,
# stop early if we hit the 'for' bit of a comprehension.
return True

index += step

return True


def handle_is_simple_look_up_prev(line: Line, index: int, disallowed: Set[int]) -> bool:
"""
Handling the determination of is_simple_lookup for the lines prior to the doublestar
token. This is required because of the need to isolate the chained expression
to determine the bracket or parenthesis belong to the single expression.
"""
contains_disallowed = False
chain = []

while 0 <= index < len(line.leaves):
current = line.leaves[index]
chain.append(current)
if not contains_disallowed and current.type in disallowed:
contains_disallowed = True
if not is_expression_chained(chain):
return not contains_disallowed

index -= 1

return True


def handle_is_simple_lookup_forward(
line: Line, index: int, disallowed: Set[int]
) -> bool:
"""
Handling decision is_simple_lookup for the lines behind the doublestar token.
This function is simplified to keep consistent with the prior logic and the forward
case are more straightforward and do not need to care about chained expressions.
"""
while 0 <= index < len(line.leaves):
current = line.leaves[index]
if current.type in disallowed:
return False
if current.type not in {token.NAME, token.DOT} or (
current.type == token.NAME and current.value == "for"
):
# If the current token isn't disallowed, we'll assume this is simple as
# only the disallowed tokens are semantically attached to this lookup
# expression we're checking. Also, stop early if we hit the 'for' bit
# of a comprehension.
return True

index += 1

return True


def is_expression_chained(chained_leaves: List[Leaf]) -> bool:
"""
Function to determine if the variable is a chained call.
(e.g., foo.lookup, foo().lookup, (foo.lookup())) will be recognized as chained call)
"""
if len(chained_leaves) < 2:
return True

current_leaf = chained_leaves[-1]
past_leaf = chained_leaves[-2]

if past_leaf.type == token.NAME:
return current_leaf.type in {token.DOT}
elif past_leaf.type in {token.RPAR, token.RSQB}:
return current_leaf.type in {token.RSQB, token.RPAR}
elif past_leaf.type in {token.LPAR, token.LSQB}:
return current_leaf.type in {token.NAME, token.LPAR, token.LSQB}
else:
return False


class StringTransformer(ABC):
"""
An implementation of the Transformer protocol that relies on its
Expand Down
14 changes: 14 additions & 0 deletions tests/data/cases/is_simple_lookup_for_doublestar_expression.py
@@ -0,0 +1,14 @@
# flags: --preview
m2 = None if not isinstance(dist, Normal) else m** 2 + s * 2
m3 = None if not isinstance(dist, Normal) else m ** 2 + s * 2
m4 = None if not isinstance(dist, Normal) else m**2 + s * 2
m5 = obj.method(another_obj.method()).attribute **2
m6 = None if ... else m**2 + s**2


# output
m2 = None if not isinstance(dist, Normal) else m**2 + s * 2
m3 = None if not isinstance(dist, Normal) else m**2 + s * 2
m4 = None if not isinstance(dist, Normal) else m**2 + s * 2
m5 = obj.method(another_obj.method()).attribute ** 2
m6 = None if ... else m**2 + s**2