Fix class scope issues in undefined-variable checker #511 #1976 #3494

CONTRIBUTORS.txt

@@ -379,3 +379,5 @@ contributors:
 * Matthew Beckers (mattlbeck): contributor
 
 * Yang Yang: contributor
+
+* Andrew J. Simmons (anjsimmo): contributor

ChangeLog

@@ -7,6 +7,15 @@ What's New in Pylint 2.5.0?
 
 Release date: TBA
 
+* Fix a false negative for ``undefined-variable`` when using class attribute in comprehension.
+
+  Close #3494
+
+* Fix a false positive for ``undefined-variable`` when using class attribute in decorator or as type hint.
+
+  Close #511
+  Close #1976
+
 * Remove HTML quoting of messages in JSON output.
 
   Close #2769

pylint/checkers/variables.py

@@ -973,13 +973,11 @@ def visit_name(self, node):
             # if the current scope is a class scope but it's not the inner
             # scope, ignore it. This prevents to access this scope instead of
             # the globals one in function members when there are some common
-            # names. The only exception is when the starting scope is a
-            # comprehension and its direct outer scope is a class
-            if (
-                current_consumer.scope_type == "class"
-                and i != start_index
-                and not (base_scope_type == "comprehension" and i == start_index - 1)
-            ):
+            # names.
+            if current_consumer.scope_type == "class" and i != start_index:
+                # The only exceptions are: when the variable forms an iter within a
+                # comprehension scope; and/or when used as a default, decorator,
+                # or annotation within a function.
                 if self._ignore_class_scope(node):
                     continue
 
@@ -1249,16 +1247,53 @@ def _allow_global_unused_variables(self):
 
     @staticmethod
     def _defined_in_function_definition(node, frame):
-        in_annotation_or_default = False
+        in_annotation_or_default_or_decorator = False
         if isinstance(frame, astroid.FunctionDef) and node.statement() is frame:
-            in_annotation_or_default = (
-                node in frame.args.annotations
-                or node in frame.args.posonlyargs_annotations
-                or node in frame.args.kwonlyargs_annotations
-                or node is frame.args.varargannotation
-                or node is frame.args.kwargannotation
-            ) or frame.args.parent_of(node)
-        return in_annotation_or_default
+            in_annotation_or_default_or_decorator = (
+                (
+                    node in frame.args.annotations
+                    or node in frame.args.posonlyargs_annotations
+                    or node in frame.args.kwonlyargs_annotations
+                    or node is frame.args.varargannotation
+                    or node is frame.args.kwargannotation
+                )
+                or frame.args.parent_of(node)
+                or (frame.decorators and frame.decorators.parent_of(node))
+                or (
+                    frame.returns
+                    and (node is frame.returns or frame.returns.parent_of(node))
+                )
+            )
+        return in_annotation_or_default_or_decorator
+
+    @staticmethod
+    def _in_lambda_or_comprehension_body(
+        node: astroid.node_classes.NodeNG, frame: astroid.node_classes.NodeNG
+    ) -> bool:
+        """return True if node within a lambda/comprehension body (or similar) and thus should not have access to class attributes in frame"""
+        child = node
+        parent = node.parent
+        while parent is not None:
+            if parent is frame:
+                return False
+            if isinstance(parent, astroid.Lambda) and not child is parent.args:
+                # Body of lambda should not have access to class attributes.
+                return True
+            if (
+                isinstance(parent, astroid.node_classes.Comprehension)
+                and not child is parent.iter
+            ):
+                # Only iter of list/set/dict/generator comprehension should have access.
+                return True
+            if isinstance(parent, astroid.scoped_nodes.ComprehensionScope) and not (
+                parent.generators and child is parent.generators[0]
+            ):
+                # Body of list/set/dict/generator comprehension should not have access to class attributes.
+                # Furthermore, only the first generator (if multiple) in comprehension should have access.
+                return True
+            child = parent
+            parent = parent.parent
+        return False
 
     @staticmethod
     def _is_variable_violation(
@@ -1419,13 +1454,19 @@ def _ignore_class_scope(self, node):
 
         name = node.name
         frame = node.statement().scope()
-        in_annotation_or_default = self._defined_in_function_definition(node, frame)
-        if in_annotation_or_default:
+        in_annotation_or_default_or_decorator = self._defined_in_function_definition(
+            node, frame
+        )
+        if in_annotation_or_default_or_decorator:
             frame_locals = frame.parent.scope().locals
         else:
             frame_locals = frame.locals
         return not (
-            (isinstance(frame, astroid.ClassDef) or in_annotation_or_default)
+            (
+                isinstance(frame, astroid.ClassDef)
+                or in_annotation_or_default_or_decorator
+            )
+            and not self._in_lambda_or_comprehension_body(node, frame)
             and name in frame_locals
         )
 

tests/functional/c/class_scope.py

@@ -7,10 +7,11 @@ class Well(object):
     """well"""
     attr = 42
     get_attr = lambda arg=attr: arg * 24
-    # +1: [used-before-assignment]
+    # +1: [undefined-variable, used-before-assignment]
     get_attr_bad = lambda arg=revattr: revattr * 42
     revattr = 24
     bad_lambda = lambda: get_attr_bad # [undefined-variable]
+    bad_gen = list(attr + i for i in range(10)) # [undefined-variable]
 
     class Data(object):
         """base hidden class"""

tests/functional/c/class_scope.txt

@@ -1,4 +1,6 @@
+undefined-variable:11:Well.<lambda>:Undefined variable 'revattr'
 used-before-assignment:11:Well.<lambda>:Using variable 'revattr' before assignment
 undefined-variable:13:Well.<lambda>:Undefined variable 'get_attr_bad'
-undefined-variable:19:Well.Sub:Undefined variable 'Data'
-undefined-variable:22:Well.func:Undefined variable 'Sub'
+undefined-variable:14:Well:Undefined variable 'attr'
+undefined-variable:20:Well.Sub:Undefined variable 'Data'
+undefined-variable:23:Well.func:Undefined variable 'Sub'

tests/unittest_checker_variables.py

@@ -119,6 +119,49 @@ def func():
         with self.assertAddsMessages(msg):
             self.checker.visit_global(node)
 
+    def test_listcomp_in_decorator(self):
+        """ Make sure class attributes in scope for listcomp in decorator.
+
+        https://github.com/PyCQA/pylint/issues/511
+        """
+        module = astroid.parse(
+            """
+        def dec(inp):
+            def inner(func):
+                print(inp)
+                return func
+            return inner
+
+
+        class Cls:
+
+            DATA = "foo"
+
+            @dec([x for x in DATA])
+            def fun(self):
+                pass
+        """
+        )
+        with self.assertNoMessages():
+            self.walk(module)
+
+    def test_return_type_annotation(self):
+        """ Make sure class attributes in scope for return type annotations.
+
+        https://github.com/PyCQA/pylint/issues/1976
+        """
+        module = astroid.parse(
+            """
+        class MyObject:
+            class MyType:
+                pass
+            def my_method(self) -> MyType:
+                pass
+        """
+        )
+        with self.assertNoMessages():
+            self.walk(module)
+
 
 class TestVariablesCheckerWithTearDown(CheckerTestCase):