From ff00dc87063160b267515e97eeb57bf0d927f763 Mon Sep 17 00:00:00 2001 From: Will Beason Date: Thu, 19 Mar 2026 12:08:00 -0500 Subject: [PATCH 1/3] Add warning if skip_lines causes empty block Specifically, this triggers if skip_lines causes the block's start to be at or after the block's end. So fully-empty blocks with no skip_lines (or no-op skip_lines, such as skip_lines=0,0) are still fine. Modify tests in skip_lines golden test to show this behavior. Modify how start and end are logged in cmd.go. As-is causes the slightly confusing behavior that "end" is always logged before start because it comes first alphabetically. Now these are logged as [start,end]=[x,y]. The warning for if start>=end shows both the original start/end and their offsets to make it more obvious how the values are arrived at. This helps finding the problematic lines, as otherwise you just see where the block start and end is, not where the directives are. --- cmd/cmd.go | 2 +- goldens/skip_lines.err | 4 ++++ goldens/skip_lines.in | 12 ++++++++++-- goldens/skip_lines.out | 12 ++++++++++-- keepsorted/block.go | 5 +++++ 5 files changed, 30 insertions(+), 5 deletions(-) create mode 100644 goldens/skip_lines.err diff --git a/cmd/cmd.go b/cmd/cmd.go index 4ef258c..af10f62 100644 --- a/cmd/cmd.go +++ b/cmd/cmd.go @@ -237,7 +237,7 @@ func fix(fixer *keepsorted.Fixer, filenames []string, modifiedLines []keepsorted if warn.Lines.Start == warn.Lines.End { log = log.Int("line", warn.Lines.Start) } else { - log = log.Int("start", warn.Lines.Start).Int("end", warn.Lines.End) + log = log.Ints("[start,end]", []int{warn.Lines.Start, warn.Lines.End}) } log.Msg(warn.Message) } diff --git a/goldens/skip_lines.err b/goldens/skip_lines.err new file mode 100644 index 0000000..f93b07f --- /dev/null +++ b/goldens/skip_lines.err @@ -0,0 +1,4 @@ +WRN block start is at or after end, possibly due to skip_lines [5+1,6-0] line=6 +WRN block start is at or after end, possibly due to skip_lines [9+0,10-1] line=9 +WRN block start is at or after end, possibly due to skip_lines [22+10,26-0] [start,end]=[32,26] +exit status 1 diff --git a/goldens/skip_lines.in b/goldens/skip_lines.in index a01443e..f28fc5f 100644 --- a/goldens/skip_lines.in +++ b/goldens/skip_lines.in @@ -1,7 +1,15 @@ -Skip lines with an empty block: +Skip no lines with an empty block is ok: +keep-sorted-test start skip_lines=0 +keep-sorted-test end + +Skip lines at start with an empty block: keep-sorted-test start skip_lines=1 keep-sorted-test end +Skip lines at end with an empty block: +keep-sorted-test start skip_lines=-1 +keep-sorted-test end + Skip two lines: // keep-sorted-test start skip_lines=2 foo @@ -11,7 +19,7 @@ b a // keep-sorted-test end -Number of skipped lines is greater than block size, so block is ignored: +Number of skipped lines is greater than block size, so skip_lines causes warning: // keep-sorted-test start skip_lines=10 z y diff --git a/goldens/skip_lines.out b/goldens/skip_lines.out index 1c3405b..b0e12c5 100644 --- a/goldens/skip_lines.out +++ b/goldens/skip_lines.out @@ -1,7 +1,15 @@ -Skip lines with an empty block: +Skip no lines with an empty block is ok: +keep-sorted-test start skip_lines=0 +keep-sorted-test end + +Skip lines at start with an empty block: keep-sorted-test start skip_lines=1 keep-sorted-test end +Skip lines at end with an empty block: +keep-sorted-test start skip_lines=-1 +keep-sorted-test end + Skip two lines: // keep-sorted-test start skip_lines=2 foo @@ -11,7 +19,7 @@ b c // keep-sorted-test end -Number of skipped lines is greater than block size, so block is ignored: +Number of skipped lines is greater than block size, so skip_lines causes warning: // keep-sorted-test start skip_lines=10 z y diff --git a/keepsorted/block.go b/keepsorted/block.go index 35a5148..e453002 100644 --- a/keepsorted/block.go +++ b/keepsorted/block.go @@ -15,6 +15,7 @@ package keepsorted import ( + "fmt" "slices" "strings" @@ -116,6 +117,10 @@ func (f *Fixer) newBlocks(filename string, lines []string, offset int, include f start.index += opts.startOffset() endIndex += opts.endOffset() if start.index >= endIndex { + warnings = append(warnings, finding(filename, start.index, endIndex, + fmt.Sprintf("block start is at or after end, possibly due to skip_lines [%d+%d,%d-%d]", + start.index-opts.startOffset(), opts.startOffset(), endIndex-opts.endOffset(), -opts.endOffset())), + ) continue } From e9f4fbec7bd45b9bd22c64ac6d3d8f9b3878ae0e Mon Sep 17 00:00:00 2001 From: Will Beason Date: Thu, 19 Mar 2026 11:39:35 -0700 Subject: [PATCH 2/3] Add offsets to start/end indices for readability and consistency Co-authored-by: Jeffrey Faer --- keepsorted/block.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/keepsorted/block.go b/keepsorted/block.go index e453002..9ff284b 100644 --- a/keepsorted/block.go +++ b/keepsorted/block.go @@ -117,9 +117,9 @@ func (f *Fixer) newBlocks(filename string, lines []string, offset int, include f start.index += opts.startOffset() endIndex += opts.endOffset() if start.index >= endIndex { - warnings = append(warnings, finding(filename, start.index, endIndex, + warnings = append(warnings, finding(filename, start.index-opts.startOffset()+offset, endIndex-opts.endOffset()+offset, fmt.Sprintf("block start is at or after end, possibly due to skip_lines [%d+%d,%d-%d]", - start.index-opts.startOffset(), opts.startOffset(), endIndex-opts.endOffset(), -opts.endOffset())), + start.index-opts.startOffset()+offset, opts.startOffset(), endIndex-opts.endOffset()+offset, -opts.endOffset())), ) continue } From a054e365b6ad8a97ec5fd862a390edfaf2844dc2 Mon Sep 17 00:00:00 2001 From: Will Beason Date: Thu, 19 Mar 2026 11:46:12 -0700 Subject: [PATCH 3/3] Update tests Now that we use the correct indices, these needed to be updated. --- goldens/skip_lines.err | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/goldens/skip_lines.err b/goldens/skip_lines.err index f93b07f..b407181 100644 --- a/goldens/skip_lines.err +++ b/goldens/skip_lines.err @@ -1,4 +1,4 @@ -WRN block start is at or after end, possibly due to skip_lines [5+1,6-0] line=6 -WRN block start is at or after end, possibly due to skip_lines [9+0,10-1] line=9 -WRN block start is at or after end, possibly due to skip_lines [22+10,26-0] [start,end]=[32,26] +WRN block start is at or after end, possibly due to skip_lines [6+1,7-0] [start,end]=[6,7] +WRN block start is at or after end, possibly due to skip_lines [10+0,11-1] [start,end]=[10,11] +WRN block start is at or after end, possibly due to skip_lines [23+10,27-0] [start,end]=[23,27] exit status 1