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

[flake8-pyi] Teach various rules that annotations might be stringized #12951

Merged
merged 1 commit into from
Sep 2, 2024

Conversation

AlexWaygood
Copy link
Member

Summary

Fixes #12928.

Most flake8-pyi rules don't currently consider the possibility that annotations can be stringized -- foo: "typing.Any" means the same thing to a type checker as foo: typing.Any. This PR adds a general-purpose helper to ruff_linter/src/rules/flake8_pyi/rules/helpers.rs, and uses it in several flake8-pyi rules to reduce false positives and false negatives on stringized annotations.

I haven't attempted to exhaustively account for stringized annotations in every flake8-pyi rule. In some rules, this would have significantly complicated the logic, for marginal gain.

Test Plan

Several new fixtures and snapshots added.

Copy link
Contributor

github-actions bot commented Aug 17, 2024

ruff-ecosystem results

Linter (stable)

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

freedomofpress/securedrop (+1 -0 violations, +0 -0 fixes)

+ securedrop/models.py:330:29: PYI032 [*] Prefer `object` to `Any` for the second parameter to `__eq__`

Changes by rule (1 rules affected)

code total + violation - violation + fix - fix
PYI032 1 1 0 0 0

Linter (preview)

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

freedomofpress/securedrop (+1 -0 violations, +0 -0 fixes)

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

+ securedrop/models.py:330:29: PYI032 [*] Prefer `object` to `Any` for the second parameter to `__eq__`

Changes by rule (1 rules affected)

code total + violation - violation + fix - fix
PYI032 1 1 0 0 0

@MichaReiser MichaReiser added bug Something isn't working rule Implementing or modifying a lint rule labels Aug 19, 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.

I leave the final review to @charliermarsh who's more familiar with that aspect of ruff

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.

Looks good though I have roughly the same concern as Micha. Caching this would make sense to me. (We already parse these once per file anyway.)

@AlexWaygood
Copy link
Member Author

Looks good though I have roughly the same concern as Micha. Caching this would make sense to me.

I separated this out into a standalone PR: #13158

Centralize the parsing of string annotations in a method on `Checker`
@AlexWaygood AlexWaygood enabled auto-merge (squash) September 2, 2024 13:37
@AlexWaygood AlexWaygood merged commit c0e2c13 into main Sep 2, 2024
17 checks passed
@AlexWaygood AlexWaygood deleted the pyi034-strings branch September 2, 2024 13:40
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
bug Something isn't working linter Related to the linter rule Implementing or modifying a lint rule
Projects
None yet
Development

Successfully merging this pull request may close these issues.

PYI034 false positive if Self is a quoted annotation
3 participants