DefaultExcludePatterns should only be used for specified linter (#1494)

Co-authored-by: zhangyunhao <[email protected]>

Author: zhangyunhao116

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)
 		}
 	}
 

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))
+	}
+}

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.

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(

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{}

test/testdata/configs/default_exclude.yml

@@ -0,0 +1,3 @@
+issues:
+  include:
+    - EXC0011 # include issues about comments from `stylecheck`

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() {
+}