Unbound splat arguments, fixes GH-54

amireh · amireh · commit 763834705725 · 2017-01-14T16:35:53.000+02:00
Unfortunately, this is a breaking change and requires a major release.
The patch makes it so that we no longer reject more than 999 repetitions
for a splat argument, and that by default we allow for unlimited
arguments (maxcount=0 or nil) instead of 1.
diff --git a/README.md b/README.md
@@ -106,6 +106,34 @@ Many thanks to everyone who reported bugs, provided fixes, and added entirely ne
 
 ## Changelog
 
+### Changes from 3.x to 4.0
+
+This release contains only a single change but since it breaks the API it is
+published as a major release.
+
+Splat arguments were reworked so that they allow for unlimited repititions
+**by default** (see [GH-54](https://github.com/amireh/lua_cliargs/issues/54)
+for more context.)
+
+Previously, if you were defining a splat argument _without_ specifying a 
+`maxcount` value (the 4th argument) the library would assume a maxcount of 1, 
+indicating that your splat argument is just an optional argument and will be 
+provided as a string value instead of a table.
+
+If you need to maintain this behavior, you must now explicitly set the maxcount
+to `1`:
+
+```lua
+-- version 3
+cli:splat('MY_SPLAT', 'Description')
+
+-- version 4
+cli:splat('MY_SPLAT', 'Description', nil, 1)
+```
+
+Also, the library internally had an arbitrary limit of 999 repetitions for the
+splat argument. That limit has been relieved.
+
 ### Changes from 2.5.x 3.0
 
 This major version release contains BREAKING API CHANGES. See the UPGRADE guide for help in updating your code to make use of it.
diff --git a/bin/coverage b/bin/coverage
@@ -24,7 +24,7 @@ if [ $? -ne 0 ]; then
   exit 1
 fi
 
+rm luacov.stats.out
 busted -c
 luacov src/
-rm luacov.stats.out
 grep -zPo "(?s)={10,}\nSummary\n={10,}.+" luacov.report.out
diff --git a/spec/core_spec.lua b/spec/core_spec.lua
@@ -97,7 +97,7 @@ describe("cliargs::core", function()
 
   describe('#redefine_default', function()
     it('allows me to change the default for an optargument', function()
-      cli:splat('ROOT', '...', 'foo')
+      cli:splat('ROOT', '...', 'foo', 1)
       assert.equal(cli:parse({}).ROOT, 'foo')
 
       cli:redefine_default('ROOT', 'bar')
diff --git a/spec/features/integration_spec.lua b/spec/features/integration_spec.lua
@@ -11,6 +11,68 @@ describe("integration: parsing", function()
     assert.are.same(helpers.parse(cli, ''), {})
   end)
 
+  describe('validating number of arguments', function()
+    context('when no arguments are defined', function()
+      it('raises nothing', function()
+        helpers.parse(cli, '')
+      end)
+    end)
+
+    context('with a required argument', function()
+      it('raises an error on extraneous arguments', function()
+        cli:argument('FOO', '...')
+
+        local _, err = helpers.parse(cli, 'foo bar')
+
+        assert.equal(err, 'bad number of arguments: expected exactly 1 argument not 2')
+      end)
+
+      it('raises an error on few arguments', function()
+        cli:argument('FOO', '...')
+
+        local _, err = helpers.parse(cli, '')
+
+        assert.equal(err, 'bad number of arguments: expected exactly 1 argument not 0')
+      end)
+    end)
+
+    context('with a splat with unlimited reptitions', function()
+      it('does not raise an error if nothing is passed in', function()
+        cli:splat('FOO', '...')
+
+        local _, err = helpers.parse(cli, '')
+
+        assert.equal(nil, err)
+      end)
+
+      it('does not raise an error if something was passed in', function()
+        cli:splat('FOO', '...')
+
+        local _, err = helpers.parse(cli, 'foo')
+
+        assert.equal(nil, err)
+      end)
+    end)
+
+    context('with a splat with bounded reptitions', function()
+      it('does not raise an error if passed count is within bounds', function()
+        cli:splat('FOO', '...', nil, 3)
+
+        local _, err = helpers.parse(cli, 'foo bar')
+
+        assert.equal(nil, err)
+      end)
+
+      it('raises an error if passed count is outside of bounds', function()
+        cli:splat('FOO', '...', nil, 3)
+
+        local _, err = helpers.parse(cli, 'foo bar bax hax')
+
+        assert.equal(err, 'bad number of arguments: expected 0-3 arguments not 4')
+      end)
+    end)
+  end)
+
   context('given a set of arguments', function()
     it('works when all are passed in', function()
       cli:argument('FOO', '...')
diff --git a/spec/features/splatarg_spec.lua b/spec/features/splatarg_spec.lua
@@ -33,6 +33,12 @@ describe("cliargs - splat arguments", function()
         cli:splat('SOME_SPLAT', 'some repeatable arg')
       end, 'Only one splat')
     end)
+
+    it('rejects repetition count less than 0', function()
+      assert.error_matches(function()
+        cli:splat('SOME_SPLAT', 'some repeatable arg', nil, -1)
+      end, 'Maxcount must be a number equal to or greater than 0')
+    end)
   end)
 
   describe('default value', function()
@@ -42,7 +48,7 @@ describe("cliargs - splat arguments", function()
 
     context('when only 1 occurrence is allowed', function()
       before_each(function()
-        cli:splat('SPLAT', 'some repeatable arg', 'foo')
+        cli:splat('SPLAT', 'some repeatable arg', 'foo', 1)
       end)
 
       it('uses the default value when nothing is passed in', function()
@@ -104,7 +110,7 @@ describe("cliargs - splat arguments", function()
     it('invokes the callback every time a value for the splat arg is parsed', function()
       local call_args = {}
 
-      cli:splat('SPLAT', 'foobar', nil, 2, function(_, value)
+      cli:splat('SPLAT', 'foobar', nil, nil, function(_, value)
         table.insert(call_args, value)
       end)
 
diff --git a/spec/printer_spec.lua b/spec/printer_spec.lua
@@ -76,6 +76,13 @@ describe('printer', function()
     it('prints a splat arg with reptitions > 2', function()
       cli:splat('OUTPUT', '...', nil, 5)
 
+      assert_msg [==[
+        Usage: [--] [OUTPUT-1 [OUTPUT-2 [... [OUTPUT-5]]]]
+      ]==]
+    end)
+    it('prints a splat arg with unlimited reptitions', function()
+      cli:splat('OUTPUT', '...', nil, 0)
+
       assert_msg [==[
         Usage: [--] [OUTPUT-1 [OUTPUT-2 [...]]]
       ]==]
diff --git a/src/cliargs/core.lua b/src/cliargs/core.lua
@@ -327,8 +327,12 @@ local function create_core()
   --- @param {*} [default=nil]
   ---        A default value.
   ---
-  --- @param {number} [maxcount=1]
+  --- @param {number} [maxcount=0]
   ---        The maximum number of occurences allowed.
+  ---        When set to 0 (the default), an unlimited amount of repetitions is
+  ---        allowed.
+  ---        When set to 1, the value of the splat argument will be provided as
+  ---        a primitive (a string) instead of a table.
   ---
   --- @param {function} [callback]
   ---        A function to call **everytime** a value for this argument is
@@ -347,10 +351,10 @@ local function create_core()
       "Default value must either be omitted or be a string"
     )
 
-    maxcount = tonumber(maxcount or 1)
+    maxcount = tonumber(maxcount or 0)
 
-    assert(maxcount > 0 and maxcount < 1000,
-      "Maxcount must be a number from 1 to 999"
+    assert(maxcount >= 0,
+      "Maxcount must be a number equal to or greater than 0"
     )
 
     assert(is_callable(callback) or callback == nil,
diff --git a/src/cliargs/parser.lua b/src/cliargs/parser.lua
@@ -215,28 +215,47 @@ end
 
 function p.validate(options, arg_count, done)
   local required = filter(options, 'type', K.TYPE_ARGUMENT)
-  local splatarg = filter(options, 'type', K.TYPE_SPLAT)[1] or { maxcount = 0 }
-
+  local splat = filter(options, 'type', K.TYPE_SPLAT)[1]
   local min_arg_count = #required
-  local max_arg_count = #required + splatarg.maxcount
-
-  -- missing any required arguments, or too many?
-  if arg_count < min_arg_count or arg_count > max_arg_count then
-    if splatarg.maxcount > 0 then
-      return nil, (
-        "bad number of arguments: " ..
-        min_arg_count .. "-" .. max_arg_count ..
-        " argument(s) must be specified, not " .. arg_count
-      )
+
+  local function get_range_description()
+    if splat and splat.maxcount == 0 then
+      return "at least " .. min_arg_count
+    elseif splat and splat.maxcount > 0 then
+      return min_arg_count .. "-" .. (#required + splat.maxcount)
     else
-      return nil, (
-        "bad number of arguments: " ..
-        min_arg_count .. " argument(s) must be specified, not " .. arg_count
-      )
+      return "exactly " .. min_arg_count
     end
   end
 
-  return done()
+  local function is_count_valid()
+    if splat and splat.maxcount == 0 then
+      return arg_count >= #required
+    elseif splat and splat.maxcount > 0 then
+      return arg_count >= #required and arg_count <= #required + splat.maxcount
+    else
+      return arg_count == #required
+    end
+  end
+
+  local function plural(word, count)
+    if count == 1 then
+      return word
+    else
+      return word .. 's'
+    end
+  end
+
+  if is_count_valid() then
+    return done()
+  else
+    return nil, (
+      "bad number of arguments: expected " ..
+      get_range_description() .. " " ..
+      plural('argument', #required + (splat and splat.maxcount or 0)) ..
+      " not " .. arg_count
+    )
+  end
 end
 
 function p.collect_results(cli_values, options)
@@ -268,7 +287,7 @@ function p.collect_results(cli_values, options)
     local maxcount = entry.maxcount
 
     if maxcount == nil then
-      maxcount = type(entry.default) == 'table' and 999 or 1
+      maxcount = type(entry.default) == 'table' and 0 or 1
     end
 
     local entry_value = entry_cli_values
diff --git a/src/cliargs/printer.lua b/src/cliargs/printer.lua
@@ -47,7 +47,7 @@ local function create_printer(get_parser_state)
 
     local required = filter(state.options, 'type', K.TYPE_ARGUMENT)
     local optional = filter(state.options, 'type', K.TYPE_OPTION)
-    local optargument = filter(state.options, 'type', K.TYPE_SPLAT)[1]
+    local splat = filter(state.options, 'type', K.TYPE_SPLAT)[1]
 
     if #state.name > 0 then
       msg = msg .. ' ' .. tostring(state.name)
@@ -57,7 +57,7 @@ local function create_printer(get_parser_state)
       msg = msg .. " [OPTIONS]"
     end
 
-    if #required > 0 or optargument then
+    if #required > 0 or splat then
       msg = msg .. " [--]"
     end
 
@@ -67,13 +67,15 @@ local function create_printer(get_parser_state)
       end
     end
 
-    if optargument then
-      if optargument.maxcount == 1 then
-        msg = msg .. " [" .. optargument.key .. "]"
-      elseif optargument.maxcount == 2 then
-        msg = msg .. " [" .. optargument.key .. "-1 [" .. optargument.key .. "-2]]"
-      elseif optargument.maxcount > 2 then
-        msg = msg .. " [" .. optargument.key .. "-1 [" .. optargument.key .. "-2 [...]]]"
+    if splat then
+      if splat.maxcount == 1 then
+        msg = msg .. " [" .. splat.key .. "]"
+      elseif splat.maxcount == 2 then
+        msg = msg .. " [" .. splat.key .. "-1 [" .. splat.key .. "-2]]"
+      elseif splat.maxcount > 0 then
+        msg = msg .. " [" .. splat.key .. "-1 [" .. splat.key .. "-2 [... [" .. splat.key .. "-" .. splat.maxcount .. "]]]]"
+      else
+        msg = msg .. " [" .. splat.key .. "-1 [" .. splat.key .. "-2 [...]]]"
       end
     end
 
