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 <gri@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>

diff --git a/src/go/doc/doc.go b/src/go/doc/doc.go
index 4d01ae458b1c78a95aab502e19c2bf0b6fc4bc65..f7e3c1bad8207b2d0cfb34177ceefe15f73e6921 100644 (file)
--- 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 7919c3a2c03eddc07d3c17cb93a6bd60543b9744..2fd54f8abb50d3a72270b429cf5657f269357a4d 100644 (file)
--- 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)
        }