Skip to content

Commit

Permalink
Merge branch 'main' into chore/add-dependabot-triggered-updates
Browse files Browse the repository at this point in the history
  • Loading branch information
maxArturo committed Mar 18, 2024
2 parents ed828d3 + 4a48a20 commit 053cebd
Show file tree
Hide file tree
Showing 15 changed files with 449 additions and 34 deletions.
47 changes: 47 additions & 0 deletions integration_tests/test_fix_missing_self_or_cls.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,47 @@
from codemodder.codemods.test import (
BaseIntegrationTest,
original_and_expected_from_code_path,
)
from core_codemods.fix_missing_self_or_cls import (
FixMissingSelfOrCls,
FixMissingSelfOrClsTransformer,
)


class TestFixMissingSelfOrCls(BaseIntegrationTest):
codemod = FixMissingSelfOrCls
code_path = "tests/samples/fix_missing_self_or_cls.py"
original_code, expected_new_code = original_and_expected_from_code_path(
code_path,
[
(
1,
""" def instance_method(self):\n""",
),
(
5,
""" def class_method(cls):\n""",
),
],
)

# fmt: off
expected_diff = (
"""--- \n"""
"""+++ \n"""
"""@@ -1,7 +1,7 @@\n"""
""" class MyClass:\n"""
"""- def instance_method():\n"""
"""+ def instance_method(self):\n"""
""" print("instance_method")\n"""
""" \n"""
""" @classmethod\n"""
"""- def class_method():\n"""
"""+ def class_method(cls):\n"""
""" print("class_method")\n"""
)
# fmt: on

expected_line_change = "2"
change_description = FixMissingSelfOrClsTransformer.change_description
num_changes = 2
45 changes: 45 additions & 0 deletions integration_tests/test_sonar_fix_missing_self_or_cls.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,45 @@
from codemodder.codemods.test import (
BaseIntegrationTest,
original_and_expected_from_code_path,
)
from core_codemods.fix_missing_self_or_cls import FixMissingSelfOrClsTransformer
from core_codemods.sonar.sonar_fix_missing_self_or_cls import SonarFixMissingSelfOrCls


class TestSonarFixMissingSelfOrCls(BaseIntegrationTest):
codemod = SonarFixMissingSelfOrCls
code_path = "tests/samples/fix_missing_self_or_cls.py"
original_code, expected_new_code = original_and_expected_from_code_path(
code_path,
[
(
1,
""" def instance_method(self):\n""",
),
(
5,
""" def class_method(cls):\n""",
),
],
)
sonar_issues_json = "tests/samples/sonar_issues.json"
# fmt: off
expected_diff = (
"""--- \n"""
"""+++ \n"""
"""@@ -1,7 +1,7 @@\n"""
""" class MyClass:\n"""
"""- def instance_method():\n"""
"""+ def instance_method(self):\n"""
""" print("instance_method")\n"""
""" \n"""
""" @classmethod\n"""
"""- def class_method():\n"""
"""+ def class_method(cls):\n"""
""" print("class_method")\n"""
)
# fmt: on

expected_line_change = "2"
change_description = FixMissingSelfOrClsTransformer.change_description
num_changes = 2
5 changes: 0 additions & 5 deletions integration_tests/test_unnecessary_f_str.py
Original file line number Diff line number Diff line change
@@ -1,5 +1,3 @@
import pytest

from codemodder.codemods.test import (
BaseIntegrationTest,
original_and_expected_from_code_path,
Expand All @@ -10,9 +8,6 @@
)


@pytest.mark.skipif(
True, reason="May fail if it runs after test_sql_parameterization. See Issue #378."
)
class TestFStr(BaseIntegrationTest):
codemod = RemoveUnnecessaryFStr
code_path = "tests/samples/unnecessary_f_str.py"
Expand Down
16 changes: 14 additions & 2 deletions src/codemodder/codemods/utils_mixin.py
Original file line number Diff line number Diff line change
Expand Up @@ -277,6 +277,18 @@ def is_builtin_function(self, node: cst.Call):
return matchers.matches(node.func, matchers.Name())
return False

def is_staticmethod(self, node: cst.FunctionDef) -> bool:
for decorator in node.decorators:
if self.find_base_name(decorator.decorator) == "builtins.staticmethod":
return True
return False

def is_classmethod(self, node: cst.FunctionDef) -> bool:
for decorator in node.decorators:
if self.find_base_name(decorator.decorator) == "builtins.classmethod":
return True
return False

def find_accesses(self, node) -> Collection[Access]:
if scope := self.get_metadata(ScopeProvider, node, None):
return scope.accesses[node]
Expand Down Expand Up @@ -437,10 +449,10 @@ def find_immediate_class_def(self, node: cst.CSTNode) -> Optional[cst.ClassDef]:

