Skip to content

Commit

Permalink
improve typecheck errors parsing
Browse files Browse the repository at this point in the history
  • Loading branch information
jirfag committed Nov 23, 2018
1 parent 55a18ae commit dba3907
Show file tree
Hide file tree
Showing 7 changed files with 70 additions and 40 deletions.
2 changes: 1 addition & 1 deletion pkg/golinters/megacheck.go
Original file line number Diff line number Diff line change
Expand Up @@ -88,7 +88,7 @@ func (m Megacheck) Run(ctx context.Context, lintCtx *linter.Context) ([]result.I
var errors []packages.Error
for _, p := range lintCtx.NotCompilingPackages {
errPkgs = append(errPkgs, p.String())
errors = append(errors, libpackages.ExtractErrors(p)...)
errors = append(errors, libpackages.ExtractErrors(p, lintCtx.ASTCache)...)
}

warnText := fmt.Sprintf("Can't run megacheck because of compilation errors in packages %s",
Expand Down
38 changes: 10 additions & 28 deletions pkg/golinters/typecheck.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,12 +2,7 @@ package golinters

import (
"context"
"fmt"
"go/token"
"strconv"
"strings"

"github.com/pkg/errors"
"golang.org/x/tools/go/packages"

"github.com/golangci/golangci-lint/pkg/lint/linter"
Expand All @@ -26,44 +21,31 @@ func (TypeCheck) Desc() string {
}

func (lint TypeCheck) parseError(srcErr packages.Error) (*result.Issue, error) {
// file:line(<optional>:colon)
parts := strings.Split(srcErr.Pos, ":")
if len(parts) == 1 {
return nil, errors.New("no colons")
}

file := parts[0]
line, err := strconv.Atoi(parts[1])
pos, err := libpackages.ParseErrorPosition(srcErr.Pos)
if err != nil {
return nil, fmt.Errorf("can't parse line number %q: %s", parts[1], err)
}

var column int
if len(parts) == 3 { // no column
column, err = strconv.Atoi(parts[2])
if err != nil {
return nil, errors.Wrapf(err, "failed to parse column from %q", parts[2])
}
return nil, err
}

return &result.Issue{
Pos: token.Position{
Filename: file,
Line: line,
Column: column,
},
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)
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)
Expand Down
2 changes: 1 addition & 1 deletion pkg/lint/load.go
Original file line number Diff line number Diff line change
Expand Up @@ -334,7 +334,7 @@ func (cl ContextLoader) Load(ctx context.Context, linters []linter.Config) (*lin
} else {
for _, pkg := range pkgs {
if pkg.IllTyped {
cl.log.Infof("Pkg %s errors: %v", pkg.ID, libpackages.ExtractErrors(pkg))
cl.log.Infof("Pkg %s errors: %v", pkg.ID, libpackages.ExtractErrors(pkg, astCache))
}
}
}
Expand Down
38 changes: 38 additions & 0 deletions pkg/packages/errors.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,38 @@
package packages

import (
"fmt"
"go/token"
"strconv"
"strings"

"github.com/pkg/errors"
)

func ParseErrorPosition(pos string) (*token.Position, error) {
// file:line(<optional>:colon)
parts := strings.Split(pos, ":")
if len(parts) == 1 {
return nil, errors.New("no colons")
}

file := parts[0]
line, err := strconv.Atoi(parts[1])
if err != nil {
return nil, fmt.Errorf("can't parse line number %q: %s", parts[1], err)
}

var column int
if len(parts) == 3 { // no column
column, err = strconv.Atoi(parts[2])
if err != nil {
return nil, errors.Wrapf(err, "failed to parse column from %q", parts[2])
}
}

return &token.Position{
Filename: file,
Line: line,
Column: column,
}, nil
}
22 changes: 13 additions & 9 deletions pkg/packages/util.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,10 +3,13 @@ package packages
import (
"fmt"

"github.com/golangci/golangci-lint/pkg/lint/astcache"

"golang.org/x/tools/go/packages"
)

func ExtractErrors(pkg *packages.Package) []packages.Error {
//nolint:gocyclo
func ExtractErrors(pkg *packages.Package, astCache *astcache.Cache) []packages.Error {
errors := extractErrorsImpl(pkg)
if len(errors) == 0 {
return errors
Expand All @@ -22,17 +25,18 @@ func ExtractErrors(pkg *packages.Package) []packages.Error {
uniqErrors = append(uniqErrors, err)
}

if len(pkg.Errors) == 0 && len(pkg.GoFiles) != 0 {
// erorrs were extracted from deps and have at leat one file in package
if len(pkg.GoFiles) != 0 {
// errors were extracted from deps and have at leat one file in package
for i := range uniqErrors {
// change pos to local file to properly process it by processors (properly read line etc)
uniqErrors[i].Msg = fmt.Sprintf("%s: %s", uniqErrors[i].Pos, uniqErrors[i].Msg)
uniqErrors[i].Pos = fmt.Sprintf("%s:1", pkg.GoFiles[0])
errPos, parseErr := ParseErrorPosition(uniqErrors[i].Pos)
if parseErr != nil || astCache.Get(errPos.Filename) == nil {
// change pos to local file to properly process it by processors (properly read line etc)
uniqErrors[i].Msg = fmt.Sprintf("%s: %s", uniqErrors[i].Pos, uniqErrors[i].Msg)
uniqErrors[i].Pos = fmt.Sprintf("%s:1", pkg.GoFiles[0])
}
}
}

// some errors like "code in directory expects import" don't have Pos, set it here
if len(pkg.GoFiles) != 0 {
// some errors like "code in directory expects import" don't have Pos, set it here
for i := range uniqErrors {
err := &uniqErrors[i]
if err.Pos == "" {
Expand Down
5 changes: 5 additions & 0 deletions pkg/result/processors/autogenerated_exclude.go
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,11 @@ func (p *AutogeneratedExclude) Process(issues []result.Issue) ([]result.Issue, e
}

func (p *AutogeneratedExclude) shouldPassIssue(i *result.Issue) (bool, error) {
if i.FromLinter == "typecheck" {
// don't hide typechecking errors in generated files: users expect to see why the project isn't compiling
return true, nil
}

fs, err := p.getOrCreateFileSummary(i)
if err != nil {
return false, err
Expand Down
3 changes: 2 additions & 1 deletion test/run_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,8 @@ func TestEmptyDirRun(t *testing.T) {

func TestNotExistingDirRun(t *testing.T) {
testshared.NewLintRunner(t).Run(getTestDataDir("no_such_dir")).
ExpectHasIssue(`cannot find package \"./testdata/no_such_dir\"`)
ExpectExitCode(exitcodes.WarningInTest).
ExpectOutputContains(`cannot find package \"./testdata/no_such_dir\"`)
}

func TestSymlinkLoop(t *testing.T) {
Expand Down

0 comments on commit dba3907

Please sign in to comment.