Skip to content

Refactor whitespace around operator#4223

Merged
MichaReiser merged 4 commits intomainfrom
refactor-whitespace-around-operator
May 5, 2023
Merged

Refactor whitespace around operator#4223
MichaReiser merged 4 commits intomainfrom
refactor-whitespace-around-operator

Conversation

@MichaReiser
Copy link
Member

@MichaReiser MichaReiser commented May 4, 2023

This PR refactors the pycodestyle missing whitespace around operator rule and fixes a few bug where our implementation didn't match the behavior of the pycodestyle rule. My main motivation for the refactor was that I simply didn't understood the rule and failed to set the right diagnostic ranges in #4113. I find the new logic easier to understand because it only uses token specific state, but some may disagree with that.

The rule itself isn't too complicated: It ensures that operators have consistent leading and trailing spaces (an operator either has leading and trailing spaces or not) and enforces mandatory spaces around some operators.

The misunderstanding that I had for a long time is that we would want to use the Modulo diagnostic whenever some spacing with the % operator is off (and the same for bitwise operations and arithmetic operations). But that's not the case. The logic is:

  • Use MissingWhitespaceAroundOperator if the leading and trailing spaces are not consistent regardless of the operator
  • Use MissingWhitespaceAroundAritmeticOperator, MissingWhitespaceAroundBitwiseOrShiftOperator, or MissingWhitespaceAroundModuloOperator if the operator has no spacing and the operator is an arithmetic operator, a bitwise or shift operator, or the modulo operator (enforce spaces).
  • Use MissingWhitespaceAroundOperator otherwise.

That means the logic is staged: first test for consistent spacing, and only then enforce missing whitespace with the operator-specific violation.

@MichaReiser
Copy link
Member Author

MichaReiser commented May 4, 2023

Current dependencies on/for this PR:

This comment was auto-generated by Graphite.

match kind {
TokenKind::Lpar | TokenKind::Lambda => parens += 1,
TokenKind::Rpar => parens -= 1,
TokenKind::Rpar => parens = parens.saturating_sub(1),
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Don't panic if there are unbalanced parens.

@MichaReiser MichaReiser marked this pull request as ready for review May 4, 2023 13:55
@github-actions
Copy link
Contributor

github-actions bot commented May 4, 2023

PR Check Results

Ecosystem

✅ ecosystem check detected no changes.

Benchmark

Linux

group                                      main                                   pr
-----                                      ----                                   --
linter/all-rules/large/dataset.py          1.04     14.5±0.08ms     2.8 MB/sec    1.00     13.9±0.12ms     2.9 MB/sec
linter/all-rules/numpy/ctypeslib.py        1.03      3.5±0.01ms     4.8 MB/sec    1.00      3.4±0.01ms     5.0 MB/sec
linter/all-rules/numpy/globals.py          1.02    424.9±0.86µs     6.9 MB/sec    1.00    417.3±1.19µs     7.1 MB/sec
linter/all-rules/pydantic/types.py         1.04      6.0±0.02ms     4.2 MB/sec    1.00      5.8±0.01ms     4.4 MB/sec
linter/default-rules/large/dataset.py      1.09      7.5±0.01ms     5.4 MB/sec    1.00      6.9±0.01ms     5.9 MB/sec
linter/default-rules/numpy/ctypeslib.py    1.06   1576.8±2.23µs    10.6 MB/sec    1.00   1490.4±4.08µs    11.2 MB/sec
linter/default-rules/numpy/globals.py      1.04    174.6±0.47µs    16.9 MB/sec    1.00    167.9±0.80µs    17.6 MB/sec
linter/default-rules/pydantic/types.py     1.06      3.3±0.01ms     7.7 MB/sec    1.00      3.1±0.01ms     8.2 MB/sec
parser/large/dataset.py                    1.01      5.5±0.00ms     7.4 MB/sec    1.00      5.4±0.01ms     7.5 MB/sec
parser/numpy/ctypeslib.py                  1.01   1066.1±2.75µs    15.6 MB/sec    1.00   1054.7±8.88µs    15.8 MB/sec
parser/numpy/globals.py                    1.00    108.3±0.18µs    27.2 MB/sec    1.00    107.9±0.49µs    27.3 MB/sec
parser/pydantic/types.py                   1.01      2.3±0.00ms    11.0 MB/sec    1.00      2.3±0.01ms    11.1 MB/sec

Windows

group                                      main                                   pr
-----                                      ----                                   --
linter/all-rules/large/dataset.py          1.00     16.4±0.14ms     2.5 MB/sec    1.04     17.1±0.52ms     2.4 MB/sec
linter/all-rules/numpy/ctypeslib.py        1.00      4.1±0.06ms     4.0 MB/sec    1.08      4.5±0.23ms     3.7 MB/sec
linter/all-rules/numpy/globals.py          1.00    490.9±5.61µs     6.0 MB/sec    1.05   517.8±41.00µs     5.7 MB/sec
linter/all-rules/pydantic/types.py         1.00      7.1±0.14ms     3.6 MB/sec    1.05      7.4±0.28ms     3.4 MB/sec
linter/default-rules/large/dataset.py      1.00      8.6±0.13ms     4.7 MB/sec    1.03      8.9±0.25ms     4.6 MB/sec
linter/default-rules/numpy/ctypeslib.py    1.00  1861.3±53.71µs     8.9 MB/sec    1.02  1890.6±65.00µs     8.8 MB/sec
linter/default-rules/numpy/globals.py      1.00    202.9±6.05µs    14.5 MB/sec    1.05   213.6±19.34µs    13.8 MB/sec
linter/default-rules/pydantic/types.py     1.00      3.9±0.10ms     6.6 MB/sec    1.03      4.0±0.18ms     6.4 MB/sec
parser/large/dataset.py                    1.00      6.7±0.10ms     6.1 MB/sec    1.02      6.9±0.17ms     5.9 MB/sec
parser/numpy/ctypeslib.py                  1.00  1270.5±17.20µs    13.1 MB/sec    1.03  1305.0±40.33µs    12.8 MB/sec
parser/numpy/globals.py                    1.00    130.4±1.57µs    22.6 MB/sec    1.03    134.6±5.28µs    21.9 MB/sec
parser/pydantic/types.py                   1.00      2.8±0.06ms     9.0 MB/sec    1.02      2.9±0.06ms     8.8 MB/sec

@MichaReiser MichaReiser merged commit e93f378 into main May 5, 2023
@MichaReiser MichaReiser deleted the refactor-whitespace-around-operator branch May 5, 2023 07:37
MichaReiser pushed a commit that referenced this pull request Jul 27, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants