A better go code formatter, and now `make fmt` can run in Windows (#17684)
* go build / format tools * re-format importstokarchuk/v1.17
wxiaoguang
3 years ago
committed by
GitHub
parent
29cc169d20
commit
750a8465f5
@ -0,0 +1,284 @@ |
||||
// Copyright 2021 The Gitea Authors. All rights reserved.
|
||||
// Use of this source code is governed by a MIT-style
|
||||
// license that can be found in the LICENSE file.
|
||||
|
||||
//go:build ignore
|
||||
// +build ignore
|
||||
|
||||
package main |
||||
|
||||
import ( |
||||
"fmt" |
||||
"log" |
||||
"os" |
||||
"os/exec" |
||||
"path/filepath" |
||||
"regexp" |
||||
"strconv" |
||||
"strings" |
||||
|
||||
"code.gitea.io/gitea/build/codeformat" |
||||
) |
||||
|
||||
// Windows has a limitation for command line arguments, the size can not exceed 32KB.
|
||||
// So we have to feed the files to some tools (like gofmt/misspell`) batch by batch
|
||||
|
||||
// We also introduce a `gitea-fmt` command, it does better import formatting than gofmt/goimports
|
||||
|
||||
var optionLogVerbose bool |
||||
|
||||
func logVerbose(msg string, args ...interface{}) { |
||||
if optionLogVerbose { |
||||
log.Printf(msg, args...) |
||||
} |
||||
} |
||||
|
||||
func passThroughCmd(cmd string, args []string) error { |
||||
foundCmd, err := exec.LookPath(cmd) |
||||
if err != nil { |
||||
log.Fatalf("can not find cmd: %s", cmd) |
||||
} |
||||
c := exec.Cmd{ |
||||
Path: foundCmd, |
||||
Args: args, |
||||
Stdin: os.Stdin, |
||||
Stdout: os.Stdout, |
||||
Stderr: os.Stderr, |
||||
} |
||||
return c.Run() |
||||
} |
||||
|
||||
type fileCollector struct { |
||||
dirs []string |
||||
includePatterns []*regexp.Regexp |
||||
excludePatterns []*regexp.Regexp |
||||
batchSize int |
||||
} |
||||
|
||||
func newFileCollector(fileFilter string, batchSize int) (*fileCollector, error) { |
||||
co := &fileCollector{batchSize: batchSize} |
||||
if fileFilter == "go-own" { |
||||
co.dirs = []string{ |
||||
"build", |
||||
"cmd", |
||||
"contrib", |
||||
"integrations", |
||||
"models", |
||||
"modules", |
||||
"routers", |
||||
"services", |
||||
"tools", |
||||
} |
||||
co.includePatterns = append(co.includePatterns, regexp.MustCompile(`.*\.go$`)) |
||||
|
||||
co.excludePatterns = append(co.excludePatterns, regexp.MustCompile(`.*\bbindata\.go$`)) |
||||
co.excludePatterns = append(co.excludePatterns, regexp.MustCompile(`integrations/gitea-repositories-meta`)) |
||||
co.excludePatterns = append(co.excludePatterns, regexp.MustCompile(`integrations/migration-test`)) |
||||
co.excludePatterns = append(co.excludePatterns, regexp.MustCompile(`modules/git/tests`)) |
||||
co.excludePatterns = append(co.excludePatterns, regexp.MustCompile(`models/fixtures`)) |
||||
co.excludePatterns = append(co.excludePatterns, regexp.MustCompile(`models/migrations/fixtures`)) |
||||
co.excludePatterns = append(co.excludePatterns, regexp.MustCompile(`services/gitdiff/testdata`)) |
||||
} |
||||
|
||||
if co.dirs == nil { |
||||
return nil, fmt.Errorf("unknown file-filter: %s", fileFilter) |
||||
} |
||||
return co, nil |
||||
} |
||||
|
||||
func (fc *fileCollector) matchPatterns(path string, regexps []*regexp.Regexp) bool { |
||||
path = strings.ReplaceAll(path, "\\", "/") |
||||
for _, re := range regexps { |
||||
if re.MatchString(path) { |
||||
return true |
||||
} |
||||
} |
||||
return false |
||||
} |
||||
|
||||
func (fc *fileCollector) collectFiles() (res [][]string, err error) { |
||||
var batch []string |
||||
for _, dir := range fc.dirs { |
||||
err = filepath.WalkDir(dir, func(path string, d os.DirEntry, err error) error { |
||||
include := len(fc.includePatterns) == 0 || fc.matchPatterns(path, fc.includePatterns) |
||||
exclude := fc.matchPatterns(path, fc.excludePatterns) |
||||
process := include && !exclude |
||||
if !process { |
||||
if d.IsDir() { |
||||
if exclude { |
||||
logVerbose("exclude dir %s", path) |
||||
return filepath.SkipDir |
||||
} |
||||
// for a directory, if it is not excluded explicitly, we should walk into
|
||||
return nil |
||||
} |
||||
// for a file, we skip it if it shouldn't be processed
|
||||
logVerbose("skip process %s", path) |
||||
return nil |
||||
} |
||||
if d.IsDir() { |
||||
// skip dir, we don't add dirs to the file list now
|
||||
return nil |
||||
} |
||||
if len(batch) >= fc.batchSize { |
||||
res = append(res, batch) |
||||
batch = nil |
||||
} |
||||
batch = append(batch, path) |
||||
return nil |
||||
}) |
||||
if err != nil { |
||||
return nil, err |
||||
} |
||||
} |
||||
res = append(res, batch) |
||||
return res, nil |
||||
} |
||||
|
||||
// substArgFiles expands the {file-list} to a real file list for commands
|
||||
func substArgFiles(args []string, files []string) []string { |
||||
for i, s := range args { |
||||
if s == "{file-list}" { |
||||
newArgs := append(args[:i], files...) |
||||
newArgs = append(newArgs, args[i+1:]...) |
||||
return newArgs |
||||
} |
||||
} |
||||
return args |
||||
} |
||||
|
||||
func exitWithCmdErrors(subCmd string, subArgs []string, cmdErrors []error) { |
||||
for _, err := range cmdErrors { |
||||
if err != nil { |
||||
if exitError, ok := err.(*exec.ExitError); ok { |
||||
exitCode := exitError.ExitCode() |
||||
log.Printf("run command failed (code=%d): %s %v", exitCode, subCmd, subArgs) |
||||
os.Exit(exitCode) |
||||
} else { |
||||
log.Fatalf("run command failed (err=%s) %s %v", err, subCmd, subArgs) |
||||
} |
||||
} |
||||
} |
||||
} |
||||
|
||||
func parseArgs() (mainOptions map[string]string, subCmd string, subArgs []string) { |
||||
mainOptions = map[string]string{} |
||||
for i := 1; i < len(os.Args); i++ { |
||||
arg := os.Args[i] |
||||
if arg == "" { |
||||
break |
||||
} |
||||
if arg[0] == '-' { |
||||
arg = strings.TrimPrefix(arg, "-") |
||||
arg = strings.TrimPrefix(arg, "-") |
||||
fields := strings.SplitN(arg, "=", 2) |
||||
if len(fields) == 1 { |
||||
mainOptions[fields[0]] = "1" |
||||
} else { |
||||
mainOptions[fields[0]] = fields[1] |
||||
} |
||||
} else { |
||||
subCmd = arg |
||||
subArgs = os.Args[i+1:] |
||||
break |
||||
} |
||||
} |
||||
return |
||||
} |
||||
|
||||
func showUsage() { |
||||
fmt.Printf(`Usage: %[1]s [options] {command} [arguments] |
||||
|
||||
Options: |
||||
--verbose |
||||
--file-filter=go-own |
||||
--batch-size=100 |
||||
|
||||
Commands: |
||||
%[1]s gofmt ... |
||||
%[1]s misspell ... |
||||
|
||||
Arguments: |
||||
{file-list} the file list |
||||
|
||||
Example: |
||||
%[1]s gofmt -s -d {file-list} |
||||
|
||||
`, "file-batch-exec") |
||||
} |
||||
|
||||
func newFileCollectorFromMainOptions(mainOptions map[string]string) (fc *fileCollector, err error) { |
||||
fileFilter := mainOptions["file-filter"] |
||||
if fileFilter == "" { |
||||
fileFilter = "go-own" |
||||
} |
||||
batchSize, _ := strconv.Atoi(mainOptions["batch-size"]) |
||||
if batchSize == 0 { |
||||
batchSize = 100 |
||||
} |
||||
|
||||
return newFileCollector(fileFilter, batchSize) |
||||
} |
||||
|
||||
func containsString(a []string, s string) bool { |
||||
for _, v := range a { |
||||
if v == s { |
||||
return true |
||||
} |
||||
} |
||||
return false |
||||
} |
||||
|
||||
func giteaFormatGoImports(files []string) error { |
||||
for _, file := range files { |
||||
if err := codeformat.FormatGoImports(file); err != nil { |
||||
log.Printf("failed to format go imports: %s, err=%v", file, err) |
||||
return err |
||||
} |
||||
} |
||||
return nil |
||||
} |
||||
|
||||
func main() { |
||||
mainOptions, subCmd, subArgs := parseArgs() |
||||
if subCmd == "" { |
||||
showUsage() |
||||
os.Exit(1) |
||||
} |
||||
optionLogVerbose = mainOptions["verbose"] != "" |
||||
|
||||
fc, err := newFileCollectorFromMainOptions(mainOptions) |
||||
if err != nil { |
||||
log.Fatalf("can not create file collector: %s", err.Error()) |
||||
} |
||||
|
||||
fileBatches, err := fc.collectFiles() |
||||
if err != nil { |
||||
log.Fatalf("can not collect files: %s", err.Error()) |
||||
} |
||||
|
||||
processed := 0 |
||||
var cmdErrors []error |
||||
for _, files := range fileBatches { |
||||
if len(files) == 0 { |
||||
break |
||||
} |
||||
substArgs := substArgFiles(subArgs, files) |
||||
logVerbose("batch cmd: %s %v", subCmd, substArgs) |
||||
switch subCmd { |
||||
case "gitea-fmt": |
||||
cmdErrors = append(cmdErrors, passThroughCmd("gofmt", substArgs)) |
||||
if containsString(subArgs, "-w") { |
||||
cmdErrors = append(cmdErrors, giteaFormatGoImports(files)) |
||||
} |
||||
case "misspell": |
||||
cmdErrors = append(cmdErrors, passThroughCmd("misspell", substArgs)) |
||||
default: |
||||
log.Fatalf("unknown cmd: %s %v", subCmd, subArgs) |
||||
} |
||||
processed += len(files) |
||||
} |
||||
|
||||
logVerbose("processed %d files", processed) |
||||
exitWithCmdErrors(subCmd, subArgs, cmdErrors) |
||||
} |
||||
@ -0,0 +1,187 @@ |
||||
// Copyright 2021 The Gitea Authors. All rights reserved.
|
||||
// Use of this source code is governed by a MIT-style
|
||||
// license that can be found in the LICENSE file.
|
||||
|
||||
package codeformat |
||||
|
||||
import ( |
||||
"bytes" |
||||
"errors" |
||||
"io" |
||||
"os" |
||||
"sort" |
||||
"strings" |
||||
) |
||||
|
||||
var importPackageGroupOrders = map[string]int{ |
||||
"": 1, // internal
|
||||
"code.gitea.io/gitea/": 2, |
||||
} |
||||
|
||||
var errInvalidCommentBetweenImports = errors.New("comments between imported packages are invalid, please move comments to the end of the package line") |
||||
|
||||
var importBlockBegin = []byte("\nimport (\n") |
||||
var importBlockEnd = []byte("\n)") |
||||
|
||||
type importLineParsed struct { |
||||
group string |
||||
pkg string |
||||
content string |
||||
} |
||||
|
||||
func parseImportLine(line string) (*importLineParsed, error) { |
||||
il := &importLineParsed{content: line} |
||||
p1 := strings.IndexRune(line, '"') |
||||
if p1 == -1 { |
||||
return nil, errors.New("invalid import line: " + line) |
||||
} |
||||
p1++ |
||||
p := strings.IndexRune(line[p1:], '"') |
||||
if p == -1 { |
||||
return nil, errors.New("invalid import line: " + line) |
||||
} |
||||
p2 := p1 + p |
||||
il.pkg = line[p1:p2] |
||||
|
||||
pDot := strings.IndexRune(il.pkg, '.') |
||||
pSlash := strings.IndexRune(il.pkg, '/') |
||||
if pDot != -1 && pDot < pSlash { |
||||
il.group = "domain-package" |
||||
} |
||||
for groupName := range importPackageGroupOrders { |
||||
if groupName == "" { |
||||
continue // skip internal
|
||||
} |
||||
if strings.HasPrefix(il.pkg, groupName) { |
||||
il.group = groupName |
||||
} |
||||
} |
||||
return il, nil |
||||
} |
||||
|
||||
type importLineGroup []*importLineParsed |
||||
type importLineGroupMap map[string]importLineGroup |
||||
|
||||
func formatGoImports(contentBytes []byte) ([]byte, error) { |
||||
p1 := bytes.Index(contentBytes, importBlockBegin) |
||||
if p1 == -1 { |
||||
return nil, nil |
||||
} |
||||
p1 += len(importBlockBegin) |
||||
p := bytes.Index(contentBytes[p1:], importBlockEnd) |
||||
if p == -1 { |
||||
return nil, nil |
||||
} |
||||
p2 := p1 + p |
||||
|
||||
importGroups := importLineGroupMap{} |
||||
r := bytes.NewBuffer(contentBytes[p1:p2]) |
||||
eof := false |
||||
for !eof { |
||||
line, err := r.ReadString('\n') |
||||
eof = err == io.EOF |
||||
if err != nil && !eof { |
||||
return nil, err |
||||
} |
||||
line = strings.TrimSpace(line) |
||||
if line != "" { |
||||
if strings.HasPrefix(line, "//") || strings.HasPrefix(line, "/*") { |
||||
return nil, errInvalidCommentBetweenImports |
||||
} |
||||
importLine, err := parseImportLine(line) |
||||
if err != nil { |
||||
return nil, err |
||||
} |
||||
importGroups[importLine.group] = append(importGroups[importLine.group], importLine) |
||||
} |
||||
} |
||||
|
||||
var groupNames []string |
||||
for groupName, importLines := range importGroups { |
||||
groupNames = append(groupNames, groupName) |
||||
sort.Slice(importLines, func(i, j int) bool { |
||||
return strings.Compare(importLines[i].pkg, importLines[j].pkg) < 0 |
||||
}) |
||||
} |
||||
|
||||
sort.Slice(groupNames, func(i, j int) bool { |
||||
n1 := groupNames[i] |
||||
n2 := groupNames[j] |
||||
o1 := importPackageGroupOrders[n1] |
||||
o2 := importPackageGroupOrders[n2] |
||||
if o1 != 0 && o2 != 0 { |
||||
return o1 < o2 |
||||
} |
||||
if o1 == 0 && o2 == 0 { |
||||
return strings.Compare(n1, n2) < 0 |
||||
} |
||||
return o1 != 0 |
||||
}) |
||||
|
||||
formattedBlock := bytes.Buffer{} |
||||
for _, groupName := range groupNames { |
||||
hasNormalImports := false |
||||
hasDummyImports := false |
||||
// non-dummy import comes first
|
||||
for _, importLine := range importGroups[groupName] { |
||||
if strings.HasPrefix(importLine.content, "_") { |
||||
hasDummyImports = true |
||||
} else { |
||||
formattedBlock.WriteString("\t" + importLine.content + "\n") |
||||
hasNormalImports = true |
||||
} |
||||
} |
||||
// dummy (_ "pkg") comes later
|
||||
if hasDummyImports { |
||||
if hasNormalImports { |
||||
formattedBlock.WriteString("\n") |
||||
} |
||||
for _, importLine := range importGroups[groupName] { |
||||
if strings.HasPrefix(importLine.content, "_") { |
||||
formattedBlock.WriteString("\t" + importLine.content + "\n") |
||||
} |
||||
} |
||||
} |
||||
formattedBlock.WriteString("\n") |
||||
} |
||||
formattedBlockBytes := bytes.TrimRight(formattedBlock.Bytes(), "\n") |
||||
|
||||
var formattedBytes []byte |
||||
formattedBytes = append(formattedBytes, contentBytes[:p1]...) |
||||
formattedBytes = append(formattedBytes, formattedBlockBytes...) |
||||
formattedBytes = append(formattedBytes, contentBytes[p2:]...) |
||||
return formattedBytes, nil |
||||
} |
||||
|
||||
//FormatGoImports format the imports by our rules (see unit tests)
|
||||
func FormatGoImports(file string) error { |
||||
f, err := os.Open(file) |
||||
if err != nil { |
||||
return err |
||||
} |
||||
var contentBytes []byte |
||||
{ |
||||
defer f.Close() |
||||
contentBytes, err = io.ReadAll(f) |
||||
if err != nil { |
||||
return err |
||||
} |
||||
} |
||||
formattedBytes, err := formatGoImports(contentBytes) |
||||
if err != nil { |
||||
return err |
||||
} |
||||
if formattedBytes == nil { |
||||
return nil |
||||
} |
||||
if bytes.Equal(contentBytes, formattedBytes) { |
||||
return nil |
||||
} |
||||
f, err = os.OpenFile(file, os.O_TRUNC|os.O_WRONLY, 0644) |
||||
if err != nil { |
||||
return err |
||||
} |
||||
defer f.Close() |
||||
_, err = f.Write(formattedBytes) |
||||
return err |
||||
} |
||||
@ -0,0 +1,125 @@ |
||||
// Copyright 2021 The Gitea Authors. All rights reserved.
|
||||
// Use of this source code is governed by a MIT-style
|
||||
// license that can be found in the LICENSE file.
|
||||
|
||||
package codeformat |
||||
|
||||
import ( |
||||
"testing" |
||||
|
||||
"github.com/stretchr/testify/assert" |
||||
) |
||||
|
||||
func TestFormatImportsSimple(t *testing.T) { |
||||
formatted, err := formatGoImports([]byte(` |
||||
package codeformat |
||||
|
||||
import ( |
||||
"github.com/stretchr/testify/assert" |
||||
"testing" |
||||
) |
||||
`)) |
||||
|
||||
expected := ` |
||||
package codeformat |
||||
|
||||
import ( |
||||
"testing" |
||||
|
||||
"github.com/stretchr/testify/assert" |
||||
) |
||||
` |
||||
|
||||
assert.NoError(t, err) |
||||
assert.Equal(t, expected, string(formatted)) |
||||
} |
||||
|
||||
func TestFormatImportsGroup(t *testing.T) { |
||||
// gofmt/goimports won't group the packages, for example, they produce such code:
|
||||
// "bytes"
|
||||
// "image"
|
||||
// (a blank line)
|
||||
// "fmt"
|
||||
// "image/color/palette"
|
||||
// our formatter does better, and these packages are grouped into one.
|
||||
|
||||
formatted, err := formatGoImports([]byte(` |
||||
package test |
||||
|
||||
import ( |
||||
"bytes" |
||||
"fmt" |
||||
"image" |
||||
"image/color/palette" |
||||
|
||||
_ "image/gif" // for processing gif images
|
||||
_ "image/jpeg" // for processing jpeg images
|
||||
_ "image/png" // for processing png images
|
||||
|
||||
"code.gitea.io/other/package" |
||||
|
||||
"code.gitea.io/gitea/modules/setting" |
||||
"code.gitea.io/gitea/modules/util" |
||||
|
||||
"xorm.io/the/package" |
||||
|
||||
"github.com/issue9/identicon" |
||||
"github.com/nfnt/resize" |
||||
"github.com/oliamb/cutter" |
||||
) |
||||
`)) |
||||
|
||||
expected := ` |
||||
package test |
||||
|
||||
import ( |
||||
"bytes" |
||||
"fmt" |
||||
"image" |
||||
"image/color/palette" |
||||
|
||||
_ "image/gif" // for processing gif images
|
||||
_ "image/jpeg" // for processing jpeg images
|
||||
_ "image/png" // for processing png images
|
||||
|
||||
"code.gitea.io/gitea/modules/setting" |
||||
"code.gitea.io/gitea/modules/util" |
||||
|
||||
"code.gitea.io/other/package" |
||||
"github.com/issue9/identicon" |
||||
"github.com/nfnt/resize" |
||||
"github.com/oliamb/cutter" |
||||
"xorm.io/the/package" |
||||
) |
||||
` |
||||
|
||||
assert.NoError(t, err) |
||||
assert.Equal(t, expected, string(formatted)) |
||||
} |
||||
|
||||
func TestFormatImportsInvalidComment(t *testing.T) { |
||||
// why we shouldn't write comments between imports: it breaks the grouping of imports
|
||||
// for example:
|
||||
// "pkg1"
|
||||
// "pkg2"
|
||||
// // a comment
|
||||
// "pkgA"
|
||||
// "pkgB"
|
||||
// the comment splits the packages into two groups, pkg1/2 are sorted separately, pkgA/B are sorted separately
|
||||
// we don't want such code, so the code should be:
|
||||
// "pkg1"
|
||||
// "pkg2"
|
||||
// "pkgA" // a comment
|
||||
// "pkgB"
|
||||
|
||||
_, err := formatGoImports([]byte(` |
||||
package test |
||||
|
||||
import ( |
||||
"image/jpeg" |
||||
// for processing gif images
|
||||
"image/gif" |
||||
) |
||||
`)) |
||||
assert.ErrorIs(t, err, errInvalidCommentBetweenImports) |
||||
} |
||||
@ -0,0 +1,27 @@ |
||||
// Copyright 2021 The Gitea Authors. All rights reserved.
|
||||
// Use of this source code is governed by a MIT-style
|
||||
// license that can be found in the LICENSE file.
|
||||
|
||||
//go:build ignore
|
||||
// +build ignore
|
||||
|
||||
package main |
||||
|
||||
import ( |
||||
"log" |
||||
"os" |
||||
|
||||
"code.gitea.io/gitea/build/codeformat" |
||||
) |
||||
|
||||
func main() { |
||||
if len(os.Args) <= 1 { |
||||
log.Fatalf("Usage: gitea-format-imports [files...]") |
||||
} |
||||
|
||||
for _, file := range os.Args[1:] { |
||||
if err := codeformat.FormatGoImports(file); err != nil { |
||||
log.Fatalf("can not format file %s, err=%v", file, err) |
||||
} |
||||
} |
||||
} |
||||
Some files were not shown because too many files have changed in this diff Show More
Loading…
Reference in new issue