diff --git a/internal/packages/load.go b/internal/packages/load.go index 055be834f5..fab4144ef7 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,54 @@ func diagnosticFileLine(pos string) (file string, line int, ok bool) { return prefix, last, true } +// 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, "# ") { + continue + } + for diagnostic := range strings.Lines(err.Msg) { + 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 +} + +// 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) + 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..582547ef74 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,68 @@ 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 + compiledGoFiles []string + want bool + }{ + { + 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"}, + compiledGoFiles: []string{goFile}, + want: true, + }, + { + name: "different Go source", + err: xpackages.Error{Kind: xpackages.ListError, Msg: "# example.com/p\nother.go:2: undefined: missing"}, + compiledGoFiles: []string{goFile}, + }, + { + 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: "unpositioned build error", + err: xpackages.Error{Kind: xpackages.ListError, Msg: "# example.com/p\ncompile: internal compiler error"}, + compiledGoFiles: []string{goFile}, + }, + { + 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}, tt.compiledGoFiles); 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..16eeff8953 100644 --- a/test/goroot/xfail.yaml +++ b/test/goroot/xfail.yaml @@ -1820,112 +1820,28 @@ flakes: xfails: - version: go1.26 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 + 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: syntax/semi4.go - reason: llgo parser recovery emits additional semicolon and brace diagnostics + 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/issue13273.go - reason: llgo parser and type checker emit additional channel-direction diagnostics + 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: syntax/semi3.go - reason: llgo parser recovery emits additional semicolon and end-of-file diagnostics + case: fixedbugs/issue4776.go + reason: llgo parser uses a different missing-package-clause diagnostic - 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