Global variables must be assigned to be used #666
Conversation
pyflakes/checker.py
Outdated
| self.name = name | ||
| self.source = source | ||
| self.used = False | ||
| self.isUnassigned = isUnassigned |
There was a problem hiding this comment.
new code should use snake_case -- we're slowly migrating over the legacy code which uses the old casing
pyflakes/checker.py
Outdated
| @@ -322,10 +322,11 @@ class Binding(object): | |||
| the node that this binding was last used. | |||
| """ | |||
|
|
|||
| def __init__(self, name, source): | |||
| def __init__(self, name, source, isUnassigned=False): | |||
There was a problem hiding this comment.
this is a bit of a double-negative and a boolean trap -- it would probably be best to change this to assigned=True and make it keyword-only
There was a problem hiding this comment.
sure! should i wait for #660 to be merged before introducing a keyword-only arg?
pyflakes/checker.py
Outdated
| in_global_scope = value.name in self.scopeStack[-1] | ||
| if existing and in_global_scope and existing.isUnassigned: | ||
| existing.isUnassigned = False |
There was a problem hiding this comment.
I don't think this is correct -- for instance a closure-scoped variable can shadow a global variable (and is a separate scope)
There was a problem hiding this comment.
sweet, added a test case for that
pyflakes/checker.py
Outdated
| except AttributeError: | ||
| pass |
There was a problem hiding this comment.
we shouldn't be blindly catching AttributeError -- this is a programming error
| def test_globalIsUndefinedIfNeverGivenValue(self): | ||
| """ | ||
| If a "global" is never given a value, it is undefined | ||
| """ | ||
| self.flakes(''' |
There was a problem hiding this comment.
I don't think this is necessarily true, a function later in the module could modify that variable and give it a value
| def test_unusedGlobalAcrossFunctions(self): | ||
| """ | ||
| An unused "global" statement in another function does not define the name. | ||
| """ | ||
| self.flakes(''' | ||
| def f1(): | ||
| m | ||
| global m | ||
|
|
||
| def f2(): | ||
| global m | ||
| m | ||
| ''', m.UndefinedName) |
There was a problem hiding this comment.
this test shouldn't have been altered -- it was demonstrating a behaviour that should happen
There was a problem hiding this comment.
at the moment, this is the one part of the diff i'm still iffy about. you're right, this should eventually just be un-skipped. pyflakes at the moment only errors when:
def f1():
m
def f2():
global mand it does not error if the global is defined in f1
for now i've just gone back to skipping it and reverted my changes, not sure if this is some bigger thing with pyflakes or if it's a simple fix i can do in here!
| @@ -499,7 +515,7 @@ def fun2(): | |||
| a | |||
| a = 2 | |||
| return a | |||
| ''', m.UndefinedLocal) | |||
| ''', m.UndefinedLocal, m.UndefinedName) | |||
There was a problem hiding this comment.
I don't think this new error here is correct -- it seems to be correct on the main branch
dannysepler
force-pushed
the
global-keyword
branch
2 times, most recently
from
bd0a4d6
to
a891547
Compare
dannysepler
force-pushed
the
global-keyword
branch
from
a891547
to
bcf8194
Compare
pyflakes/checker.py
Outdated
| @@ -1155,6 +1162,10 @@ def handleNodeLoad(self, node): | |||
| continue | |||
|
|
|||
| binding = scope.get(name, None) | |||
|
|
|||
| if getattr(binding, 'assigned', None) is False: | |||
There was a problem hiding this comment.
ah -- my point on this was more that this shouldn't be something we're guessing at -- it should always have this attribute or never have this attribute (it should be set unconditionally on construction)
| 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 |
There was a problem hiding this comment.
pyflakes running on itself is showing this is probably not quite right -- or at least it's not happy about nonlocal
dannysepler
force-pushed
the
global-keyword
branch
from
d0abe7d
to
5302f1f
Compare
Closes #590. This also adds a new
NoBindingForNonlocalerrorI'm pretty new to
pyflakesso just let me know if I messed something up