From d10734332f27a66bfcf5b78e00f719ae6d4ca8b5 Mon Sep 17 00:00:00 2001 From: win5do Date: Fri, 10 Jan 2025 15:14:55 +0800 Subject: [PATCH] cmd/goimports: add flags -r: remove blank line in imports -exit-code: exit with a non-zero exit code if files were not already formatted --- cmd/goimports/goimports.go | 15 ++- go/ast/astutil/imports.go | 48 ++++++++ go/ast/astutil/imports_test.go | 197 +++++++++++++++++++++++++++++++++ internal/imports/fix_test.go | 67 +++++++++++ internal/imports/imports.go | 10 +- 5 files changed, 332 insertions(+), 5 deletions(-) diff --git a/cmd/goimports/goimports.go b/cmd/goimports/goimports.go index dcb5023a2e7..0ce1d5df87f 100644 --- a/cmd/goimports/goimports.go +++ b/cmd/goimports/goimports.go @@ -26,10 +26,11 @@ import ( var ( // main operation modes - list = flag.Bool("l", false, "list files whose formatting differs from goimport's") - write = flag.Bool("w", false, "write result to (source) file instead of stdout") - doDiff = flag.Bool("d", false, "display diffs instead of rewriting files") - srcdir = flag.String("srcdir", "", "choose imports as if source code is from `dir`. When operating on a single file, dir may instead be the complete file name.") + list = flag.Bool("l", false, "list files whose formatting differs from goimport's") + write = flag.Bool("w", false, "write result to (source) file instead of stdout") + doDiff = flag.Bool("d", false, "display diffs instead of rewriting files") + srcdir = flag.String("srcdir", "", "choose imports as if source code is from `dir`. When operating on a single file, dir may instead be the complete file name.") + diffExitCode = flag.Bool("exit-code", false, "exit with a non-zero exit code if files were not already formatted") // use in ci lint verbose bool // verbose logging @@ -53,6 +54,7 @@ func init() { flag.BoolVar(&options.AllErrors, "e", false, "report all errors (not just the first 10 on different lines)") flag.StringVar(&options.LocalPrefix, "local", "", "put imports beginning with this string after 3rd-party packages; comma-separated list") flag.BoolVar(&options.FormatOnly, "format-only", false, "if true, don't fix imports and only format. In this mode, goimports is effectively gofmt, with the addition that imports are grouped into sections.") + flag.BoolVar(&options.RegroupImports, "r", false, "remove blank line in imports") } func report(err error) { @@ -144,6 +146,11 @@ func processFile(filename string, in io.Reader, out io.Writer, argType argumentT } if !bytes.Equal(src, res) { + if *diffExitCode { + // change exitCode when not formatted + exitCode = 1 + } + // formatting has changed if *list { fmt.Fprintln(out, filename) diff --git a/go/ast/astutil/imports.go b/go/ast/astutil/imports.go index a6b5ed0a893..1ce0dc991aa 100644 --- a/go/ast/astutil/imports.go +++ b/go/ast/astutil/imports.go @@ -6,8 +6,10 @@ package astutil // import "golang.org/x/tools/go/ast/astutil" import ( + "bytes" "fmt" "go/ast" + "go/parser" "go/token" "strconv" "strings" @@ -488,3 +490,49 @@ func Imports(fset *token.FileSet, f *ast.File) [][]*ast.ImportSpec { return groups } + +// RemoveBlankLineInImport remove blank line in import block. +func RemoveBlankLineInImport(filename string, src []byte) (out []byte, rerr error) { + tokenSet := token.NewFileSet() + astFile, err := parser.ParseFile(tokenSet, filename, src, parser.ParseComments) + if err != nil { + return src, err + } + + if len(astFile.Decls) <= 1 { + return src, nil + } + tokenFile := tokenSet.File(astFile.Pos()) + + for i := 0; i < len(astFile.Decls); i++ { + decl := astFile.Decls[i] + gen, ok := decl.(*ast.GenDecl) + if !ok || gen.Tok != token.IMPORT || declImports(gen, "C") { + continue + } + + if !gen.Lparen.IsValid() { + // Not a block: sorted by default. + continue + } + + lpLine := tokenFile.Line(gen.Lparen) + rpLine := tokenFile.Line(gen.Rparen) + src = removeEmptyLine(src, lpLine, rpLine) + } + + return src, nil +} + +func removeEmptyLine(src []byte, l, r int) []byte { + lines := bytes.Split(src, []byte("\n")) + for i := l; i < r; i++ { + if i < len(lines) && len(bytes.TrimSpace(lines[i])) == 0 { + lines = append(lines[:i], lines[i+1:]...) + i-- + r-- + } + } + + return bytes.Join(lines, []byte("\n")) +} diff --git a/go/ast/astutil/imports_test.go b/go/ast/astutil/imports_test.go index 2a383e467b7..8025f979fb6 100644 --- a/go/ast/astutil/imports_test.go +++ b/go/ast/astutil/imports_test.go @@ -2126,3 +2126,200 @@ func TestUsesImport(t *testing.T) { } } } + +var removeBlankLineInImportTests = []struct { + name string + src string + wantOut string + equalSrc bool +}{ + { + name: "blank line", + src: `package foo + +import ( + "context" + "fmt" + + "github.com/foo/a" + "github.com/foo/b" + + "go.pkg.com/bar/x" + "go.pkg.com/bar/y" + + "github.com/foo/c" + "go.pkg.com/bar/z" +) + +var ( + ctx context.Context + fmt1 fmt.Formatter + a1 a.A + b1 b.A + c1 c.A + x1 x.A + y1 y.A + z1 z.A +) +`, + wantOut: `package foo + +import ( + "context" + "fmt" + "github.com/foo/a" + "github.com/foo/b" + "go.pkg.com/bar/x" + "go.pkg.com/bar/y" + "github.com/foo/c" + "go.pkg.com/bar/z" +) + +var ( + ctx context.Context + fmt1 fmt.Formatter + a1 a.A + b1 b.A + c1 c.A + x1 x.A + y1 y.A + z1 z.A +) +`, + }, + { + name: "no blank line", + src: `package foo + +import ( + "context" + "fmt" + "github.com/foo/a" + "github.com/foo/b" + "github.com/foo/c" + "go.pkg.com/bar/x" + "go.pkg.com/bar/y" + "go.pkg.com/bar/z" +) + +var ( + ctx context.Context + fmt1 fmt.Formatter + a1 a.A + b1 b.A + c1 c.A + x1 x.A + y1 y.A + z1 z.A +) +`, + equalSrc: true, + }, + { + name: "import comment", + src: `package foo + +import ( + "context" + "fmt" + "github.com/foo/a" // comment1 + "github.com/foo/b" + "github.com/foo/c" + // comment2 + "go.pkg.com/bar/x" + "go.pkg.com/bar/y" + "go.pkg.com/bar/z" +) + +var ( + ctx context.Context + fmt1 fmt.Formatter + a1 a.A + b1 b.A + c1 c.A + x1 x.A + y1 y.A + z1 z.A +) +`, + equalSrc: true, + }, + { + name: "import comment blank line", + src: `package foo + +import ( + "context" + "fmt" + + "github.com/foo/a" // comment1 + "github.com/foo/b" + "github.com/foo/c" + + // comment2 + + "go.pkg.com/bar/x" + "go.pkg.com/bar/y" + "go.pkg.com/bar/z" +) + +var ( + ctx context.Context + fmt1 fmt.Formatter + a1 a.A + b1 b.A + c1 c.A + x1 x.A + y1 y.A + z1 z.A +) +`, + wantOut: `package foo + +import ( + "context" + "fmt" + "github.com/foo/a" // comment1 + "github.com/foo/b" + "github.com/foo/c" + // comment2 + "go.pkg.com/bar/x" + "go.pkg.com/bar/y" + "go.pkg.com/bar/z" +) + +var ( + ctx context.Context + fmt1 fmt.Formatter + a1 a.A + b1 b.A + c1 c.A + x1 x.A + y1 y.A + z1 z.A +) +`, + }, +} + +func TestRemoveBlankLineInImport(t *testing.T) { + for _, test := range removeBlankLineInImportTests { + gotOut, err := RemoveBlankLineInImport("test.go", []byte(test.src)) + if err != nil { + t.Errorf("%s: %v", test.name, err) + continue + } + + var wantOut string + if test.equalSrc { + wantOut = test.src + + } else { + wantOut = test.wantOut + } + + if string(gotOut) != wantOut { + t.Errorf("RemoveBlankLineInImport() gotOut = %s, want %s", gotOut, test.wantOut) + } + } +} diff --git a/internal/imports/fix_test.go b/internal/imports/fix_test.go index 02ddd480dfd..a2c2a94acaf 100644 --- a/internal/imports/fix_test.go +++ b/internal/imports/fix_test.go @@ -3085,3 +3085,70 @@ func BenchmarkMatchesPath(b *testing.B) { }) } } + +func TestRegroupImports(t *testing.T) { + const input = `package foo + +import ( + "fmt" + + "github.com/foo/a" + "github.com/foo/b" + + "context" + "go.pkg.com/bar/x" + "go.pkg.com/bar/y" + + "github.com/foo/c" + "go.pkg.com/bar/z" +) + +var ( + ctx context.Context + fmt1 fmt.Formatter + a1 a.A + b1 b.A + c1 c.A + x1 x.A + y1 y.A + z1 z.A +) +` + + const want = `package foo + +import ( + "context" + "fmt" + + "github.com/foo/a" + "github.com/foo/b" + "github.com/foo/c" + "go.pkg.com/bar/x" + "go.pkg.com/bar/y" + "go.pkg.com/bar/z" +) + +var ( + ctx context.Context + fmt1 fmt.Formatter + a1 a.A + b1 b.A + c1 c.A + x1 x.A + y1 y.A + z1 z.A +) +` + + testConfig{ + module: packagestest.Module{ + Name: "foo.com", + Files: fm{ + "p/test.go": input, + }, + }, + }.processTest(t, "foo.com", "p/test.go", nil, &Options{ + RegroupImports: true, + }, want) +} diff --git a/internal/imports/imports.go b/internal/imports/imports.go index 2215a12880a..d805adff996 100644 --- a/internal/imports/imports.go +++ b/internal/imports/imports.go @@ -41,11 +41,19 @@ type Options struct { TabIndent bool // Use tabs for indent (true if nil *Options provided) TabWidth int // Tab width (8 if nil *Options provided) - FormatOnly bool // Disable the insertion and deletion of imports + FormatOnly bool // Disable the insertion and deletion of imports + RegroupImports bool // Remove blank line in imports. } // Process implements golang.org/x/tools/imports.Process with explicit context in opt.Env. func Process(filename string, src []byte, opt *Options) (formatted []byte, err error) { + if opt.RegroupImports { + src, err = astutil.RemoveBlankLineInImport(filename, src) + if err != nil { + return nil, err + } + } + fileSet := token.NewFileSet() var parserMode parser.Mode if opt.Comments {