Skip to content

Commit dba3907

Browse files
committed
improve typecheck errors parsing
1 parent 55a18ae commit dba3907

File tree

7 files changed

+70
-40
lines changed

7 files changed

+70
-40
lines changed

pkg/golinters/megacheck.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -88,7 +88,7 @@ func (m Megacheck) Run(ctx context.Context, lintCtx *linter.Context) ([]result.I
8888
var errors []packages.Error
8989
for _, p := range lintCtx.NotCompilingPackages {
9090
errPkgs = append(errPkgs, p.String())
91-
errors = append(errors, libpackages.ExtractErrors(p)...)
91+
errors = append(errors, libpackages.ExtractErrors(p, lintCtx.ASTCache)...)
9292
}
9393

9494
warnText := fmt.Sprintf("Can't run megacheck because of compilation errors in packages %s",

pkg/golinters/typecheck.go

Lines changed: 10 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -2,12 +2,7 @@ package golinters
22

33
import (
44
"context"
5-
"fmt"
6-
"go/token"
7-
"strconv"
8-
"strings"
95

10-
"github.com/pkg/errors"
116
"golang.org/x/tools/go/packages"
127

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

2823
func (lint TypeCheck) parseError(srcErr packages.Error) (*result.Issue, error) {
29-
// file:line(<optional>:colon)
30-
parts := strings.Split(srcErr.Pos, ":")
31-
if len(parts) == 1 {
32-
return nil, errors.New("no colons")
33-
}
34-
35-
file := parts[0]
36-
line, err := strconv.Atoi(parts[1])
24+
pos, err := libpackages.ParseErrorPosition(srcErr.Pos)
3725
if err != nil {
38-
return nil, fmt.Errorf("can't parse line number %q: %s", parts[1], err)
39-
}
40-
41-
var column int
42-
if len(parts) == 3 { // no column
43-
column, err = strconv.Atoi(parts[2])
44-
if err != nil {
45-
return nil, errors.Wrapf(err, "failed to parse column from %q", parts[2])
46-
}
26+
return nil, err
4727
}
4828

4929
return &result.Issue{
50-
Pos: token.Position{
51-
Filename: file,
52-
Line: line,
53-
Column: column,
54-
},
30+
Pos: *pos,
5531
Text: srcErr.Msg,
5632
FromLinter: lint.Name(),
5733
}, nil
5834
}
5935

6036
func (lint TypeCheck) Run(ctx context.Context, lintCtx *linter.Context) ([]result.Issue, error) {
37+
uniqReportedIssues := map[string]bool{}
38+
6139
var res []result.Issue
6240
for _, pkg := range lintCtx.NotCompilingPackages {
63-
errors := libpackages.ExtractErrors(pkg)
41+
errors := libpackages.ExtractErrors(pkg, lintCtx.ASTCache)
6442
for _, err := range errors {
6543
i, perr := lint.parseError(err)
6644
if perr != nil { // failed to parse
45+
if uniqReportedIssues[err.Msg] {
46+
continue
47+
}
48+
uniqReportedIssues[err.Msg] = true
6749
lintCtx.Log.Errorf("typechecking error: %s", err.Msg)
6850
} else {
6951
res = append(res, *i)

pkg/lint/load.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -334,7 +334,7 @@ func (cl ContextLoader) Load(ctx context.Context, linters []linter.Config) (*lin
334334
} else {
335335
for _, pkg := range pkgs {
336336
if pkg.IllTyped {
337-
cl.log.Infof("Pkg %s errors: %v", pkg.ID, libpackages.ExtractErrors(pkg))
337+
cl.log.Infof("Pkg %s errors: %v", pkg.ID, libpackages.ExtractErrors(pkg, astCache))
338338
}
339339
}
340340
}

pkg/packages/errors.go

Lines changed: 38 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,38 @@
1+
package packages
2+
3+
import (
4+
"fmt"
5+
"go/token"
6+
"strconv"
7+
"strings"
8+
9+
"github.com/pkg/errors"
10+
)
11+
12+
func ParseErrorPosition(pos string) (*token.Position, error) {
13+
// file:line(<optional>:colon)
14+
parts := strings.Split(pos, ":")
15+
if len(parts) == 1 {
16+
return nil, errors.New("no colons")
17+
}
18+
19+
file := parts[0]
20+
line, err := strconv.Atoi(parts[1])
21+
if err != nil {
22+
return nil, fmt.Errorf("can't parse line number %q: %s", parts[1], err)
23+
}
24+
25+
var column int
26+
if len(parts) == 3 { // no column
27+
column, err = strconv.Atoi(parts[2])
28+
if err != nil {
29+
return nil, errors.Wrapf(err, "failed to parse column from %q", parts[2])
30+
}
31+
}
32+
33+
return &token.Position{
34+
Filename: file,
35+
Line: line,
36+
Column: column,
37+
}, nil
38+
}

pkg/packages/util.go

Lines changed: 13 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -3,10 +3,13 @@ package packages
33
import (
44
"fmt"
55

6+
"github.com/golangci/golangci-lint/pkg/lint/astcache"
7+
68
"golang.org/x/tools/go/packages"
79
)
810

9-
func ExtractErrors(pkg *packages.Package) []packages.Error {
11+
//nolint:gocyclo
12+
func ExtractErrors(pkg *packages.Package, astCache *astcache.Cache) []packages.Error {
1013
errors := extractErrorsImpl(pkg)
1114
if len(errors) == 0 {
1215
return errors
@@ -22,17 +25,18 @@ func ExtractErrors(pkg *packages.Package) []packages.Error {
2225
uniqErrors = append(uniqErrors, err)
2326
}
2427

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

34-
// some errors like "code in directory expects import" don't have Pos, set it here
35-
if len(pkg.GoFiles) != 0 {
39+
// some errors like "code in directory expects import" don't have Pos, set it here
3640
for i := range uniqErrors {
3741
err := &uniqErrors[i]
3842
if err.Pos == "" {

pkg/result/processors/autogenerated_exclude.go

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -42,6 +42,11 @@ func (p *AutogeneratedExclude) Process(issues []result.Issue) ([]result.Issue, e
4242
}
4343

4444
func (p *AutogeneratedExclude) shouldPassIssue(i *result.Issue) (bool, error) {
45+
if i.FromLinter == "typecheck" {
46+
// don't hide typechecking errors in generated files: users expect to see why the project isn't compiling
47+
return true, nil
48+
}
49+
4550
fs, err := p.getOrCreateFileSummary(i)
4651
if err != nil {
4752
return false, err

test/run_test.go

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -24,7 +24,8 @@ func TestEmptyDirRun(t *testing.T) {
2424

2525
func TestNotExistingDirRun(t *testing.T) {
2626
testshared.NewLintRunner(t).Run(getTestDataDir("no_such_dir")).
27-
ExpectHasIssue(`cannot find package \"./testdata/no_such_dir\"`)
27+
ExpectExitCode(exitcodes.WarningInTest).
28+
ExpectOutputContains(`cannot find package \"./testdata/no_such_dir\"`)
2829
}
2930

3031
func TestSymlinkLoop(t *testing.T) {

0 commit comments

Comments
 (0)