Skip to content

Commit

Permalink
fix: sanitize severities by output format (#5359)
Browse files Browse the repository at this point in the history
  • Loading branch information
@ldez
ldez authored Jan 31, 2025
1 parent 15a92de commit 980a911
Show file tree
Hide file tree
Showing 20 changed files with 454 additions and 230 deletions.
8 changes: 6 additions & 2 deletions pkg/logutils/logutils.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
72 changes: 43 additions & 29 deletions pkg/printers/checkstyle.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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",
}
Expand All @@ -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 {
Expand All @@ -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"`
}
57 changes: 55 additions & 2 deletions pkg/printers/checkstyle_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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"
)

Expand Down Expand Up @@ -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 := "<?xml version=\"1.0\" encoding=\"UTF-8\"?>\n\n<checkstyle version=\"5.0\">\n <file name=\"path/to/filea.go\">\n <error column=\"4\" line=\"10\" message=\"some issue\" severity=\"warning\" source=\"linter-a\"></error>\n </file>\n <file name=\"path/to/fileb.go\">\n <error column=\"9\" line=\"300\" message=\"another issue\" severity=\"error\" source=\"linter-b\"></error>\n </file>\n</checkstyle>\n"
expected := `<?xml version="1.0" encoding="UTF-8"?>
<checkstyle version="5.0">
<file name="path/to/filea.go">
<error column="4" line="10" message="some issue" severity="warning" source="linter-a"></error>
</file>
<file name="path/to/fileb.go">
<error column="9" line="300" message="another issue" severity="error" source="linter-b"></error>
</file>
<file name="path/to/filec.go">
<error column="10" line="300" message="without severity" severity="error" source="linter-c"></error>
</file>
<file name="path/to/filed.go">
<error column="11" line="300" message="unknown severity" severity="error" source="linter-d"></error>
</file>
</checkstyle>
`

assert.Equal(t, expected, strings.ReplaceAll(buf.String(), "\r", ""))
}
70 changes: 39 additions & 31 deletions pkg/printers/codeclimate.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3,63 +3,71 @@ 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()
codeClimateIssue.CheckName = issue.FromLinter
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"`
}
38 changes: 30 additions & 8 deletions pkg/printers/codeclimate_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -8,14 +8,15 @@ 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"
)

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",
Expand All @@ -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())
Expand Down
2 changes: 2 additions & 0 deletions pkg/printers/html.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
Expand Down
Loading

0 comments on commit 980a911

Please sign in to comment.