From 5043b7e168659a8ac150135e4c993c72b9d036fc Mon Sep 17 00:00:00 2001 From: Brian Schubert Date: Tue, 22 Oct 2024 21:57:13 -0400 Subject: [PATCH 1/3] Check compatibility in conditional function definitions using decorators --- mypy/checker.py | 84 +++++++++++++++------------ test-data/unit/check-functions.test | 11 ++-- test-data/unit/check-newsemanal.test | 4 +- test-data/unit/check-overloading.test | 6 +- 4 files changed, 58 insertions(+), 47 deletions(-) diff --git a/mypy/checker.py b/mypy/checker.py index 4b3d6c3298b4..7b2ac575d3ef 100644 --- a/mypy/checker.py +++ b/mypy/checker.py @@ -1072,46 +1072,50 @@ def _visit_func_def(self, defn: FuncDef) -> None: if defn.original_def: # Override previous definition. new_type = self.function_type(defn) - if isinstance(defn.original_def, FuncDef): - # Function definition overrides function definition. - 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. - orig_type = defn.original_def.type - if orig_type is None: - # If other branch is unreachable, we don't type check it and so we might - # not have a type for the original definition - return - if isinstance(orig_type, PartialType): - if orig_type.type is None: - # Ah this is a partial type. Give it the type of the function. - orig_def = defn.original_def - if isinstance(orig_def, Decorator): - var = orig_def.var - else: - var = orig_def - partial_types = self.find_partial_types(var) - if partial_types is not None: - var.type = new_type - del partial_types[var] + self.check_func_def_override(defn, new_type) + + def check_func_def_override(self, defn: FuncDef, new_type: FunctionLike) -> None: + assert defn.original_def is not None + if isinstance(defn.original_def, FuncDef): + # Function definition overrides function definition. + 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. + orig_type = defn.original_def.type + if orig_type is None: + # If other branch is unreachable, we don't type check it and so we might + # not have a type for the original definition + return + if isinstance(orig_type, PartialType): + if orig_type.type is None: + # Ah this is a partial type. Give it the type of the function. + orig_def = defn.original_def + if isinstance(orig_def, Decorator): + var = orig_def.var else: - # Trying to redefine something like partial empty list as function. - self.fail(message_registry.INCOMPATIBLE_REDEFINITION, defn) + var = orig_def + partial_types = self.find_partial_types(var) + if partial_types is not None: + var.type = new_type + del partial_types[var] else: - name_expr = NameExpr(defn.name) - name_expr.node = defn.original_def - self.binder.assign_type(name_expr, new_type, orig_type) - self.check_subtype( - new_type, - orig_type, - defn, - message_registry.INCOMPATIBLE_REDEFINITION, - "redefinition with type", - "original type", - ) + # Trying to redefine something like partial empty list as function. + self.fail(message_registry.INCOMPATIBLE_REDEFINITION, defn) + else: + name_expr = NameExpr(defn.name) + name_expr.node = defn.original_def + self.binder.assign_type(name_expr, new_type, orig_type) + self.check_subtype( + new_type, + orig_type, + defn, + message_registry.INCOMPATIBLE_REDEFINITION, + "redefinition with type", + "original type", + ) def check_func_item( self, @@ -5120,6 +5124,10 @@ def visit_decorator_inner(self, e: Decorator, allow_empty: bool = False) -> None if e.type and not isinstance(get_proper_type(e.type), (FunctionLike, AnyType)): self.fail(message_registry.BAD_CONSTRUCTOR_TYPE, e) + if e.func.original_def: + # Function definition overrides function definition. + self.check_func_def_override(e.func, sig) + def check_for_untyped_decorator( self, func: FuncDef, dec_type: Type, dec_expr: Expression ) -> None: diff --git a/test-data/unit/check-functions.test b/test-data/unit/check-functions.test index 96f9815019e6..b8a02a1ec7d4 100644 --- a/test-data/unit/check-functions.test +++ b/test-data/unit/check-functions.test @@ -1474,7 +1474,7 @@ def dec(f) -> Callable[[int], None]: pass x = int() if x: - def f(x: int) -> None: pass + def f(x: int, /) -> None: pass else: @dec def f(): pass @@ -1489,9 +1489,12 @@ x = int() if x: def f(x: str) -> None: pass else: - # TODO: Complain about incompatible redefinition @dec - def f(): pass + def f(): pass # E: All conditional function variants must have identical signatures \ + # N: Original: \ + # N: def f(x: str) -> None \ + # N: Redefinition: \ + # N: def f(int, /) -> None [case testConditionalFunctionDefinitionUnreachable] def bar() -> None: @@ -1599,7 +1602,7 @@ else: def f(): yield [file m.py] -def f(): pass +def f() -> None: pass [case testDefineConditionallyAsImportedAndDecoratedWithInference] if int(): diff --git a/test-data/unit/check-newsemanal.test b/test-data/unit/check-newsemanal.test index 511c7b003015..fe02ac3ccd5e 100644 --- a/test-data/unit/check-newsemanal.test +++ b/test-data/unit/check-newsemanal.test @@ -1908,9 +1908,9 @@ else: @dec def f(x: int) -> None: 1() # E: "int" not callable -reveal_type(f) # N: Revealed type is "def (x: builtins.str)" +reveal_type(f) # N: Revealed type is "def (builtins.str)" [file m.py] -def f(x: str) -> None: pass +def f(x: str, /) -> None: pass [case testNewAnalyzerConditionallyDefineFuncOverVar] from typing import Callable diff --git a/test-data/unit/check-overloading.test b/test-data/unit/check-overloading.test index e414c1c9b0b6..9d01ce6bd480 100644 --- a/test-data/unit/check-overloading.test +++ b/test-data/unit/check-overloading.test @@ -6463,7 +6463,7 @@ class D: ... def f1(g: A) -> A: ... if True: @overload # E: Single overload definition, multiple required - def f1(g: B) -> B: ... + def f1(g: B) -> B: ... # E: Incompatible redefinition (redefinition with type "Callable[[B], B]", original type "Callable[[A], A]") if maybe_true: # E: Condition can't be inferred, unable to merge overloads \ # E: Name "maybe_true" is not defined @overload @@ -6480,14 +6480,14 @@ if True: def f2(g: B) -> B: ... elif maybe_true: # E: Name "maybe_true" is not defined @overload # E: Single overload definition, multiple required - def f2(g: C) -> C: ... + def f2(g: C) -> C: ... # E: Incompatible redefinition (redefinition with type "Callable[[C], C]", original type "Callable[[A], A]") def f2(g): ... # E: Name "f2" already defined on line 21 @overload # E: Single overload definition, multiple required def f3(g: A) -> A: ... if True: @overload # E: Single overload definition, multiple required - def f3(g: B) -> B: ... + def f3(g: B) -> B: ... # E: Incompatible redefinition (redefinition with type "Callable[[B], B]", original type "Callable[[A], A]") if True: pass # Some other node @overload # E: Name "f3" already defined on line 32 \ From 0520d0bb49ec792a5b2f87de500634ec7b9c6a1c Mon Sep 17 00:00:00 2001 From: Brian Schubert Date: Tue, 22 Oct 2024 22:15:51 -0400 Subject: [PATCH 2/3] Move definition of check_func_def_override to simplify diff --- mypy/checker.py | 68 ++++++++++++++++++++++++------------------------- 1 file changed, 34 insertions(+), 34 deletions(-) diff --git a/mypy/checker.py b/mypy/checker.py index 7b2ac575d3ef..bdb9c1b9a8bb 100644 --- a/mypy/checker.py +++ b/mypy/checker.py @@ -1074,6 +1074,40 @@ def _visit_func_def(self, defn: FuncDef) -> None: new_type = self.function_type(defn) self.check_func_def_override(defn, new_type) + def check_func_item( + self, + defn: FuncItem, + type_override: CallableType | None = None, + name: str | None = None, + allow_empty: bool = False, + ) -> None: + """Type check a function. + + If type_override is provided, use it as the function type. + """ + self.dynamic_funcs.append(defn.is_dynamic() and not type_override) + + with self.enter_partial_types(is_function=True): + typ = self.function_type(defn) + if type_override: + typ = type_override.copy_modified(line=typ.line, column=typ.column) + if isinstance(typ, CallableType): + with self.enter_attribute_inference_context(): + self.check_func_def(defn, typ, name, allow_empty) + else: + raise RuntimeError("Not supported") + + self.dynamic_funcs.pop() + self.current_node_deferred = False + + if name == "__exit__": + self.check__exit__return_type(defn) + # TODO: the following logic should move to the dataclasses plugin + # https://github.com/python/mypy/issues/15515 + if name == "__post_init__": + if dataclasses_plugin.is_processed_dataclass(defn.info): + dataclasses_plugin.check_post_init(self, defn, defn.info) + def check_func_def_override(self, defn: FuncDef, new_type: FunctionLike) -> None: assert defn.original_def is not None if isinstance(defn.original_def, FuncDef): @@ -1117,40 +1151,6 @@ def check_func_def_override(self, defn: FuncDef, new_type: FunctionLike) -> None "original type", ) - def check_func_item( - self, - defn: FuncItem, - type_override: CallableType | None = None, - name: str | None = None, - allow_empty: bool = False, - ) -> None: - """Type check a function. - - If type_override is provided, use it as the function type. - """ - self.dynamic_funcs.append(defn.is_dynamic() and not type_override) - - with self.enter_partial_types(is_function=True): - typ = self.function_type(defn) - if type_override: - typ = type_override.copy_modified(line=typ.line, column=typ.column) - if isinstance(typ, CallableType): - with self.enter_attribute_inference_context(): - self.check_func_def(defn, typ, name, allow_empty) - else: - raise RuntimeError("Not supported") - - self.dynamic_funcs.pop() - self.current_node_deferred = False - - if name == "__exit__": - self.check__exit__return_type(defn) - # TODO: the following logic should move to the dataclasses plugin - # https://github.com/python/mypy/issues/15515 - if name == "__post_init__": - if dataclasses_plugin.is_processed_dataclass(defn.info): - dataclasses_plugin.check_post_init(self, defn, defn.info) - @contextmanager def enter_attribute_inference_context(self) -> Iterator[None]: old_types = self.inferred_attribute_types From 3e6a50bcd1f0aa701299bc150f2f114bfc6dfb87 Mon Sep 17 00:00:00 2001 From: Brian Schubert Date: Tue, 22 Oct 2024 22:19:56 -0400 Subject: [PATCH 3/3] Narrow sig for mypyc --- mypy/checker.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/mypy/checker.py b/mypy/checker.py index bdb9c1b9a8bb..f52bebdaa052 100644 --- a/mypy/checker.py +++ b/mypy/checker.py @@ -5124,7 +5124,7 @@ def visit_decorator_inner(self, e: Decorator, allow_empty: bool = False) -> None if e.type and not isinstance(get_proper_type(e.type), (FunctionLike, AnyType)): self.fail(message_registry.BAD_CONSTRUCTOR_TYPE, e) - if e.func.original_def: + if e.func.original_def and isinstance(sig, FunctionLike): # Function definition overrides function definition. self.check_func_def_override(e.func, sig)