fix: Format []ByRegexOption correctly. (#118)

If there's a mix of mappings and singular elements, the mappings will
(maybe) be quoted correctly but the singular elements won't be, which
can result in a a different YAML flow sequence or even invalid YAML if
the singular element has the right special characters.

e.g. Before the changes to options.go, the test was generating a
different flow sequence with 3 elements instead of 2: `by_regex=[foo,
bar, \"\\\\b(\\\\d{2})/(\\\\d{2})/(\\\\d{4})\\\\b\":
\"${3}-${1}-${2}\"]"`

We now generate strings that include mapping braces (`{` and `}`, e.g.
`by_regex=['foo, bar', {'\b(\d{2})/(\d{2})/(\d{4})\b':
'${3}-${1}-${2}'}]`). That's less than ideal, but it's still better than
generating invalid YAML. I don't see any options in the YAML library to
turn those braces off :/ https://pkg.go.dev/gopkg.in/yaml.v3

Author: JeffFaer

keepsorted/options.go

@@ -40,6 +40,14 @@ type ByRegexOption struct {
 	Template *string
 }
 
+func (opt ByRegexOption) MarshalYAML() (any, error) {
+	if opt.Template == nil {
+		return opt.Pattern.String(), nil
+	}
+
+	return map[string]string{opt.Pattern.String(): *opt.Template}, nil
+}
+
 // SortOrder defines whether we sort in ascending or descending order.
 type SortOrder string
 
@@ -248,19 +256,18 @@ func formatValue(val reflect.Value) (string, error) {
 		return formatIntList(val.Interface().([]int)), nil
 	case reflect.TypeFor[[]ByRegexOption]():
 		opts := val.Interface().([]ByRegexOption)
-		vals := make([]string, 0, len(opts))
+		vals := make([]string, len(opts))
 		seenTemplate := false
-		for _, opt := range opts {
+		for i, opt := range opts {
 			if opt.Template != nil {
 				seenTemplate = true
-				vals = append(vals, fmt.Sprintf(`%q: %q`, opt.Pattern.String(), *opt.Template))
-				continue
+				break
 			}
-			vals = append(vals, opt.Pattern.String())
+			vals[i] = opt.Pattern.String()
 		}
 		if seenTemplate {
 			// always presented as a yaml sequence to preserve any `k:v` items
-			return fmt.Sprintf("[%s]", strings.Join(vals, ", ")), nil
+			return formatYAMLList(opts)
 		}
 		return formatList(vals)
 	case reflect.TypeFor[[]*regexp.Regexp]():
@@ -292,6 +299,10 @@ func formatList(vals []string) (string, error) {
 		return strings.Join(vals, ","), nil
 	}
 
+	return formatYAMLList(vals)
+}
+
+func formatYAMLList[T any](vals []T) (string, error) {
 	node := new(yaml.Node)
 	if err := node.Encode(vals); err != nil {
 		return "", fmt.Errorf("while converting list to YAML: %w", err)

keepsorted/options_test.go

@@ -231,8 +231,26 @@ func TestBlockOptions(t *testing.T) {
 				AllowYAMLLists: true,
 				ByRegex: []ByRegexOption{
 					{Pattern: regexp.MustCompile(`.*`)},
-					{Pattern: regexp.MustCompile(`\b(\d{2})/(\d{2})/(\d{4})\b`),
-						Template: &[]string{"${3}-${1}-${2}"}[0]},
+					{
+						Pattern:  regexp.MustCompile(`\b(\d{2})/(\d{2})/(\d{4})\b`),
+						Template: &[]string{"${3}-${1}-${2}"}[0],
+					},
+				},
+			},
+		},
+		{
+			name:           "RegexWithTemplateAndSingletonWithSpecialChars",
+			in:             `by_regex=['foo, bar', '\b(\d{2})/(\d{2})/(\d{4})\b': '${3}-${1}-${2}']`,
+			defaultOptions: blockOptions{AllowYAMLLists: true},
+
+			want: blockOptions{
+				AllowYAMLLists: true,
+				ByRegex: []ByRegexOption{
+					{Pattern: regexp.MustCompile(`foo, bar`)},
+					{
+						Pattern:  regexp.MustCompile(`\b(\d{2})/(\d{2})/(\d{4})\b`),
+						Template: &[]string{"${3}-${1}-${2}"}[0],
+					},
 				},
 			},
 		},
@@ -277,24 +295,24 @@ func TestBlockOptions(t *testing.T) {
 			got, warns := parseBlockOptions(tc.commentMarker, tc.in, tc.defaultOptions)
 			if err := errors.Join(warns...); err != nil {
 				if tc.wantErr == "" {
-					t.Errorf("parseBlockOptions(%q, %q) = %v", tc.commentMarker, tc.in, err)
+					t.Errorf("parseBlockOptions(%#v, %#v) = %v", tc.commentMarker, tc.in, err)
 				} else if !strings.Contains(err.Error(), tc.wantErr) {
-					t.Errorf("parseBlockOptions(%q, %q) = %v, expected to contain %q", tc.commentMarker, tc.in, err, tc.wantErr)
+					t.Errorf("parseBlockOptions(%#v, %#v) = %v, expected to contain %q", tc.commentMarker, tc.in, err, tc.wantErr)
 				}
 			}
 			if diff := cmp.Diff(tc.want, got, cmp.AllowUnexported(blockOptions{}), cmpRegexp); diff != "" {
-				t.Errorf("parseBlockOptions(%q, %q) mismatch (-want +got):\n%s", tc.commentMarker, tc.in, diff)
+				t.Errorf("parseBlockOptions(%#v, %#v) mismatch (-want +got):\n%s", tc.commentMarker, tc.in, diff)
 			}
 
 			if tc.wantErr == "" {
 				t.Run("StringRoundtrip", func(t *testing.T) {
 					s := got.String()
 					got2, warns := parseBlockOptions(tc.commentMarker, s, tc.defaultOptions)
 					if err := errors.Join(warns...); err != nil {
-						t.Errorf("parseBlockOptions(%q, %q) = %v", tc.commentMarker, s, err)
+						t.Errorf("parseBlockOptions(%#v, %#v) = %v", tc.commentMarker, s, err)
 					}
 					if diff := cmp.Diff(got, got2, cmp.AllowUnexported(blockOptions{}), cmpRegexp); diff != "" {
-						t.Errorf("parseBlockOptions(%q, %q) mismatch (-want +got):\n%s", tc.commentMarker, s, diff)
+						t.Errorf("parseBlockOptions(%#v, %#v) mismatch (-want +got):\n%s", tc.commentMarker, s, diff)
 					}
 				})
 			}