diff --git a/CHANGES.md b/CHANGES.md index 7bdf90595..93327b7c4 100644 --- a/CHANGES.md +++ b/CHANGES.md @@ -2,6 +2,14 @@ Unreleased +- {class}`Argument` derives its name the same way {class}`Option` does, and must be + a valid Python identifier, else raises `TypeError`. {pr}`3827` +- `expose_value=False` no longer excuses that check on either kind. Pass an + explicit name to {class}`Option`, or rename an {class}`Argument` and pass + `metavar` to keep its display. {pr}`3827` +- Neither kind builds a parameter without a declaration. `click.argument()` and + `click.option()` with none and `expose_value=False` used to name a parameter + `""`, which showed nothing on the usage line. {pr}`3827` - Fix `copy.deepcopy()` and `pickle` on a `Parameter`, `Option` or `Command`. {pr}`3805` - A `KeyboardInterrupt` arriving while `Command.main()` reports an abort or an error, or while it exits, no longer escapes as an unhandled traceback. The command still diff --git a/docs/arguments.md b/docs/arguments.md index 14840e96c..e63774707 100644 --- a/docs/arguments.md +++ b/docs/arguments.md @@ -50,6 +50,80 @@ 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. + +```{eval-rst} +.. list-table:: Examples + :widths: 15 15 + :header-rows: 1 + + * - Decorator Arguments + - Inferred Argument Name + * - ``"foo-bar"`` + - foo_bar + * - ``"x"`` + - x + * - ``"CamelCase"`` + - camelcase + * - ``"Foo_Bar"`` + - foo_bar + * - ``"café"`` + - café + * - ``"ΟΔΟΣ"`` + - οδος + * - ``"\N{KELVIN SIGN}"`` + - k + * - ``"foo-٣"`` + - foo_٣ + * - ``"0-file"`` + - :exc:`TypeError` + * - ``"٣foo"`` + - :exc:`TypeError` + * - ``"foo.bar"`` + - :exc:`TypeError` + * - ``"foo\N{NON-BREAKING HYPHEN}bar"`` + - :exc:`TypeError` + * - ``"a\N{ZERO WIDTH SPACE}b"`` + - :exc:`TypeError` + * - ``""`` + - :exc:`TypeError` + * - ``"foo", "bar"`` + - :exc:`TypeError` +``` + +The name must satisfy {meth}`str.isidentifier`. Options apply the same check, +and the {ref}`caution about reserved keywords ` applies here +too. + +`expose_value=False` is no exception. The name is also the key the parser stores +the value under, so an argument that gave it up would share that key with the +next one. Rename the declaration and pass `metavar` to keep the old display: +`click.argument("zero_file", expose_value=False, metavar="0-FILE")`. + +(unicode-names)= + +```{caution} +Only the ASCII hyphen is replaced, so a separator that merely looks like one is +refused, as is any character that renders nothing. + +The identifier set itself moves with the Unicode table Python ships: the +zero-width joiner (`U+200D`) entered it in Python 3.13, so a declaration holding +one is refused up to Python 3.12 and names a parameter from 3.13 on. Prefer a +declaration that is already a lower-case identifier with `-` for `_`. +``` + +One difference from {ref}`option names ` remains. An option takes +several declarations, so one that is already an identifier is read as an +explicit name and kept as written. An argument takes exactly one, which serves +as both the name and the metavar, so it is always transformed. + ```{admonition} Note on Required Arguments :class: note diff --git a/docs/options.md b/docs/options.md index df78abb24..1da6b8266 100644 --- a/docs/options.md +++ b/docs/options.md @@ -68,10 +68,6 @@ follows: 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 `_`. - ```{eval-rst} .. list-table:: Examples :widths: 15 15 @@ -87,12 +83,60 @@ characters are replaced with `_`. - dest * - ``"--CamelCase"`` - camelcase + * - ``"-f", "--filename", "Dest"`` + - Dest * - ``"-f", "-fb"`` - f * - ``"--f", "--foo-bar"`` - f * - ``"---f"`` - _f + * - ``"--0-file"`` + - :exc:`TypeError` + * - ``"--foo.bar"`` + - :exc:`TypeError` +``` + +The name must satisfy {meth}`str.isidentifier`. {ref}`Arguments +` derive their name the same way and apply the same check, +including the {ref}`caution about Unicode declarations `. + +(keyword-names)= + +```{caution} +A [reserved keyword](https://docs.python.org/3/reference/lexical_analysis.html#keywords) +satisfies that check, so Click accepts one: `click.option("--from")` names its +parameter `from`. Three things follow, and Click reports none of them. + +- The callback cannot declare it. `def cmd(from)` is a {exc}`SyntaxError`, so + the command has to take `**kwargs` instead. +- That `**kwargs` then covers every other parameter too, and Python stops + checking the callback signature. An option you rename or drop used to raise + `TypeError: got an unexpected keyword argument`, and is now absorbed in + silence. +- {meth}`Context.invoke` cannot name it either. Write + `ctx.invoke(other, **{"from": value})`, because `ctx.invoke(other, from=value)` + is a {exc}`SyntaxError`. {meth}`Context.forward` is unaffected, since it + unpacks {attr}`Context.params`. + +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. Soft keywords (`match`, `case`, `type`, `_`) are contextual +and name a parameter fine, and `--True`, `--False` and `--None` lower case out +of the keyword set. +``` + +`expose_value=False` is no exception. The name is also the parser dest the value +is stored under, so two options that gave it up would share that dest and each +read the other's value. Pass an explicit name instead: +`click.option("--0-file", "zero_file", expose_value=False)`. + +```{caution} +Transformation from option name to argument name is not reversible. And is many-to-one: several option names can map to the same argument name. + +For example, `--foo-bar`, `--Foo-Bar` and `--FOO-BAR` all map to `foo_bar`. + +This is allowed so that options can deliberately form a [feature switch group](#feature-switch-group). ``` ## Basic Example @@ -509,6 +553,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/src/click/core.py b/src/click/core.py index 18a43ac5f..e796b68a0 100644 --- a/src/click/core.py +++ b/src/click/core.py @@ -2353,9 +2353,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 +2434,47 @@ 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 + + raise TypeError( + _( + "Could not determine name for {param_type} with declarations {decls!r}" + ).format(param_type=self.param_type_name, decls=decls) + ) + @property def human_readable_name(self) -> str: """Returns the human readable name of this parameter. This is the @@ -2929,6 +2965,11 @@ class Option(Parameter): :param hidden: hide this option from help outputs. :param attrs: Other command arguments described in :class:`Parameter`. + .. versionchanged:: 8.5.1 + ``expose_value=False`` no longer excuses a declaration that names no + Python identifier. Pass an explicit name, such as + ``click.option("--0-file", "zero_file", expose_value=False)``. + .. 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 +3048,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 +3299,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 +3335,9 @@ 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 = self._name_from_spec(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) - ) + name = self._resolve_name(name, decls) if not opts and not secondary_opts: raise TypeError( @@ -3708,6 +3737,13 @@ class Argument(Parameter): :param help: the help string. + .. versionchanged:: 8.5.1 + 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. This aligns with + option's behavior. Pass ``metavar`` to render a display the + declaration can no longer carry. + .. versionchanged:: 8.5.0 Added the ``help`` parameter. """ @@ -3778,22 +3814,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/tests/test_arguments.py b/tests/test_arguments.py index fea48ccba..511e2abbb 100644 --- a/tests/test_arguments.py +++ b/tests/test_arguments.py @@ -1,4 +1,6 @@ +import itertools import sys +import unicodedata from unittest import mock import pytest @@ -6,6 +8,11 @@ import click from click._utils import UNSET +# See the note beside the same constant in `test_options.py`: `os.environ` +# upper-cases its keys on Windows, so a variable answers to every spelling of +# its name there and to exactly one everywhere else. +ENV_NAMES_ARE_CASE_INSENSITIVE = sys.platform == "win32" + def test_nargs_star(runner): @click.command() @@ -79,6 +86,379 @@ 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(): + """An argument declaration is normalized even when already an identifier. + + This is the one way the two kinds still differ. An option takes several + declarations, so one of them that is already an identifier is read as an + explicit name and kept verbatim. An argument takes exactly one, which has + to serve as both the metavar source and the name, so it is always + transformed. + """ + assert click.Argument(["Foo_Bar"]).name == "foo_bar" + assert click.Option(["--x", "Foo_Bar"]).name == "Foo_Bar" + + +# Each kind, paired with the shape its declaration takes. An option carries the +# ``--`` prefix that its transform strips; an argument takes its declaration +# whole. A test parametrized over this asserts one rule for both kinds. +PARAM_KINDS = [ + pytest.param(click.Argument, "{decl}", id="argument"), + pytest.param(click.Option, "--{decl}", id="option"), +] + +# Declarations that between them cover every shape the naming transform has to +# settle: empty, bare prefixes, a leading digit, a dot, a space, and the two +# forms that do name something. +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): + """No declaration builds a parameter whose name is not a Python identifier. + + ``_resolve_name`` is the one place a name is settled, and it refuses + everything else, so every reader downstream may treat ``Parameter.name`` as + a usable identifier. + """ + 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. + + It used to build a required positional named ``""``: nothing rendered for it + in the usage line, a missing value was reported as ``Missing argument ''``, + and a second one tripped the duplicate-name warning in + :meth:`Command.get_params`. + """ + 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(): + """An unexposed argument is held to the check too. + + The name is also the key the parser stores the value under, so an argument + that gave it up would share that key with the next one. The option half is + ``test_option_name_check_applies_when_not_exposed``. + """ + with pytest.raises(TypeError, match="Could not determine name"): + 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"), + [ + # Greek capital omega transforms to its lowercase form. + pytest.param("Ω", "ω", id="omega"), + # Latin capital I with dot above transforms to two code points: the + # transform grows the name. + pytest.param("İ", "i\N{COMBINING DOT ABOVE}", id="dotted-capital-i"), + # A trailing sigma transforms to its context-sensitive final form. + pytest.param("ΟΔΟΣ", "οδος", id="final-sigma"), + # Capital sharp s transforms to the letter whose upper case is "SS". + pytest.param("ẞ", "ß", id="capital-sharp-s"), + # The Kelvin sign transforms to a plain ASCII k. + pytest.param("\N{KELVIN SIGN}", "k", id="kelvin-sign"), + # A digit outside ASCII is kept wherever it sits but the leading one. + pytest.param("foo-٣", "foo_٣", id="arabic-indic-digit"), + ], +) +def test_parameter_name_unicode_case_transform(cls, form, decl, expect): + """``str.lower()`` is neither one-to-one nor length-preserving. + + Both kinds run the same transform, so every row holds for either. + """ + assert cls([form.format(decl=decl)]).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): + """Neither kind accepts a declaration that names no Python identifier. + + Both derive a name the same way and hold it to the same check, so a + declaration is refused whichever one it is written as. The one refused + shape an argument has no equivalent of is in + ``test_option_name_must_be_an_identifier``. + """ + with pytest.raises(TypeError, match="Could not determine name"): + 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): + """Two zero-width characters answer this check differently per Python. + + Unicode 15.1 added the joiner and the non-joiner to the characters an + identifier may continue with, and Python 3.13 is the first release to carry + that table. So one declaration is refused up to Python 3.12 and names a + parameter from 3.13 on, with nothing on screen to separate it from ``ab``. + """ + 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="Could not determine name"): + 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): + """Two declarations that differ can transform to one name, and that warns. + + An option pair transforming to one name stays silent, since options may share a + name on purpose to form a feature switch group. An argument sharing a name + only ever overwrites, so the check in ``Command.get_params`` fires. + """ + + @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): + """An argument transforming onto an option's name overwrites it, and warns. + + The argument is what the warning names, and the argument is what wins: + the option's value never reaches the callback. + """ + + @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..89c171622 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,155 @@ 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. + + ``TEST_Foo_Bar`` names no variable Click looks for. Windows finds it + anyway, because the name it does look for differs only by case. + """ + + @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. + + The reproducer of pallets/click#2483: an auto envvar is upper-cased whole, + so ``yo`` finds ``YO_FLAG`` and never ``yo_FLAG``. + """ + + @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): + """Two names differing only by case share one auto envvar. + + ``Parameter.name`` keeps the case of an identifier declaration, but the + auto envvar upper-cases the name, so ``foo_bar`` and ``Foo_Bar`` are both + read from ``TEST_FOO_BAR``. + """ + + @click.command() + @click.option("--foo-bar") + @click.option("--other", "Foo_Bar") + def cmd(**kwargs): + click.echo(repr(sorted(kwargs.items()))) + + result = runner.invoke( + cmd, [], auto_envvar_prefix="TEST", env={"TEST_FOO_BAR": "foo"} + ) + assert not result.exception + assert result.output == "[('Foo_Bar', 'foo'), ('foo_bar', 'foo')]\n" + + +def test_auto_envvar_upper_can_change_length(runner): + """Deriving the envvar is not the inverse of deriving the name. + + ``--ẞ`` transforms to the name ``ß``, whose upper case is the two + letters ``SS``, so the option reads ``TEST_SS`` and no envvar carries the + letter the declaration was written with. + """ + + @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. + + Unlike the auto one, which is upper-cased whole. Windows erases the + difference, since a name there answers to any spelling. + """ + + @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. + + The other half of pallets/click#2483: these two keep their mixed case + where the auto envvar beside them would not. + """ + + @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 +1424,20 @@ 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 is taken verbatim, case included. + (["--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 +1454,164 @@ 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. + + The Kelvin sign is a distinct code point from ASCII ``K``, so the parser + keeps both options apart while the parameters share the name ``k`` and the + last one wins. ``Command.get_params`` stays silent on purpose here, since + options may share a name to form a feature switch group. The transform is + what makes that opt-in reachable by accident. + """ + + @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. + + The help screen still advertises three options, so a reader has nothing to + tell them from three independent settings, and each writes the same value. + """ + + @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. + + 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.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. + + Every other one is swept for both kinds by + ``test_arguments.py::test_parameter_name_must_be_an_identifier``, which + builds each declaration in both forms. + """ + with pytest.raises(TypeError, match="Could not determine name"): + click.Option(["-0"]) + + +def test_option_prompt_needs_no_name_guard(): + """``prompt=True`` no longer has to check whether the option has a name. + + ``Option.__init__`` used to raise ``'name' is required with 'prompt=True'``, + which was reachable while an unexposed option could be named ``""``. Naming + refuses that declaration first, so the guard became unreachable and was + dropped. ``test_parameter_name_is_always_an_identifier`` holds the invariant + it relied on. + """ + with pytest.raises(TypeError, match="Could not determine name"): + click.Option(["--0-file"], expose_value=False, prompt=True) + + +def test_option_name_check_applies_when_not_exposed(): + """An unexposed option is held to the check too. + + The name is also the parser dest the value is stored under, so two options + that gave it up would share that dest and each read the other's value. The + argument half is + ``test_arguments.py::test_argument_name_check_applies_when_not_exposed``. + """ + with pytest.raises(TypeError, match="Could not determine name"): + 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. + + ``--0foo`` derives ``0foo``, which is refused, so the parameter is named + separately and the declaration is kept as written. + """ + 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. + + The parameter is then reachable through ``**kwargs`` alone, which is why + ``test_option_names`` declares ``--from`` with an explicit ``_from`` name. + """ + + @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)