refactor(options): Define default options as an explicit instance. (#34)
This simplifies option parsing logic a bit, and sets us up to add a flag
to override the default options
(https://github.com/google/keep-sorted/pull/35).
I also tweaked most of the tests in keep_sorted_test to not rely on the
default options at all and instead be more explicit about the behavior
they need to pass.
diff --git a/keepsorted/block.go b/keepsorted/block.go
index 65e8716..c292c85 100644
--- a/keepsorted/block.go
+++ b/keepsorted/block.go
@@ -109,7 +109,7 @@
continue
}
- opts, err := f.parseBlockOptions(start.line)
+ opts, err := f.parseBlockOptions(start.line, defaultOptions)
if err != nil {
// TODO(b/250608236): Is there a better way to surface this error?
log.Err(fmt.Errorf("keep-sorted block at index %d had bad start directive: %w", start.index+offset, err)).Msg("")
diff --git a/keepsorted/keep_sorted_test.go b/keepsorted/keep_sorted_test.go
index 94a89e2..307030f 100644
--- a/keepsorted/keep_sorted_test.go
+++ b/keepsorted/keep_sorted_test.go
@@ -23,31 +23,6 @@
"github.com/rs/zerolog/log"
)
-var (
- defaultOptions = blockOptions{
- Lint: true,
- SkipLines: 0,
- Group: true,
- GroupPrefixes: nil,
- IgnorePrefixes: nil,
- Numeric: false,
- StickyComments: true,
- StickyPrefixes: map[string]bool{"//": true},
- RemoveDuplicates: true,
- PrefixOrder: nil,
- Block: false,
- NewlineSeparated: false,
- CaseSensitive: true,
- commentMarker: "//",
- }
-
- defaultMetadata = blockMetadata{
- startDirective: "keep-sorted-test start",
- endDirective: "keep-sorted-test end",
- opts: defaultOptions,
- }
-)
-
// initZerolog initializes zerolog to log as part of the test.
// It returns a function that restores zerolog to its state before this function was called.
func initZerolog(t testing.TB) {
@@ -56,16 +31,26 @@
t.Cleanup(func() { log.Logger = oldLogger })
}
-func defaultOptionsWith(f func(*blockOptions)) blockOptions {
+func defaultOptionsWithCommentMarker(marker string) blockOptions {
opts := defaultOptions
- f(&opts)
+ opts.StickyComments = true
+ opts.setCommentMarker(marker)
+ return opts
+}
+
+func optionsWithCommentMarker(marker string) blockOptions {
+ var opts blockOptions
+ opts.StickyComments = true
+ opts.setCommentMarker(marker)
return opts
}
func defaultMetadataWith(opts blockOptions) blockMetadata {
- meta := defaultMetadata
- meta.opts = opts
- return meta
+ return blockMetadata{
+ startDirective: "keep-sorted-test start",
+ endDirective: "keep-sorted-test end",
+ opts: opts,
+ }
}
func TestFix(t *testing.T) {
@@ -375,7 +360,7 @@
wantBlocks: []block{
{
- metadata: defaultMetadata,
+ metadata: defaultMetadataWith(defaultOptionsWithCommentMarker("//")),
start: 3,
end: 7,
lines: []string{
@@ -385,7 +370,7 @@
},
},
{
- metadata: defaultMetadata,
+ metadata: defaultMetadataWith(defaultOptionsWithCommentMarker("//")),
start: 9,
end: 13,
lines: []string{
@@ -412,7 +397,7 @@
wantBlocks: []block{
{
- metadata: defaultMetadata,
+ metadata: defaultMetadataWith(defaultOptionsWithCommentMarker("//")),
start: 5,
end: 7,
lines: []string{
@@ -450,7 +435,7 @@
wantBlocks: []block{
{
- metadata: defaultMetadata,
+ metadata: defaultMetadataWith(defaultOptionsWithCommentMarker("//")),
start: 3,
end: 7,
lines: []string{
@@ -478,7 +463,7 @@
wantBlocks: []block{
{
- metadata: defaultMetadata,
+ metadata: defaultMetadataWith(defaultOptionsWithCommentMarker("//")),
start: 1,
end: 6,
lines: []string{
@@ -511,7 +496,7 @@
wantBlocks: []block{
{
- metadata: defaultMetadata,
+ metadata: defaultMetadataWith(defaultOptionsWithCommentMarker("//")),
start: 1,
end: 13,
lines: []string{
@@ -529,7 +514,7 @@
},
nestedBlocks: []block{
{
- metadata: defaultMetadata,
+ metadata: defaultMetadataWith(defaultOptionsWithCommentMarker("//")),
start: 5,
end: 9,
lines: []string{
@@ -589,7 +574,7 @@
wantBlocks: []block{
{
- metadata: defaultMetadata,
+ metadata: defaultMetadataWith(defaultOptionsWithCommentMarker("//")),
start: 1,
end: 34,
lines: []string{
@@ -628,7 +613,7 @@
},
nestedBlocks: []block{
{
- metadata: defaultMetadata,
+ metadata: defaultMetadataWith(defaultOptionsWithCommentMarker("//")),
start: 5,
end: 30,
lines: []string{
@@ -659,7 +644,7 @@
},
nestedBlocks: []block{
{
- metadata: defaultMetadata,
+ metadata: defaultMetadataWith(defaultOptionsWithCommentMarker("//")),
start: 9,
end: 21,
lines: []string{
@@ -677,7 +662,7 @@
},
nestedBlocks: []block{
{
- metadata: defaultMetadata,
+ metadata: defaultMetadataWith(defaultOptionsWithCommentMarker("//")),
start: 13,
end: 17,
lines: []string{
@@ -689,7 +674,7 @@
},
},
{
- metadata: defaultMetadata,
+ metadata: defaultMetadataWith(defaultOptionsWithCommentMarker("//")),
start: 22,
end: 26,
lines: []string{
@@ -703,7 +688,7 @@
},
},
{
- metadata: defaultMetadata,
+ metadata: defaultMetadataWith(defaultOptionsWithCommentMarker("//")),
start: 35,
end: 39,
lines: []string{
@@ -729,7 +714,7 @@
wantBlocks: []block{
{
- metadata: defaultMetadata,
+ metadata: defaultMetadataWith(defaultOptionsWithCommentMarker("//")),
start: 5,
end: 7,
lines: []string{"2"},
@@ -771,8 +756,7 @@
{
name: "NothingToSort",
- opts: defaultOptions,
- in: []string{},
+ in: []string{},
want: []string{},
wantAlreadySorted: true,
@@ -780,7 +764,6 @@
{
name: "AlreadySorted",
- opts: defaultOptions,
in: []string{
"Bar",
"Baz",
@@ -799,7 +782,9 @@
{
name: "AlreadySorted_ExceptForDuplicate",
- opts: defaultOptions,
+ opts: blockOptions{
+ RemoveDuplicates: true,
+ },
in: []string{
"Bar",
"Bar",
@@ -815,9 +800,9 @@
{
name: "AlreadySorted_NewlineSeparated",
- opts: defaultOptionsWith(func(opts *blockOptions) {
- opts.NewlineSeparated = true
- }),
+ opts: blockOptions{
+ NewlineSeparated: true,
+ },
in: []string{
"Bar",
"",
@@ -838,9 +823,9 @@
{
name: "AlreadySorted_ExceptForNewlineSorted",
- opts: defaultOptionsWith(func(opts *blockOptions) {
- opts.NewlineSeparated = true
- }),
+ opts: blockOptions{
+ NewlineSeparated: true,
+ },
in: []string{
"Bar",
"Baz",
@@ -859,7 +844,6 @@
{
name: "SimpleSorting",
- opts: defaultOptions,
in: []string{
"Baz",
"Foo",
@@ -877,10 +861,7 @@
{
name: "CommentOnlyBlock",
- opts: defaultOptionsWith(func(opts *blockOptions) {
- opts.StickyComments = true
- opts.StickyPrefixes = map[string]bool{"//": true}
- }),
+ opts: optionsWithCommentMarker("//"),
in: []string{
"2",
"1",
@@ -896,9 +877,9 @@
{
name: "Prefix",
- opts: defaultOptionsWith(func(opts *blockOptions) {
- opts.PrefixOrder = []string{"INIT_", "", "FINAL_"}
- }),
+ opts: blockOptions{
+ PrefixOrder: []string{"INIT_", "", "FINAL_"},
+ },
in: []string{
// keep-sorted start prefix_order=
"DO_SOMETHING_WITH_BAR",
@@ -937,11 +918,11 @@
{
name: "RemoveDuplicates_ConsidersComments",
- opts: defaultOptionsWith(func(opts *blockOptions) {
+ opts: func() blockOptions {
+ opts := optionsWithCommentMarker("//")
opts.RemoveDuplicates = true
- opts.StickyComments = true
- opts.StickyPrefixes = map[string]bool{"//": true}
- }),
+ return opts
+ }(),
in: []string{
"// comment 1",
"foo",
@@ -963,9 +944,9 @@
{
name: "RemoveDuplicates_IgnoresTraliningCommas",
- opts: defaultOptionsWith(func(opts *blockOptions) {
- opts.RemoveDuplicates = true
- }),
+ opts: blockOptions{
+ RemoveDuplicates: true,
+ },
in: []string{
"foo,",
"bar,",
@@ -980,9 +961,9 @@
{
name: "RemoveDuplicates_IgnoresTrailingCommas_RemovesCommaIfLastElement",
- opts: defaultOptionsWith(func(opts *blockOptions) {
- opts.RemoveDuplicates = true
- }),
+ opts: blockOptions{
+ RemoveDuplicates: true,
+ },
in: []string{
"foo,",
"foo,",
@@ -997,9 +978,9 @@
{
name: "RemoveDuplicates_IgnoresTrailingCommas_RemovesCommaIfOnlyElement",
- opts: defaultOptionsWith(func(opts *blockOptions) {
- opts.RemoveDuplicates = true
- }),
+ opts: blockOptions{
+ RemoveDuplicates: true,
+ },
in: []string{
"foo,",
"foo",
@@ -1012,9 +993,9 @@
{
name: "RemoveDuplicates_Keep",
- opts: defaultOptionsWith(func(opts *blockOptions) {
- opts.RemoveDuplicates = false
- }),
+ opts: blockOptions{
+ RemoveDuplicates: false,
+ },
in: []string{
"foo",
"foo",
@@ -1030,7 +1011,6 @@
{
name: "TrailingCommas",
- opts: defaultOptions,
in: []string{
"foo,",
"baz,",
@@ -1046,9 +1026,9 @@
{
name: "IgnorePrefixes",
- opts: defaultOptionsWith(func(opts *blockOptions) {
- opts.IgnorePrefixes = []string{"fs.setBoolFlag", "fs.setIntFlag"}
- }),
+ opts: blockOptions{
+ IgnorePrefixes: []string{"fs.setBoolFlag", "fs.setIntFlag"},
+ },
in: []string{
// keep-sorted start ignore_prefixes=
`fs.setBoolFlag("paws_with_cute_toebeans", true)`,
@@ -1066,9 +1046,9 @@
{
name: "CaseInsensitive",
- opts: defaultOptionsWith(func(opts *blockOptions) {
- opts.CaseSensitive = false
- }),
+ opts: blockOptions{
+ CaseSensitive: false,
+ },
in: []string{
// keep-sorted start case=yes
"Bravo",
@@ -1086,9 +1066,9 @@
{
name: "Numeric",
- opts: defaultOptionsWith(func(opts *blockOptions) {
- opts.Numeric = true
- }),
+ opts: blockOptions{
+ Numeric: true,
+ },
in: []string{
// keep-sorted start numeric=no
"PROGRESS_100_PERCENT",
@@ -1110,10 +1090,10 @@
{
name: "MultipleTransforms",
- opts: defaultOptionsWith(func(opts *blockOptions) {
- opts.IgnorePrefixes = []string{"R2D2", "C3PO", "R4"}
- opts.Numeric = true
- }),
+ opts: blockOptions{
+ IgnorePrefixes: []string{"R2D2", "C3PO", "R4"},
+ Numeric: true,
+ },
in: []string{
// keep-sorted start ignore_prefixes= numeric=no
"C3PO_ARM_L",
@@ -1139,9 +1119,9 @@
{
name: "NewlineSeparated",
- opts: defaultOptionsWith(func(opts *blockOptions) {
- opts.NewlineSeparated = true
- }),
+ opts: blockOptions{
+ NewlineSeparated: true,
+ },
in: []string{
"B",
"",
@@ -1160,9 +1140,9 @@
{
name: "NewlineSeparated_Empty",
- opts: defaultOptionsWith(func(opts *blockOptions) {
- opts.NewlineSeparated = true
- }),
+ opts: blockOptions{
+ NewlineSeparated: true,
+ },
in: []string{},
want: []string{},
@@ -1192,7 +1172,6 @@
}{
{
name: "Simple",
- opts: defaultOptions,
want: []lineGroup{
{nil, []string{"foo"}},
@@ -1201,10 +1180,7 @@
},
{
name: "StickyComments",
- opts: defaultOptionsWith(func(opts *blockOptions) {
- opts.StickyComments = true
- opts.StickyPrefixes = map[string]bool{"//": true}
- }),
+ opts: optionsWithCommentMarker("//"),
want: []lineGroup{
{
@@ -1227,10 +1203,7 @@
},
{
name: "CommentOnlyGroup",
- opts: defaultOptionsWith(func(opts *blockOptions) {
- opts.StickyComments = true
- opts.StickyPrefixes = map[string]bool{"//": true}
- }),
+ opts: optionsWithCommentMarker("//"),
want: []lineGroup{
{
@@ -1251,10 +1224,9 @@
},
{
name: "Group",
- opts: defaultOptionsWith(func(opts *blockOptions) {
- opts.Group = true
- opts.Block = false
- }),
+ opts: blockOptions{
+ Group: true,
+ },
want: []lineGroup{
{nil, []string{
@@ -1268,10 +1240,10 @@
},
{
name: "Group_Prefixes",
- opts: defaultOptionsWith(func(opts *blockOptions) {
- opts.GroupPrefixes = map[string]bool{"and": true, "with": true}
- opts.Block = false
- }),
+ opts: blockOptions{
+ Group: true,
+ GroupPrefixes: map[string]bool{"and": true, "with": true},
+ },
want: []lineGroup{
{nil, []string{
@@ -1295,10 +1267,9 @@
},
{
name: "Group_UnindentedNewlines",
- opts: defaultOptionsWith(func(opts *blockOptions) {
- opts.Group = true
- opts.Block = false
- }),
+ opts: blockOptions{
+ Group: true,
+ },
want: []lineGroup{
{nil, []string{
@@ -1319,10 +1290,11 @@
},
{
name: "Group_NestedKeepSortedBlocksWithoutAnyIndentation",
- opts: defaultOptionsWith(func(opts *blockOptions) {
+ opts: func() blockOptions {
+ opts := optionsWithCommentMarker("//")
opts.Group = true
- opts.Block = false
- }),
+ return opts
+ }(),
want: []lineGroup{
{[]string{
@@ -1347,9 +1319,9 @@
},
{
name: "Block_Brackets",
- opts: defaultOptionsWith(func(opts *blockOptions) {
- opts.Block = true
- }),
+ opts: blockOptions{
+ Block: true,
+ },
want: []lineGroup{
{nil, []string{
@@ -1368,9 +1340,9 @@
},
{
name: "Block_Quotes",
- opts: defaultOptionsWith(func(opts *blockOptions) {
- opts.Block = true
- }),
+ opts: blockOptions{
+ Block: true,
+ },
want: []lineGroup{
{nil, []string{
@@ -1389,9 +1361,9 @@
},
{
name: "Block_EscapedQuote",
- opts: defaultOptionsWith(func(opts *blockOptions) {
- opts.Block = true
- }),
+ opts: blockOptions{
+ Block: true,
+ },
want: []lineGroup{
{nil, []string{
@@ -1410,9 +1382,9 @@
},
{
name: "Block_IgnoresQuotesWithinQuotes",
- opts: defaultOptionsWith(func(opts *blockOptions) {
- opts.Block = true
- }),
+ opts: blockOptions{
+ Block: true,
+ },
want: []lineGroup{
{nil, []string{
@@ -1431,9 +1403,9 @@
},
{
name: "Block_IgnoresBracesWithinQuotes",
- opts: defaultOptionsWith(func(opts *blockOptions) {
- opts.Block = true
- }),
+ opts: blockOptions{
+ Block: true,
+ },
want: []lineGroup{
{nil, []string{
@@ -1452,10 +1424,11 @@
},
{
name: "Block_IgnoresSpecialCharactersWithinFullLineComments",
- opts: defaultOptionsWith(func(opts *blockOptions) {
+ opts: func() blockOptions {
+ opts := optionsWithCommentMarker("//")
opts.Block = true
- opts.StickyPrefixes["//"] = true
- }),
+ return opts
+ }(),
want: []lineGroup{
{nil, []string{
@@ -1476,10 +1449,11 @@
},
{
name: "Block_IgnoresSpecialCharactersWithinTrailingComments",
- opts: defaultOptionsWith(func(opts *blockOptions) {
+ opts: func() blockOptions {
+ opts := optionsWithCommentMarker("//")
opts.Block = true
- opts.StickyPrefixes["//"] = true
- }),
+ return opts
+ }(),
want: []lineGroup{
{nil, []string{
@@ -1502,9 +1476,9 @@
},
{
name: "Block_TripleQuotes",
- opts: defaultOptionsWith(func(opts *blockOptions) {
- opts.Block = true
- }),
+ opts: blockOptions{
+ Block: true,
+ },
want: []lineGroup{
{nil, []string{
@@ -1534,106 +1508,127 @@
func TestBlockOptions(t *testing.T) {
for _, tc := range []struct {
- name string
- in string
+ name string
+ in string
+ defaultOptions blockOptions
want blockOptions
wantErr string
}{
{
- name: "DefaultOptions",
- in: "// keep-sorted-test",
+ name: "DefaultOptions",
+ in: "",
+ defaultOptions: defaultOptions,
want: defaultOptions,
},
{
- name: "SimpleSwitch",
- in: "// keep-sorted-test lint=no",
+ name: "CommentMarker",
+ in: "// keep-sorted-test start",
- want: defaultOptionsWith(func(opts *blockOptions) {
- opts.Lint = false
- }),
+ want: blockOptions{
+ commentMarker: "//",
+ },
+ },
+ {
+ name: "StickyComments",
+ in: "// keep-sorted-test start sticky_comments=yes",
+
+ want: blockOptions{
+ StickyComments: true,
+ StickyPrefixes: map[string]bool{"//": true},
+ commentMarker: "//",
+ },
+ },
+ {
+ name: "SimpleSwitch",
+ in: "lint=yes",
+
+ want: blockOptions{Lint: true},
},
{
name: "SkipLines",
- in: "// keep-sorted-test skip_lines=10",
+ in: "skip_lines=10",
- want: defaultOptionsWith(func(opts *blockOptions) {
- opts.SkipLines = 10
- }),
+ want: blockOptions{SkipLines: 10},
},
{
name: "ErrorSkipLinesIsNegative",
- in: "// keep-sorted-test skip_lines=-1",
+ in: "skip_lines=-1",
- want: defaultOptions,
- wantErr: `skip_lines has invalid value: -1`,
+ wantErr: "skip_lines has invalid value: -1",
},
{
name: "ItemList",
- in: "// keep-sorted-test prefix_order=a,b,c,d",
+ in: "prefix_order=a,b,c,d",
- want: defaultOptionsWith(func(opts *blockOptions) {
- opts.PrefixOrder = []string{"a", "b", "c", "d"}
- }),
+ want: blockOptions{
+ PrefixOrder: []string{"a", "b", "c", "d"},
+ },
},
{
name: "ItemSet",
- in: "keep-sorted-test sticky_prefixes=a,b,c,d",
+ in: "sticky_prefixes=a,b,c,d",
- want: defaultOptionsWith(func(opts *blockOptions) {
- opts.StickyPrefixes = map[string]bool{"a": true, "b": true, "c": true, "d": true}
- opts.commentMarker = ""
- }),
+ want: blockOptions{
+ StickyPrefixes: map[string]bool{
+ "a": true,
+ "b": true,
+ "c": true,
+ "d": true,
+ },
+ },
},
{
name: "ignore_prefixes",
- in: "// keep-sorted-test ignore_prefixes=a,b,c,d",
+ in: "ignore_prefixes=a,b,c,d",
- want: defaultOptionsWith(func(opts *blockOptions) {
- opts.IgnorePrefixes = []string{"a", "b", "c", "d"}
- }),
+ want: blockOptions{
+ IgnorePrefixes: []string{"a", "b", "c", "d"},
+ },
+ },
+ {
+ name: "ignore_prefixes_ChecksLongestPrefixesFirst",
+ in: "ignore_prefixes=DoSomething(,DoSomething({",
+
+ want: blockOptions{
+ IgnorePrefixes: []string{"DoSomething({", "DoSomething("},
+ },
},
{
name: "GroupPrefixesRequiresGrouping",
- in: "// keep-sorted-test group_prefixes=a,b,c group=no",
+ in: "group_prefixes=a,b,c group=no",
- want: defaultOptionsWith(func(opts *blockOptions) {
- opts.Group = false
- }),
wantErr: "group_prefixes may not be used with group=no",
},
{
- name: "ignore_prefixes_ChecksLognestPrefixesFirst",
- in: "// keep-sorted-test ignore_prefixes=DoSomething(,DoSomething({",
-
- want: defaultOptionsWith(func(opts *blockOptions) {
- opts.IgnorePrefixes = []string{"DoSomething({", "DoSomething("}
- }),
- },
- {
name: "OptionInTrailingComment",
- in: "// keep-sorted-test block=yes # lint=no",
+ in: "# keep-sorted-test start block=yes # lint=yes",
- want: defaultOptionsWith(func(opts *blockOptions) {
- opts.Block = true
- opts.Lint = false
- }),
+ want: blockOptions{
+ Block: true,
+ Lint: true,
+ commentMarker: "#",
+ },
},
{
name: "ErrorDoesNotStopParsing",
- in: "// keep-sorted-test lint=yep case=no",
+ in: "lint=nah case=no",
+ defaultOptions: blockOptions{
+ Lint: true,
+ CaseSensitive: true,
+ },
- want: defaultOptionsWith(func(opts *blockOptions) {
- opts.Lint = true // The default value should not change.
- opts.CaseSensitive = false
- }),
- wantErr: `option "lint" has unknown value "yep"`,
+ want: blockOptions{
+ Lint: true, // The default value should not change.
+ CaseSensitive: false,
+ },
+ wantErr: `option "lint" has unknown value "nah"`,
},
} {
t.Run(tc.name, func(t *testing.T) {
initZerolog(t)
- got, err := New("keep-sorted-test").parseBlockOptions(tc.in)
+ got, err := New("keep-sorted-test").parseBlockOptions(tc.in, tc.defaultOptions)
if err != nil {
if tc.wantErr == "" {
t.Errorf("parseBlockOptions(%q) = %v", tc.in, err)
diff --git a/keepsorted/options.go b/keepsorted/options.go
index fd9ea18..1e5a72c 100644
--- a/keepsorted/options.go
+++ b/keepsorted/options.go
@@ -45,7 +45,7 @@
// 4. int: key=123
type blockOptions struct {
// Lint determines whether we emit lint warnings for this block.
- Lint bool `default:"true"`
+ Lint bool
///////////////////////////
// Pre-sorting options //
@@ -54,13 +54,13 @@
// SkipLines is the number of lines to ignore before sorting.
SkipLines int `key:"skip_lines"`
// Group determines whether we group lines together based on increasing indentation.
- Group bool `default:"true"`
+ Group bool
// GroupPrefixes tells us about other types of lines that should be added to a group.
GroupPrefixes map[string]bool `key:"group_prefixes"`
// Block opts us into a more complicated algorithm to try and understand blocks of code.
- Block bool `default:"false"`
+ Block bool
// StickyComments tells us to attach comments to the line immediately below them while sorting.
- StickyComments bool `key:"sticky_comments" default:"true"`
+ StickyComments bool `key:"sticky_comments"`
// StickyPrefixes tells us about other types of lines that should behave as sticky comments.
StickyPrefixes map[string]bool `key:"sticky_prefixes"`
@@ -69,9 +69,9 @@
///////////////////////
// CaseSensitive is whether we're case sensitive while sorting.
- CaseSensitive bool `key:"case" default:"true"`
+ CaseSensitive bool `key:"case"`
// Numeric indicates that the contents should be sorted like numbers.
- Numeric bool `default:"false"`
+ Numeric bool
// PrefixOrder allows the user to explicitly order lines based on their matching prefix.
PrefixOrder []string `key:"prefix_order"`
// IgnorePrefixes is a slice of prefixes that we do not consider when sorting lines.
@@ -82,16 +82,28 @@
////////////////////////////
// NewlineSeparated indicates that the groups should be separated with newlines.
- NewlineSeparated bool `key:"newline_separated" default:"false"`
+ NewlineSeparated bool `key:"newline_separated"`
// RemoveDuplicates determines whether we drop lines that are an exact duplicate.
- RemoveDuplicates bool `key:"remove_duplicates" default:"true"`
+ RemoveDuplicates bool `key:"remove_duplicates"`
// Syntax used to start a comment for keep-sorted annotation, e.g. "//".
commentMarker string
}
-func (f *Fixer) parseBlockOptions(startLine string) (blockOptions, error) {
- ret := blockOptions{}
+var defaultOptions = blockOptions{
+ Lint: true,
+ Group: true,
+ StickyComments: true,
+ StickyPrefixes: nil, // Will be populated with the comment marker of the start directive.
+ CaseSensitive: true,
+ RemoveDuplicates: true,
+ // If we ever add a default map or slice or other reference type to this
+ // struct, we'll want to make sure we're doing a deep copy in
+ // parseBlockOptions.
+}
+
+func (f *Fixer) parseBlockOptions(startLine string, defaults blockOptions) (blockOptions, error) {
+ ret := defaults
opts := reflect.ValueOf(&ret)
var errs error
for i := 0; i < opts.Elem().NumField(); i++ {
@@ -102,10 +114,13 @@
val, err := parseBlockOption(field, startLine)
if err != nil {
+ if errors.Is(err, errOptionNotSet) {
+ continue
+ }
errs = errors.Join(errs, err)
+ } else {
+ opts.Elem().Field(i).Set(val)
}
-
- opts.Elem().Field(i).Set(val)
}
if ret.SkipLines < 0 {
@@ -119,13 +134,7 @@
}
if cm := f.guessCommentMarker(startLine); cm != "" {
- ret.commentMarker = cm
- if ret.StickyComments {
- if ret.StickyPrefixes == nil {
- ret.StickyPrefixes = make(map[string]bool)
- }
- ret.StickyPrefixes[cm] = true
- }
+ ret.setCommentMarker(cm)
}
if len(ret.IgnorePrefixes) > 1 {
// Look at longer prefixes first, in case one of these prefixes is a prefix of another.
@@ -135,6 +144,8 @@
return ret, errs
}
+var errOptionNotSet = errors.New("not set in start directive")
+
func parseBlockOption(f reflect.StructField, startLine string) (reflect.Value, error) {
key := strings.ToLower(f.Name)
if k, ok := f.Tag.Lookup("key"); ok {
@@ -146,39 +157,27 @@
val := string(regex.ExpandString(nil, "${value}", startLine, m))
return parseValue(f, key, val)
}
- return parseDefaultValue(f, key), nil
-}
-
-func parseDefaultValue(f reflect.StructField, key string) reflect.Value {
- val, err := parseValueWithDefault(f, key, f.Tag.Get("default"), func() reflect.Value { return reflect.Zero(f.Type) })
- if err != nil {
- panic(fmt.Errorf("blockOptions field %s has invalid default %q: %w", f.Name, f.Tag.Get("default"), err))
- }
- return val
+ return reflect.Zero(f.Type), fmt.Errorf("option %q: %w", key, errOptionNotSet)
}
func parseValue(f reflect.StructField, key, val string) (reflect.Value, error) {
- return parseValueWithDefault(f, key, val, func() reflect.Value { return parseDefaultValue(f, key) })
-}
-
-func parseValueWithDefault(f reflect.StructField, key, val string, defaultFn func() reflect.Value) (reflect.Value, error) {
switch f.Type {
case reflect.TypeOf(true):
b, ok := boolValues[val]
if !ok {
- return defaultFn(), fmt.Errorf("option %q has unknown value %q", key, val)
+ return reflect.Zero(f.Type), fmt.Errorf("option %q has unknown value %q", key, val)
}
return reflect.ValueOf(b), nil
case reflect.TypeOf([]string{}):
if val == "" {
- return defaultFn(), nil
+ return reflect.Zero(f.Type), nil
}
return reflect.ValueOf(strings.Split(val, ",")), nil
case reflect.TypeOf(map[string]bool{}):
if val == "" {
- return defaultFn(), nil
+ return reflect.Zero(f.Type), nil
}
sp := strings.Split(val, ",")
@@ -189,12 +188,12 @@
return reflect.ValueOf(m), nil
case reflect.TypeOf(0):
if val == "" {
- return defaultFn(), nil
+ return reflect.Zero(f.Type), nil
}
i, err := strconv.Atoi(val)
if err != nil {
- return defaultFn(), fmt.Errorf("option %q has invalid value %q: %w", key, val, err)
+ return reflect.Zero(f.Type), fmt.Errorf("option %q has invalid value %q: %w", key, val, err)
}
return reflect.ValueOf(i), nil
}
@@ -220,6 +219,16 @@
return ""
}
+func (opts *blockOptions) setCommentMarker(marker string) {
+ opts.commentMarker = marker
+ if opts.StickyComments {
+ if opts.StickyPrefixes == nil {
+ opts.StickyPrefixes = make(map[string]bool)
+ }
+ opts.StickyPrefixes[marker] = true
+ }
+}
+
// hasPrefix determines if s has one of the prefixes.
func hasPrefix(s string, prefixes map[string]bool) bool {
if len(prefixes) == 0 {
