Skip to content

Re-enable default excludes by ID #1045

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 3 commits into from
Apr 25, 2020
Merged

Re-enable default excludes by ID #1045

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
c55e761
Re-enable default excludes by ID
JohnStarich Apr 25, 2020
15c7569
Re-generated README
JohnStarich Apr 25, 2020
36d8d88
Preserve API backward compatibility
JohnStarich Apr 25, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
20 changes: 10 additions & 10 deletions README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
2 changes: 1 addition & 1 deletion pkg/commands/run.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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)),
"",
)
Expand Down
33 changes: 28 additions & 5 deletions pkg/config/config.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,70 +30,92 @@ var OutFormats = []string{
}

type ExcludePattern struct {
ID string
Pattern string
Linter string
Why string
}

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 {
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
for _, p := range DefaultExcludePatterns {
ret = append(ret, p.Pattern)
if !includeMap[p.ID] {
ret = append(ret, p.Pattern)
}
}

return ret
Expand Down Expand Up @@ -411,10 +433,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"`
Expand Down
2 changes: 1 addition & 1 deletion pkg/lint/runner.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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.GetExcludePatternsStrings(icfg.IncludeDefaultExcludes)...)
}

var excludeTotalPattern string
Expand Down