Skip to content

Use checkmember.py to check method override #18870

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.

Sign up for GitHub

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

Merged
merged 4 commits into from
Apr 2, 2025
Merged

Use checkmember.py to check method override #18870

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
fcf4c07
Use checkmember.py to check method override
ilevkivskyi Apr 1, 2025
4762fa8
Merge remote-tracking branch 'upstream/master' into method-subtype-ch…
ilevkivskyi Apr 2, 2025
ca714ae
Delete duplicate TODO
ilevkivskyi Apr 2, 2025
b23f46c
Update test case
ilevkivskyi Apr 2, 2025
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
171 changes: 53 additions & 118 deletions mypy/checker.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2134,40 +2134,17 @@ def check_method_or_accessor_override_for_base(
return None
return found_base_method

def check_setter_type_override(
self, defn: OverloadedFuncDef, base_attr: SymbolTableNode, base: TypeInfo
) -> None:
def check_setter_type_override(self, defn: OverloadedFuncDef, base: TypeInfo) -> None:
"""Check override of a setter type of a mutable attribute.

Currently, this should be only called when either base node or the current node
is a custom settable property (i.e. where setter type is different from getter type).
Note that this check is contravariant.
"""
base_node = base_attr.node
assert isinstance(base_node, (OverloadedFuncDef, Var))
original_type, is_original_setter = get_raw_setter_type(base_node)
if isinstance(base_node, Var):
expanded_type = map_type_from_supertype(original_type, defn.info, base)
original_type = get_proper_type(
expand_self_type(base_node, expanded_type, fill_typevars(defn.info))
)
else:
assert isinstance(original_type, ProperType)
assert isinstance(original_type, CallableType)
original_type = self.bind_and_map_method(base_attr, original_type, defn.info, base)
assert isinstance(original_type, CallableType)
if is_original_setter:
original_type = original_type.arg_types[0]
else:
original_type = original_type.ret_type

typ, is_setter = get_raw_setter_type(defn)
assert isinstance(typ, ProperType) and isinstance(typ, CallableType)
typ = bind_self(typ, self.scope.active_self_type())
if is_setter:
typ = typ.arg_types[0]
else:
typ = typ.ret_type
typ, _ = self.node_type_from_base(defn, defn.info, setter_type=True)
original_type, _ = self.node_type_from_base(defn, base, setter_type=True)
# The caller should handle deferrals.
assert typ is not None and original_type is not None

if not is_subtype(original_type, typ):
self.msg.incompatible_setter_override(defn.items[1], typ, original_type, base)
Expand All @@ -2192,28 +2169,19 @@ def check_method_override_for_base_with_name(
context = defn.func

# Construct the type of the overriding method.
# TODO: this logic is much less complete than similar one in checkmember.py
if isinstance(defn, (FuncDef, OverloadedFuncDef)):
typ: Type = self.function_type(defn)
override_class_or_static = defn.is_class or defn.is_static
override_class = defn.is_class
else:
assert defn.var.is_ready
assert defn.var.type is not None
typ = defn.var.type
override_class_or_static = defn.func.is_class or defn.func.is_static
override_class = defn.func.is_class
typ = get_proper_type(typ)
if isinstance(typ, FunctionLike) and not is_static(context):
typ = bind_self(typ, self.scope.active_self_type(), is_classmethod=override_class)
# Map the overridden method type to subtype context so that
# it can be checked for compatibility.
original_type = get_proper_type(base_attr.type)
typ, _ = self.node_type_from_base(defn, defn.info)
assert typ is not None

original_node = base_attr.node
# `original_type` can be partial if (e.g.) it is originally an
# instance variable from an `__init__` block that becomes deferred.
supertype_ready = True
if original_type is None or isinstance(original_type, PartialType):
original_type, _ = self.node_type_from_base(defn, base, name_override=name)
if original_type is None:
supertype_ready = False
if self.pass_num < self.last_pass:
# If there are passes left, defer this node until next pass,
Expand Down Expand Up @@ -2255,7 +2223,7 @@ def check_method_override_for_base_with_name(
# supertype is not known precisely.
if supertype_ready:
always_allow_covariant = True
self.check_setter_type_override(defn, base_attr, base)
self.check_setter_type_override(defn, base)

if isinstance(original_node, (FuncDef, OverloadedFuncDef)):
original_class_or_static = original_node.is_class or original_node.is_static
Expand All @@ -2265,41 +2233,24 @@ def check_method_override_for_base_with_name(
else:
original_class_or_static = False # a variable can't be class or static

if isinstance(original_type, FunctionLike):
original_type = self.bind_and_map_method(base_attr, original_type, defn.info, base)
if original_node and is_property(original_node):
original_type = get_property_type(original_type)

if isinstance(original_node, Var):
expanded_type = map_type_from_supertype(original_type, defn.info, base)
expanded_type = expand_self_type(
original_node, expanded_type, fill_typevars(defn.info)
)
original_type = get_proper_type(expanded_type)
typ = get_proper_type(typ)
original_type = get_proper_type(original_type)

if is_property(defn):
inner: FunctionLike | None
if isinstance(typ, FunctionLike):
inner = typ
else:
inner = self.extract_callable_type(typ, context)
if inner is not None:
typ = inner
typ = get_property_type(typ)
if (
isinstance(original_node, Var)
and not original_node.is_final
and (not original_node.is_property or original_node.is_settable_property)
and isinstance(defn, Decorator)
):
# We only give an error where no other similar errors will be given.
if not isinstance(original_type, AnyType):
self.msg.fail(
"Cannot override writeable attribute with read-only property",
# Give an error on function line to match old behaviour.
defn.func,
code=codes.OVERRIDE,
)
if (
is_property(defn)
and isinstance(original_node, Var)
and not original_node.is_final
and (not original_node.is_property or original_node.is_settable_property)
and isinstance(defn, Decorator)
):
# We only give an error where no other similar errors will be given.
if not isinstance(original_type, AnyType):
self.msg.fail(
"Cannot override writeable attribute with read-only property",
# Give an error on function line to match old behaviour.
defn.func,
code=codes.OVERRIDE,
)

if isinstance(original_type, AnyType) or isinstance(typ, AnyType):
pass
Expand Down Expand Up @@ -3412,7 +3363,7 @@ def get_variable_type_context(self, inferred: Var, rvalue: Expression) -> Type |
# For inference within class body, get supertype attribute as it would look on
# a class object for lambdas overriding methods, etc.
base_node = base.names[inferred.name].node
base_type, _ = self.lvalue_type_from_base(
base_type, _ = self.node_type_from_base(
inferred,
base,
is_class=is_method(base_node)
Expand Down Expand Up @@ -3523,7 +3474,7 @@ def check_compatibility_all_supers(self, lvalue: RefExpr, rvalue: Expression) ->
rvalue_type = self.expr_checker.accept(rvalue, lvalue_node.type)
actual_lvalue_type = lvalue_node.type
lvalue_node.type = rvalue_type
lvalue_type, _ = self.lvalue_type_from_base(lvalue_node, lvalue_node.info)
lvalue_type, _ = self.node_type_from_base(lvalue_node, lvalue_node.info)
if lvalue_node.is_inferred and not lvalue_node.explicit_self_type:
lvalue_node.type = actual_lvalue_type

Expand All @@ -3542,7 +3493,7 @@ def check_compatibility_all_supers(self, lvalue: RefExpr, rvalue: Expression) ->
if is_private(lvalue_node.name):
continue

base_type, base_node = self.lvalue_type_from_base(lvalue_node, base)
base_type, base_node = self.node_type_from_base(lvalue_node, base)
custom_setter = is_custom_settable_property(base_node)
if isinstance(base_type, PartialType):
base_type = None
Expand All @@ -3561,7 +3512,7 @@ def check_compatibility_all_supers(self, lvalue: RefExpr, rvalue: Expression) ->
# base classes are also incompatible
return
if lvalue_type and custom_setter:
base_type, _ = self.lvalue_type_from_base(
base_type, _ = self.node_type_from_base(
lvalue_node, base, setter_type=True
)
# Setter type for a custom property must be ready if
Expand Down Expand Up @@ -3612,26 +3563,33 @@ def check_compatibility_super(
)
return ok

def lvalue_type_from_base(
self, expr_node: Var, base: TypeInfo, setter_type: bool = False, is_class: bool = False
def node_type_from_base(
self,
node: SymbolNode,
base: TypeInfo,
*,
setter_type: bool = False,
is_class: bool = False,
name_override: str | None = None,
) -> tuple[Type | None, SymbolNode | None]:
"""Find a type for a variable name in base class.
"""Find a type for a name in base class.

Return the type found and the corresponding node defining the name or None
for both if the name is not defined in base or the node type is not known (yet).
The type returned is already properly mapped/bound to the subclass.
If setter_type is True, return setter types for settable properties (otherwise the
getter type is returned).
"""
expr_name = expr_node.name
base_var = base.names.get(expr_name)
name = name_override or node.name
base_node = base.names.get(name)

# TODO: defer current node if the superclass node is not ready.
if (
not base_var
or not base_var.type
or isinstance(base_var.type, PartialType)
and base_var.type.type is not None
not base_node
or isinstance(base_node.node, Var)
and not base_node.type
or isinstance(base_node.type, PartialType)
and base_node.type.type is not None
):
return None, None

Expand All @@ -3645,9 +3603,9 @@ def lvalue_type_from_base(
mx = MemberContext(
is_lvalue=setter_type,
is_super=False,
is_operator=mypy.checkexpr.is_operator_method(expr_name),
is_operator=mypy.checkexpr.is_operator_method(name),
original_type=self_type,
context=expr_node,
context=node,
chk=self,
suppress_errors=True,
)
Expand All @@ -3656,11 +3614,11 @@ def lvalue_type_from_base(
if is_class:
fallback = instance.type.metaclass_type or mx.named_type("builtins.type")
base_type = analyze_class_attribute_access(
instance, expr_name, mx, mcs_fallback=fallback, override_info=base
instance, name, mx, mcs_fallback=fallback, override_info=base
)
else:
base_type = analyze_instance_member_access(expr_name, instance, mx, base)
return base_type, base_var.node
base_type = analyze_instance_member_access(name, instance, mx, base)
return base_type, base_node.node

def check_compatibility_classvar_super(
self, node: Var, base: TypeInfo, base_node: Node | None
Expand Down Expand Up @@ -8965,29 +8923,6 @@ def is_custom_settable_property(defn: SymbolNode | None) -> bool:
return not is_same_type(get_property_type(get_proper_type(var.type)), setter_type)


def get_raw_setter_type(defn: OverloadedFuncDef | Var) -> tuple[Type, bool]:
"""Get an effective original setter type for a node.

For a variable it is simply its type. For a property it is the type
of the setter method (if not None), or the getter method (used as fallback
for the plugin generated properties).
Return the type and a flag indicating that we didn't fall back to getter.
"""
if isinstance(defn, Var):
# This function should not be called if the var is not ready.
assert defn.type is not None
return defn.type, True
first_item = defn.items[0]
assert isinstance(first_item, Decorator)
var = first_item.var
# This function may be called on non-custom properties, so we need
# to handle the situation when it is synthetic (plugin generated).
if var.setter_type is not None:
return var.setter_type, True
assert var.type is not None
return var.type, False


def get_property_type(t: ProperType) -> ProperType:
if isinstance(t, CallableType):
return get_proper_type(t.ret_type)
Expand Down
7 changes: 4 additions & 3 deletions mypy/checkmember.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -560,6 +560,8 @@ def analyze_member_var_access(
elif isinstance(v, MypyFile):
mx.chk.module_refs.add(v.fullname)
return mx.chk.expr_checker.module_type(v)
elif isinstance(v, TypeVarExpr):
return mx.chk.named_type("typing.TypeVar")
elif (
not v
and name not in ["__getattr__", "__setattr__", "__getattribute__"]
Expand Down Expand Up @@ -884,9 +886,8 @@ def analyze_var(
if isinstance(typ, FunctionLike) and not typ.is_type_obj():
call_type = typ
elif var.is_property:
call_type = get_proper_type(
_analyze_member_access("__call__", typ, mx.copy_modified(self_type=typ))
)
deco_mx = mx.copy_modified(original_type=typ, self_type=typ, is_lvalue=False)
call_type = get_proper_type(_analyze_member_access("__call__", typ, deco_mx))
else:
call_type = typ

Expand Down
10 changes: 6 additions & 4 deletions test-data/unit/check-classes.test
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7982,25 +7982,25 @@ class Parent:
class Child(Parent):
def foo(self, val: int) -> int: # E: Signature of "foo" incompatible with supertype "Parent" \
# N: Superclass: \
# N: None \
# N: <typing special form> \
# N: Subclass: \
# N: def foo(self, val: int) -> int
return val
def bar(self, val: str) -> str: # E: Signature of "bar" incompatible with supertype "Parent" \
# N: Superclass: \
# N: None \
# N: def __init__(self) -> bar \
# N: Subclass: \
# N: def bar(self, val: str) -> str
return val
def baz(self, val: float) -> float: # E: Signature of "baz" incompatible with supertype "Parent" \
# N: Superclass: \
# N: None \
# N: Module \
# N: Subclass: \
# N: def baz(self, val: float) -> float
return val
def foobar(self) -> bool: # E: Signature of "foobar" incompatible with supertype "Parent" \
# N: Superclass: \
# N: None \
# N: TypeVar \
# N: Subclass: \
# N: def foobar(self) -> bool
return False
Expand All @@ -8013,6 +8013,8 @@ a: int = child.foo(1)
b: str = child.bar("abc")
c: float = child.baz(3.4)
d: bool = child.foobar()
[builtins fixtures/module.pyi]
[typing fixtures/typing-full.pyi]

[case testGenericTupleTypeCreation]
from typing import Generic, Tuple, TypeVar
Expand Down
12 changes: 6 additions & 6 deletions test-data/unit/check-functions.test
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2819,6 +2819,8 @@ class Child(Base):
@decorator
def foo(self) -> int:
return 42
reveal_type(Child().foo) # N: Revealed type is "builtins.int"
Child().foo = 1 # E: Property "foo" defined in "Child" is read-only

reveal_type(Child().foo) # N: Revealed type is "builtins.int"

Expand All @@ -2835,15 +2837,13 @@ class not_a_decorator:
def __init__(self, fn): ...

class BadChild2(Base):
# Override error not shown as accessing 'foo' on BadChild2 returns Any.
@property
@not_a_decorator
def foo(self) -> int: # E: "not_a_decorator" not callable \
# E: Signature of "foo" incompatible with supertype "Base" \
# N: Superclass: \
# N: int \
# N: Subclass: \
# N: not_a_decorator
def foo(self) -> int:
return 42
reveal_type(BadChild2().foo) # E: "not_a_decorator" not callable \
# N: Revealed type is "Any"
[builtins fixtures/property.pyi]

[case explicitOverride]
Expand Down
8 changes: 4 additions & 4 deletions test-data/unit/check-plugin-attrs.test
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -990,10 +990,10 @@ class C(A, B): pass
@attr.s
class D(A): pass

reveal_type(A.__lt__) # N: Revealed type is "def [_AT] (self: _AT`5, other: _AT`5) -> builtins.bool"
reveal_type(B.__lt__) # N: Revealed type is "def [_AT] (self: _AT`6, other: _AT`6) -> builtins.bool"
reveal_type(C.__lt__) # N: Revealed type is "def [_AT] (self: _AT`7, other: _AT`7) -> builtins.bool"
reveal_type(D.__lt__) # N: Revealed type is "def [_AT] (self: _AT`8, other: _AT`8) -> builtins.bool"
reveal_type(A.__lt__) # N: Revealed type is "def [_AT] (self: _AT`29, other: _AT`29) -> builtins.bool"
reveal_type(B.__lt__) # N: Revealed type is "def [_AT] (self: _AT`30, other: _AT`30) -> builtins.bool"
reveal_type(C.__lt__) # N: Revealed type is "def [_AT] (self: _AT`31, other: _AT`31) -> builtins.bool"
reveal_type(D.__lt__) # N: Revealed type is "def [_AT] (self: _AT`32, other: _AT`32) -> builtins.bool"

A() < A()
B() < B()
Expand Down
Loading