don't print config parsing info logs twice

Author: jirfag

pkg/commands/executor.go

@@ -6,10 +6,12 @@ import (
 	"github.com/golangci/golangci-lint/pkg/logutils"
 	"github.com/golangci/golangci-lint/pkg/report"
 	"github.com/spf13/cobra"
+	"github.com/spf13/pflag"
 )
 
 type Executor struct {
 	rootCmd *cobra.Command
+	runCmd  *cobra.Command
 
 	exitCode              int
 	version, commit, date string
@@ -23,22 +25,47 @@ type Executor struct {
 
 func NewExecutor(version, commit, date string) *Executor {
 	e := &Executor{
-		cfg:     config.NewDefault(),
-		version: version,
-		commit:  commit,
-		date:    date,
+		cfg:       config.NewDefault(),
+		version:   version,
+		commit:    commit,
+		date:      date,
+		DBManager: lintersdb.NewManager(),
 	}
 
 	e.log = report.NewLogWrapper(logutils.NewStderrLog(""), &e.reportData)
-	e.DBManager = lintersdb.NewManager()
-	e.EnabledLintersSet = lintersdb.NewEnabledSet(e.DBManager, &lintersdb.Validator{},
-		e.log.Child("lintersdb"), e.cfg)
 
+	// to setup log level early we need to parse config from command line extra time to
+	// find `-v` option
+	commandLineCfg, err := e.getConfigForCommandLine()
+	if err != nil && err != pflag.ErrHelp {
+		e.log.Fatalf("Can't get config for command line: %s", err)
+	}
+	if commandLineCfg != nil {
+		logutils.SetupVerboseLog(e.log, commandLineCfg.Run.IsVerbose)
+	}
+
+	// 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()
 
+	// 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("config_reader"))
+	if err := r.Read(); err != nil {
+		e.log.Fatalf("Can't read config: %s", err)
+	}
+
+	// Slice options must be explicitly set for proper merging of config and command-line options.
+	fixSlicesFlags(e.runCmd.Flags())
+
+	e.EnabledLintersSet = lintersdb.NewEnabledSet(e.DBManager,
+		lintersdb.NewValidator(e.DBManager), e.log.Child("lintersdb"), e.cfg)
+
 	return e
 }
 

pkg/commands/linters.go

@@ -29,7 +29,7 @@ func IsLinterInConfigsList(name string, linters []linter.Config) bool {
 	return false
 }
 
-func (e Executor) executeLinters(cmd *cobra.Command, args []string) {
+func (e *Executor) executeLinters(cmd *cobra.Command, args []string) {
 	enabledLCs, err := e.EnabledLintersSet.Get()
 	if err != nil {
 		log.Fatalf("Can't get enabled linters: %s", err)

pkg/commands/root.go

@@ -20,8 +20,6 @@ func (e *Executor) persistentPreRun(cmd *cobra.Command, args []string) {
 
 	runtime.GOMAXPROCS(e.cfg.Run.Concurrency)
 
-	logutils.SetupVerboseLog(e.log, e.cfg.Run.IsVerbose)
-
 	if e.cfg.Run.CPUProfilePath != "" {
 		f, err := os.Create(e.cfg.Run.CPUProfilePath)
 		if err != nil {

pkg/commands/run.go

@@ -168,42 +168,48 @@ func (e *Executor) initRunConfiguration(cmd *cobra.Command) {
 	fs := cmd.Flags()
 	fs.SortFlags = false // sort them as they are defined here
 	initFlagSet(fs, e.cfg, e.DBManager)
+}
 
-	// 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, e.log.Child("config_reader"), func(fs *pflag.FlagSet, 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.
-		initFlagSet(fs, cfg, e.DBManager)
-
-		// 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)
-	})
-	if err := r.Read(); err != nil {
-		e.log.Fatalf("Can't read config: %s", err)
+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.
+	initFlagSet(fs, &cfg, e.DBManager)
+
+	// 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 err == pflag.ErrHelp {
+			return nil, err
+		}
+
+		return nil, fmt.Errorf("can't parse args: %s", err)
 	}
 
-	// Slice options must be explicitly set for proper merging of config and command-line options.
-	fixSlicesFlags(fs)
+	return &cfg, nil
 }
 
 func (e *Executor) initRun() {
-	var runCmd = &cobra.Command{
+	e.runCmd = &cobra.Command{
 		Use:   "run",
 		Short: welcomeMessage,
 		Run:   e.executeRun,
 	}
-	e.rootCmd.AddCommand(runCmd)
+	e.rootCmd.AddCommand(e.runCmd)
 
-	runCmd.SetOutput(logutils.StdOut) // use custom output to properly color it in Windows terminals
+	e.runCmd.SetOutput(logutils.StdOut) // use custom output to properly color it in Windows terminals
 
-	e.initRunConfiguration(runCmd)
+	e.initRunConfiguration(e.runCmd)
 }
 
 func fixSlicesFlags(fs *pflag.FlagSet) {

pkg/config/reader.go

@@ -5,26 +5,24 @@ import (
 	"fmt"
 	"os"
 	"path/filepath"
+	"strings"
 
 	"github.com/golangci/golangci-lint/pkg/fsutils"
 	"github.com/golangci/golangci-lint/pkg/logutils"
-	"github.com/spf13/pflag"
 	"github.com/spf13/viper"
 )
 
-type FlagSetInit func(fs *pflag.FlagSet, cfg *Config)
-
 type FileReader struct {
-	log         logutils.Log
-	cfg         *Config
-	flagSetInit FlagSetInit
+	log            logutils.Log
+	cfg            *Config
+	commandLineCfg *Config
 }
 
-func NewFileReader(toCfg *Config, log logutils.Log, flagSetInit FlagSetInit) *FileReader {
+func NewFileReader(toCfg, commandLineCfg *Config, log logutils.Log) *FileReader {
 	return &FileReader{
-		log:         log,
-		cfg:         toCfg,
-		flagSetInit: flagSetInit,
+		log:            log,
+		cfg:            toCfg,
+		commandLineCfg: commandLineCfg,
 	}
 }
 
@@ -33,9 +31,9 @@ func (r *FileReader) Read() error {
 	// 1. to access "config" option here.
 	// 2. to give config less priority than command line.
 
-	configFile, restArgs, err := r.parseConfigOption()
+	configFile, err := r.parseConfigOption()
 	if err != nil {
-		if err == errConfigDisabled || err == pflag.ErrHelp {
+		if err == errConfigDisabled {
 			return nil
 		}
 
@@ -45,7 +43,7 @@ func (r *FileReader) Read() error {
 	if configFile != "" {
 		viper.SetConfigFile(configFile)
 	} else {
-		r.setupConfigFileSearch(restArgs)
+		r.setupConfigFileSearch()
 	}
 
 	return r.parseConfig()
@@ -108,7 +106,9 @@ func (r *FileReader) validateConfig() error {
 	return nil
 }
 
-func (r *FileReader) setupConfigFileSearch(args []string) {
+func getFirstPathArg() string {
+	args := os.Args
+
 	// skip all args ([golangci-lint, run/linters]) before files/dirs list
 	for len(args) != 0 {
 		if args[0] == "run" {
@@ -121,10 +121,18 @@ func (r *FileReader) setupConfigFileSearch(args []string) {
 
 	// find first file/dir arg
 	firstArg := "./..."
-	if len(args) != 0 {
-		firstArg = args[0]
+	for _, arg := range args {
+		if !strings.HasPrefix(arg, "-") {
+			firstArg = arg
+			break
+		}
 	}
 
+	return firstArg
+}
+
+func (r *FileReader) setupConfigFileSearch() {
+	firstArg := getFirstPathArg()
 	absStartPath, err := filepath.Abs(firstArg)
 	if err != nil {
 		r.log.Warnf("Can't make abs path for %q: %s", firstArg, err)
@@ -159,34 +167,20 @@ func (r *FileReader) setupConfigFileSearch(args []string) {
 
 var errConfigDisabled = errors.New("config is disabled by --no-config")
 
-func (r *FileReader) parseConfigOption() (string, []string, 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
-	r.flagSetInit(fs, &cfg)
-
-	fs.Usage = func() {} // otherwise help text will be printed twice
-	if err := fs.Parse(os.Args); err != nil {
-		if err == pflag.ErrHelp {
-			return "", nil, err
-		}
-
-		return "", nil, fmt.Errorf("can't parse args: %s", err)
+func (r *FileReader) parseConfigOption() (string, error) {
+	cfg := r.commandLineCfg
+	if cfg == nil {
+		return "", nil
 	}
 
-	// for `-v` to work until running of preRun function
-	logutils.SetupVerboseLog(r.log, cfg.Run.IsVerbose)
-
 	configFile := cfg.Run.Config
 	if cfg.Run.NoConfig && configFile != "" {
-		return "", nil, fmt.Errorf("can't combine option --config and --no-config")
+		return "", fmt.Errorf("can't combine option --config and --no-config")
 	}
 
 	if cfg.Run.NoConfig {
-		return "", nil, errConfigDisabled
+		return "", errConfigDisabled
 	}
 
-	return configFile, fs.Args(), nil
+	return configFile, nil
 }

pkg/lint/lintersdb/enabled_set.go

@@ -129,7 +129,6 @@ func (es EnabledSet) Get() ([]linter.Config, error) {
 	}
 
 	es.verbosePrintLintersStatus(resultLinters)
-
 	return resultLinters, nil
 }
 

pkg/lint/lintersdb/enabled_set_test.go

@@ -45,7 +45,7 @@ func TestGetEnabledLintersSet(t *testing.T) {
 	}
 
 	m := NewManager()
-	es := NewEnabledSet(m, &Validator{}, nil, nil)
+	es := NewEnabledSet(m, NewValidator(m), nil, nil)
 	for _, c := range cases {
 		t.Run(c.name, func(t *testing.T) {
 			defaultLinters := []linter.Config{}

pkg/lint/lintersdb/validator.go

@@ -11,6 +11,12 @@ type Validator struct {
 	m *Manager
 }
 
+func NewValidator(m *Manager) *Validator {
+	return &Validator{
+		m: m,
+	}
+}
+
 func (v Validator) validateLintersNames(cfg *config.Linters) error {
 	allNames := append([]string{}, cfg.Enable...)
 	allNames = append(allNames, cfg.Disable...)

scripts/gen_readme/main.go

@@ -89,7 +89,7 @@ func buildTemplateContext() (map[string]interface{}, error) {
 
 func getLintersListMarkdown(enabled bool) string {
 	var neededLcs []linter.Config
-	lcs := lintersdb.GetAllSupportedLinterConfigs()
+	lcs := lintersdb.NewManager().GetAllSupportedLinterConfigs()
 	for _, lc := range lcs {
 		if lc.EnabledByDefault == enabled {
 			neededLcs = append(neededLcs, lc)
@@ -114,7 +114,7 @@ func getLintersListMarkdown(enabled bool) string {
 func getThanksList() string {
 	var lines []string
 	addedAuthors := map[string]bool{}
-	for _, lc := range lintersdb.GetAllSupportedLinterConfigs() {
+	for _, lc := range lintersdb.NewManager().GetAllSupportedLinterConfigs() {
 		if lc.OriginalURL == "" {
 			continue
 		}

test/run_test.go

@@ -170,7 +170,8 @@ func inSlice(s []string, v string) bool {
 }
 
 func getEnabledByDefaultFastLintersExcept(except ...string) []string {
-	ebdl := lintersdb.GetAllEnabledByDefaultLinters()
+	m := lintersdb.NewManager()
+	ebdl := m.GetAllEnabledByDefaultLinters()
 	ret := []string{}
 	for _, linter := range ebdl {
 		if linter.DoesFullImport {
@@ -186,7 +187,7 @@ func getEnabledByDefaultFastLintersExcept(except ...string) []string {
 }
 
 func getAllFastLintersWith(with ...string) []string {
-	linters := lintersdb.GetAllSupportedLinterConfigs()
+	linters := lintersdb.NewManager().GetAllSupportedLinterConfigs()
 	ret := append([]string{}, with...)
 	for _, linter := range linters {
 		if linter.DoesFullImport {
@@ -199,7 +200,7 @@ func getAllFastLintersWith(with ...string) []string {
 }
 
 func getEnabledByDefaultLinters() []string {
-	ebdl := lintersdb.GetAllEnabledByDefaultLinters()
+	ebdl := lintersdb.NewManager().GetAllEnabledByDefaultLinters()
 	ret := []string{}
 	for _, linter := range ebdl {
 		ret = append(ret, linter.Linter.Name())
@@ -209,7 +210,7 @@ func getEnabledByDefaultLinters() []string {
 }
 
 func getEnabledByDefaultFastLintersWith(with ...string) []string {
-	ebdl := lintersdb.GetAllEnabledByDefaultLinters()
+	ebdl := lintersdb.NewManager().GetAllEnabledByDefaultLinters()
 	ret := append([]string{}, with...)
 	for _, linter := range ebdl {
 		if linter.DoesFullImport {