Skip to content
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.

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

DefaultExcludePatterns should only be used for specified linter #1494

Merged
merged 1 commit into from Nov 12, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
19 changes: 15 additions & 4 deletions pkg/config/config.go
Expand Up @@ -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)
}
}

Expand Down
36 changes: 36 additions & 0 deletions 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))
}
}
2 changes: 1 addition & 1 deletion pkg/golinters/goanalysis/runner.go
Expand Up @@ -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.
Expand Down
21 changes: 14 additions & 7 deletions pkg/lint/runner.go
Expand Up @@ -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
Expand All @@ -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(
Expand Down
2 changes: 1 addition & 1 deletion pkg/printers/github.go
Expand Up @@ -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{}
Expand Down
3 changes: 3 additions & 0 deletions test/testdata/configs/default_exclude.yml
@@ -0,0 +1,3 @@
issues:
include:
- EXC0011 # include issues about comments from `stylecheck`
20 changes: 20 additions & 0 deletions 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() {
}