Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: new gofmt lint #43

Merged
merged 2 commits into from
Mar 5, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
192 changes: 192 additions & 0 deletions internal/linters/go/gofmt/gofmt.go
Original file line number Diff line number Diff line change
@@ -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)
}
}

}
62 changes: 62 additions & 0 deletions internal/linters/go/gofmt/gofmt_test.go
Original file line number Diff line number Diff line change
@@ -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)
}
}

}

}
}
}
28 changes: 28 additions & 0 deletions internal/linters/go/gofmt/testdata/gofmt_test.txt
Original file line number Diff line number Diff line change
@@ -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
}
1 change: 1 addition & 0 deletions main.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down
Loading