diff --git a/integration_tests/test_django_model_without_dunder_str.py b/integration_tests/test_django_model_without_dunder_str.py new file mode 100644 index 00000000..0f1b16ae --- /dev/null +++ b/integration_tests/test_django_model_without_dunder_str.py @@ -0,0 +1,52 @@ +from core_codemods.django_model_without_dunder_str import ( + DjangoModelWithoutDunderStr, + DjangoModelWithoutDunderStrTransformer, +) +from codemodder.codemods.test import ( + BaseIntegrationTest, + original_and_expected_from_code_path, +) + + +class TestDjangoModelWithoutDunderStr(BaseIntegrationTest): + codemod = DjangoModelWithoutDunderStr + code_path = "tests/samples/django-project/mysite/mysite/models.py" + original_code, expected_new_code = original_and_expected_from_code_path( + code_path, + [ + (15, """\n"""), + (16, """ def __str__(self):\n"""), + (17, """ model_name = self.__class__.__name__\n"""), + ( + 18, + """ fields_str = ", ".join((f"{field.name}={getattr(self, field.name)}" for field in self._meta.fields))\n""", + ), + (19, """ return f"{model_name}({fields_str})"\n"""), + ], + ) + + # fmt: off + expected_diff =( + """--- \n""" + """+++ \n""" + """@@ -11,3 +11,8 @@\n""" + """ content = models.CharField(max_length=200)\n""" + """ class Meta:\n""" + """ app_label = 'myapp'\n""" + """+\n""" + """+ def __str__(self):\n""" + """+ model_name = self.__class__.__name__\n""" + """+ fields_str = ", ".join((f"{field.name}={getattr(self, field.name)}" for field in self._meta.fields))\n""" + """+ return f"{model_name}({fields_str})"\n""" + ) + # fmt: on + + expected_line_change = "9" + change_description = DjangoModelWithoutDunderStrTransformer.change_description + num_changed_files = 1 + + def check_code_after(self): + """Executes models.py and instantiates the model to ensure expected str representation""" + module = super().check_code_after() + inst = module.Message(pk=1, author="name", content="content") + assert str(inst) == "Message(id=1, author=name, content=content)" diff --git a/src/codemodder/codemods/test/integration_utils.py b/src/codemodder/codemods/test/integration_utils.py index 9e970f96..284a57f9 100644 --- a/src/codemodder/codemods/test/integration_utils.py +++ b/src/codemodder/codemods/test/integration_utils.py @@ -8,6 +8,7 @@ from codemodder import __version__ from codemodder import registry from .validations import execute_code +from types import ModuleType SAMPLES_DIR = "tests/samples" # Enable import of test modules from test directory @@ -139,11 +140,13 @@ def check_code_before(self): code = f.read() assert code == self.original_code - def check_code_after(self): - with open(self.code_path, "r", encoding="utf-8") as f: + def check_code_after(self) -> ModuleType: + with open(self.code_path, "r", encoding="utf-8") as f: # type: ignore new_code = f.read() assert new_code == self.expected_new_code - execute_code(path=self.code_path, allowed_exceptions=self.allowed_exceptions) + return execute_code( + path=self.code_path, allowed_exceptions=self.allowed_exceptions + ) def test_file_rewritten(self): """ diff --git a/src/codemodder/codemods/test/validations.py b/src/codemodder/codemods/test/validations.py index 915569f5..eab41c7c 100644 --- a/src/codemodder/codemods/test/validations.py +++ b/src/codemodder/codemods/test/validations.py @@ -1,5 +1,7 @@ import importlib.util import tempfile +from types import ModuleType +from typing import Optional def execute_code(*, path=None, code=None, allowed_exceptions=None): @@ -11,20 +13,29 @@ def execute_code(*, path=None, code=None, allowed_exceptions=None): ), "Must pass either path to code or code as a str." if path: - _run_code(path, allowed_exceptions) - return + return _run_code(path, allowed_exceptions) with tempfile.NamedTemporaryFile(suffix=".py", mode="w+t") as temp: temp.write(code) - _run_code(temp.name, allowed_exceptions) + return _run_code(temp.name, allowed_exceptions) -def _run_code(path, allowed_exceptions=None): - """Execute the code in `path` in its own namespace.""" +def _run_code(path, allowed_exceptions=None) -> Optional[ModuleType]: + """ + Execute the code in `path` in its own namespace. + Return loaded module for any additional testing later on. + """ allowed_exceptions = allowed_exceptions or () spec = importlib.util.spec_from_file_location("output_code", path) + if not spec: + return None + module = importlib.util.module_from_spec(spec) + if not spec.loader: + return None try: spec.loader.exec_module(module) except allowed_exceptions: pass + + return module diff --git a/src/codemodder/codemods/utils_mixin.py b/src/codemodder/codemods/utils_mixin.py index ee64e699..4715a1b4 100644 --- a/src/codemodder/codemods/utils_mixin.py +++ b/src/codemodder/codemods/utils_mixin.py @@ -281,6 +281,16 @@ def find_accesses(self, node) -> Collection[Access]: return scope.accesses[node] return {} + def class_has_method(self, classdef: cst.ClassDef, method_name: str) -> bool: + """Check if a given class definition implements a method of name `method_name`.""" + for node in classdef.body.body: + match node: + case cst.FunctionDef( + name=cst.Name(value=value) + ) if value == method_name: + return True + return False + class AncestorPatternsMixin(MetadataDependent): METADATA_DEPENDENCIES: ClassVar[Collection[ProviderT]] = (ParentNodeProvider,) diff --git a/src/codemodder/scripts/generate_docs.py b/src/codemodder/scripts/generate_docs.py index b10fbe10..88b25670 100644 --- a/src/codemodder/scripts/generate_docs.py +++ b/src/codemodder/scripts/generate_docs.py @@ -234,6 +234,10 @@ class DocMetadata: importance="Low", guidance_explained="Manual instantiation of `asyncio.Task` is discouraged. We believe this change is safe and will not cause any issues.", ), + "django-model-without-dunder-str": DocMetadata( + importance="Low", + guidance_explained="This codemod is a great starting point for models with few fields. We encourage you to write custom `__str__` methods that best suit your Django application.", + ), } METADATA = CORE_METADATA | { diff --git a/src/core_codemods/__init__.py b/src/core_codemods/__init__.py index 0129aecf..86712f5f 100644 --- a/src/core_codemods/__init__.py +++ b/src/core_codemods/__init__.py @@ -62,6 +62,7 @@ from .lazy_logging import LazyLogging from .str_concat_in_seq_literal import StrConcatInSeqLiteral from .fix_async_task_instantiation import FixAsyncTaskInstantiation +from .django_model_without_dunder_str import DjangoModelWithoutDunderStr registry = CodemodCollection( origin="pixee", @@ -118,6 +119,7 @@ LazyLogging, StrConcatInSeqLiteral, FixAsyncTaskInstantiation, + DjangoModelWithoutDunderStr, ], ) diff --git a/src/core_codemods/django_model_without_dunder_str.py b/src/core_codemods/django_model_without_dunder_str.py new file mode 100644 index 00000000..be1da23f --- /dev/null +++ b/src/core_codemods/django_model_without_dunder_str.py @@ -0,0 +1,89 @@ +from typing import Union +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, + Reference, + ReviewGuidance, +) +from core_codemods.api.core_codemod import CoreCodemod + + +class DjangoModelWithoutDunderStrTransformer( + LibcstResultTransformer, NameResolutionMixin +): + change_description = "Add `__str__` definition to `django` Model class." + + def leave_ClassDef( + self, original_node: cst.ClassDef, updated_node: cst.ClassDef + ) -> 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. + if not any( + self.find_base_name(base.value) == "django.db.models.Model" + for base in original_node.bases + ): + return updated_node + + if self.implements_dunder_str(original_node): + return updated_node + + self.report_change(original_node) + + new_body = updated_node.body.with_changes( + body=[*updated_node.body.body, dunder_str_method()] + ) + return updated_node.with_changes(body=new_body) + + def implements_dunder_str(self, original_node: cst.ClassDef) -> bool: + """Check if a ClassDef or its bases implement `__str__`""" + if self.class_has_method(original_node, "__str__"): + return True + + for base in original_node.bases: + if maybe_assignment := self.find_single_assignment(base.value): + classdef = maybe_assignment.node + if self.class_has_method(classdef, "__str__"): + return True + return False + + +def dunder_str_method() -> cst.FunctionDef: + self_body = cst.IndentedBlock( + body=[ + cst.parse_statement("model_name = self.__class__.__name__"), + cst.parse_statement( + 'fields_str = ", ".join((f"{field.name}={getattr(self, field.name)}" for field in self._meta.fields))' + ), + cst.parse_statement('return f"{model_name}({fields_str})"'), + ] + ) + return cst.FunctionDef( + leading_lines=[cst.EmptyLine(indent=False)], + name=cst.Name("__str__"), + params=cst.Parameters(params=[cst.Param(name=cst.Name("self"))]), + body=self_body, + ) + + +DjangoModelWithoutDunderStr = CoreCodemod( + metadata=Metadata( + name="django-model-without-dunder-str", + summary="Ensure Django Model Classes Implement a `__str__` Method", + review_guidance=ReviewGuidance.MERGE_AFTER_REVIEW, + references=[ + Reference( + url="https://docs.djangoproject.com/en/5.0/ref/models/instances/#django.db.models.Model.__str__" + ), + ], + ), + transformer=LibcstTransformerPipeline(DjangoModelWithoutDunderStrTransformer), + detector=None, +) diff --git a/src/core_codemods/docs/pixee_python_django-debug-flag-on.md b/src/core_codemods/docs/pixee_python_django-debug-flag-on.md index 2e00be75..80dbfb6c 100644 --- a/src/core_codemods/docs/pixee_python_django-debug-flag-on.md +++ b/src/core_codemods/docs/pixee_python_django-debug-flag-on.md @@ -1,4 +1,4 @@ -This codemod will flip django's `DEBUG` flag to `False` if it's `True` on the `settings.py` file within django's default directory structure. +This codemod will flip Django's `DEBUG` flag to `False` if it's `True` on the `settings.py` file within Django's default directory structure. Having the debug flag on may result in sensitive information exposure. When an exception occurs while the `DEBUG` flag in on, it will dump metadata of your environment, including the settings module. The attacker can purposefully request a non-existing url to trigger an exception and gather information about your system. diff --git a/src/core_codemods/docs/pixee_python_django-model-without-dunder-str.md b/src/core_codemods/docs/pixee_python_django-model-without-dunder-str.md new file mode 100644 index 00000000..9d3ba768 --- /dev/null +++ b/src/core_codemods/docs/pixee_python_django-model-without-dunder-str.md @@ -0,0 +1,30 @@ +If you've ever actively developed or debugged a Django application, you may have noticed that the string representations of Django models and their instances can sometimes be hard to read or to distinguish from one another. Loading models in the interactive Django console or viewing them in the admin interface can be puzzling. This is because the default string representation of Django models is fairly generic. + +This codemod is intended to make the string representation of your model objects more human-readable. It will automatically detect all of your model's fields and display them as a descriptive string. + +For example, the default string representation of the `Question` model from Django's popular Poll App tutorial looks like this: +```diff +from django.db import models + +class Question(models.Model): + question_text = models.CharField(max_length=200) + pub_date = models.DateTimeField("date published") ++ ++ def __str__(self): ++ model_name = self.__class__.__name__ ++ fields_str = ", ".join((f"{field.name}={getattr(self, field.name)}" for field in self._meta.fields)) ++ return f"{model_name}({fields_str})" +``` + +Without this change, the string representation of `Question` objects look like this in the interactive Django shell: +``` +>>> Question.objects.all() +]> +``` +With this codemod's addition of `__str__`, it now looks like: +``` +>>> Question.objects.all() +]> +``` + +You'll notice this change works great for models with only a handful of fields. We encourage you to use this codemod's change as a starting point for further customization. diff --git a/src/core_codemods/docs/pixee_python_django-session-cookie-secure-off.md b/src/core_codemods/docs/pixee_python_django-session-cookie-secure-off.md index 970876c3..594de2e5 100644 --- a/src/core_codemods/docs/pixee_python_django-session-cookie-secure-off.md +++ b/src/core_codemods/docs/pixee_python_django-session-cookie-secure-off.md @@ -1,4 +1,4 @@ -This codemod will set django's `SESSION_COOKIE_SECURE` flag to `True` if it's `False` or missing on the `settings.py` file within django's default directory structure. +This codemod will set Django's `SESSION_COOKIE_SECURE` flag to `True` if it's `False` or missing on the `settings.py` file within Django's default directory structure. ```diff + SESSION_COOKIE_SECURE = True diff --git a/tests/codemods/test_django_model_without_dunder_str.py b/tests/codemods/test_django_model_without_dunder_str.py new file mode 100644 index 00000000..f230e81c --- /dev/null +++ b/tests/codemods/test_django_model_without_dunder_str.py @@ -0,0 +1,75 @@ +from core_codemods.django_model_without_dunder_str import DjangoModelWithoutDunderStr +from codemodder.codemods.test import BaseCodemodTest + + +class TestDjangoModelWithoutDunderStr(BaseCodemodTest): + codemod = DjangoModelWithoutDunderStr + + def test_name(self): + assert self.codemod.name == "django-model-without-dunder-str" + + def test_no_change(self, tmpdir): + input_code = """ + from django.db import models + + class User(models.Model): + name = models.CharField(max_length=100) + phone = models.IntegerField(blank=True) + + def __str__(self): + return "doesntmatter" + """ + self.run_and_assert(tmpdir, input_code, input_code) + + def test_no_dunder_str(self, tmpdir): + input_code = """ + from django.db import models + + class User(models.Model): + name = models.CharField(max_length=100) + phone = models.IntegerField(blank=True) + + @property + def decorated_name(self): + return f"***{self.name}***" + + def something(): + pass + """ + expected = """ + from django.db import models + + class User(models.Model): + name = models.CharField(max_length=100) + phone = models.IntegerField(blank=True) + + @property + def decorated_name(self): + return f"***{self.name}***" + + def __str__(self): + model_name = self.__class__.__name__ + fields_str = ", ".join((f"{field.name}={getattr(self, field.name)}" for field in self._meta.fields)) + return f"{model_name}({fields_str})" + + def something(): + pass + """ + self.run_and_assert(tmpdir, input_code, expected) + + def test_model_inherits_dunder_str(self, tmpdir): + input_code = """ + from django.db import models + + class Custom: + def __str__(self): + pass + + class User(Custom, models.Model): + name = models.CharField(max_length=100) + phone = models.IntegerField(blank=True) + + def something(): + pass + """ + self.run_and_assert(tmpdir, input_code, input_code) diff --git a/tests/samples/django-project/mysite/mysite/models.py b/tests/samples/django-project/mysite/mysite/models.py new file mode 100644 index 00000000..2086168c --- /dev/null +++ b/tests/samples/django-project/mysite/mysite/models.py @@ -0,0 +1,13 @@ +import django +from django.conf import settings +from django.db import models +# required to run this module standalone for testing +settings.configure() +django.setup() + + +class Message(models.Model): + author = models.CharField(max_length=100) + content = models.CharField(max_length=200) + class Meta: + app_label = 'myapp'