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

Better del support #37

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.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
196 changes: 177 additions & 19 deletions pyflakes/checker.py
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
Also, it models the Bindings and Scopes.
"""
import doctest
import functools
import os
import sys

Expand Down Expand Up @@ -92,8 +93,8 @@ class Binding(object):
which names have not. See L{Assignment} for a special type of binding that
is checked with stricter rules.

@ivar used: pair of (L{Scope}, line-number) indicating the scope and
line number that this binding was last used
@ivar used: pair of (L{Scope}, node) indicating the scope and
Copy link
Member Author

Choose a reason for hiding this comment

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

split off as #39

the node that this binding was last used
"""

def __init__(self, name, source):
Expand Down Expand Up @@ -240,6 +241,14 @@ class ModuleScope(Scope):
pass


# Doctest are similar to ModuleScope,
# and on Python 3.2 and below need to allow
# 'del' of nodes used in nested functions
# like ModuleScope.
class DoctestScope(FunctionScope):
Copy link
Member Author

Choose a reason for hiding this comment

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

split off as #38

pass


# Globally defined names which are not attributes of the builtins module, or
# are only present on some platforms.
_MAGIC_GLOBALS = ['__file__', '__builtins__', 'WindowsError']
Expand Down Expand Up @@ -304,7 +313,7 @@ def __init__(self, tree, filename='(none)', builtins=None,
self.popScope()
self.checkDeadScopes()

def deferFunction(self, callable):
def deferFunction(self, callable, node=None):
"""
Schedule a function handler to be called just before completion.

Expand All @@ -313,7 +322,7 @@ def deferFunction(self, callable):
`callable` is called, the scope at the time this is called will be
restored, however it will contain any new bindings added to it.
"""
self._deferredFunctions.append((callable, self.scopeStack[:], self.offset))
self._deferredFunctions.append((callable, self.scopeStack[:], self.offset, node))

def deferAssignment(self, callable):
"""
Expand All @@ -326,11 +335,55 @@ def runDeferred(self, deferred):
"""
Run the callables in C{deferred} using their associated scope stack.
"""
for handler, scope, offset in deferred:
for record in deferred:
# node is an optional part of the tuple
# deferAssignment does not use it, and
# deferFunction does not require it
if len(record) == 3:
handler, scope, offset = record
else:
handler, scope, offset, node = record
if node:
Copy link
Member

Choose a reason for hiding this comment

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

if node is not None?

handler = functools.partial(handler, node)

self.scopeStack = scope
self.offset = offset
handler()

def runDeferredFunctionImmediate(self, name):
"""Handle any deferred functions for `name` immediately."""
if not self._deferredFunctions:
return

saved_offset = self.offset

for method, scope, offset, node in self._deferredFunctions:
if not node:
continue

if hasattr(node, 'name') and node.name == name:
if scope[-1] == self.scope:
self.offset = offset
method(node)
break
else:
# node not found with that name
return

self.offset = saved_offset

before = len(self._deferredFunctions)

# here, `node` refers to the identified node in the for loop above
self._deferredFunctions[:] = [
(method, scope, offset, this_node)
for (method, scope, offset, this_node) in self._deferredFunctions
if this_node != node or scope[-1] != self.scope]

after = len(self._deferredFunctions)
if before != after - 1:
raise RuntimeException('more than one removed for %r' % name)

@property
def scope(self):
return self.scopeStack[-1]
Expand Down Expand Up @@ -454,6 +507,26 @@ def addBinding(self, node, value):

self.scope[value.name] = value

def addBindingsWithinNestedScope(self, parent):
"""Add bindings occuring in nested objects of current scope."""
saved_offset = self.offset

nodes_done = set()

children = list(iter_child_nodes(parent))

# FIXME: this isnt recursive.
# Need to add test case with usage only in extra level of nesting.
# Could be implemented without misusing self._deferredFunctions
for method, scope, offset, node in self._deferredFunctions[:]:
if scope[-1] == self.scope and node not in nodes_done and node in children:
nodes_done.add(node)
self.offset = offset
self.handleFunction(node,
register_deferred_assignments_checks=False)

self.offset = saved_offset

def getNodeHandler(self, node_class):
try:
return self._nodeHandlers[node_class]
Expand Down Expand Up @@ -532,13 +605,13 @@ def handleNodeStore(self, node):
binding = Assignment(name, node)
self.addBinding(node, binding)

def handleNodeDelete(self, node):
def handleNodeDelete(self, node, parent=None, remove=True):

def on_conditional_branch():
"""
Return `True` if node is part of a conditional body.
"""
current = getattr(node, 'parent', None)
current = getattr(parent, 'parent', None)
while current:
if isinstance(current, (ast.If, ast.While, ast.IfExp)):
return True
Expand All @@ -555,13 +628,22 @@ def on_conditional_branch():
return

if isinstance(self.scope, FunctionScope) and name in self.scope.globals:
self.scope.globals.remove(name)
if remove:
self.scope.globals.remove(name)
return True
else:
try:
del self.scope[name]
try:
self.runDeferredFunctionImmediate(name)
except:
pass
if remove:
del self.scope[name]
except KeyError:
self.report(messages.UndefinedName, node, name)

return True

def handleChildren(self, tree, omit=None):
for node in iter_child_nodes(tree, omit=omit):
self.handleNode(node, tree)
Expand Down Expand Up @@ -625,7 +707,7 @@ def handleDoctests(self, node):
if not examples:
return
node_offset = self.offset or (0, 0)
self.pushScope()
self.pushScope(DoctestScope)
underscore_in_builtins = '_' in self.builtIns
if not underscore_in_builtins:
self.builtIns.add('_')
Expand All @@ -650,7 +732,7 @@ def ignore(self, node):
pass

# "stmt" type nodes
DELETE = PRINT = FOR = ASYNCFOR = WHILE = IF = WITH = WITHITEM = \
PRINT = FOR = ASYNCFOR = WHILE = IF = WITH = WITHITEM = \
ASYNCWITH = ASYNCWITHITEM = RAISE = TRYFINALLY = ASSERT = EXEC = \
EXPR = ASSIGN = handleChildren

Expand Down Expand Up @@ -682,6 +764,7 @@ def GLOBAL(self, node):
Keep track of globals declarations.
"""
# In doctests, the global scope is an anonymous function at index 1.
# TODO: use DoctestScope
global_scope_index = 1 if self.withDoctest else 0
global_scope = self.scopeStack[global_scope_index]

Expand Down Expand Up @@ -719,7 +802,9 @@ def GENERATOREXP(self, node):

def NAME(self, node):
"""
Handle occurrence of Name (which can be a load/store/delete access.)
Handle occurrence of Name (which can be a load or store access.)

Delete are handled in DELETE.
"""
# Locate the name in locals / function / globals scopes.
if isinstance(node.ctx, (ast.Load, ast.AugLoad)):
Expand All @@ -731,12 +816,54 @@ def NAME(self, node):
elif isinstance(node.ctx, (ast.Store, ast.AugStore)):
self.handleNodeStore(node)
elif isinstance(node.ctx, ast.Del):
self.handleNodeDelete(node)
pass
else:
# must be a Param context -- this only happens for names in function
# arguments, but these aren't dispatched through here
raise RuntimeError("Got impossible expression context: %r" % (node.ctx,))

def DELETE(self, node):
"""
Handle delete statements.

First process all deleted nodes, including functions,
without deleting any of them from the scope.

Python 3.2 and below do not allow deletion within functions
if the node has been used in a nested function.

Finally remove all deleted nodes from the scope.
"""
nodes = list(iter_child_nodes(node))

to_remove = []
for child in nodes:
remove = self.handleNodeDelete(child, parent=node, remove=False)
if remove:
to_remove.append(child)

parent = node.parent
if isinstance(parent, ast.FunctionDef) and PY32:
self.addBindingsWithinNestedScope(parent)

for child in to_remove:
name = getNodeName(child)
if not name:
continue

if isinstance(self.scope, FunctionScope) and not isinstance(self.scope, DoctestScope) and self.scope[name].used:
# TODO: test that the scope of the usage is 'lower' than the current scope
if isinstance(self.scope[name].used[0], FunctionScope) and not isinstance(self.scope[name].used[0], DoctestScope) and self.scope[name].used[0] != self.scope:
self.report(messages.DeleteNestedUsage, node, name)

if isinstance(self.scope, FunctionScope) and name in self.scope.globals:
self.scope.globals.remove(name)
else:
try:
del self.scope[name]
except KeyError:
self.report(messages.UndefinedName, child, name)

def RETURN(self, node):
if isinstance(self.scope, ClassScope):
self.report(messages.ReturnOutsideFunction, node)
Expand All @@ -762,11 +889,11 @@ def FUNCTIONDEF(self, node):
self.LAMBDA(node)
self.addBinding(node, FunctionDefinition(node.name, node))
if self.withDoctest:
self.deferFunction(lambda: self.handleDoctests(node))
self.deferFunction(self.handleDoctests, node)

ASYNCFUNCTIONDEF = FUNCTIONDEF

def LAMBDA(self, node):
def getSpec(self, node):
args = []
annotations = []

Expand Down Expand Up @@ -803,6 +930,11 @@ def addArgs(arglist):
if is_py3_func:
annotations.append(node.returns)

return args, defaults, annotations

def LAMBDA(self, node):
args, defaults, annotations = self.getSpec(node)

if len(set(args)) < len(args):
for (idx, arg) in enumerate(args):
if arg in args[:idx]:
Expand All @@ -812,8 +944,17 @@ def addArgs(arglist):
if child:
self.handleNode(child, node)

def runFunction():
node._args = args

self.deferFunction(self.handleFunction, node)

def handleFunction(self, node, register_deferred_assignments_checks=True):
scope = None
args = node._args

# Nested function runFunction only added to assist diff tools
# align old code with new code during code review.
def runFunction():
self.pushScope()
for name in args:
self.addBinding(node, Argument(name, node))
Expand All @@ -825,6 +966,24 @@ def runFunction():
# case for Lambdas
self.handleNode(node.body, node)

scope = self.scope

if register_deferred_assignments_checks:
self.registerDeferredFunctionAssignmentChecks()

self.popScope()

runFunction()

return scope

def registerDeferredFunctionAssignmentChecks(self):
"""Register deferred function assignment checks."""

# Nested function do only added to assist diff tools
# align old code with new code during code review.
def do():

def checkUnusedAssignments():
"""
Check to see if any assignments have not been used.
Expand All @@ -843,9 +1002,8 @@ def checkReturnWithArgumentInsideGenerator():
self.report(messages.ReturnWithArgsInsideGenerator,
self.scope.returnValue)
self.deferAssignment(checkReturnWithArgumentInsideGenerator)
self.popScope()

self.deferFunction(runFunction)
do()

def CLASSDEF(self, node):
"""
Expand All @@ -862,7 +1020,7 @@ def CLASSDEF(self, node):
self.handleNode(keywordNode, node)
self.pushScope(ClassScope)
if self.withDoctest:
self.deferFunction(lambda: self.handleDoctests(node))
self.deferFunction(self.handleDoctests, node)
for stmt in node.body:
self.handleNode(stmt, node)
self.popScope()
Expand Down
11 changes: 11 additions & 0 deletions pyflakes/messages.py
Original file line number Diff line number Diff line change
Expand Up @@ -120,6 +120,17 @@ def __init__(self, filename, loc, names):
self.message_args = (names,)


class DeleteNestedUsage(Message):
"""
Indicates variable can not be deleted due to use in nested scope.
"""
message = 'can not delete variable %r referenced in nested scope'

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


class ReturnWithArgsInsideGenerator(Message):
"""
Indicates a return statement with arguments inside a generator.
Expand Down
Loading