Repository: quora/pyanalyze
Pull request #655 (2023-07-04 02:38:55)

Drop use of qcore.testing.Anything
Slightly less use of qcore



Patch:
diff --git a/docs/changelog.md b/docs/changelog.md
index 5c3bceb..cc3963e 100644
--- a/docs/changelog.md
+++ b/docs/changelog.md
@@ -2,6 +2,8 @@
 
 ## Unreleased
 
+- Fix treatment of `@property` by the `incompatible_override`
+  check (#653)
 - Drop support for Python 3.7 (#654)
 - Add hardcoded support for `pytest.raises` to avoid false
   positives (#651)
diff --git a/pyanalyze/name_check_visitor.py b/pyanalyze/name_check_visitor.py
index 072cb5d..0129232 100644
--- a/pyanalyze/name_check_visitor.py
+++ b/pyanalyze/name_check_visitor.py
@@ -394,11 +394,7 @@ class _AttrContext(CheckerAttrContext):
             self.visitor._record_type_attr_read(obj, self.attr, self.node)
 
     def get_property_type_from_argspec(self, obj: property) -> Value:
-        if obj.fget is None:
-            return UNINITIALIZED_VALUE
-
-        getter = set_self(KnownValue(obj.fget), self.root_composite.value)
-        return self.visitor.check_call(self.node, getter, [self.root_composite])
+        return self.visitor.resolve_property(obj, self.root_composite, self.node)
 
     def should_ignore_none_attributes(self) -> bool:
         return self.ignore_none
@@ -1494,7 +1490,7 @@ class NameCheckVisitor(node_visitor.ReplacingNodeVisitor):
         if varname in self.options.get_value_for(IgnoredForIncompatibleOverride):
             return
         for base_class, base_value in self._get_base_class_attributes(varname, node):
-            can_assign = self._can_assign_to_base(base_value, value)
+            can_assign = self._can_assign_to_base(base_value, value, base_class, node)
             if isinstance(can_assign, CanAssignError):
                 error = CanAssignError(
                     children=[
@@ -1513,25 +1509,84 @@ class NameCheckVisitor(node_visitor.ReplacingNodeVisitor):
     def display_value(self, value: Value) -> str:
         return self.checker.display_value(value)
 
-    def _can_assign_to_base(self, base_value: Value, child_value: Value) -> CanAssign:
+    def resolve_property(
+        self, obj: property, root_composite: Composite, node: Optional[ast.AST]
+    ) -> Value:
+        if obj.fget is None:
+            return UNINITIALIZED_VALUE
+
+        getter = set_self(KnownValue(obj.fget), root_composite.value)
+        return self.check_call(node, getter, [root_composite])
+
+    def _can_assign_to_base(
+        self,
+        base_value: Value,
+        child_value: Value,
+        base_class: Union[type, str],
+        node: ast.AST,
+    ) -> CanAssign:
         if base_value is UNINITIALIZED_VALUE:
             return {}
-        if isinstance(base_value, KnownValue) and callable(base_value.val):
-            base_sig = self.signature_from_value(base_value)
-            if not isinstance(base_sig, (Signature, OverloadedSignature)):
-                return {}
-            child_sig = self.signature_from_value(child_value)
-            if not isinstance(child_sig, (Signature, OverloadedSignature)):
-                return CanAssignError(f"{child_value} is not callable")
-            base_bound = base_sig.bind_self(ctx=self)
-            if base_bound is None:
-                return {}
-            child_bound = child_sig.bind_self(ctx=self)
-            if child_bound is None:
-                return CanAssignError(f"{child_value} is missing a 'self' argument")
-            return base_bound.can_assign(child_bound, self)
+        if isinstance(base_value, KnownValue):
+            if isinstance(base_value.val, property):
+                return self._can_assign_to_base_property(
+                    base_value.val, child_value, base_class, node
+                )
+            if callable(base_value.val):
+                return self._can_assign_to_base_callable(base_value, child_value)
         return base_value.can_assign(child_value, self)
 
+    def _can_assign_to_base_property(
+        self,
+        base_property: property,
+        child_value: Value,
+        base_class: Union[type, str],
+        node: ast.AST,
+    ) -> CanAssign:
+        if isinstance(child_value, KnownValue) and isinstance(
+            child_value.val, property
+        ):
+            if base_property.fset is not None and child_value.val.fset is None:
+                return CanAssignError(
+                    "Property is settable on base class but not on child class"
+                )
+            if base_property.fdel is not None and child_value.val.fdel is None:
+                return CanAssignError(
+                    "Property is settable on base class but not on child class"
+                )
+            assert self.current_class is not None
+            child_value = self.resolve_property(
+                child_value.val, Composite(TypedValue(self.current_class)), node
+            )
+        base_value = self.resolve_property(
+            base_property, Composite(TypedValue(base_class)), node
+        )
+        get_direction = base_value.can_assign(child_value, self)
+        if isinstance(get_direction, CanAssignError):
+            return get_direction
+        if base_property.fset is not None:
+            # settable properties behave invariantly, so we need to check both directions
+            return child_value.can_assign(base_value, self)
+        else:
+            return get_direction
+
+    def _can_assign_to_base_callable(
+        self, base_value: KnownValue, child_value: Value
+    ) -> CanAssign:
+        base_sig = self.signature_from_value(base_value)
+        if not isinstance(base_sig, (Signature, OverloadedSignature)):
+            return {}
+        child_sig = self.signature_from_value(child_value)
+        if not isinstance(child_sig, (Signature, OverloadedSignature)):
+            return CanAssignError(f"{child_value} is not callable")
+        base_bound = base_sig.bind_self(ctx=self)
+        if base_bound is None:
+            return {}
+        child_bound = child_sig.bind_self(ctx=self)
+        if child_bound is None:
+            return CanAssignError(f"{child_value} is missing a 'self' argument")
+        return base_bound.can_assign(child_bound, self)
+
     def _check_for_class_variable_redefinition(
         self, varname: str, node: ast.AST
     ) -> None:


Test patch:
diff --git a/pyanalyze/test_attributes.py b/pyanalyze/test_attributes.py
index 6ba75e5..2f36607 100644
--- a/pyanalyze/test_attributes.py
+++ b/pyanalyze/test_attributes.py
@@ -175,14 +175,15 @@ class TestAttributes(TestNameCheckVisitorBase):
 
     @assert_passes()
     def test_annotated_known(self):
-        from qcore.testing import Anything
+        from unittest.mock import ANY
         from typing_extensions import Annotated, Literal
+        from typing import Any, cast
 
         from pyanalyze.extensions import LiteralOnly
         from pyanalyze.stacked_scopes import Composite, VarnameWithOrigin
         from pyanalyze.value import CustomCheckExtension
 
-        origin = VarnameWithOrigin("encoding", Anything)  # E: incompatible_argument
+        origin = VarnameWithOrigin("encoding", cast(Any, ANY))
 
         def capybara():
             encoding: Annotated[Literal["ascii"], LiteralOnly()] = "ascii"
diff --git a/pyanalyze/test_name_check_visitor.py b/pyanalyze/test_name_check_visitor.py
index b527297..25fbd13 100644
--- a/pyanalyze/test_name_check_visitor.py
+++ b/pyanalyze/test_name_check_visitor.py
@@ -1228,12 +1228,10 @@ class TestUnboundMethodValue(TestNameCheckVisitorBase):
     @assert_passes()
     def test_metaclass_super(self):
         from typing import Any, cast
-
-        from qcore.testing import Anything
-
+        from unittest.mock import ANY
         from pyanalyze.stacked_scopes import Composite, VarnameWithOrigin
 
-        varname = VarnameWithOrigin("self", cast(Any, Anything))
+        varname = VarnameWithOrigin("self", cast(Any, ANY))
 
         class Metaclass(type):
             def __init__(self, name, bases, attrs):
@@ -1923,6 +1921,83 @@ class TestIncompatibleOverride(TestNameCheckVisitorBase):
             def pacarana(self, b: int) -> None:
                 pass
 
+    @assert_passes()
+    def test_property_unannotated(self):
+        class Unannotated:
+            @property
+            def f(self):
+                pass
+
+        class UnannotatedChild(Unannotated):
+            @property
+            def f(self):
+                pass
+
+    @assert_passes()
+    def test_property_annotated(self):
+        class Annotated:
+            @property
+            def f(self) -> int:
+                return 0
+
+        class AnnotatedChild(Annotated):
+            @property
+            def f(self) -> int:
+                return 0
+
+        class MoreSpecificChild(Annotated):
+            @property
+            def f(self) -> bool:
+                return True
+
+        class JustPutItInTheClassChild(Annotated):
+            f: int
+
+        class BadChild(Annotated):
+            @property
+            def f(self) -> str:  # E: incompatible_override
+                return ""
+
+    @assert_passes()
+    def test_property_with_setter(self):
+        class Annotated:
+            @property
+            def f(self) -> int:
+                return 0
+
+            @f.setter
+            def f(self, value: int) -> None:
+                pass
+
+        class AnnotatedChild(Annotated):
+            @property
+            def f(self) -> int:  # E: incompatible_override
+                return 0
+
+        class AnnotatedChildWithSetter(Annotated):
+            @property
+            def f(self) -> int:
+                return 0
+
+            @f.setter
+            def f(self, value: int) -> None:
+                pass
+
+        class JustPutItInTheClassChild(Annotated):
+            f: int
+
+        class JustPutItInTheClassWithMoreSpecificType(Annotated):
+            f: bool  # E: incompatible_override
+
+        class AnnotatedChildWithMoreSpecificSetter(Annotated):
+            @property
+            def f(self) -> bool:  # E: incompatible_override
+                return False
+
+            @f.setter
+            def f(self, value: bool) -> None:  # E: incompatible_override
+                pass
+
 
 class TestWalrus(TestNameCheckVisitorBase):
     def test(self):
diff --git a/pyanalyze/test_signature.py b/pyanalyze/test_signature.py
index 07c54bb..d55c055 100644
--- a/pyanalyze/test_signature.py
+++ b/pyanalyze/test_signature.py
@@ -671,7 +671,7 @@ class TestCalls(TestNameCheckVisitorBase):
         from dataclasses import dataclass
         from typing import Any, cast
 
-        from qcore.testing import Anything
+        from unittest.mock import ANY
 
         @dataclass
         class Quemisia(object):
@@ -691,7 +691,7 @@ class TestCalls(TestNameCheckVisitorBase):
             hasattr(True, False)  # E: incompatible_argument
 
         # The HasAttrGuard gets stripped.
-        inferred = AnnotatedValue(TypedValue(bool), [cast(Any, Anything)])
+        inferred = AnnotatedValue(TypedValue(bool), [cast(Any, ANY)])
 
         def only_on_class(o: object):
             val = hasattr(o, "__qualname__")
diff --git a/pyanalyze/test_typeshed.py b/pyanalyze/test_typeshed.py
index 9ba4759..6c68a40 100644
--- a/pyanalyze/test_typeshed.py
+++ b/pyanalyze/test_typeshed.py
@@ -14,7 +14,7 @@ from pathlib import Path
 from typing import Dict, Generic, List, NewType, TypeVar, Union
 from urllib.error import HTTPError
 
-from qcore.testing import Anything
+from unittest.mock import ANY
 from typeshed_client import get_search_context, Resolver
 
 from .checker import Checker
@@ -70,17 +70,15 @@ class TestTypeshedClient(TestNameCheckVisitorBase):
     def test_get_bases(self):
         tsf = TypeshedFinder(Checker(), verbose=True)
         assert [
-            GenericValue(MutableSequence, (TypeVarValue(typevar=Anything),)),
-            GenericValue(Generic, (TypeVarValue(typevar=Anything),)),
+            GenericValue(MutableSequence, (TypeVarValue(typevar=ANY),)),
+            GenericValue(Generic, (TypeVarValue(typevar=ANY),)),
         ] == tsf.get_bases(list)
         assert [
-            GenericValue(Collection, (TypeVarValue(typevar=Anything),)),
-            GenericValue(Reversible, (TypeVarValue(typevar=Anything),)),
-            GenericValue(Generic, (TypeVarValue(typevar=Anything),)),
+            GenericValue(Collection, (TypeVarValue(typevar=ANY),)),
+            GenericValue(Reversible, (TypeVarValue(typevar=ANY),)),
+            GenericValue(Generic, (TypeVarValue(typevar=ANY),)),
         ] == tsf.get_bases(Sequence)
-        assert [GenericValue(Collection, (TypeVarValue(Anything),))] == tsf.get_bases(
-            Set
-        )
+        assert [GenericValue(Collection, (TypeVarValue(ANY),))] == tsf.get_bases(Set)
 
     def test_newtype(self):
         with tempfile.TemporaryDirectory() as temp_dir_str: