Add warning if RunPython model variable name not the same as model class

CHANGELOG.md

@@ -2,6 +2,7 @@
 
 * Add Python 3.9 support
 * Make data migration model import error less strict (issue #121)
+* Add warning detection on RunPython call when model variable name is not the same as model class name
 
 Switched from Travis CI to GitHub Actions for tests
 

django_migration_linter/migration_linter.py

@@ -456,6 +456,25 @@ class MigrationLinter(object):
                     ignored.append(issue)
                 else:
                     error.append(issue)
+
+        # Detect warning if model variable name is not the same as model class
+        issues = self.get_runpython_model_variable_naming_issues(runpython.code)
+        for issue in issues:
+            if issue and issue["code"] in self.exclude_migration_tests:
+                ignored.append(issue)
+            else:
+                warning.append(issue)
+
+        if runpython.reversible:
+            issues = self.get_runpython_model_variable_naming_issues(
+                runpython.reverse_code
+            )
+            for issue in issues:
+                if issue and issue["code"] in self.exclude_migration_tests:
+                    ignored.append(issue)
+                else:
+                    warning.append(issue)
+
         return error, ignored, warning
 
     @staticmethod
@@ -487,3 +506,43 @@ class MigrationLinter(object):
                     }
                 )
         return issues
+
+    @staticmethod
+    def get_runpython_model_variable_naming_issues(code):
+        model_object_regex = re.compile(r"[^a-zA-Z]?([a-zA-Z0-9]+?)\.objects")
+
+        function_name = code.__name__
+        source_code = inspect.getsource(code)
+
+        called_models = model_object_regex.findall(source_code)
+        issues = []
+        for model in called_models:
+            has_same_model_name = (
+                re.search(
+                    "{model}.*= +\w+?\.get_model\([^\)]+?\.{model}.*?\)".format(
+                        model=model
+                    ),
+                    source_code,
+                    re.MULTILINE | re.DOTALL,
+                )
+                is not None
+                or re.search(
+                    "{model}.*= +\w+?\.get_model\([^\)]+?,[^\)]*?{model}.*?\)".format(
+                        model=model
+                    ),
+                    source_code,
+                    re.MULTILINE | re.DOTALL,
+                )
+                is not None
+            )
+            if not has_same_model_name:
+                issues.append(
+                    {
+                        "code": "DATA_MIGRATION_MODEL_VARIABLE_NAME",
+                        "msg": (
+                            "'{}': Model variable name {} is different from the "
+                            "model class name that was found in the apps.get_model(...) call."
+                        ).format(function_name, model),
+                    }
+                )
+        return issues

docs/incompatibilities.md

@@ -19,20 +19,22 @@ We are happy to add more if you can specify them to us.
 On data migrations, the linter will show a warning when:
 * you are missing a reverse migration
 * the RunPython arguments do not respect the naming convention `apps, schema_editor`
+* the model variable name is different from the model class name in the `get_model` call
 
 ## Codes
 
 You can ignore check through the `--exclude-migration-test` option and specifying any of the codes:
 
-|               Code                |            Description                                                                          |
-|-----------------------------------|----------------------------------------------------------------------|
-|`NOT_NULL`                         | Not NULL constraint on columns
-|`DROP_COLUMN`                      | Dropping columns
-|`DROP_TABLE`                       | Dropping tables
-|`RENAME_COLUMN`                    | Renaming columns
-|`RENAME_TABLE`                     | Renaming tables
-|`ALTER_COLUMN`                     | Altering columns (could be backward compatible)
-|`ADD_UNIQUE`                       | Add unique constraints
-|`REVERSIBLE_DATA_MIGRATION`        | RunPython data migration is not reversible (missing reverse code)
-|`NAMING_CONVENTION_RUNPYTHON_ARGS` | By convention, RunPython names two arguments: apps, schema_editor
-|`DATA_MIGRATION_MODEL_IMPORT`      | Missing apps.get_model() calls for model
+|               Code                  |            Description                                                                          |
+|-------------------------------------|----------------------------------------------------------------------|
+|`NOT_NULL`                           | Not NULL constraint on columns
+|`DROP_COLUMN`                        | Dropping columns
+|`DROP_TABLE`                         | Dropping tables
+|`RENAME_COLUMN`                      | Renaming columns
+|`RENAME_TABLE`                       | Renaming tables
+|`ALTER_COLUMN`                       | Altering columns (could be backward compatible)
+|`ADD_UNIQUE`                         | Add unique constraints
+|`REVERSIBLE_DATA_MIGRATION`          | RunPython data migration is not reversible (missing reverse code)
+|`NAMING_CONVENTION_RUNPYTHON_ARGS`   | By convention, RunPython names two arguments: apps, schema_editor
+|`DATA_MIGRATION_MODEL_IMPORT`        | Missing apps.get_model() calls for model
+|`DATA_MIGRATION_MODEL_VARIABLE_NAME` | The model variable name is different from the model class itself

