diff --git a/internal/linters/go/gofmt/gofmt.go b/internal/linters/go/gofmt/gofmt.go new file mode 100644 index 00000000..c9fa6600 --- /dev/null +++ b/internal/linters/go/gofmt/gofmt.go @@ -0,0 +1,192 @@ +package gofmt + +import ( + "os/exec" + "regexp" + "strconv" + "strings" + + "github.com/qiniu/reviewbot/internal/linters" + "github.com/qiniu/x/log" + "github.com/qiniu/x/xlog" +) + +var lintName = "gofmt" + +func init() { + linters.RegisterPullRequestHandler(lintName, gofmtHandler) +} + +func gofmtHandler(log *xlog.Logger, a linters.Agent) error { + + if linters.IsEmpty(a.LinterConfig.Args...) { + a.LinterConfig.Args = append([]string{}, "-d", "./") + } + + executor, err := NewgofmtExecutor(a.LinterConfig.WorkDir) + if err != nil { + log.Errorf("init gofmt executor failed: %v", err) + return err + } + + output, err := executor.Run(log, a.LinterConfig.Args...) + if err != nil { + log.Errorf("gofmt run failed: %v", err) + return err + } + parsedOutput, err := executor.Parse(log, output) + if err != nil { + log.Errorf("gofmt parse output failed: %v", err) + return err + } + + return linters.Report(log, a, parsedOutput) +} + +type Gofmt struct { + dir string + gofmt string + execute func(dir, command string, args ...string) ([]byte, error) +} + +func NewgofmtExecutor(dir string) (linters.Linter, error) { + log.Infof("gofmt executor init") + g, err := exec.LookPath("gofmt") + if err != nil { + return nil, err + } + return &Gofmt{ + dir: dir, + gofmt: g, + execute: func(dir, command string, args ...string) ([]byte, error) { + c := exec.Command(command, args...) + c.Dir = dir + log.Printf("final command: %v \n", c) + return c.Output() + }, + }, nil +} + +func (g *Gofmt) Run(log *xlog.Logger, args ...string) ([]byte, error) { + b, err := g.execute(g.dir, g.gofmt, args...) + if err != nil { + log.Errorf("gofmt run with status: %v, mark and continue", err) + return b, err + } else { + log.Infof("gofmt running succeeded") + } + return b, nil +} + +func (g *Gofmt) Parse(log *xlog.Logger, output []byte) (map[string][]linters.LinterOutput, error) { + log.Infof("gofmt output is being parsed") + return formatGofmtOutput(output) +} + +func formatGofmtOutput(output []byte) (map[string][]linters.LinterOutput, error) { + + var result = make(map[string][]linters.LinterOutput) + lines := strings.Split(string(output), "\n") + //map[$filename]map[$diffname][]string + fileErr := make(map[string]map[string][]string) + //filename eg. test/test.go + var filename string + //diffname eg. @@ -19,5 +19,5 @@ + var diffname string + for _, line := range lines { + if strings.HasPrefix(line, "diff") { + fields := strings.Fields(line) + if len(fields) >= 3 { + filename = fields[2] + } + fileErr[filename] = map[string][]string{} + } else if filename != "" { + if strings.HasPrefix(line, "@@") { + diffname = line + fileErr[filename][diffname] = []string{} + + } else if diffname != "" && !strings.HasPrefix(line, "-") { + fileErr[filename][diffname] = append(fileErr[filename][diffname], line) + } + } + } + + type eachDiffInfo struct { + // diffLineCount is means that how many diff lines + diffLineCount int + // firstDiffLine refers to the first line of diff lines + firstDiffLine int + // fixedMessage refers to the diff fixed lines + fixedMessage string + } + + regexpDiff := regexp.MustCompile(`-(\d+),(\d+)`) + for filename, errmsgs := range fileErr { + for diffname, errmsg := range errmsgs { + //"diffStartLine" refers to the starting line of a diff context, indicating the beginning position of a diff block. + var diffStartLine int64 + match := regexpDiff.FindStringSubmatch(diffname) + if len(match) > 2 { + diffStartLine, _ = strconv.ParseInt(match[1], 10, 64) + } + + var currentInfo eachDiffInfo + for i, line := range errmsg { + if strings.HasPrefix(line, "+") { + if currentInfo.firstDiffLine == 0 { + currentInfo.firstDiffLine = i + 1 + } + currentInfo.diffLineCount++ + currentInfo.fixedMessage += strings.TrimLeft(line, "+") + "\n" + } else { + if currentInfo.fixedMessage != "" { + finalMessage := " Is your code not properly formatted? Here are some suggestions below\n```suggestion\n" + currentInfo.fixedMessage + "```" + addGofmtOutput(result, filename, diffStartLine, int64(currentInfo.firstDiffLine), int64(currentInfo.diffLineCount), finalMessage) + } + currentInfo = eachDiffInfo{} + } + } + } + } + + return result, nil +} + +func addGofmtOutput(result map[string][]linters.LinterOutput, filename string, diffStartLine, firstDiffLine, diffLineCount int64, message string) { + var output *linters.LinterOutput + //If diffLineCount==1, set a single-line comment on GitHub; otherwise, set a multi-line comment. + if diffLineCount == 1 { + output = &linters.LinterOutput{ + File: filename, + Line: int(diffStartLine + firstDiffLine - 1), + Column: int(diffLineCount), + Message: message, + } + } else { + output = &linters.LinterOutput{ + File: filename, + Line: int(diffStartLine + firstDiffLine + diffLineCount - 2), + Column: int(diffLineCount), + Message: message, + StartLine: int(diffStartLine + firstDiffLine - 1), + } + } + + if outs, ok := result[output.File]; !ok { + result[output.File] = []linters.LinterOutput{*output} + } else { + // remove duplicate + var existed bool + for _, v := range outs { + if v.File == output.File && v.Line == output.Line && v.Column == output.Column && v.Message == output.Message { + existed = true + break + } + } + + if !existed { + result[output.File] = append(result[output.File], *output) + } + } + +} diff --git a/internal/linters/go/gofmt/gofmt_test.go b/internal/linters/go/gofmt/gofmt_test.go new file mode 100644 index 00000000..0ae03994 --- /dev/null +++ b/internal/linters/go/gofmt/gofmt_test.go @@ -0,0 +1,62 @@ +package gofmt + +import ( + "os" + "testing" + + "github.com/qiniu/reviewbot/internal/linters" +) + +func TestGofmtOutput(t *testing.T) { + content, err := os.ReadFile("./testdata/gofmt_test.txt") + if err != nil { + t.Errorf("open file failed ,the err is : %v", err) + return + } + tc := []struct { + input []byte + expected []linters.LinterOutput + }{ + { + content, + []linters.LinterOutput{ + { + File: "testfile/staticcheck.go", + Line: 7, + Column: 1, + Message: "", + }, + { + File: "testfile/test.go", + Line: 9, + Column: 4, + Message: "", + StartLine: 6, + }, + }, + }, + } + for _, c := range tc { + outputMap, err := formatGofmtOutput([]byte(c.input)) + for _, outputs := range outputMap { + for i, output := range outputs { + + if err != nil { + t.Errorf("unexpected error: %v", err) + } + + if output.StartLine != 0 { + if output.File != c.expected[1].File || output.StartLine != c.expected[1].StartLine || output.Line != c.expected[1].Line || output.Column != c.expected[1].Column { + t.Errorf("expected: %v, got: %v", c.expected[i], output) + } + } else { + if output.File != c.expected[0].File || output.Line != c.expected[0].Line || output.Column != c.expected[0].Column { + t.Errorf("expected: %v, got: %v", c.expected[0], output) + } + } + + } + + } + } +} diff --git a/internal/linters/go/gofmt/testdata/gofmt_test.txt b/internal/linters/go/gofmt/testdata/gofmt_test.txt new file mode 100644 index 00000000..d8d7babc --- /dev/null +++ b/internal/linters/go/gofmt/testdata/gofmt_test.txt @@ -0,0 +1,28 @@ +diff testfile/staticcheck.go.orig testfile/staticcheck.go +--- testfile/staticcheck.go.orig ++++ testfile/staticcheck.go +@@ -4,5 +4,5 @@ + + func testunnuser() { + fmt.Println("unused") +- // wrong format ++ // wrong format + } +diff testfile/test.go.orig testfile/test.go +--- testfile/test.go.orig ++++ testfile/test.go +@@ -3,10 +3,10 @@ + import "fmt" + + func test2() { +- //testerr 33333 +- +- // 222222222 +- //testerr 33333 ++ //testerr 33333 ++ ++ // 222222222 ++ //testerr 33333 + + // rrrrrrrrr + } diff --git a/main.go b/main.go index 459a867b..b0be1e68 100644 --- a/main.go +++ b/main.go @@ -31,6 +31,7 @@ import ( // linters import _ "github.com/qiniu/reviewbot/internal/linters/git-flow/commit-check" + _ "github.com/qiniu/reviewbot/internal/linters/go/gofmt" _ "github.com/qiniu/reviewbot/internal/linters/go/golangci_lint" _ "github.com/qiniu/reviewbot/internal/linters/go/staticcheck" _ "github.com/qiniu/reviewbot/internal/linters/lua/luacheck"