Repository: python/mypy
Pull request: Make `refresh_top_level()` more like `visit_file()` in the third pass
Fixes #6032 (first part)

This is a very straightforward fix. I am actually surprised no one found this before (maybe because mypy daemon is not so widely used yet).

In future, it would be great to have a mechanism to avoid such mistakes, by somehow making `refresh_top_level()` to do all the special things `visit_file()` does. I don't have any ideas now, but it probably makes sense to consider this during semantic analyzer refactoring (another possible solution is to avoid any such special things, but this should be clearly documented).

Problem statement:
Two crashes in fine grained mode with invalid types inside synthetic constructs
First traceback:
```
  File "mypy/dmypy_server.py", line 215, in serve
  File "mypy/dmypy_server.py", line 252, in run_command
  File "mypy/dmypy_server.py", line 300, in cmd_check
  File "mypy/dmypy_server.py", line 335, in check
  File "mypy/dmypy_server.py", line 382, in initialize_fine_grained
  File "mypy/server/update.py", line 235, in update
  File "mypy/server/update.py", line 287, in update_one
  File "mypy/server/update.py", line 342, in update_module
  File "mypy/server/update.py", line 549, in update_module_isolated
  File "mypy/server/update.py", line 715, in replace_modules_with_new_variants
  File "mypy/server/astmerge.py", line 88, in merge_asts
  File "mypy/server/astmerge.py", line 133, in replace_nodes_in_ast
  File "mypy/nodes.py", line 257, in accept
  File "mypy/server/astmerge.py", line 151, in visit_mypy_file__NodeVisitor_glue
  File "mypy/server/astmerge.py", line 154, in visit_mypy_file
  File "mypy/traverser.py", line 33, in visit_mypy_file
  File "mypy/nodes.py", line 968, in accept
  File "mypy/server/astmerge.py", line 195, in visit_assignment_stmt__StatementVisitor_glue
  File "mypy/server/astmerge.py", line 197, in visit_assignment_stmt
  File "mypy/traverser.py", line 79, in visit_assignment_stmt
  File "mypy/nodes.py", line 1421, in accept
  File "mypy/server/astmerge.py", line 231, in visit_call_expr__ExpressionVisitor_glue
  File "mypy/server/astmerge.py", line 232, in visit_call_expr
  File "mypy/traverser.py", line 166, in visit_call_expr
  File "mypy/nodes.py", line 2062, in accept
  File "mypy/server/astmerge.py", line 236, in visit_newtype_expr__ExpressionVisitor_glue
  File "mypy/server/astmerge.py", line 239, in visit_newtype_expr
  File "mypy/server/astmerge.py", line 318, in process_synthetic_type_info
  File "mypy/nodes.py", line 610, in accept__Node_glue
  File "mypy/nodes.py", line 611, in accept
  File "mypy/server/astmerge.py", line 160, in visit_func_def__StatementVisitor_glue
  File "mypy/server/astmerge.py", line 162, in visit_func_def
  File "mypy/server/astmerge.py", line 188, in process_base_func
  File "mypy/server/astmerge.py", line 292, in fixup_type
  File "mypy/types.py", line 838, in accept
  File "mypy/server/astmerge.py", line 346, in visit_callable_type__TypeVisitor_glue
  File "mypy/server/astmerge.py", line 348, in visit_callable_type
  File "mypy/types.py", line 541, in accept
  File "mypy/server/astmerge.py", line 335, in visit_instance__TypeVisitor_glue
  File "mypy/server/astmerge.py", line 338, in visit_instance
  File "mypy/types.py", line 1409, in accept
  File "mypy/server/astmerge.py", line 417, in visit_union_type__TypeVisitor_glue
  File "mypy/server/astmerge.py", line 419, in visit_union_type
  File "mypy/types.py", line 541, in accept
  File "mypy/server/astmerge.py", line 335, in visit_instance__TypeVisitor_glue
  File "mypy/server/astmerge.py", line 338, in visit_instance
  File "mypy/types.py", line 1597, in accept
  File "mypy/type_visitor.py", line 105, in visit_forwardref_type
RuntimeError: Internal error: unresolved forward reference
```
Second traceback:
```
File "mypy/dmypy_server.py", line 205, in serve
File "mypy/dmypy_server.py", line 242, in run_command
File "mypy/dmypy_server.py", line 290, in cmd_check
File "mypy/dmypy_server.py", line 325, in check
File "mypy/dmypy_server.py", line 372, in initialize_fine_grained
File "mypy/server/update.py", line 254, in update
File "mypy/server/update.py", line 767, in propagate_changes_using_dependencies
File "mypy/server/update.py", line 927, in reprocess_nodes
File "mypy/server/astdiff.py", line 154, in snapshot_symbol_table
File "mypy/server/astdiff.py", line 237, in snapshot_optional_type
File "mypy/server/astdiff.py", line 232, in snapshot_type
File "mypy/types.py", line 1409, in accept
File "mypy/server/astdiff.py", line 321, in visit_union_type__TypeVisitor_glue
File "mypy/server/astdiff.py", line 324, in visit_union_type
File "mypy/server/astdiff.py", line 232, in snapshot_type
File "mypy/types.py", line 1597, in accept
File "mypy/type_visitor.py", line 105, in visit_forwardref_type
RuntimeError: Internal error: unresolved forward reference
```

