feat: improve dot completions with scoring

Signed-off-by: Benjamin Coenen <5719034+bnjjj@users.noreply.github.com>
This commit is contained in:
Benjamin Coenen 2020-04-16 18:30:08 +02:00
parent 06076f95a7
commit 6ebc8bbeb0
7 changed files with 103 additions and 113 deletions

View file

@ -29,7 +29,7 @@ use crate::{
}; };
pub use crate::completion::completion_item::{ pub use crate::completion::completion_item::{
CompletionItem, CompletionItemKind, InsertTextFormat, CompletionItem, CompletionItemKind, CompletionScore, InsertTextFormat,
}; };
#[derive(Clone, Debug, PartialEq, Eq)] #[derive(Clone, Debug, PartialEq, Eq)]
@ -94,8 +94,8 @@ pub(crate) fn completions(
complete_macro_in_item_position::complete_macro_in_item_position(&mut acc, &ctx); complete_macro_in_item_position::complete_macro_in_item_position(&mut acc, &ctx);
complete_trait_impl::complete_trait_impl(&mut acc, &ctx); complete_trait_impl::complete_trait_impl(&mut acc, &ctx);
// Reorder completion items if there is a sort_option // Compute score for completion items
acc.sort(&ctx); acc.compute_score(&ctx);
Some(acc) Some(acc)
} }

View file

