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

Support visualization of scopes, borrows and moves #387

Closed
wants to merge 5 commits into from
Closed
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
14 changes: 5 additions & 9 deletions Cargo.lock

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

8 changes: 4 additions & 4 deletions Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -16,10 +16,10 @@ env_logger = "0.4"
languageserver-types = "0.11"
log = "0.3"
racer = "2.0.6"
rls-analysis = "0.4"
rls-data = "0.7"
rls-span = { version = "0.4" , features = ["serialize-serde"] }
rls-vfs = { version = "0.4", features = ["racer-impls"] }
rls-analysis = { path = "../rls-analysis" }
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pending dependency updates mentioned in top-level comment.

rls-data = { path = "../rls-data", default-features = true, features = ["borrows"] }
rls-span = { version = "0.4", features = ["serialize-serde"] }
rls-vfs = { version = "0.4.2", features = ["racer-impls"] }
rustfmt-nightly = "0.1"
serde = "1.0"
serde_json = "1.0"
Expand Down
29 changes: 29 additions & 0 deletions src/actions/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -435,6 +435,35 @@ impl ActionHandler {
}
}

pub fn borrow_info<O: Output>(&self, id: usize, params: TextDocumentPositionParams, out: O) {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do you prefer another name?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

name seems fine

let t = thread::current();
let span = self.convert_pos_to_span(&params.text_document, params.position);

trace!("borrow_info: {:?}", span);

let analysis = self.analysis.clone();
let rustw_handle = thread::spawn(move || {
let bi = analysis.borrow_info(&span);
t.unpark();
bi.map(|b| b.into())
});

thread::park_timeout(Duration::from_millis(::COMPILER_TIMEOUT));

let result = rustw_handle.join();
match result {
Ok(Ok(r)) => {
out.success(id, ResponseData::BorrowInfo(r));
},
Ok(Err(e)) => {
out.failure(id, &format!("BorrowInfo not available for that location: {:?}", e)[..]);
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm betting this is not the way to handle a "nothing to see here" type of message. How should I fix this?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should probably be able to return something morally equivalent to out.success(id, None) and also use debug! to output a message for devs

}
Err(_) => {
out.failure(id, "BorrowInfo failed to complete successfully");
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Other code wasn't logging errors. Should we start to do that? Is there a file we can write to in case writing to stderr interferes with the IDEs?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just using debug! is fine. The client can hide, display, or redirect to a file. We should be logging all errors with debug!.

Copy link
Contributor Author

@Nashenas88 Nashenas88 Jul 7, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should I output the object in the Err too? I noticed some other were following this pattern of throwing away the actual error, e.g. hover.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've been trying to output the error in the debug! log, but not in the failure message we send back to the LSP

}
}
}

