From 39f46be46090828834b920f59994c97767694238 Mon Sep 17 00:00:00 2001
From: Denis Isaev <denis@golangci.com>
Date: Sun, 21 Apr 2019 10:33:39 +0300
Subject: [PATCH] Fix staticcheck panic on packages that do not compile
The bug was introduced in golangci-lint when migrating staticcheck to go/packages.
Also, thanks to pkg.IllTyped we can analyze as max as we can by
staticcheck.
Relates: #418, #369, #429, #489
---
pkg/exitcodes/exitcodes.go | 4 +++
pkg/golinters/megacheck.go | 63 --------------------------------------
pkg/lint/load.go | 27 ++++++++--------
test/run_test.go | 2 +-
4 files changed, 17 insertions(+), 79 deletions(-)
diff --git a/pkg/exitcodes/exitcodes.go b/pkg/exitcodes/exitcodes.go
index 82bd2cdc..4a5f8d99 100644
--- a/pkg/exitcodes/exitcodes.go
+++ b/pkg/exitcodes/exitcodes.go
@@ -24,4 +24,8 @@ var (
Message: "no go files to analyze",
Code: NoGoFiles,
}
+ ErrFailure = &ExitError{
+ Message: "failed to analyze",
+ Code: Failure,
+ }
)
diff --git a/pkg/golinters/megacheck.go b/pkg/golinters/megacheck.go
index 86425341..14c66720 100644
--- a/pkg/golinters/megacheck.go
+++ b/pkg/golinters/megacheck.go
@@ -18,9 +18,6 @@ import (
"github.com/golangci/go-tools/unused"
"golang.org/x/tools/go/packages"
- "github.com/golangci/golangci-lint/pkg/fsutils"
- libpackages "github.com/golangci/golangci-lint/pkg/packages"
-
"github.com/golangci/golangci-lint/pkg/lint/linter"
"github.com/golangci/golangci-lint/pkg/result"
)
@@ -179,67 +176,7 @@ func (m MegacheckMetalinter) isValidChild(name string) bool {
return false
}
-func prettifyCompilationError(err packages.Error) error {
- i, _ := TypeCheck{}.parseError(err)
- if i == nil {
- return err
- }
-
- shortFilename, pathErr := fsutils.ShortestRelPath(i.Pos.Filename, "")
- if pathErr != nil {
- return err
- }
-
- errText := shortFilename
- if i.Line() != 0 {
- errText += fmt.Sprintf(":%d", i.Line())
- }
- errText += fmt.Sprintf(": %s", i.Text)
- return errors.New(errText)
-}
-
-func (m megacheck) canAnalyze(lintCtx *linter.Context) bool {
- if len(lintCtx.NotCompilingPackages) == 0 {
- return true
- }
-
- var errPkgs []string
- var errs []packages.Error
- for _, p := range lintCtx.NotCompilingPackages {
- if p.Name == "main" {
- // megacheck crashes on not compiling packages but main packages
- // aren't reachable by megacheck: other packages can't depend on them.
- continue
- }
-
- errPkgs = append(errPkgs, p.String())
- errs = append(errs, libpackages.ExtractErrors(p, lintCtx.ASTCache)...)
- }
-
- if len(errPkgs) == 0 { // only main packages do not compile
- return true
- }
-
- // TODO: print real linter names in this message
- warnText := fmt.Sprintf("Can't run megacheck because of compilation errors in packages %s", errPkgs)
- if len(errs) != 0 {
- warnText += fmt.Sprintf(": %s", prettifyCompilationError(errs[0]))
- if len(errs) > 1 {
- const runCmd = "golangci-lint run --no-config --disable-all -E typecheck"
- warnText += fmt.Sprintf(" and %d more errors: run `%s` to see all errors", len(errs)-1, runCmd)
- }
- }
- lintCtx.Log.Warnf("%s", warnText)
-
- // megacheck crashes if there are not compiling packages
- return false
-}
-
func (m megacheck) Run(ctx context.Context, lintCtx *linter.Context) ([]result.Issue, error) {
- if !m.canAnalyze(lintCtx) {
- return nil, nil
- }
-
issues, err := m.runMegacheck(lintCtx.Packages, lintCtx.Settings().Unused.CheckExported)
if err != nil {
return nil, errors.Wrap(err, "failed to run megacheck")
diff --git a/pkg/lint/load.go b/pkg/lint/load.go
index 1f704ad5..fa041509 100644
--- a/pkg/lint/load.go
+++ b/pkg/lint/load.go
@@ -23,7 +23,6 @@ import (
"github.com/golangci/golangci-lint/pkg/lint/astcache"
"github.com/golangci/golangci-lint/pkg/lint/linter"
"github.com/golangci/golangci-lint/pkg/logutils"
- libpackages "github.com/golangci/golangci-lint/pkg/packages"
)
type ContextLoader struct {
@@ -265,6 +264,10 @@ func (cl ContextLoader) loadPackages(ctx context.Context, loadMode packages.Load
if strings.Contains(err.Msg, "no Go files") {
return nil, errors.Wrapf(exitcodes.ErrNoGoFiles, "package %s", pkg.PkgPath)
}
+ if strings.Contains(err.Msg, "cannot find package") {
+ // when analyzing not existing directory
+ return nil, errors.Wrap(exitcodes.ErrFailure, err.Msg)
+ }
}
}
@@ -358,29 +361,23 @@ func (cl ContextLoader) Load(ctx context.Context, linters []*linter.Config) (*li
Log: cl.log,
}
- if prog != nil {
- saveNotCompilingPackages(ret)
- } else {
- for _, pkg := range pkgs {
- if pkg.IllTyped {
- cl.log.Infof("Pkg %s errors: %v", pkg.ID, libpackages.ExtractErrors(pkg, astCache))
- }
- }
- }
-
+ separateNotCompilingPackages(ret)
return ret, nil
}
-// saveNotCompilingPackages saves not compiling packages into separate slice:
-// a lot of linters crash on such packages. Leave them only for those linters
-// which can work with them.
-func saveNotCompilingPackages(lintCtx *linter.Context) {
+// separateNotCompilingPackages moves not compiling packages into separate slice:
+// a lot of linters crash on such packages
+func separateNotCompilingPackages(lintCtx *linter.Context) {
+ 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)
}
diff --git a/test/run_test.go b/test/run_test.go
index 57e997e1..a191341d 100644
--- a/test/run_test.go
+++ b/test/run_test.go
@@ -32,7 +32,7 @@ func TestEmptyDirRun(t *testing.T) {
func TestNotExistingDirRun(t *testing.T) {
testshared.NewLintRunner(t).Run(getTestDataDir("no_such_dir")).
- ExpectExitCode(exitcodes.WarningInTest).
+ ExpectExitCode(exitcodes.Failure).
ExpectOutputContains(`cannot find package \"./testdata/no_such_dir\"`)
}