Skip to content

Commit

Permalink
Fix staticcheck panic on packages that do not compile
Browse files Browse the repository at this point in the history
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
  • Loading branch information
jirfag committed Apr 21, 2019
1 parent 09677d5 commit 39f46be
Show file tree
Hide file tree
Showing 4 changed files with 17 additions and 79 deletions.
4 changes: 4 additions & 0 deletions pkg/exitcodes/exitcodes.go
Original file line number Diff line number Diff line change
Expand Up @@ -24,4 +24,8 @@ var (
Message: "no go files to analyze",
Code: NoGoFiles,
}
ErrFailure = &ExitError{
Message: "failed to analyze",
Code: Failure,
}
)
63 changes: 0 additions & 63 deletions pkg/golinters/megacheck.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
)
Expand Down Expand Up @@ -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")
Expand Down
27 changes: 12 additions & 15 deletions pkg/lint/load.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -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)
}
}
}

Expand Down Expand Up @@ -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)
}
Expand Down
2 changes: 1 addition & 1 deletion test/run_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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\"`)
}

Expand Down

0 comments on commit 39f46be

Please sign in to comment.