diff --git a/cmd/govim/internal/golang_org_x_tools/imports/fix.go b/cmd/govim/internal/golang_org_x_tools/imports/fix.go index edaf0eb22..3ef4f2f03 100644 --- a/cmd/govim/internal/golang_org_x_tools/imports/fix.go +++ b/cmd/govim/internal/golang_org_x_tools/imports/fix.go @@ -827,7 +827,11 @@ func (e *ProcessEnv) goEnv() (map[string]string, error) { } func (e *ProcessEnv) matchFile(dir, name string) (bool, error) { - return build.Default.MatchFile(dir, name) + bctx, err := e.buildContext() + if err != nil { + return false, err + } + return bctx.MatchFile(dir, name) } // CopyConfig copies the env's configuration into a new env. @@ -924,6 +928,17 @@ func (e *ProcessEnv) buildContext() (*build.Context, error) { dir.SetString(e.WorkingDir) } + // Since Go 1.11, go/build.Context.Import may invoke 'go list' depending on + // the value in GO111MODULE in the process's environment. We always want to + // run in GOPATH mode when calling Import, so we need to prevent this from + // happening. In Go 1.16, GO111MODULE defaults to "on", so this problem comes + // up more frequently. + // + // HACK: setting any of the Context I/O hooks prevents Import from invoking + // 'go list', regardless of GO111MODULE. This is undocumented, but it's + // unlikely to change before GOPATH support is removed. + ctx.ReadDir = ioutil.ReadDir + return &ctx, nil } diff --git a/cmd/govim/internal/golang_org_x_tools/lsp/cache/check.go b/cmd/govim/internal/golang_org_x_tools/lsp/cache/check.go index 6c97ce885..12ebffd98 100644 --- a/cmd/govim/internal/golang_org_x_tools/lsp/cache/check.go +++ b/cmd/govim/internal/golang_org_x_tools/lsp/cache/check.go @@ -177,8 +177,7 @@ func checkPackageKey(ctx context.Context, id packageID, pghs []*parseGoHandle, c b.WriteString(string(dep)) } for _, cgf := range pghs { - b.WriteString(string(cgf.file.URI())) - b.WriteString(cgf.file.FileIdentity().Hash) + b.WriteString(cgf.file.FileIdentity().String()) } return packageHandleKey(hashContents(b.Bytes())) } diff --git a/cmd/govim/internal/golang_org_x_tools/lsp/cache/load.go b/cmd/govim/internal/golang_org_x_tools/lsp/cache/load.go index d7d28a389..41cd7ce3c 100644 --- a/cmd/govim/internal/golang_org_x_tools/lsp/cache/load.go +++ b/cmd/govim/internal/golang_org_x_tools/lsp/cache/load.go @@ -199,13 +199,18 @@ func (s *snapshot) load(ctx context.Context, scopes ...interface{}) error { // packages.Loads that occur from within the workspace module. func (s *snapshot) tempWorkspaceModule(ctx context.Context) (_ span.URI, cleanup func(), err error) { cleanup = func() {} - if len(s.view.modules) == 0 { + if len(s.modules) == 0 { return "", cleanup, nil } - if s.view.workspaceModule == nil { - return "", cleanup, nil + wsModuleHandle, err := s.getWorkspaceModuleHandle(ctx) + if err != nil { + return "", nil, err + } + file, err := wsModuleHandle.build(ctx, s) + if err != nil { + return "", nil, err } - content, err := s.view.workspaceModule.Format() + content, err := file.Format() if err != nil { return "", cleanup, err } diff --git a/cmd/govim/internal/golang_org_x_tools/lsp/cache/session.go b/cmd/govim/internal/golang_org_x_tools/lsp/cache/session.go index 0d2252700..b74372b95 100644 --- a/cmd/govim/internal/golang_org_x_tools/lsp/cache/session.go +++ b/cmd/govim/internal/golang_org_x_tools/lsp/cache/session.go @@ -7,8 +7,6 @@ package cache import ( "context" "fmt" - "os" - "path/filepath" "strconv" "strings" "sync" @@ -173,7 +171,6 @@ func (s *Session) createView(ctx context.Context, name string, folder span.URI, name: name, folder: folder, root: folder, - modules: make(map[span.URI]*moduleRoot), filesByURI: make(map[span.URI]*fileBase), filesByBase: make(map[string][]*fileBase), } @@ -194,6 +191,7 @@ func (s *Session) createView(ctx context.Context, name string, folder span.URI, modTidyHandles: make(map[span.URI]*modTidyHandle), modUpgradeHandles: make(map[span.URI]*modUpgradeHandle), modWhyHandles: make(map[span.URI]*modWhyHandle), + modules: make(map[span.URI]*moduleRoot), } if v.session.cache.options != nil { @@ -206,7 +204,7 @@ func (s *Session) createView(ctx context.Context, name string, folder span.URI, } // Find all of the modules in the workspace. - if err := v.findWorkspaceModules(ctx, options); err != nil { + if err := v.snapshot.findWorkspaceModules(ctx, options); err != nil { return nil, nil, func() {}, err } @@ -214,11 +212,9 @@ func (s *Session) createView(ctx context.Context, name string, folder span.URI, // check if the view has a valid build configuration. v.setBuildConfiguration() - // Build the workspace module, if needed. - if options.ExperimentalWorkspaceModule { - if err := v.buildWorkspaceModule(ctx); err != nil { - return nil, nil, func() {}, err - } + // Decide if we should use the workspace module. + if v.determineWorkspaceModuleLocked() { + v.workspaceMode |= usesWorkspaceModule | moduleMode } // We have v.goEnv now. @@ -242,94 +238,12 @@ func (s *Session) createView(ctx context.Context, name string, folder span.URI, snapshot := v.snapshot release := snapshot.generation.Acquire(initCtx) go func() { - v.initialize(initCtx, snapshot, true) + snapshot.initialize(initCtx, true) release() }() return v, snapshot, snapshot.generation.Acquire(ctx), nil } -// findWorkspaceModules walks the view's root folder, looking for go.mod files. -// Any that are found are added to the view's set of modules, which are then -// used to construct the workspace module. -// -// It assumes that the caller has not yet created the view, and therefore does -// not lock any of the internal data structures before accessing them. -func (v *View) findWorkspaceModules(ctx context.Context, options *source.Options) error { - // If the user is intentionally limiting their workspace scope, add their - // folder to the roots and return early. - if !options.ExpandWorkspaceToModule { - return nil - } - // The workspace module has been disabled by the user. - if !options.ExperimentalWorkspaceModule { - return nil - } - - // Walk the view's folder to find all modules in the view. - root := v.root.Filename() - return filepath.Walk(root, func(path string, info os.FileInfo, err error) error { - if err != nil { - return err - } - // For any path that is not the workspace folder, check if the path - // would be ignored by the go command. Vendor directories also do not - // contain workspace modules. - if info.IsDir() && path != root { - suffix := strings.TrimPrefix(path, root) - switch { - case checkIgnored(suffix), - strings.Contains(filepath.ToSlash(suffix), "/vendor/"): - return filepath.SkipDir - } - } - // We're only interested in go.mod files. - if filepath.Base(path) != "go.mod" { - return nil - } - // At this point, we definitely have a go.mod file in the workspace, - // so add it to the view. - modURI := span.URIFromPath(path) - rootURI := span.URIFromPath(filepath.Dir(path)) - v.modules[rootURI] = &moduleRoot{ - rootURI: rootURI, - modURI: modURI, - sumURI: span.URIFromPath(sumFilename(modURI)), - } - return nil - }) -} - -func (v *View) buildWorkspaceModule(ctx context.Context) error { - // If the view has an invalid configuration, don't build the workspace - // module. - if !v.hasValidBuildConfiguration { - return nil - } - // If the view is not in a module and contains no modules, but still has a - // valid workspace configuration, do not create the workspace module. - // It could be using GOPATH or a different build system entirely. - if v.modURI == "" && len(v.modules) == 0 && v.hasValidBuildConfiguration { - return nil - } - v.workspaceMode |= moduleMode - - // Don't default to multi-workspace mode if one of the modules contains a - // vendor directory. We still have to decide how to handle vendoring. - for _, mod := range v.modules { - if info, _ := os.Stat(filepath.Join(mod.rootURI.Filename(), "vendor")); info != nil { - return nil - } - } - - v.workspaceMode |= usesWorkspaceModule - - // If the user does not have a gopls.mod, we need to create one, based on - // modules we found in the user's workspace. - var err error - v.workspaceModule, err = v.snapshot.buildWorkspaceModule(ctx) - return err -} - // View returns the view by name. func (s *Session) View(name string) source.View { s.viewMu.Lock() diff --git a/cmd/govim/internal/golang_org_x_tools/lsp/cache/snapshot.go b/cmd/govim/internal/golang_org_x_tools/lsp/cache/snapshot.go index c7374a170..51880d70d 100644 --- a/cmd/govim/internal/golang_org_x_tools/lsp/cache/snapshot.go +++ b/cmd/govim/internal/golang_org_x_tools/lsp/cache/snapshot.go @@ -94,6 +94,13 @@ type snapshot struct { modTidyHandles map[span.URI]*modTidyHandle modUpgradeHandles map[span.URI]*modUpgradeHandle modWhyHandles map[span.URI]*modWhyHandle + + // modules is the set of modules currently in this workspace. + modules map[span.URI]*moduleRoot + + // workspaceModuleHandle keeps track of the in-memory representation of the + // go.mod file for the workspace module. + workspaceModuleHandle *workspaceModuleHandle } type packageKey struct { @@ -138,7 +145,7 @@ func (s *snapshot) configWithDir(ctx context.Context, dir string) *packages.Conf cfg := &packages.Config{ Context: ctx, Dir: dir, - Env: append([]string{}, env...), + Env: append(append([]string{}, env...), "GO111MODULE="+s.view.go111module), BuildFlags: append([]string{}, buildFlags...), Mode: packages.NeedName | packages.NeedFiles | @@ -673,12 +680,15 @@ func (s *snapshot) GetFile(ctx context.Context, uri span.URI) (source.VersionedF s.mu.Lock() defer s.mu.Unlock() + return s.getFileLocked(ctx, f) +} +func (s *snapshot) getFileLocked(ctx context.Context, f *fileBase) (source.VersionedFileHandle, error) { if fh, ok := s.files[f.URI()]; ok { return fh, nil } - fh, err := s.view.session.cache.getFile(ctx, uri) + fh, err := s.view.session.cache.getFile(ctx, f.URI()) if err != nil { return nil, err } @@ -732,7 +742,7 @@ func (s *snapshot) AwaitInitialized(ctx context.Context) { } // We typically prefer to run something as intensive as the IWL without // blocking. I'm not sure if there is a way to do that here. - s.view.initialize(ctx, s, false) + s.initialize(ctx, false) } // reloadWorkspace reloads the metadata for all invalidated workspace packages. @@ -851,24 +861,26 @@ func (s *snapshot) clone(ctx context.Context, withoutURIs map[span.URI]source.Ve newGen := s.view.session.cache.store.Generation(generationName(s.view, s.id+1)) result := &snapshot{ - id: s.id + 1, - generation: newGen, - view: s.view, - builtin: s.builtin, - ids: make(map[span.URI][]packageID), - importedBy: make(map[packageID][]packageID), - metadata: make(map[packageID]*metadata), - packages: make(map[packageKey]*packageHandle), - actions: make(map[actionKey]*actionHandle), - files: make(map[span.URI]source.VersionedFileHandle), - goFiles: make(map[parseKey]*parseGoHandle), - workspaceDirectories: make(map[span.URI]struct{}), - workspacePackages: make(map[packageID]packagePath), - unloadableFiles: make(map[span.URI]struct{}), - parseModHandles: make(map[span.URI]*parseModHandle), - modTidyHandles: make(map[span.URI]*modTidyHandle), - modUpgradeHandles: make(map[span.URI]*modUpgradeHandle), - modWhyHandles: make(map[span.URI]*modWhyHandle), + id: s.id + 1, + generation: newGen, + view: s.view, + builtin: s.builtin, + ids: make(map[span.URI][]packageID), + importedBy: make(map[packageID][]packageID), + metadata: make(map[packageID]*metadata), + packages: make(map[packageKey]*packageHandle), + actions: make(map[actionKey]*actionHandle), + files: make(map[span.URI]source.VersionedFileHandle), + goFiles: make(map[parseKey]*parseGoHandle), + workspaceDirectories: make(map[span.URI]struct{}), + workspacePackages: make(map[packageID]packagePath), + unloadableFiles: make(map[span.URI]struct{}), + parseModHandles: make(map[span.URI]*parseModHandle), + modTidyHandles: make(map[span.URI]*modTidyHandle), + modUpgradeHandles: make(map[span.URI]*modUpgradeHandle), + modWhyHandles: make(map[span.URI]*modWhyHandle), + modules: make(map[span.URI]*moduleRoot), + workspaceModuleHandle: s.workspaceModuleHandle, } if s.builtin != nil { @@ -885,7 +897,6 @@ func (s *snapshot) clone(ctx context.Context, withoutURIs map[span.URI]source.Ve } // Copy all of the modHandles. for k, v := range s.parseModHandles { - newGen.Inherit(v.handle) result.parseModHandles[k] = v } // Copy all of the workspace directories. They may be reset later. @@ -923,6 +934,11 @@ func (s *snapshot) clone(ctx context.Context, withoutURIs map[span.URI]source.Ve result.modWhyHandles[k] = v } + // Add all of the modules now. They may be deleted or added to later. + for k, v := range s.modules { + result.modules[k] = v + } + // transitiveIDs keeps track of transitive reverse dependencies. // If an ID is present in the map, invalidate its types. // If an ID's value is true, invalidate its metadata too. @@ -957,24 +973,69 @@ func (s *snapshot) clone(ctx context.Context, withoutURIs map[span.URI]source.Ve delete(result.modWhyHandles, k) } } - if currentFH.Kind() == source.Mod { - // If the view's go.mod file's contents have changed, invalidate the - // metadata for every known package in the snapshot. + currentExists := currentFH.URI() != "" + if currentExists { + if _, err := currentFH.Read(); os.IsNotExist(err) { + currentExists = false + } + } + // If the file invalidation is for a go.mod. originalFH is nil if the + // file is newly created. + currentMod := currentExists && currentFH.Kind() == source.Mod + originalMod := originalFH != nil && originalFH.Kind() == source.Mod + if currentMod || originalMod { + // If the view's go.mod file's contents have changed, invalidate + // the metadata for every known package in the snapshot. if invalidateMetadata { for k := range s.packages { directIDs[k.id] = struct{}{} } + // If a go.mod file in the workspace has changed, we need to + // rebuild the workspace module. + result.workspaceModuleHandle = nil } - delete(result.parseModHandles, withoutURI) - if currentFH.URI() == s.view.modURI { - // The go.mod's replace directives may have changed. We may - // need to update our set of workspace directories. Use the new - // snapshot, as it can be locked without causing issues. - result.workspaceDirectories = result.findWorkspaceDirectories(ctx, currentFH) + // Check if this is a newly created go.mod file. When a new module + // is created, we have to retry the initial workspace load. + rootURI := span.URIFromPath(filepath.Dir(withoutURI.Filename())) + if currentMod { + if _, ok := result.modules[rootURI]; !ok { + result.addModule(ctx, currentFH.URI()) + result.view.definitelyReinitialize() + } + } else if originalMod { + // Similarly, we need to retry the IWL if a go.mod in the workspace + // was deleted. + if _, ok := result.modules[rootURI]; ok { + delete(result.modules, rootURI) + result.view.definitelyReinitialize() + } } } + // Keep track of the creations and deletions of go.sum files. + // Creating a go.sum without an associated go.mod has no effect on the + // set of modules. + currentSum := currentExists && currentFH.Kind() == source.Sum + originalSum := originalFH != nil && originalFH.Kind() == source.Sum + if currentSum || originalSum { + rootURI := span.URIFromPath(filepath.Dir(withoutURI.Filename())) + if currentSum { + if mod, ok := result.modules[rootURI]; ok { + mod.sumURI = currentFH.URI() + } + } else if originalSum { + if mod, ok := result.modules[rootURI]; ok { + mod.sumURI = "" + } + } + } + if withoutURI == s.view.modURI { + // The go.mod's replace directives may have changed. We may + // need to update our set of workspace directories. Use the new + // snapshot, as it can be locked without causing issues. + result.workspaceDirectories = result.findWorkspaceDirectories(ctx, currentFH) + } // If this is a file we don't yet know about, // then we do not yet know what packages it should belong to. @@ -1017,7 +1078,7 @@ func (s *snapshot) clone(ctx context.Context, withoutURIs map[span.URI]source.Ve } // Handle the invalidated file; it may have new contents or not exist. - if _, err := currentFH.Read(); os.IsNotExist(err) { + if !currentExists { delete(result.files, withoutURI) } else { result.files[withoutURI] = currentFH @@ -1087,15 +1148,21 @@ copyIDs: } // Inherit all of the go.mod-related handles. - for _, v := range s.modTidyHandles { + for _, v := range result.modTidyHandles { + newGen.Inherit(v.handle) + } + for _, v := range result.modUpgradeHandles { newGen.Inherit(v.handle) } - for _, v := range s.modUpgradeHandles { + for _, v := range result.modWhyHandles { newGen.Inherit(v.handle) } - for _, v := range s.modWhyHandles { + for _, v := range result.parseModHandles { newGen.Inherit(v.handle) } + if result.workspaceModuleHandle != nil { + newGen.Inherit(result.workspaceModuleHandle.handle) + } // Don't bother copying the importedBy graph, // as it changes each time we update metadata. @@ -1128,9 +1195,9 @@ func (s *snapshot) shouldInvalidateMetadata(ctx context.Context, newSnapshot *sn if originalFH.FileIdentity() == currentFH.FileIdentity() { return false } - // If a go.mod file's contents have changed, always invalidate metadata. + // If a go.mod in the workspace has been changed, invalidate metadata. if kind := originalFH.Kind(); kind == source.Mod { - return originalFH.URI() == s.view.modURI + return isSubdirectory(filepath.Dir(s.view.root.Filename()), filepath.Dir(originalFH.URI().Filename())) } // Get the original and current parsed files in order to check package name // and imports. Use the new snapshot to parse to avoid modifying the @@ -1263,6 +1330,64 @@ func (s *snapshot) buildBuiltinPackage(ctx context.Context, goFiles []string) er return nil } +type workspaceModuleHandle struct { + handle *memoize.Handle +} + +type workspaceModuleData struct { + file *modfile.File + err error +} + +type workspaceModuleKey string + +func (wmh *workspaceModuleHandle) build(ctx context.Context, snapshot *snapshot) (*modfile.File, error) { + v, err := wmh.handle.Get(ctx, snapshot.generation, snapshot) + if err != nil { + return nil, err + } + data := v.(*workspaceModuleData) + return data.file, data.err +} + +func (s *snapshot) getWorkspaceModuleHandle(ctx context.Context) (*workspaceModuleHandle, error) { + s.mu.Lock() + wsModule := s.workspaceModuleHandle + s.mu.Unlock() + if wsModule != nil { + return wsModule, nil + } + var fhs []source.FileHandle + for _, mod := range s.modules { + fh, err := s.GetFile(ctx, mod.modURI) + if err != nil { + return nil, err + } + fhs = append(fhs, fh) + } + sort.Slice(fhs, func(i, j int) bool { + return fhs[i].URI() < fhs[j].URI() + }) + var k string + for _, fh := range fhs { + k += fh.FileIdentity().String() + } + key := workspaceModuleKey(hashContents([]byte(k))) + h := s.generation.Bind(key, func(ctx context.Context, arg memoize.Arg) interface{} { + s := arg.(*snapshot) + data := &workspaceModuleData{} + data.file, data.err = s.buildWorkspaceModule(ctx) + return data + }) + wsModule = &workspaceModuleHandle{ + handle: h, + } + s.mu.Lock() + defer s.mu.Unlock() + s.workspaceModuleHandle = wsModule + return s.workspaceModuleHandle, nil +} + // buildWorkspaceModule generates a workspace module given the modules in the // the workspace. func (s *snapshot) buildWorkspaceModule(ctx context.Context) (*modfile.File, error) { @@ -1270,8 +1395,8 @@ func (s *snapshot) buildWorkspaceModule(ctx context.Context) (*modfile.File, err file.AddModuleStmt("gopls-workspace") paths := make(map[string]*moduleRoot) - for _, mod := range s.view.modules { - fh, err := s.view.snapshot.GetFile(ctx, mod.modURI) + for _, mod := range s.modules { + fh, err := s.GetFile(ctx, mod.modURI) if err != nil { return nil, err } @@ -1291,12 +1416,12 @@ func (s *snapshot) buildWorkspaceModule(ctx context.Context) (*modfile.File, err } // Go back through all of the modules to handle any of their replace // statements. - for _, module := range s.view.modules { - fh, err := s.view.snapshot.GetFile(ctx, module.modURI) + for _, module := range s.modules { + fh, err := s.GetFile(ctx, module.modURI) if err != nil { return nil, err } - pmf, err := s.view.snapshot.ParseMod(ctx, fh) + pmf, err := s.ParseMod(ctx, fh) if err != nil { return nil, err } @@ -1326,3 +1451,52 @@ func (s *snapshot) buildWorkspaceModule(ctx context.Context) (*modfile.File, err } return file, nil } + +// findWorkspaceModules walks the view's root folder, looking for go.mod +// files. Any that are found are added to the view's set of modules, which are +// then used to construct the workspace module. +// +// It assumes that the caller has not yet created the view, and therefore does +// not lock any of the internal data structures before accessing them. +func (s *snapshot) findWorkspaceModules(ctx context.Context, options *source.Options) error { + // Walk the view's folder to find all modules in the view. + root := s.view.root.Filename() + return filepath.Walk(root, func(path string, info os.FileInfo, err error) error { + if err != nil { + return err + } + // For any path that is not the workspace folder, check if the path + // would be ignored by the go command. Vendor directories also do not + // contain workspace modules. + if info.IsDir() && path != root { + suffix := strings.TrimPrefix(path, root) + switch { + case checkIgnored(suffix), + strings.Contains(filepath.ToSlash(suffix), "/vendor/"): + return filepath.SkipDir + } + } + // We're only interested in go.mod files. + if filepath.Base(path) != "go.mod" { + return nil + } + // At this point, we definitely have a go.mod file in the workspace, + // so add it to the view. + modURI := span.URIFromPath(path) + s.addModule(ctx, modURI) + return nil + }) +} + +func (s *snapshot) addModule(ctx context.Context, modURI span.URI) { + rootURI := span.URIFromPath(filepath.Dir(modURI.Filename())) + sumURI := span.URIFromPath(sumFilename(modURI)) + if info, _ := os.Stat(sumURI.Filename()); info == nil { + sumURI = "" + } + s.modules[rootURI] = &moduleRoot{ + rootURI: rootURI, + modURI: modURI, + sumURI: sumURI, + } +} diff --git a/cmd/govim/internal/golang_org_x_tools/lsp/cache/view.go b/cmd/govim/internal/golang_org_x_tools/lsp/cache/view.go index bfcaf776f..f85aa6af7 100644 --- a/cmd/govim/internal/golang_org_x_tools/lsp/cache/view.go +++ b/cmd/govim/internal/golang_org_x_tools/lsp/cache/view.go @@ -67,16 +67,6 @@ type View struct { // is just the folder. If we are in module mode, this is the module root. root span.URI - // TODO: The modules and workspaceModule fields should probably be moved to - // the snapshot and invalidated on file changes. - - // modules is the set of modules currently in this workspace. - modules map[span.URI]*moduleRoot - - // workspaceModule is an in-memory representation of the go.mod file for - // the workspace module. - workspaceModule *modfile.File - // importsMu guards imports-related state, particularly the ProcessEnv. importsMu sync.Mutex @@ -150,6 +140,9 @@ type View struct { // `go env` variables that need to be tracked by gopls. gocache, gomodcache, gopath, goprivate string + // The value of GO111MODULE we want to run with. + go111module string + // goEnv is the `go env` output collected when a view is created. // It includes the values of the environment variables above. goEnv map[string]string @@ -492,6 +485,8 @@ func (v *View) populateProcessEnv(ctx context.Context, modFH, sumFH source.FileH for k, v := range v.goEnv { pe.Env[k] = v } + pe.Env["GO111MODULE"] = v.go111module + modmod, err := v.needsModEqualsMod(ctx, modFH) if err != nil { return cleanup, err @@ -682,42 +677,42 @@ func (v *View) Snapshot(ctx context.Context) (source.Snapshot, func()) { return v.snapshot, v.snapshot.generation.Acquire(ctx) } -func (v *View) initialize(ctx context.Context, s *snapshot, firstAttempt bool) { +func (s *snapshot) initialize(ctx context.Context, firstAttempt bool) { select { case <-ctx.Done(): return - case v.initializationSema <- struct{}{}: + case s.view.initializationSema <- struct{}{}: } defer func() { - <-v.initializationSema + <-s.view.initializationSema }() - if v.initializeOnce == nil { + if s.view.initializeOnce == nil { return } - v.initializeOnce.Do(func() { + s.view.initializeOnce.Do(func() { defer func() { - v.initializeOnce = nil + s.view.initializeOnce = nil if firstAttempt { - close(v.initialized) + close(s.view.initialized) } }() // If we have multiple modules, we need to load them by paths. var scopes []interface{} - if len(v.modules) > 0 { + if len(s.modules) > 0 { // TODO(rstambler): Retry the initial workspace load for whichever // modules we failed to load. - for _, mod := range v.modules { + for _, mod := range s.modules { fh, err := s.GetFile(ctx, mod.modURI) if err != nil { - v.initializedErr = err + s.view.initializedErr = err continue } parsed, err := s.ParseMod(ctx, fh) if err != nil { - v.initializedErr = err + s.view.initializedErr = err continue } path := parsed.File.Module.Mod.Path @@ -733,7 +728,7 @@ func (v *View) initialize(ctx context.Context, s *snapshot, firstAttempt bool) { if err != nil { event.Error(ctx, "initial workspace load failed", err) } - v.initializedErr = err + s.view.initializedErr = err }) } @@ -774,12 +769,20 @@ func (v *View) cancelBackground() { } func (v *View) maybeReinitialize() { + v.reinitialize(false) +} + +func (v *View) definitelyReinitialize() { + v.reinitialize(true) +} + +func (v *View) reinitialize(force bool) { v.initializationSema <- struct{}{} defer func() { <-v.initializationSema }() - if v.initializedErr == nil { + if !force && v.initializedErr == nil { return } var once sync.Once @@ -795,8 +798,25 @@ func (v *View) setBuildInformation(ctx context.Context, options *source.Options) if err != nil { return err } + + v.go111module = os.Getenv("GO111MODULE") + for _, kv := range options.Env { + split := strings.SplitN(kv, "=", 2) + if len(split) != 2 { + continue + } + if split[0] == "GO111MODULE" { + v.go111module = split[1] + } + } + // If using 1.16, change the default back to auto. The primary effect of + // GO111MODULE=on is to break GOPATH, which we aren't too interested in. + if v.goversion >= 16 && v.go111module == "" { + v.go111module = "auto" + } + // Make sure to get the `go env` before continuing with initialization. - modFile, err := v.setGoEnv(ctx, options.Env) + modFile, err := v.setGoEnv(ctx, append(options.Env, "GO111MODULE="+v.go111module)) if err != nil { return err } @@ -834,6 +854,38 @@ func defaultCheckPathCase(path string) error { return nil } +func (v *View) determineWorkspaceModuleLocked() bool { + // If the user is intentionally limiting their workspace scope, add their + // folder to the roots and return early. + if !v.options.ExpandWorkspaceToModule { + return false + } + // The workspace module has been disabled by the user. + if !v.options.ExperimentalWorkspaceModule { + return false + } + // If the view has an invalid configuration, don't build the workspace + // module. + if !v.hasValidBuildConfiguration { + return false + } + // If the view is not in a module and contains no modules, but still has a + // valid workspace configuration, do not create the workspace module. + // It could be using GOPATH or a different build system entirely. + if v.modURI == "" && len(v.snapshot.modules) == 0 && v.hasValidBuildConfiguration { + return false + } + + // Don't default to multi-workspace mode if one of the modules contains a + // vendor directory. We still have to decide how to handle vendoring. + for _, mod := range v.snapshot.modules { + if info, _ := os.Stat(filepath.Join(mod.rootURI.Filename(), "vendor")); info != nil { + return false + } + } + return true +} + func (v *View) setBuildConfiguration() (isValid bool) { defer func() { v.hasValidBuildConfiguration = isValid @@ -848,7 +900,7 @@ func (v *View) setBuildConfiguration() (isValid bool) { if v.modURI != "" { return true } - if len(v.modules) > 0 { + if len(v.snapshot.modules) > 0 { return true } // The user may have a multiple directories in their GOPATH. diff --git a/cmd/govim/internal/golang_org_x_tools/lsp/command.go b/cmd/govim/internal/golang_org_x_tools/lsp/command.go index b7b565f98..ce68b7b3c 100644 --- a/cmd/govim/internal/golang_org_x_tools/lsp/command.go +++ b/cmd/govim/internal/golang_org_x_tools/lsp/command.go @@ -42,6 +42,10 @@ func (s *Server) executeCommand(ctx context.Context, params *protocol.ExecuteCom if !match { return nil, fmt.Errorf("%s is not a supported command", command.Name) } + title := command.Title + if title == "" { + title = command.Name + } // Some commands require that all files are saved to disk. If we detect // unsaved files, warn the user instead of running the commands. unsaved := false @@ -55,50 +59,19 @@ func (s *Server) executeCommand(ctx context.Context, params *protocol.ExecuteCom switch params.Command { case source.CommandTest.Name, source.CommandGenerate.Name, source.CommandToggleDetails.Name: // TODO(PJW): for Toggle, not an error if it is being disabled - err := fmt.Errorf("cannot run command %s: unsaved files in the view", params.Command) - s.client.ShowMessage(ctx, &protocol.ShowMessageParams{ - Type: protocol.Error, - Message: err.Error(), - }) + err := errors.New("unsaved files in the view") + s.showCommandError(ctx, title, err) return nil, err } } // If the command has a suggested fix function available, use it and apply // the edits to the workspace. if command.IsSuggestedFix() { - var uri protocol.DocumentURI - var rng protocol.Range - if err := source.UnmarshalArgs(params.Arguments, &uri, &rng); err != nil { - return nil, err - } - snapshot, fh, ok, release, err := s.beginFileRequest(ctx, uri, source.Go) - defer release() - if !ok { - return nil, err - } - edits, err := command.SuggestedFix(ctx, snapshot, fh, rng) + err := s.runSuggestedFixCommand(ctx, command, params.Arguments) if err != nil { - return nil, err - } - r, err := s.client.ApplyEdit(ctx, &protocol.ApplyWorkspaceEditParams{ - Edit: protocol.WorkspaceEdit{ - DocumentChanges: edits, - }, - }) - if err != nil { - return nil, err + s.showCommandError(ctx, title, err) } - if !r.Applied { - return nil, s.client.ShowMessage(ctx, &protocol.ShowMessageParams{ - Type: protocol.Error, - Message: fmt.Sprintf("%s failed: %v", params.Command, r.FailureReason), - }) - } - return nil, nil - } - title := command.Title - if title == "" { - title = command.Name + return nil, err } ctx, cancel := context.WithCancel(xcontext.Detach(ctx)) // Start progress prior to spinning off a goroutine specifically so that @@ -117,12 +90,7 @@ func (s *Server) executeCommand(ctx context.Context, params *protocol.ExecuteCom work.end(title + ": failed") // Show a message when work completes with error, because the progress end // message is typically dismissed immediately by LSP clients. - if err := s.client.ShowMessage(ctx, &protocol.ShowMessageParams{ - Type: protocol.Error, - Message: fmt.Sprintf("%s: An error occurred: %v", title, err), - }); err != nil { - event.Error(ctx, title+": failed to show message", err) - } + s.showCommandError(ctx, title, err) default: work.end(command.Name + ": completed") } @@ -130,6 +98,46 @@ func (s *Server) executeCommand(ctx context.Context, params *protocol.ExecuteCom return nil, nil } +func (s *Server) runSuggestedFixCommand(ctx context.Context, command *source.Command, args []json.RawMessage) error { + var uri protocol.DocumentURI + var rng protocol.Range + if err := source.UnmarshalArgs(args, &uri, &rng); err != nil { + return err + } + snapshot, fh, ok, release, err := s.beginFileRequest(ctx, uri, source.Go) + defer release() + if !ok { + return err + } + edits, err := command.SuggestedFix(ctx, snapshot, fh, rng) + if err != nil { + return err + } + r, err := s.client.ApplyEdit(ctx, &protocol.ApplyWorkspaceEditParams{ + Edit: protocol.WorkspaceEdit{ + DocumentChanges: edits, + }, + }) + if err != nil { + return err + } + if !r.Applied { + return errors.New(r.FailureReason) + } + return nil +} + +func (s *Server) showCommandError(ctx context.Context, title string, err error) { + // Command error messages should not be cancelable. + ctx = xcontext.Detach(ctx) + if err := s.client.ShowMessage(ctx, &protocol.ShowMessageParams{ + Type: protocol.Error, + Message: fmt.Sprintf("%s failed: %v", title, err), + }); err != nil { + event.Error(ctx, title+": failed to show message", err) + } +} + func (s *Server) runCommand(ctx context.Context, work *workDone, command *source.Command, args []json.RawMessage) error { switch command { case source.CommandTest: diff --git a/cmd/govim/internal/golang_org_x_tools/lsp/fake/editor.go b/cmd/govim/internal/golang_org_x_tools/lsp/fake/editor.go index 8704117bf..e31f83219 100644 --- a/cmd/govim/internal/golang_org_x_tools/lsp/fake/editor.go +++ b/cmd/govim/internal/golang_org_x_tools/lsp/fake/editor.go @@ -426,7 +426,7 @@ func contentPosition(content string, offset int) (Pos, error) { return Pos{}, errors.Errorf("scanning content: %w", err) } // Scan() will drop the last line if it is empty. Correct for this. - if strings.HasSuffix(content, "\n") && offset == start { + if (strings.HasSuffix(content, "\n") || content == "") && offset == start { return Pos{Line: line, Column: 0}, nil } return Pos{}, fmt.Errorf("position %d out of bounds in %q (line = %d, start = %d)", offset, content, line, start) @@ -471,6 +471,18 @@ func regexpRange(content, re string) (Pos, Pos, error) { return startPos, endPos, nil } +// RegexpRange returns the first range in the buffer bufName matching re. See +// RegexpSearch for more information on matching. +func (e *Editor) RegexpRange(bufName, re string) (Pos, Pos, error) { + e.mu.Lock() + defer e.mu.Unlock() + buf, ok := e.buffers[bufName] + if !ok { + return Pos{}, Pos{}, ErrUnknownBuffer + } + return regexpRange(buf.text(), re) +} + // RegexpSearch returns the position of the first match for re in the buffer // bufName. For convenience, RegexpSearch supports the following two modes: // 1. If re has no subgroups, return the position of the match for re itself. @@ -478,13 +490,7 @@ func regexpRange(content, re string) (Pos, Pos, error) { // It returns an error re is invalid, has more than one subgroup, or doesn't // match the buffer. func (e *Editor) RegexpSearch(bufName, re string) (Pos, error) { - e.mu.Lock() - defer e.mu.Unlock() - buf, ok := e.buffers[bufName] - if !ok { - return Pos{}, ErrUnknownBuffer - } - start, _, err := regexpRange(buf.text(), re) + start, _, err := e.RegexpRange(bufName, re) return start, err } diff --git a/cmd/govim/internal/golang_org_x_tools/lsp/fake/workdir.go b/cmd/govim/internal/golang_org_x_tools/lsp/fake/workdir.go index 63e400ae8..ef36d2dcd 100644 --- a/cmd/govim/internal/golang_org_x_tools/lsp/fake/workdir.go +++ b/cmd/govim/internal/golang_org_x_tools/lsp/fake/workdir.go @@ -115,6 +115,14 @@ func (w *Workdir) ReadFile(path string) (string, error) { return string(b), nil } +func (w *Workdir) RegexpRange(path, re string) (Pos, Pos, error) { + content, err := w.ReadFile(path) + if err != nil { + return Pos{}, Pos{}, err + } + return regexpRange(content, re) +} + // RegexpSearch searches the file corresponding to path for the first position // matching re. func (w *Workdir) RegexpSearch(path string, re string) (Pos, error) { diff --git a/cmd/govim/internal/golang_org_x_tools/lsp/source/completion/completion.go b/cmd/govim/internal/golang_org_x_tools/lsp/source/completion/completion.go index 8cc36fda9..161cdd807 100644 --- a/cmd/govim/internal/golang_org_x_tools/lsp/source/completion/completion.go +++ b/cmd/govim/internal/golang_org_x_tools/lsp/source/completion/completion.go @@ -8,6 +8,7 @@ package completion import ( "context" + "fmt" "go/ast" "go/constant" "go/scanner" @@ -89,7 +90,6 @@ type CompletionItem struct { // completionOptions holds completion specific configuration. type completionOptions struct { - deepCompletion bool unimported bool documentation bool fullDocumentation bool @@ -318,114 +318,6 @@ func (c *completer) getSurrounding() *Selection { return c.surrounding } -// found adds a candidate completion. We will also search through the object's -// members for more candidates. -func (c *completer) found(ctx context.Context, cand candidate) { - obj := cand.obj - - if obj.Pkg() != nil && obj.Pkg() != c.pkg.GetTypes() && !obj.Exported() { - // obj is not accessible because it lives in another package and is not - // exported. Don't treat it as a completion candidate. - return - } - - if c.inDeepCompletion() { - // When searching deep, just make sure we don't have a cycle in our chain. - // We don't dedupe by object because we want to allow both "foo.Baz" and - // "bar.Baz" even though "Baz" is represented the same types.Object in both. - for _, seenObj := range c.deepState.chain { - if seenObj == obj { - return - } - } - } else { - // At the top level, dedupe by object. - if c.seen[obj] { - return - } - c.seen[obj] = true - } - - // If we are running out of budgeted time we must limit our search for deep - // completion candidates. - if c.shouldPrune() { - return - } - - // If we know we want a type name, don't offer non-type name - // candidates. However, do offer package names since they can - // contain type names, and do offer any candidate without a type - // since we aren't sure if it is a type name or not (i.e. unimported - // candidate). - if c.wantTypeName() && obj.Type() != nil && !isTypeName(obj) && !isPkgName(obj) { - return - } - - if c.matchingCandidate(&cand) { - cand.score *= highScore - - if p := c.penalty(&cand); p > 0 { - cand.score *= (1 - p) - } - } else if isTypeName(obj) { - // If obj is a *types.TypeName that didn't otherwise match, check - // if a literal object of this type makes a good candidate. - - // We only care about named types (i.e. don't want builtin types). - if _, isNamed := obj.Type().(*types.Named); isNamed { - c.literal(ctx, obj.Type(), cand.imp) - } - } - - // Lower score of method calls so we prefer fields and vars over calls. - if cand.expandFuncCall { - if sig, ok := obj.Type().Underlying().(*types.Signature); ok && sig.Recv() != nil { - cand.score *= 0.9 - } - } - - // Prefer private objects over public ones. - if !obj.Exported() && obj.Parent() != types.Universe { - cand.score *= 1.1 - } - - // Favor shallow matches by lowering score according to depth. - cand.score -= cand.score * c.deepState.scorePenalty() - - if cand.score < 0 { - cand.score = 0 - } - - cand.name = c.deepState.chainString(obj.Name()) - matchScore := c.matcher.Score(cand.name) - if matchScore > 0 { - cand.score *= float64(matchScore) - - // Avoid calling c.item() for deep candidates that wouldn't be in the top - // MaxDeepCompletions anyway. - if !c.inDeepCompletion() || c.deepState.isHighScore(cand.score) { - if item, err := c.item(ctx, cand); err == nil { - c.items = append(c.items, item) - } - } - } - - c.deepSearch(ctx, cand) -} - -// penalty reports a score penalty for cand in the range (0, 1). -// For example, a candidate is penalized if it has already been used -// in another switch case statement. -func (c *completer) penalty(cand *candidate) float64 { - for _, p := range c.inference.penalized { - if c.objChainMatches(cand.obj, p.objChain) { - return p.penalty - } - } - - return 0 -} - // candidate represents a completion candidate. type candidate struct { // obj is the types.Object to complete to. @@ -486,7 +378,7 @@ func (e ErrIsDefinition) Error() string { // the client to score the quality of the completion. For instance, some clients // may tolerate imperfect matches as valid completion results, since users may make typos. func Completion(ctx context.Context, snapshot source.Snapshot, fh source.FileHandle, protoPos protocol.Position, triggerCharacter string) ([]CompletionItem, *Selection, error) { - ctx, done := event.Start(ctx, "source.Completion") + ctx, done := event.Start(ctx, "completion.Completion") defer done() startTime := time.Now() @@ -571,9 +463,12 @@ func Completion(ctx context.Context, snapshot source.Snapshot, fh source.FileHan seen: make(map[types.Object]bool), enclosingFunc: enclosingFunction(path, pkg.GetTypesInfo()), enclosingCompositeLiteral: enclosingCompositeLiteral(path, rng.Start, pkg.GetTypesInfo()), + deepState: deepCompletionState{ + enabled: opts.DeepCompletion, + curPath: &searchPath{}, + }, opts: &completionOptions{ matcher: opts.Matcher, - deepCompletion: opts.DeepCompletion, unimported: opts.CompleteUnimported, documentation: opts.CompletionDocumentation, fullDocumentation: opts.HoverKind == source.FullDocumentation, @@ -588,11 +483,6 @@ func Completion(ctx context.Context, snapshot source.Snapshot, fh source.FileHan startTime: startTime, } - if c.opts.deepCompletion { - // Initialize max search depth to unlimited. - c.deepState.maxDepth = -1 - } - var cancel context.CancelFunc if c.opts.budget == 0 { ctx, cancel = context.WithCancel(ctx) @@ -623,9 +513,9 @@ func Completion(ctx context.Context, snapshot source.Snapshot, fh source.FileHan // Inside comments, offer completions for the name of the relevant symbol. for _, comment := range pgf.File.Comments { if comment.Pos() < rng.Start && rng.Start <= comment.End() { - // deep completion doesn't work properly in comments since we don't - // have a type object to complete further - c.deepState.maxDepth = 0 + // Deep completion doesn't work properly in comments since we don't + // have a type object to complete further. + c.deepState.enabled = false c.populateCommentCompletions(ctx, comment) return c.items, c.getSurrounding(), nil } @@ -633,6 +523,11 @@ func Completion(ctx context.Context, snapshot source.Snapshot, fh source.FileHan // Struct literals are handled entirely separately. if c.wantStructFieldCompletions() { + // If we are definitely completing a struct field name, deep completions + // don't make sense. + if c.enclosingCompositeLiteral.inKey { + c.deepState.enabled = false + } if err := c.structLiteralFieldName(ctx); err != nil { return nil, nil, err } @@ -789,33 +684,66 @@ func (c *completer) emptySwitchStmt() bool { // (i.e. "golang.org/x/"). The user is meant to accept completion suggestions // until they reach a complete import path. func (c *completer) populateImportCompletions(ctx context.Context, searchImport *ast.ImportSpec) error { + importPath := searchImport.Path.Value + + // Extract the text between the quotes (if any) in an import spec. + // prefix is the part of import path before the cursor. + prefixEnd := c.pos - searchImport.Path.Pos() + prefix := strings.Trim(importPath[:prefixEnd], `"`) + + // The number of directories in the import path gives us the depth at + // which to search. + depth := len(strings.Split(prefix, "/")) - 1 + + content := importPath + start, end := searchImport.Path.Pos(), searchImport.Path.End() + namePrefix, nameSuffix := `"`, `"` + // If a starting quote is present, adjust surrounding to either after the + // cursor or after the first slash (/), except if cursor is at the starting + // quote. Otherwise we provide a completion including the starting quote. + if strings.HasPrefix(importPath, `"`) && c.pos > searchImport.Path.Pos() { + content = content[1:] + start++ + if depth > 0 { + // Adjust textEdit start to replacement range. For ex: if current + // path was "golang.or/x/to<>ols/internal/", where <> is the cursor + // position, start of the replacement range would be after + // "golang.org/x/". + path := strings.SplitAfter(prefix, "/") + numChars := len(strings.Join(path[:len(path)-1], "")) + content = content[numChars:] + start += token.Pos(numChars) + } + namePrefix = "" + } + + // We won't provide an ending quote if one is already present, except if + // cursor is after the ending quote but still in import spec. This is + // because cursor has to be in our textEdit range. + if strings.HasSuffix(importPath, `"`) && c.pos < searchImport.Path.End() { + end-- + content = content[:len(content)-1] + nameSuffix = "" + } + c.surrounding = &Selection{ - content: searchImport.Path.Value, + content: content, cursor: c.pos, - MappedRange: source.NewMappedRange(c.snapshot.FileSet(), c.mapper, searchImport.Path.Pos(), searchImport.Path.End()), + MappedRange: source.NewMappedRange(c.snapshot.FileSet(), c.mapper, start, end), } seenImports := make(map[string]struct{}) for _, importSpec := range c.file.Imports { - if importSpec.Path.Value == searchImport.Path.Value { + if importSpec.Path.Value == importPath { continue } - importPath, err := strconv.Unquote(importSpec.Path.Value) + seenImportPath, err := strconv.Unquote(importSpec.Path.Value) if err != nil { return err } - seenImports[importPath] = struct{}{} + seenImports[seenImportPath] = struct{}{} } - prefixEnd := c.pos - searchImport.Path.ValuePos - // Extract the text between the quotes (if any) in an import spec. - // prefix is the part of import path before the cursor. - prefix := strings.Trim(searchImport.Path.Value[:prefixEnd], `"`) - - // The number of directories in the import path gives us the depth at - // which to search. - depth := len(strings.Split(prefix, "/")) - 1 - var mu sync.Mutex // guard c.items locally, since searchImports is called in parallel seen := make(map[string]struct{}) searchImports := func(pkg imports.ImportFix) { @@ -832,12 +760,20 @@ func (c *completer) populateImportCompletions(ctx context.Context, searchImport } pkgToConsider := strings.Join(pkgDirList[:depth+1], "/") + name := pkgDirList[depth] + // if we're adding an opening quote to completion too, set name to full + // package path since we'll need to overwrite that range. + if namePrefix == `"` { + name = pkgToConsider + } + score := float64(pkg.Relevance) if len(pkgDirList)-1 == depth { score *= highScore } else { // For incomplete package paths, add a terminal slash to indicate that the // user should keep triggering completions. + name += "/" pkgToConsider += "/" } @@ -850,11 +786,11 @@ func (c *completer) populateImportCompletions(ctx context.Context, searchImport defer mu.Unlock() obj := types.NewPkgName(0, nil, pkg.IdentName, types.NewPackage(pkgToConsider, pkg.IdentName)) - // Running goimports logic in completions is expensive, and the - // (*completer).found method imposes a 100ms budget. Work-around this - // by adding to c.items directly. - cand := candidate{obj: obj, name: `"` + pkgToConsider + `"`, score: score} + cand := candidate{obj: obj, name: namePrefix + name + nameSuffix, score: score} + // We use c.item here to be able to manually update the detail for a + // candidate. c.found doesn't give us access to the completion item. if item, err := c.item(ctx, cand); err == nil { + item.Detail = fmt.Sprintf("%q", pkgToConsider) c.items = append(c.items, item) } } @@ -1104,7 +1040,10 @@ func (c *completer) selector(ctx context.Context, sel *ast.SelectorExpr) error { // Is sel a qualified identifier? if id, ok := sel.X.(*ast.Ident); ok { if pkgName, ok := c.pkg.GetTypesInfo().Uses[id].(*types.PkgName); ok { - c.packageMembers(ctx, pkgName.Imported(), stdScore, nil) + candidates := c.packageMembers(ctx, pkgName.Imported(), stdScore, nil) + for _, cand := range candidates { + c.found(ctx, cand) + } return nil } } @@ -1112,7 +1051,11 @@ func (c *completer) selector(ctx context.Context, sel *ast.SelectorExpr) error { // Invariant: sel is a true selector. tv, ok := c.pkg.GetTypesInfo().Types[sel.X] if ok { - return c.methodsAndFields(ctx, tv.Type, tv.Addressable(), nil) + candidates := c.methodsAndFields(ctx, tv.Type, tv.Addressable(), nil) + for _, cand := range candidates { + c.found(ctx, cand) + } + return nil } // Try unimported packages. @@ -1165,7 +1108,10 @@ func (c *completer) unimportedMembers(ctx context.Context, id *ast.Ident) error if imports.ImportPathToAssumedName(path) != pkg.GetTypes().Name() { imp.name = pkg.GetTypes().Name() } - c.packageMembers(ctx, pkg.GetTypes(), unimportedScore(relevances[path]), imp) + candidates := c.packageMembers(ctx, pkg.GetTypes(), unimportedScore(relevances[path]), imp) + for _, cand := range candidates { + c.found(ctx, cand) + } if len(c.items) >= unimportedMemberTarget { return nil } @@ -1210,20 +1156,22 @@ func unimportedScore(relevance int) float64 { return (stdScore + .1*float64(relevance)) / 2 } -func (c *completer) packageMembers(ctx context.Context, pkg *types.Package, score float64, imp *importInfo) { +func (c *completer) packageMembers(ctx context.Context, pkg *types.Package, score float64, imp *importInfo) []candidate { + var candidates []candidate scope := pkg.Scope() for _, name := range scope.Names() { obj := scope.Lookup(name) - c.found(ctx, candidate{ + candidates = append(candidates, candidate{ obj: obj, score: score, imp: imp, addressable: isVar(obj), }) } + return candidates } -func (c *completer) methodsAndFields(ctx context.Context, typ types.Type, addressable bool, imp *importInfo) error { +func (c *completer) methodsAndFields(ctx context.Context, typ types.Type, addressable bool, imp *importInfo) []candidate { mset := c.methodSetCache[methodSetKey{typ, addressable}] if mset == nil { if addressable && !types.IsInterface(typ) && !isPointer(typ) { @@ -1236,8 +1184,9 @@ func (c *completer) methodsAndFields(ctx context.Context, typ types.Type, addres c.methodSetCache[methodSetKey{typ, addressable}] = mset } + var candidates []candidate for i := 0; i < mset.Len(); i++ { - c.found(ctx, candidate{ + candidates = append(candidates, candidate{ obj: mset.At(i).Obj(), score: stdScore, imp: imp, @@ -1247,7 +1196,7 @@ func (c *completer) methodsAndFields(ctx context.Context, typ types.Type, addres // Add fields of T. eachField(typ, func(v *types.Var) { - c.found(ctx, candidate{ + candidates = append(candidates, candidate{ obj: v, score: stdScore - 0.01, imp: imp, @@ -1255,7 +1204,7 @@ func (c *completer) methodsAndFields(ctx context.Context, typ types.Type, addres }) }) - return nil + return candidates } // lexical finds completions in the lexical environment. @@ -2474,40 +2423,6 @@ func (c *completer) matchingCandidate(cand *candidate) bool { return false } -// objChainMatches reports whether cand combined with the surrounding -// object prefix matches chain. -func (c *completer) objChainMatches(cand types.Object, chain []types.Object) bool { - // For example, when completing: - // - // foo.ba<> - // - // If we are considering the deep candidate "bar.baz", cand is baz, - // objChain is [foo] and deepChain is [bar]. We would match the - // chain [foo, bar, baz]. - - if len(chain) != len(c.inference.objChain)+len(c.deepState.chain)+1 { - return false - } - - if chain[len(chain)-1] != cand { - return false - } - - for i, o := range c.inference.objChain { - if chain[i] != o { - return false - } - } - - for i, o := range c.deepState.chain { - if chain[i+len(c.inference.objChain)] != o { - return false - } - } - - return true -} - // candTypeMatches reports whether cand makes a good completion // candidate given the candidate inference. cand's score may be // mutated to downrank the candidate in certain situations. diff --git a/cmd/govim/internal/golang_org_x_tools/lsp/source/completion/completion_format.go b/cmd/govim/internal/golang_org_x_tools/lsp/source/completion/completion_format.go index b61ec689d..28eaf2690 100644 --- a/cmd/govim/internal/golang_org_x_tools/lsp/source/completion/completion_format.go +++ b/cmd/govim/internal/golang_org_x_tools/lsp/source/completion/completion_format.go @@ -160,7 +160,7 @@ func (c *completer) item(ctx context.Context, cand candidate) (CompletionItem, e Detail: detail, Kind: kind, Score: cand.score, - Depth: len(c.deepState.chain), + Depth: len(c.deepState.curPath.path), snippet: snip, obj: obj, } diff --git a/cmd/govim/internal/golang_org_x_tools/lsp/source/completion/completion_snippet.go b/cmd/govim/internal/golang_org_x_tools/lsp/source/completion/completion_snippet.go index e719c9327..a0223dc4d 100644 --- a/cmd/govim/internal/golang_org_x_tools/lsp/source/completion/completion_snippet.go +++ b/cmd/govim/internal/golang_org_x_tools/lsp/source/completion/completion_snippet.go @@ -19,7 +19,7 @@ func (c *completer) structFieldSnippet(label, detail string) *snippet.Builder { // If we are in a deep completion then we can't be completing a field // name (e.g. "Foo{f<>}" completing to "Foo{f.Bar}" should not generate // a snippet). - if c.inDeepCompletion() { + if c.deepState.inDeepCompletion() { return nil } diff --git a/cmd/govim/internal/golang_org_x_tools/lsp/source/completion/deep_completion.go b/cmd/govim/internal/golang_org_x_tools/lsp/source/completion/deep_completion.go index 3f06d13b8..c7754ac51 100644 --- a/cmd/govim/internal/golang_org_x_tools/lsp/source/completion/deep_completion.go +++ b/cmd/govim/internal/golang_org_x_tools/lsp/source/completion/deep_completion.go @@ -11,6 +11,19 @@ import ( "time" ) +// searchItem represents a candidate in deep completion search queue. +type searchItem struct { + *searchPath + cand candidate +} + +// searchPath holds the path from search root (excluding the item itself) for +// a searchItem. +type searchPath struct { + path []types.Object + names []string +} + // MaxDeepCompletions limits deep completion results because in most cases // there are too many to be useful. const MaxDeepCompletions = 3 @@ -19,17 +32,19 @@ const MaxDeepCompletions = 3 // "deep completion" refers to searching into objects' fields and methods to // find more completion candidates. type deepCompletionState struct { - // maxDepth limits the deep completion search depth. 0 means - // disabled and -1 means unlimited. - maxDepth int + // enabled indicates wether deep completion is permitted. It should be + // reset to original value if manually disabled for an individual case. + enabled bool + + // queueClosed is used to disable adding new items to search queue once + // we're running out of our time budget. + queueClosed bool - // chain holds the traversal path as we do a depth-first search through - // objects' members looking for exact type matches. - chain []types.Object + // searchQueue holds the current breadth first search queue. + searchQueue []searchItem - // chainNames holds the names of the chain objects. This allows us to - // save allocations as we build many deep completion items. - chainNames []string + // curPath tracks the current deep completion search path. + curPath *searchPath // highScores tracks the highest deep candidate scores we have found // so far. This is used to avoid work for low scoring deep candidates. @@ -40,35 +55,45 @@ type deepCompletionState struct { candidateCount int } -// push pushes obj onto our search stack. If invoke is true then -// invocation parens "()" will be appended to the object name. -func (s *deepCompletionState) push(obj types.Object, invoke bool) { - s.chain = append(s.chain, obj) - - name := obj.Name() - if invoke { - name += "()" +// enqueue adds candidates to the search queue. +func (s *deepCompletionState) enqueue(path *searchPath, candidates ...candidate) { + for _, cand := range candidates { + s.searchQueue = append(s.searchQueue, searchItem{path, cand}) } - s.chainNames = append(s.chainNames, name) } -// pop pops the last object off our search stack. -func (s *deepCompletionState) pop() { - s.chain = s.chain[:len(s.chain)-1] - s.chainNames = s.chainNames[:len(s.chainNames)-1] +// dequeue removes and returns the leftmost element from the search queue. +func (s *deepCompletionState) dequeue() *searchItem { + var item *searchItem + item, s.searchQueue = &s.searchQueue[0], s.searchQueue[1:] + return item } -// chainString joins the chain of objects' names together on ".". -func (s *deepCompletionState) chainString(finalName string) string { - s.chainNames = append(s.chainNames, finalName) - chainStr := strings.Join(s.chainNames, ".") - s.chainNames = s.chainNames[:len(s.chainNames)-1] - return chainStr +// scorePenalty computes a deep candidate score penalty. A candidate is +// penalized based on depth to favor shallower candidates. We also give a +// slight bonus to unexported objects and a slight additional penalty to +// function objects. +func (s *deepCompletionState) scorePenalty() float64 { + var deepPenalty float64 + for _, dc := range s.curPath.path { + deepPenalty++ + + if !dc.Exported() { + deepPenalty -= 0.1 + } + + if _, isSig := dc.Type().Underlying().(*types.Signature); isSig { + deepPenalty += 0.1 + } + } + + // Normalize penalty to a max depth of 10. + return deepPenalty / 10 } -// isHighScore returns whether score is among the top MaxDeepCompletions -// deep candidate scores encountered so far. If so, it adds score to -// highScores, possibly displacing an existing high score. +// isHighScore returns whether score is among the top MaxDeepCompletions deep +// candidate scores encountered so far. If so, it adds score to highScores, +// possibly displacing an existing high score. func (s *deepCompletionState) isHighScore(score float64) bool { // Invariant: s.highScores is sorted with highest score first. Unclaimed // positions are trailing zeros. @@ -91,126 +116,240 @@ func (s *deepCompletionState) isHighScore(score float64) bool { return false } -// scorePenalty computes a deep candidate score penalty. A candidate -// is penalized based on depth to favor shallower candidates. We also -// give a slight bonus to unexported objects and a slight additional -// penalty to function objects. -func (s *deepCompletionState) scorePenalty() float64 { - var deepPenalty float64 - for _, dc := range s.chain { - deepPenalty += 1 +// inDeepCompletion returns if we're currently searching an object's members. +func (s *deepCompletionState) inDeepCompletion() bool { + return len(s.curPath.path) > 0 +} - if !dc.Exported() { - deepPenalty -= 0.1 - } +// reset resets deepCompletionState since found might be called multiple times. +// We don't reset high scores since multiple calls to found still respect the +// same MaxDeepCompletions count. +func (s *deepCompletionState) reset() { + s.searchQueue = nil + s.curPath = &searchPath{} +} - if _, isSig := dc.Type().Underlying().(*types.Signature); isSig { - deepPenalty += 0.1 - } +// appendToSearchPath appends an object to a given searchPath. +func appendToSearchPath(oldPath searchPath, obj types.Object, invoke bool) *searchPath { + name := obj.Name() + if invoke { + name += "()" } - // Normalize penalty to a max depth of 10. - return deepPenalty / 10 -} + // copy the slice since we don't want to overwrite the original slice. + path := append([]types.Object{}, oldPath.path...) + names := append([]string{}, oldPath.names...) -func (c *completer) inDeepCompletion() bool { - return len(c.deepState.chain) > 0 + return &searchPath{ + path: append(path, obj), + names: append(names, name), + } } -// shouldPrune returns whether we should prune the current deep -// candidate search to reduce the overall search scope. The -// maximum search depth is reduced gradually as we use up our -// completionBudget. -func (c *completer) shouldPrune() bool { - if !c.inDeepCompletion() { - return false +// found adds a candidate to completion items if it's a valid suggestion and +// searches the candidate's subordinate objects for more completion items if +// deep completion is enabled. +func (c *completer) found(ctx context.Context, cand candidate) { + // reset state at the end so current state doesn't affect completions done + // outside c.found. + defer c.deepState.reset() + + // At the top level, dedupe by object. + if c.seen[cand.obj] { + return } + c.seen[cand.obj] = true + + c.deepState.enqueue(&searchPath{}, cand) +outer: + for len(c.deepState.searchQueue) > 0 { + item := c.deepState.dequeue() + curCand := item.cand + obj := curCand.obj + + // If obj is not accessible because it lives in another package and is + // not exported, don't treat it as a completion candidate. + if obj.Pkg() != nil && obj.Pkg() != c.pkg.GetTypes() && !obj.Exported() { + continue + } - // Check our remaining budget every 100 candidates. - if c.opts.budget > 0 && c.deepState.candidateCount%100 == 0 { - spent := float64(time.Since(c.startTime)) / float64(c.opts.budget) + // If we want a type name, don't offer non-type name candidates. + // However, do offer package names since they can contain type names, + // and do offer any candidate without a type since we aren't sure if it + // is a type name or not (i.e. unimported candidate). + if c.wantTypeName() && obj.Type() != nil && !isTypeName(obj) && !isPkgName(obj) { + continue + } - switch { - case spent >= 0.90: - // We are close to exhausting our budget. Disable deep completions. - c.deepState.maxDepth = 0 - case spent >= 0.75: - // We are running out of budget, reduce max depth again. - c.deepState.maxDepth = 2 - case spent >= 0.5: - // We have used half our budget, reduce max depth again. - c.deepState.maxDepth = 3 - case spent >= 0.25: - // We have used a good chunk of our budget, so start limiting our search. - // By default the search depth is unlimited, so this limit, while still - // generous, is normally a huge reduction in search scope that will result - // in our search completing very soon. - c.deepState.maxDepth = 4 + // When searching deep, make sure we don't have a cycle in our chain. + // We don't dedupe by object because we want to allow both "foo.Baz" + // and "bar.Baz" even though "Baz" is represented the same types.Object + // in both. + for _, seenObj := range item.path { + if seenObj == obj { + continue outer + } } - } - c.deepState.candidateCount++ + // update tracked current path since other functions might check it. + c.deepState.curPath = item.searchPath + + c.addCandidate(ctx, curCand) + + c.deepState.candidateCount++ + if c.opts.budget > 0 && c.deepState.candidateCount%100 == 0 { + spent := float64(time.Since(c.startTime)) / float64(c.opts.budget) + if spent > 1.0 { + return + } + // If we are almost out of budgeted time, no further elements + // should be added to the queue. This ensures remaining time is + // used for processing current queue. + if !c.deepState.queueClosed && spent >= 0.85 { + c.deepState.queueClosed = true + } + } - if c.deepState.maxDepth >= 0 { - return len(c.deepState.chain) >= c.deepState.maxDepth - } + // if deep search is disabled, don't add any more candidates. + if !c.deepState.enabled || c.deepState.queueClosed { + continue + } - return false -} + // Searching members for a type name doesn't make sense. + if isTypeName(obj) { + continue + } + if obj.Type() == nil { + continue + } -// deepSearch searches through cand's subordinate objects for more -// completion items. -func (c *completer) deepSearch(ctx context.Context, cand candidate) { - if c.deepState.maxDepth == 0 { - return + // Don't search embedded fields because they were already included in their + // parent's fields. + if v, ok := obj.(*types.Var); ok && v.Embedded() { + continue + } + + if sig, ok := obj.Type().Underlying().(*types.Signature); ok { + // If obj is a function that takes no arguments and returns one + // value, keep searching across the function call. + if sig.Params().Len() == 0 && sig.Results().Len() == 1 { + newSearchPath := appendToSearchPath(*item.searchPath, obj, true) + // The result of a function call is not addressable. + candidates := c.methodsAndFields(ctx, sig.Results().At(0).Type(), false, curCand.imp) + c.deepState.enqueue(newSearchPath, candidates...) + } + } + + newSearchPath := appendToSearchPath(*item.searchPath, obj, false) + switch obj := obj.(type) { + case *types.PkgName: + candidates := c.packageMembers(ctx, obj.Imported(), stdScore, curCand.imp) + c.deepState.enqueue(newSearchPath, candidates...) + default: + candidates := c.methodsAndFields(ctx, obj.Type(), curCand.addressable, curCand.imp) + c.deepState.enqueue(newSearchPath, candidates...) + } } +} +// addCandidate adds a completion candidate to suggestions, without searching +// its members for more candidates. +func (c *completer) addCandidate(ctx context.Context, cand candidate) { obj := cand.obj + if c.matchingCandidate(&cand) { + cand.score *= highScore - // If we are definitely completing a struct field name, deep completions - // don't make sense. - if c.wantStructFieldCompletions() && c.enclosingCompositeLiteral.inKey { - return + if p := c.penalty(&cand); p > 0 { + cand.score *= (1 - p) + } + } else if isTypeName(obj) { + // If obj is a *types.TypeName that didn't otherwise match, check + // if a literal object of this type makes a good candidate. + + // We only care about named types (i.e. don't want builtin types). + if _, isNamed := obj.Type().(*types.Named); isNamed { + c.literal(ctx, obj.Type(), cand.imp) + } } - // Don't search into type names. - if isTypeName(obj) { - return + // Lower score of method calls so we prefer fields and vars over calls. + if cand.expandFuncCall { + if sig, ok := obj.Type().Underlying().(*types.Signature); ok && sig.Recv() != nil { + cand.score *= 0.9 + } } - if obj.Type() == nil { - return + // Prefer private objects over public ones. + if !obj.Exported() && obj.Parent() != types.Universe { + cand.score *= 1.1 } - // Don't search embedded fields because they were already included in their - // parent's fields. - if v, ok := obj.(*types.Var); ok && v.Embedded() { - return + // Favor shallow matches by lowering score according to depth. + cand.score -= cand.score * c.deepState.scorePenalty() + + if cand.score < 0 { + cand.score = 0 } - if sig, ok := obj.Type().Underlying().(*types.Signature); ok { - // If obj is a function that takes no arguments and returns one - // value, keep searching across the function call. - if sig.Params().Len() == 0 && sig.Results().Len() == 1 { - // Pass invoke=true since the function needs to be invoked in - // the deep chain. - c.deepState.push(obj, true) - // The result of a function call is not addressable. - c.methodsAndFields(ctx, sig.Results().At(0).Type(), false, cand.imp) - c.deepState.pop() + cand.name = strings.Join(append(c.deepState.curPath.names, cand.obj.Name()), ".") + matchScore := c.matcher.Score(cand.name) + if matchScore > 0 { + cand.score *= float64(matchScore) + + // Avoid calling c.item() for deep candidates that wouldn't be in the top + // MaxDeepCompletions anyway. + if !c.deepState.inDeepCompletion() || c.deepState.isHighScore(cand.score) { + if item, err := c.item(ctx, cand); err == nil { + c.items = append(c.items, item) + } + } + } + +} + +// penalty reports a score penalty for cand in the range (0, 1). +// For example, a candidate is penalized if it has already been used +// in another switch case statement. +func (c *completer) penalty(cand *candidate) float64 { + for _, p := range c.inference.penalized { + if c.objChainMatches(cand.obj, p.objChain) { + return p.penalty } } - // Push this object onto our search stack. - c.deepState.push(obj, false) + return 0 +} + +// objChainMatches reports whether cand combined with the surrounding +// object prefix matches chain. +func (c *completer) objChainMatches(cand types.Object, chain []types.Object) bool { + // For example, when completing: + // + // foo.ba<> + // + // If we are considering the deep candidate "bar.baz", cand is baz, + // objChain is [foo] and deepChain is [bar]. We would match the + // chain [foo, bar, baz]. + + if len(chain) != len(c.inference.objChain)+len(c.deepState.curPath.path)+1 { + return false + } - switch obj := obj.(type) { - case *types.PkgName: - c.packageMembers(ctx, obj.Imported(), stdScore, cand.imp) - default: - c.methodsAndFields(ctx, obj.Type(), cand.addressable, cand.imp) + if chain[len(chain)-1] != cand { + return false + } + + for i, o := range c.inference.objChain { + if chain[i] != o { + return false + } + } + + for i, o := range c.deepState.curPath.path { + if chain[i+len(c.inference.objChain)] != o { + return false + } } - // Pop the object off our search stack. - c.deepState.pop() + return true } diff --git a/cmd/govim/internal/golang_org_x_tools/lsp/source/view.go b/cmd/govim/internal/golang_org_x_tools/lsp/source/view.go index 4c1507d1e..5a4a3cc97 100644 --- a/cmd/govim/internal/golang_org_x_tools/lsp/source/view.go +++ b/cmd/govim/internal/golang_org_x_tools/lsp/source/view.go @@ -400,6 +400,10 @@ type FileIdentity struct { Kind FileKind } +func (id FileIdentity) String() string { + return fmt.Sprintf("%s%s%s", id.URI, id.Hash, id.Kind) +} + // FileKind describes the kind of the file in question. // It can be one of Go, mod, or sum. type FileKind int diff --git a/go.mod b/go.mod index fbea1ce80..6252f229f 100644 --- a/go.mod +++ b/go.mod @@ -12,8 +12,8 @@ require ( github.com/rogpeppe/go-internal v1.6.2 golang.org/x/mod v0.3.0 golang.org/x/sync v0.0.0-20200625203802-6e8e738ad208 - golang.org/x/tools v0.0.0-20200917132429-63098cc47d65 - golang.org/x/tools/gopls v0.0.0-20200917132429-63098cc47d65 + golang.org/x/tools v0.0.0-20200918232735-d647fc253266 + golang.org/x/tools/gopls v0.0.0-20200918232735-d647fc253266 golang.org/x/xerrors v0.0.0-20200804184101-5ec99f83aff1 gopkg.in/retry.v1 v1.0.3 gopkg.in/tomb.v2 v2.0.0-20161208151619-d5d1b5820637 diff --git a/go.sum b/go.sum index eef1f07a8..ccdbc1724 100644 --- a/go.sum +++ b/go.sum @@ -69,10 +69,10 @@ golang.org/x/tools v0.0.0-20191119224855-298f0cb1881e/go.mod h1:b+2E5dAYhXwXZwtn golang.org/x/tools v0.0.0-20191130070609-6e064ea0cf2d/go.mod h1:b+2E5dAYhXwXZwtnZ6UAqBI28+e2cm9otk0dWdXHAEo= golang.org/x/tools v0.0.0-20200731060945-b5fad4ed8dd6/go.mod h1:njjCfa9FT2d7l9Bc6FUM5FLjQPp3cFF28FI3qnDFljA= golang.org/x/tools v0.0.0-20200828161849-5deb26317202/go.mod h1:njjCfa9FT2d7l9Bc6FUM5FLjQPp3cFF28FI3qnDFljA= -golang.org/x/tools v0.0.0-20200917132429-63098cc47d65 h1:pSBfdsmz/LD4Z+CedEf2tYuhDR+/nb77SHI1Ap7+WDQ= -golang.org/x/tools v0.0.0-20200917132429-63098cc47d65/go.mod h1:z6u4i615ZeAfBE4XtMziQW1fSVJXACjjbWkB/mvPzlU= -golang.org/x/tools/gopls v0.0.0-20200917132429-63098cc47d65 h1:zJ8fJlNGkSJtquEFe4Hc/UjYL8/maE6CjSceMZ0fkM4= -golang.org/x/tools/gopls v0.0.0-20200917132429-63098cc47d65/go.mod h1:ZKEbBWLaHDNmr7igDh2FBZr7V0r1eonT0J5lLqVmCw0= +golang.org/x/tools v0.0.0-20200918232735-d647fc253266 h1:k7tVuG0g1JwmD3Jh8oAl1vQ1C3jb4Hi/dUl1wWDBJpQ= +golang.org/x/tools v0.0.0-20200918232735-d647fc253266/go.mod h1:z6u4i615ZeAfBE4XtMziQW1fSVJXACjjbWkB/mvPzlU= +golang.org/x/tools/gopls v0.0.0-20200918232735-d647fc253266 h1:42S1+RsZ8vvGMt60m7BhskM9ilhaVqCCBfTUryork6I= +golang.org/x/tools/gopls v0.0.0-20200918232735-d647fc253266/go.mod h1:ZKEbBWLaHDNmr7igDh2FBZr7V0r1eonT0J5lLqVmCw0= golang.org/x/xerrors v0.0.0-20190717185122-a985d3407aa7/go.mod h1:I/5z698sn9Ka8TeJc9MKroUUfqBBauWjQqLJ2OPfmY0= golang.org/x/xerrors v0.0.0-20191011141410-1b5146add898/go.mod h1:I/5z698sn9Ka8TeJc9MKroUUfqBBauWjQqLJ2OPfmY0= golang.org/x/xerrors v0.0.0-20191204190536-9bdfabe68543/go.mod h1:I/5z698sn9Ka8TeJc9MKroUUfqBBauWjQqLJ2OPfmY0=