Rewrite the parsing logic of gofmt

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),

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 {

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)
 }

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.

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)