Merge pull request #1045 from JohnStarich/feature/include-excludes

Re-enable default excludes by ID
This commit is contained in:
Aleksandr Razumov 2020-04-26 00:31:01 +03:00 committed by GitHub
commit 4a3e9fd2bb
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
4 changed files with 40 additions and 17 deletions

View File

@ -546,34 +546,34 @@ Flags:
--fast Run only fast linters from enabled linters set (first run won't be fast) --fast Run only fast linters from enabled linters set (first run won't be fast)
-e, --exclude strings Exclude issue by regexp -e, --exclude strings Exclude issue by regexp
--exclude-use-default Use or not use default excludes: --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 - 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) - (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 - 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) - (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 - 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 - 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) - Subprocess launch(ed with variable|ing should be audited)
# gosec: Duplicated errcheck checks # EXC0008 gosec: Duplicated errcheck checks
- G104 - 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) - (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 - Potential file inclusion via variable
(default true) (default true)
--exclude-case-sensitive If set to true exclude and exclude rules regular expressions are case sensitive --exclude-case-sensitive If set to true exclude and exclude rules regular expressions are case sensitive

View File

@ -30,7 +30,7 @@ func getDefaultIssueExcludeHelp() string {
parts := []string{"Use or not use default excludes:"} parts := []string{"Use or not use default excludes:"}
for _, ep := range config.DefaultExcludePatterns { for _, ep := range config.DefaultExcludePatterns {
parts = append(parts, 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)), fmt.Sprintf(" - %s", color.YellowString(ep.Pattern)),
"", "",
) )

View File

@ -30,6 +30,7 @@ var OutFormats = []string{
} }
type ExcludePattern struct { type ExcludePattern struct {
ID string
Pattern string Pattern string
Linter string Linter string
Why string Why string
@ -37,53 +38,63 @@ type ExcludePattern struct {
var DefaultExcludePatterns = []ExcludePattern{ var DefaultExcludePatterns = []ExcludePattern{
{ {
ID: "EXC0001",
Pattern: "Error return value of .((os\\.)?std(out|err)\\..*|.*Close" + Pattern: "Error return value of .((os\\.)?std(out|err)\\..*|.*Close" +
"|.*Flush|os\\.Remove(All)?|.*printf?|os\\.(Un)?Setenv). is not checked", "|.*Flush|os\\.Remove(All)?|.*printf?|os\\.(Un)?Setenv). is not checked",
Linter: "errcheck", Linter: "errcheck",
Why: "Almost all programs ignore errors on these functions and in most cases it's ok", 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)|" + Pattern: "(comment on exported (method|function|type|const)|" +
"should have( a package)? comment|comment should be of the form)", "should have( a package)? comment|comment should be of the form)",
Linter: "golint", Linter: "golint",
Why: "Annoying issue about not having a comment. The rare codebase has such comments", 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", Pattern: "func name will be used as test\\.Test.* by other packages, and that stutters; consider calling this",
Linter: "golint", Linter: "golint",
Why: "False positive when tests are defined in package 'test'", Why: "False positive when tests are defined in package 'test'",
}, },
{ {
ID: "EXC0004",
Pattern: "(possible misuse of unsafe.Pointer|should have signature)", Pattern: "(possible misuse of unsafe.Pointer|should have signature)",
Linter: "govet", Linter: "govet",
Why: "Common false positives", Why: "Common false positives",
}, },
{ {
ID: "EXC0005",
Pattern: "ineffective break statement. Did you mean to break out of the outer loop", Pattern: "ineffective break statement. Did you mean to break out of the outer loop",
Linter: "staticcheck", Linter: "staticcheck",
Why: "Developers tend to write in C-style with an explicit 'break' in a 'switch', so it's ok to ignore", 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", Pattern: "Use of unsafe calls should be audited",
Linter: "gosec", Linter: "gosec",
Why: "Too many false-positives on 'unsafe' usage", Why: "Too many false-positives on 'unsafe' usage",
}, },
{ {
ID: "EXC0007",
Pattern: "Subprocess launch(ed with variable|ing should be audited)", Pattern: "Subprocess launch(ed with variable|ing should be audited)",
Linter: "gosec", Linter: "gosec",
Why: "Too many false-positives for parametrized shell calls", Why: "Too many false-positives for parametrized shell calls",
}, },
{ {
ID: "EXC0008",
Pattern: "G104", Pattern: "G104",
Linter: "gosec", Linter: "gosec",
Why: "Duplicated errcheck checks", Why: "Duplicated errcheck checks",
}, },
{ {
ID: "EXC0009",
Pattern: "(Expect directory permissions to be 0750 or less|Expect file permissions to be 0600 or less)", Pattern: "(Expect directory permissions to be 0750 or less|Expect file permissions to be 0600 or less)",
Linter: "gosec", Linter: "gosec",
Why: "Too many issues in popular repos", Why: "Too many issues in popular repos",
}, },
{ {
ID: "EXC0010",
Pattern: "Potential file inclusion via variable", Pattern: "Potential file inclusion via variable",
Linter: "gosec", Linter: "gosec",
Why: "False positive is triggered by 'src, err := ioutil.ReadFile(filename)'", Why: "False positive is triggered by 'src, err := ioutil.ReadFile(filename)'",
@ -91,10 +102,21 @@ var DefaultExcludePatterns = []ExcludePattern{
} }
func GetDefaultExcludePatternsStrings() []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
}
var ret []string var ret []string
for _, p := range DefaultExcludePatterns { for _, p := range DefaultExcludePatterns {
if !includeMap[p.ID] {
ret = append(ret, p.Pattern) ret = append(ret, p.Pattern)
} }
}
return ret return ret
} }
@ -411,6 +433,7 @@ func (e ExcludeRule) Validate() error {
} }
type Issues struct { type Issues struct {
IncludeDefaultExcludes []string `mapstructure:"include"`
ExcludeCaseSensitive bool `mapstructure:"exclude-case-sensitive"` ExcludeCaseSensitive bool `mapstructure:"exclude-case-sensitive"`
ExcludePatterns []string `mapstructure:"exclude"` ExcludePatterns []string `mapstructure:"exclude"`
ExcludeRules []ExcludeRule `mapstructure:"exclude-rules"` ExcludeRules []ExcludeRule `mapstructure:"exclude-rules"`

View File

@ -32,7 +32,7 @@ func NewRunner(cfg *config.Config, log logutils.Log, goenv *goutil.Env,
icfg := cfg.Issues icfg := cfg.Issues
excludePatterns := icfg.ExcludePatterns excludePatterns := icfg.ExcludePatterns
if icfg.UseDefaultExcludes { if icfg.UseDefaultExcludes {
excludePatterns = append(excludePatterns, config.GetDefaultExcludePatternsStrings()...) excludePatterns = append(excludePatterns, config.GetExcludePatternsStrings(icfg.IncludeDefaultExcludes)...)
} }
var excludeTotalPattern string var excludeTotalPattern string