dev: refactor .golangci.yml configuration and fix up nolintlint issues (#4537)

Author: alexandear

.golangci.yml

@@ -17,13 +17,15 @@ linters-settings:
       logger:
         deny:
           # logging is allowed only by logutils.Log,
-          # logrus is allowed to use only in logutils package.
           - pkg: "github.com/sirupsen/logrus"
             desc: logging is allowed only by logutils.Log.
           - pkg: "github.com/pkg/errors"
             desc: Should be replaced by standard lib errors package.
           - pkg: "github.com/instana/testify"
             desc: It's a fork of github.com/stretchr/testify.
+        files:
+          # logrus is allowed to use only in logutils package.
+          - "!**/pkg/logutils/**.go"
   dupl:
     threshold: 100
   funlen:
@@ -86,10 +88,12 @@ linters-settings:
     line-length: 140
   misspell:
     locale: US
+    ignore-words:
+      - "importas" # linter name
   nolintlint:
     allow-unused: false # report any unused nolint directives
-    require-explanation: false # don't require an explanation for nolint directives
-    require-specific: false # don't require nolint directives to be specific about which linter is being skipped
+    require-explanation: true # require an explanation for nolint directives
+    require-specific: true # require nolint directives to be specific about which linter is being skipped
   revive:
     rules:
       - name: unexported-return
@@ -144,7 +148,9 @@ issues:
   exclude-rules:
     - path: _test\.go
       linters:
+        - dupl
         - gomnd
+        - lll
 
     - path: pkg/golinters/errcheck.go
       linters: [staticcheck]
@@ -158,6 +164,10 @@ issues:
     - path: pkg/golinters/govet.go
       text: "SA1019: cfg.CheckShadowing is deprecated: the linter should be enabled inside `Enable`."
 
+    - path: pkg/golinters
+      linters:
+        - dupl
+
     - path: pkg/golinters/gofumpt.go
       linters: [staticcheck]
       text: "SA1019: settings.LangVersion is deprecated: use the global `run.go` instead."

pkg/commands/flagsets.go

@@ -80,7 +80,7 @@ func setupOutputFlagSet(v *viper.Viper, fs *pflag.FlagSet) {
 	internal.AddFlagAndBind(v, fs, fs.Bool, "show-stats", "output.show-stats", false, color.GreenString("Show statistics per linter"))
 }
 
-//nolint:gomnd
+//nolint:gomnd // magic numbers here is ok
 func setupIssuesFlagSet(v *viper.Viper, fs *pflag.FlagSet) {
 	internal.AddHackedStringSliceP(fs, "exclude", "e", color.GreenString("Exclude issue by regexp"))
 	internal.AddFlagAndBind(v, fs, fs.Bool, "exclude-use-default", "issues.exclude-use-default", true,

pkg/golinters/dogsled.go

@@ -16,7 +16,6 @@ import (
 
 const dogsledName = "dogsled"
 
-//nolint:dupl
 func NewDogsled(settings *config.DogsledSettings) *goanalysis.Linter {
 	var mu sync.Mutex
 	var resIssues []goanalysis.Issue

pkg/golinters/dupl.go

@@ -17,7 +17,6 @@ import (
 
 const duplName = "dupl"
 
-//nolint:dupl
 func NewDupl(settings *config.DuplSettings) *goanalysis.Linter {
 	var mu sync.Mutex
 	var resIssues []goanalysis.Issue

pkg/golinters/forbidigo.go

@@ -16,7 +16,6 @@ import (
 
 const forbidigoName = "forbidigo"
 
-//nolint:dupl
 func NewForbidigo(settings *config.ForbidigoSettings) *goanalysis.Linter {
 	var mu sync.Mutex
 	var resIssues []goanalysis.Issue

pkg/golinters/funlen.go

@@ -16,7 +16,6 @@ import (
 
 const funlenName = "funlen"
 
-//nolint:dupl
 func NewFunlen(settings *config.FunlenSettings) *goanalysis.Linter {
 	var mu sync.Mutex
 	var resIssues []goanalysis.Issue

pkg/golinters/goanalysis/runner.go

@@ -177,10 +177,9 @@ func (r *runner) buildActionFactDeps(act *action, a *analysis.Analyzer, pkg *pac
 	}
 }
 
-//nolint:gocritic
 func (r *runner) prepareAnalysis(pkgs []*packages.Package,
 	analyzers []*analysis.Analyzer,
-) (map[*packages.Package]bool, []*action, []*action) {
+) (initialPkgs map[*packages.Package]bool, allActions, roots []*action) {
 	// Construct the action graph.
 
 	// Each graph node (action) is one unit of analysis.
@@ -200,13 +199,13 @@ func (r *runner) prepareAnalysis(pkgs []*packages.Package,
 	actions := make(map[actKey]*action, totalActionsCount)
 	actAlloc := newActionAllocator(totalActionsCount)
 
-	initialPkgs := make(map[*packages.Package]bool, len(pkgs))
+	initialPkgs = make(map[*packages.Package]bool, len(pkgs))
 	for _, pkg := range pkgs {
 		initialPkgs[pkg] = true
 	}
 
 	// Build nodes for initial packages.
-	roots := make([]*action, 0, len(pkgs)*len(analyzers))
+	roots = make([]*action, 0, len(pkgs)*len(analyzers))
 	for _, a := range analyzers {
 		for _, pkg := range pkgs {
 			root := r.makeAction(a, pkg, initialPkgs, actions, actAlloc)
@@ -215,7 +214,7 @@ func (r *runner) prepareAnalysis(pkgs []*packages.Package,
 		}
 	}
 
-	allActions := maps.Values(actions)
+	allActions = maps.Values(actions)
 
 	debugf("Built %d actions", len(actions))
 
@@ -281,7 +280,6 @@ func (r *runner) analyze(pkgs []*packages.Package, analyzers []*analysis.Analyze
 	return rootActions
 }
 
-//nolint:nakedret
 func extractDiagnostics(roots []*action) (retDiags []Diagnostic, retErrors []error) {
 	extracted := make(map[*action]bool)
 	var extract func(*action)
@@ -338,5 +336,5 @@ func extractDiagnostics(roots []*action) (retDiags []Diagnostic, retErrors []err
 		}
 	}
 	visitAll(roots)
-	return
+	return retDiags, retErrors
 }

pkg/golinters/goanalysis/runners.go

@@ -184,10 +184,9 @@ func saveIssuesToCache(allPkgs []*packages.Package, pkgsFromCache map[*packages.
 	issuesCacheDebugf("Saved %d issues from %d packages to cache in %s", savedIssuesCount, len(allPkgs), time.Since(startedAt))
 }
 
-//nolint:gocritic
 func loadIssuesFromCache(pkgs []*packages.Package, lintCtx *linter.Context,
 	analyzers []*analysis.Analyzer,
-) ([]result.Issue, map[*packages.Package]bool) {
+) (issuesFromCache []result.Issue, pkgsFromCache map[*packages.Package]bool) {
 	startedAt := time.Now()
 
 	lintResKey := getIssuesCacheKey(analyzers)
@@ -221,17 +220,18 @@ func loadIssuesFromCache(pkgs []*packages.Package, lintCtx *linter.Context,
 				}
 
 				issues := make([]result.Issue, 0, len(pkgIssues))
-				for _, i := range pkgIssues {
+				for i := range pkgIssues {
+					issue := &pkgIssues[i]
 					issues = append(issues, result.Issue{
-						FromLinter:           i.FromLinter,
-						Text:                 i.Text,
-						Severity:             i.Severity,
-						Pos:                  i.Pos,
-						LineRange:            i.LineRange,
-						Replacement:          i.Replacement,
+						FromLinter:           issue.FromLinter,
+						Text:                 issue.Text,
+						Severity:             issue.Severity,
+						Pos:                  issue.Pos,
+						LineRange:            issue.LineRange,
+						Replacement:          issue.Replacement,
 						Pkg:                  pkg,
-						ExpectNoLint:         i.ExpectNoLint,
-						ExpectedNoLintLinter: i.ExpectedNoLintLinter,
+						ExpectNoLint:         issue.ExpectNoLint,
+						ExpectedNoLintLinter: issue.ExpectedNoLintLinter,
 					})
 				}
 				cacheRes.issues = issues
@@ -246,21 +246,20 @@ func loadIssuesFromCache(pkgs []*packages.Package, lintCtx *linter.Context,
 	wg.Wait()
 
 	loadedIssuesCount := 0
-	var issues []result.Issue
-	pkgsFromCache := map[*packages.Package]bool{}
+	pkgsFromCache = map[*packages.Package]bool{}
 	for pkg, cacheRes := range pkgToCacheRes {
 		if cacheRes.loadErr == nil {
 			loadedIssuesCount += len(cacheRes.issues)
 			pkgsFromCache[pkg] = true
-			issues = append(issues, cacheRes.issues...)
+			issuesFromCache = append(issuesFromCache, cacheRes.issues...)
 			issuesCacheDebugf("Loaded package %s issues (%d) from cache", pkg, len(cacheRes.issues))
 		} else {
 			issuesCacheDebugf("Didn't load package %s issues from cache: %s", pkg, cacheRes.loadErr)
 		}
 	}
 	issuesCacheDebugf("Loaded %d issues from cache in %s, analyzing %d/%d packages",
 		loadedIssuesCount, time.Since(startedAt), len(pkgs)-len(pkgsFromCache), len(pkgs))
-	return issues, pkgsFromCache
+	return issuesFromCache, pkgsFromCache
 }
 
 func analyzersHashID(analyzers []*analysis.Analyzer) string {

pkg/golinters/gocognit.go

@@ -16,7 +16,6 @@ import (
 
 const gocognitName = "gocognit"
 
-//nolint:dupl
 func NewGocognit(settings *config.GocognitSettings) *goanalysis.Linter {
 	var mu sync.Mutex
 	var resIssues []goanalysis.Issue

pkg/golinters/goconst.go

@@ -15,7 +15,6 @@ import (
 
 const goconstName = "goconst"
 
-//nolint:dupl
 func NewGoconst(settings *config.GoConstSettings) *goanalysis.Linter {
 	var mu sync.Mutex
 	var resIssues []goanalysis.Issue