Skip to content

Commit

Permalink
internal/lsp: exclude the module cache from the workspace
Browse files Browse the repository at this point in the history
This change treats the GOMODCACHE like a directory filter, and it
excludes any modules under the module cache from being considered part
of the workspace. This can happen when users open their entire GOPATH.

To do this, I had to propagate the view's root and gomodcache through
the exclusion functions, and I also had to add BuildGoplsMod to the
snapshot's interface.

Change-Id: Id80b359d73d09a525b380389917451e85357b784
Reviewed-on: https://go-review.googlesource.com/c/tools/+/326816
Trust: Rebecca Stambler <rstambler@golang.org>
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
gopls-CI: kokoro <noreply+kokoro@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Robert Findley <rfindley@google.com>
  • Loading branch information
stamblerre committed Jun 11, 2021
1 parent 9a55cb1 commit 4b484fb
Show file tree
Hide file tree
Showing 8 changed files with 59 additions and 15 deletions.
38 changes: 38 additions & 0 deletions gopls/internal/regtest/workspace/workspace_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -913,3 +913,41 @@ func main() {
)
})
}

// Sometimes users may have their module cache within the workspace.
// We shouldn't consider any module in the module cache to be in the workspace.
func TestGOMODCACHEInWorkspace(t *testing.T) {
const mod = `
-- a/go.mod --
module a.com
go 1.12
-- a/a.go --
package a
func _() {}
-- a/c/c.go --
package c
-- gopath/src/b/b.go --
package b
-- gopath/pkg/mod/example.com/go.mod --
module example.com
go 1.12
-- gopath/pkg/mod/example.com/main.go --
package main
`
WithOptions(
EditorConfig{Env: map[string]string{
"GOPATH": filepath.FromSlash("$SANDBOX_WORKDIR/gopath"),
}},
Modes(Singleton),
).Run(t, mod, func(t *testing.T, env *Env) {
env.Await(
// Confirm that the build configuration is seen as valid,
// even though there are technically multiple go.mod files in the
// worskpace.
LogMatching(protocol.Info, ".*valid build configuration = true.*", 1, false),
)
})
}
4 changes: 2 additions & 2 deletions internal/lsp/cache/session.go
Original file line number Diff line number Diff line change
Expand Up @@ -173,14 +173,14 @@ func (s *Session) createView(ctx context.Context, name string, folder, tempWorks
}
root := folder
if options.ExpandWorkspaceToModule {
root, err = findWorkspaceRoot(ctx, root, s, pathExcludedByFilterFunc(options), options.ExperimentalWorkspaceModule)
root, err = findWorkspaceRoot(ctx, root, s, pathExcludedByFilterFunc(root.Filename(), ws.gomodcache, options), options.ExperimentalWorkspaceModule)
if err != nil {
return nil, nil, func() {}, err
}
}

