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
25 changes: 25 additions & 0 deletions .golangci.example.yml
Expand Up @@ -382,3 +382,28 @@ issues:

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

severity:
# 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
default-severity: error

# The default value is false.
# If set to true severity-rules regular expressions become case sensitive.
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.
rules:
- linters:
- dupl
severity: info
1 change: 1 addition & 0 deletions go.mod
Expand Up @@ -9,6 +9,7 @@ require (
github.com/fatih/color v1.9.0
github.com/go-critic/go-critic v0.4.3
github.com/go-lintpack/lintpack v0.5.2
github.com/go-xmlfmt/xmlfmt v0.0.0-20191208150333-d5b6f63a941b
github.com/gofrs/flock v0.7.1
github.com/golangci/check v0.0.0-20180506172741-cfe4005ccda2
github.com/golangci/dupl v0.0.0-20180902072040-3e9179ac440a
Expand Down
65 changes: 46 additions & 19 deletions pkg/config/config.go
Expand Up @@ -407,51 +407,71 @@ type Linters struct {
Presets []string
}

type ExcludeRule struct {
type BaseRule struct {
Linters []string
Path string
Text string
Source string
}

func validateOptionalRegex(value string) error {
if value == "" {
return nil
}
_, err := regexp.Compile(value)
return err
}

func (e ExcludeRule) Validate() error {
if err := validateOptionalRegex(e.Path); err != nil {
func (b BaseRule) Validate(minConditionsCount int) error {
if err := validateOptionalRegex(b.Path); err != nil {
return fmt.Errorf("invalid path regex: %v", err)
}
if err := validateOptionalRegex(e.Text); err != nil {
if err := validateOptionalRegex(b.Text); err != nil {
return fmt.Errorf("invalid text regex: %v", err)
}
if err := validateOptionalRegex(e.Source); err != nil {
if err := validateOptionalRegex(b.Source); err != nil {
return fmt.Errorf("invalid source regex: %v", err)
}
nonBlank := 0
if len(e.Linters) > 0 {
if len(b.Linters) > 0 {
nonBlank++
}
if e.Path != "" {
if b.Path != "" {
nonBlank++
}
if e.Text != "" {
if b.Text != "" {
nonBlank++
}
if e.Source != "" {
if b.Source != "" {
nonBlank++
}
const minConditionsCount = 2
if nonBlank < minConditionsCount {
return errors.New("at least 2 of (text, source, path, linters) should be set")
return fmt.Errorf("at least %d of (text, source, path, linters) should be set", minConditionsCount)
}
return nil
}

const excludeRuleMinConditionsCount = 2

type ExcludeRule struct {
BaseRule `mapstructure:",squash"`
}

func validateOptionalRegex(value string) error {
if value == "" {
return nil
}
_, err := regexp.Compile(value)
return err
}

func (e ExcludeRule) Validate() error {
return e.BaseRule.Validate(excludeRuleMinConditionsCount)
}

const severityRuleMinConditionsCount = 1

type SeverityRule struct {
BaseRule `mapstructure:",squash"`
Severity string
}

func (s *SeverityRule) Validate() error {
return s.BaseRule.Validate(severityRuleMinConditionsCount)
}

type Issues struct {
IncludeDefaultExcludes []string `mapstructure:"include"`
ExcludeCaseSensitive bool `mapstructure:"exclude-case-sensitive"`
Expand All @@ -469,6 +489,12 @@ type Issues struct {
NeedFix bool `mapstructure:"fix"`
}

type Severity struct {
Default string `mapstructure:"default-severity"`
CaseSensitive bool `mapstructure:"case-sensitive"`
Rules []SeverityRule `mapstructure:"rules"`
}

type Config struct {
Run Run

Expand All @@ -484,6 +510,7 @@ type Config struct {
LintersSettings LintersSettings `mapstructure:"linters-settings"`
Linters Linters
Issues Issues
Severity Severity

InternalTest bool // Option is used only for testing golangci-lint code, don't use it
}
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.Severity.Rules) > 0 && c.Severity.Default == "" {
return errors.New("can't set severity rule option: no default severity defined")
}
for i, rule := range c.Severity.Rules {
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
131 changes: 92 additions & 39 deletions pkg/lint/runner.go
Expand Up @@ -31,24 +31,6 @@ type Runner struct {

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
excludePatterns := icfg.ExcludePatterns
if icfg.UseDefaultExcludes {
excludePatterns = append(excludePatterns, config.GetExcludePatternsStrings(icfg.IncludeDefaultExcludes)...)
}

var excludeTotalPattern string
if len(excludePatterns) != 0 {
excludeTotalPattern = fmt.Sprintf("(%s)", strings.Join(excludePatterns, "|"))
}

var excludeProcessor processors.Processor
if cfg.Issues.ExcludeCaseSensitive {
excludeProcessor = processors.NewExcludeCaseSensitive(excludeTotalPattern)
} else {
excludeProcessor = processors.NewExclude(excludeTotalPattern)
}

skipFilesProcessor, err := processors.NewSkipFiles(cfg.Run.SkipFiles)
if err != nil {
return nil, err
Expand All @@ -63,22 +45,6 @@ func NewRunner(cfg *config.Config, log logutils.Log, goenv *goutil.Env, es *lint
return nil, err
}

var excludeRules []processors.ExcludeRule
for _, r := range icfg.ExcludeRules {
excludeRules = append(excludeRules, processors.ExcludeRule{
Text: r.Text,
Source: r.Source,
Path: r.Path,
Linters: r.Linters,
})
}
var excludeRulesProcessor processors.Processor
if cfg.Issues.ExcludeCaseSensitive {
excludeRulesProcessor = processors.NewExcludeRulesCaseSensitive(excludeRules, lineCache, log.Child("exclude_rules"))
} else {
excludeRulesProcessor = processors.NewExcludeRules(excludeRules, lineCache, log.Child("exclude_rules"))
}

enabledLinters, err := es.GetEnabledLintersMap()
if err != nil {
return nil, errors.Wrap(err, "failed to get enabled linters")
Expand All @@ -101,17 +67,18 @@ func NewRunner(cfg *config.Config, log logutils.Log, goenv *goutil.Env, es *lint
// Must be before exclude because users see already marked output and configure excluding by it.
processors.NewIdentifierMarker(),

excludeProcessor,
excludeRulesProcessor,
getExcludeProcessor(&cfg.Issues),
getExcludeRulesProcessor(&cfg.Issues, log, lineCache),
processors.NewNolint(log.Child("nolint"), dbManager, enabledLinters),

processors.NewUniqByLine(cfg),
processors.NewDiff(icfg.Diff, icfg.DiffFromRevision, icfg.DiffPatchFilePath),
processors.NewDiff(cfg.Issues.Diff, cfg.Issues.DiffFromRevision, cfg.Issues.DiffPatchFilePath),
processors.NewMaxPerFileFromLinter(cfg),
processors.NewMaxSameIssues(icfg.MaxSameIssues, log.Child("max_same_issues"), cfg),
processors.NewMaxFromLinter(icfg.MaxIssuesPerLinter, log.Child("max_from_linter"), cfg),
processors.NewMaxSameIssues(cfg.Issues.MaxSameIssues, log.Child("max_same_issues"), cfg),
processors.NewMaxFromLinter(cfg.Issues.MaxIssuesPerLinter, log.Child("max_from_linter"), cfg),
processors.NewSourceCode(lineCache, log.Child("source_code")),
processors.NewPathShortener(),
getSeverityRulesProcessor(&cfg.Severity, log, lineCache),
},
Log: log,
}, nil
Expand Down Expand Up @@ -254,3 +221,89 @@ func (r *Runner) processIssues(issues []result.Issue, sw *timeutils.Stopwatch, s

return issues
}

func getExcludeProcessor(cfg *config.Issues) processors.Processor {
excludePatterns := cfg.ExcludePatterns
if cfg.UseDefaultExcludes {
excludePatterns = append(excludePatterns, config.GetExcludePatternsStrings(cfg.IncludeDefaultExcludes)...)
}

var excludeTotalPattern string
if len(excludePatterns) != 0 {
excludeTotalPattern = fmt.Sprintf("(%s)", strings.Join(excludePatterns, "|"))
}

var excludeProcessor processors.Processor
if cfg.ExcludeCaseSensitive {
excludeProcessor = processors.NewExcludeCaseSensitive(excludeTotalPattern)
} else {
excludeProcessor = processors.NewExclude(excludeTotalPattern)
}

return excludeProcessor
}

func getExcludeRulesProcessor(cfg *config.Issues, log logutils.Log, lineCache *fsutils.LineCache) processors.Processor {
var excludeRules []processors.ExcludeRule
for _, r := range cfg.ExcludeRules {
excludeRules = append(excludeRules, processors.ExcludeRule{
BaseRule: processors.BaseRule{
Text: r.Text,
Source: r.Source,
Path: r.Path,
Linters: r.Linters,
},
})
}

var excludeRulesProcessor processors.Processor
if cfg.ExcludeCaseSensitive {
excludeRulesProcessor = processors.NewExcludeRulesCaseSensitive(
excludeRules,
lineCache,
log.Child("exclude_rules"),
)
} else {
excludeRulesProcessor = processors.NewExcludeRules(
excludeRules,
lineCache,
log.Child("exclude_rules"),
)
}

return excludeRulesProcessor
}

func getSeverityRulesProcessor(cfg *config.Severity, log logutils.Log, lineCache *fsutils.LineCache) processors.Processor {
var severityRules []processors.SeverityRule
for _, r := range cfg.Rules {
severityRules = append(severityRules, processors.SeverityRule{
Severity: r.Severity,
BaseRule: processors.BaseRule{
Text: r.Text,
Source: r.Source,
Path: r.Path,
Linters: r.Linters,
},
})
}

var severityRulesProcessor processors.Processor
if cfg.CaseSensitive {
severityRulesProcessor = processors.NewSeverityRulesCaseSensitive(
cfg.Default,
severityRules,
lineCache,
log.Child("severity_rules"),
)
} else {
severityRulesProcessor = processors.NewSeverityRules(
cfg.Default,
severityRules,
lineCache,
log.Child("severity_rules"),
)
}

return severityRulesProcessor
}
13 changes: 10 additions & 3 deletions pkg/printers/checkstyle.go
Expand Up @@ -5,6 +5,8 @@ import (
"encoding/xml"
"fmt"

"github.com/go-xmlfmt/xmlfmt"

"github.com/golangci/golangci-lint/pkg/logutils"
"github.com/golangci/golangci-lint/pkg/result"
)
Expand All @@ -28,7 +30,7 @@ type checkstyleError struct {
Source string `xml:"source,attr"`
}

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

type Checkstyle struct{}

Expand All @@ -54,12 +56,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 All @@ -75,6 +82,6 @@ func (Checkstyle) Print(ctx context.Context, issues []result.Issue) error {
return err
}

fmt.Fprintf(logutils.StdOut, "%s%s\n", xml.Header, data)
fmt.Fprintf(logutils.StdOut, "%s%s\n", xml.Header, xmlfmt.FormatXML(string(data), "", " "))
return nil
}