fix: no export data must always be handled as a typecheck error (#6061)

ldez · web-flow · commit 88ef93e2d8f2 · 2025-09-07T22:04:10.000+02:00
diff --git a/pkg/goanalysis/pkgerrors/extract.go b/pkg/goanalysis/pkgerrors/extract.go
@@ -2,6 +2,7 @@ package pkgerrors
 
 import (
 	"fmt"
+	"maps"
 	"regexp"
 	"strings"
 
@@ -18,23 +19,45 @@ 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())
 		if seenErrors[msg] {
 			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
@@ -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
@@ -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
 		}
 	}
 
