Skip to content

Commit 34bb05f

Browse files
committed
go/packages: add support to overlays for unwritten files to existing packages
This support is not perfect, but should produce more accurate results than the previous behavior, which is to silently drop those files. Change-Id: Ia00c042bf303e9ec0fb1cbd579c0fccb29073de0 Reviewed-on: https://go-review.googlesource.com/c/151999 Run-TryBot: Michael Matloob <[email protected]> Reviewed-by: Ian Cottrell <[email protected]>
1 parent 895048a commit 34bb05f

File tree

4 files changed

+150
-12
lines changed

4 files changed

+150
-12
lines changed

go/packages/golist.go

Lines changed: 30 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -118,10 +118,6 @@ extractQueries:
118118
// types.SizesFor always returns nil or a *types.StdSizes
119119
response.Sizes, _ = sizes.(*types.StdSizes)
120120

121-
if len(containFiles) == 0 && len(packagesNamed) == 0 {
122-
return response, nil
123-
}
124-
125121
seenPkgs := make(map[string]*Package) // for deduplication. different containing queries could produce same packages
126122
for _, pkg := range response.Packages {
127123
seenPkgs[pkg.ID] = pkg
@@ -134,20 +130,44 @@ extractQueries:
134130
response.Packages = append(response.Packages, p)
135131
}
136132

137-
containsResults, err := runContainsQueries(cfg, listfunc, isFallback, addPkg, containFiles)
138-
if err != nil {
139-
return nil, err
133+
if len(containFiles) != 0 {
134+
containsResults, err := runContainsQueries(cfg, listfunc, isFallback, addPkg, containFiles)
135+
if err != nil {
136+
return nil, err
137+
}
138+
response.Roots = append(response.Roots, containsResults...)
139+
}
140+
141+
if len(packagesNamed) != 0 {
142+
namedResults, err := runNamedQueries(cfg, listfunc, addPkg, packagesNamed)
143+
if err != nil {
144+
return nil, err
145+
}
146+
response.Roots = append(response.Roots, namedResults...)
140147
}
141-
response.Roots = append(response.Roots, containsResults...)
142148

143-
namedResults, err := runNamedQueries(cfg, listfunc, addPkg, packagesNamed)
149+
needPkgs, err := processGolistOverlay(cfg, response)
144150
if err != nil {
145151
return nil, err
146152
}
147-
response.Roots = append(response.Roots, namedResults...)
153+
if len(needPkgs) > 0 {
154+
addNeededOverlayPackages(cfg, listfunc, addPkg, needPkgs)
155+
}
156+
148157
return response, nil
149158
}
150159

