Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
69 changes: 54 additions & 15 deletions internal/packages/load.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Comment thread
MeteorsLiu marked this conversation as resolved.

appendError := func(err error) {
// Convert various error types into the one true Error.
Expand All @@ -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 {
Expand All @@ -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)
Expand Down Expand Up @@ -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) {
Comment thread
MeteorsLiu marked this conversation as resolved.
pos, _, ok := strings.Cut(diagnostic, ": ")
if !ok {
continue
}
diagnosticFile, _, ok := diagnosticFileLine(pos)
if !ok {
continue
}
for _, compiledGoFile := range compiledGoFiles {
if sameDiagnosticFile(diagnosticFile, compiledGoFile) {

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

NOTE: BAD performance

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) ||
Comment thread
MeteorsLiu marked this conversation as resolved.
!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 {
Expand Down
86 changes: 86 additions & 0 deletions internal/packages/load_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,8 @@ import (
"strconv"
"strings"
"testing"

xpackages "golang.org/x/tools/go/packages"
)

func TestLoadExWithGoVersion(t *testing.T) {
Expand Down Expand Up @@ -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")
Expand Down Expand Up @@ -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, ".")
Expand Down
8 changes: 0 additions & 8 deletions test/goroot/notapplicable.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down
100 changes: 8 additions & 92 deletions test/goroot/xfail.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
Loading