From 95ec0cf21ea4720ec9555c43176ab8b1d1b035f2 Mon Sep 17 00:00:00 2001 From: Isaev Denis Date: Mon, 30 Sep 2019 16:19:41 +0300 Subject: [PATCH] dramatically reduce memory usage (#758) Run all linters per package. It allows unloading package data when it's processed. It dramatically reduces memory (and CPU because of GC) usage. Relates: #337 --- Makefile | 1 + README.md | 21 +- cmd/golangci-lint/mod_version.go | 2 +- go.mod | 4 +- go.sum | 8 +- pkg/commands/run.go | 30 +- pkg/golinters/bodyclose.go | 2 +- pkg/golinters/deadcode.go | 68 ++-- pkg/golinters/depguard.go | 106 +++--- pkg/golinters/dogsled.go | 57 +-- pkg/golinters/dupl.go | 101 ++++-- pkg/golinters/errcheck.go | 97 ++--- pkg/golinters/funlen.go | 74 ++-- pkg/golinters/goanalysis/adapters.go | 36 ++ pkg/golinters/goanalysis/interface.go | 11 - pkg/golinters/goanalysis/linter.go | 185 ++++++++-- .../linter_test.go} | 5 +- pkg/golinters/goanalysis/metalinter.go | 113 ++++-- pkg/golinters/goanalysis/runner.go | 213 ++++++----- pkg/golinters/gochecknoglobals.go | 53 ++- pkg/golinters/gochecknoinits.go | 52 ++- pkg/golinters/goconst.go | 59 +-- pkg/golinters/gocritic.go | 136 ++++--- pkg/golinters/gocyclo.go | 80 +++-- pkg/golinters/godox.go | 76 ++-- pkg/golinters/gofmt.go | 335 +++--------------- pkg/golinters/gofmt_common.go | 267 ++++++++++++++ pkg/golinters/goimports.go | 72 ++++ pkg/golinters/golint.go | 74 ++-- pkg/golinters/gosec.go | 105 +++--- pkg/golinters/gosimple.go | 19 + pkg/golinters/govet.go | 2 +- pkg/golinters/ineffassign.go | 71 ++-- pkg/golinters/interfacer.go | 81 +++-- pkg/golinters/lll.go | 75 ++-- pkg/golinters/maligned.go | 72 ++-- pkg/golinters/megacheck.go | 274 +------------- pkg/golinters/misspell.go | 136 ++++--- pkg/golinters/nakedret.go | 65 ++-- pkg/golinters/prealloc.go | 67 ++-- pkg/golinters/scopelint.go | 64 ++-- pkg/golinters/staticcheck.go | 19 + pkg/golinters/structcheck.go | 67 ++-- pkg/golinters/stylecheck.go | 19 + pkg/golinters/typecheck.go | 67 +--- pkg/golinters/unconvert.go | 67 ++-- pkg/golinters/unparam.go | 88 +++-- pkg/golinters/unused.go | 40 +++ pkg/golinters/util.go | 40 --- pkg/golinters/varcheck.go | 67 ++-- pkg/golinters/whitespace.go | 110 +++--- pkg/lint/astcache/astcache.go | 3 +- pkg/lint/linter/config.go | 42 +-- pkg/lint/linter/context.go | 19 +- pkg/lint/linter/metalinter.go | 8 - pkg/lint/lintersdb/enabled_set.go | 84 +---- pkg/lint/lintersdb/enabled_set_test.go | 27 +- pkg/lint/lintersdb/manager.go | 185 ++++------ pkg/lint/lintersdb/validator.go | 2 +- pkg/lint/load.go | 163 +-------- pkg/lint/runner.go | 207 ++--------- pkg/printers/checkstyle.go | 4 +- pkg/printers/codeclimate.go | 4 +- pkg/printers/json.go | 9 +- pkg/printers/junitxml.go | 4 +- pkg/printers/printer.go | 2 +- pkg/printers/tab.go | 4 +- pkg/printers/text.go | 4 +- pkg/result/processors/fixer.go | 50 ++- pkg/result/processors/nolint.go | 20 +- test/enabled_linters_test.go | 4 +- .../github.com/golangci/gofmt/gofmt/gofmt.go | 20 +- vendor/github.com/golangci/lint-1/lint.go | 96 +---- vendor/modules.txt | 4 +- 74 files changed, 2488 insertions(+), 2430 deletions(-) create mode 100644 pkg/golinters/goanalysis/adapters.go delete mode 100644 pkg/golinters/goanalysis/interface.go rename pkg/golinters/{typecheck_test.go => goanalysis/linter_test.go} (90%) create mode 100644 pkg/golinters/gofmt_common.go create mode 100644 pkg/golinters/goimports.go create mode 100644 pkg/golinters/gosimple.go create mode 100644 pkg/golinters/staticcheck.go create mode 100644 pkg/golinters/stylecheck.go create mode 100644 pkg/golinters/unused.go delete mode 100644 pkg/lint/linter/metalinter.go diff --git a/Makefile b/Makefile index fcaeb214bdb5..548c75c888f2 100644 --- a/Makefile +++ b/Makefile @@ -29,6 +29,7 @@ clean: test: export GOLANGCI_LINT_INSTALLED = true test: build GL_TEST_RUN=1 time ./golangci-lint run -v + time go run ./cmd/golangci-lint/main.go run -v GL_TEST_RUN=1 time ./golangci-lint run --fast --no-config -v --skip-dirs 'test/testdata_etc,internal/(cache|renameio|robustio)' GL_TEST_RUN=1 time ./golangci-lint run --no-config -v --skip-dirs 'test/testdata_etc,internal/(cache|renameio|robustio)' GL_TEST_RUN=1 time go test -v ./... diff --git a/README.md b/README.md index 6e1569b41c82..e0d18e7c0ba7 100644 --- a/README.md +++ b/README.md @@ -178,13 +178,13 @@ $ golangci-lint help linters Enabled by default linters: deadcode: Finds unused code [fast: true, auto-fix: false] errcheck: Errcheck is a program for checking for unchecked errors in go programs. These unchecked errors can be critical bugs in some cases [fast: true, auto-fix: false] -gosimple: Linter for Go source code that specializes in simplifying a code [fast: false, auto-fix: false] -govet (vet, vetshadow): Vet examines Go source code and reports suspicious constructs, such as Printf calls whose arguments do not align with the format string [fast: false, auto-fix: false] +gosimple (megacheck): Linter for Go source code that specializes in simplifying a code [fast: true, auto-fix: false] +govet (vet, vetshadow): Vet examines Go source code and reports suspicious constructs, such as Printf calls whose arguments do not align with the format string [fast: true, auto-fix: false] ineffassign: Detects when assignments to existing variables are not used [fast: true, auto-fix: false] -staticcheck: Staticcheck is a go vet on steroids, applying a ton of static analysis checks [fast: false, auto-fix: false] +staticcheck (megacheck): Staticcheck is a go vet on steroids, applying a ton of static analysis checks [fast: true, auto-fix: false] structcheck: Finds unused struct fields [fast: true, auto-fix: false] typecheck: Like the front-end of a Go compiler, parses and type-checks Go code [fast: true, auto-fix: false] -unused: Checks Go code for unused constants, variables, functions and types [fast: false, auto-fix: false] +unused (megacheck): Checks Go code for unused constants, variables, functions and types [fast: false, auto-fix: false] varcheck: Finds unused global variables and constants [fast: true, auto-fix: false] ``` @@ -194,12 +194,12 @@ and the following linters are disabled by default: $ golangci-lint help linters ... Disabled by default linters: -bodyclose: checks whether HTTP response body is closed successfully [fast: false, auto-fix: false] +bodyclose: checks whether HTTP response body is closed successfully [fast: true, auto-fix: false] depguard: Go linter that checks if package imports are in a list of acceptable packages [fast: true, auto-fix: false] dogsled: Checks assignments with too many blank identifiers (e.g. x, _, _, _, := f()) [fast: true, auto-fix: false] dupl: Tool for code clone detection [fast: true, auto-fix: false] funlen: Tool for detection of long functions [fast: true, auto-fix: false] -gochecknoglobals: Checks that no globals are present in Go code [fast: true, auto-fix: false] +gochecknoglobals: Tool for detection of long functions [fast: true, auto-fix: false] gochecknoinits: Checks that no init functions are present in Go code [fast: true, auto-fix: false] goconst: Finds repeated strings that could be replaced by a constant [fast: true, auto-fix: false] gocritic: The most opinionated Go source code linter [fast: true, auto-fix: false] @@ -209,16 +209,16 @@ gofmt: Gofmt checks whether code was gofmt-ed. By default this tool runs with -s goimports: Goimports does everything that gofmt does. Additionally it checks unused imports [fast: true, auto-fix: true] golint: Golint differs from gofmt. Gofmt reformats Go source code, whereas golint prints out style mistakes [fast: true, auto-fix: false] gosec (gas): Inspects source code for security problems [fast: true, auto-fix: false] -interfacer: Linter that suggests narrower interface types [fast: false, auto-fix: false] +interfacer: Linter that suggests narrower interface types [fast: true, auto-fix: false] lll: Reports long lines [fast: true, auto-fix: false] maligned: Tool to detect Go structs that would take less memory if their fields were sorted [fast: true, auto-fix: false] misspell: Finds commonly misspelled English words in comments [fast: true, auto-fix: true] nakedret: Finds naked returns in functions greater than a specified function length [fast: true, auto-fix: false] prealloc: Finds slice declarations that could potentially be preallocated [fast: true, auto-fix: false] scopelint: Scopelint checks for unpinned variables in go programs [fast: true, auto-fix: false] -stylecheck: Stylecheck is a replacement for golint [fast: false, auto-fix: false] +stylecheck: Stylecheck is a replacement for golint [fast: true, auto-fix: false] unconvert: Remove unnecessary type conversions [fast: true, auto-fix: false] -unparam: Reports unused function parameters [fast: false, auto-fix: false] +unparam: Reports unused function parameters [fast: true, auto-fix: false] whitespace: Tool for detection of leading and trailing whitespace [fast: true, auto-fix: true] ``` @@ -462,7 +462,7 @@ golangci-lint help linters - [scopelint](https://github.com/kyoh86/scopelint) - Scopelint checks for unpinned variables in go programs - [gocritic](https://github.com/go-critic/go-critic) - The most opinionated Go source code linter - [gochecknoinits](https://github.com/leighmcculloch/gochecknoinits) - Checks that no init functions are present in Go code -- [gochecknoglobals](https://github.com/leighmcculloch/gochecknoglobals) - Checks that no globals are present in Go code +- [gochecknoglobals](https://github.com/leighmcculloch/gochecknoglobals) - Tool for detection of long functions - [godox](https://github.com/matoous/godox) - Tool for detection of FIXME, TODO and other comment keywords - [funlen](https://github.com/ultraware/funlen) - Tool for detection of long functions - [whitespace](https://github.com/ultraware/whitespace) - Tool for detection of leading and trailing whitespace @@ -563,6 +563,7 @@ Global Flags: --mem-profile-path string Path to memory profile output file --trace-path string Path to trace output file -v, --verbose verbose output + --version Print version ``` diff --git a/cmd/golangci-lint/mod_version.go b/cmd/golangci-lint/mod_version.go index 0311853d27a1..7b596728e169 100644 --- a/cmd/golangci-lint/mod_version.go +++ b/cmd/golangci-lint/mod_version.go @@ -10,7 +10,7 @@ import ( //nolint:gochecknoinits func init() { if info, available := debug.ReadBuildInfo(); available { - if date == "" && info.Main.Version != "(devel)" { + if date == "" { version = info.Main.Version commit = fmt.Sprintf("(unknown, mod sum: %q)", info.Main.Sum) date = "(unknown)" diff --git a/go.mod b/go.mod index f95e00139ce5..b018dec505a3 100644 --- a/go.mod +++ b/go.mod @@ -13,9 +13,9 @@ require ( github.com/golangci/go-misc v0.0.0-20180628070357-927a3d87b613 github.com/golangci/goconst v0.0.0-20180610141641-041c5f2b40f3 github.com/golangci/gocyclo v0.0.0-20180528134321-2becd97e67ee - github.com/golangci/gofmt v0.0.0-20181222123516-0b8337e80d98 + github.com/golangci/gofmt v0.0.0-20190930125516-244bba706f1a github.com/golangci/ineffassign v0.0.0-20190609212857-42439a7714cc - github.com/golangci/lint-1 v0.0.0-20190420132249-ee948d087217 + github.com/golangci/lint-1 v0.0.0-20190930103755-fad67e08aa89 github.com/golangci/maligned v0.0.0-20180506175553-b1d89398deca github.com/golangci/misspell v0.0.0-20180809174111-950f5d19e770 github.com/golangci/prealloc v0.0.0-20180630174525-215b22d4de21 diff --git a/go.sum b/go.sum index 446dcf95f21a..d1d572279962 100644 --- a/go.sum +++ b/go.sum @@ -86,12 +86,12 @@ github.com/golangci/goconst v0.0.0-20180610141641-041c5f2b40f3 h1:pe9JHs3cHHDQgO github.com/golangci/goconst v0.0.0-20180610141641-041c5f2b40f3/go.mod h1:JXrF4TWy4tXYn62/9x8Wm/K/dm06p8tCKwFRDPZG/1o= github.com/golangci/gocyclo v0.0.0-20180528134321-2becd97e67ee h1:J2XAy40+7yz70uaOiMbNnluTg7gyQhtGqLQncQh+4J8= github.com/golangci/gocyclo v0.0.0-20180528134321-2becd97e67ee/go.mod h1:ozx7R9SIwqmqf5pRP90DhR2Oay2UIjGuKheCBCNwAYU= -github.com/golangci/gofmt v0.0.0-20181222123516-0b8337e80d98 h1:0OkFarm1Zy2CjCiDKfK9XHgmc2wbDlRMD2hD8anAJHU= -github.com/golangci/gofmt v0.0.0-20181222123516-0b8337e80d98/go.mod h1:9qCChq59u/eW8im404Q2WWTrnBUQKjpNYKMbU4M7EFU= +github.com/golangci/gofmt v0.0.0-20190930125516-244bba706f1a h1:iR3fYXUjHCR97qWS8ch1y9zPNsgXThGwjKPrYfqMPks= +github.com/golangci/gofmt v0.0.0-20190930125516-244bba706f1a/go.mod h1:9qCChq59u/eW8im404Q2WWTrnBUQKjpNYKMbU4M7EFU= github.com/golangci/ineffassign v0.0.0-20190609212857-42439a7714cc h1:gLLhTLMk2/SutryVJ6D4VZCU3CUqr8YloG7FPIBWFpI= github.com/golangci/ineffassign v0.0.0-20190609212857-42439a7714cc/go.mod h1:e5tpTHCfVze+7EpLEozzMB3eafxo2KT5veNg1k6byQU= -github.com/golangci/lint-1 v0.0.0-20190420132249-ee948d087217 h1:En/tZdwhAn0JNwLuXzP3k2RVtMqMmOEK7Yu/g3tmtJE= -github.com/golangci/lint-1 v0.0.0-20190420132249-ee948d087217/go.mod h1:66R6K6P6VWk9I95jvqGxkqJxVWGFy9XlDwLwVz1RCFg= +github.com/golangci/lint-1 v0.0.0-20190930103755-fad67e08aa89 h1:664ewjIQUXDvinFMbAsoH2V2Yvaro/X8BoYpIMTWGXI= +github.com/golangci/lint-1 v0.0.0-20190930103755-fad67e08aa89/go.mod h1:66R6K6P6VWk9I95jvqGxkqJxVWGFy9XlDwLwVz1RCFg= github.com/golangci/maligned v0.0.0-20180506175553-b1d89398deca h1:kNY3/svz5T29MYHubXix4aDDuE3RWHkPvopM/EDv/MA= github.com/golangci/maligned v0.0.0-20180506175553-b1d89398deca/go.mod h1:tvlJhZqDe4LMs4ZHD0oMUlt9G2LWuDGoisJTBzLMV9o= github.com/golangci/misspell v0.0.0-20180809174111-950f5d19e770 h1:EL/O5HGrF7Jaq0yNhBLucz9hTuRzj2LdwGBOaENgxIk= diff --git a/pkg/commands/run.go b/pkg/commands/run.go index 3b3a64226085..03f41706d402 100644 --- a/pkg/commands/run.go +++ b/pkg/commands/run.go @@ -265,7 +265,7 @@ func fixSlicesFlags(fs *pflag.FlagSet) { }) } -func (e *Executor) runAnalysis(ctx context.Context, args []string) (<-chan result.Issue, error) { +func (e *Executor) runAnalysis(ctx context.Context, args []string) ([]result.Issue, error) { e.cfg.Run.Args = args enabledLinters, err := e.EnabledLintersSet.Get(true) @@ -296,9 +296,9 @@ func (e *Executor) runAnalysis(ctx context.Context, args []string) (<-chan resul return nil, err } - issuesCh := runner.Run(ctx, enabledLinters, lintCtx) + issues := runner.Run(ctx, enabledLinters, lintCtx) fixer := processors.NewFixer(e.cfg, e.log, e.fileCache) - return fixer.Process(issuesCh), nil + return fixer.Process(issues), nil } func (e *Executor) setOutputToDevNull() (savedStdout, savedStderr *os.File) { @@ -313,24 +313,10 @@ func (e *Executor) setOutputToDevNull() (savedStdout, savedStderr *os.File) { return } -func (e *Executor) setExitCodeIfIssuesFound(issues <-chan result.Issue) <-chan result.Issue { - resCh := make(chan result.Issue, 1024) - - go func() { - issuesFound := false - for i := range issues { - issuesFound = true - resCh <- i - } - - if issuesFound { - e.exitCode = e.cfg.Run.ExitCodeIfIssuesFound - } - - close(resCh) - }() - - return resCh +func (e *Executor) setExitCodeIfIssuesFound(issues []result.Issue) { + if len(issues) != 0 { + e.exitCode = e.cfg.Run.ExitCodeIfIssuesFound + } } func (e *Executor) runAndPrint(ctx context.Context, args []string) error { @@ -357,7 +343,7 @@ func (e *Executor) runAndPrint(ctx context.Context, args []string) error { return err } - issues = e.setExitCodeIfIssuesFound(issues) + e.setExitCodeIfIssuesFound(issues) if err = p.Print(ctx, issues); err != nil { return fmt.Errorf("can't print %d issues: %s", len(issues), err) diff --git a/pkg/golinters/bodyclose.go b/pkg/golinters/bodyclose.go index 626550811ff8..0e03813d1a77 100644 --- a/pkg/golinters/bodyclose.go +++ b/pkg/golinters/bodyclose.go @@ -17,5 +17,5 @@ func NewBodyclose() *goanalysis.Linter { "checks whether HTTP response body is closed successfully", analyzers, nil, - ) + ).WithLoadMode(goanalysis.LoadModeTypesInfo) } diff --git a/pkg/golinters/deadcode.go b/pkg/golinters/deadcode.go index d6ab4f26e5d7..269275ceb597 100644 --- a/pkg/golinters/deadcode.go +++ b/pkg/golinters/deadcode.go @@ -1,42 +1,52 @@ package golinters import ( - "context" "fmt" + "sync" deadcodeAPI "github.com/golangci/go-misc/deadcode" + "golang.org/x/tools/go/analysis" + "github.com/golangci/golangci-lint/pkg/golinters/goanalysis" "github.com/golangci/golangci-lint/pkg/lint/linter" "github.com/golangci/golangci-lint/pkg/result" ) -type Deadcode struct{} - -func (Deadcode) Name() string { - return "deadcode" -} - -func (Deadcode) Desc() string { - return "Finds unused code" -} - -func (d Deadcode) Run(ctx context.Context, lintCtx *linter.Context) ([]result.Issue, error) { - issues, err := deadcodeAPI.Run(lintCtx.Program) - if err != nil { - return nil, err - } - - if len(issues) == 0 { - return nil, nil - } - - res := make([]result.Issue, 0, len(issues)) - for _, i := range issues { - res = append(res, result.Issue{ - Pos: i.Pos, - Text: fmt.Sprintf("%s is unused", formatCode(i.UnusedIdentName, lintCtx.Cfg)), - FromLinter: d.Name(), - }) +func NewDeadcode() *goanalysis.Linter { + const linterName = "deadcode" + var mu sync.Mutex + var resIssues []result.Issue + + analyzer := &analysis.Analyzer{ + Name: goanalysis.TheOnlyAnalyzerName, + Doc: goanalysis.TheOnlyanalyzerDoc, + Run: func(pass *analysis.Pass) (interface{}, error) { + prog := goanalysis.MakeFakeLoaderProgram(pass) + issues, err := deadcodeAPI.Run(prog) + if err != nil { + return nil, err + } + res := make([]result.Issue, 0, len(issues)) + for _, i := range issues { + res = append(res, result.Issue{ + Pos: i.Pos, + Text: fmt.Sprintf("%s is unused", formatCode(i.UnusedIdentName, nil)), + FromLinter: linterName, + }) + } + mu.Lock() + resIssues = append(resIssues, res...) + mu.Unlock() + + return nil, nil + }, } - return res, nil + return goanalysis.NewLinter( + linterName, + "Finds unused code", + []*analysis.Analyzer{analyzer}, + nil, + ).WithIssuesReporter(func(*linter.Context) []result.Issue { + return resIssues + }).WithLoadMode(goanalysis.LoadModeTypesInfo) } diff --git a/pkg/golinters/depguard.go b/pkg/golinters/depguard.go index 9fb59dfa31d1..4a09cd05ee6e 100644 --- a/pkg/golinters/depguard.go +++ b/pkg/golinters/depguard.go @@ -1,9 +1,14 @@ package golinters import ( - "context" "fmt" "strings" + "sync" + + "golang.org/x/tools/go/analysis" + "golang.org/x/tools/go/loader" + + "github.com/golangci/golangci-lint/pkg/golinters/goanalysis" depguardAPI "github.com/OpenPeeDeeP/depguard" @@ -11,12 +16,6 @@ import ( "github.com/golangci/golangci-lint/pkg/result" ) -type Depguard struct{} - -func (Depguard) Name() string { - return "depguard" -} - func setDepguardListType(dg *depguardAPI.Depguard, lintCtx *linter.Context) error { listType := lintCtx.Settings().Depguard.ListType var found bool @@ -49,42 +48,67 @@ func setupDepguardPackages(dg *depguardAPI.Depguard, lintCtx *linter.Context) { } } -func (Depguard) Desc() string { - return "Go linter that checks if package imports are in a list of acceptable packages" -} +func NewDepguard() *goanalysis.Linter { + const linterName = "depguard" + var mu sync.Mutex + var resIssues []result.Issue -func (d Depguard) Run(ctx context.Context, lintCtx *linter.Context) ([]result.Issue, error) { - dg := &depguardAPI.Depguard{ - Packages: lintCtx.Settings().Depguard.Packages, - IncludeGoRoot: lintCtx.Settings().Depguard.IncludeGoRoot, - } - if err := setDepguardListType(dg, lintCtx); err != nil { - return nil, err + analyzer := &analysis.Analyzer{ + Name: goanalysis.TheOnlyAnalyzerName, + Doc: goanalysis.TheOnlyanalyzerDoc, } - setupDepguardPackages(dg, lintCtx) + return goanalysis.NewLinter( + linterName, + "Go linter that checks if package imports are in a list of acceptable packages", + []*analysis.Analyzer{analyzer}, + nil, + ).WithContextSetter(func(lintCtx *linter.Context) { + dgSettings := &lintCtx.Settings().Depguard + analyzer.Run = func(pass *analysis.Pass) (interface{}, error) { + prog := goanalysis.MakeFakeLoaderProgram(pass) + dg := &depguardAPI.Depguard{ + Packages: dgSettings.Packages, + IncludeGoRoot: dgSettings.IncludeGoRoot, + } + if err := setDepguardListType(dg, lintCtx); err != nil { + return nil, err + } + setupDepguardPackages(dg, lintCtx) - issues, err := dg.Run(lintCtx.LoaderConfig, lintCtx.Program) - if err != nil { - return nil, err - } - if len(issues) == 0 { - return nil, nil - } - msgSuffix := "is in the blacklist" - if dg.ListType == depguardAPI.LTWhitelist { - msgSuffix = "is not in the whitelist" - } - res := make([]result.Issue, 0, len(issues)) - for _, i := range issues { - userSuppliedMsgSuffix := lintCtx.Settings().Depguard.PackagesWithErrorMessage[i.PackageName] - if userSuppliedMsgSuffix != "" { - userSuppliedMsgSuffix = ": " + userSuppliedMsgSuffix + loadConfig := &loader.Config{ + Cwd: "", // fallbacked to os.Getcwd + Build: nil, // fallbacked to build.Default + } + issues, err := dg.Run(loadConfig, prog) + if err != nil { + return nil, err + } + if len(issues) == 0 { + return nil, nil + } + msgSuffix := "is in the blacklist" + if dg.ListType == depguardAPI.LTWhitelist { + msgSuffix = "is not in the whitelist" + } + res := make([]result.Issue, 0, len(issues)) + for _, i := range issues { + userSuppliedMsgSuffix := dgSettings.PackagesWithErrorMessage[i.PackageName] + if userSuppliedMsgSuffix != "" { + userSuppliedMsgSuffix = ": " + userSuppliedMsgSuffix + } + res = append(res, result.Issue{ + Pos: i.Position, + Text: fmt.Sprintf("%s %s%s", formatCode(i.PackageName, lintCtx.Cfg), msgSuffix, userSuppliedMsgSuffix), + FromLinter: linterName, + }) + } + mu.Lock() + resIssues = append(resIssues, res...) + mu.Unlock() + + return nil, nil } - res = append(res, result.Issue{ - Pos: i.Position, - Text: fmt.Sprintf("%s %s%s", formatCode(i.PackageName, lintCtx.Cfg), msgSuffix, userSuppliedMsgSuffix), - FromLinter: d.Name(), - }) - } - return res, nil + }).WithIssuesReporter(func(*linter.Context) []result.Issue { + return resIssues + }).WithLoadMode(goanalysis.LoadModeTypesInfo) } diff --git a/pkg/golinters/dogsled.go b/pkg/golinters/dogsled.go index e98d1463d17f..4cddf40064f1 100644 --- a/pkg/golinters/dogsled.go +++ b/pkg/golinters/dogsled.go @@ -1,37 +1,54 @@ package golinters import ( - "context" "fmt" "go/ast" "go/token" + "sync" + "golang.org/x/tools/go/analysis" + + "github.com/golangci/golangci-lint/pkg/golinters/goanalysis" "github.com/golangci/golangci-lint/pkg/lint/linter" "github.com/golangci/golangci-lint/pkg/result" ) -type Dogsled struct{} - -func (Dogsled) Name() string { - return "dogsled" -} +const dogsledLinterName = "dogsled" -func (Dogsled) Desc() string { - return "Checks assignments with too many blank identifiers (e.g. x, _, _, _, := f())" -} +func NewDogsled() *goanalysis.Linter { + var mu sync.Mutex + var resIssues []result.Issue -func (d Dogsled) Run(ctx context.Context, lintCtx *linter.Context) ([]result.Issue, error) { - var res []result.Issue - for _, f := range lintCtx.ASTCache.GetAllValidFiles() { - v := returnsVisitor{ - maxBlanks: lintCtx.Settings().Dogsled.MaxBlankIdentifiers, - f: f.Fset, - } - ast.Walk(&v, f.F) - res = append(res, v.issues...) + analyzer := &analysis.Analyzer{ + Name: goanalysis.TheOnlyAnalyzerName, + Doc: goanalysis.TheOnlyanalyzerDoc, } + return goanalysis.NewLinter( + dogsledLinterName, + "Checks assignments with too many blank identifiers (e.g. x, _, _, _, := f())", + []*analysis.Analyzer{analyzer}, + nil, + ).WithContextSetter(func(lintCtx *linter.Context) { + analyzer.Run = func(pass *analysis.Pass) (interface{}, error) { + var pkgIssues []result.Issue + for _, f := range pass.Files { + v := returnsVisitor{ + maxBlanks: lintCtx.Settings().Dogsled.MaxBlankIdentifiers, + f: pass.Fset, + } + ast.Walk(&v, f) + pkgIssues = append(pkgIssues, v.issues...) + } - return res, nil + mu.Lock() + resIssues = append(resIssues, pkgIssues...) + mu.Unlock() + + return nil, nil + } + }).WithIssuesReporter(func(*linter.Context) []result.Issue { + return resIssues + }).WithLoadMode(goanalysis.LoadModeSyntax) } type returnsVisitor struct { @@ -68,7 +85,7 @@ func (v *returnsVisitor) Visit(node ast.Node) ast.Visitor { if numBlank > v.maxBlanks { v.issues = append(v.issues, result.Issue{ - FromLinter: Dogsled{}.Name(), + FromLinter: dogsledLinterName, Text: fmt.Sprintf("declaration has %v blank identifiers", numBlank), Pos: v.f.Position(assgnStmt.Pos()), }) diff --git a/pkg/golinters/dupl.go b/pkg/golinters/dupl.go index c31c87795ce0..deb149006801 100644 --- a/pkg/golinters/dupl.go +++ b/pkg/golinters/dupl.go @@ -1,60 +1,83 @@ package golinters import ( - "context" "fmt" "go/token" + "sync" duplAPI "github.com/golangci/dupl" "github.com/pkg/errors" + "golang.org/x/tools/go/analysis" "github.com/golangci/golangci-lint/pkg/fsutils" + "github.com/golangci/golangci-lint/pkg/golinters/goanalysis" "github.com/golangci/golangci-lint/pkg/lint/linter" "github.com/golangci/golangci-lint/pkg/result" ) -type Dupl struct{} +const duplLinterName = "dupl" -func (Dupl) Name() string { - return "dupl" -} - -func (Dupl) Desc() string { - return "Tool for code clone detection" -} +func NewDupl() *goanalysis.Linter { + var mu sync.Mutex + var resIssues []result.Issue -func (d Dupl) Run(ctx context.Context, lintCtx *linter.Context) ([]result.Issue, error) { - issues, err := duplAPI.Run(getAllFileNames(lintCtx), lintCtx.Settings().Dupl.Threshold) - if err != nil { - return nil, err + analyzer := &analysis.Analyzer{ + Name: goanalysis.TheOnlyAnalyzerName, + Doc: goanalysis.TheOnlyanalyzerDoc, } + return goanalysis.NewLinter( + duplLinterName, + "Tool for code clone detection", + []*analysis.Analyzer{analyzer}, + nil, + ).WithContextSetter(func(lintCtx *linter.Context) { + analyzer.Run = func(pass *analysis.Pass) (interface{}, error) { + var fileNames []string + for _, f := range pass.Files { + pos := pass.Fset.Position(f.Pos()) + fileNames = append(fileNames, pos.Filename) + } - if len(issues) == 0 { - return nil, nil - } + issues, err := duplAPI.Run(fileNames, lintCtx.Settings().Dupl.Threshold) + if err != nil { + return nil, err + } - res := make([]result.Issue, 0, len(issues)) - for _, i := range issues { - toFilename, err := fsutils.ShortestRelPath(i.To.Filename(), "") - if err != nil { - return nil, errors.Wrapf(err, "failed to get shortest rel path for %q", i.To.Filename()) + if len(issues) == 0 { + return nil, nil + } + + res := make([]result.Issue, 0, len(issues)) + for _, i := range issues { + toFilename, err := fsutils.ShortestRelPath(i.To.Filename(), "") + if err != nil { + return nil, errors.Wrapf(err, "failed to get shortest rel path for %q", i.To.Filename()) + } + dupl := fmt.Sprintf("%s:%d-%d", toFilename, i.To.LineStart(), i.To.LineEnd()) + text := fmt.Sprintf("%d-%d lines are duplicate of %s", + i.From.LineStart(), i.From.LineEnd(), + formatCode(dupl, lintCtx.Cfg)) + res = append(res, result.Issue{ + Pos: token.Position{ + Filename: i.From.Filename(), + Line: i.From.LineStart(), + }, + LineRange: &result.Range{ + From: i.From.LineStart(), + To: i.From.LineEnd(), + }, + Text: text, + FromLinter: duplLinterName, + }) + } + + mu.Lock() + resIssues = append(resIssues, res...) + mu.Unlock() + + return nil, nil } - dupl := fmt.Sprintf("%s:%d-%d", toFilename, i.To.LineStart(), i.To.LineEnd()) - text := fmt.Sprintf("%d-%d lines are duplicate of %s", - i.From.LineStart(), i.From.LineEnd(), - formatCode(dupl, lintCtx.Cfg)) - res = append(res, result.Issue{ - Pos: token.Position{ - Filename: i.From.Filename(), - Line: i.From.LineStart(), - }, - LineRange: &result.Range{ - From: i.From.LineStart(), - To: i.From.LineEnd(), - }, - Text: text, - FromLinter: d.Name(), - }) - } - return res, nil + }).WithIssuesReporter(func(*linter.Context) []result.Issue { + return resIssues + }).WithLoadMode(goanalysis.LoadModeSyntax) } diff --git a/pkg/golinters/errcheck.go b/pkg/golinters/errcheck.go index bd1d7c3ef73d..03f572eabe60 100644 --- a/pkg/golinters/errcheck.go +++ b/pkg/golinters/errcheck.go @@ -2,13 +2,17 @@ package golinters import ( "bufio" - "context" "fmt" "os" "os/user" "path/filepath" "regexp" "strings" + "sync" + + "golang.org/x/tools/go/analysis" + + "github.com/golangci/golangci-lint/pkg/golinters/goanalysis" errcheckAPI "github.com/golangci/errcheck/golangci" "github.com/pkg/errors" @@ -19,47 +23,58 @@ import ( "github.com/golangci/golangci-lint/pkg/result" ) -type Errcheck struct{} - -func (Errcheck) Name() string { - return "errcheck" -} - -func (Errcheck) Desc() string { - return "Errcheck is a program for checking for unchecked errors " + - "in go programs. These unchecked errors can be critical bugs in some cases" -} - -func (e Errcheck) Run(ctx context.Context, lintCtx *linter.Context) ([]result.Issue, error) { - errCfg, err := genConfig(&lintCtx.Settings().Errcheck) - if err != nil { - return nil, err - } - issues, err := errcheckAPI.RunWithConfig(lintCtx.Program, errCfg) - if err != nil { - return nil, err - } - - if len(issues) == 0 { - return nil, nil - } - - res := make([]result.Issue, 0, len(issues)) - for _, i := range issues { - var text string - if i.FuncName != "" { - text = fmt.Sprintf("Error return value of %s is not checked", formatCode(i.FuncName, lintCtx.Cfg)) - } else { - text = "Error return value is not checked" +func NewErrcheck() *goanalysis.Linter { + const linterName = "errcheck" + var mu sync.Mutex + var res []result.Issue + analyzer := &analysis.Analyzer{ + Name: goanalysis.TheOnlyAnalyzerName, + Doc: goanalysis.TheOnlyanalyzerDoc, + } + return goanalysis.NewLinter( + linterName, + "Errcheck is a program for checking for unchecked errors "+ + "in go programs. These unchecked errors can be critical bugs in some cases", + []*analysis.Analyzer{analyzer}, + nil, + ).WithContextSetter(func(lintCtx *linter.Context) { + analyzer.Run = func(pass *analysis.Pass) (interface{}, error) { + prog := goanalysis.MakeFakeLoaderProgram(pass) + errCfg, err := genConfig(&lintCtx.Settings().Errcheck) + if err != nil { + return nil, err + } + errcheckIssues, err := errcheckAPI.RunWithConfig(prog, errCfg) + if err != nil { + return nil, err + } + + if len(errcheckIssues) == 0 { + return nil, nil + } + + issues := make([]result.Issue, 0, len(errcheckIssues)) + for _, i := range errcheckIssues { + var text string + if i.FuncName != "" { + text = fmt.Sprintf("Error return value of %s is not checked", formatCode(i.FuncName, lintCtx.Cfg)) + } else { + text = "Error return value is not checked" + } + issues = append(issues, result.Issue{ + FromLinter: linterName, + Text: text, + Pos: i.Pos, + }) + } + mu.Lock() + res = append(res, issues...) + mu.Unlock() + return nil, nil } - res = append(res, result.Issue{ - FromLinter: e.Name(), - Text: text, - Pos: i.Pos, - }) - } - - return res, nil + }).WithIssuesReporter(func(*linter.Context) []result.Issue { + return res + }).WithLoadMode(goanalysis.LoadModeTypesInfo) } // parseIgnoreConfig was taken from errcheck in order to keep the API identical. diff --git a/pkg/golinters/funlen.go b/pkg/golinters/funlen.go index c5768ec36b74..ee9782dbab85 100644 --- a/pkg/golinters/funlen.go +++ b/pkg/golinters/funlen.go @@ -1,47 +1,65 @@ package golinters import ( - "context" "go/token" "strings" + "sync" + "golang.org/x/tools/go/analysis" + + "github.com/golangci/golangci-lint/pkg/golinters/goanalysis" "github.com/golangci/golangci-lint/pkg/lint/linter" "github.com/golangci/golangci-lint/pkg/result" "github.com/ultraware/funlen" ) -type Funlen struct{} +const funlenLinterName = "funlen" -func (Funlen) Name() string { - return "funlen" -} +func NewFunlen() *goanalysis.Linter { + var mu sync.Mutex + var resIssues []result.Issue -func (Funlen) Desc() string { - return "Tool for detection of long functions" -} - -func (f Funlen) Run(ctx context.Context, lintCtx *linter.Context) ([]result.Issue, error) { - var issues []funlen.Message - for _, file := range lintCtx.ASTCache.GetAllValidFiles() { - issues = append(issues, funlen.Run(file.F, file.Fset, lintCtx.Settings().Funlen.Lines, lintCtx.Settings().Funlen.Statements)...) + analyzer := &analysis.Analyzer{ + Name: goanalysis.TheOnlyAnalyzerName, + Doc: goanalysis.TheOnlyanalyzerDoc, } + return goanalysis.NewLinter( + funlenLinterName, + "Tool for detection of long functions", + []*analysis.Analyzer{analyzer}, + nil, + ).WithContextSetter(func(lintCtx *linter.Context) { + analyzer.Run = func(pass *analysis.Pass) (interface{}, error) { + var issues []funlen.Message + for _, file := range pass.Files { + fileIssues := funlen.Run(file, pass.Fset, lintCtx.Settings().Funlen.Lines, lintCtx.Settings().Funlen.Statements) + issues = append(issues, fileIssues...) + } - if len(issues) == 0 { - return nil, nil - } + if len(issues) == 0 { + return nil, nil + } - res := make([]result.Issue, len(issues)) - for k, i := range issues { - res[k] = result.Issue{ - Pos: token.Position{ - Filename: i.Pos.Filename, - Line: i.Pos.Line, - }, - Text: strings.TrimRight(i.Message, "\n"), - FromLinter: f.Name(), - } - } + res := make([]result.Issue, len(issues)) + for k, i := range issues { + res[k] = result.Issue{ + Pos: token.Position{ + Filename: i.Pos.Filename, + Line: i.Pos.Line, + }, + Text: strings.TrimRight(i.Message, "\n"), + FromLinter: funlenLinterName, + } + } - return res, nil + mu.Lock() + resIssues = append(resIssues, res...) + mu.Unlock() + + return nil, nil + } + }).WithIssuesReporter(func(*linter.Context) []result.Issue { + return resIssues + }).WithLoadMode(goanalysis.LoadModeSyntax) } diff --git a/pkg/golinters/goanalysis/adapters.go b/pkg/golinters/goanalysis/adapters.go new file mode 100644 index 000000000000..830c4d8822da --- /dev/null +++ b/pkg/golinters/goanalysis/adapters.go @@ -0,0 +1,36 @@ +package goanalysis + +import ( + "go/types" + + "golang.org/x/tools/go/analysis" + "golang.org/x/tools/go/loader" +) + +func MakeFakeLoaderProgram(pass *analysis.Pass) *loader.Program { + prog := &loader.Program{ + Fset: pass.Fset, + Created: []*loader.PackageInfo{ + { + Pkg: pass.Pkg, + Importable: true, // not used + TransitivelyErrorFree: true, // TODO + + Files: pass.Files, + Errors: nil, + Info: *pass.TypesInfo, + }, + }, + AllPackages: map[*types.Package]*loader.PackageInfo{ + pass.Pkg: { + Pkg: pass.Pkg, + Importable: true, + TransitivelyErrorFree: true, + Files: pass.Files, + Errors: nil, + Info: *pass.TypesInfo, + }, + }, + } + return prog +} diff --git a/pkg/golinters/goanalysis/interface.go b/pkg/golinters/goanalysis/interface.go deleted file mode 100644 index afe6219d72d1..000000000000 --- a/pkg/golinters/goanalysis/interface.go +++ /dev/null @@ -1,11 +0,0 @@ -package goanalysis - -import ( - "golang.org/x/tools/go/analysis" -) - -type SupportedLinter interface { - Analyzers() []*analysis.Analyzer - Cfg() map[string]map[string]interface{} - AnalyzerToLinterNameMapping() map[*analysis.Analyzer]string -} diff --git a/pkg/golinters/goanalysis/linter.go b/pkg/golinters/goanalysis/linter.go index 185fc51cfe3f..ed17b67fdd97 100644 --- a/pkg/golinters/goanalysis/linter.go +++ b/pkg/golinters/goanalysis/linter.go @@ -6,32 +6,95 @@ import ( "fmt" "strings" + "golang.org/x/tools/go/packages" + "github.com/pkg/errors" "golang.org/x/tools/go/analysis" "github.com/golangci/golangci-lint/pkg/lint/linter" + libpackages "github.com/golangci/golangci-lint/pkg/packages" "github.com/golangci/golangci-lint/pkg/result" ) +const ( + TheOnlyAnalyzerName = "the_only_name" + TheOnlyanalyzerDoc = "the_only_doc" +) + +type LoadMode int + +const ( + LoadModeNone LoadMode = iota + LoadModeSyntax + LoadModeTypesInfo + LoadModeWholeProgram +) + +func (loadMode LoadMode) String() string { + switch loadMode { + case LoadModeNone: + return "none" + case LoadModeSyntax: + return "syntax" + case LoadModeTypesInfo: + return "types info" + case LoadModeWholeProgram: + return "whole program" + } + panic(fmt.Sprintf("unknown load mode %d", loadMode)) +} + type Linter struct { - name, desc string - analyzers []*analysis.Analyzer - cfg map[string]map[string]interface{} + name, desc string + analyzers []*analysis.Analyzer + cfg map[string]map[string]interface{} + issuesReporter func(*linter.Context) []result.Issue + contextSetter func(*linter.Context) + loadMode LoadMode + useOriginalPackages bool + isTypecheckMode bool } func NewLinter(name, desc string, analyzers []*analysis.Analyzer, cfg map[string]map[string]interface{}) *Linter { return &Linter{name: name, desc: desc, analyzers: analyzers, cfg: cfg} } -func (lnt Linter) Name() string { +func (lnt *Linter) UseOriginalPackages() { + lnt.useOriginalPackages = true +} + +func (lnt *Linter) SetTypecheckMode() { + lnt.isTypecheckMode = true +} + +func (lnt *Linter) LoadMode() LoadMode { + return lnt.loadMode +} + +func (lnt *Linter) WithLoadMode(loadMode LoadMode) *Linter { + lnt.loadMode = loadMode + return lnt +} + +func (lnt *Linter) WithIssuesReporter(r func(*linter.Context) []result.Issue) *Linter { + lnt.issuesReporter = r + return lnt +} + +func (lnt *Linter) WithContextSetter(cs func(*linter.Context)) *Linter { + lnt.contextSetter = cs + return lnt +} + +func (lnt *Linter) Name() string { return lnt.name } -func (lnt Linter) Desc() string { +func (lnt *Linter) Desc() string { return lnt.desc } -func (lnt Linter) allAnalyzerNames() []string { +func (lnt *Linter) allAnalyzerNames() []string { var ret []string for _, a := range lnt.analyzers { ret = append(ret, a.Name) @@ -63,7 +126,7 @@ func valueToString(v interface{}) string { return fmt.Sprint(v) } -func (lnt Linter) configureAnalyzer(a *analysis.Analyzer, cfg map[string]interface{}) error { +func (lnt *Linter) configureAnalyzer(a *analysis.Analyzer, cfg map[string]interface{}) error { for k, v := range cfg { f := a.Flags.Lookup(k) if f == nil { @@ -84,7 +147,7 @@ func (lnt Linter) configureAnalyzer(a *analysis.Analyzer, cfg map[string]interfa return nil } -func (lnt Linter) configure() error { +func (lnt *Linter) configure() error { analyzersMap := map[string]*analysis.Analyzer{} for _, a := range lnt.analyzers { analyzersMap[a.Name] = a @@ -105,7 +168,63 @@ func (lnt Linter) configure() error { return nil } -func (lnt Linter) Run(ctx context.Context, lintCtx *linter.Context) ([]result.Issue, error) { +func parseError(srcErr packages.Error) (*result.Issue, error) { + pos, err := libpackages.ParseErrorPosition(srcErr.Pos) + if err != nil { + return nil, err + } + + return &result.Issue{ + Pos: *pos, + Text: srcErr.Msg, + FromLinter: "typecheck", + }, nil +} + +func buildIssuesFromErrorsForTypecheckMode(errs []error, lintCtx *linter.Context) ([]result.Issue, error) { + var issues []result.Issue + uniqReportedIssues := map[string]bool{} + for _, err := range errs { + itErr, ok := errors.Cause(err).(*IllTypedError) + if !ok { + return nil, err + } + for _, err := range libpackages.ExtractErrors(itErr.Pkg, lintCtx.ASTCache) { + i, perr := parseError(err) + if perr != nil { // failed to parse + if uniqReportedIssues[err.Msg] { + continue + } + uniqReportedIssues[err.Msg] = true + lintCtx.Log.Errorf("typechecking error: %s", err.Msg) + } else { + issues = append(issues, *i) + } + } + } + return issues, nil +} + +func buildIssues(diags []Diagnostic, linterNameBuilder func(diag *Diagnostic) string) []result.Issue { + var issues []result.Issue + for i := range diags { + diag := &diags[i] + var text string + if diag.Analyzer.Name == TheOnlyAnalyzerName { + text = diag.Message + } else { + text = fmt.Sprintf("%s: %s", diag.Analyzer.Name, diag.Message) + } + issues = append(issues, result.Issue{ + FromLinter: linterNameBuilder(diag), + Text: text, + Pos: diag.Position, + }) + } + return issues +} + +func (lnt *Linter) Run(ctx context.Context, lintCtx *linter.Context) ([]result.Issue, error) { if err := analysis.Validate(lnt.analyzers); err != nil { return nil, errors.Wrap(err, "failed to validate analyzers") } @@ -114,39 +233,37 @@ func (lnt Linter) Run(ctx context.Context, lintCtx *linter.Context) ([]result.Is return nil, errors.Wrap(err, "failed to configure analyzers") } - runner := newRunner(lnt.name, lintCtx.Log.Child("goanalysis"), lintCtx.PkgCache, lintCtx.LoadGuard, lintCtx.NeedWholeProgram) + if lnt.contextSetter != nil { + lnt.contextSetter(lintCtx) + } + + loadMode := lnt.loadMode + runner := newRunner(lnt.name, lintCtx.Log.Child("goanalysis"), + lintCtx.PkgCache, lintCtx.LoadGuard, loadMode) + + pkgs := lintCtx.Packages + if lnt.useOriginalPackages { + pkgs = lintCtx.OriginalPackages + } + + diags, errs := runner.run(lnt.analyzers, pkgs) + + linterNameBuilder := func(*Diagnostic) string { return lnt.Name() } + if lnt.isTypecheckMode { + return buildIssuesFromErrorsForTypecheckMode(errs, lintCtx) + } - diags, errs := runner.run(lnt.analyzers, lintCtx.Packages) // Don't print all errs: they can duplicate. if len(errs) != 0 { return nil, errs[0] } var issues []result.Issue - for i := range diags { - diag := &diags[i] - issues = append(issues, result.Issue{ - FromLinter: lnt.Name(), - Text: fmt.Sprintf("%s: %s", diag.Analyzer.Name, diag.Message), - Pos: diag.Position, - }) + if lnt.issuesReporter != nil { + issues = append(issues, lnt.issuesReporter(lintCtx)...) + } else { + issues = buildIssues(diags, linterNameBuilder) } return issues, nil } - -func (lnt Linter) Analyzers() []*analysis.Analyzer { - return lnt.analyzers -} - -func (lnt Linter) Cfg() map[string]map[string]interface{} { - return lnt.cfg -} - -func (lnt Linter) AnalyzerToLinterNameMapping() map[*analysis.Analyzer]string { - ret := map[*analysis.Analyzer]string{} - for _, a := range lnt.analyzers { - ret[a] = lnt.Name() - } - return ret -} diff --git a/pkg/golinters/typecheck_test.go b/pkg/golinters/goanalysis/linter_test.go similarity index 90% rename from pkg/golinters/typecheck_test.go rename to pkg/golinters/goanalysis/linter_test.go index 032180f3d97b..44ded60043c0 100644 --- a/pkg/golinters/typecheck_test.go +++ b/pkg/golinters/goanalysis/linter_test.go @@ -1,4 +1,4 @@ -package golinters +package goanalysis import ( "fmt" @@ -19,9 +19,8 @@ func TestParseError(t *testing.T) { {"f.go: 1", "", false}, } - lint := TypeCheck{} for _, c := range cases { - i, _ := lint.parseError(packages.Error{ + i, _ := parseError(packages.Error{ Pos: c.in, Msg: "msg", }) diff --git a/pkg/golinters/goanalysis/metalinter.go b/pkg/golinters/goanalysis/metalinter.go index 851303fdedc7..96ea36a4b20e 100644 --- a/pkg/golinters/goanalysis/metalinter.go +++ b/pkg/golinters/goanalysis/metalinter.go @@ -2,7 +2,6 @@ package goanalysis import ( "context" - "fmt" "github.com/pkg/errors" "golang.org/x/tools/go/analysis" @@ -12,12 +11,11 @@ import ( ) type MetaLinter struct { - linters []*Linter - analyzerToLinterName map[*analysis.Analyzer]string + linters []*Linter } -func NewMetaLinter(linters []*Linter, analyzerToLinterName map[*analysis.Analyzer]string) *MetaLinter { - return &MetaLinter{linters: linters, analyzerToLinterName: analyzerToLinterName} +func NewMetaLinter(linters []*Linter) *MetaLinter { + return &MetaLinter{linters: linters} } func (ml MetaLinter) Name() string { @@ -28,41 +26,110 @@ func (ml MetaLinter) Desc() string { return "" } -func (ml MetaLinter) Run(ctx context.Context, lintCtx *linter.Context) ([]result.Issue, error) { +func (ml MetaLinter) isTypecheckMode() bool { for _, linter := range ml.linters { - if err := analysis.Validate(linter.analyzers); err != nil { - return nil, errors.Wrapf(err, "failed to validate analyzers of %s", linter.Name()) + if linter.isTypecheckMode { + return true } } + return false +} +func (ml MetaLinter) getLoadMode() LoadMode { + loadMode := LoadModeNone for _, linter := range ml.linters { - if err := linter.configure(); err != nil { - return nil, errors.Wrapf(err, "failed to configure analyzers of %s", linter.Name()) + if linter.loadMode > loadMode { + loadMode = linter.loadMode } } + return loadMode +} +func (ml MetaLinter) runContextSetters(lintCtx *linter.Context) { + for _, linter := range ml.linters { + if linter.contextSetter != nil { + linter.contextSetter(lintCtx) + } + } +} + +func (ml MetaLinter) getAllAnalyzers() []*analysis.Analyzer { var allAnalyzers []*analysis.Analyzer for _, linter := range ml.linters { allAnalyzers = append(allAnalyzers, linter.analyzers...) } + return allAnalyzers +} + +func (ml MetaLinter) getAnalyzerToLinterNameMapping() map[*analysis.Analyzer]string { + analyzerToLinterName := map[*analysis.Analyzer]string{} + for _, linter := range ml.linters { + for _, a := range linter.analyzers { + analyzerToLinterName[a] = linter.Name() + } + } + return analyzerToLinterName +} + +func (ml MetaLinter) configure() error { + for _, linter := range ml.linters { + if err := linter.configure(); err != nil { + return errors.Wrapf(err, "failed to configure analyzers of %s", linter.Name()) + } + } + return nil +} + +func (ml MetaLinter) validate() error { + for _, linter := range ml.linters { + if err := analysis.Validate(linter.analyzers); err != nil { + return errors.Wrapf(err, "failed to validate analyzers of %s", linter.Name()) + } + } + return nil +} + +func (ml MetaLinter) Run(ctx context.Context, lintCtx *linter.Context) ([]result.Issue, error) { + if err := ml.validate(); err != nil { + return nil, err + } + + if err := ml.configure(); err != nil { + return nil, err + } + ml.runContextSetters(lintCtx) - runner := newRunner("metalinter", lintCtx.Log.Child("goanalysis"), lintCtx.PkgCache, lintCtx.LoadGuard, lintCtx.NeedWholeProgram) + analyzerToLinterName := ml.getAnalyzerToLinterNameMapping() + + runner := newRunner("metalinter", lintCtx.Log.Child("goanalysis"), + lintCtx.PkgCache, lintCtx.LoadGuard, ml.getLoadMode()) + + diags, errs := runner.run(ml.getAllAnalyzers(), lintCtx.Packages) + + buildAllIssues := func() []result.Issue { + linterNameBuilder := func(diag *Diagnostic) string { return analyzerToLinterName[diag.Analyzer] } + issues := buildIssues(diags, linterNameBuilder) + + for _, linter := range ml.linters { + if linter.issuesReporter != nil { + issues = append(issues, linter.issuesReporter(lintCtx)...) + } + } + return issues + } + + if ml.isTypecheckMode() { + issues, err := buildIssuesFromErrorsForTypecheckMode(errs, lintCtx) + if err != nil { + return nil, err + } + return append(issues, buildAllIssues()...), nil + } - diags, errs := runner.run(allAnalyzers, lintCtx.Packages) // Don't print all errs: they can duplicate. if len(errs) != 0 { return nil, errs[0] } - var issues []result.Issue - for i := range diags { - diag := &diags[i] - issues = append(issues, result.Issue{ - FromLinter: ml.analyzerToLinterName[diag.Analyzer], - Text: fmt.Sprintf("%s: %s", diag.Analyzer, diag.Message), - Pos: diag.Position, - }) - } - - return issues, nil + return buildAllIssues(), nil } diff --git a/pkg/golinters/goanalysis/runner.go b/pkg/golinters/goanalysis/runner.go index 4dee44c86c9a..df1d9433c9be 100644 --- a/pkg/golinters/goanalysis/runner.go +++ b/pkg/golinters/goanalysis/runner.go @@ -54,9 +54,7 @@ var ( // v show [v]erbose logging // - debugf = logutils.Debug("goanalysis") - isDebug = logutils.HaveDebugTag("goanalysis") - + debugf = logutils.Debug("goanalysis") factsDebugf = logutils.Debug("goanalysis/facts") factsInheritDebugf = logutils.Debug("goanalysis/facts/inherit") factsExportDebugf = logutils.Debug("goanalysis/facts") @@ -78,20 +76,20 @@ type Diagnostic struct { } type runner struct { - log logutils.Log - prefix string // ensure unique analyzer names - pkgCache *pkgcache.Cache - loadGuard *load.Guard - needWholeProgram bool + log logutils.Log + prefix string // ensure unique analyzer names + pkgCache *pkgcache.Cache + loadGuard *load.Guard + loadMode LoadMode } -func newRunner(prefix string, logger logutils.Log, pkgCache *pkgcache.Cache, loadGuard *load.Guard, needWholeProgram bool) *runner { +func newRunner(prefix string, logger logutils.Log, pkgCache *pkgcache.Cache, loadGuard *load.Guard, loadMode LoadMode) *runner { return &runner{ - prefix: prefix, - log: logger, - pkgCache: pkgCache, - loadGuard: loadGuard, - needWholeProgram: needWholeProgram, + prefix: prefix, + log: logger, + pkgCache: pkgCache, + loadGuard: loadGuard, + loadMode: loadMode, } } @@ -103,6 +101,7 @@ func newRunner(prefix string, logger logutils.Log, pkgCache *pkgcache.Cache, loa // It returns the appropriate exit code. //nolint:gocyclo func (r *runner) run(analyzers []*analysis.Analyzer, initialPackages []*packages.Package) ([]Diagnostic, []error) { + debugf("Analyzing %d packages on load mode %s", len(initialPackages), r.loadMode) defer r.pkgCache.Trim() roots := r.analyze(initialPackages, analyzers) @@ -144,7 +143,7 @@ func (r *runner) prepareAnalysis(pkgs []*packages.Package, analysisDoneCh: make(chan struct{}), objectFacts: make(map[objectFactKey]analysis.Fact), packageFacts: make(map[packageFactKey]analysis.Fact), - needWholeProgram: r.needWholeProgram, + loadMode: r.loadMode, } // Add a dependency on each required analyzers. @@ -243,7 +242,7 @@ func (r *runner) analyze(pkgs []*packages.Package, analyzers []*analysis.Analyze if lp.isInitial { wg.Add(1) go func(lp *loadingPackage) { - lp.analyzeRecursive(r.needWholeProgram, loadSem) + lp.analyzeRecursive(r.loadMode, loadSem) wg.Done() }(lp) } @@ -344,14 +343,13 @@ type action struct { result interface{} diagnostics []analysis.Diagnostic err error - duration time.Duration log logutils.Log prefix string pkgCache *pkgcache.Cache analysisDoneCh chan struct{} loadCachedFactsDone bool loadCachedFactsOk bool - needWholeProgram bool + loadMode LoadMode } type objectFactKey struct { @@ -397,6 +395,14 @@ func (act *action) waitUntilDependingAnalyzersWorked() { } } +type IllTypedError struct { + Pkg *packages.Package +} + +func (e *IllTypedError) Error() string { + return fmt.Sprintf("errors in package: %v", e.Pkg.Errors) +} + func (act *action) analyzeSafe() { defer func() { if p := recover(); p != nil { @@ -427,10 +433,7 @@ func (act *action) analyze() { // time is 5x higher than in sequential mode, even with a // semaphore limiting the number of threads here. // So use -debug=tp. - if isDebug { - t0 := time.Now() - defer func() { act.duration = time.Since(t0) }() - } + defer func(now time.Time) { analyzeDebugf("go/analysis: %s: %s: analyzed package %q in %s", act.prefix, act.a.Name, act.pkg.Name, time.Since(now)) }(time.Now()) @@ -488,8 +491,11 @@ func (act *action) analyze() { act.pass = pass var err error - if act.pkg.IllTyped && !pass.Analyzer.RunDespiteErrors { - err = fmt.Errorf("analysis skipped due to errors in package") + if act.pkg.IllTyped { + // It looks like there should be !pass.Analyzer.RunDespiteErrors + // but govet's cgocall crashes on it. Govet itself contains !pass.Analyzer.RunDespiteErrors condition here + // but it exit before it if packages.Load have failed. + err = errors.Wrap(&IllTypedError{Pkg: act.pkg}, "analysis skipped") } else { startedAt = time.Now() act.result, err = pass.Analyzer.Run(pass) @@ -910,36 +916,36 @@ func (lp *loadingPackage) decUse() { lp.actions = nil } -func (lp *loadingPackage) analyzeRecursive(needWholeProgram bool, loadSem chan struct{}) { +func (lp *loadingPackage) analyzeRecursive(loadMode LoadMode, loadSem chan struct{}) { lp.analyzeOnce.Do(func() { // Load the direct dependencies, in parallel. var wg sync.WaitGroup wg.Add(len(lp.imports)) for _, imp := range lp.imports { go func(imp *loadingPackage) { - imp.analyzeRecursive(needWholeProgram, loadSem) + imp.analyzeRecursive(loadMode, loadSem) wg.Done() }(imp) } wg.Wait() - lp.analyze(needWholeProgram, loadSem) + lp.analyze(loadMode, loadSem) }) } -func (lp *loadingPackage) analyze(needWholeProgram bool, loadSem chan struct{}) { +func (lp *loadingPackage) analyze(loadMode LoadMode, loadSem chan struct{}) { loadSem <- struct{}{} defer func() { <-loadSem }() defer func() { - if !needWholeProgram { + if loadMode < LoadModeWholeProgram { // Save memory on unused more fields. lp.decUse() } }() - if err := lp.loadWithFacts(needWholeProgram); err != nil { + if err := lp.loadWithFacts(loadMode); err != nil { werr := errors.Wrapf(err, "failed to load package %s", lp.pkg.Name) // Don't need to write error to errCh, it will be extracted and reported on another layer. // Unblock depending actions and propagate error. @@ -964,9 +970,32 @@ func (lp *loadingPackage) analyze(needWholeProgram bool, loadSem chan struct{}) actsWg.Wait() } -func (lp *loadingPackage) loadFromSource() error { +func (lp *loadingPackage) loadFromSource(loadMode LoadMode) error { pkg := lp.pkg + // Many packages have few files, much fewer than there + // are CPU cores. Additionally, parsing each individual file is + // very fast. A naive parallel implementation of this loop won't + // be faster, and tends to be slower due to extra scheduling, + // bookkeeping and potentially false sharing of cache lines. + pkg.Syntax = make([]*ast.File, 0, len(pkg.CompiledGoFiles)) + for _, file := range pkg.CompiledGoFiles { + f, err := parser.ParseFile(pkg.Fset, file, nil, parser.ParseComments) + if err != nil { + pkg.Errors = append(pkg.Errors, lp.convertError(err)...) + continue + } + pkg.Syntax = append(pkg.Syntax, f) + } + if len(pkg.Errors) != 0 { + pkg.IllTyped = true + return nil + } + + if loadMode == LoadModeSyntax { + return nil + } + // Call NewPackage directly with explicit name. // This avoids skew between golist and go/types when the files' // package declarations are inconsistent. @@ -979,20 +1008,6 @@ func (lp *loadingPackage) loadFromSource() error { pkg.IllTyped = true - // Many packages have few files, much fewer than there - // are CPU cores. Additionally, parsing each individual file is - // very fast. A naive parallel implementation of this loop won't - // be faster, and tends to be slower due to extra scheduling, - // bookkeeping and potentially false sharing of cache lines. - pkg.Syntax = make([]*ast.File, len(pkg.CompiledGoFiles)) - for i, file := range pkg.CompiledGoFiles { - f, err := parser.ParseFile(pkg.Fset, file, nil, parser.ParseComments) - if err != nil { - pkg.Errors = append(pkg.Errors, lp.convertError(err)...) - return err - } - pkg.Syntax[i] = f - } pkg.TypesInfo = &types.Info{ Types: make(map[ast.Expr]types.TypeAndValue), Defs: make(map[*ast.Ident]types.Object), @@ -1027,11 +1042,19 @@ func (lp *loadingPackage) loadFromSource() error { pkg.Errors = append(pkg.Errors, lp.convertError(err)...) }, } - err := types.NewChecker(tc, pkg.Fset, pkg.Types, pkg.TypesInfo).Files(pkg.Syntax) - if err != nil { - return err + _ = types.NewChecker(tc, pkg.Fset, pkg.Types, pkg.TypesInfo).Files(pkg.Syntax) + // Don't handle error here: errors are adding by tc.Error function. + + illTyped := len(pkg.Errors) != 0 + if !illTyped { + for _, imp := range lp.imports { + if imp.pkg.IllTyped { + illTyped = true + break + } + } } - pkg.IllTyped = false + pkg.IllTyped = illTyped return nil } @@ -1106,30 +1129,30 @@ func (lp *loadingPackage) loadFromExportData() error { return nil } -func (lp *loadingPackage) loadWithFacts(needWholeProgram bool) error { +func (act *action) markDepsForAnalyzingSource() { + // Horizontal deps (analyzer.Requires) must be loaded from source and analyzed before analyzing + // this action. + for _, dep := range act.deps { + if dep.pkg == act.pkg { + // Analyze source only for horizontal dependencies, e.g. from "buildssa". + dep.needAnalyzeSource = true // can't be set in parallel + } + } +} + +func (lp *loadingPackage) loadWithFacts(loadMode LoadMode) error { pkg := lp.pkg if pkg.PkgPath == unsafePkgName { - if !needWholeProgram { // the package wasn't loaded yet - // Fill in the blanks to avoid surprises. + // Fill in the blanks to avoid surprises. + pkg.Syntax = []*ast.File{} + if loadMode >= LoadModeTypesInfo { pkg.Types = types.Unsafe - pkg.Syntax = []*ast.File{} pkg.TypesInfo = new(types.Info) } return nil } - markDepsForAnalyzingSource := func(act *action) { - // Horizontal deps (analyzer.Requires) must be loaded from source and analyzed before analyzing - // this action. - for _, dep := range act.deps { - if dep.pkg == act.pkg { - // Analyze source only for horizontal dependencies, e.g. from "buildssa". - dep.needAnalyzeSource = true // can't be set in parallel - } - } - } - if pkg.TypesInfo != nil { // Already loaded package, e.g. because another not go/analysis linter required types for deps. // Try load cached facts for it. @@ -1139,7 +1162,7 @@ func (lp *loadingPackage) loadWithFacts(needWholeProgram bool) error { // Cached facts loading failed: analyze later the action from source. act.needAnalyzeSource = true factsCacheDebugf("Loading of facts for already loaded %s failed, analyze it from source later", act) - markDepsForAnalyzingSource(act) + act.markDepsForAnalyzingSource() } } return nil @@ -1148,32 +1171,40 @@ func (lp *loadingPackage) loadWithFacts(needWholeProgram bool) error { if lp.isInitial { // No need to load cached facts: the package will be analyzed from source // because it's the initial. - return lp.loadFromSource() + return lp.loadFromSource(loadMode) } - // Load package from export data - if err := lp.loadFromExportData(); err != nil { - // We asked Go to give us up to date export data, yet - // we can't load it. There must be something wrong. - // - // Attempt loading from source. This should fail (because - // otherwise there would be export data); we just want to - // get the compile errors. If loading from source succeeds - // we discard the result, anyway. Otherwise we'll fail - // when trying to reload from export data later. + return lp.loadImportedPackageWithFacts(loadMode) +} - // Otherwise it panics because uses already existing (from exported data) types. - pkg.Types = types.NewPackage(pkg.PkgPath, pkg.Name) - if srcErr := lp.loadFromSource(); srcErr != nil { - return srcErr +func (lp *loadingPackage) loadImportedPackageWithFacts(loadMode LoadMode) error { + pkg := lp.pkg + + // Load package from export data + if loadMode >= LoadModeTypesInfo { + if err := lp.loadFromExportData(); err != nil { + // We asked Go to give us up to date export data, yet + // we can't load it. There must be something wrong. + // + // Attempt loading from source. This should fail (because + // otherwise there would be export data); we just want to + // get the compile errors. If loading from source succeeds + // we discard the result, anyway. Otherwise we'll fail + // when trying to reload from export data later. + + // Otherwise it panics because uses already existing (from exported data) types. + pkg.Types = types.NewPackage(pkg.PkgPath, pkg.Name) + if srcErr := lp.loadFromSource(loadMode); srcErr != nil { + return srcErr + } + // Make sure this package can't be imported successfully + pkg.Errors = append(pkg.Errors, packages.Error{ + Pos: "-", + Msg: fmt.Sprintf("could not load export data: %s", err), + Kind: packages.ParseError, + }) + return errors.Wrap(err, "could not load export data") } - // Make sure this package can't be imported successfully - pkg.Errors = append(pkg.Errors, packages.Error{ - Pos: "-", - Msg: fmt.Sprintf("could not load export data: %s", err), - Kind: packages.ParseError, - }) - return errors.Wrap(err, "could not load export data") } needLoadFromSource := false @@ -1187,7 +1218,7 @@ func (lp *loadingPackage) loadWithFacts(needWholeProgram bool) error { act.needAnalyzeSource = true // can't be set in parallel needLoadFromSource = true - markDepsForAnalyzingSource(act) + act.markDepsForAnalyzingSource() } if needLoadFromSource { @@ -1195,8 +1226,10 @@ func (lp *loadingPackage) loadWithFacts(needWholeProgram bool) error { // the analysis we need to load the package from source code. // Otherwise it panics because uses already existing (from exported data) types. - pkg.Types = types.NewPackage(pkg.PkgPath, pkg.Name) - return lp.loadFromSource() + if loadMode >= LoadModeTypesInfo { + pkg.Types = types.NewPackage(pkg.PkgPath, pkg.Name) + } + return lp.loadFromSource(loadMode) } return nil diff --git a/pkg/golinters/gochecknoglobals.go b/pkg/golinters/gochecknoglobals.go index c1a67ad7c643..cdbd492ed252 100644 --- a/pkg/golinters/gochecknoglobals.go +++ b/pkg/golinters/gochecknoglobals.go @@ -1,36 +1,57 @@ package golinters import ( - "context" "fmt" "go/ast" "go/token" "strings" + "sync" + + "golang.org/x/tools/go/analysis" + + "github.com/golangci/golangci-lint/pkg/golinters/goanalysis" "github.com/golangci/golangci-lint/pkg/lint/linter" "github.com/golangci/golangci-lint/pkg/result" ) -type Gochecknoglobals struct{} +const gochecknoglobalsName = "gochecknoglobals" -func (Gochecknoglobals) Name() string { - return "gochecknoglobals" -} +//nolint:dupl +func NewGochecknoglobals() *goanalysis.Linter { + var mu sync.Mutex + var resIssues []result.Issue -func (Gochecknoglobals) Desc() string { - return "Checks that no globals are present in Go code" -} + analyzer := &analysis.Analyzer{ + Name: goanalysis.TheOnlyAnalyzerName, + Doc: goanalysis.TheOnlyanalyzerDoc, + Run: func(pass *analysis.Pass) (interface{}, error) { + var res []result.Issue + for _, file := range pass.Files { + res = append(res, checkFileForGlobals(file, pass.Fset)...) + } + if len(res) == 0 { + return nil, nil + } -func (lint Gochecknoglobals) Run(ctx context.Context, lintCtx *linter.Context) ([]result.Issue, error) { - var res []result.Issue - for _, f := range lintCtx.ASTCache.GetAllValidFiles() { - res = append(res, lint.checkFile(f.F, f.Fset)...) - } + mu.Lock() + resIssues = append(resIssues, res...) + mu.Unlock() - return res, nil + return nil, nil + }, + } + return goanalysis.NewLinter( + gochecknoglobalsName, + "Tool for detection of long functions", + []*analysis.Analyzer{analyzer}, + nil, + ).WithIssuesReporter(func(*linter.Context) []result.Issue { + return resIssues + }).WithLoadMode(goanalysis.LoadModeSyntax) } -func (lint Gochecknoglobals) checkFile(f *ast.File, fset *token.FileSet) []result.Issue { +func checkFileForGlobals(f *ast.File, fset *token.FileSet) []result.Issue { var res []result.Issue for _, decl := range f.Decls { genDecl, ok := decl.(*ast.GenDecl) @@ -51,7 +72,7 @@ func (lint Gochecknoglobals) checkFile(f *ast.File, fset *token.FileSet) []resul res = append(res, result.Issue{ Pos: fset.Position(vn.Pos()), Text: fmt.Sprintf("%s is a global variable", formatCode(vn.Name, nil)), - FromLinter: lint.Name(), + FromLinter: gochecknoglobalsName, }) } } diff --git a/pkg/golinters/gochecknoinits.go b/pkg/golinters/gochecknoinits.go index 6cd74decfd90..7d9140807baa 100644 --- a/pkg/golinters/gochecknoinits.go +++ b/pkg/golinters/gochecknoinits.go @@ -1,35 +1,55 @@ package golinters import ( - "context" "fmt" "go/ast" "go/token" + "sync" + "golang.org/x/tools/go/analysis" + + "github.com/golangci/golangci-lint/pkg/golinters/goanalysis" "github.com/golangci/golangci-lint/pkg/lint/linter" "github.com/golangci/golangci-lint/pkg/result" ) -type Gochecknoinits struct{} +const gochecknoinitsName = "gochecknoinits" -func (Gochecknoinits) Name() string { - return "gochecknoinits" -} +//nolint:dupl +func NewGochecknoinits() *goanalysis.Linter { + var mu sync.Mutex + var resIssues []result.Issue -func (Gochecknoinits) Desc() string { - return "Checks that no init functions are present in Go code" -} + analyzer := &analysis.Analyzer{ + Name: goanalysis.TheOnlyAnalyzerName, + Doc: goanalysis.TheOnlyanalyzerDoc, + Run: func(pass *analysis.Pass) (interface{}, error) { + var res []result.Issue + for _, file := range pass.Files { + res = append(res, checkFileForInits(file, pass.Fset)...) + } + if len(res) == 0 { + return nil, nil + } -func (lint Gochecknoinits) Run(ctx context.Context, lintCtx *linter.Context) ([]result.Issue, error) { - var res []result.Issue - for _, f := range lintCtx.ASTCache.GetAllValidFiles() { - res = append(res, lint.checkFile(f.F, f.Fset)...) - } + mu.Lock() + resIssues = append(resIssues, res...) + mu.Unlock() - return res, nil + return nil, nil + }, + } + return goanalysis.NewLinter( + gochecknoinitsName, + "Checks that no init functions are present in Go code", + []*analysis.Analyzer{analyzer}, + nil, + ).WithIssuesReporter(func(*linter.Context) []result.Issue { + return resIssues + }).WithLoadMode(goanalysis.LoadModeSyntax) } -func (lint Gochecknoinits) checkFile(f *ast.File, fset *token.FileSet) []result.Issue { +func checkFileForInits(f *ast.File, fset *token.FileSet) []result.Issue { var res []result.Issue for _, decl := range f.Decls { funcDecl, ok := decl.(*ast.FuncDecl) @@ -42,7 +62,7 @@ func (lint Gochecknoinits) checkFile(f *ast.File, fset *token.FileSet) []result. res = append(res, result.Issue{ Pos: fset.Position(funcDecl.Pos()), Text: fmt.Sprintf("don't use %s function", formatCode(name, nil)), - FromLinter: lint.Name(), + FromLinter: gochecknoinitsName, }) } } diff --git a/pkg/golinters/goconst.go b/pkg/golinters/goconst.go index 87011df9d275..00787c8cbcbe 100644 --- a/pkg/golinters/goconst.go +++ b/pkg/golinters/goconst.go @@ -1,45 +1,62 @@ package golinters import ( - "context" "fmt" + "sync" goconstAPI "github.com/golangci/goconst" + "golang.org/x/tools/go/analysis" + "github.com/golangci/golangci-lint/pkg/golinters/goanalysis" "github.com/golangci/golangci-lint/pkg/lint/linter" "github.com/golangci/golangci-lint/pkg/result" ) -type Goconst struct{} +const goconstName = "goconst" -func (Goconst) Name() string { - return "goconst" -} +func NewGoconst() *goanalysis.Linter { + var mu sync.Mutex + var resIssues []result.Issue + + analyzer := &analysis.Analyzer{ + Name: goanalysis.TheOnlyAnalyzerName, + Doc: goanalysis.TheOnlyanalyzerDoc, + } + return goanalysis.NewLinter( + goconstName, + "Finds repeated strings that could be replaced by a constant", + []*analysis.Analyzer{analyzer}, + nil, + ).WithContextSetter(func(lintCtx *linter.Context) { + analyzer.Run = func(pass *analysis.Pass) (interface{}, error) { + issues, err := checkConstants(pass, lintCtx) + if err != nil || len(issues) == 0 { + return nil, err + } -func (Goconst) Desc() string { - return "Finds repeated strings that could be replaced by a constant" + mu.Lock() + resIssues = append(resIssues, issues...) + mu.Unlock() + + return nil, nil + } + }).WithIssuesReporter(func(*linter.Context) []result.Issue { + return resIssues + }).WithLoadMode(goanalysis.LoadModeSyntax) } -func (lint Goconst) Run(ctx context.Context, lintCtx *linter.Context) ([]result.Issue, error) { - var goconstIssues []goconstAPI.Issue +func checkConstants(pass *analysis.Pass, lintCtx *linter.Context) ([]result.Issue, error) { cfg := goconstAPI.Config{ MatchWithConstants: true, MinStringLength: lintCtx.Settings().Goconst.MinStringLen, MinOccurrences: lintCtx.Settings().Goconst.MinOccurrencesCount, } - for _, pkg := range lintCtx.Packages { - files, fset, err := getASTFilesForGoPkg(lintCtx, pkg) - if err != nil { - return nil, err - } - - issues, err := goconstAPI.Run(files, fset, &cfg) - if err != nil { - return nil, err - } - goconstIssues = append(goconstIssues, issues...) + goconstIssues, err := goconstAPI.Run(pass.Files, pass.Fset, &cfg) + if err != nil { + return nil, err } + if len(goconstIssues) == 0 { return nil, nil } @@ -56,7 +73,7 @@ func (lint Goconst) Run(ctx context.Context, lintCtx *linter.Context) ([]result. res = append(res, result.Issue{ Pos: i.Pos, Text: textBegin + textEnd, - FromLinter: lint.Name(), + FromLinter: goconstName, }) } diff --git a/pkg/golinters/gocritic.go b/pkg/golinters/gocritic.go index ccb5721f1700..0b24e7dd90a9 100644 --- a/pkg/golinters/gocritic.go +++ b/pkg/golinters/gocritic.go @@ -1,37 +1,70 @@ package golinters import ( - "context" "fmt" "go/ast" "go/types" "path/filepath" "runtime" - "runtime/debug" "sort" "strings" "sync" + "golang.org/x/tools/go/analysis" + + "github.com/golangci/golangci-lint/pkg/golinters/goanalysis" + "github.com/golangci/golangci-lint/pkg/config" "github.com/go-lintpack/lintpack" - "golang.org/x/tools/go/loader" "github.com/golangci/golangci-lint/pkg/lint/linter" "github.com/golangci/golangci-lint/pkg/result" ) -type Gocritic struct{} +const gocriticName = "gocritic" -func (Gocritic) Name() string { - return "gocritic" -} +func NewGocritic() *goanalysis.Linter { + var mu sync.Mutex + var resIssues []result.Issue + + sizes := types.SizesFor("gc", runtime.GOARCH) + + analyzer := &analysis.Analyzer{ + Name: goanalysis.TheOnlyAnalyzerName, + Doc: goanalysis.TheOnlyanalyzerDoc, + } + return goanalysis.NewLinter( + gocriticName, + "The most opinionated Go source code linter", + []*analysis.Analyzer{analyzer}, + nil, + ).WithContextSetter(func(lintCtx *linter.Context) { + analyzer.Run = func(pass *analysis.Pass) (interface{}, error) { + lintpackCtx := lintpack.NewContext(pass.Fset, sizes) + enabledCheckers, err := buildEnabledCheckers(lintCtx, lintpackCtx) + if err != nil { + return nil, err + } -func (Gocritic) Desc() string { - return "The most opinionated Go source code linter" + lintpackCtx.SetPackageInfo(pass.TypesInfo, pass.Pkg) + res := runGocriticOnPackage(lintpackCtx, enabledCheckers, pass.Files) + if len(res) == 0 { + return nil, nil + } + + mu.Lock() + resIssues = append(resIssues, res...) + mu.Unlock() + + return nil, nil + } + }).WithIssuesReporter(func(*linter.Context) []result.Issue { + return resIssues + }).WithLoadMode(goanalysis.LoadModeTypesInfo) } -func (Gocritic) normalizeCheckerInfoParams(info *lintpack.CheckerInfo) lintpack.CheckerParams { +func normalizeCheckerInfoParams(info *lintpack.CheckerInfo) lintpack.CheckerParams { // lowercase info param keys here because golangci-lint's config parser lowercases all strings ret := lintpack.CheckerParams{} for k, v := range info.Params { @@ -41,13 +74,13 @@ func (Gocritic) normalizeCheckerInfoParams(info *lintpack.CheckerInfo) lintpack. return ret } -func (lint Gocritic) configureCheckerInfo(info *lintpack.CheckerInfo, allParams map[string]config.GocriticCheckSettings) error { +func configureCheckerInfo(info *lintpack.CheckerInfo, allParams map[string]config.GocriticCheckSettings) error { params := allParams[strings.ToLower(info.Name)] if params == nil { // no config for this checker return nil } - infoParams := lint.normalizeCheckerInfoParams(info) + infoParams := normalizeCheckerInfoParams(info) for k, p := range params { v, ok := infoParams[k] if ok { @@ -74,7 +107,7 @@ func (lint Gocritic) configureCheckerInfo(info *lintpack.CheckerInfo, allParams return nil } -func (lint Gocritic) buildEnabledCheckers(lintCtx *linter.Context, lintpackCtx *lintpack.Context) ([]*lintpack.Checker, error) { +func buildEnabledCheckers(lintCtx *linter.Context, lintpackCtx *lintpack.Context) ([]*lintpack.Checker, error) { s := lintCtx.Settings().Gocritic allParams := s.GetLowercasedParams() @@ -84,7 +117,7 @@ func (lint Gocritic) buildEnabledCheckers(lintCtx *linter.Context, lintpackCtx * continue } - if err := lint.configureCheckerInfo(info, allParams); err != nil { + if err := configureCheckerInfo(info, allParams); err != nil { return nil, err } @@ -95,73 +128,34 @@ func (lint Gocritic) buildEnabledCheckers(lintCtx *linter.Context, lintpackCtx * return enabledCheckers, nil } -func (lint Gocritic) Run(ctx context.Context, lintCtx *linter.Context) ([]result.Issue, error) { - sizes := types.SizesFor("gc", runtime.GOARCH) - lintpackCtx := lintpack.NewContext(lintCtx.Program.Fset, sizes) - - enabledCheckers, err := lint.buildEnabledCheckers(lintCtx, lintpackCtx) - if err != nil { - return nil, err - } - - issuesCh := make(chan result.Issue, 1024) - var panicErr error - go func() { - defer func() { - if err := recover(); err != nil { - panicErr = fmt.Errorf("panic occurred: %s", err) - lintCtx.Log.Warnf("Panic: %s", debug.Stack()) - } - }() - - for _, pkgInfo := range lintCtx.Program.InitialPackages() { - lintpackCtx.SetPackageInfo(&pkgInfo.Info, pkgInfo.Pkg) - lint.runOnPackage(lintpackCtx, enabledCheckers, pkgInfo, issuesCh) - } - close(issuesCh) - }() - +func runGocriticOnPackage(lintpackCtx *lintpack.Context, checkers []*lintpack.Checker, + files []*ast.File) []result.Issue { var res []result.Issue - for i := range issuesCh { - res = append(res, i) - } - if panicErr != nil { - return nil, panicErr - } - - return res, nil -} - -func (lint Gocritic) runOnPackage(lintpackCtx *lintpack.Context, checkers []*lintpack.Checker, - pkgInfo *loader.PackageInfo, ret chan<- result.Issue) { - for _, f := range pkgInfo.Files { + for _, f := range files { filename := filepath.Base(lintpackCtx.FileSet.Position(f.Pos()).Filename) lintpackCtx.SetFileInfo(filename, f) - lint.runOnFile(lintpackCtx, f, checkers, ret) + issues := runGocriticOnFile(lintpackCtx, f, checkers) + res = append(res, issues...) } + return res } -func (lint Gocritic) runOnFile(ctx *lintpack.Context, f *ast.File, checkers []*lintpack.Checker, - ret chan<- result.Issue) { - var wg sync.WaitGroup - wg.Add(len(checkers)) +func runGocriticOnFile(ctx *lintpack.Context, f *ast.File, checkers []*lintpack.Checker) []result.Issue { + var res []result.Issue + for _, c := range checkers { // All checkers are expected to use *lint.Context // as read-only structure, so no copying is required. - go func(c *lintpack.Checker) { - defer wg.Done() - - for _, warn := range c.Check(f) { - pos := ctx.FileSet.Position(warn.Node.Pos()) - ret <- result.Issue{ - Pos: pos, - Text: fmt.Sprintf("%s: %s", c.Info.Name, warn.Text), - FromLinter: lint.Name(), - } - } - }(c) + for _, warn := range c.Check(f) { + pos := ctx.FileSet.Position(warn.Node.Pos()) + res = append(res, result.Issue{ + Pos: pos, + Text: fmt.Sprintf("%s: %s", c.Info.Name, warn.Text), + FromLinter: gocriticName, + }) + } } - wg.Wait() + return res } diff --git a/pkg/golinters/gocyclo.go b/pkg/golinters/gocyclo.go index 68811caf1acc..60d439fecbc8 100644 --- a/pkg/golinters/gocyclo.go +++ b/pkg/golinters/gocyclo.go @@ -1,52 +1,68 @@ package golinters import ( - "context" "fmt" "sort" + "sync" gocycloAPI "github.com/golangci/gocyclo/pkg/gocyclo" + "golang.org/x/tools/go/analysis" + "github.com/golangci/golangci-lint/pkg/golinters/goanalysis" "github.com/golangci/golangci-lint/pkg/lint/linter" "github.com/golangci/golangci-lint/pkg/result" ) -type Gocyclo struct{} +const gocycloName = "gocyclo" -func (Gocyclo) Name() string { - return "gocyclo" -} - -func (Gocyclo) Desc() string { - return "Computes and checks the cyclomatic complexity of functions" -} +func NewGocyclo() *goanalysis.Linter { + var mu sync.Mutex + var resIssues []result.Issue -func (g Gocyclo) Run(ctx context.Context, lintCtx *linter.Context) ([]result.Issue, error) { - var stats []gocycloAPI.Stat - for _, f := range lintCtx.ASTCache.GetAllValidFiles() { - stats = gocycloAPI.BuildStats(f.F, f.Fset, stats) - } - if len(stats) == 0 { - return nil, nil + analyzer := &analysis.Analyzer{ + Name: goanalysis.TheOnlyAnalyzerName, + Doc: goanalysis.TheOnlyanalyzerDoc, } + return goanalysis.NewLinter( + gocycloName, + "Computes and checks the cyclomatic complexity of functions", + []*analysis.Analyzer{analyzer}, + nil, + ).WithContextSetter(func(lintCtx *linter.Context) { + analyzer.Run = func(pass *analysis.Pass) (interface{}, error) { + var stats []gocycloAPI.Stat + for _, f := range pass.Files { + stats = gocycloAPI.BuildStats(f, pass.Fset, stats) + } + if len(stats) == 0 { + return nil, nil + } - sort.Slice(stats, func(i, j int) bool { - return stats[i].Complexity > stats[j].Complexity - }) + sort.Slice(stats, func(i, j int) bool { + return stats[i].Complexity > stats[j].Complexity + }) - res := make([]result.Issue, 0, len(stats)) - for _, s := range stats { - if s.Complexity <= lintCtx.Settings().Gocyclo.MinComplexity { - break // Break as the stats is already sorted from greatest to least - } + res := make([]result.Issue, 0, len(stats)) + for _, s := range stats { + if s.Complexity <= lintCtx.Settings().Gocyclo.MinComplexity { + break // Break as the stats is already sorted from greatest to least + } - res = append(res, result.Issue{ - Pos: s.Pos, - Text: fmt.Sprintf("cyclomatic complexity %d of func %s is high (> %d)", - s.Complexity, formatCode(s.FuncName, lintCtx.Cfg), lintCtx.Settings().Gocyclo.MinComplexity), - FromLinter: g.Name(), - }) - } + res = append(res, result.Issue{ + Pos: s.Pos, + Text: fmt.Sprintf("cyclomatic complexity %d of func %s is high (> %d)", + s.Complexity, formatCode(s.FuncName, lintCtx.Cfg), lintCtx.Settings().Gocyclo.MinComplexity), + FromLinter: gocycloName, + }) + } - return res, nil + mu.Lock() + resIssues = append(resIssues, res...) + mu.Unlock() + + return nil, nil + } + }).WithIssuesReporter(func(*linter.Context) []result.Issue { + return resIssues + }).WithLoadMode(goanalysis.LoadModeSyntax) } diff --git a/pkg/golinters/godox.go b/pkg/golinters/godox.go index 3a8e3518e262..5c8d5921786e 100644 --- a/pkg/golinters/godox.go +++ b/pkg/golinters/godox.go @@ -1,46 +1,64 @@ package golinters import ( - "context" "go/token" "strings" + "sync" + "github.com/matoous/godox" + + "golang.org/x/tools/go/analysis" + + "github.com/golangci/golangci-lint/pkg/golinters/goanalysis" "github.com/golangci/golangci-lint/pkg/lint/linter" "github.com/golangci/golangci-lint/pkg/result" - - "github.com/matoous/godox" ) -type Godox struct{} - -func (Godox) Name() string { - return "godox" -} +const godoxName = "godox" -func (Godox) Desc() string { - return "Tool for detection of FIXME, TODO and other comment keywords" -} +func NewGodox() *goanalysis.Linter { + var mu sync.Mutex + var resIssues []result.Issue -func (f Godox) Run(ctx context.Context, lintCtx *linter.Context) ([]result.Issue, error) { - var issues []godox.Message - for _, file := range lintCtx.ASTCache.GetAllValidFiles() { - issues = append(issues, godox.Run(file.F, file.Fset, lintCtx.Settings().Godox.Keywords...)...) + analyzer := &analysis.Analyzer{ + Name: goanalysis.TheOnlyAnalyzerName, + Doc: goanalysis.TheOnlyanalyzerDoc, } + return goanalysis.NewLinter( + godoxName, + "Tool for detection of FIXME, TODO and other comment keywords", + []*analysis.Analyzer{analyzer}, + nil, + ).WithContextSetter(func(lintCtx *linter.Context) { + analyzer.Run = func(pass *analysis.Pass) (interface{}, error) { + var issues []godox.Message + for _, file := range pass.Files { + issues = append(issues, godox.Run(file, pass.Fset, lintCtx.Settings().Godox.Keywords...)...) + } - if len(issues) == 0 { - return nil, nil - } + if len(issues) == 0 { + return nil, nil + } - res := make([]result.Issue, len(issues)) - for k, i := range issues { - res[k] = result.Issue{ - Pos: token.Position{ - Filename: i.Pos.Filename, - Line: i.Pos.Line, - }, - Text: strings.TrimRight(i.Message, "\n"), - FromLinter: f.Name(), + res := make([]result.Issue, len(issues)) + for k, i := range issues { + res[k] = result.Issue{ + Pos: token.Position{ + Filename: i.Pos.Filename, + Line: i.Pos.Line, + }, + Text: strings.TrimRight(i.Message, "\n"), + FromLinter: godoxName, + } + } + + mu.Lock() + resIssues = append(resIssues, res...) + mu.Unlock() + + return nil, nil } - } - return res, nil + }).WithIssuesReporter(func(*linter.Context) []result.Issue { + return resIssues + }).WithLoadMode(goanalysis.LoadModeSyntax) } diff --git a/pkg/golinters/gofmt.go b/pkg/golinters/gofmt.go index ab46e92538a0..e3e3c44f47f0 100644 --- a/pkg/golinters/gofmt.go +++ b/pkg/golinters/gofmt.go @@ -1,318 +1,71 @@ package golinters import ( - "bytes" - "context" - "fmt" - "go/token" - "strings" + "sync" gofmtAPI "github.com/golangci/gofmt/gofmt" - goimportsAPI "github.com/golangci/gofmt/goimports" "github.com/pkg/errors" - diffpkg "github.com/sourcegraph/go-diff/diff" - "golang.org/x/tools/imports" + "golang.org/x/tools/go/analysis" + "github.com/golangci/golangci-lint/pkg/golinters/goanalysis" "github.com/golangci/golangci-lint/pkg/lint/linter" - "github.com/golangci/golangci-lint/pkg/logutils" "github.com/golangci/golangci-lint/pkg/result" ) -type Gofmt struct { - UseGoimports bool -} - -func (g Gofmt) Name() string { - if g.UseGoimports { - return "goimports" - } +const gofmtName = "gofmt" - return "gofmt" -} +func NewGofmt() *goanalysis.Linter { + var mu sync.Mutex + var resIssues []result.Issue -func (g Gofmt) Desc() string { - if g.UseGoimports { - return "Goimports does everything that gofmt does. Additionally it checks unused imports" + analyzer := &analysis.Analyzer{ + Name: goanalysis.TheOnlyAnalyzerName, + Doc: goanalysis.TheOnlyanalyzerDoc, } - - return "Gofmt checks whether code was gofmt-ed. By default " + - "this tool runs with -s option to check for code simplification" -} - -type Change struct { - LineRange result.Range - Replacement result.Replacement -} - -type diffLineType string - -const ( - diffLineAdded diffLineType = "added" - diffLineOriginal diffLineType = "original" - diffLineDeleted diffLineType = "deleted" -) - -type diffLine struct { - originalNumber int // 1-based original line number - typ diffLineType - data string // "+" or "-" stripped line -} - -type hunkChangesParser struct { - // needed because we merge currently added lines with the last original line - lastOriginalLine *diffLine - - // if the first line of diff is an adding we save all additions to replacementLinesToPrepend - replacementLinesToPrepend []string - - log logutils.Log - - lines []diffLine - - ret []Change -} - -func (p *hunkChangesParser) parseDiffLines(h *diffpkg.Hunk) { - lines := bytes.Split(h.Body, []byte{'\n'}) - currentOriginalLineNumer := int(h.OrigStartLine) - var ret []diffLine - - for i, line := range lines { - dl := diffLine{ - originalNumber: currentOriginalLineNumer, - } - - lineStr := string(line) - - //nolint:gocritic - if strings.HasPrefix(lineStr, "-") { - dl.typ = diffLineDeleted - dl.data = strings.TrimPrefix(lineStr, "-") - currentOriginalLineNumer++ - } else if strings.HasPrefix(lineStr, "+") { - dl.typ = diffLineAdded - dl.data = strings.TrimPrefix(lineStr, "+") - } else { - if i == len(lines)-1 && lineStr == "" { - // handle last \n: don't add an empty original line - break + return goanalysis.NewLinter( + gofmtName, + "Gofmt checks whether code was gofmt-ed. By default "+ + "this tool runs with -s option to check for code simplification", + []*analysis.Analyzer{analyzer}, + nil, + ).WithContextSetter(func(lintCtx *linter.Context) { + analyzer.Run = func(pass *analysis.Pass) (interface{}, error) { + var fileNames []string + for _, f := range pass.Files { + pos := pass.Fset.Position(f.Pos()) + fileNames = append(fileNames, pos.Filename) } - dl.typ = diffLineOriginal - dl.data = strings.TrimPrefix(lineStr, " ") - currentOriginalLineNumer++ - } - - ret = append(ret, dl) - } - - p.lines = ret -} - -func (p *hunkChangesParser) handleOriginalLine(line diffLine, i *int) { - if len(p.replacementLinesToPrepend) == 0 { - p.lastOriginalLine = &line - *i++ - return - } - - // check following added lines for the case: - // + added line 1 - // original line - // + added line 2 - - *i++ - var followingAddedLines []string - for ; *i < len(p.lines) && p.lines[*i].typ == diffLineAdded; *i++ { - followingAddedLines = append(followingAddedLines, p.lines[*i].data) - } - - p.ret = append(p.ret, Change{ - LineRange: result.Range{ - From: line.originalNumber, - To: line.originalNumber, - }, - Replacement: result.Replacement{ - NewLines: append(p.replacementLinesToPrepend, append([]string{line.data}, followingAddedLines...)...), - }, - }) - p.replacementLinesToPrepend = nil - p.lastOriginalLine = &line -} - -func (p *hunkChangesParser) handleDeletedLines(deletedLines []diffLine, addedLines []string) { - change := Change{ - LineRange: result.Range{ - From: deletedLines[0].originalNumber, - To: deletedLines[len(deletedLines)-1].originalNumber, - }, - } - - if len(addedLines) != 0 { - //nolint:gocritic - change.Replacement.NewLines = append(p.replacementLinesToPrepend, addedLines...) - if len(p.replacementLinesToPrepend) != 0 { - p.replacementLinesToPrepend = nil - } - - p.ret = append(p.ret, change) - return - } - - // delete-only change with possible prepending - if len(p.replacementLinesToPrepend) != 0 { - change.Replacement.NewLines = p.replacementLinesToPrepend - p.replacementLinesToPrepend = nil - } else { - change.Replacement.NeedOnlyDelete = true - } - - p.ret = append(p.ret, change) -} - -func (p *hunkChangesParser) handleAddedOnlyLines(addedLines []string) { - if p.lastOriginalLine == nil { - // the first line is added; the diff looks like: - // 1. + ... - // 2. - ... - // or - // 1. + ... - // 2. ... - - p.replacementLinesToPrepend = addedLines - return - } - - // add-only change merged into the last original line with possible prepending - p.ret = append(p.ret, Change{ - LineRange: result.Range{ - From: p.lastOriginalLine.originalNumber, - To: p.lastOriginalLine.originalNumber, - }, - Replacement: result.Replacement{ - NewLines: append(p.replacementLinesToPrepend, append([]string{p.lastOriginalLine.data}, addedLines...)...), - }, - }) - p.replacementLinesToPrepend = nil -} - -func (p *hunkChangesParser) parse(h *diffpkg.Hunk) []Change { - p.parseDiffLines(h) - - for i := 0; i < len(p.lines); { - line := p.lines[i] - if line.typ == diffLineOriginal { - p.handleOriginalLine(line, &i) //nolint:scopelint - continue - } + var issues []result.Issue - var deletedLines []diffLine - for ; i < len(p.lines) && p.lines[i].typ == diffLineDeleted; i++ { - deletedLines = append(deletedLines, p.lines[i]) - } - - var addedLines []string - for ; i < len(p.lines) && p.lines[i].typ == diffLineAdded; i++ { - addedLines = append(addedLines, p.lines[i].data) - } - - if len(deletedLines) != 0 { - p.handleDeletedLines(deletedLines, addedLines) - continue - } - - // no deletions, only addings - p.handleAddedOnlyLines(addedLines) - } - - if len(p.replacementLinesToPrepend) != 0 { - p.log.Infof("The diff contains only additions: no original or deleted lines: %#v", p.lines) - return nil - } - - return p.ret -} - -func (g Gofmt) extractIssuesFromPatch(patch string, log logutils.Log, lintCtx *linter.Context) ([]result.Issue, error) { - diffs, err := diffpkg.ParseMultiFileDiff([]byte(patch)) - if err != nil { - return nil, errors.Wrap(err, "can't parse patch") - } - - if len(diffs) == 0 { - return nil, fmt.Errorf("got no diffs from patch parser: %v", diffs) - } - - issues := []result.Issue{} - for _, d := range diffs { - if len(d.Hunks) == 0 { - log.Warnf("Got no hunks in diff %+v", d) - continue - } - - for _, hunk := range d.Hunks { - var text string - if g.UseGoimports { - text = "File is not `goimports`-ed" - } else { - text = "File is not `gofmt`-ed" - if lintCtx.Settings().Gofmt.Simplify { - text += " with `-s`" + for _, f := range fileNames { + diff, err := gofmtAPI.Run(f, lintCtx.Settings().Gofmt.Simplify) + if err != nil { // TODO: skip + return nil, err } - } - p := hunkChangesParser{ - log: log, - } - changes := p.parse(hunk) - for _, change := range changes { - change := change // fix scope - i := result.Issue{ - FromLinter: g.Name(), - Pos: token.Position{ - Filename: d.NewName, - Line: change.LineRange.From, - }, - Text: text, - Replacement: &change.Replacement, + if diff == nil { + continue } - if change.LineRange.From != change.LineRange.To { - i.LineRange = &change.LineRange + + is, err := extractIssuesFromPatch(string(diff), lintCtx.Log, lintCtx, false) + if err != nil { + return nil, errors.Wrapf(err, "can't extract issues from gofmt diff output %q", string(diff)) } - issues = append(issues, i) + issues = append(issues, is...) } - } - } - - return issues, nil -} -func (g Gofmt) Run(ctx context.Context, lintCtx *linter.Context) ([]result.Issue, error) { - var issues []result.Issue + if len(issues) == 0 { + return nil, nil + } - for _, f := range getAllFileNames(lintCtx) { - var diff []byte - var err error - if g.UseGoimports { - imports.LocalPrefix = lintCtx.Settings().Goimports.LocalPrefixes - diff, err = goimportsAPI.Run(f) - } else { - diff, err = gofmtAPI.Run(f, lintCtx.Settings().Gofmt.Simplify) - } - if err != nil { // TODO: skip - return nil, err - } - if diff == nil { - continue - } + mu.Lock() + resIssues = append(resIssues, issues...) + mu.Unlock() - is, err := g.extractIssuesFromPatch(string(diff), lintCtx.Log, lintCtx) - if err != nil { - return nil, fmt.Errorf("can't extract issues from gofmt diff output %q: %s", string(diff), err) + return nil, nil } - - issues = append(issues, is...) - } - - return issues, nil + }).WithIssuesReporter(func(*linter.Context) []result.Issue { + return resIssues + }).WithLoadMode(goanalysis.LoadModeSyntax) } diff --git a/pkg/golinters/gofmt_common.go b/pkg/golinters/gofmt_common.go new file mode 100644 index 000000000000..6f4e7dd78e0e --- /dev/null +++ b/pkg/golinters/gofmt_common.go @@ -0,0 +1,267 @@ +package golinters + +import ( + "bytes" + "fmt" + "go/token" + "strings" + + "github.com/pkg/errors" + diffpkg "github.com/sourcegraph/go-diff/diff" + + "github.com/golangci/golangci-lint/pkg/lint/linter" + "github.com/golangci/golangci-lint/pkg/logutils" + "github.com/golangci/golangci-lint/pkg/result" +) + +type Change struct { + LineRange result.Range + Replacement result.Replacement +} + +type diffLineType string + +const ( + diffLineAdded diffLineType = "added" + diffLineOriginal diffLineType = "original" + diffLineDeleted diffLineType = "deleted" +) + +type diffLine struct { + originalNumber int // 1-based original line number + typ diffLineType + data string // "+" or "-" stripped line +} + +type hunkChangesParser struct { + // needed because we merge currently added lines with the last original line + lastOriginalLine *diffLine + + // if the first line of diff is an adding we save all additions to replacementLinesToPrepend + replacementLinesToPrepend []string + + log logutils.Log + + lines []diffLine + + ret []Change +} + +func (p *hunkChangesParser) parseDiffLines(h *diffpkg.Hunk) { + lines := bytes.Split(h.Body, []byte{'\n'}) + currentOriginalLineNumer := int(h.OrigStartLine) + var ret []diffLine + + for i, line := range lines { + dl := diffLine{ + originalNumber: currentOriginalLineNumer, + } + + lineStr := string(line) + + //nolint:gocritic + if strings.HasPrefix(lineStr, "-") { + dl.typ = diffLineDeleted + dl.data = strings.TrimPrefix(lineStr, "-") + currentOriginalLineNumer++ + } else if strings.HasPrefix(lineStr, "+") { + dl.typ = diffLineAdded + dl.data = strings.TrimPrefix(lineStr, "+") + } else { + if i == len(lines)-1 && lineStr == "" { + // handle last \n: don't add an empty original line + break + } + + dl.typ = diffLineOriginal + dl.data = strings.TrimPrefix(lineStr, " ") + currentOriginalLineNumer++ + } + + ret = append(ret, dl) + } + + p.lines = ret +} + +func (p *hunkChangesParser) handleOriginalLine(line diffLine, i *int) { + if len(p.replacementLinesToPrepend) == 0 { + p.lastOriginalLine = &line + *i++ + return + } + + // check following added lines for the case: + // + added line 1 + // original line + // + added line 2 + + *i++ + var followingAddedLines []string + for ; *i < len(p.lines) && p.lines[*i].typ == diffLineAdded; *i++ { + followingAddedLines = append(followingAddedLines, p.lines[*i].data) + } + + p.ret = append(p.ret, Change{ + LineRange: result.Range{ + From: line.originalNumber, + To: line.originalNumber, + }, + Replacement: result.Replacement{ + NewLines: append(p.replacementLinesToPrepend, append([]string{line.data}, followingAddedLines...)...), + }, + }) + p.replacementLinesToPrepend = nil + p.lastOriginalLine = &line +} + +func (p *hunkChangesParser) handleDeletedLines(deletedLines []diffLine, addedLines []string) { + change := Change{ + LineRange: result.Range{ + From: deletedLines[0].originalNumber, + To: deletedLines[len(deletedLines)-1].originalNumber, + }, + } + + if len(addedLines) != 0 { + //nolint:gocritic + change.Replacement.NewLines = append(p.replacementLinesToPrepend, addedLines...) + if len(p.replacementLinesToPrepend) != 0 { + p.replacementLinesToPrepend = nil + } + + p.ret = append(p.ret, change) + return + } + + // delete-only change with possible prepending + if len(p.replacementLinesToPrepend) != 0 { + change.Replacement.NewLines = p.replacementLinesToPrepend + p.replacementLinesToPrepend = nil + } else { + change.Replacement.NeedOnlyDelete = true + } + + p.ret = append(p.ret, change) +} + +func (p *hunkChangesParser) handleAddedOnlyLines(addedLines []string) { + if p.lastOriginalLine == nil { + // the first line is added; the diff looks like: + // 1. + ... + // 2. - ... + // or + // 1. + ... + // 2. ... + + p.replacementLinesToPrepend = addedLines + return + } + + // add-only change merged into the last original line with possible prepending + p.ret = append(p.ret, Change{ + LineRange: result.Range{ + From: p.lastOriginalLine.originalNumber, + To: p.lastOriginalLine.originalNumber, + }, + Replacement: result.Replacement{ + NewLines: append(p.replacementLinesToPrepend, append([]string{p.lastOriginalLine.data}, addedLines...)...), + }, + }) + p.replacementLinesToPrepend = nil +} + +func (p *hunkChangesParser) parse(h *diffpkg.Hunk) []Change { + p.parseDiffLines(h) + + for i := 0; i < len(p.lines); { + line := p.lines[i] + if line.typ == diffLineOriginal { + p.handleOriginalLine(line, &i) //nolint:scopelint + continue + } + + var deletedLines []diffLine + for ; i < len(p.lines) && p.lines[i].typ == diffLineDeleted; i++ { + deletedLines = append(deletedLines, p.lines[i]) + } + + var addedLines []string + for ; i < len(p.lines) && p.lines[i].typ == diffLineAdded; i++ { + addedLines = append(addedLines, p.lines[i].data) + } + + if len(deletedLines) != 0 { + p.handleDeletedLines(deletedLines, addedLines) + continue + } + + // no deletions, only addings + p.handleAddedOnlyLines(addedLines) + } + + if len(p.replacementLinesToPrepend) != 0 { + p.log.Infof("The diff contains only additions: no original or deleted lines: %#v", p.lines) + return nil + } + + return p.ret +} + +func extractIssuesFromPatch(patch string, log logutils.Log, lintCtx *linter.Context, isGoimports bool) ([]result.Issue, error) { + diffs, err := diffpkg.ParseMultiFileDiff([]byte(patch)) + if err != nil { + return nil, errors.Wrap(err, "can't parse patch") + } + + if len(diffs) == 0 { + return nil, fmt.Errorf("got no diffs from patch parser: %v", diffs) + } + + issues := []result.Issue{} + for _, d := range diffs { + if len(d.Hunks) == 0 { + log.Warnf("Got no hunks in diff %+v", d) + continue + } + + for _, hunk := range d.Hunks { + var text string + if isGoimports { + text = "File is not `goimports`-ed" + } else { + text = "File is not `gofmt`-ed" + if lintCtx.Settings().Gofmt.Simplify { + text += " with `-s`" + } + } + p := hunkChangesParser{ + log: log, + } + changes := p.parse(hunk) + for _, change := range changes { + change := change // fix scope + linterName := gofmtName + if isGoimports { + linterName = goimportsName + } + i := result.Issue{ + FromLinter: linterName, + Pos: token.Position{ + Filename: d.NewName, + Line: change.LineRange.From, + }, + Text: text, + Replacement: &change.Replacement, + } + if change.LineRange.From != change.LineRange.To { + i.LineRange = &change.LineRange + } + + issues = append(issues, i) + } + } + } + + return issues, nil +} diff --git a/pkg/golinters/goimports.go b/pkg/golinters/goimports.go new file mode 100644 index 000000000000..262b666d7df9 --- /dev/null +++ b/pkg/golinters/goimports.go @@ -0,0 +1,72 @@ +package golinters + +import ( + "sync" + + goimportsAPI "github.com/golangci/gofmt/goimports" + "github.com/pkg/errors" + "golang.org/x/tools/go/analysis" + "golang.org/x/tools/imports" + + "github.com/golangci/golangci-lint/pkg/golinters/goanalysis" + "github.com/golangci/golangci-lint/pkg/lint/linter" + "github.com/golangci/golangci-lint/pkg/result" +) + +const goimportsName = "goimports" + +func NewGoimports() *goanalysis.Linter { + var mu sync.Mutex + var resIssues []result.Issue + + analyzer := &analysis.Analyzer{ + Name: goanalysis.TheOnlyAnalyzerName, + Doc: goanalysis.TheOnlyanalyzerDoc, + } + return goanalysis.NewLinter( + goimportsName, + "Goimports does everything that gofmt does. Additionally it checks unused imports", + []*analysis.Analyzer{analyzer}, + nil, + ).WithContextSetter(func(lintCtx *linter.Context) { + imports.LocalPrefix = lintCtx.Settings().Goimports.LocalPrefixes + analyzer.Run = func(pass *analysis.Pass) (interface{}, error) { + var fileNames []string + for _, f := range pass.Files { + pos := pass.Fset.Position(f.Pos()) + fileNames = append(fileNames, pos.Filename) + } + + var issues []result.Issue + + for _, f := range fileNames { + diff, err := goimportsAPI.Run(f) + if err != nil { // TODO: skip + return nil, err + } + if diff == nil { + continue + } + + is, err := extractIssuesFromPatch(string(diff), lintCtx.Log, lintCtx, true) + if err != nil { + return nil, errors.Wrapf(err, "can't extract issues from gofmt diff output %q", string(diff)) + } + + issues = append(issues, is...) + } + + if len(issues) == 0 { + return nil, nil + } + + mu.Lock() + resIssues = append(resIssues, issues...) + mu.Unlock() + + return nil, nil + } + }).WithIssuesReporter(func(*linter.Context) []result.Issue { + return resIssues + }).WithLoadMode(goanalysis.LoadModeSyntax) +} diff --git a/pkg/golinters/golint.go b/pkg/golinters/golint.go index de58d0a7a332..a77c6f1ba4df 100644 --- a/pkg/golinters/golint.go +++ b/pkg/golinters/golint.go @@ -1,51 +1,20 @@ package golinters import ( - "context" "fmt" "go/ast" "go/token" + "sync" lintAPI "github.com/golangci/lint-1" + "golang.org/x/tools/go/analysis" + "github.com/golangci/golangci-lint/pkg/golinters/goanalysis" "github.com/golangci/golangci-lint/pkg/lint/linter" "github.com/golangci/golangci-lint/pkg/result" ) -type Golint struct{} - -func (Golint) Name() string { - return "golint" -} - -func (Golint) Desc() string { - return "Golint differs from gofmt. Gofmt reformats Go source code, whereas golint prints out style mistakes" -} - -func (g Golint) Run(ctx context.Context, lintCtx *linter.Context) ([]result.Issue, error) { - var issues []result.Issue - var lintErr error - for _, pkg := range lintCtx.Packages { - files, fset, err := getASTFilesForGoPkg(lintCtx, pkg) - if err != nil { - return nil, err - } - - i, err := g.lintPkg(lintCtx.Settings().Golint.MinConfidence, files, fset) - if err != nil { - lintErr = err - continue - } - issues = append(issues, i...) - } - if lintErr != nil { - lintCtx.Log.Warnf("Golint: %s", lintErr) - } - - return issues, nil -} - -func (g Golint) lintPkg(minConfidence float64, files []*ast.File, fset *token.FileSet) ([]result.Issue, error) { +func golintProcessPkg(minConfidence float64, files []*ast.File, fset *token.FileSet) ([]result.Issue, error) { l := new(lintAPI.Linter) ps, err := l.LintASTFiles(files, fset) if err != nil { @@ -62,7 +31,7 @@ func (g Golint) lintPkg(minConfidence float64, files []*ast.File, fset *token.Fi issues = append(issues, result.Issue{ Pos: ps[idx].Position, Text: ps[idx].Text, - FromLinter: g.Name(), + FromLinter: golintName, }) // TODO: use p.Link and p.Category } @@ -70,3 +39,36 @@ func (g Golint) lintPkg(minConfidence float64, files []*ast.File, fset *token.Fi return issues, nil } + +const golintName = "golint" + +func NewGolint() *goanalysis.Linter { + var mu sync.Mutex + var resIssues []result.Issue + + analyzer := &analysis.Analyzer{ + Name: goanalysis.TheOnlyAnalyzerName, + Doc: goanalysis.TheOnlyanalyzerDoc, + } + return goanalysis.NewLinter( + golintName, + "Golint differs from gofmt. Gofmt reformats Go source code, whereas golint prints out style mistakes", + []*analysis.Analyzer{analyzer}, + nil, + ).WithContextSetter(func(lintCtx *linter.Context) { + analyzer.Run = func(pass *analysis.Pass) (interface{}, error) { + res, err := golintProcessPkg(lintCtx.Settings().Golint.MinConfidence, pass.Files, pass.Fset) + if err != nil || len(res) == 0 { + return nil, err + } + + mu.Lock() + resIssues = append(resIssues, res...) + mu.Unlock() + + return nil, nil + } + }).WithIssuesReporter(func(*linter.Context) []result.Issue { + return resIssues + }).WithLoadMode(goanalysis.LoadModeSyntax) +} diff --git a/pkg/golinters/gosec.go b/pkg/golinters/gosec.go index 3187e6e2a114..75f04a3bcfe3 100644 --- a/pkg/golinters/gosec.go +++ b/pkg/golinters/gosec.go @@ -1,12 +1,17 @@ package golinters import ( - "context" "fmt" "go/token" "io/ioutil" "log" "strconv" + "sync" + + "golang.org/x/tools/go/analysis" + "golang.org/x/tools/go/packages" + + "github.com/golangci/golangci-lint/pkg/golinters/goanalysis" "github.com/securego/gosec" "github.com/securego/gosec/rules" @@ -15,55 +20,73 @@ import ( "github.com/golangci/golangci-lint/pkg/result" ) -type Gosec struct{} - -func (Gosec) Name() string { - return "gosec" -} +const gosecName = "gosec" -func (Gosec) Desc() string { - return "Inspects source code for security problems" -} +func NewGosec() *goanalysis.Linter { + var mu sync.Mutex + var resIssues []result.Issue -func (lint Gosec) Run(ctx context.Context, lintCtx *linter.Context) ([]result.Issue, error) { gasConfig := gosec.NewConfig() enabledRules := rules.Generate() logger := log.New(ioutil.Discard, "", 0) - analyzer := gosec.NewAnalyzer(gasConfig, true, logger) - analyzer.LoadRules(enabledRules.Builders()) - for _, pkg := range lintCtx.Packages { - analyzer.Check(pkg) - } - issues, _, _ := analyzer.Report() - if len(issues) == 0 { - return nil, nil + analyzer := &analysis.Analyzer{ + Name: goanalysis.TheOnlyAnalyzerName, + Doc: goanalysis.TheOnlyanalyzerDoc, } + return goanalysis.NewLinter( + gosecName, + "Inspects source code for security problems", + []*analysis.Analyzer{analyzer}, + nil, + ).WithContextSetter(func(lintCtx *linter.Context) { + analyzer.Run = func(pass *analysis.Pass) (interface{}, error) { + gosecAnalyzer := gosec.NewAnalyzer(gasConfig, true, logger) + gosecAnalyzer.LoadRules(enabledRules.Builders()) + pkg := &packages.Package{ + Fset: pass.Fset, + Syntax: pass.Files, + Types: pass.Pkg, + TypesInfo: pass.TypesInfo, + } + gosecAnalyzer.Check(pkg) + issues, _, _ := gosecAnalyzer.Report() + if len(issues) == 0 { + return nil, nil + } + + res := make([]result.Issue, 0, len(issues)) + for _, i := range issues { + text := fmt.Sprintf("%s: %s", i.RuleID, i.What) // TODO: use severity and confidence + var r *result.Range + line, err := strconv.Atoi(i.Line) + if err != nil { + r = &result.Range{} + if n, rerr := fmt.Sscanf(i.Line, "%d-%d", &r.From, &r.To); rerr != nil || n != 2 { + lintCtx.Log.Warnf("Can't convert gosec line number %q of %v to int: %s", i.Line, i, err) + continue + } + line = r.From + } - res := make([]result.Issue, 0, len(issues)) - for _, i := range issues { - text := fmt.Sprintf("%s: %s", i.RuleID, i.What) // TODO: use severity and confidence - var r *result.Range - line, err := strconv.Atoi(i.Line) - if err != nil { - r = &result.Range{} - if n, rerr := fmt.Sscanf(i.Line, "%d-%d", &r.From, &r.To); rerr != nil || n != 2 { - lintCtx.Log.Warnf("Can't convert gosec line number %q of %v to int: %s", i.Line, i, err) - continue + res = append(res, result.Issue{ + Pos: token.Position{ + Filename: i.File, + Line: line, + }, + Text: text, + LineRange: r, + FromLinter: gosecName, + }) } - line = r.From - } - res = append(res, result.Issue{ - Pos: token.Position{ - Filename: i.File, - Line: line, - }, - Text: text, - LineRange: r, - FromLinter: lint.Name(), - }) - } + mu.Lock() + resIssues = append(resIssues, res...) + mu.Unlock() - return res, nil + return nil, nil + } + }).WithIssuesReporter(func(*linter.Context) []result.Issue { + return resIssues + }).WithLoadMode(goanalysis.LoadModeTypesInfo) } diff --git a/pkg/golinters/gosimple.go b/pkg/golinters/gosimple.go new file mode 100644 index 000000000000..788fc8c63892 --- /dev/null +++ b/pkg/golinters/gosimple.go @@ -0,0 +1,19 @@ +package golinters + +import ( + "honnef.co/go/tools/simple" + + "github.com/golangci/golangci-lint/pkg/golinters/goanalysis" +) + +func NewGosimple() *goanalysis.Linter { + analyzers := analyzersMapToSlice(simple.Analyzers) + setAnalyzersGoVersion(analyzers) + + return goanalysis.NewLinter( + "gosimple", + "Linter for Go source code that specializes in simplifying a code", + analyzers, + nil, + ).WithLoadMode(goanalysis.LoadModeTypesInfo) +} diff --git a/pkg/golinters/govet.go b/pkg/golinters/govet.go index ecb8c8a2d16b..1dec50d97cfd 100644 --- a/pkg/golinters/govet.go +++ b/pkg/golinters/govet.go @@ -171,5 +171,5 @@ func NewGovet(cfg *config.GovetSettings) *goanalysis.Linter { "such as Printf calls whose arguments do not align with the format string", analyzersFromConfig(cfg), settings, - ) + ).WithLoadMode(goanalysis.LoadModeTypesInfo) } diff --git a/pkg/golinters/ineffassign.go b/pkg/golinters/ineffassign.go index b5d047b68a96..e0df64c005c2 100644 --- a/pkg/golinters/ineffassign.go +++ b/pkg/golinters/ineffassign.go @@ -1,8 +1,12 @@ package golinters import ( - "context" "fmt" + "sync" + + "golang.org/x/tools/go/analysis" + + "github.com/golangci/golangci-lint/pkg/golinters/goanalysis" ineffassignAPI "github.com/golangci/ineffassign" @@ -10,29 +14,50 @@ import ( "github.com/golangci/golangci-lint/pkg/result" ) -type Ineffassign struct{} - -func (Ineffassign) Name() string { - return "ineffassign" -} +const ineffassignName = "ineffassign" -func (Ineffassign) Desc() string { - return "Detects when assignments to existing variables are not used" -} - -func (lint Ineffassign) Run(ctx context.Context, lintCtx *linter.Context) ([]result.Issue, error) { - issues := ineffassignAPI.Run(getAllFileNames(lintCtx)) - if len(issues) == 0 { - return nil, nil - } +func NewIneffassign() *goanalysis.Linter { + var mu sync.Mutex + var resIssues []result.Issue - res := make([]result.Issue, 0, len(issues)) - for _, i := range issues { - res = append(res, result.Issue{ - Pos: i.Pos, - Text: fmt.Sprintf("ineffectual assignment to %s", formatCode(i.IdentName, lintCtx.Cfg)), - FromLinter: lint.Name(), - }) + analyzer := &analysis.Analyzer{ + Name: goanalysis.TheOnlyAnalyzerName, + Doc: goanalysis.TheOnlyanalyzerDoc, } - return res, nil + return goanalysis.NewLinter( + ineffassignName, + "Detects when assignments to existing variables are not used", + []*analysis.Analyzer{analyzer}, + nil, + ).WithContextSetter(func(lintCtx *linter.Context) { + analyzer.Run = func(pass *analysis.Pass) (interface{}, error) { + var fileNames []string + for _, f := range pass.Files { + pos := pass.Fset.Position(f.Pos()) + fileNames = append(fileNames, pos.Filename) + } + + issues := ineffassignAPI.Run(fileNames) + if len(issues) == 0 { + return nil, nil + } + + res := make([]result.Issue, 0, len(issues)) + for _, i := range issues { + res = append(res, result.Issue{ + Pos: i.Pos, + Text: fmt.Sprintf("ineffectual assignment to %s", formatCode(i.IdentName, lintCtx.Cfg)), + FromLinter: ineffassignName, + }) + } + + mu.Lock() + resIssues = append(resIssues, res...) + mu.Unlock() + + return nil, nil + } + }).WithIssuesReporter(func(*linter.Context) []result.Issue { + return resIssues + }).WithLoadMode(goanalysis.LoadModeSyntax) } diff --git a/pkg/golinters/interfacer.go b/pkg/golinters/interfacer.go index f3652e4901d9..9cb74799622b 100644 --- a/pkg/golinters/interfacer.go +++ b/pkg/golinters/interfacer.go @@ -1,7 +1,12 @@ package golinters import ( - "context" + "sync" + + "golang.org/x/tools/go/analysis" + "golang.org/x/tools/go/analysis/passes/buildssa" + + "github.com/golangci/golangci-lint/pkg/golinters/goanalysis" "mvdan.cc/interfacer/check" @@ -9,38 +14,56 @@ import ( "github.com/golangci/golangci-lint/pkg/result" ) -type Interfacer struct{} +const interfacerName = "interfacer" -func (Interfacer) Name() string { - return "interfacer" -} +func NewInterfacer() *goanalysis.Linter { + var mu sync.Mutex + var resIssues []result.Issue -func (Interfacer) Desc() string { - return "Linter that suggests narrower interface types" -} + analyzer := &analysis.Analyzer{ + Name: goanalysis.TheOnlyAnalyzerName, + Doc: goanalysis.TheOnlyanalyzerDoc, + Requires: []*analysis.Analyzer{buildssa.Analyzer}, + } + return goanalysis.NewLinter( + interfacerName, + "Linter that suggests narrower interface types", + []*analysis.Analyzer{analyzer}, + nil, + ).WithContextSetter(func(lintCtx *linter.Context) { + analyzer.Run = func(pass *analysis.Pass) (interface{}, error) { + ssa := pass.ResultOf[buildssa.Analyzer].(*buildssa.SSA) + ssaPkg := ssa.Pkg + c := &check.Checker{} + prog := goanalysis.MakeFakeLoaderProgram(pass) + c.Program(prog) + c.ProgramSSA(ssaPkg.Prog) -func (lint Interfacer) Run(ctx context.Context, lintCtx *linter.Context) ([]result.Issue, error) { - c := new(check.Checker) - c.Program(lintCtx.Program) - c.ProgramSSA(lintCtx.SSAProgram) + issues, err := c.Check() + if err != nil { + return nil, err + } + if len(issues) == 0 { + return nil, nil + } - issues, err := c.Check() - if err != nil { - return nil, err - } - if len(issues) == 0 { - return nil, nil - } + res := make([]result.Issue, 0, len(issues)) + for _, i := range issues { + pos := pass.Fset.Position(i.Pos()) + res = append(res, result.Issue{ + Pos: pos, + Text: i.Message(), + FromLinter: interfacerName, + }) + } - res := make([]result.Issue, 0, len(issues)) - for _, i := range issues { - pos := lintCtx.SSAProgram.Fset.Position(i.Pos()) - res = append(res, result.Issue{ - Pos: pos, - Text: i.Message(), - FromLinter: lint.Name(), - }) - } + mu.Lock() + resIssues = append(resIssues, res...) + mu.Unlock() - return res, nil + return nil, nil + } + }).WithIssuesReporter(func(*linter.Context) []result.Issue { + return resIssues + }).WithLoadMode(goanalysis.LoadModeTypesInfo) } diff --git a/pkg/golinters/lll.go b/pkg/golinters/lll.go index ad90830df4ad..182de66bcbf9 100644 --- a/pkg/golinters/lll.go +++ b/pkg/golinters/lll.go @@ -2,28 +2,21 @@ package golinters import ( "bufio" - "context" "fmt" "go/token" "os" "strings" + "sync" "unicode/utf8" + "golang.org/x/tools/go/analysis" + + "github.com/golangci/golangci-lint/pkg/golinters/goanalysis" "github.com/golangci/golangci-lint/pkg/lint/linter" "github.com/golangci/golangci-lint/pkg/result" ) -type Lll struct{} - -func (Lll) Name() string { - return "lll" -} - -func (Lll) Desc() string { - return "Reports long lines" -} - -func (lint Lll) getIssuesForFile(filename string, maxLineLen int, tabSpaces string) ([]result.Issue, error) { +func getLLLIssuesForFile(filename string, maxLineLen int, tabSpaces string) ([]result.Issue, error) { var res []result.Issue f, err := os.Open(filename) @@ -45,7 +38,7 @@ func (lint Lll) getIssuesForFile(filename string, maxLineLen int, tabSpaces stri Line: lineNumber, }, Text: fmt.Sprintf("line is %d characters", lineLen), - FromLinter: lint.Name(), + FromLinter: lllName, }) } lineNumber++ @@ -70,7 +63,7 @@ func (lint Lll) getIssuesForFile(filename string, maxLineLen int, tabSpaces stri Column: 1, }, Text: fmt.Sprintf("line is more than %d characters", bufio.MaxScanTokenSize), - FromLinter: lint.Name(), + FromLinter: lllName, }) } else { return nil, fmt.Errorf("can't scan file %s: %s", filename, err) @@ -80,16 +73,50 @@ func (lint Lll) getIssuesForFile(filename string, maxLineLen int, tabSpaces stri return res, nil } -func (lint Lll) Run(ctx context.Context, lintCtx *linter.Context) ([]result.Issue, error) { - var res []result.Issue - spaces := strings.Repeat(" ", lintCtx.Settings().Lll.TabWidth) - for _, f := range getAllFileNames(lintCtx) { - issues, err := lint.getIssuesForFile(f, lintCtx.Settings().Lll.LineLength, spaces) - if err != nil { - return nil, err - } - res = append(res, issues...) +const lllName = "lll" + +func NewLLL() *goanalysis.Linter { + var mu sync.Mutex + var resIssues []result.Issue + + analyzer := &analysis.Analyzer{ + Name: goanalysis.TheOnlyAnalyzerName, + Doc: goanalysis.TheOnlyanalyzerDoc, } + return goanalysis.NewLinter( + lllName, + "Reports long lines", + []*analysis.Analyzer{analyzer}, + nil, + ).WithContextSetter(func(lintCtx *linter.Context) { + analyzer.Run = func(pass *analysis.Pass) (interface{}, error) { + var fileNames []string + for _, f := range pass.Files { + pos := pass.Fset.Position(f.Pos()) + fileNames = append(fileNames, pos.Filename) + } - return res, nil + var res []result.Issue + spaces := strings.Repeat(" ", lintCtx.Settings().Lll.TabWidth) + for _, f := range fileNames { + issues, err := getLLLIssuesForFile(f, lintCtx.Settings().Lll.LineLength, spaces) + if err != nil { + return nil, err + } + res = append(res, issues...) + } + + if len(res) == 0 { + return nil, nil + } + + mu.Lock() + resIssues = append(resIssues, res...) + mu.Unlock() + + return nil, nil + } + }).WithIssuesReporter(func(*linter.Context) []result.Issue { + return resIssues + }).WithLoadMode(goanalysis.LoadModeSyntax) } diff --git a/pkg/golinters/maligned.go b/pkg/golinters/maligned.go index 624a7774b1c6..775ffec89641 100644 --- a/pkg/golinters/maligned.go +++ b/pkg/golinters/maligned.go @@ -1,42 +1,58 @@ package golinters import ( - "context" "fmt" + "sync" malignedAPI "github.com/golangci/maligned" + "golang.org/x/tools/go/analysis" + "github.com/golangci/golangci-lint/pkg/golinters/goanalysis" "github.com/golangci/golangci-lint/pkg/lint/linter" "github.com/golangci/golangci-lint/pkg/result" ) -type Maligned struct{} - -func (Maligned) Name() string { - return "maligned" -} - -func (Maligned) Desc() string { - return "Tool to detect Go structs that would take less memory if their fields were sorted" -} - -func (m Maligned) Run(ctx context.Context, lintCtx *linter.Context) ([]result.Issue, error) { - issues := malignedAPI.Run(lintCtx.Program) - if len(issues) == 0 { - return nil, nil +func NewMaligned() *goanalysis.Linter { + const linterName = "maligned" + var mu sync.Mutex + var res []result.Issue + analyzer := &analysis.Analyzer{ + Name: goanalysis.TheOnlyAnalyzerName, + Doc: goanalysis.TheOnlyanalyzerDoc, } - - res := make([]result.Issue, 0, len(issues)) - for _, i := range issues { - text := fmt.Sprintf("struct of size %d bytes could be of size %d bytes", i.OldSize, i.NewSize) - if lintCtx.Settings().Maligned.SuggestNewOrder { - text += fmt.Sprintf(":\n%s", formatCodeBlock(i.NewStructDef, lintCtx.Cfg)) + return goanalysis.NewLinter( + linterName, + "Tool to detect Go structs that would take less memory if their fields were sorted", + []*analysis.Analyzer{analyzer}, + nil, + ).WithContextSetter(func(lintCtx *linter.Context) { + analyzer.Run = func(pass *analysis.Pass) (interface{}, error) { + prog := goanalysis.MakeFakeLoaderProgram(pass) + + malignedIssues := malignedAPI.Run(prog) + if len(malignedIssues) == 0 { + return nil, nil + } + + issues := make([]result.Issue, 0, len(malignedIssues)) + for _, i := range malignedIssues { + text := fmt.Sprintf("struct of size %d bytes could be of size %d bytes", i.OldSize, i.NewSize) + if lintCtx.Settings().Maligned.SuggestNewOrder { + text += fmt.Sprintf(":\n%s", formatCodeBlock(i.NewStructDef, lintCtx.Cfg)) + } + issues = append(issues, result.Issue{ + Pos: i.Pos, + Text: text, + FromLinter: linterName, + }) + } + + mu.Lock() + res = append(res, issues...) + mu.Unlock() + return nil, nil } - res = append(res, result.Issue{ - Pos: i.Pos, - Text: text, - FromLinter: m.Name(), - }) - } - return res, nil + }).WithIssuesReporter(func(*linter.Context) []result.Issue { + return res + }).WithLoadMode(goanalysis.LoadModeTypesInfo) } diff --git a/pkg/golinters/megacheck.go b/pkg/golinters/megacheck.go index 9b1b74ba31ef..0c17456813ed 100644 --- a/pkg/golinters/megacheck.go +++ b/pkg/golinters/megacheck.go @@ -1,183 +1,16 @@ package golinters import ( - "context" "fmt" "github.com/golangci/golangci-lint/pkg/logutils" - "honnef.co/go/tools/unused" - - "honnef.co/go/tools/lint" - "golang.org/x/tools/go/analysis" - "honnef.co/go/tools/simple" - "honnef.co/go/tools/staticcheck" - "honnef.co/go/tools/stylecheck" - - "github.com/golangci/golangci-lint/pkg/golinters/goanalysis" - "github.com/golangci/golangci-lint/pkg/lint/linter" - "github.com/golangci/golangci-lint/pkg/result" -) - -const ( - MegacheckParentName = "megacheck" - MegacheckStaticcheckName = "staticcheck" - MegacheckUnusedName = "unused" - MegacheckGosimpleName = "gosimple" - MegacheckStylecheckName = "stylecheck" ) var debugf = logutils.Debug("megacheck") -type Staticcheck struct { - megacheck -} - -func NewStaticcheck() *Staticcheck { - return &Staticcheck{ - megacheck: megacheck{ - staticcheckEnabled: true, - }, - } -} - -func (Staticcheck) Name() string { return MegacheckStaticcheckName } -func (Staticcheck) Desc() string { - return "Staticcheck is a go vet on steroids, applying a ton of static analysis checks" -} - -type Gosimple struct { - megacheck -} - -func NewGosimple() *Gosimple { - return &Gosimple{ - megacheck: megacheck{ - gosimpleEnabled: true, - }, - } -} - -func (Gosimple) Name() string { return MegacheckGosimpleName } -func (Gosimple) Desc() string { - return "Linter for Go source code that specializes in simplifying a code" -} - -type Unused struct { - megacheck -} - -func NewUnused() *Unused { - return &Unused{ - megacheck: megacheck{ - unusedEnabled: true, - }, - } -} - -func (Unused) Name() string { return MegacheckUnusedName } -func (Unused) Desc() string { - return "Checks Go code for unused constants, variables, functions and types" -} - -type Stylecheck struct { - megacheck -} - -func NewStylecheck() *Stylecheck { - return &Stylecheck{ - megacheck: megacheck{ - stylecheckEnabled: true, - }, - } -} - -func (Stylecheck) Name() string { return MegacheckStylecheckName } -func (Stylecheck) Desc() string { return "Stylecheck is a replacement for golint" } - -type megacheck struct { - unusedEnabled bool - gosimpleEnabled bool - staticcheckEnabled bool - stylecheckEnabled bool -} - -func (megacheck) Name() string { - return MegacheckParentName -} - -func (megacheck) Desc() string { - return "" // shouldn't be called -} - -func (m *megacheck) enableChildLinter(name string) error { - switch name { - case MegacheckStaticcheckName: - m.staticcheckEnabled = true - case MegacheckGosimpleName: - m.gosimpleEnabled = true - case MegacheckUnusedName: - m.unusedEnabled = true - case MegacheckStylecheckName: - m.stylecheckEnabled = true - default: - return fmt.Errorf("invalid child linter name %s for metalinter %s", name, m.Name()) - } - - return nil -} - -type MegacheckMetalinter struct{} - -func (MegacheckMetalinter) Name() string { - return MegacheckParentName -} - -func (MegacheckMetalinter) BuildLinterConfig(enabledChildren []string) (*linter.Config, error) { - var m megacheck - for _, name := range enabledChildren { - if err := m.enableChildLinter(name); err != nil { - return nil, err - } - } - - // TODO: merge linter.Config and linter.Linter or refactor it in another way - lc := &linter.Config{ - Linter: m, - EnabledByDefault: false, - NeedsSSARepr: false, - InPresets: []string{linter.PresetStyle, linter.PresetBugs, linter.PresetUnused}, - Speed: 1, - AlternativeNames: nil, - OriginalURL: "", - ParentLinterName: "", - } - if m.unusedEnabled { - lc = lc.WithLoadDepsTypeInfo() - } else { - lc = lc.WithLoadForGoAnalysis() - } - return lc, nil -} - -func (MegacheckMetalinter) DefaultChildLinterNames() []string { - // no stylecheck here for backwards compatibility for users who enabled megacheck: don't enable extra - // linter for them - return []string{MegacheckStaticcheckName, MegacheckGosimpleName, MegacheckUnusedName} -} - -func (m MegacheckMetalinter) AllChildLinterNames() []string { - return append(m.DefaultChildLinterNames(), MegacheckStylecheckName) -} - -func (m megacheck) Run(ctx context.Context, lintCtx *linter.Context) ([]result.Issue, error) { - // Use OriginalPackages not Packages because `unused` doesn't work properly - // when we deduplicate normal and test packages. - return m.runMegacheck(ctx, lintCtx) -} - -func getAnalyzers(m map[string]*analysis.Analyzer) []*analysis.Analyzer { +func analyzersMapToSlice(m map[string]*analysis.Analyzer) []*analysis.Analyzer { var ret []*analysis.Analyzer for _, v := range m { ret = append(ret, v) @@ -185,7 +18,7 @@ func getAnalyzers(m map[string]*analysis.Analyzer) []*analysis.Analyzer { return ret } -func setGoVersion(analyzers []*analysis.Analyzer) { +func setAnalyzersGoVersion(analyzers []*analysis.Analyzer) { const goVersion = 13 // TODO for _, a := range analyzers { if v := a.Flags.Lookup("go"); v != nil { @@ -195,106 +28,3 @@ func setGoVersion(analyzers []*analysis.Analyzer) { } } } - -func (m megacheck) runMegacheck(ctx context.Context, lintCtx *linter.Context) ([]result.Issue, error) { - var linters []linter.Linter - - if m.gosimpleEnabled { - analyzers := getAnalyzers(simple.Analyzers) - setGoVersion(analyzers) - lnt := goanalysis.NewLinter(MegacheckGosimpleName, "", analyzers, nil) - linters = append(linters, lnt) - } - if m.staticcheckEnabled { - analyzers := getAnalyzers(staticcheck.Analyzers) - setGoVersion(analyzers) - lnt := goanalysis.NewLinter(MegacheckStaticcheckName, "", analyzers, nil) - linters = append(linters, lnt) - } - if m.stylecheckEnabled { - analyzers := getAnalyzers(stylecheck.Analyzers) - setGoVersion(analyzers) - lnt := goanalysis.NewLinter(MegacheckStylecheckName, "", analyzers, nil) - linters = append(linters, lnt) - } - - var u lint.CumulativeChecker - if m.unusedEnabled { - u = unused.NewChecker(lintCtx.Settings().Unused.CheckExported) - analyzers := []*analysis.Analyzer{u.Analyzer()} - setGoVersion(analyzers) - lnt := goanalysis.NewLinter(MegacheckUnusedName, "", analyzers, nil) - linters = append(linters, lnt) - } - - if len(linters) == 0 { - return nil, nil - } - - var issues []result.Issue - for _, lnt := range linters { - i, err := lnt.Run(ctx, lintCtx) - if err != nil { - return nil, err - } - issues = append(issues, i...) - } - - if u != nil { - for _, ur := range u.Result() { - p := u.ProblemObject(lintCtx.Packages[0].Fset, ur) - issues = append(issues, result.Issue{ - FromLinter: MegacheckUnusedName, - Text: p.Message, - Pos: p.Pos, - }) - } - } - - return issues, nil -} - -func (m megacheck) Analyzers() []*analysis.Analyzer { - if m.unusedEnabled { - // Don't treat this linter as go/analysis linter if unused is used - // because it has non-standard API. - return nil - } - - var allAnalyzers []*analysis.Analyzer - if m.gosimpleEnabled { - allAnalyzers = append(allAnalyzers, getAnalyzers(simple.Analyzers)...) - } - if m.staticcheckEnabled { - allAnalyzers = append(allAnalyzers, getAnalyzers(staticcheck.Analyzers)...) - } - if m.stylecheckEnabled { - allAnalyzers = append(allAnalyzers, getAnalyzers(stylecheck.Analyzers)...) - } - setGoVersion(allAnalyzers) - return allAnalyzers -} - -func (megacheck) Cfg() map[string]map[string]interface{} { - return nil -} - -func (m megacheck) AnalyzerToLinterNameMapping() map[*analysis.Analyzer]string { - ret := map[*analysis.Analyzer]string{} - if m.gosimpleEnabled { - for _, a := range simple.Analyzers { - ret[a] = MegacheckGosimpleName - } - } - if m.staticcheckEnabled { - for _, a := range staticcheck.Analyzers { - ret[a] = MegacheckStaticcheckName - } - } - if m.stylecheckEnabled { - for _, a := range stylecheck.Analyzers { - ret[a] = MegacheckStylecheckName - } - } - return ret -} diff --git a/pkg/golinters/misspell.go b/pkg/golinters/misspell.go index 413c20f84855..bcc9a608b8ca 100644 --- a/pkg/golinters/misspell.go +++ b/pkg/golinters/misspell.go @@ -1,69 +1,20 @@ package golinters import ( - "context" "fmt" "go/token" "strings" + "sync" "github.com/golangci/misspell" + "golang.org/x/tools/go/analysis" + "github.com/golangci/golangci-lint/pkg/golinters/goanalysis" "github.com/golangci/golangci-lint/pkg/lint/linter" "github.com/golangci/golangci-lint/pkg/result" ) -type Misspell struct{} - -func NewMisspell() *Misspell { - return &Misspell{} -} - -func (Misspell) Name() string { - return "misspell" -} - -func (Misspell) Desc() string { - return "Finds commonly misspelled English words in comments" -} - -func (lint Misspell) Run(ctx context.Context, lintCtx *linter.Context) ([]result.Issue, error) { - r := misspell.Replacer{ - Replacements: misspell.DictMain, - } - - // Figure out regional variations - settings := lintCtx.Settings().Misspell - locale := settings.Locale - switch strings.ToUpper(locale) { - case "": - // nothing - case "US": - r.AddRuleList(misspell.DictAmerican) - case "UK", "GB": - r.AddRuleList(misspell.DictBritish) - case "NZ", "AU", "CA": - return nil, fmt.Errorf("unknown locale: %q", locale) - } - - if len(settings.IgnoreWords) != 0 { - r.RemoveRule(settings.IgnoreWords) - } - - r.Compile() - - var res []result.Issue - for _, f := range getAllFileNames(lintCtx) { - issues, err := lint.runOnFile(f, &r, lintCtx) - if err != nil { - return nil, err - } - res = append(res, issues...) - } - - return res, nil -} - -func (lint Misspell) runOnFile(fileName string, r *misspell.Replacer, lintCtx *linter.Context) ([]result.Issue, error) { +func runMisspellOnFile(fileName string, r *misspell.Replacer, lintCtx *linter.Context) ([]result.Issue, error) { var res []result.Issue fileContent, err := lintCtx.FileCache.GetFileBytes(fileName) if err != nil { @@ -93,10 +44,87 @@ func (lint Misspell) runOnFile(fileName string, r *misspell.Replacer, lintCtx *l res = append(res, result.Issue{ Pos: pos, Text: text, - FromLinter: lint.Name(), + FromLinter: misspellName, Replacement: replacement, }) } return res, nil } + +const misspellName = "misspell" + +func NewMisspell() *goanalysis.Linter { + var mu sync.Mutex + var resIssues []result.Issue + var ruleErr error + + analyzer := &analysis.Analyzer{ + Name: goanalysis.TheOnlyAnalyzerName, + Doc: goanalysis.TheOnlyanalyzerDoc, + } + return goanalysis.NewLinter( + misspellName, + "Finds commonly misspelled English words in comments", + []*analysis.Analyzer{analyzer}, + nil, + ).WithContextSetter(func(lintCtx *linter.Context) { + r := misspell.Replacer{ + Replacements: misspell.DictMain, + } + + // Figure out regional variations + settings := lintCtx.Settings().Misspell + locale := settings.Locale + switch strings.ToUpper(locale) { + case "": + // nothing + case "US": + r.AddRuleList(misspell.DictAmerican) + case "UK", "GB": + r.AddRuleList(misspell.DictBritish) + case "NZ", "AU", "CA": + ruleErr = fmt.Errorf("unknown locale: %q", locale) + } + + if ruleErr == nil { + if len(settings.IgnoreWords) != 0 { + r.RemoveRule(settings.IgnoreWords) + } + + r.Compile() + } + + analyzer.Run = func(pass *analysis.Pass) (interface{}, error) { + if ruleErr != nil { + return nil, ruleErr + } + + var fileNames []string + for _, f := range pass.Files { + pos := pass.Fset.Position(f.Pos()) + fileNames = append(fileNames, pos.Filename) + } + + var res []result.Issue + for _, f := range fileNames { + issues, err := runMisspellOnFile(f, &r, lintCtx) + if err != nil { + return nil, err + } + res = append(res, issues...) + } + if len(res) == 0 { + return nil, nil + } + + mu.Lock() + resIssues = append(resIssues, res...) + mu.Unlock() + + return nil, nil + } + }).WithIssuesReporter(func(*linter.Context) []result.Issue { + return resIssues + }).WithLoadMode(goanalysis.LoadModeSyntax) +} diff --git a/pkg/golinters/nakedret.go b/pkg/golinters/nakedret.go index b935faa7ce28..2309ca14b06c 100644 --- a/pkg/golinters/nakedret.go +++ b/pkg/golinters/nakedret.go @@ -1,25 +1,18 @@ package golinters import ( - "context" "fmt" "go/ast" "go/token" + "sync" + "golang.org/x/tools/go/analysis" + + "github.com/golangci/golangci-lint/pkg/golinters/goanalysis" "github.com/golangci/golangci-lint/pkg/lint/linter" "github.com/golangci/golangci-lint/pkg/result" ) -type Nakedret struct{} - -func (Nakedret) Name() string { - return "nakedret" -} - -func (Nakedret) Desc() string { - return "Finds naked returns in functions greater than a specified function length" -} - type nakedretVisitor struct { maxLength int f *token.FileSet @@ -50,7 +43,7 @@ func (v *nakedretVisitor) processFuncDecl(funcDecl *ast.FuncDecl) { } v.issues = append(v.issues, result.Issue{ - FromLinter: Nakedret{}.Name(), + FromLinter: nakedretName, Text: fmt.Sprintf("naked return in func `%s` with %d lines of code", funcDecl.Name.Name, functionLineLength), Pos: v.f.Position(s.Pos()), @@ -85,16 +78,44 @@ func (v *nakedretVisitor) Visit(node ast.Node) ast.Visitor { return v } -func (lint Nakedret) Run(ctx context.Context, lintCtx *linter.Context) ([]result.Issue, error) { - var res []result.Issue - for _, f := range lintCtx.ASTCache.GetAllValidFiles() { - v := nakedretVisitor{ - maxLength: lintCtx.Settings().Nakedret.MaxFuncLines, - f: f.Fset, - } - ast.Walk(&v, f.F) - res = append(res, v.issues...) +const nakedretName = "nakedret" + +func NewNakedret() *goanalysis.Linter { + var mu sync.Mutex + var resIssues []result.Issue + + analyzer := &analysis.Analyzer{ + Name: goanalysis.TheOnlyAnalyzerName, + Doc: goanalysis.TheOnlyanalyzerDoc, } + return goanalysis.NewLinter( + nakedretName, + "Finds naked returns in functions greater than a specified function length", + []*analysis.Analyzer{analyzer}, + nil, + ).WithContextSetter(func(lintCtx *linter.Context) { + analyzer.Run = func(pass *analysis.Pass) (interface{}, error) { + var res []result.Issue + for _, file := range pass.Files { + v := nakedretVisitor{ + maxLength: lintCtx.Settings().Nakedret.MaxFuncLines, + f: pass.Fset, + } + ast.Walk(&v, file) + res = append(res, v.issues...) + } - return res, nil + if len(res) == 0 { + return nil, nil + } + + mu.Lock() + resIssues = append(resIssues, res...) + mu.Unlock() + + return nil, nil + } + }).WithIssuesReporter(func(*linter.Context) []result.Issue { + return resIssues + }).WithLoadMode(goanalysis.LoadModeSyntax) } diff --git a/pkg/golinters/prealloc.go b/pkg/golinters/prealloc.go index 6b2e032c1072..49deaf46a398 100644 --- a/pkg/golinters/prealloc.go +++ b/pkg/golinters/prealloc.go @@ -1,40 +1,57 @@ package golinters import ( - "context" "fmt" - "go/ast" + "sync" "github.com/golangci/prealloc" + "golang.org/x/tools/go/analysis" + "github.com/golangci/golangci-lint/pkg/golinters/goanalysis" "github.com/golangci/golangci-lint/pkg/lint/linter" "github.com/golangci/golangci-lint/pkg/result" ) -type Prealloc struct{} +const preallocName = "prealloc" -func (Prealloc) Name() string { - return "prealloc" -} - -func (Prealloc) Desc() string { - return "Finds slice declarations that could potentially be preallocated" -} +func NewPrealloc() *goanalysis.Linter { + var mu sync.Mutex + var resIssues []result.Issue -func (lint Prealloc) Run(ctx context.Context, lintCtx *linter.Context) ([]result.Issue, error) { - var res []result.Issue - - s := &lintCtx.Settings().Prealloc - for _, f := range lintCtx.ASTCache.GetAllValidFiles() { - hints := prealloc.Check([]*ast.File{f.F}, s.Simple, s.RangeLoops, s.ForLoops) - for _, hint := range hints { - res = append(res, result.Issue{ - Pos: f.Fset.Position(hint.Pos), - Text: fmt.Sprintf("Consider preallocating %s", formatCode(hint.DeclaredSliceName, lintCtx.Cfg)), - FromLinter: lint.Name(), - }) - } + analyzer := &analysis.Analyzer{ + Name: goanalysis.TheOnlyAnalyzerName, + Doc: goanalysis.TheOnlyanalyzerDoc, } - - return res, nil + return goanalysis.NewLinter( + preallocName, + "Finds slice declarations that could potentially be preallocated", + []*analysis.Analyzer{analyzer}, + nil, + ).WithContextSetter(func(lintCtx *linter.Context) { + s := &lintCtx.Settings().Prealloc + + analyzer.Run = func(pass *analysis.Pass) (interface{}, error) { + var res []result.Issue + hints := prealloc.Check(pass.Files, s.Simple, s.RangeLoops, s.ForLoops) + for _, hint := range hints { + res = append(res, result.Issue{ + Pos: pass.Fset.Position(hint.Pos), + Text: fmt.Sprintf("Consider preallocating %s", formatCode(hint.DeclaredSliceName, lintCtx.Cfg)), + FromLinter: preallocName, + }) + } + + if len(res) == 0 { + return nil, nil + } + + mu.Lock() + resIssues = append(resIssues, res...) + mu.Unlock() + + return nil, nil + } + }).WithIssuesReporter(func(*linter.Context) []result.Issue { + return resIssues + }).WithLoadMode(goanalysis.LoadModeSyntax) } diff --git a/pkg/golinters/scopelint.go b/pkg/golinters/scopelint.go index 38f86ee1605f..1544d48a8b7f 100644 --- a/pkg/golinters/scopelint.go +++ b/pkg/golinters/scopelint.go @@ -1,40 +1,60 @@ package golinters import ( - "context" "fmt" "go/ast" "go/token" + "sync" + "golang.org/x/tools/go/analysis" + + "github.com/golangci/golangci-lint/pkg/golinters/goanalysis" "github.com/golangci/golangci-lint/pkg/lint/linter" "github.com/golangci/golangci-lint/pkg/result" ) -type Scopelint struct{} +const scopelintName = "scopelint" -func (Scopelint) Name() string { - return "scopelint" -} +func NewScopelint() *goanalysis.Linter { + var mu sync.Mutex + var resIssues []result.Issue -func (Scopelint) Desc() string { - return "Scopelint checks for unpinned variables in go programs" -} + analyzer := &analysis.Analyzer{ + Name: goanalysis.TheOnlyAnalyzerName, + Doc: goanalysis.TheOnlyanalyzerDoc, + } + return goanalysis.NewLinter( + scopelintName, + "Scopelint checks for unpinned variables in go programs", + []*analysis.Analyzer{analyzer}, + nil, + ).WithContextSetter(func(lintCtx *linter.Context) { + analyzer.Run = func(pass *analysis.Pass) (interface{}, error) { + var res []result.Issue + for _, file := range pass.Files { + n := Node{ + fset: pass.Fset, + DangerObjects: map[*ast.Object]int{}, + UnsafeObjects: map[*ast.Object]int{}, + SkipFuncs: map[*ast.FuncLit]int{}, + issues: &res, + } + ast.Walk(&n, file) + } -func (lint Scopelint) Run(ctx context.Context, lintCtx *linter.Context) ([]result.Issue, error) { - var res []result.Issue + if len(res) == 0 { + return nil, nil + } - for _, f := range lintCtx.ASTCache.GetAllValidFiles() { - n := Node{ - fset: f.Fset, - DangerObjects: map[*ast.Object]int{}, - UnsafeObjects: map[*ast.Object]int{}, - SkipFuncs: map[*ast.FuncLit]int{}, - issues: &res, - } - ast.Walk(&n, f.F) - } + mu.Lock() + resIssues = append(resIssues, res...) + mu.Unlock() - return res, nil + return nil, nil + } + }).WithIssuesReporter(func(*linter.Context) []result.Issue { + return resIssues + }).WithLoadMode(goanalysis.LoadModeSyntax) } // The code below is copy-pasted from https://github.com/kyoh86/scopelint 92cbe2cc9276abda0e309f52cc9e309d407f174e @@ -151,6 +171,6 @@ func (f *Node) errorfAt(pos token.Position, format string, args ...interface{}) *f.issues = append(*f.issues, result.Issue{ Pos: pos, Text: fmt.Sprintf(format, args...), - FromLinter: Scopelint{}.Name(), + FromLinter: scopelintName, }) } diff --git a/pkg/golinters/staticcheck.go b/pkg/golinters/staticcheck.go new file mode 100644 index 000000000000..33aa921e6b01 --- /dev/null +++ b/pkg/golinters/staticcheck.go @@ -0,0 +1,19 @@ +package golinters + +import ( + "honnef.co/go/tools/staticcheck" + + "github.com/golangci/golangci-lint/pkg/golinters/goanalysis" +) + +func NewStaticcheck() *goanalysis.Linter { + analyzers := analyzersMapToSlice(staticcheck.Analyzers) + setAnalyzersGoVersion(analyzers) + + return goanalysis.NewLinter( + "staticcheck", + "Staticcheck is a go vet on steroids, applying a ton of static analysis checks", + analyzers, + nil, + ).WithLoadMode(goanalysis.LoadModeTypesInfo) +} diff --git a/pkg/golinters/structcheck.go b/pkg/golinters/structcheck.go index 0d845a26bbf2..a22916835703 100644 --- a/pkg/golinters/structcheck.go +++ b/pkg/golinters/structcheck.go @@ -1,38 +1,55 @@ package golinters // nolint:dupl import ( - "context" "fmt" + "sync" structcheckAPI "github.com/golangci/check/cmd/structcheck" + "golang.org/x/tools/go/analysis" + "github.com/golangci/golangci-lint/pkg/golinters/goanalysis" "github.com/golangci/golangci-lint/pkg/lint/linter" "github.com/golangci/golangci-lint/pkg/result" ) -type Structcheck struct{} - -func (Structcheck) Name() string { - return "structcheck" -} - -func (Structcheck) Desc() string { - return "Finds unused struct fields" -} - -func (s Structcheck) Run(ctx context.Context, lintCtx *linter.Context) ([]result.Issue, error) { - issues := structcheckAPI.Run(lintCtx.Program, lintCtx.Settings().Structcheck.CheckExportedFields) - if len(issues) == 0 { - return nil, nil - } - - res := make([]result.Issue, 0, len(issues)) - for _, i := range issues { - res = append(res, result.Issue{ - Pos: i.Pos, - Text: fmt.Sprintf("%s is unused", formatCode(i.FieldName, lintCtx.Cfg)), - FromLinter: s.Name(), - }) +func NewStructcheck() *goanalysis.Linter { + const linterName = "structcheck" + var mu sync.Mutex + var res []result.Issue + analyzer := &analysis.Analyzer{ + Name: goanalysis.TheOnlyAnalyzerName, + Doc: goanalysis.TheOnlyanalyzerDoc, } - return res, nil + return goanalysis.NewLinter( + linterName, + "Finds unused struct fields", + []*analysis.Analyzer{analyzer}, + nil, + ).WithContextSetter(func(lintCtx *linter.Context) { + checkExported := lintCtx.Settings().Structcheck.CheckExportedFields + analyzer.Run = func(pass *analysis.Pass) (interface{}, error) { + prog := goanalysis.MakeFakeLoaderProgram(pass) + + structcheckIssues := structcheckAPI.Run(prog, checkExported) + if len(structcheckIssues) == 0 { + return nil, nil + } + + issues := make([]result.Issue, 0, len(structcheckIssues)) + for _, i := range structcheckIssues { + issues = append(issues, result.Issue{ + Pos: i.Pos, + Text: fmt.Sprintf("%s is unused", formatCode(i.FieldName, lintCtx.Cfg)), + FromLinter: linterName, + }) + } + + mu.Lock() + res = append(res, issues...) + mu.Unlock() + return nil, nil + } + }).WithIssuesReporter(func(*linter.Context) []result.Issue { + return res + }).WithLoadMode(goanalysis.LoadModeTypesInfo) } diff --git a/pkg/golinters/stylecheck.go b/pkg/golinters/stylecheck.go new file mode 100644 index 000000000000..5a076951af7c --- /dev/null +++ b/pkg/golinters/stylecheck.go @@ -0,0 +1,19 @@ +package golinters + +import ( + "honnef.co/go/tools/stylecheck" + + "github.com/golangci/golangci-lint/pkg/golinters/goanalysis" +) + +func NewStylecheck() *goanalysis.Linter { + analyzers := analyzersMapToSlice(stylecheck.Analyzers) + setAnalyzersGoVersion(analyzers) + + return goanalysis.NewLinter( + "stylecheck", + "Stylecheck is a replacement for golint", + analyzers, + nil, + ).WithLoadMode(goanalysis.LoadModeTypesInfo) +} diff --git a/pkg/golinters/typecheck.go b/pkg/golinters/typecheck.go index fde1756b32ce..ece8e9acb161 100644 --- a/pkg/golinters/typecheck.go +++ b/pkg/golinters/typecheck.go @@ -1,57 +1,26 @@ package golinters import ( - "context" + "golang.org/x/tools/go/analysis" - "golang.org/x/tools/go/packages" - - "github.com/golangci/golangci-lint/pkg/lint/linter" - libpackages "github.com/golangci/golangci-lint/pkg/packages" - "github.com/golangci/golangci-lint/pkg/result" + "github.com/golangci/golangci-lint/pkg/golinters/goanalysis" ) -type TypeCheck struct{} - -func (TypeCheck) Name() string { - return "typecheck" -} - -func (TypeCheck) Desc() string { - return "Like the front-end of a Go compiler, parses and type-checks Go code" -} - -func (lint TypeCheck) parseError(srcErr packages.Error) (*result.Issue, error) { - pos, err := libpackages.ParseErrorPosition(srcErr.Pos) - if err != nil { - return nil, err +func NewTypecheck() *goanalysis.Linter { + const linterName = "typecheck" + analyzer := &analysis.Analyzer{ + Name: goanalysis.TheOnlyAnalyzerName, + Doc: goanalysis.TheOnlyanalyzerDoc, + Run: func(pass *analysis.Pass) (interface{}, error) { + return nil, nil + }, } - - return &result.Issue{ - Pos: *pos, - Text: srcErr.Msg, - FromLinter: lint.Name(), - }, nil -} - -func (lint TypeCheck) Run(ctx context.Context, lintCtx *linter.Context) ([]result.Issue, error) { - uniqReportedIssues := map[string]bool{} - - var res []result.Issue - for _, pkg := range lintCtx.NotCompilingPackages { - errors := libpackages.ExtractErrors(pkg, lintCtx.ASTCache) - for _, err := range errors { - i, perr := lint.parseError(err) - if perr != nil { // failed to parse - if uniqReportedIssues[err.Msg] { - continue - } - uniqReportedIssues[err.Msg] = true - lintCtx.Log.Errorf("typechecking error: %s", err.Msg) - } else { - res = append(res, *i) - } - } - } - - return res, nil + linter := goanalysis.NewLinter( + linterName, + "Like the front-end of a Go compiler, parses and type-checks Go code", + []*analysis.Analyzer{analyzer}, + nil, + ).WithLoadMode(goanalysis.LoadModeTypesInfo) + linter.SetTypecheckMode() + return linter } diff --git a/pkg/golinters/unconvert.go b/pkg/golinters/unconvert.go index 20ba45dc0325..04b69d3268db 100644 --- a/pkg/golinters/unconvert.go +++ b/pkg/golinters/unconvert.go @@ -1,38 +1,53 @@ package golinters import ( - "context" + "sync" unconvertAPI "github.com/golangci/unconvert" + "golang.org/x/tools/go/analysis" + "github.com/golangci/golangci-lint/pkg/golinters/goanalysis" "github.com/golangci/golangci-lint/pkg/lint/linter" "github.com/golangci/golangci-lint/pkg/result" ) -type Unconvert struct{} - -func (Unconvert) Name() string { - return "unconvert" -} - -func (Unconvert) Desc() string { - return "Remove unnecessary type conversions" -} - -func (lint Unconvert) Run(ctx context.Context, lintCtx *linter.Context) ([]result.Issue, error) { - positions := unconvertAPI.Run(lintCtx.Program) - if len(positions) == 0 { - return nil, nil - } - - res := make([]result.Issue, 0, len(positions)) - for _, pos := range positions { - res = append(res, result.Issue{ - Pos: pos, - Text: "unnecessary conversion", - FromLinter: lint.Name(), - }) +func NewUnconvert() *goanalysis.Linter { + const linterName = "unconvert" + var mu sync.Mutex + var res []result.Issue + analyzer := &analysis.Analyzer{ + Name: goanalysis.TheOnlyAnalyzerName, + Doc: goanalysis.TheOnlyanalyzerDoc, } - - return res, nil + return goanalysis.NewLinter( + linterName, + "Remove unnecessary type conversions", + []*analysis.Analyzer{analyzer}, + nil, + ).WithContextSetter(func(lintCtx *linter.Context) { + analyzer.Run = func(pass *analysis.Pass) (interface{}, error) { + prog := goanalysis.MakeFakeLoaderProgram(pass) + + positions := unconvertAPI.Run(prog) + if len(positions) == 0 { + return nil, nil + } + + issues := make([]result.Issue, 0, len(positions)) + for _, pos := range positions { + issues = append(issues, result.Issue{ + Pos: pos, + Text: "unnecessary conversion", + FromLinter: linterName, + }) + } + + mu.Lock() + res = append(res, issues...) + mu.Unlock() + return nil, nil + } + }).WithIssuesReporter(func(*linter.Context) []result.Issue { + return res + }).WithLoadMode(goanalysis.LoadModeTypesInfo) } diff --git a/pkg/golinters/unparam.go b/pkg/golinters/unparam.go index c45a79333c88..d89168edac78 100644 --- a/pkg/golinters/unparam.go +++ b/pkg/golinters/unparam.go @@ -1,49 +1,77 @@ package golinters import ( - "context" + "sync" + "golang.org/x/tools/go/packages" + + "golang.org/x/tools/go/analysis" + "golang.org/x/tools/go/analysis/passes/buildssa" "mvdan.cc/unparam/check" + "github.com/golangci/golangci-lint/pkg/golinters/goanalysis" "github.com/golangci/golangci-lint/pkg/lint/linter" "github.com/golangci/golangci-lint/pkg/result" ) -type Unparam struct{} +func NewUnparam() *goanalysis.Linter { + const linterName = "unparam" + var mu sync.Mutex + var resIssues []result.Issue -func (Unparam) Name() string { - return "unparam" -} + analyzer := &analysis.Analyzer{ + Name: goanalysis.TheOnlyAnalyzerName, + Doc: goanalysis.TheOnlyanalyzerDoc, + Requires: []*analysis.Analyzer{buildssa.Analyzer}, + } + return goanalysis.NewLinter( + linterName, + "Reports unused function parameters", + []*analysis.Analyzer{analyzer}, + nil, + ).WithContextSetter(func(lintCtx *linter.Context) { + us := &lintCtx.Settings().Unparam + if us.Algo != "cha" { + lintCtx.Log.Warnf("`linters-settings.unparam.algo` isn't supported by the newest `unparam`") + } -func (Unparam) Desc() string { - return "Reports unused function parameters" -} + analyzer.Run = func(pass *analysis.Pass) (interface{}, error) { + ssa := pass.ResultOf[buildssa.Analyzer].(*buildssa.SSA) + ssaPkg := ssa.Pkg -func (lint Unparam) Run(ctx context.Context, lintCtx *linter.Context) ([]result.Issue, error) { - us := &lintCtx.Settings().Unparam + pkg := &packages.Package{ + Fset: pass.Fset, + Syntax: pass.Files, + Types: pass.Pkg, + TypesInfo: pass.TypesInfo, + } - if us.Algo != "cha" { - lintCtx.Log.Warnf("`linters-settings.unparam.algo` isn't supported by the newest `unparam`") - } + c := &check.Checker{} + c.CheckExportedFuncs(us.CheckExported) + c.Packages([]*packages.Package{pkg}) + c.ProgramSSA(ssaPkg.Prog) - c := &check.Checker{} - c.CheckExportedFuncs(us.CheckExported) - c.Packages(lintCtx.Packages) - c.ProgramSSA(lintCtx.SSAProgram) + unparamIssues, err := c.Check() + if err != nil { + return nil, err + } - unparamIssues, err := c.Check() - if err != nil { - return nil, err - } + var res []result.Issue + for _, i := range unparamIssues { + res = append(res, result.Issue{ + Pos: pass.Fset.Position(i.Pos()), + Text: i.Message(), + FromLinter: linterName, + }) + } - var res []result.Issue - for _, i := range unparamIssues { - res = append(res, result.Issue{ - Pos: lintCtx.Program.Fset.Position(i.Pos()), - Text: i.Message(), - FromLinter: lint.Name(), - }) - } + mu.Lock() + resIssues = append(resIssues, res...) + mu.Unlock() - return res, nil + return nil, nil + } + }).WithIssuesReporter(func(*linter.Context) []result.Issue { + return resIssues + }).WithLoadMode(goanalysis.LoadModeTypesInfo) } diff --git a/pkg/golinters/unused.go b/pkg/golinters/unused.go new file mode 100644 index 000000000000..c580e0cf0dd9 --- /dev/null +++ b/pkg/golinters/unused.go @@ -0,0 +1,40 @@ +package golinters + +import ( + "golang.org/x/tools/go/analysis" + "honnef.co/go/tools/unused" + + "github.com/golangci/golangci-lint/pkg/lint/linter" + "github.com/golangci/golangci-lint/pkg/result" + + "github.com/golangci/golangci-lint/pkg/golinters/goanalysis" +) + +func NewUnused() *goanalysis.Linter { + u := unused.NewChecker(false) + analyzers := []*analysis.Analyzer{u.Analyzer()} + setAnalyzersGoVersion(analyzers) + + const name = "unused" + lnt := goanalysis.NewLinter( + name, + "Checks Go code for unused constants, variables, functions and types", + analyzers, + nil, + ).WithIssuesReporter(func(lintCtx *linter.Context) []result.Issue { + var issues []result.Issue + for _, ur := range u.Result() { + p := u.ProblemObject(lintCtx.Packages[0].Fset, ur) + issues = append(issues, result.Issue{ + FromLinter: name, + Text: p.Message, + Pos: p.Pos, + }) + } + return issues + }).WithContextSetter(func(lintCtx *linter.Context) { + u.WholeProgram = lintCtx.Settings().Unused.CheckExported + }).WithLoadMode(goanalysis.LoadModeWholeProgram) + lnt.UseOriginalPackages() + return lnt +} diff --git a/pkg/golinters/util.go b/pkg/golinters/util.go index ee2919c0132f..1940f30e3ffd 100644 --- a/pkg/golinters/util.go +++ b/pkg/golinters/util.go @@ -2,14 +2,9 @@ package golinters import ( "fmt" - "go/ast" - "go/token" "strings" - gopackages "golang.org/x/tools/go/packages" - "github.com/golangci/golangci-lint/pkg/config" - "github.com/golangci/golangci-lint/pkg/lint/linter" ) func formatCode(code string, _ *config.Config) string { @@ -27,38 +22,3 @@ func formatCodeBlock(code string, _ *config.Config) string { return fmt.Sprintf("```\n%s\n```", code) } - -func getAllFileNames(ctx *linter.Context) []string { - var ret []string - uniqFiles := map[string]bool{} // files are duplicated for test packages - for _, pkg := range ctx.Packages { - for _, f := range pkg.GoFiles { - if uniqFiles[f] { - continue - } - uniqFiles[f] = true - ret = append(ret, f) - } - } - return ret -} - -func getASTFilesForGoPkg(ctx *linter.Context, pkg *gopackages.Package) ([]*ast.File, *token.FileSet, error) { - var files []*ast.File - var fset *token.FileSet - for _, filename := range pkg.GoFiles { - f := ctx.ASTCache.Get(filename) - if f == nil { - return nil, nil, fmt.Errorf("no AST for file %s in cache: %+v", filename, *ctx.ASTCache) - } - - if f.Err != nil { - return nil, nil, fmt.Errorf("can't load AST for file %s: %s", f.Name, f.Err) - } - - files = append(files, f.F) - fset = f.Fset - } - - return files, fset, nil -} diff --git a/pkg/golinters/varcheck.go b/pkg/golinters/varcheck.go index 17e2eee69b48..e0ff0d67f7cc 100644 --- a/pkg/golinters/varcheck.go +++ b/pkg/golinters/varcheck.go @@ -1,38 +1,55 @@ package golinters // nolint:dupl import ( - "context" "fmt" + "sync" varcheckAPI "github.com/golangci/check/cmd/varcheck" + "golang.org/x/tools/go/analysis" + "github.com/golangci/golangci-lint/pkg/golinters/goanalysis" "github.com/golangci/golangci-lint/pkg/lint/linter" "github.com/golangci/golangci-lint/pkg/result" ) -type Varcheck struct{} - -func (Varcheck) Name() string { - return "varcheck" -} - -func (Varcheck) Desc() string { - return "Finds unused global variables and constants" -} - -func (v Varcheck) Run(ctx context.Context, lintCtx *linter.Context) ([]result.Issue, error) { - issues := varcheckAPI.Run(lintCtx.Program, lintCtx.Settings().Varcheck.CheckExportedFields) - if len(issues) == 0 { - return nil, nil - } - - res := make([]result.Issue, 0, len(issues)) - for _, i := range issues { - res = append(res, result.Issue{ - Pos: i.Pos, - Text: fmt.Sprintf("%s is unused", formatCode(i.VarName, lintCtx.Cfg)), - FromLinter: v.Name(), - }) +func NewVarcheck() *goanalysis.Linter { + const linterName = "varcheck" + var mu sync.Mutex + var res []result.Issue + analyzer := &analysis.Analyzer{ + Name: goanalysis.TheOnlyAnalyzerName, + Doc: goanalysis.TheOnlyanalyzerDoc, } - return res, nil + return goanalysis.NewLinter( + linterName, + "Finds unused global variables and constants", + []*analysis.Analyzer{analyzer}, + nil, + ).WithContextSetter(func(lintCtx *linter.Context) { + checkExported := lintCtx.Settings().Varcheck.CheckExportedFields + analyzer.Run = func(pass *analysis.Pass) (interface{}, error) { + prog := goanalysis.MakeFakeLoaderProgram(pass) + + varcheckIssues := varcheckAPI.Run(prog, checkExported) + if len(varcheckIssues) == 0 { + return nil, nil + } + + issues := make([]result.Issue, 0, len(varcheckIssues)) + for _, i := range varcheckIssues { + issues = append(issues, result.Issue{ + Pos: i.Pos, + Text: fmt.Sprintf("%s is unused", formatCode(i.VarName, lintCtx.Cfg)), + FromLinter: linterName, + }) + } + + mu.Lock() + res = append(res, issues...) + mu.Unlock() + return nil, nil + } + }).WithIssuesReporter(func(*linter.Context) []result.Issue { + return res + }).WithLoadMode(goanalysis.LoadModeTypesInfo) } diff --git a/pkg/golinters/whitespace.go b/pkg/golinters/whitespace.go index d549c5b46646..35af0a8768df 100644 --- a/pkg/golinters/whitespace.go +++ b/pkg/golinters/whitespace.go @@ -1,71 +1,85 @@ package golinters import ( - "context" "go/token" + "sync" "github.com/pkg/errors" + "golang.org/x/tools/go/analysis" + "github.com/golangci/golangci-lint/pkg/golinters/goanalysis" "github.com/golangci/golangci-lint/pkg/lint/linter" "github.com/golangci/golangci-lint/pkg/result" "github.com/ultraware/whitespace" ) -type Whitespace struct { -} - -func (Whitespace) Name() string { - return "whitespace" -} +func NewWhitespace() *goanalysis.Linter { + const linterName = "whitespace" + var mu sync.Mutex + var resIssues []result.Issue -func (Whitespace) Desc() string { - return "Tool for detection of leading and trailing whitespace" -} + analyzer := &analysis.Analyzer{ + Name: goanalysis.TheOnlyAnalyzerName, + Doc: goanalysis.TheOnlyanalyzerDoc, + } + return goanalysis.NewLinter( + linterName, + "Tool for detection of leading and trailing whitespace", + []*analysis.Analyzer{analyzer}, + nil, + ).WithContextSetter(func(lintCtx *linter.Context) { + settings := whitespace.Settings{MultiIf: lintCtx.Cfg.LintersSettings.Whitespace.MultiIf} -func (w Whitespace) Run(ctx context.Context, lintCtx *linter.Context) ([]result.Issue, error) { - settings := whitespace.Settings{MultiIf: lintCtx.Cfg.LintersSettings.Whitespace.MultiIf} + analyzer.Run = func(pass *analysis.Pass) (interface{}, error) { + var issues []whitespace.Message + for _, file := range pass.Files { + issues = append(issues, whitespace.Run(file, pass.Fset, settings)...) + } - var issues []whitespace.Message - for _, file := range lintCtx.ASTCache.GetAllValidFiles() { - issues = append(issues, whitespace.Run(file.F, file.Fset, settings)...) - } + if len(issues) == 0 { + return nil, nil + } - if len(issues) == 0 { - return nil, nil - } + res := make([]result.Issue, len(issues)) + for k, i := range issues { + issue := result.Issue{ + Pos: token.Position{ + Filename: i.Pos.Filename, + Line: i.Pos.Line, + }, + LineRange: &result.Range{From: i.Pos.Line, To: i.Pos.Line}, + Text: i.Message, + FromLinter: linterName, + Replacement: &result.Replacement{}, + } - res := make([]result.Issue, len(issues)) - for k, i := range issues { - issue := result.Issue{ - Pos: token.Position{ - Filename: i.Pos.Filename, - Line: i.Pos.Line, - }, - LineRange: &result.Range{From: i.Pos.Line, To: i.Pos.Line}, - Text: i.Message, - FromLinter: w.Name(), - Replacement: &result.Replacement{}, - } + bracketLine, err := lintCtx.LineCache.GetLine(issue.Pos.Filename, issue.Pos.Line) + if err != nil { + return nil, errors.Wrapf(err, "failed to get line %s:%d", issue.Pos.Filename, issue.Pos.Line) + } - bracketLine, err := lintCtx.LineCache.GetLine(issue.Pos.Filename, issue.Pos.Line) - if err != nil { - return nil, errors.Wrapf(err, "failed to get line %s:%d", issue.Pos.Filename, issue.Pos.Line) - } + switch i.Type { + case whitespace.MessageTypeLeading: + issue.LineRange.To++ // cover two lines by the issue: opening bracket "{" (issue.Pos.Line) and following empty line + case whitespace.MessageTypeTrailing: + issue.LineRange.From-- // cover two lines by the issue: closing bracket "}" (issue.Pos.Line) and preceding empty line + issue.Pos.Line-- // set in sync with LineRange.From to not break fixer and other code features + case whitespace.MessageTypeAddAfter: + bracketLine += "\n" + } + issue.Replacement.NewLines = []string{bracketLine} - switch i.Type { - case whitespace.MessageTypeLeading: - issue.LineRange.To++ // cover two lines by the issue: opening bracket "{" (issue.Pos.Line) and following empty line - case whitespace.MessageTypeTrailing: - issue.LineRange.From-- // cover two lines by the issue: closing bracket "}" (issue.Pos.Line) and preceding empty line - issue.Pos.Line-- // set in sync with LineRange.From to not break fixer and other code features - case whitespace.MessageTypeAddAfter: - bracketLine += "\n" - } - issue.Replacement.NewLines = []string{bracketLine} + res[k] = issue + } - res[k] = issue - } + mu.Lock() + resIssues = append(resIssues, res...) + mu.Unlock() - return res, nil + return nil, nil + } + }).WithIssuesReporter(func(*linter.Context) []result.Issue { + return resIssues + }).WithLoadMode(goanalysis.LoadModeSyntax) } diff --git a/pkg/lint/astcache/astcache.go b/pkg/lint/astcache/astcache.go index ef53c71685e0..06b5373bbbff 100644 --- a/pkg/lint/astcache/astcache.go +++ b/pkg/lint/astcache/astcache.go @@ -157,6 +157,7 @@ func (c *Cache) parseFile(filePath string, fset *token.FileSet) { Name: filePath, } if err != nil { - c.log.Warnf("Can't parse AST of %s: %s", filePath, err) + c.log.Infof("Can't parse AST of %s: %s", filePath, err) + // Info level because it will be reported by typecheck linter or go/analysis. } } diff --git a/pkg/lint/linter/config.go b/pkg/lint/linter/config.go index 2dfb8e3d2907..52ff9b90d434 100644 --- a/pkg/lint/linter/config.go +++ b/pkg/lint/linter/config.go @@ -19,16 +19,12 @@ type Config struct { LoadMode packages.LoadMode - NeedsSSARepr bool - InPresets []string - Speed int // more value means faster execution of linter AlternativeNames []string - OriginalURL string // URL of original (not forked) repo, needed for autogenerated README - ParentLinterName string // used only for megacheck's children now - CanAutoFix bool - IsSlow bool + OriginalURL string // URL of original (not forked) repo, needed for autogenerated README + CanAutoFix bool + IsSlow bool } func (lc *Config) ConsiderSlow() *Config { @@ -48,24 +44,6 @@ func (lc *Config) WithLoadFiles() *Config { func (lc *Config) WithLoadForGoAnalysis() *Config { lc = lc.WithLoadFiles() lc.LoadMode |= packages.NeedImports | packages.NeedDeps | packages.NeedExportsFile | packages.NeedTypesSizes - return lc.ConsiderSlow() -} - -func (lc *Config) WithLoadTypeInfo() *Config { - lc = lc.WithLoadFiles() - lc.LoadMode |= packages.NeedImports | packages.NeedTypes | packages.NeedTypesSizes | packages.NeedTypesInfo | packages.NeedSyntax - return lc -} - -func (lc *Config) WithLoadDepsTypeInfo() *Config { - lc = lc.WithLoadTypeInfo() - lc.LoadMode |= packages.NeedDeps - return lc -} - -func (lc *Config) WithSSA() *Config { - lc = lc.WithLoadDepsTypeInfo() - lc.NeedsSSARepr = true return lc } @@ -74,11 +52,6 @@ func (lc *Config) WithPresets(presets ...string) *Config { return lc } -func (lc *Config) WithSpeed(speed int) *Config { - lc.Speed = speed - return lc -} - func (lc *Config) WithURL(url string) *Config { lc.OriginalURL = url return lc @@ -89,20 +62,11 @@ func (lc *Config) WithAlternativeNames(names ...string) *Config { return lc } -func (lc *Config) WithParent(parentLinterName string) *Config { - lc.ParentLinterName = parentLinterName - return lc -} - func (lc *Config) WithAutoFix() *Config { lc.CanAutoFix = true return lc } -func (lc *Config) GetSpeed() int { - return lc.Speed -} - func (lc *Config) AllNames() []string { return append([]string{lc.Name()}, lc.AlternativeNames...) } diff --git a/pkg/lint/linter/context.go b/pkg/lint/linter/context.go index 3cf38b9791cc..a73a6551c285 100644 --- a/pkg/lint/linter/context.go +++ b/pkg/lint/linter/context.go @@ -1,9 +1,9 @@ package linter import ( - "golang.org/x/tools/go/loader" "golang.org/x/tools/go/packages" - "golang.org/x/tools/go/ssa" + + "github.com/golangci/golangci-lint/pkg/lint/astcache" "github.com/golangci/golangci-lint/pkg/golinters/goanalysis/load" @@ -12,7 +12,6 @@ import ( "github.com/golangci/golangci-lint/pkg/fsutils" "github.com/golangci/golangci-lint/pkg/config" - "github.com/golangci/golangci-lint/pkg/lint/astcache" "github.com/golangci/golangci-lint/pkg/logutils" ) @@ -24,22 +23,14 @@ type Context struct { // version for each of packages OriginalPackages []*packages.Package - NotCompilingPackages []*packages.Package - - LoaderConfig *loader.Config // deprecated, don't use for new linters - Program *loader.Program // deprecated, use Packages for new linters - - SSAProgram *ssa.Program // for unparam and interfacer but not for megacheck (it change it) - Cfg *config.Config - ASTCache *astcache.Cache FileCache *fsutils.FileCache LineCache *fsutils.LineCache Log logutils.Log - PkgCache *pkgcache.Cache - LoadGuard *load.Guard - NeedWholeProgram bool + PkgCache *pkgcache.Cache + LoadGuard *load.Guard + ASTCache *astcache.Cache } func (c *Context) Settings() *config.LintersSettings { diff --git a/pkg/lint/linter/metalinter.go b/pkg/lint/linter/metalinter.go deleted file mode 100644 index ee235640f2ee..000000000000 --- a/pkg/lint/linter/metalinter.go +++ /dev/null @@ -1,8 +0,0 @@ -package linter - -type MetaLinter interface { - Name() string - BuildLinterConfig(enabledChildren []string) (*Config, error) - AllChildLinterNames() []string - DefaultChildLinterNames() []string -} diff --git a/pkg/lint/lintersdb/enabled_set.go b/pkg/lint/lintersdb/enabled_set.go index 7767fee290c0..8e7caa6e8215 100644 --- a/pkg/lint/lintersdb/enabled_set.go +++ b/pkg/lint/lintersdb/enabled_set.go @@ -3,8 +3,6 @@ package lintersdb import ( "sort" - "golang.org/x/tools/go/analysis" - "github.com/golangci/golangci-lint/pkg/golinters/goanalysis" "github.com/golangci/golangci-lint/pkg/config" @@ -56,71 +54,30 @@ func (es EnabledSet) build(lcfg *config.Linters, enabledByDefaultLinters []*lint // It should be after --presets to be able to run only fast linters in preset. // It should be before --enable and --disable to be able to enable or disable specific linter. if lcfg.Fast { - for name := range resultLintersSet { - if es.m.GetLinterConfig(name).IsSlowLinter() { + for name, lc := range resultLintersSet { + if lc.IsSlowLinter() { delete(resultLintersSet, name) } } } - metaLinters := es.m.GetMetaLinters() - for _, name := range lcfg.Enable { - if metaLinter := metaLinters[name]; metaLinter != nil { - // e.g. if we use --enable=megacheck we should add staticcheck,unused and gosimple to result set - for _, childLinter := range metaLinter.DefaultChildLinterNames() { - resultLintersSet[childLinter] = es.m.GetLinterConfig(childLinter) - } - continue + for _, lc := range es.m.GetLinterConfigs(name) { + // it's important to use lc.Name() nor name because name can be alias + resultLintersSet[lc.Name()] = lc } - - lc := es.m.GetLinterConfig(name) - // it's important to use lc.Name() nor name because name can be alias - resultLintersSet[lc.Name()] = lc } for _, name := range lcfg.Disable { - if metaLinter := metaLinters[name]; metaLinter != nil { - // e.g. if we use --disable=megacheck we should remove staticcheck,unused and gosimple from result set - for _, childLinter := range metaLinter.DefaultChildLinterNames() { - delete(resultLintersSet, childLinter) - } - continue + for _, lc := range es.m.GetLinterConfigs(name) { + // it's important to use lc.Name() nor name because name can be alias + delete(resultLintersSet, lc.Name()) } - - lc := es.m.GetLinterConfig(name) - // it's important to use lc.Name() nor name because name can be alias - delete(resultLintersSet, lc.Name()) } return resultLintersSet } -func (es EnabledSet) optimizeLintersSet(linters map[string]*linter.Config) { - for _, metaLinter := range es.m.GetMetaLinters() { - var children []string - for _, child := range metaLinter.AllChildLinterNames() { - if _, ok := linters[child]; ok { - children = append(children, child) - } - } - - if len(children) <= 1 { - continue - } - - for _, child := range children { - delete(linters, child) - } - builtLinterConfig, err := metaLinter.BuildLinterConfig(children) - if err != nil { - panic("shouldn't fail during linter building: " + err.Error()) - } - linters[metaLinter.Name()] = builtLinterConfig - es.log.Infof("Optimized sublinters %s into metalinter %s", children, metaLinter.Name()) - } -} - func (es EnabledSet) Get(optimize bool) ([]*linter.Config, error) { if err := es.v.validateEnabledDisabledLintersConfig(&es.cfg.Linters); err != nil { return nil, err @@ -129,9 +86,8 @@ func (es EnabledSet) Get(optimize bool) ([]*linter.Config, error) { resultLintersSet := es.build(&es.cfg.Linters, es.m.GetAllEnabledByDefaultLinters()) es.verbosePrintLintersStatus(resultLintersSet) if optimize { - es.optimizeLintersSet(resultLintersSet) + es.combineGoAnalysisLinters(resultLintersSet) } - es.combineGoAnalysisLinters(resultLintersSet) var resultLinters []*linter.Config for _, lc := range resultLintersSet { @@ -144,25 +100,19 @@ func (es EnabledSet) Get(optimize bool) ([]*linter.Config, error) { func (es EnabledSet) combineGoAnalysisLinters(linters map[string]*linter.Config) { var goanalysisLinters []*goanalysis.Linter goanalysisPresets := map[string]bool{} - analyzerToLinterName := map[*analysis.Analyzer]string{} for _, linter := range linters { - lnt, ok := linter.Linter.(goanalysis.SupportedLinter) + lnt, ok := linter.Linter.(*goanalysis.Linter) if !ok { continue } - - analyzers := lnt.Analyzers() - if len(analyzers) == 0 { - continue // e.g. if "unused" is enabled + if lnt.LoadMode() == goanalysis.LoadModeWholeProgram { + // It's ineffective by CPU and memory to run whole-program and incremental analyzers at once. + continue } - gl := goanalysis.NewLinter(linter.Name(), "", analyzers, lnt.Cfg()) - goanalysisLinters = append(goanalysisLinters, gl) + goanalysisLinters = append(goanalysisLinters, lnt) for _, p := range linter.InPresets { goanalysisPresets[p] = true } - for a, name := range lnt.AnalyzerToLinterNameMapping() { - analyzerToLinterName[a] = name - } } if len(goanalysisLinters) <= 1 { @@ -174,7 +124,7 @@ func (es EnabledSet) combineGoAnalysisLinters(linters map[string]*linter.Config) delete(linters, lnt.Name()) } - ml := goanalysis.NewMetaLinter(goanalysisLinters, analyzerToLinterName) + ml := goanalysis.NewMetaLinter(goanalysisLinters) var presets []string for p := range goanalysisPresets { @@ -184,13 +134,11 @@ func (es EnabledSet) combineGoAnalysisLinters(linters map[string]*linter.Config) mlConfig := &linter.Config{ Linter: ml, EnabledByDefault: false, - NeedsSSARepr: false, InPresets: presets, - Speed: 5, AlternativeNames: nil, OriginalURL: "", - ParentLinterName: "", } + mlConfig = mlConfig.WithLoadForGoAnalysis() linters[ml.Name()] = mlConfig diff --git a/pkg/lint/lintersdb/enabled_set_test.go b/pkg/lint/lintersdb/enabled_set_test.go index fdde6bd54a37..b2eaf383d344 100644 --- a/pkg/lint/lintersdb/enabled_set_test.go +++ b/pkg/lint/lintersdb/enabled_set_test.go @@ -4,8 +4,6 @@ import ( "sort" "testing" - "github.com/golangci/golangci-lint/pkg/golinters" - "github.com/stretchr/testify/assert" "github.com/golangci/golangci-lint/pkg/config" @@ -20,35 +18,36 @@ 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{ - Disable: []string{golinters.MegacheckMetalinter{}.Name()}, + Disable: []string{"megacheck"}, }, name: "disable all linters from megacheck", - def: golinters.MegacheckMetalinter{}.DefaultChildLinterNames(), + def: allMegacheckLinterNames, exp: nil, // all disabled }, { cfg: config.Linters{ - Disable: []string{golinters.MegacheckStaticcheckName}, + Disable: []string{"staticcheck"}, }, name: "disable only staticcheck", - def: golinters.MegacheckMetalinter{}.DefaultChildLinterNames(), - exp: []string{golinters.MegacheckGosimpleName, golinters.MegacheckUnusedName}, + def: allMegacheckLinterNames, + exp: []string{"gosimple", "unused"}, }, { name: "don't merge into megacheck", - def: golinters.MegacheckMetalinter{}.DefaultChildLinterNames(), - exp: golinters.MegacheckMetalinter{}.DefaultChildLinterNames(), + def: allMegacheckLinterNames, + exp: allMegacheckLinterNames, }, { name: "expand megacheck", cfg: config.Linters{ - Enable: []string{golinters.MegacheckMetalinter{}.Name()}, + Enable: []string{"megacheck"}, }, def: nil, - exp: golinters.MegacheckMetalinter{}.DefaultChildLinterNames(), + exp: allMegacheckLinterNames, }, { name: "don't disable anything", @@ -99,9 +98,9 @@ func TestGetEnabledLintersSet(t *testing.T) { t.Run(c.name, func(t *testing.T) { var defaultLinters []*linter.Config for _, ln := range c.def { - lc := m.GetLinterConfig(ln) - assert.NotNil(t, lc, ln) - defaultLinters = append(defaultLinters, lc) + lcs := m.GetLinterConfigs(ln) + assert.NotNil(t, lcs, ln) + defaultLinters = append(defaultLinters, lcs...) } els := es.build(&c.cfg, defaultLinters) diff --git a/pkg/lint/lintersdb/manager.go b/pkg/lint/lintersdb/manager.go index 359c56747193..07ba69202f0a 100644 --- a/pkg/lint/lintersdb/manager.go +++ b/pkg/lint/lintersdb/manager.go @@ -10,20 +10,20 @@ import ( ) type Manager struct { - nameToLC map[string]*linter.Config - cfg *config.Config + nameToLCs map[string][]*linter.Config + cfg *config.Config } func NewManager(cfg *config.Config) *Manager { m := &Manager{cfg: cfg} - nameToLC := make(map[string]*linter.Config) + nameToLCs := make(map[string][]*linter.Config) for _, lc := range m.GetAllSupportedLinterConfigs() { for _, name := range lc.AllNames() { - nameToLC[name] = lc + nameToLCs[name] = append(nameToLCs[name], lc) } } - m.nameToLC = nameToLC + m.nameToLCs = nameToLCs return m } @@ -40,17 +40,8 @@ func (m Manager) allPresetsSet() map[string]bool { return ret } -func (m Manager) GetMetaLinter(name string) linter.MetaLinter { - return m.GetMetaLinters()[name] -} - -func (m Manager) GetLinterConfig(name string) *linter.Config { - lc, ok := m.nameToLC[name] - if !ok { - return nil - } - - return lc +func (m Manager) GetLinterConfigs(name string) []*linter.Config { + return m.nameToLCs[name] } func enableLinterConfigs(lcs []*linter.Config, isEnabled func(lc *linter.Config) bool) []*linter.Config { @@ -64,215 +55,169 @@ func enableLinterConfigs(lcs []*linter.Config, isEnabled func(lc *linter.Config) return ret } -func (Manager) GetMetaLinters() map[string]linter.MetaLinter { - metaLinters := []linter.MetaLinter{ - golinters.MegacheckMetalinter{}, - } - - ret := map[string]linter.MetaLinter{} - for _, metaLinter := range metaLinters { - ret[metaLinter.Name()] = metaLinter - } - - return ret -} - //nolint:funlen func (m Manager) GetAllSupportedLinterConfigs() []*linter.Config { var govetCfg *config.GovetSettings if m.cfg != nil { govetCfg = &m.cfg.LintersSettings.Govet } + const megacheckName = "megacheck" lcs := []*linter.Config{ linter.NewConfig(golinters.NewGovet(govetCfg)). WithLoadForGoAnalysis(). WithPresets(linter.PresetBugs). - WithSpeed(4). WithAlternativeNames("vet", "vetshadow"). WithURL("https://golang.org/cmd/vet/"), linter.NewConfig(golinters.NewBodyclose()). WithLoadForGoAnalysis(). WithPresets(linter.PresetPerformance, linter.PresetBugs). - WithSpeed(4). WithURL("https://github.com/timakin/bodyclose"), - linter.NewConfig(golinters.Errcheck{}). - WithLoadTypeInfo(). + linter.NewConfig(golinters.NewErrcheck()). + WithLoadForGoAnalysis(). WithPresets(linter.PresetBugs). - WithSpeed(10). WithURL("https://github.com/kisielk/errcheck"), - linter.NewConfig(golinters.Golint{}). + linter.NewConfig(golinters.NewGolint()). WithPresets(linter.PresetStyle). - WithSpeed(3). WithURL("https://github.com/golang/lint"), linter.NewConfig(golinters.NewStaticcheck()). WithLoadForGoAnalysis(). WithPresets(linter.PresetBugs). - WithSpeed(2). + WithAlternativeNames(megacheckName). WithURL("https://staticcheck.io/"), linter.NewConfig(golinters.NewUnused()). - WithLoadDepsTypeInfo(). + WithLoadForGoAnalysis(). WithPresets(linter.PresetUnused). - WithSpeed(5). + WithAlternativeNames(megacheckName). + ConsiderSlow(). WithURL("https://github.com/dominikh/go-tools/tree/master/unused"), linter.NewConfig(golinters.NewGosimple()). WithLoadForGoAnalysis(). WithPresets(linter.PresetStyle). - WithSpeed(5). + WithAlternativeNames(megacheckName). WithURL("https://github.com/dominikh/go-tools/tree/master/simple"), linter.NewConfig(golinters.NewStylecheck()). WithLoadForGoAnalysis(). WithPresets(linter.PresetStyle). - WithSpeed(5). WithURL("https://github.com/dominikh/go-tools/tree/master/stylecheck"), - linter.NewConfig(golinters.Gosec{}). - WithLoadTypeInfo(). + linter.NewConfig(golinters.NewGosec()). + WithLoadForGoAnalysis(). WithPresets(linter.PresetBugs). - WithSpeed(8). WithURL("https://github.com/securego/gosec"). WithAlternativeNames("gas"), - linter.NewConfig(golinters.Structcheck{}). - WithLoadTypeInfo(). + linter.NewConfig(golinters.NewStructcheck()). + WithLoadForGoAnalysis(). WithPresets(linter.PresetUnused). - WithSpeed(10). WithURL("https://github.com/opennota/check"), - linter.NewConfig(golinters.Varcheck{}). - WithLoadTypeInfo(). + linter.NewConfig(golinters.NewVarcheck()). + WithLoadForGoAnalysis(). WithPresets(linter.PresetUnused). - WithSpeed(10). WithURL("https://github.com/opennota/check"), - linter.NewConfig(golinters.Interfacer{}). - WithLoadDepsTypeInfo(). - WithSSA(). + linter.NewConfig(golinters.NewInterfacer()). + WithLoadForGoAnalysis(). WithPresets(linter.PresetStyle). - WithSpeed(6). WithURL("https://github.com/mvdan/interfacer"), - linter.NewConfig(golinters.Unconvert{}). - WithLoadTypeInfo(). + linter.NewConfig(golinters.NewUnconvert()). + WithLoadForGoAnalysis(). WithPresets(linter.PresetStyle). - WithSpeed(10). WithURL("https://github.com/mdempsky/unconvert"), - linter.NewConfig(golinters.Ineffassign{}). + linter.NewConfig(golinters.NewIneffassign()). WithPresets(linter.PresetUnused). - WithSpeed(9). WithURL("https://github.com/gordonklaus/ineffassign"), - linter.NewConfig(golinters.Dupl{}). + linter.NewConfig(golinters.NewDupl()). WithPresets(linter.PresetStyle). - WithSpeed(7). WithURL("https://github.com/mibk/dupl"), - linter.NewConfig(golinters.Goconst{}). + linter.NewConfig(golinters.NewGoconst()). WithPresets(linter.PresetStyle). - WithSpeed(9). WithURL("https://github.com/jgautheron/goconst"), - linter.NewConfig(golinters.Deadcode{}). - WithLoadTypeInfo(). + linter.NewConfig(golinters.NewDeadcode()). + WithLoadForGoAnalysis(). WithPresets(linter.PresetUnused). - WithSpeed(10). WithURL("https://github.com/remyoudompheng/go-misc/tree/master/deadcode"), - linter.NewConfig(golinters.Gocyclo{}). + linter.NewConfig(golinters.NewGocyclo()). WithPresets(linter.PresetComplexity). - WithSpeed(8). WithURL("https://github.com/alecthomas/gocyclo"), - linter.NewConfig(golinters.TypeCheck{}). - WithLoadTypeInfo(). + linter.NewConfig(golinters.NewTypecheck()). + WithLoadForGoAnalysis(). WithPresets(linter.PresetBugs). - WithSpeed(10). WithURL(""), - linter.NewConfig(golinters.Gofmt{}). + linter.NewConfig(golinters.NewGofmt()). WithPresets(linter.PresetFormatting). - WithSpeed(7). WithAutoFix(). WithURL("https://golang.org/cmd/gofmt/"), - linter.NewConfig(golinters.Gofmt{UseGoimports: true}). + linter.NewConfig(golinters.NewGoimports()). WithPresets(linter.PresetFormatting). - WithSpeed(5). WithAutoFix(). WithURL("https://godoc.org/golang.org/x/tools/cmd/goimports"), - linter.NewConfig(golinters.Maligned{}). - WithLoadTypeInfo(). + linter.NewConfig(golinters.NewMaligned()). + WithLoadForGoAnalysis(). WithPresets(linter.PresetPerformance). - WithSpeed(10). WithURL("https://github.com/mdempsky/maligned"), - linter.NewConfig(golinters.Depguard{}). - WithLoadTypeInfo(). + linter.NewConfig(golinters.NewDepguard()). + WithLoadForGoAnalysis(). WithPresets(linter.PresetStyle). - WithSpeed(6). WithURL("https://github.com/OpenPeeDeeP/depguard"), - linter.NewConfig(golinters.Misspell{}). + linter.NewConfig(golinters.NewMisspell()). WithPresets(linter.PresetStyle). - WithSpeed(7). WithAutoFix(). WithURL("https://github.com/client9/misspell"), - linter.NewConfig(golinters.Lll{}). + linter.NewConfig(golinters.NewLLL()). WithPresets(linter.PresetStyle). - WithSpeed(10). WithURL("https://github.com/walle/lll"), - linter.NewConfig(golinters.Unparam{}). + linter.NewConfig(golinters.NewUnparam()). WithPresets(linter.PresetUnused). - WithSpeed(3). - WithLoadDepsTypeInfo(). - WithSSA(). + WithLoadForGoAnalysis(). WithURL("https://github.com/mvdan/unparam"), - linter.NewConfig(golinters.Dogsled{}). + linter.NewConfig(golinters.NewDogsled()). WithPresets(linter.PresetStyle). - WithSpeed(10). WithURL("https://github.com/alexkohler/dogsled"), - linter.NewConfig(golinters.Nakedret{}). + linter.NewConfig(golinters.NewNakedret()). WithPresets(linter.PresetComplexity). - WithSpeed(10). WithURL("https://github.com/alexkohler/nakedret"), - linter.NewConfig(golinters.Prealloc{}). + linter.NewConfig(golinters.NewPrealloc()). WithPresets(linter.PresetPerformance). - WithSpeed(8). WithURL("https://github.com/alexkohler/prealloc"), - linter.NewConfig(golinters.Scopelint{}). + linter.NewConfig(golinters.NewScopelint()). WithPresets(linter.PresetBugs). - WithSpeed(8). WithURL("https://github.com/kyoh86/scopelint"), - linter.NewConfig(golinters.Gocritic{}). + linter.NewConfig(golinters.NewGocritic()). WithPresets(linter.PresetStyle). - WithSpeed(5). - WithLoadTypeInfo(). + WithLoadForGoAnalysis(). WithURL("https://github.com/go-critic/go-critic"), - linter.NewConfig(golinters.Gochecknoinits{}). + linter.NewConfig(golinters.NewGochecknoinits()). WithPresets(linter.PresetStyle). - WithSpeed(10). WithURL("https://github.com/leighmcculloch/gochecknoinits"), - linter.NewConfig(golinters.Gochecknoglobals{}). + linter.NewConfig(golinters.NewGochecknoglobals()). WithPresets(linter.PresetStyle). - WithSpeed(10). WithURL("https://github.com/leighmcculloch/gochecknoglobals"), - linter.NewConfig(golinters.Godox{}). + linter.NewConfig(golinters.NewGodox()). WithPresets(linter.PresetStyle). - WithSpeed(10). WithURL("https://github.com/matoous/godox"), - linter.NewConfig(golinters.Funlen{}). + linter.NewConfig(golinters.NewFunlen()). WithPresets(linter.PresetStyle). - WithSpeed(10). WithURL("https://github.com/ultraware/funlen"), - linter.NewConfig(golinters.Whitespace{}). + linter.NewConfig(golinters.NewWhitespace()). WithPresets(linter.PresetStyle). - WithSpeed(10). WithAutoFix(). WithURL("https://github.com/ultraware/whitespace"), } isLocalRun := os.Getenv("GOLANGCI_COM_RUN") == "" enabledByDefault := map[string]bool{ - golinters.NewGovet(nil).Name(): true, - golinters.Errcheck{}.Name(): true, - golinters.Staticcheck{}.Name(): true, - golinters.Unused{}.Name(): true, - golinters.Gosimple{}.Name(): true, - golinters.Structcheck{}.Name(): true, - golinters.Varcheck{}.Name(): true, - golinters.Ineffassign{}.Name(): true, - golinters.Deadcode{}.Name(): true, + golinters.NewGovet(nil).Name(): true, + golinters.NewErrcheck().Name(): true, + golinters.NewStaticcheck().Name(): true, + golinters.NewUnused().Name(): true, + golinters.NewGosimple().Name(): true, + golinters.NewStructcheck().Name(): true, + golinters.NewVarcheck().Name(): true, + golinters.NewIneffassign().Name(): true, + golinters.NewDeadcode().Name(): true, // don't typecheck for golangci.com: too many troubles - golinters.TypeCheck{}.Name(): isLocalRun, + golinters.NewTypecheck().Name(): isLocalRun, } return enableLinterConfigs(lcs, func(lc *linter.Config) bool { return enabledByDefault[lc.Name()] diff --git a/pkg/lint/lintersdb/validator.go b/pkg/lint/lintersdb/validator.go index 7e1f878897b7..d7e3699c8513 100644 --- a/pkg/lint/lintersdb/validator.go +++ b/pkg/lint/lintersdb/validator.go @@ -21,7 +21,7 @@ func (v Validator) validateLintersNames(cfg *config.Linters) error { allNames := append([]string{}, cfg.Enable...) allNames = append(allNames, cfg.Disable...) for _, name := range allNames { - if v.m.GetLinterConfig(name) == nil && v.m.GetMetaLinter(name) == nil { + if v.m.GetLinterConfigs(name) == nil { return fmt.Errorf("no such linter %q", name) } } diff --git a/pkg/lint/load.go b/pkg/lint/load.go index 1afff37e8155..979e37e7034c 100644 --- a/pkg/lint/load.go +++ b/pkg/lint/load.go @@ -5,7 +5,6 @@ import ( "fmt" "go/build" "go/token" - "go/types" "os" "path/filepath" "regexp" @@ -19,10 +18,7 @@ import ( "github.com/golangci/golangci-lint/pkg/fsutils" "github.com/pkg/errors" - "golang.org/x/tools/go/loader" "golang.org/x/tools/go/packages" - "golang.org/x/tools/go/ssa" - "golang.org/x/tools/go/ssa/ssautil" "github.com/golangci/golangci-lint/pkg/config" "github.com/golangci/golangci-lint/pkg/exitcodes" @@ -72,84 +68,6 @@ func (cl *ContextLoader) prepareBuildContext() { build.Default.BuildTags = cl.cfg.Run.BuildTags } -func (cl *ContextLoader) makeFakeLoaderPackageInfo(pkg *packages.Package) *loader.PackageInfo { - var errs []error - for _, err := range pkg.Errors { - errs = append(errs, err) - } - - typeInfo := &types.Info{} - if pkg.TypesInfo != nil { - typeInfo = pkg.TypesInfo - } - - return &loader.PackageInfo{ - Pkg: pkg.Types, - Importable: true, // not used - TransitivelyErrorFree: !pkg.IllTyped, - - // use compiled (preprocessed) go files AST; - // AST linters use not preprocessed go files AST - Files: pkg.Syntax, - Errors: errs, - Info: *typeInfo, - } -} - -func (cl *ContextLoader) makeFakeLoaderProgram(pkgs []*packages.Package) *loader.Program { - var createdPkgs []*loader.PackageInfo - for _, pkg := range pkgs { - if pkg.IllTyped { - // some linters crash on packages with errors, - // skip them and warn about them in another place - continue - } - - pkgInfo := cl.makeFakeLoaderPackageInfo(pkg) - createdPkgs = append(createdPkgs, pkgInfo) - } - - allPkgs := map[*types.Package]*loader.PackageInfo{} - for _, pkg := range createdPkgs { - pkg := pkg - allPkgs[pkg.Pkg] = pkg - } - for _, pkg := range pkgs { - if pkg.IllTyped { - // some linters crash on packages with errors, - // skip them and warn about them in another place - continue - } - - for _, impPkg := range pkg.Imports { - // don't use astcache for imported packages: we don't find issues in cgo imported deps - pkgInfo := cl.makeFakeLoaderPackageInfo(impPkg) - allPkgs[pkgInfo.Pkg] = pkgInfo - } - } - - return &loader.Program{ - Fset: pkgs[0].Fset, - Imported: nil, // not used without .Created in any linter - Created: createdPkgs, // all initial packages - AllPackages: allPkgs, // all initial packages and their depndencies - } -} - -func (cl *ContextLoader) buildSSAProgram(pkgs []*packages.Package) *ssa.Program { - startedAt := time.Now() - var pkgsBuiltDuration time.Duration - defer func() { - cl.log.Infof("SSA repr building timing: packages building %s, total %s", - pkgsBuiltDuration, time.Since(startedAt)) - }() - - ssaProg, _ := ssautil.Packages(pkgs, ssa.GlobalDebug) - pkgsBuiltDuration = time.Since(startedAt) - ssaProg.Build() - return ssaProg -} - func (cl *ContextLoader) findLoadMode(linters []*linter.Config) packages.LoadMode { loadMode := packages.LoadMode(0) for _, lc := range linters { @@ -305,13 +223,13 @@ func (cl *ContextLoader) loadPackages(ctx context.Context, loadMode packages.Loa return cl.filterTestMainPackages(pkgs), nil } -func (cl *ContextLoader) tryParseTestPackage(pkg *packages.Package) (name, testName string, isTest bool) { +func (cl *ContextLoader) tryParseTestPackage(pkg *packages.Package) (name string, isTest bool) { matches := cl.pkgTestIDRe.FindStringSubmatch(pkg.ID) if matches == nil { - return "", "", false + return "", false } - return matches[1], matches[2], true + return matches[1], true } func (cl *ContextLoader) filterTestMainPackages(pkgs []*packages.Package) []*packages.Package { @@ -332,7 +250,7 @@ func (cl *ContextLoader) filterTestMainPackages(pkgs []*packages.Package) []*pac func (cl *ContextLoader) filterDuplicatePackages(pkgs []*packages.Package) []*packages.Package { packagesWithTests := map[string]bool{} for _, pkg := range pkgs { - name, _, isTest := cl.tryParseTestPackage(pkg) + name, isTest := cl.tryParseTestPackage(pkg) if !isTest { continue } @@ -343,7 +261,7 @@ func (cl *ContextLoader) filterDuplicatePackages(pkgs []*packages.Package) []*pa var retPkgs []*packages.Package for _, pkg := range pkgs { - _, _, isTest := cl.tryParseTestPackage(pkg) + _, isTest := cl.tryParseTestPackage(pkg) if !isTest && packagesWithTests[pkg.PkgPath] { // If tests loading is enabled, // for package with files a.go and a_test.go go/packages loads two packages: @@ -361,15 +279,6 @@ func (cl *ContextLoader) filterDuplicatePackages(pkgs []*packages.Package) []*pa return retPkgs } -func needSSA(linters []*linter.Config) bool { - for _, lc := range linters { - if lc.NeedsSSARepr { - return true - } - } - return false -} - //nolint:gocyclo func (cl *ContextLoader) Load(ctx context.Context, linters []*linter.Config) (*linter.Context, error) { loadMode := cl.findLoadMode(linters) @@ -384,21 +293,13 @@ func (cl *ContextLoader) Load(ctx context.Context, linters []*linter.Config) (*l return nil, exitcodes.ErrNoGoFiles } - var prog *loader.Program - if loadMode&packages.NeedTypes != 0 { - prog = cl.makeFakeLoaderProgram(deduplicatedPkgs) - } - - var ssaProg *ssa.Program - if needSSA(linters) { - ssaProg = cl.buildSSAProgram(deduplicatedPkgs) - } - astLog := cl.log.Child("astcache") + startedLoadingASTAt := time.Now() astCache, err := astcache.LoadFromPackages(deduplicatedPkgs, astLog) if err != nil { return nil, err } + cl.log.Infof("Loaded %d AST files in %s", len(astCache.ParsedFilenames()), time.Since(startedLoadingASTAt)) ret := &linter.Context{ Packages: deduplicatedPkgs, @@ -407,50 +308,14 @@ func (cl *ContextLoader) Load(ctx context.Context, linters []*linter.Config) (*l // see https://github.com/golangci/golangci-lint/pull/585. OriginalPackages: pkgs, - Program: prog, - SSAProgram: ssaProg, - LoaderConfig: &loader.Config{ - Cwd: "", // used by depguard and fallbacked to os.Getcwd - Build: nil, // used by depguard and megacheck and fallbacked to build.Default - }, - Cfg: cl.cfg, - ASTCache: astCache, - Log: cl.log, - FileCache: cl.fileCache, - LineCache: cl.lineCache, - PkgCache: cl.pkgCache, - LoadGuard: cl.loadGuard, - NeedWholeProgram: loadMode&packages.NeedDeps != 0 && loadMode&packages.NeedTypesInfo != 0, + Cfg: cl.cfg, + ASTCache: astCache, + Log: cl.log, + FileCache: cl.fileCache, + LineCache: cl.lineCache, + PkgCache: cl.pkgCache, + LoadGuard: cl.loadGuard, } - separateNotCompilingPackages(ret) return ret, nil } - -// separateNotCompilingPackages moves not compiling packages into separate slice: -// a lot of linters crash on such packages -func separateNotCompilingPackages(lintCtx *linter.Context) { - // Separate deduplicated packages - goodPkgs := make([]*packages.Package, 0, len(lintCtx.Packages)) - for _, pkg := range lintCtx.Packages { - if pkg.IllTyped { - lintCtx.NotCompilingPackages = append(lintCtx.NotCompilingPackages, pkg) - } else { - goodPkgs = append(goodPkgs, pkg) - } - } - - lintCtx.Packages = goodPkgs - if len(lintCtx.NotCompilingPackages) != 0 { - lintCtx.Log.Infof("Packages that do not compile: %+v", lintCtx.NotCompilingPackages) - } - - // Separate original (not deduplicated) packages - goodOriginalPkgs := make([]*packages.Package, 0, len(lintCtx.OriginalPackages)) - for _, pkg := range lintCtx.OriginalPackages { - if !pkg.IllTyped { - goodOriginalPkgs = append(goodOriginalPkgs, pkg) - } - } - lintCtx.OriginalPackages = goodOriginalPkgs -} diff --git a/pkg/lint/runner.go b/pkg/lint/runner.go index 52a0f0fdfc9e..fa8fee0fb0dc 100644 --- a/pkg/lint/runner.go +++ b/pkg/lint/runner.go @@ -4,10 +4,7 @@ import ( "context" "fmt" "runtime/debug" - "sort" "strings" - "sync" - "time" "github.com/golangci/golangci-lint/internal/errorutil" "github.com/golangci/golangci-lint/pkg/lint/lintersdb" @@ -93,12 +90,6 @@ func NewRunner(astCache *astcache.Cache, cfg *config.Config, log logutils.Log, g }, nil } -type lintRes struct { - 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) { defer func() { @@ -127,148 +118,40 @@ func (r *Runner) runLinterSafe(ctx context.Context, lintCtx *linter.Context, return issues, nil } -func (r Runner) runWorker(ctx context.Context, lintCtx *linter.Context, - tasksCh <-chan *linter.Config, lintResultsCh chan<- lintRes, name string) { - sw := timeutils.NewStopwatch(name, r.Log) - defer sw.Print() - - for { - select { - case <-ctx.Done(): - return - case lc, ok := <-tasksCh: - if !ok { - return - } - if ctx.Err() != nil { - // XXX: if check it in only int a select - // it's possible to not enter to this case until tasksCh is empty. - return - } - var issues []result.Issue - var err error - sw.TrackStage(lc.Name(), func() { - issues, err = r.runLinterSafe(ctx, lintCtx, lc) - }) - lintResultsCh <- lintRes{ - linter: lc, - err: err, - issues: issues, - } - } - } -} - -func (r Runner) logWorkersStat(workersFinishTimes []time.Time) { - lastFinishTime := workersFinishTimes[0] - for _, t := range workersFinishTimes { - if t.After(lastFinishTime) { - lastFinishTime = t - } - } - - logStrings := []string{} - for i, t := range workersFinishTimes { - if t.Equal(lastFinishTime) { - continue - } - - logStrings = append(logStrings, fmt.Sprintf("#%d: %s", i+1, lastFinishTime.Sub(t))) - } - - r.Log.Infof("Workers idle times: %s", strings.Join(logStrings, ", ")) -} - -func getSortedLintersConfigs(linters []*linter.Config) []*linter.Config { - ret := make([]*linter.Config, len(linters)) - copy(ret, linters) - - sort.Slice(ret, func(i, j int) bool { - return ret[i].GetSpeed() < ret[j].GetSpeed() - }) - - return ret -} - -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 - - workersFinishTimes := make([]time.Time, lintCtx.Cfg.Run.Concurrency) - - for i := 0; i < lintCtx.Cfg.Run.Concurrency; i++ { - wg.Add(1) - go func(i int) { - defer wg.Done() - name := fmt.Sprintf("worker.%d", i+1) - r.runWorker(ctx, lintCtx, tasksCh, lintResultsCh, name) - workersFinishTimes[i] = time.Now() - }(i) - } - - lcs := getSortedLintersConfigs(linters) - for _, lc := range lcs { - tasksCh <- lc - } - close(tasksCh) - - go func() { - wg.Wait() - close(lintResultsCh) - - r.logWorkersStat(workersFinishTimes) - }() - - return lintResultsCh -} - type processorStat struct { inCount int outCount int } -func (r Runner) processLintResults(inCh <-chan lintRes) <-chan lintRes { - outCh := make(chan lintRes, 64) +func (r Runner) processLintResults(inIssues []result.Issue) []result.Issue { + sw := timeutils.NewStopwatch("processing", r.Log) - go func() { - sw := timeutils.NewStopwatch("processing", r.Log) + var issuesBefore, issuesAfter int + statPerProcessor := map[string]processorStat{} - var issuesBefore, issuesAfter int - statPerProcessor := map[string]processorStat{} - defer close(outCh) - - for res := range inCh { - if res.err != nil { - r.Log.Warnf("Can't run linter %s: %s", res.linter.Name(), res.err) - continue - } - - if len(res.issues) != 0 { - issuesBefore += len(res.issues) - res.issues = r.processIssues(res.issues, sw, statPerProcessor) - issuesAfter += len(res.issues) - outCh <- res - } - } + var outIssues []result.Issue + if len(inIssues) != 0 { + issuesBefore += len(inIssues) + outIssues = r.processIssues(inIssues, sw, statPerProcessor) + issuesAfter += len(outIssues) + } - // finalize processors: logging, clearing, no heavy work here + // finalize processors: logging, clearing, no heavy work here - for _, p := range r.Processors { - p := p - sw.TrackStage(p.Name(), func() { - p.Finish() - }) - } + for _, p := range r.Processors { + p := p + sw.TrackStage(p.Name(), func() { + p.Finish() + }) + } - if issuesBefore != issuesAfter { - r.Log.Infof("Issues before processing: %d, after processing: %d", issuesBefore, issuesAfter) - } - r.printPerProcessorStat(statPerProcessor) - sw.PrintStages() - }() + if issuesBefore != issuesAfter { + r.Log.Infof("Issues before processing: %d, after processing: %d", issuesBefore, issuesAfter) + } + r.printPerProcessorStat(statPerProcessor) + sw.PrintStages() - return outCh + return outIssues } func (r Runner) printPerProcessorStat(stat map[string]processorStat) { @@ -283,40 +166,24 @@ func (r Runner) printPerProcessorStat(stat map[string]processorStat) { } } -func collectIssues(resCh <-chan lintRes) <-chan result.Issue { - retIssues := make(chan result.Issue, 1024) - go func() { - defer close(retIssues) - - for res := range resCh { - if len(res.issues) == 0 { - continue - } +func (r Runner) Run(ctx context.Context, linters []*linter.Config, lintCtx *linter.Context) []result.Issue { + sw := timeutils.NewStopwatch("linters", r.Log) + defer sw.Print() - for _, i := range res.issues { - retIssues <- i + var issues []result.Issue + for _, lc := range linters { + lc := lc + sw.TrackStage(lc.Name(), func() { + linterIssues, err := r.runLinterSafe(ctx, lintCtx, lc) + if err != nil { + r.Log.Warnf("Can't run linter %s: %s", lc.Linter.Name(), err) + return } - } - }() - - return retIssues -} - -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 { - // XXX: always process issues, even if timeout occurred - finishedLintersN := 0 - for range processedLintResultsCh { - finishedLintersN++ - } - - r.Log.Errorf("%d/%d linters finished: deadline exceeded", - finishedLintersN, len(linters)) + issues = append(issues, linterIssues...) + }) } - return collectIssues(processedLintResultsCh) + return r.processLintResults(issues) } func (r *Runner) processIssues(issues []result.Issue, sw *timeutils.Stopwatch, statPerProcessor map[string]processorStat) []result.Issue { diff --git a/pkg/printers/checkstyle.go b/pkg/printers/checkstyle.go index 8853b6eef3af..ca7a6ebc0a6e 100644 --- a/pkg/printers/checkstyle.go +++ b/pkg/printers/checkstyle.go @@ -36,14 +36,14 @@ func NewCheckstyle() *Checkstyle { return &Checkstyle{} } -func (Checkstyle) Print(ctx context.Context, issues <-chan result.Issue) error { +func (Checkstyle) Print(ctx context.Context, issues []result.Issue) error { out := checkstyleOutput{ Version: "5.0", } files := map[string]*checkstyleFile{} - for issue := range issues { + for _, issue := range issues { file, ok := files[issue.FilePath()] if !ok { file = &checkstyleFile{ diff --git a/pkg/printers/codeclimate.go b/pkg/printers/codeclimate.go index 0faebe329fb4..e36a59348203 100644 --- a/pkg/printers/codeclimate.go +++ b/pkg/printers/codeclimate.go @@ -30,9 +30,9 @@ func NewCodeClimate() *CodeClimate { return &CodeClimate{} } -func (p CodeClimate) Print(ctx context.Context, issues <-chan result.Issue) error { +func (p CodeClimate) Print(ctx context.Context, issues []result.Issue) error { allIssues := []CodeClimateIssue{} - for i := range issues { + for _, i := range issues { var issue CodeClimateIssue issue.Description = i.FromLinter + ": " + i.Text issue.Location.Path = i.Pos.Filename diff --git a/pkg/printers/json.go b/pkg/printers/json.go index c3778fd3e5aa..6ffa996fb0e4 100644 --- a/pkg/printers/json.go +++ b/pkg/printers/json.go @@ -25,14 +25,9 @@ type JSONResult struct { Report *report.Data } -func (p JSON) Print(ctx context.Context, issues <-chan result.Issue) error { - allIssues := []result.Issue{} - for i := range issues { - allIssues = append(allIssues, i) - } - +func (p JSON) Print(ctx context.Context, issues []result.Issue) error { res := JSONResult{ - Issues: allIssues, + Issues: issues, Report: p.rd, } diff --git a/pkg/printers/junitxml.go b/pkg/printers/junitxml.go index 825c3fdbded3..46b14ef07791 100644 --- a/pkg/printers/junitxml.go +++ b/pkg/printers/junitxml.go @@ -38,10 +38,10 @@ func NewJunitXML() *JunitXML { return &JunitXML{} } -func (JunitXML) Print(ctx context.Context, issues <-chan result.Issue) error { +func (JunitXML) Print(ctx context.Context, issues []result.Issue) error { suites := make(map[string]testSuiteXML) // use a map to group by file - for i := range issues { + for _, i := range issues { suiteName := i.FilePath() testSuite := suites[suiteName] testSuite.Suite = i.FilePath() diff --git a/pkg/printers/printer.go b/pkg/printers/printer.go index a063bd91484a..bfafb88e2a79 100644 --- a/pkg/printers/printer.go +++ b/pkg/printers/printer.go @@ -7,5 +7,5 @@ import ( ) type Printer interface { - Print(ctx context.Context, issues <-chan result.Issue) error + Print(ctx context.Context, issues []result.Issue) error } diff --git a/pkg/printers/tab.go b/pkg/printers/tab.go index 5045d771c335..8d9974c8e031 100644 --- a/pkg/printers/tab.go +++ b/pkg/printers/tab.go @@ -29,10 +29,10 @@ func (p Tab) SprintfColored(ca color.Attribute, format string, args ...interface return c.Sprintf(format, args...) } -func (p *Tab) Print(ctx context.Context, issues <-chan result.Issue) error { +func (p *Tab) Print(ctx context.Context, issues []result.Issue) error { w := tabwriter.NewWriter(logutils.StdOut, 0, 0, 2, ' ', 0) - for i := range issues { + for _, i := range issues { i := i p.printIssue(&i, w) } diff --git a/pkg/printers/text.go b/pkg/printers/text.go index c0cc5e2c844f..772b58da3999 100644 --- a/pkg/printers/text.go +++ b/pkg/printers/text.go @@ -36,8 +36,8 @@ func (p Text) SprintfColored(ca color.Attribute, format string, args ...interfac return c.Sprintf(format, args...) } -func (p *Text) Print(ctx context.Context, issues <-chan result.Issue) error { - for i := range issues { +func (p *Text) Print(ctx context.Context, issues []result.Issue) error { + for _, i := range issues { i := i p.printIssue(&i) diff --git a/pkg/result/processors/fixer.go b/pkg/result/processors/fixer.go index 463724c19458..1e6c1fce053c 100644 --- a/pkg/result/processors/fixer.go +++ b/pkg/result/processors/fixer.go @@ -40,43 +40,37 @@ func (f Fixer) printStat() { f.sw.PrintStages() } -func (f Fixer) Process(issues <-chan result.Issue) <-chan result.Issue { +func (f Fixer) Process(issues []result.Issue) []result.Issue { if !f.cfg.Issues.NeedFix { return issues } - outCh := make(chan result.Issue, 1024) + outIssues := make([]result.Issue, 0, len(issues)) + issuesToFixPerFile := map[string][]result.Issue{} + for _, issue := range issues { + if issue.Replacement == nil { + outIssues = append(outIssues, issue) + continue + } - go func() { - issuesToFixPerFile := map[string][]result.Issue{} - for issue := range issues { - if issue.Replacement == nil { - outCh <- issue - continue - } + issuesToFixPerFile[issue.FilePath()] = append(issuesToFixPerFile[issue.FilePath()], issue) + } - issuesToFixPerFile[issue.FilePath()] = append(issuesToFixPerFile[issue.FilePath()], issue) - } + for file, issuesToFix := range issuesToFixPerFile { + var err error + f.sw.TrackStage("all", func() { + err = f.fixIssuesInFile(file, issuesToFix) //nolint:scopelint + }) + if err != nil { + f.log.Errorf("Failed to fix issues in file %s: %s", file, err) - for file, issuesToFix := range issuesToFixPerFile { - var err error - f.sw.TrackStage("all", func() { - err = f.fixIssuesInFile(file, issuesToFix) //nolint:scopelint - }) - if err != nil { - f.log.Errorf("Failed to fix issues in file %s: %s", file, err) - - // show issues only if can't fix them - for _, issue := range issuesToFix { - outCh <- issue - } - } + // show issues only if can't fix them + outIssues = append(outIssues, issuesToFix...) } - f.printStat() - close(outCh) - }() + } - return outCh + f.printStat() + return outIssues } func (f Fixer) fixIssuesInFile(filePath string, issues []result.Issue) error { diff --git a/pkg/result/processors/nolint.go b/pkg/result/processors/nolint.go index a2ad04643663..3345c76854b2 100644 --- a/pkg/result/processors/nolint.go +++ b/pkg/result/processors/nolint.go @@ -7,8 +7,6 @@ import ( "sort" "strings" - "github.com/pkg/errors" - "github.com/golangci/golangci-lint/pkg/lint/astcache" "github.com/golangci/golangci-lint/pkg/lint/lintersdb" "github.com/golangci/golangci-lint/pkg/logutils" @@ -95,7 +93,8 @@ func (p *Nolint) getOrCreateFileData(i *result.Issue) (*fileData, error) { i.FilePath(), p.astCache.ParsedFilenames()) } if file.Err != nil { - return nil, errors.Wrapf(file.Err, "can't parse file %s", i.FilePath()) + // Don't report error because it's already must be reporter by typecheck or go/analysis. + return fd, nil } fd.ignoredRanges = p.buildIgnoredRangesForFile(file.F, file.Fset, i.FilePath()) @@ -221,22 +220,17 @@ func (p *Nolint) extractInlineRangeFromComment(text string, g ast.Node, fset *to var gotUnknownLinters bool for _, linter := range linterItems { linterName := strings.ToLower(strings.TrimSpace(linter)) - metaLinter := p.dbManager.GetMetaLinter(linterName) - if metaLinter != nil { - // user can set metalinter name in nolint directive (e.g. megacheck), then - // we should add to nolint all the metalinter's default children - linters = append(linters, metaLinter.DefaultChildLinterNames()...) - continue - } - lc := p.dbManager.GetLinterConfig(linterName) - if lc == nil { + lcs := p.dbManager.GetLinterConfigs(linterName) + if lcs == nil { p.unknownLintersSet[linterName] = true gotUnknownLinters = true continue } - linters = append(linters, lc.Name()) // normalize name to work with aliases + for _, lc := range lcs { + linters = append(linters, lc.Name()) // normalize name to work with aliases + } } if gotUnknownLinters { diff --git a/test/enabled_linters_test.go b/test/enabled_linters_test.go index 47aad3b59b75..436d42b29b0d 100644 --- a/test/enabled_linters_test.go +++ b/test/enabled_linters_test.go @@ -164,8 +164,8 @@ func TestEnabledLinters(t *testing.T) { }, { name: "fast option combined with enable and enable-all", - args: "--enable-all --fast --enable=staticcheck", - el: getAllFastLintersWith("staticcheck"), + args: "--enable-all --fast --enable=unused", + el: getAllFastLintersWith("unused"), noImplicitFast: true, }, } diff --git a/vendor/github.com/golangci/gofmt/gofmt/gofmt.go b/vendor/github.com/golangci/gofmt/gofmt/gofmt.go index a6b09fcd7e39..fb9c8cb37f6d 100644 --- a/vendor/github.com/golangci/gofmt/gofmt/gofmt.go +++ b/vendor/github.com/golangci/gofmt/gofmt/gofmt.go @@ -21,6 +21,7 @@ import ( "runtime" "runtime/pprof" "strings" + "sync" ) var ( @@ -42,10 +43,11 @@ const ( ) var ( - fileSet = token.NewFileSet() // per process FileSet - exitCode = 0 - rewrite func(*ast.File) *ast.File - parserMode parser.Mode + fileSet = token.NewFileSet() // per process FileSet + exitCode = 0 + rewrite func(*ast.File) *ast.File + parserMode parser.Mode + parserModeInitOnce sync.Once ) func report(err error) { @@ -59,10 +61,12 @@ func usage() { } func initParserMode() { - parserMode = parser.ParseComments - if *allErrors { - parserMode |= parser.AllErrors - } + parserModeInitOnce.Do(func() { + parserMode = parser.ParseComments + if *allErrors { + parserMode |= parser.AllErrors + } + }) } func isGoFile(f os.FileInfo) bool { diff --git a/vendor/github.com/golangci/lint-1/lint.go b/vendor/github.com/golangci/lint-1/lint.go index 0b58e928bb8c..de63631a38dd 100644 --- a/vendor/github.com/golangci/lint-1/lint.go +++ b/vendor/github.com/golangci/lint-1/lint.go @@ -241,7 +241,6 @@ func (f *file) lint() { f.lintBlankImports() f.lintExported() f.lintNames() - f.lintVarDecls() f.lintElses() f.lintRanges() f.lintErrorf() @@ -471,11 +470,26 @@ func (f *file) lintPackageComment() { } } +func (f *file) isCgo() bool { + if f.src == nil { + return false + } + newLinePos := bytes.Index(f.src, []byte("\n")) + if newLinePos < 0 { + return false + } + firstLine := string(f.src[:newLinePos]) + + // files using cgo have implicitly added comment "Created by cgo - DO NOT EDIT" for go <= 1.10 + // and "Code generated by cmd/cgo" for go >= 1.11 + return strings.Contains(firstLine, "Created by cgo") || strings.Contains(firstLine, "Code generated by cmd/cgo") +} + // lintBlankImports complains if a non-main package has blank imports that are // not documented. func (f *file) lintBlankImports() { // In package main and in tests, we don't complain about blank imports. - if f.pkg.main || f.isTest() { + if f.pkg.main || f.isTest() || f.isCgo() { return } @@ -1020,84 +1034,6 @@ var zeroLiteral = map[string]bool{ "0i": true, } -// lintVarDecls examines variable declarations. It complains about declarations with -// redundant LHS types that can be inferred from the RHS. -func (f *file) lintVarDecls() { - var lastGen *ast.GenDecl // last GenDecl entered. - - f.walk(func(node ast.Node) bool { - switch v := node.(type) { - case *ast.GenDecl: - if v.Tok != token.CONST && v.Tok != token.VAR { - return false - } - lastGen = v - return true - case *ast.ValueSpec: - if lastGen.Tok == token.CONST { - return false - } - if len(v.Names) > 1 || v.Type == nil || len(v.Values) == 0 { - return false - } - rhs := v.Values[0] - // An underscore var appears in a common idiom for compile-time interface satisfaction, - // as in "var _ Interface = (*Concrete)(nil)". - if isIdent(v.Names[0], "_") { - return false - } - // If the RHS is a zero value, suggest dropping it. - zero := false - if lit, ok := rhs.(*ast.BasicLit); ok { - zero = zeroLiteral[lit.Value] - } else if isIdent(rhs, "nil") { - zero = true - } - if zero { - f.errorf(rhs, 0.9, category("zero-value"), "should drop = %s from declaration of var %s; it is the zero value", f.render(rhs), v.Names[0]) - return false - } - lhsTyp := f.pkg.typeOf(v.Type) - rhsTyp := f.pkg.typeOf(rhs) - - if !validType(lhsTyp) || !validType(rhsTyp) { - // Type checking failed (often due to missing imports). - return false - } - - if !types.Identical(lhsTyp, rhsTyp) { - // Assignment to a different type is not redundant. - return false - } - - // The next three conditions are for suppressing the warning in situations - // where we were unable to typecheck. - - // If the LHS type is an interface, don't warn, since it is probably a - // concrete type on the RHS. Note that our feeble lexical check here - // will only pick up interface{} and other literal interface types; - // that covers most of the cases we care to exclude right now. - if _, ok := v.Type.(*ast.InterfaceType); ok { - return false - } - // If the RHS is an untyped const, only warn if the LHS type is its default type. - if defType, ok := f.isUntypedConst(rhs); ok && !isIdent(v.Type, defType) { - return false - } - - f.errorf(v.Type, 0.8, category("type-inference"), "should omit type %s from declaration of var %s; it will be inferred from the right-hand side", f.render(v.Type), v.Names[0]) - return false - } - return true - }) -} - -func validType(T types.Type) bool { - return T != nil && - T != types.Typ[types.Invalid] && - !strings.Contains(T.String(), "invalid type") // good but not foolproof -} - // lintElses examines else blocks. It complains about any else block whose if block ends in a return. func (f *file) lintElses() { // We don't want to flag if { } else if { } else { } constructions. diff --git a/vendor/modules.txt b/vendor/modules.txt index 511b25d65990..5b232b93a5f7 100644 --- a/vendor/modules.txt +++ b/vendor/modules.txt @@ -63,12 +63,12 @@ github.com/golangci/go-misc/deadcode github.com/golangci/goconst # github.com/golangci/gocyclo v0.0.0-20180528134321-2becd97e67ee github.com/golangci/gocyclo/pkg/gocyclo -# github.com/golangci/gofmt v0.0.0-20181222123516-0b8337e80d98 +# github.com/golangci/gofmt v0.0.0-20190930125516-244bba706f1a github.com/golangci/gofmt/gofmt github.com/golangci/gofmt/goimports # github.com/golangci/ineffassign v0.0.0-20190609212857-42439a7714cc github.com/golangci/ineffassign -# github.com/golangci/lint-1 v0.0.0-20190420132249-ee948d087217 +# github.com/golangci/lint-1 v0.0.0-20190930103755-fad67e08aa89 github.com/golangci/lint-1 # github.com/golangci/maligned v0.0.0-20180506175553-b1d89398deca github.com/golangci/maligned