def path_to_root(self, node: cst.CSTNode) -> list[cst.CSTNode]:
"""
Returns node's path to root. Includes self.
Returns node's path to `node` (excludes `node`).
"""
path = []
maybe_parent = node
maybe_parent = self.get_parent(node)
while maybe_parent:
path.append(maybe_parent)
maybe_parent = self.get_parent(maybe_parent)
Expand Down
9 changes: 9 additions & 0 deletions src/codemodder/scripts/generate_docs.py
Original file line number Diff line number Diff line change
Expand Up @@ -251,6 +251,10 @@ class DocMetadata:
importance="Medium",
guidance_explained="This change is safe and will prevent runtime `ValueError`.",
),
"fix-missing-self-or-cls": DocMetadata(
importance="Medium",
guidance_explained="This change is safe and will prevent errors when calling on these instance or class methods..",
),
}

METADATA = CORE_METADATA | {
Expand Down Expand Up @@ -305,6 +309,11 @@ class DocMetadata:
guidance_explained=CORE_METADATA["jwt-decode-verify"].guidance_explained,
need_sarif="Yes (Sonar)",
),
"fix-missing-self-or-cls-S5719": DocMetadata(
importance=CORE_METADATA["fix-missing-self-or-cls"].importance,
guidance_explained=CORE_METADATA["fix-missing-self-or-cls"].guidance_explained,
need_sarif="Yes (Sonar)",
),
}


Expand Down
9 changes: 6 additions & 3 deletions src/core_codemods/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@
from .fix_deprecated_logging_warn import FixDeprecatedLoggingWarn
from .fix_empty_sequence_comparison import FixEmptySequenceComparison
from .fix_hasattr_call import TransformFixHasattrCall
from .fix_missing_self_or_cls import FixMissingSelfOrCls
from .fix_mutable_params import FixMutableParams
from .flask_enable_csrf_protection import FlaskEnableCSRFProtection
from .flask_json_response_type import FlaskJsonResponseType
Expand All @@ -37,8 +38,7 @@
from .remove_debug_breakpoint import RemoveDebugBreakpoint
from .remove_future_imports import RemoveFutureImports
from .remove_module_global import RemoveModuleGlobal

# from .remove_unnecessary_f_str import RemoveUnnecessaryFStr
from .remove_unnecessary_f_str import RemoveUnnecessaryFStr
from .remove_unused_imports import RemoveUnusedImports
from .replace_flask_send_file import ReplaceFlaskSendFile
from .requests_verify import RequestsVerify
Expand All @@ -49,6 +49,7 @@
from .sonar.sonar_django_receiver_on_top import SonarDjangoReceiverOnTop
from .sonar.sonar_exception_without_raise import SonarExceptionWithoutRaise
from .sonar.sonar_fix_assert_tuple import SonarFixAssertTuple
from .sonar.sonar_fix_missing_self_or_cls import SonarFixMissingSelfOrCls
from .sonar.sonar_flask_json_response_type import SonarFlaskJsonResponseType
from .sonar.sonar_jwt_decode_verify import SonarJwtDecodeVerify
from .sonar.sonar_literal_or_new_object_identity import SonarLiteralOrNewObjectIdentity
Expand Down Expand Up @@ -89,7 +90,7 @@
OrderImports,
ProcessSandbox,
RemoveFutureImports,
# RemoveUnnecessaryFStr, # Temporarely disabled due to potential error. See Issue #378.
RemoveUnnecessaryFStr,
RemoveUnusedImports,
RequestsVerify,
SecureFlaskCookie,
Expand Down Expand Up @@ -128,6 +129,7 @@
DjangoModelWithoutDunderStr,
TransformFixHasattrCall,
FixDataclassDefaults,
FixMissingSelfOrCls,
],
)

Expand All @@ -143,5 +145,6 @@
SonarFlaskJsonResponseType,
SonarDjangoJsonResponseType,
SonarJwtDecodeVerify,
SonarFixMissingSelfOrCls,
],
)
15 changes: 15 additions & 0 deletions src/core_codemods/docs/pixee_python_fix-missing-self-or-cls.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
Python instance methods must be defined with `self` as the first argument. Likewise, class methods must have `cls` as the first argument. This codemod will add these arguments when the method/class method has no arguments defined.

Our changes look something like this:

```diff
class MyClass:
- def instance_method():
+ def instance_method(self):
print("instance_method")

@classmethod
- def class_method():
+ def class_method(cls):
print("class_method")
```
87 changes: 87 additions & 0 deletions src/core_codemods/fix_missing_self_or_cls.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,87 @@
import libcst as cst

