Skip to content

Commit

Permalink
add unused_assignment (#511)
Browse files Browse the repository at this point in the history
  • Loading branch information
JelleZijlstra authored Apr 8, 2022
1 parent 15b7d32 commit e21d2ae
Show file tree
Hide file tree
Showing 5 changed files with 34 additions and 21 deletions.
3 changes: 3 additions & 0 deletions docs/changelog.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
5 changes: 2 additions & 3 deletions pyanalyze/error_code.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -111,11 +112,8 @@ class ErrorCode(enum.Enum):
*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
Expand Down Expand Up @@ -216,6 +214,7 @@ class ErrorCode(enum.Enum):
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",
}


Expand Down
23 changes: 17 additions & 6 deletions pyanalyze/name_check_visitor.py
Original file line number Diff line number Diff line change
Expand Up @@ -2033,12 +2033,23 @@ def _check_function_unused_vars(
# 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):
Expand Down
20 changes: 10 additions & 10 deletions pyanalyze/test_name_check_visitor.py
Original file line number Diff line number Diff line change
Expand Up @@ -2162,26 +2162,26 @@ def use_suppress_exception_multi_assignment():
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))
Expand All @@ -2192,7 +2192,7 @@ def use_nested_contexts():
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))
Expand Down Expand Up @@ -2263,13 +2263,13 @@ async def use_async_suppress_exception():
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))
Expand All @@ -2280,7 +2280,7 @@ async def use_async_nested_contexts():
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))
Expand All @@ -2295,7 +2295,7 @@ async def async_empty_contextlib_manager() -> AsyncIterator[None]:
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))
4 changes: 2 additions & 2 deletions pyanalyze/test_stacked_scopes.py
Original file line number Diff line number Diff line change
Expand Up @@ -569,15 +569,15 @@ 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

@assert_passes()
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

Expand Down

0 comments on commit e21d2ae

Please sign in to comment.