Avoid flagging missing whitespace for decorators #4454

charliermarsh · 2023-05-16T16:46:50Z

No description provided.

crates/ruff_python_ast/src/token_kind.rs

MichaReiser

This is a deviation from pycodestyle, right?

charliermarsh · 2023-05-16T16:52:39Z

This is a fix that corrects a deviation from pycodestyle. It brings our behavior in-line with pycodestyle.

charliermarsh · 2023-05-16T16:54:24Z

(Sorry for the wordy answer, I can't tell if you're asking whether the old or new behavior is the deviation.)

MichaReiser · 2023-05-16T16:55:58Z

crates/ruff/src/rules/pycodestyle/rules/logical_lines/missing_whitespace_around_operator.rs

+                kind,
+                TokenKind::Star | TokenKind::DoubleStar | TokenKind::At
+            )
+        {
            let is_binary = prev_token.map_or(false, |prev_token| {
                let prev_kind = prev_token.kind();


An you explain the fix. It's unclear to me how it works. I would have expected that the @ always goes into the else branch that requires no space, and thus, doesn't create any diagnostics

@ can be used as a binary operator in Python -- it represents matrix multiplication:

#: E225 i = 1 @2 #: E225 i = 1@ 2

So equivalently to * or +, we need to detect whether we're seeing a decorator, or a binary operator.

I think in practice, for this particular piece of syntax, it will be a "unary" token IFF it's the first token in the logical line.

Oh wow, I didn't know!

Hmm, I'm wondering right now if my refactored implementation misses the handling for WS_OPTIONAL. Because the @ is part of the is_whitespace required, which returns NeedsSpace::Yes rather than setting it to NeedsSpace::Optional

https://github.com/PyCQA/pycodestyle/blob/f6500e2170406b664a721b5e3a0f5d66bcb757e1/pycodestyle.py#L108

Hmm, I will take a look in a quick follow-up PR (or you can, if you're already working on it?).

I'm not and would appreciate it if you can have a look. I believe we're missing the whole optional branch because pycodestyle has no explicit handling of the @ in the unary branch.

Okay, I now remember briefly what is happening. The arithmetic and bitwise aren't optional. It's just that they are different rules (handled here https://github.com/PyCQA/pycodestyle/blob/f6500e2170406b664a721b5e3a0f5d66bcb757e1/pycodestyle.py#LL914C26-L920C66)

What I don't understand right now is why @ would require any special treatment (more specifically, why we need where pycodestyle has none)

github-actions · 2023-05-16T17:08:07Z

PR Check Results

Ecosystem

✅ ecosystem check detected no changes.

Benchmark

Linux

group                                      main                                   pr
-----                                      ----                                   --
linter/all-rules/large/dataset.py          1.00     16.6±0.18ms     2.4 MB/sec    1.00     16.6±0.17ms     2.4 MB/sec
linter/all-rules/numpy/ctypeslib.py        1.00      4.0±0.05ms     4.1 MB/sec    1.01      4.1±0.04ms     4.1 MB/sec
linter/all-rules/numpy/globals.py          1.00    505.0±4.44µs     5.8 MB/sec    1.00    506.7±3.88µs     5.8 MB/sec
linter/all-rules/pydantic/types.py         1.00      6.9±0.07ms     3.7 MB/sec    1.01      6.9±0.06ms     3.7 MB/sec
linter/default-rules/large/dataset.py      1.00      8.0±0.07ms     5.1 MB/sec    1.00      8.0±0.08ms     5.1 MB/sec
linter/default-rules/numpy/ctypeslib.py    1.00  1726.9±20.76µs     9.6 MB/sec    1.01  1738.3±16.46µs     9.6 MB/sec
linter/default-rules/numpy/globals.py      1.00    192.9±2.70µs    15.3 MB/sec    1.02    195.8±2.63µs    15.1 MB/sec
linter/default-rules/pydantic/types.py     1.00      3.6±0.04ms     7.2 MB/sec    1.02      3.6±0.03ms     7.1 MB/sec
parser/large/dataset.py                    1.02      6.4±0.05ms     6.4 MB/sec    1.00      6.3±0.05ms     6.5 MB/sec
parser/numpy/ctypeslib.py                  1.01  1247.3±14.00µs    13.4 MB/sec    1.00  1233.7±14.80µs    13.5 MB/sec
parser/numpy/globals.py                    1.00    127.2±1.95µs    23.2 MB/sec    1.00    126.7±1.72µs    23.3 MB/sec
parser/pydantic/types.py                   1.01      2.7±0.03ms     9.4 MB/sec    1.00      2.7±0.03ms     9.5 MB/sec

Windows

group                                      main                                   pr
-----                                      ----                                   --
linter/all-rules/large/dataset.py          1.00     18.8±0.34ms     2.2 MB/sec    1.00     18.7±0.35ms     2.2 MB/sec
linter/all-rules/numpy/ctypeslib.py        1.03      4.7±0.13ms     3.6 MB/sec    1.00      4.5±0.12ms     3.7 MB/sec
linter/all-rules/numpy/globals.py          1.01   534.6±10.92µs     5.5 MB/sec    1.00   526.8±11.08µs     5.6 MB/sec
linter/all-rules/pydantic/types.py         1.00      7.8±0.26ms     3.3 MB/sec    1.00      7.8±0.13ms     3.3 MB/sec
linter/default-rules/large/dataset.py      1.00      8.8±0.27ms     4.6 MB/sec    1.00      8.9±0.13ms     4.6 MB/sec
linter/default-rules/numpy/ctypeslib.py    1.00  1830.3±40.73µs     9.1 MB/sec    1.01  1840.5±24.71µs     9.0 MB/sec
linter/default-rules/numpy/globals.py      1.00    204.3±6.52µs    14.4 MB/sec    1.01    206.1±6.98µs    14.3 MB/sec
linter/default-rules/pydantic/types.py     1.00      3.9±0.06ms     6.6 MB/sec    1.00      3.9±0.06ms     6.6 MB/sec
parser/large/dataset.py                    1.02      7.0±0.10ms     5.8 MB/sec    1.00      6.9±0.10ms     5.9 MB/sec
parser/numpy/ctypeslib.py                  1.02  1330.2±31.84µs    12.5 MB/sec    1.00  1301.7±28.86µs    12.8 MB/sec
parser/numpy/globals.py                    1.03    135.7±3.54µs    21.7 MB/sec    1.00    132.4±2.30µs    22.3 MB/sec
parser/pydantic/types.py                   1.02      3.0±0.04ms     8.6 MB/sec    1.00      2.9±0.04ms     8.8 MB/sec

charliermarsh force-pushed the charlie/dec branch from fd763cf to 90683c1 Compare May 16, 2023 16:47

charliermarsh enabled auto-merge (squash) May 16, 2023 16:50

MichaReiser reviewed May 16, 2023

View reviewed changes

crates/ruff_python_ast/src/token_kind.rs Outdated Show resolved Hide resolved

charliermarsh disabled auto-merge May 16, 2023 16:51

MichaReiser reviewed May 16, 2023

View reviewed changes

charliermarsh force-pushed the charlie/dec branch 2 times, most recently from ebbb35e to d23197a Compare May 16, 2023 16:55

MichaReiser reviewed May 16, 2023

View reviewed changes

charliermarsh force-pushed the charlie/dec branch from d23197a to 6ee2666 Compare May 16, 2023 16:56

Avoid flagging missing whitespace for decorators

e056926

charliermarsh force-pushed the charlie/dec branch from 6ee2666 to e056926 Compare May 16, 2023 16:59

charliermarsh merged commit d9c3f8e into main May 16, 2023

charliermarsh deleted the charlie/dec branch May 16, 2023 17:15

charliermarsh mentioned this pull request May 16, 2023

Remove special-casing for whitespace-around-@ #4458

Merged

chenrui333 mentioned this pull request May 18, 2023

ruff 0.0.268 Homebrew/homebrew-core#131348

Merged

p-linnane mentioned this pull request May 18, 2023

ruff 0.0.269 Homebrew/homebrew-core#131367

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Avoid flagging missing whitespace for decorators #4454

Avoid flagging missing whitespace for decorators #4454

charliermarsh commented May 16, 2023

MichaReiser left a comment

charliermarsh commented May 16, 2023

charliermarsh commented May 16, 2023

MichaReiser May 16, 2023

charliermarsh May 16, 2023

charliermarsh May 16, 2023

MichaReiser May 16, 2023 •

edited

Loading

charliermarsh May 16, 2023

MichaReiser May 16, 2023

MichaReiser May 16, 2023

github-actions bot commented May 16, 2023 •

edited

Loading

Avoid flagging missing whitespace for decorators #4454

Avoid flagging missing whitespace for decorators #4454

Conversation

charliermarsh commented May 16, 2023

MichaReiser left a comment

Choose a reason for hiding this comment

charliermarsh commented May 16, 2023

charliermarsh commented May 16, 2023

MichaReiser May 16, 2023

Choose a reason for hiding this comment

charliermarsh May 16, 2023

Choose a reason for hiding this comment

charliermarsh May 16, 2023

Choose a reason for hiding this comment

MichaReiser May 16, 2023 • edited Loading

Choose a reason for hiding this comment

charliermarsh May 16, 2023

Choose a reason for hiding this comment

MichaReiser May 16, 2023

Choose a reason for hiding this comment

MichaReiser May 16, 2023

Choose a reason for hiding this comment

github-actions bot commented May 16, 2023 • edited Loading

PR Check Results

Ecosystem

Benchmark

Linux

Windows

MichaReiser May 16, 2023 •

edited

Loading

github-actions bot commented May 16, 2023 •

edited

Loading