Repository: pallets/click (python)

Issue:
Parameter decorator factories shouldn't pop `cls` from `attrs`
Popping `cls` introduces a side-effect that prevents the reuse of the returned decorator. For example, consider the code below. The function `my_shared_argument` is a decorator that is expected to register an argument of type `ArgumentWithHelp`. Instead, what happens is:
- the first time the decorator is used, `cls=ArgumentWithHelp` will be popped from `attrs` -> `attrs` is now modified in-place
- the following times the decorator is used, `cls` won't be in `attrs`.

```python
import click


class ArgumentWithHelp(click.Argument):
    def __init__(self, *args, help=None, **attrs):
        super().__init__(*args, **attrs)
        self.help = help


def argument_with_help(*args, cls=ArgumentWithHelp, **kwargs):
    return click.argument(*args, cls=cls, **kwargs)


my_shared_argument = argument_with_help("pippo", help="very useful help")


@click.command()
@my_shared_argument
def foo(pippo):
    print(pippo)


@click.command()
@my_shared_argument
def bar(pippo):
    print(pippo)
```

Running this file as it is:
```python
Traceback (most recent call last):
  File "C:/Users/sboby/AppData/Roaming/JetBrains/PyCharmCE2022.1/scratches/scratch_4.py", line 27, in <module>
    def bar(pippo):
  File "H:\Repo\unbox\UnboxTranslate\venv\lib\site-packages\click\decorators.py", line 287, in decorator
    _param_memo(f, ArgumentClass(param_decls, **attrs))
  File "H:\Repo\unbox\UnboxTranslate\venv\lib\site-packages\click\core.py", line 2950, in __init__
    super().__init__(param_decls, required=required, **attrs)
TypeError: __init__() got an unexpected keyword argument 'help'

Process finished with exit code 1
```

<strike>The `@option`  decorator is affected by the same problem but it won't crash in a similar situation because `click.Option.__init__` ignore extra arguments: it will just silently use `click.Option` as class.</strike>

The `@option` decorator is actually not affected because `attrs` is copied. This copy is nonetheless unnecessary, since adding `cls` to the signature of the function would be a more straightforward solution.

Environment:

- Python version: irrelevant
- Click version: 8.1.3 (but all versions are affected)


Pull request description:
Add `cls` to signature of `@argument` and `@option`
This PR adds the `cls` argument to the signature of `@argument` and `@option`. This:
- fixes a bug in `@argument`, see #2294
- make copying `attrs` in `@option` unnecessary.

**Note:** I was tempted to define the argument as `cls: Type[Option] = Option` (and similarly for `Argument`) but that would have not been 100% equivalent to the existing code, which works even if you pass `None`.

Fixes #2294.

Checklist:

- [x] Add tests that demonstrate the correct behavior of the change. Tests should fail without the change.
- [x] Add or update relevant docs, in the docs folder and in code.
- [x] Add an entry in `CHANGES.rst` summarizing the change and linking to the issue.
- [x] Add `.. versionchanged::` entries in any relevant code docs.
- [x] Run `pre-commit` hooks and fix any issues.
- [x] Run `pytest` and `tox`, no tests failed.

Mypy fails with errors in files unrelated to this PR.

Patch:
diff --git a/CHANGES.rst b/CHANGES.rst
index cf27f57..14cc09d 100644
--- a/CHANGES.rst
+++ b/CHANGES.rst
@@ -24,6 +24,8 @@ Unreleased
     ``command_class``. :issue:`2416`
 -   ``multiple=True`` is allowed for flag options again and does not require
     setting ``default=()``. :issue:`2246, 2292, 2295`
+-   Make the decorators returned by ``@argument()`` and ``@option()`` reusable when the
+    ``cls`` parameter is used. :issue:`2294`
 
 
 Version 8.1.3
diff --git a/src/click/decorators.py b/src/click/decorators.py
index 9205aa7..797449d 100644
--- a/src/click/decorators.py
+++ b/src/click/decorators.py
@@ -329,7 +329,9 @@ def _param_memo(f: t.Callable[..., t.Any], param: Parameter) -> None:
         f.__click_params__.append(param)  # type: ignore
 
 
-def argument(*param_decls: str, **attrs: t.Any) -> _Decorator[FC]:
+def argument(
+    *param_decls: str, cls: t.Optional[t.Type[Argument]] = None, **attrs: t.Any
+) -> _Decorator[FC]:
     """Attaches an argument to the command.  All positional arguments are
     passed as parameter declarations to :class:`Argument`; all keyword
     arguments are forwarded unchanged (except ``cls``).
@@ -345,16 +347,19 @@ def argument(*param_decls: str, **attrs: t.Any) -> _Decorator[FC]:
         ``cls``.
     :param attrs: Passed as keyword arguments to the constructor of ``cls``.
     """
+    if cls is None:
+        cls = Argument
 
     def decorator(f: FC) -> FC:
-        ArgumentClass = attrs.pop("cls", None) or Argument
-        _param_memo(f, ArgumentClass(param_decls, **attrs))
+        _param_memo(f, cls(param_decls, **attrs))
         return f
 
     return decorator
 
 
-def option(*param_decls: str, **attrs: t.Any) -> _Decorator[FC]:
+def option(
+    *param_decls: str, cls: t.Optional[t.Type[Option]] = None, **attrs: t.Any
+) -> _Decorator[FC]:
     """Attaches an option to the command.  All positional arguments are
     passed as parameter declarations to :class:`Option`; all keyword
     arguments are forwarded unchanged (except ``cls``).
@@ -370,12 +375,11 @@ def option(*param_decls: str, **attrs: t.Any) -> _Decorator[FC]:
         ``cls``.
     :param attrs: Passed as keyword arguments to the constructor of ``cls``.
     """
+    if cls is None:
+        cls = Option
 
     def decorator(f: FC) -> FC:
-        # Issue 926, copy attrs, so pre-defined options can re-use the same cls=
-        option_attrs = attrs.copy()
-        OptionClass = option_attrs.pop("cls", None) or Option
-        _param_memo(f, OptionClass(param_decls, **option_attrs))
+        _param_memo(f, cls(param_decls, **attrs))
         return f
 
     return decorator


Test patch:
diff --git a/tests/test_arguments.py b/tests/test_arguments.py
index 3395c55..7f6629f 100644
--- a/tests/test_arguments.py
+++ b/tests/test_arguments.py
@@ -381,3 +381,23 @@ def test_nested_subcommand_help(runner):
     result = runner.invoke(cli, ["arg1", "cmd", "arg2", "subcmd", "--help"])
     assert not result.exception
     assert "Usage: cli ARG1 cmd ARG2 subcmd [OPTIONS]" in result.output
+
+
+def test_when_argument_decorator_is_used_multiple_times_cls_is_preserved():
+    class CustomArgument(click.Argument):
+        pass
+
+    reusable_argument = click.argument("art", cls=CustomArgument)
+
+    @click.command()
+    @reusable_argument
+    def foo(arg):
+        pass
+
+    @click.command()
+    @reusable_argument
+    def bar(arg):
+        pass
+
+    assert isinstance(foo.params[0], CustomArgument)
+    assert isinstance(bar.params[0], CustomArgument)