Fixed #22397 -- Issues removing M2M field with explicit through model

Changed the migration autodetector to remove models last so that FK
and M2M fields will not be left as dangling references. Added a check
in the migration state renderer to error out in the presence of
dangling references instead of leaving them as strings. Fixed a bug
in the sqlite backend to handle the deletion of M2M fields with
"through" models properly (i.e., do nothing successfully).

Thanks to melinath for report, loic for tests and andrewgodwin and
charettes for assistance with architecture.

4 files changed, 29 insertions, 15 deletions

diff --git a/django/db/backends/sqlite3/schema.py b/django/db/backends/sqlite3/schema.py
index 216ff0958a..a8417d4f66 100644
--- a/django/db/backends/sqlite3/schema.py
+++ b/django/db/backends/sqlite3/schema.py
@@ -121,11 +121,15 @@ class DatabaseSchemaEditor(BaseDatabaseSchemaEditor):
         Removes a field from a model. Usually involves deleting a column,
         but for M2Ms may involve deleting a table.
         """
-        # Special-case implicit M2M tables
-        if isinstance(field, ManyToManyField) and field.rel.through._meta.auto_created:
-            return self.delete_model(field.rel.through)
+        # M2M fields are a special case
+        if isinstance(field, ManyToManyField):
+            # For implicit M2M tables, delete the auto-created table
+            if field.rel.through._meta.auto_created:
+                self.delete_model(field.rel.through)
+            # For explicit "through" M2M fields, do nothing
         # For everything else, remake.
-        self._remake_table(model, delete_fields=[field])
+        else:
+            self._remake_table(model, delete_fields=[field])
 
     def alter_field(self, model, old_field, new_field, strict=False):
         """
diff --git a/django/db/migrations/autodetector.py b/django/db/migrations/autodetector.py
index adf3686d59..134dcfed95 100644
--- a/django/db/migrations/autodetector.py
+++ b/django/db/migrations/autodetector.py
@@ -223,16 +223,6 @@ class MigrationAutodetector(object):
                     unique_together=unique_together
                 )
             )
-        # Removing models
-        removed_models = set(old_model_keys) - set(new_model_keys)
-        for app_label, model_name in removed_models:
-            model_state = self.from_state.models[app_label, model_name]
-            self.add_to_migration(
-                app_label,
-                operations.DeleteModel(
-                    model_state.name,
-                )
-            )
         # Changes within models
         kept_models = set(old_model_keys).intersection(new_model_keys)
         old_fields = set()
@@ -348,6 +338,16 @@ class MigrationAutodetector(object):
                 )
         for app_label, operation in unique_together_operations:
             self.add_to_migration(app_label, operation)
+        # Removing models
+        removed_models = set(old_model_keys) - set(new_model_keys)
+        for app_label, model_name in removed_models:
+            model_state = self.from_state.models[app_label, model_name]
+            self.add_to_migration(
+                app_label,
+                operations.DeleteModel(
+                    model_state.name,
+                )
+            )
         # Alright, now add internal dependencies
         for app_label, migrations in self.migrations.items():
             for m1, m2 in zip(migrations, migrations[1:]):
diff --git a/django/db/migrations/migration.py b/django/db/migrations/migration.py
index 0114dba60c..e9af9cd994 100644
--- a/django/db/migrations/migration.py
+++ b/django/db/migrations/migration.py
@@ -127,7 +127,7 @@ class Migration(object):
         to_run.reverse()
         for operation, to_state, from_state in to_run:
             operation.database_backwards(self.app_label, schema_editor, from_state, to_state)
-
+        return project_state
 
 def swappable_dependency(value):
     """
diff --git a/django/db/migrations/state.py b/django/db/migrations/state.py
index d146bfd8bc..86693f4398 100644
--- a/django/db/migrations/state.py
+++ b/django/db/migrations/state.py
@@ -51,6 +51,16 @@ class ProjectState(object):
                 if len(new_unrendered_models) == len(unrendered_models):
                     raise InvalidBasesError("Cannot resolve bases for %r" % new_unrendered_models)
                 unrendered_models = new_unrendered_models
+            # make sure apps has no dangling references
+            if self.apps._pending_lookups:
+                # Raise an error with a best-effort helpful message
+                # (only for the first issue). Error message should look like:
+                # "ValueError: Lookup failed for model referenced by
+                # field migrations.Book.author: migrations.Author"
+                dangling_lookup = list(self.apps._pending_lookups.items())[0]
+                raise ValueError("Lookup failed for model referenced by field {field}: {model[0]}.{model[1]}".format(
+                    field=dangling_lookup[1][0][1],
+                    model=dangling_lookup[0]))
         return self.apps
 
     @classmethod