From c05563d71b5684fbac57f26dce6b7688d2bf724b Mon Sep 17 00:00:00 2001 From: Jane Lewis Date: Fri, 12 Apr 2024 14:26:59 -0700 Subject: [PATCH 1/7] Generate Ruff-specific source actions --- .../src/server/api/requests/code_action.rs | 49 +++++++++++-------- 1 file changed, 29 insertions(+), 20 deletions(-) diff --git a/crates/ruff_server/src/server/api/requests/code_action.rs b/crates/ruff_server/src/server/api/requests/code_action.rs index 0d697b0e591961..d23f7b406370f9 100644 --- a/crates/ruff_server/src/server/api/requests/code_action.rs +++ b/crates/ruff_server/src/server/api/requests/code_action.rs @@ -41,13 +41,14 @@ impl super::BackgroundDocumentRequestHandler for CodeActions { if snapshot.client_settings().fix_all() && supported_code_actions.contains(&SupportedCodeAction::SourceFixAll) { - response.push(fix_all(&snapshot).with_failure_code(ErrorCode::InternalError)?); + response.extend(fix_all(&snapshot).with_failure_code(ErrorCode::InternalError)?); } if snapshot.client_settings().organize_imports() && supported_code_actions.contains(&SupportedCodeAction::SourceOrganizeImports) { - response.push(organize_imports(&snapshot).with_failure_code(ErrorCode::InternalError)?); + response + .extend(organize_imports(&snapshot).with_failure_code(ErrorCode::InternalError)?); } Ok(Some(response)) @@ -87,7 +88,7 @@ fn quick_fix( .collect() } -fn fix_all(snapshot: &DocumentSnapshot) -> crate::Result { +fn fix_all(snapshot: &DocumentSnapshot) -> crate::Result> { let document = snapshot.document(); let (edit, data) = if snapshot @@ -112,17 +113,21 @@ fn fix_all(snapshot: &DocumentSnapshot) -> crate::Result { None, ) }; - let action = types::CodeAction { - title: format!("{DIAGNOSTIC_NAME}: Fix all auto-fixable problems"), - kind: Some(types::CodeActionKind::SOURCE_FIX_ALL), - edit, - data, - ..Default::default() - }; - Ok(types::CodeActionOrCommand::CodeAction(action)) + Ok(SupportedCodeAction::SourceFixAll + .kinds() + .into_iter() + .map(move |kind| types::CodeAction { + title: format!("{DIAGNOSTIC_NAME}: Fix all auto-fixable problems"), + kind: Some(kind), + edit: edit.clone(), + data: data.clone(), + ..Default::default() + }) + .map(CodeActionOrCommand::CodeAction) + .collect()) } -fn organize_imports(snapshot: &DocumentSnapshot) -> crate::Result { +fn organize_imports(snapshot: &DocumentSnapshot) -> crate::Result> { let document = snapshot.document(); let (edit, data) = if snapshot @@ -147,14 +152,18 @@ fn organize_imports(snapshot: &DocumentSnapshot) -> crate::Result Date: Mon, 15 Apr 2024 11:39:53 -0700 Subject: [PATCH 2/7] Return only `source.*.ruff` code actions --- .../src/server/api/requests/code_action.rs | 49 ++++++++----------- 1 file changed, 20 insertions(+), 29 deletions(-) diff --git a/crates/ruff_server/src/server/api/requests/code_action.rs b/crates/ruff_server/src/server/api/requests/code_action.rs index d23f7b406370f9..3af967f137593a 100644 --- a/crates/ruff_server/src/server/api/requests/code_action.rs +++ b/crates/ruff_server/src/server/api/requests/code_action.rs @@ -41,14 +41,13 @@ impl super::BackgroundDocumentRequestHandler for CodeActions { if snapshot.client_settings().fix_all() && supported_code_actions.contains(&SupportedCodeAction::SourceFixAll) { - response.extend(fix_all(&snapshot).with_failure_code(ErrorCode::InternalError)?); + response.push(fix_all(&snapshot).with_failure_code(ErrorCode::InternalError)?); } if snapshot.client_settings().organize_imports() && supported_code_actions.contains(&SupportedCodeAction::SourceOrganizeImports) { - response - .extend(organize_imports(&snapshot).with_failure_code(ErrorCode::InternalError)?); + response.push(organize_imports(&snapshot).with_failure_code(ErrorCode::InternalError)?); } Ok(Some(response)) @@ -88,7 +87,7 @@ fn quick_fix( .collect() } -fn fix_all(snapshot: &DocumentSnapshot) -> crate::Result> { +fn fix_all(snapshot: &DocumentSnapshot) -> crate::Result { let document = snapshot.document(); let (edit, data) = if snapshot @@ -113,21 +112,17 @@ fn fix_all(snapshot: &DocumentSnapshot) -> crate::Result crate::Result> { +fn organize_imports(snapshot: &DocumentSnapshot) -> crate::Result { let document = snapshot.document(); let (edit, data) = if snapshot @@ -152,18 +147,14 @@ fn organize_imports(snapshot: &DocumentSnapshot) -> crate::Result Date: Mon, 15 Apr 2024 11:42:32 -0700 Subject: [PATCH 3/7] `SupportedCodeActions::kinds` returns a static slice --- crates/ruff_server/src/server.rs | 21 +++++++++++++-------- 1 file changed, 13 insertions(+), 8 deletions(-) diff --git a/crates/ruff_server/src/server.rs b/crates/ruff_server/src/server.rs index dfdea1e2521542..d148830b5789d3 100644 --- a/crates/ruff_server/src/server.rs +++ b/crates/ruff_server/src/server.rs @@ -224,7 +224,8 @@ impl Server { CodeActionOptions { code_action_kinds: Some( SupportedCodeAction::all() - .flat_map(|action| action.kinds().into_iter()) + .flat_map(|action| action.kinds().iter()) + .cloned() .collect(), ), work_done_progress_options: WorkDoneProgressOptions { @@ -285,14 +286,18 @@ pub(crate) enum SupportedCodeAction { impl SupportedCodeAction { /// Returns the possible LSP code action kind(s) that map to this code action. - fn kinds(self) -> Vec { + fn kinds(self) -> &'static [CodeActionKind] { + static QUICKFIX: [CodeActionKind; 1] = [CodeActionKind::QUICKFIX]; + static SOURCE_FIX_ALL: [CodeActionKind; 2] = + [CodeActionKind::SOURCE_FIX_ALL, crate::SOURCE_FIX_ALL_RUFF]; + static SOURCE_ORGANIZE_IMPORTS: [CodeActionKind; 2] = [ + CodeActionKind::SOURCE_ORGANIZE_IMPORTS, + crate::SOURCE_ORGANIZE_IMPORTS_RUFF, + ]; match self { - Self::QuickFix => vec![CodeActionKind::QUICKFIX], - Self::SourceFixAll => vec![CodeActionKind::SOURCE_FIX_ALL, crate::SOURCE_FIX_ALL_RUFF], - Self::SourceOrganizeImports => vec![ - CodeActionKind::SOURCE_ORGANIZE_IMPORTS, - crate::SOURCE_ORGANIZE_IMPORTS_RUFF, - ], + Self::QuickFix => &QUICKFIX, + Self::SourceFixAll => &SOURCE_FIX_ALL, + Self::SourceOrganizeImports => &SOURCE_ORGANIZE_IMPORTS, } } From c3d69f7c69b5c2ce9a6936e1a098ce9384115a3f Mon Sep 17 00:00:00 2001 From: Jane Lewis Date: Mon, 15 Apr 2024 20:35:48 -0700 Subject: [PATCH 4/7] Rework code action support checker --- crates/ruff_server/src/server.rs | 37 +++++++------------ .../src/server/api/requests/code_action.rs | 12 ++---- .../api/requests/code_action_resolve.rs | 18 +++++---- 3 files changed, 26 insertions(+), 41 deletions(-) diff --git a/crates/ruff_server/src/server.rs b/crates/ruff_server/src/server.rs index d148830b5789d3..4c45ece7473c91 100644 --- a/crates/ruff_server/src/server.rs +++ b/crates/ruff_server/src/server.rs @@ -224,7 +224,7 @@ impl Server { CodeActionOptions { code_action_kinds: Some( SupportedCodeAction::all() - .flat_map(|action| action.kinds().iter()) + .map(SupportedCodeAction::to_kind) .cloned() .collect(), ), @@ -285,22 +285,24 @@ pub(crate) enum SupportedCodeAction { } impl SupportedCodeAction { - /// Returns the possible LSP code action kind(s) that map to this code action. - fn kinds(self) -> &'static [CodeActionKind] { - static QUICKFIX: [CodeActionKind; 1] = [CodeActionKind::QUICKFIX]; - static SOURCE_FIX_ALL: [CodeActionKind; 2] = - [CodeActionKind::SOURCE_FIX_ALL, crate::SOURCE_FIX_ALL_RUFF]; - static SOURCE_ORGANIZE_IMPORTS: [CodeActionKind; 2] = [ - CodeActionKind::SOURCE_ORGANIZE_IMPORTS, - crate::SOURCE_ORGANIZE_IMPORTS_RUFF, - ]; + /// Returns the LSP code action kind that map to this code action. + fn to_kind(self) -> &'static CodeActionKind { + static QUICK_FIX: CodeActionKind = CodeActionKind::QUICKFIX; + static SOURCE_FIX_ALL: CodeActionKind = crate::SOURCE_FIX_ALL_RUFF; + static SOURCE_ORGANIZE_IMPORTS: CodeActionKind = crate::SOURCE_ORGANIZE_IMPORTS_RUFF; match self { - Self::QuickFix => &QUICKFIX, + Self::QuickFix => &QUICK_FIX, Self::SourceFixAll => &SOURCE_FIX_ALL, Self::SourceOrganizeImports => &SOURCE_ORGANIZE_IMPORTS, } } + fn from_kind(kind: CodeActionKind) -> impl Iterator { + Self::all().filter(move |supported_kind| { + supported_kind.to_kind().as_str().starts_with(kind.as_str()) + }) + } + /// Returns all code actions kinds that the server currently supports. fn all() -> impl Iterator { [ @@ -311,16 +313,3 @@ impl SupportedCodeAction { .into_iter() } } - -impl TryFrom for SupportedCodeAction { - type Error = (); - - fn try_from(kind: CodeActionKind) -> std::result::Result { - for supported_kind in Self::all() { - if supported_kind.kinds().contains(&kind) { - return Ok(supported_kind); - } - } - Err(()) - } -} diff --git a/crates/ruff_server/src/server/api/requests/code_action.rs b/crates/ruff_server/src/server/api/requests/code_action.rs index 3af967f137593a..96eb297f55280e 100644 --- a/crates/ruff_server/src/server/api/requests/code_action.rs +++ b/crates/ruff_server/src/server/api/requests/code_action.rs @@ -166,14 +166,8 @@ fn supported_code_actions( return SupportedCodeAction::all().collect(); }; - SupportedCodeAction::all() - .filter(move |action| { - action_filter.iter().any(|filter| { - action - .kinds() - .iter() - .any(|kind| kind.as_str().starts_with(filter.as_str())) - }) - }) + action_filter + .into_iter() + .flat_map(SupportedCodeAction::from_kind) .collect() } diff --git a/crates/ruff_server/src/server/api/requests/code_action_resolve.rs b/crates/ruff_server/src/server/api/requests/code_action_resolve.rs index c752a10827383e..f9c47267d9f9ee 100644 --- a/crates/ruff_server/src/server/api/requests/code_action_resolve.rs +++ b/crates/ruff_server/src/server/api/requests/code_action_resolve.rs @@ -30,14 +30,16 @@ impl super::BackgroundDocumentRequestHandler for CodeActionResolve { ) -> Result { let document = snapshot.document(); - let action_kind: SupportedCodeAction = action - .kind - .clone() - .ok_or(anyhow::anyhow!("No kind was given for code action")) - .with_failure_code(ErrorCode::InvalidParams)? - .try_into() - .map_err(|()| anyhow::anyhow!("Code action was of an invalid kind")) - .with_failure_code(ErrorCode::InvalidParams)?; + let action_kind = SupportedCodeAction::from_kind( + action + .kind + .clone() + .ok_or(anyhow::anyhow!("No kind was given for code action")) + .with_failure_code(ErrorCode::InvalidParams)?, + ) + .next() + .ok_or(anyhow::anyhow!("Code action was of an invalid kind")) + .with_failure_code(ErrorCode::InvalidParams)?; action.edit = match action_kind { SupportedCodeAction::SourceFixAll => Some( From e5c43ecf8efd84b19101ba2d43a0c3f2ad8b7b95 Mon Sep 17 00:00:00 2001 From: Jane Lewis Date: Mon, 15 Apr 2024 20:58:38 -0700 Subject: [PATCH 5/7] Ensure that a code action to be resolved maps to exactly one supported code action --- .../server/api/requests/code_action_resolve.rs | 15 +++++++++++---- 1 file changed, 11 insertions(+), 4 deletions(-) diff --git a/crates/ruff_server/src/server/api/requests/code_action_resolve.rs b/crates/ruff_server/src/server/api/requests/code_action_resolve.rs index f9c47267d9f9ee..bca16c3e0bc777 100644 --- a/crates/ruff_server/src/server/api/requests/code_action_resolve.rs +++ b/crates/ruff_server/src/server/api/requests/code_action_resolve.rs @@ -30,16 +30,23 @@ impl super::BackgroundDocumentRequestHandler for CodeActionResolve { ) -> Result { let document = snapshot.document(); - let action_kind = SupportedCodeAction::from_kind( + let code_actions = SupportedCodeAction::from_kind( action .kind .clone() .ok_or(anyhow::anyhow!("No kind was given for code action")) .with_failure_code(ErrorCode::InvalidParams)?, ) - .next() - .ok_or(anyhow::anyhow!("Code action was of an invalid kind")) - .with_failure_code(ErrorCode::InvalidParams)?; + .collect::>(); + + // Ensure that the code action maps to _exactly one_ supported code action + let [action_kind] = code_actions.as_slice() else { + return Err(anyhow::anyhow!( + "Code action resolver did not expect code action kind {:?}", + action.kind.as_ref().unwrap() + )) + .with_failure_code(ErrorCode::InvalidParams); + }; action.edit = match action_kind { SupportedCodeAction::SourceFixAll => Some( From df63c4bec4c680947116cba4d740db1e3a32b8fa Mon Sep 17 00:00:00 2001 From: Jane Lewis Date: Tue, 16 Apr 2024 11:13:20 -0700 Subject: [PATCH 6/7] to_kind returns an owned CodeActionKind --- crates/ruff_server/src/server.rs | 12 ++++-------- 1 file changed, 4 insertions(+), 8 deletions(-) diff --git a/crates/ruff_server/src/server.rs b/crates/ruff_server/src/server.rs index 4c45ece7473c91..0374217fb95b3b 100644 --- a/crates/ruff_server/src/server.rs +++ b/crates/ruff_server/src/server.rs @@ -225,7 +225,6 @@ impl Server { code_action_kinds: Some( SupportedCodeAction::all() .map(SupportedCodeAction::to_kind) - .cloned() .collect(), ), work_done_progress_options: WorkDoneProgressOptions { @@ -286,14 +285,11 @@ pub(crate) enum SupportedCodeAction { impl SupportedCodeAction { /// Returns the LSP code action kind that map to this code action. - fn to_kind(self) -> &'static CodeActionKind { - static QUICK_FIX: CodeActionKind = CodeActionKind::QUICKFIX; - static SOURCE_FIX_ALL: CodeActionKind = crate::SOURCE_FIX_ALL_RUFF; - static SOURCE_ORGANIZE_IMPORTS: CodeActionKind = crate::SOURCE_ORGANIZE_IMPORTS_RUFF; + fn to_kind(self) -> CodeActionKind { match self { - Self::QuickFix => &QUICK_FIX, - Self::SourceFixAll => &SOURCE_FIX_ALL, - Self::SourceOrganizeImports => &SOURCE_ORGANIZE_IMPORTS, + Self::QuickFix => CodeActionKind::QUICKFIX, + Self::SourceFixAll => crate::SOURCE_FIX_ALL_RUFF, + Self::SourceOrganizeImports => crate::SOURCE_ORGANIZE_IMPORTS_RUFF, } } From d0fd8270f8bed668d043e70aa35a45651aa88be9 Mon Sep 17 00:00:00 2001 From: Jane Lewis Date: Tue, 16 Apr 2024 11:13:47 -0700 Subject: [PATCH 7/7] remove unnecessary clones --- crates/ruff_server/src/server/api/requests/code_action.rs | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/crates/ruff_server/src/server/api/requests/code_action.rs b/crates/ruff_server/src/server/api/requests/code_action.rs index 96eb297f55280e..49f969918c5417 100644 --- a/crates/ruff_server/src/server/api/requests/code_action.rs +++ b/crates/ruff_server/src/server/api/requests/code_action.rs @@ -116,8 +116,8 @@ fn fix_all(snapshot: &DocumentSnapshot) -> crate::Result { Ok(CodeActionOrCommand::CodeAction(types::CodeAction { title: format!("{DIAGNOSTIC_NAME}: Fix all auto-fixable problems"), kind: Some(crate::SOURCE_FIX_ALL_RUFF), - edit: edit.clone(), - data: data.clone(), + edit, + data, ..Default::default() })) } @@ -151,8 +151,8 @@ fn organize_imports(snapshot: &DocumentSnapshot) -> crate::Result