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

[pycodestyle] Do not trigger E225 and E275 when the next token is a ')' #10315

Merged
merged 5 commits into from Mar 11, 2024

Conversation

hoel-bagard
Copy link
Contributor

@hoel-bagard hoel-bagard commented Mar 9, 2024

Summary

Fixes #10295.

E225 (Missing whitespace around operator) and E275 (Missing whitespace after keyword) try to add a white space even when the next character is a ) (which is a syntax error in most cases, the exceptions already being handled). This causes E202 (Whitespace before close bracket) to try to remove the added whitespace, resulting in an infinite loop when E225/E275 re-add it.
This PR adds an exception in E225 and E275 to not trigger in case the next token is a ). It is a bit simplistic, but it solves the example given in the issue without introducing a change in behavior (according to the fixtures).

Test Plan

cargo test and the ruff-ecosystem check were used to check that the PR's changes do not have side-effects.
A new fixture was added to check that running the 3 rules on the example given in the issue does not cause ruff to fail to converge.

Copy link

github-actions bot commented Mar 9, 2024

ruff-ecosystem results

Linter (stable)

ℹ️ ecosystem check detected linter changes. (+8 -0 violations, +0 -0 fixes in 4 projects; 39 projects unchanged)

DisnakeDev/disnake (+2 -0 violations, +0 -0 fixes)

+ disnake/backoff.py:45:16: S311 Standard pseudo-random generators are not suitable for cryptographic purposes
+ disnake/colour.py:135:44: S311 Standard pseudo-random generators are not suitable for cryptographic purposes

apache/airflow (+2 -0 violations, +0 -0 fixes)

ruff check --no-cache --exit-zero --ignore RUF9 --output-format concise --no-preview --select ALL

+ airflow/dag_processing/manager.py:1156:13: S311 Standard pseudo-random generators are not suitable for cryptographic purposes
+ tests/dag_processing/test_job_runner.py:363:9: S311 Standard pseudo-random generators are not suitable for cryptographic purposes

model-bakers/model_bakery (+1 -0 violations, +0 -0 fixes)

+ model_bakery/random_gen.py:30:16: S311 Standard pseudo-random generators are not suitable for cryptographic purposes

zulip/zulip (+3 -0 violations, +0 -0 fixes)

ruff check --no-cache --exit-zero --ignore RUF9 --output-format concise --no-preview --select ALL

+ analytics/lib/fixtures.py:39:11: S311 Standard pseudo-random generators are not suitable for cryptographic purposes
+ tools/lib/provision.py:280:60: S605 Starting a process with a shell: seems safe, but may be changed in the future; consider rewriting without `shell`
+ tools/lib/provision.py:280:60: S607 Starting a process with a partial executable path

Changes by rule (3 rules affected)

code total + violation - violation + fix - fix
S311 6 6 0 0 0
S605 1 1 0 0 0
S607 1 1 0 0 0

Linter (preview)

ℹ️ ecosystem check detected linter changes. (+8 -0 violations, +0 -0 fixes in 4 projects; 39 projects unchanged)

DisnakeDev/disnake (+2 -0 violations, +0 -0 fixes)

ruff check --no-cache --exit-zero --ignore RUF9 --output-format concise --preview

+ disnake/backoff.py:45:16: S311 Standard pseudo-random generators are not suitable for cryptographic purposes
+ disnake/colour.py:135:44: S311 Standard pseudo-random generators are not suitable for cryptographic purposes

apache/airflow (+2 -0 violations, +0 -0 fixes)

ruff check --no-cache --exit-zero --ignore RUF9 --output-format concise --preview --select ALL

+ airflow/dag_processing/manager.py:1156:13: S311 Standard pseudo-random generators are not suitable for cryptographic purposes
+ tests/dag_processing/test_job_runner.py:363:9: S311 Standard pseudo-random generators are not suitable for cryptographic purposes

model-bakers/model_bakery (+1 -0 violations, +0 -0 fixes)

ruff check --no-cache --exit-zero --ignore RUF9 --output-format concise --preview

+ model_bakery/random_gen.py:30:16: S311 Standard pseudo-random generators are not suitable for cryptographic purposes

zulip/zulip (+3 -0 violations, +0 -0 fixes)

ruff check --no-cache --exit-zero --ignore RUF9 --output-format concise --preview --select ALL

+ analytics/lib/fixtures.py:39:11: S311 Standard pseudo-random generators are not suitable for cryptographic purposes
+ tools/lib/provision.py:280:60: S605 Starting a process with a shell: seems safe, but may be changed in the future; consider rewriting without `shell`
+ tools/lib/provision.py:280:60: S607 Starting a process with a partial executable path

Changes by rule (3 rules affected)

code total + violation - violation + fix - fix
S311 6 6 0 0 0
S605 1 1 0 0 0
S607 1 1 0 0 0

Formatter (stable)

✅ ecosystem check detected no format changes.

Formatter (preview)

✅ ecosystem check detected no format changes.

@hoel-bagard hoel-bagard marked this pull request as ready for review March 10, 2024 02:19
@charliermarsh charliermarsh enabled auto-merge (squash) March 11, 2024 21:16
Copy link
Member

@charliermarsh charliermarsh left a comment

Choose a reason for hiding this comment

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

That's great, thanks!

@charliermarsh charliermarsh added the bug Something isn't working label Mar 11, 2024
@charliermarsh charliermarsh merged commit 8d73866 into astral-sh:main Mar 11, 2024
17 checks passed
@hoel-bagard hoel-bagard deleted the hoel/fix_10295 branch March 11, 2024 22:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Infinite loop with invalid syntax
2 participants