Detect bad list.append calls without a varname (#522)

quora · Apr 14, 2022 · 5080a78 · 5080a78
1 parent 0f995f4
commit 5080a78
Show file tree

Hide file tree

Showing 4 changed files with 24 additions and 20 deletions.
diff --git a/docs/changelog.md b/docs/changelog.md
@@ -2,6 +2,8 @@
 
 ## Unreleased
 
+- Detect incompatible types for some calls to `list.append`,
+  `list.extend`, `list.__add__`, and `set.add` (#522)
 - Optimize local variables with very complex inferred types (#521)
 
 ## Version 0.7.0 (April 13, 2022)

diff --git a/pyanalyze/implementation.py b/pyanalyze/implementation.py
@@ -352,20 +352,20 @@ def inner(iterable: Value) -> Value:
 def _list_append_impl(ctx: CallContext) -> ImplReturn:
     lst = replace_known_sequence_value(ctx.vars["self"])
     element = ctx.vars["object"]
-    varname = ctx.visitor.varname_for_self_constraint(ctx.node)
-    if varname is not None:
-        if isinstance(lst, SequenceValue):
+    if isinstance(lst, SequenceValue):
+        varname = ctx.visitor.varname_for_self_constraint(ctx.node)
+        if varname is not None:
             no_return_unless = Constraint(
                 varname,
                 ConstraintType.is_value_object,
                 True,
                 SequenceValue.make_or_known(list, (*lst.members, (False, element))),
             )
             return ImplReturn(KnownValue(None), no_return_unless=no_return_unless)
-        elif isinstance(lst, GenericValue):
-            return _check_generic_container(
-                "list.append", "object", ctx.vars["self"], lst, element, ctx, list
-            )
+    if isinstance(lst, GenericValue):
+        return _check_generic_container(
+            "list.append", "object", ctx.vars["self"], lst, element, ctx, list
+        )
     return ImplReturn(KnownValue(None))
 
 
@@ -999,11 +999,7 @@ def inner(lst: Value, iterable: Value) -> ImplReturn:
                     varname, ConstraintType.is_value_object, True, constrained_value
                 )
                 return ImplReturn(KnownValue(None), no_return_unless=no_return_unless)
-        elif (
-            varname is not None
-            and isinstance(cleaned_lst, GenericValue)
-            and isinstance(iterable, TypedValue)
-        ):
+        if isinstance(cleaned_lst, GenericValue) and isinstance(iterable, TypedValue):
             actual_type = iterable.get_generic_arg_for_type(
                 collections.abc.Iterable, ctx.visitor, 0
             )
@@ -1058,9 +1054,9 @@ def _check_generic_container(
 def _set_add_impl(ctx: CallContext) -> ImplReturn:
     set_value = replace_known_sequence_value(ctx.vars["self"])
     element = ctx.vars["object"]
-    varname = ctx.visitor.varname_for_self_constraint(ctx.node)
-    if varname is not None:
-        if isinstance(set_value, SequenceValue):
+    if isinstance(set_value, SequenceValue):
+        varname = ctx.visitor.varname_for_self_constraint(ctx.node)
+        if varname is not None:
             no_return_unless = Constraint(
                 varname,
                 ConstraintType.is_value_object,
@@ -1070,10 +1066,10 @@ def _set_add_impl(ctx: CallContext) -> ImplReturn:
                 ),
             )
             return ImplReturn(KnownValue(None), no_return_unless=no_return_unless)
-        elif isinstance(set_value, GenericValue):
-            return _check_generic_container(
-                "set.add", "object", ctx.vars["self"], set_value, element, ctx, set
-            )
+    if isinstance(set_value, GenericValue):
+        return _check_generic_container(
+            "set.add", "object", ctx.vars["self"], set_value, element, ctx, set
+        )
     return ImplReturn(KnownValue(None))
 
 

diff --git a/pyanalyze/node_visitor.py b/pyanalyze/node_visitor.py
@@ -167,7 +167,7 @@ class BaseNodeVisitor(ast.NodeVisitor):
     should_check_environ_for_files: bool = True
     caught_errors: Optional[List[Dict[str, Any]]] = None
 
-    _changes_for_fixer = collections.defaultdict(list)
+    _changes_for_fixer: Dict[str, List[Replacement]] = collections.defaultdict(list)
 
     tree: ast.Module
     all_failures: List[Failure]

diff --git a/pyanalyze/test_implementation.py b/pyanalyze/test_implementation.py
@@ -419,6 +419,12 @@ def capybara():
             assert_is_value(lst, GenericValue(list, [TypedValue(str)]))
             lst.append(1)  # E: incompatible_argument
 
+            double_lst: List[List[int]] = [[42]]
+            assert_is_value(
+                double_lst, GenericValue(list, [GenericValue(list, [TypedValue(int)])])
+            )
+            double_lst[0].append("x")  # E: incompatible_argument
+
     @assert_passes()
     def test_set_add(self):
         from typing import Set