Fix ClassDef Name binding

.github/workflows/xtest.yaml

@@ -17,6 +17,11 @@ jobs:
     container:
       image: danielflook/python-minifier-build:${{ matrix.python }}-2024-01-12
     steps:
+      - name: Cleanup
+        run: |
+          echo rm -vrf "$HOME/.*" "$HOME/*" "$GITHUB_WORKSPACE/.*" "$GITHUB_WORKSPACE/*"
+          rm -vrf "$HOME/.*" "$HOME/*" "$GITHUB_WORKSPACE/.*" "$GITHUB_WORKSPACE/*"
+
       - name: Checkout
         uses: actions/checkout@v3
         with:

corpus_test/generate_results.py

@@ -1,5 +1,4 @@
 import argparse
-import datetime
 import gzip
 import os
 import sys
@@ -63,6 +62,9 @@ def minify_corpus_entry(corpus_path, corpus_entry):
         result.time = end_time - start_time
         result.outcome = 'UnstableMinification'
 
+    except AssertionError as assertion_error:
+        result.outcome = 'Exception: AssertionError'
+
     except Exception as exception:
         result.outcome = 'Exception: ' + str(exception)
 

src/python_minifier/__init__.py

@@ -228,7 +228,6 @@ def unparse(module):
     try:
         compare_ast(module, minified_module)
     except CompareError as compare_error:
-        print(printer.code)
         raise UnstableMinification(compare_error, '', printer.code)
 
     return printer.code

src/python_minifier/rename/bind_names.py

@@ -24,11 +24,6 @@ def get_binding(self, name, namespace):
         # nonlocal names should not create a binding in any context
         assert name not in namespace.nonlocal_names
 
-        if isinstance(namespace, ast.ClassDef):
-            binding = self.get_binding(name, get_nonlocal_namespace(namespace))
-            binding.disallow_rename()
-            return binding
-
         for binding in namespace.bindings:
             if binding.name == name:
                 break
@@ -43,6 +38,10 @@ def get_binding(self, name, namespace):
             # This is actually a syntax error - but we want the same syntax error after minifying!
             binding.disallow_rename()
 
+        if isinstance(namespace, ast.ClassDef):
+            # This name will become an attribute of the class, so it can't be renamed
+            binding.disallow_rename()
+
         return binding
 
     def visit_Name(self, node):

src/python_minifier/rename/binding.py

@@ -11,7 +11,6 @@ class Binding(object):
     :param name: A name for this binding
     :type name: str or None
     :param bool allow_rename: If this binding may be renamed
-    :param int rename_cost: The cost of renaming this binding in bytes, NOT including the difference in name lengths
 
     """
 
@@ -289,7 +288,7 @@ def __init__(self, name, *args, **kwargs):
             self.disallow_rename()
 
     def __repr__(self):
-        return self.__class__.__name__ + '(name=%r) <references=%r>' % (self._name, len(self._references))
+        return self.__class__.__name__ + '(name=%r, allow_rename=%r) <references=%r>' % (self._name, self._allow_rename, len(self._references))
 
     def should_rename(self, new_name):
         """
@@ -448,3 +447,25 @@ def rename(self, new_name):
                 ),
             )
         )
+
+    def is_redefined(self):
+        """
+        Do one of the references to this builtin name redefine it?
+
+        Could some references actually not be references to the builtin?
+
+        This can happen with code like:
+
+        class MyClass:
+            IndexError = IndexError
+
+        """
+
+        for node in self.references:
+            if not isinstance(node, ast.Name):
+                return True
+
+            if not isinstance(node.ctx, ast.Load):
+                return True
+
+        return False

src/python_minifier/rename/mapper.py

@@ -160,6 +160,13 @@ def add_parent(node, parent=None, namespace=None):
     if is_ast_node(node, 'Nonlocal'):
         namespace.nonlocal_names.update(node.names)
 
+    if isinstance(node, ast.Name):
+        if isinstance(namespace, ast.ClassDef):
+            if isinstance(node.ctx, ast.Load):
+                namespace.nonlocal_names.add(node.id)
+            elif isinstance(node.ctx, ast.Store) and isinstance(node.parent, ast.AugAssign):
+                namespace.nonlocal_names.add(node.id)
+
     for child in ast.iter_child_nodes(node):
         add_parent(child, parent=node, namespace=namespace)
 

