diff --git a/CONTRIBUTORS.txt b/CONTRIBUTORS.txt index cd15199a8a..420c624177 100644 --- a/CONTRIBUTORS.txt +++ b/CONTRIBUTORS.txt @@ -379,3 +379,5 @@ contributors: * Matthew Beckers (mattlbeck): contributor * Yang Yang: contributor + +* Andrew J. Simmons (anjsimmo): contributor diff --git a/ChangeLog b/ChangeLog index b794688b54..3ed3e66daf 100644 --- a/ChangeLog +++ b/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 diff --git a/pylint/checkers/variables.py b/pylint/checkers/variables.py index e93e1b3d08..62fadd2fdc 100644 --- a/pylint/checkers/variables.py +++ b/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 ) diff --git a/tests/functional/c/class_scope.py b/tests/functional/c/class_scope.py index 309ebd6da3..527e5efa28 100644 --- a/tests/functional/c/class_scope.py +++ b/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""" diff --git a/tests/functional/c/class_scope.txt b/tests/functional/c/class_scope.txt index ea6c45abf1..348c2b5103 100644 --- a/tests/functional/c/class_scope.txt +++ b/tests/functional/c/class_scope.txt @@ -1,4 +1,6 @@ +undefined-variable:11:Well.:Undefined variable 'revattr' used-before-assignment:11:Well.:Using variable 'revattr' before assignment undefined-variable:13:Well.: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' diff --git a/tests/unittest_checker_variables.py b/tests/unittest_checker_variables.py index 5c6769f4b1..121c8234f5 100644 --- a/tests/unittest_checker_variables.py +++ b/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):