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

Handle type[TypeVar] (fixes #14755) #14756

Merged

Conversation

sterliakov
Copy link
Contributor

Fixes #14755.

This case is already covered by check-classes.test::testTypeUsingTypeCClassMethodFromTypeVarUnionBound, now we just remove from it errors that shouldn't be reported.

@github-actions
Copy link
Contributor

Diff from mypy_primer, showing the effect of this PR on open source code:

steam.py (https://github.com/Gobot1234/steam.py)
- steam/app.py:129: note: Maybe you forgot to use "await"?
- steam/chat.py:401: error: No overload variant of "__new__" of "type" matches argument type "Type[ChatMessageT]"  [call-overload]
- steam/chat.py:401: note: Possible overload variants:
- steam/chat.py:401: note:     def __new__(cls: Type[type], object, /) -> type
- steam/chat.py:401: note:     def [_typeshed.Self] __new__(cls: Type[_typeshed.Self], str, Tuple[type, ...], Dict[str, Any], /, **kwds: Any) -> _typeshed.Self

urllib3 (https://github.com/urllib3/urllib3)
+ test/__init__.py:97: error: Module has no attribute "PROTOCOL_TLS"  [attr-defined]
+ test/__init__.py:97: note: Error code "attr-defined" not covered by "type: ignore" comment

@sterliakov
Copy link
Contributor Author

sterliakov commented Feb 22, 2023

The changes seem legit:

  • steap/app.py: I don't fully understand, there is no error for that line, why it used to be a note there?
  • steam/chat.py: there was a case of A.__new__(A), where A is some class - correct, was a bug, because A was defined by TypeVar with union upper bound (here)
  • urllib3: the error was simply not reported before (see added else branch: before this PR it was possible to fall through context.has_no_attr without emitting error message) - here.

Copy link
Collaborator

@hauntsaninja hauntsaninja 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!

re: steam/app.py, there is an error for that line, it's just that the error existed both before and after this change so it doesn't show up in the diff. Why the note goes away isn't exactly clear to me, I will take a look later

@hauntsaninja hauntsaninja merged commit 9cb4872 into python:master Mar 15, 2023
@hauntsaninja
Copy link
Collaborator

Thank you!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

type[TypeVar] not checked properly with upper bound
2 participants