Skip to content

Commit

Permalink
Merge #8014
Browse files Browse the repository at this point in the history 
8014: increase completion relevance for items in local scope r=matklad a=JoshMcguigan

This PR provides a small completion relevance score bonus for items in local scope. The changes here are relatively minimal, since `coc` by default pre-sorts by position in file. But as we move toward fully server side sorting #7935 I think we'll want some relevance score bump for items in local scope. 

### Before

Note `let~` and `syntax` are both ahead of locals. Ultimately we may decide that `let~` is a high relevance completion given my cursor position here, but that should be done with some explicit scoring on the server side, rather than being caused by (I think) `coc` preferring shorter completions. 

![pre-local-score](https://user-images.githubusercontent.com/22216761/111073414-c97ad600-849b-11eb-84e7-fcee130536f0.png)

### After

![post-local-score](https://user-images.githubusercontent.com/22216761/111073422-d0094d80-849b-11eb-92ec-7ae5ec3b190d.png)


Co-authored-by: Josh Mcguigan <joshmcg88@gmail.com>
  • Loading branch information
@bors @JoshMcguigan
bors[bot] and JoshMcguigan committed Mar 14, 2021
2 parents 406e4be + ba924d0 commit af8440b
Show file tree
Hide file tree
Showing 3 changed files with 88 additions and 32 deletions.
37 changes: 35 additions & 2 deletions crates/ide_completion/src/item.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -144,6 +144,21 @@ pub struct CompletionRelevance {
/// }
/// ```
pub exact_type_match: bool,
/// This is set in cases like these:
///
/// ```
/// fn foo(bar: u32) {
/// $0 // `bar` is local
/// }
/// ```
///
/// ```
/// fn foo() {
/// let bar = 0;
/// $0 // `bar` is local
/// }
/// ```
pub is_local: bool,
}

impl CompletionRelevance {
Expand All @@ -163,6 +178,9 @@ impl CompletionRelevance {
score += 1;
}
if self.exact_type_match {
score += 3;
}
if self.is_local {
score += 1;
}

Expand Down Expand Up @@ -551,9 +569,24 @@ mod tests {
vec![CompletionRelevance::default()],
vec![
CompletionRelevance { exact_name_match: true, ..CompletionRelevance::default() },
CompletionRelevance { exact_type_match: true, ..CompletionRelevance::default() },
CompletionRelevance { is_local: true, ..CompletionRelevance::default() },
],
vec![CompletionRelevance { exact_name_match: true, exact_type_match: true }],
vec![CompletionRelevance {
exact_name_match: true,
is_local: true,
..CompletionRelevance::default()
}],
vec![CompletionRelevance { exact_type_match: true, ..CompletionRelevance::default() }],
vec![CompletionRelevance {
exact_name_match: true,
exact_type_match: true,
..CompletionRelevance::default()
}],
vec![CompletionRelevance {
exact_name_match: true,
exact_type_match: true,
is_local: true,
}],
];

check_relevance_score_ordered(expected_relevance_order);
Expand Down
79 changes: 51 additions & 28 deletions crates/ide_completion/src/render.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -157,9 +157,7 @@ impl<'a> Render<'a> {
.set_documentation(field.docs(self.ctx.db()))
.set_deprecated(is_deprecated);

if let Some(relevance) = compute_relevance(&self.ctx, &ty, &name.to_string()) {
item.set_relevance(relevance);
}
item.set_relevance(compute_relevance(&self.ctx, &ty, &name.to_string()));

item.build()
}
Expand Down Expand Up @@ -254,9 +252,9 @@ impl<'a> Render<'a> {
if let ScopeDef::Local(local) = resolution {
let ty = local.ty(self.ctx.db());

if let Some(relevance) = compute_relevance(&self.ctx, &ty, &local_name) {
item.set_relevance(relevance);
}
let mut relevance = compute_relevance(&self.ctx, &ty, &local_name);
relevance.is_local = true;
item.set_relevance(relevance);

if let Some((_expected_name, expected_type)) = self.ctx.expected_name_and_type() {
if ty != expected_type {
Expand Down Expand Up @@ -328,12 +326,15 @@ impl<'a> Render<'a> {
}
}

fn compute_relevance(ctx: &RenderContext, ty: &Type, name: &str) -> Option<CompletionRelevance> {
let (expected_name, expected_type) = ctx.expected_name_and_type()?;
fn compute_relevance(ctx: &RenderContext, ty: &Type, name: &str) -> CompletionRelevance {
let mut res = CompletionRelevance::default();
res.exact_type_match = ty == &expected_type;
res.exact_name_match = name == &expected_name;
Some(res)

if let Some((expected_name, expected_type)) = ctx.expected_name_and_type() {
res.exact_type_match = ty == &expected_type;
res.exact_name_match = name == &expected_name;
}

res
}

fn relevance_type_match(db: &dyn HirDatabase, ty: &Type, expected_type: &Type) -> bool {
Expand All @@ -343,6 +344,7 @@ fn relevance_type_match(db: &dyn HirDatabase, ty: &Type, expected_type: &Type) -
#[cfg(test)]
mod tests {
use expect_test::{expect, Expect};
use itertools::Itertools;

use crate::{
test_utils::{check_edit, do_completion, get_all_items, TEST_CONFIG},
Expand All @@ -355,15 +357,17 @@ mod tests {
}

fn check_relevance(ra_fixture: &str, expect: Expect) {
fn display_relevance(relevance: CompletionRelevance) -> &'static str {
match relevance {
CompletionRelevance { exact_type_match: true, exact_name_match: true } => {
"[type+name]"
}
CompletionRelevance { exact_type_match: true, exact_name_match: false } => "[type]",
CompletionRelevance { exact_type_match: false, exact_name_match: true } => "[name]",
CompletionRelevance { exact_type_match: false, exact_name_match: false } => "[]",
}
fn display_relevance(relevance: CompletionRelevance) -> String {
let relevance_factors = vec![
(relevance.exact_type_match, "type"),
(relevance.exact_name_match, "name"),
(relevance.is_local, "local"),
]
.into_iter()
.filter_map(|(cond, desc)| if cond { Some(desc) } else { None })
.join("+");

format!("[{}]", relevance_factors)
}

let actual = get_all_items(TEST_CONFIG, ra_fixture)
Expand Down Expand Up @@ -918,7 +922,7 @@ struct WorldSnapshot { _f: () };
fn go(world: &WorldSnapshot) { go(w$0) }
"#,
expect![[r#"
lc world [type+name]
lc world [type+name+local]
st WorldSnapshot []
fn go(…) []
"#]],
Expand All @@ -933,7 +937,7 @@ struct Foo;
fn f(foo: &Foo) { f(foo, w$0) }
"#,
expect![[r#"
lc foo []
lc foo [local]
st Foo []
fn f(…) []
"#]],
Expand Down Expand Up @@ -998,6 +1002,7 @@ fn main() {
relevance: CompletionRelevance {
exact_name_match: true,
exact_type_match: false,
is_local: true,
},
ref_match: "&mut ",
},
Expand Down Expand Up @@ -1037,9 +1042,9 @@ fn main() {
}
"#,
expect![[r#"
lc m []
lc t []
lc &t [type]
lc m [local]
lc t [local]
lc &t [type+local]
st T []
st S []
fn main() []
Expand Down Expand Up @@ -1091,9 +1096,9 @@ fn main() {
}
"#,
expect![[r#"
lc m []
lc t []
lc &mut t [type]
lc m [local]
lc t [local]
lc &mut t [type+local]
tt DerefMut []
tt Deref []
fn foo(…) []
Expand All @@ -1103,4 +1108,22 @@ fn main() {
"#]],
)
}

#[test]
fn locals() {
check_relevance(
r#"
fn foo(bar: u32) {
let baz = 0;
f$0
}
"#,
expect![[r#"
lc baz [local]
lc bar [local]
fn foo(…) []
"#]],
);
}
}
4 changes: 2 additions & 2 deletions crates/rust-analyzer/src/to_proto.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1117,13 +1117,13 @@ mod tests {
(
"&arg",
Some(
"fffffffd",
"fffffffa",
),
),
(
"arg",
Some(
"fffffffe",
"fffffffd",
),
),
]
Expand Down

0 comments on commit af8440b

Please sign in to comment.