aboutsummaryrefslogtreecommitdiff
path: root/crates/ide_completion
diff options
context:
space:
mode:
authorbors[bot] <26634292+bors[bot]@users.noreply.github.com>2021-03-15 12:59:47 +0000
committerGitHub <[email protected]>2021-03-15 12:59:47 +0000
commit0ac7a19d0c35f0c77a9c3a6209d8ff0e2cf1b035 (patch)
tree6e383cf36600b19ab2e69e7b7a62d132de4f8ba2 /crates/ide_completion
parent6139bd764974318814edfd5427e2a2e8220b211b (diff)
parentd91151c3b1af6d4e4c29f5f82df8566b046266e3 (diff)
Merge #8008
8008: Completion context expected type r=matklad a=JoshMcguigan Currently there are two ways completions use to determine the expected type. There is the `expected_type` field on the `CompletionContext`, as well as the `expected_name_and_type` method on the `RenderContext`. These two things returned slightly different results, and their results were only valid if you had pre-checked some (undocumented) invariants. A simple combination of the two approaches doesn't work because they are both too willing to go far up the syntax tree to find something that fits what they are looking for. This PR makes the following changes: 1. Updates the algorithm that sets `expected_type` on `CompletionContext` 2. Adds `expected_name` field to `CompletionContext` 3. Re-writes the `expected_name_and_type` method to simply return the underlying fields from `CompletionContext` (I'd like to save actually removing this method for a follow up PR just to keep the scope of the changes down) 4. Adds unit tests for the `expected_type`/`expected_name` fields All the existing unit tests still pass (unmodified), but this new algorithm certainly has some gaps (although I believe all the `FIXME` introduced in this PR are also flaws in the current code). I wanted to stop here and get some feedback though - is this approach fundamentally sound? Co-authored-by: Josh Mcguigan <[email protected]>
Diffstat (limited to 'crates/ide_completion')
-rw-r--r--crates/ide_completion/src/context.rs297
-rw-r--r--crates/ide_completion/src/render.rs15
2 files changed, 286 insertions, 26 deletions
diff --git a/crates/ide_completion/src/context.rs b/crates/ide_completion/src/context.rs
index 17d9a3adf..89e9bda78 100644
--- a/crates/ide_completion/src/context.rs
+++ b/crates/ide_completion/src/context.rs
@@ -35,6 +35,7 @@ pub(crate) struct CompletionContext<'a> {
35 /// The token before the cursor, in the macro-expanded file. 35 /// The token before the cursor, in the macro-expanded file.
36 pub(super) token: SyntaxToken, 36 pub(super) token: SyntaxToken,
37 pub(super) krate: Option<hir::Crate>, 37 pub(super) krate: Option<hir::Crate>,
38 pub(super) expected_name: Option<String>,
38 pub(super) expected_type: Option<Type>, 39 pub(super) expected_type: Option<Type>,
39 pub(super) name_ref_syntax: Option<ast::NameRef>, 40 pub(super) name_ref_syntax: Option<ast::NameRef>,
40 pub(super) function_syntax: Option<ast::Fn>, 41 pub(super) function_syntax: Option<ast::Fn>,
@@ -135,6 +136,7 @@ impl<'a> CompletionContext<'a> {
135 original_token, 136 original_token,
136 token, 137 token,
137 krate, 138 krate,
139 expected_name: None,
138 expected_type: None, 140 expected_type: None,
139 name_ref_syntax: None, 141 name_ref_syntax: None,
140 function_syntax: None, 142 function_syntax: None,
@@ -290,23 +292,96 @@ impl<'a> CompletionContext<'a> {
290 file_with_fake_ident: SyntaxNode, 292 file_with_fake_ident: SyntaxNode,
291 offset: TextSize, 293 offset: TextSize,
292 ) { 294 ) {
293 // FIXME: this is wrong in at least two cases: 295 let expected = {
294 // * when there's no token `foo($0)` 296 let mut node = self.token.parent();
295 // * when there is a token, but it happens to have type of it's own 297 loop {
296 self.expected_type = self 298 let ret = match_ast! {
297 .token
298 .ancestors()
299 .find_map(|node| {
300 let ty = match_ast! {
301 match node { 299 match node {
302 ast::Pat(it) => self.sema.type_of_pat(&it), 300 ast::LetStmt(it) => {
303 ast::Expr(it) => self.sema.type_of_expr(&it), 301 cov_mark::hit!(expected_type_let_with_leading_char);
304 _ => return None, 302 cov_mark::hit!(expected_type_let_without_leading_char);
303 let ty = it.pat()
304 .and_then(|pat| self.sema.type_of_pat(&pat));
305 let name = if let Some(ast::Pat::IdentPat(ident)) = it.pat() {
306 Some(ident.syntax().text().to_string())
307 } else {
308 None
309 };
310
311 (ty, name)
312 },
313 ast::ArgList(it) => {
314 cov_mark::hit!(expected_type_fn_param_with_leading_char);
315 cov_mark::hit!(expected_type_fn_param_without_leading_char);
316 ActiveParameter::at_token(
317 &self.sema,
318 self.token.clone(),
319 ).map(|ap| (Some(ap.ty), Some(ap.name)))
320 .unwrap_or((None, None))
321 },
322 ast::RecordExprFieldList(it) => {
323 cov_mark::hit!(expected_type_struct_field_without_leading_char);
324 self.token.prev_sibling_or_token()
325 .and_then(|se| se.into_node())
326 .and_then(|node| ast::RecordExprField::cast(node))
327 .and_then(|rf| self.sema.resolve_record_field(&rf))
328 .map(|f|(
329 Some(f.0.signature_ty(self.db)),
330 Some(f.0.name(self.db).to_string()),
331 ))
332 .unwrap_or((None, None))
333 },
334 ast::RecordExprField(it) => {
335 cov_mark::hit!(expected_type_struct_field_with_leading_char);
336 self.sema
337 .resolve_record_field(&it)
338 .map(|f|(
339 Some(f.0.signature_ty(self.db)),
340 Some(f.0.name(self.db).to_string()),
341 ))
342 .unwrap_or((None, None))
343 },
344 ast::MatchExpr(it) => {
345 cov_mark::hit!(expected_type_match_arm_without_leading_char);
346 let ty = it.expr()
347 .and_then(|e| self.sema.type_of_expr(&e));
348
349 (ty, None)
350 },
351 ast::IdentPat(it) => {
352 cov_mark::hit!(expected_type_if_let_with_leading_char);
353 cov_mark::hit!(expected_type_if_let_without_leading_char);
354 cov_mark::hit!(expected_type_match_arm_with_leading_char);
355 let ty = self.sema.type_of_pat(&ast::Pat::from(it));
356
357 (ty, None)
358 },
359 ast::Fn(it) => {
360 cov_mark::hit!(expected_type_fn_ret_with_leading_char);
361 cov_mark::hit!(expected_type_fn_ret_without_leading_char);
362 let ty = self.token.ancestors()
363 .find_map(|ancestor| ast::Expr::cast(ancestor))
364 .and_then(|expr| self.sema.type_of_expr(&expr));
365
366 (ty, None)
367 },
368 _ => {
369 match node.parent() {
370 Some(n) => {
371 node = n;
372 continue;
373 },
374 None => (None, None),
375 }
376 },
305 } 377 }
306 }; 378 };
307 Some(ty) 379
308 }) 380 break ret;
309 .flatten(); 381 }
382 };
383 self.expected_type = expected.0;
384 self.expected_name = expected.1;
310 self.attribute_under_caret = find_node_at_offset(&file_with_fake_ident, offset); 385 self.attribute_under_caret = find_node_at_offset(&file_with_fake_ident, offset);
311 386
312 // First, let's try to complete a reference to some declaration. 387 // First, let's try to complete a reference to some declaration.
@@ -535,3 +610,197 @@ fn path_or_use_tree_qualifier(path: &ast::Path) -> Option<ast::Path> {
535 let use_tree = use_tree_list.syntax().parent().and_then(ast::UseTree::cast)?; 610 let use_tree = use_tree_list.syntax().parent().and_then(ast::UseTree::cast)?;
536 use_tree.path() 611 use_tree.path()
537} 612}
613
614#[cfg(test)]
615mod tests {
616 use expect_test::{expect, Expect};
617 use hir::HirDisplay;
618
619 use crate::test_utils::{position, TEST_CONFIG};
620
621 use super::CompletionContext;
622
623 fn check_expected_type_and_name(ra_fixture: &str, expect: Expect) {
624 let (db, pos) = position(ra_fixture);
625 let completion_context = CompletionContext::new(&db, pos, &TEST_CONFIG).unwrap();
626
627 let ty = completion_context
628 .expected_type
629 .map(|t| t.display_test(&db).to_string())
630 .unwrap_or("?".to_owned());
631
632 let name = completion_context.expected_name.unwrap_or("?".to_owned());
633
634 expect.assert_eq(&format!("ty: {}, name: {}", ty, name));
635 }
636
637 #[test]
638 fn expected_type_let_without_leading_char() {
639 cov_mark::check!(expected_type_let_without_leading_char);
640 check_expected_type_and_name(
641 r#"
642fn foo() {
643 let x: u32 = $0;
644}
645"#,
646 expect![[r#"ty: u32, name: x"#]],
647 );
648 }
649
650 #[test]
651 fn expected_type_let_with_leading_char() {
652 cov_mark::check!(expected_type_let_with_leading_char);
653 check_expected_type_and_name(
654 r#"
655fn foo() {
656 let x: u32 = c$0;
657}
658"#,
659 expect![[r#"ty: u32, name: x"#]],
660 );
661 }
662
663 #[test]
664 fn expected_type_fn_param_without_leading_char() {
665 cov_mark::check!(expected_type_fn_param_without_leading_char);
666 check_expected_type_and_name(
667 r#"
668fn foo() {
669 bar($0);
670}
671
672fn bar(x: u32) {}
673"#,
674 expect![[r#"ty: u32, name: x"#]],
675 );
676 }
677
678 #[test]
679 fn expected_type_fn_param_with_leading_char() {
680 cov_mark::check!(expected_type_fn_param_with_leading_char);
681 check_expected_type_and_name(
682 r#"
683fn foo() {
684 bar(c$0);
685}
686
687fn bar(x: u32) {}
688"#,
689 expect![[r#"ty: u32, name: x"#]],
690 );
691 }
692
693 #[test]
694 fn expected_type_struct_field_without_leading_char() {
695 cov_mark::check!(expected_type_struct_field_without_leading_char);
696 check_expected_type_and_name(
697 r#"
698struct Foo { a: u32 }
699fn foo() {
700 Foo { a: $0 };
701}
702"#,
703 expect![[r#"ty: u32, name: a"#]],
704 )
705 }
706
707 #[test]
708 fn expected_type_struct_field_with_leading_char() {
709 cov_mark::check!(expected_type_struct_field_with_leading_char);
710 check_expected_type_and_name(
711 r#"
712struct Foo { a: u32 }
713fn foo() {
714 Foo { a: c$0 };
715}
716"#,
717 expect![[r#"ty: u32, name: a"#]],
718 );
719 }
720
721 #[test]
722 fn expected_type_match_arm_without_leading_char() {
723 cov_mark::check!(expected_type_match_arm_without_leading_char);
724 check_expected_type_and_name(
725 r#"
726enum E { X }
727fn foo() {
728 match E::X { $0 }
729}
730"#,
731 expect![[r#"ty: E, name: ?"#]],
732 );
733 }
734
735 #[test]
736 fn expected_type_match_arm_with_leading_char() {
737 cov_mark::check!(expected_type_match_arm_with_leading_char);
738 check_expected_type_and_name(
739 r#"
740enum E { X }
741fn foo() {
742 match E::X { c$0 }
743}
744"#,
745 expect![[r#"ty: E, name: ?"#]],
746 );
747 }
748
749 #[test]
750 fn expected_type_if_let_without_leading_char() {
751 cov_mark::check!(expected_type_if_let_without_leading_char);
752 check_expected_type_and_name(
753 r#"
754enum Foo { Bar, Baz, Quux }
755
756fn foo() {
757 let f = Foo::Quux;
758 if let $0 = f { }
759}
760"#,
761 expect![[r#"ty: (), name: ?"#]],
762 ) // FIXME should be `ty: u32, name: ?`
763 }
764
765 #[test]
766 fn expected_type_if_let_with_leading_char() {
767 cov_mark::check!(expected_type_if_let_with_leading_char);
768 check_expected_type_and_name(
769 r#"
770enum Foo { Bar, Baz, Quux }
771
772fn foo() {
773 let f = Foo::Quux;
774 if let c$0 = f { }
775}
776"#,
777 expect![[r#"ty: Foo, name: ?"#]],
778 )
779 }
780
781 #[test]
782 fn expected_type_fn_ret_without_leading_char() {
783 cov_mark::check!(expected_type_fn_ret_without_leading_char);
784 check_expected_type_and_name(
785 r#"
786fn foo() -> u32 {
787 $0
788}
789"#,
790 expect![[r#"ty: (), name: ?"#]],
791 ) // FIXME this should be `ty: u32, name: ?`
792 }
793
794 #[test]
795 fn expected_type_fn_ret_with_leading_char() {
796 cov_mark::check!(expected_type_fn_ret_with_leading_char);
797 check_expected_type_and_name(
798 r#"
799fn foo() -> u32 {
800 c$0
801}
802"#,
803 expect![[r#"ty: u32, name: ?"#]],
804 )
805 }
806}
diff --git a/crates/ide_completion/src/render.rs b/crates/ide_completion/src/render.rs
index 905f0b197..3e1bff4d6 100644
--- a/crates/ide_completion/src/render.rs
+++ b/crates/ide_completion/src/render.rs
@@ -119,17 +119,10 @@ impl<'a> RenderContext<'a> {
119 node.docs(self.db()) 119 node.docs(self.db())
120 } 120 }
121 121
122 // FIXME delete this method in favor of directly using the fields
123 // on CompletionContext
122 fn expected_name_and_type(&self) -> Option<(String, Type)> { 124 fn expected_name_and_type(&self) -> Option<(String, Type)> {
123 if let Some(record_field) = &self.completion.record_field_syntax { 125 Some((self.completion.expected_name.clone()?, self.completion.expected_type.clone()?))
124 cov_mark::hit!(record_field_type_match);
125 let (struct_field, _local) = self.completion.sema.resolve_record_field(record_field)?;
126 Some((struct_field.name(self.db()).to_string(), struct_field.signature_ty(self.db())))
127 } else if let Some(active_parameter) = &self.completion.active_parameter {
128 cov_mark::hit!(active_param_type_match);
129 Some((active_parameter.name.clone(), active_parameter.ty.clone()))
130 } else {
131 None
132 }
133 } 126 }
134} 127}
135 128
@@ -852,7 +845,6 @@ fn foo(xs: Vec<i128>)
852 845
853 #[test] 846 #[test]
854 fn active_param_relevance() { 847 fn active_param_relevance() {
855 cov_mark::check!(active_param_type_match);
856 check_relevance( 848 check_relevance(
857 r#" 849 r#"
858struct S { foo: i64, bar: u32, baz: u32 } 850struct S { foo: i64, bar: u32, baz: u32 }
@@ -869,7 +861,6 @@ fn foo(s: S) { test(s.$0) }
869 861
870 #[test] 862 #[test]
871 fn record_field_relevances() { 863 fn record_field_relevances() {
872 cov_mark::check!(record_field_type_match);
873 check_relevance( 864 check_relevance(
874 r#" 865 r#"
875struct A { foo: i64, bar: u32, baz: u32 } 866struct A { foo: i64, bar: u32, baz: u32 }