-
Notifications
You must be signed in to change notification settings - Fork 179
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
jayvdb
wants to merge
1
commit into
PyCQA:main
Choose a base branch
from
jayvdb:better-del
base: main
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
Changes from all commits
Commits
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -5,6 +5,7 @@ | |
Also, it models the Bindings and Scopes. | ||
""" | ||
import doctest | ||
import functools | ||
import os | ||
import sys | ||
|
||
|
@@ -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 | ||
the node that this binding was last used | ||
""" | ||
|
||
def __init__(self, name, source): | ||
|
@@ -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): | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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'] | ||
|
@@ -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. | ||
|
||
|
@@ -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): | ||
""" | ||
|
@@ -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: | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
|
||
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] | ||
|
@@ -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] | ||
|
@@ -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 | ||
|
@@ -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) | ||
|
@@ -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('_') | ||
|
@@ -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 | ||
|
||
|
@@ -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] | ||
|
||
|
@@ -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)): | ||
|
@@ -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) | ||
|
@@ -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 = [] | ||
|
||
|
@@ -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]: | ||
|
@@ -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)) | ||
|
@@ -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. | ||
|
@@ -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): | ||
""" | ||
|
@@ -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() | ||
|
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
split off as #39