Skip to content

Commit a9b0bf2

Browse files
lionel-lionel-
committed
Add Document::url for verbatim wire output
1 parent 11deb77 commit a9b0bf2

8 files changed

Lines changed: 82 additions & 88 deletions

File tree

‎crates/ark/src/lsp/document.rs‎

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -11,13 +11,26 @@ use aether_lsp_utils::proto::PositionEncoding;
1111
use anyhow::anyhow;
1212
use oak_semantic::semantic_index::SemanticIndex;
1313
use tower_lsp::lsp_types;
14+
use tower_lsp::lsp_types::Url;
1415
use tree_sitter::Parser;
1516
use tree_sitter::Tree;
1617

1718
use crate::lsp::config::DocumentConfig;
1819

20+
/// Placeholder URL used by `Document::new`. Production paths replace
21+
/// this with the editor's verbatim URL via `WorldState::insert_document`.
22+
/// Tests that don't go through `WorldState` leave the placeholder.
23+
const PLACEHOLDER_URL: &str = "ark://internal/document-url-unset";
24+
1925
#[derive(Clone)]
2026
pub struct Document {
27+
/// The editor's URL for this document, byte-for-byte as received in
28+
/// `did_open`. Used for wire output (diagnostics, etc.) so the
29+
/// frontend always sees the URI it sent us, never a normalised
30+
/// round-trip. `WorldState::insert_document` populates this; the
31+
/// default constructor uses a placeholder.
32+
pub url: Url,
33+
2134
/// The document's textual contents.
2235
pub contents: String,
2336

@@ -76,6 +89,7 @@ impl Document {
7689
let line_index = biome_line_index::LineIndex::new(&contents);
7790

7891
Self {
92+
url: Url::parse(PLACEHOLDER_URL).expect("placeholder URL parses"),
7993
contents,
8094
version,
8195
ast,

‎crates/ark/src/lsp/goto_definition.rs‎

Lines changed: 24 additions & 52 deletions
Original file line numberDiff line numberDiff line change
@@ -164,7 +164,7 @@ mod parked_salsa_tests {
164164

165165
fn make_state(uri: &lsp_types::Url, doc: &Document) -> WorldState {
166166
let mut state = WorldState::default();
167-
state.documents.insert(uri.clone(), doc.clone());
167+
state.insert_document(uri.clone(), doc.clone());
168168
state
169169
}
170170

@@ -229,8 +229,8 @@ mod parked_salsa_tests {
229229
let pkg = Package::from_parts(pkg_root, desc, ns);
230230

231231
let mut state = WorldState::default();
232-
state.documents.insert(uri_aaa.clone(), doc_aaa.clone());
233-
state.documents.insert(uri_ccc.clone(), doc_ccc.clone());
232+
state.insert_document(uri_aaa.clone(), doc_aaa.clone());
233+
state.insert_document(uri_ccc.clone(), doc_ccc.clone());
234234
state.root = Some(SourceRoot::Package(pkg));
235235

236236
// ccc.R sees bbb.R's definition (later in collation)
@@ -283,9 +283,9 @@ mod parked_salsa_tests {
283283
let pkg = Package::from_parts(pkg_root, desc, ns);
284284

285285
let mut state = WorldState::default();
286-
state.documents.insert(uri_aaa.clone(), doc_aaa.clone());
287-
state.documents.insert(uri_bbb.clone(), doc_bbb);
288-
state.documents.insert(uri_ccc.clone(), doc_ccc.clone());
286+
state.insert_document(uri_aaa.clone(), doc_aaa.clone());
287+
state.insert_document(uri_bbb.clone(), doc_bbb);
288+
state.insert_document(uri_ccc.clone(), doc_ccc.clone());
289289
state.root = Some(SourceRoot::Package(pkg));
290290

291291
// aaa.R is now last in collation, so it can see bbb.R's definition
@@ -332,7 +332,7 @@ mod parked_salsa_tests {
332332
let pkg = Package::from_parts(pkg_root, desc, ns);
333333

334334
let mut state = WorldState::default();
335-
state.documents.insert(uri_aaa.clone(), doc_aaa.clone());
335+
state.insert_document(uri_aaa.clone(), doc_aaa.clone());
336336
state.root = Some(SourceRoot::Package(pkg));
337337

338338
// Cursor on `helper` inside the function body (line 0, col 16)
@@ -464,7 +464,7 @@ mod parked_salsa_tests {
464464
// `is.null` is NOT in the INDEX
465465
let doc_null = Document::new("is.null(1)\n", None);
466466
let uri_null = lsp_types::Url::from_file_path(pkg_root.join("R/bar.R")).unwrap();
467-
state.documents.insert(uri_null.clone(), doc_null.clone());
467+
state.insert_document(uri_null.clone(), doc_null.clone());
468468

469469
let params = make_params(uri_null, 0, 0);
470470
let result = goto_definition(&doc_null, params, &state).unwrap();
@@ -523,9 +523,7 @@ mod parked_salsa_tests {
523523
let helpers_uri = lsp_types::Url::from_file_path(dir.path().join("helpers.R")).unwrap();
524524

525525
let mut state = WorldState::default();
526-
state
527-
.documents
528-
.insert(script_uri.clone(), script_doc.clone());
526+
state.insert_document(script_uri.clone(), script_doc.clone());
529527
state.workspace.folders = vec![lsp_types::Url::from_directory_path(dir.path()).unwrap()];
530528

531529
let params = make_params(script_uri, 1, 0);
@@ -551,10 +549,8 @@ mod parked_salsa_tests {
551549
let script_uri = lsp_types::Url::from_file_path(script_dir.join("script.R")).unwrap();
552550

553551
let mut state = WorldState::default();
554-
state.documents.insert(helpers_uri.clone(), helpers_doc);
555-
state
556-
.documents
557-
.insert(script_uri.clone(), script_doc.clone());
552+
state.insert_document(helpers_uri.clone(), helpers_doc);
553+
state.insert_document(script_uri.clone(), script_doc.clone());
558554

559555
// Cursor on `helper` (line 1, col 0)
560556
let params = make_params(script_uri, 1, 0);
@@ -594,7 +590,7 @@ mod parked_salsa_tests {
594590

595591
let mut state = make_state(&script_uri, &script_doc);
596592
state.library = library;
597-
state.documents.insert(helpers_uri.clone(), helpers_doc);
593+
state.insert_document(helpers_uri.clone(), helpers_doc);
598594

599595
// `mutate` (line 1) resolves via dplyr, attached by helpers.R's library() call.
600596
// Package symbol, no NavigationTarget.
@@ -634,9 +630,7 @@ mod parked_salsa_tests {
634630
lsp_types::Url::from_file_path(script_dir.path().join("script.R")).unwrap();
635631

636632
let mut state = WorldState::default();
637-
state
638-
.documents
639-
.insert(script_uri.clone(), script_doc.clone());
633+
state.insert_document(script_uri.clone(), script_doc.clone());
640634
// helpers.R is intentionally NOT inserted into state.documents
641635

642636
let params = make_params(script_uri, 1, 0);
@@ -755,9 +749,7 @@ mod parked_salsa_tests {
755749
let script_uri = lsp_types::Url::from_file_path(dir.path().join("script.R")).unwrap();
756750

757751
let mut state = WorldState::default();
758-
state
759-
.documents
760-
.insert(script_uri.clone(), script_doc.clone());
752+
state.insert_document(script_uri.clone(), script_doc.clone());
761753

762754
// Should resolve without hanging. Both symbols are reachable
763755
// because a.R is visited first (gets its exports + b.R's exports),
@@ -801,9 +793,7 @@ mod parked_salsa_tests {
801793
let script_uri = lsp_types::Url::from_file_path(dir.path().join("script.R")).unwrap();
802794

803795
let mut state = WorldState::default();
804-
state
805-
.documents
806-
.insert(script_uri.clone(), script_doc.clone());
796+
state.insert_document(script_uri.clone(), script_doc.clone());
807797

808798
let helpers_uri = lsp_types::Url::from_file_path(dir.path().join("helpers.R")).unwrap();
809799

@@ -848,9 +838,7 @@ mod parked_salsa_tests {
848838
let script_uri = lsp_types::Url::from_file_path(dir.path().join("script.R")).unwrap();
849839

850840
let mut state = WorldState::default();
851-
state
852-
.documents
853-
.insert(script_uri.clone(), script_doc.clone());
841+
state.insert_document(script_uri.clone(), script_doc.clone());
854842

855843
// `foo` on line 2 should resolve to the LOCAL definition on line 1,
856844
// not to the sourced one from helpers.R.
@@ -897,9 +885,7 @@ mod parked_salsa_tests {
897885
let script_uri = lsp_types::Url::from_file_path(dir.path().join("script.R")).unwrap();
898886

899887
let mut state = WorldState::default();
900-
state
901-
.documents
902-
.insert(script_uri.clone(), script_doc.clone());
888+
state.insert_document(script_uri.clone(), script_doc.clone());
903889

904890
let helpers_uri = lsp_types::Url::from_file_path(dir.path().join("helpers.R")).unwrap();
905891
let a_uri = lsp_types::Url::from_file_path(dir.path().join("a.R")).unwrap();
@@ -947,9 +933,7 @@ mod parked_salsa_tests {
947933
let script_doc = Document::new("source(\"script.R\")\nmy_var <- 1\nmy_var\n", None);
948934

949935
let mut state = WorldState::default();
950-
state
951-
.documents
952-
.insert(script_uri.clone(), script_doc.clone());
936+
state.insert_document(script_uri.clone(), script_doc.clone());
953937

954938
// `my_var` (line 2) should resolve to its own definition on line 1,
955939
// not to a Sourced duplicate.
@@ -993,9 +977,7 @@ mod parked_salsa_tests {
993977
let script_uri = lsp_types::Url::from_file_path(dir.path().join("script.R")).unwrap();
994978

995979
let mut state = WorldState::default();
996-
state
997-
.documents
998-
.insert(script_uri.clone(), script_doc.clone());
980+
state.insert_document(script_uri.clone(), script_doc.clone());
999981

1000982
let (index, file_scope) = state.file_analysis(&script_uri, &script_doc);
1001983

@@ -1033,9 +1015,7 @@ mod parked_salsa_tests {
10331015
let script_uri = lsp_types::Url::from_file_path(dir.path().join("script.R")).unwrap();
10341016

10351017
let mut state = WorldState::default();
1036-
state
1037-
.documents
1038-
.insert(script_uri.clone(), script_doc.clone());
1018+
state.insert_document(script_uri.clone(), script_doc.clone());
10391019

10401020
let b_uri = lsp_types::Url::from_file_path(dir.path().join("b.R")).unwrap();
10411021

@@ -1063,9 +1043,7 @@ mod parked_salsa_tests {
10631043
let script_uri = lsp_types::Url::from_file_path(dir.path().join("script.R")).unwrap();
10641044

10651045
let mut state = WorldState::default();
1066-
state
1067-
.documents
1068-
.insert(script_uri.clone(), script_doc.clone());
1046+
state.insert_document(script_uri.clone(), script_doc.clone());
10691047

10701048
let helpers_uri = lsp_types::Url::from_file_path(dir.path().join("helpers.R")).unwrap();
10711049

@@ -1103,9 +1081,7 @@ mod parked_salsa_tests {
11031081
let script_uri = lsp_types::Url::from_file_path(dir.path().join("script.R")).unwrap();
11041082

11051083
let mut state = WorldState::default();
1106-
state
1107-
.documents
1108-
.insert(script_uri.clone(), script_doc.clone());
1084+
state.insert_document(script_uri.clone(), script_doc.clone());
11091085

11101086
let helpers_uri = lsp_types::Url::from_file_path(dir.path().join("helpers.R")).unwrap();
11111087

@@ -1138,9 +1114,7 @@ mod parked_salsa_tests {
11381114
let helpers_uri = lsp_types::Url::from_file_path(dir.path().join("helpers.R")).unwrap();
11391115

11401116
let mut state = WorldState::default();
1141-
state
1142-
.documents
1143-
.insert(script_uri.clone(), script_doc.clone());
1117+
state.insert_document(script_uri.clone(), script_doc.clone());
11441118

11451119
// `helper` on line 2 should resolve to helpers.R (source() shadows local)
11461120
let params = make_params(script_uri, 2, 0);
@@ -1177,9 +1151,7 @@ mod parked_salsa_tests {
11771151
let helpers_uri = lsp_types::Url::from_file_path(dir.path().join("helpers.R")).unwrap();
11781152

11791153
let mut state = WorldState::default();
1180-
state
1181-
.documents
1182-
.insert(script_uri.clone(), script_doc.clone());
1154+
state.insert_document(script_uri.clone(), script_doc.clone());
11831155

11841156
// `fn` on line 1 should resolve to the SECOND def in helpers.R (line 1)
11851157
let params = make_params(script_uri, 1, 0);

‎crates/ark/src/lsp/main_loop.rs‎

Lines changed: 5 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -455,12 +455,7 @@ impl GlobalState {
455455
// time: a buffer may have opened or closed since the scan
456456
// kicked off. The buffer-drain inside `apply_scan_completed` uses
457457
// this set as its watcher-event `skip` argument.
458-
let editor_owned: HashSet<FilePath> = self.world
459-
.documents
460-
.keys()
461-
.map(FilePath::from_url)
462-
.collect();
463-
458+
let editor_owned: HashSet<FilePath> = self.world.documents.keys().cloned().collect();
464459
let followups = self.lsp_state.oak_scheduler.apply_scan_completed(
465460
&mut self.world.db,
466461
scan,
@@ -1000,7 +995,7 @@ async fn process_diagnostics_batch(batch: Vec<RefreshDiagnosticsTask>) {
1000995
futures.push(task::spawn_blocking(move || {
1001996
let _span = tracing::info_span!("diagnostics_refresh", uri = %uri).entered();
1002997

1003-
if let Some(document) = state.documents.get(&uri) {
998+
if let Some(document) = state.documents.get(&FilePath::from_url(&uri)) {
1004999
// Special case testthat-specific behaviour. This is a simple
10051000
// stopgap approach that has some false positives (e.g. when we
10061001
// work on testthat itself the flag will always be true), but
@@ -1136,8 +1131,8 @@ pub(crate) fn diagnostics_refresh_all(state: WorldState) {
11361131
n = state.documents.len()
11371132
);
11381133

1139-
for (uri, _document) in state.documents.iter() {
1140-
if !ExtUrl::should_diagnose(uri) {
1134+
for document in state.documents.values() {
1135+
if !ExtUrl::should_diagnose(&document.url) {
11411136
continue;
11421137
}
11431138

@@ -1147,7 +1142,7 @@ pub(crate) fn diagnostics_refresh_all(state: WorldState) {
11471142
// non-oak state).
11481143
INDEXER_QUEUE
11491144
.send(IndexerQueueTask::Diagnostics(RefreshDiagnosticsTask {
1150-
uri: uri.clone(),
1145+
uri: document.url.clone(),
11511146
state: state.legacy_snapshot(),
11521147
}))
11531148
.unwrap_or_else(|err| lsp::log_error!("Failed to queue diagnostics refresh: {err}"));

‎crates/ark/src/lsp/state.rs‎

Lines changed: 23 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
use std::collections::HashMap;
22
use std::path::Path;
33

4+
use aether_path::FilePath;
45
use anyhow::anyhow;
56
use oak_db::OakDatabase;
67
use oak_semantic::library::Library;
@@ -18,8 +19,10 @@ pub(crate) struct WorldState {
1819
/// Salsa input tree for Oak queries.
1920
pub(crate) db: OakDatabase,
2021

21-
/// Watched documents
22-
pub(crate) documents: HashMap<Url, Document>,
22+
/// Watched documents, keyed on the normalised [`FilePath`] form.
23+
/// The verbatim editor URL is preserved on each [`Document::url`]
24+
/// for wire output.
25+
pub(crate) documents: HashMap<FilePath, Document>,
2326

2427
/// Watched folders
2528
pub(crate) workspace: Workspace,
@@ -79,15 +82,17 @@ impl WorldState {
7982
}
8083

8184
pub(crate) fn get_document(&self, uri: &Url) -> anyhow::Result<&Document> {
82-
if let Some(doc) = self.documents.get(uri) {
85+
let key = FilePath::from_url(uri);
86+
if let Some(doc) = self.documents.get(&key) {
8387
Ok(doc)
8488
} else {
8589
Err(anyhow!("Can't find document for URI {uri}"))
8690
}
8791
}
8892

8993
pub(crate) fn get_document_mut(&mut self, uri: &Url) -> anyhow::Result<&mut Document> {
90-
if let Some(doc) = self.documents.get_mut(uri) {
94+
let key = FilePath::from_url(uri);
95+
if let Some(doc) = self.documents.get_mut(&key) {
9196
Ok(doc)
9297
} else {
9398
Err(anyhow!("Can't find document for URI {uri}"))
@@ -115,6 +120,15 @@ impl WorldState {
115120
..self.clone()
116121
}
117122
}
123+
124+
/// Insert a document, keying on the normalised [`FilePath`] and
125+
/// stashing the verbatim editor URL on [`Document::url`] for wire
126+
/// output.
127+
pub(crate) fn insert_document(&mut self, uri: Url, mut doc: Document) {
128+
let key = FilePath::from_url(&uri);
129+
doc.url = uri;
130+
self.documents.insert(key, doc);
131+
}
118132
}
119133

120134
pub(crate) fn with_document<T, F>(
@@ -151,8 +165,11 @@ where
151165
}
152166

153167
pub(crate) fn workspace_uris(state: &WorldState) -> Vec<Url> {
154-
let uris: Vec<Url> = state.documents.iter().map(|elt| elt.0.clone()).collect();
155-
uris
168+
state
169+
.documents
170+
.values()
171+
.map(|doc| doc.url.clone())
172+
.collect()
156173
}
157174

158175
#[cfg(test)]

0 commit comments

Comments
 (0)