Fix undefined name in annotations (#535)

* Fix undefined name in annotations

Variable annotations without a value don't create a name, but they still
can be used as variable annotation themselves as long as the annotation
is quoted or "from __future__ import annotations" is used.

The implementation introduces a new binding "Annotation" for these kinds
of variable annotations. This can potentially be used for further
annotation-related checks in the future.

Annotation handling is extended to be able to detect both forms of postponed
annotations (quoted and future import) during checks.

Fixes #486

* Fix wrong tuple syntax

* Refactor annotation handling

* Introduce AnnotationState "enum"
* Pass AnnotationState to _enter_annotation() instead of a "string" flag

* Add a test for unused annotations

* Add a few more tests for unused variables

Author: srittau

pyflakes/checker.py

@@ -79,6 +79,10 @@ def getAlternatives(n):
     LOOP_TYPES = (ast.While, ast.For)
     FUNCTION_TYPES = (ast.FunctionDef,)
 
+if PY36_PLUS:
+    ANNASSIGN_TYPES = (ast.AnnAssign,)
+else:
+    ANNASSIGN_TYPES = ()
 
 if PY38_PLUS:
     def _is_singleton(node):  # type: (ast.AST) -> bool
@@ -528,6 +532,16 @@ class Assignment(Binding):
     """
 
 
+class Annotation(Binding):
+    """
+    Represents binding a name to a type without an associated value.
+
+    As long as this name is not assigned a value in another binding, it is considered
+    undefined for most purposes. One notable exception is using the name as a type
+    annotation.
+    """
+
+
 class FunctionDefinition(Definition):
     pass
 
@@ -732,6 +746,12 @@ def is_typing_overload(value, scope_stack):
     )
 
 
+class AnnotationState:
+    NONE = 0
+    STRING = 1
+    BARE = 2
+
+
 def in_annotation(func):
     @functools.wraps(func)
     def in_annotation_func(self, *args, **kwargs):
@@ -740,6 +760,14 @@ def in_annotation_func(self, *args, **kwargs):
     return in_annotation_func
 
 
+def in_string_annotation(func):
+    @functools.wraps(func)
+    def in_annotation_func(self, *args, **kwargs):
+        with self._enter_annotation(AnnotationState.STRING):
+            return func(self, *args, **kwargs)
+    return in_annotation_func
+
+
 def make_tokens(code):
     # PY3: tokenize.tokenize requires readline of bytes
     if not isinstance(code, bytes):
@@ -826,7 +854,7 @@ class Checker(object):
     nodeDepth = 0
     offset = None
     traceTree = False
-    _in_annotation = False
+    _in_annotation = AnnotationState.NONE
     _in_typing_literal = False
     _in_deferred = False
 
@@ -1146,8 +1174,11 @@ def handleNodeLoad(self, node):
                     # iteration
                     continue
 
-            if (name == 'print' and
-                    isinstance(scope.get(name, None), Builtin)):
+            binding = scope.get(name, None)
+            if isinstance(binding, Annotation) and not self._in_postponed_annotation:
+                continue
+
+            if name == 'print' and isinstance(binding, Builtin):
                 parent = self.getParent(node)
                 if (isinstance(parent, ast.BinOp) and
                         isinstance(parent.op, ast.RShift)):
@@ -1222,7 +1253,9 @@ def handleNodeStore(self, node):
                     break
 
         parent_stmt = self.getParent(node)
-        if isinstance(parent_stmt, (FOR_TYPES, ast.comprehension)) or (
+        if isinstance(parent_stmt, ANNASSIGN_TYPES) and parent_stmt.value is None:
+            binding = Annotation(name, node)
+        elif isinstance(parent_stmt, (FOR_TYPES, ast.comprehension)) or (
                 parent_stmt != node._pyflakes_parent and
                 not self.isLiteralTupleUnpacking(parent_stmt)):
             binding = Binding(name, node)
@@ -1265,13 +1298,20 @@ def on_conditional_branch():
                 self.report(messages.UndefinedName, node, name)
 
     @contextlib.contextmanager
-    def _enter_annotation(self):
-        orig, self._in_annotation = self._in_annotation, True
+    def _enter_annotation(self, ann_type=AnnotationState.BARE):
+        orig, self._in_annotation = self._in_annotation, ann_type
         try:
             yield
         finally:
             self._in_annotation = orig
 
+    @property
+    def _in_postponed_annotation(self):
+        return (
+            self._in_annotation == AnnotationState.STRING or
+            self.annotationsFutureEnabled
+        )
+
     def _handle_type_comments(self, node):
         for (lineno, col_offset), comment in self._type_comments.get(node, ()):
             comment = comment.split(':', 1)[1].strip()
@@ -1399,7 +1439,7 @@ def handleDoctests(self, node):
         self.popScope()
         self.scopeStack = saved_stack
 
-    @in_annotation
+    @in_string_annotation
     def handleStringAnnotation(self, s, node, ref_lineno, ref_col_offset, err):
         try:
             tree = ast.parse(s)
@@ -1611,7 +1651,7 @@ def CALL(self, node):
             len(node.args) >= 1 and
             isinstance(node.args[0], ast.Str)
         ):
-            with self._enter_annotation():
+            with self._enter_annotation(AnnotationState.STRING):
                 self.handleNode(node.args[0], node)
 
         self.handleChildren(node)
@@ -2224,11 +2264,7 @@ def EXCEPTHANDLER(self, node):
             self.scope[node.name] = prev_definition
 
     def ANNASSIGN(self, node):
-        if node.value:
-            # Only bind the *targets* if the assignment has a value.
-            # Otherwise it's not really ast.Store and shouldn't silence
-            # UndefinedLocal warnings.
-            self.handleNode(node.target, node)
+        self.handleNode(node.target, node)
         self.handleAnnotation(node.annotation, node)
         if node.value:
             # If the assignment has value, handle the *value* now.

pyflakes/test/test_type_annotations.py

@@ -263,6 +263,14 @@ class A: pass
         class A: pass
         ''')
         self.flakes('''
+        T: object
+        def f(t: T): pass
+        ''', m.UndefinedName)
+        self.flakes('''
+        T: object
+        def g(t: 'T'): pass
+        ''')
+        self.flakes('''
         a: 'A B'
         ''', m.ForwardAnnotationSyntaxError)
         self.flakes('''
@@ -275,6 +283,26 @@ class A: pass
         a: 'a: "A"'
         ''', m.ForwardAnnotationSyntaxError)
 
+    @skipIf(version_info < (3, 6), 'new in Python 3.6')
+    def test_unused_annotation(self):
+        # Unused annotations are fine in module and class scope
+        self.flakes('''
+        x: int
+        class Cls:
+            y: int
+        ''')
+        # TODO: this should print a UnusedVariable message
+        self.flakes('''
+        def f():
+            x: int
+        ''')
+        # This should only print one UnusedVariable message
+        self.flakes('''
+        def f():
+            x: int
+            x = 3
+        ''', m.UnusedVariable)
+
     @skipIf(version_info < (3, 5), 'new in Python 3.5')
     def test_annotated_async_def(self):
         self.flakes('''
@@ -300,6 +328,13 @@ class A:
         class B: pass
         ''', m.UndefinedName)
 
+        self.flakes('''
+        from __future__ import annotations
+        T: object
+        def f(t: T): pass
+        def g(t: 'T'): pass
+        ''')
+
     def test_typeCommentsMarkImportsAsUsed(self):
         self.flakes("""
         from mod import A, B, C, D, E, F, G