refactor: output parser v2

internal/linters/go/golangci_lint/golangci_lint.go

@@ -40,24 +40,34 @@ func golangciLintHandler(log *xlog.Logger, a linters.Agent) error {
 }
 
 func parser(log *xlog.Logger, output []byte) (map[string][]linters.LinterOutput, []string) {
-	var lineParser = func(line string) (*linters.LinterOutput, error) {
-		if strings.HasSuffix(line, "(typecheck)") {
-			// refer: https://github.com/qiniu/reviewbot/issues/82#issuecomment-2002340788
-			return nil, fmt.Errorf("skip golangci-lint typecheck error: %s", line)
+	log.Infof("golangci-lint raw output: %v", string(output))
+
+	var trainer = func(o linters.LinterOutput) (*linters.LinterOutput, []string) {
+		// Perhaps it may not be precise enough？
+		// refer: https://golangci-lint.run/usage/linters/
+		if strings.Contains(o.Message, "(typecheck)") {
+			unexpected := fmt.Sprintf("%s:%d:%d: %s", o.File, o.Line, o.Column, o.Message)
+			return nil, []string{unexpected}
 		}
 
+		return &o, nil
+	}
+
+	var unexpected []string
+	rawResults, rawUnexpected := linters.ParseV2(log, output, trainer)
+	for _, unexpected := range rawUnexpected {
 		// skip the warning level log
 		// example: level=warning msg="[linters_context] copyloopvar: this linter is disabled because the Go version (1.18) of your project is lower than Go 1.22"
 		// the warning level log is not a real lint error, so we need to skip it
-		if strings.Contains(line, "level=warning") {
-			log.Warnf("skip golangci-lint warning: %s", line)
-			return nil, nil
+		if strings.Contains(unexpected, "level=warning") {
+			log.Warnf("skip golangci-lint warning: %s", unexpected)
+			continue
 		}
-
-		return linters.GeneralLineParser(line)
+		unexpected = strings.TrimSpace(unexpected)
 	}
 
-	return linters.Parse(log, output, lineParser)
+	log.Infof("golangci-lint results: %v", rawResults)
+	return rawResults, unexpected
 }
 
 // argsApply is used to set the default parameters for golangci-lint
@@ -214,6 +224,7 @@ func workDirApply(a linters.Agent) linters.Agent {
 	}
 
 	a.LinterConfig.WorkDir = path.Dir(gomodPath)
+	log.Infof("go mod tidy workdir: %v", a.LinterConfig.WorkDir)
 
 	// Execute go mod tidy when go.mod exists.
 	cmd := exec.Command("go", "mod", "tidy")

internal/linters/go/golangci_lint/golangci_lint_test.go

@@ -394,4 +394,5 @@ func TestWorkDirApply(t *testing.T) {
 			}
 		})
 	}
+
 }

internal/linters/linters.go

