From 260a2cdd58c700e71f13a8d7a4e223b60c74954b Mon Sep 17 00:00:00 2001 From: Igor Matuszewski Date: Thu, 1 Nov 2018 21:14:51 +0100 Subject: [PATCH 1/7] Don't load file from VFS to calculate reformat range --- src/actions/requests.rs | 2 +- src/lsp_data.rs | 14 +++----------- 2 files changed, 4 insertions(+), 12 deletions(-) diff --git a/src/actions/requests.rs b/src/actions/requests.rs index fc0cdaba58a..c2fe90513c3 100644 --- a/src/actions/requests.rs +++ b/src/actions/requests.rs @@ -746,7 +746,7 @@ fn reformat( } }; - let range_whole_file = ls_util::range_from_vfs_file(&ctx.vfs, &path); + let range_whole_file = ls_util::range_from_file_string(&input); let mut config = ctx.fmt_config().get_rustfmt_config().clone(); if !config.was_set().hard_tabs() { config.set().hard_tabs(!opts.insert_spaces); diff --git a/src/lsp_data.rs b/src/lsp_data.rs index 54fc0fff98f..c9f5b3dafbe 100644 --- a/src/lsp_data.rs +++ b/src/lsp_data.rs @@ -18,7 +18,6 @@ use languageserver_types as ls_types; use racer; use rls_analysis::DefKind; use rls_span as span; -use rls_vfs::FileContents; use serde_derive::{Deserialize, Serialize}; use url::Url; @@ -87,9 +86,6 @@ pub mod ls_util { use super::*; use crate::Span; - use rls_vfs::Vfs; - use std::path::Path; - /// Convert a language server protocol range into an RLS range. pub fn range_to_rls(r: Range) -> span::Range { span::Range::from_positions(position_to_rls(r.start), position_to_rls(r.end)) @@ -146,13 +142,9 @@ pub mod ls_util { /// Creates a `Range` spanning the whole file as currently known by `Vfs` /// /// Panics if `Vfs` cannot load the file. - pub fn range_from_vfs_file(vfs: &Vfs, fname: &Path) -> Range { - // FIXME load_file clones the entire file text, this could be much more - // efficient by adding a `with_file` fn to the VFS. - let content = match vfs.load_file(fname).unwrap() { - FileContents::Text(t) => t, - _ => panic!("unexpected binary file: {:?}", fname), - }; + pub fn range_from_file_string(content: impl AsRef) -> Range { + let content = content.as_ref(); + if content.is_empty() { Range { start: Position::new(0, 0), From 5abd217f5614080953fc11f576b0b70cf6e0db47 Mon Sep 17 00:00:00 2001 From: Igor Matuszewski Date: Fri, 2 Nov 2018 16:08:24 +0100 Subject: [PATCH 2/7] Add failing UTF-16 text edit test --- tests/tests.rs | 59 ++++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 59 insertions(+) diff --git a/tests/tests.rs b/tests/tests.rs index a184ec9c241..3b454aa890a 100644 --- a/tests/tests.rs +++ b/tests/tests.rs @@ -1033,3 +1033,62 @@ fn cmd_invalid_member_dependency_resolution() { rls.shutdown(rls_timeout()); } + +#[test] +fn cmd_handle_utf16_unit_text_edits() { + let project = project("cmd_handle_utf16_unit_text_edits") + .file( + "Cargo.toml", + r#"[package] + name = "root_is_fine" + version = "0.1.0" + authors = ["example@example.com"] + "#, + ) + .file("src/main.rs", "fn main() {}") + .file("src/unrelated.rs", "😢") + .build(); + let root_path = project.root(); + let mut rls = project.spawn_rls(); + + rls.request( + 0, + "initialize", + Some(json!({ + "rootPath": root_path, + "capabilities": {} + })), + ) + .unwrap(); + + rls.wait_until_done_indexing(rls_timeout()); + + rls.notify( + "textDocument/didChange", + Some(json!( + {"textDocument": { + "uri": format!("file://{}/src/unrelated.rs", root_path.as_path().display()), + "version": 1 + }, + // "😢" -> "" + "contentChanges": [ + { + "range": { + "start": { + "line":0, + "character":0 + }, + "end":{ + "line":0, + "character":2 + } + }, + "rangeLength":2, + "text":"" + } + ] + })) + ).unwrap(); + + rls.shutdown(rls_timeout()); +} From 2849553dd9aee31bf1173565de9677bab9b2d47d Mon Sep 17 00:00:00 2001 From: Igor Matuszewski Date: Fri, 2 Nov 2018 16:10:48 +0100 Subject: [PATCH 3/7] Pull rls-vfs branch with UTF-16-compatible edits --- Cargo.lock | 9 +++++---- Cargo.toml | 5 ++++- src/actions/notifications.rs | 4 +++- src/lsp_data.rs | 2 ++ 4 files changed, 14 insertions(+), 6 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 2d009e18458..35c030bfefc 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1150,7 +1150,7 @@ dependencies = [ "rls-data 0.18.1 (registry+https://github.com/rust-lang/crates.io-index)", "rls-rustc 0.5.0 (registry+https://github.com/rust-lang/crates.io-index)", "rls-span 0.4.0 (registry+https://github.com/rust-lang/crates.io-index)", - "rls-vfs 0.6.0 (registry+https://github.com/rust-lang/crates.io-index)", + "rls-vfs 0.7.0 (git+https://github.com/Xanewok/rls-vfs?branch=handle-utf16-offset)", "rustc-serialize 0.3.24 (registry+https://github.com/rust-lang/crates.io-index)", "rustc-workspace-hack 1.0.0 (registry+https://github.com/rust-lang/crates.io-index)", "rustc_tools_util 0.1.0 (git+https://github.com/rust-lang-nursery/rust-clippy?rev=d8b426901a75b1eb975f52b4537f2736f2b94436)", @@ -1211,9 +1211,10 @@ dependencies = [ [[package]] name = "rls-vfs" -version = "0.6.0" -source = "registry+https://github.com/rust-lang/crates.io-index" +version = "0.7.0" +source = "git+https://github.com/Xanewok/rls-vfs?branch=handle-utf16-offset#3fde04814b2670f49bd335572a7091cbc083a7f6" dependencies = [ + "log 0.4.5 (registry+https://github.com/rust-lang/crates.io-index)", "rls-span 0.4.0 (registry+https://github.com/rust-lang/crates.io-index)", ] @@ -1918,7 +1919,7 @@ dependencies = [ "checksum rls-data 0.18.1 (registry+https://github.com/rust-lang/crates.io-index)" = "3a209ce46bb52813cbe0786a7baadc0c1a3f5543ef93f179eda3b841ed72cf2e" "checksum rls-rustc 0.5.0 (registry+https://github.com/rust-lang/crates.io-index)" = "2f9dba7390427aefa953608429701e3665192ca810ba8ae09301e001b7c7bed0" "checksum rls-span 0.4.0 (registry+https://github.com/rust-lang/crates.io-index)" = "5d7c7046dc6a92f2ae02ed302746db4382e75131b9ce20ce967259f6b5867a6a" -"checksum rls-vfs 0.6.0 (registry+https://github.com/rust-lang/crates.io-index)" = "fcc93379d5ce372822a5928d19687e143f9e66ecda8ab8b0c721462ea3ba6f47" +"checksum rls-vfs 0.7.0 (git+https://github.com/Xanewok/rls-vfs?branch=handle-utf16-offset)" = "" "checksum rustc-ap-arena 274.0.0 (registry+https://github.com/rust-lang/crates.io-index)" = "866fda692855b38f9d6562f0e952c75c6ebe4032d7dd63c608a88e7c4d3f8087" "checksum rustc-ap-rustc_cratesio_shim 274.0.0 (registry+https://github.com/rust-lang/crates.io-index)" = "b6c2343e11a97b4eb3bee29cd5f9314ea409a87baee5d3fec8d1516d1633412e" "checksum rustc-ap-rustc_data_structures 274.0.0 (registry+https://github.com/rust-lang/crates.io-index)" = "b88f905f7ab99bf14220a3a87eff60ec143af8136fd0ca8573641c78be532ec8" diff --git a/Cargo.toml b/Cargo.toml index 91285385aae..389c1ead885 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -10,6 +10,9 @@ categories = ["development-tools"] build = "build.rs" +[patch.crates-io] +rls-vfs = { version = "0.7", git = "https://github.com/Xanewok/rls-vfs", branch = "handle-utf16-offset" } + [dependencies] cargo = { git = "https://github.com/rust-lang/cargo", rev = "2d0863f657e6f45159fc7412267eee3e659185e5" } cargo_metadata = "0.6" @@ -30,7 +33,7 @@ rls-blacklist = "0.1.2" rls-data = { version = "0.18.1", features = ["serialize-serde", "serialize-rustc"] } rls-rustc = "0.5.0" rls-span = { version = "0.4", features = ["serialize-serde"] } -rls-vfs = "0.6" +rls-vfs = "0.7" rustc_tools_util = { git = "https://github.com/rust-lang-nursery/rust-clippy", rev = "d8b426901a75b1eb975f52b4537f2736f2b94436" } rustfmt-nightly = "0.99.6" rustc-serialize = "0.3" diff --git a/src/actions/notifications.rs b/src/actions/notifications.rs index b3b1d5f85ff..ed513762ce4 100644 --- a/src/actions/notifications.rs +++ b/src/actions/notifications.rs @@ -14,7 +14,7 @@ use crate::actions::{FileWatch, InitActionContext, VersionOrdering}; use crate::config::Config; use crate::Span; use log::{debug, trace, warn}; -use rls_vfs::Change; +use rls_vfs::{Change, SpanAtom}; use serde::de::Error; use serde::Deserialize; use serde_json; @@ -113,6 +113,8 @@ impl BlockingNotificationAction for DidChangeTextDocument { if let Some(range) = i.range { let range = ls_util::range_to_rls(range); Change::ReplaceText { + // LSP uses UTF-16 code units based offsets and length + atom: SpanAtom::Utf16CodeUnit, span: Span::from_range(range, file_path.clone()), len: i.range_length, text: i.text.clone(), diff --git a/src/lsp_data.rs b/src/lsp_data.rs index c9f5b3dafbe..a07ab0bb95e 100644 --- a/src/lsp_data.rs +++ b/src/lsp_data.rs @@ -87,6 +87,8 @@ pub mod ls_util { use crate::Span; /// Convert a language server protocol range into an RLS range. + /// NOTE: This does not translate LSP UTF-16 code units offsets into Unicode + /// Scalar Value offsets as expected by RLS/Rust. pub fn range_to_rls(r: Range) -> span::Range { span::Range::from_positions(position_to_rls(r.start), position_to_rls(r.end)) } From 21dd055551c1c8a4f637108e06c7922ff6ab3453 Mon Sep 17 00:00:00 2001 From: Igor Matuszewski Date: Fri, 2 Nov 2018 16:44:47 +0100 Subject: [PATCH 4/7] Fix whole-file LSP Range --- src/lsp_data.rs | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/lsp_data.rs b/src/lsp_data.rs index a07ab0bb95e..88684712888 100644 --- a/src/lsp_data.rs +++ b/src/lsp_data.rs @@ -163,7 +163,9 @@ pub mod ls_util { .last() .expect("String is not empty.") .chars() - .count() as u64 + // LSP uses UTF-16 code units offset + .map(|chr| chr.len_utf16() as u64) + .sum() }; // range is zero-based and the end position is exclusive Range { From 507257047dbba9ecfcb30d75743000af0648ff58 Mon Sep 17 00:00:00 2001 From: Igor Matuszewski Date: Fri, 2 Nov 2018 18:48:31 +0100 Subject: [PATCH 5/7] Add whole-file LSP range regression test --- tests/tests.rs | 64 +++++++++++++++++++++++++++++++++++++++++++++++++- 1 file changed, 63 insertions(+), 1 deletion(-) diff --git a/tests/tests.rs b/tests/tests.rs index 3b454aa890a..25a695bb3fb 100644 --- a/tests/tests.rs +++ b/tests/tests.rs @@ -1040,7 +1040,7 @@ fn cmd_handle_utf16_unit_text_edits() { .file( "Cargo.toml", r#"[package] - name = "root_is_fine" + name = "cmd_handle_utf16_unit_text_edits" version = "0.1.0" authors = ["example@example.com"] "#, @@ -1092,3 +1092,65 @@ fn cmd_handle_utf16_unit_text_edits() { rls.shutdown(rls_timeout()); } + +/// Ensures that wide characters do not prevent RLS from calculating correct +/// 'whole file' LSP range. +#[test] +fn cmd_format_utf16_range() { + let project = project("cmd_format_utf16_range") + .file( + "Cargo.toml", + r#"[package] + name = "cmd_format_utf16_range" + version = "0.1.0" + authors = ["example@example.com"] + "#, + ) + .file("src/main.rs", "/* 😢😢😢😢😢😢😢 */ fn main() { }") + .build(); + let root_path = project.root(); + let mut rls = project.spawn_rls(); + + rls.request( + 0, + "initialize", + Some(json!({ + "rootPath": root_path, + "capabilities": {} + })), + ) + .unwrap(); + + rls.wait_until_done_indexing(rls_timeout()); + + let request_id = 66; + rls.request( + request_id, + "textDocument/formatting", + Some(json!( + { + "textDocument": { + "uri": format!("file://{}/src/main.rs", root_path.as_path().display()), + "version": 1 + }, + "options": { + "tabSize": 4, + "insertSpaces": true + } + })) + ).unwrap(); + + let json = rls.wait_until_json_id(request_id, rls_timeout()); + eprintln!("{:#?}", json); + + let result = json["result"].as_array().unwrap(); + let newText: Vec<_> = result + .into_iter() + .map(|o| o["newText"].as_str().unwrap()) + .collect(); + // Actual formatting isn't important - what is, is that the buffer isn't + // malformed and code stays semantically equivalent. + assert_eq!(newText, vec!["/* 😢😢😢😢😢😢😢 */\nfn main() {}\n"]); + + rls.shutdown(rls_timeout()); +} From b672685a7f33185d64d47c920db80fd7fa6625c5 Mon Sep 17 00:00:00 2001 From: Igor Matuszewski Date: Tue, 6 Nov 2018 19:30:46 +0100 Subject: [PATCH 6/7] Take updated VFS span type into account From commit: https://github.com/nrc/rls-vfs/pull/24/commits/3fde04814b2670f49bd335572a7091cbc083a7f6. --- src/actions/notifications.rs | 11 ++++++----- tests/tests.rs | 4 ++-- 2 files changed, 8 insertions(+), 7 deletions(-) diff --git a/src/actions/notifications.rs b/src/actions/notifications.rs index ed513762ce4..ff00a4aacfa 100644 --- a/src/actions/notifications.rs +++ b/src/actions/notifications.rs @@ -14,7 +14,7 @@ use crate::actions::{FileWatch, InitActionContext, VersionOrdering}; use crate::config::Config; use crate::Span; use log::{debug, trace, warn}; -use rls_vfs::{Change, SpanAtom}; +use rls_vfs::{Change, VfsSpan}; use serde::de::Error; use serde::Deserialize; use serde_json; @@ -113,10 +113,11 @@ impl BlockingNotificationAction for DidChangeTextDocument { if let Some(range) = i.range { let range = ls_util::range_to_rls(range); Change::ReplaceText { - // LSP uses UTF-16 code units based offsets and length - atom: SpanAtom::Utf16CodeUnit, - span: Span::from_range(range, file_path.clone()), - len: i.range_length, + // LSP sends UTF-16 code units based offsets and length + span: VfsSpan::from_utf16( + Span::from_range(range, file_path.clone()), + i.range_length + ), text: i.text.clone(), } } else { diff --git a/tests/tests.rs b/tests/tests.rs index 25a695bb3fb..a0ae0310627 100644 --- a/tests/tests.rs +++ b/tests/tests.rs @@ -1144,13 +1144,13 @@ fn cmd_format_utf16_range() { eprintln!("{:#?}", json); let result = json["result"].as_array().unwrap(); - let newText: Vec<_> = result + let new_text: Vec<_> = result .into_iter() .map(|o| o["newText"].as_str().unwrap()) .collect(); // Actual formatting isn't important - what is, is that the buffer isn't // malformed and code stays semantically equivalent. - assert_eq!(newText, vec!["/* 😢😢😢😢😢😢😢 */\nfn main() {}\n"]); + assert_eq!(new_text, vec!["/* 😢😢😢😢😢😢😢 */\nfn main() {}\n"]); rls.shutdown(rls_timeout()); } From 1ff3a383bad8c51f53f52350c8cfbe99652c381f Mon Sep 17 00:00:00 2001 From: Igor Matuszewski Date: Fri, 9 Nov 2018 13:35:58 +0100 Subject: [PATCH 7/7] Use rls-vfs 0.7 from crates.io --- Cargo.lock | 6 +++--- Cargo.toml | 3 --- 2 files changed, 3 insertions(+), 6 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 35c030bfefc..4cff9f36d03 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1150,7 +1150,7 @@ dependencies = [ "rls-data 0.18.1 (registry+https://github.com/rust-lang/crates.io-index)", "rls-rustc 0.5.0 (registry+https://github.com/rust-lang/crates.io-index)", "rls-span 0.4.0 (registry+https://github.com/rust-lang/crates.io-index)", - "rls-vfs 0.7.0 (git+https://github.com/Xanewok/rls-vfs?branch=handle-utf16-offset)", + "rls-vfs 0.7.0 (registry+https://github.com/rust-lang/crates.io-index)", "rustc-serialize 0.3.24 (registry+https://github.com/rust-lang/crates.io-index)", "rustc-workspace-hack 1.0.0 (registry+https://github.com/rust-lang/crates.io-index)", "rustc_tools_util 0.1.0 (git+https://github.com/rust-lang-nursery/rust-clippy?rev=d8b426901a75b1eb975f52b4537f2736f2b94436)", @@ -1212,7 +1212,7 @@ dependencies = [ [[package]] name = "rls-vfs" version = "0.7.0" -source = "git+https://github.com/Xanewok/rls-vfs?branch=handle-utf16-offset#3fde04814b2670f49bd335572a7091cbc083a7f6" +source = "registry+https://github.com/rust-lang/crates.io-index" dependencies = [ "log 0.4.5 (registry+https://github.com/rust-lang/crates.io-index)", "rls-span 0.4.0 (registry+https://github.com/rust-lang/crates.io-index)", @@ -1919,7 +1919,7 @@ dependencies = [ "checksum rls-data 0.18.1 (registry+https://github.com/rust-lang/crates.io-index)" = "3a209ce46bb52813cbe0786a7baadc0c1a3f5543ef93f179eda3b841ed72cf2e" "checksum rls-rustc 0.5.0 (registry+https://github.com/rust-lang/crates.io-index)" = "2f9dba7390427aefa953608429701e3665192ca810ba8ae09301e001b7c7bed0" "checksum rls-span 0.4.0 (registry+https://github.com/rust-lang/crates.io-index)" = "5d7c7046dc6a92f2ae02ed302746db4382e75131b9ce20ce967259f6b5867a6a" -"checksum rls-vfs 0.7.0 (git+https://github.com/Xanewok/rls-vfs?branch=handle-utf16-offset)" = "" +"checksum rls-vfs 0.7.0 (registry+https://github.com/rust-lang/crates.io-index)" = "72d56425bd5aa86d9d4372b76f0381d3b4bda9c0220e71956c9fcc929f45c1f1" "checksum rustc-ap-arena 274.0.0 (registry+https://github.com/rust-lang/crates.io-index)" = "866fda692855b38f9d6562f0e952c75c6ebe4032d7dd63c608a88e7c4d3f8087" "checksum rustc-ap-rustc_cratesio_shim 274.0.0 (registry+https://github.com/rust-lang/crates.io-index)" = "b6c2343e11a97b4eb3bee29cd5f9314ea409a87baee5d3fec8d1516d1633412e" "checksum rustc-ap-rustc_data_structures 274.0.0 (registry+https://github.com/rust-lang/crates.io-index)" = "b88f905f7ab99bf14220a3a87eff60ec143af8136fd0ca8573641c78be532ec8" diff --git a/Cargo.toml b/Cargo.toml index 389c1ead885..aaf2bee6628 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -10,9 +10,6 @@ categories = ["development-tools"] build = "build.rs" -[patch.crates-io] -rls-vfs = { version = "0.7", git = "https://github.com/Xanewok/rls-vfs", branch = "handle-utf16-offset" } - [dependencies] cargo = { git = "https://github.com/rust-lang/cargo", rev = "2d0863f657e6f45159fc7412267eee3e659185e5" } cargo_metadata = "0.6"