Skip to content

Commit

Permalink
Hug power operators if its operands are "simple" (#2726)
Browse files Browse the repository at this point in the history
Since power operators almost always have the highest binding power in expressions, it's often more readable to hug it with its operands. The main exception to this is when its operands are non-trivial in which case the power operator will not hug, the rule for this is the following:

> For power ops, an operand is considered "simple" if it's only a NAME, numeric CONSTANT, or attribute access (chained attribute access is allowed), with or without a preceding unary operator. 

Fixes GH-538.
Closes GH-2095.

diff-shades results: https://gist.github.com/ichard26/ca6c6ad4bd1de5152d95418c8645354b

Co-authored-by: Diego <dpalma@evernote.com>
Co-authored-by: Felix Hildén <felix.hilden@gmail.com>
Co-authored-by: Jelle Zijlstra <jelle.zijlstra@gmail.com>
  • Loading branch information
4 people committed Jan 25, 2022
1 parent 73cb6e7 commit 6417c99
Show file tree
Hide file tree
Showing 13 changed files with 293 additions and 51 deletions.
1 change: 1 addition & 0 deletions CHANGES.md
Expand Up @@ -20,6 +20,7 @@
- Tuple unpacking on `return` and `yield` constructs now implies 3.8+ (#2700)
- Unparenthesized tuples on annotated assignments (e.g
`values: Tuple[int, ...] = 1, 2, 3`) now implies 3.8+ (#2708)
- Remove spaces around power operators if both operands are simple (#2726)
- Allow setting custom cache directory on all platforms with environment variable
`BLACK_CACHE_DIR` (#2739).
- Text coloring added in the final statistics (#2712)
Expand Down
20 changes: 20 additions & 0 deletions docs/the_black_code_style/current_style.md
Expand Up @@ -284,6 +284,26 @@ multiple lines. This is so that _Black_ is compliant with the recent changes in
[PEP 8](https://www.python.org/dev/peps/pep-0008/#should-a-line-break-before-or-after-a-binary-operator)
style guide, which emphasizes that this approach improves readability.

Almost all operators will be surrounded by single spaces, the only exceptions are unary
operators (`+`, `-`, and `~`), and power operators when both operands are simple. For
powers, an operand is considered simple if it's only a NAME, numeric CONSTANT, or
attribute access (chained attribute access is allowed), with or without a preceding
unary operator.

```python
# For example, these won't be surrounded by whitespace
a = x**y
b = config.base**5.2
c = config.base**runtime.config.exponent
d = 2**5
e = 2**~5

# ... but these will be surrounded by whitespace
f = 2 ** get_exponent()
g = get_x() ** get_y()
h = config['base'] ** 2
```

### Slices

PEP 8
Expand Down
7 changes: 5 additions & 2 deletions src/black/linegen.py
Expand Up @@ -21,8 +21,8 @@
from black.numerics import normalize_numeric_literal
from black.strings import get_string_prefix, fix_docstring
from black.strings import normalize_string_prefix, normalize_string_quotes
from black.trans import Transformer, CannotTransform, StringMerger
from black.trans import StringSplitter, StringParenWrapper, StringParenStripper
from black.trans import Transformer, CannotTransform, StringMerger, StringSplitter
from black.trans import StringParenWrapper, StringParenStripper, hug_power_op
from black.mode import Mode, Feature, Preview

from blib2to3.pytree import Node, Leaf
Expand Down Expand Up @@ -404,6 +404,9 @@ def _rhs(
transformers = [delimiter_split, standalone_comment_split, rhs]
else:
transformers = [rhs]
# It's always safe to attempt hugging of power operations and pretty much every line
# could match.
transformers.append(hug_power_op)

for transform in transformers:
# We are accumulating lines in `result` because we might want to abort
Expand Down
86 changes: 84 additions & 2 deletions src/black/trans.py
Expand Up @@ -24,9 +24,9 @@
import sys

if sys.version_info < (3, 8):
from typing_extensions import Final
from typing_extensions import Literal, Final
else:
from typing import Final
from typing import Literal, Final

from mypy_extensions import trait

Expand Down Expand Up @@ -71,6 +71,88 @@ def TErr(err_msg: str) -> Err[CannotTransform]:
return Err(cant_transform)


def hug_power_op(line: Line, features: Collection[Feature]) -> Iterator[Line]:
"""A transformer which normalizes spacing around power operators."""

# Performance optimization to avoid unnecessary Leaf clones and other ops.
for leaf in line.leaves:
if leaf.type == token.DOUBLESTAR:
break
else:
raise CannotTransform("No doublestar token was found in the line.")

def is_simple_lookup(index: int, step: 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

index += step

return True

def is_simple_operand(index: int, kind: Literal["base", "exponent"]) -> 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))

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
# for simplicity starting from the next token (so it'll hit the check
# above).
return is_simple_lookup(index + 1, step=1)

return False

leaves: List[Leaf] = []
should_hug = False
for idx, leaf in enumerate(line.leaves):
new_leaf = leaf.clone()
if should_hug:
new_leaf.prefix = ""
should_hug = False

should_hug = (
(0 < idx < len(line.leaves) - 1)
and leaf.type == token.DOUBLESTAR
and is_simple_operand(idx - 1, kind="base")
and line.leaves[idx - 1].value != "lambda"
and is_simple_operand(idx + 1, kind="exponent")
)
if should_hug:
new_leaf.prefix = ""

leaves.append(new_leaf)

yield Line(
mode=line.mode,
depth=line.depth,
leaves=leaves,
comments=line.comments,
bracket_tracker=line.bracket_tracker,
inside_brackets=line.inside_brackets,
should_split_rhs=line.should_split_rhs,
magic_trailing_comma=line.magic_trailing_comma,
)


class StringTransformer(ABC):
"""
An implementation of the Transformer protocol that relies on its
Expand Down
2 changes: 1 addition & 1 deletion src/black_primer/primer.json
Expand Up @@ -81,7 +81,7 @@
},
"flake8-bugbear": {
"cli_arguments": ["--experimental-string-processing"],
"expect_formatting_changes": false,
"expect_formatting_changes": true,
"git_clone_url": "https://github.com/PyCQA/flake8-bugbear.git",
"long_checkout": false,
"py_versions": ["all"]
Expand Down
44 changes: 30 additions & 14 deletions tests/data/expression.diff
Expand Up @@ -11,15 +11,25 @@
True
False
1
@@ -29,63 +29,96 @@
@@ -21,71 +21,104 @@
Name1 or (Name2 and Name3) or Name4
Name1 or Name2 and Name3 or Name4
v1 << 2
1 >> v2
1 % finished
-1 + v2 - v3 * 4 ^ 5 ** v6 / 7 // 8
-((1 + v2) - (v3 * 4)) ^ (((5 ** v6) / 7) // 8)
+1 + v2 - v3 * 4 ^ 5**v6 / 7 // 8
+((1 + v2) - (v3 * 4)) ^ (((5**v6) / 7) // 8)
not great
~great
+value
-1
~int and not v1 ^ 123 + v2 | True
(~int) and (not ((v1 ^ (123 + v2)) | True))
-+really ** -confusing ** ~operator ** -precedence
-flags & ~ select.EPOLLIN and waiters.write_task is not None
++(really ** -(confusing ** ~(operator ** -precedence)))
++(really ** -(confusing ** ~(operator**-precedence)))
+flags & ~select.EPOLLIN and waiters.write_task is not None
lambda arg: None
lambda a=True: a
Expand Down Expand Up @@ -88,15 +98,19 @@
+ *more,
+]
{i for i in (1, 2, 3)}
{(i ** 2) for i in (1, 2, 3)}
-{(i ** 2) for i in (1, 2, 3)}
-{(i ** 2) for i, _ in ((1, 'a'), (2, 'b'), (3, 'c'))}
+{(i ** 2) for i, _ in ((1, "a"), (2, "b"), (3, "c"))}
{((i ** 2) + j) for i in (1, 2, 3) for j in (1, 2, 3)}
-{((i ** 2) + j) for i in (1, 2, 3) for j in (1, 2, 3)}
+{(i**2) for i in (1, 2, 3)}
+{(i**2) for i, _ in ((1, "a"), (2, "b"), (3, "c"))}
+{((i**2) + j) for i in (1, 2, 3) for j in (1, 2, 3)}
[i for i in (1, 2, 3)]
[(i ** 2) for i in (1, 2, 3)]
-[(i ** 2) for i in (1, 2, 3)]
-[(i ** 2) for i, _ in ((1, 'a'), (2, 'b'), (3, 'c'))]
+[(i ** 2) for i, _ in ((1, "a"), (2, "b"), (3, "c"))]
[((i ** 2) + j) for i in (1, 2, 3) for j in (1, 2, 3)]
-[((i ** 2) + j) for i in (1, 2, 3) for j in (1, 2, 3)]
+[(i**2) for i in (1, 2, 3)]
+[(i**2) for i, _ in ((1, "a"), (2, "b"), (3, "c"))]
+[((i**2) + j) for i in (1, 2, 3) for j in (1, 2, 3)]
{i: 0 for i in (1, 2, 3)}
-{i: j for i, j in ((1, 'a'), (2, 'b'), (3, 'c'))}
+{i: j for i, j in ((1, "a"), (2, "b"), (3, "c"))}
Expand Down Expand Up @@ -181,10 +195,12 @@
SomeName
(Good, Bad, Ugly)
(i for i in (1, 2, 3))
((i ** 2) for i in (1, 2, 3))
-((i ** 2) for i in (1, 2, 3))
-((i ** 2) for i, _ in ((1, 'a'), (2, 'b'), (3, 'c')))
+((i ** 2) for i, _ in ((1, "a"), (2, "b"), (3, "c")))
(((i ** 2) + j) for i in (1, 2, 3) for j in (1, 2, 3))
-(((i ** 2) + j) for i in (1, 2, 3) for j in (1, 2, 3))
+((i**2) for i in (1, 2, 3))
+((i**2) for i, _ in ((1, "a"), (2, "b"), (3, "c")))
+(((i**2) + j) for i in (1, 2, 3) for j in (1, 2, 3))
(*starred,)
-{"id": "1","type": "type","started_at": now(),"ended_at": now() + timedelta(days=10),"priority": 1,"import_session_id": 1,**kwargs}
+{
Expand Down Expand Up @@ -403,13 +419,13 @@
+ return True
+if (
+ ~aaaa.a + aaaa.b - aaaa.c * aaaa.d / aaaa.e
+ | aaaa.f & aaaa.g % aaaa.h ^ aaaa.i << aaaa.k >> aaaa.l ** aaaa.m // aaaa.n
+ | aaaa.f & aaaa.g % aaaa.h ^ aaaa.i << aaaa.k >> aaaa.l**aaaa.m // aaaa.n
+):
+ return True
+if (
+ ~aaaaaaaa.a + aaaaaaaa.b - aaaaaaaa.c @ aaaaaaaa.d / aaaaaaaa.e
+ | aaaaaaaa.f & aaaaaaaa.g % aaaaaaaa.h
+ ^ aaaaaaaa.i << aaaaaaaa.k >> aaaaaaaa.l ** aaaaaaaa.m // aaaaaaaa.n
+ ^ aaaaaaaa.i << aaaaaaaa.k >> aaaaaaaa.l**aaaaaaaa.m // aaaaaaaa.n
+):
+ return True
+if (
Expand All @@ -419,7 +435,7 @@
+ | aaaaaaaaaaaaaaaa.f & aaaaaaaaaaaaaaaa.g % aaaaaaaaaaaaaaaa.h
+ ^ aaaaaaaaaaaaaaaa.i
+ << aaaaaaaaaaaaaaaa.k
+ >> aaaaaaaaaaaaaaaa.l ** aaaaaaaaaaaaaaaa.m // aaaaaaaaaaaaaaaa.n
+ >> aaaaaaaaaaaaaaaa.l**aaaaaaaaaaaaaaaa.m // aaaaaaaaaaaaaaaa.n
+):
+ return True
+(
Expand Down
30 changes: 15 additions & 15 deletions tests/data/expression.py
Expand Up @@ -282,15 +282,15 @@ async def f():
v1 << 2
1 >> v2
1 % finished
1 + v2 - v3 * 4 ^ 5 ** v6 / 7 // 8
((1 + v2) - (v3 * 4)) ^ (((5 ** v6) / 7) // 8)
1 + v2 - v3 * 4 ^ 5**v6 / 7 // 8
((1 + v2) - (v3 * 4)) ^ (((5**v6) / 7) // 8)
not great
~great
+value
-1
~int and not v1 ^ 123 + v2 | True
(~int) and (not ((v1 ^ (123 + v2)) | True))
+(really ** -(confusing ** ~(operator ** -precedence)))
+(really ** -(confusing ** ~(operator**-precedence)))
flags & ~select.EPOLLIN and waiters.write_task is not None
lambda arg: None
lambda a=True: a
Expand Down Expand Up @@ -347,13 +347,13 @@ async def f():
*more,
]
{i for i in (1, 2, 3)}
{(i ** 2) for i in (1, 2, 3)}
{(i ** 2) for i, _ in ((1, "a"), (2, "b"), (3, "c"))}
{((i ** 2) + j) for i in (1, 2, 3) for j in (1, 2, 3)}
{(i**2) for i in (1, 2, 3)}
{(i**2) for i, _ in ((1, "a"), (2, "b"), (3, "c"))}
{((i**2) + j) for i in (1, 2, 3) for j in (1, 2, 3)}
[i for i in (1, 2, 3)]
[(i ** 2) for i in (1, 2, 3)]
[(i ** 2) for i, _ in ((1, "a"), (2, "b"), (3, "c"))]
[((i ** 2) + j) for i in (1, 2, 3) for j in (1, 2, 3)]
[(i**2) for i in (1, 2, 3)]
[(i**2) for i, _ in ((1, "a"), (2, "b"), (3, "c"))]
[((i**2) + j) for i in (1, 2, 3) for j in (1, 2, 3)]
{i: 0 for i in (1, 2, 3)}
{i: j for i, j in ((1, "a"), (2, "b"), (3, "c"))}
{a: b * 2 for a, b in dictionary.items()}
Expand Down Expand Up @@ -441,9 +441,9 @@ async def f():
SomeName
(Good, Bad, Ugly)
(i for i in (1, 2, 3))
((i ** 2) for i in (1, 2, 3))
((i ** 2) for i, _ in ((1, "a"), (2, "b"), (3, "c")))
(((i ** 2) + j) for i in (1, 2, 3) for j in (1, 2, 3))
((i**2) for i in (1, 2, 3))
((i**2) for i, _ in ((1, "a"), (2, "b"), (3, "c")))
(((i**2) + j) for i in (1, 2, 3) for j in (1, 2, 3))
(*starred,)
{
"id": "1",
Expand Down Expand Up @@ -588,13 +588,13 @@ async def f():
return True
if (
~aaaa.a + aaaa.b - aaaa.c * aaaa.d / aaaa.e
| aaaa.f & aaaa.g % aaaa.h ^ aaaa.i << aaaa.k >> aaaa.l ** aaaa.m // aaaa.n
| aaaa.f & aaaa.g % aaaa.h ^ aaaa.i << aaaa.k >> aaaa.l**aaaa.m // aaaa.n
):
return True
if (
~aaaaaaaa.a + aaaaaaaa.b - aaaaaaaa.c @ aaaaaaaa.d / aaaaaaaa.e
| aaaaaaaa.f & aaaaaaaa.g % aaaaaaaa.h
^ aaaaaaaa.i << aaaaaaaa.k >> aaaaaaaa.l ** aaaaaaaa.m // aaaaaaaa.n
^ aaaaaaaa.i << aaaaaaaa.k >> aaaaaaaa.l**aaaaaaaa.m // aaaaaaaa.n
):
return True
if (
Expand All @@ -604,7 +604,7 @@ async def f():
| aaaaaaaaaaaaaaaa.f & aaaaaaaaaaaaaaaa.g % aaaaaaaaaaaaaaaa.h
^ aaaaaaaaaaaaaaaa.i
<< aaaaaaaaaaaaaaaa.k
>> aaaaaaaaaaaaaaaa.l ** aaaaaaaaaaaaaaaa.m // aaaaaaaaaaaaaaaa.n
>> aaaaaaaaaaaaaaaa.l**aaaaaaaaaaaaaaaa.m // aaaaaaaaaaaaaaaa.n
):
return True
(
Expand Down

0 comments on commit 6417c99

Please sign in to comment.