@@ -175,7 +175,7 @@ func ExecRun(a Agent) ([]byte, error) {
 	c.Env = append(os.Environ(), fmt.Sprintf("ARTIFACT=%s", artifact))
 	c.Env = append(c.Env, a.LinterConfig.Env...)
 
-	log.Infof("run command: %v", c)
+	log.Infof("run command: %v, workdir: %v, env: %v", c, c.Dir, c.Env)
 	output, execErr := c.CombinedOutput()
 
 	// read all files under the artifact dir
@@ -190,7 +190,7 @@ func ExecRun(a Agent) ([]byte, error) {
 		if file.IsDir() {
 			continue
 		}
-
+		log.Infof("artifact file: %v", file.Name())
 		content, err := os.ReadFile(fmt.Sprintf("%s/%s", artifact, file.Name()))
 		if err != nil {
 			return nil, err
@@ -342,6 +342,90 @@ func Parse(log *xlog.Logger, output []byte, lineParser LineParser) (results map[
 	return
 }
 
+// trainer is a function that trains the linter output.
+// It returns the trained linter output and unexpected lines.
+type trainer func(LinterOutput) (LinterOutput, []string)
+
+func ParseV2(log *xlog.Logger, output []byte, trainer func(LinterOutput) (*LinterOutput, []string)) (map[string][]LinterOutput, []string) {
+	pattern := `(.*?):(\d+):(\d+)?:?`
+	regex := regexp.MustCompile(pattern)
+	indices := regex.FindAllStringSubmatchIndex(string(output), -1)
+	if len(indices) == 0 {
+		return nil, strings.Split(string(output), "\n")
+	}
+
+	var unexpected []string
+	var prefix string
+	if len(indices) > 0 && indices[0][0] > 0 {
+		prefix = strings.TrimSpace(string(output[:indices[0][0]]))
+		unexpected = append(unexpected, strings.Split(prefix, "\n")...)
+	}
+
+	var results = make(map[string][]LinterOutput, len(indices))
+	for i := 0; i < len(indices); i++ {
+		issue := LinterOutput{
+			File: string(output[indices[i][2]:indices[i][3]]),
+		}
+
+		line, err := strconv.ParseInt(string(output[indices[i][4]:indices[i][5]]), 10, 64)
+		if err != nil {
+			log.Errorf("unexpected line number: %s, err: %v", string(output[indices[i][4]:indices[i][5]]), err)
+			continue
+		}
+		issue.Line = int(line)
+
+		var msgStart = indices[i][5] + 1
+		if indices[i][6] != -1 && indices[i][7] != -1 {
+			column, err := strconv.ParseInt(string(output[indices[i][6]:indices[i][7]]), 10, 64)
+			if err != nil {
+				log.Errorf("unexpected column number: %s, err: %v", string(output[indices[i][6]:indices[i][7]]), err)
+				continue
+			}
+			issue.Column = int(column)
+			msgStart = indices[i][7] + 1
+		}
+
+		if msgStart < len(output) {
+			if i+1 < len(indices) {
+				issue.Message = strings.TrimSpace(string(output[msgStart:indices[i+1][0]]))
+			} else {
+				issue.Message = strings.TrimSpace(string(output[msgStart:]))
+			}
+		}
+
+		if trainer != nil {
+			t, u := trainer(issue)
+			if len(u) > 0 {
+				unexpected = append(unexpected, u...)
+			}
+
+			if t == nil {
+				// skip this issue
+				continue
+			}
+			issue = *t
+		}
+
+		if outputs, ok := results[issue.File]; !ok {
+			results[issue.File] = []LinterOutput{issue}
+		} else {
+			// remove duplicate
+			var existed bool
+			for _, v := range outputs {
+				if v.File == issue.File && v.Line == issue.Line && v.Column == issue.Column && v.Message == issue.Message {
+					existed = true
+					break
+				}
+			}
+			if !existed {
+				results[issue.File] = append(outputs, issue)
+			}
+		}
+	}
+
+	return results, unexpected
+}
+
 // common format LinterLine
 func GeneralLineParser(line string) (*LinterOutput, error) {
 	pattern := `^(.*?):(\d+):(\d+)?:? (.*)$`

internal/linters/linters_test.go

@@ -23,6 +23,7 @@ import (
 	"testing"
 
 	"github.com/qiniu/reviewbot/config"
+	"github.com/qiniu/x/xlog"
 )
 
 func TestFormatStaticcheckLine(t *testing.T) {
@@ -236,3 +237,169 @@ func TestExecRun(t *testing.T) {
 		})
 	}
 }
+
+func TestParseV2(t *testing.T) {
+	tcs := []struct {
+		id         string
+		input      []byte
+		trainer    func(LinterOutput) (*LinterOutput, []string)
+		expected   map[string][]LinterOutput
+		unexpected []string
+	}{
+		{
+			id:      "case1 - general case",
+			input:   []byte("file.txt:6:7: message\nfile2.txt:6:7: message\nfile2.txt:7:9: message\n"),
+			trainer: nil,
+			expected: map[string][]LinterOutput{
+				"file.txt": []LinterOutput{
+					{
+						File:    "file.txt",
+						Line:    6,
+						Column:  7,
+						Message: "message",
+					},
+				},
+				"file2.txt": []LinterOutput{
+					{
+						File:    "file2.txt",
+						Line:    6,
+						Column:  7,
+						Message: "message",
+					},
+					{
+						File:    "file2.txt",
+						Line:    7,
+						Column:  9,
+						Message: "message",
+					},
+				},
+			},
+			unexpected: nil,
+		},
+		{
+			id: "case2 - complex case",
+			input: []byte(`
+level=warning msg="[linters_context] copyloopvar"
+level=error msg="[linters_context]"
+golangci_lint.go:16:1: warning: (gochecknoglobals)
+xxx.go:18:1: warning: xxxx
+xxx.gox (gochecknoglobals)
+golangci_lint.go:17:1: warning: (gochecknoglobals)
+io/upv2/upform/internal/mime/multipart/multipart_test.go:744:7: wrapperFunc: use strings.ReplaceAll method in ` + "`strings.Replace(`--08b84578eabc563dcba96\n7a945cd<...>: application/octet-stream\n\n`, \"\\n\", \"\\r\\n\", -1)` (gocritic)"),
+			trainer: nil,
+			expected: map[string][]LinterOutput{
+				"golangci_lint.go": []LinterOutput{
+					{
+						File:    "golangci_lint.go",
+						Line:    16,
+						Column:  1,
+						Message: "warning: (gochecknoglobals)",
+					},
+					{
+						File:    "golangci_lint.go",
+						Line:    17,
+						Column:  1,
+						Message: "warning: (gochecknoglobals)",
+					},
+				},
+				"xxx.go": []LinterOutput{
+					{
+						File:    "xxx.go",
+						Line:    18,
+						Column:  1,
+						Message: "warning: xxxx\nxxx.gox (gochecknoglobals)",
+					},
+				},
+				"io/upv2/upform/internal/mime/multipart/multipart_test.go": []LinterOutput{
+					{
+						File:    "io/upv2/upform/internal/mime/multipart/multipart_test.go",
+						Line:    744,
+						Column:  7,
+						Message: "wrapperFunc: use strings.ReplaceAll method in `strings.Replace(`--08b84578eabc563dcba96\n7a945cd<...>: application/octet-stream\n\n`, \"\\n\", \"\\r\\n\", -1)` (gocritic)",
+					},
+				},
+			},
+			unexpected: []string{"level=warning msg=\"[linters_context] copyloopvar\"", "level=error msg=\"[linters_context]\""},
+		},
+		{
+			id:         "case3 - invalid input",
+			input:      []byte("case3 - invalid input"),
+			trainer:    nil,
+			expected:   nil,
+			unexpected: []string{"case3 - invalid input"},
+		},
+		{
+			id:      "case4 - msg reached max length",
+			input:   []byte("file.txt:6:7: message\nfile2.txt:6:7: "),
+			trainer: nil,
+			expected: map[string][]LinterOutput{
+				"file.txt": []LinterOutput{
+					{
+						File:    "file.txt",
+						Line:    6,
+						Column:  7,
+						Message: "message",
+					},
+				},
+				"file2.txt": []LinterOutput{
+					{
+						File:    "file2.txt",
+						Line:    6,
+						Column:  7,
+						Message: "",
+					},
+				},
+			},
+			unexpected: nil,
+		},
+		{
+			id:      "case5 - duplicate",
+			input:   []byte("file.txt:6:7: message\nfile.txt:6:7: message\n"),
+			trainer: nil,
+			expected: map[string][]LinterOutput{
+				"file.txt": []LinterOutput{
+					{
+						File:    "file.txt",
+						Line:    6,
+						Column:  7,
+						Message: "message",
+					},
+				},
+			},
+			unexpected: nil,
+		},
+		{
+			id:    "case6 - with trainer",
+			input: []byte("file.txt:6:7: message\nfile2.txt:7:9: message\n"),
+			trainer: func(o LinterOutput) (*LinterOutput, []string) {
+				if o.Line == 6 {
+					return nil, []string{fmt.Sprintf("%s:%d:%d: %s", o.File, o.Line, o.Column, o.Message)}
+				}
+				return &o, nil
+			},
+			expected: map[string][]LinterOutput{
+				"file2.txt": []LinterOutput{
+					{
+						File:    "file2.txt",
+						Line:    7,
+						Column:  9,
+						Message: "message",
+					},
+				},
+			},
+			unexpected: []string{"file.txt:6:7: message"},
+		},
+	}
+	for _, tc := range tcs {
+		t.Run(tc.id, func(t *testing.T) {
+			got, unexpected := ParseV2(xlog.New("ut"), tc.input, tc.trainer)
+			if !reflect.DeepEqual(tc.expected, got) {
+				t.Errorf("expected: %v, got: %v", tc.expected, got)
+			}
+
+			if !reflect.DeepEqual(tc.unexpected, unexpected) {
+				t.Errorf("expected: %v, got: %v", tc.unexpected, unexpected)
+			}
+		})
+	}
+}