Skip to content

Filter out code actions if unsupported by the client and advertise our capabilities #4167

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 4 commits into from
May 1, 2020
Merged
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
29 changes: 21 additions & 8 deletions crates/rust-analyzer/src/caps.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,13 +3,13 @@
use crate::semantic_tokens;

use lsp_types::{
CallHierarchyServerCapability, CodeActionProviderCapability, CodeLensOptions,
CompletionOptions, DocumentOnTypeFormattingOptions, FoldingRangeProviderCapability,
ImplementationProviderCapability, RenameOptions, RenameProviderCapability, SaveOptions,
SelectionRangeProviderCapability, SemanticTokensDocumentProvider, SemanticTokensLegend,
SemanticTokensOptions, ServerCapabilities, SignatureHelpOptions, TextDocumentSyncCapability,
TextDocumentSyncKind, TextDocumentSyncOptions, TypeDefinitionProviderCapability,
WorkDoneProgressOptions,
CallHierarchyServerCapability, CodeActionOptions, CodeActionProviderCapability,
CodeLensOptions, CompletionOptions, DocumentOnTypeFormattingOptions,
FoldingRangeProviderCapability, ImplementationProviderCapability, RenameOptions,
RenameProviderCapability, SaveOptions, SelectionRangeProviderCapability,
SemanticTokensDocumentProvider, SemanticTokensLegend, SemanticTokensOptions,
ServerCapabilities, SignatureHelpOptions, TextDocumentSyncCapability, TextDocumentSyncKind,
TextDocumentSyncOptions, TypeDefinitionProviderCapability, WorkDoneProgressOptions,
};

pub fn server_capabilities() -> ServerCapabilities {
Expand Down Expand Up @@ -40,7 +40,20 @@ pub fn server_capabilities() -> ServerCapabilities {
document_highlight_provider: Some(true),
document_symbol_provider: Some(true),
workspace_symbol_provider: Some(true),
code_action_provider: Some(CodeActionProviderCapability::Simple(true)),
code_action_provider: Some(CodeActionProviderCapability::Options(CodeActionOptions {
// Advertise support for all built-in CodeActionKinds
code_action_kinds: Some(vec![
String::new(),
lsp_types::code_action_kind::QUICKFIX.to_string(),
lsp_types::code_action_kind::REFACTOR.to_string(),
lsp_types::code_action_kind::REFACTOR_EXTRACT.to_string(),
lsp_types::code_action_kind::REFACTOR_INLINE.to_string(),
lsp_types::code_action_kind::REFACTOR_REWRITE.to_string(),
lsp_types::code_action_kind::SOURCE.to_string(),
lsp_types::code_action_kind::SOURCE_ORGANIZE_IMPORTS.to_string(),
]),
work_done_progress_options: Default::default(),
})),
code_lens_provider: Some(CodeLensOptions { resolve_provider: Some(true) }),
document_formatting_provider: Some(true),
document_range_formatting_provider: None,
Expand Down
6 changes: 6 additions & 0 deletions crates/rust-analyzer/src/config.rs
Original file line number Diff line number Diff line change
Expand Up @@ -70,6 +70,7 @@ pub struct ClientCapsConfig {
pub location_link: bool,
pub line_folding_only: bool,
pub hierarchical_symbols: bool,
pub code_action_literals: bool,
}

impl Default for Config {
Expand Down Expand Up @@ -221,6 +222,11 @@ impl Config {
{
self.client_caps.hierarchical_symbols = value
}
if let Some(value) =
caps.code_action.as_ref().and_then(|it| Some(it.code_action_literal_support.is_some()))
{
self.client_caps.code_action_literals = value;
}
self.completion.allow_snippets(false);
if let Some(completion) = &caps.completion {
if let Some(completion_item) = &completion.completion_item {
Expand Down
28 changes: 19 additions & 9 deletions crates/rust-analyzer/src/main_loop/handlers.rs
Original file line number Diff line number Diff line change
Expand Up @@ -19,8 +19,7 @@ use lsp_types::{
TextEdit, Url, WorkspaceEdit,
};
use ra_ide::{
Assist, AssistId, FileId, FilePosition, FileRange, Query, RangeInfo, Runnable, RunnableKind,
SearchScope,
Assist, FileId, FilePosition, FileRange, Query, RangeInfo, Runnable, RunnableKind, SearchScope,
};
use ra_prof::profile;
use ra_syntax::{AstNode, SyntaxKind, TextRange, TextSize};
Expand Down Expand Up @@ -702,15 +701,9 @@ fn create_single_code_action(assist: Assist, world: &WorldSnapshot) -> Result<Co
arguments: Some(vec![arg]),
};

let kind = match assist.id {
AssistId("introduce_variable") => Some("refactor.extract.variable".to_string()),
AssistId("add_custom_impl") => Some("refactor.rewrite.add_custom_impl".to_string()),
_ => None,
};

Ok(CodeAction {
title,
kind,
kind: Some(String::new()),
diagnostics: None,
edit: None,
command: Some(command),
Expand Down Expand Up @@ -812,6 +805,23 @@ pub fn handle_code_action(
}
}

// If the client only supports commands then filter the list
// and remove and actions that depend on edits.
if !world.config.client_caps.code_action_literals {
// FIXME: use drain_filter once it hits stable.
res = res
Copy link
Member

Choose a reason for hiding this comment

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

I think .retain would be a more convenietn API to use here

Copy link
Member

Choose a reason for hiding this comment

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

ah no, we do need to transform things... https://doc.rust-lang.org/stable/std/vec/struct.Vec.html#method.drain_filter would work, but it's nighly

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 added a comment to move to drain_filter once stable.

.into_iter()
.filter_map(|it| match it {
cmd @ lsp_types::CodeActionOrCommand::Command(_) => Some(cmd),
lsp_types::CodeActionOrCommand::CodeAction(action) => match action.command {
Some(cmd) if action.edit.is_none() => {
Some(lsp_types::CodeActionOrCommand::Command(cmd))
}
_ => None,
},
})
.collect();
}
Ok(Some(res))
}

Expand Down
6 changes: 5 additions & 1 deletion crates/rust-analyzer/tests/heavy_tests/support.rs
Original file line number Diff line number Diff line change
Expand Up @@ -77,7 +77,11 @@ impl<'a> Project<'a> {
let roots = self.roots.into_iter().map(|root| tmp_dir.path().join(root)).collect();

let mut config = Config {
client_caps: ClientCapsConfig { location_link: true, ..Default::default() },
client_caps: ClientCapsConfig {
location_link: true,
code_action_literals: true,
..Default::default()
},
with_sysroot: self.with_sysroot,
..Config::default()
};
Expand Down