Skip to content

Commit 649e3a4

Browse files
authored
fix: show clear error when mrt fix cannot auto-fix Django operations (#51)
Previously, mrt fix silently returned 'No fix needed' for Django migrations containing AddField NOT NULL, AlterField, RenameField, or RenameModel — even though mrt check would flag those as warnings. This was misleading. Now mrt fix detects these unsupported operations and exits 1 with a clear message explaining which operations were found and that they require manual intervention. Null-safe AddField and operations with defaults are correctly still treated as 'no fix needed'. Fixes #37
1 parent e45b496 commit 649e3a4

3 files changed

Lines changed: 80 additions & 0 deletions

File tree

pytest_mrt/adapters/django_fixer.py

Lines changed: 48 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -158,6 +158,9 @@ class DjangoFixSuggestion:
158158
patched_source: str
159159
confidence: str
160160
warning: str | None = None
161+
# Operations detected by mrt check but not auto-fixable (e.g. AddField NOT NULL,
162+
# AlterField). Non-empty means the migration has issues mrt fix cannot resolve.
163+
unsupported_ops: list[str] | None = None
161164

162165
@property
163166
def issue(self) -> str:
@@ -197,6 +200,25 @@ def _has_kwarg(op: ast.Call, name: str) -> bool:
197200
return any(kw.arg == name for kw in op.keywords)
198201

199202

203+
def _add_field_is_problematic(op: ast.Call) -> bool:
204+
"""Return True if AddField is NOT NULL with no default (MRT411 condition)."""
205+
for kw in op.keywords:
206+
if kw.arg == "field" and isinstance(kw.value, ast.Call):
207+
field_call = kw.value
208+
# null=True means it's safe
209+
for fkw in field_call.keywords:
210+
if fkw.arg == "null":
211+
if isinstance(fkw.value, ast.Constant) and fkw.value.value is True:
212+
return False
213+
# has default= or server_default= -> safe
214+
field_kwarg_names = {fkw.arg for fkw in field_call.keywords}
215+
if "default" in field_kwarg_names or "server_default" in field_kwarg_names:
216+
return False
217+
# No null=True and no default -> problematic
218+
return True
219+
return False
220+
221+
200222
def _has_positional_arg(op: ast.Call, index: int) -> bool:
201223
return len(op.args) > index
202224

@@ -536,7 +558,15 @@ def generate_django_fix(migration_path: str) -> DjangoFixSuggestion | None:
536558
migration_name = f"{app_label}.{path.stem}"
537559
migration_stem = path.stem
538560

561+
# Operations that mrt check may flag but mrt fix cannot auto-resolve.
562+
# Only truly irreversible variants are listed — e.g. AddField(null=True) is
563+
# fine, but AddField NOT NULL without a default is flagged by mrt check and
564+
# cannot be auto-fixed. We detect these by name only (the detector handles
565+
# the nuance); here we just surface them to the user.
566+
_UNSUPPORTED_OPS = {"AlterField", "RenameField", "RenameModel"}
567+
539568
patches: list[DjangoOpPatch] = []
569+
unsupported: list[str] = []
540570

541571
for op in ops_list:
542572
if not isinstance(op, ast.Call):
@@ -552,11 +582,29 @@ def generate_django_fix(migration_path: str) -> DjangoFixSuggestion | None:
552582
patch = _patch_remove_field(source, op, app_label, migration_stem)
553583
elif name == "DeleteModel":
554584
patch = _patch_delete_model(source, op, app_label, migration_stem)
585+
elif name == "AddField":
586+
# Only flag AddField that is NOT NULL with no default — same condition
587+
# that mrt check (MRT411) flags. null=True or a default= means it's safe.
588+
if _add_field_is_problematic(op):
589+
unsupported.append("AddField NOT NULL without default")
590+
elif name in _UNSUPPORTED_OPS:
591+
unsupported.append(name)
555592

556593
if patch is not None:
557594
patches.append(patch)
558595

559596
if not patches:
597+
# Return a suggestion with no patches but unsupported ops listed,
598+
# so the caller can show a meaningful message instead of "no fix needed".
599+
if unsupported:
600+
return DjangoFixSuggestion(
601+
file=path.name,
602+
migration_name=migration_name,
603+
patches=[],
604+
patched_source=source,
605+
confidence="none",
606+
unsupported_ops=unsupported,
607+
)
560608
return None
561609

562610
patched_source = _apply_patches_to_source(source, patches)

pytest_mrt/commands/fix.py

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -99,6 +99,18 @@ def _fix_django(migration_file: str, apply: bool) -> None:
9999
)
100100
raise typer.Exit(0)
101101

102+
if fix_suggestion.unsupported_ops and not fix_suggestion.patches:
103+
ops = ", ".join(sorted(set(fix_suggestion.unsupported_ops)))
104+
console.print(
105+
f"[yellow]mrt fix cannot auto-fix this migration.[/yellow]\n"
106+
f"Operation(s) found: [bold]{ops}[/bold]\n\n"
107+
"These operations must be fixed manually:\n"
108+
" - AddField / AlterField: add server_default or handle nullable migration\n"
109+
" - RenameField / RenameModel: verify old_name is correct and reversible\n\n"
110+
"Run [bold]mrt check[/bold] to see the specific warnings."
111+
)
112+
raise typer.Exit(1)
113+
102114
console.print()
103115
console.print(
104116
f"[bold]{fix_suggestion.file}[/bold] [dim]{fix_suggestion.migration_name}[/dim]"

tests/test_django_fixer.py

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -400,6 +400,7 @@ def test_issue_multiple(tmp_path):
400400

401401

402402
def test_fix_command_routes_to_django(tmp_path):
403+
"""mrt fix on a Django migration with AddField NOT NULL shows a clear error."""
403404
from typer.testing import CliRunner
404405

405406
from pytest_mrt.cli import app
@@ -412,6 +413,25 @@ def test_fix_command_routes_to_django(tmp_path):
412413
)
413414
runner = CliRunner()
414415
result = runner.invoke(app, ["fix", p])
416+
assert result.exit_code == 1
417+
assert "cannot auto-fix" in result.output
418+
assert "AddField" in result.output
419+
420+
421+
def test_fix_command_routes_to_django_nullable(tmp_path):
422+
"""mrt fix on AddField(null=True) reports no fix needed — it is reversible."""
423+
from typer.testing import CliRunner
424+
425+
from pytest_mrt.cli import app
426+
427+
p = _write(
428+
tmp_path,
429+
HEADER
430+
+ " migrations.AddField(model_name='u', name='x', field=models.TextField(null=True)),\n"
431+
+ FOOTER,
432+
)
433+
runner = CliRunner()
434+
result = runner.invoke(app, ["fix", p])
415435
assert result.exit_code == 0
416436
assert "No fix needed" in result.output
417437

0 commit comments

Comments
 (0)