addressing some preliminary review feedback

data/sqlutil/macros.go

@@ -142,6 +142,9 @@ func getMacroMatches(input string, name string) ([]macroMatch, error) {
 	for _, window := range rgx.FindAllStringIndex(input, -1) {
 		start, end := window[0], window[1]
 		args, length := parseArgs(input[end:])
+		if length < 0 {
+			return nil, fmt.Errorf("failed to parse macro arguments (missing close bracket?)")
+		}
 		matches = append(matches, macroMatch{full: input[start : end+length], args: args})
 	}
 	return matches, nil
@@ -184,9 +187,9 @@ func parseArgs(argString string) ([]string, int) {
 		}
 		arg = append(arg, r)
 	}
-	// FIXME: We only get here if we don't see a matching bracket
-	// before the next arg or the end of the string, which is a kind
-	// of syntax error. How should this be handled?
+	// This means we did not see a closing bracket, which we
+	// will consider an error. Formerly this would lead to
+	// a panic.
 	return nil, -1
 }
 

data/sqlutil/macros_test.go

@@ -5,7 +5,6 @@ import (
 	"testing"
 
 	"github.com/stretchr/testify/assert"
-	"github.com/stretchr/testify/require"
 )
 
 func staticMacro(r string) func(*Query, []string) (string, error) {
@@ -35,20 +34,21 @@ var macros = Macros{
 		}
 		return r
 	}),
+	"big":    staticMacro("10000"),
+	"medium": staticMacro("100"),
+	"little": staticMacro("10"),
 	// overwrite a default macro
 	"timeGroup": staticMacro("grouped!"),
-	"big":       staticMacro("10000"),
-	"medium":    staticMacro("100"),
-	"little":    staticMacro("10"),
 }
 
 func TestInterpolate(t *testing.T) {
 	tableName := "my_table"
 	tableColumn := "my_col"
 	type test struct {
-		name   string
-		input  string
-		output string
+		name      string
+		input     string
+		output    string
+		wantError bool
 	}
 	tests := []test{
 		{
@@ -176,11 +176,16 @@ func TestInterpolate(t *testing.T) {
 			output: "select * from foo where bar_FUNC(foo, bar)",
 			name:   "function in macro with multiple parameters",
 		},
-		{ // FIXME: this test is nondeterministic
+		{
 			input:  "select * from foo where ( date <= $__toTime and date >= $__fromTime ) limit 100",
 			output: "select * from foo where ( date <= f(1) and date >= f(0) ) limit 100",
 			name:   "stop on space outside of parens (see https://github.com/grafana/sqlds/pull/83)",
 		},
+		{
+			input:     "select * from foo where ( $__multiParams(100,400",
+			name:      "missing close parentheses is an error",
+			wantError: true,
+		},
 		{
 			input:  "select * from foo where ( $__big*($__little+$__medium) > 1000000) limit 100",
 			output: "select * from foo where ( 10000*(10+100) > 1000000) limit 100",
@@ -195,8 +200,10 @@ func TestInterpolate(t *testing.T) {
 				Column: tableColumn,
 			}
 			interpolatedQuery, err := Interpolate(query, macros)
-			require.Nil(t, err)
-			assert.Equal(t, tc.output, interpolatedQuery)
+			assert.Equal(t, err != nil, tc.wantError)
+			if !tc.wantError {
+				assert.Equal(t, tc.output, interpolatedQuery)
+			}
 		})
 	}
 }