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

Fix scoping for function annotations, decorators and base classes #3713

Merged
merged 2 commits into from
Jul 12, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
4 changes: 4 additions & 0 deletions ChangeLog
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,10 @@ What's New in Pylint 2.6.0?

Release date: TBA

* Fix various scope-related bugs in ``undefined-variable`` checker

Close #1082, #3434, #3461

* bad-continuation and bad-whitespace have been removed, black or another formatter can help you with this better than Pylint

Close #246, #289, #638, #747, #1148, #1179, #1943, #2041, #2301, #2304, #2944, #3565
Expand Down
12 changes: 8 additions & 4 deletions pylint/checkers/utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -374,13 +374,17 @@ def is_defined_before(var_node: astroid.Name) -> bool:
return False


def is_default_argument(node: astroid.node_classes.NodeNG) -> bool:
def is_default_argument(
node: astroid.node_classes.NodeNG,
scope: Optional[astroid.node_classes.NodeNG] = None,
) -> bool:
"""return true if the given Name node is used in function or lambda
default argument's value
"""
parent = node.scope()
if isinstance(parent, (astroid.FunctionDef, astroid.Lambda)):
for default_node in parent.args.defaults:
if not scope:
scope = node.scope()
if isinstance(scope, (astroid.FunctionDef, astroid.Lambda)):
for default_node in scope.args.defaults:
for default_name_node in default_node.nodes_of_class(astroid.Name):
if default_name_node is node:
return True
Expand Down
52 changes: 33 additions & 19 deletions pylint/checkers/variables.py
Original file line number Diff line number Diff line change
Expand Up @@ -512,6 +512,7 @@ class NamesConsumer:

def __init__(self, node, scope_type):
self._atomic = ScopeConsumer(copy.copy(node.locals), {}, scope_type)
self.node = node

def __repr__(self):
msg = "\nto_consume : {:s}\n".format(
Expand Down Expand Up @@ -958,17 +959,7 @@ def visit_name(self, node):

name = node.name
frame = stmt.scope()
# if the name node is used as a function default argument's value or as
# a decorator, then start from the parent frame of the function instead
# of the function frame - and thus open an inner class scope
if (
utils.is_default_argument(node)
or utils.is_func_decorator(node)
or utils.is_ancestor_name(frame, node)
):
start_index = len(self._to_consume) - 2
else:
start_index = len(self._to_consume) - 1
start_index = len(self._to_consume) - 1

undefined_variable_is_enabled = self.linter.is_message_enabled(
"undefined-variable"
Expand All @@ -982,16 +973,33 @@ def visit_name(self, node):
# pylint: disable=too-many-nested-blocks; refactoring this block is a pain.
for i in range(start_index, -1, -1):
current_consumer = self._to_consume[i]
# if the current scope is a class scope but it's not the inner

# The list of base classes in the class definition is not part
# of the class body.
# 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.
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
if current_consumer.scope_type == "class" and (
utils.is_ancestor_name(current_consumer.node, node)
or (i != start_index and self._ignore_class_scope(node))
):
continue

# if the name node is used as a function default argument's value or as
# a decorator, then start from the parent frame of the function instead
# of the function frame - and thus open an inner class scope
if (
current_consumer.scope_type == "function"
and self._defined_in_function_definition(node, current_consumer.node)
):
# ignore function scope if is an annotation/default/decorator, as not in the body
continue

if current_consumer.scope_type == "lambda" and utils.is_default_argument(
node, current_consumer.node
):
continue

# the name has already been consumed, only check it's not a loop
# variable used outside the loop
Expand Down Expand Up @@ -1463,13 +1471,19 @@ def _ignore_class_scope(self, node):
# tp = 2
# def func(self, arg=tp):
# ...
# class C:
# class Tp:
# pass
# class D(Tp):
# ...

name = node.name
frame = node.statement().scope()
in_annotation_or_default_or_decorator = self._defined_in_function_definition(
node, frame
)
if in_annotation_or_default_or_decorator:
in_ancestor_list = utils.is_ancestor_name(frame, node)
if in_annotation_or_default_or_decorator or in_ancestor_list:
frame_locals = frame.parent.scope().locals
else:
frame_locals = frame.locals
Expand Down
20 changes: 20 additions & 0 deletions tests/checkers/unittest_variables.py
Original file line number Diff line number Diff line change
Expand Up @@ -152,6 +152,26 @@ def fun(self):
with self.assertNoMessages():
self.walk(module)

def test_listcomp_in_ancestors(self):
""" Ensure list comprehensions in base classes are scoped correctly

https://github.com/PyCQA/pylint/issues/3434
"""
module = astroid.parse(
"""
import collections


l = ["a","b","c"]


class Foo(collections.namedtuple("Foo",[x+"_foo" for x in l])):
pass
"""
)
with self.assertNoMessages():
self.walk(module)

def test_return_type_annotation(self):
""" Make sure class attributes in scope for return type annotations.

Expand Down
20 changes: 20 additions & 0 deletions tests/functional/c/class_scope.py
Original file line number Diff line number Diff line change
Expand Up @@ -21,3 +21,23 @@ class Sub(Data):
def func(self):
"""check Sub is not defined here"""
return Sub(), self # [undefined-variable]


class Right:
"""right"""
class Result1:
"""result one"""
OK = 0
def work(self) -> Result1:
"""good type hint"""
return self.Result1.OK


class Wrong:
"""wrong"""
class Result2:
"""result two"""
OK = 0
def work(self) -> self.Result2: # [undefined-variable]
"""bad type hint"""
return self.Result2.OK
1 change: 1 addition & 0 deletions tests/functional/c/class_scope.txt
Original file line number Diff line number Diff line change
Expand Up @@ -4,3 +4,4 @@ undefined-variable:13:Well.<lambda>:Undefined variable 'get_attr_bad'
undefined-variable:14:Well:Undefined variable 'attr'
undefined-variable:20:Well.Sub:Undefined variable 'Data'
undefined-variable:23:Well.func:Undefined variable 'Sub'
undefined-variable:41:Wrong.work:Undefined variable 'self'
7 changes: 7 additions & 0 deletions tests/functional/u/undefined_variable.py
Original file line number Diff line number Diff line change
Expand Up @@ -287,3 +287,10 @@ class DunderClass:
def method(self):
# This name is not defined in the AST but it's present at runtime
return __class__


def undefined_annotation(a:x): # [undefined-variable]
if x == 2: # [used-before-assignment]
for x in [1, 2]:
pass
return a
2 changes: 2 additions & 0 deletions tests/functional/u/undefined_variable.txt
Original file line number Diff line number Diff line change
Expand Up @@ -26,3 +26,5 @@ undefined-variable:225:LambdaClass4.<lambda>:Undefined variable 'LambdaClass4'
undefined-variable:233:LambdaClass5.<lambda>:Undefined variable 'LambdaClass5'
used-before-assignment:254:func_should_fail:Using variable 'datetime' before assignment
undefined-variable:281:not_using_loop_variable_accordingly:Undefined variable 'iteree'
undefined-variable:292:undefined_annotation:Undefined variable 'x'
used-before-assignment:293:undefined_annotation:Using variable 'x' before assignment