From 79c78d6f6f802d66b4da6257b5578d53191c7d61 Mon Sep 17 00:00:00 2001 From: Ludovic Fernandez Date: Thu, 29 Jun 2023 22:40:59 +0200 Subject: [PATCH] feat: explain typecheck and remove it from the linter list (#3929) --- docs/src/docs/usage/faq.mdx | 19 ++++++++++++++++++ pkg/commands/help.go | 10 +++++++++- pkg/commands/linters.go | 14 ++++++++++--- pkg/lint/linter/config.go | 6 ++++++ pkg/lint/lintersdb/enabled_set.go | 20 ++++++++++++++++--- pkg/lint/lintersdb/enabled_set_test.go | 23 +++++++++++++--------- pkg/lint/lintersdb/manager.go | 1 + pkg/result/processors/nolint_test.go | 4 ++++ scripts/expand_website_templates/main.go | 4 ++++ test/enabled_linters_test.go | 4 ++++ test/run_test.go | 2 +- test/testdata/linedirective/gomodguard.yml | 2 +- test/testdata/linedirective/hello.go | 6 +++++- 13 files changed, 96 insertions(+), 19 deletions(-) diff --git a/docs/src/docs/usage/faq.mdx b/docs/src/docs/usage/faq.mdx index 35ca3bda..22a140ff 100644 --- a/docs/src/docs/usage/faq.mdx +++ b/docs/src/docs/usage/faq.mdx @@ -32,3 +32,22 @@ The same as the Go team (the 2 last minor versions) Because the first run caches type information. All subsequent runs will be fast. Usually this options is used during development on local machine and compilation was already performed. + +## Why do you have `typecheck` errors? + +`typecheck` is like the front-end of a Go compiler, parses and type-checks Go code, it manages compilation errors. + +It cannot be disabled because of that. + +Of course, this is just as good as the compiler itself and a lot of compilation issues will not properly show where in the code your error lies. + +`typecheck` is not a real linter, it's just a way to parse compiling errors (produced by the `types.Checker`) and some linter errors. + +If there are `typecheck` errors, golangci-lint will not able to produce other reports because that kind of error doesn't allow it to perform analysis. + +How to troubleshoot: + +- [ ] Ensure the version of `golangci-lint` is built with a compatible version of Go. +- [ ] Ensure dependencies are up-to-date with `go mod tidy`. +- [ ] Ensure building works with `go run ./...`/`go build ./...` - whole package. +- [ ] If using CGO, ensure all require system libraries are installed. diff --git a/pkg/commands/help.go b/pkg/commands/help.go index 61d362e7..a06d508f 100644 --- a/pkg/commands/help.go +++ b/pkg/commands/help.go @@ -63,6 +63,10 @@ func printLinterConfigs(lcs []*linter.Config) { func (e *Executor) executeLintersHelp(_ *cobra.Command, _ []string) { var enabledLCs, disabledLCs []*linter.Config for _, lc := range e.DBManager.GetAllSupportedLinterConfigs() { + if lc.Internal { + continue + } + if lc.EnabledByDefault { enabledLCs = append(enabledLCs, lc) } else { @@ -78,8 +82,12 @@ func (e *Executor) executeLintersHelp(_ *cobra.Command, _ []string) { color.Green("\nLinters presets:") for _, p := range e.DBManager.AllPresets() { linters := e.DBManager.GetAllLinterConfigsForPreset(p) - linterNames := make([]string, 0, len(linters)) + var linterNames []string for _, lc := range linters { + if lc.Internal { + continue + } + linterNames = append(linterNames, lc.Name()) } sort.Strings(linterNames) diff --git a/pkg/commands/linters.go b/pkg/commands/linters.go index 13bb944d..292713ec 100644 --- a/pkg/commands/linters.go +++ b/pkg/commands/linters.go @@ -29,14 +29,22 @@ func (e *Executor) executeLinters(_ *cobra.Command, _ []string) error { } color.Green("Enabled by your configuration linters:\n") - enabledLinters := make([]*linter.Config, 0, len(enabledLintersMap)) - for _, linter := range enabledLintersMap { - enabledLinters = append(enabledLinters, linter) + var enabledLinters []*linter.Config + for _, lc := range enabledLintersMap { + if lc.Internal { + continue + } + + enabledLinters = append(enabledLinters, lc) } printLinterConfigs(enabledLinters) var disabledLCs []*linter.Config for _, lc := range e.DBManager.GetAllSupportedLinterConfigs() { + if lc.Internal { + continue + } + if enabledLintersMap[lc.Name()] == nil { disabledLCs = append(disabledLCs, lc) } diff --git a/pkg/lint/linter/config.go b/pkg/lint/linter/config.go index 5891ec27..0376afba 100644 --- a/pkg/lint/linter/config.go +++ b/pkg/lint/linter/config.go @@ -42,6 +42,7 @@ type Config struct { AlternativeNames []string OriginalURL string // URL of original (not forked) repo, needed for autogenerated README + Internal bool // Internal linters cannot be disabled (ex: typecheck). CanAutoFix bool IsSlow bool DoesChangeTypes bool @@ -55,6 +56,11 @@ func (lc *Config) WithEnabledByDefault() *Config { return lc } +func (lc *Config) WithInternal() *Config { + lc.Internal = true + return lc +} + func (lc *Config) ConsiderSlow() *Config { lc.IsSlow = true return lc diff --git a/pkg/lint/lintersdb/enabled_set.go b/pkg/lint/lintersdb/enabled_set.go index 92615b57..c5c7874e 100644 --- a/pkg/lint/lintersdb/enabled_set.go +++ b/pkg/lint/lintersdb/enabled_set.go @@ -31,8 +31,10 @@ func NewEnabledSet(m *Manager, v *Validator, log logutils.Log, cfg *config.Confi } } +//nolint:gocyclo // the complexity cannot be reduced. func (es EnabledSet) build(lcfg *config.Linters, enabledByDefaultLinters []*linter.Config) map[string]*linter.Config { es.debugf("Linters config: %#v", lcfg) + resultLintersSet := map[string]*linter.Config{} switch { case len(lcfg.Presets) != 0: @@ -78,6 +80,14 @@ func (es EnabledSet) build(lcfg *config.Linters, enabledByDefaultLinters []*lint } } + // typecheck is not a real linter and cannot be disabled. + if _, ok := resultLintersSet["typecheck"]; !ok && (es.cfg == nil || !es.cfg.InternalCmdTest) { + for _, lc := range es.m.GetLinterConfigs("typecheck") { + // it's important to use lc.Name() nor name because name can be alias + resultLintersSet[lc.Name()] = lc + } + } + return resultLintersSet } @@ -134,8 +144,8 @@ func (es EnabledSet) GetOptimizedLinters() ([]*linter.Config, error) { func (es EnabledSet) combineGoAnalysisLinters(linters map[string]*linter.Config) { var goanalysisLinters []*goanalysis.Linter goanalysisPresets := map[string]bool{} - for _, linter := range linters { - lnt, ok := linter.Linter.(*goanalysis.Linter) + for _, lc := range linters { + lnt, ok := lc.Linter.(*goanalysis.Linter) if !ok { continue } @@ -144,7 +154,7 @@ func (es EnabledSet) combineGoAnalysisLinters(linters map[string]*linter.Config) continue } goanalysisLinters = append(goanalysisLinters, lnt) - for _, p := range linter.InPresets { + for _, p := range lc.InPresets { goanalysisPresets[p] = true } } @@ -197,6 +207,10 @@ func (es EnabledSet) combineGoAnalysisLinters(linters map[string]*linter.Config) func (es EnabledSet) verbosePrintLintersStatus(lcs map[string]*linter.Config) { var linterNames []string for _, lc := range lcs { + if lc.Internal { + continue + } + linterNames = append(linterNames, lc.Name()) } sort.StringSlice(linterNames).Sort() diff --git a/pkg/lint/lintersdb/enabled_set_test.go b/pkg/lint/lintersdb/enabled_set_test.go index 3a6baa18..1ab75d4d 100644 --- a/pkg/lint/lintersdb/enabled_set_test.go +++ b/pkg/lint/lintersdb/enabled_set_test.go @@ -16,7 +16,9 @@ func TestGetEnabledLintersSet(t *testing.T) { def []string // enabled by default linters exp []string // alphabetically ordered enabled linter names } + allMegacheckLinterNames := []string{"gosimple", "staticcheck", "unused"} + cases := []cs{ { cfg: config.Linters{ @@ -24,7 +26,7 @@ func TestGetEnabledLintersSet(t *testing.T) { }, name: "disable all linters from megacheck", def: allMegacheckLinterNames, - exp: nil, // all disabled + exp: []string{"typecheck"}, // all disabled }, { cfg: config.Linters{ @@ -32,12 +34,12 @@ func TestGetEnabledLintersSet(t *testing.T) { }, name: "disable only staticcheck", def: allMegacheckLinterNames, - exp: []string{"gosimple", "unused"}, + exp: []string{"gosimple", "typecheck", "unused"}, }, { name: "don't merge into megacheck", def: allMegacheckLinterNames, - exp: allMegacheckLinterNames, + exp: []string{"gosimple", "staticcheck", "typecheck", "unused"}, }, { name: "expand megacheck", @@ -45,33 +47,33 @@ func TestGetEnabledLintersSet(t *testing.T) { Enable: []string{"megacheck"}, }, def: nil, - exp: allMegacheckLinterNames, + exp: []string{"gosimple", "staticcheck", "typecheck", "unused"}, }, { name: "don't disable anything", - def: []string{"gofmt", "govet"}, - exp: []string{"gofmt", "govet"}, + def: []string{"gofmt", "govet", "typecheck"}, + exp: []string{"gofmt", "govet", "typecheck"}, }, { name: "enable gosec by gas alias", cfg: config.Linters{ Enable: []string{"gas"}, }, - exp: []string{"gosec"}, + exp: []string{"gosec", "typecheck"}, }, { name: "enable gosec by primary name", cfg: config.Linters{ Enable: []string{"gosec"}, }, - exp: []string{"gosec"}, + exp: []string{"gosec", "typecheck"}, }, { name: "enable gosec by both names", cfg: config.Linters{ Enable: []string{"gosec", "gas"}, }, - exp: []string{"gosec"}, + exp: []string{"gosec", "typecheck"}, }, { name: "disable gosec by gas alias", @@ -79,6 +81,7 @@ func TestGetEnabledLintersSet(t *testing.T) { Disable: []string{"gas"}, }, def: []string{"gosec"}, + exp: []string{"typecheck"}, }, { name: "disable gosec by primary name", @@ -86,11 +89,13 @@ func TestGetEnabledLintersSet(t *testing.T) { Disable: []string{"gosec"}, }, def: []string{"gosec"}, + exp: []string{"typecheck"}, }, } m := NewManager(nil, nil) es := NewEnabledSet(m, NewValidator(m), nil, nil) + for _, c := range cases { c := c t.Run(c.name, func(t *testing.T) { diff --git a/pkg/lint/lintersdb/manager.go b/pkg/lint/lintersdb/manager.go index ca227c9a..16b1babf 100644 --- a/pkg/lint/lintersdb/manager.go +++ b/pkg/lint/lintersdb/manager.go @@ -804,6 +804,7 @@ func (m Manager) GetAllSupportedLinterConfigs() []*linter.Config { WithURL("https://github.com/moricho/tparallel"), linter.NewConfig(golinters.NewTypecheck()). + WithInternal(). WithEnabledByDefault(). WithSince("v1.3.0"). WithLoadForGoAnalysis(). diff --git a/pkg/result/processors/nolint_test.go b/pkg/result/processors/nolint_test.go index 0ebe8c9a..5e40531c 100644 --- a/pkg/result/processors/nolint_test.go +++ b/pkg/result/processors/nolint_test.go @@ -279,11 +279,15 @@ func TestNolintUnused(t *testing.T) { createProcessor := func(t *testing.T, log *logutils.MockLog, enabledLinters []string) *Nolint { enabledSetLog := logutils.NewMockLog() enabledSetLog.On("Infof", "Active %d linters: %s", len(enabledLinters), enabledLinters) + cfg := &config.Config{Linters: config.Linters{DisableAll: true, Enable: enabledLinters}} dbManager := lintersdb.NewManager(cfg, nil) + enabledLintersSet := lintersdb.NewEnabledSet(dbManager, lintersdb.NewValidator(dbManager), enabledSetLog, cfg) + enabledLintersMap, err := enabledLintersSet.GetEnabledLintersMap() assert.NoError(t, err) + return NewNolint(log, dbManager, enabledLintersMap) } diff --git a/scripts/expand_website_templates/main.go b/scripts/expand_website_templates/main.go index 1d49885f..f5566a95 100644 --- a/scripts/expand_website_templates/main.go +++ b/scripts/expand_website_templates/main.go @@ -238,6 +238,10 @@ func getLintersListMarkdown(enabled bool) string { var neededLcs []*linter.Config lcs := lintersdb.NewManager(nil, nil).GetAllSupportedLinterConfigs() for _, lc := range lcs { + if lc.Internal { + continue + } + if lc.EnabledByDefault == enabled { neededLcs = append(neededLcs, lc) } diff --git a/test/enabled_linters_test.go b/test/enabled_linters_test.go index 622b0a2c..4705c94f 100644 --- a/test/enabled_linters_test.go +++ b/test/enabled_linters_test.go @@ -170,6 +170,10 @@ func getEnabledByDefaultLinters() []string { ebdl := lintersdb.NewManager(nil, nil).GetAllEnabledByDefaultLinters() var ret []string for _, lc := range ebdl { + if lc.Internal { + continue + } + ret = append(ret, lc.Name()) } diff --git a/test/run_test.go b/test/run_test.go index ec532541..b1f245dc 100644 --- a/test/run_test.go +++ b/test/run_test.go @@ -243,7 +243,7 @@ func TestLineDirective(t *testing.T) { }, configPath: "testdata/linedirective/gomodguard.yml", targetPath: "linedirective", - expected: "import of package `github.com/ryancurrah/gomodguard` is blocked because the module is not " + + expected: "import of package `golang.org/x/tools/go/analysis` is blocked because the module is not " + "in the allowed modules list. (gomodguard)", }, { diff --git a/test/testdata/linedirective/gomodguard.yml b/test/testdata/linedirective/gomodguard.yml index 6e1b0ef8..e6f4125b 100644 --- a/test/testdata/linedirective/gomodguard.yml +++ b/test/testdata/linedirective/gomodguard.yml @@ -2,4 +2,4 @@ linters-settings: gomodguard: allowed: domains: - - golang.org + - github.com diff --git a/test/testdata/linedirective/hello.go b/test/testdata/linedirective/hello.go index c511f8b5..b4699f7a 100644 --- a/test/testdata/linedirective/hello.go +++ b/test/testdata/linedirective/hello.go @@ -3,7 +3,7 @@ package main import ( - "github.com/ryancurrah/gomodguard" + "golang.org/x/tools/go/analysis" ) func _() { @@ -26,6 +26,10 @@ func b() { fmt.Println("foo") } +func c(){ + _ = analysis.Analyzer{} +} + func wsl() bool { return true