Skip to content

Fix del builtin #355

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

Open
wants to merge 2 commits 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
16 changes: 13 additions & 3 deletions pyflakes/checker.py
Original file line number Diff line number Diff line change
Expand Up @@ -185,6 +185,13 @@ class Builtin(Definition):

def __init__(self, name):
super(Builtin, self).__init__(name, None)
self.can_delete = False
# __builtins__ and __file__ can always be deleted.
# __debug__ can be deleted sometimes and not deleted other times.
Copy link
Contributor

Choose a reason for hiding this comment

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

Can you please elaborate on this, it is failing for me.

surfer-172-20-4-99-hotspot:Desktop macbox$ python py-test.py 
Traceback (most recent call last):
  File "py-test.py", line 1, in <module>
    del __debug__
NameError: name '__debug__' is not defined
surfer-172-20-4-99-hotspot:Desktop macbox$ python3 py-test.py 
Traceback (most recent call last):
  File "py-test.py", line 1, in <module>
    del __debug__
NameError: name '__debug__' is not defined

Copy link
Member Author

Choose a reason for hiding this comment

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

On my Python 3.7 (Rawhide), __debug__ can be deleted.
On earlier Python, locally, it isnt.
However it might come down to whether the Python build had debug enabled in it.
We might need to dig into the CPython source to find out, as I dont see any mention of this in peps

# Safest course of action is to assume it can be deleted, in
# order that no error is reported by pyflakes
elif name in ('__builtins__', '__file__', '__debug__'):
Copy link
Member

Choose a reason for hiding this comment

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

elif should be if.

self.can_delete = True

def __repr__(self):
return '<%s object %r at 0x%x>' % (self.__class__.__name__,
Expand Down Expand Up @@ -871,10 +878,13 @@ def on_conditional_branch():
if isinstance(self.scope, FunctionScope) and name in self.scope.globals:
self.scope.globals.remove(name)
else:
try:
del self.scope[name]
except KeyError:
binding = self.scope.get(name, None)
if not binding or (
isinstance(binding, Builtin) and not binding.can_delete):
self.report(messages.UndefinedName, node, name)
return

del self.scope[name]

def handleChildren(self, tree, omit=None):
for node in iter_child_nodes(tree, omit=omit):
Expand Down
10 changes: 10 additions & 0 deletions pyflakes/test/harness.py
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,16 @@ class TestCase(unittest.TestCase):

withDoctest = False

def pythonException(self, input, *expectedOutputs, **kw):
try:
compile(textwrap.dedent(input), '<test>', 'exec', PyCF_ONLY_AST)
except BaseException as e:
return e
try:
exec(textwrap.dedent(input), {})
except BaseException as e:
return e

def flakes(self, input, *expectedOutputs, **kw):
tree = compile(textwrap.dedent(input), "<test>", "exec", PyCF_ONLY_AST)
if kw.get('is_segment'):
Expand Down
55 changes: 55 additions & 0 deletions pyflakes/test/test_builtin.py
Original file line number Diff line number Diff line change
Expand Up @@ -4,8 +4,15 @@
from sys import version_info

from pyflakes import messages as m
from pyflakes.checker import Checker
from pyflakes.test.harness import TestCase, skipIf

try:
WindowsError
WIN = True
except NameError:
WIN = False


class TestBuiltins(TestCase):

Expand Down Expand Up @@ -39,3 +46,51 @@ def f():
f()
''', m.UndefinedLocal)


class TestLiveBuiltins(TestCase):

def test_exists(self):
for name in sorted(Checker.builtIns):
# __file__ does exist in this test harness
if name == '__file__':
continue

if name == 'WindowsError' and not WIN:
continue

source = '''
%s
''' % name
e = self.pythonException(source)
self.assertIsNone(e)

def test_del(self):
for name in sorted(Checker.builtIns):
# __file__ does exist in this test harness
if name == '__file__':
continue

# __debug__ can be deleted sometimes and not deleted other times.
# Safest course of action is to assume it can be deleted, in
# order that no error is reported by pyflakes
if name == '__debug__':
continue

source = '''
del %s
''' % name

e = self.pythonException(source)

if isinstance(e, SyntaxError):
if version_info < (3,):
# SyntaxError: invalid syntax
self.assertIn(name, ('print'))
else:
# SyntaxError: can't delete keyword
self.assertIn(name, ('None', 'True', 'False'))
elif isinstance(e, NameError):
self.flakes(source, m.UndefinedName)
else:
self.flakes(source)
13 changes: 13 additions & 0 deletions pyflakes/test/test_undefined_names.py
Original file line number Diff line number Diff line change
Expand Up @@ -371,6 +371,19 @@ def test_delUndefined(self):
"""Del an undefined name."""
self.flakes('del a', m.UndefinedName)

def test_del_builtin(self):
"""Del a builtin not possible."""
self.flakes('del range', m.UndefinedName)

def test_del_shadowed_builtin(self):
"""
Del a function with same name as builtin.
"""
self.flakes('''
def range(): pass
del range
''')

def test_delConditional(self):
"""
Ignores conditional bindings deletion.
Expand Down