-
-
Notifications
You must be signed in to change notification settings - Fork 3k
Generalize class/static method and property alias support #19297
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
base: master
Are you sure you want to change the base?
Conversation
Diff from mypy_primer, showing the effect of this PR on open source code: antidote (https://github.com/Finistere/antidote)
+ tests/core/test_inject_wrapper.py:228: error: Unused "type: ignore" comment [unused-ignore]
+ tests/core/test_inject_wrapper.py:234: error: Unused "type: ignore" comment [unused-ignore]
+ tests/core/test_inject_wrapper.py:240: error: Unused "type: ignore" comment [unused-ignore]
+ tests/core/test_inject_wrapper.py:336: error: Unused "type: ignore" comment [unused-ignore]
+ tests/core/test_inject_wrapper.py:342: error: Unused "type: ignore" comment [unused-ignore]
+ tests/core/test_inject_wrapper.py:348: error: Unused "type: ignore" comment [unused-ignore]
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice, this fixes a highly-upvoted issue. Looks good, left some minor comments.
|
||
reveal_type(A().g) # N: Revealed type is "builtins.int" | ||
reveal_type(A().g2) # N: Revealed type is "builtins.int" | ||
A().g2 = 1 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Test assignment to A().g
(should generate an error).
def foo2(cls, x): | ||
... | ||
|
||
bar2 = foo2 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is it worth it to also test static methods? Did they already work?
@@ -8531,15 +8545,21 @@ def visit_type_alias_type(self, t: TypeAliasType) -> Type: | |||
return t.copy_modified(args=[a.accept(self) for a in t.args]) | |||
|
|||
|
|||
def is_node_class(node: Node | None) -> bool | None: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nit: Maybe rename to is_classmethod_node
or similar? My initial thought what that this checks if the target is a "node class".
Fixes #6700
This is another followup for the
checkmember
work. Currently, we only support non-instance method aliasing in few very specific cases. I am making this support general.