[red-knot] Extend instance/class attribute tests #15959
Conversation
sharkdp
added
testing
sharkdp
requested review from
carljm,
MichaReiser and
AlexWaygood
as code owners
| # TODO: We should probably infer `int | str` here. | ||
| reveal_type(c_instance.z) # revealed: int |
There was a problem hiding this comment.
Since we'll have emitted a diagnostic above complaining about the redeclaration, I think the current behaviour here is also reasonable TBH. I wouldn't worry too much about inferring a union here if it complicates the implementation.
| # TODO: should be `Literal["overwritten on class"]` once/if we support | ||
| # local narrowing. |
There was a problem hiding this comment.
interesting... do mypy/pyright support narrowing like this, where you narrow it on the class but then read it from an instance?
There was a problem hiding this comment.
Mypy doesn't seem to support this detection of implicit pure_class_variables at all. Pyright supports narrowing in the case above (C.pure_class_variable), but not here.
Maybe I should remove the TODO here?
There was a problem hiding this comment.
Yeah, I don't think this is something we "have" to support -- I don't think it's something users will expect of us. (I think they will expect attribute narrowing when the attribute is accessed on the same variable, such as C.pure_class_variable above.) Obviously if it's easy and naturally falls out of the implementation, then that's fantastic -- I think it isn't any more unsound than attribute narrowing in general. But I don't think it's something we need a TODO for.
| reveal_type(D.x) # revealed: Unknown | ||
|
|
||
| # TODO: this should raise `unresolved-attribute` as well, and the type should be `Unknown` | ||
| reveal_type(D().x) # revealed: Unknown | Literal[1] |
There was a problem hiding this comment.
Pyright revals int here, which is wrong.
There was a problem hiding this comment.
oof. might be worth a bug report to pyright!
There was a problem hiding this comment.
There was a problem hiding this comment.
I won't be surprised if aliasing staticmethod is just considered out-of-scope for pyright to handle. I will be very happy if we can handle it, however!
Summary
In preparation for creating some (sub) issues for #14164, I'm trying to document the current behavior (and a bug) a bit better.