gocritic: update default checks list && fix new added gocritic linting errs

.golangci.example.yml

@@ -151,9 +151,10 @@ linters-settings:
     for-loops: false # Report preallocation suggestions on for loops, false by default
   gocritic:
     # which checks should be enabled; can't be combined with 'disabled-checks';
-    # default are: [appendAssign assignOp caseOrder dupArg dupBranchBody dupCase flagDeref
-    # ifElseChain regexpMust singleCaseSwitch sloppyLen switchTrue typeSwitchVar underef
-    # unlambda unslice rangeValCopy defaultCaseOrder];
+    # default are: [appendAssign appendCombine assignOp builtinShadow captLocal caseOrder defaultCaseOrder
+    # dupArg dupBranchBody dupCase elseif flagDeref ifElseChain importShadow indexAlloc paramTypeCombine
+    # rangeExprCopy rangeValCopy regexpMust singleCaseSwitch sloppyLen switchTrue typeSwitchVar typeUnparen
+    # underef unlambda unslice dupSubExpr hugeParam];
     # all checks list: https://github.com/go-critic/checkers
     enabled-checks:
       - rangeValCopy

README.md

@@ -667,9 +667,10 @@ linters-settings:
     for-loops: false # Report preallocation suggestions on for loops, false by default
   gocritic:
     # which checks should be enabled; can't be combined with 'disabled-checks';
-    # default are: [appendAssign assignOp caseOrder dupArg dupBranchBody dupCase flagDeref
-    # ifElseChain regexpMust singleCaseSwitch sloppyLen switchTrue typeSwitchVar underef
-    # unlambda unslice rangeValCopy defaultCaseOrder];
+    # default are: [appendAssign appendCombine assignOp builtinShadow captLocal caseOrder defaultCaseOrder
+    # dupArg dupBranchBody dupCase elseif flagDeref ifElseChain importShadow indexAlloc paramTypeCombine
+    # rangeExprCopy rangeValCopy regexpMust singleCaseSwitch sloppyLen switchTrue typeSwitchVar typeUnparen
+    # underef unlambda unslice dupSubExpr hugeParam];
     # all checks list: https://github.com/go-critic/checkers
     enabled-checks:
       - rangeValCopy

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

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()
 }

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)

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)

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

pkg/config/config.go

@@ -257,24 +257,38 @@ func (s GocriticSettings) IsCheckEnabled(name string) bool {
 	return s.inferredEnabledChecks[strings.ToLower(name)]
 }
 
+// Its a good idea to keep this list in sync with the gocritic stable checks list in:
+// https://github.com/go-critic/go-critic/blob/master/checkers/checkers_test.go#L63
 var defaultGocriticEnabledChecks = []string{
 	"appendAssign",
+	"appendCombine",
 	"assignOp",
+	"builtinShadow",
+	"captLocal",
 	"caseOrder",
+	"defaultCaseOrder",
 	"dupArg",
 	"dupBranchBody",
 	"dupCase",
+	"elseif",
 	"flagDeref",
 	"ifElseChain",
+	"importShadow",
+	"indexAlloc",
+	"paramTypeCombine",
+	"rangeExprCopy",
+	"rangeValCopy",
 	"regexpMust",
 	"singleCaseSwitch",
 	"sloppyLen",
 	"switchTrue",
 	"typeSwitchVar",
+	"typeUnparen",
 	"underef",
 	"unlambda",
 	"unslice",
-	"defaultCaseOrder",
+	"dupSubExpr",
+	"hugeParam",
 }
 
 var defaultLintersSettings = LintersSettings{

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

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

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

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,19 +97,19 @@ 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
 		return true
 	}
 
 	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)

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()
 }
 

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

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