Merge pull request #152 from grafana/leizor/query-squad/issues/1833

Skip all whitespace during expr parsing

pkg/parser/parser.go

@@ -511,11 +511,19 @@ func (e *expr) insertFirstArg(exp *expr) error {
 	return nil
 }
 
-func parseExprWithoutPipe(e string) (Expr, string, error) {
-	// skip whitespace
-	for len(e) > 1 && e[0] == ' ' {
-		e = e[1:]
+func skipWhitespace(e string) string {
+	skipTo := len(e)
+	for i, r := range e {
+		if !unicode.IsSpace(r) {
+			skipTo = i
+			break
+		}
 	}
+	return e[skipTo:]
+}
+
+func parseExprWithoutPipe(e string) (Expr, string, error) {
+	e = skipWhitespace(e)
 
 	if e == "" {
 		return nil, "", ErrMissingExpr
@@ -583,9 +591,7 @@ func ParseExpr(e string) (Expr, string, error) {
 }
 
 func pipe(exp *expr, e string) (*expr, string, error) {
-	for len(e) > 1 && e[0] == ' ' {
-		e = e[1:]
-	}
+	e = skipWhitespace(e)
 
 	if e == "" || e[0] != '|' {
 		return exp, e, nil
@@ -645,7 +651,7 @@ func parseArgList(e string) (string, []*expr, map[string]*expr, string, error) {
 	e = e[1:]
 
 	// check for empty args
-	t := strings.TrimLeft(e, " ")
+	t := skipWhitespace(e)
 	if t != "" && t[0] == ')' {
 		return "", posArgs, namedArgs, t[1:], nil
 	}
@@ -718,9 +724,7 @@ func parseArgList(e string) (string, []*expr, map[string]*expr, string, error) {
 		}
 
 		// after the argument, trim any trailing spaces
-		for len(e) > 0 && e[0] == ' ' {
-			e = e[1:]
-		}
+		e = skipWhitespace(e)
 
 		if e[0] == ')' {
 			return argStringBuffer.String(), posArgs, namedArgs, e[1:], nil
@@ -860,7 +864,6 @@ FOR:
 }
 
 func parseString(s string) (string, string, error) {
-
 	if s[0] != '\'' && s[0] != '"' {
 		panic("string should start with open quote")
 	}

pkg/parser/parser_test.go

@@ -6,6 +6,38 @@ import (
 	"github.com/stretchr/testify/assert"
 )
 
+func TestSkipWhitespace(t *testing.T) {
+	testCases := []struct{ in, expected string }{
+		{
+			in:       " ",
+			expected: "",
+		},
+		{
+			in:       " foo",
+			expected: "foo",
+		},
+		{
+			in:       " foo ",
+			expected: "foo ",
+		},
+		{
+			in:       "\nfoo",
+			expected: "foo",
+		},
+		{
+			in:       "\tfoo",
+			expected: "foo",
+		},
+	}
+
+	for _, tc := range testCases {
+		t.Run(tc.in, func(t *testing.T) {
+			out := skipWhitespace(tc.in)
+			assert.Equal(t, tc.expected, out)
+		})
+	}
+}
+
 func TestParseExpr(t *testing.T) {
 	tests := []struct {
 		s string
@@ -368,6 +400,92 @@ func TestParseExpr(t *testing.T) {
 		{"hello&world",
 			&expr{target: "hello&world"},
 		},
+		{
+			"foo.bar\n.baz\t",
+			&expr{
+				target: "foo.bar",
+				etype:  EtName,
+			},
+		},
+		{
+			"absolute( baz )\n",
+			&expr{
+				target: "absolute",
+				etype:  EtFunc,
+				args: []*expr{
+					{target: "baz"},
+				},
+				argString: " baz ",
+			},
+		},
+		{
+			"func1(\"example blah\")",
+			&expr{
+				target: "func1",
+				etype:  EtFunc,
+				args: []*expr{
+					{
+						etype:  EtString,
+						valStr: "example blah",
+					},
+				},
+				argString: "\"example blah\"",
+			},
+		},
+		{
+			"foobar(\n)",
+			&expr{
+				target: "foobar",
+				etype:  EtFunc,
+			},
+		},
+		{
+			"foobar(asdf,\n\tzxcv,\n\tqwer\n)",
+			&expr{
+				target: "foobar",
+				etype:  EtFunc,
+				args: []*expr{
+					{target: "asdf"},
+					{target: "zxcv"},
+					{target: "qwer"},
+				},
+				argString: "asdf,\n\tzxcv,\n\tqwer\n",
+			},
+		},
+		{
+			"func1(foo.bar)\n| func2(foo.baz)|\n func3(\n\tfunc4(asdf.zxcv.qwer)\n)",
+			&expr{
+				target: "func3",
+				etype:  EtFunc,
+				args: []*expr{
+					{
+						target: "func2",
+						etype:  EtFunc,
+						args: []*expr{
+							{
+								target: "func1",
+								etype:  EtFunc,
+								args: []*expr{
+									{target: "foo.bar"},
+								},
+								argString: "foo.bar",
+							},
+							{target: "foo.baz"},
+						},
+						argString: "func1(foo.bar),foo.baz",
+					},
+					{
+						target: "func4",
+						etype:  EtFunc,
+						args: []*expr{
+							{target: "asdf.zxcv.qwer"},
+						},
+						argString: "asdf.zxcv.qwer",
+					},
+				},
+				argString: "func2(func1(foo.bar),foo.baz),func4(asdf.zxcv.qwer)",
+			},
+		},
 	}
 
 	for _, tt := range tests {