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>

Author: stamblerre

internal/lsp/cache/snapshot.go

@@ -694,7 +694,7 @@ func (s *snapshot) CachedImportPaths(ctx context.Context) (map[string]source.Pac
 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 @@ func (s *snapshot) shouldInvalidateMetadata(ctx context.Context, newSnapshot *sn
 	}
 	// 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

internal/lsp/cache/view.go

@@ -698,87 +698,13 @@ func validBuildConfiguration(folder span.URI, ws *workspaceInformation, modFiles
 	// 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{}

internal/lsp/cache/workspace.go

@@ -258,7 +258,7 @@ func (wm *workspace) invalidate(ctx context.Context, changes map[span.URI]*fileC
 				// 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
 			}

internal/lsp/general.go

@@ -348,13 +348,20 @@ func (s *Server) registerWatchedDirectoriesLocked(ctx context.Context, dirs map[
 	}}
 	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.

internal/lsp/source/util.go

@@ -441,3 +441,77 @@ func isDirective(c string) bool {
 	}
 	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 ""
+	}
+}