Skip to content

Commit

Permalink
feat: use problem matchers for GitHub Action format
Browse files Browse the repository at this point in the history
  • Loading branch information
ldez committed May 1, 2024
1 parent 87db2a3 commit 2fa6aa0
Show file tree
Hide file tree
Showing 4 changed files with 259 additions and 47 deletions.
136 changes: 119 additions & 17 deletions pkg/printers/github.go
Original file line number Diff line number Diff line change
@@ -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
}
Expand All @@ -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
}
119 changes: 111 additions & 8 deletions pkg/printers/github_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,13 @@ package printers

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

"github.com/stretchr/testify/assert"
Expand Down Expand Up @@ -43,20 +48,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",
Expand All @@ -67,13 +79,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")
}
Expand All @@ -88,8 +100,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")
}
4 changes: 4 additions & 0 deletions pkg/printers/printer_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -173,6 +173,10 @@ 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)

Expand Down
47 changes: 25 additions & 22 deletions pkg/printers/testdata/golden-github-actions.txt
Original file line number Diff line number Diff line change
@@ -1,22 +1,25 @@
::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)
::debug::problem matcher definition file: /tmp/golangci-lint-action-problem-matchers.json
::add-matcher::/tmp/golangci-lint-action-problem-matchers.json
error pkg/experimental/myplugin/myplugin.go:13:1: don't use `init` function (gochecknoinits)
error pkg/lint/lintersdb/builder_plugin.go:59:69: hugeParam: settings is heavy (80 bytes); consider passing it by pointer (gocritic)
error pkg/printers/printer_test.go:6: File is not `goimports`-ed with -local github.com/golangci/golangci-lint (goimports)
error pkg/config/issues.go:107:13: struct of size 144 bytes could be of size 128 bytes (maligned)
error pkg/config/linters_settings.go:200:22: struct of size 3144 bytes could be of size 3096 bytes (maligned)
error pkg/config/linters_settings.go:383:25: struct of size 72 bytes could be of size 64 bytes (maligned)
error pkg/config/linters_settings.go:470:22: struct of size 72 bytes could be of size 56 bytes (maligned)
error pkg/config/linters_settings.go:482:23: struct of size 136 bytes could be of size 128 bytes (maligned)
error pkg/config/linters_settings.go:584:27: struct of size 64 bytes could be of size 56 bytes (maligned)
error pkg/config/linters_settings.go:591:20: struct of size 88 bytes could be of size 80 bytes (maligned)
error pkg/config/linters_settings.go:710:25: struct of size 40 bytes could be of size 32 bytes (maligned)
error pkg/config/linters_settings.go:762:21: struct of size 112 bytes could be of size 104 bytes (maligned)
error pkg/config/linters_settings.go:787:23: struct of size 32 bytes could be of size 24 bytes (maligned)
error pkg/config/linters_settings.go:817:23: struct of size 40 bytes could be of size 32 bytes (maligned)
error pkg/config/linters_settings.go:902:25: struct of size 80 bytes could be of size 72 bytes (maligned)
error pkg/config/linters_settings.go:928:18: struct of size 112 bytes could be of size 96 bytes (maligned)
error pkg/config/run.go:6:10: struct of size 168 bytes could be of size 160 bytes (maligned)
error pkg/lint/linter/config.go:36:13: struct of size 128 bytes could be of size 120 bytes (maligned)
error pkg/golinters/govet_test.go:70:23: struct of size 96 bytes could be of size 88 bytes (maligned)
error pkg/result/processors/diff.go:17:11: struct of size 64 bytes could be of size 56 bytes (maligned)
warning pkg/experimental/myplugin/myplugin.go:49:14: unused-parameter: parameter 'pass' seems to be unused, consider removing or renaming it as _ (revive)
error pkg/commands/run.go:47:7: const `defaultFileMode` is unused (unused)
::remove-matcher owner=golangci-lint-action::

0 comments on commit 2fa6aa0

Please sign in to comment.