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

[flake8-pyi] Fix several correctness issues with custom-type-var-return-type (PYI019) #15851

Merged
merged 4 commits into from
Jan 31, 2025

Conversation

AlexWaygood
Copy link
Member

Summary

This PR fixes several correctness issues in our PYI019 rule:

  • The rule would panic on this snippet. There shouldn't even be a diagnostic on this snippet, since it's not a valid use of the S type variable:
    class F:
        @classmethod
        def m[S](cls: type[S], /) -> S[int]: ...
  • The rule would panic on this snippet. Again, there shouldn't even be a diagnostic emitted here; it's an invalid use of a type variable:
    class F:
        def m[S](self: S) -> S[int]: ...
  • The rule had a false positive on this snippet, where type is shadowed locally:
    def shadowed_type():
        type = 1
        class A:
            @classmethod
            def m[S](cls: type[S]) -> S: ...
  • The rule had a false negative on this snippet, where the fully qualified version of builtins.type rather than the implicit builtin binding:
    import builtins
    
    class UsesFullyQualifiedType:
        @classmethod
        def m[S](cls: builtins.type[S]) -> S: ...  # PYI019
  • The rule had a false negative on this snippet, where the return annotation is type[S] rather than simply S:
    class Foo:
        @classmethod
        def f[S](cls: type[S]) -> type[S]: ...

Fixes #15849

Test Plan

I've added new fixtures added that trigger panics and/or incorrect behaviour on main.

@AlexWaygood AlexWaygood added bug Something isn't working rule Implementing or modifying a lint rule labels Jan 31, 2025
@AlexWaygood AlexWaygood requested review from ntBre and dylwil3 January 31, 2025 13:35
Copy link
Contributor

github-actions bot commented Jan 31, 2025

ruff-ecosystem results

Linter (stable)

✅ ecosystem check detected no linter changes.

Linter (preview)

✅ ecosystem check detected no linter changes.

Copy link
Contributor

@ntBre ntBre left a comment

Choose a reason for hiding this comment

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

This looks great! I had to throw in one nit, but the changes were easy to follow, especially with the new variable names, and the tests made it clear what was going on.

@AlexWaygood AlexWaygood enabled auto-merge (squash) January 31, 2025 14:14
@AlexWaygood AlexWaygood merged commit 44ac17b into main Jan 31, 2025
20 checks passed
@AlexWaygood AlexWaygood deleted the alex/pyi-panic branch January 31, 2025 14:19
dcreager added a commit that referenced this pull request Jan 31, 2025
* main:
  [`flake8-pyi`] Fix several correctness issues with `custom-type-var-return-type` (`PYI019`) (#15851)
  [`pyupgrade`] Reuse replacement logic from `UP046` and `UP047` (`UP040`) (#15840)
  [`refurb`] Avoid `None | None` as well as better detection and fix (`FURB168`) (#15779)
  Remove non-existing `lint.extendIgnore` editor setting (#15844)
  [`refurb`] Mark fix as unsafe if there are comments (`FURB171`) (#15832)
  [`flake8-comprehensions`] Skip when `TypeError` present from too many (kw)args for `C410`,`C411`, and `C418` (#15838)
  [`pyflakes`] Visit forward annotations in `TypeAliasType` as types (`F401`) (#15829)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working rule Implementing or modifying a lint rule
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Panic in PYI019 autofix
2 participants