mikros/rust
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

Merge #8056

Browse Source 
8056: completion relevance consider if types can be unified r=JoshMcguigan a=JoshMcguigan

This PR improves completion relevance scoring for generic types, in cases where the types could unify. 

### Before

![pre-could-unify](https://user-images.githubusercontent.com/22216761/111338556-46d94e80-8634-11eb-9936-2b20eb9e6756.png)

### After

![post-could-unify](https://user-images.githubusercontent.com/22216761/111338598-4e005c80-8634-11eb-92e0-69c2c1cda6fc.png)

changelog feature improve completions

Co-authored-by: Josh Mcguigan <joshmcg88@gmail.com>
This commit is contained in:
bors[bot] 2021-03-26 16:29:20 +00:00 committed by GitHub
commit 4ecaad98e0
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
9 changed files with 130 additions and 49 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

6
crates/hir/src/lib.rs
View File

@ -51,7 +51,7 @@
};
use hir_expand::{diagnostics::DiagnosticSink, name::name, MacroDefKind};
use hir_ty::{
autoderef,
autoderef, could_unify,
method_resolution::{self, TyFingerprint},
primitive::UintTy,
to_assoc_type_id,
@ -2154,6 +2154,10 @@ fn walk_type(db: &dyn HirDatabase, type_: &Type, cb: &mut impl FnMut(Type)) {
walk_type(db, self, &mut cb);
}
pub fn could_unify_with(&self, other: &Type) -> bool {
could_unify(&self.ty, &other.ty)
}
}
// FIXME: closures

5
crates/hir_ty/src/infer.rs
View File

@ -45,6 +45,11 @@
to_assoc_type_id, to_chalk_trait_id, AliasEq, AliasTy, Interner, TyKind,
};
// This lint has a false positive here. See the link below for details.
//
// https://github.com/rust-lang/rust/issues/57411
#[allow(unreachable_pub)]
pub use unify::could_unify;
pub(crate) use unify::unify;
mod unify;

4
crates/hir_ty/src/infer/unify.rs
View File

