New semantic analyzer: don't add submodules to symbol tables (#7005)

Previously we added each submodule implicitly to the the symbol table of
the parent package. This PR removes this and instead we look up names from
the modules dictionary if they aren't found in the symbol table. The change
only affects the new semantic analyzer.

This provides a foundation that should make #6828 much easier to address. It 
also arguably cleans up the code.

Also refactor some related code to avoid duplication.

This isn't a pure refactor since some error messages are slightly different.

Author: JukkaL

mypy/build.py

@@ -1876,20 +1876,17 @@ def patch_dependency_parents(self) -> None:
         details.
 
         However, this patching process can occur after `a` has been parsed and
-        serialized during increment mode. Consequently, we need to repeat this
+        serialized during incremental mode. Consequently, we need to repeat this
         patch when deserializing a cached file.
 
         This function should be called only when processing fresh SCCs -- the
         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:
+        if not self.manager.options.new_semantic_analyzer:
             analyzer = self.manager.semantic_analyzer
-        for dep in self.dependencies:
-            analyzer.add_submodules_to_parent_modules(dep, True)
+            for dep in self.dependencies:
+                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

@@ -99,9 +99,7 @@
     Plugin, ClassDefContext, SemanticAnalyzerPluginInterface,
     DynamicClassDefContext
 )
