6 files changed, 63 insertions, 114 deletions

diff --git a/crates/hir/src/attrs.rs b/crates/hir/src/attrs.rs
index fb2631b3e..1f2ee2580 100644
--- a/crates/hir/src/attrs.rs
+++ b/crates/hir/src/attrs.rs
@@ -1,6 +1,9 @@
 //! Attributes & documentation for hir types.
 use hir_def::{
-    attr::Attrs, docs::Documentation, path::ModPath, resolver::HasResolver, AttrDefId, ModuleDefId,
+    attr::{Attrs, Documentation},
+    path::ModPath,
+    resolver::HasResolver,
+    AttrDefId, ModuleDefId,
 };
 use hir_expand::hygiene::Hygiene;
 use hir_ty::db::HirDatabase;
diff --git a/crates/hir/src/lib.rs b/crates/hir/src/lib.rs
index 93bdb4472..c7c7377d7 100644
--- a/crates/hir/src/lib.rs
+++ b/crates/hir/src/lib.rs
@@ -44,10 +44,9 @@ pub use crate::{
 
 pub use hir_def::{
     adt::StructKind,
-    attr::Attrs,
+    attr::{Attrs, Documentation},
     body::scope::ExprScopes,
     builtin_type::BuiltinType,
-    docs::Documentation,
     find_path::PrefixKind,
     import_map,
     item_scope::ItemInNs,
diff --git a/crates/hir_def/src/attr.rs b/crates/hir_def/src/attr.rs
index 7825290e6..98293aad3 100644
--- a/crates/hir_def/src/attr.rs
+++ b/crates/hir_def/src/attr.rs
@@ -15,7 +15,6 @@ use tt::Subtree;
 
 use crate::{
     db::DefDatabase,
-    docs::Documentation,
     item_tree::{ItemTreeId, ItemTreeNode},
     nameres::ModuleSource,
     path::ModPath,
@@ -23,6 +22,22 @@ use crate::{
     AdtId, AttrDefId, Lookup,
 };
 
+/// Holds documentation
+#[derive(Debug, Clone, PartialEq, Eq)]
+pub struct Documentation(Arc<str>);
+
+impl Documentation {
+    pub fn as_str(&self) -> &str {
+        &self.0
+    }
+}
+
+impl Into<String> for Documentation {
+    fn into(self) -> String {
+        self.as_str().to_owned()
+    }
+}
+
 #[derive(Default, Debug, Clone, PartialEq, Eq)]
 pub struct Attrs {
     entries: Option<Arc<[Attr]>>,
@@ -102,7 +117,7 @@ impl Attrs {
             },
         );
         let mut attrs = owner.attrs().peekable();
-        let entries = if attrs.peek().is_none() {
+        let entries = if attrs.peek().is_none() && docs.is_none() {
             // Avoid heap allocation
             None
         } else {
@@ -154,7 +169,11 @@ impl Attrs {
             .intersperse(&SmolStr::new_inline("\n"))
             // No FromIterator<SmolStr> for String
             .for_each(|s| docs.push_str(s.as_str()));
-        if docs.is_empty() { None } else { Some(docs) }.map(|it| Documentation::new(&it))
+        if docs.is_empty() {
+            None
+        } else {
+            Some(Documentation(docs.into()))
+        }
     }
 }
 
diff --git a/crates/hir_def/src/docs.rs b/crates/hir_def/src/docs.rs
deleted file mode 100644
index 6a27effef..000000000
--- a/crates/hir_def/src/docs.rs
+++ /dev/null
@@ -1,79 +0,0 @@
-//! Defines hir documentation.
-//!
-//! This really shouldn't exist, instead, we should deshugar doc comments into attributes, see
-//! https://github.com/rust-analyzer/rust-analyzer/issues/2148#issuecomment-550519102
-
-use std::sync::Arc;
-
-use itertools::Itertools;
-use syntax::{ast, SmolStr};
-
-/// Holds documentation
-#[derive(Debug, Clone, PartialEq, Eq)]
-pub struct Documentation(Arc<str>);
-
-impl Into<String> for Documentation {
-    fn into(self) -> String {
-        self.as_str().to_owned()
-    }
-}
-
-impl Documentation {
-    pub fn new(s: &str) -> Documentation {
-        Documentation(s.into())
-    }
-
-    pub fn from_ast<N>(node: &N) -> Option<Documentation>
-    where
-        N: ast::DocCommentsOwner + ast::AttrsOwner,
-    {
-        docs_from_ast(node)
-    }
-
-    pub fn as_str(&self) -> &str {
-        &*self.0
-    }
-}
-
-pub(crate) fn docs_from_ast<N>(node: &N) -> Option<Documentation>
-where
-    N: ast::DocCommentsOwner + ast::AttrsOwner,
-{
-    let doc_comment_text = node.doc_comment_text();
-    let doc_attr_text = expand_doc_attrs(node);
-    let docs = merge_doc_comments_and_attrs(doc_comment_text, doc_attr_text);
-    docs.map(|it| Documentation::new(&it))
-}
-
-fn merge_doc_comments_and_attrs(
-    doc_comment_text: Option<String>,
-    doc_attr_text: Option<String>,
-) -> Option<String> {
-    match (doc_comment_text, doc_attr_text) {
-        (Some(mut comment_text), Some(attr_text)) => {
-            comment_text.reserve(attr_text.len() + 1);
-            comment_text.push('\n');
-            comment_text.push_str(&attr_text);
-            Some(comment_text)
-        }
-        (Some(comment_text), None) => Some(comment_text),
-        (None, Some(attr_text)) => Some(attr_text),
-        (None, None) => None,
-    }
-}
-
-fn expand_doc_attrs(owner: &dyn ast::AttrsOwner) -> Option<String> {
-    let mut docs = String::new();
-    owner
-        .attrs()
-        .filter_map(|attr| attr.as_simple_key_value().filter(|(key, _)| key == "doc"))
-        .map(|(_, value)| value)
-        .intersperse(SmolStr::new_inline("\n"))
-        // No FromIterator<SmolStr> for String
-        .for_each(|s| docs.push_str(s.as_str()));
-    if docs.is_empty() {
-        None
-    } else {
-        Some(docs)
-    }
-}
diff --git a/crates/hir_def/src/lib.rs b/crates/hir_def/src/lib.rs
index b41c5acb2..02ed30e4d 100644
--- a/crates/hir_def/src/lib.rs
+++ b/crates/hir_def/src/lib.rs
@@ -31,7 +31,6 @@ pub mod adt;
 pub mod data;
 pub mod generics;
 pub mod lang_item;
-pub mod docs;
 
 pub mod expr;
 pub mod body;
diff --git a/crates/ide/src/hover.rs b/crates/ide/src/hover.rs
index dc9621f46..1b6ff6d21 100644
--- a/crates/ide/src/hover.rs
+++ b/crates/ide/src/hover.rs
@@ -1,6 +1,6 @@
 use hir::{
-    Adt, AsAssocItem, AssocItemContainer, Documentation, FieldSource, HasSource, HirDisplay,
-    Module, ModuleDef, ModuleSource, Semantics,
+    Adt, AsAssocItem, AssocItemContainer, FieldSource, HasAttrs, HasSource, HirDisplay, Module,
+    ModuleDef, ModuleSource, Semantics,
 };
 use ide_db::base_db::SourceDatabase;
 use ide_db::{
@@ -319,31 +319,27 @@ fn hover_for_definition(db: &RootDatabase, def: Definition) -> Option<Markup> {
     let mod_path = definition_mod_path(db, &def);
     return match def {
         Definition::Macro(it) => {
-            let src = it.source(db);
-            let docs = Documentation::from_ast(&src.value).map(Into::into);
-            hover_markup(docs, Some(macro_label(&src.value)), mod_path)
+            let label = macro_label(&it.source(db).value);
+            from_def_source_labeled(db, it, Some(label), mod_path)
         }
-        Definition::Field(it) => {
-            let src = it.source(db);
-            match src.value {
-                FieldSource::Named(it) => {
-                    let docs = Documentation::from_ast(&it).map(Into::into);
-                    hover_markup(docs, it.short_label(), mod_path)
-                }
-                _ => None,
+        Definition::Field(def) => {
+            let src = def.source(db).value;
+            if let FieldSource::Named(it) = src {
+                from_def_source_labeled(db, def, it.short_label(), mod_path)
+            } else {
+                None
             }
         }
         Definition::ModuleDef(it) => match it {
-            ModuleDef::Module(it) => match it.definition_source(db).value {
-                ModuleSource::Module(it) => {
-                    let docs = Documentation::from_ast(&it).map(Into::into);
-                    hover_markup(docs, it.short_label(), mod_path)
-                }
-                ModuleSource::SourceFile(it) => {
-                    let docs = Documentation::from_ast(&it).map(Into::into);
-                    hover_markup(docs, it.short_label(), mod_path)
-                }
-            },
+            ModuleDef::Module(it) => from_def_source_labeled(
+                db,
+                it,
+                match it.definition_source(db).value {
+                    ModuleSource::Module(it) => it.short_label(),
+                    ModuleSource::SourceFile(it) => it.short_label(),
+                },
+                mod_path,
+            ),
             ModuleDef::Function(it) => from_def_source(db, it, mod_path),
             ModuleDef::Adt(Adt::Struct(it)) => from_def_source(db, it, mod_path),
             ModuleDef::Adt(Adt::Union(it)) => from_def_source(db, it, mod_path),
@@ -371,12 +367,24 @@ fn hover_for_definition(db: &RootDatabase, def: Definition) -> Option<Markup> {
 
     fn from_def_source<A, D>(db: &RootDatabase, def: D, mod_path: Option<String>) -> Option<Markup>
     where
-        D: HasSource<Ast = A>,
-        A: ast::DocCommentsOwner + ast::NameOwner + ShortLabel + ast::AttrsOwner,
+        D: HasSource<Ast = A> + HasAttrs + Copy,
+        A: ShortLabel,
+    {
+        let short_label = def.source(db).value.short_label();
+        from_def_source_labeled(db, def, short_label, mod_path)
+    }
+
+    fn from_def_source_labeled<D>(
+        db: &RootDatabase,
+        def: D,
+        short_label: Option<String>,
+        mod_path: Option<String>,
+    ) -> Option<Markup>
+    where
+        D: HasAttrs,
     {
-        let src = def.source(db);
-        let docs = Documentation::from_ast(&src.value).map(Into::into);
-        hover_markup(docs, src.value.short_label(), mod_path)
+        let docs = def.attrs(db).docs().map(Into::into);
+        hover_markup(docs, short_label, mod_path)
     }
 }