query: refactor to reuse the split function for both query and token

5 files changed, 78 insertions, 100 deletions

diff --git a/query/lexer.go b/query/lexer.go
index 45f657df..77830a47 100644
--- a/query/lexer.go
+++ b/query/lexer.go
@@ -39,10 +39,10 @@ func newTokenKV(qualifier, value string) token {
 
 func newTokenKVV(qualifier, subQualifier, value string) token {
 	return token{
-		kind:      tokenKindKVV,
-		qualifier: qualifier,
+		kind:         tokenKindKVV,
+		qualifier:    qualifier,
 		subQualifier: subQualifier,
-		value:     value,
+		value:        value,
 	}
 }
 
@@ -56,73 +56,68 @@ func newTokenSearch(term string) token {
 // tokenize parse and break a input into tokens ready to be
 // interpreted later by a parser to get the semantic.
 func tokenize(query string) ([]token, error) {
-	fields, err := splitQuery(query)
+	fields, err := splitFunc(query, unicode.IsSpace)
 	if err != nil {
 		return nil, err
 	}
 
 	var tokens []token
 	for _, field := range fields {
-		// Split using ':' as separator, but separators inside '"' don't count.
-		quoted := false
-		split := strings.FieldsFunc(field, func(r rune) bool {
-			if r == '"' {
-				quoted = !quoted
-			}
-			return !quoted && r == ':'
-		})
-		if (strings.HasPrefix(field, ":")) {
-			split = append([]string{""}, split...)
-		}
-		if (strings.HasSuffix(field, ":")) {
-			split = append(split, "")
-		}
-		if (quoted) {
-			return nil, fmt.Errorf("can't tokenize \"%s\": unmatched quote", field)
+		chunks, err := splitFunc(field, func(r rune) bool { return r == ':' })
+		if err != nil {
+			return nil, err
 		}
 
-		// full text search
-		if len(split) == 1 {
-			tokens = append(tokens, newTokenSearch(removeQuote(field)))
-			continue
+		if strings.HasPrefix(field, ":") || strings.HasSuffix(field, ":") {
+			return nil, fmt.Errorf("empty qualifier or value")
 		}
 
-		if len(split) > 3 {
-			return nil, fmt.Errorf("can't tokenize \"%s\": too many separators", field)
+		// pre-process chunks
+		for i, chunk := range chunks {
+			if len(chunk) == 0 {
+				return nil, fmt.Errorf("empty qualifier or value")
+			}
+			chunks[i] = removeQuote(chunk)
 		}
 
-		if len(split[0]) == 0 {
-			return nil, fmt.Errorf("can't tokenize \"%s\": empty qualifier", field)
-		}
+		switch len(chunks) {
+		case 1: // full text search
+			tokens = append(tokens, newTokenSearch(chunks[0]))
 
-		if len(split) == 2 {
-			if len(split[1]) == 0 {
-				return nil, fmt.Errorf("empty value for qualifier \"%s\"", split[0])
-			}
+		case 2: // KV
+			tokens = append(tokens, newTokenKV(chunks[0], chunks[1]))
 
-			tokens = append(tokens, newTokenKV(split[0], removeQuote(split[1])))
-		} else {
-			if len(split[1]) == 0 {
-				return nil, fmt.Errorf("empty sub-qualifier for qualifier \"%s\"", split[0])
-			}
+		case 3: // KVV
+			tokens = append(tokens, newTokenKVV(chunks[0], chunks[1], chunks[2]))
 
-			if len(split[2]) == 0 {
-				return nil, fmt.Errorf("empty value for qualifier \"%s:%s\"", split[0], split[1])
-			}
-
-			tokens = append(tokens, newTokenKVV(split[0], removeQuote(split[1]), removeQuote(split[2])))
+		default:
+			return nil, fmt.Errorf("can't tokenize \"%s\": too many separators", field)
 		}
 	}
 	return tokens, nil
 }
 
-// split the query into chunks by splitting on whitespaces but respecting
+func removeQuote(field string) string {
+	runes := []rune(field)
+	if len(runes) >= 2 {
+		r1 := runes[0]
+		r2 := runes[len(runes)-1]
+
+		if r1 == r2 && isQuote(r1) {
+			return string(runes[1 : len(runes)-1])
+		}
+	}
+	return field
+}
+
+// split the input into chunks by splitting according to separatorFunc but respecting
 // quotes
-func splitQuery(query string) ([]string, error) {
+func splitFunc(input string, separatorFunc func(r rune) bool) ([]string, error) {
 	lastQuote := rune(0)
 	inQuote := false
 
-	isToken := func(r rune) bool {
+	// return true if it's part of a chunk, or false if it's a rune that delimit one, as determined by the separatorFunc.
+	isChunk := func(r rune) bool {
 		switch {
 		case !inQuote && isQuote(r):
 			lastQuote = r
@@ -135,19 +130,19 @@ func splitQuery(query string) ([]string, error) {
 		case inQuote:
 			return true
 		default:
-			return !unicode.IsSpace(r)
+			return !separatorFunc(r)
 		}
 	}
 
 	var result []string
-	var token strings.Builder
-	for _, r := range query {
-		if isToken(r) {
-			token.WriteRune(r)
+	var chunk strings.Builder
+	for _, r := range input {
+		if isChunk(r) {
+			chunk.WriteRune(r)
 		} else {
-			if token.Len() > 0 {
-				result = append(result, token.String())
-				token.Reset()
+			if chunk.Len() > 0 {
+				result = append(result, chunk.String())
+				chunk.Reset()
 			}
 		}
 	}
@@ -156,8 +151,8 @@ func splitQuery(query string) ([]string, error) {
 		return nil, fmt.Errorf("unmatched quote")
 	}
 
-	if token.Len() > 0 {
-		result = append(result, token.String())
+	if chunk.Len() > 0 {
+		result = append(result, chunk.String())
 	}
 
 	return result, nil
@@ -166,16 +161,3 @@ func splitQuery(query string) ([]string, error) {
 func isQuote(r rune) bool {
 	return r == '"' || r == '\''
 }
-
-func removeQuote(field string) string {
-	runes := []rune(field)
-	if len(runes) >= 2 {
-		r1 := runes[0]
-		r2 := runes[len(runes)-1]
-
-		if r1 == r2 && isQuote(r1) {
-			return string(runes[1 : len(runes)-1])
-		}
-	}
-	return field
-}
diff --git a/query/lexer_test.go b/query/lexer_test.go
index 4ffb35a0..6ef679d2 100644
--- a/query/lexer_test.go
+++ b/query/lexer_test.go
@@ -3,7 +3,7 @@ package query
 import (
 	"testing"
 
-	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
 )
 
 func TestTokenize(t *testing.T) {
@@ -37,7 +37,7 @@ func TestTokenize(t *testing.T) {
 		{`key:'value value`, nil},
 		{`key:value value'`, nil},
 
-		// sub-qualifier posive testing
+		// sub-qualifier positive testing
 		{`key:subkey:"value:value"`, []token{newTokenKVV("key", "subkey", "value:value")}},
 
 		// sub-qualifier negative testing
@@ -59,13 +59,15 @@ func TestTokenize(t *testing.T) {
 	}
 
 	for _, tc := range tests {
-		tokens, err := tokenize(tc.input)
-		if tc.tokens == nil {
-			assert.Error(t, err)
-			assert.Nil(t, tokens)
-		} else {
-			assert.NoError(t, err)
-			assert.Equal(t, tc.tokens, tokens)
-		}
+		t.Run(tc.input, func(t *testing.T) {
+			tokens, err := tokenize(tc.input)
+			if tc.tokens == nil {
+				require.Error(t, err)
+				require.Nil(t, tokens)
+			} else {
+				require.NoError(t, err)
+				require.Equal(t, tc.tokens, tokens)
+			}
+		})
 	}
 }
diff --git a/query/parser.go b/query/parser.go
index e820c629..6fd5cd74 100644
--- a/query/parser.go
+++ b/query/parser.go
@@ -71,13 +71,7 @@ func Parse(query string) (*Query, error) {
 		case tokenKindKVV:
 			switch t.qualifier {
 			case "metadata":
-				if len(t.subQualifier) == 0 {
-					return nil, fmt.Errorf("empty value for sub-qualifier \"metadata:%s\"", t.subQualifier)
-				}
-				var pair StringPair
-				pair.Key = t.subQualifier
-				pair.Value = t.value
-				q.Metadata = append(q.Metadata, pair)
+				q.Metadata = append(q.Metadata, StringPair{Key: t.subQualifier, Value: t.value})
 
 			default:
 				return nil, fmt.Errorf("unknown qualifier \"%s:%s\"", t.qualifier, t.subQualifier)
diff --git a/query/parser_test.go b/query/parser_test.go
index 6d91d6cc..cef01ffd 100644
--- a/query/parser_test.go
+++ b/query/parser_test.go
@@ -3,7 +3,7 @@ package query
 import (
 	"testing"
 
-	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
 
 	"github.com/MichaelMure/git-bug/bug"
 )
@@ -62,6 +62,11 @@ func TestParse(t *testing.T) {
 		}},
 		{"sort:unknown", nil},
 
+		// KVV
+		{`metadata:key:"https://www.example.com/"`, &Query{
+			Filters: Filters{Metadata: []StringPair{{"key", "https://www.example.com/"}}},
+		}},
+
 		// Search
 		{"search", &Query{
 			Search: []string{"search"},
@@ -84,28 +89,23 @@ func TestParse(t *testing.T) {
 				OrderDirection: OrderDescending,
 			},
 		},
-
-		// Metadata
-		{`metadata:key:"https://www.example.com/"`, &Query{
-			Filters: Filters{Metadata: []StringPair{{"key", "https://www.example.com/"}}},
-		}},
 	}
 
 	for _, tc := range tests {
 		t.Run(tc.input, func(t *testing.T) {
 			query, err := Parse(tc.input)
 			if tc.output == nil {
-				assert.Error(t, err)
-				assert.Nil(t, query)
+				require.Error(t, err)
+				require.Nil(t, query)
 			} else {
-				assert.NoError(t, err)
+				require.NoError(t, err)
 				if tc.output.OrderBy != 0 {
-					assert.Equal(t, tc.output.OrderBy, query.OrderBy)
+					require.Equal(t, tc.output.OrderBy, query.OrderBy)
 				}
 				if tc.output.OrderDirection != 0 {
-					assert.Equal(t, tc.output.OrderDirection, query.OrderDirection)
+					require.Equal(t, tc.output.OrderDirection, query.OrderDirection)
 				}
-				assert.Equal(t, tc.output.Filters, query.Filters)
+				require.Equal(t, tc.output.Filters, query.Filters)
 			}
 		})
 	}
diff --git a/query/query.go b/query/query.go
index 3a2321cf..cce61a54 100644
--- a/query/query.go
+++ b/query/query.go
@@ -23,9 +23,9 @@ func NewQuery() *Query {
 
 type Search []string
 
-// Used for key-value pairs when filtering based on metadata
+// StringPair is a key/value pair of strings
 type StringPair struct {
-	Key string
+	Key   string
 	Value string
 }