src/python_minifier/rename/resolve_names.py

@@ -34,6 +34,14 @@ def get_binding(name, namespace):
             namespace.bindings.append(binding)
             return binding
 
+def get_binding_disallow_class_namespace_rename(name, namespace):
+    binding = get_binding(name, namespace)
+
+    if isinstance(namespace, ast.ClassDef):
+        # This name will become an attribute of a class, so it can't be renamed
+        binding.disallow_rename()
+
+    return binding
 
 def resolve_names(node):
     """
@@ -47,39 +55,49 @@ def resolve_names(node):
     if isinstance(node, ast.Name) and isinstance(node.ctx, ast.Load):
         get_binding(node.id, node.namespace).add_reference(node)
     elif isinstance(node, ast.Name) and node.id in node.namespace.nonlocal_names:
-        get_binding(node.id, node.namespace).add_reference(node)
+        binding = get_binding(node.id, node.namespace)
+        binding.add_reference(node)
+
+        if isinstance(node.ctx, ast.Store) and isinstance(node.namespace, ast.ClassDef):
+            binding.disallow_rename()
 
     elif isinstance(node, ast.ClassDef) and node.name in node.namespace.nonlocal_names:
-        get_binding(node.name, node.namespace).add_reference(node)
+        binding = get_binding_disallow_class_namespace_rename(node.name, node.namespace)
+        binding.add_reference(node)
+
     elif is_ast_node(node, (ast.FunctionDef, 'AsyncFunctionDef')) and node.name in node.namespace.nonlocal_names:
-        get_binding(node.name, node.namespace).add_reference(node)
+        binding = get_binding_disallow_class_namespace_rename(node.name, node.namespace)
+        binding.add_reference(node)
+
     elif isinstance(node, ast.alias):
 
         if node.asname is not None:
             if node.asname in node.namespace.nonlocal_names:
-                get_binding(node.asname, node.namespace).add_reference(node)
+                binding = get_binding_disallow_class_namespace_rename(node.asname, node.namespace)
+                binding.add_reference(node)
+
         else:
             # This binds the root module only for a dotted import
             root_module = node.name.split('.')[0]
 
             if root_module in node.namespace.nonlocal_names:
-                binding = get_binding(root_module, node.namespace)
+                binding = get_binding_disallow_class_namespace_rename(root_module, node.namespace)
                 binding.add_reference(node)
 
                 if '.' in node.name:
                     binding.disallow_rename()
 
     elif isinstance(node, ast.ExceptHandler) and node.name is not None:
         if isinstance(node.name, str) and node.name in node.namespace.nonlocal_names:
-            get_binding(node.name, node.namespace).add_reference(node)
+            get_binding_disallow_class_namespace_rename(node.name, node.namespace).add_reference(node)
 
     elif is_ast_node(node, 'Nonlocal'):
         for name in node.names:
-            get_binding(name, node.namespace).add_reference(node)
+            get_binding_disallow_class_namespace_rename(name, node.namespace).add_reference(node)
     elif is_ast_node(node, ('MatchAs', 'MatchStar')) and node.name in node.namespace.nonlocal_names:
-        get_binding(node.name, node.namespace).add_reference(node)
+        get_binding_disallow_class_namespace_rename(node.name, node.namespace).add_reference(node)
     elif is_ast_node(node, 'MatchMapping') and node.rest in node.namespace.nonlocal_names:
-        get_binding(node.rest, node.namespace).add_reference(node)
+        get_binding_disallow_class_namespace_rename(node.rest, node.namespace).add_reference(node)
 
     elif is_ast_node(node, 'Exec'):
         get_global_namespace(node).tainted = True

src/python_minifier/rename/util.py

@@ -23,6 +23,15 @@ def create_is_namespace():
 is_namespace = create_is_namespace()
 
 
+def iter_child_namespaces(node):
+
+    for child in ast.iter_child_nodes(node):
+        if is_namespace(child):
+            yield child
+        else:
+            for c in iter_child_namespaces(child):
+                yield c
+
 def get_global_namespace(node):
     """
     Return the global namespace for a node

src/python_minifier/transforms/combine_imports.py

