From 8c9359b072098403eb0b37a1b0822bb7b4f3b8ba Mon Sep 17 00:00:00 2001 From: Lukas Wirth Date: Tue, 9 Aug 2022 17:53:16 +0200 Subject: [PATCH 1/2] Fix pattern field completions not working for unions --- crates/ide-completion/src/completions.rs | 1 - crates/ide-completion/src/completions/expr.rs | 116 ++++++++++-------- .../ide-completion/src/completions/record.rs | 47 ++++--- crates/ide-completion/src/context.rs | 1 + crates/ide-completion/src/tests/record.rs | 91 ++++++++------ 5 files changed, 140 insertions(+), 116 deletions(-) diff --git a/crates/ide-completion/src/completions.rs b/crates/ide-completion/src/completions.rs index 72579e6026a..55c3e28392a 100644 --- a/crates/ide-completion/src/completions.rs +++ b/crates/ide-completion/src/completions.rs @@ -617,7 +617,6 @@ pub(super) fn complete_name_ref( dot::complete_undotted_self(acc, ctx, path_ctx, expr_ctx); item_list::complete_item_list_in_expr(acc, ctx, path_ctx, expr_ctx); - record::complete_record_expr_func_update(acc, ctx, path_ctx, expr_ctx); snippet::complete_expr_snippet(acc, ctx, path_ctx, expr_ctx); } PathKind::Type { location } => { diff --git a/crates/ide-completion/src/completions/expr.rs b/crates/ide-completion/src/completions/expr.rs index 5d0ddaaf2a2..e6c4bdf5206 100644 --- a/crates/ide-completion/src/completions/expr.rs +++ b/crates/ide-completion/src/completions/expr.rs @@ -1,8 +1,10 @@ //! Completion of names from the current scope in expression position. use hir::ScopeDef; +use syntax::ast; use crate::{ + completions::record::add_default_update, context::{ExprCtx, PathCompletionCtx, Qualified}, CompletionContext, Completions, }; @@ -219,60 +221,76 @@ pub(crate) fn complete_expr_path( _ => (), }); - if is_func_update.is_none() { - let mut add_keyword = - |kw, snippet| acc.add_keyword_snippet_expr(ctx, incomplete_let, kw, snippet); + match is_func_update { + Some(record_expr) => { + let ty = ctx.sema.type_of_expr(&ast::Expr::RecordExpr(record_expr.clone())); - if !in_block_expr { - add_keyword("unsafe", "unsafe {\n $0\n}"); + match ty.as_ref().and_then(|t| t.original.as_adt()) { + Some(hir::Adt::Union(_)) => (), + _ => { + cov_mark::hit!(functional_update); + let missing_fields = + ctx.sema.record_literal_missing_fields(record_expr); + add_default_update(acc, ctx, ty, &missing_fields); + } + }; } - add_keyword("match", "match $1 {\n $0\n}"); - add_keyword("while", "while $1 {\n $0\n}"); - add_keyword("while let", "while let $1 = $2 {\n $0\n}"); - add_keyword("loop", "loop {\n $0\n}"); - if in_match_guard { - add_keyword("if", "if $0"); - } else { - add_keyword("if", "if $1 {\n $0\n}"); - } - add_keyword("if let", "if let $1 = $2 {\n $0\n}"); - add_keyword("for", "for $1 in $2 {\n $0\n}"); - add_keyword("true", "true"); - add_keyword("false", "false"); + None => { + let mut add_keyword = |kw, snippet| { + acc.add_keyword_snippet_expr(ctx, incomplete_let, kw, snippet) + }; - if in_condition || in_block_expr { - add_keyword("let", "let"); - } - - if after_if_expr { - add_keyword("else", "else {\n $0\n}"); - add_keyword("else if", "else if $1 {\n $0\n}"); - } - - if wants_mut_token { - add_keyword("mut", "mut "); - } - - if in_loop_body { - if in_block_expr { - add_keyword("continue", "continue;"); - add_keyword("break", "break;"); - } else { - add_keyword("continue", "continue"); - add_keyword("break", "break"); + if !in_block_expr { + add_keyword("unsafe", "unsafe {\n $0\n}"); } - } + add_keyword("match", "match $1 {\n $0\n}"); + add_keyword("while", "while $1 {\n $0\n}"); + add_keyword("while let", "while let $1 = $2 {\n $0\n}"); + add_keyword("loop", "loop {\n $0\n}"); + if in_match_guard { + add_keyword("if", "if $0"); + } else { + add_keyword("if", "if $1 {\n $0\n}"); + } + add_keyword("if let", "if let $1 = $2 {\n $0\n}"); + add_keyword("for", "for $1 in $2 {\n $0\n}"); + add_keyword("true", "true"); + add_keyword("false", "false"); - if let Some(ty) = innermost_ret_ty { - add_keyword( - "return", - match (in_block_expr, ty.is_unit()) { - (true, true) => "return ;", - (true, false) => "return;", - (false, true) => "return $0", - (false, false) => "return", - }, - ); + if in_condition || in_block_expr { + add_keyword("let", "let"); + } + + if after_if_expr { + add_keyword("else", "else {\n $0\n}"); + add_keyword("else if", "else if $1 {\n $0\n}"); + } + + if wants_mut_token { + add_keyword("mut", "mut "); + } + + if in_loop_body { + if in_block_expr { + add_keyword("continue", "continue;"); + add_keyword("break", "break;"); + } else { + add_keyword("continue", "continue"); + add_keyword("break", "break"); + } + } + + if let Some(ty) = innermost_ret_ty { + add_keyword( + "return", + match (in_block_expr, ty.is_unit()) { + (true, true) => "return ;", + (true, false) => "return;", + (false, true) => "return $0", + (false, false) => "return", + }, + ); + } } } } diff --git a/crates/ide-completion/src/completions/record.rs b/crates/ide-completion/src/completions/record.rs index 1c9042390d3..3f85b45a13c 100644 --- a/crates/ide-completion/src/completions/record.rs +++ b/crates/ide-completion/src/completions/record.rs @@ -3,7 +3,7 @@ use syntax::ast::{self, Expr}; use crate::{ - context::{DotAccess, DotAccessKind, ExprCtx, PathCompletionCtx, PatternContext, Qualified}, + context::{DotAccess, DotAccessKind, PatternContext}, CompletionContext, CompletionItem, CompletionItemKind, CompletionRelevance, CompletionRelevancePostfixMatch, Completions, }; @@ -14,7 +14,24 @@ pub(crate) fn complete_record_pattern_fields( pattern_ctx: &PatternContext, ) { if let PatternContext { record_pat: Some(record_pat), .. } = pattern_ctx { - complete_fields(acc, ctx, ctx.sema.record_pattern_missing_fields(record_pat)); + let ty = ctx.sema.type_of_pat(&ast::Pat::RecordPat(record_pat.clone())); + let missing_fields = match ty.as_ref().and_then(|t| t.original.as_adt()) { + Some(hir::Adt::Union(un)) => { + // ctx.sema.record_pat_missing_fields will always return + // an empty Vec on a union literal. This is normally + // reasonable, but here we'd like to present the full list + // of fields if the literal is empty. + let were_fields_specified = + record_pat.record_pat_field_list().and_then(|fl| fl.fields().next()).is_some(); + + match were_fields_specified { + false => un.fields(ctx.db).into_iter().map(|f| (f, f.ty(ctx.db))).collect(), + true => return, + } + } + _ => ctx.sema.record_pattern_missing_fields(record_pat), + }; + complete_fields(acc, ctx, missing_fields); } } @@ -44,6 +61,7 @@ pub(crate) fn complete_record_expr_fields( let missing_fields = ctx.sema.record_literal_missing_fields(record_expr); add_default_update(acc, ctx, ty, &missing_fields); if dot_prefix { + cov_mark::hit!(functional_update_one_dot); let mut item = CompletionItem::new(CompletionItemKind::Snippet, ctx.source_range(), ".."); item.insert_text("."); @@ -56,30 +74,7 @@ pub(crate) fn complete_record_expr_fields( complete_fields(acc, ctx, missing_fields); } -// FIXME: This should probably be part of complete_path_expr -pub(crate) fn complete_record_expr_func_update( - acc: &mut Completions, - ctx: &CompletionContext<'_>, - path_ctx: &PathCompletionCtx, - expr_ctx: &ExprCtx, -) { - if !matches!(path_ctx.qualified, Qualified::No) { - return; - } - if let ExprCtx { is_func_update: Some(record_expr), .. } = expr_ctx { - let ty = ctx.sema.type_of_expr(&Expr::RecordExpr(record_expr.clone())); - - match ty.as_ref().and_then(|t| t.original.as_adt()) { - Some(hir::Adt::Union(_)) => (), - _ => { - let missing_fields = ctx.sema.record_literal_missing_fields(record_expr); - add_default_update(acc, ctx, ty, &missing_fields); - } - }; - } -} - -fn add_default_update( +pub(crate) fn add_default_update( acc: &mut Completions, ctx: &CompletionContext<'_>, ty: Option, diff --git a/crates/ide-completion/src/context.rs b/crates/ide-completion/src/context.rs index e35f79d2b69..759742d3472 100644 --- a/crates/ide-completion/src/context.rs +++ b/crates/ide-completion/src/context.rs @@ -134,6 +134,7 @@ pub(crate) struct ExprCtx { pub(crate) in_condition: bool, pub(crate) incomplete_let: bool, pub(crate) ref_expr_parent: Option, + /// The surrounding RecordExpression we are completing a functional update pub(crate) is_func_update: Option, pub(crate) self_param: Option, pub(crate) innermost_ret_ty: Option, diff --git a/crates/ide-completion/src/tests/record.rs b/crates/ide-completion/src/tests/record.rs index f6accc68e5e..e9ee5161179 100644 --- a/crates/ide-completion/src/tests/record.rs +++ b/crates/ide-completion/src/tests/record.rs @@ -103,47 +103,9 @@ fn foo(f: Struct) { } #[test] -fn functional_update() { - // FIXME: This should filter out all completions that do not have the type `Foo` - check( - r#" -//- minicore:default -struct Foo { foo1: u32, foo2: u32 } -impl Default for Foo { - fn default() -> Self { loop {} } -} +fn in_functional_update() { + cov_mark::check!(functional_update); -fn main() { - let thing = 1; - let foo = Foo { foo1: 0, foo2: 0 }; - let foo2 = Foo { thing, $0 } -} -"#, - expect![[r#" - fd ..Default::default() - fd foo1 u32 - fd foo2 u32 - "#]], - ); - check( - r#" -//- minicore:default -struct Foo { foo1: u32, foo2: u32 } -impl Default for Foo { - fn default() -> Self { loop {} } -} - -fn main() { - let thing = 1; - let foo = Foo { foo1: 0, foo2: 0 }; - let foo2 = Foo { thing, .$0 } -} -"#, - expect![[r#" - fd ..Default::default() - sn .. - "#]], - ); check( r#" //- minicore:default @@ -192,6 +154,55 @@ fn default() (as Default) fn() -> Self ); } +#[test] +fn functional_update_no_dot() { + // FIXME: This should filter out all completions that do not have the type `Foo` + check( + r#" +//- minicore:default +struct Foo { foo1: u32, foo2: u32 } +impl Default for Foo { + fn default() -> Self { loop {} } +} + +fn main() { + let thing = 1; + let foo = Foo { foo1: 0, foo2: 0 }; + let foo2 = Foo { thing, $0 } +} +"#, + expect![[r#" + fd ..Default::default() + fd foo1 u32 + fd foo2 u32 + "#]], + ); +} + +#[test] +fn functional_update_one_dot() { + cov_mark::check!(functional_update_one_dot); + check( + r#" +//- minicore:default +struct Foo { foo1: u32, foo2: u32 } +impl Default for Foo { + fn default() -> Self { loop {} } +} + +fn main() { + let thing = 1; + let foo = Foo { foo1: 0, foo2: 0 }; + let foo2 = Foo { thing, .$0 } +} +"#, + expect![[r#" + fd ..Default::default() + sn .. + "#]], + ); +} + #[test] fn empty_union_literal() { check( From b3ac58dfb80e6dad9fc777f23247fd379424b65e Mon Sep 17 00:00:00 2001 From: Lukas Wirth Date: Tue, 9 Aug 2022 18:08:05 +0200 Subject: [PATCH 2/2] Add some more `cov_mark`s --- crates/ide-completion/src/completions/expr.rs | 4 +++- crates/ide-completion/src/completions/record.rs | 12 ++++++++---- crates/ide-completion/src/tests/record.rs | 1 + 3 files changed, 12 insertions(+), 5 deletions(-) diff --git a/crates/ide-completion/src/completions/expr.rs b/crates/ide-completion/src/completions/expr.rs index e6c4bdf5206..4d66af9e8d5 100644 --- a/crates/ide-completion/src/completions/expr.rs +++ b/crates/ide-completion/src/completions/expr.rs @@ -231,7 +231,9 @@ pub(crate) fn complete_expr_path( cov_mark::hit!(functional_update); let missing_fields = ctx.sema.record_literal_missing_fields(record_expr); - add_default_update(acc, ctx, ty, &missing_fields); + if !missing_fields.is_empty() { + add_default_update(acc, ctx, ty); + } } }; } diff --git a/crates/ide-completion/src/completions/record.rs b/crates/ide-completion/src/completions/record.rs index 3f85b45a13c..bfb98b9f277 100644 --- a/crates/ide-completion/src/completions/record.rs +++ b/crates/ide-completion/src/completions/record.rs @@ -59,7 +59,11 @@ pub(crate) fn complete_record_expr_fields( } _ => { let missing_fields = ctx.sema.record_literal_missing_fields(record_expr); - add_default_update(acc, ctx, ty, &missing_fields); + + if !missing_fields.is_empty() { + cov_mark::hit!(functional_update_field); + add_default_update(acc, ctx, ty); + } if dot_prefix { cov_mark::hit!(functional_update_one_dot); let mut item = @@ -78,14 +82,14 @@ pub(crate) fn add_default_update( acc: &mut Completions, ctx: &CompletionContext<'_>, ty: Option, - missing_fields: &[(hir::Field, hir::Type)], ) { let default_trait = ctx.famous_defs().core_default_Default(); - let impl_default_trait = default_trait + let impls_default_trait = default_trait .zip(ty.as_ref()) .map_or(false, |(default_trait, ty)| ty.original.impls_trait(ctx.db, default_trait, &[])); - if impl_default_trait && !missing_fields.is_empty() { + if impls_default_trait { // FIXME: This should make use of scope_def like completions so we get all the other goodies + // that is we should handle this like actually completing the default function let completion_text = "..Default::default()"; let mut item = CompletionItem::new(SymbolKind::Field, ctx.source_range(), completion_text); let completion_text = diff --git a/crates/ide-completion/src/tests/record.rs b/crates/ide-completion/src/tests/record.rs index e9ee5161179..328faaa060f 100644 --- a/crates/ide-completion/src/tests/record.rs +++ b/crates/ide-completion/src/tests/record.rs @@ -156,6 +156,7 @@ fn default() (as Default) fn() -> Self #[test] fn functional_update_no_dot() { + cov_mark::check!(functional_update_field); // FIXME: This should filter out all completions that do not have the type `Foo` check( r#"