Repository: quora/pyanalyze
Pull request #511 (2022-04-07 20:55:32)

add unused_assignment
Based on feedback that the existing error message was confusing.



Patch:
diff --git a/docs/changelog.md b/docs/changelog.md
index f0c6392..7dcec0d 100644
--- a/docs/changelog.md
+++ b/docs/changelog.md
@@ -2,6 +2,9 @@
 
 ## Unreleased
 
+- Add `unused_assignment` error code, separated out from
+  `unused_variable`. Enable these error codes and
+  `possibly_undefined_name` by default (#511)
 - Fix handling of overloaded methods called on literals (#513)
 - Partial support for running on Python 3.11 (#512)
 - Basic support for checking `Final` and for checking re-assignments
diff --git a/pyanalyze/error_code.py b/pyanalyze/error_code.py
index 1699a7a..65fbe31 100644
--- a/pyanalyze/error_code.py
+++ b/pyanalyze/error_code.py
@@ -95,6 +95,7 @@ class ErrorCode(enum.Enum):
     implicit_any = 77
     already_declared = 78
     invalid_annotated_assignment = 79
+    unused_assignment = 80
 
 
 # Allow testing unannotated functions without too much fuss
@@ -111,11 +112,8 @@ DISABLED_BY_DEFAULT = {
     *DISABLED_IN_TESTS,
     ErrorCode.method_first_arg,
     ErrorCode.value_always_true,
-    # TODO(jelle): This needs more work
-    ErrorCode.unused_variable,
     ErrorCode.use_fstrings,
     ErrorCode.unused_ignore,
-    ErrorCode.possibly_undefined_name,
     ErrorCode.missing_f,
     ErrorCode.bare_ignore,
     # TODO: turn this on
@@ -216,6 +214,7 @@ ERROR_DESCRIPTION = {
     ErrorCode.implicit_any: "Value is inferred as Any",
     ErrorCode.already_declared: "Name is already declared",
     ErrorCode.invalid_annotated_assignment: "Invalid annotated assignment",
+    ErrorCode.unused_assignment: "Assigned value is never used",
 }
 
 
diff --git a/pyanalyze/name_check_visitor.py b/pyanalyze/name_check_visitor.py
index c166afa..8a931a7 100644
--- a/pyanalyze/name_check_visitor.py
+++ b/pyanalyze/name_check_visitor.py
@@ -2033,12 +2033,23 @@ class NameCheckVisitor(node_visitor.ReplacingNodeVisitor):
                     # ignore assignments in AnnAssign nodes, which don't actually
                     # bind the name
                     continue
-            self._show_error_if_checking(
-                unused,
-                "Variable {} is not read after being written to".format(unused.id),
-                error_code=ErrorCode.unused_variable,
-                replacement=replacement,
-            )
+            if all(
+                node in all_unused_nodes
+                for node in scope.name_to_all_definition_nodes[unused.id]
+            ):
+                self._show_error_if_checking(
+                    unused,
+                    f"Variable {unused.id} is never accessed",
+                    error_code=ErrorCode.unused_variable,
+                    replacement=replacement,
+                )
+            else:
+                self._show_error_if_checking(
+                    unused,
+                    f"Assigned value of {unused.id} is never accessed",
+                    error_code=ErrorCode.unused_assignment,
+                    replacement=replacement,
+                )
 
     def value_of_annotation(self, node: ast.expr) -> Value:
         with qcore.override(self, "state", VisitorState.collect_names):


Test patch:
diff --git a/pyanalyze/test_name_check_visitor.py b/pyanalyze/test_name_check_visitor.py
index f785448..d063822 100644
--- a/pyanalyze/test_name_check_visitor.py
+++ b/pyanalyze/test_name_check_visitor.py
@@ -2162,26 +2162,26 @@ class TestContextManagerWithSuppression(TestNameCheckVisitorBase):
             assert_is_value(a, KnownValue(2) | KnownValue(3) | KnownValue(4))
 
         def use_empty_context():
-            a = 2  # E: unused_variable
+            a = 2  # E: unused_assignment
             with EmptyContext():
-                a = 3  # E: unused_variable
+                a = 3  # E: unused_assignment
                 a = 4
             assert_is_value(a, KnownValue(4))
 
         def use_context_manager():
-            a = 2  # E: unused_variable
+            a = 2  # E: unused_assignment
             with empty_context_manager():
                 a = 3
             assert_is_value(a, KnownValue(3))
 
         def use_builtin_function():
-            a = 2  # E: unused_variable
+            a = 2  # E: unused_assignment
             with open("test_file.txt"):
                 a = 3
             assert_is_value(a, KnownValue(3))
 
         def use_contextlib_manager():
-            a = 2  # E: unused_variable
+            a = 2  # E: unused_assignment
             with empty_contextlib_manager():
                 a = 3
             assert_is_value(a, KnownValue(3))
@@ -2192,7 +2192,7 @@ class TestContextManagerWithSuppression(TestNameCheckVisitorBase):
                 assert_is_value(b, KnownValue(None))
             assert_is_value(b, KnownValue(2) | KnownValue(None))
 
-            c = 2  # E: unused_variable
+            c = 2  # E: unused_assignment
             with EmptyContext() as c, SuppressException():
                 assert_is_value(c, KnownValue(None))
             assert_is_value(c, KnownValue(None))
@@ -2263,13 +2263,13 @@ class TestContextManagerWithSuppression(TestNameCheckVisitorBase):
             assert_is_value(a, KnownValue(2) | KnownValue(3))
 
         async def use_async_empty_context():
-            a = 2  # E: unused_variable
+            a = 2  # E: unused_assignment
             async with AsyncEmptyContext():
                 a = 3
             assert_is_value(a, KnownValue(3))
 
         async def use_async_context_manager():
-            a = 2  # E: unused_variable
+            a = 2  # E: unused_assignment
             async with async_empty_context_manager():
                 a = 3
             assert_is_value(a, KnownValue(3))
@@ -2280,7 +2280,7 @@ class TestContextManagerWithSuppression(TestNameCheckVisitorBase):
                 assert_is_value(b, KnownValue(None))
             assert_is_value(b, KnownValue(2) | KnownValue(None))
 
-            c = 2  # E: unused_variable
+            c = 2  # E: unused_assignment
             async with AsyncEmptyContext() as c, AsyncSuppressException():
                 assert_is_value(c, KnownValue(None))
             assert_is_value(c, KnownValue(None))
@@ -2295,7 +2295,7 @@ class TestContextManagerWithSuppression(TestNameCheckVisitorBase):
             yield
 
         async def use_async_contextlib_manager():
-            a = 2  # E: unused_variable
+            a = 2  # E: unused_assignment
             async with async_empty_contextlib_manager():
                 a = 3
             assert_is_value(a, KnownValue(3))
diff --git a/pyanalyze/test_stacked_scopes.py b/pyanalyze/test_stacked_scopes.py
index 64a2180..7f79df7 100644
--- a/pyanalyze/test_stacked_scopes.py
+++ b/pyanalyze/test_stacked_scopes.py
@@ -569,7 +569,7 @@ def capybara():
     @assert_passes()
     def test_unused_then_used(self):
         def capybara():
-            y = 3  # E: unused_variable
+            y = 3  # E: unused_assignment
             y = 4
             return y
 
@@ -577,7 +577,7 @@ def capybara():
     def test_unused_in_if(self):
         def capybara(condition):
             if condition:
-                x = 3  # E: unused_variable
+                x = 3  # E: unused_assignment
             x = 4
             return x