markers: fix a bug when parsing expressions with commas present in values

Signed-off-by: Bryce Palmer <bpalmer@redhat.com>

Author: everettraven

pkg/analysis/helpers/markers/analyzer.go

@@ -19,6 +19,7 @@ import (
 	"go/ast"
 	"go/token"
 	"reflect"
+	"regexp"
 	"strings"
 
 	"golang.org/x/tools/go/analysis"
@@ -229,24 +230,24 @@ func extractKnownMarkerIDAndExpressions(id string, marker string) (string, map[s
 	return id, extractExpressions(strings.TrimPrefix(marker, id))
 }
 
-func extractExpressions(expressions string) map[string]string {
+var expressionRegex = regexp.MustCompile(`\w*=(?:'[^']*'|"(\\"|[^"])*"|[\w;\-"]+)`)
+
+func extractExpressions(expressionStr string) map[string]string {
 	expressionsMap := map[string]string{}
 
 	// Do some normalization work to ensure we can parse expressions in
 	// a standard way. Trim any lingering colons (:) and replace all ':='s with '='
-	expressions = strings.TrimPrefix(expressions, ":")
-	expressions = strings.ReplaceAll(expressions, ":=", "=")
-
-	// split expression string on commas (,) to handle multiple expressions
-	// in a single marker
-	chainedExpressions := strings.SplitSeq(expressions, ",")
-	for chainedExpression := range chainedExpressions {
-		exps := strings.SplitN(chainedExpression, "=", 2)
-		if len(exps) < 2 {
+	expressionStr = strings.TrimPrefix(expressionStr, ":")
+	expressionStr = strings.ReplaceAll(expressionStr, ":=", "=")
+
+	expressions := expressionRegex.FindAllString(expressionStr, -1)
+	for _, expression := range expressions {
+		key, value, ok := strings.Cut(expression, "=")
+		if !ok {
 			continue
 		}
 
-		expressionsMap[exps[0]] = exps[1]
+		expressionsMap[key] = value
 	}
 
 	return expressionsMap

pkg/analysis/helpers/markers/analyzer_test.go

@@ -69,14 +69,66 @@ func TestExtractMarkerIdAndExpressions(t *testing.T) {
 				"": "\"foo\"",
 			},
 		},
+		{
+			name:       "registered marker with expression with a comma in its value",
+			marker:     `kubebuilder:validation:XValidation:rule='self.map(a, a == "someValue")',message='must have field!'`,
+			expectedID: "kubebuilder:validation:XValidation",
+			expectedExpressions: map[string]string{
+				"rule":    `'self.map(a, a == "someValue")'`,
+				"message": "'must have field!'",
+			},
+		},
+		{
+			name:       "registered marker with expression with a comma in its value with double quotes",
+			marker:     `kubebuilder:validation:XValidation:rule="self.map(a, a == \"someValue\")",message="must have field!"`,
+			expectedID: "kubebuilder:validation:XValidation",
+			expectedExpressions: map[string]string{
+				"rule":    `"self.map(a, a == \"someValue\")"`,
+				"message": `"must have field!"`,
+			},
+		},
+		{
+			name:       "registered marker with expression ending in a valid double quote",
+			marker:     `kubebuilder:validation:Enum:=foo;bar;baz;""`,
+			expectedID: "kubebuilder:validation:Enum",
+			expectedExpressions: map[string]string{
+				"": `foo;bar;baz;""`,
+			},
+		},
+		{
+			name:       "registered marker with chained expressions without quotes",
+			marker:     `custom:marker:fruit=apple,color=blue,country=UK`,
+			expectedID: "custom:marker",
+			expectedExpressions: map[string]string{
+				"fruit":   "apple",
+				"color":   "blue",
+				"country": "UK",
+			},
+		},
+		{
+			name:       "registered marker with numeric value",
+			marker:     `kubebuilder:validation:Minimum=10`,
+			expectedID: "kubebuilder:validation:Minimum",
+			expectedExpressions: map[string]string{
+				"": "10",
+			},
+		},
+		{
+			name:       "registered marker with negative numeric value",
+			marker:     `kubebuilder:validation:Minimum=-10`,
+			expectedID: "kubebuilder:validation:Minimum",
+			expectedExpressions: map[string]string{
+				"": "-10",
+			},
+		},
 	}
 
 	for _, tc := range testcases {
 		t.Run(tc.name, func(t *testing.T) {
 			g := NewWithT(t)
 
 			reg := NewRegistry()
-			reg.Register("kubebuilder:object:root", "required", "kubebuilder:validation:XValidation")
+			reg.Register(tc.expectedID)
 
 			id, expressions := extractMarkerIDAndExpressions(reg, tc.marker)