clear types only after unused running

Ensure that `unused` is always the last
in execution order. It can speed up packages loading
a bit.

Refactor enabled linters set to remove extra logging.

Relates: #944

Author: jirfag

go.mod

@@ -52,12 +52,8 @@ require (
 	github.com/valyala/quicktemplate v1.2.0
 	golang.org/x/tools v0.0.0-20200422022333-3d57cf2e726e
 	gopkg.in/yaml.v2 v2.2.8
+	honnef.co/go/tools v0.0.1-2020.1.3
 	mvdan.cc/interfacer v0.0.0-20180901003855-c20040233aed
 	mvdan.cc/lint v0.0.0-20170908181259-adc824a0674b // indirect
 	mvdan.cc/unparam v0.0.0-20190720180237-d51796306d8f
 )
-
-// See https://github.com/golangci/golangci-lint/issues/995
-// Update only after mitigating this issue.
-// TODO: Enable back tests with skip "Issue955" after update.
-require honnef.co/go/tools v0.0.1-2020.1.3

go.sum

@@ -366,7 +366,6 @@ golang.org/x/tools v0.0.0-20190311215038-5c2858a9cfe5/go.mod h1:LCzVGOaR6xXOjkQ3
 golang.org/x/tools v0.0.0-20190322203728-c1a832b0ad89/go.mod h1:LCzVGOaR6xXOjkQ3onu1FJEFr0SW1gC7cKk1uF8kGRs=
 golang.org/x/tools v0.0.0-20190328211700-ab21143f2384/go.mod h1:LCzVGOaR6xXOjkQ3onu1FJEFr0SW1gC7cKk1uF8kGRs=
 golang.org/x/tools v0.0.0-20190521203540-521d6ed310dd/go.mod h1:RgjU9mgBXZiqYHBnxXauZ1Gv1EHHAz9KjViQ78xBX0Q=
-golang.org/x/tools v0.0.0-20190621195816-6e04913cbbac/go.mod h1:/rFqwRUd4F7ZHNgwSSTFct+R/Kf4OFW1sUzUTQQTgfc=
 golang.org/x/tools v0.0.0-20190719005602-e377ae9d6386/go.mod h1:jcCCGcm9btYwXyDqrUWc6MKQKKGJCWEQ3AfLSRIbEuI=
 golang.org/x/tools v0.0.0-20190910044552-dd2b5c81c578/go.mod h1:b+2E5dAYhXwXZwtnZ6UAqBI28+e2cm9otk0dWdXHAEo=
 golang.org/x/tools v0.0.0-20191108193012-7d206e10da11/go.mod h1:b+2E5dAYhXwXZwtnZ6UAqBI28+e2cm9otk0dWdXHAEo=
@@ -378,7 +377,6 @@ golang.org/x/tools v0.0.0-20200324003944-a576cf524670/go.mod h1:Sl4aGygMT6LrqrWc
 golang.org/x/tools v0.0.0-20200414032229-332987a829c3/go.mod h1:EkVYQZoAsY45+roYkvgYkIh4xh/qjgUK9TdY2XT94GE=
 golang.org/x/tools v0.0.0-20200422022333-3d57cf2e726e h1:3Dzrrxi54Io7Aoyb0PYLsI47K2TxkRQg+cqUn+m04do=
 golang.org/x/tools v0.0.0-20200422022333-3d57cf2e726e/go.mod h1:EkVYQZoAsY45+roYkvgYkIh4xh/qjgUK9TdY2XT94GE=
-golang.org/x/tools v0.0.0-20200502202811-ed308ab3e770 h1:M9Fif0OxNji8w+HvmhVQ8KJtiZOsjU9RgslJGhn95XE=
 golang.org/x/xerrors v0.0.0-20190717185122-a985d3407aa7 h1:9zdDQZ7Thm29KFXgAX/+yaf3eVbP7djjWp/dXAppNCc=
 golang.org/x/xerrors v0.0.0-20190717185122-a985d3407aa7/go.mod h1:I/5z698sn9Ka8TeJc9MKroUUfqBBauWjQqLJ2OPfmY0=
 golang.org/x/xerrors v0.0.0-20191011141410-1b5146add898 h1:/atklqdjdhuosWIl6AIbOeHJjicWYPqR9bpxqxYG2pA=
