A better go code formatter, and now `make fmt` can run in Windows (#17684)

* go build / format tools
* re-format imports

4 files changed, 623 insertions, 0 deletions

diff --git a/build/code-batch-process.go b/build/code-batch-process.go
new file mode 100644
index 0000000000..4759baf906
--- /dev/null
+++ b/build/code-batch-process.go
@@ -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)
+}
diff --git a/build/codeformat/formatimports.go b/build/codeformat/formatimports.go
new file mode 100644
index 0000000000..c6427f6a99
--- /dev/null
+++ b/build/codeformat/formatimports.go
@@ -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
+}
diff --git a/build/codeformat/formatimports_test.go b/build/codeformat/formatimports_test.go
new file mode 100644
index 0000000000..d308353bda
--- /dev/null
+++ b/build/codeformat/formatimports_test.go
@@ -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)
+}
diff --git a/build/gitea-format-imports.go b/build/gitea-format-imports.go
new file mode 100644
index 0000000000..67c8397b2d
--- /dev/null
+++ b/build/gitea-format-imports.go
@@ -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)
+		}
+	}
+}