diff --git a/cmd/cue/cmd/integration/workspace/workspace_folders_test.go b/cmd/cue/cmd/integration/workspace/workspace_folders_test.go index 7d3ef23a1..857ed0786 100644 --- a/cmd/cue/cmd/integration/workspace/workspace_folders_test.go +++ b/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)) + }) +} diff --git a/internal/golangorgx/gopls/cache/session.go b/internal/golangorgx/gopls/cache/session.go index 9ed53748e..25f40975d 100644 --- a/internal/golangorgx/gopls/cache/session.go +++ b/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. diff --git a/internal/golangorgx/gopls/cache/view.go b/internal/golangorgx/gopls/cache/view.go index ed063c2cf..6e0a788fb 100644 --- a/internal/golangorgx/gopls/cache/view.go +++ b/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