From 6d8d0230c01d56860bf46a6667dac52f38869010 Mon Sep 17 00:00:00 2001 From: ferhat elmas Date: Fri, 17 Feb 2023 16:04:40 +0100 Subject: [PATCH 01/12] feat: add team city output format fixes #3597 --- pkg/commands/run.go | 2 + pkg/config/output.go | 2 + pkg/printers/teamcity.go | 151 ++++++++++++++++++++++++++++++++++ pkg/printers/teamcity_test.go | 81 ++++++++++++++++++ 4 files changed, 236 insertions(+) create mode 100644 pkg/printers/teamcity.go create mode 100644 pkg/printers/teamcity_test.go diff --git a/pkg/commands/run.go b/pkg/commands/run.go index 56d6dfcbf3c4..5ba188fea154 100644 --- a/pkg/commands/run.go +++ b/pkg/commands/run.go @@ -494,6 +494,8 @@ func (e *Executor) createPrinter(format string, w io.Writer) (printers.Printer, p = printers.NewJunitXML(w) case config.OutFormatGithubActions: p = printers.NewGithub(w) + case config.OutFormatTeamCity: + p = printers.NewTeamCity(&e.reportData, w, nil) default: return nil, fmt.Errorf("unknown output format %s", format) } diff --git a/pkg/config/output.go b/pkg/config/output.go index d67f110f678d..fd425cf2513a 100644 --- a/pkg/config/output.go +++ b/pkg/config/output.go @@ -10,6 +10,7 @@ const ( OutFormatHTML = "html" OutFormatJunitXML = "junit-xml" OutFormatGithubActions = "github-actions" + OutFormatTeamCity = "team-city" ) var OutFormats = []string{ @@ -22,6 +23,7 @@ var OutFormats = []string{ OutFormatHTML, OutFormatJunitXML, OutFormatGithubActions, + OutFormatTeamCity, } type Output struct { diff --git a/pkg/printers/teamcity.go b/pkg/printers/teamcity.go new file mode 100644 index 000000000000..eeb9fde3493e --- /dev/null +++ b/pkg/printers/teamcity.go @@ -0,0 +1,151 @@ +package printers + +import ( + "bytes" + "context" + "fmt" + "io" + "sort" + "strings" + "time" + + "github.com/golangci/golangci-lint/pkg/report" + "github.com/golangci/golangci-lint/pkg/result" +) + +const ( + timestampFormat = "2006-01-02T15:04:05.000" + testStarted = "##teamcity[testStarted timestamp='%s' name='%s']\n" + testStdErr = "##teamcity[testStdErr timestamp='%s' name='%s' out='%s']\n" + testFailed = "##teamcity[testFailed timestamp='%s' name='%s']\n" + testIgnored = "##teamcity[testIgnored timestamp='%s' name='%s']\n" + testFinished = "##teamcity[testFinished timestamp='%s' name='%s']\n" +) + +type teamcityLinter struct { + data *report.LinterData + issues []string +} + +func (l *teamcityLinter) getName() string { + return fmt.Sprintf("linter: %s", l.data.Name) +} + +func (l *teamcityLinter) failed() bool { + return len(l.issues) > 0 +} + +type teamcity struct { + linters map[string]*teamcityLinter + w io.Writer + err error + now now +} + +type now func() string + +// NewTeamCity output format outputs issues according to TeamCity service message format +func NewTeamCity(rd *report.Data, w io.Writer, nower now) Printer { + t := &teamcity{ + linters: map[string]*teamcityLinter{}, + w: w, + now: nower, + } + if t.now == nil { + t.now = func() string { + return time.Now().Format(timestampFormat) + } + } + for i, l := range rd.Linters { + t.linters[l.Name] = &teamcityLinter{ + data: &rd.Linters[i], + } + } + return t +} + +func (p *teamcity) getSortedLinterNames() []string { + names := make([]string, 0, len(p.linters)) + for name := range p.linters { + names = append(names, name) + } + sort.Strings(names) + return names +} + +// escape transforms strings for TeamCity service messages +// https://www.jetbrains.com/help/teamcity/service-messages.html#Escaped+values +func (p *teamcity) escape(s string) string { + var buf bytes.Buffer + for { + nextSpecial := strings.IndexAny(s, "'\n\r|[]") + switch nextSpecial { + case -1: + if buf.Len() == 0 { + return s + } + return buf.String() + s + case 0: + default: + buf.WriteString(s[:nextSpecial]) + } + switch s[nextSpecial] { + case '\'': + buf.WriteString("|'") + case '\n': + buf.WriteString("|n") + case '\r': + buf.WriteString("|r") + case '|': + buf.WriteString("||") + case '[': + buf.WriteString("|[") + case ']': + buf.WriteString("|]") + } + s = s[nextSpecial+1:] + } +} + +func (p *teamcity) print(format string, args ...any) { + if p.err != nil { + return + } + args = append([]any{p.now()}, args...) + _, p.err = fmt.Fprintf(p.w, format, args...) +} + +func (p *teamcity) Print(_ context.Context, issues []result.Issue) error { + for i := range issues { + issue := &issues[i] + + var col string + if issue.Pos.Column != 0 { + col = fmt.Sprintf(":%d", issue.Pos.Column) + } + + formatted := fmt.Sprintf("%s:%v%s - %s", issue.FilePath(), issue.Line(), col, issue.Text) + p.linters[issue.FromLinter].issues = append(p.linters[issue.FromLinter].issues, formatted) + } + + for _, linterName := range p.getSortedLinterNames() { + linter := p.linters[linterName] + + name := p.escape(linter.getName()) + p.print(testStarted, name) + if !linter.data.Enabled && !linter.data.EnabledByDefault { + p.print(testIgnored, name) + continue + } + + if linter.failed() { + for _, issue := range linter.issues { + p.print(testStdErr, name, p.escape(issue)) + } + p.print(testFailed, name) + } else { + p.print(testFinished, name) + } + } + return p.err +} diff --git a/pkg/printers/teamcity_test.go b/pkg/printers/teamcity_test.go new file mode 100644 index 000000000000..e71bcb835fa1 --- /dev/null +++ b/pkg/printers/teamcity_test.go @@ -0,0 +1,81 @@ +package printers + +import ( + "bytes" + "context" + "go/token" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + + "github.com/golangci/golangci-lint/pkg/report" + "github.com/golangci/golangci-lint/pkg/result" +) + +func TestTeamCity_Print(t *testing.T) { + issues := []result.Issue{ + { + FromLinter: "linter-a", + Severity: "error", + Text: "some issue", + Pos: token.Position{ + Filename: "path/to/filea.go", + Offset: 2, + Line: 10, + Column: 4, + }, + }, + { + FromLinter: "linter-a", + Severity: "error", + Text: "some issue 2", + Pos: token.Position{ + Filename: "path/to/filea.go", + Offset: 2, + Line: 10, + }, + }, + { + FromLinter: "linter-b", + Severity: "error", + Text: "another issue", + SourceLines: []string{ + "func foo() {", + "\tfmt.Println(\"bar\")", + "}", + }, + Pos: token.Position{ + Filename: "path/to/fileb.go", + Offset: 5, + Line: 300, + Column: 9, + }, + }, + } + + buf := new(bytes.Buffer) + rd := &report.Data{ + Linters: []report.LinterData{ + {Name: "linter-a", Enabled: true}, + {Name: "linter-b", Enabled: false}, + }, + } + nower := func() string { + return "2023-02-17T15:42:23.630" + } + printer := NewTeamCity(rd, buf, nower) + + err := printer.Print(context.Background(), issues) + require.NoError(t, err) + + expected := `##teamcity[testStarted timestamp='2023-02-17T15:42:23.630' name='linter: linter-a'] +##teamcity[testStdErr timestamp='2023-02-17T15:42:23.630' name='linter: linter-a' out='path/to/filea.go:10:4 - some issue'] +##teamcity[testStdErr timestamp='2023-02-17T15:42:23.630' name='linter: linter-a' out='path/to/filea.go:10 - some issue 2'] +##teamcity[testFailed timestamp='2023-02-17T15:42:23.630' name='linter: linter-a'] +##teamcity[testStarted timestamp='2023-02-17T15:42:23.630' name='linter: linter-b'] +##teamcity[testIgnored timestamp='2023-02-17T15:42:23.630' name='linter: linter-b'] +` + + assert.Equal(t, expected, buf.String()) +} From d404923a736ab43c619cca186443db835a2b5b85 Mon Sep 17 00:00:00 2001 From: Fernandez Ludovic Date: Sat, 25 Feb 2023 03:50:07 +0100 Subject: [PATCH 02/12] review --- pkg/commands/run.go | 2 +- pkg/printers/teamcity.go | 198 ++++++++++++++-------------------- pkg/printers/teamcity_test.go | 23 ++-- 3 files changed, 90 insertions(+), 133 deletions(-) diff --git a/pkg/commands/run.go b/pkg/commands/run.go index 5ba188fea154..a9971f5e02b2 100644 --- a/pkg/commands/run.go +++ b/pkg/commands/run.go @@ -495,7 +495,7 @@ func (e *Executor) createPrinter(format string, w io.Writer) (printers.Printer, case config.OutFormatGithubActions: p = printers.NewGithub(w) case config.OutFormatTeamCity: - p = printers.NewTeamCity(&e.reportData, w, nil) + p = printers.NewTeamCity(w) default: return nil, fmt.Errorf("unknown output format %s", format) } diff --git a/pkg/printers/teamcity.go b/pkg/printers/teamcity.go index eeb9fde3493e..50d3dba5569f 100644 --- a/pkg/printers/teamcity.go +++ b/pkg/printers/teamcity.go @@ -1,151 +1,119 @@ package printers import ( - "bytes" "context" "fmt" "io" - "sort" "strings" - "time" + "unicode/utf8" - "github.com/golangci/golangci-lint/pkg/report" "github.com/golangci/golangci-lint/pkg/result" ) +// Field limits. const ( - timestampFormat = "2006-01-02T15:04:05.000" - testStarted = "##teamcity[testStarted timestamp='%s' name='%s']\n" - testStdErr = "##teamcity[testStdErr timestamp='%s' name='%s' out='%s']\n" - testFailed = "##teamcity[testFailed timestamp='%s' name='%s']\n" - testIgnored = "##teamcity[testIgnored timestamp='%s' name='%s']\n" - testFinished = "##teamcity[testFinished timestamp='%s' name='%s']\n" + smallLimit = 255 + largeLimit = 4000 ) -type teamcityLinter struct { - data *report.LinterData - issues []string +// TeamCity printer for TeamCity format. +type TeamCity struct { + w io.Writer + escaper *strings.Replacer } -func (l *teamcityLinter) getName() string { - return fmt.Sprintf("linter: %s", l.data.Name) +// NewTeamCity output format outputs issues according to TeamCity service message format +func NewTeamCity(w io.Writer) *TeamCity { + return &TeamCity{ + w: w, + // https://www.jetbrains.com/help/teamcity/service-messages.html#Escaped+Values + escaper: strings.NewReplacer( + "'", "|'", + "\n", "|n", + "\r", "|r", + "|", "||", + "[", "|[", + "]", "|]", + ), + } } -func (l *teamcityLinter) failed() bool { - return len(l.issues) > 0 -} +func (p *TeamCity) Print(_ context.Context, issues []result.Issue) error { + uniqLinters := map[string]struct{}{} -type teamcity struct { - linters map[string]*teamcityLinter - w io.Writer - err error - now now -} + for i := range issues { + issue := issues[i] + + _, ok := uniqLinters[issue.FromLinter] + if !ok { + inspectionType := InspectionType{ + id: issue.FromLinter, + name: issue.FromLinter, + description: issue.FromLinter, + category: "Golangci-lint reports", + } -type now func() string + _, err := inspectionType.Print(p.w, p.escaper) + if err != nil { + return err + } -// NewTeamCity output format outputs issues according to TeamCity service message format -func NewTeamCity(rd *report.Data, w io.Writer, nower now) Printer { - t := &teamcity{ - linters: map[string]*teamcityLinter{}, - w: w, - now: nower, - } - if t.now == nil { - t.now = func() string { - return time.Now().Format(timestampFormat) + uniqLinters[issue.FromLinter] = struct{}{} } - } - for i, l := range rd.Linters { - t.linters[l.Name] = &teamcityLinter{ - data: &rd.Linters[i], + + instance := InspectionInstance{ + typeID: issue.FromLinter, + message: issue.Text, + file: issue.FilePath(), + line: issue.Line(), + additionalAttribute: issue.Severity, } - } - return t -} -func (p *teamcity) getSortedLinterNames() []string { - names := make([]string, 0, len(p.linters)) - for name := range p.linters { - names = append(names, name) + _, err := instance.Print(p.w, p.escaper) + if err != nil { + return err + } } - sort.Strings(names) - return names + + return nil } -// escape transforms strings for TeamCity service messages -// https://www.jetbrains.com/help/teamcity/service-messages.html#Escaped+values -func (p *teamcity) escape(s string) string { - var buf bytes.Buffer - for { - nextSpecial := strings.IndexAny(s, "'\n\r|[]") - switch nextSpecial { - case -1: - if buf.Len() == 0 { - return s - } - return buf.String() + s - case 0: - default: - buf.WriteString(s[:nextSpecial]) - } - switch s[nextSpecial] { - case '\'': - buf.WriteString("|'") - case '\n': - buf.WriteString("|n") - case '\r': - buf.WriteString("|r") - case '|': - buf.WriteString("||") - case '[': - buf.WriteString("|[") - case ']': - buf.WriteString("|]") - } - s = s[nextSpecial+1:] - } +// InspectionType Each specific warning or an error in code (inspection instance) has an inspection type. +// https://www.jetbrains.com/help/teamcity/service-messages.html#Inspection+Type +type InspectionType struct { + id string // (mandatory) limited by 255 characters. + name string // (mandatory) limited by 255 characters. + description string // (mandatory) limited by 255 characters. + category string // (mandatory) limited by 4000 characters. } -func (p *teamcity) print(format string, args ...any) { - if p.err != nil { - return - } - args = append([]any{p.now()}, args...) - _, p.err = fmt.Fprintf(p.w, format, args...) +func (i InspectionType) Print(w io.Writer, escaper *strings.Replacer) (int, error) { + return fmt.Fprintf(w, "##teamcity[inspectionType id='%s' name='%s' description='%s' category='%s']\n", + limit(i.id, smallLimit), limit(i.name, smallLimit), limit(escaper.Replace(i.description), largeLimit), limit(i.category, smallLimit)) } -func (p *teamcity) Print(_ context.Context, issues []result.Issue) error { - for i := range issues { - issue := &issues[i] +// InspectionInstance Reports a specific defect, warning, error message. +// Includes location, description, and various optional and custom attributes. +// https://www.jetbrains.com/help/teamcity/service-messages.html#Inspection+Instance +type InspectionInstance struct { + typeID string // (mandatory) limited by 255 characters. + message string // (optional) limited by 4000 characters. + file string // (mandatory) file path limited by 4000 characters. + line int // (optional) line of the file, integer. + additionalAttribute string +} - var col string - if issue.Pos.Column != 0 { - col = fmt.Sprintf(":%d", issue.Pos.Column) - } +func (i InspectionInstance) Print(w io.Writer, replacer *strings.Replacer) (int, error) { + return fmt.Fprintf(w, "##teamcity[inspection typeId='%s' message='%s' file='%s' line='%d' additional attribute='%s']\n", + limit(i.typeID, smallLimit), limit(replacer.Replace(i.message), largeLimit), limit(i.file, largeLimit), i.line, i.additionalAttribute) +} - formatted := fmt.Sprintf("%s:%v%s - %s", issue.FilePath(), issue.Line(), col, issue.Text) - p.linters[issue.FromLinter].issues = append(p.linters[issue.FromLinter].issues, formatted) +func limit(s string, max int) string { + var size, count int + for i := 0; i < max && count < len(s); i++ { + _, size = utf8.DecodeRuneInString(s[count:]) + count += size } - for _, linterName := range p.getSortedLinterNames() { - linter := p.linters[linterName] - - name := p.escape(linter.getName()) - p.print(testStarted, name) - if !linter.data.Enabled && !linter.data.EnabledByDefault { - p.print(testIgnored, name) - continue - } - - if linter.failed() { - for _, issue := range linter.issues { - p.print(testStdErr, name, p.escape(issue)) - } - p.print(testFailed, name) - } else { - p.print(testFinished, name) - } - } - return p.err + return s[:count] } diff --git a/pkg/printers/teamcity_test.go b/pkg/printers/teamcity_test.go index e71bcb835fa1..050a3e4cd600 100644 --- a/pkg/printers/teamcity_test.go +++ b/pkg/printers/teamcity_test.go @@ -9,7 +9,6 @@ import ( "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" - "github.com/golangci/golangci-lint/pkg/report" "github.com/golangci/golangci-lint/pkg/result" ) @@ -55,26 +54,16 @@ func TestTeamCity_Print(t *testing.T) { } buf := new(bytes.Buffer) - rd := &report.Data{ - Linters: []report.LinterData{ - {Name: "linter-a", Enabled: true}, - {Name: "linter-b", Enabled: false}, - }, - } - nower := func() string { - return "2023-02-17T15:42:23.630" - } - printer := NewTeamCity(rd, buf, nower) + printer := NewTeamCity(buf) err := printer.Print(context.Background(), issues) require.NoError(t, err) - expected := `##teamcity[testStarted timestamp='2023-02-17T15:42:23.630' name='linter: linter-a'] -##teamcity[testStdErr timestamp='2023-02-17T15:42:23.630' name='linter: linter-a' out='path/to/filea.go:10:4 - some issue'] -##teamcity[testStdErr timestamp='2023-02-17T15:42:23.630' name='linter: linter-a' out='path/to/filea.go:10 - some issue 2'] -##teamcity[testFailed timestamp='2023-02-17T15:42:23.630' name='linter: linter-a'] -##teamcity[testStarted timestamp='2023-02-17T15:42:23.630' name='linter: linter-b'] -##teamcity[testIgnored timestamp='2023-02-17T15:42:23.630' name='linter: linter-b'] + expected := `##teamcity[inspectionType id='linter-a' name='linter-a' description='linter-a' category='Golangci-lint reports'] +##teamcity[inspection typeId='linter-a' message='some issue' file='path/to/filea.go' line='10' additional attribute='error'] +##teamcity[inspection typeId='linter-a' message='some issue 2' file='path/to/filea.go' line='10' additional attribute='error'] +##teamcity[inspectionType id='linter-b' name='linter-b' description='linter-b' category='Golangci-lint reports'] +##teamcity[inspection typeId='linter-b' message='another issue' file='path/to/fileb.go' line='300' additional attribute='error'] ` assert.Equal(t, expected, buf.String()) From e799af1c185ba40b1ab88570f2c1144cfb510f4a Mon Sep 17 00:00:00 2001 From: Oleksandr Redko Date: Mon, 27 Feb 2023 15:05:51 +0200 Subject: [PATCH 03/12] fix `additional attribute`: should not contain spaces --- pkg/printers/teamcity.go | 13 ++++++++----- 1 file changed, 8 insertions(+), 5 deletions(-) diff --git a/pkg/printers/teamcity.go b/pkg/printers/teamcity.go index 50d3dba5569f..d7682f931e76 100644 --- a/pkg/printers/teamcity.go +++ b/pkg/printers/teamcity.go @@ -62,11 +62,14 @@ func (p *TeamCity) Print(_ context.Context, issues []result.Issue) error { } instance := InspectionInstance{ - typeID: issue.FromLinter, - message: issue.Text, - file: issue.FilePath(), - line: issue.Line(), - additionalAttribute: issue.Severity, + typeID: issue.FromLinter, + message: issue.Text, + file: issue.FilePath(), + line: issue.Line(), + } + + if issue.Severity != "" { + instance.additionalAttribute = issue.Severity } _, err := instance.Print(p.w, p.escaper) From f9532b7126ad50e1c4bffbf7c5fbf6c0b252dd80 Mon Sep 17 00:00:00 2001 From: Fernandez Ludovic Date: Mon, 27 Feb 2023 14:11:24 +0100 Subject: [PATCH 04/12] Revert "fix `additional attribute`: should not contain spaces" This reverts commit e799af1c185ba40b1ab88570f2c1144cfb510f4a. --- pkg/printers/teamcity.go | 13 +++++-------- 1 file changed, 5 insertions(+), 8 deletions(-) diff --git a/pkg/printers/teamcity.go b/pkg/printers/teamcity.go index d7682f931e76..50d3dba5569f 100644 --- a/pkg/printers/teamcity.go +++ b/pkg/printers/teamcity.go @@ -62,14 +62,11 @@ func (p *TeamCity) Print(_ context.Context, issues []result.Issue) error { } instance := InspectionInstance{ - typeID: issue.FromLinter, - message: issue.Text, - file: issue.FilePath(), - line: issue.Line(), - } - - if issue.Severity != "" { - instance.additionalAttribute = issue.Severity + typeID: issue.FromLinter, + message: issue.Text, + file: issue.FilePath(), + line: issue.Line(), + additionalAttribute: issue.Severity, } _, err := instance.Print(p.w, p.escaper) From 283573cf25704d80c12bea518ea7bb335172154f Mon Sep 17 00:00:00 2001 From: Fernandez Ludovic Date: Mon, 27 Feb 2023 14:14:47 +0100 Subject: [PATCH 05/12] review: trim and remove additional attribute if empty --- pkg/printers/teamcity.go | 23 ++++++++++++++++++++--- 1 file changed, 20 insertions(+), 3 deletions(-) diff --git a/pkg/printers/teamcity.go b/pkg/printers/teamcity.go index 50d3dba5569f..38e413e55189 100644 --- a/pkg/printers/teamcity.go +++ b/pkg/printers/teamcity.go @@ -66,7 +66,7 @@ func (p *TeamCity) Print(_ context.Context, issues []result.Issue) error { message: issue.Text, file: issue.FilePath(), line: issue.Line(), - additionalAttribute: issue.Severity, + additionalAttribute: strings.TrimSpace(issue.Severity), } _, err := instance.Print(p.w, p.escaper) @@ -104,8 +104,25 @@ type InspectionInstance struct { } func (i InspectionInstance) Print(w io.Writer, replacer *strings.Replacer) (int, error) { - return fmt.Fprintf(w, "##teamcity[inspection typeId='%s' message='%s' file='%s' line='%d' additional attribute='%s']\n", - limit(i.typeID, smallLimit), limit(replacer.Replace(i.message), largeLimit), limit(i.file, largeLimit), i.line, i.additionalAttribute) + _, err := fmt.Fprintf(w, "##teamcity[inspection typeId='%s' message='%s' file='%s' line='%d'", + limit(i.typeID, smallLimit), limit(replacer.Replace(i.message), largeLimit), limit(i.file, largeLimit), i.line) + if err != nil { + return 0, err + } + + if i.additionalAttribute != "" { + _, err = fmt.Fprintf(w, " additional attribute='%s'", i.additionalAttribute) + if err != nil { + return 0, err + } + } + + _, err = fmt.Fprintln(w, "]") + if err != nil { + return 0, err + } + + return 0, nil } func limit(s string, max int) string { From 7a49b5090c8df84f57bef5a3234324391458a04b Mon Sep 17 00:00:00 2001 From: ferhat elmas Date: Mon, 27 Feb 2023 14:18:49 +0100 Subject: [PATCH 06/12] review: tiny simplification for branching --- pkg/printers/teamcity.go | 7 +------ 1 file changed, 1 insertion(+), 6 deletions(-) diff --git a/pkg/printers/teamcity.go b/pkg/printers/teamcity.go index 38e413e55189..43d967c3170d 100644 --- a/pkg/printers/teamcity.go +++ b/pkg/printers/teamcity.go @@ -117,12 +117,7 @@ func (i InspectionInstance) Print(w io.Writer, replacer *strings.Replacer) (int, } } - _, err = fmt.Fprintln(w, "]") - if err != nil { - return 0, err - } - - return 0, nil + return fmt.Fprintln(w, "]") } func limit(s string, max int) string { From e52e27953a1533d968924b69948a0d86b22ed035 Mon Sep 17 00:00:00 2001 From: ferhat elmas Date: Mon, 27 Feb 2023 14:24:52 +0100 Subject: [PATCH 07/12] review: add test for limit --- pkg/printers/teamcity_test.go | 38 +++++++++++++++++++++++++++++++++++ 1 file changed, 38 insertions(+) diff --git a/pkg/printers/teamcity_test.go b/pkg/printers/teamcity_test.go index 050a3e4cd600..c7f81cd968c7 100644 --- a/pkg/printers/teamcity_test.go +++ b/pkg/printers/teamcity_test.go @@ -68,3 +68,41 @@ func TestTeamCity_Print(t *testing.T) { assert.Equal(t, expected, buf.String()) } + +func TestLimit(t *testing.T) { + tests := []struct { + input string + max int + expected string + }{ + { + input: "golangci-lint", + max: 0, + expected: "", + }, + { + input: "golangci-lint", + max: 8, + expected: "golangci", + }, + { + input: "golangci-lint", + max: 13, + expected: "golangci-lint", + }, + { + input: "golangci-lint", + max: 15, + expected: "golangci-lint", + }, + { + input: "こんにちは", + max: 3, + expected: "こんに", + }, + } + + for _, tc := range tests { + require.Equal(t, tc.expected, limit(tc.input, tc.max)) + } +} From 748c5ba11aeb8bae946fed8fcde2188b66b66f21 Mon Sep 17 00:00:00 2001 From: ferhat elmas Date: Mon, 27 Feb 2023 14:27:31 +0100 Subject: [PATCH 08/12] review: add comment for additional attr --- pkg/printers/teamcity.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/printers/teamcity.go b/pkg/printers/teamcity.go index 43d967c3170d..0e089fd6c491 100644 --- a/pkg/printers/teamcity.go +++ b/pkg/printers/teamcity.go @@ -100,7 +100,7 @@ type InspectionInstance struct { message string // (optional) limited by 4000 characters. file string // (mandatory) file path limited by 4000 characters. line int // (optional) line of the file, integer. - additionalAttribute string + additionalAttribute string // (optional) severity of the inspection; debug, info, warning, error, etc. } func (i InspectionInstance) Print(w io.Writer, replacer *strings.Replacer) (int, error) { From 314b687589015c33ee40ebe602557f61a5d23c77 Mon Sep 17 00:00:00 2001 From: ferhat elmas Date: Mon, 27 Feb 2023 14:35:30 +0100 Subject: [PATCH 09/12] review: drop comment for additional attr Co-authored-by: Ludovic Fernandez --- pkg/printers/teamcity.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/printers/teamcity.go b/pkg/printers/teamcity.go index 0e089fd6c491..ec3e48bcf535 100644 --- a/pkg/printers/teamcity.go +++ b/pkg/printers/teamcity.go @@ -100,7 +100,7 @@ type InspectionInstance struct { message string // (optional) limited by 4000 characters. file string // (mandatory) file path limited by 4000 characters. line int // (optional) line of the file, integer. - additionalAttribute string // (optional) severity of the inspection; debug, info, warning, error, etc. + additionalAttribute string // (optional) can be any attribute. } func (i InspectionInstance) Print(w io.Writer, replacer *strings.Replacer) (int, error) { From f40b25d0354f364dc7f6256f7f408a16eaecb7fc Mon Sep 17 00:00:00 2001 From: Oleksandr Redko Date: Mon, 27 Feb 2023 18:29:15 +0200 Subject: [PATCH 10/12] fix TeamCity printer Changes: - print SEVERITY; - populate inspectionType from linter's Name and Desc; - test escaping; - extend .golangci.reference.yml with TeamCity; - minor documentation fixes. --- .golangci.reference.yml | 1 + pkg/commands/run.go | 2 +- pkg/printers/teamcity.go | 89 ++++++++++++++++++----------------- pkg/printers/teamcity_test.go | 54 ++++++++++++++++----- 4 files changed, 89 insertions(+), 57 deletions(-) diff --git a/.golangci.reference.yml b/.golangci.reference.yml index 241fb7fe4df2..e03c4aada9b7 100644 --- a/.golangci.reference.yml +++ b/.golangci.reference.yml @@ -2339,6 +2339,7 @@ severity: # - Code climate: https://docs.codeclimate.com/docs/issues#issue-severity # - Checkstyle: https://checkstyle.sourceforge.io/property_types.html#SeverityLevel # - GitHub: https://help.github.com/en/actions/reference/workflow-commands-for-github-actions#setting-an-error-message + # - TeamCity: https://www.jetbrains.com/help/teamcity/service-messages.html#Inspection+Instance # # Default value is an empty string. default-severity: error diff --git a/pkg/commands/run.go b/pkg/commands/run.go index a9971f5e02b2..6417a6b072b9 100644 --- a/pkg/commands/run.go +++ b/pkg/commands/run.go @@ -495,7 +495,7 @@ func (e *Executor) createPrinter(format string, w io.Writer) (printers.Printer, case config.OutFormatGithubActions: p = printers.NewGithub(w) case config.OutFormatTeamCity: - p = printers.NewTeamCity(w) + p = printers.NewTeamCity(w, e.DBManager) default: return nil, fmt.Errorf("unknown output format %s", format) } diff --git a/pkg/printers/teamcity.go b/pkg/printers/teamcity.go index ec3e48bcf535..1d5ef1e3b9ff 100644 --- a/pkg/printers/teamcity.go +++ b/pkg/printers/teamcity.go @@ -7,6 +7,7 @@ import ( "strings" "unicode/utf8" + "github.com/golangci/golangci-lint/pkg/lint/linter" "github.com/golangci/golangci-lint/pkg/result" ) @@ -16,16 +17,23 @@ const ( largeLimit = 4000 ) +// Configer used for accessing linter.Config by its name for printing instanceType values. +type Configer interface { + GetLinterConfigs(name string) []*linter.Config +} + // TeamCity printer for TeamCity format. type TeamCity struct { w io.Writer + conf Configer escaper *strings.Replacer } -// NewTeamCity output format outputs issues according to TeamCity service message format -func NewTeamCity(w io.Writer) *TeamCity { +// NewTeamCity output format outputs issues according to TeamCity service message format. +func NewTeamCity(w io.Writer, conf Configer) *TeamCity { return &TeamCity{ - w: w, + w: w, + conf: conf, // https://www.jetbrains.com/help/teamcity/service-messages.html#Escaped+Values escaper: strings.NewReplacer( "'", "|'", @@ -46,27 +54,30 @@ func (p *TeamCity) Print(_ context.Context, issues []result.Issue) error { _, ok := uniqLinters[issue.FromLinter] if !ok { - inspectionType := InspectionType{ - id: issue.FromLinter, - name: issue.FromLinter, - description: issue.FromLinter, - category: "Golangci-lint reports", - } - - _, err := inspectionType.Print(p.w, p.escaper) - if err != nil { - return err + linterConfigs := p.conf.GetLinterConfigs(issue.FromLinter) + for _, config := range linterConfigs { + inspectionType := inspectionType{ + id: config.Linter.Name(), + name: config.Linter.Name(), + description: config.Linter.Desc(), + category: "Golangci-lint reports", + } + + _, err := inspectionType.Print(p.w, p.escaper) + if err != nil { + return err + } } uniqLinters[issue.FromLinter] = struct{}{} } - instance := InspectionInstance{ - typeID: issue.FromLinter, - message: issue.Text, - file: issue.FilePath(), - line: issue.Line(), - additionalAttribute: strings.TrimSpace(issue.Severity), + instance := inspectionInstance{ + typeID: issue.FromLinter, + message: issue.Text, + file: issue.FilePath(), + line: issue.Line(), + severity: issue.Severity, } _, err := instance.Print(p.w, p.escaper) @@ -78,46 +89,36 @@ func (p *TeamCity) Print(_ context.Context, issues []result.Issue) error { return nil } -// InspectionType Each specific warning or an error in code (inspection instance) has an inspection type. +// inspectionType is the unique description of the conducted inspection. Each specific warning or +// an error in code (inspection instance) has an inspection type. // https://www.jetbrains.com/help/teamcity/service-messages.html#Inspection+Type -type InspectionType struct { +type inspectionType struct { id string // (mandatory) limited by 255 characters. name string // (mandatory) limited by 255 characters. description string // (mandatory) limited by 255 characters. category string // (mandatory) limited by 4000 characters. } -func (i InspectionType) Print(w io.Writer, escaper *strings.Replacer) (int, error) { +func (i inspectionType) Print(w io.Writer, escaper *strings.Replacer) (int, error) { return fmt.Fprintf(w, "##teamcity[inspectionType id='%s' name='%s' description='%s' category='%s']\n", limit(i.id, smallLimit), limit(i.name, smallLimit), limit(escaper.Replace(i.description), largeLimit), limit(i.category, smallLimit)) } -// InspectionInstance Reports a specific defect, warning, error message. +// inspectionInstance reports a specific defect, warning, error message. // Includes location, description, and various optional and custom attributes. // https://www.jetbrains.com/help/teamcity/service-messages.html#Inspection+Instance -type InspectionInstance struct { - typeID string // (mandatory) limited by 255 characters. - message string // (optional) limited by 4000 characters. - file string // (mandatory) file path limited by 4000 characters. - line int // (optional) line of the file, integer. - additionalAttribute string // (optional) can be any attribute. +type inspectionInstance struct { + typeID string // (mandatory) limited by 255 characters. + message string // (optional) limited by 4000 characters. + file string // (mandatory) file path limited by 4000 characters. + line int // (optional) line of the file. + severity string // (optional) severity attribute: INFO, ERROR, WARNING, WEAK WARNING. } -func (i InspectionInstance) Print(w io.Writer, replacer *strings.Replacer) (int, error) { - _, err := fmt.Fprintf(w, "##teamcity[inspection typeId='%s' message='%s' file='%s' line='%d'", - limit(i.typeID, smallLimit), limit(replacer.Replace(i.message), largeLimit), limit(i.file, largeLimit), i.line) - if err != nil { - return 0, err - } - - if i.additionalAttribute != "" { - _, err = fmt.Fprintf(w, " additional attribute='%s'", i.additionalAttribute) - if err != nil { - return 0, err - } - } - - return fmt.Fprintln(w, "]") +func (i inspectionInstance) Print(w io.Writer, replacer *strings.Replacer) (int, error) { + return fmt.Fprintf(w, "##teamcity[inspection typeId='%s' message='%s' file='%s' line='%d' SEVERITY='%s']\n", + limit(i.typeID, smallLimit), limit(replacer.Replace(i.message), largeLimit), limit(i.file, largeLimit), i.line, + strings.ToUpper(i.severity)) } func limit(s string, max int) string { diff --git a/pkg/printers/teamcity_test.go b/pkg/printers/teamcity_test.go index c7f81cd968c7..8bf7cd059ccd 100644 --- a/pkg/printers/teamcity_test.go +++ b/pkg/printers/teamcity_test.go @@ -9,6 +9,7 @@ import ( "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" + "github.com/golangci/golangci-lint/pkg/lint/linter" "github.com/golangci/golangci-lint/pkg/result" ) @@ -16,8 +17,8 @@ func TestTeamCity_Print(t *testing.T) { issues := []result.Issue{ { FromLinter: "linter-a", - Severity: "error", - Text: "some issue", + Severity: "warning", + Text: "warning issue", Pos: token.Position{ Filename: "path/to/filea.go", Offset: 2, @@ -28,7 +29,7 @@ func TestTeamCity_Print(t *testing.T) { { FromLinter: "linter-a", Severity: "error", - Text: "some issue 2", + Text: "error issue", Pos: token.Position{ Filename: "path/to/filea.go", Offset: 2, @@ -37,8 +38,8 @@ func TestTeamCity_Print(t *testing.T) { }, { FromLinter: "linter-b", - Severity: "error", - Text: "another issue", + Severity: "info", + Text: "info issue", SourceLines: []string{ "func foo() {", "\tfmt.Println(\"bar\")", @@ -54,22 +55,35 @@ func TestTeamCity_Print(t *testing.T) { } buf := new(bytes.Buffer) - printer := NewTeamCity(buf) + printer := NewTeamCity(buf, configerMock( + map[string][]*linter.Config{ + "linter-a": { + { + Linter: &linterMock{name: "linter-a", desc: "description for linter-a"}, + }, + }, + "linter-b": { + { + Linter: &linterMock{name: "linter-b", desc: "description for linter-b with escape '\n\r|[] characters"}, + }, + }, + }, + )) err := printer.Print(context.Background(), 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='some issue' file='path/to/filea.go' line='10' additional attribute='error'] -##teamcity[inspection typeId='linter-a' message='some issue 2' file='path/to/filea.go' line='10' additional attribute='error'] -##teamcity[inspectionType id='linter-b' name='linter-b' description='linter-b' category='Golangci-lint reports'] -##teamcity[inspection typeId='linter-b' message='another issue' file='path/to/fileb.go' line='300' additional attribute='error'] + expected := `##teamcity[inspectionType id='linter-a' name='linter-a' description='description for linter-a' category='Golangci-lint reports'] +##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='description for linter-b with escape |'|n|r|||[|] characters' category='Golangci-lint reports'] +##teamcity[inspection typeId='linter-b' message='info issue' file='path/to/fileb.go' line='300' SEVERITY='INFO'] ` assert.Equal(t, expected, buf.String()) } -func TestLimit(t *testing.T) { +func TestTeamCity_limit(t *testing.T) { tests := []struct { input string max int @@ -106,3 +120,19 @@ func TestLimit(t *testing.T) { require.Equal(t, tc.expected, limit(tc.input, tc.max)) } } + +type configerMock map[string][]*linter.Config + +func (c configerMock) GetLinterConfigs(name string) []*linter.Config { + return c[name] +} + +type linterMock struct { + linter.Noop + name string + desc string +} + +func (l linterMock) Name() string { return l.name } + +func (l linterMock) Desc() string { return l.desc } From f6b0cc2813ad46a30f473a46e8efde5f1c620688 Mon Sep 17 00:00:00 2001 From: Oleksandr Redko Date: Mon, 27 Feb 2023 19:39:47 +0200 Subject: [PATCH 11/12] Add nolint:lll --- pkg/printers/teamcity_test.go | 1 + 1 file changed, 1 insertion(+) diff --git a/pkg/printers/teamcity_test.go b/pkg/printers/teamcity_test.go index 8bf7cd059ccd..7446c9d08c58 100644 --- a/pkg/printers/teamcity_test.go +++ b/pkg/printers/teamcity_test.go @@ -73,6 +73,7 @@ func TestTeamCity_Print(t *testing.T) { err := printer.Print(context.Background(), issues) require.NoError(t, err) + //nolint:lll expected := `##teamcity[inspectionType id='linter-a' name='linter-a' description='description for linter-a' category='Golangci-lint reports'] ##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'] From a4cc230d5e5130dec7ffcbe3c9dd2fac0f2c2fd4 Mon Sep 17 00:00:00 2001 From: Fernandez Ludovic Date: Mon, 27 Feb 2023 21:03:10 +0100 Subject: [PATCH 12/12] review: remove the call to the DBManager --- pkg/commands/run.go | 2 +- pkg/config/output.go | 2 +- pkg/printers/teamcity.go | 59 +++++++++++++++-------------------- pkg/printers/teamcity_test.go | 43 +++---------------------- 4 files changed, 32 insertions(+), 74 deletions(-) diff --git a/pkg/commands/run.go b/pkg/commands/run.go index 6417a6b072b9..a9971f5e02b2 100644 --- a/pkg/commands/run.go +++ b/pkg/commands/run.go @@ -495,7 +495,7 @@ func (e *Executor) createPrinter(format string, w io.Writer) (printers.Printer, case config.OutFormatGithubActions: p = printers.NewGithub(w) case config.OutFormatTeamCity: - p = printers.NewTeamCity(w, e.DBManager) + p = printers.NewTeamCity(w) default: return nil, fmt.Errorf("unknown output format %s", format) } diff --git a/pkg/config/output.go b/pkg/config/output.go index fd425cf2513a..2c49ea7f4e66 100644 --- a/pkg/config/output.go +++ b/pkg/config/output.go @@ -10,7 +10,7 @@ const ( OutFormatHTML = "html" OutFormatJunitXML = "junit-xml" OutFormatGithubActions = "github-actions" - OutFormatTeamCity = "team-city" + OutFormatTeamCity = "teamcity" ) var OutFormats = []string{ diff --git a/pkg/printers/teamcity.go b/pkg/printers/teamcity.go index 1d5ef1e3b9ff..790f30a26fd1 100644 --- a/pkg/printers/teamcity.go +++ b/pkg/printers/teamcity.go @@ -7,7 +7,6 @@ import ( "strings" "unicode/utf8" - "github.com/golangci/golangci-lint/pkg/lint/linter" "github.com/golangci/golangci-lint/pkg/result" ) @@ -17,23 +16,16 @@ const ( largeLimit = 4000 ) -// Configer used for accessing linter.Config by its name for printing instanceType values. -type Configer interface { - GetLinterConfigs(name string) []*linter.Config -} - // TeamCity printer for TeamCity format. type TeamCity struct { w io.Writer - conf Configer escaper *strings.Replacer } // NewTeamCity output format outputs issues according to TeamCity service message format. -func NewTeamCity(w io.Writer, conf Configer) *TeamCity { +func NewTeamCity(w io.Writer) *TeamCity { return &TeamCity{ - w: w, - conf: conf, + w: w, // https://www.jetbrains.com/help/teamcity/service-messages.html#Escaped+Values escaper: strings.NewReplacer( "'", "|'", @@ -54,25 +46,22 @@ func (p *TeamCity) Print(_ context.Context, issues []result.Issue) error { _, ok := uniqLinters[issue.FromLinter] if !ok { - linterConfigs := p.conf.GetLinterConfigs(issue.FromLinter) - for _, config := range linterConfigs { - inspectionType := inspectionType{ - id: config.Linter.Name(), - name: config.Linter.Name(), - description: config.Linter.Desc(), - category: "Golangci-lint reports", - } - - _, err := inspectionType.Print(p.w, p.escaper) - if err != nil { - return err - } + inspectionType := InspectionType{ + id: issue.FromLinter, + name: issue.FromLinter, + description: issue.FromLinter, + category: "Golangci-lint reports", + } + + _, err := inspectionType.Print(p.w, p.escaper) + if err != nil { + return err } uniqLinters[issue.FromLinter] = struct{}{} } - instance := inspectionInstance{ + instance := InspectionInstance{ typeID: issue.FromLinter, message: issue.Text, file: issue.FilePath(), @@ -89,36 +78,38 @@ func (p *TeamCity) Print(_ context.Context, issues []result.Issue) error { return nil } -// inspectionType is the unique description of the conducted inspection. Each specific warning or +// InspectionType is the unique description of the conducted inspection. Each specific warning or // an error in code (inspection instance) has an inspection type. // https://www.jetbrains.com/help/teamcity/service-messages.html#Inspection+Type -type inspectionType struct { +type InspectionType struct { id string // (mandatory) limited by 255 characters. name string // (mandatory) limited by 255 characters. description string // (mandatory) limited by 255 characters. category string // (mandatory) limited by 4000 characters. } -func (i inspectionType) Print(w io.Writer, escaper *strings.Replacer) (int, error) { - return fmt.Fprintf(w, "##teamcity[inspectionType id='%s' name='%s' description='%s' category='%s']\n", +func (i InspectionType) Print(w io.Writer, escaper *strings.Replacer) (int, error) { + return fmt.Fprintf(w, "##teamcity[InspectionType id='%s' name='%s' description='%s' category='%s']\n", limit(i.id, smallLimit), limit(i.name, smallLimit), limit(escaper.Replace(i.description), largeLimit), limit(i.category, smallLimit)) } -// inspectionInstance reports a specific defect, warning, error message. +// InspectionInstance reports a specific defect, warning, error message. // Includes location, description, and various optional and custom attributes. // https://www.jetbrains.com/help/teamcity/service-messages.html#Inspection+Instance -type inspectionInstance struct { +type InspectionInstance struct { typeID string // (mandatory) limited by 255 characters. message string // (optional) limited by 4000 characters. file string // (mandatory) file path limited by 4000 characters. line int // (optional) line of the file. - severity string // (optional) severity attribute: INFO, ERROR, WARNING, WEAK WARNING. + severity string // (optional) any linter severity. } -func (i inspectionInstance) Print(w io.Writer, replacer *strings.Replacer) (int, error) { +func (i InspectionInstance) Print(w io.Writer, replacer *strings.Replacer) (int, error) { return fmt.Fprintf(w, "##teamcity[inspection typeId='%s' message='%s' file='%s' line='%d' SEVERITY='%s']\n", - limit(i.typeID, smallLimit), limit(replacer.Replace(i.message), largeLimit), limit(i.file, largeLimit), i.line, - strings.ToUpper(i.severity)) + limit(i.typeID, smallLimit), + limit(replacer.Replace(i.message), largeLimit), + limit(i.file, largeLimit), + i.line, strings.ToUpper(i.severity)) } func limit(s string, max int) string { diff --git a/pkg/printers/teamcity_test.go b/pkg/printers/teamcity_test.go index 7446c9d08c58..7f1843e9bec1 100644 --- a/pkg/printers/teamcity_test.go +++ b/pkg/printers/teamcity_test.go @@ -9,7 +9,6 @@ import ( "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" - "github.com/golangci/golangci-lint/pkg/lint/linter" "github.com/golangci/golangci-lint/pkg/result" ) @@ -17,7 +16,6 @@ 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", @@ -38,7 +36,6 @@ func TestTeamCity_Print(t *testing.T) { }, { FromLinter: "linter-b", - Severity: "info", Text: "info issue", SourceLines: []string{ "func foo() {", @@ -55,30 +52,16 @@ func TestTeamCity_Print(t *testing.T) { } buf := new(bytes.Buffer) - printer := NewTeamCity(buf, configerMock( - map[string][]*linter.Config{ - "linter-a": { - { - Linter: &linterMock{name: "linter-a", desc: "description for linter-a"}, - }, - }, - "linter-b": { - { - Linter: &linterMock{name: "linter-b", desc: "description for linter-b with escape '\n\r|[] characters"}, - }, - }, - }, - )) + printer := NewTeamCity(buf) err := printer.Print(context.Background(), issues) require.NoError(t, err) - //nolint:lll - expected := `##teamcity[inspectionType id='linter-a' name='linter-a' description='description for linter-a' category='Golangci-lint reports'] -##teamcity[inspection typeId='linter-a' message='warning issue' file='path/to/filea.go' line='10' SEVERITY='WARNING'] + 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='error issue' file='path/to/filea.go' line='10' SEVERITY='ERROR'] -##teamcity[inspectionType id='linter-b' name='linter-b' description='description for linter-b with escape |'|n|r|||[|] characters' category='Golangci-lint reports'] -##teamcity[inspection typeId='linter-b' message='info issue' file='path/to/fileb.go' line='300' SEVERITY='INFO'] +##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=''] ` assert.Equal(t, expected, buf.String()) @@ -121,19 +104,3 @@ func TestTeamCity_limit(t *testing.T) { require.Equal(t, tc.expected, limit(tc.input, tc.max)) } } - -type configerMock map[string][]*linter.Config - -func (c configerMock) GetLinterConfigs(name string) []*linter.Config { - return c[name] -} - -type linterMock struct { - linter.Noop - name string - desc string -} - -func (l linterMock) Name() string { return l.name } - -func (l linterMock) Desc() string { return l.desc }