From 173bb10a7698a84faa2e346dc78c8649751942c9 Mon Sep 17 00:00:00 2001 From: Lukas Wirth Date: Sat, 18 Jun 2022 00:47:28 +0200 Subject: [PATCH] internal: Split flyimport into its 3 applicable contexts --- .../src/completions/flyimport.rs | 191 +++++++++++------- crates/ide-completion/src/context.rs | 37 +--- crates/ide-completion/src/context/analysis.rs | 5 +- crates/ide-completion/src/lib.rs | 36 ++-- crates/ide-completion/src/render/function.rs | 14 +- crates/ide-completion/src/tests/flyimport.rs | 20 +- 6 files changed, 171 insertions(+), 132 deletions(-) diff --git a/crates/ide-completion/src/completions/flyimport.rs b/crates/ide-completion/src/completions/flyimport.rs index 33ae365cff7..15e431f9c50 100644 --- a/crates/ide-completion/src/completions/flyimport.rs +++ b/crates/ide-completion/src/completions/flyimport.rs @@ -1,16 +1,16 @@ //! See [`import_on_the_fly`]. use hir::{ItemInNs, ModuleDef}; use ide_db::imports::{ - import_assets::{ImportAssets, ImportCandidate, LocatedImport}, + import_assets::{ImportAssets, LocatedImport}, insert_use::ImportScope, }; use itertools::Itertools; -use syntax::{AstNode, SyntaxNode, T}; +use syntax::{ast, AstNode, SyntaxNode, T}; use crate::{ context::{ - CompletionContext, NameRefContext, NameRefKind, PathCompletionCtx, PathKind, - PatternContext, TypeLocation, + CompletionContext, DotAccess, PathCompletionCtx, PathKind, PatternContext, Qualified, + TypeLocation, }, render::{render_resolution_with_import, RenderContext}, }; @@ -108,45 +108,108 @@ use super::Completions; // The feature can be forcefully turned off in the settings with the `rust-analyzer.completion.autoimport.enable` flag. // Note that having this flag set to `true` does not guarantee that the feature is enabled: your client needs to have the corresponding // capability enabled. -pub(crate) fn import_on_the_fly(acc: &mut Completions, ctx: &CompletionContext) -> Option<()> { +pub(crate) fn import_on_the_fly_path( + acc: &mut Completions, + ctx: &CompletionContext, + path_ctx: &PathCompletionCtx, +) -> Option<()> { if !ctx.config.enable_imports_on_the_fly { return None; } - let path_kind = match ctx.nameref_ctx() { - Some(NameRefContext { + let (kind, qualified) = match path_ctx { + PathCompletionCtx { kind: - Some(NameRefKind::Path(PathCompletionCtx { - kind: - kind @ (PathKind::Expr { .. } - | PathKind::Type { .. } - | PathKind::Attr { .. } - | PathKind::Derive { .. } - | PathKind::Pat), - .. - })), + kind @ (PathKind::Expr { .. } + | PathKind::Type { .. } + | PathKind::Attr { .. } + | PathKind::Derive { .. } + | PathKind::Pat), + qualified, .. - }) => Some(kind), - Some(NameRefContext { kind: Some(NameRefKind::DotAccess(_)), .. }) => None, - None if matches!(ctx.pattern_ctx, Some(PatternContext { record_pat: None, .. })) => { - Some(&PathKind::Pat) - } + } => (Some(kind), qualified), + _ => return None, + }; + let potential_import_name = import_name(ctx); + let qualifier = match qualified { + Qualified::With { path, .. } => Some(path.clone()), + _ => None, + }; + let import_assets = import_assets_for_path(ctx, &potential_import_name, qualifier.clone())?; + + import_on_the_fly( + acc, + ctx, + kind, + import_assets, + qualifier.map(|it| it.syntax().clone()).or_else(|| ctx.original_token.parent())?, + potential_import_name, + ) +} + +pub(crate) fn import_on_the_fly_dot( + acc: &mut Completions, + ctx: &CompletionContext, + dot_access: &DotAccess, +) -> Option<()> { + if !ctx.config.enable_imports_on_the_fly { + return None; + } + let receiver = dot_access.receiver.as_ref()?; + let ty = dot_access.receiver_ty.as_ref()?; + let potential_import_name = import_name(ctx); + let import_assets = ImportAssets::for_fuzzy_method_call( + ctx.module, + ty.original.clone(), + potential_import_name.clone(), + receiver.syntax().clone(), + )?; + + import_on_the_fly( + acc, + ctx, + None, + import_assets, + receiver.syntax().clone(), + potential_import_name, + ) +} + +pub(crate) fn import_on_the_fly_pat( + acc: &mut Completions, + ctx: &CompletionContext, + pat_ctx: &PatternContext, +) -> Option<()> { + if !ctx.config.enable_imports_on_the_fly { + return None; + } + let kind = match pat_ctx { + PatternContext { record_pat: None, .. } => PathKind::Pat, _ => return None, }; - let potential_import_name = { - let token_kind = ctx.token.kind(); - if matches!(token_kind, T![.] | T![::]) { - String::new() - } else { - ctx.token.to_string() - } - }; + let potential_import_name = import_name(ctx); + let import_assets = import_assets_for_path(ctx, &potential_import_name, None)?; + import_on_the_fly( + acc, + ctx, + Some(&kind), + import_assets, + ctx.original_token.parent()?, + potential_import_name, + ) +} + +fn import_on_the_fly( + acc: &mut Completions, + ctx: &CompletionContext, + path_kind: Option<&PathKind>, + import_assets: ImportAssets, + position: SyntaxNode, + potential_import_name: String, +) -> Option<()> { let _p = profile::span("import_on_the_fly").detail(|| potential_import_name.clone()); - let user_input_lowercased = potential_import_name.to_lowercase(); - let import_assets = import_assets(ctx, potential_import_name)?; - let position = position_for_import(ctx, Some(import_assets.import_candidate()))?; if ImportScope::find_insert_use_container(&position, &ctx.sema).is_none() { return None; } @@ -194,6 +257,7 @@ pub(crate) fn import_on_the_fly(acc: &mut Completions, ctx: &CompletionContext) (PathKind::Derive { .. }, _) => false, } }; + let user_input_lowercased = potential_import_name.to_lowercase(); acc.add_all( import_assets @@ -216,47 +280,36 @@ pub(crate) fn import_on_the_fly(acc: &mut Completions, ctx: &CompletionContext) Some(()) } -pub(crate) fn position_for_import( - ctx: &CompletionContext, - import_candidate: Option<&ImportCandidate>, -) -> Option { - Some( - match import_candidate { - Some(ImportCandidate::TraitAssocItem(_)) => ctx.path_qual()?.syntax(), - Some(ImportCandidate::TraitMethod(_)) => ctx.dot_receiver()?.syntax(), - Some(ImportCandidate::Path(_)) | None => return ctx.original_token.parent(), - } - .clone(), - ) -} - -fn import_assets(ctx: &CompletionContext, fuzzy_name: String) -> Option { - let current_module = ctx.module; - if let Some(dot_receiver) = ctx.dot_receiver() { - ImportAssets::for_fuzzy_method_call( - current_module, - ctx.sema.type_of_expr(dot_receiver)?.original, - fuzzy_name, - dot_receiver.syntax().clone(), - ) +fn import_name(ctx: &CompletionContext) -> String { + let token_kind = ctx.token.kind(); + if matches!(token_kind, T![.] | T![::]) { + String::new() } else { - let fuzzy_name_length = fuzzy_name.len(); - let mut assets_for_path = ImportAssets::for_fuzzy_path( - current_module, - ctx.path_qual().cloned(), - fuzzy_name, - &ctx.sema, - ctx.token.parent()?, - )?; - if fuzzy_name_length < 3 { - cov_mark::hit!(flyimport_exact_on_short_path); - assets_for_path.path_fuzzy_name_to_exact(false); - } - Some(assets_for_path) + ctx.token.to_string() } } -pub(crate) fn compute_fuzzy_completion_order_key( +fn import_assets_for_path( + ctx: &CompletionContext, + potential_import_name: &str, + qualifier: Option, +) -> Option { + let fuzzy_name_length = potential_import_name.len(); + let mut assets_for_path = ImportAssets::for_fuzzy_path( + ctx.module, + qualifier, + potential_import_name.to_owned(), + &ctx.sema, + ctx.token.parent()?, + )?; + if fuzzy_name_length < 3 { + cov_mark::hit!(flyimport_exact_on_short_path); + assets_for_path.path_fuzzy_name_to_exact(false); + } + Some(assets_for_path) +} + +fn compute_fuzzy_completion_order_key( proposed_mod_path: &hir::ModPath, user_input_lowercased: &str, ) -> usize { diff --git a/crates/ide-completion/src/context.rs b/crates/ide-completion/src/context.rs index 825047c5cf9..5c408c0cc05 100644 --- a/crates/ide-completion/src/context.rs +++ b/crates/ide-completion/src/context.rs @@ -328,8 +328,8 @@ pub(crate) struct CompletionContext<'a> { // FIXME: This shouldn't exist pub(super) previous_token: Option, + // We might wanna split these out of CompletionContext pub(super) ident_ctx: IdentContext, - pub(super) pattern_ctx: Option, pub(super) qualifier_ctx: QualifierCtx, @@ -362,41 +362,6 @@ impl<'a> CompletionContext<'a> { FamousDefs(&self.sema, self.krate) } - // FIXME: This shouldn't exist - pub(super) fn nameref_ctx(&self) -> Option<&NameRefContext> { - match &self.ident_ctx { - IdentContext::NameRef(it) => Some(it), - _ => None, - } - } - - // FIXME: This shouldn't exist - pub(crate) fn dot_receiver(&self) -> Option<&ast::Expr> { - match self.nameref_ctx() { - Some(NameRefContext { - kind: Some(NameRefKind::DotAccess(DotAccess { receiver, .. })), - .. - }) => receiver.as_ref(), - _ => None, - } - } - - // FIXME: This shouldn't exist - pub(crate) fn path_context(&self) -> Option<&PathCompletionCtx> { - self.nameref_ctx().and_then(|ctx| match &ctx.kind { - Some(NameRefKind::Path(path)) => Some(path), - _ => None, - }) - } - - // FIXME: This shouldn't exist - pub(crate) fn path_qual(&self) -> Option<&ast::Path> { - self.path_context().and_then(|it| match &it.qualified { - Qualified::With { path, .. } => Some(path), - _ => None, - }) - } - /// Checks if an item is visible and not `doc(hidden)` at the completion site. pub(crate) fn is_visible(&self, item: &I) -> Visible where diff --git a/crates/ide-completion/src/context/analysis.rs b/crates/ide-completion/src/context/analysis.rs index 25b384d3238..0efeb852071 100644 --- a/crates/ide-completion/src/context/analysis.rs +++ b/crates/ide-completion/src/context/analysis.rs @@ -430,9 +430,12 @@ impl<'a> CompletionContext<'a> { let (nameref_ctx, pat_ctx, qualifier_ctx) = Self::classify_name_ref(&self.sema, &original_file, name_ref, parent.clone()); + if !matches!(nameref_ctx.kind, Some(NameRefKind::Path(_))) { + // FIXME: Pattern context should probably be part of ident_ctx + self.pattern_ctx = pat_ctx; + } self.qualifier_ctx = qualifier_ctx; self.ident_ctx = IdentContext::NameRef(nameref_ctx); - self.pattern_ctx = pat_ctx; } ast::NameLike::Name(name) => { let (name_ctx, pat_ctx) = Self::classify_name(&self.sema, original_file, name)?; diff --git a/crates/ide-completion/src/lib.rs b/crates/ide-completion/src/lib.rs index e0e4926463c..9f548c888ad 100644 --- a/crates/ide-completion/src/lib.rs +++ b/crates/ide-completion/src/lib.rs @@ -10,7 +10,6 @@ mod render; mod tests; mod snippet; -use completions::flyimport::position_for_import; use ide_db::{ base_db::FilePosition, helpers::mod_path_to_ast, @@ -25,11 +24,7 @@ use text_edit::TextEdit; use crate::{ completions::Completions, - context::{ - CompletionContext, - IdentContext::{self, NameRef}, - NameRefContext, NameRefKind, - }, + context::{CompletionContext, IdentContext, NameRefContext, NameRefKind}, }; pub use crate::{ @@ -156,8 +151,10 @@ pub fn completions( // prevent `(` from triggering unwanted completion noise if trigger_character == Some('(') { - if let NameRef(NameRefContext { kind: Some(NameRefKind::Path(path_ctx)), .. }) = - &ctx.ident_ctx + if let IdentContext::NameRef(NameRefContext { + kind: Some(NameRefKind::Path(path_ctx)), + .. + }) = &ctx.ident_ctx { completions::vis::complete_vis_path(&mut completions, ctx, path_ctx); } @@ -171,16 +168,18 @@ pub fn completions( match &ctx.ident_ctx { IdentContext::Name(name_ctx) => { completions::field::complete_field_list_record_variant(acc, ctx, name_ctx); - completions::mod_::complete_mod(acc, ctx, name_ctx); completions::item_list::trait_impl::complete_trait_impl_name(acc, ctx, name_ctx); + completions::mod_::complete_mod(acc, ctx, name_ctx); } - NameRef(name_ref_ctx @ NameRefContext { kind, .. }) => { + IdentContext::NameRef(name_ref_ctx @ NameRefContext { kind, .. }) => { completions::expr::complete_expr_path(acc, ctx, name_ref_ctx); completions::field::complete_field_list_tuple_variant(acc, ctx, name_ref_ctx); - completions::use_::complete_use_tree(acc, ctx, name_ref_ctx); completions::item_list::complete_item_list(acc, ctx, name_ref_ctx); + completions::use_::complete_use_tree(acc, ctx, name_ref_ctx); + match kind { Some(NameRefKind::Path(path_ctx)) => { + completions::flyimport::import_on_the_fly_path(acc, ctx, path_ctx); completions::record::complete_record_expr_func_update(acc, ctx, path_ctx); completions::attribute::complete_attribute(acc, ctx, path_ctx); completions::attribute::complete_derive(acc, ctx, path_ctx); @@ -193,6 +192,7 @@ pub fn completions( completions::vis::complete_vis_path(acc, ctx, path_ctx); } Some(NameRefKind::DotAccess(dot_access)) => { + completions::flyimport::import_on_the_fly_dot(acc, ctx, dot_access); completions::dot::complete_dot(acc, ctx, dot_access); completions::postfix::complete_postfix(acc, ctx, dot_access); } @@ -224,19 +224,11 @@ pub fn completions( } if let Some(pattern_ctx) = &ctx.pattern_ctx { + completions::flyimport::import_on_the_fly_pat(acc, ctx, pattern_ctx); completions::fn_param::complete_fn_param(acc, ctx, pattern_ctx); + completions::pattern::complete_pattern(acc, ctx, pattern_ctx); completions::record::complete_record_pattern_fields(acc, ctx, pattern_ctx); - // FIXME: this check is odd, we shouldn't need this? - if !matches!( - ctx.ident_ctx, - IdentContext::NameRef(NameRefContext { kind: Some(NameRefKind::Path(_)), .. }) - ) { - completions::pattern::complete_pattern(acc, ctx, pattern_ctx); - } } - - // FIXME: This should be split - completions::flyimport::import_on_the_fly(acc, ctx); } Some(completions) @@ -252,7 +244,7 @@ pub fn resolve_completion_edits( ) -> Option> { let _p = profile::span("resolve_completion_edits"); let ctx = CompletionContext::new(db, position, config)?; - let position_for_import = &position_for_import(&ctx, None)?; + let position_for_import = &ctx.original_token.parent()?; let scope = ImportScope::find_insert_use_container(position_for_import, &ctx.sema)?; let current_module = ctx.sema.scope(position_for_import)?.module(); diff --git a/crates/ide-completion/src/render/function.rs b/crates/ide-completion/src/render/function.rs index ad240503c14..811a88704d5 100644 --- a/crates/ide-completion/src/render/function.rs +++ b/crates/ide-completion/src/render/function.rs @@ -9,7 +9,7 @@ use syntax::SmolStr; use crate::{ context::{ CompletionContext, DotAccess, DotAccessKind, IdentContext, NameRefContext, NameRefKind, - PathCompletionCtx, PathKind, + PathCompletionCtx, PathKind, Qualified, }, item::{Builder, CompletionItem, CompletionItemKind, CompletionRelevance}, render::{compute_exact_name_match, compute_ref_match, compute_type_match, RenderContext}, @@ -80,7 +80,17 @@ fn render( // FIXME For now we don't properly calculate the edits for ref match // completions on methods or qualified paths, so we've disabled them. // See #8058. - if matches!(func_kind, FuncKind::Function) && ctx.completion.path_qual().is_none() { + let qualified_path = matches!( + ctx.completion.ident_ctx, + IdentContext::NameRef(NameRefContext { + kind: Some(NameRefKind::Path(PathCompletionCtx { + qualified: Qualified::With { .. }, + .. + })), + .. + }) + ); + if matches!(func_kind, FuncKind::Function) && !qualified_path { item.ref_match(ref_match); } } diff --git a/crates/ide-completion/src/tests/flyimport.rs b/crates/ide-completion/src/tests/flyimport.rs index 41e6cf7aeeb..ca9408fb138 100644 --- a/crates/ide-completion/src/tests/flyimport.rs +++ b/crates/ide-completion/src/tests/flyimport.rs @@ -1,6 +1,9 @@ use expect_test::{expect, Expect}; -use crate::tests::{check_edit, check_edit_with_config, TEST_CONFIG}; +use crate::{ + context::NameRefKind, + tests::{check_edit, check_edit_with_config, TEST_CONFIG}, +}; fn check(ra_fixture: &str, expect: Expect) { let config = TEST_CONFIG; @@ -8,7 +11,20 @@ fn check(ra_fixture: &str, expect: Expect) { let ctx = crate::context::CompletionContext::new(&db, position, &config).unwrap(); let mut acc = crate::completions::Completions::default(); - crate::completions::flyimport::import_on_the_fly(&mut acc, &ctx); + if let Some(pattern_ctx) = &ctx.pattern_ctx { + crate::completions::flyimport::import_on_the_fly_pat(&mut acc, &ctx, pattern_ctx); + } + if let crate::context::IdentContext::NameRef(name_ref_ctx) = &ctx.ident_ctx { + match &name_ref_ctx.kind { + Some(NameRefKind::Path(path)) => { + crate::completions::flyimport::import_on_the_fly_path(&mut acc, &ctx, path); + } + Some(NameRefKind::DotAccess(dot_access)) => { + crate::completions::flyimport::import_on_the_fly_dot(&mut acc, &ctx, dot_access); + } + _ => (), + } + } expect.assert_eq(&super::render_completion_list(Vec::from(acc))); }