7945: Future proof completion scores r=matklad a=matklad

bors r+
🤖

Co-authored-by: Aleksey Kladov <aleksey.kladov@gmail.com>
This commit is contained in:
bors[bot] 2021-03-09 17:25:23 +00:00 committed by GitHub
commit 84eed2136b
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
4 changed files with 96 additions and 72 deletions

View file

@ -63,8 +63,14 @@ pub struct CompletionItem {
/// after completion. /// after completion.
trigger_call_info: bool, trigger_call_info: bool,
/// Score is useful to pre select or display in better order completion items /// We use this to sort completion. Relevance records facts like "do the
score: Option<CompletionScore>, /// types align precisely?". We can't sort by relevances directly, they are
/// only partially ordered.
///
/// Note that Relevance ignores fuzzy match score. We compute Relevance for
/// all possible items, and then separately build an ordered completion list
/// based on relevance and fuzzy matching with the already typed identifier.
relevance: Relevance,
/// Indicates that a reference or mutable reference to this variable is a /// Indicates that a reference or mutable reference to this variable is a
/// possible match. /// possible match.
@ -101,8 +107,8 @@ 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 { if self.relevance.is_relevant() {
s.field("score", score); s.field("relevance", &self.relevance);
} }
if let Some(mutability) = &self.ref_match { if let Some(mutability) = &self.ref_match {
s.field("ref_match", &format!("&{}", mutability.as_keyword_for_ref())); s.field("ref_match", &format!("&{}", mutability.as_keyword_for_ref()));
@ -122,6 +128,36 @@ pub enum CompletionScore {
TypeAndNameMatch, TypeAndNameMatch,
} }
#[derive(Debug, Clone, Copy, Ord, PartialOrd, Eq, PartialEq, Default)]
pub struct Relevance {
/// This is set in cases like these:
///
/// ```
/// fn f(spam: String) {}
/// fn main {
/// let spam = 92;
/// f($0) // name of local matches the name of param
/// }
/// ```
pub exact_name_match: bool,
/// This is set in cases like these:
///
/// ```
/// fn f(spam: String) {}
/// fn main {
/// let foo = String::new();
/// f($0) // type of local matches the type of param
/// }
/// ```
pub exact_type_match: bool,
}
impl Relevance {
pub fn is_relevant(&self) -> bool {
self != &Relevance::default()
}
}
#[derive(Debug, Clone, Copy, PartialEq, Eq)] #[derive(Debug, Clone, Copy, PartialEq, Eq)]
pub enum CompletionItemKind { pub enum CompletionItemKind {
SymbolKind(SymbolKind), SymbolKind(SymbolKind),
@ -213,7 +249,7 @@ impl CompletionItem {
text_edit: None, text_edit: None,
deprecated: false, deprecated: false,
trigger_call_info: None, trigger_call_info: None,
score: None, relevance: Relevance::default(),
ref_match: None, ref_match: None,
import_to_add: None, import_to_add: None,
} }
@ -256,8 +292,8 @@ impl CompletionItem {
self.deprecated self.deprecated
} }
pub fn score(&self) -> Option<CompletionScore> { pub fn relevance(&self) -> Relevance {
self.score self.relevance
} }
pub fn trigger_call_info(&self) -> bool { pub fn trigger_call_info(&self) -> bool {
@ -313,7 +349,7 @@ pub(crate) struct Builder {
text_edit: Option<TextEdit>, text_edit: Option<TextEdit>,
deprecated: bool, deprecated: bool,
trigger_call_info: Option<bool>, trigger_call_info: Option<bool>,
score: Option<CompletionScore>, relevance: Relevance,
ref_match: Option<Mutability>, ref_match: Option<Mutability>,
} }
@ -360,7 +396,7 @@ impl Builder {
completion_kind: self.completion_kind, completion_kind: self.completion_kind,
deprecated: self.deprecated, deprecated: self.deprecated,
trigger_call_info: self.trigger_call_info.unwrap_or(false), trigger_call_info: self.trigger_call_info.unwrap_or(false),
score: self.score, relevance: self.relevance,
ref_match: self.ref_match, ref_match: self.ref_match,
import_to_add: self.import_to_add, import_to_add: self.import_to_add,
} }
@ -421,8 +457,8 @@ impl Builder {
self.deprecated = deprecated; self.deprecated = deprecated;
self self
} }
pub(crate) fn set_score(mut self, score: CompletionScore) -> Builder { pub(crate) fn set_relevance(mut self, relevance: Relevance) -> Builder {
self.score = Some(score); self.relevance = relevance;
self self
} }
pub(crate) fn trigger_call_info(mut self) -> Builder { pub(crate) fn trigger_call_info(mut self) -> Builder {

View file

@ -23,7 +23,10 @@ use crate::{completions::Completions, context::CompletionContext, item::Completi
pub use crate::{ pub use crate::{
config::CompletionConfig, config::CompletionConfig,
item::{CompletionItem, CompletionItemKind, CompletionScore, ImportEdit, InsertTextFormat}, item::{
CompletionItem, CompletionItemKind, CompletionScore, ImportEdit, InsertTextFormat,
Relevance,
},
}; };
//FIXME: split the following feature into fine-grained features. //FIXME: split the following feature into fine-grained features.

View file

@ -20,8 +20,8 @@ use ide_db::{
use syntax::TextRange; use syntax::TextRange;
use crate::{ use crate::{
item::ImportEdit, CompletionContext, CompletionItem, CompletionItemKind, CompletionKind, item::{ImportEdit, Relevance},
CompletionScore, CompletionContext, CompletionItem, CompletionItemKind, CompletionKind,
}; };
use crate::render::{enum_variant::render_variant, function::render_fn, macro_::render_macro}; use crate::render::{enum_variant::render_variant, function::render_fn, macro_::render_macro};
@ -117,7 +117,7 @@ impl<'a> RenderContext<'a> {
node.docs(self.db()) node.docs(self.db())
} }
fn active_name_and_type(&self) -> Option<(String, Type)> { fn expected_name_and_type(&self) -> Option<(String, Type)> {
if let Some(record_field) = &self.completion.record_field_syntax { if let Some(record_field) = &self.completion.record_field_syntax {
cov_mark::hit!(record_field_type_match); cov_mark::hit!(record_field_type_match);
let (struct_field, _local) = self.completion.sema.resolve_record_field(record_field)?; let (struct_field, _local) = self.completion.sema.resolve_record_field(record_field)?;
@ -155,8 +155,8 @@ impl<'a> Render<'a> {
.set_documentation(field.docs(self.ctx.db())) .set_documentation(field.docs(self.ctx.db()))
.set_deprecated(is_deprecated); .set_deprecated(is_deprecated);
if let Some(score) = compute_score(&self.ctx, &ty, &name.to_string()) { if let Some(relevance) = compute_relevance(&self.ctx, &ty, &name.to_string()) {
item = item.set_score(score); item = item.set_relevance(relevance);
} }
item.build() item.build()
@ -247,18 +247,15 @@ impl<'a> Render<'a> {
}; };
if let ScopeDef::Local(local) = resolution { if let ScopeDef::Local(local) = resolution {
if let Some((active_name, active_type)) = self.ctx.active_name_and_type() {
let ty = local.ty(self.ctx.db()); let ty = local.ty(self.ctx.db());
if let Some(score) = if let Some(relevance) = compute_relevance(&self.ctx, &ty, &local_name) {
compute_score_from_active(&active_type, &active_name, &ty, &local_name) item = item.set_relevance(relevance)
{
item = item.set_score(score);
} }
if let Some((_expected_name, expected_type)) = self.ctx.expected_name_and_type() {
if let Some(ty_without_ref) = active_type.remove_ref() { if let Some(ty_without_ref) = expected_type.remove_ref() {
if ty_without_ref == ty { if ty_without_ref == ty {
cov_mark::hit!(suggest_ref); cov_mark::hit!(suggest_ref);
let mutability = if active_type.is_mutable_reference() { let mutability = if expected_type.is_mutable_reference() {
Mutability::Mut Mutability::Mut
} else { } else {
Mutability::Shared Mutability::Shared
@ -326,33 +323,14 @@ impl<'a> Render<'a> {
} }
} }
fn compute_score_from_active( fn compute_relevance(ctx: &RenderContext, ty: &Type, name: &str) -> Option<Relevance> {
active_type: &Type, let (expected_name, expected_type) = ctx.expected_name_and_type()?;
active_name: &str, let mut res = Relevance::default();
ty: &Type, res.exact_type_match = ty == &expected_type;
name: &str, res.exact_name_match = name == &expected_name;
) -> Option<CompletionScore> {
// Compute score
// For the same type
if active_type != ty {
return None;
}
let mut res = CompletionScore::TypeMatch;
// If same type + same name then go top position
if active_name == name {
res = CompletionScore::TypeAndNameMatch
}
Some(res) Some(res)
} }
fn compute_score(ctx: &RenderContext, ty: &Type, name: &str) -> Option<CompletionScore> {
let (active_name, active_type) = ctx.active_name_and_type()?;
compute_score_from_active(&active_type, &active_name, ty, name)
}
#[cfg(test)] #[cfg(test)]
mod tests { mod tests {
use std::cmp::Reverse; use std::cmp::Reverse;
@ -361,7 +339,7 @@ mod tests {
use crate::{ use crate::{
test_utils::{check_edit, do_completion, get_all_items, TEST_CONFIG}, test_utils::{check_edit, do_completion, get_all_items, TEST_CONFIG},
CompletionKind, CompletionScore, CompletionKind, Relevance,
}; };
fn check(ra_fixture: &str, expect: Expect) { fn check(ra_fixture: &str, expect: Expect) {
@ -369,24 +347,25 @@ mod tests {
expect.assert_debug_eq(&actual); expect.assert_debug_eq(&actual);
} }
fn check_scores(ra_fixture: &str, expect: Expect) { fn check_relevance(ra_fixture: &str, expect: Expect) {
fn display_score(score: Option<CompletionScore>) -> &'static str { fn display_relevance(relevance: Relevance) -> &'static str {
match score { match relevance {
Some(CompletionScore::TypeMatch) => "[type]", Relevance { exact_type_match: true, exact_name_match: true } => "[type+name]",
Some(CompletionScore::TypeAndNameMatch) => "[type+name]", Relevance { exact_type_match: true, exact_name_match: false } => "[type]",
None => "[]".into(), Relevance { exact_type_match: false, exact_name_match: true } => "[name]",
Relevance { exact_type_match: false, exact_name_match: false } => "[]",
} }
} }
let mut completions = get_all_items(TEST_CONFIG, ra_fixture); let mut completions = get_all_items(TEST_CONFIG, ra_fixture);
completions.sort_by_key(|it| (Reverse(it.score()), it.label().to_string())); completions.sort_by_key(|it| (Reverse(it.relevance()), it.label().to_string()));
let actual = completions let actual = completions
.into_iter() .into_iter()
.filter(|it| it.completion_kind == CompletionKind::Reference) .filter(|it| it.completion_kind == CompletionKind::Reference)
.map(|it| { .map(|it| {
let tag = it.kind().unwrap().tag(); let tag = it.kind().unwrap().tag();
let score = display_score(it.score()); let relevance = display_relevance(it.relevance());
format!("{} {} {}\n", tag, it.label(), score) format!("{} {} {}\n", tag, it.label(), relevance)
}) })
.collect::<String>(); .collect::<String>();
expect.assert_eq(&actual); expect.assert_eq(&actual);
@ -849,9 +828,9 @@ fn foo(xs: Vec<i128>)
} }
#[test] #[test]
fn active_param_score() { fn active_param_relevance() {
cov_mark::check!(active_param_type_match); cov_mark::check!(active_param_type_match);
check_scores( check_relevance(
r#" r#"
struct S { foo: i64, bar: u32, baz: u32 } struct S { foo: i64, bar: u32, baz: u32 }
fn test(bar: u32) { } fn test(bar: u32) { }
@ -866,9 +845,9 @@ fn foo(s: S) { test(s.$0) }
} }
#[test] #[test]
fn record_field_scores() { fn record_field_relevances() {
cov_mark::check!(record_field_type_match); cov_mark::check!(record_field_type_match);
check_scores( check_relevance(
r#" r#"
struct A { foo: i64, bar: u32, baz: u32 } struct A { foo: i64, bar: u32, baz: u32 }
struct B { x: (), y: f32, bar: u32 } struct B { x: (), y: f32, bar: u32 }
@ -883,8 +862,8 @@ fn foo(a: A) { B { bar: a.$0 }; }
} }
#[test] #[test]
fn record_field_and_call_scores() { fn record_field_and_call_relevances() {
check_scores( check_relevance(
r#" r#"
struct A { foo: i64, bar: u32, baz: u32 } struct A { foo: i64, bar: u32, baz: u32 }
struct B { x: (), y: f32, bar: u32 } struct B { x: (), y: f32, bar: u32 }
@ -897,7 +876,7 @@ fn foo(a: A) { B { bar: f(a.$0) }; }
fd baz [] fd baz []
"#]], "#]],
); );
check_scores( check_relevance(
r#" r#"
struct A { foo: i64, bar: u32, baz: u32 } struct A { foo: i64, bar: u32, baz: u32 }
struct B { x: (), y: f32, bar: u32 } struct B { x: (), y: f32, bar: u32 }
@ -914,7 +893,7 @@ fn foo(a: A) { f(B { bar: a.$0 }); }
#[test] #[test]
fn prioritize_exact_ref_match() { fn prioritize_exact_ref_match() {
check_scores( check_relevance(
r#" r#"
struct WorldSnapshot { _f: () }; struct WorldSnapshot { _f: () };
fn go(world: &WorldSnapshot) { go(w$0) } fn go(world: &WorldSnapshot) { go(w$0) }
@ -929,7 +908,7 @@ fn go(world: &WorldSnapshot) { go(w$0) }
#[test] #[test]
fn too_many_arguments() { fn too_many_arguments() {
check_scores( check_relevance(
r#" r#"
struct Foo; struct Foo;
fn f(foo: &Foo) { f(foo, w$0) } fn f(foo: &Foo) { f(foo, w$0) }
@ -997,6 +976,10 @@ fn main() {
Local, Local,
), ),
detail: "S", detail: "S",
relevance: Relevance {
exact_name_match: true,
exact_type_match: false,
},
ref_match: "&mut ", ref_match: "&mut ",
}, },
] ]

View file

@ -213,7 +213,7 @@ pub(crate) fn completion_item(
..Default::default() ..Default::default()
}; };
if item.score().is_some() { if item.relevance().is_relevant() {
lsp_item.preselect = Some(true); lsp_item.preselect = Some(true);
// HACK: sort preselect items first // HACK: sort preselect items first
lsp_item.sort_text = Some(format!(" {}", item.label())); lsp_item.sort_text = Some(format!(" {}", item.label()));
@ -1106,7 +1106,9 @@ mod tests {
[ [
( (
"&arg", "&arg",
None, Some(
" arg",
),
), ),
( (
"arg", "arg",