From de2b9b5ffb12bc59b44b2a44abb654c4193c0ea2 Mon Sep 17 00:00:00 2001 From: Daniel Flook Date: Mon, 11 Nov 2024 16:48:53 +0000 Subject: [PATCH 1/5] Correctly set the namespace for NamedExpr targets NamedExpr targets should not be bound in a comprehension namespace, but rather it's parent scope. --- src/python_minifier/rename/mapper.py | 24 +++++++++++++++++++++++- 1 file changed, 23 insertions(+), 1 deletion(-) diff --git a/src/python_minifier/rename/mapper.py b/src/python_minifier/rename/mapper.py index 4456786..a60e104 100644 --- a/src/python_minifier/rename/mapper.py +++ b/src/python_minifier/rename/mapper.py @@ -112,10 +112,27 @@ def add_parent_to_comprehension(node, namespace): add_parent(generator.target, namespace=node) add_parent(generator.iter, namespace=iter_namespace) - iter_namespace = node + for if_ in generator.ifs: add_parent(if_, namespace=node) + iter_namespace = node + +def namedexpr_namespace(node): + """ + Get the namespace for a NamedExpr target + """ + + if not isinstance(node, (ast.ListComp, ast.DictComp, ast.SetComp, ast.GeneratorExp)): + return node + + return namedexpr_namespace(node.namespace) + +def add_parent_to_namedexpr(node): + assert isinstance(node, ast.NamedExpr) + + add_parent(node.target, namespace=namedexpr_namespace(node.namespace)) + add_parent(node.value, namespace=node.namespace) def add_parent(node, namespace=None): """ @@ -161,6 +178,11 @@ def add_parent(node, namespace=None): elif isinstance(node.ctx, ast.Store) and isinstance(get_parent(node), ast.AugAssign): namespace.nonlocal_names.add(node.id) + if isinstance(node, ast.NamedExpr): + # NamedExpr is 'special' + add_parent_to_namedexpr(node) + return + for child in ast.iter_child_nodes(node): add_parent(child, namespace=namespace) From c3b8bb6a1a75707c9dd61f656a81e9cd89de7355 Mon Sep 17 00:00:00 2001 From: Daniel Flook Date: Mon, 11 Nov 2024 16:51:18 +0000 Subject: [PATCH 2/5] Fix printing namespaces with hoisted literals These bindings do not have a name to sort by, so use their value instead. --- test/helpers.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/helpers.py b/test/helpers.py index b145213..1b9f22f 100644 --- a/test/helpers.py +++ b/test/helpers.py @@ -43,7 +43,7 @@ def namespace_name(node): for name in sorted(namespace.nonlocal_names): s += indent + ' - nonlocal ' + name + '\n' - for binding in sorted(namespace.bindings, key=lambda b: b.name): + for binding in sorted(namespace.bindings, key=lambda b: b.name or str(b.value)): s += indent + ' - ' + repr(binding) + '\n' for child in iter_child_namespaces(namespace): From cfd1ab33b3749b796b49e2894f0d227f6744a119 Mon Sep 17 00:00:00 2001 From: Daniel Flook Date: Mon, 11 Nov 2024 17:01:39 +0000 Subject: [PATCH 3/5] Add NamedExpr binding test --- test/test_bind_names_python312.py | 60 +++++++++++++++++++++++++++++++ 1 file changed, 60 insertions(+) diff --git a/test/test_bind_names_python312.py b/test/test_bind_names_python312.py index c751c7b..43f5c08 100644 --- a/test/test_bind_names_python312.py +++ b/test/test_bind_names_python312.py @@ -165,3 +165,63 @@ def test_alias_paramspec_default(): ''' assert_namespace_tree(source, expected_namespaces) + +def test_namedexpr_in_module(): + if sys.version_info < (3, 8): + pytest.skip('Test is for > python3.8 only') + + source = ''' +(a := 1) +''' + + expected_namespaces = ''' ++ Module + - NameBinding(name='a', allow_rename=True) +''' + + assert_namespace_tree(source, expected_namespaces) + +def test_namedexpr_in_function(): + if sys.version_info < (3, 8): + pytest.skip('Test is for > python3.8 only') + + source = ''' +def test(): + (a := 1) +lambda x: (x := 1) +''' + + expected_namespaces = ''' ++ Module + - NameBinding(name='test', allow_rename=True) + + Function test + - NameBinding(name='a', allow_rename=True) + + Lambda + - NameBinding(name='x', allow_rename=False) +''' + + assert_namespace_tree(source, expected_namespaces) + +def test_namedexpr(): + if sys.version_info < (3, 8): + pytest.skip('Test is for > python3.8 only') + + source = ''' +def f(arg, /): + print([x for y in range(10) if (x := y // 2) & 1]) + print(arg, arg) +''' + + expected_namespaces = ''' ++ Module + - NameBinding(name='f', allow_rename=True) + - BuiltinBinding(name='print', allow_rename=True) + - BuiltinBinding(name='range', allow_rename=True) + + Function f + - NameBinding(name='arg', allow_rename=True) + - NameBinding(name='x', allow_rename=True) + + ListComp + - NameBinding(name='y', allow_rename=True) +''' + + assert_namespace_tree(source, expected_namespaces) From 8d4fbeea56052f009b08b0ecb0f4f914515c1c5e Mon Sep 17 00:00:00 2001 From: Daniel Flook Date: Mon, 11 Nov 2024 17:17:23 +0000 Subject: [PATCH 4/5] Stop skipping regression tests --- .github/workflows/xtest.yaml | 1 - 1 file changed, 1 deletion(-) diff --git a/.github/workflows/xtest.yaml b/.github/workflows/xtest.yaml index d9b9c87..f8e76fc 100644 --- a/.github/workflows/xtest.yaml +++ b/.github/workflows/xtest.yaml @@ -32,7 +32,6 @@ jobs: with: image: danielflook/python-minifier-build:${{ matrix.python }}-2024-09-15 run: | - exit 0 if [[ "${{ matrix.python }}" == "python3.4" ]]; then (cd /usr/lib64/python3.4/test && python3.4 make_ssl_certs.py) From 99616a9b29cd543dba69a8b8b89d1160057b289e Mon Sep 17 00:00:00 2001 From: Daniel Flook Date: Tue, 12 Nov 2024 10:57:12 +0000 Subject: [PATCH 5/5] Add some additional tests for comprehension types --- test/test_bind_names_namedexpr.py | 259 ++++++++++++++++++++++++++++++ test/test_bind_names_python312.py | 60 ------- 2 files changed, 259 insertions(+), 60 deletions(-) create mode 100644 test/test_bind_names_namedexpr.py diff --git a/test/test_bind_names_namedexpr.py b/test/test_bind_names_namedexpr.py new file mode 100644 index 0000000..a0e4ccb --- /dev/null +++ b/test/test_bind_names_namedexpr.py @@ -0,0 +1,259 @@ +import sys + +import pytest + +from helpers import assert_namespace_tree + +def test_namedexpr_in_module(): + if sys.version_info < (3, 8): + pytest.skip('Test is for >= python3.8 only') + + source = ''' +(a := 1) +''' + + expected_namespaces = ''' ++ Module + - NameBinding(name='a', allow_rename=True) +''' + + assert_namespace_tree(source, expected_namespaces) + +def test_namedexpr_in_function(): + if sys.version_info < (3, 8): + pytest.skip('Test is for >= python3.8 only') + + source = ''' +def test(): + (a := 1) +lambda x: (x := 1) +''' + + expected_namespaces = ''' ++ Module + - NameBinding(name='test', allow_rename=True) + + Function test + - NameBinding(name='a', allow_rename=True) + + Lambda + - NameBinding(name='x', allow_rename=False) +''' + + assert_namespace_tree(source, expected_namespaces) + +def test_namedexpr_in_listcomp_if_nonlocal(): + if sys.version_info < (3, 8): + pytest.skip('Test is for >= python3.8 only') + + source = ''' +def f(arg, /): + nonlocal x + print([x for y in range(10) if (x := y // 2) & 1]) + print(arg, arg) +''' + + expected_namespaces = ''' ++ Module + - NameBinding(name='f', allow_rename=True) + - BuiltinBinding(name='print', allow_rename=True) + - BuiltinBinding(name='range', allow_rename=True) + - NameBinding(name='x', allow_rename=False) + + Function f + - nonlocal x + - NameBinding(name='arg', allow_rename=True) + + ListComp + - NameBinding(name='y', allow_rename=True) +''' + + assert_namespace_tree(source, expected_namespaces) + + +def test_namedexpr_in_listcomp_if_global(): + if sys.version_info < (3, 8): + pytest.skip('Test is for >= python3.8 only') + + source = ''' +def f2(): + def f(arg, /): + global x + print([x for y in range(10) if (x := y // 2) & 1]) + print(arg, arg) +''' + + expected_namespaces = ''' ++ Module + - NameBinding(name='f2', allow_rename=True) + - BuiltinBinding(name='print', allow_rename=True) + - BuiltinBinding(name='range', allow_rename=True) + - NameBinding(name='x', allow_rename=True) + + Function f2 + - NameBinding(name='f', allow_rename=True) + + Function f + - global x + - NameBinding(name='arg', allow_rename=True) + + ListComp + - NameBinding(name='y', allow_rename=True) +''' + + assert_namespace_tree(source, expected_namespaces) + + +def test_namedexpr_in_listcomp_if(): + if sys.version_info < (3, 8): + pytest.skip('Test is for >= python3.8 only') + + source = ''' +def f(arg, /): + print([x for y in range(10) if (x := y // 2) & 1]) + print(arg, arg) +''' + + expected_namespaces = ''' ++ Module + - NameBinding(name='f', allow_rename=True) + - BuiltinBinding(name='print', allow_rename=True) + - BuiltinBinding(name='range', allow_rename=True) + + Function f + - NameBinding(name='arg', allow_rename=True) + - NameBinding(name='x', allow_rename=True) + + ListComp + - NameBinding(name='y', allow_rename=True) +''' + + assert_namespace_tree(source, expected_namespaces) + + +def test_namedexpr_in_listcomp_body(): + if sys.version_info < (3, 8): + pytest.skip('Test is for >= python3.8 only') + + source = ''' +def f(arg, /): + print([(x := y // 2) for _ in range(x)]) + print(arg, arg) +''' + + expected_namespaces = ''' ++ Module + - NameBinding(name='f', allow_rename=True) + - BuiltinBinding(name='print', allow_rename=True) + - BuiltinBinding(name='range', allow_rename=True) + - NameBinding(name='y', allow_rename=False) + + Function f + - NameBinding(name='arg', allow_rename=True) + - NameBinding(name='x', allow_rename=True) + + ListComp + - NameBinding(name='_', allow_rename=True) +''' + + assert_namespace_tree(source, expected_namespaces) + +def test_namedexpr_in_dictcomp_body(): + if sys.version_info < (3, 8): + pytest.skip('Test is for >= python3.8 only') + + source = ''' +{i: (x := i // 2) for i in range(1)} +''' + + expected_namespaces = ''' ++ Module + - BuiltinBinding(name='range', allow_rename=True) + - NameBinding(name='x', allow_rename=True) + + DictComp + - NameBinding(name='i', allow_rename=True) +''' + + assert_namespace_tree(source, expected_namespaces) + + +def test_namedexpr_in_dictcomp_if(): + if sys.version_info < (3, 8): + pytest.skip('Test is for >= python3.8 only') + + source = ''' +{x: y for y in range(1) if (x := y // 2)} +''' + + expected_namespaces = ''' ++ Module + - BuiltinBinding(name='range', allow_rename=True) + - NameBinding(name='x', allow_rename=True) + + DictComp + - NameBinding(name='y', allow_rename=True) +''' + + assert_namespace_tree(source, expected_namespaces) + +def test_namedexpr_in_setcomp_body(): + if sys.version_info < (3, 8): + pytest.skip('Test is for >= python3.8 only') + + source = ''' +{(x := y // 2) for y in range(1)} +''' + + expected_namespaces = ''' ++ Module + - BuiltinBinding(name='range', allow_rename=True) + - NameBinding(name='x', allow_rename=True) + + SetComp + - NameBinding(name='y', allow_rename=True) +''' + + assert_namespace_tree(source, expected_namespaces) + + +def test_namedexpr_in_setcomp_if(): + if sys.version_info < (3, 8): + pytest.skip('Test is for >= python3.8 only') + + source = ''' +{x for y in range(1) if (x := y // 2)} +''' + + expected_namespaces = ''' ++ Module + - BuiltinBinding(name='range', allow_rename=True) + - NameBinding(name='x', allow_rename=True) + + SetComp + - NameBinding(name='y', allow_rename=True) +''' + + assert_namespace_tree(source, expected_namespaces) + +def test_namedexpr_in_generatorexp_body(): + if sys.version_info < (3, 8): + pytest.skip('Test is for >= python3.8 only') + + source = ''' +((x := y // 2) for y in range(1)) +''' + + expected_namespaces = ''' ++ Module + - BuiltinBinding(name='range', allow_rename=True) + - NameBinding(name='x', allow_rename=True) + + GeneratorExp + - NameBinding(name='y', allow_rename=True) +''' + + assert_namespace_tree(source, expected_namespaces) + + +def test_namedexpr_in_generatorexp_if(): + if sys.version_info < (3, 8): + pytest.skip('Test is for >= python3.8 only') + + source = ''' +(x for y in range(1) if (x := y // 2)) +''' + + expected_namespaces = ''' ++ Module + - BuiltinBinding(name='range', allow_rename=True) + - NameBinding(name='x', allow_rename=True) + + GeneratorExp + - NameBinding(name='y', allow_rename=True) +''' + + assert_namespace_tree(source, expected_namespaces) diff --git a/test/test_bind_names_python312.py b/test/test_bind_names_python312.py index 43f5c08..c751c7b 100644 --- a/test/test_bind_names_python312.py +++ b/test/test_bind_names_python312.py @@ -165,63 +165,3 @@ def test_alias_paramspec_default(): ''' assert_namespace_tree(source, expected_namespaces) - -def test_namedexpr_in_module(): - if sys.version_info < (3, 8): - pytest.skip('Test is for > python3.8 only') - - source = ''' -(a := 1) -''' - - expected_namespaces = ''' -+ Module - - NameBinding(name='a', allow_rename=True) -''' - - assert_namespace_tree(source, expected_namespaces) - -def test_namedexpr_in_function(): - if sys.version_info < (3, 8): - pytest.skip('Test is for > python3.8 only') - - source = ''' -def test(): - (a := 1) -lambda x: (x := 1) -''' - - expected_namespaces = ''' -+ Module - - NameBinding(name='test', allow_rename=True) - + Function test - - NameBinding(name='a', allow_rename=True) - + Lambda - - NameBinding(name='x', allow_rename=False) -''' - - assert_namespace_tree(source, expected_namespaces) - -def test_namedexpr(): - if sys.version_info < (3, 8): - pytest.skip('Test is for > python3.8 only') - - source = ''' -def f(arg, /): - print([x for y in range(10) if (x := y // 2) & 1]) - print(arg, arg) -''' - - expected_namespaces = ''' -+ Module - - NameBinding(name='f', allow_rename=True) - - BuiltinBinding(name='print', allow_rename=True) - - BuiltinBinding(name='range', allow_rename=True) - + Function f - - NameBinding(name='arg', allow_rename=True) - - NameBinding(name='x', allow_rename=True) - + ListComp - - NameBinding(name='y', allow_rename=True) -''' - - assert_namespace_tree(source, expected_namespaces)