Skip to content

#49: don't fill string slice 2 times because of double parsing of opt… #58

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Jun 2, 2018
Merged

#49: don't fill string slice 2 times because of double parsing of opt… #58

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -206,6 +206,10 @@ golangci-lint linters
- [depguard](https://github.com/OpenPeeDeeP/depguard) - Go linter that checks if package imports are in a list of acceptable packages

# Configuration
Configuration file has lower priority than command-line: if the same bool/string/int option defined in the command-line
and in the configuration file, option from command-line will be used.
Slice options (e.g. list of enabled/disabled linters) are combined from the command-line and configuration file.

## Command-Line Options
```
golangci-lint run -h
Expand Down
4 changes: 4 additions & 0 deletions README.md.tmpl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -164,6 +164,10 @@ golangci-lint linters
{{.DisabledByDefaultLinters}}

# Configuration
Configuration file has lower priority than command-line: if the same bool/string/int option defined in the command-line
and in the configuration file, option from command-line will be used.
Slice options (e.g. list of enabled/disabled linters) are combined from the command-line and configuration file.

## Command-Line Options
```
golangci-lint run -h
Expand Down
19 changes: 12 additions & 7 deletions pkg/commands/root.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ import (
"github.com/golangci/golangci-lint/pkg/printers"
"github.com/sirupsen/logrus"
"github.com/spf13/cobra"
"github.com/spf13/pflag"
)

func (e *Executor) persistentPostRun(cmd *cobra.Command, args []string) {
Expand Down Expand Up @@ -77,13 +78,17 @@ func (e *Executor) initRoot() {
PersistentPreRun: e.persistentPostRun,
PersistentPostRun: e.persistentPreRun,
}
rootCmd.PersistentFlags().BoolVarP(&e.cfg.Run.IsVerbose, "verbose", "v", false, wh("verbose output"))
rootCmd.PersistentFlags().StringVar(&e.cfg.Run.CPUProfilePath, "cpu-profile-path", "", wh("Path to CPU profile output file"))
rootCmd.PersistentFlags().StringVar(&e.cfg.Run.MemProfilePath, "mem-profile-path", "", wh("Path to memory profile output file"))
rootCmd.PersistentFlags().IntVarP(&e.cfg.Run.Concurrency, "concurrency", "j", getDefaultConcurrency(), wh("Concurrency (default NumCPU)"))
if e.commit != "" {
rootCmd.PersistentFlags().BoolVar(&e.cfg.Run.PrintVersion, "version", false, wh("Print version"))
}

e.initRootFlagSet(rootCmd.PersistentFlags())
e.rootCmd = rootCmd
}

func (e *Executor) initRootFlagSet(fs *pflag.FlagSet) {
fs.BoolVarP(&e.cfg.Run.IsVerbose, "verbose", "v", false, wh("verbose output"))
fs.StringVar(&e.cfg.Run.CPUProfilePath, "cpu-profile-path", "", wh("Path to CPU profile output file"))
fs.StringVar(&e.cfg.Run.MemProfilePath, "mem-profile-path", "", wh("Path to memory profile output file"))
fs.IntVarP(&e.cfg.Run.Concurrency, "concurrency", "j", getDefaultConcurrency(), wh("Concurrency (default NumCPU)"))
if e.commit != "" {
fs.BoolVar(&e.cfg.Run.PrintVersion, "version", false, wh("Print version"))
}
}
124 changes: 71 additions & 53 deletions pkg/commands/run.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -46,43 +46,33 @@ func wh(text string) string {
return color.GreenString(text)
}

func (e *Executor) initRun() {
var runCmd = &cobra.Command{
Use: "run",
Short: welcomeMessage,
Run: e.executeRun,
}
e.rootCmd.AddCommand(runCmd)

runCmd.SetOutput(printers.StdOut) // use custom output to properly color it in Windows terminals
runCmd.Flags().SortFlags = false // sort them as they are defined here

func (e *Executor) initFlagSet(fs *pflag.FlagSet) {
hideFlag := func(name string) {
if err := runCmd.Flags().MarkHidden(name); err != nil {
if err := fs.MarkHidden(name); err != nil {
panic(err)
}
}

// Output config
oc := &e.cfg.Output
runCmd.Flags().StringVar(&oc.Format, "out-format",
fs.StringVar(&oc.Format, "out-format",
config.OutFormatColoredLineNumber,
wh(fmt.Sprintf("Format of output: %s", strings.Join(config.OutFormats, "|"))))
runCmd.Flags().BoolVar(&oc.PrintIssuedLine, "print-issued-lines", true, wh("Print lines of code with issue"))
runCmd.Flags().BoolVar(&oc.PrintLinterName, "print-linter-name", true, wh("Print linter name in issue line"))
runCmd.Flags().BoolVar(&oc.PrintWelcomeMessage, "print-welcome", false, wh("Print welcome message"))
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.PrintWelcomeMessage, "print-welcome", false, wh("Print welcome message"))
hideFlag("print-welcome") // no longer used

// Run config
rc := &e.cfg.Run
runCmd.Flags().IntVar(&rc.ExitCodeIfIssuesFound, "issues-exit-code",
fs.IntVar(&rc.ExitCodeIfIssuesFound, "issues-exit-code",
1, wh("Exit code when issues were found"))
runCmd.Flags().StringSliceVar(&rc.BuildTags, "build-tags", []string{}, wh("Build tags (not all linters support them)"))
runCmd.Flags().DurationVar(&rc.Deadline, "deadline", time.Minute, wh("Deadline for total work"))
runCmd.Flags().BoolVar(&rc.AnalyzeTests, "tests", false, wh("Analyze tests (*_test.go)"))
runCmd.Flags().BoolVar(&rc.PrintResourcesUsage, "print-resources-usage", false, wh("Print avg and max memory usage of golangci-lint and total time"))
runCmd.Flags().StringVarP(&rc.Config, "config", "c", "", wh("Read config from file path `PATH`"))
runCmd.Flags().BoolVar(&rc.NoConfig, "no-config", false, wh("Don't read config"))
fs.StringSliceVar(&rc.BuildTags, "build-tags", []string{}, wh("Build tags (not all linters support them)"))
fs.DurationVar(&rc.Deadline, "deadline", time.Minute, wh("Deadline for total work"))
fs.BoolVar(&rc.AnalyzeTests, "tests", false, 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.StringVarP(&rc.Config, "config", "c", "", wh("Read config from file path `PATH`"))
fs.BoolVar(&rc.NoConfig, "no-config", false, wh("Don't read config"))

// Linters settings config
lsc := &e.cfg.LintersSettings
Expand All @@ -91,72 +81,88 @@ func (e *Executor) initRun() {
// but when number of linters started to grow it became ovious 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.
runCmd.Flags().BoolVar(&lsc.Errcheck.CheckTypeAssertions, "errcheck.check-type-assertions", false, "Errcheck: check for ignored type assertion results")
fs.BoolVar(&lsc.Errcheck.CheckTypeAssertions, "errcheck.check-type-assertions", false, "Errcheck: check for ignored type assertion results")
hideFlag("errcheck.check-type-assertions")

runCmd.Flags().BoolVar(&lsc.Errcheck.CheckAssignToBlank, "errcheck.check-blank", false, "Errcheck: check for errors assigned to blank identifier: _ = errFunc()")
fs.BoolVar(&lsc.Errcheck.CheckAssignToBlank, "errcheck.check-blank", false, "Errcheck: check for errors assigned to blank identifier: _ = errFunc()")
hideFlag("errcheck.check-blank")

runCmd.Flags().BoolVar(&lsc.Govet.CheckShadowing, "govet.check-shadowing", false, "Govet: check for shadowed variables")
fs.BoolVar(&lsc.Govet.CheckShadowing, "govet.check-shadowing", false, "Govet: check for shadowed variables")
hideFlag("govet.check-shadowing")

runCmd.Flags().Float64Var(&lsc.Golint.MinConfidence, "golint.min-confidence", 0.8, "Golint: minimum confidence of a problem to print it")
fs.Float64Var(&lsc.Golint.MinConfidence, "golint.min-confidence", 0.8, "Golint: minimum confidence of a problem to print it")
hideFlag("golint.min-confidence")

runCmd.Flags().BoolVar(&lsc.Gofmt.Simplify, "gofmt.simplify", true, "Gofmt: simplify code")
fs.BoolVar(&lsc.Gofmt.Simplify, "gofmt.simplify", true, "Gofmt: simplify code")
hideFlag("gofmt.simplify")

runCmd.Flags().IntVar(&lsc.Gocyclo.MinComplexity, "gocyclo.min-complexity",
fs.IntVar(&lsc.Gocyclo.MinComplexity, "gocyclo.min-complexity",
30, "Minimal complexity of function to report it")
hideFlag("gocyclo.min-complexity")

runCmd.Flags().BoolVar(&lsc.Maligned.SuggestNewOrder, "maligned.suggest-new", false, "Maligned: print suggested more optimal struct fields ordering")
fs.BoolVar(&lsc.Maligned.SuggestNewOrder, "maligned.suggest-new", false, "Maligned: print suggested more optimal struct fields ordering")
hideFlag("maligned.suggest-new")

runCmd.Flags().IntVar(&lsc.Dupl.Threshold, "dupl.threshold",
fs.IntVar(&lsc.Dupl.Threshold, "dupl.threshold",
150, "Dupl: Minimal threshold to detect copy-paste")
hideFlag("dupl.threshold")

runCmd.Flags().IntVar(&lsc.Goconst.MinStringLen, "goconst.min-len",
fs.IntVar(&lsc.Goconst.MinStringLen, "goconst.min-len",
3, "Goconst: minimum constant string length")
hideFlag("goconst.min-len")
runCmd.Flags().IntVar(&lsc.Goconst.MinOccurrencesCount, "goconst.min-occurrences",
fs.IntVar(&lsc.Goconst.MinOccurrencesCount, "goconst.min-occurrences",
3, "Goconst: minimum occurrences of constant string count to trigger issue")
hideFlag("goconst.min-occurrences")

// (@dixonwille) These flag is only used for testing purposes.
runCmd.Flags().StringSliceVar(&lsc.Depguard.Packages, "depguard.packages", nil,
fs.StringSliceVar(&lsc.Depguard.Packages, "depguard.packages", nil,
"Depguard: packages to add to the list")
hideFlag("depguard.packages")

runCmd.Flags().BoolVar(&lsc.Depguard.IncludeGoRoot, "depguard.include-go-root", false,
fs.BoolVar(&lsc.Depguard.IncludeGoRoot, "depguard.include-go-root", false,
"Depguard: check list against standard lib")
hideFlag("depguard.include-go-root")

// Linters config
lc := &e.cfg.Linters
runCmd.Flags().StringSliceVarP(&lc.Enable, "enable", "E", []string{}, wh("Enable specific linter"))
runCmd.Flags().StringSliceVarP(&lc.Disable, "disable", "D", []string{}, wh("Disable specific linter"))
runCmd.Flags().BoolVar(&lc.EnableAll, "enable-all", false, wh("Enable all linters"))
runCmd.Flags().BoolVar(&lc.DisableAll, "disable-all", false, wh("Disable all linters"))
runCmd.Flags().StringSliceVarP(&lc.Presets, "presets", "p", []string{},
fs.StringSliceVarP(&lc.Enable, "enable", "E", []string{}, wh("Enable specific linter"))
fs.StringSliceVarP(&lc.Disable, "disable", "D", []string{}, wh("Disable specific linter"))
fs.BoolVar(&lc.EnableAll, "enable-all", false, wh("Enable all linters"))
fs.BoolVar(&lc.DisableAll, "disable-all", false, wh("Disable all linters"))
fs.StringSliceVarP(&lc.Presets, "presets", "p", []string{},
wh(fmt.Sprintf("Enable presets (%s) of linters. Run 'golangci-lint linters' to see them. This option implies option --disable-all", strings.Join(lintersdb.AllPresets(), "|"))))
runCmd.Flags().BoolVar(&lc.Fast, "fast", false, wh("Run only fast linters from enabled linters set"))
fs.BoolVar(&lc.Fast, "fast", false, wh("Run only fast linters from enabled linters set"))

// Issues config
ic := &e.cfg.Issues
runCmd.Flags().StringSliceVarP(&ic.ExcludePatterns, "exclude", "e", []string{}, wh("Exclude issue by regexp"))
runCmd.Flags().BoolVar(&ic.UseDefaultExcludes, "exclude-use-default", true, getDefaultExcludeHelp())
fs.StringSliceVarP(&ic.ExcludePatterns, "exclude", "e", []string{}, wh("Exclude issue by regexp"))
fs.BoolVar(&ic.UseDefaultExcludes, "exclude-use-default", true, getDefaultExcludeHelp())

runCmd.Flags().IntVar(&ic.MaxIssuesPerLinter, "max-issues-per-linter", 50, wh("Maximum issues count per one linter. Set to 0 to disable"))
runCmd.Flags().IntVar(&ic.MaxSameIssues, "max-same-issues", 3, wh("Maximum count of issues with the same text. Set to 0 to disable"))
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"))

runCmd.Flags().BoolVarP(&ic.Diff, "new", "n", false,
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 don't allow issues in new code"))
runCmd.Flags().StringVar(&ic.DiffFromRevision, "new-from-rev", "", wh("Show only new issues created after git revision `REV`"))
runCmd.Flags().StringVar(&ic.DiffPatchFilePath, "new-from-patch", "", wh("Show only new issues created in git patch with file path `PATH`"))
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`"))

}

func (e *Executor) initRun() {
var runCmd = &cobra.Command{
Use: "run",
Short: welcomeMessage,
Run: e.executeRun,
}
e.rootCmd.AddCommand(runCmd)

e.parseConfig(runCmd)
runCmd.SetOutput(printers.StdOut) // use custom output to properly color it in Windows terminals

fs := runCmd.Flags()
fs.SortFlags = false // sort them as they are defined here
e.initFlagSet(fs)

e.parseConfig()
}

func (e *Executor) runAnalysis(ctx context.Context, args []string) (<-chan result.Issue, error) {
Expand Down Expand Up @@ -274,16 +280,28 @@ func (e *Executor) executeRun(cmd *cobra.Command, args []string) {
}
}

func (e *Executor) parseConfig(cmd *cobra.Command) {
// XXX: hack with double parsing to access "config" option here
if err := cmd.ParseFlags(os.Args); err != nil {
func (e *Executor) parseConfig() {
// XXX: hack with double parsing for 2 purposes:
// 1. to access "config" option here.
// 2. to give config less priority than command line.

// 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)

// Don't do `fs.AddFlagSet(cmd.Flags())` because it shared flags representations:
// `changed` variable inside string slice vars will be shared.
e.initFlagSet(fs)
e.initRootFlagSet(fs)

if err := fs.Parse(os.Args); err != nil {
if err == pflag.ErrHelp {
return
}
logrus.Fatalf("Can't parse args: %s", err)
}

if err := viper.BindPFlags(cmd.Flags()); err != nil {
if err := viper.BindPFlags(fs); err != nil {
logrus.Fatalf("Can't bind cobra's flags to viper: %s", err)
}

Expand Down
17 changes: 2 additions & 15 deletions pkg/lint/lintersdb/lintersdb.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -323,6 +323,7 @@ func getEnabledLintersSet(lcfg *config.Linters, enabledByDefaultLinters []linter
// --presets can only add linters to default set
for _, p := range lcfg.Presets {
for _, lc := range GetAllLinterConfigsForPreset(p) {
lc := lc
resultLintersSet[lc.Linter.Name()] = &lc
}
}
Expand Down Expand Up @@ -413,20 +414,6 @@ func GetEnabledLinters(cfg *config.Config) ([]linter.Config, error) {
return resultLinters, nil
}

func uniqStrings(ss []string) []string {
us := map[string]bool{}
for _, s := range ss {
us[s] = true
}

var ret []string
for k := range us {
ret = append(ret, k)
}

return ret
}

func verbosePrintLintersStatus(cfg *config.Config, lcs []linter.Config) {
var linterNames []string
for _, lc := range lcs {
Expand All @@ -435,6 +422,6 @@ func verbosePrintLintersStatus(cfg *config.Config, lcs []linter.Config) {
logrus.Infof("Active linters: %s", linterNames)

if len(cfg.Linters.Presets) != 0 {
logrus.Infof("Active presets: %s", uniqStrings(cfg.Linters.Presets))
logrus.Infof("Active presets: %s", cfg.Linters.Presets)
}
}