From d2ffca7faa7f4bb962ca0dc6f089f20b17abaec3 Mon Sep 17 00:00:00 2001 From: Gary Thompson Date: Sun, 14 Jan 2024 13:58:54 +1000 Subject: [PATCH] Add module-level remove_unused_platforms option --- docs/source/transforms/index.rst | 1 + .../transforms/remove_unused_platforms.py | 34 ++ .../transforms/remove_unused_platforms.rst | 38 ++ src/python_minifier/__init__.py | 24 +- src/python_minifier/__main__.py | 39 +- .../remove_unused_platform_options.py | 27 ++ .../transforms/remove_unused_platforms.py | 172 +++++++++ test/test_remove_unused_platforms.py | 337 ++++++++++++++++++ 8 files changed, 668 insertions(+), 4 deletions(-) create mode 100644 docs/source/transforms/remove_unused_platforms.py create mode 100644 docs/source/transforms/remove_unused_platforms.rst create mode 100644 src/python_minifier/transforms/remove_unused_platform_options.py create mode 100644 src/python_minifier/transforms/remove_unused_platforms.py create mode 100644 test/test_remove_unused_platforms.py diff --git a/docs/source/transforms/index.rst b/docs/source/transforms/index.rst index eefb681d..b6fb2607 100644 --- a/docs/source/transforms/index.rst +++ b/docs/source/transforms/index.rst @@ -29,3 +29,4 @@ They can be enabled or disabled through the minify function, or passing options rename_globals remove_asserts remove_debug + remove_unused_platforms diff --git a/docs/source/transforms/remove_unused_platforms.py b/docs/source/transforms/remove_unused_platforms.py new file mode 100644 index 00000000..df5adc53 --- /dev/null +++ b/docs/source/transforms/remove_unused_platforms.py @@ -0,0 +1,34 @@ +value = 10 +_PLATFORM = "linux" # Normally this is derived from sys.uname or platform. + +# Supported Statements that will be kept in this example +if _PLATFORM == "linux": + value += 1 + + +# Supported Statements that will be removed in this example +if _PLATFORM == "armchair": + value += 1 + +# Basic if/elif can be used + +if _PLATFORM == "linux": + value += 1 +elif _PLATFORM == "armchair": + value += 1 + +# So can else +if _PLATFORM == "armchair": + value += 1 +else: + value += 1 + +# Statements that are not supported by PyMinify +if _PLATFORM: + value += 1 + +if _PLATFORM in ["linux", "windows"]: + value += 1 + + +print(value) diff --git a/docs/source/transforms/remove_unused_platforms.rst b/docs/source/transforms/remove_unused_platforms.rst new file mode 100644 index 00000000..3810a6a7 --- /dev/null +++ b/docs/source/transforms/remove_unused_platforms.rst @@ -0,0 +1,38 @@ +Remove Unused Platforms +======================= + +This transform removes ``if`` blocks that do not match a platform value. This only supports +module level if blocks and is configured to keep a single explicit match. + +The transform is disabled by default. + +When using the API, enable it by either passing ``remove_unused_platforms=True`` +argument to the :func:`python_minifier.minify`, or by passing ``remove_unused_platforms=unused_option`` +to the function where unused_option is an instance of :class:`RemoveUnusedPlatformOptions`. + +When using the pyminify command, enable it with ``--remove-unused-platforms`` and set the options +as required. + +Options +------- + +These arguments can be used with the pyminify command as shown by the following examples: + +``--platform-test-key=_PLATFORM`` The variable name that is testing the platform + +``--platform-preserve-value=linux`` The value that matches the target platform + + +Example +------- + +Input +~~~~~ + +.. literalinclude:: remove_unused_platforms.py + +Output +~~~~~~ + +.. literalinclude:: remove_unused_platforms.min.py + :language: python diff --git a/src/python_minifier/__init__.py b/src/python_minifier/__init__.py index bbeeeb62..b7641e55 100644 --- a/src/python_minifier/__init__.py +++ b/src/python_minifier/__init__.py @@ -32,6 +32,8 @@ from python_minifier.transforms.remove_object_base import RemoveObject from python_minifier.transforms.remove_pass import RemovePass from python_minifier.transforms.remove_posargs import remove_posargs +from python_minifier.transforms.remove_unused_platform_options import RemoveUnusedPlatformOptions +from python_minifier.transforms.remove_unused_platforms import RemoveUnusedPlatforms class UnstableMinification(RuntimeError): @@ -72,12 +74,13 @@ def minify( remove_debug=False, remove_explicit_return_none=True, remove_builtin_exception_brackets=True, - constant_folding=True + constant_folding=True, + remove_unused_platforms=RemoveUnusedPlatformOptions(), ): """ Minify a python module - The module is transformed according the the arguments. + The module is transformed according the arguments. If all transformation arguments are False, no transformations are made to the AST, the returned string will parse into exactly the same module. @@ -106,7 +109,8 @@ def minify( :param bool remove_explicit_return_none: If explicit return None statements should be replaced with a bare return :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 remove_unused_platforms: If top level platform masking blocks can be removed. + :type remove_unused_platforms: bool or RemoveUnusedPlatformOptions :rtype: str """ @@ -157,6 +161,20 @@ def minify( if constant_folding: module = FoldConstants()(module) + if isinstance(remove_unused_platforms, bool): + remove_unused_platforms_options = RemoveUnusedPlatformOptions( + platform_test_key=RemoveUnusedPlatformOptions.platform_test_key, + platform_preserve_value=RemoveUnusedPlatformOptions.platform_preserve_value, + ) + elif isinstance(remove_unused_platforms, RemoveUnusedPlatformOptions): + remove_unused_platforms_options = remove_unused_platforms + else: + raise TypeError('remove_unused_platforms must be a bool or RemoveUnusedPlatformOptions') + + if remove_unused_platforms_options: + module = RemoveUnusedPlatforms(remove_unused_platforms_options)(module) + + bind_names(module) resolve_names(module) diff --git a/src/python_minifier/__main__.py b/src/python_minifier/__main__.py index 6c81b4aa..8a30cca8 100644 --- a/src/python_minifier/__main__.py +++ b/src/python_minifier/__main__.py @@ -8,6 +8,7 @@ from python_minifier import minify from python_minifier.transforms.remove_annotations_options import RemoveAnnotationsOptions +from python_minifier.transforms.remove_unused_platform_options import RemoveUnusedPlatformOptions try: version = get_distribution('python_minifier').version @@ -229,6 +230,30 @@ def parse_args(): dest='remove_class_attribute_annotations', ) + + platform_options = parser.add_argument_group('remove unused platform options', 'Options that affect platform removal') + platform_options.add_argument( + '--remove-unused-platforms', + action='store_true', + help='Remove code blocks that are masked out for a specific platform', + dest='remove_unused_platforms', + ) + platform_options.add_argument( + '--platform-test-key', + type=str, + default="_PLATFORM", + help='The variable name that is testing for a platform', + dest='platform_test_key', + ) + platform_options.add_argument( + '--platform-preserve-value', + type=str, + default="linux", + help='The value that matches the target platform', + dest='platform_preserve_value', + ) + + parser.add_argument('--version', '-v', action='version', version=version) args = parser.parse_args() @@ -296,6 +321,17 @@ def do_minify(source, filename, minification_args): remove_class_attribute_annotations=minification_args.remove_class_attribute_annotations, ) + if minification_args.remove_unused_platforms is False: + remove_unused_platforms = RemoveUnusedPlatformOptions( + platform_test_key="", + platform_preserve_value="" + ) + else: + remove_unused_platforms = RemoveUnusedPlatformOptions( + platform_test_key=minification_args.platform_test_key, + platform_preserve_value=minification_args.platform_preserve_value + ) + return minify( source, filename=filename, @@ -315,7 +351,8 @@ def do_minify(source, filename, minification_args): remove_debug=minification_args.remove_debug, remove_explicit_return_none=minification_args.remove_explicit_return_none, remove_builtin_exception_brackets=minification_args.remove_exception_brackets, - constant_folding=minification_args.constant_folding + constant_folding=minification_args.constant_folding, + remove_unused_platforms=remove_unused_platforms ) diff --git a/src/python_minifier/transforms/remove_unused_platform_options.py b/src/python_minifier/transforms/remove_unused_platform_options.py new file mode 100644 index 00000000..518c1727 --- /dev/null +++ b/src/python_minifier/transforms/remove_unused_platform_options.py @@ -0,0 +1,27 @@ +class RemoveUnusedPlatformOptions(object): + """ + Options for the RemoveUnusedPlatform transform + + This can be passed to the minify function as the remove_unused_platforms argument + + :param platform_test_key: The key used to indicate a platform check statement + :type platform_test_key: str + :param platform_preserve_value: The value of the test to keep + :type platform_preserve_value: str + """ + + platform_test_key = "_PLATFORM" + platform_preserve_value = "linux" + + def __init__(self, platform_test_key="", platform_preserve_value=""): + self.platform_test_key = platform_test_key + self.platform_preserve_value = platform_preserve_value + + def __repr__(self): + return 'RemoveUnusedPlatformOptions(platform_test_key=%s, platform_preserve_value=%s)' % (self.platform_test_key, self.platform_preserve_value) + + def __nonzero__(self): + return any((self.platform_test_key, self.platform_preserve_value)) + + def __bool__(self): + return self.__nonzero__() diff --git a/src/python_minifier/transforms/remove_unused_platforms.py b/src/python_minifier/transforms/remove_unused_platforms.py new file mode 100644 index 00000000..4b94443b --- /dev/null +++ b/src/python_minifier/transforms/remove_unused_platforms.py @@ -0,0 +1,172 @@ +import ast + +from python_minifier.transforms.remove_unused_platform_options import ( + RemoveUnusedPlatformOptions, +) +from python_minifier.transforms.suite_transformer import SuiteTransformer + + +class RemoveUnusedPlatforms(SuiteTransformer): + """ + Remove if statements where the condition tests a platform check if statement + + If a statement is syntactically necessary, use an empty expression instead + + Modelled off the __debug__ test. If this needs extending, then that is a + good example. + """ + + def __init__(self, options): + assert isinstance(options, RemoveUnusedPlatformOptions) + self._options = options + super(RemoveUnusedPlatforms, self).__init__() + + def __call__(self, node): + return self.visit(node) + + @staticmethod + def is_single_equal_explicit_if_comparison(node): + """ + Is this a simple single operator, such as `left == 'right'` + """ + + return ( + isinstance(node, ast.If) + and isinstance(node.test, ast.Compare) + and len(node.test.ops) == 1 + and isinstance(node.test.ops[0], ast.Eq) + and len(node.test.comparators) == 1 + ) + + def split_platform_tests(self, node): + """ + Returns None if this is an unrelated or unsupported statement. + + Otherwise, returns a tuple with the following elements: + + A list of matching `if PLATFORM == Target:` nodes + A list of matching `if PLATFORM == NonTarget:` nodes + The open else nodes + """ + if not self.is_single_equal_explicit_if_comparison(node): + return None + + # Collect if node with multiple elif nodes + comparisons = [] + open_else_nodes = [] + current_node = node + while current_node is not None: + if not self.is_single_equal_explicit_if_comparison(current_node): + return None + comparisons.append(current_node) + if current_node.orelse: + if isinstance(current_node.orelse[0], ast.If): + # elif + current_node = current_node.orelse[0] + else: + # else body + open_else_nodes = current_node.orelse + current_node = None + else: + current_node = None + + matching_platform_test = [] + unmatched_platform_test = [] + other_tests = [] + + # Check each if statement for a platform match: + for if_comparison in comparisons: + test_key = None + test_value = None + left = if_comparison.test.left + if isinstance(left, ast.Constant): + test_value = left.value + elif isinstance(left, ast.Name): + test_key = left.id + + comparator = if_comparison.test.comparators[0] + if isinstance(comparator, ast.Constant): + test_value = comparator.value + elif isinstance(comparator, ast.Name): + test_key = comparator.id + + if test_key == self._options.platform_test_key: + if test_value == self._options.platform_preserve_value: + matching_platform_test.append(if_comparison) + else: + unmatched_platform_test.append(if_comparison) + else: + other_tests.append(if_comparison) + + if other_tests: + return None + + return matching_platform_test, unmatched_platform_test, open_else_nodes + + def can_remove(self, node): + target_test = self.split_platform_tests(node) + if target_test is None: + # No target tests, do not remove node + return False + elif target_test[1] and not target_test[0] and not target_test[2]: + # Only non-target platforms specified, can be removed + return True + else: + # Keep everything else + return False + + def flattened_node(self, node): + """ + Returns a modified node if the existing node can be flattened or removed. NOOP nodes + are returned as None. + """ + if isinstance(node, ast.Pass) or node == ast.Num(0): + return None + + target_split_nodes = self.split_platform_tests(node) + if target_split_nodes: + ( + matching_platform_test, + unmatched_platform_test, + open_else_nodes, + ) = target_split_nodes + + if not matching_platform_test and open_else_nodes: + matching_platform_bodies = [open_else_nodes] + else: + matching_platform_bodies = [n.body for n in matching_platform_test] + + combined_bodies = [] + + for m in matching_platform_bodies: + if len(m) == 1 and (isinstance(m[0], ast.Pass) or m[0] == ast.Num(0)): + pass + else: + combined_bodies += m + + return combined_bodies + + return node + + def suite(self, node_list, parent): + if not isinstance(parent, ast.Module): + return node_list + + # Logical Parse of the tree + without_unused_platform = [ + self.visit(a) for a in filter(lambda _n: not self.can_remove(_n), node_list) + ] + + # Clean up remaining NOOP statements or blocks + flattened_nodes = [] + for n in without_unused_platform: + processed_n = self.flattened_node(n) + if isinstance(processed_n, list): + flattened_nodes += processed_n + elif processed_n: + flattened_nodes.append(processed_n) + + if len(flattened_nodes) == 0: + return [] + + return flattened_nodes diff --git a/test/test_remove_unused_platforms.py b/test/test_remove_unused_platforms.py new file mode 100644 index 00000000..0816de82 --- /dev/null +++ b/test/test_remove_unused_platforms.py @@ -0,0 +1,337 @@ +import ast + +import pytest + +from python_minifier.ast_compare import compare_ast +from python_minifier.rename import add_namespace, bind_names, resolve_names +from python_minifier.transforms.remove_unused_platform_options import ( + RemoveUnusedPlatformOptions, +) +from python_minifier.transforms.remove_unused_platforms import RemoveUnusedPlatforms + + +def remove_unused_platform(source): + module = ast.parse(source, "remove_unused_platform") + + add_namespace(module) + bind_names(module) + resolve_names(module) + return RemoveUnusedPlatforms( + RemoveUnusedPlatformOptions( + platform_test_key=RemoveUnusedPlatformOptions.platform_test_key, + platform_preserve_value=RemoveUnusedPlatformOptions.platform_preserve_value, + ) + )(module) + + +def test_empty_module_remove_platform_no_equals__expect_no_change(): + source = "if _PLATFORM: pass" + expected = "if _PLATFORM: pass" + + expected_ast = ast.parse(expected) + actual_ast = remove_unused_platform(source) + compare_ast(expected_ast, actual_ast) + + +def test_empty_module_remove_platform_equals_non_target___expect_gone(): + source = 'if _PLATFORM == "bsd": pass' + expected = "" + + expected_ast = ast.parse(expected) + actual_ast = remove_unused_platform(source) + compare_ast(expected_ast, actual_ast) + + +def test_empty_module_remove_platform_equals_non_target_reverse_test___expect_gone(): + source = 'if "bsd" == _PLATFORM: pass' + expected = "" + + expected_ast = ast.parse(expected) + actual_ast = remove_unused_platform(source) + compare_ast(expected_ast, actual_ast) + + +def test_empty_module_remove_platform_equals_target___expect_flattened(): + source = 'if _PLATFORM == "linux": x=1' + expected = "x=1" + + expected_ast = ast.parse(expected) + actual_ast = remove_unused_platform(source) + compare_ast(expected_ast, actual_ast) + + +def test_remove_platform_simple_non_target_module__expect_removed(): + source = """ +import collections +if _PLATFORM == "bsd": pass +a = 1 +if _PLATFORM == "bsd": pass +""" + + expected = """import collections +a=1""" + + expected_ast = ast.parse(expected) + actual_ast = remove_unused_platform(source) + compare_ast(expected_ast, actual_ast) + + +def test_remove_platform_simple_target_module__expect_flattened(): + source = """ +import collections +if _PLATFORM == "linux": a = 1 +b = 2 +if _PLATFORM == "linux": c = 3 +""" + + expected = """import collections +a = 1 +b = 2 +c = 3 +""" + + expected_ast = ast.parse(expected) + actual_ast = remove_unused_platform(source) + compare_ast(expected_ast, actual_ast) + + +def test_do_not_remove_nested_if(): + """ + The intention is only to remove module level statements to keep the + behaviour simple and to encourage not having highly nested platform + specific code (see PEP20). + """ + + source = """ +if True: + if _PLATFORM == "linux": pass +""" + + expected = """if True: + if _PLATFORM == "linux": pass""" + + expected_ast = ast.parse(expected) + actual_ast = remove_unused_platform(source) + compare_ast(expected_ast, actual_ast) + + +def test_do_not_remove_from_class(): + """ + PEP20 + """ + + source = """class A: + if _PLATFORM == "linux": pass + a = 1 + if _PLATFORM == "linux": pass + def b(): + if _PLATFORM == "linux": pass + return 1 +""" + expected = source + + expected_ast = ast.parse(expected) + actual_ast = remove_unused_platform(source) + compare_ast(expected_ast, actual_ast) + + +def test_remove_platform_multi_target_if_module__expect_non_target_removed(): + source = """ +import collections +if _PLATFORM == "linux": pass +a = 1 +if _PLATFORM == "linux": pass + +if _PLATFORM == "bsd": + class A: + pass + +if _PLATFORM == "win": + class B: + pass + +if _PLATFORM == "linux": + class C: + pass + +""" + + expected = """import collections +a = 1 +class C: + pass""" + + expected_ast = ast.parse(expected) + actual_ast = remove_unused_platform(source) + compare_ast(expected_ast, actual_ast) + + +@pytest.mark.parametrize( + "source", + [ + # Target Platform in elif + """ +if _PLATFORM == "bsd": + class A: + pass + +elif _PLATFORM == "win": + class B: + pass + +elif _PLATFORM == "linux": + class C: + pass +else: + raise NotImplemented(_PLATFORM) + +""", + # Target Platform in if + """ +if _PLATFORM == "linux": + class C: + pass + +elif _PLATFORM == "win": + class B: + pass + +elif _PLATFORM == "bsd": + class A: + pass +else: + raise NotImplemented(_PLATFORM) + +""", + # Target Platform is else + """ +if _PLATFORM == "bsd": + class A: + pass + +elif _PLATFORM == "win": + class B: + pass + +elif _PLATFORM == "other": + raise NotImplemented(_PLATFORM) + +else: + class C: + pass +""", + ], +) +def test_remove_platform_multi_target_elif_module__expect_non_target_removed(source): + expected = """class C: + pass""" + + expected_ast = ast.parse(expected) + actual_ast = remove_unused_platform(source) + compare_ast(expected_ast, actual_ast) + + +@pytest.mark.parametrize( + "source", + [ + # "in" is not currently supported, irrespective of where + # it is in the if/elif block. + """ +if _PLATFORM == "bsd": + class A: + pass + +elif _PLATFORM in ["win", "mac"]: + class B: + pass + +elif _PLATFORM == "linux": + class C: + pass +else: + raise NotImplemented(_PLATFORM) + +""", + # "in" is not currently supported, irrespective of where + # it is in the if/elif block. + """ +if _PLATFORM == "bsd": + class A: + pass + +elif _PLATFORM == "win": + class B: + pass + +elif _PLATFORM in ["linux"]: + class C: + pass +else: + raise NotImplemented(_PLATFORM) + +""", + # any non-platform tests will also cause the block to be + # left along + """ +if _PLATFORM == "bsd": + class A: + pass + +elif some_variable == 23: + class D: + pass + +elif _PLATFORM == "win": + class B: + pass + +elif _PLATFORM == "linux": + class C: + pass +else: + raise NotImplemented(_PLATFORM) + +""", + ], +) +def test_unsupported_if_statements__expect_no_change(source): + expected = source + + expected_ast = ast.parse(expected) + actual_ast = remove_unused_platform(source) + compare_ast(expected_ast, actual_ast) + + +def test_elif_statements_no_target__expect_gone(): + source = """ +if _PLATFORM == "bsd": + class A: + pass + +elif _PLATFORM == "win": + class B: + pass +""" + + expected = "" + + expected_ast = ast.parse(expected) + actual_ast = remove_unused_platform(source) + compare_ast(expected_ast, actual_ast) + + +def test_if_statements_results_in_noop__expect_gone(): + source = """ +pass +if _PLATFORM == "linux": + pass +elif _PLATFORM == "win": + class B: + pass +""" + + expected = "" + + expected_ast = ast.parse(expected) + actual_ast = remove_unused_platform(source) + compare_ast(expected_ast, actual_ast)