From 980a911773a522b423226f76ab7c9f98e480f764 Mon Sep 17 00:00:00 2001 From: Ludovic Fernandez Date: Fri, 31 Jan 2025 14:19:21 +0100 Subject: [PATCH] fix: sanitize severities by output format (#5359) --- pkg/logutils/logutils.go | 8 +- pkg/printers/checkstyle.go | 72 ++++++++----- pkg/printers/checkstyle_test.go | 57 +++++++++- pkg/printers/codeclimate.go | 70 ++++++------ pkg/printers/codeclimate_test.go | 38 +++++-- pkg/printers/html.go | 2 + pkg/printers/json.go | 5 +- pkg/printers/json_test.go | 2 +- pkg/printers/junitxml.go | 62 ++++++----- pkg/printers/junitxml_test.go | 2 +- pkg/printers/printer.go | 57 +++++++--- pkg/printers/sarif.go | 125 ++++++++++++---------- pkg/printers/sarif_test.go | 52 ++++++--- pkg/printers/tab.go | 5 +- pkg/printers/tab_test.go | 2 +- pkg/printers/teamcity.go | 31 ++++-- pkg/printers/teamcity_test.go | 41 +++++-- pkg/printers/testdata/golden-teamcity.txt | 42 ++++---- pkg/printers/text.go | 9 +- pkg/printers/text_test.go | 2 +- 20 files changed, 454 insertions(+), 230 deletions(-) diff --git a/pkg/logutils/logutils.go b/pkg/logutils/logutils.go index 99af10cbed5d..0454d7927162 100644 --- a/pkg/logutils/logutils.go +++ b/pkg/logutils/logutils.go @@ -36,8 +36,12 @@ const ( // Printers. const ( - DebugKeyTabPrinter = "tab_printer" - DebugKeyTextPrinter = "text_printer" + DebugKeyCheckstylePrinter = "checkstyle_printer" + DebugKeyCodeClimatePrinter = "codeclimate_printer" + DebugKeySarifPrinter = "sarif_printer" + DebugKeyTabPrinter = "tab_printer" + DebugKeyTeamCityPrinter = "teamcity_printer" + DebugKeyTextPrinter = "text_printer" ) // Processors. diff --git a/pkg/printers/checkstyle.go b/pkg/printers/checkstyle.go index e32eef7f51f5..4dca4564be69 100644 --- a/pkg/printers/checkstyle.go +++ b/pkg/printers/checkstyle.go @@ -9,39 +9,34 @@ import ( "github.com/go-xmlfmt/xmlfmt" "golang.org/x/exp/maps" + "github.com/golangci/golangci-lint/pkg/logutils" "github.com/golangci/golangci-lint/pkg/result" ) const defaultCheckstyleSeverity = "error" -type checkstyleOutput struct { - XMLName xml.Name `xml:"checkstyle"` - Version string `xml:"version,attr"` - Files []*checkstyleFile `xml:"file"` -} - -type checkstyleFile struct { - Name string `xml:"name,attr"` - Errors []*checkstyleError `xml:"error"` -} - -type checkstyleError struct { - Column int `xml:"column,attr"` - Line int `xml:"line,attr"` - Message string `xml:"message,attr"` - Severity string `xml:"severity,attr"` - Source string `xml:"source,attr"` -} - +// Checkstyle prints issues in the Checkstyle format. +// https://checkstyle.org/config.html type Checkstyle struct { - w io.Writer + log logutils.Log + w io.Writer + sanitizer severitySanitizer } -func NewCheckstyle(w io.Writer) *Checkstyle { - return &Checkstyle{w: w} +func NewCheckstyle(log logutils.Log, w io.Writer) *Checkstyle { + return &Checkstyle{ + log: log.Child(logutils.DebugKeyCheckstylePrinter), + w: w, + sanitizer: severitySanitizer{ + // https://checkstyle.org/config.html#Severity + // https://checkstyle.org/property_types.html#SeverityLevel + allowedSeverities: []string{"ignore", "info", "warning", defaultCheckstyleSeverity}, + defaultSeverity: defaultCheckstyleSeverity, + }, + } } -func (p Checkstyle) Print(issues []result.Issue) error { +func (p *Checkstyle) Print(issues []result.Issue) error { out := checkstyleOutput{ Version: "5.0", } @@ -59,22 +54,22 @@ func (p Checkstyle) Print(issues []result.Issue) error { files[issue.FilePath()] = file } - severity := defaultCheckstyleSeverity - if issue.Severity != "" { - severity = issue.Severity - } - newError := &checkstyleError{ Column: issue.Column(), Line: issue.Line(), Message: issue.Text, Source: issue.FromLinter, - Severity: severity, + Severity: p.sanitizer.Sanitize(issue.Severity), } file.Errors = append(file.Errors, newError) } + err := p.sanitizer.Err() + if err != nil { + p.log.Infof("%v", err) + } + out.Files = maps.Values(files) sort.Slice(out.Files, func(i, j int) bool { @@ -93,3 +88,22 @@ func (p Checkstyle) Print(issues []result.Issue) error { return nil } + +type checkstyleOutput struct { + XMLName xml.Name `xml:"checkstyle"` + Version string `xml:"version,attr"` + Files []*checkstyleFile `xml:"file"` +} + +type checkstyleFile struct { + Name string `xml:"name,attr"` + Errors []*checkstyleError `xml:"error"` +} + +type checkstyleError struct { + Column int `xml:"column,attr"` + Line int `xml:"line,attr"` + Message string `xml:"message,attr"` + Severity string `xml:"severity,attr"` + Source string `xml:"source,attr"` +} diff --git a/pkg/printers/checkstyle_test.go b/pkg/printers/checkstyle_test.go index 9c2691311b56..23d77839515d 100644 --- a/pkg/printers/checkstyle_test.go +++ b/pkg/printers/checkstyle_test.go @@ -9,6 +9,7 @@ import ( "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" + "github.com/golangci/golangci-lint/pkg/logutils" "github.com/golangci/golangci-lint/pkg/result" ) @@ -41,15 +42,67 @@ func TestCheckstyle_Print(t *testing.T) { Column: 9, }, }, + { + FromLinter: "linter-c", + Severity: "", + Text: "without severity", + SourceLines: []string{ + "func foo() {", + "\tfmt.Println(\"bar\")", + "}", + }, + Pos: token.Position{ + Filename: "path/to/filec.go", + Offset: 5, + Line: 300, + Column: 10, + }, + }, + { + FromLinter: "linter-d", + Severity: "foo", + Text: "unknown severity", + SourceLines: []string{ + "func foo() {", + "\tfmt.Println(\"bar\")", + "}", + }, + Pos: token.Position{ + Filename: "path/to/filed.go", + Offset: 5, + Line: 300, + Column: 11, + }, + }, } buf := new(bytes.Buffer) - printer := NewCheckstyle(buf) + + log := logutils.NewStderrLog(logutils.DebugKeyEmpty) + log.SetLevel(logutils.LogLevelDebug) + + printer := NewCheckstyle(log, buf) err := printer.Print(issues) require.NoError(t, err) - expected := "\n\n\n \n \n \n \n \n \n\n" + expected := ` + + + + + + + + + + + + + + + +` assert.Equal(t, expected, strings.ReplaceAll(buf.String(), "\r", "")) } diff --git a/pkg/printers/codeclimate.go b/pkg/printers/codeclimate.go index 49b59f8e3c77..7739778e1350 100644 --- a/pkg/printers/codeclimate.go +++ b/pkg/printers/codeclimate.go @@ -3,48 +3,38 @@ package printers import ( "encoding/json" "io" - "slices" + "github.com/golangci/golangci-lint/pkg/logutils" "github.com/golangci/golangci-lint/pkg/result" ) const defaultCodeClimateSeverity = "critical" -// CodeClimateIssue is a subset of the Code Climate spec. -// https://github.com/codeclimate/platform/blob/master/spec/analyzers/SPEC.md#data-types -// It is just enough to support GitLab CI Code Quality. -// https://docs.gitlab.com/ee/ci/testing/code_quality.html#code-quality-report-format -type CodeClimateIssue struct { - Description string `json:"description"` - CheckName string `json:"check_name"` - Severity string `json:"severity,omitempty"` - Fingerprint string `json:"fingerprint"` - Location struct { - Path string `json:"path"` - Lines struct { - Begin int `json:"begin"` - } `json:"lines"` - } `json:"location"` -} - +// CodeClimate prints issues in the Code Climate format. +// https://github.com/codeclimate/platform/blob/master/spec/analyzers/SPEC.md type CodeClimate struct { - w io.Writer - - allowedSeverities []string + log logutils.Log + w io.Writer + sanitizer severitySanitizer } -func NewCodeClimate(w io.Writer) *CodeClimate { +func NewCodeClimate(log logutils.Log, w io.Writer) *CodeClimate { return &CodeClimate{ - w: w, - allowedSeverities: []string{"info", "minor", "major", defaultCodeClimateSeverity, "blocker"}, + log: log.Child(logutils.DebugKeyCodeClimatePrinter), + w: w, + sanitizer: severitySanitizer{ + // https://github.com/codeclimate/platform/blob/master/spec/analyzers/SPEC.md#data-types + allowedSeverities: []string{"info", "minor", "major", defaultCodeClimateSeverity, "blocker"}, + defaultSeverity: defaultCodeClimateSeverity, + }, } } -func (p CodeClimate) Print(issues []result.Issue) error { +func (p *CodeClimate) Print(issues []result.Issue) error { codeClimateIssues := make([]CodeClimateIssue, 0, len(issues)) for i := range issues { - issue := &issues[i] + issue := issues[i] codeClimateIssue := CodeClimateIssue{} codeClimateIssue.Description = issue.Description() @@ -52,14 +42,32 @@ func (p CodeClimate) Print(issues []result.Issue) error { codeClimateIssue.Location.Path = issue.Pos.Filename codeClimateIssue.Location.Lines.Begin = issue.Pos.Line codeClimateIssue.Fingerprint = issue.Fingerprint() - codeClimateIssue.Severity = defaultCodeClimateSeverity - - if slices.Contains(p.allowedSeverities, issue.Severity) { - codeClimateIssue.Severity = issue.Severity - } + codeClimateIssue.Severity = p.sanitizer.Sanitize(issue.Severity) codeClimateIssues = append(codeClimateIssues, codeClimateIssue) } + err := p.sanitizer.Err() + if err != nil { + p.log.Infof("%v", err) + } + return json.NewEncoder(p.w).Encode(codeClimateIssues) } + +// CodeClimateIssue is a subset of the Code Climate spec. +// https://github.com/codeclimate/platform/blob/master/spec/analyzers/SPEC.md#data-types +// It is just enough to support GitLab CI Code Quality. +// https://docs.gitlab.com/ee/ci/testing/code_quality.html#code-quality-report-format +type CodeClimateIssue struct { + Description string `json:"description"` + CheckName string `json:"check_name"` + Severity string `json:"severity,omitempty"` + Fingerprint string `json:"fingerprint"` + Location struct { + Path string `json:"path"` + Lines struct { + Begin int `json:"begin"` + } `json:"lines"` + } `json:"location"` +} diff --git a/pkg/printers/codeclimate_test.go b/pkg/printers/codeclimate_test.go index 236ed70f33c5..3ec6ad73cfae 100644 --- a/pkg/printers/codeclimate_test.go +++ b/pkg/printers/codeclimate_test.go @@ -8,6 +8,7 @@ import ( "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" + "github.com/golangci/golangci-lint/pkg/logutils" "github.com/golangci/golangci-lint/pkg/result" ) @@ -15,7 +16,7 @@ func TestCodeClimate_Print(t *testing.T) { issues := []result.Issue{ { FromLinter: "linter-a", - Severity: "warning", + Severity: "minor", Text: "some issue", Pos: token.Position{ Filename: "path/to/filea.go", @@ -42,28 +43,49 @@ func TestCodeClimate_Print(t *testing.T) { }, { FromLinter: "linter-c", - Text: "issue c", + Severity: "", + Text: "without severity", SourceLines: []string{ "func foo() {", - "\tfmt.Println(\"ccc\")", + "\tfmt.Println(\"bar\")", "}", }, Pos: token.Position{ Filename: "path/to/filec.go", - Offset: 6, - Line: 200, - Column: 2, + Offset: 5, + Line: 300, + Column: 10, + }, + }, + { + FromLinter: "linter-d", + Severity: "foo", + Text: "unknown severity", + SourceLines: []string{ + "func foo() {", + "\tfmt.Println(\"bar\")", + "}", + }, + Pos: token.Position{ + Filename: "path/to/filed.go", + Offset: 5, + Line: 300, + Column: 11, }, }, } buf := new(bytes.Buffer) - printer := NewCodeClimate(buf) + + log := logutils.NewStderrLog(logutils.DebugKeyEmpty) + log.SetLevel(logutils.LogLevelDebug) + + printer := NewCodeClimate(log, buf) err := printer.Print(issues) require.NoError(t, err) - expected := `[{"description":"linter-a: some issue","check_name":"linter-a","severity":"critical","fingerprint":"BA73C5DF4A6FD8462FFF1D3140235777","location":{"path":"path/to/filea.go","lines":{"begin":10}}},{"description":"linter-b: another issue","check_name":"linter-b","severity":"major","fingerprint":"0777B4FE60242BD8B2E9B7E92C4B9521","location":{"path":"path/to/fileb.go","lines":{"begin":300}}},{"description":"linter-c: issue c","check_name":"linter-c","severity":"critical","fingerprint":"BEE6E9FBB6BFA4B7DB9FB036697FB036","location":{"path":"path/to/filec.go","lines":{"begin":200}}}] + expected := `[{"description":"linter-a: some issue","check_name":"linter-a","severity":"minor","fingerprint":"BA73C5DF4A6FD8462FFF1D3140235777","location":{"path":"path/to/filea.go","lines":{"begin":10}}},{"description":"linter-b: another issue","check_name":"linter-b","severity":"major","fingerprint":"0777B4FE60242BD8B2E9B7E92C4B9521","location":{"path":"path/to/fileb.go","lines":{"begin":300}}},{"description":"linter-c: without severity","check_name":"linter-c","severity":"critical","fingerprint":"84F89700554F16CCEB6C0BB481B44AD2","location":{"path":"path/to/filec.go","lines":{"begin":300}}},{"description":"linter-d: unknown severity","check_name":"linter-d","severity":"critical","fingerprint":"340BB02E7B583B9727D73765CB64F56F","location":{"path":"path/to/filed.go","lines":{"begin":300}}}] ` assert.Equal(t, expected, buf.String()) diff --git a/pkg/printers/html.go b/pkg/printers/html.go index 7dd1e5c623d0..6fc6bc62a2b8 100644 --- a/pkg/printers/html.go +++ b/pkg/printers/html.go @@ -122,6 +122,8 @@ type htmlIssue struct { Code string } +// HTML prints issues in an HTML page. +// It uses the Cloudflare CDN (cdnjs) and React. type HTML struct { w io.Writer } diff --git a/pkg/printers/json.go b/pkg/printers/json.go index 28509cac459b..8fc94649f768 100644 --- a/pkg/printers/json.go +++ b/pkg/printers/json.go @@ -8,12 +8,13 @@ import ( "github.com/golangci/golangci-lint/pkg/result" ) +// JSON prints issues in a JSON representation. type JSON struct { - rd *report.Data // TODO(ldez) should be drop in v2. Only use by JSON reporter. + rd *report.Data w io.Writer } -func NewJSON(rd *report.Data, w io.Writer) *JSON { +func NewJSON(w io.Writer, rd *report.Data) *JSON { return &JSON{ rd: rd, w: w, diff --git a/pkg/printers/json_test.go b/pkg/printers/json_test.go index a28dbcf35369..1ce3690fb168 100644 --- a/pkg/printers/json_test.go +++ b/pkg/printers/json_test.go @@ -44,7 +44,7 @@ func TestJSON_Print(t *testing.T) { buf := new(bytes.Buffer) - printer := NewJSON(nil, buf) + printer := NewJSON(buf, nil) err := printer.Print(issues) require.NoError(t, err) diff --git a/pkg/printers/junitxml.go b/pkg/printers/junitxml.go index 7d0a703b0a5e..bc755b256d96 100644 --- a/pkg/printers/junitxml.go +++ b/pkg/printers/junitxml.go @@ -12,40 +12,16 @@ import ( "github.com/golangci/golangci-lint/pkg/result" ) -type testSuitesXML struct { - XMLName xml.Name `xml:"testsuites"` - TestSuites []testSuiteXML -} - -type testSuiteXML struct { - XMLName xml.Name `xml:"testsuite"` - Suite string `xml:"name,attr"` - Tests int `xml:"tests,attr"` - Errors int `xml:"errors,attr"` - Failures int `xml:"failures,attr"` - TestCases []testCaseXML `xml:"testcase"` -} - -type testCaseXML struct { - Name string `xml:"name,attr"` - ClassName string `xml:"classname,attr"` - Failure failureXML `xml:"failure"` - File string `xml:"file,attr,omitempty"` - Line int `xml:"line,attr,omitempty"` -} - -type failureXML struct { - Message string `xml:"message,attr"` - Type string `xml:"type,attr"` - Content string `xml:",cdata"` -} - +// JunitXML prints issues in the Junit XML format. +// There is no official specification for the JUnit XML file format, +// and various tools generate and support different flavors of this format. +// https://github.com/testmoapp/junitxml type JunitXML struct { extended bool w io.Writer } -func NewJunitXML(extended bool, w io.Writer) *JunitXML { +func NewJunitXML(w io.Writer, extended bool) *JunitXML { return &JunitXML{ extended: extended, w: w, @@ -97,3 +73,31 @@ func (p JunitXML) Print(issues []result.Issue) error { } return nil } + +type testSuitesXML struct { + XMLName xml.Name `xml:"testsuites"` + TestSuites []testSuiteXML +} + +type testSuiteXML struct { + XMLName xml.Name `xml:"testsuite"` + Suite string `xml:"name,attr"` + Tests int `xml:"tests,attr"` + Errors int `xml:"errors,attr"` + Failures int `xml:"failures,attr"` + TestCases []testCaseXML `xml:"testcase"` +} + +type testCaseXML struct { + Name string `xml:"name,attr"` + ClassName string `xml:"classname,attr"` + Failure failureXML `xml:"failure"` + File string `xml:"file,attr,omitempty"` + Line int `xml:"line,attr,omitempty"` +} + +type failureXML struct { + Message string `xml:"message,attr"` + Type string `xml:"type,attr"` + Content string `xml:",cdata"` +} diff --git a/pkg/printers/junitxml_test.go b/pkg/printers/junitxml_test.go index e4dfde195dfa..4cf99789124c 100644 --- a/pkg/printers/junitxml_test.go +++ b/pkg/printers/junitxml_test.go @@ -105,7 +105,7 @@ Details: func foo() { t.Parallel() buf := new(bytes.Buffer) - printer := NewJunitXML(test.extended, buf) + printer := NewJunitXML(buf, test.extended) err := printer.Print(issues) require.NoError(t, err) diff --git a/pkg/printers/printer.go b/pkg/printers/printer.go index 20be02e0158e..b7d3bc82b441 100644 --- a/pkg/printers/printer.go +++ b/pkg/printers/printer.go @@ -6,6 +6,8 @@ import ( "io" "os" "path/filepath" + "slices" + "strings" "github.com/golangci/golangci-lint/pkg/config" "github.com/golangci/golangci-lint/pkg/logutils" @@ -114,32 +116,63 @@ func (c *Printer) createPrinter(format string, w io.Writer) (issuePrinter, error switch format { case config.OutFormatJSON: - p = NewJSON(c.reportData, w) + p = NewJSON(w, c.reportData) case config.OutFormatLineNumber, config.OutFormatColoredLineNumber: - p = NewText(c.cfg.PrintIssuedLine, - format == config.OutFormatColoredLineNumber, c.cfg.PrintLinterName, - c.log.Child(logutils.DebugKeyTextPrinter), w) + p = NewText(c.log, w, c.cfg.PrintLinterName, c.cfg.PrintIssuedLine, format == config.OutFormatColoredLineNumber) case config.OutFormatTab, config.OutFormatColoredTab: - p = NewTab(c.cfg.PrintLinterName, - format == config.OutFormatColoredTab, - c.log.Child(logutils.DebugKeyTabPrinter), w) + p = NewTab(c.log, w, c.cfg.PrintLinterName, format == config.OutFormatColoredTab) case config.OutFormatCheckstyle: - p = NewCheckstyle(w) + p = NewCheckstyle(c.log, w) case config.OutFormatCodeClimate: - p = NewCodeClimate(w) + p = NewCodeClimate(c.log, w) case config.OutFormatHTML: p = NewHTML(w) case config.OutFormatJunitXML, config.OutFormatJunitXMLExtended: - p = NewJunitXML(format == config.OutFormatJunitXMLExtended, w) + p = NewJunitXML(w, format == config.OutFormatJunitXMLExtended) case config.OutFormatGithubActions: p = NewGitHubAction(w) case config.OutFormatTeamCity: - p = NewTeamCity(w) + p = NewTeamCity(c.log, w) case config.OutFormatSarif: - p = NewSarif(w) + p = NewSarif(c.log, w) default: return nil, fmt.Errorf("unknown output format %q", format) } return p, nil } + +type severitySanitizer struct { + allowedSeverities []string + defaultSeverity string + + unsupportedSeverities map[string]struct{} +} + +func (s *severitySanitizer) Sanitize(severity string) string { + if slices.Contains(s.allowedSeverities, severity) { + return severity + } + + if s.unsupportedSeverities == nil { + s.unsupportedSeverities = make(map[string]struct{}) + } + + s.unsupportedSeverities[severity] = struct{}{} + + return s.defaultSeverity +} + +func (s *severitySanitizer) Err() error { + if len(s.unsupportedSeverities) == 0 { + return nil + } + + var foo []string + for k := range s.unsupportedSeverities { + foo = append(foo, "'"+k+"'") + } + + return fmt.Errorf("some severities (%v) are not inside supported values (%v), fallback to '%s'", + strings.Join(foo, ", "), strings.Join(s.allowedSeverities, ", "), s.defaultSeverity) +} diff --git a/pkg/printers/sarif.go b/pkg/printers/sarif.go index 8b1dd2ee29ea..c06c11624434 100644 --- a/pkg/printers/sarif.go +++ b/pkg/printers/sarif.go @@ -4,6 +4,7 @@ import ( "encoding/json" "io" + "github.com/golangci/golangci-lint/pkg/logutils" "github.com/golangci/golangci-lint/pkg/result" ) @@ -12,6 +13,73 @@ const ( sarifSchemaURI = "https://schemastore.azurewebsites.net/schemas/json/sarif-2.1.0-rtm.6.json" ) +const defaultSarifSeverity = "error" + +// Sarif prints issues in the SARIF format. +// https://sarifweb.azurewebsites.net/ +// https://docs.oasis-open.org/sarif/sarif/v2.1.0/ +type Sarif struct { + log logutils.Log + w io.Writer + sanitizer severitySanitizer +} + +func NewSarif(log logutils.Log, w io.Writer) *Sarif { + return &Sarif{ + log: log.Child(logutils.DebugKeySarifPrinter), + w: w, + sanitizer: severitySanitizer{ + // https://docs.oasis-open.org/sarif/sarif/v2.1.0/errata01/os/sarif-v2.1.0-errata01-os-complete.html#_Toc141790898 + allowedSeverities: []string{"none", "note", "warning", defaultSarifSeverity}, + defaultSeverity: defaultSarifSeverity, + }, + } +} + +func (p *Sarif) Print(issues []result.Issue) error { + run := sarifRun{} + run.Tool.Driver.Name = "golangci-lint" + run.Results = make([]sarifResult, 0) + + for i := range issues { + issue := issues[i] + + sr := sarifResult{ + RuleID: issue.FromLinter, + Level: p.sanitizer.Sanitize(issue.Severity), + Message: sarifMessage{Text: issue.Text}, + Locations: []sarifLocation{ + { + PhysicalLocation: sarifPhysicalLocation{ + ArtifactLocation: sarifArtifactLocation{URI: issue.FilePath()}, + Region: sarifRegion{ + StartLine: issue.Line(), + // If startColumn is absent, it SHALL default to 1. + // https://docs.oasis-open.org/sarif/sarif/v2.1.0/errata01/os/sarif-v2.1.0-errata01-os-complete.html#_Toc141790941 + StartColumn: max(1, issue.Column()), + }, + }, + }, + }, + } + + run.Results = append(run.Results, sr) + } + + err := p.sanitizer.Err() + if err != nil { + p.log.Infof("%v", err) + } + + output := SarifOutput{ + Version: sarifVersion, + Schema: sarifSchemaURI, + Runs: []sarifRun{run}, + } + + return json.NewEncoder(p.w).Encode(output) +} + type SarifOutput struct { Version string `json:"version"` Schema string `json:"$schema"` @@ -58,60 +126,3 @@ type sarifRegion struct { StartLine int `json:"startLine"` StartColumn int `json:"startColumn"` } - -type Sarif struct { - w io.Writer -} - -func NewSarif(w io.Writer) *Sarif { - return &Sarif{w: w} -} - -func (p Sarif) Print(issues []result.Issue) error { - run := sarifRun{} - run.Tool.Driver.Name = "golangci-lint" - run.Results = make([]sarifResult, 0) - - for i := range issues { - issue := issues[i] - - severity := issue.Severity - - switch severity { - // https://docs.oasis-open.org/sarif/sarif/v2.1.0/errata01/os/sarif-v2.1.0-errata01-os-complete.html#_Toc141790898 - case "none", "note", "warning", "error": - // Valid levels. - default: - severity = "error" - } - - sr := sarifResult{ - RuleID: issue.FromLinter, - Level: severity, - Message: sarifMessage{Text: issue.Text}, - Locations: []sarifLocation{ - { - PhysicalLocation: sarifPhysicalLocation{ - ArtifactLocation: sarifArtifactLocation{URI: issue.FilePath()}, - Region: sarifRegion{ - StartLine: issue.Line(), - // If startColumn is absent, it SHALL default to 1. - // https://docs.oasis-open.org/sarif/sarif/v2.1.0/errata01/os/sarif-v2.1.0-errata01-os-complete.html#_Toc141790941 - StartColumn: max(1, issue.Column()), - }, - }, - }, - }, - } - - run.Results = append(run.Results, sr) - } - - output := SarifOutput{ - Version: sarifVersion, - Schema: sarifSchemaURI, - Runs: []sarifRun{run}, - } - - return json.NewEncoder(p.w).Encode(output) -} diff --git a/pkg/printers/sarif_test.go b/pkg/printers/sarif_test.go index 87c833de448c..f90606045682 100644 --- a/pkg/printers/sarif_test.go +++ b/pkg/printers/sarif_test.go @@ -8,6 +8,7 @@ import ( "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" + "github.com/golangci/golangci-lint/pkg/logutils" "github.com/golangci/golangci-lint/pkg/result" ) @@ -41,36 +42,60 @@ func TestSarif_Print(t *testing.T) { }, }, { - FromLinter: "linter-a", - Severity: "low", - Text: "some issue 2", + FromLinter: "linter-c", + Severity: "error", + Text: "some issue without column", Pos: token.Position{ - Filename: "path/to/filec.go", + Filename: "path/to/filed.go", Offset: 3, Line: 11, - Column: 5, }, }, { FromLinter: "linter-c", - Severity: "error", - Text: "some issue without column", + Severity: "", + Text: "without severity", + SourceLines: []string{ + "func foo() {", + "\tfmt.Println(\"bar\")", + "}", + }, + Pos: token.Position{ + Filename: "path/to/filec.go", + Offset: 5, + Line: 300, + Column: 10, + }, + }, + { + FromLinter: "linter-d", + Severity: "foo", + Text: "unknown severity", + SourceLines: []string{ + "func foo() {", + "\tfmt.Println(\"bar\")", + "}", + }, Pos: token.Position{ Filename: "path/to/filed.go", - Offset: 3, - Line: 11, + Offset: 5, + Line: 300, + Column: 11, }, }, } buf := new(bytes.Buffer) - printer := NewSarif(buf) + log := logutils.NewStderrLog(logutils.DebugKeyEmpty) + log.SetLevel(logutils.LogLevelDebug) + + printer := NewSarif(log, buf) err := printer.Print(issues) require.NoError(t, err) - expected := `{"version":"2.1.0","$schema":"https://schemastore.azurewebsites.net/schemas/json/sarif-2.1.0-rtm.6.json","runs":[{"tool":{"driver":{"name":"golangci-lint"}},"results":[{"ruleId":"linter-a","level":"warning","message":{"text":"some issue"},"locations":[{"physicalLocation":{"artifactLocation":{"uri":"path/to/filea.go","index":0},"region":{"startLine":10,"startColumn":4}}}]},{"ruleId":"linter-b","level":"error","message":{"text":"another issue"},"locations":[{"physicalLocation":{"artifactLocation":{"uri":"path/to/fileb.go","index":0},"region":{"startLine":300,"startColumn":9}}}]},{"ruleId":"linter-a","level":"error","message":{"text":"some issue 2"},"locations":[{"physicalLocation":{"artifactLocation":{"uri":"path/to/filec.go","index":0},"region":{"startLine":11,"startColumn":5}}}]},{"ruleId":"linter-c","level":"error","message":{"text":"some issue without column"},"locations":[{"physicalLocation":{"artifactLocation":{"uri":"path/to/filed.go","index":0},"region":{"startLine":11,"startColumn":1}}}]}]}]} + expected := `{"version":"2.1.0","$schema":"https://schemastore.azurewebsites.net/schemas/json/sarif-2.1.0-rtm.6.json","runs":[{"tool":{"driver":{"name":"golangci-lint"}},"results":[{"ruleId":"linter-a","level":"warning","message":{"text":"some issue"},"locations":[{"physicalLocation":{"artifactLocation":{"uri":"path/to/filea.go","index":0},"region":{"startLine":10,"startColumn":4}}}]},{"ruleId":"linter-b","level":"error","message":{"text":"another issue"},"locations":[{"physicalLocation":{"artifactLocation":{"uri":"path/to/fileb.go","index":0},"region":{"startLine":300,"startColumn":9}}}]},{"ruleId":"linter-c","level":"error","message":{"text":"some issue without column"},"locations":[{"physicalLocation":{"artifactLocation":{"uri":"path/to/filed.go","index":0},"region":{"startLine":11,"startColumn":1}}}]},{"ruleId":"linter-c","level":"error","message":{"text":"without severity"},"locations":[{"physicalLocation":{"artifactLocation":{"uri":"path/to/filec.go","index":0},"region":{"startLine":300,"startColumn":10}}}]},{"ruleId":"linter-d","level":"error","message":{"text":"unknown severity"},"locations":[{"physicalLocation":{"artifactLocation":{"uri":"path/to/filed.go","index":0},"region":{"startLine":300,"startColumn":11}}}]}]}]} ` assert.Equal(t, expected, buf.String()) @@ -79,7 +104,10 @@ func TestSarif_Print(t *testing.T) { func TestSarif_Print_empty(t *testing.T) { buf := new(bytes.Buffer) - printer := NewSarif(buf) + log := logutils.NewStderrLog(logutils.DebugKeyEmpty) + log.SetLevel(logutils.LogLevelDebug) + + printer := NewSarif(log, buf) err := printer.Print(nil) require.NoError(t, err) diff --git a/pkg/printers/tab.go b/pkg/printers/tab.go index c6d390d188cf..0a0b176b4a6b 100644 --- a/pkg/printers/tab.go +++ b/pkg/printers/tab.go @@ -11,6 +11,7 @@ import ( "github.com/golangci/golangci-lint/pkg/result" ) +// Tab prints issues using tabulation as field separator. type Tab struct { printLinterName bool useColors bool @@ -19,11 +20,11 @@ type Tab struct { w io.Writer } -func NewTab(printLinterName, useColors bool, log logutils.Log, w io.Writer) *Tab { +func NewTab(log logutils.Log, w io.Writer, printLinterName, useColors bool) *Tab { return &Tab{ printLinterName: printLinterName, useColors: useColors, - log: log, + log: log.Child(logutils.DebugKeyTabPrinter), w: w, } } diff --git a/pkg/printers/tab_test.go b/pkg/printers/tab_test.go index fbd5dca6c65b..4fc9fa034589 100644 --- a/pkg/printers/tab_test.go +++ b/pkg/printers/tab_test.go @@ -87,7 +87,7 @@ path/to/fileb.go:300:9 another issue buf := new(bytes.Buffer) - printer := NewTab(test.printLinterName, test.useColors, logutils.NewStderrLog(logutils.DebugKeyEmpty), buf) + printer := NewTab(logutils.NewStderrLog(logutils.DebugKeyEmpty), buf, test.printLinterName, test.useColors) err := printer.Print(issues) require.NoError(t, err) diff --git a/pkg/printers/teamcity.go b/pkg/printers/teamcity.go index 83c4959114f4..9ff5fe5bc9dc 100644 --- a/pkg/printers/teamcity.go +++ b/pkg/printers/teamcity.go @@ -5,6 +5,7 @@ import ( "io" "strings" + "github.com/golangci/golangci-lint/pkg/logutils" "github.com/golangci/golangci-lint/pkg/result" ) @@ -14,16 +15,22 @@ const ( largeLimit = 4000 ) -// TeamCity printer for TeamCity format. +const defaultTeamCitySeverity = "ERROR" + +// TeamCity prints issues in the TeamCity format. +// https://www.jetbrains.com/help/teamcity/service-messages.html type TeamCity struct { - w io.Writer - escaper *strings.Replacer + log logutils.Log + w io.Writer + escaper *strings.Replacer + sanitizer severitySanitizer } // NewTeamCity output format outputs issues according to TeamCity service message format. -func NewTeamCity(w io.Writer) *TeamCity { +func NewTeamCity(log logutils.Log, w io.Writer) *TeamCity { return &TeamCity{ - w: w, + log: log.Child(logutils.DebugKeyTeamCityPrinter), + w: w, // https://www.jetbrains.com/help/teamcity/service-messages.html#Escaped+Values escaper: strings.NewReplacer( "'", "|'", @@ -33,6 +40,11 @@ func NewTeamCity(w io.Writer) *TeamCity { "[", "|[", "]", "|]", ), + sanitizer: severitySanitizer{ + // https://www.jetbrains.com/help/teamcity/service-messages.html#Inspection+Instance + allowedSeverities: []string{"INFO", defaultTeamCitySeverity, "WARNING", "WEAK WARNING"}, + defaultSeverity: defaultTeamCitySeverity, + }, } } @@ -64,7 +76,7 @@ func (p *TeamCity) Print(issues []result.Issue) error { message: issue.Text, file: issue.FilePath(), line: issue.Line(), - severity: issue.Severity, + severity: p.sanitizer.Sanitize(strings.ToUpper(issue.Severity)), } _, err := instance.Print(p.w, p.escaper) @@ -73,6 +85,11 @@ func (p *TeamCity) Print(issues []result.Issue) error { } } + err := p.sanitizer.Err() + if err != nil { + p.log.Infof("%v", err) + } + return nil } @@ -107,7 +124,7 @@ func (i InspectionInstance) Print(w io.Writer, replacer *strings.Replacer) (int, cutVal(i.typeID, smallLimit), cutVal(replacer.Replace(i.message), largeLimit), cutVal(i.file, largeLimit), - i.line, strings.ToUpper(i.severity)) + i.line, i.severity) } func cutVal(s string, limit int) string { diff --git a/pkg/printers/teamcity_test.go b/pkg/printers/teamcity_test.go index f333a3b65d5b..1eb29d33931d 100644 --- a/pkg/printers/teamcity_test.go +++ b/pkg/printers/teamcity_test.go @@ -8,6 +8,7 @@ import ( "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" + "github.com/golangci/golangci-lint/pkg/logutils" "github.com/golangci/golangci-lint/pkg/result" ) @@ -15,6 +16,7 @@ func TestTeamCity_Print(t *testing.T) { issues := []result.Issue{ { FromLinter: "linter-a", + Severity: "WARNING", Text: "warning issue", Pos: token.Position{ Filename: "path/to/filea.go", @@ -34,33 +36,56 @@ func TestTeamCity_Print(t *testing.T) { }, }, { - FromLinter: "linter-b", - Text: "info issue", + FromLinter: "linter-c", + Severity: "", + Text: "without severity", SourceLines: []string{ "func foo() {", "\tfmt.Println(\"bar\")", "}", }, Pos: token.Position{ - Filename: "path/to/fileb.go", + Filename: "path/to/filec.go", Offset: 5, Line: 300, - Column: 9, + Column: 10, + }, + }, + { + FromLinter: "linter-d", + Severity: "foo", + Text: "unknown severity", + SourceLines: []string{ + "func foo() {", + "\tfmt.Println(\"bar\")", + "}", + }, + Pos: token.Position{ + Filename: "path/to/filed.go", + Offset: 5, + Line: 300, + Column: 11, }, }, } buf := new(bytes.Buffer) - printer := NewTeamCity(buf) + + log := logutils.NewStderrLog(logutils.DebugKeyEmpty) + log.SetLevel(logutils.LogLevelDebug) + + printer := NewTeamCity(log, buf) err := printer.Print(issues) require.NoError(t, err) expected := `##teamcity[inspectionType id='linter-a' name='linter-a' description='linter-a' category='Golangci-lint reports'] -##teamcity[inspection typeId='linter-a' message='warning issue' file='path/to/filea.go' line='10' SEVERITY=''] +##teamcity[inspection typeId='linter-a' message='warning issue' file='path/to/filea.go' line='10' SEVERITY='WARNING'] ##teamcity[inspection typeId='linter-a' message='error issue' file='path/to/filea.go' line='10' SEVERITY='ERROR'] -##teamcity[inspectionType id='linter-b' name='linter-b' description='linter-b' category='Golangci-lint reports'] -##teamcity[inspection typeId='linter-b' message='info issue' file='path/to/fileb.go' line='300' SEVERITY=''] +##teamcity[inspectionType id='linter-c' name='linter-c' description='linter-c' category='Golangci-lint reports'] +##teamcity[inspection typeId='linter-c' message='without severity' file='path/to/filec.go' line='300' SEVERITY='ERROR'] +##teamcity[inspectionType id='linter-d' name='linter-d' description='linter-d' category='Golangci-lint reports'] +##teamcity[inspection typeId='linter-d' message='unknown severity' file='path/to/filed.go' line='300' SEVERITY='ERROR'] ` assert.Equal(t, expected, buf.String()) diff --git a/pkg/printers/testdata/golden-teamcity.txt b/pkg/printers/testdata/golden-teamcity.txt index 1d32bd36be6c..6f4bb014c1ca 100644 --- a/pkg/printers/testdata/golden-teamcity.txt +++ b/pkg/printers/testdata/golden-teamcity.txt @@ -1,28 +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[inspection typeId='gochecknoinits' message='don|'t use `init` function' file='pkg/experimental/myplugin/myplugin.go' line='13' SEVERITY='ERROR'] ##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[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='ERROR'] ##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[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='ERROR'] ##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[inspection typeId='maligned' message='struct of size 144 bytes could be of size 128 bytes' file='pkg/config/issues.go' line='107' SEVERITY='ERROR'] +##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='ERROR'] +##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='ERROR'] +##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='ERROR'] +##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='ERROR'] +##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='ERROR'] +##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='ERROR'] +##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='ERROR'] +##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='ERROR'] +##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='ERROR'] +##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='ERROR'] +##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='ERROR'] +##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='ERROR'] +##teamcity[inspection typeId='maligned' message='struct of size 168 bytes could be of size 160 bytes' file='pkg/config/run.go' line='6' SEVERITY='ERROR'] +##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='ERROR'] +##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='ERROR'] +##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='ERROR'] ##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=''] +##teamcity[inspection typeId='unused' message='const `defaultFileMode` is unused' file='pkg/commands/run.go' line='47' SEVERITY='ERROR'] diff --git a/pkg/printers/text.go b/pkg/printers/text.go index 56cced769695..9e60408f0400 100644 --- a/pkg/printers/text.go +++ b/pkg/printers/text.go @@ -11,21 +11,22 @@ import ( "github.com/golangci/golangci-lint/pkg/result" ) +// Text prints issues with a human friendly representation. type Text struct { - printIssuedLine bool printLinterName bool + printIssuedLine bool useColors bool log logutils.Log w io.Writer } -func NewText(printIssuedLine, useColors, printLinterName bool, log logutils.Log, w io.Writer) *Text { +func NewText(log logutils.Log, w io.Writer, printLinterName, printIssuedLine, useColors bool) *Text { return &Text{ - printIssuedLine: printIssuedLine, printLinterName: printLinterName, + printIssuedLine: printIssuedLine, useColors: useColors, - log: log, + log: log.Child(logutils.DebugKeyTextPrinter), w: w, } } diff --git a/pkg/printers/text_test.go b/pkg/printers/text_test.go index bec16f72618a..55eda1827914 100644 --- a/pkg/printers/text_test.go +++ b/pkg/printers/text_test.go @@ -115,7 +115,7 @@ path/to/fileb.go:300:9: another issue buf := new(bytes.Buffer) - printer := NewText(test.printIssuedLine, test.useColors, test.printLinterName, logutils.NewStderrLog(logutils.DebugKeyEmpty), buf) + printer := NewText(logutils.NewStderrLog(logutils.DebugKeyEmpty), buf, test.printLinterName, test.printIssuedLine, test.useColors) err := printer.Print(issues) require.NoError(t, err)