gopls/internal/protocol: refine DocumentURI Clean method and its usages

This CL tracks the comments left by Alan Donovan in CL663295, thanks again.

* change protocol.Clean function to DocumentURI method
* ensure the same key when access and operate viewMap
* factor some logics

Change-Id: Ib717705495997379ed0f872d438f238ca1308a23
Reviewed-on: https://go-review.googlesource.com/c/tools/+/678416
Auto-Submit: Alan Donovan <[email protected]>
Reviewed-by: Robert Findley <[email protected]>
LUCI-TryBot-Result: Go LUCI <[email protected]>
Reviewed-by: Alan Donovan <[email protected]>

Author: xieyuschen

gopls/internal/cache/session.go

@@ -64,7 +64,7 @@ type Session struct {
 
 	viewMu  sync.Mutex
 	views   []*View
-	viewMap map[protocol.DocumentURI]*View // file->best view or nil; nil after shutdown
+	viewMap map[protocol.DocumentURI]*View // file->best view or nil; nil after shutdown; the key must be a clean uri.
 
 	// snapshots is a counting semaphore that records the number
 	// of unreleased snapshots associated with this session.
@@ -139,15 +139,16 @@ func (s *Session) NewView(ctx context.Context, folder *Folder) (*View, *Snapshot
 	}
 	view, snapshot, release := s.createView(ctx, def)
 	s.views = append(s.views, view)
-	s.viewMap[protocol.Clean(folder.Dir)] = view
+	s.viewMap[folder.Dir.Clean()] = view
 	return view, snapshot, release, nil
 }
 
 // HasView checks whether the uri's view exists.
 func (s *Session) HasView(uri protocol.DocumentURI) bool {
+	uri = uri.Clean()
 	s.viewMu.Lock()
 	defer s.viewMu.Unlock()
-	_, ok := s.viewMap[protocol.Clean(uri)]
+	_, ok := s.viewMap[uri]
 	return ok
 }
 
@@ -379,6 +380,7 @@ func (s *Session) View(id string) (*View, error) {
 //
 // On success, the caller must call the returned function to release the snapshot.
 func (s *Session) SnapshotOf(ctx context.Context, uri protocol.DocumentURI) (*Snapshot, func(), error) {
+	uri = uri.Clean()
 	// Fast path: if the uri has a static association with a view, return it.
 	s.viewMu.Lock()
 	v, err := s.viewOfLocked(ctx, uri)
@@ -396,7 +398,7 @@ func (s *Session) SnapshotOf(ctx context.Context, uri protocol.DocumentURI) (*Sn
 		// View is shut down. Forget this association.
 		s.viewMu.Lock()
 		if s.viewMap[uri] == v {
-			delete(s.viewMap, protocol.Clean(uri))
+			delete(s.viewMap, uri)
 		}
 		s.viewMu.Unlock()
 	}
@@ -473,7 +475,7 @@ var errNoViews = errors.New("no views")
 // viewOfLocked evaluates the best view for uri, memoizing its result in
 // s.viewMap.
 //
-// Precondition: caller holds s.viewMu lock.
+// Precondition: caller holds s.viewMu lock; uri must be clean.
 //
 // May return (nil, nil) if no best view can be determined.
 func (s *Session) viewOfLocked(ctx context.Context, uri protocol.DocumentURI) (*View, error) {
@@ -500,7 +502,7 @@ func (s *Session) viewOfLocked(ctx context.Context, uri protocol.DocumentURI) (*
 			// (as in golang/go#60776).
 			v = relevantViews[0]
 		}
-		s.viewMap[protocol.Clean(uri)] = v // may be nil
+		s.viewMap[uri] = v // may be nil
 	}
 	return v, nil
 }
@@ -748,7 +750,7 @@ func (s *Session) ResetView(ctx context.Context, uri protocol.DocumentURI) (*Vie
 		return nil, fmt.Errorf("session is shut down")
 	}
 
-	view, err := s.viewOfLocked(ctx, uri)
+	view, err := s.viewOfLocked(ctx, uri.Clean())
 	if err != nil {
 		return nil, err
 	}

gopls/internal/protocol/uri.go

@@ -68,7 +68,7 @@ func (uri *DocumentURI) UnmarshalText(data []byte) (err error) {
 }
 
 // Clean returns the cleaned uri by triggering filepath.Clean underlying.
-func Clean(uri DocumentURI) DocumentURI {
+func (uri DocumentURI) Clean() DocumentURI {
 	return URIFromPath(filepath.Clean(uri.Path()))
 }
 

gopls/internal/test/integration/fake/editor.go

@@ -327,7 +327,11 @@ func (e *Editor) initialize(ctx context.Context) error {
 	params.InitializationOptions = makeSettings(e.sandbox, config, nil)
 
 	params.WorkspaceFolders = makeWorkspaceFolders(e.sandbox, config.WorkspaceFolders, config.NoDefaultWorkspaceFiles)
-	params.RootURI = protocol.DocumentURI(makeRootURI(e.sandbox, config.RelRootPath))
+	params.RootURI = protocol.URIFromPath(config.RelRootPath)
+	if !uriRE.MatchString(config.RelRootPath) { // relative file path
+		params.RootURI = e.sandbox.Workdir.URI(config.RelRootPath)
+	}
+
 	capabilities, err := clientCapabilities(config)
 	if err != nil {
 		return fmt.Errorf("unmarshalling EditorConfig.CapabilitiesJSON: %v", err)
@@ -447,10 +451,10 @@ var uriRE = regexp.MustCompile(`^[a-z][a-z0-9+\-.]*://\S+`)
 // makeWorkspaceFolders creates a slice of workspace folders to use for
 // this editing session, based on the editor configuration.
 func makeWorkspaceFolders(sandbox *Sandbox, paths []string, useEmpty bool) (folders []protocol.WorkspaceFolder) {
-	if len(paths) == 0 && useEmpty {
-		return nil
-	}
 	if len(paths) == 0 {
+		if useEmpty {
+			return nil
+		}
 		paths = []string{string(sandbox.Workdir.RelativeTo)}
 	}
 
@@ -468,14 +472,6 @@ func makeWorkspaceFolders(sandbox *Sandbox, paths []string, useEmpty bool) (fold
 	return folders
 }
 
-func makeRootURI(sandbox *Sandbox, path string) string {
-	uri := path
-	if !uriRE.MatchString(path) { // relative file path
-		uri = string(sandbox.Workdir.URI(path))
-	}
-	return uri
-}
-
 // onFileChanges is registered to be called by the Workdir on any writes that
 // go through the Workdir API. It is called synchronously by the Workdir.
 func (e *Editor) onFileChanges(ctx context.Context, evts []protocol.FileEvent) {