diff --git a/.gitignore b/.gitignore index 098acb3b711e..1d85c0743b1a 100644 --- a/.gitignore +++ b/.gitignore @@ -2,3 +2,4 @@ /*.pprof /dist/ /.idea/ +/test/path diff --git a/.golangci.yml b/.golangci.yml index f60eb893fa48..4c60348721de 100644 --- a/.golangci.yml +++ b/.golangci.yml @@ -12,6 +12,12 @@ linters-settings: goconst: min-len: 2 min-occurrences: 2 + depguard: + list-type: blacklist + packages: + # logging is allowed only by logutils.Log, logrus + # is allowed to use only in logutils package + - github.com/sirupsen/logrus linters: enable-all: true diff --git a/Makefile b/Makefile index 0e5aad6597a2..687d414ab01f 100644 --- a/Makefile +++ b/Makefile @@ -1,9 +1,9 @@ test: - go install ./cmd/... # needed for govet and golint - golangci-lint run -v - golangci-lint run --fast --no-config -v - golangci-lint run --no-config -v - go test -v ./... + go install ./cmd/... # needed for govet and golint if go < 1.10 + GL_TEST_RUN=1 golangci-lint run -v + GL_TEST_RUN=1 golangci-lint run --fast --no-config -v + GL_TEST_RUN=1 golangci-lint run --no-config -v + GL_TEST_RUN=1 go test -v ./... assets: svg-term --cast=183662 --out docs/demo.svg --window --width 110 --height 30 --from 2000 --to 20000 --profile Dracula --term iterm2 diff --git a/README.md b/README.md index 06ac8954320e..77c6d042e2dd 100644 --- a/README.md +++ b/README.md @@ -255,7 +255,7 @@ Flags: - Error return value of .((os\.)?std(out|err)\..*|.*Close|.*Flush|os\.Remove(All)?|.*printf?|os\.(Un)?Setenv). is not checked # golint: Annoying issue about not having a comment. The rare codebase has such comments - - (comment on exported (method|function|type)|should have( a package)? comment|comment should be of the form) + - (comment on exported (method|function|type|const)|should have( a package)? comment|comment should be of the form) # golint: False positive when tests are defined in package 'test' - func name will be used as test\.Test.* by other packages, and that stutters; consider calling this @@ -329,6 +329,12 @@ linters-settings: goconst: min-len: 2 min-occurrences: 2 + depguard: + list-type: blacklist + packages: + # logging is allowed only by logutils.Log, logrus + # is allowed to use only in logutils package + - github.com/sirupsen/logrus linters: enable-all: true diff --git a/pkg/commands/executor.go b/pkg/commands/executor.go index a52e572a5768..a62b1c1d23d6 100644 --- a/pkg/commands/executor.go +++ b/pkg/commands/executor.go @@ -2,7 +2,7 @@ package commands import ( "github.com/golangci/golangci-lint/pkg/config" - "github.com/sirupsen/logrus" + "github.com/golangci/golangci-lint/pkg/logutils" "github.com/spf13/cobra" ) @@ -14,6 +14,8 @@ type Executor struct { exitCode int version, commit, date string + + log logutils.Log } func NewExecutor(version, commit, date string) *Executor { @@ -22,10 +24,9 @@ func NewExecutor(version, commit, date string) *Executor { version: version, commit: commit, date: date, + log: logutils.NewStderrLog(""), } - logrus.SetLevel(logrus.WarnLevel) - e.initRoot() e.initRun() e.initLinters() diff --git a/pkg/commands/root.go b/pkg/commands/root.go index d4ac1b725e5a..0716331db24a 100644 --- a/pkg/commands/root.go +++ b/pkg/commands/root.go @@ -2,7 +2,6 @@ package commands import ( "fmt" - "log" "os" "runtime" "runtime/pprof" @@ -10,20 +9,10 @@ import ( "github.com/golangci/golangci-lint/pkg/config" "github.com/golangci/golangci-lint/pkg/logutils" "github.com/golangci/golangci-lint/pkg/printers" - "github.com/sirupsen/logrus" "github.com/spf13/cobra" "github.com/spf13/pflag" ) -func setupLog(isVerbose bool) { - log.SetFlags(0) // don't print time - if logutils.IsDebugEnabled() { - logrus.SetLevel(logrus.DebugLevel) - } else if isVerbose { - logrus.SetLevel(logrus.InfoLevel) - } -} - func (e *Executor) persistentPreRun(cmd *cobra.Command, args []string) { if e.cfg.Run.PrintVersion { fmt.Fprintf(printers.StdOut, "golangci-lint has version %s built from %s on %s\n", e.version, e.commit, e.date) @@ -32,15 +21,15 @@ func (e *Executor) persistentPreRun(cmd *cobra.Command, args []string) { runtime.GOMAXPROCS(e.cfg.Run.Concurrency) - setupLog(e.cfg.Run.IsVerbose) + logutils.SetupVerboseLog(e.log, e.cfg.Run.IsVerbose) if e.cfg.Run.CPUProfilePath != "" { f, err := os.Create(e.cfg.Run.CPUProfilePath) if err != nil { - logrus.Fatal(err) + e.log.Fatalf("Can't create file %s: %s", e.cfg.Run.CPUProfilePath, err) } if err := pprof.StartCPUProfile(f); err != nil { - logrus.Fatal(err) + e.log.Fatalf("Can't start CPU profiling: %s", err) } } } @@ -52,11 +41,11 @@ func (e *Executor) persistentPostRun(cmd *cobra.Command, args []string) { if e.cfg.Run.MemProfilePath != "" { f, err := os.Create(e.cfg.Run.MemProfilePath) if err != nil { - logrus.Fatal(err) + e.log.Fatalf("Can't create file %s: %s", e.cfg.Run.MemProfilePath, err) } runtime.GC() // get up-to-date statistics if err := pprof.WriteHeapProfile(f); err != nil { - logrus.Fatal("could not write memory profile: ", err) + e.log.Fatalf("Can't write heap profile: %s", err) } } @@ -78,7 +67,7 @@ func (e *Executor) initRoot() { Long: `Smart, fast linters runner. Run it in cloud for every GitHub pull request on https://golangci.com`, Run: func(cmd *cobra.Command, args []string) { if err := cmd.Help(); err != nil { - logrus.Fatal(err) + e.log.Fatalf("Can't run help: %s", err) } }, PersistentPreRun: e.persistentPreRun, @@ -89,6 +78,10 @@ func (e *Executor) initRoot() { e.rootCmd = rootCmd } +func (e *Executor) needVersionOption() bool { + return e.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.Run.CPUProfilePath, "cpu-profile-path", "", wh("Path to CPU profile output file")) diff --git a/pkg/commands/run.go b/pkg/commands/run.go index 780541d76e72..ef00dc7a4826 100644 --- a/pkg/commands/run.go +++ b/pkg/commands/run.go @@ -12,22 +12,16 @@ import ( "github.com/fatih/color" "github.com/golangci/golangci-lint/pkg/config" + "github.com/golangci/golangci-lint/pkg/exitcodes" "github.com/golangci/golangci-lint/pkg/lint" "github.com/golangci/golangci-lint/pkg/lint/lintersdb" "github.com/golangci/golangci-lint/pkg/logutils" "github.com/golangci/golangci-lint/pkg/printers" "github.com/golangci/golangci-lint/pkg/result" - "github.com/golangci/golangci-lint/pkg/result/processors" - "github.com/sirupsen/logrus" "github.com/spf13/cobra" "github.com/spf13/pflag" ) -const ( - exitCodeIfFailure = 3 - exitCodeIfTimeout = 4 -) - func getDefaultExcludeHelp() string { parts := []string{"Use or not use default excludes:"} for _, ep := range config.DefaultExcludePatterns { @@ -64,7 +58,7 @@ func initFlagSet(fs *pflag.FlagSet, cfg *config.Config) { // Run config rc := &cfg.Run fs.IntVar(&rc.ExitCodeIfIssuesFound, "issues-exit-code", - 1, wh("Exit code when issues were found")) + exitcodes.IssuesFound, wh("Exit code when issues were found")) fs.StringSliceVar(&rc.BuildTags, "build-tags", nil, wh("Build tags")) fs.DurationVar(&rc.Deadline, "deadline", time.Minute, wh("Deadline for total work")) fs.BoolVar(&rc.AnalyzeTests, "tests", true, wh("Analyze tests (*_test.go)")) @@ -165,65 +159,76 @@ func (e *Executor) initRun() { // 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. - e.parseConfig() - // Slice options must be explicitly set for properly merging. + 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) + + // 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) + } + + // Slice options must be explicitly set for proper merging of config and command-line options. fixSlicesFlags(fs) } +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 + } + + // calling Set sets Changed to true: next Set calls will append, not overwrite + _ = f.Value.Set(strings.Join(s, ",")) + }) +} + func (e *Executor) runAnalysis(ctx context.Context, args []string) (<-chan result.Issue, error) { e.cfg.Run.Args = args - linters, err := lintersdb.GetEnabledLinters(e.cfg) + linters, err := lintersdb.GetEnabledLinters(e.cfg, e.log.Child("lintersdb")) if err != nil { return nil, err } - lintCtx, err := lint.LoadContext(ctx, linters, e.cfg) + lintCtx, err := lint.LoadContext(ctx, linters, e.cfg, e.log.Child("load")) if err != nil { return nil, err } - excludePatterns := e.cfg.Issues.ExcludePatterns - if e.cfg.Issues.UseDefaultExcludes { - excludePatterns = append(excludePatterns, config.GetDefaultExcludePatternsStrings()...) - } - var excludeTotalPattern string - if len(excludePatterns) != 0 { - excludeTotalPattern = fmt.Sprintf("(%s)", strings.Join(excludePatterns, "|")) - } - - skipFilesProcessor, err := processors.NewSkipFiles(e.cfg.Run.SkipFiles) + runner, err := lint.NewRunner(lintCtx.ASTCache, e.cfg, e.log.Child("runner")) if err != nil { return nil, err } - runner := lint.SimpleRunner{ - Processors: []processors.Processor{ - processors.NewPathPrettifier(), // must be before diff, nolint and exclude autogenerated processor at least - processors.NewCgo(), - skipFilesProcessor, - - processors.NewAutogeneratedExclude(lintCtx.ASTCache), - processors.NewExclude(excludeTotalPattern), - processors.NewNolint(lintCtx.ASTCache), - - processors.NewUniqByLine(), - processors.NewDiff(e.cfg.Issues.Diff, e.cfg.Issues.DiffFromRevision, e.cfg.Issues.DiffPatchFilePath), - processors.NewMaxPerFileFromLinter(), - processors.NewMaxSameIssues(e.cfg.Issues.MaxSameIssues), - processors.NewMaxFromLinter(e.cfg.Issues.MaxIssuesPerLinter), - }, - } - return runner.Run(ctx, linters, lintCtx), nil } -func setOutputToDevNull() (savedStdout, savedStderr *os.File) { +func (e *Executor) setOutputToDevNull() (savedStdout, savedStderr *os.File) { savedStdout, savedStderr = os.Stdout, os.Stderr devNull, err := os.Open(os.DevNull) if err != nil { - logrus.Warnf("can't open null device %q: %s", os.DevNull, err) + e.log.Warnf("Can't open null device %q: %s", os.DevNull, err) return } @@ -235,7 +240,7 @@ func (e *Executor) runAndPrint(ctx context.Context, args []string) error { if !logutils.HaveDebugTag("linters_output") { // Don't allow linters and loader to print anything log.SetOutput(ioutil.Discard) - savedStdout, savedStderr := setOutputToDevNull() + savedStdout, savedStderr := e.setOutputToDevNull() defer func() { os.Stdout, os.Stderr = savedStdout, savedStderr }() @@ -253,9 +258,11 @@ func (e *Executor) runAndPrint(ctx context.Context, args []string) error { p = printers.NewJSON() case config.OutFormatColoredLineNumber, config.OutFormatLineNumber: p = printers.NewText(e.cfg.Output.PrintIssuedLine, - format == config.OutFormatColoredLineNumber, e.cfg.Output.PrintLinterName) + format == config.OutFormatColoredLineNumber, e.cfg.Output.PrintLinterName, + e.log.Child("text_printer")) case config.OutFormatTab: - p = printers.NewTab(e.cfg.Output.PrintLinterName) + p = printers.NewTab(e.cfg.Output.PrintLinterName, + e.log.Child("tab_printer")) case config.OutFormatCheckstyle: p = printers.NewCheckstyle() default: @@ -288,22 +295,22 @@ func (e *Executor) executeRun(cmd *cobra.Command, args []string) { defer cancel() if needTrackResources { - go watchResources(ctx, trackResourcesEndCh) + go watchResources(ctx, trackResourcesEndCh, e.log) } if err := e.runAndPrint(ctx, args); err != nil { - logrus.Warnf("running error: %s", err) - if e.exitCode == 0 { - e.exitCode = exitCodeIfFailure + e.log.Errorf("Running error: %s", err) + if e.exitCode == exitcodes.Success { + e.exitCode = exitcodes.Failure } } - if e.exitCode == 0 && ctx.Err() != nil { - e.exitCode = exitCodeIfTimeout + if e.exitCode == exitcodes.Success && ctx.Err() != nil { + e.exitCode = exitcodes.Timeout } } -func watchResources(ctx context.Context, done chan struct{}) { +func watchResources(ctx context.Context, done chan struct{}, log logutils.Log) { startedAt := time.Now() rssValues := []uint64{} @@ -339,8 +346,8 @@ func watchResources(ctx context.Context, done chan struct{}) { const MB = 1024 * 1024 maxMB := float64(max) / MB - logrus.Infof("Memory: %d samples, avg is %.1fMB, max is %.1fMB", + log.Infof("Memory: %d samples, avg is %.1fMB, max is %.1fMB", len(rssValues), float64(avg)/MB, maxMB) - logrus.Infof("Execution took %s", time.Since(startedAt)) + log.Infof("Execution took %s", time.Since(startedAt)) close(done) } diff --git a/pkg/commands/run_config.go b/pkg/commands/run_config.go deleted file mode 100644 index 20b8bf9154e7..000000000000 --- a/pkg/commands/run_config.go +++ /dev/null @@ -1,217 +0,0 @@ -package commands - -import ( - "errors" - "fmt" - "os" - "path/filepath" - "strings" - - "github.com/golangci/golangci-lint/pkg/config" - "github.com/golangci/golangci-lint/pkg/fsutils" - "github.com/golangci/golangci-lint/pkg/logutils" - "github.com/golangci/golangci-lint/pkg/printers" - "github.com/sirupsen/logrus" - "github.com/spf13/pflag" - "github.com/spf13/viper" -) - -func (e *Executor) parseConfigImpl() { - if err := viper.ReadInConfig(); err != nil { - if _, ok := err.(viper.ConfigFileNotFoundError); ok { - return - } - logrus.Fatalf("Can't read viper config: %s", err) - } - - usedConfigFile := viper.ConfigFileUsed() - if usedConfigFile == "" { - return - } - logrus.Infof("Used config file %s", getRelPath(usedConfigFile)) - - if err := viper.Unmarshal(&e.cfg); err != nil { - logrus.Fatalf("Can't unmarshal config by viper: %s", err) - } - - if err := e.validateConfig(); err != nil { - logrus.Fatal(err) - } - - if e.cfg.InternalTest { // just for testing purposes: to detect config file usage - fmt.Fprintln(printers.StdOut, "test") - os.Exit(0) - } -} - -func (e *Executor) validateConfig() error { - c := e.cfg - if len(c.Run.Args) != 0 { - return errors.New("option run.args in config isn't supported now") - } - - if c.Run.CPUProfilePath != "" { - return errors.New("option run.cpuprofilepath in config isn't allowed") - } - - if c.Run.MemProfilePath != "" { - return errors.New("option run.memprofilepath in config isn't allowed") - } - - if c.Run.IsVerbose { - return errors.New("can't set run.verbose option with config: only on command-line") - } - - return nil -} - -func setupConfigFileSearch(args []string) { - // skip all args ([golangci-lint, run/linters]) before files/dirs list - for len(args) != 0 { - if args[0] == "run" { - args = args[1:] - break - } - - args = args[1:] - } - - // find first file/dir arg - firstArg := "./..." - if len(args) != 0 { - firstArg = args[0] - } - - absStartPath, err := filepath.Abs(firstArg) - if err != nil { - logutils.HiddenWarnf("Can't make abs path for %q: %s", firstArg, err) - absStartPath = filepath.Clean(firstArg) - } - - // start from it - var curDir string - if fsutils.IsDir(absStartPath) { - curDir = absStartPath - } else { - curDir = filepath.Dir(absStartPath) - } - - // find all dirs from it up to the root - configSearchPaths := []string{"./"} - for { - configSearchPaths = append(configSearchPaths, curDir) - newCurDir := filepath.Dir(curDir) - if curDir == newCurDir || newCurDir == "" { - break - } - curDir = newCurDir - } - - logrus.Infof("Config search paths: %s", configSearchPaths) - viper.SetConfigName(".golangci") - for _, p := range configSearchPaths { - viper.AddConfigPath(p) - } -} - -func getRelPath(p string) string { - wd, err := os.Getwd() - if err != nil { - logutils.HiddenWarnf("Can't get wd: %s", err) - return p - } - - r, err := filepath.Rel(wd, p) - if err != nil { - logutils.HiddenWarnf("Can't make path %s relative to %s: %s", p, wd, err) - return p - } - - return r -} - -func (e *Executor) needVersionOption() bool { - return e.date != "" -} - -func 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) - - // 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. - var cfg config.Config - initFlagSet(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 err == pflag.ErrHelp { - return "", nil, err - } - - logrus.Fatalf("Can't parse args: %s", err) - } - - setupLog(cfg.Run.IsVerbose) // for `-v` to work until running of preRun function - - configFile := cfg.Run.Config - if cfg.Run.NoConfig && configFile != "" { - logrus.Fatal("can't combine option --config and --no-config") - } - - if cfg.Run.NoConfig { - return "", nil, fmt.Errorf("no need to use config") - } - - return configFile, fs.Args(), 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. - - configFile, restArgs, err := parseConfigOption() - if err != nil { - return // skippable error, e.g. --no-config - } - - if configFile != "" { - viper.SetConfigFile(configFile) - } else { - setupConfigFileSearch(restArgs) - } - - e.parseConfigImpl() -} - -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 - } - - // calling Set sets Changed to true: next Set calls will append, not overwrite - _ = f.Value.Set(strings.Join(s, ",")) - }) -} diff --git a/pkg/config/config.go b/pkg/config/config.go index 261a9f8dfa87..5423cd1ea4da 100644 --- a/pkg/config/config.go +++ b/pkg/config/config.go @@ -35,7 +35,7 @@ var DefaultExcludePatterns = []ExcludePattern{ Why: "Almost all programs ignore errors on these functions and in most cases it's ok", }, { - Pattern: "(comment on exported (method|function|type)|should have( a package)? comment|comment should be of the form)", + Pattern: "(comment on exported (method|function|type|const)|should have( a package)? comment|comment should be of the form)", Linter: "golint", Why: "Annoying issue about not having a comment. The rare codebase has such comments", }, diff --git a/pkg/config/reader.go b/pkg/config/reader.go new file mode 100644 index 000000000000..a5538c6f3a31 --- /dev/null +++ b/pkg/config/reader.go @@ -0,0 +1,193 @@ +package config + +import ( + "errors" + "fmt" + "os" + "path/filepath" + + "github.com/golangci/golangci-lint/pkg/fsutils" + "github.com/golangci/golangci-lint/pkg/logutils" + "github.com/golangci/golangci-lint/pkg/printers" + "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 +} + +func NewFileReader(toCfg *Config, log logutils.Log, flagSetInit FlagSetInit) *FileReader { + return &FileReader{ + log: log, + cfg: toCfg, + flagSetInit: flagSetInit, + } +} + +func (r *FileReader) Read() error { + // XXX: hack with double parsing for 2 purposes: + // 1. to access "config" option here. + // 2. to give config less priority than command line. + + configFile, restArgs, err := r.parseConfigOption() + if err != nil { + if err == errConfigDisabled || err == pflag.ErrHelp { + return nil + } + + return fmt.Errorf("can't parse --config option: %s", err) + } + + if configFile != "" { + viper.SetConfigFile(configFile) + } else { + r.setupConfigFileSearch(restArgs) + } + + return r.parseConfig() +} + +func (r *FileReader) parseConfig() error { + if err := viper.ReadInConfig(); err != nil { + if _, ok := err.(viper.ConfigFileNotFoundError); ok { + return nil + } + + return fmt.Errorf("can't read viper config: %s", err) + } + + usedConfigFile := viper.ConfigFileUsed() + if usedConfigFile == "" { + return nil + } + + usedConfigFile, err := fsutils.ShortestRelPath(usedConfigFile, "") + if err != nil { + r.log.Warnf("Can't pretty print config file path: %s", err) + } + r.log.Infof("Used config file %s", usedConfigFile) + + if err := viper.Unmarshal(r.cfg); err != nil { + return fmt.Errorf("can't unmarshal config by viper: %s", err) + } + + if err := r.validateConfig(); err != nil { + return fmt.Errorf("can't validate config: %s", err) + } + + if r.cfg.InternalTest { // just for testing purposes: to detect config file usage + fmt.Fprintln(printers.StdOut, "test") + os.Exit(0) + } + + return nil +} + +func (r *FileReader) validateConfig() error { + c := r.cfg + if len(c.Run.Args) != 0 { + return errors.New("option run.args in config isn't supported now") + } + + if c.Run.CPUProfilePath != "" { + return errors.New("option run.cpuprofilepath in config isn't allowed") + } + + if c.Run.MemProfilePath != "" { + return errors.New("option run.memprofilepath in config isn't allowed") + } + + if c.Run.IsVerbose { + return errors.New("can't set run.verbose option with config: only on command-line") + } + + return nil +} + +func (r *FileReader) setupConfigFileSearch(args []string) { + // skip all args ([golangci-lint, run/linters]) before files/dirs list + for len(args) != 0 { + if args[0] == "run" { + args = args[1:] + break + } + + args = args[1:] + } + + // find first file/dir arg + firstArg := "./..." + if len(args) != 0 { + firstArg = args[0] + } + + absStartPath, err := filepath.Abs(firstArg) + if err != nil { + r.log.Warnf("Can't make abs path for %q: %s", firstArg, err) + absStartPath = filepath.Clean(firstArg) + } + + // start from it + var curDir string + if fsutils.IsDir(absStartPath) { + curDir = absStartPath + } else { + curDir = filepath.Dir(absStartPath) + } + + // find all dirs from it up to the root + configSearchPaths := []string{"./"} + for { + configSearchPaths = append(configSearchPaths, curDir) + newCurDir := filepath.Dir(curDir) + if curDir == newCurDir || newCurDir == "" { + break + } + curDir = newCurDir + } + + r.log.Infof("Config search paths: %s", configSearchPaths) + viper.SetConfigName(".golangci") + for _, p := range configSearchPaths { + viper.AddConfigPath(p) + } +} + +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) + } + + // 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") + } + + if cfg.Run.NoConfig { + return "", nil, errConfigDisabled + } + + return configFile, fs.Args(), nil +} diff --git a/pkg/exitcodes/exitcodes.go b/pkg/exitcodes/exitcodes.go new file mode 100644 index 000000000000..5ae21906f57f --- /dev/null +++ b/pkg/exitcodes/exitcodes.go @@ -0,0 +1,9 @@ +package exitcodes + +const ( + Success = 0 + IssuesFound = 1 + WarningInTest = 2 + Failure = 3 + Timeout = 4 +) diff --git a/pkg/fsutils/fsutils.go b/pkg/fsutils/fsutils.go index 4c3a0db1bc8e..75cca99bd410 100644 --- a/pkg/fsutils/fsutils.go +++ b/pkg/fsutils/fsutils.go @@ -1,10 +1,40 @@ package fsutils import ( + "fmt" "os" + "path/filepath" ) func IsDir(filename string) bool { fi, err := os.Stat(filename) return err == nil && fi.IsDir() } + +func ShortestRelPath(path string, wd string) (string, error) { + if wd == "" { // get it if user don't have cached working dir + var err error + wd, err = os.Getwd() + if err != nil { + return "", fmt.Errorf("can't get working directory: %s", err) + } + } + + // make path absolute and then relative to be able to fix this case: + // we'are in /test dir, we want to normalize ../test, and have file file.go in this dir; + // it must have normalized path file.go, not ../test/file.go, + var absPath string + if filepath.IsAbs(path) { + absPath = path + } else { + absPath = filepath.Join(wd, path) + } + + relPath, err := filepath.Rel(wd, absPath) + if err != nil { + return "", fmt.Errorf("can't get relative path for path %s and root %s: %s", + absPath, wd, err) + } + + return relPath, nil +} diff --git a/pkg/golinters/gas.go b/pkg/golinters/gas.go index 7c37a639f107..4f78f6b442c2 100644 --- a/pkg/golinters/gas.go +++ b/pkg/golinters/gas.go @@ -11,7 +11,6 @@ import ( "github.com/GoASTScanner/gas" "github.com/GoASTScanner/gas/rules" "github.com/golangci/golangci-lint/pkg/lint/linter" - "github.com/golangci/golangci-lint/pkg/logutils" "github.com/golangci/golangci-lint/pkg/result" ) @@ -46,7 +45,7 @@ func (lint Gas) Run(ctx context.Context, lintCtx *linter.Context) ([]result.Issu if err != nil { r = &result.Range{} if n, rerr := fmt.Sscanf(i.Line, "%d-%d", &r.From, &r.To); rerr != nil || n != 2 { - logutils.HiddenWarnf("Can't convert gas line number %q of %v to int: %s", i.Line, i, err) + lintCtx.Log.Warnf("Can't convert gas line number %q of %v to int: %s", i.Line, i, err) continue } line = r.From diff --git a/pkg/golinters/gofmt.go b/pkg/golinters/gofmt.go index 76822d3eb432..7051e5f6f349 100644 --- a/pkg/golinters/gofmt.go +++ b/pkg/golinters/gofmt.go @@ -9,8 +9,8 @@ import ( gofmtAPI "github.com/golangci/gofmt/gofmt" goimportsAPI "github.com/golangci/gofmt/goimports" "github.com/golangci/golangci-lint/pkg/lint/linter" + "github.com/golangci/golangci-lint/pkg/logutils" "github.com/golangci/golangci-lint/pkg/result" - "github.com/sirupsen/logrus" "sourcegraph.com/sourcegraph/go-diff/diff" ) @@ -55,7 +55,7 @@ func getFirstDeletedAndAddedLineNumberInHunk(h *diff.Hunk) (int, int, error) { return 0, firstAddedLineNumber, fmt.Errorf("didn't find deletion line in hunk %s", string(h.Body)) } -func (g Gofmt) extractIssuesFromPatch(patch string) ([]result.Issue, error) { +func (g Gofmt) extractIssuesFromPatch(patch string, log logutils.Log) ([]result.Issue, error) { diffs, err := diff.ParseMultiFileDiff([]byte(patch)) if err != nil { return nil, fmt.Errorf("can't parse patch: %s", err) @@ -68,7 +68,7 @@ func (g Gofmt) extractIssuesFromPatch(patch string) ([]result.Issue, error) { issues := []result.Issue{} for _, d := range diffs { if len(d.Hunks) == 0 { - logrus.Warnf("Got no hunks in diff %+v", d) + log.Warnf("Got no hunks in diff %+v", d) continue } @@ -119,7 +119,7 @@ func (g Gofmt) Run(ctx context.Context, lintCtx *linter.Context) ([]result.Issue continue } - is, err := g.extractIssuesFromPatch(string(diff)) + is, err := g.extractIssuesFromPatch(string(diff), lintCtx.Log) if err != nil { return nil, fmt.Errorf("can't extract issues from gofmt diff output %q: %s", string(diff), err) } diff --git a/pkg/golinters/golint.go b/pkg/golinters/golint.go index 32cbcfeef6b3..a230db541cfe 100644 --- a/pkg/golinters/golint.go +++ b/pkg/golinters/golint.go @@ -7,7 +7,6 @@ import ( "go/token" "github.com/golangci/golangci-lint/pkg/lint/linter" - "github.com/golangci/golangci-lint/pkg/logutils" "github.com/golangci/golangci-lint/pkg/result" lintAPI "github.com/golangci/lint-1" ) @@ -39,7 +38,7 @@ func (g Golint) Run(ctx context.Context, lintCtx *linter.Context) ([]result.Issu issues = append(issues, i...) } if lintErr != nil { - logutils.HiddenWarnf("golint: %s", lintErr) + lintCtx.Log.Warnf("Golint: %s", lintErr) } return issues, nil diff --git a/pkg/golinters/megacheck.go b/pkg/golinters/megacheck.go index 69cd53c5dc22..ac4521199bcf 100644 --- a/pkg/golinters/megacheck.go +++ b/pkg/golinters/megacheck.go @@ -8,7 +8,6 @@ import ( megacheckAPI "github.com/golangci/go-tools/cmd/megacheck" "github.com/golangci/golangci-lint/pkg/lint/linter" "github.com/golangci/golangci-lint/pkg/result" - "github.com/sirupsen/logrus" ) type Megacheck struct { @@ -57,7 +56,7 @@ func (m Megacheck) Run(ctx context.Context, lintCtx *linter.Context) ([]result.I for _, p := range lintCtx.NotCompilingPackages { packages = append(packages, p.String()) } - logrus.Warnf("Can't run megacheck because of compilation errors in packages "+ + lintCtx.Log.Warnf("Can't run megacheck because of compilation errors in packages "+ "%s: run `typecheck` linter to see errors", packages) // megacheck crashes if there are not compiling packages return nil, nil diff --git a/pkg/golinters/utils.go b/pkg/golinters/utils.go index 21dd29bed10f..d3b72e0a5396 100644 --- a/pkg/golinters/utils.go +++ b/pkg/golinters/utils.go @@ -35,7 +35,7 @@ func getASTFilesForPkg(ctx *linter.Context, pkg *packages.Package) ([]*ast.File, for _, filename := range filenames { f := ctx.ASTCache.Get(filename) if f == nil { - return nil, nil, fmt.Errorf("no AST for file %s in cache", filename) + return nil, nil, fmt.Errorf("no AST for file %s in cache: %+v", filename, *ctx.ASTCache) } if f.Err != nil { diff --git a/pkg/lint/astcache/astcache.go b/pkg/lint/astcache/astcache.go index 5415529f3698..15b9d1526fe6 100644 --- a/pkg/lint/astcache/astcache.go +++ b/pkg/lint/astcache/astcache.go @@ -8,7 +8,7 @@ import ( "os" "path/filepath" - "github.com/sirupsen/logrus" + "github.com/golangci/golangci-lint/pkg/logutils" "golang.org/x/tools/go/loader" ) @@ -20,13 +20,15 @@ type File struct { } type Cache struct { - m map[string]*File - s []*File + m map[string]*File + s []*File + log logutils.Log } -func NewCache() *Cache { +func NewCache(log logutils.Log) *Cache { return &Cache{ - m: map[string]*File{}, + m: map[string]*File{}, + log: log, } } @@ -40,7 +42,7 @@ func (c Cache) GetOrParse(filename string) *File { return f } - logrus.Infof("Parse AST for file %s on demand", filename) + c.log.Infof("Parse AST for file %s on demand", filename) c.parseFile(filename, nil) return c.m[filename] } @@ -79,7 +81,7 @@ func LoadFromProgram(prog *loader.Program) (*Cache, error) { relPath, err := filepath.Rel(root, pos.Filename) if err != nil { - logrus.Warnf("Can't get relative path for %s and %s: %s", + c.log.Warnf("Can't get relative path for %s and %s: %s", root, pos.Filename, err) continue } @@ -108,6 +110,9 @@ func (c *Cache) parseFile(filePath string, fset *token.FileSet) { Err: err, Name: filePath, } + if err != nil { + c.log.Warnf("Can't parse AST of %s: %s", filePath, err) + } } func LoadFromFiles(files []string) (*Cache, error) { diff --git a/pkg/lint/linter/context.go b/pkg/lint/linter/context.go index 7a9684e48cc2..b33593c11455 100644 --- a/pkg/lint/linter/context.go +++ b/pkg/lint/linter/context.go @@ -4,6 +4,7 @@ import ( "github.com/golangci/go-tools/ssa" "github.com/golangci/golangci-lint/pkg/config" "github.com/golangci/golangci-lint/pkg/lint/astcache" + "github.com/golangci/golangci-lint/pkg/logutils" "github.com/golangci/golangci-lint/pkg/packages" "golang.org/x/tools/go/loader" ) @@ -16,6 +17,7 @@ type Context struct { LoaderConfig *loader.Config ASTCache *astcache.Cache NotCompilingPackages []*loader.PackageInfo + Log logutils.Log } func (c *Context) Settings() *config.LintersSettings { diff --git a/pkg/lint/lintersdb/lintersdb.go b/pkg/lint/lintersdb/lintersdb.go index 67853e7c4c9a..4f35fd66e3e8 100644 --- a/pkg/lint/lintersdb/lintersdb.go +++ b/pkg/lint/lintersdb/lintersdb.go @@ -10,7 +10,7 @@ import ( "github.com/golangci/golangci-lint/pkg/config" "github.com/golangci/golangci-lint/pkg/golinters" "github.com/golangci/golangci-lint/pkg/lint/linter" - "github.com/sirupsen/logrus" + "github.com/golangci/golangci-lint/pkg/logutils" ) func AllPresets() []string { @@ -398,7 +398,7 @@ func optimizeLintersSet(linters map[string]*linter.Config) { linters[m.Name()] = &lc } -func GetEnabledLinters(cfg *config.Config) ([]linter.Config, error) { +func GetEnabledLinters(cfg *config.Config, log logutils.Log) ([]linter.Config, error) { if err := validateEnabledDisabledLintersConfig(&cfg.Linters); err != nil { return nil, err } @@ -410,21 +410,21 @@ func GetEnabledLinters(cfg *config.Config) ([]linter.Config, error) { resultLinters = append(resultLinters, *lc) } - verbosePrintLintersStatus(cfg, resultLinters) + verbosePrintLintersStatus(cfg, resultLinters, log) return resultLinters, nil } -func verbosePrintLintersStatus(cfg *config.Config, lcs []linter.Config) { +func verbosePrintLintersStatus(cfg *config.Config, lcs []linter.Config, log logutils.Log) { var linterNames []string for _, lc := range lcs { linterNames = append(linterNames, lc.Linter.Name()) } sort.StringSlice(linterNames).Sort() - logrus.Infof("Active %d linters: %s", len(linterNames), linterNames) + log.Infof("Active %d linters: %s", len(linterNames), linterNames) if len(cfg.Linters.Presets) != 0 { sort.StringSlice(cfg.Linters.Presets).Sort() - logrus.Infof("Active presets: %s", cfg.Linters.Presets) + log.Infof("Active presets: %s", cfg.Linters.Presets) } } diff --git a/pkg/lint/load.go b/pkg/lint/load.go index f97f2a7b68d5..9f089f5e0bf7 100644 --- a/pkg/lint/load.go +++ b/pkg/lint/load.go @@ -19,7 +19,6 @@ import ( "github.com/golangci/golangci-lint/pkg/lint/astcache" "github.com/golangci/golangci-lint/pkg/lint/linter" "github.com/golangci/golangci-lint/pkg/packages" - "github.com/sirupsen/logrus" "golang.org/x/tools/go/loader" ) @@ -105,7 +104,7 @@ func isLocalProjectAnalysis(args []string) bool { return true } -func getTypeCheckFuncBodies(cfg *config.Run, linters []linter.Config, pkgProg *packages.Program) func(string) bool { +func getTypeCheckFuncBodies(cfg *config.Run, linters []linter.Config, pkgProg *packages.Program, log logutils.Log) func(string) bool { if !isLocalProjectAnalysis(cfg.Args) { loadDebugf("analysis in nonlocal, don't optimize loading by not typechecking func bodies") return nil @@ -123,7 +122,7 @@ func getTypeCheckFuncBodies(cfg *config.Run, linters []linter.Config, pkgProg *p projPath, err := getCurrentProjectImportPath() if err != nil { - logrus.Infof("can't get cur project path: %s", err) + log.Infof("Can't get cur project path: %s", err) return nil } @@ -151,14 +150,14 @@ func getTypeCheckFuncBodies(cfg *config.Run, linters []linter.Config, pkgProg *p } } -func loadWholeAppIfNeeded(ctx context.Context, linters []linter.Config, cfg *config.Run, pkgProg *packages.Program) (*loader.Program, *loader.Config, error) { +func loadWholeAppIfNeeded(ctx context.Context, linters []linter.Config, cfg *config.Run, pkgProg *packages.Program, log logutils.Log) (*loader.Program, *loader.Config, error) { if !isFullImportNeeded(linters) { return nil, nil, nil } startedAt := time.Now() defer func() { - logrus.Infof("Program loading took %s", time.Since(startedAt)) + log.Infof("Program loading took %s", time.Since(startedAt)) }() bctx := pkgProg.BuildContext() @@ -166,7 +165,7 @@ func loadWholeAppIfNeeded(ctx context.Context, linters []linter.Config, cfg *con Build: bctx, AllowErrors: true, // Try to analyze partially ParserMode: parser.ParseComments, // AST will be reused by linters - TypeCheckFuncBodies: getTypeCheckFuncBodies(cfg, linters, pkgProg), + TypeCheckFuncBodies: getTypeCheckFuncBodies(cfg, linters, pkgProg, log), } var loaderArgs []string @@ -195,13 +194,22 @@ func loadWholeAppIfNeeded(ctx context.Context, linters []linter.Config, cfg *con return nil, nil, fmt.Errorf("can't load program from paths %v: %s", loaderArgs, err) } + if len(prog.InitialPackages()) == 1 { + pkg := prog.InitialPackages()[0] + var files []string + for _, f := range pkg.Files { + files = append(files, prog.Fset.Position(f.Pos()).Filename) + } + log.Infof("pkg %s files: %s", pkg, files) + } + return prog, loadcfg, nil } -func buildSSAProgram(ctx context.Context, lprog *loader.Program) *ssa.Program { +func buildSSAProgram(ctx context.Context, lprog *loader.Program, log logutils.Log) *ssa.Program { startedAt := time.Now() defer func() { - logrus.Infof("SSA repr building took %s", time.Since(startedAt)) + log.Infof("SSA repr building took %s", time.Since(startedAt)) }() ssaProg := ssautil.CreateProgram(lprog, ssa.GlobalDebug) @@ -249,10 +257,14 @@ func separateNotCompilingPackages(lintCtx *linter.Context) { } } } + + if len(lintCtx.NotCompilingPackages) != 0 { + lintCtx.Log.Infof("Not compiling packages: %+v", lintCtx.NotCompilingPackages) + } } //nolint:gocyclo -func LoadContext(ctx context.Context, linters []linter.Config, cfg *config.Config) (*linter.Context, error) { +func LoadContext(ctx context.Context, linters []linter.Config, cfg *config.Config, log logutils.Log) (*linter.Context, error) { // Set GOROOT to have working cross-compilation: cross-compiled binaries // have invalid GOROOT. XXX: can't use runtime.GOROOT(). goroot, err := discoverGoRoot() @@ -269,7 +281,7 @@ func LoadContext(ctx context.Context, linters []linter.Config, cfg *config.Confi skipDirs := append([]string{}, packages.StdExcludeDirRegexps...) skipDirs = append(skipDirs, cfg.Run.SkipDirs...) - r, err := packages.NewResolver(cfg.Run.BuildTags, skipDirs) + r, err := packages.NewResolver(cfg.Run.BuildTags, skipDirs, log.Child("path_resolver")) if err != nil { return nil, err } @@ -279,14 +291,14 @@ func LoadContext(ctx context.Context, linters []linter.Config, cfg *config.Confi return nil, err } - prog, loaderConfig, err := loadWholeAppIfNeeded(ctx, linters, &cfg.Run, pkgProg) + prog, loaderConfig, err := loadWholeAppIfNeeded(ctx, linters, &cfg.Run, pkgProg, log) if err != nil { return nil, err } var ssaProg *ssa.Program if prog != nil && isSSAReprNeeded(linters) { - ssaProg = buildSSAProgram(ctx, prog) + ssaProg = buildSSAProgram(ctx, prog, log) } var astCache *astcache.Cache @@ -306,6 +318,7 @@ func LoadContext(ctx context.Context, linters []linter.Config, cfg *config.Confi SSAProgram: ssaProg, LoaderConfig: loaderConfig, ASTCache: astCache, + Log: log, } if prog != nil { diff --git a/pkg/lint/load_test.go b/pkg/lint/load_test.go index ad231a83b17c..eaaeb700a802 100644 --- a/pkg/lint/load_test.go +++ b/pkg/lint/load_test.go @@ -4,6 +4,8 @@ import ( "context" "testing" + "github.com/golangci/golangci-lint/pkg/logutils" + "github.com/golangci/golangci-lint/pkg/config" "github.com/golangci/golangci-lint/pkg/lint/astcache" "github.com/golangci/golangci-lint/pkg/lint/linter" @@ -19,7 +21,7 @@ func TestASTCacheLoading(t *testing.T) { inputPaths := []string{"./...", "./", "./load.go", "load.go"} for _, inputPath := range inputPaths { - r, err := packages.NewResolver(nil, nil) + r, err := packages.NewResolver(nil, nil, logutils.NewStderrLog("")) assert.NoError(t, err) pkgProg, err := r.Resolve(inputPath) @@ -30,7 +32,7 @@ func TestASTCacheLoading(t *testing.T) { prog, _, err := loadWholeAppIfNeeded(ctx, linters, &config.Run{ AnalyzeTests: true, - }, pkgProg) + }, pkgProg, logutils.NewStderrLog("")) assert.NoError(t, err) astCacheFromProg, err := astcache.LoadFromProgram(prog) diff --git a/pkg/lint/runner.go b/pkg/lint/runner.go index e15afc740dfd..1f7a90f4c20b 100644 --- a/pkg/lint/runner.go +++ b/pkg/lint/runner.go @@ -9,16 +9,55 @@ import ( "sync" "time" + "github.com/golangci/golangci-lint/pkg/config" + "github.com/golangci/golangci-lint/pkg/lint/astcache" "github.com/golangci/golangci-lint/pkg/lint/linter" "github.com/golangci/golangci-lint/pkg/logutils" "github.com/golangci/golangci-lint/pkg/result" "github.com/golangci/golangci-lint/pkg/result/processors" "github.com/golangci/golangci-lint/pkg/timeutils" - "github.com/sirupsen/logrus" ) -type SimpleRunner struct { +type Runner struct { Processors []processors.Processor + Log logutils.Log +} + +func NewRunner(astCache *astcache.Cache, cfg *config.Config, log logutils.Log) (*Runner, error) { + icfg := cfg.Issues + excludePatterns := icfg.ExcludePatterns + if icfg.UseDefaultExcludes { + excludePatterns = append(excludePatterns, config.GetDefaultExcludePatternsStrings()...) + } + + var excludeTotalPattern string + if len(excludePatterns) != 0 { + excludeTotalPattern = fmt.Sprintf("(%s)", strings.Join(excludePatterns, "|")) + } + + skipFilesProcessor, err := processors.NewSkipFiles(cfg.Run.SkipFiles) + if err != nil { + return nil, err + } + + return &Runner{ + Processors: []processors.Processor{ + processors.NewPathPrettifier(), // must be before diff, nolint and exclude autogenerated processor at least + processors.NewCgo(), + skipFilesProcessor, + + processors.NewAutogeneratedExclude(astCache), + processors.NewExclude(excludeTotalPattern), + processors.NewNolint(astCache), + + processors.NewUniqByLine(), + processors.NewDiff(icfg.Diff, icfg.DiffFromRevision, icfg.DiffPatchFilePath), + processors.NewMaxPerFileFromLinter(), + processors.NewMaxSameIssues(icfg.MaxSameIssues, log.Child("max_same_issues")), + processors.NewMaxFromLinter(icfg.MaxIssuesPerLinter, log.Child("max_from_linter")), + }, + Log: log, + }, nil } type lintRes struct { @@ -27,15 +66,17 @@ type lintRes struct { issues []result.Issue } -func runLinterSafe(ctx context.Context, lintCtx *linter.Context, lc linter.Config) (ret []result.Issue, err error) { +func (r Runner) runLinterSafe(ctx context.Context, lintCtx *linter.Context, lc linter.Config) (ret []result.Issue, err error) { defer func() { if panicData := recover(); panicData != nil { err = fmt.Errorf("panic occured: %s", panicData) - logutils.HiddenWarnf("Panic stack trace: %s", debug.Stack()) + r.Log.Warnf("Panic stack trace: %s", debug.Stack()) } }() - issues, err := lc.Linter.Run(ctx, lintCtx) + specificLintCtx := *lintCtx + specificLintCtx.Log = lintCtx.Log.Child(lc.Linter.Name()) + issues, err := lc.Linter.Run(ctx, &specificLintCtx) if err != nil { return nil, err } @@ -47,8 +88,8 @@ func runLinterSafe(ctx context.Context, lintCtx *linter.Context, lc linter.Confi return issues, nil } -func runWorker(ctx context.Context, lintCtx *linter.Context, tasksCh <-chan linter.Config, lintResultsCh chan<- lintRes, name string) { - sw := timeutils.NewStopwatch(name) +func (r Runner) runWorker(ctx context.Context, lintCtx *linter.Context, tasksCh <-chan linter.Config, lintResultsCh chan<- lintRes, name string) { + sw := timeutils.NewStopwatch(name, r.Log) defer sw.Print() for { @@ -67,7 +108,7 @@ func runWorker(ctx context.Context, lintCtx *linter.Context, tasksCh <-chan lint var issues []result.Issue var err error sw.TrackStage(lc.Linter.Name(), func() { - issues, err = runLinterSafe(ctx, lintCtx, lc) + issues, err = r.runLinterSafe(ctx, lintCtx, lc) }) lintResultsCh <- lintRes{ linter: lc, @@ -78,7 +119,7 @@ func runWorker(ctx context.Context, lintCtx *linter.Context, tasksCh <-chan lint } } -func logWorkersStat(workersFinishTimes []time.Time) { +func (r Runner) logWorkersStat(workersFinishTimes []time.Time) { lastFinishTime := workersFinishTimes[0] for _, t := range workersFinishTimes { if t.After(lastFinishTime) { @@ -95,7 +136,7 @@ func logWorkersStat(workersFinishTimes []time.Time) { logStrings = append(logStrings, fmt.Sprintf("#%d: %s", i+1, lastFinishTime.Sub(t))) } - logrus.Infof("Workers idle times: %s", strings.Join(logStrings, ", ")) + r.Log.Infof("Workers idle times: %s", strings.Join(logStrings, ", ")) } func getSortedLintersConfigs(linters []linter.Config) []linter.Config { @@ -109,7 +150,7 @@ func getSortedLintersConfigs(linters []linter.Config) []linter.Config { return ret } -func (r *SimpleRunner) runWorkers(ctx context.Context, lintCtx *linter.Context, linters []linter.Config) <-chan lintRes { +func (r *Runner) runWorkers(ctx context.Context, lintCtx *linter.Context, linters []linter.Config) <-chan lintRes { tasksCh := make(chan linter.Config, len(linters)) lintResultsCh := make(chan lintRes, len(linters)) var wg sync.WaitGroup @@ -121,7 +162,7 @@ func (r *SimpleRunner) runWorkers(ctx context.Context, lintCtx *linter.Context, go func(i int) { defer wg.Done() name := fmt.Sprintf("worker.%d", i+1) - runWorker(ctx, lintCtx, tasksCh, lintResultsCh, name) + r.runWorker(ctx, lintCtx, tasksCh, lintResultsCh, name) workersFinishTimes[i] = time.Now() }(i) } @@ -136,23 +177,23 @@ func (r *SimpleRunner) runWorkers(ctx context.Context, lintCtx *linter.Context, wg.Wait() close(lintResultsCh) - logWorkersStat(workersFinishTimes) + r.logWorkersStat(workersFinishTimes) }() return lintResultsCh } -func (r SimpleRunner) processLintResults(ctx context.Context, inCh <-chan lintRes) <-chan lintRes { +func (r Runner) processLintResults(ctx context.Context, inCh <-chan lintRes) <-chan lintRes { outCh := make(chan lintRes, 64) go func() { - sw := timeutils.NewStopwatch("processing") + sw := timeutils.NewStopwatch("processing", r.Log) defer close(outCh) for res := range inCh { if res.err != nil { - logutils.HiddenWarnf("Can't run linter %s: %s", res.linter.Linter.Name(), res.err) + r.Log.Warnf("Can't run linter %s: %s", res.linter.Linter.Name(), res.err) continue } @@ -195,7 +236,7 @@ func collectIssues(ctx context.Context, resCh <-chan lintRes) <-chan result.Issu return retIssues } -func (r SimpleRunner) Run(ctx context.Context, linters []linter.Config, lintCtx *linter.Context) <-chan result.Issue { +func (r Runner) Run(ctx context.Context, linters []linter.Config, lintCtx *linter.Context) <-chan result.Issue { lintResultsCh := r.runWorkers(ctx, lintCtx, linters) processedLintResultsCh := r.processLintResults(ctx, lintResultsCh) if ctx.Err() != nil { @@ -205,14 +246,14 @@ func (r SimpleRunner) Run(ctx context.Context, linters []linter.Config, lintCtx finishedLintersN++ } - logrus.Warnf("%d/%d linters finished: deadline exceeded: try increase it by passing --deadline option", + r.Log.Errorf("%d/%d linters finished: deadline exceeded: try increase it by passing --deadline option", finishedLintersN, len(linters)) } return collectIssues(ctx, processedLintResultsCh) } -func (r *SimpleRunner) processIssues(ctx context.Context, issues []result.Issue, sw *timeutils.Stopwatch) []result.Issue { +func (r *Runner) processIssues(ctx context.Context, issues []result.Issue, sw *timeutils.Stopwatch) []result.Issue { for _, p := range r.Processors { var newIssues []result.Issue var err error @@ -221,7 +262,7 @@ func (r *SimpleRunner) processIssues(ctx context.Context, issues []result.Issue, }) if err != nil { - logutils.HiddenWarnf("Can't process result by %s processor: %s", p.Name(), err) + r.Log.Warnf("Can't process result by %s processor: %s", p.Name(), err) } else { issues = newIssues } diff --git a/pkg/logutils/log.go b/pkg/logutils/log.go new file mode 100644 index 000000000000..e87cbee9d6fe --- /dev/null +++ b/pkg/logutils/log.go @@ -0,0 +1,30 @@ +package logutils + +type Log interface { + Fatalf(format string, args ...interface{}) + Errorf(format string, args ...interface{}) + Warnf(format string, args ...interface{}) + Infof(format string, args ...interface{}) + + Child(name string) Log + SetLevel(level LogLevel) +} + +type LogLevel int + +const ( + // debug message, write to debug logs only by logutils.Debug + LogLevelDebug LogLevel = 0 + + // information messages, don't write too much messages, + // only useful ones: they are shown when running with -v + LogLevelInfo LogLevel = 1 + + // hidden errors: non critical errors: work can be continued, no need to fail whole program; + // tests will crash if any warning occured. + LogLevelWarn LogLevel = 2 + + // only not hidden from user errors: whole program failing, usually + // error logging happens in 1-2 places: in the "main" function. + LogLevelError LogLevel = 3 +) diff --git a/pkg/logutils/logutils.go b/pkg/logutils/logutils.go index 94b2351eb277..9ba7bdfff1af 100644 --- a/pkg/logutils/logutils.go +++ b/pkg/logutils/logutils.go @@ -3,20 +3,8 @@ package logutils import ( "os" "strings" - - "github.com/sirupsen/logrus" ) -var isGolangCIRun = os.Getenv("GOLANGCI_COM_RUN") == "1" - -func HiddenWarnf(format string, args ...interface{}) { - if isGolangCIRun { - logrus.Warnf(format, args...) - } else { - logrus.Infof(format, args...) - } -} - func getEnabledDebugs() map[string]bool { ret := map[string]bool{} debugVar := os.Getenv("GL_DEBUG") @@ -43,8 +31,9 @@ func Debug(tag string) DebugFunc { } return func(format string, args ...interface{}) { - newArgs := append([]interface{}{tag}, args...) - logrus.Debugf("%s: "+format, newArgs...) + logger := NewStderrLog(tag) + logger.SetLevel(LogLevelDebug) + logger.Debugf(format, args...) } } @@ -55,3 +44,9 @@ func IsDebugEnabled() bool { func HaveDebugTag(tag string) bool { return enabledDebugs[tag] } + +func SetupVerboseLog(log Log, isVerbose bool) { + if isVerbose { + log.SetLevel(LogLevelInfo) + } +} diff --git a/pkg/logutils/stderr_log.go b/pkg/logutils/stderr_log.go new file mode 100644 index 000000000000..08b8d77ab5ae --- /dev/null +++ b/pkg/logutils/stderr_log.go @@ -0,0 +1,106 @@ +package logutils + +import ( + "fmt" + "os" + + "github.com/golangci/golangci-lint/pkg/exitcodes" + "github.com/sirupsen/logrus" //nolint:depguard +) + +var isTestRun = os.Getenv("GL_TEST_RUN") == "1" + +type StderrLog struct { + name string + logger *logrus.Logger + level LogLevel +} + +var _ Log = NewStderrLog("") + +func NewStderrLog(name string) *StderrLog { + sl := &StderrLog{ + name: name, + logger: logrus.New(), + level: LogLevelWarn, + } + + // control log level in logutils, not in logrus + sl.logger.SetLevel(logrus.DebugLevel) + sl.logger.Formatter = &logrus.TextFormatter{ + DisableTimestamp: true, // `INFO[0007] msg` -> `INFO msg` + } + return sl +} + +func exitIfTest() { + if isTestRun { + os.Exit(exitcodes.WarningInTest) + } +} + +func (sl StderrLog) prefix() string { + prefix := "" + if sl.name != "" { + prefix = fmt.Sprintf("[%s] ", sl.name) + } + + return prefix +} + +func (sl StderrLog) Fatalf(format string, args ...interface{}) { + sl.logger.Errorf("%s%s", sl.prefix(), fmt.Sprintf(format, args...)) + os.Exit(exitcodes.Failure) +} + +func (sl StderrLog) Errorf(format string, args ...interface{}) { + if sl.level > LogLevelError { + return + } + + sl.logger.Errorf("%s%s", sl.prefix(), fmt.Sprintf(format, args...)) + // don't call exitIfTest() because the idea is to + // crash on hidden errors (warnings); but Errorf MUST NOT be + // called on hidden errors, see log levels comments. +} + +func (sl StderrLog) Warnf(format string, args ...interface{}) { + if sl.level > LogLevelWarn { + return + } + + sl.logger.Warnf("%s%s", sl.prefix(), fmt.Sprintf(format, args...)) + exitIfTest() +} + +func (sl StderrLog) Infof(format string, args ...interface{}) { + if sl.level > LogLevelInfo { + return + } + + sl.logger.Infof("%s%s", sl.prefix(), fmt.Sprintf(format, args...)) +} + +func (sl StderrLog) Debugf(format string, args ...interface{}) { + if sl.level > LogLevelDebug { + return + } + + sl.logger.Debugf("%s%s", sl.prefix(), fmt.Sprintf(format, args...)) +} + +func (sl StderrLog) Child(name string) Log { + prefix := "" + if sl.name != "" { + prefix = sl.name + ":" + } + + child := sl + child.name = prefix + name + + return &child +} + +func (sl *StderrLog) SetLevel(level LogLevel) { + sl.level = level +} diff --git a/pkg/packages/resolver.go b/pkg/packages/resolver.go index fcbedd9fcd88..d224e665cee7 100644 --- a/pkg/packages/resolver.go +++ b/pkg/packages/resolver.go @@ -10,7 +10,8 @@ import ( "strings" "time" - "github.com/sirupsen/logrus" + "github.com/golangci/golangci-lint/pkg/fsutils" + "github.com/golangci/golangci-lint/pkg/logutils" ) type Resolver struct { @@ -18,9 +19,12 @@ type Resolver struct { buildTags []string skippedDirs []string + log logutils.Log + + wd string // working directory } -func NewResolver(buildTags, excludeDirs []string) (*Resolver, error) { +func NewResolver(buildTags, excludeDirs []string, log logutils.Log) (*Resolver, error) { excludeDirsMap := map[string]*regexp.Regexp{} for _, dir := range excludeDirs { re, err := regexp.Compile(dir) @@ -31,9 +35,16 @@ func NewResolver(buildTags, excludeDirs []string) (*Resolver, error) { excludeDirsMap[dir] = re } + wd, err := os.Getwd() + if err != nil { + return nil, fmt.Errorf("can't get working dir: %s", err) + } + return &Resolver{ excludeDirs: excludeDirsMap, buildTags: buildTags, + log: log, + wd: wd, }, nil } @@ -80,6 +91,15 @@ func (r *Resolver) resolveRecursively(root string, prog *Program) error { subdir := filepath.Join(root, fi.Name()) + // Normalize each subdir because working directory can be one of these subdirs: + // working dir = /app/subdir, resolve root is ../, without this normalization + // path of subdir will be "../subdir" but it must be ".". + // Normalize path before checking is ignored dir. + subdir, err := r.normalizePath(subdir) + if err != nil { + return err + } + if r.isIgnoredDir(subdir) { r.skippedDirs = append(r.skippedDirs, subdir) continue @@ -128,7 +148,7 @@ func (r Resolver) addFakePackage(filePath string, prog *Program) { func (r Resolver) Resolve(paths ...string) (prog *Program, err error) { startedAt := time.Now() defer func() { - logrus.Infof("Paths resolving took %s: %s", time.Since(startedAt), prog) + r.log.Infof("Paths resolving took %s: %s", time.Since(startedAt), prog) }() if len(paths) == 0 { @@ -141,25 +161,24 @@ func (r Resolver) Resolve(paths ...string) (prog *Program, err error) { bctx: bctx, } - root, err := os.Getwd() - if err != nil { - return nil, fmt.Errorf("can't get working dir: %s", err) - } - for _, path := range paths { - if err := r.resolvePath(path, prog, root); err != nil { + if err := r.resolvePath(path, prog); err != nil { return nil, err } } if len(r.skippedDirs) != 0 { - logrus.Infof("Skipped dirs: %s", r.skippedDirs) + r.log.Infof("Skipped dirs: %s", r.skippedDirs) } return prog, nil } -func (r *Resolver) resolvePath(path string, prog *Program, root string) error { +func (r *Resolver) normalizePath(path string) (string, error) { + return fsutils.ShortestRelPath(path, r.wd) +} + +func (r *Resolver) resolvePath(path string, prog *Program) error { needRecursive := strings.HasSuffix(path, "/...") if needRecursive { path = filepath.Dir(path) @@ -171,14 +190,9 @@ func (r *Resolver) resolvePath(path string, prog *Program, root string) error { } path = evalPath - if filepath.IsAbs(path) { - var relPath string - relPath, err = filepath.Rel(root, path) - if err != nil { - return fmt.Errorf("can't get relative path for path %s and root %s: %s", - path, root, err) - } - path = relPath + path, err = r.normalizePath(path) + if err != nil { + return err } if needRecursive { diff --git a/pkg/packages/resolver_test.go b/pkg/packages/resolver_test.go index f5adf45ddd7d..083c39f57f07 100644 --- a/pkg/packages/resolver_test.go +++ b/pkg/packages/resolver_test.go @@ -8,6 +8,7 @@ import ( "strings" "testing" + "github.com/golangci/golangci-lint/pkg/logutils" "github.com/golangci/golangci-lint/pkg/packages" "github.com/stretchr/testify/assert" ) @@ -57,7 +58,7 @@ func prepareFS(t *testing.T, paths ...string) *fsPreparer { } func newTestResolver(t *testing.T, excludeDirs []string) *packages.Resolver { - r, err := packages.NewResolver(nil, excludeDirs) + r, err := packages.NewResolver(nil, excludeDirs, logutils.NewStderrLog("")) assert.NoError(t, err) return r diff --git a/pkg/printers/tab.go b/pkg/printers/tab.go index c677d480496c..7b0e4c04e0c3 100644 --- a/pkg/printers/tab.go +++ b/pkg/printers/tab.go @@ -7,17 +7,19 @@ import ( "text/tabwriter" "github.com/fatih/color" + "github.com/golangci/golangci-lint/pkg/logutils" "github.com/golangci/golangci-lint/pkg/result" - "github.com/sirupsen/logrus" ) type Tab struct { printLinterName bool + log logutils.Log } -func NewTab(printLinterName bool) *Tab { +func NewTab(printLinterName bool, log logutils.Log) *Tab { return &Tab{ printLinterName: printLinterName, + log: log, } } @@ -36,14 +38,14 @@ func (p *Tab) Print(ctx context.Context, issues <-chan result.Issue) (bool, erro } if issuesN != 0 { - logrus.Infof("Found %d issues", issuesN) + p.log.Infof("Found %d issues", issuesN) } else if ctx.Err() == nil { // don't print "congrats" if timeouted outStr := p.SprintfColored(color.FgGreen, "Congrats! No issues were found.") fmt.Fprintln(StdOut, outStr) } if err := w.Flush(); err != nil { - logrus.Warnf("Can't flush tab writer: %s", err) + p.log.Warnf("Can't flush tab writer: %s", err) } return issuesN != 0, nil diff --git a/pkg/printers/text.go b/pkg/printers/text.go index 6d4c33298e52..c291265a3a97 100644 --- a/pkg/printers/text.go +++ b/pkg/printers/text.go @@ -9,8 +9,8 @@ import ( "time" "github.com/fatih/color" + "github.com/golangci/golangci-lint/pkg/logutils" "github.com/golangci/golangci-lint/pkg/result" - "github.com/sirupsen/logrus" ) type linesCache [][]byte @@ -22,14 +22,16 @@ type Text struct { printLinterName bool cache filesCache + log logutils.Log } -func NewText(printIssuedLine, useColors, printLinterName bool) *Text { +func NewText(printIssuedLine, useColors, printLinterName bool, log logutils.Log) *Text { return &Text{ printIssuedLine: printIssuedLine, useColors: useColors, printLinterName: printLinterName, cache: filesCache{}, + log: log, } } @@ -62,7 +64,7 @@ func (p *Text) getFileLinesForIssue(i *result.Issue) (linesCache, error) { func (p *Text) Print(ctx context.Context, issues <-chan result.Issue) (bool, error) { var issuedLineExtractingDuration time.Duration defer func() { - logrus.Infof("Extracting issued lines took %s", issuedLineExtractingDuration) + p.log.Infof("Extracting issued lines took %s", issuedLineExtractingDuration) }() issuesN := 0 @@ -88,7 +90,7 @@ func (p *Text) Print(ctx context.Context, issues <-chan result.Issue) (bool, err } if issuesN != 0 { - logrus.Infof("Found %d issues", issuesN) + p.log.Infof("Found %d issues", issuesN) } else if ctx.Err() == nil { // don't print "congrats" if timeouted outStr := p.SprintfColored(color.FgGreen, "Congrats! No issues were found.") fmt.Fprintln(StdOut, outStr) @@ -119,7 +121,7 @@ func (p Text) printIssuedLines(i *result.Issue, lines linesCache) { zeroIndexedLine := line - 1 if zeroIndexedLine >= len(lines) { - logrus.Warnf("No line %d in file %s", line, i.FilePath()) + p.log.Warnf("No line %d in file %s", line, i.FilePath()) break } diff --git a/pkg/result/processors/max_from_linter.go b/pkg/result/processors/max_from_linter.go index 67e9873fe086..f669cab22475 100644 --- a/pkg/result/processors/max_from_linter.go +++ b/pkg/result/processors/max_from_linter.go @@ -1,21 +1,23 @@ package processors import ( + "github.com/golangci/golangci-lint/pkg/logutils" "github.com/golangci/golangci-lint/pkg/result" - "github.com/sirupsen/logrus" ) type MaxFromLinter struct { lc linterToCountMap limit int + log logutils.Log } var _ Processor = &MaxFromLinter{} -func NewMaxFromLinter(limit int) *MaxFromLinter { +func NewMaxFromLinter(limit int, log logutils.Log) *MaxFromLinter { return &MaxFromLinter{ lc: linterToCountMap{}, limit: limit, + log: log, } } @@ -37,7 +39,7 @@ func (p *MaxFromLinter) Process(issues []result.Issue) ([]result.Issue, error) { func (p MaxFromLinter) Finish() { walkStringToIntMapSortedByValue(p.lc, func(linter string, count int) { if count > p.limit { - logrus.Infof("%d/%d issues from linter %s were hidden, use --max-issues-per-linter", + p.log.Infof("%d/%d issues from linter %s were hidden, use --max-issues-per-linter", count-p.limit, count, linter) } }) diff --git a/pkg/result/processors/max_from_linter_test.go b/pkg/result/processors/max_from_linter_test.go index 05ffb632309d..15ccb8bb7d05 100644 --- a/pkg/result/processors/max_from_linter_test.go +++ b/pkg/result/processors/max_from_linter_test.go @@ -2,10 +2,12 @@ package processors import ( "testing" + + "github.com/golangci/golangci-lint/pkg/logutils" ) func TestMaxFromLinter(t *testing.T) { - p := NewMaxFromLinter(1) + p := NewMaxFromLinter(1, logutils.NewStderrLog("")) gosimple := newFromLinterIssue("gosimple") gofmt := newFromLinterIssue("gofmt") processAssertSame(t, p, gosimple) // ok diff --git a/pkg/result/processors/max_same_issues.go b/pkg/result/processors/max_same_issues.go index 48e98e300fd6..b59b7a3227da 100644 --- a/pkg/result/processors/max_same_issues.go +++ b/pkg/result/processors/max_same_issues.go @@ -3,8 +3,8 @@ package processors import ( "sort" + "github.com/golangci/golangci-lint/pkg/logutils" "github.com/golangci/golangci-lint/pkg/result" - "github.com/sirupsen/logrus" ) type textToCountMap map[string]int @@ -12,14 +12,16 @@ type textToCountMap map[string]int type MaxSameIssues struct { tc textToCountMap limit int + log logutils.Log } var _ Processor = &MaxSameIssues{} -func NewMaxSameIssues(limit int) *MaxSameIssues { +func NewMaxSameIssues(limit int, log logutils.Log) *MaxSameIssues { return &MaxSameIssues{ tc: textToCountMap{}, limit: limit, + log: log, } } @@ -41,7 +43,7 @@ func (p *MaxSameIssues) Process(issues []result.Issue) ([]result.Issue, error) { func (p MaxSameIssues) Finish() { walkStringToIntMapSortedByValue(p.tc, func(text string, count int) { if count > p.limit { - logrus.Infof("%d/%d issues with text %q were hidden, use --max-same-issues", + p.log.Infof("%d/%d issues with text %q were hidden, use --max-same-issues", count-p.limit, count, text) } }) diff --git a/pkg/result/processors/max_same_issues_test.go b/pkg/result/processors/max_same_issues_test.go index 7f3486e2910c..3fad8fd57be3 100644 --- a/pkg/result/processors/max_same_issues_test.go +++ b/pkg/result/processors/max_same_issues_test.go @@ -3,11 +3,12 @@ package processors import ( "testing" + "github.com/golangci/golangci-lint/pkg/logutils" "github.com/golangci/golangci-lint/pkg/result" ) func TestMaxSameIssues(t *testing.T) { - p := NewMaxSameIssues(1) + p := NewMaxSameIssues(1, logutils.NewStderrLog("")) i1 := result.Issue{ Text: "1", } diff --git a/pkg/result/processors/nolint_test.go b/pkg/result/processors/nolint_test.go index de255fa2ba37..36299d89880c 100644 --- a/pkg/result/processors/nolint_test.go +++ b/pkg/result/processors/nolint_test.go @@ -6,6 +6,7 @@ import ( "testing" "github.com/golangci/golangci-lint/pkg/lint/astcache" + "github.com/golangci/golangci-lint/pkg/logutils" "github.com/golangci/golangci-lint/pkg/result" "github.com/stretchr/testify/assert" ) @@ -21,7 +22,7 @@ func newNolintFileIssue(line int, fromLinter string) result.Issue { } func TestNolint(t *testing.T) { - p := NewNolint(astcache.NewCache()) + p := NewNolint(astcache.NewCache(logutils.NewStderrLog(""))) // test inline comments processAssertEmpty(t, p, newNolintFileIssue(3, "gofmt")) diff --git a/pkg/timeutils/stopwatch.go b/pkg/timeutils/stopwatch.go index e17deaf616f6..4390bf35a937 100644 --- a/pkg/timeutils/stopwatch.go +++ b/pkg/timeutils/stopwatch.go @@ -7,22 +7,24 @@ import ( "sync" "time" - "github.com/sirupsen/logrus" + "github.com/golangci/golangci-lint/pkg/logutils" ) type Stopwatch struct { name string startedAt time.Time stages map[string]time.Duration + log logutils.Log sync.Mutex } -func NewStopwatch(name string) *Stopwatch { +func NewStopwatch(name string, log logutils.Log) *Stopwatch { return &Stopwatch{ name: name, startedAt: time.Now(), stages: map[string]time.Duration{}, + log: log, } } @@ -53,11 +55,11 @@ func (s *Stopwatch) sprintStages() string { func (s *Stopwatch) Print() { p := fmt.Sprintf("%s took %s", s.name, time.Since(s.startedAt)) if len(s.stages) == 0 { - logrus.Info(p) + s.log.Infof("%s", p) return } - logrus.Infof("%s with %s", p, s.sprintStages()) + s.log.Infof("%s with %s", p, s.sprintStages()) } func (s *Stopwatch) PrintStages() { @@ -65,7 +67,7 @@ func (s *Stopwatch) PrintStages() { for _, s := range s.stages { stagesDuration += s } - logrus.Infof("%s took %s with %s", s.name, stagesDuration, s.sprintStages()) + s.log.Infof("%s took %s with %s", s.name, stagesDuration, s.sprintStages()) } func (s *Stopwatch) TrackStage(name string, f func()) { diff --git a/pkg/timeutils/track.go b/pkg/timeutils/track.go index 279e4e7b4d65..ada1b9a4a7c2 100644 --- a/pkg/timeutils/track.go +++ b/pkg/timeutils/track.go @@ -4,9 +4,9 @@ import ( "fmt" "time" - "github.com/sirupsen/logrus" + "github.com/golangci/golangci-lint/pkg/logutils" ) -func Track(from time.Time, format string, args ...interface{}) { - logrus.Infof("%s took %s", fmt.Sprintf(format, args...), time.Since(from)) +func Track(from time.Time, log logutils.Log, format string, args ...interface{}) { + log.Infof("%s took %s", fmt.Sprintf(format, args...), time.Since(from)) } diff --git a/test/bench_test.go b/test/bench_test.go index bc2c85503c94..31d8659e7a00 100644 --- a/test/bench_test.go +++ b/test/bench_test.go @@ -4,6 +4,7 @@ import ( "bytes" "fmt" "go/build" + "log" "os" "os/exec" "path/filepath" @@ -15,7 +16,6 @@ import ( "github.com/golangci/golangci-lint/pkg/config" gops "github.com/mitchellh/go-ps" "github.com/shirou/gopsutil/process" - "github.com/sirupsen/logrus" ) func chdir(b *testing.B, dir string) { @@ -90,7 +90,7 @@ func printCommand(cmd string, args ...string) { quotedArgs = append(quotedArgs, strconv.Quote(a)) } - logrus.Warnf("%s %s", cmd, strings.Join(quotedArgs, " ")) + log.Printf("%s %s", cmd, strings.Join(quotedArgs, " ")) } func runGometalinter(b *testing.B) { @@ -215,7 +215,7 @@ func compare(b *testing.B, gometalinterRun, golangciLintRun func(*testing.B), re if mode != "" { mode = " " + mode } - logrus.Warnf("%s (%d kLoC): golangci-lint%s: time: %s, %.1f times faster; memory: %dMB, %.1f times less", + log.Printf("%s (%d kLoC): golangci-lint%s: time: %s, %.1f times faster; memory: %dMB, %.1f times less", repoName, kLOC, mode, golangciLintRes.duration, gometalinterRes.duration.Seconds()/golangciLintRes.duration.Seconds(), golangciLintRes.peakMemMB, float64(gometalinterRes.peakMemMB)/float64(golangciLintRes.peakMemMB), diff --git a/test/linters_test.go b/test/linters_test.go index 6fb8d3bba853..62c3821d120f 100644 --- a/test/linters_test.go +++ b/test/linters_test.go @@ -9,6 +9,7 @@ import ( "strings" "testing" + "github.com/golangci/golangci-lint/pkg/exitcodes" "github.com/stretchr/testify/assert" ) @@ -94,10 +95,10 @@ func TestGolintConsumesXTestFiles(t *testing.T) { const expIssue = "if block ends with a return statement, so drop this else and outdent its block" out, ec := runGolangciLint(t, "--no-config", "--disable-all", "-Egolint", dir) - assert.Equal(t, 1, ec) + assert.Equal(t, exitcodes.IssuesFound, ec) assert.Contains(t, out, expIssue) out, ec = runGolangciLint(t, "--no-config", "--disable-all", "-Egolint", filepath.Join(dir, "p_test.go")) - assert.Equal(t, 1, ec) + assert.Equal(t, exitcodes.IssuesFound, ec) assert.Contains(t, out, expIssue) } diff --git a/test/run_test.go b/test/run_test.go index 27cf4f38f0ea..fa9647f2091e 100644 --- a/test/run_test.go +++ b/test/run_test.go @@ -13,6 +13,7 @@ import ( "syscall" "testing" + "github.com/golangci/golangci-lint/pkg/exitcodes" "github.com/golangci/golangci-lint/pkg/lint/lintersdb" "github.com/stretchr/testify/assert" @@ -28,7 +29,7 @@ func installBinary(t assert.TestingT) { } func checkNoIssuesRun(t *testing.T, out string, exitCode int) { - assert.Equal(t, 0, exitCode) + assert.Equal(t, exitcodes.Success, exitCode) assert.Equal(t, "Congrats! No issues were found.\n", out) } @@ -47,9 +48,20 @@ func TestSymlinkLoop(t *testing.T) { checkNoIssuesRun(t, out, exitCode) } +func TestRunOnAbsPath(t *testing.T) { + absPath, err := filepath.Abs(filepath.Join(testdataDir, "..")) + assert.NoError(t, err) + + out, exitCode := runGolangciLint(t, "--no-config", "--fast", absPath) + checkNoIssuesRun(t, out, exitCode) + + out, exitCode = runGolangciLint(t, "--no-config", absPath) + checkNoIssuesRun(t, out, exitCode) +} + func TestDeadline(t *testing.T) { - out, exitCode := runGolangciLint(t, "--deadline=1ms", "../...") - assert.Equal(t, 4, exitCode) + out, exitCode := runGolangciLint(t, "--deadline=1ms", filepath.Join("..", "...")) + assert.Equal(t, exitcodes.Timeout, exitCode) assert.Contains(t, out, "deadline exceeded: try increase it by passing --deadline option") assert.NotContains(t, out, "Congrats! No issues were found.") } @@ -79,7 +91,7 @@ func runGolangciLint(t *testing.T, args ...string) (string, int) { func runGolangciLintWithYamlConfig(t *testing.T, cfg string, args ...string) string { out, ec := runGolangciLintWithYamlConfigWithCode(t, cfg, args...) - assert.Equal(t, 0, ec) + assert.Equal(t, exitcodes.Success, ec) return out } @@ -107,12 +119,12 @@ func runGolangciLintWithYamlConfigWithCode(t *testing.T, cfg string, args ...str func TestTestsAreLintedByDefault(t *testing.T) { out, exitCode := runGolangciLint(t, "./testdata/withtests") - assert.Equal(t, 0, exitCode, out) + assert.Equal(t, exitcodes.Success, exitCode, out) } func TestConfigFileIsDetected(t *testing.T) { checkGotConfig := func(out string, exitCode int) { - assert.Equal(t, 0, exitCode, out) + assert.Equal(t, exitcodes.Success, exitCode, out) assert.Equal(t, "test\n", out) // test config contains InternalTest: true, it triggers such output } @@ -208,7 +220,7 @@ func TestEnableAllFastAndEnableCanCoexist(t *testing.T) { checkNoIssuesRun(t, out, exitCode) _, exitCode = runGolangciLint(t, "--enable-all", "--enable=typecheck") - assert.Equal(t, 3, exitCode) + assert.Equal(t, exitcodes.Failure, exitCode) } @@ -327,7 +339,7 @@ func TestEnabledLinters(t *testing.T) { func TestEnabledPresetsAreNotDuplicated(t *testing.T) { out, ec := runGolangciLint(t, "--no-config", "-v", "-p", "style,bugs") - assert.Equal(t, 0, ec) + assert.Equal(t, exitcodes.Success, ec) assert.Contains(t, out, "Active presets: [bugs style]") } @@ -371,7 +383,7 @@ func TestDisallowedOptionsInConfig(t *testing.T) { for _, c := range cases { // Run with disallowed option set only in config _, ec := runGolangciLintWithYamlConfigWithCode(t, c.cfg) - assert.Equal(t, 1, ec) + assert.Equal(t, exitcodes.Failure, ec) if c.option == "" { continue @@ -381,10 +393,10 @@ func TestDisallowedOptionsInConfig(t *testing.T) { // Run with disallowed option set only in command-line _, ec = runGolangciLint(t, args...) - assert.Equal(t, 0, ec) + assert.Equal(t, exitcodes.Success, ec) // Run with disallowed option set both in command-line and in config _, ec = runGolangciLintWithYamlConfigWithCode(t, c.cfg, args...) - assert.Equal(t, 1, ec) + assert.Equal(t, exitcodes.Failure, ec) } }