Merge #11825

11825: fix: Don't complete `Drop::drop` for qualified paths r=Veykril a=Veykril

bors r+

Co-authored-by: Lukas Wirth <lukastw97@gmail.com>

crates/hir_def/src/item_tree.rs

@@ -144,6 +144,37 @@ impl ItemTree {
         Arc::new(item_tree)
     }
 
+    /// Returns an iterator over all items located at the top level of the `HirFileId` this
+    /// `ItemTree` was created from.
+    pub fn top_level_items(&self) -> &[ModItem] {
+        &self.top_level
+    }
+
+    /// Returns the inner attributes of the source file.
+    pub fn top_level_attrs(&self, db: &dyn DefDatabase, krate: CrateId) -> Attrs {
+        self.attrs.get(&AttrOwner::TopLevel).unwrap_or(&RawAttrs::EMPTY).clone().filter(db, krate)
+    }
+
+    pub(crate) fn raw_attrs(&self, of: AttrOwner) -> &RawAttrs {
+        self.attrs.get(&of).unwrap_or(&RawAttrs::EMPTY)
+    }
+
+    pub(crate) fn attrs(&self, db: &dyn DefDatabase, krate: CrateId, of: AttrOwner) -> Attrs {
+        self.raw_attrs(of).clone().filter(db, krate)
+    }
+
+    pub fn pretty_print(&self) -> String {
+        pretty::print_item_tree(self)
+    }
+
+    fn data(&self) -> &ItemTreeData {
+        self.data.as_ref().expect("attempted to access data of empty ItemTree")
+    }
+
+    fn data_mut(&mut self) -> &mut ItemTreeData {
+        self.data.get_or_insert_with(Box::default)
+    }
+
     fn block_item_tree(db: &dyn DefDatabase, block: BlockId) -> Arc<ItemTree> {
         let loc = db.lookup_intern_block(block);
         let block = loc.ast_id.to_node(db.upcast());
@@ -199,37 +230,6 @@ impl ItemTree {
             vis.arena.shrink_to_fit();
         }
     }
-
-    /// Returns an iterator over all items located at the top level of the `HirFileId` this
-    /// `ItemTree` was created from.
-    pub fn top_level_items(&self) -> &[ModItem] {
-        &self.top_level
-    }
-
-    /// Returns the inner attributes of the source file.
-    pub fn top_level_attrs(&self, db: &dyn DefDatabase, krate: CrateId) -> Attrs {
-        self.attrs.get(&AttrOwner::TopLevel).unwrap_or(&RawAttrs::EMPTY).clone().filter(db, krate)
-    }
-
-    pub(crate) fn raw_attrs(&self, of: AttrOwner) -> &RawAttrs {
-        self.attrs.get(&of).unwrap_or(&RawAttrs::EMPTY)
-    }
-
-    pub(crate) fn attrs(&self, db: &dyn DefDatabase, krate: CrateId, of: AttrOwner) -> Attrs {
-        self.raw_attrs(of).clone().filter(db, krate)
-    }
-
-    pub fn pretty_print(&self) -> String {
-        pretty::print_item_tree(self)
-    }
-
-    fn data(&self) -> &ItemTreeData {
-        self.data.as_ref().expect("attempted to access data of empty ItemTree")
-    }
-
-    fn data_mut(&mut self) -> &mut ItemTreeData {
-        self.data.get_or_insert_with(Box::default)
-    }
 }
 
 #[derive(Default, Debug, Eq, PartialEq)]

crates/hir_def/src/nameres/collector.rs

@@ -3,7 +3,7 @@
 //! `DefCollector::collect` contains the fixed-point iteration loop which
 //! resolves imports and expands macros.
 
-use std::iter;
+use std::{iter, mem};
 
 use base_db::{CrateId, Edition, FileId};
 use cfg::{CfgExpr, CfgOptions};
@@ -15,9 +15,9 @@ use hir_expand::{
     builtin_fn_macro::find_builtin_macro,
     name::{name, AsName, Name},
     proc_macro::ProcMacroExpander,
-    ExpandTo, HirFileId, MacroCallId, MacroCallKind, MacroDefId, MacroDefKind,
+    ExpandTo, HirFileId, InFile, MacroCallId, MacroCallKind, MacroCallLoc, MacroDefId,
+    MacroDefKind,
 };
-use hir_expand::{InFile, MacroCallLoc};
 use itertools::Itertools;
 use la_arena::Idx;
 use limit::Limit;
