Keep nested keep-sorted blocks together in the same group, regardless of indentation level. (#19)
diff --git a/goldens/group.in b/goldens/group.in
index d2c7ac7..544db49 100644
--- a/goldens/group.in
+++ b/goldens/group.in
@@ -137,3 +137,22 @@
// keep-sorted-test end
];
// keep-sorted-test end
+
+Nested keep-sorted without indentation
+// keep-sorted-test start group=yes newline_separated=yes
+
+// def
+// keep-sorted-test start
+3
+1
+2
+// keep-sorted-test end
+
+// abc
+// keep-sorted-test start
+b
+c
+a
+// keep-sorted-test end
+
+// keep-sorted-test end
diff --git a/goldens/group.out b/goldens/group.out
index abd5eda..94e065c 100644
--- a/goldens/group.out
+++ b/goldens/group.out
@@ -138,3 +138,21 @@
// keep-sorted-test end
];
// keep-sorted-test end
+
+Nested keep-sorted without indentation
+// keep-sorted-test start group=yes newline_separated=yes
+// def
+// keep-sorted-test start
+1
+2
+3
+// keep-sorted-test end
+
+// abc
+// keep-sorted-test start
+a
+b
+c
+// keep-sorted-test end
+
+// keep-sorted-test end
diff --git a/keepsorted/block.go b/keepsorted/block.go
index 5065f85..d75991e 100644
--- a/keepsorted/block.go
+++ b/keepsorted/block.go
@@ -24,7 +24,7 @@
)
type block struct {
- opts blockOptions
+ metadata blockMetadata
start, end int
// lines are the content of this block from the original file.
@@ -40,6 +40,11 @@
nestedBlocks []block
}
+type blockMetadata struct {
+ startDirective, endDirective string
+ opts blockOptions
+}
+
type incompleteBlock struct {
line int
dir directive
@@ -114,7 +119,11 @@
// directives will have depth >= 1 based on how deep it is.
depth := len(starts)
block := block{
- opts: opts,
+ metadata: blockMetadata{
+ startDirective: f.startDirective,
+ endDirective: f.endDirective,
+ opts: opts,
+ },
start: start.index + offset,
end: endIndex + offset,
lines: lines[start.index+1 : endIndex],
@@ -223,8 +232,8 @@
}
}
- groups := groupLines(lines, b.opts)
- log.Printf("%d groups for block at index %d are (options %#v)", len(groups), b.start, b.opts)
+ groups := groupLines(lines, b.metadata)
+ log.Printf("%d groups for block at index %d are (options %#v)", len(groups), b.start, b.metadata.opts)
for _, lg := range groups {
log.Printf("%#v", lg)
}
@@ -232,7 +241,7 @@
trimTrailingComma := handleTrailingComma(groups)
wasNewlineSeparated := true
- if b.opts.NewlineSeparated {
+ if b.metadata.opts.NewlineSeparated {
wasNewlineSeparated = isNewlineSeparated(groups)
var withoutNewlines []lineGroup
for _, lg := range groups {
@@ -245,7 +254,7 @@
}
removedDuplicate := false
- if b.opts.RemoveDuplicates {
+ if b.metadata.opts.RemoveDuplicates {
seen := map[string]bool{}
var deduped []lineGroup
for _, lg := range groups {
@@ -270,7 +279,7 @@
trimTrailingComma(groups)
- if b.opts.NewlineSeparated {
+ if b.metadata.opts.NewlineSeparated {
var separated []lineGroup
newline := lineGroup{lines: []string{""}}
for _, lg := range groups {
@@ -375,8 +384,8 @@
// An empty prefix can be used to move all remaining entries to a position
// between other prefixes.
var prefixWeights []prefixWeight
- for i, p := range b.opts.PrefixOrder {
- prefixWeights = append(prefixWeights, prefixWeight{p, i - len(b.opts.PrefixOrder)})
+ for i, p := range b.metadata.opts.PrefixOrder {
+ prefixWeights = append(prefixWeights, prefixWeight{p, i - len(b.metadata.opts.PrefixOrder)})
}
slices.SortStableFunc(prefixWeights, func(a, b prefixWeight) int {
return cmp.Compare(b.prefix, a.prefix)
@@ -411,13 +420,13 @@
// foo_123
transformOrder := comparingPropertyWith(func(lg lineGroup) numericTokens {
l := lg.joinedLines()
- if s, ok := b.opts.removeIgnorePrefix(l); ok {
+ if s, ok := b.metadata.opts.removeIgnorePrefix(l); ok {
l = s
}
- if !b.opts.CaseSensitive {
+ if !b.metadata.opts.CaseSensitive {
l = strings.ToLower(l)
}
- return b.opts.maybeParseNumeric(l)
+ return b.metadata.opts.maybeParseNumeric(l)
}, numericTokens.compare)
return func(a, b lineGroup) int {
diff --git a/keepsorted/keep_sorted.go b/keepsorted/keep_sorted.go
index 8f6ad11..4196d58 100644
--- a/keepsorted/keep_sorted.go
+++ b/keepsorted/keep_sorted.go
@@ -128,7 +128,7 @@
var fs []*Finding
for _, b := range blocks {
- if considerLintOption && !b.opts.Lint {
+ if considerLintOption && !b.metadata.opts.Lint {
continue
}
if s, alreadySorted := b.sorted(); !alreadySorted {
diff --git a/keepsorted/keep_sorted_test.go b/keepsorted/keep_sorted_test.go
index 038299e..5521046 100644
--- a/keepsorted/keep_sorted_test.go
+++ b/keepsorted/keep_sorted_test.go
@@ -39,6 +39,12 @@
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.
@@ -55,6 +61,12 @@
return opts
}
+func defaultMetadataWith(opts blockOptions) blockMetadata {
+ meta := defaultMetadata
+ meta.opts = opts
+ return meta
+}
+
func TestFix(t *testing.T) {
for _, tc := range []struct {
name string
@@ -348,9 +360,9 @@
wantBlocks: []block{
{
- opts: defaultOptions,
- start: 3,
- end: 7,
+ metadata: defaultMetadata,
+ start: 3,
+ end: 7,
lines: []string{
"c",
"b",
@@ -358,9 +370,9 @@
},
},
{
- opts: defaultOptions,
- start: 9,
- end: 13,
+ metadata: defaultMetadata,
+ start: 9,
+ end: 13,
lines: []string{
"1",
"2",
@@ -385,9 +397,9 @@
wantBlocks: []block{
{
- opts: defaultOptions,
- start: 5,
- end: 7,
+ metadata: defaultMetadata,
+ start: 5,
+ end: 7,
lines: []string{
"baz",
},
@@ -423,9 +435,9 @@
wantBlocks: []block{
{
- opts: defaultOptions,
- start: 3,
- end: 7,
+ metadata: defaultMetadata,
+ start: 3,
+ end: 7,
lines: []string{
"c",
"b",
@@ -451,9 +463,9 @@
wantBlocks: []block{
{
- opts: defaultOptions,
- start: 1,
- end: 6,
+ metadata: defaultMetadata,
+ start: 1,
+ end: 6,
lines: []string{
"",
"1",
@@ -484,9 +496,9 @@
wantBlocks: []block{
{
- opts: defaultOptions,
- start: 1,
- end: 13,
+ metadata: defaultMetadata,
+ start: 1,
+ end: 13,
lines: []string{
"a",
"b",
@@ -502,9 +514,9 @@
},
nestedBlocks: []block{
{
- opts: defaultOptions,
- start: 5,
- end: 9,
+ metadata: defaultMetadata,
+ start: 5,
+ end: 9,
lines: []string{
"d",
"e",
@@ -562,9 +574,9 @@
wantBlocks: []block{
{
- opts: defaultOptions,
- start: 1,
- end: 34,
+ metadata: defaultMetadata,
+ start: 1,
+ end: 34,
lines: []string{
"0.1",
"0.2",
@@ -601,9 +613,9 @@
},
nestedBlocks: []block{
{
- opts: defaultOptions,
- start: 5,
- end: 30,
+ metadata: defaultMetadata,
+ start: 5,
+ end: 30,
lines: []string{
"1.1",
"1.2",
@@ -632,9 +644,9 @@
},
nestedBlocks: []block{
{
- opts: defaultOptions,
- start: 9,
- end: 21,
+ metadata: defaultMetadata,
+ start: 9,
+ end: 21,
lines: []string{
"2.1",
"2.2",
@@ -650,9 +662,9 @@
},
nestedBlocks: []block{
{
- opts: defaultOptions,
- start: 13,
- end: 17,
+ metadata: defaultMetadata,
+ start: 13,
+ end: 17,
lines: []string{
"3.1",
"3.2",
@@ -662,9 +674,9 @@
},
},
{
- opts: defaultOptions,
- start: 22,
- end: 26,
+ metadata: defaultMetadata,
+ start: 22,
+ end: 26,
lines: []string{
"4.1",
"4.2",
@@ -676,9 +688,9 @@
},
},
{
- opts: defaultOptions,
- start: 35,
- end: 39,
+ metadata: defaultMetadata,
+ start: 35,
+ end: 39,
lines: []string{
"5.1",
"5.2",
@@ -702,10 +714,10 @@
wantBlocks: []block{
{
- opts: defaultOptions,
- start: 5,
- end: 7,
- lines: []string{"2"},
+ metadata: defaultMetadata,
+ start: 5,
+ end: 7,
+ lines: []string{"2"},
},
},
wantIncompleteBlocks: []incompleteBlock{
@@ -721,7 +733,7 @@
}
gotBlocks, gotIncompleteBlocks := New("keep-sorted-test").newBlocks(strings.Split(tc.in, "\n"), 0, tc.include)
- if diff := cmp.Diff(tc.wantBlocks, gotBlocks, cmp.AllowUnexported(block{}), cmp.AllowUnexported(blockOptions{})); diff != "" {
+ if diff := cmp.Diff(tc.wantBlocks, gotBlocks, cmp.AllowUnexported(block{}, blockMetadata{}, blockOptions{})); diff != "" {
t.Errorf("blocks diff (-want +got):\n%s", diff)
}
if diff := cmp.Diff(tc.wantIncompleteBlocks, gotIncompleteBlocks, cmp.AllowUnexported(incompleteBlock{})); diff != "" {
@@ -1144,7 +1156,7 @@
} {
t.Run(tc.name, func(t *testing.T) {
initZerolog(t)
- got, gotAlreadySorted := block{lines: tc.in, opts: tc.opts}.sorted()
+ got, gotAlreadySorted := block{lines: tc.in, metadata: defaultMetadataWith(tc.opts)}.sorted()
if gotAlreadySorted != tc.wantAlreadySorted {
t.Errorf("alreadySorted mismatch: got %t want %t", gotAlreadySorted, tc.wantAlreadySorted)
}
@@ -1226,6 +1238,7 @@
name: "Group",
opts: defaultOptionsWith(func(opts *blockOptions) {
opts.Group = true
+ opts.Block = false
}),
want: []lineGroup{
@@ -1242,6 +1255,7 @@
name: "Group_UnindentedNewlines",
opts: defaultOptionsWith(func(opts *blockOptions) {
opts.Group = true
+ opts.Block = false
}),
want: []lineGroup{
@@ -1262,6 +1276,34 @@
},
},
{
+ name: "Group_NestedKeepSortedBlocksWithoutAnyIndentation",
+ opts: defaultOptionsWith(func(opts *blockOptions) {
+ opts.Group = true
+ opts.Block = false
+ }),
+
+ want: []lineGroup{
+ {[]string{
+ "// def",
+ }, []string{
+ "// keep-sorted-test start",
+ "3",
+ "1",
+ "2",
+ "// keep-sorted-test end",
+ }},
+ {[]string{
+ "// abc",
+ }, []string{
+ "// keep-sorted-test start",
+ "b",
+ "c",
+ "a",
+ "// keep-sorted-test end",
+ }},
+ },
+ },
+ {
name: "Block_Brackets",
opts: defaultOptionsWith(func(opts *blockOptions) {
opts.Block = true
@@ -1440,7 +1482,7 @@
in = append(in, lg.lines...)
}
- got := groupLines(in, tc.opts)
+ got := groupLines(in, defaultMetadataWith(tc.opts))
if diff := cmp.Diff(tc.want, got, cmp.AllowUnexported(lineGroup{})); diff != "" {
t.Errorf("groupLines mismatch (-want +got):\n%s", diff)
}
diff --git a/keepsorted/line_group.go b/keepsorted/line_group.go
index 43c7682..5d56c56 100644
--- a/keepsorted/line_group.go
+++ b/keepsorted/line_group.go
@@ -30,23 +30,44 @@
}
// groupLines splits lines into one or more lineGroups based on the provided options.
-func groupLines(lines []string, opts blockOptions) []lineGroup {
+func groupLines(lines []string, metadata blockMetadata) []lineGroup {
var groups []lineGroup
+ // Tracks which subsection of lines contains the comments for the current lineGroup.
var commentRange indexRange
+ // Tracks which subsection of lines contains the content for the current lineGroup.
var lineRange indexRange
+
+ // group=yes and block=no, these pieces of information are used to determine
+ // when we group lines together into a single group.
+
+ // Indent: All lines indented further than the first line are grouped together.
+ // Edge case: Whitespace-only lines are included in the group based on the
+ // indentation of the next non-empty line after the whitespace-only line.
var indents []int
var initialIndent *int
+ // Counts the number of unmatched start directives we've seen in the current group.
+ // We will include entire keep-sorted blocks as grouped lines to avoid
+ // breaking nested keep-sorted blocks that don't have indentation.
+ var numUnmatchedStartDirectives int
+
+ // block=yes: The code block that we're constructing until we have matched braces and quotations.
var block codeBlock
- if opts.Group {
+ if metadata.opts.Group {
indents = calculateIndents(lines)
}
// append a line to both lineRange, and block, if necessary.
appendLine := func(i int, l string) {
lineRange.append(i)
- if opts.Block {
- block.append(l, opts)
+ if metadata.opts.Block {
+ block.append(l, metadata.opts)
+ } else if metadata.opts.Group {
+ if strings.Contains(l, metadata.startDirective) {
+ numUnmatchedStartDirectives++
+ } else if strings.Contains(l, metadata.endDirective) {
+ numUnmatchedStartDirectives--
+ }
}
}
// finish an outstanding lineGroup and reset our state to prepare for a new lineGroup.
@@ -57,21 +78,27 @@
block = codeBlock{}
}
for i, l := range lines {
- if opts.Block && !lineRange.empty() && block.expectsContinuation() {
+ if metadata.opts.Block && !lineRange.empty() && block.expectsContinuation() {
appendLine(i, l)
- } else if opts.Group && !lineRange.empty() && initialIndent != nil && indents[i] > *initialIndent {
+ } else if metadata.opts.Group && (!lineRange.empty() && initialIndent != nil && indents[i] > *initialIndent || numUnmatchedStartDirectives > 0) {
appendLine(i, l)
- } else if opts.hasStickyPrefix(l) {
+ } else if metadata.opts.hasStickyPrefix(l) {
if !lineRange.empty() {
finishGroup()
}
- commentRange.append(i)
+ if !metadata.opts.Block && metadata.opts.Group && strings.Contains(l, metadata.startDirective) {
+ // We don't need to check for end directives here because this makes
+ // numUnmatchedStartDirectives > 0, so we'll take the code path above through appendLine.
+ appendLine(i, l)
+ } else {
+ commentRange.append(i)
+ }
} else {
if !lineRange.empty() {
finishGroup()
}
- if opts.Group && initialIndent == nil {
+ if metadata.opts.Group && initialIndent == nil {
initialIndent = &indents[i]
}
appendLine(i, l)
