Skip to content

Commit 60367ad

Browse files
committed
lsp/cache: notice on-disk creation of files satisfying failed imports
When deciding whether a changed file that is not open in the editor can influence any loaded package, DidModifyFiles considers the workspace's active files and directories, and embed attributes which match the file. But like embed attributes, import specs declare an interest in paths which may not exist yet: a loaded package can hold an import of a package which is yet to be created. Consequently, when the files of such an imported package were created on disk (not in the editor) in a directory containing no other active files, the creation was ignored entirely, and the import stayed unresolved until the importing package happened to be reloaded for some other reason. Also inspect a changed file when its directory is where the package of some loaded package's unresolved import would live, within the enclosing module or under the old module system's cue.mod/{gen|pkg|usr} hierarchies. The existing machinery then does the rest: the new file's package is loaded, and reloadPackages notices that the failed importers must be reloaded. Change-Id: I8f36fb8f0d725945dff78a6d85596a91539bc608 Signed-off-by: Matthew Sackman <matthew@cue.works> Reviewed-on: https://cue.gerrithub.io/c/cue-lang/cue/+/1242883 TryBot-Result: CUEcueckoo <cueckoo@cuelang.org> Reviewed-by: Roger Peppe <rogpeppe@gmail.com> Unity-Result: CUE porcuepine <cue.porcuepine@gmail.com>
1 parent 1a0b507 commit 60367ad

2 files changed

Lines changed: 71 additions & 10 deletions

File tree

cmd/cue/cmd/integration/workspace/imports_late_test.go

Lines changed: 16 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -64,9 +64,9 @@ ax: sub.y
6464

6565
// TestImportCreatedLaterOnDisk is like TestImportCreatedLater,
6666
// except the imported package is created on disk (not in the
67-
// editor), in a directory which contains no other active files. This
68-
// shows bad behaviour: the file's creation is ignored entirely, so
69-
// the new package is never loaded and the import stays unresolved.
67+
// editor), in a directory which contains no other active files. The
68+
// file's creation must still be noticed, because a loaded package
69+
// has an unresolved import which it helps satisfy.
7070
func TestImportCreatedLaterOnDisk(t *testing.T) {
7171
const files = `
7272
-- cue.mod/module.cue --
@@ -90,21 +90,27 @@ ax: sub.y
9090
)
9191

9292
// Now create the imported package on disk, without opening
93-
// any of its files in the editor. The creation is ignored:
94-
// the new package is never loaded, and the importing package
95-
// is not reloaded.
93+
// any of its files in the editor.
9694
env.WriteWorkspaceFile("sub/sub.cue", "package sub\n\ny: 4\n")
9795
env.Await(
9896
env.DoneWithChangeWatchedFiles(),
99-
NoLogMatching(protocol.Debug, `Package dirs=\[%v/sub\]`, rootURI),
100-
LogExactf(protocol.Debug, 1, false, "Package dirs=[%v] importPath=mod.example/x@v0:a Reloaded", rootURI),
97+
LogExactf(protocol.Debug, 1, false, "Package dirs=[%v/sub] importPath=mod.example/x/sub@v0 Reloaded", rootURI),
98+
// The importing package must be reloaded so that its
99+
// import now resolves.
100+
LogExactf(protocol.Debug, 2, false, "Package dirs=[%v] importPath=mod.example/x@v0:a Reloaded", rootURI),
101101
)
102102

103-
// Definitions on "y" within "ax: sub.y" find nothing.
104103
gotDefs := env.Definition(protocol.Location{
105104
URI: rootURI + "/a.cue",
106105
Range: protocol.Range{Start: protocol.Position{Line: 4, Character: 8}},
107106
})
108-
qt.Assert(t, qt.HasLen(gotDefs, 0))
107+
wantDefs := []protocol.Location{{
108+
URI: rootURI + "/sub/sub.cue",
109+
Range: protocol.Range{
110+
Start: protocol.Position{Line: 2, Character: 0},
111+
End: protocol.Position{Line: 2, Character: 1},
112+
},
113+
}}
114+
qt.Assert(t, qt.DeepEquals(gotDefs, wantDefs))
109115
})
110116
}

internal/lsp/cache/workspace.go

Lines changed: 55 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -366,6 +366,13 @@ func (w *Workspace) DidModifyFiles(ctx context.Context, modifications []file.Mod
366366
// matches this file, but does not currently embed it -
367367
// this file must be new in some way.
368368
needsInspecting = true
369+
} else if w.matchesUnresolvedImport(uri) {
370+
// A loaded cue package has an import which does not
371+
// currently resolve, and this file's directory is
372+
// where that import's package would live. Inspecting
373+
// this file will load its package, and reloadPackages
374+
// will then reload the failed importers.
375+
needsInspecting = true
369376
}
370377

371378
if !needsInspecting {
@@ -1118,6 +1125,54 @@ func (w *Workspace) matchesUnknownEmbedding(file protocol.DocumentURI) bool {
11181125
return false
11191126
}
11201127

1128+
// matchesUnresolvedImport reports whether any loaded cue package has
1129+
// an unresolved import which the given file could help satisfy: that
1130+
// is, whether the file's directory is a package directory implied by
1131+
// some unresolved import path.
1132+
func (w *Workspace) matchesUnresolvedImport(file protocol.DocumentURI) bool {
1133+
dir := file.Dir()
1134+
1135+
// Gather the unresolved import paths of all loaded packages: any
1136+
// loaded package, in any module, could hold an import which this
1137+
// file helps satisfy.
1138+
unresolved := make(map[ast.ImportPath]struct{})
1139+
for _, m := range w.modules {
1140+
for _, pkg := range m.packages {
1141+
maps.Copy(unresolved, pkg.unresolvedImports)
1142+
}
1143+
}
1144+
if len(unresolved) == 0 {
1145+
return false
1146+
}
1147+
1148+
// For each module enclosing the file, test whether the file's
1149+
// directory is where some unresolved import's package would
1150+
// live, either within the module itself, or under the old module
1151+
// system's cue.mod/{gen|pkg|usr} hierarchies.
1152+
for _, m := range w.modules {
1153+
if m.modFile == nil || !m.rootURI.Encloses(file) {
1154+
continue
1155+
}
1156+
modPath, _, _ := ast.SplitPackageVersion(m.modFile.QualifiedModule())
1157+
for ip := range unresolved {
1158+
if ip.Path == modPath && m.rootURI == dir {
1159+
return true
1160+
}
1161+
if relPath, ok := strings.CutPrefix(ip.Path, modPath+"/"); ok {
1162+
if joinURI(m.rootURI, relPath) == dir {
1163+
return true
1164+
}
1165+
}
1166+
for _, prefix := range []string{"cue.mod/gen/", "cue.mod/pkg/", "cue.mod/usr/"} {
1167+
if joinURI(m.rootURI, prefix+ip.Path) == dir {
1168+
return true
1169+
}
1170+
}
1171+
}
1172+
}
1173+
return false
1174+
}
1175+
11211176
// markUnresolvedImportersDirty marks as dirty every loaded package
11221177
// which has an unresolved import that a package with the given
11231178
// import path would satisfy. It reports whether any package was

0 commit comments

Comments
 (0)