Merge #11397

11397: internal: Refactor completion module split r=Veykril a=Veykril

Currently our completion infra is split into several modules, each trying to do completions for something specific. This "something" is rather unstructured as it stands now, we have a module for `flyimporting path` completions, `unqualified` and `qualified path` completions, modules for `pattern position` completions that only try to complete extra things for patterns that aren't done in the path modules, `attribute` completions that again only try to add builtin attribute completions without adding the normal path completions and a bunch of other special "entity" completions like lifetimes, method call/field access, function param cloning, ... which serve a more specific purpose than the previous listed ones.

As is evident, the former mentioned ones have some decent overlap which requires extra filtering in them so that they don't collide with each other duplicating a bunch of completions(which we had happen in the past at times).

Now this overlap mostly happens with path completions(and keyword completions as well in some sense) which gives me the feeling that having `qualified` and `unqualified` path completions be separate from the rest gives us more troubles than benefits in the long run.
So this is an attempt at changing this structure to instead still go by rough entity for special cases, but when it comes to paths we instead do the module split on the "path kinds"/"locations"(think pattern, type, expr position etc) that exist. This goes hand in hand with the test refactoring I have done that moved tests to "location oriented" modules as well as the `CompletionContext` refactoring that actually already started splitting the context up for path kinds.

This PR moves some path completions out of the `qualified` and `unqualified` path modules namely attribute, visibility, use and pattern paths.

Co-authored-by: Lukas Wirth <[email protected]>

Author: bors[bot]

crates/hir/src/display.rs