@@ -14,19 +14,22 @@ class CombineImports(SuiteTransformer):
     def _combine_import(self, node_list, parent):
 
         alias = []
+        namespace = None
 
         for statement in node_list:
+            namespace = statement.namespace
             if isinstance(statement, ast.Import):
                 alias += statement.names
             else:
                 if alias:
-                    yield self.add_child(ast.Import(names=alias), parent=parent)
+                    yield self.add_child(ast.Import(names=alias), parent=parent, namespace=namespace)
                     alias = []
 
                 yield statement
 
         if alias:
-            yield self.add_child(ast.Import(names=alias), parent=parent)
+            yield self.add_child(ast.Import(names=alias), parent=parent, namespace=namespace)
+
 
     def _combine_import_from(self, node_list, parent):
 
@@ -55,15 +58,15 @@ def combine(statement):
             else:
                 if alias:
                     yield self.add_child(
-                        ast.ImportFrom(module=prev_import.module, names=alias, level=prev_import.level), parent=parent
+                        ast.ImportFrom(module=prev_import.module, names=alias, level=prev_import.level), parent=parent, namespace=prev_import.namespace
                     )
                     alias = []
 
                 yield statement
 
         if alias:
             yield self.add_child(
-                ast.ImportFrom(module=prev_import.module, names=alias, level=prev_import.level), parent=parent
+                ast.ImportFrom(module=prev_import.module, names=alias, level=prev_import.level), parent=parent, namespace=prev_import.namespace
             )
 
     def suite(self, node_list, parent):

src/python_minifier/transforms/remove_exception_brackets.py

@@ -77,7 +77,8 @@ def _remove_empty_call(binding):
     assert isinstance(binding, BuiltinBinding)
 
     for name_node in binding.references:
-        assert isinstance(name_node, ast.Name)  # For this to be a builtin, all references must be name nodes as it is not defined anywhere
+        # For this to be a builtin, all references must be name nodes as it is not defined anywhere
+        assert isinstance(name_node, ast.Name) and isinstance(name_node.ctx, ast.Load)
 
         if not isinstance(name_node.parent, ast.Call):
             # This is not a call
@@ -110,7 +111,13 @@ def remove_no_arg_exception_call(module):
         return module
 
     for binding in module.bindings:
-        if isinstance(binding, BuiltinBinding) and binding.name in builtin_exceptions:
+        if not isinstance(binding, BuiltinBinding):
+            continue
+
+        if binding.is_redefined():
+            continue
+
+        if binding.name in builtin_exceptions:
             # We can remove any calls to builtin exceptions
             _remove_empty_call(binding)
 

test/helpers.py

@@ -0,0 +1,50 @@
+import ast
+
+from python_minifier.rename import add_namespace, resolve_names
+from python_minifier.rename.bind_names import bind_names
+from python_minifier.rename.util import iter_child_namespaces
+from python_minifier.util import is_ast_node
+
+
+def assert_namespace_tree(source, expected_tree):
+    tree = ast.parse(source)
+
+    add_namespace(tree)
+    bind_names(tree)
+    resolve_names(tree)
+
+    actual = print_namespace(tree)
+
+    print(actual)
+    assert actual.strip() == expected_tree.strip()
+
+
+def print_namespace(namespace, indent=''):
+    s = ''
+
+    if not indent:
+        s += '\n'
+
+    def namespace_name(node):
+        if is_ast_node(node, (ast.FunctionDef, 'AsyncFunctionDef')):
+            return 'Function ' + node.name
+        elif isinstance(node, ast.ClassDef):
+            return 'Class ' + node.name
+        else:
+            return namespace.__class__.__name__
+
+    s += indent + '+ ' + namespace_name(namespace) + '\n'
+
+    for name in sorted(namespace.global_names):
+        s += indent + '  - global ' + name + '\n'
+
+    for name in sorted(namespace.nonlocal_names):
+        s += indent + '  - nonlocal ' + name + '\n'
+
+    for binding in sorted(namespace.bindings, key=lambda b: b.name):
+        s += indent + '  - ' + repr(binding) + '\n'
+
+    for child in iter_child_namespaces(namespace):
+        s += print_namespace(child, indent=indent + '  ')
+
+    return s