From ab766cc8357e3a839f1e8b27be3c942c1f40cccb Mon Sep 17 00:00:00 2001 From: Brian Schubert Date: Thu, 26 Sep 2024 16:16:42 -0400 Subject: [PATCH 1/5] Fix detection of decorated function always returning None --- mypy/checkexpr.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/mypy/checkexpr.py b/mypy/checkexpr.py index 08d7345452fb..a4bbbdf57ed3 100644 --- a/mypy/checkexpr.py +++ b/mypy/checkexpr.py @@ -716,6 +716,8 @@ def always_returns_none(self, node: Expression) -> bool: def defn_returns_none(self, defn: SymbolNode | None) -> bool: """Check if `defn` can _only_ return None.""" + if isinstance(defn, Decorator): + defn = defn.func if isinstance(defn, FuncDef): return isinstance(defn.type, CallableType) and isinstance( get_proper_type(defn.type.ret_type), NoneType From db07091ddb3907e397ca96d276f2b8c57069651b Mon Sep 17 00:00:00 2001 From: Brian Schubert Date: Tue, 22 Oct 2024 14:25:07 -0400 Subject: [PATCH 2/5] Add test case for decorated functions that always return none Co-authored-by: Harald Husum --- test-data/unit/check-expressions.test | 26 ++++++++++++++++++++++++++ 1 file changed, 26 insertions(+) diff --git a/test-data/unit/check-expressions.test b/test-data/unit/check-expressions.test index d5ddc910bcd6..e493fbeeab83 100644 --- a/test-data/unit/check-expressions.test +++ b/test-data/unit/check-expressions.test @@ -1144,6 +1144,32 @@ f() and b # E: "f" does not return a value (it only ever returns None) b or f() # E: "f" does not return a value (it only ever returns None) [builtins fixtures/bool.pyi] +[case testNoneReturnDecorated] +from typing import Any, Callable, TypeVar + +F = TypeVar('F', bound=Callable[..., Any]) + +def deco(f: F) -> F: + pass + +@deco +@deco +def f() -> None: + pass + +class A: + @staticmethod + def s() -> None: + pass + +if int(): + x = f() # E: "f" does not return a value (it only ever returns None) +if int(): + x = A.s() # E: "s" of "A" does not return a value (it only ever returns None) +if int(): + x = A().s() # E: "s" of "A" does not return a value (it only ever returns None) +[builtins fixtures/staticmethod.pyi] + -- Slicing -- ------- From 21d8078c7fb1baf6f7fa6667c19c783ec577605e Mon Sep 17 00:00:00 2001 From: Brian Schubert Date: Tue, 22 Oct 2024 15:33:51 -0400 Subject: [PATCH 3/5] Fix handling of decorators that modify the function signature --- mypy/checkexpr.py | 8 +++----- test-data/unit/check-expressions.test | 24 ++++++++++++++++++++++-- 2 files changed, 25 insertions(+), 7 deletions(-) diff --git a/mypy/checkexpr.py b/mypy/checkexpr.py index a4bbbdf57ed3..020fbad14292 100644 --- a/mypy/checkexpr.py +++ b/mypy/checkexpr.py @@ -717,7 +717,7 @@ def always_returns_none(self, node: Expression) -> bool: def defn_returns_none(self, defn: SymbolNode | None) -> bool: """Check if `defn` can _only_ return None.""" if isinstance(defn, Decorator): - defn = defn.func + defn = defn.var if isinstance(defn, FuncDef): return isinstance(defn.type, CallableType) and isinstance( get_proper_type(defn.type.ret_type), NoneType @@ -726,10 +726,8 @@ def defn_returns_none(self, defn: SymbolNode | None) -> bool: return all(self.defn_returns_none(item) for item in defn.items) if isinstance(defn, Var): typ = get_proper_type(defn.type) - if ( - not defn.is_inferred - and isinstance(typ, CallableType) - and isinstance(get_proper_type(typ.ret_type), NoneType) + if isinstance(typ, CallableType) and isinstance( + get_proper_type(typ.ret_type), NoneType ): return True if isinstance(typ, Instance): diff --git a/test-data/unit/check-expressions.test b/test-data/unit/check-expressions.test index e493fbeeab83..8576c13fdf91 100644 --- a/test-data/unit/check-expressions.test +++ b/test-data/unit/check-expressions.test @@ -1152,9 +1152,25 @@ F = TypeVar('F', bound=Callable[..., Any]) def deco(f: F) -> F: pass +def deco_return_none(f: object) -> Callable[..., None]: + pass + +def deco_return_not_none(f: object) -> Callable[..., int]: + pass + @deco @deco -def f() -> None: +def f1() -> None: + pass + +@deco +@deco_return_none +def f2() -> int: + pass + +@deco +@deco_return_not_none +def f3() -> None: pass class A: @@ -1163,7 +1179,11 @@ class A: pass if int(): - x = f() # E: "f" does not return a value (it only ever returns None) + x = f1() # E: "f1" does not return a value (it only ever returns None) +if int(): + x = f2() # E: "f2" does not return a value (it only ever returns None) +if int(): + x = f3() if int(): x = A.s() # E: "s" of "A" does not return a value (it only ever returns None) if int(): From 5cc0f238998b853e8a8a333c99b57f3fb99a9164 Mon Sep 17 00:00:00 2001 From: Brian Schubert Date: Tue, 22 Oct 2024 15:49:56 -0400 Subject: [PATCH 4/5] Update test with changed behavior --- test-data/unit/check-optional.test | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/test-data/unit/check-optional.test b/test-data/unit/check-optional.test index 683ce0446915..93f8f61f57dd 100644 --- a/test-data/unit/check-optional.test +++ b/test-data/unit/check-optional.test @@ -119,8 +119,8 @@ reveal_type(z2) # N: Revealed type is "Union[Literal[0], builtins.str, None]" [case testLambdaReturningNone] f = lambda: None -x = f() -reveal_type(x) # N: Revealed type is "None" +x = f() # E: Function does not return a value (it only ever returns None) +reveal_type(x) # N: Revealed type is "Any" [case testNoneArgumentType] def f(x: None) -> None: pass From fe52ee1402f659be280ac40452fc8163ac59e6bf Mon Sep 17 00:00:00 2001 From: Brian Schubert Date: Tue, 22 Oct 2024 17:05:45 -0400 Subject: [PATCH 5/5] Preserve original behavior for testLambdaReturningNone --- mypy/checkexpr.py | 8 ++++++-- test-data/unit/check-optional.test | 4 ++-- 2 files changed, 8 insertions(+), 4 deletions(-) diff --git a/mypy/checkexpr.py b/mypy/checkexpr.py index 020fbad14292..7a13c68b6fd2 100644 --- a/mypy/checkexpr.py +++ b/mypy/checkexpr.py @@ -716,7 +716,9 @@ def always_returns_none(self, node: Expression) -> bool: def defn_returns_none(self, defn: SymbolNode | None) -> bool: """Check if `defn` can _only_ return None.""" + allow_inferred = False if isinstance(defn, Decorator): + allow_inferred = True defn = defn.var if isinstance(defn, FuncDef): return isinstance(defn.type, CallableType) and isinstance( @@ -726,8 +728,10 @@ def defn_returns_none(self, defn: SymbolNode | None) -> bool: return all(self.defn_returns_none(item) for item in defn.items) if isinstance(defn, Var): typ = get_proper_type(defn.type) - if isinstance(typ, CallableType) and isinstance( - get_proper_type(typ.ret_type), NoneType + if ( + (allow_inferred or not defn.is_inferred) + and isinstance(typ, CallableType) + and isinstance(get_proper_type(typ.ret_type), NoneType) ): return True if isinstance(typ, Instance): diff --git a/test-data/unit/check-optional.test b/test-data/unit/check-optional.test index 93f8f61f57dd..683ce0446915 100644 --- a/test-data/unit/check-optional.test +++ b/test-data/unit/check-optional.test @@ -119,8 +119,8 @@ reveal_type(z2) # N: Revealed type is "Union[Literal[0], builtins.str, None]" [case testLambdaReturningNone] f = lambda: None -x = f() # E: Function does not return a value (it only ever returns None) -reveal_type(x) # N: Revealed type is "Any" +x = f() +reveal_type(x) # N: Revealed type is "None" [case testNoneArgumentType] def f(x: None) -> None: pass