diff --git a/django/utils/deprecation.py b/django/utils/deprecation.py index 6f3852cc04..7d5289bd3c 100644 --- a/django/utils/deprecation.py +++ b/django/utils/deprecation.py @@ -1,8 +1,13 @@ +import functools import inspect +import os import warnings +from collections import Counter from asgiref.sync import iscoroutinefunction, markcoroutinefunction, sync_to_async +import django + class RemovedInDjango61Warning(DeprecationWarning): pass @@ -83,6 +88,181 @@ class RenameMethodsBase(type): return new_class +def deprecate_posargs(deprecation_warning, remappable_names, /): + """ + Function/method decorator to deprecate some or all positional arguments. + + The decorated function will map any positional arguments after the ``*`` to + the corresponding keyword arguments and issue a deprecation warning. + + The decorator takes two arguments: a RemovedInDjangoXXWarning warning + category and a list of parameter names that have been changed from + positional-or-keyword to keyword-only, in their original positional order. + + Works on both functions and methods. To apply to a class constructor, + decorate its __init__() method. To apply to a staticmethod or classmethod, + use @deprecate_posargs after @staticmethod or @classmethod. + + Example: to deprecate passing option1 or option2 as posargs, change:: + + def some_func(request, option1, option2=True): + ... + + to:: + + @deprecate_posargs(RemovedInDjangoXXWarning, ["option1", "option2"]) + def some_func(request, *, option1, option2=True): + ... + + After the deprecation period, remove the decorator (but keep the ``*``):: + + def some_func(request, *, option1, option2=True): + ... + + Caution: during the deprecation period, do not add any new *positional* + parameters or change the remaining ones. For example, this attempt to add a + new param would break code using the deprecated posargs:: + + @deprecate_posargs(RemovedInDjangoXXWarning, ["option1", "option2"]) + def some_func(request, wrong_new_param=None, *, option1, option2=True): + # Broken: existing code may pass a value intended as option1 in the + # wrong_new_param position. + ... + + However, it's acceptable to add new *keyword-only* parameters and to + re-order the existing ones, so long as the list passed to + @deprecate_posargs is kept in the original posargs order. This change will + work without breaking existing code:: + + @deprecate_posargs(RemovedInDjangoXXWarning, ["option1", "option2"]) + def some_func(request, *, new_param=None, option2=True, option1): + ... + + The @deprecate_posargs decorator adds a small amount of overhead. In most + cases it won't be significant, but use with care in performance-critical + code paths. + """ + + def decorator(func): + if isinstance(func, type): + raise TypeError( + "@deprecate_posargs cannot be applied to a class. (Apply it " + "to the __init__ method.)" + ) + if isinstance(func, classmethod): + raise TypeError("Apply @classmethod before @deprecate_posargs.") + if isinstance(func, staticmethod): + raise TypeError("Apply @staticmethod before @deprecate_posargs.") + + params = inspect.signature(func).parameters + num_by_kind = Counter(param.kind for param in params.values()) + + if num_by_kind[inspect.Parameter.VAR_POSITIONAL] > 0: + raise TypeError( + "@deprecate_posargs() cannot be used with variable positional `*args`." + ) + + num_positional_params = ( + num_by_kind[inspect.Parameter.POSITIONAL_ONLY] + + num_by_kind[inspect.Parameter.POSITIONAL_OR_KEYWORD] + ) + num_keyword_only_params = num_by_kind[inspect.Parameter.KEYWORD_ONLY] + if num_keyword_only_params < 1: + raise TypeError( + "@deprecate_posargs() requires at least one keyword-only parameter " + "(after a `*` entry in the parameters list)." + ) + if any( + name not in params or params[name].kind != inspect.Parameter.KEYWORD_ONLY + for name in remappable_names + ): + raise TypeError( + "@deprecate_posargs() requires all remappable_names to be " + "keyword-only parameters." + ) + + num_remappable_args = len(remappable_names) + max_positional_args = num_positional_params + num_remappable_args + + func_name = func.__name__ + if func_name == "__init__": + # In the warning, show "ClassName()" instead of "__init__()". + # The class isn't defined yet, but its name is in __qualname__. + # Some examples of __qualname__: + # - ClassName.__init__ + # - Nested.ClassName.__init__ + # - MyTests.test_case..ClassName.__init__ + local_name = func.__qualname__.rsplit(".", 1)[-1] + class_name = local_name.replace(".__init__", "") + func_name = class_name + + def remap_deprecated_args(args, kwargs): + """ + Move deprecated positional args to kwargs and issue a warning. + Return updated (args, kwargs). + """ + if (num_positional_args := len(args)) > max_positional_args: + raise TypeError( + f"{func_name}() takes at most {max_positional_args} positional " + f"argument(s) (including {num_remappable_args} deprecated) but " + f"{num_positional_args} were given." + ) + + # Identify which of the _potentially remappable_ params are + # actually _being remapped_ in this particular call. + remapped_names = remappable_names[ + : num_positional_args - num_positional_params + ] + conflicts = set(remapped_names) & set(kwargs) + if conflicts: + # Report duplicate names in the original parameter order. + conflicts_str = ", ".join( + f"'{name}'" for name in remapped_names if name in conflicts + ) + raise TypeError( + f"{func_name}() got both deprecated positional and keyword " + f"argument values for {conflicts_str}." + ) + + # Do the remapping. + remapped_kwargs = dict( + zip(remapped_names, args[num_positional_params:], strict=True) + ) + remaining_args = args[:num_positional_params] + updated_kwargs = kwargs | remapped_kwargs + + # Issue the deprecation warning. + remapped_names_str = ", ".join(f"'{name}'" for name in remapped_names) + warnings.warn( + f"Passing positional argument(s) {remapped_names_str} to {func_name}() " + "is deprecated. Use keyword arguments instead.", + deprecation_warning, + skip_file_prefixes=(os.path.dirname(django.__file__),), + ) + + return remaining_args, updated_kwargs + + if iscoroutinefunction(func): + + @functools.wraps(func) + async def wrapper(*args, **kwargs): + if len(args) > num_positional_params: + args, kwargs = remap_deprecated_args(args, kwargs) + return await func(*args, **kwargs) + + else: + + @functools.wraps(func) + def wrapper(*args, **kwargs): + if len(args) > num_positional_params: + args, kwargs = remap_deprecated_args(args, kwargs) + return func(*args, **kwargs) + + return wrapper + + return decorator + + class MiddlewareMixin: sync_capable = True async_capable = True diff --git a/docs/internals/contributing/writing-code/submitting-patches.txt b/docs/internals/contributing/writing-code/submitting-patches.txt index fe354a0ba1..b8ead27ded 100644 --- a/docs/internals/contributing/writing-code/submitting-patches.txt +++ b/docs/internals/contributing/writing-code/submitting-patches.txt @@ -312,6 +312,11 @@ Once you have completed these steps, you are finished with the deprecation. In each :term:`feature release `, all ``RemovedInDjangoXXWarning``\s matching the new version are removed. +The ``django.utils.deprecation`` module provides some helpful deprecation +utilities, such as a ``@deprecate_posargs`` decorator to assist with converting +positional-or-keyword arguments to keyword-only. See the inline documentation +in the module source. + JavaScript contributions ======================== diff --git a/tests/deprecation/test_deprecate_posargs.py b/tests/deprecation/test_deprecate_posargs.py new file mode 100644 index 0000000000..166fe5dd69 --- /dev/null +++ b/tests/deprecation/test_deprecate_posargs.py @@ -0,0 +1,404 @@ +import inspect + +from django.test import SimpleTestCase +from django.utils.deprecation import RemovedAfterNextVersionWarning, deprecate_posargs + + +class DeprecatePosargsTests(SimpleTestCase): + # Note: these tests use the generic RemovedAfterNextVersionWarning so they + # don't need to be updated each release. In actual use, you must substitute + # a specific RemovedInDjangoXXWarning. + + def assertDeprecated(self, params, name): + msg = ( + "Passing positional argument(s) {0} to {1}() is deprecated. Use keyword " + "arguments instead." + ) + return self.assertWarnsMessage( + RemovedAfterNextVersionWarning, msg.format(params, name) + ) + + def test_all_keyword_only_params(self): + """All positional arguments are remapped to keyword-only arguments.""" + + @deprecate_posargs(RemovedAfterNextVersionWarning, ["a", "b"]) + def some_func(*, a=1, b=2): + return a, b + + with ( + self.subTest("Multiple affected args"), + self.assertDeprecated("'a', 'b'", "some_func"), + ): + result = some_func(10, 20) + self.assertEqual(result, (10, 20)) + + with ( + self.subTest("One affected arg"), + self.assertDeprecated("'a'", "some_func"), + ): + result = some_func(10) + self.assertEqual(result, (10, 2)) + + def test_some_keyword_only_params(self): + """Works when keeping some params as positional-or-keyword.""" + + @deprecate_posargs(RemovedAfterNextVersionWarning, ["b"]) + def some_func(a, *, b=1): + return a, b + + with self.assertDeprecated("'b'", "some_func"): + result = some_func(10, 20) + self.assertEqual(result, (10, 20)) + + def test_no_warning_when_not_needed(self): + @deprecate_posargs(RemovedAfterNextVersionWarning, ["b"]) + def some_func(a=0, *, b=1): + return a, b + + with self.subTest("All arguments supplied"), self.assertNoLogs(level="WARNING"): + result = some_func(10, b=20) + self.assertEqual(result, (10, 20)) + + with self.subTest("All default arguments"), self.assertNoLogs(level="WARNING"): + result = some_func() + self.assertEqual(result, (0, 1)) + + with ( + self.subTest("Partial arguments supplied"), + self.assertNoLogs(level="WARNING"), + ): + result = some_func(10) + self.assertEqual(result, (10, 1)) + + def test_allows_reordering_keyword_only_params(self): + """Keyword-only params can be freely added and rearranged.""" + + # Original signature: some_func(b=2, a=1), and remappable_names + # reflects the original positional argument order. + @deprecate_posargs(RemovedAfterNextVersionWarning, ["b", "a"]) + def some_func(*, aa_new=0, a=1, b=2): + return aa_new, a, b + + with self.assertDeprecated("'b', 'a'", "some_func"): + result = some_func(20, 10) + self.assertEqual(result, (0, 10, 20)) + + def test_detects_duplicate_arguments(self): + @deprecate_posargs(RemovedAfterNextVersionWarning, ["b", "c"]) + def func(a, *, b=1, c=2): + return a, b, c + + msg = ( + "func() got both deprecated positional and keyword argument values for {0}" + ) + with ( + self.subTest("One duplicate"), + self.assertRaisesMessage(TypeError, msg.format("'b'")), + ): + func(0, 10, b=12) + + with ( + self.subTest("Multiple duplicates"), + self.assertRaisesMessage(TypeError, msg.format("'b', 'c'")), + ): + func(0, 10, 20, b=12, c=22) + + with ( + self.subTest("No false positives for valid kwargs"), + # Deprecation warning for 'b', not TypeError for duplicate 'c'. + self.assertDeprecated("'b'", "func"), + ): + result = func(0, 11, c=22) + self.assertEqual(result, (0, 11, 22)) + + def test_detects_extra_positional_arguments(self): + @deprecate_posargs(RemovedAfterNextVersionWarning, ["b"]) + def func(a, *, b=1): + return a, b + + with self.assertRaisesMessage( + TypeError, + "func() takes at most 2 positional argument(s) (including 1 deprecated) " + "but 3 were given.", + ): + func(10, 20, 30) + + def test_avoids_remapping_to_new_keyword_arguments(self): + # Only 'b' is moving; 'c' was added later. + @deprecate_posargs(RemovedAfterNextVersionWarning, ["b"]) + def func(a, *, b=1, c=2): + return a, b, c + + with self.assertRaisesMessage( + TypeError, + "func() takes at most 2 positional argument(s) (including 1 deprecated) " + "but 3 were given.", + ): + func(10, 20, 30) + + def test_variable_kwargs(self): + """Works with **kwargs.""" + + @deprecate_posargs(RemovedAfterNextVersionWarning, ["b"]) + def some_func(a, *, b=1, **kwargs): + return a, b, kwargs + + with ( + self.subTest("Called with additional kwargs"), + self.assertDeprecated("'b'", "some_func"), + ): + result = some_func(10, 20, c=30) + self.assertEqual(result, (10, 20, {"c": 30})) + + with ( + self.subTest("Called without additional kwargs"), + self.assertDeprecated("'b'", "some_func"), + ): + result = some_func(10, 20) + self.assertEqual(result, (10, 20, {})) + + with ( + self.subTest("Called with too many positional arguments"), + # Similar to test_detects_extra_positional_arguments() above, + # but verifying logic is not confused by variable **kwargs. + self.assertRaisesMessage( + TypeError, + "some_func() takes at most 2 positional argument(s) (including 1 " + "deprecated) but 3 were given.", + ), + ): + some_func(10, 20, 30) + + with self.subTest("No warning needed"): + result = some_func(10, b=20, c=30) + self.assertEqual(result, (10, 20, {"c": 30})) + + def test_positional_only_params(self): + @deprecate_posargs(RemovedAfterNextVersionWarning, ["c"]) + def some_func(a, /, b, *, c=3): + return a, b, c + + with self.assertDeprecated("'c'", "some_func"): + result = some_func(10, 20, 30) + self.assertEqual(result, (10, 20, 30)) + + def test_class_methods(self): + """ + Deprecations for class methods should be bound properly and should + omit the `self` or `cls` argument from the suggested replacement. + """ + + class SomeClass: + @deprecate_posargs(RemovedAfterNextVersionWarning, ["a", "b"]) + def __init__(self, *, a=0, b=1): + self.a = a + self.b = b + + @deprecate_posargs(RemovedAfterNextVersionWarning, ["a", "b"]) + def some_method(self, *, a, b=1): + return self.a, self.b, a, b + + @staticmethod + @deprecate_posargs(RemovedAfterNextVersionWarning, ["a", "b"]) + def some_static_method(*, a, b=1): + return a, b + + @classmethod + @deprecate_posargs(RemovedAfterNextVersionWarning, ["a", "b"]) + def some_class_method(cls, *, a, b=1): + return cls.__name__, a, b + + with ( + self.subTest("Constructor"), + # Warning should use the class name, not `__init__()`. + self.assertDeprecated("'a', 'b'", "SomeClass"), + ): + instance = SomeClass(10, 20) + self.assertEqual(instance.a, 10) + self.assertEqual(instance.b, 20) + + with ( + self.subTest("Instance method"), + self.assertDeprecated("'a', 'b'", "some_method"), + ): + result = SomeClass().some_method(10, 20) + self.assertEqual(result, (0, 1, 10, 20)) + + with ( + self.subTest("Static method on instance"), + self.assertDeprecated("'a', 'b'", "some_static_method"), + ): + result = SomeClass().some_static_method(10, 20) + self.assertEqual(result, (10, 20)) + + with ( + self.subTest("Static method on class"), + self.assertDeprecated("'a', 'b'", "some_static_method"), + ): + result = SomeClass.some_static_method(10, 20) + self.assertEqual(result, (10, 20)) + + with ( + self.subTest("Class method on instance"), + self.assertDeprecated("'a', 'b'", "some_class_method"), + ): + result = SomeClass().some_class_method(10, 20) + self.assertEqual(result, ("SomeClass", 10, 20)) + + with ( + self.subTest("Class method on class"), + self.assertDeprecated("'a', 'b'", "some_class_method"), + ): + result = SomeClass.some_class_method(10, 20) + self.assertEqual(result, ("SomeClass", 10, 20)) + + def test_incorrect_classmethod_order(self): + """Catch classmethod applied in wrong order.""" + with self.assertRaisesMessage( + TypeError, "Apply @classmethod before @deprecate_posargs." + ): + + class SomeClass: + @deprecate_posargs(RemovedAfterNextVersionWarning, ["a"]) + @classmethod + def some_class_method(cls, *, a): + pass + + def test_incorrect_staticmethod_order(self): + """Catch staticmethod applied in wrong order.""" + with self.assertRaisesMessage( + TypeError, "Apply @staticmethod before @deprecate_posargs." + ): + + class SomeClass: + @deprecate_posargs(RemovedAfterNextVersionWarning, ["a"]) + @staticmethod + def some_static_method(*, a): + pass + + async def test_async(self): + """A decorated async function is still async.""" + + @deprecate_posargs(RemovedAfterNextVersionWarning, ["a", "b"]) + async def some_func(*, a, b=1): + return a, b + + self.assertTrue(inspect.iscoroutinefunction(some_func.__wrapped__)) + self.assertTrue(inspect.iscoroutinefunction(some_func)) + + with ( + self.subTest("With deprecation warning"), + self.assertDeprecated("'a', 'b'", "some_func"), + ): + result = await some_func(10, 20) + self.assertEqual(result, (10, 20)) + + with ( + self.subTest("Without deprecation warning"), + self.assertNoLogs(level="WARNING"), + ): + result = await some_func(a=10, b=20) + self.assertEqual(result, (10, 20)) + + def test_applied_to_lambda(self): + """ + Please don't try to deprecate lambda args! What does that even mean?! + (But if it happens, the decorator should do something reasonable.) + """ + lambda_func = deprecate_posargs(RemovedAfterNextVersionWarning, ["b"])( + lambda a, *, b=1: (a, b) + ) + with self.assertDeprecated("'b'", ""): + result = lambda_func(10, 20) + self.assertEqual(result, (10, 20)) + + def test_bare_init(self): + """Can't replace '__init__' with class name if not in a class.""" + + @deprecate_posargs(RemovedAfterNextVersionWarning, ["a"]) + def __init__(*, a): + pass + + with self.assertDeprecated("'a'", "__init__"): + __init__(10) + + def test_warning_source_location(self): + """The warning points to caller, not the decorator implementation.""" + + @deprecate_posargs(RemovedAfterNextVersionWarning, "a") + def some_func(*, a): + return a + + with self.assertWarns(RemovedAfterNextVersionWarning) as cm: + some_func(10) + self.assertEqual(cm.filename, __file__) + self.assertEqual(cm.lineno, inspect.currentframe().f_lineno - 2) + + def test_decorator_requires_keyword_only_params(self): + with self.assertRaisesMessage( + TypeError, + "@deprecate_posargs() requires at least one keyword-only parameter " + "(after a `*` entry in the parameters list).", + ): + + @deprecate_posargs(RemovedAfterNextVersionWarning, ["b"]) + def func(a, b=1): + return a, b + + def test_decorator_rejects_var_positional_param(self): + with self.assertRaisesMessage( + TypeError, + "@deprecate_posargs() cannot be used with variable positional `*args`.", + ): + + @deprecate_posargs(RemovedAfterNextVersionWarning, ["b"]) + def func(*args, b=1): + return args, b + + def test_decorator_does_not_apply_to_class(self): + with self.assertRaisesMessage( + TypeError, + "@deprecate_posargs cannot be applied to a class. (Apply it to the " + "__init__ method.)", + ): + + @deprecate_posargs(RemovedAfterNextVersionWarning, ["b"]) + class NotThisClass: + pass + + def test_decorator_requires_remappable_names_be_keyword_only(self): + """remappable_names cannot refer to positional-or-keyword params.""" + with self.assertRaisesMessage( + TypeError, + "@deprecate_posargs() requires all remappable_names to be keyword-only " + "parameters.", + ): + + @deprecate_posargs(RemovedAfterNextVersionWarning, ["a", "b"]) + def func(a, *, b=1): + return a, b + + def test_decorator_requires_remappable_names_exist(self): + """remappable_names cannot refer to variable kwargs.""" + with self.assertRaisesMessage( + TypeError, + "@deprecate_posargs() requires all remappable_names to be keyword-only " + "parameters.", + ): + + @deprecate_posargs(RemovedAfterNextVersionWarning, ["b", "c"]) + def func(a, *, b=1, **kwargs): + c = kwargs.get("c") + return a, b, c + + def test_decorator_preserves_signature_and_metadata(self): + + def original(a, b=1, *, c=2): + """Docstring.""" + return a, b, c + + decorated = deprecate_posargs(RemovedAfterNextVersionWarning, ["c"])(original) + self.assertEqual(original.__name__, decorated.__name__) + self.assertEqual(original.__qualname__, decorated.__qualname__) + self.assertEqual(original.__doc__, decorated.__doc__) + self.assertEqual(inspect.signature(original), inspect.signature(decorated))