From f610e2c2edfe8f3fdd0c9719e748af55093b449e Mon Sep 17 00:00:00 2001 From: Lukas Wirth Date: Sat, 2 Apr 2022 01:42:21 +0200 Subject: [PATCH] Simplify completion import insertion --- crates/ide/src/lib.rs | 2 +- .../src/completions/flyimport.rs | 16 +++---- .../ide_completion/src/completions/postfix.rs | 6 ++- .../ide_completion/src/completions/snippet.rs | 6 ++- crates/ide_completion/src/item.rs | 44 +++---------------- crates/ide_completion/src/lib.rs | 2 - crates/ide_completion/src/render.rs | 20 +++++---- crates/ide_completion/src/snippet.rs | 23 +++------- crates/ide_completion/src/tests.rs | 29 +++++++----- crates/rust-analyzer/src/to_proto.rs | 2 +- 10 files changed, 58 insertions(+), 92 deletions(-) diff --git a/crates/ide/src/lib.rs b/crates/ide/src/lib.rs index c9b53b1c48..e986331105 100644 --- a/crates/ide/src/lib.rs +++ b/crates/ide/src/lib.rs @@ -102,7 +102,7 @@ pub use ide_assists::{ Assist, AssistConfig, AssistId, AssistKind, AssistResolveStrategy, SingleResolve, }; pub use ide_completion::{ - CompletionConfig, CompletionItem, CompletionItemKind, CompletionRelevance, ImportEdit, Snippet, + CompletionConfig, CompletionItem, CompletionItemKind, CompletionRelevance, Snippet, SnippetScope, }; pub use ide_db::{ diff --git a/crates/ide_completion/src/completions/flyimport.rs b/crates/ide_completion/src/completions/flyimport.rs index 7d5709ec90..2154a1b3cb 100644 --- a/crates/ide_completion/src/completions/flyimport.rs +++ b/crates/ide_completion/src/completions/flyimport.rs @@ -10,7 +10,6 @@ use syntax::{AstNode, SyntaxNode, T}; use crate::{ context::{CompletionContext, PathKind}, render::{render_resolution_with_import, RenderContext}, - ImportEdit, }; use super::Completions; @@ -136,10 +135,10 @@ pub(crate) fn import_on_the_fly(acc: &mut Completions, ctx: &CompletionContext) let user_input_lowercased = potential_import_name.to_lowercase(); let import_assets = import_assets(ctx, potential_import_name)?; - let import_scope = ImportScope::find_insert_use_container( - &position_for_import(ctx, Some(import_assets.import_candidate()))?, - &ctx.sema, - )?; + let position = position_for_import(ctx, Some(import_assets.import_candidate()))?; + if ImportScope::find_insert_use_container(&position, &ctx.sema).is_none() { + return None; + } let path_kind = match ctx.path_kind() { Some(kind) => Some(kind), @@ -199,12 +198,7 @@ pub(crate) fn import_on_the_fly(acc: &mut Completions, ctx: &CompletionContext) &user_input_lowercased, ) }) - .filter_map(|import| { - render_resolution_with_import( - RenderContext::new(ctx), - ImportEdit { import, scope: import_scope.clone() }, - ) - }), + .filter_map(|import| render_resolution_with_import(RenderContext::new(ctx), import)), ); Some(()) } diff --git a/crates/ide_completion/src/completions/postfix.rs b/crates/ide_completion/src/completions/postfix.rs index 302694ea7c..175d6f1623 100644 --- a/crates/ide_completion/src/completions/postfix.rs +++ b/crates/ide_completion/src/completions/postfix.rs @@ -261,10 +261,12 @@ fn add_custom_postfix_completions( postfix_snippet: impl Fn(&str, &str, &str) -> Builder, receiver_text: &str, ) -> Option<()> { - let import_scope = ImportScope::find_insert_use_container(&ctx.token.parent()?, &ctx.sema)?; + if ImportScope::find_insert_use_container(&ctx.token.parent()?, &ctx.sema).is_none() { + return None; + } ctx.config.postfix_snippets().filter(|(_, snip)| snip.scope == SnippetScope::Expr).for_each( |(trigger, snippet)| { - let imports = match snippet.imports(ctx, &import_scope) { + let imports = match snippet.imports(ctx) { Some(imports) => imports, None => return, }; diff --git a/crates/ide_completion/src/completions/snippet.rs b/crates/ide_completion/src/completions/snippet.rs index 673ee51f78..0a3d594e41 100644 --- a/crates/ide_completion/src/completions/snippet.rs +++ b/crates/ide_completion/src/completions/snippet.rs @@ -104,10 +104,12 @@ fn add_custom_completions( cap: SnippetCap, scope: SnippetScope, ) -> Option<()> { - let import_scope = ImportScope::find_insert_use_container(&ctx.token.parent()?, &ctx.sema)?; + if ImportScope::find_insert_use_container(&ctx.token.parent()?, &ctx.sema).is_none() { + return None; + } ctx.config.prefix_snippets().filter(|(_, snip)| snip.scope == scope).for_each( |(trigger, snip)| { - let imports = match snip.imports(ctx, &import_scope) { + let imports = match snip.imports(ctx) { Some(imports) => imports, None => return, }; diff --git a/crates/ide_completion/src/item.rs b/crates/ide_completion/src/item.rs index 2b7202bb6d..3c2be32f4e 100644 --- a/crates/ide_completion/src/item.rs +++ b/crates/ide_completion/src/item.rs @@ -3,17 +3,10 @@ use std::fmt; use hir::{Documentation, Mutability}; -use ide_db::{ - helpers::mod_path_to_ast, - imports::{ - import_assets::LocatedImport, - insert_use::{self, ImportScope, InsertUseConfig}, - }, - SnippetCap, SymbolKind, -}; +use ide_db::{imports::import_assets::LocatedImport, SnippetCap, SymbolKind}; use smallvec::SmallVec; use stdx::{impl_from, never}; -use syntax::{algo, SmolStr, TextRange}; +use syntax::{SmolStr, TextRange}; use text_edit::TextEdit; /// `CompletionItem` describes a single completion variant in the editor pop-up. @@ -73,7 +66,7 @@ pub struct CompletionItem { ref_match: Option, /// The import data to add to completion's edits. - import_to_add: SmallVec<[ImportEdit; 1]>, + import_to_add: SmallVec<[LocatedImport; 1]>, } // We use custom debug for CompletionItem to make snapshot tests more readable. @@ -380,40 +373,17 @@ impl CompletionItem { self.ref_match.map(|mutability| (mutability, relevance)) } - pub fn imports_to_add(&self) -> &[ImportEdit] { + pub fn imports_to_add(&self) -> &[LocatedImport] { &self.import_to_add } } -/// An extra import to add after the completion is applied. -#[derive(Debug, Clone)] -pub struct ImportEdit { - pub import: LocatedImport, - pub scope: ImportScope, -} - -impl ImportEdit { - /// Attempts to insert the import to the given scope, producing a text edit. - /// May return no edit in edge cases, such as scope already containing the import. - pub fn to_text_edit(&self, cfg: InsertUseConfig) -> Option { - let _p = profile::span("ImportEdit::to_text_edit"); - - let new_ast = self.scope.clone_for_update(); - insert_use::insert_use(&new_ast, mod_path_to_ast(&self.import.import_path), &cfg); - let mut import_insert = TextEdit::builder(); - algo::diff(self.scope.as_syntax_node(), new_ast.as_syntax_node()) - .into_text_edit(&mut import_insert); - - Some(import_insert.finish()) - } -} - /// A helper to make `CompletionItem`s. #[must_use] #[derive(Clone)] pub(crate) struct Builder { source_range: TextRange, - imports_to_add: SmallVec<[ImportEdit; 1]>, + imports_to_add: SmallVec<[LocatedImport; 1]>, trait_name: Option, label: SmolStr, insert_text: Option, @@ -439,7 +409,7 @@ impl Builder { if let [import_edit] = &*self.imports_to_add { // snippets can have multiple imports, but normal completions only have up to one - if let Some(original_path) = import_edit.import.original_path.as_ref() { + if let Some(original_path) = import_edit.original_path.as_ref() { lookup = lookup.or_else(|| Some(label.clone())); label = SmolStr::from(format!("{} (use {})", label, original_path)); } @@ -533,7 +503,7 @@ impl Builder { self.trigger_call_info = Some(true); self } - pub(crate) fn add_import(&mut self, import_to_add: ImportEdit) -> &mut Builder { + pub(crate) fn add_import(&mut self, import_to_add: LocatedImport) -> &mut Builder { self.imports_to_add.push(import_to_add); self } diff --git a/crates/ide_completion/src/lib.rs b/crates/ide_completion/src/lib.rs index 0cbe82abba..8eb7615f8f 100644 --- a/crates/ide_completion/src/lib.rs +++ b/crates/ide_completion/src/lib.rs @@ -30,7 +30,6 @@ pub use crate::{ config::CompletionConfig, item::{ CompletionItem, CompletionItemKind, CompletionRelevance, CompletionRelevancePostfixMatch, - ImportEdit, }, snippet::{Snippet, SnippetScope}, }; @@ -193,7 +192,6 @@ pub fn resolve_completion_edits( let new_ast = scope.clone_for_update(); let mut import_insert = TextEdit::builder(); - // FIXME: lift out and make some tests here, this is ImportEdit::to_text_edit but changed to work with multiple edits imports.into_iter().for_each(|(full_import_path, imported_name)| { let items_with_name = items_locator::items_with_name( &ctx.sema, diff --git a/crates/ide_completion/src/render.rs b/crates/ide_completion/src/render.rs index a18c064960..e5ec273212 100644 --- a/crates/ide_completion/src/render.rs +++ b/crates/ide_completion/src/render.rs @@ -11,12 +11,14 @@ pub(crate) mod union_literal; pub(crate) mod literal; use hir::{AsAssocItem, HasAttrs, HirDisplay, ScopeDef}; -use ide_db::{helpers::item_name, RootDatabase, SnippetCap, SymbolKind}; +use ide_db::{ + helpers::item_name, imports::import_assets::LocatedImport, RootDatabase, SnippetCap, SymbolKind, +}; use syntax::{SmolStr, SyntaxKind, TextRange}; use crate::{ context::{PathCompletionCtx, PathKind}, - item::{CompletionRelevanceTypeMatch, ImportEdit}, + item::CompletionRelevanceTypeMatch, render::{function::render_fn, literal::render_variant_lit, macro_::render_macro}, CompletionContext, CompletionItem, CompletionItemKind, CompletionRelevance, }; @@ -25,7 +27,7 @@ use crate::{ pub(crate) struct RenderContext<'a> { completion: &'a CompletionContext<'a>, is_private_editable: bool, - import_to_add: Option, + import_to_add: Option, } impl<'a> RenderContext<'a> { @@ -38,7 +40,7 @@ impl<'a> RenderContext<'a> { self } - pub(crate) fn import_to_add(mut self, import_to_add: Option) -> Self { + pub(crate) fn import_to_add(mut self, import_to_add: Option) -> Self { self.import_to_add = import_to_add; self } @@ -156,14 +158,14 @@ pub(crate) fn render_resolution_simple( pub(crate) fn render_resolution_with_import( ctx: RenderContext<'_>, - import_edit: ImportEdit, + import_edit: LocatedImport, ) -> Option { - let resolution = ScopeDef::from(import_edit.import.original_item); + let resolution = ScopeDef::from(import_edit.original_item); let local_name = match resolution { ScopeDef::ModuleDef(hir::ModuleDef::Function(f)) => f.name(ctx.completion.db), ScopeDef::ModuleDef(hir::ModuleDef::Const(c)) => c.name(ctx.completion.db)?, ScopeDef::ModuleDef(hir::ModuleDef::TypeAlias(t)) => t.name(ctx.completion.db), - _ => item_name(ctx.db(), import_edit.import.original_item)?, + _ => item_name(ctx.db(), import_edit.original_item)?, }; Some(render_resolution_(ctx, local_name, Some(import_edit), resolution)) } @@ -171,7 +173,7 @@ pub(crate) fn render_resolution_with_import( fn render_resolution_( ctx: RenderContext<'_>, local_name: hir::Name, - import_to_add: Option, + import_to_add: Option, resolution: ScopeDef, ) -> CompletionItem { let _p = profile::span("render_resolution"); @@ -200,7 +202,7 @@ fn render_resolution_( fn render_resolution_simple_( ctx: RenderContext<'_>, local_name: hir::Name, - import_to_add: Option, + import_to_add: Option, resolution: ScopeDef, ) -> CompletionItem { let _p = profile::span("render_resolution"); diff --git a/crates/ide_completion/src/snippet.rs b/crates/ide_completion/src/snippet.rs index 1340080fdf..82632f440d 100644 --- a/crates/ide_completion/src/snippet.rs +++ b/crates/ide_completion/src/snippet.rs @@ -102,11 +102,11 @@ use std::ops::Deref; // } // ---- -use ide_db::imports::{import_assets::LocatedImport, insert_use::ImportScope}; +use ide_db::imports::import_assets::LocatedImport; use itertools::Itertools; use syntax::{ast, AstNode, GreenNode, SyntaxNode}; -use crate::{context::CompletionContext, ImportEdit}; +use crate::context::CompletionContext; /// A snippet scope describing where a snippet may apply to. /// These may differ slightly in meaning depending on the snippet trigger. @@ -156,12 +156,8 @@ impl Snippet { } /// Returns [`None`] if the required items do not resolve. - pub(crate) fn imports( - &self, - ctx: &CompletionContext, - import_scope: &ImportScope, - ) -> Option> { - import_edits(ctx, import_scope, &self.requires) + pub(crate) fn imports(&self, ctx: &CompletionContext) -> Option> { + import_edits(ctx, &self.requires) } pub fn snippet(&self) -> String { @@ -173,11 +169,7 @@ impl Snippet { } } -fn import_edits( - ctx: &CompletionContext, - import_scope: &ImportScope, - requires: &[GreenNode], -) -> Option> { +fn import_edits(ctx: &CompletionContext, requires: &[GreenNode]) -> Option> { let resolve = |import: &GreenNode| { let path = ast::Path::cast(SyntaxNode::new_root(import.clone()))?; let item = match ctx.scope.speculative_resolve(&path)? { @@ -186,10 +178,7 @@ fn import_edits( }; let path = ctx.module.find_use_path_prefixed(ctx.db, item, ctx.config.insert_use.prefix_kind)?; - Some((path.len() > 1).then(|| ImportEdit { - import: LocatedImport::new(path.clone(), item, item, None), - scope: import_scope.clone(), - })) + Some((path.len() > 1).then(|| LocatedImport::new(path.clone(), item, item, None))) }; let mut res = Vec::with_capacity(requires.len()); for import in requires { diff --git a/crates/ide_completion/src/tests.rs b/crates/ide_completion/src/tests.rs index f505e82d22..470fc37ca7 100644 --- a/crates/ide_completion/src/tests.rs +++ b/crates/ide_completion/src/tests.rs @@ -35,7 +35,7 @@ use stdx::{format_to, trim_indent}; use syntax::{AstNode, NodeOrToken, SyntaxElement}; use test_utils::assert_eq_text; -use crate::{CompletionConfig, CompletionItem, CompletionItemKind}; +use crate::{resolve_completion_edits, CompletionConfig, CompletionItem, CompletionItemKind}; /// Lots of basic item definitions const BASE_ITEMS_FIXTURE: &str = r#" @@ -178,15 +178,24 @@ pub(crate) fn check_edit_with_config( let mut actual = db.file_text(position.file_id).to_string(); let mut combined_edit = completion.text_edit().to_owned(); - completion - .imports_to_add() - .iter() - .filter_map(|edit| edit.to_text_edit(config.insert_use)) - .for_each(|text_edit| { - combined_edit.union(text_edit).expect( - "Failed to apply completion resolve changes: change ranges overlap, but should not", - ) - }); + + resolve_completion_edits( + &db, + &config, + position, + completion.imports_to_add().iter().filter_map(|import_edit| { + let import_path = &import_edit.import_path; + let import_name = import_path.segments().last()?; + Some((import_path.to_string(), import_name.to_string())) + }), + ) + .into_iter() + .flatten() + .for_each(|text_edit| { + combined_edit.union(text_edit).expect( + "Failed to apply completion resolve changes: change ranges overlap, but should not", + ) + }); combined_edit.apply(&mut actual); assert_eq_text!(&ra_fixture_after, &actual) diff --git a/crates/rust-analyzer/src/to_proto.rs b/crates/rust-analyzer/src/to_proto.rs index 7c5785bfb8..9b377f742b 100644 --- a/crates/rust-analyzer/src/to_proto.rs +++ b/crates/rust-analyzer/src/to_proto.rs @@ -283,7 +283,7 @@ fn completion_item( let imports: Vec<_> = imports .iter() .filter_map(|import_edit| { - let import_path = &import_edit.import.import_path; + let import_path = &import_edit.import_path; let import_name = import_path.segments().last()?; Some(lsp_ext::CompletionImport { full_import_path: import_path.to_string(),