From a6b92a8cc00c4a4c451e6da2dd4e2a2e8e7bf749 Mon Sep 17 00:00:00 2001 From: Lukas Wirth Date: Fri, 28 May 2021 20:46:09 +0200 Subject: simplify --- crates/base_db/src/fixture.rs | 10 +- crates/ide/src/fixture.rs | 17 +- crates/ide_completion/src/completions/keyword.rs | 192 +++++++++++------------ crates/ide_completion/src/context.rs | 9 +- crates/ide_completion/src/patterns.rs | 12 +- crates/ide_completion/src/test_utils.rs | 16 +- crates/ide_db/src/call_info/tests.rs | 6 +- crates/ide_db/src/traits/tests.rs | 6 +- crates/test_utils/src/lib.rs | 15 ++ 9 files changed, 140 insertions(+), 143 deletions(-) diff --git a/crates/base_db/src/fixture.rs b/crates/base_db/src/fixture.rs index 0132565e4..69ceba735 100644 --- a/crates/base_db/src/fixture.rs +++ b/crates/base_db/src/fixture.rs @@ -34,19 +34,13 @@ pub trait WithFixture: Default + SourceDatabaseExt + 'static { fn with_position(ra_fixture: &str) -> (Self, FilePosition) { let (db, file_id, range_or_offset) = Self::with_range_or_offset(ra_fixture); - let offset = match range_or_offset { - RangeOrOffset::Range(_) => panic!("Expected a cursor position, got a range instead"), - RangeOrOffset::Offset(it) => it, - }; + let offset = range_or_offset.expect_offset(); (db, FilePosition { file_id, offset }) } fn with_range(ra_fixture: &str) -> (Self, FileRange) { let (db, file_id, range_or_offset) = Self::with_range_or_offset(ra_fixture); - let range = match range_or_offset { - RangeOrOffset::Range(it) => it, - RangeOrOffset::Offset(_) => panic!("Expected a cursor range, got a position instead"), - }; + let range = range_or_offset.expect_range(); (db, FileRange { file_id, range }) } diff --git a/crates/ide/src/fixture.rs b/crates/ide/src/fixture.rs index cc6641ba1..6780af617 100644 --- a/crates/ide/src/fixture.rs +++ b/crates/ide/src/fixture.rs @@ -1,7 +1,7 @@ //! Utilities for creating `Analysis` instances for tests. use ide_db::base_db::fixture::ChangeFixture; use syntax::{TextRange, TextSize}; -use test_utils::{extract_annotations, RangeOrOffset}; +use test_utils::extract_annotations; use crate::{Analysis, AnalysisHost, FileId, FilePosition, FileRange}; @@ -27,10 +27,7 @@ pub(crate) fn position(ra_fixture: &str) -> (Analysis, FilePosition) { let change_fixture = ChangeFixture::parse(ra_fixture); host.db.apply_change(change_fixture.change); let (file_id, range_or_offset) = change_fixture.file_position.expect("expected a marker ($0)"); - let offset = match range_or_offset { - RangeOrOffset::Range(_) => panic!(), - RangeOrOffset::Offset(it) => it, - }; + let offset = range_or_offset.expect_offset(); (host.analysis(), FilePosition { file_id, offset }) } @@ -40,10 +37,7 @@ pub(crate) fn range(ra_fixture: &str) -> (Analysis, FileRange) { let change_fixture = ChangeFixture::parse(ra_fixture); host.db.apply_change(change_fixture.change); let (file_id, range_or_offset) = change_fixture.file_position.expect("expected a marker ($0)"); - let range = match range_or_offset { - RangeOrOffset::Range(it) => it, - RangeOrOffset::Offset(_) => panic!(), - }; + let range = range_or_offset.expect_range(); (host.analysis(), FileRange { file_id, range }) } @@ -53,10 +47,7 @@ pub(crate) fn annotations(ra_fixture: &str) -> (Analysis, FilePosition, Vec<(Fil let change_fixture = ChangeFixture::parse(ra_fixture); host.db.apply_change(change_fixture.change); let (file_id, range_or_offset) = change_fixture.file_position.expect("expected a marker ($0)"); - let offset = match range_or_offset { - RangeOrOffset::Range(_) => panic!(), - RangeOrOffset::Offset(it) => it, - }; + let offset = range_or_offset.expect_offset(); let annotations = change_fixture .files diff --git a/crates/ide_completion/src/completions/keyword.rs b/crates/ide_completion/src/completions/keyword.rs index c9673df85..662c389fe 100644 --- a/crates/ide_completion/src/completions/keyword.rs +++ b/crates/ide_completion/src/completions/keyword.rs @@ -39,6 +39,8 @@ pub(crate) fn complete_use_tree_keyword(acc: &mut Completions, ctx: &CompletionC } } +trait Foo {} + pub(crate) fn complete_expr_keyword(acc: &mut Completions, ctx: &CompletionContext) { if ctx.token.kind() == SyntaxKind::COMMENT { cov_mark::hit!(no_keyword_completion_in_comments); @@ -48,91 +50,92 @@ pub(crate) fn complete_expr_keyword(acc: &mut Completions, ctx: &CompletionConte cov_mark::hit!(no_keyword_completion_in_record_lit); return; } + let mut add_keyword = |kw, snippet| add_keyword(ctx, acc, kw, snippet); let expects_assoc_item = ctx.expects_assoc_item(); let has_block_expr_parent = ctx.has_block_expr_parent(); let expects_item = ctx.expects_item(); + if ctx.has_impl_or_trait_prev_sibling() { - add_keyword(ctx, acc, "where", "where "); + // FIXME this also incorrectly shows up after a complete trait/impl + add_keyword("where", "where "); return; } if ctx.previous_token_is(T![unsafe]) { - if expects_item || has_block_expr_parent { - add_keyword(ctx, acc, "fn", "fn $1($2) {\n $0\n}") + if expects_item || expects_assoc_item || has_block_expr_parent { + add_keyword("fn", "fn $1($2) {\n $0\n}") } if expects_item || has_block_expr_parent { - add_keyword(ctx, acc, "trait", "trait $1 {\n $0\n}"); - add_keyword(ctx, acc, "impl", "impl $1 {\n $0\n}"); + add_keyword("trait", "trait $1 {\n $0\n}"); + add_keyword("impl", "impl $1 {\n $0\n}"); } return; } + + if expects_item || ctx.expects_non_trait_assoc_item() || ctx.expect_record_field() { + add_keyword("pub(crate)", "pub(crate) "); + add_keyword("pub", "pub "); + } + + if expects_item || expects_assoc_item || has_block_expr_parent || ctx.is_match_arm { + add_keyword("unsafe", "unsafe "); + } + if expects_item || expects_assoc_item || has_block_expr_parent { - add_keyword(ctx, acc, "fn", "fn $1($2) {\n $0\n}"); + add_keyword("fn", "fn $1($2) {\n $0\n}"); + add_keyword("const", "const $0"); + add_keyword("type", "type $0"); } + if expects_item || has_block_expr_parent { - add_keyword(ctx, acc, "use", "use "); - add_keyword(ctx, acc, "impl", "impl $1 {\n $0\n}"); - add_keyword(ctx, acc, "trait", "trait $1 {\n $0\n}"); + add_keyword("use", "use $0"); + add_keyword("impl", "impl $1 {\n $0\n}"); + add_keyword("trait", "trait $1 {\n $0\n}"); + add_keyword("static", "static $0"); + add_keyword("extern", "extern $0"); + add_keyword("mod", "mod $0"); } if expects_item { - add_keyword(ctx, acc, "enum", "enum $1 {\n $0\n}"); - add_keyword(ctx, acc, "struct", "struct $0"); - add_keyword(ctx, acc, "union", "union $1 {\n $0\n}"); + add_keyword("enum", "enum $1 {\n $0\n}"); + add_keyword("struct", "struct $0"); + add_keyword("union", "union $1 {\n $0\n}"); } - if ctx.is_expr { - add_keyword(ctx, acc, "match", "match $1 {\n $0\n}"); - add_keyword(ctx, acc, "while", "while $1 {\n $0\n}"); - add_keyword(ctx, acc, "while let", "while let $1 = $2 {\n $0\n}"); - add_keyword(ctx, acc, "loop", "loop {\n $0\n}"); - add_keyword(ctx, acc, "if", "if $1 {\n $0\n}"); - add_keyword(ctx, acc, "if let", "if let $1 = $2 {\n $0\n}"); - add_keyword(ctx, acc, "for", "for $1 in $2 {\n $0\n}"); + if ctx.expects_expression() { + 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}"); + 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}"); } if ctx.previous_token_is(T![if]) || ctx.previous_token_is(T![while]) || has_block_expr_parent { - add_keyword(ctx, acc, "let", "let "); + add_keyword("let", "let "); } if ctx.after_if { - add_keyword(ctx, acc, "else", "else {\n $0\n}"); - add_keyword(ctx, acc, "else if", "else if $1 {\n $0\n}"); - } - if expects_item || has_block_expr_parent { - add_keyword(ctx, acc, "mod", "mod $0"); + add_keyword("else", "else {\n $0\n}"); + add_keyword("else if", "else if $1 {\n $0\n}"); } + if ctx.expects_ident_pat_or_ref_expr() { - add_keyword(ctx, acc, "mut", "mut "); - } - if expects_item || expects_assoc_item || has_block_expr_parent { - add_keyword(ctx, acc, "const", "const "); - add_keyword(ctx, acc, "type", "type "); - } - if expects_item || has_block_expr_parent { - add_keyword(ctx, acc, "static", "static "); - }; - if expects_item || has_block_expr_parent { - add_keyword(ctx, acc, "extern", "extern "); - } - if expects_item || expects_assoc_item || has_block_expr_parent || ctx.is_match_arm { - add_keyword(ctx, acc, "unsafe", "unsafe "); + add_keyword("mut", "mut "); } + if ctx.in_loop_body { if ctx.can_be_stmt { - add_keyword(ctx, acc, "continue", "continue;"); - add_keyword(ctx, acc, "break", "break;"); + add_keyword("continue", "continue;"); + add_keyword("break", "break;"); } else { - add_keyword(ctx, acc, "continue", "continue"); - add_keyword(ctx, acc, "break", "break"); + add_keyword("continue", "continue"); + add_keyword("break", "break"); } } - if expects_item || ctx.expects_non_trait_assoc_item() || ctx.expect_record_field() { - add_keyword(ctx, acc, "pub(crate)", "pub(crate) "); - add_keyword(ctx, acc, "pub", "pub "); - } if !ctx.is_trivial_path { return; @@ -143,8 +146,6 @@ pub(crate) fn complete_expr_keyword(acc: &mut Completions, ctx: &CompletionConte }; add_keyword( - ctx, - acc, "return", match (ctx.can_be_stmt, fn_def.ret_type().is_some()) { (true, true) => "return $0;", @@ -161,15 +162,12 @@ fn add_keyword(ctx: &CompletionContext, acc: &mut Completions, kw: &str, snippet match ctx.config.snippet_cap { Some(cap) => { - let tmp; - let snippet = if snippet.ends_with('}') && ctx.incomplete_let { + if snippet.ends_with('}') && ctx.incomplete_let { cov_mark::hit!(let_semi); - tmp = format!("{};", snippet); - &tmp + item.insert_snippet(cap, format!("{};", snippet)); } else { - snippet - }; - item.insert_snippet(cap, snippet); + item.insert_snippet(cap, snippet); + } } None => { item.insert_text(if snippet.contains('$') { kw } else { snippet }); @@ -232,21 +230,21 @@ mod tests { check( r"m$0", expect![[r#" + kw pub(crate) + kw pub + kw unsafe kw fn + kw const + kw type kw use kw impl kw trait + kw static + kw extern + kw mod kw enum kw struct kw union - kw mod - kw const - kw type - kw static - kw extern - kw unsafe - kw pub(crate) - kw pub "#]], ); } @@ -256,10 +254,16 @@ mod tests { check( r"fn quux() { $0 }", expect![[r#" + kw unsafe kw fn + kw const + kw type kw use kw impl kw trait + kw static + kw extern + kw mod kw match kw while kw while let @@ -268,12 +272,6 @@ mod tests { kw if let kw for kw let - kw mod - kw const - kw type - kw static - kw extern - kw unsafe kw return "#]], ); @@ -284,10 +282,16 @@ mod tests { check( r"fn quux() { if true { $0 } }", expect![[r#" + kw unsafe kw fn + kw const + kw type kw use kw impl kw trait + kw static + kw extern + kw mod kw match kw while kw while let @@ -296,12 +300,6 @@ mod tests { kw if let kw for kw let - kw mod - kw const - kw type - kw static - kw extern - kw unsafe kw return "#]], ); @@ -312,10 +310,16 @@ mod tests { check( r#"fn quux() { if true { () } $0 }"#, expect![[r#" + kw unsafe kw fn + kw const + kw type kw use kw impl kw trait + kw static + kw extern + kw mod kw match kw while kw while let @@ -326,12 +330,6 @@ mod tests { kw let kw else kw else if - kw mod - kw const - kw type - kw static - kw extern - kw unsafe kw return "#]], ); @@ -353,6 +351,7 @@ fn quux() -> i32 { } "#, expect![[r#" + kw unsafe kw match kw while kw while let @@ -360,7 +359,6 @@ fn quux() -> i32 { kw if kw if let kw for - kw unsafe kw return "#]], ); @@ -371,10 +369,10 @@ fn quux() -> i32 { check( r"trait My { $0 }", expect![[r#" + kw unsafe kw fn kw const kw type - kw unsafe "#]], ); } @@ -384,12 +382,12 @@ fn quux() -> i32 { check( r"impl My { $0 }", expect![[r#" + kw pub(crate) + kw pub + kw unsafe kw fn kw const kw type - kw unsafe - kw pub(crate) - kw pub "#]], ); } @@ -399,12 +397,12 @@ fn quux() -> i32 { check( r"impl My { #[foo] $0 }", expect![[r#" + kw pub(crate) + kw pub + kw unsafe kw fn kw const kw type - kw unsafe - kw pub(crate) - kw pub "#]], ); } @@ -414,10 +412,16 @@ fn quux() -> i32 { check( r"fn my() { loop { $0 } }", expect![[r#" + kw unsafe kw fn + kw const + kw type kw use kw impl kw trait + kw static + kw extern + kw mod kw match kw while kw while let @@ -426,12 +430,6 @@ fn quux() -> i32 { kw if let kw for kw let - kw mod - kw const - kw type - kw static - kw extern - kw unsafe kw continue kw break kw return diff --git a/crates/ide_completion/src/context.rs b/crates/ide_completion/src/context.rs index 923e35dbb..faf8469a5 100644 --- a/crates/ide_completion/src/context.rs +++ b/crates/ide_completion/src/context.rs @@ -288,6 +288,10 @@ impl<'a> CompletionContext<'a> { matches!(self.completion_location, Some(ImmediateLocation::ItemList)) } + pub(crate) fn expects_expression(&self) -> bool { + self.is_expr + } + pub(crate) fn has_block_expr_parent(&self) -> bool { matches!(self.completion_location, Some(ImmediateLocation::BlockExpr)) } @@ -316,7 +320,7 @@ impl<'a> CompletionContext<'a> { fn fill_keyword_patterns(&mut self, file_with_fake_ident: &SyntaxNode, offset: TextSize) { let fake_ident_token = file_with_fake_ident.token_at_offset(offset).right_biased().unwrap(); - let syntax_element = NodeOrToken::Token(fake_ident_token.clone()); + let syntax_element = NodeOrToken::Token(fake_ident_token); self.previous_token = previous_token(syntax_element.clone()); self.in_loop_body = is_in_loop_body(syntax_element.clone()); self.is_match_arm = is_match_arm(syntax_element.clone()); @@ -338,8 +342,6 @@ impl<'a> CompletionContext<'a> { let fn_is_prev = self.previous_token_is(T![fn]); let for_is_prev2 = for_is_prev2(syntax_element.clone()); self.no_completion_required = (fn_is_prev && !inside_impl_trait_block) || for_is_prev2; - - self.completion_location = determine_location(fake_ident_token); } fn fill_impl_def(&mut self) { @@ -465,6 +467,7 @@ impl<'a> CompletionContext<'a> { Some(it) => it, None => return, }; + self.completion_location = determine_location(&name_like); match name_like { ast::NameLike::Lifetime(lifetime) => { self.classify_lifetime(original_file, lifetime, offset); diff --git a/crates/ide_completion/src/patterns.rs b/crates/ide_completion/src/patterns.rs index c8a88367d..f04471b57 100644 --- a/crates/ide_completion/src/patterns.rs +++ b/crates/ide_completion/src/patterns.rs @@ -24,12 +24,12 @@ pub(crate) enum ImmediateLocation { ItemList, } -pub(crate) fn determine_location(tok: SyntaxToken) -> Option { +pub(crate) fn determine_location(name_like: &ast::NameLike) -> Option { // First walk the element we are completing up to its highest node that has the same text range // as the element so that we can check in what context it immediately lies. We only do this for // NameRef -> Path as that's the only thing that makes sense to being "expanded" semantically. // We only wanna do this if the NameRef is the last segment of the path. - let node = match tok.parent().and_then(ast::NameLike::cast)? { + let node = match name_like { ast::NameLike::NameRef(name_ref) => { if let Some(segment) = name_ref.syntax().parent().and_then(ast::PathSegment::cast) { let p = segment.parent_path(); @@ -93,7 +93,8 @@ pub(crate) fn determine_location(tok: SyntaxToken) -> Option #[cfg(test)] fn check_location(code: &str, loc: ImmediateLocation) { check_pattern_is_applicable(code, |e| { - assert_eq!(determine_location(e.into_token().expect("Expected a token")), Some(loc)); + let name = &e.parent().and_then(ast::NameLike::cast).expect("Expected a namelike"); + assert_eq!(determine_location(name), Some(loc)); true }); } @@ -199,6 +200,11 @@ fn test_has_impl_as_prev_sibling() { check_pattern_is_applicable(r"impl A w$0 {}", |it| has_prev_sibling(it, IMPL)); } +#[test] +fn test_has_trait_as_prev_sibling() { + check_pattern_is_applicable(r"trait A w$0 {}", |it| has_prev_sibling(it, TRAIT)); +} + pub(crate) fn is_in_loop_body(element: SyntaxElement) -> bool { element .ancestors() diff --git a/crates/ide_completion/src/test_utils.rs b/crates/ide_completion/src/test_utils.rs index 6656fd725..93c7c872c 100644 --- a/crates/ide_completion/src/test_utils.rs +++ b/crates/ide_completion/src/test_utils.rs @@ -12,7 +12,7 @@ use ide_db::{ use itertools::Itertools; use stdx::{format_to, trim_indent}; use syntax::{AstNode, NodeOrToken, SyntaxElement}; -use test_utils::{assert_eq_text, RangeOrOffset}; +use test_utils::assert_eq_text; use crate::{item::CompletionKind, CompletionConfig, CompletionItem}; @@ -36,10 +36,7 @@ pub(crate) fn position(ra_fixture: &str) -> (RootDatabase, FilePosition) { let mut database = RootDatabase::default(); database.apply_change(change_fixture.change); let (file_id, range_or_offset) = change_fixture.file_position.expect("expected a marker ($0)"); - let offset = match range_or_offset { - RangeOrOffset::Range(_) => panic!(), - RangeOrOffset::Offset(it) => it, - }; + let offset = range_or_offset.expect_offset(); (database, FilePosition { file_id, offset }) } @@ -52,10 +49,11 @@ pub(crate) fn do_completion_with_config( code: &str, kind: CompletionKind, ) -> Vec { - let mut kind_completions: Vec = - get_all_items(config, code).into_iter().filter(|c| c.completion_kind == kind).collect(); - kind_completions.sort_by(|l, r| l.label().cmp(r.label())); - kind_completions + get_all_items(config, code) + .into_iter() + .filter(|c| c.completion_kind == kind) + .sorted_by(|l, r| l.label().cmp(r.label())) + .collect() } pub(crate) fn completion_list(code: &str, kind: CompletionKind) -> String { diff --git a/crates/ide_db/src/call_info/tests.rs b/crates/ide_db/src/call_info/tests.rs index 1aeda08e5..b585085f3 100644 --- a/crates/ide_db/src/call_info/tests.rs +++ b/crates/ide_db/src/call_info/tests.rs @@ -1,6 +1,5 @@ use base_db::{fixture::ChangeFixture, FilePosition}; use expect_test::{expect, Expect}; -use test_utils::RangeOrOffset; use crate::RootDatabase; @@ -10,10 +9,7 @@ pub(crate) fn position(ra_fixture: &str) -> (RootDatabase, FilePosition) { let mut database = RootDatabase::default(); database.apply_change(change_fixture.change); let (file_id, range_or_offset) = change_fixture.file_position.expect("expected a marker ($0)"); - let offset = match range_or_offset { - RangeOrOffset::Range(_) => panic!(), - RangeOrOffset::Offset(it) => it, - }; + let offset = range_or_offset.expect_offset(); (database, FilePosition { file_id, offset }) } diff --git a/crates/ide_db/src/traits/tests.rs b/crates/ide_db/src/traits/tests.rs index 2a5482024..de994407c 100644 --- a/crates/ide_db/src/traits/tests.rs +++ b/crates/ide_db/src/traits/tests.rs @@ -2,7 +2,6 @@ use base_db::{fixture::ChangeFixture, FilePosition}; use expect_test::{expect, Expect}; use hir::Semantics; use syntax::ast::{self, AstNode}; -use test_utils::RangeOrOffset; use crate::RootDatabase; @@ -12,10 +11,7 @@ pub(crate) fn position(ra_fixture: &str) -> (RootDatabase, FilePosition) { let mut database = RootDatabase::default(); database.apply_change(change_fixture.change); let (file_id, range_or_offset) = change_fixture.file_position.expect("expected a marker ($0)"); - let offset = match range_or_offset { - RangeOrOffset::Range(_) => panic!(), - RangeOrOffset::Offset(it) => it, - }; + let offset = range_or_offset.expect_offset(); (database, FilePosition { file_id, offset }) } diff --git a/crates/test_utils/src/lib.rs b/crates/test_utils/src/lib.rs index fce4fd6bf..bd017567c 100644 --- a/crates/test_utils/src/lib.rs +++ b/crates/test_utils/src/lib.rs @@ -96,6 +96,21 @@ pub enum RangeOrOffset { Offset(TextSize), } +impl RangeOrOffset { + pub fn expect_offset(self) -> TextSize { + match self { + RangeOrOffset::Offset(it) => it, + RangeOrOffset::Range(_) => panic!("expected an offset but got a range instead"), + } + } + pub fn expect_range(self) -> TextRange { + match self { + RangeOrOffset::Range(it) => it, + RangeOrOffset::Offset(_) => panic!("expected a range but got an offset"), + } + } +} + impl From for TextRange { fn from(selection: RangeOrOffset) -> Self { match selection { -- cgit v1.2.3 From 47ad752e6cca5eb5499168ec7f859879e384a5ab Mon Sep 17 00:00:00 2001 From: Lukas Wirth Date: Fri, 28 May 2021 22:03:31 +0200 Subject: Implement prev sibling determination for `CompletionContext` --- crates/ide_completion/src/completions/keyword.rs | 2 +- crates/ide_completion/src/context.rs | 40 +++---- crates/ide_completion/src/patterns.rs | 126 ++++++++++++++++++----- 3 files changed, 114 insertions(+), 54 deletions(-) diff --git a/crates/ide_completion/src/completions/keyword.rs b/crates/ide_completion/src/completions/keyword.rs index 662c389fe..06789b704 100644 --- a/crates/ide_completion/src/completions/keyword.rs +++ b/crates/ide_completion/src/completions/keyword.rs @@ -118,7 +118,7 @@ pub(crate) fn complete_expr_keyword(acc: &mut Completions, ctx: &CompletionConte add_keyword("let", "let "); } - if ctx.after_if { + if ctx.after_if() { add_keyword("else", "else {\n $0\n}"); add_keyword("else if", "else if $1 {\n $0\n}"); } diff --git a/crates/ide_completion/src/context.rs b/crates/ide_completion/src/context.rs index faf8469a5..8d6440cb2 100644 --- a/crates/ide_completion/src/context.rs +++ b/crates/ide_completion/src/context.rs @@ -17,8 +17,8 @@ use text_edit::Indel; use crate::{ patterns::{ - determine_location, for_is_prev2, has_prev_sibling, inside_impl_trait_block, - is_in_loop_body, is_match_arm, previous_token, ImmediateLocation, + determine_location, determine_prev_sibling, for_is_prev2, inside_impl_trait_block, + is_in_loop_body, is_match_arm, previous_token, ImmediateLocation, ImmediatePrevSibling, }, CompletionConfig, }; @@ -29,12 +29,6 @@ pub(crate) enum PatternRefutability { Irrefutable, } -#[derive(Copy, Clone, Debug, PartialEq, Eq)] -pub(crate) enum PrevSibling { - Trait, - Impl, -} - /// `CompletionContext` is created early during completion to figure out, where /// exactly is the cursor, syntax-wise. #[derive(Debug)] @@ -76,6 +70,7 @@ pub(crate) struct CompletionContext<'a> { pub(super) is_param: bool, pub(super) completion_location: Option, + pub(super) prev_sibling: Option, /// FIXME: `ActiveParameter` is string-based, which is very very wrong pub(super) active_parameter: Option, @@ -83,7 +78,6 @@ pub(crate) struct CompletionContext<'a> { pub(super) is_trivial_path: bool, /// If not a trivial path, the prefix (qualifier). pub(super) path_qual: Option, - pub(super) after_if: bool, /// `true` if we are a statement or a last expr in the block. pub(super) can_be_stmt: bool, /// `true` if we expect an expression at the cursor position. @@ -107,7 +101,6 @@ pub(crate) struct CompletionContext<'a> { // keyword patterns pub(super) previous_token: Option, - pub(super) prev_sibling: Option, pub(super) in_loop_body: bool, pub(super) is_match_arm: bool, pub(super) incomplete_let: bool, @@ -173,7 +166,6 @@ impl<'a> CompletionContext<'a> { is_pat_or_const: None, is_trivial_path: false, path_qual: None, - after_if: false, can_be_stmt: false, is_expr: false, is_new_item: false, @@ -308,7 +300,14 @@ impl<'a> CompletionContext<'a> { } pub(crate) fn has_impl_or_trait_prev_sibling(&self) -> bool { - self.prev_sibling.is_some() + matches!( + self.prev_sibling, + Some(ImmediatePrevSibling::ImplDefType) | Some(ImmediatePrevSibling::TraitDefName) + ) + } + + pub(crate) fn after_if(&self) -> bool { + matches!(self.prev_sibling, Some(ImmediatePrevSibling::IfExpr)) } pub(crate) fn is_path_disallowed(&self) -> bool { @@ -324,11 +323,6 @@ impl<'a> CompletionContext<'a> { self.previous_token = previous_token(syntax_element.clone()); self.in_loop_body = is_in_loop_body(syntax_element.clone()); self.is_match_arm = is_match_arm(syntax_element.clone()); - if has_prev_sibling(syntax_element.clone(), IMPL) { - self.prev_sibling = Some(PrevSibling::Impl) - } else if has_prev_sibling(syntax_element.clone(), TRAIT) { - self.prev_sibling = Some(PrevSibling::Trait) - } self.mod_declaration_under_caret = find_node_at_offset::(&file_with_fake_ident, offset) @@ -468,6 +462,7 @@ impl<'a> CompletionContext<'a> { None => return, }; self.completion_location = determine_location(&name_like); + self.prev_sibling = determine_prev_sibling(&name_like); match name_like { ast::NameLike::Lifetime(lifetime) => { self.classify_lifetime(original_file, lifetime, offset); @@ -656,17 +651,6 @@ impl<'a> CompletionContext<'a> { }) .unwrap_or(false); self.is_expr = path.syntax().parent().and_then(ast::PathExpr::cast).is_some(); - - if let Some(off) = name_ref.syntax().text_range().start().checked_sub(2.into()) { - if let Some(if_expr) = - self.sema.find_node_at_offset_with_macros::(original_file, off) - { - if if_expr.syntax().text_range().end() < name_ref.syntax().text_range().start() - { - self.after_if = true; - } - } - } } if let Some(field_expr) = ast::FieldExpr::cast(parent.clone()) { diff --git a/crates/ide_completion/src/patterns.rs b/crates/ide_completion/src/patterns.rs index f04471b57..8c4bdbed2 100644 --- a/crates/ide_completion/src/patterns.rs +++ b/crates/ide_completion/src/patterns.rs @@ -4,12 +4,19 @@ use syntax::{ algo::non_trivia_sibling, ast::{self, LoopBodyOwner}, match_ast, AstNode, Direction, NodeOrToken, SyntaxElement, - SyntaxKind::{self, *}, + SyntaxKind::*, SyntaxNode, SyntaxToken, T, }; #[cfg(test)] use crate::test_utils::{check_pattern_is_applicable, check_pattern_is_not_applicable}; +/// Direct parent container of the cursor position +#[derive(Copy, Clone, Debug, PartialEq, Eq)] +pub(crate) enum ImmediatePrevSibling { + IfExpr, + TraitDefName, + ImplDefType, +} /// Direct parent container of the cursor position #[derive(Copy, Clone, Debug, PartialEq, Eq)] @@ -24,35 +31,61 @@ pub(crate) enum ImmediateLocation { ItemList, } -pub(crate) fn determine_location(name_like: &ast::NameLike) -> Option { - // First walk the element we are completing up to its highest node that has the same text range - // as the element so that we can check in what context it immediately lies. We only do this for - // NameRef -> Path as that's the only thing that makes sense to being "expanded" semantically. - // We only wanna do this if the NameRef is the last segment of the path. - let node = match name_like { - ast::NameLike::NameRef(name_ref) => { - if let Some(segment) = name_ref.syntax().parent().and_then(ast::PathSegment::cast) { - let p = segment.parent_path(); - if p.parent_path().is_none() { - p.syntax() - .ancestors() - .take_while(|it| it.text_range() == p.syntax().text_range()) - .last()? - } else { - return None; +pub(crate) fn determine_prev_sibling(name_like: &ast::NameLike) -> Option { + let node = maximize_name_ref(name_like)?; + let node = match node.parent().and_then(ast::MacroCall::cast) { + // When a path is being typed after the name of a trait/type of an impl it is being + // parsed as a macro, so when the trait/impl has a block following it an we are between the + // name and block the macro will attach the block to itself so maximizing fails to take + // that into account + // FIXME path expr and statement have a similar problem with attrs + Some(call) + if call.excl_token().is_none() + && call.token_tree().map_or(false, |t| t.l_curly_token().is_some()) + && call.semicolon_token().is_none() => + { + call.syntax().clone() + } + _ => node, + }; + let prev_sibling = non_trivia_sibling(node.into(), Direction::Prev)?.into_node()?; + let res = match_ast! { + match prev_sibling { + ast::ExprStmt(it) => { + let node = it.expr()?.syntax().clone(); + match_ast! { + match node { + ast::IfExpr(_it) => ImmediatePrevSibling::IfExpr, + _ => return None, + } } - } else { - return None; - } + }, + ast::Trait(it) => if it.assoc_item_list().is_none() { + ImmediatePrevSibling::TraitDefName + } else { + return None + }, + ast::Impl(it) => if it.assoc_item_list().is_none() + && (it.for_token().is_none() || it.self_ty().is_some()) { + ImmediatePrevSibling::ImplDefType + } else { + return None + }, + _ => return None, } - it @ ast::NameLike::Name(_) | it @ ast::NameLike::Lifetime(_) => it.syntax().clone(), }; + Some(res) +} + +pub(crate) fn determine_location(name_like: &ast::NameLike) -> Option { + let node = maximize_name_ref(name_like)?; let parent = match node.parent() { Some(parent) => match ast::MacroCall::cast(parent.clone()) { // When a path is being typed in an (Assoc)ItemList the parser will always emit a macro_call. // This is usually fine as the node expansion code above already accounts for that with // the ancestors call, but there is one exception to this which is that when an attribute // precedes it the code above will not walk the Path to the parent MacroCall as their ranges differ. + // FIXME path expr and statement have a similar problem Some(call) if call.excl_token().is_none() && call.token_tree().is_none() @@ -90,6 +123,32 @@ pub(crate) fn determine_location(name_like: &ast::NameLike) -> Option Option { + // First walk the element we are completing up to its highest node that has the same text range + // as the element so that we can check in what context it immediately lies. We only do this for + // NameRef -> Path as that's the only thing that makes sense to being "expanded" semantically. + // We only wanna do this if the NameRef is the last segment of the path. + let node = match name_like { + ast::NameLike::NameRef(name_ref) => { + if let Some(segment) = name_ref.syntax().parent().and_then(ast::PathSegment::cast) { + let p = segment.parent_path(); + if p.parent_path().is_none() { + p.syntax() + .ancestors() + .take_while(|it| it.text_range() == p.syntax().text_range()) + .last()? + } else { + return None; + } + } else { + return None; + } + } + it @ ast::NameLike::Name(_) | it @ ast::NameLike::Lifetime(_) => it.syntax().clone(), + }; + Some(node) +} + #[cfg(test)] fn check_location(code: &str, loc: ImmediateLocation) { check_pattern_is_applicable(code, |e| { @@ -192,17 +251,34 @@ fn test_for_is_prev2() { check_pattern_is_applicable(r"for i i$0", for_is_prev2); } -pub(crate) fn has_prev_sibling(element: SyntaxElement, kind: SyntaxKind) -> bool { - previous_sibling_or_ancestor_sibling(element).filter(|it| it.kind() == kind).is_some() +#[cfg(test)] +fn check_prev_sibling(code: &str, sibling: impl Into>) { + check_pattern_is_applicable(code, |e| { + let name = &e.parent().and_then(ast::NameLike::cast).expect("Expected a namelike"); + assert_eq!(determine_prev_sibling(name), sibling.into()); + true + }); } + #[test] fn test_has_impl_as_prev_sibling() { - check_pattern_is_applicable(r"impl A w$0 {}", |it| has_prev_sibling(it, IMPL)); + check_prev_sibling(r"impl A w$0 ", ImmediatePrevSibling::ImplDefType); + check_prev_sibling(r"impl A w$0 {}", ImmediatePrevSibling::ImplDefType); + check_prev_sibling(r"impl A for A w$0 ", ImmediatePrevSibling::ImplDefType); + check_prev_sibling(r"impl A for A w$0 {}", ImmediatePrevSibling::ImplDefType); + check_prev_sibling(r"impl A for w$0 {}", None); + check_prev_sibling(r"impl A for w$0", None); } #[test] fn test_has_trait_as_prev_sibling() { - check_pattern_is_applicable(r"trait A w$0 {}", |it| has_prev_sibling(it, TRAIT)); + check_prev_sibling(r"trait A w$0 ", ImmediatePrevSibling::TraitDefName); + check_prev_sibling(r"trait A w$0 {}", ImmediatePrevSibling::TraitDefName); +} + +#[test] +fn test_has_if_expr_as_prev_sibling() { + check_prev_sibling(r"fn foo() { if true {} w$0", ImmediatePrevSibling::IfExpr); } pub(crate) fn is_in_loop_body(element: SyntaxElement) -> bool { -- cgit v1.2.3 From e42c448077ca2b7675320da3c5294bf4bebaedeb Mon Sep 17 00:00:00 2001 From: Lukas Wirth Date: Fri, 28 May 2021 22:18:52 +0200 Subject: More completion pattern tests --- crates/ide_completion/src/completions/keyword.rs | 2 - crates/ide_completion/src/patterns.rs | 193 +++++++++++++---------- 2 files changed, 109 insertions(+), 86 deletions(-) diff --git a/crates/ide_completion/src/completions/keyword.rs b/crates/ide_completion/src/completions/keyword.rs index 06789b704..e71a04b6e 100644 --- a/crates/ide_completion/src/completions/keyword.rs +++ b/crates/ide_completion/src/completions/keyword.rs @@ -39,8 +39,6 @@ pub(crate) fn complete_use_tree_keyword(acc: &mut Completions, ctx: &CompletionC } } -trait Foo {} - pub(crate) fn complete_expr_keyword(acc: &mut Completions, ctx: &CompletionContext) { if ctx.token.kind() == SyntaxKind::COMMENT { cov_mark::hit!(no_keyword_completion_in_comments); diff --git a/crates/ide_completion/src/patterns.rs b/crates/ide_completion/src/patterns.rs index 8c4bdbed2..caf0ef39f 100644 --- a/crates/ide_completion/src/patterns.rs +++ b/crates/ide_completion/src/patterns.rs @@ -52,7 +52,7 @@ pub(crate) fn determine_prev_sibling(name_like: &ast::NameLike) -> Option { - let node = it.expr()?.syntax().clone(); + let node = it.expr().filter(|_| it.semicolon_token().is_none())?.syntax().clone(); match_ast! { match node { ast::IfExpr(_it) => ImmediatePrevSibling::IfExpr, @@ -149,59 +149,6 @@ fn maximize_name_ref(name_like: &ast::NameLike) -> Option { Some(node) } -#[cfg(test)] -fn check_location(code: &str, loc: ImmediateLocation) { - check_pattern_is_applicable(code, |e| { - let name = &e.parent().and_then(ast::NameLike::cast).expect("Expected a namelike"); - assert_eq!(determine_location(name), Some(loc)); - true - }); -} - -#[test] -fn test_has_trait_parent() { - check_location(r"trait A { f$0 }", ImmediateLocation::Trait); -} - -#[test] -fn test_has_use_parent() { - check_location(r"use f$0", ImmediateLocation::Use); -} - -#[test] -fn test_has_impl_parent() { - check_location(r"impl A { f$0 }", ImmediateLocation::Impl); -} -#[test] -fn test_has_field_list_parent() { - check_location(r"struct Foo { f$0 }", ImmediateLocation::RecordField); - check_location(r"struct Foo { f$0 pub f: i32}", ImmediateLocation::RecordField); -} - -#[test] -fn test_has_block_expr_parent() { - check_location(r"fn my_fn() { let a = 2; f$0 }", ImmediateLocation::BlockExpr); -} - -#[test] -fn test_has_ident_pat_parent() { - check_location(r"fn my_fn(m$0) {}", ImmediateLocation::IdentPat); - check_location(r"fn my_fn() { let m$0 }", ImmediateLocation::IdentPat); - check_location(r"fn my_fn(&m$0) {}", ImmediateLocation::IdentPat); - check_location(r"fn my_fn() { let &m$0 }", ImmediateLocation::IdentPat); -} - -#[test] -fn test_has_ref_expr_parent() { - check_location(r"fn my_fn() { let x = &m$0 foo; }", ImmediateLocation::RefExpr); -} - -#[test] -fn test_has_item_list_or_source_file_parent() { - check_location(r"i$0", ImmediateLocation::ItemList); - check_location(r"mod foo { f$0 }", ImmediateLocation::ItemList); -} - pub(crate) fn inside_impl_trait_block(element: SyntaxElement) -> bool { // Here we search `impl` keyword up through the all ancestors, unlike in `has_impl_parent`, // where we only check the first parent with different text range. @@ -251,36 +198,6 @@ fn test_for_is_prev2() { check_pattern_is_applicable(r"for i i$0", for_is_prev2); } -#[cfg(test)] -fn check_prev_sibling(code: &str, sibling: impl Into>) { - check_pattern_is_applicable(code, |e| { - let name = &e.parent().and_then(ast::NameLike::cast).expect("Expected a namelike"); - assert_eq!(determine_prev_sibling(name), sibling.into()); - true - }); -} - -#[test] -fn test_has_impl_as_prev_sibling() { - check_prev_sibling(r"impl A w$0 ", ImmediatePrevSibling::ImplDefType); - check_prev_sibling(r"impl A w$0 {}", ImmediatePrevSibling::ImplDefType); - check_prev_sibling(r"impl A for A w$0 ", ImmediatePrevSibling::ImplDefType); - check_prev_sibling(r"impl A for A w$0 {}", ImmediatePrevSibling::ImplDefType); - check_prev_sibling(r"impl A for w$0 {}", None); - check_prev_sibling(r"impl A for w$0", None); -} - -#[test] -fn test_has_trait_as_prev_sibling() { - check_prev_sibling(r"trait A w$0 ", ImmediatePrevSibling::TraitDefName); - check_prev_sibling(r"trait A w$0 {}", ImmediatePrevSibling::TraitDefName); -} - -#[test] -fn test_has_if_expr_as_prev_sibling() { - check_prev_sibling(r"fn foo() { if true {} w$0", ImmediatePrevSibling::IfExpr); -} - pub(crate) fn is_in_loop_body(element: SyntaxElement) -> bool { element .ancestors() @@ -329,3 +246,111 @@ fn previous_sibling_or_ancestor_sibling(element: SyntaxElement) -> Option>) { + check_pattern_is_applicable(code, |e| { + let name = &e.parent().and_then(ast::NameLike::cast).expect("Expected a namelike"); + assert_eq!(determine_location(name), loc.into()); + true + }); + } + + fn check_prev_sibling(code: &str, sibling: impl Into>) { + check_pattern_is_applicable(code, |e| { + let name = &e.parent().and_then(ast::NameLike::cast).expect("Expected a namelike"); + assert_eq!(determine_prev_sibling(name), sibling.into()); + true + }); + } + + #[test] + fn test_trait_loc() { + check_location(r"trait A { f$0 }", ImmediateLocation::Trait); + check_location(r"trait A { #[attr] f$0 }", ImmediateLocation::Trait); + check_location(r"trait A { f$0 fn f() {} }", ImmediateLocation::Trait); + check_location(r"trait A { fn f() {} f$0 }", ImmediateLocation::Trait); + check_location(r"trait A$0 {}", None); + check_location(r"trait A { fn f$0 }", None); + } + + #[test] + fn test_impl_loc() { + check_location(r"impl A { f$0 }", ImmediateLocation::Impl); + check_location(r"impl A { #[attr] f$0 }", ImmediateLocation::Impl); + check_location(r"impl A { f$0 fn f() {} }", ImmediateLocation::Impl); + check_location(r"impl A { fn f() {} f$0 }", ImmediateLocation::Impl); + check_location(r"impl A$0 {}", None); + check_location(r"impl A { fn f$0 }", None); + } + + #[test] + fn test_use_loc() { + check_location(r"use f$0", ImmediateLocation::Use); + check_location(r"use f$0;", ImmediateLocation::Use); + check_location(r"use f::{f$0}", None); + check_location(r"use {f$0}", None); + } + + #[test] + fn test_record_field_loc() { + check_location(r"struct Foo { f$0 }", ImmediateLocation::RecordField); + check_location(r"struct Foo { f$0 pub f: i32}", ImmediateLocation::RecordField); + check_location(r"struct Foo { pub f: i32, f$0 }", ImmediateLocation::RecordField); + } + + #[test] + fn test_block_expr_loc() { + check_location(r"fn my_fn() { let a = 2; f$0 }", ImmediateLocation::BlockExpr); + check_location(r"fn my_fn() { f$0 f }", ImmediateLocation::BlockExpr); + } + + #[test] + fn test_ident_pat_loc() { + check_location(r"fn my_fn(m$0) {}", ImmediateLocation::IdentPat); + check_location(r"fn my_fn() { let m$0 }", ImmediateLocation::IdentPat); + check_location(r"fn my_fn(&m$0) {}", ImmediateLocation::IdentPat); + check_location(r"fn my_fn() { let &m$0 }", ImmediateLocation::IdentPat); + } + + #[test] + fn test_ref_expr_loc() { + check_location(r"fn my_fn() { let x = &m$0 foo; }", ImmediateLocation::RefExpr); + } + + #[test] + fn test_item_list_loc() { + check_location(r"i$0", ImmediateLocation::ItemList); + check_location(r"#[attr] i$0", ImmediateLocation::ItemList); + check_location(r"fn f() {} i$0", ImmediateLocation::ItemList); + check_location(r"mod foo { f$0 }", ImmediateLocation::ItemList); + check_location(r"mod foo { #[attr] f$0 }", ImmediateLocation::ItemList); + check_location(r"mod foo { fn f() {} f$0 }", ImmediateLocation::ItemList); + check_location(r"mod foo$0 {}", None); + } + + #[test] + fn test_impl_prev_sibling() { + check_prev_sibling(r"impl A w$0 ", ImmediatePrevSibling::ImplDefType); + check_prev_sibling(r"impl A w$0 {}", ImmediatePrevSibling::ImplDefType); + check_prev_sibling(r"impl A for A w$0 ", ImmediatePrevSibling::ImplDefType); + check_prev_sibling(r"impl A for A w$0 {}", ImmediatePrevSibling::ImplDefType); + check_prev_sibling(r"impl A for w$0 {}", None); + check_prev_sibling(r"impl A for w$0", None); + } + + #[test] + fn test_trait_prev_sibling() { + check_prev_sibling(r"trait A w$0 ", ImmediatePrevSibling::TraitDefName); + check_prev_sibling(r"trait A w$0 {}", ImmediatePrevSibling::TraitDefName); + } + + #[test] + fn test_if_expr_prev_sibling() { + check_prev_sibling(r"fn foo() { if true {} w$0", ImmediatePrevSibling::IfExpr); + check_prev_sibling(r"fn foo() { if true {}; w$0", None); + } +} -- cgit v1.2.3