Browse Source

Fixed #34355 -- Deprecated passing positional arguments to BaseConstraint.

Xavier Fernandez 2 years ago
parent
commit
ad18a0102c

+ 24 - 3
django/db/models/constraints.py

@@ -1,3 +1,4 @@
+import warnings
 from enum import Enum
 from enum import Enum
 from types import NoneType
 from types import NoneType
 
 
@@ -9,6 +10,7 @@ from django.db.models.lookups import Exact
 from django.db.models.query_utils import Q
 from django.db.models.query_utils import Q
 from django.db.models.sql.query import Query
 from django.db.models.sql.query import Query
 from django.db.utils import DEFAULT_DB_ALIAS
 from django.db.utils import DEFAULT_DB_ALIAS
+from django.utils.deprecation import RemovedInDjango60Warning
 from django.utils.translation import gettext_lazy as _
 from django.utils.translation import gettext_lazy as _
 
 
 __all__ = ["BaseConstraint", "CheckConstraint", "Deferrable", "UniqueConstraint"]
 __all__ = ["BaseConstraint", "CheckConstraint", "Deferrable", "UniqueConstraint"]
@@ -18,12 +20,31 @@ class BaseConstraint:
     default_violation_error_message = _("Constraint “%(name)s” is violated.")
     default_violation_error_message = _("Constraint “%(name)s” is violated.")
     violation_error_message = None
     violation_error_message = None
 
 
-    def __init__(self, name, violation_error_message=None):
+    # RemovedInDjango60Warning: When the deprecation ends, replace with:
+    # def __init__(self, *, name, violation_error_message=None):
+    def __init__(self, *args, name=None, violation_error_message=None):
+        # RemovedInDjango60Warning.
+        if name is None and not args:
+            raise TypeError(
+                f"{self.__class__.__name__}.__init__() missing 1 required keyword-only "
+                f"argument: 'name'"
+            )
         self.name = name
         self.name = name
         if violation_error_message is not None:
         if violation_error_message is not None:
             self.violation_error_message = violation_error_message
             self.violation_error_message = violation_error_message
         else:
         else:
             self.violation_error_message = self.default_violation_error_message
             self.violation_error_message = self.default_violation_error_message
+        # RemovedInDjango60Warning.
+        if args:
+            warnings.warn(
+                f"Passing positional arguments to {self.__class__.__name__} is "
+                f"deprecated.",
+                RemovedInDjango60Warning,
+                stacklevel=2,
+            )
+            for arg, attr in zip(args, ["name", "violation_error_message"]):
+                if arg:
+                    setattr(self, attr, arg)
 
 
     @property
     @property
     def contains_expressions(self):
     def contains_expressions(self):
@@ -67,7 +88,7 @@ class CheckConstraint(BaseConstraint):
             raise TypeError(
             raise TypeError(
                 "CheckConstraint.check must be a Q instance or boolean expression."
                 "CheckConstraint.check must be a Q instance or boolean expression."
             )
             )
-        super().__init__(name, violation_error_message=violation_error_message)
+        super().__init__(name=name, violation_error_message=violation_error_message)
 
 
     def _get_check_sql(self, model, schema_editor):
     def _get_check_sql(self, model, schema_editor):
         query = Query(model=model, alias_cols=False)
         query = Query(model=model, alias_cols=False)
@@ -186,7 +207,7 @@ class UniqueConstraint(BaseConstraint):
             F(expression) if isinstance(expression, str) else expression
             F(expression) if isinstance(expression, str) else expression
             for expression in expressions
             for expression in expressions
         )
         )
-        super().__init__(name, violation_error_message=violation_error_message)
+        super().__init__(name=name, violation_error_message=violation_error_message)
 
 
     @property
     @property
     def contains_expressions(self):
     def contains_expressions(self):

+ 3 - 0
docs/internals/deprecation.txt

