From d29e316043cc1a5ac404a1ad5bbebb3d3d9d280e Mon Sep 17 00:00:00 2001 From: Jukka Lehtosalo Date: Mon, 5 Mar 2018 17:13:58 +0000 Subject: [PATCH 01/14] Resolve more precise types for some imports in import cycles Previously we could fall back to the `Any` type if we imported an imported name. This depended on the order in which an SCC was processed. This fixes the behavior for `from m import x` and also makes the behavior consistent with fine-grained incremental mode. I'll fix other kinds of imports separately. --- mypy/nodes.py | 29 +++++++++++++++++++ mypy/semanal.py | 47 +++++++++++++++++++++++++++---- mypy/semanal_pass1.py | 31 ++++++++++++++------ test-data/unit/check-modules.test | 16 +++++++++++ 4 files changed, 110 insertions(+), 13 deletions(-) diff --git a/mypy/nodes.py b/mypy/nodes.py index d7c44c6194cb..30dff4502b65 100644 --- a/mypy/nodes.py +++ b/mypy/nodes.py @@ -325,6 +325,35 @@ def accept(self, visitor: StatementVisitor[T]) -> T: return visitor.visit_import_all(self) +class ImportedName(SymbolNode): + """Indirect reference to a fullname stored in symbol table. + + This node is not present in the original program as such. This is + just a temporary artifact in binding imported names. After semantic + analysis pass 2, these references should be replaced with direct + reference to a real AST node. + + Note that this is neither a Statement nor an Expressions so this + can't be visited. + """ + + def __init__(self, target_fullname: str) -> None: + self.target_fullname = target_fullname + + def name(self) -> str: + return self.target_fullname.split('.')[-1] + + def fullname(self) -> str: + return self.target_fullname + + def serialize(self) -> JsonDict: + assert False, "ImportedName leaked from semantic analysis" + + @classmethod + def deserialize(cls, data: JsonDict) -> 'ImportedName': + assert False, "ImportedName should never be serialized" + + class FuncBase(Node): """Abstract base class for function-like nodes""" diff --git a/mypy/semanal.py b/mypy/semanal.py index a924689cae54..c47a82a34bf5 100644 --- a/mypy/semanal.py +++ b/mypy/semanal.py @@ -55,7 +55,7 @@ YieldFromExpr, NamedTupleExpr, TypedDictExpr, NonlocalDecl, SymbolNode, SetComprehension, DictionaryComprehension, TYPE_ALIAS, TypeAliasExpr, YieldExpr, ExecStmt, Argument, BackquoteExpr, ImportBase, AwaitExpr, - IntExpr, FloatExpr, UnicodeExpr, EllipsisExpr, TempNode, EnumCallExpr, + IntExpr, FloatExpr, UnicodeExpr, EllipsisExpr, TempNode, EnumCallExpr, ImportedName, COVARIANT, CONTRAVARIANT, INVARIANT, UNBOUND_IMPORTED, LITERAL_YES, ARG_OPT, nongen_builtins, collections_type_aliases, get_member_expr_fullname, ) @@ -1488,6 +1488,8 @@ def visit_import_from(self, imp: ImportFrom) -> None: module = self.modules.get(import_id) for id, as_id in imp.names: node = module.names.get(id) if module else None + node = self.dereference_module_cross_ref(node) + missing = False possible_module_id = import_id + '.' + id @@ -1516,11 +1518,14 @@ def visit_import_from(self, imp: ImportFrom) -> None: ast_node = Var(name, type=typ) symbol = SymbolTableNode(GDEF, ast_node) self.add_symbol(name, symbol, imp) - return + continue if node and node.kind != UNBOUND_IMPORTED and not node.module_hidden: node = self.normalize_type_alias(node, imp) if not node: - return + # Normalization failed because target is not defined. Avoid duplicate + # error messages by marking the imported name as unknown. + self.add_unknown_symbol(as_id or id, imp, is_import=True) + continue imported_id = as_id or id existing_symbol = self.globals.get(imported_id) if existing_symbol: @@ -1550,6 +1555,26 @@ def visit_import_from(self, imp: ImportFrom) -> None: # Missing module. self.add_unknown_symbol(as_id or id, imp, is_import=True) + def dereference_module_cross_ref( + self, node: Optional[SymbolTableNode]) -> Optional[SymbolTableNode]: + """Dereference cross references to other modules (if any). + + If the node is not a cross reference, return it unmodified. + """ + seen = set() # type: Set[str] + # Continue until we reach a node that's nota cross reference (or until we find + # nothing). + while node and isinstance(node.node, ImportedName): + fullname = node.node.fullname() + if fullname in seen: + # Looks like a reference cycle. Just break it. + # TODO: Generate a more specific error message. + node = None + break + node = self.lookup_fully_qualified_or_none(fullname) + seen.add(fullname) + return node + def process_import_over_existing_name(self, imported_id: str, existing_symbol: SymbolTableNode, module_symbol: SymbolTableNode, @@ -1573,6 +1598,14 @@ def process_import_over_existing_name(self, def normalize_type_alias(self, node: SymbolTableNode, ctx: Context) -> Optional[SymbolTableNode]: + """If node refers to a built-intype alias, normalize it. + + An example normalization is 'typing.List' -> '__builtins__.list'. + + By default, if the node doesn't refer to a built-in type alias, return + the original node. If normalization fails because the target isn't + defined, return None. + """ normalized = False fullname = node.fullname if fullname in type_aliases: @@ -3861,6 +3894,8 @@ def is_module_scope(self) -> bool: def add_symbol(self, name: str, node: SymbolTableNode, context: Context) -> None: + # NOTE: This logic mostly parallels SemanticAnalyzerPass1.add_symbol. If you change + # this, you may have to change the other method as well. if self.is_func_scope(): assert self.locals[-1] is not None if name in self.locals[-1]: @@ -3873,8 +3908,10 @@ def add_symbol(self, name: str, node: SymbolTableNode, self.type.names[name] = node else: existing = self.globals.get(name) - if existing and (not isinstance(node.node, MypyFile) or - existing.node != node.node) and existing.kind != UNBOUND_IMPORTED: + if (existing + and (not isinstance(node.node, MypyFile) or existing.node != node.node) + and existing.kind != UNBOUND_IMPORTED + and not isinstance(existing.node, ImportedName)): # Modules can be imported multiple times to support import # of multiple submodules of a package (e.g. a.x and a.y). ok = False diff --git a/mypy/semanal_pass1.py b/mypy/semanal_pass1.py index 15ee31ecc8e6..55c1d4057eb1 100644 --- a/mypy/semanal_pass1.py +++ b/mypy/semanal_pass1.py @@ -23,14 +23,15 @@ from mypy.nodes import ( MypyFile, SymbolTable, SymbolTableNode, Var, Block, AssignmentStmt, FuncDef, Decorator, ClassDef, TypeInfo, ImportFrom, Import, ImportAll, IfStmt, WhileStmt, ForStmt, WithStmt, - TryStmt, OverloadedFuncDef, Lvalue, Context, LDEF, GDEF, MDEF, UNBOUND_IMPORTED, MODULE_REF, - implicit_module_attrs + TryStmt, OverloadedFuncDef, Lvalue, Context, ImportedName, LDEF, GDEF, MDEF, UNBOUND_IMPORTED, + MODULE_REF, implicit_module_attrs ) from mypy.types import Type, UnboundType, UnionType, AnyType, TypeOfAny, NoneTyp from mypy.semanal import SemanticAnalyzerPass2, infer_reachability_of_if_statement from mypy.options import Options from mypy.sametypes import is_same_type from mypy.visitor import NodeVisitor +from mypy.util import correct_relative_import class SemanticAnalyzerPass1(NodeVisitor[None]): @@ -62,6 +63,7 @@ def visit_file(self, file: MypyFile, fnam: str, mod_id: str, options: Options) - self.pyversion = options.python_version self.platform = options.platform sem.cur_mod_id = mod_id + sem.cur_mod_node = file sem.errors.set_file(fnam, mod_id) sem.globals = SymbolTable() sem.global_decls = [set()] @@ -148,7 +150,8 @@ def visit_func_def(self, func: FuncDef) -> None: if at_module and func.name() in sem.globals: # Already defined in this module. original_sym = sem.globals[func.name()] - if original_sym.kind == UNBOUND_IMPORTED: + if (original_sym.kind == UNBOUND_IMPORTED or + isinstance(original_sym.node, ImportedName)): # Ah this is an imported name. We can't resolve them now, so we'll postpone # this until the main phase of semantic analysis. return @@ -241,7 +244,16 @@ def visit_import_from(self, node: ImportFrom) -> None: for name, as_name in node.names: imported_name = as_name or name if imported_name not in self.sem.globals: - self.add_symbol(imported_name, SymbolTableNode(UNBOUND_IMPORTED, None), node) + target_module, ok = correct_relative_import( + self.sem.cur_mod_id, + node.relative, + node.id, + self.sem.cur_mod_node.is_package_init_file()) + if ok: + target_name = '%s.%s' % (target_module, imported_name) + link_node = ImportedName(target_name) + sym = SymbolTableNode(GDEF, link_node) + self.add_symbol(imported_name, sym, context=node) def visit_import(self, node: Import) -> None: node.is_top_level = self.sem.is_module_scope() @@ -312,8 +324,9 @@ def kind_by_scope(self) -> int: def add_symbol(self, name: str, node: SymbolTableNode, context: Context) -> None: - # This is related to SemanticAnalyzerPass2.add_symbol. Since both methods will - # be called on top-level definitions, they need to co-operate. + # NOTE: This is closely related to SemanticAnalyzerPass2.add_symbol. Since both methods + # will be called on top-level definitions, they need to co-operate. If you change + # this, you may have to change the other method as well. if self.sem.is_func_scope(): assert self.sem.locals[-1] is not None if name in self.sem.locals[-1]: @@ -325,8 +338,10 @@ def add_symbol(self, name: str, node: SymbolTableNode, else: assert self.sem.type is None # Pass 1 doesn't look inside classes existing = self.sem.globals.get(name) - if existing and (not isinstance(node.node, MypyFile) or - existing.node != node.node) and existing.kind != UNBOUND_IMPORTED: + if (existing + and (not isinstance(node.node, MypyFile) or existing.node != node.node) + and existing.kind != UNBOUND_IMPORTED + and not isinstance(existing.node, ImportedName)): # Modules can be imported multiple times to support import # of multiple submodules of a package (e.g. a.x and a.y). ok = False diff --git a/test-data/unit/check-modules.test b/test-data/unit/check-modules.test index 06e926c7711c..c354bd1739cf 100644 --- a/test-data/unit/check-modules.test +++ b/test-data/unit/check-modules.test @@ -1330,6 +1330,8 @@ pass [file b] pass +-- Misc + [case testScriptsAreNotModules] # cmd: mypy a b [file a] @@ -1955,3 +1957,17 @@ import c tmp/b.py:1: error: Cannot find module named 'c' main:1: error: Cannot find module named 'c' main:1: note: (Perhaps setting MYPYPATH or using the "--ignore-missing-imports" flag would help) + +[case testIndirectImportWithinCycle1] +import a +[file a.py] +from b import f +from c import x +[file b.py] +from c import y +from a import x +def f() -> None: pass +reveal_type(x) # E: Revealed type is 'builtins.str' +[file c.py] +x = str() +y = int() From 80b9e2052b07f92ce095e95680380e923722b596 Mon Sep 17 00:00:00 2001 From: Jukka Lehtosalo Date: Mon, 5 Mar 2018 17:51:51 +0000 Subject: [PATCH 02/14] Fix crash on ignored __init__.py and add test cases --- mypy/semanal.py | 25 +++++++++++------------ test-data/unit/check-modules.test | 33 ++++++++++++++++++++++++++++++- 2 files changed, 44 insertions(+), 14 deletions(-) diff --git a/mypy/semanal.py b/mypy/semanal.py index c47a82a34bf5..f4e6ab1bb160 100644 --- a/mypy/semanal.py +++ b/mypy/semanal.py @@ -3846,22 +3846,21 @@ def lookup_fully_qualified(self, name: str) -> SymbolTableNode: n = next_sym.node return n.names[parts[-1]] - def lookup_fully_qualified_or_none(self, name: str) -> Optional[SymbolTableNode]: - """Lookup a fully qualified name. + def lookup_fully_qualified_or_none(self, fullname: str) -> Optional[SymbolTableNode]: + """Lookup a fully qualified name that refers to a module-level definition. Don't assume that the name is defined. This happens in the global namespace -- - the local module namespace is ignored. + the local module namespace is ignored. This does not dereference indirect + refs. + + Note that this can't be used for names nested in class namespaces. """ - assert '.' in name - parts = name.split('.') - n = self.modules[parts[0]] - for i in range(1, len(parts) - 1): - next_sym = n.names.get(parts[i]) - if not next_sym: - return None - assert isinstance(next_sym.node, MypyFile) - n = next_sym.node - return n.names.get(parts[-1]) + assert '.' in fullname + module, name = fullname.rsplit('.', maxsplit=1) + if module not in self.modules: + return None + filenode = self.modules[module] + return filenode.names.get(name) def qualified_name(self, n: str) -> str: if self.type is not None: diff --git a/test-data/unit/check-modules.test b/test-data/unit/check-modules.test index c354bd1739cf..e0a883d85701 100644 --- a/test-data/unit/check-modules.test +++ b/test-data/unit/check-modules.test @@ -1958,7 +1958,7 @@ tmp/b.py:1: error: Cannot find module named 'c' main:1: error: Cannot find module named 'c' main:1: note: (Perhaps setting MYPYPATH or using the "--ignore-missing-imports" flag would help) -[case testIndirectImportWithinCycle1] +[case testIndirectFromImportWithinCycle] import a [file a.py] from b import f @@ -1971,3 +1971,34 @@ reveal_type(x) # E: Revealed type is 'builtins.str' [file c.py] x = str() y = int() + +[case testIndirectFromImportWithinCycleInPackage] +import p.a +[file p/__init__.py] +[file p/a.py] +from p.b import f +from p.c import x +[file p/b.py] +from p.c import y +from p.a import x +def f() -> None: pass +reveal_type(x) # E: Revealed type is 'builtins.str' +[file p/c.py] +x = str() +y = int() + +[case testIndirectFromImportWithinCycleInPackageIgnoredInit] +# cmd: mypy -m p.a p.b p.c +# flags: --follow-imports=skip --ignore-missing-imports +[file p/__init__.py] +[file p/a.py] +from p.b import f +from p.c import x +[file p/b.py] +from p.c import y +from p.a import x +def f() -> None: pass +reveal_type(x) # E: Revealed type is 'builtins.str' +[file p/c.py] +x = str() +y = int() From 92d7cfc92d32ad08b3adfbd418d3cf9bdd883cfe Mon Sep 17 00:00:00 2001 From: Jukka Lehtosalo Date: Tue, 6 Mar 2018 12:20:53 +0000 Subject: [PATCH 03/14] Fix type forward references in properties --- mypy/semanal_pass3.py | 10 +++++- test-data/unit/check-incremental.test | 19 +++++++++++ test-data/unit/check-modules.test | 16 ++++++++++ test-data/unit/check-serialize.test | 45 +++++++++++++++++++++++++++ 4 files changed, 89 insertions(+), 1 deletion(-) diff --git a/mypy/semanal_pass3.py b/mypy/semanal_pass3.py index 1b815dd450e4..ca9398222c60 100644 --- a/mypy/semanal_pass3.py +++ b/mypy/semanal_pass3.py @@ -18,7 +18,7 @@ Node, Expression, MypyFile, FuncDef, FuncItem, Decorator, RefExpr, Context, TypeInfo, ClassDef, Block, TypedDictExpr, NamedTupleExpr, AssignmentStmt, IndexExpr, TypeAliasExpr, NameExpr, CallExpr, NewTypeExpr, ForStmt, WithStmt, CastExpr, TypeVarExpr, TypeApplication, Lvalue, - TupleExpr, RevealTypeExpr, SymbolTableNode, Var, ARG_POS, OverloadedFuncDef + TupleExpr, RevealTypeExpr, SymbolTableNode, SymbolTable, Var, ARG_POS, OverloadedFuncDef ) from mypy.types import ( Type, Instance, AnyType, TypeOfAny, CallableType, TupleType, TypeVarType, TypedDictType, @@ -67,6 +67,7 @@ def visit_file(self, file_node: MypyFile, fnam: str, options: Options, with experiments.strict_optional_set(options.strict_optional): self.scope.enter_file(file_node.fullname()) self.accept(file_node) + self.analyze_symbol_table(file_node.names) self.scope.leave() del self.cur_mod_node self.patches = [] @@ -151,6 +152,7 @@ def visit_class_def(self, tdef: ClassDef) -> None: self.analyze(tdef.analyzed.info.tuple_type, tdef.analyzed, warn=True) self.analyze_info(tdef.analyzed.info) super().visit_class_def(tdef) + self.analyze_symbol_table(tdef.info.names) self.scope.leave() def visit_decorator(self, dec: Decorator) -> None: @@ -386,6 +388,12 @@ def analyze_types(self, types: List[Type], node: Node) -> None: self.cur_mod_node.alias_deps[target].update(analyzer.aliases_used) self.generate_type_patches(node, indicator, warn=False) + def analyze_symbol_table(self, names: SymbolTable) -> None: + """Analyze types in symbol table nodes only (shallow).""" + for node in names.values(): + if node.type_override: + self.analyze(node.type_override, node) + def generate_type_patches(self, node: Union[Node, SymbolTableNode], indicator: Dict[str, bool], diff --git a/test-data/unit/check-incremental.test b/test-data/unit/check-incremental.test index 03582bad4e0e..720df07d6cda 100644 --- a/test-data/unit/check-incremental.test +++ b/test-data/unit/check-incremental.test @@ -4127,3 +4127,22 @@ class Foo: x = Foo() [out] [out2] + +[case testForwardReferenceToImportedAliasInProperty] +import a + +class A: + @property + def p(self) -> C: pass + @p.setter + def p(self, c: C) -> None: pass + @p.deleter + def p(self) -> None: pass + +from m import C +[file m.py] +C = str +[file a.py] +[file a.py.2] +# dummy +[builtins fixtures/property.pyi] diff --git a/test-data/unit/check-modules.test b/test-data/unit/check-modules.test index e0a883d85701..4200e4d72b7d 100644 --- a/test-data/unit/check-modules.test +++ b/test-data/unit/check-modules.test @@ -2002,3 +2002,19 @@ reveal_type(x) # E: Revealed type is 'builtins.str' [file p/c.py] x = str() y = int() + +[case testForwardReferenceToImportedAlias] +class A: + @property + def p(self) -> C: pass + @p.setter + def p(self, c: C) -> None: pass + @p.deleter + def p(self) -> None: pass + +#reveal_type(A().x) + +from m import C +[file m.py] +C = str +[builtins fixtures/property.pyi] diff --git a/test-data/unit/check-serialize.test b/test-data/unit/check-serialize.test index a230c07dd181..7f26549af7c1 100644 --- a/test-data/unit/check-serialize.test +++ b/test-data/unit/check-serialize.test @@ -1268,3 +1268,48 @@ class Test: tmp/a.py:2: error: Revealed type is 'b.' [out2] tmp/a.py:2: error: Revealed type is 'b.' + +[case testSerializeForwardReferenceToAliasInProperty] +import a +[file a.py] +import b +[file a.py.2] +import b +reveal_type(b.A().p) +[file b.py] + +class A: + @property + def p(self) -> C: pass + @p.setter + def p(self, c: C) -> None: pass + @p.deleter + def p(self) -> None: pass + +C = str +[builtins fixtures/property.pyi] +[out2] +tmp/a.py:2: error: Revealed type is 'builtins.str' + +[case testSerializeForwardReferenceToImportedAliasInProperty] +import a +[file a.py] +import b +[file a.py.2] +import b +reveal_type(b.A().p) +[file b.py] +class A: + @property + def p(self) -> C: pass + @p.setter + def p(self, c: C) -> None: pass + @p.deleter + def p(self) -> None: pass + +from m import C +[file m.py] +C = str +[builtins fixtures/property.pyi] +[out2] +tmp/a.py:2: error: Revealed type is 'builtins.str' From e14578c20befaf3dce5f5215c5723037d7796ce3 Mon Sep 17 00:00:00 2001 From: Jukka Lehtosalo Date: Tue, 6 Mar 2018 14:33:55 +0000 Subject: [PATCH 04/14] Refactoring: use abstract class instead of passing multiple callbacks This makes it easier to add additional functions to the interface. --- mypy/semanal.py | 16 +++++-------- mypy/semanal_pass3.py | 23 ++++++++++++++----- mypy/semanal_shared.py | 35 +++++++++++++++++++++++++++++ mypy/typeanal.py | 51 +++++++++++++++++------------------------- 4 files changed, 79 insertions(+), 46 deletions(-) diff --git a/mypy/semanal.py b/mypy/semanal.py index f4e6ab1bb160..c319eafb55da 100644 --- a/mypy/semanal.py +++ b/mypy/semanal.py @@ -84,7 +84,7 @@ from mypy.plugin import Plugin, ClassDefContext, SemanticAnalyzerPluginInterface from mypy import join from mypy.util import get_prefix, correct_relative_import -from mypy.semanal_shared import PRIORITY_FALLBACKS +from mypy.semanal_shared import SemanticAnalyzerInterface, PRIORITY_FALLBACKS from mypy.scope import Scope @@ -174,7 +174,9 @@ } -class SemanticAnalyzerPass2(NodeVisitor[None], SemanticAnalyzerPluginInterface): +class SemanticAnalyzerPass2(NodeVisitor[None], + SemanticAnalyzerInterface, + SemanticAnalyzerPluginInterface): """Semantically analyze parsed mypy files. The analyzer binds names and does various consistency checks for a @@ -1703,11 +1705,8 @@ def type_analyzer(self, *, third_pass: bool = False) -> TypeAnalyser: if tvar_scope is None: tvar_scope = self.tvar_scope - tpan = TypeAnalyser(self.lookup_qualified, - self.lookup_fully_qualified, + tpan = TypeAnalyser(self, tvar_scope, - self.fail, - self.note, self.plugin, self.options, self.is_typeshed_stub_file, @@ -1836,11 +1835,8 @@ def analyze_alias(self, rvalue: Expression, dynamic = bool(self.function_stack and self.function_stack[-1].is_dynamic()) global_scope = not self.type and not self.function_stack res = analyze_type_alias(rvalue, - self.lookup_qualified, - self.lookup_fully_qualified, + self, self.tvar_scope, - self.fail, - self.note, self.plugin, self.options, self.is_typeshed_stub_file, diff --git a/mypy/semanal_pass3.py b/mypy/semanal_pass3.py index ca9398222c60..5cd3acf329e6 100644 --- a/mypy/semanal_pass3.py +++ b/mypy/semanal_pass3.py @@ -33,10 +33,12 @@ from mypy.subtypes import is_subtype from mypy.sametypes import is_same_type from mypy.scope import Scope +from mypy.semanal_shared import SemanticAnalyzerInterface import mypy.semanal -class SemanticAnalyzerPass3(TraverserVisitor): +class SemanticAnalyzerPass3(TraverserVisitor, + SemanticAnalyzerInterface): """The third and final pass of semantic analysis. Check type argument counts and values of generic types, and perform some @@ -421,10 +423,7 @@ def analyze_info(self, info: TypeInfo) -> None: self.analyze(sym.node.type, sym.node) def make_type_analyzer(self, indicator: Dict[str, bool]) -> TypeAnalyserPass3: - return TypeAnalyserPass3(self.sem.lookup_qualified, - self.sem.lookup_fully_qualified, - self.fail, - self.sem.note, + return TypeAnalyserPass3(self, self.sem.plugin, self.options, self.is_typeshed_file, @@ -439,12 +438,24 @@ def check_for_omitted_generics(self, typ: Type) -> None: if t.type_of_any == TypeOfAny.from_omitted_generics: self.fail(messages.BARE_GENERIC, t) - def fail(self, msg: str, ctx: Context, *, blocker: bool = False) -> None: + def lookup_qualified(self, name: str, ctx: Context, + suppress_errors: bool = False) -> Optional[SymbolTableNode]: + return self.sem.lookup_qualified(name, ctx, suppress_errors=suppress_errors) + + def lookup_fully_qualified(self, fullname: str) -> Optional[SymbolTableNode]: + return self.sem.lookup_fully_qualified(fullname) + + def fail(self, msg: str, ctx: Context, serious: bool = False, *, + blocker: bool = False) -> None: + # TODO: Process all arguments self.errors.report(ctx.get_line(), ctx.get_column(), msg) def fail_blocker(self, msg: str, ctx: Context) -> None: self.fail(msg, ctx, blocker=True) + def note(self, msg: str, ctx: Context) -> None: + self.sem.note(msg, ctx) + def builtin_type(self, name: str, args: Optional[List[Type]] = None) -> Instance: names = self.modules['builtins'] sym = names.names[name] diff --git a/mypy/semanal_shared.py b/mypy/semanal_shared.py index b7ecbe16397f..d541a07b2da4 100644 --- a/mypy/semanal_shared.py +++ b/mypy/semanal_shared.py @@ -1,5 +1,11 @@ """Shared definitions used by different parts of semantic analysis.""" +from abc import abstractmethod +from typing import Optional + +from mypy.nodes import Context, SymbolTableNode + + # Priorities for ordering of patches within the final "patch" phase of semantic analysis # (after pass 3): @@ -9,3 +15,32 @@ PRIORITY_FALLBACKS = 1 # Checks type var values (does subtype checks) PRIORITY_TYPEVAR_VALUES = 2 + + +class SemanticAnalyzerInterface: + """A limited abstract interface to some generic semantic analyzer functionality. + + We use this interface for various reasons: + + * Looser coupling + * Cleaner import graph + * Less need to pass around callback functions + """ + + @abstractmethod + def lookup_qualified(self, name: str, ctx: Context, + suppress_errors: bool = False) -> Optional[SymbolTableNode]: + raise NotImplementedError + + @abstractmethod + def lookup_fully_qualified(self, name: str) -> SymbolTableNode: + raise NotImplementedError + + @abstractmethod + def fail(self, msg: str, ctx: Context, serious: bool = False, *, + blocker: bool = False) -> None: + raise NotImplementedError + + @abstractmethod + def note(self, msg: str, ctx: Context) -> None: + raise NotImplementedError diff --git a/mypy/typeanal.py b/mypy/typeanal.py index 337854d05709..15254e261305 100644 --- a/mypy/typeanal.py +++ b/mypy/typeanal.py @@ -1,5 +1,6 @@ """Semantic analysis of types""" +from abc import abstractmethod from collections import OrderedDict from typing import Callable, List, Optional, Set, Tuple, Iterator, TypeVar, Iterable, Dict, Union @@ -28,6 +29,7 @@ from mypy.tvar_scope import TypeVarScope from mypy.exprtotype import expr_to_unanalyzed_type, TypeTranslationError from mypy.plugin import Plugin, TypeAnalyzerPluginInterface, AnalyzeTypeContext +from mypy.semanal_shared import SemanticAnalyzerInterface from mypy import nodes, messages @@ -53,11 +55,8 @@ def analyze_type_alias(node: Expression, - lookup_func: Callable[[str, Context], Optional[SymbolTableNode]], - lookup_fqn_func: Callable[[str], SymbolTableNode], + api: SemanticAnalyzerInterface, tvar_scope: TypeVarScope, - fail_func: Callable[[str, Context], None], - note_func: Callable[[str, Context], None], plugin: Plugin, options: Options, is_typeshed_stub: bool, @@ -79,7 +78,7 @@ def analyze_type_alias(node: Expression, # class-referenced type variable as a type alias. It's easier to catch # that one in checkmember.py if node.kind == TVAR: - fail_func('Type variable "{}" is invalid as target for type alias'.format( + api.fail('Type variable "{}" is invalid as target for type alias'.format( node.fullname), node) return None if not (isinstance(node.node, TypeInfo) or @@ -102,8 +101,8 @@ def analyze_type_alias(node: Expression, elif isinstance(node, CallExpr): if (isinstance(node.callee, NameExpr) and len(node.args) == 1 and isinstance(node.args[0], NameExpr)): - call = lookup_func(node.callee.name, node.callee) - arg = lookup_func(node.args[0].name, node.args[0]) + call = api.lookup_qualified(node.callee.name, node.callee) + arg = api.lookup_qualified(node.args[0].name, node.args[0]) if (call is not None and call.node and call.node.fullname() == 'builtins.type' and arg is not None and arg.node and arg.node.fullname() == 'builtins.None'): return NoneTyp(), set() @@ -116,10 +115,9 @@ def analyze_type_alias(node: Expression, try: type = expr_to_unanalyzed_type(node) except TypeTranslationError: - fail_func('Invalid type alias', node) + api.fail('Invalid type alias', node) return None - analyzer = TypeAnalyser(lookup_func, lookup_fqn_func, tvar_scope, fail_func, note_func, - plugin, options, is_typeshed_stub, aliasing=True, + analyzer = TypeAnalyser(api, tvar_scope, plugin, options, is_typeshed_stub, aliasing=True, allow_unnormalized=allow_unnormalized, warn_bound_tvar=warn_bound_tvar) analyzer.in_dynamic_func = in_dynamic_func analyzer.global_scope = global_scope @@ -147,11 +145,8 @@ class TypeAnalyser(SyntheticTypeVisitor[Type], TypeAnalyzerPluginInterface): global_scope = True # type: bool def __init__(self, - lookup_func: Callable[[str, Context], Optional[SymbolTableNode]], - lookup_fqn_func: Callable[[str], SymbolTableNode], + api: SemanticAnalyzerInterface, tvar_scope: Optional[TypeVarScope], - fail_func: Callable[[str, Context], None], - note_func: Callable[[str, Context], None], plugin: Plugin, options: Options, is_typeshed_stub: bool, *, @@ -160,10 +155,11 @@ def __init__(self, allow_unnormalized: bool = False, third_pass: bool = False, warn_bound_tvar: bool = False) -> None: - self.lookup = lookup_func - self.lookup_fqn_func = lookup_fqn_func - self.fail_func = fail_func - self.note_func = note_func + self.api = api + self.lookup = api.lookup_qualified + self.lookup_fqn_func = api.lookup_fully_qualified + self.fail_func = api.fail + self.note_func = api.note self.tvar_scope = tvar_scope self.aliasing = aliasing self.allow_tuple_literal = allow_tuple_literal @@ -659,19 +655,17 @@ class TypeAnalyserPass3(TypeVisitor[None]): """ def __init__(self, - lookup_func: Callable[[str, Context], Optional[SymbolTableNode]], - lookup_fqn_func: Callable[[str], SymbolTableNode], - fail_func: Callable[[str, Context], None], - note_func: Callable[[str, Context], None], + api: SemanticAnalyzerInterface, plugin: Plugin, options: Options, is_typeshed_stub: bool, indicator: Dict[str, bool], patches: List[Tuple[int, Callable[[], None]]]) -> None: - self.lookup_func = lookup_func - self.lookup_fqn_func = lookup_fqn_func - self.fail = fail_func - self.note_func = note_func + self.api = api + self.lookup_func = api.lookup_qualified + self.lookup_fqn_func = api.lookup_fully_qualified + self.fail = api.fail + self.note_func = api.note self.options = options self.plugin = plugin self.is_typeshed_stub = is_typeshed_stub @@ -799,11 +793,8 @@ def visit_forwardref_type(self, t: ForwardRef) -> None: t.resolve(resolved) def anal_type(self, tp: UnboundType) -> Type: - tpan = TypeAnalyser(self.lookup_func, - self.lookup_fqn_func, + tpan = TypeAnalyser(self.api, None, - self.fail, - self.note_func, self.plugin, self.options, self.is_typeshed_stub, From de24b3c1e5d9fed448bcb0c53eb059fe77478126 Mon Sep 17 00:00:00 2001 From: Jukka Lehtosalo Date: Tue, 6 Mar 2018 15:13:44 +0000 Subject: [PATCH 05/14] Fix forward reference to typing alias import --- mypy/semanal_pass3.py | 4 ++++ mypy/semanal_shared.py | 5 +++++ mypy/typeanal.py | 12 +++++++++++- test-data/unit/check-modules.test | 25 +++++++++++-------------- 4 files changed, 31 insertions(+), 15 deletions(-) diff --git a/mypy/semanal_pass3.py b/mypy/semanal_pass3.py index 5cd3acf329e6..8aa0f7b456f6 100644 --- a/mypy/semanal_pass3.py +++ b/mypy/semanal_pass3.py @@ -445,6 +445,10 @@ def lookup_qualified(self, name: str, ctx: Context, def lookup_fully_qualified(self, fullname: str) -> Optional[SymbolTableNode]: return self.sem.lookup_fully_qualified(fullname) + def dereference_module_cross_ref( + self, node: Optional[SymbolTableNode]) -> Optional[SymbolTableNode]: + return self.sem.dereference_module_cross_ref(node) + def fail(self, msg: str, ctx: Context, serious: bool = False, *, blocker: bool = False) -> None: # TODO: Process all arguments diff --git a/mypy/semanal_shared.py b/mypy/semanal_shared.py index d541a07b2da4..244fec6ea947 100644 --- a/mypy/semanal_shared.py +++ b/mypy/semanal_shared.py @@ -36,6 +36,11 @@ def lookup_qualified(self, name: str, ctx: Context, def lookup_fully_qualified(self, name: str) -> SymbolTableNode: raise NotImplementedError + @abstractmethod + def dereference_module_cross_ref( + self, node: Optional[SymbolTableNode]) -> Optional[SymbolTableNode]: + raise NotImplementedError + @abstractmethod def fail(self, msg: str, ctx: Context, serious: bool = False, *, blocker: bool = False) -> None: diff --git a/mypy/typeanal.py b/mypy/typeanal.py index 15254e261305..29ac59a260ce 100644 --- a/mypy/typeanal.py +++ b/mypy/typeanal.py @@ -24,7 +24,7 @@ TVAR, TYPE_ALIAS, UNBOUND_IMPORTED, TypeInfo, Context, SymbolTableNode, Var, Expression, IndexExpr, RefExpr, nongen_builtins, check_arg_names, check_arg_kinds, ARG_POS, ARG_NAMED, ARG_OPT, ARG_NAMED_OPT, ARG_STAR, ARG_STAR2, TypeVarExpr, FuncDef, CallExpr, NameExpr, - Decorator, Node + Decorator, Node, type_aliases ) from mypy.tvar_scope import TypeVarScope from mypy.exprtotype import expr_to_unanalyzed_type, TypeTranslationError @@ -181,7 +181,17 @@ def visit_unbound_type(self, t: UnboundType) -> Type: # wrapping Anys: Union simplification will take care of that. return make_optional_type(self.visit_unbound_type(t)) sym = self.lookup(t.name, t, suppress_errors=self.third_pass) # type: ignore + sym = self.api.dereference_module_cross_ref(sym) if sym is not None: + if sym.fullname in type_aliases: + # Resolve forward reference to type alias like 'typing.List'. + # TODO: Unify how type aliases are handled; currently we resolve them in two + # places (the other is in the semantic analyzer pass 2). + resolved = type_aliases[sym.fullname] + new = self.api.lookup_qualified(resolved, t) + if new: + sym = new.copy() + sym.normalized = True if sym.node is None: # UNBOUND_IMPORTED can happen if an unknown name was imported. if sym.kind != UNBOUND_IMPORTED: diff --git a/test-data/unit/check-modules.test b/test-data/unit/check-modules.test index 4200e4d72b7d..f1511a1bda38 100644 --- a/test-data/unit/check-modules.test +++ b/test-data/unit/check-modules.test @@ -2003,18 +2003,15 @@ reveal_type(x) # E: Revealed type is 'builtins.str' x = str() y = int() -[case testForwardReferenceToImportedAlias] +[case testForwardReferenceToListAlias] +x: List[int] +reveal_type(x) # E: Revealed type is 'builtins.list[builtins.int]' +def f() -> 'List[int]': pass +reveal_type(f) # E: Revealed type is 'def () -> builtins.list[builtins.int]' class A: - @property - def p(self) -> C: pass - @p.setter - def p(self, c: C) -> None: pass - @p.deleter - def p(self) -> None: pass - -#reveal_type(A().x) - -from m import C -[file m.py] -C = str -[builtins fixtures/property.pyi] + y: 'List[str]' + def g(self, x: 'List[int]') -> None: pass +reveal_type(A().y) # E: Revealed type is 'builtins.list[builtins.str]' +reveal_type(A().g) # E: Revealed type is 'def (x: builtins.list[builtins.int])' +from typing import List +[builtins fixtures/list.pyi] From c1b362904f674579ecb2225d2ac1f15cb6cab53c Mon Sep 17 00:00:00 2001 From: Jukka Lehtosalo Date: Tue, 6 Mar 2018 16:09:41 +0000 Subject: [PATCH 06/14] Also handle "from m import *" --- mypy/semanal.py | 3 +++ test-data/unit/check-modules.test | 14 ++++++++++++++ 2 files changed, 17 insertions(+) diff --git a/mypy/semanal.py b/mypy/semanal.py index c319eafb55da..ecfdbc4cff47 100644 --- a/mypy/semanal.py +++ b/mypy/semanal.py @@ -1648,6 +1648,9 @@ def visit_import_all(self, i: ImportAll) -> None: m = self.modules[i_id] self.add_submodules_to_parent_modules(i_id, True) for name, node in m.names.items(): + node = self.dereference_module_cross_ref(node) + if node is None: + continue new_node = self.normalize_type_alias(node, i) # if '__all__' exists, all nodes not included have had module_public set to # False, and we can skip checking '_' because it's been explicitly included. diff --git a/test-data/unit/check-modules.test b/test-data/unit/check-modules.test index f1511a1bda38..fa0534f71b19 100644 --- a/test-data/unit/check-modules.test +++ b/test-data/unit/check-modules.test @@ -2015,3 +2015,17 @@ reveal_type(A().y) # E: Revealed type is 'builtins.list[builtins.str]' reveal_type(A().g) # E: Revealed type is 'def (x: builtins.list[builtins.int])' from typing import List [builtins fixtures/list.pyi] + +[case testIndirectStarImportWithinCycle] +import a +[file a.py] +from b import f +from c import x +[file b.py] +from c import y +from a import * +def f() -> None: pass +reveal_type(x) # E: Revealed type is 'builtins.str' +[file c.py] +x = str() +y = int() From 68736fd3c22fb966fd39ebd7fee3e9a3450310b4 Mon Sep 17 00:00:00 2001 From: Jukka Lehtosalo Date: Wed, 7 Mar 2018 11:20:02 +0000 Subject: [PATCH 07/14] Remove type forward reference handling to maintain backward compatibility The previous, more correct behavior caused too many breakages in an internal Dropbox repository. I'll enable this later after the breakages have been fixed. --- mypy/typeanal.py | 9 +++++++-- mypy/types.py | 2 ++ test-data/unit/check-incremental.test | 19 ------------------- test-data/unit/check-modules.test | 22 +++++++++++++++++++++- test-data/unit/check-serialize.test | 3 ++- 5 files changed, 32 insertions(+), 23 deletions(-) diff --git a/mypy/typeanal.py b/mypy/typeanal.py index 29ac59a260ce..00ea60efc239 100644 --- a/mypy/typeanal.py +++ b/mypy/typeanal.py @@ -24,7 +24,7 @@ TVAR, TYPE_ALIAS, UNBOUND_IMPORTED, TypeInfo, Context, SymbolTableNode, Var, Expression, IndexExpr, RefExpr, nongen_builtins, check_arg_names, check_arg_kinds, ARG_POS, ARG_NAMED, ARG_OPT, ARG_NAMED_OPT, ARG_STAR, ARG_STAR2, TypeVarExpr, FuncDef, CallExpr, NameExpr, - Decorator, Node, type_aliases + Decorator, Node, ImportedName, type_aliases ) from mypy.tvar_scope import TypeVarScope from mypy.exprtotype import expr_to_unanalyzed_type, TypeTranslationError @@ -181,8 +181,13 @@ def visit_unbound_type(self, t: UnboundType) -> Type: # wrapping Anys: Union simplification will take care of that. return make_optional_type(self.visit_unbound_type(t)) sym = self.lookup(t.name, t, suppress_errors=self.third_pass) # type: ignore - sym = self.api.dereference_module_cross_ref(sym) + # TODO: Replace "if isinstance(...)" below with this, which is more correct: + # sym = self.api.dereference_module_cross_ref(sym) if sym is not None: + if isinstance(sym.node, ImportedName): + # Forward reference to an imported name that hasn't been processed yet. + # To maintain backward compatibility, these get translated to Any. + return AnyType(TypeOfAny.implementation_artifact) if sym.fullname in type_aliases: # Resolve forward reference to type alias like 'typing.List'. # TODO: Unify how type aliases are handled; currently we resolve them in two diff --git a/mypy/types.py b/mypy/types.py index ef3b092d3a42..b4ad8e68c8f4 100644 --- a/mypy/types.py +++ b/mypy/types.py @@ -284,6 +284,8 @@ class TypeOfAny(Enum): special_form = 'special_form' # Does this Any come from interaction with another Any? from_another_any = 'from_another_any' + # Does this Any come from an implementation limitation/bug? + implementation_artifact = 'implementation_artifact' class AnyType(Type): diff --git a/test-data/unit/check-incremental.test b/test-data/unit/check-incremental.test index 720df07d6cda..03582bad4e0e 100644 --- a/test-data/unit/check-incremental.test +++ b/test-data/unit/check-incremental.test @@ -4127,22 +4127,3 @@ class Foo: x = Foo() [out] [out2] - -[case testForwardReferenceToImportedAliasInProperty] -import a - -class A: - @property - def p(self) -> C: pass - @p.setter - def p(self, c: C) -> None: pass - @p.deleter - def p(self) -> None: pass - -from m import C -[file m.py] -C = str -[file a.py] -[file a.py.2] -# dummy -[builtins fixtures/property.pyi] diff --git a/test-data/unit/check-modules.test b/test-data/unit/check-modules.test index fa0534f71b19..5338f275f978 100644 --- a/test-data/unit/check-modules.test +++ b/test-data/unit/check-modules.test @@ -2003,7 +2003,9 @@ reveal_type(x) # E: Revealed type is 'builtins.str' x = str() y = int() -[case testForwardReferenceToListAlias] +[case testForwardReferenceToListAlias-skip] +# TODO: This fails because of missing ImportedName handling in +# mypy.typeanal.TypeAnalyser.visit_unbound_type. x: List[int] reveal_type(x) # E: Revealed type is 'builtins.list[builtins.int]' def f() -> 'List[int]': pass @@ -2029,3 +2031,21 @@ reveal_type(x) # E: Revealed type is 'builtins.str' [file c.py] x = str() y = int() + +[case testIndirectFromImportWithinCycleUsedAsBaseClass-skip] +-- TODO: Fails because of missing ImportedName handling in mypy.typeanal +import a +[file a.py] +from b import f +from c import B +[file b.py] +from c import y +class A(B): pass +reveal_type(A().x) # E: Revealed type is 'builtins.str' +from a import B +def f() -> None: pass +[file c.py] +class B: + x: int +x = str() +y = int() diff --git a/test-data/unit/check-serialize.test b/test-data/unit/check-serialize.test index 7f26549af7c1..8f13eae7892d 100644 --- a/test-data/unit/check-serialize.test +++ b/test-data/unit/check-serialize.test @@ -1312,4 +1312,5 @@ from m import C C = str [builtins fixtures/property.pyi] [out2] -tmp/a.py:2: error: Revealed type is 'builtins.str' +-- TODO: Should be 'builtins.str' +tmp/a.py:2: error: Revealed type is 'Any' From d138ec8318766d35bc29c1987b74ff4e6abb72ec Mon Sep 17 00:00:00 2001 From: Jukka Lehtosalo Date: Wed, 7 Mar 2018 12:42:04 +0000 Subject: [PATCH 08/14] Add dereferencing on module attribute references like m.x --- mypy/semanal.py | 1 + test-data/unit/check-incremental.test | 23 +++++++++++++++++++++++ 2 files changed, 24 insertions(+) diff --git a/mypy/semanal.py b/mypy/semanal.py index ecfdbc4cff47..50ee878c05e3 100644 --- a/mypy/semanal.py +++ b/mypy/semanal.py @@ -3440,6 +3440,7 @@ def visit_member_expr(self, expr: MemberExpr) -> None: # else: # names = file.names n = file.names.get(expr.name, None) if file is not None else None + n = self.dereference_module_cross_ref(n) if n and not n.module_hidden: n = self.normalize_type_alias(n, expr) if not n: diff --git a/test-data/unit/check-incremental.test b/test-data/unit/check-incremental.test index 03582bad4e0e..729a098707e6 100644 --- a/test-data/unit/check-incremental.test +++ b/test-data/unit/check-incremental.test @@ -4127,3 +4127,26 @@ class Foo: x = Foo() [out] [out2] + +[case testImportReExportInCycle] +from m import One +[file m/__init__.py] +from .one import One +from .two import Two +[file m/one.py] +class One: + pass +[file m/two.py] +import m +class Two: + pass +[file m/one.py.2] +class One: + name: str +[file m/two.py.2] +import m +reveal_type(m.One.name) +class Two: + pass +[out2] +tmp/m/two.py:2: error: Revealed type is 'builtins.str' From aedd22054e98550b7ad25e1ef85fcc487cfb6935 Mon Sep 17 00:00:00 2001 From: Jukka Lehtosalo Date: Wed, 7 Mar 2018 12:55:13 +0000 Subject: [PATCH 09/14] Handle more indirect references in type annotations and add tests --- mypy/typeanal.py | 10 ++- test-data/unit/check-modules.test | 103 ++++++++++++++++++++++++++++++ 2 files changed, 111 insertions(+), 2 deletions(-) diff --git a/mypy/typeanal.py b/mypy/typeanal.py index 00ea60efc239..00d81c224007 100644 --- a/mypy/typeanal.py +++ b/mypy/typeanal.py @@ -181,12 +181,18 @@ def visit_unbound_type(self, t: UnboundType) -> Type: # wrapping Anys: Union simplification will take care of that. return make_optional_type(self.visit_unbound_type(t)) sym = self.lookup(t.name, t, suppress_errors=self.third_pass) # type: ignore - # TODO: Replace "if isinstance(...)" below with this, which is more correct: - # sym = self.api.dereference_module_cross_ref(sym) + if '.' in t.name: + # Handle indirect references to imported names. + # + # TODO: Do this for module-local references as well and remove ImportedName + # type check below. + sym = self.api.dereference_module_cross_ref(sym) if sym is not None: if isinstance(sym.node, ImportedName): # Forward reference to an imported name that hasn't been processed yet. # To maintain backward compatibility, these get translated to Any. + # + # TODO: Remove this special case. return AnyType(TypeOfAny.implementation_artifact) if sym.fullname in type_aliases: # Resolve forward reference to type alias like 'typing.List'. diff --git a/test-data/unit/check-modules.test b/test-data/unit/check-modules.test index 5338f275f978..8b2742405e38 100644 --- a/test-data/unit/check-modules.test +++ b/test-data/unit/check-modules.test @@ -2049,3 +2049,106 @@ class B: x: int x = str() y = int() + +[case testImportFromReExportInCycleUsingRelativeImport1] +from m import One +reveal_type(One) +[file m/__init__.py] +from .one import One +from .two import Two +reveal_type(One) +[file m/one.py] +class One: + pass +[file m/two.py] +from m import One +reveal_type(One) +x: One +reveal_type(x) + +class Two(One): + pass +y: Two +y = x +x = y +[out] +tmp/m/two.py:2: error: Revealed type is 'def () -> m.one.One' +tmp/m/two.py:4: error: Revealed type is 'm.one.One' +tmp/m/two.py:9: error: Incompatible types in assignment (expression has type "One", variable has type "Two") +tmp/m/__init__.py:3: error: Revealed type is 'def () -> m.one.One' +main:2: error: Revealed type is 'def () -> m.one.One' + +[case testImportReExportInCycleUsingRelativeImport2] +from m import One +reveal_type(One) +[file m/__init__.py] +from .one import One +from .two import Two +reveal_type(One) +[file m/one.py] +class One: + pass +[file m/two.py] +import m +reveal_type(m.One) +x: m.One +reveal_type(x) +class Two: + pass +[out] +tmp/m/two.py:2: error: Revealed type is 'def () -> m.one.One' +tmp/m/two.py:4: error: Revealed type is 'm.one.One' +tmp/m/__init__.py:3: error: Revealed type is 'def () -> m.one.One' +main:2: error: Revealed type is 'def () -> m.one.One' + +[case testImportReExportedNamedTupleInCycle1] +from m import One +[file m/__init__.py] +from .one import One +from .two import Two +[file m/one.py] +from typing import NamedTuple +class One(NamedTuple): + name: str +[file m/two.py] +import m +x = m.One(name="Foo") +reveal_type(x.name) +class Two: + pass +[out] +tmp/m/two.py:3: error: Revealed type is 'builtins.str' + +[case testImportReExportedNamedTupleInCycle2] +from m import One +[file m/__init__.py] +from .one import One +from .two import Two +[file m/one.py] +from typing import NamedTuple +One = NamedTuple('One', [('name', str)]) +[file m/two.py] +import m +x = m.One(name="Foo") +reveal_type(x.name) +class Two: + pass +[out] +tmp/m/two.py:3: error: Revealed type is 'builtins.str' + +[case testImportReExportedTypeAliasInCycle] +from m import One +[file m/__init__.py] +from .one import One +from .two import Two +[file m/one.py] +from typing import Union +One = Union[int, str] +[file m/two.py] +import m +x: m.One +reveal_type(x) +class Two: + pass +[out] +tmp/m/two.py:3: error: Revealed type is 'Union[builtins.int, builtins.str]' From 1625832ae4aac15b33936fa5acda462fb0b51ded Mon Sep 17 00:00:00 2001 From: Jukka Lehtosalo Date: Wed, 7 Mar 2018 13:39:02 +0000 Subject: [PATCH 10/14] Update AST strip --- mypy/semanal_pass1.py | 15 ++++++--------- mypy/semanal_shared.py | 24 +++++++++++++++++++++++- mypy/server/aststrip.py | 16 ++++++++++++---- 3 files changed, 41 insertions(+), 14 deletions(-) diff --git a/mypy/semanal_pass1.py b/mypy/semanal_pass1.py index 55c1d4057eb1..1ed5e5494a6b 100644 --- a/mypy/semanal_pass1.py +++ b/mypy/semanal_pass1.py @@ -28,6 +28,7 @@ ) from mypy.types import Type, UnboundType, UnionType, AnyType, TypeOfAny, NoneTyp from mypy.semanal import SemanticAnalyzerPass2, infer_reachability_of_if_statement +from mypy.semanal_shared import create_indirect_imported_name from mypy.options import Options from mypy.sametypes import is_same_type from mypy.visitor import NodeVisitor @@ -244,15 +245,11 @@ def visit_import_from(self, node: ImportFrom) -> None: for name, as_name in node.names: imported_name = as_name or name if imported_name not in self.sem.globals: - target_module, ok = correct_relative_import( - self.sem.cur_mod_id, - node.relative, - node.id, - self.sem.cur_mod_node.is_package_init_file()) - if ok: - target_name = '%s.%s' % (target_module, imported_name) - link_node = ImportedName(target_name) - sym = SymbolTableNode(GDEF, link_node) + sym = create_indirect_imported_name(self.sem.cur_mod_node, + node.id, + node.relative, + name) + if sym: self.add_symbol(imported_name, sym, context=node) def visit_import(self, node: Import) -> None: diff --git a/mypy/semanal_shared.py b/mypy/semanal_shared.py index 244fec6ea947..09d9132de886 100644 --- a/mypy/semanal_shared.py +++ b/mypy/semanal_shared.py @@ -3,7 +3,8 @@ from abc import abstractmethod from typing import Optional -from mypy.nodes import Context, SymbolTableNode +from mypy.nodes import Context, SymbolTableNode, MypyFile, ImportedName, GDEF +from mypy.util import correct_relative_import # Priorities for ordering of patches within the final "patch" phase of semantic analysis @@ -49,3 +50,24 @@ def fail(self, msg: str, ctx: Context, serious: bool = False, *, @abstractmethod def note(self, msg: str, ctx: Context) -> None: raise NotImplementedError + + +def create_indirect_imported_name(file_node: MypyFile, + module: str, + relative: int, + imported_name: str) -> Optional[SymbolTableNode]: + """Create symbol table entry for a name imported from another module. + + These entries act as indirect references. + """ + target_module, ok = correct_relative_import( + file_node.fullname(), + relative, + module, + file_node.is_package_init_file()) + if not ok: + return None + target_name = '%s.%s' % (target_module, imported_name) + link = ImportedName(target_name) + # Use GDEF since this refers to a module-level definition. + return SymbolTableNode(GDEF, link) diff --git a/mypy/server/aststrip.py b/mypy/server/aststrip.py index c949d72cfd2b..ee51a3674cbf 100644 --- a/mypy/server/aststrip.py +++ b/mypy/server/aststrip.py @@ -43,8 +43,9 @@ from mypy.nodes import ( Node, FuncDef, NameExpr, MemberExpr, RefExpr, MypyFile, FuncItem, ClassDef, AssignmentStmt, ImportFrom, Import, TypeInfo, SymbolTable, Var, CallExpr, Decorator, OverloadedFuncDef, - SuperExpr, UNBOUND_IMPORTED, GDEF, MDEF, IndexExpr + SuperExpr, UNBOUND_IMPORTED, GDEF, MDEF, IndexExpr, SymbolTableNode ) +from mypy.semanal_shared import create_indirect_imported_name from mypy.traverser import TraverserVisitor @@ -65,6 +66,7 @@ class NodeStripVisitor(TraverserVisitor): def __init__(self) -> None: self.type = None # type: Optional[TypeInfo] self.names = None # type: Optional[SymbolTable] + self.file_node = None # type: Optional[MypyFile] self.is_class_body = False # By default, process function definitions. If False, don't -- this is used for # processing module top levels. @@ -73,6 +75,7 @@ def __init__(self) -> None: def strip_file_top_level(self, file_node: MypyFile) -> None: """Strip a module top-level (don't recursive into functions).""" self.names = file_node.names + self.file_node = file_node self.recurse_into_functions = False file_node.accept(self) @@ -165,9 +168,14 @@ def visit_import_from(self, node: ImportFrom) -> None: # assigns to an existing name instead of defining a new one. for name, as_name in node.names: imported_name = as_name or name - symnode = self.names[imported_name] - symnode.kind = UNBOUND_IMPORTED - symnode.node = None + # This assert is safe since we check for self.names above. + assert self.file_node is not None + sym = create_indirect_imported_name(self.file_node, + node.id, + node.relative, + name) + if sym: + self.names[imported_name] = sym def visit_import(self, node: Import) -> None: if node.assignments: From ef7681f199d02fd7d29a528ace61bd6fc532b363 Mon Sep 17 00:00:00 2001 From: Jukka Lehtosalo Date: Wed, 7 Mar 2018 13:49:44 +0000 Subject: [PATCH 11/14] Fix type check --- mypy/semanal.py | 4 ++-- mypy/semanal_pass3.py | 2 +- mypy/typeanal.py | 2 +- 3 files changed, 4 insertions(+), 4 deletions(-) diff --git a/mypy/semanal.py b/mypy/semanal.py index 50ee878c05e3..b10558633dbc 100644 --- a/mypy/semanal.py +++ b/mypy/semanal.py @@ -1647,8 +1647,8 @@ def visit_import_all(self, i: ImportAll) -> None: if i_id in self.modules: m = self.modules[i_id] self.add_submodules_to_parent_modules(i_id, True) - for name, node in m.names.items(): - node = self.dereference_module_cross_ref(node) + for name, orig_node in m.names.items(): + node = self.dereference_module_cross_ref(orig_node) if node is None: continue new_node = self.normalize_type_alias(node, i) diff --git a/mypy/semanal_pass3.py b/mypy/semanal_pass3.py index 8aa0f7b456f6..cb3948a1788f 100644 --- a/mypy/semanal_pass3.py +++ b/mypy/semanal_pass3.py @@ -442,7 +442,7 @@ def lookup_qualified(self, name: str, ctx: Context, suppress_errors: bool = False) -> Optional[SymbolTableNode]: return self.sem.lookup_qualified(name, ctx, suppress_errors=suppress_errors) - def lookup_fully_qualified(self, fullname: str) -> Optional[SymbolTableNode]: + def lookup_fully_qualified(self, fullname: str) -> SymbolTableNode: return self.sem.lookup_fully_qualified(fullname) def dereference_module_cross_ref( diff --git a/mypy/typeanal.py b/mypy/typeanal.py index 00d81c224007..6bafaae0fb11 100644 --- a/mypy/typeanal.py +++ b/mypy/typeanal.py @@ -180,7 +180,7 @@ def visit_unbound_type(self, t: UnboundType) -> Type: # We don't need to worry about double-wrapping Optionals or # wrapping Anys: Union simplification will take care of that. return make_optional_type(self.visit_unbound_type(t)) - sym = self.lookup(t.name, t, suppress_errors=self.third_pass) # type: ignore + sym = self.lookup(t.name, t, suppress_errors=self.third_pass) if '.' in t.name: # Handle indirect references to imported names. # From 80917241d8351286ef1c9080b199a45bf981ef69 Mon Sep 17 00:00:00 2001 From: Jukka Lehtosalo Date: Wed, 7 Mar 2018 15:51:26 +0000 Subject: [PATCH 12/14] Fix str() of ImportedName --- mypy/nodes.py | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/mypy/nodes.py b/mypy/nodes.py index 30dff4502b65..ab9f80008ce3 100644 --- a/mypy/nodes.py +++ b/mypy/nodes.py @@ -333,7 +333,7 @@ class ImportedName(SymbolNode): analysis pass 2, these references should be replaced with direct reference to a real AST node. - Note that this is neither a Statement nor an Expressions so this + Note that this is neither a Statement nor an Expression so this can't be visited. """ @@ -353,6 +353,9 @@ def serialize(self) -> JsonDict: def deserialize(cls, data: JsonDict) -> 'ImportedName': assert False, "ImportedName should never be serialized" + def __str__(self) -> str: + return 'ImportedName(%s)' % self.target_fullname + class FuncBase(Node): """Abstract base class for function-like nodes""" From 85129b110fe4b5e6eb10e07f49638deaa6e1111c Mon Sep 17 00:00:00 2001 From: Jukka Lehtosalo Date: Wed, 7 Mar 2018 15:51:59 +0000 Subject: [PATCH 13/14] Also fix #4429 --- mypy/semanal.py | 3 +++ test-data/unit/check-modules.test | 19 +++++++++++++++++++ 2 files changed, 22 insertions(+) diff --git a/mypy/semanal.py b/mypy/semanal.py index b10558633dbc..a38c816fc2cf 100644 --- a/mypy/semanal.py +++ b/mypy/semanal.py @@ -1568,6 +1568,9 @@ def dereference_module_cross_ref( # nothing). while node and isinstance(node.node, ImportedName): fullname = node.node.fullname() + if fullname in self.modules: + # This is a module reference. + return SymbolTableNode(MODULE_REF, self.modules[fullname]) if fullname in seen: # Looks like a reference cycle. Just break it. # TODO: Generate a more specific error message. diff --git a/test-data/unit/check-modules.test b/test-data/unit/check-modules.test index 8b2742405e38..b4a121111b02 100644 --- a/test-data/unit/check-modules.test +++ b/test-data/unit/check-modules.test @@ -2152,3 +2152,22 @@ class Two: pass [out] tmp/m/two.py:3: error: Revealed type is 'Union[builtins.int, builtins.str]' + +[case testImportCycleSpecialCase] +import p +[file p/__init__.py] +from . import a +from . import b +reveal_type(a.foo()) +[file p/a.py] +import p +def foo() -> int: pass +[file p/b.py] +import p + +def run() -> None: + reveal_type(p.a.foo()) +[builtins fixtures/module.pyi] +[out] +tmp/p/b.py:4: error: Revealed type is 'builtins.int' +tmp/p/__init__.py:3: error: Revealed type is 'builtins.int' From 842b1c74325b9c61ec43a47806035034aefaddc9 Mon Sep 17 00:00:00 2001 From: Jukka Lehtosalo Date: Thu, 8 Mar 2018 10:36:55 +0000 Subject: [PATCH 14/14] Update based on feedback --- mypy/semanal.py | 2 +- mypy/semanal_pass3.py | 3 +-- test-data/unit/check-modules.test | 32 +++++++++++++++++++++++++++++-- 3 files changed, 32 insertions(+), 5 deletions(-) diff --git a/mypy/semanal.py b/mypy/semanal.py index a38c816fc2cf..3d849f085ed6 100644 --- a/mypy/semanal.py +++ b/mypy/semanal.py @@ -1603,7 +1603,7 @@ def process_import_over_existing_name(self, def normalize_type_alias(self, node: SymbolTableNode, ctx: Context) -> Optional[SymbolTableNode]: - """If node refers to a built-intype alias, normalize it. + """If node refers to a built-in type alias, normalize it. An example normalization is 'typing.List' -> '__builtins__.list'. diff --git a/mypy/semanal_pass3.py b/mypy/semanal_pass3.py index cb3948a1788f..7a8a881852e5 100644 --- a/mypy/semanal_pass3.py +++ b/mypy/semanal_pass3.py @@ -451,8 +451,7 @@ def dereference_module_cross_ref( def fail(self, msg: str, ctx: Context, serious: bool = False, *, blocker: bool = False) -> None: - # TODO: Process all arguments - self.errors.report(ctx.get_line(), ctx.get_column(), msg) + self.sem.fail(msg, ctx, serious, blocker=blocker) def fail_blocker(self, msg: str, ctx: Context) -> None: self.fail(msg, ctx, blocker=True) diff --git a/test-data/unit/check-modules.test b/test-data/unit/check-modules.test index b4a121111b02..100556d5583b 100644 --- a/test-data/unit/check-modules.test +++ b/test-data/unit/check-modules.test @@ -1958,7 +1958,7 @@ tmp/b.py:1: error: Cannot find module named 'c' main:1: error: Cannot find module named 'c' main:1: note: (Perhaps setting MYPYPATH or using the "--ignore-missing-imports" flag would help) -[case testIndirectFromImportWithinCycle] +[case testIndirectFromImportWithinCycle1] import a [file a.py] from b import f @@ -1972,6 +1972,20 @@ reveal_type(x) # E: Revealed type is 'builtins.str' x = str() y = int() +[case testIndirectFromImportWithinCycle2] +import a +[file a.py] +from c import y +from b import x +def f() -> None: pass +reveal_type(x) # E: Revealed type is 'builtins.str' +[file b.py] +from a import f +from c import x +[file c.py] +x = str() +y = int() + [case testIndirectFromImportWithinCycleInPackage] import p.a [file p/__init__.py] @@ -2018,7 +2032,7 @@ reveal_type(A().g) # E: Revealed type is 'def (x: builtins.list[builtins.int])' from typing import List [builtins fixtures/list.pyi] -[case testIndirectStarImportWithinCycle] +[case testIndirectStarImportWithinCycle1] import a [file a.py] from b import f @@ -2032,6 +2046,20 @@ reveal_type(x) # E: Revealed type is 'builtins.str' x = str() y = int() +[case testIndirectStarImportWithinCycle2] +import a +[file a.py] +from c import y +from b import * +def f() -> None: pass +reveal_type(x) # E: Revealed type is 'builtins.str' +[file b.py] +from a import f +from c import x +[file c.py] +x = str() +y = int() + [case testIndirectFromImportWithinCycleUsedAsBaseClass-skip] -- TODO: Fails because of missing ImportedName handling in mypy.typeanal import a