from codemodder.codemods.libcst_transformer import (
LibcstResultTransformer,
LibcstTransformerPipeline,
)
from codemodder.codemods.utils_mixin import NameAndAncestorResolutionMixin
from core_codemods.api import Metadata, ReviewGuidance
from core_codemods.api.core_codemod import CoreCodemod


class FixMissingSelfOrClsTransformer(
LibcstResultTransformer, NameAndAncestorResolutionMixin
):
change_description = "Add `self` or `cls` parameter to instance or class method."

def leave_FunctionDef(
self, original_node: cst.FunctionDef, updated_node: cst.FunctionDef
) -> cst.FunctionDef:
# TODO: add filter by include or exclude that works for nodes
# that that have different start/end numbers.

if not self.find_immediate_class_def(original_node):
# If `original_node` is not inside a class, nothing to do.
return original_node

if self.find_immediate_function_def(original_node):
# If `original_node` is inside a class but also nested within a function/method
# We won't touch it.
return original_node

if original_node.decorators:
if self.is_staticmethod(original_node):
return updated_node
if self.is_classmethod(original_node):
if self.has_no_args(original_node):
self.report_change(original_node)
return updated_node.with_changes(
params=updated_node.params.with_changes(
params=[cst.Param(name=cst.Name("cls"))]
)
)
else:
if self.has_no_args(original_node):
self.report_change(original_node)
return updated_node.with_changes(
params=updated_node.params.with_changes(
params=[cst.Param(name=self._pick_arg_name(original_node))]
)
)
return updated_node

def _pick_arg_name(self, node: cst.FunctionDef) -> cst.Name:
match node.name:
case cst.Name(value="__new__") | cst.Name(value="__init_subclass__"):
new_name = "cls"
case _:
new_name = "self"
return cst.Name(value=new_name)

def has_no_args(self, node: cst.FunctionDef) -> bool:
converted_star_arg = (
None
if node.params.star_arg is cst.MaybeSentinel.DEFAULT
else node.params.star_arg
)
return not any(
(
node.params.params,
converted_star_arg,
node.params.kwonly_params,
node.params.star_kwarg,
node.params.posonly_params,
)
)


FixMissingSelfOrCls = CoreCodemod(
metadata=Metadata(
name="fix-missing-self-or-cls",
review_guidance=ReviewGuidance.MERGE_AFTER_CURSORY_REVIEW,
summary="Add Missing Positional Parameter for Instance and Class Methods",
references=[],
),
transformer=LibcstTransformerPipeline(FixMissingSelfOrClsTransformer),
detector=None,
)
18 changes: 6 additions & 12 deletions src/core_codemods/remove_unnecessary_f_str.py
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
from typing import cast

import libcst as cst
import libcst.matchers as m
from libcst.codemod import CodemodContext
from libcst.codemod.commands.unnecessary_format_string import UnnecessaryFormatString

from codemodder.codemods.libcst_transformer import (
Expand All @@ -11,18 +12,9 @@
from core_codemods.api.core_codemod import CoreCodemod


class RemoveUnnecessaryFStrTransform(LibcstResultTransformer, UnnecessaryFormatString):

class RemoveUnnecessaryFStrTransform(LibcstResultTransformer):
change_description = "Remove unnecessary f-string"

def __init__(
self, codemod_context: CodemodContext, *codemod_args, **codemod_kwargs
):
UnnecessaryFormatString.__init__(self, codemod_context)
LibcstResultTransformer.__init__(
self, codemod_context, *codemod_args, **codemod_kwargs
)

@m.leave(m.FormattedString(parts=(m.FormattedStringText(),)))
def _check_formatted_string(
self,
Expand All @@ -34,7 +26,9 @@ def _check_formatted_string(
):
return updated_node

transformed_node = super()._check_formatted_string(_original_node, updated_node)
transformed_node = UnnecessaryFormatString._check_formatted_string(
cast(UnnecessaryFormatString, self), _original_node, updated_node
)
if not _original_node.deep_equals(transformed_node):
self.report_change(_original_node)
return transformed_node
Expand Down
10 changes: 10 additions & 0 deletions src/core_codemods/sonar/sonar_fix_missing_self_or_cls.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
from codemodder.codemods.sonar import SonarCodemod
from core_codemods.fix_missing_self_or_cls import FixMissingSelfOrCls

SonarFixMissingSelfOrCls = SonarCodemod.from_core_codemod(
name="fix-missing-self-or-cls-S5719",
other=FixMissingSelfOrCls,
rule_id="python:S5719",
rule_name="Instance and class methods should have at least one positional parameter",
rule_url="https://rules.sonarsource.com/python/RSPEC-5719/",
)
Loading

0 comments on commit 053cebd

Please sign in to comment.