From 177d688e54708904d5e6f7bc5ab9da566cdfb878 Mon Sep 17 00:00:00 2001 From: Fernandez Ludovic Date: Wed, 1 May 2024 03:53:37 +0200 Subject: [PATCH] feat: use problem matchers for GitHub Action format --- pkg/printers/github.go | 136 +++++++++++++++--- pkg/printers/github_test.go | 123 ++++++++++++++-- pkg/printers/printer_test.go | 10 +- .../testdata/golden-github-actions.txt | 22 --- pkg/printers/testdata/golden-teamcity.txt | 28 ++++ 5 files changed, 269 insertions(+), 50 deletions(-) delete mode 100644 pkg/printers/testdata/golden-github-actions.txt create mode 100644 pkg/printers/testdata/golden-teamcity.txt diff --git a/pkg/printers/github.go b/pkg/printers/github.go index e396119da1cc..636a735d0774 100644 --- a/pkg/printers/github.go +++ b/pkg/printers/github.go @@ -1,28 +1,102 @@ 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 } -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} } -// 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. + // + // 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 := 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 formatIssueAsGitHub(issue *result.Issue) string { + severity := defaultGitHubSeverity if issue.Severity != "" { severity = issue.Severity } @@ -32,21 +106,49 @@ 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 - } +func storeProblemMatcher() (string, error) { + //nolint:gosec // To be able to clean the file during tests, we need a deterministic filepath. + file, err := os.Create(filepath.Join(os.TempDir(), filenameGitHubActionProblemMatchers)) + 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, + }, + }, + }, + }, } - return nil } diff --git a/pkg/printers/github_test.go b/pkg/printers/github_test.go index c6abd2de87aa..c3c7358b608b 100644 --- a/pkg/printers/github_test.go +++ b/pkg/printers/github_test.go @@ -2,8 +2,13 @@ package printers import ( "bytes" + "fmt" "go/token" + "os" + "path/filepath" + "regexp" "runtime" + "strings" "testing" "github.com/stretchr/testify/assert" @@ -13,6 +18,10 @@ import ( ) func TestGitHub_Print(t *testing.T) { + if runtime.GOOS != "linux" { + t.Skip("Skipping tests because temp folder depends on OS") + } + issues := []result.Issue{ { FromLinter: "linter-a", @@ -43,20 +52,27 @@ func TestGitHub_Print(t *testing.T) { }, } + t.Cleanup(func() { + _ = os.RemoveAll(filepath.Join(t.TempDir(), filenameGitHubActionProblemMatchers)) + }) + buf := new(bytes.Buffer) printer := NewGitHub(buf) 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:: ` 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 +83,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 +104,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") } diff --git a/pkg/printers/printer_test.go b/pkg/printers/printer_test.go index 591b1ab478ff..8c47d0297edb 100644 --- a/pkg/printers/printer_test.go +++ b/pkg/printers/printer_test.go @@ -173,18 +173,22 @@ func TestPrinter_Print_file(t *testing.T) { func TestPrinter_Print_multiple(t *testing.T) { logger := logutils.NewStderrLog("skip") + t.Cleanup(func() { + _ = os.RemoveAll(filepath.Join(t.TempDir(), filenameGitHubActionProblemMatchers)) + }) + var issues []result.Issue unmarshalFile(t, "in-issues.json", &issues) 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 +214,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) diff --git a/pkg/printers/testdata/golden-github-actions.txt b/pkg/printers/testdata/golden-github-actions.txt deleted file mode 100644 index 93704a489d1b..000000000000 --- a/pkg/printers/testdata/golden-github-actions.txt +++ /dev/null @@ -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) diff --git a/pkg/printers/testdata/golden-teamcity.txt b/pkg/printers/testdata/golden-teamcity.txt new file mode 100644 index 000000000000..1d32bd36be6c --- /dev/null +++ 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='']