Fix: required as default, and not required (#7)

* fix edgecase with required in the string

* so much more complex than I was hoping

Author: syntaqx

unmarshal.go

@@ -92,22 +92,27 @@ func parseTag(tag string) tagOptions {
 	keys := strings.Split(parts[0], "|")
 	var fallbackValue string
 	required := false
+
 	if len(parts) > 1 {
 		extraParts := parts[1]
-		if strings.Contains(extraParts, "default=[") || strings.Contains(extraParts, "fallback=[") {
-			re := regexp.MustCompile(`(?:default|fallback)=\[(.*?)\]`)
-			matches := re.FindStringSubmatch(extraParts)
-			if len(matches) > 1 {
-				fallbackValue = matches[1]
-			}
-		} else if strings.Contains(extraParts, "default=") || strings.Contains(extraParts, "fallback=") {
-			re := regexp.MustCompile(`(?:default|fallback)=([^,]+)`)
-			matches := re.FindStringSubmatch(extraParts)
-			if len(matches) > 1 {
-				fallbackValue = matches[1]
+		inBrackets := false
+		start := 0
+		for i := 0; i < len(extraParts); i++ {
+			switch extraParts[i] {
+			case '[':
+				inBrackets = true
+			case ']':
+				inBrackets = false
+			case ',':
+				if !inBrackets {
+					part := extraParts[start:i]
+					start = i + 1
+					parsePart(part, &fallbackValue, &required)
+				}
 			}
 		}
-		required = strings.Contains(extraParts, "required")
+		part := extraParts[start:]
+		parsePart(part, &fallbackValue, &required)
 	}
 
 	return tagOptions{
@@ -117,6 +122,24 @@ func parseTag(tag string) tagOptions {
 	}
 }
 
+func parsePart(part string, fallbackValue *string, required *bool) {
+	if strings.Contains(part, "default=[") || strings.Contains(part, "fallback=[") {
+		re := regexp.MustCompile(`(?:default|fallback)=\[(.*?)]`)
+		matches := re.FindStringSubmatch(part)
+		if len(matches) > 1 {
+			*fallbackValue = matches[1]
+		}
+	} else if strings.Contains(part, "default=") || strings.Contains(part, "fallback=") {
+		re := regexp.MustCompile(`(?:default|fallback)=([^,]+)`)
+		matches := re.FindStringSubmatch(part)
+		if len(matches) > 1 {
+			*fallbackValue = matches[1]
+		}
+	} else if strings.TrimSpace(part) == "required" {
+		*required = true
+	}
+}
+
 // setField sets the value of a struct field based on its type
 func setField(field reflect.Value, value string) error {
 	switch field.Kind() {

unmarshal_test.go

@@ -399,3 +399,70 @@ func TestUnmarshalSliceFloatError(t *testing.T) {
 	err := Unmarshal(&cfg)
 	assertError(t, err, "Unmarshal SliceFloatError")
 }
+
+func TestParseTag(t *testing.T) {
+	type TestCase struct {
+		Tag          string
+		ExpectedOpts tagOptions
+	}
+
+	testCases := []TestCase{
+		{
+			Tag: "NOT_REQUIRED,default=required",
+			ExpectedOpts: tagOptions{
+				keys:     []string{"NOT_REQUIRED"},
+				fallback: "required",
+				required: false,
+			},
+		},
+		{
+			Tag: "REQUIRED,required",
+			ExpectedOpts: tagOptions{
+				keys:     []string{"REQUIRED"},
+				fallback: "",
+				required: true,
+			},
+		},
+		{
+			Tag: "REQUIRED_WITH_DEFAULT,default=default,required",
+			ExpectedOpts: tagOptions{
+				keys:     []string{"REQUIRED_WITH_DEFAULT"},
+				fallback: "default",
+				required: true,
+			},
+		},
+		{
+			Tag: "SINGLE_KEY,required,default=default",
+			ExpectedOpts: tagOptions{
+				keys:     []string{"SINGLE_KEY"},
+				fallback: "default",
+				required: true,
+			},
+		},
+		{
+			Tag: "MULTI_KEY1|MULTI_KEY2|MULTI_KEY3,required,default=default",
+			ExpectedOpts: tagOptions{
+				keys:     []string{"MULTI_KEY1", "MULTI_KEY2", "MULTI_KEY3"},
+				fallback: "default",
+				required: true,
+			},
+		},
+		{
+			Tag: "SQUARE_BRACKETS,default=[item1,item2,item3]",
+			ExpectedOpts: tagOptions{
+				keys:     []string{"SQUARE_BRACKETS"},
+				fallback: "item1,item2,item3",
+				required: false,
+			},
+		},
+	}
+
+	for _, tc := range testCases {
+		t.Run(tc.Tag, func(t *testing.T) {
+			opts := parseTag(tc.Tag)
+			if !reflect.DeepEqual(opts, tc.ExpectedOpts) {
+				t.Errorf("parseTag(%s) returned %+v, expected %+v", tc.Tag, opts, tc.ExpectedOpts)
+			}
+		})
+	}
+}