Skip to content

Pr/strict optional #1562

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.

Sign up for GitHub

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 31 commits into from
Jun 10, 2016
Merged

Pr/strict optional #1562

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
31 commits
Select commit Hold shift + click to select a range
3dcb752
Basic strict Optional checking
ddfisher May 4, 2016
30f3a0f
WIP
ddfisher May 12, 2016
b5e8433
cleanup
ddfisher May 19, 2016
8f8fb49
the return of uninhabitedtype
ddfisher May 19, 2016
8c0577a
fix tests WIP
ddfisher May 19, 2016
b136ef5
tests, bug fixes, partial types
ddfisher May 20, 2016
e274c13
isinstance checks
ddfisher May 20, 2016
6047860
fixup todos
ddfisher May 20, 2016
d60c93f
allow None class variable declarations
ddfisher May 20, 2016
48f9311
Merge branch 'master' into PR/strict-optional
ddfisher May 20, 2016
07cce60
Rename experimental -> experiments
ddfisher May 20, 2016
15cad44
add missing type annotations
ddfisher May 20, 2016
53ebea0
fix lint errors
ddfisher May 21, 2016
5625c75
rename UnboundType.ret_type -> is_ret_type
ddfisher May 23, 2016
157f57d
fix lack of type narrowing for member variables
ddfisher May 23, 2016
0a2c4e1
gate new isinstance checks behind experiments flag
ddfisher May 24, 2016
b6f6032
fix tests broken by member type variable narrowing
ddfisher May 24, 2016
35a6b2e
strengthen tests per Jukka's suggestions
ddfisher May 24, 2016
8b49b8f
Fixup type lattice per Reid's comments
ddfisher May 26, 2016
d9996da
Address Reid's remaining comments
ddfisher May 26, 2016
ea46191
Generalize some NoneTyp isinstance checks found by Reid
ddfisher May 26, 2016
161a2bc
Fix other crash as per Reid
ddfisher May 27, 2016
2149c11
Merge branch 'master' into PR/strict-optional
ddfisher May 27, 2016
ffd4301
Merge branch 'master' into PR/strict-optional
ddfisher Jun 6, 2016
a40636e
unnest functions
ddfisher Jun 6, 2016
6661f93
Fixup tests; add new test
ddfisher Jun 6, 2016
fbbab64
update docstrings
ddfisher Jun 6, 2016
b9e309f
fix lint errors
ddfisher Jun 6, 2016
87f46a0
add test for overloads and fix overloading
ddfisher Jun 7, 2016
7247430
minor test fixup
ddfisher Jun 10, 2016
ba743ec
Merge branch 'master' into PR/strict-optional
ddfisher Jun 10, 2016
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
165 changes: 124 additions & 41 deletions mypy/checker.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@
from mypy.types import (
Type, AnyType, CallableType, Void, FunctionLike, Overloaded, TupleType,
Instance, NoneTyp, ErrorType, strip_type,
UnionType, TypeVarType, PartialType, DeletedType
UnionType, TypeVarType, PartialType, DeletedType, UninhabitedType
)
from mypy.sametypes import is_same_type
from mypy.messages import MessageBuilder
Expand All @@ -53,6 +53,8 @@
from mypy.treetransform import TransformVisitor
from mypy.meet import meet_simple, nearest_builtin_ancestor, is_overlapping_types

from mypy import experiments


T = TypeVar('T')

Expand Down Expand Up @@ -223,14 +225,14 @@ def get_declaration(self, expr: Any) -> Type:
else:
return self.frames[0].get(expr.literal_hash)

