Skip to content
New issue

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

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

Already on GitHub? Sign in to your account

add ability to set issue severity #1155

Merged
merged 10 commits into from May 25, 2020
2 changes: 2 additions & 0 deletions .gitignore
Expand Up @@ -15,3 +15,5 @@
/.vscode/
*.test
.DS_Store
coverage.out
coverage.xml
24 changes: 24 additions & 0 deletions .golangci.example.yml
Expand Up @@ -382,3 +382,27 @@ issues:

# Show only new issues created in git patch with set file path.
new-from-patch: path/to/patch/file

# Default value is empty string.
# Set the default severity for issues. If severity rules are defined and the issues
# do not match or no severity is provided to the rule this will be the default
# severity applied. Severities should match the supported severity names of the
# selected out format.
# - Code climate: https://docs.codeclimate.com/docs/issues#issue-severity
# - Checkstyle: https://checkstyle.sourceforge.io/property_types.html#severity
# - Github: https://help.github.com/en/actions/reference/workflow-commands-for-github-actions#setting-an-error-message
severity-default: error
ryancurrah marked this conversation as resolved.
Show resolved Hide resolved

# The default value is false.
# If set to true severity-rules regular expressions become case sensitive.
severity-case-sensitive: false

# Default value is empty list.
# When a list of severity rules are provided, severity information will be added to lint
# issues. Severity rules have the same filtering capability as exclude rules except you
# are allowed to specify one matcher per severity rule.
# Only affects out formats that support setting severity information.
severity-rules:
- linters:
- dupl
severity: info
44 changes: 43 additions & 1 deletion pkg/config/config.go
Expand Up @@ -422,7 +422,7 @@ func validateOptionalRegex(value string) error {
return err
}

