From 3549c0e5e229c8e44ab2e9d541a0e5c1d8121aca Mon Sep 17 00:00:00 2001 From: Ivan Levkivskyi Date: Wed, 3 Jul 2019 08:23:09 +0100 Subject: [PATCH] Fix error reporting context for missing generic type arguments (#7100) Fixes https://github.com/python/mypy/issues/7077 The logic of the fix is quite straightforward: emit the error immediately when we fix an instance type, and not in a traversal after the main pass. Note that I fix this only for the new analyser, because the old one will be anyway deprecated soon. This causes a bit of code churn because we need to pass the flags to several functions, but the logic should not change much. I didn't add many new tests, since we have a bunch of existing tests. It would probably make sense to play with this, suggestions for more tests are very welcome. --- mypy/newsemanal/semanal.py | 6 +- mypy/newsemanal/semanal_typeargs.py | 9 +-- mypy/newsemanal/typeanal.py | 95 +++++++++++++++++----------- test-data/unit/check-flags.test | 9 +++ test-data/unit/check-newsemanal.test | 15 +++++ 5 files changed, 88 insertions(+), 46 deletions(-) diff --git a/mypy/newsemanal/semanal.py b/mypy/newsemanal/semanal.py index 772f8e838c1d..260cd5cb3dfb 100644 --- a/mypy/newsemanal/semanal.py +++ b/mypy/newsemanal/semanal.py @@ -431,7 +431,7 @@ def add_builtin_aliases(self, tree: MypyFile) -> None: target = self.named_type_or_none(target_name, []) assert target is not None # Transform List to List[Any], etc. - fix_instance_types(target, self.fail) + fix_instance_types(target, self.fail, disallow_any=False) alias_node = TypeAlias(target, alias, line=-1, column=-1, # there is no context no_args=True, normalized=True) @@ -2372,7 +2372,9 @@ def check_and_set_up_type_alias(self, s: AssignmentStmt) -> bool: # 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) + fix_instance_types(res, self.fail, + disallow_any=self.options.disallow_any_generics and + not self.is_typeshed_stub_file and not no_args) if isinstance(s.rvalue, (IndexExpr, CallExpr)): # CallExpr is for `void = type(None)` s.rvalue.analyzed = TypeAliasExpr(res, alias_tvars, no_args) s.rvalue.analyzed.line = s.line diff --git a/mypy/newsemanal/semanal_typeargs.py b/mypy/newsemanal/semanal_typeargs.py index b7f564190a15..c54c3bb6c601 100644 --- a/mypy/newsemanal/semanal_typeargs.py +++ b/mypy/newsemanal/semanal_typeargs.py @@ -8,7 +8,7 @@ from typing import List from mypy.nodes import TypeInfo, Context, MypyFile, FuncItem, ClassDef, Block -from mypy.types import Type, Instance, TypeVarType, AnyType, TypeOfAny +from mypy.types import Type, Instance, TypeVarType, AnyType from mypy.mixedtraverser import MixedTraverserVisitor from mypy.subtypes import is_subtype from mypy.sametypes import is_same_type @@ -69,13 +69,6 @@ def visit_instance(self, t: Instance) -> None: arg, info.name(), tvar.upper_bound), t) super().visit_instance(t) - def visit_any(self, t: AnyType) -> None: - if not self.options.disallow_any_generics or self.is_typeshed_file: - return - - if t.type_of_any == TypeOfAny.from_omitted_generics: - self.fail(message_registry.BARE_GENERIC, t) - def check_type_var_values(self, type: TypeInfo, actuals: List[Type], arg_name: str, valids: List[Type], arg_number: int, context: Context) -> None: for actual in actuals: diff --git a/mypy/newsemanal/typeanal.py b/mypy/newsemanal/typeanal.py index 79e3016f0a0b..61290e8ee822 100644 --- a/mypy/newsemanal/typeanal.py +++ b/mypy/newsemanal/typeanal.py @@ -207,12 +207,14 @@ def visit_unbound_type_nonoptional(self, t: UnboundType, defining_literal: bool) all_vars = node.alias_tvars target = node.target an_args = self.anal_array(t.args) - res = expand_type_alias(target, all_vars, an_args, self.fail, node.no_args, t) + disallow_any = self.options.disallow_any_generics and not self.is_typeshed_stub + res = expand_type_alias(target, all_vars, an_args, self.fail, node.no_args, t, + disallow_any=disallow_any) # The only case where expand_type_alias() can return an incorrect instance is # when it is top-level instance, so no need to recurse. if (isinstance(res, Instance) and len(res.args) != len(res.type.type_vars) and not self.defining_alias): - fix_instance(res, self.fail) + fix_instance(res, self.fail, disallow_any=disallow_any, use_generic_error=True) return res elif isinstance(node, TypeInfo): return self.analyze_type_with_type_info(node, t.args, t) @@ -254,9 +256,9 @@ def try_analyze_special_unbound_type(self, t: UnboundType, fullname: str) -> Opt return AnyType(TypeOfAny.special_form) if len(t.args) == 0 and not t.empty_tuple_index: # Bare 'Tuple' is same as 'tuple' - if self.options.disallow_any_generics and not self.is_typeshed_stub: - self.fail(message_registry.BARE_GENERIC, t) - return self.named_type('builtins.tuple', line=t.line, column=t.column) + any_type = self.get_omitted_any(t) + return self.named_type('builtins.tuple', [any_type], + line=t.line, column=t.column) if len(t.args) == 2 and isinstance(t.args[1], EllipsisType): # Tuple[T, ...] (uniform, variable-length tuple) instance = self.named_type('builtins.tuple', [self.anal_type(t.args[0])]) @@ -276,8 +278,7 @@ def try_analyze_special_unbound_type(self, t: UnboundType, fullname: str) -> Opt return self.analyze_callable_type(t) elif fullname == 'typing.Type': if len(t.args) == 0: - any_type = AnyType(TypeOfAny.from_omitted_generics, - line=t.line, column=t.column) + any_type = self.get_omitted_any(t) return TypeType(any_type, line=t.line, column=t.column) if len(t.args) != 1: self.fail('Type[...] must have exactly one type argument', t) @@ -302,6 +303,10 @@ def try_analyze_special_unbound_type(self, t: UnboundType, fullname: str) -> Opt return self.analyze_literal_type(t) return None + def get_omitted_any(self, ctx: Context, fullname: Optional[str] = None) -> AnyType: + disallow_any = not self.is_typeshed_stub and self.options.disallow_any_generics + return get_omitted_any(disallow_any, self.fail, ctx, fullname) + def analyze_type_with_type_info(self, info: TypeInfo, args: List[Type], ctx: Context) -> Type: """Bind unbound type when were able to find target TypeInfo. @@ -318,19 +323,9 @@ def analyze_type_with_type_info(self, info: TypeInfo, args: List[Type], ctx: Con instance = Instance(info, self.anal_array(args), ctx.line, ctx.column) # Check type argument count. if len(instance.args) != len(info.type_vars) and not self.defining_alias: - fix_instance(instance, self.fail) - if not args and self.options.disallow_any_generics and not self.defining_alias: - # We report/patch invalid built-in instances already during second pass. - # This is done to avoid storing additional state on instances. - # All other (including user defined) generics will be patched/reported - # in the third pass. - if not self.is_typeshed_stub and info.fullname() in nongen_builtins: - alternative = nongen_builtins[info.fullname()] - self.fail(message_registry.IMPLICIT_GENERIC_ANY_BUILTIN.format(alternative), ctx) - any_type = AnyType(TypeOfAny.from_error, line=ctx.line) - else: - any_type = AnyType(TypeOfAny.from_omitted_generics, line=ctx.line) - instance.args = [any_type] * len(info.type_vars) + fix_instance(instance, self.fail, + disallow_any=self.options.disallow_any_generics and + not self.is_typeshed_stub) tup = info.tuple_type if tup is not None: @@ -557,8 +552,7 @@ def analyze_callable_type(self, t: UnboundType) -> Type: fallback = self.named_type('builtins.function') if len(t.args) == 0: # Callable (bare). Treat as Callable[..., Any]. - any_type = AnyType(TypeOfAny.from_omitted_generics, - line=t.line, column=t.column) + any_type = self.get_omitted_any(t) ret = CallableType([any_type, any_type], [nodes.ARG_STAR, nodes.ARG_STAR2], [None, None], @@ -846,14 +840,33 @@ def tuple_type(self, items: List[Type]) -> TupleType: TypeVarList = List[Tuple[str, TypeVarExpr]] -def fix_instance(t: Instance, fail: Callable[[str, Context], None]) -> None: +def get_omitted_any(disallow_any: bool, fail: Callable[[str, Context], None], + ctx: Context, fullname: Optional[str] = None) -> AnyType: + if disallow_any: + if fullname in nongen_builtins: + # We use a dedicated error message for builtin generics (as the most common case). + alternative = nongen_builtins[fullname] + fail(message_registry.IMPLICIT_GENERIC_ANY_BUILTIN.format(alternative), ctx) + else: + fail(message_registry.BARE_GENERIC, ctx) + any_type = AnyType(TypeOfAny.from_error, line=ctx.line, column=ctx.column) + else: + any_type = AnyType(TypeOfAny.from_omitted_generics, line=ctx.line, column=ctx.column) + return any_type + + +def fix_instance(t: Instance, fail: Callable[[str, Context], None], + disallow_any: bool, use_generic_error: bool = False) -> None: """Fix a malformed instance by replacing all type arguments with Any. Also emit a suitable error if this is not due to implicit Any's. """ if len(t.args) == 0: - any_type = AnyType(TypeOfAny.from_omitted_generics, - line=t.line, column=t.column) + if use_generic_error: + fullname = None # type: Optional[str] + else: + fullname = t.type.fullname() + any_type = get_omitted_any(disallow_any, fail, t, fullname) t.args = [any_type] * len(t.type.type_vars) return # Invalid number of type parameters. @@ -876,7 +889,8 @@ def fix_instance(t: Instance, fail: Callable[[str, Context], None]) -> None: def expand_type_alias(target: Type, alias_tvars: List[str], args: List[Type], - fail: Callable[[str, Context], None], no_args: bool, ctx: Context) -> Type: + fail: Callable[[str, Context], None], no_args: bool, ctx: Context, *, + disallow_any: bool = False) -> Type: """Expand a (generic) type alias target following the rules outlined in TypeAlias docstring. Here: @@ -892,7 +906,8 @@ def expand_type_alias(target: Type, alias_tvars: List[str], args: List[Type], if exp_len > 0 and act_len == 0: # Interpret bare Alias same as normal generic, i.e., Alias[Any, Any, ...] assert alias_tvars is not None - return set_any_tvars(target, alias_tvars, ctx.line, ctx.column) + return set_any_tvars(target, alias_tvars, ctx.line, ctx.column, + disallow_any=disallow_any, fail=fail) if exp_len == 0 and act_len == 0: if no_args: assert isinstance(target, Instance) @@ -907,7 +922,7 @@ def expand_type_alias(target: Type, alias_tvars: List[str], args: List[Type], fail('Bad number of arguments for type alias, expected: %s, given: %s' % (exp_len, act_len), ctx) return set_any_tvars(target, alias_tvars or [], - ctx.line, ctx.column, implicit=False) + ctx.line, ctx.column, from_error=True) typ = replace_alias_tvars(target, alias_tvars, args, ctx.line, ctx.column) # HACK: Implement FlexibleAlias[T, typ] by expanding it to typ here. if (isinstance(typ, Instance) @@ -938,11 +953,17 @@ def replace_alias_tvars(tp: Type, vars: List[str], subs: List[Type], def set_any_tvars(tp: Type, vars: List[str], - newline: int, newcolumn: int, implicit: bool = True) -> Type: - if implicit: - type_of_any = TypeOfAny.from_omitted_generics + newline: int, newcolumn: int, *, + from_error: bool = False, + disallow_any: bool = False, + fail: Optional[Callable[[str, Context], None]] = None) -> Type: + if from_error or disallow_any: + type_of_any = TypeOfAny.from_error else: - type_of_any = TypeOfAny.special_form + type_of_any = TypeOfAny.from_omitted_generics + if disallow_any: + assert fail is not None + fail(message_registry.BARE_GENERIC, Context(newline, newcolumn)) any_type = AnyType(type_of_any, line=newline, column=newcolumn) return replace_alias_tvars(tp, vars, [any_type] * len(vars), newline, newcolumn) @@ -1112,20 +1133,22 @@ def make_optional_type(t: Type) -> Type: return UnionType([t, NoneType()], t.line, t.column) -def fix_instance_types(t: Type, fail: Callable[[str, Context], None]) -> None: +def fix_instance_types(t: Type, fail: Callable[[str, Context], None], + disallow_any: bool) -> None: """Recursively fix all instance types (type argument count) in a given type. For example 'Union[Dict, List[str, int]]' will be transformed into 'Union[Dict[Any, Any], List[Any]]' in place. """ - t.accept(InstanceFixer(fail)) + t.accept(InstanceFixer(fail, disallow_any)) class InstanceFixer(TypeTraverserVisitor): - def __init__(self, fail: Callable[[str, Context], None]) -> None: + def __init__(self, fail: Callable[[str, Context], None], disallow_any: bool) -> None: self.fail = fail + self.disallow_any = disallow_any def visit_instance(self, typ: Instance) -> None: super().visit_instance(typ) if len(typ.args) != len(typ.type.type_vars): - fix_instance(typ, self.fail) + fix_instance(typ, self.fail, self.disallow_any, use_generic_error=True) diff --git a/test-data/unit/check-flags.test b/test-data/unit/check-flags.test index 6c9c6e1d3daa..d88da3940ee8 100644 --- a/test-data/unit/check-flags.test +++ b/test-data/unit/check-flags.test @@ -1162,3 +1162,12 @@ implicit_reexport = True implicit_reexport = False [out] main:2: error: Module 'other_module_2' has no attribute 'a' + +[case testImplicitAnyOKForNoArgs] +# flags: --disallow-any-generics --show-column-numbers +from typing import List + +A = List # OK +B = List[A] # E:10: Missing type parameters for generic type +x: A # E:4: Missing type parameters for generic type +[builtins fixtures/list.pyi] diff --git a/test-data/unit/check-newsemanal.test b/test-data/unit/check-newsemanal.test index 3c5cc74f821a..5314210706a3 100644 --- a/test-data/unit/check-newsemanal.test +++ b/test-data/unit/check-newsemanal.test @@ -2813,6 +2813,21 @@ S = TypeVar('S', covariant=True, contravariant=True) # E: TypeVar cannot be bot class Yes: ... [builtins fixtures/bool.pyi] +[case testNewAnalyzerDisallowAnyGenericsMessages] +# mypy: disallow-any-generics +from a import B +x: B + +[file a.py] +from typing import TypeVar, List + +T = TypeVar('T') + +A = List[T] +B = A + +[builtins fixtures/list.pyi] + [case testNewAnalyzerVarTypeVarNoCrash] from typing import Callable, TypeVar