Auto merge of rust-lang#12573 - Veykril:completion, r=Veykril

bors · bors · commit 69f30964ea52 · 2022-06-17T22:48:14.000Z
internal: Split flyimport into its 3 applicable contexts
diff --git 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())?;
 
-    let potential_import_name = {
-        let token_kind = ctx.token.kind();
-        if matches!(token_kind, T![.] | T![::]) {
-            String::new()
-        } else {
-            ctx.token.to_string()
-        }
+    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 = 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<SyntaxNode> {
-    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_name(ctx: &CompletionContext) -> String {
+    let token_kind = ctx.token.kind();
+    if matches!(token_kind, T![.] | T![::]) {
+        String::new()
+    } else {
+        ctx.token.to_string()
+    }
 }
 
-fn import_assets(ctx: &CompletionContext, fuzzy_name: String) -> Option<ImportAssets> {
-    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(),
-        )
-    } 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)
+fn import_assets_for_path(
+    ctx: &CompletionContext,
+    potential_import_name: &str,
+    qualifier: Option<ast::Path>,
+) -> Option<ImportAssets> {
+    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)
 }
 
-pub(crate) fn compute_fuzzy_completion_order_key(
+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
@@ -328,8 +328,8 @@ pub(crate) struct CompletionContext<'a> {
     // FIXME: This shouldn't exist
     pub(super) previous_token: Option<SyntaxToken>,
 
+    // We might wanna split these out of CompletionContext
     pub(super) ident_ctx: IdentContext,
-
     pub(super) pattern_ctx: Option<PatternContext>,
     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<I>(&self, item: &I) -> Visible
     where
diff --git 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
@@ -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<Vec<TextEdit>> {
     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
diff --git a/crates/ide-completion/src/tests/flyimport.rs b/crates/ide-completion/src/tests/flyimport.rs
