Skip to content
This repository has been archived by the owner on Dec 29, 2022. It is now read-only.

Fix crashes when editing wide characters spanning 2 u16s (UTF-16) #1112

Merged
merged 7 commits into from
Nov 11, 2018
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 4 additions & 3 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down
9 changes: 6 additions & 3 deletions src/actions/notifications.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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 {
Expand Down
2 changes: 1 addition & 1 deletion src/actions/requests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
20 changes: 8 additions & 12 deletions src/lsp_data.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand Down Expand Up @@ -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))
}
Expand Down Expand Up @@ -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 {
Xanewok marked this conversation as resolved.
Show resolved Hide resolved
let content = content.as_ref();

if content.is_empty() {
Range {
start: Position::new(0, 0),
Expand All @@ -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 {
Expand Down
121 changes: 121 additions & 0 deletions tests/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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 = ["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());
}

/// 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 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());
}