Merge #1385

1385: Refactor and queryfy documentation handling in HIR r=matklad a=matklad



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

crates/ra_hir/src/code_model.rs

@@ -12,7 +12,6 @@ use crate::{
     ty::{TraitRef, InferenceResult, primitive::{IntTy, FloatTy, Signedness, IntBitness, FloatBitness}},
     adt::{EnumVariantId, StructFieldId, VariantDef},
     generics::HasGenericParams,
-    docs::{Documentation, Docs, docs_from_ast},
     ids::{FunctionId, StructId, EnumId, AstItemDef, ConstId, StaticId, TraitId, TypeAliasId, MacroDefId},
     impl_block::ImplBlock,
     resolve::Resolver,
@@ -197,7 +196,7 @@ impl Module {
     /// `None` for the crate root.
     pub fn declaration_source(
         self,
-        db: &impl HirDatabase,
+        db: &(impl DefDatabase + AstDatabase),
     ) -> Option<(HirFileId, TreeArc<ast::Module>)> {
         let def_map = db.crate_def_map(self.krate);
         let decl = def_map[self.module_id].declaration?;
@@ -319,12 +318,6 @@ impl Module {
     }
 }
 
-impl Docs for Module {
-    fn docs(&self, db: &impl HirDatabase) -> Option<Documentation> {
-        self.declaration_source(db).and_then(|it| docs_from_ast(&*it.1))
-    }
-}
-
 #[derive(Debug, Clone, Copy, PartialEq, Eq, Hash)]
 pub struct StructField {
     pub(crate) parent: VariantDef,
@@ -355,15 +348,6 @@ impl StructField {
     }
 }
 
-impl Docs for StructField {
-    fn docs(&self, db: &impl HirDatabase) -> Option<Documentation> {
-        match self.source(db).1 {
-            FieldSource::Named(named) => docs_from_ast(&*named),
-            FieldSource::Pos(..) => return None,
-        }
-    }
-}
-
 #[derive(Debug, Clone, Copy, PartialEq, Eq, Hash)]
 pub struct Struct {
     pub(crate) id: StructId,
@@ -425,12 +409,6 @@ impl Struct {
     }
 }
 
-impl Docs for Struct {
-    fn docs(&self, db: &impl HirDatabase) -> Option<Documentation> {
-        docs_from_ast(&*self.source(db).1)
-    }
-}
-
 #[derive(Debug, Clone, Copy, PartialEq, Eq, Hash)]
 pub struct Union {
     pub(crate) id: StructId,
@@ -464,12 +442,6 @@ impl Union {
     }
 }
 
-impl Docs for Union {
-    fn docs(&self, db: &impl HirDatabase) -> Option<Documentation> {
-        docs_from_ast(&*self.source(db).1)
-    }
-}
-
 #[derive(Debug, Clone, Copy, PartialEq, Eq, Hash)]
 pub struct Enum {
     pub(crate) id: EnumId,
@@ -519,12 +491,6 @@ impl Enum {
     }
 }
 
-impl Docs for Enum {
-    fn docs(&self, db: &impl HirDatabase) -> Option<Documentation> {
-        docs_from_ast(&*self.source(db).1)
-    }
-}
-
 #[derive(Debug, Clone, Copy, PartialEq, Eq, Hash)]
 pub struct EnumVariant {
     pub(crate) parent: Enum,
@@ -568,12 +534,6 @@ impl EnumVariant {
     }
 }
 
-impl Docs for EnumVariant {
-    fn docs(&self, db: &impl HirDatabase) -> Option<Documentation> {
-        docs_from_ast(&*self.source(db).1)
-    }
-}
-
 /// The defs which have a body.
 #[derive(Debug, Clone, Copy, PartialEq, Eq, Hash)]
 pub enum DefWithBody {
@@ -757,12 +717,6 @@ impl Function {
     }
 }
 
-impl Docs for Function {
-    fn docs(&self, db: &impl HirDatabase) -> Option<Documentation> {
-        docs_from_ast(&*self.source(db).1)
-    }
-}
-
 #[derive(Debug, Clone, Copy, PartialEq, Eq, Hash)]
 pub struct Const {
     pub(crate) id: ConstId,
@@ -806,12 +760,6 @@ impl Const {
     }
 }
 
-impl Docs for Const {
-    fn docs(&self, db: &impl HirDatabase) -> Option<Documentation> {
-        docs_from_ast(&*self.source(db).1)
-    }
-}
-
 /// The declared signature of a const.
 #[derive(Debug, Clone, PartialEq, Eq)]
 pub struct ConstSignature {
@@ -884,12 +832,6 @@ impl Static {
     }
 }
 
-impl Docs for Static {
-    fn docs(&self, db: &impl HirDatabase) -> Option<Documentation> {
-        docs_from_ast(&*self.source(db).1)
-    }
-}
-
 #[derive(Debug, Clone, Copy, PartialEq, Eq, Hash)]
 pub struct Trait {
     pub(crate) id: TraitId,
@@ -936,12 +878,6 @@ impl Trait {
     }
 }
 
-impl Docs for Trait {
-    fn docs(&self, db: &impl HirDatabase) -> Option<Documentation> {
-        docs_from_ast(&*self.source(db).1)
-    }
-}
-
 #[derive(Debug, Clone, Copy, PartialEq, Eq, Hash)]
 pub struct TypeAlias {
     pub(crate) id: TypeAliasId,
@@ -998,16 +934,20 @@ impl TypeAlias {
     }
 }
 
-impl Docs for TypeAlias {
-    fn docs(&self, db: &impl HirDatabase) -> Option<Documentation> {
-        docs_from_ast(&*self.source(db).1)
-    }
-}
 #[derive(Debug, Clone, Copy, PartialEq, Eq, Hash)]
 pub struct MacroDef {
     pub(crate) id: MacroDefId,
 }
 
+impl MacroDef {
+    pub fn source(
+        &self,
+        db: &(impl DefDatabase + AstDatabase),
+    ) -> (HirFileId, TreeArc<ast::MacroCall>) {
+        (self.id.0.file_id(), self.id.0.to_node(db))
+    }
+}
+
 pub enum Container {
     Trait(Trait),
     ImplBlock(ImplBlock),

crates/ra_hir/src/db.rs

@@ -127,6 +127,9 @@ pub trait DefDatabase: SourceDatabase {
 
     #[salsa::invoke(crate::lang_item::LangItems::lang_item_query)]
     fn lang_item(&self, start_crate: Crate, item: SmolStr) -> Option<LangItemTarget>;
+
+    #[salsa::invoke(crate::docs::documentation_query)]
+    fn documentation(&self, def: crate::docs::DocDef) -> Option<crate::docs::Documentation>;
 }
 
 #[salsa::query_group(HirDatabaseStorage)]

crates/ra_hir/src/docs.rs

@@ -1,24 +1,60 @@
+use std::sync::Arc;
+
 use ra_syntax::ast;
 
-use crate::HirDatabase;
+use crate::{
+    HirDatabase, DefDatabase, AstDatabase,
+    Module, StructField, Struct, Enum, EnumVariant, Static, Const, Function, Union, Trait, TypeAlias, FieldSource, MacroDef,
+};
+
+#[derive(Clone, Copy, Debug, PartialEq, Eq, Hash)]
+pub enum DocDef {
+    Module(Module),
+    StructField(StructField),
+    Struct(Struct),
+    Enum(Enum),
+    EnumVariant(EnumVariant),
+    Static(Static),
+    Const(Const),
+    Function(Function),
+    Union(Union),
+    Trait(Trait),
+    TypeAlias(TypeAlias),
+    MacroDef(MacroDef),
+}
+
+impl_froms!(
+    DocDef: Module,
+    StructField,
+    Struct,
+    Enum,
+    EnumVariant,
+    Static,
+    Const,
+    Function,
+    Union,
+    Trait,
+    TypeAlias,
+    MacroDef
+);
 
 /// Holds documentation
-#[derive(Debug, Clone)]
-pub struct Documentation(String);
+#[derive(Debug, Clone, PartialEq, Eq)]
+pub struct Documentation(Arc<str>);
 
 impl Documentation {
-    pub fn new(s: &str) -> Self {
-        Self(s.into())
+    fn new(s: &str) -> Documentation {
+        Documentation(s.into())
     }
 
-    pub fn contents(&self) -> &str {
-        &self.0
+    pub fn as_str(&self) -> &str {
+        &*self.0
     }
 }
 
 impl Into<String> for Documentation {
     fn into(self) -> String {
-        self.contents().into()
+        self.as_str().to_owned()
     }
 }
 
@@ -29,3 +65,32 @@ pub trait Docs {
 pub(crate) fn docs_from_ast(node: &impl ast::DocCommentsOwner) -> Option<Documentation> {
     node.doc_comment_text().map(|it| Documentation::new(&it))
 }
+
+pub(crate) fn documentation_query(
+    db: &(impl DefDatabase + AstDatabase),
+    def: DocDef,
+) -> Option<Documentation> {
+    match def {
+        DocDef::Module(it) => docs_from_ast(&*it.declaration_source(db)?.1),
+        DocDef::StructField(it) => match it.source(db).1 {
+            FieldSource::Named(named) => docs_from_ast(&*named),
+            FieldSource::Pos(..) => return None,
+        },
+        DocDef::Struct(it) => docs_from_ast(&*it.source(db).1),
+        DocDef::Enum(it) => docs_from_ast(&*it.source(db).1),
+        DocDef::EnumVariant(it) => docs_from_ast(&*it.source(db).1),
+        DocDef::Static(it) => docs_from_ast(&*it.source(db).1),
+        DocDef::Const(it) => docs_from_ast(&*it.source(db).1),
+        DocDef::Function(it) => docs_from_ast(&*it.source(db).1),
+        DocDef::Union(it) => docs_from_ast(&*it.source(db).1),
+        DocDef::Trait(it) => docs_from_ast(&*it.source(db).1),
+        DocDef::TypeAlias(it) => docs_from_ast(&*it.source(db).1),
+        DocDef::MacroDef(it) => docs_from_ast(&*it.source(db).1),
+    }
+}
+
+impl<T: Into<DocDef> + Copy> Docs for T {
+    fn docs(&self, db: &impl HirDatabase) -> Option<Documentation> {
+        db.documentation((*self).into())
+    }
+}

crates/ra_hir/src/lib.rs

@@ -69,7 +69,7 @@ pub use self::{
     expr::ExprScopes,
     resolve::Resolution,
     generics::{GenericParams, GenericParam, HasGenericParams},
-    source_binder::{SourceAnalyzer, PathResolution, ScopeEntryWithSyntax,MacroByExampleDef},
+    source_binder::{SourceAnalyzer, PathResolution, ScopeEntryWithSyntax},
 };
 
 pub use self::code_model::{

crates/ra_hir/src/nameres.rs

@@ -323,6 +323,8 @@ impl CrateDefMap {
         (res.resolved_def, res.segment_index)
     }
 
+    // FIXME: This seems to do the same work as `resolve_path_with_macro`, but
+    // using a completely different code path. Seems bad, huh?
     pub(crate) fn find_macro(
         &self,
         db: &impl DefDatabase,

crates/ra_hir/src/resolve.rs

@@ -148,8 +148,8 @@ impl Resolver {
             PathResult::from_resolution(self.resolve_name(db, &Name::self_param()))
         } else {
             let (item_map, module) = match self.module() {
-                Some(m) => m,
-                _ => return PathResult::empty(),
+                Some(it) => it,
+                None => return PathResult::empty(),
             };
             let (module_res, segment_index) = item_map.resolve_path(db, module, path);
 

crates/ra_hir/src/source_binder.rs

@@ -10,7 +10,7 @@ use std::sync::Arc;
 use rustc_hash::{FxHashSet, FxHashMap};
 use ra_db::{FileId, FilePosition};
 use ra_syntax::{
-    SyntaxNode, AstPtr, TextUnit, SyntaxNodePtr, TextRange,TreeArc,
+    SyntaxNode, AstPtr, TextUnit, SyntaxNodePtr, TextRange,
     ast::{self, AstNode, NameOwner},
     algo::find_node_at_offset,
     SyntaxKind::*,
@@ -18,10 +18,9 @@ use ra_syntax::{
 
 use crate::{
     HirDatabase, Function, Struct, Enum, Const, Static, Either, DefWithBody, PerNs, Name,
-    AsName, Module, HirFileId, Crate, Trait, Resolver, Ty,Path,
+    AsName, Module, HirFileId, Crate, Trait, Resolver, Ty, Path, MacroDef,
     expr::{BodySourceMap, scope::{ScopeId, ExprScopes}},
-    ids::{LocationCtx, MacroDefId},
-    docs::{docs_from_ast,Documentation},
+    ids::LocationCtx,
     expr, AstId,
 };
 
@@ -182,27 +181,10 @@ pub enum PathResolution {
     /// A generic parameter
     GenericParam(u32),
     SelfType(crate::ImplBlock),
-    Macro(MacroByExampleDef),
+    Macro(MacroDef),
     AssocItem(crate::ImplItem),
 }
 
-#[derive(Debug, Clone, Copy, PartialEq, Eq, Hash)]
-pub struct MacroByExampleDef {
-    pub(crate) id: MacroDefId,
-}
-
-impl MacroByExampleDef {
-    pub fn source(&self, db: &impl HirDatabase) -> (HirFileId, TreeArc<ast::MacroCall>) {
-        (self.id.0.file_id(), self.id.0.to_node(db))
-    }
-}
-
-impl crate::Docs for MacroByExampleDef {
-    fn docs(&self, db: &impl HirDatabase) -> Option<Documentation> {
-        docs_from_ast(&*self.source(db).1)
-    }
-}
-
 #[derive(Debug, Clone, PartialEq, Eq)]
 pub struct ScopeEntryWithSyntax {
     pub(crate) name: Name,
@@ -284,10 +266,10 @@ impl SourceAnalyzer {
         &self,
         db: &impl HirDatabase,
         macro_call: &ast::MacroCall,
-    ) -> Option<MacroByExampleDef> {
+    ) -> Option<MacroDef> {
         let id =
             self.resolver.resolve_macro_call(db, macro_call.path().and_then(Path::from_ast))?;
-        Some(MacroByExampleDef { id })
+        Some(MacroDef { id })
     }
 
     pub fn resolve_hir_path(

crates/ra_ide_api/src/display/navigation_target.rs

@@ -238,10 +238,7 @@ impl NavigationTarget {
         }
     }
 
-    pub(crate) fn from_macro_def(
-        db: &RootDatabase,
-        macro_call: hir::MacroByExampleDef,
-    ) -> NavigationTarget {
+    pub(crate) fn from_macro_def(db: &RootDatabase, macro_call: hir::MacroDef) -> NavigationTarget {
         let (file_id, node) = macro_call.source(db);
         log::debug!("nav target {}", node.syntax().debug_dump());
         NavigationTarget::from_named(file_id.original_file(db), &*node)

crates/ra_ide_api/src/name_ref_kind.rs

@@ -1,11 +1,12 @@
 use ra_syntax::{AstNode, AstPtr, ast};
 use hir::Either;
-use crate::db::RootDatabase;
 use test_utils::tested_by;
 
+use crate::db::RootDatabase;
+
 pub enum NameRefKind {
     Method(hir::Function),
-    Macro(hir::MacroByExampleDef),
+    Macro(hir::MacroDef),
     FieldAccess(hir::StructField),
     AssocItem(hir::ImplItem),
     Def(hir::ModuleDef),

crates/ra_lsp_server/src/conv.rs

@@ -171,7 +171,7 @@ impl Conv for ra_ide_api::Documentation {
     fn conv(self) -> Documentation {
         Documentation::MarkupContent(MarkupContent {
             kind: MarkupKind::Markdown,
-            value: crate::markdown::sanitize_markdown(self).into(),
+            value: crate::markdown::mark_fenced_blocks_as_rust(self.as_str()).into(),
         })
     }
 }

crates/ra_lsp_server/src/markdown.rs

@@ -1,26 +1,20 @@
-use ra_ide_api::Documentation;
-
-pub(crate) fn sanitize_markdown(docs: Documentation) -> Documentation {
-    let docs: String = docs.into();
-
-    // Massage markdown
+pub(crate) fn mark_fenced_blocks_as_rust(src: &str) -> String {
     let mut processed_lines = Vec::new();
     let mut in_code_block = false;
-    for line in docs.lines() {
+    for line in src.lines() {
         if line.starts_with("```") {
-            in_code_block = !in_code_block;
+            in_code_block ^= true
         }
 
         let line = if in_code_block && line.starts_with("```") && !line.contains("rust") {
-            "```rust".into()
+            "```rust"
         } else {
-            line.to_string()
+            line
         };
 
         processed_lines.push(line);
     }
-
-    Documentation::new(&processed_lines.join("\n"))
+    processed_lines.join("\n")
 }
 
 #[cfg(test)]
@@ -30,9 +24,6 @@ mod tests {
     #[test]
     fn test_codeblock_adds_rust() {
         let comment = "```\nfn some_rust() {}\n```";
-        assert_eq!(
-            sanitize_markdown(Documentation::new(comment)).contents(),
-            "```rust\nfn some_rust() {}\n```"
-        );
+        assert_eq!(mark_fenced_blocks_as_rust(comment), "```rust\nfn some_rust() {}\n```");
     }
 }