Skip to content
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

Fix compatibility checks for conditional function definitions using decorators #18020

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
88 changes: 48 additions & 40 deletions mypy/checker.py
Original file line number Diff line number Diff line change
Expand Up @@ -1072,46 +1072,7 @@ 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]
else:
# 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",
)
self.check_func_def_override(defn, new_type)

def check_func_item(
self,
Expand Down Expand Up @@ -1147,6 +1108,49 @@ def check_func_item(
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):
# 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]
else:
# 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",
)

@contextmanager
def enter_attribute_inference_context(self) -> Iterator[None]:
old_types = self.inferred_attribute_types
Expand Down Expand Up @@ -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 and isinstance(sig, FunctionLike):
# 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:
Expand Down
11 changes: 7 additions & 4 deletions test-data/unit/check-functions.test
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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:
Expand Down Expand Up @@ -1599,7 +1602,7 @@ else:
def f():
yield
[file m.py]
def f(): pass
def f() -> None: pass

[case testDefineConditionallyAsImportedAndDecoratedWithInference]
if int():
Expand Down
4 changes: 2 additions & 2 deletions test-data/unit/check-newsemanal.test
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
6 changes: 3 additions & 3 deletions test-data/unit/check-overloading.test
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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 \
Expand Down
Loading