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>

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

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)
 	}