Skip to content

Commit 97cff9d

Browse files
committed
Fix __debug__ comparison check
The check for the `__debug__` name was only working for the simple case of `if __debug__:`, any other comparison of (== True, is True, not False) was incorrectly removed
1 parent c63a92b commit 97cff9d

File tree

2 files changed

+86
-8
lines changed

2 files changed

+86
-8
lines changed

src/python_minifier/transforms/remove_debug.py

Lines changed: 28 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -27,18 +27,38 @@ def can_remove(self, node):
2727
if not isinstance(node, ast.If):
2828
return False
2929

30-
if isinstance(node.test, ast.Name) and node.test.id == '__debug__':
31-
return True
30+
def is_simple_debug_check():
31+
# Simple case: if __debug__:
32+
if isinstance(node.test, ast.Name) and node.test.id == '__debug__':
33+
return True
34+
return False
3235

33-
if isinstance(node.test, ast.Compare) and len(node.test.ops) == 1 and isinstance(node.test.ops[0], ast.Is) and self.constant_value(node.test.comparators[0]) is True:
34-
return True
36+
def is_truthy_debug_comparison():
37+
# Comparison case: if __debug__ is True / False / etc.
38+
if not isinstance(node.test, ast.Compare):
39+
return False
3540

36-
if isinstance(node.test, ast.Compare) and len(node.test.ops) == 1 and isinstance(node.test.ops[0], ast.IsNot) and self.constant_value(node.test.comparators[0]) is False:
37-
return True
41+
if not isinstance(node.test.left, ast.Name):
42+
return False
3843

39-
if isinstance(node.test, ast.Compare) and len(node.test.ops) == 1 and isinstance(node.test.ops[0], ast.Eq) and self.constant_value(node.test.comparators[0]) is True:
40-
return True
44+
if node.test.left.id != '__debug__':
45+
return False
4146

47+
if len(node.test.ops) == 1:
48+
op = node.test.ops[0]
49+
comparator_value = self.constant_value(node.test.comparators[0])
50+
51+
if isinstance(op, ast.Is) and comparator_value is True:
52+
return True
53+
if isinstance(op, ast.IsNot) and comparator_value is False:
54+
return True
55+
if isinstance(op, ast.Eq) and comparator_value is True:
56+
return True
57+
58+
return False
59+
60+
if is_simple_debug_check() or is_truthy_debug_comparison():
61+
return True
4262
return False
4363

4464
def suite(self, node_list, parent):

test/test_remove_debug.py

Lines changed: 58 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -167,3 +167,61 @@ def test_no_remove_falsy_debug(condition):
167167
expected_ast = ast.parse(expected)
168168
actual_ast = remove_debug(source)
169169
compare_ast(expected_ast, actual_ast)
170+
171+
172+
@pytest.mark.parametrize(
173+
'condition', [
174+
'__sandwich__',
175+
'__sandwich__ is True',
176+
'__sandwich__ is False',
177+
'__sandwich__ is not False',
178+
'__sandwich__ == True',
179+
'__sandwich__ == __debug__',
180+
'__sandwich() == True',
181+
'func() is True',
182+
'some_call(a, b) is True',
183+
'obj.method() is True',
184+
'obj.attr is True',
185+
'True is something',
186+
'True == something',
187+
]
188+
)
189+
def test_no_remove_not_debug(condition):
190+
source = '''
191+
value = 10
192+
193+
# Not a __debug__ test
194+
if ''' + condition + ''':
195+
value += 1
196+
197+
print(value)
198+
'''
199+
200+
expected = source
201+
202+
expected_ast = ast.parse(expected)
203+
actual_ast = remove_debug(source)
204+
compare_ast(expected_ast, actual_ast)
205+
206+
207+
def test_no_remove_is_true_in_elif_chain():
208+
"""Regression test for issue #142 - if/elif/else with 'is True' comparisons"""
209+
source = '''
210+
def check_is_internet_working(c):
211+
url, url_hostname = get_url_and_url_hostname(c)
212+
213+
if is_internet_working_socket_test(c, url_hostname) is True:
214+
c.is_internet_connected = True
215+
elif is_internet_working_urllib_open(c, url) is True:
216+
c.is_internet_connected = True
217+
else:
218+
c.is_internet_connected = False
219+
220+
return c.is_internet_connected
221+
'''
222+
223+
expected = source
224+
225+
expected_ast = ast.parse(expected)
226+
actual_ast = remove_debug(source)
227+
compare_ast(expected_ast, actual_ast)

0 commit comments

Comments
 (0)