From ea5a765ca607ae10224a03a2bdb97fbb1d0c6c2d Mon Sep 17 00:00:00 2001 From: Sean Hanson Date: Fri, 20 Apr 2018 14:13:57 -0400 Subject: [PATCH 1/2] Support conversion of string to bool --- expr/expr.go | 26 ++++++++++++++++++++------ expr/parse.go | 25 +++++++++++++++++++------ expr/parse_test.go | 16 ++++++++++++++++ expr/plan_test.go | 29 +++++++++++++++++++++++++++++ 4 files changed, 84 insertions(+), 12 deletions(-) diff --git a/expr/expr.go b/expr/expr.go index a9e86845f8..25bd3e1d21 100644 --- a/expr/expr.go +++ b/expr/expr.go @@ -149,10 +149,17 @@ func (e expr) consumeBasicArg(pos int, exp Arg) (int, error) { } *v.val = re case ArgBool: - if got.etype != etBool { - return 0, ErrBadArgumentStr{"string", got.etype.String()} + if got.etype == etBool { + *v.val = got.bool + break + } + if got.etype == etString { + if val, wasBool := strToBool(got.str); wasBool { + *v.val = val + break + } } - *v.val = got.bool + return 0, ErrBadArgumentStr{"boolean", got.etype.String()} case ArgStringsOrInts: // consume all args (if any) in args that will yield a string or int for ; len(e.args) > pos && (e.args[pos].etype == etString || e.args[pos].etype == etInt); pos++ { @@ -271,10 +278,17 @@ func (e expr) consumeKwarg(key string, optArgs []Arg) error { } *v.val = got.str case ArgBool: - if got.etype != etBool { - return ErrBadKwarg{key, exp, got.etype} + if got.etype == etBool { + *v.val = got.bool + break + } + if got.etype == etString { + if val, wasBool := strToBool(got.str); wasBool { + *v.val = val + break + } } - *v.val = got.bool + return ErrBadKwarg{key, exp, got.etype} default: return fmt.Errorf("unsupported type %T for consumeKwarg", exp) } diff --git a/expr/parse.go b/expr/parse.go index d7e8d37a08..185beae1a2 100644 --- a/expr/parse.go +++ b/expr/parse.go @@ -112,12 +112,13 @@ func Parse(e string) (*expr, string, error) { return parseConst(e) } - if strings.HasPrefix(e, "True") || strings.HasPrefix(e, "true") { - return &expr{bool: true, str: e[:4], etype: etBool}, e[4:], nil - } - - if strings.HasPrefix(e, "False") || strings.HasPrefix(e, "false") { - return &expr{bool: false, str: e[:5], etype: etBool}, e[5:], nil + if val, wasBool := strToBool(e); wasBool { + // 'false' is 5 chars, 'true' is 4 + size := 5 + if val { + size = 4 + } + return &expr{bool: val, str: e[:size], etype: etBool}, e[size:], nil } if e[0] == '\'' || e[0] == '"' { @@ -151,6 +152,18 @@ func Parse(e string) (*expr, string, error) { return &expr{str: name, etype: etName}, e, nil } +func strToBool(val string) (bool, bool) { + if strings.HasPrefix(val, "True") || strings.HasPrefix(val, "true") { + return true, true + } + + if strings.HasPrefix(val, "False") || strings.HasPrefix(val, "false") { + return false, true + } + + return false, false +} + // caller must assure s starts with opening paren func parseArgList(e string) (string, []*expr, map[string]*expr, string, error) { diff --git a/expr/parse_test.go b/expr/parse_test.go index 33d4047434..5fa011218e 100644 --- a/expr/parse_test.go +++ b/expr/parse_test.go @@ -325,6 +325,22 @@ func TestParse(t *testing.T) { }, nil, }, + { + "func(metric, key2='true', key1='false')", + &expr{ + str: "func", + etype: etFunc, + args: []*expr{ + {str: "metric"}, + }, + namedArgs: map[string]*expr{ + "key2": {etype: etString, str: "true"}, + "key1": {etype: etString, str: "false"}, + }, + argsStr: "metric, key2='true', key1='false'", + }, + nil, + }, { `foo.{bar,baz}.qux`, diff --git a/expr/plan_test.go b/expr/plan_test.go index 2c03b2068d..6cd7f4e429 100644 --- a/expr/plan_test.go +++ b/expr/plan_test.go @@ -48,6 +48,20 @@ func TestArgs(t *testing.T) { }, nil, }, + { + "2 args normal, 2 optional by position (bools as strings)", + []*expr{ + {etype: etName, str: "foo.bar.*"}, + {etype: etString, str: "1hour"}, + {etype: etString, str: "sum"}, + {etype: etString, str: "false"}, + }, + nil, + []Req{ + NewReq("foo.bar.*", from, to, 0), + }, + nil, + }, { "2 args normal, 2 optional by key", []*expr{ @@ -63,6 +77,21 @@ func TestArgs(t *testing.T) { }, nil, }, + { + "2 args normal, 2 optional by key (bools as strings)", + []*expr{ + {etype: etName, str: "foo.bar.*"}, + {etype: etString, str: "1hour"}, + }, + map[string]*expr{ + "func": {etype: etString, str: "sum"}, + "alignToFrom": {etype: etString, str: "true"}, + }, + []Req{ + NewReq("foo.bar.*", from, to, 0), + }, + nil, + }, { "2 args normal, 1 by position, 1 by keyword", []*expr{ From 954fd2b7fe74acdd3740828ec5df82dacd9f3e9f Mon Sep 17 00:00:00 2001 From: Sean Hanson Date: Mon, 30 Apr 2018 13:57:21 -0400 Subject: [PATCH 2/2] use idiomatic name 'ok' for if --- expr/expr.go | 4 ++-- expr/parse.go | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/expr/expr.go b/expr/expr.go index 25bd3e1d21..ceae5af1aa 100644 --- a/expr/expr.go +++ b/expr/expr.go @@ -154,7 +154,7 @@ func (e expr) consumeBasicArg(pos int, exp Arg) (int, error) { break } if got.etype == etString { - if val, wasBool := strToBool(got.str); wasBool { + if val, ok := strToBool(got.str); ok { *v.val = val break } @@ -283,7 +283,7 @@ func (e expr) consumeKwarg(key string, optArgs []Arg) error { break } if got.etype == etString { - if val, wasBool := strToBool(got.str); wasBool { + if val, ok := strToBool(got.str); ok { *v.val = val break } diff --git a/expr/parse.go b/expr/parse.go index 185beae1a2..67e1f3484b 100644 --- a/expr/parse.go +++ b/expr/parse.go @@ -112,7 +112,7 @@ func Parse(e string) (*expr, string, error) { return parseConst(e) } - if val, wasBool := strToBool(e); wasBool { + if val, ok := strToBool(e); ok { // 'false' is 5 chars, 'true' is 4 size := 5 if val {