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
Detect unused annotations in functions #668
Changes from 1 commit
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -600,6 +600,17 @@ def unusedAssignments(self): | |
isinstance(binding, Assignment)): | ||
yield name, binding | ||
|
||
def unusedAnnotations(self): | ||
""" | ||
Return a generator for the annotations which have not been used. | ||
""" | ||
for name, binding in self.items(): | ||
if (not binding.used and | ||
name not in self.globals and | ||
not self.usesLocals and | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I know this is copied from above, but are these still relevant? does annotating a global do anything? does There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Nah I like your point. Seems like it can be simpler, yet still do what we need? |
||
isinstance(binding, Annotation)): | ||
yield name, binding | ||
|
||
|
||
class GeneratorScope(Scope): | ||
pass | ||
|
@@ -1156,6 +1167,7 @@ def handleNodeLoad(self, node): | |
|
||
binding = scope.get(name, None) | ||
if isinstance(binding, Annotation) and not self._in_postponed_annotation: | ||
scope[name].used = True | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. i only really need this to be truthy, but should i define it as a tuple, like in the |
||
continue | ||
|
||
if name == 'print' and isinstance(binding, Builtin): | ||
|
@@ -2090,7 +2102,16 @@ def checkUnusedAssignments(): | |
""" | ||
for name, binding in self.scope.unusedAssignments(): | ||
self.report(messages.UnusedVariable, binding.source, name) | ||
|
||
def checkUnusedAnnotations(): | ||
""" | ||
Check to see if any annotations have not been used. | ||
""" | ||
for name, binding in self.scope.unusedAnnotations(): | ||
self.report(messages.UnusedAnnotation, binding.source, name) | ||
|
||
self.deferAssignment(checkUnusedAssignments) | ||
self.deferAssignment(checkUnusedAnnotations) | ||
|
||
self.popScope() | ||
|
||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -156,6 +156,25 @@ def __init__(self, filename, loc, names): | |
self.message_args = (names,) | ||
|
||
|
||
class UnusedAnnotation(Message): | ||
""" | ||
Indicates that a variable has been explicitly annotated to but not actually | ||
used. | ||
""" | ||
message = 'local variable %r is annotated but never used' | ||
|
||
def __init__(self, filename, loc, names): | ||
Message.__init__(self, filename, loc) | ||
self.message_args = (names,) | ||
|
||
|
||
class ReturnWithArgsInsideGenerator(Message): | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. merge conflict was resolved incorrectly it looks like |
||
""" | ||
Indicates a return statement with arguments inside a generator. | ||
""" | ||
message = '\'return\' with argument inside generator' | ||
|
||
|
||
class ReturnOutsideFunction(Message): | ||
""" | ||
Indicates a return statement outside of a function/method. | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -174,7 +174,7 @@ class C: | |
def f(): | ||
name: str | ||
age: int | ||
''') | ||
''', m.UnusedAnnotation, m.UnusedAnnotation) | ||
self.flakes(''' | ||
def f(): | ||
name: str = 'Bob' | ||
|
@@ -190,7 +190,7 @@ def f(): | |
from typing import Any | ||
def f(): | ||
a: Any | ||
''') | ||
''', m.UnusedAnnotation) | ||
self.flakes(''' | ||
foo: not_a_real_type | ||
''', m.UndefinedName) | ||
|
@@ -356,18 +356,24 @@ def test_unused_annotation(self): | |
class Cls: | ||
y: int | ||
''') | ||
# TODO: this should print a UnusedVariable message | ||
# This should print an UnusedAnnotation message | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. can just delete the comment now \o/ |
||
self.flakes(''' | ||
def f(): | ||
x: int | ||
''') | ||
''', m.UnusedAnnotation) | ||
# This should only print one UnusedVariable message | ||
self.flakes(''' | ||
def f(): | ||
x: int | ||
x = 3 | ||
''', m.UnusedVariable) | ||
|
||
def test_unassigned_annotation_is_undefined(self): | ||
self.flakes(''' | ||
name: str | ||
print(name) | ||
''', m.UndefinedName) | ||
|
||
def test_annotated_async_def(self): | ||
self.flakes(''' | ||
class c: pass | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
the camel case is legacy code -- new stuff uses snake case
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I felt awkward having
unused_annotations
be snake case whileunusedAssignments
was pascal case, so I made them both snake case