Cleanup runnables canonical path impl

Author: Veykril

crates/hir/src/lib.rs

@@ -295,12 +295,12 @@ impl ModuleDef {
     }
 
     pub fn canonical_path(&self, db: &dyn HirDatabase) -> Option<String> {
-        let mut segments = vec![self.name(db)?.to_string()];
+        let mut segments = vec![self.name(db)?];
         for m in self.module(db)?.path_to_root(db) {
-            segments.extend(m.name(db).map(|it| it.to_string()))
+            segments.extend(m.name(db))
         }
         segments.reverse();
-        Some(segments.join("::"))
+        Some(segments.into_iter().join("::"))
     }
 
     pub fn name(self, db: &dyn HirDatabase) -> Option<Name> {

crates/ide/src/runnables.rs

@@ -157,7 +157,7 @@ pub(crate) fn runnables(db: &RootDatabase, file_id: FileId) -> Vec<Runnable> {
                 hir::ModuleDef::Function(it) => runnable_fn(&sema, it),
                 _ => None,
             };
-            add_opt(runnable.or_else(|| module_def_doctest(&sema, def)), Some(def));
+            add_opt(runnable.or_else(|| module_def_doctest(sema.db, def)), Some(def));
         }
         Either::Right(impl_) => {
             add_opt(runnable_impl(&sema, &impl_), None);
@@ -168,9 +168,9 @@ pub(crate) fn runnables(db: &RootDatabase, file_id: FileId) -> Vec<Runnable> {
                     (
                         match assoc {
                             hir::AssocItem::Function(it) => runnable_fn(&sema, it)
-                                .or_else(|| module_def_doctest(&sema, it.into())),
-                            hir::AssocItem::Const(it) => module_def_doctest(&sema, it.into()),
-                            hir::AssocItem::TypeAlias(it) => module_def_doctest(&sema, it.into()),
+                                .or_else(|| module_def_doctest(sema.db, it.into())),
+                            hir::AssocItem::Const(it) => module_def_doctest(sema.db, it.into()),
+                            hir::AssocItem::TypeAlias(it) => module_def_doctest(sema.db, it.into()),
                         },
                         assoc,
                     )
@@ -382,61 +382,59 @@ fn runnable_mod_outline_definition(
     }
 }
 
-fn module_def_doctest(sema: &Semantics<RootDatabase>, def: hir::ModuleDef) -> Option<Runnable> {
+fn module_def_doctest(db: &RootDatabase, def: hir::ModuleDef) -> Option<Runnable> {
     let attrs = match def {
-        hir::ModuleDef::Module(it) => it.attrs(sema.db),
-        hir::ModuleDef::Function(it) => it.attrs(sema.db),
-        hir::ModuleDef::Adt(it) => it.attrs(sema.db),
-        hir::ModuleDef::Variant(it) => it.attrs(sema.db),
-        hir::ModuleDef::Const(it) => it.attrs(sema.db),
-        hir::ModuleDef::Static(it) => it.attrs(sema.db),
-        hir::ModuleDef::Trait(it) => it.attrs(sema.db),
-        hir::ModuleDef::TypeAlias(it) => it.attrs(sema.db),
+        hir::ModuleDef::Module(it) => it.attrs(db),
+        hir::ModuleDef::Function(it) => it.attrs(db),
+        hir::ModuleDef::Adt(it) => it.attrs(db),
+        hir::ModuleDef::Variant(it) => it.attrs(db),
+        hir::ModuleDef::Const(it) => it.attrs(db),
+        hir::ModuleDef::Static(it) => it.attrs(db),
+        hir::ModuleDef::Trait(it) => it.attrs(db),
+        hir::ModuleDef::TypeAlias(it) => it.attrs(db),
         hir::ModuleDef::BuiltinType(_) => return None,
     };
     if !has_runnable_doc_test(&attrs) {
         return None;
     }
-    let def_name = def.name(sema.db).map(|it| it.to_string());
-    let test_id = def
-        .canonical_path(sema.db)
-        // This probably belongs to canonical path?
-        .map(|path| {
-            let assoc_def = match def {
-                hir::ModuleDef::Function(it) => it.as_assoc_item(sema.db),
-                hir::ModuleDef::Const(it) => it.as_assoc_item(sema.db),
-                hir::ModuleDef::TypeAlias(it) => it.as_assoc_item(sema.db),
-                _ => None,
-            };
-            // FIXME: this also looks very wrong
-            if let Some(assoc_def) = assoc_def {
-                if let hir::AssocItemContainer::Impl(imp) = assoc_def.container(sema.db) {
-                    let ty = imp.self_ty(sema.db);
-                    if let Some(adt) = ty.as_adt() {
-                        let name = adt.name(sema.db);
-                        let idx = path.rfind(':').map_or(0, |idx| idx + 1);
-                        let (prefix, suffix) = path.split_at(idx);
-                        let mut ty_args = ty.type_arguments().peekable();
-                        let params = if ty_args.peek().is_some() {
-                            format!(
-                                "<{}>",
-                                ty_args.format_with(", ", |ty, cb| cb(&ty.display(sema.db)))
-                            )
-                        } else {
-                            String::new()
-                        };
-                        return format!("{}{}{}::{}", prefix, name, params, suffix);
+    let def_name = def.name(db)?;
+    let path = (|| {
+        let mut path = String::new();
+        def.module(db)?
+            .path_to_root(db)
+            .into_iter()
+            .rev()
+            .flat_map(|it| it.name(db))
+            .for_each(|name| format_to!(path, "{}::", name));
+        // This probably belongs to canonical_path?
+        if let Some(assoc_item) = def.as_assoc_item(db) {
+            if let hir::AssocItemContainer::Impl(imp) = assoc_item.container(db) {
+                let ty = imp.self_ty(db);
+                if let Some(adt) = ty.as_adt() {
+                    let name = adt.name(db);
+                    let mut ty_args = ty.type_arguments().peekable();
+                    format_to!(path, "{}", name);
+                    if ty_args.peek().is_some() {
+                        format_to!(
+                            path,
+                            "<{}>",
+                            ty_args.format_with(", ", |ty, cb| cb(&ty.display(db)))
+                        );
                     }
+                    format_to!(path, "::{}", def_name);
+                    return Some(path);
                 }
             }
-            path
-        })
-        .map(TestId::Path)
-        .or_else(|| def_name.clone().map(TestId::Name))?;
+        }
+        format_to!(path, "{}", def_name);
+        Some(path)
+    })();
+
+    let test_id = path.map_or_else(|| TestId::Name(def_name.to_string()), TestId::Path);
 
     let mut nav = match def {
-        hir::ModuleDef::Module(def) => NavigationTarget::from_module_to_decl(sema.db, def),
-        def => def.try_to_nav(sema.db)?,
+        hir::ModuleDef::Module(def) => NavigationTarget::from_module_to_decl(db, def),
+        def => def.try_to_nav(db)?,
     };
     nav.focus_range = None;
     nav.description = None;