Merge pull request #1112 from Xanewok/fix-format-range

Fix crashes when editing wide characters spanning 2 `u16`s (UTF-16)

Author: nrc

Cargo.lock

@@ -30,7 +30,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"

Cargo.toml

@@ -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, VfsSpan};
 use serde::de::Error;
 use serde::Deserialize;
 use serde_json;
@@ -113,8 +113,11 @@ impl BlockingNotificationAction for DidChangeTextDocument {
                 if let Some(range) = i.range {
                     let range = ls_util::range_to_rls(range);
                     Change::ReplaceText {
-                        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 {

src/actions/notifications.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);

src/actions/requests.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,10 +86,9 @@ 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.
+    /// 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::ZeroIndexed> {
         span::Range::from_positions(position_to_rls(r.start), position_to_rls(r.end))
     }
@@ -146,13 +144,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<str>) -> Range {
+        let content = content.as_ref();
+
         if content.is_empty() {
             Range {
                 start: Position::new(0, 0),
@@ -169,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 {

src/lsp_data.rs

@@ -1033,3 +1033,124 @@ 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 = "cmd_handle_utf16_unit_text_edits"
+            version = "0.1.0"
+            authors = ["[email protected]"]
+            "#,
+        )
+        .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());
+}
+
+/// 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 = ["[email protected]"]
+            "#,
+        )
+        .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 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!(new_text, vec!["/* 😢😢😢😢😢😢😢 */\nfn main() {}\n"]);
+
+    rls.shutdown(rls_timeout());
+}