Repository: rust-analyzer/rust-analyzer
Pull request #2251 (2019-11-15 07:45:41)

Privatize modules


Patch:
diff --git a/.gitattributes b/.gitattributes
index e749e1dc98..183e9b5211 100644
--- a/.gitattributes
+++ b/.gitattributes
@@ -1,2 +1,2 @@
-* text=auto eol=lf
 crates/ra_syntax/test_data/** -text eof=LF
+crates/ra_ide_api/src/snapshots/** -text eof=LF
diff --git a/crates/ra_assists/src/assists/add_new.rs b/crates/ra_assists/src/assists/add_new.rs
deleted file mode 100644
index a8839cfba3..0000000000
--- a/crates/ra_assists/src/assists/add_new.rs
+++ /dev/null
@@ -1,379 +0,0 @@
-use format_buf::format;
-use hir::{db::HirDatabase, FromSource};
-use join_to_string::join;
-use ra_syntax::{
-    ast::{
-        self, AstNode, NameOwner, StructKind, TypeAscriptionOwner, TypeParamsOwner, VisibilityOwner,
-    },
-    TextUnit, T,
-};
-use std::fmt::Write;
-
-use crate::{Assist, AssistCtx, AssistId};
-
-// Assist: add_new
-//
-// Adds a new inherent impl for a type.
-//
-// ```
-// struct Ctx<T: Clone> {
-//      data: T,<|>
-// }
-// ```
-// ->
-// ```
-// struct Ctx<T: Clone> {
-//      data: T,
-// }
-//
-// impl<T: Clone> Ctx<T> {
-//     fn new(data: T) -> Self { Self { data } }
-// }
-//
-// ```
-pub(crate) fn add_new(ctx: AssistCtx<impl HirDatabase>) -> Option<Assist> {
-    let strukt = ctx.find_node_at_offset::<ast::StructDef>()?;
-
-    // We want to only apply this to non-union structs with named fields
-    let field_list = match (strukt.kind(), strukt.is_union()) {
-        (StructKind::Named(named), false) => named,
-        _ => return None,
-    };
-
-    // Return early if we've found an existing new fn
-    let impl_block = find_struct_impl(&ctx, &strukt)?;
-
-    ctx.add_assist(AssistId("add_new"), "add new fn", |edit| {
-        edit.target(strukt.syntax().text_range());
-
-        let mut buf = String::with_capacity(512);
-
-        if impl_block.is_some() {
-            buf.push('\n');
-        }
-
-        let vis = strukt.visibility().map(|v| format!("{} ", v.syntax()));
-        let vis = vis.as_ref().map(String::as_str).unwrap_or("");
-        write!(&mut buf, "    {}fn new(", vis).unwrap();
-
-        join(field_list.fields().map(|f| {
-            format!(
-                "{}: {}",
-                f.name().unwrap().syntax().text(),
-                f.ascribed_type().unwrap().syntax().text()
-            )
-        }))
-        .separator(", ")
-        .to_buf(&mut buf);
-
-        buf.push_str(") -> Self { Self {");
-
-        join(field_list.fields().map(|f| f.name().unwrap().syntax().text()))
-            .separator(", ")
-            .surround_with(" ", " ")
-            .to_buf(&mut buf);
-
-        buf.push_str("} }");
-
-        let (start_offset, end_offset) = if let Some(impl_block) = impl_block {
-            buf.push('\n');
-            let start = impl_block
-                .syntax()
-                .descendants_with_tokens()
-                .find(|t| t.kind() == T!['{'])
-                .unwrap()
-                .text_range()
-                .end();
-
-            (start, TextUnit::from_usize(1))
-        } else {
-            buf = generate_impl_text(&strukt, &buf);
-            let start = strukt.syntax().text_range().end();
-
-            (start, TextUnit::from_usize(3))
-        };
-
-        edit.set_cursor(start_offset + TextUnit::of_str(&buf) - end_offset);
-        edit.insert(start_offset, buf);
-    })
-}
-
-// Generates the surrounding `impl Type { <code> }` including type and lifetime
-// parameters
-fn generate_impl_text(strukt: &ast::StructDef, code: &str) -> String {
-    let type_params = strukt.type_param_list();
-    let mut buf = String::with_capacity(code.len());
-    buf.push_str("\n\nimpl");
-    if let Some(type_params) = &type_params {
-        format!(buf, "{}", type_params.syntax());
-    }
-    buf.push_str(" ");
-    buf.push_str(strukt.name().unwrap().text().as_str());
-    if let Some(type_params) = type_params {
-        let lifetime_params = type_params
-            .lifetime_params()
-            .filter_map(|it| it.lifetime_token())
-            .map(|it| it.text().clone());
-        let type_params =
-            type_params.type_params().filter_map(|it| it.name()).map(|it| it.text().clone());
-        join(lifetime_params.chain(type_params)).surround_with("<", ">").to_buf(&mut buf);
-    }
-
-    format!(&mut buf, " {{\n{}\n}}\n", code);
-
-    buf
-}
-
-// Uses a syntax-driven approach to find any impl blocks for the struct that
-// exist within the module/file
-//
-// Returns `None` if we've found an existing `new` fn
-//
-// FIXME: change the new fn checking to a more semantic approach when that's more
-// viable (e.g. we process proc macros, etc)
-fn find_struct_impl(
-    ctx: &AssistCtx<impl HirDatabase>,
-    strukt: &ast::StructDef,
-) -> Option<Option<ast::ImplBlock>> {
-    let db = ctx.db;
-    let module = strukt.syntax().ancestors().find(|node| {
-        ast::Module::can_cast(node.kind()) || ast::SourceFile::can_cast(node.kind())
-    })?;
-
-    let struct_ty = {
-        let src = hir::Source { file_id: ctx.frange.file_id.into(), ast: strukt.clone() };
-        hir::Struct::from_source(db, src).unwrap().ty(db)
-    };
-
-    let mut found_new_fn = false;
-
-    let block = module.descendants().filter_map(ast::ImplBlock::cast).find(|impl_blk| {
-        if found_new_fn {
-            return false;
-        }
-
-        let src = hir::Source { file_id: ctx.frange.file_id.into(), ast: impl_blk.clone() };
-        let blk = hir::ImplBlock::from_source(db, src).unwrap();
-
-        let same_ty = blk.target_ty(db) == struct_ty;
-        let not_trait_impl = blk.target_trait(db).is_none();
-
-        found_new_fn = has_new_fn(impl_blk);
-
-        same_ty && not_trait_impl
-    });
-
-    if found_new_fn {
-        None
-    } else {
-        Some(block)
-    }
-}
-
-fn has_new_fn(imp: &ast::ImplBlock) -> bool {
-    if let Some(il) = imp.item_list() {
-        for item in il.impl_items() {
-            if let ast::ImplItem::FnDef(f) = item {
-                if f.name().unwrap().text().eq_ignore_ascii_case("new") {
-                    return true;
-                }
-            }
-        }
-    }
-
-    false
-}
-
-#[cfg(test)]
-mod tests {
-    use super::*;
-    use crate::helpers::{check_assist, check_assist_not_applicable, check_assist_target};
-
-    #[test]
-    #[rustfmt::skip]
-    fn test_add_new() {
-        // Check output of generation
-        check_assist(
-            add_new,
-"struct Foo {<|>}",
-"struct Foo {}
-
-impl Foo {
-    fn new() -> Self { Self {  } }<|>
-}
-",
-        );
-        check_assist(
-            add_new,
-"struct Foo<T: Clone> {<|>}",
-"struct Foo<T: Clone> {}
-
-impl<T: Clone> Foo<T> {
-    fn new() -> Self { Self {  } }<|>
-}
-",
-        );
-        check_assist(
-            add_new,
-"struct Foo<'a, T: Foo<'a>> {<|>}",
-"struct Foo<'a, T: Foo<'a>> {}
-
-impl<'a, T: Foo<'a>> Foo<'a, T> {
-    fn new() -> Self { Self {  } }<|>
-}
-",
-        );
-        check_assist(
-            add_new,
-"struct Foo { baz: String <|>}",
-"struct Foo { baz: String }
-
-impl Foo {
-    fn new(baz: String) -> Self { Self { baz } }<|>
-}
-",
-        );
-        check_assist(
-            add_new,
-"struct Foo { baz: String, qux: Vec<i32> <|>}",
-"struct Foo { baz: String, qux: Vec<i32> }
-
-impl Foo {
-    fn new(baz: String, qux: Vec<i32>) -> Self { Self { baz, qux } }<|>
-}
-",
-        );
-
-        // Check that visibility modifiers don't get brought in for fields
-        check_assist(
-            add_new,
-"struct Foo { pub baz: String, pub qux: Vec<i32> <|>}",
-"struct Foo { pub baz: String, pub qux: Vec<i32> }
-
-impl Foo {
-    fn new(baz: String, qux: Vec<i32>) -> Self { Self { baz, qux } }<|>
-}
-",
-        );
-
-        // Check that it reuses existing impls
-        check_assist(
-            add_new,
-"struct Foo {<|>}
-
-impl Foo {}
-",
-"struct Foo {}
-
-impl Foo {
-    fn new() -> Self { Self {  } }<|>
-}
-",
-        );
-        check_assist(
-            add_new,
-"struct Foo {<|>}
-
-impl Foo {
-    fn qux(&self) {}
-}
-",
-"struct Foo {}
-
-impl Foo {
-    fn new() -> Self { Self {  } }<|>
-
-    fn qux(&self) {}
-}
-",
-        );
-
-        check_assist(
-            add_new,
-"struct Foo {<|>}
-
-impl Foo {
-    fn qux(&self) {}
-    fn baz() -> i32 {
-        5
-    }
-}
-",
-"struct Foo {}
-
-impl Foo {
-    fn new() -> Self { Self {  } }<|>
-
-    fn qux(&self) {}
-    fn baz() -> i32 {
-        5
-    }
-}
-",
-        );
-
-        // Check visibility of new fn based on struct
-        check_assist(
-            add_new,
-"pub struct Foo {<|>}",
-"pub struct Foo {}
-
-impl Foo {
-    pub fn new() -> Self { Self {  } }<|>
-}
-",
-        );
-        check_assist(
-            add_new,
-"pub(crate) struct Foo {<|>}",
-"pub(crate) struct Foo {}
-
-impl Foo {
-    pub(crate) fn new() -> Self { Self {  } }<|>
-}
-",
-        );
-    }
-
-    #[test]
-    fn add_new_not_applicable_if_fn_exists() {
-        check_assist_not_applicable(
-            add_new,
-            "
-struct Foo {<|>}
-
-impl Foo {
-    fn new() -> Self {
-        Self
-    }    
-}",
-        );
-
-        check_assist_not_applicable(
-            add_new,
-            "
-struct Foo {<|>}
-
-impl Foo {
-    fn New() -> Self {
-        Self
-    }    
-}",
-        );
-    }
-
-    #[test]
-    fn add_new_target() {
-        check_assist_target(
-            add_new,
-            "
-struct SomeThingIrrelevant;
-/// Has a lifetime parameter
-struct Foo<'a, T: Foo<'a>> {<|>}
-struct EvenMoreIrrelevant;
-",
-            "/// Has a lifetime parameter
-struct Foo<'a, T: Foo<'a>> {}",
-        );
-    }
-}
diff --git a/crates/ra_assists/src/lib.rs b/crates/ra_assists/src/lib.rs
index f2f0dacbf7..39c1c283f4 100644
--- a/crates/ra_assists/src/lib.rs
+++ b/crates/ra_assists/src/lib.rs
@@ -95,7 +95,6 @@ mod assists {
     mod add_derive;
     mod add_explicit_type;
     mod add_impl;
-    mod add_new;
     mod apply_demorgan;
     mod flip_comma;
     mod flip_binexpr;
@@ -120,7 +119,6 @@ mod assists {
             add_derive::add_derive,
             add_explicit_type::add_explicit_type,
             add_impl::add_impl,
-            add_new::add_new,
             apply_demorgan::apply_demorgan,
             change_visibility::change_visibility,
             fill_match_arms::fill_match_arms,
diff --git a/crates/ra_hir/src/code_model.rs b/crates/ra_hir/src/code_model.rs
index 962d5a8c15..dd43271f4c 100644
--- a/crates/ra_hir/src/code_model.rs
+++ b/crates/ra_hir/src/code_model.rs
@@ -157,7 +157,7 @@ impl Module {
     }
 
     /// Finds a child module with the specified name.
-    pub fn child(self, db: &impl HirDatabase, name: &Name) -> Option<Module> {
+    pub fn child(self, db: &impl DefDatabase, name: &Name) -> Option<Module> {
         let def_map = db.crate_def_map(self.id.krate);
         let child_id = def_map[self.id.module_id].children.get(name)?;
         Some(self.with_module_id(*child_id))
diff --git a/crates/ra_hir/src/from_source.rs b/crates/ra_hir/src/from_source.rs
index 9793af8587..7e5523c54a 100644
--- a/crates/ra_hir/src/from_source.rs
+++ b/crates/ra_hir/src/from_source.rs
@@ -1,6 +1,6 @@
 //! FIXME: write short doc here
 
-use hir_def::{StructId, StructOrUnionId, UnionId};
+use hir_def::{ModuleId, StructId, StructOrUnionId, UnionId};
 use hir_expand::name::AsName;
 use ra_syntax::{
     ast::{self, AstNode, NameOwner},
@@ -10,9 +10,9 @@ use ra_syntax::{
 use crate::{
     db::{AstDatabase, DefDatabase, HirDatabase},
     ids::{AstItemDef, LocationCtx},
-    AstId, Const, Crate, DefWithBody, Enum, EnumVariant, FieldSource, Function, HasBody, HasSource,
-    ImplBlock, Local, Module, ModuleSource, Source, Static, Struct, StructField, Trait, TypeAlias,
-    Union, VariantDef,
+    Const, DefWithBody, Enum, EnumVariant, FieldSource, Function, HasBody, HasSource, ImplBlock,
+    Local, Module, ModuleSource, Source, Static, Struct, StructField, Trait, TypeAlias, Union,
+    VariantDef,
 };
 
 pub trait FromSource: Sized {
@@ -152,44 +152,48 @@ impl Local {
 }
 
 impl Module {
-    pub fn from_declaration(db: &impl HirDatabase, src: Source<ast::Module>) -> Option<Self> {
-        let src_parent = Source {
-            file_id: src.file_id,
-            ast: ModuleSource::new(db, Some(src.file_id.original_file(db)), None),
-        };
-        let parent_module = Module::from_definition(db, src_parent)?;
+    pub fn from_declaration(db: &impl DefDatabase, src: Source<ast::Module>) -> Option<Self> {
+        let parent_declaration = src.ast.syntax().ancestors().skip(1).find_map(ast::Module::cast);
+
+        let parent_module = match parent_declaration {
+            Some(parent_declaration) => {
+                let src_parent = Source { file_id: src.file_id, ast: parent_declaration };
+                Module::from_declaration(db, src_parent)
+            }
+            _ => {
+                let src_parent = Source {
+                    file_id: src.file_id,
+                    ast: ModuleSource::new(db, Some(src.file_id.original_file(db)), None),
+                };
+                Module::from_definition(db, src_parent)
+            }
+        }?;
+
         let child_name = src.ast.name()?;
         parent_module.child(db, &child_name.as_name())
     }
 
-    pub fn from_definition(
-        db: &(impl DefDatabase + AstDatabase),
-        src: Source<ModuleSource>,
-    ) -> Option<Self> {
-        let decl_id = match src.ast {
+    pub fn from_definition(db: &impl DefDatabase, src: Source<ModuleSource>) -> Option<Self> {
+        match src.ast {
             ModuleSource::Module(ref module) => {
                 assert!(!module.has_semi());
-                let ast_id_map = db.ast_id_map(src.file_id);
-                let item_id = AstId::new(src.file_id, ast_id_map.ast_id(module));
-                Some(item_id)
+                return Module::from_declaration(
+                    db,
+                    Source { file_id: src.file_id, ast: module.clone() },
+                );
             }
-            ModuleSource::SourceFile(_) => None,
+            ModuleSource::SourceFile(_) => (),
         };
 
-        db.relevant_crates(src.file_id.original_file(db)).iter().find_map(|&crate_id| {
-            let def_map = db.crate_def_map(crate_id);
-
-            let (module_id, _module_data) =
-                def_map.modules.iter().find(|(_module_id, module_data)| {
-                    if decl_id.is_some() {
-                        module_data.declaration == decl_id
-                    } else {
-                        module_data.definition.map(|it| it.into()) == Some(src.file_id)
-                    }
-                })?;
+        let original_file = src.file_id.original_file(db);
 
-            Some(Module::new(Crate { crate_id }, module_id))
-        })
+        let (krate, module_id) =
+            db.relevant_crates(original_file).iter().find_map(|&crate_id| {
+                let crate_def_map = db.crate_def_map(crate_id);
+                let local_module_id = crate_def_map.modules_for_file(original_file).next()?;
+                Some((crate_id, local_module_id))
+            })?;
+        Some(Module { id: ModuleId { krate, module_id } })
     }
 }
 
diff --git a/crates/ra_hir_def/src/nameres.rs b/crates/ra_hir_def/src/nameres.rs
index d3ecabb9be..5fc5921500 100644
--- a/crates/ra_hir_def/src/nameres.rs
+++ b/crates/ra_hir_def/src/nameres.rs
@@ -87,7 +87,7 @@ pub struct CrateDefMap {
     prelude: Option<ModuleId>,
     extern_prelude: FxHashMap<Name, ModuleDefId>,
     root: CrateModuleId,
-    pub modules: Arena<CrateModuleId, ModuleData>,
+    modules: Arena<CrateModuleId, ModuleData>,
 
     /// Some macros are not well-behavior, which leads to infinite loop
     /// e.g. macro_rules! foo { ($ty:ty) => { foo!($ty); } }
@@ -258,6 +258,17 @@ impl CrateDefMap {
         let res = self.resolve_path_fp_with_macro(db, ResolveMode::Other, original_module, path);
         (res.resolved_def, res.segment_index)
     }
+
+    pub fn modules(&self) -> impl Iterator<Item = CrateModuleId> + '_ {
+        self.modules.iter().map(|(id, _data)| id)
+    }
+
+    pub fn modules_for_file(&self, file_id: FileId) -> impl Iterator<Item = CrateModuleId> + '_ {
+        self.modules
+            .iter()
+            .filter(move |(_id, data)| data.definition == Some(file_id))
+            .map(|(id, _data)| id)
+    }
 }
 
 mod diagnostics {
diff --git a/crates/ra_ide_api/src/hover.rs b/crates/ra_ide_api/src/hover.rs
index 07d511fb31..086e6dec3a 100644
--- a/crates/ra_ide_api/src/hover.rs
+++ b/crates/ra_ide_api/src/hover.rs
@@ -5,7 +5,7 @@ use ra_db::SourceDatabase;
 use ra_syntax::{
     algo::{ancestors_at_offset, find_covering_element, find_node_at_offset},
     ast::{self, DocCommentsOwner},
-    AstNode,
+    match_ast, AstNode,
 };
 
 use crate::{
@@ -14,7 +14,7 @@ use crate::{
         description_from_symbol, docs_from_symbol, macro_label, rust_code_markup,
         rust_code_markup_with_doc, ShortLabel,
     },
-    references::{classify_name, classify_name_ref, NameKind, NameKind::*},
+    references::{classify_name_ref, NameKind::*},
     FilePosition, FileRange, RangeInfo,
 };
 
@@ -92,88 +92,65 @@ fn hover_text(docs: Option<String>, desc: Option<String>) -> Option<String> {
     }
 }
 
-fn hover_text_from_name_kind(
-    db: &RootDatabase,
-    name_kind: NameKind,
-    no_fallback: &mut bool,
-) -> Option<String> {
-    return match name_kind {
-        Macro(it) => {
-            let src = it.source(db);
-            hover_text(src.ast.doc_comment_text(), Some(macro_label(&src.ast)))
-        }
-        Field(it) => {
-            let src = it.source(db);
-            match src.ast {
-                hir::FieldSource::Named(it) => hover_text(it.doc_comment_text(), it.short_label()),
-                _ => None,
-            }
-        }
-        AssocItem(it) => match it {
-            hir::AssocItem::Function(it) => from_def_source(db, it),
-            hir::AssocItem::Const(it) => from_def_source(db, it),
-            hir::AssocItem::TypeAlias(it) => from_def_source(db, it),
-        },
-        Def(it) => match it {
-            hir::ModuleDef::Module(it) => match it.definition_source(db).ast {
-                hir::ModuleSource::Module(it) => {
-                    hover_text(it.doc_comment_text(), it.short_label())
-                }
-                _ => None,
-            },
-            hir::ModuleDef::Function(it) => from_def_source(db, it),
-            hir::ModuleDef::Adt(Adt::Struct(it)) => from_def_source(db, it),
-            hir::ModuleDef::Adt(Adt::Union(it)) => from_def_source(db, it),
-            hir::ModuleDef::Adt(Adt::Enum(it)) => from_def_source(db, it),
-            hir::ModuleDef::EnumVariant(it) => from_def_source(db, it),
-            hir::ModuleDef::Const(it) => from_def_source(db, it),
-            hir::ModuleDef::Static(it) => from_def_source(db, it),
-            hir::ModuleDef::Trait(it) => from_def_source(db, it),
-            hir::ModuleDef::TypeAlias(it) => from_def_source(db, it),
-            hir::ModuleDef::BuiltinType(it) => Some(it.to_string()),
-        },
-        SelfType(ty) => match ty.as_adt() {
-            Some((adt_def, _)) => match adt_def {
-                hir::Adt::Struct(it) => from_def_source(db, it),
-                hir::Adt::Union(it) => from_def_source(db, it),
-                hir::Adt::Enum(it) => from_def_source(db, it),
-            },
-            _ => None,
-        },
-        Local(_) => {
-            // Hover for these shows type names
-            *no_fallback = true;
-            None
-        }
-        GenericParam(_) => {
-            // FIXME: Hover for generic param
-            None
-        }
-    };
-
-    fn from_def_source<A, D>(db: &RootDatabase, def: D) -> Option<String>
-    where
-        D: HasSource<Ast = A>,
-        A: ast::DocCommentsOwner + ast::NameOwner + ShortLabel,
-    {
-        let src = def.source(db);
-        hover_text(src.ast.doc_comment_text(), src.ast.short_label())
-    }
-}
-
 pub(crate) fn hover(db: &RootDatabase, position: FilePosition) -> Option<RangeInfo<HoverResult>> {
     let parse = db.parse(position.file_id);
     let file = parse.tree();
-
     let mut res = HoverResult::new();
 
-    let mut range = if let Some(name_ref) =
-        find_node_at_offset::<ast::NameRef>(file.syntax(), position.offset)
-    {
+    let mut range = None;
+    if let Some(name_ref) = find_node_at_offset::<ast::NameRef>(file.syntax(), position.offset) {
         let mut no_fallback = false;
-        if let Some(name_kind) = classify_name_ref(db, position.file_id, &name_ref).map(|d| d.kind)
-        {
-            res.extend(hover_text_from_name_kind(db, name_kind, &mut no_fallback))
+        let name_kind = classify_name_ref(db, position.file_id, &name_ref).map(|d| d.kind);
+        match name_kind {
+            Some(Macro(it)) => {
+                let src = it.source(db);
+                res.extend(hover_text(src.ast.doc_comment_text(), Some(macro_label(&src.ast))));
+            }
+            Some(Field(it)) => {
+                let src = it.source(db);
+                if let hir::FieldSource::Named(it) = src.ast {
+                    res.extend(hover_text(it.doc_comment_text(), it.short_label()));
+                }
+            }
+            Some(AssocItem(it)) => res.extend(match it {
+                hir::AssocItem::Function(it) => from_def_source(db, it),
+                hir::AssocItem::Const(it) => from_def_source(db, it),
+                hir::AssocItem::TypeAlias(it) => from_def_source(db, it),
+            }),
+            Some(Def(it)) => match it {
+                hir::ModuleDef::Module(it) => {
+                    if let hir::ModuleSource::Module(it) = it.definition_source(db).ast {
+                        res.extend(hover_text(it.doc_comment_text(), it.short_label()))
+                    }
+                }
+                hir::ModuleDef::Function(it) => res.extend(from_def_source(db, it)),
+                hir::ModuleDef::Adt(Adt::Struct(it)) => res.extend(from_def_source(db, it)),
+                hir::ModuleDef::Adt(Adt::Union(it)) => res.extend(from_def_source(db, it)),
+                hir::ModuleDef::Adt(Adt::Enum(it)) => res.extend(from_def_source(db, it)),
+                hir::ModuleDef::EnumVariant(it) => res.extend(from_def_source(db, it)),
+                hir::ModuleDef::Const(it) => res.extend(from_def_source(db, it)),
+                hir::ModuleDef::Static(it) => res.extend(from_def_source(db, it)),
+                hir::ModuleDef::Trait(it) => res.extend(from_def_source(db, it)),
+                hir::ModuleDef::TypeAlias(it) => res.extend(from_def_source(db, it)),
+                hir::ModuleDef::BuiltinType(it) => res.extend(Some(it.to_string())),
+            },
+            Some(SelfType(ty)) => {
+                if let Some((adt_def, _)) = ty.as_adt() {
+                    res.extend(match adt_def {
+                        hir::Adt::Struct(it) => from_def_source(db, it),
+                        hir::Adt::Union(it) => from_def_source(db, it),
+                        hir::Adt::Enum(it) => from_def_source(db, it),
+                    })
+                }
+            }
+            Some(Local(_)) => {
+                // Hover for these shows type names
+                no_fallback = true;
+            }
+            Some(GenericParam(_)) => {
+                // FIXME: Hover for generic param
+            }
+            None => {}
         }
 
         if res.is_empty() && !no_fallback {
@@ -187,24 +164,55 @@ pub(crate) fn hover(db: &RootDatabase, position: FilePosition) -> Option<RangeIn
         }
 
         if !res.is_empty() {
-            Some(name_ref.syntax().text_range())
-        } else {
-            None
+            range = Some(name_ref.syntax().text_range())
         }
     } else if let Some(name) = find_node_at_offset::<ast::Name>(file.syntax(), position.offset) {
-        if let Some(name_kind) = classify_name(db, position.file_id, &name).map(|d| d.kind) {
-            let mut _b: bool = true;
-            res.extend(hover_text_from_name_kind(db, name_kind, &mut _b));
+        if let Some(parent) = name.syntax().parent() {
+            let text = match_ast! {
+                match parent {
+                    ast::StructDef(it) => {
+                        hover_text(it.doc_comment_text(), it.short_label())
+                    },
+                    ast::EnumDef(it) => {
+                        hover_text(it.doc_comment_text(), it.short_label())
+                    },
+                    ast::EnumVariant(it) => {
+                        hover_text(it.doc_comment_text(), it.short_label())
+                    },
+                    ast::FnDef(it) => {
+                        hover_text(it.doc_comment_text(), it.short_label())
+                    },
+                    ast::TypeAliasDef(it) => {
+                        hover_text(it.doc_comment_text(), it.short_label())
+                    },
+                    ast::ConstDef(it) => {
+                        hover_text(it.doc_comment_text(), it.short_label())
+                    },
+                    ast::StaticDef(it) => {
+                        hover_text(it.doc_comment_text(), it.short_label())
+                    },
+                    ast::TraitDef(it) => {
+                        hover_text(it.doc_comment_text(), it.short_label())
+                    },
+                    ast::RecordFieldDef(it) => {
+                        hover_text(it.doc_comment_text(), it.short_label())
+                    },
+                    ast::Module(it) => {
+                        hover_text(it.doc_comment_text(), it.short_label())
+                    },
+                    ast::MacroCall(it) => {
+                        hover_text(it.doc_comment_text(), None)
+                    },
+                    _ => None,
+                }
+            };
+            res.extend(text);
         }
 
-        if !res.is_empty() {
-            Some(name.syntax().text_range())
-        } else {
-            None
+        if !res.is_empty() && range.is_none() {
+            range = Some(name.syntax().text_range());
         }
-    } else {
-        None
-    };
+    }
 
     if range.is_none() {
         let node = ancestors_at_offset(file.syntax(), position.offset).find(|n| {
@@ -213,13 +221,23 @@ pub(crate) fn hover(db: &RootDatabase, position: FilePosition) -> Option<RangeIn
         let frange = FileRange { file_id: position.file_id, range: node.text_range() };
         res.extend(type_of(db, frange).map(rust_code_markup));
         range = Some(node.text_range());
-    };
+    }
 
     let range = range?;
     if res.is_empty() {
         return None;
     }
-    Some(RangeInfo::new(range, res))
+    let res = RangeInfo::new(range, res);
+    return Some(res);
+
+    fn from_def_source<A, D>(db: &RootDatabase, def: D) -> Option<String>
+    where
+        D: HasSource<Ast = A>,
+        A: ast::DocCommentsOwner + ast::NameOwner + ShortLabel,
+    {
+        let src = def.source(db);
+        hover_text(src.ast.doc_comment_text(), src.ast.short_label())
+    }
 }
 
 pub(crate) fn type_of(db: &RootDatabase, frange: FileRange) -> Option<String> {
diff --git a/crates/ra_prof/Cargo.toml b/crates/ra_prof/Cargo.toml
index 751bcdeb8a..bb241258cf 100644
--- a/crates/ra_prof/Cargo.toml
+++ b/crates/ra_prof/Cargo.toml
@@ -1,19 +1,19 @@
-[package]
-edition = "2018"
-name = "ra_prof"
-version = "0.1.0"
-authors = ["rust-analyzer developers"]
-publish = false
-
-[dependencies]
-once_cell = "1.0.1"
-itertools = "0.8.0"
-backtrace = "0.3.28"
-
-[target.'cfg(not(target_env = "msvc"))'.dependencies]
-jemallocator = { version = "0.3.2", optional = true }
-jemalloc-ctl = { version = "0.3.2", optional = true }
-
-[features]
-jemalloc = [ "jemallocator", "jemalloc-ctl" ]
-cpu_profiler = []
+[package]
+edition = "2018"
+name = "ra_prof"
+version = "0.1.0"
+authors = ["rust-analyzer developers"]
+publish = false
+
+[dependencies]
+once_cell = "1.0.1"
+itertools = "0.8.0"
+backtrace = "0.3.28"
+
+[target.'cfg(not(target_env = "msvc"))'.dependencies]
+jemallocator = { version = "0.3.2", optional = true }
+jemalloc-ctl = { version = "0.3.2", optional = true }
+
+[features]
+jemalloc = [ "jemallocator", "jemalloc-ctl" ]
+cpu_profiler = []
diff --git a/docs/user/assists.md b/docs/user/assists.md
index 8da7578e2f..303353e742 100644
--- a/docs/user/assists.md
+++ b/docs/user/assists.md
@@ -150,27 +150,6 @@ use std::collections::HashMap;
 fn process(map: HashMap<String, String>) {}
 ```
 
-## `add_new`
-
-Adds a new inherent impl for a type.
-
-```rust
-// BEFORE
-struct Ctx<T: Clone> {
-     data: T,┃
-}
-
-// AFTER
-struct Ctx<T: Clone> {
-     data: T,
-}
-
-impl<T: Clone> Ctx<T> {
-    fn new(data: T) -> Self { Self { data } }
-}
-
-```
-
 ## `apply_demorgan`
 
 Apply [De Morgan's law](https://en.wikipedia.org/wiki/De_Morgan%27s_laws).


Test patch:
diff --git a/crates/ra_assists/src/doc_tests/generated.rs b/crates/ra_assists/src/doc_tests/generated.rs
index 176761efb9..1bee76f59a 100644
--- a/crates/ra_assists/src/doc_tests/generated.rs
+++ b/crates/ra_assists/src/doc_tests/generated.rs
@@ -156,28 +156,6 @@ fn process(map: HashMap<String, String>) {}
     )
 }
 
-#[test]
-fn doctest_add_new() {
-    check(
-        "add_new",
-        r#####"
-struct Ctx<T: Clone> {
-     data: T,<|>
-}
-"#####,
-        r#####"
-struct Ctx<T: Clone> {
-     data: T,
-}
-
-impl<T: Clone> Ctx<T> {
-    fn new(data: T) -> Self { Self { data } }
-}
-
-"#####,
-    )
-}
-
 #[test]
 fn doctest_apply_demorgan() {
     check(
diff --git a/crates/ra_hir/src/test_db.rs b/crates/ra_hir/src/test_db.rs
index 5237b303af..1caa2e8759 100644
--- a/crates/ra_hir/src/test_db.rs
+++ b/crates/ra_hir/src/test_db.rs
@@ -81,7 +81,7 @@ impl TestDB {
         let crate_graph = self.crate_graph();
         for krate in crate_graph.iter().next() {
             let crate_def_map = self.crate_def_map(krate);
-            for (module_id, _) in crate_def_map.modules.iter() {
+            for module_id in crate_def_map.modules() {
                 let module_id = ModuleId { krate, module_id };
                 let module = crate::Module::from(module_id);
                 module.diagnostics(