def assign_type(self, expr: Node, type: Type,
def assign_type(self, expr: Node,
type: Type,
declared_type: Type,
restrict_any: bool = False) -> None:
Copy link
Collaborator

Choose a reason for hiding this comment

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

Add docstring (at least explain declared_type and what it's used for).

if not expr.literal:
return
self.invalidate_dependencies(expr)

declared_type = self.get_declaration(expr)

if declared_type is None:
# Not sure why this happens. It seems to mainly happen in
# member initialization.
Expand Down Expand Up @@ -1200,18 +1202,31 @@ def check_assignment(self, lvalue: Node, rvalue: Node, infer_lvalue_type: bool =
partial_types = self.find_partial_types(var)
if partial_types is not None:
if not self.current_node_deferred:
var.type = rvalue_type
if experiments.STRICT_OPTIONAL:
var.type = UnionType.make_simplified_union(
Copy link
Collaborator

Choose a reason for hiding this comment

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

The declared type of variable that gets assigned here is correct (Union[t, None] where t is the type of rvalue). However, the effective conditional type should be just t within the current block. For example, this should be valid:

x = None
if foo:
    x = 1
    print(x + 1)  # type of x is int here
# type of x is Optional[int] here

[rvalue_type, NoneTyp()])
else:
var.type = rvalue_type
else:
var.type = None
del partial_types[var]
# Try to infer a partial type. No need to check the return value, as
# an error will be reported elsewhere.
self.infer_partial_type(lvalue_type.var, lvalue, rvalue_type)
return
rvalue_type = self.check_simple_assignment(lvalue_type, rvalue, lvalue)
if (is_literal_none(rvalue) and
isinstance(lvalue, NameExpr) and
isinstance(lvalue.node, Var) and
lvalue.node.is_initialized_in_class):
# Allow None's to be assigned to class variables with non-Optional types.
Copy link
Collaborator

Choose a reason for hiding this comment

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

Worth adding a test case for multiple assignment? For example (this seems to work already):

class A:
    x, y = None, None  # type: int, str
    ... etc

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Added

rvalue_type = lvalue_type
else:
rvalue_type = self.check_simple_assignment(lvalue_type, rvalue, lvalue)

if rvalue_type and infer_lvalue_type:
self.binder.assign_type(lvalue, rvalue_type,
self.binder.assign_type(lvalue,
rvalue_type,
lvalue_type,
self.typing_mode_weak())
elif index_lvalue:
self.check_indexed_assignment(index_lvalue, rvalue, rvalue)
Expand Down Expand Up @@ -1444,7 +1459,7 @@ def infer_variable_type(self, name: Var, lvalue: Node,
"""Infer the type of initialized variables from initializer type."""
if self.typing_mode_weak():
self.set_inferred_type(name, lvalue, AnyType())
self.binder.assign_type(lvalue, init_type, True)
self.binder.assign_type(lvalue, init_type, self.binder.get_declaration(lvalue), True)
elif isinstance(init_type, Void):
self.check_not_void(init_type, context)
self.set_inference_error_fallback_type(name, lvalue, init_type, context)
Expand All @@ -1467,16 +1482,16 @@ def infer_variable_type(self, name: Var, lvalue: Node,
self.set_inferred_type(name, lvalue, init_type)

def infer_partial_type(self, name: Var, lvalue: Node, init_type: Type) -> bool:
if isinstance(init_type, NoneTyp):
partial_type = PartialType(None, name)
if isinstance(init_type, (NoneTyp, UninhabitedType)):
partial_type = PartialType(None, name, [init_type])
elif isinstance(init_type, Instance):
fullname = init_type.type.fullname()
if ((fullname == 'builtins.list' or fullname == 'builtins.set' or
fullname == 'builtins.dict')
and isinstance(init_type.args[0], NoneTyp)
and (fullname != 'builtins.dict' or isinstance(init_type.args[1], NoneTyp))
and isinstance(lvalue, NameExpr)):
partial_type = PartialType(init_type.type, name)
if (isinstance(lvalue, NameExpr) and
(fullname == 'builtins.list' or
fullname == 'builtins.set' or
fullname == 'builtins.dict') and
all(isinstance(t, (NoneTyp, UninhabitedType)) for t in init_type.args)):
partial_type = PartialType(init_type.type, name, init_type.args)
else:
return False
else:
Expand Down Expand Up @@ -1559,8 +1574,8 @@ def try_infer_partial_type_from_indexed_assignment(
self, lvalue: IndexExpr, rvalue: Node) -> None:
# TODO: Should we share some of this with try_infer_partial_type?
if isinstance(lvalue.base, RefExpr) and isinstance(lvalue.base.node, Var):
var = cast(Var, lvalue.base.node)
if var is not None and isinstance(var.type, PartialType):
var = lvalue.base.node
if isinstance(var.type, PartialType):
type_type = var.type.type
if type_type is None:
return # The partial type is None.
Expand All @@ -1572,10 +1587,15 @@ def try_infer_partial_type_from_indexed_assignment(
# TODO: Don't infer things twice.
key_type = self.accept(lvalue.index)
value_type = self.accept(rvalue)
if is_valid_inferred_type(key_type) and is_valid_inferred_type(value_type):
full_key_type = UnionType.make_simplified_union(
[key_type, var.type.inner_types[0]])
full_value_type = UnionType.make_simplified_union(
[value_type, var.type.inner_types[1]])
if (is_valid_inferred_type(full_key_type) and
is_valid_inferred_type(full_value_type)):
if not self.current_node_deferred:
var.type = self.named_generic_type('builtins.dict',
[key_type, value_type])
[full_key_type, full_value_type])
del partial_types[var]

def visit_expression_stmt(self, s: ExpressionStmt) -> Type:
Expand Down Expand Up @@ -1881,7 +1901,10 @@ def analyze_iterable_item_type(self, expr: Node) -> Type:

self.check_not_void(iterable, expr)
if isinstance(iterable, TupleType):
joined = NoneTyp() # type: Type
if experiments.STRICT_OPTIONAL:
joined = UninhabitedType() # type: Type
else:
joined = NoneTyp()
for item in iterable.items:
joined = join_types(joined, item)
if isinstance(joined, ErrorType):
Expand Down Expand Up @@ -1932,7 +1955,9 @@ def flatten(t: Node) -> List[Node]:
s.expr.accept(self)
for elt in flatten(s.expr):
if isinstance(elt, NameExpr):
self.binder.assign_type(elt, DeletedType(source=elt.name),
self.binder.assign_type(elt,
DeletedType(source=elt.name),
self.binder.get_declaration(elt),
self.typing_mode_weak())
return None

Expand Down Expand Up @@ -2311,8 +2336,12 @@ def leave_partial_types(self) -> None:
partial_types = self.partial_types.pop()
if not self.current_node_deferred:
for var, context in partial_types.items():
self.msg.fail(messages.NEED_ANNOTATION_FOR_VAR, context)
var.type = AnyType()
if experiments.STRICT_OPTIONAL and cast(PartialType, var.type).type is None:
# None partial type: assume variable is intended to have type None
var.type = NoneTyp()
else:
self.msg.fail(messages.NEED_ANNOTATION_FOR_VAR, context)
var.type = AnyType()

def find_partial_types(self, var: Var) -> Optional[Dict[Var, Context]]:
for partial_types in reversed(self.partial_types):
Expand Down Expand Up @@ -2356,11 +2385,48 @@ def method_type(self, func: FuncBase) -> FunctionLike:
return method_type_with_fallback(func, self.named_type('builtins.function'))


def conditional_type_map(expr: Node,
current_type: Optional[Type],
proposed_type: Optional[Type],
*,
weak: bool = False
) -> Tuple[Optional[Dict[Node, Type]], Optional[Dict[Node, Type]]]:
"""Takes in an expression, the current type of the expression, and a
proposed type of that expression.

Returns a 2-tuple: The first element is a map from the expression to
the proposed type, if the expression can be the proposed type. The
second element is a map from the expression to the type it would hold
if it was not the proposed type, if any."""
if proposed_type:
if current_type:
if is_proper_subtype(current_type, proposed_type):
return {expr: proposed_type}, None
elif not is_overlapping_types(current_type, proposed_type):
return None, {expr: current_type}
else:
remaining_type = restrict_subtype_away(current_type, proposed_type)
return {expr: proposed_type}, {expr: remaining_type}
else:
return {expr: proposed_type}, {}
else:
# An isinstance check, but we don't understand the type
if weak:
return {expr: AnyType()}, {expr: current_type}
else:
return {}, {}


def is_literal_none(n: Node) -> bool:
return isinstance(n, NameExpr) and n.fullname == 'builtins.None'


def find_isinstance_check(node: Node,
type_map: Dict[Node, Type],
weak: bool=False) \
-> Tuple[Optional[Dict[Node, Type]], Optional[Dict[Node, Type]]]:
"""Find any isinstance checks (within a chain of ands).
weak: bool=False
) -> Tuple[Optional[Dict[Node, Type]], Optional[Dict[Node, Type]]]:
"""Find any isinstance checks (within a chain of ands). Includes
implicit and explicit checks for None.

Return value is a map of variables to their types if the condition
is true and a map of variables to their types if the condition is false.
Expand All @@ -2376,20 +2442,31 @@ def find_isinstance_check(node: Node,
if expr.literal == LITERAL_TYPE:
vartype = type_map[expr]
type = get_isinstance_type(node.args[1], type_map)
if type:
elsetype = vartype
if vartype:
if is_proper_subtype(vartype, type):
return {expr: type}, None
elif not is_overlapping_types(vartype, type):
return None, {expr: elsetype}
else:
elsetype = restrict_subtype_away(vartype, type)
return {expr: type}, {expr: elsetype}
else:
# An isinstance check, but we don't understand the type
if weak:
return {expr: AnyType()}, {expr: vartype}
return conditional_type_map(expr, vartype, type, weak=weak)
elif (isinstance(node, ComparisonExpr) and any(is_literal_none(n) for n in node.operands) and
experiments.STRICT_OPTIONAL):
# Check for `x is None` and `x is not None`.
is_not = node.operators == ['is not']
if is_not or node.operators == ['is']:
if_vars = {} # type: Dict[Node, Type]
else_vars = {} # type: Dict[Node, Type]
for expr in node.operands:
if expr.literal == LITERAL_TYPE and not is_literal_none(expr) and expr in type_map:
# This should only be true at most once: there should be
# two elements in node.operands, and at least one of them
# should represent a None.
vartype = type_map[expr]
Copy link
Contributor

Choose a reason for hiding this comment

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

The other crash is here; I suppose you can just do nothing if type_map[expr] is not present.

if_vars, else_vars = conditional_type_map(expr, vartype, NoneTyp(), weak=weak)
break

if is_not:
if_vars, else_vars = else_vars, if_vars
return if_vars, else_vars
elif isinstance(node, RefExpr) and experiments.STRICT_OPTIONAL:
# The type could be falsy, so we can't deduce anything new about the else branch
vartype = type_map[node]
_, if_vars = conditional_type_map(node, vartype, NoneTyp(), weak=weak)
return if_vars, {}
elif isinstance(node, OpExpr) and node.op == 'and':
left_if_vars, right_else_vars = find_isinstance_check(
node.left,
Expand Down Expand Up @@ -2571,6 +2648,12 @@ def is_valid_inferred_type(typ: Type) -> bool:
Examples of invalid types include the None type or a type with a None component.
"""
if is_same_type(typ, NoneTyp()):
# With strict Optional checking, we *may* eventually infer NoneTyp, but
# we only do that if we can't infer a specific Optional type. This
# resolution happens in leave_partial_types when we pop a partial types
# scope.
return False
Copy link
Contributor

Choose a reason for hiding this comment

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

Might add a comment here about how with strict Optional checking we'll infer NoneTyp later (in leave_partial_types I think) if we can't infer a specific Optional type by then, but we don't want to commit to NoneTyp now.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Agreed -- added!

if is_same_type(typ, UninhabitedType()):
return False
elif isinstance(typ, Instance):
for arg in typ.args:
Expand Down
Loading