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

Global variables must be assigned to be used #666

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open
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
34 changes: 27 additions & 7 deletions pyflakes/checker.py
Original file line number Diff line number Diff line change
Expand Up @@ -250,10 +250,11 @@ class Binding:
the node that this binding was last used.
"""

def __init__(self, name, source):
def __init__(self, name, source, *, assigned=True):
self.name = name
self.source = source
self.used = False
self.assigned = assigned

def __str__(self):
return self.name
Expand Down Expand Up @@ -1007,6 +1008,12 @@ def addBinding(self, node, value):
break
existing = scope.get(value.name)

global_scope = self.scopeStack[-1]
if (existing and global_scope.get(value.name) == existing and
not existing.assigned):
# make sure the variable is in the global scope before setting as assigned
existing.assigned = True
Comment on lines +1011 to +1015
Copy link
Member

Choose a reason for hiding this comment

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

pyflakes running on itself is showing this is probably not quite right -- or at least it's not happy about nonlocal


if (existing and not isinstance(existing, Builtin) and
not self.differentForks(node, existing.source)):

Expand Down Expand Up @@ -1089,6 +1096,10 @@ def handleNodeLoad(self, node):
continue

binding = scope.get(name, None)

if binding and binding.assigned is False:
self.report(messages.UndefinedName, node, name)

if isinstance(binding, Annotation) and not self._in_postponed_annotation:
scope[name].used = True
continue
Expand Down Expand Up @@ -1159,12 +1170,19 @@ def handleNodeStore(self, node):
continue
# if the name was defined in that scope, and the name has
# been accessed already in the current scope, and hasn't
# been declared global
# been assigned globally
used = name in scope and scope[name].used
if used and used[0] is self.scope and name not in self.scope.globals:
# then it's probably a mistake
self.report(messages.UndefinedLocal,
scope[name].used[1], name, scope[name].source)

# and remove UndefinedName messages already reported for this name
self.messages = [
m for m in self.messages if not
isinstance(m, messages.UndefinedName) or
m.message_args[0] != name]

break

parent_stmt = self.getParent(node)
Expand Down Expand Up @@ -1836,7 +1854,7 @@ def ASSERT(self, node):
self.report(messages.AssertTuple, node)
self.handleChildren(node)

def GLOBAL(self, node):
def GLOBAL(self, node, assign_by_default=False):
"""
Keep track of globals declarations.
"""
Expand All @@ -1848,11 +1866,9 @@ def GLOBAL(self, node):

# One 'global' statement can bind multiple (comma-delimited) names.
for node_name in node.names:
node_value = Assignment(node_name, node)
node_value = Assignment(node_name, node, assigned=assign_by_default)

# Remove UndefinedName messages already reported for this name.
# TODO: if the global is not used in this scope, it does not
# become a globally defined name. See test_unused_global.
self.messages = [
m for m in self.messages if not
isinstance(m, messages.UndefinedName) or
Expand All @@ -1866,7 +1882,11 @@ def GLOBAL(self, node):
for scope in self.scopeStack[global_scope_index + 1:]:
scope[node_name] = node_value

NONLOCAL = GLOBAL
def NONLOCAL(self, node):
for node_name in node.names:
if not any(node_name in scope for scope in self.scopeStack[:-1]):
self.report(messages.NoBindingForNonlocal, node, node_name)
self.GLOBAL(node, assign_by_default=True)

def GENERATOREXP(self, node):
self.pushScope(GeneratorScope)
Expand Down
8 changes: 8 additions & 0 deletions pyflakes/messages.py
Original file line number Diff line number Diff line change
Expand Up @@ -73,6 +73,14 @@ def __init__(self, filename, loc, name):
self.message_args = (name,)


class NoBindingForNonlocal(Message):
message = 'no binding for nonlocal %r found'

def __init__(self, filename, loc, name):
Message.__init__(self, filename, loc)
self.message_args = (name,)


class DoctestSyntaxError(Message):
message = 'syntax error in doctest'

Expand Down
56 changes: 56 additions & 0 deletions pyflakes/test/test_undefined_names.py
Original file line number Diff line number Diff line change
Expand Up @@ -298,6 +298,62 @@ def c(): bar
def b(): global bar; bar = 1
''')

def test_unassigned_global_is_undefined(self):
"""
If a "global" is never given a value, it is undefined
"""
self.flakes('''
def a():
global fu
fu
''', m.UndefinedName)

self.flakes('''
global fu
fu
''', m.UndefinedName)

def test_scope_defined_global(self):
"""
If a "global" is defined inside of a function only,
outside of the function it is undefined
"""
self.flakes('''
global fu
def a():
fu = 1
fu
a()
fu
''', m.UndefinedName)

def test_scope_defined_nonlocal(self):
"""
If a "nonlocal" is declared in a previous scope,
it is defined
"""
self.flakes('''
def a():
fu = 1
def b():
nonlocal fu
fu
''')

def test_scope_undefined_nonlocal(self):
"""
If a "nonlocal" is never given a value, it is undefined
"""
self.flakes('''
def a():
nonlocal fu
''', m.NoBindingForNonlocal)

self.flakes('''
def a():
nonlocal fu, bar
''', m.NoBindingForNonlocal, m.NoBindingForNonlocal)

def test_definedByGlobalMultipleNames(self):
"""
"global" can accept multiple names.
Expand Down