Auto merge of rust-lang#12735 - feniljain:fix-completions, r=feniljain

fix(completion): `super::` completion at crate root and module depth aware - should close rust-lang#12439 - Suggest super according to the depth in tree
rust-lang-ci · Jul 14, 2022 · 5af3ef5 · 5af3ef5
2 parents 794ecd5 + 4cbde4e
commit 5af3ef5
Show file tree

Hide file tree

Showing 19 changed files with 140 additions and 112 deletions.
diff --git a/crates/ide-completion/src/completions.rs b/crates/ide-completion/src/completions.rs
@@ -91,11 +91,31 @@ impl Completions {
     }
 
     pub(crate) fn add_nameref_keywords_with_colon(&mut self, ctx: &CompletionContext) {
-        ["self::", "super::", "crate::"].into_iter().for_each(|kw| self.add_keyword(ctx, kw));
+        ["self::", "crate::"].into_iter().for_each(|kw| self.add_keyword(ctx, kw));
+
+        if ctx.depth_from_crate_root > 0 {
+            self.add_keyword(ctx, "super::");
+        }
     }
 
     pub(crate) fn add_nameref_keywords(&mut self, ctx: &CompletionContext) {
-        ["self", "super", "crate"].into_iter().for_each(|kw| self.add_keyword(ctx, kw));
+        ["self", "crate"].into_iter().for_each(|kw| self.add_keyword(ctx, kw));
+
+        if ctx.depth_from_crate_root > 0 {
+            self.add_keyword(ctx, "super");
+        }
+    }
+
+    pub(crate) fn add_super_keyword(
+        &mut self,
+        ctx: &CompletionContext,
+        super_chain_len: Option<usize>,
+    ) {
+        if let Some(len) = super_chain_len {
+            if len > 0 && len < ctx.depth_from_crate_root {
+                self.add_keyword(ctx, "super::");
+            }
+        }
     }
 
     pub(crate) fn add_keyword_snippet_expr(

diff --git a/crates/ide-completion/src/completions/attribute.rs b/crates/ide-completion/src/completions/attribute.rs
@@ -82,12 +82,10 @@ pub(crate) fn complete_attribute_path(
     match qualified {
         Qualified::With {
             resolution: Some(hir::PathResolution::Def(hir::ModuleDef::Module(module))),
-            is_super_chain,
+            super_chain_len,
             ..
         } => {
-            if *is_super_chain {
-                acc.add_keyword(ctx, "super::");
-            }
+            acc.add_super_keyword(ctx, *super_chain_len);
 
             for (name, def) in module.scope(ctx.db, Some(ctx.module)) {
                 match def {

diff --git a/crates/ide-completion/src/completions/attribute/derive.rs b/crates/ide-completion/src/completions/attribute/derive.rs
@@ -21,12 +21,10 @@ pub(crate) fn complete_derive_path(
     match qualified {
         Qualified::With {
             resolution: Some(hir::PathResolution::Def(hir::ModuleDef::Module(module))),
-            is_super_chain,
+            super_chain_len,
             ..
         } => {
-            if *is_super_chain {
-                acc.add_keyword(ctx, "super::");
-            }
+            acc.add_super_keyword(ctx, *super_chain_len);
 
             for (name, def) in module.scope(ctx.db, Some(ctx.module)) {
                 match def {

diff --git a/crates/ide-completion/src/completions/item_list.rs b/crates/ide-completion/src/completions/item_list.rs
@@ -36,7 +36,7 @@ pub(crate) fn complete_item_list(
     match qualified {
         Qualified::With {
             resolution: Some(hir::PathResolution::Def(hir::ModuleDef::Module(module))),
-            is_super_chain,
+            super_chain_len,
             ..
         } => {
             for (name, def) in module.scope(ctx.db, Some(ctx.module)) {
@@ -51,9 +51,7 @@ pub(crate) fn complete_item_list(
                 }
             }
 
-            if *is_super_chain {
-                acc.add_keyword(ctx, "super::");
-            }
+            acc.add_super_keyword(ctx, *super_chain_len);
         }
         Qualified::Absolute => acc.add_crate_roots(ctx, path_ctx),
         Qualified::No if ctx.qualifier_ctx.none() => {

diff --git a/crates/ide-completion/src/completions/pattern.rs b/crates/ide-completion/src/completions/pattern.rs
@@ -111,10 +111,8 @@ pub(crate) fn complete_pattern_path(
     path_ctx @ PathCompletionCtx { qualified, .. }: &PathCompletionCtx,
 ) {
     match qualified {
-        Qualified::With { resolution: Some(resolution), is_super_chain, .. } => {
-            if *is_super_chain {
-                acc.add_keyword(ctx, "super::");
-            }
+        Qualified::With { resolution: Some(resolution), super_chain_len, .. } => {
+            acc.add_super_keyword(ctx, *super_chain_len);
 
             match resolution {
                 hir::PathResolution::Def(hir::ModuleDef::Module(module)) => {

diff --git a/crates/ide-completion/src/completions/use_.rs b/crates/ide-completion/src/completions/use_.rs
@@ -17,10 +17,9 @@ pub(crate) fn complete_use_path(
     name_ref: &Option<ast::NameRef>,
 ) {
     match qualified {
-        Qualified::With { path, resolution: Some(resolution), is_super_chain } => {
-            if *is_super_chain {
-                acc.add_keyword(ctx, "super::");
-            }
+        Qualified::With { path, resolution: Some(resolution), super_chain_len } => {
+            acc.add_super_keyword(ctx, *super_chain_len);
+
             // only show `self` in a new use-tree when the qualifier doesn't end in self
             let not_preceded_by_self = *use_tree_parent
                 && !matches!(

diff --git a/crates/ide-completion/src/completions/vis.rs b/crates/ide-completion/src/completions/vis.rs
@@ -14,7 +14,7 @@ pub(crate) fn complete_vis_path(
     match qualified {
         Qualified::With {
             resolution: Some(hir::PathResolution::Def(hir::ModuleDef::Module(module))),
-            is_super_chain,
+            super_chain_len,
             ..
         } => {
             // Try completing next child module of the path that is still a parent of the current module
@@ -27,9 +27,7 @@ pub(crate) fn complete_vis_path(
                 }
             }
 
-            if *is_super_chain {
-                acc.add_keyword(ctx, "super::");
-            }
+            acc.add_super_keyword(ctx, *super_chain_len);
         }
         Qualified::Absolute | Qualified::Infer | Qualified::With { .. } => {}
         Qualified::No => {

diff --git a/crates/ide-completion/src/context.rs b/crates/ide-completion/src/context.rs
@@ -4,6 +4,8 @@ mod analysis;
 #[cfg(test)]
 mod tests;
 
+use std::iter;
+
 use base_db::SourceDatabaseExt;
 use hir::{
     HasAttrs, Local, Name, PathResolution, ScopeDef, Semantics, SemanticsScope, Type, TypeInfo,
@@ -174,8 +176,17 @@ pub(super) enum Qualified {
     With {
         path: ast::Path,
         resolution: Option<PathResolution>,
-        /// Whether this path consists solely of `super` segments
-        is_super_chain: bool,
+        /// How many `super` segments are present in the path
+        ///
+        /// This would be None, if path is not solely made of
+        /// `super` segments, e.g.
+        ///
+        /// ```rust
+        ///   use super::foo;
+        /// ```
+        ///
+        /// Otherwise it should be Some(count of `super`)
+        super_chain_len: Option<usize>,
     },
     /// <_>::
     Infer,
@@ -343,6 +354,12 @@ pub(crate) struct CompletionContext<'a> {
     pub(super) qualifier_ctx: QualifierCtx,
 
     pub(super) locals: FxHashMap<Name, Local>,
+
+    /// - crate-root
+    ///  - mod foo
+    ///   - mod bar
+    /// Here depth will be 2: {[bar<->foo], [foo<->crate-root]}
+    pub(super) depth_from_crate_root: usize,
 }
 
 impl<'a> CompletionContext<'a> {
@@ -521,6 +538,8 @@ impl<'a> CompletionContext<'a> {
             }
         });
 
+        let depth_from_crate_root = iter::successors(module.parent(db), |m| m.parent(db)).count();
+
         let mut ctx = CompletionContext {
             sema,
             scope,
@@ -535,6 +554,7 @@ impl<'a> CompletionContext<'a> {
             expected_type: None,
             qualifier_ctx: Default::default(),
             locals,
+            depth_from_crate_root,
         };
         let ident_ctx = ctx.expand_and_analyze(
             original_file.syntax().clone(),

diff --git a/crates/ide-completion/src/context/analysis.rs b/crates/ide-completion/src/context/analysis.rs
@@ -899,10 +899,25 @@ impl<'a> CompletionContext<'a> {
                         Qualified::Infer
                     } else {
                         let res = sema.resolve_path(&path);
-                        let is_super_chain =
-                            iter::successors(Some(path.clone()), |p| p.qualifier())
-                                .all(|p| p.segment().and_then(|s| s.super_token()).is_some());
-                        Qualified::With { path, resolution: res, is_super_chain }
+
+                        // For understanding how and why super_chain_len is calculated the way it
+                        // is check the documentation at it's definition
+                        let mut segment_count = 0;
+                        let super_count = iter::successors(Some(path.clone()), |p| p.qualifier())
+                            .take_while(|p| {
+                                p.segment()
+                                    .and_then(|s| {
+                                        segment_count += 1;
+                                        s.super_token()
+                                    })
+                                    .is_some()
+                            })
+                            .count();
+
+                        let super_chain_len =
+                            if segment_count > super_count { None } else { Some(super_count) };
+
+                        Qualified::With { path, resolution: res, super_chain_len }
                     }
                 };
             }