Merge pull request #693 from bckohan/field-error

move model definition errors to system checks

Author: bckohan

docs/changelog.rst

@@ -1,8 +1,10 @@
 Changelog
 =========
 
-.. v4.4.0 (202X-XX-XX)
-.. -------------------
+v4.4.0 (202X-XX-XX)
+-------------------
+
+* Fixed `Change model definition errors to be system checks <https://github.com/jazzband/django-polymorphic/pull/693>`_
 
 v4.3.0 (2025-12-09)
 -------------------

src/polymorphic/apps.py

@@ -0,0 +1,43 @@
+from django.apps import AppConfig, apps
+from django.core.checks import Error, Tags, register
+
+
+@register(Tags.models)
+def check_reserved_field_names(app_configs, **kwargs):
+    """
+    System check that ensures models don't use reserved field names.
+    """
+    errors = []
+
+    # If app_configs is None, check all installed apps
+    if app_configs is None:
+        app_configs = apps.get_app_configs()
+
+    for app_config in app_configs:
+        for model in app_config.get_models():
+            errors.extend(_check_model_reserved_field_names(model))
+
+    return errors
+
+
+def _check_model_reserved_field_names(model):
+    from polymorphic.base import POLYMORPHIC_SPECIAL_Q_KWORDS
+
+    errors = []
+
+    for field in model._meta.get_fields():
+        if field.name in POLYMORPHIC_SPECIAL_Q_KWORDS:
+            errors.append(
+                Error(
+                    f"Field '{field.name}' on model '{model.__name__}' is a reserved name.",
+                    obj=field,
+                    id="polymorphic.E001",
+                )
+            )
+
+    return errors
+
+
+class PolymorphicConfig(AppConfig):
+    name = "polymorphic"
+    verbose_name = "Django Polymorphic"

src/polymorphic/base.py

@@ -15,7 +15,7 @@
 
 # PolymorphicQuerySet Q objects (and filter()) support these additional key words.
 # These are forbidden as field names (a descriptive exception is raised)
-POLYMORPHIC_SPECIAL_Q_KWORDS = ["instance_of", "not_instance_of"]
+POLYMORPHIC_SPECIAL_Q_KWORDS = {"instance_of", "not_instance_of"}
 
 DUMPDATA_COMMAND = os.path.join("django", "core", "management", "commands", "dumpdata.py")
 
@@ -63,9 +63,6 @@ def __new__(cls, model_name, bases, attrs, **kwargs):
             # is higher in the MRO
             new_class._meta.base_manager_name = "objects"
 
-        # check if the model fields are all allowed
-        cls.validate_model_fields(new_class)
-
         # validate resulting default manager
         if not new_class._meta.abstract and not new_class._meta.swapped:
             cls.validate_model_manager(new_class.objects, model_name, "objects")
@@ -80,27 +77,6 @@ def __new__(cls, model_name, bases, attrs, **kwargs):
 
         return new_class
 
-    @classmethod
-    def call_superclass_new_method(self, model_name, bases, attrs, **kwargs):
-        """call __new__ method of super class and return the newly created class.
-        Also work around a limitation in Django's ModelBase."""
-        # There seems to be a general limitation in Django's app_label handling
-        # regarding abstract models (in ModelBase). See issue 1 on github - TODO: propose patch for Django
-        # We run into this problem if polymorphic.py is located in a top-level directory
-        # which is directly in the python path. To work around this we temporarily set
-        # app_label here for PolymorphicModel.
-        return super().__new__(self, model_name, bases, attrs, **kwargs)
-
-    @classmethod
-    def validate_model_fields(self, new_class):
-        "check if all fields names are allowed (i.e. not in POLYMORPHIC_SPECIAL_Q_KWORDS)"
-        for f in new_class._meta.fields:
-            if f.name in POLYMORPHIC_SPECIAL_Q_KWORDS:
-                raise AssertionError(
-                    f'PolymorphicModel: "{new_class.__name__}" - '
-                    f'field name "{f.name}" is not allowed in polymorphic models'
-                )
-
     @classmethod
     def validate_model_manager(cls, manager, model_name, manager_name):
         """check if the manager is derived from PolymorphicManager

src/polymorphic/tests/errata/__init__.py

@@ -0,0 +1,6 @@
+from django.db import models
+
+
+class BadModel(models.Model):
+    instance_of = models.CharField(max_length=100)
+    not_instance_of = models.IntegerField()

src/polymorphic/tests/errata/migrations/__init__.py

@@ -0,0 +1,34 @@
+from django.core.checks import Error, run_checks
+from django.test.utils import override_settings
+from django.test import SimpleTestCase
+
+
+@override_settings(
+    INSTALLED_APPS=[
+        "polymorphic.tests.errata",
+        "django.contrib.contenttypes",
+        "django.contrib.auth",
+    ]
+)
+class TestErrata(SimpleTestCase):
+    def test_reserved_field_name_triggers_system_check(self):
+        """Test that using reserved field names triggers polymorphic.E001 system check."""
+
+        # Run the check function directly on the model
+        errors = run_checks()
+
+        assert len(errors) == 2, f"Expected 2 system check errors but got {len(errors)}: {errors}"
+
+        # Verify all errors are the correct type
+        assert all(isinstance(err, Error) and err.id == "polymorphic.E001" for err in errors), (
+            f"Expected all errors to have ID 'polymorphic.E001' but got: {errors}"
+        )
+
+        # Verify the error messages mention the correct field names
+        error_messages = [err.msg for err in errors]
+        assert any("instance_of" in msg for msg in error_messages), (
+            f"Expected error for 'instance_of' field but got: {error_messages}"
+        )
+        assert any("not_instance_of" in msg for msg in error_messages), (
+            f"Expected error for 'not_instance_of' field but got: {error_messages}"
+        )