From fe2bf9009ec82d32139866b698854d4d235f94d6 Mon Sep 17 00:00:00 2001 From: Igor Matuszewski Date: Mon, 10 Jul 2017 01:43:20 +0200 Subject: [PATCH] Fix rebase and apply feedback --- Cargo.lock | 4 ++-- src/actions/mod.rs | 22 +++++++++++----------- src/cmd.rs | 2 +- src/server.rs | 27 +++++++++++++-------------- src/test/mod.rs | 3 +-- 5 files changed, 28 insertions(+), 30 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index a1736f7ded7..7bd1e42c40d 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -402,8 +402,8 @@ 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.8 (registry+https://github.com/rust-lang/crates.io-index)", - "serde_derive 1.0.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)", ] diff --git a/src/actions/mod.rs b/src/actions/mod.rs index 010c1ab691e..ee3bd125507 100644 --- a/src/actions/mod.rs +++ b/src/actions/mod.rs @@ -405,7 +405,7 @@ impl ActionHandler { } _ => { debug!("Error in Racer"); - out.failure_simple(id, ErrorCode::InternalError, "GotoDef failed to complete successfully"); + out.failure_message(id, ErrorCode::InternalError, "GotoDef failed to complete successfully"); } } } @@ -449,7 +449,7 @@ impl ActionHandler { out.success(id, ResponseData::HoverSuccess(r)); } Err(_) => { - out.failure_simple(id, ErrorCode::InternalError, "Hover failed to complete successfully"); + out.failure_message(id, ErrorCode::InternalError, "Hover failed to complete successfully"); } } } @@ -463,7 +463,7 @@ impl ActionHandler { } c => { debug!("Unknown command: {}", c); - out.failure_simple(id, ErrorCode::MethodNotFound, "Unknown command"); + out.failure_message(id, ErrorCode::MethodNotFound, "Unknown command"); } } } @@ -520,17 +520,17 @@ impl ActionHandler { // Start by checking that the user has selected a glob import. if span.range.start() == span.range.end() { - out.failure_simple(id, ErrorCode::InvalidParams, "Empty selection"); + out.failure_message(id, ErrorCode::InvalidParams, "Empty selection"); return; } match self.vfs.load_span(span.clone()) { Ok(ref s) if s != "*" => { - out.failure_simple(id, ErrorCode::InvalidParams, "Not a glob"); + out.failure_message(id, ErrorCode::InvalidParams, "Not a glob"); return; } Err(e) => { debug!("Deglob failed: {:?}", e); - out.failure_simple(id, ErrorCode::InternalError, "Couldn't open file"); + out.failure_message(id, ErrorCode::InternalError, "Couldn't open file"); return; } _ => {} @@ -551,7 +551,7 @@ impl ActionHandler { let mut deglob_str = match result { Ok(Ok(s)) => s, _ => { - out.failure_simple(id, ErrorCode::InternalError, "Couldn't get info from analysis"); + out.failure_message(id, ErrorCode::InternalError, "Couldn't get info from analysis"); return; } }; @@ -582,12 +582,12 @@ impl ActionHandler { Ok(FileContents::Text(s)) => FmtInput::Text(s), Ok(_) => { debug!("Reformat failed, found binary file"); - out.failure_simple(id, ErrorCode::InternalError, "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_simple(id, ErrorCode::InternalError, "Reformat failed to complete successfully"); + out.failure_message(id, ErrorCode::InternalError, "Reformat failed to complete successfully"); return; } }; @@ -629,12 +629,12 @@ impl ActionHandler { } else { debug!("reformat: format_input failed: has errors, summary = {:?}", summary); - out.failure_simple(id, ErrorCode::InternalError, "Reformat failed to complete successfully"); + out.failure_message(id, ErrorCode::InternalError, "Reformat failed to complete successfully"); } } Err(e) => { debug!("Reformat failed: {:?}", e); - out.failure_simple(id, ErrorCode::InternalError, "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 1d312e8e68c..251bfcac07f 100644 --- a/src/cmd.rs +++ b/src/cmd.rs @@ -14,7 +14,7 @@ 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; diff --git a/src/server.rs b/src/server.rs index a1aa6e4bee4..06ad0fa4353 100644 --- a/src/server.rs +++ b/src/server.rs @@ -27,8 +27,8 @@ 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: id, - error: error, + id, + error, } } @@ -117,6 +117,8 @@ macro_rules! messages { $($method_name$(($method_arg))*,)* } + // TODO: Change return type to Result>, + // since Notifications don't have id and only Requests can return a response 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"); @@ -131,14 +133,11 @@ macro_rules! messages { }); } - let id = ls_command.get("id").expect("no id").to_owned(); - let id: jsonrpc::Id = serde_json::from_value(id).expect("Can't extract id"); - - // FIXME: Currently we only support numbers as strings - // TODO: Change used id from usize to u64 or ideally jsonrpc::Id + // 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()); let parsed_numeric_id = match &id { - &Id::Num(ref num) => Some(num.clone() as usize), - &Id::Str(ref string) => usize::from_str_radix(string, 10).ok(), + &Some(Id::Num(n)) => Some(n as usize), + &Some(Id::Str(ref s)) => usize::from_str_radix(s, 10).ok(), _ => None, }; @@ -149,7 +148,7 @@ macro_rules! messages { $method_str => { match parsed_numeric_id { Some(id) => Ok(ServerMessage::Request(Request{id, method: Method::$method_name$((params_as!($method_arg)))* })), - None => Err(server_failure(id, jsonrpc::Error::invalid_params("Id is not a number or numeric string"))), + None => Err(server_failure(id.unwrap_or(Id::Null), jsonrpc::Error::invalid_params("Id is not a number or numeric string"))), } } )* @@ -160,14 +159,14 @@ macro_rules! messages { )* $( // If $other_expr is Err, then we need to pass id of actual message we're handling - $other_str => { ($other_expr).map_err(|err| server_failure(id, err)) } + $other_str => { ($other_expr).map_err(|err| server_failure(id.unwrap_or(Id::Null), err)) } )* } } else { - Err(server_failure(id, jsonrpc::Error::invalid_params("Method is not a string"))) + Err(server_failure(id.unwrap_or(Id::Null), jsonrpc::Error::invalid_params("Method is not a string"))) } } else { - Err(server_failure(id, jsonrpc::Error::method_not_found())) + Err(server_failure(id.unwrap_or(Id::Null), jsonrpc::Error::method_not_found())) } } @@ -566,7 +565,7 @@ pub trait Output: Sync + Send + Clone + 'static { self.response(serde_json::to_string(&response).unwrap()); } - fn failure_simple>(&self, id: usize, code: jsonrpc::ErrorCode, msg: M) { + fn failure_message>(&self, id: usize, code: jsonrpc::ErrorCode, msg: M) { let error = jsonrpc::Error { code: code, message: msg.into(), diff --git a/src/test/mod.rs b/src/test/mod.rs index a464362c182..e80f467e579 100644 --- a/src/test/mod.rs +++ b/src/test/mod.rs @@ -465,6 +465,5 @@ fn test_parse_error_on_malformed_input() { let failure: jsonrpc_core::Failure = serde_json::from_str(&error) .expect("Couldn't parse json failure response"); - const PARSE_ERROR: jsonrpc_core::ErrorCode = jsonrpc_core::ErrorCode::ParseError; - assert!(failure.error.code == PARSE_ERROR); + assert!(failure.error.code == jsonrpc_core::ErrorCode::ParseError); }