-
-
Notifications
You must be signed in to change notification settings - Fork 2.9k
Add a SYMBOL_FUNCBASE_TYPES constant to avoid multiple-inheritance bug #6481
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -403,7 +403,18 @@ def __str__(self) -> str: | |
|
||
|
||
class FuncBase(Node): | ||
"""Abstract base class for function-like nodes""" | ||
"""Abstract base class for function-like nodes. | ||
|
||
N.B: Although this has SymbolNode subclasses (FuncDef, | ||
OverloadedFuncDef), avoid calling isinstance(..., FuncBase) on | ||
something that is typed as SymbolNode. This is to work around | ||
mypy bug #3603, in which mypy doesn't understand multiple | ||
inheritance very well, and will assume that a SymbolNode | ||
cannot be a FuncBase. | ||
|
||
Instead, test against SYMBOL_FUNCBASE_TYPES, which enumerates | ||
SymbolNode subclasses that are also FuncBase subclasses. | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Sounds tedious. :-( |
||
""" | ||
|
||
__slots__ = ('type', | ||
'unanalyzed_type', | ||
|
@@ -668,6 +679,11 @@ def deserialize(cls, data: JsonDict) -> 'FuncDef': | |
return ret | ||
|
||
|
||
# All types that are both SymbolNodes and FuncBases. See the FuncBase | ||
# docstring for the rationale. | ||
SYMBOL_FUNCBASE_TYPES = (OverloadedFuncDef, FuncDef) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. How do we ensure this stays up to date? If a new class were to be added, would tests for that new class fail clearly? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. They would fail, I think, though I'm not sure how cleanly. |
||
|
||
|
||
class Decorator(SymbolNode, Statement): | ||
"""A decorated function. | ||
|
||
|
@@ -2857,10 +2873,7 @@ def fullname(self) -> Optional[str]: | |
@property | ||
def type(self) -> 'Optional[mypy.types.Type]': | ||
node = self.node | ||
if (isinstance(node, Var) and node.type is not None): | ||
return node.type | ||
# mypy thinks this branch is unreachable but it is wrong (#3603) | ||
elif (isinstance(node, FuncBase) and node.type is not None): | ||
if isinstance(node, (Var, SYMBOL_FUNCBASE_TYPES)) and node.type is not None: | ||
return node.type | ||
elif isinstance(node, Decorator): | ||
return node.var.type | ||
|
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.
So the hack now is that this is a variable and the binder doesn't know whether it's true or false?
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.
There's no hack, anymore, really. We're just explicitly enumerating the overlap between SymbolNode and FuncBase instead of using isinstance on FuncBase