Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Give a UserWarning when Parameter is overriden by name #2397

Merged
merged 10 commits into from
Nov 3, 2024
2 changes: 2 additions & 0 deletions CHANGES.rst
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,8 @@ Unreleased
``Context.call_on_close`` callbacks and context managers added via
``Context.with_resource`` to be closed on exit as well. :pr:`2680`
- Add ``ProgressBar(hidden: bool)`` to allow hiding the progressbar. :issue:`2609`
- A ``UserWarning`` will be shown when multiple parameters attempt to use the
same name. :issue:`2396``


Version 8.1.8
Expand Down
29 changes: 25 additions & 4 deletions src/click/core.py
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
import sys
import typing as t
from collections import abc
from collections import Counter
from contextlib import AbstractContextManager
from contextlib import contextmanager
from contextlib import ExitStack
Expand Down Expand Up @@ -957,13 +958,29 @@ def get_usage(self, ctx: Context) -> str:
return formatter.getvalue().rstrip("\n")

def get_params(self, ctx: Context) -> list[Parameter]:
rv = self.params
params = self.params
help_option = self.get_help_option(ctx)

if help_option is not None:
rv = [*rv, help_option]
params = [*params, help_option]

return rv
if __debug__:
import warnings

opts = [opt for param in params for opt in param.opts]
opts_counter = Counter(opts)
duplicate_opts = (opt for opt, count in opts_counter.items() if count > 1)

for duplicate_opt in duplicate_opts:
warnings.warn(
(
f"The parameter {duplicate_opt} is used more than once. "
"Remove its duplicate as parameters should be unique."
),
stacklevel=3,
)

return params

def format_usage(self, ctx: Context, formatter: HelpFormatter) -> None:
"""Writes the usage line into the formatter.
Expand Down Expand Up @@ -1973,6 +1990,10 @@ class Parameter:
of :class:`~click.shell_completion.CompletionItem` or a list of
strings.

.. versionchanged:: 8.2
Adding duplicate parameter names to a :class:`~click.core.Command` will
result in a ``UserWarning`` being shown.

.. versionchanged:: 8.0
``process_value`` validates required parameters and bounded
``nargs``, and invokes the parameter callback before returning
Expand Down Expand Up @@ -2974,7 +2995,7 @@ def _parse_decls(
else:
raise TypeError(
"Arguments take exactly one parameter declaration, got"
f" {len(decls)}."
f" {len(decls)}: {decls}."
)
return name, [arg], []

Expand Down
20 changes: 20 additions & 0 deletions tests/test_arguments.py
Original file line number Diff line number Diff line change
Expand Up @@ -401,3 +401,23 @@ def bar(arg):

assert isinstance(foo.params[0], CustomArgument)
assert isinstance(bar.params[0], CustomArgument)


@pytest.mark.parametrize(
"args_one,args_two",
[
(
("aardvark",),
("aardvark",),
),
],
)
def test_duplicate_names_warning(runner, args_one, args_two):
@click.command()
@click.argument(*args_one)
@click.argument(*args_two)
def cli(one, two):
pass

with pytest.warns(UserWarning):
runner.invoke(cli, [])
26 changes: 26 additions & 0 deletions tests/test_options.py
Original file line number Diff line number Diff line change
Expand Up @@ -989,3 +989,29 @@ def cli_without_choices(g):

result = runner.invoke(cli_without_choices, ["--help"])
assert metavars in result.output


@pytest.mark.parametrize(
"opts_one,opts_two",
[
# No duplicate shortnames
(
("-a", "--aardvark"),
("-a", "--avocado"),
),
# No duplicate long names
(
("-a", "--aardvark"),
("-b", "--aardvark"),
),
],
)
def test_duplicate_names_warning(runner, opts_one, opts_two):
@click.command()
@click.option(*opts_one)
@click.option(*opts_two)
def cli(one, two):
pass

with pytest.warns(UserWarning):
runner.invoke(cli, [])