From d5561e2a7a7bdaab91b6046607fa9da36387e318 Mon Sep 17 00:00:00 2001 From: Ivan Levkivskyi Date: Mon, 24 Sep 2018 00:43:17 +0100 Subject: [PATCH 1/4] Allow returning inferred None from functions --- mypy/checkexpr.py | 8 +-- mypy/typeanal.py | 2 +- mypy/types.py | 12 +++-- test-data/unit/check-dataclasses.test | 24 +++++++++ test-data/unit/check-functions.test | 58 +++++++++++++++++++++ test-data/unit/check-inference-context.test | 3 +- test-data/unit/check-optional.test | 3 +- test-data/unit/check-overloading.test | 3 +- test-data/unit/check-protocols.test | 4 +- 9 files changed, 104 insertions(+), 13 deletions(-) diff --git a/mypy/checkexpr.py b/mypy/checkexpr.py index c7cb08ffd6eb2..56222c61dee28 100644 --- a/mypy/checkexpr.py +++ b/mypy/checkexpr.py @@ -329,7 +329,8 @@ def visit_call_expr_inner(self, e: CallExpr, allow_none_return: bool = False) -> self.check_protocol_issubclass(e) if isinstance(ret_type, UninhabitedType) and not ret_type.ambiguous: self.chk.binder.unreachable() - if not allow_none_return and isinstance(ret_type, NoneTyp): + if (not allow_none_return and isinstance(ret_type, NoneTyp) and + ret_type.explicit and not isinstance(callee_type, Overloaded)): self.chk.msg.does_not_return_value(callee_type, e) return AnyType(TypeOfAny.from_error) return ret_type @@ -3171,9 +3172,10 @@ def visit_yield_from_expr(self, e: YieldFromExpr, allow_none_return: bool = Fals if isinstance(actual_item_type, AnyType): expr_type = AnyType(TypeOfAny.from_another_any, source_any=actual_item_type) else: - expr_type = NoneTyp() + # Treat `Iterator[X]` as a shorthand for `Generator[X, None, Any]`. + expr_type = NoneTyp(explicit=True) - if not allow_none_return and isinstance(expr_type, NoneTyp): + if not allow_none_return and isinstance(expr_type, NoneTyp) and expr_type.explicit: self.chk.msg.does_not_return_value(None, e) return expr_type diff --git a/mypy/typeanal.py b/mypy/typeanal.py index 802287d548609..145660ce88c16 100644 --- a/mypy/typeanal.py +++ b/mypy/typeanal.py @@ -228,7 +228,7 @@ def visit_unbound_type_nonoptional(self, t: UnboundType) -> Type: t.name), t) return TypeVarType(tvar_def, t.line) elif fullname == 'builtins.None': - return NoneTyp() + return NoneTyp(explicit=True) elif fullname == 'typing.Any' or fullname == 'builtins.Any': return AnyType(TypeOfAny.explicit) elif fullname in ('typing.Final', 'typing_extensions.Final'): diff --git a/mypy/types.py b/mypy/types.py index acad55a12d759..9ec1000421aba 100644 --- a/mypy/types.py +++ b/mypy/types.py @@ -436,10 +436,13 @@ class NoneTyp(Type): This type can be written by users as 'None'. """ - __slots__ = () + __slots__ = ('explicit',) - def __init__(self, line: int = -1, column: int = -1) -> None: + def __init__(self, explicit: bool = False, line: int = -1, column: int = -1) -> None: super().__init__(line, column) + # Does this `None` appears in an explicit type annotation? + # For all inferred `None` types this must be `False`. + self.explicit = explicit def can_be_true_default(self) -> bool: return False @@ -454,12 +457,13 @@ def accept(self, visitor: 'TypeVisitor[T]') -> T: return visitor.visit_none_type(self) def serialize(self) -> JsonDict: - return {'.class': 'NoneTyp'} + return {'.class': 'NoneTyp', + 'explicit': self.explicit} @classmethod def deserialize(cls, data: JsonDict) -> 'NoneTyp': assert data['.class'] == 'NoneTyp' - return NoneTyp() + return NoneTyp(data['explicit']) class ErasedType(Type): diff --git a/test-data/unit/check-dataclasses.test b/test-data/unit/check-dataclasses.test index 8d1d9803b705e..cb28ff995f122 100644 --- a/test-data/unit/check-dataclasses.test +++ b/test-data/unit/check-dataclasses.test @@ -500,3 +500,27 @@ class A: reveal_type(cls()) # E: Revealed type is 'T`-1' return cls() [builtins fixtures/classmethod.pyi] + +[case testNoComplainFieldNone] +# flags: --python-version 3.6 +# flags: --no-strict-optional +from dataclasses import dataclass, field +from typing import Optional + +@dataclass +class Foo: + bar: Optional[int] = field(default=None) +[builtins fixtures/list.pyi] +[out] + +[case testNoComplainFieldNoneStrict] +# flags: --python-version 3.6 +# flags: --strict-optional +from dataclasses import dataclass, field +from typing import Optional + +@dataclass +class Foo: + bar: Optional[int] = field(default=None) +[builtins fixtures/list.pyi] +[out] diff --git a/test-data/unit/check-functions.test b/test-data/unit/check-functions.test index 64936e30623bc..fa4e7c98f9345 100644 --- a/test-data/unit/check-functions.test +++ b/test-data/unit/check-functions.test @@ -2319,3 +2319,61 @@ main:6: error: b: builtins.int main:6: error: c: builtins.int main:9: error: Revealed local types are: main:9: error: a: builtins.float + +[case testNoComplainOverloadNone] +# flags: --no-strict-optional +from typing import overload, Optional +@overload +def bar(x: None) -> None: + ... +@overload +def bar(x: int) -> str: + ... +def bar(x: Optional[int]) -> Optional[str]: + if x is None: + return None + return "number" + +reveal_type(bar(None)) # E: Revealed type is 'None' +[builtins fixtures/isinstance.pyi] +[out] + +[case testNoComplainOverloadNoneStrict] +# flags: --strict-optional +from typing import overload, Optional +@overload +def bar(x: None) -> None: + ... +@overload +def bar(x: int) -> str: + ... +def bar(x: Optional[int]) -> Optional[str]: + if x is None: + return None + return "number" + +reveal_type(bar(None)) # E: Revealed type is 'None' +[builtins fixtures/isinstance.pyi] +[out] + +[case testNoComplainInferredNone] +# flags: --no-strict-optional +from typing import TypeVar, Optional +T = TypeVar('T') +def X(val: T) -> T: ... +x_in = None +def Y(x: Optional[str] = X(x_in)): ... + +xx: Optional[int] = X(x_in) +[out] + +[case testNoComplainInferredNoneStrict] +# flags: --strict-optional +from typing import TypeVar, Optional +T = TypeVar('T') +def X(val: T) -> T: ... +x_in = None +def Y(x: Optional[str] = X(x_in)): ... + +xx: Optional[int] = X(x_in) +[out] diff --git a/test-data/unit/check-inference-context.test b/test-data/unit/check-inference-context.test index ffa203f51ed2c..7b9495f62d2f7 100644 --- a/test-data/unit/check-inference-context.test +++ b/test-data/unit/check-inference-context.test @@ -887,6 +887,7 @@ reveal_type(c.f(None)) # E: Revealed type is 'builtins.list[builtins.int]' [builtins fixtures/list.pyi] [case testGenericMethodCalledInGenericContext] +# flags: --strict-optional from typing import TypeVar, Generic _KT = TypeVar('_KT') @@ -897,7 +898,7 @@ class M(Generic[_KT, _VT]): def get(self, k: _KT, default: _T) -> _T: ... def f(d: M[_KT, _VT], k: _KT) -> _VT: - return d.get(k, None) # E: "get" of "M" does not return a value + return d.get(k, None) # E: Incompatible return value type (got "None", expected "_VT") [case testGenericMethodCalledInGenericContext2] from typing import TypeVar, Generic, Union diff --git a/test-data/unit/check-optional.test b/test-data/unit/check-optional.test index 955dd1cfe3191..c189c06c0746c 100644 --- a/test-data/unit/check-optional.test +++ b/test-data/unit/check-optional.test @@ -119,7 +119,8 @@ reveal_type(z2) # E: Revealed type is 'Union[builtins.int, builtins.str, None]' [case testLambdaReturningNone] f = lambda: None -x = f() # E: Function does not return a value +x = f() +reveal_type(x) # E: Revealed type is 'None' [case testNoneArgumentType] def f(x: None) -> None: pass diff --git a/test-data/unit/check-overloading.test b/test-data/unit/check-overloading.test index 21f1b16329d63..dbd097c1d156a 100644 --- a/test-data/unit/check-overloading.test +++ b/test-data/unit/check-overloading.test @@ -1211,7 +1211,8 @@ def g(x: U, y: V) -> None: # N: Possible overload variants: \ # N: def [T <: str] f(x: T) -> T \ # N: def [T <: str] f(x: List[T]) -> None - a = f([x]) # E: "f" does not return a value + a = f([x]) + reveal_type(a) # E: Revealed type is 'None' f([y]) # E: Value of type variable "T" of "f" cannot be "V" f([x, y]) # E: Value of type variable "T" of "f" cannot be "object" [builtins fixtures/list.pyi] diff --git a/test-data/unit/check-protocols.test b/test-data/unit/check-protocols.test index 689ec46a83cb5..030804ed719b5 100644 --- a/test-data/unit/check-protocols.test +++ b/test-data/unit/check-protocols.test @@ -1238,8 +1238,8 @@ class P2(Protocol): T = TypeVar('T') def f(x: Callable[[T, T], None]) -> T: pass def g(x: P, y: P2) -> None: pass -x = f(g) # E: "f" does not return a value - +x = f(g) +reveal_type(x) # E: Revealed type is 'None' [case testMeetProtocolWithNormal] from typing import Protocol, Callable, TypeVar From edd86bcf1ca89eab7cb2a8941859c3a6d9f80d63 Mon Sep 17 00:00:00 2001 From: Ivan Levkivskyi Date: Mon, 24 Sep 2018 16:55:59 +0100 Subject: [PATCH 2/4] Don't use a new flag --- mypy/checkexpr.py | 21 +++++++++++++++++---- mypy/semanal.py | 10 +++++----- mypy/typeanal.py | 2 +- mypy/types.py | 12 ++++-------- test-data/unit/check-bound.test | 6 ++++-- test-data/unit/check-expressions.test | 2 +- test-data/unit/check-functions.test | 3 +-- 7 files changed, 33 insertions(+), 23 deletions(-) diff --git a/mypy/checkexpr.py b/mypy/checkexpr.py index 56222c61dee28..b980ab38f5507 100644 --- a/mypy/checkexpr.py +++ b/mypy/checkexpr.py @@ -329,12 +329,25 @@ def visit_call_expr_inner(self, e: CallExpr, allow_none_return: bool = False) -> self.check_protocol_issubclass(e) if isinstance(ret_type, UninhabitedType) and not ret_type.ambiguous: self.chk.binder.unreachable() - if (not allow_none_return and isinstance(ret_type, NoneTyp) and - ret_type.explicit and not isinstance(callee_type, Overloaded)): + if (not allow_none_return and + isinstance(e.callee, RefExpr) and self.always_returns_none(e.callee)): self.chk.msg.does_not_return_value(callee_type, e) return AnyType(TypeOfAny.from_error) return ret_type + def always_returns_none(self, node: RefExpr) -> bool: + """Check if `node` refers to something explicitly annotated as _only_ returning None.""" + if isinstance(node.node, FuncDef): + return (isinstance(node.node.type, CallableType) and + isinstance(node.node.type.ret_type, NoneTyp)) + if isinstance(node.node, OverloadedFuncDef): + return all(isinstance(item.type, CallableType) and + isinstance(item.type.ret_type, NoneTyp) for item in node.node.items) + if isinstance(node.node, Var): + return (not node.node.is_inferred and isinstance(node.node.type, CallableType) and + isinstance(node.node.type.ret_type, NoneTyp)) + return False + def check_runtime_protocol_test(self, e: CallExpr) -> None: for expr in mypy.checker.flatten(e.args[1]): tp = self.chk.type_map[expr] @@ -3173,9 +3186,9 @@ def visit_yield_from_expr(self, e: YieldFromExpr, allow_none_return: bool = Fals expr_type = AnyType(TypeOfAny.from_another_any, source_any=actual_item_type) else: # Treat `Iterator[X]` as a shorthand for `Generator[X, None, Any]`. - expr_type = NoneTyp(explicit=True) + expr_type = NoneTyp() - if not allow_none_return and isinstance(expr_type, NoneTyp) and expr_type.explicit: + if not allow_none_return and isinstance(expr_type, NoneTyp): self.chk.msg.does_not_return_value(None, e) return expr_type diff --git a/mypy/semanal.py b/mypy/semanal.py index 09b1fc22bb748..a4174de03742b 100644 --- a/mypy/semanal.py +++ b/mypy/semanal.py @@ -2017,7 +2017,7 @@ def analyze_name_lvalue(self, not self.type) if (add_global or nested_global) and lval.name not in self.globals: # Define new global name. - v = self.make_name_lvalue_var(lval, GDEF) + v = self.make_name_lvalue_var(lval, GDEF, not explicit_type) self.globals[lval.name] = SymbolTableNode(GDEF, v) elif isinstance(lval.node, Var) and lval.is_new_def: if lval.kind == GDEF: @@ -2029,7 +2029,7 @@ def analyze_name_lvalue(self, lval.name not in self.global_decls[-1] and lval.name not in self.nonlocal_decls[-1]): # Define new local name. - v = self.make_name_lvalue_var(lval, LDEF) + v = self.make_name_lvalue_var(lval, LDEF, not explicit_type) self.add_local(v, lval) if lval.name == '_': # Special case for assignment to local named '_': always infer 'Any'. @@ -2038,16 +2038,16 @@ def analyze_name_lvalue(self, elif not self.is_func_scope() and (self.type and lval.name not in self.type.names): # Define a new attribute within class body. - v = self.make_name_lvalue_var(lval, MDEF) - v.is_inferred = not explicit_type + v = self.make_name_lvalue_var(lval, MDEF, not explicit_type) self.type.names[lval.name] = SymbolTableNode(MDEF, v) else: self.make_name_lvalue_point_to_existing_def(lval, explicit_type, final_cb) - def make_name_lvalue_var(self, lvalue: NameExpr, kind: int) -> Var: + def make_name_lvalue_var(self, lvalue: NameExpr, kind: int, inferred: bool) -> Var: """Return a Var node for an lvalue that is a name expression.""" v = Var(lvalue.name) v.set_line(lvalue) + v.is_inferred = inferred if kind == MDEF: assert self.type is not None v.info = self.type diff --git a/mypy/typeanal.py b/mypy/typeanal.py index 145660ce88c16..802287d548609 100644 --- a/mypy/typeanal.py +++ b/mypy/typeanal.py @@ -228,7 +228,7 @@ def visit_unbound_type_nonoptional(self, t: UnboundType) -> Type: t.name), t) return TypeVarType(tvar_def, t.line) elif fullname == 'builtins.None': - return NoneTyp(explicit=True) + return NoneTyp() elif fullname == 'typing.Any' or fullname == 'builtins.Any': return AnyType(TypeOfAny.explicit) elif fullname in ('typing.Final', 'typing_extensions.Final'): diff --git a/mypy/types.py b/mypy/types.py index 9ec1000421aba..acad55a12d759 100644 --- a/mypy/types.py +++ b/mypy/types.py @@ -436,13 +436,10 @@ class NoneTyp(Type): This type can be written by users as 'None'. """ - __slots__ = ('explicit',) + __slots__ = () - def __init__(self, explicit: bool = False, line: int = -1, column: int = -1) -> None: + def __init__(self, line: int = -1, column: int = -1) -> None: super().__init__(line, column) - # Does this `None` appears in an explicit type annotation? - # For all inferred `None` types this must be `False`. - self.explicit = explicit def can_be_true_default(self) -> bool: return False @@ -457,13 +454,12 @@ def accept(self, visitor: 'TypeVisitor[T]') -> T: return visitor.visit_none_type(self) def serialize(self) -> JsonDict: - return {'.class': 'NoneTyp', - 'explicit': self.explicit} + return {'.class': 'NoneTyp'} @classmethod def deserialize(cls, data: JsonDict) -> 'NoneTyp': assert data['.class'] == 'NoneTyp' - return NoneTyp(data['explicit']) + return NoneTyp() class ErasedType(Type): diff --git a/test-data/unit/check-bound.test b/test-data/unit/check-bound.test index 043148f6423d7..e49075d455690 100644 --- a/test-data/unit/check-bound.test +++ b/test-data/unit/check-bound.test @@ -56,7 +56,8 @@ class C(Generic[T]): return self.t c1 = None # type: C[None] c1.get() -d = c1.get() # E: "get" of "C" does not return a value +d = c1.get() +reveal_type(d) # E: Revealed type is 'None' [case testBoundAny] @@ -82,7 +83,8 @@ def f(g: Callable[[], T]) -> T: return g() def h() -> None: pass f(h) -a = f(h) # E: "f" does not return a value +a = f(h) +reveal_type(a) # E: Revealed type is 'None' [case testBoundInheritance] diff --git a/test-data/unit/check-expressions.test b/test-data/unit/check-expressions.test index fd2bc496deb1c..da12bc74c67da 100644 --- a/test-data/unit/check-expressions.test +++ b/test-data/unit/check-expressions.test @@ -936,7 +936,7 @@ main:3: error: "A" not callable a, o = None, None # type: (A, object) a = f() # E: "f" does not return a value -o = a() # E: Function does not return a value +o = a() o = A().g(a) # E: "g" of "A" does not return a value A().g(f()) # E: "f" does not return a value x: A = f() # E: "f" does not return a value diff --git a/test-data/unit/check-functions.test b/test-data/unit/check-functions.test index fa4e7c98f9345..1ace4b249fe8f 100644 --- a/test-data/unit/check-functions.test +++ b/test-data/unit/check-functions.test @@ -2246,8 +2246,7 @@ def h() -> Dict[Union[str, int], str]: def i() -> List[Union[int, float]]: x: List[int] = [1] - return x # E: Incompatible return value type (got "List[int]", expected "List[Union[int, float]]") \ -# N: Perhaps you need a type annotation for "x"? Suggestion: "List[Union[int, float]]" + return x # E: Incompatible return value type (got "List[int]", expected "List[Union[int, float]]") [builtins fixtures/dict.pyi] From 2dabcc6dabbb050dabc82bf1eb32f3e2edc1edab Mon Sep 17 00:00:00 2001 From: Ivan Levkivskyi Date: Mon, 24 Sep 2018 17:25:57 +0100 Subject: [PATCH 3/4] Update one more test --- test-data/unit/check-expressions.test | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test-data/unit/check-expressions.test b/test-data/unit/check-expressions.test index da12bc74c67da..fa1ccbf5ae76f 100644 --- a/test-data/unit/check-expressions.test +++ b/test-data/unit/check-expressions.test @@ -937,7 +937,7 @@ main:3: error: "A" not callable a, o = None, None # type: (A, object) a = f() # E: "f" does not return a value o = a() -o = A().g(a) # E: "g" of "A" does not return a value +o = A().g(a) A().g(f()) # E: "f" does not return a value x: A = f() # E: "f" does not return a value f() From b008f6a0c73239b607ef5d82f2ffa276ad57a207 Mon Sep 17 00:00:00 2001 From: Ivan Levkivskyi Date: Tue, 25 Sep 2018 16:39:02 +0100 Subject: [PATCH 4/4] Add some more special-casing --- mypy/checkexpr.py | 49 ++++++++++++++++++++------- test-data/unit/check-expressions.test | 5 +-- 2 files changed, 39 insertions(+), 15 deletions(-) diff --git a/mypy/checkexpr.py b/mypy/checkexpr.py index b980ab38f5507..8d57e42258a8f 100644 --- a/mypy/checkexpr.py +++ b/mypy/checkexpr.py @@ -31,7 +31,7 @@ ConditionalExpr, ComparisonExpr, TempNode, SetComprehension, DictionaryComprehension, ComplexExpr, EllipsisExpr, StarExpr, AwaitExpr, YieldExpr, YieldFromExpr, TypedDictExpr, PromoteExpr, NewTypeExpr, NamedTupleExpr, TypeVarExpr, - TypeAliasExpr, BackquoteExpr, EnumCallExpr, TypeAlias, ClassDef, Block, SymbolTable, + TypeAliasExpr, BackquoteExpr, EnumCallExpr, TypeAlias, ClassDef, Block, SymbolNode, ARG_POS, ARG_OPT, ARG_NAMED, ARG_STAR, ARG_STAR2, MODULE_REF, LITERAL_TYPE, REVEAL_TYPE ) from mypy.literals import literal @@ -329,23 +329,46 @@ def visit_call_expr_inner(self, e: CallExpr, allow_none_return: bool = False) -> self.check_protocol_issubclass(e) if isinstance(ret_type, UninhabitedType) and not ret_type.ambiguous: self.chk.binder.unreachable() - if (not allow_none_return and - isinstance(e.callee, RefExpr) and self.always_returns_none(e.callee)): + if not allow_none_return and self.always_returns_none(e.callee): self.chk.msg.does_not_return_value(callee_type, e) return AnyType(TypeOfAny.from_error) return ret_type - def always_returns_none(self, node: RefExpr) -> bool: - """Check if `node` refers to something explicitly annotated as _only_ returning None.""" - if isinstance(node.node, FuncDef): - return (isinstance(node.node.type, CallableType) and - isinstance(node.node.type.ret_type, NoneTyp)) - if isinstance(node.node, OverloadedFuncDef): + def always_returns_none(self, node: Expression) -> bool: + """Check if `node` refers to something explicitly annotated as only returning None.""" + if isinstance(node, RefExpr): + if self.defn_returns_none(node.node): + return True + if isinstance(node, MemberExpr) and node.node is None: # instance or class attribute + typ = self.chk.type_map.get(node.expr) + if isinstance(typ, Instance): + info = typ.type + elif (isinstance(typ, CallableType) and typ.is_type_obj() and + isinstance(typ.ret_type, Instance)): + info = typ.ret_type.type + else: + return False + sym = info.get(node.name) + if sym and self.defn_returns_none(sym.node): + return True + return False + + def defn_returns_none(self, defn: Optional[SymbolNode]) -> bool: + """Check if `defn` can _only_ return None.""" + if isinstance(defn, FuncDef): + return (isinstance(defn.type, CallableType) and + isinstance(defn.type.ret_type, NoneTyp)) + if isinstance(defn, OverloadedFuncDef): return all(isinstance(item.type, CallableType) and - isinstance(item.type.ret_type, NoneTyp) for item in node.node.items) - if isinstance(node.node, Var): - return (not node.node.is_inferred and isinstance(node.node.type, CallableType) and - isinstance(node.node.type.ret_type, NoneTyp)) + isinstance(item.type.ret_type, NoneTyp) for item in defn.items) + if isinstance(defn, Var): + if (not defn.is_inferred and isinstance(defn.type, CallableType) and + isinstance(defn.type.ret_type, NoneTyp)): + return True + if isinstance(defn.type, Instance): + sym = defn.type.type.get('__call__') + if sym and self.defn_returns_none(sym.node): + return True return False def check_runtime_protocol_test(self, e: CallExpr) -> None: diff --git a/test-data/unit/check-expressions.test b/test-data/unit/check-expressions.test index fa1ccbf5ae76f..26a86ca4b8013 100644 --- a/test-data/unit/check-expressions.test +++ b/test-data/unit/check-expressions.test @@ -936,8 +936,9 @@ main:3: error: "A" not callable a, o = None, None # type: (A, object) a = f() # E: "f" does not return a value -o = a() -o = A().g(a) +o = a() # E: Function does not return a value +o = A().g(a) # E: "g" of "A" does not return a value +o = A.g(a, a) # E: "g" of "A" does not return a value A().g(f()) # E: "f" does not return a value x: A = f() # E: "f" does not return a value f()