@@ -481,7 +481,7 @@ impl HirDisplay for Module {
         // FIXME: Module doesn't have visibility saved in data.
         match self.name(f.db) {
             Some(name) => write!(f, "mod {}", name),
-            None if self.crate_root(f.db) == *self => match self.krate().display_name(f.db) {
+            None if self.is_crate_root(f.db) => match self.krate().display_name(f.db) {
                 Some(name) => write!(f, "extern crate {}", name),
                 None => write!(f, "extern crate {{unknown}}"),
             },

crates/hir/src/lib.rs

@@ -452,6 +452,11 @@ impl Module {
         Module { id: def_map.module_id(def_map.root()) }
     }
 
+    pub fn is_crate_root(self, db: &dyn HirDatabase) -> bool {
+        let def_map = db.crate_def_map(self.id.krate());
+        def_map.root() == self.id.local_id
+    }
+
     /// Iterates over all child modules.
     pub fn children(self, db: &dyn HirDatabase) -> impl Iterator<Item = Module> {
         let def_map = self.id.def_map(db.upcast());

crates/hir/src/semantics.rs

@@ -949,12 +949,15 @@ impl<'db> SemanticsImpl<'db> {
         })?;
 
         match res {
-            Either::Left(path) => resolve_hir_path(
-                self.db,
-                &self.scope(derive.syntax()).resolver,
-                &Path::from_known_path(path, []),
-            )
-            .filter(|res| matches!(res, PathResolution::Def(ModuleDef::Module(_)))),
+            Either::Left(path) => {
+                let len = path.len();
+                resolve_hir_path(
+                    self.db,
+                    &self.scope(derive.syntax()).resolver,
+                    &Path::from_known_path(path, vec![None; len]),
+                )
+                .filter(|res| matches!(res, PathResolution::Def(ModuleDef::Module(_))))
+            }
             Either::Right(derive) => derive
                 .map(|call| MacroDef { id: self.db.lookup_intern_macro_call(call).def })
                 .map(PathResolution::Macro),

crates/hir_def/src/path.rs

@@ -92,7 +92,9 @@ impl Path {
         path: ModPath,
         generic_args: impl Into<Box<[Option<Interned<GenericArgs>>]>>,
     ) -> Path {
-        Path { type_anchor: None, mod_path: Interned::new(path), generic_args: generic_args.into() }
+        let generic_args = generic_args.into();
+        assert_eq!(path.len(), generic_args.len());
+        Path { type_anchor: None, mod_path: Interned::new(path), generic_args }
     }
 
     pub fn kind(&self) -> &PathKind {

crates/hir_ty/src/infer.rs

@@ -253,6 +253,7 @@ pub enum PointerCast {
     /// Go from a mut raw pointer to a const raw pointer.
     MutToConstPointer,
 
+    #[allow(dead_code)]
     /// Go from `*const [T; N]` to `*const T`
     ArrayToPointer,
 

crates/ide_completion/src/completions.rs

@@ -4,6 +4,7 @@ pub(crate) mod attribute;
 pub(crate) mod dot;
 pub(crate) mod flyimport;
 pub(crate) mod fn_param;
+pub(crate) mod format_string;
 pub(crate) mod keyword;
 pub(crate) mod lifetime;
 pub(crate) mod mod_;
@@ -14,7 +15,8 @@ pub(crate) mod record;
 pub(crate) mod snippet;
 pub(crate) mod trait_impl;
 pub(crate) mod unqualified_path;
-pub(crate) mod format_string;
+pub(crate) mod use_;
+pub(crate) mod vis;
 
 use std::iter;
 
@@ -97,6 +99,19 @@ impl Completions {
         item.add_to(self);
     }
 
+    pub(crate) fn add_nameref_keywords(&mut self, ctx: &CompletionContext) {
+        ["self::", "super::", "crate::"].into_iter().for_each(|kw| self.add_keyword(ctx, kw));
+    }
+
+    pub(crate) fn add_crate_roots(&mut self, ctx: &CompletionContext) {
+        ctx.process_all_names(&mut |name, res| match res {
+            ScopeDef::ModuleDef(hir::ModuleDef::Module(m)) if m.is_crate_root(ctx.db) => {
+                self.add_resolution(ctx, name, res);
+            }
+            _ => (),
+        });
+    }
+
     pub(crate) fn add_resolution(
         &mut self,
         ctx: &CompletionContext,

crates/ide_completion/src/completions/attribute.rs

@@ -1,8 +1,6 @@
-//! Completion for attributes
+//! Completion for (built-in) attributes, derives and lints.
 //!
-//! This module uses a bit of static metadata to provide completions
-//! for built-in attributes.
-//! Non-built-in attribute (excluding derives attributes) completions are done in [`super::unqualified_path`].
+//! This module uses a bit of static metadata to provide completions for builtin-in attributes and lints.
 
 use ide_db::{
     helpers::{
@@ -16,62 +14,107 @@ use ide_db::{
 use itertools::Itertools;
 use once_cell::sync::Lazy;
 use rustc_hash::FxHashMap;
-use syntax::{algo::non_trivia_sibling, ast, AstNode, Direction, SyntaxKind, T};
+use syntax::{
+    ast::{self, AttrKind},
+    AstNode, SyntaxKind, T,
+};
 
-use crate::{context::CompletionContext, item::CompletionItem, Completions};
+use crate::{
+    completions::module_or_attr,
+    context::{CompletionContext, PathCompletionCtx, PathKind, PathQualifierCtx},
+    item::CompletionItem,
+    Completions,
+};
 
 mod cfg;
 mod derive;
 mod lint;
 mod repr;
 
-pub(crate) fn complete_attribute(acc: &mut Completions, ctx: &CompletionContext) -> Option<()> {
+/// Complete inputs to known builtin attributes as well as derive attributes
+pub(crate) fn complete_known_attribute_input(
+    acc: &mut Completions,
+    ctx: &CompletionContext,
+) -> Option<()> {
     let attribute = ctx.fake_attribute_under_caret.as_ref()?;
     let name_ref = match attribute.path() {
         Some(p) => Some(p.as_single_name_ref()?),
         None => None,
     };
-    match (name_ref, attribute.token_tree()) {
-        (Some(path), Some(tt)) if tt.l_paren_token().is_some() => match path.text().as_str() {
-            "repr" => repr::complete_repr(acc, ctx, tt),
-            "derive" => derive::complete_derive(acc, ctx, ctx.attr.as_ref()?),
-            "feature" => lint::complete_lint(acc, ctx, &parse_tt_as_comma_sep_paths(tt)?, FEATURES),
-            "allow" | "warn" | "deny" | "forbid" => {
-                let existing_lints = parse_tt_as_comma_sep_paths(tt)?;
+    let (path, tt) = name_ref.zip(attribute.token_tree())?;
+    if tt.l_paren_token().is_none() {
+        return None;
+    }
 
-                let lints: Vec<Lint> = CLIPPY_LINT_GROUPS
-                    .iter()
-                    .map(|g| &g.lint)
-                    .chain(DEFAULT_LINTS.iter())
-                    .chain(CLIPPY_LINTS.iter())
-                    .chain(RUSTDOC_LINTS)
-                    .cloned()
-                    .collect();
+    match path.text().as_str() {
+        "repr" => repr::complete_repr(acc, ctx, tt),
+        "derive" => derive::complete_derive(acc, ctx, ctx.attr.as_ref()?),
+        "feature" => lint::complete_lint(acc, ctx, &parse_tt_as_comma_sep_paths(tt)?, FEATURES),
+        "allow" | "warn" | "deny" | "forbid" => {
+            let existing_lints = parse_tt_as_comma_sep_paths(tt)?;
 
-                lint::complete_lint(acc, ctx, &existing_lints, &lints);
-            }
-            "cfg" => {
-                cfg::complete_cfg(acc, ctx);
-            }
-            _ => (),
-        },
-        (_, Some(_)) => (),
-        (_, None) if attribute.expr().is_some() => (),
-        (_, None) => complete_new_attribute(acc, ctx, attribute),
+            let lints: Vec<Lint> = CLIPPY_LINT_GROUPS
+                .iter()
+                .map(|g| &g.lint)
+                .chain(DEFAULT_LINTS)
+                .chain(CLIPPY_LINTS)
+                .chain(RUSTDOC_LINTS)
+                .cloned()
+                .collect();
+
+            lint::complete_lint(acc, ctx, &existing_lints, &lints);
+        }
+        "cfg" => {
+            cfg::complete_cfg(acc, ctx);
+        }
+        _ => (),
     }
     Some(())
 }
 
-// FIXME?: Move this functionality to (un)qualified_path, make this module work solely for builtin/known attributes for their inputs?
-fn complete_new_attribute(acc: &mut Completions, ctx: &CompletionContext, attribute: &ast::Attr) {
-    let is_inner = attribute.kind() == ast::AttrKind::Inner;
-    let attribute_annotated_item_kind =
-        attribute.syntax().parent().map(|it| it.kind()).filter(|_| {
-            is_inner
-            // If we got nothing coming after the attribute it could be anything so filter it the kind out
-                || non_trivia_sibling(attribute.syntax().clone().into(), Direction::Next).is_some()
-        });
-    let attributes = attribute_annotated_item_kind.and_then(|kind| {
+pub(crate) fn complete_attribute(acc: &mut Completions, ctx: &CompletionContext) {
+    let (is_absolute_path, qualifier, is_inner, annotated_item_kind) = match ctx.path_context {
+        Some(PathCompletionCtx {
+            kind: Some(PathKind::Attr { kind, annotated_item_kind }),
+            is_absolute_path,
+            ref qualifier,
+            ..
+        }) => (is_absolute_path, qualifier, kind == AttrKind::Inner, annotated_item_kind),
+        _ => return,
+    };
+
+    match qualifier {
+        Some(PathQualifierCtx { resolution, is_super_chain, .. }) => {
+            if *is_super_chain {
+                acc.add_keyword(ctx, "super::");
+            }
+
+            let module = match resolution {
+                Some(hir::PathResolution::Def(hir::ModuleDef::Module(it))) => it,
+                _ => return,
+            };
+
+            for (name, def) in module.scope(ctx.db, ctx.module) {
+                if let Some(def) = module_or_attr(def) {
+                    acc.add_resolution(ctx, name, def);
+                }
+            }
+            return;
+        }
+        // fresh use tree with leading colon2, only show crate roots
+        None if is_absolute_path => acc.add_crate_roots(ctx),
+        // only show modules in a fresh UseTree
+        None => {
+            ctx.process_all_names(&mut |name, def| {
+                if let Some(def) = module_or_attr(def) {
+                    acc.add_resolution(ctx, name, def);
+                }
+            });
+            acc.add_nameref_keywords(ctx);
+        }
+    }
+
+    let attributes = annotated_item_kind.and_then(|kind| {
         if ast::Expr::can_cast(kind) {
             Some(EXPR_ATTRIBUTES)
         } else {

crates/ide_completion/src/completions/dot.rs

@@ -32,7 +32,7 @@ fn complete_undotted_self(acc: &mut Completions, ctx: &CompletionContext) {
     if !ctx.config.enable_self_on_the_fly {
         return;
     }
-    if !ctx.is_trivial_path() || ctx.is_path_disallowed() || !ctx.expects_expression() {
+    if ctx.is_non_trivial_path() || ctx.is_path_disallowed() || !ctx.expects_expression() {
         return;
     }
     if let Some(func) = ctx.function_def.as_ref().and_then(|fn_| ctx.sema.to_def(fn_)) {

crates/ide_completion/src/completions/flyimport.rs

@@ -171,8 +171,8 @@ pub(crate) fn import_on_the_fly(acc: &mut Completions, ctx: &CompletionContext)
             (PathKind::Type, ItemInNs::Types(_)) => true,
             (PathKind::Type, ItemInNs::Values(_)) => false,
 
-            (PathKind::Attr, ItemInNs::Macros(mac)) => mac.is_attr(),
-            (PathKind::Attr, _) => false,
+            (PathKind::Attr { .. }, ItemInNs::Macros(mac)) => mac.is_attr(),
+            (PathKind::Attr { .. }, _) => false,
         }
     };
 

crates/ide_completion/src/completions/keyword.rs

@@ -5,7 +5,7 @@
 use syntax::{SyntaxKind, T};
 
 use crate::{
-    context::{PathCompletionContext, PathKind},
+    context::{PathCompletionCtx, PathKind},
     patterns::ImmediateLocation,
     CompletionContext, CompletionItem, CompletionItemKind, Completions,
 };
@@ -27,18 +27,17 @@ pub(crate) fn complete_expr_keyword(acc: &mut Completions, ctx: &CompletionConte
         cov_mark::hit!(no_keyword_completion_in_non_trivial_path);
         return;
     }
+    if ctx.pattern_ctx.is_some() {
+        return;
+    }
 
     let mut add_keyword = |kw, snippet| add_keyword(acc, ctx, 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 let Some(PathKind::Vis { has_in_token }) = ctx.path_kind() {
-        if !has_in_token {
-            cov_mark::hit!(kw_completion_in);
-            add_keyword("in", "in");
-        }
+    if let Some(PathKind::Vis { .. }) = ctx.path_kind() {
         return;
     }
     if ctx.has_impl_or_trait_prev_sibling() {
@@ -121,14 +120,14 @@ pub(crate) fn complete_expr_keyword(acc: &mut Completions, ctx: &CompletionConte
         add_keyword("else if", "else if $1 {\n    $0\n}");
     }
 
-    if ctx.expects_ident_pat_or_ref_expr() {
+    if ctx.expects_ident_ref_expr() {
         add_keyword("mut", "mut ");
     }
 
     let (can_be_stmt, in_loop_body) = match ctx.path_context {
-        Some(PathCompletionContext {
-            is_trivial_path: true, can_be_stmt, in_loop_body, ..
-        }) => (can_be_stmt, in_loop_body),
+        Some(PathCompletionCtx { is_absolute_path: false, can_be_stmt, in_loop_body, .. }) => {
+            (can_be_stmt, in_loop_body)
+        }
         _ => return,
     };