lsp: assert that a WorkspaceFolder must correspond to a CUE module

The restrictions on workspace folders for day 1 of 'cue lsp' are now
updated at https://github.com/cue-lang/cue/wiki/cue-lsp.

This CL enforces the requirement that a workspace folder must correspond
directly to the root of a CUE module, and that furthermore a directory
is defined as a module root if it contains a cue.mod/module.cue file
(see the wiki for an explanation regarding this tightened requirement).

Tests are added to verify we have enforced the check and that error
messages are shown to the user as appropriate.

The restriction on opening a single workspace folder already exists and
is tested for.

Whilst here, remove viewDefinition.cuemod. As the comment suggests it's
superfluous.

Signed-off-by: Paul Jolly <paul@myitcv.io>
Change-Id: I532fb698b738560ac3ac1badcccd5b4e250440c7
Dispatch-Trailer: {"type":"trybot","CL":1206614,"patchset":7,"ref":"refs/changes/14/1206614/7","targetBranch":"master"}

cmd/cue/cmd/integration/workspace/workspace_folders_test.go

@@ -1,6 +1,8 @@
 package workspace
 
 import (
+	"fmt"
+	"path/filepath"
 	"testing"
 
 	"cuelang.org/go/internal/golangorgx/gopls/hooks"
@@ -66,3 +68,44 @@ package a
 		InitializeError("initialize: got 2 WorkspaceFolders; expected 1"),
 	).Run(t, files, nil)
 }
+
+// TODO(myitcv): add a test that verifies we get an error in the case that a
+// .cue file is opened "standalone", i.e. outside of the context of a workspace
+// folder. This is possible in VSCode at least. We currently implement the
+// error handling in vscode-cue in that instance but perhaps it should live in
+// 'cue lsp'.
+
+// TestNoContainingModule verifies that user is shown an error message in the
+// case that they open a .cue file in the context of a workspace folder where
+// the workspace folder does not correspond to the root of a CUE module. In
+// this case there is simply no CUE module.
+func TestNoContainingModule(t *testing.T) {
+	const files = `
+-- a.cue --
+package a
+`
+	WithOptions().Run(t, files, func(t *testing.T, env *Env) {
+		want := fmt.Sprintf("WorkspaceFolder %s does not correspond to a CUE module", env.Sandbox.Workdir.RootURI().Path())
+		env.Await(ShownMessage(want))
+	})
+}
+
+// TestNoContainingModule verifies that user is shown an error message in the
+// case that they open a .cue file in the context of a workspace folder where
+// the workspace folder does not correspond to the root of a CUE module. In
+// this case, the parent directory corresponds to the root of CUE module, but
+// the workspace folder itself corresponds to a subdirectory in the CUE module.
+func TestWorkspaceFolderWithCUEModInParent(t *testing.T) {
+	const files = `
+-- cue.mod/module.cue --
+-- a/a.cue --
+package a
+`
+	WithOptions(
+		WorkspaceFolders("a"),
+	).Run(t, files, func(t *testing.T, env *Env) {
+		workspaceFolder := filepath.Join(env.Sandbox.Workdir.RootURI().Path(), "a")
+		want := fmt.Sprintf("WorkspaceFolder %s does not correspond to a CUE module", workspaceFolder)
+		env.Await(ShownMessage(want))
+	})
+}

internal/golangorgx/gopls/cache/session.go

