Repository: pallets/click (python)

Issue:
Can't mix nargs=-1 with envvar
`@click.argument(name, nargs=2, envvar=NAME)`

Works as expected. If you set `NAME="a b"` and run your command `name` in the called function will be a tuple of `('a', 'b')`. 

`@click.argument(name, nargs=-1, envvar=NAME)`

on the other hand doesn't work as expected. If you call it in the same exact way, `name` will be set to the empty tuple.

Pull request description:
nargs=-1 works with envvar 
This PR aims to fix two bugs:

1. Initially combining `nargs=-1` and `envvar` would yield an empty tuple where the expected behaviour of `nargs=-1` is to take on an unlimited number of arguments passed in `envvar`. @amy-lei and I added a fix so that now combining `nargs=-1` with `envvar` should output all the expected values from envvar rather than an empty tuple.

2. We also added error catching for nargs > len(envvar) where we would raise: `Got unexpected extra arguments` and when nargs < len(envvar) we would raise: `argument [argname] takes [nargs] values`.

The changelog is also updated with our new changes. Let us know what you think @davidism!

fixes #729

Patch:
diff --git a/CHANGES.rst b/CHANGES.rst
index adb5805..d7a780d 100644
--- a/CHANGES.rst
+++ b/CHANGES.rst
@@ -31,6 +31,8 @@ Unreleased
 -   The ``BOOL`` type accepts the values "on" and "off". :issue:`1629`
 -   A ``Group`` with ``invoke_without_command=True`` will always invoke
     its result callback. :issue:`1178`
+-   ``nargs == -1`` and ``nargs > 1`` is parsed and validated for
+    values from environment variables and defaults. :issue:`729`
 
 
 Version 7.1.2
diff --git a/src/click/core.py b/src/click/core.py
index 9651deb..346350b 100644
--- a/src/click/core.py
+++ b/src/click/core.py
@@ -1624,12 +1624,26 @@ class Parameter:
 
         if value is None and not ctx.resilient_parsing:
             value = self.get_default(ctx)
+
             if value is not None:
                 ctx.set_parameter_source(self.name, ParameterSource.DEFAULT)
 
         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 self.nargs != len(value)
+            and isinstance(value, (tuple, list))
+        ):
+            were = "was" if len(value) == 1 else "were"
+            ctx.fail(
+                f"Argument {self.name!r} takes {self.nargs} values but"
+                f" {len(value)} {were} given."
+            )
+
         return value
 
     def resolve_envvar_value(self, ctx):
diff --git a/src/click/parser.py b/src/click/parser.py
index 158abb0..b2ed7ad 100644
--- a/src/click/parser.py
+++ b/src/click/parser.py
@@ -183,6 +183,10 @@ class Argument:
                 raise BadArgumentUsage(
                     f"argument {self.dest} takes {self.nargs} values"
                 )
+
+        if self.nargs == -1 and self.obj.envvar is not None:
+            value = None
+
         state.opts[self.dest] = value
         state.order.append(self.obj)
 


Test patch:
diff --git a/tests/test_arguments.py b/tests/test_arguments.py
index c5fee42..445b880 100644
--- a/tests/test_arguments.py
+++ b/tests/test_arguments.py
@@ -18,7 +18,7 @@ def test_nargs_star(runner):
     assert result.output.splitlines() == ["src=foo.txt|bar.txt", "dst=dir"]
 
 
-def test_nargs_default(runner):
+def test_argument_unbounded_nargs_cant_have_default(runner):
     with pytest.raises(TypeError, match="nargs=-1"):
 
         @click.command()
@@ -163,26 +163,31 @@ def test_stdout_default(runner):
     assert result.output == "Foo bar baz\n"
 
 
-def test_nargs_envvar(runner):
-    @click.command()
-    @click.option("--arg", nargs=2)
-    def cmd(arg):
-        click.echo("|".join(arg))
-
-    result = runner.invoke(
-        cmd, [], auto_envvar_prefix="TEST", env={"TEST_ARG": "foo bar"}
-    )
-    assert not result.exception
-    assert result.output == "foo|bar\n"
+@pytest.mark.parametrize(
+    ("nargs", "value", "code", "output"),
+    [
+        (2, "", 2, "Argument 'arg' takes 2 values but 0 were given."),
+        (2, "a", 2, "Argument 'arg' takes 2 values but 1 was given."),
+        (2, "a b", 0, "len 2"),
+        (2, "a b c", 2, "Argument 'arg' takes 2 values but 3 were given."),
+        (-1, "a b c", 0, "len 3"),
+        (-1, "", 0, "len 0"),
+    ],
+)
+def test_nargs_envvar(runner, nargs, value, code, output):
+    if nargs == -1:
+        param = click.argument("arg", envvar="X", nargs=nargs)
+    else:
+        param = click.option("--arg", envvar="X", nargs=nargs)
 
     @click.command()
-    @click.option("--arg", envvar="X", nargs=2)
+    @param
     def cmd(arg):
-        click.echo("|".join(arg))
+        click.echo(f"len {len(arg)}")
 
-    result = runner.invoke(cmd, [], env={"X": "foo bar"})
-    assert not result.exception
-    assert result.output == "foo|bar\n"
+    result = runner.invoke(cmd, env={"X": value})
+    assert result.exit_code == code
+    assert output in result.output
 
 
 def test_empty_nargs(runner):
@@ -303,3 +308,23 @@ def test_multiple_param_decls_not_allowed(runner):
         @click.argument("x", click.Choice(["a", "b"]))
         def copy(x):
             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)}")
+
+    result = runner.invoke(cmd)
+    assert result.exit_code == code
+    assert output in result.output