diff --git a/.github/workflows/test.yaml b/.github/workflows/test.yaml index 19b6b598..0cabc6fc 100644 --- a/.github/workflows/test.yaml +++ b/.github/workflows/test.yaml @@ -71,7 +71,7 @@ jobs: - name: Set up Python if: ${{ matrix.python-version == '2.7' }} - uses: LizardByte/actions/actions/setup_python@eddc8fc8b27048e25040e37e3585bd3ef9a968ed # master + uses: LizardByte/actions/actions/setup_python@a46850981292c4bbc0c41715f286ad95adaaaf4a # v2026.625.20301 with: python-version: ${{ matrix.python-version }} diff --git a/.gitignore b/.gitignore index 4bece855..a346d3cf 100644 --- a/.gitignore +++ b/.gitignore @@ -19,3 +19,4 @@ docs/source/transforms/*.min.py .coverage .mypy_cache/ .tox/ +docs/source/minification_options/*.min.py diff --git a/CHANGELOG.md b/CHANGELOG.md index 2e1f480c..eca822c2 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -10,6 +10,15 @@ and will output source code compatible with the version of the interpreter it is This means that if you minify code written for Python 3.11 using python-minifier running with Python 3.12, the minified code may only run with Python 3.12. +## [Unreleased] + +### Added +- New transform to remove `if` statement branches that have no effect because the condition is always `True` or `False`. This is enabled by default and can be disabled with the `--no-remove-dead-branches` option. +- Constant folding can now fold boolean (`and`/`or`) and comparison (`==`, `is`, `<`, ...) expressions with literal operands. + +### Fixed +- The remove debug transform no longer removes branches if they affect the program, even though they aren't executed, e.g. they contain `yield`, a `global`/`nonlocal` declaration, or the only binding of a local name. It also now properly inlines the else branch. + ## [3.2.0] - 2025-12-31 ### Added diff --git a/corpus_test/generate_report.py b/corpus_test/generate_report.py index c311478b..89ce84da 100644 --- a/corpus_test/generate_report.py +++ b/corpus_test/generate_report.py @@ -277,8 +277,8 @@ def report_larger_than_base(results_dir: str, python_versions: list[str], minifi yield ''' ## Top 10 Larger than base -| Corpus Entry | Original Size | Minified Size | -|--------------|--------------:|--------------:|''' +| Corpus Entry | Python Version | Original Size | Minified Size | +|--------------|----------------|--------------:|--------------:|''' there_are_some_larger_than_base = False @@ -293,17 +293,17 @@ def report_larger_than_base(results_dir: str, python_versions: list[str], minifi except FileNotFoundError: continue - larger_than_original = sorted(summary.compare_size_increase(base_summary), key=lambda result: result.original_size)[:10] + larger_than_base = sorted(summary.compare_size_increase(base_summary), key=lambda result: result.original_size)[:10] - for entry in larger_than_original: + for entry in larger_than_base: there_are_some_larger_than_base = True - yield f'| {entry.corpus_entry} | {entry.original_size} | {entry.minified_size} ({entry.minified_size - base_summary.entries[entry.corpus_entry].minified_size:+}) |' + yield f'| {entry.corpus_entry} | {python_version} | {entry.original_size} | {entry.minified_size} ({entry.minified_size - base_summary.entries[entry.corpus_entry].minified_size:+}) |' if not there_are_some_larger_than_base: - yield '| None | | |' + yield '| None | | | |' -def report_slowest(results_dir: str, python_versions: list[str], minifier_sha: str) -> str: +def report_slowest(results_dir: str, python_versions: list[str], minifier_sha: str, base_sha: str) -> str: yield ''' ## Top 10 Slowest @@ -316,8 +316,18 @@ def report_slowest(results_dir: str, python_versions: list[str], minifier_sha: s except FileNotFoundError: continue + try: + base_summary = result_summary(results_dir, python_version, base_sha) + except FileNotFoundError: + base_summary = None + for entry in sorted(summary.entries.values(), key=lambda entry: entry.time, reverse=True)[:10]: - yield f'| {entry.corpus_entry} | {entry.original_size} | {entry.minified_size} | {entry.time:.3f} |' + time_detail = f'{entry.time:.3f}' + + if base_summary is not None and entry.corpus_entry in base_summary.entries: + time_detail += f' ({entry.time - base_summary.entries[entry.corpus_entry].time:+.3f})' + + yield f'| {entry.corpus_entry} | {entry.original_size} | {entry.minified_size} | {time_detail} |' def format_size_change_detail(summary, base_summary) -> str: mean_percent_of_original_change = summary.mean_percent_of_original - base_summary.mean_percent_of_original @@ -400,11 +410,13 @@ def report(results_dir: str, minifier_ref: str, minifier_sha: str, base_ref: str f'| {format_difference(summary.exception(), base_summary.exception())} |' ) + all_versions = ['2.7', '3.3', '3.4', '3.5', '3.6', '3.7', '3.8', '3.9', '3.10', '3.11', '3.12', '3.13', '3.14'] + yield from report_larger_than_original(results_dir, ['3.14'], minifier_sha) - yield from report_larger_than_base(results_dir, ['3.13'], minifier_sha, base_sha) - yield from report_slowest(results_dir, ['3.14'], minifier_sha) - yield from report_unstable(results_dir, ['2.7', '3.3', '3.4', '3.5', '3.6', '3.7', '3.8', '3.9', '3.10', '3.11', '3.12', '3.13', '3.14'], minifier_sha) - yield from report_exceptions(results_dir, ['2.7', '3.3', '3.4', '3.5', '3.6', '3.7', '3.8', '3.9', '3.10', '3.11', '3.12', '3.13', '3.14'], minifier_sha) + yield from report_larger_than_base(results_dir, all_versions, minifier_sha, base_sha) + yield from report_slowest(results_dir, ['3.14'], minifier_sha, base_sha) + yield from report_unstable(results_dir, all_versions, minifier_sha) + yield from report_exceptions(results_dir, all_versions, minifier_sha) def main(): diff --git a/docs/source/conf.py b/docs/source/conf.py index 288e6826..ebe48517 100644 --- a/docs/source/conf.py +++ b/docs/source/conf.py @@ -48,11 +48,18 @@ def create_example(option): 'prefer_single_line': False } + with open(f'minification_options/{option}.py') as source: + source_code = source.read() + + # Output with the option enabled options[option] = True + with open(f'minification_options/{option}.min.py', 'w') as minified: + minified.write(minify(source_code, filename=f'{option}.py', **options)) - with open(f'minification_options/{option}.py') as source: - with open(f'minification_options/{option}.min.py', 'w') as minified: - minified.write(minify(source.read(), filename=f'{option}.py', **options)) + # Output with the option disabled, for pages that show both + options[option] = False + with open(f'minification_options/{option}_false.min.py', 'w') as minified: + minified.write(minify(source_code, filename=f'{option}.py', **options)) for file in os.listdir('minification_options'): if file.endswith('.py') and not file.endswith('.min.py'): diff --git a/docs/source/minification_options/combine_imports.min.py b/docs/source/minification_options/combine_imports.min.py deleted file mode 100644 index 07404519..00000000 --- a/docs/source/minification_options/combine_imports.min.py +++ /dev/null @@ -1,3 +0,0 @@ -import requests,collections -from typing import Dict,List,Optional -import sys,os \ No newline at end of file diff --git a/docs/source/minification_options/constant_folding.min.py b/docs/source/minification_options/constant_folding.min.py deleted file mode 100644 index 68af9431..00000000 --- a/docs/source/minification_options/constant_folding.min.py +++ /dev/null @@ -1,2 +0,0 @@ -SECONDS_IN_A_DAY=86400 -SECONDS_IN_A_WEEK=SECONDS_IN_A_DAY*7 \ No newline at end of file diff --git a/docs/source/minification_options/convert_posargs_to_args.min.py b/docs/source/minification_options/convert_posargs_to_args.min.py deleted file mode 100644 index 9598b85f..00000000 --- a/docs/source/minification_options/convert_posargs_to_args.min.py +++ /dev/null @@ -1,6 +0,0 @@ -def name(p1,p2,p_or_kw,*,kw):pass -def name(p1,p2=None,p_or_kw=None,*,kw):pass -def name(p1,p2=None,*,kw):pass -def name(p1,p2=None):pass -def name(p1,p2,p_or_kw):pass -def name(p1,p2):pass \ No newline at end of file diff --git a/docs/source/minification_options/hoist_literals.min.py b/docs/source/minification_options/hoist_literals.min.py deleted file mode 100644 index 6e074e1e..00000000 --- a/docs/source/minification_options/hoist_literals.min.py +++ /dev/null @@ -1,12 +0,0 @@ -def validate(arn,props): - H='Value';G='Type';F='Name';E='ValidationStatus';D='PENDING_VALIDATION';C=False;B='ValidationMethod';A='ResourceRecord' - if B in props and props[B]=='DNS': - all_records_created=C - while not all_records_created: - all_records_created=True;certificate=acm.describe_certificate(CertificateArn=arn)['Certificate'] - if certificate['Status']!=D:return - for v in certificate['DomainValidationOptions']: - if E not in v or A not in v:all_records_created=C;continue - records=[] - if v[E]==D:records.append({'Action':'UPSERT','ResourceRecordSet':{F:v[A][F],G:v[A][G],'TTL':60,'ResourceRecords':[{H:v[A][H]}]}}) - if records:response=boto3.client('route53').change_resource_record_sets(HostedZoneId=get_zone_for(v['DomainName'],props),ChangeBatch={'Comment':'Domain validation for %s'%arn,'Changes':records}) \ No newline at end of file diff --git a/docs/source/minification_options/index.rst b/docs/source/minification_options/index.rst index 6de332d7..e19b2eab 100644 --- a/docs/source/minification_options/index.rst +++ b/docs/source/minification_options/index.rst @@ -21,6 +21,7 @@ They can be enabled or disabled through the minify function, or passing options remove_explicit_return_none remove_builtin_exception_brackets constant_folding + remove_dead_branches .. toctree:: :caption: Disabled by default diff --git a/docs/source/minification_options/prefer_single_line.min.py b/docs/source/minification_options/prefer_single_line.min.py deleted file mode 100644 index 8a4a1ee8..00000000 --- a/docs/source/minification_options/prefer_single_line.min.py +++ /dev/null @@ -1 +0,0 @@ -import os;import sys;name='world';print('Hello, '+name) \ No newline at end of file diff --git a/docs/source/minification_options/prefer_single_line_false.min.py b/docs/source/minification_options/prefer_single_line_false.min.py deleted file mode 100644 index c526a35f..00000000 --- a/docs/source/minification_options/prefer_single_line_false.min.py +++ /dev/null @@ -1,3 +0,0 @@ -import os,sys -name="world" -print("Hello, "+name) diff --git a/docs/source/minification_options/preserve_shebang.min.py b/docs/source/minification_options/preserve_shebang.min.py deleted file mode 100644 index 873b46b0..00000000 --- a/docs/source/minification_options/preserve_shebang.min.py +++ /dev/null @@ -1,3 +0,0 @@ -#!/usr/bin/python -import sys -print(sys.executable) \ No newline at end of file diff --git a/docs/source/minification_options/remove_annotations.min.py b/docs/source/minification_options/remove_annotations.min.py deleted file mode 100644 index b0104f9e..00000000 --- a/docs/source/minification_options/remove_annotations.min.py +++ /dev/null @@ -1,3 +0,0 @@ -class A: - b:0;c=2 - def a(self,val):b:0;c=2 \ No newline at end of file diff --git a/docs/source/minification_options/remove_asserts.min.py b/docs/source/minification_options/remove_asserts.min.py deleted file mode 100644 index 60ef6f3e..00000000 --- a/docs/source/minification_options/remove_asserts.min.py +++ /dev/null @@ -1,2 +0,0 @@ -word='hello' -print(word) \ No newline at end of file diff --git a/docs/source/minification_options/remove_builtin_exception_brackets.min.py b/docs/source/minification_options/remove_builtin_exception_brackets.min.py deleted file mode 100644 index bcce5fef..00000000 --- a/docs/source/minification_options/remove_builtin_exception_brackets.min.py +++ /dev/null @@ -1,2 +0,0 @@ -class MyBaseClass: - def override_me(self):raise NotImplementedError \ No newline at end of file diff --git a/docs/source/minification_options/remove_dead_branches.py b/docs/source/minification_options/remove_dead_branches.py new file mode 100644 index 00000000..37f2da4d --- /dev/null +++ b/docs/source/minification_options/remove_dead_branches.py @@ -0,0 +1,11 @@ +if False: + print('never runs') + +if True: + print('always runs') +else: + print('never runs') + +DEBUG = False +if DEBUG: + print('DEBUG is not a literal, so this is left alone') diff --git a/docs/source/minification_options/remove_dead_branches.rst b/docs/source/minification_options/remove_dead_branches.rst new file mode 100644 index 00000000..31743c40 --- /dev/null +++ b/docs/source/minification_options/remove_dead_branches.rst @@ -0,0 +1,36 @@ +Remove Dead Branches +==================== + +This transform removes ``if`` statement branches that can never run because the condition is a constant. + +An ``if`` statement with a constant ``False`` condition is removed, keeping its ``else`` branch. An ``if`` statement +with a constant ``True`` condition is replaced by its body, dropping any ``else`` branch. Only the literals ``True`` +and ``False`` are treated as constants; the condition is not otherwise evaluated, so a name such as ``DEBUG`` is left +alone even if it was assigned a constant value. + +A branch is only removed when doing so does not change the meaning of the program. A branch is kept if removing it +would change the enclosing scope, for example if it contains a ``yield``, a ``global`` or ``nonlocal`` declaration, or +the only assignment to a local variable. At module and class scope, where name resolution is dynamic, unreachable +assignments and imports are removed. + +If a branch is removed and a statement is still required, it is replaced by a zero expression statement. + +This transform also removes the ``if __debug__`` branches marked by the :doc:`remove_debug` transform, so enabling +that transform requires this one to remain enabled. + +This transform is safe and enabled by default. Disable it by passing the ``remove_dead_branches=False`` argument to the +:func:`python_minifier.minify` function, or passing ``--no-remove-dead-branches`` to the pyminify command. + +Example +------- + +Input +~~~~~ + +.. literalinclude:: remove_dead_branches.py + +Output +~~~~~~ + +.. literalinclude:: remove_dead_branches.min.py + :language: python diff --git a/docs/source/minification_options/remove_debug.min.py b/docs/source/minification_options/remove_debug.min.py deleted file mode 100644 index 16e86260..00000000 --- a/docs/source/minification_options/remove_debug.min.py +++ /dev/null @@ -1,6 +0,0 @@ -value=10 -if not __debug__:value+=1 -if __debug__ is False:value+=1 -if __debug__ is not True:value+=1 -if __debug__==False:value+=1 -print(value) \ No newline at end of file diff --git a/docs/source/minification_options/remove_debug.rst b/docs/source/minification_options/remove_debug.rst index 051e6da8..5f573a0f 100644 --- a/docs/source/minification_options/remove_debug.rst +++ b/docs/source/minification_options/remove_debug.rst @@ -4,12 +4,16 @@ Remove Debug This transform removes ``if`` statements that test ``__debug__`` is ``True``. The builtin ``__debug__`` constant is True if Python is not started with the ``-O`` option. -This transform is only safe to use if the minified output will by run with the ``-O`` option, or +This transform is only safe to use if the minified output will be run with the ``-O`` option, or you are certain that any ``if`` statement that tests ``__debug__`` can be removed. -The condition is not evaluated. The statement is only removed if the condition exactly matches one of the forms in the example below. +The condition is not evaluated. A statement is only affected if the condition exactly matches one of the truthy forms +in the example below. -If a statement is required, the ``if`` statement will be replaced by a zero expression statement. +The removal is performed by the :doc:`remove_dead_branches` transform, which this transform marks the ``if`` statements +for. That transform must remain enabled (it is by default) for this option to have any effect, and it will keep a branch +that cannot be removed without changing the meaning of the program. If a branch is removed and a statement is still +required, it is replaced by a zero expression statement. The transform is disabled by default. Enable it by passing the ``remove_debug=True`` argument to the :func:`python_minifier.minify` function, or passing ``--remove-debug`` to the pyminify command. diff --git a/docs/source/minification_options/remove_explicit_return_none.min.py b/docs/source/minification_options/remove_explicit_return_none.min.py deleted file mode 100644 index bde322a9..00000000 --- a/docs/source/minification_options/remove_explicit_return_none.min.py +++ /dev/null @@ -1,4 +0,0 @@ -def important(a): - if a>3:return a - if a<2:return - a.adjust(1) \ No newline at end of file diff --git a/docs/source/minification_options/remove_literal_statements.min.py b/docs/source/minification_options/remove_literal_statements.min.py deleted file mode 100644 index 15a66540..00000000 --- a/docs/source/minification_options/remove_literal_statements.min.py +++ /dev/null @@ -1 +0,0 @@ -def test():0 \ No newline at end of file diff --git a/docs/source/minification_options/remove_object_base.min.py b/docs/source/minification_options/remove_object_base.min.py deleted file mode 100644 index b2497ebd..00000000 --- a/docs/source/minification_options/remove_object_base.min.py +++ /dev/null @@ -1 +0,0 @@ -class MyClass:pass \ No newline at end of file diff --git a/docs/source/minification_options/remove_pass.min.py b/docs/source/minification_options/remove_pass.min.py deleted file mode 100644 index 15a66540..00000000 --- a/docs/source/minification_options/remove_pass.min.py +++ /dev/null @@ -1 +0,0 @@ -def test():0 \ No newline at end of file diff --git a/docs/source/minification_options/rename_globals.min.py b/docs/source/minification_options/rename_globals.min.py deleted file mode 100644 index 47d4fe04..00000000 --- a/docs/source/minification_options/rename_globals.min.py +++ /dev/null @@ -1,5 +0,0 @@ -A=print -import collections as B -C=B.Counter([True,True,True,False,False]) -A('Contents:') -A(list(C)) \ No newline at end of file diff --git a/docs/source/minification_options/rename_locals.min.py b/docs/source/minification_options/rename_locals.min.py deleted file mode 100644 index 9c378370..00000000 --- a/docs/source/minification_options/rename_locals.min.py +++ /dev/null @@ -1,6 +0,0 @@ -def rename_locals_example(module,another_argument=False,third_argument=None): - B=module;A=third_argument - if A is None:A=[] - A.extend(B) - for C in B.things: - if another_argument is False or C.name in A:C.my_method() \ No newline at end of file diff --git a/src/python_minifier/__init__.py b/src/python_minifier/__init__.py index cb3a9b78..8b091d2b 100644 --- a/src/python_minifier/__init__.py +++ b/src/python_minifier/__init__.py @@ -25,6 +25,7 @@ from python_minifier.transforms.remove_annotations import RemoveAnnotations from python_minifier.transforms.remove_annotations_options import RemoveAnnotationsOptions from python_minifier.transforms.remove_asserts import RemoveAsserts +from python_minifier.transforms.remove_dead_branches import RemoveDeadBranches from python_minifier.transforms.remove_debug import RemoveDebug from python_minifier.transforms.remove_exception_brackets import remove_no_arg_exception_call from python_minifier.transforms.remove_explicit_return_none import RemoveExplicitReturnNone @@ -74,6 +75,7 @@ def minify( remove_builtin_exception_brackets=True, constant_folding=True, prefer_single_line=False, + remove_dead_branches=True ): """ Minify a python module @@ -109,6 +111,7 @@ def minify( :param bool remove_builtin_exception_brackets: If brackets should be removed when raising exceptions with no arguments :param bool constant_folding: If literal expressions should be evaluated :param bool prefer_single_line: If semi-colons should be preferred over newlines where there is no difference in output size + :param bool remove_dead_branches: If if-statements with a constant False test should be removed :rtype: str @@ -155,12 +158,15 @@ def minify( if remove_debug: module = RemoveDebug()(module) - if remove_explicit_return_none: - module = RemoveExplicitReturnNone()(module) - if constant_folding: module = FoldConstants()(module) + if remove_dead_branches: + module = RemoveDeadBranches()(module) + + if remove_explicit_return_none: + module = RemoveExplicitReturnNone()(module) + bind_names(module) resolve_names(module) diff --git a/src/python_minifier/__init__.pyi b/src/python_minifier/__init__.pyi index efeefb20..fde0d748 100644 --- a/src/python_minifier/__init__.pyi +++ b/src/python_minifier/__init__.pyi @@ -29,7 +29,8 @@ def minify( remove_explicit_return_none: bool = ..., remove_builtin_exception_brackets: bool = ..., constant_folding: bool = ..., - prefer_single_line: bool = ... + prefer_single_line: bool = ..., + remove_dead_branches: bool = ... ) -> Text: ... diff --git a/src/python_minifier/__main__.py b/src/python_minifier/__main__.py index 4941bb6a..83cd2dd7 100644 --- a/src/python_minifier/__main__.py +++ b/src/python_minifier/__main__.py @@ -250,6 +250,12 @@ def parse_args(): help='Disable evaluating literal expressions', dest='constant_folding', ) + minification_options.add_argument( + '--no-remove-dead-branches', + action='store_false', + help='Disable removing branches that are never executed', + dest='remove_dead_branches', + ) annotation_options = parser.add_argument_group('remove annotations options', 'Options that affect how annotations are removed') annotation_options.add_argument( @@ -305,6 +311,10 @@ def parse_args(): sys.stderr.write('error: --remove-class-attribute-annotations would do nothing when used with --no-remove-annotations\n') sys.exit(1) + if args.remove_debug and not args.remove_dead_branches: + sys.stderr.write('error: --remove-debug would do nothing when used with --no-remove-dead-branches\n') + sys.exit(1) + return args diff --git a/src/python_minifier/module_printer.py b/src/python_minifier/module_printer.py index 1a5c5971..c26d922b 100644 --- a/src/python_minifier/module_printer.py +++ b/src/python_minifier/module_printer.py @@ -602,7 +602,7 @@ def visit_match_case(self, node): if node.guard is not None: self.printer.keyword('if') - self.visit(node.guard) + self._expression(node.guard) self.printer.delimiter(':') self._suite(node.body) diff --git a/src/python_minifier/rename/rename_literals.py b/src/python_minifier/rename/rename_literals.py index e707cf9c..e3b110f6 100644 --- a/src/python_minifier/rename/rename_literals.py +++ b/src/python_minifier/rename/rename_literals.py @@ -1,3 +1,5 @@ +from collections import OrderedDict + import python_minifier.ast_compat as ast from python_minifier.ast_annotation import get_parent, set_parent @@ -118,7 +120,7 @@ class HoistLiterals(NodeVisitor): def __call__(self, module, ignore_slots=True): self.module = module self._ignore_slots = ignore_slots - self._hoisted = {} + self._hoisted = OrderedDict() self.visit(module) self.place_bindings() diff --git a/src/python_minifier/transforms/constant_folding.py b/src/python_minifier/transforms/constant_folding.py index 99ae678a..db0c2cd9 100644 --- a/src/python_minifier/transforms/constant_folding.py +++ b/src/python_minifier/transforms/constant_folding.py @@ -14,7 +14,7 @@ def is_foldable_constant(node): """ Check if a node is a constant expression that can participate in folding. - We can asume that children have already been folded, so foldable constants are either: + We can assume that children have already been folded, so foldable constants are either: - Simple literals (Num, NameConstant) - UnaryOp(USub/Invert) on a Num - these don't fold to shorter forms, so they remain after child visiting. UAdd and Not would have been @@ -42,7 +42,14 @@ def fold(self, node): # Evaluate the expression try: original_expression = unparse_expression(node) - original_value = safe_eval(original_expression) + + if isinstance(node, ast.UnaryOp) and isinstance(node.op, ast.Invert) and is_constant_node(node.operand, ast.NameConstant) and isinstance(node.operand.value, bool): + # ~ on a bool is deprecated from python 3.12, to be removed in 3.16. + if sys.version_info >= (3, 16): + return node + original_value = ~int(node.operand.value) + else: + original_value = safe_eval(original_expression) except Exception: return node @@ -119,6 +126,30 @@ def visit_BinOp(self, node): return self.fold(node) + def visit_BoolOp(self, node): + node.values = [self.visit(v) for v in node.values] + + # Check this is a constant expression that could be folded + if not all(is_foldable_constant(v) for v in node.values): + return node + + return self.fold(node) + + def visit_Compare(self, node): + node.left = self.visit(node.left) + node.comparators = [self.visit(c) for c in node.comparators] + + operands = [node.left] + node.comparators + if not all(is_foldable_constant(n) for n in operands): + return node + + if any(isinstance(op, (ast.Is, ast.IsNot)) for op in node.ops): + # Identity is only language-guaranteed for the singletons + if not all(is_constant_node(n, ast.NameConstant) for n in operands): + return node + + return self.fold(node) + def visit_UnaryOp(self, node): node.operand = self.visit(node.operand) diff --git a/src/python_minifier/transforms/remove_dead_branches.py b/src/python_minifier/transforms/remove_dead_branches.py new file mode 100644 index 00000000..a3f6f4e0 --- /dev/null +++ b/src/python_minifier/transforms/remove_dead_branches.py @@ -0,0 +1,291 @@ +import python_minifier.ast_compat as ast + +from python_minifier.transforms.suite_transformer import SuiteTransformer +from python_minifier.util import is_constant_node + +class NamespaceProperties(object): + """ + The analysed properties of a namespace, including the names defined in it and whether it is a generator + """ + + def __init__(self): + self.local_names = set() + self.nonlocal_names = set() + self.global_names = set() + self.is_generator = False + + def __eq__(self, other): + assert isinstance(other, NamespaceProperties) + return ( + self.local_names == other.local_names and + self.nonlocal_names == other.nonlocal_names and + self.global_names == other.global_names and + self.is_generator is other.is_generator + ) + + def __ne__(self, other): + return not self == other + + def __repr__(self): + return 'NamespaceProperties(local_names=%r, nonlocal_names=%r, global_names=%r, is_generator=%r)' % ( + self.local_names, self.nonlocal_names, self.global_names, self.is_generator + ) + + +def in_function_scope(namespace): + """ + Is this namespace nested in a function scope? + + A name bound anywhere in a class body - even in unreachable code - makes + loads of that name in the class body fall through to the global scope, + instead of an enclosing function scope cell. Removing such a binding is + only safe when there is no enclosing function scope to fall through to. + """ + while not isinstance(namespace, ast.Module): + if isinstance(namespace, (ast.FunctionDef, ast.AsyncFunctionDef)): + return True + namespace = namespace.namespace + return False + + +class RemoveDeadBranches(SuiteTransformer): + """ + Remove if statements where the condition tests False + """ + + def __call__(self, node): + # Suites that have been condemned during this run, but may not have been + # detached from the tree yet. Shared across all suites so that decisions + # in nested or sibling suites don't count already-removed bindings. + self._removed_suites = [] + return self.visit(node) + + def get_namespace_properties(self, namespace, removed_suites=None): + """ + Gather the binding properties of a namespace + + Walks the namespace and collects the names it binds (as local, nonlocal + and global name sets) and whether it is a generator. + + :param namespace: The namespace node to analyse + :param removed_suites: Suites to skip while walking, for + branches that are being removed. + :type removed_suites: list or None + :rtype: NamespaceProperties + """ + if removed_suites is None: + removed_suites = [] + + properties = NamespaceProperties() + + def explore_namespace(node): + binds_here = getattr(node, 'namespace', namespace) is namespace + + if binds_here: + if isinstance(node, ast.Name): + if isinstance(node.ctx, (ast.Store, ast.Del, ast.Param)): + properties.local_names.add(node.id) + elif isinstance(node, (ast.ClassDef, ast.FunctionDef, ast.AsyncFunctionDef)): + properties.local_names.add(node.name) + elif isinstance(node, ast.alias): + # What about import * + + if node.asname is not None: + properties.local_names.add(node.asname) + else: + properties.local_names.add(node.name.split('.')[0]) + elif isinstance(node, ast.arguments): + if isinstance(node.vararg, str): + properties.local_names.add(node.vararg) + if isinstance(node.kwarg, str): + properties.local_names.add(node.kwarg) + elif isinstance(node, ast.arg): + properties.local_names.add(node.arg) + elif isinstance(node, ast.ExceptHandler): + if isinstance(node.name, str): + properties.local_names.add(node.name) + + elif isinstance(node, ast.Global): + properties.global_names.update(node.names) + elif isinstance(node, ast.Nonlocal): + properties.nonlocal_names.update(node.names) + + elif isinstance(node, ast.MatchAs): + if isinstance(node.name, str): + properties.local_names.add(node.name) + elif isinstance(node, ast.MatchStar): + if isinstance(node.name, str): + properties.local_names.add(node.name) + elif isinstance(node, ast.MatchMapping): + if isinstance(node.rest, str): + properties.local_names.add(node.rest) + + elif isinstance(node, ast.TypeVar): + properties.local_names.add(node.name) + elif isinstance(node, ast.TypeVarTuple): + properties.local_names.add(node.name) + elif isinstance(node, ast.ParamSpec): + properties.local_names.add(node.name) + + elif isinstance(node, (ast.Yield, ast.YieldFrom)): + properties.is_generator = True + + # Skip recursing into the body field of child namespaces, as they can only affect the child namespace + # Other fields (decorators, annotations, arguments...) can affect this namespace, + # so we do want to recurse into them + skip_body = node is not namespace and isinstance(node, (ast.FunctionDef, ast.AsyncFunctionDef, ast.Lambda, ast.ClassDef)) + + for name, field in ast.iter_fields(node): + if field in removed_suites: + continue + + if skip_body and name == 'body': + continue + + if isinstance(field, ast.AST): + explore_namespace(field) + elif isinstance(field, list): + for item in field: + if isinstance(item, ast.AST): + explore_namespace(item) + + explore_namespace(namespace) + + return properties + + def changes_semantics(self, namespace, candidate_suite): + """ + Does removing this branch change the semantics of the program? + + :param namespace: The namespace of the If statement + :param candidate_suite: The branch that is being removed + :return: True if removing the branch changes the semantics of the program, False otherwise + """ + + # Gather properties in the namespace, excluding suites already condemned + properties = self.get_namespace_properties(namespace, removed_suites=self._removed_suites) + + # Gather properties in the namespace, additionally excluding the candidate branch + candidate_properties = self.get_namespace_properties(namespace, removed_suites=[candidate_suite] + self._removed_suites) + + declarations_changed = ( + properties.is_generator != candidate_properties.is_generator + or properties.nonlocal_names != candidate_properties.nonlocal_names + or properties.global_names != candidate_properties.global_names + ) + + if isinstance(namespace, ast.Module): + # The module namespace is dynamic, removing unreachable bindings doesn't + # change their resolution behaviour, so we can safely remove them. + return declarations_changed + elif isinstance(namespace, ast.ClassDef) and not in_function_scope(namespace): + # Class namespaces are also dynamic, but a binding in the class body stops + # loads deferring to an enclosing function scope cell, so unreachable local + # bindings can only be ignored when no function scope encloses the class. + return declarations_changed + else: + return properties != candidate_properties + + def remove_false_branches(self, node_list): + suite = [] + + for node in node_list: + if not isinstance(node, ast.If): + suite.append(self.visit(node)) + continue + + # This is an If statement + + if hasattr(node, 'resolved_test'): + condition = node.resolved_test + elif is_constant_node(node.test, ast.NameConstant) and isinstance(node.test.value, bool): + condition = node.test.value + else: + # The test is not a constant, so we can't remove this branch + suite.append(self.visit(node)) + continue + + # We can possibly remove one of the branches, but we need to check if it changes the semantics of the program + + if condition is True: + if self.changes_semantics(node.namespace, node.orelse): + # Removing the else branch changes the semantics of the program, so we can't remove it + suite.append(self.visit(node)) + else: + # The else branch is dead, keep only the body + self._removed_suites.append(node.orelse) + suite.extend(self.remove_false_branches(node.body)) + + elif condition is False: + if self.changes_semantics(node.namespace, node.body): + # Removing the body branch changes the semantics of the program, so we can't remove it + suite.append(self.visit(node)) + else: + # The body is dead, keep only the else branch + self._removed_suites.append(node.body) + suite.extend(self.remove_false_branches(node.orelse)) + + return suite + + def suite(self, node_list, parent): + + without_dead_branches = self.remove_false_branches(node_list) + + if len(without_dead_branches) == 0: + if isinstance(parent, ast.Module): + return [] + else: + return [self.add_child(ast.Expr(value=ast.Num(0)), parent=parent)] + + return without_dead_branches + + def _is_empty_suite(self, suite): + """ + Is the suite empty? + + An empty suite is either a zero length list, or a list containing a single expression statement that is the constant 0. + The constant 0 is used by various transforms as a placeholder for an empty suite, because not all suites can be empty (e.g. the body of a function or class). + + :param suite: + :type suite: list + :rtype: bool + """ + if len(suite) == 0: + return True + return len(suite) == 1 and isinstance(suite[0], ast.Expr) and is_constant_node(suite[0].value, ast.Num) and suite[0].value.n == 0 + + def visit_If(self, node): + node = super(RemoveDeadBranches, self).visit_If(node) + + # Empty orelse suite can be omitted + if self._is_empty_suite(node.orelse): + node.orelse = [] + + return node + + def visit_While(self, node): + node = super(RemoveDeadBranches, self).visit_While(node) + + # Empty orelse suite can be omitted + if self._is_empty_suite(node.orelse): + node.orelse = [] + + return node + + def visit_For(self, node): + node = super(RemoveDeadBranches, self).visit_For(node) + + # Empty orelse suite can be omitted + if self._is_empty_suite(node.orelse): + node.orelse = [] + + return node + + def visit_Try(self, node): + node = super(RemoveDeadBranches, self).visit_Try(node) + + # Empty orelse suite can be omitted + if self._is_empty_suite(node.orelse): + node.orelse = [] + + return node diff --git a/src/python_minifier/transforms/remove_debug.py b/src/python_minifier/transforms/remove_debug.py index 6d90e299..776f553e 100644 --- a/src/python_minifier/transforms/remove_debug.py +++ b/src/python_minifier/transforms/remove_debug.py @@ -8,9 +8,11 @@ class RemoveDebug(SuiteTransformer): """ - Remove if statements where the condition tests __debug__ is True + Mark if statements whose condition tests __debug__ is True as dead - If a statement is syntactically necessary, use an empty expression instead + The marked statements are removed by the RemoveDeadBranches transform, which + keeps any else branch and any branch that can't be removed without changing + the meaning of the program. """ def __call__(self, node): @@ -18,7 +20,11 @@ def __call__(self, node): def constant_value(self, node): if sys.version_info < (3, 4): - return node.id == 'True' + # True and False are Name nodes before python 3.4. + # The comparator may be any expression, so check it is a Name. + if isinstance(node, ast.Name) and node.id in ('True', 'False'): + return node.id == 'True' + return None elif is_constant_node(node, ast.NameConstant): return node.value return None @@ -61,14 +67,10 @@ def is_truthy_debug_comparison(): return True return False - def suite(self, node_list, parent): + def visit_If(self, node): + assert isinstance(node, ast.If) - without_debug = [self.visit(a) for a in filter(lambda n: not self.can_remove(n), node_list)] + if self.can_remove(node): + node.resolved_test = False - if len(without_debug) == 0: - if isinstance(parent, ast.Module): - return [] - else: - return [self.add_child(ast.Expr(value=ast.Num(0)), parent=parent)] - - return without_debug + return self.generic_visit(node) diff --git a/src/python_minifier/transforms/suite_transformer.py b/src/python_minifier/transforms/suite_transformer.py index b66afa2d..debf7b55 100644 --- a/src/python_minifier/transforms/suite_transformer.py +++ b/src/python_minifier/transforms/suite_transformer.py @@ -117,6 +117,51 @@ def visit_Try(self, node): return node + def visit_TryStar(self, node): + return self.visit_Try(node) + + def visit_TryFinally(self, node): + # Python 2 + node.body = self.suite(node.body, parent=node) + node.finalbody = self.suite(node.finalbody, parent=node) + return node + + def visit_TryExcept(self, node): + # Python 2 + node.body = self.suite(node.body, parent=node) + + node.handlers = [self.visit(h) for h in node.handlers] + + if node.orelse: + node.orelse = self.suite(node.orelse, parent=node) + + return node + + def visit_ExceptHandler(self, node): + if node.type is not None: + node.type = self.visit(node.type) + + if node.name is not None and isinstance(node.name, ast.AST): + # Python 2 uses a Name node + node.name = self.visit(node.name) + + node.body = self.suite(node.body, parent=node) + return node + + def visit_Match(self, node): + node.subject = self.visit(node.subject) + node.cases = [self.visit(c) for c in node.cases] + return node + + def visit_match_case(self, node): + node.pattern = self.visit(node.pattern) + + if node.guard is not None: + node.guard = self.visit(node.guard) + + node.body = self.suite(node.body, parent=node) + return node + def visit_While(self, node): node.test = self.visit(node.test) diff --git a/test/test_combine_imports.py b/test/test_combine_imports.py index 9f08f604..e2bec847 100644 --- a/test/test_combine_imports.py +++ b/test/test_combine_imports.py @@ -86,6 +86,25 @@ def test_import_in_function(): compare_ast(expected_ast, actual_ast) +def test_import_in_except_handler(): + # SuiteTransformer now visits except handler bodies + source = '''try: + pass +except Exception: + import collection as c + import builtins +''' + expected = '''try: + pass +except Exception: + import collection as c, builtins +''' + + expected_ast = ast.parse(expected) + actual_ast = combine_imports(ast.parse(source)) + compare_ast(expected_ast, actual_ast) + + def test_import_star(): source = ''' from breakfast import hashbrown diff --git a/test/test_folding.py b/test/test_folding.py index 5eaebf18..e112cd8f 100644 --- a/test/test_folding.py +++ b/test/test_folding.py @@ -544,8 +544,8 @@ def test_double_invert_folded(source, expected): ('-5 + True', '-4'), ('10 * False', '0'), ('True + True', '2'), - ('~True', '-2'), # ~1 = -2, shorter than ~True - ('~False', '-1'), # ~0 = -1, shorter than ~False + ('~True', '-2'), + ('~False', '-1') ] ) def test_mixed_numeric_bool_folded(source, expected): @@ -553,6 +553,136 @@ def test_mixed_numeric_bool_folded(source, expected): Test folding of expressions mixing numeric and boolean operands. Python treats True as 1 and False as 0 in numeric contexts. + + ~ on a bool is deprecated from python 3.12, so it is folded by computing the + value directly rather than evaluating it (which would emit a DeprecationWarning). + """ + if sys.version_info < (3, 4): + pytest.skip('NameConstant not in python < 3.4') + + run_test(source, expected) + + +@pytest.mark.parametrize( + ('source', 'expected'), [ + ('~True', '~True'), + ('~False', '~False'), + ('~~False', '~~False'), + ] +) +def test_invert_bool_not_folded_from_316(source, expected, monkeypatch): + """ + ~ on a bool is removed in python 3.16, where it raises. + The minified code should fail the same way the original would. + """ + if sys.version_info < (3, 4): + pytest.skip('NameConstant not in python < 3.4') + + monkeypatch.setattr(sys, 'version_info', (3, 16, 0)) + run_test(source, expected) + + +@pytest.mark.parametrize( + ('source', 'expected'), [ + ('1 or 2', '1'), + ('0 or 2', '2'), + ('1 and 2', '2'), + ('0 and 2', '0'), + ] +) +def test_boolop_numeric_folded(source, expected): + """ + Test that and/or expressions with numeric operands are folded. + + BoolOp returns an operand value, not necessarily a bool. + """ + run_test(source, expected) + + +@pytest.mark.parametrize( + ('source', 'expected'), [ + ('True and False', 'False'), + ('False or True', 'True'), + ('None or False', 'False'), + ('True and True and True', 'True'), + ] +) +def test_boolop_folded(source, expected): + """ + Test that and/or expressions with constant operands are folded. + """ + if sys.version_info < (3, 4): + pytest.skip('NameConstant not in python < 3.4') + + run_test(source, expected) + + +@pytest.mark.parametrize( + ('source', 'expected'), [ + # Partially constant expressions must not fold, even where + # short circuit evaluation makes the result predictable + ('True and x', 'True and x'), + ('True or x', 'True or x'), + ('x and False', 'x and False'), + + # fold() doesn't construct None constants, only bool and numbers + ('False or None', 'False or None'), + ] +) +def test_boolop_not_folded(source, expected): + """ + Test that and/or expressions with non-constant operands are not folded. + """ + if sys.version_info < (3, 4): + pytest.skip('NameConstant not in python < 3.4') + + run_test(source, expected) + + +@pytest.mark.parametrize( + ('source', 'expected'), [ + ('True is True', 'True'), + ('True is not False', 'True'), + ('None is None', 'True'), + ('True == True', 'True'), + ('1 < 2 < 3', 'True'), + ('3 < 2 < 1 < 0', 'False'), + ('1 == 1.0', 'True'), + ] +) +def test_compare_folded(source, expected): + """ + Test that comparisons of constants are folded. + """ + if sys.version_info < (3, 4): + pytest.skip('NameConstant not in python < 3.4') + + run_test(source, expected) + + +@pytest.mark.parametrize( + ('source', 'expected'), [ + # Not smaller than the folded result + ('1 < 2', '1 < 2'), + ('1 == 1', '1 == 1'), + + # Identity of non-singletons is implementation defined + ('1000 is 1000', '1000 is 1000'), + ('1000 is not 1000', '1000 is not 1000'), + ('1 < 2 is 2', '1 < 2 is 2'), + + # Raises at runtime, must be preserved + ('1 in 2', '1 in 2'), + ('1 < None', '1 < None'), + + # Not constant + ('x < 2', 'x < 2'), + ('True is x', 'True is x'), + ] +) +def test_compare_not_folded(source, expected): + """ + Test comparisons that must not be folded. """ if sys.version_info < (3, 4): pytest.skip('NameConstant not in python < 3.4') diff --git a/test/test_match.py b/test/test_match.py index 78ebbab8..171047fa 100644 --- a/test/test_match.py +++ b/test/test_match.py @@ -7,6 +7,34 @@ from python_minifier.ast_compare import compare_ast +def test_match_guard_unparse(): + if sys.version_info < (3, 10): + pytest.skip('Match statement not in python < 3.10') + + source = ''' +match p: + case x if (1, 2): + pass + case x if a and b: + pass + case x if a if b else c: + pass + case x if a < b < c: + pass + case x if (y := f()): + pass + +def g(): + match p: + case x if (yield): + pass +''' + + expected_ast = ast.parse(source) + actual_ast = unparse(expected_ast) + compare_ast(expected_ast, ast.parse(actual_ast)) + + def test_pep635_unparse(): if sys.version_info < (3, 10): pytest.skip('Match statement not in python < 3.10') diff --git a/test/test_remove_assert.py b/test/test_remove_assert.py index b2253f31..6d648350 100644 --- a/test/test_remove_assert.py +++ b/test/test_remove_assert.py @@ -1,4 +1,7 @@ import ast +import sys + +import pytest from python_minifier.ast_annotation import add_parent from python_minifier.ast_compare import compare_ast @@ -116,3 +119,56 @@ def b(): 0''' expected_ast = ast.parse(expected) actual_ast = remove_asserts(source) compare_ast(expected_ast, actual_ast) + + +def test_remove_from_except_handler(): + source = '''try: + a() +except: + assert x +''' + expected = '''try: + a() +except: + 0 +''' + + expected_ast = ast.parse(expected) + actual_ast = remove_asserts(source) + compare_ast(expected_ast, actual_ast) + + +def test_remove_from_finally(): + source = '''try: + a() +finally: + assert x + b() +''' + expected = '''try: + a() +finally: + b() +''' + + expected_ast = ast.parse(expected) + actual_ast = remove_asserts(source) + compare_ast(expected_ast, actual_ast) + + +def test_remove_from_match_case(): + if sys.version_info < (3, 10): + pytest.skip('Match statement not in python < 3.10') + + source = '''match x: + case 1: + assert y +''' + expected = '''match x: + case 1: + 0 +''' + + expected_ast = ast.parse(expected) + actual_ast = remove_asserts(source) + compare_ast(expected_ast, actual_ast) diff --git a/test/test_remove_dead_branches.py b/test/test_remove_dead_branches.py new file mode 100644 index 00000000..6a9bafa5 --- /dev/null +++ b/test/test_remove_dead_branches.py @@ -0,0 +1,658 @@ +import ast +import sys + +import pytest + +from python_minifier.ast_annotation import add_parent +from python_minifier.ast_compare import compare_ast +from python_minifier.rename import add_namespace, bind_names, resolve_names +from python_minifier.transforms.remove_dead_branches import RemoveDeadBranches + + +def parse(source): + module = ast.parse(source, 'remove_dead_branches') + + add_parent(module) + add_namespace(module) + bind_names(module) + resolve_names(module) + return module + + +def remove_dead_branches(source): + return RemoveDeadBranches()(parse(source)) + + +def run_test(source, expected): + expected_ast = ast.parse(expected) + actual_ast = remove_dead_branches(source) + compare_ast(expected_ast, actual_ast) + + +def skip_if_no_nameconstant(): + if sys.version_info < (3, 4): + pytest.skip('NameConstant not in python < 3.4') + + +# region constant tests + +def test_removes_false_branch(): + skip_if_no_nameconstant() + run_test(''' +if False: + a() +else: + b() +''', 'b()') + + +def test_removes_true_else_branch(): + skip_if_no_nameconstant() + run_test(''' +if True: + a() +else: + b() +''', 'a()') + + +def test_removes_bare_false_branch(): + skip_if_no_nameconstant() + run_test(''' +if False: + a() +b() +''', 'b()') + + +def test_elif_chain_collapses(): + skip_if_no_nameconstant() + run_test(''' +if False: + a() +elif False: + b() +elif x: + c() +else: + d() +''', ''' +if x: + c() +else: + d() +''') + + +def test_pads_empty_function(): + skip_if_no_nameconstant() + run_test(''' +def f(): + if False: + a() +''', ''' +def f(): + 0 +''') + + +def test_pads_empty_class(): + skip_if_no_nameconstant() + run_test(''' +class A: + if False: + a() +''', ''' +class A: + 0 +''') + + +def test_no_stray_padding_mid_suite(): + skip_if_no_nameconstant() + run_test(''' +def f(): + x = 1 + if False: + a() + return x +''', ''' +def f(): + x = 1 + return x +''') + + +def test_pads_live_if_body(): + skip_if_no_nameconstant() + run_test(''' +if x: + if False: + a() +else: + b() +''', ''' +if x: + 0 +else: + b() +''') + + +def test_non_constant_test_kept(): + run_test(''' +if x: + a() +''', ''' +if x: + a() +''') + + +def test_none_test_kept(): + skip_if_no_nameconstant() + run_test(''' +if None: + a() +''', ''' +if None: + a() +''') + + +def test_number_test_kept(): + run_test(''' +if 0: + a() +if 1: + b() +''', ''' +if 0: + a() +if 1: + b() +''') + + +def test_name_constants_kept_before_34(): + # On python 2 (and < 3.4) True and False are reassignable names + if sys.version_info >= (3, 4): + pytest.skip('True and False are NameConstant in python >= 3.4') + + run_test(''' +if True: + a() +if False: + b() +''', ''' +if True: + a() +if False: + b() +''') + +# endregion + +# region resolved_test marks + +def test_resolved_test_false(): + module = parse(''' +if x: + a() +else: + b() +''') + module.body[0].resolved_test = False + compare_ast(ast.parse('b()'), RemoveDeadBranches()(module)) + + +def test_resolved_test_true(): + module = parse(''' +if x: + a() +else: + b() +''') + module.body[0].resolved_test = True + compare_ast(ast.parse('a()'), RemoveDeadBranches()(module)) + + +def test_resolved_test_overrides_constant(): + skip_if_no_nameconstant() + + module = parse(''' +if True: + a() +else: + b() +''') + module.body[0].resolved_test = False + compare_ast(ast.parse('b()'), RemoveDeadBranches()(module)) + +# endregion + +# region symbol table guards + +def test_keeps_sole_local_binding(): + skip_if_no_nameconstant() + source = ''' +def f(): + if False: + x = 1 + return x +''' + run_test(source, source) + + +def test_removes_local_binding_bound_elsewhere(): + skip_if_no_nameconstant() + run_test(''' +def f(): + if False: + x = 1 + x = 2 + return x +''', ''' +def f(): + x = 2 + return x +''') + + +def test_keeps_generator_yield(): + skip_if_no_nameconstant() + source = ''' +def f(): + if False: + yield +''' + run_test(source, source) + + +def test_keeps_global_declaration(): + skip_if_no_nameconstant() + source = ''' +def f(): + if False: + global x + x = 1 +''' + run_test(source, source) + + +def test_keeps_nonlocal_declaration(): + skip_if_no_nameconstant() + source = ''' +def f(): + x = 1 + def g(): + if False: + nonlocal x + x = 2 +''' + run_test(source, source) + + +def test_nested_function_binding_does_not_mask(): + # g's local x must not count as a binding of x in f + skip_if_no_nameconstant() + source = ''' +def f(): + if False: + x = 1 + def g(): + x = 2 + return x +''' + run_test(source, source) + + +def test_sibling_dead_branches_keep_one_binding(): + # Both branches can't be removed, or x would stop being a local. + # The first branch found is removed greedily. + skip_if_no_nameconstant() + run_test(''' +def f(): + if False: + x = 1 + if False: + x = 2 + return x +''', ''' +def f(): + if False: + x = 2 + return x +''') + + +def test_nested_dead_branches_keep_one_binding(): + # The dead branch inside the removed True branch is spliced into the + # same suite, and must still count the condemned sibling binding + skip_if_no_nameconstant() + run_test(''' +def f(): + if False: + x = 1 + if True: + if False: + x = 2 + return x +''', ''' +def f(): + if False: + x = 2 + return x +''') + + +def test_keeps_namedexpr_in_comprehension(): + if sys.version_info < (3, 8): + pytest.skip('NamedExpr not in python < 3.8') + + source = ''' +def f(): + if False: + [x for x in range(3) if (y := x)] + return y +''' + run_test(source, source) + + +def test_removes_when_namedexpr_bound_elsewhere(): + # The same NamedExpr binding also exists outside the dead branch, so removing + # the branch does not change f's bindings and it can be removed. + if sys.version_info < (3, 8): + pytest.skip('NamedExpr not in python < 3.8') + + run_test(''' +def f(): + if False: + [x for x in range(3) if (y := x)] + [x for x in range(3) if (y := x)] + return y +''', ''' +def f(): + [x for x in range(3) if (y := x)] + return y +''') + +# endregion + +# region module and class namespaces + +def test_removes_module_binding(): + skip_if_no_nameconstant() + run_test(''' +if False: + DEBUG = True +print(1) +''', 'print(1)') + + +def test_removes_module_import(): + skip_if_no_nameconstant() + run_test(''' +if False: + import logging +print(1) +''', 'print(1)') + + +def test_keeps_module_dead_nonlocal(): + # The original is a compile time SyntaxError, removing the branch shouldn't fix it + skip_if_no_nameconstant() + source = ''' +if False: + nonlocal x +''' + run_test(source, source) + + +def test_keeps_module_dead_yield(): + # The original is a compile time SyntaxError, removing the branch shouldn't fix it + skip_if_no_nameconstant() + source = ''' +if False: + yield +''' + run_test(source, source) + + +def test_removes_class_binding(): + # Class name resolution is dynamic when no function scope encloses the class + skip_if_no_nameconstant() + run_test(''' +class C: + if False: + x = 1 + y = x +''', ''' +class C: + y = x +''') + + +def test_keeps_class_binding_in_function(): + # A binding in the class body stops loads deferring to the enclosing + # function scope cell, so it can't be removed + skip_if_no_nameconstant() + source = ''' +def f(): + x = 'cell' + class C: + if False: + x = 1 + y = x + return C.y +''' + run_test(source, source) + + +def test_removes_class_branch_in_function_without_bindings(): + skip_if_no_nameconstant() + run_test(''' +def f(): + class C: + if False: + a() + b() +''', ''' +def f(): + class C: + b() +''') + + +def test_keeps_class_global_declaration(): + # global in a class body applies to the whole class body + skip_if_no_nameconstant() + source = ''' +class C: + if False: + global x + x = 1 +''' + run_test(source, source) + +# endregion + +# region suite coverage + +def test_removes_from_except_handler(): + skip_if_no_nameconstant() + run_test(''' +try: + f() +except: + if False: + g() + h() +''', ''' +try: + f() +except: + h() +''') + + +def test_removes_from_finally(): + skip_if_no_nameconstant() + run_test(''' +try: + f() +finally: + if False: + g() + h() +''', ''' +try: + f() +finally: + h() +''') + + +def test_removes_from_match_case(): + if sys.version_info < (3, 10): + pytest.skip('Match statement not in python < 3.10') + + run_test(''' +match x: + case 1: + if False: + g() + h() +''', ''' +match x: + case 1: + h() +''') + +# endregion + +# region orelse cleanup + +def test_drops_emptied_if_else(): + # A live if whose else is emptied by dead-branch removal drops the else + # rather than keeping the 0 padding + skip_if_no_nameconstant() + run_test(''' +if x: + a() +else: + if False: + b() +''', ''' +if x: + a() +''') + + +def test_keeps_live_if_else(): + # The else still has a live statement, so it is kept + skip_if_no_nameconstant() + run_test(''' +if x: + a() +else: + if False: + b() + c() +''', ''' +if x: + a() +else: + c() +''') + + +def test_drops_user_written_else_zero(): + # A bare 0 expression else is a no-op, so it is dropped + run_test(''' +if x: + a() +else: + 0 +''', ''' +if x: + a() +''') + + +def test_keeps_padded_if_body(): + # The body of a live if is not optional, so its 0 padding is kept + skip_if_no_nameconstant() + run_test(''' +if x: + if False: + a() +''', ''' +if x: + 0 +''') + + +def test_drops_emptied_while_else(): + skip_if_no_nameconstant() + run_test(''' +while x: + a() +else: + if False: + b() +''', ''' +while x: + a() +''') + + +def test_drops_emptied_for_else(): + skip_if_no_nameconstant() + run_test(''' +for i in x: + a() +else: + if False: + b() +''', ''' +for i in x: + a() +''') + + +def test_drops_emptied_try_else(): + skip_if_no_nameconstant() + run_test(''' +try: + a() +except: + b() +else: + if False: + c() +''', ''' +try: + a() +except: + b() +''') + + +def test_keeps_padded_try_finally(): + # finally can't be dropped when there are no handlers, or try: would be bare + skip_if_no_nameconstant() + run_test(''' +try: + a() +finally: + if False: + c() +''', ''' +try: + a() +finally: + 0 +''') + +# endregion diff --git a/test/test_remove_debug.py b/test/test_remove_debug.py index 9496c759..3f91134e 100644 --- a/test/test_remove_debug.py +++ b/test/test_remove_debug.py @@ -5,6 +5,7 @@ from python_minifier.ast_annotation import add_parent from python_minifier.ast_compare import compare_ast from python_minifier.rename import add_namespace, bind_names, resolve_names +from python_minifier.transforms.remove_dead_branches import RemoveDeadBranches from python_minifier.transforms.remove_debug import RemoveDebug @@ -15,7 +16,10 @@ def remove_debug(source): add_namespace(module) bind_names(module) resolve_names(module) - return RemoveDebug()(module) + + # RemoveDebug only marks debug branches as dead, RemoveDeadBranches removes them + module = RemoveDebug()(module) + return RemoveDeadBranches()(module) def test_remove_debug_empty_module(): @@ -41,9 +45,9 @@ def test_remove_debug_module(): def test_remove_if_empty(): - source = '''if True: + source = '''if x: if __debug__: pass''' - expected = '''if True: 0''' + expected = '''if x: 0''' expected_ast = ast.parse(expected) actual_ast = remove_debug(source) @@ -51,12 +55,12 @@ def test_remove_if_empty(): def test_remove_suite(): - source = '''if True: + source = '''if x: if __debug__: pass a=1 if __debug__: pass return None''' - expected = '''if True: + expected = '''if x: a=1 return None''' @@ -225,3 +229,151 @@ def check_is_internet_working(c): expected_ast = ast.parse(expected) actual_ast = remove_debug(source) compare_ast(expected_ast, actual_ast) + + +def test_keeps_else_branch(): + # Under -O the else branch is the live branch, it must survive + source = ''' +if __debug__: + a() +else: + b() +''' + expected = 'b()' + + expected_ast = ast.parse(expected) + actual_ast = remove_debug(source) + compare_ast(expected_ast, actual_ast) + + +def test_elif_debug_splices(): + source = ''' +if x: + a() +elif __debug__: + d() +else: + b() +''' + expected = ''' +if x: + a() +else: + b() +''' + + expected_ast = ast.parse(expected) + actual_ast = remove_debug(source) + compare_ast(expected_ast, actual_ast) + + +def test_keeps_generator_function(): + # Removing the only yield would stop this being a generator function + source = ''' +def f(): + if __debug__: + yield 1 +''' + expected = source + + expected_ast = ast.parse(expected) + actual_ast = remove_debug(source) + compare_ast(expected_ast, actual_ast) + + +def test_keeps_global_declaration(): + # The global declaration applies to the whole scope, even if unreachable + source = ''' +x = 1 +def f(): + if __debug__: + global x + x = 2 +''' + expected = source + + expected_ast = ast.parse(expected) + actual_ast = remove_debug(source) + compare_ast(expected_ast, actual_ast) + + +def test_keeps_sole_local_binding(): + # Removing the only binding of x would change UnboundLocalError into a global lookup + source = ''' +def f(): + if __debug__: + x = 1 + return x +''' + expected = source + + expected_ast = ast.parse(expected) + actual_ast = remove_debug(source) + compare_ast(expected_ast, actual_ast) + + +def test_removes_local_binding_bound_elsewhere(): + # x is still a local without the debug branch, so removal is safe + source = ''' +def f(): + if __debug__: + x = 1 + x = 2 + return x +''' + expected = ''' +def f(): + x = 2 + return x +''' + + expected_ast = ast.parse(expected) + actual_ast = remove_debug(source) + compare_ast(expected_ast, actual_ast) + + +def test_removes_module_binding(): + # Module name resolution is dynamic, an unreachable binding is invisible + source = ''' +if __debug__: + DEBUG = True +print(1) +''' + expected = 'print(1)' + + expected_ast = ast.parse(expected) + actual_ast = remove_debug(source) + compare_ast(expected_ast, actual_ast) + + +def test_removes_module_import(): + source = ''' +if __debug__: + import logging +print(1) +''' + expected = 'print(1)' + + expected_ast = ast.parse(expected) + actual_ast = remove_debug(source) + compare_ast(expected_ast, actual_ast) + + +def test_removes_from_except_handler(): + source = ''' +try: + f() +except: + if __debug__: + log() +''' + expected = ''' +try: + f() +except: + 0 +''' + + expected_ast = ast.parse(expected) + actual_ast = remove_debug(source) + compare_ast(expected_ast, actual_ast) diff --git a/test/test_remove_pass.py b/test/test_remove_pass.py index 4947ad0e..26ce64ff 100644 --- a/test/test_remove_pass.py +++ b/test/test_remove_pass.py @@ -1,4 +1,7 @@ import ast +import sys + +import pytest from python_minifier.ast_annotation import add_parent from python_minifier.ast_compare import compare_ast @@ -116,3 +119,79 @@ def b(): 0''' expected_ast = ast.parse(expected) actual_ast = remove_literals(source) compare_ast(expected_ast, actual_ast) + + +def test_remove_from_except_handler(): + source = '''try: + a() +except: + pass +''' + expected = '''try: + a() +except: + 0 +''' + + expected_ast = ast.parse(expected) + actual_ast = remove_literals(source) + compare_ast(expected_ast, actual_ast) + + +def test_remove_from_except_handler_suite(): + source = '''try: + a() +except Exception: + pass + b() + pass +''' + expected = '''try: + a() +except Exception: + b() +''' + + expected_ast = ast.parse(expected) + actual_ast = remove_literals(source) + compare_ast(expected_ast, actual_ast) + + +def test_remove_from_finally(): + source = '''try: + a() +finally: + pass +''' + expected = '''try: + a() +finally: + 0 +''' + + expected_ast = ast.parse(expected) + actual_ast = remove_literals(source) + compare_ast(expected_ast, actual_ast) + + +def test_remove_from_match_case(): + if sys.version_info < (3, 10): + pytest.skip('Match statement not in python < 3.10') + + source = '''match x: + case 1: + pass + case _: + pass + b() +''' + expected = '''match x: + case 1: + 0 + case _: + b() +''' + + expected_ast = ast.parse(expected) + actual_ast = remove_literals(source) + compare_ast(expected_ast, actual_ast)