@@ -8,6 +8,7 @@ import (
 	"context"
 	"errors"
 	"fmt"
+	"path"
 	"strconv"
 	"strings"
 	"sync"
@@ -404,61 +405,37 @@ type viewDefiner interface{ definition() *viewDefinition }
 func bestView[V viewDefiner](ctx context.Context, fs file.Source, fh file.Handle, views []V) (V, error) {
 	var zero V
 
-	if len(views) == 0 {
-		return zero, nil // avoid the call to findRootPattern
+	// Given current limitations of cue lsp (exactly one workspace folder
+	// supported), and that we create a single view per workspace folder, we
+	// should assert here that we have a single view. Otherwise we have problems
+	if len(views) != 1 {
+		return zero, fmt.Errorf("expected exactly 1 view; saw %d", len(views))
 	}
-	uri := fh.URI()
-	dir := uri.Dir()
-	modURI, err := findRootPattern(ctx, dir, "cue.mod/module.cue", fs)
+
+	v := views[0]
+
+	fileDir := fh.URI().Dir()
+	fileBase := path.Base(string(fh.URI()))
+	modRoot, err := findRootPattern(ctx, fileDir, fileBase, fs)
 	if err != nil {
 		return zero, err
 	}
 
-	// Prefer GoWork > GoMod > GOPATH > GoPackages > AdHoc.
-	var (
-		goPackagesViews []V // prefer longest
-		workViews       []V // prefer longest
-		modViews        []V // exact match
-		gopathViews     []V // prefer longest
-		adHocViews      []V // exact match
-	)
-
-	for _, view := range views {
-		switch def := view.definition(); def.Type() {
-		case CUEModView:
-			if _, ok := def.workspaceModFiles[modURI]; ok {
-				modViews = append(modViews, view)
-			}
-		case AdHocView:
-			if def.root == dir {
-				adHocViews = append(adHocViews, view)
-			}
-		}
+	// Only if the module root corresponds to that of the view (workspace folder)
+	// do we match.
+	if modRoot == v.definition().root {
+		return v, nil
 	}
 
-	// Now that we've collected matching views, choose the best match,
-	// considering ports.
-	//
-	// We only consider one type of view, since the matching view created by
-	// defineView should be of the best type.
-	var bestViews []V
-	switch {
-	case len(workViews) > 0:
-		bestViews = workViews
-	case len(modViews) > 0:
-		bestViews = modViews
-	case len(gopathViews) > 0:
-		bestViews = gopathViews
-	case len(goPackagesViews) > 0:
-		bestViews = goPackagesViews
-	case len(adHocViews) > 0:
-		bestViews = adHocViews
-	default:
+	// TODO(myitcv): in the case of a nested module, we could prompt the user to
+	// do something here when we add support for multiple workspace folders. For
+	// now we simply return the zero view.
+	if v.definition().folder.Dir.Encloses(fh.URI()) {
 		return zero, nil
 	}
 
-	// TODO: we need to fix this
-	return bestViews[0], nil
+	// Perhaps a random file opened by the user?
+	return zero, nil
 }
 
 // updateViewLocked recreates the view with the given options.

internal/golangorgx/gopls/cache/view.go

@@ -143,11 +143,6 @@ type viewDefinition struct {
 	// the WorkspaceFolder folder
 	root protocol.DocumentURI
 
-	// cuemod is the path of the the nearest ancestor cue.mod directory
-	//
-	// TODO(myitcv): why do we need this if we have root?
-	cuemod protocol.DocumentURI
-
 	// workspaceModFiles holds the set of mod files active in this snapshot.
 	//
 	// For a go.work workspace, this is the set of workspace modfiles. For a
@@ -216,8 +211,7 @@ func viewDefinitionsEqual(x, y *viewDefinition) bool {
 	}
 	return x.folder == y.folder &&
 		x.typ == y.typ &&
-		x.root == y.root &&
-		x.cuemod == y.cuemod
+		x.root == y.root
 }
 
 // A ViewType describes how we load package information for a view.
@@ -552,44 +546,33 @@ func defineView(ctx context.Context, fs file.Source, folder *Folder, forFile fil
 	if err := checkPathValid(folder.Dir.Path()); err != nil {
 		return nil, fmt.Errorf("invalid workspace folder path: %w; check that the spelling of the configured workspace folder path agrees with the spelling reported by the operating system", err)
 	}
-	dir := folder.Dir.Path()
 
 	if forFile != nil {
-		// TODO(myitcv): it's still not totally clear what codepath leads us to this point.
-		// Under what conditions do we have forFile? We want to limit (for now) that files
-		// opened within the workspace on which we do analysis are part of the CUE module
-		// that contains the WorkspaceFolder. If forFile != nil, do we already have that
-		// guarantee here? Hopefully... because this feels awfully late to be doing anything
-		// about it.
-		dir = filepath.Dir(forFile.URI().Path())
+		// TODO(myitcv): fix the implementation here. forFile != nil when we are trying
+		// to compute the set of views given the set of open files/known folders. This is
+		// part of the zero config approach in gopls, and we don't have anything like that
+		// yet for 'cue lsp'.
+		return nil, fmt.Errorf("defineView with forFile != nil; not yet supported")
 	}
 
 	def := new(viewDefinition)
 	def.folder = folder
+	def.root = folder.Dir
 
-	var err error
-	dirURI := protocol.URIFromPath(dir)
-
-	// When deriving the best view for a given file, we only want to search
-	// up the directory hierarchy for cue.mod directories.
-	//
-	// TODO(myitcv): update this logic to support finding the cue.mod directory,
-	// which is the true marker of the CUE module.
-	moduleCUE, err := findRootPattern(ctx, dirURI, filepath.FromSlash("cue.mod/module.cue"), fs)
+	// Enforce that the workspace folder corresponds exactly to the root of a
+	// CUE module defined by the existence of a cue.mod/module.cue file.
+	targetFile := filepath.Join(folder.Dir.Path(), filepath.FromSlash("cue.mod/module.cue"))
+	targetURI := protocol.URIFromPath(targetFile)
+	modFile, err := fs.ReadFile(ctx, targetURI)
 	if err != nil {
-		return nil, err
+		return nil, err // cancelled
 	}
-	def.cuemod = moduleCUE.Dir()
-
-	// For now, we only support establishing a workspace folder when contained
-	// by a CUE module.
-	if def.cuemod != "" {
-		def.typ = CUEModView
-		def.root = def.cuemod.Dir()
-		return def, nil
+	if !fileExists(modFile) {
+		return nil, fmt.Errorf("WorkspaceFolder %s does not correspond to a CUE module", folder.Dir.Path())
 	}
 
-	return nil, fmt.Errorf("WorkspaceFolder %s is not contained by a CUE module", dir)
+	def.typ = CUEModView
+	return def, nil
 }
 
 // FetchGoEnv queries the environment and Go command to collect environment