Refs kiwitcms/Kiwi#1774 New linter: warn about missing backwards migrations

Author: brymut

CONTRIBUTORS.md

@@ -15,4 +15,5 @@
 * [psrb](https://github.com/psrb)
 * [WayneLambert](https://github.com/WayneLambert)
 * [alejandro-angulo](https://github.com/alejandro-angulo)
+* [brymut](https://github.com/brymut)
 

pylint_django/checkers/db_performance.py

@@ -14,6 +14,7 @@
 from pylint.checkers import utils
 from pylint_django.__pkginfo__ import BASE_ID
 from pylint_django import compat
+from pylint_django.utils import is_migrations_module
 
 
 def _is_addfield_with_default(call):
@@ -37,13 +38,6 @@ def _is_addfield_with_default(call):
     return False
 
 
-def _is_migrations_module(node):
-    if not isinstance(node, astroid.Module):
-        return False
-
-    return 'migrations' in node.path[0] and not node.path[0].endswith('__init__.py')
-
-
 class NewDbFieldWithDefaultChecker(checkers.BaseChecker):
     """
     Looks for migrations which add new model fields and these fields have a
@@ -69,7 +63,7 @@ class NewDbFieldWithDefaultChecker(checkers.BaseChecker):
     _possible_offences = {}
 
     def visit_module(self, node):
-        if _is_migrations_module(node):
+        if is_migrations_module(node):
             self._migration_modules.append(node)
 
     def visit_call(self, node):
@@ -78,7 +72,7 @@ def visit_call(self, node):
         except:  # noqa: E722, pylint: disable=bare-except
             return
 
-        if not _is_migrations_module(module):
+        if not is_migrations_module(module):
             return
 
         if _is_addfield_with_default(node):
@@ -114,6 +108,38 @@ def _path(node):
                     self.add_message('new-db-field-with-default', args=module.name, node=node)
 
 
+class MissingBackwardsMigrationChecker(checkers.BaseChecker):
+    __implements__ = (interfaces.IAstroidChecker,)
+
+    name = 'missing-backwards-migration-callable'
+
+    msgs = {'W%s05' % BASE_ID: ('%s Always include backwards migration callable',
+                                'missing-backwards-migration-callable',
+                                'Always include a backwards/reverse callable counterpart'
+                                ' so that the migration is not irreversable.')}
+
+    @utils.check_messages('missing-backwards-migration-callable')
+    def visit_call(self, node):
+        try:
+            module = node.frame().parent
+        except:  # noqa: E722, pylint: disable=bare-except
+            return
+
+        if not is_migrations_module(module):
+            return
+
+        if node.func.as_string().endswith('RunPython') and len(node.args) < 2:
+            if node.keywords:
+                for keyword in node.keywords:
+                    if keyword.arg == 'reverse_code':
+                        return
+                self.add_message('missing-backwards-migration-callable',
+                                 args=module.name, node=node)
+            else:
+                self.add_message('missing-backwards-migration-callable',
+                                 args=module.name, node=node)
+
+
 def load_configuration(linter):
     # don't blacklist migrations for this checker
     new_black_list = list(linter.config.black_list)
@@ -125,5 +151,6 @@ def load_configuration(linter):
 def register(linter):
     """Required method to auto register this checker."""
     linter.register_checker(NewDbFieldWithDefaultChecker(linter))
+    linter.register_checker(MissingBackwardsMigrationChecker(linter))
     if not compat.LOAD_CONFIGURATION_SUPPORTED:
         load_configuration(linter)

pylint_django/tests/input/migrations/0003_without_backwards.py

@@ -0,0 +1,20 @@
+# pylint: disable=missing-docstring, invalid-name
+from django.db import migrations
+
+
+# pylint: disable=unused-argument
+def forwards_test(apps, schema_editor):
+    pass
+
+
+class Migration(migrations.Migration):
+
+    operations = [
+        migrations.RunPython(),  # [missing-backwards-migration-callable]
+        migrations.RunPython(  # [missing-backwards-migration-callable]
+            forwards_test),
+        migrations.RunPython(  # [missing-backwards-migration-callable]
+            code=forwards_test),
+        migrations.RunPython(  # [missing-backwards-migration-callable]
+            code=forwards_test, atomic=False)
+    ]

pylint_django/tests/input/migrations/0003_without_backwards.txt

@@ -0,0 +1,4 @@
+missing-backwards-migration-callable:13:Migration:pylint_django.tests.input.migrations.0003_without_backwards Always include backwards migration callable
+missing-backwards-migration-callable:14:Migration:pylint_django.tests.input.migrations.0003_without_backwards Always include backwards migration callable
+missing-backwards-migration-callable:16:Migration:pylint_django.tests.input.migrations.0003_without_backwards Always include backwards migration callable
+missing-backwards-migration-callable:18:Migration:pylint_django.tests.input.migrations.0003_without_backwards Always include backwards migration callable

pylint_django/tests/input/migrations/0004_noerror_with_backwards.py

@@ -0,0 +1,21 @@
+# pylint: disable=missing-docstring, invalid-name
+from django.db import migrations
+
+
+# pylint: disable=unused-argument
+def forwards_test(apps, schema_editor):
+    pass
+
+
+# pylint: disable=unused-argument
+def backwards_test(apps, schema_editor):
+    pass
+
+
+class Migration(migrations.Migration):
+
+    operations = [
+        migrations.RunPython(forwards_test, backwards_test),
+        migrations.RunPython(forwards_test, reverse_code=backwards_test),
+        migrations.RunPython(code=forwards_test, reverse_code=backwards_test)
+    ]

pylint_django/utils.py

@@ -1,5 +1,6 @@
 """Utils."""
 import sys
+import astroid
 
 from astroid.bases import Instance
 from astroid.exceptions import InferenceError
@@ -30,3 +31,10 @@ def node_is_subclass(cls, *subclass_names):
             continue
 
     return False
+
+
+def is_migrations_module(node):
+    if not isinstance(node, astroid.Module):
+        return False
+
+    return 'migrations' in node.path[0] and not node.path[0].endswith('__init__.py')