More checks for indexes

Author: kalekseev

CHANGELOG.md

@@ -2,28 +2,35 @@
 
 ### Unreleased
 
+### 0.7.0
+
+- Check `field-foreign-key-index` now accepts `when: indexes` instead of `when: unique_toegether` because now it search for duplicate indexes in `Meta.indexes`, `Meta.unique_toegether` and `Meta.constraints`
+- Add checks:
+    - `no-unique-together`
+    - `no-index-together`
+
 ### 0.6.0
 
 - Add checks:
-    - field-default-null
+    - `field-default-null`
 
 ### 0.5.0
 
-- Fix ignore_checks
-- Skip models fields not inherited from fields.Field
-- Add ignore_types option
+- Fix `ignore_checks`
+- Skip models fields not inherited from `fields.Field`
+- Add `ignore_types` option
 
 ### 0.4.1
 
-- Fix message for *field-verbose-name-gettext-case*
+- Fix message for `field-verbose-name-gettext-case`
 
 ### 0.4.0
 
 - Add infra for rest framework serializers checks
 - Add checks:
-    - drf-model-serializer-extra-kwargs
-    - drf-model-serializer-meta-attribute
-    - model-admin
+    - `drf-model-serializer-extra-kwargs`
+    - `drf-model-serializer-meta-attribute`
+    - `model-admin`
 
 ### 0.3.0
 

README.md

