Prevent crash on retrieving TypeVar from class #2544
Conversation
|
Can you handle the merge conflict? (We just merged a PR that touches almost all test files... Sorry...) |
gvanrossum
changed the title
|
Can you deal with the conflict? (Same cause as before -- there's just no way to land such a PR without causing conflicts.) |
|
Merged 👍 |
| T = TypeVar('T', int) | ||
| def f(self, x: T) -> T: | ||
| A = C.T | ||
| return x |
There was a problem hiding this comment.
Can you add the missing newline here?
| @@ -378,6 +378,9 @@ def analyze_class_attribute_access(itype: Instance, | |||
| not_ready_callback(name, context) | |||
| return AnyType() | |||
|
|
|||
| if isinstance(node.node, TypeVarExpr): | |||
| return node.node.upper_bound | |||
There was a problem hiding this comment.
If we don't support it, this should also report an error message. (In error recovery mode, returning the upper bound sounds fine.)
There was a problem hiding this comment.
As far as I know (not very far) we theoretically do support accessing TypeVars that are members of a class, we just don't support aliasing them. I'll try and come up with a valid example (or counterexample) after I finish work.
There was a problem hiding this comment.
Also, WRT returning the upper bound, I feel like it's wrong in this context - the type of accessing a class member TypeVar should be TypeVar(I'm not 100% on how that's internally represented), and aliasing should still fail.
|
OK, it sounds like you know what you want to do, I'll wait until you tell me you're ready for the next review. |
|
LGTM now -- if you're happy with it I can merge it. |
|
Yeah, I've got a few things I want to do regarding warnings but they're better off in another PR. Thanks :) |
|
Thanks! |
Fixes #2530.
The alias is still broken because it's not at the top level, but we no longer crash. The choice to return the upper bound of the TypeVar as the type of the expression was a best guess, I wasn't sure what to do there, so please correct me if I'm wrong.