@@ -18,6 +18,9 @@ details on these changes.
 * The ``DjangoDivFormRenderer`` and ``Jinja2DivFormRenderer`` transitional form
 * The ``DjangoDivFormRenderer`` and ``Jinja2DivFormRenderer`` transitional form
   renderers will be removed.
   renderers will be removed.
 
 
+* Support for passing positional arguments to ``BaseConstraint`` will be
+  removed.
+
 .. _deprecation-removed-in-5.1:
 .. _deprecation-removed-in-5.1:
 
 
 5.1
 5.1

+ 5 - 1
docs/ref/models/constraints.txt

@@ -48,12 +48,16 @@ option.
 ``BaseConstraint``
 ``BaseConstraint``
 ==================
 ==================
 
 
-.. class:: BaseConstraint(name, violation_error_message=None)
+.. class:: BaseConstraint(*, name, violation_error_message=None)
 
 
     Base class for all constraints. Subclasses must implement
     Base class for all constraints. Subclasses must implement
     ``constraint_sql()``, ``create_sql()``, ``remove_sql()`` and
     ``constraint_sql()``, ``create_sql()``, ``remove_sql()`` and
     ``validate()`` methods.
     ``validate()`` methods.
 
 
+    .. deprecated:: 5.0
+
+        Support for passing positional arguments is deprecated.
+
 All constraints have the following parameters in common:
 All constraints have the following parameters in common:
 
 
 ``name``
 ``name``

+ 4 - 0
docs/releases/5.0.txt

@@ -267,6 +267,10 @@ Miscellaneous
 * The ``DjangoDivFormRenderer`` and ``Jinja2DivFormRenderer`` transitional form
 * The ``DjangoDivFormRenderer`` and ``Jinja2DivFormRenderer`` transitional form
   renderers are deprecated.
   renderers are deprecated.
 
 
+* Passing positional arguments  ``name`` and ``violation_error_message`` to
+  :class:`~django.db.models.BaseConstraint` is deprecated in favor of
+  keyword-only arguments.
+
 Features removed in 5.0
 Features removed in 5.0
 =======================
 =======================
 
 

+ 28 - 9
tests/constraints/tests.py

@@ -7,6 +7,8 @@ from django.db.models.constraints import BaseConstraint, UniqueConstraint
 from django.db.models.functions import Lower
 from django.db.models.functions import Lower
 from django.db.transaction import atomic
 from django.db.transaction import atomic
 from django.test import SimpleTestCase, TestCase, skipIfDBFeature, skipUnlessDBFeature
 from django.test import SimpleTestCase, TestCase, skipIfDBFeature, skipUnlessDBFeature
