Refs #60 -- Adjust pre-RemoveField(db_default) for ForeignKey with callable defaults.

Just like pre-AddField the get_default() call have to be adjusted to account
for unbounded fields.

Thanks to @alitycast for the report.

Author: charettes

CHANGELOG.rst

@@ -5,6 +5,8 @@
 
 - Address crash of system checks when an installed app has an empty
   `migrations` directory. (#62)
+- Address a ``makemigration`` crash when removing a ``ForeignKey`` with a
+  ``default`` on Django 5.2+. (#60)
 
 1.2.1
 =====

syzygy/operations.py

@@ -122,6 +122,25 @@ def describe(self):
         return "Set field %s of %s NULLable" % (self.name, self.model_name)
 
 
+def _get_field_default(field):
+    if isinstance(field, models.ForeignKey):
+        # XXX: Replicate ForeignKey.get_default() logic in a way that
+        # doesn't require the field references to be pre-emptively
+        # resolved. This will need to be fixed upstream if we ever
+        # implement model state schema alterations.
+        # See  https://code.djangoproject.com/ticket/29898
+        field_default = super(models.ForeignKey, field).get_default()
+        if (
+            isinstance(field_default, models.Model)
+            and field_default._meta.label_lower == field.related_model.lower()
+        ):
+            target_field = field.to_fields[0] or "pk"
+            field_default = getattr(field_default, target_field)
+    else:
+        field_default = field.get_default()
+    return field_default
+
+
 if field_db_default_supported:
 
     def get_pre_remove_field_operation(model_name, name, field):
@@ -131,7 +150,7 @@ def get_pre_remove_field_operation(model_name, name, field):
             )
         field = field.clone()
         if field.has_default():
-            field.db_default = field.get_default()
+            field.db_default = _get_field_default(field)
             fragment = f"set_db_default_{model_name.lower()}_{name}"
             description = f"Set database DEFAULT of field {name} on {model_name}"
         else:
@@ -202,22 +221,7 @@ def get_pre_add_field_operation(model_name, name, field, preserve_default=True):
                 "Fields with a db_default don't require a pre-deployment operation."
             )
         field = field.clone()
-        if isinstance(field, models.ForeignKey):
-            # XXX: Replicate ForeignKey.get_default() logic in a way that
-            # doesn't require the field references to be pre-emptively
-            # resolved. This will need to be fixed upstream if we ever
-            # implement model state schema alterations.
-            # See  https://code.djangoproject.com/ticket/29898
-            field_default = super(models.ForeignKey, field).get_default()
-            if (
-                isinstance(field_default, models.Model)
-                and field_default._meta.label_lower == field.related_model.lower()
-            ):
-                target_field = field.to_fields[0] or "pk"
-                field_default = getattr(field_default, target_field)
-        else:
-            field_default = field.get_default()
-        field.db_default = field_default
+        field.db_default = _get_field_default(field)
         operation = operations.AddField(model_name, name, field, preserve_default)
         return operation
 

tests/test_autodetector.py

@@ -153,7 +153,7 @@ def test_db_default_field_addition(self):
         self.assertEqual(len(changes), 1)
         self.assertEqual(get_migration_stage(changes[0]), Stage.PRE_DEPLOY)
 
-    def _test_field_removal(self, field):
+    def _test_field_removal(self, field, expected_db_default=None):
         from_model = ModelState("tests", "Model", [("field", field)])
         to_model = ModelState("tests", "Model", [])
         changes = self.get_changes([from_model], [to_model])["tests"]
@@ -165,7 +165,9 @@ def _test_field_removal(self, field):
         if field_db_default_supported:
             self.assertIsInstance(pre_operation, migrations.AlterField)
             if field.has_default():
-                self.assertEqual(pre_operation.field.db_default, 42)
+                self.assertEqual(
+                    pre_operation.field.db_default, expected_db_default or 42
+                )
             else:
                 self.assertIs(pre_operation.field.null, True)
         else:
@@ -182,10 +184,33 @@ def test_field_removal(self):
             models.IntegerField(),
             models.IntegerField(default=42),
             models.IntegerField(null=True, default=42),
+            models.IntegerField(null=True, default=lambda: 42),
+            # Foreign keys with callable defaults should have their associated
+            # db_default generated with care.
+            (models.ForeignKey("tests.Model", models.CASCADE, default=42), 42),
+            (
+                models.ForeignKey(
+                    "tests.Bar", models.CASCADE, default=lambda: Bar(id=42)
+                ),
+                42,
+            ),
+            (
+                models.ForeignKey(
+                    "tests.bar",
+                    models.CASCADE,
+                    to_field="name",
+                    default=lambda: Bar(id=123, name="bar"),
+                ),
+                "bar",
+            ),
         ]
         for field in fields:
+            if isinstance(field, tuple):
+                field, expected_db_default = field
+            else:
+                expected_db_default = None
             with self.subTest(field=field):
-                self._test_field_removal(field)
+                self._test_field_removal(field, expected_db_default)
 
     def test_many_to_many_removal(self):
         from_model = ModelState(