From 126988e24955cebafce90d8bfc692c180c065a2d Mon Sep 17 00:00:00 2001 From: Fernandez Ludovic Date: Sun, 18 Feb 2024 15:56:08 +0100 Subject: [PATCH 01/45] chore: remove deprecated flags --- pkg/commands/root.go | 16 ++------ pkg/commands/run.go | 97 -------------------------------------------- pkg/config/run.go | 5 +-- 3 files changed, 4 insertions(+), 114 deletions(-) diff --git a/pkg/commands/root.go b/pkg/commands/root.go index 4425be10fd57..ac90b11a4699 100644 --- a/pkg/commands/root.go +++ b/pkg/commands/root.go @@ -150,26 +150,16 @@ func (e *Executor) needVersionOption() bool { func initRootFlagSet(fs *pflag.FlagSet, cfg *config.Config, needVersionOption bool) { fs.BoolVarP(&cfg.Run.IsVerbose, "verbose", "v", false, wh("Verbose output")) - - var silent bool - fs.BoolVarP(&silent, "silent", "s", false, wh("Disables congrats outputs")) - if err := fs.MarkHidden("silent"); err != nil { - panic(err) - } - err := fs.MarkDeprecated("silent", - "now golangci-lint by default is silent: it doesn't print Congrats message") - if err != nil { - panic(err) - } + fs.StringVar(&cfg.Output.Color, "color", "auto", wh("Use color when printing; can be 'always', 'auto', or 'never'")) fs.StringVar(&cfg.Run.CPUProfilePath, "cpu-profile-path", "", wh("Path to CPU profile output file")) fs.StringVar(&cfg.Run.MemProfilePath, "mem-profile-path", "", wh("Path to memory profile output file")) fs.StringVar(&cfg.Run.TracePath, "trace-path", "", wh("Path to trace output file")) + fs.IntVarP(&cfg.Run.Concurrency, "concurrency", "j", getDefaultConcurrency(), wh("Number of CPUs to use (Default: number of logical CPUs)")) + if needVersionOption { fs.BoolVar(&cfg.Run.PrintVersion, "version", false, wh("Print version")) } - - fs.StringVar(&cfg.Output.Color, "color", "auto", wh("Use color when printing; can be 'always', 'auto', or 'never'")) } diff --git a/pkg/commands/run.go b/pkg/commands/run.go index 5c7083c307a5..51522d0832f5 100644 --- a/pkg/commands/run.go +++ b/pkg/commands/run.go @@ -40,20 +40,6 @@ const ( //nolint:funlen,gomnd func (e *Executor) initFlagSet(fs *pflag.FlagSet, cfg *config.Config, isFinalInit bool) { - hideFlag := func(name string) { - if err := fs.MarkHidden(name); err != nil { - panic(err) - } - - // we run initFlagSet multiple times, but we wouldn't like to see deprecation message multiple times - if isFinalInit { - const deprecateMessage = "flag will be removed soon, please, use .golangci.yml config" - if err := fs.MarkDeprecated(name, deprecateMessage); err != nil { - panic(err) - } - } - } - // Config file config rc := &cfg.Run initConfigFileFlagSet(fs, rc) @@ -69,7 +55,6 @@ func (e *Executor) initFlagSet(fs *pflag.FlagSet, cfg *config.Config, isFinalIni fs.BoolVar(&oc.SortResults, "sort-results", false, wh("Sort linter results")) fs.BoolVar(&oc.PrintWelcomeMessage, "print-welcome", false, wh("Print welcome message")) fs.StringVar(&oc.PathPrefix, "path-prefix", "", wh("Path prefix to add to output")) - hideFlag("print-welcome") // no longer used fs.BoolVar(&cfg.InternalCmdTest, "internal-cmd-test", false, wh("Option is used only for testing golangci-lint command, don't use it")) if err := fs.MarkHidden("internal-cmd-test"); err != nil { @@ -84,10 +69,6 @@ func (e *Executor) initFlagSet(fs *pflag.FlagSet, cfg *config.Config, isFinalIni fs.StringVar(&rc.Go, "go", "", wh("Targeted Go version")) fs.StringSliceVar(&rc.BuildTags, "build-tags", nil, wh("Build tags")) - fs.DurationVar(&rc.Timeout, "deadline", defaultTimeout, wh("Deadline for total work")) - if err := fs.MarkHidden("deadline"); err != nil { - panic(err) - } fs.DurationVar(&rc.Timeout, "timeout", defaultTimeout, wh("Timeout for total work")) fs.BoolVar(&rc.AnalyzeTests, "tests", true, wh("Analyze tests (*_test.go)")) @@ -105,75 +86,6 @@ func (e *Executor) initFlagSet(fs *pflag.FlagSet, cfg *config.Config, isFinalIni fs.BoolVar(&rc.AllowSerialRunners, "allow-serial-runners", false, wh(allowSerialDesc)) fs.BoolVar(&rc.ShowStats, "show-stats", false, wh("Show statistics per linter")) - // Linters settings config - lsc := &cfg.LintersSettings - - // Hide all linters settings flags: they were initially visible, - // but when number of linters started to grow it became obvious that - // we can't fill 90% of flags by linters settings: common flags became hard to find. - // New linters settings should be done only through config file. - fs.BoolVar(&lsc.Errcheck.CheckTypeAssertions, "errcheck.check-type-assertions", - false, "Errcheck: check for ignored type assertion results") - hideFlag("errcheck.check-type-assertions") - fs.BoolVar(&lsc.Errcheck.CheckAssignToBlank, "errcheck.check-blank", false, - "Errcheck: check for errors assigned to blank identifier: _ = errFunc()") - hideFlag("errcheck.check-blank") - fs.StringVar(&lsc.Errcheck.Exclude, "errcheck.exclude", "", - "Path to a file containing a list of functions to exclude from checking") - hideFlag("errcheck.exclude") - fs.StringVar(&lsc.Errcheck.Ignore, "errcheck.ignore", "fmt:.*", - `Comma-separated list of pairs of the form pkg:regex. The regex is used to ignore names within pkg`) - hideFlag("errcheck.ignore") - - fs.BoolVar(&lsc.Govet.CheckShadowing, "govet.check-shadowing", false, - "Govet: check for shadowed variables") - hideFlag("govet.check-shadowing") - - fs.Float64Var(&lsc.Golint.MinConfidence, "golint.min-confidence", 0.8, - "Golint: minimum confidence of a problem to print it") - hideFlag("golint.min-confidence") - - fs.BoolVar(&lsc.Gofmt.Simplify, "gofmt.simplify", true, "Gofmt: simplify code") - hideFlag("gofmt.simplify") - - fs.IntVar(&lsc.Gocyclo.MinComplexity, "gocyclo.min-complexity", - 30, "Minimal complexity of function to report it") - hideFlag("gocyclo.min-complexity") - - fs.BoolVar(&lsc.Maligned.SuggestNewOrder, "maligned.suggest-new", false, - "Maligned: print suggested more optimal struct fields ordering") - hideFlag("maligned.suggest-new") - - fs.IntVar(&lsc.Dupl.Threshold, "dupl.threshold", - 150, "Dupl: Minimal threshold to detect copy-paste") - hideFlag("dupl.threshold") - - fs.BoolVar(&lsc.Goconst.MatchWithConstants, "goconst.match-constant", - true, "Goconst: look for existing constants matching the values") - hideFlag("goconst.match-constant") - fs.IntVar(&lsc.Goconst.MinStringLen, "goconst.min-len", - 3, "Goconst: minimum constant string length") - hideFlag("goconst.min-len") - fs.IntVar(&lsc.Goconst.MinOccurrencesCount, "goconst.min-occurrences", - 3, "Goconst: minimum occurrences of constant string count to trigger issue") - hideFlag("goconst.min-occurrences") - fs.BoolVar(&lsc.Goconst.ParseNumbers, "goconst.numbers", - false, "Goconst: search also for duplicated numbers") - hideFlag("goconst.numbers") - fs.IntVar(&lsc.Goconst.NumberMin, "goconst.min", - 3, "minimum value, only works with goconst.numbers") - hideFlag("goconst.min") - fs.IntVar(&lsc.Goconst.NumberMax, "goconst.max", - 3, "maximum value, only works with goconst.numbers") - hideFlag("goconst.max") - fs.BoolVar(&lsc.Goconst.IgnoreCalls, "goconst.ignore-calls", - true, "Goconst: ignore when constant is not used as function argument") - hideFlag("goconst.ignore-calls") - - fs.IntVar(&lsc.Lll.TabWidth, "lll.tab-width", 1, - "Lll: tab width in spaces") - hideFlag("lll.tab-width") - // Linters config lc := &cfg.Linters e.initLintersFlagSet(fs, lc) @@ -497,7 +409,6 @@ func (e *Executor) executeRun(_ *cobra.Command, args []string) { } }() - e.setTimeoutToDeadlineIfOnlyDeadlineIsSet() ctx, cancel := context.WithTimeout(context.Background(), e.cfg.Run.Timeout) defer cancel() @@ -520,14 +431,6 @@ func (e *Executor) executeRun(_ *cobra.Command, args []string) { e.setupExitCode(ctx) } -// to be removed when deadline is finally decommissioned -func (e *Executor) setTimeoutToDeadlineIfOnlyDeadlineIsSet() { - deadlineValue := e.cfg.Run.Deadline - if deadlineValue != 0 && e.cfg.Run.Timeout == defaultTimeout { - e.cfg.Run.Timeout = deadlineValue - } -} - func (e *Executor) setupExitCode(ctx context.Context) { if ctx.Err() != nil { e.exitCode = exitcodes.Timeout diff --git a/pkg/config/run.go b/pkg/config/run.go index 2bb21d9fa224..0676499d558b 100644 --- a/pkg/config/run.go +++ b/pkg/config/run.go @@ -25,10 +25,7 @@ type Run struct { ExitCodeIfIssuesFound int `mapstructure:"issues-exit-code"` AnalyzeTests bool `mapstructure:"tests"` - // Deprecated: Deadline exists for historical compatibility - // and should not be used. To set run timeout use Timeout instead. - Deadline time.Duration - Timeout time.Duration + Timeout time.Duration PrintVersion bool SkipFiles []string `mapstructure:"skip-files"` From d0eb53b60cbbadf663010a53527f6695348d4bf6 Mon Sep 17 00:00:00 2001 From: Fernandez Ludovic Date: Sun, 18 Feb 2024 16:29:05 +0100 Subject: [PATCH 02/45] chore: re-organize methods and functions --- pkg/commands/cache.go | 68 ++++++++++++++++++++ pkg/commands/executor.go | 128 ++++++++----------------------------- pkg/commands/help.go | 54 ++++++++-------- pkg/commands/root.go | 74 +++++++++++----------- pkg/commands/run.go | 133 ++++++++++++++++++++------------------- pkg/commands/version.go | 26 ++++---- 6 files changed, 241 insertions(+), 242 deletions(-) diff --git a/pkg/commands/cache.go b/pkg/commands/cache.go index 6fdaebaede8f..1ddbab0780c4 100644 --- a/pkg/commands/cache.go +++ b/pkg/commands/cache.go @@ -1,13 +1,19 @@ package commands import ( + "bytes" + "crypto/sha256" "fmt" + "io" "os" "path/filepath" + "strings" "github.com/spf13/cobra" + "gopkg.in/yaml.v3" "github.com/golangci/golangci-lint/internal/cache" + "github.com/golangci/golangci-lint/pkg/config" "github.com/golangci/golangci-lint/pkg/fsutils" "github.com/golangci/golangci-lint/pkg/logutils" ) @@ -60,6 +66,68 @@ func (e *Executor) executeCacheStatus(_ *cobra.Command, _ []string) { } } +func (e *Executor) initHashSalt(version string) error { + binSalt, err := computeBinarySalt(version) + if err != nil { + return fmt.Errorf("failed to calculate binary salt: %w", err) + } + + configSalt, err := computeConfigSalt(e.cfg) + if err != nil { + return fmt.Errorf("failed to calculate config salt: %w", err) + } + + b := bytes.NewBuffer(binSalt) + b.Write(configSalt) + cache.SetSalt(b.Bytes()) + return nil +} + +func computeBinarySalt(version string) ([]byte, error) { + if version != "" && version != "(devel)" { + return []byte(version), nil + } + + if logutils.HaveDebugTag(logutils.DebugKeyBinSalt) { + return []byte("debug"), nil + } + + p, err := os.Executable() + if err != nil { + return nil, err + } + f, err := os.Open(p) + if err != nil { + return nil, err + } + defer f.Close() + h := sha256.New() + if _, err := io.Copy(h, f); err != nil { + return nil, err + } + return h.Sum(nil), nil +} + +func computeConfigSalt(cfg *config.Config) ([]byte, error) { + // We don't hash all config fields to reduce meaningless cache + // invalidations. At least, it has a huge impact on tests speed. + + lintersSettingsBytes, err := yaml.Marshal(cfg.LintersSettings) + if err != nil { + return nil, fmt.Errorf("failed to json marshal config linter settings: %w", err) + } + + configData := bytes.NewBufferString("linters-settings=") + configData.Write(lintersSettingsBytes) + configData.WriteString("\nbuild-tags=%s" + strings.Join(cfg.Run.BuildTags, ",")) + + h := sha256.New() + if _, err := h.Write(configData.Bytes()); err != nil { + return nil, err + } + return h.Sum(nil), nil +} + func dirSizeBytes(path string) (int64, error) { var size int64 err := filepath.Walk(path, func(_ string, info os.FileInfo, err error) error { diff --git a/pkg/commands/executor.go b/pkg/commands/executor.go index a6e08a48b00d..779b588e3a11 100644 --- a/pkg/commands/executor.go +++ b/pkg/commands/executor.go @@ -1,14 +1,7 @@ package commands import ( - "bytes" - "context" - "crypto/sha256" "errors" - "fmt" - "io" - "os" - "path/filepath" "strings" "time" @@ -16,9 +9,7 @@ import ( "github.com/gofrs/flock" "github.com/spf13/cobra" "github.com/spf13/pflag" - "gopkg.in/yaml.v3" - "github.com/golangci/golangci-lint/internal/cache" "github.com/golangci/golangci-lint/internal/pkgcache" "github.com/golangci/golangci-lint/pkg/config" "github.com/golangci/golangci-lint/pkg/fsutils" @@ -163,103 +154,36 @@ func (e *Executor) Execute() error { return e.rootCmd.Execute() } -func (e *Executor) initHashSalt(version string) error { - binSalt, err := computeBinarySalt(version) - if err != nil { - return fmt.Errorf("failed to calculate binary salt: %w", err) - } - - configSalt, err := computeConfigSalt(e.cfg) - if err != nil { - return fmt.Errorf("failed to calculate config salt: %w", err) - } - - b := bytes.NewBuffer(binSalt) - b.Write(configSalt) - cache.SetSalt(b.Bytes()) - return nil -} - -func computeBinarySalt(version string) ([]byte, error) { - if version != "" && version != "(devel)" { - return []byte(version), nil - } - - if logutils.HaveDebugTag(logutils.DebugKeyBinSalt) { - return []byte("debug"), nil - } - - p, err := os.Executable() - if err != nil { - return nil, err - } - f, err := os.Open(p) - if err != nil { - return nil, err - } - defer f.Close() - h := sha256.New() - if _, err := io.Copy(h, f); err != nil { - return nil, err - } - return h.Sum(nil), nil -} - -func computeConfigSalt(cfg *config.Config) ([]byte, error) { - // We don't hash all config fields to reduce meaningless cache - // invalidations. At least, it has a huge impact on tests speed. - - lintersSettingsBytes, err := yaml.Marshal(cfg.LintersSettings) - if err != nil { - return nil, fmt.Errorf("failed to json marshal config linter settings: %w", err) - } - - configData := bytes.NewBufferString("linters-settings=") - configData.Write(lintersSettingsBytes) - configData.WriteString("\nbuild-tags=%s" + strings.Join(cfg.Run.BuildTags, ",")) - - h := sha256.New() - if _, err := h.Write(configData.Bytes()); err != nil { - return nil, err - } - return h.Sum(nil), nil -} +func fixSlicesFlags(fs *pflag.FlagSet) { + // It's a dirty hack to set flag.Changed to true for every string slice flag. + // It's necessary to merge config and command-line slices: otherwise command-line + // flags will always overwrite ones from the config. + fs.VisitAll(func(f *pflag.Flag) { + if f.Value.Type() != "stringSlice" { + return + } -func (e *Executor) acquireFileLock() bool { - if e.cfg.Run.AllowParallelRunners { - e.debugf("Parallel runners are allowed, no locking") - return true - } + s, err := fs.GetStringSlice(f.Name) + if err != nil { + return + } - lockFile := filepath.Join(os.TempDir(), "golangci-lint.lock") - e.debugf("Locking on file %s...", lockFile) - f := flock.New(lockFile) - const retryDelay = time.Second + if s == nil { // assume that every string slice flag has nil as the default + return + } - ctx := context.Background() - if !e.cfg.Run.AllowSerialRunners { - const totalTimeout = 5 * time.Second - var cancel context.CancelFunc - ctx, cancel = context.WithTimeout(ctx, totalTimeout) - defer cancel() - } - if ok, _ := f.TryLockContext(ctx, retryDelay); !ok { - return false - } + var safe []string + for _, v := range s { + // add quotes to escape comma because spf13/pflag use a CSV parser: + // https://github.com/spf13/pflag/blob/85dd5c8bc61cfa382fecd072378089d4e856579d/string_slice.go#L43 + safe = append(safe, `"`+v+`"`) + } - e.flock = f - return true + // calling Set sets Changed to true: next Set calls will append, not overwrite + _ = f.Value.Set(strings.Join(safe, ",")) + }) } -func (e *Executor) releaseFileLock() { - if e.cfg.Run.AllowParallelRunners { - return - } - - if err := e.flock.Unlock(); err != nil { - e.debugf("Failed to unlock on file: %s", err) - } - if err := os.Remove(e.flock.Path()); err != nil { - e.debugf("Failed to remove lock file: %s", err) - } +func wh(text string) string { + return color.GreenString(text) } diff --git a/pkg/commands/help.go b/pkg/commands/help.go index a06d508f27c2..82fba21481db 100644 --- a/pkg/commands/help.go +++ b/pkg/commands/help.go @@ -33,33 +33,6 @@ func (e *Executor) initHelp() { helpCmd.AddCommand(lintersHelpCmd) } -func printLinterConfigs(lcs []*linter.Config) { - sort.Slice(lcs, func(i, j int) bool { - return lcs[i].Name() < lcs[j].Name() - }) - for _, lc := range lcs { - altNamesStr := "" - if len(lc.AlternativeNames) != 0 { - altNamesStr = fmt.Sprintf(" (%s)", strings.Join(lc.AlternativeNames, ", ")) - } - - // If the linter description spans multiple lines, truncate everything following the first newline - linterDescription := lc.Linter.Desc() - firstNewline := strings.IndexRune(linterDescription, '\n') - if firstNewline > 0 { - linterDescription = linterDescription[:firstNewline] - } - - deprecatedMark := "" - if lc.IsDeprecated() { - deprecatedMark = " [" + color.RedString("deprecated") + "]" - } - - fmt.Fprintf(logutils.StdOut, "%s%s%s: %s [fast: %t, auto-fix: %t]\n", color.YellowString(lc.Name()), - altNamesStr, deprecatedMark, linterDescription, !lc.IsSlowLinter(), lc.CanAutoFix) - } -} - func (e *Executor) executeLintersHelp(_ *cobra.Command, _ []string) { var enabledLCs, disabledLCs []*linter.Config for _, lc := range e.DBManager.GetAllSupportedLinterConfigs() { @@ -94,3 +67,30 @@ func (e *Executor) executeLintersHelp(_ *cobra.Command, _ []string) { fmt.Fprintf(logutils.StdOut, "%s: %s\n", color.YellowString(p), strings.Join(linterNames, ", ")) } } + +func printLinterConfigs(lcs []*linter.Config) { + sort.Slice(lcs, func(i, j int) bool { + return lcs[i].Name() < lcs[j].Name() + }) + for _, lc := range lcs { + altNamesStr := "" + if len(lc.AlternativeNames) != 0 { + altNamesStr = fmt.Sprintf(" (%s)", strings.Join(lc.AlternativeNames, ", ")) + } + + // If the linter description spans multiple lines, truncate everything following the first newline + linterDescription := lc.Linter.Desc() + firstNewline := strings.IndexRune(linterDescription, '\n') + if firstNewline > 0 { + linterDescription = linterDescription[:firstNewline] + } + + deprecatedMark := "" + if lc.IsDeprecated() { + deprecatedMark = " [" + color.RedString("deprecated") + "]" + } + + fmt.Fprintf(logutils.StdOut, "%s%s%s: %s [fast: %t, auto-fix: %t]\n", color.YellowString(lc.Name()), + altNamesStr, deprecatedMark, linterDescription, !lc.IsSlowLinter(), lc.CanAutoFix) + } +} diff --git a/pkg/commands/root.go b/pkg/commands/root.go index ac90b11a4699..fa90545709f6 100644 --- a/pkg/commands/root.go +++ b/pkg/commands/root.go @@ -22,6 +22,23 @@ const ( envMemProfileRate = "GL_MEM_PROFILE_RATE" ) +func (e *Executor) initRoot() { + rootCmd := &cobra.Command{ + Use: "golangci-lint", + Short: "golangci-lint is a smart linters runner.", + Long: `Smart, fast linters runner.`, + Args: cobra.NoArgs, + RunE: func(cmd *cobra.Command, _ []string) error { + return cmd.Help() + }, + PersistentPreRunE: e.persistentPreRun, + PersistentPostRunE: e.persistentPostRun, + } + + initRootFlagSet(rootCmd.PersistentFlags(), e.cfg, e.needVersionOption()) + e.rootCmd = rootCmd +} + func (e *Executor) persistentPreRun(_ *cobra.Command, _ []string) error { if e.cfg.Run.PrintVersion { _ = printVersion(logutils.StdOut, e.buildInfo) @@ -89,6 +106,26 @@ func (e *Executor) persistentPostRun(_ *cobra.Command, _ []string) error { return nil } +func (e *Executor) needVersionOption() bool { + return e.buildInfo.Date != "" +} + +func initRootFlagSet(fs *pflag.FlagSet, cfg *config.Config, needVersionOption bool) { + fs.BoolVarP(&cfg.Run.IsVerbose, "verbose", "v", false, wh("Verbose output")) + fs.StringVar(&cfg.Output.Color, "color", "auto", wh("Use color when printing; can be 'always', 'auto', or 'never'")) + + fs.StringVar(&cfg.Run.CPUProfilePath, "cpu-profile-path", "", wh("Path to CPU profile output file")) + fs.StringVar(&cfg.Run.MemProfilePath, "mem-profile-path", "", wh("Path to memory profile output file")) + fs.StringVar(&cfg.Run.TracePath, "trace-path", "", wh("Path to trace output file")) + + fs.IntVarP(&cfg.Run.Concurrency, "concurrency", "j", getDefaultConcurrency(), + wh("Number of CPUs to use (Default: number of logical CPUs)")) + + if needVersionOption { + fs.BoolVar(&cfg.Run.PrintVersion, "version", false, wh("Print version")) + } +} + func printMemStats(ms *runtime.MemStats, logger logutils.Log) { logger.Infof("Mem stats: alloc=%s total_alloc=%s sys=%s "+ "heap_alloc=%s heap_sys=%s heap_idle=%s heap_released=%s heap_in_use=%s "+ @@ -126,40 +163,3 @@ func getDefaultConcurrency() int { return runtime.NumCPU() } - -func (e *Executor) initRoot() { - rootCmd := &cobra.Command{ - Use: "golangci-lint", - Short: "golangci-lint is a smart linters runner.", - Long: `Smart, fast linters runner.`, - Args: cobra.NoArgs, - RunE: func(cmd *cobra.Command, _ []string) error { - return cmd.Help() - }, - PersistentPreRunE: e.persistentPreRun, - PersistentPostRunE: e.persistentPostRun, - } - - initRootFlagSet(rootCmd.PersistentFlags(), e.cfg, e.needVersionOption()) - e.rootCmd = rootCmd -} - -func (e *Executor) needVersionOption() bool { - return e.buildInfo.Date != "" -} - -func initRootFlagSet(fs *pflag.FlagSet, cfg *config.Config, needVersionOption bool) { - fs.BoolVarP(&cfg.Run.IsVerbose, "verbose", "v", false, wh("Verbose output")) - fs.StringVar(&cfg.Output.Color, "color", "auto", wh("Use color when printing; can be 'always', 'auto', or 'never'")) - - fs.StringVar(&cfg.Run.CPUProfilePath, "cpu-profile-path", "", wh("Path to CPU profile output file")) - fs.StringVar(&cfg.Run.MemProfilePath, "mem-profile-path", "", wh("Path to memory profile output file")) - fs.StringVar(&cfg.Run.TracePath, "trace-path", "", wh("Path to trace output file")) - - fs.IntVarP(&cfg.Run.Concurrency, "concurrency", "j", getDefaultConcurrency(), - wh("Number of CPUs to use (Default: number of logical CPUs)")) - - if needVersionOption { - fs.BoolVar(&cfg.Run.PrintVersion, "version", false, wh("Print version")) - } -} diff --git a/pkg/commands/run.go b/pkg/commands/run.go index 51522d0832f5..7dff26087571 100644 --- a/pkg/commands/run.go +++ b/pkg/commands/run.go @@ -7,12 +7,14 @@ import ( "io" "log" "os" + "path/filepath" "runtime" "sort" "strings" "time" "github.com/fatih/color" + "github.com/gofrs/flock" "github.com/spf13/cobra" "github.com/spf13/pflag" "golang.org/x/exp/maps" @@ -38,6 +40,35 @@ const ( envMemLogEvery = "GL_MEM_LOG_EVERY" ) +func (e *Executor) initRun() { + e.runCmd = &cobra.Command{ + Use: "run", + Short: "Run the linters", + Run: e.executeRun, + PreRunE: func(_ *cobra.Command, _ []string) error { + if ok := e.acquireFileLock(); !ok { + return errors.New("parallel golangci-lint is running") + } + return nil + }, + PostRun: func(_ *cobra.Command, _ []string) { + e.releaseFileLock() + }, + } + e.rootCmd.AddCommand(e.runCmd) + + e.runCmd.SetOut(logutils.StdOut) // use custom output to properly color it in Windows terminals + e.runCmd.SetErr(logutils.StdErr) + + e.initRunConfiguration(e.runCmd) +} + +func (e *Executor) initRunConfiguration(cmd *cobra.Command) { + fs := cmd.Flags() + fs.SortFlags = false // sort them as they are defined here + e.initFlagSet(fs, e.cfg, true) +} + //nolint:funlen,gomnd func (e *Executor) initFlagSet(fs *pflag.FlagSet, cfg *config.Config, isFinalInit bool) { // Config file config @@ -118,12 +149,6 @@ func (e *Executor) initFlagSet(fs *pflag.FlagSet, cfg *config.Config, isFinalIni fs.BoolVar(&ic.NeedFix, "fix", false, wh("Fix found issues (if it's supported by the linter)")) } -func (e *Executor) initRunConfiguration(cmd *cobra.Command) { - fs := cmd.Flags() - fs.SortFlags = false // sort them as they are defined here - e.initFlagSet(fs, e.cfg, true) -} - func (e *Executor) getConfigForCommandLine() (*config.Config, error) { // We use another pflag.FlagSet here to not set `changed` flag // on cmd.Flags() options. Otherwise, string slice options will be duplicated. @@ -154,59 +179,6 @@ func (e *Executor) getConfigForCommandLine() (*config.Config, error) { return &cfg, nil } -func (e *Executor) initRun() { - e.runCmd = &cobra.Command{ - Use: "run", - Short: "Run the linters", - Run: e.executeRun, - PreRunE: func(_ *cobra.Command, _ []string) error { - if ok := e.acquireFileLock(); !ok { - return errors.New("parallel golangci-lint is running") - } - return nil - }, - PostRun: func(_ *cobra.Command, _ []string) { - e.releaseFileLock() - }, - } - e.rootCmd.AddCommand(e.runCmd) - - e.runCmd.SetOut(logutils.StdOut) // use custom output to properly color it in Windows terminals - e.runCmd.SetErr(logutils.StdErr) - - e.initRunConfiguration(e.runCmd) -} - -func fixSlicesFlags(fs *pflag.FlagSet) { - // It's a dirty hack to set flag.Changed to true for every string slice flag. - // It's necessary to merge config and command-line slices: otherwise command-line - // flags will always overwrite ones from the config. - fs.VisitAll(func(f *pflag.Flag) { - if f.Value.Type() != "stringSlice" { - return - } - - s, err := fs.GetStringSlice(f.Name) - if err != nil { - return - } - - if s == nil { // assume that every string slice flag has nil as the default - return - } - - var safe []string - for _, v := range s { - // add quotes to escape comma because spf13/pflag use a CSV parser: - // https://github.com/spf13/pflag/blob/85dd5c8bc61cfa382fecd072378089d4e856579d/string_slice.go#L43 - safe = append(safe, `"`+v+`"`) - } - - // calling Set sets Changed to true: next Set calls will append, not overwrite - _ = f.Value.Set(strings.Join(safe, ",")) - }) -} - // runAnalysis executes the linters that have been enabled in the configuration. func (e *Executor) runAnalysis(ctx context.Context, args []string) ([]result.Issue, error) { e.cfg.Run.Args = args @@ -455,6 +427,45 @@ func (e *Executor) setupExitCode(ctx context.Context) { } } +func (e *Executor) acquireFileLock() bool { + if e.cfg.Run.AllowParallelRunners { + e.debugf("Parallel runners are allowed, no locking") + return true + } + + lockFile := filepath.Join(os.TempDir(), "golangci-lint.lock") + e.debugf("Locking on file %s...", lockFile) + f := flock.New(lockFile) + const retryDelay = time.Second + + ctx := context.Background() + if !e.cfg.Run.AllowSerialRunners { + const totalTimeout = 5 * time.Second + var cancel context.CancelFunc + ctx, cancel = context.WithTimeout(ctx, totalTimeout) + defer cancel() + } + if ok, _ := f.TryLockContext(ctx, retryDelay); !ok { + return false + } + + e.flock = f + return true +} + +func (e *Executor) releaseFileLock() { + if e.cfg.Run.AllowParallelRunners { + return + } + + if err := e.flock.Unlock(); err != nil { + e.debugf("Failed to unlock on file: %s", err) + } + if err := os.Remove(e.flock.Path()); err != nil { + e.debugf("Failed to remove lock file: %s", err) + } +} + func watchResources(ctx context.Context, done chan struct{}, logger logutils.Log, debugf logutils.DebugFunc) { startedAt := time.Now() debugf("Started tracking time") @@ -531,7 +542,3 @@ func getDefaultDirectoryExcludeHelp() string { parts = append(parts, "") return strings.Join(parts, "\n") } - -func wh(text string) string { - return color.GreenString(text) -} diff --git a/pkg/commands/version.go b/pkg/commands/version.go index 10ab7c1bb1b1..bd0b4cbdd04e 100644 --- a/pkg/commands/version.go +++ b/pkg/commands/version.go @@ -19,19 +19,6 @@ type versionInfo struct { BuildInfo *debug.BuildInfo } -func (e *Executor) initVersionConfiguration(cmd *cobra.Command) { - fs := cmd.Flags() - fs.SortFlags = false // sort them as they are defined here - initVersionFlagSet(fs, e.cfg) -} - -func initVersionFlagSet(fs *pflag.FlagSet, cfg *config.Config) { - // Version config - vc := &cfg.Version - fs.StringVar(&vc.Format, "format", "", wh("The version's format can be: 'short', 'json'")) - fs.BoolVar(&vc.Debug, "debug", false, wh("Add build information")) -} - func (e *Executor) initVersion() { versionCmd := &cobra.Command{ Use: "version", @@ -76,6 +63,19 @@ func (e *Executor) initVersion() { e.initVersionConfiguration(versionCmd) } +func (e *Executor) initVersionConfiguration(cmd *cobra.Command) { + fs := cmd.Flags() + fs.SortFlags = false // sort them as they are defined here + initVersionFlagSet(fs, e.cfg) +} + +func initVersionFlagSet(fs *pflag.FlagSet, cfg *config.Config) { + // Version config + vc := &cfg.Version + fs.StringVar(&vc.Format, "format", "", wh("The version's format can be: 'short', 'json'")) + fs.BoolVar(&vc.Debug, "debug", false, wh("Add build information")) +} + func printVersion(w io.Writer, buildInfo BuildInfo) error { _, err := fmt.Fprintf(w, "golangci-lint has version %s built with %s from %s on %s\n", buildInfo.Version, buildInfo.GoVersion, buildInfo.Commit, buildInfo.Date) From 8db073c23b2b4a13a30995c8069f9ea3a4da0313 Mon Sep 17 00:00:00 2001 From: Fernandez Ludovic Date: Sun, 18 Feb 2024 16:36:51 +0100 Subject: [PATCH 03/45] chore: clean dead code --- pkg/commands/run.go | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/pkg/commands/run.go b/pkg/commands/run.go index 7dff26087571..325bed8484a2 100644 --- a/pkg/commands/run.go +++ b/pkg/commands/run.go @@ -66,11 +66,11 @@ func (e *Executor) initRun() { func (e *Executor) initRunConfiguration(cmd *cobra.Command) { fs := cmd.Flags() fs.SortFlags = false // sort them as they are defined here - e.initFlagSet(fs, e.cfg, true) + e.initFlagSet(fs, e.cfg) } -//nolint:funlen,gomnd -func (e *Executor) initFlagSet(fs *pflag.FlagSet, cfg *config.Config, isFinalInit bool) { +//nolint:gomnd +func (e *Executor) initFlagSet(fs *pflag.FlagSet, cfg *config.Config) { // Config file config rc := &cfg.Run initConfigFileFlagSet(fs, rc) @@ -159,7 +159,7 @@ func (e *Executor) getConfigForCommandLine() (*config.Config, error) { // `changed` variable inside string slice vars will be shared. // Use another config variable here, not e.cfg, to not // affect main parsing by this parsing of only config option. - e.initFlagSet(fs, &cfg, false) + e.initFlagSet(fs, &cfg) initVersionFlagSet(fs, &cfg) // Parse max options, even force version option: don't want From 62dd1a8f8075ddf7142a8941f462c8f182109302 Mon Sep 17 00:00:00 2001 From: Fernandez Ludovic Date: Sun, 18 Feb 2024 16:51:42 +0100 Subject: [PATCH 04/45] chore: re-organize comments related to cache --- pkg/commands/cache.go | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/pkg/commands/cache.go b/pkg/commands/cache.go index 1ddbab0780c4..a9fb4a2b5c08 100644 --- a/pkg/commands/cache.go +++ b/pkg/commands/cache.go @@ -108,10 +108,11 @@ func computeBinarySalt(version string) ([]byte, error) { return h.Sum(nil), nil } +// computeConfigSalt computes configuration hash. +// We don't hash all config fields to reduce meaningless cache invalidations. +// At least, it has a huge impact on tests speed. +// Fields: `LintersSettings` and `Run.BuildTags`. func computeConfigSalt(cfg *config.Config) ([]byte, error) { - // We don't hash all config fields to reduce meaningless cache - // invalidations. At least, it has a huge impact on tests speed. - lintersSettingsBytes, err := yaml.Marshal(cfg.LintersSettings) if err != nil { return nil, fmt.Errorf("failed to json marshal config linter settings: %w", err) From 1e2ba1fe7135829226816f5039a2405b67d915c4 Mon Sep 17 00:00:00 2001 From: Fernandez Ludovic Date: Sun, 18 Feb 2024 22:44:41 +0100 Subject: [PATCH 05/45] chore: unify fonction naming --- pkg/commands/run.go | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/pkg/commands/run.go b/pkg/commands/run.go index 325bed8484a2..dc3aef80a9c6 100644 --- a/pkg/commands/run.go +++ b/pkg/commands/run.go @@ -66,11 +66,11 @@ func (e *Executor) initRun() { func (e *Executor) initRunConfiguration(cmd *cobra.Command) { fs := cmd.Flags() fs.SortFlags = false // sort them as they are defined here - e.initFlagSet(fs, e.cfg) + e.initRunFlagSet(fs, e.cfg) } //nolint:gomnd -func (e *Executor) initFlagSet(fs *pflag.FlagSet, cfg *config.Config) { +func (e *Executor) initRunFlagSet(fs *pflag.FlagSet, cfg *config.Config) { // Config file config rc := &cfg.Run initConfigFileFlagSet(fs, rc) @@ -159,7 +159,7 @@ func (e *Executor) getConfigForCommandLine() (*config.Config, error) { // `changed` variable inside string slice vars will be shared. // Use another config variable here, not e.cfg, to not // affect main parsing by this parsing of only config option. - e.initFlagSet(fs, &cfg) + e.initRunFlagSet(fs, &cfg) initVersionFlagSet(fs, &cfg) // Parse max options, even force version option: don't want From 865d6eab106a480da1e32ac8552d3ba0fb8ebca3 Mon Sep 17 00:00:00 2001 From: Fernandez Ludovic Date: Sun, 18 Feb 2024 23:09:19 +0100 Subject: [PATCH 06/45] chore: move BuildInfo to version --- pkg/commands/executor.go | 7 ------- pkg/commands/version.go | 7 +++++++ 2 files changed, 7 insertions(+), 7 deletions(-) diff --git a/pkg/commands/executor.go b/pkg/commands/executor.go index 779b588e3a11..8823fa466975 100644 --- a/pkg/commands/executor.go +++ b/pkg/commands/executor.go @@ -22,13 +22,6 @@ import ( "github.com/golangci/golangci-lint/pkg/timeutils" ) -type BuildInfo struct { - GoVersion string `json:"goVersion"` - Version string `json:"version"` - Commit string `json:"commit"` - Date string `json:"date"` -} - type Executor struct { rootCmd *cobra.Command runCmd *cobra.Command diff --git a/pkg/commands/version.go b/pkg/commands/version.go index bd0b4cbdd04e..1978bb2923bf 100644 --- a/pkg/commands/version.go +++ b/pkg/commands/version.go @@ -14,6 +14,13 @@ import ( "github.com/golangci/golangci-lint/pkg/config" ) +type BuildInfo struct { + GoVersion string `json:"goVersion"` + Version string `json:"version"` + Commit string `json:"commit"` + Date string `json:"date"` +} + type versionInfo struct { Info BuildInfo BuildInfo *debug.BuildInfo From d917656f4539b91c4d072983464c6fb134ea9580 Mon Sep 17 00:00:00 2001 From: Fernandez Ludovic Date: Mon, 19 Feb 2024 23:44:21 +0100 Subject: [PATCH 07/45] fix: reports default flag values into defaultLintersSettings --- pkg/config/linters_settings.go | 23 +++++++++++++++++++++++ 1 file changed, 23 insertions(+) diff --git a/pkg/config/linters_settings.go b/pkg/config/linters_settings.go index 701beb2241b0..7158f0a9d725 100644 --- a/pkg/config/linters_settings.go +++ b/pkg/config/linters_settings.go @@ -21,6 +21,12 @@ var defaultLintersSettings = LintersSettings{ Dogsled: DogsledSettings{ MaxBlankIdentifiers: 2, }, + Dupl: DuplSettings{ + Threshold: 150, + }, + Errcheck: ErrcheckSettings{ + Ignore: "fmt:.*", + }, ErrorLint: ErrorLintSettings{ Errorf: true, ErrorfMulti: true, @@ -46,9 +52,20 @@ var defaultLintersSettings = LintersSettings{ Gocognit: GocognitSettings{ MinComplexity: 30, }, + Goconst: GoConstSettings{ + MatchWithConstants: true, + MinStringLen: 3, + MinOccurrencesCount: 3, + NumberMin: 3, + NumberMax: 3, + IgnoreCalls: true, + }, Gocritic: GoCriticSettings{ SettingsPerCheck: map[string]GoCriticCheckSettings{}, }, + Gocyclo: GoCycloSettings{ + MinComplexity: 30, + }, Godox: GodoxSettings{ Keywords: []string{}, }, @@ -56,11 +73,17 @@ var defaultLintersSettings = LintersSettings{ Scope: "declarations", Period: true, }, + Gofmt: GoFmtSettings{ + Simplify: true, + }, Gofumpt: GofumptSettings{ LangVersion: "", ModulePath: "", ExtraRules: false, }, + Golint: GoLintSettings{ + MinConfidence: 0.8, + }, Gosec: GoSecSettings{ Concurrency: runtime.NumCPU(), }, From b7b86a6cd70c35463748b6a72454321552be9d02 Mon Sep 17 00:00:00 2001 From: Fernandez Ludovic Date: Tue, 20 Feb 2024 01:53:36 +0100 Subject: [PATCH 08/45] chore: reformat some comments --- pkg/commands/executor.go | 11 ++++------- 1 file changed, 4 insertions(+), 7 deletions(-) diff --git a/pkg/commands/executor.go b/pkg/commands/executor.go index 8823fa466975..68fb9fed917b 100644 --- a/pkg/commands/executor.go +++ b/pkg/commands/executor.go @@ -60,8 +60,7 @@ func NewExecutor(buildInfo BuildInfo) *Executor { e.debugf("Starting execution...") e.log = report.NewLogWrapper(logutils.NewStderrLog(logutils.DebugKeyEmpty), &e.reportData) - // to setup log level early we need to parse config from command line extra time to - // find `-v` option + // to set up log level early we need to parse config from command line extra time to find `-v` option. commandLineCfg, err := e.getConfigForCommandLine() if err != nil && !errors.Is(err, pflag.ErrHelp) { e.log.Fatalf("Can't get config for command line: %s", err) @@ -81,8 +80,7 @@ func NewExecutor(buildInfo BuildInfo) *Executor { } } - // init of commands must be done before config file reading because - // init sets config with the default values of flags + // init of commands must be done before config file reading because init sets config with the default values of flags. e.initRoot() e.initRun() e.initHelp() @@ -91,9 +89,8 @@ func NewExecutor(buildInfo BuildInfo) *Executor { e.initVersion() e.initCache() - // init e.cfg by values from config: flags parse will see these values - // like the default ones. It will overwrite them only if the same option - // is found in command-line: it's ok, command-line has higher priority. + // init e.cfg by values from config: flags parse will see these values like the default ones. + // It will overwrite them only if the same option is found in command-line: it's ok, command-line has higher priority. r := config.NewFileReader(e.cfg, commandLineCfg, e.log.Child(logutils.DebugKeyConfigReader)) if err = r.Read(); err != nil { From 14bc543c13a7a0f43c0b4f4aaf92715ee20668a4 Mon Sep 17 00:00:00 2001 From: Fernandez Ludovic Date: Tue, 20 Feb 2024 01:59:37 +0100 Subject: [PATCH 09/45] chore(presets): converts methods to functions --- pkg/commands/help.go | 3 +- pkg/commands/linters.go | 3 +- pkg/lint/lintersdb/manager.go | 72 ++++++++++++++++----------------- pkg/lint/lintersdb/validator.go | 4 +- 4 files changed, 42 insertions(+), 40 deletions(-) diff --git a/pkg/commands/help.go b/pkg/commands/help.go index 82fba21481db..6a8d534a6bf6 100644 --- a/pkg/commands/help.go +++ b/pkg/commands/help.go @@ -6,6 +6,7 @@ import ( "strings" "github.com/fatih/color" + "github.com/golangci/golangci-lint/pkg/lint/lintersdb" "github.com/spf13/cobra" "github.com/golangci/golangci-lint/pkg/lint/linter" @@ -53,7 +54,7 @@ func (e *Executor) executeLintersHelp(_ *cobra.Command, _ []string) { printLinterConfigs(disabledLCs) color.Green("\nLinters presets:") - for _, p := range e.DBManager.AllPresets() { + for _, p := range lintersdb.AllPresets() { linters := e.DBManager.GetAllLinterConfigsForPreset(p) var linterNames []string for _, lc := range linters { diff --git a/pkg/commands/linters.go b/pkg/commands/linters.go index 69df211542be..d68250b9091d 100644 --- a/pkg/commands/linters.go +++ b/pkg/commands/linters.go @@ -5,6 +5,7 @@ import ( "strings" "github.com/fatih/color" + "github.com/golangci/golangci-lint/pkg/lint/lintersdb" "github.com/spf13/cobra" "github.com/spf13/pflag" @@ -38,7 +39,7 @@ func (e *Executor) initLintersFlagSet(fs *pflag.FlagSet, cfg *config.Linters) { fs.BoolVar(&cfg.Fast, "fast", false, wh("Enable only fast linters from enabled linters set (first run won't be fast)")) fs.StringSliceVarP(&cfg.Presets, "presets", "p", nil, wh(fmt.Sprintf("Enable presets (%s) of linters. Run 'golangci-lint help linters' to see "+ - "them. This option implies option --disable-all", strings.Join(e.DBManager.AllPresets(), "|")))) + "them. This option implies option --disable-all", strings.Join(lintersdb.AllPresets(), "|")))) } // executeLinters runs the 'linters' CLI command, which displays the supported linters. diff --git a/pkg/lint/lintersdb/manager.go b/pkg/lint/lintersdb/manager.go index e6a171f1ff5d..f04849895f63 100644 --- a/pkg/lint/lintersdb/manager.go +++ b/pkg/lint/lintersdb/manager.go @@ -33,32 +33,6 @@ func NewManager(cfg *config.Config, log logutils.Log) *Manager { return m } -func (Manager) AllPresets() []string { - return []string{ - linter.PresetBugs, - linter.PresetComment, - linter.PresetComplexity, - linter.PresetError, - linter.PresetFormatting, - linter.PresetImport, - linter.PresetMetaLinter, - linter.PresetModule, - linter.PresetPerformance, - linter.PresetSQL, - linter.PresetStyle, - linter.PresetTest, - linter.PresetUnused, - } -} - -func (m Manager) allPresetsSet() map[string]bool { - ret := map[string]bool{} - for _, p := range m.AllPresets() { - ret[p] = true - } - return ret -} - func (m Manager) GetLinterConfigs(name string) []*linter.Config { return m.nameToLCs[name] } @@ -964,16 +938,6 @@ func (m Manager) GetAllEnabledByDefaultLinters() []*linter.Config { return ret } -func linterConfigsToMap(lcs []*linter.Config) map[string]*linter.Config { - ret := map[string]*linter.Config{} - for _, lc := range lcs { - lc := lc // local copy - ret[lc.Name()] = lc - } - - return ret -} - func (m Manager) GetAllLinterConfigsForPreset(p string) []*linter.Config { var ret []*linter.Config for _, lc := range m.GetAllSupportedLinterConfigs() { @@ -992,6 +956,42 @@ func (m Manager) GetAllLinterConfigsForPreset(p string) []*linter.Config { return ret } +func linterConfigsToMap(lcs []*linter.Config) map[string]*linter.Config { + ret := map[string]*linter.Config{} + for _, lc := range lcs { + lc := lc // local copy + ret[lc.Name()] = lc + } + + return ret +} + +func AllPresets() []string { + return []string{ + linter.PresetBugs, + linter.PresetComment, + linter.PresetComplexity, + linter.PresetError, + linter.PresetFormatting, + linter.PresetImport, + linter.PresetMetaLinter, + linter.PresetModule, + linter.PresetPerformance, + linter.PresetSQL, + linter.PresetStyle, + linter.PresetTest, + linter.PresetUnused, + } +} + +func allPresetsSet() map[string]bool { + ret := map[string]bool{} + for _, p := range AllPresets() { + ret[p] = true + } + return ret +} + // Trims the Go version to keep only M.m. // Since Go 1.21 the version inside the go.mod can be a patched version (ex: 1.21.0). // https://go.dev/doc/toolchain#versions diff --git a/pkg/lint/lintersdb/validator.go b/pkg/lint/lintersdb/validator.go index 52a70d85900d..2b4f52da0bb7 100644 --- a/pkg/lint/lintersdb/validator.go +++ b/pkg/lint/lintersdb/validator.go @@ -39,11 +39,11 @@ func (v Validator) validateLintersNames(cfg *config.Linters) error { } func (v Validator) validatePresets(cfg *config.Linters) error { - allPresets := v.m.allPresetsSet() + allPresets := allPresetsSet() for _, p := range cfg.Presets { if !allPresets[p] { return fmt.Errorf("no such preset %q: only next presets exist: (%s)", - p, strings.Join(v.m.AllPresets(), "|")) + p, strings.Join(AllPresets(), "|")) } } From 86be41019a3097f9653f28142b29a8473483f40b Mon Sep 17 00:00:00 2001 From: Fernandez Ludovic Date: Tue, 20 Feb 2024 02:01:37 +0100 Subject: [PATCH 10/45] chore: same receiver for all Manager methods --- pkg/lint/lintersdb/manager.go | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/pkg/lint/lintersdb/manager.go b/pkg/lint/lintersdb/manager.go index f04849895f63..1f94579e8f31 100644 --- a/pkg/lint/lintersdb/manager.go +++ b/pkg/lint/lintersdb/manager.go @@ -33,12 +33,12 @@ func NewManager(cfg *config.Config, log logutils.Log) *Manager { return m } -func (m Manager) GetLinterConfigs(name string) []*linter.Config { +func (m *Manager) GetLinterConfigs(name string) []*linter.Config { return m.nameToLCs[name] } //nolint:funlen -func (m Manager) GetAllSupportedLinterConfigs() []*linter.Config { +func (m *Manager) GetAllSupportedLinterConfigs() []*linter.Config { var ( asasalintCfg *config.AsasalintSettings bidichkCfg *config.BiDiChkSettings @@ -927,7 +927,7 @@ func (m Manager) GetAllSupportedLinterConfigs() []*linter.Config { return linters } -func (m Manager) GetAllEnabledByDefaultLinters() []*linter.Config { +func (m *Manager) GetAllEnabledByDefaultLinters() []*linter.Config { var ret []*linter.Config for _, lc := range m.GetAllSupportedLinterConfigs() { if lc.EnabledByDefault { @@ -938,7 +938,7 @@ func (m Manager) GetAllEnabledByDefaultLinters() []*linter.Config { return ret } -func (m Manager) GetAllLinterConfigsForPreset(p string) []*linter.Config { +func (m *Manager) GetAllLinterConfigsForPreset(p string) []*linter.Config { var ret []*linter.Config for _, lc := range m.GetAllSupportedLinterConfigs() { if lc.IsDeprecated() { From aaa99dcf4fcb7114a321f126811bea65bb08bfde Mon Sep 17 00:00:00 2001 From: Fernandez Ludovic Date: Tue, 20 Feb 2024 04:10:35 +0100 Subject: [PATCH 11/45] chore: unexports some fields --- pkg/commands/executor.go | 12 ++++++------ pkg/commands/help.go | 4 ++-- pkg/commands/linters.go | 4 ++-- pkg/commands/run.go | 8 ++++---- 4 files changed, 14 insertions(+), 14 deletions(-) diff --git a/pkg/commands/executor.go b/pkg/commands/executor.go index 68fb9fed917b..8ab9a2fde8bc 100644 --- a/pkg/commands/executor.go +++ b/pkg/commands/executor.go @@ -33,8 +33,8 @@ type Executor struct { cfg *config.Config // cfg is the unmarshaled data from the golangci config file. log logutils.Log reportData report.Data - DBManager *lintersdb.Manager - EnabledLintersSet *lintersdb.EnabledSet + dbManager *lintersdb.Manager + enabledLintersSet *lintersdb.EnabledSet contextLoader *lint.ContextLoader goenv *goutil.Env fileCache *fsutils.FileCache @@ -53,7 +53,7 @@ func NewExecutor(buildInfo BuildInfo) *Executor { e := &Executor{ cfg: config.NewDefault(), buildInfo: buildInfo, - DBManager: lintersdb.NewManager(nil, nil), + dbManager: lintersdb.NewManager(nil, nil), debugf: logutils.Debug(logutils.DebugKeyExec), } @@ -113,14 +113,14 @@ func NewExecutor(buildInfo BuildInfo) *Executor { } // recreate after getting config - e.DBManager = lintersdb.NewManager(e.cfg, e.log) + e.dbManager = lintersdb.NewManager(e.cfg, e.log) // Slice options must be explicitly set for proper merging of config and command-line options. fixSlicesFlags(e.runCmd.Flags()) fixSlicesFlags(e.lintersCmd.Flags()) - e.EnabledLintersSet = lintersdb.NewEnabledSet(e.DBManager, - lintersdb.NewValidator(e.DBManager), e.log.Child(logutils.DebugKeyLintersDB), e.cfg) + e.enabledLintersSet = lintersdb.NewEnabledSet(e.dbManager, + lintersdb.NewValidator(e.dbManager), e.log.Child(logutils.DebugKeyLintersDB), e.cfg) e.goenv = goutil.NewEnv(e.log.Child(logutils.DebugKeyGoEnv)) e.fileCache = fsutils.NewFileCache() e.lineCache = fsutils.NewLineCache(e.fileCache) diff --git a/pkg/commands/help.go b/pkg/commands/help.go index 6a8d534a6bf6..0d4519c0867f 100644 --- a/pkg/commands/help.go +++ b/pkg/commands/help.go @@ -36,7 +36,7 @@ func (e *Executor) initHelp() { func (e *Executor) executeLintersHelp(_ *cobra.Command, _ []string) { var enabledLCs, disabledLCs []*linter.Config - for _, lc := range e.DBManager.GetAllSupportedLinterConfigs() { + for _, lc := range e.dbManager.GetAllSupportedLinterConfigs() { if lc.Internal { continue } @@ -55,7 +55,7 @@ func (e *Executor) executeLintersHelp(_ *cobra.Command, _ []string) { color.Green("\nLinters presets:") for _, p := range lintersdb.AllPresets() { - linters := e.DBManager.GetAllLinterConfigsForPreset(p) + linters := e.dbManager.GetAllLinterConfigsForPreset(p) var linterNames []string for _, lc := range linters { if lc.Internal { diff --git a/pkg/commands/linters.go b/pkg/commands/linters.go index d68250b9091d..6f47193ceea9 100644 --- a/pkg/commands/linters.go +++ b/pkg/commands/linters.go @@ -44,7 +44,7 @@ func (e *Executor) initLintersFlagSet(fs *pflag.FlagSet, cfg *config.Linters) { // executeLinters runs the 'linters' CLI command, which displays the supported linters. func (e *Executor) executeLinters(_ *cobra.Command, _ []string) error { - enabledLintersMap, err := e.EnabledLintersSet.GetEnabledLintersMap() + enabledLintersMap, err := e.enabledLintersSet.GetEnabledLintersMap() if err != nil { return fmt.Errorf("can't get enabled linters: %w", err) } @@ -52,7 +52,7 @@ func (e *Executor) executeLinters(_ *cobra.Command, _ []string) error { var enabledLinters []*linter.Config var disabledLCs []*linter.Config - for _, lc := range e.DBManager.GetAllSupportedLinterConfigs() { + for _, lc := range e.dbManager.GetAllSupportedLinterConfigs() { if lc.Internal { continue } diff --git a/pkg/commands/run.go b/pkg/commands/run.go index dc3aef80a9c6..4cf19f731501 100644 --- a/pkg/commands/run.go +++ b/pkg/commands/run.go @@ -183,17 +183,17 @@ func (e *Executor) getConfigForCommandLine() (*config.Config, error) { func (e *Executor) runAnalysis(ctx context.Context, args []string) ([]result.Issue, error) { e.cfg.Run.Args = args - lintersToRun, err := e.EnabledLintersSet.GetOptimizedLinters() + lintersToRun, err := e.enabledLintersSet.GetOptimizedLinters() if err != nil { return nil, err } - enabledLintersMap, err := e.EnabledLintersSet.GetEnabledLintersMap() + enabledLintersMap, err := e.enabledLintersSet.GetEnabledLintersMap() if err != nil { return nil, err } - for _, lc := range e.DBManager.GetAllSupportedLinterConfigs() { + for _, lc := range e.dbManager.GetAllSupportedLinterConfigs() { isEnabled := enabledLintersMap[lc.Name()] != nil e.reportData.AddLinter(lc.Name(), isEnabled, lc.EnabledByDefault) } @@ -205,7 +205,7 @@ func (e *Executor) runAnalysis(ctx context.Context, args []string) ([]result.Iss lintCtx.Log = e.log.Child(logutils.DebugKeyLintersContext) runner, err := lint.NewRunner(e.cfg, e.log.Child(logutils.DebugKeyRunner), - e.goenv, e.EnabledLintersSet, e.lineCache, e.fileCache, e.DBManager, lintCtx.Packages) + e.goenv, e.enabledLintersSet, e.lineCache, e.fileCache, e.dbManager, lintCtx.Packages) if err != nil { return nil, err } From 607608314562fc303a22e2cb749389cb5c4aac27 Mon Sep 17 00:00:00 2001 From: Fernandez Ludovic Date: Tue, 20 Feb 2024 04:12:14 +0100 Subject: [PATCH 12/45] chore: move getConfigForCommandLine to executor file --- pkg/commands/executor.go | 32 ++++++++++++++++++++++++++++++++ pkg/commands/run.go | 30 ------------------------------ 2 files changed, 32 insertions(+), 30 deletions(-) diff --git a/pkg/commands/executor.go b/pkg/commands/executor.go index 8ab9a2fde8bc..002f8e82373a 100644 --- a/pkg/commands/executor.go +++ b/pkg/commands/executor.go @@ -2,6 +2,8 @@ package commands import ( "errors" + "fmt" + "os" "strings" "time" @@ -144,6 +146,36 @@ func (e *Executor) Execute() error { return e.rootCmd.Execute() } +func (e *Executor) getConfigForCommandLine() (*config.Config, error) { + // We use another pflag.FlagSet here to not set `changed` flag + // on cmd.Flags() options. Otherwise, string slice options will be duplicated. + fs := pflag.NewFlagSet("config flag set", pflag.ContinueOnError) + + var cfg config.Config + // Don't do `fs.AddFlagSet(cmd.Flags())` because it shares flags representations: + // `changed` variable inside string slice vars will be shared. + // Use another config variable here, not e.cfg, to not + // affect main parsing by this parsing of only config option. + e.initRunFlagSet(fs, &cfg) + initVersionFlagSet(fs, &cfg) + + // Parse max options, even force version option: don't want + // to get access to Executor here: it's error-prone to use + // cfg vs e.cfg. + initRootFlagSet(fs, &cfg, true) + + fs.Usage = func() {} // otherwise, help text will be printed twice + if err := fs.Parse(os.Args); err != nil { + if errors.Is(err, pflag.ErrHelp) { + return nil, err + } + + return nil, fmt.Errorf("can't parse args: %w", err) + } + + return &cfg, nil +} + func fixSlicesFlags(fs *pflag.FlagSet) { // It's a dirty hack to set flag.Changed to true for every string slice flag. // It's necessary to merge config and command-line slices: otherwise command-line diff --git a/pkg/commands/run.go b/pkg/commands/run.go index 4cf19f731501..2d16bbc218b3 100644 --- a/pkg/commands/run.go +++ b/pkg/commands/run.go @@ -149,36 +149,6 @@ func (e *Executor) initRunFlagSet(fs *pflag.FlagSet, cfg *config.Config) { fs.BoolVar(&ic.NeedFix, "fix", false, wh("Fix found issues (if it's supported by the linter)")) } -func (e *Executor) getConfigForCommandLine() (*config.Config, error) { - // We use another pflag.FlagSet here to not set `changed` flag - // on cmd.Flags() options. Otherwise, string slice options will be duplicated. - fs := pflag.NewFlagSet("config flag set", pflag.ContinueOnError) - - var cfg config.Config - // Don't do `fs.AddFlagSet(cmd.Flags())` because it shares flags representations: - // `changed` variable inside string slice vars will be shared. - // Use another config variable here, not e.cfg, to not - // affect main parsing by this parsing of only config option. - e.initRunFlagSet(fs, &cfg) - initVersionFlagSet(fs, &cfg) - - // Parse max options, even force version option: don't want - // to get access to Executor here: it's error-prone to use - // cfg vs e.cfg. - initRootFlagSet(fs, &cfg, true) - - fs.Usage = func() {} // otherwise, help text will be printed twice - if err := fs.Parse(os.Args); err != nil { - if errors.Is(err, pflag.ErrHelp) { - return nil, err - } - - return nil, fmt.Errorf("can't parse args: %w", err) - } - - return &cfg, nil -} - // runAnalysis executes the linters that have been enabled in the configuration. func (e *Executor) runAnalysis(ctx context.Context, args []string) ([]result.Issue, error) { e.cfg.Run.Args = args From eaecfbd087c2d7d6a59b22565d5e7120787e468f Mon Sep 17 00:00:00 2001 From: Fernandez Ludovic Date: Tue, 20 Feb 2024 04:15:10 +0100 Subject: [PATCH 13/45] chore: converts FlagSet methods to functions --- pkg/commands/executor.go | 2 +- pkg/commands/linters.go | 24 +++--- pkg/commands/run.go | 162 +++++++++++++++++++-------------------- 3 files changed, 94 insertions(+), 94 deletions(-) diff --git a/pkg/commands/executor.go b/pkg/commands/executor.go index 002f8e82373a..68fbe4d1c387 100644 --- a/pkg/commands/executor.go +++ b/pkg/commands/executor.go @@ -156,7 +156,7 @@ func (e *Executor) getConfigForCommandLine() (*config.Config, error) { // `changed` variable inside string slice vars will be shared. // Use another config variable here, not e.cfg, to not // affect main parsing by this parsing of only config option. - e.initRunFlagSet(fs, &cfg) + initRunFlagSet(fs, &cfg) initVersionFlagSet(fs, &cfg) // Parse max options, even force version option: don't want diff --git a/pkg/commands/linters.go b/pkg/commands/linters.go index 6f47193ceea9..acc22450e2c8 100644 --- a/pkg/commands/linters.go +++ b/pkg/commands/linters.go @@ -26,22 +26,11 @@ func (e *Executor) initLinters() { fs.SortFlags = false // sort them as they are defined here initConfigFileFlagSet(fs, &e.cfg.Run) - e.initLintersFlagSet(fs, &e.cfg.Linters) + initLintersFlagSet(fs, &e.cfg.Linters) e.rootCmd.AddCommand(e.lintersCmd) } -func (e *Executor) initLintersFlagSet(fs *pflag.FlagSet, cfg *config.Linters) { - fs.StringSliceVarP(&cfg.Disable, "disable", "D", nil, wh("Disable specific linter")) - fs.BoolVar(&cfg.DisableAll, "disable-all", false, wh("Disable all linters")) - fs.StringSliceVarP(&cfg.Enable, "enable", "E", nil, wh("Enable specific linter")) - fs.BoolVar(&cfg.EnableAll, "enable-all", false, wh("Enable all linters")) - fs.BoolVar(&cfg.Fast, "fast", false, wh("Enable only fast linters from enabled linters set (first run won't be fast)")) - fs.StringSliceVarP(&cfg.Presets, "presets", "p", nil, - wh(fmt.Sprintf("Enable presets (%s) of linters. Run 'golangci-lint help linters' to see "+ - "them. This option implies option --disable-all", strings.Join(lintersdb.AllPresets(), "|")))) -} - // executeLinters runs the 'linters' CLI command, which displays the supported linters. func (e *Executor) executeLinters(_ *cobra.Command, _ []string) error { enabledLintersMap, err := e.enabledLintersSet.GetEnabledLintersMap() @@ -71,3 +60,14 @@ func (e *Executor) executeLinters(_ *cobra.Command, _ []string) error { return nil } + +func initLintersFlagSet(fs *pflag.FlagSet, cfg *config.Linters) { + fs.StringSliceVarP(&cfg.Disable, "disable", "D", nil, wh("Disable specific linter")) + fs.BoolVar(&cfg.DisableAll, "disable-all", false, wh("Disable all linters")) + fs.StringSliceVarP(&cfg.Enable, "enable", "E", nil, wh("Enable specific linter")) + fs.BoolVar(&cfg.EnableAll, "enable-all", false, wh("Enable all linters")) + fs.BoolVar(&cfg.Fast, "fast", false, wh("Enable only fast linters from enabled linters set (first run won't be fast)")) + fs.StringSliceVarP(&cfg.Presets, "presets", "p", nil, + wh(fmt.Sprintf("Enable presets (%s) of linters. Run 'golangci-lint help linters' to see "+ + "them. This option implies option --disable-all", strings.Join(lintersdb.AllPresets(), "|")))) +} diff --git a/pkg/commands/run.go b/pkg/commands/run.go index 2d16bbc218b3..16f734817d6c 100644 --- a/pkg/commands/run.go +++ b/pkg/commands/run.go @@ -66,87 +66,7 @@ func (e *Executor) initRun() { func (e *Executor) initRunConfiguration(cmd *cobra.Command) { fs := cmd.Flags() fs.SortFlags = false // sort them as they are defined here - e.initRunFlagSet(fs, e.cfg) -} - -//nolint:gomnd -func (e *Executor) initRunFlagSet(fs *pflag.FlagSet, cfg *config.Config) { - // Config file config - rc := &cfg.Run - initConfigFileFlagSet(fs, rc) - - // Output config - oc := &cfg.Output - fs.StringVar(&oc.Format, "out-format", - config.OutFormatColoredLineNumber, - wh(fmt.Sprintf("Format of output: %s", strings.Join(config.OutFormats, "|")))) - fs.BoolVar(&oc.PrintIssuedLine, "print-issued-lines", true, wh("Print lines of code with issue")) - fs.BoolVar(&oc.PrintLinterName, "print-linter-name", true, wh("Print linter name in issue line")) - fs.BoolVar(&oc.UniqByLine, "uniq-by-line", true, wh("Make issues output unique by line")) - fs.BoolVar(&oc.SortResults, "sort-results", false, wh("Sort linter results")) - fs.BoolVar(&oc.PrintWelcomeMessage, "print-welcome", false, wh("Print welcome message")) - fs.StringVar(&oc.PathPrefix, "path-prefix", "", wh("Path prefix to add to output")) - - fs.BoolVar(&cfg.InternalCmdTest, "internal-cmd-test", false, wh("Option is used only for testing golangci-lint command, don't use it")) - if err := fs.MarkHidden("internal-cmd-test"); err != nil { - panic(err) - } - - // Run config - fs.StringVar(&rc.ModulesDownloadMode, "modules-download-mode", "", - wh("Modules download mode. If not empty, passed as -mod= to go tools")) - fs.IntVar(&rc.ExitCodeIfIssuesFound, "issues-exit-code", - exitcodes.IssuesFound, wh("Exit code when issues were found")) - fs.StringVar(&rc.Go, "go", "", wh("Targeted Go version")) - fs.StringSliceVar(&rc.BuildTags, "build-tags", nil, wh("Build tags")) - - fs.DurationVar(&rc.Timeout, "timeout", defaultTimeout, wh("Timeout for total work")) - - fs.BoolVar(&rc.AnalyzeTests, "tests", true, wh("Analyze tests (*_test.go)")) - fs.BoolVar(&rc.PrintResourcesUsage, "print-resources-usage", false, - wh("Print avg and max memory usage of golangci-lint and total time")) - fs.StringSliceVar(&rc.SkipDirs, "skip-dirs", nil, wh("Regexps of directories to skip")) - fs.BoolVar(&rc.UseDefaultSkipDirs, "skip-dirs-use-default", true, getDefaultDirectoryExcludeHelp()) - fs.StringSliceVar(&rc.SkipFiles, "skip-files", nil, wh("Regexps of files to skip")) - - const allowParallelDesc = "Allow multiple parallel golangci-lint instances running. " + - "If false (default) - golangci-lint acquires file lock on start." - fs.BoolVar(&rc.AllowParallelRunners, "allow-parallel-runners", false, wh(allowParallelDesc)) - const allowSerialDesc = "Allow multiple golangci-lint instances running, but serialize them around a lock. " + - "If false (default) - golangci-lint exits with an error if it fails to acquire file lock on start." - fs.BoolVar(&rc.AllowSerialRunners, "allow-serial-runners", false, wh(allowSerialDesc)) - fs.BoolVar(&rc.ShowStats, "show-stats", false, wh("Show statistics per linter")) - - // Linters config - lc := &cfg.Linters - e.initLintersFlagSet(fs, lc) - - // Issues config - ic := &cfg.Issues - fs.StringSliceVarP(&ic.ExcludePatterns, "exclude", "e", nil, wh("Exclude issue by regexp")) - fs.BoolVar(&ic.UseDefaultExcludes, "exclude-use-default", true, getDefaultIssueExcludeHelp()) - fs.BoolVar(&ic.ExcludeCaseSensitive, "exclude-case-sensitive", false, wh("If set to true exclude "+ - "and exclude rules regular expressions are case sensitive")) - - fs.IntVar(&ic.MaxIssuesPerLinter, "max-issues-per-linter", 50, - wh("Maximum issues count per one linter. Set to 0 to disable")) - fs.IntVar(&ic.MaxSameIssues, "max-same-issues", 3, - wh("Maximum count of issues with the same text. Set to 0 to disable")) - - fs.BoolVarP(&ic.Diff, "new", "n", false, - wh("Show only new issues: if there are unstaged changes or untracked files, only those changes "+ - "are analyzed, else only changes in HEAD~ are analyzed.\nIt's a super-useful option for integration "+ - "of golangci-lint into existing large codebase.\nIt's not practical to fix all existing issues at "+ - "the moment of integration: much better to not allow issues in new code.\nFor CI setups, prefer "+ - "--new-from-rev=HEAD~, as --new can skip linting the current patch if any scripts generate "+ - "unstaged files before golangci-lint runs.")) - fs.StringVar(&ic.DiffFromRevision, "new-from-rev", "", - wh("Show only new issues created after git revision `REV`")) - fs.StringVar(&ic.DiffPatchFilePath, "new-from-patch", "", - wh("Show only new issues created in git patch with file path `PATH`")) - fs.BoolVar(&ic.WholeFiles, "whole-files", false, - wh("Show issues in any part of update files (requires new-from-rev or new-from-patch)")) - fs.BoolVar(&ic.NeedFix, "fix", false, wh("Fix found issues (if it's supported by the linter)")) + initRunFlagSet(fs, e.cfg) } // runAnalysis executes the linters that have been enabled in the configuration. @@ -436,6 +356,86 @@ func (e *Executor) releaseFileLock() { } } +//nolint:gomnd +func initRunFlagSet(fs *pflag.FlagSet, cfg *config.Config) { + // Config file config + rc := &cfg.Run + initConfigFileFlagSet(fs, rc) + + // Output config + oc := &cfg.Output + fs.StringVar(&oc.Format, "out-format", + config.OutFormatColoredLineNumber, + wh(fmt.Sprintf("Format of output: %s", strings.Join(config.OutFormats, "|")))) + fs.BoolVar(&oc.PrintIssuedLine, "print-issued-lines", true, wh("Print lines of code with issue")) + fs.BoolVar(&oc.PrintLinterName, "print-linter-name", true, wh("Print linter name in issue line")) + fs.BoolVar(&oc.UniqByLine, "uniq-by-line", true, wh("Make issues output unique by line")) + fs.BoolVar(&oc.SortResults, "sort-results", false, wh("Sort linter results")) + fs.BoolVar(&oc.PrintWelcomeMessage, "print-welcome", false, wh("Print welcome message")) + fs.StringVar(&oc.PathPrefix, "path-prefix", "", wh("Path prefix to add to output")) + + fs.BoolVar(&cfg.InternalCmdTest, "internal-cmd-test", false, wh("Option is used only for testing golangci-lint command, don't use it")) + if err := fs.MarkHidden("internal-cmd-test"); err != nil { + panic(err) + } + + // Run config + fs.StringVar(&rc.ModulesDownloadMode, "modules-download-mode", "", + wh("Modules download mode. If not empty, passed as -mod= to go tools")) + fs.IntVar(&rc.ExitCodeIfIssuesFound, "issues-exit-code", + exitcodes.IssuesFound, wh("Exit code when issues were found")) + fs.StringVar(&rc.Go, "go", "", wh("Targeted Go version")) + fs.StringSliceVar(&rc.BuildTags, "build-tags", nil, wh("Build tags")) + + fs.DurationVar(&rc.Timeout, "timeout", defaultTimeout, wh("Timeout for total work")) + + fs.BoolVar(&rc.AnalyzeTests, "tests", true, wh("Analyze tests (*_test.go)")) + fs.BoolVar(&rc.PrintResourcesUsage, "print-resources-usage", false, + wh("Print avg and max memory usage of golangci-lint and total time")) + fs.StringSliceVar(&rc.SkipDirs, "skip-dirs", nil, wh("Regexps of directories to skip")) + fs.BoolVar(&rc.UseDefaultSkipDirs, "skip-dirs-use-default", true, getDefaultDirectoryExcludeHelp()) + fs.StringSliceVar(&rc.SkipFiles, "skip-files", nil, wh("Regexps of files to skip")) + + const allowParallelDesc = "Allow multiple parallel golangci-lint instances running. " + + "If false (default) - golangci-lint acquires file lock on start." + fs.BoolVar(&rc.AllowParallelRunners, "allow-parallel-runners", false, wh(allowParallelDesc)) + const allowSerialDesc = "Allow multiple golangci-lint instances running, but serialize them around a lock. " + + "If false (default) - golangci-lint exits with an error if it fails to acquire file lock on start." + fs.BoolVar(&rc.AllowSerialRunners, "allow-serial-runners", false, wh(allowSerialDesc)) + fs.BoolVar(&rc.ShowStats, "show-stats", false, wh("Show statistics per linter")) + + // Linters config + lc := &cfg.Linters + initLintersFlagSet(fs, lc) + + // Issues config + ic := &cfg.Issues + fs.StringSliceVarP(&ic.ExcludePatterns, "exclude", "e", nil, wh("Exclude issue by regexp")) + fs.BoolVar(&ic.UseDefaultExcludes, "exclude-use-default", true, getDefaultIssueExcludeHelp()) + fs.BoolVar(&ic.ExcludeCaseSensitive, "exclude-case-sensitive", false, wh("If set to true exclude "+ + "and exclude rules regular expressions are case sensitive")) + + fs.IntVar(&ic.MaxIssuesPerLinter, "max-issues-per-linter", 50, + wh("Maximum issues count per one linter. Set to 0 to disable")) + fs.IntVar(&ic.MaxSameIssues, "max-same-issues", 3, + wh("Maximum count of issues with the same text. Set to 0 to disable")) + + fs.BoolVarP(&ic.Diff, "new", "n", false, + wh("Show only new issues: if there are unstaged changes or untracked files, only those changes "+ + "are analyzed, else only changes in HEAD~ are analyzed.\nIt's a super-useful option for integration "+ + "of golangci-lint into existing large codebase.\nIt's not practical to fix all existing issues at "+ + "the moment of integration: much better to not allow issues in new code.\nFor CI setups, prefer "+ + "--new-from-rev=HEAD~, as --new can skip linting the current patch if any scripts generate "+ + "unstaged files before golangci-lint runs.")) + fs.StringVar(&ic.DiffFromRevision, "new-from-rev", "", + wh("Show only new issues created after git revision `REV`")) + fs.StringVar(&ic.DiffPatchFilePath, "new-from-patch", "", + wh("Show only new issues created in git patch with file path `PATH`")) + fs.BoolVar(&ic.WholeFiles, "whole-files", false, + wh("Show issues in any part of update files (requires new-from-rev or new-from-patch)")) + fs.BoolVar(&ic.NeedFix, "fix", false, wh("Fix found issues (if it's supported by the linter)")) +} + func watchResources(ctx context.Context, done chan struct{}, logger logutils.Log, debugf logutils.DebugFunc) { startedAt := time.Now() debugf("Started tracking time") From 0bfebae54313c7a2265816f0f4fbfb00cc85ea87 Mon Sep 17 00:00:00 2001 From: Fernandez Ludovic Date: Tue, 20 Feb 2024 04:16:48 +0100 Subject: [PATCH 14/45] chore: linarize initVersionConfiguration --- pkg/commands/version.go | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/pkg/commands/version.go b/pkg/commands/version.go index 1978bb2923bf..44fa8e576fe9 100644 --- a/pkg/commands/version.go +++ b/pkg/commands/version.go @@ -67,11 +67,8 @@ func (e *Executor) initVersion() { } e.rootCmd.AddCommand(versionCmd) - e.initVersionConfiguration(versionCmd) -} -func (e *Executor) initVersionConfiguration(cmd *cobra.Command) { - fs := cmd.Flags() + fs := versionCmd.Flags() fs.SortFlags = false // sort them as they are defined here initVersionFlagSet(fs, e.cfg) } From 41570487047c689f6a8eed62999fd35566f9cca4 Mon Sep 17 00:00:00 2001 From: Fernandez Ludovic Date: Tue, 20 Feb 2024 04:18:58 +0100 Subject: [PATCH 15/45] chore: reformat some comments --- pkg/commands/config.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/pkg/commands/config.go b/pkg/commands/config.go index 45c4fcd77cfd..573033e75125 100644 --- a/pkg/commands/config.go +++ b/pkg/commands/config.go @@ -40,8 +40,8 @@ func (e *Executor) initConfig() { cmd.AddCommand(pathCmd) } -// getUsedConfig returns the resolved path to the golangci config file, or the empty string -// if no configuration could be found. +// getUsedConfig returns the resolved path to the golangci config file, +// or the empty string if no configuration could be found. func (e *Executor) getUsedConfig() string { usedConfigFile := viper.ConfigFileUsed() if usedConfigFile == "" { From d1d6731969182122c1e3dc41c676525521ee3638 Mon Sep 17 00:00:00 2001 From: Fernandez Ludovic Date: Tue, 20 Feb 2024 04:23:38 +0100 Subject: [PATCH 16/45] chore: isolate initHashSalt --- pkg/commands/cache.go | 24 +++++++++++++----------- 1 file changed, 13 insertions(+), 11 deletions(-) diff --git a/pkg/commands/cache.go b/pkg/commands/cache.go index a9fb4a2b5c08..3774b95d501a 100644 --- a/pkg/commands/cache.go +++ b/pkg/commands/cache.go @@ -66,6 +66,19 @@ func (e *Executor) executeCacheStatus(_ *cobra.Command, _ []string) { } } +func dirSizeBytes(path string) (int64, error) { + var size int64 + err := filepath.Walk(path, func(_ string, info os.FileInfo, err error) error { + if err == nil && !info.IsDir() { + size += info.Size() + } + return err + }) + return size, err +} + +// --- Related to cache but not used directly by the cache command. + func (e *Executor) initHashSalt(version string) error { binSalt, err := computeBinarySalt(version) if err != nil { @@ -128,14 +141,3 @@ func computeConfigSalt(cfg *config.Config) ([]byte, error) { } return h.Sum(nil), nil } - -func dirSizeBytes(path string) (int64, error) { - var size int64 - err := filepath.Walk(path, func(_ string, info os.FileInfo, err error) error { - if err == nil && !info.IsDir() { - size += info.Size() - } - return err - }) - return size, err -} From d938daefe5415c4b493b075ba34c70cb74231d31 Mon Sep 17 00:00:00 2001 From: Fernandez Ludovic Date: Tue, 20 Feb 2024 04:26:05 +0100 Subject: [PATCH 17/45] chore: isolate init methods --- pkg/commands/executor.go | 20 ++++++++++++-------- 1 file changed, 12 insertions(+), 8 deletions(-) diff --git a/pkg/commands/executor.go b/pkg/commands/executor.go index 68fbe4d1c387..b8923650b25e 100644 --- a/pkg/commands/executor.go +++ b/pkg/commands/executor.go @@ -51,7 +51,6 @@ type Executor struct { // NewExecutor creates and initializes a new command executor. func NewExecutor(buildInfo BuildInfo) *Executor { - startedAt := time.Now() e := &Executor{ cfg: config.NewDefault(), buildInfo: buildInfo, @@ -59,6 +58,7 @@ func NewExecutor(buildInfo BuildInfo) *Executor { debugf: logutils.Debug(logutils.DebugKeyExec), } + startedAt := time.Now() e.debugf("Starting execution...") e.log = report.NewLogWrapper(logutils.NewStderrLog(logutils.DebugKeyEmpty), &e.reportData) @@ -83,13 +83,7 @@ func NewExecutor(buildInfo BuildInfo) *Executor { } // init of commands must be done before config file reading because init sets config with the default values of flags. - e.initRoot() - e.initRun() - e.initHelp() - e.initLinters() - e.initConfig() - e.initVersion() - e.initCache() + e.initCommands() // init e.cfg by values from config: flags parse will see these values like the default ones. // It will overwrite them only if the same option is found in command-line: it's ok, command-line has higher priority. @@ -142,6 +136,16 @@ func NewExecutor(buildInfo BuildInfo) *Executor { return e } +func (e *Executor) initCommands() { + e.initRoot() + e.initRun() + e.initHelp() + e.initLinters() + e.initConfig() + e.initVersion() + e.initCache() +} + func (e *Executor) Execute() error { return e.rootCmd.Execute() } From f8ff68d9a428158b9d93012b80b2f8dfb123c742 Mon Sep 17 00:00:00 2001 From: Fernandez Ludovic Date: Tue, 20 Feb 2024 04:28:07 +0100 Subject: [PATCH 18/45] chore: remove first call to lintersdb.NewManager from NewExecutor Since the presets are not linked to the Executor, this first call is useless. --- pkg/commands/executor.go | 1 - 1 file changed, 1 deletion(-) diff --git a/pkg/commands/executor.go b/pkg/commands/executor.go index b8923650b25e..657f9cc681c9 100644 --- a/pkg/commands/executor.go +++ b/pkg/commands/executor.go @@ -54,7 +54,6 @@ func NewExecutor(buildInfo BuildInfo) *Executor { e := &Executor{ cfg: config.NewDefault(), buildInfo: buildInfo, - dbManager: lintersdb.NewManager(nil, nil), debugf: logutils.Debug(logutils.DebugKeyExec), } From 2b271d2ba72c99120413daead70420e19917beb6 Mon Sep 17 00:00:00 2001 From: Fernandez Ludovic Date: Tue, 20 Feb 2024 04:30:06 +0100 Subject: [PATCH 19/45] chore: move initCommands at the begining of NewExecutor --- pkg/commands/executor.go | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/pkg/commands/executor.go b/pkg/commands/executor.go index 657f9cc681c9..209654a90151 100644 --- a/pkg/commands/executor.go +++ b/pkg/commands/executor.go @@ -57,6 +57,9 @@ func NewExecutor(buildInfo BuildInfo) *Executor { debugf: logutils.Debug(logutils.DebugKeyExec), } + // init of commands must be done before config file reading because init sets config with the default values of flags. + e.initCommands() + startedAt := time.Now() e.debugf("Starting execution...") e.log = report.NewLogWrapper(logutils.NewStderrLog(logutils.DebugKeyEmpty), &e.reportData) @@ -81,9 +84,6 @@ func NewExecutor(buildInfo BuildInfo) *Executor { } } - // init of commands must be done before config file reading because init sets config with the default values of flags. - e.initCommands() - // init e.cfg by values from config: flags parse will see these values like the default ones. // It will overwrite them only if the same option is found in command-line: it's ok, command-line has higher priority. From 34269cbe187b9a3a4436b3db028864de019a0a37 Mon Sep 17 00:00:00 2001 From: Fernandez Ludovic Date: Tue, 20 Feb 2024 04:32:34 +0100 Subject: [PATCH 20/45] chore: isolate Executor initialization code --- pkg/commands/executor.go | 29 +++++++++++++++++------------ 1 file changed, 17 insertions(+), 12 deletions(-) diff --git a/pkg/commands/executor.go b/pkg/commands/executor.go index 209654a90151..b68099d62fd5 100644 --- a/pkg/commands/executor.go +++ b/pkg/commands/executor.go @@ -60,6 +60,22 @@ func NewExecutor(buildInfo BuildInfo) *Executor { // init of commands must be done before config file reading because init sets config with the default values of flags. e.initCommands() + e.initExecutor() + + return e +} + +func (e *Executor) initCommands() { + e.initRoot() + e.initRun() + e.initHelp() + e.initLinters() + e.initConfig() + e.initVersion() + e.initCache() +} + +func (e *Executor) initExecutor() { startedAt := time.Now() e.debugf("Starting execution...") e.log = report.NewLogWrapper(logutils.NewStderrLog(logutils.DebugKeyEmpty), &e.reportData) @@ -128,21 +144,10 @@ func NewExecutor(buildInfo BuildInfo) *Executor { e.loadGuard = load.NewGuard() e.contextLoader = lint.NewContextLoader(e.cfg, e.log.Child(logutils.DebugKeyLoader), e.goenv, e.lineCache, e.fileCache, e.pkgCache, e.loadGuard) - if err = e.initHashSalt(buildInfo.Version); err != nil { + if err = e.initHashSalt(e.buildInfo.Version); err != nil { e.log.Fatalf("Failed to init hash salt: %s", err) } e.debugf("Initialized executor in %s", time.Since(startedAt)) - return e -} - -func (e *Executor) initCommands() { - e.initRoot() - e.initRun() - e.initHelp() - e.initLinters() - e.initConfig() - e.initVersion() - e.initCache() } func (e *Executor) Execute() error { From ea79bbc7c59baadc110dbc36580d7f52ff9c8884 Mon Sep 17 00:00:00 2001 From: Fernandez Ludovic Date: Tue, 20 Feb 2024 04:48:21 +0100 Subject: [PATCH 21/45] chore: fix imports --- pkg/commands/help.go | 2 +- pkg/commands/linters.go | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/pkg/commands/help.go b/pkg/commands/help.go index 0d4519c0867f..2105942394bb 100644 --- a/pkg/commands/help.go +++ b/pkg/commands/help.go @@ -6,10 +6,10 @@ import ( "strings" "github.com/fatih/color" - "github.com/golangci/golangci-lint/pkg/lint/lintersdb" "github.com/spf13/cobra" "github.com/golangci/golangci-lint/pkg/lint/linter" + "github.com/golangci/golangci-lint/pkg/lint/lintersdb" "github.com/golangci/golangci-lint/pkg/logutils" ) diff --git a/pkg/commands/linters.go b/pkg/commands/linters.go index acc22450e2c8..cb1466ef7ca3 100644 --- a/pkg/commands/linters.go +++ b/pkg/commands/linters.go @@ -5,12 +5,12 @@ import ( "strings" "github.com/fatih/color" - "github.com/golangci/golangci-lint/pkg/lint/lintersdb" "github.com/spf13/cobra" "github.com/spf13/pflag" "github.com/golangci/golangci-lint/pkg/config" "github.com/golangci/golangci-lint/pkg/lint/linter" + "github.com/golangci/golangci-lint/pkg/lint/lintersdb" ) func (e *Executor) initLinters() { From fae562cde073aadc8a94709e4c0753e7bd7343e6 Mon Sep 17 00:00:00 2001 From: Fernandez Ludovic Date: Tue, 20 Feb 2024 05:02:56 +0100 Subject: [PATCH 22/45] chore: reorganize initRunFlagSet --- pkg/commands/run.go | 27 ++++++++++++++++----------- 1 file changed, 16 insertions(+), 11 deletions(-) diff --git a/pkg/commands/run.go b/pkg/commands/run.go index 16f734817d6c..4d42b3226e8d 100644 --- a/pkg/commands/run.go +++ b/pkg/commands/run.go @@ -358,11 +358,13 @@ func (e *Executor) releaseFileLock() { //nolint:gomnd func initRunFlagSet(fs *pflag.FlagSet, cfg *config.Config) { - // Config file config - rc := &cfg.Run - initConfigFileFlagSet(fs, rc) + fs.BoolVar(&cfg.InternalCmdTest, "internal-cmd-test", false, wh("Option is used only for testing golangci-lint command, don't use it")) + if err := fs.MarkHidden("internal-cmd-test"); err != nil { + panic(err) + } + + // --- Output config - // Output config oc := &cfg.Output fs.StringVar(&oc.Format, "out-format", config.OutFormatColoredLineNumber, @@ -374,12 +376,13 @@ func initRunFlagSet(fs *pflag.FlagSet, cfg *config.Config) { fs.BoolVar(&oc.PrintWelcomeMessage, "print-welcome", false, wh("Print welcome message")) fs.StringVar(&oc.PathPrefix, "path-prefix", "", wh("Path prefix to add to output")) - fs.BoolVar(&cfg.InternalCmdTest, "internal-cmd-test", false, wh("Option is used only for testing golangci-lint command, don't use it")) - if err := fs.MarkHidden("internal-cmd-test"); err != nil { - panic(err) - } + // --- Run config + + rc := &cfg.Run + + // Config file config + initConfigFileFlagSet(fs, rc) - // Run config fs.StringVar(&rc.ModulesDownloadMode, "modules-download-mode", "", wh("Modules download mode. If not empty, passed as -mod= to go tools")) fs.IntVar(&rc.ExitCodeIfIssuesFound, "issues-exit-code", @@ -404,11 +407,13 @@ func initRunFlagSet(fs *pflag.FlagSet, cfg *config.Config) { fs.BoolVar(&rc.AllowSerialRunners, "allow-serial-runners", false, wh(allowSerialDesc)) fs.BoolVar(&rc.ShowStats, "show-stats", false, wh("Show statistics per linter")) - // Linters config + // --- Linters config + lc := &cfg.Linters initLintersFlagSet(fs, lc) - // Issues config + // --- Issues config + ic := &cfg.Issues fs.StringSliceVarP(&ic.ExcludePatterns, "exclude", "e", nil, wh("Exclude issue by regexp")) fs.BoolVar(&ic.UseDefaultExcludes, "exclude-use-default", true, getDefaultIssueExcludeHelp()) From 7cc7c45db5086c639e3c072661eaa5590b31ae0c Mon Sep 17 00:00:00 2001 From: Fernandez Ludovic Date: Tue, 20 Feb 2024 13:58:44 +0100 Subject: [PATCH 23/45] chore: remove useless needVersionOption method --- pkg/commands/executor.go | 6 +++--- pkg/commands/root.go | 12 +++--------- 2 files changed, 6 insertions(+), 12 deletions(-) diff --git a/pkg/commands/executor.go b/pkg/commands/executor.go index b68099d62fd5..6c522a15b0fa 100644 --- a/pkg/commands/executor.go +++ b/pkg/commands/executor.go @@ -81,7 +81,7 @@ func (e *Executor) initExecutor() { e.log = report.NewLogWrapper(logutils.NewStderrLog(logutils.DebugKeyEmpty), &e.reportData) // to set up log level early we need to parse config from command line extra time to find `-v` option. - commandLineCfg, err := e.getConfigForCommandLine() + commandLineCfg, err := getConfigForCommandLine() if err != nil && !errors.Is(err, pflag.ErrHelp) { e.log.Fatalf("Can't get config for command line: %s", err) } @@ -154,7 +154,7 @@ func (e *Executor) Execute() error { return e.rootCmd.Execute() } -func (e *Executor) getConfigForCommandLine() (*config.Config, error) { +func getConfigForCommandLine() (*config.Config, error) { // We use another pflag.FlagSet here to not set `changed` flag // on cmd.Flags() options. Otherwise, string slice options will be duplicated. fs := pflag.NewFlagSet("config flag set", pflag.ContinueOnError) @@ -170,7 +170,7 @@ func (e *Executor) getConfigForCommandLine() (*config.Config, error) { // Parse max options, even force version option: don't want // to get access to Executor here: it's error-prone to use // cfg vs e.cfg. - initRootFlagSet(fs, &cfg, true) + initRootFlagSet(fs, &cfg) fs.Usage = func() {} // otherwise, help text will be printed twice if err := fs.Parse(os.Args); err != nil { diff --git a/pkg/commands/root.go b/pkg/commands/root.go index fa90545709f6..392737d24795 100644 --- a/pkg/commands/root.go +++ b/pkg/commands/root.go @@ -35,7 +35,7 @@ func (e *Executor) initRoot() { PersistentPostRunE: e.persistentPostRun, } - initRootFlagSet(rootCmd.PersistentFlags(), e.cfg, e.needVersionOption()) + initRootFlagSet(rootCmd.PersistentFlags(), e.cfg) e.rootCmd = rootCmd } @@ -106,11 +106,7 @@ func (e *Executor) persistentPostRun(_ *cobra.Command, _ []string) error { return nil } -func (e *Executor) needVersionOption() bool { - return e.buildInfo.Date != "" -} - -func initRootFlagSet(fs *pflag.FlagSet, cfg *config.Config, needVersionOption bool) { +func initRootFlagSet(fs *pflag.FlagSet, cfg *config.Config) { fs.BoolVarP(&cfg.Run.IsVerbose, "verbose", "v", false, wh("Verbose output")) fs.StringVar(&cfg.Output.Color, "color", "auto", wh("Use color when printing; can be 'always', 'auto', or 'never'")) @@ -121,9 +117,7 @@ func initRootFlagSet(fs *pflag.FlagSet, cfg *config.Config, needVersionOption bo fs.IntVarP(&cfg.Run.Concurrency, "concurrency", "j", getDefaultConcurrency(), wh("Number of CPUs to use (Default: number of logical CPUs)")) - if needVersionOption { - fs.BoolVar(&cfg.Run.PrintVersion, "version", false, wh("Print version")) - } + fs.BoolVar(&cfg.Run.PrintVersion, "version", false, wh("Print version")) } func printMemStats(ms *runtime.MemStats, logger logutils.Log) { From dfa6eb8cc4e10222a390d3718a15b9198607a14b Mon Sep 17 00:00:00 2001 From: Fernandez Ludovic Date: Tue, 20 Feb 2024 14:44:16 +0100 Subject: [PATCH 24/45] chore: inline help linters command --- pkg/commands/help.go | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/pkg/commands/help.go b/pkg/commands/help.go index 2105942394bb..fe530f86c72f 100644 --- a/pkg/commands/help.go +++ b/pkg/commands/help.go @@ -24,14 +24,13 @@ func (e *Executor) initHelp() { } e.rootCmd.SetHelpCommand(helpCmd) - lintersHelpCmd := &cobra.Command{ + helpCmd.AddCommand(&cobra.Command{ Use: "linters", Short: "Help about linters", Args: cobra.NoArgs, ValidArgsFunction: cobra.NoFileCompletions, Run: e.executeLintersHelp, - } - helpCmd.AddCommand(lintersHelpCmd) + }) } func (e *Executor) executeLintersHelp(_ *cobra.Command, _ []string) { From 13682c232aabc25b7a8fef5b607ffa06e2619bac Mon Sep 17 00:00:00 2001 From: Fernandez Ludovic Date: Tue, 20 Feb 2024 14:53:33 +0100 Subject: [PATCH 25/45] chore: reorganize initXX methods --- pkg/commands/cache.go | 3 ++- pkg/commands/config.go | 6 +++--- pkg/commands/help.go | 3 ++- pkg/commands/linters.go | 8 +++++--- pkg/commands/root.go | 1 + pkg/commands/run.go | 18 +++++++++--------- pkg/commands/version.go | 5 +++-- 7 files changed, 25 insertions(+), 19 deletions(-) diff --git a/pkg/commands/cache.go b/pkg/commands/cache.go index 3774b95d501a..48f3a3cf0c48 100644 --- a/pkg/commands/cache.go +++ b/pkg/commands/cache.go @@ -27,7 +27,6 @@ func (e *Executor) initCache() { return cmd.Help() }, } - e.rootCmd.AddCommand(cacheCmd) cacheCmd.AddCommand(&cobra.Command{ Use: "clean", @@ -45,6 +44,8 @@ func (e *Executor) initCache() { }) // TODO: add trim command? + + e.rootCmd.AddCommand(cacheCmd) } func (e *Executor) executeCleanCache(_ *cobra.Command, _ []string) error { diff --git a/pkg/commands/config.go b/pkg/commands/config.go index 573033e75125..69cad42e656e 100644 --- a/pkg/commands/config.go +++ b/pkg/commands/config.go @@ -14,7 +14,7 @@ import ( ) func (e *Executor) initConfig() { - cmd := &cobra.Command{ + configCmd := &cobra.Command{ Use: "config", Short: "Config file information", Args: cobra.NoArgs, @@ -22,7 +22,6 @@ func (e *Executor) initConfig() { return cmd.Help() }, } - e.rootCmd.AddCommand(cmd) pathCmd := &cobra.Command{ Use: "path", @@ -37,7 +36,8 @@ func (e *Executor) initConfig() { initConfigFileFlagSet(fs, &e.cfg.Run) - cmd.AddCommand(pathCmd) + configCmd.AddCommand(pathCmd) + e.rootCmd.AddCommand(configCmd) } // getUsedConfig returns the resolved path to the golangci config file, diff --git a/pkg/commands/help.go b/pkg/commands/help.go index fe530f86c72f..3241f22c0682 100644 --- a/pkg/commands/help.go +++ b/pkg/commands/help.go @@ -22,7 +22,6 @@ func (e *Executor) initHelp() { return cmd.Help() }, } - e.rootCmd.SetHelpCommand(helpCmd) helpCmd.AddCommand(&cobra.Command{ Use: "linters", @@ -31,6 +30,8 @@ func (e *Executor) initHelp() { ValidArgsFunction: cobra.NoFileCompletions, Run: e.executeLintersHelp, }) + + e.rootCmd.SetHelpCommand(helpCmd) } func (e *Executor) executeLintersHelp(_ *cobra.Command, _ []string) { diff --git a/pkg/commands/linters.go b/pkg/commands/linters.go index cb1466ef7ca3..5ed2353fd546 100644 --- a/pkg/commands/linters.go +++ b/pkg/commands/linters.go @@ -14,7 +14,7 @@ import ( ) func (e *Executor) initLinters() { - e.lintersCmd = &cobra.Command{ + lintersCmd := &cobra.Command{ Use: "linters", Short: "List current linters configuration", Args: cobra.NoArgs, @@ -22,13 +22,15 @@ func (e *Executor) initLinters() { RunE: e.executeLinters, } - fs := e.lintersCmd.Flags() + fs := lintersCmd.Flags() fs.SortFlags = false // sort them as they are defined here initConfigFileFlagSet(fs, &e.cfg.Run) initLintersFlagSet(fs, &e.cfg.Linters) - e.rootCmd.AddCommand(e.lintersCmd) + e.rootCmd.AddCommand(lintersCmd) + + e.lintersCmd = lintersCmd } // executeLinters runs the 'linters' CLI command, which displays the supported linters. diff --git a/pkg/commands/root.go b/pkg/commands/root.go index 392737d24795..d6e87f14df2c 100644 --- a/pkg/commands/root.go +++ b/pkg/commands/root.go @@ -36,6 +36,7 @@ func (e *Executor) initRoot() { } initRootFlagSet(rootCmd.PersistentFlags(), e.cfg) + e.rootCmd = rootCmd } diff --git a/pkg/commands/run.go b/pkg/commands/run.go index 4d42b3226e8d..24e6d2f86d61 100644 --- a/pkg/commands/run.go +++ b/pkg/commands/run.go @@ -41,7 +41,7 @@ const ( ) func (e *Executor) initRun() { - e.runCmd = &cobra.Command{ + runCmd := &cobra.Command{ Use: "run", Short: "Run the linters", Run: e.executeRun, @@ -55,18 +55,18 @@ func (e *Executor) initRun() { e.releaseFileLock() }, } - e.rootCmd.AddCommand(e.runCmd) - e.runCmd.SetOut(logutils.StdOut) // use custom output to properly color it in Windows terminals - e.runCmd.SetErr(logutils.StdErr) + runCmd.SetOut(logutils.StdOut) // use custom output to properly color it in Windows terminals + runCmd.SetErr(logutils.StdErr) - e.initRunConfiguration(e.runCmd) -} - -func (e *Executor) initRunConfiguration(cmd *cobra.Command) { - fs := cmd.Flags() + fs := runCmd.Flags() fs.SortFlags = false // sort them as they are defined here + initRunFlagSet(fs, e.cfg) + + e.rootCmd.AddCommand(runCmd) + + e.runCmd = runCmd } // runAnalysis executes the linters that have been enabled in the configuration. diff --git a/pkg/commands/version.go b/pkg/commands/version.go index 44fa8e576fe9..89d29edef0a2 100644 --- a/pkg/commands/version.go +++ b/pkg/commands/version.go @@ -66,11 +66,12 @@ func (e *Executor) initVersion() { }, } - e.rootCmd.AddCommand(versionCmd) - fs := versionCmd.Flags() fs.SortFlags = false // sort them as they are defined here + initVersionFlagSet(fs, e.cfg) + + e.rootCmd.AddCommand(versionCmd) } func initVersionFlagSet(fs *pflag.FlagSet, cfg *config.Config) { From 9b6eabd8a7337397da520ab53385124b39482986 Mon Sep 17 00:00:00 2001 From: Fernandez Ludovic Date: Tue, 20 Feb 2024 15:01:02 +0100 Subject: [PATCH 26/45] chore: reorganize command methods --- pkg/commands/cache.go | 4 +- pkg/commands/config.go | 22 +++--- pkg/commands/help.go | 4 +- pkg/commands/run.go | 146 ++++++++++++++++++++-------------------- pkg/commands/version.go | 66 +++++++++--------- 5 files changed, 122 insertions(+), 120 deletions(-) diff --git a/pkg/commands/cache.go b/pkg/commands/cache.go index 48f3a3cf0c48..3a8f737862e2 100644 --- a/pkg/commands/cache.go +++ b/pkg/commands/cache.go @@ -33,7 +33,7 @@ func (e *Executor) initCache() { Short: "Clean cache", Args: cobra.NoArgs, ValidArgsFunction: cobra.NoFileCompletions, - RunE: e.executeCleanCache, + RunE: e.executeCacheClean, }) cacheCmd.AddCommand(&cobra.Command{ Use: "status", @@ -48,7 +48,7 @@ func (e *Executor) initCache() { e.rootCmd.AddCommand(cacheCmd) } -func (e *Executor) executeCleanCache(_ *cobra.Command, _ []string) error { +func (e *Executor) executeCacheClean(_ *cobra.Command, _ []string) error { cacheDir := cache.DefaultDir() if err := os.RemoveAll(cacheDir); err != nil { return fmt.Errorf("failed to remove dir %s: %w", cacheDir, err) diff --git a/pkg/commands/config.go b/pkg/commands/config.go index 69cad42e656e..2662514a6f9f 100644 --- a/pkg/commands/config.go +++ b/pkg/commands/config.go @@ -28,7 +28,7 @@ func (e *Executor) initConfig() { Short: "Print used config path", Args: cobra.NoArgs, ValidArgsFunction: cobra.NoFileCompletions, - Run: e.executePathCmd, + Run: e.executePath, } fs := pathCmd.Flags() @@ -40,6 +40,16 @@ func (e *Executor) initConfig() { e.rootCmd.AddCommand(configCmd) } +func (e *Executor) executePath(_ *cobra.Command, _ []string) { + usedConfigFile := e.getUsedConfig() + if usedConfigFile == "" { + e.log.Warnf("No config file detected") + os.Exit(exitcodes.NoConfigFileDetected) + } + + fmt.Println(usedConfigFile) +} + // getUsedConfig returns the resolved path to the golangci config file, // or the empty string if no configuration could be found. func (e *Executor) getUsedConfig() string { @@ -57,16 +67,6 @@ func (e *Executor) getUsedConfig() string { return prettyUsedConfigFile } -func (e *Executor) executePathCmd(_ *cobra.Command, _ []string) { - usedConfigFile := e.getUsedConfig() - if usedConfigFile == "" { - e.log.Warnf("No config file detected") - os.Exit(exitcodes.NoConfigFileDetected) - } - - fmt.Println(usedConfigFile) -} - func initConfigFileFlagSet(fs *pflag.FlagSet, cfg *config.Run) { fs.StringVarP(&cfg.Config, "config", "c", "", wh("Read config from file path `PATH`")) fs.BoolVar(&cfg.NoConfig, "no-config", false, wh("Don't read config file")) diff --git a/pkg/commands/help.go b/pkg/commands/help.go index 3241f22c0682..016df2c13a56 100644 --- a/pkg/commands/help.go +++ b/pkg/commands/help.go @@ -28,13 +28,13 @@ func (e *Executor) initHelp() { Short: "Help about linters", Args: cobra.NoArgs, ValidArgsFunction: cobra.NoFileCompletions, - Run: e.executeLintersHelp, + Run: e.executeHelp, }) e.rootCmd.SetHelpCommand(helpCmd) } -func (e *Executor) executeLintersHelp(_ *cobra.Command, _ []string) { +func (e *Executor) executeHelp(_ *cobra.Command, _ []string) { var enabledLCs, disabledLCs []*linter.Config for _, lc := range e.dbManager.GetAllSupportedLinterConfigs() { if lc.Internal { diff --git a/pkg/commands/run.go b/pkg/commands/run.go index 24e6d2f86d61..6af4e2863837 100644 --- a/pkg/commands/run.go +++ b/pkg/commands/run.go @@ -69,6 +69,79 @@ func (e *Executor) initRun() { e.runCmd = runCmd } +// executeRun executes the 'run' CLI command, which runs the linters. +func (e *Executor) executeRun(_ *cobra.Command, args []string) { + needTrackResources := e.cfg.Run.IsVerbose || e.cfg.Run.PrintResourcesUsage + trackResourcesEndCh := make(chan struct{}) + defer func() { // XXX: this defer must be before ctx.cancel defer + if needTrackResources { // wait until resource tracking finished to print properly + <-trackResourcesEndCh + } + }() + + ctx, cancel := context.WithTimeout(context.Background(), e.cfg.Run.Timeout) + defer cancel() + + if needTrackResources { + go watchResources(ctx, trackResourcesEndCh, e.log, e.debugf) + } + + if err := e.runAndPrint(ctx, args); err != nil { + e.log.Errorf("Running error: %s", err) + if e.exitCode == exitcodes.Success { + var exitErr *exitcodes.ExitError + if errors.As(err, &exitErr) { + e.exitCode = exitErr.Code + } else { + e.exitCode = exitcodes.Failure + } + } + } + + e.setupExitCode(ctx) +} + +func (e *Executor) runAndPrint(ctx context.Context, args []string) error { + if err := e.goenv.Discover(ctx); err != nil { + e.log.Warnf("Failed to discover go env: %s", err) + } + + if !logutils.HaveDebugTag(logutils.DebugKeyLintersOutput) { + // Don't allow linters and loader to print anything + log.SetOutput(io.Discard) + savedStdout, savedStderr := e.setOutputToDevNull() + defer func() { + os.Stdout, os.Stderr = savedStdout, savedStderr + }() + } + + issues, err := e.runAnalysis(ctx, args) + if err != nil { + return err // XXX: don't loose type + } + + formats := strings.Split(e.cfg.Output.Format, ",") + for _, format := range formats { + out := strings.SplitN(format, ":", 2) + if len(out) < 2 { + out = append(out, "") + } + + err := e.printReports(issues, out[1], out[0]) + if err != nil { + return err + } + } + + e.printStats(issues) + + e.setExitCodeIfIssuesFound(issues) + + e.fileCache.PrintStats(e.log) + + return nil +} + // runAnalysis executes the linters that have been enabled in the configuration. func (e *Executor) runAnalysis(ctx context.Context, args []string) ([]result.Issue, error) { e.cfg.Run.Args = args @@ -121,47 +194,6 @@ func (e *Executor) setExitCodeIfIssuesFound(issues []result.Issue) { } } -func (e *Executor) runAndPrint(ctx context.Context, args []string) error { - if err := e.goenv.Discover(ctx); err != nil { - e.log.Warnf("Failed to discover go env: %s", err) - } - - if !logutils.HaveDebugTag(logutils.DebugKeyLintersOutput) { - // Don't allow linters and loader to print anything - log.SetOutput(io.Discard) - savedStdout, savedStderr := e.setOutputToDevNull() - defer func() { - os.Stdout, os.Stderr = savedStdout, savedStderr - }() - } - - issues, err := e.runAnalysis(ctx, args) - if err != nil { - return err // XXX: don't loose type - } - - formats := strings.Split(e.cfg.Output.Format, ",") - for _, format := range formats { - out := strings.SplitN(format, ":", 2) - if len(out) < 2 { - out = append(out, "") - } - - err := e.printReports(issues, out[1], out[0]) - if err != nil { - return err - } - } - - e.printStats(issues) - - e.setExitCodeIfIssuesFound(issues) - - e.fileCache.PrintStats(e.log) - - return nil -} - func (e *Executor) printReports(issues []result.Issue, path, format string) error { w, shouldClose, err := e.createWriter(path) if err != nil { @@ -261,38 +293,6 @@ func (e *Executor) printStats(issues []result.Issue) { } } -// executeRun executes the 'run' CLI command, which runs the linters. -func (e *Executor) executeRun(_ *cobra.Command, args []string) { - needTrackResources := e.cfg.Run.IsVerbose || e.cfg.Run.PrintResourcesUsage - trackResourcesEndCh := make(chan struct{}) - defer func() { // XXX: this defer must be before ctx.cancel defer - if needTrackResources { // wait until resource tracking finished to print properly - <-trackResourcesEndCh - } - }() - - ctx, cancel := context.WithTimeout(context.Background(), e.cfg.Run.Timeout) - defer cancel() - - if needTrackResources { - go watchResources(ctx, trackResourcesEndCh, e.log, e.debugf) - } - - if err := e.runAndPrint(ctx, args); err != nil { - e.log.Errorf("Running error: %s", err) - if e.exitCode == exitcodes.Success { - var exitErr *exitcodes.ExitError - if errors.As(err, &exitErr) { - e.exitCode = exitErr.Code - } else { - e.exitCode = exitcodes.Failure - } - } - } - - e.setupExitCode(ctx) -} - func (e *Executor) setupExitCode(ctx context.Context) { if ctx.Err() != nil { e.exitCode = exitcodes.Timeout diff --git a/pkg/commands/version.go b/pkg/commands/version.go index 89d29edef0a2..18e5716b7e90 100644 --- a/pkg/commands/version.go +++ b/pkg/commands/version.go @@ -32,38 +32,7 @@ func (e *Executor) initVersion() { Short: "Version", Args: cobra.NoArgs, ValidArgsFunction: cobra.NoFileCompletions, - RunE: func(_ *cobra.Command, _ []string) error { - if e.cfg.Version.Debug { - info, ok := debug.ReadBuildInfo() - if !ok { - return nil - } - - switch strings.ToLower(e.cfg.Version.Format) { - case "json": - return json.NewEncoder(os.Stdout).Encode(versionInfo{ - Info: e.buildInfo, - BuildInfo: info, - }) - - default: - fmt.Println(info.String()) - return printVersion(os.Stdout, e.buildInfo) - } - } - - switch strings.ToLower(e.cfg.Version.Format) { - case "short": - fmt.Println(e.buildInfo.Version) - return nil - - case "json": - return json.NewEncoder(os.Stdout).Encode(e.buildInfo) - - default: - return printVersion(os.Stdout, e.buildInfo) - } - }, + RunE: e.executeVersion, } fs := versionCmd.Flags() @@ -74,6 +43,39 @@ func (e *Executor) initVersion() { e.rootCmd.AddCommand(versionCmd) } +func (e *Executor) executeVersion(_ *cobra.Command, _ []string) error { + if e.cfg.Version.Debug { + info, ok := debug.ReadBuildInfo() + if !ok { + return nil + } + + switch strings.ToLower(e.cfg.Version.Format) { + case "json": + return json.NewEncoder(os.Stdout).Encode(versionInfo{ + Info: e.buildInfo, + BuildInfo: info, + }) + + default: + fmt.Println(info.String()) + return printVersion(os.Stdout, e.buildInfo) + } + } + + switch strings.ToLower(e.cfg.Version.Format) { + case "short": + fmt.Println(e.buildInfo.Version) + return nil + + case "json": + return json.NewEncoder(os.Stdout).Encode(e.buildInfo) + + default: + return printVersion(os.Stdout, e.buildInfo) + } +} + func initVersionFlagSet(fs *pflag.FlagSet, cfg *config.Config) { // Version config vc := &cfg.Version From 283946f582d89dcf6fb06a4823780368959ce7de Mon Sep 17 00:00:00 2001 From: Fernandez Ludovic Date: Tue, 20 Feb 2024 15:21:54 +0100 Subject: [PATCH 27/45] chore: documment cmd fields --- pkg/commands/executor.go | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/pkg/commands/executor.go b/pkg/commands/executor.go index 6c522a15b0fa..19b687fe4c16 100644 --- a/pkg/commands/executor.go +++ b/pkg/commands/executor.go @@ -25,9 +25,10 @@ import ( ) type Executor struct { - rootCmd *cobra.Command - runCmd *cobra.Command - lintersCmd *cobra.Command + rootCmd *cobra.Command + + runCmd *cobra.Command // used by fixSlicesFlags, printStats + lintersCmd *cobra.Command // used by fixSlicesFlags exitCode int buildInfo BuildInfo From 1e8a19923c07dfe5075ba3989f62f969e15f9e22 Mon Sep 17 00:00:00 2001 From: Fernandez Ludovic Date: Tue, 20 Feb 2024 15:30:28 +0100 Subject: [PATCH 28/45] chore: remove loadGuard field --- pkg/commands/executor.go | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/pkg/commands/executor.go b/pkg/commands/executor.go index 19b687fe4c16..6c62e3ed74ca 100644 --- a/pkg/commands/executor.go +++ b/pkg/commands/executor.go @@ -46,8 +46,7 @@ type Executor struct { debugf logutils.DebugFunc sw *timeutils.Stopwatch - loadGuard *load.Guard - flock *flock.Flock + flock *flock.Flock } // NewExecutor creates and initializes a new command executor. @@ -133,21 +132,25 @@ func (e *Executor) initExecutor() { e.enabledLintersSet = lintersdb.NewEnabledSet(e.dbManager, lintersdb.NewValidator(e.dbManager), e.log.Child(logutils.DebugKeyLintersDB), e.cfg) + e.goenv = goutil.NewEnv(e.log.Child(logutils.DebugKeyGoEnv)) + e.fileCache = fsutils.NewFileCache() e.lineCache = fsutils.NewLineCache(e.fileCache) e.sw = timeutils.NewStopwatch("pkgcache", e.log.Child(logutils.DebugKeyStopwatch)) + e.pkgCache, err = pkgcache.NewCache(e.sw, e.log.Child(logutils.DebugKeyPkgCache)) if err != nil { e.log.Fatalf("Failed to build packages cache: %s", err) } - e.loadGuard = load.NewGuard() + e.contextLoader = lint.NewContextLoader(e.cfg, e.log.Child(logutils.DebugKeyLoader), e.goenv, - e.lineCache, e.fileCache, e.pkgCache, e.loadGuard) + e.lineCache, e.fileCache, e.pkgCache, load.NewGuard()) if err = e.initHashSalt(e.buildInfo.Version); err != nil { e.log.Fatalf("Failed to init hash salt: %s", err) } + e.debugf("Initialized executor in %s", time.Since(startedAt)) } From 34572a8e90c01daa5cade995240a4e0280f0678d Mon Sep 17 00:00:00 2001 From: Fernandez Ludovic Date: Tue, 20 Feb 2024 15:32:10 +0100 Subject: [PATCH 29/45] chore: group executor fields --- pkg/commands/executor.go | 28 +++++++++++++++++----------- 1 file changed, 17 insertions(+), 11 deletions(-) diff --git a/pkg/commands/executor.go b/pkg/commands/executor.go index 6c62e3ed74ca..782725011473 100644 --- a/pkg/commands/executor.go +++ b/pkg/commands/executor.go @@ -30,21 +30,27 @@ type Executor struct { runCmd *cobra.Command // used by fixSlicesFlags, printStats lintersCmd *cobra.Command // used by fixSlicesFlags - exitCode int + exitCode int + buildInfo BuildInfo - cfg *config.Config // cfg is the unmarshaled data from the golangci config file. - log logutils.Log - reportData report.Data + cfg *config.Config // cfg is the unmarshaled data from the golangci config file. + + log logutils.Log + debugf logutils.DebugFunc + reportData report.Data + dbManager *lintersdb.Manager enabledLintersSet *lintersdb.EnabledSet - contextLoader *lint.ContextLoader - goenv *goutil.Env - fileCache *fsutils.FileCache - lineCache *fsutils.LineCache - pkgCache *pkgcache.Cache - debugf logutils.DebugFunc - sw *timeutils.Stopwatch + + contextLoader *lint.ContextLoader + goenv *goutil.Env + + fileCache *fsutils.FileCache + lineCache *fsutils.LineCache + pkgCache *pkgcache.Cache + + sw *timeutils.Stopwatch flock *flock.Flock } From 18935d21f32fe337d6d8395c8e808da0b6ed5630 Mon Sep 17 00:00:00 2001 From: Fernandez Ludovic Date: Tue, 20 Feb 2024 15:53:29 +0100 Subject: [PATCH 30/45] chore: clean goutil.Env --- pkg/goutil/env.go | 15 +++++++++------ 1 file changed, 9 insertions(+), 6 deletions(-) diff --git a/pkg/goutil/env.go b/pkg/goutil/env.go index 93922f85a717..7b748d8e9031 100644 --- a/pkg/goutil/env.go +++ b/pkg/goutil/env.go @@ -33,20 +33,23 @@ func NewEnv(log logutils.Log) *Env { } } -func (e *Env) Discover(ctx context.Context) error { +func (e Env) Discover(ctx context.Context) error { startedAt := time.Now() - args := []string{"env", "-json"} - args = append(args, string(EnvGoCache), string(EnvGoRoot)) - out, err := exec.CommandContext(ctx, "go", args...).Output() + + //nolint:gosec // Everything is static here. + cmd := exec.CommandContext(ctx, "go", "env", "-json", string(EnvGoCache), string(EnvGoRoot)) + + out, err := cmd.Output() if err != nil { - return fmt.Errorf("failed to run 'go env': %w", err) + return fmt.Errorf("failed to run '%s': %w", strings.Join(cmd.Args, " "), err) } if err = json.Unmarshal(out, &e.vars); err != nil { - return fmt.Errorf("failed to parse 'go %s' json: %w", strings.Join(args, " "), err) + return fmt.Errorf("failed to parse '%s' json: %w", strings.Join(cmd.Args, " "), err) } e.debugf("Read go env for %s: %#v", time.Since(startedAt), e.vars) + return nil } From b5b2a799898b71833cc36c553e981da2b5a47c31 Mon Sep 17 00:00:00 2001 From: Fernandez Ludovic Date: Tue, 20 Feb 2024 16:01:45 +0100 Subject: [PATCH 31/45] chore: remove sw and pkgCache fields --- pkg/commands/executor.go | 9 +++------ 1 file changed, 3 insertions(+), 6 deletions(-) diff --git a/pkg/commands/executor.go b/pkg/commands/executor.go index 782725011473..9a053955ac2f 100644 --- a/pkg/commands/executor.go +++ b/pkg/commands/executor.go @@ -48,9 +48,6 @@ type Executor struct { fileCache *fsutils.FileCache lineCache *fsutils.LineCache - pkgCache *pkgcache.Cache - - sw *timeutils.Stopwatch flock *flock.Flock } @@ -144,15 +141,15 @@ func (e *Executor) initExecutor() { e.fileCache = fsutils.NewFileCache() e.lineCache = fsutils.NewLineCache(e.fileCache) - e.sw = timeutils.NewStopwatch("pkgcache", e.log.Child(logutils.DebugKeyStopwatch)) + sw := timeutils.NewStopwatch("pkgcache", e.log.Child(logutils.DebugKeyStopwatch)) - e.pkgCache, err = pkgcache.NewCache(e.sw, e.log.Child(logutils.DebugKeyPkgCache)) + pkgCache, err := pkgcache.NewCache(sw, e.log.Child(logutils.DebugKeyPkgCache)) if err != nil { e.log.Fatalf("Failed to build packages cache: %s", err) } e.contextLoader = lint.NewContextLoader(e.cfg, e.log.Child(logutils.DebugKeyLoader), e.goenv, - e.lineCache, e.fileCache, e.pkgCache, load.NewGuard()) + e.lineCache, e.fileCache, pkgCache, load.NewGuard()) if err = e.initHashSalt(e.buildInfo.Version); err != nil { e.log.Fatalf("Failed to init hash salt: %s", err) } From f470c2c38cd7ca797b8564cc8e158933374c4807 Mon Sep 17 00:00:00 2001 From: Fernandez Ludovic Date: Tue, 20 Feb 2024 16:10:42 +0100 Subject: [PATCH 32/45] chore: fixSlicesFlags before NewManager --- pkg/commands/executor.go | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/pkg/commands/executor.go b/pkg/commands/executor.go index 9a053955ac2f..edaf1106fb95 100644 --- a/pkg/commands/executor.go +++ b/pkg/commands/executor.go @@ -126,13 +126,13 @@ func (e *Executor) initExecutor() { e.cfg.Run.Go = config.DetectGoVersion() } - // recreate after getting config - e.dbManager = lintersdb.NewManager(e.cfg, e.log) - // Slice options must be explicitly set for proper merging of config and command-line options. fixSlicesFlags(e.runCmd.Flags()) fixSlicesFlags(e.lintersCmd.Flags()) + // recreate after getting config + e.dbManager = lintersdb.NewManager(e.cfg, e.log) + e.enabledLintersSet = lintersdb.NewEnabledSet(e.dbManager, lintersdb.NewValidator(e.dbManager), e.log.Child(logutils.DebugKeyLintersDB), e.cfg) From e7a19c74e2fa5f05c7d5ba14d0e84a1c1c679176 Mon Sep 17 00:00:00 2001 From: Fernandez Ludovic Date: Tue, 20 Feb 2024 16:20:18 +0100 Subject: [PATCH 33/45] chore: extract initConfiguration from initExecutor --- pkg/commands/executor.go | 19 +++++++++++-------- 1 file changed, 11 insertions(+), 8 deletions(-) diff --git a/pkg/commands/executor.go b/pkg/commands/executor.go index edaf1106fb95..20c7d572733e 100644 --- a/pkg/commands/executor.go +++ b/pkg/commands/executor.go @@ -60,11 +60,19 @@ func NewExecutor(buildInfo BuildInfo) *Executor { debugf: logutils.Debug(logutils.DebugKeyExec), } + e.log = report.NewLogWrapper(logutils.NewStderrLog(logutils.DebugKeyEmpty), &e.reportData) + // init of commands must be done before config file reading because init sets config with the default values of flags. e.initCommands() + startedAt := time.Now() + e.debugf("Starting execution...") + + e.initConfiguration() e.initExecutor() + e.debugf("Initialized executor in %s", time.Since(startedAt)) + return e } @@ -78,11 +86,7 @@ func (e *Executor) initCommands() { e.initCache() } -func (e *Executor) initExecutor() { - startedAt := time.Now() - e.debugf("Starting execution...") - e.log = report.NewLogWrapper(logutils.NewStderrLog(logutils.DebugKeyEmpty), &e.reportData) - +func (e *Executor) initConfiguration() { // to set up log level early we need to parse config from command line extra time to find `-v` option. commandLineCfg, err := getConfigForCommandLine() if err != nil && !errors.Is(err, pflag.ErrHelp) { @@ -129,8 +133,9 @@ func (e *Executor) initExecutor() { // Slice options must be explicitly set for proper merging of config and command-line options. fixSlicesFlags(e.runCmd.Flags()) fixSlicesFlags(e.lintersCmd.Flags()) +} - // recreate after getting config +func (e *Executor) initExecutor() { e.dbManager = lintersdb.NewManager(e.cfg, e.log) e.enabledLintersSet = lintersdb.NewEnabledSet(e.dbManager, @@ -153,8 +158,6 @@ func (e *Executor) initExecutor() { if err = e.initHashSalt(e.buildInfo.Version); err != nil { e.log.Fatalf("Failed to init hash salt: %s", err) } - - e.debugf("Initialized executor in %s", time.Since(startedAt)) } func (e *Executor) Execute() error { From d99541506ab1e4667f01d7404d4cf52b6f55bc0b Mon Sep 17 00:00:00 2001 From: Fernandez Ludovic Date: Tue, 20 Feb 2024 16:46:00 +0100 Subject: [PATCH 34/45] chore: remove Deadline tests --- .golangci.yml | 2 -- test/bench/bench_test.go | 2 +- test/run_test.go | 60 ++++++++-------------------------------- 3 files changed, 13 insertions(+), 51 deletions(-) diff --git a/.golangci.yml b/.golangci.yml index 214bb4600c95..b56d9fddaf81 100644 --- a/.golangci.yml +++ b/.golangci.yml @@ -141,8 +141,6 @@ issues: text: "SA1019: errCfg.Exclude is deprecated: use ExcludeFunctions instead" - path: pkg/commands/run.go text: "SA1019: lsc.Errcheck.Exclude is deprecated: use ExcludeFunctions instead" - - path: pkg/commands/run.go - text: "SA1019: e.cfg.Run.Deadline is deprecated: Deadline exists for historical compatibility and should not be used." - path: pkg/golinters/gofumpt.go text: "SA1019: settings.LangVersion is deprecated: use the global `run.go` instead." diff --git a/test/bench/bench_test.go b/test/bench/bench_test.go index a6ef3fe2184e..bd7530b3e2f3 100644 --- a/test/bench/bench_test.go +++ b/test/bench/bench_test.go @@ -76,7 +76,7 @@ func printCommand(cmd string, args ...string) { } func getGolangciLintCommonArgs() []string { - return []string{"run", "--no-config", "--issues-exit-code=0", "--deadline=30m", "--disable-all", "--enable=govet"} + return []string{"run", "--no-config", "--issues-exit-code=0", "--timeout=30m", "--disable-all", "--enable=govet"} } func runGolangciLintForBench(b testing.TB) { diff --git a/test/run_test.go b/test/run_test.go index 9fad30e13dc2..3b4870c485fc 100644 --- a/test/run_test.go +++ b/test/run_test.go @@ -54,20 +54,6 @@ func TestSymlinkLoop(t *testing.T) { ExpectNoIssues() } -// TODO(ldez): remove this in v2. -func TestDeadline(t *testing.T) { - projectRoot := filepath.Join("..", "...") - - testshared.NewRunnerBuilder(t). - WithArgs("--deadline=1ms"). - WithTargetPath(projectRoot). - Runner(). - Install(). - Run(). - ExpectExitCode(exitcodes.Timeout). - ExpectOutputContains(`Timeout exceeded: try increasing it by passing --timeout option`) -} - func TestTimeout(t *testing.T) { projectRoot := filepath.Join("..", "...") @@ -82,43 +68,21 @@ func TestTimeout(t *testing.T) { } func TestTimeoutInConfig(t *testing.T) { - cases := []struct { - cfg string - }{ - { - cfg: ` - run: - deadline: 1ms - `, - }, - { - cfg: ` - run: - timeout: 1ms - `, - }, - { - // timeout should override deadline - cfg: ` + testshared.InstallGolangciLint(t) + + cfg := ` run: - deadline: 100s timeout: 1ms - `, - }, - } - - testshared.InstallGolangciLint(t) + ` - for _, c := range cases { - // Run with disallowed option set only in config - testshared.NewRunnerBuilder(t). - WithConfig(c.cfg). - WithTargetPath(testdataDir, minimalPkg). - Runner(). - Run(). - ExpectExitCode(exitcodes.Timeout). - ExpectOutputContains(`Timeout exceeded: try increasing it by passing --timeout option`) - } + // Run with disallowed option set only in config + testshared.NewRunnerBuilder(t). + WithConfig(cfg). + WithTargetPath(testdataDir, minimalPkg). + Runner(). + Run(). + ExpectExitCode(exitcodes.Timeout). + ExpectOutputContains(`Timeout exceeded: try increasing it by passing --timeout option`) } func TestTestsAreLintedByDefault(t *testing.T) { From 9159274276768db291ad3ba84e9d4a5d13a356f4 Mon Sep 17 00:00:00 2001 From: Fernandez Ludovic Date: Tue, 20 Feb 2024 20:59:03 +0100 Subject: [PATCH 35/45] chore: converts initHashSalt from methods to function --- pkg/commands/cache.go | 4 ++-- pkg/commands/executor.go | 3 ++- 2 files changed, 4 insertions(+), 3 deletions(-) diff --git a/pkg/commands/cache.go b/pkg/commands/cache.go index 3a8f737862e2..3dbe2427f578 100644 --- a/pkg/commands/cache.go +++ b/pkg/commands/cache.go @@ -80,13 +80,13 @@ func dirSizeBytes(path string) (int64, error) { // --- Related to cache but not used directly by the cache command. -func (e *Executor) initHashSalt(version string) error { +func initHashSalt(version string, cfg *config.Config) error { binSalt, err := computeBinarySalt(version) if err != nil { return fmt.Errorf("failed to calculate binary salt: %w", err) } - configSalt, err := computeConfigSalt(e.cfg) + configSalt, err := computeConfigSalt(cfg) if err != nil { return fmt.Errorf("failed to calculate config salt: %w", err) } diff --git a/pkg/commands/executor.go b/pkg/commands/executor.go index 20c7d572733e..211466cfb645 100644 --- a/pkg/commands/executor.go +++ b/pkg/commands/executor.go @@ -155,7 +155,8 @@ func (e *Executor) initExecutor() { e.contextLoader = lint.NewContextLoader(e.cfg, e.log.Child(logutils.DebugKeyLoader), e.goenv, e.lineCache, e.fileCache, pkgCache, load.NewGuard()) - if err = e.initHashSalt(e.buildInfo.Version); err != nil { + + if err = initHashSalt(e.buildInfo.Version, e.cfg); err != nil { e.log.Fatalf("Failed to init hash salt: %s", err) } } From 51c1ee6875475f2cdd7edb3578bcf6d79359a282 Mon Sep 17 00:00:00 2001 From: Fernandez Ludovic Date: Tue, 20 Feb 2024 21:01:03 +0100 Subject: [PATCH 36/45] chore: removes useless flags from config command --- pkg/commands/config.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/pkg/commands/config.go b/pkg/commands/config.go index 2662514a6f9f..f269d881bd75 100644 --- a/pkg/commands/config.go +++ b/pkg/commands/config.go @@ -34,8 +34,6 @@ func (e *Executor) initConfig() { fs := pathCmd.Flags() fs.SortFlags = false // sort them as they are defined here - initConfigFileFlagSet(fs, &e.cfg.Run) - configCmd.AddCommand(pathCmd) e.rootCmd.AddCommand(configCmd) } @@ -67,6 +65,8 @@ func (e *Executor) getUsedConfig() string { return prettyUsedConfigFile } +// --- Related to config but not used directly by the config command. + func initConfigFileFlagSet(fs *pflag.FlagSet, cfg *config.Run) { fs.StringVarP(&cfg.Config, "config", "c", "", wh("Read config from file path `PATH`")) fs.BoolVar(&cfg.NoConfig, "no-config", false, wh("Don't read config file")) From 7a0bc08d52e1528de1d0045495c5770dfafdb70f Mon Sep 17 00:00:00 2001 From: Fernandez Ludovic Date: Tue, 20 Feb 2024 21:14:09 +0100 Subject: [PATCH 37/45] chore: fix typos --- pkg/config/config.go | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/pkg/config/config.go b/pkg/config/config.go index 2eb82938dc69..78fc7f4ed1fa 100644 --- a/pkg/config/config.go +++ b/pkg/config/config.go @@ -8,10 +8,11 @@ import ( "github.com/ldez/gomoddirectives" ) -// Config encapsulates the config data specified in the golangci yaml config file. +// Config encapsulates the config data specified in the golangci-lint yaml config file. type Config struct { - cfgDir string // The directory containing the golangci config file. - Run Run + cfgDir string // The directory containing the golangci-lint config file. + + Run Run Output Output From 074c369550968e14d890f4407ae283ba3ba71d4f Mon Sep 17 00:00:00 2001 From: Fernandez Ludovic Date: Tue, 20 Feb 2024 21:18:26 +0100 Subject: [PATCH 38/45] chore(offtopic): rename a variable inside NewGochecknoglobals --- pkg/golinters/gochecknoglobals.go | 19 ++++++++----------- 1 file changed, 8 insertions(+), 11 deletions(-) diff --git a/pkg/golinters/gochecknoglobals.go b/pkg/golinters/gochecknoglobals.go index 6e18aeb27d90..d1edd48f6408 100644 --- a/pkg/golinters/gochecknoglobals.go +++ b/pkg/golinters/gochecknoglobals.go @@ -8,22 +8,19 @@ import ( ) func NewGochecknoglobals() *goanalysis.Linter { - gochecknoglobals := checknoglobals.Analyzer() + a := checknoglobals.Analyzer() - // gochecknoglobals only lints test files if the `-t` flag is passed, so we - // pass the `t` flag as true to the analyzer before running it. This can be - // turned off by using the regular golangci-lint flags such as `--tests` or - // `--skip-files`. + // gochecknoglobals only lints test files if the `-t` flag is passed, + // so we pass the `t` flag as true to the analyzer before running it. + // This can be turned off by using the regular golangci-lint flags such as `--tests` or `--skip-files`. linterConfig := map[string]map[string]any{ - gochecknoglobals.Name: { - "t": true, - }, + a.Name: {"t": true}, } return goanalysis.NewLinter( - gochecknoglobals.Name, - gochecknoglobals.Doc, - []*analysis.Analyzer{gochecknoglobals}, + a.Name, + a.Doc, + []*analysis.Analyzer{a}, linterConfig, ).WithLoadMode(goanalysis.LoadModeTypesInfo) } From 3e97d56c513ee7d64b2747d83e1b380dff8e283e Mon Sep 17 00:00:00 2001 From: Fernandez Ludovic Date: Tue, 20 Feb 2024 21:24:09 +0100 Subject: [PATCH 39/45] chore(offtopic): extract govet validation from FileReader --- pkg/config/linters_settings.go | 1 + pkg/config/reader.go | 23 +++++++++++------------ pkg/golinters/govet.go | 5 +++++ 3 files changed, 17 insertions(+), 12 deletions(-) diff --git a/pkg/config/linters_settings.go b/pkg/config/linters_settings.go index 7158f0a9d725..57d126d940e1 100644 --- a/pkg/config/linters_settings.go +++ b/pkg/config/linters_settings.go @@ -606,6 +606,7 @@ type GovetSettings struct { } func (cfg *GovetSettings) Validate() error { + // TODO(ldez) need to be move into the linter file. if cfg.EnableAll && cfg.DisableAll { return errors.New("enable-all and disable-all can't be combined") } diff --git a/pkg/config/reader.go b/pkg/config/reader.go index 0c4fa13b0636..336ac9fd5793 100644 --- a/pkg/config/reader.go +++ b/pkg/config/reader.go @@ -116,42 +116,41 @@ func (r *FileReader) parseConfig() error { } func (r *FileReader) validateConfig() error { - c := r.cfg - if len(c.Run.Args) != 0 { + if len(r.cfg.Run.Args) != 0 { return errors.New("option run.args in config isn't supported now") } - if c.Run.CPUProfilePath != "" { + if r.cfg.Run.CPUProfilePath != "" { return errors.New("option run.cpuprofilepath in config isn't allowed") } - if c.Run.MemProfilePath != "" { + if r.cfg.Run.MemProfilePath != "" { return errors.New("option run.memprofilepath in config isn't allowed") } - if c.Run.TracePath != "" { + if r.cfg.Run.TracePath != "" { return errors.New("option run.tracepath in config isn't allowed") } - if c.Run.IsVerbose { + if r.cfg.Run.IsVerbose { return errors.New("can't set run.verbose option with config: only on command-line") } - for i, rule := range c.Issues.ExcludeRules { + + for i, rule := range r.cfg.Issues.ExcludeRules { if err := rule.Validate(); err != nil { return fmt.Errorf("error in exclude rule #%d: %w", i, err) } } - if len(c.Severity.Rules) > 0 && c.Severity.Default == "" { + + if len(r.cfg.Severity.Rules) > 0 && r.cfg.Severity.Default == "" { return errors.New("can't set severity rule option: no default severity defined") } - for i, rule := range c.Severity.Rules { + for i, rule := range r.cfg.Severity.Rules { if err := rule.Validate(); err != nil { return fmt.Errorf("error in severity rule #%d: %w", i, err) } } - if err := c.LintersSettings.Govet.Validate(); err != nil { - return fmt.Errorf("error in govet config: %w", err) - } + return nil } diff --git a/pkg/golinters/govet.go b/pkg/golinters/govet.go index 066f7e682a3d..75539755781c 100644 --- a/pkg/golinters/govet.go +++ b/pkg/golinters/govet.go @@ -142,6 +142,11 @@ func NewGovet(settings *config.GovetSettings) *goanalysis.Linter { conf = settings.Settings } + err := settings.Validate() + if err != nil { + linterLogger.Fatalf("govet configuration: %v", err) + } + return goanalysis.NewLinter( "govet", "Vet examines Go source code and reports suspicious constructs. "+ From 3b276853a3894321bafb578bef05d9a1ead1df6c Mon Sep 17 00:00:00 2001 From: Fernandez Ludovic Date: Tue, 20 Feb 2024 22:57:52 +0100 Subject: [PATCH 40/45] chore: isolate Version field in Config --- pkg/config/config.go | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/pkg/config/config.go b/pkg/config/config.go index 78fc7f4ed1fa..633e91b2731e 100644 --- a/pkg/config/config.go +++ b/pkg/config/config.go @@ -20,7 +20,8 @@ type Config struct { Linters Linters Issues Issues Severity Severity - Version Version + + Version Version InternalCmdTest bool `mapstructure:"internal-cmd-test"` // Option is used only for testing golangci-lint command, don't use it InternalTest bool // Option is used only for testing golangci-lint code, don't use it From c1d711d953adbdf588c675e2b7b3907f1707045e Mon Sep 17 00:00:00 2001 From: Fernandez Ludovic Date: Tue, 20 Feb 2024 23:08:33 +0100 Subject: [PATCH 41/45] chore: use errors.As --- pkg/config/reader.go | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/pkg/config/reader.go b/pkg/config/reader.go index 336ac9fd5793..40ff6f0e1c68 100644 --- a/pkg/config/reader.go +++ b/pkg/config/reader.go @@ -61,7 +61,8 @@ func (r *FileReader) Read() error { func (r *FileReader) parseConfig() error { if err := viper.ReadInConfig(); err != nil { - if _, ok := err.(viper.ConfigFileNotFoundError); ok { + var configFileNotFoundError viper.ConfigFileNotFoundError + if errors.As(err, &configFileNotFoundError) { return nil } From 858a41c21758bc5a94f8cd2eebfc892fcb63d4c3 Mon Sep 17 00:00:00 2001 From: Fernandez Ludovic Date: Wed, 21 Feb 2024 01:12:52 +0100 Subject: [PATCH 42/45] chore: annotate and re-organize configuration structures --- pkg/config/config.go | 14 +++++++------- pkg/config/output.go | 4 ++-- pkg/config/run.go | 33 +++++++++++++++++++-------------- 3 files changed, 28 insertions(+), 23 deletions(-) diff --git a/pkg/config/config.go b/pkg/config/config.go index 633e91b2731e..f73563a2a4ef 100644 --- a/pkg/config/config.go +++ b/pkg/config/config.go @@ -12,18 +12,18 @@ import ( type Config struct { cfgDir string // The directory containing the golangci-lint config file. - Run Run + Run Run `mapstructure:"run"` - Output Output + Output Output `mapstructure:"output"` LintersSettings LintersSettings `mapstructure:"linters-settings"` - Linters Linters - Issues Issues - Severity Severity + Linters Linters `mapstructure:"linters"` + Issues Issues `mapstructure:"issues"` + Severity Severity `mapstructure:"severity"` - Version Version + Version Version // Flag only. // TODO(ldez) only used by the version command. - InternalCmdTest bool `mapstructure:"internal-cmd-test"` // Option is used only for testing golangci-lint command, don't use it + InternalCmdTest bool // Option is used only for testing golangci-lint command, don't use it InternalTest bool // Option is used only for testing golangci-lint code, don't use it } diff --git a/pkg/config/output.go b/pkg/config/output.go index e8726392055d..28e4f29b3414 100644 --- a/pkg/config/output.go +++ b/pkg/config/output.go @@ -28,7 +28,7 @@ var OutFormats = []string{ } type Output struct { - Format string + Format string `mapstructure:"format"` PrintIssuedLine bool `mapstructure:"print-issued-lines"` PrintLinterName bool `mapstructure:"print-linter-name"` UniqByLine bool `mapstructure:"uniq-by-line"` @@ -37,5 +37,5 @@ type Output struct { PathPrefix string `mapstructure:"path-prefix"` // only work with CLI flags because the setup of logs is done before the config file parsing. - Color string + Color string // Flag only. } diff --git a/pkg/config/run.go b/pkg/config/run.go index 0676499d558b..52c6ce262b57 100644 --- a/pkg/config/run.go +++ b/pkg/config/run.go @@ -4,18 +4,9 @@ import "time" // Run encapsulates the config options for running the linter analysis. type Run struct { - IsVerbose bool `mapstructure:"verbose"` - Silent bool - CPUProfilePath string - MemProfilePath string - TracePath string - Concurrency int - PrintResourcesUsage bool `mapstructure:"print-resources-usage"` + Timeout time.Duration `mapstructure:"timeout"` - Config string // The path to the golangci config file, as specified with the --config argument. - NoConfig bool - - Args []string + Concurrency int `mapstructure:"concurrency"` Go string `mapstructure:"go"` @@ -25,9 +16,6 @@ type Run struct { ExitCodeIfIssuesFound int `mapstructure:"issues-exit-code"` AnalyzeTests bool `mapstructure:"tests"` - Timeout time.Duration - - PrintVersion bool SkipFiles []string `mapstructure:"skip-files"` SkipDirs []string `mapstructure:"skip-dirs"` UseDefaultSkipDirs bool `mapstructure:"skip-dirs-use-default"` @@ -36,4 +24,21 @@ type Run struct { AllowSerialRunners bool `mapstructure:"allow-serial-runners"` ShowStats bool `mapstructure:"show-stats"` + + // --- Flags only section. + + IsVerbose bool `mapstructure:"verbose"` // Flag only + + PrintVersion bool // Flag only. (used by the root command) + + CPUProfilePath string // Flag only. + MemProfilePath string // Flag only. + TracePath string // Flag only. + + PrintResourcesUsage bool `mapstructure:"print-resources-usage"` // Flag only. // TODO(ldez) need to be enforced. + + Config string // Flag only. The path to the golangci config file, as specified with the --config argument. + NoConfig bool // Flag only. + + Args []string // Flag only. // TODO(ldez) identify the real need and usage. } From bfccfbda12f1a973486809f2af6658046f05b189 Mon Sep 17 00:00:00 2001 From: Fernandez Ludovic Date: Wed, 21 Feb 2024 02:50:02 +0100 Subject: [PATCH 43/45] chore: fix typo --- pkg/commands/root.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/commands/root.go b/pkg/commands/root.go index d6e87f14df2c..0ae05480fa73 100644 --- a/pkg/commands/root.go +++ b/pkg/commands/root.go @@ -93,7 +93,7 @@ func (e *Executor) persistentPostRun(_ *cobra.Command, _ []string) error { printMemStats(&ms, e.log) if err := pprof.WriteHeapProfile(f); err != nil { - return fmt.Errorf("cCan't write heap profile: %w", err) + return fmt.Errorf("can't write heap profile: %w", err) } _ = f.Close() } From 33416b267629d99333eb5697abd3e2a86ee76b8e Mon Sep 17 00:00:00 2001 From: Fernandez Ludovic Date: Wed, 21 Feb 2024 15:24:12 +0100 Subject: [PATCH 44/45] fix: govet settings validation --- pkg/golinters/govet.go | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/pkg/golinters/govet.go b/pkg/golinters/govet.go index 75539755781c..2ed8f252f606 100644 --- a/pkg/golinters/govet.go +++ b/pkg/golinters/govet.go @@ -139,12 +139,12 @@ var ( func NewGovet(settings *config.GovetSettings) *goanalysis.Linter { var conf map[string]map[string]any if settings != nil { - conf = settings.Settings - } + err := settings.Validate() + if err != nil { + linterLogger.Fatalf("govet configuration: %v", err) + } - err := settings.Validate() - if err != nil { - linterLogger.Fatalf("govet configuration: %v", err) + conf = settings.Settings } return goanalysis.NewLinter( From a3618d71719bff8e57ff856aab22a55d202c2aff Mon Sep 17 00:00:00 2001 From: Fernandez Ludovic Date: Wed, 21 Feb 2024 22:34:42 +0100 Subject: [PATCH 45/45] chore: remove allPresets function --- pkg/lint/lintersdb/manager.go | 8 -------- pkg/lint/lintersdb/validator.go | 8 +++++--- 2 files changed, 5 insertions(+), 11 deletions(-) diff --git a/pkg/lint/lintersdb/manager.go b/pkg/lint/lintersdb/manager.go index 1f94579e8f31..707a0bd15664 100644 --- a/pkg/lint/lintersdb/manager.go +++ b/pkg/lint/lintersdb/manager.go @@ -984,14 +984,6 @@ func AllPresets() []string { } } -func allPresetsSet() map[string]bool { - ret := map[string]bool{} - for _, p := range AllPresets() { - ret[p] = true - } - return ret -} - // Trims the Go version to keep only M.m. // Since Go 1.21 the version inside the go.mod can be a patched version (ex: 1.21.0). // https://go.dev/doc/toolchain#versions diff --git a/pkg/lint/lintersdb/validator.go b/pkg/lint/lintersdb/validator.go index 2b4f52da0bb7..9ea568f1a44e 100644 --- a/pkg/lint/lintersdb/validator.go +++ b/pkg/lint/lintersdb/validator.go @@ -3,6 +3,7 @@ package lintersdb import ( "errors" "fmt" + "slices" "strings" "github.com/golangci/golangci-lint/pkg/config" @@ -39,11 +40,12 @@ func (v Validator) validateLintersNames(cfg *config.Linters) error { } func (v Validator) validatePresets(cfg *config.Linters) error { - allPresets := allPresetsSet() + presets := AllPresets() + for _, p := range cfg.Presets { - if !allPresets[p] { + if !slices.Contains(presets, p) { return fmt.Errorf("no such preset %q: only next presets exist: (%s)", - p, strings.Join(AllPresets(), "|")) + p, strings.Join(presets, "|")) } }