gocritic: support of enable-all and disable-all options (#4335)

Co-authored-by: Ludovic Fernandez <ldez@users.noreply.github.com>

.golangci.reference.yml

@@ -529,22 +529,32 @@ linters-settings:
     ignore-strings: 'foo.+'
 
   gocritic:
-    # Which checks should be enabled; can't be combined with 'disabled-checks'.
+    # Disable all checks.
-    # See https://go-critic.github.io/overview#checks-overview.
+    # Default: false
-    # To check which checks are enabled run `GL_DEBUG=gocritic golangci-lint run`.
+    disable-all: true
-    # By default, list of stable checks is used.
+    # Which checks should be enabled in addition to default checks; can't be combined with 'disabled-checks'.
+    # By default, list of stable checks is used (https://go-critic.github.io/overview#checks-overview):
+    #   appendAssign, argOrder, assignOp, badCall, badCond, captLocal, caseOrder, codegenComment, commentFormatting,
+    #   defaultCaseOrder, deprecatedComment, dupArg, dupBranchBody, dupCase, dupSubExpr, elseif, exitAfterDefer,
+    #   flagDeref, flagName, ifElseChain, mapKey, newDeref, offBy1, regexpMust, singleCaseSwitch, sloppyLen,
+    #   sloppyTypeAssert, switchTrue, typeSwitchVar, underef, unlambda, unslice, valSwap, wrapperFunc
+    # To see which checks are enabled run `GL_DEBUG=gocritic golangci-lint run --enable=gocritic`.
     enabled-checks:
       - nestingReduce
       - unnamedResult
       - ruleguard
       - truncateCmp
 
+    # Enable all checks.
+    # Default: false
+    enable-all: true
     # Which checks should be disabled; can't be combined with 'enabled-checks'.
     # Default: []
     disabled-checks:
       - regexpMust
 
-    # Enable multiple checks by tags, run `GL_DEBUG=gocritic golangci-lint run` to see all tags and checks.
+    # Enable multiple checks by tags in addition to default checks.
+    # Run `GL_DEBUG=gocritic golangci-lint run --enable=gocritic` to see all tags and checks.
     # See https://github.com/go-critic/go-critic#usage -> section "Tags".
     # Default: []
     enabled-tags:

pkg/config/linters_settings.go

@@ -460,7 +460,9 @@ type GoConstSettings struct {
 
 type GoCriticSettings struct {
 	Go               string                           `mapstructure:"-"`
+	DisableAll       bool                             `mapstructure:"disable-all"`
 	EnabledChecks    []string                         `mapstructure:"enabled-checks"`
+	EnableAll        bool                             `mapstructure:"enable-all"`
 	DisabledChecks   []string                         `mapstructure:"disabled-checks"`
 	EnabledTags      []string                         `mapstructure:"enabled-tags"`
 	DisabledTags     []string                         `mapstructure:"disabled-tags"`

pkg/golinters/gocritic.go

@@ -15,6 +15,7 @@ import (
 	"github.com/go-critic/go-critic/checkers"
 	gocriticlinter "github.com/go-critic/go-critic/linter"
 	"golang.org/x/exp/maps"
+	"golang.org/x/exp/slices"
 	"golang.org/x/tools/go/analysis"
 
 	"github.com/golangci/golangci-lint/pkg/config"
@@ -36,8 +37,8 @@ func NewGoCritic(settings *config.GoCriticSettings, cfg *config.Config) *goanaly
 	var resIssues []goanalysis.Issue
 
 	wrapper := &goCriticWrapper{
-		cfg:   cfg,
+		getConfigDir: cfg.GetConfigDir, // Config directory is filled after calling this constructor.
-		sizes: types.SizesFor("gc", runtime.GOARCH),
+		sizes:        types.SizesFor("gc", runtime.GOARCH),
 	}
 
 	analyzer := &analysis.Analyzer{
@@ -74,12 +75,13 @@ Dynamic rules are written declaratively with AST patterns, filters, report messa
 		}).
 		WithIssuesReporter(func(*linter.Context) []goanalysis.Issue {
 			return resIssues
-		}).WithLoadMode(goanalysis.LoadModeTypesInfo)
+		}).
+		WithLoadMode(goanalysis.LoadModeTypesInfo)
 }
 
 type goCriticWrapper struct {
 	settingsWrapper *goCriticSettingsWrapper
-	cfg             *config.Config
+	getConfigDir    func() string
 	sizes           types.Sizes
 	once            sync.Once
 }
