diff --git a/.gitignore b/.gitignore index 896f0c3e5e9a..8e5b12b54241 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 4f9cdb5cb88f..7d9317194eb4 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 9c485d2175ea..5ec160cbb3eb 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 c9175fc63897..b29e8f607d73 100644 --- a/pkg/config/config.go +++ b/pkg/config/config.go @@ -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"` @@ -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 1e355e72213a..e6b18a7a690c 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 05dc51ba9d74..f77778b286f9 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 f36bc108adc1..c5b948a98d29 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 5d45c4eb3019..35a22ce99a72 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 - - // 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] + 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() + + 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 fa11a2839a33..b8d70140a80c 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 16d9a8a8c19d..eafdbc4a958a 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 000000000000..b6ce4f2159e9 --- /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 b926af5b1d47..d4d6569f4ce3 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 80759d4f286f..c247e6ab0f84 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 7965f2fd26bc..3b92ccd75f83 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 000000000000..02e7a666aac4 --- /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 000000000000..5f11b54101c9 --- /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 000000000000..d3ff6b2f2c8f --- /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 000000000000..026fb2aa13b5 --- /dev/null +++ b/pkg/result/processors/testdata/severity_rules.go @@ -0,0 +1,3 @@ +package testdata + +//go:dosomething