-
-
Notifications
You must be signed in to change notification settings - Fork 2.7k
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
Pr/strict optional #1562
Pr/strict optional #1562
Changes from 8 commits
3dcb752
30f3a0f
b5e8433
8f8fb49
8c0577a
b136ef5
e274c13
6047860
d60c93f
48f9311
07cce60
15cad44
53ebea0
5625c75
157f57d
0a2c4e1
b6f6032
35a6b2e
8b49b8f
d9996da
ea46191
161a2bc
2149c11
ffd4301
a40636e
6661f93
fbbab64
b9e309f
87f46a0
7247430
ba743ec
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -30,7 +30,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 | ||
|
@@ -51,6 +51,8 @@ | |
from mypy.treetransform import TransformVisitor | ||
from mypy.meet import meet_simple, nearest_builtin_ancestor, is_overlapping_types | ||
|
||
from mypy import experimental | ||
|
||
|
||
T = TypeVar('T') | ||
|
||
|
@@ -1171,7 +1173,10 @@ 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 experimental.STRICT_OPTIONAL: | ||
var.type = UnionType.make_simplified_union([rvalue_type, NoneTyp()]) | ||
else: | ||
var.type = rvalue_type | ||
else: | ||
var.type = None | ||
del partial_types[var] | ||
|
@@ -1438,16 +1443,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: | ||
|
@@ -1530,8 +1535,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. | ||
|
@@ -1543,10 +1548,12 @@ 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[0]]) | ||
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: | ||
|
@@ -1856,7 +1863,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 experimental.STRICT_OPTIONAL: | ||
joined = UninhabitedType() # type: Type | ||
else: | ||
joined = NoneTyp() | ||
for item in iterable.items: | ||
joined = join_types(joined, item) | ||
if isinstance(joined, ErrorType): | ||
|
@@ -2282,8 +2292,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 experimental.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): | ||
|
@@ -2331,7 +2345,8 @@ 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). | ||
"""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. | ||
|
@@ -2341,26 +2356,59 @@ def find_isinstance_check(node: Node, | |
|
||
Guaranteed to not return None, None. (But may return {}, {}) | ||
""" | ||
def split_types(current_type: Optional[Type], type_if_true: Optional[Type], expr: Node) \ | ||
-> Tuple[Optional[Dict[Node, Type]], Optional[Dict[Node, Type]]]: | ||
if type_if_true: | ||
if current_type: | ||
if is_proper_subtype(current_type, type_if_true): | ||
return {expr: type_if_true}, None | ||
elif not is_overlapping_types(current_type, type_if_true): | ||
return None, {expr: current_type} | ||
else: | ||
type_if_false = restrict_subtype_away(current_type, type_if_true) | ||
return {expr: type_if_true}, {expr: type_if_false} | ||
else: | ||
return {expr: type_if_true}, {} | ||
else: | ||
# An isinstance check, but we don't understand the type | ||
if weak: | ||
return {expr: AnyType()}, {expr: vartype} | ||
else: | ||
return {}, {} | ||
|
||
def is_none(n: Node) -> bool: | ||
return isinstance(n, NameExpr) and n.fullname == 'builtins.None' | ||
|
||
if isinstance(node, CallExpr): | ||
if refers_to_fullname(node.callee, 'builtins.isinstance'): | ||
expr = node.args[0] | ||
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 split_types(vartype, type, expr) | ||
elif (isinstance(node, ComparisonExpr) and any(is_none(n) for n in node.operands)): | ||
# 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 = {} | ||
else_vars = {} | ||
for expr in node.operands: | ||
if expr.literal == LITERAL_TYPE and not is_none(expr): | ||
# 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] | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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 |
||
if_vars, else_vars = split_types(vartype, NoneTyp(), expr) | ||
break | ||
|
||
if is_not: | ||
if_vars, else_vars = else_vars, if_vars | ||
return if_vars, else_vars | ||
elif (isinstance(node, RefExpr)): | ||
# The type could be falsy, so we can't deduce anything new about the else branch | ||
vartype = type_map[node] | ||
_, if_vars = split_types(vartype, NoneTyp(), node) | ||
return if_vars, {} | ||
elif isinstance(node, OpExpr) and node.op == 'and': | ||
left_if_vars, right_else_vars = find_isinstance_check( | ||
node.left, | ||
|
@@ -2543,6 +2591,8 @@ def is_valid_inferred_type(typ: Type) -> bool: | |
""" | ||
if is_same_type(typ, NoneTyp()): | ||
return False | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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 There was a problem hiding this comment. Choose a reason for hiding this commentThe 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: | ||
if not is_valid_inferred_type(arg): | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1 @@ | ||
STRICT_OPTIONAL = False | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I don't think an adjective make a good module name. Can you make it a noun, e.g. 'experiments'? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This function probably deserves its own docstring (it was easier to follow what was going on from context in the old version).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Agreed. Tried to give it a bit of a better name too.