Skip to content

Commit

Permalink
Fix error reporting context for missing generic type arguments (#7100)
Browse files Browse the repository at this point in the history
Fixes #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.
  • Loading branch information
ilevkivskyi authored Jul 3, 2019
1 parent b279f4c commit 3549c0e
Show file tree
Hide file tree
Showing 5 changed files with 88 additions and 46 deletions.
6 changes: 4 additions & 2 deletions mypy/newsemanal/semanal.py
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down Expand Up @@ -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
Expand Down
9 changes: 1 addition & 8 deletions mypy/newsemanal/semanal_typeargs.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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:
Expand Down
95 changes: 59 additions & 36 deletions mypy/newsemanal/typeanal.py
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down Expand Up @@ -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])])
Expand All @@ -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)
Expand All @@ -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.
Expand All @@ -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:
Expand Down Expand Up @@ -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],
Expand Down Expand Up @@ -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.
Expand All @@ -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:
Expand All @@ -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)
Expand All @@ -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)
Expand Down Expand Up @@ -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)

Expand Down Expand Up @@ -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)
9 changes: 9 additions & 0 deletions test-data/unit/check-flags.test
Original file line number Diff line number Diff line change
Expand Up @@ -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]
15 changes: 15 additions & 0 deletions test-data/unit/check-newsemanal.test
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down

0 comments on commit 3549c0e

Please sign in to comment.