5659: Revert LineIndex optimizations to fix #5656 r=matklad a=lazear

This PR reverts the changes from https://github.com/rust-analyzer/rust-analyzer/pull/5532, which was causing issues as described in
https://github.com/rust-analyzer/rust-analyzer/issues/5656

Co-authored-by: Michael Lazear <lazear@scripps.edu>
This commit is contained in:
bors[bot] 2020-08-03 23:18:16 +00:00 committed by GitHub
commit 2b0c2f6771
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
2 changed files with 23 additions and 31 deletions

View file

@ -1,5 +1,5 @@
//! Utilities for LSP-related boilerplate code. //! Utilities for LSP-related boilerplate code.
use std::{borrow::Cow, error::Error, ops::Range}; use std::{error::Error, ops::Range};
use lsp_server::Notification; use lsp_server::Notification;
use ra_db::Canceled; use ra_db::Canceled;
@ -84,8 +84,8 @@ impl GlobalState {
pub(crate) fn apply_document_changes( pub(crate) fn apply_document_changes(
old_text: &mut String, old_text: &mut String,
content_changes: Vec<lsp_types::TextDocumentContentChangeEvent>, content_changes: Vec<lsp_types::TextDocumentContentChangeEvent>,
mut line_index: Cow<'_, LineIndex>,
) { ) {
let mut line_index = LineIndex::new(old_text);
// The changes we got must be applied sequentially, but can cross lines so we // The changes we got must be applied sequentially, but can cross lines so we
// have to keep our line index updated. // have to keep our line index updated.
// Some clients (e.g. Code) sort the ranges in reverse. As an optimization, we // Some clients (e.g. Code) sort the ranges in reverse. As an optimization, we
@ -110,7 +110,7 @@ pub(crate) fn apply_document_changes(
match change.range { match change.range {
Some(range) => { Some(range) => {
if !index_valid.covers(range.end.line) { if !index_valid.covers(range.end.line) {
line_index = Cow::Owned(LineIndex::new(old_text)); line_index = LineIndex::new(&old_text);
} }
index_valid = IndexValid::UpToLineExclusive(range.start.line); index_valid = IndexValid::UpToLineExclusive(range.start.line);
let range = from_proto::text_range(&line_index, range); let range = from_proto::text_range(&line_index, range);
@ -145,15 +145,10 @@ mod tests {
}; };
} }
fn run(text: &mut String, changes: Vec<TextDocumentContentChangeEvent>) {
let line_index = Cow::Owned(LineIndex::new(&text));
super::apply_document_changes(text, changes, line_index);
}
let mut text = String::new(); let mut text = String::new();
run(&mut text, vec![]); apply_document_changes(&mut text, vec![]);
assert_eq!(text, ""); assert_eq!(text, "");
run( apply_document_changes(
&mut text, &mut text,
vec![TextDocumentContentChangeEvent { vec![TextDocumentContentChangeEvent {
range: None, range: None,
@ -162,36 +157,39 @@ mod tests {
}], }],
); );
assert_eq!(text, "the"); assert_eq!(text, "the");
run(&mut text, c![0, 3; 0, 3 => " quick"]); apply_document_changes(&mut text, c![0, 3; 0, 3 => " quick"]);
assert_eq!(text, "the quick"); assert_eq!(text, "the quick");
run(&mut text, c![0, 0; 0, 4 => "", 0, 5; 0, 5 => " foxes"]); apply_document_changes(&mut text, c![0, 0; 0, 4 => "", 0, 5; 0, 5 => " foxes"]);
assert_eq!(text, "quick foxes"); assert_eq!(text, "quick foxes");
run(&mut text, c![0, 11; 0, 11 => "\ndream"]); apply_document_changes(&mut text, c![0, 11; 0, 11 => "\ndream"]);
assert_eq!(text, "quick foxes\ndream"); assert_eq!(text, "quick foxes\ndream");
run(&mut text, c![1, 0; 1, 0 => "have "]); apply_document_changes(&mut text, c![1, 0; 1, 0 => "have "]);
assert_eq!(text, "quick foxes\nhave dream"); assert_eq!(text, "quick foxes\nhave dream");
run(&mut text, c![0, 0; 0, 0 => "the ", 1, 4; 1, 4 => " quiet", 1, 16; 1, 16 => "s\n"]); apply_document_changes(
&mut text,
c![0, 0; 0, 0 => "the ", 1, 4; 1, 4 => " quiet", 1, 16; 1, 16 => "s\n"],
);
assert_eq!(text, "the quick foxes\nhave quiet dreams\n"); assert_eq!(text, "the quick foxes\nhave quiet dreams\n");
run(&mut text, c![0, 15; 0, 15 => "\n", 2, 17; 2, 17 => "\n"]); apply_document_changes(&mut text, c![0, 15; 0, 15 => "\n", 2, 17; 2, 17 => "\n"]);
assert_eq!(text, "the quick foxes\n\nhave quiet dreams\n\n"); assert_eq!(text, "the quick foxes\n\nhave quiet dreams\n\n");
run( apply_document_changes(
&mut text, &mut text,
c![1, 0; 1, 0 => "DREAM", 2, 0; 2, 0 => "they ", 3, 0; 3, 0 => "DON'T THEY?"], c![1, 0; 1, 0 => "DREAM", 2, 0; 2, 0 => "they ", 3, 0; 3, 0 => "DON'T THEY?"],
); );
assert_eq!(text, "the quick foxes\nDREAM\nthey have quiet dreams\nDON'T THEY?\n"); assert_eq!(text, "the quick foxes\nDREAM\nthey have quiet dreams\nDON'T THEY?\n");
run(&mut text, c![0, 10; 1, 5 => "", 2, 0; 2, 12 => ""]); apply_document_changes(&mut text, c![0, 10; 1, 5 => "", 2, 0; 2, 12 => ""]);
assert_eq!(text, "the quick \nthey have quiet dreams\n"); assert_eq!(text, "the quick \nthey have quiet dreams\n");
text = String::from("❤️"); text = String::from("❤️");
run(&mut text, c![0, 0; 0, 0 => "a"]); apply_document_changes(&mut text, c![0, 0; 0, 0 => "a"]);
assert_eq!(text, "a❤"); assert_eq!(text, "a❤");
text = String::from("a\nb"); text = String::from("a\nb");
run(&mut text, c![0, 1; 1, 0 => "\nțc", 0, 1; 1, 1 => "d"]); apply_document_changes(&mut text, c![0, 1; 1, 0 => "\nțc", 0, 1; 1, 1 => "d"]);
assert_eq!(text, "adcb"); assert_eq!(text, "adcb");
text = String::from("a\nb"); text = String::from("a\nb");
run(&mut text, c![0, 1; 1, 0 => "ț\nc", 0, 2; 0, 2 => "c"]); apply_document_changes(&mut text, c![0, 1; 1, 0 => "ț\nc", 0, 2; 0, 2 => "c"]);
assert_eq!(text, "ațc\ncb"); assert_eq!(text, "ațc\ncb");
} }
} }

View file

@ -1,14 +1,13 @@
//! The main loop of `rust-analyzer` responsible for dispatching LSP //! The main loop of `rust-analyzer` responsible for dispatching LSP
//! requests/replies and notifications back to the client. //! requests/replies and notifications back to the client.
use std::{ use std::{
borrow::Cow,
env, fmt, panic, env, fmt, panic,
time::{Duration, Instant}, time::{Duration, Instant},
}; };
use crossbeam_channel::{select, Receiver}; use crossbeam_channel::{select, Receiver};
use lsp_server::{Connection, Notification, Request, Response}; use lsp_server::{Connection, Notification, Request, Response};
use lsp_types::{notification::Notification as _, DidChangeTextDocumentParams}; use lsp_types::notification::Notification as _;
use ra_db::VfsPath; use ra_db::VfsPath;
use ra_ide::{Canceled, FileId}; use ra_ide::{Canceled, FileId};
use ra_prof::profile; use ra_prof::profile;
@ -422,20 +421,15 @@ impl GlobalState {
})? })?
.on::<lsp_types::notification::DidChangeTextDocument>(|this, params| { .on::<lsp_types::notification::DidChangeTextDocument>(|this, params| {
if let Ok(path) = from_proto::vfs_path(&params.text_document.uri) { if let Ok(path) = from_proto::vfs_path(&params.text_document.uri) {
let DidChangeTextDocumentParams { text_document, content_changes } = params; let doc = this.mem_docs.get_mut(&path).unwrap();
let vfs = &mut this.vfs.write().0; let vfs = &mut this.vfs.write().0;
let world = this.snapshot();
let file_id = vfs.file_id(&path).unwrap(); let file_id = vfs.file_id(&path).unwrap();
// let file_id = vfs.file_id(&path).unwrap();
let mut text = String::from_utf8(vfs.file_contents(file_id).to_vec()).unwrap(); let mut text = String::from_utf8(vfs.file_contents(file_id).to_vec()).unwrap();
let line_index = world.analysis.file_line_index(file_id)?; apply_document_changes(&mut text, params.content_changes);
apply_document_changes(&mut text, content_changes, Cow::Borrowed(&line_index));
// The version passed in DidChangeTextDocument is the version after all edits are applied // The version passed in DidChangeTextDocument is the version after all edits are applied
// so we should apply it before the vfs is notified. // so we should apply it before the vfs is notified.
let doc = this.mem_docs.get_mut(&path).unwrap(); doc.version = params.text_document.version;
doc.version = text_document.version;
vfs.set_file_contents(path.clone(), Some(text.into_bytes())); vfs.set_file_contents(path.clone(), Some(text.into_bytes()));
} }