-from mypy.util import (
-    get_prefix, correct_relative_import, unmangle, module_prefix
-)
+from mypy.util import correct_relative_import, unmangle, module_prefix
 from mypy.scope import Scope
 from mypy.newsemanal.semanal_shared import (
     SemanticAnalyzerInterface, set_callable_name, calculate_tuple_fallback, PRIORITY_FALLBACKS
@@ -1621,44 +1619,6 @@ def visit_import(self, i: Import) -> None:
                 base = id.split('.')[0]
                 self.add_module_symbol(base, base, module_public=module_public,
                                        context=i, module_hidden=not module_public)
-                self.add_submodules_to_parent_modules(id, module_public)
-
-    def add_submodules_to_parent_modules(self, id: str, module_public: bool) -> None:
-        """Recursively adds a reference to a newly loaded submodule to its parent.
-
-        When you import a submodule in any way, Python will add a reference to that
-        submodule to its parent. So, if you do something like `import A.B` or
-        `from A import B` or `from A.B import Foo`, Python will add a reference to
-        module A.B to A's namespace.
-
-        Note that this "parent patching" process is completely independent from any
-        changes made to the *importer's* namespace. For example, if you have a file
-        named `foo.py` where you do `from A.B import Bar`, then foo's namespace will
-        be modified to contain a reference to only Bar. Independently, A's namespace
-        will be modified to contain a reference to `A.B`.
-        """
-        while '.' in id:
-            parent, child = id.rsplit('.', 1)
-            parent_mod = self.modules.get(parent)
-            if parent_mod and self.allow_patching(parent_mod, child):
-                child_mod = self.modules.get(id)
-                if child_mod:
-                    sym = SymbolTableNode(GDEF, child_mod,
-                                          module_public=module_public,
-                                          no_serialize=True)
-                else:
-                    # Construct a dummy Var with Any type.
-                    any_type = AnyType(TypeOfAny.from_unimported_type,
-                                       missing_import_name=id)
-                    var = Var(child, any_type)
-                    var._fullname = child
-                    var.is_ready = True
-                    var.is_suppressed_import = True
-                    sym = SymbolTableNode(GDEF, var,
-                                          module_public=module_public,
-                                          no_serialize=True)
-                parent_mod.names[child] = sym
-            id = parent
 
     def allow_patching(self, parent_mod: MypyFile, child: str) -> bool:
         if child not in parent_mod.names:
@@ -1671,7 +1631,6 @@ def allow_patching(self, parent_mod: MypyFile, child: str) -> bool:
     def visit_import_from(self, imp: ImportFrom) -> None:
         self.statement = imp
         import_id = self.correct_relative_import(imp)
-        self.add_submodules_to_parent_modules(import_id, True)
         module = self.modules.get(import_id)
         for id, as_id in imp.names:
             node = module.names.get(id) if module else None
@@ -1687,13 +1646,12 @@ def visit_import_from(self, imp: ImportFrom) -> None:
                 if mod is not None:
                     kind = self.current_symbol_kind()
                     node = SymbolTableNode(kind, mod)
-                    self.add_submodules_to_parent_modules(possible_module_id, True)
                 elif possible_module_id in self.missing_modules:
                     missing = True
             # If it is still not resolved, check for a module level __getattr__
             if (module and not node and (module.is_stub or self.options.python_version >= (3, 7))
                     and '__getattr__' in module.names):
-                # We use the fullname of the orignal definition so that we can
+                # We use the fullname of the original definition so that we can
                 # detect whether two imported names refer to the same thing.
                 fullname = import_id + '.' + id
                 gvar = self.create_getattr_var(module.names['__getattr__'], imported_id, fullname)
@@ -1823,7 +1781,6 @@ def visit_import_all(self, i: ImportAll) -> None:
                 # Any names could be missing from the current namespace if the target module
                 # namespace is incomplete.
                 self.mark_incomplete('*', i)
-            self.add_submodules_to_parent_modules(i_id, True)
             for name, node in m.names.items():
                 if node is None:
                     continue
@@ -3457,61 +3414,16 @@ def check_fixed_args(self, expr: CallExpr, numargs: int,
     def visit_member_expr(self, expr: MemberExpr) -> None:
         base = expr.expr
         base.accept(self)
-        # Bind references to module attributes.
         if isinstance(base, RefExpr) and isinstance(base.node, MypyFile):
-            # This branch handles the case foo.bar where foo is a module.
-            # In this case base.node is the module's MypyFile and we look up
-            # bar in its namespace.  This must be done for all types of bar.
-            file = base.node
-            # TODO: Should we actually use this? Not sure if this makes a difference.
-            # if file.fullname() == self.cur_mod_id:
-            #     names = self.globals
-            # else:
-            #     names = file.names
-            n = file.names.get(expr.name, None)
-            if n and not n.module_hidden:
-                n = self.rebind_symbol_table_node(n)
-                if n:
-                    if isinstance(n.node, PlaceholderNode):
-                        self.process_placeholder(expr.name, 'attribute', expr)
-                        return
-                    # TODO: What if None?
-                    expr.kind = n.kind
-                    expr.fullname = n.fullname
-                    expr.node = n.node
-            elif (file is not None and (file.is_stub or self.options.python_version >= (3, 7))
-                    and '__getattr__' in file.names):
-                # If there is a module-level __getattr__, then any attribute on the module is valid
-                # per PEP 484.
-                getattr_defn = file.names['__getattr__']
-                if not getattr_defn:
-                    typ = AnyType(TypeOfAny.from_error)  # type: Type
-                elif isinstance(getattr_defn.node, (FuncDef, Var)):
-                    if isinstance(getattr_defn.node.type, CallableType):
-                        typ = getattr_defn.node.type.ret_type
-                    else:
-                        typ = AnyType(TypeOfAny.from_error)
-                else:
-                    typ = AnyType(TypeOfAny.from_error)
-                expr.kind = GDEF
-                expr.fullname = '{}.{}'.format(file.fullname(), expr.name)
-                expr.node = Var(expr.name, type=typ)
-            else:
-                if self.is_incomplete_namespace(file.fullname()):
-                    self.record_incomplete_ref()
+            # Handle module attribute.
+            sym = self.get_module_symbol(base.node, expr.name)
+            if sym:
+                if isinstance(sym.node, PlaceholderNode):
+                    self.process_placeholder(expr.name, 'attribute', expr)
                     return
-                # We only catch some errors here; the rest will be
-                # caught during type checking.
-                #
-                # This way we can report a larger number of errors in
-                # one type checker run. If we reported errors here,
-                # the build would terminate after semantic analysis
-                # and we wouldn't be able to report any type errors.
-                full_name = '%s.%s' % (file.fullname() if file is not None else None, expr.name)
-                mod_name = " '%s'" % file.fullname() if file is not None else ''
-                if full_name in obsolete_name_mapping:
-                    self.fail("Module%s has no attribute %r (it's now called %r)" % (
-                        mod_name, expr.name, obsolete_name_mapping[full_name]), expr)
+                expr.kind = sym.kind
+                expr.fullname = sym.fullname
+                expr.node = sym.node
         elif isinstance(base, RefExpr):
             # This branch handles the case C.bar (or cls.bar or self.bar inside
             # a classmethod/method), where C is a class and bar is a type
@@ -3851,10 +3763,11 @@ def lookup_qualified(self, name: str, ctx: Context,
         if sym:
             for i in range(1, len(parts)):
                 node = sym.node
+                part = parts[i]
                 if isinstance(node, TypeInfo):
-                    nextsym = node.get(parts[i])
+                    nextsym = node.get(part)
                 elif isinstance(node, MypyFile):
-                    nextsym = self.get_module_symbol(node, parts[i:])
+                    nextsym = self.get_module_symbol(node, part)
                     namespace = node.fullname()
                 elif isinstance(node, PlaceholderNode):
                     return sym
@@ -3881,27 +3794,40 @@ def lookup_type_node(self, expr: Expression) -> Optional[SymbolTableNode]:
             return n
         return None
 
-    def get_module_symbol(self, node: MypyFile, parts: List[str]) -> Optional[SymbolTableNode]:
-        """Look up a symbol from the module symbol table."""
-        # TODO: Use this logic in more places?
+    def get_module_symbol(self, node: MypyFile, name: str) -> Optional[SymbolTableNode]:
+        """Look up a symbol from a module.
+
+        Return None if no matching symbol could be bound.
+        """
         module = node.fullname()
         names = node.names
-        # Rebind potential references to old version of current module in
-        # fine-grained incremental mode.
-        if module == self.cur_mod_id:
-            names = self.globals
-        sym = names.get(parts[0], None)
-        if (not sym
-                and '__getattr__' in names
-                and not self.is_incomplete_namespace(module)
-                and (node.is_stub or self.options.python_version >= (3, 7))):
-            fullname = module + '.' + '.'.join(parts)
-            gvar = self.create_getattr_var(names['__getattr__'],
-                                           parts[0], fullname)
-            if gvar:
-                sym = SymbolTableNode(GDEF, gvar)
+        sym = names.get(name)
+        if not sym:
+            fullname = module + '.' + name
+            if fullname in self.modules:
+                sym = SymbolTableNode(GDEF, self.modules[fullname])
+            elif self.is_incomplete_namespace(module):
+                self.record_incomplete_ref()
+            elif ('__getattr__' in names
+                    and (node.is_stub
+                         or self.options.python_version >= (3, 7))):
+                gvar = self.create_getattr_var(names['__getattr__'], name, fullname)
+                if gvar:
+                    sym = SymbolTableNode(GDEF, gvar)
+            elif self.is_missing_module(fullname):
+                # We use the fullname of the original definition so that we can
+                # detect whether two names refer to the same thing.
+                var_type = AnyType(TypeOfAny.from_unimported_type)
+                v = Var(name, type=var_type)
+                v._fullname = fullname
+                sym = SymbolTableNode(GDEF, v)
+        elif sym.module_hidden:
+            sym = None
         return sym
 
+    def is_missing_module(self, module: str) -> bool:
+        return self.options.ignore_missing_imports or module in self.missing_modules
+
     def implicit_symbol(self, sym: SymbolTableNode, name: str, parts: List[str],
                         source_type: AnyType) -> SymbolTableNode:
         """Create symbol for a qualified name reference through Any type."""
@@ -4308,24 +4234,6 @@ def process_placeholder(self, name: str, kind: str, ctx: Context) -> None:
     def cannot_resolve_name(self, name: str, kind: str, ctx: Context) -> None:
         self.fail('Cannot resolve {} "{}" (possible cyclic definition)'.format(kind, name), ctx)
 
-    def rebind_symbol_table_node(self, n: SymbolTableNode) -> Optional[SymbolTableNode]:
-        """If node refers to old version of module, return reference to new version.
-
-        If the reference is removed in the new version, return None.
-        """
-        # TODO: Handle type variables and other sorts of references
-        if isinstance(n.node, (FuncDef, OverloadedFuncDef, TypeInfo, Var, TypeAlias)):
-            # TODO: Why is it possible for fullname() to be None, even though it's not
-            #   annotated as Optional[str]?
-            # TODO: Do this for all modules in the set of modified files
-            # TODO: This doesn't work for things nested within classes
-            if n.node.fullname() and get_prefix(n.node.fullname()) == self.cur_mod_id:
-                # This is an indirect reference to a name defined in the current module.
-                # Rebind it.
-                return self.globals.get(n.node.name())
-        # No need to rebind.
-        return n
-
     def qualified_name(self, name: str) -> str:
         if self.type is not None:
             return self.type._fullname + '.' + name

test-data/unit/check-modules.test

@@ -2557,6 +2557,10 @@ import whatever.works
 import a.b
 x = whatever.works.f()
 y = a.b.f()
+xx: whatever.works.C
+yy: a.b.C
+xx2: whatever.works.C.D
+yy2: a.b.C.D
 [file a/__init__.pyi]
 # empty
 [out]

test-data/unit/check-newsemanal.test

@@ -2497,3 +2497,22 @@ def force(x: Literal[42]) -> None: pass
 force(reveal_type(var))  # E: Revealed type is 'Literal[42]'
 
 class Yes: ...
+
+[case testNewAnalyzerImportCycleWithIgnoreMissingImports]
+# flags: --ignore-missing-imports
+import p
+reveal_type(p.get)  # E: Revealed type is 'def () -> builtins.int'
+
+[file p/__init__.pyi]
+from . import api
+get = api.get
+
+[file p/api.pyi]
+import p
+
+def get() -> int: ...
+
+[case testUseObsoleteNameForTypeVar3]
+import typing
+t = typing.typevar('t') # E: Module has no attribute "typevar"
+[builtins fixtures/module.pyi]

test-data/unit/fine-grained-modules.test

@@ -812,6 +812,7 @@ class Baz:
 ==
 
 [case testDeleteFileWithinPackage]
+# flags: --new-semantic-analyzer
 import a
 [file a.py]
 import m.x
@@ -826,6 +827,7 @@ a.py:2: error: Too many arguments for "g"
 ==
 a.py:1: error: Cannot find module named 'm.x'
 a.py:1: note: See https://mypy.readthedocs.io/en/latest/running_mypy.html#missing-imports
+a.py:2: error: Module has no attribute "x"
 
 [case testDeletePackage1]
 import p.a
@@ -859,6 +861,7 @@ main:1: error: Cannot find module named 'p'
 main:1: note: See https://mypy.readthedocs.io/en/latest/running_mypy.html#missing-imports
 
 [case testDeletePackage3]
+# flags: --new-semantic-analyzer
 import p.a
 p.a.f(1)
 [file p/__init__.py]
@@ -868,14 +871,15 @@ def f(x: str) -> None: pass
 [delete p/__init__.py.3]
 [builtins fixtures/module.pyi]
 [out]
-main:2: error: Argument 1 to "f" has incompatible type "int"; expected "str"
+main:3: error: Argument 1 to "f" has incompatible type "int"; expected "str"
 ==
-main:1: error: Cannot find module named 'p.a'
-main:1: note: See https://mypy.readthedocs.io/en/latest/running_mypy.html#missing-imports
+main:2: error: Cannot find module named 'p.a'
+main:2: note: See https://mypy.readthedocs.io/en/latest/running_mypy.html#missing-imports
+main:3: error: Module has no attribute "a"
 ==
-main:1: error: Cannot find module named 'p.a'
-main:1: note: See https://mypy.readthedocs.io/en/latest/running_mypy.html#missing-imports
-main:1: error: Cannot find module named 'p'
+main:2: error: Cannot find module named 'p.a'
+main:2: note: See https://mypy.readthedocs.io/en/latest/running_mypy.html#missing-imports
+main:2: error: Cannot find module named 'p'
 
 [case testDeletePackage4]
 import p.a

test-data/unit/semanal-errors.test

@@ -1273,11 +1273,11 @@ main:1: error: Name 'typevar' is not defined
 main:1: note: Did you forget to import it from "typing"? (Suggestion: "from typing import TypeVar")
 
 [case testUseObsoleteNameForTypeVar3]
+# flags: --no-new-semantic-analyzer
 import typing
 t = typing.typevar('t')
 [out]
-main:2: error: Module 'typing' has no attribute 'typevar' (it's now called 'typing.TypeVar')
---' (work around syntax highlighting :-/)
+main:3: error: Module 'typing' has no attribute 'typevar' (it's now called 'typing.TypeVar')
 
 [case testInvalidTypeAnnotation]
 import typing