From c55e7614d4357121d80fe3e72789e35b347ad63a Mon Sep 17 00:00:00 2001 From: John Starich Date: Sat, 25 Apr 2020 13:49:11 -0500 Subject: [PATCH 1/3] Re-enable default excludes by ID --- pkg/commands/run.go | 2 +- pkg/config/config.go | 31 +++++++++++++++++++++++++------ pkg/lint/runner.go | 2 +- test/bench/bench_test.go | 2 +- 4 files changed, 28 insertions(+), 9 deletions(-) diff --git a/pkg/commands/run.go b/pkg/commands/run.go index 17f0f3fb..434b85d5 100644 --- a/pkg/commands/run.go +++ b/pkg/commands/run.go @@ -30,7 +30,7 @@ func getDefaultIssueExcludeHelp() string { parts := []string{"Use or not use default excludes:"} for _, ep := range config.DefaultExcludePatterns { parts = append(parts, - fmt.Sprintf(" # %s: %s", ep.Linter, ep.Why), + fmt.Sprintf(" # %s %s: %s", ep.ID, ep.Linter, ep.Why), fmt.Sprintf(" - %s", color.YellowString(ep.Pattern)), "", ) diff --git a/pkg/config/config.go b/pkg/config/config.go index 2b3048cb..f5cf3b4c 100644 --- a/pkg/config/config.go +++ b/pkg/config/config.go @@ -30,6 +30,7 @@ var OutFormats = []string{ } type ExcludePattern struct { + ID string Pattern string Linter string Why string @@ -37,63 +38,80 @@ type ExcludePattern struct { var DefaultExcludePatterns = []ExcludePattern{ { + ID: "EXC0001", Pattern: "Error return value of .((os\\.)?std(out|err)\\..*|.*Close" + "|.*Flush|os\\.Remove(All)?|.*printf?|os\\.(Un)?Setenv). is not checked", Linter: "errcheck", Why: "Almost all programs ignore errors on these functions and in most cases it's ok", }, { + ID: "EXC0002", Pattern: "(comment on exported (method|function|type|const)|" + "should have( a package)? comment|comment should be of the form)", Linter: "golint", Why: "Annoying issue about not having a comment. The rare codebase has such comments", }, { + ID: "EXC0003", Pattern: "func name will be used as test\\.Test.* by other packages, and that stutters; consider calling this", Linter: "golint", Why: "False positive when tests are defined in package 'test'", }, { + ID: "EXC0004", Pattern: "(possible misuse of unsafe.Pointer|should have signature)", Linter: "govet", Why: "Common false positives", }, { + ID: "EXC0005", Pattern: "ineffective break statement. Did you mean to break out of the outer loop", Linter: "staticcheck", Why: "Developers tend to write in C-style with an explicit 'break' in a 'switch', so it's ok to ignore", }, { + ID: "EXC0006", Pattern: "Use of unsafe calls should be audited", Linter: "gosec", Why: "Too many false-positives on 'unsafe' usage", }, { + ID: "EXC0007", Pattern: "Subprocess launch(ed with variable|ing should be audited)", Linter: "gosec", Why: "Too many false-positives for parametrized shell calls", }, { + ID: "EXC0008", Pattern: "G104", Linter: "gosec", Why: "Duplicated errcheck checks", }, { + ID: "EXC0009", Pattern: "(Expect directory permissions to be 0750 or less|Expect file permissions to be 0600 or less)", Linter: "gosec", Why: "Too many issues in popular repos", }, { + ID: "EXC0010", Pattern: "Potential file inclusion via variable", Linter: "gosec", Why: "False positive is triggered by 'src, err := ioutil.ReadFile(filename)'", }, } -func GetDefaultExcludePatternsStrings() []string { +func GetDefaultExcludePatternsStrings(include []string) []string { + includeMap := make(map[string]bool, len(include)) + for _, inc := range include { + includeMap[inc] = true + } + var ret []string for _, p := range DefaultExcludePatterns { - ret = append(ret, p.Pattern) + if !includeMap[p.ID] { + ret = append(ret, p.Pattern) + } } return ret @@ -411,10 +429,11 @@ func (e ExcludeRule) Validate() error { } type Issues struct { - ExcludeCaseSensitive bool `mapstructure:"exclude-case-sensitive"` - ExcludePatterns []string `mapstructure:"exclude"` - ExcludeRules []ExcludeRule `mapstructure:"exclude-rules"` - UseDefaultExcludes bool `mapstructure:"exclude-use-default"` + IncludeDefaultExcludes []string `mapstructure:"include"` + ExcludeCaseSensitive bool `mapstructure:"exclude-case-sensitive"` + ExcludePatterns []string `mapstructure:"exclude"` + ExcludeRules []ExcludeRule `mapstructure:"exclude-rules"` + UseDefaultExcludes bool `mapstructure:"exclude-use-default"` MaxIssuesPerLinter int `mapstructure:"max-issues-per-linter"` MaxSameIssues int `mapstructure:"max-same-issues"` diff --git a/pkg/lint/runner.go b/pkg/lint/runner.go index dd9e6946..3be2700a 100644 --- a/pkg/lint/runner.go +++ b/pkg/lint/runner.go @@ -32,7 +32,7 @@ func NewRunner(cfg *config.Config, log logutils.Log, goenv *goutil.Env, icfg := cfg.Issues excludePatterns := icfg.ExcludePatterns if icfg.UseDefaultExcludes { - excludePatterns = append(excludePatterns, config.GetDefaultExcludePatternsStrings()...) + excludePatterns = append(excludePatterns, config.GetDefaultExcludePatternsStrings(icfg.IncludeDefaultExcludes)...) } var excludeTotalPattern string diff --git a/test/bench/bench_test.go b/test/bench/bench_test.go index 42caffd7..5084ee96 100644 --- a/test/bench/bench_test.go +++ b/test/bench/bench_test.go @@ -77,7 +77,7 @@ func getGometalinterCommonArgs() []string { "--vendor", "--cyclo-over=30", "--dupl-threshold=150", - "--exclude", fmt.Sprintf("(%s)", strings.Join(config.GetDefaultExcludePatternsStrings(), "|")), + "--exclude", fmt.Sprintf("(%s)", strings.Join(config.GetDefaultExcludePatternsStrings(nil), "|")), "--disable-all", "--enable=vet", "--enable=vetshadow", From 15c75690fcef82b4685b1c6518822140e50b7ac4 Mon Sep 17 00:00:00 2001 From: John Starich Date: Sat, 25 Apr 2020 14:08:00 -0500 Subject: [PATCH 2/3] Re-generated README --- README.md | 20 ++++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/README.md b/README.md index 29b1efbf..c2e445cf 100644 --- a/README.md +++ b/README.md @@ -546,34 +546,34 @@ Flags: --fast Run only fast linters from enabled linters set (first run won't be fast) -e, --exclude strings Exclude issue by regexp --exclude-use-default Use or not use default excludes: - # errcheck: Almost all programs ignore errors on these functions and in most cases it's ok + # EXC0001 errcheck: Almost all programs ignore errors on these functions and in most cases it's ok - Error return value of .((os\.)?std(out|err)\..*|.*Close|.*Flush|os\.Remove(All)?|.*printf?|os\.(Un)?Setenv). is not checked - # golint: Annoying issue about not having a comment. The rare codebase has such comments + # EXC0002 golint: Annoying issue about not having a comment. The rare codebase has such comments - (comment on exported (method|function|type|const)|should have( a package)? comment|comment should be of the form) - # golint: False positive when tests are defined in package 'test' + # EXC0003 golint: False positive when tests are defined in package 'test' - func name will be used as test\.Test.* by other packages, and that stutters; consider calling this - # govet: Common false positives + # EXC0004 govet: Common false positives - (possible misuse of unsafe.Pointer|should have signature) - # staticcheck: Developers tend to write in C-style with an explicit 'break' in a 'switch', so it's ok to ignore + # EXC0005 staticcheck: Developers tend to write in C-style with an explicit 'break' in a 'switch', so it's ok to ignore - ineffective break statement. Did you mean to break out of the outer loop - # gosec: Too many false-positives on 'unsafe' usage + # EXC0006 gosec: Too many false-positives on 'unsafe' usage - Use of unsafe calls should be audited - # gosec: Too many false-positives for parametrized shell calls + # EXC0007 gosec: Too many false-positives for parametrized shell calls - Subprocess launch(ed with variable|ing should be audited) - # gosec: Duplicated errcheck checks + # EXC0008 gosec: Duplicated errcheck checks - G104 - # gosec: Too many issues in popular repos + # EXC0009 gosec: Too many issues in popular repos - (Expect directory permissions to be 0750 or less|Expect file permissions to be 0600 or less) - # gosec: False positive is triggered by 'src, err := ioutil.ReadFile(filename)' + # EXC0010 gosec: False positive is triggered by 'src, err := ioutil.ReadFile(filename)' - Potential file inclusion via variable (default true) --exclude-case-sensitive If set to true exclude and exclude rules regular expressions are case sensitive From 36d8d881f928cfbd107d8a6cfba991b447755181 Mon Sep 17 00:00:00 2001 From: John Starich Date: Sat, 25 Apr 2020 14:14:42 -0500 Subject: [PATCH 3/3] Preserve API backward compatibility --- pkg/config/config.go | 6 +++++- pkg/lint/runner.go | 2 +- test/bench/bench_test.go | 2 +- 3 files changed, 7 insertions(+), 3 deletions(-) diff --git a/pkg/config/config.go b/pkg/config/config.go index f5cf3b4c..d3401839 100644 --- a/pkg/config/config.go +++ b/pkg/config/config.go @@ -101,7 +101,11 @@ var DefaultExcludePatterns = []ExcludePattern{ }, } -func GetDefaultExcludePatternsStrings(include []string) []string { +func GetDefaultExcludePatternsStrings() []string { + return GetExcludePatternsStrings(nil) +} + +func GetExcludePatternsStrings(include []string) []string { includeMap := make(map[string]bool, len(include)) for _, inc := range include { includeMap[inc] = true diff --git a/pkg/lint/runner.go b/pkg/lint/runner.go index 3be2700a..22859afc 100644 --- a/pkg/lint/runner.go +++ b/pkg/lint/runner.go @@ -32,7 +32,7 @@ func NewRunner(cfg *config.Config, log logutils.Log, goenv *goutil.Env, icfg := cfg.Issues excludePatterns := icfg.ExcludePatterns if icfg.UseDefaultExcludes { - excludePatterns = append(excludePatterns, config.GetDefaultExcludePatternsStrings(icfg.IncludeDefaultExcludes)...) + excludePatterns = append(excludePatterns, config.GetExcludePatternsStrings(icfg.IncludeDefaultExcludes)...) } var excludeTotalPattern string diff --git a/test/bench/bench_test.go b/test/bench/bench_test.go index 5084ee96..42caffd7 100644 --- a/test/bench/bench_test.go +++ b/test/bench/bench_test.go @@ -77,7 +77,7 @@ func getGometalinterCommonArgs() []string { "--vendor", "--cyclo-over=30", "--dupl-threshold=150", - "--exclude", fmt.Sprintf("(%s)", strings.Join(config.GetDefaultExcludePatternsStrings(nil), "|")), + "--exclude", fmt.Sprintf("(%s)", strings.Join(config.GetDefaultExcludePatternsStrings(), "|")), "--disable-all", "--enable=vet", "--enable=vetshadow",