From 9948153575c7fb3bcebc08228c95f327cfa2454c Mon Sep 17 00:00:00 2001 From: ZhangYunHao Date: Thu, 12 Nov 2020 22:21:19 +0800 Subject: [PATCH] DefaultExcludePatterns should only be used for specified linter (#1494) Co-authored-by: zhangyunhao --- pkg/config/config.go | 19 +++++++++--- pkg/config/config_test.go | 36 +++++++++++++++++++++++ pkg/golinters/goanalysis/runner.go | 2 +- pkg/lint/runner.go | 21 ++++++++----- pkg/printers/github.go | 2 +- test/testdata/configs/default_exclude.yml | 3 ++ test/testdata/default_exclude.go | 20 +++++++++++++ 7 files changed, 90 insertions(+), 13 deletions(-) create mode 100644 pkg/config/config_test.go create mode 100644 test/testdata/configs/default_exclude.yml create mode 100644 test/testdata/default_exclude.go diff --git a/pkg/config/config.go b/pkg/config/config.go index d17ec3589ea5..cf3a3d59bba2 100644 --- a/pkg/config/config.go +++ b/pkg/config/config.go @@ -99,22 +99,33 @@ var DefaultExcludePatterns = []ExcludePattern{ Linter: "gosec", Why: "False positive is triggered by 'src, err := ioutil.ReadFile(filename)'", }, + { + ID: "EXC0011", + Pattern: "(comment on exported (method|function|type|const)|" + + "should have( a package)? comment|comment should be of the form)", + Linter: "stylecheck", + Why: "Annoying issue about not having a comment. The rare codebase has such comments", + }, } func GetDefaultExcludePatternsStrings() []string { - return GetExcludePatternsStrings(nil) + ret := make([]string, len(DefaultExcludePatterns)) + for i, p := range DefaultExcludePatterns { + ret[i] = p.Pattern + } + return ret } -func GetExcludePatternsStrings(include []string) []string { +func GetExcludePatterns(include []string) []ExcludePattern { includeMap := make(map[string]bool, len(include)) for _, inc := range include { includeMap[inc] = true } - var ret []string + var ret []ExcludePattern for _, p := range DefaultExcludePatterns { if !includeMap[p.ID] { - ret = append(ret, p.Pattern) + ret = append(ret, p) } } diff --git a/pkg/config/config_test.go b/pkg/config/config_test.go new file mode 100644 index 000000000000..208188bc810a --- /dev/null +++ b/pkg/config/config_test.go @@ -0,0 +1,36 @@ +package config + +import ( + "fmt" + "testing" + + "github.com/stretchr/testify/assert" +) + +func TestGetExcludePatterns(t *testing.T) { + assert.Equal(t, GetExcludePatterns(nil), DefaultExcludePatterns) + + include := make([]string, 2) + include[0], include[1] = DefaultExcludePatterns[0].ID, DefaultExcludePatterns[1].ID + + exclude := GetExcludePatterns(include) + assert.Equal(t, len(exclude), len(DefaultExcludePatterns)-len(include)) + + for _, p := range exclude { + // Not in include. + for _, i := range include { + if i == p.ID { + t.Fatalf("%s can't appear inside include.", p.ID) + } + } + // Must in DefaultExcludePatterns. + var inDefaultExc bool + for _, i := range DefaultExcludePatterns { + if i == p { + inDefaultExc = true + break + } + } + assert.True(t, inDefaultExc, fmt.Sprintf("%s must appear inside DefaultExcludePatterns.", p.ID)) + } +} diff --git a/pkg/golinters/goanalysis/runner.go b/pkg/golinters/goanalysis/runner.go index db193f37bb20..81225277eb77 100644 --- a/pkg/golinters/goanalysis/runner.go +++ b/pkg/golinters/goanalysis/runner.go @@ -3,7 +3,7 @@ // Use of this source code is governed by a BSD-style // license that can be found in the LICENSE file. -// Package checker defines the implementation of the checker commands. +// Package goanalysis defines the implementation of the checker commands. // The same code drives the multi-analysis driver, the single-analysis // driver that is conventionally provided for convenience along with // each analysis package, and the test driver. diff --git a/pkg/lint/runner.go b/pkg/lint/runner.go index 084912226b63..5278f3ac2263 100644 --- a/pkg/lint/runner.go +++ b/pkg/lint/runner.go @@ -225,14 +225,10 @@ func (r *Runner) processIssues(issues []result.Issue, sw *timeutils.Stopwatch, s } 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, "|")) + excludeGlobalPatterns := cfg.ExcludePatterns + if len(excludeGlobalPatterns) != 0 { + excludeTotalPattern = fmt.Sprintf("(%s)", strings.Join(excludeGlobalPatterns, "|")) } var excludeProcessor processors.Processor @@ -258,6 +254,17 @@ func getExcludeRulesProcessor(cfg *config.Issues, log logutils.Log, lineCache *f }) } + if cfg.UseDefaultExcludes { + for _, r := range config.GetExcludePatterns(cfg.IncludeDefaultExcludes) { + excludeRules = append(excludeRules, processors.ExcludeRule{ + BaseRule: processors.BaseRule{ + Text: r.Pattern, + Linters: []string{r.Linter}, + }, + }) + } + } + var excludeRulesProcessor processors.Processor if cfg.ExcludeCaseSensitive { excludeRulesProcessor = processors.NewExcludeRulesCaseSensitive( diff --git a/pkg/printers/github.go b/pkg/printers/github.go index b8d70140a80c..4ebc26685784 100644 --- a/pkg/printers/github.go +++ b/pkg/printers/github.go @@ -13,7 +13,7 @@ type github struct { const defaultGithubSeverity = "error" -// Github output format outputs issues according to Github actions format: +// NewGithub 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 { return &github{} diff --git a/test/testdata/configs/default_exclude.yml b/test/testdata/configs/default_exclude.yml new file mode 100644 index 000000000000..8c8699ee8e4c --- /dev/null +++ b/test/testdata/configs/default_exclude.yml @@ -0,0 +1,3 @@ +issues: + include: + - EXC0011 # include issues about comments from `stylecheck` \ No newline at end of file diff --git a/test/testdata/default_exclude.go b/test/testdata/default_exclude.go new file mode 100644 index 000000000000..dad1feca6959 --- /dev/null +++ b/test/testdata/default_exclude.go @@ -0,0 +1,20 @@ +//args: -Estylecheck,golint +//config_path: testdata/configs/default_exclude.yml + +/*Package testdata ...*/ +package testdata + +// InvalidFuncComment, both golint and stylecheck will complain about this, // ERROR `ST1020: comment on exported function ExportedFunc1 should be of the form "ExportedFunc1 ..."` +// if include EXC0011, only the warning from golint will be ignored. +// And only the warning from stylecheck will start with "ST1020". +func ExportedFunc1() { +} + +// InvalidFuncComment // ERROR `ST1020: comment on exported function ExportedFunc2 should be of the form "ExportedFunc2 ..."` +// nolint:golint +func ExportedFunc2() { +} + +// nolint:stylecheck +func IgnoreAll() { +}