Hints from issue discussion:
Also there is something about mypyc tracebacks: why `xxx__ClassName_glue` has line number of `xxx` _minus two_ (or one, or three)?
Note: this error will be likely fixed by the upcoming semantic analyzer refactoring.
OK, here is a simple possible repro:
```
[case testNamedTupleForwardFunction]
from typing import NamedTuple
from a import A

NT = NamedTuple('NT', [('x', A)])
[file a.py]
from b import B
A = B
[file b.py.2]
def func(x): pass
B = func
[out]
```
I am not sure what is the crux, but most likely using alias to a function as a field type in named tuple is problematic. It crashes instead of giving an `Invalid type`-like error.
A repro for the second crash (very similar):
```
[case testAliasForwardFunction]
from typing import Optional
from a import A

Alias = Optional[A]
[file a.py]
from b import B
A = B
[file b.py.2]
def func(x): pass
B = func
[out]
```

Commit message: Fix bugs

Patch to review:
diff --git a/mypy/semanal_pass3.py b/mypy/semanal_pass3.py
index 1505d76c0152..f869fde7b115 100644
--- a/mypy/semanal_pass3.py
+++ b/mypy/semanal_pass3.py
@@ -133,6 +133,7 @@ def refresh_top_level(self, file_node: MypyFile) -> None:
         """Reanalyze a stale module top-level in fine-grained incremental mode."""
         for d in file_node.defs:
             self.accept(d)
+        self.analyze_symbol_table(file_node.names)
 
     def accept(self, node: Node) -> None:
         try:
@@ -191,7 +192,7 @@ def visit_class_def(self, tdef: ClassDef) -> None:
                 self.analyze(tdef.analyzed.info.typeddict_type, tdef.analyzed, warn=True)
             elif isinstance(tdef.analyzed, NamedTupleExpr):
                 self.analyze(tdef.analyzed.info.tuple_type, tdef.analyzed, warn=True)
-                self.analyze_info(tdef.analyzed.info)
+                self.analyze_synthetic_info(tdef.analyzed.info)
         super().visit_class_def(tdef)
         self.analyze_symbol_table(tdef.info.names)
         self.scope.leave()
@@ -272,7 +273,7 @@ def visit_assignment_stmt(self, s: AssignmentStmt) -> None:
                 if analyzed.info:
                     # Currently NewTypes only have __init__, but to be future proof,
                     # we analyze all symbols.
-                    self.analyze_info(analyzed.info)
+                    self.analyze_synthetic_info(analyzed.info)
                 if analyzed.info and analyzed.info.mro:
                     analyzed.info.mro = []  # Force recomputation
                     self.sem.calculate_class_mro(analyzed.info.defn)
@@ -287,7 +288,7 @@ def visit_assignment_stmt(self, s: AssignmentStmt) -> None:
                 self.analyze(analyzed.info.typeddict_type, analyzed, warn=True)
             if isinstance(analyzed, NamedTupleExpr):
                 self.analyze(analyzed.info.tuple_type, analyzed, warn=True)
