Mirrors/rust-analyzer
2
0
Fork 0
mirror of https://github.com/rust-lang/rust-analyzer synced 2024-12-27 05:23:24 +00:00
Code Issues Projects Releases Packages Wiki Activity

Merge #7043

Browse source 
7043: Simplify assists resolution API r=matklad a=matklad

bors r+
🤖

Co-authored-by: Aleksey Kladov <aleksey.kladov@gmail.com>
This commit is contained in:
bors[bot] 2020-12-26 11:14:19 +00:00 committed by GitHub
commit 1d874a2a65
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
6 changed files with 69 additions and 120 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

51
crates/assists/src/assist_context.rs
View file

@ -19,7 +19,7 @@ use text_edit::{TextEdit, TextEditBuilder};
use crate::{ use crate::{
assist_config::{AssistConfig, SnippetCap}, assist_config::{AssistConfig, SnippetCap},
Assist, AssistId, AssistKind, GroupLabel, ResolvedAssist, Assist, AssistId, AssistKind, GroupLabel,
}; };
/// `AssistContext` allows to apply an assist or check if it could be applied. /// `AssistContext` allows to apply an assist or check if it could be applied.
@ -105,46 +105,23 @@ impl<'a> AssistContext<'a> {
pub(crate) struct Assists { pub(crate) struct Assists {
resolve: bool, resolve: bool,
file: FileId, file: FileId,
buf: Vec<(Assist, Option<SourceChange>)>, buf: Vec<Assist>,
allowed: Option<Vec<AssistKind>>, allowed: Option<Vec<AssistKind>>,
} }
impl Assists { impl Assists {
pub(crate) fn new_resolved(ctx: &AssistContext) -> Assists { pub(crate) fn new(ctx: &AssistContext, resolve: bool) -> Assists {
Assists { Assists {
resolve: true, resolve,
file: ctx.frange.file_id, file: ctx.frange.file_id,
buf: Vec::new(), buf: Vec::new(),
allowed: ctx.config.allowed.clone(), allowed: ctx.config.allowed.clone(),
} }
} }
pub(crate) fn new_unresolved(ctx: &AssistContext) -> Assists { pub(crate) fn finish(mut self) -> Vec<Assist> {
Assists { self.buf.sort_by_key(|assist| assist.target.len());
resolve: false, self.buf
file: ctx.frange.file_id,
buf: Vec::new(),
allowed: ctx.config.allowed.clone(),
}
}
pub(crate) fn finish_unresolved(self) -> Vec<Assist> {
assert!(!self.resolve);
self.finish()
.into_iter()
.map(|(label, edit)| {
assert!(edit.is_none());
label
})
.collect()
}
pub(crate) fn finish_resolved(self) -> Vec<ResolvedAssist> {
assert!(self.resolve);
self.finish()
.into_iter()
.map(|(label, edit)| ResolvedAssist { assist: label, source_change: edit.unwrap() })
.collect()
} }
pub(crate) fn add( pub(crate) fn add(
@ -158,7 +135,7 @@ impl Assists {
return None; return None;
} }
let label = Label::new(label.into()); let label = Label::new(label.into());
let assist = Assist { id, label, group: None, target }; let assist = Assist { id, label, group: None, target, source_change: None };
self.add_impl(assist, f) self.add_impl(assist, f)
} }
@ -174,11 +151,11 @@ impl Assists {
return None; return None;
} }
let label = Label::new(label.into()); let label = Label::new(label.into());
let assist = Assist { id, label, group: Some(group.clone()), target }; let assist = Assist { id, label, group: Some(group.clone()), target, source_change: None };
self.add_impl(assist, f) self.add_impl(assist, f)
} }
fn add_impl(&mut self, assist: Assist, f: impl FnOnce(&mut AssistBuilder)) -> Option<()> { fn add_impl(&mut self, mut assist: Assist, f: impl FnOnce(&mut AssistBuilder)) -> Option<()> {
let source_change = if self.resolve { let source_change = if self.resolve {
let mut builder = AssistBuilder::new(self.file); let mut builder = AssistBuilder::new(self.file);
f(&mut builder); f(&mut builder);
@ -186,16 +163,12 @@ impl Assists {
} else { } else {
None None
}; };
assist.source_change = source_change.clone();
self.buf.push((assist, source_change)); self.buf.push(assist);
Some(()) Some(())
} }
fn finish(mut self) -> Vec<(Assist, Option<SourceChange>)> {
self.buf.sort_by_key(|(label, _edit)| label.target.len());
self.buf
}
fn is_allowed(&self, id: &AssistId) -> bool { fn is_allowed(&self, id: &AssistId) -> bool {
match &self.allowed { match &self.allowed {
Some(allowed) => allowed.iter().any(|kind| kind.contains(id.1)), Some(allowed) => allowed.iter().any(|kind| kind.contains(id.1)),

41
crates/assists/src/lib.rs
View file

@ -73,45 +73,32 @@ pub struct Assist {
/// Target ranges are used to sort assists: the smaller the target range, /// Target ranges are used to sort assists: the smaller the target range,
/// the more specific assist is, and so it should be sorted first. /// the more specific assist is, and so it should be sorted first.
pub target: TextRange, pub target: TextRange,
} /// Computing source change sometimes is much more costly then computing the
/// other fields. Additionally, the actual change is not required to show
#[derive(Debug, Clone)] /// the lightbulb UI, it only is needed when the user tries to apply an
pub struct ResolvedAssist { /// assist. So, we compute it lazily: the API allow requesting assists with
pub assist: Assist, /// or without source change. We could (and in fact, used to) distinguish
pub source_change: SourceChange, /// between resolved and unresolved assists at the type level, but this is
/// cumbersome, especially if you want to embed an assist into another data
/// structure, such as a diagnostic.
pub source_change: Option<SourceChange>,
} }
impl Assist { impl Assist {
/// Return all the assists applicable at the given position. /// Return all the assists applicable at the given position.
/// pub fn get(
/// Assists are returned in the "unresolved" state, that is only labels are
/// returned, without actual edits.
pub fn unresolved(db: &RootDatabase, config: &AssistConfig, range: FileRange) -> Vec<Assist> {
let sema = Semantics::new(db);
let ctx = AssistContext::new(sema, config, range);
let mut acc = Assists::new_unresolved(&ctx);
handlers::all().iter().for_each(|handler| {
handler(&mut acc, &ctx);
});
acc.finish_unresolved()
}
/// Return all the assists applicable at the given position.
///
/// Assists are returned in the "resolved" state, that is with edit fully
/// computed.
pub fn resolved(
db: &RootDatabase, db: &RootDatabase,
config: &AssistConfig, config: &AssistConfig,
resolve: bool,
range: FileRange, range: FileRange,
) -> Vec<ResolvedAssist> { ) -> Vec<Assist> {
let sema = Semantics::new(db); let sema = Semantics::new(db);
let ctx = AssistContext::new(sema, config, range); let ctx = AssistContext::new(sema, config, range);
let mut acc = Assists::new_resolved(&ctx); let mut acc = Assists::new(&ctx, resolve);
handlers::all().iter().for_each(|handler| { handlers::all().iter().for_each(|handler| {
handler(&mut acc, &ctx); handler(&mut acc, &ctx);
}); });
acc.finish_resolved() acc.finish()
} }
} }

48
crates/assists/src/tests.rs
View file

@ -48,24 +48,25 @@ fn check_doc_test(assist_id: &str, before: &str, after: &str) {
let before = db.file_text(file_id).to_string(); let before = db.file_text(file_id).to_string();
let frange = FileRange { file_id, range: selection.into() }; let frange = FileRange { file_id, range: selection.into() };
let assist = Assist::resolved(&db, &AssistConfig::default(), frange) let assist = Assist::get(&db, &AssistConfig::default(), true, frange)
.into_iter() .into_iter()
.find(|assist| assist.assist.id.0 == assist_id) .find(|assist| assist.id.0 == assist_id)
.unwrap_or_else(|| { .unwrap_or_else(|| {
panic!( panic!(
"\n\nAssist is not applicable: {}\nAvailable assists: {}", "\n\nAssist is not applicable: {}\nAvailable assists: {}",
assist_id, assist_id,
Assist::resolved(&db, &AssistConfig::default(), frange) Assist::get(&db, &AssistConfig::default(), false, frange)
.into_iter() .into_iter()
.map(|assist| assist.assist.id.0) .map(|assist| assist.id.0)
.collect::<Vec<_>>() .collect::<Vec<_>>()
.join(", ") .join(", ")
) )
}); });
let actual = { let actual = {
let source_change = assist.source_change.unwrap();
let mut actual = before; let mut actual = before;
for source_file_edit in assist.source_change.source_file_edits { for source_file_edit in source_change.source_file_edits {
if source_file_edit.file_id == file_id { if source_file_edit.file_id == file_id {
source_file_edit.edit.apply(&mut actual) source_file_edit.edit.apply(&mut actual)
} }
@ -90,18 +91,18 @@ fn check(handler: Handler, before: &str, expected: ExpectedResult, assist_label:
let sema = Semantics::new(&db); let sema = Semantics::new(&db);
let config = AssistConfig::default(); let config = AssistConfig::default();
let ctx = AssistContext::new(sema, &config, frange); let ctx = AssistContext::new(sema, &config, frange);
let mut acc = Assists::new_resolved(&ctx); let mut acc = Assists::new(&ctx, true);
handler(&mut acc, &ctx); handler(&mut acc, &ctx);
let mut res = acc.finish_resolved(); let mut res = acc.finish();
let assist = match assist_label { let assist = match assist_label {
Some(label) => res.into_iter().find(|resolved| resolved.assist.label == label), Some(label) => res.into_iter().find(|resolved| resolved.label == label),
None => res.pop(), None => res.pop(),
}; };
match (assist, expected) { match (assist, expected) {
(Some(assist), ExpectedResult::After(after)) => { (Some(assist), ExpectedResult::After(after)) => {
let mut source_change = assist.source_change; let mut source_change = assist.source_change.unwrap();
assert!(!source_change.source_file_edits.is_empty()); assert!(!source_change.source_file_edits.is_empty());
let skip_header = source_change.source_file_edits.len() == 1 let skip_header = source_change.source_file_edits.len() == 1
&& source_change.file_system_edits.len() == 0; && source_change.file_system_edits.len() == 0;
@ -138,7 +139,7 @@ fn check(handler: Handler, before: &str, expected: ExpectedResult, assist_label:
assert_eq_text!(after, &buf); assert_eq_text!(after, &buf);
} }
(Some(assist), ExpectedResult::Target(target)) => { (Some(assist), ExpectedResult::Target(target)) => {
let range = assist.assist.target; let range = assist.target;
assert_eq_text!(&text_without_caret[range], target); assert_eq_text!(&text_without_caret[range], target);
} }
(Some(_), ExpectedResult::NotApplicable) => panic!("assist should not be applicable!"), (Some(_), ExpectedResult::NotApplicable) => panic!("assist should not be applicable!"),
@ -155,14 +156,11 @@ fn assist_order_field_struct() {
let (before_cursor_pos, before) = extract_offset(before); let (before_cursor_pos, before) = extract_offset(before);
let (db, file_id) = with_single_file(&before); let (db, file_id) = with_single_file(&before);
let frange = FileRange { file_id, range: TextRange::empty(before_cursor_pos) }; let frange = FileRange { file_id, range: TextRange::empty(before_cursor_pos) };
let assists = Assist::resolved(&db, &AssistConfig::default(), frange); let assists = Assist::get(&db, &AssistConfig::default(), false, frange);
let mut assists = assists.iter(); let mut assists = assists.iter();
assert_eq!( assert_eq!(assists.next().expect("expected assist").label, "Change visibility to pub(crate)");
assists.next().expect("expected assist").assist.label, assert_eq!(assists.next().expect("expected assist").label, "Add `#[derive]`");
"Change visibility to pub(crate)"
);
assert_eq!(assists.next().expect("expected assist").assist.label, "Add `#[derive]`");
} }
#[test] #[test]
@ -178,11 +176,11 @@ fn assist_order_if_expr() {
let (range, before) = extract_range(before); let (range, before) = extract_range(before);
let (db, file_id) = with_single_file(&before); let (db, file_id) = with_single_file(&before);
let frange = FileRange { file_id, range }; let frange = FileRange { file_id, range };
let assists = Assist::resolved(&db, &AssistConfig::default(), frange); let assists = Assist::get(&db, &AssistConfig::default(), false, frange);
let mut assists = assists.iter(); let mut assists = assists.iter();
assert_eq!(assists.next().expect("expected assist").assist.label, "Extract into variable"); assert_eq!(assists.next().expect("expected assist").label, "Extract into variable");
assert_eq!(assists.next().expect("expected assist").assist.label, "Replace with match"); assert_eq!(assists.next().expect("expected assist").label, "Replace with match");
} }
#[test] #[test]
@ -203,27 +201,27 @@ fn assist_filter_works() {
let mut cfg = AssistConfig::default(); let mut cfg = AssistConfig::default();
cfg.allowed = Some(vec![AssistKind::Refactor]); cfg.allowed = Some(vec![AssistKind::Refactor]);
let assists = Assist::resolved(&db, &cfg, frange); let assists = Assist::get(&db, &cfg, false, frange);
let mut assists = assists.iter(); let mut assists = assists.iter();
assert_eq!(assists.next().expect("expected assist").assist.label, "Extract into variable"); assert_eq!(assists.next().expect("expected assist").label, "Extract into variable");
assert_eq!(assists.next().expect("expected assist").assist.label, "Replace with match"); assert_eq!(assists.next().expect("expected assist").label, "Replace with match");
} }
{ {
let mut cfg = AssistConfig::default(); let mut cfg = AssistConfig::default();
cfg.allowed = Some(vec![AssistKind::RefactorExtract]); cfg.allowed = Some(vec![AssistKind::RefactorExtract]);
let assists = Assist::resolved(&db, &cfg, frange); let assists = Assist::get(&db, &cfg, false, frange);
assert_eq!(assists.len(), 1); assert_eq!(assists.len(), 1);
let mut assists = assists.iter(); let mut assists = assists.iter();
assert_eq!(assists.next().expect("expected assist").assist.label, "Extract into variable"); assert_eq!(assists.next().expect("expected assist").label, "Extract into variable");
} }
{ {
let mut cfg = AssistConfig::default(); let mut cfg = AssistConfig::default();
cfg.allowed = Some(vec![AssistKind::QuickFix]); cfg.allowed = Some(vec![AssistKind::QuickFix]);
let assists = Assist::resolved(&db, &cfg, frange); let assists = Assist::get(&db, &cfg, false, frange);
assert!(assists.is_empty(), "All asserts but quickfixes should be filtered out"); assert!(assists.is_empty(), "All asserts but quickfixes should be filtered out");
} }
} }

22
crates/ide/src/lib.rs
View file

@ -79,7 +79,7 @@ pub use crate::{
HighlightedRange, HighlightedRange,
}, },
}; };
pub use assists::{Assist, AssistConfig, AssistId, AssistKind, ResolvedAssist}; pub use assists::{Assist, AssistConfig, AssistId, AssistKind};
pub use completion::{ pub use completion::{
CompletionConfig, CompletionItem, CompletionItemKind, CompletionResolveCapability, CompletionConfig, CompletionItem, CompletionItemKind, CompletionResolveCapability,
CompletionScore, ImportEdit, InsertTextFormat, CompletionScore, ImportEdit, InsertTextFormat,
@ -491,22 +491,16 @@ impl Analysis {
} }
/// Computes assists (aka code actions aka intentions) for the given /// Computes assists (aka code actions aka intentions) for the given
/// position. Computes enough info to show the lightbulb list in the editor, /// position. If `resolve == false`, computes enough info to show the
/// but doesn't compute actual edits, to improve performance. /// lightbulb list in the editor, but doesn't compute actual edits, to
/// /// improve performance.
/// When the user clicks on the assist, call `resolve_assists` to get the pub fn assists(
/// edit.
pub fn assists(&self, config: &AssistConfig, frange: FileRange) -> Cancelable<Vec<Assist>> {
self.with_db(|db| Assist::unresolved(db, config, frange))
}
/// Computes resolved assists with source changes for the given position.
pub fn resolve_assists(
&self, &self,
config: &AssistConfig, config: &AssistConfig,
resolve: bool,
frange: FileRange, frange: FileRange,
) -> Cancelable<Vec<ResolvedAssist>> { ) -> Cancelable<Vec<Assist>> {
self.with_db(|db| assists::Assist::resolved(db, config, frange)) self.with_db(|db| Assist::get(db, config, resolve, frange))
} }
/// Computes the set of diagnostics for the given file. /// Computes the set of diagnostics for the given file.

8
crates/rust-analyzer/src/handlers.rs
View file

@ -946,12 +946,12 @@ pub(crate) fn handle_code_action(
if snap.config.client_caps.code_action_resolve { if snap.config.client_caps.code_action_resolve {
for (index, assist) in for (index, assist) in
snap.analysis.assists(&assists_config, frange)?.into_iter().enumerate() snap.analysis.assists(&assists_config, false, frange)?.into_iter().enumerate()
{ {
res.push(to_proto::unresolved_code_action(&snap, params.clone(), assist, index)?); res.push(to_proto::unresolved_code_action(&snap, params.clone(), assist, index)?);
} }
} else { } else {
for assist in snap.analysis.resolve_assists(&assists_config, frange)?.into_iter() { for assist in snap.analysis.assists(&assists_config, true, frange)?.into_iter() {
res.push(to_proto::resolved_code_action(&snap, assist)?); res.push(to_proto::resolved_code_action(&snap, assist)?);
} }
} }
@ -1014,11 +1014,11 @@ pub(crate) fn handle_code_action_resolve(
.only .only
.map(|it| it.into_iter().filter_map(from_proto::assist_kind).collect()); .map(|it| it.into_iter().filter_map(from_proto::assist_kind).collect());
let assists = snap.analysis.resolve_assists(&snap.config.assist, frange)?; let assists = snap.analysis.assists(&snap.config.assist, true, frange)?;
let (id, index) = split_once(&params.id, ':').unwrap(); let (id, index) = split_once(&params.id, ':').unwrap();
let index = index.parse::<usize>().unwrap(); let index = index.parse::<usize>().unwrap();
let assist = &assists[index]; let assist = &assists[index];
assert!(assist.assist.id.0 == id); assert!(assist.id.0 == id);
let edit = to_proto::resolved_code_action(&snap, assist.clone())?.edit; let edit = to_proto::resolved_code_action(&snap, assist.clone())?.edit;
code_action.edit = edit; code_action.edit = edit;
Ok(code_action) Ok(code_action)

19
crates/rust-analyzer/src/to_proto.rs
View file

@ -8,8 +8,8 @@ use ide::{
Assist, AssistKind, CallInfo, CompletionItem, CompletionItemKind, Documentation, FileId, Assist, AssistKind, CallInfo, CompletionItem, CompletionItemKind, Documentation, FileId,
FileRange, FileSystemEdit, Fold, FoldKind, Highlight, HighlightModifier, HighlightTag, FileRange, FileSystemEdit, Fold, FoldKind, Highlight, HighlightModifier, HighlightTag,
HighlightedRange, Indel, InlayHint, InlayKind, InsertTextFormat, LineIndex, Markup, HighlightedRange, Indel, InlayHint, InlayKind, InsertTextFormat, LineIndex, Markup,
NavigationTarget, ReferenceAccess, ResolvedAssist, Runnable, Severity, SourceChange, NavigationTarget, ReferenceAccess, Runnable, Severity, SourceChange, SourceFileEdit,
SourceFileEdit, SymbolKind, TextEdit, TextRange, TextSize, SymbolKind, TextEdit, TextRange, TextSize,
}; };
use itertools::Itertools; use itertools::Itertools;
@ -780,6 +780,7 @@ pub(crate) fn unresolved_code_action(
assist: Assist, assist: Assist,
index: usize, index: usize,
) -> Result<lsp_ext::CodeAction> { ) -> Result<lsp_ext::CodeAction> {
assert!(assist.source_change.is_none());
let res = lsp_ext::CodeAction { let res = lsp_ext::CodeAction {
title: assist.label.to_string(), title: assist.label.to_string(),
group: assist.group.filter(|_| snap.config.client_caps.code_action_group).map(|gr| gr.0), group: assist.group.filter(|_| snap.config.client_caps.code_action_group).map(|gr| gr.0),
@ -796,18 +797,14 @@ pub(crate) fn unresolved_code_action(
pub(crate) fn resolved_code_action( pub(crate) fn resolved_code_action(
snap: &GlobalStateSnapshot, snap: &GlobalStateSnapshot,
assist: ResolvedAssist, assist: Assist,
) -> Result<lsp_ext::CodeAction> { ) -> Result<lsp_ext::CodeAction> {
let change = assist.source_change; let change = assist.source_change.unwrap();
let res = lsp_ext::CodeAction { let res = lsp_ext::CodeAction {
edit: Some(snippet_workspace_edit(snap, change)?), edit: Some(snippet_workspace_edit(snap, change)?),
title: assist.assist.label.to_string(), title: assist.label.to_string(),
group: assist group: assist.group.filter(|_| snap.config.client_caps.code_action_group).map(|gr| gr.0),
.assist kind: Some(code_action_kind(assist.id.1)),
.group
.filter(|_| snap.config.client_caps.code_action_group)
.map(|gr| gr.0),
kind: Some(code_action_kind(assist.assist.id.1)),
is_preferred: None, is_preferred: None,
data: None, data: None,
}; };