From 8df382c18ca8707a573853c1b399db74324e3eb9 Mon Sep 17 00:00:00 2001 From: Fernandez Ludovic Date: Tue, 1 Apr 2025 08:29:58 +0200 Subject: [PATCH 1/5] chore: split wrapper and settingsWrapper --- pkg/golinters/gocritic/gocritic.go | 316 ------------------- pkg/golinters/gocritic/gocritic_settings.go | 330 ++++++++++++++++++++ 2 files changed, 330 insertions(+), 316 deletions(-) create mode 100644 pkg/golinters/gocritic/gocritic_settings.go diff --git a/pkg/golinters/gocritic/gocritic.go b/pkg/golinters/gocritic/gocritic.go index 4fcd9b124385..53e775db1924 100644 --- a/pkg/golinters/gocritic/gocritic.go +++ b/pkg/golinters/gocritic/gocritic.go @@ -227,312 +227,6 @@ func runOnFile(pass *analysis.Pass, f *ast.File, checks []*gocriticlinter.Checke } } -type goCriticChecks[T any] map[string]T - -func (m goCriticChecks[T]) has(name string) bool { - _, ok := m[name] - return ok -} - -type settingsWrapper struct { - *config.GoCriticSettings - - logger logutils.Log - - allCheckers []*gocriticlinter.CheckerInfo - - 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 newSettingsWrapper(settings *config.GoCriticSettings, logger logutils.Log) *settingsWrapper { - allCheckers := gocriticlinter.GetCheckersInfo() - - allChecks := make(goCriticChecks[struct{}], len(allCheckers)) - allChecksLowerCased := make(goCriticChecks[struct{}], len(allCheckers)) - 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 := slices.Sorted(maps.Keys(allChecksByTag)) - - return &settingsWrapper{ - GoCriticSettings: settings, - logger: logger, - allCheckers: allCheckers, - allChecks: allChecks, - allChecksLowerCased: allChecksLowerCased, - allChecksByTag: allChecksByTag, - allTagsSorted: allTagsSorted, - inferredEnabledChecks: make(goCriticChecks[struct{}]), - inferredEnabledChecksLowerCased: make(goCriticChecks[struct{}]), - } -} - -func (s *settingsWrapper) IsCheckEnabled(name string) bool { - return s.inferredEnabledChecks.has(name) -} - -func (s *settingsWrapper) GetLowerCasedParams() map[string]config.GoCriticCheckSettings { - return normalizeMap(s.SettingsPerCheck) -} - -// InferEnabledChecks tries to be consistent with (lintersdb.Manager).build. -func (s *settingsWrapper) InferEnabledChecks() { - s.debugChecksInitialState() - - enabledByDefaultChecks, disabledByDefaultChecks := s.buildEnabledAndDisabledByDefaultChecks() - - debugChecksListf(enabledByDefaultChecks, "Enabled by default") - debugChecksListf(disabledByDefaultChecks, "Disabled by default") - - enabledChecks := make(goCriticChecks[struct{}]) - - if s.EnableAll { - enabledChecks = make(goCriticChecks[struct{}], len(s.allCheckers)) - for _, info := range s.allCheckers { - enabledChecks[info.Name] = struct{}{} - } - } 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{}{} - } - } - - if len(s.EnabledTags) != 0 { - enabledFromTags := s.expandTagsToChecks(s.EnabledTags) - - debugChecksListf(enabledFromTags, "Enabled by config tags %s", s.EnabledTags) - - for _, check := range enabledFromTags { - enabledChecks[check] = struct{}{} - } - } - - if len(s.EnabledChecks) != 0 { - debugChecksListf(s.EnabledChecks, "Enabled by config") - - for _, check := range s.EnabledChecks { - if enabledChecks.has(check) { - s.logger.Warnf("%s: no need to enable check %q: it's already enabled", linterName, check) - continue - } - enabledChecks[check] = struct{}{} - } - } - - if len(s.DisabledTags) != 0 { - disabledFromTags := s.expandTagsToChecks(s.DisabledTags) - - debugChecksListf(disabledFromTags, "Disabled by config tags %s", s.DisabledTags) - - for _, check := range disabledFromTags { - delete(enabledChecks, check) - } - } - - if len(s.DisabledChecks) != 0 { - debugChecksListf(s.DisabledChecks, "Disabled by config") - - for _, check := range s.DisabledChecks { - if !enabledChecks.has(check) { - s.logger.Warnf("%s: no need to disable check %q: it's already disabled", linterName, check) - continue - } - delete(enabledChecks, check) - } - } - - s.inferredEnabledChecks = enabledChecks - s.inferredEnabledChecksLowerCased = normalizeMap(s.inferredEnabledChecks) - - s.debugChecksFinalState() -} - -func (s *settingsWrapper) 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 *settingsWrapper) expandTagsToChecks(tags []string) []string { - var checks []string - for _, tag := range tags { - checks = append(checks, s.allChecksByTag[tag]...) - } - return checks -} - -func (s *settingsWrapper) debugChecksInitialState() { - if !isDebug { - return - } - - debugf("All gocritic existing tags and checks:") - for _, tag := range s.allTagsSorted { - debugChecksListf(s.allChecksByTag[tag], " tag %q", tag) - } -} - -func (s *settingsWrapper) debugChecksFinalState() { - if !isDebug { - return - } - - var enabledChecks []string - var disabledChecks []string - - for _, checker := range s.allCheckers { - check := checker.Name - if s.inferredEnabledChecks.has(check) { - enabledChecks = append(enabledChecks, check) - } else { - disabledChecks = append(disabledChecks, check) - } - } - - debugChecksListf(enabledChecks, "Final used") - - if len(disabledChecks) == 0 { - debugf("All checks are enabled") - } else { - debugChecksListf(disabledChecks, "Final not used") - } -} - -// Validate tries to be consistent with (lintersdb.Validator).validateEnabledDisabledLintersConfig. -func (s *settingsWrapper) Validate() error { - for _, v := range []func() error{ - s.validateOptionsCombinations, - s.validateCheckerTags, - s.validateCheckerNames, - s.validateDisabledAndEnabledAtOneMoment, - s.validateAtLeastOneCheckerEnabled, - } { - if err := v(); err != nil { - return err - } - } - return nil -} - -func (s *settingsWrapper) validateOptionsCombinations() error { - if s.EnableAll { - if s.DisableAll { - return errors.New("enable-all and disable-all options must not be combined") - } - - if len(s.EnabledTags) != 0 { - return errors.New("enable-all and enabled-tags options must not be combined") - } - - if len(s.EnabledChecks) != 0 { - return errors.New("enable-all and enabled-checks options must not be combined") - } - } - - if s.DisableAll { - if len(s.DisabledTags) != 0 { - return errors.New("disable-all and disabled-tags options must not be combined") - } - - if len(s.DisabledChecks) != 0 { - return errors.New("disable-all and disabled-checks options must not be combined") - } - - if len(s.EnabledTags) == 0 && len(s.EnabledChecks) == 0 { - return errors.New("all checks were disabled, but no one check was enabled: at least one must be enabled") - } - } - - return nil -} - -func (s *settingsWrapper) validateCheckerTags() error { - for _, tag := range s.EnabledTags { - if !s.allChecksByTag.has(tag) { - return fmt.Errorf("enabled tag %q doesn't exist, see %s's documentation", tag, linterName) - } - } - - for _, tag := range s.DisabledTags { - if !s.allChecksByTag.has(tag) { - return fmt.Errorf("disabled tag %q doesn't exist, see %s's documentation", tag, linterName) - } - } - - return nil -} - -func (s *settingsWrapper) validateCheckerNames() error { - for _, check := range s.EnabledChecks { - if !s.allChecks.has(check) { - return fmt.Errorf("enabled check %q doesn't exist, see %s's documentation", check, linterName) - } - } - - for _, check := range s.DisabledChecks { - if !s.allChecks.has(check) { - return fmt.Errorf("disabled check %q doesn't exist, see %s documentation", check, linterName) - } - } - - for check := range s.SettingsPerCheck { - lcName := strings.ToLower(check) - if !s.allChecksLowerCased.has(lcName) { - return fmt.Errorf("invalid check settings: check %q doesn't exist, see %s documentation", check, linterName) - } - if !s.inferredEnabledChecksLowerCased.has(lcName) { - s.logger.Warnf("%s: settings were provided for disabled check %q", check, linterName) - } - } - - return nil -} - -func (s *settingsWrapper) validateDisabledAndEnabledAtOneMoment() error { - for _, tag := range s.DisabledTags { - if slices.Contains(s.EnabledTags, tag) { - return fmt.Errorf("tag %q disabled and enabled at one moment", tag) - } - } - - 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 *settingsWrapper) 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 { @@ -548,13 +242,3 @@ func isEnabledByDefaultGoCriticChecker(info *gocriticlinter.CheckerInfo) bool { !info.HasTag(gocriticlinter.PerformanceTag) && !info.HasTag(gocriticlinter.SecurityTag) } - -func debugChecksListf(checks []string, format string, args ...any) { - if !isDebug { - return - } - - v := slices.Sorted(slices.Values(checks)) - - debugf("%s checks (%d): %s", fmt.Sprintf(format, args...), len(checks), strings.Join(v, ", ")) -} diff --git a/pkg/golinters/gocritic/gocritic_settings.go b/pkg/golinters/gocritic/gocritic_settings.go new file mode 100644 index 000000000000..13e1f8f2a5b9 --- /dev/null +++ b/pkg/golinters/gocritic/gocritic_settings.go @@ -0,0 +1,330 @@ +package gocritic + +import ( + "errors" + "fmt" + "maps" + "slices" + "strings" + + gocriticlinter "github.com/go-critic/go-critic/linter" + + "github.com/golangci/golangci-lint/v2/pkg/config" + "github.com/golangci/golangci-lint/v2/pkg/logutils" +) + +type settingsWrapper struct { + *config.GoCriticSettings + + logger logutils.Log + + allCheckers []*gocriticlinter.CheckerInfo + + 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 newSettingsWrapper(settings *config.GoCriticSettings, logger logutils.Log) *settingsWrapper { + allCheckers := gocriticlinter.GetCheckersInfo() + + allChecks := make(goCriticChecks[struct{}], len(allCheckers)) + allChecksLowerCased := make(goCriticChecks[struct{}], len(allCheckers)) + 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 := slices.Sorted(maps.Keys(allChecksByTag)) + + return &settingsWrapper{ + GoCriticSettings: settings, + logger: logger, + allCheckers: allCheckers, + allChecks: allChecks, + allChecksLowerCased: allChecksLowerCased, + allChecksByTag: allChecksByTag, + allTagsSorted: allTagsSorted, + inferredEnabledChecks: make(goCriticChecks[struct{}]), + inferredEnabledChecksLowerCased: make(goCriticChecks[struct{}]), + } +} + +func (s *settingsWrapper) IsCheckEnabled(name string) bool { + return s.inferredEnabledChecks.has(name) +} + +func (s *settingsWrapper) GetLowerCasedParams() map[string]config.GoCriticCheckSettings { + return normalizeMap(s.SettingsPerCheck) +} + +// InferEnabledChecks tries to be consistent with (lintersdb.Manager).build. +func (s *settingsWrapper) InferEnabledChecks() { + s.debugChecksInitialState() + + enabledByDefaultChecks, disabledByDefaultChecks := s.buildEnabledAndDisabledByDefaultChecks() + + debugChecksListf(enabledByDefaultChecks, "Enabled by default") + debugChecksListf(disabledByDefaultChecks, "Disabled by default") + + enabledChecks := make(goCriticChecks[struct{}]) + + if s.EnableAll { + enabledChecks = make(goCriticChecks[struct{}], len(s.allCheckers)) + for _, info := range s.allCheckers { + enabledChecks[info.Name] = struct{}{} + } + } 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{}{} + } + } + + if len(s.EnabledTags) != 0 { + enabledFromTags := s.expandTagsToChecks(s.EnabledTags) + + debugChecksListf(enabledFromTags, "Enabled by config tags %s", s.EnabledTags) + + for _, check := range enabledFromTags { + enabledChecks[check] = struct{}{} + } + } + + if len(s.EnabledChecks) != 0 { + debugChecksListf(s.EnabledChecks, "Enabled by config") + + for _, check := range s.EnabledChecks { + if enabledChecks.has(check) { + s.logger.Warnf("%s: no need to enable check %q: it's already enabled", linterName, check) + continue + } + enabledChecks[check] = struct{}{} + } + } + + if len(s.DisabledTags) != 0 { + disabledFromTags := s.expandTagsToChecks(s.DisabledTags) + + debugChecksListf(disabledFromTags, "Disabled by config tags %s", s.DisabledTags) + + for _, check := range disabledFromTags { + delete(enabledChecks, check) + } + } + + if len(s.DisabledChecks) != 0 { + debugChecksListf(s.DisabledChecks, "Disabled by config") + + for _, check := range s.DisabledChecks { + if !enabledChecks.has(check) { + s.logger.Warnf("%s: no need to disable check %q: it's already disabled", linterName, check) + continue + } + delete(enabledChecks, check) + } + } + + s.inferredEnabledChecks = enabledChecks + s.inferredEnabledChecksLowerCased = normalizeMap(s.inferredEnabledChecks) + + s.debugChecksFinalState() +} + +func (s *settingsWrapper) 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 *settingsWrapper) expandTagsToChecks(tags []string) []string { + var checks []string + for _, tag := range tags { + checks = append(checks, s.allChecksByTag[tag]...) + } + return checks +} + +func (s *settingsWrapper) debugChecksInitialState() { + if !isDebug { + return + } + + debugf("All gocritic existing tags and checks:") + for _, tag := range s.allTagsSorted { + debugChecksListf(s.allChecksByTag[tag], " tag %q", tag) + } +} + +func (s *settingsWrapper) debugChecksFinalState() { + if !isDebug { + return + } + + var enabledChecks []string + var disabledChecks []string + + for _, checker := range s.allCheckers { + check := checker.Name + if s.inferredEnabledChecks.has(check) { + enabledChecks = append(enabledChecks, check) + } else { + disabledChecks = append(disabledChecks, check) + } + } + + debugChecksListf(enabledChecks, "Final used") + + if len(disabledChecks) == 0 { + debugf("All checks are enabled") + } else { + debugChecksListf(disabledChecks, "Final not used") + } +} + +// Validate tries to be consistent with (lintersdb.Validator).validateEnabledDisabledLintersConfig. +func (s *settingsWrapper) Validate() error { + for _, v := range []func() error{ + s.validateOptionsCombinations, + s.validateCheckerTags, + s.validateCheckerNames, + s.validateDisabledAndEnabledAtOneMoment, + s.validateAtLeastOneCheckerEnabled, + } { + if err := v(); err != nil { + return err + } + } + return nil +} + +func (s *settingsWrapper) validateOptionsCombinations() error { + if s.EnableAll { + if s.DisableAll { + return errors.New("enable-all and disable-all options must not be combined") + } + + if len(s.EnabledTags) != 0 { + return errors.New("enable-all and enabled-tags options must not be combined") + } + + if len(s.EnabledChecks) != 0 { + return errors.New("enable-all and enabled-checks options must not be combined") + } + } + + if s.DisableAll { + if len(s.DisabledTags) != 0 { + return errors.New("disable-all and disabled-tags options must not be combined") + } + + if len(s.DisabledChecks) != 0 { + return errors.New("disable-all and disabled-checks options must not be combined") + } + + if len(s.EnabledTags) == 0 && len(s.EnabledChecks) == 0 { + return errors.New("all checks were disabled, but no one check was enabled: at least one must be enabled") + } + } + + return nil +} + +func (s *settingsWrapper) validateCheckerTags() error { + for _, tag := range s.EnabledTags { + if !s.allChecksByTag.has(tag) { + return fmt.Errorf("enabled tag %q doesn't exist, see %s's documentation", tag, linterName) + } + } + + for _, tag := range s.DisabledTags { + if !s.allChecksByTag.has(tag) { + return fmt.Errorf("disabled tag %q doesn't exist, see %s's documentation", tag, linterName) + } + } + + return nil +} + +func (s *settingsWrapper) validateCheckerNames() error { + for _, check := range s.EnabledChecks { + if !s.allChecks.has(check) { + return fmt.Errorf("enabled check %q doesn't exist, see %s's documentation", check, linterName) + } + } + + for _, check := range s.DisabledChecks { + if !s.allChecks.has(check) { + return fmt.Errorf("disabled check %q doesn't exist, see %s documentation", check, linterName) + } + } + + for check := range s.SettingsPerCheck { + lcName := strings.ToLower(check) + if !s.allChecksLowerCased.has(lcName) { + return fmt.Errorf("invalid check settings: check %q doesn't exist, see %s documentation", check, linterName) + } + if !s.inferredEnabledChecksLowerCased.has(lcName) { + s.logger.Warnf("%s: settings were provided for disabled check %q", check, linterName) + } + } + + return nil +} + +func (s *settingsWrapper) validateDisabledAndEnabledAtOneMoment() error { + for _, tag := range s.DisabledTags { + if slices.Contains(s.EnabledTags, tag) { + return fmt.Errorf("tag %q disabled and enabled at one moment", tag) + } + } + + 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 *settingsWrapper) validateAtLeastOneCheckerEnabled() error { + if len(s.inferredEnabledChecks) == 0 { + return errors.New("eventually all checks were disabled: at least one must be enabled") + } + return nil +} + +type goCriticChecks[T any] map[string]T + +func (m goCriticChecks[T]) has(name string) bool { + _, ok := m[name] + return ok +} + +func debugChecksListf(checks []string, format string, args ...any) { + if !isDebug { + return + } + + v := slices.Sorted(slices.Values(checks)) + + debugf("%s checks (%d): %s", fmt.Sprintf(format, args...), len(checks), strings.Join(v, ", ")) +} From 11ea4ab6d2fc0c8fe536f63bb52e9ab1cfd7d449 Mon Sep 17 00:00:00 2001 From: Fernandez Ludovic Date: Tue, 1 Apr 2025 08:32:47 +0200 Subject: [PATCH 2/5] chore: remove runOnPackage --- pkg/golinters/gocritic/gocritic.go | 10 +++------- 1 file changed, 3 insertions(+), 7 deletions(-) diff --git a/pkg/golinters/gocritic/gocritic.go b/pkg/golinters/gocritic/gocritic.go index 53e775db1924..1cfd7eb7d1b4 100644 --- a/pkg/golinters/gocritic/gocritic.go +++ b/pkg/golinters/gocritic/gocritic.go @@ -113,7 +113,9 @@ func (w *goCriticWrapper) run(pass *analysis.Pass) error { linterCtx.SetPackageInfo(pass.TypesInfo, pass.Pkg) - runOnPackage(pass, enabledCheckers, pass.Files) + for _, f := range pass.Files { + runOnFile(pass, f, enabledCheckers) + } return nil } @@ -195,12 +197,6 @@ func (w *goCriticWrapper) normalizeCheckerParamsValue(p any) any { } } -func runOnPackage(pass *analysis.Pass, checks []*gocriticlinter.Checker, files []*ast.File) { - for _, f := range files { - runOnFile(pass, f, checks) - } -} - func runOnFile(pass *analysis.Pass, f *ast.File, checks []*gocriticlinter.Checker) { for _, c := range checks { // All checkers are expected to use *lint.Context From 20686056e028e08ca76665dd461f184541a5c2c2 Mon Sep 17 00:00:00 2001 From: Fernandez Ludovic Date: Tue, 1 Apr 2025 08:40:46 +0200 Subject: [PATCH 3/5] fix: importShadow --- pkg/golinters/gocritic/gocritic.go | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/pkg/golinters/gocritic/gocritic.go b/pkg/golinters/gocritic/gocritic.go index 1cfd7eb7d1b4..0a9ec0561cd0 100644 --- a/pkg/golinters/gocritic/gocritic.go +++ b/pkg/golinters/gocritic/gocritic.go @@ -88,6 +88,7 @@ func (w *goCriticWrapper) init(logger logutils.Log, settings *config.GoCriticSet settingsWrapper := newSettingsWrapper(settings, logger) 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 { @@ -113,7 +114,17 @@ func (w *goCriticWrapper) run(pass *analysis.Pass) error { linterCtx.SetPackageInfo(pass.TypesInfo, pass.Pkg) + needFileInfo := slices.ContainsFunc(enabledCheckers, func(c *gocriticlinter.Checker) bool { + // Related to https://github.com/go-critic/go-critic/blob/440ff466685b41e67d231a1c4a8f5e093374ee93/checkers/importShadow_checker.go#L23 + return strings.EqualFold(c.Info.Name, "importShadow") + }) + for _, f := range pass.Files { + if needFileInfo { + // Related to https://github.com/go-critic/go-critic/blob/440ff466685b41e67d231a1c4a8f5e093374ee93/checkers/importShadow_checker.go#L23 + linterCtx.SetFileInfo(f.Name.Name, f) + } + runOnFile(pass, f, enabledCheckers) } From 7c37d1fe7a0acb74f309dfaf15d018cad5d4636d Mon Sep 17 00:00:00 2001 From: Fernandez Ludovic Date: Tue, 1 Apr 2025 08:42:52 +0200 Subject: [PATCH 4/5] chore: lint --- pkg/goformat/runner.go | 8 ++++---- pkg/golinters/revive/revive.go | 6 +++--- pkg/lint/package.go | 4 ++-- 3 files changed, 9 insertions(+), 9 deletions(-) diff --git a/pkg/goformat/runner.go b/pkg/goformat/runner.go index 4105b9fbf718..3207f117d552 100644 --- a/pkg/goformat/runner.go +++ b/pkg/goformat/runner.go @@ -12,7 +12,7 @@ import ( "regexp" "strings" - "github.com/rogpeppe/go-internal/diff" + rpdiff "github.com/rogpeppe/go-internal/diff" "github.com/golangci/golangci-lint/v2/pkg/config" "github.com/golangci/golangci-lint/v2/pkg/fsutils" @@ -32,11 +32,11 @@ type Runner struct { exitCode int } -func NewRunner(log logutils.Log, +func NewRunner(logger logutils.Log, metaFormatter *goformatters.MetaFormatter, matcher *processors.GeneratedFileMatcher, opts RunnerOptions) *Runner { return &Runner{ - log: log, + log: logger, matcher: matcher, metaFormatter: metaFormatter, opts: opts, @@ -128,7 +128,7 @@ func (c *Runner) process(path string, stdout io.Writer, in io.Reader) error { if c.opts.diff { newName := filepath.ToSlash(path) oldName := newName + ".orig" - _, err = stdout.Write(diff.Diff(oldName, input, newName, output)) + _, err = stdout.Write(rpdiff.Diff(oldName, input, newName, output)) if err != nil { return err } diff --git a/pkg/golinters/revive/revive.go b/pkg/golinters/revive/revive.go index 544f398a06b1..5f9b3a90ace1 100644 --- a/pkg/golinters/revive/revive.go +++ b/pkg/golinters/revive/revive.go @@ -453,11 +453,11 @@ func extractRulesName(rules []lint.Rule) []string { // Extracted from https://github.com/mgechev/revive/blob/v1.7.0/formatter/severity.go // Modified to use pointers (related to hugeParam rule). -func severity(config *lint.Config, failure *lint.Failure) lint.Severity { - if cfg, ok := config.Rules[failure.RuleName]; ok && cfg.Severity == lint.SeverityError { +func severity(cfg *lint.Config, failure *lint.Failure) lint.Severity { + if cfg, ok := cfg.Rules[failure.RuleName]; ok && cfg.Severity == lint.SeverityError { return lint.SeverityError } - if cfg, ok := config.Directives[failure.RuleName]; ok && cfg.Severity == lint.SeverityError { + if cfg, ok := cfg.Directives[failure.RuleName]; ok && cfg.Severity == lint.SeverityError { return lint.SeverityError } return lint.SeverityWarning diff --git a/pkg/lint/package.go b/pkg/lint/package.go index f9bf2230d074..3127a24b8e56 100644 --- a/pkg/lint/package.go +++ b/pkg/lint/package.go @@ -39,13 +39,13 @@ type PackageLoader struct { } // NewPackageLoader creates a new PackageLoader. -func NewPackageLoader(log logutils.Log, cfg *config.Config, args []string, goenv *goutil.Env, loadGuard *load.Guard) *PackageLoader { +func NewPackageLoader(log logutils.Log, cfg *config.Config, args []string, env *goutil.Env, loadGuard *load.Guard) *PackageLoader { return &PackageLoader{ cfg: cfg, args: args, log: log, debugf: logutils.Debug(logutils.DebugKeyLoader), - goenv: goenv, + goenv: env, pkgTestIDRe: regexp.MustCompile(`^(.*) \[(.*)\.test\]`), loadGuard: loadGuard, } From 950b770d800ffead47ea4e71cd7d64934e8c7bcc Mon Sep 17 00:00:00 2001 From: Fernandez Ludovic Date: Tue, 1 Apr 2025 13:28:40 +0200 Subject: [PATCH 5/5] test: add importShadow test --- .../gocritic/testdata/gocritic_importShadow.go | 17 +++++++++++++++++ .../gocritic/testdata/gocritic_inportShadow.yml | 8 ++++++++ 2 files changed, 25 insertions(+) create mode 100644 pkg/golinters/gocritic/testdata/gocritic_importShadow.go create mode 100644 pkg/golinters/gocritic/testdata/gocritic_inportShadow.yml diff --git a/pkg/golinters/gocritic/testdata/gocritic_importShadow.go b/pkg/golinters/gocritic/testdata/gocritic_importShadow.go new file mode 100644 index 000000000000..5f0242ba5a99 --- /dev/null +++ b/pkg/golinters/gocritic/testdata/gocritic_importShadow.go @@ -0,0 +1,17 @@ +//golangcitest:args -Egocritic +//golangcitest:config_path testdata/gocritic_inportShadow.yml +package testdata + +import ( + "fmt" + "path/filepath" +) + +func Bar() { + filepath.Join("a", "b") +} + +func foo() { + filepath := "foo.txt" // want "importShadow: shadow of imported package 'filepath'" + fmt.Printf("File path: %s\n", filepath) +} diff --git a/pkg/golinters/gocritic/testdata/gocritic_inportShadow.yml b/pkg/golinters/gocritic/testdata/gocritic_inportShadow.yml new file mode 100644 index 000000000000..14a2192e72b9 --- /dev/null +++ b/pkg/golinters/gocritic/testdata/gocritic_inportShadow.yml @@ -0,0 +1,8 @@ +version: "2" + +linters: + settings: + gocritic: + disable-all: true + enabled-checks: + - importShadow