From 9181ca7175a3e608ab0e50e554bd186b81e47234 Mon Sep 17 00:00:00 2001 From: Denis Isaev Date: Wed, 13 Jun 2018 22:37:48 +0300 Subject: [PATCH] Fix #78: log all warnings 1. Log all warnings, don't hide none of them 2. Write fatal messages (stop analysis) with error log level 3. Remove ugly timestamp counter from logrus output 4. Print nested module prefix in log 5. Make logger abstraction: no global logging anymore 6. Refactor config reading to config.FileReader struct to avoid passing logger into every function 7. Replace exit codes hardcoding with constants in exitcodes package 8. Fail test if any warning was logged 9. Fix calculation of relative path if we analyze parent dir ../ 10. Move Runner initialization from Executor to NewRunner func 11. Log every AST parsing error 12. Properly print used config file path in verbose mode 13. Print package files if only 1 package is analyzedin verbose mode, print not compiling packages in verbose mode 14. Forbid usage of github.com/sirupsen/logrus by DepGuard linter 15. Add default ignore pattern to folint: "comment on exported const" --- .gitignore | 1 + .golangci.yml | 6 + Makefile | 10 +- README.md | 8 +- pkg/commands/executor.go | 7 +- pkg/commands/root.go | 27 +-- pkg/commands/run.go | 115 +++++----- pkg/commands/run_config.go | 217 ------------------ pkg/config/config.go | 2 +- pkg/config/reader.go | 193 ++++++++++++++++ pkg/exitcodes/exitcodes.go | 9 + pkg/fsutils/fsutils.go | 30 +++ pkg/golinters/gas.go | 3 +- pkg/golinters/gofmt.go | 8 +- pkg/golinters/golint.go | 3 +- pkg/golinters/megacheck.go | 3 +- pkg/golinters/utils.go | 2 +- pkg/lint/astcache/astcache.go | 19 +- pkg/lint/linter/context.go | 2 + pkg/lint/lintersdb/lintersdb.go | 12 +- pkg/lint/load.go | 37 ++- pkg/lint/load_test.go | 6 +- pkg/lint/runner.go | 81 +++++-- pkg/logutils/log.go | 30 +++ pkg/logutils/logutils.go | 23 +- pkg/logutils/stderr_log.go | 106 +++++++++ pkg/packages/resolver.go | 52 +++-- pkg/packages/resolver_test.go | 3 +- pkg/printers/tab.go | 10 +- pkg/printers/text.go | 12 +- pkg/result/processors/max_from_linter.go | 8 +- pkg/result/processors/max_from_linter_test.go | 4 +- pkg/result/processors/max_same_issues.go | 8 +- pkg/result/processors/max_same_issues_test.go | 3 +- pkg/result/processors/nolint_test.go | 3 +- pkg/timeutils/stopwatch.go | 12 +- pkg/timeutils/track.go | 6 +- test/bench_test.go | 6 +- test/linters_test.go | 5 +- test/run_test.go | 34 ++- 40 files changed, 694 insertions(+), 432 deletions(-) delete mode 100644 pkg/commands/run_config.go create mode 100644 pkg/config/reader.go create mode 100644 pkg/exitcodes/exitcodes.go create mode 100644 pkg/logutils/log.go create mode 100644 pkg/logutils/stderr_log.go diff --git a/.gitignore b/.gitignore index 098acb3b..1d85c074 100644 --- a/.gitignore +++ b/.gitignore @@ -2,3 +2,4 @@ /*.pprof /dist/ /.idea/ +/test/path diff --git a/.golangci.yml b/.golangci.yml index f60eb893..4c603487 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 0e5aad65..687d414a 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 06ac8954..77c6d042 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 a52e572a..a62b1c1d 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 d4ac1b72..0716331d 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 780541d7..ef00dc7a 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 20b8bf91..00000000 --- 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 261a9f8d..5423cd1e 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 00000000..a5538c6f --- /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 00000000..5ae21906 --- /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 4c3a0db1..75cca99b 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 7c37a639..4f78f6b4 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 76822d3e..7051e5f6 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 32cbcfee..a230db54 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 69cd53c5..ac452119 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 21dd29be..d3b72e0a 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 5415529f..15b9d152 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 7a9684e4..b33593c1 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 67853e7c..4f35fd66 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 f97f2a7b..9f089f5e 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 ad231a83..eaaeb700 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 e15afc74..1f7a90f4 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 00000000..e87cbee9 --- /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 94b2351e..9ba7bdff 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 00000000..08b8d77a --- /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 fcbedd9f..d224e665 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 f5adf45d..083c39f5 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 c677d480..7b0e4c04 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 6d4c3329..c291265a 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 67e9873f..f669cab2 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 05ffb632..15ccb8bb 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 48e98e30..b59b7a32 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 7f3486e2..3fad8fd5 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 de255fa2..36299d89 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 e17deaf6..4390bf35 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 279e4e7b..ada1b9a4 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 bc2c8550..31d8659e 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 6fb8d3bb..62c3821d 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 27cf4f38..fa9647f2 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) } }