adamchainz / django-upgrade

Automatically upgrade your Django projects.
MIT License
1.01k stars 64 forks source link

Extensions to `django-upgrade` #492

Open UnknownPlatypus opened 1 month ago

UnknownPlatypus commented 1 month ago

Description

Hey Adam,

For the past year, I've been using django-upgrade foundations a lot to add some more autofixes rules at work and on some personal projects. They might not all be exactly in scope for django-upgrade (some of them are more like linting rules with autofixes) but I believe they could benefit the Django community. I've been using them in production for almost 6 months now without any issues so far :crossed_fingers:

The goal of this issue would be to know if you want to me to add some of the fixers to django-upgrade, maybe behind a flag or opt-in using the new rule selection capability. Otherwise, I'm seeking some advice on how to keep my fork up-to-date not to painfully (maybe it should be a package with a dependency on yours?) and package it properly so that some people can use the added fixers -- currently I'm not sure how to do that properly.

Anyway, here is an overview of the fixers I added, if you believe some would be good additions to django-upgrade let me know, I'll be happy to submit PRs (as always). I linked existing PR's in my fork for more details.

1. Always use lazy relationships when defining ForeignKey/ManyToMany fields

A common issue in Django is circular imports when defining foreign keys with direct imports. One workaround Django provides is using a lazy relationship, which this fixer enforces.

from myapp.models import House

class MyModel(models.Model):
-    house = models.ForeignKey(House, null=False, on_delete=models.CASCADE)
+    house = models.ForeignKey("myapp.House", null=False, on_delete=models.CASCADE)

See PR

2. Remove redundant select_related / prefetch_related arguments

In Django, select_related("author__hometown") already includes the author relationship, making select_related("author") redundant. This change removes the duplication and simplifies the query, making the code more concise (Django does the same deduplication work internally at runtime, because it could lead to duplicated SQL queries in some cases).

-Book.objects.select_related("author", "author__hometown")
+Book.objects.select_related("author__hometown")

See PR

3. Reorder Model's inner classes, methods, and fields to follow the Django Style Guide

In large codebase, some Django models tend to become very big. Discovering the Meta attribute or finding some django specific methods (save, get_absolute_url, ...) can become time consuming when there is no convention enforced. Some method also usually work together (clean & save for ex) and it make more sense to keep them close. Manager order also matters and having them all around the place can lead to weird bugs when they are moved around

This mostly implement autofix capability for the existing flake8 linting rule (which is impossible to add to an existing projet if you have to manually fix everything). Careful attention has been given to preserving comments and maintaining the existing style.

The current implementation is not really configurable but uses sane defaults I believe. I've been using it in a team of 10-15 devs for 6 months without any churn yet. It would be really easy to add configurations options though if needed.

See PR

4. Reorder model field kwargs

This fixer standardizes the order of keyword arguments in Django model fields to improve readability and consistency. By arranging arguments in a consistent order, the code becomes more predictable and making it simpler for developers to quickly identify key attributes in model definitions.

Careful attention has been given to preserving comments and maintaining the existing style. For now the fixer only format simple django fields. Support for ForeignKey/ManyToMany fiels is not available yet

-data_1 = models.CharField(default="", verbose_name="Donnée 1", blank=True, max_length=191)
-data_2 = models.CharField(blank=True, max_length=191, verbose_name="Donnée 2", default="")
-data_3 = models.CharField(max_length=191, verbose_name="Donnée 3", default="", blank=True)
+data_1 = models.CharField(verbose_name="Donnée 1", default="", blank=True, max_length=191)
+data_2 = models.CharField(verbose_name="Donnée 2", default="", blank=True, max_length=191)
+data_3 = models.CharField(verbose_name="Donnée 3", default="", blank=True, max_length=191)

PR

5. Improve consistency of django.utils.timezone usages

I've seen many people quite confused with when to use timezone.localtime / timezone.make_aware and timezone.localdate leading to a lot of weird pattern that could be achieved in more simple ways. This fixer simplify some of these cases.

-timezone.localtime(aware_date_or_datetime).date()
+timezone.localdate(aware_date_or_datetime)

-timezone.make_aware(dt.datetime.now())
+timezone.localtime()

-timezone.localtime(timezone.now())
+timezone.localtime()
-timezone.localdate(timezone.now())
+timezone.localdate()

-timezone.make_aware(dt.datetime(2022, 4, 21), timezone=timezone.get_current_timezone())
+timezone.make_aware(dt.datetime(2022, 4, 21))
-timezone.localtime(dt.datetime(2022, 4, 21), timezone.get_current_timezone())
+timezone.localtime(dt.datetime(2022, 4, 21))

PR

For curious people I've also written a few python general purpose fixer that I plan to port to `pyupgrade`/`ruff` eventually. - Prefer `pytest.params(.., id=...)` over `pytest.mark.parametrize(..., ids=...)` ( [PR](https://github.com/UnknownPlatypus/django-upgrade/pull/3) ) - Use `.fromisoformat` over `.strptime` when applicable (~100x faster) ( [PR](https://github.com/UnknownPlatypus/django-upgrade/pull/9) ) - Convert `strftime` in f-string to use date format specifier ( [PR](https://github.com/UnknownPlatypus/django-upgrade/pull/10) )

Have a nice day !

matthiask commented 1 month ago

For the record, 2-5 sound very useful to me as well!