+from django.test.utils import ignore_warnings
+from django.utils.deprecation import RemovedInDjango60Warning
 
 
 from .models import (
 from .models import (
     ChildModel,
     ChildModel,
@@ -26,48 +28,48 @@ def get_constraints(table):
 
 
 class BaseConstraintTests(SimpleTestCase):
 class BaseConstraintTests(SimpleTestCase):
     def test_constraint_sql(self):
     def test_constraint_sql(self):
-        c = BaseConstraint("name")
+        c = BaseConstraint(name="name")
         msg = "This method must be implemented by a subclass."
         msg = "This method must be implemented by a subclass."
         with self.assertRaisesMessage(NotImplementedError, msg):
         with self.assertRaisesMessage(NotImplementedError, msg):
             c.constraint_sql(None, None)
             c.constraint_sql(None, None)
 
 
     def test_contains_expressions(self):
     def test_contains_expressions(self):
-        c = BaseConstraint("name")
+        c = BaseConstraint(name="name")
         self.assertIs(c.contains_expressions, False)
         self.assertIs(c.contains_expressions, False)
 
 
     def test_create_sql(self):
     def test_create_sql(self):
-        c = BaseConstraint("name")
+        c = BaseConstraint(name="name")
         msg = "This method must be implemented by a subclass."
         msg = "This method must be implemented by a subclass."
         with self.assertRaisesMessage(NotImplementedError, msg):
         with self.assertRaisesMessage(NotImplementedError, msg):
             c.create_sql(None, None)
             c.create_sql(None, None)
 
 
     def test_remove_sql(self):
     def test_remove_sql(self):
-        c = BaseConstraint("name")
+        c = BaseConstraint(name="name")
         msg = "This method must be implemented by a subclass."
         msg = "This method must be implemented by a subclass."
         with self.assertRaisesMessage(NotImplementedError, msg):
         with self.assertRaisesMessage(NotImplementedError, msg):
             c.remove_sql(None, None)
             c.remove_sql(None, None)
 
 
     def test_validate(self):
     def test_validate(self):
-        c = BaseConstraint("name")
+        c = BaseConstraint(name="name")
         msg = "This method must be implemented by a subclass."
         msg = "This method must be implemented by a subclass."
         with self.assertRaisesMessage(NotImplementedError, msg):
         with self.assertRaisesMessage(NotImplementedError, msg):
             c.validate(None, None)
             c.validate(None, None)
 
 
     def test_default_violation_error_message(self):
     def test_default_violation_error_message(self):
-        c = BaseConstraint("name")
+        c = BaseConstraint(name="name")
         self.assertEqual(
         self.assertEqual(
             c.get_violation_error_message(), "Constraint “name” is violated."
             c.get_violation_error_message(), "Constraint “name” is violated."
         )
         )
 
 
     def test_custom_violation_error_message(self):
     def test_custom_violation_error_message(self):
         c = BaseConstraint(
         c = BaseConstraint(
-            "base_name", violation_error_message="custom %(name)s message"
+            name="base_name", violation_error_message="custom %(name)s message"
         )
         )
         self.assertEqual(c.get_violation_error_message(), "custom base_name message")
         self.assertEqual(c.get_violation_error_message(), "custom base_name message")
 
 
     def test_custom_violation_error_message_clone(self):
     def test_custom_violation_error_message_clone(self):
         constraint = BaseConstraint(
         constraint = BaseConstraint(
-            "base_name",
+            name="base_name",
             violation_error_message="custom %(name)s message",
             violation_error_message="custom %(name)s message",
         ).clone()
         ).clone()
         self.assertEqual(
         self.assertEqual(
@@ -77,7 +79,7 @@ class BaseConstraintTests(SimpleTestCase):
 
 
     def test_deconstruction(self):
     def test_deconstruction(self):
         constraint = BaseConstraint(
         constraint = BaseConstraint(
-            "base_name",
+            name="base_name",
             violation_error_message="custom %(name)s message",
             violation_error_message="custom %(name)s message",
         )
         )
         path, args, kwargs = constraint.deconstruct()
         path, args, kwargs = constraint.deconstruct()
@@ -88,6 +90,23 @@ class BaseConstraintTests(SimpleTestCase):
             {"name": "base_name", "violation_error_message": "custom %(name)s message"},
             {"name": "base_name", "violation_error_message": "custom %(name)s message"},
         )
         )
 
 
+    def test_deprecation(self):
+        msg = "Passing positional arguments to BaseConstraint is deprecated."
+        with self.assertRaisesMessage(RemovedInDjango60Warning, msg):
+            BaseConstraint("name", "violation error message")
+
+    def test_name_required(self):
+        msg = (
+            "BaseConstraint.__init__() missing 1 required keyword-only argument: 'name'"
+        )
+        with self.assertRaisesMessage(TypeError, msg):
+            BaseConstraint()
+
+    @ignore_warnings(category=RemovedInDjango60Warning)
+    def test_positional_arguments(self):
+        c = BaseConstraint("name", "custom %(name)s message")
+        self.assertEqual(c.get_violation_error_message(), "custom name message")
+
 
 
 class CheckConstraintTests(TestCase):
 class CheckConstraintTests(TestCase):
     def test_eq(self):
     def test_eq(self):