feat: use problem matchers for GitHub Action format (#4685)

2024-05-02 22:58:31 +02:00 · 2024-05-02 22:58:31 +02:00 · 28b3813c88
commit 28b3813c88
parent 24bcca2ebd
5 changed files with 268 additions and 55 deletions
--- a/pkg/printers/github.go
+++ b/pkg/printers/github.go
@ -1,28 +1,143 @@
 package printers

 import (
+	"encoding/json"
 	"fmt"
 	"io"
+	"os"
 	"path/filepath"

 	"github.com/golangci/golangci-lint/pkg/result"
 )

+const defaultGitHubSeverity = "error"
+
+const filenameGitHubActionProblemMatchers = "golangci-lint-action-problem-matchers.json"
+
+// GitHubProblemMatchers defines the root of problem matchers.
+// - https://github.com/actions/toolkit/blob/main/docs/problem-matchers.md
+// - https://github.com/actions/toolkit/blob/main/docs/commands.md#problem-matchers
+type GitHubProblemMatchers struct {
+	Matchers []GitHubMatcher `json:"problemMatcher,omitempty"`
+}
+
+// GitHubMatcher defines a problem matcher.
+type GitHubMatcher struct {
+	// Owner an ID field that can be used to remove or replace the problem matcher.
+	// **required**
+	Owner string `json:"owner,omitempty"`
+	// Severity indicates the default severity, either 'warning' or 'error' case-insensitive.
+	// Defaults to 'error'.
+	Severity string          `json:"severity,omitempty"`
+	Pattern  []GitHubPattern `json:"pattern,omitempty"`
+}
+
+// GitHubPattern defines a pattern for a problem matcher.
+type GitHubPattern struct {
+	// Regexp the regexp pattern that provides the groups to match against.
+	// **required**
+	Regexp string `json:"regexp,omitempty"`
+	// File a group number containing the file name.
+	File int `json:"file,omitempty"`
+	// FromPath a group number containing a filepath used to root the file (e.g. a project file).
+	FromPath int `json:"fromPath,omitempty"`
+	// Line a group number containing the line number.
+	Line int `json:"line,omitempty"`
+	// Column a group number containing the column information.
+	Column int `json:"column,omitempty"`
+	// Severity a group number containing either 'warning' or 'error' case-insensitive.
+	// Defaults to `error`.
+	Severity int `json:"severity,omitempty"`
+	// Code a group number containing the error code.
+	Code int `json:"code,omitempty"`
+	// Message a group number containing the error message.
+	// **required** at least one pattern must set the message.
+	Message int `json:"message,omitempty"`
+	// Loop whether to loop until a match is not found,
+	// only valid on the last pattern of a multi-pattern matcher.
+	Loop bool `json:"loop,omitempty"`
+}
+
 type GitHub struct {
-	w io.Writer
+	tempPath string
+	w        io.Writer
 }

-const defaultGithubSeverity = "error"
-
-// NewGitHub output format outputs issues according to GitHub actions format:
-// https://docs.github.com/en/actions/using-workflows/workflow-commands-for-github-actions#setting-an-error-message
+// NewGitHub output format outputs issues according to GitHub actions the problem matcher regexp.
 func NewGitHub(w io.Writer) *GitHub {
-	return &GitHub{w: w}
+	return &GitHub{
+		tempPath: filepath.Join(os.TempDir(), filenameGitHubActionProblemMatchers),
+		w:        w,
+	}
 }

-// print each line as: ::error file=app.js,line=10,col=15::Something went wrong
-func formatIssueAsGithub(issue *result.Issue) string {
-	severity := defaultGithubSeverity
+func (p *GitHub) Print(issues []result.Issue) error {
+	// Note: the file with the problem matcher definition should not be removed.
+	// A sleep can mitigate this problem but this will be flaky.
+	//
+	// Result if the file is removed prematurely:
+	// Error: Unable to process command '::add-matcher::/tmp/golangci-lint-action-problem-matchers.json' successfully.
+	// Error: Could not find file '/tmp/golangci-lint-action-problem-matchers.json'.
+	filename, err := p.storeProblemMatcher()
+	if err != nil {
+		return err
+	}
+
+	_, _ = fmt.Fprintln(p.w, "::debug::problem matcher definition file: "+filename)
+
+	_, _ = fmt.Fprintln(p.w, "::add-matcher::"+filename)
+
+	for ind := range issues {
+		_, err := fmt.Fprintln(p.w, formatIssueAsGitHub(&issues[ind]))
+		if err != nil {
+			return err
+		}
+	}
+
+	_, _ = fmt.Fprintln(p.w, "::remove-matcher owner=golangci-lint-action::")
+
+	return nil
+}
+
+func (p *GitHub) storeProblemMatcher() (string, error) {
+	file, err := os.Create(p.tempPath)
+	if err != nil {
+		return "", err
+	}
+
+	defer file.Close()
+
+	err = json.NewEncoder(file).Encode(generateProblemMatcher())
+	if err != nil {
+		return "", err
+	}
+
+	return file.Name(), nil
+}
+
+func generateProblemMatcher() GitHubProblemMatchers {
+	return GitHubProblemMatchers{
+		Matchers: []GitHubMatcher{
+			{
+				Owner:    "golangci-lint-action",
+				Severity: "error",
+				Pattern: []GitHubPattern{
+					{
+						Regexp:   `^([^\s]+)\s+([^:]+):(\d+):(?:(\d+):)?\s+(.+)$`,
+						Severity: 1,
+						File:     2,
+						Line:     3,
+						Column:   4,
+						Message:  5,
+					},
+				},
+			},
+		},
+	}
+}
+
+func formatIssueAsGitHub(issue *result.Issue) string {
+	severity := defaultGitHubSeverity
 	if issue.Severity != "" {
 		severity = issue.Severity
 	}
@ -32,21 +147,11 @@ func formatIssueAsGithub(issue *result.Issue) string {
 	// Otherwise, GitHub won't be able to show the annotations pointing to the file path with backslashes.
 	file := filepath.ToSlash(issue.FilePath())

-	ret := fmt.Sprintf("::%s file=%s,line=%d", severity, file, issue.Line())
+	ret := fmt.Sprintf("%s\t%s:%d:", severity, file, issue.Line())
 	if issue.Pos.Column != 0 {
-		ret += fmt.Sprintf(",col=%d", issue.Pos.Column)
+		ret += fmt.Sprintf("%d:", issue.Pos.Column)
 	}

-	ret += fmt.Sprintf("::%s (%s)", issue.Text, issue.FromLinter)
+	ret += fmt.Sprintf("\t%s (%s)", issue.Text, issue.FromLinter)
 	return ret
 }
-
-func (p *GitHub) Print(issues []result.Issue) error {
-	for ind := range issues {
-		_, err := fmt.Fprintln(p.w, formatIssueAsGithub(&issues[ind]))
-		if err != nil {
-			return err
-		}
-	}
-	return nil
-}
--- a/pkg/printers/github_test.go
+++ b/pkg/printers/github_test.go
@ -2,8 +2,12 @@ package printers

 import (
 	"bytes"
+	"fmt"
 	"go/token"
+	"path/filepath"
+	"regexp"
 	"runtime"
+	"strings"
 	"testing"

 	"github.com/stretchr/testify/assert"
@ -44,19 +48,26 @@ func TestGitHub_Print(t *testing.T) {
 	}

 	buf := new(bytes.Buffer)
+
 	printer := NewGitHub(buf)
+	printer.tempPath = filepath.Join(t.TempDir(), filenameGitHubActionProblemMatchers)

 	err := printer.Print(issues)
 	require.NoError(t, err)

-	expected := `::warning file=path/to/filea.go,line=10,col=4::some issue (linter-a)
-::error file=path/to/fileb.go,line=300,col=9::another issue (linter-b)
+	expected := `::debug::problem matcher definition file: /tmp/golangci-lint-action-problem-matchers.json
+::add-matcher::/tmp/golangci-lint-action-problem-matchers.json
+warning	path/to/filea.go:10:4:	some issue (linter-a)
+error	path/to/fileb.go:300:9:	another issue (linter-b)
+::remove-matcher owner=golangci-lint-action::
 `
+	// To support all the OS.
+	expected = strings.ReplaceAll(expected, "/tmp/golangci-lint-action-problem-matchers.json", printer.tempPath)

 	assert.Equal(t, expected, buf.String())
 }

-func Test_formatIssueAsGithub(t *testing.T) {
+func Test_formatIssueAsGitHub(t *testing.T) {
 	sampleIssue := result.Issue{
 		FromLinter: "sample-linter",
 		Text:       "some issue",
@ -67,13 +78,13 @@ func Test_formatIssueAsGithub(t *testing.T) {
 			Column:   4,
 		},
 	}
-	require.Equal(t, "::error file=path/to/file.go,line=10,col=4::some issue (sample-linter)", formatIssueAsGithub(&sampleIssue))
+	require.Equal(t, "error\tpath/to/file.go:10:4:\tsome issue (sample-linter)", formatIssueAsGitHub(&sampleIssue))

 	sampleIssue.Pos.Column = 0
-	require.Equal(t, "::error file=path/to/file.go,line=10::some issue (sample-linter)", formatIssueAsGithub(&sampleIssue))
+	require.Equal(t, "error\tpath/to/file.go:10:\tsome issue (sample-linter)", formatIssueAsGitHub(&sampleIssue))
 }

-func Test_formatIssueAsGithub_Windows(t *testing.T) {
+func Test_formatIssueAsGitHub_Windows(t *testing.T) {
 	if runtime.GOOS != "windows" {
 		t.Skip("Skipping test on non Windows")
 	}
@ -88,8 +99,99 @@ func Test_formatIssueAsGithub_Windows(t *testing.T) {
 			Column:   4,
 		},
 	}
-	require.Equal(t, "::error file=path/to/file.go,line=10,col=4::some issue (sample-linter)", formatIssueAsGithub(&sampleIssue))
+	require.Equal(t, "error\tpath/to/file.go:10:4:\tsome issue (sample-linter)", formatIssueAsGitHub(&sampleIssue))

 	sampleIssue.Pos.Column = 0
-	require.Equal(t, "::error file=path/to/file.go,line=10::some issue (sample-linter)", formatIssueAsGithub(&sampleIssue))
+	require.Equal(t, "error\tpath/to/file.go:10:\tsome issue (sample-linter)", formatIssueAsGitHub(&sampleIssue))
+}
+
+func Test_generateProblemMatcher(t *testing.T) {
+	pattern := generateProblemMatcher().Matchers[0].Pattern[0]
+
+	exp := regexp.MustCompile(pattern.Regexp)
+
+	testCases := []struct {
+		desc     string
+		line     string
+		expected string
+	}{
+		{
+			desc: "error",
+			line: "error\tpath/to/filea.go:10:4:\tsome issue (sample-linter)",
+			expected: `File: path/to/filea.go
+Line: 10
+Column: 4
+Severity: error
+Message: some issue (sample-linter)`,
+		},
+		{
+			desc: "warning",
+			line: "warning\tpath/to/fileb.go:1:4:\tsome issue (sample-linter)",
+			expected: `File: path/to/fileb.go
+Line: 1
+Column: 4
+Severity: warning
+Message: some issue (sample-linter)`,
+		},
+		{
+			desc: "no column",
+			line: "error\t \tpath/to/fileb.go:40:\t Foo bar",
+			expected: `File: path/to/fileb.go
+Line: 40
+Column: 
+Severity: error
+Message: Foo bar`,
+		},
+	}
+
+	for _, test := range testCases {
+		test := test
+		t.Run(test.desc, func(t *testing.T) {
+			t.Parallel()
+
+			assert.True(t, exp.MatchString(test.line), test.line)
+
+			actual := exp.ReplaceAllString(test.line, createReplacement(&pattern))
+
+			assert.Equal(t, test.expected, actual)
+		})
+	}
+}
+
+func createReplacement(pattern *GitHubPattern) string {
+	var repl []string
+
+	if pattern.File > 0 {
+		repl = append(repl, fmt.Sprintf("File: $%d", pattern.File))
+	}
+
+	if pattern.FromPath > 0 {
+		repl = append(repl, fmt.Sprintf("FromPath: $%d", pattern.FromPath))
+	}
+
+	if pattern.Line > 0 {
+		repl = append(repl, fmt.Sprintf("Line: $%d", pattern.Line))
+	}
+
+	if pattern.Column > 0 {
+		repl = append(repl, fmt.Sprintf("Column: $%d", pattern.Column))
+	}
+
+	if pattern.Severity > 0 {
+		repl = append(repl, fmt.Sprintf("Severity: $%d", pattern.Severity))
+	}
+
+	if pattern.Code > 0 {
+		repl = append(repl, fmt.Sprintf("Code: $%d", pattern.Code))
+	}
+
+	if pattern.Message > 0 {
+		repl = append(repl, fmt.Sprintf("Message: $%d", pattern.Message))
+	}
+
+	if pattern.Loop {
+		repl = append(repl, fmt.Sprintf("Loop: $%v", pattern.Loop))
+	}
+
+	return strings.Join(repl, "\n")
 }
--- a/pkg/printers/printer_test.go
+++ b/pkg/printers/printer_test.go
@ -179,12 +179,12 @@ func TestPrinter_Print_multiple(t *testing.T) {
 	data := &report.Data{}
 	unmarshalFile(t, "in-report-data.json", data)

-	outputPath := filepath.Join(t.TempDir(), "github-actions.txt")
+	outputPath := filepath.Join(t.TempDir(), "teamcity.txt")

 	cfg := &config.Output{
 		Formats: []config.OutputFormat{
 			{
-				Format: "github-actions",
+				Format: "teamcity",
 				Path:   outputPath,
 			},
 			{
@ -210,7 +210,7 @@ func TestPrinter_Print_multiple(t *testing.T) {
 	err = p.Print(issues)
 	require.NoError(t, err)

-	goldenGitHub, err := os.ReadFile(filepath.Join("testdata", "golden-github-actions.txt"))
+	goldenGitHub, err := os.ReadFile(filepath.Join("testdata", "golden-teamcity.txt"))
 	require.NoError(t, err)

 	actual, err := os.ReadFile(outputPath)
--- a/pkg/printers/testdata/golden-github-actions.txt
+++ b/pkg/printers/testdata/golden-github-actions.txt
@ -1,22 +0,0 @@
-::error file=pkg/experimental/myplugin/myplugin.go,line=13,col=1::don't use `init` function (gochecknoinits)
-::error file=pkg/lint/lintersdb/builder_plugin.go,line=59,col=69::hugeParam: settings is heavy (80 bytes); consider passing it by pointer (gocritic)
-::error file=pkg/printers/printer_test.go,line=6::File is not `goimports`-ed with -local github.com/golangci/golangci-lint (goimports)
-::error file=pkg/config/issues.go,line=107,col=13::struct of size 144 bytes could be of size 128 bytes (maligned)
-::error file=pkg/config/linters_settings.go,line=200,col=22::struct of size 3144 bytes could be of size 3096 bytes (maligned)
-::error file=pkg/config/linters_settings.go,line=383,col=25::struct of size 72 bytes could be of size 64 bytes (maligned)
-::error file=pkg/config/linters_settings.go,line=470,col=22::struct of size 72 bytes could be of size 56 bytes (maligned)
-::error file=pkg/config/linters_settings.go,line=482,col=23::struct of size 136 bytes could be of size 128 bytes (maligned)
-::error file=pkg/config/linters_settings.go,line=584,col=27::struct of size 64 bytes could be of size 56 bytes (maligned)
-::error file=pkg/config/linters_settings.go,line=591,col=20::struct of size 88 bytes could be of size 80 bytes (maligned)
-::error file=pkg/config/linters_settings.go,line=710,col=25::struct of size 40 bytes could be of size 32 bytes (maligned)
-::error file=pkg/config/linters_settings.go,line=762,col=21::struct of size 112 bytes could be of size 104 bytes (maligned)
-::error file=pkg/config/linters_settings.go,line=787,col=23::struct of size 32 bytes could be of size 24 bytes (maligned)
-::error file=pkg/config/linters_settings.go,line=817,col=23::struct of size 40 bytes could be of size 32 bytes (maligned)
-::error file=pkg/config/linters_settings.go,line=902,col=25::struct of size 80 bytes could be of size 72 bytes (maligned)
-::error file=pkg/config/linters_settings.go,line=928,col=18::struct of size 112 bytes could be of size 96 bytes (maligned)
-::error file=pkg/config/run.go,line=6,col=10::struct of size 168 bytes could be of size 160 bytes (maligned)
-::error file=pkg/lint/linter/config.go,line=36,col=13::struct of size 128 bytes could be of size 120 bytes (maligned)
-::error file=pkg/golinters/govet_test.go,line=70,col=23::struct of size 96 bytes could be of size 88 bytes (maligned)
-::error file=pkg/result/processors/diff.go,line=17,col=11::struct of size 64 bytes could be of size 56 bytes (maligned)
-::warning file=pkg/experimental/myplugin/myplugin.go,line=49,col=14::unused-parameter: parameter 'pass' seems to be unused, consider removing or renaming it as _ (revive)
-::error file=pkg/commands/run.go,line=47,col=7::const `defaultFileMode` is unused (unused)
--- a/pkg/printers/testdata/golden-teamcity.txt
+++ b/pkg/printers/testdata/golden-teamcity.txt
@ -0,0 +1,28 @@
+##teamcity[inspectionType id='gochecknoinits' name='gochecknoinits' description='gochecknoinits' category='Golangci-lint reports']
+##teamcity[inspection typeId='gochecknoinits' message='don|'t use `init` function' file='pkg/experimental/myplugin/myplugin.go' line='13' SEVERITY='']
+##teamcity[inspectionType id='gocritic' name='gocritic' description='gocritic' category='Golangci-lint reports']
+##teamcity[inspection typeId='gocritic' message='hugeParam: settings is heavy (80 bytes); consider passing it by pointer' file='pkg/lint/lintersdb/builder_plugin.go' line='59' SEVERITY='']
+##teamcity[inspectionType id='goimports' name='goimports' description='goimports' category='Golangci-lint reports']
+##teamcity[inspection typeId='goimports' message='File is not `goimports`-ed with -local github.com/golangci/golangci-lint' file='pkg/printers/printer_test.go' line='6' SEVERITY='']
+##teamcity[inspectionType id='maligned' name='maligned' description='maligned' category='Golangci-lint reports']
+##teamcity[inspection typeId='maligned' message='struct of size 144 bytes could be of size 128 bytes' file='pkg/config/issues.go' line='107' SEVERITY='']
+##teamcity[inspection typeId='maligned' message='struct of size 3144 bytes could be of size 3096 bytes' file='pkg/config/linters_settings.go' line='200' SEVERITY='']
+##teamcity[inspection typeId='maligned' message='struct of size 72 bytes could be of size 64 bytes' file='pkg/config/linters_settings.go' line='383' SEVERITY='']
+##teamcity[inspection typeId='maligned' message='struct of size 72 bytes could be of size 56 bytes' file='pkg/config/linters_settings.go' line='470' SEVERITY='']
+##teamcity[inspection typeId='maligned' message='struct of size 136 bytes could be of size 128 bytes' file='pkg/config/linters_settings.go' line='482' SEVERITY='']
+##teamcity[inspection typeId='maligned' message='struct of size 64 bytes could be of size 56 bytes' file='pkg/config/linters_settings.go' line='584' SEVERITY='']
+##teamcity[inspection typeId='maligned' message='struct of size 88 bytes could be of size 80 bytes' file='pkg/config/linters_settings.go' line='591' SEVERITY='']
+##teamcity[inspection typeId='maligned' message='struct of size 40 bytes could be of size 32 bytes' file='pkg/config/linters_settings.go' line='710' SEVERITY='']
+##teamcity[inspection typeId='maligned' message='struct of size 112 bytes could be of size 104 bytes' file='pkg/config/linters_settings.go' line='762' SEVERITY='']
+##teamcity[inspection typeId='maligned' message='struct of size 32 bytes could be of size 24 bytes' file='pkg/config/linters_settings.go' line='787' SEVERITY='']
+##teamcity[inspection typeId='maligned' message='struct of size 40 bytes could be of size 32 bytes' file='pkg/config/linters_settings.go' line='817' SEVERITY='']
+##teamcity[inspection typeId='maligned' message='struct of size 80 bytes could be of size 72 bytes' file='pkg/config/linters_settings.go' line='902' SEVERITY='']
+##teamcity[inspection typeId='maligned' message='struct of size 112 bytes could be of size 96 bytes' file='pkg/config/linters_settings.go' line='928' SEVERITY='']
+##teamcity[inspection typeId='maligned' message='struct of size 168 bytes could be of size 160 bytes' file='pkg/config/run.go' line='6' SEVERITY='']
+##teamcity[inspection typeId='maligned' message='struct of size 128 bytes could be of size 120 bytes' file='pkg/lint/linter/config.go' line='36' SEVERITY='']
+##teamcity[inspection typeId='maligned' message='struct of size 96 bytes could be of size 88 bytes' file='pkg/golinters/govet_test.go' line='70' SEVERITY='']
+##teamcity[inspection typeId='maligned' message='struct of size 64 bytes could be of size 56 bytes' file='pkg/result/processors/diff.go' line='17' SEVERITY='']
+##teamcity[inspectionType id='revive' name='revive' description='revive' category='Golangci-lint reports']
+##teamcity[inspection typeId='revive' message='unused-parameter: parameter |'pass|' seems to be unused, consider removing or renaming it as _' file='pkg/experimental/myplugin/myplugin.go' line='49' SEVERITY='WARNING']
+##teamcity[inspectionType id='unused' name='unused' description='unused' category='Golangci-lint reports']
+##teamcity[inspection typeId='unused' message='const `defaultFileMode` is unused' file='pkg/commands/run.go' line='47' SEVERITY='']