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.

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

dmypy crash with “Invalid type comment or annotation” #10007

Closed
andrewdotn opened this issue Feb 1, 2021 · 0 comments · Fixed by #17495
Closed

dmypy crash with “Invalid type comment or annotation” #10007

andrewdotn opened this issue Feb 1, 2021 · 0 comments · Fixed by #17495

Comments

@andrewdotn
Copy link

Crash Report

On a fresh checkout of mypy HEAD, with foo.py containing

from typing import TypedDict

class C(TypedDict):
    a: {"c": int}

the command python -m mypy.dmypy run foo.py crashes the daemon.

If I run just mypy, without the daemon, I get the error

foo.py:4: error: Invalid type comment or annotation

but dmypy doesn’t seem to get far enough to report that.

Traceback

Daemon started
Daemon crashed!
Traceback (most recent call last):
File "~/git/mypy/mypy/dmypy_server.py", line 221, in serve
    resp = self.run_command(command, data)
File "~/git/mypy/mypy/dmypy_server.py", line 264, in run_command
    return method(self, **data)
File "~/git/mypy/mypy/dmypy_server.py", line 323, in cmd_run
    return self.check(sources, is_tty, terminal_width)
File "~/git/mypy/mypy/dmypy_server.py", line 380, in check
    res = self.initialize_fine_grained(sources, is_tty, terminal_width)
