From 1de4e2e0ed68f172341c3248b5902afda44078b3 Mon Sep 17 00:00:00 2001 From: clavedeluna Date: Mon, 11 Mar 2024 13:22:48 -0300 Subject: [PATCH 1/8] codemod to fix missing self/cls --- src/codemodder/codemods/utils_mixin.py | 12 ++ .../pixee_python_fix-missing-self-or-cls.md | 0 src/core_codemods/fix_missing_self_or_cls.py | 78 +++++++++++++ .../codemods/test_fix_missing_self_or_cls.py | 110 ++++++++++++++++++ 4 files changed, 200 insertions(+) create mode 100644 src/core_codemods/docs/pixee_python_fix-missing-self-or-cls.md create mode 100644 src/core_codemods/fix_missing_self_or_cls.py create mode 100644 tests/codemods/test_fix_missing_self_or_cls.py diff --git a/src/codemodder/codemods/utils_mixin.py b/src/codemodder/codemods/utils_mixin.py index 5047bd57..4e7cfe75 100644 --- a/src/codemodder/codemods/utils_mixin.py +++ b/src/codemodder/codemods/utils_mixin.py @@ -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] diff --git a/src/core_codemods/docs/pixee_python_fix-missing-self-or-cls.md b/src/core_codemods/docs/pixee_python_fix-missing-self-or-cls.md new file mode 100644 index 00000000..e69de29b diff --git a/src/core_codemods/fix_missing_self_or_cls.py b/src/core_codemods/fix_missing_self_or_cls.py new file mode 100644 index 00000000..424d2bca --- /dev/null +++ b/src/core_codemods/fix_missing_self_or_cls.py @@ -0,0 +1,78 @@ +import libcst as cst + +from codemodder.codemods.utils_mixin import NameResolutionMixin +from core_codemods.api import Metadata, ReviewGuidance, SimpleCodemod + + +class FixMissingSelfOrCls(SimpleCodemod, NameResolutionMixin): + metadata = Metadata( + name="fix-missing-self-or-cls", + review_guidance=ReviewGuidance.MERGE_AFTER_CURSORY_REVIEW, + summary="todo", + references=[], + ) + + change_description = "todo" + + def __init__(self, *args, **kwargs): + super().__init__(*args, **kwargs) + self.current_class_name = None + + def visit_ClassDef(self, node: cst.ClassDef) -> bool: + self.current_class_name = node.name.value + return True + + def leave_ClassDef( + self, original_node: cst.ClassDef, updated_node: cst.ClassDef + ) -> cst.ClassDef: + self.current_class_name = None + return updated_node + + def leave_FunctionDef( + self, original_node: cst.FunctionDef, updated_node: cst.FunctionDef + ) -> cst.FunctionDef: + if self.current_class_name: + 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, + ) + ) diff --git a/tests/codemods/test_fix_missing_self_or_cls.py b/tests/codemods/test_fix_missing_self_or_cls.py new file mode 100644 index 00000000..e64a0135 --- /dev/null +++ b/tests/codemods/test_fix_missing_self_or_cls.py @@ -0,0 +1,110 @@ +import pytest + +from codemodder.codemods.test import BaseCodemodTest +from core_codemods.fix_missing_self_or_cls import FixMissingSelfOrCls + + +class TestFixMissingSelfOrCls(BaseCodemodTest): + codemod = FixMissingSelfOrCls + + def test_name(self): + assert self.codemod.name == "fix-missing-self-or-cls" + + def test_change(self, tmpdir): + input_code = """ + class A: + def method(): + pass + + @classmethod + def clsmethod(): + pass + + def __new__(): + pass + def __init_subclass__(): + pass + """ + expected = """ + class A: + def method(self): + pass + + @classmethod + def clsmethod(cls): + pass + + def __new__(cls): + pass + def __init_subclass__(cls): + pass + """ + self.run_and_assert(tmpdir, input_code, expected, num_changes=4) + + @pytest.mark.parametrize( + "code", + [ + """ + def my_function(): + pass + """, + """ + class A: + def method(self, arg): + pass + + @classmethod + def clsmethod(cls, arg): + pass + + @staticmethod + def my_static(): + pass + """, + """ + class MyBaseClass: + def __new__(*args, **kwargs): + pass + def __init_subclass__(**kwargs): + pass + """, + ], + ) + def test_no_change(self, tmpdir, code): + self.run_and_assert(tmpdir, code, code) + + def test_with_args_no_change(self, tmpdir): + input_code = """ + class A: + def method(one, two): + pass + + def method(*args, two=2): + pass + + def method(**kwargs): + pass + + @classmethod + def clsmethod(one, two): + pass + + @classmethod + def kls(**kwargs): + pass + """ + self.run_and_assert(tmpdir, input_code, input_code) + + # def test_exclude_line(self, tmpdir): + # input_code = ( + # expected + # ) = """ + # assert (1, 2) + # """ + # lines_to_exclude = [2] + # self.run_and_assert( + # tmpdir, + # input_code, + # expected, + # lines_to_exclude=lines_to_exclude, + # ) From 9035bf1ff98587a3e274f4e2ddacabcc98fc7259 Mon Sep 17 00:00:00 2001 From: clavedeluna Date: Mon, 11 Mar 2024 13:41:34 -0300 Subject: [PATCH 2/8] document missing self or cls codemod --- .../test_fix_missing_self_or_cls.py | 44 +++++++++++++++++++ src/codemodder/scripts/generate_docs.py | 4 ++ src/core_codemods/__init__.py | 2 + .../pixee_python_fix-missing-self-or-cls.md | 15 +++++++ src/core_codemods/fix_missing_self_or_cls.py | 6 +-- tests/samples/fix_missing_self_or_cls.py | 7 +++ 6 files changed, 75 insertions(+), 3 deletions(-) create mode 100644 integration_tests/test_fix_missing_self_or_cls.py create mode 100644 tests/samples/fix_missing_self_or_cls.py diff --git a/integration_tests/test_fix_missing_self_or_cls.py b/integration_tests/test_fix_missing_self_or_cls.py new file mode 100644 index 00000000..a830a4b5 --- /dev/null +++ b/integration_tests/test_fix_missing_self_or_cls.py @@ -0,0 +1,44 @@ +from codemodder.codemods.test import ( + BaseIntegrationTest, + original_and_expected_from_code_path, +) +from core_codemods.fix_missing_self_or_cls import FixMissingSelfOrCls + + +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 = FixMissingSelfOrCls.change_description + num_changes = 2 diff --git a/src/codemodder/scripts/generate_docs.py b/src/codemodder/scripts/generate_docs.py index 6fd25321..98937eb5 100644 --- a/src/codemodder/scripts/generate_docs.py +++ b/src/codemodder/scripts/generate_docs.py @@ -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 | { diff --git a/src/core_codemods/__init__.py b/src/core_codemods/__init__.py index dafd67cc..d3f5270f 100644 --- a/src/core_codemods/__init__.py +++ b/src/core_codemods/__init__.py @@ -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 @@ -127,6 +128,7 @@ DjangoModelWithoutDunderStr, TransformFixHasattrCall, FixDataclassDefaults, + FixMissingSelfOrCls, ], ) diff --git a/src/core_codemods/docs/pixee_python_fix-missing-self-or-cls.md b/src/core_codemods/docs/pixee_python_fix-missing-self-or-cls.md index e69de29b..cf63089d 100644 --- a/src/core_codemods/docs/pixee_python_fix-missing-self-or-cls.md +++ b/src/core_codemods/docs/pixee_python_fix-missing-self-or-cls.md @@ -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 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") +``` diff --git a/src/core_codemods/fix_missing_self_or_cls.py b/src/core_codemods/fix_missing_self_or_cls.py index 424d2bca..2b958a16 100644 --- a/src/core_codemods/fix_missing_self_or_cls.py +++ b/src/core_codemods/fix_missing_self_or_cls.py @@ -7,12 +7,12 @@ class FixMissingSelfOrCls(SimpleCodemod, NameResolutionMixin): metadata = Metadata( name="fix-missing-self-or-cls", - review_guidance=ReviewGuidance.MERGE_AFTER_CURSORY_REVIEW, - summary="todo", + review_guidance=ReviewGuidance.MERGE_WITHOUT_REVIEW, + summary="Fix Missing Positional Parameter for Instance and Class Methods", references=[], ) - change_description = "todo" + change_description = "Add `self` or `cls` parameter to instance or class method." def __init__(self, *args, **kwargs): super().__init__(*args, **kwargs) diff --git a/tests/samples/fix_missing_self_or_cls.py b/tests/samples/fix_missing_self_or_cls.py new file mode 100644 index 00000000..9cfad558 --- /dev/null +++ b/tests/samples/fix_missing_self_or_cls.py @@ -0,0 +1,7 @@ +class MyClass: + def instance_method(): + print("instance_method") + + @classmethod + def class_method(): + print("class_method") From 8925cdf82ef30e0a22bc8ddd3aeb54577e356ffc Mon Sep 17 00:00:00 2001 From: clavedeluna Date: Mon, 11 Mar 2024 13:49:06 -0300 Subject: [PATCH 3/8] add sonar fix or missing self or cls codemod --- src/core_codemods/fix_missing_self_or_cls.py | 28 +++++++++++++------ .../sonar/sonar_fix_missing_self_or_cls.py | 12 ++++++++ 2 files changed, 31 insertions(+), 9 deletions(-) create mode 100644 src/core_codemods/sonar/sonar_fix_missing_self_or_cls.py diff --git a/src/core_codemods/fix_missing_self_or_cls.py b/src/core_codemods/fix_missing_self_or_cls.py index 2b958a16..eb6f9df2 100644 --- a/src/core_codemods/fix_missing_self_or_cls.py +++ b/src/core_codemods/fix_missing_self_or_cls.py @@ -1,17 +1,15 @@ import libcst as cst +from codemodder.codemods.libcst_transformer import ( + LibcstResultTransformer, + LibcstTransformerPipeline, +) from codemodder.codemods.utils_mixin import NameResolutionMixin -from core_codemods.api import Metadata, ReviewGuidance, SimpleCodemod +from core_codemods.api import Metadata, ReviewGuidance +from core_codemods.api.core_codemod import CoreCodemod -class FixMissingSelfOrCls(SimpleCodemod, NameResolutionMixin): - metadata = Metadata( - name="fix-missing-self-or-cls", - review_guidance=ReviewGuidance.MERGE_WITHOUT_REVIEW, - summary="Fix Missing Positional Parameter for Instance and Class Methods", - references=[], - ) - +class FixMissingSelfOrClsTransformer(LibcstResultTransformer, NameResolutionMixin): change_description = "Add `self` or `cls` parameter to instance or class method." def __init__(self, *args, **kwargs): @@ -76,3 +74,15 @@ def has_no_args(self, node: cst.FunctionDef) -> bool: node.params.posonly_params, ) ) + + +FixMissingSelfOrCls = CoreCodemod( + metadata=Metadata( + name="fix-missing-self-or-cls", + review_guidance=ReviewGuidance.MERGE_WITHOUT_REVIEW, + summary="Fix Missing Positional Parameter for Instance and Class Methods", + references=[], + ), + transformer=LibcstTransformerPipeline(FixMissingSelfOrClsTransformer), + detector=None, +) diff --git a/src/core_codemods/sonar/sonar_fix_missing_self_or_cls.py b/src/core_codemods/sonar/sonar_fix_missing_self_or_cls.py new file mode 100644 index 00000000..8ab6dea7 --- /dev/null +++ b/src/core_codemods/sonar/sonar_fix_missing_self_or_cls.py @@ -0,0 +1,12 @@ +from codemodder.codemods.base_codemod import Reference +from codemodder.codemods.sonar import SonarCodemod +from core_codemods.fix_missing_self_or_cls import FixMissingSelfOrCls + +SonarNumpyNanEquality = SonarCodemod.from_core_codemod( + name="fix-missing-self-or-cls-S5719", + other=FixMissingSelfOrCls, + rules=["python:S5719"], + new_references=[ + Reference(url="https://rules.sonarsource.com/python/RSPEC-5719/"), + ], +) From 3a4eb7a0f39750d1057b2586a3c9864ca9f38361 Mon Sep 17 00:00:00 2001 From: clavedeluna Date: Mon, 11 Mar 2024 14:26:02 -0300 Subject: [PATCH 4/8] test sonar fix missing self or cls codemod --- .../test_fix_missing_self_or_cls.py | 7 +- .../test_sonar_fix_missing_self_or_cls.py | 45 +++++++++++++ src/codemodder/scripts/generate_docs.py | 5 ++ src/core_codemods/__init__.py | 2 + .../sonar/sonar_fix_missing_self_or_cls.py | 2 +- .../test_sonar_fix_missing_self_or_cls.py | 65 +++++++++++++++++++ 6 files changed, 123 insertions(+), 3 deletions(-) create mode 100644 integration_tests/test_sonar_fix_missing_self_or_cls.py create mode 100644 tests/codemods/test_sonar_fix_missing_self_or_cls.py diff --git a/integration_tests/test_fix_missing_self_or_cls.py b/integration_tests/test_fix_missing_self_or_cls.py index a830a4b5..afe6bac5 100644 --- a/integration_tests/test_fix_missing_self_or_cls.py +++ b/integration_tests/test_fix_missing_self_or_cls.py @@ -2,7 +2,10 @@ BaseIntegrationTest, original_and_expected_from_code_path, ) -from core_codemods.fix_missing_self_or_cls import FixMissingSelfOrCls +from core_codemods.fix_missing_self_or_cls import ( + FixMissingSelfOrCls, + FixMissingSelfOrClsTransformer, +) class TestFixMissingSelfOrCls(BaseIntegrationTest): @@ -40,5 +43,5 @@ class TestFixMissingSelfOrCls(BaseIntegrationTest): # fmt: on expected_line_change = "2" - change_description = FixMissingSelfOrCls.change_description + change_description = FixMissingSelfOrClsTransformer.change_description num_changes = 2 diff --git a/integration_tests/test_sonar_fix_missing_self_or_cls.py b/integration_tests/test_sonar_fix_missing_self_or_cls.py new file mode 100644 index 00000000..01bcc6b5 --- /dev/null +++ b/integration_tests/test_sonar_fix_missing_self_or_cls.py @@ -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 diff --git a/src/codemodder/scripts/generate_docs.py b/src/codemodder/scripts/generate_docs.py index 98937eb5..975773a8 100644 --- a/src/codemodder/scripts/generate_docs.py +++ b/src/codemodder/scripts/generate_docs.py @@ -309,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)", + ), } diff --git a/src/core_codemods/__init__.py b/src/core_codemods/__init__.py index d3f5270f..f5a731f5 100644 --- a/src/core_codemods/__init__.py +++ b/src/core_codemods/__init__.py @@ -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 @@ -144,5 +145,6 @@ SonarFlaskJsonResponseType, SonarDjangoJsonResponseType, SonarJwtDecodeVerify, + SonarFixMissingSelfOrCls, ], ) diff --git a/src/core_codemods/sonar/sonar_fix_missing_self_or_cls.py b/src/core_codemods/sonar/sonar_fix_missing_self_or_cls.py index 8ab6dea7..44c58c52 100644 --- a/src/core_codemods/sonar/sonar_fix_missing_self_or_cls.py +++ b/src/core_codemods/sonar/sonar_fix_missing_self_or_cls.py @@ -2,7 +2,7 @@ from codemodder.codemods.sonar import SonarCodemod from core_codemods.fix_missing_self_or_cls import FixMissingSelfOrCls -SonarNumpyNanEquality = SonarCodemod.from_core_codemod( +SonarFixMissingSelfOrCls = SonarCodemod.from_core_codemod( name="fix-missing-self-or-cls-S5719", other=FixMissingSelfOrCls, rules=["python:S5719"], diff --git a/tests/codemods/test_sonar_fix_missing_self_or_cls.py b/tests/codemods/test_sonar_fix_missing_self_or_cls.py new file mode 100644 index 00000000..39bda2ec --- /dev/null +++ b/tests/codemods/test_sonar_fix_missing_self_or_cls.py @@ -0,0 +1,65 @@ +import json + +from codemodder.codemods.test import BaseSASTCodemodTest +from core_codemods.sonar.sonar_fix_missing_self_or_cls import SonarFixMissingSelfOrCls + + +class TestSonarFixMissingSelfOrCls(BaseSASTCodemodTest): + codemod = SonarFixMissingSelfOrCls + tool = "sonar" + + def test_name(self): + assert self.codemod.name == "fix-missing-self-or-cls-S5719" + + def test_simple(self, tmpdir): + input_code = """ + class A: + def method(): + pass + + @classmethod + def clsmethod(): + pass + """ + expected_output = """ + class A: + def method(self): + pass + + @classmethod + def clsmethod(cls): + pass + """ + issues = { + "issues": [ + { + "rule": "python:S5719", + "status": "OPEN", + "component": "code.py", + "textRange": { + "startLine": 2, + "endLine": 2, + "startOffset": 4, + "endOffset": 25, + }, + }, + { + "rule": "python:S5719", + "status": "OPEN", + "component": "code.py", + "textRange": { + "startLine": 6, + "endLine": 6, + "startOffset": 4, + "endOffset": 22, + }, + }, + ] + } + self.run_and_assert( + tmpdir, + input_code, + expected_output, + results=json.dumps(issues), + num_changes=2, + ) From 277058f0c3dd5b5494919a404f2865e2d3ffa29f Mon Sep 17 00:00:00 2001 From: Dani Alcala <112832187+clavedeluna@users.noreply.github.com> Date: Tue, 12 Mar 2024 16:03:39 -0300 Subject: [PATCH 5/8] Apply suggestions from code review Co-authored-by: Dan D'Avella --- src/core_codemods/docs/pixee_python_fix-missing-self-or-cls.md | 2 +- src/core_codemods/fix_missing_self_or_cls.py | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/core_codemods/docs/pixee_python_fix-missing-self-or-cls.md b/src/core_codemods/docs/pixee_python_fix-missing-self-or-cls.md index cf63089d..b06bc5bf 100644 --- a/src/core_codemods/docs/pixee_python_fix-missing-self-or-cls.md +++ b/src/core_codemods/docs/pixee_python_fix-missing-self-or-cls.md @@ -1,4 +1,4 @@ -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 the method/class method has no arguments defined. +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 --git a/src/core_codemods/fix_missing_self_or_cls.py b/src/core_codemods/fix_missing_self_or_cls.py index eb6f9df2..e023ee9e 100644 --- a/src/core_codemods/fix_missing_self_or_cls.py +++ b/src/core_codemods/fix_missing_self_or_cls.py @@ -80,7 +80,7 @@ def has_no_args(self, node: cst.FunctionDef) -> bool: metadata=Metadata( name="fix-missing-self-or-cls", review_guidance=ReviewGuidance.MERGE_WITHOUT_REVIEW, - summary="Fix Missing Positional Parameter for Instance and Class Methods", + summary="Add Missing Positional Parameter for Instance and Class Methods", references=[], ), transformer=LibcstTransformerPipeline(FixMissingSelfOrClsTransformer), From 13608e5e387d8483e309ad6e52d9de4bf2829fef Mon Sep 17 00:00:00 2001 From: clavedeluna Date: Wed, 13 Mar 2024 11:40:09 -0300 Subject: [PATCH 6/8] `path_to_root` should not include original node --- src/codemodder/codemods/utils_mixin.py | 4 ++-- tests/test_ancestorpatterns_mixin.py | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/src/codemodder/codemods/utils_mixin.py b/src/codemodder/codemods/utils_mixin.py index 4e7cfe75..a7876a00 100644 --- a/src/codemodder/codemods/utils_mixin.py +++ b/src/codemodder/codemods/utils_mixin.py @@ -449,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) diff --git a/tests/test_ancestorpatterns_mixin.py b/tests/test_ancestorpatterns_mixin.py index ba86e824..8c683b6e 100644 --- a/tests/test_ancestorpatterns_mixin.py +++ b/tests/test_ancestorpatterns_mixin.py @@ -46,7 +46,7 @@ def transform_module_impl(self, tree: cst.Module) -> cst.Module: node = stmt.body[0] path = self.path_to_root(node) - assert len(path) == 3 + assert len(path) == 2 return tree input_code = dedent( From dbfb1fb5932f91aa128baea9788ce6751818183a Mon Sep 17 00:00:00 2001 From: clavedeluna Date: Wed, 13 Mar 2024 11:47:58 -0300 Subject: [PATCH 7/8] fix missing self or cls will not change nested functions --- src/core_codemods/fix_missing_self_or_cls.py | 61 +++++++++---------- .../codemods/test_fix_missing_self_or_cls.py | 57 ++++++++++++----- 2 files changed, 72 insertions(+), 46 deletions(-) diff --git a/src/core_codemods/fix_missing_self_or_cls.py b/src/core_codemods/fix_missing_self_or_cls.py index e023ee9e..56b5d4d7 100644 --- a/src/core_codemods/fix_missing_self_or_cls.py +++ b/src/core_codemods/fix_missing_self_or_cls.py @@ -4,51 +4,50 @@ LibcstResultTransformer, LibcstTransformerPipeline, ) -from codemodder.codemods.utils_mixin import NameResolutionMixin +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, NameResolutionMixin): +class FixMissingSelfOrClsTransformer( + LibcstResultTransformer, NameAndAncestorResolutionMixin +): change_description = "Add `self` or `cls` parameter to instance or class method." - def __init__(self, *args, **kwargs): - super().__init__(*args, **kwargs) - self.current_class_name = None - - def visit_ClassDef(self, node: cst.ClassDef) -> bool: - self.current_class_name = node.name.value - return True - - def leave_ClassDef( - self, original_node: cst.ClassDef, updated_node: cst.ClassDef - ) -> cst.ClassDef: - self.current_class_name = None - return updated_node - def leave_FunctionDef( self, original_node: cst.FunctionDef, updated_node: cst.FunctionDef ) -> cst.FunctionDef: - if self.current_class_name: - 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: + # 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=self._pick_arg_name(original_node))] + 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: @@ -79,7 +78,7 @@ def has_no_args(self, node: cst.FunctionDef) -> bool: FixMissingSelfOrCls = CoreCodemod( metadata=Metadata( name="fix-missing-self-or-cls", - review_guidance=ReviewGuidance.MERGE_WITHOUT_REVIEW, + review_guidance=ReviewGuidance.MERGE_AFTER_CURSORY_REVIEW, summary="Add Missing Positional Parameter for Instance and Class Methods", references=[], ), diff --git a/tests/codemods/test_fix_missing_self_or_cls.py b/tests/codemods/test_fix_missing_self_or_cls.py index e64a0135..3116c6c1 100644 --- a/tests/codemods/test_fix_missing_self_or_cls.py +++ b/tests/codemods/test_fix_missing_self_or_cls.py @@ -41,6 +41,41 @@ def __init_subclass__(cls): """ self.run_and_assert(tmpdir, input_code, expected, num_changes=4) + def test_change_not_nested(self, tmpdir): + input_code = """ + class A: + def method(): + def inner(): + pass + + @classmethod + def clsmethod(): + def other_inner(): + pass + + def wrapper(): + class B: + def method(): + pass + """ + expected = """ + class A: + def method(self): + def inner(): + pass + + @classmethod + def clsmethod(cls): + def other_inner(): + pass + + def wrapper(): + class B: + def method(): + pass + """ + self.run_and_assert(tmpdir, input_code, expected, num_changes=2) + @pytest.mark.parametrize( "code", [ @@ -56,7 +91,7 @@ def method(self, arg): @classmethod def clsmethod(cls, arg): pass - + @staticmethod def my_static(): pass @@ -68,6 +103,12 @@ def __new__(*args, **kwargs): def __init_subclass__(**kwargs): pass """, + """ + class A(): + def f(self): + def g(): + pass + """, ], ) def test_no_change(self, tmpdir, code): @@ -94,17 +135,3 @@ def kls(**kwargs): pass """ self.run_and_assert(tmpdir, input_code, input_code) - - # def test_exclude_line(self, tmpdir): - # input_code = ( - # expected - # ) = """ - # assert (1, 2) - # """ - # lines_to_exclude = [2] - # self.run_and_assert( - # tmpdir, - # input_code, - # expected, - # lines_to_exclude=lines_to_exclude, - # ) From 1289a747544f1dc08bbcfc38932c0fa09f86c66f Mon Sep 17 00:00:00 2001 From: clavedeluna Date: Mon, 18 Mar 2024 09:10:21 -0300 Subject: [PATCH 8/8] convert sonar codemod to new api --- src/core_codemods/sonar/sonar_fix_missing_self_or_cls.py | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/src/core_codemods/sonar/sonar_fix_missing_self_or_cls.py b/src/core_codemods/sonar/sonar_fix_missing_self_or_cls.py index 44c58c52..d624c86c 100644 --- a/src/core_codemods/sonar/sonar_fix_missing_self_or_cls.py +++ b/src/core_codemods/sonar/sonar_fix_missing_self_or_cls.py @@ -1,12 +1,10 @@ -from codemodder.codemods.base_codemod import Reference 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, - rules=["python:S5719"], - new_references=[ - Reference(url="https://rules.sonarsource.com/python/RSPEC-5719/"), - ], + 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/", )