// Build the gopls workspace, collecting active modules in the view.
workspace, err := newWorkspace(ctx, root, s, pathExcludedByFilterFunc(options), ws.userGo111Module == off, options.ExperimentalWorkspaceModule)
workspace, err := newWorkspace(ctx, root, s, pathExcludedByFilterFunc(root.Filename(), ws.gomodcache, options), ws.userGo111Module == off, options.ExperimentalWorkspaceModule)
if err != nil {
return nil, nil, func() {}, err
}
Expand Down
4 changes: 2 additions & 2 deletions internal/lsp/cache/snapshot.go
Original file line number Diff line number Diff line change
Expand Up @@ -1905,8 +1905,8 @@ func (s *snapshot) setBuiltin(path string) {

// BuildGoplsMod generates a go.mod file for all modules in the workspace. It
// bypasses any existing gopls.mod.
func BuildGoplsMod(ctx context.Context, root span.URI, s source.Snapshot) (*modfile.File, error) {
allModules, err := findModules(root, pathExcludedByFilterFunc(s.View().Options()), 0)
func (s *snapshot) BuildGoplsMod(ctx context.Context) (*modfile.File, error) {
allModules, err := findModules(s.view.folder, pathExcludedByFilterFunc(s.view.rootURI.Filename(), s.view.gomodcache, s.View().Options()), 0)
if err != nil {
return nil, err
}
Expand Down
15 changes: 8 additions & 7 deletions internal/lsp/cache/view.go
Original file line number Diff line number Diff line change
Expand Up @@ -343,7 +343,7 @@ func (s *snapshot) locateTemplateFiles(ctx context.Context) {
if err != nil {
return err
}
if strings.HasSuffix(filepath.Ext(path), "tmpl") && !pathExcludedByFilter(path, s.view.options) &&
if strings.HasSuffix(filepath.Ext(path), "tmpl") && !pathExcludedByFilter(path, dir, s.view.gomodcache, s.view.options) &&
!fi.IsDir() {
k := span.URIFromPath(path)
fh, err := s.GetVersionedFile(ctx, k)
Expand Down Expand Up @@ -371,7 +371,7 @@ func (v *View) contains(uri span.URI) bool {
}
// Filters are applied relative to the workspace folder.
if inFolder {
return !pathExcludedByFilter(strings.TrimPrefix(uri.Filename(), v.folder.Filename()), v.Options())
return !pathExcludedByFilter(strings.TrimPrefix(uri.Filename(), v.folder.Filename()), v.rootURI.Filename(), v.gomodcache, v.Options())
}
return true
}
Expand Down Expand Up @@ -1017,24 +1017,25 @@ func (v *View) allFilesExcluded(pkg *packages.Package) bool {
if !strings.HasPrefix(f, folder) {
return false
}
if !pathExcludedByFilter(strings.TrimPrefix(f, folder), opts) {
if !pathExcludedByFilter(strings.TrimPrefix(f, folder), v.rootURI.Filename(), v.gomodcache, opts) {
return false
}
}
return true
}

func pathExcludedByFilterFunc(opts *source.Options) func(string) bool {
func pathExcludedByFilterFunc(root, gomodcache string, opts *source.Options) func(string) bool {
return func(path string) bool {
return pathExcludedByFilter(path, opts)
return pathExcludedByFilter(path, root, gomodcache, opts)
}
}

func pathExcludedByFilter(path string, opts *source.Options) bool {
func pathExcludedByFilter(path, root, gomodcache string, opts *source.Options) bool {
path = strings.TrimPrefix(filepath.ToSlash(path), "/")
gomodcache = strings.TrimPrefix(filepath.ToSlash(strings.TrimPrefix(gomodcache, root)), "/")

excluded := false
for _, filter := range opts.DirectoryFilters {
for _, filter := range append(opts.DirectoryFilters, "-"+gomodcache) {
op, prefix := filter[0], filter[1:]
// Non-empty prefixes have to be precise directory matches.
if prefix != "" {
Expand Down
4 changes: 2 additions & 2 deletions internal/lsp/cache/view_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -161,12 +161,12 @@ func TestFilters(t *testing.T) {
opts := &source.Options{}
opts.DirectoryFilters = tt.filters
for _, inc := range tt.included {
if pathExcludedByFilter(inc, opts) {
if pathExcludedByFilter(inc, "root", "root/gopath/pkg/mod", opts) {
t.Errorf("filters %q excluded %v, wanted included", tt.filters, inc)
}
}
for _, exc := range tt.excluded {
if !pathExcludedByFilter(exc, opts) {
if !pathExcludedByFilter(exc, "root", "root/gopath/pkg/mod", opts) {
t.Errorf("filters %q included %v, wanted excluded", tt.filters, exc)
}
}
Expand Down
3 changes: 1 addition & 2 deletions internal/lsp/command.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,6 @@ import (
"golang.org/x/mod/modfile"
"golang.org/x/tools/internal/event"
"golang.org/x/tools/internal/gocommand"
"golang.org/x/tools/internal/lsp/cache"
"golang.org/x/tools/internal/lsp/command"
"golang.org/x/tools/internal/lsp/debug"
"golang.org/x/tools/internal/lsp/protocol"
Expand Down Expand Up @@ -647,7 +646,7 @@ func (c *commandHandler) GenerateGoplsMod(ctx context.Context, args command.URIA
v := views[0]
snapshot, release := v.Snapshot(ctx)
defer release()
modFile, err := cache.BuildGoplsMod(ctx, snapshot.View().Folder(), snapshot)
modFile, err := snapshot.BuildGoplsMod(ctx)
if err != nil {
return errors.Errorf("getting workspace mod file: %w", err)
}
Expand Down
2 changes: 2 additions & 0 deletions internal/lsp/fake/editor.go
Original file line number Diff line number Diff line change
Expand Up @@ -206,9 +206,11 @@ func (e *Editor) Client() *Client {
func (e *Editor) overlayEnv() map[string]string {
env := make(map[string]string)
for k, v := range e.defaultEnv {
v = strings.ReplaceAll(v, "$SANDBOX_WORKDIR", e.sandbox.Workdir.RootURI().SpanURI().Filename())
env[k] = v
}
for k, v := range e.Config.Env {
v = strings.ReplaceAll(v, "$SANDBOX_WORKDIR", e.sandbox.Workdir.RootURI().SpanURI().Filename())
env[k] = v
}
return env
Expand Down
4 changes: 4 additions & 0 deletions internal/lsp/source/view.go
Original file line number Diff line number Diff line change
Expand Up @@ -160,6 +160,10 @@ type Snapshot interface {

// GetCriticalError returns any critical errors in the workspace.
GetCriticalError(ctx context.Context) *CriticalError

// BuildGoplsMod generates a go.mod file for all modules in the workspace.
// It bypasses any existing gopls.mod.
BuildGoplsMod(ctx context.Context) (*modfile.File, error)
}

// PackageFilter sets how a package is filtered out from a set of packages
Expand Down

0 comments on commit 4b484fb

Please sign in to comment.