Skip to content

feat: use problem matchers for GitHub Action format #4685

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 3 commits into from
May 2, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
149 changes: 127 additions & 22 deletions pkg/printers/github.go
Original file line number Diff line number Diff line change
@@ -1,28 +1,143 @@
package printers

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

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

type GitHub struct {
w io.Writer
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"`
}

const defaultGithubSeverity = "error"
// 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"`
}

// 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
type GitHub struct {
tempPath string
w io.Writer
}

// 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,
}
}

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

// print each line as: ::error file=app.js,line=10,col=15::Something went wrong
func formatIssueAsGithub(issue *result.Issue) string {
severity := defaultGithubSeverity
func formatIssueAsGitHub(issue *result.Issue) string {
severity := defaultGitHubSeverity
if issue.Severity != "" {
severity = issue.Severity
}
Expand All @@ -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
}
118 changes: 110 additions & 8 deletions pkg/printers/github_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,12 @@ package printers

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

"github.com/stretchr/testify/assert"
Expand Down Expand Up @@ -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",
Expand All @@ -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")
}
Expand All @@ -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")
}
6 changes: 3 additions & 3 deletions pkg/printers/printer_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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,
},
{
Expand All @@ -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)
Expand Down
22 changes: 0 additions & 22 deletions pkg/printers/testdata/golden-github-actions.txt

This file was deleted.

Loading
Loading