tests/functional/test_data_migrations.py

@@ -133,3 +133,109 @@ class DataMigrationModelImportTestCase(unittest.TestCase):
 
         issues = MigrationLinter.get_runpython_model_import_issues(forward_method)
         self.assertEqual(0, len(issues))
+
+
+class DataMigrationModelVariableNamingTestCase(unittest.TestCase):
+    def test_same_variable_name(self):
+        def forward_op(apps, schema_editor):
+            MyModel = apps.get_model("app", "MyModel")
+
+            MyModel.objects.filter(id=1).first()
+
+        issues = MigrationLinter.get_runpython_model_variable_naming_issues(forward_op)
+        self.assertEqual(0, len(issues))
+
+    def test_same_variable_name_multiline(self):
+        def forward_op(apps, schema_editor):
+            MyModelVeryLongLongLongLongLong = apps.get_model(
+                "app", "MyModelVeryLongLongLongLongLong"
+            )
+
+            MyModelVeryLongLongLongLongLong.objects.filter(id=1).first()
+
+        issues = MigrationLinter.get_runpython_model_variable_naming_issues(forward_op)
+        self.assertEqual(0, len(issues))
+
+    def test_same_variable_name_multiline2(self):
+        def forward_op(apps, schema_editor):
+            MyModelVeryLongLongLongLongLong = apps.get_model(
+                "app_name_longlonglonglongapp",
+                "MyModelVeryLongLongLongLongLong",
+            )
+
+            MyModelVeryLongLongLongLongLong.objects.filter(id=1).first()
+
+        issues = MigrationLinter.get_runpython_model_variable_naming_issues(forward_op)
+        self.assertEqual(0, len(issues))
+
+    def test_different_variable_name(self):
+        def forward_op(apps, schema_editor):
+            some_model = apps.get_model("app", "MyModel")
+
+            some_model.objects.filter(id=1).first()
+
+        issues = MigrationLinter.get_runpython_model_variable_naming_issues(forward_op)
+        self.assertEqual(1, len(issues))
+
+    def test_diff_variable_name_multiline(self):
+        def forward_op(apps, schema_editor):
+            MyModelVeryLongLongLongLongLongNot = apps.get_model(
+                "app", "MyModelVeryLongLongLongLongLong"
+            )
+
+            MyModelVeryLongLongLongLongLongNot.objects.filter(id=1).first()
+
+        issues = MigrationLinter.get_runpython_model_variable_naming_issues(forward_op)
+        self.assertEqual(1, len(issues))
+
+    def test_diff_variable_name_multiline2(self):
+        def forward_op(apps, schema_editor):
+            MyModelVeryLongLongLongLongLongNot = apps.get_model(
+                "app_name_longlonglonglongapp",
+                "MyModelVeryLongLongLongLongLong",
+            )
+
+            MyModelVeryLongLongLongLongLongNot.objects.filter(id=1).first()
+
+        issues = MigrationLinter.get_runpython_model_variable_naming_issues(forward_op)
+        self.assertEqual(1, len(issues))
+
+    def test_same_variable_name_one_param(self):
+        def forward_op(apps, schema_editor):
+            MyModel = apps.get_model("app.MyModel")
+
+            MyModel.objects.filter(id=1).first()
+
+        issues = MigrationLinter.get_runpython_model_variable_naming_issues(forward_op)
+        self.assertEqual(0, len(issues))
+
+    def test_different_variable_name_one_param(self):
+        def forward_op(apps, schema_editor):
+            mymodel = apps.get_model("app.MyModel")
+
+            mymodel.objects.filter(id=1).first()
+
+        issues = MigrationLinter.get_runpython_model_variable_naming_issues(forward_op)
+        self.assertEqual(1, len(issues))
+
+    def test_correct_variable_name_one_param_multiline(self):
+        def forward_op(apps, schema_editor):
+            AVeryLongModelName = apps.get_model(
+                "quite_long_app_name.AVeryLongModelName"
+            )
+
+            AVeryLongModelName.objects.filter(id=1).first()
+
+        issues = MigrationLinter.get_runpython_model_variable_naming_issues(forward_op)
+        self.assertEqual(0, len(issues))
+
+    def test_different_variable_name_one_param_multiline(self):
+        def forward_op(apps, schema_editor):
+            m = apps.get_model(
+                "quite_long_app_name_name_name.AVeryLongModelNameNameName"
+            )
+
+            m.objects.filter(id=1).first()
+
+        issues = MigrationLinter.get_runpython_model_variable_naming_issues(forward_op)
+        self.assertEqual(1, len(issues))