Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

add unused_assignment #511

Merged
merged 3 commits into from
Apr 8, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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