From f8c51b1a6c3d6ebfdbeab3c81d4157aefe0e8b71 Mon Sep 17 00:00:00 2001 From: Alan Donovan Date: Wed, 21 May 2025 11:21:31 -0400 Subject: [PATCH] go/doc: NewFromFiles: fix panic on Files with SkipObjectResolution This CL fixes a panic in NewFromFiles when it is provided files produced by the parser in SkipObjectResolution mode, which skips the step of connecting ast.Idents to (deprecated) ast.Objects. Instead of calling ast.NewPackage, which performs a number of unnecessary steps, we just construct the ast.Package directly. Fixes #66290 Change-Id: Id55bd30d8afb9d396c3901070e7607c5a22030d5 Reviewed-on: https://go-review.googlesource.com/c/go/+/675036 Reviewed-by: Robert Griesemer LUCI-TryBot-Result: Go LUCI --- src/go/doc/doc.go | 44 +++++++++++++++----------------------- src/go/doc/example_test.go | 2 +- 2 files changed, 18 insertions(+), 28 deletions(-) diff --git a/src/go/doc/doc.go b/src/go/doc/doc.go index 4d01ae458b..f7e3c1bad8 100644 --- a/src/go/doc/doc.go +++ b/src/go/doc/doc.go @@ -188,6 +188,7 @@ func (p *Package) collectFuncs(funcs []*Func) { // // The package is specified by a list of *ast.Files and corresponding // file set, which must not be nil. +// // NewFromFiles uses all provided files when computing documentation, // so it is the caller's responsibility to provide only the files that // match the desired build context. "go/build".Context.MatchFile can @@ -226,49 +227,38 @@ func NewFromFiles(fset *token.FileSet, files []*ast.File, importPath string, opt // Collect .go and _test.go files. var ( + pkgName string goFiles = make(map[string]*ast.File) testGoFiles []*ast.File ) - for i := range files { - f := fset.File(files[i].Pos()) + for i, file := range files { + f := fset.File(file.Pos()) if f == nil { return nil, fmt.Errorf("file files[%d] is not found in the provided file set", i) } - switch name := f.Name(); { - case strings.HasSuffix(name, ".go") && !strings.HasSuffix(name, "_test.go"): - goFiles[name] = files[i] - case strings.HasSuffix(name, "_test.go"): - testGoFiles = append(testGoFiles, files[i]) + switch filename := f.Name(); { + case strings.HasSuffix(filename, "_test.go"): + testGoFiles = append(testGoFiles, file) + case strings.HasSuffix(filename, ".go"): + pkgName = file.Name.Name + goFiles[filename] = file default: - return nil, fmt.Errorf("file files[%d] filename %q does not have a .go extension", i, name) + return nil, fmt.Errorf("file files[%d] filename %q does not have a .go extension", i, filename) } } - // TODO(dmitshur,gri): A relatively high level call to ast.NewPackage with a simpleImporter - // ast.Importer implementation is made below. It might be possible to short-circuit and simplify. - // Compute package documentation. - pkg, _ := ast.NewPackage(fset, goFiles, simpleImporter, nil) // Ignore errors that can happen due to unresolved identifiers. + // + // Since this package doesn't need Package.{Scope,Imports}, or + // handle errors, and ast.File's Scope field is unset in files + // parsed with parser.SkipObjectResolution, we construct the + // Package directly instead of calling [ast.NewPackage]. + pkg := &ast.Package{Name: pkgName, Files: goFiles} p := New(pkg, importPath, mode) classifyExamples(p, Examples(testGoFiles...)) return p, nil } -// simpleImporter returns a (dummy) package object named by the last path -// component of the provided package path (as is the convention for packages). -// This is sufficient to resolve package identifiers without doing an actual -// import. It never returns an error. -func simpleImporter(imports map[string]*ast.Object, path string) (*ast.Object, error) { - pkg := imports[path] - if pkg == nil { - // note that strings.LastIndex returns -1 if there is no "/" - pkg = ast.NewObj(ast.Pkg, path[strings.LastIndex(path, "/")+1:]) - pkg.Data = ast.NewScope(nil) // required by ast.NewPackage for dot-import - imports[path] = pkg - } - return pkg, nil -} - // lookupSym reports whether the package has a given symbol or method. // // If recv == "", HasSym reports whether the package has a top-level diff --git a/src/go/doc/example_test.go b/src/go/doc/example_test.go index 7919c3a2c0..2fd54f8abb 100644 --- a/src/go/doc/example_test.go +++ b/src/go/doc/example_test.go @@ -328,7 +328,7 @@ func exampleNames(exs []*doc.Example) (out []string) { } func mustParse(fset *token.FileSet, filename, src string) *ast.File { - f, err := parser.ParseFile(fset, filename, src, parser.ParseComments) + f, err := parser.ParseFile(fset, filename, src, parser.ParseComments|parser.SkipObjectResolution) if err != nil { panic(err) }