[tarantool-patches] [PATCH 1/2] app: fix boolean handling in argparse module
Vladislav Shpilevoy
v.shpilevoy at tarantool.org
Thu Sep 19 02:00:30 MSK 2019
There was a complaint that tarantoolctl --show-system option is
very hard to use. It incorrectly parsed passed values, and
provided strange errors.
tarantoolctl cat --show-system true
Bad input for parameter "show-system". Expected boolean, got "true"
tarantoolctl cat --show-system 1
Bad input for parameter "show-system". Expected boolean, got "1"
tarantoolctl cat --show-system=true
Bad input for parameter "show-system". Expected boolean, got "true"
First of all, appeared that the complaining people didn't read
documentation in 'tarantoolctl --help'. It explicitly says, that
'--show-system' should go after a file name, and does not have a value.
Secondly, even having taken the documentation into account, the
errors indeed look ridiculous. 'Expected boolean, got "true"'
looks especially weird.
The problem appeared to be with argparse module, how it parses
boolean parameters, and how stores parameter values not specified
in a command line.
All parameters were parsed into a dictionary: parameter name ->
value. If a name is alone (no value), then it is boolean true.
Otherwise it was always a string value. An attempt to specify
an explicit parameter value 'true' led to storing string 'true' in
that dictionary.
Consequential check for boolean parameters was trivial:
type(value) == 'boolean', which was obviously wrong, and didn't
pass for 'true' string, but passed for an empty value.
Closes #4076
---
src/lua/argparse.lua | 18 ++++++++----
test/app/argparse.result | 60 +++++++++++++++++++++++++++++++++++---
test/app/argparse.test.lua | 20 +++++++++++++
3 files changed, 89 insertions(+), 9 deletions(-)
diff --git a/src/lua/argparse.lua b/src/lua/argparse.lua
index 84d9d4cd1..1471d5846 100644
--- a/src/lua/argparse.lua
+++ b/src/lua/argparse.lua
@@ -31,12 +31,20 @@ local function convert_parameter_simple(name, convert_from, convert_to)
end
return converted
elseif convert_to == 'boolean' then
- if type(convert_from) ~= 'boolean' then
- error(
- ('Bad input for parameter "%s". Expected boolean, got "%s"')
- :format(name, convert_from)
- )
+ if type(convert_from) == 'boolean' then
+ return convert_from
+ end
+ convert_from = string.lower(convert_from)
+ if convert_from == '0' or convert_from == 'false' then
+ return false
end
+ if convert_from == '1' or convert_from == 'true' then
+ return true
+ end
+ error(
+ ('Bad input for parameter "%s". Expected boolean, got "%s"')
+ :format(name, convert_from)
+ )
elseif convert_to == 'string' then
if type(convert_from) ~= 'string' then
error(
diff --git a/test/app/argparse.result b/test/app/argparse.result
index 739ce34f7..d5be7fe2d 100644
--- a/test/app/argparse.result
+++ b/test/app/argparse.result
@@ -1,4 +1,11 @@
-- internal argparse test
+test_run = require('test_run').new()
+---
+...
+test_run:cmd("push filter ".."'\\.lua.*:[0-9]+: ' to '.lua:<line>\"]: '")
+---
+- true
+...
argparse = require('internal.argparse').parse
---
...
@@ -92,7 +99,7 @@ argparse({'tarantoolctl', 'start', 'instance', '--start', 'lalochka'})
...
argparse({'tarantoolctl', 'start', 'instance', '--start', '--', 'lalochka'})
---
-- error: 'builtin/internal.argparse.lua:105: bad argument #5: ID not valid'
+- error: 'builtin/internal.argparse.lua:<line>"]: bad argument #5: ID not valid'
...
argparse({'tarantoolctl', 'start', 'instance', '--start', '-', 'lalochka'})
---
@@ -130,17 +137,17 @@ argparse({'--verh=42'}, {'verh'})
...
argparse({'--verh=42'}, {{'verh', 'boolean'}})
---
-- error: 'builtin/internal.argparse.lua:35: Bad input for parameter "verh". Expected
+- error: 'builtin/internal.argparse.lua:<line>"]: Bad input for parameter "verh". Expected
boolean, got "42"'
...
argparse({'--verh=42'}, {{'verh', 'boolean+'}})
---
-- error: 'builtin/internal.argparse.lua:35: Bad input for parameter "verh". Expected
+- error: 'builtin/internal.argparse.lua:<line>"]: Bad input for parameter "verh". Expected
boolean, got "42"'
...
argparse({'--verh=42'}, {'niz'})
---
-- error: 'builtin/internal.argparse.lua:142: unknown options: verh'
+- error: 'builtin/internal.argparse.lua:<line>"]: unknown options: verh'
...
argparse({'--super-option'})
---
@@ -156,3 +163,48 @@ argparse({'tarantoolctl', 'start', 'instance', '--start=lalochka', 'option', '-'
6: another option
start: lalochka
...
+--
+-- gh-4076: argparse incorrectly processed boolean parameters,
+-- that led to problems with tarantoolctl usage.
+--
+params = {}
+---
+...
+params[1] = {'flag1', 'boolean'}
+---
+...
+params[2] = {'flag2', 'boolean'}
+---
+...
+params[3] = {'flag3', 'boolean'}
+---
+...
+params[4] = {'flag4', 'boolean'}
+---
+...
+params[5] = {'flag5', 'boolean'}
+---
+...
+args = {'--flag1', 'true', '--flag2', '1', '--flag3', 'false', '--flag4', '0', '--flag5', 'TrUe'}
+---
+...
+argparse(args, params)
+---
+- flag4: false
+ flag1: true
+ flag5: true
+ flag2: true
+ flag3: false
+...
+args = {'--flag1', 'abc'}
+---
+...
+argparse(args, params)
+---
+- error: 'builtin/internal.argparse.lua:<line>"]: Bad input for parameter "flag1". Expected
+ boolean, got "abc"'
+...
+test_run:cmd("clear filter")
+---
+- true
+...
diff --git a/test/app/argparse.test.lua b/test/app/argparse.test.lua
index 78b24b962..9315451d3 100644
--- a/test/app/argparse.test.lua
+++ b/test/app/argparse.test.lua
@@ -1,4 +1,6 @@
-- internal argparse test
+test_run = require('test_run').new()
+test_run:cmd("push filter ".."'\\.lua.*:[0-9]+: ' to '.lua:<line>\"]: '")
argparse = require('internal.argparse').parse
@@ -28,3 +30,21 @@ argparse({'--verh=42'}, {{'verh', 'boolean+'}})
argparse({'--verh=42'}, {'niz'})
argparse({'--super-option'})
argparse({'tarantoolctl', 'start', 'instance', '--start=lalochka', 'option', '-', 'another option'})
+
+--
+-- gh-4076: argparse incorrectly processed boolean parameters,
+-- that led to problems with tarantoolctl usage.
+--
+params = {}
+params[1] = {'flag1', 'boolean'}
+params[2] = {'flag2', 'boolean'}
+params[3] = {'flag3', 'boolean'}
+params[4] = {'flag4', 'boolean'}
+params[5] = {'flag5', 'boolean'}
+args = {'--flag1', 'true', '--flag2', '1', '--flag3', 'false', '--flag4', '0', '--flag5', 'TrUe'}
+argparse(args, params)
+
+args = {'--flag1', 'abc'}
+argparse(args, params)
+
+test_run:cmd("clear filter")
--
2.20.1 (Apple Git-117)
More information about the Tarantool-patches
mailing list