From a65eedd5043b3002a577f08807c7e2d259131402 Mon Sep 17 00:00:00 2001 From: wangcheng Date: Mon, 26 Feb 2024 18:26:41 +0800 Subject: [PATCH 1/2] feat: gofmt lint --- internal/linters/go/gofmt/gofmt.go | 212 ++++++++++++++++++++++++ internal/linters/go/gofmt/gofmt_test.go | 25 +++ main.go | 1 + testdata/gofmt_test.txt | 59 +++++++ 4 files changed, 297 insertions(+) create mode 100644 internal/linters/go/gofmt/gofmt.go create mode 100644 internal/linters/go/gofmt/gofmt_test.go create mode 100644 testdata/gofmt_test.txt diff --git a/internal/linters/go/gofmt/gofmt.go b/internal/linters/go/gofmt/gofmt.go new file mode 100644 index 00000000..47251006 --- /dev/null +++ b/internal/linters/go/gofmt/gofmt.go @@ -0,0 +1,212 @@ +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 succeeded") + } + return b, nil +} + +func (g *Gofmt) Parse(log *xlog.Logger, output []byte) (map[string][]linters.LinterOutput, error) { + return formatGofmtOutput(log, output) +} + +//formatGofmtOutput formats the output of gofmt +//example: +// diff internal/linters/go/golangci-lint/golangci-lint.go.orig internal/linters/go/golangci-lint/golangci-lint.go +// --- internal/linters/go/golangci-lint/golangci-lint.go.orig +// +++ internal/linters/go/golangci-lint/golangci-lint.go +// @@ -17,7 +17,7 @@ + +// var lintName = "golangci-lint" + +// - // test3333 +// +// test3333 +// func init() { +// linters.RegisterCodeReviewHandler(lintName, golangciLintHandler) +// } +// @@ -33,7 +33,7 @@ +// // turn off compile errors by default +// linterConfig.Args = append([]string{}, "-debug.no-compile-errors=true", "./...") +// } +// - //test 4444444 +// + //test 4444444 +// output, err := executor.Run(log, linterConfig.Args...) +// if err != nil { +// log.Errorf("golangci-lint run failed: %v", err) + +//output: map[file][]linters.LinterOutput +// map[internal/linters/go/golangci-lint/golangci-lint.go:[ +//{internal/linters/go/golangci-lint/golangci-lint.go 24 7 +// var lintName = "golangci-lint" + +// - // test3333 +// +// test3333 +// func init() { +// linters.RegisterCodeReviewHandler(lintName, golangciLintHandler) +// }} +// {internal/linters/go/golangci-lint/golangci-lint.go 40 7 +// // turn off compile errors by default +// linterConfig.Args = append([]string{}, "-debug.no-compile-errors=true", "./...") +// } +// - //test 4444444 +// + //test 4444444 +// output, err := executor.Run(log, linterConfig.Args...) +// if err != nil { +// log.Errorf("golangci-lint run failed: %v", err) +// }]] + +func formatGofmtOutput(log *xlog.Logger, output []byte) (map[string][]linters.LinterOutput, error) { + lines := strings.Split(string(output), "\n") + var result = make(map[string][]linters.LinterOutput) + fileErr := make(map[string][]string) + var filename string + for _, line := range lines { + if strings.HasPrefix(line, "diff") { + fields := strings.Fields(line) + if len(fields) >= 3 { + filename = fields[2] + } + fileErr[filename] = []string{} + } else if filename != "" { + fileErr[filename] = append(fileErr[filename], line) + } + } + + for filename, errmsgs := range fileErr { + var message string + var locationLine, lineNumber int64 + var nonFirstTime bool + for _, errmsg := range errmsgs { + if strings.HasPrefix(errmsg, "@@") { + //Add the previous set of error messages + if nonFirstTime { + message += "```" + addGofmtOutput(result, filename, locationLine, lineNumber, message) + } + + //Parameter Reset + message = " Is your code not properly formatted? Here are some suggestions below\n```suggestion" + locationLine = 0 + lineNumber = 0 + nonFirstTime = true + + //Extract row information + re := regexp.MustCompile(`-(\d+),(\d+)`) + match := re.FindStringSubmatch(errmsg) + if len(match) > 2 { + locationLine, _ = strconv.ParseInt(match[1], 10, 64) + lineNumber, _ = strconv.ParseInt(match[2], 10, 64) + } + + } else if strings.HasPrefix(errmsg, "+") { + message += " \n " + strings.TrimLeft(errmsg, "+") + } + } + nonFirstTime = false + //Add once to the tail + message += "\n```" + addGofmtOutput(result, filename, locationLine, lineNumber, message) + + } + + return result, nil +} + +func addGofmtOutput(result map[string][]linters.LinterOutput, filename string, locationLine, lineNumber int64, message string) { + output := &linters.LinterOutput{ + File: filename, + Line: int(locationLine + lineNumber - 1), + Column: int(lineNumber), + Message: message, + StratLine: int(locationLine + 2), + } + 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..c21a95af --- /dev/null +++ b/internal/linters/go/gofmt/gofmt_test.go @@ -0,0 +1,25 @@ +package gofmt + +import ( + "fmt" + "os" + "testing" + + "github.com/qiniu/x/xlog" +) + +func TestFormatGofmt(t *testing.T) { + content, err := os.ReadFile("../../../../testdata/gofmt_test.txt") + if err != nil { + fmt.Println("无法读取文件:", err) + return + } + result, _ := formatGofmtOutput(&xlog.Logger{}, content) + for key, value := range result { + fmt.Printf("filename : %s \n ", key) + for _, v := range value { + fmt.Println("message: \n", v) + } + } + fmt.Println(result) +} 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" diff --git a/testdata/gofmt_test.txt b/testdata/gofmt_test.txt new file mode 100644 index 00000000..699336bb --- /dev/null +++ b/testdata/gofmt_test.txt @@ -0,0 +1,59 @@ +diff hunk.go.orig hunk.go +--- hunk.go.orig ++++ hunk.go +@@ -1,12 +1,12 @@ + /* + Copyright 2024 Qiniu Cloud (qiniu.com). +- ++ + Licensed under the Apache License, Version 2.0 (the "License"); + you may not use this file except in compliance with the License. + You may obtain a copy of the License at +- ++ + http://www.apache.org/licenses/LICENSE-2.0 +- ++ + Unless required by applicable law or agreed to in writing, software + distributed under the License is distributed on an "AS IS" BASIS, + WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +diff hunk_test.go.orig hunk_test.go +--- hunk_test.go.orig ++++ hunk_test.go +@@ -1,12 +1,12 @@ + /* + Copyright 2024 Qiniu Cloud (qiniu.com). +- ++ + Licensed under the Apache License, Version 2.0 (the "License"); + you may not use this file except in compliance with the License. + You may obtain a copy of the License at +- ++ + http://www.apache.org/licenses/LICENSE-2.0 +- ++ + Unless required by applicable law or agreed to in writing, software + distributed under the License is distributed on an "AS IS" BASIS, + WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +diff internal/linters/go/golangci-lint/golangci-lint.go.orig internal/linters/go/golangci-lint/golangci-lint.go +--- internal/linters/go/golangci-lint/golangci-lint.go.orig ++++ internal/linters/go/golangci-lint/golangci-lint.go +@@ -17,7 +17,7 @@ + + var lintName = "golangci-lint" + +- // test3333 ++// test3333 + func init() { + linters.RegisterCodeReviewHandler(lintName, golangciLintHandler) + } +@@ -33,7 +33,7 @@ + // turn off compile errors by default + linterConfig.Args = append([]string{}, "-debug.no-compile-errors=true", "./...") + } +- //test 4444444 ++ //test 4444444 + output, err := executor.Run(log, linterConfig.Args...) + if err != nil { + log.Errorf("golangci-lint run failed: %v", err) From 904f884c1fe1971b7d5c715ad3b193e48e0fee2a Mon Sep 17 00:00:00 2001 From: wwcchh0123 <1170142389@qq.com> Date: Fri, 1 Mar 2024 18:53:52 +0800 Subject: [PATCH 2/2] Rewrite the parsing logic of gofmt --- internal/linters/go/gofmt/gofmt.go | 158 ++++++++---------- internal/linters/go/gofmt/gofmt_test.go | 59 +++++-- .../linters/go/gofmt/testdata/gofmt_test.txt | 28 ++++ testdata/gofmt_test.txt | 59 ------- 4 files changed, 145 insertions(+), 159 deletions(-) create mode 100644 internal/linters/go/gofmt/testdata/gofmt_test.txt delete mode 100644 testdata/gofmt_test.txt diff --git a/internal/linters/go/gofmt/gofmt.go b/internal/linters/go/gofmt/gofmt.go index 47251006..c9fa6600 100644 --- a/internal/linters/go/gofmt/gofmt.go +++ b/internal/linters/go/gofmt/gofmt.go @@ -73,125 +73,105 @@ func (g *Gofmt) Run(log *xlog.Logger, args ...string) ([]byte, error) { log.Errorf("gofmt run with status: %v, mark and continue", err) return b, err } else { - log.Infof("gofmt succeeded") + log.Infof("gofmt running succeeded") } return b, nil } func (g *Gofmt) Parse(log *xlog.Logger, output []byte) (map[string][]linters.LinterOutput, error) { - return formatGofmtOutput(log, output) + log.Infof("gofmt output is being parsed") + return formatGofmtOutput(output) } -//formatGofmtOutput formats the output of gofmt -//example: -// diff internal/linters/go/golangci-lint/golangci-lint.go.orig internal/linters/go/golangci-lint/golangci-lint.go -// --- internal/linters/go/golangci-lint/golangci-lint.go.orig -// +++ internal/linters/go/golangci-lint/golangci-lint.go -// @@ -17,7 +17,7 @@ - -// var lintName = "golangci-lint" - -// - // test3333 -// +// test3333 -// func init() { -// linters.RegisterCodeReviewHandler(lintName, golangciLintHandler) -// } -// @@ -33,7 +33,7 @@ -// // turn off compile errors by default -// linterConfig.Args = append([]string{}, "-debug.no-compile-errors=true", "./...") -// } -// - //test 4444444 -// + //test 4444444 -// output, err := executor.Run(log, linterConfig.Args...) -// if err != nil { -// log.Errorf("golangci-lint run failed: %v", err) - -//output: map[file][]linters.LinterOutput -// map[internal/linters/go/golangci-lint/golangci-lint.go:[ -//{internal/linters/go/golangci-lint/golangci-lint.go 24 7 -// var lintName = "golangci-lint" - -// - // test3333 -// +// test3333 -// func init() { -// linters.RegisterCodeReviewHandler(lintName, golangciLintHandler) -// }} -// {internal/linters/go/golangci-lint/golangci-lint.go 40 7 -// // turn off compile errors by default -// linterConfig.Args = append([]string{}, "-debug.no-compile-errors=true", "./...") -// } -// - //test 4444444 -// + //test 4444444 -// output, err := executor.Run(log, linterConfig.Args...) -// if err != nil { -// log.Errorf("golangci-lint run failed: %v", err) -// }]] - -func formatGofmtOutput(log *xlog.Logger, output []byte) (map[string][]linters.LinterOutput, error) { - lines := strings.Split(string(output), "\n") +func formatGofmtOutput(output []byte) (map[string][]linters.LinterOutput, error) { + var result = make(map[string][]linters.LinterOutput) - fileErr := make(map[string][]string) + 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] = []string{} + fileErr[filename] = map[string][]string{} } else if filename != "" { - fileErr[filename] = append(fileErr[filename], line) + 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 { - var message string - var locationLine, lineNumber int64 - var nonFirstTime bool - for _, errmsg := range errmsgs { - if strings.HasPrefix(errmsg, "@@") { - //Add the previous set of error messages - if nonFirstTime { - message += "```" - addGofmtOutput(result, filename, locationLine, lineNumber, message) - } + 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) + } - //Parameter Reset - message = " Is your code not properly formatted? Here are some suggestions below\n```suggestion" - locationLine = 0 - lineNumber = 0 - nonFirstTime = true - - //Extract row information - re := regexp.MustCompile(`-(\d+),(\d+)`) - match := re.FindStringSubmatch(errmsg) - if len(match) > 2 { - locationLine, _ = strconv.ParseInt(match[1], 10, 64) - lineNumber, _ = strconv.ParseInt(match[2], 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{} } - - } else if strings.HasPrefix(errmsg, "+") { - message += " \n " + strings.TrimLeft(errmsg, "+") } } - nonFirstTime = false - //Add once to the tail - message += "\n```" - addGofmtOutput(result, filename, locationLine, lineNumber, message) - } return result, nil } -func addGofmtOutput(result map[string][]linters.LinterOutput, filename string, locationLine, lineNumber int64, message string) { - output := &linters.LinterOutput{ - File: filename, - Line: int(locationLine + lineNumber - 1), - Column: int(lineNumber), - Message: message, - StratLine: int(locationLine + 2), +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 { diff --git a/internal/linters/go/gofmt/gofmt_test.go b/internal/linters/go/gofmt/gofmt_test.go index c21a95af..0ae03994 100644 --- a/internal/linters/go/gofmt/gofmt_test.go +++ b/internal/linters/go/gofmt/gofmt_test.go @@ -1,25 +1,62 @@ package gofmt import ( - "fmt" "os" "testing" - "github.com/qiniu/x/xlog" + "github.com/qiniu/reviewbot/internal/linters" ) -func TestFormatGofmt(t *testing.T) { - content, err := os.ReadFile("../../../../testdata/gofmt_test.txt") +func TestGofmtOutput(t *testing.T) { + content, err := os.ReadFile("./testdata/gofmt_test.txt") if err != nil { - fmt.Println("无法读取文件:", err) + t.Errorf("open file failed ,the err is : %v", err) return } - result, _ := formatGofmtOutput(&xlog.Logger{}, content) - for key, value := range result { - fmt.Printf("filename : %s \n ", key) - for _, v := range value { - fmt.Println("message: \n", v) + 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) + } + } + + } + } } - fmt.Println(result) } 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/testdata/gofmt_test.txt b/testdata/gofmt_test.txt deleted file mode 100644 index 699336bb..00000000 --- a/testdata/gofmt_test.txt +++ /dev/null @@ -1,59 +0,0 @@ -diff hunk.go.orig hunk.go ---- hunk.go.orig -+++ hunk.go -@@ -1,12 +1,12 @@ - /* - Copyright 2024 Qiniu Cloud (qiniu.com). -- -+ - Licensed under the Apache License, Version 2.0 (the "License"); - you may not use this file except in compliance with the License. - You may obtain a copy of the License at -- -+ - http://www.apache.org/licenses/LICENSE-2.0 -- -+ - Unless required by applicable law or agreed to in writing, software - distributed under the License is distributed on an "AS IS" BASIS, - WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -diff hunk_test.go.orig hunk_test.go ---- hunk_test.go.orig -+++ hunk_test.go -@@ -1,12 +1,12 @@ - /* - Copyright 2024 Qiniu Cloud (qiniu.com). -- -+ - Licensed under the Apache License, Version 2.0 (the "License"); - you may not use this file except in compliance with the License. - You may obtain a copy of the License at -- -+ - http://www.apache.org/licenses/LICENSE-2.0 -- -+ - Unless required by applicable law or agreed to in writing, software - distributed under the License is distributed on an "AS IS" BASIS, - WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -diff internal/linters/go/golangci-lint/golangci-lint.go.orig internal/linters/go/golangci-lint/golangci-lint.go ---- internal/linters/go/golangci-lint/golangci-lint.go.orig -+++ internal/linters/go/golangci-lint/golangci-lint.go -@@ -17,7 +17,7 @@ - - var lintName = "golangci-lint" - -- // test3333 -+// test3333 - func init() { - linters.RegisterCodeReviewHandler(lintName, golangciLintHandler) - } -@@ -33,7 +33,7 @@ - // turn off compile errors by default - linterConfig.Args = append([]string{}, "-debug.no-compile-errors=true", "./...") - } -- //test 4444444 -+ //test 4444444 - output, err := executor.Run(log, linterConfig.Args...) - if err != nil { - log.Errorf("golangci-lint run failed: %v", err)