30 files changed, 831 insertions, 177 deletions
diff --git a/Cargo.lock b/Cargo.lock
index 7d2911076..b63b5ae13 100644
--- a/Cargo.lock
+++ b/Cargo.lock
@@ -10,7 +10,7 @@ dependencies = [
 [[package]]
 name = "anyhow"
-version = "1.0.25"
+version = "1.0.26"
 source = "registry+https://github.com/rust-lang/crates.io-index"
 [[package]]
@@ -237,7 +237,7 @@ dependencies = [
 "crossbeam-channel 0.4.0 (registry+https://github.com/rust-lang/crates.io-index)",
 "crossbeam-deque 0.7.2 (registry+https://github.com/rust-lang/crates.io-index)",
 "crossbeam-epoch 0.8.0 (registry+https://github.com/rust-lang/crates.io-index)",
- "crossbeam-queue 0.2.0 (registry+https://github.com/rust-lang/crates.io-index)",
+ "crossbeam-queue 0.2.1 (registry+https://github.com/rust-lang/crates.io-index)",
 "crossbeam-utils 0.7.0 (registry+https://github.com/rust-lang/crates.io-index)",
 ]
@@ -273,9 +273,10 @@ dependencies = [
 [[package]]
 name = "crossbeam-queue"
-version = "0.2.0"
+version = "0.2.1"
 source = "registry+https://github.com/rust-lang/crates.io-index"
 dependencies = [
+ "cfg-if 0.1.10 (registry+https://github.com/rust-lang/crates.io-index)",
 "crossbeam-utils 0.7.0 (registry+https://github.com/rust-lang/crates.io-index)",
 ]
@@ -1349,7 +1350,7 @@ version = "1.7.0"
 source = "registry+https://github.com/rust-lang/crates.io-index"
 dependencies = [
 "crossbeam-deque 0.7.2 (registry+https://github.com/rust-lang/crates.io-index)",
- "crossbeam-queue 0.2.0 (registry+https://github.com/rust-lang/crates.io-index)",
+ "crossbeam-queue 0.2.1 (registry+https://github.com/rust-lang/crates.io-index)",
 "crossbeam-utils 0.7.0 (registry+https://github.com/rust-lang/crates.io-index)",
 "lazy_static 1.4.0 (registry+https://github.com/rust-lang/crates.io-index)",
 "num_cpus 1.11.1 (registry+https://github.com/rust-lang/crates.io-index)",
@@ -1767,7 +1768,7 @@ dependencies = [
 name = "xtask"
 version = "0.1.0"
 dependencies = [
- "anyhow 1.0.25 (registry+https://github.com/rust-lang/crates.io-index)",
+ "anyhow 1.0.26 (registry+https://github.com/rust-lang/crates.io-index)",
 "pico-args 0.3.0 (registry+https://github.com/rust-lang/crates.io-index)",
 "proc-macro2 1.0.6 (registry+https://github.com/rust-lang/crates.io-index)",
 "quote 1.0.2 (registry+https://github.com/rust-lang/crates.io-index)",
@@ -1786,7 +1787,7 @@ dependencies = [
 [metadata]
 "checksum aho-corasick 0.7.6 (registry+https://github.com/rust-lang/crates.io-index)" = "58fb5e95d83b38284460a5fda7d6470aa0b8844d283a0b614b8535e880800d2d"
-"checksum anyhow 1.0.25 (registry+https://github.com/rust-lang/crates.io-index)" = "9267dff192e68f3399525901e709a48c1d3982c9c072fa32f2127a0cb0babf14"
+"checksum anyhow 1.0.26 (registry+https://github.com/rust-lang/crates.io-index)" = "7825f6833612eb2414095684fcf6c635becf3ce97fe48cf6421321e93bfbd53c"
 "checksum anymap 0.12.1 (registry+https://github.com/rust-lang/crates.io-index)" = "33954243bd79057c2de7338850b85983a44588021f8a5fee574a8888c6de4344"
 "checksum arrayvec 0.5.1 (registry+https://github.com/rust-lang/crates.io-index)" = "cff77d8686867eceff3105329d4698d96c2391c176d5d03adc90c7389162b5b8"
 "checksum atty 0.2.13 (registry+https://github.com/rust-lang/crates.io-index)" = "1803c647a3ec87095e7ae7acfca019e98de5ec9a7d01343f611cf3152ed71a90"
@@ -1817,7 +1818,7 @@ dependencies = [
 "checksum crossbeam-channel 0.4.0 (registry+https://github.com/rust-lang/crates.io-index)" = "acec9a3b0b3559f15aee4f90746c4e5e293b701c0f7d3925d24e01645267b68c"
 "checksum crossbeam-deque 0.7.2 (registry+https://github.com/rust-lang/crates.io-index)" = "c3aa945d63861bfe624b55d153a39684da1e8c0bc8fba932f7ee3a3c16cea3ca"
 "checksum crossbeam-epoch 0.8.0 (registry+https://github.com/rust-lang/crates.io-index)" = "5064ebdbf05ce3cb95e45c8b086f72263f4166b29b97f6baff7ef7fe047b55ac"
-"checksum crossbeam-queue 0.2.0 (registry+https://github.com/rust-lang/crates.io-index)" = "dfd6515864a82d2f877b42813d4553292c6659498c9a2aa31bab5a15243c2700"
+"checksum crossbeam-queue 0.2.1 (registry+https://github.com/rust-lang/crates.io-index)" = "c695eeca1e7173472a32221542ae469b3e9aac3a4fc81f7696bcad82029493db"
 "checksum crossbeam-utils 0.7.0 (registry+https://github.com/rust-lang/crates.io-index)" = "ce446db02cdc3165b94ae73111e570793400d0794e46125cc4056c81cbb039f4"
 "checksum difference 2.0.0 (registry+https://github.com/rust-lang/crates.io-index)" = "524cbf6897b527295dff137cec09ecf3a05f4fddffd7dfcd1585403449e74198"
 "checksum drop_bomb 0.1.4 (registry+https://github.com/rust-lang/crates.io-index)" = "69b26e475fd29098530e709294e94e661974c851aed42512793f120fed4e199f"
diff --git a/crates/ra_assists/src/assist_ctx.rs b/crates/ra_assists/src/assist_ctx.rs
index 993aebc47..28152f724 100644
--- a/crates/ra_assists/src/assist_ctx.rs
+++ b/crates/ra_assists/src/assist_ctx.rs
@@ -46,7 +46,7 @@ pub(crate) enum Assist {
 ///
 /// Note, however, that we don't actually use such two-phase logic at the
 /// moment, because the LSP API is pretty awkward in this place, and it's much
-/// easier to just compute the edit eagerly :-)#[derive(Debug, Clone)]
+/// easier to just compute the edit eagerly :-)
 #[derive(Debug)]
 pub(crate) struct AssistCtx<'a, DB> {
    pub(crate) db: &'a DB,
diff --git a/crates/ra_assists/src/assists/add_missing_impl_members.rs b/crates/ra_assists/src/assists/add_missing_impl_members.rs
index cef669cb5..bc49e71fe 100644
--- a/crates/ra_assists/src/assists/add_missing_impl_members.rs
+++ b/crates/ra_assists/src/assists/add_missing_impl_members.rs
@@ -1,3 +1,5 @@
+use std::collections::HashMap;
 use hir::{db::HirDatabase, HasSource};
 use ra_syntax::{
    ast::{self, edit, make, AstNode, NameOwner},
@@ -17,26 +19,26 @@ enum AddMissingImplMembersMode {
 // Adds scaffold for required impl members.
 //
 // ```
-// trait T {
+// trait Trait<T> {
 //     Type X;
-//     fn foo(&self);
+//     fn foo(&self) -> T;
 //     fn bar(&self) {}
 // }
 //
-// impl T for () {<|>
+// impl Trait<u32> for () {<|>
 //
 // }
 // ```
 // ->
 // ```
-// trait T {
+// trait Trait<T> {
 //     Type X;
-//     fn foo(&self);
+//     fn foo(&self) -> T;
 //     fn bar(&self) {}
 // }
 //
-// impl T for () {
+// impl Trait<u32> for () {
-//     fn foo(&self) { unimplemented!() }
+//     fn foo(&self) -> u32 { unimplemented!() }
 //
 // }
 // ```
@@ -54,13 +56,13 @@ pub(crate) fn add_missing_impl_members(ctx: AssistCtx<impl HirDatabase>) -> Opti
 // Adds scaffold for overriding default impl members.
 //
 // ```
-// trait T {
+// trait Trait {
 //     Type X;
 //     fn foo(&self);
 //     fn bar(&self) {}
 // }
 //
-// impl T for () {
+// impl Trait for () {
 //     Type X = ();
 //     fn foo(&self) {}<|>
 //
@@ -68,13 +70,13 @@ pub(crate) fn add_missing_impl_members(ctx: AssistCtx<impl HirDatabase>) -> Opti
 // ```
 // ->
 // ```
-// trait T {
+// trait Trait {
 //     Type X;
 //     fn foo(&self);
 //     fn bar(&self) {}
 // }
 //
-// impl T for () {
+// impl Trait for () {
 //     Type X = ();
 //     fn foo(&self) {}
 //     fn bar(&self) {}
@@ -99,7 +101,7 @@ fn add_missing_impl_members_inner(
    let impl_node = ctx.find_node_at_offset::<ast::ImplBlock>()?;
    let impl_item_list = impl_node.item_list()?;
-    let trait_def = {
+    let (trait_, trait_def) = {
        let analyzer = ctx.source_analyzer(impl_node.syntax(), None);
        resolve_target_trait_def(ctx.db, &analyzer, &impl_node)?
@@ -132,10 +134,25 @@ fn add_missing_impl_members_inner(
        return None;
    }
+    let file_id = ctx.frange.file_id;
+    let db = ctx.db;
    ctx.add_assist(AssistId(assist_id), label, |edit| {
        let n_existing_items = impl_item_list.impl_items().count();
+        let substs = get_syntactic_substs(impl_node).unwrap_or_default();
+        let generic_def: hir::GenericDef = trait_.into();
+        let substs_by_param: HashMap<_, _> = generic_def
+            .params(db)
+            .into_iter()
+            // this is a trait impl, so we need to skip the first type parameter -- this is a bit hacky
+            .skip(1)
+            .zip(substs.into_iter())
+            .collect();
        let items = missing_items
            .into_iter()
+            .map(|it| {
+                substitute_type_params(db, hir::InFile::new(file_id.into(), it), &substs_by_param)
+            })
            .map(|it| match it {
                ast::ImplItem::FnDef(def) => ast::ImplItem::FnDef(add_body(def)),
                _ => it,
@@ -160,13 +177,63 @@ fn add_body(fn_def: ast::FnDef) -> ast::FnDef {
    }
 }
+// FIXME: It would probably be nicer if we could get this via HIR (i.e. get the
+// trait ref, and then go from the types in the substs back to the syntax)
+// FIXME: This should be a general utility (not even just for assists)
+fn get_syntactic_substs(impl_block: ast::ImplBlock) -> Option<Vec<ast::TypeRef>> {
+    let target_trait = impl_block.target_trait()?;
+    let path_type = match target_trait {
+        ast::TypeRef::PathType(path) => path,
+        _ => return None,
+    };
+    let type_arg_list = path_type.path()?.segment()?.type_arg_list()?;
+    let mut result = Vec::new();
+    for type_arg in type_arg_list.type_args() {
+        let type_arg: ast::TypeArg = type_arg;
+        result.push(type_arg.type_ref()?);
+    }
+    Some(result)
+}
+// FIXME: This should be a general utility (not even just for assists)
+fn substitute_type_params<N: AstNode>(
+    db: &impl HirDatabase,
+    node: hir::InFile<N>,
+    substs: &HashMap<hir::TypeParam, ast::TypeRef>,
+) -> N {
+    let type_param_replacements = node
+        .value
+        .syntax()
+        .descendants()
+        .filter_map(ast::TypeRef::cast)
+        .filter_map(|n| {
+            let path = match &n {
+                ast::TypeRef::PathType(path_type) => path_type.path()?,
+                _ => return None,
+            };
+            let analyzer = hir::SourceAnalyzer::new(db, node.with_value(n.syntax()), None);
+            let resolution = analyzer.resolve_path(db, &path)?;
+            match resolution {
+                hir::PathResolution::TypeParam(tp) => Some((n, substs.get(&tp)?.clone())),
+                _ => None,
+            }
+        })
+        .collect::<Vec<_>>();
+    if type_param_replacements.is_empty() {
+        node.value
+    } else {
+        edit::replace_descendants(&node.value, type_param_replacements.into_iter())
+    }
+}
 /// Given an `ast::ImplBlock`, resolves the target trait (the one being
 /// implemented) to a `ast::TraitDef`.
 fn resolve_target_trait_def(
    db: &impl HirDatabase,
    analyzer: &hir::SourceAnalyzer,
    impl_block: &ast::ImplBlock,
-) -> Option<ast::TraitDef> {
+) -> Option<(hir::Trait, ast::TraitDef)> {
    let ast_path = impl_block
        .target_trait()
        .map(|it| it.syntax().clone())
@@ -174,7 +241,9 @@ fn resolve_target_trait_def(
        .path()?;
    match analyzer.resolve_path(db, &ast_path) {
-        Some(hir::PathResolution::Def(hir::ModuleDef::Trait(def))) => Some(def.source(db).value),
+        Some(hir::PathResolution::Def(hir::ModuleDef::Trait(def))) => {
+            Some((def, def.source(db).value))
+        }
        _ => None,
    }
 }
@@ -281,6 +350,40 @@ impl Foo for S {
    }
    #[test]
+    fn fill_in_type_params_1() {
+        check_assist(
+            add_missing_impl_members,
+            "
+trait Foo<T> { fn foo(&self, t: T) -> &T; }
+struct S;
+impl Foo<u32> for S { <|> }",
+            "
+trait Foo<T> { fn foo(&self, t: T) -> &T; }
+struct S;
+impl Foo<u32> for S {
+    <|>fn foo(&self, t: u32) -> &u32 { unimplemented!() }
+}",
+        );
+    }
+    #[test]
+    fn fill_in_type_params_2() {
+        check_assist(
+            add_missing_impl_members,
+            "
+trait Foo<T> { fn foo(&self, t: T) -> &T; }
+struct S;
+impl<U> Foo<U> for S { <|> }",
+            "
+trait Foo<T> { fn foo(&self, t: T) -> &T; }
+struct S;
+impl<U> Foo<U> for S {
+    <|>fn foo(&self, t: U) -> &U { unimplemented!() }
+}",
+        );
+    }
+    #[test]
    fn test_cursor_after_empty_impl_block() {
        check_assist(
            add_missing_impl_members,
diff --git a/crates/ra_assists/src/doc_tests/generated.rs b/crates/ra_assists/src/doc_tests/generated.rs
index 4586eeb59..7d84dc8fb 100644
--- a/crates/ra_assists/src/doc_tests/generated.rs
+++ b/crates/ra_assists/src/doc_tests/generated.rs
@@ -101,26 +101,26 @@ fn doctest_add_impl_default_members() {
    check(
        "add_impl_default_members",
        r#####"
-trait T {
+trait Trait {
    Type X;
    fn foo(&self);
    fn bar(&self) {}
 }
-impl T for () {
+impl Trait for () {
    Type X = ();
    fn foo(&self) {}<|>
 }
 "#####,
        r#####"
-trait T {
+trait Trait {
    Type X;
    fn foo(&self);
    fn bar(&self) {}
 }
-impl T for () {
+impl Trait for () {
    Type X = ();
    fn foo(&self) {}
    fn bar(&self) {}
@@ -135,25 +135,25 @@ fn doctest_add_impl_missing_members() {
    check(
        "add_impl_missing_members",
        r#####"
-trait T {
+trait Trait<T> {
    Type X;
-    fn foo(&self);
+    fn foo(&self) -> T;
    fn bar(&self) {}
 }
-impl T for () {<|>
+impl Trait<u32> for () {<|>
 }
 "#####,
        r#####"
-trait T {
+trait Trait<T> {
    Type X;
-    fn foo(&self);
+    fn foo(&self) -> T;
    fn bar(&self) {}
 }
-impl T for () {
+impl Trait<u32> for () {
-    fn foo(&self) { unimplemented!() }
+    fn foo(&self) -> u32 { unimplemented!() }
 }
 "#####,
diff --git a/crates/ra_hir/src/code_model.rs b/crates/ra_hir/src/code_model.rs
index bcfc0d03e..488f74cfb 100644
--- a/crates/ra_hir/src/code_model.rs
+++ b/crates/ra_hir/src/code_model.rs
@@ -118,7 +118,7 @@ impl_froms!(
    BuiltinType
 );
-pub use hir_def::attr::Attrs;
+pub use hir_def::{attr::Attrs, visibility::Visibility};
 impl Module {
    pub(crate) fn new(krate: Crate, crate_module_id: LocalModuleId) -> Module {
@@ -255,6 +255,15 @@ impl StructField {
    }
 }
+impl HasVisibility for StructField {
+    fn visibility(&self, db: &impl HirDatabase) -> Visibility {
+        let variant_data = self.parent.variant_data(db);
+        let visibility = &variant_data.fields()[self.id].visibility;
+        let parent_id: hir_def::VariantId = self.parent.into();
+        visibility.resolve(db, &parent_id.resolver(db))
+    }
+}
 #[derive(Debug, Clone, Copy, PartialEq, Eq, Hash)]
 pub struct Struct {
    pub(crate) id: StructId,
@@ -644,6 +653,17 @@ impl_froms!(
    Const
 );
+impl GenericDef {
+    pub fn params(self, db: &impl HirDatabase) -> Vec<TypeParam> {
+        let generics: Arc<hir_def::generics::GenericParams> = db.generic_params(self.into());
+        generics
+            .types
+            .iter()
+            .map(|(local_id, _)| TypeParam { id: TypeParamId { parent: self.into(), local_id } })
+            .collect()
+    }
+}
 #[derive(Clone, Copy, Debug, PartialEq, Eq, Hash)]
 pub struct Local {
    pub(crate) parent: DefWithBody,
@@ -1030,3 +1050,11 @@ impl<T: Into<AttrDef> + Copy> Docs for T {
        db.documentation(def.into())
    }
 }
+pub trait HasVisibility {
+    fn visibility(&self, db: &impl HirDatabase) -> Visibility;
+    fn is_visible_from(&self, db: &impl HirDatabase, module: Module) -> bool {
+        let vis = self.visibility(db);
+        vis.is_visible_from(db, module.id)
+    }
+}
diff --git a/crates/ra_hir/src/lib.rs b/crates/ra_hir/src/lib.rs
index 0008a8858..3d13978d4 100644
--- a/crates/ra_hir/src/lib.rs
+++ b/crates/ra_hir/src/lib.rs
@@ -40,8 +40,8 @@ mod from_source;
 pub use crate::{
    code_model::{
        Adt, AssocItem, AttrDef, Const, Crate, CrateDependency, DefWithBody, Docs, Enum,
-        EnumVariant, FieldSource, Function, GenericDef, HasAttrs, ImplBlock, Local, MacroDef,
+        EnumVariant, FieldSource, Function, GenericDef, HasAttrs, HasVisibility, ImplBlock, Local,
-        Module, ModuleDef, ScopeDef, Static, Struct, StructField, Trait, Type, TypeAlias,
+        MacroDef, Module, ModuleDef, ScopeDef, Static, Struct, StructField, Trait, Type, TypeAlias,
        TypeParam, Union, VariantDef,
    },
    from_source::FromSource,
diff --git a/crates/ra_hir_def/src/adt.rs b/crates/ra_hir_def/src/adt.rs
index d9ea693e3..aac5f3e15 100644
--- a/crates/ra_hir_def/src/adt.rs
+++ b/crates/ra_hir_def/src/adt.rs
@@ -9,11 +9,12 @@ use hir_expand::{
 };
 use ra_arena::{map::ArenaMap, Arena};
 use ra_prof::profile;
-use ra_syntax::ast::{self, NameOwner, TypeAscriptionOwner};
+use ra_syntax::ast::{self, NameOwner, TypeAscriptionOwner, VisibilityOwner};
 use crate::{
-    db::DefDatabase, src::HasChildSource, src::HasSource, trace::Trace, type_ref::TypeRef, EnumId,
+    db::DefDatabase, src::HasChildSource, src::HasSource, trace::Trace, type_ref::TypeRef,
-    LocalEnumVariantId, LocalStructFieldId, Lookup, StructId, UnionId, VariantId,
+    visibility::RawVisibility, EnumId, LocalEnumVariantId, LocalStructFieldId, Lookup, StructId,
+    UnionId, VariantId,
 };
 /// Note that we use `StructData` for unions as well!
@@ -47,13 +48,14 @@ pub enum VariantData {
 pub struct StructFieldData {
    pub name: Name,
    pub type_ref: TypeRef,
+    pub visibility: RawVisibility,
 }
 impl StructData {
    pub(crate) fn struct_data_query(db: &impl DefDatabase, id: StructId) -> Arc<StructData> {
        let src = id.lookup(db).source(db);
        let name = src.value.name().map_or_else(Name::missing, |n| n.as_name());
-        let variant_data = VariantData::new(src.value.kind());
+        let variant_data = VariantData::new(db, src.map(|s| s.kind()));
        let variant_data = Arc::new(variant_data);
        Arc::new(StructData { name, variant_data })
    }
@@ -61,10 +63,12 @@ impl StructData {
        let src = id.lookup(db).source(db);
        let name = src.value.name().map_or_else(Name::missing, |n| n.as_name());
        let variant_data = VariantData::new(
-            src.value
+            db,
-                .record_field_def_list()
+            src.map(|s| {
-                .map(ast::StructKind::Record)
+                s.record_field_def_list()
-                .unwrap_or(ast::StructKind::Unit),
+                    .map(ast::StructKind::Record)
+                    .unwrap_or(ast::StructKind::Unit)
+            }),
        );
        let variant_data = Arc::new(variant_data);
        Arc::new(StructData { name, variant_data })
@@ -77,7 +81,7 @@ impl EnumData {
        let src = e.lookup(db).source(db);
        let name = src.value.name().map_or_else(Name::missing, |n| n.as_name());
        let mut trace = Trace::new_for_arena();
-        lower_enum(&mut trace, &src.value);
+        lower_enum(db, &mut trace, &src);
        Arc::new(EnumData { name, variants: trace.into_arena() })
    }
@@ -93,30 +97,31 @@ impl HasChildSource for EnumId {
    fn child_source(&self, db: &impl DefDatabase) -> InFile<ArenaMap<Self::ChildId, Self::Value>> {
        let src = self.lookup(db).source(db);
        let mut trace = Trace::new_for_map();
-        lower_enum(&mut trace, &src.value);
+        lower_enum(db, &mut trace, &src);
        src.with_value(trace.into_map())
    }
 }
 fn lower_enum(
+    db: &impl DefDatabase,
    trace: &mut Trace<LocalEnumVariantId, EnumVariantData, ast::EnumVariant>,
-    ast: &ast::EnumDef,
+    ast: &InFile<ast::EnumDef>,
 ) {
-    for var in ast.variant_list().into_iter().flat_map(|it| it.variants()) {
+    for var in ast.value.variant_list().into_iter().flat_map(|it| it.variants()) {
        trace.alloc(
            || var.clone(),
            || EnumVariantData {
                name: var.name().map_or_else(Name::missing, |it| it.as_name()),
-                variant_data: Arc::new(VariantData::new(var.kind())),
+                variant_data: Arc::new(VariantData::new(db, ast.with_value(var.kind()))),
            },
        );
    }
 }
 impl VariantData {
-    fn new(flavor: ast::StructKind) -> Self {
+    fn new(db: &impl DefDatabase, flavor: InFile<ast::StructKind>) -> Self {
        let mut trace = Trace::new_for_arena();
-        match lower_struct(&mut trace, &flavor) {
+        match lower_struct(db, &mut trace, &flavor) {
            StructKind::Tuple => VariantData::Tuple(trace.into_arena()),
            StructKind::Record => VariantData::Record(trace.into_arena()),
            StructKind::Unit => VariantData::Unit,
@@ -163,7 +168,7 @@ impl HasChildSource for VariantId {
            }),
        };
        let mut trace = Trace::new_for_map();
-        lower_struct(&mut trace, &src.value);
+        lower_struct(db, &mut trace, &src);
        src.with_value(trace.into_map())
    }
 }
@@ -175,14 +180,15 @@ enum StructKind {
 }
 fn lower_struct(
+    db: &impl DefDatabase,
    trace: &mut Trace<
        LocalStructFieldId,
        StructFieldData,
        Either<ast::TupleFieldDef, ast::RecordFieldDef>,
    >,
-    ast: &ast::StructKind,
+    ast: &InFile<ast::StructKind>,
 ) -> StructKind {
-    match ast {
+    match &ast.value {
        ast::StructKind::Tuple(fl) => {
            for (i, fd) in fl.fields().enumerate() {
                trace.alloc(
@@ -190,6 +196,7 @@ fn lower_struct(
                    || StructFieldData {
                        name: Name::new_tuple_field(i),
                        type_ref: TypeRef::from_ast_opt(fd.type_ref()),
+                        visibility: RawVisibility::from_ast(db, ast.with_value(fd.visibility())),
                    },
                );
            }
@@ -202,6 +209,7 @@ fn lower_struct(
                    || StructFieldData {
                        name: fd.name().map(|n| n.as_name()).unwrap_or_else(Name::missing),
                        type_ref: TypeRef::from_ast_opt(fd.ascribed_type()),
+                        visibility: RawVisibility::from_ast(db, ast.with_value(fd.visibility())),
                    },
                );
            }
diff --git a/crates/ra_hir_def/src/body/lower.rs b/crates/ra_hir_def/src/body/lower.rs
index 5323af097..fc3a028e0 100644
--- a/crates/ra_hir_def/src/body/lower.rs
+++ b/crates/ra_hir_def/src/body/lower.rs
@@ -543,7 +543,10 @@ where
            };
            self.body.item_scope.define_def(def);
            if let Some(name) = name {
-                self.body.item_scope.push_res(name.as_name(), def.into());
+                let vis = crate::visibility::Visibility::Public; // FIXME determine correctly
+                self.body
+                    .item_scope
+                    .push_res(name.as_name(), crate::per_ns::PerNs::from_def(def, vis));
            }
        }
    }
diff --git a/crates/ra_hir_def/src/item_scope.rs b/crates/ra_hir_def/src/item_scope.rs
index b0288ee8d..fe7bb9779 100644
--- a/crates/ra_hir_def/src/item_scope.rs
+++ b/crates/ra_hir_def/src/item_scope.rs
@@ -5,7 +5,10 @@ use hir_expand::name::Name;
 use once_cell::sync::Lazy;
 use rustc_hash::FxHashMap;
-use crate::{per_ns::PerNs, AdtId, BuiltinType, ImplId, MacroDefId, ModuleDefId, TraitId};
+use crate::{
+    per_ns::PerNs, visibility::Visibility, AdtId, BuiltinType, ImplId, MacroDefId, ModuleDefId,
+    TraitId,
+};
 #[derive(Debug, Default, PartialEq, Eq)]
 pub struct ItemScope {
@@ -30,7 +33,7 @@ pub struct ItemScope {
 static BUILTIN_SCOPE: Lazy<FxHashMap<Name, PerNs>> = Lazy::new(|| {
    BuiltinType::ALL
        .iter()
-        .map(|(name, ty)| (name.clone(), PerNs::types(ty.clone().into())))
+        .map(|(name, ty)| (name.clone(), PerNs::types(ty.clone().into(), Visibility::Public)))
        .collect()
 });
@@ -144,8 +147,8 @@ impl ItemScope {
        changed
    }
-    pub(crate) fn collect_resolutions(&self) -> Vec<(Name, PerNs)> {
+    pub(crate) fn resolutions<'a>(&'a self) -> impl Iterator<Item = (Name, PerNs)> + 'a {
-        self.visible.iter().map(|(name, res)| (name.clone(), res.clone())).collect()
+        self.visible.iter().map(|(name, res)| (name.clone(), res.clone()))
    }
    pub(crate) fn collect_legacy_macros(&self) -> FxHashMap<Name, MacroDefId> {
@@ -153,20 +156,20 @@ impl ItemScope {
    }
 }
-impl From<ModuleDefId> for PerNs {
+impl PerNs {
-    fn from(def: ModuleDefId) -> PerNs {
+    pub(crate) fn from_def(def: ModuleDefId, v: Visibility) -> PerNs {
        match def {
-            ModuleDefId::ModuleId(_) => PerNs::types(def),
+            ModuleDefId::ModuleId(_) => PerNs::types(def, v),
-            ModuleDefId::FunctionId(_) => PerNs::values(def),
+            ModuleDefId::FunctionId(_) => PerNs::values(def, v),
            ModuleDefId::AdtId(adt) => match adt {
-                AdtId::StructId(_) | AdtId::UnionId(_) => PerNs::both(def, def),
+                AdtId::StructId(_) | AdtId::UnionId(_) => PerNs::both(def, def, v),
-                AdtId::EnumId(_) => PerNs::types(def),
+                AdtId::EnumId(_) => PerNs::types(def, v),
            },
-            ModuleDefId::EnumVariantId(_) => PerNs::both(def, def),
+            ModuleDefId::EnumVariantId(_) => PerNs::both(def, def, v),
-            ModuleDefId::ConstId(_) | ModuleDefId::StaticId(_) => PerNs::values(def),
+            ModuleDefId::ConstId(_) | ModuleDefId::StaticId(_) => PerNs::values(def, v),
-            ModuleDefId::TraitId(_) => PerNs::types(def),
+            ModuleDefId::TraitId(_) => PerNs::types(def, v),
-            ModuleDefId::TypeAliasId(_) => PerNs::types(def),
+            ModuleDefId::TypeAliasId(_) => PerNs::types(def, v),
-            ModuleDefId::BuiltinType(_) => PerNs::types(def),
+            ModuleDefId::BuiltinType(_) => PerNs::types(def, v),
        }
    }
 }
diff --git a/crates/ra_hir_def/src/lib.rs b/crates/ra_hir_def/src/lib.rs
index f6c7f38d1..61f044ecf 100644
--- a/crates/ra_hir_def/src/lib.rs
+++ b/crates/ra_hir_def/src/lib.rs
@@ -36,6 +36,8 @@ pub mod nameres;
 pub mod src;
 pub mod child_by_source;
+pub mod visibility;
 #[cfg(test)]
 mod test_db;
 #[cfg(test)]
diff --git a/crates/ra_hir_def/src/nameres/collector.rs b/crates/ra_hir_def/src/nameres/collector.rs
index b9f40d3dd..8a22b0585 100644
--- a/crates/ra_hir_def/src/nameres/collector.rs
+++ b/crates/ra_hir_def/src/nameres/collector.rs
@@ -24,6 +24,7 @@ use crate::{
    },
    path::{ModPath, PathKind},
    per_ns::PerNs,
+    visibility::Visibility,
    AdtId, AstId, ConstLoc, ContainerId, EnumLoc, EnumVariantId, FunctionLoc, ImplLoc, Intern,
    LocalModuleId, ModuleDefId, ModuleId, StaticLoc, StructLoc, TraitLoc, TypeAliasLoc, UnionLoc,
 };
@@ -108,7 +109,7 @@ struct MacroDirective {
 struct DefCollector<'a, DB> {
    db: &'a DB,
    def_map: CrateDefMap,
-    glob_imports: FxHashMap<LocalModuleId, Vec<(LocalModuleId, raw::Import)>>,
+    glob_imports: FxHashMap<LocalModuleId, Vec<(LocalModuleId, Visibility)>>,
    unresolved_imports: Vec<ImportDirective>,
    resolved_imports: Vec<ImportDirective>,
    unexpanded_macros: Vec<MacroDirective>,
@@ -214,7 +215,11 @@ where
        // In Rust, `#[macro_export]` macros are unconditionally visible at the
        // crate root, even if the parent modules is **not** visible.
        if export {
-            self.update(self.def_map.root, &[(name, PerNs::macros(macro_))]);
+            self.update(
+                self.def_map.root,
+                &[(name, PerNs::macros(macro_, Visibility::Public))],
+                Visibility::Public,
+            );
        }
    }
@@ -348,9 +353,12 @@ where
    fn record_resolved_import(&mut self, directive: &ImportDirective) {
        let module_id = directive.module_id;
-        let import_id = directive.import_id;
        let import = &directive.import;
        let def = directive.status.namespaces();
+        let vis = self
+            .def_map
+            .resolve_visibility(self.db, module_id, &directive.import.visibility)
+            .unwrap_or(Visibility::Public);
        if import.is_glob {
            log::debug!("glob import: {:?}", import);
@@ -366,9 +374,16 @@ where
                        let scope = &item_map[m.local_id].scope;
                        // Module scoped macros is included
-                        let items = scope.collect_resolutions();
+                        let items = scope
+                            .resolutions()
-                        self.update(module_id, &items);
+                            // only keep visible names...
+                            .map(|(n, res)| {
+                                (n, res.filter_visibility(|v| v.is_visible_from_other_crate()))
+                            })
+                            .filter(|(_, res)| !res.is_none())
+                            .collect::<Vec<_>>();
+                        self.update(module_id, &items, vis);
                    } else {
                        // glob import from same crate => we do an initial
                        // import, and then need to propagate any further
@@ -376,13 +391,25 @@ where
                        let scope = &self.def_map[m.local_id].scope;
                        // Module scoped macros is included
-                        let items = scope.collect_resolutions();
+                        let items = scope
+                            .resolutions()
-                        self.update(module_id, &items);
+                            // only keep visible names...
+                            .map(|(n, res)| {
+                                (
+                                    n,
+                                    res.filter_visibility(|v| {
+                                        v.is_visible_from_def_map(&self.def_map, module_id)
+                                    }),
+                                )
+                            })
+                            .filter(|(_, res)| !res.is_none())
+                            .collect::<Vec<_>>();
+                        self.update(module_id, &items, vis);
                        // record the glob import in case we add further items
                        let glob = self.glob_imports.entry(m.local_id).or_default();
-                        if !glob.iter().any(|it| *it == (module_id, import_id)) {
+                        if !glob.iter().any(|(mid, _)| *mid == module_id) {
-                            glob.push((module_id, import_id));
+                            glob.push((module_id, vis));
                        }
                    }
                }
@@ -396,11 +423,11 @@ where
                        .map(|(local_id, variant_data)| {
                            let name = variant_data.name.clone();
                            let variant = EnumVariantId { parent: e, local_id };
-                            let res = PerNs::both(variant.into(), variant.into());
+                            let res = PerNs::both(variant.into(), variant.into(), vis);
                            (name, res)
                        })
                        .collect::<Vec<_>>();
-                    self.update(module_id, &resolutions);
+                    self.update(module_id, &resolutions, vis);
                }
                Some(d) => {
                    log::debug!("glob import {:?} from non-module/enum {:?}", import, d);
@@ -422,21 +449,24 @@ where
                        }
                    }
-                    self.update(module_id, &[(name, def)]);
+                    self.update(module_id, &[(name, def)], vis);
                }
                None => tested_by!(bogus_paths),
            }
        }
    }
-    fn update(&mut self, module_id: LocalModuleId, resolutions: &[(Name, PerNs)]) {
+    fn update(&mut self, module_id: LocalModuleId, resolutions: &[(Name, PerNs)], vis: Visibility) {
-        self.update_recursive(module_id, resolutions, 0)
+        self.update_recursive(module_id, resolutions, vis, 0)
    }
    fn update_recursive(
        &mut self,
        module_id: LocalModuleId,
        resolutions: &[(Name, PerNs)],
+        // All resolutions are imported with this visibility; the visibilies in
+        // the `PerNs` values are ignored and overwritten
+        vis: Visibility,
        depth: usize,
    ) {
        if depth > 100 {
@@ -446,7 +476,7 @@ where
        let scope = &mut self.def_map.modules[module_id].scope;
        let mut changed = false;
        for (name, res) in resolutions {
-            changed |= scope.push_res(name.clone(), *res);
+            changed |= scope.push_res(name.clone(), res.with_visibility(vis));
        }
        if !changed {
@@ -459,9 +489,13 @@ where
            .flat_map(|v| v.iter())
            .cloned()
            .collect::<Vec<_>>();
-        for (glob_importing_module, _glob_import) in glob_imports {
+        for (glob_importing_module, glob_import_vis) in glob_imports {
-            // We pass the glob import so that the tracked import in those modules is that glob import
+            // we know all resolutions have the same visibility (`vis`), so we
-            self.update_recursive(glob_importing_module, resolutions, depth + 1);
+            // just need to check that once
+            if !vis.is_visible_from_def_map(&self.def_map, glob_importing_module) {
+                continue;
+            }
+            self.update_recursive(glob_importing_module, resolutions, glob_import_vis, depth + 1);
        }
    }
@@ -633,9 +667,13 @@ where
        let is_macro_use = attrs.by_key("macro_use").exists();
        match module {
            // inline module, just recurse
-            raw::ModuleData::Definition { name, items, ast_id } => {
+            raw::ModuleData::Definition { name, visibility, items, ast_id } => {
-                let module_id =
+                let module_id = self.push_child_module(
-                    self.push_child_module(name.clone(), AstId::new(self.file_id, *ast_id), None);
+                    name.clone(),
+                    AstId::new(self.file_id, *ast_id),
+                    None,
+                    &visibility,
+                );
                ModCollector {
                    def_collector: &mut *self.def_collector,
@@ -650,7 +688,7 @@ where
                }
            }
            // out of line module, resolve, parse and recurse
-            raw::ModuleData::Declaration { name, ast_id } => {
+            raw::ModuleData::Declaration { name, visibility, ast_id } => {
                let ast_id = AstId::new(self.file_id, *ast_id);
                match self.mod_dir.resolve_declaration(
                    self.def_collector.db,
@@ -659,7 +697,12 @@ where
                    path_attr,
                ) {
                    Ok((file_id, mod_dir)) => {
-                        let module_id = self.push_child_module(name.clone(), ast_id, Some(file_id));
+                        let module_id = self.push_child_module(
+                            name.clone(),
+                            ast_id,
+                            Some(file_id),
+                            &visibility,
+                        );
                        let raw_items = self.def_collector.db.raw_items(file_id.into());
                        ModCollector {
                            def_collector: &mut *self.def_collector,
@@ -690,7 +733,13 @@ where
        name: Name,
        declaration: AstId<ast::Module>,
        definition: Option<FileId>,
+        visibility: &crate::visibility::RawVisibility,
    ) -> LocalModuleId {
+        let vis = self
+            .def_collector
+            .def_map
+            .resolve_visibility(self.def_collector.db, self.module_id, visibility)
+            .unwrap_or(Visibility::Public);
        let modules = &mut self.def_collector.def_map.modules;
        let res = modules.alloc(ModuleData::default());
        modules[res].parent = Some(self.module_id);
@@ -702,7 +751,7 @@ where
        let module = ModuleId { krate: self.def_collector.def_map.krate, local_id: res };
        let def: ModuleDefId = module.into();
        self.def_collector.def_map.modules[self.module_id].scope.define_def(def);
-        self.def_collector.update(self.module_id, &[(name, def.into())]);
+        self.def_collector.update(self.module_id, &[(name, PerNs::from_def(def, vis))], vis);
        res
    }
@@ -716,6 +765,7 @@ where
        let name = def.name.clone();
        let container = ContainerId::ModuleId(module);
+        let vis = &def.visibility;
        let def: ModuleDefId = match def.kind {
            raw::DefKind::Function(ast_id) => FunctionLoc {
                container: container.into(),
@@ -761,7 +811,12 @@ where
            .into(),
        };
        self.def_collector.def_map.modules[self.module_id].scope.define_def(def);
-        self.def_collector.update(self.module_id, &[(name, def.into())])
+        let vis = self
+            .def_collector
+            .def_map
+            .resolve_visibility(self.def_collector.db, self.module_id, vis)
+            .unwrap_or(Visibility::Public);
+        self.def_collector.update(self.module_id, &[(name, PerNs::from_def(def, vis))], vis)
    }
    fn collect_derives(&mut self, attrs: &Attrs, def: &raw::DefData) {
diff --git a/crates/ra_hir_def/src/nameres/path_resolution.rs b/crates/ra_hir_def/src/nameres/path_resolution.rs
index 695014c7b..fd6422d60 100644
--- a/crates/ra_hir_def/src/nameres/path_resolution.rs
+++ b/crates/ra_hir_def/src/nameres/path_resolution.rs
@@ -21,6 +21,7 @@ use crate::{
    nameres::{BuiltinShadowMode, CrateDefMap},
    path::{ModPath, PathKind},
    per_ns::PerNs,
+    visibility::{RawVisibility, Visibility},
    AdtId, CrateId, EnumVariantId, LocalModuleId, ModuleDefId, ModuleId,
 };
@@ -61,7 +62,35 @@ impl ResolvePathResult {
 impl CrateDefMap {
    pub(super) fn resolve_name_in_extern_prelude(&self, name: &Name) -> PerNs {
-        self.extern_prelude.get(name).map_or(PerNs::none(), |&it| PerNs::types(it))
+        self.extern_prelude
+            .get(name)
+            .map_or(PerNs::none(), |&it| PerNs::types(it, Visibility::Public))
+    }
+    pub(crate) fn resolve_visibility(
+        &self,
+        db: &impl DefDatabase,
+        original_module: LocalModuleId,
+        visibility: &RawVisibility,
+    ) -> Option<Visibility> {
+        match visibility {
+            RawVisibility::Module(path) => {
+                let (result, remaining) =
+                    self.resolve_path(db, original_module, &path, BuiltinShadowMode::Module);
+                if remaining.is_some() {
+                    return None;
+                }
+                let types = result.take_types()?;
+                match types {
+                    ModuleDefId::ModuleId(m) => Some(Visibility::Module(m)),
+                    _ => {
+                        // error: visibility needs to refer to module
+                        None
+                    }
+                }
+            }
+            RawVisibility::Public => Some(Visibility::Public),
+        }
    }
    // Returns Yes if we are sure that additions to `ItemMap` wouldn't change
@@ -88,17 +117,21 @@ impl CrateDefMap {
            PathKind::DollarCrate(krate) => {
                if krate == self.krate {
                    tested_by!(macro_dollar_crate_self);
-                    PerNs::types(ModuleId { krate: self.krate, local_id: self.root }.into())
+                    PerNs::types(
+                        ModuleId { krate: self.krate, local_id: self.root }.into(),
+                        Visibility::Public,
+                    )
                } else {
                    let def_map = db.crate_def_map(krate);
                    let module = ModuleId { krate, local_id: def_map.root };
                    tested_by!(macro_dollar_crate_other);
-                    PerNs::types(module.into())
+                    PerNs::types(module.into(), Visibility::Public)
                }
            }
-            PathKind::Crate => {
+            PathKind::Crate => PerNs::types(
-                PerNs::types(ModuleId { krate: self.krate, local_id: self.root }.into())
+                ModuleId { krate: self.krate, local_id: self.root }.into(),
-            }
+                Visibility::Public,
+            ),
            // plain import or absolute path in 2015: crate-relative with
            // fallback to extern prelude (with the simplification in
            // rust-lang/rust#57745)
@@ -126,7 +159,10 @@ impl CrateDefMap {
                let m = successors(Some(original_module), |m| self.modules[*m].parent)
                    .nth(lvl as usize);
                if let Some(local_id) = m {
-                    PerNs::types(ModuleId { krate: self.krate, local_id }.into())
+                    PerNs::types(
+                        ModuleId { krate: self.krate, local_id }.into(),
+                        Visibility::Public,
+                    )
                } else {
                    log::debug!("super path in root module");
                    return ResolvePathResult::empty(ReachedFixedPoint::Yes);
@@ -140,7 +176,7 @@ impl CrateDefMap {
                };
                if let Some(def) = self.extern_prelude.get(&segment) {
                    log::debug!("absolute path {:?} resolved to crate {:?}", path, def);
-                    PerNs::types(*def)
+                    PerNs::types(*def, Visibility::Public)
                } else {
                    return ResolvePathResult::empty(ReachedFixedPoint::No); // extern crate declarations can add to the extern prelude
                }
@@ -148,7 +184,7 @@ impl CrateDefMap {
        };
        for (i, segment) in segments {
-            let curr = match curr_per_ns.take_types() {
+            let (curr, vis) = match curr_per_ns.take_types_vis() {
                Some(r) => r,
                None => {
                    // we still have path segments left, but the path so far
@@ -189,11 +225,11 @@ impl CrateDefMap {
                    match enum_data.variant(&segment) {
                        Some(local_id) => {
                            let variant = EnumVariantId { parent: e, local_id };
-                            PerNs::both(variant.into(), variant.into())
+                            PerNs::both(variant.into(), variant.into(), Visibility::Public)
                        }
                        None => {
                            return ResolvePathResult::with(
-                                PerNs::types(e.into()),
+                                PerNs::types(e.into(), vis),
                                ReachedFixedPoint::Yes,
                                Some(i),
                                Some(self.krate),
@@ -211,7 +247,7 @@ impl CrateDefMap {
                    );
                    return ResolvePathResult::with(
-                        PerNs::types(s),
+                        PerNs::types(s, vis),
                        ReachedFixedPoint::Yes,
                        Some(i),
                        Some(self.krate),
@@ -235,11 +271,15 @@ impl CrateDefMap {
        //  - current module / scope
        //  - extern prelude
        //  - std prelude
-        let from_legacy_macro =
+        let from_legacy_macro = self[module]
-            self[module].scope.get_legacy_macro(name).map_or_else(PerNs::none, PerNs::macros);
+            .scope
+            .get_legacy_macro(name)
+            .map_or_else(PerNs::none, |m| PerNs::macros(m, Visibility::Public));
        let from_scope = self[module].scope.get(name, shadow);
-        let from_extern_prelude =
+        let from_extern_prelude = self
-            self.extern_prelude.get(name).map_or(PerNs::none(), |&it| PerNs::types(it));
+            .extern_prelude
+            .get(name)
+            .map_or(PerNs::none(), |&it| PerNs::types(it, Visibility::Public));
        let from_prelude = self.resolve_in_prelude(db, name, shadow);
        from_legacy_macro.or(from_scope).or(from_extern_prelude).or(from_prelude)
diff --git a/crates/ra_hir_def/src/nameres/raw.rs b/crates/ra_hir_def/src/nameres/raw.rs
index 73dc08745..fac1169ef 100644
--- a/crates/ra_hir_def/src/nameres/raw.rs
+++ b/crates/ra_hir_def/src/nameres/raw.rs
@@ -16,12 +16,15 @@ use hir_expand::{
 use ra_arena::{impl_arena_id, Arena, RawId};
 use ra_prof::profile;
 use ra_syntax::{
-    ast::{self, AttrsOwner, NameOwner},
+    ast::{self, AttrsOwner, NameOwner, VisibilityOwner},
    AstNode,
 };
 use test_utils::tested_by;
-use crate::{attr::Attrs, db::DefDatabase, path::ModPath, FileAstId, HirFileId, InFile};
+use crate::{
+    attr::Attrs, db::DefDatabase, path::ModPath, visibility::RawVisibility, FileAstId, HirFileId,
+    InFile,
+};
 /// `RawItems` is a set of top-level items in a file (except for impls).
 ///
@@ -122,8 +125,17 @@ impl_arena_id!(Module);
 #[derive(Debug, PartialEq, Eq)]
 pub(super) enum ModuleData {
-    Declaration { name: Name, ast_id: FileAstId<ast::Module> },
+    Declaration {
-    Definition { name: Name, ast_id: FileAstId<ast::Module>, items: Vec<RawItem> },
+        name: Name,
+        visibility: RawVisibility,
+        ast_id: FileAstId<ast::Module>,
+    },
+    Definition {
+        name: Name,
+        visibility: RawVisibility,
+        ast_id: FileAstId<ast::Module>,
+        items: Vec<RawItem>,
+    },
 }
 #[derive(Debug, Clone, Copy, PartialEq, Eq, Hash)]
@@ -138,6 +150,7 @@ pub struct ImportData {
    pub(super) is_prelude: bool,
    pub(super) is_extern_crate: bool,
    pub(super) is_macro_use: bool,
+    pub(super) visibility: RawVisibility,
 }
 #[derive(Debug, Clone, Copy, PartialEq, Eq, Hash)]
@@ -148,6 +161,7 @@ impl_arena_id!(Def);
 pub(super) struct DefData {
    pub(super) name: Name,
    pub(super) kind: DefKind,
+    pub(super) visibility: RawVisibility,
 }
 #[derive(Debug, PartialEq, Eq, Clone, Copy)]
@@ -218,6 +232,7 @@ impl RawItemsCollector {
    fn add_item(&mut self, current_module: Option<Module>, item: ast::ModuleItem) {
        let attrs = self.parse_attrs(&item);
+        let visibility = RawVisibility::from_ast_with_hygiene(item.visibility(), &self.hygiene);
        let (kind, name) = match item {
            ast::ModuleItem::Module(module) => {
                self.add_module(current_module, module);
@@ -266,7 +281,7 @@ impl RawItemsCollector {
        };
        if let Some(name) = name {
            let name = name.as_name();
-            let def = self.raw_items.defs.alloc(DefData { name, kind });
+            let def = self.raw_items.defs.alloc(DefData { name, kind, visibility });
            self.push_item(current_module, attrs, RawItemKind::Def(def));
        }
    }
@@ -277,10 +292,12 @@ impl RawItemsCollector {
            None => return,
        };
        let attrs = self.parse_attrs(&module);
+        let visibility = RawVisibility::from_ast_with_hygiene(module.visibility(), &self.hygiene);
        let ast_id = self.source_ast_id_map.ast_id(&module);
        if module.has_semi() {
-            let item = self.raw_items.modules.alloc(ModuleData::Declaration { name, ast_id });
+            let item =
+                self.raw_items.modules.alloc(ModuleData::Declaration { name, visibility, ast_id });
            self.push_item(current_module, attrs, RawItemKind::Module(item));
            return;
        }
@@ -288,6 +305,7 @@ impl RawItemsCollector {
        if let Some(item_list) = module.item_list() {
            let item = self.raw_items.modules.alloc(ModuleData::Definition {
                name,
+                visibility,
                ast_id,
                items: Vec::new(),
            });
@@ -302,6 +320,7 @@ impl RawItemsCollector {
        // FIXME: cfg_attr
        let is_prelude = use_item.has_atom_attr("prelude_import");
        let attrs = self.parse_attrs(&use_item);
+        let visibility = RawVisibility::from_ast_with_hygiene(use_item.visibility(), &self.hygiene);
        let mut buf = Vec::new();
        ModPath::expand_use_item(
@@ -315,6 +334,7 @@ impl RawItemsCollector {
                    is_prelude,
                    is_extern_crate: false,
                    is_macro_use: false,
+                    visibility: visibility.clone(),
                };
                buf.push(import_data);
            },
@@ -331,6 +351,8 @@ impl RawItemsCollector {
    ) {
        if let Some(name_ref) = extern_crate.name_ref() {
            let path = ModPath::from_name_ref(&name_ref);
+            let visibility =
+                RawVisibility::from_ast_with_hygiene(extern_crate.visibility(), &self.hygiene);
            let alias = extern_crate.alias().and_then(|a| a.name()).map(|it| it.as_name());
            let attrs = self.parse_attrs(&extern_crate);
            // FIXME: cfg_attr
@@ -342,6 +364,7 @@ impl RawItemsCollector {
                is_prelude: false,
                is_extern_crate: true,
                is_macro_use,
+                visibility,
            };
            self.push_import(current_module, attrs, import_data);
        }
diff --git a/crates/ra_hir_def/src/nameres/tests.rs b/crates/ra_hir_def/src/nameres/tests.rs
index ff474b53b..78bcdc850 100644
--- a/crates/ra_hir_def/src/nameres/tests.rs
+++ b/crates/ra_hir_def/src/nameres/tests.rs
@@ -12,8 +12,8 @@ use test_utils::covers;
 use crate::{db::DefDatabase, nameres::*, test_db::TestDB, LocalModuleId};
-fn def_map(fixtute: &str) -> String {
+fn def_map(fixture: &str) -> String {
-    let dm = compute_crate_def_map(fixtute);
+    let dm = compute_crate_def_map(fixture);
    render_crate_def_map(&dm)
 }
@@ -32,7 +32,7 @@ fn render_crate_def_map(map: &CrateDefMap) -> String {
        *buf += path;
        *buf += "\n";
-        let mut entries = map.modules[module].scope.collect_resolutions();
+        let mut entries: Vec<_> = map.modules[module].scope.resolutions().collect();
        entries.sort_by_key(|(name, _)| name.clone());
        for (name, def) in entries {
diff --git a/crates/ra_hir_def/src/nameres/tests/globs.rs b/crates/ra_hir_def/src/nameres/tests/globs.rs
index 5e24cb94d..71fa0abe8 100644
--- a/crates/ra_hir_def/src/nameres/tests/globs.rs
+++ b/crates/ra_hir_def/src/nameres/tests/globs.rs
@@ -74,6 +74,83 @@ fn glob_2() {
 }
 #[test]
+fn glob_privacy_1() {
+    let map = def_map(
+        "
+        //- /lib.rs
+        mod foo;
+        use foo::*;
+        //- /foo/mod.rs
+        pub mod bar;
+        pub use self::bar::*;
+        struct PrivateStructFoo;
+        //- /foo/bar.rs
+        pub struct Baz;
+        struct PrivateStructBar;
+        pub use super::*;
+        ",
+    );
+    assert_snapshot!(map, @r###"
+    crate
+    Baz: t v
+    bar: t
+    foo: t
+    
+    crate::foo
+    Baz: t v
+    PrivateStructFoo: t v
+    bar: t
+    
+    crate::foo::bar
+    Baz: t v
+    PrivateStructBar: t v
+    PrivateStructFoo: t v
+    bar: t
+    "###
+    );
+}
+#[test]
+fn glob_privacy_2() {
+    let map = def_map(
+        "
+        //- /lib.rs
+        mod foo;
+        use foo::*;
+        use foo::bar::*;
+        //- /foo/mod.rs
+        mod bar;
+        fn Foo() {};
+        pub struct Foo {};
+        //- /foo/bar.rs
+        pub(super) struct PrivateBaz;
+        struct PrivateBar;
+        pub(crate) struct PubCrateStruct;
+        ",
+    );
+    assert_snapshot!(map, @r###"
+    crate
+    Foo: t
+    PubCrateStruct: t v
+    foo: t
+    
+    crate::foo
+    Foo: t v
+    bar: t
+    
+    crate::foo::bar
+    PrivateBar: t v
+    PrivateBaz: t v
+    PubCrateStruct: t v
+    "###
+    );
+}
+#[test]
 fn glob_across_crates() {
    covers!(glob_across_crates);
    let map = def_map(
@@ -93,6 +170,26 @@ fn glob_across_crates() {
 }
 #[test]
+fn glob_privacy_across_crates() {
+    covers!(glob_across_crates);
+    let map = def_map(
+        "
+        //- /main.rs crate:main deps:test_crate
+        use test_crate::*;
+        //- /lib.rs crate:test_crate
+        pub struct Baz;
+        struct Foo;
+        ",
+    );
+    assert_snapshot!(map, @r###"
+   ⋮crate
+   ⋮Baz: t v
+    "###
+    );
+}
+#[test]
 fn glob_enum() {
    covers!(glob_enum);
    let map = def_map(
diff --git a/crates/ra_hir_def/src/nameres/tests/incremental.rs b/crates/ra_hir_def/src/nameres/tests/incremental.rs
index ef2e9435c..faeb7aa4d 100644
--- a/crates/ra_hir_def/src/nameres/tests/incremental.rs
+++ b/crates/ra_hir_def/src/nameres/tests/incremental.rs
@@ -116,7 +116,7 @@ fn typing_inside_a_macro_should_not_invalidate_def_map() {
        let events = db.log_executed(|| {
            let crate_def_map = db.crate_def_map(krate);
            let (_, module_data) = crate_def_map.modules.iter().last().unwrap();
-            assert_eq!(module_data.scope.collect_resolutions().len(), 1);
+            assert_eq!(module_data.scope.resolutions().collect::<Vec<_>>().len(), 1);
        });
        assert!(format!("{:?}", events).contains("crate_def_map"), "{:#?}", events)
    }
@@ -126,7 +126,7 @@ fn typing_inside_a_macro_should_not_invalidate_def_map() {
        let events = db.log_executed(|| {
            let crate_def_map = db.crate_def_map(krate);
            let (_, module_data) = crate_def_map.modules.iter().last().unwrap();
-            assert_eq!(module_data.scope.collect_resolutions().len(), 1);
+            assert_eq!(module_data.scope.resolutions().collect::<Vec<_>>().len(), 1);
        });
        assert!(!format!("{:?}", events).contains("crate_def_map"), "{:#?}", events)
    }
diff --git a/crates/ra_hir_def/src/per_ns.rs b/crates/ra_hir_def/src/per_ns.rs
index 3a5105028..6e435c8c1 100644
--- a/crates/ra_hir_def/src/per_ns.rs
+++ b/crates/ra_hir_def/src/per_ns.rs
@@ -5,13 +5,13 @@
 use hir_expand::MacroDefId;
-use crate::ModuleDefId;
+use crate::{visibility::Visibility, ModuleDefId};
-#[derive(Debug, Copy, Clone, PartialEq, Eq, Hash)]
+#[derive(Debug, Copy, Clone, PartialEq, Eq)]
 pub struct PerNs {
-    pub types: Option<ModuleDefId>,
+    pub types: Option<(ModuleDefId, Visibility)>,
-    pub values: Option<ModuleDefId>,
+    pub values: Option<(ModuleDefId, Visibility)>,
-    pub macros: Option<MacroDefId>,
+    pub macros: Option<(MacroDefId, Visibility)>,
 }
 impl Default for PerNs {
@@ -25,20 +25,20 @@ impl PerNs {
        PerNs { types: None, values: None, macros: None }
    }
-    pub fn values(t: ModuleDefId) -> PerNs {
+    pub fn values(t: ModuleDefId, v: Visibility) -> PerNs {
-        PerNs { types: None, values: Some(t), macros: None }
+        PerNs { types: None, values: Some((t, v)), macros: None }
    }
-    pub fn types(t: ModuleDefId) -> PerNs {
+    pub fn types(t: ModuleDefId, v: Visibility) -> PerNs {
-        PerNs { types: Some(t), values: None, macros: None }
+        PerNs { types: Some((t, v)), values: None, macros: None }
    }
-    pub fn both(types: ModuleDefId, values: ModuleDefId) -> PerNs {
+    pub fn both(types: ModuleDefId, values: ModuleDefId, v: Visibility) -> PerNs {
-        PerNs { types: Some(types), values: Some(values), macros: None }
+        PerNs { types: Some((types, v)), values: Some((values, v)), macros: None }
    }
-    pub fn macros(macro_: MacroDefId) -> PerNs {
+    pub fn macros(macro_: MacroDefId, v: Visibility) -> PerNs {
-        PerNs { types: None, values: None, macros: Some(macro_) }
+        PerNs { types: None, values: None, macros: Some((macro_, v)) }
    }
    pub fn is_none(&self) -> bool {
@@ -46,15 +46,35 @@ impl PerNs {
    }
    pub fn take_types(self) -> Option<ModuleDefId> {
+        self.types.map(|it| it.0)
+    }
+    pub fn take_types_vis(self) -> Option<(ModuleDefId, Visibility)> {
        self.types
    }
    pub fn take_values(self) -> Option<ModuleDefId> {
-        self.values
+        self.values.map(|it| it.0)
    }
    pub fn take_macros(self) -> Option<MacroDefId> {
-        self.macros
+        self.macros.map(|it| it.0)
+    }
+    pub fn filter_visibility(self, mut f: impl FnMut(Visibility) -> bool) -> PerNs {
+        PerNs {
+            types: self.types.filter(|(_, v)| f(*v)),
+            values: self.values.filter(|(_, v)| f(*v)),
+            macros: self.macros.filter(|(_, v)| f(*v)),
+        }
+    }
+    pub fn with_visibility(self, vis: Visibility) -> PerNs {
+        PerNs {
+            types: self.types.map(|(it, _)| (it, vis)),
+            values: self.values.map(|(it, _)| (it, vis)),
+            macros: self.macros.map(|(it, _)| (it, vis)),
+        }
    }
    pub fn or(self, other: PerNs) -> PerNs {
diff --git a/crates/ra_hir_def/src/resolver.rs b/crates/ra_hir_def/src/resolver.rs
index cf3c33d78..43dc751d9 100644
--- a/crates/ra_hir_def/src/resolver.rs
+++ b/crates/ra_hir_def/src/resolver.rs
@@ -19,6 +19,7 @@ use crate::{
    nameres::CrateDefMap,
    path::{ModPath, PathKind},
    per_ns::PerNs,
+    visibility::{RawVisibility, Visibility},
    AdtId, AssocContainerId, ConstId, ContainerId, DefWithBodyId, EnumId, EnumVariantId,
    FunctionId, GenericDefId, HasModule, ImplId, LocalModuleId, Lookup, ModuleDefId, ModuleId,
    StaticId, StructId, TraitId, TypeAliasId, TypeParamId, VariantId,
@@ -231,6 +232,23 @@ impl Resolver {
        Some(res)
    }
+    pub fn resolve_visibility(
+        &self,
+        db: &impl DefDatabase,
+        visibility: &RawVisibility,
+    ) -> Option<Visibility> {
+        match visibility {
+            RawVisibility::Module(_) => {
+                let (item_map, module) = match self.module() {
+                    Some(it) => it,
+                    None => return None,
+                };
+                item_map.resolve_visibility(db, module, visibility)
+            }
+            RawVisibility::Public => Some(Visibility::Public),
+        }
+    }
    pub fn resolve_path_in_value_ns(
        &self,
        db: &impl DefDatabase,
@@ -448,10 +466,10 @@ impl Scope {
                    f(name.clone(), ScopeDef::PerNs(def));
                });
                m.crate_def_map[m.module_id].scope.legacy_macros().for_each(|(name, macro_)| {
-                    f(name.clone(), ScopeDef::PerNs(PerNs::macros(macro_)));
+                    f(name.clone(), ScopeDef::PerNs(PerNs::macros(macro_, Visibility::Public)));
                });
                m.crate_def_map.extern_prelude.iter().for_each(|(name, &def)| {
-                    f(name.clone(), ScopeDef::PerNs(PerNs::types(def.into())));
+                    f(name.clone(), ScopeDef::PerNs(PerNs::types(def.into(), Visibility::Public)));
                });
                if let Some(prelude) = m.crate_def_map.prelude {
                    let prelude_def_map = db.crate_def_map(prelude.krate);
diff --git a/crates/ra_hir_def/src/visibility.rs b/crates/ra_hir_def/src/visibility.rs
new file mode 100644
index 000000000..d8296da4b
--- /dev/null
+++ b/crates/ra_hir_def/src/visibility.rs
@@ -0,0 +1,120 @@
+//! Defines hir-level representation of visibility (e.g. `pub` and `pub(crate)`).
+use hir_expand::{hygiene::Hygiene, InFile};
+use ra_syntax::ast;
+use crate::{
+    db::DefDatabase,
+    path::{ModPath, PathKind},
+    ModuleId,
+};
+/// Visibility of an item, not yet resolved.
+#[derive(Debug, Clone, PartialEq, Eq)]
+pub enum RawVisibility {
+    /// `pub(in module)`, `pub(crate)` or `pub(super)`. Also private, which is
+    /// equivalent to `pub(self)`.
+    Module(ModPath),
+    /// `pub`.
+    Public,
+}
+impl RawVisibility {
+    const fn private() -> RawVisibility {
+        let path = ModPath { kind: PathKind::Super(0), segments: Vec::new() };
+        RawVisibility::Module(path)
+    }
+    pub(crate) fn from_ast(
+        db: &impl DefDatabase,
+        node: InFile<Option<ast::Visibility>>,
+    ) -> RawVisibility {
+        Self::from_ast_with_hygiene(node.value, &Hygiene::new(db, node.file_id))
+    }
+    pub(crate) fn from_ast_with_hygiene(
+        node: Option<ast::Visibility>,
+        hygiene: &Hygiene,
+    ) -> RawVisibility {
+        let node = match node {
+            None => return RawVisibility::private(),
+            Some(node) => node,
+        };
+        match node.kind() {
+            ast::VisibilityKind::In(path) => {
+                let path = ModPath::from_src(path, hygiene);
+                let path = match path {
+                    None => return RawVisibility::private(),
+                    Some(path) => path,
+                };
+                RawVisibility::Module(path)
+            }
+            ast::VisibilityKind::PubCrate => {
+                let path = ModPath { kind: PathKind::Crate, segments: Vec::new() };
+                RawVisibility::Module(path)
+            }
+            ast::VisibilityKind::PubSuper => {
+                let path = ModPath { kind: PathKind::Super(1), segments: Vec::new() };
+                RawVisibility::Module(path)
+            }
+            ast::VisibilityKind::Pub => RawVisibility::Public,
+        }
+    }
+    pub fn resolve(
+        &self,
+        db: &impl DefDatabase,
+        resolver: &crate::resolver::Resolver,
+    ) -> Visibility {
+        // we fall back to public visibility (i.e. fail open) if the path can't be resolved
+        resolver.resolve_visibility(db, self).unwrap_or(Visibility::Public)
+    }
+}
+/// Visibility of an item, with the path resolved.
+#[derive(Debug, Copy, Clone, PartialEq, Eq)]
+pub enum Visibility {
+    /// Visibility is restricted to a certain module.
+    Module(ModuleId),
+    /// Visibility is unrestricted.
+    Public,
+}
+impl Visibility {
+    pub fn is_visible_from(self, db: &impl DefDatabase, from_module: ModuleId) -> bool {
+        let to_module = match self {
+            Visibility::Module(m) => m,
+            Visibility::Public => return true,
+        };
+        // if they're not in the same crate, it can't be visible
+        if from_module.krate != to_module.krate {
+            return false;
+        }
+        let def_map = db.crate_def_map(from_module.krate);
+        self.is_visible_from_def_map(&def_map, from_module.local_id)
+    }
+    pub(crate) fn is_visible_from_other_crate(self) -> bool {
+        match self {
+            Visibility::Module(_) => false,
+            Visibility::Public => true,
+        }
+    }
+    pub(crate) fn is_visible_from_def_map(
+        self,
+        def_map: &crate::nameres::CrateDefMap,
+        from_module: crate::LocalModuleId,
+    ) -> bool {
+        let to_module = match self {
+            Visibility::Module(m) => m,
+            Visibility::Public => return true,
+        };
+        // from_module needs to be a descendant of to_module
+        let mut ancestors = std::iter::successors(Some(from_module), |m| {
+            let parent_id = def_map[*m].parent?;
+            Some(parent_id)
+        });
+        ancestors.any(|m| m == to_module.local_id)
+    }
+}
diff --git a/crates/ra_hir_ty/src/display.rs b/crates/ra_hir_ty/src/display.rs
index dcca1bace..37def7c03 100644
--- a/crates/ra_hir_ty/src/display.rs
+++ b/crates/ra_hir_ty/src/display.rs
@@ -10,7 +10,7 @@ pub struct HirFormatter<'a, 'b, DB> {
    buf: String,
    curr_size: usize,
    max_size: Option<usize>,
-    should_display_default_types: bool,
+    omit_verbose_types: bool,
 }
 pub trait HirDisplay {
@@ -20,7 +20,7 @@ pub trait HirDisplay {
    where
        Self: Sized,
    {
-        HirDisplayWrapper(db, self, None, true)
+        HirDisplayWrapper(db, self, None, false)
    }
    fn display_truncated<'a, DB>(
@@ -31,7 +31,7 @@ pub trait HirDisplay {
    where
        Self: Sized,
    {
-        HirDisplayWrapper(db, self, max_size, false)
+        HirDisplayWrapper(db, self, max_size, true)
    }
 }
@@ -74,8 +74,8 @@ where
        }
    }
-    pub fn should_display_default_types(&self) -> bool {
+    pub fn omit_verbose_types(&self) -> bool {
-        self.should_display_default_types
+        self.omit_verbose_types
    }
 }
@@ -93,7 +93,7 @@ where
            buf: String::with_capacity(20),
            curr_size: 0,
            max_size: self.2,
-            should_display_default_types: self.3,
+            omit_verbose_types: self.3,
        })
    }
 }
diff --git a/crates/ra_hir_ty/src/lib.rs b/crates/ra_hir_ty/src/lib.rs
index 48abf97c9..55b6dd836 100644
--- a/crates/ra_hir_ty/src/lib.rs
+++ b/crates/ra_hir_ty/src/lib.rs
@@ -821,6 +821,8 @@ impl TypeWalk for Ty {
    }
 }
+const TYPE_HINT_TRUNCATION: &str = "…";
 impl HirDisplay for &Ty {
    fn hir_fmt(&self, f: &mut HirFormatter<impl HirDatabase>) -> fmt::Result {
        HirDisplay::hir_fmt(*self, f)
@@ -830,7 +832,7 @@ impl HirDisplay for &Ty {
 impl HirDisplay for ApplicationTy {
    fn hir_fmt(&self, f: &mut HirFormatter<impl HirDatabase>) -> fmt::Result {
        if f.should_truncate() {
-            return write!(f, "…");
+            return write!(f, "{}", TYPE_HINT_TRUNCATION);
        }
        match self.ctor {
@@ -908,9 +910,7 @@ impl HirDisplay for ApplicationTy {
                    write!(f, "<")?;
                    let mut non_default_parameters = Vec::with_capacity(self.parameters.len());
-                    let parameters_to_write = if f.should_display_default_types() {
+                    let parameters_to_write = if f.omit_verbose_types() {
-                        self.parameters.0.as_ref()
-                    } else {
                        match self
                            .ctor
                            .as_generic_def()
@@ -935,6 +935,8 @@ impl HirDisplay for ApplicationTy {
                                &non_default_parameters
                            }
                        }
+                    } else {
+                        self.parameters.0.as_ref()
                    };
                    f.write_joined(parameters_to_write, ", ")?;
@@ -959,9 +961,16 @@ impl HirDisplay for ApplicationTy {
                let sig = self.parameters[0]
                    .callable_sig(f.db)
                    .expect("first closure parameter should contain signature");
-                write!(f, "|")?;
+                let return_type_hint = sig.ret().display(f.db);
-                f.write_joined(sig.params(), ", ")?;
+                if sig.params().is_empty() {
-                write!(f, "| -> {}", sig.ret().display(f.db))?;
+                    write!(f, "|| -> {}", return_type_hint)?;
+                } else if f.omit_verbose_types() {
+                    write!(f, "|{}| -> {}", TYPE_HINT_TRUNCATION, return_type_hint)?;
+                } else {
+                    write!(f, "|")?;
+                    f.write_joined(sig.params(), ", ")?;
+                    write!(f, "| -> {}", return_type_hint)?;
+                };
            }
        }
        Ok(())
@@ -971,7 +980,7 @@ impl HirDisplay for ApplicationTy {
 impl HirDisplay for ProjectionTy {
    fn hir_fmt(&self, f: &mut HirFormatter<impl HirDatabase>) -> fmt::Result {
        if f.should_truncate() {
-            return write!(f, "…");
+            return write!(f, "{}", TYPE_HINT_TRUNCATION);
        }
        let trait_name = f.db.trait_data(self.trait_(f.db)).name.clone();
@@ -989,7 +998,7 @@ impl HirDisplay for ProjectionTy {
 impl HirDisplay for Ty {
    fn hir_fmt(&self, f: &mut HirFormatter<impl HirDatabase>) -> fmt::Result {
        if f.should_truncate() {
-            return write!(f, "…");
+            return write!(f, "{}", TYPE_HINT_TRUNCATION);
        }
        match self {
@@ -1074,7 +1083,7 @@ impl HirDisplay for Ty {
 impl TraitRef {
    fn hir_fmt_ext(&self, f: &mut HirFormatter<impl HirDatabase>, use_as: bool) -> fmt::Result {
        if f.should_truncate() {
-            return write!(f, "…");
+            return write!(f, "{}", TYPE_HINT_TRUNCATION);
        }
        self.substs[0].hir_fmt(f)?;
@@ -1108,7 +1117,7 @@ impl HirDisplay for &GenericPredicate {
 impl HirDisplay for GenericPredicate {
    fn hir_fmt(&self, f: &mut HirFormatter<impl HirDatabase>) -> fmt::Result {
        if f.should_truncate() {
-            return write!(f, "…");
+            return write!(f, "{}", TYPE_HINT_TRUNCATION);
        }
        match self {
diff --git a/crates/ra_ide/src/completion/complete_dot.rs b/crates/ra_ide/src/completion/complete_dot.rs
index 294964887..210a685e4 100644
--- a/crates/ra_ide/src/completion/complete_dot.rs
+++ b/crates/ra_ide/src/completion/complete_dot.rs
@@ -1,6 +1,6 @@
 //! FIXME: write short doc here
-use hir::Type;
+use hir::{HasVisibility, Type};
 use crate::completion::completion_item::CompletionKind;
 use crate::{
@@ -38,9 +38,15 @@ pub(super) fn complete_dot(acc: &mut Completions, ctx: &CompletionContext) {
 fn complete_fields(acc: &mut Completions, ctx: &CompletionContext, receiver: &Type) {
    for receiver in receiver.autoderef(ctx.db) {
        for (field, ty) in receiver.fields(ctx.db) {
+            if ctx.module.map_or(false, |m| !field.is_visible_from(ctx.db, m)) {
+                // Skip private field. FIXME: If the definition location of the
+                // field is editable, we should show the completion
+                continue;
+            }
            acc.add_field(ctx, field, &ty);
        }
        for (i, ty) in receiver.tuple_fields(ctx.db).into_iter().enumerate() {
+            // FIXME: Handle visibility
            acc.add_tuple_field(ctx, i, &ty);
        }
    }
@@ -187,6 +193,55 @@ mod tests {
    }
    #[test]
+    fn test_struct_field_visibility_private() {
+        assert_debug_snapshot!(
+            do_ref_completion(
+                r"
+            mod inner {
+                struct A {
+                    private_field: u32,
+                    pub pub_field: u32,
+                    pub(crate) crate_field: u32,
+                    pub(super) super_field: u32,
+                }
+            }
+            fn foo(a: inner::A) {
+               a.<|>
+            }
+            ",
+            ),
+            @r###"
+        [
+            CompletionItem {
+                label: "crate_field",
+                source_range: [313; 313),
+                delete: [313; 313),
+                insert: "crate_field",
+                kind: Field,
+                detail: "u32",
+            },
+            CompletionItem {
+                label: "pub_field",
+                source_range: [313; 313),
+                delete: [313; 313),
+                insert: "pub_field",
+                kind: Field,
+                detail: "u32",
+            },
+            CompletionItem {
+                label: "super_field",
+                source_range: [313; 313),
+                delete: [313; 313),
+                insert: "super_field",
+                kind: Field,
+                detail: "u32",
+            },
+        ]
+        "###
+        );
+    }
+    #[test]
    fn test_method_completion() {
        assert_debug_snapshot!(
        do_ref_completion(
diff --git a/crates/ra_ide/src/inlay_hints.rs b/crates/ra_ide/src/inlay_hints.rs
index c5e406977..977aafc51 100644
--- a/crates/ra_ide/src/inlay_hints.rs
+++ b/crates/ra_ide/src/inlay_hints.rs
@@ -293,7 +293,7 @@ fn main() {
    }
    #[test]
-    fn closure_parameter() {
+    fn closure_parameters() {
        let (analysis, file_id) = single_file(
            r#"
 fn main() {
@@ -301,6 +301,11 @@ fn main() {
    (0..2).for_each(|increment| {
        start += increment;
    })
+    let multiply = |a, b, c, d| a * b * c * d;
+    let _: i32 = multiply(1, 2, 3, 4);
+    let return_42 = || 42;
 }"#,
        );
@@ -316,6 +321,36 @@ fn main() {
                kind: TypeHint,
                label: "i32",
            },
+            InlayHint {
+                range: [114; 122),
+                kind: TypeHint,
+                label: "|…| -> i32",
+            },
+            InlayHint {
+                range: [126; 127),
+                kind: TypeHint,
+                label: "i32",
+            },
+            InlayHint {
+                range: [129; 130),
+                kind: TypeHint,
+                label: "i32",
+            },
+            InlayHint {
+                range: [132; 133),
+                kind: TypeHint,
+                label: "i32",
+            },
+            InlayHint {
+                range: [135; 136),
+                kind: TypeHint,
+                label: "i32",
+            },
+            InlayHint {
+                range: [201; 210),
+                kind: TypeHint,
+                label: "|| -> i32",
+            },
        ]
        "###
        );
diff --git a/crates/ra_project_model/src/cargo_workspace.rs b/crates/ra_project_model/src/cargo_workspace.rs
index c862d3912..1b3c246c7 100644
--- a/crates/ra_project_model/src/cargo_workspace.rs
+++ b/crates/ra_project_model/src/cargo_workspace.rs
@@ -165,7 +165,7 @@ impl CargoWorkspace {
            // FIXME: `NoDefaultFeatures` is mutual exclusive with `SomeFeatures`
            // https://github.com/oli-obk/cargo_metadata/issues/79
            meta.features(CargoOpt::NoDefaultFeatures);
-        } else {
+        } else if cargo_features.features.len() > 0 {
            meta.features(CargoOpt::SomeFeatures(cargo_features.features.clone()));
        }
        if let Some(parent) = cargo_toml.parent() {
diff --git a/crates/ra_syntax/src/ast.rs b/crates/ra_syntax/src/ast.rs
index 277532a8c..89cb9a9f3 100644
--- a/crates/ra_syntax/src/ast.rs
+++ b/crates/ra_syntax/src/ast.rs
@@ -17,7 +17,9 @@ use crate::{
 pub use self::{
    expr_extensions::{ArrayExprKind, BinOp, ElseBranch, LiteralKind, PrefixOp, RangeOp},
-    extensions::{FieldKind, PathSegmentKind, SelfParamKind, StructKind, TypeBoundKind},
+    extensions::{
+        FieldKind, PathSegmentKind, SelfParamKind, StructKind, TypeBoundKind, VisibilityKind,
+    },
    generated::*,
    tokens::*,
    traits::*,
diff --git a/crates/ra_syntax/src/ast/extensions.rs b/crates/ra_syntax/src/ast/extensions.rs
index baaef3023..d9666cdca 100644
--- a/crates/ra_syntax/src/ast/extensions.rs
+++ b/crates/ra_syntax/src/ast/extensions.rs
@@ -413,3 +413,32 @@ impl ast::TraitDef {
        self.syntax().children_with_tokens().any(|t| t.kind() == T![auto])
    }
 }
+pub enum VisibilityKind {
+    In(ast::Path),
+    PubCrate,
+    PubSuper,
+    Pub,
+}
+impl ast::Visibility {
+    pub fn kind(&self) -> VisibilityKind {
+        if let Some(path) = children(self).next() {
+            VisibilityKind::In(path)
+        } else if self.is_pub_crate() {
+            VisibilityKind::PubCrate
+        } else if self.is_pub_super() {
+            VisibilityKind::PubSuper
+        } else {
+            VisibilityKind::Pub
+        }
+    }
+    fn is_pub_crate(&self) -> bool {
+        self.syntax().children_with_tokens().any(|it| it.kind() == T![crate])
+    }
+    fn is_pub_super(&self) -> bool {
+        self.syntax().children_with_tokens().any(|it| it.kind() == T![super])
+    }
+}
diff --git a/crates/ra_syntax/src/ast/generated.rs b/crates/ra_syntax/src/ast/generated.rs
index 9f9d6e63c..e64c83d33 100644
--- a/crates/ra_syntax/src/ast/generated.rs
+++ b/crates/ra_syntax/src/ast/generated.rs
@@ -1064,6 +1064,7 @@ impl AstNode for ExternCrateItem {
    }
 }
 impl ast::AttrsOwner for ExternCrateItem {}
+impl ast::VisibilityOwner for ExternCrateItem {}
 impl ExternCrateItem {
    pub fn name_ref(&self) -> Option<NameRef> {
        AstChildren::new(&self.syntax).next()
@@ -2006,6 +2007,7 @@ impl AstNode for ModuleItem {
    }
 }
 impl ast::AttrsOwner for ModuleItem {}
+impl ast::VisibilityOwner for ModuleItem {}
 impl ModuleItem {}
 #[derive(Debug, Clone, PartialEq, Eq, Hash)]
 pub struct Name {
@@ -3893,6 +3895,7 @@ impl AstNode for UseItem {
    }
 }
 impl ast::AttrsOwner for UseItem {}
+impl ast::VisibilityOwner for UseItem {}
 impl UseItem {
    pub fn use_tree(&self) -> Option<UseTree> {
        AstChildren::new(&self.syntax).next()
diff --git a/crates/ra_syntax/src/grammar.ron b/crates/ra_syntax/src/grammar.ron
index 08aafb610..e43a724f0 100644
--- a/crates/ra_syntax/src/grammar.ron
+++ b/crates/ra_syntax/src/grammar.ron
@@ -412,7 +412,7 @@ Grammar(
        "ModuleItem": (
            enum: ["StructDef", "UnionDef", "EnumDef", "FnDef", "TraitDef", "TypeAliasDef", "ImplBlock",
                   "UseItem", "ExternCrateItem", "ConstDef", "StaticDef", "Module" ],
-            traits: ["AttrsOwner"],
+            traits: ["AttrsOwner", "VisibilityOwner"],
        ),
        "ImplItem": (
            enum: ["FnDef", "TypeAliasDef", "ConstDef"],
@@ -683,7 +683,7 @@ Grammar(
            ]
        ),
        "UseItem": (
-            traits: ["AttrsOwner"],
+            traits: ["AttrsOwner", "VisibilityOwner"],
            options: [ "UseTree" ],
        ),
        "UseTree": (
@@ -696,7 +696,7 @@ Grammar(
            collections: [("use_trees", "UseTree")]
        ),
        "ExternCrateItem": (
-            traits: ["AttrsOwner"],
+            traits: ["AttrsOwner", "VisibilityOwner"],
            options: ["NameRef", "Alias"],
        ),
        "ArgList": (
diff --git a/docs/user/assists.md b/docs/user/assists.md
index 334ba450f..ecf206f71 100644
--- a/docs/user/assists.md
+++ b/docs/user/assists.md
@@ -98,26 +98,26 @@ Adds scaffold for overriding default impl members.
 ```rust
 // BEFORE
-trait T {
+trait Trait {
    Type X;
    fn foo(&self);
    fn bar(&self) {}
 }
-impl T for () {
+impl Trait for () {
    Type X = ();
    fn foo(&self) {}┃
 }
 // AFTER
-trait T {
+trait Trait {
    Type X;
    fn foo(&self);
    fn bar(&self) {}
 }
-impl T for () {
+impl Trait for () {
    Type X = ();
    fn foo(&self) {}
    fn bar(&self) {}
@@ -131,25 +131,25 @@ Adds scaffold for required impl members.
 ```rust
 // BEFORE
-trait T {
+trait Trait<T> {
    Type X;
-    fn foo(&self);
+    fn foo(&self) -> T;
    fn bar(&self) {}
 }
-impl T for () {┃
+impl Trait<u32> for () {┃
 }
 // AFTER
-trait T {
+trait Trait<T> {
    Type X;
-    fn foo(&self);
+    fn foo(&self) -> T;
    fn bar(&self) {}
 }
-impl T for () {
+impl Trait<u32> for () {
-    fn foo(&self) { unimplemented!() }
+    fn foo(&self) -> u32 { unimplemented!() }
 }
 ```
diff --git a/xtask/tests/tidy-tests/cli.rs b/xtask/tests/tidy-tests/cli.rs
index 573ffadbf..f9ca45292 100644
--- a/xtask/tests/tidy-tests/cli.rs
+++ b/xtask/tests/tidy-tests/cli.rs
@@ -43,7 +43,7 @@ fn no_todo() {
            return;
        }
        let text = std::fs::read_to_string(e.path()).unwrap();
-        if text.contains("TODO") || text.contains("TOOD") {
+        if text.contains("TODO") || text.contains("TOOD") || text.contains("todo!") {
            panic!(
                "\nTODO markers should not be committed to the master branch,\n\
                 use FIXME instead\n\