diff --git a/CHANGES.md b/CHANGES.md index 7bdf90595..ec161c3d2 100644 --- a/CHANGES.md +++ b/CHANGES.md @@ -1,3 +1,18 @@ +## Version 9.0.0 + +Unreleased + +- Breaking: an automatically derived parameter name must be a valid Python + identifier, or `TypeError` is raised. {class}`Argument` and {class}`Option` + derive their name the same way and apply the same check. {pr}`3827` +- Breaking: `expose_value=False` no longer bypass the name check for a parameter. + {pr}`3827` +- Breaking: an {class}`Option` now normalizes its declaration written as a Python + identifier to determine the parameter name. {pr}`3827` +- Breaking: neither kind builds a parameter without a declaration. + `click.argument(expose_value=False)` and `click.option(expose_value=False)` + used to name a parameter `""`. {pr}`3827` + ## Version 8.5.1 Unreleased diff --git a/docs/arguments.md b/docs/arguments.md index 14840e96c..fdfa73423 100644 --- a/docs/arguments.md +++ b/docs/arguments.md @@ -50,6 +50,32 @@ recognized, otherwise {data}`STRING` is used. If no default value is provided, the type is assumed to be {data}`STRING`. See {ref}`type-inference` for the types that are recognized. +(argument-names)= + +## Argument Names + +The single declaration is not used as the name verbatim. Every `-` is replaced +with `_` and the result is lower cased, so `click.argument("input-file")` names +its parameter `input_file`. That is the same transform options apply, and it is +likewise not reversible. + +The name must satisfy {meth}`str.isidentifier`, so that the callback can +receive it as a keyword argument. {ref}`Options ` derive their +name the same way and apply the same check. The +{ref}`caution about reserved keywords ` applies here too. + +An argument takes exactly one declaration, and passing more raises +{exc}`TypeError`. That declaration becomes the name through the +{ref}`transform every parameter shares `, where the examples +live. + +`expose_value=False` is no exception, because the name is also the key the +parser stores the value under. + +An argument takes exactly one declaration, where an option takes several and +{ref}`picks one of them `. Past that choice both kinds derive the +name the same way. + ```{admonition} Note on Required Arguments :class: note diff --git a/docs/options.md b/docs/options.md index df78abb24..d44d238f2 100644 --- a/docs/options.md +++ b/docs/options.md @@ -60,41 +60,67 @@ and converting underscores to dashes. invoke(echo, args=['--string-to-echo', 'Hi!']) ``` -More formally, Click will try to infer the decorated function argument name as -follows: +(option-names)= + +## Option Names + +An option carries several declarations, so Click picks one of them to name the +parameter: 1. If a positional argument is a valid [Python identifier](https://docs.python.org/3/reference/lexical_analysis.html#identifiers) (and thus does not have dashes), it is chosen. 2. If multiple positional arguments are prefixed with `--`, the first one declared is chosen. 3. Otherwise, the first positional argument prefixed with `-` is chosen. -To get the argument name, the chosen positional argument is converted to lower -case, a leading `-` or `--` is removed if found, and any remaining `-` -characters are replaced with `_`. +The chosen declaration is not used as the name verbatim. Its `-` or `--` prefix +is dropped, every remaining `-` is replaced with `_` and the result is lower +cased, so `click.option("--input-file")` names its parameter `input_file`. That +holds for rule 1 too: an identifier declaration says which declaration names the +parameter, not what the name is spelled like. + +The name must satisfy {meth}`str.isidentifier`, so that the callback can +receive it as a keyword argument. {ref}`Arguments ` derive their +name the same way and apply the same check, including the +{ref}`caution about reserved keywords `. ```{eval-rst} .. list-table:: Examples - :widths: 15 15 + :widths: 25 15 :header-rows: 1 * - Decorator Arguments - - Inferred Argument Name + - Declaration Chosen * - ``"-f", "--foo-bar"`` - - foo_bar - * - ``"-x"`` - - x - * - ``"-f", "--filename", "dest"`` - - dest - * - ``"--CamelCase"`` - - camelcase - * - ``"-f", "-fb"`` - - f + - ``--foo-bar`` * - ``"--f", "--foo-bar"`` - - f + - ``--f`` + * - ``"-f", "-fb"`` + - ``-f`` + * - ``"-f", "--filename", "dest"`` + - ``dest`` + * - ``"-f", "--filename", "Dest"`` + - ``Dest`` * - ``"---f"`` - - _f + - ``---f`` ``` +The chosen declaration then becomes the name through the +{ref}`transform every parameter shares `: the `-` or `--` +prefix is dropped, every remaining `-` becomes a `_`, and the result is lower +cased and checked. So `"-f", "--filename", "Dest"` names `dest`. + +Only the leading one or two dashes are ever a prefix. Every other dash becomes +an underscore wherever it sits, so `"---f"` names `_f` and `"---a----b--"` +names `_a____b__`. + +That transform is many-to-one, which is deliberate here: it lets several +options share a name to form a +[feature switch group](#feature-switch-group). + +`expose_value=False` is no exception, because the name is also the parser dest +the value is stored under. Pass an explicit name instead: +`click.option("--0-file", "zero_file", expose_value=False)`. + ## Basic Example A simple {class}`click.Option` takes one option name. By default, it's assumed @@ -509,6 +535,8 @@ literally. ¹: `default=True` is substituted with `flag_value`. ``` +(feature-switch-group)= + #### Feature switch groups (multiple flags sharing one variable) Several `flag_value` options can target the same parameter name to form a diff --git a/docs/parameters.md b/docs/parameters.md index 58f321989..4873b341e 100644 --- a/docs/parameters.md +++ b/docs/parameters.md @@ -36,7 +36,7 @@ the Python argument name when calling the decorated function with values. In the example, the argument's name is `filename`. The name must match the python arg name. To provide a different name for use in help text, see {ref}`doc-meta-variables`. -The option's names are `-t` and `--times`. More names are available for options and are covered in {ref}`options`. +The option's names are `-t` and `--times`. More names are available for options and are covered in {ref}`option names `. ```{eval-rst} .. click:example:: @@ -53,3 +53,96 @@ The option's names are `-t` and `--times`. More names are available for options invoke(multi_echo, ['--times=3', 'index.txt'], prog_name='multi_echo') ``` + +(name-transform)= + +Both kinds derive that name the same way. An option drops its prefix first, and +only the leading one or two dashes are ever a prefix. From there every `-` +becomes a `_`, wherever it sits, and the result is lower cased. +It must then satisfy {meth}`str.isidentifier`, so that the callback can receive +it as a keyword argument, and {exc}`TypeError` is raised when it does not. + +```{eval-rst} +.. list-table:: Examples + :widths: 20 20 15 + :header-rows: 1 + + * - Argument Declaration + - Option Declaration + - Inferred Name + * - ``"foo-bar"`` + - ``"--foo-bar"`` + - foo_bar + * - ``"Foo-Bar"`` + - ``"--Foo-Bar"`` + - foo_bar + * - ``"Foo_Bar"`` + - ``"--Foo_Bar"`` + - foo_bar + * - ``"x"`` + - ``"-x"`` + - x + * - ``"CamelCase"`` + - ``"--CamelCase"`` + - camelcase + * - ``"café"`` + - ``"--café"`` + - café + * - ``"ΟΔΟΣ"`` + - ``"--ΟΔΟΣ"`` + - οδος + * - ``"\N{KELVIN SIGN}"`` + - ``"--\N{KELVIN SIGN}"`` + - k + * - ``"foo-٣"`` + - ``"--foo-٣"`` + - foo_٣ + * - ``"a-----b"`` + - ``"--a-----b"`` + - a_____b + * - ``"a--"`` + - ``"--a--"`` + - a__ + * - ``"-a----b--"`` + - ``"---a----b--"`` + - _a____b__ + * - ``"--"`` + - ``"----"`` + - __ + * - ``"0-file"`` + - ``"--0-file"`` + - :exc:`TypeError` + * - ``"٣foo"`` + - ``"--٣foo"`` + - :exc:`TypeError` + * - ``"foo.bar"`` + - ``"--foo.bar"`` + - :exc:`TypeError` + * - ``"foo\N{NON-BREAKING HYPHEN}bar"`` + - ``"--foo\N{NON-BREAKING HYPHEN}bar"`` + - :exc:`TypeError` + * - ``"a\N{ZERO WIDTH SPACE}b"`` + - ``"--a\N{ZERO WIDTH SPACE}b"`` + - :exc:`TypeError` + * - ``""`` + - ``"--"`` + - :exc:`TypeError` +``` + +The transform is many-to-one and not reversible: the three spellings of +`foo-bar` above all name one parameter. Which declaration is transformed in the +first place is the only thing that differs between the two kinds, covered in +{ref}`option names ` and {ref}`argument names `. + +(keyword-names)= + +```{caution} +A [reserved keyword](https://docs.python.org/3/reference/lexical_analysis.html#keywords) +satisfies {meth}`str.isidentifier`, so Click accepts one: `click.option("--from")` +names its parameter `from`. No callback can declare that, so the command has to +accept `**kwargs`, and Python then stops checking the callback signature at all. + +Pass an explicit name instead: `click.option("--from", "source")`. An argument +takes one declaration and has no explicit-name channel, so rename the +declaration there. +``` diff --git a/src/click/core.py b/src/click/core.py index 18a43ac5f..8f72c5917 100644 --- a/src/click/core.py +++ b/src/click/core.py @@ -763,16 +763,14 @@ def ensure_object(self, object_type: type[V]) -> V: self.obj = rv = object_type() return rv - def _default_map_has(self, name: str | None) -> bool: + def _default_map_has(self, name: str) -> bool: """Check if :attr:`default_map` contains a real value for ``name``. - Returns ``False`` when the key is absent, the map is ``None``, - ``name`` is ``None``, or the stored value is the internal - :data:`UNSET` sentinel. + Returns ``False`` when the key is absent, the map is ``None``, or the + stored value is the internal :data:`UNSET` sentinel. """ return ( - name is not None - and self.default_map is not None + self.default_map is not None and name in self.default_map and self.default_map[name] is not UNSET ) @@ -2353,9 +2351,7 @@ def __init__( | None = None, deprecated: bool | str = False, ) -> None: - self.name, self.opts, self.secondary_opts = self._parse_decls( - param_decls or (), expose_value - ) + self.name, self.opts, self.secondary_opts = self._parse_decls(param_decls or ()) self.type: types.ParamType[t.Any] = types.convert_type(type, default) # Default nargs to what the type tells us if we have that @@ -2436,9 +2432,60 @@ def __repr__(self) -> str: @abstractmethod def _parse_decls( - self, decls: cabc.Sequence[str], expose_value: bool + self, decls: cabc.Sequence[str] ) -> tuple[str, list[str], list[str]]: ... + @staticmethod + def _name_from_spec(spec: str) -> str: + """Derive a parameter name from a single declaration. + + The declaration is lower-cased and every ``-`` becomes a ``_``, so + ``--input-file``, ``--Input-File`` and ``INPUT_FILE`` all name + ``input_file``. An option passes the declaration with its prefix + already stripped; an argument passes its sole declaration whole. + + The transform is many-to-one, and so cannot be reversed: the name does + not tell you which declaration produced it. + """ + return spec.replace("-", "_").lower() + + def _resolve_name(self, name: str | None, decls: cabc.Sequence[str]) -> str: + """Settle the name derived from ``decls``, or refuse it. + + A parameter's value reaches the command callback as a keyword + argument. The name must satisfy :meth:`str.isidentifier`. A keyword + such as ``from`` passes, and can then only be received by a + ``**kwargs`` callback. Every kind of parameter is held to this. + + ``expose_value=False`` is no exception. The name is also the key the + parser stores the value under, so two parameters that gave it up would + share that key and each read the other's value. + + :raises TypeError: when no name was derived, or the one derived is not + an identifier. + """ + if name is not None and name.isidentifier(): + return name + + if name is None: + message = _( + "Could not derive a valid Python identifier to name" + " {param_type} {decls!r}. Add a valid name to the parameter" + " declaration." + ).format(param_type=self.param_type_name, decls=decls) + else: + message = _( + "{param_type} {decls!r} tried to use {name!r} as its name, but" + " it is not a valid Python identifier. Add a valid name to the" + " parameter declaration." + ).format( + param_type=self.param_type_name.capitalize(), + decls=decls, + name=name, + ) + + raise TypeError(message) + @property def human_readable_name(self) -> str: """Returns the human readable name of this parameter. This is the @@ -2929,6 +2976,12 @@ class Option(Parameter): :param hidden: hide this option from help outputs. :param attrs: Other command arguments described in :class:`Parameter`. + .. versionchanged:: 9.0.0 + An automatic name must be a Python identifier. A declaration written as + an identifier picks which declaration names the parameter; it no longer + also sets the spelling, so ``click.option("--x", "Foo_Bar")`` names + ``foo_bar``. + .. versionchanged:: 8.4.0 Non-basic ``flag_value`` types (not ``str``, ``int``, ``float``, or ``bool``) are passed through unchanged instead of being stringified. @@ -3007,9 +3060,6 @@ def __init__( # Phase 1: prompt-related attributes. ``_infer_flag_kind`` reads ``self.prompt`` # and ``self.prompt_required`` so this must run first. if prompt is True: - if not self.name: - raise TypeError("'name' is required with 'prompt=True'.") - prompt_text = self.name.replace("_", " ").capitalize() elif prompt is False: prompt_text = None @@ -3261,7 +3311,7 @@ def get_error_hint(self, ctx: Context | None) -> str: return result def _parse_decls( - self, decls: cabc.Sequence[str], expose_value: bool + self, decls: cabc.Sequence[str] ) -> tuple[str, list[str], list[str]]: opts = [] secondary_opts = [] @@ -3297,18 +3347,14 @@ def _parse_decls( if name is None and possible_names: possible_names.sort(key=lambda x: -len(x[0])) # group long options first - name = possible_names[0][1].replace("-", "_").lower() - if not name.isidentifier(): - name = None + name = possible_names[0][1] - if name is None: - if not expose_value: - return "", opts, secondary_opts - raise TypeError( - _( - "Could not determine name for option with declarations {decls!r}" - ).format(decls=decls) - ) + # Whichever declaration won, it goes through the one transform. A + # declaration written as an identifier is not exempt from it. + if name is not None: + name = self._name_from_spec(name) + + name = self._resolve_name(name, decls) if not opts and not secondary_opts: raise TypeError( @@ -3426,11 +3472,7 @@ def get_help_extra(self, ctx: Context) -> types.OptionHelpExtra: envvar = self.envvar if envvar is None: - if ( - self.allow_from_autoenv - and ctx.auto_envvar_prefix is not None - and self.name - ): + if self.allow_from_autoenv and ctx.auto_envvar_prefix is not None: envvar = f"{ctx.auto_envvar_prefix}_{self.name.upper()}" if envvar is not None: @@ -3568,7 +3610,7 @@ def resolve_envvar_value(self, ctx: Context) -> str | None: if rv is not None: return rv - if self.allow_from_autoenv and ctx.auto_envvar_prefix is not None and self.name: + if self.allow_from_autoenv and ctx.auto_envvar_prefix is not None: envvar = f"{ctx.auto_envvar_prefix}_{self.name.upper()}" rv = os.environ.get(envvar) @@ -3708,6 +3750,11 @@ class Argument(Parameter): :param help: the help string. + .. versionchanged:: 9.0.0 + Exactly one declaration is required, and it must name a Python + identifier once it is lower-cased and every ``-`` is replaced with + ``_``. ``expose_value=False`` is no exception. + .. versionchanged:: 8.5.0 Added the ``help`` parameter. """ @@ -3778,22 +3825,18 @@ def make_metavar(self, ctx: Context) -> str: return var def _parse_decls( - self, decls: cabc.Sequence[str], expose_value: bool + self, decls: cabc.Sequence[str] ) -> tuple[str, list[str], list[str]]: - if not decls: - if not expose_value: - return "", [], [] - raise TypeError("Argument is marked as exposed, but does not have a name.") - if len(decls) == 1: - name = arg = decls[0] - name = name.replace("-", "_").lower() - else: + if len(decls) != 1: raise TypeError( _( "Arguments take exactly one parameter declaration, got" " {length}: {decls}." ).format(length=len(decls), decls=decls) ) + + arg = decls[0] + name = self._resolve_name(self._name_from_spec(arg), decls) return name, [arg], [] def get_usage_pieces(self, ctx: Context) -> list[str]: diff --git a/src/click/parser.py b/src/click/parser.py index 4fcbf7caa..adb323c3d 100644 --- a/src/click/parser.py +++ b/src/click/parser.py @@ -129,7 +129,7 @@ def __init__( self, obj: CoreOption, opts: cabc.Sequence[str], - dest: str | None, + dest: str, action: str | None = None, nargs: int = 1, const: t.Any | None = None, @@ -168,22 +168,22 @@ def takes_value(self) -> bool: def process(self, value: t.Any, state: _ParsingState) -> None: if self.action == "store": - state.opts[self.dest] = value # type: ignore + state.opts[self.dest] = value elif self.action == "store_const": - state.opts[self.dest] = self.const # type: ignore + state.opts[self.dest] = self.const elif self.action == "append": - state.opts.setdefault(self.dest, []).append(value) # type: ignore + state.opts.setdefault(self.dest, []).append(value) elif self.action == "append_const": - state.opts.setdefault(self.dest, []).append(self.const) # type: ignore + state.opts.setdefault(self.dest, []).append(self.const) elif self.action == "count": - state.opts[self.dest] = state.opts.get(self.dest, 0) + 1 # type: ignore + state.opts[self.dest] = state.opts.get(self.dest, 0) + 1 else: raise ValueError(f"unknown action '{self.action}'") state.order.append(self.obj) class _Argument: - def __init__(self, obj: CoreArgument, dest: str | None, nargs: int = 1): + def __init__(self, obj: CoreArgument, dest: str, nargs: int = 1): self.dest = dest self.nargs = nargs self.obj = obj @@ -209,7 +209,7 @@ def process( if value == (): value = UNSET - state.opts[self.dest] = value # type: ignore + state.opts[self.dest] = value state.order.append(self.obj) @@ -266,7 +266,7 @@ def add_option( self, obj: CoreOption, opts: cabc.Sequence[str], - dest: str | None, + dest: str, action: str | None = None, nargs: int = 1, const: t.Any | None = None, @@ -287,7 +287,7 @@ def add_option( for opt in option._long_opts: self._long_opt[opt] = option - def add_argument(self, obj: CoreArgument, dest: str | None, nargs: int = 1) -> None: + def add_argument(self, obj: CoreArgument, dest: str, nargs: int = 1) -> None: """Adds a positional argument named `dest` to the parser. The `obj` can be used to identify the option in the order list diff --git a/tests/test_arguments.py b/tests/test_arguments.py index fea48ccba..42b40bfb8 100644 --- a/tests/test_arguments.py +++ b/tests/test_arguments.py @@ -1,7 +1,10 @@ +import itertools import sys +import unicodedata from unittest import mock import pytest +from test_options import ENV_NAMES_ARE_CASE_INSENSITIVE import click from click._utils import UNSET @@ -79,6 +82,342 @@ def copy(x): assert "Got unexpected extra argument (bar)" in result.output +@pytest.mark.parametrize( + ("decl", "expect"), + [ + ("src", "src"), + ("foo-bar", "foo_bar"), + ("FOO-BAR", "foo_bar"), + ("Foo_Bar", "foo_bar"), + ("foo__bar", "foo__bar"), + ("_foo", "_foo"), + ("__foo", "__foo"), + ], +) +def test_argument_names(runner, decl, expect): + @click.command() + @click.argument(decl) + def cmd(**kwargs): + click.echo(kwargs[expect]) + + assert cmd.params[0].name == expect + + result = runner.invoke(cmd, ["value"]) + assert not result.exception + assert result.output == "value\n" + + +def test_argument_normalizes_an_identifier_decl(): + assert click.Argument(["Foo_Bar"]).name == "foo_bar" + assert click.Option(["--x", "Foo_Bar"]).name == "foo_bar" + + +PARAM_KINDS = [ + pytest.param(click.Argument, "{decl}", id="argument"), + pytest.param(click.Option, "--{decl}", id="option"), +] + +# Declarations whose lower casing is neither one-to-one nor length-preserving. +UNICODE_CASE_DECLS = [ + pytest.param("Ω", "ω", id="omega"), + pytest.param("İ", "i\N{COMBINING DOT ABOVE}", id="dotted-capital-i"), + pytest.param("ΟΔΟΣ", "οδος", id="final-sigma"), + pytest.param("ẞ", "ß", id="capital-sharp-s"), + pytest.param("\N{KELVIN SIGN}", "k", id="kelvin-sign"), + pytest.param("foo-٣", "foo_٣", id="arabic-indic-digit"), +] + +# Declarations covering every shape the naming transform accepts. +NAME_SWEEP_DECLS = [ + "", + "-", + "--", + "---", + "0", + "--0", + "0-file", + "--0-file", + "foo.bar", + "--foo.bar", + "foo bar", + "x", + "--x", + "X_Y", + "--X-Y", +] + + +@pytest.mark.parametrize("count", [1, 2]) +@pytest.mark.parametrize("expose_value", [True, False]) +def test_parameter_name_is_always_an_identifier(count, expose_value): + built = 0 + + for decls in itertools.product(NAME_SWEEP_DECLS, repeat=count): + for cls in (click.Option, click.Argument): + try: + param = cls(list(decls), expose_value=expose_value) + except (TypeError, ValueError): + continue + + built += 1 + assert param.name.isidentifier(), ( + f"{cls.__name__}({list(decls)!r}, expose_value={expose_value})" + f" named its parameter {param.name!r}" + ) + + assert built, "the sweep built no parameter, so it proves nothing" + + +def test_argument_requires_its_one_declaration(): + """An argument with no declaration is refused, whatever ``expose_value`` says.""" + with pytest.raises(TypeError, match="exactly one parameter declaration"): + click.Argument([]) + + with pytest.raises(TypeError, match="exactly one parameter declaration"): + click.Argument([], expose_value=False) + + +def test_argument_name_check_applies_when_not_exposed(): + with pytest.raises(TypeError, match="valid Python identifier"): + click.Argument(["0foo"], expose_value=False) + + +def test_argument_metavar_renders_what_a_declaration_may_not(runner): + """``metavar`` carries a display the declaration is no longer allowed to. + + An argument takes exactly one declaration and has no explicit-name channel, + so a display such as ``0FOO`` is reached by naming the parameter separately + and passing the display as ``metavar``. + """ + seen = [] + + def record(ctx, param, value): + seen.append(value) + + @click.command() + @click.argument("zero_foo", expose_value=False, callback=record, metavar="0FOO") + def cmd(**kwargs): + click.echo(repr(kwargs)) + + assert cmd.params[0].name == "zero_foo" + + result = runner.invoke(cmd, ["value"]) + assert not result.exception + assert result.output == "{}\n" + assert seen == ["value"] + + result = runner.invoke(cmd, ["--help"]) + assert "0FOO" in result.output + + +@pytest.mark.parametrize(("cls", "form"), PARAM_KINDS) +@pytest.mark.parametrize(("decl", "expect"), UNICODE_CASE_DECLS) +def test_parameter_name_unicode_case_transform(cls, form, decl, expect): + """``str.lower()`` is neither one-to-one nor length-preserving.""" + assert cls([form.format(decl=decl)]).name == expect + + +@pytest.mark.parametrize(("decl", "expect"), UNICODE_CASE_DECLS) +def test_option_explicit_name_runs_the_same_case_transform(decl, expect): + """An explicit name is transformed exactly as a derived one is.""" + if not decl.isidentifier(): + assert click.Option(["--x", decl]).name == "x" + return + + assert click.Option(["--x", decl]).name == expect + + +@pytest.mark.parametrize( + ("arg_decl", "opt_decl", "expect"), + [ + pytest.param("a-b-c", "--a-b-c", "a_b_c", id="interior-singles"), + pytest.param("a-----b", "--a-----b", "a_____b", id="interior-run"), + pytest.param("a--", "--a--", "a__", id="trailing"), + pytest.param("-a-", "---a-", "_a_", id="leading-survives-the-prefix"), + pytest.param("-a----b--", "---a----b--", "_a____b__", id="everywhere"), + pytest.param("--", "----", "__", id="dashes-only"), + pytest.param("_-_", "--_-_", "___", id="around-an-underscore"), + ], +) +def test_parameter_name_keeps_every_dash_past_the_prefix(arg_decl, opt_decl, expect): + """Only an option's leading dashes are considered a prefix.""" + assert click.Argument([arg_decl]).name == expect + assert click.Option([opt_decl], is_flag=True).name == expect + + +@pytest.mark.parametrize(("cls", "form"), PARAM_KINDS) +@pytest.mark.parametrize( + "decl", + [ + pytest.param("0foo", id="leading-digit"), + pytest.param("0", id="digit-only"), + pytest.param("foo.bar", id="dot"), + pytest.param("foo bar", id="space"), + pytest.param("\u0663foo", id="leading-arabic-indic-digit"), + # Separators that read as a hyphen but are not the one replaced. + pytest.param("foo\N{NON-BREAKING HYPHEN}bar", id="non-breaking-hyphen"), + pytest.param("foo\u2013bar", id="en-dash"), + pytest.param("foo\u2212bar", id="minus-sign"), + # Characters that occupy no width at all. + pytest.param("a\N{ZERO WIDTH SPACE}b", id="zero-width-space"), + pytest.param("a\N{SOFT HYPHEN}b", id="soft-hyphen"), + pytest.param("a\N{RIGHT-TO-LEFT OVERRIDE}b", id="right-to-left-override"), + # Even nothing at all, which reaches an option as a bare ``--``. + pytest.param("", id="empty"), + ], +) +def test_parameter_name_must_be_an_identifier(cls, form, decl): + with pytest.raises(TypeError, match="valid Python identifier"): + cls([form.format(decl=decl)]) + + +@pytest.mark.parametrize(("cls", "form"), PARAM_KINDS) +@pytest.mark.parametrize( + "char", + [ + pytest.param("\N{ZERO WIDTH JOINER}", id="zero-width-joiner"), + pytest.param("\N{ZERO WIDTH NON-JOINER}", id="zero-width-non-joiner"), + ], +) +def test_parameter_name_identifier_check_follows_the_unicode_table(cls, form, char): + """Python 3.13 allows unicode zero-width joiner and non-joiner in identifiers.""" + name = f"a{char}b" + assert name.isidentifier() == (sys.version_info >= (3, 13)) + decl = form.format(decl=name) + + if not name.isidentifier(): + with pytest.raises(TypeError, match="valid Python identifier"): + cls([decl]) + return + + assert cls([decl]).name == name + + +@pytest.mark.parametrize( + ("decorator", "decl", "argv"), + [ + pytest.param(click.argument, "fi", ["value"], id="argument"), + pytest.param(click.option, "--fi", ["--fi", "value"], id="option"), + ], +) +def test_parameter_name_is_not_nfkc_normalized(runner, decorator, decl, argv): + """``str.isidentifier()`` is not the test for "can be a parameter name". + + Python normalizes an identifier written in source to NFKC, so the ligature + "fi" compiles to the two letters. ``_parse_decls`` runs no normalization, + so the name keeps the ligature and only ``**kwargs`` can carry it. + """ + + @click.command() + @decorator(decl) + def cmd(**kwargs): + click.echo(repr(kwargs)) + + name = cmd.params[0].name + assert name == "fi" + assert name.isidentifier() + assert unicodedata.normalize("NFKC", name) == "fi" + + result = runner.invoke(cmd, argv) + assert not result.exception + assert result.output == "{'fi': 'value'}\n" + + +def test_argument_name_keeps_its_normalization_form(runner): + """A composed and a decomposed declaration are two distinct arguments. + + Both render as ``café`` and both are valid identifiers, so the pair + coexists on one command with nothing on screen to tell them apart. + """ + decomposed = "cafe\N{COMBINING ACUTE ACCENT}" + composed = unicodedata.normalize("NFC", decomposed) + + @click.command() + @click.argument(composed) + @click.argument(decomposed) + def cmd(**kwargs): + click.echo(repr(sorted(kwargs))) + + assert [p.name for p in cmd.params] == [composed, decomposed] + + result = runner.invoke(cmd, ["one", "two"]) + assert not result.exception + assert result.output == f"['{decomposed}', '{composed}']\n" + + +def test_argument_name_case_transform_can_collide(runner): + + @click.command() + @click.argument("Foo-Bar") + @click.argument("foo_bar") + def cmd(**kwargs): + click.echo(repr(kwargs)) + + with pytest.warns(UserWarning, match="is used by an argument"): + result = runner.invoke(cmd, ["one", "two"], catch_exceptions=False) + + assert result.output == "{'foo_bar': 'two'}\n" + + +def test_argument_name_can_collide_with_an_option(runner): + + @click.command() + @click.option("--foo-bar") + @click.argument("Foo-Bar", required=False) + def cmd(**kwargs): + click.echo(repr(kwargs)) + + assert [p.name for p in cmd.params] == ["foo_bar", "foo_bar"] + + with pytest.warns(UserWarning, match="is used by an argument"): + result = runner.invoke( + cmd, ["--foo-bar", "from-option", "from-argument"], catch_exceptions=False + ) + + assert result.output == "{'foo_bar': 'from-argument'}\n" + + +def test_argument_has_no_auto_envvar(runner): + """An argument reads only the envvars it names, never a derived one.""" + + @click.command() + @click.argument("Foo-Bar", required=False) + def cmd(**kwargs): + click.echo(repr(kwargs)) + + result = runner.invoke( + cmd, [], auto_envvar_prefix="TEST", env={"TEST_FOO_BAR": "foo"} + ) + assert not result.exception + assert result.output == "{'foo_bar': None}\n" + + +@pytest.mark.parametrize( + ("env", "expect"), + [ + pytest.param({"ArG": "foo"}, "'foo'", id="exact"), + pytest.param({"ARG": "foo"}, "None", id="upper"), + pytest.param({"arg": "foo"}, "None", id="lower"), + ], +) +def test_argument_explicit_envvar_case_sensitivity(runner, env, expect): + """An argument matches its named envvar exactly, like an option does. + + And loses the distinction on Windows, like an option does. + """ + + @click.command() + @click.argument("arg", envvar="ArG", required=False) + def cmd(arg): + click.echo(repr(arg)) + + result = runner.invoke(cmd, [], env=env) + assert not result.exception + if ENV_NAMES_ARE_CASE_INSENSITIVE: + expect = "'foo'" + assert result.output == f"{expect}\n" + + def test_bytes_args(runner, monkeypatch): @click.command() @click.argument("arg") diff --git a/tests/test_options.py b/tests/test_options.py index 0656a0b54..7a88dcc30 100644 --- a/tests/test_options.py +++ b/tests/test_options.py @@ -3,6 +3,8 @@ import re import sys import tempfile +import unicodedata +import warnings from contextlib import nullcontext from typing import Literal @@ -813,6 +815,131 @@ def cmd(arg): assert result.output == "foo\n" +# CPython upper-cases every key of ``os.environ`` when ``os.name == "nt"``, +# see ``os._createenviron``. ``CliRunner`` writes the ``env`` mapping through +# ``os.environ`` and ``Parameter.resolve_envvar_value`` reads it back the same +# way, so on Windows a variable answers to every spelling of its name and +# elsewhere to exactly one. Both halves are asserted rather than skipped: the +# difference is the behaviour being pinned. See pallets/click#2483. +ENV_NAMES_ARE_CASE_INSENSITIVE = sys.platform == "win32" + + +def test_auto_envvar_uses_the_transformed_name(runner): + """The auto envvar is built from the name, which the transform lower-cased.""" + + @click.command() + @click.option("--Foo-Bar") + def cmd(foo_bar): + click.echo(foo_bar) + + result = runner.invoke( + cmd, [], auto_envvar_prefix="TEST", env={"TEST_FOO_BAR": "foo"} + ) + assert not result.exception + assert result.output == "foo\n" + + +def test_auto_envvar_ignores_decl_case(runner): + """The case written in the declaration never reaches the auto envvar.""" + + @click.command() + @click.option("--Foo-Bar") + def cmd(foo_bar): + click.echo(repr(foo_bar)) + + result = runner.invoke( + cmd, [], auto_envvar_prefix="TEST", env={"TEST_Foo_Bar": "foo"} + ) + assert not result.exception + expect = "'foo'" if ENV_NAMES_ARE_CASE_INSENSITIVE else "None" + assert result.output == f"{expect}\n" + + +def test_auto_envvar_prefix_is_upper_cased(runner): + """A lower-case prefix reaches an upper-case variable, and only that one.""" + + @click.command() + @click.option("--flag/--no-flag") + def cmd(flag): + click.echo(repr(flag)) + + result = runner.invoke(cmd, [], auto_envvar_prefix="yo", env={"YO_FLAG": "1"}) + assert not result.exception + assert result.output == "True\n" + + result = runner.invoke(cmd, [], auto_envvar_prefix="yo", env={"yo_FLAG": "1"}) + assert not result.exception + assert result.output == f"{ENV_NAMES_ARE_CASE_INSENSITIVE}\n" + + +def test_auto_envvar_flattens_name_case(runner): + """Declarations differing only by case name one parameter, and one envvar.""" + + @click.command() + @click.option("--foo-bar") + @click.option("--other", "Foo_Bar") + def cmd(**kwargs): + click.echo(repr(sorted(kwargs.items()))) + + assert [p.name for p in cmd.params if p.name] == ["foo_bar", "foo_bar"] + + result = runner.invoke( + cmd, [], auto_envvar_prefix="TEST", env={"TEST_FOO_BAR": "foo"} + ) + assert not result.exception + assert result.output == "[('foo_bar', 'foo')]\n" + + +def test_auto_envvar_upper_can_change_length(runner): + """Deriving the envvar is not the inverse of deriving the name.""" + + @click.command() + @click.option("--ẞ") + def cmd(**kwargs): + click.echo(repr(kwargs)) + + result = runner.invoke(cmd, [], auto_envvar_prefix="TEST", env={"TEST_SS": "foo"}) + assert not result.exception + assert result.output == "{'ß': 'foo'}\n" + + +@pytest.mark.parametrize( + ("env", "expect"), + [ + pytest.param({"ArG": "foo"}, "'foo'", id="exact"), + pytest.param({"ARG": "foo"}, "None", id="upper"), + pytest.param({"arg": "foo"}, "None", id="lower"), + ], +) +def test_explicit_envvar_case_sensitivity(runner, env, expect): + """An explicitly named envvar keeps the case it was registered with.""" + + @click.command() + @click.option("--arg", envvar="ArG") + def cmd(arg): + click.echo(repr(arg)) + + result = runner.invoke(cmd, [], env=env) + assert not result.exception + if ENV_NAMES_ARE_CASE_INSENSITIVE: + expect = "'foo'" + assert result.output == f"{expect}\n" + + +@pytest.mark.parametrize("name", ("FlAg", "sUper")) +def test_explicit_envvar_list_keeps_each_spelling(runner, name): + """Every name of an envvar list is matched with its own case.""" + + @click.command() + @click.option("--flag/--no-flag", envvar=["FlAg", "sUper"]) + def cmd(flag): + click.echo(repr(flag)) + + result = runner.invoke(cmd, [], env={name: "1"}) + assert not result.exception + assert result.output == "True\n" + + def test_nargs_envvar(runner): @click.command() @click.option("--arg", nargs=2) @@ -1273,6 +1400,21 @@ def cli_alt(warnings): (["-c", "-a", "--cantaloupe", "-b", "--banana", "--apple"], "cantaloupe"), (["--from", "-f", "_from"], "_from"), (["--return", "-r", "_ret"], "_ret"), + # A name derived from an option string is lower-cased. + (["--Foo-Bar"], "foo_bar"), + (["--FOO-BAR", "-F"], "foo_bar"), + # An identifier declaration goes through the same transform. + (["--foo-bar", "-f", "explicit_name"], "explicit_name"), + (["--foo-bar", "-f", "Explicit_Name"], "explicit_name"), + # Underscores survive, and every dash past the prefix becomes one. + (["--foo__bar"], "foo__bar"), + (["--foo--bar"], "foo__bar"), + (["--_foo"], "_foo"), + (["--__foo"], "__foo"), + (["---foo"], "_foo"), + (["-_"], "_"), + # A digit is only refused in the leading position. + (["--foo-0"], "foo_0"), ], ) def test_option_names(runner, option_args, expected): @@ -1289,6 +1431,122 @@ def cmd(**kwargs): assert result.output == "True\n" +def test_option_name_case_transform_can_collide(runner): + """Two declarations that differ can transform to one name, with no warning.""" + + @click.command() + @click.option("--\N{KELVIN SIGN}") + @click.option("--k") + def cmd(**kwargs): + click.echo(repr(kwargs)) + + assert [p.name for p in cmd.params] == ["k", "k"] + + result = runner.invoke(cmd, ["--\N{KELVIN SIGN}", "kelvin", "--k", "ascii"]) + assert not result.exception + assert result.output == "{'k': 'ascii'}\n" + + +def test_option_name_case_variants_share_one_parameter(runner): + """Case variants of one option collapse onto a single parameter.""" + + @click.command() + @click.option("--foo-bar") + @click.option("--Foo-Bar") + @click.option("--FOO-BAR") + def cmd(**kwargs): + click.echo(repr(kwargs)) + + assert [p.name for p in cmd.params] == ["foo_bar"] * 3 + + with warnings.catch_warnings(record=True) as caught: + warnings.simplefilter("always") + result = runner.invoke(cmd, ["--help"], catch_exceptions=False) + + assert not [w for w in caught if issubclass(w.category, UserWarning)] + for spelling in ("--foo-bar", "--Foo-Bar", "--FOO-BAR"): + assert spelling in result.output + + for spelling, value in (("--foo-bar", "a"), ("--Foo-Bar", "b"), ("--FOO-BAR", "c")): + result = runner.invoke(cmd, [spelling, value]) + assert not result.exception + assert result.output == f"{{'foo_bar': {value!r}}}\n" + + # Sharing one slot, the last spelling on the command line wins. + result = runner.invoke(cmd, ["--foo-bar", "a", "--FOO-BAR", "c"]) + assert result.output == "{'foo_bar': 'c'}\n" + + +def test_option_name_keeps_its_normalization_form(runner): + """A composed and a decomposed declaration are two distinct parameters.""" + decomposed = "cafe\N{COMBINING ACUTE ACCENT}" + composed = unicodedata.normalize("NFC", decomposed) + + @click.command() + @click.option(f"--{composed}") + @click.option(f"--{decomposed}") + def cmd(**kwargs): + click.echo(repr(sorted(kwargs))) + + assert [p.name for p in cmd.params] == [composed, decomposed] + + result = runner.invoke(cmd, []) + assert not result.exception + assert result.output == f"['{decomposed}', '{composed}']\n" + + +def test_option_name_must_be_an_identifier(): + """A short option is the one refused shape an argument cannot be written as.""" + with pytest.raises(TypeError, match="valid Python identifier"): + click.Option(["-0"]) + + +def test_option_prompt_needs_no_name_guard(): + """``prompt=True`` is refused by naming first, so it needs no guard of its own.""" + with pytest.raises(TypeError, match="valid Python identifier"): + click.Option(["--0-file"], expose_value=False, prompt=True) + + +def test_option_name_check_applies_when_not_exposed(): + with pytest.raises(TypeError, match="valid Python identifier"): + click.Option(["--0foo"], expose_value=False) + + +def test_option_explicit_name_carries_a_refused_declaration(runner): + """An explicit name reaches a declaration the transform cannot name.""" + seen = [] + + def record(ctx, param, value): + seen.append(value) + + @click.command() + @click.option("--0foo", "zero_foo", expose_value=False, callback=record) + def cmd(**kwargs): + click.echo(repr(kwargs)) + + assert cmd.params[0].name == "zero_foo" + + result = runner.invoke(cmd, ["--0foo", "value"]) + assert not result.exception + assert result.output == "{}\n" + assert seen == ["value"] + + +def test_option_name_may_be_a_python_keyword(runner): + """``str.isidentifier()`` accepts a keyword, so the check lets one through.""" + + @click.command() + @click.option("--from") + def cmd(**kwargs): + click.echo(repr(kwargs)) + + assert cmd.params[0].name == "from" + + result = runner.invoke(cmd, ["--from", "here"]) + assert not result.exception + assert result.output == "{'from': 'here'}\n" + + def test_flag_duplicate_names(runner): with pytest.raises(ValueError, match="cannot use the same flag for true/false"): click.Option(["--foo/--foo"], default=False)