diff --git a/Cargo.lock b/Cargo.lock index cb80ac7259..79aa61c25c 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -203,16 +203,14 @@ checksum = "320119579fcad9c21884f5c4861d16174d0e06250625266f50fe6898340abefa" [[package]] name = "aether_lsp_utils" version = "0.0.0" -source = "git+https://github.com/posit-dev/air?rev=d2659d5b158374bf486b594625ca50abbd0ac879#d2659d5b158374bf486b594625ca50abbd0ac879" dependencies = [ "anyhow", "biome_line_index", "biome_text_size", "dissimilar", - "itertools 0.13.0", - "line_ending", - "settings", - "tower-lsp", + "itertools", + "memchr", + "tower-lsp-server", ] [[package]] @@ -410,6 +408,7 @@ dependencies = [ "base64", "biome_line_index", "biome_rowan", + "biome_text_size", "blake3", "bus", "camino", @@ -419,14 +418,14 @@ dependencies = [ "crossbeam", "ctor", "dap", - "dashmap 6.2.1", + "dashmap", "ego-tree", "embed-resource", "futures", "harp", "home", "insta", - "itertools 0.15.0", + "itertools", "libc", "libr", "log", @@ -459,7 +458,7 @@ dependencies = [ "strum_macros", "tempfile", "tokio", - "tower-lsp", + "tower-lsp-server", "tracing", "tracing-appender", "tracing-error", @@ -498,9 +497,10 @@ dependencies = [ "regex", "serde", "serde_json", + "socket2 0.6.3", "stdext", "tempfile", - "tower-lsp", + "tower-lsp-server", "tree-sitter", "url", "uuid", @@ -547,17 +547,6 @@ version = "1.1.2" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "1505bd5d3d116872e7271a6d4e16d81d0c8570876c8de68093a09ac269d8aac0" -[[package]] -name = "auto_impl" -version = "1.3.0" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "ffdcb70bdbc4d478427380519163274ac86e52916e10f0a8889adf0f96d3fee7" -dependencies = [ - "proc-macro2", - "quote", - "syn 2.0.119", -] - [[package]] name = "autocfg" version = "1.5.0" @@ -741,6 +730,12 @@ dependencies = [ "hybrid-array", ] +[[package]] +name = "borrow-or-share" +version = "0.2.4" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "dc0b364ead1874514c8c2855ab558056ebfeb775653e7ae45ff72f28f8f3166c" + [[package]] name = "boxcar" version = "0.2.14" @@ -1137,19 +1132,6 @@ dependencies = [ "syn 2.0.119", ] -[[package]] -name = "dashmap" -version = "5.5.3" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "978747c1d849a7d2ee5e8adc0159961c48fb7e5db2f06af6723b80123bb53856" -dependencies = [ - "cfg-if", - "hashbrown 0.14.5", - "lock_api", - "once_cell", - "parking_lot_core", -] - [[package]] name = "dashmap" version = "6.2.1" @@ -1440,6 +1422,17 @@ dependencies = [ "miniz_oxide", ] +[[package]] +name = "fluent-uri" +version = "0.4.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "bc74ac4d8359ae70623506d512209619e5cf8f347124910440dbc221714b328e" +dependencies = [ + "borrow-or-share", + "ref-cast", + "serde", +] + [[package]] name = "fnv" version = "1.0.7" @@ -1659,7 +1652,7 @@ dependencies = [ "ctor", "embed-resource", "harp_macros", - "itertools 0.15.0", + "itertools", "libc", "libloading", "libr", @@ -2152,15 +2145,6 @@ version = "1.70.2" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "a6cb138bb79a146c1bd460005623e142ef0181e3d0219cb493e02f7d08a35695" -[[package]] -name = "itertools" -version = "0.13.0" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "413ee7dfc52ee1a4949ceeb7dbc8a33f2d6c088194d9f922fb8318faf1f01186" -dependencies = [ - "either", -] - [[package]] name = "itertools" version = "0.15.0" @@ -2305,15 +2289,6 @@ dependencies = [ "libc", ] -[[package]] -name = "line_ending" -version = "0.0.0" -source = "git+https://github.com/posit-dev/air?rev=d2659d5b158374bf486b594625ca50abbd0ac879#d2659d5b158374bf486b594625ca50abbd0ac879" -dependencies = [ - "memchr", - "settings", -] - [[package]] name = "link-section" version = "0.19.1" @@ -2371,16 +2346,16 @@ source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "0ceec5bc11778974d1bcb055b18002eba7f4b3518b6a0081b3af5f21666da9ad" [[package]] -name = "lsp-types" -version = "0.94.1" +name = "ls-types" +version = "0.0.6" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "c66bfd44a06ae10647fe3f8214762e9369fd4248df1350924b4ef9e770a85ea1" +checksum = "896e16b8e17d8732b9efe4d5b66cb0cc162b3023a2d8122f2aea6f7f185e0a67" dependencies = [ - "bitflags 1.3.2", + "bitflags 2.11.1", + "fluent-uri", + "percent-encoding", "serde", "serde_json", - "serde_repr", - "url", ] [[package]] @@ -2661,7 +2636,7 @@ dependencies = [ "anyhow", "biome_rowan", "biome_text_size", - "itertools 0.15.0", + "itertools", "log", "oak_core", "oak_index_vec", @@ -2817,26 +2792,6 @@ dependencies = [ "siphasher", ] -[[package]] -name = "pin-project" -version = "1.1.11" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "f1749c7ed4bcaf4c3d0a3efc28538844fb29bcdd7d2b67b2be7e20ba861ff517" -dependencies = [ - "pin-project-internal", -] - -[[package]] -name = "pin-project-internal" -version = "1.1.11" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "d9b20ed30f105399776b9c883e68e536ef602a16ae6f596d2c473591d6ad64c6" -dependencies = [ - "proc-macro2", - "quote", - "syn 2.0.119", -] - [[package]] name = "pin-project-lite" version = "0.2.17" @@ -3124,7 +3079,7 @@ dependencies = [ "serde_json", "sync_wrapper", "tokio", - "tower 0.5.3", + "tower", "tower-http", "tower-service", "url", @@ -3556,11 +3511,6 @@ dependencies = [ "stable_deref_trait", ] -[[package]] -name = "settings" -version = "0.0.0" -source = "git+https://github.com/posit-dev/air?rev=d2659d5b158374bf486b594625ca50abbd0ac879#d2659d5b158374bf486b594625ca50abbd0ac879" - [[package]] name = "sha1" version = "0.11.0" @@ -4087,20 +4037,6 @@ version = "1.1.1+spec-1.1.0" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "756daf9b1013ebe47a8776667b466417e2d4c5679d441c26230efd9ef78692db" -[[package]] -name = "tower" -version = "0.4.13" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "b8fa9be0de6cf49e536ce1851f987bd21a43b771b09473c3549a6c853db37c1c" -dependencies = [ - "futures-core", - "futures-util", - "pin-project", - "pin-project-lite", - "tower-layer", - "tower-service", -] - [[package]] name = "tower" version = "0.5.3" @@ -4129,7 +4065,7 @@ dependencies = [ "http-body", "iri-string", "pin-project-lite", - "tower 0.5.3", + "tower", "tower-layer", "tower-service", ] @@ -4141,37 +4077,25 @@ source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "121c2a6cda46980bb0fcd1647ffaf6cd3fc79a013de288782836f6df9c48780e" [[package]] -name = "tower-lsp" -version = "0.20.0" -source = "git+https://github.com/lionel-/tower-lsp?branch=bugfix%2Fpatches#49ef549eaa9f74b71e212cf513283af4ad748a81" +name = "tower-lsp-server" +version = "0.23.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "2f0e711655c89181a6bc6a2cc348131fcd9680085f5b06b6af13427a393a6e72" dependencies = [ - "async-trait", - "auto_impl", "bytes", - "dashmap 5.5.3", + "dashmap", "futures", "httparse", - "lsp-types", + "ls-types", "memchr", "serde", "serde_json", "tokio", "tokio-util", - "tower 0.4.13", - "tower-lsp-macros", + "tower", "tracing", ] -[[package]] -name = "tower-lsp-macros" -version = "0.9.0" -source = "git+https://github.com/lionel-/tower-lsp?branch=bugfix%2Fpatches#49ef549eaa9f74b71e212cf513283af4ad748a81" -dependencies = [ - "proc-macro2", - "quote", - "syn 2.0.119", -] - [[package]] name = "tower-service" version = "0.3.3" @@ -4405,7 +4329,6 @@ dependencies = [ "idna", "percent-encoding", "serde", - "serde_derive", ] [[package]] diff --git a/Cargo.toml b/Cargo.toml index 97faf751ff..3338a7f440 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -26,7 +26,7 @@ too_many_arguments = "allow" [workspace.dependencies] actix-web = "4.13.0" aether_factory = { git = "https://github.com/posit-dev/air", package = "air_r_factory", rev = "d2659d5b158374bf486b594625ca50abbd0ac879" } -aether_lsp_utils = { git = "https://github.com/posit-dev/air", rev = "d2659d5b158374bf486b594625ca50abbd0ac879" } +aether_lsp_utils = { path = "crates/aether_lsp_utils" } aether_parser = { git = "https://github.com/posit-dev/air", package = "air_r_parser", rev = "d2659d5b158374bf486b594625ca50abbd0ac879" } aether_syntax = { git = "https://github.com/posit-dev/air", package = "air_r_syntax", rev = "d2659d5b158374bf486b594625ca50abbd0ac879" } aether_path = { path = "crates/aether_path" } @@ -53,6 +53,7 @@ ctor = "1.0.0" dap = { branch = "main", git = "https://github.com/sztomi/dap-rs" } dashmap = "6.1.0" dirs = "6.0.0" +dissimilar = "1.0.9" ego-tree = "0.11.0" embed-resource = "3.0.9" env_logger = "0.11.10" @@ -72,6 +73,7 @@ libc = "0.2.186" libloading = "0.9.0" libr = { path = "crates/libr" } log = "0.4.29" +memchr = "2.7.4" mime_guess = "2.0.5" nix = { version = "0.31", features = ["fs", "poll", "process", "signal"] } notify = "8.2.0" @@ -108,6 +110,7 @@ serde_repr = "0.1.20" serde_with = "3.19.0" sha2 = "0.11.0" smallvec = "1.15.1" +socket2 = "0.6.3" stdext = { path = "crates/stdext" } streaming-iterator = "0.1.9" strum = "0.28.0" @@ -116,8 +119,7 @@ syn = { version = "2.0.117", features = ["full"] } tar = "0.4.45" tempfile = "3.27.0" tokio = { version = "1.52.1", features = ["full"] } -# For https://github.com/ebkalderon/tower-lsp/pull/428 -tower-lsp = { branch = "bugfix/patches", git = "https://github.com/lionel-/tower-lsp" } +tower-lsp-server = "0.23.0" tracing = "0.1.44" tracing-appender = "0.2.5" tracing-error = "0.2.1" diff --git a/crates/aether_lsp_utils/Cargo.toml b/crates/aether_lsp_utils/Cargo.toml new file mode 100644 index 0000000000..3e9c68a011 --- /dev/null +++ b/crates/aether_lsp_utils/Cargo.toml @@ -0,0 +1,19 @@ +[package] +name = "aether_lsp_utils" +version = "0.0.0" +authors.workspace = true +edition.workspace = true +license.workspace = true +rust-version.workspace = true + +[dependencies] +anyhow.workspace = true +biome_line_index.workspace = true +biome_text_size.workspace = true +dissimilar.workspace = true +itertools.workspace = true +memchr.workspace = true +tower-lsp-server.workspace = true + +[lints] +workspace = true diff --git a/crates/aether_lsp_utils/src/diff.rs b/crates/aether_lsp_utils/src/diff.rs new file mode 100644 index 0000000000..95a90cb218 --- /dev/null +++ b/crates/aether_lsp_utils/src/diff.rs @@ -0,0 +1,45 @@ +// --- source +// authors = ["rust-analyzer team"] +// license = "MIT OR Apache-2.0" +// origin = "https://github.com/rust-lang/rust-analyzer/blob/8d5e91c9/crates/rust-analyzer/src/handlers/request.rs#L2483" +// --- + +use biome_text_size::TextRange; +use biome_text_size::TextSize; + +use super::text_edit::TextEdit; + +pub fn diff(left: &str, right: &str) -> TextEdit { + use dissimilar::Chunk; + + let chunks = dissimilar::diff(left, right); + + let mut builder = TextEdit::builder(); + let mut pos = TextSize::default(); + + let mut chunks = chunks.into_iter().peekable(); + while let Some(chunk) = chunks.next() { + if let (Chunk::Delete(deleted), Some(&Chunk::Insert(inserted))) = (chunk, chunks.peek()) { + chunks.next().unwrap(); + let deleted_len = TextSize::of(deleted); + builder.replace(TextRange::at(pos, deleted_len), inserted.into()); + pos += deleted_len; + continue; + } + + match chunk { + Chunk::Equal(text) => { + pos += TextSize::of(text); + }, + Chunk::Delete(deleted) => { + let deleted_len = TextSize::of(deleted); + builder.delete(TextRange::at(pos, deleted_len)); + pos += deleted_len; + }, + Chunk::Insert(inserted) => { + builder.insert(pos, inserted.into()); + }, + } + } + builder.finish() +} diff --git a/crates/aether_lsp_utils/src/lib.rs b/crates/aether_lsp_utils/src/lib.rs new file mode 100644 index 0000000000..a97e071db8 --- /dev/null +++ b/crates/aether_lsp_utils/src/lib.rs @@ -0,0 +1,4 @@ +pub mod diff; +pub mod line_ending; +pub mod proto; +pub mod text_edit; diff --git a/crates/aether_lsp_utils/src/line_ending.rs b/crates/aether_lsp_utils/src/line_ending.rs new file mode 100644 index 0000000000..c5a6869582 --- /dev/null +++ b/crates/aether_lsp_utils/src/line_ending.rs @@ -0,0 +1,134 @@ +//! We maintain the invariant that all internal strings use `\n` as line separator. +//! This module does line ending conversion and detection (so that we can +//! convert back to `\r\n` on the way out as needed). +//! +//! Vendored from `posit-dev/air`'s `line_ending` and `settings::LineEnding` +//! crates, dropping the parts unrelated to LSP position/text conversion +//! (`settings::LineEnding`'s `biome_formatter` conversions). + +use std::borrow::Cow; +use std::fmt; +use std::sync::LazyLock; + +use memchr::memmem; + +#[derive(Copy, Clone, Default, Debug, Eq, PartialEq)] +pub enum LineEnding { + /// Line endings will be converted to `\n` as is common on Unix. + #[default] + Lf, + + /// Line endings will be converted to `\r\n` as is common on Windows. + Crlf, +} + +impl fmt::Display for LineEnding { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + match self { + Self::Lf => write!(f, "LF"), + Self::Crlf => write!(f, "CRLF"), + } + } +} + +static FINDER: LazyLock = LazyLock::new(|| memmem::Finder::new(b"\r\n")); + +pub fn infer(x: &str) -> LineEnding { + match FINDER.find(x.as_bytes()) { + // Saw `\r\n` + Some(_) => LineEnding::Crlf, + // No `\r\n`, or empty file + None => LineEnding::Lf, + } +} + +/// Normalize line endings within a string +/// +/// We replace `\r\n` with `\n` in-place, which doesn't break utf-8 encoding. +/// While we *can* call `as_mut_vec` and do surgery on the live string +/// directly, let's rather steal the contents of `x`. This makes the code +/// safe even if a panic occurs. +/// +/// # Source +/// +/// --- +/// authors = ["rust-analyzer team"] +/// license = "MIT OR Apache-2.0" +/// origin = "https://github.com/rust-lang/rust-analyzer/blob/master/crates/rust-analyzer/src/line_index.rs" +/// --- +pub fn normalize(x: String) -> String { + let mut buf = x.into_bytes(); + let mut gap_len = 0; + let mut tail = buf.as_mut_slice(); + let mut crlf_seen = false; + + loop { + let idx = match FINDER.find(&tail[gap_len..]) { + None if crlf_seen => tail.len(), + // SAFETY: buf is unchanged and therefore still contains utf8 data + None => return unsafe { String::from_utf8_unchecked(buf) }, + Some(idx) => { + crlf_seen = true; + idx + gap_len + }, + }; + tail.copy_within(gap_len..idx, 0); + tail = &mut tail[idx - gap_len..]; + if tail.len() == gap_len { + break; + } + gap_len += 1; + } + + // Account for removed `\r`. + // After `set_len`, `buf` is guaranteed to contain utf-8 again. + unsafe { + let new_len = buf.len() - gap_len; + buf.set_len(new_len); + String::from_utf8_unchecked(buf) + } +} + +/// Normalize line endings, only allocating if required +/// +/// Prefer [normalize()] when you have an owned [String] as that modifies in place, +/// reusing the owned [String]'s existing allocation. +pub fn normalize_ref(x: &str) -> Cow<'_, str> { + match infer(x) { + LineEnding::Lf => Cow::Borrowed(x), + LineEnding::Crlf => Cow::Owned(normalize(x.to_string())), + } +} + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn unix() { + let src = "a\nb\nc\n\n\n\n"; + assert_eq!(infer(src), LineEnding::Lf); + assert_eq!(normalize(src.to_string()), src); + } + + #[test] + fn dos() { + let src = "\r\na\r\n\r\nb\r\nc\r\n\r\n\r\n\r\n"; + assert_eq!(infer(src), LineEnding::Crlf); + assert_eq!(normalize(src.to_string()), "\na\n\nb\nc\n\n\n\n"); + } + + #[test] + fn mixed() { + let src = "a\r\nb\r\nc\r\n\n\r\n\n"; + assert_eq!(infer(src), LineEnding::Crlf); + assert_eq!(normalize(src.to_string()), "a\nb\nc\n\n\n\n"); + } + + #[test] + fn none() { + let src = "abc"; + assert_eq!(infer(src), LineEnding::Lf); + assert_eq!(normalize(src.to_string()), src); + } +} diff --git a/crates/aether_lsp_utils/src/proto.rs b/crates/aether_lsp_utils/src/proto.rs new file mode 100644 index 0000000000..2c7ea03416 --- /dev/null +++ b/crates/aether_lsp_utils/src/proto.rs @@ -0,0 +1,10 @@ +pub mod from_proto; +pub mod to_proto; + +/// Our representation of `tower_lsp_server::ls_types::PositionEncodingKind` +/// From `biome_lsp_converters::PositionEncoding` +#[derive(Clone, Copy, Debug)] +pub enum PositionEncoding { + Utf8, + Wide(biome_line_index::WideEncoding), +} diff --git a/crates/aether_lsp_utils/src/proto/from_proto.rs b/crates/aether_lsp_utils/src/proto/from_proto.rs new file mode 100644 index 0000000000..5c922e2dcd --- /dev/null +++ b/crates/aether_lsp_utils/src/proto/from_proto.rs @@ -0,0 +1,165 @@ +use std::ops::Range; + +use anyhow::Context; +use biome_line_index::LineCol; +use biome_line_index::LineIndex; +use biome_line_index::WideLineCol; +use tower_lsp_server::ls_types as lsp_types; + +use crate::proto::PositionEncoding; + +/// The function is used to convert a LSP position to LineCol. +pub fn line_col_from_position( + position: lsp_types::Position, + line_index: &LineIndex, + position_encoding: PositionEncoding, +) -> LineCol { + match position_encoding { + PositionEncoding::Utf8 => LineCol { + line: position.line, + col: position.character, + }, + PositionEncoding::Wide(enc) => { + let line_col = WideLineCol { + line: position.line, + col: position.character, + }; + line_index.to_utf8(enc, line_col) + }, + } +} + +/// The function is used to convert a LSP position to TextSize. +/// From `biome_lsp_converters::from_proto::offset()`. +pub fn offset_from_position( + position: lsp_types::Position, + line_index: &LineIndex, + position_encoding: PositionEncoding, +) -> anyhow::Result { + let line_col = line_col_from_position(position, line_index, position_encoding); + + line_index + .offset(line_col) + .with_context(|| format!("Position {position:?} is out of range")) +} + +/// The function is used to convert a LSP range to TextRange. +/// From `biome_lsp_converters::from_proto::text_range()`. +pub fn text_range( + range: lsp_types::Range, + line_index: &LineIndex, + position_encoding: PositionEncoding, +) -> anyhow::Result { + let start = offset_from_position(range.start, line_index, position_encoding)?; + let end = offset_from_position(range.end, line_index, position_encoding)?; + Ok(biome_text_size::TextRange::new(start, end)) +} + +/// Apply text changes to document contents. +/// +/// The protocol mandates that all `TextDocumentContentChangeEvent` be applied +/// exactly in order. Each change depends on the preceding change. +/// +/// If at least one of the changes is a full document change, uses the last of them +/// as the starting point and ignores all previous changes. We then know that all +/// changes after this (if any!) are incremental changes. +/// +/// Handles all incremental changes after a full document change. We don't +/// typically get >1 incremental change as the user types, but we do get them in a +/// batch after a find-and-replace, or after a format-on-save request. +/// +/// Some editors like VS Code send the edits in reverse order (from the bottom of +/// file -> top of file). We can take advantage of this, because applying an edit +/// on, say, line 10, doesn't invalidate the `line_index` if we then need to apply +/// an additional edit on line 5. That said, we may still have edits that cross +/// lines, so rebuilding the `line_index` is not always unavoidable. +/// +/// We also normalize line endings. Changing the line length of inserted or +/// replaced text can't invalidate the text change events since the location of the +/// change itself is specified with [line, col] coordinates, separate from the +/// actual contents of the change. +pub fn apply_text_changes( + contents: &mut String, + mut changes: Vec, + line_index: &mut LineIndex, + position_encoding: PositionEncoding, +) { + // If we do have a full document change, that implies the `last_start_line` + // corresponding to that change is line 0, which will correctly force a rebuild + // of the line index before applying any incremental changes. We don't go ahead + // and rebuild the line index here, because it is guaranteed to be rebuilt for + // us on the way out. + let (changes, mut last_start_line) = + match changes.iter().rposition(|change| change.range.is_none()) { + Some(idx) => { + let incremental = changes.split_off(idx + 1); + // Unwrap: `rposition()` confirmed this index contains a full document change + let change = changes.pop().unwrap(); + *contents = crate::line_ending::normalize(change.text); + (incremental, 0) + }, + None => (changes, u32::MAX), + }; + + for change in changes { + let range = change + .range + .expect("`None` case already handled by finding the last full document change."); + + // If the end of this change is at or past the start of the last change, then + // the `line_index` needed to apply this change is now invalid, so we have to + // rebuild it. + if range.end.line >= last_start_line { + *line_index = biome_line_index::LineIndex::new(contents); + } + last_start_line = range.start.line; + + // This is a panic if we can't convert. It means we can't keep the document up + // to date and something is very wrong. + let range: Range = text_range(range, line_index, position_encoding) + .expect("Can convert `range` from `Position` to `TextRange`.") + .into(); + + contents.replace_range(range, &crate::line_ending::normalize(change.text)); + } +} + +/// Simple wrapper around `apply_text_changes()` that converts `TextEdit` to +/// `TextDocumentContentChangeEvent`. Prefer `apply_text_changes()` but be aware +/// of different sorting requirements mandated by the LSP protocol. +/// +/// The protocol does not mandate a precise order for `TextEdit[]`. The only +/// requirements are that edits must not overlap, and for multiple inserts at +/// the same position, the array order determines the insertion order. +pub fn apply_text_edits( + contents: &mut String, + mut edits: Vec, + line_index: &mut LineIndex, + position_encoding: PositionEncoding, +) { + // Sort edits in reverse order by start position to ensure that edits + // applied later in the document don’t shift the ranges of edits earlier in + // the document. This way, edits in the bottom don't invalidate the + // positions of edits at the top. Use stable sort to preserve the original + // order for edits at the same position (which defines insertion order per + // LSP spec). + edits.sort_by(|a, b| { + let a_start = a.range.start; + let b_start = b.range.start; + b_start + .line + .cmp(&a_start.line) + .then_with(|| b_start.character.cmp(&a_start.character)) + }); + + let changes: Vec = edits + .into_iter() + .map(|edit| lsp_types::TextDocumentContentChangeEvent { + range: Some(edit.range), + range_length: None, + text: edit.new_text, + }) + .collect(); + + apply_text_changes(contents, changes, line_index, position_encoding); +} diff --git a/crates/aether_lsp_utils/src/proto/to_proto.rs b/crates/aether_lsp_utils/src/proto/to_proto.rs new file mode 100644 index 0000000000..b9aa233581 --- /dev/null +++ b/crates/aether_lsp_utils/src/proto/to_proto.rs @@ -0,0 +1,122 @@ +// Utilites for converting internal types to LSP types + +use anyhow::Context; +use biome_line_index::LineCol; +use biome_line_index::LineIndex; +use biome_text_size::TextRange; +use biome_text_size::TextSize; +use tower_lsp_server::ls_types as lsp_types; + +use crate::line_ending::LineEnding; +use crate::proto::PositionEncoding; +use crate::text_edit::Indel; +use crate::text_edit::TextEdit; + +/// The function is used to convert LineCol to a LSP position. +pub fn position_from_line_col( + line_col: LineCol, + line_index: &LineIndex, + position_encoding: PositionEncoding, +) -> anyhow::Result { + match position_encoding { + PositionEncoding::Utf8 => Ok(lsp_types::Position::new(line_col.line, line_col.col)), + PositionEncoding::Wide(enc) => { + let line_col = line_index + .to_wide(enc, line_col) + .with_context(|| format!("Could not convert {line_col:?} into wide line column"))?; + Ok(lsp_types::Position::new(line_col.line, line_col.col)) + }, + } +} + +/// The function is used to convert TextSize to a LSP position. +/// From `biome_lsp_converters::to_proto::position()`. +pub fn position_from_offset( + offset: TextSize, + line_index: &LineIndex, + position_encoding: PositionEncoding, +) -> anyhow::Result { + let line_col = line_index + .line_col(offset) + .with_context(|| format!("Could not convert offset {offset:?} into a line-column index"))?; + + position_from_line_col(line_col, line_index, position_encoding) +} + +/// The function is used to convert TextRange to a LSP range. +/// From `biome_lsp_converters::to_proto::range()`. +pub fn range( + range: TextRange, + line_index: &LineIndex, + position_encoding: PositionEncoding, +) -> anyhow::Result { + let start = position_from_offset(range.start(), line_index, position_encoding)?; + let end = position_from_offset(range.end(), line_index, position_encoding)?; + Ok(lsp_types::Range::new(start, end)) +} + +pub fn text_edit( + indel: Indel, + line_index: &LineIndex, + position_encoding: PositionEncoding, + endings: LineEnding, +) -> anyhow::Result { + let range = range(indel.delete, line_index, position_encoding)?; + let new_text = match endings { + LineEnding::Lf => indel.insert, + LineEnding::Crlf => indel.insert.replace('\n', "\r\n"), + }; + Ok(lsp_types::TextEdit { range, new_text }) +} + +pub fn text_edit_vec( + text_edit: TextEdit, + line_index: &LineIndex, + position_encoding: PositionEncoding, + endings: LineEnding, +) -> anyhow::Result> { + text_edit + .into_iter() + .map(|indel| self::text_edit(indel, line_index, position_encoding, endings)) + .collect() +} + +pub fn doc_edit_vec( + text_edit: TextEdit, + line_index: &LineIndex, + position_encoding: PositionEncoding, + endings: LineEnding, +) -> anyhow::Result> { + let edits = text_edit_vec(text_edit, line_index, position_encoding, endings)?; + + Ok(edits + .into_iter() + .map(|edit| lsp_types::TextDocumentContentChangeEvent { + range: Some(edit.range), + range_length: None, + text: edit.new_text, + }) + .collect()) +} + +pub fn replace_range_edit( + range: TextRange, + replace_with: String, + line_index: &LineIndex, + position_encoding: PositionEncoding, + endings: LineEnding, +) -> anyhow::Result> { + let edit = TextEdit::replace(range, replace_with); + text_edit_vec(edit, line_index, position_encoding, endings) +} + +pub fn replace_all_edit( + text: &str, + replace_with: &str, + line_index: &LineIndex, + position_encoding: PositionEncoding, + endings: LineEnding, +) -> anyhow::Result> { + let edit = crate::diff::diff(text, replace_with); + text_edit_vec(edit, line_index, position_encoding, endings) +} diff --git a/crates/aether_lsp_utils/src/text_edit.rs b/crates/aether_lsp_utils/src/text_edit.rs new file mode 100644 index 0000000000..404161fc75 --- /dev/null +++ b/crates/aether_lsp_utils/src/text_edit.rs @@ -0,0 +1,336 @@ +// --- source +// authors = ["rust-analyzer team"] +// license = "MIT OR Apache-2.0" +// origin = "https://github.com/rust-lang/rust-analyzer/blob/master/crates/ide-db/src/text_edit.rs" +// --- + +//! Representation of a `TextEdit`. +//! +//! `rust-analyzer` never mutates text itself and only sends diffs to clients, +//! so `TextEdit` is the ultimate representation of the work done by +//! rust-analyzer. + +use std::cmp::max; + +use biome_text_size::TextRange; +use biome_text_size::TextSize; +use itertools::Itertools; + +/// `InsertDelete` -- a single "atomic" change to text +/// +/// Must not overlap with other `InDel`s +#[derive(Debug, Clone, PartialEq, Eq, Hash)] +pub struct Indel { + pub insert: String, + /// Refers to offsets in the original text + pub delete: TextRange, +} + +#[derive(Default, Debug, Clone)] +pub struct TextEdit { + /// Invariant: disjoint and sorted by `delete`. + indels: Vec, +} + +#[derive(Debug, Default, Clone)] +pub struct TextEditBuilder { + indels: Vec, +} + +impl Indel { + pub fn insert(offset: TextSize, text: String) -> Indel { + Indel::replace(TextRange::empty(offset), text) + } + pub fn delete(range: TextRange) -> Indel { + Indel::replace(range, String::new()) + } + pub fn replace(range: TextRange, replace_with: String) -> Indel { + Indel { + delete: range, + insert: replace_with, + } + } + + pub fn apply(&self, text: &mut String) { + let start: usize = self.delete.start().into(); + let end: usize = self.delete.end().into(); + text.replace_range(start..end, &self.insert); + } +} + +impl TextEdit { + pub fn builder() -> TextEditBuilder { + TextEditBuilder::default() + } + + pub fn insert(offset: TextSize, text: String) -> TextEdit { + let mut builder = TextEdit::builder(); + builder.insert(offset, text); + builder.finish() + } + + pub fn delete(range: TextRange) -> TextEdit { + let mut builder = TextEdit::builder(); + builder.delete(range); + builder.finish() + } + + pub fn replace(range: TextRange, replace_with: String) -> TextEdit { + let mut builder = TextEdit::builder(); + builder.replace(range, replace_with); + builder.finish() + } + + pub fn len(&self) -> usize { + self.indels.len() + } + + pub fn is_empty(&self) -> bool { + self.indels.is_empty() + } + + pub fn iter(&self) -> std::slice::Iter<'_, Indel> { + self.into_iter() + } + + pub fn apply(&self, text: &mut String) { + match self.len() { + 0 => return, + 1 => { + self.indels[0].apply(text); + return; + }, + _ => (), + } + + let text_size = TextSize::of(&*text); + let mut total_len = text_size; + let mut max_total_len = text_size; + for indel in &self.indels { + total_len += TextSize::of(&indel.insert); + total_len -= indel.delete.len(); + max_total_len = max(max_total_len, total_len); + } + + if let Some(additional) = max_total_len.checked_sub(text_size) { + text.reserve(additional.into()); + } + + for indel in self.indels.iter().rev() { + indel.apply(text); + } + + assert_eq!(TextSize::of(&*text), total_len); + } + + pub fn union(&mut self, other: TextEdit) -> Result<(), TextEdit> { + let iter_merge = self + .iter() + .merge_by(other.iter(), |l, r| l.delete.start() <= r.delete.start()); + if !check_disjoint(&mut iter_merge.clone()) { + return Err(other); + } + + // Only dedup deletions and replacements, keep all insertions + self.indels = iter_merge + .dedup_by(|a, b| a == b && !a.delete.is_empty()) + .cloned() + .collect(); + Ok(()) + } + + pub fn apply_to_offset(&self, offset: TextSize) -> Option { + let mut res = offset; + for indel in &self.indels { + if indel.delete.start() >= offset { + break; + } + if offset < indel.delete.end() { + return None; + } + res += TextSize::of(&indel.insert); + res -= indel.delete.len(); + } + Some(res) + } +} + +impl IntoIterator for TextEdit { + type Item = Indel; + type IntoIter = std::vec::IntoIter; + + fn into_iter(self) -> Self::IntoIter { + self.indels.into_iter() + } +} + +impl<'a> IntoIterator for &'a TextEdit { + type Item = &'a Indel; + type IntoIter = std::slice::Iter<'a, Indel>; + + fn into_iter(self) -> Self::IntoIter { + self.indels.iter() + } +} + +impl TextEditBuilder { + pub fn is_empty(&self) -> bool { + self.indels.is_empty() + } + pub fn replace(&mut self, range: TextRange, replace_with: String) { + self.indel(Indel::replace(range, replace_with)); + } + pub fn delete(&mut self, range: TextRange) { + self.indel(Indel::delete(range)); + } + pub fn insert(&mut self, offset: TextSize, text: String) { + self.indel(Indel::insert(offset, text)); + } + pub fn finish(self) -> TextEdit { + let mut indels = self.indels; + assert_disjoint_or_equal(&mut indels); + indels = coalesce_indels(indels); + TextEdit { indels } + } + pub fn invalidates_offset(&self, offset: TextSize) -> bool { + self.indels + .iter() + .any(|indel| indel.delete.contains_inclusive(offset)) + } + fn indel(&mut self, indel: Indel) { + self.indels.push(indel); + if self.indels.len() <= 16 { + assert_disjoint_or_equal(&mut self.indels); + } + } +} + +fn assert_disjoint_or_equal(indels: &mut [Indel]) { + assert!(check_disjoint_and_sort(indels)); +} + +fn check_disjoint_and_sort(indels: &mut [Indel]) -> bool { + indels.sort_by_key(|indel| (indel.delete.start(), indel.delete.end())); + check_disjoint(&mut indels.iter()) +} + +fn check_disjoint<'a, I>(indels: &mut I) -> bool +where + I: std::iter::Iterator + Clone, +{ + indels + .clone() + .zip(indels.skip(1)) + .all(|(l, r)| l.delete.end() <= r.delete.start() || l == r) +} + +fn coalesce_indels(indels: Vec) -> Vec { + indels + .into_iter() + .coalesce(|mut a, b| { + if a.delete.end() == b.delete.start() { + a.insert.push_str(&b.insert); + a.delete = TextRange::new(a.delete.start(), b.delete.end()); + Ok(a) + } else { + Err((a, b)) + } + }) + .collect_vec() +} + +#[cfg(test)] +mod tests { + use super::TextEdit; + use super::TextEditBuilder; + use super::TextRange; + + fn range(start: u32, end: u32) -> TextRange { + TextRange::new(start.into(), end.into()) + } + + #[test] + fn test_apply() { + let mut text = "_11h1_2222_xx3333_4444_6666".to_owned(); + let mut builder = TextEditBuilder::default(); + builder.replace(range(3, 4), "1".to_owned()); + builder.delete(range(11, 13)); + builder.insert(22.into(), "_5555".to_owned()); + + let text_edit = builder.finish(); + text_edit.apply(&mut text); + + assert_eq!(text, "_1111_2222_3333_4444_5555_6666") + } + + #[test] + fn test_union() { + let mut edit1 = TextEdit::delete(range(7, 11)); + let mut builder = TextEditBuilder::default(); + builder.delete(range(1, 5)); + builder.delete(range(13, 17)); + + let edit2 = builder.finish(); + assert!(edit1.union(edit2).is_ok()); + assert_eq!(edit1.indels.len(), 3); + } + + #[test] + fn test_union_with_duplicates() { + let mut builder1 = TextEditBuilder::default(); + builder1.delete(range(7, 11)); + builder1.delete(range(13, 17)); + + let mut builder2 = TextEditBuilder::default(); + builder2.delete(range(1, 5)); + builder2.delete(range(13, 17)); + + let mut edit1 = builder1.finish(); + let edit2 = builder2.finish(); + assert!(edit1.union(edit2).is_ok()); + assert_eq!(edit1.indels.len(), 3); + } + + #[test] + fn test_union_panics() { + let mut edit1 = TextEdit::delete(range(7, 11)); + let edit2 = TextEdit::delete(range(9, 13)); + assert!(edit1.union(edit2).is_err()); + } + + #[test] + fn test_coalesce_disjoint() { + let mut builder = TextEditBuilder::default(); + builder.replace(range(1, 3), "aa".into()); + builder.replace(range(5, 7), "bb".into()); + let edit = builder.finish(); + + assert_eq!(edit.indels.len(), 2); + } + + #[test] + fn test_coalesce_adjacent() { + let mut builder = TextEditBuilder::default(); + builder.replace(range(1, 3), "aa".into()); + builder.replace(range(3, 5), "bb".into()); + + let edit = builder.finish(); + assert_eq!(edit.indels.len(), 1); + assert_eq!(edit.indels[0].insert, "aabb"); + assert_eq!(edit.indels[0].delete, range(1, 5)); + } + + #[test] + fn test_coalesce_adjacent_series() { + let mut builder = TextEditBuilder::default(); + builder.replace(range(1, 3), "au".into()); + builder.replace(range(3, 5), "www".into()); + builder.replace(range(5, 8), String::new()); + builder.replace(range(8, 9), "ub".into()); + + let edit = builder.finish(); + assert_eq!(edit.indels.len(), 1); + assert_eq!(edit.indels[0].insert, "auwwwub"); + assert_eq!(edit.indels[0].delete, range(1, 9)); + } +} diff --git a/crates/ark/Cargo.toml b/crates/ark/Cargo.toml index 30cefbe0a5..9a014ec742 100644 --- a/crates/ark/Cargo.toml +++ b/crates/ark/Cargo.toml @@ -30,6 +30,7 @@ async-trait.workspace = true base64.workspace = true biome_line_index.workspace = true biome_rowan.workspace = true +biome_text_size.workspace = true blake3.workspace = true bus.workspace = true camino.workspace = true @@ -73,7 +74,7 @@ streaming-iterator.workspace = true strum.workspace = true strum_macros.workspace = true tokio.workspace = true -tower-lsp.workspace = true +tower-lsp-server.workspace = true tracing.workspace = true tracing-appender.workspace = true tracing-error.workspace = true diff --git a/crates/ark/src/lsp.rs b/crates/ark/src/lsp.rs index ef4ecfada9..1b7dcc9255 100644 --- a/crates/ark/src/lsp.rs +++ b/crates/ark/src/lsp.rs @@ -50,13 +50,13 @@ mod tests; // These send LSP messages in a non-async and non-blocking way. // The LOG level is not timestamped so we're not using it. macro_rules! log_info { - ($($arg:tt)+) => ($crate::lsp::_log!(tower_lsp::lsp_types::MessageType::INFO, $($arg)+)) + ($($arg:tt)+) => ($crate::lsp::_log!(tower_lsp_server::ls_types::MessageType::INFO, $($arg)+)) } macro_rules! log_warn { - ($($arg:tt)+) => ($crate::lsp::_log!(tower_lsp::lsp_types::MessageType::WARNING, $($arg)+)) + ($($arg:tt)+) => ($crate::lsp::_log!(tower_lsp_server::ls_types::MessageType::WARNING, $($arg)+)) } macro_rules! log_error { - ($($arg:tt)+) => ($crate::lsp::_log!(tower_lsp::lsp_types::MessageType::ERROR, $($arg)+)) + ($($arg:tt)+) => ($crate::lsp::_log!(tower_lsp_server::ls_types::MessageType::ERROR, $($arg)+)) } macro_rules! _log { ($lvl:expr, $($arg:tt)+) => ({ diff --git a/crates/ark/src/lsp/backend.rs b/crates/ark/src/lsp/backend.rs index d281f2200c..3380112b58 100644 --- a/crates/ark/src/lsp/backend.rs +++ b/crates/ark/src/lsp/backend.rs @@ -22,17 +22,17 @@ use tokio::net::TcpListener; use tokio::runtime::Runtime; use tokio::sync::mpsc::unbounded_channel as tokio_unbounded_channel; use tokio::sync::mpsc::UnboundedSender as AsyncUnboundedSender; -use tower_lsp::jsonrpc; -use tower_lsp::jsonrpc::Result; -use tower_lsp::lsp_types::request::GotoImplementationParams; -use tower_lsp::lsp_types::request::GotoImplementationResponse; -use tower_lsp::lsp_types::FoldingRange; -use tower_lsp::lsp_types::SelectionRange; -use tower_lsp::lsp_types::*; -use tower_lsp::Client; -use tower_lsp::LanguageServer; -use tower_lsp::LspService; -use tower_lsp::Server; +use tower_lsp_server::jsonrpc; +use tower_lsp_server::jsonrpc::Result; +use tower_lsp_server::ls_types::request::GotoImplementationParams; +use tower_lsp_server::ls_types::request::GotoImplementationResponse; +use tower_lsp_server::ls_types::FoldingRange; +use tower_lsp_server::ls_types::SelectionRange; +use tower_lsp_server::ls_types::*; +use tower_lsp_server::Client; +use tower_lsp_server::LanguageServer; +use tower_lsp_server::LspService; +use tower_lsp_server::Server; use super::main_loop::LSP_HAS_CRASHED; use crate::console::Console; @@ -284,7 +284,6 @@ impl Backend { } } -#[tower_lsp::async_trait] impl LanguageServer for Backend { async fn initialize(&self, params: InitializeParams) -> Result { cast_response!( @@ -319,12 +318,13 @@ impl LanguageServer for Backend { async fn symbol( &self, params: WorkspaceSymbolParams, - ) -> Result>> { - cast_response!( + ) -> Result> { + let info: Option> = cast_response!( self, self.request(LspRequest::WorkspaceSymbol(params)).await, LspResponse::WorkspaceSymbol - ) + )?; + Ok(info.map(WorkspaceSymbolResponse::Flat)) } async fn document_symbol( @@ -526,7 +526,7 @@ impl Backend { async fn virtual_document( &self, params: VirtualDocumentParams, - ) -> tower_lsp::jsonrpc::Result { + ) -> tower_lsp_server::jsonrpc::Result { cast_response!( self, self.request(LspRequest::VirtualDocument(params)).await, @@ -537,7 +537,7 @@ impl Backend { async fn input_boundaries( &self, params: InputBoundariesParams, - ) -> tower_lsp::jsonrpc::Result { + ) -> tower_lsp_server::jsonrpc::Result { cast_response!( self, self.request(LspRequest::InputBoundaries(params)).await, diff --git a/crates/ark/src/lsp/capabilities.rs b/crates/ark/src/lsp/capabilities.rs index e501db14ec..fe7556f28e 100644 --- a/crates/ark/src/lsp/capabilities.rs +++ b/crates/ark/src/lsp/capabilities.rs @@ -5,11 +5,11 @@ // // -use tower_lsp::lsp_types; -use tower_lsp::lsp_types::CodeActionKind; -use tower_lsp::lsp_types::CodeActionOptions; -use tower_lsp::lsp_types::CodeActionProviderCapability; -use tower_lsp::lsp_types::WorkDoneProgressOptions; +use tower_lsp_server::ls_types as lsp_types; +use tower_lsp_server::ls_types::CodeActionKind; +use tower_lsp_server::ls_types::CodeActionOptions; +use tower_lsp_server::ls_types::CodeActionProviderCapability; +use tower_lsp_server::ls_types::WorkDoneProgressOptions; /// Capabilities negotiated with [lsp_types::ClientCapabilities] #[derive(Debug, Default)] diff --git a/crates/ark/src/lsp/code_action.rs b/crates/ark/src/lsp/code_action.rs index ec700d10c9..32e9d2ec88 100644 --- a/crates/ark/src/lsp/code_action.rs +++ b/crates/ark/src/lsp/code_action.rs @@ -12,10 +12,9 @@ use std::collections::HashMap; use aether_lsp_utils::proto::PositionEncoding; use oak_db::Db; use oak_db::File; -use tower_lsp::lsp_types; +use tower_lsp_server::ls_types as lsp_types; use tree_sitter::Point; use tree_sitter::Range; -use url::Url; use crate::lsp::capabilities::Capabilities; use crate::lsp::db::ArkDb; @@ -97,7 +96,7 @@ impl CodeActions { /// Assemble into the LSP response. This is the boundary where the `OpenFile` /// and the position encoding enter. It converts the tree-sitter coordinates - /// into LSP `TextEdit`s and stamps each edit with the document's wire URL + /// into LSP `TextEdit`s and stamps each edit with the document's wire `Uri` /// and version. pub(crate) fn into_response( self, @@ -107,6 +106,7 @@ impl CodeActions { capabilities: &Capabilities, ) -> lsp_types::CodeActionResponse { let line_index = file.line_index(db); + let uri = file.wire_uri().clone(); self.actions .into_iter() @@ -128,7 +128,7 @@ impl CodeActions { .ok()?; let workspace_edit = code_action_workspace_text_edit( - file.wire_url().clone(), + uri.clone(), file.version(), edits, capabilities, @@ -164,7 +164,7 @@ pub(crate) fn code_action( /// Creates a common kind of `WorkspaceEdit` composed of one or more `TextEdit`s to /// apply to a single document pub(crate) fn code_action_workspace_text_edit( - uri: Url, + uri: lsp_types::Uri, version: Option, edits: Vec, capabilities: &Capabilities, diff --git a/crates/ark/src/lsp/code_action/roxygen.rs b/crates/ark/src/lsp/code_action/roxygen.rs index 54b8ebc9aa..80b73f3428 100644 --- a/crates/ark/src/lsp/code_action/roxygen.rs +++ b/crates/ark/src/lsp/code_action/roxygen.rs @@ -1,5 +1,5 @@ use oak_db::File; -use tower_lsp::lsp_types; +use tower_lsp_server::ls_types as lsp_types; use crate::lsp::capabilities::Capabilities; use crate::lsp::code_action::CodeActionEdit; @@ -182,11 +182,11 @@ fn documentation_from_lines(lines: Vec, indent_size: usize) -> String { #[cfg(test)] mod tests { use aether_lsp_utils::proto::PositionEncoding; - use tower_lsp::lsp_types::CodeActionOrCommand; - use tower_lsp::lsp_types::Position; + use tower_lsp_server::ls_types::CodeActionOrCommand; + use tower_lsp_server::ls_types::Position; + use tower_lsp_server::ls_types::Uri; use tree_sitter::Point; use tree_sitter::Range; - use url::Url; use super::roxygen_documentation; use crate::fixtures::point_and_offset_from_cursor; @@ -344,7 +344,7 @@ f@n <- function(a, b) {} #[test] fn test_uses_hash_map_of_text_edits_without_document_changes_support() { - let uri = Url::parse("file:///test.R").unwrap(); + let uri: Uri = "file:///test.R".parse().unwrap(); // NOTE: `with_workspace_edit_document_changes(false)` let capabilities = Capabilities::default() diff --git a/crates/ark/src/lsp/completions/completion_item.rs b/crates/ark/src/lsp/completions/completion_item.rs index 311f30c1d1..8886ef6c8b 100644 --- a/crates/ark/src/lsp/completions/completion_item.rs +++ b/crates/ark/src/lsp/completions/completion_item.rs @@ -23,17 +23,17 @@ use libr::PROMSXP; use libr::PRVALUE; use libr::SEXP; use stdext::*; -use tower_lsp::lsp_types::Command; -use tower_lsp::lsp_types::CompletionItem; -use tower_lsp::lsp_types::CompletionItemKind; -use tower_lsp::lsp_types::CompletionItemLabelDetails; -use tower_lsp::lsp_types::CompletionTextEdit; -use tower_lsp::lsp_types::Documentation; -use tower_lsp::lsp_types::InsertTextFormat; -use tower_lsp::lsp_types::MarkupContent; -use tower_lsp::lsp_types::MarkupKind; -use tower_lsp::lsp_types::Range; -use tower_lsp::lsp_types::TextEdit; +use tower_lsp_server::ls_types::Command; +use tower_lsp_server::ls_types::CompletionItem; +use tower_lsp_server::ls_types::CompletionItemKind; +use tower_lsp_server::ls_types::CompletionItemLabelDetails; +use tower_lsp_server::ls_types::CompletionTextEdit; +use tower_lsp_server::ls_types::Documentation; +use tower_lsp_server::ls_types::InsertTextFormat; +use tower_lsp_server::ls_types::MarkupContent; +use tower_lsp_server::ls_types::MarkupKind; +use tower_lsp_server::ls_types::Range; +use tower_lsp_server::ls_types::TextEdit; use tree_sitter::Node; use crate::lsp::completions::function_context::ArgumentsStatus; diff --git a/crates/ark/src/lsp/completions/function_context.rs b/crates/ark/src/lsp/completions/function_context.rs index d4647a5e46..40e5ae0e72 100644 --- a/crates/ark/src/lsp/completions/function_context.rs +++ b/crates/ark/src/lsp/completions/function_context.rs @@ -6,8 +6,8 @@ // use stdext::result::ResultExt; -use tower_lsp::lsp_types; -use tower_lsp::lsp_types::Range; +use tower_lsp_server::ls_types as lsp_types; +use tower_lsp_server::ls_types::Range; use tree_sitter::Node; use crate::lsp::document_context::DocumentContext; diff --git a/crates/ark/src/lsp/completions/provide.rs b/crates/ark/src/lsp/completions/provide.rs index 9036430efb..430dd0028b 100644 --- a/crates/ark/src/lsp/completions/provide.rs +++ b/crates/ark/src/lsp/completions/provide.rs @@ -5,7 +5,7 @@ // // -use tower_lsp::lsp_types::CompletionItem; +use tower_lsp_server::ls_types::CompletionItem; use crate::lsp::completions::completion_context::CompletionContext; use crate::lsp::completions::sources::composite; diff --git a/crates/ark/src/lsp/completions/resolve.rs b/crates/ark/src/lsp/completions/resolve.rs index f2fc4dd6e3..171511da68 100644 --- a/crates/ark/src/lsp/completions/resolve.rs +++ b/crates/ark/src/lsp/completions/resolve.rs @@ -7,10 +7,10 @@ use anyhow::bail; use stdext::*; -use tower_lsp::lsp_types::CompletionItem; -use tower_lsp::lsp_types::Documentation; -use tower_lsp::lsp_types::MarkupContent; -use tower_lsp::lsp_types::MarkupKind; +use tower_lsp_server::ls_types::CompletionItem; +use tower_lsp_server::ls_types::Documentation; +use tower_lsp_server::ls_types::MarkupContent; +use tower_lsp_server::ls_types::MarkupKind; use crate::lsp::completions::types::CompletionData; use crate::lsp::help::RHtmlHelp; diff --git a/crates/ark/src/lsp/completions/sources.rs b/crates/ark/src/lsp/completions/sources.rs index d16132db8a..5ab1447f07 100644 --- a/crates/ark/src/lsp/completions/sources.rs +++ b/crates/ark/src/lsp/completions/sources.rs @@ -10,7 +10,7 @@ pub(crate) mod composite; pub(crate) mod unique; pub(crate) mod utils; -use tower_lsp::lsp_types::CompletionItem; +use tower_lsp_server::ls_types::CompletionItem; use crate::lsp::completions::completion_context::CompletionContext; diff --git a/crates/ark/src/lsp/completions/sources/composite.rs b/crates/ark/src/lsp/completions/sources/composite.rs index 319e0ba2ac..3fbd1a1a58 100644 --- a/crates/ark/src/lsp/completions/sources/composite.rs +++ b/crates/ark/src/lsp/completions/sources/composite.rs @@ -16,8 +16,8 @@ mod workspace; use std::collections::HashMap; use stdext::*; -use tower_lsp::lsp_types::CompletionItem; -use tower_lsp::lsp_types::CompletionItemKind; +use tower_lsp_server::ls_types::CompletionItem; +use tower_lsp_server::ls_types::CompletionItemKind; use tree_sitter::Node; use crate::lsp::completions::completion_context::CompletionContext; diff --git a/crates/ark/src/lsp/completions/sources/composite/call.rs b/crates/ark/src/lsp/completions/sources/composite/call.rs index 7fa8b4de27..ad9f9ac6b7 100644 --- a/crates/ark/src/lsp/completions/sources/composite/call.rs +++ b/crates/ark/src/lsp/completions/sources/composite/call.rs @@ -11,7 +11,7 @@ use harp::exec::RFunction; use harp::exec::RFunctionExt; use harp::object::RObject; use harp::utils::r_is_function; -use tower_lsp::lsp_types::CompletionItem; +use tower_lsp_server::ls_types::CompletionItem; use tree_sitter::Node; use crate::console; diff --git a/crates/ark/src/lsp/completions/sources/composite/document.rs b/crates/ark/src/lsp/completions/sources/composite/document.rs index a101af48ca..81e3395d74 100644 --- a/crates/ark/src/lsp/completions/sources/composite/document.rs +++ b/crates/ark/src/lsp/completions/sources/composite/document.rs @@ -6,7 +6,7 @@ // use stdext::*; -use tower_lsp::lsp_types::CompletionItem; +use tower_lsp_server::ls_types::CompletionItem; use tree_sitter::Node; use crate::lsp::completions::completion_context::CompletionContext; diff --git a/crates/ark/src/lsp/completions/sources/composite/keyword.rs b/crates/ark/src/lsp/completions/sources/composite/keyword.rs index 5e02c2e25b..0d6ddb6695 100644 --- a/crates/ark/src/lsp/completions/sources/composite/keyword.rs +++ b/crates/ark/src/lsp/completions/sources/composite/keyword.rs @@ -6,13 +6,13 @@ // use stdext::unwrap; -use tower_lsp::lsp_types::CompletionItem; -use tower_lsp::lsp_types::CompletionItemKind; -use tower_lsp::lsp_types::CompletionItemLabelDetails; -use tower_lsp::lsp_types::Documentation; -use tower_lsp::lsp_types::InsertTextFormat; -use tower_lsp::lsp_types::MarkupContent; -use tower_lsp::lsp_types::MarkupKind; +use tower_lsp_server::ls_types::CompletionItem; +use tower_lsp_server::ls_types::CompletionItemKind; +use tower_lsp_server::ls_types::CompletionItemLabelDetails; +use tower_lsp_server::ls_types::Documentation; +use tower_lsp_server::ls_types::InsertTextFormat; +use tower_lsp_server::ls_types::MarkupContent; +use tower_lsp_server::ls_types::MarkupKind; use crate::lsp::completions::completion_context::CompletionContext; use crate::lsp::completions::completion_item::completion_item; @@ -175,8 +175,8 @@ fn add_keyword_snippets(completions: &mut Vec) { #[cfg(test)] mod tests { - use tower_lsp::lsp_types::CompletionItemLabelDetails; - use tower_lsp::lsp_types::{self}; + use tower_lsp_server::ls_types as lsp_types; + use tower_lsp_server::ls_types::CompletionItemLabelDetails; #[test] fn test_presence_bare_keywords() { diff --git a/crates/ark/src/lsp/completions/sources/composite/pipe.rs b/crates/ark/src/lsp/completions/sources/composite/pipe.rs index 15b0d1b0d2..2be04809f0 100644 --- a/crates/ark/src/lsp/completions/sources/composite/pipe.rs +++ b/crates/ark/src/lsp/completions/sources/composite/pipe.rs @@ -8,7 +8,7 @@ use harp::error::Error; use harp::eval::RParseEvalOptions; use harp::object::RObject; -use tower_lsp::lsp_types::CompletionItem; +use tower_lsp_server::ls_types::CompletionItem; use tree_sitter::Node; use crate::console; diff --git a/crates/ark/src/lsp/completions/sources/composite/search_path.rs b/crates/ark/src/lsp/completions/sources/composite/search_path.rs index a387567f40..be00855444 100644 --- a/crates/ark/src/lsp/completions/sources/composite/search_path.rs +++ b/crates/ark/src/lsp/completions/sources/composite/search_path.rs @@ -15,7 +15,7 @@ use harp::vector::Vector; use harp::RObject; use libr::R_EmptyEnv; use libr::R_lsInternal; -use tower_lsp::lsp_types::CompletionItem; +use tower_lsp_server::ls_types::CompletionItem; use crate::console; use crate::lsp::completions::completion_context::CompletionContext; diff --git a/crates/ark/src/lsp/completions/sources/composite/subset.rs b/crates/ark/src/lsp/completions/sources/composite/subset.rs index bfe9ad5939..c1c352131c 100644 --- a/crates/ark/src/lsp/completions/sources/composite/subset.rs +++ b/crates/ark/src/lsp/completions/sources/composite/subset.rs @@ -5,7 +5,7 @@ // // -use tower_lsp::lsp_types::CompletionItem; +use tower_lsp_server::ls_types::CompletionItem; use crate::lsp::completions::completion_context::CompletionContext; use crate::lsp::completions::sources::common::subset::is_within_subset_delimiters; diff --git a/crates/ark/src/lsp/completions/sources/composite/workspace.rs b/crates/ark/src/lsp/completions/sources/composite/workspace.rs index 21706c72d9..58240b743a 100644 --- a/crates/ark/src/lsp/completions/sources/composite/workspace.rs +++ b/crates/ark/src/lsp/completions/sources/composite/workspace.rs @@ -5,10 +5,10 @@ // // -use tower_lsp::lsp_types::CompletionItem; -use tower_lsp::lsp_types::Documentation; -use tower_lsp::lsp_types::MarkupContent; -use tower_lsp::lsp_types::MarkupKind; +use tower_lsp_server::ls_types::CompletionItem; +use tower_lsp_server::ls_types::Documentation; +use tower_lsp_server::ls_types::MarkupContent; +use tower_lsp_server::ls_types::MarkupKind; use crate::lsp::completions::completion_context::CompletionContext; use crate::lsp::completions::completion_item::completion_item_from_function; @@ -76,7 +76,7 @@ fn completions_from_workspace( match &entry.data { indexer::IndexEntryData::Function { name, .. } => { - let uri = state.wire_url(file); + let path = file.path(state.db()); let fun_context = match completion_context.function_context() { Ok(fun_context) => fun_context, @@ -95,25 +95,21 @@ fn completions_from_workspace( }; // Add some metadata about where the completion was found - let mut path = uri.as_str().to_owned(); - - if uri.scheme() == "file" { - if let Ok(file_path) = uri.to_file_path() { - for folder in &state.workspace.folders { - let Ok(folder_path) = folder.to_file_path() else { - continue; - }; - if let Ok(relative_path) = file_path.strip_prefix(&folder_path) { - path = relative_path.to_string_lossy().to_string(); - break; - } + let mut display_path = path.to_string(); + + if let Some(abs_path) = path.as_file() { + for folder in &state.workspace.folders { + if let Ok(relative_path) = abs_path.as_path().strip_prefix(folder.as_path()) + { + display_path = relative_path.to_string(); + break; } - }; + } } let value = format!( "Defined in `{}` on line {}.", - path, + display_path, entry.range.start.row + 1 ); let markup = MarkupContent { diff --git a/crates/ark/src/lsp/completions/sources/unique.rs b/crates/ark/src/lsp/completions/sources/unique.rs index 69ef532dd9..12a95d768e 100644 --- a/crates/ark/src/lsp/completions/sources/unique.rs +++ b/crates/ark/src/lsp/completions/sources/unique.rs @@ -14,7 +14,7 @@ mod namespace; mod string; mod subset; -use tower_lsp::lsp_types::CompletionItem; +use tower_lsp_server::ls_types::CompletionItem; use crate::lsp::completions::completion_context::CompletionContext; use crate::lsp::completions::sources::collect_completions; diff --git a/crates/ark/src/lsp/completions/sources/unique/colon.rs b/crates/ark/src/lsp/completions/sources/unique/colon.rs index 8db3316a99..ffb6cc44ca 100644 --- a/crates/ark/src/lsp/completions/sources/unique/colon.rs +++ b/crates/ark/src/lsp/completions/sources/unique/colon.rs @@ -5,7 +5,7 @@ // // -use tower_lsp::lsp_types::CompletionItem; +use tower_lsp_server::ls_types::CompletionItem; use crate::lsp::completions::completion_context::CompletionContext; use crate::lsp::completions::sources::CompletionSource; diff --git a/crates/ark/src/lsp/completions/sources/unique/comment.rs b/crates/ark/src/lsp/completions/sources/unique/comment.rs index 23aa727a10..e6b5c9aa04 100644 --- a/crates/ark/src/lsp/completions/sources/unique/comment.rs +++ b/crates/ark/src/lsp/completions/sources/unique/comment.rs @@ -11,11 +11,11 @@ use std::sync::LazyLock; use oak_db::OakDatabase; use oak_db::RootKind; use regex::Regex; -use tower_lsp::lsp_types::CompletionItem; -use tower_lsp::lsp_types::Documentation; -use tower_lsp::lsp_types::InsertTextFormat; -use tower_lsp::lsp_types::MarkupContent; -use tower_lsp::lsp_types::MarkupKind; +use tower_lsp_server::ls_types::CompletionItem; +use tower_lsp_server::ls_types::Documentation; +use tower_lsp_server::ls_types::InsertTextFormat; +use tower_lsp_server::ls_types::MarkupContent; +use tower_lsp_server::ls_types::MarkupKind; use yaml_rust2::YamlLoader; use crate::lsp::completions::completion_context::CompletionContext; diff --git a/crates/ark/src/lsp/completions/sources/unique/custom.rs b/crates/ark/src/lsp/completions/sources/unique/custom.rs index bf5ecaabf9..6fc3210aee 100644 --- a/crates/ark/src/lsp/completions/sources/unique/custom.rs +++ b/crates/ark/src/lsp/completions/sources/unique/custom.rs @@ -15,8 +15,8 @@ use libr::VECSXP; use libr::VECTOR_ELT; use stdext::unwrap; use stdext::IntoResult; -use tower_lsp::lsp_types; -use tower_lsp::lsp_types::CompletionItem; +use tower_lsp_server::ls_types as lsp_types; +use tower_lsp_server::ls_types::CompletionItem; use crate::lsp; use crate::lsp::completions::completion_context::CompletionContext; diff --git a/crates/ark/src/lsp/completions/sources/unique/extractor.rs b/crates/ark/src/lsp/completions/sources/unique/extractor.rs index c6cfa74ff0..a64b15ff91 100644 --- a/crates/ark/src/lsp/completions/sources/unique/extractor.rs +++ b/crates/ark/src/lsp/completions/sources/unique/extractor.rs @@ -13,7 +13,7 @@ use harp::utils::r_env_has; use harp::utils::r_typeof; use harp::Error; use libr::STRSXP; -use tower_lsp::lsp_types::CompletionItem; +use tower_lsp_server::ls_types::CompletionItem; use tree_sitter::Node; use crate::console; diff --git a/crates/ark/src/lsp/completions/sources/unique/file_path.rs b/crates/ark/src/lsp/completions/sources/unique/file_path.rs index fc6f250c74..bd1767e9e9 100644 --- a/crates/ark/src/lsp/completions/sources/unique/file_path.rs +++ b/crates/ark/src/lsp/completions/sources/unique/file_path.rs @@ -11,7 +11,7 @@ use std::path::PathBuf; use harp::utils::r_is_string; use harp::utils::r_normalize_path; use stdext::unwrap; -use tower_lsp::lsp_types::CompletionItem; +use tower_lsp_server::ls_types::CompletionItem; use tree_sitter::Node; use crate::lsp::completions::completion_item::completion_item_from_direntry; diff --git a/crates/ark/src/lsp/completions/sources/unique/namespace.rs b/crates/ark/src/lsp/completions/sources/unique/namespace.rs index dea4106da3..75ed89ca9c 100644 --- a/crates/ark/src/lsp/completions/sources/unique/namespace.rs +++ b/crates/ark/src/lsp/completions/sources/unique/namespace.rs @@ -14,7 +14,7 @@ use libr::R_lsInternal; use libr::Rboolean_TRUE; use libr::Rf_findVarInFrame; use libr::SEXP; -use tower_lsp::lsp_types::CompletionItem; +use tower_lsp_server::ls_types::CompletionItem; use tree_sitter::Node; use tree_sitter::Point; @@ -240,7 +240,7 @@ fn list_namespace_exports(namespace: SEXP) -> RObject { #[cfg(test)] mod tests { - use tower_lsp::lsp_types::CompletionItem; + use tower_lsp_server::ls_types::CompletionItem; use crate::fixtures::point_from_cursor; use crate::lsp::completions::completion_context::CompletionContext; diff --git a/crates/ark/src/lsp/completions/sources/unique/string.rs b/crates/ark/src/lsp/completions/sources/unique/string.rs index 478e2d5dcd..727879b741 100644 --- a/crates/ark/src/lsp/completions/sources/unique/string.rs +++ b/crates/ark/src/lsp/completions/sources/unique/string.rs @@ -5,7 +5,7 @@ // // -use tower_lsp::lsp_types::CompletionItem; +use tower_lsp_server::ls_types::CompletionItem; use super::file_path::completions_from_string_file_path; use crate::lsp::completions::completion_context::CompletionContext; diff --git a/crates/ark/src/lsp/completions/sources/unique/subset.rs b/crates/ark/src/lsp/completions/sources/unique/subset.rs index 921749bc33..cffe0f416e 100644 --- a/crates/ark/src/lsp/completions/sources/unique/subset.rs +++ b/crates/ark/src/lsp/completions/sources/unique/subset.rs @@ -5,7 +5,7 @@ // // -use tower_lsp::lsp_types::CompletionItem; +use tower_lsp_server::ls_types::CompletionItem; use tree_sitter::Node; use crate::lsp::completions::sources::common::subset::is_within_subset_delimiters; diff --git a/crates/ark/src/lsp/completions/sources/utils.rs b/crates/ark/src/lsp/completions/sources/utils.rs index 2be22ed892..bb55545068 100644 --- a/crates/ark/src/lsp/completions/sources/utils.rs +++ b/crates/ark/src/lsp/completions/sources/utils.rs @@ -12,7 +12,7 @@ use harp::exec::RFunctionExt; use harp::object::RObject; use harp::utils::r_inherits; use regex::Regex; -use tower_lsp::lsp_types::CompletionItem; +use tower_lsp_server::ls_types::CompletionItem; use tree_sitter::Node; use tree_sitter::Point; diff --git a/crates/ark/src/lsp/completions/tests/function_completions.rs b/crates/ark/src/lsp/completions/tests/function_completions.rs index 4351baf987..176f3ded2d 100644 --- a/crates/ark/src/lsp/completions/tests/function_completions.rs +++ b/crates/ark/src/lsp/completions/tests/function_completions.rs @@ -6,8 +6,8 @@ #[cfg(test)] mod function_call_tests { - use tower_lsp::lsp_types::CompletionItemKind; - use tower_lsp::lsp_types::InsertTextFormat; + use tower_lsp_server::ls_types::CompletionItemKind; + use tower_lsp_server::ls_types::InsertTextFormat; use crate::lsp::completions::tests::utils::assert_has_parameter_hints; use crate::lsp::completions::tests::utils::find_completion_by_label; @@ -81,8 +81,8 @@ mod function_call_tests { #[cfg(test)] mod function_reference_tests { - use tower_lsp::lsp_types::CompletionItemKind; - use tower_lsp::lsp_types::InsertTextFormat; + use tower_lsp_server::ls_types::CompletionItemKind; + use tower_lsp_server::ls_types::InsertTextFormat; use crate::lsp::completions::tests::utils::assert_no_command; use crate::lsp::completions::tests::utils::find_completion_by_label; @@ -157,8 +157,8 @@ mod function_reference_tests { /// Tests where namespace is added retroactively #[cfg(test)] mod namespace_post_hoc_tests { - use tower_lsp::lsp_types::CompletionItemKind; - use tower_lsp::lsp_types::InsertTextFormat; + use tower_lsp_server::ls_types::CompletionItemKind; + use tower_lsp_server::ls_types::InsertTextFormat; use crate::lsp::completions::tests::utils::assert_has_parameter_hints; use crate::lsp::completions::tests::utils::assert_sort_text_has_priority_prefix; @@ -244,7 +244,7 @@ mod namespace_post_hoc_tests { #[cfg(test)] mod empty_args_tests { - use tower_lsp::lsp_types::CompletionItemKind; + use tower_lsp_server::ls_types::CompletionItemKind; use crate::lsp::completions::tests::utils::assert_has_parameter_hints; use crate::lsp::completions::tests::utils::assert_text_edit; @@ -292,7 +292,7 @@ mod empty_args_tests { #[cfg(test)] mod nonempty_args_tests { - use tower_lsp::lsp_types::CompletionItemKind; + use tower_lsp_server::ls_types::CompletionItemKind; use crate::lsp::completions::tests::utils::assert_no_command; use crate::lsp::completions::tests::utils::assert_text_edit; diff --git a/crates/ark/src/lsp/completions/tests/utils.rs b/crates/ark/src/lsp/completions/tests/utils.rs index 36953c0765..833fa3630e 100644 --- a/crates/ark/src/lsp/completions/tests/utils.rs +++ b/crates/ark/src/lsp/completions/tests/utils.rs @@ -4,9 +4,9 @@ // Copyright (C) 2025 Posit Software, PBC. All rights reserved. // -use tower_lsp::lsp_types; -use tower_lsp::lsp_types::CompletionItem; -use tower_lsp::lsp_types::CompletionTextEdit; +use tower_lsp_server::ls_types as lsp_types; +use tower_lsp_server::ls_types::CompletionItem; +use tower_lsp_server::ls_types::CompletionTextEdit; use crate::fixtures::point_from_cursor; use crate::lsp::completions::provide_completions; diff --git a/crates/ark/src/lsp/content_changes.rs b/crates/ark/src/lsp/content_changes.rs index 47bcb5bda8..366f956707 100644 --- a/crates/ark/src/lsp/content_changes.rs +++ b/crates/ark/src/lsp/content_changes.rs @@ -1,6 +1,6 @@ use aether_lsp_utils::proto::from_proto; use aether_lsp_utils::proto::PositionEncoding; -use tower_lsp::lsp_types; +use tower_lsp_server::ls_types as lsp_types; // --- source // authors = ["rust-analyzer team"] diff --git a/crates/ark/src/lsp/diagnostics.rs b/crates/ark/src/lsp/diagnostics.rs index e3bdb8c471..c090fe108f 100644 --- a/crates/ark/src/lsp/diagnostics.rs +++ b/crates/ark/src/lsp/diagnostics.rs @@ -16,8 +16,8 @@ use harp::syntax::is_valid_symbol; use harp::syntax::sym_quote_invalid; use oak_db::File; use stdext::*; -use tower_lsp::lsp_types::Diagnostic; -use tower_lsp::lsp_types::DiagnosticSeverity; +use tower_lsp_server::ls_types::Diagnostic; +use tower_lsp_server::ls_types::DiagnosticSeverity; use tree_sitter::Node; use tree_sitter::Point; use tree_sitter::Range; @@ -1175,8 +1175,8 @@ mod tests { use oak_db::OakDatabase; use oak_scan::DbScan; use tempfile::TempDir; - use tower_lsp::lsp_types; - use tower_lsp::lsp_types::Position; + use tower_lsp_server::ls_types as lsp_types; + use tower_lsp_server::ls_types::Position; use crate::console::console_inputs; use crate::lsp::state::WorldState; diff --git a/crates/ark/src/lsp/diagnostics_syntax.rs b/crates/ark/src/lsp/diagnostics_syntax.rs index eec663b0cb..2754367d87 100644 --- a/crates/ark/src/lsp/diagnostics_syntax.rs +++ b/crates/ark/src/lsp/diagnostics_syntax.rs @@ -5,7 +5,7 @@ // // -use tower_lsp::lsp_types::Diagnostic; +use tower_lsp_server::ls_types::Diagnostic; use tree_sitter::Node; use tree_sitter::Range; @@ -433,8 +433,8 @@ fn new_syntax_diagnostic( #[cfg(test)] mod tests { use aether_lsp_utils::proto::PositionEncoding; - use tower_lsp::lsp_types::Diagnostic; - use tower_lsp::lsp_types::Position; + use tower_lsp_server::ls_types::Diagnostic; + use tower_lsp_server::ls_types::Position; use crate::lsp::diagnostics::DiagnosticContext; use crate::lsp::diagnostics_syntax::syntax_diagnostics; diff --git a/crates/ark/src/lsp/find_references.rs b/crates/ark/src/lsp/find_references.rs index 28c57b368a..f3b89e6c9c 100644 --- a/crates/ark/src/lsp/find_references.rs +++ b/crates/ark/src/lsp/find_references.rs @@ -1,36 +1,44 @@ use aether_lsp_utils::proto::from_proto; use aether_lsp_utils::proto::to_proto; -use aether_path::FilePath; use oak_db::Db; use stdext::result::ResultExt; -use tower_lsp::lsp_types::Location; -use tower_lsp::lsp_types::ReferenceParams; +use tower_lsp_server::ls_types::Location; +use tower_lsp_server::ls_types::ReferenceParams; use crate::lsp::state::WorldState; +use crate::lsp::traits::url::UriExt; pub(crate) fn find_references( params: ReferenceParams, state: &WorldState, ) -> anyhow::Result> { - let uri = params.text_document_position.text_document.uri; + let path = params + .text_document_position + .text_document + .uri + .to_document_path()?; let position = params.text_document_position.position; let include_declaration = params.context.include_declaration; let db = &state.db; let encoding = state.config.position_encoding; - let Some(file) = db.file_by_path(&FilePath::from_url(&uri)) else { + let Some(file) = db.file_by_path(&path) else { return Ok(Vec::new()); }; let offset = from_proto::offset_from_position(position, file.line_index(db), encoding)?; let file_ranges = oak_ide::find_references(db, file, offset, include_declaration); + // A reference we can't convert is dropped rather than failing the whole + // request, so the user still sees the ones we could resolve. let locations = file_ranges .iter() - .filter_map(|fr| { - let range = to_proto::range(fr.range, fr.file.line_index(db), encoding).log_err()?; - Some(Location::new(state.wire_url(fr.file), range)) + .filter_map(|file_range| { + let line_index = file_range.file.line_index(db); + let range = to_proto::range(file_range.range, line_index, encoding).log_err()?; + let uri = state.wire_uri(file_range.file).log_err()?; + Some(Location::new(uri, range)) }) .collect(); diff --git a/crates/ark/src/lsp/folding_range.rs b/crates/ark/src/lsp/folding_range.rs index 46363f8d4a..124500c6a8 100644 --- a/crates/ark/src/lsp/folding_range.rs +++ b/crates/ark/src/lsp/folding_range.rs @@ -10,8 +10,8 @@ use std::sync::LazyLock; use oak_db::File; use regex::Regex; -use tower_lsp::lsp_types::FoldingRange; -use tower_lsp::lsp_types::FoldingRangeKind; +use tower_lsp_server::ls_types::FoldingRange; +use tower_lsp_server::ls_types::FoldingRangeKind; use super::symbols::parse_comment_as_section; use crate::lsp; diff --git a/crates/ark/src/lsp/goto_definition.rs b/crates/ark/src/lsp/goto_definition.rs index c5a2cdd83c..7677e2e7ac 100644 --- a/crates/ark/src/lsp/goto_definition.rs +++ b/crates/ark/src/lsp/goto_definition.rs @@ -1,26 +1,31 @@ use aether_lsp_utils::proto::from_proto; use aether_lsp_utils::proto::to_proto; use aether_lsp_utils::proto::PositionEncoding; -use aether_path::FilePath; use oak_db::Db; use oak_ide::NavigationTarget; -use tower_lsp::lsp_types::GotoDefinitionParams; -use tower_lsp::lsp_types::GotoDefinitionResponse; -use tower_lsp::lsp_types::LocationLink; +use stdext::result::ResultExt; +use tower_lsp_server::ls_types::GotoDefinitionParams; +use tower_lsp_server::ls_types::GotoDefinitionResponse; +use tower_lsp_server::ls_types::LocationLink; use crate::lsp::state::WorldState; +use crate::lsp::traits::url::UriExt; pub(crate) fn goto_definition( params: GotoDefinitionParams, state: &WorldState, ) -> anyhow::Result> { - let uri = ¶ms.text_document_position_params.text_document.uri; + let path = params + .text_document_position_params + .text_document + .uri + .to_document_path()?; let position = params.text_document_position_params.position; let db = &state.db; let encoding = state.config.position_encoding; - let Some(file) = db.file_by_path(&FilePath::from_url(uri)) else { + let Some(file) = db.file_by_path(&path) else { return Ok(None); }; @@ -32,11 +37,17 @@ pub(crate) fn goto_definition( } // An ambiguous name (e.g. defined on both arms of an `if`/`else`) resolves - // to several bindings; the client offers all of them. - let links = targets + // to several bindings. A target we can't convert is dropped rather than + // failing the whole request, so the user can see the definitions we did + // resolve. + let links: Vec = targets .iter() - .map(|target| nav_target_to_link(state, encoding, target)) - .collect::>>()?; + .filter_map(|target| nav_target_to_link(state, encoding, target).log_err()) + .collect(); + + if links.is_empty() { + return Ok(None); + } Ok(Some(GotoDefinitionResponse::Link(links))) } @@ -56,7 +67,7 @@ fn nav_target_to_link( Ok(LocationLink { origin_selection_range: None, - target_uri: state.wire_url(target.file), + target_uri: state.wire_uri(target.file)?, target_range, target_selection_range, }) diff --git a/crates/ark/src/lsp/handlers.rs b/crates/ark/src/lsp/handlers.rs index 9c537298ac..846d7d926d 100644 --- a/crates/ark/src/lsp/handlers.rs +++ b/crates/ark/src/lsp/handlers.rs @@ -9,40 +9,40 @@ use anyhow::anyhow; use serde_json::Value; use stdext::result::ResultExt; use stdext::unwrap; -use tower_lsp::lsp_types::CodeActionParams; -use tower_lsp::lsp_types::CodeActionResponse; -use tower_lsp::lsp_types::CompletionItem; -use tower_lsp::lsp_types::CompletionParams; -use tower_lsp::lsp_types::CompletionResponse; -use tower_lsp::lsp_types::DidChangeWatchedFilesRegistrationOptions; -use tower_lsp::lsp_types::DocumentOnTypeFormattingParams; -use tower_lsp::lsp_types::DocumentSymbolParams; -use tower_lsp::lsp_types::DocumentSymbolResponse; -use tower_lsp::lsp_types::FileSystemWatcher; -use tower_lsp::lsp_types::FoldingRange; -use tower_lsp::lsp_types::FoldingRangeParams; -use tower_lsp::lsp_types::GlobPattern; -use tower_lsp::lsp_types::GotoDefinitionParams; -use tower_lsp::lsp_types::GotoDefinitionResponse; -use tower_lsp::lsp_types::Hover; -use tower_lsp::lsp_types::HoverContents; -use tower_lsp::lsp_types::HoverParams; -use tower_lsp::lsp_types::Location; -use tower_lsp::lsp_types::MessageType; -use tower_lsp::lsp_types::PrepareRenameResponse; -use tower_lsp::lsp_types::ReferenceParams; -use tower_lsp::lsp_types::Registration; -use tower_lsp::lsp_types::RenameParams; -use tower_lsp::lsp_types::SelectionRange; -use tower_lsp::lsp_types::SelectionRangeParams; -use tower_lsp::lsp_types::SignatureHelp; -use tower_lsp::lsp_types::SignatureHelpParams; -use tower_lsp::lsp_types::SymbolInformation; -use tower_lsp::lsp_types::TextDocumentPositionParams; -use tower_lsp::lsp_types::TextEdit; -use tower_lsp::lsp_types::WorkspaceEdit; -use tower_lsp::lsp_types::WorkspaceSymbolParams; -use tower_lsp::Client; +use tower_lsp_server::ls_types::CodeActionParams; +use tower_lsp_server::ls_types::CodeActionResponse; +use tower_lsp_server::ls_types::CompletionItem; +use tower_lsp_server::ls_types::CompletionParams; +use tower_lsp_server::ls_types::CompletionResponse; +use tower_lsp_server::ls_types::DidChangeWatchedFilesRegistrationOptions; +use tower_lsp_server::ls_types::DocumentOnTypeFormattingParams; +use tower_lsp_server::ls_types::DocumentSymbolParams; +use tower_lsp_server::ls_types::DocumentSymbolResponse; +use tower_lsp_server::ls_types::FileSystemWatcher; +use tower_lsp_server::ls_types::FoldingRange; +use tower_lsp_server::ls_types::FoldingRangeParams; +use tower_lsp_server::ls_types::GlobPattern; +use tower_lsp_server::ls_types::GotoDefinitionParams; +use tower_lsp_server::ls_types::GotoDefinitionResponse; +use tower_lsp_server::ls_types::Hover; +use tower_lsp_server::ls_types::HoverContents; +use tower_lsp_server::ls_types::HoverParams; +use tower_lsp_server::ls_types::Location; +use tower_lsp_server::ls_types::MessageType; +use tower_lsp_server::ls_types::PrepareRenameResponse; +use tower_lsp_server::ls_types::ReferenceParams; +use tower_lsp_server::ls_types::Registration; +use tower_lsp_server::ls_types::RenameParams; +use tower_lsp_server::ls_types::SelectionRange; +use tower_lsp_server::ls_types::SelectionRangeParams; +use tower_lsp_server::ls_types::SignatureHelp; +use tower_lsp_server::ls_types::SignatureHelpParams; +use tower_lsp_server::ls_types::SymbolInformation; +use tower_lsp_server::ls_types::TextDocumentPositionParams; +use tower_lsp_server::ls_types::TextEdit; +use tower_lsp_server::ls_types::WorkspaceEdit; +use tower_lsp_server::ls_types::WorkspaceSymbolParams; +use tower_lsp_server::Client; use tracing::Instrument; use crate::analysis::input_boundaries::input_boundaries; @@ -77,6 +77,7 @@ use crate::lsp::statement_range::statement_range; use crate::lsp::statement_range::StatementRangeParams; use crate::lsp::statement_range::StatementRangeResponse; use crate::lsp::symbols; +use crate::lsp::traits::url::UriExt; use crate::r_task; pub static ARK_VDOC_REQUEST: &str = "ark/internal/virtualDocument"; @@ -187,8 +188,8 @@ pub(crate) fn handle_folding_range( params: FoldingRangeParams, state: &WorldState, ) -> LspResult>> { - let uri = ¶ms.text_document.uri; - let file = state.open_file(uri)?.file(); + let path = params.text_document.uri.to_document_path()?; + let file = state.open_file(&path)?.file(); let db = &state.db; match folding_range(db, file) { Ok(foldings) => Ok(Some(foldings)), @@ -213,8 +214,12 @@ pub(crate) fn handle_completion( params: CompletionParams, state: &WorldState, ) -> LspResult> { - let uri = params.text_document_position.text_document.uri; - let file = state.open_file(&uri)?.file(); + let path = params + .text_document_position + .text_document + .uri + .to_document_path()?; + let file = state.open_file(&path)?.file(); let db = &state.db; let encoding = state.config.position_encoding; @@ -259,8 +264,12 @@ pub(crate) fn handle_completion_resolve(mut item: CompletionItem) -> LspResult LspResult> { - let uri = params.text_document_position_params.text_document.uri; - let file = state.open_file(&uri)?.file(); + let path = params + .text_document_position_params + .text_document + .uri + .to_document_path()?; + let file = state.open_file(&path)?.file(); let db = &state.db; let encoding = state.config.position_encoding; @@ -302,8 +311,12 @@ pub(crate) fn handle_signature_help( params: SignatureHelpParams, state: &WorldState, ) -> LspResult> { - let uri = params.text_document_position_params.text_document.uri; - let file = state.open_file(&uri)?.file(); + let path = params + .text_document_position_params + .text_document + .uri + .to_document_path()?; + let file = state.open_file(&path)?.file(); let db = &state.db; let encoding = state.config.position_encoding; @@ -349,8 +362,8 @@ pub(crate) fn handle_selection_range( params: SelectionRangeParams, state: &WorldState, ) -> LspResult>> { - let uri = ¶ms.text_document.uri; - let file = state.open_file(uri)?.file(); + let path = params.text_document.uri.to_document_path()?; + let file = state.open_file(&path)?.file(); let db = &state.db; let encoding = state.config.position_encoding; @@ -420,8 +433,8 @@ pub(crate) fn handle_statement_range( params: StatementRangeParams, state: &WorldState, ) -> LspResult> { - let uri = ¶ms.text_document.uri; - let file = state.open_file(uri)?.file(); + let path = params.text_document.uri.to_document_path()?; + let file = state.open_file(&path)?.file(); let db = &state.db; let encoding = state.config.position_encoding; let point = @@ -434,8 +447,8 @@ pub(crate) fn handle_help_topic( params: HelpTopicParams, state: &WorldState, ) -> LspResult> { - let uri = ¶ms.text_document.uri; - let file = state.open_file(uri)?.file(); + let path = params.text_document.uri.to_document_path()?; + let file = state.open_file(&path)?.file(); let db = &state.db; let encoding = state.config.position_encoding; let point = @@ -449,8 +462,8 @@ pub(crate) fn handle_indent( state: &WorldState, ) -> LspResult>> { let ctxt = params.text_document_position; - let uri = &ctxt.text_document.uri; - let open_file = state.open_file(uri)?; + let path = ctxt.text_document.uri.to_document_path()?; + let open_file = state.open_file(&path)?; let encoding = state.config.position_encoding; let db = &state.db; @@ -480,8 +493,8 @@ pub(crate) fn handle_code_action( lsp_state: &LspState, state: &WorldState, ) -> LspResult> { - let uri = params.text_document.uri; - let file = state.open_file(&uri)?; + let path = params.text_document.uri.to_document_path()?; + let file = state.open_file(&path)?; let db = &state.db; let encoding = state.config.position_encoding; let range = tree_sitter_range_from_lsp_range(params.range, file.line_index(db), encoding)?; diff --git a/crates/ark/src/lsp/help.rs b/crates/ark/src/lsp/help.rs index 0f48f2bd3f..537cd222fb 100644 --- a/crates/ark/src/lsp/help.rs +++ b/crates/ark/src/lsp/help.rs @@ -17,8 +17,8 @@ use scraper::Selector; use stdext::push; use stdext::unwrap; use stdext::unwrap::IntoResult; -use tower_lsp::lsp_types::MarkupContent; -use tower_lsp::lsp_types::MarkupKind; +use tower_lsp_server::ls_types::MarkupContent; +use tower_lsp_server::ls_types::MarkupKind; use crate::lsp::markdown::*; @@ -371,7 +371,7 @@ fn for_each_section(doc: &Html, mut callback: impl FnMut(ElementRef, Vec FileIndex { } /// Visit every workspace symbol across all indexable files. Callers that need a -/// URL for messages to the frontend can resolve it from `File` via -/// `WorldState::wire_url`. +/// URI for messages to the frontend can resolve it from `File` via +/// `WorldState::wire_uri`. pub(crate) fn map(db: &dyn ArkDb, mut callback: impl FnMut(File, &str, &IndexEntry)) { for &file in oak_db::workspace_files(db) { if !is_indexable(db, file) { @@ -180,10 +181,7 @@ pub(crate) fn warm(db: &dyn ArkDb) { } fn is_indexable(db: &dyn ArkDb, file: File) -> bool { - match file.path(db) { - aether_path::FilePath::File(_) => true, - aether_path::FilePath::Virtual(uri) => uri.as_url().scheme() != "ark", - } + !file.path(db).is_ark_virtual_doc() } fn index_insert(index: &mut rustc_hash::FxHashMap, entry: IndexEntry) { diff --git a/crates/ark/src/lsp/main_loop.rs b/crates/ark/src/lsp/main_loop.rs index 87c59c4a5f..12d3f7a6c1 100644 --- a/crates/ark/src/lsp/main_loop.rs +++ b/crates/ark/src/lsp/main_loop.rs @@ -32,12 +32,12 @@ use tokio::sync::mpsc; use tokio::sync::mpsc::unbounded_channel as tokio_unbounded_channel; use tokio::sync::oneshot; use tokio::task::JoinHandle; -use tower_lsp::jsonrpc; -use tower_lsp::lsp_types; -use tower_lsp::lsp_types::Diagnostic; -use tower_lsp::lsp_types::MessageType; -use tower_lsp::Client; -use url::Url; +use tower_lsp_server::jsonrpc; +use tower_lsp_server::ls_types as lsp_types; +use tower_lsp_server::ls_types::Diagnostic; +use tower_lsp_server::ls_types::MessageType; +use tower_lsp_server::ls_types::Uri; +use tower_lsp_server::Client; use super::backend::RequestResponse; use crate::console::ConsoleNotification; @@ -61,7 +61,8 @@ use crate::lsp::state::WorldState; use crate::lsp::state::WorldStateSnapshot; use crate::lsp::state_handlers; use crate::lsp::state_handlers::ConsoleInputs; -use crate::url::ExtUrl; +use crate::lsp::traits::url::UriExt; +use crate::url::FilePathExt; pub(crate) type TokioUnboundedSender = tokio::sync::mpsc::UnboundedSender; pub(crate) type TokioUnboundedReceiver = tokio::sync::mpsc::UnboundedReceiver; @@ -133,7 +134,7 @@ pub(crate) struct DidCloseVirtualDocumentParams { #[derive(Debug)] pub(crate) enum AuxiliaryEvent { Log(lsp_types::MessageType, String), - PublishDiagnostics(Url, Vec, Option), + PublishDiagnostics(DiagnosticsPublication), SpawnedTask(JoinHandle>>), Shutdown, } @@ -231,7 +232,7 @@ struct AuxiliaryState { /// Last non-empty diagnostics published per file. A refresh re-runs every /// open file, but most runs produce the same result, so we skip the publish /// when it matches what the client already has. - published_diagnostics: HashMap>, + published_diagnostics: HashMap>, } impl GlobalState { @@ -484,7 +485,9 @@ impl GlobalState { respond(tx, || handlers::handle_help_topic(params, &self.world), LspResponse::HelpTopic)?; }, LspRequest::OnTypeFormatting(params) => { - state_handlers::did_change_formatting_options(¶ms.text_document_position.text_document.uri, ¶ms.options, &mut self.world); + if let Some(path) = params.text_document_position.text_document.uri.to_document_path().log_err() { + state_handlers::did_change_formatting_options(&path, ¶ms.options, &mut self.world); + } respond(tx, || handlers::handle_indent(params, &self.world), LspResponse::OnTypeFormatting)?; }, LspRequest::CodeAction(params) => { @@ -794,8 +797,8 @@ impl AuxiliaryState { match self.next_event().await { AuxiliaryEvent::Log(level, message) => self.log(level, message).await, AuxiliaryEvent::SpawnedTask(handle) => self.tasks.push(Box::pin(handle)), - AuxiliaryEvent::PublishDiagnostics(uri, diagnostics, version) => { - self.publish_diagnostics(uri, diagnostics, version).await + AuxiliaryEvent::PublishDiagnostics(publication) => { + self.publish_diagnostics(publication).await }, AuxiliaryEvent::Shutdown => break, } @@ -830,19 +833,21 @@ impl AuxiliaryState { } } - /// Publish diagnostics for `uri`, skipping the client round-trip when the - /// set is identical to what we last sent for that file. Only non-empty - /// sets are remembered, and an absent entry counts as empty. So an empty - /// result is published only when it clears diagnostics the client is - /// currently showing, and the map stays bounded by the files on screen - /// with diagnostics. - async fn publish_diagnostics( - &mut self, - uri: Url, - diagnostics: Vec, - version: Option, - ) { - let unchanged = match self.published_diagnostics.get(&uri) { + /// Publish diagnostics, skipping the client round-trip when the set is + /// identical to what we last sent for that file. Only non-empty sets are + /// remembered, and an absent entry counts as empty. So an empty result is + /// published only when it clears diagnostics the client is currently + /// showing, and the map stays bounded by the files on screen with + /// diagnostics. + async fn publish_diagnostics(&mut self, publication: DiagnosticsPublication) { + let DiagnosticsPublication { + path, + uri, + diagnostics, + version, + } = publication; + + let unchanged = match self.published_diagnostics.get(&path) { Some(old) => diagnostics == *old, None => diagnostics.is_empty(), }; @@ -851,10 +856,9 @@ impl AuxiliaryState { } if diagnostics.is_empty() { - self.published_diagnostics.remove(&uri); + self.published_diagnostics.remove(&path); } else { - self.published_diagnostics - .insert(uri.clone(), diagnostics.clone()); + self.published_diagnostics.insert(path, diagnostics.clone()); } self.client @@ -963,12 +967,8 @@ where send_auxiliary(AuxiliaryEvent::SpawnedTask(handle)); } -pub(crate) fn publish_diagnostics(uri: Url, diagnostics: Vec, version: Option) { - send_auxiliary(AuxiliaryEvent::PublishDiagnostics( - uri, - diagnostics, - version, - )); +pub(crate) fn publish_diagnostics(publication: DiagnosticsPublication) { + send_auxiliary(AuxiliaryEvent::PublishDiagnostics(publication)); } impl KernelNotification { @@ -1004,10 +1004,14 @@ pub(crate) struct RefreshDiagnosticsTask { } #[derive(Debug)] -struct RefreshDiagnosticsResult { - uri: Url, - diagnostics: Vec, - version: Option, +pub(crate) struct DiagnosticsPublication { + /// Identity for the dedup cache. Two spellings of the same document + /// have the same identity. + pub(crate) path: FilePath, + /// Wire information. + pub(crate) uri: Uri, + pub(crate) diagnostics: Vec, + pub(crate) version: Option, } static DIAGNOSTICS_QUEUE: LazyLock> = @@ -1019,7 +1023,7 @@ static DIAGNOSTICS_QUEUE: LazyLock) { - // Deduplicate tasks by keeping only the last one for each URI. We use a - // `HashMap` so only the last insertion is retained. This is effectively a - // way of cancelling diagnostics tasks for outdated documents. - let batch: HashMap<_, _> = batch + // Deduplicate tasks by keeping only the last one for each file, which is + // effectively a way of cancelling diagnostics tasks for outdated documents. + let batch: HashMap = batch .into_iter() - .map(|task| (task.file.wire_url().clone(), task)) + .map(|task| (task.file.file().path(task.state.db()).clone(), task)) .collect(); tracing::trace!("Processing {n} diagnostic tasks", n = batch.len()); @@ -1054,43 +1057,42 @@ fn process_diagnostics_batch(batch: Vec) { // Each file is its own blocking task. `spawn_blocking()` catches salsa // cancellation, so a pass cancelled by a concurrent edit just produces no // event. The publish happens via the returned [`AuxiliaryEvent`]. - for (_uri, task) in batch { + for (_path, task) in batch { lsp::spawn_blocking(move || { - let result = refresh_diagnostics(task); - Ok(Some(AuxiliaryEvent::PublishDiagnostics( - result.uri, - result.diagnostics, - result.version, - ))) + let publication = refresh_diagnostics(task); + Ok(Some(AuxiliaryEvent::PublishDiagnostics(publication))) }); } } -fn refresh_diagnostics(task: RefreshDiagnosticsTask) -> RefreshDiagnosticsResult { +fn refresh_diagnostics(task: RefreshDiagnosticsTask) -> DiagnosticsPublication { let RefreshDiagnosticsTask { file, state } = task; - let uri = file.wire_url().clone(); + let path = file.file().path(state.db()).clone(); + let uri = file.wire_uri().clone(); let version = file.version(); - let _span = tracing::info_span!("diagnostics_refresh", uri = %uri).entered(); + let _span = tracing::info_span!("diagnostics_refresh", uri = %uri.as_str()).entered(); // Special case testthat-specific behaviour. This is a simple stopgap // approach that has some false positives (e.g. when we work on testthat // itself the flag will always be true), but that shouldn't have much // practical impact. - let testthat = Path::new(uri.path()) - .components() - .any(|c| c.as_os_str() == "testthat"); + let testthat = path + .as_path() + .is_some_and(|path| path.components().any(|c| c.as_str() == "testthat")); let now = std::time::Instant::now(); - lsp::log_info!("Generating diagnostics for file: {uri}"); + lsp::log_info!("Generating diagnostics for file: {}", uri.as_str()); let diagnostics = generate_diagnostics(file.file(), state, testthat); lsp::log_info!( - "Finished diagnostics for file: {uri} in {:.0?}", + "Finished diagnostics for file: {} in {:.0?}", + uri.as_str(), now.elapsed() ); - RefreshDiagnosticsResult { + DiagnosticsPublication { + path, uri, diagnostics, version, @@ -1109,7 +1111,7 @@ pub(crate) fn diagnostics_refresh_all(state: &WorldState) { ); for file in state.open_files.values() { - if !ExtUrl::should_diagnose(file.wire_url()) { + if !file.file().path(&state.db).should_diagnose() { continue; } @@ -1148,7 +1150,7 @@ fn warm_workspace_index(state: WorldStateSnapshot) { mod tests { use aether_path::FilePath; use oak_scan::DbScan; - use tower_lsp::jsonrpc; + use tower_lsp_server::jsonrpc; use url::Url; use super::catch_cancellation; @@ -1160,6 +1162,7 @@ mod tests { use crate::lsp::backend::LspResponse; use crate::lsp::backend::RequestResponse; use crate::lsp::state::WorldState; + use crate::lsp::traits::url::UrlExt; /// A salsa cancellation during the pass is swallowed into `None` by /// `catch_cancellation`, the wrapper `spawn_blocking` applies to every task, @@ -1177,9 +1180,9 @@ mod tests { let file = state .db .upsert_editor(FilePath::from_url(&uri), code.to_string()); - state.insert_open_file(uri.clone(), file, None); + state.insert_open_file(uri.to_uri().unwrap(), FilePath::from_url(&uri), file, None); - let file = state.open_file(&uri).unwrap().clone(); + let file = state.open_file(&FilePath::from_url(&uri)).unwrap().clone(); let snapshot = state.diagnostics_snapshot(); snapshot.cancellation_token().cancel(); @@ -1201,9 +1204,9 @@ mod tests { let file = state .db .upsert_editor(FilePath::from_url(&uri), "foo".to_string()); - state.insert_open_file(uri.clone(), file, None); + state.insert_open_file(uri.to_uri().unwrap(), FilePath::from_url(&uri), file, None); - let file = state.open_file(&uri).unwrap().clone(); + let file = state.open_file(&FilePath::from_url(&uri)).unwrap().clone(); let snapshot = state.diagnostics_snapshot(); snapshot.cancellation_token().cancel(); diff --git a/crates/ark/src/lsp/open_file.rs b/crates/ark/src/lsp/open_file.rs index ed74a43bbe..d0afe9ce50 100644 --- a/crates/ark/src/lsp/open_file.rs +++ b/crates/ark/src/lsp/open_file.rs @@ -3,7 +3,8 @@ use aether_lsp_utils::proto::to_proto; use aether_lsp_utils::proto::PositionEncoding; use oak_db::Db; use oak_db::File; -use tower_lsp::lsp_types; +use tower_lsp_server::ls_types as lsp_types; +#[cfg(test)] use url::Url; use crate::lsp::config::DocumentConfig; @@ -19,16 +20,16 @@ pub(crate) struct OpenFile { file: File, version: Option, config: DocumentConfig, - wire_url: Url, + wire_uri: lsp_types::Uri, } impl OpenFile { - pub(crate) fn new(file: File, version: Option, wire_url: Url) -> Self { + pub(crate) fn new(file: File, version: Option, wire_uri: lsp_types::Uri) -> Self { Self { file, version, config: DocumentConfig::default(), - wire_url, + wire_uri, } } @@ -61,10 +62,10 @@ impl OpenFile { &self.config } - /// The verbatim editor URL, preserved so wire output echoes the URI the - /// editor sent us. See [`crate::lsp::state::WorldState::wire_url`]. - pub(crate) fn wire_url(&self) -> &Url { - &self.wire_url + /// The verbatim `Uri` the editor sent us, preserved so wire output + /// replays it exactly. See [`crate::lsp::state::WorldState::wire_uri`]. + pub(crate) fn wire_uri(&self) -> &lsp_types::Uri { + &self.wire_uri } pub(crate) fn set_version(&mut self, version: Option) { @@ -165,6 +166,7 @@ pub(crate) fn test_open_file(code: &str) -> (oak_db::OakDatabase, OpenFile) { let db = oak_db::OakDatabase::new(); let url = Url::parse("file:///test.R").unwrap(); + let uri: lsp_types::Uri = url.as_str().parse().unwrap(); let key = FilePath::from_url(&url); let inner = File::new( &db, @@ -173,7 +175,7 @@ pub(crate) fn test_open_file(code: &str) -> (oak_db::OakDatabase, OpenFile) { Some(code.to_string()), None, ); - let file = OpenFile::new(inner, None, url); + let file = OpenFile::new(inner, None, uri); (db, file) } diff --git a/crates/ark/src/lsp/rename.rs b/crates/ark/src/lsp/rename.rs index 8a5b28f6fe..b49f4576c4 100644 --- a/crates/ark/src/lsp/rename.rs +++ b/crates/ark/src/lsp/rename.rs @@ -2,29 +2,30 @@ use std::collections::HashMap; use aether_lsp_utils::proto::from_proto; use aether_lsp_utils::proto::to_proto; -use aether_path::FilePath; +use anyhow::Context; use oak_core::identifier::to_identifier_text; use oak_db::Db; -use tower_lsp::lsp_types; -use tower_lsp::lsp_types::PrepareRenameResponse; -use tower_lsp::lsp_types::RenameParams; -use tower_lsp::lsp_types::TextDocumentPositionParams; -use tower_lsp::lsp_types::TextEdit; -use tower_lsp::lsp_types::WorkspaceEdit; +use tower_lsp_server::ls_types as lsp_types; +use tower_lsp_server::ls_types::PrepareRenameResponse; +use tower_lsp_server::ls_types::RenameParams; +use tower_lsp_server::ls_types::TextDocumentPositionParams; +use tower_lsp_server::ls_types::TextEdit; +use tower_lsp_server::ls_types::WorkspaceEdit; use crate::lsp::state::WorldState; +use crate::lsp::traits::url::UriExt; pub(crate) fn prepare_rename( params: TextDocumentPositionParams, state: &WorldState, ) -> anyhow::Result> { - let uri = params.text_document.uri; + let path = params.text_document.uri.to_document_path()?; let position = params.position; let db = &state.db; let encoding = state.config.position_encoding; - let Some(file) = db.file_by_path(&FilePath::from_url(&uri)) else { + let Some(file) = db.file_by_path(&path) else { return Ok(None); }; @@ -45,14 +46,18 @@ pub(crate) fn rename( params: RenameParams, state: &WorldState, ) -> anyhow::Result> { - let uri = params.text_document_position.text_document.uri; + let path = params + .text_document_position + .text_document + .uri + .to_document_path()?; let position = params.text_document_position.position; let new_name = params.new_name; let db = &state.db; let encoding = state.config.position_encoding; - let Some(file) = db.file_by_path(&FilePath::from_url(&uri)) else { + let Some(file) = db.file_by_path(&path) else { return Ok(None); }; @@ -63,12 +68,21 @@ pub(crate) fn rename( let new_text = to_identifier_text(&new_name)?; let ranges = oak_ide::rename(db, file, offset)?; - let mut changes: HashMap> = HashMap::new(); - for range in ranges { - let line_index = range.file.line_index(db); - let target_url = state.wire_url(range.file); - let range = to_proto::range(range.range, line_index, encoding)?; - changes.entry(target_url).or_default().push(TextEdit { + let mut changes: HashMap> = HashMap::new(); + for file_range in ranges { + let line_index = file_range.file.line_index(db); + let path = file_range.file.path(db); + + // A rename is all or nothing. Skipping a file here would rename some + // uses of the symbol and silently leave the rest behind, so we bail + // instead. + let target_uri = state + .wire_uri(file_range.file) + .with_context(|| format!("Can't rename: no valid URI for `{path}`."))?; + let range = to_proto::range(file_range.range, line_index, encoding) + .with_context(|| format!("Can't rename: no valid text range in `{path}`."))?; + + changes.entry(target_uri).or_default().push(TextEdit { range, new_text: new_text.clone(), }); diff --git a/crates/ark/src/lsp/selection_range.rs b/crates/ark/src/lsp/selection_range.rs index e0dce7c5f6..ba80bae110 100644 --- a/crates/ark/src/lsp/selection_range.rs +++ b/crates/ark/src/lsp/selection_range.rs @@ -7,7 +7,7 @@ use aether_lsp_utils::proto::PositionEncoding; use oak_db::File; -use tower_lsp::lsp_types; +use tower_lsp_server::ls_types as lsp_types; use tree_sitter::Node; use tree_sitter::Point; use tree_sitter::Range; @@ -18,7 +18,7 @@ use crate::lsp::open_file::lsp_range_from_tree_sitter_range; use crate::treesitter::NodeType; use crate::treesitter::NodeTypeExt; -/// A mirror of `tower_lsp::lsp_types::SelectionRange`, but using tree-sitter ranges +/// A mirror of `tower_lsp_server::ls_types::SelectionRange`, but using tree-sitter ranges pub struct SelectionRange { pub range: Range, pub parent: Option>, diff --git a/crates/ark/src/lsp/signature_help.rs b/crates/ark/src/lsp/signature_help.rs index 691c80a7e2..f668666e01 100644 --- a/crates/ark/src/lsp/signature_help.rs +++ b/crates/ark/src/lsp/signature_help.rs @@ -19,11 +19,11 @@ use libr::*; use log::info; use stdext::unwrap; use stdext::unwrap::IntoResult; -use tower_lsp::lsp_types::Documentation; -use tower_lsp::lsp_types::ParameterInformation; -use tower_lsp::lsp_types::ParameterLabel; -use tower_lsp::lsp_types::SignatureHelp; -use tower_lsp::lsp_types::SignatureInformation; +use tower_lsp_server::ls_types::Documentation; +use tower_lsp_server::ls_types::ParameterInformation; +use tower_lsp_server::ls_types::ParameterLabel; +use tower_lsp_server::ls_types::SignatureHelp; +use tower_lsp_server::ls_types::SignatureInformation; use tree_sitter::Node; use tree_sitter::Point; @@ -506,7 +506,7 @@ mod tests { use harp::r_symbol; use harp::RObject; use libr::R_xlen_t; - use tower_lsp::lsp_types::ParameterLabel; + use tower_lsp_server::ls_types::ParameterLabel; use crate::fixtures::point_from_cursor; use crate::lsp::document_context::TestDocument; diff --git a/crates/ark/src/lsp/state.rs b/crates/ark/src/lsp/state.rs index 7a8584378a..b3c2e61be9 100644 --- a/crates/ark/src/lsp/state.rs +++ b/crates/ark/src/lsp/state.rs @@ -1,15 +1,17 @@ use std::collections::HashMap; +use aether_path::AbsPathBuf; use aether_path::FilePath; use anyhow::anyhow; use oak_db::File; use oak_db::OakDatabase; use salsa::Database; -use url::Url; +use tower_lsp_server::ls_types::Uri; use crate::lsp::config::LspConfig; use crate::lsp::db::ArkDb; use crate::lsp::open_file::OpenFile; +use crate::lsp::traits::url::UrlExt; #[derive(Clone, Default, Debug)] /// The world state, i.e. all the inputs necessary for analysing or refactoring @@ -26,7 +28,7 @@ pub(crate) struct WorldState { pub(crate) db: OakDatabase, /// Watched documents, keyed on the normalised [`FilePath`] form. - /// The verbatim editor URL is preserved on each [`OpenFile::wire_url`] + /// The verbatim editor `Uri` is preserved on each [`OpenFile::wire_uri`] /// for wire output. pub(crate) open_files: HashMap, @@ -68,7 +70,7 @@ pub(crate) struct WorldState { #[derive(Clone, Default, Debug)] pub(crate) struct Workspace { - pub folders: Vec, + pub folders: Vec, } impl WorldState { @@ -80,29 +82,27 @@ impl WorldState { } /// Full read-only snapshot for a background reader that needs more than the - /// db, e.g. completions read `open_files` and the workspace. Same shape as - /// `self.clone()`, but the db is only reachable as `&dyn ArkDb`. + /// db, e.g. completions read the workspace. Same shape as `self.clone()`, + /// but the db is only reachable as `&dyn ArkDb`. pub(crate) fn snapshot(&self) -> WorldStateSnapshot { WorldStateSnapshot { db: self.db.clone(), console_scopes: self.console_scopes.clone(), installed_packages: self.installed_packages.clone(), config: self.config.clone(), - open_files: self.open_files.clone(), workspace: self.workspace.clone(), } } /// Trimmed read-only snapshot for the diagnostics worker, which runs off - /// the main loop and queries oak. Drops the open-file / workspace maps the - /// diagnostics pass doesn't read. + /// the main loop and queries oak. Drops the workspace map the diagnostics + /// pass doesn't read. pub(crate) fn diagnostics_snapshot(&self) -> WorldStateSnapshot { WorldStateSnapshot { db: self.db.clone(), console_scopes: self.console_scopes.clone(), installed_packages: self.installed_packages.clone(), config: self.config.clone(), - open_files: HashMap::new(), workspace: Workspace::default(), } } @@ -117,46 +117,56 @@ impl WorldState { self.db.synthetic_write(salsa::Durability::LOW); } - pub(crate) fn open_file_mut(&mut self, uri: &Url) -> anyhow::Result<&mut OpenFile> { - let key = FilePath::from_url(uri); - if let Some(open_file) = self.open_files.get_mut(&key) { - Ok(open_file) - } else { - Err(anyhow!("Can't find document for URI {uri}")) - } + pub(crate) fn open_file_mut(&mut self, path: &FilePath) -> anyhow::Result<&mut OpenFile> { + self.open_files + .get_mut(path) + .ok_or_else(|| anyhow!("Can't find document for path {path}")) } /// The stored [`OpenFile`] for a request. - pub(crate) fn open_file(&self, uri: &Url) -> anyhow::Result<&OpenFile> { - let key = FilePath::from_url(uri); - let Some(open_file) = self.open_files.get(&key) else { - return Err(anyhow!("Can't find document for URI {uri}")); - }; - Ok(open_file) + pub(crate) fn open_file(&self, path: &FilePath) -> anyhow::Result<&OpenFile> { + self.open_files + .get(path) + .ok_or_else(|| anyhow!("Can't find document for path {path}")) } - /// URL to put on the wire for `file`. Open buffers keep the editor's - /// verbatim URL so the frontend sees the URI it sent us. Files that were - /// never opened in the editor (disk-scanned files, resolution targets) have - /// no verbatim URL, so synthesise one from the normalised path. - pub(crate) fn wire_url(&self, file: File) -> Url { + /// The [`Uri`] to put on the wire for `file`. Open buffers replay the + /// editor's verbatim `Uri`, so the frontend gets back exactly the bytes it + /// sent us. Files that were never opened in the editor (disk-scanned + /// files, resolution targets) have no verbatim `Uri`, so synthesise one + /// from the normalised path. + /// + /// Fails only for a `Virtual` file that was never opened: `Url` tolerates + /// characters in a path (`[`, `]`, `|`, `^`) that `Uri` rejects, so + /// [`aether_path::VirtualUri`]'s stored `Url` can be one `Uri` can't parse. + pub(crate) fn wire_uri(&self, file: File) -> anyhow::Result { let path = file.path(&self.db); - self.open_files - .get(path) - .map(|open_file| open_file.wire_url().clone()) - .unwrap_or_else(|| path.to_url()) + + if let Some(open_file) = self.open_files.get(path) { + return Ok(open_file.wire_uri().clone()); + } + + match path { + FilePath::File(abs_path) => Uri::from_file_path(abs_path.as_path()) + .ok_or_else(|| anyhow!("error building a URI for path {abs_path}")), + FilePath::Virtual(uri) => uri.as_url().to_uri(), + } } - /// Register an editor buffer in `open_files`, keying on the normalised - /// [`FilePath`] and stashing the verbatim editor URL on [`OpenFile::wire_url`] for - /// wire output. + /// Register an editor buffer in `open_files`, keying on `path` and stashing + /// the verbatim editor `uri` on [`OpenFile::wire_uri`] for wire output. /// /// The caller is in charge of pushing the contents into `oak` via /// `upsert_editor()` and handing us the resulting [`File`]. - pub(crate) fn insert_open_file(&mut self, url: Url, file: File, version: Option) { - let key = FilePath::from_url(&url); - let open_file = OpenFile::new(file, version, url); - self.open_files.insert(key, open_file); + pub(crate) fn insert_open_file( + &mut self, + uri: Uri, + path: FilePath, + file: File, + version: Option, + ) { + let open_file = OpenFile::new(file, version, uri); + self.open_files.insert(path, open_file); } } @@ -168,7 +178,6 @@ impl WorldState { pub(crate) struct WorldStateSnapshot { /// Private so readers can only reach it through [`Self::db`]. db: OakDatabase, - pub(crate) open_files: HashMap, pub(crate) workspace: Workspace, pub(crate) console_scopes: Vec>, pub(crate) installed_packages: Vec, @@ -190,21 +199,14 @@ impl WorldStateSnapshot { pub(crate) fn cancellation_token(&self) -> salsa::CancellationToken { salsa::Database::cancellation_token(&self.db) } - - /// URL to put on the wire for `file`. Same rule as [`WorldState::wire_url`]. - pub(crate) fn wire_url(&self, file: File) -> Url { - let path = file.path(self.db()); - self.open_files - .get(path) - .map(|open_file| open_file.wire_url().clone()) - .unwrap_or_else(|| path.to_url()) - } } -pub(crate) fn open_file_wire_urls(state: &WorldState) -> Vec { +/// The wire `Uri` of every open buffer, paired with the [`FilePath`] it's keyed +/// on so a caller can look the buffer back up without converting. +pub(crate) fn open_file_wire_uris(state: &WorldState) -> Vec<(FilePath, Uri)> { state .open_files - .values() - .map(|doc| doc.wire_url().clone()) + .iter() + .map(|(path, open_file)| (path.clone(), open_file.wire_uri().clone())) .collect() } diff --git a/crates/ark/src/lsp/state_handlers.rs b/crates/ark/src/lsp/state_handlers.rs index c62a4f7d76..c2154848c2 100644 --- a/crates/ark/src/lsp/state_handlers.rs +++ b/crates/ark/src/lsp/state_handlers.rs @@ -8,43 +8,44 @@ use std::collections::HashSet; use std::path::PathBuf; +use aether_path::AbsPathBuf; use aether_path::FilePath; use anyhow::anyhow; use oak_scan::DbScan; use oak_scan::FileEvent; use oak_scan::FileEventKind; use stdext::result::ResultExt; -use tower_lsp::lsp_types; -use tower_lsp::lsp_types::CompletionOptions; -use tower_lsp::lsp_types::CompletionOptionsCompletionItem; -use tower_lsp::lsp_types::DidChangeConfigurationParams; -use tower_lsp::lsp_types::DidChangeTextDocumentParams; -use tower_lsp::lsp_types::DidChangeWatchedFilesParams; -use tower_lsp::lsp_types::DidChangeWorkspaceFoldersParams; -use tower_lsp::lsp_types::DidCloseTextDocumentParams; -use tower_lsp::lsp_types::DidOpenTextDocumentParams; -use tower_lsp::lsp_types::DocumentOnTypeFormattingOptions; -use tower_lsp::lsp_types::ExecuteCommandOptions; -use tower_lsp::lsp_types::FileChangeType; -use tower_lsp::lsp_types::FoldingRangeProviderCapability; -use tower_lsp::lsp_types::FormattingOptions; -use tower_lsp::lsp_types::HoverProviderCapability; -use tower_lsp::lsp_types::ImplementationProviderCapability; -use tower_lsp::lsp_types::InitializeParams; -use tower_lsp::lsp_types::InitializeResult; -use tower_lsp::lsp_types::OneOf; -use tower_lsp::lsp_types::RenameOptions; -use tower_lsp::lsp_types::SelectionRangeProviderCapability; -use tower_lsp::lsp_types::ServerCapabilities; -use tower_lsp::lsp_types::ServerInfo; -use tower_lsp::lsp_types::SignatureHelpOptions; -use tower_lsp::lsp_types::TextDocumentSyncCapability; -use tower_lsp::lsp_types::TextDocumentSyncKind; -use tower_lsp::lsp_types::WorkDoneProgressOptions; -use tower_lsp::lsp_types::WorkspaceFoldersServerCapabilities; -use tower_lsp::lsp_types::WorkspaceServerCapabilities; +use tower_lsp_server::ls_types as lsp_types; +use tower_lsp_server::ls_types::CompletionOptions; +use tower_lsp_server::ls_types::CompletionOptionsCompletionItem; +use tower_lsp_server::ls_types::DidChangeConfigurationParams; +use tower_lsp_server::ls_types::DidChangeTextDocumentParams; +use tower_lsp_server::ls_types::DidChangeWatchedFilesParams; +use tower_lsp_server::ls_types::DidChangeWorkspaceFoldersParams; +use tower_lsp_server::ls_types::DidCloseTextDocumentParams; +use tower_lsp_server::ls_types::DidOpenTextDocumentParams; +use tower_lsp_server::ls_types::DocumentOnTypeFormattingOptions; +use tower_lsp_server::ls_types::ExecuteCommandOptions; +use tower_lsp_server::ls_types::FileChangeType; +use tower_lsp_server::ls_types::FoldingRangeProviderCapability; +use tower_lsp_server::ls_types::FormattingOptions; +use tower_lsp_server::ls_types::HoverProviderCapability; +use tower_lsp_server::ls_types::ImplementationProviderCapability; +use tower_lsp_server::ls_types::InitializeParams; +use tower_lsp_server::ls_types::InitializeResult; +use tower_lsp_server::ls_types::OneOf; +use tower_lsp_server::ls_types::RenameOptions; +use tower_lsp_server::ls_types::SelectionRangeProviderCapability; +use tower_lsp_server::ls_types::ServerCapabilities; +use tower_lsp_server::ls_types::ServerInfo; +use tower_lsp_server::ls_types::SignatureHelpOptions; +use tower_lsp_server::ls_types::TextDocumentSyncCapability; +use tower_lsp_server::ls_types::TextDocumentSyncKind; +use tower_lsp_server::ls_types::Uri; +use tower_lsp_server::ls_types::WorkDoneProgressOptions; +use tower_lsp_server::ls_types::WorkspaceFoldersServerCapabilities; +use tower_lsp_server::ls_types::WorkspaceServerCapabilities; use tracing::Instrument; -use url::Url; use crate::console::ConsoleNotification; use crate::lsp; @@ -55,13 +56,15 @@ use crate::lsp::config::DOCUMENT_SETTINGS; use crate::lsp::config::GLOBAL_SETTINGS; use crate::lsp::content_changes::apply_content_changes; use crate::lsp::main_loop::dispatch_scan_requests; +use crate::lsp::main_loop::DiagnosticsPublication; use crate::lsp::main_loop::DidCloseVirtualDocumentParams; use crate::lsp::main_loop::DidOpenVirtualDocumentParams; use crate::lsp::main_loop::Event; use crate::lsp::main_loop::LspState; use crate::lsp::main_loop::TokioUnboundedSender; -use crate::lsp::state::open_file_wire_urls; +use crate::lsp::state::open_file_wire_uris; use crate::lsp::state::WorldState; +use crate::lsp::traits::url::UriExt; // Handlers that mutate the world state @@ -89,25 +92,18 @@ pub(crate) fn initialize( state: &mut WorldState, events_tx: &TokioUnboundedSender, ) -> LspResult { - let workspace_uris = effective_workspace_uris(¶ms); + let workspace_paths = effective_workspace_paths(¶ms); lsp_state.capabilities = Capabilities::new(params.capabilities); - // Initialize the workspace folders - let mut workspace_paths: Vec = Vec::new(); - - for uri in workspace_uris { - state.workspace.folders.push(uri.clone()); - if let Ok(path) = uri.to_file_path() { - workspace_paths.push(path.clone()); - } - } + state.workspace.folders = workspace_paths; // Kick off the initial workspace scan let editor_owned: HashSet = state.open_files.keys().cloned().collect(); - let requests = - lsp_state - .oak_scheduler - .set_workspace_paths(&mut state.db, &workspace_paths, &editor_owned); + let requests = lsp_state.oak_scheduler.set_workspace_paths( + &mut state.db, + &to_std_paths(&state.workspace.folders), + &editor_owned, + ); dispatch_scan_requests(events_tx, requests); let result = InitializeResult { @@ -175,6 +171,7 @@ pub(crate) fn initialize( }), ..ServerCapabilities::default() }, + offset_encoding: None, }; Ok(result) @@ -184,12 +181,15 @@ pub(crate) fn initialize( /// /// We read only `workspaceFolders`, the modern field , without falling back to /// the deprecated `rootUri`. An empty or absent list means single-file mode. -pub(super) fn effective_workspace_uris(params: &InitializeParams) -> Vec { +/// A folder that isn't a `file:` URI, or whose path we can't make sense of, is +/// silently dropped: it never backs a directory we could scan. +pub(super) fn effective_workspace_paths(params: &InitializeParams) -> Vec { params .workspace_folders .iter() .flatten() - .map(|folder| folder.uri.clone()) + .filter_map(|folder| folder.uri.to_url().log_err()) + .filter_map(|url| AbsPathBuf::from_url(&url)) .collect() } @@ -199,11 +199,12 @@ pub(crate) fn did_open( state: &mut WorldState, ) -> anyhow::Result<()> { let contents = params.text_document.text; - let uri = params.text_document.uri; + let wire_uri = params.text_document.uri; + let path = wire_uri.to_document_path()?; let version = params.text_document.version; - let file = state.db.upsert_editor(FilePath::from_url(&uri), contents); - state.insert_open_file(uri.clone(), file, Some(version)); + let file = state.db.upsert_editor(path.clone(), contents); + state.insert_open_file(wire_uri, path, file, Some(version)); // NOTE: Do we need to call `update_config()` here? // update_config(vec![uri]).await; @@ -217,12 +218,11 @@ pub(crate) fn did_change( lsp_state: &mut LspState, state: &mut WorldState, ) -> anyhow::Result<()> { - let uri = ¶ms.text_document.uri; - let key = FilePath::from_url(uri); + let path = params.text_document.uri.to_document_path()?; let new_version = params.text_document.version; let encoding = state.config.position_encoding; - let file = state.open_file(uri)?; + let file = state.open_file(&path)?; // Reject out-of-order change notifications. The spec allows version numbers // to skip values but requires them to increase monotonically. A lower @@ -243,14 +243,14 @@ pub(crate) fn did_change( ¶ms.content_changes, encoding, ); - state.db.upsert_editor(key.clone(), new_contents); + state.db.upsert_editor(path.clone(), new_contents); - state.open_file_mut(uri)?.set_version(Some(new_version)); + state.open_file_mut(&path)?.set_version(Some(new_version)); // Notify console about document change to invalidate breakpoints. lsp_state .console_notification_tx - .send(ConsoleNotification::DidChangeDocument(key)) + .send(ConsoleNotification::DidChangeDocument(path)) .log_err(); Ok(()) @@ -261,20 +261,28 @@ pub(crate) fn did_close( params: DidCloseTextDocumentParams, state: &mut WorldState, ) -> anyhow::Result<()> { - let uri = params.text_document.uri; + let wire_uri = params.text_document.uri; + let path = wire_uri.to_document_path()?; // Publish empty set of diagnostics to clear them - lsp::publish_diagnostics(uri.clone(), Vec::new(), None); + lsp::publish_diagnostics(DiagnosticsPublication { + path: path.clone(), + uri: wire_uri.clone(), + diagnostics: Vec::new(), + version: None, + }); + + state.open_files.remove(&path).ok_or(anyhow!( + "Failed to remove document for URI: {}", + wire_uri.as_str() + ))?; - state - .open_files - .remove(&FilePath::from_url(&uri)) - .ok_or(anyhow!("Failed to remove document for URI: {uri}"))?; - - let path = FilePath::from_url(&uri); state.db.close_editor(&path); - lsp::log_info!("did_close(): closed document with URI: '{uri}'."); + lsp::log_info!( + "did_close(): closed document with URI: '{}'.", + wire_uri.as_str() + ); Ok(()) } @@ -295,7 +303,7 @@ pub(crate) fn did_change_watched_files( .iter() .filter_map(|change| { Some(FileEvent { - path: FilePath::from_url(&change.uri), + path: change.uri.to_document_path().log_err()?, kind: file_event_kind(change.typ)?, }) }) @@ -326,38 +334,56 @@ pub(crate) fn did_change_workspace_folders( lsp_state: &mut LspState, events_tx: &TokioUnboundedSender, ) -> anyhow::Result<()> { - let removed: HashSet = params.event.removed.iter().map(|f| f.uri.clone()).collect(); - state.workspace.folders.retain(|uri| !removed.contains(uri)); + let removed: HashSet = params + .event + .removed + .iter() + .filter_map(|f| f.uri.to_url().log_err()) + .filter_map(|url| AbsPathBuf::from_url(&url)) + .collect(); + state + .workspace + .folders + .retain(|path| !removed.contains(path)); for folder in params.event.added { - if !state.workspace.folders.contains(&folder.uri) { - state.workspace.folders.push(folder.uri); + let Some(url) = folder.uri.to_url().log_err() else { + continue; + }; + let Some(path) = AbsPathBuf::from_url(&url) else { + continue; + }; + if !state.workspace.folders.contains(&path) { + state.workspace.folders.push(path); } } - let workspace_paths: Vec = state - .workspace - .folders - .iter() - .filter_map(|uri| uri.to_file_path().ok()) - .collect(); - // Editor-owned URLs survive eviction in `OrphanRoot` so the user's // open buffers keep getting analysed even when their workspace // folder goes away. let editor_owned: HashSet = state.open_files.keys().cloned().collect(); - let requests = - lsp_state - .oak_scheduler - .set_workspace_paths(&mut state.db, &workspace_paths, &editor_owned); + let requests = lsp_state.oak_scheduler.set_workspace_paths( + &mut state.db, + &to_std_paths(&state.workspace.folders), + &editor_owned, + ); dispatch_scan_requests(events_tx, requests); Ok(()) } +/// Convert workspace folders to the `std::path::PathBuf`s the scan scheduler +/// takes. +fn to_std_paths(paths: &[AbsPathBuf]) -> Vec { + paths + .iter() + .map(|path| path.as_path().as_std_path().to_path_buf()) + .collect() +} + pub(crate) async fn did_change_configuration( _params: DidChangeConfigurationParams, - client: &tower_lsp::Client, + client: &tower_lsp_server::Client, state: &mut WorldState, ) -> anyhow::Result<()> { // The notification params sometimes contain data but it seems in practice @@ -367,18 +393,18 @@ pub(crate) async fn did_change_configuration( // Note that the client sends notifications for settings for which we have // declared interest in. This registration is done in `handle_initialized()`. - update_config(open_file_wire_urls(state), client, state) + update_config(open_file_wire_uris(state), client, state) .instrument(tracing::info_span!("did_change_configuration")) .await } #[tracing::instrument(level = "info", skip_all)] pub(crate) fn did_change_formatting_options( - uri: &Url, + path: &FilePath, opts: &FormattingOptions, state: &mut WorldState, ) { - let Ok(doc) = state.open_file_mut(uri) else { + let Ok(doc) = state.open_file_mut(path) else { return; }; @@ -404,8 +430,8 @@ pub(crate) fn did_change_formatting_options( } async fn update_config( - uris: Vec, - client: &tower_lsp::Client, + open_files: Vec<(FilePath, Uri)>, + client: &tower_lsp_server::Client, state: &mut WorldState, ) -> anyhow::Result<()> { // Keep track of existing config to detect whether it was changed @@ -426,12 +452,12 @@ async fn update_config( // For document items we create a n_uris * n_document_settings array that we'll // handle by batch in a double loop over URIs and document settings - let mut document_items: Vec<_> = uris + let mut document_items: Vec<_> = open_files .iter() - .flat_map(|uri| { + .flat_map(|(_path, uri)| { DOCUMENT_SETTINGS .iter() - .map(|mapping| lsp_types::ConfigurationItem { + .map(move |mapping| lsp_types::ConfigurationItem { scope_uri: Some(uri.clone()), section: Some(mapping.key.to_string()), }) @@ -464,14 +490,14 @@ async fn update_config( let mut remaining = document_configs; - for uri in uris.into_iter() { + for (path, _uri) in open_files { // Need to juggle a bit because `split_off()` returns the tail of the // split and updates the vector with the head let tail = remaining.split_off(DOCUMENT_SETTINGS.len()); let head = std::mem::replace(&mut remaining, tail); for (mapping, value) in DOCUMENT_SETTINGS.iter().zip(head) { - if let Ok(doc) = state.open_file_mut(&uri) { + if let Ok(doc) = state.open_file_mut(&path) { (mapping.set)(doc.config_mut(), value); } } diff --git a/crates/ark/src/lsp/statement_range.rs b/crates/ark/src/lsp/statement_range.rs index dd38958555..906085faec 100644 --- a/crates/ark/src/lsp/statement_range.rs +++ b/crates/ark/src/lsp/statement_range.rs @@ -14,9 +14,9 @@ use regex::Regex; use serde::Deserialize; use serde::Serialize; use stdext::unwrap; -use tower_lsp::lsp_types; -use tower_lsp::lsp_types::Position; -use tower_lsp::lsp_types::VersionedTextDocumentIdentifier; +use tower_lsp_server::ls_types as lsp_types; +use tower_lsp_server::ls_types::Position; +use tower_lsp_server::ls_types::VersionedTextDocumentIdentifier; use tree_sitter::Node; use tree_sitter::Point; @@ -844,7 +844,7 @@ fn contains_row_at_different_start_position(node: Node, row: usize) -> Option { @@ -160,8 +164,8 @@ pub(crate) fn document_symbols( state: &WorldState, params: &DocumentSymbolParams, ) -> anyhow::Result> { - let uri = ¶ms.text_document.uri; - let file = state.open_file(uri)?.file(); + let path = params.text_document.uri.to_document_path()?; + let file = state.open_file(&path)?.file(); let db = &state.db; let ast = file.tree_sitter(db); @@ -792,7 +796,7 @@ pub(crate) fn parse_comment_as_section(comment: &str) -> Option<(usize, String)> mod tests { use aether_lsp_utils::proto::PositionEncoding; use oak_scan::DbScan; - use tower_lsp::lsp_types::Position; + use tower_lsp_server::ls_types::Position; use super::*; use crate::lsp::config::LspConfig; diff --git a/crates/ark/src/lsp/tests/diagnostics.rs b/crates/ark/src/lsp/tests/diagnostics.rs index 0e924b20e6..3ba95f8088 100644 --- a/crates/ark/src/lsp/tests/diagnostics.rs +++ b/crates/ark/src/lsp/tests/diagnostics.rs @@ -4,6 +4,7 @@ use url::Url; use crate::lsp::diagnostics::generate_diagnostics; use crate::lsp::state::WorldState; +use crate::lsp::traits::url::UrlExt; use crate::r_task; #[test] @@ -21,13 +22,13 @@ fn test_diagnostics_published_through_refresh_snapshot() { let file = state .db .upsert_editor(FilePath::from_url(&uri), code.to_string()); - state.insert_open_file(uri.clone(), file, None); + state.insert_open_file(uri.to_uri().unwrap(), FilePath::from_url(&uri), file, None); // Mirror `diagnostics_refresh_all`: fetch the `File` from the live // state, then hand the worker the `diagnostics_snapshot`. The snapshot's // oak must still serve that file. let file = state - .open_file(&uri) + .open_file(&FilePath::from_url(&uri)) .expect("file is open in live state") .file(); diff --git a/crates/ark/src/lsp/tests/find_references.rs b/crates/ark/src/lsp/tests/find_references.rs index afb130b489..94ab9f8026 100644 --- a/crates/ark/src/lsp/tests/find_references.rs +++ b/crates/ark/src/lsp/tests/find_references.rs @@ -1,8 +1,10 @@ use aether_path::FilePath; -use tower_lsp::lsp_types; -use tower_lsp::lsp_types::Location; -use tower_lsp::lsp_types::ReferenceContext; -use tower_lsp::lsp_types::ReferenceParams; +use tower_lsp_server::ls_types as lsp_types; +use tower_lsp_server::ls_types::Location; +use tower_lsp_server::ls_types::ReferenceContext; +use tower_lsp_server::ls_types::ReferenceParams; +use tower_lsp_server::ls_types::Uri; +use url::Url; use super::utils::insert_file; use super::utils::make_state; @@ -10,15 +12,10 @@ use super::utils::range; use crate::lsp::find_references::find_references; use crate::lsp::util::test_path; -fn make_params( - uri: lsp_types::Url, - line: u32, - character: u32, - include_decl: bool, -) -> ReferenceParams { +fn make_params(uri: &Uri, line: u32, character: u32, include_decl: bool) -> ReferenceParams { ReferenceParams { text_document_position: lsp_types::TextDocumentPositionParams { - text_document: lsp_types::TextDocumentIdentifier { uri }, + text_document: lsp_types::TextDocumentIdentifier { uri: uri.clone() }, position: lsp_types::Position::new(line, character), }, context: ReferenceContext { @@ -29,18 +26,26 @@ fn make_params( } } +/// Build the `Location` we expect `find_references()` to return for `uri`. +fn location(uri: &Uri, range: lsp_types::Range) -> Location { + Location::new(uri.clone(), range) +} + +fn at(loc: &Location, uri: &Uri) -> bool { + &loc.uri == uri +} + #[test] fn test_intra_file_use_and_def() { let code = "foo <- 1\nfoo + foo\n"; - let uri = test_path("test.R"); - let state = make_state(&uri, code); + let (state, uri) = make_state(test_path("test.R").as_str(), code); - let params = make_params(uri.clone(), 1, 0, true); + let params = make_params(&uri, 1, 0, true); let locs = find_references(params, &state).unwrap(); let expected: Vec = vec![ - Location::new(uri.clone(), range((0, 0), (0, 3))), - Location::new(uri.clone(), range((1, 0), (1, 3))), - Location::new(uri, range((1, 6), (1, 9))), + location(&uri, range((0, 0), (0, 3))), + location(&uri, range((1, 0), (1, 3))), + location(&uri, range((1, 6), (1, 9))), ]; assert_eq!(locs, expected); } @@ -48,21 +53,19 @@ fn test_intra_file_use_and_def() { #[test] fn test_excludes_declaration() { let code = "foo <- 1\nfoo\n"; - let uri = test_path("test.R"); - let state = make_state(&uri, code); + let (state, uri) = make_state(test_path("test.R").as_str(), code); - let params = make_params(uri.clone(), 1, 0, false); + let params = make_params(&uri, 1, 0, false); let locs = find_references(params, &state).unwrap(); - assert_eq!(locs, vec![Location::new(uri, range((1, 0), (1, 3)))]); + assert_eq!(locs, vec![location(&uri, range((1, 0), (1, 3)))]); } #[test] fn test_no_identifier_returns_empty() { let code = "x <- 1\n"; - let uri = test_path("test.R"); - let state = make_state(&uri, code); + let (state, uri) = make_state(test_path("test.R").as_str(), code); - let params = make_params(uri, 0, 3, true); + let params = make_params(&uri, 0, 3, true); let locs = find_references(params, &state).unwrap(); assert!(locs.is_empty()); } @@ -71,15 +74,14 @@ fn test_no_identifier_returns_empty() { fn test_cursor_past_trailing_edge_resolves() { // Cursor at column 3: one past the last character of `foo` (0..3). let code = "foo <- 1\nfoo\n"; - let uri = test_path("trailing.R"); - let state = make_state(&uri, code); + let (state, uri) = make_state(test_path("trailing.R").as_str(), code); - let params = make_params(uri.clone(), 1, 3, true); + let params = make_params(&uri, 1, 3, true); let locs = find_references(params, &state).unwrap(); let expected: Vec = vec![ - Location::new(uri.clone(), range((0, 0), (0, 3))), - Location::new(uri, range((1, 0), (1, 3))), + location(&uri, range((0, 0), (0, 3))), + location(&uri, range((1, 0), (1, 3))), ]; assert_eq!(locs, expected); } @@ -88,10 +90,9 @@ fn test_cursor_past_trailing_edge_resolves() { fn test_unbound_symbol_returns_empty() { // `mutate` has no definition in the db: precise resolution gives nothing. let code = "mutate\n"; - let uri = test_path("a.R"); - let state = make_state(&uri, code); + let (state, uri) = make_state(test_path("a.R").as_str(), code); - let params = make_params(uri, 0, 0, true); + let params = make_params(&uri, 0, 0, true); let locs = find_references(params, &state).unwrap(); assert!(locs.is_empty()); } @@ -101,16 +102,14 @@ fn test_different_binding_not_included() { // file2 has its own `foo` binding. Cursor on file1's `foo` -- the // resolve_at confirm step excludes file2's `foo`. let code1 = "foo <- 1\nfoo\n"; - let uri1 = test_path("a.R"); - let mut state = make_state(&uri1, code1); + let (mut state, uri1) = make_state(test_path("a.R").as_str(), code1); let code2 = "foo <- 99\nfoo\n"; - let uri2 = test_path("b.R"); - insert_file(&mut state, &uri2, code2); + insert_file(&mut state, test_path("b.R").as_str(), code2); - let params = make_params(uri1.clone(), 0, 0, true); + let params = make_params(&uri1, 0, 0, true); let locs = find_references(params, &state).unwrap(); - assert!(locs.iter().all(|l| l.uri == uri1)); + assert!(locs.iter().all(|l| at(l, &uri1))); assert_eq!(locs.len(), 2); } @@ -118,21 +117,19 @@ fn test_different_binding_not_included() { fn test_function_scope_target_stays_in_file() { // Parameter `x` is function-scoped, so only file1 is searched. let code1 = "f <- function(x) {\n x + 1\n}\n"; - let uri1 = test_path("a.R"); - let mut state = make_state(&uri1, code1); + let (mut state, uri1) = make_state(test_path("a.R").as_str(), code1); let code2 = "x <- 99\nx\n"; - let uri2 = test_path("b.R"); - insert_file(&mut state, &uri2, code2); + insert_file(&mut state, test_path("b.R").as_str(), code2); // Cursor on the parameter `x` at line 0, column 14. - let params = make_params(uri1.clone(), 0, 14, true); + let params = make_params(&uri1, 0, 14, true); let locs = find_references(params, &state).unwrap(); // Only file1's parameter and its use. No file2 hits. let expected: Vec = vec![ - Location::new(uri1.clone(), range((0, 14), (0, 15))), - Location::new(uri1, range((1, 2), (1, 3))), + location(&uri1, range((0, 14), (0, 15))), + location(&uri1, range((1, 2), (1, 3))), ]; assert_eq!(locs, expected); } @@ -142,51 +139,59 @@ fn test_cross_file_dollar_kind() { // Cursor on `bar` in `foo$bar` -- structural member scan matches all `$bar` // occurrences regardless of LHS, and excludes plain `bar` (non-member). let code1 = "foo <- list()\nfoo$bar\n"; - let uri1 = test_path("a.R"); - let mut state = make_state(&uri1, code1); + let (mut state, uri1) = make_state(test_path("a.R").as_str(), code1); let code2 = "foo$bar\nbaz$bar\nbar\n"; - let uri2 = test_path("b.R"); - insert_file(&mut state, &uri2, code2); + let uri2 = insert_file(&mut state, test_path("b.R").as_str(), code2); // Cursor on `bar` (RHS of `$`) at line 1, col 4. - let params = make_params(uri1.clone(), 1, 4, true); + let params = make_params(&uri1, 1, 4, true); let locs = find_references(params, &state).unwrap(); // a.R's `foo$bar` member assert!(locs .iter() - .any(|l| l.uri == uri1 && l.range == range((1, 4), (1, 7)))); + .any(|l| at(l, &uri1) && l.range == range((1, 4), (1, 7)))); // b.R's `foo$bar` member (matches) assert!(locs .iter() - .any(|l| l.uri == uri2 && l.range == range((0, 4), (0, 7)))); + .any(|l| at(l, &uri2) && l.range == range((0, 4), (0, 7)))); // b.R's `baz$bar` member (matches -- LHS is irrelevant for member scan) assert!(locs .iter() - .any(|l| l.uri == uri2 && l.range == range((1, 4), (1, 7)))); + .any(|l| at(l, &uri2) && l.range == range((1, 4), (1, 7)))); // b.R's plain `bar` (Symbol kind, does NOT match) assert!(!locs .iter() - .any(|l| l.uri == uri2 && l.range == range((2, 0), (2, 3)))); + .any(|l| at(l, &uri2) && l.range == range((2, 0), (2, 3)))); } #[test] fn test_locations_use_verbatim_url() { - // The doubled slash survives in the editor's URL but `FilePath` normalises - // it away, so a path round-trip would change it. Locations must carry the - // exact URI the editor opened the buffer with. + // `FilePath` normalisation would collapse the doubled slash, decode and + // re-encode the percent-escaped drive colon with different casing, and + // leave the encoded brackets raw, so a `Url` round-trip changes all three + // wire strings below. Locations must carry the exact bytes the editor + // opened the buffer with. let code = "foo <- 1\nfoo\n"; - let uri = lsp_types::Url::parse("file:///C:/proj//foo.R").unwrap(); - let state = make_state(&uri, code); - - let params = make_params(uri.clone(), 1, 0, true); - let locs = find_references(params, &state).unwrap(); - - assert!(!locs.is_empty()); - assert!(locs.iter().all(|loc| loc.uri == uri)); - // Confirm the round-trip really would have differed, so the check above bites. - assert_ne!(FilePath::from_url(&uri).to_url(), uri); + for wire in [ + "file:///C:/proj//foo.R", + "file:///c%3A/proj/foo.R", + "file:///C:/proj/f%5B1%5D.R", + ] { + let (state, uri) = make_state(wire, code); + + let params = make_params(&uri, 1, 0, true); + let locs = find_references(params, &state).unwrap(); + + assert!(!locs.is_empty()); + assert!(locs.iter().all(|loc| loc.uri.as_str() == wire)); + + // Confirm the round-trip really would have differed, so the check + // above bites. + let url = Url::parse(wire).unwrap(); + assert_ne!(FilePath::from_url(&url).to_url(), url); + } } #[test] @@ -203,31 +208,29 @@ fn test_cross_file_namespace_access() { // can resolve-and-compare like the variable path (folding the structural // scan into it). A bare `mutate()` would then be a match here. let code1 = "dplyr::mutate\n"; - let uri1 = test_path("a.R"); - let mut state = make_state(&uri1, code1); + let (mut state, uri1) = make_state(test_path("a.R").as_str(), code1); let code2 = "dplyr::mutate\ntidyr::mutate\nmutate()\n"; - let uri2 = test_path("b.R"); - insert_file(&mut state, &uri2, code2); + let uri2 = insert_file(&mut state, test_path("b.R").as_str(), code2); // Cursor on `mutate` (RHS of `::`) at line 0, col 7. - let params = make_params(uri1.clone(), 0, 7, true); + let params = make_params(&uri1, 0, 7, true); let locs = find_references(params, &state).unwrap(); // a.R's `dplyr::mutate` assert!(locs .iter() - .any(|l| l.uri == uri1 && l.range == range((0, 7), (0, 13)))); + .any(|l| at(l, &uri1) && l.range == range((0, 7), (0, 13)))); // b.R's `dplyr::mutate` (matches) assert!(locs .iter() - .any(|l| l.uri == uri2 && l.range == range((0, 7), (0, 13)))); + .any(|l| at(l, &uri2) && l.range == range((0, 7), (0, 13)))); // b.R's `tidyr::mutate` (different namespace, does NOT match) assert!(!locs .iter() - .any(|l| l.uri == uri2 && l.range == range((1, 7), (1, 13)))); + .any(|l| at(l, &uri2) && l.range == range((1, 7), (1, 13)))); // b.R's bare `mutate()` (not a namespace access, does NOT match) TODO assert!(!locs .iter() - .any(|l| l.uri == uri2 && l.range == range((2, 0), (2, 6)))); + .any(|l| at(l, &uri2) && l.range == range((2, 0), (2, 6)))); } diff --git a/crates/ark/src/lsp/tests/goto_definition.rs b/crates/ark/src/lsp/tests/goto_definition.rs index 22dc5ad629..9abd5c68fe 100644 --- a/crates/ark/src/lsp/tests/goto_definition.rs +++ b/crates/ark/src/lsp/tests/goto_definition.rs @@ -6,9 +6,10 @@ use assert_matches::assert_matches; use oak_db::Db; use oak_db::OakDatabase; use oak_scan::DbScan; -use tower_lsp::lsp_types; -use tower_lsp::lsp_types::GotoDefinitionParams; -use tower_lsp::lsp_types::GotoDefinitionResponse; +use tower_lsp_server::ls_types as lsp_types; +use tower_lsp_server::ls_types::GotoDefinitionParams; +use tower_lsp_server::ls_types::GotoDefinitionResponse; +use tower_lsp_server::ls_types::Uri; use url::Url; use super::source_handler::TestBehavior; @@ -27,12 +28,13 @@ use crate::lsp::main_loop::GlobalState; use crate::lsp::main_loop::LspState; use crate::lsp::sources::SourceScheduler; use crate::lsp::state::WorldState; +use crate::lsp::traits::url::UrlExt; use crate::lsp::util::test_path; -fn make_params(uri: lsp_types::Url, line: u32, character: u32) -> GotoDefinitionParams { +fn make_params(uri: &Uri, line: u32, character: u32) -> GotoDefinitionParams { GotoDefinitionParams { text_document_position_params: lsp_types::TextDocumentPositionParams { - text_document: lsp_types::TextDocumentIdentifier { uri }, + text_document: lsp_types::TextDocumentIdentifier { uri: uri.clone() }, position: lsp_types::Position::new(line, character), }, work_done_progress_params: Default::default(), @@ -42,20 +44,19 @@ fn make_params(uri: lsp_types::Url, line: u32, character: u32) -> GotoDefinition /// A state with several open files, each mirrored into `oak` like `did_open` /// does, so `source()` targets resolve through `file_by_path`. -fn make_state_with(files: &[(&Url, &str)]) -> WorldState { +fn make_state_with(files: &[(&str, &str)]) -> WorldState { let mut state = WorldState::default(); - for (uri, code) in files { - insert_file(&mut state, uri, code); + for (wire, code) in files { + insert_file(&mut state, wire, code); } state } #[test] fn test_goto_definition() { - let uri = test_path("test.R"); - let state = make_state(&uri, "foo <- 42\nprint(foo)\n"); + let (state, uri) = make_state(test_path("test.R").as_str(), "foo <- 42\nprint(foo)\n"); - let params = make_params(uri, 1, 6); + let params = make_params(&uri, 1, 6); assert_matches!( goto_definition(params, &state).unwrap(), @@ -67,10 +68,9 @@ fn test_goto_definition() { #[test] fn test_goto_definition_prefers_local_symbol() { - let uri = test_path("file.R"); - let state = make_state(&uri, "foo <- 1\nfoo\n"); + let (state, uri) = make_state(test_path("file.R").as_str(), "foo <- 1\nfoo\n"); - let params = make_params(uri.clone(), 1, 0); + let params = make_params(&uri, 1, 0); assert_matches!( goto_definition(params, &state).unwrap(), @@ -83,41 +83,50 @@ fn test_goto_definition_prefers_local_symbol() { #[test] fn test_target_uri_is_verbatim() { - // The doubled slash normalises away in `FilePath`, so the target URI is - // only correct if it comes from the buffer's verbatim URL. - let uri = lsp_types::Url::parse("file:///C:/proj//file.R").unwrap(); - let state = make_state(&uri, "foo <- 1\nfoo\n"); - - let params = make_params(uri.clone(), 1, 0); - - assert_matches!( - goto_definition(params, &state).unwrap(), - Some(GotoDefinitionResponse::Link(ref links)) => { - assert_eq!(links[0].target_uri, uri); - } - ); - assert_ne!(FilePath::from_url(&uri).to_url(), uri); + // `FilePath` normalisation would collapse the doubled slash, decode and + // re-encode the percent-escaped drive colon with different casing, and + // leave the encoded brackets raw, so a `Url` round-trip changes all three + // wire strings below. The target URI is only correct if it comes from the + // buffer's verbatim URI. + for wire in [ + "file:///C:/proj//file.R", + "file:///c%3A/proj/file.R", + "file:///C:/proj/f%5B1%5D.R", + ] { + let (state, uri) = make_state(wire, "foo <- 1\nfoo\n"); + + let params = make_params(&uri, 1, 0); + + assert_matches!( + goto_definition(params, &state).unwrap(), + Some(GotoDefinitionResponse::Link(ref links)) => { + assert_eq!(links[0].target_uri.as_str(), wire); + } + ); + // Confirm the round-trip really would have differed, so the check + // above bites. + let url = Url::parse(wire).unwrap(); + assert_ne!(FilePath::from_url(&url).to_url(), url); + } } #[test] fn test_unbound_identifier_returns_none() { // A free identifier with no reachable binding returns `None`, matching how // rust-analyzer and ty handle the same case. - let uri = test_path("file.R"); - let state = make_state(&uri, "foo\n"); + let (state, uri) = make_state(test_path("file.R").as_str(), "foo\n"); - let params = make_params(uri, 0, 0); + let params = make_params(&uri, 0, 0); assert_eq!(goto_definition(params, &state).unwrap(), None); } #[test] fn test_cursor_on_operator_returns_none() { // Cursor on `<-`, not on an identifier use: nothing to resolve. - let uri = test_path("file.R"); - let state = make_state(&uri, "foo <- 1\n"); + let (state, uri) = make_state(test_path("file.R").as_str(), "foo <- 1\n"); // Cursor on the `<` of `<-` at column 4. - let params = make_params(uri, 0, 4); + let params = make_params(&uri, 0, 4); assert_eq!(goto_definition(params, &state).unwrap(), None); } @@ -126,11 +135,14 @@ fn test_unlinked_cross_file_returns_none() { // `foo` is defined in another open file, but this file doesn't `source()` // it, so R semantics can't reach it. goto-def is precise: it returns `None` // rather than guessing by name across the workspace like the legacy ark handler. - let uses_uri = test_path("uses.R"); - let defs_uri = test_path("defs.R"); - let state = make_state_with(&[(&uses_uri, "foo\n"), (&defs_uri, "foo <- function() 1\n")]); + let uses_wire = test_path("uses.R"); + let uses_uri: Uri = uses_wire.as_str().parse().unwrap(); + let state = make_state_with(&[ + (uses_wire.as_str(), "foo\n"), + (test_path("defs.R").as_str(), "foo <- function() 1\n"), + ]); - let params = make_params(uses_uri, 0, 0); + let params = make_params(&uses_uri, 0, 0); assert_eq!(goto_definition(params, &state).unwrap(), None); } @@ -141,14 +153,16 @@ fn test_resolves_across_source_directive() { // `definition_to_link` (the target file's own line index + URL). The // resolution itself is covered exhaustively by `oak_db`'s `file_resolve_at` // tests; this checks the goto-def wiring on top of it. - let script_uri = test_path("script.R"); - let helpers_uri = test_path("helpers.R"); + let script_wire = test_path("script.R"); + let helpers_wire = test_path("helpers.R"); + let helpers_uri: Uri = helpers_wire.as_str().parse().unwrap(); let state = make_state_with(&[ - (&script_uri, "source(\"helpers.R\")\nhelper\n"), - (&helpers_uri, "helper <- function() 1\n"), + (script_wire.as_str(), "source(\"helpers.R\")\nhelper\n"), + (helpers_wire.as_str(), "helper <- function() 1\n"), ]); - let params = make_params(script_uri, 1, 0); + let script_uri: Uri = script_wire.as_str().parse().unwrap(); + let params = make_params(&script_uri, 1, 0); assert_matches!( goto_definition(params, &state).unwrap(), Some(GotoDefinitionResponse::Link(ref links)) => { @@ -163,14 +177,18 @@ fn test_local_def_shadows_sourced() { // A local `<-` after a `source()` shadows the sourced binding, so the use // resolves to the local def (in this file), not the sourced one. The link // range must point at the local def. - let script_uri = test_path("script.R"); - let helpers_uri = test_path("helpers.R"); + let script_wire = test_path("script.R"); + let helpers_wire = test_path("helpers.R"); + let script_uri: Uri = script_wire.as_str().parse().unwrap(); let state = make_state_with(&[ - (&script_uri, "source(\"helpers.R\")\nfoo <- 1\nfoo\n"), - (&helpers_uri, "foo <- function() 2\n"), + ( + script_wire.as_str(), + "source(\"helpers.R\")\nfoo <- 1\nfoo\n", + ), + (helpers_wire.as_str(), "foo <- function() 2\n"), ]); - let params = make_params(script_uri.clone(), 2, 0); + let params = make_params(&script_uri, 2, 0); assert_matches!( goto_definition(params, &state).unwrap(), Some(GotoDefinitionResponse::Link(ref links)) => { @@ -186,14 +204,16 @@ fn test_sourced_file_with_repeated_def_offers_both() { // `if`/`else`, either arm could run, so goto-def offers both candidate // definitions, in definition order. Ranges are in the target file's // coordinates. - let script_uri = test_path("script.R"); - let helpers_uri = test_path("helpers.R"); + let script_wire = test_path("script.R"); + let helpers_wire = test_path("helpers.R"); + let helpers_uri: Uri = helpers_wire.as_str().parse().unwrap(); let state = make_state_with(&[ - (&script_uri, "source(\"helpers.R\")\nfn\n"), - (&helpers_uri, "if (cond) fn <- 1 else fn <- 2\n"), + (script_wire.as_str(), "source(\"helpers.R\")\nfn\n"), + (helpers_wire.as_str(), "if (cond) fn <- 1 else fn <- 2\n"), ]); - let params = make_params(script_uri, 1, 0); + let script_uri: Uri = script_wire.as_str().parse().unwrap(); + let params = make_params(&script_uri, 1, 0); assert_matches!( goto_definition(params, &state).unwrap(), Some(GotoDefinitionResponse::Link(ref links)) => { @@ -211,17 +231,19 @@ fn test_sourced_file_with_sequential_redef_offers_runtime_winner() { // When the sourced file binds the same name twice in sequence, the second // overwrites the first, so only the last binding is in effect when // `source()` finishes. goto-def offers just that one. - let script_uri = test_path("script.R"); - let helpers_uri = test_path("helpers.R"); + let script_wire = test_path("script.R"); + let helpers_wire = test_path("helpers.R"); + let helpers_uri: Uri = helpers_wire.as_str().parse().unwrap(); let state = make_state_with(&[ - (&script_uri, "source(\"helpers.R\")\nfn\n"), + (script_wire.as_str(), "source(\"helpers.R\")\nfn\n"), ( - &helpers_uri, + helpers_wire.as_str(), "fn <- function() 'first'\nfn <- function() 'second'\n", ), ]); - let params = make_params(script_uri, 1, 0); + let script_uri: Uri = script_wire.as_str().parse().unwrap(); + let params = make_params(&script_uri, 1, 0); assert_matches!( goto_definition(params, &state).unwrap(), Some(GotoDefinitionResponse::Link(ref links)) => { @@ -277,7 +299,7 @@ async fn test_goto_definition_resolves_unqualified_import_into_package() { .import("foopkg") .write(workspace.path()); let use_path = workspace.path().join("R").join("use.R"); - let use_uri = Url::from_file_path(&use_path).unwrap(); + let use_uri: Uri = Url::from_file_path(&use_path).unwrap().to_uri().unwrap(); write_sources(&workspace.path().join("R"), &[("use.R", "foo()\n")]); // Open the workspace folder, triggering a workspace scan @@ -289,10 +311,10 @@ async fn test_goto_definition_resolves_unqualified_import_into_package() { let foo_file = world.db.package_by_name("foopkg").unwrap().files(&world.db)[0]; assert_matches!( - goto_definition(make_params(use_uri, 0, 0), world).unwrap(), + goto_definition(make_params(&use_uri, 0, 0), world).unwrap(), Some(GotoDefinitionResponse::Link(ref links)) => { assert_eq!(links.len(), 1); - assert_eq!(links[0].target_uri, world.wire_url(foo_file)); + assert_eq!(links[0].target_uri, world.wire_uri(foo_file).unwrap()); assert_eq!(links[0].target_range, range((0, 0), (0, 3))); } ); @@ -343,7 +365,7 @@ async fn test_goto_definition_resolves_unqualified_import_from_into_package() { .import_from("barpkg", "bar") .write(workspace.path()); let use_path = workspace.path().join("R").join("use.R"); - let use_uri = Url::from_file_path(&use_path).unwrap(); + let use_uri: Uri = Url::from_file_path(&use_path).unwrap().to_uri().unwrap(); write_sources(&workspace.path().join("R"), &[("use.R", "bar()\n")]); // Open the workspace folder, triggering a workspace scan @@ -355,10 +377,10 @@ async fn test_goto_definition_resolves_unqualified_import_from_into_package() { let bar_file = world.db.package_by_name("barpkg").unwrap().files(&world.db)[0]; assert_matches!( - goto_definition(make_params(use_uri, 0, 0), world).unwrap(), + goto_definition(make_params(&use_uri, 0, 0), world).unwrap(), Some(GotoDefinitionResponse::Link(ref links)) => { assert_eq!(links.len(), 1); - assert_eq!(links[0].target_uri, world.wire_url(bar_file)); + assert_eq!(links[0].target_uri, world.wire_uri(bar_file).unwrap()); assert_eq!(links[0].target_range, range((0, 0), (0, 3))); } ); @@ -405,7 +427,7 @@ async fn test_goto_definition_resolves_namespace_accesses() { // Workspace usage pkg functions let workspace = tempfile::tempdir().unwrap(); let use_path = workspace.path().join("R").join("use.R"); - let use_uri = Url::from_file_path(&use_path).unwrap(); + let use_uri: Uri = Url::from_file_path(&use_path).unwrap().to_uri().unwrap(); write_sources(&workspace.path().join("R"), &[( "use.R", "pkg::foo()\npkg:::bar()\npkg:::foo()\npkg::bar()\n", @@ -421,37 +443,37 @@ async fn test_goto_definition_resolves_namespace_accesses() { // `pkg::f<@>oo()`, exported `foo` reached through `::` assert_matches!( - goto_definition(make_params(use_uri.clone(), 0, 6), world).unwrap(), + goto_definition(make_params(&use_uri, 0, 6), world).unwrap(), Some(GotoDefinitionResponse::Link(ref links)) => { assert_eq!(links.len(), 1); - assert_eq!(links[0].target_uri, world.wire_url(file)); + assert_eq!(links[0].target_uri, world.wire_uri(file).unwrap()); assert_eq!(links[0].target_range, range((0, 0), (0, 3))); } ); // `pkg:::b<@>ar()`, internal `bar` reached through `:::` assert_matches!( - goto_definition(make_params(use_uri.clone(), 1, 7), world).unwrap(), + goto_definition(make_params(&use_uri, 1, 7), world).unwrap(), Some(GotoDefinitionResponse::Link(ref links)) => { assert_eq!(links.len(), 1); - assert_eq!(links[0].target_uri, world.wire_url(file)); + assert_eq!(links[0].target_uri, world.wire_uri(file).unwrap()); assert_eq!(links[0].target_range, range((1, 0), (1, 3))); } ); // `pkg:::f<@>oo()`, `:::` also reaches an exported binding assert_matches!( - goto_definition(make_params(use_uri.clone(), 2, 7), world).unwrap(), + goto_definition(make_params(&use_uri, 2, 7), world).unwrap(), Some(GotoDefinitionResponse::Link(ref links)) => { assert_eq!(links.len(), 1); - assert_eq!(links[0].target_uri, world.wire_url(file)); + assert_eq!(links[0].target_uri, world.wire_uri(file).unwrap()); assert_eq!(links[0].target_range, range((0, 0), (0, 3))); } ); // `pkg::b<@>ar()`, `::` can't reach the unexported `bar` assert_eq!( - goto_definition(make_params(use_uri, 3, 6), world).unwrap(), + goto_definition(make_params(&use_uri, 3, 6), world).unwrap(), None ); } diff --git a/crates/ark/src/lsp/tests/main_loop.rs b/crates/ark/src/lsp/tests/main_loop.rs index acdd39d010..cc80bb87ca 100644 --- a/crates/ark/src/lsp/tests/main_loop.rs +++ b/crates/ark/src/lsp/tests/main_loop.rs @@ -9,10 +9,10 @@ use oak_db::DbInputs; use oak_db::OakDatabase; -use tower_lsp::lsp_types::DidChangeWorkspaceFoldersParams; -use tower_lsp::lsp_types::WorkspaceFolder; -use tower_lsp::lsp_types::WorkspaceFoldersChangeEvent; -use url::Url; +use tower_lsp_server::ls_types::DidChangeWorkspaceFoldersParams; +use tower_lsp_server::ls_types::Uri; +use tower_lsp_server::ls_types::WorkspaceFolder; +use tower_lsp_server::ls_types::WorkspaceFoldersChangeEvent; use super::utils::test_client; use super::utils::write_sources; @@ -52,7 +52,7 @@ async fn test_workspace_folder_scan_drives_through_main_loop() { let params = DidChangeWorkspaceFoldersParams { event: WorkspaceFoldersChangeEvent { added: vec![WorkspaceFolder { - uri: Url::from_file_path(tmp.path()).unwrap(), + uri: Uri::from_file_path(tmp.path()).unwrap(), name: String::new(), }], removed: vec![], diff --git a/crates/ark/src/lsp/tests/rename.rs b/crates/ark/src/lsp/tests/rename.rs index 46bb940955..94afb9c4c0 100644 --- a/crates/ark/src/lsp/tests/rename.rs +++ b/crates/ark/src/lsp/tests/rename.rs @@ -4,39 +4,33 @@ use oak_db::DbInputs; use oak_db::Root; use oak_db::RootKind; use salsa::Setter; -use tower_lsp::lsp_types; -use tower_lsp::lsp_types::PrepareRenameResponse; -use tower_lsp::lsp_types::RenameParams; -use tower_lsp::lsp_types::TextDocumentPositionParams; -use tower_lsp::lsp_types::TextEdit; +use tower_lsp_server::ls_types as lsp_types; +use tower_lsp_server::ls_types::PrepareRenameResponse; +use tower_lsp_server::ls_types::RenameParams; +use tower_lsp_server::ls_types::TextDocumentPositionParams; +use tower_lsp_server::ls_types::TextEdit; +use tower_lsp_server::ls_types::Uri; +use url::Url; use super::utils::insert_file; use super::utils::make_state; use super::utils::range; use crate::lsp::rename::prepare_rename; use crate::lsp::rename::rename; +use crate::lsp::traits::url::UriExt; use crate::lsp::util::test_path; -fn make_prepare_params( - uri: lsp_types::Url, - line: u32, - character: u32, -) -> TextDocumentPositionParams { +fn make_prepare_params(uri: &Uri, line: u32, character: u32) -> TextDocumentPositionParams { TextDocumentPositionParams { - text_document: lsp_types::TextDocumentIdentifier { uri }, + text_document: lsp_types::TextDocumentIdentifier { uri: uri.clone() }, position: lsp_types::Position::new(line, character), } } -fn make_rename_params( - uri: lsp_types::Url, - line: u32, - character: u32, - new_name: &str, -) -> RenameParams { +fn make_rename_params(uri: &Uri, line: u32, character: u32, new_name: &str) -> RenameParams { RenameParams { text_document_position: TextDocumentPositionParams { - text_document: lsp_types::TextDocumentIdentifier { uri }, + text_document: lsp_types::TextDocumentIdentifier { uri: uri.clone() }, position: lsp_types::Position::new(line, character), }, new_name: new_name.to_string(), @@ -47,10 +41,9 @@ fn make_rename_params( #[test] fn test_prepare_rename_returns_range_and_placeholder() { let code = "foo <- 1\nfoo\n"; - let uri = test_path("test.R"); - let state = make_state(&uri, code); + let (state, uri) = make_state(test_path("test.R").as_str(), code); - let params = make_prepare_params(uri, 0, 0); + let params = make_prepare_params(&uri, 0, 0); let result = prepare_rename(params, &state).unwrap().unwrap(); let PrepareRenameResponse::RangeWithPlaceholder { @@ -67,36 +60,47 @@ fn test_prepare_rename_returns_range_and_placeholder() { #[test] fn test_prepare_rename_on_namespace_access_returns_none() { let code = "dplyr::mutate\n"; - let uri = test_path("test.R"); - let state = make_state(&uri, code); + let (state, uri) = make_state(test_path("test.R").as_str(), code); - let params = make_prepare_params(uri, 0, 7); + let params = make_prepare_params(&uri, 0, 7); assert!(prepare_rename(params, &state).unwrap().is_none()); } #[test] fn test_edits_keyed_by_verbatim_url() { - // The `WorkspaceEdit` must key its changes on the buffer's verbatim URL, + // The `WorkspaceEdit` must key its changes on the buffer's verbatim URI, // not a normalised round-trip, or the editor won't match them to the file. + // `FilePath` normalisation would collapse the doubled slash, decode and + // re-encode the percent-escaped drive colon with different casing, and + // leave the encoded brackets raw, so a `Url` round-trip changes all three + // wire strings below. let code = "foo <- 1\nfoo\n"; - let uri = lsp_types::Url::parse("file:///C:/proj//file.R").unwrap(); - let state = make_state(&uri, code); - - let params = make_rename_params(uri.clone(), 0, 0, "bar"); - let edit = rename(params, &state).unwrap().unwrap(); - - let changes = edit.changes.expect("changes map"); - assert!(changes.contains_key(&uri)); - assert_ne!(FilePath::from_url(&uri).to_url(), uri); + for wire in [ + "file:///C:/proj//file.R", + "file:///c%3A/proj/file.R", + "file:///C:/proj/f%5B1%5D.R", + ] { + let (state, uri) = make_state(wire, code); + + let params = make_rename_params(&uri, 0, 0, "bar"); + let edit = rename(params, &state).unwrap().unwrap(); + + let changes = edit.changes.expect("changes map"); + assert!(changes.keys().any(|key| key.as_str() == wire)); + + // Confirm the round-trip really would have differed, so the check + // above bites. + let url = Url::parse(wire).unwrap(); + assert_ne!(FilePath::from_url(&url).to_url(), url); + } } #[test] fn test_rename_emits_edits_for_def_and_uses() { let code = "foo <- 1\nfoo + foo\n"; - let uri = test_path("test.R"); - let state = make_state(&uri, code); + let (state, uri) = make_state(test_path("test.R").as_str(), code); - let params = make_rename_params(uri.clone(), 0, 0, "bar"); + let params = make_rename_params(&uri, 0, 0, "bar"); let edit = rename(params, &state).unwrap().unwrap(); let changes = edit.changes.expect("changes map"); @@ -125,16 +129,14 @@ fn test_rename_excludes_independent_binding_in_other_file() { // file2 has its own separate `foo` -- rename of file1's `foo` should // NOT touch file2's independent binding. let code1 = "foo <- 1\nfoo\n"; - let uri1 = test_path("a.R"); - let mut state = make_state(&uri1, code1); + let (mut state, uri1) = make_state(test_path("a.R").as_str(), code1); // file2 has its own separate `foo` -- rename of file1's `foo` should // NOT touch file2's independent binding. let code2 = "foo <- 99\nfoo\n"; - let uri2 = test_path("b.R"); - insert_file(&mut state, &uri2, code2); + let uri2 = insert_file(&mut state, test_path("b.R").as_str(), code2); - let params = make_rename_params(uri1.clone(), 0, 0, "bar"); + let params = make_rename_params(&uri1, 0, 0, "bar"); let edit = rename(params, &state).unwrap().unwrap(); let changes = edit.changes.expect("changes map"); @@ -149,18 +151,16 @@ fn test_rename_cross_file_via_source() { // helpers.R defines `helper`; script.R sources it and uses it. // After registering both in a workspace root, rename spans both files. let code1 = "helper <- function() 1\n"; - let uri1 = test_path("helpers.R"); - let mut state = make_state(&uri1, code1); + let (mut state, uri1) = make_state(test_path("helpers.R").as_str(), code1); let code2 = "source(\"helpers.R\")\nhelper\n"; - let uri2 = test_path("script.R"); - insert_file(&mut state, &uri2, code2); + let uri2 = insert_file(&mut state, test_path("script.R").as_str(), code2); // Register both files in a workspace root whose path is the temp // directory. `anchor_dir` uses the root path as the anchor, so // `source("helpers.R")` resolves to `/helpers.R`. - let fp1 = FilePath::from_url(&uri1); - let fp2 = FilePath::from_url(&uri2); + let fp1 = FilePath::from_url(&uri1.to_url().unwrap()); + let fp2 = FilePath::from_url(&uri2.to_url().unwrap()); let file1 = state.db.file_by_path(&fp1).unwrap(); let file2 = state.db.file_by_path(&fp2).unwrap(); let root_path = FilePath::from_path_buf(std::env::temp_dir()).unwrap(); @@ -178,7 +178,7 @@ fn test_rename_cross_file_via_source() { .to(vec![root]); // Cursor on `helper` use in script.R (line 1, col 0). - let params = make_rename_params(uri2.clone(), 1, 0, "renamed"); + let params = make_rename_params(&uri2, 1, 0, "renamed"); let edit = rename(params, &state).unwrap().unwrap(); let changes = edit.changes.expect("changes map"); @@ -202,10 +202,9 @@ fn test_rename_cross_file_via_source() { #[test] fn test_rename_to_reserved_word_errors() { let code = "foo <- 1\n"; - let uri = test_path("test.R"); - let state = make_state(&uri, code); + let (state, uri) = make_state(test_path("test.R").as_str(), code); - let params = make_rename_params(uri, 0, 0, "if"); + let params = make_rename_params(&uri, 0, 0, "if"); let err = rename(params, &state).unwrap_err(); assert!(err.to_string().contains("reserved")); } @@ -213,10 +212,9 @@ fn test_rename_to_reserved_word_errors() { #[test] fn test_rename_to_name_with_space_wraps_in_backticks() { let code = "foo <- 1\nfoo\n"; - let uri = test_path("test.R"); - let state = make_state(&uri, code); + let (state, uri) = make_state(test_path("test.R").as_str(), code); - let params = make_rename_params(uri.clone(), 0, 0, "new name"); + let params = make_rename_params(&uri, 0, 0, "new name"); let edit = rename(params, &state).unwrap().unwrap(); let edits = edit.changes.unwrap().remove(&uri).unwrap(); assert!(edits.iter().all(|e| e.new_text == "`new name`")); @@ -225,10 +223,9 @@ fn test_rename_to_name_with_space_wraps_in_backticks() { #[test] fn test_rename_to_name_with_starting_digit_wraps_in_backticks() { let code = "foo <- 1\nfoo\n"; - let uri = test_path("test.R"); - let state = make_state(&uri, code); + let (state, uri) = make_state(test_path("test.R").as_str(), code); - let params = make_rename_params(uri.clone(), 0, 0, "1foo"); + let params = make_rename_params(&uri, 0, 0, "1foo"); let edit = rename(params, &state).unwrap().unwrap(); let edits = edit.changes.unwrap().remove(&uri).unwrap(); assert!(edits.iter().all(|e| e.new_text == "`1foo`")); @@ -237,10 +234,9 @@ fn test_rename_to_name_with_starting_digit_wraps_in_backticks() { #[test] fn test_rename_to_empty_name_errors() { let code = "foo <- 1\n"; - let uri = test_path("test.R"); - let state = make_state(&uri, code); + let (state, uri) = make_state(test_path("test.R").as_str(), code); - let params = make_rename_params(uri, 0, 0, ""); + let params = make_rename_params(&uri, 0, 0, ""); let err = rename(params, &state).unwrap_err(); assert!(err.to_string().contains("empty")); } @@ -248,10 +244,9 @@ fn test_rename_to_empty_name_errors() { #[test] fn test_rename_to_name_with_backtick_errors() { let code = "foo <- 1\n"; - let uri = test_path("test.R"); - let state = make_state(&uri, code); + let (state, uri) = make_state(test_path("test.R").as_str(), code); - let params = make_rename_params(uri, 0, 0, "foo`bar"); + let params = make_rename_params(&uri, 0, 0, "foo`bar"); let err = rename(params, &state).unwrap_err(); assert!(err.to_string().contains("backtick")); } diff --git a/crates/ark/src/lsp/tests/state.rs b/crates/ark/src/lsp/tests/state.rs index 617775c1b0..0e7c4a9b62 100644 --- a/crates/ark/src/lsp/tests/state.rs +++ b/crates/ark/src/lsp/tests/state.rs @@ -1,29 +1,77 @@ use aether_path::FilePath; +use oak_db::File; use oak_scan::DbScan; +use tower_lsp_server::ls_types::Uri; use url::Url; use crate::lsp::state::WorldState; +use crate::lsp::traits::url::UriExt; -#[test] -fn test_wire_url_open_buffer_keeps_verbatim_url() { - let mut state = WorldState::default(); - let url = Url::parse("file:///C:/proj//foo.R").unwrap(); +/// Register an open buffer the way `did_open` does, starting from the bytes an +/// editor would put on the wire rather than from a `Url`. Starting from a `Url` +/// would hide any normalisation the `Uri` -> `Url` conversion applies. +fn open_buffer_from_wire(state: &mut WorldState, wire: &str) -> File { + let uri: Uri = wire.parse().unwrap(); + let url = uri.to_url().unwrap(); let file = state .db .upsert_editor(FilePath::from_url(&url), "x <- 1\n".to_string()); - state.insert_open_file(url.clone(), file, None); - assert_eq!(state.wire_url(file), url); + state.insert_open_file(uri, FilePath::from_url(&url), file, None); + file } #[test] -fn test_wire_url_non_open_file_synthesises_url() { +fn test_wire_uri_non_open_file_synthesises_uri() { let mut state = WorldState::default(); let url = Url::parse("file:///C:/proj//bar.R").unwrap(); let file = state .db .upsert_editor(FilePath::from_url(&url), "y <- 2\n".to_string()); - // Not inserted into open_files, so wire_url synthesises from path - let wire = state.wire_url(file); - assert_eq!(wire, file.path(&state.db).to_url()); - assert_ne!(wire, url); + // Not inserted into open_files, so wire_uri synthesises from the + // normalised path (dropping the doubled slash) and encodes the drive + // colon the way `Uri` requires, rather than replaying the original bytes. + let wire = state.wire_uri(file).unwrap(); + assert_eq!(wire.as_str(), "file:///C%3A/proj/bar.R"); + assert_ne!(wire.as_str(), url.as_str()); +} + +#[test] +fn test_wire_uri_open_buffer_replays_editor_bytes() { + // The editor matches a response to its document by URI, so what we send + // back has to be byte-identical to what it sent us. Each of these survives + // `FilePath` normalisation only because the buffer stashes the verbatim URI. + for wire in [ + "file:///C:/proj//foo.R", // doubled slash + "file:///c%3A/proj/foo.R", // percent-encoded drive colon + "file:///C:/proj/a%20b.R", // encoded space + "file:///C:/proj/f%5B1%5D.R", // encoded brackets + "untitled:Untitled-1", + ] { + let mut state = WorldState::default(); + let file = open_buffer_from_wire(&mut state, wire); + assert_eq!(state.wire_uri(file).unwrap().as_str(), wire); + } +} + +#[test] +fn test_wire_uri_non_open_file_encodes_reserved_characters() { + // `[` and `]` are legal in a filename but reserved in a URI. `Url` leaves + // them raw in a path, so a file like `f[1].R` that the editor never opened + // has no `Url`-based route we could turn into a `Uri` at all. + let mut state = WorldState::default(); + let path = FilePath::parse("file:///C:/proj/f%5B1%5D.R").unwrap(); + let file = state.db.upsert_editor(path, "y <- 2\n".to_string()); + + // Not open, so this takes the `Uri::from_file_path()` fallback. + let uri = state.wire_uri(file).unwrap(); + assert_eq!(uri.as_str(), "file:///C%3A/proj/f%5B1%5D.R"); + + // Confirm a `Url`-based route really would have failed, so the check + // above bites. + let disk_path = file.path(&state.db).as_path().unwrap(); + assert!(Url::from_file_path(disk_path) + .unwrap() + .as_str() + .parse::() + .is_err()); } diff --git a/crates/ark/src/lsp/tests/state_handlers.rs b/crates/ark/src/lsp/tests/state_handlers.rs index ef8e813119..b1eaa4c0f0 100644 --- a/crates/ark/src/lsp/tests/state_handlers.rs +++ b/crates/ark/src/lsp/tests/state_handlers.rs @@ -9,21 +9,23 @@ use std::fs; use std::path::Path; use std::path::PathBuf; +use aether_path::AbsPathBuf; use aether_path::FilePath; use oak_db::Db; use oak_db::DbInputs; use oak_scan::DbScan; use oak_scan::ScanRequest; use oak_scan::ScanScheduler; -use tower_lsp::lsp_types::DidChangeWatchedFilesParams; -use tower_lsp::lsp_types::DidChangeWorkspaceFoldersParams; -use tower_lsp::lsp_types::DidCloseTextDocumentParams; -use tower_lsp::lsp_types::FileChangeType; -use tower_lsp::lsp_types::FileEvent; -use tower_lsp::lsp_types::InitializeParams; -use tower_lsp::lsp_types::TextDocumentIdentifier; -use tower_lsp::lsp_types::WorkspaceFolder; -use tower_lsp::lsp_types::WorkspaceFoldersChangeEvent; +use tower_lsp_server::ls_types::DidChangeWatchedFilesParams; +use tower_lsp_server::ls_types::DidChangeWorkspaceFoldersParams; +use tower_lsp_server::ls_types::DidCloseTextDocumentParams; +use tower_lsp_server::ls_types::FileChangeType; +use tower_lsp_server::ls_types::FileEvent; +use tower_lsp_server::ls_types::InitializeParams; +use tower_lsp_server::ls_types::TextDocumentIdentifier; +use tower_lsp_server::ls_types::Uri; +use tower_lsp_server::ls_types::WorkspaceFolder; +use tower_lsp_server::ls_types::WorkspaceFoldersChangeEvent; use url::Url; use crate::lsp::main_loop::dispatch_scan_requests; @@ -36,8 +38,9 @@ use crate::lsp::sources::SourceScheduler; use crate::lsp::state::WorldState; use crate::lsp::state_handlers::did_change_console_inputs; use crate::lsp::state_handlers::did_close; -use crate::lsp::state_handlers::effective_workspace_uris; +use crate::lsp::state_handlers::effective_workspace_paths; use crate::lsp::state_handlers::ConsoleInputs; +use crate::lsp::traits::url::UrlExt; /// Local sync wrappers around the async-shaped scheduler API. Tests /// don't need the timing flexibility, so each operation kicks off @@ -163,7 +166,7 @@ fn write_package(dir: &Path, name: &str, r_files: &[(&str, &str)]) { fn event(path: &Path, typ: FileChangeType) -> FileEvent { FileEvent { - uri: Url::from_file_path(path).unwrap(), + uri: Uri::from_file_path(path).unwrap(), typ, } } @@ -173,7 +176,7 @@ fn workspace_state(workspace: &Path) -> WorldState { state .workspace .folders - .push(Url::from_file_path(workspace).unwrap()); + .push(AbsPathBuf::from_path_buf(workspace.to_path_buf()).unwrap()); set_workspace_paths(&mut state, &[workspace.to_path_buf()], &HashSet::new()); state } @@ -271,7 +274,7 @@ fn test_r_file_changed_for_editor_open_file_is_skipped() { let file = state .db .upsert_editor(file_path.clone(), "editor_v2\n".to_string()); - state.insert_open_file(url.clone(), file, None); + state.insert_open_file(url.to_uri().unwrap(), FilePath::from_url(&url), file, None); // Now disk-side `Changed` fires with stale disk content. fs::write(&path, "disk_v3\n").unwrap(); @@ -357,7 +360,7 @@ fn test_r_file_deleted_for_editor_open_file_is_skipped() { let file = state .db .upsert_editor(file_path.clone(), "editor_v2\n".to_string()); - state.insert_open_file(url.clone(), file, None); + state.insert_open_file(url.to_uri().unwrap(), FilePath::from_url(&url), file, None); fs::remove_file(&path).unwrap(); let params = DidChangeWatchedFilesParams { @@ -402,51 +405,57 @@ fn test_description_deleted_demotes_package_to_scripts() { fn folder(uri: &str) -> WorkspaceFolder { WorkspaceFolder { - uri: Url::parse(uri).unwrap(), + uri: uri.parse().unwrap(), name: String::new(), } } #[test] -fn test_effective_workspace_uris_reads_workspace_folders() { +fn test_effective_workspace_paths_reads_workspace_folders() { + // Drive-letter URIs, because a folder has to survive as an `AbsPathBuf`. + // `file:///a` has no drive so it isn't an absolute path on Windows, and + // would be dropped there. let params = InitializeParams { - workspace_folders: Some(vec![folder("file:///a"), folder("file:///b")]), + workspace_folders: Some(vec![folder("file:///C:/a"), folder("file:///C:/b")]), ..Default::default() }; - let uris = effective_workspace_uris(¶ms); - assert_eq!(uris.len(), 2); - assert_eq!(uris[0].as_str(), "file:///a"); - assert_eq!(uris[1].as_str(), "file:///b"); + let paths = effective_workspace_paths(¶ms); + assert_eq!(paths.len(), 2); + assert_eq!(paths[0].as_path().file_name(), Some("a")); + assert_eq!(paths[1].as_path().file_name(), Some("b")); } +// `root_uri` is deprecated in the protocol, but that's the point of these tests +#[allow(deprecated)] #[test] -fn test_effective_workspace_uris_ignores_legacy_root_uri() { +fn test_effective_workspace_paths_ignores_legacy_root_uri() { // We dropped the `root_uri` fallback, so a client sending only the // deprecated field gets single-file mode (empty), whether // `workspace_folders` is absent or an empty list. let absent = InitializeParams { workspace_folders: None, - root_uri: Some(Url::parse("file:///legacy").unwrap()), + root_uri: Some("file:///legacy".parse().unwrap()), ..Default::default() }; - assert!(effective_workspace_uris(&absent).is_empty()); + assert!(effective_workspace_paths(&absent).is_empty()); let empty = InitializeParams { workspace_folders: Some(vec![]), - root_uri: Some(Url::parse("file:///legacy").unwrap()), + root_uri: Some("file:///legacy".parse().unwrap()), ..Default::default() }; - assert!(effective_workspace_uris(&empty).is_empty()); + assert!(effective_workspace_paths(&empty).is_empty()); } +#[allow(deprecated)] #[test] -fn test_effective_workspace_uris_single_file_mode() { +fn test_effective_workspace_paths_single_file_mode() { let params = InitializeParams { workspace_folders: None, root_uri: None, ..Default::default() }; - assert!(effective_workspace_uris(¶ms).is_empty()); + assert!(effective_workspace_paths(¶ms).is_empty()); } fn folders_change( @@ -460,7 +469,7 @@ fn folders_change( fn folder_for(path: &Path) -> WorkspaceFolder { WorkspaceFolder { - uri: Url::from_file_path(path).unwrap(), + uri: Uri::from_file_path(path).unwrap(), name: String::new(), } } @@ -498,11 +507,11 @@ fn test_did_change_workspace_folders_removes_folder() { state .workspace .folders - .push(Url::from_file_path(first.path()).unwrap()); + .push(AbsPathBuf::from_path_buf(first.path().to_path_buf()).unwrap()); state .workspace .folders - .push(Url::from_file_path(second.path()).unwrap()); + .push(AbsPathBuf::from_path_buf(second.path().to_path_buf()).unwrap()); set_workspace_paths( &mut state, &[first.path().to_path_buf(), second.path().to_path_buf()], @@ -574,7 +583,7 @@ fn test_did_change_workspace_folders_preserves_open_buffer_across_churn() { let file = state .db .upsert_editor(file_path.clone(), "editor <- 2\n".to_string()); - state.insert_open_file(url.clone(), file, None); + state.insert_open_file(url.to_uri().unwrap(), FilePath::from_url(&url), file, None); let file_before = state.db.file_by_path(&file_path).unwrap(); @@ -631,7 +640,7 @@ fn test_did_close_releases_orphan_file_to_stale() { let file = state .db .upsert_editor(file_path.clone(), "edited\n".to_string()); - state.insert_open_file(url.clone(), file, None); + state.insert_open_file(url.to_uri().unwrap(), FilePath::from_url(&url), file, None); // Remove the workspace folder; file goes to orphan (editor-owned). did_change_workspace_folders( @@ -649,7 +658,9 @@ fn test_did_close_releases_orphan_file_to_stale() { // Now close the buffer. File should move from orphan to stale. let params = DidCloseTextDocumentParams { - text_document: TextDocumentIdentifier { uri: url.clone() }, + text_document: TextDocumentIdentifier { + uri: url.to_uri().unwrap(), + }, }; did_close(params, &mut state).unwrap(); @@ -658,10 +669,11 @@ fn test_did_close_releases_orphan_file_to_stale() { // did_close() clears diagnostics for the closed file. let event = aux_rx.try_recv().unwrap(); - assert!(matches!( - event, - AuxiliaryEvent::PublishDiagnostics(u, diags, _) if u == url && diags.is_empty() - )); + let AuxiliaryEvent::PublishDiagnostics(publication) = event else { + panic!("expected a PublishDiagnostics event"); + }; + assert_eq!(publication.path, FilePath::from_url(&url)); + assert!(publication.diagnostics.is_empty()); } /// A console-inputs push carries no oak write, but diagnostics read the console diff --git a/crates/ark/src/lsp/tests/utils/events.rs b/crates/ark/src/lsp/tests/utils/events.rs index c075339dc8..bda4ea0b74 100644 --- a/crates/ark/src/lsp/tests/utils/events.rs +++ b/crates/ark/src/lsp/tests/utils/events.rs @@ -1,11 +1,11 @@ use std::path::Path; -use tower_lsp::lsp_types::DidChangeWorkspaceFoldersParams; -use tower_lsp::lsp_types::DidOpenTextDocumentParams; -use tower_lsp::lsp_types::TextDocumentItem; -use tower_lsp::lsp_types::WorkspaceFolder; -use tower_lsp::lsp_types::WorkspaceFoldersChangeEvent; -use url::Url; +use tower_lsp_server::ls_types::DidChangeWorkspaceFoldersParams; +use tower_lsp_server::ls_types::DidOpenTextDocumentParams; +use tower_lsp_server::ls_types::TextDocumentItem; +use tower_lsp_server::ls_types::Uri; +use tower_lsp_server::ls_types::WorkspaceFolder; +use tower_lsp_server::ls_types::WorkspaceFoldersChangeEvent; use crate::lsp::backend::LspMessage; use crate::lsp::backend::LspNotification; @@ -16,7 +16,7 @@ pub(crate) fn did_change_workspace_folders(path: &Path) -> Event { LspNotification::DidChangeWorkspaceFolders(DidChangeWorkspaceFoldersParams { event: WorkspaceFoldersChangeEvent { added: vec![WorkspaceFolder { - uri: Url::from_file_path(path).unwrap(), + uri: Uri::from_file_path(path).unwrap(), name: String::new(), }], removed: vec![], @@ -29,7 +29,7 @@ pub(crate) fn did_open(path: &Path, contents: &str) -> Event { Event::Lsp(LspMessage::Notification( LspNotification::DidOpenTextDocument(DidOpenTextDocumentParams { text_document: TextDocumentItem { - uri: Url::from_file_path(path).unwrap(), + uri: Uri::from_file_path(path).unwrap(), language_id: String::from("r"), version: 0, text: contents.to_string(), diff --git a/crates/ark/src/lsp/tests/utils/mod.rs b/crates/ark/src/lsp/tests/utils/mod.rs index 9eb59fb087..8aa2c1d494 100644 --- a/crates/ark/src/lsp/tests/utils/mod.rs +++ b/crates/ark/src/lsp/tests/utils/mod.rs @@ -10,12 +10,14 @@ pub(super) use events::did_change_workspace_folders; pub(super) use events::did_open; pub(super) use namespace_writer::NamespaceWriter; use oak_scan::DbScan; -use tower_lsp::lsp_types; -use tower_lsp::Client; -use tower_lsp::LanguageServer; -use tower_lsp::LspService; +use tower_lsp_server::ls_types as lsp_types; +use tower_lsp_server::ls_types::Uri; +use tower_lsp_server::Client; +use tower_lsp_server::LanguageServer; +use tower_lsp_server::LspService; use crate::lsp::state::WorldState; +use crate::lsp::traits::url::UriExt; /// Get a real `Client` without a live connection. `LspService::new` hands a /// `Client` to its init closure; we capture it and drop the service. The @@ -24,15 +26,14 @@ use crate::lsp::state::WorldState; pub(super) fn test_client() -> Client { struct Dummy; - #[tower_lsp::async_trait] impl LanguageServer for Dummy { async fn initialize( &self, _: lsp_types::InitializeParams, - ) -> tower_lsp::jsonrpc::Result { + ) -> tower_lsp_server::jsonrpc::Result { Ok(lsp_types::InitializeResult::default()) } - async fn shutdown(&self) -> tower_lsp::jsonrpc::Result<()> { + async fn shutdown(&self) -> tower_lsp_server::jsonrpc::Result<()> { Ok(()) } } @@ -57,20 +58,27 @@ pub(super) fn write_sources(dir: &Path, files: &[(&str, &str)]) { } } -pub(super) fn make_state(uri: &lsp_types::Url, contents: &str) -> WorldState { +pub(super) fn make_state(wire: &str, contents: &str) -> (WorldState, Uri) { let mut state = WorldState::default(); - insert_file(&mut state, uri, contents); - state + let uri = insert_file(&mut state, wire, contents); + (state, uri) } /// Insert an editor buffer, the same as `did_open` performs, so handlers /// reading either `state.documents` or `state.db` (via `file_by_path`) see a /// consistent file. -pub(super) fn insert_file(state: &mut WorldState, uri: &lsp_types::Url, contents: &str) { +/// +/// Starts from `wire`, the raw bytes an editor would send, rather than a +/// `Url`, so tests can see `Uri` -> `Url` normalisation instead of it being +/// hidden by starting from an already-normalised `Url`. +pub(super) fn insert_file(state: &mut WorldState, wire: &str, contents: &str) -> Uri { + let uri: Uri = wire.parse().unwrap(); + let url = uri.to_url().unwrap(); let file = state .db - .upsert_editor(FilePath::from_url(uri), contents.to_string()); - state.insert_open_file(uri.clone(), file, None); + .upsert_editor(FilePath::from_url(&url), contents.to_string()); + state.insert_open_file(uri.clone(), FilePath::from_url(&url), file, None); + uri } pub(super) fn range(start: (u32, u32), end: (u32, u32)) -> lsp_types::Range { diff --git a/crates/ark/src/lsp/traits/url.rs b/crates/ark/src/lsp/traits/url.rs index dd01e6f4c5..8d4c63aaf6 100644 --- a/crates/ark/src/lsp/traits/url.rs +++ b/crates/ark/src/lsp/traits/url.rs @@ -1,27 +1,52 @@ // // url.rs // -// Copyright (C) 2022 Posit Software, PBC. All rights reserved. +// Copyright (C) 2022-2026 Posit Software, PBC. All rights reserved. // // -use std::path::PathBuf; -use std::result::Result::Ok; - +use aether_path::FilePath; use anyhow::*; -use stdext::unwrap; -use tower_lsp::lsp_types::Url; +use tower_lsp_server::ls_types::Uri; +use url::Url; + +/// `tower_lsp_server` carries document URIs on the wire as `ls_types::Uri`, +/// a `fluent_uri`-backed type. Convert at the LSP boundary so downstream code +/// never has to deal with two URI types. +pub trait UriExt { + /// The document identity for this URI. + /// + /// Not named `to_file_path()` because of a conflict with `Uri::to_file_path()`. + fn to_document_path(&self) -> anyhow::Result; + + /// The URI as a [`Url`]. Prefer [`Self::to_document_path()`] for identity. + /// This is for the few places that need URL structure on the way to + /// something else, such as deriving an [`aether_path::AbsPathBuf`] for a + /// workspace folder. + fn to_url(&self) -> anyhow::Result; +} + +impl UriExt for Uri { + fn to_document_path(&self) -> anyhow::Result { + Ok(FilePath::from_url(&self.to_url()?)) + } + fn to_url(&self) -> anyhow::Result { + Url::parse(self.as_str()) + .with_context(|| format!("error converting URI {} to URL", self.as_str())) + } +} + +/// The reverse of [`UriExt::to_url()`], for building outgoing LSP responses +/// that carry a `Uri` (e.g. `Location`, `WorkspaceEdit`). pub trait UrlExt { - fn file_path(&self) -> anyhow::Result; + fn to_uri(&self) -> anyhow::Result; } impl UrlExt for Url { - fn file_path(&self) -> anyhow::Result { - let pathbuf = unwrap!(self.to_file_path(), Err(_) => { - return Err(anyhow!("error converting URI {} to PathBuf", self)); - }); - - Ok(pathbuf) + fn to_uri(&self) -> anyhow::Result { + self.as_str() + .parse::() + .map_err(|err| anyhow!("error converting URL {self} to URI: {err}")) } } diff --git a/crates/ark/src/url.rs b/crates/ark/src/url.rs index 83fc031af9..5a842f9979 100644 --- a/crates/ark/src/url.rs +++ b/crates/ark/src/url.rs @@ -7,57 +7,58 @@ use aether_path::FilePath; use amalthea::wire::execute_request::CodeLocation; -use url::Url; /// Extract a canonical [`FilePath`] from a [`CodeLocation`]. pub fn file_path_from_code_location(loc: &CodeLocation) -> FilePath { FilePath::from_url(&loc.uri) } -/// Extended URL utilities. -/// -/// These operate on raw `Url` values and don't require canonicalization. -/// For identity-sensitive operations (HashMap keys, breakpoint matching), -/// use [`FilePath`] instead. -pub struct ExtUrl; - -impl ExtUrl { - /// Whether this URI should get diagnostics. Currently uses an exclude list: - /// only `ark://` virtual documents are excluded since they show foreign - /// code the user can't edit. - pub fn should_diagnose(uri: &Url) -> bool { - !Self::is_ark_virtual_doc(uri) +/// Ark-specific identity questions about a [`FilePath`]. +pub trait FilePathExt { + /// Whether this identifies an `ark://` virtual document (e.g. debugger + /// vdocs showing foreign code the user can't edit). + fn is_ark_virtual_doc(&self) -> bool; + + /// Whether this document should get diagnostics. Currently uses an + /// exclude list: only `ark://` virtual documents are excluded. + fn should_diagnose(&self) -> bool; +} + +impl FilePathExt for FilePath { + fn is_ark_virtual_doc(&self) -> bool { + self.as_virtual() + .is_some_and(|uri| uri.as_url().scheme() == "ark") } - /// Whether this URI points to an `ark://` virtual document (e.g. debugger - /// vdocs showing foreign code). - pub fn is_ark_virtual_doc(uri: &Url) -> bool { - uri.scheme() == "ark" + fn should_diagnose(&self) -> bool { + !self.is_ark_virtual_doc() } } #[cfg(test)] mod tests { + use url::Url; + use super::*; #[test] fn test_is_ark_virtual_doc() { let ark_uri = Url::parse("ark://namespace/test.R").unwrap(); - assert!(ExtUrl::is_ark_virtual_doc(&ark_uri)); + assert!(FilePath::from_url(&ark_uri).is_ark_virtual_doc()); let file_uri = Url::parse("file:///home/user/test.R").unwrap(); - assert!(!ExtUrl::is_ark_virtual_doc(&file_uri)); + assert!(!FilePath::from_url(&file_uri).is_ark_virtual_doc()); } #[test] fn test_should_diagnose() { let file_uri = Url::parse("file:///home/user/test.R").unwrap(); - assert!(ExtUrl::should_diagnose(&file_uri)); + assert!(FilePath::from_url(&file_uri).should_diagnose()); let git_uri = Url::parse("git:///home/user/test.R?ref=HEAD").unwrap(); - assert!(ExtUrl::should_diagnose(&git_uri)); + assert!(FilePath::from_url(&git_uri).should_diagnose()); let ark_uri = Url::parse("ark://namespace/test.R").unwrap(); - assert!(!ExtUrl::should_diagnose(&ark_uri)); + assert!(!FilePath::from_url(&ark_uri).should_diagnose()); } } diff --git a/crates/ark/tests/integration/dap_breakpoints.rs b/crates/ark/tests/integration/dap_breakpoints.rs index 18d63faf55..cdff5d768a 100644 --- a/crates/ark/tests/integration/dap_breakpoints.rs +++ b/crates/ark/tests/integration/dap_breakpoints.rs @@ -8,7 +8,7 @@ use amalthea::fixtures::dummy_frontend::ExecuteRequestOptions; use ark_test::DummyArkFrontend; use ark_test::SourceFile; -use tower_lsp::lsp_types; +use tower_lsp_server::ls_types as lsp_types; #[test] fn test_dap_set_breakpoints_unverified() { @@ -284,7 +284,7 @@ foo() // Open the file on the LSP using its real URL so the LSP-side `UrlId` // canonicalizes to the same one the DAP indexed the breakpoint under. - let uri = lsp_types::Url::from_file_path(&file.path).unwrap(); + let uri = lsp_types::Uri::from_file_path(&file.path).unwrap(); lsp.send_notification( "textDocument/didOpen", serde_json::to_value(lsp_types::DidOpenTextDocumentParams { diff --git a/crates/ark/tests/integration/lsp.rs b/crates/ark/tests/integration/lsp.rs index 0ac9b25b33..fe77ec2e3c 100644 --- a/crates/ark/tests/integration/lsp.rs +++ b/crates/ark/tests/integration/lsp.rs @@ -8,6 +8,10 @@ // The lsp files in ark_test are for only integration tests with the Jupyter // kernel, i.e. LSP features that require dynamic access to the R session. +use std::sync::Arc; +use std::sync::Mutex; +use std::time::Duration; + use ark_test::DummyArkFrontend; use serde_json::json; @@ -20,6 +24,59 @@ fn test_lsp_init() { assert!(lsp.server_capabilities().completion_provider.is_some()); } +// Reproduces https://github.com/posit-dev/ark/issues/1361: an abrupt client +// disconnect (TCP reset, e.g. the extension host being recycled) hits an +// `unreachable!()` in our pinned tower-lsp fork's transport and panics the +// `ark-lsp` thread. +#[test] +fn test_lsp_survives_abrupt_disconnect() { + // The panic happens on the `ark-lsp` thread, so a plain `#[should_panic]` + // or `catch_unwind` on this (the main test) thread can't see it. Install + // a hook that records panics by thread name instead, and chain to the + // previous hook so panic output still gets printed. + let panic_message: Arc>> = Arc::new(Mutex::new(None)); + let panic_message_hook = Arc::clone(&panic_message); + let previous_hook = std::panic::take_hook(); + std::panic::set_hook(Box::new(move |info| { + if std::thread::current().name() == Some("ark-lsp") { + *panic_message_hook.lock().unwrap() = Some(info.to_string()); + } + previous_hook(info); + })); + + let frontend = DummyArkFrontend::lock(); + let lsp = frontend.start_lsp(); + + lsp.disconnect_abruptly(); + + // Give the LSP thread a moment to observe the reset. + std::thread::sleep(Duration::from_millis(200)); + + if let Some(message) = panic_message.lock().unwrap().take() { + panic!("`ark-lsp` thread panicked on abrupt disconnect: {message}"); + } + + // The LSP should still be usable after a client disconnects abruptly. + let lsp2 = frontend.start_lsp(); + assert!(lsp2.server_capabilities().completion_provider.is_some()); +} + +// Reproduces https://github.com/ebkalderon/tower-lsp/issues/399 and #424: +// some clients send `exit` but never close their own end of the connection +// afterward. The server must hang up on its own rather than waiting forever +// for more input that will never arrive. +#[test] +fn test_lsp_exits_promptly_after_exit_without_client_close() { + let frontend = DummyArkFrontend::lock(); + let mut lsp = frontend.start_lsp(); + + // Sends `shutdown`/`exit` but deliberately leaves our end of the socket + // open, unlike the client's normal teardown on `Drop`. + lsp.shutdown(); + + lsp.expect_server_closes_connection(Duration::from_secs(5)); +} + // The two cases below test errors that don't depend on the rename // implementation's resolution capabilities. New-name validation always // applies (R language constraints), so these tests stay valid once diff --git a/crates/ark_test/Cargo.toml b/crates/ark_test/Cargo.toml index 7ddab7abba..0368be2bcf 100644 --- a/crates/ark_test/Cargo.toml +++ b/crates/ark_test/Cargo.toml @@ -26,9 +26,10 @@ log.workspace = true regex.workspace = true serde.workspace = true serde_json.workspace = true +socket2.workspace = true stdext.workspace = true tempfile.workspace = true -tower-lsp.workspace = true +tower-lsp-server.workspace = true tree-sitter.workspace = true url.workspace = true uuid.workspace = true diff --git a/crates/ark_test/src/lsp_client.rs b/crates/ark_test/src/lsp_client.rs index 95fe661203..f134e8a896 100644 --- a/crates/ark_test/src/lsp_client.rs +++ b/crates/ark_test/src/lsp_client.rs @@ -15,7 +15,7 @@ use std::time::Duration; use serde_json::json; use serde_json::Value; -use tower_lsp::lsp_types; +use tower_lsp_server::ls_types as lsp_types; const DEFAULT_TIMEOUT: Duration = Duration::from_secs(30); @@ -31,11 +31,13 @@ pub struct LspClient { next_id: i64, initialized: bool, /// Documents opened by this client, closed on drop - open_documents: Vec, + open_documents: Vec, /// Server capabilities from the initialize response server_capabilities: Option, /// Buffered diagnostics notifications, keyed by document URI - diagnostics: std::collections::HashMap>, + diagnostics: std::collections::HashMap>, + /// Set by `disconnect_abruptly()` so `Drop` skips the graceful `shutdown`/`exit` sequence + killed: bool, } impl LspClient { @@ -56,9 +58,23 @@ impl LspClient { open_documents: Vec::new(), server_capabilities: None, diagnostics: std::collections::HashMap::new(), + killed: false, }) } + /// Sever the connection with a TCP reset instead of a graceful close. + /// + /// A zero `SO_LINGER` makes the close abortive, so the socket sends a `RST` + /// rather than a `FIN`. That's what an abrupt client crash or network drop + /// looks like, as opposed to `shutdown()`, which closes cleanly. + pub fn disconnect_abruptly(mut self) { + self.killed = true; + + socket2::SockRef::from(self.writer.get_ref()) + .set_linger(Some(Duration::ZERO)) + .unwrap(); + } + /// Initialize the LSP session. /// /// Sends the `initialize` request followed by the `initialized` notification. @@ -91,20 +107,20 @@ impl LspClient { } /// Returns diagnostics for a document, if any have been received. - pub fn diagnostics(&self, uri: &lsp_types::Url) -> Option<&Vec> { + pub fn diagnostics(&self, uri: &lsp_types::Uri) -> Option<&Vec> { self.diagnostics.get(uri) } /// Clears buffered diagnostics for a document. - pub fn clear_diagnostics(&mut self, uri: &lsp_types::Url) { + pub fn clear_diagnostics(&mut self, uri: &lsp_types::Uri) { self.diagnostics.remove(uri); } /// Notify the server that a document was opened. /// /// Returns the URI assigned to the document (based on the provided `name`). - pub fn open_document(&mut self, name: &str, text: &str) -> lsp_types::Url { - let uri = lsp_types::Url::parse(&format!("file:///test/{name}")).unwrap(); + pub fn open_document(&mut self, name: &str, text: &str) -> lsp_types::Uri { + let uri: lsp_types::Uri = format!("file:///test/{name}").parse().unwrap(); let params = lsp_types::DidOpenTextDocumentParams { text_document: lsp_types::TextDocumentItem { @@ -125,7 +141,7 @@ impl LspClient { } /// Close a previously opened document. - pub fn close_document(&mut self, uri: &lsp_types::Url) { + pub fn close_document(&mut self, uri: &lsp_types::Uri) { let params = lsp_types::DidCloseTextDocumentParams { text_document: lsp_types::TextDocumentIdentifier { uri: uri.clone() }, }; @@ -141,7 +157,7 @@ impl LspClient { /// Request completions at the given 0-based line and character position. pub fn completions( &mut self, - uri: &lsp_types::Url, + uri: &lsp_types::Uri, line: u32, character: u32, ) -> Vec { @@ -234,7 +250,7 @@ impl LspClient { } // Close any open documents first - let uris: Vec = std::mem::take(&mut self.open_documents); + let uris: Vec = std::mem::take(&mut self.open_documents); for uri in &uris { self.close_document(uri); } @@ -244,6 +260,59 @@ impl LspClient { self.initialized = false; } + /// Assert the server closes its end of the connection within `timeout`. + /// + /// Meant to be called right after `shutdown()`. Some clients never close + /// their own socket after sending `exit` + /// (), so the server + /// must hang up on its own rather than waiting forever for more input. + /// + /// Tolerates benign notifications (e.g. `window/logMessage`) the server + /// may still emit while winding down. + /// + /// # Panics + /// + /// Panics on an unexpected request/response, or if `timeout` elapses + /// without the connection closing. + #[track_caller] + pub fn expect_server_closes_connection(&mut self, timeout: Duration) { + let deadline = std::time::Instant::now() + timeout; + + loop { + let remaining = deadline.saturating_duration_since(std::time::Instant::now()); + if remaining.is_zero() { + panic!("Server did not close the connection within {timeout:?} after `exit`"); + } + self.reader + .get_ref() + .set_read_timeout(Some(remaining)) + .unwrap(); + + match self.recv_message() { + Ok(message) if message.contains_key("id") => { + panic!("Unexpected message while waiting for connection to close: {message:?}") + }, + // A benign notification, e.g. `window/logMessage`; keep waiting for the close. + Ok(_) => continue, + Err(err) => { + let timed_out = err.downcast_ref::().is_some_and(|io_err| { + matches!( + io_err.kind(), + std::io::ErrorKind::WouldBlock | std::io::ErrorKind::TimedOut + ) + }); + if timed_out { + panic!( + "Server did not close the connection within {timeout:?} after `exit`" + ); + } + // Any other error here (e.g. clean EOF) means the server hung up, as expected. + return; + }, + } + } + } + fn send_raw(&mut self, message: &Value) { let body = serde_json::to_string(message).unwrap(); write!( @@ -471,7 +540,7 @@ impl LspClient { impl Drop for LspClient { fn drop(&mut self) { - if std::thread::panicking() { + if self.killed || std::thread::panicking() { return; } self.shutdown();