@@ -387,6 +385,7 @@ golang.org/x/xerrors v0.0.0-20191204190536-9bdfabe68543 h1:E7g+9GITq07hpfrRu66IV
 golang.org/x/xerrors v0.0.0-20191204190536-9bdfabe68543/go.mod h1:I/5z698sn9Ka8TeJc9MKroUUfqBBauWjQqLJ2OPfmY0=
 google.golang.org/appengine v1.1.0 h1:igQkv0AAhEIvTEpD5LIpAfav2eeVO9HBTjvKHVJPRSs=
 google.golang.org/appengine v1.1.0/go.mod h1:EbEs0AVv82hx2wNQdGPgUI5lhzA/G0D9YwlJXL52JkM=
+google.golang.org/appengine v1.4.0 h1:/wp5JvzpHIxhs/dumFmF7BXTf3Z+dd4uXta4kVyO508=
 google.golang.org/appengine v1.4.0/go.mod h1:xpcJRLb0r/rnEns0DIKYYv+WjYCduHsrkT7/EB5XEv4=
 google.golang.org/genproto v0.0.0-20180817151627-c66870c02cf8/go.mod h1:JiN7NxoALGmiZfu7CAH4rXhgtRTLTxftemlI0sWmxmc=
 google.golang.org/grpc v1.19.0/go.mod h1:mqu4LbDTu4XGKhr4mRzUsmM4RtVoemTSY81AxZiDr8c=
@@ -413,8 +412,6 @@ gopkg.in/yaml.v2 v2.2.4/go.mod h1:hI93XBmqTisBFMUTm0b8Fm+jr3Dg1NNxqwp+5A1VGuI=
 gopkg.in/yaml.v2 v2.2.8 h1:obN1ZagJSUGI0Ek/LBmuj4SNLPfIny3KsKFopxRdj10=
 gopkg.in/yaml.v2 v2.2.8/go.mod h1:hI93XBmqTisBFMUTm0b8Fm+jr3Dg1NNxqwp+5A1VGuI=
 honnef.co/go/tools v0.0.0-20190102054323-c2f93a96b099/go.mod h1:rf3lG4BRIbNafJWhAfAdb/ePZxsR/4RtNHQocxwk9r4=
-honnef.co/go/tools v0.0.1-2019.2.3 h1:3JgtbtFHMiCmsznwGVTUWbgGov+pVqnlf1dEJTNAXeM=
-honnef.co/go/tools v0.0.1-2019.2.3/go.mod h1:a3bituU0lyd329TUQxRnasdCoJDkEUEAqEt0JzvZhAg=
 honnef.co/go/tools v0.0.1-2020.1.3 h1:sXmLre5bzIR6ypkjXCDI3jHPssRhc8KD/Ome589sc3U=
 honnef.co/go/tools v0.0.1-2020.1.3/go.mod h1:X/FiERA/W4tHapMX5mGpAtMSVEeEUOyHaw9vFzvIQ3k=
 mvdan.cc/interfacer v0.0.0-20180901003855-c20040233aed h1:WX1yoOaKQfddO/mLzdV4wptyWgoH/6hwLs7QHTixo0I=

pkg/commands/linters.go

@@ -20,32 +20,26 @@ func (e *Executor) initLinters() {
 	e.initRunConfiguration(lintersCmd)
 }
 
