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

[Ruff 0.5] Stabilise 11 FURB rules #12043

Merged
merged 4 commits into from
Jun 26, 2024
Merged

Conversation

AlexWaygood
Copy link
Member

@AlexWaygood AlexWaygood commented Jun 26, 2024

Summary

Stabilise the following rules for release 0.5:

  • print-empty-string (FURB105)
  • readlines-in-for (FURB129)
  • if-expr-min-max (FURB136)
  • math-constant (FURB152)
  • bit-count (FURB161)
  • redundant-log-base (FURB163)
  • regex-flag-alias (FURB167)
  • isinstance-type-none (FURB168)
  • type-none-comparison (FURB169)
  • implicit-cwd (FURB177)
  • hashlib-digest-hex (FURB181)
  • list-reverse-copy (FURB187)

These rules have all been in preview for over 90 days and several minor releases. There are no open issues about any of them, and there have not been any open issues about any of them for many months. For each rule, the documentation gives an accurate description of what the rule does and the implementation seems sound.

These are all stylistic rules, but I think their recommendations are all fairly uncontroverisal. Several other Refurb rules have been kept in preview for this release, as there are either outstanding issues with the implementation or their recommendations are (in my opinion) highly opinionated/controversial.

Test Plan

cargo test -p ruff_linter

@AlexWaygood AlexWaygood added the rule Implementing or modifying a lint rule label Jun 26, 2024
Copy link
Contributor

github-actions bot commented Jun 26, 2024

ruff-ecosystem results

Linter (stable)

ℹ️ ecosystem check detected linter changes. (+42 -0 violations, +0 -0 fixes in 3 projects; 47 projects unchanged)

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

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

+ airflow/jobs/backfill_job_runner.py:923:13: FURB187 [*] Use of assignment of `reversed` on list `dagrun_infos`
+ airflow/providers/apache/kafka/operators/consume.py:158:30: FURB136 [*] Replace `if` expression with `min(messages_left, self.max_batch_size)`
+ airflow/providers/apache/spark/hooks/spark_submit.py:306:19: FURB167 [*] Use of regular expression alias `re.I`
+ airflow/providers/cncf/kubernetes/executors/kubernetes_executor_utils.py:149:17: PERF403 Use a dictionary comprehension instead of a for-loop
+ airflow/providers/exasol/hooks/exasol.py:68:17: PERF403 Use a dictionary comprehension instead of a for-loop
+ airflow/providers/mysql/hooks/mysql.py:171:17: PERF403 Use a dictionary comprehension instead of a for-loop
+ airflow/providers/postgres/hooks/postgres.py:173:17: PERF403 Use a dictionary comprehension instead of a for-loop
+ dev/breeze/src/airflow_breeze/commands/release_management_commands.py:2486:13: PERF403 Use a dictionary comprehension instead of a for-loop
+ dev/breeze/src/airflow_breeze/global_constants.py:401:21: FURB129 Instead of calling `readlines()`, iterate over file object directly
+ dev/perf/sql_queries.py:143:41: FURB129 Instead of calling `readlines()`, iterate over file object directly
+ docs/exts/redirects.py:44:21: FURB129 Instead of calling `readlines()`, iterate over file object directly
+ scripts/ci/pre_commit/newsfragments.py:36:43: FURB129 Instead of calling `readlines()`, iterate over file object directly
+ scripts/ci/testing/summarize_captured_warnings.py:246:11: FURB177 Prefer `Path.cwd()` over `Path().resolve()` for current-directory lookups
+ tests/providers/apache/spark/hooks/test_spark_submit.py:74:17: PERF403 Use a dictionary comprehension instead of a for-loop
... 1 additional changes omitted for rule PERF403
+ tests/system/providers/google/cloud/gcs/resources/transform_script.py:26:39: FURB129 Instead of calling `readlines()`, iterate over file object directly

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

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

+ src/bokeh/core/property/visual.py:109:116: FURB167 [*] Use of regular expression alias `re.I`
+ src/bokeh/embed/util.py:366:60: FURB167 [*] Use of regular expression alias `re.S`
+ src/bokeh/embed/util.py:381:60: FURB167 [*] Use of regular expression alias `re.S`

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

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