-                self.analyze_info(analyzed.info)
+                self.analyze_synthetic_info(analyzed.info)
         if isinstance(s.lvalues[0], RefExpr) and isinstance(s.lvalues[0].node, Var):
             self.analyze(s.lvalues[0].node.type, s.lvalues[0].node)
         # Subclass attribute assignments with no type annotation should be
@@ -471,12 +472,18 @@ def patch2() -> None:
 
             self.patches.append((PRIORITY_TYPEVAR_VALUES, self.make_scoped_patch(patch2)))
 
-    def analyze_info(self, info: TypeInfo) -> None:
+    def analyze_synthetic_info(self, info: TypeInfo) -> None:
         # Similar to above but for nodes with synthetic TypeInfos (NamedTuple and NewType).
         for name in info.names:
             sym = info.names[name]
             if isinstance(sym.node, (FuncDef, Decorator)):
+                # Since we are analyzing a synthetic type info, the methods there
+                # are not real independent targets, and should be processed when
+                # the enclosing synthetic type is processed.
+                old_recurse = self.recurse_into_functions
+                self.recurse_into_functions = True
                 self.accept(sym.node)
+                self.recurse_into_functions = old_recurse
             if isinstance(sym.node, Var):
                 self.analyze(sym.node.type, sym.node)
 
diff --git a/test-data/unit/fine-grained.test b/test-data/unit/fine-grained.test
index cee3fa709d47..727d0da4767e 100644
--- a/test-data/unit/fine-grained.test
+++ b/test-data/unit/fine-grained.test
@@ -8134,3 +8134,61 @@ x = 'no way'
 ==
 ==
 main:10: error: Incompatible types in assignment (expression has type "str", variable has type "int")
+
+[case testNamedTupleForwardFunctionDirect]
+# flags: --ignore-missing-imports
+from typing import NamedTuple
+from b import B
+
+NT = NamedTuple('NT', [('x', B)])
+[file b.py.2]
+def func(x): pass
+B = func
+[out]
+==
+main:5: error: Invalid type "b.B"
+
+[case testNamedTupleForwardFunctionIndirect]
+# flags: --ignore-missing-imports
+from typing import NamedTuple
+from a import A
+
+NT = NamedTuple('NT', [('x', A)])
+[file a.py]
+from b import B
+A = B
+[file b.py.2]
+def func(x): pass
+B = func
+[out]
+==
+main:5: error: Invalid type "a.A"
+
+[case testAliasForwardFunctionDirect]
+# flags: --ignore-missing-imports
+from typing import Optional
+from b import B
+
+Alias = Optional[B]
+[file b.py.2]
+def func(x): pass
+B = int()
+[out]
+==
+main:5: error: Invalid type "b.B"
+
+[case testAliasForwardFunctionIndirect]
+# flags: --ignore-missing-imports
+from typing import Optional
+from a import A
+
+Alias = Optional[A]
+[file a.py]
+from b import B
+A = B
+[file b.py.2]
+def func(x): pass
+B = func
+[out]
+==
+main:5: error: Invalid type "a.A"


Code review comments:
diff_hunk: @@ -471,12 +472,18 @@ def patch2() -> None:
 
             self.patches.append((PRIORITY_TYPEVAR_VALUES, self.make_scoped_patch(patch2)))
 
-    def analyze_info(self, info: TypeInfo) -> None:
+    def analyze_synthetic_info(self, info: TypeInfo) -> None:
         # Similar to above but for nodes with synthetic TypeInfos (NamedTuple and NewType).
         for name in info.names:
             sym = info.names[name]
             if isinstance(sym.node, (FuncDef, Decorator)):
+                # Since we are analyzing a synthetic type info, the methods there
+                # are not real independent targets, and should be processed when
+                # the enclosing synthetic type is processed.
+                old_recurse = self.recurse_into_functions
+                self.recurse_into_functions = True
path: mypy/semanal_pass3.py
text: @user1:
If this is not needed to fix the test cases, could you move this to a separate PR?

@author:
Hm, I think I can split it by moving one of the tests added to the second PR.