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.

Sign up for GitHub

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

Jump to bottom

Fix crash on type alias definition inside dataclass declaration #12792

Merged
merged 3 commits into from May 20, 2022
Merged

Fix crash on type alias definition inside dataclass declaration #12792

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
7908bad
Fix crash on type alias definition inside dataclass declaration
AlexWaygood May 15, 2022
766bd38
Mostly address review
AlexWaygood May 18, 2022
ffb9935
Skip processing the node, avoid modifying the SymbolTable
AlexWaygood May 18, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
38 changes: 35 additions & 3 deletions mypy/plugins/dataclasses.py
View file
Open in desktop
@@ -1,11 +1,11 @@
"""Plugin that provides support for dataclasses."""

from typing import Dict, List, Set, Tuple, Optional
from typing import Dict, List, Set, Tuple, Optional, Union
from typing_extensions import Final

from mypy.nodes import (
ARG_OPT, ARG_NAMED, ARG_NAMED_OPT, ARG_POS, ARG_STAR, ARG_STAR2, MDEF,
Argument, AssignmentStmt, CallExpr, Context, Expression, JsonDict,
Argument, AssignmentStmt, CallExpr, TypeAlias, Context, Expression, JsonDict,
NameExpr, RefExpr, SymbolTableNode, TempNode, TypeInfo, Var, TypeVarExpr,
PlaceholderNode
)
Expand All @@ -16,7 +16,7 @@
from mypy.typeops import map_type_from_supertype
from mypy.types import (
Type, Instance, NoneType, TypeVarType, CallableType, TupleType, LiteralType,
get_proper_type, AnyType, TypeOfAny,
get_proper_type, AnyType, TypeOfAny, TypeType,
)
from mypy.server.trigger import make_wildcard_trigger
from mypy.state import state
Expand Down Expand Up @@ -333,6 +333,38 @@ def collect_attributes(self) -> Optional[List[DataclassAttribute]]:

node = sym.node
assert not isinstance(node, PlaceholderNode)

if isinstance(node, TypeAlias):
ctx.api.fail(
(
'Type aliases inside dataclass definitions '
'are not supported at runtime.'
AlexWaygood marked this conversation as resolved.
Show resolved Hide resolved
),
Context(line=node.line, column=node.column)
AlexWaygood marked this conversation as resolved.
Show resolved Hide resolved
)
# Now do our best to simulate the runtime,
# which treates a TypeAlias definition in a dataclass class
# as an instance field with a default value.
#
# Replace the `TypeAlias` node with a `Var` node, so that we do the same.
target, fullname = node.target, node.fullname
proper_target = get_proper_type(target)
var_type: Union[TypeType, AnyType]
if isinstance(proper_target, Instance):
var_type = TypeType(proper_target, line=node.line, column=node.column)
# Something else -- fallback to Any
else:
var_type = AnyType(TypeOfAny.from_error)
var = Var(name=fullname, type=var_type)
AlexWaygood marked this conversation as resolved.
Show resolved Hide resolved
var.info = cls.info
var._fullname = fullname
cls.info.names[fullname] = SymbolTableNode(
kind=MDEF,
node=var,
plugin_generated=True,
)
sym.node = node = var
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Modifying the symbol table in a plugin is a little dangerous, since we can apply the plugin hook multiple times, and the error would generated only on the first run, I think. This might be fine here, but it would be easier to reason about what goes on if we'd just skip processing TypeAlias nodes, similar to what we do with ClassVar below. This wouldn't match runtime behavior, but it would be okay since the definition is invalid in any case. I'm not sure if this would cause other problems, however.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The theoretical issue with this is that skipping the processing of the node means mypy will construct an incorrect __init__ signature. Given the following (weird) dataclass definition:

@dataclass
class Foo:
    bar: TypeAlias = int

mypy will infer the following __init__ signature:

def __init__(self) -> None: ...

But the actual signature generated by the runtime is

def __init__(self, bar: type[int] = ...) -> None: ...

But as you say, maybe this doesn't really matter that much, since we've already warned the user that using TypeAlias inside a dataclass definition isn't properly supported.


assert isinstance(node, Var)

# x: ClassVar[int] is ignored by dataclasses.
Expand Down
30 changes: 30 additions & 0 deletions test-data/unit/check-dataclasses.test
View file
Open in desktop
Expand Up @@ -513,6 +513,36 @@ Application.COUNTER = 1

[builtins fixtures/dataclasses.pyi]

[case testTypeAliasInDataclassDoesNotCrash]
# flags: --python-version 3.7
from dataclasses import dataclass
from typing import Callable
from typing_extensions import TypeAlias

@dataclass
class Foo:
x: int

@dataclass
class One:
S: TypeAlias = Foo # E: Type aliases inside dataclass definitions are not supported at runtime.

a = One()
b = One(Foo)
reveal_type(a.S) # N: Revealed type is "Type[__main__.Foo]"
reveal_type(b.S) # N: Revealed type is "Type[__main__.Foo]"
a.S() # E: Missing positional argument "x" in call to "Foo"
reveal_type(a.S(5)) # N: Revealed type is "__main__.Foo"
reveal_type(b.S(98)) # N: Revealed type is "__main__.Foo"

@dataclass
class Two:
S: TypeAlias = Callable[[int], str] # E: Type aliases inside dataclass definitions are not supported at runtime.

c = Two()
reveal_type(c.S) # N: Revealed type is "Any"
[builtins fixtures/dataclasses.pyi]

[case testDataclassOrdering]
# flags: --python-version 3.7
from dataclasses import dataclass
Expand Down