From b13b29cd8b0e6a914f3981c74c53d5a8ffaaabb3 Mon Sep 17 00:00:00 2001 From: Rick Guo Date: Wed, 5 Aug 2026 22:36:21 +0800 Subject: [PATCH 1/4] fix(packages): prefer go list source diagnostics --- internal/packages/load.go | 59 +++++++++++++++------ internal/packages/load_test.go | 67 ++++++++++++++++++++++++ test/goroot/notapplicable.yaml | 8 --- test/goroot/xfail.yaml | 96 ---------------------------------- 4 files changed, 111 insertions(+), 119 deletions(-) diff --git a/internal/packages/load.go b/internal/packages/load.go index 055be834f5..fe7a068e04 100644 --- a/internal/packages/load.go +++ b/internal/packages/load.go @@ -257,19 +257,10 @@ func loadPackageEx(dedup Deduper, ld *loader, lpkg *loaderPackage) { return // not a source package, don't get syntax trees } - // go list has already captured cmd/compile's authoritative diagnostics in - // this block. For example, an unexpected else stops gc before recovery AST - // errors are reported, so do not append the local parser/type follow-ons. - hasCompilerSyntaxError := false - for _, err := range lpkg.Errors { - if err.Kind != packages.ListError || !strings.HasPrefix(err.Msg, "# ") { - continue - } - if _, diagnostics, ok := strings.Cut(err.Msg, "\n"); ok && strings.Contains(diagnostics, ": syntax error: ") { - hasCompilerSyntaxError = true - break - } - } + // A go list build diagnostic for this package's Go source is authoritative, + // so do not append local parser/type follow-ons or process the recovery AST + // as a valid package. + hasCompilerDiagnostics := hasGoSourceListDiagnostics(lpkg.Errors, lpkg.CompiledGoFiles) appendError := func(err error) { // Convert various error types into the one true Error. @@ -289,7 +280,7 @@ func loadPackageEx(dedup Deduper, ld *loader, lpkg *loaderPackage) { case scanner.ErrorList: // from parser - if hasCompilerSyntaxError { + if hasCompilerDiagnostics { return } for _, err := range err { @@ -302,7 +293,7 @@ func loadPackageEx(dedup Deduper, ld *loader, lpkg *loaderPackage) { case types.Error: // from type checker - if hasCompilerSyntaxError { + if hasCompilerDiagnostics { return } lpkg.TypeErrors = append(lpkg.TypeErrors, err) @@ -632,6 +623,44 @@ func diagnosticFileLine(pos string) (file string, line int, ok bool) { return prefix, last, true } +func hasGoSourceListDiagnostics(errs []packages.Error, compiledGoFiles []string) bool { + for _, err := range errs { + if err.Kind != packages.ListError || !strings.HasPrefix(err.Msg, "# ") { + continue + } + _, diagnostics, ok := strings.Cut(err.Msg, "\n") + if !ok { + continue + } + for _, diagnostic := range strings.Split(diagnostics, "\n") { + pos, _, ok := strings.Cut(diagnostic, ": ") + if !ok { + continue + } + diagnosticFile, _, ok := diagnosticFileLine(pos) + if !ok { + continue + } + for _, compiledGoFile := range compiledGoFiles { + if sameDiagnosticFile(diagnosticFile, compiledGoFile) { + return true + } + } + } + } + return false +} + +func sameDiagnosticFile(left, right string) bool { + left = filepath.Clean(left) + right = filepath.Clean(right) + if left == right { + return true + } + return !filepath.IsAbs(left) && strings.HasSuffix(right, string(filepath.Separator)+left) || + !filepath.IsAbs(right) && strings.HasSuffix(left, string(filepath.Separator)+right) +} + func localVarHasDocComment(file *ast.File, comment *ast.Comment) bool { found := false ast.Inspect(file, func(node ast.Node) bool { diff --git a/internal/packages/load_test.go b/internal/packages/load_test.go index c6bd6eee9a..0184477db7 100644 --- a/internal/packages/load_test.go +++ b/internal/packages/load_test.go @@ -11,6 +11,8 @@ import ( "strconv" "strings" "testing" + + xpackages "golang.org/x/tools/go/packages" ) func TestLoadExWithGoVersion(t *testing.T) { @@ -257,6 +259,28 @@ func f() { } }) + t.Run("compiler type errors are authoritative", func(t *testing.T) { + dir := t.TempDir() + writeLoadTestFile(t, filepath.Join(dir, "go.mod"), "module example.com/typeerror\ngo 1.24\n") + writeLoadTestFile(t, filepath.Join(dir, "load.go"), `package typeerror +var x = struct{ X int }{Y: 1} +`) + cfg := loadTestConfig(dir) + cfg.BuildFlags = []string{"-gcflags=all=-e"} + pkgs, err := LoadExWithGoVersion(nil, nil, cfg, "go1.24", ".") + if err != nil { + t.Fatal(err) + } + if len(pkgs) != 1 { + t.Fatalf("load returned %d packages, want 1", len(pkgs)) + } + pkg := pkgs[0] + assertPackageError(t, pkg, "unknown field Y") + if len(pkg.Errors) != 1 { + t.Fatalf("load returned %d package errors, want the compiler diagnostic only: %+v", len(pkg.Errors), pkg.Errors) + } + }) + t.Run("embed local var", func(t *testing.T) { dir := t.TempDir() writeLoadTestFile(t, filepath.Join(dir, "go.mod"), "module example.com/embedlocal\ngo 1.24\n") @@ -297,6 +321,49 @@ var x string`) } +func TestHasGoSourceListDiagnostics(t *testing.T) { + goFile := filepath.Join(string(filepath.Separator), "tmp", "example", "load.go") + tests := []struct { + name string + err xpackages.Error + want bool + }{ + { + name: "absolute Go source", + err: xpackages.Error{Kind: xpackages.ListError, Msg: "# example.com/p\n" + goFile + ":2:3: undefined: missing"}, + want: true, + }, + { + name: "relative Go source", + err: xpackages.Error{Kind: xpackages.ListError, Msg: "# example.com/p\nload.go:2: undefined: missing"}, + want: true, + }, + { + name: "different Go source", + err: xpackages.Error{Kind: xpackages.ListError, Msg: "# example.com/p\nother.go:2: undefined: missing"}, + }, + { + name: "assembly source", + err: xpackages.Error{Kind: xpackages.ListError, Msg: "# example.com/p\nasm.s:2: unexpected token"}, + }, + { + name: "metadata list error", + err: xpackages.Error{Kind: xpackages.ListError, Msg: "import cycle not allowed"}, + }, + { + name: "non-list error", + err: xpackages.Error{Kind: xpackages.TypeError, Msg: "# example.com/p\nload.go:2: undefined: missing"}, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + if got := hasGoSourceListDiagnostics([]xpackages.Error{tt.err}, []string{goFile}); got != tt.want { + t.Fatalf("hasGoSourceListDiagnostics() = %t, want %t", got, tt.want) + } + }) + } +} + func loadOnePackage(t *testing.T, dir, goVersion string) *Package { t.Helper() pkgs, err := LoadExWithGoVersion(nil, nil, loadTestConfig(dir), goVersion, ".") diff --git a/test/goroot/notapplicable.yaml b/test/goroot/notapplicable.yaml index 53798ae520..dcb1447771 100644 --- a/test/goroot/notapplicable.yaml +++ b/test/goroot/notapplicable.yaml @@ -91,10 +91,6 @@ not_applicable: directive: errorcheck case: fixedbugs/issue18419.dir/test.go reason: "not applicable: this case asserts cmd/compile-specific -m optimization diagnostics; LLGo uses go/types plus LLVM and does not reproduce gc compiler diagnostic output; supporting this toolchain-specific behavior is not an LLGo compatibility goal" - - version: go1.26 - directive: errorcheck - case: fixedbugs/bug388.go - reason: "not applicable: this case asserts gc runtime pseudo-type diagnostics; LLGo uses its own runtime and does not expose gc's private compiler diagnostics; supporting this toolchain-specific behavior is not an LLGo compatibility goal" - version: go1.26 directive: errorcheck case: escape2.go @@ -155,10 +151,6 @@ not_applicable: directive: errorcheck case: escape_map.go reason: "not applicable: this case asserts cmd/compile-specific -m escape-analysis diagnostics; LLGo uses go/types plus LLVM and does not reproduce gc compiler diagnostic output; supporting this toolchain-specific behavior is not an LLGo compatibility goal" - - version: go1.26 - directive: errorcheck - case: runtime.go - reason: "not applicable: this case asserts gc runtime private-symbol compiler diagnostics; LLGo uses its own runtime and does not expose gc's private compiler diagnostics; supporting this toolchain-specific behavior is not an LLGo compatibility goal" - version: go1.26 directive: errorcheck case: fixedbugs/notinheap.go diff --git a/test/goroot/xfail.yaml b/test/goroot/xfail.yaml index 6183ce5d5e..5174ce179a 100644 --- a/test/goroot/xfail.yaml +++ b/test/goroot/xfail.yaml @@ -1822,110 +1822,14 @@ xfails: directive: errorcheck case: fixedbugs/issue4776.go reason: llgo parser uses a different missing-package-clause diagnostic - - version: go1.26 - directive: errorcheck - case: fixedbugs/issue4405.go - reason: llgo parser recovery emits additional malformed-statement diagnostics - - version: go1.26 - directive: errorcheck - case: syntax/semi4.go - reason: llgo parser recovery emits additional semicolon and brace diagnostics - - version: go1.26 - directive: errorcheck - case: fixedbugs/issue13273.go - reason: llgo parser and type checker emit additional channel-direction diagnostics - - version: go1.26 - directive: errorcheck - case: syntax/semi3.go - reason: llgo parser recovery emits additional semicolon and end-of-file diagnostics - version: go1.26 directive: errorcheck case: fixedbugs/issue13266.go reason: llgo parser uses a different malformed-package-clause diagnostic - - version: go1.26 - directive: errorcheck - case: fixedbugs/issue19667.go - reason: llgo parser recovery emits additional malformed-call diagnostics - - version: go1.26 - directive: errorcheck - case: syntax/semi2.go - reason: llgo parser recovery emits additional semicolon and brace diagnostics - - version: go1.26 - directive: errorcheck - case: fixedbugs/issue13248.go - reason: llgo parser recovery emits additional malformed-expression diagnostics - - version: go1.26 - directive: errorcheck - case: fixedbugs/issue17328.go - reason: llgo parser recovery emits additional end-of-file diagnostics - - version: go1.26 - directive: errorcheck - case: syntax/semi1.go - reason: llgo parser recovery emits additional semicolon and end-of-file diagnostics - - version: go1.26 - directive: errorcheck - case: fixedbugs/issue20789.go - reason: llgo parser does not emit the expected malformed-name diagnostic - - version: go1.26 - directive: errorcheck - case: syntax/initvar.go - reason: llgo parser recovery emits additional invalid-initializer diagnostics - version: go1.26 directive: errorcheck case: fixedbugs/bug050.go reason: llgo parser emits an additional malformed-declaration diagnostic - - version: go1.26 - directive: errorcheck - case: bombad.go - reason: llgo parser recovery emits additional byte-order-mark diagnostics - - version: go1.26 - directive: errorcheck - case: fixedbugs/bug435.go - reason: llgo parser recovery emits additional end-of-file diagnostics - - version: go1.26 - directive: errorcheck - case: fixedbugs/issue14006.go - reason: llgo parser recovery emits additional malformed-label diagnostics - - version: go1.26 - directive: errorcheck - case: fixedbugs/issue22581.go - reason: llgo parser recovery emits additional malformed-control-clause diagnostics - - version: go1.26 - directive: errorcheck - case: syntax/ddd.go - reason: llgo parser recovery emits additional malformed-selector diagnostics - - version: go1.26 - directive: errorcheck - case: fixedbugs/bug121.go - reason: llgo parser recovery emits additional malformed-declaration diagnostics - - version: go1.26 - directive: errorcheck - case: fixedbugs/bug228.go - reason: llgo parser emits an additional malformed-parameter diagnostic - - version: go1.26 - directive: errorcheck - case: syntax/chan1.go - reason: llgo parser recovery emits additional malformed-channel diagnostics - - version: go1.26 - directive: errorcheck - case: fixedbugs/issue18747.go - reason: llgo parser recovery emits additional undefined-name diagnostics - - version: go1.26 - directive: errorcheck - case: fixedbugs/issue11610.go - reason: llgo parser reports additional illegal-character diagnostics - - version: go1.26 - directive: errorcheck - case: fixedbugs/issue22164.go - reason: llgo parser recovery emits additional diagnostics for a malformed argument list - - version: go1.26 - directive: errorcheck - case: syntax/vareq1.go - reason: llgo parser recovery emits an additional diagnostic after the expected syntax error - - version: go1.26 - directive: errorcheck - case: syntax/vareq.go - reason: llgo parser recovery emits additional diagnostics after the expected syntax error - version: go1.24 platform: darwin/arm64 directive: run From bd9b714343379843784b10f2ff7a33d521722ba7 Mon Sep 17 00:00:00 2001 From: Rick Guo Date: Thu, 6 Aug 2026 00:39:57 +0800 Subject: [PATCH 2/4] fix(packages): clarify list diagnostic filtering --- internal/packages/load.go | 14 +++++---- internal/packages/load_test.go | 55 +++++++++++++++++++++++----------- 2 files changed, 46 insertions(+), 23 deletions(-) diff --git a/internal/packages/load.go b/internal/packages/load.go index fe7a068e04..50f63a19c5 100644 --- a/internal/packages/load.go +++ b/internal/packages/load.go @@ -623,16 +623,16 @@ func diagnosticFileLine(pos string) (file string, line int, ok bool) { return prefix, last, true } +// go list reports compiler and child-tool failures as ListError. For example, +// an assembler failure names asm.s even though only asm.go is in +// CompiledGoFiles. Restricting positioned diagnostics to CompiledGoFiles keeps +// child-tool failures from being mistaken for Go frontend diagnostics. func hasGoSourceListDiagnostics(errs []packages.Error, compiledGoFiles []string) bool { for _, err := range errs { if err.Kind != packages.ListError || !strings.HasPrefix(err.Msg, "# ") { continue } - _, diagnostics, ok := strings.Cut(err.Msg, "\n") - if !ok { - continue - } - for _, diagnostic := range strings.Split(diagnostics, "\n") { + for diagnostic := range strings.Lines(err.Msg) { pos, _, ok := strings.Cut(diagnostic, ": ") if !ok { continue @@ -651,6 +651,10 @@ func hasGoSourceListDiagnostics(errs []packages.Error, compiledGoFiles []string) return false } +// sameDiagnosticFile accepts relative suffixes because go list diagnostics may +// shorten an absolute CompiledGoFiles path. For example, example/load.go +// matches /tmp/project/example/load.go. Requiring a path separator before the +// suffix prevents load.go from matching myload.go. func sameDiagnosticFile(left, right string) bool { left = filepath.Clean(left) right = filepath.Clean(right) diff --git a/internal/packages/load_test.go b/internal/packages/load_test.go index 0184477db7..582547ef74 100644 --- a/internal/packages/load_test.go +++ b/internal/packages/load_test.go @@ -323,41 +323,60 @@ var x string`) func TestHasGoSourceListDiagnostics(t *testing.T) { goFile := filepath.Join(string(filepath.Separator), "tmp", "example", "load.go") + cgoFile := filepath.Join(string(filepath.Separator), "tmp", "example", "cgoerr", "cgo.go") tests := []struct { - name string - err xpackages.Error - want bool + name string + err xpackages.Error + compiledGoFiles []string + want bool }{ { - name: "absolute Go source", - err: xpackages.Error{Kind: xpackages.ListError, Msg: "# example.com/p\n" + goFile + ":2:3: undefined: missing"}, - want: true, + name: "absolute Go source", + err: xpackages.Error{Kind: xpackages.ListError, Msg: "# example.com/p\n" + goFile + ":2:3: undefined: missing"}, + compiledGoFiles: []string{goFile}, + want: true, }, { - name: "relative Go source", - err: xpackages.Error{Kind: xpackages.ListError, Msg: "# example.com/p\nload.go:2: undefined: missing"}, - want: true, + name: "relative Go source", + err: xpackages.Error{Kind: xpackages.ListError, Msg: "# example.com/p\nload.go:2: undefined: missing"}, + compiledGoFiles: []string{goFile}, + want: true, }, { - name: "different Go source", - err: xpackages.Error{Kind: xpackages.ListError, Msg: "# example.com/p\nother.go:2: undefined: missing"}, + name: "different Go source", + err: xpackages.Error{Kind: xpackages.ListError, Msg: "# example.com/p\nother.go:2: undefined: missing"}, + compiledGoFiles: []string{goFile}, }, { - name: "assembly source", - err: xpackages.Error{Kind: xpackages.ListError, Msg: "# example.com/p\nasm.s:2: unexpected token"}, + name: "cgo diagnostic in Go source", + err: xpackages.Error{Kind: xpackages.ListError, Msg: `# example.com/diagcompare/cgoerr +cgoerr/cgo.go:4:2: error: "intentional cgo failure" +#error "intentional cgo failure" + ^ +1 error generated. +`}, + compiledGoFiles: []string{cgoFile}, + want: true, }, { - name: "metadata list error", - err: xpackages.Error{Kind: xpackages.ListError, Msg: "import cycle not allowed"}, + name: "unpositioned build error", + err: xpackages.Error{Kind: xpackages.ListError, Msg: "# example.com/p\ncompile: internal compiler error"}, + compiledGoFiles: []string{goFile}, }, { - name: "non-list error", - err: xpackages.Error{Kind: xpackages.TypeError, Msg: "# example.com/p\nload.go:2: undefined: missing"}, + name: "metadata list error", + err: xpackages.Error{Kind: xpackages.ListError, Msg: "import cycle not allowed"}, + compiledGoFiles: []string{goFile}, + }, + { + name: "non-list error", + err: xpackages.Error{Kind: xpackages.TypeError, Msg: "# example.com/p\nload.go:2: undefined: missing"}, + compiledGoFiles: []string{goFile}, }, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - if got := hasGoSourceListDiagnostics([]xpackages.Error{tt.err}, []string{goFile}); got != tt.want { + if got := hasGoSourceListDiagnostics([]xpackages.Error{tt.err}, tt.compiledGoFiles); got != tt.want { t.Fatalf("hasGoSourceListDiagnostics() = %t, want %t", got, tt.want) } }) From 97c5f1e84b783926b286094cda71ebd4cf8e5d26 Mon Sep 17 00:00:00 2001 From: Rick Guo Date: Thu, 6 Aug 2026 00:43:34 +0800 Subject: [PATCH 3/4] docs(packages): explain list diagnostic filtering --- internal/packages/load.go | 14 ++++++++++---- 1 file changed, 10 insertions(+), 4 deletions(-) diff --git a/internal/packages/load.go b/internal/packages/load.go index 50f63a19c5..fab4144ef7 100644 --- a/internal/packages/load.go +++ b/internal/packages/load.go @@ -623,10 +623,16 @@ func diagnosticFileLine(pos string) (file string, line int, ok bool) { return prefix, last, true } -// go list reports compiler and child-tool failures as ListError. For example, -// an assembler failure names asm.s even though only asm.go is in -// CompiledGoFiles. Restricting positioned diagnostics to CompiledGoFiles keeps -// child-tool failures from being mistaken for Go frontend diagnostics. +// hasGoSourceListDiagnostics reports whether go list returned a positioned +// ListError for one of the package's compiled Go files. For example: +// +// # example.com/p +// load.go:2: undefined: missing +// +// The caller treats that go list block as authoritative and suppresses local +// parser/type follow-on errors. The CompiledGoFiles check matters because go +// list also forwards child-tool failures: an asm.s diagnostic must not match a +// package whose compiled Go source is asm.go. func hasGoSourceListDiagnostics(errs []packages.Error, compiledGoFiles []string) bool { for _, err := range errs { if err.Kind != packages.ListError || !strings.HasPrefix(err.Msg, "# ") { From 15c415828e8ade8ba4ee24e22af47afab9c1a5f7 Mon Sep 17 00:00:00 2001 From: Rick Guo Date: Fri, 7 Aug 2026 15:18:44 +0800 Subject: [PATCH 4/4] test(goroot): restore invalid receiver xfails --- test/goroot/xfail.yaml | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/test/goroot/xfail.yaml b/test/goroot/xfail.yaml index 5174ce179a..16eeff8953 100644 --- a/test/goroot/xfail.yaml +++ b/test/goroot/xfail.yaml @@ -1818,6 +1818,18 @@ flakes: case: fixedbugs/issue25897a.go reason: reflect-generated goroutines under a single-P continuous-GC loop can complete or exceed the 1m run timeout on darwin/arm64 xfails: + - version: go1.26 + directive: errorcheck + case: fixedbugs/bug299.go + reason: preload traverses a malformed parenthesized receiver AST and panics before the authoritative go list diagnostics are emitted + - version: go1.26 + directive: errorcheck + case: fixedbugs/issue20789.go + reason: preload traverses a parser-recovery receiver AST and panics before the authoritative go list diagnostic is emitted + - version: go1.26 + directive: errorcheck + case: fixedbugs/issue5089.go + reason: preload assumes the non-local selector receiver is a local named type and panics before the authoritative go list diagnostic is emitted - version: go1.26 directive: errorcheck case: fixedbugs/issue4776.go