From fa7adcbda949c940968107bda093a5dae022b8e3 Mon Sep 17 00:00:00 2001 From: Ryan Currah Date: Mon, 25 May 2020 08:21:42 -0400 Subject: [PATCH] add ability to set issue severity (#1155) * add ability to set issue severity for out formats that support it based on severity rules * fix lint issues * change log child name * code climate omit severity if empty * add tests for severity rules, add support for case sensitive rules, fix lint issues, better doc comments, share processor test * deduplicated rule logic into a base rule that can be used by multiple rule types, moved severity config to it's own parent key named severity, reduced size of NewRunner function to make it easier to read * put validate function under base rule struct * better validation error wording * add Fingerprint and Description methods to Issue struct, made codeclimate reporter easier to read, checkstyle output is now pretty printed --- .gitignore | 2 + .golangci.example.yml | 25 +++ go.mod | 1 + pkg/config/config.go | 83 ++++++--- pkg/config/reader.go | 8 + pkg/lint/runner.go | 131 +++++++++---- pkg/printers/checkstyle.go | 13 +- pkg/printers/codeclimate.go | 31 ++-- pkg/printers/github.go | 9 +- pkg/result/issue.go | 20 ++ pkg/result/processors/base_rule.go | 69 +++++++ pkg/result/processors/exclude_rules.go | 63 +------ pkg/result/processors/exclude_rules_test.go | 103 +++++------ pkg/result/processors/exclude_test.go | 28 +-- pkg/result/processors/processor_test.go | 51 ++++++ pkg/result/processors/severity_rules.go | 103 +++++++++++ pkg/result/processors/severity_rules_test.go | 173 ++++++++++++++++++ .../processors/testdata/severity_rules.go | 3 + 18 files changed, 691 insertions(+), 225 deletions(-) create mode 100644 pkg/result/processors/base_rule.go create mode 100644 pkg/result/processors/processor_test.go create mode 100644 pkg/result/processors/severity_rules.go create mode 100644 pkg/result/processors/severity_rules_test.go create mode 100644 pkg/result/processors/testdata/severity_rules.go diff --git a/.gitignore b/.gitignore index 896f0c3e..8e5b12b5 100644 --- a/.gitignore +++ b/.gitignore @@ -15,3 +15,5 @@ /.vscode/ *.test .DS_Store +coverage.out +coverage.xml \ No newline at end of file diff --git a/.golangci.example.yml b/.golangci.example.yml index 8ee42020..2a1fd199 100644 --- a/.golangci.example.yml +++ b/.golangci.example.yml @@ -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 diff --git a/go.mod b/go.mod index 0030f467..49812525 100644 --- a/go.mod +++ b/go.mod @@ -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 diff --git a/pkg/config/config.go b/pkg/config/config.go index c9175fc6..b29e8f60 100644 --- a/pkg/config/config.go +++ b/pkg/config/config.go @@ -407,13 +407,48 @@ type Linters struct { Presets []string } -type ExcludeRule struct { +type BaseRule struct { Linters []string Path string Text string Source string } +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(b.Text); err != nil { + return fmt.Errorf("invalid text regex: %v", err) + } + if err := validateOptionalRegex(b.Source); err != nil { + return fmt.Errorf("invalid source regex: %v", err) + } + nonBlank := 0 + if len(b.Linters) > 0 { + nonBlank++ + } + if b.Path != "" { + nonBlank++ + } + if b.Text != "" { + nonBlank++ + } + if b.Source != "" { + nonBlank++ + } + if nonBlank < minConditionsCount { + 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 @@ -423,33 +458,18 @@ func validateOptionalRegex(value string) error { } func (e ExcludeRule) Validate() error { - if err := validateOptionalRegex(e.Path); err != nil { - return fmt.Errorf("invalid path regex: %v", err) - } - if err := validateOptionalRegex(e.Text); err != nil { - return fmt.Errorf("invalid text regex: %v", err) - } - if err := validateOptionalRegex(e.Source); err != nil { - return fmt.Errorf("invalid source regex: %v", err) - } - nonBlank := 0 - if len(e.Linters) > 0 { - nonBlank++ - } - if e.Path != "" { - nonBlank++ - } - if e.Text != "" { - nonBlank++ - } - if e.Source != "" { - nonBlank++ - } - const minConditionsCount = 2 - if nonBlank < minConditionsCount { - return errors.New("at least 2 of (text, source, path, linters) should be set") - } - return nil + 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 { @@ -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 @@ -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 } diff --git a/pkg/config/reader.go b/pkg/config/reader.go index 1e355e72..e6b18a7a 100644 --- a/pkg/config/reader.go +++ b/pkg/config/reader.go @@ -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) } diff --git a/pkg/lint/runner.go b/pkg/lint/runner.go index 05dc51ba..f77778b2 100644 --- a/pkg/lint/runner.go +++ b/pkg/lint/runner.go @@ -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 @@ -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") @@ -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 @@ -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 +} diff --git a/pkg/printers/checkstyle.go b/pkg/printers/checkstyle.go index f36bc108..c5b948a9 100644 --- a/pkg/printers/checkstyle.go +++ b/pkg/printers/checkstyle.go @@ -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" ) @@ -28,7 +30,7 @@ type checkstyleError struct { Source string `xml:"source,attr"` } -const defaultSeverity = "error" +const defaultCheckstyleSeverity = "error" type Checkstyle struct{} @@ -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) @@ -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 } diff --git a/pkg/printers/codeclimate.go b/pkg/printers/codeclimate.go index 5d45c4eb..35a22ce9 100644 --- a/pkg/printers/codeclimate.go +++ b/pkg/printers/codeclimate.go @@ -2,7 +2,6 @@ package printers import ( "context" - "crypto/md5" //nolint:gosec "encoding/json" "fmt" @@ -14,6 +13,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"` @@ -31,28 +31,23 @@ func NewCodeClimate() *CodeClimate { } func (p CodeClimate) Print(ctx context.Context, issues []result.Issue) error { - allIssues := []CodeClimateIssue{} - for ind := range issues { - i := &issues[ind] - var issue CodeClimateIssue - issue.Description = i.FromLinter + ": " + i.Text - issue.Location.Path = i.Pos.Filename - issue.Location.Lines.Begin = i.Pos.Line + codeClimateIssues := []CodeClimateIssue{} + for i := range issues { + issue := &issues[i] + codeClimateIssue := CodeClimateIssue{} + codeClimateIssue.Description = issue.Description() + codeClimateIssue.Location.Path = issue.Pos.Filename + codeClimateIssue.Location.Lines.Begin = issue.Pos.Line + codeClimateIssue.Fingerprint = issue.Fingerprint() - // 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 { - firstLine = i.SourceLines[0] + if issue.Severity != "" { + codeClimateIssue.Severity = issue.Severity } - hash := md5.New() //nolint:gosec - _, _ = hash.Write([]byte(i.Pos.Filename + i.Text + firstLine)) - issue.Fingerprint = fmt.Sprintf("%X", hash.Sum(nil)) - - allIssues = append(allIssues, issue) + codeClimateIssues = append(codeClimateIssues, codeClimateIssue) } - outputJSON, err := json.Marshal(allIssues) + outputJSON, err := json.Marshal(codeClimateIssues) if err != nil { return err } diff --git a/pkg/printers/github.go b/pkg/printers/github.go index fa11a283..b8d70140 100644 --- a/pkg/printers/github.go +++ b/pkg/printers/github.go @@ -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 { @@ -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) } diff --git a/pkg/result/issue.go b/pkg/result/issue.go index 16d9a8a8..eafdbc4a 100644 --- a/pkg/result/issue.go +++ b/pkg/result/issue.go @@ -1,6 +1,8 @@ package result import ( + "crypto/md5" //nolint:gosec + "fmt" "go/token" "golang.org/x/tools/go/packages" @@ -26,6 +28,8 @@ type Issue struct { FromLinter string Text string + Severity string + // Source lines of a code with the issue to show SourceLines []string @@ -76,3 +80,19 @@ func (i *Issue) GetLineRange() Range { return *i.LineRange } + +func (i *Issue) Description() string { + return fmt.Sprintf("%s: %s", i.FromLinter, i.Text) +} + +func (i *Issue) Fingerprint() string { + firstLine := "" + if len(i.SourceLines) > 0 { + firstLine = i.SourceLines[0] + } + + hash := md5.New() //nolint:gosec + _, _ = hash.Write([]byte(fmt.Sprintf("%s%s%s", i.Pos.Filename, i.Text, firstLine))) + + return fmt.Sprintf("%X", hash.Sum(nil)) +} diff --git a/pkg/result/processors/base_rule.go b/pkg/result/processors/base_rule.go new file mode 100644 index 00000000..b6ce4f21 --- /dev/null +++ b/pkg/result/processors/base_rule.go @@ -0,0 +1,69 @@ +package processors + +import ( + "regexp" + + "github.com/golangci/golangci-lint/pkg/fsutils" + "github.com/golangci/golangci-lint/pkg/logutils" + "github.com/golangci/golangci-lint/pkg/result" +) + +type BaseRule struct { + Text string + Source string + Path string + Linters []string +} + +type baseRule struct { + text *regexp.Regexp + source *regexp.Regexp + path *regexp.Regexp + linters []string +} + +func (r *baseRule) isEmpty() bool { + return r.text == nil && r.source == nil && r.path == nil && len(r.linters) == 0 +} + +func (r *baseRule) match(issue *result.Issue, lineCache *fsutils.LineCache, log logutils.Log) bool { + if r.isEmpty() { + return false + } + if r.text != nil && !r.text.MatchString(issue.Text) { + return false + } + if r.path != nil && !r.path.MatchString(issue.FilePath()) { + return false + } + if len(r.linters) != 0 && !r.matchLinter(issue) { + return false + } + + // the most heavyweight checking last + if r.source != nil && !r.matchSource(issue, lineCache, log) { + return false + } + + return true +} + +func (r *baseRule) matchLinter(issue *result.Issue) bool { + for _, linter := range r.linters { + if linter == issue.FromLinter { + return true + } + } + + return false +} + +func (r *baseRule) matchSource(issue *result.Issue, lineCache *fsutils.LineCache, log logutils.Log) bool { // nolint:interfacer + sourceLine, errSourceLine := lineCache.GetLine(issue.FilePath(), issue.Line()) + if errSourceLine != nil { + log.Warnf("Failed to get line %s:%d from line cache: %s", issue.FilePath(), issue.Line(), errSourceLine) + return false // can't properly match + } + + return r.source.MatchString(sourceLine) +} diff --git a/pkg/result/processors/exclude_rules.go b/pkg/result/processors/exclude_rules.go index b926af5b..d4d6569f 100644 --- a/pkg/result/processors/exclude_rules.go +++ b/pkg/result/processors/exclude_rules.go @@ -9,21 +9,11 @@ import ( ) type excludeRule struct { - text *regexp.Regexp - source *regexp.Regexp - path *regexp.Regexp - linters []string -} - -func (r *excludeRule) isEmpty() bool { - return r.text == nil && r.path == nil && len(r.linters) == 0 + baseRule } type ExcludeRule struct { - Text string - Source string - Path string - Linters []string + BaseRule } type ExcludeRules struct { @@ -45,9 +35,8 @@ func NewExcludeRules(rules []ExcludeRule, lineCache *fsutils.LineCache, log logu func createRules(rules []ExcludeRule, prefix string) []excludeRule { parsedRules := make([]excludeRule, 0, len(rules)) for _, rule := range rules { - parsedRule := excludeRule{ - linters: rule.Linters, - } + parsedRule := excludeRule{} + parsedRule.linters = rule.Linters if rule.Text != "" { parsedRule.text = regexp.MustCompile(prefix + rule.Text) } @@ -69,7 +58,7 @@ func (p ExcludeRules) Process(issues []result.Issue) ([]result.Issue, error) { return filterIssues(issues, func(i *result.Issue) bool { for _, rule := range p.rules { rule := rule - if p.match(i, &rule) { + if rule.match(i, p.lineCache, p.log) { return false } } @@ -77,48 +66,6 @@ func (p ExcludeRules) Process(issues []result.Issue) ([]result.Issue, error) { }), nil } -func (p ExcludeRules) matchLinter(i *result.Issue, r *excludeRule) bool { - for _, linter := range r.linters { - if linter == i.FromLinter { - return true - } - } - - return false -} - -func (p ExcludeRules) matchSource(i *result.Issue, r *excludeRule) bool { //nolint:interfacer - sourceLine, err := p.lineCache.GetLine(i.FilePath(), i.Line()) - if err != nil { - p.log.Warnf("Failed to get line %s:%d from line cache: %s", i.FilePath(), i.Line(), err) - return false // can't properly match - } - - return r.source.MatchString(sourceLine) -} - -func (p ExcludeRules) match(i *result.Issue, r *excludeRule) bool { - if r.isEmpty() { - return false - } - if r.text != nil && !r.text.MatchString(i.Text) { - return false - } - if r.path != nil && !r.path.MatchString(i.FilePath()) { - return false - } - if len(r.linters) != 0 && !p.matchLinter(i, r) { - return false - } - - // the most heavyweight checking last - if r.source != nil && !p.matchSource(i, r) { - return false - } - - return true -} - func (ExcludeRules) Name() string { return "exclude-rules" } func (ExcludeRules) Finish() {} diff --git a/pkg/result/processors/exclude_rules_test.go b/pkg/result/processors/exclude_rules_test.go index 80759d4f..c247e6ab 100644 --- a/pkg/result/processors/exclude_rules_test.go +++ b/pkg/result/processors/exclude_rules_test.go @@ -1,7 +1,6 @@ package processors import ( - "go/token" "path/filepath" "testing" @@ -15,24 +14,32 @@ func TestExcludeRulesMultiple(t *testing.T) { lineCache := fsutils.NewLineCache(fsutils.NewFileCache()) p := NewExcludeRules([]ExcludeRule{ { - Text: "^exclude$", - Linters: []string{"linter"}, + BaseRule: BaseRule{ + Text: "^exclude$", + Linters: []string{"linter"}, + }, }, { - Linters: []string{"testlinter"}, - Path: `_test\.go`, + BaseRule: BaseRule{ + Linters: []string{"testlinter"}, + Path: `_test\.go`, + }, }, { - Text: "^testonly$", - Path: `_test\.go`, + BaseRule: BaseRule{ + Text: "^testonly$", + Path: `_test\.go`, + }, }, { - Source: "^//go:generate ", - Linters: []string{"lll"}, + BaseRule: BaseRule{ + Source: "^//go:generate ", + Linters: []string{"lll"}, + }, }, }, 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"}, @@ -43,19 +50,19 @@ 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"}, @@ -63,30 +70,12 @@ func TestExcludeRulesMultiple(t *testing.T) { 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{ { - Text: "^exclude$", - Linters: []string{ - "linter", + BaseRule: BaseRule{ + Text: "^exclude$", + Linters: []string{"linter"}, }, }, }, nil, nil) @@ -110,31 +99,39 @@ 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) { lineCache := fsutils.NewLineCache(fsutils.NewFileCache()) p := NewExcludeRulesCaseSensitive([]ExcludeRule{ { - Text: "^exclude$", - Linters: []string{"linter"}, + BaseRule: BaseRule{ + Text: "^exclude$", + Linters: []string{"linter"}, + }, }, { - Linters: []string{"testlinter"}, - Path: `_test\.go`, + BaseRule: BaseRule{ + Linters: []string{"testlinter"}, + Path: `_test\.go`, + }, }, { - Text: "^testonly$", - Path: `_test\.go`, + BaseRule: BaseRule{ + Text: "^testonly$", + Path: `_test\.go`, + }, }, { - Source: "^//go:generate ", - Linters: []string{"lll"}, + BaseRule: BaseRule{ + Source: "^//go:generate ", + Linters: []string{"lll"}, + }, }, }, 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"}, @@ -147,19 +144,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"}, @@ -173,9 +170,9 @@ func TestExcludeRulesCaseSensitiveMultiple(t *testing.T) { func TestExcludeRulesCaseSensitiveText(t *testing.T) { p := NewExcludeRulesCaseSensitive([]ExcludeRule{ { - Text: "^exclude$", - Linters: []string{ - "linter", + BaseRule: BaseRule{ + Text: "^exclude$", + Linters: []string{"linter"}, }, }, }, nil, nil) @@ -199,5 +196,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")) } diff --git a/pkg/result/processors/exclude_test.go b/pkg/result/processors/exclude_test.go index 7965f2fd..3b92ccd7 100644 --- a/pkg/result/processors/exclude_test.go +++ b/pkg/result/processors/exclude_test.go @@ -8,34 +8,12 @@ import ( "github.com/golangci/golangci-lint/pkg/result" ) -func newTextIssue(text string) result.Issue { - return result.Issue{ - Text: text, - } -} - -func process(t *testing.T, p Processor, issues ...result.Issue) []result.Issue { - processedIssues, err := p.Process(issues) - assert.NoError(t, err) - return processedIssues -} - -func processAssertSame(t *testing.T, p Processor, issues ...result.Issue) { - processedIssues := process(t, p, issues...) - assert.Equal(t, issues, processedIssues) -} - -func processAssertEmpty(t *testing.T, p Processor, issues ...result.Issue) { - processedIssues := process(t, p, issues...) - assert.Empty(t, processedIssues) -} - func TestExclude(t *testing.T) { p := NewExclude("^exclude$") texts := []string{"excLude", "1", "", "exclud", "notexclude"} var issues []result.Issue for _, t := range texts { - issues = append(issues, newTextIssue(t)) + issues = append(issues, newIssueFromTextTestCase(t)) } processedIssues := process(t, p, issues...) @@ -49,7 +27,7 @@ func TestExclude(t *testing.T) { } func TestNoExclude(t *testing.T) { - processAssertSame(t, NewExclude(""), newTextIssue("test")) + processAssertSame(t, NewExclude(""), newIssueFromTextTestCase("test")) } func TestExcludeCaseSensitive(t *testing.T) { @@ -57,7 +35,7 @@ func TestExcludeCaseSensitive(t *testing.T) { texts := []string{"excLude", "1", "", "exclud", "exclude"} var issues []result.Issue for _, t := range texts { - issues = append(issues, newTextIssue(t)) + issues = append(issues, newIssueFromTextTestCase(t)) } processedIssues := process(t, p, issues...) diff --git a/pkg/result/processors/processor_test.go b/pkg/result/processors/processor_test.go new file mode 100644 index 00000000..02e7a666 --- /dev/null +++ b/pkg/result/processors/processor_test.go @@ -0,0 +1,51 @@ +package processors + +import ( + "go/token" + "testing" + + "github.com/golangci/golangci-lint/pkg/result" + + "github.com/stretchr/testify/assert" +) + +type issueTestCase struct { + Path string + Line int + Text string + Linter string + Severity string +} + +func newIssueFromIssueTestCase(c issueTestCase) result.Issue { + return result.Issue{ + Text: c.Text, + FromLinter: c.Linter, + Pos: token.Position{ + Filename: c.Path, + Line: c.Line, + }, + } +} + +func newIssueFromTextTestCase(text string) result.Issue { + return result.Issue{ + Text: text, + } +} + +func process(t *testing.T, p Processor, issues ...result.Issue) []result.Issue { + processedIssues, err := p.Process(issues) + assert.NoError(t, err) + return processedIssues +} + +func processAssertSame(t *testing.T, p Processor, issues ...result.Issue) { + processedIssues := process(t, p, issues...) + assert.Equal(t, issues, processedIssues) +} + +func processAssertEmpty(t *testing.T, p Processor, issues ...result.Issue) { + processedIssues := process(t, p, issues...) + assert.Empty(t, processedIssues) +} diff --git a/pkg/result/processors/severity_rules.go b/pkg/result/processors/severity_rules.go new file mode 100644 index 00000000..5f11b541 --- /dev/null +++ b/pkg/result/processors/severity_rules.go @@ -0,0 +1,103 @@ +package processors + +import ( + "regexp" + + "github.com/golangci/golangci-lint/pkg/fsutils" + "github.com/golangci/golangci-lint/pkg/logutils" + "github.com/golangci/golangci-lint/pkg/result" +) + +type severityRule struct { + baseRule + severity string +} + +type SeverityRule struct { + BaseRule + Severity string +} + +type SeverityRules struct { + defaultSeverity string + rules []severityRule + lineCache *fsutils.LineCache + log logutils.Log +} + +func NewSeverityRules(defaultSeverity string, rules []SeverityRule, lineCache *fsutils.LineCache, log logutils.Log) *SeverityRules { + r := &SeverityRules{ + lineCache: lineCache, + log: log, + defaultSeverity: defaultSeverity, + } + r.rules = createSeverityRules(rules, "(?i)") + + return r +} + +func createSeverityRules(rules []SeverityRule, prefix string) []severityRule { + parsedRules := make([]severityRule, 0, len(rules)) + for _, rule := range rules { + parsedRule := severityRule{} + parsedRule.linters = rule.Linters + parsedRule.severity = rule.Severity + if rule.Text != "" { + parsedRule.text = regexp.MustCompile(prefix + rule.Text) + } + if rule.Source != "" { + parsedRule.source = regexp.MustCompile(prefix + rule.Source) + } + if rule.Path != "" { + parsedRule.path = regexp.MustCompile(rule.Path) + } + parsedRules = append(parsedRules, parsedRule) + } + return parsedRules +} + +func (p SeverityRules) Process(issues []result.Issue) ([]result.Issue, error) { + if len(p.rules) == 0 { + return issues, nil + } + return transformIssues(issues, func(i *result.Issue) *result.Issue { + for _, rule := range p.rules { + rule := rule + + ruleSeverity := p.defaultSeverity + if rule.severity != "" { + ruleSeverity = rule.severity + } + + if rule.match(i, p.lineCache, p.log) { + i.Severity = ruleSeverity + return i + } + } + i.Severity = p.defaultSeverity + return i + }), nil +} + +func (SeverityRules) Name() string { return "severity-rules" } +func (SeverityRules) Finish() {} + +var _ Processor = SeverityRules{} + +type SeverityRulesCaseSensitive struct { + *SeverityRules +} + +func NewSeverityRulesCaseSensitive(defaultSeverity string, rules []SeverityRule, + lineCache *fsutils.LineCache, log logutils.Log) *SeverityRulesCaseSensitive { + r := &SeverityRules{ + lineCache: lineCache, + log: log, + defaultSeverity: defaultSeverity, + } + r.rules = createSeverityRules(rules, "") + + return &SeverityRulesCaseSensitive{r} +} + +func (SeverityRulesCaseSensitive) Name() string { return "severity-rules-case-sensitive" } diff --git a/pkg/result/processors/severity_rules_test.go b/pkg/result/processors/severity_rules_test.go new file mode 100644 index 00000000..d3ff6b2f --- /dev/null +++ b/pkg/result/processors/severity_rules_test.go @@ -0,0 +1,173 @@ +package processors + +import ( + "path/filepath" + "testing" + + "github.com/stretchr/testify/assert" + + "github.com/golangci/golangci-lint/pkg/fsutils" + "github.com/golangci/golangci-lint/pkg/logutils" + "github.com/golangci/golangci-lint/pkg/report" + "github.com/golangci/golangci-lint/pkg/result" +) + +func TestSeverityRulesMultiple(t *testing.T) { + lineCache := fsutils.NewLineCache(fsutils.NewFileCache()) + log := report.NewLogWrapper(logutils.NewStderrLog(""), &report.Data{}) + p := NewSeverityRules("error", []SeverityRule{ + { + Severity: "info", + BaseRule: BaseRule{ + Text: "^ssl$", + Linters: []string{"gosec"}, + }, + }, + { + Severity: "info", + BaseRule: BaseRule{ + Linters: []string{"linter"}, + Path: "e.go", + }, + }, + { + Severity: "info", + BaseRule: BaseRule{ + Text: "^testonly$", + Path: `_test\.go`, + }, + }, + { + BaseRule: BaseRule{ + Source: "^//go:generate ", + Linters: []string{"lll"}, + }, + }, + { + Severity: "info", + BaseRule: BaseRule{ + Source: "^//go:dosomething", + }, + }, + { + Severity: "info", + BaseRule: BaseRule{ + Linters: []string{"someotherlinter"}, + }, + }, + { + Severity: "info", + BaseRule: BaseRule{ + Linters: []string{"somelinter"}, + }, + }, + { + Severity: "info", + }, + }, lineCache, log) + + cases := []issueTestCase{ + {Path: "ssl.go", Text: "ssl", Linter: "gosec"}, + {Path: "e.go", Text: "some", Linter: "linter"}, + {Path: "e_test.go", Text: "testonly", Linter: "testlinter"}, + {Path: filepath.Join("testdata", "exclude_rules.go"), Line: 3, Linter: "lll"}, + {Path: filepath.Join("testdata", "severity_rules.go"), Line: 3, Linter: "invalidgo"}, + {Path: "someotherlinter.go", Text: "someotherlinter", Linter: "someotherlinter"}, + {Path: "somenotmatchlinter.go", Text: "somenotmatchlinter", Linter: "somenotmatchlinter"}, + {Path: "empty.go", Text: "empty", Linter: "empty"}, + } + var issues []result.Issue + for _, c := range cases { + issues = append(issues, newIssueFromIssueTestCase(c)) + } + processedIssues := process(t, p, issues...) + var resultingCases []issueTestCase + for _, i := range processedIssues { + resultingCases = append(resultingCases, issueTestCase{ + Path: i.FilePath(), + Linter: i.FromLinter, + Text: i.Text, + Line: i.Line(), + Severity: i.Severity, + }) + } + expectedCases := []issueTestCase{ + {Path: "ssl.go", Text: "ssl", Linter: "gosec", Severity: "info"}, + {Path: "e.go", Text: "some", Linter: "linter", Severity: "info"}, + {Path: "e_test.go", Text: "testonly", Linter: "testlinter", Severity: "info"}, + {Path: filepath.Join("testdata", "exclude_rules.go"), Line: 3, Linter: "lll", Severity: "error"}, + {Path: filepath.Join("testdata", "severity_rules.go"), Line: 3, Linter: "invalidgo", Severity: "info"}, + {Path: "someotherlinter.go", Text: "someotherlinter", Linter: "someotherlinter", Severity: "info"}, + {Path: "somenotmatchlinter.go", Text: "somenotmatchlinter", Linter: "somenotmatchlinter", Severity: "error"}, + {Path: "empty.go", Text: "empty", Linter: "empty", Severity: "error"}, + } + assert.Equal(t, expectedCases, resultingCases) +} + +func TestSeverityRulesText(t *testing.T) { + p := NewSeverityRules("", []SeverityRule{ + { + BaseRule: BaseRule{ + Text: "^severity$", + Linters: []string{"linter"}, + }, + }, + }, nil, nil) + texts := []string{"seveRity", "1", "", "serverit", "notseverity"} + var issues []result.Issue + for _, t := range texts { + issues = append(issues, result.Issue{ + Text: t, + FromLinter: "linter", + }) + } + + processedIssues := process(t, p, issues...) + assert.Len(t, processedIssues, len(issues)) + + var processedTexts []string + for _, i := range processedIssues { + processedTexts = append(processedTexts, i.Text) + } + assert.Equal(t, texts, processedTexts) +} + +func TestSeverityRulesEmpty(t *testing.T) { + processAssertSame(t, NewSeverityRules("", nil, nil, nil), newIssueFromTextTestCase("test")) +} + +func TestSeverityRulesCaseSensitive(t *testing.T) { + lineCache := fsutils.NewLineCache(fsutils.NewFileCache()) + p := NewSeverityRulesCaseSensitive("error", []SeverityRule{ + { + Severity: "info", + BaseRule: BaseRule{ + Text: "^ssl$", + Linters: []string{"gosec", "someotherlinter"}, + }, + }, + }, lineCache, nil) + + cases := []issueTestCase{ + {Path: "e.go", Text: "ssL", Linter: "gosec"}, + } + var issues []result.Issue + for _, c := range cases { + issues = append(issues, newIssueFromIssueTestCase(c)) + } + processedIssues := process(t, p, issues...) + var resultingCases []issueTestCase + for _, i := range processedIssues { + resultingCases = append(resultingCases, issueTestCase{ + Path: i.FilePath(), + Linter: i.FromLinter, + Text: i.Text, + Line: i.Line(), + Severity: i.Severity, + }) + } + expectedCases := []issueTestCase{ + {Path: "e.go", Text: "ssL", Linter: "gosec", Severity: "error"}, + } + assert.Equal(t, expectedCases, resultingCases) +} diff --git a/pkg/result/processors/testdata/severity_rules.go b/pkg/result/processors/testdata/severity_rules.go new file mode 100644 index 00000000..026fb2aa --- /dev/null +++ b/pkg/result/processors/testdata/severity_rules.go @@ -0,0 +1,3 @@ +package testdata + +//go:dosomething