func (e ExcludeRule) Validate() error {
func (e ExcludeRule) Validate() error { // nolint:dupl
if err := validateOptionalRegex(e.Path); err != nil {
return fmt.Errorf("invalid path regex: %v", err)
}
Expand Down Expand Up @@ -452,13 +452,55 @@ func (e ExcludeRule) Validate() error {
return nil
}

type SeverityRule struct {
Severity string
Linters []string
Path string
Text string
Source string
}

func (s *SeverityRule) Validate() error { // nolint:dupl
ryancurrah marked this conversation as resolved.
Show resolved Hide resolved
if err := validateOptionalRegex(s.Path); err != nil {
return fmt.Errorf("invalid path regex: %v", err)
}
if err := validateOptionalRegex(s.Text); err != nil {
return fmt.Errorf("invalid text regex: %v", err)
}
if err := validateOptionalRegex(s.Source); err != nil {
return fmt.Errorf("invalid source regex: %v", err)
}
nonBlank := 0
if len(s.Linters) > 0 {
nonBlank++
}
if s.Path != "" {
nonBlank++
}
if s.Text != "" {
nonBlank++
}
if s.Source != "" {
nonBlank++
}
const minConditionsCount = 1
if nonBlank < minConditionsCount {
return errors.New("at least 1 of (text, source, path, linters) should be set")
}
return nil
}

type Issues struct {
IncludeDefaultExcludes []string `mapstructure:"include"`
ExcludeCaseSensitive bool `mapstructure:"exclude-case-sensitive"`
ExcludePatterns []string `mapstructure:"exclude"`
ExcludeRules []ExcludeRule `mapstructure:"exclude-rules"`
UseDefaultExcludes bool `mapstructure:"exclude-use-default"`

SeverityDefault string `mapstructure:"severity-default"`
SeverityCaseSensitive bool `mapstructure:"severity-case-sensitive"`
SeverityRules []SeverityRule `mapstructure:"severity-rules"`

MaxIssuesPerLinter int `mapstructure:"max-issues-per-linter"`
MaxSameIssues int `mapstructure:"max-same-issues"`

Expand Down
8 changes: 8 additions & 0 deletions pkg/config/reader.go
Expand Up @@ -113,6 +113,14 @@ func (r *FileReader) validateConfig() error {
return fmt.Errorf("error in exclude rule #%d: %v", i, err)
}
}
if len(c.Issues.SeverityRules) > 0 && c.Issues.SeverityDefault == "" {
return errors.New("can't set severity rule option: no severity default defined")
}
for i, rule := range c.Issues.SeverityRules {
if err := rule.Validate(); err != nil {
return fmt.Errorf("error in severity rule #%d: %v", i, err)
}
}
if err := c.LintersSettings.Govet.Validate(); err != nil {
return fmt.Errorf("error in govet config: %v", err)
}
Expand Down
30 changes: 30 additions & 0 deletions pkg/lint/runner.go
Expand Up @@ -29,6 +29,7 @@ type Runner struct {
Log logutils.Log
}

//nolint:funlen
ryancurrah marked this conversation as resolved.
Show resolved Hide resolved
func NewRunner(cfg *config.Config, log logutils.Log, goenv *goutil.Env, es *lintersdb.EnabledSet,
lineCache *fsutils.LineCache, dbManager *lintersdb.Manager, pkgs []*gopackages.Package) (*Runner, error) {
icfg := cfg.Issues
Expand Down Expand Up @@ -84,6 +85,34 @@ func NewRunner(cfg *config.Config, log logutils.Log, goenv *goutil.Env, es *lint
return nil, errors.Wrap(err, "failed to get enabled linters")
}

var severityRules []processors.SeverityRule
for _, r := range icfg.SeverityRules {
severityRules = append(severityRules, processors.SeverityRule{
Severity: r.Severity,
Text: r.Text,
Source: r.Source,
Path: r.Path,
Linters: r.Linters,
})
}

var severityRulesProcessor processors.Processor
if cfg.Issues.SeverityCaseSensitive {
severityRulesProcessor = processors.NewSeverityRulesCaseSensitive(
icfg.SeverityDefault,
severityRules,
lineCache,
log.Child("severity_rules"),
)
} else {
severityRulesProcessor = processors.NewSeverityRules(
icfg.SeverityDefault,
severityRules,
lineCache,
log.Child("severity_rules"),
)
}

return &Runner{
Processors: []processors.Processor{
processors.NewCgo(goenv),
Expand Down Expand Up @@ -112,6 +141,7 @@ func NewRunner(cfg *config.Config, log logutils.Log, goenv *goutil.Env, es *lint
processors.NewMaxFromLinter(icfg.MaxIssuesPerLinter, log.Child("max_from_linter"), cfg),
processors.NewSourceCode(lineCache, log.Child("source_code")),
processors.NewPathShortener(),
severityRulesProcessor,
},
Log: log,
}, nil
Expand Down
9 changes: 7 additions & 2 deletions pkg/printers/checkstyle.go
Expand Up @@ -28,7 +28,7 @@ type checkstyleError struct {
Source string `xml:"source,attr"`
}

const defaultSeverity = "error"
const defaultCheckstyleSeverity = "error"

type Checkstyle struct{}

Expand All @@ -54,12 +54,17 @@ func (Checkstyle) Print(ctx context.Context, 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: defaultSeverity,
Severity: severity,
}

file.Errors = append(file.Errors, newError)
Expand Down
5 changes: 5 additions & 0 deletions pkg/printers/codeclimate.go
Expand Up @@ -14,6 +14,7 @@ import (
// It is just enough to support GitLab CI Code Quality - https://docs.gitlab.com/ee/user/project/merge_requests/code_quality.html
type CodeClimateIssue struct {
Description string `json:"description"`
Severity string `json:"severity,omitempty"`
Fingerprint string `json:"fingerprint"`
Location struct {
Path string `json:"path"`
Expand All @@ -39,6 +40,10 @@ func (p CodeClimate) Print(ctx context.Context, issues []result.Issue) error {
issue.Location.Path = i.Pos.Filename
issue.Location.Lines.Begin = i.Pos.Line

if i.Severity != "" {
issue.Severity = i.Severity
}

// Need a checksum of the issue, so we use MD5 of the filename, text, and first line of source if there is any
var firstLine string
if len(i.SourceLines) > 0 {
Expand Down
9 changes: 8 additions & 1 deletion pkg/printers/github.go
Expand Up @@ -11,6 +11,8 @@ import (
type github struct {
}

const defaultGithubSeverity = "error"

// Github output format outputs issues according to Github actions format:
// https://help.github.com/en/actions/reference/workflow-commands-for-github-actions#setting-an-error-message
func NewGithub() Printer {
Expand All @@ -19,7 +21,12 @@ func NewGithub() Printer {

// print each line as: ::error file=app.js,line=10,col=15::Something went wrong
func formatIssueAsGithub(issue *result.Issue) string {
ret := fmt.Sprintf("::error file=%s,line=%d", issue.FilePath(), issue.Line())
severity := defaultGithubSeverity
if issue.Severity != "" {
severity = issue.Severity
}

ret := fmt.Sprintf("::%s file=%s,line=%d", severity, issue.FilePath(), issue.Line())
if issue.Pos.Column != 0 {
ret += fmt.Sprintf(",col=%d", issue.Pos.Column)
}
Expand Down
2 changes: 2 additions & 0 deletions pkg/result/issue.go
Expand Up @@ -26,6 +26,8 @@ type Issue struct {
FromLinter string
Text string

Severity string

// Source lines of a code with the issue to show
SourceLines []string

Expand Down
2 changes: 1 addition & 1 deletion pkg/result/processors/exclude_rules.go
Expand Up @@ -97,7 +97,7 @@ func (p ExcludeRules) matchSource(i *result.Issue, r *excludeRule) bool { //noli
return r.source.MatchString(sourceLine)
}

func (p ExcludeRules) match(i *result.Issue, r *excludeRule) bool {
func (p ExcludeRules) match(i *result.Issue, r *excludeRule) bool { //nolint:dupl
if r.isEmpty() {
return false
}
Expand Down
43 changes: 12 additions & 31 deletions pkg/result/processors/exclude_rules_test.go
@@ -1,7 +1,6 @@
package processors

import (
"go/token"
"path/filepath"
"testing"

Expand Down Expand Up @@ -32,7 +31,7 @@ func TestExcludeRulesMultiple(t *testing.T) {
},
}, lineCache, nil)

cases := []issueCase{
cases := []issueTestCase{
{Path: "e.go", Text: "exclude", Linter: "linter"},
{Path: "e.go", Text: "some", Linter: "linter"},
{Path: "e_test.go", Text: "normal", Linter: "testlinter"},
Expand All @@ -43,44 +42,26 @@ func TestExcludeRulesMultiple(t *testing.T) {
}
var issues []result.Issue
for _, c := range cases {
issues = append(issues, newIssueCase(c))
issues = append(issues, newIssueFromIssueTestCase(c))
}
processedIssues := process(t, p, issues...)
var resultingCases []issueCase
var resultingCases []issueTestCase
for _, i := range processedIssues {
resultingCases = append(resultingCases, issueCase{
resultingCases = append(resultingCases, issueTestCase{
Path: i.FilePath(),
Linter: i.FromLinter,
Text: i.Text,
Line: i.Line(),
})
}
expectedCases := []issueCase{
expectedCases := []issueTestCase{
{Path: "e.go", Text: "some", Linter: "linter"},
{Path: "e_Test.go", Text: "normal", Linter: "testlinter"},
{Path: "e_test.go", Text: "another", Linter: "linter"},
}
assert.Equal(t, expectedCases, resultingCases)
}

type issueCase struct {
Path string
Line int
Text string
Linter string
}

func newIssueCase(c issueCase) result.Issue {
return result.Issue{
Text: c.Text,
FromLinter: c.Linter,
Pos: token.Position{
Filename: c.Path,
Line: c.Line,
},
}
}

func TestExcludeRulesText(t *testing.T) {
p := NewExcludeRules([]ExcludeRule{
{
Expand Down Expand Up @@ -110,7 +91,7 @@ func TestExcludeRulesText(t *testing.T) {
}

func TestExcludeRulesEmpty(t *testing.T) {
processAssertSame(t, NewExcludeRules(nil, nil, nil), newTextIssue("test"))
processAssertSame(t, NewExcludeRules(nil, nil, nil), newIssueFromTextTestCase("test"))
}

func TestExcludeRulesCaseSensitiveMultiple(t *testing.T) {
Expand All @@ -134,7 +115,7 @@ func TestExcludeRulesCaseSensitiveMultiple(t *testing.T) {
},
}, lineCache, nil)

cases := []issueCase{
cases := []issueTestCase{
{Path: "e.go", Text: "exclude", Linter: "linter"},
{Path: "e.go", Text: "excLude", Linter: "linter"},
{Path: "e.go", Text: "some", Linter: "linter"},
Expand All @@ -147,19 +128,19 @@ func TestExcludeRulesCaseSensitiveMultiple(t *testing.T) {
}
var issues []result.Issue
for _, c := range cases {
issues = append(issues, newIssueCase(c))
issues = append(issues, newIssueFromIssueTestCase(c))
}
processedIssues := process(t, p, issues...)
var resultingCases []issueCase
var resultingCases []issueTestCase
for _, i := range processedIssues {
resultingCases = append(resultingCases, issueCase{
resultingCases = append(resultingCases, issueTestCase{
Path: i.FilePath(),
Linter: i.FromLinter,
Text: i.Text,
Line: i.Line(),
})
}
expectedCases := []issueCase{
expectedCases := []issueTestCase{
{Path: "e.go", Text: "excLude", Linter: "linter"},
{Path: "e.go", Text: "some", Linter: "linter"},
{Path: "e_Test.go", Text: "normal", Linter: "testlinter"},
Expand Down Expand Up @@ -199,5 +180,5 @@ func TestExcludeRulesCaseSensitiveText(t *testing.T) {
}

func TestExcludeRulesCaseSensitiveEmpty(t *testing.T) {
processAssertSame(t, NewExcludeRulesCaseSensitive(nil, nil, nil), newTextIssue("test"))
processAssertSame(t, NewExcludeRulesCaseSensitive(nil, nil, nil), newIssueFromTextTestCase("test"))
}