@@ -101,7 +101,6 @@ pub(super) fn collect_defs(db: &dyn DefDatabase, mut def_map: DefMap, tree_id: T
         mod_dirs: FxHashMap::default(),
         cfg_options,
         proc_macros,
-        exports_proc_macros: false,
         from_glob_import: Default::default(),
         skip_attrs: Default::default(),
         derive_helpers_in_scope: Default::default(),
@@ -247,7 +246,6 @@ struct DefCollector<'a> {
     /// them).
     proc_macros: Vec<(Name, ProcMacroExpander)>,
     is_proc_macro: bool,
-    exports_proc_macros: bool,
     from_glob_import: PerNsGlobImports,
     /// If we fail to resolve an attribute on a `ModItem`, we fall back to ignoring the attribute.
     /// This map is used to skip all attributes up to and including the one that failed to resolve,
@@ -403,10 +401,10 @@ impl DefCollector<'_> {
         self.unresolved_imports.extend(partial_resolved);
         self.resolve_imports();
 
-        let unresolved_imports = std::mem::take(&mut self.unresolved_imports);
+        let unresolved_imports = mem::take(&mut self.unresolved_imports);
         // show unresolved imports in completion, etc
         for directive in &unresolved_imports {
-            self.record_resolved_import(directive)
+            self.record_resolved_import(directive);
         }
         self.unresolved_imports = unresolved_imports;
 
@@ -435,7 +433,7 @@ impl DefCollector<'_> {
     fn reseed_with_unresolved_attribute(&mut self) -> ReachedFixedPoint {
         cov_mark::hit!(unresolved_attribute_fallback);
 
-        let mut unresolved_macros = std::mem::take(&mut self.unresolved_macros);
+        let mut unresolved_macros = mem::take(&mut self.unresolved_macros);
         let pos = unresolved_macros.iter().position(|directive| {
             if let MacroDirectiveKind::Attr { ast_id, mod_item, attr, tree } = &directive.kind {
                 self.skip_attrs.insert(ast_id.ast_id.with_value(*mod_item), attr.id);
@@ -458,10 +456,9 @@ impl DefCollector<'_> {
         });
 
         if let Some(pos) = pos {
-            unresolved_macros.remove(pos);
+            unresolved_macros.swap_remove(pos);
         }
 
-        // The collection above might add new unresolved macros (eg. derives), so merge the lists.
         self.unresolved_macros.extend(unresolved_macros);
 
         if pos.is_some() {
@@ -558,8 +555,6 @@ impl DefCollector<'_> {
         fn_id: FunctionId,
         module_id: ModuleId,
     ) {
-        self.exports_proc_macros = true;
-
         let kind = def.kind.to_basedb_kind();
         let (expander, kind) = match self.proc_macros.iter().find(|(n, _)| n == &def.name) {
             Some(&(_, expander)) => (expander, kind),
@@ -714,7 +709,7 @@ impl DefCollector<'_> {
     /// Tries to resolve every currently unresolved import.
     fn resolve_imports(&mut self) -> ReachedFixedPoint {
         let mut res = ReachedFixedPoint::Yes;
-        let imports = std::mem::take(&mut self.unresolved_imports);
+        let imports = mem::take(&mut self.unresolved_imports);
         let imports = imports
             .into_iter()
             .filter_map(|mut directive| {
@@ -1051,7 +1046,7 @@ impl DefCollector<'_> {
     }
 
     fn resolve_macros(&mut self) -> ReachedFixedPoint {
-        let mut macros = std::mem::take(&mut self.unresolved_macros);
+        let mut macros = mem::take(&mut self.unresolved_macros);
         let mut resolved = Vec::new();
         let mut push_resolved = |directive: &MacroDirective, call_id| {
             resolved.push((directive.module_id, directive.depth, directive.container, call_id));
@@ -2095,7 +2090,6 @@ mod tests {
             mod_dirs: FxHashMap::default(),
             cfg_options: &CfgOptions::default(),
             proc_macros: Default::default(),
-            exports_proc_macros: false,
             from_glob_import: Default::default(),
             skip_attrs: Default::default(),
             derive_helpers_in_scope: Default::default(),

crates/ide/src/runnables.rs

@@ -544,7 +544,8 @@ mod tests {
         expect: Expect,
     ) {
         let (analysis, position) = fixture::position(ra_fixture);
-        let runnables = analysis.runnables(position.file_id).unwrap();
+        let mut runnables = analysis.runnables(position.file_id).unwrap();
+        runnables.sort_by_key(|it| (it.nav.full_range.start(), it.nav.name.clone()));
         expect.assert_debug_eq(&runnables);
         assert_eq!(
             actions,
@@ -580,9 +581,24 @@ mod not_a_root {
     fn main() {}
 }
 "#,
-            &[Bin, Test, Test, Bench, TestMod],
+            &[TestMod, Bin, Test, Test, Bench],
             expect![[r#"
                 [
+                    Runnable {
+                        use_name_in_title: false,
+                        nav: NavigationTarget {
+                            file_id: FileId(
+                                0,
+                            ),
+                            full_range: 0..137,
+                            name: "",
+                            kind: Module,
+                        },
+                        kind: TestMod {
+                            path: "",
+                        },
+                        cfg: None,
+                    },
                     Runnable {
                         use_name_in_title: false,
                         nav: NavigationTarget {
@@ -657,21 +673,6 @@ mod not_a_root {
                         },
                         cfg: None,
                     },
-                    Runnable {
-                        use_name_in_title: false,
-                        nav: NavigationTarget {
-                            file_id: FileId(
-                                0,
-                            ),
-                            full_range: 0..137,
-                            name: "",
-                            kind: Module,
-                        },
-                        kind: TestMod {
-                            path: "",
-                        },
-                        cfg: None,
-                    },
                 ]
             "#]],
         );
@@ -1065,7 +1066,7 @@ mod root_tests {
     mod nested_tests_4 {}
 }
 "#,
-            &[TestMod, TestMod, TestMod, Test, Test, Test],
+            &[TestMod, TestMod, Test, Test, TestMod, Test],
             expect![[r#"
                 [
                     Runnable {
@@ -1102,23 +1103,6 @@ mod root_tests {
                         },
                         cfg: None,
                     },
-                    Runnable {
-                        use_name_in_title: false,
-                        nav: NavigationTarget {
-                            file_id: FileId(
-                                0,
-                            ),
-                            full_range: 202..286,
-                            focus_range: 206..220,
-                            name: "nested_tests_2",
-                            kind: Module,
-                            description: "mod nested_tests_2",
-                        },
-                        kind: TestMod {
-                            path: "root_tests::nested_tests_0::nested_tests_2",
-                        },
-                        cfg: None,
-                    },
                     Runnable {
                         use_name_in_title: false,
                         nav: NavigationTarget {
@@ -1161,6 +1145,23 @@ mod root_tests {
                         },
                         cfg: None,
                     },
+                    Runnable {
+                        use_name_in_title: false,
+                        nav: NavigationTarget {
+                            file_id: FileId(
+                                0,
+                            ),
+                            full_range: 202..286,
+                            focus_range: 206..220,
+                            name: "nested_tests_2",
+                            kind: Module,
+                            description: "mod nested_tests_2",
+                        },
+                        kind: TestMod {
+                            path: "root_tests::nested_tests_0::nested_tests_2",
+                        },
+                        cfg: None,
+                    },
                     Runnable {
                         use_name_in_title: false,
                         nav: NavigationTarget {
@@ -1197,9 +1198,24 @@ $0
 #[cfg(feature = "foo")]
 fn test_foo1() {}
 "#,
-            &[Test, TestMod],
+            &[TestMod, Test],
             expect![[r#"
                 [
+                    Runnable {
+                        use_name_in_title: false,
+                        nav: NavigationTarget {
+                            file_id: FileId(
+                                0,
+                            ),
+                            full_range: 0..51,
+                            name: "",
+                            kind: Module,
+                        },
+                        kind: TestMod {
+                            path: "",
+                        },
+                        cfg: None,
+                    },
                     Runnable {
                         use_name_in_title: false,
                         nav: NavigationTarget {
@@ -1228,21 +1244,6 @@ fn test_foo1() {}
                             ),
                         ),
                     },
-                    Runnable {
-                        use_name_in_title: false,
-                        nav: NavigationTarget {
-                            file_id: FileId(
-                                0,
-                            ),
-                            full_range: 0..51,
-                            name: "",
-                            kind: Module,
-                        },
-                        kind: TestMod {
-                            path: "",
-                        },
-                        cfg: None,
-                    },
                 ]
             "#]],
         );
@@ -1258,9 +1259,24 @@ $0
 #[cfg(all(feature = "foo", feature = "bar"))]
 fn test_foo1() {}
 "#,
-            &[Test, TestMod],
+            &[TestMod, Test],
             expect![[r#"
                 [
+                    Runnable {
+                        use_name_in_title: false,
+                        nav: NavigationTarget {
+                            file_id: FileId(
+                                0,
+                            ),
+                            full_range: 0..73,
+                            name: "",
+                            kind: Module,
+                        },
+                        kind: TestMod {
+                            path: "",
+                        },
+                        cfg: None,
+                    },
                     Runnable {
                         use_name_in_title: false,
                         nav: NavigationTarget {
@@ -1299,21 +1315,6 @@ fn test_foo1() {}
                             ),
                         ),
                     },
-                    Runnable {
-                        use_name_in_title: false,
-                        nav: NavigationTarget {
-                            file_id: FileId(
-                                0,
-                            ),
-                            full_range: 0..73,
-                            name: "",
-                            kind: Module,
-                        },
-                        kind: TestMod {
-                            path: "",
-                        },
-                        cfg: None,
-                    },
                 ]
             "#]],
         );
@@ -1400,9 +1401,24 @@ mod tests {
 }
 gen2!();
 "#,
-            &[TestMod, TestMod, TestMod, Test, Test],
+            &[TestMod, TestMod, Test, Test, TestMod],
             expect![[r#"
                 [
+                    Runnable {
+                        use_name_in_title: false,
+                        nav: NavigationTarget {
+                            file_id: FileId(
+                                0,
+                            ),
+                            full_range: 0..237,
+                            name: "",
+                            kind: Module,
+                        },
+                        kind: TestMod {
+                            path: "",
+                        },
+                        cfg: None,
+                    },
                     Runnable {
                         use_name_in_title: false,
                         nav: NavigationTarget {
@@ -1426,28 +1442,17 @@ gen2!();
                             file_id: FileId(
                                 0,
                             ),
-                            full_range: 0..237,
+                            full_range: 218..225,
-                            name: "",
+                            name: "foo_test",
-                            kind: Module,
+                            kind: Function,
                         },
-                        kind: TestMod {
+                        kind: Test {
-                            path: "",
+                            test_id: Path(
-                        },
+                                "tests::foo_test",
-                        cfg: None,
-                    },
-                    Runnable {
-                        use_name_in_title: true,
-                        nav: NavigationTarget {
-                            file_id: FileId(
-                                0,
                             ),
-                            full_range: 228..236,
+                            attr: TestAttr {
-                            name: "tests2",
+                                ignore: false,
-                            kind: Module,
-                            description: "mod tests2",
                             },
-                        kind: TestMod {
-                            path: "tests2",
                         },
                         cfg: None,
                     },
@@ -1472,22 +1477,18 @@ gen2!();
                         cfg: None,
                     },
                     Runnable {
-                        use_name_in_title: false,
+                        use_name_in_title: true,
                         nav: NavigationTarget {
                             file_id: FileId(
                                 0,
                             ),
-                            full_range: 218..225,
+                            full_range: 228..236,
-                            name: "foo_test",
+                            name: "tests2",
-                            kind: Function,
+                            kind: Module,
-                        },
+                            description: "mod tests2",
-                        kind: Test {
-                            test_id: Path(
-                                "tests::foo_test",
-                            ),
-                            attr: TestAttr {
-                                ignore: false,
                         },
+                        kind: TestMod {
+                            path: "tests2",
                         },
                         cfg: None,
                     },
@@ -1516,25 +1517,9 @@ macro_rules! foo {
 }
 foo!();
 "#,
-            &[TestMod, Test, Test, Test],
+            &[Test, Test, Test, TestMod],
             expect![[r#"
                 [
-                    Runnable {
-                        use_name_in_title: true,
-                        nav: NavigationTarget {
-                            file_id: FileId(
-                                0,
-                            ),
-                            full_range: 210..217,
-                            name: "foo_tests",
-                            kind: Module,
-                            description: "mod foo_tests",
-                        },
-                        kind: TestMod {
-                            path: "foo_tests",
-                        },
-                        cfg: None,
-                    },
                     Runnable {
                         use_name_in_title: true,
                         nav: NavigationTarget {
@@ -1595,6 +1580,22 @@ foo!();
                         },
                         cfg: None,
                     },
+                    Runnable {
+                        use_name_in_title: true,
+                        nav: NavigationTarget {
+                            file_id: FileId(
+                                0,
+                            ),
+                            full_range: 210..217,
+                            name: "foo_tests",
+                            kind: Module,
+                            description: "mod foo_tests",
+                        },
+                        kind: TestMod {
+                            path: "foo_tests",
+                        },
+                        cfg: None,
+                    },
                 ]
             "#]],
         );
@@ -1671,9 +1672,24 @@ fn t0() {}
 #[test]
 fn t1() {}
 "#,
-            &[Test, Test, TestMod],
+            &[TestMod, Test, Test],
             expect![[r#"
                 [
+                    Runnable {
+                        use_name_in_title: false,
+                        nav: NavigationTarget {
+                            file_id: FileId(
+                                1,
+                            ),
+                            full_range: 0..39,
+                            name: "m",
+                            kind: Module,
+                        },
+                        kind: TestMod {
+                            path: "m",
+                        },
+                        cfg: None,
+                    },
                     Runnable {
                         use_name_in_title: false,
                         nav: NavigationTarget {
@@ -1716,21 +1732,6 @@ fn t1() {}
                         },
                         cfg: None,
                     },
-                    Runnable {
-                        use_name_in_title: false,
-                        nav: NavigationTarget {
-                            file_id: FileId(
-                                1,
-                            ),
-                            full_range: 0..39,
-                            name: "m",
-                            kind: Module,
-                        },
-                        kind: TestMod {
-                            path: "m",
-                        },
-                        cfg: None,
-                    },
                 ]
             "#]],
         );

crates/ide_completion/src/completions/qualified_path.rs

@@ -1,6 +1,7 @@
 //! Completion of paths, i.e. `some::prefix::$0`.
 
 use hir::{ScopeDef, Trait};
+use ide_db::famous_defs::FamousDefs;
 use rustc_hash::FxHashSet;
 use syntax::ast;
 
@@ -23,6 +24,13 @@ pub(crate) fn complete_qualified_path(acc: &mut Completions, ctx: &CompletionCon
         Some(PathCompletionCtx { qualifier: Some(ref qualifier), kind, .. }) => (qualifier, kind),
         _ => return,
     };
+    let traits_in_scope = |ctx: &CompletionContext| {
+        let mut traits_in_scope = ctx.scope.visible_traits();
+        if let Some(drop) = FamousDefs(&ctx.sema, ctx.krate).core_ops_Drop() {
+            traits_in_scope.remove(&drop.into());
+        }
+        traits_in_scope
+    };
 
     // special case `<_>::$0` as this doesn't resolve to anything.
     if qualifier.path.qualifier().is_none() {
@@ -34,8 +42,7 @@ pub(crate) fn complete_qualified_path(acc: &mut Completions, ctx: &CompletionCon
             })
         ) {
             cov_mark::hit!(completion_type_anchor_empty);
-            ctx.scope
+            traits_in_scope(ctx)
-                .visible_traits()
                 .into_iter()
                 .flat_map(|it| Trait::from(it).items(ctx.sema.db))
                 .for_each(|item| add_assoc_item(acc, ctx, item));
@@ -141,7 +148,7 @@ pub(crate) fn complete_qualified_path(acc: &mut Completions, ctx: &CompletionCon
 
             let krate = ctx.krate;
             if let Some(krate) = krate {
-                let traits_in_scope = ctx.scope.visible_traits();
+                let traits_in_scope = traits_in_scope(ctx);
                 ty.iterate_path_candidates(
                     ctx.db,
                     &ctx.scope,
@@ -179,8 +186,7 @@ pub(crate) fn complete_qualified_path(acc: &mut Completions, ctx: &CompletionCon
             if let Some(hir::Adt::Enum(e)) = ty.as_adt() {
                 add_enum_variants(acc, ctx, e);
             }
-
+            let traits_in_scope = traits_in_scope(ctx);
-            let traits_in_scope = ctx.scope.visible_traits();
             let mut seen = FxHashSet::default();
             ty.iterate_path_candidates(
                 ctx.db,

crates/ide_completion/src/tests/expression.rs

@@ -553,6 +553,23 @@ fn func() {
     );
 }
 
+#[test]
+fn ty_qualified_no_drop() {
+    check_empty(
+        r#"
+//- minicore: drop
+struct Foo;
+impl Drop for Foo {
+    fn drop(&mut self) {}
+}
+fn func() {
+    Foo::$0
+}
+"#,
+        expect![[r#""#]],
+    );
+}
+
 #[test]
 fn with_parens() {
     check_empty(