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

add include/exclude check to codemods that did not have it #208

Merged
merged 5 commits into from
Jan 16, 2024
Merged
Show file tree
Hide file tree
Changes from 3 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
2 changes: 2 additions & 0 deletions src/core_codemods/django_receiver_on_top.py
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,8 @@ def leave_FunctionDef(
) -> Union[
cst.BaseStatement, cst.FlattenSentinel[cst.BaseStatement], cst.RemovalSentinel
]:
# TODO: add filter by include or exclude that works for nodes
# that that have different start/end numbers.
maybe_receiver_with_index = None
for i, decorator in enumerate(original_node.decorators):
true_name = self.find_base_name(decorator.decorator)
Expand Down
5 changes: 5 additions & 0 deletions src/core_codemods/exception_without_raise.py
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,11 @@ def leave_SimpleStatementLine(
) -> Union[
cst.BaseStatement, cst.FlattenSentinel[cst.BaseStatement], cst.RemovalSentinel
]:
if not self.filter_by_path_includes_or_excludes(
self.node_position(original_node)
):
return original_node

match original_node:
case cst.SimpleStatementLine(
body=[cst.Expr(cst.Name() | cst.Attribute() as name)]
Expand Down
5 changes: 5 additions & 0 deletions src/core_codemods/fix_deprecated_abstractproperty.py
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,11 @@ class FixDeprecatedAbstractproperty(BaseCodemod, NameResolutionMixin):
def leave_Decorator(
self, original_node: cst.Decorator, updated_node: cst.Decorator
):
if not self.filter_by_path_includes_or_excludes(
self.node_position(original_node)
):
return original_node
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry for not realizing this before, but we should return updated_node here just in case some node within the original_node subtree was changed. Otherwise we may revert those previous changes.

I don't think it will affect the current codemods that contains this snippet, but this will probably be copy/pasted a lot, so it should include the "safe" version.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I noticed we don't always return updated_node like the docs indicate we should. I kinda wanted this to read "if we are excluding this node bc of line, don't change". so returning original_node reads "no change happened" but it's true that's not always the case.


if (
base_name := self.find_base_name(original_node.decorator)
) and base_name == "abc.abstractproperty":
Expand Down
2 changes: 2 additions & 0 deletions src/core_codemods/fix_mutable_params.py
Original file line number Diff line number Diff line change
Expand Up @@ -154,6 +154,8 @@ def leave_FunctionDef(
updated_node: cst.FunctionDef,
):
"""Transforms function definitions with mutable default parameters"""
# TODO: add filter by include or exclude that works for nodes
# that that have different start/end numbers.
(
updated_params,
new_var_decls,
Expand Down
5 changes: 5 additions & 0 deletions src/core_codemods/remove_unnecessary_f_str.py
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,11 @@ def _check_formatted_string(
_original_node: cst.FormattedString,
updated_node: cst.FormattedString,
):
if not self.filter_by_path_includes_or_excludes(
self.node_position(_original_node)
):
return _original_node
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same


transformed_node = super()._check_formatted_string(_original_node, updated_node)
if not _original_node.deep_equals(transformed_node):
self.report_change(_original_node)
Expand Down
5 changes: 5 additions & 0 deletions src/core_codemods/use_generator.py
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,11 @@ class UseGenerator(BaseCodemod, NameResolutionMixin):
]

def leave_Call(self, original_node: cst.Call, updated_node: cst.Call):
if not self.filter_by_path_includes_or_excludes(
self.node_position(original_node)
):
return original_node
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same


match original_node.func:
# NOTE: could also support things like `list` and `tuple`
# but it's a less compelling use case
Expand Down
2 changes: 2 additions & 0 deletions src/core_codemods/use_walrus_if.py
Original file line number Diff line number Diff line change
Expand Up @@ -118,6 +118,8 @@ def visit_If(self, node: cst.If):
)

def leave_If(self, original_node, updated_node):
# TODO: add filter by include or exclude that works for nodes
# that that have different start/end numbers.
if (result := self._if_stack.pop()) is not None:
position, named_expr = result
is_name = m.matches(updated_node.test, m.Name())
Expand Down
26 changes: 26 additions & 0 deletions tests/codemods/base_codemod_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,32 @@ def run_and_assert(self, tmpdir, input_code, expected):
tmp_file_path = Path(tmpdir / "code.py")
self.run_and_assert_filepath(tmpdir, tmp_file_path, input_code, expected)

def assert_no_change_line_excluded(
self, tmpdir, input_code, expected, lines_to_exclude
):
tmp_file_path = Path(tmpdir / "code.py")
input_tree = cst.parse_module(dedent(input_code))
self.execution_context = CodemodExecutionContext(
directory=tmpdir,
dry_run=True,
verbose=False,
registry=mock.MagicMock(),
repo_manager=mock.MagicMock(),
)

self.file_context = FileContext(
tmpdir,
tmp_file_path,
lines_to_exclude,
[],
[],
)
codemod_instance = self.initialize_codemod(input_tree)
output_tree = codemod_instance.transform_module(input_tree)

assert output_tree.code == dedent(expected)
assert len(self.file_context.codemod_changes) == 0

def run_and_assert_filepath(self, root, file_path, input_code, expected):
input_tree = cst.parse_module(dedent(input_code))
self.execution_context = CodemodExecutionContext(
Expand Down
10 changes: 10 additions & 0 deletions tests/codemods/test_exception_without_raise.py
Original file line number Diff line number Diff line change
Expand Up @@ -54,3 +54,13 @@ def test_raised_exception(self, tmpdir):
"""
self.run_and_assert(tmpdir, dedent(input_code), dedent(input_code))
assert len(self.file_context.codemod_changes) == 0

def test_exclude_line(self, tmpdir):
input_code = expected = """\
print(1)
ValueError("Bad value!")
"""
lines_to_exclude = [2]
self.assert_no_change_line_excluded(
tmpdir, input_code, expected, lines_to_exclude
)
14 changes: 14 additions & 0 deletions tests/codemods/test_fix_deprecated_abstractproperty.py
Original file line number Diff line number Diff line change
Expand Up @@ -121,3 +121,17 @@ def foo(self):
pass
"""
self.run_and_assert(tmpdir, original_code, new_code)

def test_exclude_line(self, tmpdir):
input_code = expected = """\
import abc

class A:
@abc.abstractproperty
def foo(self):
pass
"""
lines_to_exclude = [4]
self.assert_no_change_line_excluded(
tmpdir, input_code, expected, lines_to_exclude
)
9 changes: 9 additions & 0 deletions tests/codemods/test_remove_unnecessary_f_str.py
Original file line number Diff line number Diff line change
Expand Up @@ -32,3 +32,12 @@ def test_change(self, tmpdir):
"""
self.run_and_assert(tmpdir, before, after)
assert len(self.file_context.codemod_changes) == 3

def test_exclude_line(self, tmpdir):
input_code = expected = """\
bad: str = f"bad" + "bad"
"""
lines_to_exclude = [1]
self.assert_no_change_line_excluded(
tmpdir, input_code, expected, lines_to_exclude
)
9 changes: 9 additions & 0 deletions tests/codemods/test_use_generator.py
Original file line number Diff line number Diff line change
Expand Up @@ -29,3 +29,12 @@ def test_not_global_function(self, tmpdir):
x = any([i for i in range(10)])
"""
self.run_and_assert(tmpdir, original_code, expected)

def test_exclude_line(self, tmpdir):
input_code = expected = """\
x = any([i for i in range(10)])
"""
lines_to_exclude = [1]
self.assert_no_change_line_excluded(
tmpdir, input_code, expected, lines_to_exclude
)
Loading