diff --git a/crates/hir-def/src/adt.rs b/crates/hir-def/src/adt.rs index dcea679567..9bc1c54a3c 100644 --- a/crates/hir-def/src/adt.rs +++ b/crates/hir-def/src/adt.rs @@ -2,9 +2,10 @@ use std::sync::Arc; -use crate::tt::{Delimiter, DelimiterKind, Leaf, Subtree, TokenTree}; use base_db::CrateId; +use cfg::CfgOptions; use either::Either; + use hir_expand::{ name::{AsName, Name}, HirFileId, InFile, @@ -24,12 +25,12 @@ use crate::{ src::HasChildSource, src::HasSource, trace::Trace, + tt::{Delimiter, DelimiterKind, Leaf, Subtree, TokenTree}, type_ref::TypeRef, visibility::RawVisibility, EnumId, LocalEnumVariantId, LocalFieldId, LocalModuleId, Lookup, ModuleId, StructId, UnionId, VariantId, }; -use cfg::CfgOptions; /// Note that we use `StructData` for unions as well! #[derive(Debug, Clone, PartialEq, Eq)] diff --git a/crates/ide-completion/src/context.rs b/crates/ide-completion/src/context.rs index aa77f44953..ea54068b0f 100644 --- a/crates/ide-completion/src/context.rs +++ b/crates/ide-completion/src/context.rs @@ -571,28 +571,25 @@ impl<'a> CompletionContext<'a> { // try to skip completions on path with invalid colons // this approach works in normal path and inside token tree - match original_token.kind() { - T![:] => { - // return if no prev token before colon - let prev_token = original_token.prev_token()?; + if original_token.kind() == T![:] { + // return if no prev token before colon + let prev_token = original_token.prev_token()?; - // only has a single colon - if prev_token.kind() != T![:] { - return None; - } - - // has 3 colon or 2 coloncolon in a row - // special casing this as per discussion in https://github.com/rust-lang/rust-analyzer/pull/13611#discussion_r1031845205 - // and https://github.com/rust-lang/rust-analyzer/pull/13611#discussion_r1032812751 - if prev_token - .prev_token() - .map(|t| t.kind() == T![:] || t.kind() == T![::]) - .unwrap_or(false) - { - return None; - } + // only has a single colon + if prev_token.kind() != T![:] { + return None; + } + + // has 3 colon or 2 coloncolon in a row + // special casing this as per discussion in https://github.com/rust-lang/rust-analyzer/pull/13611#discussion_r1031845205 + // and https://github.com/rust-lang/rust-analyzer/pull/13611#discussion_r1032812751 + if prev_token + .prev_token() + .map(|t| t.kind() == T![:] || t.kind() == T![::]) + .unwrap_or(false) + { + return None; } - _ => {} } let AnalysisResult { diff --git a/crates/ide-completion/src/context/analysis.rs b/crates/ide-completion/src/context/analysis.rs index b5eb253d03..db0045aef6 100644 --- a/crates/ide-completion/src/context/analysis.rs +++ b/crates/ide-completion/src/context/analysis.rs @@ -29,6 +29,7 @@ pub(super) struct AnalysisResult { pub(super) analysis: CompletionAnalysis, pub(super) expected: (Option, Option), pub(super) qualifier_ctx: QualifierCtx, + /// the original token of the expanded file pub(super) token: SyntaxToken, pub(super) offset: TextSize, } @@ -213,15 +214,6 @@ fn analyze( let _p = profile::span("CompletionContext::analyze"); let ExpansionResult { original_file, speculative_file, offset, fake_ident_token, derive_ctx } = expansion_result; - let syntax_element = NodeOrToken::Token(fake_ident_token); - if is_in_token_of_for_loop(syntax_element.clone()) { - // for pat $0 - // there is nothing to complete here except `in` keyword - // don't bother populating the context - // FIXME: the completion calculations should end up good enough - // such that this special case becomes unnecessary - return None; - } // Overwrite the path kind for derives if let Some((original_file, file_with_fake_ident, offset, origin_attr)) = derive_ctx { @@ -249,37 +241,35 @@ fn analyze( return None; } - let name_like = match find_node_at_offset(&speculative_file, offset) { - Some(it) => it, - None => { - let analysis = if let Some(original) = ast::String::cast(original_token.clone()) { - CompletionAnalysis::String { - original, - expanded: ast::String::cast(self_token.clone()), + let Some(name_like) = find_node_at_offset(&speculative_file, offset) else { + let analysis = if let Some(original) = ast::String::cast(original_token.clone()) { + CompletionAnalysis::String { + original, + expanded: ast::String::cast(self_token.clone()), + } + } else { + // Fix up trailing whitespace problem + // #[attr(foo = $0 + let token = syntax::algo::skip_trivia_token(self_token.clone(), Direction::Prev)?; + let p = token.parent()?; + if p.kind() == SyntaxKind::TOKEN_TREE + && p.ancestors().any(|it| it.kind() == SyntaxKind::META) + { + let colon_prefix = previous_non_trivia_token(self_token.clone()) + .map_or(false, |it| T![:] == it.kind()); + CompletionAnalysis::UnexpandedAttrTT { + fake_attribute_under_caret: fake_ident_token + .parent_ancestors() + .find_map(ast::Attr::cast), + colon_prefix, } } else { - // Fix up trailing whitespace problem - // #[attr(foo = $0 - let token = syntax::algo::skip_trivia_token(self_token.clone(), Direction::Prev)?; - let p = token.parent()?; - if p.kind() == SyntaxKind::TOKEN_TREE - && p.ancestors().any(|it| it.kind() == SyntaxKind::META) - { - let colon_prefix = previous_non_trivia_token(self_token.clone()) - .map_or(false, |it| T![:] == it.kind()); - CompletionAnalysis::UnexpandedAttrTT { - fake_attribute_under_caret: syntax_element - .ancestors() - .find_map(ast::Attr::cast), - colon_prefix, - } - } else { - return None; - } - }; - return Some((analysis, (None, None), QualifierCtx::default())); - } + return None; + } + }; + return Some((analysis, (None, None), QualifierCtx::default())); }; + let expected = expected_type_and_name(sema, self_token, &name_like); let mut qual_ctx = QualifierCtx::default(); let analysis = match name_like { @@ -290,6 +280,22 @@ fn analyze( let parent = name_ref.syntax().parent()?; let (nameref_ctx, qualifier_ctx) = classify_name_ref(sema, &original_file, name_ref, parent)?; + + if let NameRefContext { + kind: + NameRefKind::Path(PathCompletionCtx { kind: PathKind::Expr { .. }, path, .. }, ..), + .. + } = &nameref_ctx + { + if is_in_token_of_for_loop(path) { + // for pat $0 + // there is nothing to complete here except `in` keyword + // don't bother populating the context + // Ideally this special casing wouldn't be needed, but the parser recovers + return None; + } + } + qual_ctx = qualifier_ctx; CompletionAnalysis::NameRef(nameref_ctx) } @@ -323,16 +329,14 @@ fn expected_type_and_name( ast::FieldExpr(e) => e .syntax() .ancestors() - .map_while(ast::FieldExpr::cast) - .last() - .map(|it| it.syntax().clone()), + .take_while(|it| ast::FieldExpr::can_cast(it.kind())) + .last(), ast::PathSegment(e) => e .syntax() .ancestors() .skip(1) .take_while(|it| ast::Path::can_cast(it.kind()) || ast::PathExpr::can_cast(it.kind())) - .find_map(ast::PathExpr::cast) - .map(|it| it.syntax().clone()), + .find(|it| ast::PathExpr::can_cast(it.kind())), _ => None } }; @@ -1270,40 +1274,29 @@ fn path_or_use_tree_qualifier(path: &ast::Path) -> Option<(ast::Path, bool)> { Some((use_tree.path()?, true)) } -pub(crate) fn is_in_token_of_for_loop(element: SyntaxElement) -> bool { +fn is_in_token_of_for_loop(path: &ast::Path) -> bool { // oh my ... (|| { - let syntax_token = element.into_token()?; - let range = syntax_token.text_range(); - let for_expr = syntax_token.parent_ancestors().find_map(ast::ForExpr::cast)?; - - // check if the current token is the `in` token of a for loop - if let Some(token) = for_expr.in_token() { - return Some(syntax_token == token); + let expr = path.syntax().parent().and_then(ast::PathExpr::cast)?; + let for_expr = expr.syntax().parent().and_then(ast::ForExpr::cast)?; + if for_expr.in_token().is_some() { + return Some(false); } let pat = for_expr.pat()?; - if range.end() < pat.syntax().text_range().end() { - // if we are inside or before the pattern we can't be at the `in` token position - return None; - } let next_sibl = next_non_trivia_sibling(pat.syntax().clone().into())?; Some(match next_sibl { - // the loop body is some node, if our token is at the start we are at the `in` position, - // otherwise we could be in a recovered expression, we don't wanna ruin completions there - syntax::NodeOrToken::Node(n) => n.text_range().start() == range.start(), - // the loop body consists of a single token, if we are this we are certainly at the `in` token position - syntax::NodeOrToken::Token(t) => t == syntax_token, + syntax::NodeOrToken::Node(n) => { + n.text_range().start() == path.syntax().text_range().start() + } + syntax::NodeOrToken::Token(t) => { + t.text_range().start() == path.syntax().text_range().start() + } }) })() .unwrap_or(false) } -#[test] -fn test_for_is_prev2() { - crate::tests::check_pattern_is_applicable(r"fn __() { for i i$0 }", is_in_token_of_for_loop); -} - -pub(crate) fn is_in_loop_body(node: &SyntaxNode) -> bool { +fn is_in_loop_body(node: &SyntaxNode) -> bool { node.ancestors() .take_while(|it| it.kind() != SyntaxKind::FN && it.kind() != SyntaxKind::CLOSURE_EXPR) .find_map(|it| { diff --git a/crates/ide-completion/src/item.rs b/crates/ide-completion/src/item.rs index 657eab5b1b..e607763560 100644 --- a/crates/ide-completion/src/item.rs +++ b/crates/ide-completion/src/item.rs @@ -14,9 +14,9 @@ use crate::{ render::{render_path_resolution, RenderContext}, }; -/// `CompletionItem` describes a single completion variant in the editor pop-up. -/// It is basically a POD with various properties. To construct a -/// `CompletionItem`, use `new` method and the `Builder` struct. +/// `CompletionItem` describes a single completion entity which expands to 1 or more entries in the +/// editor pop-up. It is basically a POD with various properties. To construct a +/// [`CompletionItem`], use [`Builder::new`] method and the [`Builder`] struct. #[derive(Clone)] pub struct CompletionItem { /// Label in the completion pop up which identifies completion. @@ -396,14 +396,20 @@ impl CompletionItem { self.trigger_call_info } - pub fn ref_match(&self) -> Option<(Mutability, TextSize, CompletionRelevance)> { + pub fn ref_match(&self) -> Option<(String, text_edit::Indel, CompletionRelevance)> { // Relevance of the ref match should be the same as the original // 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.type_match = Some(CompletionRelevanceTypeMatch::Exact); - self.ref_match.map(|(mutability, offset)| (mutability, offset, relevance)) + self.ref_match.map(|(mutability, offset)| { + ( + format!("&{}{}", mutability.as_keyword_for_ref(), self.label()), + text_edit::Indel::insert(offset, format!("&{}", mutability.as_keyword_for_ref())), + relevance, + ) + }) } pub fn imports_to_add(&self) -> &[LocatedImport] { diff --git a/crates/ide-completion/src/lib.rs b/crates/ide-completion/src/lib.rs index 4b48ec6bc3..6fe7811140 100644 --- a/crates/ide-completion/src/lib.rs +++ b/crates/ide-completion/src/lib.rs @@ -156,13 +156,15 @@ pub fn completions( // prevent `(` from triggering unwanted completion noise if trigger_character == Some('(') { - if let CompletionAnalysis::NameRef(NameRefContext { kind, .. }) = &analysis { - if let NameRefKind::Path( - path_ctx @ PathCompletionCtx { kind: PathKind::Vis { has_in_token }, .. }, - ) = kind - { - completions::vis::complete_vis_path(&mut completions, ctx, path_ctx, has_in_token); - } + if let CompletionAnalysis::NameRef(NameRefContext { + kind: + NameRefKind::Path( + path_ctx @ PathCompletionCtx { kind: PathKind::Vis { has_in_token }, .. }, + ), + .. + }) = analysis + { + completions::vis::complete_vis_path(&mut completions, ctx, path_ctx, has_in_token); } return Some(completions.into()); } @@ -170,7 +172,7 @@ pub fn completions( { let acc = &mut completions; - match &analysis { + match analysis { CompletionAnalysis::Name(name_ctx) => completions::complete_name(acc, ctx, name_ctx), CompletionAnalysis::NameRef(name_ref_ctx) => { completions::complete_name_ref(acc, ctx, name_ref_ctx) diff --git a/crates/ide-completion/src/render.rs b/crates/ide-completion/src/render.rs index bc578a2820..47d3e01aa7 100644 --- a/crates/ide-completion/src/render.rs +++ b/crates/ide-completion/src/render.rs @@ -529,8 +529,7 @@ mod tests { let relevance = display_relevance(it.relevance()); items.push(format!("{tag} {} {relevance}\n", it.label())); - if let Some((mutability, _offset, relevance)) = it.ref_match() { - let label = format!("&{}{}", mutability.as_keyword_for_ref(), it.label()); + if let Some((label, _indel, relevance)) = it.ref_match() { let relevance = display_relevance(relevance); items.push(format!("{tag} {label} {relevance}\n")); diff --git a/crates/ide-completion/src/tests.rs b/crates/ide-completion/src/tests.rs index 540b0fd0ef..578edcbaba 100644 --- a/crates/ide-completion/src/tests.rs +++ b/crates/ide-completion/src/tests.rs @@ -23,7 +23,7 @@ mod type_pos; mod use_tree; mod visibility; -use hir::{db::DefDatabase, PrefixKind, Semantics}; +use hir::{db::DefDatabase, PrefixKind}; use ide_db::{ base_db::{fixture::ChangeFixture, FileLoader, FilePosition}, imports::insert_use::{ImportGranularity, InsertUseConfig}, @@ -31,7 +31,6 @@ use ide_db::{ }; use itertools::Itertools; use stdx::{format_to, trim_indent}; -use syntax::{AstNode, NodeOrToken, SyntaxElement}; use test_utils::assert_eq_text; use crate::{ @@ -216,15 +215,6 @@ pub(crate) fn check_edit_with_config( assert_eq_text!(&ra_fixture_after, &actual) } -pub(crate) fn check_pattern_is_applicable(code: &str, check: impl FnOnce(SyntaxElement) -> bool) { - let (db, pos) = position(code); - - let sema = Semantics::new(&db); - let original_file = sema.parse(pos.file_id); - let token = original_file.syntax().token_at_offset(pos.offset).left_biased().unwrap(); - assert!(check(NodeOrToken::Token(token))); -} - pub(crate) fn get_all_items( config: CompletionConfig, code: &str, @@ -246,8 +236,9 @@ pub(crate) fn get_all_items( } #[test] -fn test_no_completions_required() { +fn test_no_completions_in_for_loop_in_kw_pos() { assert_eq!(completion_list(r#"fn foo() { for i i$0 }"#), String::new()); + assert_eq!(completion_list(r#"fn foo() { for i in$0 }"#), String::new()); } #[test] diff --git a/crates/rust-analyzer/src/to_proto.rs b/crates/rust-analyzer/src/to_proto.rs index af2c868893..bee85cdd13 100644 --- a/crates/rust-analyzer/src/to_proto.rs +++ b/crates/rust-analyzer/src/to_proto.rs @@ -242,15 +242,16 @@ fn completion_item( let text_edit = { let mut text_edit = None; let source_range = item.source_range(); - for indel in item.text_edit().iter() { + for indel in item.text_edit() { if indel.delete.contains_range(source_range) { + // Extract this indel as the main edit text_edit = Some(if indel.delete == source_range { self::completion_text_edit(line_index, insert_replace_support, indel.clone()) } else { assert!(source_range.end() == indel.delete.end()); let range1 = TextRange::new(indel.delete.start(), source_range.start()); let range2 = source_range; - let indel1 = Indel::replace(range1, String::new()); + let indel1 = Indel::delete(range1); let indel2 = Indel::replace(range2, indel.insert.clone()); additional_text_edits.push(self::text_edit(line_index, indel1)); self::completion_text_edit(line_index, insert_replace_support, indel2) @@ -316,18 +317,13 @@ fn completion_item( } } - if let Some((mutability, offset, relevance)) = item.ref_match() { - let mut lsp_item_with_ref = lsp_item.clone(); + if let Some((label, indel, relevance)) = item.ref_match() { + let mut lsp_item_with_ref = lsp_types::CompletionItem { label, ..lsp_item.clone() }; + lsp_item_with_ref + .additional_text_edits + .get_or_insert_with(Default::default) + .push(self::text_edit(line_index, indel)); set_score(&mut lsp_item_with_ref, max_relevance, relevance); - lsp_item_with_ref.label = - format!("&{}{}", mutability.as_keyword_for_ref(), lsp_item_with_ref.label); - lsp_item_with_ref.additional_text_edits.get_or_insert_with(Default::default).push( - self::text_edit( - line_index, - Indel::insert(offset, format!("&{}", mutability.as_keyword_for_ref())), - ), - ); - acc.push(lsp_item_with_ref); };