-func IsLinterInConfigsList(name string, linters []*linter.Config) bool {
-	for _, lc := range linters {
-		if lc.Name() == name {
-			return true
-		}
-	}
-
-	return false
-}
-
 func (e *Executor) executeLinters(_ *cobra.Command, args []string) {
 	if len(args) != 0 {
 		e.log.Fatalf("Usage: golangci-lint linters")
 	}
 
-	enabledLCs, err := e.EnabledLintersSet.Get(false)
+	enabledLintersMap, err := e.EnabledLintersSet.GetEnabledLintersMap()
 	if err != nil {
 		log.Fatalf("Can't get enabled linters: %s", err)
 	}
 
 	color.Green("Enabled by your configuration linters:\n")
-	printLinterConfigs(enabledLCs)
+	enabledLinters := make([]*linter.Config, 0, len(enabledLintersMap))
+	for _, linter := range enabledLintersMap {
+		enabledLinters = append(enabledLinters, linter)
+	}
+	printLinterConfigs(enabledLinters)
 
 	var disabledLCs []*linter.Config
 	for _, lc := range e.DBManager.GetAllSupportedLinterConfigs() {
-		if !IsLinterInConfigsList(lc.Name(), enabledLCs) {
+		if enabledLintersMap[lc.Name()] == nil {
 			disabledLCs = append(disabledLCs, lc)
 		}
 	}

pkg/commands/run.go

@@ -285,40 +285,34 @@ func fixSlicesFlags(fs *pflag.FlagSet) {
 func (e *Executor) runAnalysis(ctx context.Context, args []string) ([]result.Issue, error) {
 	e.cfg.Run.Args = args
 
-	enabledLinters, err := e.EnabledLintersSet.Get(true)
+	lintersToRun, err := e.EnabledLintersSet.GetOptimizedLinters()
 	if err != nil {
 		return nil, err
 	}
 
-	enabledOriginalLinters, err := e.EnabledLintersSet.Get(false)
+	enabledLintersMap, err := e.EnabledLintersSet.GetEnabledLintersMap()
 	if err != nil {
 		return nil, err
 	}
 
 	for _, lc := range e.DBManager.GetAllSupportedLinterConfigs() {
-		isEnabled := false
-		for _, enabledLC := range enabledOriginalLinters {
-			if enabledLC.Name() == lc.Name() {
-				isEnabled = true
-				break
-			}
-		}
+		isEnabled := enabledLintersMap[lc.Name()] != nil
 		e.reportData.AddLinter(lc.Name(), isEnabled, lc.EnabledByDefault)
 	}
 
-	lintCtx, err := e.contextLoader.Load(ctx, enabledLinters)
+	lintCtx, err := e.contextLoader.Load(ctx, lintersToRun)
 	if err != nil {
 		return nil, errors.Wrap(err, "context loading failed")
 	}
 	lintCtx.Log = e.log.Child("linters context")
 
 	runner, err := lint.NewRunner(e.cfg, e.log.Child("runner"),
-		e.goenv, e.lineCache, e.DBManager, lintCtx.Packages)
+		e.goenv, e.EnabledLintersSet, e.lineCache, e.DBManager, lintCtx.Packages)
 	if err != nil {
 		return nil, err
 	}
 
-	issues, err := runner.Run(ctx, enabledLinters, lintCtx)
+	issues, err := runner.Run(ctx, lintersToRun, lintCtx)
 	if err != nil {
 		return nil, err
 	}

pkg/lint/linter/config.go

@@ -22,9 +22,10 @@ type Config struct {
 	InPresets        []string
 	AlternativeNames []string
 
-	OriginalURL string // URL of original (not forked) repo, needed for autogenerated README
-	CanAutoFix  bool
-	IsSlow      bool
+	OriginalURL     string // URL of original (not forked) repo, needed for autogenerated README
+	CanAutoFix      bool
+	IsSlow          bool
+	DoesChangeTypes bool
 }
 
 func (lc *Config) ConsiderSlow() *Config {
@@ -67,6 +68,11 @@ func (lc *Config) WithAutoFix() *Config {
 	return lc
 }
 
+func (lc *Config) WithChangeTypes() *Config {
+	lc.DoesChangeTypes = true
+	return lc
+}
+
 func (lc *Config) AllNames() []string {
 	return append([]string{lc.Name()}, lc.AlternativeNames...)
 }

pkg/lint/lintersdb/enabled_set.go

@@ -77,16 +77,22 @@ func (es EnabledSet) build(lcfg *config.Linters, enabledByDefaultLinters []*lint
 	return resultLintersSet
 }
 
-func (es EnabledSet) Get(optimize bool) ([]*linter.Config, error) {
+func (es EnabledSet) GetEnabledLintersMap() (map[string]*linter.Config, error) {
+	if err := es.v.validateEnabledDisabledLintersConfig(&es.cfg.Linters); err != nil {
+		return nil, err
+	}
+
+	return es.build(&es.cfg.Linters, es.m.GetAllEnabledByDefaultLinters()), nil
+}
+
+func (es EnabledSet) GetOptimizedLinters() ([]*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())
 	es.verbosePrintLintersStatus(resultLintersSet)
-	if optimize {
-		es.combineGoAnalysisLinters(resultLintersSet)
-	}
+	es.combineGoAnalysisLinters(resultLintersSet)
 
 	var resultLinters []*linter.Config
 	for _, lc := range resultLintersSet {
@@ -95,7 +101,11 @@ func (es EnabledSet) Get(optimize bool) ([]*linter.Config, error) {
 
 	// Make order of execution of linters (go/analysis metalinter and unused) stable.
 	sort.Slice(resultLinters, func(i, j int) bool {
-		return strings.Compare(resultLinters[i].Name(), resultLinters[j].Name()) <= 0
+		a, b := resultLinters[i], resultLinters[j]
+		if a.DoesChangeTypes != b.DoesChangeTypes {
+			return b.DoesChangeTypes // move type-changing linters to the end to optimize speed
+		}
+		return strings.Compare(a.Name(), b.Name()) < 0
 	})
 
 	return resultLinters, nil

pkg/lint/lintersdb/manager.go

@@ -125,6 +125,7 @@ func (m Manager) GetAllSupportedLinterConfigs() []*linter.Config {
 			WithPresets(linter.PresetUnused).
 			WithAlternativeNames(megacheckName).
 			ConsiderSlow().
+			WithChangeTypes().
 			WithURL("https://github.com/dominikh/go-tools/tree/master/unused"),
 		linter.NewConfig(golinters.NewGosimple()).
 			WithLoadForGoAnalysis().

pkg/lint/runner.go

@@ -7,6 +7,8 @@ import (
 	"runtime/debug"
 	"strings"
 
+	"github.com/pkg/errors"
+
 	"github.com/golangci/golangci-lint/internal/errorutil"
 	"github.com/golangci/golangci-lint/pkg/config"
 	"github.com/golangci/golangci-lint/pkg/fsutils"
@@ -27,7 +29,7 @@ type Runner struct {
 	Log        logutils.Log
 }
 
-func NewRunner(cfg *config.Config, log logutils.Log, goenv *goutil.Env,
+func NewRunner(cfg *config.Config, log logutils.Log, goenv *goutil.Env, es *lintersdb.EnabledSet,
 	lineCache *fsutils.LineCache, dbManager *lintersdb.Manager, pkgs []*gopackages.Package) (*Runner, error) {
 	icfg := cfg.Issues
 	excludePatterns := icfg.ExcludePatterns
@@ -77,11 +79,9 @@ func NewRunner(cfg *config.Config, log logutils.Log, goenv *goutil.Env,
 		excludeRulesProcessor = processors.NewExcludeRules(excludeRules, lineCache, log.Child("exclude_rules"))
 	}
 
-	enabledLintersSet := lintersdb.NewEnabledSet(dbManager,
-		lintersdb.NewValidator(dbManager), log.Child("enabledLinters"), cfg)
-	lcs, err := enabledLintersSet.Get(false)
+	enabledLinters, err := es.GetEnabledLintersMap()
 	if err != nil {
-		return nil, err
+		return nil, errors.Wrap(err, "failed to get enabled linters")
 	}
 
 	return &Runner{
@@ -103,7 +103,7 @@ func NewRunner(cfg *config.Config, log logutils.Log, goenv *goutil.Env,
 
 			excludeProcessor,
 			excludeRulesProcessor,
-			processors.NewNolint(log.Child("nolint"), dbManager, lcs),
+			processors.NewNolint(log.Child("nolint"), dbManager, enabledLinters),
 
 			processors.NewUniqByLine(cfg),
 			processors.NewDiff(icfg.Diff, icfg.DiffFromRevision, icfg.DiffPatchFilePath),
@@ -131,14 +131,17 @@ func (r *Runner) runLinterSafe(ctx context.Context, lintCtx *linter.Context,
 		}
 	}()
 
-	specificLintCtx := *lintCtx
-	specificLintCtx.Log = r.Log.Child(lc.Name())
+	issues, err := lc.Linter.Run(ctx, lintCtx)
+
+	if lc.DoesChangeTypes {
+		// Packages in lintCtx might be dirty due to the last analysis,
+		// which affects to the next analysis.
+		// To avoid this issue, we clear type information from the packages.
+		// See https://github.com/golangci/golangci-lint/pull/944.
+		// Currently DoesChangeTypes is true only for `unused`.
+		lintCtx.ClearTypesInPackages()
+	}
 
-	// Packages in lintCtx might be dirty due to the last analysis,
-	// which affects to the next analysis.
-	// To avoid this issue, we clear type information from the packages.
-	specificLintCtx.ClearTypesInPackages()
-	issues, err := lc.Linter.Run(ctx, &specificLintCtx)
 	if err != nil {
 		return nil, err
 	}

pkg/result/processors/nolint.go

@@ -60,18 +60,13 @@ type filesCache map[string]*fileData
 type Nolint struct {
 	cache          filesCache
 	dbManager      *lintersdb.Manager
-	enabledLinters map[string]bool
+	enabledLinters map[string]*linter.Config
 	log            logutils.Log
 
 	unknownLintersSet map[string]bool
 }
 
-func NewNolint(log logutils.Log, dbManager *lintersdb.Manager, enabledLCs []*linter.Config) *Nolint {
-	enabledLinters := make(map[string]bool, len(enabledLCs))
-	for _, lc := range enabledLCs {
-		enabledLinters[lc.Name()] = true
-	}
-
+func NewNolint(log logutils.Log, dbManager *lintersdb.Manager, enabledLinters map[string]*linter.Config) *Nolint {
 	return &Nolint{
 		cache:             filesCache{},
 		dbManager:         dbManager,
@@ -154,7 +149,7 @@ func (p *Nolint) shouldPassIssue(i *result.Issue) (bool, error) {
 		if i.ExpectedNoLintLinter != "" {
 			// don't expect disabled linters to cover their nolint statements
 			nolintDebugf("enabled linters: %v", p.enabledLinters)
-			if !p.enabledLinters[i.ExpectedNoLintLinter] {
+			if p.enabledLinters[i.ExpectedNoLintLinter] == nil {
 				return false, nil
 			}
 			nolintDebugf("checking that lint issue was used for %s: %v", i.ExpectedNoLintLinter, i)

pkg/result/processors/nolint_test.go

@@ -253,9 +253,9 @@ func TestNolintUnused(t *testing.T) {
 		cfg := &config.Config{Linters: config.Linters{DisableAll: true, Enable: enabledLinters}}
 		dbManager := lintersdb.NewManager(cfg, nil)
 		enabledLintersSet := lintersdb.NewEnabledSet(dbManager, lintersdb.NewValidator(dbManager), enabledSetLog, cfg)
-		lcs, err := enabledLintersSet.Get(false)
+		enabledLintersMap, err := enabledLintersSet.GetEnabledLintersMap()
 		assert.NoError(t, err)
-		return NewNolint(log, dbManager, lcs)
+		return NewNolint(log, dbManager, enabledLintersMap)
 	}
 
 	// the issues below the nolintlint issues that would be generated for the test file
@@ -297,9 +297,9 @@ func TestNolintUnused(t *testing.T) {
 		dbManager := lintersdb.NewManager(cfg, nil)
 		enabledLintersSet := lintersdb.NewEnabledSet(dbManager, lintersdb.NewValidator(dbManager), enabledSetLog, cfg)
 
-		lcs, err := enabledLintersSet.Get(false)
+		enabledLintersMap, err := enabledLintersSet.GetEnabledLintersMap()
 		assert.NoError(t, err)
-		p := NewNolint(log, dbManager, lcs)
+		p := NewNolint(log, dbManager, enabledLintersMap)
 		defer p.Finish()
 
 		processAssertEmpty(t, p, nolintlintIssueVarcheck)