Skip to content
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

feat: use problem matchers for GitHub Action format #4685

Merged
merged 3 commits into from
May 2, 2024
Merged
Show file tree
Hide file tree
Changes from 1 commit
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
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'.
//
ldez marked this conversation as resolved.
Show resolved Hide resolved
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
}
123 changes: 115 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 All @@ -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",
Expand Down Expand Up @@ -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",
Expand All @@ -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")
}
Expand All @@ -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")
}
10 changes: 7 additions & 3 deletions pkg/printers/printer_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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,
},
{
Expand All @@ -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)
Expand Down
Loading
Loading