diff --git a/crates/rust-analyzer/src/from_proto.rs b/crates/rust-analyzer/src/from_proto.rs index 28f3fa2786..a95836edda 100644 --- a/crates/rust-analyzer/src/from_proto.rs +++ b/crates/rust-analyzer/src/from_proto.rs @@ -10,7 +10,9 @@ use crate::{ from_json, global_state::GlobalStateSnapshot, line_index::{LineIndex, OffsetEncoding}, - lsp_ext, LspError, Result, + lsp_ext, + lsp_utils::invalid_params_error, + Result, }; pub(crate) fn abs_path(url: &lsp_types::Url) -> Result { @@ -85,10 +87,8 @@ pub(crate) fn annotation( snap: &GlobalStateSnapshot, code_lens: lsp_types::CodeLens, ) -> Result { - let data = code_lens.data.ok_or_else(|| LspError { - code: lsp_server::ErrorCode::InvalidParams as i32, - message: "code lens without data".to_string(), - }); + let data = + code_lens.data.ok_or_else(|| invalid_params_error("code lens without data".to_string()))?; let resolve = from_json::("CodeLensResolveData", data)?; match resolve { diff --git a/crates/rust-analyzer/src/handlers.rs b/crates/rust-analyzer/src/handlers.rs index ed912cb160..9de059a2c3 100644 --- a/crates/rust-analyzer/src/handlers.rs +++ b/crates/rust-analyzer/src/handlers.rs @@ -40,7 +40,7 @@ use crate::{ self, InlayHint, InlayHintsParams, PositionOrRange, ViewCrateGraphParams, WorkspaceSymbolParams, }, - lsp_utils::all_edits_are_disjoint, + lsp_utils::{all_edits_are_disjoint, invalid_params_error}, to_proto, LspError, Result, }; @@ -767,9 +767,8 @@ pub(crate) fn handle_completion_resolve( let _p = profile::span("handle_completion_resolve"); if !all_edits_are_disjoint(&original_completion, &[]) { - return Err(LspError::new( - ErrorCode::InvalidParams as i32, - "Received a completion with overlapping edits, this is not LSP-compliant".into(), + return Err(invalid_params_error( + "Received a completion with overlapping edits, this is not LSP-compliant".to_string(), ) .into()); } @@ -1038,10 +1037,7 @@ pub(crate) fn handle_code_action_resolve( let _p = profile::span("handle_code_action_resolve"); let params = match code_action.data.take() { Some(it) => it, - None => Err(LspError { - code: lsp_server::ErrorCode::InvalidParams as i32, - message: format!("code action without data"), - })?, + None => return Err(invalid_params_error(format!("code action without data")).into()), }; let file_id = from_proto::file_id(&snap, ¶ms.code_action_params.text_document.uri)?; @@ -1059,10 +1055,10 @@ pub(crate) fn handle_code_action_resolve( let (assist_index, assist_resolve) = match parse_action_id(¶ms.id) { Ok(parsed_data) => parsed_data, Err(e) => { - return Err(LspError::new( - ErrorCode::InvalidParams as i32, - format!("Failed to parse action id string '{}': {}", params.id, e), - ) + return Err(invalid_params_error(format!( + "Failed to parse action id string '{}': {}", + params.id, e + )) .into()) } }; @@ -1079,23 +1075,17 @@ pub(crate) fn handle_code_action_resolve( let assist = match assists.get(assist_index) { Some(assist) => assist, - None => return Err(LspError::new( - ErrorCode::InvalidParams as i32, - format!( - "Failed to find the assist for index {} provided by the resolve request. Resolve request assist id: {}", - assist_index, params.id, - ), - ) + None => return Err(invalid_params_error(format!( + "Failed to find the assist for index {} provided by the resolve request. Resolve request assist id: {}", + assist_index, params.id, + )) .into()) }; if assist.id.0 != expected_assist_id || assist.id.1 != expected_kind { - return Err(LspError::new( - ErrorCode::InvalidParams as i32, - format!( - "Mismatching assist at index {} for the resolve parameters given. Resolve request assist id: {}, actual id: {:?}.", - assist_index, params.id, assist.id - ), - ) + return Err(invalid_params_error(format!( + "Mismatching assist at index {} for the resolve parameters given. Resolve request assist id: {}, actual id: {:?}.", + assist_index, params.id, assist.id + )) .into()); } let edit = to_proto::code_action(&snap, assist.clone(), None)?.edit; diff --git a/crates/rust-analyzer/src/lsp_utils.rs b/crates/rust-analyzer/src/lsp_utils.rs index 29fa3e2d0e..09178521b5 100644 --- a/crates/rust-analyzer/src/lsp_utils.rs +++ b/crates/rust-analyzer/src/lsp_utils.rs @@ -8,8 +8,13 @@ use crate::{ from_proto, global_state::GlobalState, line_index::{LineEndings, LineIndex, OffsetEncoding}, + LspError, }; +pub(crate) fn invalid_params_error(message: String) -> LspError { + LspError { code: lsp_server::ErrorCode::InvalidParams as i32, message } +} + pub(crate) fn is_cancelled(e: &(dyn Error + 'static)) -> bool { e.downcast_ref::().is_some() } diff --git a/crates/rust-analyzer/src/to_proto.rs b/crates/rust-analyzer/src/to_proto.rs index 8dcfff5c71..277476981f 100644 --- a/crates/rust-analyzer/src/to_proto.rs +++ b/crates/rust-analyzer/src/to_proto.rs @@ -21,7 +21,9 @@ use crate::{ config::Config, global_state::GlobalStateSnapshot, line_index::{LineEndings, LineIndex, OffsetEncoding}, - lsp_ext, semantic_tokens, Result, + lsp_ext, + lsp_utils::invalid_params_error, + semantic_tokens, Result, }; pub(crate) fn position(line_index: &LineIndex, offset: TextSize) -> lsp_types::Position { @@ -1198,7 +1200,9 @@ pub(crate) fn markup_content(markup: Markup) -> lsp_types::MarkupContent { } pub(crate) fn rename_error(err: RenameError) -> crate::LspError { - crate::LspError { code: lsp_server::ErrorCode::InvalidParams as i32, message: err.to_string() } + // This is wrong, but we don't have a better alternative I suppose? + // https://github.com/microsoft/language-server-protocol/issues/1341 + invalid_params_error(err.to_string()) } #[cfg(test)] diff --git a/docs/dev/style.md b/docs/dev/style.md index 6131cdcbdc..92e79508b6 100644 --- a/docs/dev/style.md +++ b/docs/dev/style.md @@ -895,6 +895,29 @@ fn foo() -> Option { **Rationale:** reduce cognitive stack usage. +Use `return Err(err)` to throw an error: + +```rust +// GOOD +fn f() -> Result<(), ()> { + if condition { + return Err(()); + } + Ok(()) +} + +// BAD +fn f() -> Result<(), ()> { + if condition { + Err(())?; + } + Ok(()) +} +``` + +**Rationale:** `return` has type `!`, which allows the compiler to flag dead +code (`Err(...)?` is of unconstrained generic type `T`). + ## Comparisons When doing multiple comparisons use `<`/`<=`, avoid `>`/`>=`.