+ tools/lib/provision_inner.py:101:51: FURB129 Instead of calling `readlines()`, iterate over file object directly
+ zerver/lib/compatibility.py:46:60: FURB167 [*] Use of regular expression alias `re.X`
+ zerver/lib/compatibility.py:97:48: FURB167 [*] Use of regular expression alias `re.I`
+ zerver/lib/compatibility.py:97:55: FURB167 [*] Use of regular expression alias `re.X`
+ zerver/lib/compatibility.py:99:61: FURB167 [*] Use of regular expression alias `re.I`
+ zerver/lib/compatibility.py:99:68: FURB167 [*] Use of regular expression alias `re.X`
+ zerver/lib/markdown/include.py:30:48: FURB167 [*] Use of regular expression alias `re.M`
+ zerver/lib/user_agent.py:12:5: FURB167 [*] Use of regular expression alias `re.X`
+ zerver/management/commands/create_default_stream_groups.py:67:13: FURB105 [*] Unnecessary empty string passed to `print`
+ zerver/management/commands/deactivate_user.py:38:9: FURB105 [*] Unnecessary empty string passed to `print`
+ zerver/management/commands/delete_old_unclaimed_attachments.py:64:13: FURB105 [*] Unnecessary empty string passed to `print`
+ zerver/management/commands/delete_old_unclaimed_attachments.py:68:13: FURB105 [*] Unnecessary empty string passed to `print`
+ zerver/management/commands/delete_old_unclaimed_attachments.py:72:13: FURB105 [*] Unnecessary empty string passed to `print`
+ zerver/management/commands/register_server.py:103:13: FURB105 [*] Unnecessary empty string passed to `print`
+ zerver/migrations/0041_create_attachments_for_old_messages.py:14:52: FURB167 [*] Use of regular expression alias `re.M`
+ zerver/migrations/0371_invalid_characters_in_topics.py:35:5: FURB105 [*] Unnecessary empty string passed to `print`
+ zerver/migrations/0375_invalid_characters_in_stream_names.py:33:5: FURB105 [*] Unnecessary empty string passed to `print`
+ zerver/migrations/0383_revoke_invitations_from_deactivated_users.py:47:5: FURB105 [*] Unnecessary empty string passed to `print`
+ zerver/migrations/0423_fix_email_gateway_attachment_owner.py:40:5: FURB105 [*] Unnecessary empty string passed to `print`
... 3 additional changes omitted for rule FURB105
+ zerver/openapi/markdown_extension.py:173:42: FURB167 [*] Use of regular expression alias `re.M`
+ zerver/openapi/markdown_extension.py:173:49: FURB167 [*] Use of regular expression alias `re.S`
... 2 additional changes omitted for rule FURB167

Changes by rule (7 rules affected)

code total + violation - violation + fix - fix
FURB167 15 15 0 0 0
FURB105 12 12 0 0 0
PERF403 6 6 0 0 0
FURB129 6 6 0 0 0
FURB187 1 1 0 0 0
FURB136 1 1 0 0 0
FURB177 1 1 0 0 0

Linter (preview)

✅ ecosystem check detected no linter changes.

@AlexWaygood
Copy link
Member Author

Having seen the ecosystem report, I'll remove math-constant from the proposed stabilisations. It seems it has quite a few false positives.

@AlexWaygood AlexWaygood changed the title [Ruff 0.5] Stabilise 12 FURB rules [Ruff 0.5] Stabilise 11 FURB rules Jun 26, 2024
@AlexWaygood
Copy link
Member Author

Having seen the ecosystem report, I'll remove math-constant from the proposed stabilisations. It seems it has quite a few false positives.

Now that I've made that change, the updated ecosystem hits all LGTM. I think in each instance, following the recommendation would be a reasonably uncontroversial style/readability improvement.

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.

These rules are quite complex so I anticipate some issues. But we're clearly not getting more issues by keeping them in preview at this point, so seems fine.

@AlexWaygood AlexWaygood merged commit d9a44de into ruff-0.5 Jun 26, 2024
20 checks passed
@AlexWaygood AlexWaygood deleted the refurb-stabilisations branch June 26, 2024 12:24
@MichaReiser MichaReiser mentioned this pull request Jun 26, 2024
MichaReiser pushed a commit that referenced this pull request Jun 27, 2024
MichaReiser pushed a commit that referenced this pull request Jun 27, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
rule Implementing or modifying a lint rule
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants