internal/lsp: fix the logic to avoid duplicate file watching
The logic that checked if a file was already being watched by the
default glob pattern was incorrect. Fix it, and use the newly added
InDir function.
Change-Id: Ia7e3851ab5b9fa1fa7590cae3b370676201a9141
Reviewed-on: https://go-review.googlesource.com/c/tools/+/267119
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: Heschi Kreinick <heschi@google.com>
diff --git a/internal/lsp/cache/snapshot.go b/internal/lsp/cache/snapshot.go
index d916de3..15b9855 100644
--- a/internal/lsp/cache/snapshot.go
+++ b/internal/lsp/cache/snapshot.go
@@ -694,7 +694,7 @@
func (s *snapshot) GoModForFile(ctx context.Context, uri span.URI) span.URI {
var match span.URI
for modURI := range s.workspace.activeModFiles() {
- if !inDir(dirURI(modURI).Filename(), uri.Filename()) {
+ if !source.InDir(dirURI(modURI).Filename(), uri.Filename()) {
continue
}
if len(modURI) > len(match) {
@@ -1377,7 +1377,7 @@
}
// If a go.mod in the workspace has been changed, invalidate metadata.
if kind := originalFH.Kind(); kind == source.Mod {
- return inDir(filepath.Dir(s.view.rootURI.Filename()), filepath.Dir(originalFH.URI().Filename()))
+ return source.InDir(filepath.Dir(s.view.rootURI.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
diff --git a/internal/lsp/cache/view.go b/internal/lsp/cache/view.go
index f27d8ca..598908a 100644
--- a/internal/lsp/cache/view.go
+++ b/internal/lsp/cache/view.go
@@ -698,87 +698,13 @@
// The user may have a multiple directories in their GOPATH.
// Check if the workspace is within any of them.
for _, gp := range filepath.SplitList(ws.gopath) {
- if inDir(filepath.Join(gp, "src"), folder.Filename()) {
+ if source.InDir(filepath.Join(gp, "src"), folder.Filename()) {
return true
}
}
return false
}
-// Copied and slightly adjusted from go/src/cmd/go/internal/search/search.go.
-//
-// inDir checks whether path is in the file tree rooted at dir.
-// If so, InDir returns an equivalent path relative to dir.
-// If not, InDir returns an empty string.
-// InDir makes some effort to succeed even in the presence of symbolic links.
-func inDir(dir, path string) bool {
- if rel := inDirLex(path, dir); rel != "" {
- return true
- }
- xpath, err := filepath.EvalSymlinks(path)
- if err != nil || xpath == path {
- xpath = ""
- } else {
- if rel := inDirLex(xpath, dir); rel != "" {
- return true
- }
- }
-
- xdir, err := filepath.EvalSymlinks(dir)
- if err == nil && xdir != dir {
- if rel := inDirLex(path, xdir); rel != "" {
- return true
- }
- if xpath != "" {
- if rel := inDirLex(xpath, xdir); rel != "" {
- return true
- }
- }
- }
- return false
-}
-
-// Copied from go/src/cmd/go/internal/search/search.go.
-//
-// inDirLex is like inDir but only checks the lexical form of the file names.
-// It does not consider symbolic links.
-// TODO(rsc): This is a copy of str.HasFilePathPrefix, modified to
-// return the suffix. Most uses of str.HasFilePathPrefix should probably
-// be calling InDir instead.
-func inDirLex(path, dir string) string {
- pv := strings.ToUpper(filepath.VolumeName(path))
- dv := strings.ToUpper(filepath.VolumeName(dir))
- path = path[len(pv):]
- dir = dir[len(dv):]
- switch {
- default:
- return ""
- case pv != dv:
- return ""
- case len(path) == len(dir):
- if path == dir {
- return "."
- }
- return ""
- case dir == "":
- return path
- case len(path) > len(dir):
- if dir[len(dir)-1] == filepath.Separator {
- if path[:len(dir)] == dir {
- return path[len(dir):]
- }
- return ""
- }
- if path[len(dir)] == filepath.Separator && path[:len(dir)] == dir {
- if len(path) == len(dir)+1 {
- return "."
- }
- return path[len(dir)+1:]
- }
- return ""
- }
-}
-
// getGoEnv gets the view's various GO* values.
func (s *Session) getGoEnv(ctx context.Context, folder string, configEnv []string) (environmentVariables, map[string]string, error) {
envVars := environmentVariables{}
diff --git a/internal/lsp/cache/workspace.go b/internal/lsp/cache/workspace.go
index 673843a..4d05adc 100644
--- a/internal/lsp/cache/workspace.go
+++ b/internal/lsp/cache/workspace.go
@@ -258,7 +258,7 @@
// Legacy mode only considers a module a workspace root.
continue
}
- if !inDir(wm.root.Filename(), uri.Filename()) {
+ if !source.InDir(wm.root.Filename(), uri.Filename()) {
// Otherwise, the module must be contained within the workspace root.
continue
}
diff --git a/internal/lsp/general.go b/internal/lsp/general.go
index 0dd0ad2..1c2a8bd 100644
--- a/internal/lsp/general.go
+++ b/internal/lsp/general.go
@@ -348,13 +348,20 @@
}}
for dir := range dirs {
filename := dir.Filename()
+
// If the directory is within a workspace folder, we're already
// watching it via the relative path above.
+ var matched bool
for _, view := range s.session.Views() {
- if isSubdirectory(view.Folder().Filename(), filename) {
- continue
+ if source.InDir(view.Folder().Filename(), filename) {
+ matched = true
+ break
}
}
+ if matched {
+ continue
+ }
+
// If microsoft/vscode#100870 is resolved before
// microsoft/vscode#104387, we will need a work-around for Windows
// drive letter casing.
diff --git a/internal/lsp/source/util.go b/internal/lsp/source/util.go
index 4e01099..b3d87f9 100644
--- a/internal/lsp/source/util.go
+++ b/internal/lsp/source/util.go
@@ -441,3 +441,77 @@
}
return true
}
+
+// InDir checks whether path is in the file tree rooted at dir.
+// If so, InDir returns an equivalent path relative to dir.
+// If not, InDir returns an empty string.
+// InDir makes some effort to succeed even in the presence of symbolic links.
+//
+// Copied and slightly adjusted from go/src/cmd/go/internal/search/search.go.
+func InDir(dir, path string) bool {
+ if rel := inDirLex(path, dir); rel != "" {
+ return true
+ }
+ xpath, err := filepath.EvalSymlinks(path)
+ if err != nil || xpath == path {
+ xpath = ""
+ } else {
+ if rel := inDirLex(xpath, dir); rel != "" {
+ return true
+ }
+ }
+
+ xdir, err := filepath.EvalSymlinks(dir)
+ if err == nil && xdir != dir {
+ if rel := inDirLex(path, xdir); rel != "" {
+ return true
+ }
+ if xpath != "" {
+ if rel := inDirLex(xpath, xdir); rel != "" {
+ return true
+ }
+ }
+ }
+ return false
+}
+
+// Copied from go/src/cmd/go/internal/search/search.go.
+//
+// inDirLex is like inDir but only checks the lexical form of the file names.
+// It does not consider symbolic links.
+// TODO(rsc): This is a copy of str.HasFilePathPrefix, modified to
+// return the suffix. Most uses of str.HasFilePathPrefix should probably
+// be calling InDir instead.
+func inDirLex(path, dir string) string {
+ pv := strings.ToUpper(filepath.VolumeName(path))
+ dv := strings.ToUpper(filepath.VolumeName(dir))
+ path = path[len(pv):]
+ dir = dir[len(dv):]
+ switch {
+ default:
+ return ""
+ case pv != dv:
+ return ""
+ case len(path) == len(dir):
+ if path == dir {
+ return "."
+ }
+ return ""
+ case dir == "":
+ return path
+ case len(path) > len(dir):
+ if dir[len(dir)-1] == filepath.Separator {
+ if path[:len(dir)] == dir {
+ return path[len(dir):]
+ }
+ return ""
+ }
+ if path[len(dir)] == filepath.Separator && path[:len(dir)] == dir {
+ if len(path) == len(dir)+1 {
+ return "."
+ }
+ return path[len(dir)+1:]
+ }
+ return ""
+ }
+}