File "~/git/mypy/mypy/dmypy_server.py", line 410, in initialize_fine_grained
    result = mypy.build.build(sources=sources,
File "~/git/mypy/mypy/build.py", line 180, in build
    result = _build(
File "~/git/mypy/mypy/build.py", line 254, in _build
    graph = dispatch(sources, manager, stdout)
File "~/git/mypy/mypy/build.py", line 2656, in dispatch
    process_graph(graph, manager)
File "~/git/mypy/mypy/build.py", line 2982, in process_graph
    process_stale_scc(graph, scc, manager)
File "~/git/mypy/mypy/build.py", line 3083, in process_stale_scc
    graph[id].finish_passes()
File "~/git/mypy/mypy/build.py", line 2177, in finish_passes
    self.update_fine_grained_deps(self.manager.fg_deps)
File "~/git/mypy/mypy/build.py", line 2225, in update_fine_grained_deps
    merge_dependencies(self.compute_fine_grained_deps(), deps)
File "~/git/mypy/mypy/build.py", line 2216, in compute_fine_grained_deps
    return get_dependencies(target=self.tree,
File "~/git/mypy/mypy/server/deps.py", line 114, in get_dependencies
    target.accept(visitor)
File "~/git/mypy/mypy/nodes.py", line 305, in accept
    return visitor.visit_mypy_file(self)
File "~/git/mypy/mypy/server/deps.py", line 176, in visit_mypy_file
    super().visit_mypy_file(o)
File "~/git/mypy/mypy/traverser.py", line 35, in visit_mypy_file
    d.accept(self)
File "~/git/mypy/mypy/nodes.py", line 950, in accept
    return visitor.visit_class_def(self)
File "~/git/mypy/mypy/server/deps.py", line 239, in visit_class_def
    super().visit_class_def(o)
File "~/git/mypy/mypy/traverser.py", line 71, in visit_class_def
    o.defs.accept(self)
File "~/git/mypy/mypy/nodes.py", line 1015, in accept
    return visitor.visit_block(self)
File "~/git/mypy/mypy/server/deps.py", line 351, in visit_block
    super().visit_block(o)
File "~/git/mypy/mypy/traverser.py", line 39, in visit_block
    s.accept(self)
File "~/git/mypy/mypy/nodes.py", line 1073, in accept
    return visitor.visit_assignment_stmt(self)
File "~/git/mypy/mypy/server/deps.py", line 411, in visit_assignment_stmt
    self.add_type_dependencies(o.type)
File "~/git/mypy/mypy/server/deps.py", line 808, in add_type_dependencies
    for trigger in self.get_type_triggers(typ):
File "~/git/mypy/mypy/server/deps.py", line 861, in get_type_triggers
    return get_type_triggers(typ, self.use_logical_deps())
File "~/git/mypy/mypy/server/deps.py", line 866, in get_type_triggers
    return typ.accept(TypeTriggersVisitor(use_logical_deps))
File "~/git/mypy/mypy/types.py", line 1620, in accept
    assert isinstance(visitor, SyntheticTypeVisitor)
AssertionError

To Reproduce

See above

Your Environment

python 3.9.1 on macOS 11.1

Michael0x2a added a commit to Michael0x2a/mypy that referenced this issue Sep 26, 2022
Fixes python#10007

Mypy currently crashes when you try:

1. Creating a class-based TypedDict containing a malformed type hint
2. Asking it to compute fine-grained dependencies, either via
   running dmypy or by setting the `--cache-fine-grained` flag.

Here is the exact sequence of events that leads to this crash:

1. Since the type annotation is malformed, semanal initially gives
   the type annotation a type of `RawExpressionType`.

2. TypedDictAnalyzer (correctly) determines determines that the
   type of the malformed type annotation should be treated as
   just `Any` in:
   https://github.com/python/mypy/blob/f5ce4ee6ca7e2d8bb1cde8a2b49865f53bbacff5/mypy/semanal_typeddict.py#L289

3. TypedDictAnalyzer forgets to modify `stmt.type` like we normally
   do after calling `self.anal_type` in normal classes:
   https://github.com/python/mypy/blob/f5ce4ee6ca7e2d8bb1cde8a2b49865f53bbacff5/mypy/semanal.py#L3022

4. Mypy _does_ use the `Any` type when constructing the TypeInfo
   for the TypedDict. This is why mypy will not crash under most
   conditions: the correct type is being used in most places.

5. Setting `--cache-fine-grained` will make mypy perform one final
   pass against the AST to compute fine-grained dependencies. As
   a part of this process, it traverses the AssigmentStatement's `type`
   field using TypeTriggersVisitor.

6. TypeTriggersVisitor is _not_ a SyntheticTypeVisitor. So, the
   visitor trips an assert when we try traversing into the
   RawExpressionType.

Interestingly, this same crash does not occur for NamedTuples
despite the fact that NamedTupleAnalyzer also does not set `stmt.type`:
https://github.com/python/mypy/blob/f5ce4ee6ca7e2d8bb1cde8a2b49865f53bbacff5/mypy/semanal_namedtuple.py#L177

It turns out this is because semanal.py will end up calling the
`analyze_class_body_common(...)` function after NamedTupleAnalyzer
runs, but _not_ after TypedDictAnalyzer runs:

- https://github.com/python/mypy/blob/f5ce4ee6ca7e2d8bb1cde8a2b49865f53bbacff5/mypy/semanal.py#L1510
- https://github.com/python/mypy/blob/f5ce4ee6ca7e2d8bb1cde8a2b49865f53bbacff5/mypy/semanal.py#L1479

I'm not sure why this is: ideally, the two analyzers ought to have
as similar semantics as possible. But refactoring this felt potentially
disruptive, so I went for the narrower route of just patching TypedDictAnalyzer.
Michael0x2a added a commit to Michael0x2a/mypy that referenced this issue Sep 29, 2022
Fixes python#10007

Mypy currently crashes when you try:

1. Creating a class-based TypedDict containing a malformed type hint
2. Asking it to compute fine-grained dependencies, either via
   running dmypy or by setting the `--cache-fine-grained` flag.

Here is the exact sequence of events that leads to this crash:

1. Since the type annotation is malformed, semanal initially gives
   the type annotation a type of `RawExpressionType`.

2. TypedDictAnalyzer (correctly) determines determines that the
   type of the malformed type annotation should be treated as
   just `Any` in:
   https://github.com/python/mypy/blob/f5ce4ee6ca7e2d8bb1cde8a2b49865f53bbacff5/mypy/semanal_typeddict.py#L289

3. TypedDictAnalyzer forgets to modify `stmt.type` like we normally
   do after calling `self.anal_type` in normal classes:
   https://github.com/python/mypy/blob/f5ce4ee6ca7e2d8bb1cde8a2b49865f53bbacff5/mypy/semanal.py#L3022

4. Mypy _does_ use the `Any` type when constructing the TypeInfo
   for the TypedDict. This is why mypy will not crash under most
   conditions: the correct type is being used in most places.

5. Setting `--cache-fine-grained` will make mypy perform one final
   pass against the AST to compute fine-grained dependencies. As
   a part of this process, it traverses the AssigmentStatement's `type`
   field using TypeTriggersVisitor.

6. TypeTriggersVisitor is _not_ a SyntheticTypeVisitor. So, the
   visitor trips an assert when we try traversing into the
   RawExpressionType.

Interestingly, this same crash does not occur for NamedTuples
despite the fact that NamedTupleAnalyzer also does not set `stmt.type`:
https://github.com/python/mypy/blob/f5ce4ee6ca7e2d8bb1cde8a2b49865f53bbacff5/mypy/semanal_namedtuple.py#L177

It turns out this is because semanal.py will end up calling the
`analyze_class_body_common(...)` function after NamedTupleAnalyzer
runs, but _not_ after TypedDictAnalyzer runs:

- https://github.com/python/mypy/blob/f5ce4ee6ca7e2d8bb1cde8a2b49865f53bbacff5/mypy/semanal.py#L1510
- https://github.com/python/mypy/blob/f5ce4ee6ca7e2d8bb1cde8a2b49865f53bbacff5/mypy/semanal.py#L1479

I'm not sure why this is: ideally, the two analyzers ought to have
as similar semantics as possible. But refactoring this felt potentially
disruptive, so I went for the narrower route of just patching TypedDictAnalyzer.
Michael0x2a added a commit to Michael0x2a/mypy that referenced this issue Oct 23, 2022
Fixes python#10007

Mypy currently crashes when you try:

1. Creating a class-based TypedDict containing a malformed type hint
2. Asking it to compute fine-grained dependencies, either via
   running dmypy or by setting the `--cache-fine-grained` flag.

Here is the exact sequence of events that leads to this crash:

1. Since the type annotation is malformed, semanal initially gives
   the type annotation a type of `RawExpressionType`.

2. TypedDictAnalyzer (correctly) determines determines that the
   type of the malformed type annotation should be treated as
   just `Any` in:
   https://github.com/python/mypy/blob/f5ce4ee6ca7e2d8bb1cde8a2b49865f53bbacff5/mypy/semanal_typeddict.py#L289

3. TypedDictAnalyzer forgets to modify `stmt.type` like we normally
   do after calling `self.anal_type` in normal classes:
   https://github.com/python/mypy/blob/f5ce4ee6ca7e2d8bb1cde8a2b49865f53bbacff5/mypy/semanal.py#L3022

4. Mypy _does_ use the `Any` type when constructing the TypeInfo
   for the TypedDict. This is why mypy will not crash under most
   conditions: the correct type is being used in most places.

5. Setting `--cache-fine-grained` will make mypy perform one final
   pass against the AST to compute fine-grained dependencies. As
   a part of this process, it traverses the AssigmentStatement's `type`
   field using TypeTriggersVisitor.

6. TypeTriggersVisitor is _not_ a SyntheticTypeVisitor. So, the
   visitor trips an assert when we try traversing into the
   RawExpressionType.

Interestingly, this same crash does not occur for NamedTuples
despite the fact that NamedTupleAnalyzer also does not set `stmt.type`:
https://github.com/python/mypy/blob/f5ce4ee6ca7e2d8bb1cde8a2b49865f53bbacff5/mypy/semanal_namedtuple.py#L177

It turns out this is because semanal.py will end up calling the
`analyze_class_body_common(...)` function after NamedTupleAnalyzer
runs, but _not_ after TypedDictAnalyzer runs:

- https://github.com/python/mypy/blob/f5ce4ee6ca7e2d8bb1cde8a2b49865f53bbacff5/mypy/semanal.py#L1510
- https://github.com/python/mypy/blob/f5ce4ee6ca7e2d8bb1cde8a2b49865f53bbacff5/mypy/semanal.py#L1479

I'm not sure why this is: ideally, the two analyzers ought to have
as similar semantics as possible. But refactoring this felt potentially
disruptive, so I went for the narrower route of just patching TypedDictAnalyzer.
Michael0x2a added a commit to Michael0x2a/mypy that referenced this issue Oct 29, 2022
Fixes python#13066

During the semanal phase, mypy opts to ignore and skip processing
any malformed or illegal statements inside of a TypedDict class
definition, such as method definitions.

Skipping semanal analysis on these statements can cause any number
of odd downstream problems: the type-checking phase assumes that all
semanal-only semantic constructs (e.g. FakeInfo) have been purged
by this point, and so can crash at any point once this precondition has
been violated.

This diff opts to solve this problem by filtering down the list of
statements so we keep only the ones we know are legal within a TypedDict
definition.

The other possible solution to this problem is to modify mypy so we
skip checking TypedDict class bodies entirely during type checking and
fine-grained deps analysis. Doing this would also let address python#10007
and supersede my other diff python#13732.

I decided against doing this for now because:

1. I wasn't sure if this was actually safe, especially in the
   fine-grained deps phase and for mypyc.
2. I think no matter what, the semanal phase should not leak
   semanal-only types: relaxing this postcondition would make
   it harder to reason about mypy. So, we'd probably want to make this
   change regardless of what we do in the later phases.
JukkaL pushed a commit that referenced this issue Oct 31, 2022
Fixes #13066

During the semanal phase, mypy opts to ignore and skip processing any
malformed or illegal statements inside of a TypedDict class definition,
such as method definitions.

Skipping semanal analysis on these statements can cause any number of
odd downstream problems: the type-checking phase assumes that all
semanal-only semantic constructs (e.g. FakeInfo) have been purged by
this point, and so can crash at any point once this precondition has
been violated.

This diff opts to solve this problem by filtering down the list of
statements so we keep only the ones we know are legal within a TypedDict
definition.

The other possible solution to this problem is to modify mypy so we skip
checking TypedDict class bodies entirely during type checking and
fine-grained deps analysis. Doing this would also let address #10007 and
supersede my other diff #13732.

I decided against doing this for now because:

1. I wasn't sure if this was actually safe, especially in the
fine-grained deps phase and for mypyc.
2. I think no matter what, the semanal phase should not leak
semanal-only types: relaxing this postcondition would make it harder to
reason about mypy. So, we'd probably want to make this change regardless
of what we do in the later phases.
JukkaL pushed a commit that referenced this issue Jul 18, 2024
Fixes #10007
Fixes #17477

This fixes the crash as proposed in
#13732, but also fixes some
inconsistencies in `Any` types exposed by the fix.

---------

Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
2 participants