Skip to content

Make type inference failures more consistent #7079

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 5 commits into from
Jun 27, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
48 changes: 23 additions & 25 deletions mypy/checker.py
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@
Instance, NoneType, strip_type, TypeType, TypeOfAny,
UnionType, TypeVarId, TypeVarType, PartialType, DeletedType, UninhabitedType, TypeVarDef,
true_only, false_only, function_type, is_named_instance, union_items, TypeQuery, LiteralType,
is_optional, remove_optional
is_optional, remove_optional, TypeTranslator
)
from mypy.sametypes import is_same_type
from mypy.messages import MessageBuilder, make_inferred_type_note, append_invariance_notes
Expand Down Expand Up @@ -2520,7 +2520,7 @@ def infer_variable_type(self, name: Var, lvalue: Lvalue,
# gets generated in assignment like 'x = []' where item type is not known.
if not self.infer_partial_type(name, lvalue, init_type):
self.msg.need_annotation_for_var(name, context, self.options.python_version)
self.set_inference_error_fallback_type(name, lvalue, init_type, context)
self.set_inference_error_fallback_type(name, lvalue, init_type)
elif (isinstance(lvalue, MemberExpr) and self.inferred_attribute_types is not None
and lvalue.def_var and lvalue.def_var in self.inferred_attribute_types
and not is_same_type(self.inferred_attribute_types[lvalue.def_var], init_type)):
Expand Down Expand Up @@ -2569,20 +2569,18 @@ def set_inferred_type(self, var: Var, lvalue: Lvalue, type: Type) -> None:
self.inferred_attribute_types[lvalue.def_var] = type
self.store_type(lvalue, type)

def set_inference_error_fallback_type(self, var: Var, lvalue: Lvalue, type: Type,
context: Context) -> None:
"""If errors on context line are ignored, store dummy type for variable.
def set_inference_error_fallback_type(self, var: Var, lvalue: Lvalue, type: Type) -> None:
"""Store best known type for variable if type inference failed.

If a program ignores error on type inference error, the variable should get some
inferred type so that if can used later on in the program. Example:

x = [] # type: ignore
x.append(1) # Should be ok!

We implement this here by giving x a valid type (Any).
We implement this here by giving x a valid type (replacing inferred <nothing> with Any).
"""
if context.get_line() in self.errors.ignored_lines[self.errors.file]:
self.set_inferred_type(var, lvalue, AnyType(TypeOfAny.from_error))
self.set_inferred_type(var, lvalue, type.accept(SetNothingToAny()))

def check_simple_assignment(self, lvalue_type: Optional[Type], rvalue: Expression,
context: Context,
Expand Down Expand Up @@ -4300,26 +4298,26 @@ def is_valid_inferred_type(typ: Type) -> bool:
# specific Optional type. This resolution happens in
# leave_partial_types when we pop a partial types scope.
return False
return is_valid_inferred_type_component(typ)
return not typ.accept(NothingSeeker())


def is_valid_inferred_type_component(typ: Type) -> bool:
"""Is this part of a type a valid inferred type?
class NothingSeeker(TypeQuery[bool]):
"""Find any <nothing> types resulting from failed (ambiguous) type inference."""
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Style nit: add empty line after class docstring.


In strict Optional mode this excludes bare None types, as otherwise every
type containing None would be invalid.
"""
if is_same_type(typ, UninhabitedType()):
return False
elif isinstance(typ, Instance):
for arg in typ.args:
if not is_valid_inferred_type_component(arg):
return False
elif isinstance(typ, TupleType):
for item in typ.items:
if not is_valid_inferred_type_component(item):
return False
return True
def __init__(self) -> None:
super().__init__(any)

def visit_uninhabited_type(self, t: UninhabitedType) -> bool:
return t.ambiguous


class SetNothingToAny(TypeTranslator):
"""Replace all ambiguous <nothing> types with Any (to avoid spurious extra errors)."""

def visit_uninhabited_type(self, t: UninhabitedType) -> Type:
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Style nit: add empty line after class docstring.

if t.ambiguous:
return AnyType(TypeOfAny.from_error)
return t


def is_node_static(node: Optional[Node]) -> Optional[bool]:
Expand Down
3 changes: 2 additions & 1 deletion mypy/newsemanal/typeanal.py
Original file line number Diff line number Diff line change
Expand Up @@ -406,7 +406,8 @@ def analyze_unbound_type_without_type_info(self, t: UnboundType, sym: SymbolTabl

# TODO: Would it be better to always return Any instead of UnboundType
# in case of an error? On one hand, UnboundType has a name so error messages
# are more detailed, on the other hand, some of them may be bogus.
# are more detailed, on the other hand, some of them may be bogus,
# see https://github.com/python/mypy/issues/4987.
return t

def visit_any(self, t: AnyType) -> Type:
Expand Down
3 changes: 2 additions & 1 deletion test-data/unit/check-generics.test
Original file line number Diff line number Diff line change
Expand Up @@ -996,7 +996,8 @@ IntNode[int](1, 1)
IntNode[int](1, 'a') # E: Argument 2 to "Node" has incompatible type "str"; expected "int"

SameNode = Node[T, T]
ff = SameNode[T](1, 1) # E: Need type annotation for 'ff'
# TODO: fix https://github.com/python/mypy/issues/7084.
ff = SameNode[T](1, 1)
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think that this should generate an error, since T is not in scope here. If this seems unrelated, you can create an issue about this (and add a TODO comment here).

a = SameNode(1, 'x')
reveal_type(a) # N: Revealed type is '__main__.Node[Any, Any]'
b = SameNode[int](1, 1)
Expand Down
34 changes: 25 additions & 9 deletions test-data/unit/check-inference.test
Original file line number Diff line number Diff line change
Expand Up @@ -440,8 +440,7 @@ a = None # type: A

def ff() -> None:
x = f() # E: Need type annotation for 'x'
reveal_type(x) # N: Revealed type is 'Any' \
# E: Cannot determine type of 'x'
reveal_type(x) # N: Revealed type is 'Any'

g(None) # Ok
f() # Ok because not used to infer local variable type
Expand Down Expand Up @@ -969,9 +968,8 @@ for x in [A()]:
a = x

for y in []: # E: Need type annotation for 'y'
a = y # E: Cannot determine type of 'y'
reveal_type(y) # N: Revealed type is 'Any' \
# E: Cannot determine type of 'y'
a = y
reveal_type(y) # N: Revealed type is 'Any'

class A: pass
class B: pass
Expand Down Expand Up @@ -1017,10 +1015,8 @@ for x, y in [[A()]]:

for e, f in [[]]: # E: Need type annotation for 'e' \
# E: Need type annotation for 'f'
reveal_type(e) # N: Revealed type is 'Any' \
# E: Cannot determine type of 'e'
reveal_type(f) # N: Revealed type is 'Any' \
# E: Cannot determine type of 'f'
reveal_type(e) # N: Revealed type is 'Any'
reveal_type(f) # N: Revealed type is 'Any'

class A: pass
class B: pass
Expand Down Expand Up @@ -2724,3 +2720,23 @@ class A:
class B(A):
x = None # E: Incompatible types in assignment (expression has type "None", base class "A" defined the type as "str")
x = ''

[case testNeedAnnotationForCallable]
from typing import TypeVar, Optional, Callable

T = TypeVar('T')

def f(x: Optional[T] = None) -> Callable[..., T]: ...

x = f() # E: Need type annotation for 'x'
y = x

[case testDontNeedAnnotationForCallable]
from typing import TypeVar, Optional, Callable, NoReturn

T = TypeVar('T')

def f() -> Callable[..., NoReturn]: ...

x = f()
reveal_type(x) # N: Revealed type is 'def (*Any, **Any) -> <nothing>'
2 changes: 1 addition & 1 deletion test-data/unit/check-literal.test
Original file line number Diff line number Diff line change
Expand Up @@ -700,7 +700,7 @@ def f() -> NotAType['also' + 'not' + 'a' + 'type']: ... # E: Invalid type "__mai

# Note: this makes us re-inspect the type (e.g. via '_patch_indirect_dependencies'
# in build.py) so we can confirm the RawExpressionType did not leak out.
indirect = f() # E: Need type annotation for 'indirect'
indirect = f()
[out]

--
Expand Down
40 changes: 40 additions & 0 deletions test-data/unit/check-newsemanal.test
Original file line number Diff line number Diff line change
Expand Up @@ -2716,3 +2716,43 @@ class N(NamedTuple):
)
b
[builtins fixtures/tuple.pyi]

[case testNewAnalyzerLessErrorsNeedAnnotation]
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What about adding a test case for x = [] # type: ignore followed by reveal_type(x)?

from typing import TypeVar, Optional

T = TypeVar('T')

def f(x: Optional[T] = None) -> T: ...

x = f() # E: Need type annotation for 'x'
y = x

def g() -> None:
x = f() # E: Need type annotation for 'x'
y = x

[case testNewAnalyzerLessErrorsNeedAnnotationList]
x = [] # type: ignore
reveal_type(x) # N: Revealed type is 'builtins.list[Any]'

def g() -> None:
x = [] # type: ignore
reveal_type(x) # N: Revealed type is 'builtins.list[Any]'
[builtins fixtures/list.pyi]

[case testNewAnalyzerLessErrorsNeedAnnotationNested]
from typing import TypeVar, Optional, Generic

T = TypeVar('T')
class G(Generic[T]): ...

def f(x: Optional[T] = None) -> G[T]: ...

x = f() # E: Need type annotation for 'x'
y = x
reveal_type(y) # N: Revealed type is '__main__.G[Any]'

def g() -> None:
x = f() # E: Need type annotation for 'x'
y = x
reveal_type(y) # N: Revealed type is '__main__.G[Any]'
3 changes: 1 addition & 2 deletions test-data/unit/fine-grained-cycles.test
Original file line number Diff line number Diff line change
Expand Up @@ -205,8 +205,7 @@ def h() -> None:
[out]
==
a.py:3: error: Invalid type "b.C"
b.py:6: error: Need type annotation for 'c'
b.py:7: error: Cannot determine type of 'c'
b.py:7: error: C? has no attribute "g"
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm do we now propagate unbound types in type inference? Not sure if that's a good or bad thing.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not sure if that's a good or bad thing.

This is rather bad, but I believe the right solution is #4987.


-- TODO: More import cycle:
--
Expand Down