From 847692497db11c3521949b515e658c770be3ae7e Mon Sep 17 00:00:00 2001 From: Nathan Nutter Date: Thu, 5 Nov 2026 01:11:21 -0600 Subject: [PATCH] Leave no blank line at the end of a file after a move A removal already takes the blank lines that follow the declaration it removes, but the declaration that ends a file keeps the blank line above it, so the file ends with a blank line that gofmt rewrites. Take that blank line too, but only when a removal reaches the end of the file, so a removal in the middle of a file keeps the blank line that separates its neighbours. Change-Id: Ia4bfa9da48bbb7d3b959f56d9e7406be91c46d60 --- internal/analysis/methodical/fix.go | 58 +++++++++++++++++++++++++++++ internal/driver/iterate_test.go | 17 +++++++++ 2 files changed, 75 insertions(+) diff --git a/internal/analysis/methodical/fix.go b/internal/analysis/methodical/fix.go index fc2d5cc..5bdff9f 100644 --- a/internal/analysis/methodical/fix.go +++ b/internal/analysis/methodical/fix.go @@ -219,6 +219,7 @@ func makeSuggestedFix(candidates []fixMethod, unsafe map[types.Object]bool) *ana } var edits []analysis.TextEdit for anchor, group := range groups { + trimMovedTail(group) slices.SortFunc(group, func(a, b fixMethod) int { return cmp.Or(cmp.Compare(a.target.order, b.target.order), cmp.Compare(genericRank(a.method.decl), genericRank(b.method.decl)), strings.Compare(a.method.methodName, b.method.methodName)) }) @@ -236,6 +237,63 @@ func makeSuggestedFix(candidates []fixMethod, unsafe map[types.Object]bool) *ana return &analysis.SuggestedFix{Message: "Move and order methods after their types", TextEdits: edits} } +// trimMovedTail takes the blank line before the declarations that a move +// removes from the end of a file. They kept the blank line that separated them +// from the declaration above, which would end the file as a blank line. +func trimMovedTail(group []fixMethod) { + slices.SortFunc(group, func(a, b fixMethod) int { + return cmp.Compare(a.start, b.start) + }) + last := len(group) - 1 + fix := group[last].file + if fix.token.Offset(group[last].end) != len(fix.source) { + return + } + first := last + for isAdjacentToPrevious(group, first) { + first-- + } + limit := 0 + if first > 0 { + limit = fix.token.Offset(group[first-1].end) + } + start := trimBlankTail(fix.source, fix.token.Offset(group[first].start)) + if start > limit { + group[first].start = fix.token.Pos(start) + } +} + +// trimBlankTail moves an offset back over the blank lines before it and leaves +// the newline that ends the line above. +func trimBlankTail(src []byte, offset int) int { + end := offset + for end > 0 && isBlankByte(src[end-1]) { + end-- + } + if n := bytes.IndexByte(src[end:offset], '\n'); n >= 0 { + return end + n + 1 + } + return offset +} + +func isBlankByte(b byte) bool { + switch b { + case '\n', '\r', ' ', '\t': + return true + default: + return false + } +} + +// isAdjacentToPrevious reports whether a removal starts where the one before it +// ends, so both reach back to the same blank line. +func isAdjacentToPrevious(group []fixMethod, i int) bool { + if i == 0 { + return false + } + return group[i-1].end == group[i].start +} + func genericRank(decl *ast.FuncDecl) int { if decl.Type.TypeParams != nil { return 0 diff --git a/internal/driver/iterate_test.go b/internal/driver/iterate_test.go index daff2af..b6d4857 100644 --- a/internal/driver/iterate_test.go +++ b/internal/driver/iterate_test.go @@ -3,6 +3,7 @@ package driver_test import ( "fmt" "go/ast" + "go/format" "os" "os/exec" "path/filepath" @@ -59,6 +60,22 @@ func runFix(t *testing.T, analyzer *analysis.Analyzer, diff bool) (int, string, return code, string(output), string(diagnostics) } +func TestMethodicalFixTrimsTrailingBlankLine(t *testing.T) { + source := "package fix\n\ntype Widget struct{}\n\nfunc (Widget) Z() {}\n\nfunc (Widget) A() {}\n" + fixRepo(t, source) + code, output, diagnostics := runFix(t, methodical.Analyzer, false) + require.Equal(t, 0, code) + require.Empty(t, output) + require.Empty(t, diagnostics) + want := "package fix\n\ntype Widget struct{}\n\nfunc (Widget) A() {}\n\nfunc (Widget) Z() {}\n" + data, err := os.ReadFile("fix.go") + require.NoError(t, err) + require.Equal(t, want, string(data)) + formatted, err := format.Source(data) + require.NoError(t, err) + require.Equal(t, string(formatted), string(data), "the fixed file is not formatted") +} + func TestMethodicalCumulativeFix(t *testing.T) { source := "package fix\n\ntype First struct{}\n\nfunc (First) Z() {}\n\nfunc (First) A() {}\n\ntype Second struct{}\n\nfunc (Second) Z() {}\n\nfunc (Second) A() {}\n" dir := fixRepo(t, source)