diff --git a/CHANGELOG.md b/CHANGELOG.md index ebf18cc..a164d35 100644 --- a/CHANGELOG.md +++ b/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 diff --git a/django_migration_linter/migration_linter.py b/django_migration_linter/migration_linter.py index 4bfe6ff..611c3ca 100644 --- a/django_migration_linter/migration_linter.py +++ b/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 diff --git a/docs/incompatibilities.md b/docs/incompatibilities.md index 0dd0ad1..721378b 100644 --- a/docs/incompatibilities.md +++ b/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 diff --git a/tests/functional/test_data_migrations.py b/tests/functional/test_data_migrations.py index 20ea999..a5b7989 100644 --- a/tests/functional/test_data_migrations.py +++ b/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))