@ -157,6 +157,10 @@ pub(super) fn apply_solution(
}
}
pub fn could_unify(t1: &Ty, t2: &Ty) -> bool {
InferenceTable::new().unify(t1, t2)
}
pub(crate) fn unify(tys: &Canonical<(Ty, Ty)>) -> Option<Substitution> {
let mut table = InferenceTable::new();
let vars = Substitution(

2
crates/hir_ty/src/lib.rs
View File

@ -41,7 +41,7 @@ macro_rules! eprintln {
};
pub use autoderef::autoderef;
pub use infer::{InferenceResult, InferenceVar};
pub use infer::{could_unify, InferenceResult, InferenceVar};
pub use lower::{
associated_type_shorthand_candidates, callable_item_sig, CallableDefId, ImplTraitLoweringMode,
TyDefId, TyLoweringContext, ValueTyDefId,

71
crates/ide_completion/src/item.rs
View File

@ -122,7 +122,7 @@ fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
}
}
#[derive(Debug, Clone, Copy, Ord, PartialOrd, Eq, PartialEq, Default)]
#[derive(Debug, Clone, Copy, Eq, PartialEq, Default)]
pub struct CompletionRelevance {
/// This is set in cases like these:
///
@ -134,6 +134,31 @@ pub struct CompletionRelevance {
/// }
/// ```
pub exact_name_match: bool,
/// See CompletionRelevanceTypeMatch doc comments for cases where this is set.
pub type_match: Option<CompletionRelevanceTypeMatch>,
/// This is set in cases like these:
///
/// ```
/// fn foo(a: u32) {
/// let b = 0;
/// $0 // `a` and `b` are local
/// }
/// ```
pub is_local: bool,
}
#[derive(Debug, Clone, Copy, Eq, PartialEq)]
pub enum CompletionRelevanceTypeMatch {
/// This is set in cases like these:
///
/// ```
/// enum Option<T> { Some(T), None }
/// fn f(a: Option<u32>) {}
/// fn main {
/// f(Option::N$0) // type `Option<T>` could unify with `Option<u32>`
/// }
/// ```
CouldUnify,
/// This is set in cases like these:
///
/// ```
@ -143,22 +168,7 @@ pub struct CompletionRelevance {
/// f($0) // type of local matches the type of param
/// }
/// ```
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,
Exact,
}
impl CompletionRelevance {
@ -177,9 +187,11 @@ pub fn score(&self) -> u32 {
if self.exact_name_match {
score += 1;
}
if self.exact_type_match {
score += 3;
}
score += match self.type_match {
Some(CompletionRelevanceTypeMatch::Exact) => 4,
Some(CompletionRelevanceTypeMatch::CouldUnify) => 3,
None => 0,
};
if self.is_local {
score += 1;
}
@ -342,7 +354,7 @@ pub fn ref_match(&self) -> Option<(Mutability, CompletionRelevance)> {
// match, but with exact type match set because self.ref_match
// is only set if there is an exact type match.
let mut relevance = self.relevance;
relevance.exact_type_match = true;
relevance.type_match = Some(CompletionRelevanceTypeMatch::Exact);
self.ref_match.map(|mutability| (mutability, relevance))
}
@ -523,7 +535,7 @@ mod tests {
use itertools::Itertools;
use test_utils::assert_eq_text;
use super::CompletionRelevance;
use super::{CompletionRelevance, CompletionRelevanceTypeMatch};
/// Check that these are CompletionRelevance are sorted in ascending order
/// by their relevance score.
@ -576,15 +588,22 @@ fn relevance_score() {
is_local: true,
..CompletionRelevance::default()
}],
vec![CompletionRelevance { exact_type_match: true, ..CompletionRelevance::default() }],
vec![CompletionRelevance {
exact_name_match: true,
exact_type_match: true,
type_match: Some(CompletionRelevanceTypeMatch::CouldUnify),
..CompletionRelevance::default()
}],
vec![CompletionRelevance {
type_match: Some(CompletionRelevanceTypeMatch::Exact),
..CompletionRelevance::default()
}],
vec![CompletionRelevance {
exact_name_match: true,
exact_type_match: true,
type_match: Some(CompletionRelevanceTypeMatch::Exact),
..CompletionRelevance::default()
}],
vec![CompletionRelevance {
exact_name_match: true,
type_match: Some(CompletionRelevanceTypeMatch::Exact),
is_local: true,
}],
];

81
crates/ide_completion/src/render.rs
View File

@ -20,8 +20,8 @@
use syntax::TextRange;
use crate::{
item::ImportEdit, CompletionContext, CompletionItem, CompletionItemKind, CompletionKind,
CompletionRelevance,
item::{CompletionRelevanceTypeMatch, ImportEdit},
CompletionContext, CompletionItem, CompletionItemKind, CompletionKind, CompletionRelevance,
};
use crate::render::{enum_variant::render_variant, function::render_fn, macro_::render_macro};
@ -143,7 +143,7 @@ fn add_field(&mut self, field: hir::Field, ty: &Type) -> CompletionItem {
.set_deprecated(is_deprecated);
item.set_relevance(CompletionRelevance {
exact_type_match: compute_exact_type_match(self.ctx.completion, ty),
type_match: compute_type_match(self.ctx.completion, ty),
exact_name_match: compute_exact_name_match(self.ctx.completion, name.to_string()),
..CompletionRelevance::default()
});
@ -245,7 +245,7 @@ fn render_resolution(
}
item.set_relevance(CompletionRelevance {
exact_type_match: compute_exact_type_match(self.ctx.completion, &ty),
type_match: compute_type_match(self.ctx.completion, &ty),
exact_name_match: compute_exact_name_match(self.ctx.completion, &local_name),
is_local: true,
..CompletionRelevance::default()
@ -309,14 +309,24 @@ fn is_deprecated(&self, resolution: &ScopeDef) -> bool {
}
}
fn compute_exact_type_match(ctx: &CompletionContext, completion_ty: &hir::Type) -> bool {
match ctx.expected_type.as_ref() {
Some(expected_type) => {
// We don't ever consider unit type to be an exact type match, since
// nearly always this is not meaningful to the user.
completion_ty == expected_type && !expected_type.is_unit()
}
None => false,
fn compute_type_match(
ctx: &CompletionContext,
completion_ty: &hir::Type,
) -> Option<CompletionRelevanceTypeMatch> {
let expected_type = ctx.expected_type.as_ref()?;
// We don't ever consider unit type to be an exact type match, since
// nearly always this is not meaningful to the user.
if expected_type.is_unit() {
return None;
}
if completion_ty == expected_type {
Some(CompletionRelevanceTypeMatch::Exact)
} else if expected_type.could_unify_with(completion_ty) {
Some(CompletionRelevanceTypeMatch::CouldUnify)
} else {
None
}
}
@ -348,6 +358,7 @@ mod tests {
use itertools::Itertools;
use crate::{
item::CompletionRelevanceTypeMatch,
test_utils::{check_edit, do_completion, get_all_items, TEST_CONFIG},
CompletionKind, CompletionRelevance,
};
@ -360,7 +371,11 @@ fn check(ra_fixture: &str, expect: Expect) {
fn check_relevance(ra_fixture: &str, expect: Expect) {
fn display_relevance(relevance: CompletionRelevance) -> String {
let relevance_factors = vec![
(relevance.exact_type_match, "type"),
(relevance.type_match == Some(CompletionRelevanceTypeMatch::Exact), "type"),
(
relevance.type_match == Some(CompletionRelevanceTypeMatch::CouldUnify),
"type_could_unify",
),
(relevance.exact_name_match, "name"),
(relevance.is_local, "local"),
]
@ -533,7 +548,9 @@ fn main() { let _: m::Spam = S$0 }
detail: "(i32)",
relevance: CompletionRelevance {
exact_name_match: false,
exact_type_match: true,
type_match: Some(
Exact,
),
is_local: false,
},
trigger_call_info: true,
@ -559,7 +576,9 @@ fn main() { let _: m::Spam = S$0 }
detail: "()",
relevance: CompletionRelevance {
exact_name_match: false,
exact_type_match: true,
type_match: Some(
Exact,
),
is_local: false,
},
},
@ -1108,7 +1127,7 @@ fn main() {
detail: "S",
relevance: CompletionRelevance {
exact_name_match: true,
exact_type_match: false,
type_match: None,
is_local: true,
},
ref_match: "&mut ",
@ -1353,4 +1372,34 @@ fn foo(f: Foo) { let _: &u32 = f.b$0 }
"#]],
);
}
#[test]
fn generic_enum() {
check_relevance(
r#"
enum Foo<T> { A(T), B }
// bar() should not be an exact type match
// because the generic parameters are different
fn bar() -> Foo<u8> { Foo::B }
// FIXME baz() should be an exact type match
// because the types could unify, but it currently
// is not. This is due to the T here being
// TyKind::Placeholder rather than TyKind::Missing.
fn baz<T>() -> Foo<T> { Foo::B }
fn foo() {
let foo: Foo<u32> = Foo::B;
let _: Foo<u32> = f$0;
}
"#,
expect![[r#"
ev Foo::A(…) [type_could_unify]
ev Foo::B [type_could_unify]
lc foo [type+local]
en Foo []
fn baz() []
fn bar() []
fn foo() []
"#]],
);
}
}

4
crates/ide_completion/src/render/enum_variant.rs
View File

@ -6,7 +6,7 @@
use crate::{
item::{CompletionItem, CompletionKind, ImportEdit},
render::{builder_ext::Params, compute_exact_type_match, compute_ref_match, RenderContext},
render::{builder_ext::Params, compute_ref_match, compute_type_match, RenderContext},
CompletionRelevance,
};
@ -77,7 +77,7 @@ fn render(self, import_to_add: Option<ImportEdit>) -> CompletionItem {
let ty = self.variant.parent_enum(self.ctx.completion.db).ty(self.ctx.completion.db);
item.set_relevance(CompletionRelevance {
exact_type_match: compute_exact_type_match(self.ctx.completion, &ty),
type_match: compute_type_match(self.ctx.completion, &ty),
..CompletionRelevance::default()
});

4
crates/ide_completion/src/render/function.rs
View File

@ -8,7 +8,7 @@
use crate::{
item::{CompletionItem, CompletionItemKind, CompletionKind, CompletionRelevance, ImportEdit},
render::{
builder_ext::Params, compute_exact_name_match, compute_exact_type_match, compute_ref_match,
builder_ext::Params, compute_exact_name_match, compute_ref_match, compute_type_match,
RenderContext,
},
};
@ -73,7 +73,7 @@ fn render(self, import_to_add: Option<ImportEdit>) -> CompletionItem {
let ret_type = self.func.ret_type(self.ctx.db());
item.set_relevance(CompletionRelevance {
exact_type_match: compute_exact_type_match(self.ctx.completion, &ret_type),
type_match: compute_type_match(self.ctx.completion, &ret_type),
exact_name_match: compute_exact_name_match(self.ctx.completion, self.name.clone()),
..CompletionRelevance::default()
});

2
crates/rust-analyzer/src/to_proto.rs
View File

@ -1138,7 +1138,7 @@ fn main() {
(
"&arg",
Some(
"fffffffa",
"fffffff9",
),
),
(