Avoid flagging missing whitespace for decorators by charliermarsh · Pull Request #4454 · astral-sh/ruff

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

+            )
+        {
            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 merged 1 commit intomainfrom
charlie/dec

charliermarsh commented May 16, 2023

Uh oh!

Uh oh!

MichaReiser left a comment

Uh oh!

charliermarsh commented May 16, 2023

Uh oh!

charliermarsh commented May 16, 2023

Uh oh!

MichaReiser May 16, 2023

Uh oh!

charliermarsh May 16, 2023

Uh oh!

charliermarsh May 16, 2023

Uh oh!

MichaReiser May 16, 2023 •

edited

Loading

Uh oh!

charliermarsh May 16, 2023

Uh oh!

MichaReiser May 16, 2023

Uh oh!

MichaReiser May 16, 2023

Uh oh!

github-actions bot commented May 16, 2023 •

edited

Loading

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Conversation

charliermarsh commented May 16, 2023

Uh oh!

Uh oh!

MichaReiser left a comment

Choose a reason for hiding this comment

Uh oh!

charliermarsh commented May 16, 2023

Uh oh!

charliermarsh commented May 16, 2023

Uh oh!

MichaReiser May 16, 2023

Choose a reason for hiding this comment

Uh oh!

charliermarsh May 16, 2023

Choose a reason for hiding this comment

Uh oh!

charliermarsh May 16, 2023

Choose a reason for hiding this comment

Uh oh!

MichaReiser May 16, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

charliermarsh May 16, 2023

Choose a reason for hiding this comment

Uh oh!

MichaReiser May 16, 2023

Choose a reason for hiding this comment

Uh oh!

MichaReiser May 16, 2023

Choose a reason for hiding this comment

Uh oh!

github-actions bot commented May 16, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

PR Check Results

Ecosystem

Benchmark

Linux

Windows

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

MichaReiser May 16, 2023 •

edited

Loading

github-actions bot commented May 16, 2023 •

edited

Loading