diff --git a/CHANGES.rst b/CHANGES.rst index 1b8af9a5f..fbb3254d3 100644 --- a/CHANGES.rst +++ b/CHANGES.rst @@ -1,5 +1,14 @@ .. currentmodule:: click +Version 8.2.2 +------------- + +Unreleased + +- Fix reconciliation of `default`, `flag_value` and `type` parameters for + flag options, as well as parsing and normalization of environment variables. + :issue:`2952` :pr:`2956` + Version 8.2.1 ------------- diff --git a/docs/options.md b/docs/options.md index c2dd2b692..48c35c801 100644 --- a/docs/options.md +++ b/docs/options.md @@ -368,6 +368,31 @@ If a list is passed to `envvar`, the first environment variable found is picked. ``` +Variable names are: + - [Case-insensitive on Windows but not on other platforms](https://github.com/python/cpython/blob/aa9eb5f757ceff461e6e996f12c89e5d9b583b01/Lib/os.py#L777-L789). + - Not stripped of whitespaces and should match the exact name provided to the `envvar` argument. + +For flag options, there is two concepts to consider: the activation of the flag driven by the environment variable, and the value of the flag if it is activated. + +The environment variable need to be interpreted, because values read from them are always strings. We need to transform these strings into boolean values that will determine if the flag is activated or not. + +Here are the rules used to parse environment variable values for flag options: + - `true`, `1`, `yes`, `on`, `t`, `y` are interpreted as activating the flag + - `false`, `0`, `no`, `off`, `f`, `n` are interpreted as deactivating the flag + - The presence of the environment variable without value is interpreted as deactivating the flag + - Empty strings are interpreted as deactivating the flag + - Values are case-insensitive, so the `True`, `TRUE`, `tRuE` strings are all activating the flag + - Values are stripped of leading and trailing whitespaces before being interpreted, so the `" True "` string is transformed to `"true"` and so activates the flag + - If the flag option has a `flag_value` argument, passing that value in the environment variable will activate the flag, in addition to all the cases described above + - Any other value is interpreted as deactivating the flag + +.. caution:: + For boolean flags with a pair of values, the only recognized environment variable is the one provided to the `envvar` argument. + + So an option defined as `--flag\--no-flag`, with a `envvar="FLAG"` parameter, there is no magical `NO_FLAG=` variable that is recognized. Only the `FLAG=` environment variable is recognized. + +Once the status of the flag has been determine to be activated or not, the `flag_value` is used as the value of the flag if it is activated. If the flag is not activated, the value of the flag is set to `None` by default. + ## Multiple Options from Environment Values As options can accept multiple values, pulling in such values from diff --git a/src/click/core.py b/src/click/core.py index f57ada62b..445460bc5 100644 --- a/src/click/core.py +++ b/src/click/core.py @@ -2554,7 +2554,6 @@ def __init__( if help: help = inspect.cleandoc(help) - default_is_missing = "default" not in attrs super().__init__( param_decls, type=type, multiple=multiple, deprecated=deprecated, **attrs ) @@ -2588,15 +2587,15 @@ def __init__( self._flag_needs_value = self.prompt is not None and not self.prompt_required if is_flag is None: + # Implicitly a flag because flag_value was set. if flag_value is not None: - # Implicitly a flag because flag_value was set. is_flag = True + # Not a flag, but when used as a flag it shows a prompt. elif self._flag_needs_value: - # Not a flag, but when used as a flag it shows a prompt. is_flag = False - else: - # Implicitly a flag because flag options were given. - is_flag = bool(self.secondary_opts) + # Implicitly a flag because flag options were given. + elif self.secondary_opts: + is_flag = True elif is_flag is False and not self._flag_needs_value: # Not a flag, and prompt is not enabled, can be used as a # flag if flag_value is set. @@ -2604,14 +2603,19 @@ def __init__( self.default: t.Any | t.Callable[[], t.Any] - if is_flag and default_is_missing and not self.required: - if multiple: - self.default = () - else: - self.default = False + if is_flag: + # Set missing default for flags if not explicitly required or prompted. + if self.default is None and not self.required and not self.prompt: + if multiple: + self.default = () + else: + self.default = False - if is_flag and flag_value is None: - flag_value = not self.default + if flag_value is None: + # A boolean flag presence in the command line is enough to set + # the value: to the default if it is not blank, or to True + # otherwise. + flag_value = self.default if self.default else True self.type: types.ParamType if is_flag and type is None: @@ -2619,8 +2623,10 @@ def __init__( # default. self.type = types.convert_type(None, flag_value) - self.is_flag: bool = is_flag - self.is_bool_flag: bool = is_flag and isinstance(self.type, types.BoolParamType) + self.is_flag: bool = bool(is_flag) + self.is_bool_flag: bool = bool( + is_flag and isinstance(self.type, types.BoolParamType) + ) self.flag_value: t.Any = flag_value # Counting @@ -2628,7 +2634,7 @@ def __init__( if count: if type is None: self.type = types.IntRange(min=0) - if default_is_missing: + if self.default is None: self.default = 0 self.allow_from_autoenv = allow_from_autoenv @@ -2918,8 +2924,8 @@ def get_default( # value as default. if self.is_flag and not self.is_bool_flag: for param in ctx.command.params: - if param.name == self.name and param.default: - return t.cast(Option, param).flag_value + if param.name == self.name and param.default is not None: + return t.cast(Option, param).default return None @@ -2959,11 +2965,21 @@ def prompt_for_value(self, ctx: Context) -> t.Any: ) def resolve_envvar_value(self, ctx: Context) -> str | None: + """Find which environment variable to read for this option and return + its value. + + Returns the value of the environment variable if it exists, or ``None`` + if it does not. + + .. caution:: + + The raw value extracted from the environment is not normalized and + is returned as-is. Any normalization or reconciation with the + option's type should happen later. + """ rv = super().resolve_envvar_value(ctx) if rv is not None: - if self.is_flag and self.flag_value: - return str(self.flag_value) return rv if ( @@ -2980,18 +2996,32 @@ def resolve_envvar_value(self, ctx: Context) -> str | None: return None def value_from_envvar(self, ctx: Context) -> t.Any | None: - rv: t.Any | None = self.resolve_envvar_value(ctx) + """Normalize the value from the environment variable, if it exists.""" + rv: str | None = self.resolve_envvar_value(ctx) if rv is None: return None + # Non-boolean flags are more liberal in what they accept. But a flag being a + # flag, its envvar value still needs to analyzed to determine if the flag is + # activated or not. + if self.is_flag and not self.is_bool_flag: + # If the flag_value is set and match the envvar value, return it + # directly. + if self.flag_value is not None and rv == self.flag_value: + return self.flag_value + # Analyze the envvar value as a boolean to know if the flag is + # activated or not. + return types.BoolParamType.str_to_bool(rv) + + # Split the envvar value if it is allowed to be repeated. value_depth = (self.nargs != 1) + bool(self.multiple) - if value_depth > 0: - rv = self.type.split_envvar_value(rv) - + multi_rv = self.type.split_envvar_value(rv) if self.multiple and self.nargs != 1: - rv = batch(rv, self.nargs) + multi_rv = batch(multi_rv, self.nargs) # type: ignore[assignment] + + return multi_rv return rv @@ -3011,6 +3041,17 @@ def consume_value( value = self.flag_value source = ParameterSource.COMMANDLINE + # A flag which is activated and has a flag_value set, should returns + # the latter, unless the value comes from the explicitly sets default. + elif ( + self.is_flag + and value is True + and not self.is_bool_flag + and self.flag_value is not None + and source is not ParameterSource.DEFAULT + ): + value = self.flag_value + elif ( self.multiple and value is not None diff --git a/src/click/types.py b/src/click/types.py index 684cb3b1e..e71c1c21e 100644 --- a/src/click/types.py +++ b/src/click/types.py @@ -661,23 +661,67 @@ def _clamp(self, bound: float, dir: t.Literal[1, -1], open: bool) -> float: class BoolParamType(ParamType): name = "boolean" - def convert( - self, value: t.Any, param: Parameter | None, ctx: Context | None - ) -> t.Any: - if value in {False, True}: - return bool(value) + bool_states: dict[str, bool] = { + "1": True, + "0": False, + "yes": True, + "no": False, + "true": True, + "false": False, + "on": True, + "off": False, + "t": True, + "f": False, + "y": True, + "n": False, + # Absence of value is considered False. + "": False, + } + """A mapping of string values to boolean states. + + Mapping is inspired by :py:attr:`configparser.ConfigParser.BOOLEAN_STATES` + and extends it. + + .. caution:: + String values are lower-cased, as the ``str_to_bool`` comparison function + below is case-insensitive. + + .. warning:: + The mapping is not exhaustive, and does not cover all possible boolean strings + representations. It will remains as it is to avoid endless bikeshedding. + + Future work my be considered to make this mapping user-configurable from public + API. + """ - norm = value.strip().lower() + @staticmethod + def str_to_bool(value: str | bool) -> bool | None: + """Convert a string to a boolean value. - if norm in {"1", "true", "t", "yes", "y", "on"}: - return True + If the value is already a boolean, it is returned as-is. If the value is a + string, it is stripped of whitespaces and lower-cased, then checked against + the known boolean states pre-defined in the `BoolParamType.bool_states` mapping + above. - if norm in {"0", "false", "f", "no", "n", "off"}: - return False + Returns `None` if the value does not match any known boolean state. + """ + if isinstance(value, bool): + return value + return BoolParamType.bool_states.get(value.strip().lower()) - self.fail( - _("{value!r} is not a valid boolean.").format(value=value), param, ctx - ) + def convert( + self, value: t.Any, param: Parameter | None, ctx: Context | None + ) -> bool: + normalized = self.str_to_bool(value) + if normalized is None: + self.fail( + _( + "{value!r} is not a valid boolean. Recognized values: {states}" + ).format(value=value, states=", ".join(sorted(self.bool_states))), + param, + ctx, + ) + return normalized def __repr__(self) -> str: return "BOOL" diff --git a/tests/test_arguments.py b/tests/test_arguments.py index 50db8e606..53ab8d181 100644 --- a/tests/test_arguments.py +++ b/tests/test_arguments.py @@ -198,19 +198,6 @@ def cmd(arg): assert result.return_value == expect -def test_envvar_flag_value(runner): - @click.command() - # is_flag is implicitly true - @click.option("--upper", flag_value="upper", envvar="UPPER") - def cmd(upper): - click.echo(upper) - return upper - - # For whatever value of the `env` variable, if it exists, the flag should be `upper` - result = runner.invoke(cmd, env={"UPPER": "whatever"}) - assert result.output.strip() == "upper" - - def test_nargs_envvar_only_if_values_empty(runner): @click.command() @click.argument("arg", envvar="X", nargs=-1) diff --git a/tests/test_basic.py b/tests/test_basic.py index b84ae73d6..d773c3714 100644 --- a/tests/test_basic.py +++ b/tests/test_basic.py @@ -222,17 +222,21 @@ def cli(on): assert result.return_value is expect -@pytest.mark.parametrize("default", [True, False]) -@pytest.mark.parametrize(("args", "expect"), [(["--f"], True), ([], False)]) +@pytest.mark.parametrize( + ("default", "args", "expect"), + ( + (True, ["--f"], True), + (True, [], True), + (False, ["--f"], True), + (False, [], False), + ), +) def test_boolean_flag(runner, default, args, expect): @click.command() @click.option("--f", is_flag=True, default=default) def cli(f): return f - if default: - expect = not expect - result = runner.invoke(cli, args, standalone_mode=False) assert result.return_value is expect diff --git a/tests/test_imports.py b/tests/test_imports.py index e5e5119f2..6b44c3989 100644 --- a/tests/test_imports.py +++ b/tests/test_imports.py @@ -27,29 +27,30 @@ def tracking_import(module, locals=None, globals=None, fromlist=None, ALLOWED_IMPORTS = { "__future__", - "weakref", - "os", - "struct", + "codecs", "collections", "collections.abc", - "sys", + "configparser", "contextlib", + "datetime", + "enum", + "errno", + "fcntl", "functools", - "stat", - "re", - "codecs", + "gettext", "inspect", - "itertools", "io", + "itertools", + "os", + "re", + "shutil", + "stat", + "struct", + "sys", "threading", - "errno", - "fcntl", - "datetime", - "enum", - "typing", "types", - "gettext", - "shutil", + "typing", + "weakref", } if WIN: diff --git a/tests/test_options.py b/tests/test_options.py index 5c3041828..f5017384d 100644 --- a/tests/test_options.py +++ b/tests/test_options.py @@ -247,15 +247,95 @@ def cmd(arg): assert result.output == "foo|bar\n" -def test_trailing_blanks_boolean_envvar(runner): +@pytest.mark.parametrize( + ("name", "value", "expected"), + ( + # Lower-cased variations of value. + ("SHOUT", "true", True), + ("SHOUT", "false", False), + # Title-cased variations of value. + ("SHOUT", "True", True), + ("SHOUT", "False", False), + # Upper-cased variations of value. + ("SHOUT", "TRUE", True), + ("SHOUT", "FALSE", False), + # Random-cased variations of value. + ("SHOUT", "TruE", True), + ("SHOUT", "truE", True), + ("SHOUT", "FaLsE", False), + ("SHOUT", "falsE", False), + # Extra spaces around the value. + ("SHOUT", "true ", True), + ("SHOUT", " true", True), + ("SHOUT", " true ", True), + ("SHOUT", "false ", False), + ("SHOUT", " false", False), + ("SHOUT", " false ", False), + # Integer variations. + ("SHOUT", "1", True), + ("SHOUT", "0", False), + # Alternative states. + ("SHOUT", "t", True), + ("SHOUT", "f", False), + ("SHOUT", "y", True), + ("SHOUT", "n", False), + ("SHOUT", "yes", True), + ("SHOUT", "no", False), + ("SHOUT", "on", True), + ("SHOUT", "off", False), + # Blank value variations. + ("SHOUT", None, False), + ("SHOUT", "", False), + ("SHOUT", " ", False), + ("SHOUT", " ", False), + # Variable names are not stripped of spaces and so don't match the + # flag, which then naturraly takes its default value. + ("SHOUT ", "True", False), + ("SHOUT ", "False", False), + (" SHOUT ", "True", False), + (" SHOUT ", "False", False), + # Same for random and reverse environment variable names: they are not + # recognized by the option. + ("RANDOM", "True", False), + ("NO_SHOUT", "True", False), + ("NO_SHOUT", "False", False), + ), +) +def test_boolean_envvar_normalization(runner, name, value, expected): @click.command() @click.option("--shout/--no-shout", envvar="SHOUT") def cli(shout): click.echo(f"shout: {shout!r}") - result = runner.invoke(cli, [], env={"SHOUT": " true "}) + result = runner.invoke(cli, [], env={name: value}) assert result.exit_code == 0 - assert result.output == "shout: True\n" + assert result.output == f"shout: {expected}\n" + + +@pytest.mark.parametrize( + ("name", "value"), + ( + ("SHOUT", "tr ue"), + ("SHOUT", "10"), + ("SHOUT", "01"), + ("SHOUT", "00"), + ("SHOUT", "11"), + ("SHOUT", "randomstring"), + ("SHOUT", "None"), + ), +) +def test_boolean_envvar_bad_values(runner, name, value): + @click.command() + @click.option("--shout/--no-shout", envvar="SHOUT") + def cli(shout): + click.echo(f"shout: {shout!r}") + + result = runner.invoke(cli, [], env={name: value}) + assert result.exit_code == 2 + assert ( + f"Invalid value for '--shout': {value!r} is not a valid boolean." + in result.output + ) def test_multiple_default_help(runner): @@ -1008,28 +1088,169 @@ def test_type_from_flag_value(): @pytest.mark.parametrize( - ("opts", "pass_flag", "expected"), + ("opt_params", "pass_flag", "expected"), [ - pytest.param({"type": bool}, False, "False"), - pytest.param({"type": bool}, True, "True"), - pytest.param({"type": bool, "default": True}, False, "True"), - pytest.param({"type": bool, "default": True}, True, "False"), - pytest.param({"type": click.BOOL}, False, "False"), - pytest.param({"type": click.BOOL}, True, "True"), - pytest.param({"type": click.BOOL, "default": True}, False, "True"), - pytest.param({"type": click.BOOL, "default": True}, True, "False"), - pytest.param({"type": str}, False, ""), - pytest.param({"type": str}, True, "True"), + # Effect of the presence/absence of flag depending on type + ({"type": bool}, False, False), + ({"type": bool}, True, True), + ({"type": click.BOOL}, False, False), + ({"type": click.BOOL}, True, True), + ({"type": str}, False, "False"), + ({"type": str}, True, "True"), + # Effects of default value + ({"type": bool, "default": True}, False, True), + ({"type": bool, "default": True}, True, True), + ({"type": bool, "default": False}, False, False), + ({"type": bool, "default": False}, True, True), + ({"type": bool, "default": None}, False, False), + ({"type": bool, "default": None}, True, True), + ({"type": click.BOOL, "default": True}, False, True), + ({"type": click.BOOL, "default": True}, True, True), + ({"type": click.BOOL, "default": False}, False, False), + ({"type": click.BOOL, "default": False}, True, True), + ({"type": click.BOOL, "default": None}, False, False), + ({"type": click.BOOL, "default": None}, True, True), + ({"type": str, "default": True}, False, "True"), + ({"type": str, "default": True}, True, "True"), + ({"type": str, "default": False}, False, "False"), + ({"type": str, "default": False}, True, "True"), + ({"type": str, "default": "foo"}, False, "foo"), + ({"type": str, "default": "foo"}, True, "foo"), + ({"type": str, "default": None}, False, "False"), + ({"type": str, "default": None}, True, "True"), + # Effects of flag_value + ({"type": bool, "flag_value": True}, False, False), + ({"type": bool, "flag_value": True}, True, True), + ({"type": bool, "flag_value": False}, False, False), + ({"type": bool, "flag_value": False}, True, False), + ({"type": bool, "flag_value": None}, False, False), + ({"type": bool, "flag_value": None}, True, True), + ({"type": click.BOOL, "flag_value": True}, False, False), + ({"type": click.BOOL, "flag_value": True}, True, True), + ({"type": click.BOOL, "flag_value": False}, False, False), + ({"type": click.BOOL, "flag_value": False}, True, False), + ({"type": click.BOOL, "flag_value": None}, False, False), + ({"type": click.BOOL, "flag_value": None}, True, True), + ({"type": str, "flag_value": True}, False, "False"), + ({"type": str, "flag_value": True}, True, "True"), + ({"type": str, "flag_value": False}, False, "False"), + ({"type": str, "flag_value": False}, True, "False"), + ({"type": str, "flag_value": "foo"}, False, "False"), + ({"type": str, "flag_value": "foo"}, True, "foo"), + ({"type": str, "flag_value": None}, False, "False"), + ({"type": str, "flag_value": None}, True, "True"), + # Not passing --foo returns the default value as-is + ({"type": bool, "default": True, "flag_value": True}, False, True), + ({"type": bool, "default": True, "flag_value": False}, False, True), + ({"type": bool, "default": False, "flag_value": True}, False, False), + ({"type": bool, "default": False, "flag_value": False}, False, False), + ({"type": bool, "default": None, "flag_value": True}, False, False), + ({"type": bool, "default": None, "flag_value": False}, False, False), + ({"type": str, "default": True, "flag_value": True}, False, "True"), + ({"type": str, "default": True, "flag_value": False}, False, "True"), + ({"type": str, "default": False, "flag_value": True}, False, "False"), + ({"type": str, "default": False, "flag_value": False}, False, "False"), + ({"type": str, "default": "foo", "flag_value": True}, False, "foo"), + ({"type": str, "default": "foo", "flag_value": False}, False, "foo"), + ({"type": str, "default": "foo", "flag_value": "bar"}, False, "foo"), + ({"type": str, "default": "foo", "flag_value": None}, False, "foo"), + ({"type": str, "default": None, "flag_value": True}, False, "False"), + ({"type": str, "default": None, "flag_value": False}, False, "False"), + # Passing --foo returns the explicitly set flag_value + ({"type": bool, "default": True, "flag_value": True}, True, True), + ({"type": bool, "default": True, "flag_value": False}, True, False), + ({"type": bool, "default": False, "flag_value": True}, True, True), + ({"type": bool, "default": False, "flag_value": False}, True, False), + ({"type": bool, "default": None, "flag_value": True}, True, True), + ({"type": bool, "default": None, "flag_value": False}, True, False), + ({"type": str, "default": True, "flag_value": True}, True, "True"), + ({"type": str, "default": True, "flag_value": False}, True, "False"), + ({"type": str, "default": False, "flag_value": True}, True, "True"), + ({"type": str, "default": False, "flag_value": False}, True, "False"), + ({"type": str, "default": "foo", "flag_value": True}, True, "True"), + ({"type": str, "default": "foo", "flag_value": False}, True, "False"), + ({"type": str, "default": "foo", "flag_value": "bar"}, True, "bar"), + ({"type": str, "default": "foo", "flag_value": None}, True, "foo"), + ({"type": str, "default": None, "flag_value": True}, True, "True"), + ({"type": str, "default": None, "flag_value": False}, True, "False"), ], ) -def test_flag_value_is_correctly_set(runner, opts, pass_flag, expected): +def test_flag_value_and_default(runner, opt_params, pass_flag, expected): + @click.command() + @click.option("--foo", is_flag=True, **opt_params) + def cmd(foo): + click.echo(repr(foo)) + + result = runner.invoke(cmd, ["--foo"] if pass_flag else []) + assert result.output == f"{expected!r}\n" + + +@pytest.mark.parametrize( + "opts", + [ + {"type": bool, "default": "foo"}, + {"type": bool, "flag_value": "foo"}, + {"type": click.BOOL, "default": "foo"}, + {"type": click.BOOL, "flag_value": "foo"}, + ], +) +def test_invalid_flag_definition(runner, opts): @click.command() @click.option("--foo", is_flag=True, **opts) def cmd(foo): click.echo(foo) - result = runner.invoke(cmd, ["--foo"] if pass_flag else []) - assert result.output == f"{expected}\n" + result = runner.invoke(cmd, ["--foo"]) + assert ( + "Error: Invalid value for '--foo': 'foo' is not a valid boolean" + in result.output + ) + + +@pytest.mark.parametrize( + ("flag_value", "envvar_value", "expected"), + [ + # Envvar is set to false, so the flag default value is returned. + ("bar", "False", "False"), + # Same as above with alternative states and blank values. + ("bar", "0", "False"), + ("bar", "f", "False"), + ("bar", "", "False"), + # Envvar is set to true, so the flag_value is returned. + ("bar", "True", "bar"), + ("bar", "1", "bar"), + ("bar", "t", "bar"), + # So far we have the same cases as the test_flag_value_and_default + # test case. Now instead of passing a boolean-like value, let's use + # the flag_value as the envvar value. + ("bar", "bar", "bar"), + # flag_value is expected to be the exact same in the envvar for the flag to be + # activated. + ("bar", " bar ", "False"), + ("bar", "BAR", "False"), + ("bar", "random", "False"), + ("bar", "bar random", "False"), + ("bar", "random bar", "False"), + ("BAR", "BAR", "BAR"), + ("BAR", "bar", "False"), + (" bar ", " bar ", " bar "), + (" bar ", "bar", "False"), + (" bar ", "BAR", "False"), + ], +) +def test_envvar_flag_value(runner, flag_value, envvar_value, expected): + """Ensure that flag_value is recognized by the envvar.""" + + @click.command() + @click.option( + "--upper", type=str, is_flag=True, flag_value=flag_value, envvar="UPPER" + ) + def cmd(upper): + click.echo(repr(upper)) + + result = runner.invoke(cmd, env={"UPPER": envvar_value}) + assert result.exit_code == 0 + assert result.output == f"{expected!r}\n" @pytest.mark.parametrize( @@ -1047,7 +1268,7 @@ def cmd(foo): pytest.param(Option(["-a"], flag_value=True), True, id="bool flag_value"), ], ) -def test_is_bool_flag_is_correctly_set(option, expected): +def test_bool_flag_auto_detection(option, expected): assert option.is_bool_flag is expected