160+
func addNeededOverlayPackages(cfg *Config, driver driver, addPkg func(*Package), pkgs []string) error {
161+
response, err := driver(cfg, pkgs...)
162+
if err != nil {
163+
return err
164+
}
165+
for _, pkg := range response.Packages {
166+
addPkg(pkg)
167+
}
168+
return nil
169+
}
170+
151171
func runContainsQueries(cfg *Config, driver driver, isFallback bool, addPkg func(*Package), queries []string) ([]string, error) {
152172
var results []string
153173
for _, query := range queries {

go/packages/golist_overlay.go

Lines changed: 98 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,98 @@
1+
package packages
2+
3+
import (
4+
"go/parser"
5+
"go/token"
6+
"path/filepath"
7+
"strconv"
8+
"strings"
9+
)
10+
11+
// processGolistOverlay provides rudimentary support for adding
12+
// files that don't exist on disk to an overlay. The results can be
13+
// sometimes incorrect.
14+
// TODO(matloob): Handle unsupported cases, including the following:
15+
// - test files
16+
// - adding test and non-test files to test variants of packages
17+
// - determining the correct package to add given a new import path
18+
// - creating packages that don't exist
19+
func processGolistOverlay(cfg *Config, response *driverResponse) (needPkgs []string, err error) {
20+
havePkgs := make(map[string]string) // importPath -> non-test package ID
21+
needPkgsSet := make(map[string]bool)
22+
23+
for _, pkg := range response.Packages {
24+
// This is an approximation of import path to id. This can be
25+
// wrong for tests, vendored packages, and a number of other cases.
26+
havePkgs[pkg.PkgPath] = pkg.ID
27+
}
28+
29+
outer:
30+
for path, contents := range cfg.Overlay {
31+
base := filepath.Base(path)
32+
if strings.HasSuffix(path, "_test.go") {
33+
// Overlays don't support adding new test files yet.
34+
// TODO(matloob): support adding new test files.
35+
continue
36+
}
37+
dir := filepath.Dir(path)
38+
for _, pkg := range response.Packages {
39+
var dirContains, fileExists bool
40+
for _, f := range pkg.GoFiles {
41+
if sameFile(filepath.Dir(f), dir) {
42+
dirContains = true
43+
}
44+
if filepath.Base(f) == base {
45+
fileExists = true
46+
}
47+
}
48+
if dirContains {
49+
if !fileExists {
50+
pkg.GoFiles = append(pkg.GoFiles, path) // TODO(matloob): should the file just be added to GoFiles?
51+
pkg.CompiledGoFiles = append(pkg.CompiledGoFiles, path)
52+
}
53+
imports, err := extractImports(path, contents)
54+
if err != nil {
55+
// Let the parser or type checker report errors later.
56+
continue outer
57+
}
58+
for _, imp := range imports {
59+
_, found := pkg.Imports[imp]
60+
if !found {
61+
needPkgsSet[imp] = true
62+
// TODO(matloob): Handle cases when the following block isn't correct.
63+
// These include imports of test variants, imports of vendored packages, etc.
64+
id, ok := havePkgs[imp]
65+
if !ok {
66+
id = imp
67+
}
68+
pkg.Imports[imp] = &Package{ID: id}
69+
}
70+
}
71+
continue outer
72+
}
73+
}
74+
}
75+
76+
needPkgs = make([]string, 0, len(needPkgsSet))
77+
for pkg := range needPkgsSet {
78+
needPkgs = append(needPkgs, pkg)
79+
}
80+
return needPkgs, err
81+
}
82+
83+
func extractImports(filename string, contents []byte) ([]string, error) {
84+
f, err := parser.ParseFile(token.NewFileSet(), filename, contents, parser.ImportsOnly) // TODO(matloob): reuse fileset?
85+
if err != nil {
86+
return nil, err
87+
}
88+
var res []string
89+
for _, imp := range f.Imports {
90+
quotedPath := imp.Path.Value
91+
path, err := strconv.Unquote(quotedPath)
92+
if err != nil {
93+
return nil, err
94+
}
95+
res = append(res, path)
96+
}
97+
return res, nil
98+
}

go/packages/packages.go

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -432,6 +432,7 @@ func (ld *loader) refine(roots []string, list ...*Package) ([]*Package, error) {
432432
ld.Mode >= LoadTypes && rootIndex >= 0,
433433
needsrc: ld.Mode >= LoadAllSyntax ||
434434
ld.Mode >= LoadSyntax && rootIndex >= 0 ||
435+
ld.Overlay != nil || // Overlays can invalidate export data. TODO(matloob): make this check fine-grained based on dependencies on overlaid files
435436
pkg.ExportFile == "" && pkg.PkgPath != "unsafe",
436437
}
437438
ld.pkgs[lpkg.ID] = lpkg
@@ -819,6 +820,15 @@ func (ld *loader) parseFiles(filenames []string) ([]*ast.File, []error) {
819820
// the same file.
820821
//
821822
func sameFile(x, y string) bool {
823+
if x == y {
824+
// It could be the case that y doesn't exist.
825+
// For instance, it may be an overlay file that
826+
// hasn't been written to disk. To handle that case
827+
// let x == y through. (We added the exact absolute path
828+
// string to the CompiledGoFiles list, so the unwritten
829+
// overlay case implies x==y.)
830+
return true
831+
}
822832
if filepath.Base(x) == filepath.Base(y) { // (optimisation)
823833
if xi, err := os.Stat(x); err == nil {
824834
if yi, err := os.Stat(y); err == nil {

go/packages/packages_test.go

Lines changed: 12 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -824,8 +824,18 @@ func testOverlay(t *testing.T, exporter packagestest.Exporter) {
824824
{map[string][]byte{}, `"abc"`, nil}, // empty overlay
825825
{map[string][]byte{exported.File("golang.org/fake", "c/c.go"): []byte(`package c; const C = "C"`)}, `"abC"`, nil},
826826
{map[string][]byte{exported.File("golang.org/fake", "b/b.go"): []byte(`package b; import "golang.org/fake/c"; const B = "B" + c.C`)}, `"aBc"`, nil},
827-
{map[string][]byte{exported.File("golang.org/fake", "b/b.go"): []byte(`package b; import "d"; const B = "B" + d.D`)}, `unknown`,
828-
[]string{`could not import d (no metadata for d)`}},
827+
// Overlay with an existing file in an existing package adding a new import.
828+
{map[string][]byte{exported.File("golang.org/fake", "b/b.go"): []byte(`package b; import "golang.org/fake/d"; const B = "B" + d.D`)}, `"aBd"`, nil},
829+
// Overlay with a new file in an existing package.
830+
{map[string][]byte{
831+
exported.File("golang.org/fake", "c/c.go"): []byte(`package c;`),
832+
filepath.Join(filepath.Dir(exported.File("golang.org/fake", "c/c.go")), "c_new_file.go"): []byte(`package c; const C = "Ç"`)},
833+
`"abÇ"`, nil},
834+
// Overlay with a new file in an existing package, adding a new dependency to that package.
835+
{map[string][]byte{
836+
exported.File("golang.org/fake", "c/c.go"): []byte(`package c;`),
837+
filepath.Join(filepath.Dir(exported.File("golang.org/fake", "c/c.go")), "c_new_file.go"): []byte(`package c; import "golang.org/fake/d"; const C = "c" + d.D`)},
838+
`"abcd"`, nil},
829839
} {
830840
exported.Config.Overlay = test.overlay
831841
exported.Config.Mode = packages.LoadAllSyntax

0 commit comments

Comments
 (0)