From b6fd5d9adcf1bbecd1c38ede883e17e1a5c6e069 Mon Sep 17 00:00:00 2001 From: Ivan Levkivskyi Date: Fri, 15 Feb 2019 21:52:26 +0000 Subject: [PATCH 01/23] Work towards better assignment analysis --- mypy/newsemanal/semanal.py | 173 ++++++++++++++++++------- mypy/newsemanal/typeanal.py | 51 +------- test-data/unit/check-newsemanal.test | 43 +++++- test-data/unit/check-type-aliases.test | 5 +- 4 files changed, 167 insertions(+), 105 deletions(-) diff --git a/mypy/newsemanal/semanal.py b/mypy/newsemanal/semanal.py index b5224bbb81a9d..6332e57755882 100644 --- a/mypy/newsemanal/semanal.py +++ b/mypy/newsemanal/semanal.py @@ -89,7 +89,7 @@ from mypy.newsemanal.typeanal import ( TypeAnalyser, analyze_type_alias, no_subscript_builtin_alias, TypeVariableQuery, TypeVarList, remove_dups, has_any_from_unimported_type, - check_for_explicit_any + check_for_explicit_any, type_constructors ) from mypy.exprtotype import expr_to_unanalyzed_type, TypeTranslationError from mypy.options import Options @@ -179,6 +179,9 @@ Tag = int +MAX_WAIT = 5 + + class NewSemanticAnalyzer(NodeVisitor[None], SemanticAnalyzerInterface, SemanticAnalyzerPluginInterface): @@ -272,6 +275,7 @@ def __init__(self, # for processing module top levels in fine-grained incremental mode. self.recurse_into_functions = True self.scope = Scope() + self.wait_list = {} # type: Dict[str, Dict[str,int]] # mypyc doesn't properly handle implementing an abstractproperty # with a regular attribute so we make them properties @@ -536,6 +540,8 @@ def _visit_func_def(self, defn: FuncDef) -> None: # class-level imported names and type variables are in scope. analyzer = self.type_analyzer() defn.type = analyzer.visit_callable_type(defn.type, nested=False) + if has_placeholder(defn.type): + self.defer() self.add_type_alias_deps(analyzer.aliases_used) self.check_function_signature(defn) if isinstance(defn, FuncDef): @@ -1882,14 +1888,36 @@ def add_type_alias_deps(self, aliases_used: Iterable[str], target = self.scope.current_target() self.cur_mod_node.alias_deps[target].update(aliases_used) - def is_not_ready_type_ref(self, rv: Expression) -> bool: - """Does this expression refers to a not-ready class? + def is_none_alias(self, node: Expression) -> bool: + if isinstance(node, CallExpr): + if (isinstance(node.callee, NameExpr) and len(node.args) == 1 and + isinstance(node.args[0], NameExpr)): + call = self.lookup_qualified(node.callee.name, node.callee) + arg = self.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 True + return False + + def is_type_ref(self, rv: Expression, bare: bool = False) -> bool: + """Does this expression refers to a type?""" + if not isinstance(rv, RefExpr): + return False + if isinstance(rv.node, TypeVarExpr): + self.fail('Type variable "{}" is invalid as target for type alias'.format( + rv.fullname), rv) + return False + + valid_refs = {'typing.Any', 'typing.Tuple', 'typing.Callable'} if bare else type_constructors + + if isinstance(rv.node, TypeAlias) or rv.fullname in valid_refs: + return True + + if isinstance(rv.node, TypeInfo): + if bare: + return True + return not rv.node.is_enum - This includes 'Ref' and 'Ref[Arg1, Arg2, ...]', where 'Ref' - refers to a PlaceholderNode with becomes_typeinfo=True. - """ - if isinstance(rv, IndexExpr) and isinstance(rv.base, RefExpr): - return self.is_not_ready_type_ref(rv.base) if isinstance(rv, NameExpr): n = self.lookup(rv.name, rv) if n and isinstance(n.node, PlaceholderNode) and n.node.becomes_typeinfo: @@ -1902,38 +1930,64 @@ def is_not_ready_type_ref(self, rv: Expression) -> bool: return True return False + def should_wait(self, rv: Expression) -> bool: + wait_list = self.wait_list.setdefault(self.cur_mod_id, {}) + if isinstance(rv, NameExpr): + n = self.lookup(rv.name, rv) + if n and isinstance(n.node, PlaceholderNode) and not n.node.becomes_typeinfo: + if rv.name in wait_list: + if wait_list[rv.name] == MAX_WAIT: + return False + wait_list[rv.name] += 1 + else: + wait_list[rv.name] = 0 + return True + elif isinstance(rv, MemberExpr): + fname = get_member_expr_fullname(rv) + if fname: + n = self.lookup_qualified(fname, rv) + if n and isinstance(n.node, PlaceholderNode) and not n.node.becomes_typeinfo: + if fname in wait_list: + if wait_list[fname] == MAX_WAIT: + return False + wait_list[fname] += 1 + else: + wait_list[fname] = 0 + return True + elif isinstance(rv, IndexExpr) and isinstance(rv.base, RefExpr): + return self.should_wait(rv.base) + return False + + def can_be_type_alias(self, rv: Expression) -> bool: + if isinstance(rv, RefExpr) and self.is_type_ref(rv, bare=True): + return True + if isinstance(rv, IndexExpr) and self.is_type_ref(rv.base, bare=False): + return True + if self.is_none_alias(rv): + return True + return False + def visit_assignment_stmt(self, s: AssignmentStmt) -> None: s.is_final_def = self.unwrap_final(s) tag = self.track_incomplete_refs() s.rvalue.accept(self) - if isinstance(s.rvalue, IndexExpr) and isinstance(s.rvalue.base, RefExpr): - # Special case: analyze index expression _as a type_ to trigger - # incomplete refs for string forward references, for example - # Union['ClassA', 'ClassB']. - # We throw away the results of the analysis and we only care about - # the detection of incomplete references (this doesn't change the expression - # in place). - self.analyze_alias(s.rvalue, allow_placeholder=True) - top_level_not_ready = self.is_not_ready_type_ref(s.rvalue) - # NOTE: the first check is insufficient. We want to defer creation of a Var. - if self.found_incomplete_ref(tag) or top_level_not_ready: + if self.found_incomplete_ref(tag) or self.should_wait(s.rvalue): # Initializer couldn't be fully analyzed. Defer the current node and give up. # Make sure that if we skip the definition of some local names, they can't be # added later in this scope, since an earlier definition should take precedence. for expr in names_modified_by_assignment(s): - # NOTE: Currently for aliases like 'X = List[Y]', where 'Y' is not ready - # we proceed forward and create a Var. The latter will be replaced with - # a type alias it r.h.s. is a valid alias. - self.mark_incomplete(expr.name, expr, becomes_typeinfo=top_level_not_ready) + self.mark_incomplete(expr.name, expr) return if self.analyze_namedtuple_assign(s): return + if self.check_and_set_up_type_alias(s): + s.is_alias_def = True + return self.analyze_lvalues(s) self.check_final_implicit_def(s) self.check_classvar(s) self.process_type_annotation(s) self.apply_dynamic_class_hook(s) - self.check_and_set_up_type_alias(s) self.newtype_analyzer.process_newtype_declaration(s) self.process_typevar_declaration(s) self.typed_dict_analyzer.process_typeddict_definition(s, self.is_func_scope()) @@ -2101,6 +2155,9 @@ def process_type_annotation(self, s: AssignmentStmt) -> None: analyzed = self.anal_type(s.type, allow_tuple_literal=allow_tuple_literal) if analyzed is None: return + if has_placeholder(analyzed): + self.defer() + return s.type = analyzed if (self.type and self.type.is_protocol and isinstance(lvalue, NameExpr) and isinstance(s.rvalue, TempNode) and s.rvalue.no_rhs): @@ -2196,7 +2253,7 @@ def analyze_alias(self, rvalue: Expression, qualified_tvars = [] return typ, alias_tvars, depends_on, qualified_tvars - def check_and_set_up_type_alias(self, s: AssignmentStmt) -> None: + def check_and_set_up_type_alias(self, s: AssignmentStmt) -> bool: """Check if assignment creates a type alias and set it up as needed. For simple aliases like L = List we use a simpler mechanism, just copying TypeInfo. @@ -2206,10 +2263,10 @@ def check_and_set_up_type_alias(self, s: AssignmentStmt) -> None: lvalue = s.lvalues[0] if len(s.lvalues) > 1 or not isinstance(lvalue, NameExpr): # First rule: Only simple assignments like Alias = ... create aliases. - return - if s.type: + return False + if s.unanalyzed_type is not None: # Second rule: Explicit type (cls: Type[A] = A) always creates variable, not alias. - return + return False non_global_scope = self.type or self.is_func_scope() if isinstance(s.rvalue, RefExpr) and non_global_scope and lvalue.is_inferred_def: # Third rule: Non-subscripted right hand side creates a variable @@ -2222,20 +2279,21 @@ def check_and_set_up_type_alias(self, s: AssignmentStmt) -> None: # # without this rule, this typical use case will require a lot of explicit # annotations (see the second rule). - return + return False rvalue = s.rvalue - tag = self.track_incomplete_refs() - res, alias_tvars, depends_on, qualified_tvars = self.analyze_alias(rvalue) - if not res or self.found_incomplete_ref(tag): - return - if (isinstance(res, Instance) and res.type.name() == lvalue.name and - res.type.module_name == self.cur_mod_id): - # Aliases like C = C is a no-op. - return - s.is_alias_def = True - node = self.lookup(lvalue.name, lvalue) - assert node is not None - assert node.node is not None + if not self.can_be_type_alias(rvalue): + return False + if self.is_none_alias(rvalue): + res = NoneTyp() + alias_tvars, depends_on, qualified_tvars = [], set(), [] + else: + tag = self.track_incomplete_refs() + res, alias_tvars, depends_on, qualified_tvars = self.analyze_alias(rvalue, allow_placeholder=True) + if not res: + return False + if self.found_incomplete_ref(tag): + self.add_symbol(lvalue.name, PlaceholderNode(self.qualified_name(lvalue.name), rvalue, True), None) + return True self.add_type_alias_deps(depends_on) # In addition to the aliases used, we add deps on unbound # type variables, since they are erased from target type. @@ -2243,13 +2301,15 @@ def check_and_set_up_type_alias(self, s: AssignmentStmt) -> None: # The above are only direct deps on other aliases. # For subscripted aliases, type deps from expansion are added in deps.py # (because the type is stored) - if not lvalue.is_inferred_def: - # Type aliases can't be re-defined. - if isinstance(node.node, (TypeAlias, TypeInfo)): + existing = self.current_symbol_table().get(lvalue.name) + # Type aliases can't be re-defined. + if existing and (isinstance(existing.node, Var) or + isinstance(existing.node, TypeAlias) and not s.is_alias_def): + if isinstance(existing.node, TypeAlias) and not s.is_alias_def: self.fail('Cannot assign multiple types to name "{}"' ' without an explicit "Type[...]" annotation' .format(lvalue.name), lvalue) - return + return False check_for_explicit_any(res, self.options, self.is_typeshed_stub_file, self.msg, context=s) # when this type alias gets "inlined", the Any is not explicit anymore, @@ -2263,10 +2323,15 @@ def check_and_set_up_type_alias(self, s: AssignmentStmt) -> None: s.rvalue.analyzed.column = res.column elif isinstance(s.rvalue, RefExpr): s.rvalue.is_alias_rvalue = True - node.node = TypeAlias(res, self.qualified_name(lvalue.name), s.line, s.column, - alias_tvars=alias_tvars, no_args=no_args) + alias_node = TypeAlias(res, self.qualified_name(lvalue.name), s.line, s.column, + alias_tvars=alias_tvars, no_args=no_args) + if existing: + existing.node = alias_node + else: + self.add_symbol(lvalue.name, alias_node, s) if isinstance(rvalue, RefExpr) and isinstance(rvalue.node, TypeAlias): - node.node.normalized = rvalue.node.normalized + alias_node.normalized = rvalue.node.normalized + return True def analyze_lvalue(self, lval: Lvalue, nested: bool = False, explicit_type: bool = False, @@ -4152,6 +4217,18 @@ def schedule_patch(self, priority: int, patch: Callable[[], None]) -> None: self.patches.append((priority, patch)) +class HasPlaceholders(TypeQuery[bool]): + def __init__(self) -> None: + super().__init__(any) + + def visit_placeholder_type(self, t: PlaceholderType) -> bool: + return True + + +def has_placeholder(typ) -> bool: + return typ.accept(HasPlaceholders()) + + def replace_implicit_first_type(sig: FunctionLike, new: Type) -> FunctionLike: if isinstance(sig, CallableType): if len(sig.arg_types) == 0: diff --git a/mypy/newsemanal/typeanal.py b/mypy/newsemanal/typeanal.py index 00b11e6b13add..10ebbd8f52277 100644 --- a/mypy/newsemanal/typeanal.py +++ b/mypy/newsemanal/typeanal.py @@ -74,48 +74,6 @@ def analyze_type_alias(node: Expression, full names of type aliases it depends on (directly or indirectly). Return None otherwise. 'node' must have been semantically analyzed. """ - # Quickly return None if the expression doesn't look like a type. Note - # that we don't support straight string literals as type aliases - # (only string literals within index expressions). - if isinstance(node, RefExpr): - # Note that this misses the case where someone tried to use a - # class-referenced type variable as a type alias. It's easier to catch - # that one in checkmember.py - if isinstance(node.node, TypeVarExpr): - api.fail('Type variable "{}" is invalid as target for type alias'.format( - node.fullname), node) - return None - if not (isinstance(node.node, TypeInfo) or - node.fullname in ('typing.Any', 'typing.Tuple', 'typing.Callable') or - isinstance(node.node, TypeAlias)): - return None - elif isinstance(node, IndexExpr): - base = node.base - if isinstance(base, RefExpr): - if not (isinstance(base.node, TypeInfo) or - base.fullname in type_constructors or - isinstance(base.node, TypeAlias)): - return None - # Enums can't be generic, and without this check we may incorrectly interpret indexing - # an Enum class as creating a type alias. - if isinstance(base.node, TypeInfo) and base.node.is_enum: - return None - else: - return None - elif isinstance(node, CallExpr): - if (isinstance(node.callee, NameExpr) and len(node.args) == 1 and - isinstance(node.args[0], NameExpr)): - 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() - return None - return None - else: - return None - - # It's a type alias (though it may be an invalid one). try: type = expr_to_unanalyzed_type(node) except TypeTranslationError: @@ -408,14 +366,7 @@ def analyze_unbound_type_without_type_info(self, t: UnboundType, sym: SymbolTabl (not self.tvar_scope or self.tvar_scope.get_binding(sym) is None)) if self.allow_unbound_tvars and unbound_tvar and not self.third_pass: return t - # None of the above options worked, we give up. - # NOTE: 'final_iteration' is iteration when we hit the maximum number of iterations limit. - if self.api.final_iteration: - # TODO: This is problematic, since we will have to wait until the maximum number - # of iterations to report an invalid type. - self.fail('Invalid type "{}"'.format(name), t) - else: - self.api.defer() + self.fail('Invalid type "{}"'.format(name), t) if self.third_pass and isinstance(sym.node, TypeVarExpr): self.note_func("Forward references to type variables are prohibited", t) return AnyType(TypeOfAny.from_error) diff --git a/test-data/unit/check-newsemanal.test b/test-data/unit/check-newsemanal.test index 0a4f8fbc521ee..43600de930ed6 100644 --- a/test-data/unit/check-newsemanal.test +++ b/test-data/unit/check-newsemanal.test @@ -1055,7 +1055,7 @@ class Test: def __init__(self) -> None: some_module = self.a -[case testNewAnalyzerAliasToNotReadyClass] +[case testNewAnalyzerAliasToNotReadyClass1] import a [file a.py] from b import B @@ -1188,7 +1188,7 @@ class C: y: int [builtins fixtures/list.pyi] -[case testNewAnalyzerAliasToNotReadyTwoDeferrals-skip] +[case testNewAnalyzerAliasToNotReadyTwoDeferrals] from typing import List x: B @@ -1196,10 +1196,11 @@ B = List[C] A = C class C(List[A]): pass -reveal_type(x) +reveal_type(x) # E: Revealed type is 'builtins.list[__main__.C]' +reveal_type(x[0][0]) # E: Revealed type is '__main__.C*' [builtins fixtures/list.pyi] -[case testNewAnalyzerAliasToNotReadyDirectBase-skip] +[case testNewAnalyzerAliasToNotReadyDirectBase] from typing import List x: B @@ -1207,7 +1208,39 @@ B = List[C] class C(B): pass reveal_type(x) # E: Revealed type is 'builtins.list[__main__.C]' -reveal_type(x[0][0]) # E: Revealed type is '__main__.C' +reveal_type(x[0][0]) # E: Revealed type is '__main__.C*' +[builtins fixtures/list.pyi] + +[case testNewAnalyzerAliasToNotReadyTwoDeferralsFunction] +import a +[file a.py] +from typing import List +from b import D + +def f(x: B) -> List[B]: ... +B = List[C] +A = C +class C(List[A]): pass +[file b.py] +from a import f +class D: ... +reveal_type(f) # E: Revealed type is 'def (x: builtins.list[a.C]) -> builtins.list[builtins.list[a.C]]' +[builtins fixtures/list.pyi] + +[case testNewAnalyzerAliasToNotReadyDirectBaseFunction] +import a +[file a.py] +from typing import List +from b import D + +def f(x: B) -> List[B]: ... +B = List[C] +class C(B): pass + +[file b.py] +from a import f +class D: ... +reveal_type(f) # E: Revealed type is 'def (x: builtins.list[a.C]) -> builtins.list[builtins.list[a.C]]' [builtins fixtures/list.pyi] [case testNewAnalyzerAliasToNotReadyMixed] diff --git a/test-data/unit/check-type-aliases.test b/test-data/unit/check-type-aliases.test index bada7486bd6e8..27fa6e198c56f 100644 --- a/test-data/unit/check-type-aliases.test +++ b/test-data/unit/check-type-aliases.test @@ -336,6 +336,7 @@ reveal_type(w) # E: Revealed type is '__main__.C[builtins.int]' [out] [case testTypeAliasesToNamedTuple] +# flags: --new-semantic-analyzer from nt import C, D, E A1 = C @@ -358,11 +359,11 @@ reveal_type(a2) # E: Revealed type is 'Tuple[builtins.str, fallback=nt.D]' a3 = A3() reveal_type(a3) # E: Revealed type is 'Tuple[builtins.int, builtins.str, fallback=nt.E]' -Cls.A1('no') # E: Argument 1 has incompatible type "str"; expected "int" +Cls.A1('no') # E: Argument 1 to "C" has incompatible type "str"; expected "int" ca1 = Cls.A1(1) reveal_type(ca1) # E: Revealed type is 'Tuple[builtins.int, fallback=nt.C]' -Cls.A2(0) # E: Argument 1 has incompatible type "int"; expected "str" +Cls.A2(0) # E: Argument 1 to "D" has incompatible type "int"; expected "str" ca2 = Cls.A2('yes') reveal_type(ca2) # E: Revealed type is 'Tuple[builtins.str, fallback=nt.D]' From 6a80f22864001139c62c74212b842af667eccdfe Mon Sep 17 00:00:00 2001 From: Ivan Levkivskyi Date: Wed, 6 Mar 2019 11:49:36 +0000 Subject: [PATCH 02/23] Minor fixes after merge --- mypy/newsemanal/semanal.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/mypy/newsemanal/semanal.py b/mypy/newsemanal/semanal.py index 08bb5339822f4..0a651c6284f0a 100644 --- a/mypy/newsemanal/semanal.py +++ b/mypy/newsemanal/semanal.py @@ -84,6 +84,7 @@ CallableType, Overloaded, Instance, Type, AnyType, LiteralType, LiteralValue, TypeTranslator, TypeOfAny, TypeType, NoneTyp, PlaceholderType ) +from mypy.type_visitor import TypeQuery from mypy.nodes import implicit_module_attrs from mypy.newsemanal.typeanal import ( TypeAnalyser, analyze_type_alias, no_subscript_builtin_alias, @@ -1942,7 +1943,7 @@ def should_wait(self, rv: Expression) -> bool: elif isinstance(rv, MemberExpr): fname = get_member_expr_fullname(rv) if fname: - n = self.lookup_qualified(fname, rv) + n = self.lookup_qualified(fname, rv, suppress_errors=True) if n and isinstance(n.node, PlaceholderNode) and not n.node.becomes_typeinfo: if fname in wait_list: if wait_list[fname] == MAX_WAIT: From 88dec62c2cbb184e2a91ef4693fa3fba60d5a358 Mon Sep 17 00:00:00 2001 From: Ivan Levkivskyi Date: Thu, 7 Mar 2019 17:53:31 +0000 Subject: [PATCH 03/23] Update logic and fix tests --- mypy/newsemanal/semanal.py | 66 +++++++++++++++----------- test-data/unit/check-newsemanal.test | 4 +- test-data/unit/check-type-aliases.test | 4 +- test-data/unit/fine-grained.test | 4 +- 4 files changed, 43 insertions(+), 35 deletions(-) diff --git a/mypy/newsemanal/semanal.py b/mypy/newsemanal/semanal.py index 0a651c6284f0a..e90892d15123b 100644 --- a/mypy/newsemanal/semanal.py +++ b/mypy/newsemanal/semanal.py @@ -181,9 +181,6 @@ Tag = int -MAX_WAIT = 5 - - class NewSemanticAnalyzer(NodeVisitor[None], SemanticAnalyzerInterface, SemanticAnalyzerPluginInterface): @@ -283,7 +280,6 @@ def __init__(self, # for processing module top levels in fine-grained incremental mode. self.recurse_into_functions = True self.scope = Scope() - self.wait_list = {} # type: Dict[str, Dict[str,int]] # mypyc doesn't properly handle implementing an abstractproperty # with a regular attribute so we make them properties @@ -1929,28 +1925,18 @@ def is_type_ref(self, rv: Expression, bare: bool = False) -> bool: return False def should_wait(self, rv: Expression) -> bool: - wait_list = self.wait_list.setdefault(self.cur_mod_id, {}) + if self.final_iteration: + # No chance, nothing has changed. + return False if isinstance(rv, NameExpr): n = self.lookup(rv.name, rv) if n and isinstance(n.node, PlaceholderNode) and not n.node.becomes_typeinfo: - if rv.name in wait_list: - if wait_list[rv.name] == MAX_WAIT: - return False - wait_list[rv.name] += 1 - else: - wait_list[rv.name] = 0 return True elif isinstance(rv, MemberExpr): fname = get_member_expr_fullname(rv) if fname: n = self.lookup_qualified(fname, rv, suppress_errors=True) if n and isinstance(n.node, PlaceholderNode) and not n.node.becomes_typeinfo: - if fname in wait_list: - if wait_list[fname] == MAX_WAIT: - return False - wait_list[fname] += 1 - else: - wait_list[fname] = 0 return True elif isinstance(rv, IndexExpr) and isinstance(rv.base, RefExpr): return self.should_wait(rv.base) @@ -2301,8 +2287,19 @@ def check_and_set_up_type_alias(self, s: AssignmentStmt) -> bool: if s.unanalyzed_type is not None: # Second rule: Explicit type (cls: Type[A] = A) always creates variable, not alias. return False + + existing = self.current_symbol_table().get(lvalue.name) + # Type aliases can't be re-defined. + if existing and (isinstance(existing.node, Var) or + isinstance(existing.node, TypeAlias) and not s.is_alias_def): + if isinstance(existing.node, TypeAlias) and not s.is_alias_def: + self.fail('Cannot assign multiple types to name "{}"' + ' without an explicit "Type[...]" annotation' + .format(lvalue.name), lvalue) + return False + non_global_scope = self.type or self.is_func_scope() - if isinstance(s.rvalue, RefExpr) and non_global_scope and lvalue.is_inferred_def: + if isinstance(s.rvalue, RefExpr) and non_global_scope: # Third rule: Non-subscripted right hand side creates a variable # at class and function scopes. For example: # @@ -2335,15 +2332,6 @@ def check_and_set_up_type_alias(self, s: AssignmentStmt) -> bool: # The above are only direct deps on other aliases. # For subscripted aliases, type deps from expansion are added in deps.py # (because the type is stored) - existing = self.current_symbol_table().get(lvalue.name) - # Type aliases can't be re-defined. - if existing and (isinstance(existing.node, Var) or - isinstance(existing.node, TypeAlias) and not s.is_alias_def): - if isinstance(existing.node, TypeAlias) and not s.is_alias_def: - self.fail('Cannot assign multiple types to name "{}"' - ' without an explicit "Type[...]" annotation' - .format(lvalue.name), lvalue) - return False check_for_explicit_any(res, self.options, self.is_typeshed_stub_file, self.msg, context=s) # when this type alias gets "inlined", the Any is not explicit anymore, @@ -2361,11 +2349,21 @@ def check_and_set_up_type_alias(self, s: AssignmentStmt) -> bool: alias_node = TypeAlias(res, self.qualified_name(lvalue.name), s.line, s.column, alias_tvars=alias_tvars, no_args=no_args) if existing: + # Alias got updated. + self.progress = True existing.node = alias_node else: self.add_symbol(lvalue.name, alias_node, s) if isinstance(rvalue, RefExpr) and isinstance(rvalue.node, TypeAlias): alias_node.normalized = rvalue.node.normalized + + # TODO: This is needed for one-to-one compatibility with old analyzer. + # See also the TODOs for named tuples and typed dicts: #6458. + lvalue.fullname = self.qualified_name(lvalue.name) + lvalue.is_inferred_def = True + lvalue.kind = kind = self.current_symbol_kind() + lvalue.node = self.make_name_lvalue_var(lvalue, kind, inferred=True) + return True def analyze_lvalue(self, lval: Lvalue, nested: bool = False, @@ -3232,7 +3230,12 @@ def visit_name_expr(self, expr: NameExpr) -> None: self.fail("'{}' is a type variable and only valid in type " "context".format(expr.name), expr) elif isinstance(n.node, PlaceholderNode): - self.defer() + if self.final_iteration: + self.fail('Cannot resolve name "{}",' + ' possible cyclic definition'.format(expr.name), + expr) + else: + self.defer() else: expr.kind = n.kind expr.node = n.node @@ -3445,7 +3448,12 @@ def visit_member_expr(self, expr: MemberExpr) -> None: n = self.rebind_symbol_table_node(n) if n: if isinstance(n.node, PlaceholderNode): - self.defer() + if self.final_iteration: + self.fail('Cannot resolve attribute "{}",' + ' possible cyclic definition'.format(expr.name), + expr) + else: + self.defer() return # TODO: What if None? expr.kind = n.kind diff --git a/test-data/unit/check-newsemanal.test b/test-data/unit/check-newsemanal.test index d3987a77d375c..240b6a8e1cbcd 100644 --- a/test-data/unit/check-newsemanal.test +++ b/test-data/unit/check-newsemanal.test @@ -339,12 +339,12 @@ def main() -> None: x # E: Name 'x' is not defined [case testNewAnalyzerCyclicDefinitions] -gx = gy # E: Cannot determine type of 'gy' +gx = gy # E: Cannot resolve name "gy", possible cyclic definition gy = gx def main() -> None: class C: def meth(self) -> None: - lx = ly # E: Cannot determine type of 'ly' + lx = ly # E: Cannot resolve name "ly", possible cyclic definition ly = lx [case testNewAnalyzerMutuallyRecursiveOverloadedFunctions] diff --git a/test-data/unit/check-type-aliases.test b/test-data/unit/check-type-aliases.test index 27fa6e198c56f..8974c0b5dae1a 100644 --- a/test-data/unit/check-type-aliases.test +++ b/test-data/unit/check-type-aliases.test @@ -359,11 +359,11 @@ reveal_type(a2) # E: Revealed type is 'Tuple[builtins.str, fallback=nt.D]' a3 = A3() reveal_type(a3) # E: Revealed type is 'Tuple[builtins.int, builtins.str, fallback=nt.E]' -Cls.A1('no') # E: Argument 1 to "C" has incompatible type "str"; expected "int" +Cls.A1('no') # E: Argument 1 has incompatible type "str"; expected "int" ca1 = Cls.A1(1) reveal_type(ca1) # E: Revealed type is 'Tuple[builtins.int, fallback=nt.C]' -Cls.A2(0) # E: Argument 1 to "D" has incompatible type "int"; expected "str" +Cls.A2(0) # E: Argument 1 has incompatible type "int"; expected "str" ca2 = Cls.A2('yes') reveal_type(ca2) # E: Revealed type is 'Tuple[builtins.str, fallback=nt.D]' diff --git a/test-data/unit/fine-grained.test b/test-data/unit/fine-grained.test index 6cdb45ce115ba..232bafc52e593 100644 --- a/test-data/unit/fine-grained.test +++ b/test-data/unit/fine-grained.test @@ -5334,8 +5334,8 @@ main:4: error: "C" expects no type arguments, but 1 given main:4: error: Invalid type "a.T" main:6: error: Free type variable expected in Generic[...] main:7: error: Invalid type "a.T" -main:10: error: Bad number of arguments for type alias, expected: 0, given: 1 main:10: error: Invalid type "a.T" +main:10: error: Bad number of arguments for type alias, expected: 0, given: 1 [case testChangeTypeVarToModule] # flags: --new-semantic-analyzer @@ -5363,8 +5363,8 @@ main:4: error: "C" expects no type arguments, but 1 given main:4: error: Invalid type "T" main:6: error: Free type variable expected in Generic[...] main:7: error: Invalid type "T" -main:10: error: Bad number of arguments for type alias, expected: 0, given: 1 main:10: error: Invalid type "T" +main:10: error: Bad number of arguments for type alias, expected: 0, given: 1 [case testChangeClassToModule] import a From e27727fb6a18251afbd168afea90965816d141c0 Mon Sep 17 00:00:00 2001 From: Ivan Levkivskyi Date: Thu, 7 Mar 2019 19:36:06 +0000 Subject: [PATCH 04/23] Make all assignment types follow the same pattern --- mypy/newsemanal/semanal.py | 79 ++++++++++++++++++++---------- mypy/newsemanal/semanal_enum.py | 17 ++++--- mypy/newsemanal/semanal_newtype.py | 36 +++++++------- mypy/newsemanal/semanal_shared.py | 6 ++- test-data/unit/check-redefine.test | 2 +- 5 files changed, 88 insertions(+), 52 deletions(-) diff --git a/mypy/newsemanal/semanal.py b/mypy/newsemanal/semanal.py index e90892d15123b..ad7cc2f23d86c 100644 --- a/mypy/newsemanal/semanal.py +++ b/mypy/newsemanal/semanal.py @@ -71,7 +71,8 @@ IntExpr, FloatExpr, UnicodeExpr, TempNode, OverloadPart, PlaceholderNode, COVARIANT, CONTRAVARIANT, INVARIANT, nongen_builtins, get_member_expr_fullname, REVEAL_TYPE, - REVEAL_LOCALS, is_final_node, TypedDictExpr, type_aliases_target_versions + REVEAL_LOCALS, is_final_node, TypedDictExpr, type_aliases_target_versions, + EnumCallExpr ) from mypy.tvar_scope import TypeVarScope from mypy.typevars import fill_typevars @@ -1940,6 +1941,9 @@ def should_wait(self, rv: Expression) -> bool: return True elif isinstance(rv, IndexExpr) and isinstance(rv.base, RefExpr): return self.should_wait(rv.base) + elif isinstance(rv, CallExpr) and isinstance(rv.callee, RefExpr): + # Only relevant for builtin SCC where things like 'TypeVar' may be not ready. + return self.should_wait(rv.callee) return False def can_be_type_alias(self, rv: Expression) -> bool: @@ -1952,7 +1956,6 @@ def can_be_type_alias(self, rv: Expression) -> bool: return False def visit_assignment_stmt(self, s: AssignmentStmt) -> None: - s.is_final_def = self.unwrap_final(s) tag = self.track_incomplete_refs() s.rvalue.accept(self) if self.found_incomplete_ref(tag) or self.should_wait(s.rvalue): @@ -1962,26 +1965,52 @@ def visit_assignment_stmt(self, s: AssignmentStmt) -> None: for expr in names_modified_by_assignment(s): self.mark_incomplete(expr.name, expr) return + + # The r.h.s. is now ready to be classified, first check if it is a special form: + # * type alias + if self.check_and_set_up_type_alias(s): + s.is_alias_def = True + return + + # * type variable definition + if self.process_typevar_declaration(s): + return + + # * type constructors if self.analyze_namedtuple_assign(s): return if self.analyze_typeddict_assign(s): return - if self.check_and_set_up_type_alias(s): - s.is_alias_def = True + if self.newtype_analyzer.process_newtype_declaration(s): return + if self.analyze_enum_assign(s): + return + + # OK, this is a regular assignment, perform the necessary analysis steps. + s.is_final_def = self.unwrap_final(s) self.analyze_lvalues(s) self.check_final_implicit_def(s) self.check_classvar(s) self.process_type_annotation(s) self.apply_dynamic_class_hook(s) - self.newtype_analyzer.process_newtype_declaration(s) - self.process_typevar_declaration(s) - self.enum_call_analyzer.process_enum_call(s, self.is_func_scope()) self.store_final_status(s) if not s.type: self.process_module_assignment(s.lvalues, s.rvalue, s) self.process__all__(s) + def analyze_enum_assign(self, s: AssignmentStmt) -> bool: + if not self.enum_call_analyzer.process_enum_call(s, self.is_func_scope()): + return False + # TODO: This is needed for one-to-one compatibility with old analyzer. + # See also the TODOs for named tuples and typed dicts: #6458. + lvalue = s.lvalues[0] + assert isinstance(lvalue, NameExpr) + lvalue.fullname = self.qualified_name(lvalue.name) + lvalue.is_inferred_def = True + lvalue.kind = kind = self.current_symbol_kind() + lvalue.node = self.make_name_lvalue_var(lvalue, kind, inferred=True) + return True + def analyze_namedtuple_assign(self, s: AssignmentStmt) -> bool: """Check if s defines a namedtuple.""" if isinstance(s.rvalue, CallExpr) and isinstance(s.rvalue.analyzed, NamedTupleExpr): @@ -2635,24 +2664,26 @@ def store_declared_types(self, lvalue: Lvalue, typ: Type) -> None: # This has been flagged elsewhere as an error, so just ignore here. pass - def process_typevar_declaration(self, s: AssignmentStmt) -> None: + def process_typevar_declaration(self, s: AssignmentStmt) -> bool: """Check if s declares a TypeVar; it yes, store it in symbol table.""" call = self.get_typevar_declaration(s) if not call: - return + return False lvalue = s.lvalues[0] assert isinstance(lvalue, NameExpr) + if s.type: + self.fail("Cannot declare the type of a type variable", s) + return False name = lvalue.name - if not lvalue.is_inferred_def: - if s.type: - self.fail("Cannot declare the type of a type variable", s) - else: - self.fail("Cannot redefine '%s' as a type variable" % name, s) - return + names = self.current_symbol_table() + existing = names.get(name) + if existing and not isinstance(existing.node, (TypeVarExpr, PlaceholderNode)): + self.fail("Cannot redefine '%s' as a type variable" % name, s) + return False if not self.check_typevar_name(call, name, s): - return + return False # Constraining types n_values = call.arg_kinds[1:].count(ARG_POS) @@ -2664,7 +2695,7 @@ def process_typevar_declaration(self, s: AssignmentStmt) -> None: n_values, s) if res is None: - return + return False variance, upper_bound = res if self.options.disallow_any_unimported: @@ -2688,21 +2719,19 @@ def process_typevar_declaration(self, s: AssignmentStmt) -> None: upper_bound = AnyType(TypeOfAny.implementation_artifact) # Yes, it's a valid type variable definition! Add it to the symbol table. - node = self.lookup(name, s) - assert node is not None - assert node.fullname is not None - node.kind = self.current_symbol_kind() - if isinstance(node.node, TypeVarExpr): + if existing and isinstance(existing.node, TypeVarExpr): # Existing definition from previous semanal iteration, use it. - type_var = node.node + type_var = existing.node type_var.values = values type_var.upper_bound = upper_bound type_var.variance = variance else: - type_var = TypeVarExpr(name, node.fullname, values, upper_bound, variance) + type_var = TypeVarExpr(name, self.qualified_name(name), + values, upper_bound, variance) type_var.line = call.line call.analyzed = type_var - node.node = type_var + self.add_symbol(name, type_var, s) + return True def check_typevar_name(self, call: CallExpr, name: str, context: Context) -> bool: name = unmangle(name) diff --git a/mypy/newsemanal/semanal_enum.py b/mypy/newsemanal/semanal_enum.py index 06999b60cbfaf..10ce89c8f3522 100644 --- a/mypy/newsemanal/semanal_enum.py +++ b/mypy/newsemanal/semanal_enum.py @@ -19,20 +19,23 @@ def __init__(self, options: Options, api: SemanticAnalyzerInterface) -> None: self.options = options self.api = api - def process_enum_call(self, s: AssignmentStmt, is_func_scope: bool) -> None: + def process_enum_call(self, s: AssignmentStmt, is_func_scope: bool) -> bool: """Check if s defines an Enum; if yes, store the definition in symbol table.""" if len(s.lvalues) != 1 or not isinstance(s.lvalues[0], NameExpr): - return + return False lvalue = s.lvalues[0] name = lvalue.name enum_call = self.check_enum_call(s.rvalue, name, is_func_scope) if enum_call is None: - return + return False # Yes, it's a valid Enum definition. Add it to the symbol table. - node = self.api.lookup(name, s) - if node: - node.kind = GDEF # TODO locally defined Enum - node.node = enum_call + names = self.api.current_symbol_table() + existing = names.get(name) + if existing: + existing.node = enum_call + else: + self.api.add_symbol(name, enum_call, s) + return True def check_enum_call(self, node: Expression, diff --git a/mypy/newsemanal/semanal_newtype.py b/mypy/newsemanal/semanal_newtype.py index d68c7c5107d71..c4d06de804cf6 100644 --- a/mypy/newsemanal/semanal_newtype.py +++ b/mypy/newsemanal/semanal_newtype.py @@ -5,10 +5,11 @@ from typing import Tuple, Optional -from mypy.types import Type, Instance, CallableType, NoneTyp, TupleType, AnyType, TypeOfAny +from mypy.types import Type, Instance, CallableType, NoneTyp, TupleType, AnyType from mypy.nodes import ( AssignmentStmt, NewTypeExpr, CallExpr, NameExpr, RefExpr, Context, StrExpr, BytesExpr, - UnicodeExpr, Block, FuncDef, Argument, TypeInfo, Var, SymbolTableNode, GDEF, MDEF, ARG_POS + UnicodeExpr, Block, FuncDef, Argument, TypeInfo, Var, SymbolTableNode, MDEF, ARG_POS, + PlaceholderNode ) from mypy.newsemanal.semanal_shared import SemanticAnalyzerInterface from mypy.options import Options @@ -26,17 +27,17 @@ def __init__(self, self.api = api self.msg = msg - def process_newtype_declaration(self, s: AssignmentStmt) -> None: + def process_newtype_declaration(self, s: AssignmentStmt) -> bool: """Check if s declares a NewType; if yes, store it in symbol table.""" # Extract and check all information from newtype declaration name, call = self.analyze_newtype_declaration(s) if name is None or call is None: - return + return False old_type = self.check_newtype_args(name, call, s) call.analyzed = NewTypeExpr(name, old_type, line=call.line) if old_type is None: - return + return True # Create the corresponding class definition if the aliased type is subtypeable if isinstance(old_type, TupleType): @@ -50,7 +51,7 @@ def process_newtype_declaration(self, s: AssignmentStmt) -> None: else: message = "Argument 2 to NewType(...) must be subclassable (got {})" self.fail(message.format(self.msg.format(old_type)), s) - return + return True check_for_explicit_any(old_type, self.options, self.api.is_typeshed_stub_file, self.msg, context=s) @@ -59,13 +60,9 @@ def process_newtype_declaration(self, s: AssignmentStmt) -> None: self.msg.unimported_type_becomes_any("Argument 2 to NewType(...)", old_type, s) # If so, add it to the symbol table. - node = self.api.lookup(name, s) - if node is None: - self.fail("Could not find {} in current namespace".format(name), s) - return - # TODO: why does NewType work in local scopes despite always being of kind GDEF? - node.kind = GDEF - call.analyzed.info = node.node = newtype_class_info + self.api.add_symbol(name, newtype_class_info, s) + call.analyzed.info = newtype_class_info + return True def analyze_newtype_declaration(self, s: AssignmentStmt) -> Tuple[Optional[str], Optional[CallExpr]]: @@ -78,11 +75,14 @@ def analyze_newtype_declaration(self, and s.rvalue.callee.fullname == 'typing.NewType'): lvalue = s.lvalues[0] name = s.lvalues[0].name - if not lvalue.is_inferred_def: - if s.type: - self.fail("Cannot declare the type of a NewType declaration", s) - else: - self.fail("Cannot redefine '%s' as a NewType" % name, s) + + if s.type: + self.fail("Cannot declare the type of a NewType declaration", s) + + names = self.api.current_symbol_table() + existing = names.get(name) + if existing and not isinstance(existing.node, (NewTypeExpr, PlaceholderNode)): + self.fail("Cannot redefine '%s' as a NewType" % name, s) # This dummy NewTypeExpr marks the call as sufficiently analyzed; it will be # overwritten later with a fully complete NewTypeExpr if there are no other diff --git a/mypy/newsemanal/semanal_shared.py b/mypy/newsemanal/semanal_shared.py index 82d5d1889a6a2..cb47956d90a45 100644 --- a/mypy/newsemanal/semanal_shared.py +++ b/mypy/newsemanal/semanal_shared.py @@ -6,7 +6,7 @@ from mypy.nodes import ( Context, SymbolTableNode, MypyFile, ImportedName, FuncDef, Node, TypeInfo, Expression, GDEF, - SymbolNode + SymbolNode, SymbolTable ) from mypy.util import correct_relative_import from mypy.types import Type, FunctionLike, Instance, TupleType @@ -123,6 +123,10 @@ def add_symbol_table_node(self, name: str, stnode: SymbolTableNode) -> bool: """Add node to the current symbol table.""" raise NotImplementedError + @abstractmethod + def current_symol_table(self) -> SymbolTable: + raise NotImplementedError + @abstractmethod def add_symbol(self, name: str, node: SymbolNode, context: Optional[Context], module_public: bool = True, module_hidden: bool = False) -> bool: diff --git a/test-data/unit/check-redefine.test b/test-data/unit/check-redefine.test index eef95ccf9154f..7c3e9a805a756 100644 --- a/test-data/unit/check-redefine.test +++ b/test-data/unit/check-redefine.test @@ -272,7 +272,7 @@ from typing import TypeVar def f() -> None: x = TypeVar('x') x = 1 # E: Invalid assignment target - reveal_type(x) # E: Revealed type is 'builtins.int' + reveal_type(x) # E: Revealed type is 'Any' y = 1 # NOTE: '"int" not callable' is due to test stubs y = TypeVar('y') # E: Cannot redefine 'y' as a type variable \ From 9a6e3c3a8c599d5ad2a0102ad0a191271ae33f65 Mon Sep 17 00:00:00 2001 From: Ivan Levkivskyi Date: Thu, 7 Mar 2019 21:54:01 +0000 Subject: [PATCH 05/23] Replace temporary Var hacks with a simpler and better hack --- mypy/checker.py | 6 ++- mypy/newsemanal/semanal.py | 70 +++++++++--------------------- mypy/nodes.py | 3 +- mypy/strconv.py | 4 +- mypy/treetransform.py | 1 + test-data/unit/check-redefine.test | 2 +- 6 files changed, 33 insertions(+), 53 deletions(-) diff --git a/mypy/checker.py b/mypy/checker.py index 267355cdf12de..517c8fbc1d50a 100644 --- a/mypy/checker.py +++ b/mypy/checker.py @@ -1715,6 +1715,9 @@ def visit_assignment_stmt(self, s: AssignmentStmt) -> None: with self.enter_final_context(s.is_final_def): self.check_assignment(s.lvalues[-1], s.rvalue, s.type is None, s.new_syntax) + if s.is_alias_def: + self.store_type(s.lvalues[-1], self.expr_checker.accept(s.rvalue)) + if (s.type is not None and self.options.disallow_any_unimported and has_any_from_unimported_type(s.type)): @@ -1824,7 +1827,8 @@ def check_assignment(self, lvalue: Lvalue, rvalue: Expression, infer_lvalue_type self.msg.concrete_only_assign(lvalue_type, rvalue) return if rvalue_type and infer_lvalue_type and not isinstance(lvalue_type, PartialType): - self.binder.assign_type(lvalue, rvalue_type, lvalue_type, False) + if not (isinstance(lvalue, NameExpr) and lvalue.is_special_form): + self.binder.assign_type(lvalue, rvalue_type, lvalue_type, False) elif index_lvalue: self.check_indexed_assignment(index_lvalue, rvalue, lvalue) diff --git a/mypy/newsemanal/semanal.py b/mypy/newsemanal/semanal.py index ad7cc2f23d86c..ec6c67395f4a8 100644 --- a/mypy/newsemanal/semanal.py +++ b/mypy/newsemanal/semanal.py @@ -1967,23 +1967,34 @@ def visit_assignment_stmt(self, s: AssignmentStmt) -> None: return # The r.h.s. is now ready to be classified, first check if it is a special form: + special_form = False # * type alias if self.check_and_set_up_type_alias(s): s.is_alias_def = True - return + special_form = True # * type variable definition - if self.process_typevar_declaration(s): - return + elif self.process_typevar_declaration(s): + special_form = True # * type constructors - if self.analyze_namedtuple_assign(s): - return - if self.analyze_typeddict_assign(s): - return - if self.newtype_analyzer.process_newtype_declaration(s): - return - if self.analyze_enum_assign(s): + elif self.analyze_namedtuple_assign(s): + special_form = True + elif self.analyze_typeddict_assign(s): + special_form = True + elif self.newtype_analyzer.process_newtype_declaration(s): + special_form = True + elif self.enum_call_analyzer.process_enum_call(s, self.is_func_scope()): + special_form = True + + if special_form: + lvalue = s.lvalues[0] + assert isinstance(lvalue, NameExpr) + lvalue.is_special_form = True + # Add basic compatibility info. + if self.current_symbol_kind() == GDEF: + lvalue.fullname = self.qualified_name(lvalue.name) + lvalue.kind = self.current_symbol_kind() return # OK, this is a regular assignment, perform the necessary analysis steps. @@ -1998,19 +2009,6 @@ def visit_assignment_stmt(self, s: AssignmentStmt) -> None: self.process_module_assignment(s.lvalues, s.rvalue, s) self.process__all__(s) - def analyze_enum_assign(self, s: AssignmentStmt) -> bool: - if not self.enum_call_analyzer.process_enum_call(s, self.is_func_scope()): - return False - # TODO: This is needed for one-to-one compatibility with old analyzer. - # See also the TODOs for named tuples and typed dicts: #6458. - lvalue = s.lvalues[0] - assert isinstance(lvalue, NameExpr) - lvalue.fullname = self.qualified_name(lvalue.name) - lvalue.is_inferred_def = True - lvalue.kind = kind = self.current_symbol_kind() - lvalue.node = self.make_name_lvalue_var(lvalue, kind, inferred=True) - return True - def analyze_namedtuple_assign(self, s: AssignmentStmt) -> bool: """Check if s defines a namedtuple.""" if isinstance(s.rvalue, CallExpr) and isinstance(s.rvalue.analyzed, NamedTupleExpr): @@ -2026,16 +2024,6 @@ def analyze_namedtuple_assign(self, s: AssignmentStmt) -> bool: # Yes, it's a valid namedtuple, but defer if it is not ready. if not info: self.mark_incomplete(name, lvalue, becomes_typeinfo=True) - else: - # TODO: This is needed for one-to-one compatibility with old analyzer, otherwise - # type checker will try to infer Any for the l.h.s. causing named tuple class - # object to have type Any when it appears in runtime context. - # Remove this and update the checker after new analyzer is the default one! - # See also #6458. - lvalue.fullname = self.qualified_name(name) - lvalue.is_inferred_def = True - lvalue.kind = kind = self.current_symbol_kind() - lvalue.node = self.make_name_lvalue_var(lvalue, kind, inferred=True) return True def analyze_typeddict_assign(self, s: AssignmentStmt) -> bool: @@ -2053,14 +2041,6 @@ def analyze_typeddict_assign(self, s: AssignmentStmt) -> bool: # Yes, it's a valid typed dict, but defer if it is not ready. if not info: self.mark_incomplete(name, lvalue, becomes_typeinfo=True) - else: - # TODO: This is needed for one-to-one compatibility with old analyzer, otherwise - # type checker will try to infer Any for the l.h.s. - # Remove this after new analyzer is the default one! - lvalue.fullname = self.qualified_name(name) - lvalue.is_inferred_def = True - lvalue.kind = kind = self.current_symbol_kind() - lvalue.node = self.make_name_lvalue_var(lvalue, kind, inferred=True) return True def analyze_lvalues(self, s: AssignmentStmt) -> None: @@ -2385,14 +2365,6 @@ def check_and_set_up_type_alias(self, s: AssignmentStmt) -> bool: self.add_symbol(lvalue.name, alias_node, s) if isinstance(rvalue, RefExpr) and isinstance(rvalue.node, TypeAlias): alias_node.normalized = rvalue.node.normalized - - # TODO: This is needed for one-to-one compatibility with old analyzer. - # See also the TODOs for named tuples and typed dicts: #6458. - lvalue.fullname = self.qualified_name(lvalue.name) - lvalue.is_inferred_def = True - lvalue.kind = kind = self.current_symbol_kind() - lvalue.node = self.make_name_lvalue_var(lvalue, kind, inferred=True) - return True def analyze_lvalue(self, lval: Lvalue, nested: bool = False, diff --git a/mypy/nodes.py b/mypy/nodes.py index 6c17d6d3b1f14..3c59c7d995a72 100644 --- a/mypy/nodes.py +++ b/mypy/nodes.py @@ -1418,11 +1418,12 @@ class NameExpr(RefExpr): This refers to a local name, global name or a module. """ - __slots__ = ('name',) + __slots__ = ('name', 'is_special_form') def __init__(self, name: str) -> None: super().__init__() self.name = name # Name referred to (may be qualified) + self.is_special_form = False def accept(self, visitor: ExpressionVisitor[T]) -> T: return visitor.visit_name_expr(self) diff --git a/mypy/strconv.py b/mypy/strconv.py index de9548910ab7e..21a25876f21f4 100644 --- a/mypy/strconv.py +++ b/mypy/strconv.py @@ -345,7 +345,9 @@ def visit_star_expr(self, o: 'mypy.nodes.StarExpr') -> str: return self.dump([o.expr], o) def visit_name_expr(self, o: 'mypy.nodes.NameExpr') -> str: - pretty = self.pretty_name(o.name, o.kind, o.fullname, o.is_inferred_def, o.node) + pretty = self.pretty_name(o.name, o.kind, o.fullname, + o.is_inferred_def or o.is_special_form, + o.node) if isinstance(o.node, mypy.nodes.Var) and o.node.is_final: pretty += ' = {}'.format(o.node.final_value) return short_type(o) + '(' + pretty + ')' diff --git a/mypy/treetransform.py b/mypy/treetransform.py index 64345e59060b9..f68794d8907b4 100644 --- a/mypy/treetransform.py +++ b/mypy/treetransform.py @@ -329,6 +329,7 @@ def duplicate_name(self, node: NameExpr) -> NameExpr: # visit_name_expr() is used when there is no such restriction. new = NameExpr(node.name) self.copy_ref(new, node) + new.is_special_form = node.is_special_form return new def visit_member_expr(self, node: MemberExpr) -> MemberExpr: diff --git a/test-data/unit/check-redefine.test b/test-data/unit/check-redefine.test index 7c3e9a805a756..eef95ccf9154f 100644 --- a/test-data/unit/check-redefine.test +++ b/test-data/unit/check-redefine.test @@ -272,7 +272,7 @@ from typing import TypeVar def f() -> None: x = TypeVar('x') x = 1 # E: Invalid assignment target - reveal_type(x) # E: Revealed type is 'Any' + reveal_type(x) # E: Revealed type is 'builtins.int' y = 1 # NOTE: '"int" not callable' is due to test stubs y = TypeVar('y') # E: Cannot redefine 'y' as a type variable \ From 7ba8c8f51eb164b713eba6b16f03b7a49b4e2b8a Mon Sep 17 00:00:00 2001 From: Ivan Levkivskyi Date: Thu, 7 Mar 2019 23:08:56 +0000 Subject: [PATCH 06/23] Some cleanups and comments/docstrings --- mypy/checker.py | 2 + mypy/newsemanal/semanal.py | 73 +++++++++++++++++++------- mypy/nodes.py | 1 + test-data/unit/check-type-aliases.test | 1 - 4 files changed, 58 insertions(+), 19 deletions(-) diff --git a/mypy/checker.py b/mypy/checker.py index 517c8fbc1d50a..5e26e837ecdac 100644 --- a/mypy/checker.py +++ b/mypy/checker.py @@ -1716,6 +1716,8 @@ def visit_assignment_stmt(self, s: AssignmentStmt) -> None: self.check_assignment(s.lvalues[-1], s.rvalue, s.type is None, s.new_syntax) if s.is_alias_def: + # We do this mostly for compatibility with old semantic analyzer. + # TODO: should we get rid of this? self.store_type(s.lvalues[-1], self.expr_checker.accept(s.rvalue)) if (s.type is not None and diff --git a/mypy/newsemanal/semanal.py b/mypy/newsemanal/semanal.py index ec6c67395f4a8..793f14a034c14 100644 --- a/mypy/newsemanal/semanal.py +++ b/mypy/newsemanal/semanal.py @@ -1882,6 +1882,11 @@ def add_type_alias_deps(self, aliases_used: Iterable[str], self.cur_mod_node.alias_deps[target].update(aliases_used) def is_none_alias(self, node: Expression) -> bool: + """Is this a r.h.s. for a None alias? + + We special case the assignments like Void = type(None), to allow using + Void in type annotations. + """ if isinstance(node, CallExpr): if (isinstance(node.callee, NameExpr) and len(node.args) == 1 and isinstance(node.args[0], NameExpr)): @@ -1893,7 +1898,15 @@ def is_none_alias(self, node: Expression) -> bool: return False def is_type_ref(self, rv: Expression, bare: bool = False) -> bool: - """Does this expression refers to a type?""" + """Does this expression refers to a type? + + Thus includes: + * Special forms, like Any or Union + * Classes (except subscripted enums) + * Other type aliases + * PlaceholderNodes with becomes_typeinfo=True (these can be not ready class + definitions, and not ready aliases). + """ if not isinstance(rv, RefExpr): return False if isinstance(rv.node, TypeVarExpr): @@ -1901,11 +1914,13 @@ def is_type_ref(self, rv: Expression, bare: bool = False) -> bool: rv.fullname), rv) return False - valid_refs = {'typing.Any', 'typing.Tuple', 'typing.Callable'} if bare else type_constructors + if bare: + valid_refs = {'typing.Any', 'typing.Tuple', 'typing.Callable'} + else: + valid_refs = type_constructors if isinstance(rv.node, TypeAlias) or rv.fullname in valid_refs: return True - if isinstance(rv.node, TypeInfo): if bare: return True @@ -1926,6 +1941,13 @@ def is_type_ref(self, rv: Expression, bare: bool = False) -> bool: return False def should_wait(self, rv: Expression) -> bool: + """Can we already classify this r.h.s. of an assignment or should wait? + + This returns True if we don't have enough information to decide whether + an assignment is just a normal variable definition or a special form. + Always return False if this is a final iteration, this will typically cause + the assignment to be classified as variable plus emmit an error. + """ if self.final_iteration: # No chance, nothing has changed. return False @@ -1942,11 +1964,17 @@ def should_wait(self, rv: Expression) -> bool: elif isinstance(rv, IndexExpr) and isinstance(rv.base, RefExpr): return self.should_wait(rv.base) elif isinstance(rv, CallExpr) and isinstance(rv.callee, RefExpr): - # Only relevant for builtin SCC where things like 'TypeVar' may be not ready. + # This is only relevant for builtin SCC where things like 'TypeVar' + # may be not ready. return self.should_wait(rv.callee) return False def can_be_type_alias(self, rv: Expression) -> bool: + """Is this a valid r.h.s. for an alias definition? + + Note: this function should be only called for expressions where self.should_wait() + returns False. + """ if isinstance(rv, RefExpr) and self.is_type_ref(rv, bare=True): return True if isinstance(rv, IndexExpr) and self.is_type_ref(rv.base, bare=False): @@ -1955,6 +1983,18 @@ def can_be_type_alias(self, rv: Expression) -> bool: return True return False + def record_special_form_lvalue(self, s: AssignmentStmt) -> None: + """Record minimal necessary information about l.h.s. of a special form. + + This exists mostly for compatibility with the old semantic analyzer. + """ + lvalue = s.lvalues[0] + assert isinstance(lvalue, NameExpr) + lvalue.is_special_form = True + if self.current_symbol_kind() == GDEF: + lvalue.fullname = self.qualified_name(lvalue.name) + lvalue.kind = self.current_symbol_kind() + def visit_assignment_stmt(self, s: AssignmentStmt) -> None: tag = self.track_incomplete_refs() s.rvalue.accept(self) @@ -1972,11 +2012,9 @@ def visit_assignment_stmt(self, s: AssignmentStmt) -> None: if self.check_and_set_up_type_alias(s): s.is_alias_def = True special_form = True - # * type variable definition elif self.process_typevar_declaration(s): special_form = True - # * type constructors elif self.analyze_namedtuple_assign(s): special_form = True @@ -1986,15 +2024,8 @@ def visit_assignment_stmt(self, s: AssignmentStmt) -> None: special_form = True elif self.enum_call_analyzer.process_enum_call(s, self.is_func_scope()): special_form = True - if special_form: - lvalue = s.lvalues[0] - assert isinstance(lvalue, NameExpr) - lvalue.is_special_form = True - # Add basic compatibility info. - if self.current_symbol_kind() == GDEF: - lvalue.fullname = self.qualified_name(lvalue.name) - lvalue.kind = self.current_symbol_kind() + self.record_special_form_lvalue(s) return # OK, this is a regular assignment, perform the necessary analysis steps. @@ -2328,11 +2359,15 @@ def check_and_set_up_type_alias(self, s: AssignmentStmt) -> bool: alias_tvars, depends_on, qualified_tvars = [], set(), [] else: tag = self.track_incomplete_refs() - res, alias_tvars, depends_on, qualified_tvars = self.analyze_alias(rvalue, allow_placeholder=True) + res, alias_tvars, depends_on, qualified_tvars = \ + self.analyze_alias(rvalue, allow_placeholder=True) if not res: return False if self.found_incomplete_ref(tag): - self.add_symbol(lvalue.name, PlaceholderNode(self.qualified_name(lvalue.name), rvalue, True), None) + # Since we have got here, we know this must be a type alias (incomplete refs + # may appear in nested positions), therefore use becomes_typeinfo=True. + self.add_symbol(lvalue.name, PlaceholderNode(self.qualified_name(lvalue.name), + rvalue, becomes_typeinfo=True), s) return True self.add_type_alias_deps(depends_on) # In addition to the aliases used, we add deps on unbound @@ -2358,8 +2393,10 @@ def check_and_set_up_type_alias(self, s: AssignmentStmt) -> bool: alias_node = TypeAlias(res, self.qualified_name(lvalue.name), s.line, s.column, alias_tvars=alias_tvars, no_args=no_args) if existing: - # Alias got updated. - self.progress = True + # Did alias get updated? + if (isinstance(existing.node, PlaceholderNode) or + isinstance(existing.node, TypeAlias) and existing.node.target != res): + self.progress = True existing.node = alias_node else: self.add_symbol(lvalue.name, alias_node, s) diff --git a/mypy/nodes.py b/mypy/nodes.py index 3c59c7d995a72..b08cda22d5de7 100644 --- a/mypy/nodes.py +++ b/mypy/nodes.py @@ -1423,6 +1423,7 @@ class NameExpr(RefExpr): def __init__(self, name: str) -> None: super().__init__() self.name = name # Name referred to (may be qualified) + # Is this a l.h.s. of a special form assignment like typed dict or type variable? self.is_special_form = False def accept(self, visitor: ExpressionVisitor[T]) -> T: diff --git a/test-data/unit/check-type-aliases.test b/test-data/unit/check-type-aliases.test index 8974c0b5dae1a..bada7486bd6e8 100644 --- a/test-data/unit/check-type-aliases.test +++ b/test-data/unit/check-type-aliases.test @@ -336,7 +336,6 @@ reveal_type(w) # E: Revealed type is '__main__.C[builtins.int]' [out] [case testTypeAliasesToNamedTuple] -# flags: --new-semantic-analyzer from nt import C, D, E A1 = C From 746a424a0507a2c337989fda2e9072ce805fd702 Mon Sep 17 00:00:00 2001 From: Ivan Levkivskyi Date: Thu, 7 Mar 2019 23:16:50 +0000 Subject: [PATCH 07/23] Add one more test --- test-data/unit/check-newsemanal.test | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/test-data/unit/check-newsemanal.test b/test-data/unit/check-newsemanal.test index 240b6a8e1cbcd..693b0c8e00cad 100644 --- a/test-data/unit/check-newsemanal.test +++ b/test-data/unit/check-newsemanal.test @@ -347,6 +347,17 @@ def main() -> None: lx = ly # E: Cannot resolve name "ly", possible cyclic definition ly = lx +[case testNewAnalyzerCyclicDefinitionCrossModule] +import b +[file a.py] +import b +x = b.x # E: Cannot determine type of 'x' +[file b.py] +import a +x = a.x # E: Cannot resolve attribute "x", possible cyclic definition \ + # E: Module has no attribute "x" +[builtins fixtures/module.pyi] + [case testNewAnalyzerMutuallyRecursiveOverloadedFunctions] from typing import overload, Union From 1e1804b2fc28e7887750a75b9ac34c7b98e8a118 Mon Sep 17 00:00:00 2001 From: Ivan Levkivskyi Date: Fri, 8 Mar 2019 05:53:28 -0800 Subject: [PATCH 08/23] Some more cleanups --- mypy/checker.py | 1 + mypy/newsemanal/semanal.py | 38 ++++++++++++++++++++++----------- mypy/newsemanal/semanal_enum.py | 10 +++++++-- 3 files changed, 34 insertions(+), 15 deletions(-) diff --git a/mypy/checker.py b/mypy/checker.py index 5e26e837ecdac..2fb94e5f3abd2 100644 --- a/mypy/checker.py +++ b/mypy/checker.py @@ -1829,6 +1829,7 @@ def check_assignment(self, lvalue: Lvalue, rvalue: Expression, infer_lvalue_type self.msg.concrete_only_assign(lvalue_type, rvalue) return if rvalue_type and infer_lvalue_type and not isinstance(lvalue_type, PartialType): + # Don't use type binder for definitions of special forms, like named tuples. if not (isinstance(lvalue, NameExpr) and lvalue.is_special_form): self.binder.assign_type(lvalue, rvalue_type, lvalue_type, False) diff --git a/mypy/newsemanal/semanal.py b/mypy/newsemanal/semanal.py index 793f14a034c14..be4be946676d1 100644 --- a/mypy/newsemanal/semanal.py +++ b/mypy/newsemanal/semanal.py @@ -560,11 +560,11 @@ def _visit_func_def(self, defn: FuncDef) -> None: analyzer = self.type_analyzer() tag = self.track_incomplete_refs() result = analyzer.visit_callable_type(defn.type, nested=False) - if self.found_incomplete_ref(tag): + # Don't store not ready types (including placeholders). + if self.found_incomplete_ref(tag) or has_placeholder(result): + self.defer() return defn.type = result - if has_placeholder(defn.type): - self.defer() self.add_type_alias_deps(analyzer.aliases_used) self.check_function_signature(defn) if isinstance(defn, FuncDef): @@ -1898,7 +1898,7 @@ def is_none_alias(self, node: Expression) -> bool: return False def is_type_ref(self, rv: Expression, bare: bool = False) -> bool: - """Does this expression refers to a type? + """Does this expression refer to a type? Thus includes: * Special forms, like Any or Union @@ -1906,6 +1906,9 @@ def is_type_ref(self, rv: Expression, bare: bool = False) -> bool: * Other type aliases * PlaceholderNodes with becomes_typeinfo=True (these can be not ready class definitions, and not ready aliases). + + If bare is True, this is not a base of an index expression, so some special + forms are not valid (like a bare Union). """ if not isinstance(rv, RefExpr): return False @@ -1924,6 +1927,7 @@ def is_type_ref(self, rv: Expression, bare: bool = False) -> bool: if isinstance(rv.node, TypeInfo): if bare: return True + # Assignment color = Color['RED'] defines a variable, not an alias. return not rv.node.is_enum if isinstance(rv, NameExpr): @@ -2213,9 +2217,8 @@ def process_type_annotation(self, s: AssignmentStmt) -> None: lvalue = s.lvalues[-1] allow_tuple_literal = isinstance(lvalue, TupleExpr) analyzed = self.anal_type(s.type, allow_tuple_literal=allow_tuple_literal) - if analyzed is None: - return - if has_placeholder(analyzed): + # Don't store not ready types (including placeholders). + if analyzed is None or has_placeholder(analyzed): self.defer() return s.type = analyzed @@ -2329,9 +2332,14 @@ def check_and_set_up_type_alias(self, s: AssignmentStmt) -> bool: return False existing = self.current_symbol_table().get(lvalue.name) - # Type aliases can't be re-defined. + # Third rule: type aliases can't be re-defined. For example: + # A: Type[float] = int + # A = float # OK, but this doesn't define an alias + # B = int + # B = float # Error! if existing and (isinstance(existing.node, Var) or isinstance(existing.node, TypeAlias) and not s.is_alias_def): + # Note: if is_alias_def=True, this is just a node from previous iteration. if isinstance(existing.node, TypeAlias) and not s.is_alias_def: self.fail('Cannot assign multiple types to name "{}"' ' without an explicit "Type[...]" annotation' @@ -2340,7 +2348,7 @@ def check_and_set_up_type_alias(self, s: AssignmentStmt) -> bool: non_global_scope = self.type or self.is_func_scope() if isinstance(s.rvalue, RefExpr) and non_global_scope: - # Third rule: Non-subscripted right hand side creates a variable + # Fourth rule (special case): Non-subscripted right hand side creates a variable # at class and function scopes. For example: # # class Model: @@ -2375,11 +2383,11 @@ def check_and_set_up_type_alias(self, s: AssignmentStmt) -> bool: self.add_type_alias_deps(qualified_tvars) # The above are only direct deps on other aliases. # For subscripted aliases, type deps from expansion are added in deps.py - # (because the type is stored) + # (because the type is stored). check_for_explicit_any(res, self.options, self.is_typeshed_stub_file, self.msg, context=s) - # when this type alias gets "inlined", the Any is not explicit anymore, - # so we need to replace it with non-explicit Anys + # When this type alias gets "inlined", the Any is not explicit anymore, + # so we need to replace it with non-explicit Anys. res = make_any_non_explicit(res) no_args = isinstance(res, Instance) and not res.args fix_instance_types(res, self.fail) @@ -2674,7 +2682,11 @@ def store_declared_types(self, lvalue: Lvalue, typ: Type) -> None: pass def process_typevar_declaration(self, s: AssignmentStmt) -> bool: - """Check if s declares a TypeVar; it yes, store it in symbol table.""" + """Check if s declares a TypeVar; it yes, store it in symbol table. + + Return True if this looks like a type variable declaration (but maybe + with errors), otherwise return False. + """ call = self.get_typevar_declaration(s) if not call: return False diff --git a/mypy/newsemanal/semanal_enum.py b/mypy/newsemanal/semanal_enum.py index 10ce89c8f3522..2b10105ece9c5 100644 --- a/mypy/newsemanal/semanal_enum.py +++ b/mypy/newsemanal/semanal_enum.py @@ -20,7 +20,11 @@ def __init__(self, options: Options, api: SemanticAnalyzerInterface) -> None: self.api = api def process_enum_call(self, s: AssignmentStmt, is_func_scope: bool) -> bool: - """Check if s defines an Enum; if yes, store the definition in symbol table.""" + """Check if s defines an Enum; if yes, store the definition in symbol table. + + Return True if this looks like a type variable declaration (but maybe + with errors), otherwise return False. + """ if len(s.lvalues) != 1 or not isinstance(s.lvalues[0], NameExpr): return False lvalue = s.lvalues[0] @@ -31,7 +35,9 @@ def process_enum_call(self, s: AssignmentStmt, is_func_scope: bool) -> bool: # Yes, it's a valid Enum definition. Add it to the symbol table. names = self.api.current_symbol_table() existing = names.get(name) - if existing: + if existing and isinstance(existing.node, TypeInfo) and existing.info.is_enum: + # Existing definition from previous semanal iteration, use it. + # Some types might get updated. existing.node = enum_call else: self.api.add_symbol(name, enum_call, s) From 555a2091c418f252682fe574b91636d212cd37cf Mon Sep 17 00:00:00 2001 From: Ivan Levkivskyi Date: Fri, 8 Mar 2019 05:58:59 -0800 Subject: [PATCH 09/23] Typo and comment --- mypy/newsemanal/semanal.py | 2 ++ mypy/newsemanal/semanal_enum.py | 2 +- 2 files changed, 3 insertions(+), 1 deletion(-) diff --git a/mypy/newsemanal/semanal.py b/mypy/newsemanal/semanal.py index be4be946676d1..0b05b69744c47 100644 --- a/mypy/newsemanal/semanal.py +++ b/mypy/newsemanal/semanal.py @@ -1918,6 +1918,8 @@ def is_type_ref(self, rv: Expression, bare: bool = False) -> bool: return False if bare: + # These three are valid even if bare, for example + # A = Tuple is just equivalent to A = Tuple[Any, ...]. valid_refs = {'typing.Any', 'typing.Tuple', 'typing.Callable'} else: valid_refs = type_constructors diff --git a/mypy/newsemanal/semanal_enum.py b/mypy/newsemanal/semanal_enum.py index 2b10105ece9c5..41feed2a9d1b0 100644 --- a/mypy/newsemanal/semanal_enum.py +++ b/mypy/newsemanal/semanal_enum.py @@ -35,7 +35,7 @@ def process_enum_call(self, s: AssignmentStmt, is_func_scope: bool) -> bool: # Yes, it's a valid Enum definition. Add it to the symbol table. names = self.api.current_symbol_table() existing = names.get(name) - if existing and isinstance(existing.node, TypeInfo) and existing.info.is_enum: + if existing and isinstance(existing.node, TypeInfo) and existing.node.is_enum: # Existing definition from previous semanal iteration, use it. # Some types might get updated. existing.node = enum_call From 4378bd7badf6c37392286dacbac1b1ba92b3354e Mon Sep 17 00:00:00 2001 From: Ivan Levkivskyi Date: Fri, 8 Mar 2019 06:20:15 -0800 Subject: [PATCH 10/23] Add enum symbols exactly once --- mypy/newsemanal/semanal.py | 9 +++++++- mypy/newsemanal/semanal_enum.py | 34 +++++++++++++--------------- test-data/unit/check-newsemanal.test | 6 +++++ 3 files changed, 30 insertions(+), 19 deletions(-) diff --git a/mypy/newsemanal/semanal.py b/mypy/newsemanal/semanal.py index 0b05b69744c47..974e7526c3fd7 100644 --- a/mypy/newsemanal/semanal.py +++ b/mypy/newsemanal/semanal.py @@ -2028,7 +2028,7 @@ def visit_assignment_stmt(self, s: AssignmentStmt) -> None: special_form = True elif self.newtype_analyzer.process_newtype_declaration(s): special_form = True - elif self.enum_call_analyzer.process_enum_call(s, self.is_func_scope()): + elif self.analyze_enum_assign(s): special_form = True if special_form: self.record_special_form_lvalue(s) @@ -2046,6 +2046,13 @@ def visit_assignment_stmt(self, s: AssignmentStmt) -> None: self.process_module_assignment(s.lvalues, s.rvalue, s) self.process__all__(s) + def analyze_enum_assign(self, s: AssignmentStmt) -> bool: + """Check if s defines an Enum.""" + if isinstance(s.rvalue, CallExpr) and isinstance(s.rvalue.analyzed, EnumCallExpr): + # Already analyzed enum -- nothing to do here. + return True + return self.enum_call_analyzer.process_enum_call(s, self.is_func_scope()) + def analyze_namedtuple_assign(self, s: AssignmentStmt) -> bool: """Check if s defines a namedtuple.""" if isinstance(s.rvalue, CallExpr) and isinstance(s.rvalue.analyzed, NamedTupleExpr): diff --git a/mypy/newsemanal/semanal_enum.py b/mypy/newsemanal/semanal_enum.py index 41feed2a9d1b0..e149ff54ae16d 100644 --- a/mypy/newsemanal/semanal_enum.py +++ b/mypy/newsemanal/semanal_enum.py @@ -33,14 +33,7 @@ def process_enum_call(self, s: AssignmentStmt, is_func_scope: bool) -> bool: if enum_call is None: return False # Yes, it's a valid Enum definition. Add it to the symbol table. - names = self.api.current_symbol_table() - existing = names.get(name) - if existing and isinstance(existing.node, TypeInfo) and existing.node.is_enum: - # Existing definition from previous semanal iteration, use it. - # Some types might get updated. - existing.node = enum_call - else: - self.api.add_symbol(name, enum_call, s) + self.api.add_symbol(name, enum_call, s) return True def check_enum_call(self, @@ -71,18 +64,19 @@ class A(enum.Enum): items, values, ok = self.parse_enum_call_args(call, fullname.split('.')[-1]) if not ok: # Error. Construct dummy return value. - return self.build_enum_call_typeinfo(var_name, [], fullname) - name = cast(Union[StrExpr, UnicodeExpr], call.args[0]).value - if name != var_name or is_func_scope: - # Give it a unique name derived from the line number. - name += '@' + str(call.line) - info = self.build_enum_call_typeinfo(name, items, fullname) - # Store it as a global just in case it would remain anonymous. - # (Or in the nearest class if there is one.) - stnode = SymbolTableNode(GDEF, info) - self.api.add_symbol_table_node(name, stnode) + info = self.build_enum_call_typeinfo(var_name, [], fullname) + else: + name = cast(Union[StrExpr, UnicodeExpr], call.args[0]).value + if name != var_name or is_func_scope: + # Give it a unique name derived from the line number. + name += '@' + str(call.line) + info = self.build_enum_call_typeinfo(name, items, fullname) + # Store generated TypeInfo under both names, see semanal_namedtuple for more details. + if name != var_name or is_func_scope: + self.api.add_symbol_skip_local(name, info) call.analyzed = EnumCallExpr(info, items, values) call.analyzed.set_line(call.line, call.column) + info.line = node.line return info def build_enum_call_typeinfo(self, name: str, items: List[str], fullname: str) -> TypeInfo: @@ -102,6 +96,10 @@ def build_enum_call_typeinfo(self, name: str, items: List[str], fullname: str) - def parse_enum_call_args(self, call: CallExpr, class_name: str) -> Tuple[List[str], List[Optional[Expression]], bool]: + """Parse arguments of an Enum call. + + Return a tuple of fields, values, was there an error. + """ args = call.args if len(args) < 2: return self.fail_enum_call_arg("Too few arguments for %s()" % class_name, call) diff --git a/test-data/unit/check-newsemanal.test b/test-data/unit/check-newsemanal.test index 693b0c8e00cad..df8ebb3837c24 100644 --- a/test-data/unit/check-newsemanal.test +++ b/test-data/unit/check-newsemanal.test @@ -1760,3 +1760,9 @@ class C(metaclass=Meta): [case testNewAnalyzerFunctionError] def f(x: asdf) -> None: # E: Name 'asdf' is not defined pass + +[case testNewAnalyzerEnumRedefinition] +from enum import Enum + +A = Enum('A', ['x', 'y']) +A = Enum('A', ['z', 't']) # E: Name 'A' already defined on line 3 From 259eb37805f1114e1af9a5cccc2c14812fce715e Mon Sep 17 00:00:00 2001 From: Ivan Levkivskyi Date: Fri, 8 Mar 2019 11:59:08 -0800 Subject: [PATCH 11/23] Update NewType to mimic the logic in ClassDef --- mypy/newsemanal/semanal.py | 6 ++- mypy/newsemanal/semanal_newtype.py | 68 ++++++++++++++++++++++------ test-data/unit/check-newsemanal.test | 58 ++++++++++++++++++++++++ 3 files changed, 115 insertions(+), 17 deletions(-) diff --git a/mypy/newsemanal/semanal.py b/mypy/newsemanal/semanal.py index 974e7526c3fd7..fb60db9a0e9ac 100644 --- a/mypy/newsemanal/semanal.py +++ b/mypy/newsemanal/semanal.py @@ -942,7 +942,6 @@ def analyze_class(self, defn: ClassDef) -> None: if self.analyze_namedtuple_classdef(defn): return - # Create TypeInfo for class now that base classes and the MRO can be calculated. self.prepare_class_def(defn) @@ -4198,7 +4197,10 @@ def add_symbol_table_node(self, name: str, symbol: SymbolTableNode, if (existing is not None and context is not None and (not isinstance(existing.node, PlaceholderNode) - or isinstance(symbol.node, PlaceholderNode))): + or isinstance(symbol.node, PlaceholderNode) and + # Allow replacing becomes_typeinfo=False with becomes_typeinfo=True, + # this can happend for type aliases and NewTypes. + not symbol.node.becomes_typeinfo)): # There is an existing node, so this may be a redefinition. # If the new node points to the same node as the old one, # or if both old and new nodes are placeholders, we don't diff --git a/mypy/newsemanal/semanal_newtype.py b/mypy/newsemanal/semanal_newtype.py index c4d06de804cf6..3885628562036 100644 --- a/mypy/newsemanal/semanal_newtype.py +++ b/mypy/newsemanal/semanal_newtype.py @@ -5,7 +5,9 @@ from typing import Tuple, Optional -from mypy.types import Type, Instance, CallableType, NoneTyp, TupleType, AnyType +from mypy.types import ( + Type, Instance, CallableType, NoneTyp, TupleType, AnyType, PlaceholderType +) from mypy.nodes import ( AssignmentStmt, NewTypeExpr, CallExpr, NameExpr, RefExpr, Context, StrExpr, BytesExpr, UnicodeExpr, Block, FuncDef, Argument, TypeInfo, Var, SymbolTableNode, MDEF, ARG_POS, @@ -28,15 +30,34 @@ def __init__(self, self.msg = msg def process_newtype_declaration(self, s: AssignmentStmt) -> bool: - """Check if s declares a NewType; if yes, store it in symbol table.""" - # Extract and check all information from newtype declaration + """Check if s declares a NewType; if yes, store it in symbol table. + + Return True if it is a valid declaration, but maybe defer until base type + is known. + + The logic in this function mostly copies the logic for visit_class_def() + with a single (non-Generic) base. + """ name, call = self.analyze_newtype_declaration(s) if name is None or call is None: return False - - old_type = self.check_newtype_args(name, call, s) - call.analyzed = NewTypeExpr(name, old_type, line=call.line) + # OK, now we now this is a NewType, but the base type may be not ready yet, + # add placeholder as we do for ClassDef. + + fullname = self.api.qualified_name(name) + if (not call.analyzed or + isinstance(call.analyzed, NewTypeExpr) and not call.analyzed.info): + # Start from labeling this as future class with becomes_typeinfo=True, + # as we do for normal ClassDefs. + self.api.add_symbol(name, PlaceholderNode(fullname, s, True), s) + + old_type, should_defer = self.check_newtype_args(name, call, s) + if not call.analyzed: + call.analyzed = NewTypeExpr(name, old_type, line=call.line) if old_type is None: + if should_defer: + # Base type is not ready. + self.api.defer() return True # Create the corresponding class definition if the aliased type is subtypeable @@ -60,8 +81,15 @@ def process_newtype_declaration(self, s: AssignmentStmt) -> bool: self.msg.unimported_type_becomes_any("Argument 2 to NewType(...)", old_type, s) # If so, add it to the symbol table. - self.api.add_symbol(name, newtype_class_info, s) - call.analyzed.info = newtype_class_info + assert isinstance(call.analyzed, NewTypeExpr) + # As we do for normal classes, create the TypeInfo only once, then just + # update base classes on next iterations (to get rid of placeholders there). + if not call.analyzed.info: + call.analyzed.info = newtype_class_info + else: + call.analyzed.info.bases = newtype_class_info.bases + self.api.add_symbol(name, call.analyzed.info, s) + newtype_class_info.line = s.line return True def analyze_newtype_declaration(self, @@ -73,7 +101,6 @@ def analyze_newtype_declaration(self, and isinstance(s.rvalue, CallExpr) and isinstance(s.rvalue.callee, RefExpr) and s.rvalue.callee.fullname == 'typing.NewType'): - lvalue = s.lvalues[0] name = s.lvalues[0].name if s.type: @@ -81,7 +108,10 @@ def analyze_newtype_declaration(self, names = self.api.current_symbol_table() existing = names.get(name) - if existing and not isinstance(existing.node, (NewTypeExpr, PlaceholderNode)): + # Give a better error message that generic "Name already defined", + # like the old semantic analyzer does. + if (existing and + not isinstance(existing.node, PlaceholderNode) and not s.rvalue.analyzed): self.fail("Cannot redefine '%s' as a NewType" % name, s) # This dummy NewTypeExpr marks the call as sufficiently analyzed; it will be @@ -91,12 +121,17 @@ def analyze_newtype_declaration(self, return name, call - def check_newtype_args(self, name: str, call: CallExpr, context: Context) -> Optional[Type]: + def check_newtype_args(self, name: str, call: CallExpr, + context: Context) -> Tuple[Optional[Type], bool]: + """Ananlyze base type in NewType call. + + Return a tuple (type, should defer). + """ has_failed = False args, arg_kinds = call.args, call.arg_kinds if len(args) != 2 or arg_kinds[0] != ARG_POS or arg_kinds[1] != ARG_POS: self.fail("NewType(...) expects exactly two positional arguments", context) - return None + return None, False # Check first argument if not isinstance(args[0], (StrExpr, BytesExpr, UnicodeExpr)): @@ -113,19 +148,22 @@ def check_newtype_args(self, name: str, call: CallExpr, context: Context) -> Opt unanalyzed_type = expr_to_unanalyzed_type(args[1]) except TypeTranslationError: self.fail(msg, context) - return None + return None, False # We want to use our custom error message (see above), so we suppress # the default error message for invalid types here. old_type = self.api.anal_type(unanalyzed_type, report_invalid_types=False) + should_defer = False + if old_type is None or isinstance(old_type, PlaceholderType): + should_defer = True # The caller of this function assumes that if we return a Type, it's always # a valid one. So, we translate AnyTypes created from errors into None. if isinstance(old_type, AnyType) and old_type.is_from_error: self.fail(msg, context) - return None + return None, False - return None if has_failed else old_type + return None if has_failed else old_type, should_defer def build_newtype_typeinfo(self, name: str, old_type: Type, base_type: Instance) -> TypeInfo: info = self.api.basic_new_typeinfo(name, base_type) diff --git a/test-data/unit/check-newsemanal.test b/test-data/unit/check-newsemanal.test index df8ebb3837c24..002acc79cb31a 100644 --- a/test-data/unit/check-newsemanal.test +++ b/test-data/unit/check-newsemanal.test @@ -1766,3 +1766,61 @@ from enum import Enum A = Enum('A', ['x', 'y']) A = Enum('A', ['z', 't']) # E: Name 'A' already defined on line 3 + +[case testNewAnalyzerNewTypeRedefinition] +from typing import NewType + +A = NewType('A', int) +A = NewType('A', str) # E: Cannot redefine 'A' as a NewType \ + # E: Name 'A' already defined on line 3 + +[case testNewAnalyzerNewTypeForwardClass] +from typing import NewType, List + +x: C +reveal_type(x[0]) # E: Revealed type is '__main__.C*' + +C = NewType('C', B) + +class B(List[C]): + pass +[builtins fixtures/list.pyi] + +[case testNewAnalyzerNewTypeForwardClassAlias] +from typing import NewType, List + +x: D +reveal_type(x[0]) # E: Revealed type is '__main__.C*' + +D = C +C = NewType('C', B) + +class B(List[D]): + pass +[builtins fixtures/list.pyi] + +[case testNewAnalyzerNewTypeForwardClassAliasReversed] +from typing import NewType, List + +x: D +reveal_type(x[0][0]) # E: Revealed type is '__main__.C*' + +D = C +C = NewType('C', List[B]) + +class B(List[C]): + pass +[builtins fixtures/list.pyi] + +[case testNewAnalyzerNewTypeForwardClassAliasDirect] +from typing import NewType, List + +x: D +reveal_type(x[0][0]) # E: Revealed type is '__main__.C*' + +D = List[C] +C = NewType('C', B) + +class B(D): + pass +[builtins fixtures/list.pyi] From 2e6e8e814d76129bad592792bdc5b921973c305b Mon Sep 17 00:00:00 2001 From: Ivan Levkivskyi Date: Fri, 8 Mar 2019 13:46:20 -0800 Subject: [PATCH 12/23] Minor cleanups, self-check, and lint --- mypy/newsemanal/semanal.py | 10 ++++++++-- mypy/newsemanal/semanal_shared.py | 6 +++++- test-data/unit/check-newsemanal.test | 2 +- 3 files changed, 14 insertions(+), 4 deletions(-) diff --git a/mypy/newsemanal/semanal.py b/mypy/newsemanal/semanal.py index fb60db9a0e9ac..aa28ec18c2b2e 100644 --- a/mypy/newsemanal/semanal.py +++ b/mypy/newsemanal/semanal.py @@ -2370,9 +2370,12 @@ def check_and_set_up_type_alias(self, s: AssignmentStmt) -> bool: rvalue = s.rvalue if not self.can_be_type_alias(rvalue): return False + + res = None # type: Optional[Type] if self.is_none_alias(rvalue): res = NoneTyp() - alias_tvars, depends_on, qualified_tvars = [], set(), [] + alias_tvars, depends_on, qualified_tvars = \ + [], set(), [] # type: List[str], Set[str], List[str] else: tag = self.track_incomplete_refs() res, alias_tvars, depends_on, qualified_tvars = \ @@ -2750,6 +2753,8 @@ def process_typevar_declaration(self, s: AssignmentStmt) -> bool: # Yes, it's a valid type variable definition! Add it to the symbol table. if existing and isinstance(existing.node, TypeVarExpr): # Existing definition from previous semanal iteration, use it. + # TODO: this may be confused with a duplicate TypeVar definition. + # Fix this and add corresponding tests. type_var = existing.node type_var.values = values type_var.upper_bound = upper_bound @@ -4430,7 +4435,8 @@ def visit_placeholder_type(self, t: PlaceholderType) -> bool: return True -def has_placeholder(typ) -> bool: +def has_placeholder(typ: Type) -> bool: + """Check if a type contains any placeholder types (recursively).""" return typ.accept(HasPlaceholders()) diff --git a/mypy/newsemanal/semanal_shared.py b/mypy/newsemanal/semanal_shared.py index cb47956d90a45..75cbc1ccf0508 100644 --- a/mypy/newsemanal/semanal_shared.py +++ b/mypy/newsemanal/semanal_shared.py @@ -124,7 +124,11 @@ def add_symbol_table_node(self, name: str, stnode: SymbolTableNode) -> bool: raise NotImplementedError @abstractmethod - def current_symol_table(self) -> SymbolTable: + def current_symbol_table(self) -> SymbolTable: + """Get currently active symbol table. + + May be module, class, or local namespace. + """ raise NotImplementedError @abstractmethod diff --git a/test-data/unit/check-newsemanal.test b/test-data/unit/check-newsemanal.test index 002acc79cb31a..4e24dc7f6c393 100644 --- a/test-data/unit/check-newsemanal.test +++ b/test-data/unit/check-newsemanal.test @@ -1119,7 +1119,7 @@ class Test: def __init__(self) -> None: some_module = self.a -[case testNewAnalyzerAliasToNotReadyClass1] +[case testNewAnalyzerAliasToNotReadyClass] import a [file a.py] from b import B From b3321dc1e8b554bb6605107e5e4683c75fde8caf Mon Sep 17 00:00:00 2001 From: Ivan Levkivskyi Date: Fri, 8 Mar 2019 14:20:23 -0800 Subject: [PATCH 13/23] Re-enable previously mistakenly skipped file --- mypy/test/hacks.py | 1 - test-data/unit/check-semanal-error.test | 12 ++++++------ 2 files changed, 6 insertions(+), 7 deletions(-) diff --git a/mypy/test/hacks.py b/mypy/test/hacks.py index 64776fd7b5af2..8aa76ac717b1c 100644 --- a/mypy/test/hacks.py +++ b/mypy/test/hacks.py @@ -21,7 +21,6 @@ 'check-overloading.test', 'check-protocols.test', 'check-python2.test', - 'check-semanal-error.test', 'check-statements.test', 'check-unions.test', 'check-unreachable-code.test', diff --git a/test-data/unit/check-semanal-error.test b/test-data/unit/check-semanal-error.test index d7aa02b070b9a..3be47bade3863 100644 --- a/test-data/unit/check-semanal-error.test +++ b/test-data/unit/check-semanal-error.test @@ -49,16 +49,15 @@ A().foo(1) A().x = '' # E: Incompatible types in assignment (expression has type "str", variable has type "int") [case testInvalidBaseClass2] -# flags: --new-semantic-analyzer X = 1 class A(X): # E x = 1 A().foo(1) A().x = '' # E [out] -main:3: error: Invalid base class -main:3: error: Invalid type "__main__.X" -main:6: error: Incompatible types in assignment (expression has type "str", variable has type "int") +main:2: error: Invalid type "__main__.X" +main:2: error: Invalid base class +main:5: error: Incompatible types in assignment (expression has type "str", variable has type "int") [case testInvalidNumberOfTypeArgs] @@ -90,9 +89,10 @@ def m() -> None: ... # E: Name 'm' already defined (by an import) [out] [case testIgnoredImportDup] +# flags: --new-semantic-analyzer import m # type: ignore from m import f # type: ignore -def m() -> None: ... # ok -def f() -> None: ... # ok +def m() -> None: ... # E: Name 'm' already defined (possibly by an import) +def f() -> None: ... # E: Name 'f' already defined (possibly by an import) [out] From ca9bf7c140fa016240cd0d7a66427017fca4733f Mon Sep 17 00:00:00 2001 From: Ivan Levkivskyi Date: Fri, 8 Mar 2019 14:34:16 -0800 Subject: [PATCH 14/23] Update order of errors in one more test --- test-data/unit/check-custom-plugin.test | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/test-data/unit/check-custom-plugin.test b/test-data/unit/check-custom-plugin.test index 1b033874fc038..b810b8ba46b21 100644 --- a/test-data/unit/check-custom-plugin.test +++ b/test-data/unit/check-custom-plugin.test @@ -497,12 +497,12 @@ from mod import declarative_base, Column, Instr, non_declarative_base Bad1 = non_declarative_base() Bad2 = Bad3 = declarative_base() -class C1(Bad1): ... # E: Invalid base class \ - # E: Invalid type "__main__.Bad1" -class C2(Bad2): ... # E: Invalid base class \ - # E: Invalid type "__main__.Bad2" -class C3(Bad3): ... # E: Invalid base class \ - # E: Invalid type "__main__.Bad3" +class C1(Bad1): ... # E: Invalid type "__main__.Bad1" \ + # E: Invalid base class +class C2(Bad2): ... # E: Invalid type "__main__.Bad2" \ + # E: Invalid base class +class C3(Bad3): ... # E: Invalid type "__main__.Bad3" \ + # E: Invalid base class [file mod.py] from typing import Generic, TypeVar def declarative_base(): ... From a9b746e4d5c3c3826212f42a33613f29943f327d Mon Sep 17 00:00:00 2001 From: Ivan Levkivskyi Date: Fri, 8 Mar 2019 14:58:58 -0800 Subject: [PATCH 15/23] Whitespace --- mypy/newsemanal/semanal.py | 1 + 1 file changed, 1 insertion(+) diff --git a/mypy/newsemanal/semanal.py b/mypy/newsemanal/semanal.py index aa28ec18c2b2e..57de2217f56ce 100644 --- a/mypy/newsemanal/semanal.py +++ b/mypy/newsemanal/semanal.py @@ -942,6 +942,7 @@ def analyze_class(self, defn: ClassDef) -> None: if self.analyze_namedtuple_classdef(defn): return + # Create TypeInfo for class now that base classes and the MRO can be calculated. self.prepare_class_def(defn) From e4f4003ab4a49522ae74fe277a9fde2ef32e9ba5 Mon Sep 17 00:00:00 2001 From: Ivan Levkivskyi Date: Fri, 8 Mar 2019 15:03:56 -0800 Subject: [PATCH 16/23] Update a (way outdated) docstring --- mypy/newsemanal/semanal.py | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/mypy/newsemanal/semanal.py b/mypy/newsemanal/semanal.py index 57de2217f56ce..8e52b45ddb14d 100644 --- a/mypy/newsemanal/semanal.py +++ b/mypy/newsemanal/semanal.py @@ -2328,9 +2328,10 @@ def analyze_alias(self, rvalue: Expression, def check_and_set_up_type_alias(self, s: AssignmentStmt) -> bool: """Check if assignment creates a type alias and set it up as needed. - For simple aliases like L = List we use a simpler mechanism, just copying TypeInfo. - For subscripted (including generic) aliases the resulting types are stored - in rvalue.analyzed. + Return True if it is a type alias (even if the target is not ready), + or False otherwise. + Note: the resulting types for subscripted (including generic) aliases + are also stored in rvalue.analyzed. """ lvalue = s.lvalues[0] if len(s.lvalues) > 1 or not isinstance(lvalue, NameExpr): From 98c8da89c35e4d22592d54b13c1929c82f3d06bc Mon Sep 17 00:00:00 2001 From: Ivan Levkivskyi Date: Fri, 8 Mar 2019 16:12:01 -0800 Subject: [PATCH 17/23] Enable (and fix) newtype test file --- mypy/newsemanal/semanal_newtype.py | 8 ++++++-- mypy/test/hacks.py | 1 - test-data/unit/check-newtype.test | 13 ++++++++----- 3 files changed, 14 insertions(+), 8 deletions(-) diff --git a/mypy/newsemanal/semanal_newtype.py b/mypy/newsemanal/semanal_newtype.py index 3885628562036..96e6134e9bc92 100644 --- a/mypy/newsemanal/semanal_newtype.py +++ b/mypy/newsemanal/semanal_newtype.py @@ -6,7 +6,8 @@ from typing import Tuple, Optional from mypy.types import ( - Type, Instance, CallableType, NoneTyp, TupleType, AnyType, PlaceholderType + Type, Instance, CallableType, NoneTyp, TupleType, AnyType, PlaceholderType, + TypeOfAny ) from mypy.nodes import ( AssignmentStmt, NewTypeExpr, CallExpr, NameExpr, RefExpr, Context, StrExpr, BytesExpr, @@ -72,7 +73,10 @@ def process_newtype_declaration(self, s: AssignmentStmt) -> bool: else: message = "Argument 2 to NewType(...) must be subclassable (got {})" self.fail(message.format(self.msg.format(old_type)), s) - return True + any_typ = AnyType(TypeOfAny.from_error) + object_typ = self.api.named_type('__builtins__.object') + newtype_class_info = self.build_newtype_typeinfo(name, any_typ, object_typ) + newtype_class_info.fallback_to_any = True check_for_explicit_any(old_type, self.options, self.api.is_typeshed_stub_file, self.msg, context=s) diff --git a/mypy/test/hacks.py b/mypy/test/hacks.py index 3215a823da7bb..94f42127b454b 100644 --- a/mypy/test/hacks.py +++ b/mypy/test/hacks.py @@ -15,7 +15,6 @@ 'check-incremental.test', 'check-literal.test', 'check-modules.test', - 'check-newtype.test', 'check-overloading.test', 'check-protocols.test', 'check-python2.test', diff --git a/test-data/unit/check-newtype.test b/test-data/unit/check-newtype.test index 076821fb9b194..ea1a1c9a988a3 100644 --- a/test-data/unit/check-newtype.test +++ b/test-data/unit/check-newtype.test @@ -306,16 +306,18 @@ main:3: error: Invalid type "__main__.T" main:4: error: Invalid type "__main__.T" [case testNewTypeRedefiningVariablesFails] +# flags: --new-semantic-analyzer from typing import NewType a = 3 def f(): a -a = NewType('a', int) # E: Cannot redefine 'a' as a NewType +a = NewType('a', int) # E: Cannot redefine 'a' as a NewType \ + # E: Name 'a' already defined on line 4 b = NewType('b', int) def g(): b -b = NewType('b', float) # E: Cannot assign to a type \ - # E: Cannot redefine 'b' as a NewType +b = NewType('b', float) # E: Cannot redefine 'b' as a NewType \ + # E: Name 'b' already defined on line 8 c = NewType('c', str) # type: str # E: Cannot declare the type of a NewType declaration @@ -337,7 +339,7 @@ class C(B): pass # E: Cannot subclass NewType from typing import Protocol, NewType class P(Protocol): - attr: int + attr: int = 0 class D: attr: int @@ -364,8 +366,9 @@ issubclass(object, T) # E: Cannot use issubclass() with a NewType type [builtins fixtures/isinstancelist.pyi] [case testInvalidNewTypeCrash] +# flags: --new-semantic-analyzer from typing import List, NewType, Union N = NewType('N', XXX) # E: Argument 2 to NewType(...) must be subclassable (got "Any") \ # E: Name 'XXX' is not defined -x: List[Union[N, int]] # E: Invalid type "__main__.N" +x: List[Union[N, int]] [builtins fixtures/list.pyi] From 469ad279f9896ba395ebfc861ee862886d7579c5 Mon Sep 17 00:00:00 2001 From: Ivan Levkivskyi Date: Fri, 8 Mar 2019 18:15:38 -0800 Subject: [PATCH 18/23] Always store info if it is a NewType (there is no way back to Var) --- mypy/newsemanal/semanal_newtype.py | 8 +++++--- mypy/test/hacks.py | 2 -- test-data/unit/check-newsemanal.test | 14 ++++++++++++++ 3 files changed, 19 insertions(+), 5 deletions(-) diff --git a/mypy/newsemanal/semanal_newtype.py b/mypy/newsemanal/semanal_newtype.py index 96e6134e9bc92..bf9e7489c55bc 100644 --- a/mypy/newsemanal/semanal_newtype.py +++ b/mypy/newsemanal/semanal_newtype.py @@ -59,7 +59,7 @@ def process_newtype_declaration(self, s: AssignmentStmt) -> bool: if should_defer: # Base type is not ready. self.api.defer() - return True + return True # Create the corresponding class definition if the aliased type is subtypeable if isinstance(old_type, TupleType): @@ -71,8 +71,10 @@ def process_newtype_declaration(self, s: AssignmentStmt) -> bool: self.fail("NewType cannot be used with protocol classes", s) newtype_class_info = self.build_newtype_typeinfo(name, old_type, old_type) else: - message = "Argument 2 to NewType(...) must be subclassable (got {})" - self.fail(message.format(self.msg.format(old_type)), s) + if old_type is not None: + message = "Argument 2 to NewType(...) must be subclassable (got {})" + self.fail(message.format(self.msg.format(old_type)), s) + # Otherwise the error was already reported. any_typ = AnyType(TypeOfAny.from_error) object_typ = self.api.named_type('__builtins__.object') newtype_class_info = self.build_newtype_typeinfo(name, any_typ, object_typ) diff --git a/mypy/test/hacks.py b/mypy/test/hacks.py index 94f42127b454b..9f4d5a4c68d6a 100644 --- a/mypy/test/hacks.py +++ b/mypy/test/hacks.py @@ -8,11 +8,9 @@ # Files to not run with new semantic analyzer. new_semanal_blacklist = [ 'check-async-await.test', - 'check-classes.test', 'check-expressions.test', 'check-flags.test', 'check-functions.test', - 'check-incremental.test', 'check-literal.test', 'check-modules.test', 'check-overloading.test', diff --git a/test-data/unit/check-newsemanal.test b/test-data/unit/check-newsemanal.test index 4e24dc7f6c393..bc8c39251f129 100644 --- a/test-data/unit/check-newsemanal.test +++ b/test-data/unit/check-newsemanal.test @@ -1824,3 +1824,17 @@ C = NewType('C', B) class B(D): pass [builtins fixtures/list.pyi] + +-- Copied from check-classes.test (tricky corner case). +[case testNewAnalyzerNoCrashForwardRefToBrokenDoubleNewTypeClass] +from typing import Any, Dict, List, NewType + +Foo = NewType('NotFoo', int) # type: ignore +Foos = NewType('Foos', List[Foo]) + +x: C +class C: + def frob(self, foos: Dict[Any, Foos]) -> None: + foo = foos.get(1) + dict(foo) +[builtins fixtures/dict.pyi] From 4a80d0cf5dfce997ce0d72f29b1aac2cd12e4c5b Mon Sep 17 00:00:00 2001 From: Ivan Levkivskyi Date: Fri, 8 Mar 2019 18:32:35 -0800 Subject: [PATCH 19/23] One more tricky corner case --- mypy/newsemanal/semanal.py | 2 ++ mypy/test/hacks.py | 2 ++ test-data/unit/check-newsemanal.test | 20 +++++++++++++++++++- 3 files changed, 23 insertions(+), 1 deletion(-) diff --git a/mypy/newsemanal/semanal.py b/mypy/newsemanal/semanal.py index 8e52b45ddb14d..0e5b6a17ae970 100644 --- a/mypy/newsemanal/semanal.py +++ b/mypy/newsemanal/semanal.py @@ -2418,6 +2418,8 @@ def check_and_set_up_type_alias(self, s: AssignmentStmt) -> bool: if (isinstance(existing.node, PlaceholderNode) or isinstance(existing.node, TypeAlias) and existing.node.target != res): self.progress = True + # We need to defer so that this change can get propagated to base classes. + self.defer() existing.node = alias_node else: self.add_symbol(lvalue.name, alias_node, s) diff --git a/mypy/test/hacks.py b/mypy/test/hacks.py index 9f4d5a4c68d6a..94f42127b454b 100644 --- a/mypy/test/hacks.py +++ b/mypy/test/hacks.py @@ -8,9 +8,11 @@ # Files to not run with new semantic analyzer. new_semanal_blacklist = [ 'check-async-await.test', + 'check-classes.test', 'check-expressions.test', 'check-flags.test', 'check-functions.test', + 'check-incremental.test', 'check-literal.test', 'check-modules.test', 'check-overloading.test', diff --git a/test-data/unit/check-newsemanal.test b/test-data/unit/check-newsemanal.test index bc8c39251f129..ac98ad1e59a0e 100644 --- a/test-data/unit/check-newsemanal.test +++ b/test-data/unit/check-newsemanal.test @@ -1825,7 +1825,7 @@ class B(D): pass [builtins fixtures/list.pyi] --- Copied from check-classes.test (tricky corner case). +-- Copied from check-classes.test (tricky corner cases). [case testNewAnalyzerNoCrashForwardRefToBrokenDoubleNewTypeClass] from typing import Any, Dict, List, NewType @@ -1838,3 +1838,21 @@ class C: foo = foos.get(1) dict(foo) [builtins fixtures/dict.pyi] + +[case testNewAnalyzerForwardTypeAliasInBase] +from typing import List, Generic, TypeVar, NamedTuple +T = TypeVar('T') + +class C(A, B): + pass +class G(Generic[T]): pass +A = G[C] +class B(NamedTuple): + x: int + +y: C +reveal_type(y.x) # E: Revealed type is 'builtins.int' +reveal_type(y[0]) # E: Revealed type is 'builtins.int' +x: A +reveal_type(x) # E: Revealed type is '__main__.G[Tuple[builtins.int, fallback=__main__.C]]' +[builtins fixtures/list.pyi] From 7c25d3d08c4211372501a72dd07b5b4fcac2fa4a Mon Sep 17 00:00:00 2001 From: Ivan Levkivskyi Date: Mon, 11 Mar 2019 09:41:54 -0700 Subject: [PATCH 20/23] Fix self-check --- mypy/newsemanal/semanal_newtype.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/mypy/newsemanal/semanal_newtype.py b/mypy/newsemanal/semanal_newtype.py index bf9e7489c55bc..00b3888162d1c 100644 --- a/mypy/newsemanal/semanal_newtype.py +++ b/mypy/newsemanal/semanal_newtype.py @@ -75,9 +75,9 @@ def process_newtype_declaration(self, s: AssignmentStmt) -> bool: message = "Argument 2 to NewType(...) must be subclassable (got {})" self.fail(message.format(self.msg.format(old_type)), s) # Otherwise the error was already reported. - any_typ = AnyType(TypeOfAny.from_error) - object_typ = self.api.named_type('__builtins__.object') - newtype_class_info = self.build_newtype_typeinfo(name, any_typ, object_typ) + old_type = AnyType(TypeOfAny.from_error) + object_type = self.api.named_type('__builtins__.object') + newtype_class_info = self.build_newtype_typeinfo(name, old_type, object_type) newtype_class_info.fallback_to_any = True check_for_explicit_any(old_type, self.options, self.api.is_typeshed_stub_file, self.msg, From 9e6ad038019edb5cb4decf22aaff3d5f256703a4 Mon Sep 17 00:00:00 2001 From: Ivan Levkivskyi Date: Thu, 14 Mar 2019 17:04:04 -0700 Subject: [PATCH 21/23] Ban placeholder alias targets (they can be only nested, like in base classes) --- mypy/newsemanal/semanal.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/mypy/newsemanal/semanal.py b/mypy/newsemanal/semanal.py index 0e5b6a17ae970..7f7e23dd959e5 100644 --- a/mypy/newsemanal/semanal.py +++ b/mypy/newsemanal/semanal.py @@ -2384,7 +2384,7 @@ def check_and_set_up_type_alias(self, s: AssignmentStmt) -> bool: self.analyze_alias(rvalue, allow_placeholder=True) if not res: return False - if self.found_incomplete_ref(tag): + if self.found_incomplete_ref(tag) or isinstance(res, PlaceholderType): # Since we have got here, we know this must be a type alias (incomplete refs # may appear in nested positions), therefore use becomes_typeinfo=True. self.add_symbol(lvalue.name, PlaceholderNode(self.qualified_name(lvalue.name), From 58c88b5370bafcd7a0f241c71ab06f9aa9068e51 Mon Sep 17 00:00:00 2001 From: Ivan Levkivskyi Date: Thu, 14 Mar 2019 17:24:03 -0700 Subject: [PATCH 22/23] Address CR --- mypy/newsemanal/semanal.py | 26 +++++++++++++------------- mypy/newsemanal/semanal_enum.py | 4 ++-- mypy/newsemanal/semanal_newtype.py | 14 +++++++------- 3 files changed, 22 insertions(+), 22 deletions(-) diff --git a/mypy/newsemanal/semanal.py b/mypy/newsemanal/semanal.py index 7f7e23dd959e5..0d37c985d8d88 100644 --- a/mypy/newsemanal/semanal.py +++ b/mypy/newsemanal/semanal.py @@ -1900,7 +1900,7 @@ def is_none_alias(self, node: Expression) -> bool: def is_type_ref(self, rv: Expression, bare: bool = False) -> bool: """Does this expression refer to a type? - Thus includes: + This includes: * Special forms, like Any or Union * Classes (except subscripted enums) * Other type aliases @@ -1946,13 +1946,13 @@ def is_type_ref(self, rv: Expression, bare: bool = False) -> bool: return True return False - def should_wait(self, rv: Expression) -> bool: - """Can we already classify this r.h.s. of an assignment or should wait? + def should_wait_rhs(self, rv: Expression) -> bool: + """Can we already classify this r.h.s. of an assignment or should we wait? This returns True if we don't have enough information to decide whether an assignment is just a normal variable definition or a special form. - Always return False if this is a final iteration, this will typically cause - the assignment to be classified as variable plus emmit an error. + Always return False if this is a final iteration. This will typically cause + the lvalue to be classified as a variable plus emit an error. """ if self.final_iteration: # No chance, nothing has changed. @@ -1968,17 +1968,17 @@ def should_wait(self, rv: Expression) -> bool: if n and isinstance(n.node, PlaceholderNode) and not n.node.becomes_typeinfo: return True elif isinstance(rv, IndexExpr) and isinstance(rv.base, RefExpr): - return self.should_wait(rv.base) + return self.should_wait_rhs(rv.base) elif isinstance(rv, CallExpr) and isinstance(rv.callee, RefExpr): # This is only relevant for builtin SCC where things like 'TypeVar' # may be not ready. - return self.should_wait(rv.callee) + return self.should_wait_rhs(rv.callee) return False def can_be_type_alias(self, rv: Expression) -> bool: """Is this a valid r.h.s. for an alias definition? - Note: this function should be only called for expressions where self.should_wait() + Note: this function should be only called for expressions where self.should_wait_rhs() returns False. """ if isinstance(rv, RefExpr) and self.is_type_ref(rv, bare=True): @@ -2004,7 +2004,7 @@ def record_special_form_lvalue(self, s: AssignmentStmt) -> None: def visit_assignment_stmt(self, s: AssignmentStmt) -> None: tag = self.track_incomplete_refs() s.rvalue.accept(self) - if self.found_incomplete_ref(tag) or self.should_wait(s.rvalue): + if self.found_incomplete_ref(tag) or self.should_wait_rhs(s.rvalue): # Initializer couldn't be fully analyzed. Defer the current node and give up. # Make sure that if we skip the definition of some local names, they can't be # added later in this scope, since an earlier definition should take precedence. @@ -2757,8 +2757,8 @@ def process_typevar_declaration(self, s: AssignmentStmt) -> bool: # Yes, it's a valid type variable definition! Add it to the symbol table. if existing and isinstance(existing.node, TypeVarExpr): # Existing definition from previous semanal iteration, use it. - # TODO: this may be confused with a duplicate TypeVar definition. - # Fix this and add corresponding tests. + # TODO: This may be confused with a duplicate TypeVar definition. + # Fix this and add corresponding tests. type_var = existing.node type_var.values = values type_var.upper_bound = upper_bound @@ -4207,8 +4207,8 @@ def add_symbol_table_node(self, name: str, symbol: SymbolTableNode, and context is not None and (not isinstance(existing.node, PlaceholderNode) or isinstance(symbol.node, PlaceholderNode) and - # Allow replacing becomes_typeinfo=False with becomes_typeinfo=True, - # this can happend for type aliases and NewTypes. + # Allow replacing becomes_typeinfo=False with becomes_typeinfo=True. + # This can happen for type aliases and NewTypes. not symbol.node.becomes_typeinfo)): # There is an existing node, so this may be a redefinition. # If the new node points to the same node as the old one, diff --git a/mypy/newsemanal/semanal_enum.py b/mypy/newsemanal/semanal_enum.py index e149ff54ae16d..3a6c33ab077b3 100644 --- a/mypy/newsemanal/semanal_enum.py +++ b/mypy/newsemanal/semanal_enum.py @@ -22,8 +22,8 @@ def __init__(self, options: Options, api: SemanticAnalyzerInterface) -> None: def process_enum_call(self, s: AssignmentStmt, is_func_scope: bool) -> bool: """Check if s defines an Enum; if yes, store the definition in symbol table. - Return True if this looks like a type variable declaration (but maybe - with errors), otherwise return False. + Return True if this looks like an Enum definition (but maybe with errors), + otherwise return False. """ if len(s.lvalues) != 1 or not isinstance(s.lvalues[0], NameExpr): return False diff --git a/mypy/newsemanal/semanal_newtype.py b/mypy/newsemanal/semanal_newtype.py index 00b3888162d1c..cc295d040e304 100644 --- a/mypy/newsemanal/semanal_newtype.py +++ b/mypy/newsemanal/semanal_newtype.py @@ -33,8 +33,9 @@ def __init__(self, def process_newtype_declaration(self, s: AssignmentStmt) -> bool: """Check if s declares a NewType; if yes, store it in symbol table. - Return True if it is a valid declaration, but maybe defer until base type - is known. + Return True if it's a NewType declaration. The current target may be + deferred as a side effect if the base type is not ready, even if + the return value is True. The logic in this function mostly copies the logic for visit_class_def() with a single (non-Generic) base. @@ -42,15 +43,14 @@ def process_newtype_declaration(self, s: AssignmentStmt) -> bool: name, call = self.analyze_newtype_declaration(s) if name is None or call is None: return False - # OK, now we now this is a NewType, but the base type may be not ready yet, + # OK, now we know this is a NewType. But the base type may be not ready yet, # add placeholder as we do for ClassDef. fullname = self.api.qualified_name(name) if (not call.analyzed or isinstance(call.analyzed, NewTypeExpr) and not call.analyzed.info): - # Start from labeling this as future class with becomes_typeinfo=True, - # as we do for normal ClassDefs. - self.api.add_symbol(name, PlaceholderNode(fullname, s, True), s) + # Start from labeling this as a future class, as we do for normal ClassDefs. + self.api.add_symbol(name, PlaceholderNode(fullname, s, becomes_typeinfo=True), s) old_type, should_defer = self.check_newtype_args(name, call, s) if not call.analyzed: @@ -114,7 +114,7 @@ def analyze_newtype_declaration(self, names = self.api.current_symbol_table() existing = names.get(name) - # Give a better error message that generic "Name already defined", + # Give a better error message than generic "Name already defined", # like the old semantic analyzer does. if (existing and not isinstance(existing.node, PlaceholderNode) and not s.rvalue.analyzed): From bf330d566d5400dc62cbcfb28198d94f0d261308 Mon Sep 17 00:00:00 2001 From: Ivan Levkivskyi Date: Fri, 15 Mar 2019 14:25:06 -0700 Subject: [PATCH 23/23] Address CR --- mypy/newsemanal/semanal.py | 34 ++++++++++++++++++---------- test-data/unit/check-newsemanal.test | 6 ++--- 2 files changed, 25 insertions(+), 15 deletions(-) diff --git a/mypy/newsemanal/semanal.py b/mypy/newsemanal/semanal.py index 0d37c985d8d88..2ed7756636857 100644 --- a/mypy/newsemanal/semanal.py +++ b/mypy/newsemanal/semanal.py @@ -1909,6 +1909,11 @@ def is_type_ref(self, rv: Expression, bare: bool = False) -> bool: If bare is True, this is not a base of an index expression, so some special forms are not valid (like a bare Union). + + Note: This method should be only used in context of a type alias definition. + This method can only return True for RefExprs, to check if C[int] is a valid + target for type alias call this method on expr.base (i.e. on C in C[int]). + See also can_be_type_alias(). """ if not isinstance(rv, RefExpr): return False @@ -3297,12 +3302,7 @@ def visit_name_expr(self, expr: NameExpr) -> None: self.fail("'{}' is a type variable and only valid in type " "context".format(expr.name), expr) elif isinstance(n.node, PlaceholderNode): - if self.final_iteration: - self.fail('Cannot resolve name "{}",' - ' possible cyclic definition'.format(expr.name), - expr) - else: - self.defer() + self.process_placeholder(expr.name, 'name', expr) else: expr.kind = n.kind expr.node = n.node @@ -3515,12 +3515,7 @@ def visit_member_expr(self, expr: MemberExpr) -> None: n = self.rebind_symbol_table_node(n) if n: if isinstance(n.node, PlaceholderNode): - if self.final_iteration: - self.fail('Cannot resolve attribute "{}",' - ' possible cyclic definition'.format(expr.name), - expr) - else: - self.defer() + self.process_placeholder(expr.name, 'attribute', expr) return # TODO: What if None? expr.kind = n.kind @@ -3591,6 +3586,21 @@ def visit_member_expr(self, expr: MemberExpr) -> None: expr.fullname = n.fullname expr.node = n.node + def process_placeholder(self, name: str, kind: str, ctx: Context) -> None: + """Process a reference targeting placeholder node. + + If this is not a final iteration, defer current node, + otherwise report an error. + + The 'kind' argument indicates if this a name or attribute expression + (used for better error message). + """ + if self.final_iteration: + self.fail('Cannot resolve {} "{}" (possible cyclic definition)'.format(kind, name), + ctx) + else: + self.defer() + def visit_op_expr(self, expr: OpExpr) -> None: expr.left.accept(self) diff --git a/test-data/unit/check-newsemanal.test b/test-data/unit/check-newsemanal.test index ac98ad1e59a0e..5733fd0580ee7 100644 --- a/test-data/unit/check-newsemanal.test +++ b/test-data/unit/check-newsemanal.test @@ -339,12 +339,12 @@ def main() -> None: x # E: Name 'x' is not defined [case testNewAnalyzerCyclicDefinitions] -gx = gy # E: Cannot resolve name "gy", possible cyclic definition +gx = gy # E: Cannot resolve name "gy" (possible cyclic definition) gy = gx def main() -> None: class C: def meth(self) -> None: - lx = ly # E: Cannot resolve name "ly", possible cyclic definition + lx = ly # E: Cannot resolve name "ly" (possible cyclic definition) ly = lx [case testNewAnalyzerCyclicDefinitionCrossModule] @@ -354,7 +354,7 @@ import b x = b.x # E: Cannot determine type of 'x' [file b.py] import a -x = a.x # E: Cannot resolve attribute "x", possible cyclic definition \ +x = a.x # E: Cannot resolve attribute "x" (possible cyclic definition) \ # E: Module has no attribute "x" [builtins fixtures/module.pyi]