From 091ab9d412e0c9410a48ef92539e23a01bd94950 Mon Sep 17 00:00:00 2001
From: Vladislav Shpilevoy <v.shpilevoy@tarantool.org>
Date: Thu, 19 Sep 2019 00:47:01 +0200
Subject: [PATCH] app: fix error messages for not specified parameters in
argparse
Argparse module stores unspecified parameter values as boolean
true. It led to a problem, that a command line '--value' with
'value' defined as a number or a string, showed a strange error
message:
Expected number/string, got "true"
Even though a user didn't pass any value. Now it shows 'nothing'
instead of '"true"'. That is clearer.
Follow up #4076
(cherry picked from commit c214d086f5d2519572ae05872a5e39796096315e)
---
src/lua/argparse.lua | 27 +++++++++---------
test/app/argparse.result | 4 +--
...h-4076-argparse-wrong-bool-handling.result | 28 ++++++++++++++++++-
...4076-argparse-wrong-bool-handling.test.lua | 12 ++++++++
4 files changed, 55 insertions(+), 16 deletions(-)
diff --git a/src/lua/argparse.lua b/src/lua/argparse.lua
index 1471d58460..8dae388b1e 100644
--- a/src/lua/argparse.lua
+++ b/src/lua/argparse.lua
@@ -20,14 +20,21 @@ local function result_set_add(t_out, key, val)
end
end
+local function err_bad_parameter_value(name, got, expected)
+ if type(got) ~= 'string' then
+ got = 'nothing'
+ else
+ got = string.format('"%s"', got)
+ end
+ error(string.format('Bad value for parameter "%s". Expected %s, got %s',
+ name, expected, got))
+end
+
local function convert_parameter_simple(name, convert_from, convert_to)
if convert_to == 'number' then
local converted = tonumber(convert_from)
if converted == nil then
- error(
- ('Bad value for parameter %s. expected type %s, got "%s"')
- :format(name, convert_to, convert_from)
- )
+ return err_bad_parameter_value(name, convert_from, convert_to)
end
return converted
elseif convert_to == 'boolean' then
@@ -41,20 +48,14 @@ local function convert_parameter_simple(name, convert_from, convert_to)
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)
- )
+ return err_bad_parameter_value(name, convert_from, convert_to)
elseif convert_to == 'string' then
if type(convert_from) ~= 'string' then
- error(
- ('Bad input for parameter "%s". Expected string, got "%s"')
- :format(name, convert_from)
- )
+ return err_bad_parameter_value(name, convert_from, convert_to)
end
else
error(
- ('Bad convertion format "%s" provided for %s')
+ ('Bad conversion format "%s" provided for %s')
:format(convert_to, name)
)
end
diff --git a/test/app/argparse.result b/test/app/argparse.result
index d57693bc26..04f0439996 100644
--- a/test/app/argparse.result
+++ b/test/app/argparse.result
@@ -137,12 +137,12 @@ argparse({'--verh=42'}, {'verh'})
...
argparse({'--verh=42'}, {{'verh', 'boolean'}})
---
-- error: 'builtin/internal.argparse.lua:<line>"]: Bad input for parameter "verh". Expected
+- error: 'builtin/internal.argparse.lua:<line>"]: Bad value for parameter "verh". Expected
boolean, got "42"'
...
argparse({'--verh=42'}, {{'verh', 'boolean+'}})
---
-- error: 'builtin/internal.argparse.lua:<line>"]: Bad input for parameter "verh". Expected
+- error: 'builtin/internal.argparse.lua:<line>"]: Bad value for parameter "verh". Expected
boolean, got "42"'
...
argparse({'--verh=42'}, {'niz'})
diff --git a/test/app/gh-4076-argparse-wrong-bool-handling.result b/test/app/gh-4076-argparse-wrong-bool-handling.result
index 91895b9ed9..fd933a37dc 100644
--- a/test/app/gh-4076-argparse-wrong-bool-handling.result
+++ b/test/app/gh-4076-argparse-wrong-bool-handling.result
@@ -48,10 +48,36 @@ args = {'--flag1', 'abc'}
| ...
argparse(args, params)
| ---
- | - error: 'builtin/internal.argparse.lua:<line>"]: Bad input for parameter "flag1". Expected
+ | - error: 'builtin/internal.argparse.lua:<line>"]: Bad value for parameter "flag1". Expected
| boolean, got "abc"'
| ...
+--
+-- When parameter value was omitted, it was replaced internally
+-- with boolean true, and sometimes was showed in error messages.
+-- Now it is 'nothing'.
+--
+params = {}
+ | ---
+ | ...
+params[1] = {'value', 'number'}
+ | ---
+ | ...
+argparse({'--value'}, params)
+ | ---
+ | - error: 'builtin/internal.argparse.lua:<line>"]: Bad value for parameter "value". Expected
+ | number, got nothing'
+ | ...
+
+params[1][2] = 'string'
+ | ---
+ | ...
+argparse({'--value'}, params)
+ | ---
+ | - error: 'builtin/internal.argparse.lua:<line>"]: Bad value for parameter "value". Expected
+ | string, got nothing'
+ | ...
+
test_run:cmd("clear filter")
| ---
| - true
diff --git a/test/app/gh-4076-argparse-wrong-bool-handling.test.lua b/test/app/gh-4076-argparse-wrong-bool-handling.test.lua
index abd6c6927a..a8ee5e6703 100644
--- a/test/app/gh-4076-argparse-wrong-bool-handling.test.lua
+++ b/test/app/gh-4076-argparse-wrong-bool-handling.test.lua
@@ -17,4 +17,16 @@ argparse(args, params)
args = {'--flag1', 'abc'}
argparse(args, params)
+--
+-- When parameter value was omitted, it was replaced internally
+-- with boolean true, and sometimes was showed in error messages.
+-- Now it is 'nothing'.
+--
+params = {}
+params[1] = {'value', 'number'}
+argparse({'--value'}, params)
+
+params[1][2] = 'string'
+argparse({'--value'}, params)
+
test_run:cmd("clear filter")
--
GitLab