Skip to content
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

Implementation of lazy assits #4717

Merged
merged 6 commits into from
Jun 3, 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
39 changes: 17 additions & 22 deletions crates/ra_ide/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -77,7 +77,7 @@ pub use crate::{
};

pub use hir::Documentation;
pub use ra_assists::{AssistConfig, AssistId};
pub use ra_assists::{Assist, AssistConfig, AssistId, ResolvedAssist};
pub use ra_db::{
Canceled, CrateGraph, CrateId, Edition, FileId, FilePosition, FileRange, SourceRootId,
};
Expand Down Expand Up @@ -142,14 +142,6 @@ pub struct AnalysisHost {
db: RootDatabase,
}

#[derive(Debug)]
pub struct Assist {
pub id: AssistId,
pub label: String,
pub group_label: Option<String>,
pub source_change: SourceChange,
}

impl AnalysisHost {
pub fn new(lru_capacity: Option<usize>) -> AnalysisHost {
AnalysisHost { db: RootDatabase::new(lru_capacity) }
Expand Down Expand Up @@ -470,20 +462,23 @@ impl Analysis {
self.with_db(|db| completion::completions(db, config, position).map(Into::into))
}

/// Computes assists (aka code actions aka intentions) for the given
/// Computes resolved assists with source changes for the given position.
pub fn resolved_assists(
&self,
config: &AssistConfig,
frange: FileRange,
) -> Cancelable<Vec<ResolvedAssist>> {
self.with_db(|db| ra_assists::Assist::resolved(db, config, frange))
}

/// Computes unresolved assists (aka code actions aka intentions) for the given
/// position.
pub fn assists(&self, config: &AssistConfig, frange: FileRange) -> Cancelable<Vec<Assist>> {
self.with_db(|db| {
ra_assists::Assist::resolved(db, config, frange)
.into_iter()
.map(|assist| Assist {
id: assist.assist.id,
label: assist.assist.label,
group_label: assist.assist.group.map(|it| it.0),
source_change: assist.source_change,
})
.collect()
})
pub fn unresolved_assists(
&self,
config: &AssistConfig,
frange: FileRange,
) -> Cancelable<Vec<Assist>> {
self.with_db(|db| Assist::unresolved(db, config, frange))
}

/// Computes the set of diagnostics for the given file.
Expand Down
7 changes: 6 additions & 1 deletion crates/rust-analyzer/src/config.rs
Original file line number Diff line number Diff line change
Expand Up @@ -123,6 +123,7 @@ pub struct ClientCapsConfig {
pub code_action_literals: bool,
pub work_done_progress: bool,
pub code_action_group: bool,
pub resolve_code_action: bool,
}

impl Default for Config {
Expand Down Expand Up @@ -336,7 +337,11 @@ impl Config {

let code_action_group =
experimental.get("codeActionGroup").and_then(|it| it.as_bool()) == Some(true);
self.client_caps.code_action_group = code_action_group
self.client_caps.code_action_group = code_action_group;

let resolve_code_action =
experimental.get("resolveCodeAction").and_then(|it| it.as_bool()) == Some(true);
self.client_caps.resolve_code_action = resolve_code_action;
}
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -65,6 +65,7 @@ expression: diag
fixes: [
CodeAction {
title: "return the expression directly",
id: None,
group: None,
kind: Some(
"quickfix",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,7 @@ expression: diag
fixes: [
CodeAction {
title: "consider prefixing with an underscore",
id: None,
group: None,
kind: Some(
"quickfix",
Expand Down
1 change: 1 addition & 0 deletions crates/rust-analyzer/src/diagnostics/to_proto.rs
Original file line number Diff line number Diff line change
Expand Up @@ -145,6 +145,7 @@ fn map_rust_child_diagnostic(
} else {
MappedRustChildDiagnostic::SuggestedFix(lsp_ext::CodeAction {
title: rd.message.clone(),
id: None,
group: None,
kind: Some("quickfix".to_string()),
edit: Some(lsp_ext::SnippetWorkspaceEdit {
Expand Down
18 changes: 18 additions & 0 deletions crates/rust-analyzer/src/lsp_ext.rs
Original file line number Diff line number Diff line change
Expand Up @@ -97,6 +97,22 @@ pub struct JoinLinesParams {
pub ranges: Vec<Range>,
}

pub enum ResolveCodeActionRequest {}

impl Request for ResolveCodeActionRequest {
type Params = ResolveCodeActionParams;
type Result = Option<SnippetWorkspaceEdit>;
const METHOD: &'static str = "experimental/resolveCodeAction";
}

/// Params for the ResolveCodeActionRequest
#[derive(Debug, Eq, PartialEq, Clone, Deserialize, Serialize)]
#[serde(rename_all = "camelCase")]
pub struct ResolveCodeActionParams {
pub code_action_params: lsp_types::CodeActionParams,
pub id: String,
}

pub enum OnEnter {}

impl Request for OnEnter {
Expand Down Expand Up @@ -202,6 +218,8 @@ impl Request for CodeActionRequest {
pub struct CodeAction {
Copy link
Contributor

Choose a reason for hiding this comment

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

This is missing pub diagnostics: Option<Vec<Diagnostic>>,. In addition it is going to conflict with edit in 3.16.

Copy link
Member

Choose a reason for hiding this comment

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

We don’t use diagnostics, so that’s why it is missing. What exactly changes in edit in 3.16?

Copy link
Contributor

Choose a reason for hiding this comment

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

Ignore me. I was thinking of insert/replace edits in CompletionItem

pub title: String,
#[serde(skip_serializing_if = "Option::is_none")]
pub id: Option<String>,
#[serde(skip_serializing_if = "Option::is_none")]
pub group: Option<String>,
#[serde(skip_serializing_if = "Option::is_none")]
pub kind: Option<String>,
Expand Down
1 change: 1 addition & 0 deletions crates/rust-analyzer/src/main_loop.rs
Original file line number Diff line number Diff line change
Expand Up @@ -509,6 +509,7 @@ fn on_request(
.on::<lsp_ext::Runnables>(handlers::handle_runnables)?
.on::<lsp_ext::InlayHints>(handlers::handle_inlay_hints)?
.on::<lsp_ext::CodeActionRequest>(handlers::handle_code_action)?
.on::<lsp_ext::ResolveCodeActionRequest>(handlers::handle_resolve_code_action)?
.on::<lsp_types::request::OnTypeFormatting>(handlers::handle_on_type_formatting)?
.on::<lsp_types::request::DocumentSymbolRequest>(handlers::handle_document_symbol)?
.on::<lsp_types::request::WorkspaceSymbol>(handlers::handle_workspace_symbol)?
Expand Down
77 changes: 58 additions & 19 deletions crates/rust-analyzer/src/main_loop/handlers.rs
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ use ra_project_model::TargetKind;
use ra_syntax::{AstNode, SyntaxKind, TextRange, TextSize};
use serde::{Deserialize, Serialize};
use serde_json::to_value;
use stdx::format_to;
use stdx::{format_to, split1};

use crate::{
cargo_target_spec::CargoTargetSpec,
Expand Down Expand Up @@ -701,37 +701,27 @@ pub fn handle_formatting(
}]))
}

pub fn handle_code_action(
snap: GlobalStateSnapshot,
params: lsp_types::CodeActionParams,
) -> Result<Option<Vec<lsp_ext::CodeAction>>> {
let _p = profile("handle_code_action");
// We intentionally don't support command-based actions, as those either
// requires custom client-code anyway, or requires server-initiated edits.
// Server initiated edits break causality, so we avoid those as well.
if !snap.config.client_caps.code_action_literals {
return Ok(None);
}

fn handle_fixes(
snap: &GlobalStateSnapshot,
params: &lsp_types::CodeActionParams,
res: &mut Vec<lsp_ext::CodeAction>,
) -> Result<()> {
let file_id = from_proto::file_id(&snap, &params.text_document.uri)?;
let line_index = snap.analysis().file_line_index(file_id)?;
let range = from_proto::text_range(&line_index, params.range);
let frange = FileRange { file_id, range };

let diagnostics = snap.analysis().diagnostics(file_id)?;
let mut res: Vec<lsp_ext::CodeAction> = Vec::new();

let fixes_from_diagnostics = diagnostics
.into_iter()
.filter_map(|d| Some((d.range, d.fix?)))
.filter(|(diag_range, _fix)| diag_range.intersect(range).is_some())
.map(|(_range, fix)| fix);

for fix in fixes_from_diagnostics {
let title = fix.label;
let edit = to_proto::snippet_workspace_edit(&snap, fix.source_change)?;
let action = lsp_ext::CodeAction {
title,
id: None,
group: None,
kind: Some(lsp_types::code_action_kind::QUICKFIX.into()),
edit: Some(edit),
mcrakhman marked this conversation as resolved.
Show resolved Hide resolved
Expand All @@ -747,13 +737,62 @@ pub fn handle_code_action(
}
res.push(fix.action.clone());
}
Ok(())
}

pub fn handle_code_action(
snap: GlobalStateSnapshot,
params: lsp_types::CodeActionParams,
) -> Result<Option<Vec<lsp_ext::CodeAction>>> {
let _p = profile("handle_code_action");
// We intentionally don't support command-based actions, as those either
// requires custom client-code anyway, or requires server-initiated edits.
// Server initiated edits break causality, so we avoid those as well.
if !snap.config.client_caps.code_action_literals {
return Ok(None);
}

let file_id = from_proto::file_id(&snap, &params.text_document.uri)?;
let line_index = snap.analysis().file_line_index(file_id)?;
let range = from_proto::text_range(&line_index, params.range);
let frange = FileRange { file_id, range };
let mut res: Vec<lsp_ext::CodeAction> = Vec::new();

for assist in snap.analysis().assists(&snap.config.assist, frange)?.into_iter() {
res.push(to_proto::code_action(&snap, assist)?.into());
handle_fixes(&snap, &params, &mut res)?;

if snap.config.client_caps.resolve_code_action {
for (index, assist) in
snap.analysis().unresolved_assists(&snap.config.assist, frange)?.into_iter().enumerate()
{
res.push(to_proto::unresolved_code_action(&snap, assist, index)?);
}
} else {
for assist in snap.analysis().resolved_assists(&snap.config.assist, frange)?.into_iter() {
res.push(to_proto::resolved_code_action(&snap, assist)?);
}
}

Ok(Some(res))
}

pub fn handle_resolve_code_action(
snap: GlobalStateSnapshot,
params: lsp_ext::ResolveCodeActionParams,
) -> Result<Option<lsp_ext::SnippetWorkspaceEdit>> {
let _p = profile("handle_resolve_code_action");
let file_id = from_proto::file_id(&snap, &params.code_action_params.text_document.uri)?;
let line_index = snap.analysis().file_line_index(file_id)?;
let range = from_proto::text_range(&line_index, params.code_action_params.range);
let frange = FileRange { file_id, range };

let assists = snap.analysis().resolved_assists(&snap.config.assist, frange)?;
let (id_string, index) = split1(&params.id, ':').unwrap();
let index = index.parse::<usize>().unwrap();
let assist = &assists[index];
assert!(assist.assist.id.0 == id_string);
Ok(to_proto::resolved_code_action(&snap, assist.clone())?.edit)
}

pub fn handle_code_lens(
snap: GlobalStateSnapshot,
params: lsp_types::CodeLensParams,
Expand Down
26 changes: 21 additions & 5 deletions crates/rust-analyzer/src/to_proto.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,8 +3,8 @@ use ra_db::{FileId, FileRange};
use ra_ide::{
Assist, CompletionItem, CompletionItemKind, Documentation, FileSystemEdit, Fold, FoldKind,
FunctionSignature, Highlight, HighlightModifier, HighlightTag, HighlightedRange, Indel,
InlayHint, InlayKind, InsertTextFormat, LineIndex, NavigationTarget, ReferenceAccess, Runnable,
RunnableKind, Severity, SourceChange, SourceFileEdit, TextEdit,
InlayHint, InlayKind, InsertTextFormat, LineIndex, NavigationTarget, ReferenceAccess,
ResolvedAssist, Runnable, RunnableKind, Severity, SourceChange, SourceFileEdit, TextEdit,
};
use ra_syntax::{SyntaxKind, TextRange, TextSize};
use ra_vfs::LineEndings;
Expand Down Expand Up @@ -623,20 +623,36 @@ fn main() <fold>{
}
}

pub(crate) fn code_action(
pub(crate) fn unresolved_code_action(
snap: &GlobalStateSnapshot,
assist: Assist,
index: usize,
) -> Result<lsp_ext::CodeAction> {
let res = lsp_ext::CodeAction {
title: assist.label,
group: if snap.config.client_caps.code_action_group { assist.group_label } else { None },
id: Some(format!("{}:{}", assist.id.0.to_owned(), index.to_string())),
group: assist.group.filter(|_| snap.config.client_caps.code_action_group).map(|gr| gr.0),
kind: Some(String::new()),
edit: Some(snippet_workspace_edit(snap, assist.source_change)?),
edit: None,
command: None,
};
Ok(res)
}

pub(crate) fn resolved_code_action(
snap: &GlobalStateSnapshot,
assist: ResolvedAssist,
) -> Result<lsp_ext::CodeAction> {
let change = assist.source_change;
unresolved_code_action(snap, assist.assist, 0).and_then(|it| {
Ok(lsp_ext::CodeAction {
id: None,
edit: Some(snippet_workspace_edit(snap, change)?),
..it
})
})
}

pub(crate) fn runnable(
snap: &GlobalStateSnapshot,
file_id: FileId,
Expand Down
24 changes: 24 additions & 0 deletions docs/dev/lsp-extensions.md
Original file line number Diff line number Diff line change
Expand Up @@ -97,6 +97,30 @@ Invoking code action at this position will yield two code actions for importing
* Is a fixed two-level structure enough?
* Should we devise a general way to encode custom interaction protocols for GUI refactorings?

## Lazy assists with `ResolveCodeAction`

**Issue:** https://github.com/microsoft/language-server-protocol/issues/787

**Client Capability** `{ "resolveCodeAction": boolean }`

If this capability is set, the assists will be computed lazily. Thus `CodeAction` returned from the server will only contain `id` but not `edit` or `command` fields. The only exclusion from the rule is the diagnostic edits.

After the client got the id, it should then call `experimental/resolveCodeAction` command on the server and provide the following payload:

```typescript
interface ResolveCodeActionParams {
id: string;
codeActionParams: lc.CodeActionParams;
}
```

As a result of the command call the client will get the respective workspace edit (`lc.WorkspaceEdit`).

### Unresolved Questions

* Apply smarter filtering for ids?
* Upon `resolveCodeAction` command only call the assits which should be resolved and not all of them?

## Parent Module

**Issue:** https://github.com/microsoft/language-server-protocol/issues/1002
Expand Down
Loading