Merge #6919

6919: Rewrite doctest runnables r=matklad a=matklad

bors r+
🤖

Co-authored-by: Aleksey Kladov <aleksey.kladov@gmail.com>

crates/ide/src/display/navigation_target.rs

@@ -117,25 +117,6 @@ impl NavigationTarget {
         )
     }
 
-    /// Allows `NavigationTarget` to be created from a `DocCommentsOwner` and a `NameOwner`
-    pub(crate) fn from_doc_commented(
-        db: &RootDatabase,
-        named: InFile<&dyn ast::NameOwner>,
-        node: InFile<&dyn ast::DocCommentsOwner>,
-    ) -> NavigationTarget {
-        let name =
-            named.value.name().map(|it| it.text().clone()).unwrap_or_else(|| SmolStr::new("_"));
-        let frange = node.map(|it| it.syntax()).original_file_range(db);
-
-        NavigationTarget::from_syntax(
-            frange.file_id,
-            name,
-            None,
-            frange.range,
-            node.value.syntax().kind(),
-        )
-    }
-
     fn from_syntax(
         file_id: FileId,
         name: SmolStr,

crates/ide/src/runnables.rs

@@ -10,7 +10,10 @@ use syntax::{
     match_ast, SyntaxNode,
 };
 
-use crate::{display::ToNav, FileId, NavigationTarget};
+use crate::{
+    display::{ToNav, TryToNav},
+    FileId, NavigationTarget,
+};
 
 #[derive(Debug, Clone)]
 pub struct Runnable {
@@ -101,125 +104,109 @@ pub(crate) fn runnable(
     item: SyntaxNode,
     file_id: FileId,
 ) -> Option<Runnable> {
-    match_ast! {
+    let runnable_item = match_ast! {
-        match item {
+        match (item.clone()) {
-            ast::Struct(it) => runnable_struct(sema, it, file_id),
             ast::Fn(it) => runnable_fn(sema, it, file_id),
             ast::Module(it) => runnable_mod(sema, it),
             _ => None,
         }
-    }
+    };
+    runnable_item.or_else(|| runnable_doctest(sema, item))
 }
 
-fn runnable_fn(
+fn runnable_fn(sema: &Semantics<RootDatabase>, func: ast::Fn, file_id: FileId) -> Option<Runnable> {
-    sema: &Semantics<RootDatabase>,
+    let def = sema.to_def(&func)?;
-    fn_def: ast::Fn,
+    let name_string = func.name()?.text().to_string();
-    file_id: FileId,
-) -> Option<Runnable> {
-    let def = sema.to_def(&fn_def)?;
-    let name_string = fn_def.name()?.text().to_string();
 
-    let attrs = def.attrs(sema.db);
     let kind = if name_string == "main" {
         RunnableKind::Bin
     } else {
-        let test_id = match sema.to_def(&fn_def).map(|def| def.module(sema.db)) {
+        let canonical_path = sema.to_def(&func).and_then(|def| {
-            Some(module) => {
+            let def: hir::ModuleDef = def.into();
-                let def = sema.to_def(&fn_def)?;
+            def.canonical_path(sema.db)
-                let impl_trait_name = def.as_assoc_item(sema.db).and_then(|assoc_item| {
+        });
-                    match assoc_item.container(sema.db) {
+        let test_id = canonical_path.map(TestId::Path).unwrap_or(TestId::Name(name_string));
-                        hir::AssocItemContainer::Trait(trait_item) => {
-                            Some(trait_item.name(sema.db).to_string())
-                        }
-                        hir::AssocItemContainer::Impl(impl_def) => impl_def
-                            .target_ty(sema.db)
-                            .as_adt()
-                            .map(|adt| adt.name(sema.db).to_string()),
-                    }
-                });
 
-                let path_iter = module
+        if test_related_attribute(&func).is_some() {
-                    .path_to_root(sema.db)
+            let attr = TestAttr::from_fn(&func);
-                    .into_iter()
-                    .rev()
-                    .filter_map(|it| it.name(sema.db))
-                    .map(|name| name.to_string());
-
-                let path = if let Some(impl_trait_name) = impl_trait_name {
-                    path_iter
-                        .chain(std::iter::once(impl_trait_name))
-                        .chain(std::iter::once(name_string))
-                        .join("::")
-                } else {
-                    path_iter.chain(std::iter::once(name_string)).join("::")
-                };
-
-                TestId::Path(path)
-            }
-            None => TestId::Name(name_string),
-        };
-
-        if test_related_attribute(&fn_def).is_some() {
-            let attr = TestAttr::from_fn(&fn_def);
             RunnableKind::Test { test_id, attr }
-        } else if fn_def.has_atom_attr("bench") {
+        } else if func.has_atom_attr("bench") {
             RunnableKind::Bench { test_id }
-        } else if has_runnable_doc_test(&attrs) {
-            RunnableKind::DocTest { test_id }
         } else {
             return None;
         }
     };
 
-    let cfg = attrs.cfg();
+    let nav = NavigationTarget::from_named(sema.db, InFile::new(file_id.into(), &func));
-
+    let cfg = def.attrs(sema.db).cfg();
-    let nav = if let RunnableKind::DocTest { .. } = kind {
-        NavigationTarget::from_doc_commented(
-            sema.db,
-            InFile::new(file_id.into(), &fn_def),
-            InFile::new(file_id.into(), &fn_def),
-        )
-    } else {
-        NavigationTarget::from_named(sema.db, InFile::new(file_id.into(), &fn_def))
-    };
     Some(Runnable { nav, kind, cfg })
 }
 
-fn runnable_struct(
+fn runnable_doctest(sema: &Semantics<RootDatabase>, item: SyntaxNode) -> Option<Runnable> {
-    sema: &Semantics<RootDatabase>,
+    match_ast! {
-    struct_def: ast::Struct,
+        match item {
-    file_id: FileId,
+            ast::Fn(it) => module_def_doctest(sema, sema.to_def(&it)?.into()),
-) -> Option<Runnable> {
+            ast::Struct(it) => module_def_doctest(sema, sema.to_def(&it)?.into()),
-    let def = sema.to_def(&struct_def)?;
+            ast::Enum(it) => module_def_doctest(sema, sema.to_def(&it)?.into()),
-    let name_string = struct_def.name()?.text().to_string();
+            ast::Union(it) => module_def_doctest(sema, sema.to_def(&it)?.into()),
+            ast::Trait(it) => module_def_doctest(sema, sema.to_def(&it)?.into()),
+            ast::Const(it) => module_def_doctest(sema, sema.to_def(&it)?.into()),
+            ast::Static(it) => module_def_doctest(sema, sema.to_def(&it)?.into()),
+            ast::TypeAlias(it) => module_def_doctest(sema, sema.to_def(&it)?.into()),
+            _ => None,
+        }
+    }
+}
 
-    let attrs = def.attrs(sema.db);
+fn module_def_doctest(sema: &Semantics<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::EnumVariant(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::BuiltinType(_) => return None,
+    };
     if !has_runnable_doc_test(&attrs) {
         return None;
     }
-    let cfg = attrs.cfg();
+    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) {
+                    if let Some(adt) = imp.target_ty(sema.db).as_adt() {
+                        let name = adt.name(sema.db).to_string();
+                        let idx = path.rfind(':').unwrap_or(0);
+                        let (prefix, suffix) = path.split_at(idx);
+                        return format!("{}{}::{}", prefix, name, suffix);
+                    }
+                }
+            }
+            path
+        })
+        .map(TestId::Path)
+        .or_else(|| def_name.clone().map(TestId::Name))?;
 
-    let test_id = match sema.to_def(&struct_def).map(|def| def.module(sema.db)) {
+    let mut nav = def.try_to_nav(sema.db)?;
-        Some(module) => {
+    nav.focus_range = None;
-            let path_iter = module
+    nav.description = None;
-                .path_to_root(sema.db)
+    nav.docs = None;
-                .into_iter()
+    nav.kind = syntax::SyntaxKind::COMMENT;
-                .rev()
+    let res = Runnable { nav, kind: RunnableKind::DocTest { test_id }, cfg: attrs.cfg() };
-                .filter_map(|it| it.name(sema.db))
+    Some(res)
-                .map(|name| name.to_string());
-            let path = path_iter.chain(std::iter::once(name_string)).join("::");
-
-            TestId::Path(path)
-        }
-        None => TestId::Name(name_string),
-    };
-
-    let nav = NavigationTarget::from_doc_commented(
-        sema.db,
-        InFile::new(file_id.into(), &struct_def),
-        InFile::new(file_id.into(), &struct_def),
-    );
-    Some(Runnable { nav, kind: RunnableKind::DocTest { test_id }, cfg })
 }
 
 #[derive(Debug, Copy, Clone)]
@@ -317,7 +304,7 @@ mod tests {
 
     use crate::fixture;
 
-    use super::{RunnableAction, BENCH, BIN, DOCTEST, TEST};
+    use super::*;
 
     fn check(
         ra_fixture: &str,
@@ -546,7 +533,7 @@ struct StructWithRunnable(String);
                             full_range: 15..74,
                             focus_range: None,
                             name: "should_have_runnable",
-                            kind: FN,
+                            kind: COMMENT,
                             container_name: None,
                             description: None,
                             docs: None,
@@ -566,7 +553,7 @@ struct StructWithRunnable(String);
                             full_range: 76..148,
                             focus_range: None,
                             name: "should_have_runnable_1",
-                            kind: FN,
+                            kind: COMMENT,
                             container_name: None,
                             description: None,
                             docs: None,
@@ -586,7 +573,7 @@ struct StructWithRunnable(String);
                             full_range: 150..254,
                             focus_range: None,
                             name: "should_have_runnable_2",
-                            kind: FN,
+                            kind: COMMENT,
                             container_name: None,
                             description: None,
                             docs: None,
@@ -606,7 +593,7 @@ struct StructWithRunnable(String);
                             full_range: 756..821,
                             focus_range: None,
                             name: "StructWithRunnable",
-                            kind: STRUCT,
+                            kind: COMMENT,
                             container_name: None,
                             description: None,
                             docs: None,
@@ -668,7 +655,7 @@ impl Data {
                             full_range: 44..98,
                             focus_range: None,
                             name: "foo",
-                            kind: FN,
+                            kind: COMMENT,
                             container_name: None,
                             description: None,
                             docs: None,