From 6c95b758b525108851952a8832f0e079b33d87b1 Mon Sep 17 00:00:00 2001 From: hauntsaninja Date: Sat, 3 Sep 2022 22:25:58 -0700 Subject: [PATCH 1/4] Better diagnostic for conditional function mismatch Fixes #10575 --- mypy/checker.py | 5 +++-- mypy/messages.py | 6 +++++- test-data/unit/check-functions.test | 16 ++++++++++++---- test-data/unit/check-modules.test | 4 +++- test-data/unit/check-newsemanal.test | 9 +++++++-- test-data/unit/check-unreachable-code.test | 8 ++++++-- 6 files changed, 36 insertions(+), 12 deletions(-) diff --git a/mypy/checker.py b/mypy/checker.py index 1c5b834c1d25..1f244c4fbf8a 100644 --- a/mypy/checker.py +++ b/mypy/checker.py @@ -950,8 +950,9 @@ def _visit_func_def(self, defn: FuncDef) -> None: new_type = self.function_type(defn) if isinstance(defn.original_def, FuncDef): # Function definition overrides function definition. - if not is_same_type(new_type, self.function_type(defn.original_def)): - self.msg.incompatible_conditional_function_def(defn) + old_type = self.function_type(defn.original_def) + if not is_same_type(new_type, old_type): + self.msg.incompatible_conditional_function_def(defn, old_type, new_type) else: # Function definition overrides a variable initialized via assignment or a # decorated function. diff --git a/mypy/messages.py b/mypy/messages.py index 4e54f0d57d11..f31c4746e5bf 100644 --- a/mypy/messages.py +++ b/mypy/messages.py @@ -1301,8 +1301,12 @@ def incompatible_self_argument( context, ) - def incompatible_conditional_function_def(self, defn: FuncDef) -> None: + def incompatible_conditional_function_def( + self, defn: FuncDef, old_type: FunctionLike, new_type: FunctionLike + ) -> None: self.fail("All conditional function variants must have identical signatures", defn) + self.note(f"Original signature: {old_type}", defn) + self.note(f"Redefinition signature: {new_type}", defn) def cannot_instantiate_abstract_class( self, class_name: str, abstract_attributes: dict[str, bool], context: Context diff --git a/test-data/unit/check-functions.test b/test-data/unit/check-functions.test index 61f6c8ad02fc..c400df3e75f5 100644 --- a/test-data/unit/check-functions.test +++ b/test-data/unit/check-functions.test @@ -1393,7 +1393,9 @@ x = None # type: Any if x: def f(x: int) -> None: pass else: - def f(x): pass # E: All conditional function variants must have identical signatures + def f(x): pass # E: All conditional function variants must have identical signatures \ + # N: Original signature: def (x: builtins.int) \ + # N: Redefinition signature: def (x: Any) -> Any [case testIncompatibleConditionalFunctionDefinition2] from typing import Any @@ -1401,7 +1403,9 @@ x = None # type: Any if x: def f(x: int) -> None: pass else: - def f(y: int) -> None: pass # E: All conditional function variants must have identical signatures + def f(y: int) -> None: pass # E: All conditional function variants must have identical signatures \ + # N: Original signature: def (x: builtins.int) \ + # N: Redefinition signature: def (y: builtins.int) [case testIncompatibleConditionalFunctionDefinition3] from typing import Any @@ -1409,7 +1413,9 @@ x = None # type: Any if x: def f(x: int) -> None: pass else: - def f(x: int = 0) -> None: pass # E: All conditional function variants must have identical signatures + def f(x: int = 0) -> None: pass # E: All conditional function variants must have identical signatures \ + # N: Original signature: def (x: builtins.int) \ + # N: Redefinition signature: def (x: builtins.int =) [case testConditionalFunctionDefinitionUsingDecorator1] from typing import Callable @@ -1640,7 +1646,9 @@ class A: if x: def f(self, x: int) -> None: pass else: - def f(self, x): pass # E: All conditional function variants must have identical signatures + def f(self, x): pass # E: All conditional function variants must have identical signatures \ + # N: Original signature: def (self: __main__.A, x: builtins.int) \ + # N: Redefinition signature: def (self: __main__.A, x: Any) -> Any [out] [case testConditionalFunctionDefinitionInTry] diff --git a/test-data/unit/check-modules.test b/test-data/unit/check-modules.test index 03f3105c5be3..47e609129bcc 100644 --- a/test-data/unit/check-modules.test +++ b/test-data/unit/check-modules.test @@ -625,7 +625,9 @@ try: from m import f, g except: def f(x): pass - def g(x): pass # E: All conditional function variants must have identical signatures + def g(x): pass # E: All conditional function variants must have identical signatures \ + # N: Original signature: def (x: Any, y: Any) -> Any \ + # N: Redefinition signature: def (x: Any) -> Any [file m.py] def f(x): pass def g(x, y): pass diff --git a/test-data/unit/check-newsemanal.test b/test-data/unit/check-newsemanal.test index bf612f95b3a2..703f46b1fb41 100644 --- a/test-data/unit/check-newsemanal.test +++ b/test-data/unit/check-newsemanal.test @@ -1864,7 +1864,9 @@ if int(): elif bool(): def f(x: int) -> None: 1() # E: "int" not callable - def g(x: str) -> None: # E: All conditional function variants must have identical signatures + def g(x: str) -> None: # E: All conditional function variants must have identical signatures \ + # N: Original signature: def (x: builtins.int) \ + # N: Redefinition signature: def (x: builtins.str) pass else: def f(x: int) -> None: @@ -1881,7 +1883,10 @@ if int(): else: def f(x: A) -> None: 1() # E: "int" not callable - def g(x: str) -> None: # E: All conditional function variants must have identical signatures + def g(x: str) -> None: # E: All conditional function variants must have identical signatures \ + # N: Original signature: def (x: __main__.A) \ + # N: Redefinition signature: def (x: builtins.str) + pass reveal_type(g) # N: Revealed type is "def (x: __main__.A)" diff --git a/test-data/unit/check-unreachable-code.test b/test-data/unit/check-unreachable-code.test index 64736e55e2dd..1d029474e7b3 100644 --- a/test-data/unit/check-unreachable-code.test +++ b/test-data/unit/check-unreachable-code.test @@ -242,7 +242,9 @@ import sys if sys.version_info >= (3, 5, 0): def foo() -> int: return 0 else: - def foo() -> str: return '' # E: All conditional function variants must have identical signatures + def foo() -> str: return '' # E: All conditional function variants must have identical signatures \ + # N: Original signature: def () -> builtins.int \ + # N: Redefinition signature: def () -> builtins.str [builtins fixtures/ops.pyi] [out] @@ -253,7 +255,9 @@ import sys if sys.version_info[1:] >= (5, 0): def foo() -> int: return 0 else: - def foo() -> str: return '' # E: All conditional function variants must have identical signatures + def foo() -> str: return '' # E: All conditional function variants must have identical signatures \ + # N: Original signature: def () -> builtins.int \ + # N: Redefinition signature: def () -> builtins.str [builtins fixtures/ops.pyi] [out] From 1078ebd541e0410ce0ed133e0db3d6d4e91c9f5f Mon Sep 17 00:00:00 2001 From: hauntsaninja Date: Sat, 3 Sep 2022 23:14:16 -0700 Subject: [PATCH 2/4] fix more tests --- test-data/unit/check-functions.test | 8 ++++++-- test-data/unit/check-unions.test | 12 +++++++++--- 2 files changed, 15 insertions(+), 5 deletions(-) diff --git a/test-data/unit/check-functions.test b/test-data/unit/check-functions.test index c400df3e75f5..e0d134bb734a 100644 --- a/test-data/unit/check-functions.test +++ b/test-data/unit/check-functions.test @@ -1473,14 +1473,18 @@ from typing import Any def f(x: str) -> None: pass x = None # type: Any if x: - def f(x: int) -> None: pass # E: All conditional function variants must have identical signatures + def f(x: int) -> None: pass # E: All conditional function variants must have identical signatures \ + # N: Original signature: def (x: builtins.str) \ + # N: Redefinition signature: def (x: builtins.int) [case testConditionalRedefinitionOfAnUnconditionalFunctionDefinition2] from typing import Any def f(x: int) -> None: pass # N: "f" defined here x = None # type: Any if x: - def f(y: int) -> None: pass # E: All conditional function variants must have identical signatures + def f(y: int) -> None: pass # E: All conditional function variants must have identical signatures \ + # N: Original signature: def (x: builtins.int) \ + # N: Redefinition signature: def (y: builtins.int) f(x=1) # The first definition takes precedence. f(y=1) # E: Unexpected keyword argument "y" for "f" diff --git a/test-data/unit/check-unions.test b/test-data/unit/check-unions.test index e772b489a6d2..816ef438f6c6 100644 --- a/test-data/unit/check-unions.test +++ b/test-data/unit/check-unions.test @@ -193,11 +193,15 @@ elif foo(): elif foo(): def f(x: Union[int, str, int, int, str]) -> None: pass elif foo(): - def f(x: Union[int, str, float]) -> None: pass # E: All conditional function variants must have identical signatures + def f(x: Union[int, str, float]) -> None: pass # E: All conditional function variants must have identical signatures \ + # N: Original signature: def (x: Union[builtins.int, builtins.str]) \ + # N: Redefinition signature: def (x: Union[builtins.int, builtins.str, builtins.float]) elif foo(): def f(x: Union[S, T]) -> None: pass elif foo(): - def f(x: Union[str]) -> None: pass # E: All conditional function variants must have identical signatures + def f(x: Union[str]) -> None: pass # E: All conditional function variants must have identical signatures \ + # N: Original signature: def (x: Union[builtins.int, builtins.str]) \ + # N: Redefinition signature: def (x: builtins.str) else: def f(x: Union[Union[int, T], Union[S, T], str]) -> None: pass @@ -206,7 +210,9 @@ else: if foo(): def g(x: Union[int, str, bytes]) -> None: pass else: - def g(x: Union[int, str]) -> None: pass # E: All conditional function variants must have identical signatures + def g(x: Union[int, str]) -> None: pass # E: All conditional function variants must have identical signatures \ + # N: Original signature: def (x: Union[builtins.int, builtins.str, builtins.bytes]) \ + # N: Redefinition signature: def (x: Union[builtins.int, builtins.str]) [case testUnionSimplificationSpecialCases] from typing import Any, TypeVar, Union From 8fcb26e3a481eb319f612a2ce95503448ca00a0b Mon Sep 17 00:00:00 2001 From: hauntsaninja Date: Sat, 3 Sep 2022 23:30:37 -0700 Subject: [PATCH 3/4] use pretty callable --- mypy/messages.py | 9 +++++++-- test-data/unit/check-functions.test | 6 ++++-- 2 files changed, 11 insertions(+), 4 deletions(-) diff --git a/mypy/messages.py b/mypy/messages.py index f31c4746e5bf..5b58ca45cd5e 100644 --- a/mypy/messages.py +++ b/mypy/messages.py @@ -1305,8 +1305,13 @@ def incompatible_conditional_function_def( self, defn: FuncDef, old_type: FunctionLike, new_type: FunctionLike ) -> None: self.fail("All conditional function variants must have identical signatures", defn) - self.note(f"Original signature: {old_type}", defn) - self.note(f"Redefinition signature: {new_type}", defn) + if isinstance(old_type, (CallableType, Overloaded)) and isinstance( + new_type, (CallableType, Overloaded) + ): + self.note("Original:", defn) + self.pretty_callable_or_overload(old_type, defn, offset=4) + self.note("Redefinition:", defn) + self.pretty_callable_or_overload(new_type, defn, offset=4) def cannot_instantiate_abstract_class( self, class_name: str, abstract_attributes: dict[str, bool], context: Context diff --git a/test-data/unit/check-functions.test b/test-data/unit/check-functions.test index e0d134bb734a..d73b57a54ff1 100644 --- a/test-data/unit/check-functions.test +++ b/test-data/unit/check-functions.test @@ -1394,8 +1394,10 @@ if x: def f(x: int) -> None: pass else: def f(x): pass # E: All conditional function variants must have identical signatures \ - # N: Original signature: def (x: builtins.int) \ - # N: Redefinition signature: def (x: Any) -> Any + # N: Original: \ + # N: def f(x: int) -> None \ + # N: Redefinition: \ + # N: def f(x: Any) -> Any [case testIncompatibleConditionalFunctionDefinition2] from typing import Any From b64193039a7890b221c7962f4297dcc41bbfe10e Mon Sep 17 00:00:00 2001 From: hauntsaninja Date: Sun, 25 Sep 2022 13:50:03 -0700 Subject: [PATCH 4/4] fix tests --- test-data/unit/check-functions.test | 30 ++++++++++++++-------- test-data/unit/check-modules.test | 6 +++-- test-data/unit/check-newsemanal.test | 12 ++++++--- test-data/unit/check-unions.test | 20 ++++++++++----- test-data/unit/check-unreachable-code.test | 12 ++++++--- 5 files changed, 53 insertions(+), 27 deletions(-) diff --git a/test-data/unit/check-functions.test b/test-data/unit/check-functions.test index d73b57a54ff1..2ebb56ddeb45 100644 --- a/test-data/unit/check-functions.test +++ b/test-data/unit/check-functions.test @@ -1406,8 +1406,10 @@ if x: def f(x: int) -> None: pass else: def f(y: int) -> None: pass # E: All conditional function variants must have identical signatures \ - # N: Original signature: def (x: builtins.int) \ - # N: Redefinition signature: def (y: builtins.int) + # N: Original: \ + # N: def f(x: int) -> None \ + # N: Redefinition: \ + # N: def f(y: int) -> None [case testIncompatibleConditionalFunctionDefinition3] from typing import Any @@ -1416,8 +1418,10 @@ if x: def f(x: int) -> None: pass else: def f(x: int = 0) -> None: pass # E: All conditional function variants must have identical signatures \ - # N: Original signature: def (x: builtins.int) \ - # N: Redefinition signature: def (x: builtins.int =) + # N: Original: \ + # N: def f(x: int) -> None \ + # N: Redefinition: \ + # N: def f(x: int = ...) -> None [case testConditionalFunctionDefinitionUsingDecorator1] from typing import Callable @@ -1476,8 +1480,10 @@ def f(x: str) -> None: pass x = None # type: Any if x: def f(x: int) -> None: pass # E: All conditional function variants must have identical signatures \ - # N: Original signature: def (x: builtins.str) \ - # N: Redefinition signature: def (x: builtins.int) + # N: Original: \ + # N: def f(x: str) -> None \ + # N: Redefinition: \ + # N: def f(x: int) -> None [case testConditionalRedefinitionOfAnUnconditionalFunctionDefinition2] from typing import Any @@ -1485,8 +1491,10 @@ def f(x: int) -> None: pass # N: "f" defined here x = None # type: Any if x: def f(y: int) -> None: pass # E: All conditional function variants must have identical signatures \ - # N: Original signature: def (x: builtins.int) \ - # N: Redefinition signature: def (y: builtins.int) + # N: Original: \ + # N: def f(x: int) -> None \ + # N: Redefinition: \ + # N: def f(y: int) -> None f(x=1) # The first definition takes precedence. f(y=1) # E: Unexpected keyword argument "y" for "f" @@ -1653,8 +1661,10 @@ class A: def f(self, x: int) -> None: pass else: def f(self, x): pass # E: All conditional function variants must have identical signatures \ - # N: Original signature: def (self: __main__.A, x: builtins.int) \ - # N: Redefinition signature: def (self: __main__.A, x: Any) -> Any + # N: Original: \ + # N: def f(self: A, x: int) -> None \ + # N: Redefinition: \ + # N: def f(self: A, x: Any) -> Any [out] [case testConditionalFunctionDefinitionInTry] diff --git a/test-data/unit/check-modules.test b/test-data/unit/check-modules.test index 47e609129bcc..3ba552584145 100644 --- a/test-data/unit/check-modules.test +++ b/test-data/unit/check-modules.test @@ -626,8 +626,10 @@ try: except: def f(x): pass def g(x): pass # E: All conditional function variants must have identical signatures \ - # N: Original signature: def (x: Any, y: Any) -> Any \ - # N: Redefinition signature: def (x: Any) -> Any + # N: Original: \ + # N: def g(x: Any, y: Any) -> Any \ + # N: Redefinition: \ + # N: def g(x: Any) -> Any [file m.py] def f(x): pass def g(x, y): pass diff --git a/test-data/unit/check-newsemanal.test b/test-data/unit/check-newsemanal.test index 703f46b1fb41..d784aadffd67 100644 --- a/test-data/unit/check-newsemanal.test +++ b/test-data/unit/check-newsemanal.test @@ -1865,8 +1865,10 @@ elif bool(): def f(x: int) -> None: 1() # E: "int" not callable def g(x: str) -> None: # E: All conditional function variants must have identical signatures \ - # N: Original signature: def (x: builtins.int) \ - # N: Redefinition signature: def (x: builtins.str) + # N: Original: \ + # N: def g(x: int) -> None \ + # N: Redefinition: \ + # N: def g(x: str) -> None pass else: def f(x: int) -> None: @@ -1884,8 +1886,10 @@ else: def f(x: A) -> None: 1() # E: "int" not callable def g(x: str) -> None: # E: All conditional function variants must have identical signatures \ - # N: Original signature: def (x: __main__.A) \ - # N: Redefinition signature: def (x: builtins.str) + # N: Original: \ + # N: def g(x: A) -> None \ + # N: Redefinition: \ + # N: def g(x: str) -> None pass diff --git a/test-data/unit/check-unions.test b/test-data/unit/check-unions.test index 816ef438f6c6..f29e9d4b3f6b 100644 --- a/test-data/unit/check-unions.test +++ b/test-data/unit/check-unions.test @@ -194,14 +194,18 @@ elif foo(): def f(x: Union[int, str, int, int, str]) -> None: pass elif foo(): def f(x: Union[int, str, float]) -> None: pass # E: All conditional function variants must have identical signatures \ - # N: Original signature: def (x: Union[builtins.int, builtins.str]) \ - # N: Redefinition signature: def (x: Union[builtins.int, builtins.str, builtins.float]) + # N: Original: \ + # N: def f(x: Union[int, str]) -> None \ + # N: Redefinition: \ + # N: def f(x: Union[int, str, float]) -> None elif foo(): def f(x: Union[S, T]) -> None: pass elif foo(): def f(x: Union[str]) -> None: pass # E: All conditional function variants must have identical signatures \ - # N: Original signature: def (x: Union[builtins.int, builtins.str]) \ - # N: Redefinition signature: def (x: builtins.str) + # N: Original: \ + # N: def f(x: Union[int, str]) -> None \ + # N: Redefinition: \ + # N: def f(x: str) -> None else: def f(x: Union[Union[int, T], Union[S, T], str]) -> None: pass @@ -210,9 +214,11 @@ else: if foo(): def g(x: Union[int, str, bytes]) -> None: pass else: - def g(x: Union[int, str]) -> None: pass # E: All conditional function variants must have identical signatures \ - # N: Original signature: def (x: Union[builtins.int, builtins.str, builtins.bytes]) \ - # N: Redefinition signature: def (x: Union[builtins.int, builtins.str]) + def g(x: Union[int, str]) -> None: pass # E: All conditional function variants must have identical signatures \ + # N: Original: \ + # N: def g(x: Union[int, str, bytes]) -> None \ + # N: Redefinition: \ + # N: def g(x: Union[int, str]) -> None [case testUnionSimplificationSpecialCases] from typing import Any, TypeVar, Union diff --git a/test-data/unit/check-unreachable-code.test b/test-data/unit/check-unreachable-code.test index 1d029474e7b3..44e6b66c02e6 100644 --- a/test-data/unit/check-unreachable-code.test +++ b/test-data/unit/check-unreachable-code.test @@ -243,8 +243,10 @@ if sys.version_info >= (3, 5, 0): def foo() -> int: return 0 else: def foo() -> str: return '' # E: All conditional function variants must have identical signatures \ - # N: Original signature: def () -> builtins.int \ - # N: Redefinition signature: def () -> builtins.str + # N: Original: \ + # N: def foo() -> int \ + # N: Redefinition: \ + # N: def foo() -> str [builtins fixtures/ops.pyi] [out] @@ -256,8 +258,10 @@ if sys.version_info[1:] >= (5, 0): def foo() -> int: return 0 else: def foo() -> str: return '' # E: All conditional function variants must have identical signatures \ - # N: Original signature: def () -> builtins.int \ - # N: Redefinition signature: def () -> builtins.str + # N: Original: \ + # N: def foo() -> int \ + # N: Redefinition: \ + # N: def foo() -> str [builtins fixtures/ops.pyi] [out]