Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

[refurb] implement hardcoded-string-charset (FURB156) #13530

Merged
merged 5 commits into from
Oct 7, 2024

Conversation

alex-700
Copy link
Contributor

Summary

Implement hardcoded-string-charset (FURB156)
See:

Test Plan

cargo test

Copy link
Contributor

github-actions bot commented Sep 26, 2024

ruff-ecosystem results

Linter (stable)

✅ ecosystem check detected no linter changes.

Linter (preview)

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

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

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

+ dev/breeze/src/airflow_breeze/utils/packages.py:433:48: FURB156 [*] Use of hardcoded string charset

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

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

+ examples/reference/models/Text.py:9:5: FURB156 [*] Use of hardcoded string charset
+ tests/unit/bokeh/util/test_token.py:69:20: FURB156 [*] Use of hardcoded string charset

Changes by rule (1 rules affected)

code total + violation - violation + fix - fix
FURB156 3 3 0 0 0

@zanieb zanieb self-assigned this Sep 26, 2024
@MichaReiser MichaReiser added rule Implementing or modifying a lint rule preview Related to preview mode features labels Sep 27, 2024
Copy link
Member

@MichaReiser MichaReiser left a comment

Choose a reason for hiding this comment

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

Thanks. This looks great.

I left a few smaller comments. It would also be great to add some documentation to Charset and bitset, considering that the representations are non-trivial and make some assumptions about how they're used.

Copy link
Member

@MichaReiser MichaReiser left a comment

Choose a reason for hiding this comment

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

Nice, thanks for following up. Let's add a test for when the expression is parenthesized to verify that the fix is correct (if not, take a look at parenthesized_range.

I also think that we may be able to remove the factory methods on AsciiCharSet and reduce them to just one.

Copy link
Member

@MichaReiser MichaReiser left a comment

Choose a reason for hiding this comment

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

Nice thanks. This looks great!

@MichaReiser MichaReiser enabled auto-merge (squash) October 7, 2024 07:26
@MichaReiser MichaReiser merged commit 73aa6ea into astral-sh:main Oct 7, 2024
17 checks passed
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
preview Related to preview mode features rule Implementing or modifying a lint rule
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants