Skip to content

Commit c9d1c7d

Browse files
authored
Migrate the workspace indexer to ArkFile and make it a query (#1266)
Branched from #1264 Progress towards #1212 With this PR the Ark indexer that currently powers legacy workspace symbols, completions, and diagnostics is migrated to sources from the Oak DB, instead of walking the disk and watching. Benefits: - Single source of truth - Less memory consumption - Reactivity to changes for workspace files that are not opened in the editor This last property is quite nice when changing git branches or editing files with an agent. The changes used to be ignored until the file was opened, but now the IDE reacts instantly. https://github.com/user-attachments/assets/0b99be22-54a8-4522-b233-164b02be979f - Building a file index is now a Salsa query over `oak_db::File`. Making it a query per file is especially nice during initial workspace scans where Salsa cancellations don't invalidate finished work per file, they remain cached across edits without having to finish building the entire index. This replaces the old global mutable index. - New `workspace_files()` helper that collects, as the name says, all workspace files. Unlike `all_files()`, library roots are excluded. This is what `indexer::find()` and `map()` use to walk over files. - Dropped the combined indexer + diagnostics queue. Now diagnostics query the index, so the ordering we used to enforce by hand (finish indexing, then run diagnostics) just falls out of the pull model. I've kept diagnostics batching and deduping as these are still useful. - Now that the indexer is a query, it's fully lazy. That's a good property in principle but can lead to user-visible delays when requesting workspace symbols in an empty workspace on startup. If a file is opened, diagnostics will query the index and warm it up, but if none are opened, nothing drives that. To fix this issue, we now warm up the index manually after the initial workspace scan has settled. This mirrors how Rust-Analyser warms up its own general-purpose queries after the language server reaches "quiescence" (after the initial scan, after a workspace refresh caused by changing Cargo.toml, etc). There is no such mechanism in ty, it's fully on-demand. I opted to go the r-a way as it's closer to what we had before. - Diagnostics triggering is now simplified by looking at `salsa::plumbing::current_revision()` from the main loop. Rust-Analyser uses it too, ty doesn't (they have a different pull model for diagnostics that I didn't investigate much yet, our push model is closer to R-A's). - The indexer was the only consumer of the rename events. We now watch files purely through `DidChangeWatchedFiles`, which is consistent which is also what ty and r-a do. The one event we might want to subscribe later on is `WillRename`. This would be useful to e.g. update `source()` paths when a file is renamed.
2 parents 09f4b1f + 26f55c8 commit c9d1c7d

17 files changed

Lines changed: 498 additions & 857 deletions

crates/ark/src/lsp/backend.rs

Lines changed: 0 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -136,9 +136,6 @@ pub(crate) enum LspNotification {
136136
DidChangeTextDocument(DidChangeTextDocumentParams),
137137
DidSaveTextDocument(DidSaveTextDocumentParams),
138138
DidCloseTextDocument(DidCloseTextDocumentParams),
139-
DidCreateFiles(CreateFilesParams),
140-
DidDeleteFiles(DeleteFilesParams),
141-
DidRenameFiles(RenameFilesParams),
142139
}
143140

144141
#[derive(Debug)]
@@ -297,18 +294,6 @@ impl LanguageServer for Backend {
297294
self.notify(LspNotification::DidChangeWatchedFiles(params));
298295
}
299296

300-
async fn did_create_files(&self, params: CreateFilesParams) {
301-
self.notify(LspNotification::DidCreateFiles(params));
302-
}
303-
304-
async fn did_delete_files(&self, params: DeleteFilesParams) {
305-
self.notify(LspNotification::DidDeleteFiles(params));
306-
}
307-
308-
async fn did_rename_files(&self, params: RenameFilesParams) {
309-
self.notify(LspNotification::DidRenameFiles(params));
310-
}
311-
312297
async fn symbol(
313298
&self,
314299
params: WorkspaceSymbolParams,

crates/ark/src/lsp/completions/sources/composite/call.rs

Lines changed: 7 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,7 @@ use crate::lsp::completions::sources::utils::call_node_position_type;
2121
use crate::lsp::completions::sources::utils::set_sort_text_by_first_appearance;
2222
use crate::lsp::completions::sources::utils::CallNodePositionType;
2323
use crate::lsp::completions::sources::CompletionSource;
24+
use crate::lsp::db::ArkDb;
2425
use crate::lsp::document_context::DocumentContext;
2526
use crate::lsp::indexer;
2627
use crate::lsp::traits::node::NodeExt;
@@ -93,7 +94,7 @@ fn completions_from_call(
9394
},
9495
};
9596

96-
completions_from_arguments(document_context, callee, object)
97+
completions_from_arguments(&context.state.db, document_context, callee, object)
9798
}
9899

99100
fn get_first_argument(context: &DocumentContext, node: &Node) -> anyhow::Result<Option<RObject>> {
@@ -156,6 +157,7 @@ fn get_first_argument(context: &DocumentContext, node: &Node) -> anyhow::Result<
156157
}
157158

158159
fn completions_from_arguments(
160+
db: &dyn ArkDb,
159161
context: &DocumentContext,
160162
callable: &str,
161163
object: RObject,
@@ -168,7 +170,7 @@ fn completions_from_arguments(
168170
return Ok(Some(completions));
169171
}
170172

171-
if let Some(completions) = completions_from_workspace_arguments(context, callable)? {
173+
if let Some(completions) = completions_from_workspace_arguments(db, callable, context)? {
172174
return Ok(Some(completions));
173175
}
174176

@@ -234,14 +236,15 @@ fn completions_from_session_arguments(
234236
}
235237

236238
fn completions_from_workspace_arguments(
237-
context: &DocumentContext,
239+
db: &dyn ArkDb,
238240
callable: &str,
241+
context: &DocumentContext,
239242
) -> anyhow::Result<Option<Vec<CompletionItem>>> {
240243
log::trace!("completions_from_workspace_arguments({callable:?})");
241244

242245
// Try to find the `callable` in the workspace and use its arguments
243246
// if we can
244-
let Some((_path, entry)) = indexer::find(callable) else {
247+
let Some(entry) = indexer::find(db, callable) else {
245248
// Didn't find any workspace object with this name
246249
return Ok(None);
247250
};

crates/ark/src/lsp/completions/sources/composite/workspace.rs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -69,7 +69,7 @@ fn completions_from_workspace(
6969
let token = token.as_str();
7070

7171
// get entries from the index
72-
indexer::map(|uri, symbol, entry| {
72+
indexer::map(&state.db, |uri, symbol, entry| {
7373
if !symbol.fuzzy_matches(token) {
7474
return;
7575
}
@@ -112,7 +112,7 @@ fn completions_from_workspace(
112112
let value = format!(
113113
"Defined in `{}` on line {}.",
114114
path,
115-
entry.range.start.line + 1
115+
entry.range.start.row + 1
116116
);
117117
let markup = MarkupContent {
118118
kind: MarkupKind::Markdown,

crates/ark/src/lsp/diagnostics.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -171,7 +171,7 @@ pub(crate) fn generate_diagnostics(
171171
context.document_symbols.push(HashMap::new());
172172

173173
// Add the current workspace symbols.
174-
indexer::map(|_uri, _symbol, entry| match &entry.data {
174+
indexer::map(db, |_uri, _symbol, entry| match &entry.data {
175175
indexer::IndexEntryData::Function { name, arguments: _ } => {
176176
context.workspace_symbols.insert(name.to_string());
177177
},

0 commit comments

Comments
 (0)