Repository: python/mypy
Pull request: Check __setattr__ when property is not settable
Fixes #9160

Problem statement:
class with __setattr__ reports properties are read-only
When a class implements `__setattr__`, mypy still reports properties as read-only.

Minimum example:

````
class Foo:
    def __init__(self) -> None:
        self._d = {'a': 1}

    def __setattr__(self, name: str, value: Any) -> None:
        if hasattr(self, '_d'):
            self._d[name] = value
        else:
            super(self.__class__, self).__setattr__(name, value)

    @property
    def a(self) -> int:
        return self._d['a']

f = Foo()
f.a = 2
````

The error points to the last line in the above code: `error: Property "a" defined in "Foo" is read-only`.

Expectation: Since `Foo` implements `__setattr__`, mypy should allow the attribute to be set, even though it overlaps with a class property.

There are no issues identified if the property `a` is removed, and I am using version 0.781.

Hints from issue discussion:
Yeah, this looks like false positive.
Anybody want to contribute a fix -- it should be easy?

Add a check about `__setattr__` in `mypy.checkmember.analyze_var` (before the call to `mx.msg.read_only_property`).
@JukkaL I can take care of this.

Commit message: Check __setattr__ before calling read_only_property

Patch to review:
diff --git a/mypy/checkmember.py b/mypy/checkmember.py
index c9a5a2c86d97..fa06c171cbb8 100644
--- a/mypy/checkmember.py
+++ b/mypy/checkmember.py
@@ -550,7 +550,8 @@ def analyze_var(name: str,
             return mx.chk.handle_partial_var_type(typ, mx.is_lvalue, var, mx.context)
         if mx.is_lvalue and var.is_property and not var.is_settable_property:
             # TODO allow setting attributes in subclass (although it is probably an error)
-            mx.msg.read_only_property(name, itype.type, mx.context)
+            if info.names.get('__setattr__') is None:
+                mx.msg.read_only_property(name, itype.type, mx.context)
         if mx.is_lvalue and var.is_classvar:
             mx.msg.cant_assign_to_classvar(name, mx.context)
         t = get_proper_type(expand_type_by_instance(typ, itype))
@@ -560,7 +561,8 @@ def analyze_var(name: str,
             if mx.is_lvalue:
                 if var.is_property:
                     if not var.is_settable_property:
-                        mx.msg.read_only_property(name, itype.type, mx.context)
+                        if info.names.get('__setattr__') is None:
+                            mx.msg.read_only_property(name, itype.type, mx.context)
                 else:
                     mx.msg.cant_assign_to_method(mx.context)
 
diff --git a/mypy/test/testcheck.py b/mypy/test/testcheck.py
index 49a85861b6e3..3b86214a4b8d 100644
--- a/mypy/test/testcheck.py
+++ b/mypy/test/testcheck.py
@@ -89,6 +89,7 @@
     'check-reports.test',
     'check-errorcodes.test',
     'check-annotated.test',
+    'check-setattr.test',
 ]
 
 # Tests that use Python 3.8-only AST features (like expression-scoped ignores):
diff --git a/test-data/unit/check-setattr.test b/test-data/unit/check-setattr.test
new file mode 100644
index 000000000000..4a1e03a7dc62
--- /dev/null
+++ b/test-data/unit/check-setattr.test
@@ -0,0 +1,12 @@
+[case testSetAttrWithProperty]
+from typing import Any
+class Foo:
+    _a = 1
+    def __setattr__(self, name: str, value: Any) -> None:
+        self._a = value
+    @property
+    def a(self) -> int:
+        return self._a
+f = Foo()
+f.a = 2
+[builtins fixtures/property.pyi]


Code review comments:
diff_hunk: @@ -550,7 +550,8 @@ def analyze_var(name: str,
             return mx.chk.handle_partial_var_type(typ, mx.is_lvalue, var, mx.context)
         if mx.is_lvalue and var.is_property and not var.is_settable_property:
             # TODO allow setting attributes in subclass (although it is probably an error)
-            mx.msg.read_only_property(name, itype.type, mx.context)
+            if info.names.get('__setattr__') is None:
path: mypy/checkmember.py
text: @user1:
how about `if '__setattr__' not in info.names:`

@user2:
The best thing would be to use `info.get`, since that also looks at base classes that might define `__setattr__`.