Support properties with setter type different from getter type (#18510)

Fixes #3004
Fixes #11892
Fixes #12892
Fixes #14301

_Note:_ this PR should be reviewed with "hide whitespace" option (in
couple long functions I replace huge `if x: ...` with `if not x: return;
...` to reduce indent level).

The core logic is quite straightforward (almost trivial). The only
couple things are:
* We should be careful with binder (we simpy can't use it for properties
with setter type different from getter type, since we don't know
underlying property implementation)
* We need to handle gracefully existing settable properties that are
generated by plugins

The tricky part is subclassing and protocols. The summary is as
following:
* For protocols I simply implement everything the "correct way", i.e.
for settable attributes (whether it is a variable or a settable
property) compare getter types covariantly and setter types
contravariantly. The tricky part here is generating meaningful error
messages that are also not too verbose.
* For subclassing I cannot simply do the same, because there is a flag
about covariant mutable override, that is off by default. So instead
what I do is if either subclass node, or superclass node is a "custom
property" (i.e. a property with setter type different from getter type),
then I use the "correct way", otherwise the old logic (i.e. flag
dependent check) is used.

Two things that are not implemented are multiple inheritance, and new
generic syntax (inferred variance). In these cases setter types are
simply ignored. There is nothing conceptually difficult about these, I
simply run out of steam (and the PR is getting big). I left `TODO`s in
code for these. In most cases these will generate false negatives (and
they are already kind of corner cases) so I think it is OK to postpone
these indefinitely.

Author: ilevkivskyi

mypy/checker.py

@@ -658,7 +658,10 @@ def analyze_descriptor_access(
     if isinstance(descriptor_type, UnionType):
         # Map the access over union types
         return make_simplified_union(
-            [analyze_descriptor_access(typ, mx) for typ in descriptor_type.items]
+            [
+                analyze_descriptor_access(typ, mx, assignment=assignment)
+                for typ in descriptor_type.items
+            ]
         )
     elif not isinstance(descriptor_type, Instance):
         return orig_descriptor_type
@@ -776,7 +779,13 @@ def analyze_var(
     # Found a member variable.
     original_itype = itype
     itype = map_instance_to_supertype(itype, var.info)
-    typ = var.type
+    if var.is_settable_property and mx.is_lvalue:
+        typ: Type | None = var.setter_type
+        if typ is None and var.is_ready:
+            # Existing synthetic properties may not set setter type. Fall back to getter.
+            typ = var.type
+    else:
+        typ = var.type
     if typ:
         if isinstance(typ, PartialType):
             return mx.chk.handle_partial_var_type(typ, mx.is_lvalue, var, mx.context)
@@ -834,7 +843,10 @@ def analyze_var(
                 if var.is_property:
                     # A property cannot have an overloaded type => the cast is fine.
                     assert isinstance(expanded_signature, CallableType)
-                    result = expanded_signature.ret_type
+                    if var.is_settable_property and mx.is_lvalue and var.setter_type is not None:
+                        result = expanded_signature.arg_types[0]
+                    else:
+                        result = expanded_signature.ret_type
                 else:
                     result = expanded_signature
     else:

mypy/checkmember.py

@@ -39,7 +39,7 @@
     codes.OVERRIDE,
 }
 
-allowed_duplicates: Final = ["@overload", "Got:", "Expected:"]
+allowed_duplicates: Final = ["@overload", "Got:", "Expected:", "Expected setter type:"]
 
 BASE_RTD_URL: Final = "https://mypy.rtfd.io/en/stable/_refs.html#code"
 
@@ -172,10 +172,12 @@ def __init__(
         *,
         filter_errors: bool | Callable[[str, ErrorInfo], bool] = False,
         save_filtered_errors: bool = False,
+        filter_deprecated: bool = False,
     ) -> None:
         self.errors = errors
         self._has_new_errors = False
         self._filter = filter_errors
+        self._filter_deprecated = filter_deprecated
         self._filtered: list[ErrorInfo] | None = [] if save_filtered_errors else None
 
     def __enter__(self) -> ErrorWatcher:
@@ -196,7 +198,8 @@ def on_error(self, file: str, info: ErrorInfo) -> bool:
         ErrorWatcher further down the stack and from being recorded by Errors
         """
         if info.code == codes.DEPRECATED:
-            return False
+            # Deprecated is not a type error, so it is handled on opt-in basis here.
+            return self._filter_deprecated
 
         self._has_new_errors = True
         if isinstance(self._filter, bool):

mypy/errors.py

@@ -210,6 +210,8 @@ def visit_var(self, v: Var) -> None:
             v.info = self.current_info
         if v.type is not None:
             v.type.accept(self.type_fixer)
+        if v.setter_type is not None:
+            v.setter_type.accept(self.type_fixer)
 
     def visit_type_alias(self, a: TypeAlias) -> None:
         a.target.accept(self.type_fixer)

mypy/fixup.py

@@ -56,6 +56,7 @@
 from mypy.subtypes import (
     IS_CLASS_OR_STATIC,
     IS_CLASSVAR,
+    IS_EXPLICIT_SETTER,
     IS_SETTABLE,
     IS_VAR,
     find_member,
@@ -186,9 +187,13 @@ def filter_errors(
         *,
         filter_errors: bool | Callable[[str, ErrorInfo], bool] = True,
         save_filtered_errors: bool = False,
+        filter_deprecated: bool = False,
     ) -> ErrorWatcher:
         return ErrorWatcher(
-            self.errors, filter_errors=filter_errors, save_filtered_errors=save_filtered_errors
+            self.errors,
+            filter_errors=filter_errors,
+            save_filtered_errors=save_filtered_errors,
+            filter_deprecated=filter_deprecated,
         )
 
     def add_errors(self, errors: list[ErrorInfo]) -> None:
@@ -1164,6 +1169,20 @@ def overload_signature_incompatible_with_supertype(
         note_template = 'Overload variants must be defined in the same order as they are in "{}"'
         self.note(note_template.format(supertype), context, code=codes.OVERRIDE)
 
+    def incompatible_setter_override(
+        self, defn: Context, typ: Type, original_type: Type, base: TypeInfo
+    ) -> None:
+        self.fail("Incompatible override of a setter type", defn, code=codes.OVERRIDE)
+        base_str, override_str = format_type_distinctly(original_type, typ, options=self.options)
+        self.note(
+            f' (base class "{base.name}" defined the type as {base_str},',
+            defn,
+            code=codes.OVERRIDE,
+        )
+        self.note(f" override has type {override_str})", defn, code=codes.OVERRIDE)
+        if is_subtype(typ, original_type):
+            self.note(" Setter types should behave contravariantly", defn, code=codes.OVERRIDE)
+
     def signature_incompatible_with_supertype(
         self,
         name: str,
@@ -2201,22 +2220,34 @@ def report_protocol_problems(
         ):
             type_name = format_type(subtype, self.options, module_names=True)
             self.note(f"Following member(s) of {type_name} have conflicts:", context, code=code)
-            for name, got, exp in conflict_types[:MAX_ITEMS]:
+            for name, got, exp, is_lvalue in conflict_types[:MAX_ITEMS]:
                 exp = get_proper_type(exp)
                 got = get_proper_type(got)
+                setter_suffix = " setter type" if is_lvalue else ""
                 if not isinstance(exp, (CallableType, Overloaded)) or not isinstance(
                     got, (CallableType, Overloaded)
                 ):
                     self.note(
-                        "{}: expected {}, got {}".format(
-                            name, *format_type_distinctly(exp, got, options=self.options)
+                        "{}: expected{} {}, got {}".format(
+                            name,
+                            setter_suffix,
+                            *format_type_distinctly(exp, got, options=self.options),
                         ),
                         context,
                         offset=OFFSET,
                         code=code,
                     )
+                    if is_lvalue and is_subtype(got, exp, options=self.options):
+                        self.note(
+                            "Setter types should behave contravariantly",
+                            context,
+                            offset=OFFSET,
+                            code=code,
+                        )
                 else:
-                    self.note("Expected:", context, offset=OFFSET, code=code)
+                    self.note(
+                        "Expected{}:".format(setter_suffix), context, offset=OFFSET, code=code
+                    )
                     if isinstance(exp, CallableType):
                         self.note(
                             pretty_callable(exp, self.options, skip_self=class_obj or is_module),
@@ -3029,12 +3060,12 @@ def get_missing_protocol_members(left: Instance, right: Instance, skip: list[str
 
 def get_conflict_protocol_types(
     left: Instance, right: Instance, class_obj: bool = False, options: Options | None = None
-) -> list[tuple[str, Type, Type]]:
+) -> list[tuple[str, Type, Type, bool]]:
     """Find members that are defined in 'left' but have incompatible types.
-    Return them as a list of ('member', 'got', 'expected').
+    Return them as a list of ('member', 'got', 'expected', 'is_lvalue').
     """
     assert right.type.is_protocol
-    conflicts: list[tuple[str, Type, Type]] = []
+    conflicts: list[tuple[str, Type, Type, bool]] = []
     for member in right.type.protocol_members:
         if member in ("__init__", "__new__"):
             continue
@@ -3044,10 +3075,29 @@ def get_conflict_protocol_types(
         if not subtype:
             continue
         is_compat = is_subtype(subtype, supertype, ignore_pos_arg_names=True, options=options)
-        if IS_SETTABLE in get_member_flags(member, right):
-            is_compat = is_compat and is_subtype(supertype, subtype, options=options)
         if not is_compat:
-            conflicts.append((member, subtype, supertype))
+            conflicts.append((member, subtype, supertype, False))
+        superflags = get_member_flags(member, right)
+        if IS_SETTABLE not in superflags:
+            continue
+        different_setter = False
+        if IS_EXPLICIT_SETTER in superflags:
+            set_supertype = find_member(member, right, left, is_lvalue=True)
+            if set_supertype and not is_same_type(set_supertype, supertype):
+                different_setter = True
+            supertype = set_supertype
+        if IS_EXPLICIT_SETTER in get_member_flags(member, left):
+            set_subtype = mypy.typeops.get_protocol_member(left, member, class_obj, is_lvalue=True)
+            if set_subtype and not is_same_type(set_subtype, subtype):
+                different_setter = True
+            subtype = set_subtype
+        if not is_compat and not different_setter:
+            # We already have this conflict listed, avoid duplicates.
+            continue
+        assert supertype is not None and subtype is not None
+        is_compat = is_subtype(supertype, subtype, options=options)
+        if not is_compat:
+            conflicts.append((member, subtype, supertype, different_setter))
     return conflicts
 
 

mypy/messages.py

@@ -977,6 +977,7 @@ class Var(SymbolNode):
         "_fullname",
         "info",
         "type",
+        "setter_type",
         "final_value",
         "is_self",
         "is_cls",
@@ -1011,6 +1012,8 @@ def __init__(self, name: str, type: mypy.types.Type | None = None) -> None:
         # TODO: Should be Optional[TypeInfo]
         self.info = VAR_NO_INFO
         self.type: mypy.types.Type | None = type  # Declared or inferred type, or None
+        # The setter type for settable properties.
+        self.setter_type: mypy.types.CallableType | None = None
         # Is this the first argument to an ordinary method (usually "self")?
         self.is_self = False
         # Is this the first argument to a classmethod (typically "cls")?
@@ -1076,6 +1079,7 @@ def serialize(self) -> JsonDict:
             "name": self._name,
             "fullname": self._fullname,
             "type": None if self.type is None else self.type.serialize(),
+            "setter_type": None if self.setter_type is None else self.setter_type.serialize(),
             "flags": get_flags(self, VAR_FLAGS),
         }
         if self.final_value is not None:
@@ -1087,7 +1091,18 @@ def deserialize(cls, data: JsonDict) -> Var:
         assert data[".class"] == "Var"
         name = data["name"]
         type = None if data["type"] is None else mypy.types.deserialize_type(data["type"])
+        setter_type = (
+            None
+            if data["setter_type"] is None
+            else mypy.types.deserialize_type(data["setter_type"])
+        )
         v = Var(name, type)
+        assert (
+            setter_type is None
+            or isinstance(setter_type, mypy.types.ProperType)
+            and isinstance(setter_type, mypy.types.CallableType)
+        )
+        v.setter_type = setter_type
         v.is_ready = False  # Override True default set in __init__
         v._fullname = data["fullname"]
         set_flags(v, data["flags"])

mypy/nodes.py

@@ -245,6 +245,11 @@ def snapshot_definition(node: SymbolNode | None, common: SymbolSnapshot) -> Symb
             impl = node
         elif isinstance(node, OverloadedFuncDef) and node.impl:
             impl = node.impl.func if isinstance(node.impl, Decorator) else node.impl
+        setter_type = None
+        if isinstance(node, OverloadedFuncDef) and node.items:
+            first_item = node.items[0]
+            if isinstance(first_item, Decorator) and first_item.func.is_property:
+                setter_type = snapshot_optional_type(first_item.var.setter_type)
         is_trivial_body = impl.is_trivial_body if impl else False
         dataclass_transform_spec = find_dataclass_transform_spec(node)
         return (
@@ -258,6 +263,7 @@ def snapshot_definition(node: SymbolNode | None, common: SymbolSnapshot) -> Symb
             is_trivial_body,
             dataclass_transform_spec.serialize() if dataclass_transform_spec is not None else None,
             node.deprecated if isinstance(node, FuncDef) else None,
+            setter_type,  # multi-part properties are stored as OverloadedFuncDef
         )
     elif isinstance(node, Var):
         return ("Var", common, snapshot_optional_type(node.type), node.is_final)

mypy/server/astdiff.py

@@ -330,6 +330,7 @@ def visit_enum_call_expr(self, node: EnumCallExpr) -> None:
     def visit_var(self, node: Var) -> None:
         node.info = self.fixup(node.info)
         self.fixup_type(node.type)
+        self.fixup_type(node.setter_type)
         super().visit_var(node)
 
     def visit_type_alias(self, node: TypeAlias) -> None: