From 00cc2336c7733b24036e18f292860bd929d0a819 Mon Sep 17 00:00:00 2001 From: kaka_ace Date: Mon, 20 Apr 2020 21:06:12 +0800 Subject: [PATCH] feat: [gocritic] support disabled-tags --- .golangci.example.yml | 2 + README.md | 2 + pkg/config/config_gocritic.go | 74 ++++++++++++++++++++++++++++-- pkg/config/config_gocritic_test.go | 54 ++++++++++++++++++++++ 4 files changed, 127 insertions(+), 5 deletions(-) create mode 100644 pkg/config/config_gocritic_test.go diff --git a/.golangci.example.yml b/.golangci.example.yml index a6be95a0..3da655a9 100644 --- a/.golangci.example.yml +++ b/.golangci.example.yml @@ -117,6 +117,8 @@ linters-settings: # Empty list by default. See https://github.com/go-critic/go-critic#usage -> section "Tags". enabled-tags: - performance + disabled-tags: + - experimental settings: # settings passed to gocritic captLocal: # must be valid enabled check name diff --git a/README.md b/README.md index 9a86b13f..8413f023 100644 --- a/README.md +++ b/README.md @@ -727,6 +727,8 @@ linters-settings: # Empty list by default. See https://github.com/go-critic/go-critic#usage -> section "Tags". enabled-tags: - performance + disabled-tags: + - experimental settings: # settings passed to gocritic captLocal: # must be valid enabled check name diff --git a/pkg/config/config_gocritic.go b/pkg/config/config_gocritic.go index b8397262..bbfd7bb1 100644 --- a/pkg/config/config_gocritic.go +++ b/pkg/config/config_gocritic.go @@ -15,10 +15,18 @@ import ( const gocriticDebugKey = "gocritic" -var gocriticDebugf = logutils.Debug(gocriticDebugKey) -var isGocriticDebug = logutils.HaveDebugTag(gocriticDebugKey) - -var allGocriticCheckers = lintpack.GetCheckersInfo() +var ( + gocriticDebugf = logutils.Debug(gocriticDebugKey) + isGocriticDebug = logutils.HaveDebugTag(gocriticDebugKey) + allGocriticCheckers = lintpack.GetCheckersInfo() + allGocriticCheckerMap = func() map[string]*lintpack.CheckerInfo { + checkInfoMap := make(map[string]*lintpack.CheckerInfo) + for _, checkInfo := range allGocriticCheckers { + checkInfoMap[checkInfo.Name] = checkInfo + } + return checkInfoMap + }() +) type GocriticCheckSettings map[string]interface{} @@ -26,6 +34,7 @@ type GocriticSettings struct { EnabledChecks []string `mapstructure:"enabled-checks"` DisabledChecks []string `mapstructure:"disabled-checks"` EnabledTags []string `mapstructure:"enabled-tags"` + DisabledTags []string `mapstructure:"disabled-tags"` SettingsPerCheck map[string]GocriticCheckSettings `mapstructure:"settings"` inferredEnabledChecks map[string]bool @@ -107,6 +116,8 @@ func (s *GocriticSettings) InferEnabledChecks(log logutils.Log) { debugChecksListf(disabledByDefaultChecks, "Disabled by default") var enabledChecks []string + + // EnabledTags if len(s.EnabledTags) != 0 { tagToCheckers := buildGocriticTagToCheckersMap() for _, tag := range s.EnabledTags { @@ -120,6 +131,12 @@ func (s *GocriticSettings) InferEnabledChecks(log logutils.Log) { enabledChecks = append(enabledChecks, enabledByDefaultChecks...) } + // DisabledTags + if len(s.DisabledTags) != 0 { + enabledChecks = filterByDisableTags(enabledChecks, s.DisabledTags, log) + } + + // EnabledChecks if len(s.EnabledChecks) != 0 { debugChecksListf(s.EnabledChecks, "Enabled by config") @@ -133,6 +150,7 @@ func (s *GocriticSettings) InferEnabledChecks(log logutils.Log) { } } + // DisabledChecks if len(s.DisabledChecks) != 0 { debugChecksListf(s.DisabledChecks, "Disabled by config") @@ -174,6 +192,22 @@ func validateStringsUniq(ss []string) error { return nil } +func intersectStringSlice(s1, s2 []string) []string { + s1Map := make(map[string]struct{}) + for _, s := range s1 { + s1Map[s] = struct{}{} + } + + result := make([]string, 0) + for _, s := range s2 { + if _, exists := s1Map[s]; exists { + result = append(result, s) + } + } + + return result +} + func (s *GocriticSettings) Validate(log logutils.Log) error { if len(s.EnabledTags) == 0 { if len(s.EnabledChecks) != 0 && len(s.DisabledChecks) != 0 { @@ -187,7 +221,16 @@ func (s *GocriticSettings) Validate(log logutils.Log) error { tagToCheckers := buildGocriticTagToCheckersMap() for _, tag := range s.EnabledTags { if _, ok := tagToCheckers[tag]; !ok { - return fmt.Errorf("gocritic tag %q doesn't exist", tag) + return fmt.Errorf("gocritic [enabled]tag %q doesn't exist", tag) + } + } + } + + if len(s.DisabledTags) > 0 { + tagToCheckers := buildGocriticTagToCheckersMap() + for _, tag := range s.EnabledTags { + if _, ok := tagToCheckers[tag]; !ok { + return fmt.Errorf("gocritic [disabled]tag %q doesn't exist", tag) } } } @@ -301,3 +344,24 @@ func (s *GocriticSettings) GetLowercasedParams() map[string]GocriticCheckSetting } return ret } + +func filterByDisableTags(enabledChecks, disableTags []string, log logutils.Log) []string { + enabledChecksSet := stringsSliceToSet(enabledChecks) + for _, enabledCheck := range enabledChecks { + checkInfo, checkInfoExists := allGocriticCheckerMap[enabledCheck] + if !checkInfoExists { + log.Warnf("Gocritic check %q was not exists via filtering disabled tags", enabledCheck) + continue + } + hitTags := intersectStringSlice(checkInfo.Tags, disableTags) + if len(hitTags) != 0 { + delete(enabledChecksSet, enabledCheck) + } + debugChecksListf(enabledChecks, "Disabled by config tags %s", sprintStrings(disableTags)) + } + enabledChecks = nil + for enabledCheck := range enabledChecksSet { + enabledChecks = append(enabledChecks, enabledCheck) + } + return enabledChecks +} diff --git a/pkg/config/config_gocritic_test.go b/pkg/config/config_gocritic_test.go new file mode 100644 index 00000000..173c61e0 --- /dev/null +++ b/pkg/config/config_gocritic_test.go @@ -0,0 +1,54 @@ +package config + +import ( + "fmt" + "sort" + "testing" + + "github.com/golangci/golangci-lint/pkg/logutils" + + "github.com/stretchr/testify/assert" +) + +func TestUtils(t *testing.T) { + s1 := []string{"diagnostic", "experimental", "opinionated"} + s2 := []string{"opinionated", "experimental"} + s3 := intersectStringSlice(s1, s2) + sort.Strings(s3) + assert.Equal(t, s3, []string{"experimental", "opinionated"}) +} + +type tLog struct{} + +func (l *tLog) Fatalf(format string, args ...interface{}) { + fmt.Printf(fmt.Sprintf(format, args...) + "\n") +} + +func (l *tLog) Panicf(format string, args ...interface{}) { + fmt.Printf(fmt.Sprintf(format, args...) + "\n") +} + +func (l *tLog) Errorf(format string, args ...interface{}) { + fmt.Printf(fmt.Sprintf(format, args...) + "\n") +} + +func (l *tLog) Warnf(format string, args ...interface{}) { + fmt.Printf(fmt.Sprintf(format, args...) + "\n") +} + +func (l *tLog) Infof(format string, args ...interface{}) { + fmt.Printf(fmt.Sprintf(format, args...) + "\n") +} + +func (l *tLog) Child(name string) logutils.Log { return nil } + +func (l *tLog) SetLevel(level logutils.LogLevel) {} + +func TestFilterByDisableTags(t *testing.T) { + testLog := &tLog{} + disabledTags := []string{"experimental", "opinionated"} + enabledChecks := []string{"appendAssign", "argOrder", "caseOrder", "codegenComment"} + filterEnabledChecks := filterByDisableTags(enabledChecks, disabledTags, testLog) + sort.Strings(filterEnabledChecks) + assert.Equal(t, []string{"appendAssign", "caseOrder"}, filterEnabledChecks) +}