go/packages: add support for entire packages defined in overlays
This change adds support in go/packages for defining an entire package
in an overlay. We also add corresponding tests for this in gopls, to
confirm that it works as expected.
Fixes golang/go#31467
Change-Id: Iead203ab2964a7ac4f571be97624b725ac5de7e0
Reviewed-on: https://go-review.googlesource.com/c/tools/+/172409
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Cottrell <iancottrell@google.com>
diff --git a/go/packages/golist.go b/go/packages/golist.go
index 132d283..3a0d4b0 100644
--- a/go/packages/golist.go
+++ b/go/packages/golist.go
@@ -166,12 +166,8 @@
containsCandidates = append(containsCandidates, modifiedPkgs...)
containsCandidates = append(containsCandidates, needPkgs...)
}
-
- if len(needPkgs) > 0 {
- addNeededOverlayPackages(cfg, golistDriver, response, needPkgs)
- if err != nil {
- return nil, err
- }
+ if err := addNeededOverlayPackages(cfg, golistDriver, response, needPkgs); err != nil {
+ return nil, err
}
// Check candidate packages for containFiles.
if len(containFiles) > 0 {
@@ -191,6 +187,9 @@
}
func addNeededOverlayPackages(cfg *Config, driver driver, response *responseDeduper, pkgs []string) error {
+ if len(pkgs) == 0 {
+ return nil
+ }
dr, err := driver(cfg, pkgs...)
if err != nil {
return err
@@ -198,6 +197,11 @@
for _, pkg := range dr.Packages {
response.addPackage(pkg)
}
+ _, needPkgs, err := processGolistOverlay(cfg, response.dr)
+ if err != nil {
+ return err
+ }
+ addNeededOverlayPackages(cfg, driver, response, needPkgs)
return nil
}
diff --git a/go/packages/golist_overlay.go b/go/packages/golist_overlay.go
index 71ffcd9..ce9206f 100644
--- a/go/packages/golist_overlay.go
+++ b/go/packages/golist_overlay.go
@@ -46,7 +46,9 @@
fileExists = true
}
}
- if dirContains {
+ // The overlay could have included an entirely new package.
+ isNewPackage := extractPackage(pkg, path, contents)
+ if dirContains || isNewPackage {
if !fileExists {
pkg.GoFiles = append(pkg.GoFiles, path) // TODO(matloob): should the file just be added to GoFiles?
pkg.CompiledGoFiles = append(pkg.CompiledGoFiles, path)
@@ -102,3 +104,30 @@
}
return res, nil
}
+
+func extractPackage(pkg *Package, filename string, contents []byte) bool {
+ // TODO(rstambler): Check the message of the actual error?
+ // It differs between $GOPATH and module mode.
+ if len(pkg.Errors) != 1 {
+ return false
+ }
+ if pkg.Name != "" || pkg.ExportFile != "" {
+ return false
+ }
+ if len(pkg.GoFiles) > 0 || len(pkg.CompiledGoFiles) > 0 || len(pkg.OtherFiles) > 0 {
+ return false
+ }
+ if len(pkg.Imports) > 0 {
+ return false
+ }
+ f, err := parser.ParseFile(token.NewFileSet(), filename, contents, parser.PackageClauseOnly) // TODO(matloob): reuse fileset?
+ if err != nil {
+ return false
+ }
+ if filepath.Base(pkg.PkgPath) != f.Name.Name {
+ return false
+ }
+ pkg.Name = f.Name.Name
+ pkg.Errors = []Error{}
+ return true
+}
diff --git a/go/packages/packages_test.go b/go/packages/packages_test.go
index 0d10533..d9ba0f4 100644
--- a/go/packages/packages_test.go
+++ b/go/packages/packages_test.go
@@ -921,6 +921,87 @@
}
}
+func TestNewPackagesInOverlay(t *testing.T) { packagestest.TestAll(t, testNewPackagesInOverlay) }
+func testNewPackagesInOverlay(t *testing.T, exporter packagestest.Exporter) {
+ exported := packagestest.Export(t, exporter, []packagestest.Module{{
+ Name: "golang.org/fake",
+ Files: map[string]interface{}{
+ "a/a.go": `package a; import "golang.org/fake/b"; const A = "a" + b.B`,
+ "b/b.go": `package b; import "golang.org/fake/c"; const B = "b" + c.C`,
+ "c/c.go": `package c; const C = "c"`,
+ "d/d.go": `package d; const D = "d"`,
+ }}})
+ defer exported.Cleanup()
+
+ dir := filepath.Dir(filepath.Dir(exported.File("golang.org/fake", "a/a.go")))
+
+ for i, test := range []struct {
+ overlay map[string][]byte
+ want string // expected value of e.E
+ }{
+ // Overlay with one file.
+ {map[string][]byte{
+ filepath.Join(dir, "e", "e.go"): []byte(`package e; import "golang.org/fake/a"; const E = "e" + a.A`)},
+ `"eabc"`},
+ // Overlay with multiple files in the same package.
+ {map[string][]byte{
+ filepath.Join(dir, "e", "e.go"): []byte(`package e; import "golang.org/fake/a"; const E = "e" + a.A + underscore`),
+ filepath.Join(dir, "e", "e_util.go"): []byte(`package e; const underscore = "_"`),
+ },
+ `"eabc_"`},
+ // Overlay with multiple files in different packages.
+ {map[string][]byte{
+ filepath.Join(dir, "e", "e.go"): []byte(`package e; import "golang.org/fake/f"; const E = "e" + f.F + underscore`),
+ filepath.Join(dir, "e", "e_util.go"): []byte(`package e; const underscore = "_"`),
+ filepath.Join(dir, "f", "f.go"): []byte(`package f; const F = "f"`),
+ },
+ `"ef_"`},
+ {map[string][]byte{
+ filepath.Join(dir, "e", "e.go"): []byte(`package e; import "golang.org/fake/f"; const E = "e" + f.F + underscore`),
+ filepath.Join(dir, "e", "e_util.go"): []byte(`package e; const underscore = "_"`),
+ filepath.Join(dir, "f", "f.go"): []byte(`package f; import "golang.org/fake/g"; const F = "f" + g.G`),
+ filepath.Join(dir, "g", "g.go"): []byte(`package g; const G = "g"`),
+ },
+ `"efg_"`},
+ {map[string][]byte{
+ filepath.Join(dir, "e", "e.go"): []byte(`package e; import "golang.org/fake/f"; import "golang.org/fake/h"; const E = "e" + f.F + h.H + underscore`),
+ filepath.Join(dir, "e", "e_util.go"): []byte(`package e; const underscore = "_"`),
+ filepath.Join(dir, "f", "f.go"): []byte(`package f; import "golang.org/fake/g"; const F = "f" + g.G`),
+ filepath.Join(dir, "g", "g.go"): []byte(`package g; const G = "g"`),
+ filepath.Join(dir, "h", "h.go"): []byte(`package h; const H = "h"`),
+ },
+ `"efgh_"`},
+ {map[string][]byte{
+ filepath.Join(dir, "e", "e.go"): []byte(`package e; import "golang.org/fake/f"; const E = "e" + f.F + underscore`),
+ filepath.Join(dir, "e", "e_util.go"): []byte(`package e; const underscore = "_"`),
+ filepath.Join(dir, "f", "f.go"): []byte(`package f; import "golang.org/fake/g"; const F = "f" + g.G`),
+ filepath.Join(dir, "g", "g.go"): []byte(`package g; import "golang.org/fake/h"; const G = "g" + h.H`),
+ filepath.Join(dir, "h", "h.go"): []byte(`package h; const H = "h"`),
+ },
+ `"efgh_"`},
+ } {
+ exported.Config.Overlay = test.overlay
+ exported.Config.Mode = packages.LoadAllSyntax
+ initial, err := packages.Load(exported.Config, "golang.org/fake/e")
+ if err != nil {
+ t.Error(err)
+ continue
+ }
+
+ // Check value of e.E.
+ e := initial[0]
+ eE := constant(e, "E")
+ if eE == nil {
+ t.Errorf("%d. e.E: got nil", i)
+ continue
+ }
+ got := eE.Val().String()
+ if got != test.want {
+ t.Errorf("%d. e.E: got %s, want %s", i, got, test.want)
+ }
+ }
+}
+
func TestLoadAllSyntaxImportErrors(t *testing.T) {
packagestest.TestAll(t, testLoadAllSyntaxImportErrors)
}
diff --git a/go/packages/packagestest/expect.go b/go/packages/packagestest/expect.go
index 3a1fa3f..3a5d88e 100644
--- a/go/packages/packagestest/expect.go
+++ b/go/packages/packagestest/expect.go
@@ -141,6 +141,9 @@
dirs = append(dirs, filepath.Dir(filename))
}
}
+ for filename := range e.Config.Overlay {
+ dirs = append(dirs, filepath.Dir(filename))
+ }
pkgs, err := packages.Load(e.Config, dirs...)
if err != nil {
return fmt.Errorf("unable to load packages for directories %s: %v", dirs, err)
diff --git a/internal/lsp/cache/file.go b/internal/lsp/cache/file.go
index db16db6..9efcccb 100644
--- a/internal/lsp/cache/file.go
+++ b/internal/lsp/cache/file.go
@@ -112,9 +112,15 @@
return
}
}
+ // We might have the content saved in an overlay.
+ if content, ok := f.view.Config.Overlay[f.filename]; ok {
+ f.content = content
+ return
+ }
// We don't know the content yet, so read it.
content, err := ioutil.ReadFile(f.filename)
if err != nil {
+ f.view.Logger().Errorf(ctx, "unable to read file %s: %v", f.filename, err)
return
}
f.content = content
diff --git a/internal/lsp/lsp_test.go b/internal/lsp/lsp_test.go
index d6c419a..e836861 100644
--- a/internal/lsp/lsp_test.go
+++ b/internal/lsp/lsp_test.go
@@ -18,6 +18,7 @@
"strings"
"testing"
+ "golang.org/x/tools/go/packages"
"golang.org/x/tools/go/packages/packagestest"
"golang.org/x/tools/internal/lsp/cache"
"golang.org/x/tools/internal/lsp/diff"
@@ -37,7 +38,7 @@
// We hardcode the expected number of test cases to ensure that all tests
// are being executed. If a test is added, this number must be changed.
- const expectedCompletionsCount = 64
+ const expectedCompletionsCount = 65
const expectedDiagnosticsCount = 16
const expectedFormatCount = 4
const expectedDefinitionsCount = 17
@@ -62,7 +63,10 @@
defer exported.Cleanup()
// Merge the exported.Config with the view.Config.
+ addUnsavedFiles(t, exported.Config, exported)
+
cfg := *exported.Config
+
cfg.Fset = token.NewFileSet()
cfg.Context = context.Background()
cfg.ParseFile = func(fset *token.FileSet, filename string, src []byte) (*ast.File, error) {
@@ -174,6 +178,25 @@
})
}
+func addUnsavedFiles(t *testing.T, cfg *packages.Config, exported *packagestest.Exported) {
+ if cfg.Overlay == nil {
+ cfg.Overlay = make(map[string][]byte)
+ }
+ // For now, we hardcode a file that we know is in the testdata.
+ // TODO(rstambler): Figure out a way to do this better.
+ dir := filepath.Dir(filepath.Dir(exported.File("golang.org/x/tools/internal/lsp", filepath.Join("complit", "complit.go"))))
+ cfg.Overlay[filepath.Join(dir, "nodisk", "nodisk.go")] = []byte(`package nodisk
+
+import (
+ "golang.org/x/tools/internal/lsp/foo"
+)
+
+func _() {
+ foo.Foo() //@complete("F", Foo, IntFoo, StructFoo)
+}
+`)
+}
+
type diagnostics map[span.URI][]protocol.Diagnostic
type completionItems map[token.Pos]*protocol.CompletionItem
type completions map[token.Position][]token.Pos
diff --git a/internal/lsp/testdata/nodisk/empty b/internal/lsp/testdata/nodisk/empty
new file mode 100644
index 0000000..2cecac2
--- /dev/null
+++ b/internal/lsp/testdata/nodisk/empty
@@ -0,0 +1 @@
+an empty file so that the directory exists
\ No newline at end of file
diff --git a/internal/span/utf16.go b/internal/span/utf16.go
index e1f5dd8..0d356e2 100644
--- a/internal/span/utf16.go
+++ b/internal/span/utf16.go
@@ -56,6 +56,9 @@
if chr <= 1 {
return p, nil
}
+ if p.Offset() >= len(content) {
+ return p, fmt.Errorf("offset (%v) greater than length of content (%v)", p.Offset(), len(content))
+ }
remains := content[p.Offset():]
// scan forward the specified number of characters
for count := 1; count < chr; count++ {
