diff --git a/Cargo.lock b/Cargo.lock index adbf200c9cf..7bd1e42c40d 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -3,8 +3,8 @@ name = "rls" version = "0.1.0" dependencies = [ "cargo 0.21.0 (git+https://github.com/rust-lang/cargo)", - "derive-new 0.3.0 (registry+https://github.com/rust-lang/crates.io-index)", "env_logger 0.4.3 (registry+https://github.com/rust-lang/crates.io-index)", + "jsonrpc-core 7.0.1 (registry+https://github.com/rust-lang/crates.io-index)", "languageserver-types 0.11.1 (registry+https://github.com/rust-lang/crates.io-index)", "log 0.3.8 (registry+https://github.com/rust-lang/crates.io-index)", "racer 2.0.9 (registry+https://github.com/rust-lang/crates.io-index)", @@ -322,6 +322,11 @@ dependencies = [ "winapi 0.2.8 (registry+https://github.com/rust-lang/crates.io-index)", ] +[[package]] +name = "futures" +version = "0.1.14" +source = "registry+https://github.com/rust-lang/crates.io-index" + [[package]] name = "gcc" version = "0.3.51" @@ -390,6 +395,18 @@ dependencies = [ "rand 0.3.15 (registry+https://github.com/rust-lang/crates.io-index)", ] +[[package]] +name = "jsonrpc-core" +version = "7.0.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +dependencies = [ + "futures 0.1.14 (registry+https://github.com/rust-lang/crates.io-index)", + "log 0.3.8 (registry+https://github.com/rust-lang/crates.io-index)", + "serde 1.0.9 (registry+https://github.com/rust-lang/crates.io-index)", + "serde_derive 1.0.9 (registry+https://github.com/rust-lang/crates.io-index)", + "serde_json 1.0.2 (registry+https://github.com/rust-lang/crates.io-index)", +] + [[package]] name = "kernel32-sys" version = "0.2.2" @@ -1108,6 +1125,7 @@ dependencies = [ "checksum flate2 0.2.19 (registry+https://github.com/rust-lang/crates.io-index)" = "36df0166e856739905cd3d7e0b210fe818592211a008862599845e012d8d304c" "checksum foreign-types 0.2.0 (registry+https://github.com/rust-lang/crates.io-index)" = "3e4056b9bd47f8ac5ba12be771f77a0dae796d1bbaaf5fd0b9c2d38b69b8a29d" "checksum fs2 0.4.2 (registry+https://github.com/rust-lang/crates.io-index)" = "9ab76cfd2aaa59b7bf6688ad9ba15bbae64bff97f04ea02144cfd3443e5c2866" +"checksum futures 0.1.14 (registry+https://github.com/rust-lang/crates.io-index)" = "4b63a4792d4f8f686defe3b39b92127fea6344de5d38202b2ee5a11bbbf29d6a" "checksum gcc 0.3.51 (registry+https://github.com/rust-lang/crates.io-index)" = "120d07f202dcc3f72859422563522b66fe6463a4c513df062874daad05f85f0a" "checksum getopts 0.2.14 (registry+https://github.com/rust-lang/crates.io-index)" = "d9047cfbd08a437050b363d35ef160452c5fe8ea5187ae0a624708c91581d685" "checksum git2 0.6.6 (registry+https://github.com/rust-lang/crates.io-index)" = "aa01936ac96555c083c0e8553f672616274408d9d3fc5b8696603fbf63ff43ee" @@ -1117,6 +1135,7 @@ dependencies = [ "checksum idna 0.1.2 (registry+https://github.com/rust-lang/crates.io-index)" = "2233d4940b1f19f0418c158509cd7396b8d70a5db5705ce410914dc8fa603b37" "checksum itoa 0.3.1 (registry+https://github.com/rust-lang/crates.io-index)" = "eb2f404fbc66fd9aac13e998248505e7ecb2ad8e44ab6388684c5fb11c6c251c" "checksum jobserver 0.1.5 (registry+https://github.com/rust-lang/crates.io-index)" = "4e28adc987f6d0521ef66ad60b055968107b164b3bb3cf3dc8474e0a380474a6" +"checksum jsonrpc-core 7.0.1 (registry+https://github.com/rust-lang/crates.io-index)" = "da622868a84d3f4fd897f6408ba6714aabf663302802358564b384157c1a5bfa" "checksum kernel32-sys 0.2.2 (registry+https://github.com/rust-lang/crates.io-index)" = "7507624b29483431c0ba2d82aece8ca6cdba9382bff4ddd0f7490560c056098d" "checksum languageserver-types 0.11.1 (registry+https://github.com/rust-lang/crates.io-index)" = "680aee78c75504fdcb172635a7b7da0dccaafa4c42d935e19576c14b27942362" "checksum lazy_static 0.2.8 (registry+https://github.com/rust-lang/crates.io-index)" = "3b37545ab726dd833ec6420aaba8231c5b320814b9029ad585555d2a03e94fbf" diff --git a/Cargo.toml b/Cargo.toml index 6dc9b66e9f6..b1dbfc4d17e 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -11,7 +11,6 @@ build = "build.rs" [dependencies] cargo = { git = "https://github.com/rust-lang/cargo" } -derive-new = "0.3" env_logger = "0.4" languageserver-types = "0.11.1" log = "0.3" @@ -27,3 +26,4 @@ serde_derive = "1.0" toml = "0.4" url = "1.1.0" url_serde = "0.2.0" +jsonrpc-core = "7.0.1" diff --git a/src/actions/mod.rs b/src/actions/mod.rs index dc2d5527057..dace1e83568 100644 --- a/src/actions/mod.rs +++ b/src/actions/mod.rs @@ -24,6 +24,7 @@ use Span; use build::*; use lsp_data::*; use server::{ResponseData, Output, Ack}; +use jsonrpc_core::types::ErrorCode; use std::collections::HashMap; use std::panic; @@ -401,7 +402,7 @@ impl ActionHandler { } _ => { debug!("Error in Racer"); - out.failure(id, "GotoDef failed to complete successfully"); + out.failure_message(id, ErrorCode::InternalError, "GotoDef failed to complete successfully"); } } } @@ -445,7 +446,7 @@ impl ActionHandler { out.success(id, ResponseData::HoverSuccess(r)); } Err(_) => { - out.failure(id, "Hover failed to complete successfully"); + out.failure_message(id, ErrorCode::InternalError, "Hover failed to complete successfully"); } } } @@ -459,7 +460,7 @@ impl ActionHandler { } c => { debug!("Unknown command: {}", c); - out.failure(id, "Unknown command"); + out.failure_message(id, ErrorCode::MethodNotFound, "Unknown command"); } } } @@ -516,21 +517,20 @@ impl ActionHandler { // Start by checking that the user has selected a glob import. if span.range.start() == span.range.end() { - out.failure(id, "Empty selection"); + out.failure_message(id, ErrorCode::InvalidParams, "Empty selection"); return; } match self.vfs.load_span(span.clone()) { - Ok(s) => { - if s != "*" { - out.failure(id, "Not a glob"); - return; - } + Ok(ref s) if s != "*" => { + out.failure_message(id, ErrorCode::InvalidParams, "Not a glob"); + return; } Err(e) => { debug!("Deglob failed: {:?}", e); - out.failure(id, "Couldn't open file"); + out.failure_message(id, ErrorCode::InternalError, "Couldn't open file"); return; } + _ => {} } // Save-analysis exports the deglobbed version of a glob import as its type string. @@ -548,7 +548,7 @@ impl ActionHandler { let mut deglob_str = match result { Ok(Ok(s)) => s, _ => { - out.failure(id, "Couldn't get info from analysis"); + out.failure_message(id, ErrorCode::InternalError, "Couldn't get info from analysis"); return; } }; @@ -579,12 +579,12 @@ impl ActionHandler { Ok(FileContents::Text(s)) => FmtInput::Text(s), Ok(_) => { debug!("Reformat failed, found binary file"); - out.failure(id, "Reformat failed to complete successfully"); + out.failure_message(id, ErrorCode::InternalError, "Reformat failed to complete successfully"); return; } Err(e) => { debug!("Reformat failed: {:?}", e); - out.failure(id, "Reformat failed to complete successfully"); + out.failure_message(id, ErrorCode::InternalError, "Reformat failed to complete successfully"); return; } }; @@ -626,12 +626,12 @@ impl ActionHandler { } else { debug!("reformat: format_input failed: has errors, summary = {:?}", summary); - out.failure(id, "Reformat failed to complete successfully"); + out.failure_message(id, ErrorCode::InternalError, "Reformat failed to complete successfully"); } } Err(e) => { debug!("Reformat failed: {:?}", e); - out.failure(id, "Reformat failed to complete successfully"); + out.failure_message(id, ErrorCode::InternalError, "Reformat failed to complete successfully"); } } } diff --git a/src/cmd.rs b/src/cmd.rs index 5128878ad01..5e1eeb7c632 100644 --- a/src/cmd.rs +++ b/src/cmd.rs @@ -14,8 +14,9 @@ use analysis::{AnalysisHost, Target}; use config::Config; -use server::{self, ServerMessage, Request, Notification, Method, LsService, ParseError, ResponseData}; +use server::{self, ServerMessage, Request, Notification, Method, LsService, ResponseData}; use vfs::Vfs; +use jsonrpc_core; use ls_types::{ClientCapabilities, TextDocumentPositionParams, TextDocumentIdentifier, TraceOption, Position, InitializeParams, RenameParams}; use std::time::Duration; @@ -54,7 +55,7 @@ pub fn run() { Some(a) => a, None => continue, }; - + // Switch on the action and build an appropriate message. let msg = match action { "def" => { @@ -216,7 +217,7 @@ impl ChannelMsgReader { } impl server::MessageReader for ChannelMsgReader { - fn parsed_message(&self) -> Option> { + fn parsed_message(&self) -> Option>> { let channel = self.channel.lock().unwrap(); let msg = channel.recv().expect("Error reading from channel"); Some(Ok(msg)) diff --git a/src/main.rs b/src/main.rs index 363af490c9b..9256f045e40 100644 --- a/src/main.rs +++ b/src/main.rs @@ -15,8 +15,6 @@ #![feature(fnbox)] extern crate cargo; -#[macro_use] -extern crate derive_new; extern crate env_logger; extern crate languageserver_types as ls_types; #[macro_use] @@ -40,6 +38,7 @@ extern crate serde_json; extern crate toml; extern crate url; extern crate url_serde; +extern crate jsonrpc_core; use std::sync::Arc; diff --git a/src/server.rs b/src/server.rs index 49f664c6ebd..9351fce856e 100644 --- a/src/server.rs +++ b/src/server.rs @@ -17,11 +17,21 @@ use actions::ActionHandler; use config::Config; use std::fmt; -use std::io::{self, Read, Write, ErrorKind}; +use std::io::{self, Read, Write}; use std::sync::{Arc, Mutex}; use std::sync::atomic::{AtomicBool, Ordering, AtomicU32}; use std::path::PathBuf; +use jsonrpc_core::{self as jsonrpc, Id, response, version}; + +pub fn server_failure(id: jsonrpc::Id, error: jsonrpc::Error) -> jsonrpc::Failure { + jsonrpc::Failure { + jsonrpc: Some(version::Version::V2), + id, + error, + } +} + #[cfg(test)] #[allow(non_upper_case_globals)] pub const REQUEST__Deglob: &'static str = "rustWorkspace/deglob"; @@ -29,13 +39,6 @@ pub const REQUEST__Deglob: &'static str = "rustWorkspace/deglob"; #[derive(Debug, Serialize)] pub struct Ack; -#[derive(Debug, new)] -pub struct ParseError { - pub kind: ErrorKind, - pub message: &'static str, - pub id: Option, -} - #[derive(Debug)] pub enum ServerMessage { Request(Request), @@ -113,43 +116,56 @@ macro_rules! messages { pub enum Method { $($method_name$(($method_arg))*,)* } - fn parse_message(input: &str) -> Result { + + // Notifications can't return a response, hence why Err is an Option + fn parse_message(input: &str) -> Result> { trace!("parse_message `{}`", input); - let ls_command: serde_json::Value = serde_json::from_str(input).expect("Can't make value"); + let ls_command: serde_json::Value = match serde_json::from_str(input) { + Ok(value) => value, + Err(_) => return Err(Some(server_failure(Id::Null, jsonrpc::Error::parse_error()))), + }; + + // Per JSON-RPC/LSP spec, Requests must have id, whereas Notifications can't + let id = ls_command.get("id").map(|id| serde_json::from_value(id.to_owned()).unwrap()); + // TODO: We only support numeric responses, ideally we should switch from using parsed usize + // to using jsonrpc_core::Id + let parsed_numeric_id = match &id { + &Some(Id::Num(n)) => Some(n as usize), + &Some(Id::Str(ref s)) => usize::from_str_radix(s, 10).ok(), + _ => None, + }; let params = ls_command.get("params"); macro_rules! params_as { ($ty: ty) => ({ - let method: $ty = - serde_json::from_value(params.expect("no params").to_owned()).expect("Can't extract params"); + let params = match params { + Some(value) => value, + None => return Err(Some(server_failure(id.unwrap_or(Id::Null), jsonrpc::Error::invalid_request()))), + }; + + let method: $ty = match serde_json::from_value(params.to_owned()) { + Ok(value) => value, + Err(_) => return Err(Some(server_failure(id.unwrap_or(Id::Null), + jsonrpc::Error::invalid_params(format!("Expected {}", stringify!($ty)))))), + }; method }); } - let id: Option = match ls_command.get("id") { - Some(v) => { - if v.is_u64() { - Some(v.as_u64().unwrap() as usize) - } else if v.is_string() { - match usize::from_str_radix(v.as_str().unwrap(), 10) { - Ok(v) => Some(v), - Err(_) => None - } - } else { - None - } - }, - None => None - }; if let Some(v) = ls_command.get("method") { if let Some(name) = v.as_str() { match name { $( $method_str => { - match id { + match parsed_numeric_id { Some(id) => Ok(ServerMessage::Request(Request{id, method: Method::$method_name$((params_as!($method_arg)))* })), - None => Err(ParseError::new(ErrorKind::InvalidData, "Id is not a number or numeric string", None)), + None => match id { + None => Err(Some(server_failure(Id::Null, jsonrpc::Error::invalid_request()))), + // FIXME: This behaviour is custom and non conformant to the protocol + Some(id) => Err(Some(server_failure(id, + jsonrpc::Error::invalid_params("Id is not a number or numeric string")))), + } } } )* @@ -159,14 +175,18 @@ macro_rules! messages { } )* $( - $other_str => $other_expr, + // If $other_expr is Err, then we need to pass id of actual message we're handling + $other_str => { ($other_expr).map_err(|err| id.map(|id| server_failure(id, err))) } )* } } else { - Err(ParseError::new(ErrorKind::InvalidData, "Method is not a string", id)) + // Message has a "method" field, so it can be a Notification/Request - if it doesn't have id then we + // assume it's a Notification for which we can't return a response, so return Err(None) + Err(id.map(|id| server_failure(id, jsonrpc::Error::invalid_request()))) } } else { - Err(ParseError::new(ErrorKind::InvalidData, "Method not found", id)) + // FIXME: Handle possible client responses to server->client requests (which don't have "method" field) + Err(Some(server_failure(id.unwrap_or(Id::Null), jsonrpc::Error::invalid_request()))) } } @@ -277,9 +297,7 @@ messages! { "$/cancelRequest" => Cancel(CancelParams); "workspace/didChangeConfiguration" => WorkspaceChangeConfiguration(DidChangeConfigurationParams); } - // TODO handle me - "$/setTraceNotification" => Err(ParseError::new(ErrorKind::InvalidData, "setTraceNotification", None)); - _ => Err(ParseError::new(ErrorKind::InvalidData, "Unknown command", None)); + _ => Err(jsonrpc::Error::method_not_found()); // TODO: Handle more possible messages } pub struct LsService { @@ -410,8 +428,8 @@ impl LsService { }, Err(e) => { trace!("parsing invalid message: {:?}", e); - if let Some(id) = e.id { - this.output.failure(id, "Unsupported message"); + if let Some(failure) = e { + this.output.failure(failure.id, failure.error); } }, } @@ -421,7 +439,7 @@ impl LsService { let message = match this.msg_reader.parsed_message() { Some(m) => m, None => { - this.output.parse_error(); + this.output.failure(Id::Null, jsonrpc::Error::parse_error()); return ServerStateChange::Break }, }; @@ -488,7 +506,7 @@ pub trait MessageReader { None } - fn parsed_message(&self) -> Option> { + fn parsed_message(&self) -> Option>> { self.read_message().map(|m| parse_message(&m)) } } @@ -551,37 +569,23 @@ pub trait Output: Sync + Send + Clone + 'static { fn response(&self, output: String); fn provide_id(&self) -> u32; - fn parse_error(&self) { - self.response(r#"{"jsonrpc": "2.0", "error": {"code": -32700, "message": "Parse error"}, "id": null}"#.to_owned()); - } - - fn failure(&self, id: usize, message: &str) { - // For now this is a catch-all for any error back to the consumer of the RLS - const METHOD_NOT_FOUND: i64 = -32601; - - #[derive(Serialize)] - struct ResponseError { - code: i64, - message: String - } + fn failure(&self, id: jsonrpc::Id, error: jsonrpc::Error) { + let response = response::Failure { + jsonrpc: Some(version::Version::V2), + id: id, + error: error + }; - #[derive(Serialize)] - struct ResponseFailure { - jsonrpc: &'static str, - id: usize, - error: ResponseError, - } + self.response(serde_json::to_string(&response).unwrap()); + } - let rf = ResponseFailure { - jsonrpc: "2.0", - id: id, - error: ResponseError { - code: METHOD_NOT_FOUND, - message: message.to_owned(), - }, + fn failure_message>(&self, id: usize, code: jsonrpc::ErrorCode, msg: M) { + let error = jsonrpc::Error { + code: code, + message: msg.into(), + data: None }; - let output = serde_json::to_string(&rf).unwrap(); - self.response(output); + self.failure(Id::Num(id as u64), error); } fn success(&self, id: usize, data: ResponseData) { diff --git a/src/test/mod.rs b/src/test/mod.rs index 500aa858538..e80f467e579 100644 --- a/src/test/mod.rs +++ b/src/test/mod.rs @@ -15,10 +15,12 @@ mod harness; use std::sync::{Arc, Mutex}; use env_logger; +use serde_json; use analysis; use config::Config; use server::{self as ls_server, ServerMessage, Request, Method}; +use jsonrpc_core; use vfs; use self::harness::{expect_messages, ExpectedMessage, init_env, mock_server, mock_server_with_config, RecordOutput, src}; @@ -459,5 +461,9 @@ fn test_parse_error_on_malformed_input() { let error = results.lock().unwrap() .pop().expect("no error response"); - assert!(error.contains(r#""code": -32700"#)) + + let failure: jsonrpc_core::Failure = serde_json::from_str(&error) + .expect("Couldn't parse json failure response"); + + assert!(failure.error.code == jsonrpc_core::ErrorCode::ParseError); }