From bf27c58412b2c7493aebb5bbc809fd033a205b63 Mon Sep 17 00:00:00 2001 From: clavedeluna Date: Wed, 27 Dec 2023 11:11:10 -0300 Subject: [PATCH 1/3] add remove global at mod level codemod --- .../test_remove_module_global.py | 19 ++++++ src/codemodder/scripts/generate_docs.py | 4 ++ src/core_codemods/__init__.py | 2 + .../docs/pixee_python_remove-module-global.md | 10 +++ src/core_codemods/remove_module_global.py | 22 +++++++ tests/codemods/test_remove_module_global.py | 63 +++++++++++++++++++ tests/samples/module_global.py | 4 ++ 7 files changed, 124 insertions(+) create mode 100644 integration_tests/test_remove_module_global.py create mode 100644 src/core_codemods/docs/pixee_python_remove-module-global.md create mode 100644 src/core_codemods/remove_module_global.py create mode 100644 tests/codemods/test_remove_module_global.py create mode 100644 tests/samples/module_global.py diff --git a/integration_tests/test_remove_module_global.py b/integration_tests/test_remove_module_global.py new file mode 100644 index 00000000..5daf5fca --- /dev/null +++ b/integration_tests/test_remove_module_global.py @@ -0,0 +1,19 @@ +from core_codemods.remove_module_global import RemoveModuleGlobal +from integration_tests.base_test import ( + BaseIntegrationTest, + original_and_expected_from_code_path, +) + + +class TestRemoveModuleGlobal(BaseIntegrationTest): + codemod = RemoveModuleGlobal + code_path = "tests/samples/module_global.py" + original_code, _ = original_and_expected_from_code_path(code_path, []) + expected_new_code = """ +price = 25 +print("hello") +price = 30 +""".lstrip() + expected_diff = '--- \n+++ \n@@ -1,4 +1,3 @@\n price = 25\n print("hello")\n-global price\n price = 30\n' + expected_line_change = "3" + change_description = RemoveModuleGlobal.CHANGE_DESCRIPTION diff --git a/src/codemodder/scripts/generate_docs.py b/src/codemodder/scripts/generate_docs.py index d86f2153..43a972d7 100644 --- a/src/codemodder/scripts/generate_docs.py +++ b/src/codemodder/scripts/generate_docs.py @@ -186,6 +186,10 @@ class DocMetadata: importance="Low", guidance_explained="We believe this change is safe and will not cause any issues.", ), + "remove-module-global": DocMetadata( + importance="Low", + guidance_explained="Since the `global` keyword is intended for use in non-module scopes, using it at the module scope is unnecessary.", + ), } diff --git a/src/core_codemods/__init__.py b/src/core_codemods/__init__.py index 38a3e84c..1405bb16 100644 --- a/src/core_codemods/__init__.py +++ b/src/core_codemods/__init__.py @@ -40,6 +40,7 @@ from .exception_without_raise import ExceptionWithoutRaise from .literal_or_new_object_identity import LiteralOrNewObjectIdentity from .subprocess_shell_false import SubprocessShellFalse +from .remove_module_global import RemoveModuleGlobal registry = CodemodCollection( origin="pixee", @@ -86,5 +87,6 @@ FlaskJsonResponseType, ExceptionWithoutRaise, LiteralOrNewObjectIdentity, + RemoveModuleGlobal, ], ) diff --git a/src/core_codemods/docs/pixee_python_remove-module-global.md b/src/core_codemods/docs/pixee_python_remove-module-global.md new file mode 100644 index 00000000..3970caad --- /dev/null +++ b/src/core_codemods/docs/pixee_python_remove-module-global.md @@ -0,0 +1,10 @@ +Using the `global` keyword is necessary only when you intend to modify a module-level (aka global) variable within a non-global scope, such as within a class or function. It is unnecessary to call `global` at the module-level. + +Our changes look something like this: + +```diff + price = 25 + print("hello") +- global price + price = 30 +``` diff --git a/src/core_codemods/remove_module_global.py b/src/core_codemods/remove_module_global.py new file mode 100644 index 00000000..e320c436 --- /dev/null +++ b/src/core_codemods/remove_module_global.py @@ -0,0 +1,22 @@ +import libcst as cst +from libcst.metadata import GlobalScope, ScopeProvider +from typing import Union +from codemodder.codemods.api import BaseCodemod, ReviewGuidance +from codemodder.codemods.utils_mixin import NameResolutionMixin + + +class RemoveModuleGlobal(BaseCodemod, NameResolutionMixin): + NAME = "remove-module-global" + SUMMARY = "Remove Module-level Global Call" + REVIEW_GUIDANCE = ReviewGuidance.MERGE_WITHOUT_REVIEW + DESCRIPTION = "Remove Lines with `global` keyword at Module Level" + REFERENCES: list = [] + + def leave_Global( + self, original_node: cst.Global, _ + ) -> Union[cst.Global, cst.RemovalSentinel,]: + scope = self.get_metadata(ScopeProvider, original_node) + if isinstance(scope, GlobalScope): + self.report_change(original_node) + return cst.RemovalSentinel.REMOVE + return original_node diff --git a/tests/codemods/test_remove_module_global.py b/tests/codemods/test_remove_module_global.py new file mode 100644 index 00000000..12e95f79 --- /dev/null +++ b/tests/codemods/test_remove_module_global.py @@ -0,0 +1,63 @@ +from textwrap import dedent +from core_codemods.remove_module_global import RemoveModuleGlobal +from tests.codemods.base_codemod_test import BaseCodemodTest + + +class TestJwtDecodeVerify(BaseCodemodTest): + codemod = RemoveModuleGlobal + + def test_name(self): + assert self.codemod.name() == "remove-module-global" + + def test_simple(self, tmpdir): + input_code = """\ + price = 25 + global price + """ + expected = """\ + price = 25 + """ + self.run_and_assert(tmpdir, dedent(input_code), dedent(expected)) + assert len(self.file_context.codemod_changes) == 1 + + def test_reassigned(self, tmpdir): + input_code = """\ + price = 25 + print("hello") + global price + price = 30 + """ + expected = """\ + price = 25 + print("hello") + price = 30 + """ + self.run_and_assert(tmpdir, dedent(input_code), dedent(expected)) + assert len(self.file_context.codemod_changes) == 1 + + def test_attr_call(self, tmpdir): + input_code = """\ + class Price: + pass + PRICE = Price() + global PRICE + PRICE.__repr__ + """ + expected = """\ + class Price: + pass + PRICE = Price() + PRICE.__repr__ + """ + self.run_and_assert(tmpdir, dedent(input_code), dedent(expected)) + assert len(self.file_context.codemod_changes) == 1 + + def test_correct_scope(self, tmpdir): + input_code = """\ + price = 25 + def change_price(): + global price + price = 30 + """ + self.run_and_assert(tmpdir, dedent(input_code), dedent(input_code)) + assert len(self.file_context.codemod_changes) == 0 diff --git a/tests/samples/module_global.py b/tests/samples/module_global.py new file mode 100644 index 00000000..ff1d3eb2 --- /dev/null +++ b/tests/samples/module_global.py @@ -0,0 +1,4 @@ +price = 25 +print("hello") +global price +price = 30 From ea0d2ec23195e7cf56da331a17d4fff27eb19481 Mon Sep 17 00:00:00 2001 From: clavedeluna Date: Tue, 2 Jan 2024 17:04:23 -0300 Subject: [PATCH 2/3] add filter --- src/core_codemods/remove_module_global.py | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/src/core_codemods/remove_module_global.py b/src/core_codemods/remove_module_global.py index e320c436..8bbe67b5 100644 --- a/src/core_codemods/remove_module_global.py +++ b/src/core_codemods/remove_module_global.py @@ -15,6 +15,10 @@ class RemoveModuleGlobal(BaseCodemod, NameResolutionMixin): def leave_Global( self, original_node: cst.Global, _ ) -> Union[cst.Global, cst.RemovalSentinel,]: + if not self.filter_by_path_includes_or_excludes( + self.node_position(original_node) + ): + return original_node scope = self.get_metadata(ScopeProvider, original_node) if isinstance(scope, GlobalScope): self.report_change(original_node) From 1e0885c5417cdb44eb50ebc2098cc851812a261c Mon Sep 17 00:00:00 2001 From: clavedeluna Date: Tue, 2 Jan 2024 17:23:59 -0300 Subject: [PATCH 3/3] update description --- src/core_codemods/remove_module_global.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/core_codemods/remove_module_global.py b/src/core_codemods/remove_module_global.py index 8bbe67b5..1b404093 100644 --- a/src/core_codemods/remove_module_global.py +++ b/src/core_codemods/remove_module_global.py @@ -7,9 +7,9 @@ class RemoveModuleGlobal(BaseCodemod, NameResolutionMixin): NAME = "remove-module-global" - SUMMARY = "Remove Module-level Global Call" + SUMMARY = "Remove `global` Usage at Module Level" REVIEW_GUIDANCE = ReviewGuidance.MERGE_WITHOUT_REVIEW - DESCRIPTION = "Remove Lines with `global` keyword at Module Level" + DESCRIPTION = "Remove `global` usage at module level." REFERENCES: list = [] def leave_Global(