Repository: quora/pyanalyze
Pull request #726 (2024-02-24 21:32:46)

Improve dict.__delitem__ impl


Patch:
diff --git a/docs/changelog.md b/docs/changelog.md
index a85063d..64261bd 100644
--- a/docs/changelog.md
+++ b/docs/changelog.md
@@ -2,6 +2,7 @@
 
 ## Unreleased
 
+- Add special handling for `dict.__delitem__` (#723, #726)
 - Add support for the `ReadOnly` type qualifier (PEP 705) and
   for the `closed=True` TypedDict argument (PEP 728) (#723)
 - Fix some higher-order behavior of `TypeGuard` and `TypeIs` (#719)
diff --git a/pyanalyze/implementation.py b/pyanalyze/implementation.py
index 6336f94..0421fe1 100644
--- a/pyanalyze/implementation.py
+++ b/pyanalyze/implementation.py
@@ -753,7 +753,7 @@ def _dict_get_impl(ctx: CallContext) -> ImplReturn:
 
 def _dict_delitem_impl(ctx: CallContext) -> ImplReturn:
     key = ctx.vars["key"]
-    varname = ctx.visitor.varname_for_self_constraint(ctx.node)
+    varname = ctx.varname_for_arg("self")
     self_value = replace_known_sequence_value(ctx.vars["self"])
 
     if not _check_dict_key_hashability(key, ctx, "key"):
@@ -775,16 +775,16 @@ def _dict_delitem_impl(ctx: CallContext) -> ImplReturn:
             except Exception:
                 pass
             else:
-                if entry.required:
+                if entry.readonly:
                     ctx.show_error(
-                        f"Cannot delete required TypedDict key {key}",
-                        error_code=ErrorCode.incompatible_argument,
+                        f"Cannot delete readonly TypedDict key {key}",
+                        error_code=ErrorCode.readonly_typeddict,
                         arg="key",
                     )
-                elif entry.readonly:
+                elif entry.required:
                     ctx.show_error(
-                        f"Cannot delete readonly TypedDict key {key}",
-                        error_code=ErrorCode.readonly_typeddict,
+                        f"Cannot delete required TypedDict key {key}",
+                        error_code=ErrorCode.incompatible_argument,
                         arg="key",
                     )
                 return ImplReturn(KnownValue(None))
@@ -814,11 +814,7 @@ def _dict_delitem_impl(ctx: CallContext) -> ImplReturn:
         else:
             no_return_unless = NULL_CONSTRAINT
         if not is_present:
-            ctx.show_error(
-                f"Key {key} does not exist in dictionary {self_value}",
-                error_code=ErrorCode.incompatible_argument,
-                arg="key",
-            )
+            # No error; it might have been added where we couldn't see it
             return ImplReturn(KnownValue(None))
         return ImplReturn(KnownValue(None), no_return_unless=no_return_unless)
     elif isinstance(self_value, TypedValue):


Test patch:
diff --git a/pyanalyze/test_implementation.py b/pyanalyze/test_implementation.py
index f056dc6..d5d5dc0 100644
--- a/pyanalyze/test_implementation.py
+++ b/pyanalyze/test_implementation.py
@@ -921,6 +921,96 @@ class TestGenericMutators(TestNameCheckVisitorBase):
             )
 
 
+class TestDictDelitem(TestNameCheckVisitorBase):
+    @assert_passes()
+    def test_incomplete(self) -> None:
+        d1 = {}
+        d2 = {"a": 1, "b": 2}
+
+        def capybara() -> None:
+            del d1["a"]  # ok
+            assert_is_value(d1, KnownValue({}))
+            del d2["a"]  # ok
+            assert_is_value(
+                d2, DictIncompleteValue(dict, [KVPair(KnownValue("b"), KnownValue(2))])
+            )
+
+        def pacarana() -> None:
+            d = {"a": 1, "b": 2}
+            del d["a"]
+            assert_is_value(
+                d, DictIncompleteValue(dict, [KVPair(KnownValue("b"), KnownValue(2))])
+            )
+            del d["c"]  # ok
+            assert_is_value(
+                d, DictIncompleteValue(dict, [KVPair(KnownValue("b"), KnownValue(2))])
+            )
+
+    @assert_passes()
+    def test_typed(self) -> None:
+        from typing import Any, Dict
+
+        def capybara(d: Dict[str, int]) -> None:
+            del d["x"]
+            del d[1]  # E: incompatible_argument
+
+        def pacarana(d: Dict[Any, Any]) -> None:
+            del d["x"]
+            del d[1]
+            del d[{}]  # E: unhashable_key
+
+    @assert_passes()
+    def test_typeddict(self) -> None:
+        from typing_extensions import NotRequired, ReadOnly, TypedDict
+
+        class TD(TypedDict):
+            a: str
+            b: NotRequired[int]
+            c: ReadOnly[str]
+
+        class ClosedTD(TypedDict, closed=True):
+            a: str
+            b: NotRequired[int]
+
+        class ExtraItemsTD(TypedDict, closed=True):
+            a: str
+            b: NotRequired[int]
+            __extra_items__: int
+
+        class ReadOnlyExtraItemsTD(TypedDict, closed=True):
+            a: str
+            b: NotRequired[int]
+            __extra_items__: ReadOnly[int]
+
+        def capybara(
+            td: TD,
+            closed: ClosedTD,
+            extra_items: ExtraItemsTD,
+            readonly_extra: ReadOnlyExtraItemsTD,
+            s: str,
+        ) -> None:
+            del td[1]  # E: invalid_typeddict_key
+            del td["a"]  # E: incompatible_argument
+            del td["b"]  # ok
+            del td["c"]  # E: readonly_typeddict
+            del td[s]  # E: invalid_typeddict_key
+
+            del closed["a"]  # E: incompatible_argument
+            del closed["b"]  # ok
+            del closed["c"]  # E: invalid_typeddict_key
+            del closed[s]  # E: invalid_typeddict_key
+
+            del extra_items["a"]  # E: incompatible_argument
+            del extra_items["b"]  # ok
+            del extra_items["c"]  # ok
+            del extra_items[s]  # ok
+
+            del readonly_extra["a"]  # E: incompatible_argument
+            del readonly_extra["b"]  # ok
+            del readonly_extra["c"]  # E: readonly_typeddict
+            del readonly_extra[s]  # E: readonly_typeddict
+
+
 class TestSequenceGetItem(TestNameCheckVisitorBase):
     @assert_passes()
     def test_list(self):
diff --git a/pyanalyze/test_typeddict.py b/pyanalyze/test_typeddict.py
index 441e6d6..090a68d 100644
--- a/pyanalyze/test_typeddict.py
+++ b/pyanalyze/test_typeddict.py
@@ -288,7 +288,7 @@ class TestReadOnly(TestNameCheckVisitorBase):
             td.pop("a")  # E: readonly_typeddict
             td.pop("b")  # E: incompatible_argument
             del td["a"]  # E: readonly_typeddict
-            del td["b"]  # E: incompatible_argument
+            del td["b"]  # E: readonly_typeddict
 
     @assert_passes()
     def test_compatibility(self):