New semantic analyzer: Fix (de)serialization of broken named tuples (#6421)

Fixes #6413

In my original PR I missed the fact that "broken" named tuples (those where variable name and named tuple name are different) are stored under both names (one to actually access it, and other for (de)serialization).

I also extend the semantic analyzer API, and clean-up/clarify the symbol adding logic a bit.

Author: ilevkivskyi

mypy/build.py

@@ -1794,8 +1794,13 @@ def patch_dependency_parents(self) -> None:
         semantic analyzer will perform this patch for us when processing stale
         SCCs.
         """
+        Analyzer = Union[SemanticAnalyzerPass2, NewSemanticAnalyzer]  # noqa
+        if self.manager.options.new_semantic_analyzer:
+            analyzer = self.manager.new_semantic_analyzer  # type: Analyzer
+        else:
+            analyzer = self.manager.semantic_analyzer
         for dep in self.dependencies:
-            self.manager.semantic_analyzer.add_submodules_to_parent_modules(dep, True)
+            analyzer.add_submodules_to_parent_modules(dep, True)
 
     def fix_suppressed_dependencies(self, graph: Graph) -> None:
         """Corrects whether dependencies are considered stale in silent mode.

mypy/newsemanal/semanal.py

@@ -3955,6 +3955,28 @@ def add_symbol(self, name: str, node: SymbolNode, context: Optional[Context],
                                  module_hidden=module_hidden)
         return self.add_symbol_table_node(name, symbol, context)
 
+    def add_symbol_skip_local(self, name: str, node: SymbolNode) -> None:
+        """Same as above, but skipping the local namespace.
+
+        This doesn't check for previous definition and is only used
+        for serialization of method-level classes.
+
+        Classes defined within methods can be exposed through an
+        attribute type, but method-level symbol tables aren't serialized.
+        This method can be used to add such classes to an enclosing,
+        serialized symbol table.
+        """
+        # TODO: currently this is only used by named tuples. Use this method
+        # also by typed dicts and normal classes, see issue #6422.
+        if self.type is not None:
+            names = self.type.names
+            kind = MDEF
+        else:
+            names = self.globals
+            kind = GDEF
+        symbol = SymbolTableNode(kind, node)
+        names[name] = symbol
+
     def current_symbol_table(self) -> SymbolTable:
         if self.is_func_scope():
             assert self.locals[-1] is not None

mypy/newsemanal/semanal_namedtuple.py

@@ -179,7 +179,18 @@ def check_namedtuple(self,
             return True, info
         name = cast(Union[StrExpr, BytesExpr, UnicodeExpr], call.args[0]).value
         if name != var_name or is_func_scope:
-            # Give it a unique name derived from the line number.
+            # There are three special cases where need to give it a unique name derived
+            # from the line number:
+            #   * There is a name mismatch with l.h.s., therefore we need to disambiguate
+            #     situations like:
+            #         A = NamedTuple('Same', [('x', int)])
+            #         B = NamedTuple('Same', [('y', str)])
+            #   * This is a base class expression, since it often matches the class name:
+            #         class NT(NamedTuple('NT', [...])):
+            #             ...
+            #   * This is a local (function or method level) named tuple, since
+            #     two methods of a class can define a named tuple with the same name,
+            #     and they will be stored in the same namespace (see below).
             name += '@' + str(call.line)
         if len(defaults) > 0:
             default_items = {
@@ -189,15 +200,31 @@ def check_namedtuple(self,
         else:
             default_items = {}
         info = self.build_namedtuple_typeinfo(name, items, types, default_items)
-        # Store it as a global just in case it would remain anonymous.
-        # (Or in the nearest class if there is one.)
-        self.store_namedtuple_info(info, var_name or name, call, is_typed)
+        # If var_name is not None (i.e. this is not a base class expression), we always
+        # store the generated TypeInfo under var_name in the current scope, so that
+        # other definitions can use it.
+        if var_name:
+            self.store_namedtuple_info(info, var_name, call, is_typed)
+        # There are three cases where we need to store the generated TypeInfo
+        # second time (for the purpose of serialization):
+        #   * If there is a name mismatch like One = NamedTuple('Other', [...])
+        #     we also store the info under name 'Other@lineno', this is needed
+        #     because classes are (de)serialized using their actual fullname, not
+        #     the name of l.h.s.
+        #   * If this is a method level named tuple. It can leak from the method
+        #     via assignment to self attribute and therefore needs to be serialized
+        #     (local namespaces are not serialized).
+        #   * If it is a base class expression. It was not stored above, since
+        #     there is no var_name (but it still needs to be serialized
+        #     since it is in MRO of some class).
+        if name != var_name or is_func_scope:
+            # NOTE: we skip local namespaces since they are not serialized.
+            self.api.add_symbol_skip_local(name, info)
         return True, info
 
     def store_namedtuple_info(self, info: TypeInfo, name: str,
                               call: CallExpr, is_typed: bool) -> None:
-        stnode = SymbolTableNode(GDEF, info)
-        self.api.add_symbol_table_node(name, stnode)
+        self.api.add_symbol(name, info, call)
         call.analyzed = NamedTupleExpr(info, is_typed=is_typed)
         call.analyzed.set_line(call.line, call.column)
 

mypy/newsemanal/semanal_shared.py

@@ -5,7 +5,8 @@
 from mypy_extensions import trait
 
 from mypy.nodes import (
-    Context, SymbolTableNode, MypyFile, ImportedName, FuncDef, Node, TypeInfo, Expression, GDEF
+    Context, SymbolTableNode, MypyFile, ImportedName, FuncDef, Node, TypeInfo, Expression, GDEF,
+    SymbolNode
 )
 from mypy.util import correct_relative_import
 from mypy.types import Type, FunctionLike, Instance
@@ -122,7 +123,23 @@ def schedule_patch(self, priority: int, fn: Callable[[], None]) -> None:
 
     @abstractmethod
     def add_symbol_table_node(self, name: str, stnode: SymbolTableNode) -> bool:
-        """Add node to global symbol table (or to nearest class if there is one)."""
+        """Add node to the current symbol table."""
+        raise NotImplementedError
+
+    @abstractmethod
+    def add_symbol(self, name: str, node: SymbolNode, context: Optional[Context],
+                   module_public: bool = True, module_hidden: bool = False) -> bool:
+        """Add symbol to the current symbol table."""
+        raise NotImplementedError
+
+    @abstractmethod
+    def add_symbol_skip_local(self, name: str, node: SymbolNode) -> None:
+        """Add symbol to the current symbol table, skipping locals.
+
+        This is used to store symbol nodes in a symbol table that
+        is going to be serialized (local namespaces are not serialized).
+        See implementation docstring for more details.
+        """
         raise NotImplementedError
 
     @abstractmethod

test-data/unit/check-incremental.test

@@ -4931,3 +4931,40 @@ import pack.mod
 import a
 [out]
 [out2]
+
+[case testNewAnalyzerIncrementalBrokenNamedTuple]
+# flags: --new-semantic-analyzer
+import a
+[file a.py]
+from b import NT
+x: NT
+[file a.py.2]
+from b import NT
+x: NT
+reveal_type(x)
+[file b.py]
+from typing import NamedTuple
+NT = NamedTuple('BadName', [('x', int)])
+[out]
+[out2]
+tmp/a.py:3: error: Revealed type is 'Tuple[builtins.int, fallback=b.BadName@2]'
+
+[case testNewAnalyzerIncrementalMethodNamedTuple]
+# flags: --new-semantic-analyzer
+import a
+[file a.py]
+from b import C
+x: C
+[file a.py.2]
+from b import C
+x: C
+reveal_type(x.h)
+[file b.py]
+from typing import NamedTuple
+class C:
+    def __init__(self) -> None:
+        self.h: Hidden
+        Hidden = NamedTuple('Hidden', [('x', int)])
+[out]
+[out2]
+tmp/a.py:3: error: Revealed type is 'Tuple[builtins.int, fallback=b.C.Hidden@5]'