Update Ruff and enable F821 in stubs #11771
Conversation
|
According to mypy_primer, this change has no effect on the checked open source code. 🤖🎉 |
Me neither, but I figured some of the reasoning about testing / feedback loop in #11734 (comment) could apply here as well. Although Flake8-pyi's unit tests should essentially cover changes and regressions. Happy to make a follow-up PR to see what others think (mainly from flake8-pyi maintainers). |
| @@ -132,9 +132,6 @@ if sys.version_info >= (3, 12): | |||
| ContextManager = AbstractContextManager | |||
| AsyncContextManager = AbstractAsyncContextManager | |||
|
|
|||
| # This itself is only available during type checking | |||
| def type_check_only(func_or_cls: _F) -> _F: ... | |||
There was a problem hiding this comment.
Why were these moved? This is legal in stubs, even if it might be better style to move the function after the definition of _F.
There was a problem hiding this comment.
Ruff does not currently support generalised forward references in stub files in the same way that mypy and pyright do. We considered implementing that support, but when we looked, we couldn't actually find a reference anywhere in any PEP or typing spec that specified that all forward references should be allowed in stub files. We therefore decided to only support forward references in situations where it could plausibly be useful to have forward references, such as class bases. See astral-sh/ruff#10779 (comment).
This is similar in concept to the limited way flake8-pyi monkeypatches flake8. pyflakes-monkeypatched-by-flake8-pyi will allow this:
class Foo(list[Bar]): ...
class Bar: ...But it won't allow this, because there's no known use case for it:
class Foo(Bar): ...
class Bar: ...(Ruff will allow ^both of those, but it's the same idea of "allowing pragmatic forward references, but not arbitrary forward references")
There was a problem hiding this comment.
(If it does get added to a spec somewhere or we think there is a good reason to support arbitrary forward references in stubs, I'm personally very open to adding support for it at Ruff.)
There was a problem hiding this comment.
That makes sense in general; the second change in this PR (connect = Connection before Connection is defined) is definitely some very odd code.
I'm not sure why Ruff has an issue with the definition of def type_check_only, though. _F is used only in an annotation; why does the definition of type_check_only need to be after the definition of _F?
There was a problem hiding this comment.
The issue in typing.pyi is that the TypeVar _F is bound to Callable before Callable is defined
There was a problem hiding this comment.
Ah, I guess that's the kind of case we only encounter in typeshed :)
Thanks to @AlexWaygood 's work on astral-sh/ruff#3011 , we no longer need to ignore
F821in Ruff configs, whilst still preferring to avoid forward-references.I've left in
F821from Flake8 as per my in-code comment: typeshed being a testing ground for F821, it's great to catch changes to the monkeypatching ofF821