Add check for changing field type (#16)

README.md

@@ -153,6 +153,17 @@ class Migration(migrations.Migration):
 ```
 
 
+### Changing field type
+
+Changing the type of a field is generally not safe to do because it causes a
+full table rewrite, during which the table will be fully locked. Additionally
+old code still running after the migration has been applied might write
+unsupported values to the column.
+
+Rather than changing the type of a column you should add a new column and
+manually migrate data from the old column to the new one.
+
+
 ### Adding indexes
 
 Checks if the migration contains an `AddIndex` operation and suggests using

migration_checker/checks.py

@@ -7,6 +7,7 @@
     AddConstraint,
     AddField,
     AddIndex,
+    AlterField,
     Migration,
     RemoveField,
     RenameField,
@@ -16,13 +17,14 @@
 )
 from django.db.migrations.operations.fields import FieldOperation
 from django.db.migrations.operations.models import ModelOperation
-from django.db.migrations.state import ProjectState
+from django.db.migrations.state import ModelState, ProjectState
 
 from .warnings import (
     ADD_INDEX_IN_SEPARATE_MIGRATION,
     ADDING_CONSTRAINT,
     ADDING_FIELD_WITH_CHECK,
     ADDING_NON_NULLABLE_FIELD,
+    ALTER_FIELD,
     ALTERING_MULTIPLE_MODELS,
     ATOMIC_DATA_MIGRATION,
     REMOVING_FIELD,
@@ -166,6 +168,25 @@ def check_validate_constraint(
         yield VALIDATE_CONSTRAINT_SEPARATELY
 
 
+def check_alter_field(
+    *, migration: Migration, state: ProjectState
+) -> Iterable[Warning]:
+    def changes_type(operation: AlterField) -> bool:
+        # Extract the old field
+        model: ModelState = state.models[(migration.app_label, operation.model_name)]
+        field = model.fields[operation.name]
+        # TODO: Also check other things that have changed
+        return type(field) is not type(operation.field)  # noqa: E721
+
+    alter_fields = [
+        operation
+        for operation in migration.operations
+        if isinstance(operation, AlterField)
+    ]
+    if any(changes_type(operation) for operation in alter_fields):
+        yield ALTER_FIELD
+
+
 ALL_CHECKS: list[Check] = [
     check_add_index,
     check_add_non_nullable_field,
@@ -178,6 +199,7 @@ def check_validate_constraint(
     check_field_with_check_constraint,
     check_add_constraint,
     check_validate_constraint,
+    check_alter_field,
 ]
 
 

migration_checker/warnings.py

@@ -162,3 +162,13 @@ def __repr__(self) -> str:
         "constraint validation operations in a separate migration."
     ),
 )
+
+ALTER_FIELD = Warning(
+    level=Level.DANGER,
+    title="Changing column type",
+    description=(
+        "This migration is changing the type of a column, which is generally "
+        "not safe because it fully locks the table and can cause issues with "
+        "writes from old code."
+    ),
+)

tests/test_checks.py

@@ -5,6 +5,7 @@
     AddConstraint,
     AddField,
     AddIndex,
+    AlterField,
     RemoveField,
     RenameField,
     RenameModel,
@@ -14,6 +15,7 @@
 from django.db.migrations.state import ProjectState
 from django.db.models import (
     AutoField,
+    BigAutoField,
     CheckConstraint,
     Index,
     IntegerField,
@@ -27,6 +29,7 @@
     ADDING_CONSTRAINT,
     ADDING_FIELD_WITH_CHECK,
     ADDING_NON_NULLABLE_FIELD,
+    ALTER_FIELD,
     ALTERING_MULTIPLE_MODELS,
     REMOVING_FIELD,
     RENAMING_FIELD,
@@ -131,6 +134,16 @@ def test_add_constraint() -> None:
     assert check_migration(operation) == {ADDING_CONSTRAINT}
 
 
+def test_alter_field() -> None:
+    operation = AlterField(model_name="foo", name="id", field=BigAutoField())
+    assert check_migration(operation) == {ALTER_FIELD}
+
+
+def test_alter_field_to_nullable() -> None:
+    operation = AlterField(model_name="foo", name="id", field=AutoField(null=True))
+    assert check_migration(operation) == set()
+
+
 @pytest.mark.skipif(django.VERSION < (4, 0), reason="Not supported in Django < 4.0")
 def test_add_field_and_validate_constraint() -> None:
     from django.contrib.postgres.operations import ValidateConstraint