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

Django models __str__ codemod #302

Merged
merged 8 commits into from
Feb 28, 2024
Merged
Show file tree
Hide file tree
Changes from all 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
52 changes: 52 additions & 0 deletions integration_tests/test_django_model_without_dunder_str.py
Original file line number Diff line number Diff line change
@@ -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)"
9 changes: 6 additions & 3 deletions src/codemodder/codemods/test/integration_utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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):
"""
Expand Down
21 changes: 16 additions & 5 deletions src/codemodder/codemods/test/validations.py
Original file line number Diff line number Diff line change
@@ -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):
Expand All @@ -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
10 changes: 10 additions & 0 deletions src/codemodder/codemods/utils_mixin.py
Original file line number Diff line number Diff line change
Expand Up @@ -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,)
Expand Down
4 changes: 4 additions & 0 deletions src/codemodder/scripts/generate_docs.py
Original file line number Diff line number Diff line change
Expand Up @@ -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 | {
Expand Down
2 changes: 2 additions & 0 deletions src/core_codemods/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down Expand Up @@ -118,6 +119,7 @@
LazyLogging,
StrConcatInSeqLiteral,
FixAsyncTaskInstantiation,
DjangoModelWithoutDunderStr,
],
)

Expand Down
89 changes: 89 additions & 0 deletions src/core_codemods/django_model_without_dunder_str.py
Original file line number Diff line number Diff line change
@@ -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",
Copy link
Member

Choose a reason for hiding this comment

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

Not a huge deal but I feel like we're sometimes inconsistent as to whether a codemod is phrased in terms of a problem or a fix. I think we should prefer names in terms of fixes add-django-model-dunder-str but maybe we can just take it as a suggestion going forward.

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,
)
Original file line number Diff line number Diff line change
@@ -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.

Expand Down
Original file line number Diff line number Diff line change
@@ -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.
Original file line number Diff line number Diff line change
@@ -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
Expand Down
75 changes: 75 additions & 0 deletions tests/codemods/test_django_model_without_dunder_str.py
Original file line number Diff line number Diff line change
@@ -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)
13 changes: 13 additions & 0 deletions tests/samples/django-project/mysite/mysite/models.py
Original file line number Diff line number Diff line change
@@ -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'
Loading