@@ -64,6 +64,8 @@ EXTRA_CHECKS = {
 - **extra-checks-config** - settings.EXTRA_CHECKS is valid config for django-extra-checks (always enabled).
 - **model-attribute** - Each Model in the project must have all attributes from `attrs` setting specified.
 - **model-meta-attribute** - Each Model.Meta in the project must have all attributes from `attrs` setting specified.
+- **no-unique-together** - Use UniqueConstraint with the constraints option instead.
+- **no-index-together** - Use the indexes option instead.
 - **model-admin** - Each model must be registered in admin.
 - **field-file-upload-to** - FileField/ImageField must have non empty `upload_to` argument.
 - **field-verbose-name** - All model's fields must have verbose name.
@@ -73,7 +75,7 @@ EXTRA_CHECKS = {
 - **field-text-null** - text fields shoudn't use `null=True`.
 - **field-boolean-null** - prefer using `BooleanField(null=True)` instead of `NullBooleanField`.
 - **field-null** - don't pass `null=False` to model fields (this is django default).
-- **field-foreign-key-index** - ForeignKey fields must specify `db_index` explicitly (to apply to unique together only: `when: unique_together`).
+- **field-foreign-key-index** - ForeignKey fields must specify `db_index` explicitly (to apply only to fields in indexes: `when: indexes`).
 - **field-default-null** - If field nullable (`null=True`), then
     `default=None` argument is redundant and should be removed.
     **WARNING** Be aware that setting is database dependent,

setup.cfg

@@ -1,6 +1,6 @@
 [metadata]
 name = django-extra-checks
-version = 0.6.0
+version = 0.7.0
 author = Konstantin Alekseev
 author_email = [email protected]
 description = Collection of useful checks for Django Checks Framework

src/extra_checks/check_id.py

@@ -6,6 +6,8 @@ class CheckId(str, enum.Enum):
     X010 = "model-attribute"
     X011 = "model-meta-attribute"
     X012 = "model-admin"
+    X013 = "no-unique-together"
+    X014 = "no-index-together"
     X050 = "field-verbose-name"
     X051 = "field-verbose-name-gettext"
     X052 = "field-verbose-name-gettext-case"

src/extra_checks/checks/model_checks.py

@@ -155,3 +155,29 @@ def apply(
     ) -> Iterator[django.core.checks.CheckMessage]:
         if model not in self.models_with_admin:
             yield self.message("The model is not registered in admin.", obj=model)
+
+
+@registry.register(django.core.checks.Tags.models)
+class CheckNoUniqueTogether(CheckModel):
+    Id = CheckId.X013
+    level = django.core.checks.ERROR
+
+    def apply(
+        self, model: Type[models.Model], model_ast: ModelAST
+    ) -> Iterator[django.core.checks.CheckMessage]:
+        if "unique_together" in model_ast.meta_vars:
+            yield self.message(
+                "Use UniqueConstraint with the constraints option instead.", obj=model,
+            )
+
+
+@registry.register(django.core.checks.Tags.models)
+class CheckNoIndexTogether(CheckModel):
+    Id = CheckId.X014
+    level = django.core.checks.ERROR
+
+    def apply(
+        self, model: Type[models.Model], model_ast: ModelAST
+    ) -> Iterator[django.core.checks.CheckMessage]:
+        if "index_together" in model_ast.meta_vars:
+            yield self.message("Use the indexes option instead", obj=model)

src/extra_checks/checks/model_field_checks.py

@@ -198,16 +198,40 @@ class CheckFieldForeignKeyIndex(CheckModelField):
 
     class CheckFieldForeignKeyIndexForm(BaseCheckForm):
         when = forms.ChoiceField(
-            choices=[("unique_together", "unique_together"), ("always", "always")],
-            required=False,
+            choices=[("indexes", "indexes"), ("always", "always")], required=False,
         )
 
     settings_form_class = CheckFieldForeignKeyIndexForm
 
     def __init__(self, when: str, **kwargs: Any) -> None:
-        self.when = when or "unique_together"
+        self.when = when or "indexes"
         super().__init__(**kwargs)
 
+    @classmethod
+    def get_index_values_in_meta(cls, model: Type[models.Model]) -> Iterator[str]:
+        for entry in model._meta.unique_together:
+            if isinstance(entry, str):
+                yield entry
+            else:
+                yield from entry
+        for entry in model._meta.index_together:
+            if isinstance(entry, str):
+                yield entry
+            else:
+                yield from entry
+        for constraint in model._meta.constraints:
+            if isinstance(constraint, models.UniqueConstraint):
+                yield from constraint.fields
+        for index in model._meta.indexes:
+            yield from index.fields
+
+    @classmethod
+    def get_fields_with_indexes_in_meta(
+        cls, model: Type[models.Model]
+    ) -> Iterator[str]:
+        for entry in cls.get_index_values_in_meta(model):
+            yield entry.lstrip("-")
+
     def apply(
         self,
         field: models.fields.Field,
@@ -216,10 +240,8 @@ def apply(
     ) -> Iterator[django.core.checks.CheckMessage]:
         if isinstance(field, models.fields.related.RelatedField):
             if field.many_to_one and field_ast.kwargs.get("db_index") is None:
-                if self.when == "unique_together":
-                    if any(
-                        field.name in index for index in model._meta.unique_together
-                    ):
+                if self.when == "indexes":
+                    if field.name in self.get_fields_with_indexes_in_meta(model):
                         yield self.message(
                             "ForeignKey must set `db_index` explicitly if it present in unique_together.",
                             hint="Specify `db_index` field argument.",

tests/example/apps.py

@@ -1,5 +1,5 @@
 from django.apps import AppConfig
 
 
-class ProductsConfig(AppConfig):
-    name = "products"
+class ExampleConfig(AppConfig):
+    name = "tests.example"

tests/example/models.py

@@ -98,7 +98,9 @@ class ModelFieldNullDefault(models.Model):
 
 
 class ModelFieldForeignKeyIndex(models.Model):
-    article = models.ForeignKey(Article, on_delete=models.CASCADE, related_name="+")
+    article = models.ForeignKey(
+        Article, on_delete=models.CASCADE, related_name="+", db_index=True
+    )
     author = models.ForeignKey(Author, on_delete=models.CASCADE, related_name="+")
     another_article = models.ForeignKey(
         Article, on_delete=models.CASCADE, related_name="+"
@@ -107,14 +109,33 @@ class ModelFieldForeignKeyIndex(models.Model):
         Article, on_delete=models.CASCADE, related_name="+", db_index=True
     )
     field_one = models.ForeignKey(
-        ModelFieldTextNull, on_delete=models.CASCADE, related_name="+", db_index=False
+        ModelFieldTextNull, on_delete=models.CASCADE, related_name="+"
     )
     field_two = models.ForeignKey(
         ModelFieldNullFalse, on_delete=models.CASCADE, related_name="+", db_index=True
     )
+    field_three = models.ForeignKey(
+        ModelFieldNullFalse, on_delete=models.CASCADE, related_name="+"
+    )
+    field_in_indexes = models.ForeignKey(
+        ModelFieldNullFalse, on_delete=models.CASCADE, related_name="+"
+    )
+    field_index_desc = models.ForeignKey(
+        ModelFieldNullFalse, on_delete=models.CASCADE, related_name="+"
+    )
 
     class Meta:
-        unique_together = (("article", "author"), ("field_one", "field_two"))
+        unique_together = [("author", "article")]
+        index_together = ("field_one", "field_two")
+        constraints = [
+            models.UniqueConstraint(
+                fields=("author", "field_three"), name="fi_author_field_unique"
+            )
+        ]
+        indexes = [
+            models.Index(fields=("field_in_indexes",)),
+            models.Index(fields=("field_in_indexes", "-field_index_desc")),
+        ]
 
 
 class GenericKeyOne(models.Model):

tests/test_model_checks.py

@@ -1,7 +1,12 @@
 import pytest
 
 from extra_checks.checks import model_checks
-from tests.example.models import Article, Author, ModelFieldTextNull
+from tests.example.models import (
+    Article,
+    Author,
+    ModelFieldForeignKeyIndex,
+    ModelFieldTextNull,
+)
 
 
 @pytest.fixture
@@ -80,3 +85,25 @@ def test_admin_models(test_case):
     messages = test_case.models(ModelFieldTextNull).run()
     assert len(messages) == 1
     assert messages[0].id == model_checks.CheckModelAdmin.Id.name
+
+
+def test_no_unique_together(test_case):
+    messages = (
+        test_case.models(ModelFieldForeignKeyIndex)
+        .settings({"checks": [model_checks.CheckNoUniqueTogether.Id.value]})
+        .check(model_checks.CheckNoUniqueTogether)
+        .run()
+    )
+    assert len(messages) == 1
+    assert messages[0].id == model_checks.CheckNoUniqueTogether.Id.name
+
+
+def test_no_index_together(test_case):
+    messages = (
+        test_case.models(ModelFieldForeignKeyIndex)
+        .settings({"checks": [model_checks.CheckNoIndexTogether.Id.value]})
+        .check(model_checks.CheckNoIndexTogether)
+        .run()
+    )
+    assert len(messages) == 1
+    assert messages[0].id == model_checks.CheckNoIndexTogether.Id.name

tests/test_model_field_checks.py

@@ -140,8 +140,11 @@ def test_check_field_foreign_key_index(test_case):
         .run()
     )
     assert {m.obj.name for m in messages} == {
-        "article",
+        "field_one",
         "author",
+        "field_three",
+        "field_in_indexes",
+        "field_index_desc",
     }
 
 
@@ -161,9 +164,12 @@ def test_check_field_foreign_key_index_always(test_case):
         .run()
     )
     assert {m.obj.name for m in messages} == {
-        "article",
+        "field_one",
         "author",
         "another_article",
+        "field_three",
+        "field_in_indexes",
+        "field_index_desc",
     }