Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

typecheck: display compilation errors as report instead of error #1861

Merged
merged 6 commits into from Mar 25, 2021
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
6 changes: 2 additions & 4 deletions pkg/golinters/goanalysis/runner_action.go
Expand Up @@ -177,21 +177,19 @@ func (act *action) analyze() {
act.r.passToPkg[pass] = act.pkg
act.r.passToPkgGuard.Unlock()

var err error
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")
act.err = errors.Wrap(&IllTypedError{Pkg: act.pkg}, "analysis skipped")
} else {
startedAt = time.Now()
act.result, err = pass.Analyzer.Run(pass)
act.result, act.err = pass.Analyzer.Run(pass)
analyzedIn := time.Since(startedAt)
if analyzedIn > time.Millisecond*10 {
debugf("%s: run analyzer in %s", act, analyzedIn)
}
}
act.err = err

// disallow calls after Run
pass.ExportObjectFact = nil
Expand Down
4 changes: 1 addition & 3 deletions pkg/golinters/goanalysis/runners.go
Expand Up @@ -79,9 +79,7 @@ func runAnalyzers(cfg runAnalyzersConfig, lintCtx *linter.Context) ([]result.Iss
}

issues = append(issues, errIssues...)
if len(errIssues) == 0 {
issues = append(issues, buildAllIssues()...)
}
issues = append(issues, buildAllIssues()...)

return issues, nil
}
Expand Down
40 changes: 31 additions & 9 deletions pkg/packages/util.go
Expand Up @@ -2,6 +2,7 @@ package packages

import (
"fmt"
"strings"

"golang.org/x/tools/go/packages"
)
Expand All @@ -15,22 +16,30 @@ func ExtractErrors(pkg *packages.Package) []packages.Error {
seenErrors := map[string]bool{}
var uniqErrors []packages.Error
for _, err := range errors {
if seenErrors[err.Msg] {
msg := stackCrusher(err.Error())
if seenErrors[msg] {
continue
}
seenErrors[err.Msg] = true

if msg != err.Error() {
continue
}

seenErrors[msg] = true

uniqErrors = append(uniqErrors, err)
}

if len(pkg.GoFiles) != 0 {
// errors were extracted from deps and have at leat one file in package
// errors were extracted from deps and have at least one file in package
for i := range uniqErrors {
_, parseErr := ParseErrorPosition(uniqErrors[i].Pos)
if parseErr != 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])
if _, parseErr := ParseErrorPosition(uniqErrors[i].Pos); parseErr == nil {
bombsimon marked this conversation as resolved.
Show resolved Hide resolved
continue
}

// 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
Expand All @@ -55,7 +64,7 @@ func extractErrorsImpl(pkg *packages.Package, seenPackages map[*packages.Package
return nil
}

if len(pkg.Errors) != 0 {
if len(pkg.Errors) > 0 {
return pkg.Errors
}

Expand All @@ -69,3 +78,16 @@ func extractErrorsImpl(pkg *packages.Package, seenPackages map[*packages.Package

return errors
}

func stackCrusher(msg string) string {
index := strings.Index(msg, "(")
lastIndex := strings.LastIndex(msg, ")")

if index == -1 || index == len(msg)-1 || lastIndex == -1 || lastIndex != len(msg)-1 {
return msg
}

frag := msg[index+1 : lastIndex]

return stackCrusher(frag)
}
43 changes: 43 additions & 0 deletions pkg/packages/util_test.go
@@ -0,0 +1,43 @@
package packages

import (
"testing"

"github.com/stretchr/testify/assert"
)

//nolint:lll
func Test_stackCrusher(t *testing.T) {
testCases := []struct {
desc string
stack string
expected string
}{
{
desc: "large stack",
stack: `/home/ldez/sources/go/src/github.com/golangci/golangci-lint/pkg/result/processors/nolint.go:13:2: /home/ldez/sources/go/src/github.com/golangci/golangci-lint/pkg/result/processors/nolint.go:13:2: could not import github.com/golangci/golangci-lint/pkg/lint/lintersdb (/home/ldez/sources/go/src/github.com/golangci/golangci-lint/pkg/lint/lintersdb/manager.go:13:2: could not import github.com/golangci/golangci-lint/pkg/golinters (/home/ldez/sources/go/src/github.com/golangci/golangci-lint/pkg/golinters/deadcode.go:21:9: undeclared name: linterName))`,
expected: "/home/ldez/sources/go/src/github.com/golangci/golangci-lint/pkg/golinters/deadcode.go:21:9: undeclared name: linterName",
},
{
desc: "no stack",
stack: `/home/ldez/sources/go/src/github.com/golangci/golangci-lint/pkg/golinters/deadcode.go:45:3: undeclared name: linterName`,
expected: "/home/ldez/sources/go/src/github.com/golangci/golangci-lint/pkg/golinters/deadcode.go:45:3: undeclared name: linterName",
},
{
desc: "no stack but message with parenthesis",
stack: `/home/ldez/sources/go/src/github.com/golangci/golangci-lint/pkg/golinters/deadcode.go:20:32: cannot use mu (variable of type sync.Mutex) as goanalysis.Issue value in argument to append`,
expected: "/home/ldez/sources/go/src/github.com/golangci/golangci-lint/pkg/golinters/deadcode.go:20:32: cannot use mu (variable of type sync.Mutex) as goanalysis.Issue value in argument to append",
},
}

for _, test := range testCases {
test := test
t.Run(test.desc, func(t *testing.T) {
t.Parallel()

actual := stackCrusher(test.stack)

assert.Equal(t, test.expected, actual)
})
}
}