diff --git a/pkg/goanalysis/pkgerrors/extract.go b/pkg/goanalysis/pkgerrors/extract.go index d1257e6638d6..76a4c90222ce 100644 --- a/pkg/goanalysis/pkgerrors/extract.go +++ b/pkg/goanalysis/pkgerrors/extract.go @@ -2,6 +2,7 @@ package pkgerrors import ( "fmt" + "maps" "regexp" "strings" @@ -18,7 +19,9 @@ func extractErrors(pkg *packages.Package) []packages.Error { return errors } + skippedErrors := map[string]packages.Error{} seenErrors := map[string]bool{} + var uniqErrors []packages.Error for _, err := range errors { msg := stackCrusher(err.Error()) @@ -26,15 +29,35 @@ func extractErrors(pkg *packages.Package) []packages.Error { continue } + // This `if` is important to avoid duplicate errors. + // The goal is to keep the most relevant error. if msg != err.Error() { + prev, alreadySkip := skippedErrors[msg] + if !alreadySkip { + skippedErrors[msg] = err + continue + } + + if len(err.Error()) < len(prev.Error()) { + skippedErrors[msg] = err + } + continue } + delete(skippedErrors, msg) + seenErrors[msg] = true uniqErrors = append(uniqErrors, err) } + // In some cases, the error stack doesn't contain the tip error. + // We must keep at least one of the original errors that contain the specific message. + for skippedError := range maps.Values(skippedErrors) { + uniqErrors = append(uniqErrors, skippedError) + } + if len(pkg.GoFiles) != 0 { // errors were extracted from deps and have at least one file in package for i := range uniqErrors { diff --git a/pkg/goanalysis/pkgerrors/extract_test.go b/pkg/goanalysis/pkgerrors/extract_test.go index 02c801e0b106..611808c59114 100644 --- a/pkg/goanalysis/pkgerrors/extract_test.go +++ b/pkg/goanalysis/pkgerrors/extract_test.go @@ -4,8 +4,130 @@ import ( "testing" "github.com/stretchr/testify/assert" + "golang.org/x/tools/go/packages" ) +func Test_extractErrors(t *testing.T) { + testCases := []struct { + desc string + pkg *packages.Package + + expected []packages.Error + }{ + { + desc: "package with errors", + pkg: &packages.Package{ + IllTyped: true, + Errors: []packages.Error{ + {Pos: "/home/ldez/sources/golangci/sandbox/main.go:6:11", Msg: "test"}, + }, + }, + expected: []packages.Error{ + {Pos: "/home/ldez/sources/golangci/sandbox/main.go:6:11", Msg: "test"}, + }, + }, + { + desc: "full error stack deduplication", + pkg: &packages.Package{ + IllTyped: true, + Imports: map[string]*packages.Package{ + "test": { + IllTyped: true, + Errors: []packages.Error{ + { + Pos: "/home/ldez/sources/golangci/sandbox/main.go:6:11", + Msg: `/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))`, + Kind: 3, + }, + { + Pos: "/home/ldez/sources/go/src/github.com/golangci/golangci-lint/pkg/result/processors/nolint.go:13:2", + Msg: `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))`, + Kind: 3, + }, + { + Pos: "/home/ldez/sources/go/src/github.com/golangci/golangci-lint/pkg/lint/lintersdb/manager.go:13:2", + Msg: `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)`, + Kind: 3, + }, + { + Pos: "/home/ldez/sources/go/src/github.com/golangci/golangci-lint/pkg/golinters/deadcode.go:21:9", + Msg: `undeclared name: linterName`, + Kind: 3, + }, + }, + }, + }, + }, + expected: []packages.Error{{ + Pos: "/home/ldez/sources/go/src/github.com/golangci/golangci-lint/pkg/golinters/deadcode.go:21:9", + Msg: "undeclared name: linterName", + Kind: 3, + }}, + }, + { + desc: "package with import errors but with only one error and without tip error", + pkg: &packages.Package{ + IllTyped: true, + Imports: map[string]*packages.Package{ + "test": { + IllTyped: true, + Errors: []packages.Error{ + { + Pos: "/home/ldez/sources/golangci/sandbox/main.go:6:11", + Msg: "could not import github.com/example/foo (main.go:6:2: missing go.sum entry for module providing package github.com/example/foo (imported by github.com/golangci/sandbox); to add:\n\tgo get github.com/golangci/sandbox)", + Kind: 3, + }, + }, + }, + }, + }, + expected: []packages.Error{{ + Pos: "/home/ldez/sources/golangci/sandbox/main.go:6:11", + Msg: "could not import github.com/example/foo (main.go:6:2: missing go.sum entry for module providing package github.com/example/foo (imported by github.com/golangci/sandbox); to add:\n\tgo get github.com/golangci/sandbox)", + Kind: 3, + }}, + }, + { + desc: "package with import errors but without tip error", + pkg: &packages.Package{ + IllTyped: true, + Imports: map[string]*packages.Package{ + "test": { + IllTyped: true, + Errors: []packages.Error{ + { + Pos: "/home/ldez/sources/golangci/sandbox/main.go:6:1", + Msg: "foo (/home/ldez/sources/golangci/sandbox/main.go:6:11: could not import github.com/example/foo (main.go:6:2: missing go.sum entry for module providing package github.com/example/foo (imported by github.com/golangci/sandbox); to add:\n\tgo get github.com/golangci/sandbox))", + Kind: 3, + }, + { + Pos: "/home/ldez/sources/golangci/sandbox/main.go:6:11", + Msg: "could not import github.com/example/foo (main.go:6:2: missing go.sum entry for module providing package github.com/example/foo (imported by github.com/golangci/sandbox); to add:\n\tgo get github.com/golangci/sandbox)", + Kind: 3, + }, + }, + }, + }, + }, + expected: []packages.Error{{ + Pos: "/home/ldez/sources/golangci/sandbox/main.go:6:11", + Msg: "could not import github.com/example/foo (main.go:6:2: missing go.sum entry for module providing package github.com/example/foo (imported by github.com/golangci/sandbox); to add:\n\tgo get github.com/golangci/sandbox)", + Kind: 3, + }}, + }, + } + + for _, test := range testCases { + t.Run(test.desc, func(t *testing.T) { + t.Parallel() + + errors := extractErrors(test.pkg) + + assert.Equal(t, test.expected, errors) + }) + } +} + func Test_stackCrusher(t *testing.T) { testCases := []struct { desc string diff --git a/pkg/goanalysis/runner_loadingpackage.go b/pkg/goanalysis/runner_loadingpackage.go index 29a27089c1c2..aefcf69f6b70 100644 --- a/pkg/goanalysis/runner_loadingpackage.go +++ b/pkg/goanalysis/runner_loadingpackage.go @@ -78,6 +78,9 @@ func (lp *loadingPackage) analyze(ctx context.Context, cancel context.CancelFunc defer lp.decUse(loadMode < LoadModeWholeProgram) if err := lp.loadWithFacts(loadMode); err != nil { + // Note: this error is ignored when there is no facts loading (e.g. with 98% of linters). + // But this is not a problem because the errors are added to the package.Errors. + // You through an error, try to add it to actions, but there is no action annnddd it's gone! werr := fmt.Errorf("failed to load package %s: %w", lp.pkg.Name, err) // Don't need to write error to errCh, it will be extracted and reported on another layer. @@ -88,6 +91,10 @@ func (lp *loadingPackage) analyze(ctx context.Context, cancel context.CancelFunc act.Err = werr } + if len(lp.actions) == 0 { + lp.log.Warnf("no action but there is an error: %v", err) + } + return } @@ -239,9 +246,11 @@ func (lp *loadingPackage) loadFromExportData() error { return fmt.Errorf("dependency %q hasn't been loaded yet", path) } } + if pkg.ExportFile == "" { return fmt.Errorf("no export data for %q", pkg.ID) } + f, err := os.Open(pkg.ExportFile) if err != nil { return err @@ -332,13 +341,15 @@ func (lp *loadingPackage) loadImportedPackageWithFacts(loadMode LoadMode) error if srcErr := lp.loadFromSource(loadMode); srcErr != nil { return srcErr } + // Make sure this package can't be imported successfully pkg.Errors = append(pkg.Errors, packages.Error{ Pos: "-", Msg: fmt.Sprintf("could not load export data: %s", err), Kind: packages.ParseError, }) - return fmt.Errorf("could not load export data: %w", err) + + return nil } }