@@ -92,15 +94,15 @@ func (w *goCriticWrapper) init(settings *config.GoCriticSettings, logger logutil
 	w.once.Do(func() {
 		err := checkers.InitEmbeddedRules()
 		if err != nil {
-			logger.Fatalf("%s: %v: setting an explicit GOROOT can fix this problem.", goCriticName, err)
+			logger.Fatalf("%s: %v: setting an explicit GOROOT can fix this problem", goCriticName, err)
 		}
 	})
 
 	settingsWrapper := newGoCriticSettingsWrapper(settings, logger)
-
+	settingsWrapper.InferEnabledChecks()
-	settingsWrapper.inferEnabledChecks()
+	// Validate must be after InferEnabledChecks, not before.
-
+	// Because it uses gathered information about tags set and finally enabled checks.
-	if err := settingsWrapper.validate(); err != nil {
+	if err := settingsWrapper.Validate(); err != nil {
 		logger.Fatalf("%s: invalid settings: %s", goCriticName, err)
 	}
 
@@ -109,7 +111,7 @@ func (w *goCriticWrapper) init(settings *config.GoCriticSettings, logger logutil
 
 func (w *goCriticWrapper) run(pass *analysis.Pass) ([]goanalysis.Issue, error) {
 	if w.settingsWrapper == nil {
-		return nil, fmt.Errorf("the settings wrapper is nil")
+		return nil, errors.New("the settings wrapper is nil")
 	}
 
 	linterCtx := gocriticlinter.NewContext(pass.Fset, w.sizes)
@@ -123,7 +125,7 @@ func (w *goCriticWrapper) run(pass *analysis.Pass) ([]goanalysis.Issue, error) {
 
 	linterCtx.SetPackageInfo(pass.TypesInfo, pass.Pkg)
 
-	pkgIssues := runGocriticOnPackage(linterCtx, enabledCheckers, pass.Files)
+	pkgIssues := runGoCriticOnPackage(linterCtx, enabledCheckers, pass.Files)
 
 	issues := make([]goanalysis.Issue, 0, len(pkgIssues))
 	for i := range pkgIssues {
@@ -134,15 +136,15 @@ func (w *goCriticWrapper) run(pass *analysis.Pass) ([]goanalysis.Issue, error) {
 }
 
 func (w *goCriticWrapper) buildEnabledCheckers(linterCtx *gocriticlinter.Context) ([]*gocriticlinter.Checker, error) {
-	allParams := w.settingsWrapper.getLowerCasedParams()
+	allLowerCasedParams := w.settingsWrapper.GetLowerCasedParams()
 
 	var enabledCheckers []*gocriticlinter.Checker
 	for _, info := range gocriticlinter.GetCheckersInfo() {
-		if !w.settingsWrapper.isCheckEnabled(info.Name) {
+		if !w.settingsWrapper.IsCheckEnabled(info.Name) {
 			continue
 		}
 
-		if err := w.configureCheckerInfo(info, allParams); err != nil {
+		if err := w.configureCheckerInfo(info, allLowerCasedParams); err != nil {
 			return nil, err
 		}
 
@@ -156,20 +158,73 @@ func (w *goCriticWrapper) buildEnabledCheckers(linterCtx *gocriticlinter.Context
 	return enabledCheckers, nil
 }
 
-func runGocriticOnPackage(linterCtx *gocriticlinter.Context, checks []*gocriticlinter.Checker,
+func (w *goCriticWrapper) configureCheckerInfo(
-	files []*ast.File) []result.Issue {
+	info *gocriticlinter.CheckerInfo,
+	allLowerCasedParams map[string]config.GoCriticCheckSettings,
+) error {
+	params := allLowerCasedParams[strings.ToLower(info.Name)]
+	if params == nil { // no config for this checker
+		return nil
+	}
+
+	// To lowercase info param keys here because golangci-lint's config parser lowercases all strings.
+	infoParams := normalizeMap(info.Params)
+	for k, p := range params {
+		v, ok := infoParams[k]
+		if ok {
+			v.Value = w.normalizeCheckerParamsValue(p)
+			continue
+		}
+
+		// param `k` isn't supported
+		if len(info.Params) == 0 {
+			return fmt.Errorf("checker %s config param %s doesn't exist: checker doesn't have params",
+				info.Name, k)
+		}
+
+		supportedKeys := maps.Keys(info.Params)
+		sort.Strings(supportedKeys)
+
+		return fmt.Errorf("checker %s config param %s doesn't exist, all existing: %s",
+			info.Name, k, supportedKeys)
+	}
+
+	return nil
+}
+
+// normalizeCheckerParamsValue normalizes value types.
+// go-critic asserts that CheckerParam.Value has some specific types,
+// but the file parsers (TOML, YAML, JSON) don't create the same representation for raw type.
+// then we have to convert value types into the expected value types.
+// Maybe in the future, this kind of conversion will be done in go-critic itself.
+func (w *goCriticWrapper) normalizeCheckerParamsValue(p any) any {
+	rv := reflect.ValueOf(p)
+	switch rv.Type().Kind() {
+	case reflect.Int64, reflect.Int32, reflect.Int16, reflect.Int8, reflect.Int:
+		return int(rv.Int())
+	case reflect.Bool:
+		return rv.Bool()
+	case reflect.String:
+		// Perform variable substitution.
+		return strings.ReplaceAll(rv.String(), "${configDir}", w.getConfigDir())
+	default:
+		return p
+	}
+}
+
+func runGoCriticOnPackage(linterCtx *gocriticlinter.Context, checks []*gocriticlinter.Checker, files []*ast.File) []result.Issue {
 	var res []result.Issue
 	for _, f := range files {
 		filename := filepath.Base(linterCtx.FileSet.Position(f.Pos()).Filename)
 		linterCtx.SetFileInfo(filename, f)
 
-		issues := runGocriticOnFile(linterCtx, f, checks)
+		issues := runGoCriticOnFile(linterCtx, f, checks)
 		res = append(res, issues...)
 	}
 	return res
 }
 
-func runGocriticOnFile(linterCtx *gocriticlinter.Context, f *ast.File, checks []*gocriticlinter.Checker) []result.Issue {
+func runGoCriticOnFile(linterCtx *gocriticlinter.Context, f *ast.File, checks []*gocriticlinter.Checker) []result.Issue {
 	var res []result.Issue
 
 	for _, c := range checks {
@@ -200,416 +255,323 @@ func runGocriticOnFile(linterCtx *gocriticlinter.Context, f *ast.File, checks []
 	return res
 }
 
-func (w *goCriticWrapper) configureCheckerInfo(info *gocriticlinter.CheckerInfo, allParams map[string]config.GoCriticCheckSettings) error {
+type goCriticChecks[T any] map[string]T
-	params := allParams[strings.ToLower(info.Name)]
-	if params == nil { // no config for this checker
-		return nil
-	}
 
-	infoParams := normalizeCheckerInfoParams(info)
+func (m goCriticChecks[T]) has(name string) bool {
-	for k, p := range params {
+	_, ok := m[name]
-		v, ok := infoParams[k]
+	return ok
-		if ok {
-			v.Value = w.normalizeCheckerParamsValue(p)
-			continue
-		}
-
-		// param `k` isn't supported
-		if len(info.Params) == 0 {
-			return fmt.Errorf("checker %s config param %s doesn't exist: checker doesn't have params",
-				info.Name, k)
-		}
-
-		supportedKeys := maps.Keys(info.Params)
-		sort.Strings(supportedKeys)
-
-		return fmt.Errorf("checker %s config param %s doesn't exist, all existing: %s",
-			info.Name, k, supportedKeys)
-	}
-
-	return nil
 }
 
-func normalizeCheckerInfoParams(info *gocriticlinter.CheckerInfo) gocriticlinter.CheckerParams {
-	// lowercase info param keys here because golangci-lint's config parser lowercases all strings
-	ret := gocriticlinter.CheckerParams{}
-	for k, v := range info.Params {
-		ret[strings.ToLower(k)] = v
-	}
-
-	return ret
-}
-
-// normalizeCheckerParamsValue normalizes value types.
-// go-critic asserts that CheckerParam.Value has some specific types,
-// but the file parsers (TOML, YAML, JSON) don't create the same representation for raw type.
-// then we have to convert value types into the expected value types.
-// Maybe in the future, this kind of conversion will be done in go-critic itself.
-func (w *goCriticWrapper) normalizeCheckerParamsValue(p any) any {
-	rv := reflect.ValueOf(p)
-	switch rv.Type().Kind() {
-	case reflect.Int64, reflect.Int32, reflect.Int16, reflect.Int8, reflect.Int:
-		return int(rv.Int())
-	case reflect.Bool:
-		return rv.Bool()
-	case reflect.String:
-		// Perform variable substitution.
-		return strings.ReplaceAll(rv.String(), "${configDir}", w.cfg.GetConfigDir())
-	default:
-		return p
-	}
-}
-
-// TODO(ldez): rewrite and simplify goCriticSettingsWrapper.
-
 type goCriticSettingsWrapper struct {
 	*config.GoCriticSettings
 
 	logger logutils.Log
 
-	allCheckers   []*gocriticlinter.CheckerInfo
+	allCheckers []*gocriticlinter.CheckerInfo
-	allCheckerMap map[string]*gocriticlinter.CheckerInfo
 
-	inferredEnabledChecks map[string]bool
+	allChecks             goCriticChecks[struct{}]
+	allChecksByTag        goCriticChecks[[]string]
+	allTagsSorted         []string
+	inferredEnabledChecks goCriticChecks[struct{}]
+
+	// *LowerCased fields are used for GoCriticSettings.SettingsPerCheck validation only.
+
+	allChecksLowerCased             goCriticChecks[struct{}]
+	inferredEnabledChecksLowerCased goCriticChecks[struct{}]
 }
 
 func newGoCriticSettingsWrapper(settings *config.GoCriticSettings, logger logutils.Log) *goCriticSettingsWrapper {
 	allCheckers := gocriticlinter.GetCheckersInfo()
 
-	allCheckerMap := make(map[string]*gocriticlinter.CheckerInfo)
+	allChecks := make(goCriticChecks[struct{}], len(allCheckers))
-	for _, checkInfo := range allCheckers {
+	allChecksLowerCased := make(goCriticChecks[struct{}], len(allCheckers))
-		allCheckerMap[checkInfo.Name] = checkInfo
+	allChecksByTag := make(goCriticChecks[[]string])
+	for _, checker := range allCheckers {
+		allChecks[checker.Name] = struct{}{}
+		allChecksLowerCased[strings.ToLower(checker.Name)] = struct{}{}
+
+		for _, tag := range checker.Tags {
+			allChecksByTag[tag] = append(allChecksByTag[tag], checker.Name)
+		}
 	}
 
+	allTagsSorted := maps.Keys(allChecksByTag)
+	sort.Strings(allTagsSorted)
+
 	return &goCriticSettingsWrapper{
-		GoCriticSettings:      settings,
+		GoCriticSettings:                settings,
-		logger:                logger,
+		logger:                          logger,
-		allCheckers:           allCheckers,
+		allCheckers:                     allCheckers,
-		allCheckerMap:         allCheckerMap,
+		allChecks:                       allChecks,
-		inferredEnabledChecks: map[string]bool{},
+		allChecksLowerCased:             allChecksLowerCased,
+		allChecksByTag:                  allChecksByTag,
+		allTagsSorted:                   allTagsSorted,
+		inferredEnabledChecks:           make(goCriticChecks[struct{}]),
+		inferredEnabledChecksLowerCased: make(goCriticChecks[struct{}]),
 	}
 }
 
-func (s *goCriticSettingsWrapper) buildTagToCheckersMap() map[string][]string {
+func (s *goCriticSettingsWrapper) IsCheckEnabled(name string) bool {
-	tagToCheckers := map[string][]string{}
+	return s.inferredEnabledChecks.has(name)
-
-	for _, checker := range s.allCheckers {
-		for _, tag := range checker.Tags {
-			tagToCheckers[tag] = append(tagToCheckers[tag], checker.Name)
-		}
-	}
-
-	return tagToCheckers
 }
 
-func (s *goCriticSettingsWrapper) checkerTagsDebugf() {
+func (s *goCriticSettingsWrapper) GetLowerCasedParams() map[string]config.GoCriticCheckSettings {
-	if !isGoCriticDebug {
+	return normalizeMap(s.SettingsPerCheck)
-		return
-	}
-
-	tagToCheckers := s.buildTagToCheckersMap()
-
-	allTags := maps.Keys(tagToCheckers)
-	sort.Strings(allTags)
-
-	goCriticDebugf("All gocritic existing tags and checks:")
-	for _, tag := range allTags {
-		debugChecksListf(tagToCheckers[tag], "  tag %q", tag)
-	}
 }
 
-func (s *goCriticSettingsWrapper) disabledCheckersDebugf() {
+// InferEnabledChecks tries to be consistent with (lintersdb.EnabledSet).build.
-	if !isGoCriticDebug {
+func (s *goCriticSettingsWrapper) InferEnabledChecks() {
-		return
+	s.debugChecksInitialState()
-	}
 
-	var disabledCheckers []string
+	enabledByDefaultChecks, disabledByDefaultChecks := s.buildEnabledAndDisabledByDefaultChecks()
-	for _, checker := range s.allCheckers {
-		if s.inferredEnabledChecks[strings.ToLower(checker.Name)] {
-			continue
-		}
-
-		disabledCheckers = append(disabledCheckers, checker.Name)
-	}
-
-	if len(disabledCheckers) == 0 {
-		goCriticDebugf("All checks are enabled")
-	} else {
-		debugChecksListf(disabledCheckers, "Final not used")
-	}
-}
-
-func (s *goCriticSettingsWrapper) inferEnabledChecks() {
-	s.checkerTagsDebugf()
-
-	enabledByDefaultChecks := s.getDefaultEnabledCheckersNames()
 	debugChecksListf(enabledByDefaultChecks, "Enabled by default")
-
-	disabledByDefaultChecks := s.getDefaultDisabledCheckersNames()
 	debugChecksListf(disabledByDefaultChecks, "Disabled by default")
 
-	enabledChecks := make([]string, 0, len(s.EnabledTags)+len(enabledByDefaultChecks))
+	enabledChecks := make(goCriticChecks[struct{}])
 
-	// EnabledTags
+	if s.EnableAll {
-	if len(s.EnabledTags) != 0 {
+		enabledChecks = make(goCriticChecks[struct{}], len(s.allCheckers))
-		tagToCheckers := s.buildTagToCheckersMap()
+		for _, info := range s.allCheckers {
-		for _, tag := range s.EnabledTags {
+			enabledChecks[info.Name] = struct{}{}
-			enabledChecks = append(enabledChecks, tagToCheckers[tag]...)
+		}
+	} else if !s.DisableAll {
+		// enable-all/disable-all revokes the default settings.
+		enabledChecks = make(goCriticChecks[struct{}], len(enabledByDefaultChecks))
+		for _, check := range enabledByDefaultChecks {
+			enabledChecks[check] = struct{}{}
 		}
-
-		debugChecksListf(enabledChecks, "Enabled by config tags %s", sprintStrings(s.EnabledTags))
 	}
 
-	if !(len(s.EnabledTags) == 0 && len(s.EnabledChecks) != 0) {
+	if len(s.EnabledTags) != 0 {
-		// don't use default checks only if we have no enabled tags and enable some checks manually
+		enabledFromTags := s.expandTagsToChecks(s.EnabledTags)
-		enabledChecks = append(enabledChecks, enabledByDefaultChecks...)
+		debugChecksListf(enabledFromTags, "Enabled by config tags %s", sprintSortedStrings(s.EnabledTags))
+
+		for _, check := range enabledFromTags {
+			enabledChecks[check] = struct{}{}
+		}
 	}
 
-	// DisabledTags
-	if len(s.DisabledTags) != 0 {
-		enabledChecks = s.filterByDisableTags(enabledChecks, s.DisabledTags)
-	}
-
-	// EnabledChecks
 	if len(s.EnabledChecks) != 0 {
 		debugChecksListf(s.EnabledChecks, "Enabled by config")
 
-		alreadyEnabledChecksSet := stringsSliceToSet(enabledChecks)
+		for _, check := range s.EnabledChecks {
-		for _, enabledCheck := range s.EnabledChecks {
+			if enabledChecks.has(check) {
-			if alreadyEnabledChecksSet[enabledCheck] {
+				s.logger.Warnf("%s: no need to enable check %q: it's already enabled", goCriticName, check)
-				s.logger.Warnf("%s: no need to enable check %q: it's already enabled", goCriticName, enabledCheck)
 				continue
 			}
-			enabledChecks = append(enabledChecks, enabledCheck)
+			enabledChecks[check] = struct{}{}
+		}
+	}
+
+	if len(s.DisabledTags) != 0 {
+		disabledFromTags := s.expandTagsToChecks(s.DisabledTags)
+		debugChecksListf(disabledFromTags, "Disabled by config tags %s", sprintSortedStrings(s.DisabledTags))
+
+		for _, check := range disabledFromTags {
+			delete(enabledChecks, check)
 		}
 	}
 
-	// DisabledChecks
 	if len(s.DisabledChecks) != 0 {
 		debugChecksListf(s.DisabledChecks, "Disabled by config")
 
-		enabledChecksSet := stringsSliceToSet(enabledChecks)
+		for _, check := range s.DisabledChecks {
-		for _, disabledCheck := range s.DisabledChecks {
+			if !enabledChecks.has(check) {
-			if !enabledChecksSet[disabledCheck] {
+				s.logger.Warnf("%s: no need to disable check %q: it's already disabled", goCriticName, check)
-				s.logger.Warnf("%s: check %q was explicitly disabled via config. However, as this check "+
-					"is disabled by default, there is no need to explicitly disable it via config.", goCriticName, disabledCheck)
 				continue
 			}
-			delete(enabledChecksSet, disabledCheck)
+			delete(enabledChecks, check)
-		}
-
-		enabledChecks = nil
-		for enabledCheck := range enabledChecksSet {
-			enabledChecks = append(enabledChecks, enabledCheck)
 		}
 	}
 
-	s.inferredEnabledChecks = map[string]bool{}
+	s.inferredEnabledChecks = enabledChecks
-	for _, check := range enabledChecks {
+	s.inferredEnabledChecksLowerCased = normalizeMap(s.inferredEnabledChecks)
-		s.inferredEnabledChecks[strings.ToLower(check)] = true
+	s.debugChecksFinalState()
+}
+
+func (s *goCriticSettingsWrapper) buildEnabledAndDisabledByDefaultChecks() (enabled, disabled []string) {
+	for _, info := range s.allCheckers {
+		if enabledByDef := isEnabledByDefaultGoCriticChecker(info); enabledByDef {
+			enabled = append(enabled, info.Name)
+		} else {
+			disabled = append(disabled, info.Name)
+		}
+	}
+	return enabled, disabled
+}
+
+func (s *goCriticSettingsWrapper) expandTagsToChecks(tags []string) []string {
+	var checks []string
+	for _, tag := range tags {
+		checks = append(checks, s.allChecksByTag[tag]...)
+	}
+	return checks
+}
+
+func (s *goCriticSettingsWrapper) debugChecksInitialState() {
+	if !isGoCriticDebug {
+		return
+	}
+
+	goCriticDebugf("All gocritic existing tags and checks:")
+	for _, tag := range s.allTagsSorted {
+		debugChecksListf(s.allChecksByTag[tag], "  tag %q", tag)
+	}
+}
+
+func (s *goCriticSettingsWrapper) debugChecksFinalState() {
+	if !isGoCriticDebug {
+		return
+	}
+
+	var enabledChecks []string
+	var disabledChecks []string
+
+	for _, checker := range s.allCheckers {
+		name := checker.Name
+		if s.inferredEnabledChecks.has(name) {
+			enabledChecks = append(enabledChecks, name)
+		} else {
+			disabledChecks = append(disabledChecks, name)
+		}
 	}
 
 	debugChecksListf(enabledChecks, "Final used")
 
-	s.disabledCheckersDebugf()
+	if len(disabledChecks) == 0 {
+		goCriticDebugf("All checks are enabled")
+	} else {
+		debugChecksListf(disabledChecks, "Final not used")
+	}
 }
 
-func (s *goCriticSettingsWrapper) validate() error {
+// Validate tries to be consistent with (lintersdb.Validator).validateEnabledDisabledLintersConfig.
-	if len(s.EnabledTags) == 0 {
+func (s *goCriticSettingsWrapper) Validate() error {
-		if len(s.EnabledChecks) != 0 && len(s.DisabledChecks) != 0 {
+	for _, v := range []func() error{
-			return errors.New("both enabled and disabled check aren't allowed for gocritic")
+		s.validateOptionsCombinations,
+		s.validateCheckerTags,
+		s.validateCheckerNames,
+		s.validateDisabledAndEnabledAtOneMoment,
+		s.validateAtLeastOneCheckerEnabled,
+	} {
+		if err := v(); err != nil {
+			return err
 		}
-	} else {
+	}
-		if err := validateStringsUniq(s.EnabledTags); err != nil {
+	return nil
-			return fmt.Errorf("validate enabled tags: %w", err)
+}
+
+func (s *goCriticSettingsWrapper) validateOptionsCombinations() error {
+	if s.EnableAll {
+		if s.DisableAll {
+			return errors.New("enable-all and disable-all options must not be combined")
 		}
 
-		tagToCheckers := s.buildTagToCheckersMap()
+		if len(s.EnabledTags) != 0 {
+			return errors.New("enable-all and enabled-tags options must not be combined")
+		}
 
-		for _, tag := range s.EnabledTags {
+		if len(s.EnabledChecks) != 0 {
-			if _, ok := tagToCheckers[tag]; !ok {
+			return errors.New("enable-all and enabled-checks options must not be combined")
-				return fmt.Errorf("gocritic [enabled]tag %q doesn't exist", tag)
-			}
 		}
 	}
 
-	if len(s.DisabledTags) > 0 {
+	if s.DisableAll {
-		tagToCheckers := s.buildTagToCheckersMap()
+		if len(s.DisabledTags) != 0 {
-		for _, tag := range s.EnabledTags {
+			return errors.New("disable-all and disabled-tags options must not be combined")
-			if _, ok := tagToCheckers[tag]; !ok {
-				return fmt.Errorf("gocritic [disabled]tag %q doesn't exist", tag)
-			}
 		}
-	}
 
-	if err := validateStringsUniq(s.EnabledChecks); err != nil {
+		if len(s.DisabledChecks) != 0 {
-		return fmt.Errorf("validate enabled checks: %w", err)
+			return errors.New("disable-all and disabled-checks options must not be combined")
-	}
+		}
 
-	if err := validateStringsUniq(s.DisabledChecks); err != nil {
+		if len(s.EnabledTags) == 0 && len(s.EnabledChecks) == 0 {
-		return fmt.Errorf("validate disabled checks: %w", err)
+			return errors.New("all checks were disabled, but no one check was enabled: at least one must be enabled")
-	}
+		}
-
-	if err := s.validateCheckerNames(); err != nil {
-		return fmt.Errorf("validation failed: %w", err)
 	}
 
 	return nil
 }
 
-func (s *goCriticSettingsWrapper) isCheckEnabled(name string) bool {
+func (s *goCriticSettingsWrapper) validateCheckerTags() error {
-	return s.inferredEnabledChecks[strings.ToLower(name)]
+	for _, tag := range s.EnabledTags {
-}
+		if !s.allChecksByTag.has(tag) {
-
+			return fmt.Errorf("enabled tag %q doesn't exist, see %s's documentation", tag, goCriticName)
-// getAllCheckerNames returns a map containing all checker names supported by gocritic.
-func (s *goCriticSettingsWrapper) getAllCheckerNames() map[string]bool {
-	allCheckerNames := make(map[string]bool, len(s.allCheckers))
-
-	for _, checker := range s.allCheckers {
-		allCheckerNames[strings.ToLower(checker.Name)] = true
-	}
-
-	return allCheckerNames
-}
-
-func (s *goCriticSettingsWrapper) getDefaultEnabledCheckersNames() []string {
-	var enabled []string
-
-	for _, info := range s.allCheckers {
-		enable := s.isEnabledByDefaultCheck(info)
-		if enable {
-			enabled = append(enabled, info.Name)
 		}
 	}
 
-	return enabled
+	for _, tag := range s.DisabledTags {
-}
+		if !s.allChecksByTag.has(tag) {
-
+			return fmt.Errorf("disabled tag %q doesn't exist, see %s's documentation", tag, goCriticName)
-func (s *goCriticSettingsWrapper) getDefaultDisabledCheckersNames() []string {
-	var disabled []string
-
-	for _, info := range s.allCheckers {
-		enable := s.isEnabledByDefaultCheck(info)
-		if !enable {
-			disabled = append(disabled, info.Name)
 		}
 	}
 
-	return disabled
+	return nil
 }
 
 func (s *goCriticSettingsWrapper) validateCheckerNames() error {
-	allowedNames := s.getAllCheckerNames()
-
 	for _, name := range s.EnabledChecks {
-		if !allowedNames[strings.ToLower(name)] {
+		if !s.allChecks.has(name) {
-			return fmt.Errorf("enabled checker %s doesn't exist, all existing checkers: %s",
+			return fmt.Errorf("enabled check %q doesn't exist, see %s's documentation", name, goCriticName)
-				name, sprintAllowedCheckerNames(allowedNames))
 		}
 	}
 
 	for _, name := range s.DisabledChecks {
-		if !allowedNames[strings.ToLower(name)] {
+		if !s.allChecks.has(name) {
-			return fmt.Errorf("disabled checker %s doesn't exist, all existing checkers: %s",
+			return fmt.Errorf("disabled check %q doesn't exist, see %s documentation", name, goCriticName)
-				name, sprintAllowedCheckerNames(allowedNames))
 		}
 	}
 
-	for checkName := range s.SettingsPerCheck {
+	for name := range s.SettingsPerCheck {
-		if _, ok := allowedNames[checkName]; !ok {
+		lcName := strings.ToLower(name)
-			return fmt.Errorf("invalid setting, checker %s doesn't exist, all existing checkers: %s",
+		if !s.allChecksLowerCased.has(lcName) {
-				checkName, sprintAllowedCheckerNames(allowedNames))
+			return fmt.Errorf("invalid check settings: check %q doesn't exist, see %s documentation", name, goCriticName)
 		}
-
+		if !s.inferredEnabledChecksLowerCased.has(lcName) {
-		if !s.isCheckEnabled(checkName) {
+			s.logger.Warnf("%s: settings were provided for disabled check %q", goCriticName, name)
-			s.logger.Warnf("%s: settings were provided for not enabled check %q", goCriticName, checkName)
 		}
 	}
 
 	return nil
 }
 
-func (s *goCriticSettingsWrapper) getLowerCasedParams() map[string]config.GoCriticCheckSettings {
+func (s *goCriticSettingsWrapper) validateDisabledAndEnabledAtOneMoment() error {
-	ret := make(map[string]config.GoCriticCheckSettings, len(s.SettingsPerCheck))
+	for _, tag := range s.DisabledTags {
-
+		if slices.Contains(s.EnabledTags, tag) {
-	for checker, params := range s.SettingsPerCheck {
+			return fmt.Errorf("tag %q disabled and enabled at one moment", tag)
-		ret[strings.ToLower(checker)] = params
+		}
 	}
 
+	for _, check := range s.DisabledChecks {
+		if slices.Contains(s.EnabledChecks, check) {
+			return fmt.Errorf("check %q disabled and enabled at one moment", check)
+		}
+	}
+
+	return nil
+}
+
+func (s *goCriticSettingsWrapper) validateAtLeastOneCheckerEnabled() error {
+	if len(s.inferredEnabledChecks) == 0 {
+		return errors.New("eventually all checks were disabled: at least one must be enabled")
+	}
+	return nil
+}
+
+func normalizeMap[ValueT any](in map[string]ValueT) map[string]ValueT {
+	ret := make(map[string]ValueT, len(in))
+	for k, v := range in {
+		ret[strings.ToLower(k)] = v
+	}
 	return ret
 }
 
-func (s *goCriticSettingsWrapper) filterByDisableTags(enabledChecks, disableTags []string) []string {
+func isEnabledByDefaultGoCriticChecker(info *gocriticlinter.CheckerInfo) bool {
-	enabledChecksSet := stringsSliceToSet(enabledChecks)
+	// https://github.com/go-critic/go-critic/blob/5b67cfd487ae9fe058b4b19321901b3131810f65/cmd/gocritic/check.go#L342-L345
-
+	return !info.HasTag(gocriticlinter.ExperimentalTag) &&
-	for _, enabledCheck := range enabledChecks {
+		!info.HasTag(gocriticlinter.OpinionatedTag) &&
-		checkInfo, checkInfoExists := s.allCheckerMap[enabledCheck]
+		!info.HasTag(gocriticlinter.PerformanceTag) &&
-		if !checkInfoExists {
+		!info.HasTag(gocriticlinter.SecurityTag)
-			s.logger.Warnf("%s: check %q was not exists via filtering disabled tags", goCriticName, 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
-}
-
-func (s *goCriticSettingsWrapper) isEnabledByDefaultCheck(info *gocriticlinter.CheckerInfo) bool {
-	return !info.HasTag("experimental") &&
-		!info.HasTag("opinionated") &&
-		!info.HasTag("performance")
-}
-
-func validateStringsUniq(ss []string) error {
-	set := map[string]bool{}
-
-	for _, s := range ss {
-		_, ok := set[s]
-		if ok {
-			return fmt.Errorf("%q occurs multiple times in list", s)
-		}
-		set[s] = true
-	}
-
-	return nil
-}
-
-func intersectStringSlice(s1, s2 []string) []string {
-	s1Map := make(map[string]struct{}, len(s1))
-
-	for _, s := range s1 {
-		s1Map[s] = struct{}{}
-	}
-
-	results := make([]string, 0)
-	for _, s := range s2 {
-		if _, exists := s1Map[s]; exists {
-			results = append(results, s)
-		}
-	}
-
-	return results
-}
-
-func sprintAllowedCheckerNames(allowedNames map[string]bool) string {
-	namesSlice := maps.Keys(allowedNames)
-	return sprintStrings(namesSlice)
-}
-
-func sprintStrings(ss []string) string {
-	sort.Strings(ss)
-	return fmt.Sprint(ss)
 }
 
 func debugChecksListf(checks []string, format string, args ...any) {
@@ -617,14 +579,10 @@ func debugChecksListf(checks []string, format string, args ...any) {
 		return
 	}
 
-	goCriticDebugf("%s checks (%d): %s", fmt.Sprintf(format, args...), len(checks), sprintStrings(checks))
+	goCriticDebugf("%s checks (%d): %s", fmt.Sprintf(format, args...), len(checks), sprintSortedStrings(checks))
 }
 
-func stringsSliceToSet(ss []string) map[string]bool {
+func sprintSortedStrings(v []string) string {
-	ret := make(map[string]bool, len(ss))
+	sort.Strings(slices.Clone(v))
-	for _, s := range ss {
+	return fmt.Sprint(v)
-		ret[s] = true
-	}
-
-	return ret
 }

pkg/golinters/gocritic_test.go

@@ -1,56 +1,456 @@
 package golinters
 
 import (
-	"log"
+	"strings"
 	"testing"
 
+	"github.com/go-critic/go-critic/checkers"
+	gocriticlinter "github.com/go-critic/go-critic/linter"
 	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
+	"golang.org/x/exp/maps"
+	"golang.org/x/exp/slices"
 
+	"github.com/golangci/golangci-lint/pkg/config"
 	"github.com/golangci/golangci-lint/pkg/logutils"
 )
 
-func Test_intersectStringSlice(t *testing.T) {
+// https://go-critic.com/overview.html
-	s1 := []string{"diagnostic", "experimental", "opinionated"}
+func Test_goCriticSettingsWrapper_InferEnabledChecks(t *testing.T) {
-	s2 := []string{"opinionated", "experimental"}
+	err := checkers.InitEmbeddedRules()
+	require.NoError(t, err)
 
-	s3 := intersectStringSlice(s1, s2)
+	allCheckersInfo := gocriticlinter.GetCheckersInfo()
 
-	assert.ElementsMatch(t, []string{"experimental", "opinionated"}, s3)
+	allChecksByTag := make(map[string][]string)
+	allChecks := make([]string, 0, len(allCheckersInfo))
+	for _, checker := range allCheckersInfo {
+		allChecks = append(allChecks, checker.Name)
+		for _, tag := range checker.Tags {
+			allChecksByTag[tag] = append(allChecksByTag[tag], checker.Name)
+		}
+	}
+
+	enabledByDefaultChecks := make([]string, 0, len(allCheckersInfo))
+	for _, info := range allCheckersInfo {
+		if isEnabledByDefaultGoCriticChecker(info) {
+			enabledByDefaultChecks = append(enabledByDefaultChecks, info.Name)
+		}
+	}
+	t.Logf("enabled by default checks:\n%s", strings.Join(enabledByDefaultChecks, "\n"))
+
+	insert := func(in []string, toInsert ...string) []string {
+		return append(slices.Clone(in), toInsert...)
+	}
+
+	remove := func(in []string, toRemove ...string) []string {
+		result := slices.Clone(in)
+		for _, v := range toRemove {
+			if i := slices.Index(result, v); i != -1 {
+				result = slices.Delete(result, i, i+1)
+			}
+		}
+		return result
+	}
+
+	uniq := func(in []string) []string {
+		result := slices.Clone(in)
+		slices.Sort(result)
+		return slices.Compact(result)
+	}
+
+	cases := []struct {
+		name                  string
+		sett                  *config.GoCriticSettings
+		expectedEnabledChecks []string
+	}{
+		{
+			name:                  "no configuration",
+			sett:                  &config.GoCriticSettings{},
+			expectedEnabledChecks: enabledByDefaultChecks,
+		},
+		{
+			name: "enable checks",
+			sett: &config.GoCriticSettings{
+				EnabledChecks: []string{"assignOp", "badCall", "emptyDecl"},
+			},
+			expectedEnabledChecks: insert(enabledByDefaultChecks, "emptyDecl"),
+		},
+		{
+			name: "disable checks",
+			sett: &config.GoCriticSettings{
+				DisabledChecks: []string{"assignOp", "emptyDecl"},
+			},
+			expectedEnabledChecks: remove(enabledByDefaultChecks, "assignOp"),
+		},
+		{
+			name: "enable tags",
+			sett: &config.GoCriticSettings{
+				EnabledTags: []string{"style", "experimental"},
+			},
+			expectedEnabledChecks: uniq(insert(insert(
+				enabledByDefaultChecks,
+				allChecksByTag["style"]...),
+				allChecksByTag["experimental"]...)),
+		},
+		{
+			name: "disable tags",
+			sett: &config.GoCriticSettings{
+				DisabledTags: []string{"diagnostic"},
+			},
+			expectedEnabledChecks: remove(enabledByDefaultChecks, allChecksByTag["diagnostic"]...),
+		},
+		{
+			name: "enable checks disable checks",
+			sett: &config.GoCriticSettings{
+				EnabledChecks:  []string{"badCall", "badLock"},
+				DisabledChecks: []string{"assignOp", "badSorting"},
+			},
+			expectedEnabledChecks: insert(remove(enabledByDefaultChecks, "assignOp"), "badLock"),
+		},
+		{
+			name: "enable checks enable tags",
+			sett: &config.GoCriticSettings{
+				EnabledChecks: []string{"badCall", "badLock", "hugeParam"},
+				EnabledTags:   []string{"diagnostic"},
+			},
+			expectedEnabledChecks: uniq(insert(insert(enabledByDefaultChecks,
+				allChecksByTag["diagnostic"]...),
+				"hugeParam")),
+		},
+		{
+			name: "enable checks disable tags",
+			sett: &config.GoCriticSettings{
+				EnabledChecks: []string{"badCall", "badLock", "boolExprSimplify", "hugeParam"},
+				DisabledTags:  []string{"style", "diagnostic"},
+			},
+			expectedEnabledChecks: insert(remove(remove(enabledByDefaultChecks,
+				allChecksByTag["style"]...),
+				allChecksByTag["diagnostic"]...),
+				"hugeParam"),
+		},
+		{
+			name: "enable all checks via tags",
+			sett: &config.GoCriticSettings{
+				EnabledTags: []string{"diagnostic", "experimental", "opinionated", "performance", "style"},
+			},
+			expectedEnabledChecks: allChecks,
+		},
+		{
+			name: "disable checks enable tags",
+			sett: &config.GoCriticSettings{
+				DisabledChecks: []string{"assignOp", "badCall", "badLock", "hugeParam"},
+				EnabledTags:    []string{"style", "diagnostic"},
+			},
+			expectedEnabledChecks: remove(uniq(insert(insert(enabledByDefaultChecks,
+				allChecksByTag["style"]...),
+				allChecksByTag["diagnostic"]...)),
+				"assignOp", "badCall", "badLock"),
+		},
+		{
+			name: "disable checks disable tags",
+			sett: &config.GoCriticSettings{
+				DisabledChecks: []string{"badCall", "badLock", "codegenComment", "hugeParam"},
+				DisabledTags:   []string{"style"},
+			},
+			expectedEnabledChecks: remove(remove(enabledByDefaultChecks,
+				allChecksByTag["style"]...),
+				"badCall", "codegenComment"),
+		},
+		{
+			name: "enable tags disable tags",
+			sett: &config.GoCriticSettings{
+				EnabledTags:  []string{"experimental"},
+				DisabledTags: []string{"style"},
+			},
+			expectedEnabledChecks: remove(uniq(insert(enabledByDefaultChecks,
+				allChecksByTag["experimental"]...)),
+				allChecksByTag["style"]...),
+		},
+		{
+			name: "enable checks disable checks enable tags",
+			sett: &config.GoCriticSettings{
+				EnabledChecks:  []string{"badCall", "badLock", "boolExprSimplify", "indexAlloc", "hugeParam"},
+				DisabledChecks: []string{"deprecatedComment", "typeSwitchVar"},
+				EnabledTags:    []string{"experimental"},
+			},
+			expectedEnabledChecks: remove(uniq(insert(insert(enabledByDefaultChecks,
+				allChecksByTag["experimental"]...),
+				"indexAlloc", "hugeParam")),
+				"deprecatedComment", "typeSwitchVar"),
+		},
+		{
+			name: "enable checks disable checks enable tags disable tags",
+			sett: &config.GoCriticSettings{
+				EnabledChecks:  []string{"badCall", "badCond", "badLock", "indexAlloc", "hugeParam"},
+				DisabledChecks: []string{"deprecatedComment", "typeSwitchVar"},
+				EnabledTags:    []string{"experimental"},
+				DisabledTags:   []string{"performance"},
+			},
+			expectedEnabledChecks: remove(remove(uniq(insert(insert(enabledByDefaultChecks,
+				allChecksByTag["experimental"]...),
+				"badCond")),
+				allChecksByTag["performance"]...),
+				"deprecatedComment", "typeSwitchVar"),
+		},
+		{
+			name: "enable single tag only",
+			sett: &config.GoCriticSettings{
+				DisableAll:  true,
+				EnabledTags: []string{"experimental"},
+			},
+			expectedEnabledChecks: allChecksByTag["experimental"],
+		},
+		{
+			name: "enable two tags only",
+			sett: &config.GoCriticSettings{
+				DisableAll:  true,
+				EnabledTags: []string{"experimental", "performance"},
+			},
+			expectedEnabledChecks: uniq(insert(allChecksByTag["experimental"], allChecksByTag["performance"]...)),
+		},
+		{
+			name: "disable single tag only",
+			sett: &config.GoCriticSettings{
+				EnableAll:    true,
+				DisabledTags: []string{"style"},
+			},
+			expectedEnabledChecks: remove(allChecks, allChecksByTag["style"]...),
+		},
+		{
+			name: "disable two tags only",
+			sett: &config.GoCriticSettings{
+				EnableAll:    true,
+				DisabledTags: []string{"style", "diagnostic"},
+			},
+			expectedEnabledChecks: remove(remove(allChecks, allChecksByTag["style"]...), allChecksByTag["diagnostic"]...),
+		},
+		{
+			name: "enable some checks only",
+			sett: &config.GoCriticSettings{
+				DisableAll:    true,
+				EnabledChecks: []string{"deferInLoop", "dupImport", "ifElseChain", "mapKey"},
+			},
+			expectedEnabledChecks: []string{"deferInLoop", "dupImport", "ifElseChain", "mapKey"},
+		},
+		{
+			name: "disable some checks only",
+			sett: &config.GoCriticSettings{
+				EnableAll:      true,
+				DisabledChecks: []string{"deferInLoop", "dupImport", "ifElseChain", "mapKey"},
+			},
+			expectedEnabledChecks: remove(allChecks, "deferInLoop", "dupImport", "ifElseChain", "mapKey"),
+		},
+		{
+			name: "enable single tag and some checks from another tag only",
+			sett: &config.GoCriticSettings{
+				DisableAll:    true,
+				EnabledTags:   []string{"experimental"},
+				EnabledChecks: []string{"importShadow"},
+			},
+			expectedEnabledChecks: insert(allChecksByTag["experimental"], "importShadow"),
+		},
+		{
+			name: "disable single tag and some checks from another tag only",
+			sett: &config.GoCriticSettings{
+				EnableAll:      true,
+				DisabledTags:   []string{"experimental"},
+				DisabledChecks: []string{"importShadow"},
+			},
+			expectedEnabledChecks: remove(remove(allChecks, allChecksByTag["experimental"]...), "importShadow"),
+		},
+	}
+
+	for _, tt := range cases {
+		tt := tt
+		t.Run(tt.name, func(t *testing.T) {
+			t.Parallel()
+
+			lg := logutils.NewStderrLog("Test_goCriticSettingsWrapper_InferEnabledChecks")
+			wr := newGoCriticSettingsWrapper(tt.sett, lg)
+
+			wr.InferEnabledChecks()
+			assert.ElementsMatch(t, tt.expectedEnabledChecks, maps.Keys(wr.inferredEnabledChecks))
+			assert.NoError(t, wr.Validate())
+		})
+	}
 }
 
-func Test_filterByDisableTags(t *testing.T) {
+func Test_goCriticSettingsWrapper_Validate(t *testing.T) {
-	disabledTags := []string{"experimental", "opinionated"}
+	cases := []struct {
-	enabledChecks := []string{"appendAssign", "sortSlice", "caseOrder", "dupImport"}
+		name        string
+		sett        *config.GoCriticSettings
+		expectedErr bool
+	}{
+		{
+			name: "combine enable-all and disable-all",
+			sett: &config.GoCriticSettings{
+				EnableAll:  true,
+				DisableAll: true,
+			},
+			expectedErr: true,
+		},
+		{
+			name: "combine enable-all and enabled-tags",
+			sett: &config.GoCriticSettings{
+				EnableAll:   true,
+				EnabledTags: []string{"experimental"},
+			},
+			expectedErr: true,
+		},
+		{
+			name: "combine enable-all and enabled-checks",
+			sett: &config.GoCriticSettings{
+				EnableAll:     true,
+				EnabledChecks: []string{"dupImport"},
+			},
+			expectedErr: true,
+		},
+		{
+			name: "combine disable-all and disabled-tags",
+			sett: &config.GoCriticSettings{
+				DisableAll:   true,
+				DisabledTags: []string{"style"},
+			},
+			expectedErr: true,
+		},
+		{
+			name: "combine disable-all and disable-checks",
+			sett: &config.GoCriticSettings{
+				DisableAll:     true,
+				DisabledChecks: []string{"appendAssign"},
+			},
+			expectedErr: true,
+		},
+		{
+			name: "disable-all and no one check enabled",
+			sett: &config.GoCriticSettings{
+				DisableAll: true,
+			},
+			expectedErr: true,
+		},
+		{
+			name: "unknown enabled tag",
+			sett: &config.GoCriticSettings{
+				EnabledTags: []string{"diagnostic", "go-proverbs"},
+			},
+			expectedErr: true,
+		},
+		{
+			name: "unknown disabled tag",
+			sett: &config.GoCriticSettings{
+				DisabledTags: []string{"style", "go-proverbs"},
+			},
+			expectedErr: true,
+		},
+		{
+			name: "unknown enabled check",
+			sett: &config.GoCriticSettings{
+				EnabledChecks: []string{"appendAssign", "noExitAfterDefer", "underef"},
+			},
+			expectedErr: true,
+		},
+		{
+			name: "unknown disabled check",
+			sett: &config.GoCriticSettings{
+				DisabledChecks: []string{"dupSubExpr", "noExitAfterDefer", "returnAfterHttpError"},
+			},
+			expectedErr: true,
+		},
+		{
+			name: "settings for unknown check",
+			sett: &config.GoCriticSettings{
+				SettingsPerCheck: map[string]config.GoCriticCheckSettings{
+					"captLocall":    {"paramsOnly": false},
+					"unnamedResult": {"checkExported": true},
+				},
+			},
+			expectedErr: true,
+		},
+		{
+			name: "settings for disabled check",
+			sett: &config.GoCriticSettings{
+				DisabledChecks: []string{"elseif"},
+				SettingsPerCheck: map[string]config.GoCriticCheckSettings{
+					"elseif": {"skipBalanced": true},
+				},
+			},
+			expectedErr: false, // Just logging.
+		},
+		{
+			name: "settings by lower-cased checker name",
+			sett: &config.GoCriticSettings{
+				EnabledChecks: []string{"tooManyResultsChecker"},
+				SettingsPerCheck: map[string]config.GoCriticCheckSettings{
+					"toomanyresultschecker": {"maxResults": 3},
+					"unnamedResult":         {"checkExported": true},
+				},
+			},
+			expectedErr: false,
+		},
+		{
+			name: "enabled and disabled at one moment check",
+			sett: &config.GoCriticSettings{
+				EnabledChecks:  []string{"appendAssign", "codegenComment", "underef"},
+				DisabledChecks: []string{"elseif", "underef"},
+			},
+			expectedErr: true,
+		},
+		{
+			name: "enabled and disabled at one moment tag",
+			sett: &config.GoCriticSettings{
+				EnabledTags:  []string{"performance", "style"},
+				DisabledTags: []string{"style", "diagnostic"},
+			},
+			expectedErr: true,
+		},
+		{
+			name: "disable all checks via tags",
+			sett: &config.GoCriticSettings{
+				DisabledTags: []string{"diagnostic", "experimental", "opinionated", "performance", "style"},
+			},
+			expectedErr: true,
+		},
+		{
+			name: "enable-all and disable all checks via tags",
+			sett: &config.GoCriticSettings{
+				EnableAll:    true,
+				DisabledTags: []string{"diagnostic", "experimental", "opinionated", "performance", "style"},
+			},
+			expectedErr: true,
+		},
+		{
+			name: "valid configuration",
+			sett: &config.GoCriticSettings{
+				EnabledTags:    []string{"performance"},
+				DisabledChecks: []string{"dupImport", "ifElseChain", "octalLiteral", "whyNoLint"},
+				SettingsPerCheck: map[string]config.GoCriticCheckSettings{
+					"hugeParam":    {"sizeThreshold": 100},
+					"rangeValCopy": {"skipTestFuncs": true},
+				},
+			},
+			expectedErr: false,
+		},
+	}
 
-	settingsWrapper := newGoCriticSettingsWrapper(nil, &tLog{})
+	for _, tt := range cases {
+		tt := tt
+		t.Run(tt.name, func(t *testing.T) {
+			t.Parallel()
 
-	filterEnabledChecks := settingsWrapper.filterByDisableTags(enabledChecks, disabledTags)
+			lg := logutils.NewStderrLog("Test_goCriticSettingsWrapper_Validate")
+			wr := newGoCriticSettingsWrapper(tt.sett, lg)
 
-	assert.ElementsMatch(t, filterEnabledChecks, []string{"appendAssign", "caseOrder"})
+			wr.InferEnabledChecks()
+
+			err := wr.Validate()
+			if tt.expectedErr {
+				if assert.Error(t, err) {
+					t.Log(err)
+				}
+			} else {
+				assert.NoError(t, err)
+			}
+		})
+	}
 }
-
-type tLog struct{}
-
-func (l *tLog) Fatalf(format string, args ...any) {
-	log.Printf(format, args...)
-}
-
-func (l *tLog) Panicf(format string, args ...any) {
-	log.Printf(format, args...)
-}
-
-func (l *tLog) Errorf(format string, args ...any) {
-	log.Printf(format, args...)
-}
-
-func (l *tLog) Warnf(format string, args ...any) {
-	log.Printf(format, args...)
-}
-
-func (l *tLog) Infof(format string, args ...any) {
-	log.Printf(format, args...)
-}
-
-func (l *tLog) Child(_ string) logutils.Log { return nil }
-
-func (l *tLog) SetLevel(_ logutils.LogLevel) {}

test/ruleguard/README.md

@@ -1 +1,14 @@
-This directory contains ruleguard files that are used in functional tests.
+This directory contains ruleguard files that are used in functional tests:
+
+```bash
+T=gocritic.go make test_linters
+```
+
+```bash
+T=gocritic.go make test_fix
+```
+
+Helpful:
+
+- https://go-critic.com/overview.html
+- https://github.com/go-critic/go-critic/blob/master/checkers/rules/rules.go

test/ruleguard/dup.go

@@ -1,23 +0,0 @@
-//go:build ruleguard
-
-package ruleguard
-
-import "github.com/quasilyte/go-ruleguard/dsl"
-
-// Suppose that we want to report the duplicated left and right operands of binary operations.
-//
-// But if the operand has some side effects, this rule can cause false positives:
-// `f() && f()` can make sense (although it's not the best piece of code).
-//
-// This is where *filters* come to the rescue.
-func DupSubExpr(m dsl.Matcher) {
-	// All filters are written as a Where() argument.
-	// In our case, we need to assert that $x is "pure".
-	// It can be achieved by checking the m["x"] member Pure field.
-	m.Match(`$x || $x`,
-		`$x && $x`,
-		`$x | $x`,
-		`$x & $x`).
-		Where(m["x"].Pure).
-		Report(`suspicious identical LHS and RHS`)
-}

test/ruleguard/preferWriteString.go

@@ -0,0 +1,12 @@
+//go:build ruleguard
+
+package ruleguard
+
+import "github.com/quasilyte/go-ruleguard/dsl"
+
+func preferWriteString(m dsl.Matcher) {
+	m.Match(`$w.Write([]byte($s))`).
+		Where(m["w"].Type.Implements("io.StringWriter")).
+		Suggest("$w.WriteString($s)").
+		Report(`$w.WriteString($s) should be preferred to the $$`)
+}

test/ruleguard/rangeExprCopy.go

@@ -6,7 +6,7 @@ import (
 	"github.com/quasilyte/go-ruleguard/dsl"
 )
 
-func RangeExprVal(m dsl.Matcher) {
+func rangeExprCopy(m dsl.Matcher) {
 	m.Match(`for _, $_ := range $x { $*_ }`, `for _, $_ = range $x { $*_ }`).
 		Where(m["x"].Addressable && m["x"].Type.Size >= 512).
 		Report(`$x copy can be avoided with &$x`).

test/ruleguard/stringsSimplify.go

@@ -4,7 +4,7 @@ package ruleguard
 
 import "github.com/quasilyte/go-ruleguard/dsl"
 
-func StringsSimplify(m dsl.Matcher) {
+func stringsSimplify(m dsl.Matcher) {
 	// Some issues have simple fixes that can be expressed as
 	// a replacement pattern. Rules can use Suggest() function
 	// to add a quickfix action for such issues.

test/testdata/configs/gocritic-fix.yml

@@ -4,5 +4,4 @@ linters-settings:
       - ruleguard
     settings:
       ruleguard:
-        rules: 'ruleguard/rangeExprCopy.go,ruleguard/strings_simplify.go'
+        rules: 'ruleguard/rangeExprCopy.go,ruleguard/stringsSimplify.go'
-

test/testdata/configs/gocritic.yml

@@ -1,20 +1,20 @@
 linters-settings:
   gocritic:
+    disabled-checks:
+      - appendAssign
+      - switchTrue
     enabled-checks:
-      - rangeValCopy
+      - hugeParam
-      - flagDeref
-      - wrapperFunc
       - ruleguard
     settings:
-      rangeValCopy:
+      hugeParam:
-        sizeThreshold: 2
+        sizeThreshold: 24
       ruleguard:
-        debug: dupSubExpr
         failOn: dsl,import
-        # comma-separated paths to ruleguard files.
+        # Comma-separated paths to ruleguard files.
         # The ${configDir} is substituted by the directory containing the golangci-lint config file.
         # Note about the directory structure for functional tests:
         #   The ruleguard files used in functional tests cannot be under the 'testdata' directory.
         #   This is because they import the 'github.com/quasilyte/go-ruleguard/dsl' package,
         #   which needs to be added to go.mod. The testdata directory is ignored by go mod.
-        rules: '${configDir}/../../ruleguard/strings_simplify.go,${configDir}/../../ruleguard/dup.go'
+        rules: '${configDir}/../../ruleguard/preferWriteString.go,${configDir}/../../ruleguard/stringsSimplify.go'

test/testdata/gocritic.go

@@ -4,6 +4,7 @@ package testdata
 
 import (
 	"flag"
+	"io"
 	"log"
 	"strings"
 )
@@ -11,38 +12,61 @@ import (
 var _ = *flag.Bool("global1", false, "") // want `flagDeref: immediate deref in \*flag.Bool\(.global1., false, ..\) is most likely an error; consider using flag\.BoolVar`
 
 type size1 struct {
-	a bool
+	a [12]bool
 }
 
 type size2 struct {
 	size1
-	b bool
+	b [12]bool
 }
 
-func gocriticRangeValCopySize1(ss []size1) {
+func gocriticAppendAssign() {
-	for _, s := range ss {
+	var positives, negatives []int
-		log.Print(s)
+	positives = append(negatives, 1)
+	negatives = append(negatives, -1)
+	log.Print(positives, negatives)
+}
+
+func gocriticDupSubExpr(x bool) {
+	if x && x { // want "dupSubExpr: suspicious identical LHS and RHS.*"
+		log.Print("x is true")
 	}
 }
 
-func gocriticRangeValCopySize2(ss []size2) {
+func gocriticHugeParamSize1(ss size1) {
-	for _, s := range ss { // want "rangeValCopy: each iteration copies 2 bytes.*"
+	log.Print(ss)
-		log.Print(s)
+}
+
+func gocriticHugeParamSize2(ss size2) { // want "hugeParam: ss is heavy \\(24 bytes\\); consider passing it by pointer"
+	log.Print(ss)
+}
+
+func gocriticHugeParamSize2Ptr(ss *size2) {
+	log.Print(*ss)
+}
+
+func gocriticSwitchTrue() {
+	switch true {
+	case false:
+		log.Print("false")
+	default:
+		log.Print("default")
 	}
 }
 
+func goCriticPreferStringWriter(w interface {
+	io.Writer
+	io.StringWriter
+}) {
+	w.Write([]byte("test")) // want "ruleguard: w\\.WriteString\\(\"test\"\\) should be preferred.*"
+}
+
 func gocriticStringSimplify() {
 	s := "Most of the time, travellers worry about their luggage."
 	s = strings.Replace(s, ",", "", -1) // want "ruleguard: this Replace call can be simplified.*"
 	log.Print(s)
 }
 
-func gocriticDup(x bool) {
-	if x && x { // want "ruleguard: suspicious identical LHS and RHS.*"
-		log.Print("x is true")
-	}
-}
-
 func gocriticRuleWrapperFunc() {
 	strings.Replace("abcabc", "a", "d", -1) // want "ruleguard: this Replace call can be simplified.*"
 }