Django models __str__ codemod (#302)

* initial django dunder str codmeod

* attempt to add leading line

* make dedent=False

* test django dunder str codemod correctly formats model

* django dunder str codemod can detect if parent class has a dunder str

* document django str dunder codemod

* Apply suggestions from code review

Co-authored-by: Dan D'Avella <[email protected]>

* change from list to gen

---------

Co-authored-by: Dan D'Avella <[email protected]>

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)"

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):
         """

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

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,)

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 | {

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,
     ],
 )
 

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,
+)

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.
 

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()
+<QuerySet [<Question: Question object (1)>]>
+```
+With this codemod's addition of `__str__`, it now looks like:
+```
+>>> Question.objects.all()
+<QuerySet [<Question: Question(id=1, question_text=What's new?, pub_date=2024-02-21 14:28:45.631782+00:00)>]>
+```
+
+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.

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

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)

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'