From 6ebc8bbeb005f5d3f2b00d1ae1f1804116e3a8f5 Mon Sep 17 00:00:00 2001 From: Benjamin Coenen <5719034+bnjjj@users.noreply.github.com> Date: Thu, 16 Apr 2020 18:30:08 +0200 Subject: feat: improve dot completions with scoring Signed-off-by: Benjamin Coenen <5719034+bnjjj@users.noreply.github.com> --- crates/ra_ide/src/completion.rs | 6 +- crates/ra_ide/src/completion/complete_dot.rs | 114 +++++++++--------------- crates/ra_ide/src/completion/completion_item.rs | 62 ++++++++----- crates/ra_ide/src/completion/presentation.rs | 2 +- crates/ra_ide/src/completion/test_utils.rs | 11 +-- crates/ra_ide/src/lib.rs | 4 +- crates/rust-analyzer/src/conv.rs | 17 ++-- 7 files changed, 103 insertions(+), 113 deletions(-) diff --git a/crates/ra_ide/src/completion.rs b/crates/ra_ide/src/completion.rs index 185450508..38c8aed8d 100644 --- a/crates/ra_ide/src/completion.rs +++ b/crates/ra_ide/src/completion.rs @@ -29,7 +29,7 @@ use crate::{ }; pub use crate::completion::completion_item::{ - CompletionItem, CompletionItemKind, InsertTextFormat, + CompletionItem, CompletionItemKind, CompletionScore, InsertTextFormat, }; #[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_trait_impl::complete_trait_impl(&mut acc, &ctx); - // Reorder completion items if there is a sort_option - acc.sort(&ctx); + // Compute score for completion items + acc.compute_score(&ctx); Some(acc) } diff --git a/crates/ra_ide/src/completion/complete_dot.rs b/crates/ra_ide/src/completion/complete_dot.rs index 2e228b638..174b39964 100644 --- a/crates/ra_ide/src/completion/complete_dot.rs +++ b/crates/ra_ide/src/completion/complete_dot.rs @@ -6,12 +6,11 @@ use hir::{ Type, }; -use crate::completion::completion_item::CompletionKind; use crate::{ call_info::call_info, completion::{ completion_context::CompletionContext, - completion_item::{Completions, SortOption}, + completion_item::{CompletionKind, Completions, ScoreOption}, }, // CallInfo, CompletionItem, @@ -49,40 +48,12 @@ fn complete_fields(acc: &mut Completions, ctx: &CompletionContext, receiver: &Ty for receiver in receiver.autoderef(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 { - 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) { - 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 { 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 @@ -116,20 +87,13 @@ fn complete_methods(acc: &mut Completions, ctx: &CompletionContext, receiver: &T #[cfg(test)] mod tests { - use crate::completion::{ - test_utils::{do_completion, do_completion_without_sort}, - CompletionItem, CompletionKind, - }; + use crate::completion::{test_utils::do_completion, CompletionItem, CompletionKind}; use insta::assert_debug_snapshot; fn do_ref_completion(code: &str) -> Vec { do_completion(code, CompletionKind::Reference) } - fn do_ref_completion_without_sort(code: &str) -> Vec { - do_completion_without_sort(code, CompletionKind::Reference) - } - #[test] fn test_struct_field_completion() { assert_debug_snapshot!( @@ -159,7 +123,7 @@ mod tests { #[test] fn test_struct_field_completion_in_func_call() { assert_debug_snapshot!( - do_ref_completion_without_sort( + do_ref_completion( r" struct A { another_field: i64, the_field: u32, my_string: String } fn test(my_param: u32) -> u32 { my_param } @@ -170,14 +134,6 @@ mod tests { ), @r###" [ - CompletionItem { - label: "the_field", - source_range: [201; 201), - delete: [201; 201), - insert: "the_field", - kind: Field, - detail: "u32", - }, CompletionItem { label: "another_field", source_range: [201; 201), @@ -194,6 +150,15 @@ mod tests { kind: Field, 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] fn test_struct_field_completion_in_func_call_with_type_and_name() { assert_debug_snapshot!( - do_ref_completion_without_sort( + do_ref_completion( r" struct A { another_field: i64, another_good_type: u32, the_field: u32 } fn test(the_field: u32) -> u32 { the_field } @@ -214,12 +179,12 @@ mod tests { @r###" [ CompletionItem { - label: "the_field", + label: "another_field", source_range: [208; 208), delete: [208; 208), - insert: "the_field", + insert: "another_field", kind: Field, - detail: "u32", + detail: "i64", }, CompletionItem { label: "another_good_type", @@ -228,14 +193,16 @@ mod tests { insert: "another_good_type", kind: Field, detail: "u32", + score: TypeMatch, }, CompletionItem { - label: "another_field", + label: "the_field", source_range: [208; 208), delete: [208; 208), - insert: "another_field", + insert: "the_field", kind: Field, - detail: "i64", + detail: "u32", + score: TypeAndNameMatch, }, ] "### @@ -245,7 +212,7 @@ mod tests { #[test] fn test_struct_field_completion_in_record_lit() { assert_debug_snapshot!( - do_ref_completion_without_sort( + do_ref_completion( r" struct A { another_field: i64, another_good_type: u32, the_field: u32 } struct B { my_string: String, my_vec: Vec, the_field: u32 } @@ -259,12 +226,12 @@ mod tests { @r###" [ CompletionItem { - label: "the_field", + label: "another_field", source_range: [270; 270), delete: [270; 270), - insert: "the_field", + insert: "another_field", kind: Field, - detail: "u32", + detail: "i64", }, CompletionItem { label: "another_good_type", @@ -273,14 +240,16 @@ mod tests { insert: "another_good_type", kind: Field, detail: "u32", + score: TypeMatch, }, CompletionItem { - label: "another_field", + label: "the_field", source_range: [270; 270), delete: [270; 270), - insert: "another_field", + insert: "the_field", kind: Field, - detail: "i64", + detail: "u32", + score: TypeAndNameMatch, }, ] "### @@ -290,7 +259,7 @@ mod tests { #[test] fn test_struct_field_completion_in_record_lit_and_fn_call() { assert_debug_snapshot!( - do_ref_completion_without_sort( + do_ref_completion( r" struct A { another_field: i64, another_good_type: u32, the_field: u32 } struct B { my_string: String, my_vec: Vec, the_field: u32 } @@ -311,6 +280,7 @@ mod tests { insert: "another_field", kind: Field, detail: "i64", + score: TypeMatch, }, CompletionItem { label: "another_good_type", @@ -336,7 +306,7 @@ mod tests { #[test] fn test_struct_field_completion_in_fn_call_and_record_lit() { assert_debug_snapshot!( - do_ref_completion_without_sort( + do_ref_completion( r" struct A { another_field: i64, another_good_type: u32, the_field: u32 } struct B { my_string: String, my_vec: Vec, the_field: u32 } @@ -351,12 +321,12 @@ mod tests { @r###" [ CompletionItem { - label: "the_field", + label: "another_field", source_range: [328; 328), delete: [328; 328), - insert: "the_field", + insert: "another_field", kind: Field, - detail: "u32", + detail: "i64", }, CompletionItem { label: "another_good_type", @@ -365,14 +335,16 @@ mod tests { insert: "another_good_type", kind: Field, detail: "u32", + score: TypeMatch, }, CompletionItem { - label: "another_field", + label: "the_field", source_range: [328; 328), delete: [328; 328), - insert: "another_field", + insert: "the_field", kind: Field, - detail: "i64", + detail: "u32", + score: TypeAndNameMatch, }, ] "### diff --git a/crates/ra_ide/src/completion/completion_item.rs b/crates/ra_ide/src/completion/completion_item.rs index c9c3fdc0e..84d51bafe 100644 --- a/crates/ra_ide/src/completion/completion_item.rs +++ b/crates/ra_ide/src/completion/completion_item.rs @@ -53,6 +53,9 @@ pub struct CompletionItem { /// If completing a function call, ask the editor to show parameter popup /// after completion. trigger_call_info: bool, + + /// Score is usefull to pre select or display in better order completion items + score: Option, } // 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 { s.field("deprecated", &true); } + if let Some(score) = &self.score { + s.field("score", score); + } if self.trigger_call_info { s.field("trigger_call_info", &true); } @@ -149,6 +155,7 @@ impl CompletionItem { text_edit: None, deprecated: None, trigger_call_info: None, + score: None, } } /// What user sees in pop-up in the UI. @@ -188,6 +195,10 @@ impl CompletionItem { self.deprecated } + pub fn score(&self) -> Option { + self.score.clone() + } + pub fn trigger_call_info(&self) -> bool { self.trigger_call_info } @@ -208,6 +219,7 @@ pub(crate) struct Builder { text_edit: Option, deprecated: Option, trigger_call_info: Option, + score: Option, } impl Builder { @@ -237,6 +249,7 @@ impl Builder { completion_kind: self.completion_kind, deprecated: self.deprecated.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) -> Builder { @@ -287,6 +300,10 @@ impl Builder { self.deprecated = Some(deprecated); 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 { self.trigger_call_info = Some(true); self @@ -300,16 +317,22 @@ impl<'a> Into for Builder { } #[derive(Debug)] -pub(crate) enum SortOption { +pub(crate) enum ScoreOption { CallFn(CallInfo), RecordField(RecordField), } +#[derive(Debug, Clone)] +pub enum CompletionScore { + TypeMatch, + TypeAndNameMatch, +} + /// Represents an in-progress set of completions being built. #[derive(Debug, Default)] pub(crate) struct Completions { buf: Vec, - sort_option: Option, + score_option: Option, } impl Completions { @@ -324,17 +347,17 @@ impl Completions { items.into_iter().for_each(|item| self.add(item.into())) } - pub(crate) fn with_sort_option(&mut self, sort_option: SortOption) { - self.sort_option = Some(sort_option); + pub(crate) fn with_score_option(&mut self, score_option: ScoreOption) { + self.score_option = Some(score_option); } - pub(crate) fn sort(&mut self, ctx: &CompletionContext) { - if self.sort_option.is_none() { + pub(crate) fn compute_score(&mut self, ctx: &CompletionContext) { + if self.score_option.is_none() { return; } - let (active_name, active_type) = match self.sort_option.as_ref().unwrap() { - SortOption::CallFn(call_info) => { + let (active_name, active_type) = match self.score_option.as_ref().unwrap() { + ScoreOption::CallFn(call_info) => { if call_info.active_parameter_type().is_none() || call_info.active_parameter_name().is_none() { @@ -345,7 +368,7 @@ impl Completions { 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) { ( 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 - if let Some(a_parameter_type) = &a.detail { + if let Some(a_parameter_type) = &completion_item.detail { if &active_type == a_parameter_type { // If same type + same name then go top position - if active_name != a.label { - if let Some(b_parameter_type) = &b.detail { - if &active_type == b_parameter_type { - return Ordering::Equal; - } - } + if active_name == completion_item.label { + completion_item.score = Some(CompletionScore::TypeAndNameMatch); + } else { + completion_item.score = Some(CompletionScore::TypeMatch); } - Ordering::Less - } else { - Ordering::Greater } - } else { - Ordering::Greater } - }); + } } } diff --git a/crates/ra_ide/src/completion/presentation.rs b/crates/ra_ide/src/completion/presentation.rs index 8be2d02d0..55f75b15a 100644 --- a/crates/ra_ide/src/completion/presentation.rs +++ b/crates/ra_ide/src/completion/presentation.rs @@ -367,7 +367,7 @@ mod tests { ra_fixture: &str, options: CompletionConfig, ) -> Vec { - do_completion_with_options(ra_fixture, CompletionKind::Reference, &options, true) + do_completion_with_options(ra_fixture, CompletionKind::Reference, &options) } #[test] diff --git a/crates/ra_ide/src/completion/test_utils.rs b/crates/ra_ide/src/completion/test_utils.rs index f54d15a90..eb90b5279 100644 --- a/crates/ra_ide/src/completion/test_utils.rs +++ b/crates/ra_ide/src/completion/test_utils.rs @@ -7,18 +7,13 @@ use crate::{ }; pub(crate) fn do_completion(code: &str, kind: CompletionKind) -> Vec { - do_completion_with_options(code, kind, &CompletionConfig::default(), true) -} - -pub(crate) fn do_completion_without_sort(code: &str, kind: CompletionKind) -> Vec { - do_completion_with_options(code, kind, &CompletionConfig::default(), false) + do_completion_with_options(code, kind, &CompletionConfig::default()) } pub(crate) fn do_completion_with_options( code: &str, kind: CompletionKind, options: &CompletionConfig, - sort_by_key: bool, ) -> Vec { let (analysis, position) = if code.contains("//-") { analysis_and_position(code) @@ -29,8 +24,6 @@ pub(crate) fn do_completion_with_options( let completion_items: Vec = completions.into(); let mut kind_completions: Vec = 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 } diff --git a/crates/ra_ide/src/lib.rs b/crates/ra_ide/src/lib.rs index 357c01cdf..ddaa30a16 100644 --- a/crates/ra_ide/src/lib.rs +++ b/crates/ra_ide/src/lib.rs @@ -67,7 +67,9 @@ use crate::display::ToNav; pub use crate::{ assists::{Assist, AssistId}, call_hierarchy::CallItem, - completion::{CompletionConfig, CompletionItem, CompletionItemKind, InsertTextFormat}, + completion::{ + CompletionConfig, CompletionItem, CompletionItemKind, CompletionScore, InsertTextFormat, + }, diagnostics::Severity, display::{file_structure, FunctionSignature, NavigationTarget, StructureNode}, expand_macro::ExpandedMacro, diff --git a/crates/rust-analyzer/src/conv.rs b/crates/rust-analyzer/src/conv.rs index 79bc1fa41..f47d931fd 100644 --- a/crates/rust-analyzer/src/conv.rs +++ b/crates/rust-analyzer/src/conv.rs @@ -9,10 +9,10 @@ use lsp_types::{ TextDocumentPositionParams, Url, VersionedTextDocumentIdentifier, WorkspaceEdit, }; use ra_ide::{ - translate_offset_with_edit, CompletionItem, CompletionItemKind, FileId, FilePosition, - FileRange, FileSystemEdit, Fold, FoldKind, Highlight, HighlightModifier, HighlightTag, - InlayHint, InlayKind, InsertTextFormat, LineCol, LineIndex, NavigationTarget, RangeInfo, - ReferenceAccess, Severity, SourceChange, SourceFileEdit, + translate_offset_with_edit, CompletionItem, CompletionItemKind, CompletionScore, FileId, + FilePosition, FileRange, FileSystemEdit, Fold, FoldKind, Highlight, HighlightModifier, + HighlightTag, InlayHint, InlayKind, InsertTextFormat, LineCol, LineIndex, NavigationTarget, + RangeInfo, ReferenceAccess, Severity, SourceChange, SourceFileEdit, }; use ra_syntax::{SyntaxKind, TextRange, TextUnit}; use ra_text_edit::{AtomTextEdit, TextEdit}; @@ -147,7 +147,6 @@ impl ConvWith<(&LineIndex, LineEndings, usize)> for CompletionItem { filter_text: Some(self.lookup().to_string()), kind: self.kind().map(|it| it.conv()), text_edit: Some(text_edit), - sort_text: Some(format!("{:02}", ctx.2)), additional_text_edits: Some(additional_text_edits), documentation: self.documentation().map(|it| it.conv()), deprecated: Some(self.deprecated()), @@ -164,6 +163,14 @@ impl ConvWith<(&LineIndex, LineEndings, usize)> for CompletionItem { ..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() { res.tags = Some(vec![lsp_types::CompletionItemTag::Deprecated]) } -- cgit v1.2.3