Repository: pallets/click (python)

Issue:
Move iterable checks from type_cast_value to __init__
Both the errors raised in `type_cast_value` should be in `__init__` instead. The composite error should be removed from `type_cast_value`, it makes no sense at runtime. Both places that `value` is iterated in `type_cast_value` should raise a `BadParameter` error instead of `TypeError`.

_Originally posted by @davidism in https://github.com/pallets/click/issues/1805#issuecomment-791522850_

Pull request:
refactor type_cast_value
* Don't need recursion to validate `mutliple` and `nargs`.
* When validating value at runtime, only raise `BadParameter`, not `TypeError` or `ValueError`.
* Validate `default` value against `multiple` and `nargs` in `__init__`. Can't use `type_cast_value` on `default` as that can have side effects such as opening a file.
* For `multiple` and `nargs=-1`, validate that the value is iterable (and not a string). For `nargs>0` validate that the length matches. For `multiple` and `nargs>0`, validate the length of each item.
* Reword error messages for consistency.
* Reword error message about "arity" since that's a technical term. For `types.Tuple.convert`, raise a `BadParameter` error and match the argument count error that Python would raise.
* For ease of validation, needed to make `multiple` a parameter for `Parameter` instead of only `Option`. `Argument` checks that it's not given.

<!--
Link to relevant issues or previous PRs, one per line. Use "fixes" to
automatically close an issue.
-->

- fixes #1806 

<!--
Ensure each step in CONTRIBUTING.rst is complete by adding an "x" to
each box below.

If only docs were changed, these aren't relevant and can be removed.
-->

Checklist:

- [x] Add tests that demonstrate the correct behavior of the change. Tests should fail without the change.
- [x] Add or update relevant docs, in the docs folder and in code.
- [x] Add an entry in `CHANGES.rst` summarizing the change and linking to the issue.
- [x] Add `.. versionchanged::` entries in any relevant code docs.
- [x] Run `pre-commit` hooks and fix any issues.
- [x] Run `pytest` and `tox`, no tests failed.


Patch:
diff --git a/CHANGES.rst b/CHANGES.rst
index d532792..51f47dc 100644
--- a/CHANGES.rst
+++ b/CHANGES.rst
@@ -212,6 +212,9 @@ Unreleased
     :issue:`1101`
 -   Fix a bug of ``click.utils.make_default_short_help`` for which the
     returned string could be as long as ``max_width + 3``. :issue:`1849`
+-   When defining a parameter, ``default`` is validated with
+    ``multiple`` and ``nargs``. More validation is done for values being
+    processed as well. :issue:`1806`
 
 
 Version 7.1.2
diff --git a/src/click/core.py b/src/click/core.py
index e7e33f5..14ef872 100644
--- a/src/click/core.py
+++ b/src/click/core.py
@@ -5,6 +5,7 @@ import sys
 import typing as t
 from contextlib import contextmanager
 from contextlib import ExitStack
+from functools import partial
 from functools import update_wrapper
 from gettext import gettext as _
 from gettext import ngettext
@@ -1795,6 +1796,16 @@ class CommandCollection(MultiCommand):
         return sorted(rv)
 
 
+def _check_iter(value):
+    """Check if the value is iterable but not a string. Raises a type
+    error, or return an iterator over the value.
+    """
+    if isinstance(value, str):
+        raise TypeError
+
+    return iter(value)
+
+
 class Parameter:
     r"""A parameter to a command comes in two versions: they are either
     :class:`Option`\s or :class:`Argument`\s.  Other subclasses are currently
@@ -1879,6 +1890,7 @@ class Parameter:
         default=None,
         callback=None,
         nargs=None,
+        multiple=False,
         metavar=None,
         expose_value=True,
         is_eager=False,
@@ -1903,7 +1915,7 @@ class Parameter:
         self.required = required
         self.callback = callback
         self.nargs = nargs
-        self.multiple = False
+        self.multiple = multiple
         self.expose_value = expose_value
         self.default = default
         self.is_eager = is_eager
@@ -1939,6 +1951,47 @@ class Parameter:
 
         self._custom_shell_complete = shell_complete
 
+        if __debug__:
+            if self.type.is_composite and nargs != self.type.arity:
+                raise ValueError(
+                    f"'nargs' must be {self.type.arity} (or None) for"
+                    f" type {self.type!r}, but it was {nargs}."
+                )
+
+            # Skip no default or callable default.
+            check_default = default if not callable(default) else None
+
+            if check_default is not None:
+                if multiple:
+                    try:
+                        # Only check the first value against nargs.
+                        check_default = next(_check_iter(check_default), None)
+                    except TypeError:
+                        raise ValueError(
+                            "'default' must be a list when 'multiple' is true."
+                        ) from None
+
+                # Can be None for multiple with empty default.
+                if nargs != 1 and check_default is not None:
+                    try:
+                        _check_iter(check_default)
+                    except TypeError:
+                        if multiple:
+                            message = (
+                                "'default' must be a list of lists when 'multiple' is"
+                                " true and 'nargs' != 1."
+                            )
+                        else:
+                            message = "'default' must be a list when 'nargs' != 1."
+
+                        raise ValueError(message) from None
+
+                    if nargs > 1 and len(check_default) != nargs:
+                        subject = "item length" if multiple else "length"
+                        raise ValueError(
+                            f"'default' {subject} must match nargs={nargs}."
+                        )
+
     def to_info_dict(self):
         """Gather information that could be useful for a tool generating
         user-facing documentation.
