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

Detect unused annotations in functions #668

Merged
merged 2 commits into from Sep 16, 2022

Conversation

dannysepler
Copy link
Contributor

Closes #546

@@ -1208,6 +1219,7 @@ def handleNodeLoad(self, node):

binding = scope.get(name, None)
if isinstance(binding, Annotation) and not self._in_postponed_annotation:
scope[name].used = True
Copy link
Contributor Author

Choose a reason for hiding this comment

The 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 try below?

@@ -600,6 +600,17 @@ def unusedAssignments(self):
isinstance(binding, Assignment)):
yield name, binding

def unusedAnnotations(self):
Copy link
Member

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

Copy link
Contributor Author

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 while unusedAssignments was pascal case, so I made them both snake case

Comment on lines 609 to 610
name not in self.globals and
not self.usesLocals and
Copy link
Member

Choose a reason for hiding this comment

The 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 locals() include annotated-only things?

Copy link
Contributor Author

Choose a reason for hiding this comment

The 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?

self.message_args = (names,)


class ReturnWithArgsInsideGenerator(Message):
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

merge conflict was resolved incorrectly it looks like

@@ -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
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can just delete the comment now \o/

Copy link
Member

@asottile asottile left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@asottile asottile merged commit 4a2407d into PyCQA:master Sep 16, 2022
@dannysepler dannysepler deleted the unused-annotation branch October 6, 2022 05:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Is an unused variable annotation able to be detected?
2 participants