Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

New codemod to remove global lines at module level #194

Merged
merged 3 commits into from
Jan 3, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
19 changes: 19 additions & 0 deletions integration_tests/test_remove_module_global.py
Original file line number Diff line number Diff line change
@@ -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
4 changes: 4 additions & 0 deletions src/codemodder/scripts/generate_docs.py
Original file line number Diff line number Diff line change
Expand Up @@ -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.",
),
}


Expand Down
2 changes: 2 additions & 0 deletions src/core_codemods/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down Expand Up @@ -86,5 +87,6 @@
FlaskJsonResponseType,
ExceptionWithoutRaise,
LiteralOrNewObjectIdentity,
RemoveModuleGlobal,
],
)
10 changes: 10 additions & 0 deletions src/core_codemods/docs/pixee_python_remove-module-global.md
Original file line number Diff line number Diff line change
@@ -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
```
26 changes: 26 additions & 0 deletions src/core_codemods/remove_module_global.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
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 `global` Usage at Module Level"
REVIEW_GUIDANCE = ReviewGuidance.MERGE_WITHOUT_REVIEW
DESCRIPTION = "Remove `global` usage at module level."
REFERENCES: list = []

def leave_Global(
self, original_node: cst.Global, _
) -> Union[cst.Global, cst.RemovalSentinel,]:
clavedeluna marked this conversation as resolved.
Show resolved Hide resolved
if not self.filter_by_path_includes_or_excludes(
self.node_position(original_node)
):
return original_node

Check warning on line 21 in src/core_codemods/remove_module_global.py

View check run for this annotation

Codecov / codecov/patch

src/core_codemods/remove_module_global.py#L21

Added line #L21 was not covered by tests
scope = self.get_metadata(ScopeProvider, original_node)
if isinstance(scope, GlobalScope):
self.report_change(original_node)
return cst.RemovalSentinel.REMOVE
return original_node
63 changes: 63 additions & 0 deletions tests/codemods/test_remove_module_global.py
Original file line number Diff line number Diff line change
@@ -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
4 changes: 4 additions & 0 deletions tests/samples/module_global.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
price = 25
print("hello")
global price
price = 30
Loading