Merge pull request #895 from bloomberg/feature_strBool

Accept bool strings for ArgBool

Author: Dieterbe

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, ok := strToBool(got.str); ok {
+				*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, ok := strToBool(got.str); ok {
+				*v.val = val
+				break
+			}
 		}
-		*v.val = got.bool
+		return ErrBadKwarg{key, exp, got.etype}
 	default:
 		return fmt.Errorf("unsupported type %T for consumeKwarg", exp)
 	}

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, ok := strToBool(e); ok {
+		// '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) {
 

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`,

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{