@ -6,12 +6,11 @@ use hir::{
Type, Type,
}; };
use crate::completion::completion_item::CompletionKind;
use crate::{ use crate::{
call_info::call_info, call_info::call_info,
completion::{ completion::{
completion_context::CompletionContext, completion_context::CompletionContext,
completion_item::{Completions, SortOption}, completion_item::{CompletionKind, Completions, ScoreOption},
}, },
// CallInfo, // CallInfo,
CompletionItem, CompletionItem,
@ -49,40 +48,12 @@ fn complete_fields(acc: &mut Completions, ctx: &CompletionContext, receiver: &Ty
for receiver in receiver.autoderef(ctx.db) { for receiver in receiver.autoderef(ctx.db) {
let fields = receiver.fields(ctx.db); let fields = receiver.fields(ctx.db);
// If we use this implementation we can delete call_info in the CompletionContext
if let Some(record_field) = &ctx.record_field_syntax { if let Some(record_field) = &ctx.record_field_syntax {
acc.with_sort_option(SortOption::RecordField(record_field.clone())); acc.with_score_option(ScoreOption::RecordField(record_field.clone()));
} else if let Some(call_info) = call_info(ctx.db, ctx.file_position) { } else if let Some(call_info) = call_info(ctx.db, ctx.file_position) {
acc.with_sort_option(SortOption::CallFn(call_info)); acc.with_score_option(ScoreOption::CallFn(call_info));
} }
// // For Call Fn
// if let Some(call_info) = &ctx.call_info {
// if let Some(active_parameter_type) = call_info.active_parameter_type() {
// let active_parameter_name = call_info.active_parameter_name().unwrap();
// fields.sort_by(|a, b| {
// // For the same type
// if active_parameter_type == a.1.display(ctx.db).to_string() {
// // If same type + same name then go top position
// if active_parameter_name == a.0.name(ctx.db).to_string() {
// Ordering::Less
// } else {
// if active_parameter_type == b.1.display(ctx.db).to_string() {
// Ordering::Equal
// } else {
// Ordering::Less
// }
// }
// } else {
// Ordering::Greater
// }
// });
// }
// }
// For Lit struct fields
// ---
for (field, ty) in fields { for (field, ty) in fields {
if ctx.scope().module().map_or(false, |m| !field.is_visible_from(ctx.db, m)) { if ctx.scope().module().map_or(false, |m| !field.is_visible_from(ctx.db, m)) {
// Skip private field. FIXME: If the definition location of the // Skip private field. FIXME: If the definition location of the
@ -116,20 +87,13 @@ fn complete_methods(acc: &mut Completions, ctx: &CompletionContext, receiver: &T
#[cfg(test)] #[cfg(test)]
mod tests { mod tests {
use crate::completion::{ use crate::completion::{test_utils::do_completion, CompletionItem, CompletionKind};
test_utils::{do_completion, do_completion_without_sort},
CompletionItem, CompletionKind,
};
use insta::assert_debug_snapshot; use insta::assert_debug_snapshot;
fn do_ref_completion(code: &str) -> Vec<CompletionItem> { fn do_ref_completion(code: &str) -> Vec<CompletionItem> {
do_completion(code, CompletionKind::Reference) do_completion(code, CompletionKind::Reference)
} }
fn do_ref_completion_without_sort(code: &str) -> Vec<CompletionItem> {
do_completion_without_sort(code, CompletionKind::Reference)
}
#[test] #[test]
fn test_struct_field_completion() { fn test_struct_field_completion() {
assert_debug_snapshot!( assert_debug_snapshot!(
@ -159,7 +123,7 @@ mod tests {
#[test] #[test]
fn test_struct_field_completion_in_func_call() { fn test_struct_field_completion_in_func_call() {
assert_debug_snapshot!( assert_debug_snapshot!(
do_ref_completion_without_sort( do_ref_completion(
r" r"
struct A { another_field: i64, the_field: u32, my_string: String } struct A { another_field: i64, the_field: u32, my_string: String }
fn test(my_param: u32) -> u32 { my_param } fn test(my_param: u32) -> u32 { my_param }
@ -170,14 +134,6 @@ mod tests {
), ),
@r###" @r###"
[ [
CompletionItem {
label: "the_field",
source_range: [201; 201),
delete: [201; 201),
insert: "the_field",
kind: Field,
detail: "u32",
},
CompletionItem { CompletionItem {
label: "another_field", label: "another_field",
source_range: [201; 201), source_range: [201; 201),
@ -194,6 +150,15 @@ mod tests {
kind: Field, kind: Field,
detail: "{unknown}", detail: "{unknown}",
}, },
CompletionItem {
label: "the_field",
source_range: [201; 201),
delete: [201; 201),
insert: "the_field",
kind: Field,
detail: "u32",
score: TypeMatch,
},
] ]
"### "###
); );
@ -202,7 +167,7 @@ mod tests {
#[test] #[test]
fn test_struct_field_completion_in_func_call_with_type_and_name() { fn test_struct_field_completion_in_func_call_with_type_and_name() {
assert_debug_snapshot!( assert_debug_snapshot!(
do_ref_completion_without_sort( do_ref_completion(
r" r"
struct A { another_field: i64, another_good_type: u32, the_field: u32 } struct A { another_field: i64, another_good_type: u32, the_field: u32 }
fn test(the_field: u32) -> u32 { the_field } fn test(the_field: u32) -> u32 { the_field }
@ -214,12 +179,12 @@ mod tests {
@r###" @r###"
[ [
CompletionItem { CompletionItem {
label: "the_field", label: "another_field",
source_range: [208; 208), source_range: [208; 208),
delete: [208; 208), delete: [208; 208),
insert: "the_field", insert: "another_field",
kind: Field, kind: Field,
detail: "u32", detail: "i64",
}, },
CompletionItem { CompletionItem {
label: "another_good_type", label: "another_good_type",
@ -228,14 +193,16 @@ mod tests {
insert: "another_good_type", insert: "another_good_type",
kind: Field, kind: Field,
detail: "u32", detail: "u32",
score: TypeMatch,
}, },
CompletionItem { CompletionItem {
label: "another_field", label: "the_field",
source_range: [208; 208), source_range: [208; 208),
delete: [208; 208), delete: [208; 208),
insert: "another_field", insert: "the_field",
kind: Field, kind: Field,
detail: "i64", detail: "u32",
score: TypeAndNameMatch,
}, },
] ]
"### "###
@ -245,7 +212,7 @@ mod tests {
#[test] #[test]
fn test_struct_field_completion_in_record_lit() { fn test_struct_field_completion_in_record_lit() {
assert_debug_snapshot!( assert_debug_snapshot!(
do_ref_completion_without_sort( do_ref_completion(
r" r"
struct A { another_field: i64, another_good_type: u32, the_field: u32 } struct A { another_field: i64, another_good_type: u32, the_field: u32 }
struct B { my_string: String, my_vec: Vec<u32>, the_field: u32 } struct B { my_string: String, my_vec: Vec<u32>, the_field: u32 }
@ -259,12 +226,12 @@ mod tests {
@r###" @r###"
[ [
CompletionItem { CompletionItem {
label: "the_field", label: "another_field",
source_range: [270; 270), source_range: [270; 270),
delete: [270; 270), delete: [270; 270),
insert: "the_field", insert: "another_field",
kind: Field, kind: Field,
detail: "u32", detail: "i64",
}, },
CompletionItem { CompletionItem {
label: "another_good_type", label: "another_good_type",
@ -273,14 +240,16 @@ mod tests {
insert: "another_good_type", insert: "another_good_type",
kind: Field, kind: Field,
detail: "u32", detail: "u32",
score: TypeMatch,
}, },
CompletionItem { CompletionItem {
label: "another_field", label: "the_field",
source_range: [270; 270), source_range: [270; 270),
delete: [270; 270), delete: [270; 270),
insert: "another_field", insert: "the_field",
kind: Field, kind: Field,
detail: "i64", detail: "u32",
score: TypeAndNameMatch,
}, },
] ]
"### "###
@ -290,7 +259,7 @@ mod tests {
#[test] #[test]
fn test_struct_field_completion_in_record_lit_and_fn_call() { fn test_struct_field_completion_in_record_lit_and_fn_call() {
assert_debug_snapshot!( assert_debug_snapshot!(
do_ref_completion_without_sort( do_ref_completion(
r" r"
struct A { another_field: i64, another_good_type: u32, the_field: u32 } struct A { another_field: i64, another_good_type: u32, the_field: u32 }
struct B { my_string: String, my_vec: Vec<u32>, the_field: u32 } struct B { my_string: String, my_vec: Vec<u32>, the_field: u32 }
@ -311,6 +280,7 @@ mod tests {
insert: "another_field", insert: "another_field",
kind: Field, kind: Field,
detail: "i64", detail: "i64",
score: TypeMatch,
}, },
CompletionItem { CompletionItem {
label: "another_good_type", label: "another_good_type",
@ -336,7 +306,7 @@ mod tests {
#[test] #[test]
fn test_struct_field_completion_in_fn_call_and_record_lit() { fn test_struct_field_completion_in_fn_call_and_record_lit() {
assert_debug_snapshot!( assert_debug_snapshot!(
do_ref_completion_without_sort( do_ref_completion(
r" r"
struct A { another_field: i64, another_good_type: u32, the_field: u32 } struct A { another_field: i64, another_good_type: u32, the_field: u32 }
struct B { my_string: String, my_vec: Vec<u32>, the_field: u32 } struct B { my_string: String, my_vec: Vec<u32>, the_field: u32 }
@ -351,12 +321,12 @@ mod tests {
@r###" @r###"
[ [
CompletionItem { CompletionItem {
label: "the_field", label: "another_field",
source_range: [328; 328), source_range: [328; 328),
delete: [328; 328), delete: [328; 328),
insert: "the_field", insert: "another_field",
kind: Field, kind: Field,
detail: "u32", detail: "i64",
}, },
CompletionItem { CompletionItem {
label: "another_good_type", label: "another_good_type",
@ -365,14 +335,16 @@ mod tests {
insert: "another_good_type", insert: "another_good_type",
kind: Field, kind: Field,
detail: "u32", detail: "u32",
score: TypeMatch,
}, },
CompletionItem { CompletionItem {
label: "another_field", label: "the_field",
source_range: [328; 328), source_range: [328; 328),
delete: [328; 328), delete: [328; 328),
insert: "another_field", insert: "the_field",
kind: Field, kind: Field,
detail: "i64", detail: "u32",
score: TypeAndNameMatch,
}, },
] ]
"### "###

View file

@ -53,6 +53,9 @@ pub struct CompletionItem {
/// If completing a function call, ask the editor to show parameter popup /// If completing a function call, ask the editor to show parameter popup
/// after completion. /// after completion.
trigger_call_info: bool, trigger_call_info: bool,
/// Score is usefull to pre select or display in better order completion items
score: Option<CompletionScore>,
} }
// We use custom debug for CompletionItem to make `insta`'s diffs more readable. // We use custom debug for CompletionItem to make `insta`'s diffs more readable.
@ -82,6 +85,9 @@ impl fmt::Debug for CompletionItem {
if self.deprecated { if self.deprecated {
s.field("deprecated", &true); s.field("deprecated", &true);
} }
if let Some(score) = &self.score {
s.field("score", score);
}
if self.trigger_call_info { if self.trigger_call_info {
s.field("trigger_call_info", &true); s.field("trigger_call_info", &true);
} }
@ -149,6 +155,7 @@ impl CompletionItem {
text_edit: None, text_edit: None,
deprecated: None, deprecated: None,
trigger_call_info: None, trigger_call_info: None,
score: None,
} }
} }
/// What user sees in pop-up in the UI. /// What user sees in pop-up in the UI.
@ -188,6 +195,10 @@ impl CompletionItem {
self.deprecated self.deprecated
} }
pub fn score(&self) -> Option<CompletionScore> {
self.score.clone()
}
pub fn trigger_call_info(&self) -> bool { pub fn trigger_call_info(&self) -> bool {
self.trigger_call_info self.trigger_call_info
} }
@ -208,6 +219,7 @@ pub(crate) struct Builder {
text_edit: Option<TextEdit>, text_edit: Option<TextEdit>,
deprecated: Option<bool>, deprecated: Option<bool>,
trigger_call_info: Option<bool>, trigger_call_info: Option<bool>,
score: Option<CompletionScore>,
} }
impl Builder { impl Builder {
@ -237,6 +249,7 @@ impl Builder {
completion_kind: self.completion_kind, completion_kind: self.completion_kind,
deprecated: self.deprecated.unwrap_or(false), deprecated: self.deprecated.unwrap_or(false),
trigger_call_info: self.trigger_call_info.unwrap_or(false), trigger_call_info: self.trigger_call_info.unwrap_or(false),
score: self.score,
} }
} }
pub(crate) fn lookup_by(mut self, lookup: impl Into<String>) -> Builder { pub(crate) fn lookup_by(mut self, lookup: impl Into<String>) -> Builder {
@ -287,6 +300,10 @@ impl Builder {
self.deprecated = Some(deprecated); self.deprecated = Some(deprecated);
self self
} }
pub(crate) fn set_score(mut self, score: CompletionScore) -> Builder {
self.score = Some(score);
self
}
pub(crate) fn trigger_call_info(mut self) -> Builder { pub(crate) fn trigger_call_info(mut self) -> Builder {
self.trigger_call_info = Some(true); self.trigger_call_info = Some(true);
self self
@ -300,16 +317,22 @@ impl<'a> Into<CompletionItem> for Builder {
} }
#[derive(Debug)] #[derive(Debug)]
pub(crate) enum SortOption { pub(crate) enum ScoreOption {
CallFn(CallInfo), CallFn(CallInfo),
RecordField(RecordField), RecordField(RecordField),
} }
#[derive(Debug, Clone)]
pub enum CompletionScore {
TypeMatch,
TypeAndNameMatch,
}
/// Represents an in-progress set of completions being built. /// Represents an in-progress set of completions being built.
#[derive(Debug, Default)] #[derive(Debug, Default)]
pub(crate) struct Completions { pub(crate) struct Completions {
buf: Vec<CompletionItem>, buf: Vec<CompletionItem>,
sort_option: Option<SortOption>, score_option: Option<ScoreOption>,
} }
impl Completions { impl Completions {
@ -324,17 +347,17 @@ impl Completions {
items.into_iter().for_each(|item| self.add(item.into())) items.into_iter().for_each(|item| self.add(item.into()))
} }
pub(crate) fn with_sort_option(&mut self, sort_option: SortOption) { pub(crate) fn with_score_option(&mut self, score_option: ScoreOption) {
self.sort_option = Some(sort_option); self.score_option = Some(score_option);
} }
pub(crate) fn sort(&mut self, ctx: &CompletionContext) { pub(crate) fn compute_score(&mut self, ctx: &CompletionContext) {
if self.sort_option.is_none() { if self.score_option.is_none() {
return; return;
} }
let (active_name, active_type) = match self.sort_option.as_ref().unwrap() { let (active_name, active_type) = match self.score_option.as_ref().unwrap() {
SortOption::CallFn(call_info) => { ScoreOption::CallFn(call_info) => {
if call_info.active_parameter_type().is_none() if call_info.active_parameter_type().is_none()
|| call_info.active_parameter_name().is_none() || call_info.active_parameter_name().is_none()
{ {
@ -345,7 +368,7 @@ impl Completions {
call_info.active_parameter_type().unwrap(), call_info.active_parameter_type().unwrap(),
) )
} }
SortOption::RecordField(record_field) => { ScoreOption::RecordField(record_field) => {
if let Some((struct_field, _)) = ctx.sema.resolve_record_field(record_field) { if let Some((struct_field, _)) = ctx.sema.resolve_record_field(record_field) {
( (
struct_field.name(ctx.db).to_string(), struct_field.name(ctx.db).to_string(),
@ -357,26 +380,19 @@ impl Completions {
} }
}; };
self.buf.sort_by(|a, b| { for completion_item in &mut self.buf {
// For the same type // For the same type
if let Some(a_parameter_type) = &a.detail { if let Some(a_parameter_type) = &completion_item.detail {
if &active_type == a_parameter_type { if &active_type == a_parameter_type {
// If same type + same name then go top position // If same type + same name then go top position
if active_name != a.label { if active_name == completion_item.label {
if let Some(b_parameter_type) = &b.detail { completion_item.score = Some(CompletionScore::TypeAndNameMatch);
if &active_type == b_parameter_type { } else {
return Ordering::Equal; completion_item.score = Some(CompletionScore::TypeMatch);
}
}
} }
Ordering::Less
} else {
Ordering::Greater
} }
} else {
Ordering::Greater
} }
}); }
} }
} }

View file

@ -367,7 +367,7 @@ mod tests {
ra_fixture: &str, ra_fixture: &str,
options: CompletionConfig, options: CompletionConfig,
) -> Vec<CompletionItem> { ) -> Vec<CompletionItem> {
do_completion_with_options(ra_fixture, CompletionKind::Reference, &options, true) do_completion_with_options(ra_fixture, CompletionKind::Reference, &options)
} }
#[test] #[test]

View file

@ -7,18 +7,13 @@ use crate::{
}; };
pub(crate) fn do_completion(code: &str, kind: CompletionKind) -> Vec<CompletionItem> { pub(crate) fn do_completion(code: &str, kind: CompletionKind) -> Vec<CompletionItem> {
do_completion_with_options(code, kind, &CompletionConfig::default(), true) do_completion_with_options(code, kind, &CompletionConfig::default())
}
pub(crate) fn do_completion_without_sort(code: &str, kind: CompletionKind) -> Vec<CompletionItem> {
do_completion_with_options(code, kind, &CompletionConfig::default(), false)
} }
pub(crate) fn do_completion_with_options( pub(crate) fn do_completion_with_options(
code: &str, code: &str,
kind: CompletionKind, kind: CompletionKind,
options: &CompletionConfig, options: &CompletionConfig,
sort_by_key: bool,
) -> Vec<CompletionItem> { ) -> Vec<CompletionItem> {
let (analysis, position) = if code.contains("//-") { let (analysis, position) = if code.contains("//-") {
analysis_and_position(code) analysis_and_position(code)
@ -29,8 +24,6 @@ pub(crate) fn do_completion_with_options(
let completion_items: Vec<CompletionItem> = completions.into(); let completion_items: Vec<CompletionItem> = completions.into();
let mut kind_completions: Vec<CompletionItem> = let mut kind_completions: Vec<CompletionItem> =
completion_items.into_iter().filter(|c| c.completion_kind == kind).collect(); completion_items.into_iter().filter(|c| c.completion_kind == kind).collect();
if sort_by_key { kind_completions.sort_by_key(|c| c.label().to_owned());
kind_completions.sort_by_key(|c| c.label().to_owned());
}
kind_completions kind_completions
} }

View file

@ -67,7 +67,9 @@ use crate::display::ToNav;
pub use crate::{ pub use crate::{
assists::{Assist, AssistId}, assists::{Assist, AssistId},
call_hierarchy::CallItem, call_hierarchy::CallItem,
completion::{CompletionConfig, CompletionItem, CompletionItemKind, InsertTextFormat}, completion::{
CompletionConfig, CompletionItem, CompletionItemKind, CompletionScore, InsertTextFormat,
},
diagnostics::Severity, diagnostics::Severity,
display::{file_structure, FunctionSignature, NavigationTarget, StructureNode}, display::{file_structure, FunctionSignature, NavigationTarget, StructureNode},
expand_macro::ExpandedMacro, expand_macro::ExpandedMacro,

View file

@ -9,10 +9,10 @@ use lsp_types::{
TextDocumentPositionParams, Url, VersionedTextDocumentIdentifier, WorkspaceEdit, TextDocumentPositionParams, Url, VersionedTextDocumentIdentifier, WorkspaceEdit,
}; };
use ra_ide::{ use ra_ide::{
translate_offset_with_edit, CompletionItem, CompletionItemKind, FileId, FilePosition, translate_offset_with_edit, CompletionItem, CompletionItemKind, CompletionScore, FileId,
FileRange, FileSystemEdit, Fold, FoldKind, Highlight, HighlightModifier, HighlightTag, FilePosition, FileRange, FileSystemEdit, Fold, FoldKind, Highlight, HighlightModifier,
InlayHint, InlayKind, InsertTextFormat, LineCol, LineIndex, NavigationTarget, RangeInfo, HighlightTag, InlayHint, InlayKind, InsertTextFormat, LineCol, LineIndex, NavigationTarget,
ReferenceAccess, Severity, SourceChange, SourceFileEdit, RangeInfo, ReferenceAccess, Severity, SourceChange, SourceFileEdit,
}; };
use ra_syntax::{SyntaxKind, TextRange, TextUnit}; use ra_syntax::{SyntaxKind, TextRange, TextUnit};
use ra_text_edit::{AtomTextEdit, TextEdit}; use ra_text_edit::{AtomTextEdit, TextEdit};
@ -147,7 +147,6 @@ impl ConvWith<(&LineIndex, LineEndings, usize)> for CompletionItem {
filter_text: Some(self.lookup().to_string()), filter_text: Some(self.lookup().to_string()),
kind: self.kind().map(|it| it.conv()), kind: self.kind().map(|it| it.conv()),
text_edit: Some(text_edit), text_edit: Some(text_edit),
sort_text: Some(format!("{:02}", ctx.2)),
additional_text_edits: Some(additional_text_edits), additional_text_edits: Some(additional_text_edits),
documentation: self.documentation().map(|it| it.conv()), documentation: self.documentation().map(|it| it.conv()),
deprecated: Some(self.deprecated()), deprecated: Some(self.deprecated()),
@ -164,6 +163,14 @@ impl ConvWith<(&LineIndex, LineEndings, usize)> for CompletionItem {
..Default::default() ..Default::default()
}; };
if let Some(score) = self.score() {
match score {
CompletionScore::TypeAndNameMatch => res.preselect = Some(true),
CompletionScore::TypeMatch => {}
}
res.sort_text = Some(format!("{:02}", ctx.2));
}
if self.deprecated() { if self.deprecated() {
res.tags = Some(vec![lsp_types::CompletionItemTag::Deprecated]) res.tags = Some(vec![lsp_types::CompletionItemTag::Deprecated])
} }