From d9a1bdb8319c365555b83f041f886203573cb0ec Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?David=20L=C3=B3pez?= Date: Thu, 3 Jan 2019 11:33:05 +0100 Subject: [PATCH] gocritic: fix code to pass newly added gocritic checks Fix code to pass newly added gocritic checks, mainly pointer receivers and import shadows --- pkg/commands/config.go | 2 +- pkg/commands/executor.go | 2 +- pkg/commands/help.go | 6 +++--- pkg/commands/linters.go | 4 ++-- pkg/commands/run.go | 18 +++++++++-------- pkg/fsutils/fsutils.go | 2 +- pkg/golinters/gofmt.go | 6 +++--- pkg/golinters/golint.go | 8 ++++---- pkg/golinters/megacheck.go | 12 +++++------ pkg/lint/linter/config.go | 18 ++++++++--------- pkg/lint/lintersdb/enabled_set.go | 12 +++++------ pkg/lint/lintersdb/enabled_set_test.go | 4 ++-- pkg/lint/lintersdb/manager.go | 28 +++++++++++++------------- pkg/lint/load.go | 4 ++-- pkg/lint/runner.go | 18 ++++++++--------- pkg/result/issue.go | 8 ++++---- scripts/gen_readme/main.go | 2 +- 17 files changed, 78 insertions(+), 76 deletions(-) diff --git a/pkg/commands/config.go b/pkg/commands/config.go index 7e4915dd..45202473 100644 --- a/pkg/commands/config.go +++ b/pkg/commands/config.go @@ -34,7 +34,7 @@ func (e *Executor) initConfig() { } -func (e Executor) executePathCmd(cmd *cobra.Command, args []string) { +func (e *Executor) executePathCmd(cmd *cobra.Command, args []string) { usedConfigFile := viper.ConfigFileUsed() if usedConfigFile == "" { e.log.Warnf("No config file detected") diff --git a/pkg/commands/executor.go b/pkg/commands/executor.go index 4da64e3b..0075668e 100644 --- a/pkg/commands/executor.go +++ b/pkg/commands/executor.go @@ -82,6 +82,6 @@ func NewExecutor(version, commit, date string) *Executor { return e } -func (e Executor) Execute() error { +func (e *Executor) Execute() error { return e.rootCmd.Execute() } diff --git a/pkg/commands/help.go b/pkg/commands/help.go index fed59c12..abcacb9d 100644 --- a/pkg/commands/help.go +++ b/pkg/commands/help.go @@ -32,7 +32,7 @@ func (e *Executor) initHelp() { helpCmd.AddCommand(lintersHelpCmd) } -func printLinterConfigs(lcs []linter.Config) { +func printLinterConfigs(lcs []*linter.Config) { for _, lc := range lcs { altNamesStr := "" if len(lc.AlternativeNames) != 0 { @@ -43,12 +43,12 @@ func printLinterConfigs(lcs []linter.Config) { } } -func (e Executor) executeLintersHelp(cmd *cobra.Command, args []string) { +func (e *Executor) executeLintersHelp(cmd *cobra.Command, args []string) { if len(args) != 0 { e.log.Fatalf("Usage: golangci-lint help linters") } - var enabledLCs, disabledLCs []linter.Config + var enabledLCs, disabledLCs []*linter.Config for _, lc := range e.DBManager.GetAllSupportedLinterConfigs() { if lc.EnabledByDefault { enabledLCs = append(enabledLCs, lc) diff --git a/pkg/commands/linters.go b/pkg/commands/linters.go index 22500489..786a2d08 100644 --- a/pkg/commands/linters.go +++ b/pkg/commands/linters.go @@ -20,7 +20,7 @@ func (e *Executor) initLinters() { e.initRunConfiguration(lintersCmd) } -func IsLinterInConfigsList(name string, linters []linter.Config) bool { +func IsLinterInConfigsList(name string, linters []*linter.Config) bool { for _, lc := range linters { if lc.Name() == name { return true @@ -43,7 +43,7 @@ func (e *Executor) executeLinters(cmd *cobra.Command, args []string) { color.Green("Enabled by your configuration linters:\n") printLinterConfigs(enabledLCs) - var disabledLCs []linter.Config + var disabledLCs []*linter.Config for _, lc := range e.DBManager.GetAllSupportedLinterConfigs() { if !IsLinterInConfigsList(lc.Name(), enabledLCs) { disabledLCs = append(disabledLCs, lc) diff --git a/pkg/commands/run.go b/pkg/commands/run.go index 1601c123..3e7fe672 100644 --- a/pkg/commands/run.go +++ b/pkg/commands/run.go @@ -27,9 +27,11 @@ import ( func getDefaultExcludeHelp() string { parts := []string{"Use or not use default excludes:"} for _, ep := range config.DefaultExcludePatterns { - parts = append(parts, fmt.Sprintf(" # %s: %s", ep.Linter, ep.Why)) - parts = append(parts, fmt.Sprintf(" - %s", color.YellowString(ep.Pattern))) - parts = append(parts, "") + parts = append(parts, + fmt.Sprintf(" # %s: %s", ep.Linter, ep.Why), + fmt.Sprintf(" - %s", color.YellowString(ep.Pattern)), + "", + ) } return strings.Join(parts, "\n") } @@ -184,7 +186,7 @@ func (e *Executor) initRunConfiguration(cmd *cobra.Command) { initFlagSet(fs, e.cfg, e.DBManager, true) } -func (e Executor) getConfigForCommandLine() (*config.Config, error) { +func (e *Executor) getConfigForCommandLine() (*config.Config, error) { // We use another pflag.FlagSet here to not set `changed` flag // on cmd.Flags() options. Otherwise string slice options will be duplicated. fs := pflag.NewFlagSet("config flag set", pflag.ContinueOnError) @@ -412,10 +414,10 @@ func (e *Executor) setupExitCode(ctx context.Context) { } } -func watchResources(ctx context.Context, done chan struct{}, log logutils.Log) { +func watchResources(ctx context.Context, done chan struct{}, logger logutils.Log) { startedAt := time.Now() - rssValues := []uint64{} + var rssValues []uint64 ticker := time.NewTicker(100 * time.Millisecond) defer ticker.Stop() @@ -448,8 +450,8 @@ func watchResources(ctx context.Context, done chan struct{}, log logutils.Log) { const MB = 1024 * 1024 maxMB := float64(max) / MB - log.Infof("Memory: %d samples, avg is %.1fMB, max is %.1fMB", + logger.Infof("Memory: %d samples, avg is %.1fMB, max is %.1fMB", len(rssValues), float64(avg)/MB, maxMB) - log.Infof("Execution took %s", time.Since(startedAt)) + logger.Infof("Execution took %s", time.Since(startedAt)) close(done) } diff --git a/pkg/fsutils/fsutils.go b/pkg/fsutils/fsutils.go index d2b880e7..fa5fda6a 100644 --- a/pkg/fsutils/fsutils.go +++ b/pkg/fsutils/fsutils.go @@ -65,7 +65,7 @@ func EvalSymlinks(path string) (string, error) { return er.path, er.err } -func ShortestRelPath(path string, wd string) (string, error) { +func ShortestRelPath(path, wd string) (string, error) { if wd == "" { // get it if user don't have cached working dir var err error wd, err = Getwd() diff --git a/pkg/golinters/gofmt.go b/pkg/golinters/gofmt.go index 97b21d4f..de2c39fd 100644 --- a/pkg/golinters/gofmt.go +++ b/pkg/golinters/gofmt.go @@ -9,7 +9,7 @@ import ( gofmtAPI "github.com/golangci/gofmt/gofmt" goimportsAPI "github.com/golangci/gofmt/goimports" "golang.org/x/tools/imports" - "sourcegraph.com/sourcegraph/go-diff/diff" + diffpkg "sourcegraph.com/sourcegraph/go-diff/diff" "github.com/golangci/golangci-lint/pkg/lint/linter" "github.com/golangci/golangci-lint/pkg/logutils" @@ -37,7 +37,7 @@ func (g Gofmt) Desc() string { "this tool runs with -s option to check for code simplification" } -func getFirstDeletedAndAddedLineNumberInHunk(h *diff.Hunk) (int, int, error) { +func getFirstDeletedAndAddedLineNumberInHunk(h *diffpkg.Hunk) (int, int, error) { lines := bytes.Split(h.Body, []byte{'\n'}) lineNumber := int(h.OrigStartLine - 1) firstAddedLineNumber := -1 @@ -59,7 +59,7 @@ func getFirstDeletedAndAddedLineNumberInHunk(h *diff.Hunk) (int, int, error) { } func (g Gofmt) extractIssuesFromPatch(patch string, log logutils.Log) ([]result.Issue, error) { - diffs, err := diff.ParseMultiFileDiff([]byte(patch)) + diffs, err := diffpkg.ParseMultiFileDiff([]byte(patch)) if err != nil { return nil, fmt.Errorf("can't parse patch: %s", err) } diff --git a/pkg/golinters/golint.go b/pkg/golinters/golint.go index 07be07e9..4850e605 100644 --- a/pkg/golinters/golint.go +++ b/pkg/golinters/golint.go @@ -57,11 +57,11 @@ func (g Golint) lintPkg(minConfidence float64, files []*ast.File, fset *token.Fi } issues := make([]result.Issue, 0, len(ps)) // This is worst case - for _, p := range ps { - if p.Confidence >= minConfidence { + for idx := range ps { + if ps[idx].Confidence >= minConfidence { issues = append(issues, result.Issue{ - Pos: p.Position, - Text: markIdentifiers(p.Text), + Pos: ps[idx].Position, + Text: markIdentifiers(ps[idx].Text), FromLinter: g.Name(), }) // TODO: use p.Link and p.Category diff --git a/pkg/golinters/megacheck.go b/pkg/golinters/megacheck.go index f131cfa6..f2a19029 100644 --- a/pkg/golinters/megacheck.go +++ b/pkg/golinters/megacheck.go @@ -88,7 +88,7 @@ func (m Megacheck) canAnalyze(lintCtx *linter.Context) bool { } var errPkgs []string - var errors []packages.Error + var errs []packages.Error for _, p := range lintCtx.NotCompilingPackages { if p.Name == "main" { // megacheck crashes on not compiling packages but main packages @@ -97,7 +97,7 @@ func (m Megacheck) canAnalyze(lintCtx *linter.Context) bool { } errPkgs = append(errPkgs, p.String()) - errors = append(errors, libpackages.ExtractErrors(p, lintCtx.ASTCache)...) + errs = append(errs, libpackages.ExtractErrors(p, lintCtx.ASTCache)...) } if len(errPkgs) == 0 { // only main packages do not compile @@ -105,11 +105,11 @@ func (m Megacheck) canAnalyze(lintCtx *linter.Context) bool { } warnText := fmt.Sprintf("Can't run megacheck because of compilation errors in packages %s", errPkgs) - if len(errors) != 0 { - warnText += fmt.Sprintf(": %s", prettifyCompilationError(errors[0])) - if len(errors) > 1 { + if len(errs) != 0 { + warnText += fmt.Sprintf(": %s", prettifyCompilationError(errs[0])) + if len(errs) > 1 { const runCmd = "golangci-lint run --no-config --disable-all -E typecheck" - warnText += fmt.Sprintf(" and %d more errors: run `%s` to see all errors", len(errors)-1, runCmd) + warnText += fmt.Sprintf(" and %d more errors: run `%s` to see all errors", len(errs)-1, runCmd) } } lintCtx.Log.Warnf("%s", warnText) diff --git a/pkg/lint/linter/config.go b/pkg/lint/linter/config.go index 0c5d9cd8..64ac4327 100644 --- a/pkg/lint/linter/config.go +++ b/pkg/lint/linter/config.go @@ -23,46 +23,46 @@ type Config struct { OriginalURL string // URL of original (not forked) repo, needed for autogenerated README } -func (lc Config) WithTypeInfo() Config { +func (lc *Config) WithTypeInfo() *Config { lc.NeedsTypeInfo = true return lc } -func (lc Config) WithSSA() Config { +func (lc *Config) WithSSA() *Config { lc.NeedsTypeInfo = true lc.NeedsSSARepr = true return lc } -func (lc Config) WithPresets(presets ...string) Config { +func (lc *Config) WithPresets(presets ...string) *Config { lc.InPresets = presets return lc } -func (lc Config) WithSpeed(speed int) Config { +func (lc *Config) WithSpeed(speed int) *Config { lc.Speed = speed return lc } -func (lc Config) WithURL(url string) Config { +func (lc *Config) WithURL(url string) *Config { lc.OriginalURL = url return lc } -func (lc Config) WithAlternativeNames(names ...string) Config { +func (lc *Config) WithAlternativeNames(names ...string) *Config { lc.AlternativeNames = names return lc } -func (lc Config) GetSpeed() int { +func (lc *Config) GetSpeed() int { return lc.Speed } -func (lc Config) AllNames() []string { +func (lc *Config) AllNames() []string { return append([]string{lc.Name()}, lc.AlternativeNames...) } -func (lc Config) Name() string { +func (lc *Config) Name() string { return lc.Linter.Name() } diff --git a/pkg/lint/lintersdb/enabled_set.go b/pkg/lint/lintersdb/enabled_set.go index f4484c61..7e29c11e 100644 --- a/pkg/lint/lintersdb/enabled_set.go +++ b/pkg/lint/lintersdb/enabled_set.go @@ -26,7 +26,7 @@ func NewEnabledSet(m *Manager, v *Validator, log logutils.Log, cfg *config.Confi } // nolint:gocyclo -func (es EnabledSet) build(lcfg *config.Linters, enabledByDefaultLinters []linter.Config) map[string]*linter.Config { +func (es EnabledSet) build(lcfg *config.Linters, enabledByDefaultLinters []*linter.Config) map[string]*linter.Config { resultLintersSet := map[string]*linter.Config{} switch { case len(lcfg.Presets) != 0: @@ -43,7 +43,7 @@ func (es EnabledSet) build(lcfg *config.Linters, enabledByDefaultLinters []linte for _, p := range lcfg.Presets { for _, lc := range es.m.GetAllLinterConfigsForPreset(p) { lc := lc - resultLintersSet[lc.Name()] = &lc + resultLintersSet[lc.Name()] = lc } } @@ -121,23 +121,23 @@ func (es EnabledSet) optimizeLintersSet(linters map[string]*linter.Config) { linters[mega.Name()] = &lc } -func (es EnabledSet) Get() ([]linter.Config, error) { +func (es EnabledSet) Get() ([]*linter.Config, error) { if err := es.v.validateEnabledDisabledLintersConfig(&es.cfg.Linters); err != nil { return nil, err } resultLintersSet := es.build(&es.cfg.Linters, es.m.GetAllEnabledByDefaultLinters()) - var resultLinters []linter.Config + var resultLinters []*linter.Config for _, lc := range resultLintersSet { - resultLinters = append(resultLinters, *lc) + resultLinters = append(resultLinters, lc) } es.verbosePrintLintersStatus(resultLinters) return resultLinters, nil } -func (es EnabledSet) verbosePrintLintersStatus(lcs []linter.Config) { +func (es EnabledSet) verbosePrintLintersStatus(lcs []*linter.Config) { var linterNames []string for _, lc := range lcs { linterNames = append(linterNames, lc.Name()) diff --git a/pkg/lint/lintersdb/enabled_set_test.go b/pkg/lint/lintersdb/enabled_set_test.go index bd33dc09..2c7a3a67 100644 --- a/pkg/lint/lintersdb/enabled_set_test.go +++ b/pkg/lint/lintersdb/enabled_set_test.go @@ -85,9 +85,9 @@ func TestGetEnabledLintersSet(t *testing.T) { for _, c := range cases { c := c t.Run(c.name, func(t *testing.T) { - defaultLinters := []linter.Config{} + var defaultLinters []*linter.Config for _, ln := range c.def { - defaultLinters = append(defaultLinters, *m.GetLinterConfig(ln)) + defaultLinters = append(defaultLinters, m.GetLinterConfig(ln)) } els := es.build(&c.cfg, defaultLinters) var enabledLinters []string diff --git a/pkg/lint/lintersdb/manager.go b/pkg/lint/lintersdb/manager.go index 7c25664d..64958823 100644 --- a/pkg/lint/lintersdb/manager.go +++ b/pkg/lint/lintersdb/manager.go @@ -8,12 +8,12 @@ import ( ) type Manager struct { - nameToLC map[string]linter.Config + nameToLC map[string]*linter.Config } func NewManager() *Manager { m := &Manager{} - nameToLC := make(map[string]linter.Config) + nameToLC := make(map[string]*linter.Config) for _, lc := range m.GetAllSupportedLinterConfigs() { for _, name := range lc.AllNames() { nameToLC[name] = lc @@ -43,22 +43,22 @@ func (m Manager) GetLinterConfig(name string) *linter.Config { return nil } - return &lc + return lc } -func enableLinterConfigs(lcs []linter.Config, isEnabled func(lc *linter.Config) bool) []linter.Config { - var ret []linter.Config +func enableLinterConfigs(lcs []*linter.Config, isEnabled func(lc *linter.Config) bool) []*linter.Config { + var ret []*linter.Config for _, lc := range lcs { lc := lc - lc.EnabledByDefault = isEnabled(&lc) + lc.EnabledByDefault = isEnabled(lc) ret = append(ret, lc) } return ret } -func (Manager) GetAllSupportedLinterConfigs() []linter.Config { - lcs := []linter.Config{ +func (Manager) GetAllSupportedLinterConfigs() []*linter.Config { + lcs := []*linter.Config{ linter.NewConfig(golinters.Govet{}). WithTypeInfo(). WithPresets(linter.PresetBugs). @@ -226,8 +226,8 @@ func (Manager) GetAllSupportedLinterConfigs() []linter.Config { }) } -func (m Manager) GetAllEnabledByDefaultLinters() []linter.Config { - var ret []linter.Config +func (m Manager) GetAllEnabledByDefaultLinters() []*linter.Config { + var ret []*linter.Config for _, lc := range m.GetAllSupportedLinterConfigs() { if lc.EnabledByDefault { ret = append(ret, lc) @@ -237,18 +237,18 @@ func (m Manager) GetAllEnabledByDefaultLinters() []linter.Config { return ret } -func linterConfigsToMap(lcs []linter.Config) map[string]*linter.Config { +func linterConfigsToMap(lcs []*linter.Config) map[string]*linter.Config { ret := map[string]*linter.Config{} for _, lc := range lcs { lc := lc // local copy - ret[lc.Name()] = &lc + ret[lc.Name()] = lc } return ret } -func (m Manager) GetAllLinterConfigsForPreset(p string) []linter.Config { - ret := []linter.Config{} +func (m Manager) GetAllLinterConfigsForPreset(p string) []*linter.Config { + var ret []*linter.Config for _, lc := range m.GetAllSupportedLinterConfigs() { for _, ip := range lc.InPresets { if p == ip { diff --git a/pkg/lint/load.go b/pkg/lint/load.go index 7c443896..925275bf 100644 --- a/pkg/lint/load.go +++ b/pkg/lint/load.go @@ -140,7 +140,7 @@ func (cl ContextLoader) buildSSAProgram(pkgs []*packages.Package, name string) * return ssaProg } -func (cl ContextLoader) findLoadMode(linters []linter.Config) packages.LoadMode { +func (cl ContextLoader) findLoadMode(linters []*linter.Config) packages.LoadMode { maxLoadMode := packages.LoadFiles for _, lc := range linters { curLoadMode := packages.LoadFiles @@ -316,7 +316,7 @@ func (cl ContextLoader) filterPackages(pkgs []*packages.Package) []*packages.Pac } //nolint:gocyclo -func (cl ContextLoader) Load(ctx context.Context, linters []linter.Config) (*linter.Context, error) { +func (cl ContextLoader) Load(ctx context.Context, linters []*linter.Config) (*linter.Context, error) { loadMode := cl.findLoadMode(linters) pkgs, err := cl.loadPackages(ctx, loadMode) if err != nil { diff --git a/pkg/lint/runner.go b/pkg/lint/runner.go index 169aa5d7..cd6c9a4e 100644 --- a/pkg/lint/runner.go +++ b/pkg/lint/runner.go @@ -73,13 +73,13 @@ func NewRunner(astCache *astcache.Cache, cfg *config.Config, log logutils.Log, g } type lintRes struct { - linter linter.Config + linter *linter.Config err error issues []result.Issue } -func (r Runner) 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 { @@ -103,7 +103,7 @@ func (r Runner) runLinterSafe(ctx context.Context, lintCtx *linter.Context, } func (r Runner) runWorker(ctx context.Context, lintCtx *linter.Context, - tasksCh <-chan linter.Config, lintResultsCh chan<- lintRes, name string) { + tasksCh <-chan *linter.Config, lintResultsCh chan<- lintRes, name string) { sw := timeutils.NewStopwatch(name, r.Log) defer sw.Print() @@ -155,8 +155,8 @@ func (r Runner) logWorkersStat(workersFinishTimes []time.Time) { r.Log.Infof("Workers idle times: %s", strings.Join(logStrings, ", ")) } -func getSortedLintersConfigs(linters []linter.Config) []linter.Config { - ret := make([]linter.Config, len(linters)) +func getSortedLintersConfigs(linters []*linter.Config) []*linter.Config { + ret := make([]*linter.Config, len(linters)) copy(ret, linters) sort.Slice(ret, func(i, j int) bool { @@ -166,8 +166,8 @@ func getSortedLintersConfigs(linters []linter.Config) []linter.Config { return ret } -func (r *Runner) runWorkers(ctx context.Context, lintCtx *linter.Context, linters []linter.Config) <-chan lintRes { - tasksCh := make(chan linter.Config, len(linters)) +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 @@ -259,7 +259,7 @@ func collectIssues(resCh <-chan lintRes) <-chan result.Issue { return retIssues } -func (r Runner) 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(lintResultsCh) if ctx.Err() != nil { diff --git a/pkg/result/issue.go b/pkg/result/issue.go index 39281e04..3e27267f 100644 --- a/pkg/result/issue.go +++ b/pkg/result/issue.go @@ -17,19 +17,19 @@ type Issue struct { SourceLines []string } -func (i Issue) FilePath() string { +func (i *Issue) FilePath() string { return i.Pos.Filename } -func (i Issue) Line() int { +func (i *Issue) Line() int { return i.Pos.Line } -func (i Issue) Column() int { +func (i *Issue) Column() int { return i.Pos.Column } -func (i Issue) GetLineRange() Range { +func (i *Issue) GetLineRange() Range { if i.LineRange == nil { return Range{ From: i.Line(), diff --git a/scripts/gen_readme/main.go b/scripts/gen_readme/main.go index 61070dd9..a8b23cc0 100644 --- a/scripts/gen_readme/main.go +++ b/scripts/gen_readme/main.go @@ -88,7 +88,7 @@ func buildTemplateContext() (map[string]interface{}, error) { } func getLintersListMarkdown(enabled bool) string { - var neededLcs []linter.Config + var neededLcs []*linter.Config lcs := lintersdb.NewManager().GetAllSupportedLinterConfigs() for _, lc := range lcs { if lc.EnabledByDefault == enabled {