diff --git a/internal/linters/github.go b/internal/linters/github.go index c6be2445..0dd90137 100644 --- a/internal/linters/github.go +++ b/internal/linters/github.go @@ -173,12 +173,12 @@ func BuildPullRequestCommentBody(linterName string, lintErrs map[string][]Linter message := fmt.Sprintf("[%s] %s\n%s", linterName, lintErr.Message, CommentFooter) - if lintErr.StratLine != 0 { + if lintErr.StartLine != 0 { comments = append(comments, &github.PullRequestComment{ Body: github.String(message), Path: github.String(file), Line: github.Int(lintErr.Line), - StartLine: github.Int(lintErr.StratLine), + StartLine: github.Int(lintErr.StartLine), StartSide: github.String("RIGHT"), Side: github.String("RIGHT"), CommitID: github.String(commitID), diff --git a/internal/linters/go/gofmt/gofmt.go b/internal/linters/go/gofmt/gofmt.go index 47251006..3c471627 100644 --- a/internal/linters/go/gofmt/gofmt.go +++ b/internal/linters/go/gofmt/gofmt.go @@ -82,116 +82,95 @@ func (g *Gofmt) Parse(log *xlog.Logger, output []byte) (map[string][]linters.Lin 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) + //map[filename]map[diff][]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 + } + 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 + re := regexp.MustCompile(`-(\d+),(\d+)`) + match := re.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..611ea947 100644 --- a/internal/linters/go/gofmt/gofmt_test.go +++ b/internal/linters/go/gofmt/gofmt_test.go @@ -5,21 +5,62 @@ import ( "os" "testing" + "github.com/qiniu/reviewbot/internal/linters" "github.com/qiniu/x/xlog" ) -func TestFormatGofmt(t *testing.T) { +func TestGofmtOutput(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) + 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, + }, + }, + }, + } + + var log *xlog.Logger + for _, c := range tc { + outputMap, err := formatGofmtOutput(log, []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/linters.go b/internal/linters/linters.go index 215272ad..a142e4d7 100644 --- a/internal/linters/linters.go +++ b/internal/linters/linters.go @@ -79,8 +79,8 @@ type LinterOutput struct { Column int // Message is the staticcheck Message Message string - //StratLine required when using multi-line comments - StratLine int + //StartLine required when using multi-line comments + StartLine int } // Agent knows necessary information in order to run linters. diff --git a/testdata/gofmt_test.txt b/testdata/gofmt_test.txt index 699336bb..d8d7babc 100644 --- a/testdata/gofmt_test.txt +++ b/testdata/gofmt_test.txt @@ -1,59 +1,28 @@ -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 +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 + - 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) ++ // 222222222 ++ //testerr 33333 + + // rrrrrrrrr } -@@ -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)