pub fn execute_command<O: Output>(&self, id: usize, params: ExecuteCommandParams, out: O) {
match &*params.command {
"rls.applySuggestion" => {
Expand Down
3 changes: 3 additions & 0 deletions src/build.rs
Original file line number Diff line number Diff line change
Expand Up @@ -570,6 +570,7 @@ impl BuildQueue {
let analysis = self.analysis.clone();

result.after_analysis.callback = Box::new(move |state| {
let borrow_analysis = state.borrow_analysis_map.take().unwrap();
// There are two ways to move the data from rustc to the RLS, either
// directly or by serialising and deserialising. We only want to do
// the latter when there are compatibility issues between crates.
Expand All @@ -578,6 +579,7 @@ impl BuildQueue {
// save::process_crate(state.tcx.unwrap(),
// state.expanded_crate.unwrap(),
// state.analysis.unwrap(),
// borrow_analysis,
// state.crate_name.unwrap(),
// save::DumpHandler::new(save::Format::Json,
// state.out_dir,
Expand All @@ -586,6 +588,7 @@ impl BuildQueue {
save::process_crate(state.tcx.unwrap(),
state.expanded_crate.unwrap(),
state.analysis.unwrap(),
borrow_analysis,
state.crate_name.unwrap(),
CallbackHandler {
callback: &mut |a| {
Expand Down
23 changes: 23 additions & 0 deletions src/cmd.rs
Original file line number Diff line number Diff line change
Expand Up @@ -76,6 +76,12 @@ pub fn run() {
let col = bits.next().expect("Expected column number");
hover(file_name, row, col)
}
"borrows" => {
let file_name = bits.next().expect("Expected file name");
let row = bits.next().expect("Expected line number");
let col = bits.next().expect("Expected column number");
borrow_info(file_name, row, col)
}
"h" | "help" => {
help();
continue;
Expand Down Expand Up @@ -127,6 +133,19 @@ fn rename(file_name: &str, row: &str, col: &str, new_name: &str) -> ServerMessag
ServerMessage::Request(request)
}

fn borrow_info(file_name: &str, row: &str, col: &str) -> ServerMessage {
let params = TextDocumentPositionParams {
text_document: TextDocumentIdentifier::new(url(file_name)),
position: Position::new(u64::from_str(row).expect("Bad line number"),
u64::from_str(col).expect("Bad column number")),
};
let request = Request {
id: next_id(),
method: Method::BorrowInfo(params),
};
ServerMessage::Request(request)
}

fn hover(file_name: &str, row: &str, col: &str) -> ServerMessage {
let params = TextDocumentPositionParams {
text_document: TextDocumentIdentifier::new(url(file_name)),
Expand Down Expand Up @@ -258,4 +277,8 @@ fn help() {
println!(" hover file_name line_number column_number");
println!(" textDocument/hover");
println!(" used for 'hover'");
println!("");
println!(" borrows file_name line_number column_number");
println!(" rustDocument/borrowInfo");
println!(" used for 'borrow info'");
}
77 changes: 77 additions & 0 deletions src/custom_types.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,77 @@
use lsp_data::Range;
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since the language server types crate seemed to only be for types that are part of the spec, this seemed to be the best place for these structs. Are there any concerns with this setup?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not clear what the difference between these types and the rls-data types is? Ideally we'd just use one set of types. If that is not possible, then this setup seems fine.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If I remember correctly it's the serialization derives. One is RustcSerialize and the other is Serializable. I'll double check after work.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We ought to be able to derive both if we need to (I think we have feature gates for that, or maybe that is span and we could add them for data)

use lsp_data::ls_util::rls_to_range;
use std::convert::From;
use analysis;

#[derive(Debug, Serialize, Deserialize)]
pub struct BorrowData {
pub scopes: Vec<Scope>,
pub loans: Vec<Loan>,
pub moves: Vec<Move>,
}

impl From<analysis::BorrowData> for BorrowData {
fn from(borrows: analysis::BorrowData) -> BorrowData {
BorrowData {
scopes: borrows.scopes.into_iter().map(|a| a.into()).collect(),
moves: borrows.moves.into_iter().map(|m| m.into()).collect(),
loans: borrows.loans.into_iter().map(|l| l.into()).collect(),
}
}
}

#[derive(Debug, Serialize, Deserialize)]
pub enum BorrowKind {
ImmBorrow,
MutBorrow,
}

impl From<analysis::BorrowKind> for BorrowKind {
fn from(kind: analysis::BorrowKind) -> BorrowKind {
match kind {
analysis::BorrowKind::ImmBorrow => BorrowKind::ImmBorrow,
analysis::BorrowKind::MutBorrow => BorrowKind::MutBorrow,
}
}
}

#[derive(Debug, Serialize, Deserialize)]
pub struct Loan {
pub kind: BorrowKind,
pub range: Range,
}

impl From<analysis::Loan> for Loan {
fn from(loan: analysis::Loan) -> Loan {
Loan {
kind: loan.kind.into(),
range: rls_to_range(loan.span.range),
}
}
}

#[derive(Debug, Serialize, Deserialize)]
pub struct Move {
pub range: Range,
}

impl From<analysis::Move> for Move {
fn from(mov: analysis::Move) -> Move {
Move {
range: rls_to_range(mov.span.range),
}
}
}

#[derive(Debug, Serialize, Deserialize)]
pub struct Scope {
pub range: Range,
}

impl From<analysis::Scope> for Scope {
fn from(scope: analysis::Scope) -> Scope {
Scope {
range: rls_to_range(scope.span.range),
}
}
}
1 change: 1 addition & 0 deletions src/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,7 @@ mod cmd;
mod config;
mod lsp_data;
mod server;
mod custom_types;

#[cfg(test)]
mod test;
Expand Down
4 changes: 4 additions & 0 deletions src/server.rs
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ use serde_json;
use build::*;
use lsp_data::*;
use actions::ActionHandler;
use custom_types::BorrowData;

use std::fmt;
use std::io::{self, Read, Write, ErrorKind};
Expand Down Expand Up @@ -93,6 +94,7 @@ serializable_enum!(ResponseData,
Locations(Vec<Location>),
Highlights(Vec<DocumentHighlight>),
HoverSuccess(Hover),
BorrowInfo(BorrowData),
Commands(Vec<Command>),
Ack(Ack)
);
Expand Down Expand Up @@ -244,6 +246,7 @@ messages! {
"textDocument/references" => References(ReferenceParams);
"textDocument/completion" => Completion(TextDocumentPositionParams);
"textDocument/documentHighlight" => DocumentHighlight(TextDocumentPositionParams);
"rustDocument/borrowInfo" => BorrowInfo(TextDocumentPositionParams);
// currently, we safely ignore this as a pass-through since we fully handle
// textDocument/completion. In the future, we may want to use this method as a
// way to more lazily fill out completion information
Expand Down Expand Up @@ -442,6 +445,7 @@ impl<O: Output> LsService<O> {
References(params) => { action: find_all_refs };
Completion(params) => { action: complete };
DocumentHighlight(params) => { action: highlight };
BorrowInfo(params) => { action: borrow_info };
ResolveCompletionItem(params) => {
this.output.success(id, ResponseData::CompletionItems(vec![params]))
};
Expand Down