go/internal/gcimporter: populate (*types.Package).Imports

This is a copy of an upstream change to the tools repo:
https://go-review.googlesource.com/#/c/8924/

This is a second attempt at CL 8954, with the necessary change to
go/build's deps test.

Change-Id: Ib798498cf85fea0baec5667e9324d11f6ae8ad64
Reviewed-on: https://go-review.googlesource.com/9173
Reviewed-by: Robert Griesemer <gri@golang.org>
Reviewed-by: Russ Cox <rsc@golang.org>

diff --git a/src/go/build/deps_test.go b/src/go/build/deps_test.go
index fb64418866cc97344e40a898d6de37ec47ddf1a4..b183aea98656eae4ff00827966d742c25a189ea0 100644 (file)
--- a/src/go/build/deps_test.go
+++ b/src/go/build/deps_test.go
@@ -344,7 +344,7 @@ var pkgDeps = map[string][]string{
        "go/constant":                       {"fmt", "go/token", "math/big", "strconv"},
        "go/format":                         {"bytes", "fmt", "go/ast", "go/parser", "go/printer", "go/token", "internal/format", "io"},
        "go/importer":                       {"go/internal/gcimporter", "go/internal/gccgoimporter", "go/types", "io", "runtime"},
-       "go/internal/gcimporter":            {"bufio", "errors", "fmt", "go/build", "go/constant", "go/token", "go/types", "io", "os", "path/filepath", "strconv", "strings", "text/scanner"},
+       "go/internal/gcimporter":            {"bufio", "errors", "fmt", "go/build", "go/constant", "go/token", "go/types", "io", "os", "path/filepath", "sort", "strconv", "strings", "text/scanner"},
        "go/internal/gccgoimporter":         {"bufio", "bytes", "debug/elf", "errors", "fmt", "go/constant", "go/token", "go/types", "io", "os", "os/exec", "path/filepath", "strconv", "strings", "text/scanner"},
        "go/types":                          {"bytes", "container/heap", "fmt", "go/ast", "go/constant", "go/parser", "go/token", "io", "math", "path", "sort", "strconv", "strings", "sync", "unicode"},
        "image/internal/imageutil":          {"image"},

diff --git a/src/go/internal/gcimporter/gcimporter.go b/src/go/internal/gcimporter/gcimporter.go
index 4e987f5e06760b769c0f550b9bf07f73c12b4601..1d485cf9cbf4aafc9330280874c916487289fda5 100644 (file)
--- a/src/go/internal/gcimporter/gcimporter.go
+++ b/src/go/internal/gcimporter/gcimporter.go
@@ -14,6 +14,7 @@ import (
        "io"
        "os"
        "path/filepath"
+       "sort"
        "strconv"
        "strings"
        "text/scanner"
@@ -74,18 +75,18 @@ func FindPkg(path, srcDir string) (filename, id string) {
 }
 
 // ImportData imports a package by reading the gc-generated export data,
-// adds the corresponding package object to the imports map indexed by id,
+// adds the corresponding package object to the packages map indexed by id,
 // and returns the object.
 //
-// The imports map must contains all packages already imported. The data
+// The packages map must contains all packages already imported. The data
 // reader position must be the beginning of the export data section. The
 // filename is only used in error messages.
 //
-// If imports[id] contains the completely imported package, that package
+// If packages[id] contains the completely imported package, that package
 // can be used directly, and there is no need to call this function (but
 // there is also no harm but for extra time used).
 //
-func ImportData(imports map[string]*types.Package, filename, id string, data io.Reader) (pkg *types.Package, err error) {
+func ImportData(packages map[string]*types.Package, filename, id string, data io.Reader) (pkg *types.Package, err error) {
        // support for parser error handling
        defer func() {
                switch r := recover().(type) {
@@ -99,18 +100,18 @@ func ImportData(imports map[string]*types.Package, filename, id string, data io.
        }()
 
        var p parser
-       p.init(filename, id, data, imports)
+       p.init(filename, id, data, packages)
        pkg = p.parseExport()
 
        return
 }
 
 // Import imports a gc-generated package given its import path, adds the
-// corresponding package object to the imports map, and returns the object.
+// corresponding package object to the packages map, and returns the object.
 // Local import paths are interpreted relative to the current working directory.
-// The imports map must contains all packages already imported.
+// The packages map must contain all packages already imported.
 //
-func Import(imports map[string]*types.Package, path string) (pkg *types.Package, err error) {
+func Import(packages map[string]*types.Package, path string) (pkg *types.Package, err error) {
        // package "unsafe" is handled by the type checker
        if path == "unsafe" {
                panic(`gcimporter.Import called for package "unsafe"`)
@@ -131,7 +132,7 @@ func Import(imports map[string]*types.Package, path string) (pkg *types.Package,
        }
 
        // no need to re-import if the package was imported completely before
-       if pkg = imports[id]; pkg != nil && pkg.Complete() {
+       if pkg = packages[id]; pkg != nil && pkg.Complete() {
                return
        }
 
@@ -153,7 +154,7 @@ func Import(imports map[string]*types.Package, path string) (pkg *types.Package,
                return
        }
 
-       pkg, err = ImportData(imports, filename, id, buf)
+       pkg, err = ImportData(packages, filename, id, buf)
 
        return
 }
@@ -170,14 +171,15 @@ func Import(imports map[string]*types.Package, path string) (pkg *types.Package,
 // parser parses the exports inside a gc compiler-produced
 // object/archive file and populates its scope with the results.
 type parser struct {
-       scanner scanner.Scanner
-       tok     rune                      // current token
-       lit     string                    // literal string; only valid for Ident, Int, String tokens
-       id      string                    // package id of imported package
-       imports map[string]*types.Package // package id -> package object
+       scanner    scanner.Scanner
+       tok        rune                      // current token
+       lit        string                    // literal string; only valid for Ident, Int, String tokens
+       id         string                    // package id of imported package
+       sharedPkgs map[string]*types.Package // package id -> package object (across importer)
+       localPkgs  map[string]*types.Package // package id -> package object (just this package)
 }
 
-func (p *parser) init(filename, id string, src io.Reader, imports map[string]*types.Package) {
+func (p *parser) init(filename, id string, src io.Reader, packages map[string]*types.Package) {
        p.scanner.Init(src)
        p.scanner.Error = func(_ *scanner.Scanner, msg string) { p.error(msg) }
        p.scanner.Mode = scanner.ScanIdents | scanner.ScanInts | scanner.ScanChars | scanner.ScanStrings | scanner.ScanComments | scanner.SkipComments
@@ -185,10 +187,10 @@ func (p *parser) init(filename, id string, src io.Reader, imports map[string]*ty
        p.scanner.Filename = filename // for good error messages
        p.next()
        p.id = id
-       p.imports = imports
+       p.sharedPkgs = packages
        if debug {
-               // check consistency of imports map
-               for _, pkg := range imports {
+               // check consistency of packages map
+               for _, pkg := range packages {
                        if pkg.Name() == "" {
                                fmt.Printf("no package name for %s\n", pkg.Path())
                        }
@@ -334,17 +336,32 @@ func (p *parser) parseQualifiedName() (id, name string) {
 
 // getPkg returns the package for a given id. If the package is
 // not found but we have a package name, create the package and
-// add it to the p.imports map.
+// add it to the p.localPkgs and p.sharedPkgs maps.
+//
+// id identifies a package, usually by a canonical package path like
+// "encoding/json" but possibly by a non-canonical import path like
+// "./json".
 //
 func (p *parser) getPkg(id, name string) *types.Package {
-       // package unsafe is not in the imports map - handle explicitly
+       // package unsafe is not in the packages maps - handle explicitly
        if id == "unsafe" {
                return types.Unsafe
        }
-       pkg := p.imports[id]
+
+       pkg := p.localPkgs[id]
        if pkg == nil && name != "" {
-               pkg = types.NewPackage(id, name)
-               p.imports[id] = pkg
+               // first import of id from this package
+               pkg = p.sharedPkgs[id]
+               if pkg == nil {
+                       // first import of id by this importer
+                       pkg = types.NewPackage(id, name)
+                       p.sharedPkgs[id] = pkg
+               }
+
+               if p.localPkgs == nil {
+                       p.localPkgs = make(map[string]*types.Package)
+               }
+               p.localPkgs[id] = pkg
        }
        return pkg
 }
@@ -405,21 +422,21 @@ func (p *parser) parseMapType() types.Type {
 //
 // If materializePkg is set, the returned package is guaranteed to be set.
 // For fully qualified names, the returned package may be a fake package
-// (without name, scope, and not in the p.imports map), created for the
+// (without name, scope, and not in the p.sharedPkgs map), created for the
 // sole purpose of providing a package path. Fake packages are created
-// when the package id is not found in the p.imports map; in that case
+// when the package id is not found in the p.sharedPkgs map; in that case
 // we cannot create a real package because we don't have a package name.
 // For non-qualified names, the returned package is the imported package.
 //
 func (p *parser) parseName(materializePkg bool) (pkg *types.Package, name string) {
        switch p.tok {
        case scanner.Ident:
-               pkg = p.imports[p.id]
+               pkg = p.sharedPkgs[p.id]
                name = p.lit
                p.next()
        case '?':
                // anonymous
-               pkg = p.imports[p.id]
+               pkg = p.sharedPkgs[p.id]
                p.next()
        case '@':
                // exported name prefixed with package path
@@ -950,8 +967,25 @@ func (p *parser) parseExport() *types.Package {
                p.errorf("expected no scanner errors, got %d", n)
        }
 
+       // Record all referenced packages as imports.
+       var imports []*types.Package
+       for id, pkg2 := range p.localPkgs {
+               if id == p.id {
+                       continue // avoid self-edge
+               }
+               imports = append(imports, pkg2)
+       }
+       sort.Sort(byPath(imports))
+       pkg.SetImports(imports)
+
        // package was imported completely and without errors
        pkg.MarkComplete()
 
        return pkg
 }
+
+type byPath []*types.Package
+
+func (a byPath) Len() int           { return len(a) }
+func (a byPath) Swap(i, j int)      { a[i], a[j] = a[j], a[i] }
+func (a byPath) Less(i, j int) bool { return a[i].Path() < a[j].Path() }

diff --git a/src/go/internal/gcimporter/gcimporter_test.go b/src/go/internal/gcimporter/gcimporter_test.go
index 85846a1348c5331affe40a06999dc1fcea9414a8..a4b038c91ecc7dbe13732b34398cf562d6713a2b 100644 (file)
--- a/src/go/internal/gcimporter/gcimporter_test.go
+++ b/src/go/internal/gcimporter/gcimporter_test.go
@@ -5,6 +5,7 @@
 package gcimporter
 
 import (
+       "fmt"
        "go/build"
        "io/ioutil"
        "os"
@@ -58,15 +59,15 @@ func compile(t *testing.T, dirname, filename string) string {
 // as if all tested packages were imported into a single package.
 var imports = make(map[string]*types.Package)
 
-func testPath(t *testing.T, path string) bool {
+func testPath(t *testing.T, path string) *types.Package {
        t0 := time.Now()
-       _, err := Import(imports, path)
+       pkg, err := Import(imports, path)
        if err != nil {
                t.Errorf("testPath(%s): %s", path, err)
-               return false
+               return nil
        }
        t.Logf("testPath(%s): %v", path, time.Since(t0))
-       return true
+       return pkg
 }
 
 const maxTime = 30 * time.Second
@@ -88,7 +89,7 @@ func testDir(t *testing.T, dir string, endTime time.Time) (nimports int) {
                        for _, ext := range pkgExts {
                                if strings.HasSuffix(f.Name(), ext) {
                                        name := f.Name()[0 : len(f.Name())-len(ext)] // remove extension
-                                       if testPath(t, filepath.Join(dir, name)) {
+                                       if testPath(t, filepath.Join(dir, name)) != nil {
                                                nimports++
                                        }
                                }
@@ -118,8 +119,17 @@ func TestImport(t *testing.T) {
        }
 
        nimports := 0
-       if testPath(t, "./testdata/exports") {
+       if pkg := testPath(t, "./testdata/exports"); pkg != nil {
                nimports++
+               // The package's Imports should include all the types
+               // referenced by the exportdata, which may be more than
+               // the import statements in the package's source, but
+               // fewer than the transitive closure of dependencies.
+               want := `[package ast ("go/ast") package token ("go/token") package runtime ("runtime")]`
+               got := fmt.Sprint(pkg.Imports())
+               if got != want {
+                       t.Errorf(`Package("exports").Imports() = %s, want %s`, got, want)
+               }
        }
        nimports += testDir(t, "", time.Now().Add(maxTime)) // installed packages
        t.Logf("tested %d imports", nimports)

diff --git a/src/go/types/package.go b/src/go/types/package.go
index 27d6ea73393f3d3878b897168f8223db4ffb42a7..48fe8398fe64d4ff8ca10153bf82661a4e975c6d 100644 (file)
--- a/src/go/types/package.go
+++ b/src/go/types/package.go
@@ -48,8 +48,12 @@ func (pkg *Package) Complete() bool { return pkg.complete }
 // MarkComplete marks a package as complete.
 func (pkg *Package) MarkComplete() { pkg.complete = true }
 
-// Imports returns the list of packages explicitly imported by
+// Imports returns the list of packages directly imported by
 // pkg; the list is in source order. Package unsafe is excluded.
+//
+// If pkg was loaded from export data, Imports includes packages that
+// provide package-level objects referenced by pkg.  This may be more or
+// less than the set of packages directly imported by pkg's source code.
 func (pkg *Package) Imports() []*Package { return pkg.imports }
 
 // SetImports sets the list of explicitly imported packages to list.