@@ -2031,47 +2084,60 @@ class Parameter:
         return value, source
 
     def type_cast_value(self, ctx, value):
-        """Given a value this runs it properly through the type system.
-        This automatically handles things like `nargs` and `multiple` as
-        well as composite types.
+        """Convert and validate a value against the option's
+        :attr:`type`, :attr:`multiple`, and :attr:`nargs`.
         """
         if value is None:
             return () if self.multiple or self.nargs == -1 else None
 
-        if self.type.is_composite:
-            if self.nargs <= 1:
-                raise TypeError(
-                    "Attempted to invoke composite type but nargs has"
-                    f" been set to {self.nargs}. This is not supported;"
-                    " nargs needs to be set to a fixed value > 1."
-                )
-
-            if self.multiple:
-                return tuple(self.type(x, self, ctx) for x in value)
-
-            return self.type(value, self, ctx)
-
-        def _convert(value, level):
-            if level == 0:
-                return self.type(value, self, ctx)
-
+        def check_iter(value):
             try:
-                iter_value = iter(value)
+                return _check_iter(value)
             except TypeError:
-                raise TypeError(
-                    "Value for parameter with multiple = True or nargs > 1"
-                    " should be an iterable."
-                )
+                # This should only happen when passing in args manually,
+                # the parser should construct an iterable when parsing
+                # the command line.
+                raise BadParameter(
+                    _("Value must be an iterable."), ctx=ctx, param=self
+                ) from None
+
+        if self.nargs == 1 or self.type.is_composite:
+            convert = partial(self.type, param=self, ctx=ctx)
+        elif self.nargs == -1:
+
+            def convert(value):
+                return tuple(self.type(x, self, ctx) for x in check_iter(value))
+
+        else:  # nargs > 1
+
+            def convert(value):
+                value = tuple(check_iter(value))
+
+                if len(value) != self.nargs:
+                    raise BadParameter(
+                        ngettext(
+                            "Takes {nargs} values but 1 was given.",
+                            "Takes {nargs} values but {len} were given.",
+                            len(value),
+                        ).format(nargs=self.nargs, len=len(value)),
+                        ctx=ctx,
+                        param=self,
+                    )
+
+                return tuple(self.type(x, self, ctx) for x in value)
 
-            return tuple(_convert(x, level - 1) for x in iter_value)
+        if self.multiple:
+            return tuple(convert(x) for x in check_iter(value))
 
-        return _convert(value, (self.nargs != 1) + bool(self.multiple))
+        return convert(value)
 
     def value_is_missing(self, value):
         if value is None:
             return True
+
         if (self.nargs != 1 or self.multiple) and value == ():
             return True
+
         return False
 
     def process_value(self, ctx, value):
@@ -2081,25 +2147,6 @@ class Parameter:
         if self.required and self.value_is_missing(value):
             raise MissingParameter(ctx=ctx, param=self)
 
-        # For bounded nargs (!= -1), validate the number of values.
-        if (
-            not ctx.resilient_parsing
-            and self.nargs > 1
-            and isinstance(value, (tuple, list))
-            and (
-                any(len(v) != self.nargs for v in value)
-                if self.multiple
-                else len(value) != self.nargs
-            )
-        ):
-            ctx.fail(
-                ngettext(
-                    "Argument {name!r} takes {nargs} values but 1 was given.",
-                    "Argument {name!r} takes {nargs} values but {len} were given.",
-                    len(value),
-                ).format(name=self.name, nargs=self.nargs, len=len(value))
-            )
-
         if self.callback is not None:
             value = self.callback(ctx, self, value)
 
@@ -2250,7 +2297,7 @@ class Option(Parameter):
         **attrs,
     ):
         default_is_missing = attrs.get("default", _missing) is _missing
-        super().__init__(param_decls, type=type, **attrs)
+        super().__init__(param_decls, type=type, multiple=multiple, **attrs)
 
         if prompt is True:
             prompt_text = self.name.replace("_", " ").capitalize()
@@ -2307,32 +2354,33 @@ class Option(Parameter):
             if default_is_missing:
                 self.default = 0
 
-        self.multiple = multiple
         self.allow_from_autoenv = allow_from_autoenv
         self.help = help
         self.show_default = show_default
         self.show_choices = show_choices
         self.show_envvar = show_envvar
 
-        # Sanity check for stuff we don't support
         if __debug__:
-            if self.nargs < 0:
-                raise TypeError("Options cannot have nargs < 0")
+            if self.nargs == -1:
+                raise TypeError("nargs=-1 is not supported for options.")
+
             if self.prompt and self.is_flag and not self.is_bool_flag:
-                raise TypeError("Cannot prompt for flags that are not bools.")
+                raise TypeError("'prompt' is not valid for non-boolean flag.")
+
             if not self.is_bool_flag and self.secondary_opts:
-                raise TypeError("Got secondary option for non boolean flag.")
+                raise TypeError("Secondary flag is not valid for non-boolean flag.")
+
             if self.is_bool_flag and self.hide_input and self.prompt is not None:
-                raise TypeError("Hidden input does not work with boolean flag prompts.")
+                raise TypeError(
+                    "'prompt' with 'hide_input' is not valid for boolean flag."
+                )
+
             if self.count:
                 if self.multiple:
-                    raise TypeError(
-                        "Options cannot be multiple and count at the same time."
-                    )
-                elif self.is_flag:
-                    raise TypeError(
-                        "Options cannot be count and flags at the same time."
-                    )
+                    raise TypeError("'count' is not valid with 'multiple'.")
+
+                if self.is_flag:
+                    raise TypeError("'count' is not valid with 'is_flag'.")
 
     def to_info_dict(self):
         info_dict = super().to_info_dict()
@@ -2608,12 +2656,14 @@ class Argument(Parameter):
             else:
                 required = attrs.get("nargs", 1) > 0
 
+        if "multiple" in attrs:
+            raise TypeError("__init__() got an unexpected keyword argument 'multiple'.")
+
         super().__init__(param_decls, required=required, **attrs)
 
-        if self.default is not None and self.nargs < 0:
-            raise TypeError(
-                "nargs=-1 in combination with a default value is not supported."
-            )
+        if __debug__:
+            if self.default is not None and self.nargs == -1:
+                raise TypeError("'default' is not supported for nargs=-1.")
 
     @property
     def human_readable_name(self):
diff --git a/src/click/types.py b/src/click/types.py
index eebf830..0d21092 100644
--- a/src/click/types.py
+++ b/src/click/types.py
@@ -864,11 +864,20 @@ class Tuple(CompositeParamType):
         return len(self.types)
 
     def convert(self, value, param, ctx):
-        if len(value) != len(self.types):
-            raise TypeError(
-                "It would appear that nargs is set to conflict with the"
-                " composite type arity."
+        len_type = len(self.types)
+        len_value = len(value)
+
+        if len_value != len_type:
+            self.fail(
+                ngettext(
+                    "{len_type} values are required, but {len_value} was given.",
+                    "{len_type} values are required, but {len_value} were given.",
+                    len_value,
+                ).format(len_type=len_type, len_value=len_value),
+                param=param,
+                ctx=ctx,
             )
+
         return tuple(ty(x, param, ctx) for ty, x in zip(self.types, value))
 
 


Test patch:
diff --git a/tests/test_arguments.py b/tests/test_arguments.py
index e1a6de9..2045f6f 100644
--- a/tests/test_arguments.py
+++ b/tests/test_arguments.py
@@ -22,7 +22,7 @@ def test_argument_unbounded_nargs_cant_have_default(runner):
     with pytest.raises(TypeError, match="nargs=-1"):
 
         @click.command()
-        @click.argument("src", nargs=-1, default=42)
+        @click.argument("src", nargs=-1, default=["42"])
         def copy(src):
             pass
 
@@ -167,9 +167,9 @@ def test_stdout_default(runner):
     ("nargs", "value", "expect"),
     [
         (2, "", None),
-        (2, "a", "Argument 'arg' takes 2 values but 1 was given."),
+        (2, "a", "Takes 2 values but 1 was given."),
         (2, "a b", ("a", "b")),
-        (2, "a b c", "Argument 'arg' takes 2 values but 3 were given."),
+        (2, "a b c", "Takes 2 values but 3 were given."),
         (-1, "a b c", ("a", "b", "c")),
         (-1, "", ()),
     ],
@@ -188,7 +188,8 @@ def test_nargs_envvar(runner, nargs, value, expect):
     result = runner.invoke(cmd, env={"X": value}, standalone_mode=False)
 
     if isinstance(expect, str):
-        assert expect in str(result.exception)
+        assert isinstance(result.exception, click.BadParameter)
+        assert expect in result.exception.format_message()
     else:
         assert result.return_value == expect
 
@@ -313,24 +314,15 @@ def test_multiple_param_decls_not_allowed(runner):
             click.echo(x)
 
 
-@pytest.mark.parametrize(
-    ("value", "code", "output"),
-    [
-        ((), 2, "Argument 'arg' takes 2 values but 0 were given."),
-        (("a",), 2, "Argument 'arg' takes 2 values but 1 was given."),
-        (("a", "b"), 0, "len 2"),
-        (("a", "b", "c"), 2, "Argument 'arg' takes 2 values but 3 were given."),
-    ],
-)
-def test_nargs_default(runner, value, code, output):
-    @click.command()
-    @click.argument("arg", nargs=2, default=value)
-    def cmd(arg):
-        click.echo(f"len {len(arg)}")
+def test_multiple_not_allowed():
+    with pytest.raises(TypeError, match="multiple"):
+        click.Argument(["a"], multiple=True)
+
 
-    result = runner.invoke(cmd)
-    assert result.exit_code == code
-    assert output in result.output
+@pytest.mark.parametrize("value", [(), ("a",), ("a", "b", "c")])
+def test_nargs_bad_default(runner, value):
+    with pytest.raises(ValueError, match="nargs=2"):
+        click.Argument(["a"], nargs=2, default=value)
 
 
 def test_subcommand_help(runner):
diff --git a/tests/test_options.py b/tests/test_options.py
index 009f80e..94e5eb8 100644
--- a/tests/test_options.py
+++ b/tests/test_options.py
@@ -33,7 +33,7 @@ def test_invalid_option(runner):
 
 
 def test_invalid_nargs(runner):
-    with pytest.raises(TypeError, match="nargs < 0"):
+    with pytest.raises(TypeError, match="nargs=-1"):
 
         @click.command()
         @click.option("--foo", nargs=-1)
@@ -117,18 +117,39 @@ def test_multiple_required(runner):
     assert "Error: Missing option '-m' / '--message'." in result.output
 
 
-def test_multiple_bad_default(runner):
-    @click.command()
-    @click.option("--flags", multiple=True, default=False)
-    def cli(flags):
-        pass
+@pytest.mark.parametrize(
+    ("multiple", "nargs", "default"),
+    [
+        (True, 1, []),
+        (True, 1, [1]),
+        # (False, -1, []),
+        # (False, -1, [1]),
+        (False, 2, [1, 2]),
+        # (True, -1, [[]]),
+        # (True, -1, []),
+        # (True, -1, [[1]]),
+        (True, 2, []),
+        (True, 2, [[1, 2]]),
+    ],
+)
+def test_init_good_default_list(runner, multiple, nargs, default):
+    click.Option(["-a"], multiple=multiple, nargs=nargs, default=default)
 
-    result = runner.invoke(cli, [])
-    assert result.exception
-    assert (
-        "Value for parameter with multiple = True or nargs > 1 should be an iterable."
-        in result.exception.args
-    )
+
+@pytest.mark.parametrize(
+    ("multiple", "nargs", "default"),
+    [
+        (True, 1, 1),
+        # (False, -1, 1),
+        (False, 2, [1]),
+        (True, 2, [[1]]),
+    ],
+)
+def test_init_bad_default_list(runner, multiple, nargs, default):
+    type = (str, str) if nargs == 2 else None
+
+    with pytest.raises(ValueError, match="default"):
+        click.Option(["-a"], type=type, multiple=multiple, nargs=nargs, default=default)
 
 
 def test_empty_envvar(runner):