Skip to content

Commit

Permalink
Auto merge of rust-lang#16136 - saiintbrisson:fix/completion/a-tad-sm…
Browse files Browse the repository at this point in the history
…arter-with-fns, r=Veykril

fix(completion): make the expected type a tad smarter with `Fn`s

This commit changes how the expected type is calculated when
working with Fn pointers, making the parenthesis stop vanishing
when completing the function name.

I've been bugged by the behavior of parenthesis completion for
a long while now. R-a assumes that the `LetStmt` type is the same
as the function type I've just written. Worse is that all parenthesis
vanish, even from functions that have completely different signatures.
It will now verify if the signature is the same.

While working on this, I noticed that record fields behave the same,
so I also made it prioritize the field type instead of the current
expression when possible, but I'm unsure if this is OK, so input is
appreciated.

ImplTraits as return types will still behave weirdly because lowering
is disallowed at the time it resolves the function types.

![image](https://github.com/rust-lang/rust-analyzer/assets/29989290/c06d6c93-5cac-4ebe-a93b-923017a6ae8c)
![image](https://github.com/rust-lang/rust-analyzer/assets/29989290/31594d82-fa4d-446c-a77e-47e9de1a9a67)
![image](https://github.com/rust-lang/rust-analyzer/assets/29989290/cf33856e-a485-411b-91af-11090d78a44e)
  • Loading branch information
bors committed Jan 2, 2024
2 parents 34df296 + 9a36bc3 commit 7659109
Show file tree
Hide file tree
Showing 4 changed files with 27 additions and 20 deletions.
3 changes: 3 additions & 0 deletions crates/hir/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4657,6 +4657,9 @@ impl Callable {
pub fn return_type(&self) -> Type {
self.ty.derived(self.sig.ret().clone())
}
pub fn sig(&self) -> &CallableSig {
&self.sig
}
}

fn closure_source(db: &dyn HirDatabase, closure: ClosureId) -> Option<ast::ClosureExpr> {
Expand Down
23 changes: 12 additions & 11 deletions crates/ide-completion/src/context/analysis.rs
Original file line number Diff line number Diff line change
Expand Up @@ -361,7 +361,12 @@ fn expected_type_and_name(
let ty = it.pat()
.and_then(|pat| sema.type_of_pat(&pat))
.or_else(|| it.initializer().and_then(|it| sema.type_of_expr(&it)))
.map(TypeInfo::original);
.map(TypeInfo::original)
.filter(|ty| {
// don't infer the let type if the expr is a function,
// preventing parenthesis from vanishing
it.ty().is_some() || !ty.is_fn()
});
let name = match it.pat() {
Some(ast::Pat::IdentPat(ident)) => ident.name().map(NameOrNameRef::Name),
Some(_) | None => None,
Expand Down Expand Up @@ -415,20 +420,16 @@ fn expected_type_and_name(
})().unwrap_or((None, None))
},
ast::RecordExprField(it) => {
let field_ty = sema.resolve_record_field(&it).map(|(_, _, ty)| ty);
let field_name = it.field_name().map(NameOrNameRef::NameRef);
if let Some(expr) = it.expr() {
cov_mark::hit!(expected_type_struct_field_with_leading_char);
(
sema.type_of_expr(&expr).map(TypeInfo::original),
it.field_name().map(NameOrNameRef::NameRef),
)
let ty = field_ty
.or_else(|| sema.type_of_expr(&expr).map(TypeInfo::original));
(ty, field_name)
} else {
cov_mark::hit!(expected_type_struct_field_followed_by_comma);
let ty = sema.resolve_record_field(&it)
.map(|(_, _, ty)| ty);
(
ty,
it.field_name().map(NameOrNameRef::NameRef),
)
(field_ty, field_name)
}
},
// match foo { $0 }
Expand Down
6 changes: 3 additions & 3 deletions crates/ide-completion/src/render.rs
Original file line number Diff line number Diff line change
Expand Up @@ -837,11 +837,11 @@ fn main() {
}
"#,
expect![[r#"
fn main []
fn test []
fn main() []
fn test(…) []
md dep []
fn function (use dep::test_mod_a::function) [requires_import]
fn function (use dep::test_mod_b::function) [requires_import]
fn function(…) (use dep::test_mod_b::function) [requires_import]
"#]],
);
}
Expand Down
15 changes: 9 additions & 6 deletions crates/ide-completion/src/render/function.rs
Original file line number Diff line number Diff line change
Expand Up @@ -305,12 +305,15 @@ fn params(
return None;
}

// Don't add parentheses if the expected type is some function reference.
if let Some(ty) = &ctx.expected_type {
// FIXME: check signature matches?
if ty.is_fn() {
cov_mark::hit!(no_call_parens_if_fn_ptr_needed);
return None;
// Don't add parentheses if the expected type is a function reference with the same signature.
if let Some(expected) = ctx.expected_type.as_ref().filter(|e| e.is_fn()) {
if let Some(expected) = expected.as_callable(ctx.db) {
if let Some(completed) = func.ty(ctx.db).as_callable(ctx.db) {
if expected.sig() == completed.sig() {
cov_mark::hit!(no_call_parens_if_fn_ptr_needed);
return None;
}
}
}
}

Expand Down

0 comments on commit 7659109

Please sign in to comment.