Split change_ and fix_ visibility assists

4 files changed, 581 insertions, 506 deletions

diff --git a/crates/ra_assists/src/handlers/change_visibility.rs b/crates/ra_assists/src/handlers/change_visibility.rs
index 71d55e0c3..1d9b8e645 100644
--- a/crates/ra_assists/src/handlers/change_visibility.rs
+++ b/crates/ra_assists/src/handlers/change_visibility.rs
@@ -5,14 +5,11 @@ use ra_syntax::{
         ATTR, COMMENT, CONST_DEF, ENUM_DEF, FN_DEF, MODULE, STRUCT_DEF, TRAIT_DEF, VISIBILITY,
         WHITESPACE,
     },
-    SyntaxNode, TextRange, TextSize, T,
+    SyntaxNode, TextSize, T,
 };
-
-use hir::{db::HirDatabase, HasSource, HasVisibility, PathResolution};
 use test_utils::mark;
 
 use crate::{AssistContext, AssistId, Assists};
-use ra_db::FileId;
 
 // Assist: change_visibility
 //
@@ -30,8 +27,6 @@ pub(crate) fn change_visibility(acc: &mut Assists, ctx: &AssistContext) -> Optio
         return change_vis(acc, vis);
     }
     add_vis(acc, ctx)
-        .or_else(|| add_vis_to_referenced_module_def(acc, ctx))
-        .or_else(|| add_vis_to_referenced_record_field(acc, ctx))
 }
 
 fn add_vis(acc: &mut Assists, ctx: &AssistContext) -> Option<()> {
@@ -77,143 +72,6 @@ fn add_vis(acc: &mut Assists, ctx: &AssistContext) -> Option<()> {
     })
 }
 
-fn add_vis_to_referenced_module_def(acc: &mut Assists, ctx: &AssistContext) -> Option<()> {
-    let path: ast::Path = ctx.find_node_at_offset()?;
-    let path_res = ctx.sema.resolve_path(&path)?;
-    let def = match path_res {
-        PathResolution::Def(def) => def,
-        _ => return None,
-    };
-
-    let current_module = ctx.sema.scope(&path.syntax()).module()?;
-    let target_module = def.module(ctx.db)?;
-
-    let vis = target_module.visibility_of(ctx.db, &def)?;
-    if vis.is_visible_from(ctx.db, current_module.into()) {
-        return None;
-    };
-
-    let (offset, target, target_file, target_name) = target_data_for_def(ctx.db, def)?;
-
-    let missing_visibility =
-        if current_module.krate() == target_module.krate() { "pub(crate)" } else { "pub" };
-
-    let assist_label = match target_name {
-        None => format!("Change visibility to {}", missing_visibility),
-        Some(name) => format!("Change visibility of {} to {}", name, missing_visibility),
-    };
-
-    acc.add(AssistId("change_visibility"), assist_label, target, |edit| {
-        edit.set_file(target_file);
-        edit.insert(offset, format!("{} ", missing_visibility));
-        edit.set_cursor(offset);
-    })
-}
-
-fn add_vis_to_referenced_record_field(acc: &mut Assists, ctx: &AssistContext) -> Option<()> {
-    let record_field: ast::RecordField = ctx.find_node_at_offset()?;
-    let (record_field_def, _) = ctx.sema.resolve_record_field(&record_field)?;
-
-    let current_module = ctx.sema.scope(record_field.syntax()).module()?;
-    let visibility = record_field_def.visibility(ctx.db);
-    if visibility.is_visible_from(ctx.db, current_module.into()) {
-        return None;
-    }
-
-    let parent = record_field_def.parent_def(ctx.db);
-    let parent_name = parent.name(ctx.db);
-    let target_module = parent.module(ctx.db);
-
-    let in_file_source = record_field_def.source(ctx.db);
-    let (offset, target) = match in_file_source.value {
-        hir::FieldSource::Named(it) => {
-            let s = it.syntax();
-            (vis_offset(s), s.text_range())
-        }
-        hir::FieldSource::Pos(it) => {
-            let s = it.syntax();
-            (vis_offset(s), s.text_range())
-        }
-    };
-
-    let missing_visibility =
-        if current_module.krate() == target_module.krate() { "pub(crate)" } else { "pub" };
-    let target_file = in_file_source.file_id.original_file(ctx.db);
-
-    let target_name = record_field_def.name(ctx.db);
-    let assist_label =
-        format!("Change visibility of {}.{} to {}", parent_name, target_name, missing_visibility);
-
-    acc.add(AssistId("change_visibility"), assist_label, target, |edit| {
-        edit.set_file(target_file);
-        edit.insert(offset, format!("{} ", missing_visibility));
-        edit.set_cursor(offset)
-    })
-}
-
-fn target_data_for_def(
-    db: &dyn HirDatabase,
-    def: hir::ModuleDef,
-) -> Option<(TextSize, TextRange, FileId, Option<hir::Name>)> {
-    fn offset_target_and_file_id<S, Ast>(
-        db: &dyn HirDatabase,
-        x: S,
-    ) -> (TextSize, TextRange, FileId)
-    where
-        S: HasSource<Ast = Ast>,
-        Ast: AstNode,
-    {
-        let source = x.source(db);
-        let in_file_syntax = source.syntax();
-        let file_id = in_file_syntax.file_id;
-        let syntax = in_file_syntax.value;
-        (vis_offset(syntax), syntax.text_range(), file_id.original_file(db.upcast()))
-    }
-
-    let target_name;
-    let (offset, target, target_file) = match def {
-        hir::ModuleDef::Function(f) => {
-            target_name = Some(f.name(db));
-            offset_target_and_file_id(db, f)
-        }
-        hir::ModuleDef::Adt(adt) => {
-            target_name = Some(adt.name(db));
-            match adt {
-                hir::Adt::Struct(s) => offset_target_and_file_id(db, s),
-                hir::Adt::Union(u) => offset_target_and_file_id(db, u),
-                hir::Adt::Enum(e) => offset_target_and_file_id(db, e),
-            }
-        }
-        hir::ModuleDef::Const(c) => {
-            target_name = c.name(db);
-            offset_target_and_file_id(db, c)
-        }
-        hir::ModuleDef::Static(s) => {
-            target_name = s.name(db);
-            offset_target_and_file_id(db, s)
-        }
-        hir::ModuleDef::Trait(t) => {
-            target_name = Some(t.name(db));
-            offset_target_and_file_id(db, t)
-        }
-        hir::ModuleDef::TypeAlias(t) => {
-            target_name = Some(t.name(db));
-            offset_target_and_file_id(db, t)
-        }
-        hir::ModuleDef::Module(m) => {
-            target_name = m.name(db);
-            let in_file_source = m.declaration_source(db)?;
-            let file_id = in_file_source.file_id.original_file(db.upcast());
-            let syntax = in_file_source.value.syntax();
-            (vis_offset(syntax), syntax.text_range(), file_id)
-        }
-        // Enum variants can't be private, we can't modify builtin types
-        hir::ModuleDef::EnumVariant(_) | hir::ModuleDef::BuiltinType(_) => return None,
-    };
-
-    Some((offset, target, target_file, target_name))
-}
-
 fn vis_offset(node: &SyntaxNode) -> TextSize {
     node.children_with_tokens()
         .skip_while(|it| match it.kind() {
@@ -334,193 +192,6 @@ mod tests {
     }
 
     #[test]
-    fn change_visibility_of_fn_via_path() {
-        check_assist(
-            change_visibility,
-            r"mod foo { fn foo() {} }
-              fn main() { foo::foo<|>() } ",
-            r"mod foo { <|>pub(crate) fn foo() {} }
-              fn main() { foo::foo() } ",
-        );
-        check_assist_not_applicable(
-            change_visibility,
-            r"mod foo { pub fn foo() {} }
-              fn main() { foo::foo<|>() } ",
-        )
-    }
-
-    #[test]
-    fn change_visibility_of_adt_in_submodule_via_path() {
-        check_assist(
-            change_visibility,
-            r"mod foo { struct Foo; }
-              fn main() { foo::Foo<|> } ",
-            r"mod foo { <|>pub(crate) struct Foo; }
-              fn main() { foo::Foo } ",
-        );
-        check_assist_not_applicable(
-            change_visibility,
-            r"mod foo { pub struct Foo; }
-              fn main() { foo::Foo<|> } ",
-        );
-        check_assist(
-            change_visibility,
-            r"mod foo { enum Foo; }
-              fn main() { foo::Foo<|> } ",
-            r"mod foo { <|>pub(crate) enum Foo; }
-              fn main() { foo::Foo } ",
-        );
-        check_assist_not_applicable(
-            change_visibility,
-            r"mod foo { pub enum Foo; }
-              fn main() { foo::Foo<|> } ",
-        );
-        check_assist(
-            change_visibility,
-            r"mod foo { union Foo; }
-              fn main() { foo::Foo<|> } ",
-            r"mod foo { <|>pub(crate) union Foo; }
-              fn main() { foo::Foo } ",
-        );
-        check_assist_not_applicable(
-            change_visibility,
-            r"mod foo { pub union Foo; }
-              fn main() { foo::Foo<|> } ",
-        );
-    }
-
-    #[test]
-    fn change_visibility_of_adt_in_other_file_via_path() {
-        check_assist(
-            change_visibility,
-            r"
-              //- /main.rs
-              mod foo;
-              fn main() { foo::Foo<|> }
-
-              //- /foo.rs
-              struct Foo;
-              ",
-            r"<|>pub(crate) struct Foo;
-
-",
-        );
-    }
-
-    #[test]
-    fn change_visibility_of_struct_field_via_path() {
-        check_assist(
-            change_visibility,
-            r"mod foo { pub struct Foo { bar: (), } }
-              fn main() { foo::Foo { <|>bar: () }; } ",
-            r"mod foo { pub struct Foo { <|>pub(crate) bar: (), } }
-              fn main() { foo::Foo { bar: () }; } ",
-        );
-        check_assist(
-            change_visibility,
-            r"//- /lib.rs
-              mod foo;
-              fn main() { foo::Foo { <|>bar: () }; }
-              //- /foo.rs
-              pub struct Foo { bar: () }
-              ",
-            r"pub struct Foo { <|>pub(crate) bar: () }
-
-",
-        );
-        check_assist_not_applicable(
-            change_visibility,
-            r"mod foo { pub struct Foo { pub bar: (), } }
-              fn main() { foo::Foo { <|>bar: () }; } ",
-        );
-        check_assist_not_applicable(
-            change_visibility,
-            r"//- /lib.rs
-              mod foo;
-              fn main() { foo::Foo { <|>bar: () }; }
-              //- /foo.rs
-              pub struct Foo { pub bar: () }
-              ",
-        );
-    }
-
-    #[test]
-    fn change_visibility_of_enum_variant_field_via_path() {
-        check_assist(
-            change_visibility,
-            r"mod foo { pub enum Foo { Bar { bar: () } } }
-              fn main() { foo::Foo::Bar { <|>bar: () }; } ",
-            r"mod foo { pub enum Foo { Bar { <|>pub(crate) bar: () } } }
-              fn main() { foo::Foo::Bar { bar: () }; } ",
-        );
-        check_assist(
-            change_visibility,
-            r"//- /lib.rs
-              mod foo;
-              fn main() { foo::Foo::Bar { <|>bar: () }; }
-              //- /foo.rs
-              pub enum Foo { Bar { bar: () } }
-              ",
-            r"pub enum Foo { Bar { <|>pub(crate) bar: () } }
-
-",
-        );
-        check_assist_not_applicable(
-            change_visibility,
-            r"mod foo { pub struct Foo { pub bar: (), } }
-              fn main() { foo::Foo { <|>bar: () }; } ",
-        );
-        check_assist_not_applicable(
-            change_visibility,
-            r"//- /lib.rs
-              mod foo;
-              fn main() { foo::Foo { <|>bar: () }; }
-              //- /foo.rs
-              pub struct Foo { pub bar: () }
-              ",
-        );
-    }
-
-    #[test]
-    #[ignore]
-    // FIXME reenable this test when `Semantics::resolve_record_field` works with union fields
-    fn change_visibility_of_union_field_via_path() {
-        check_assist(
-            change_visibility,
-            r"mod foo { pub union Foo { bar: (), } }
-              fn main() { foo::Foo { <|>bar: () }; } ",
-            r"mod foo { pub union Foo { <|>pub(crate) bar: (), } }
-              fn main() { foo::Foo { bar: () }; } ",
-        );
-        check_assist(
-            change_visibility,
-            r"//- /lib.rs
-              mod foo;
-              fn main() { foo::Foo { <|>bar: () }; }
-              //- /foo.rs
-              pub union Foo { bar: () }
-              ",
-            r"pub union Foo { <|>pub(crate) bar: () }
-
-",
-        );
-        check_assist_not_applicable(
-            change_visibility,
-            r"mod foo { pub union Foo { pub bar: (), } }
-              fn main() { foo::Foo { <|>bar: () }; } ",
-        );
-        check_assist_not_applicable(
-            change_visibility,
-            r"//- /lib.rs
-              mod foo;
-              fn main() { foo::Foo { <|>bar: () }; }
-              //- /foo.rs
-              pub union Foo { pub bar: () }
-              ",
-        );
-    }
-
-    #[test]
     fn not_applicable_for_enum_variants() {
         check_assist_not_applicable(
             change_visibility,
@@ -530,182 +201,6 @@ mod tests {
     }
 
     #[test]
-    fn change_visibility_of_const_via_path() {
-        check_assist(
-            change_visibility,
-            r"mod foo { const FOO: () = (); }
-              fn main() { foo::FOO<|> } ",
-            r"mod foo { <|>pub(crate) const FOO: () = (); }
-              fn main() { foo::FOO } ",
-        );
-        check_assist_not_applicable(
-            change_visibility,
-            r"mod foo { pub const FOO: () = (); }
-              fn main() { foo::FOO<|> } ",
-        );
-    }
-
-    #[test]
-    fn change_visibility_of_static_via_path() {
-        check_assist(
-            change_visibility,
-            r"mod foo { static FOO: () = (); }
-              fn main() { foo::FOO<|> } ",
-            r"mod foo { <|>pub(crate) static FOO: () = (); }
-              fn main() { foo::FOO } ",
-        );
-        check_assist_not_applicable(
-            change_visibility,
-            r"mod foo { pub static FOO: () = (); }
-              fn main() { foo::FOO<|> } ",
-        );
-    }
-
-    #[test]
-    fn change_visibility_of_trait_via_path() {
-        check_assist(
-            change_visibility,
-            r"mod foo { trait Foo { fn foo(&self) {} } }
-              fn main() { let x: &dyn foo::<|>Foo; } ",
-            r"mod foo { <|>pub(crate) trait Foo { fn foo(&self) {} } }
-              fn main() { let x: &dyn foo::Foo; } ",
-        );
-        check_assist_not_applicable(
-            change_visibility,
-            r"mod foo { pub trait Foo { fn foo(&self) {} } }
-              fn main() { let x: &dyn foo::Foo<|>; } ",
-        );
-    }
-
-    #[test]
-    fn change_visibility_of_type_alias_via_path() {
-        check_assist(
-            change_visibility,
-            r"mod foo { type Foo = (); }
-              fn main() { let x: foo::Foo<|>; } ",
-            r"mod foo { <|>pub(crate) type Foo = (); }
-              fn main() { let x: foo::Foo; } ",
-        );
-        check_assist_not_applicable(
-            change_visibility,
-            r"mod foo { pub type Foo = (); }
-              fn main() { let x: foo::Foo<|>; } ",
-        );
-    }
-
-    #[test]
-    fn change_visibility_of_module_via_path() {
-        check_assist(
-            change_visibility,
-            r"mod foo { mod bar { fn bar() {} } }
-              fn main() { foo::bar<|>::bar(); } ",
-            r"mod foo { <|>pub(crate) mod bar { fn bar() {} } }
-              fn main() { foo::bar::bar(); } ",
-        );
-
-        check_assist(
-            change_visibility,
-            r"
-            //- /main.rs
-            mod foo;
-            fn main() { foo::bar<|>::baz(); }
-
-            //- /foo.rs
-            mod bar {
-                pub fn baz() {}
-            }
-            ",
-            r"<|>pub(crate) mod bar {
-    pub fn baz() {}
-}
-
-",
-        );
-
-        check_assist_not_applicable(
-            change_visibility,
-            r"mod foo { pub mod bar { pub fn bar() {} } }
-              fn main() { foo::bar<|>::bar(); } ",
-        );
-    }
-
-    #[test]
-    fn change_visibility_of_inline_module_in_other_file_via_path() {
-        check_assist(
-            change_visibility,
-            r"
-            //- /main.rs
-            mod foo;
-            fn main() { foo::bar<|>::baz(); }
-
-            //- /foo.rs
-            mod bar;
-
-            //- /foo/bar.rs
-            pub fn baz() {}
-            }
-            ",
-            r"<|>pub(crate) mod bar;
-",
-        );
-    }
-
-    #[test]
-    fn change_visibility_of_module_declaration_in_other_file_via_path() {
-        check_assist(
-            change_visibility,
-            r"//- /main.rs
-              mod foo;
-              fn main() { foo::bar<|>>::baz(); }
-
-              //- /foo.rs
-              mod bar {
-                  pub fn baz() {}
-              }",
-            r"<|>pub(crate) mod bar {
-    pub fn baz() {}
-}
-",
-        );
-    }
-
-    #[test]
-    #[ignore]
-    // FIXME handle reexports properly
-    fn change_visibility_of_reexport() {
-        check_assist(
-            change_visibility,
-            r"
-            mod foo {
-                use bar::Baz;
-                mod bar { pub(super) struct Baz; }
-            }
-            foo::Baz<|>
-            ",
-            r"
-            mod foo {
-                <|>pub(crate) use bar::Baz;
-                mod bar { pub(super) struct Baz; }
-            }
-            foo::Baz
-            ",
-        )
-    }
-
-    #[test]
-    fn adds_pub_when_target_is_in_another_crate() {
-        check_assist(
-            change_visibility,
-            r"//- /main.rs crate:a deps:foo
-              foo::Bar<|>
-              //- /lib.rs crate:foo
-              struct Bar;",
-            r"<|>pub struct Bar;
-",
-        )
-    }
-
-    #[test]
     fn change_visibility_target() {
         check_assist_target(change_visibility, "<|>fn foo() {}", "fn");
         check_assist_target(change_visibility, "pub(crate)<|> fn foo() {}", "pub(crate)");
diff --git a/crates/ra_assists/src/handlers/fix_visibility.rs b/crates/ra_assists/src/handlers/fix_visibility.rs
new file mode 100644
index 000000000..48ce07ca5
--- /dev/null
+++ b/crates/ra_assists/src/handlers/fix_visibility.rs
@@ -0,0 +1,555 @@
+use hir::{db::HirDatabase, HasSource, HasVisibility, PathResolution};
+use ra_db::FileId;
+use ra_syntax::{
+    ast, AstNode,
+    SyntaxKind::{ATTR, COMMENT, WHITESPACE},
+    SyntaxNode, TextRange, TextSize,
+};
+
+use crate::{AssistContext, AssistId, Assists};
+
+// FIXME: this really should be a fix for diagnostic, rather than an assist.
+
+// Assist: fix_visibility
+//
+// Makes inaccessible item public.
+//
+// ```
+// mod m {
+//     fn frobnicate() {}
+// }
+// fn main() {
+//     m::frobnicate<|>() {}
+// }
+// ```
+// ->
+// ```
+// mod m {
+//     pub(crate) fn frobnicate() {}
+// }
+// fn main() {
+//     m::frobnicate() {}
+// }
+// ```
+pub(crate) fn fix_visibility(acc: &mut Assists, ctx: &AssistContext) -> Option<()> {
+    add_vis_to_referenced_module_def(acc, ctx)
+        .or_else(|| add_vis_to_referenced_record_field(acc, ctx))
+}
+
+fn add_vis_to_referenced_module_def(acc: &mut Assists, ctx: &AssistContext) -> Option<()> {
+    let path: ast::Path = ctx.find_node_at_offset()?;
+    let path_res = ctx.sema.resolve_path(&path)?;
+    let def = match path_res {
+        PathResolution::Def(def) => def,
+        _ => return None,
+    };
+
+    let current_module = ctx.sema.scope(&path.syntax()).module()?;
+    let target_module = def.module(ctx.db)?;
+
+    let vis = target_module.visibility_of(ctx.db, &def)?;
+    if vis.is_visible_from(ctx.db, current_module.into()) {
+        return None;
+    };
+
+    let (offset, target, target_file, target_name) = target_data_for_def(ctx.db, def)?;
+
+    let missing_visibility =
+        if current_module.krate() == target_module.krate() { "pub(crate)" } else { "pub" };
+
+    let assist_label = match target_name {
+        None => format!("Change visibility to {}", missing_visibility),
+        Some(name) => format!("Change visibility of {} to {}", name, missing_visibility),
+    };
+
+    acc.add(AssistId("fix_visibility"), assist_label, target, |edit| {
+        edit.set_file(target_file);
+        edit.insert(offset, format!("{} ", missing_visibility));
+        edit.set_cursor(offset);
+    })
+}
+
+fn add_vis_to_referenced_record_field(acc: &mut Assists, ctx: &AssistContext) -> Option<()> {
+    let record_field: ast::RecordField = ctx.find_node_at_offset()?;
+    let (record_field_def, _) = ctx.sema.resolve_record_field(&record_field)?;
+
+    let current_module = ctx.sema.scope(record_field.syntax()).module()?;
+    let visibility = record_field_def.visibility(ctx.db);
+    if visibility.is_visible_from(ctx.db, current_module.into()) {
+        return None;
+    }
+
+    let parent = record_field_def.parent_def(ctx.db);
+    let parent_name = parent.name(ctx.db);
+    let target_module = parent.module(ctx.db);
+
+    let in_file_source = record_field_def.source(ctx.db);
+    let (offset, target) = match in_file_source.value {
+        hir::FieldSource::Named(it) => {
+            let s = it.syntax();
+            (vis_offset(s), s.text_range())
+        }
+        hir::FieldSource::Pos(it) => {
+            let s = it.syntax();
+            (vis_offset(s), s.text_range())
+        }
+    };
+
+    let missing_visibility =
+        if current_module.krate() == target_module.krate() { "pub(crate)" } else { "pub" };
+    let target_file = in_file_source.file_id.original_file(ctx.db);
+
+    let target_name = record_field_def.name(ctx.db);
+    let assist_label =
+        format!("Change visibility of {}.{} to {}", parent_name, target_name, missing_visibility);
+
+    acc.add(AssistId("fix_visibility"), assist_label, target, |edit| {
+        edit.set_file(target_file);
+        edit.insert(offset, format!("{} ", missing_visibility));
+        edit.set_cursor(offset)
+    })
+}
+
+fn target_data_for_def(
+    db: &dyn HirDatabase,
+    def: hir::ModuleDef,
+) -> Option<(TextSize, TextRange, FileId, Option<hir::Name>)> {
+    fn offset_target_and_file_id<S, Ast>(
+        db: &dyn HirDatabase,
+        x: S,
+    ) -> (TextSize, TextRange, FileId)
+    where
+        S: HasSource<Ast = Ast>,
+        Ast: AstNode,
+    {
+        let source = x.source(db);
+        let in_file_syntax = source.syntax();
+        let file_id = in_file_syntax.file_id;
+        let syntax = in_file_syntax.value;
+        (vis_offset(syntax), syntax.text_range(), file_id.original_file(db.upcast()))
+    }
+
+    let target_name;
+    let (offset, target, target_file) = match def {
+        hir::ModuleDef::Function(f) => {
+            target_name = Some(f.name(db));
+            offset_target_and_file_id(db, f)
+        }
+        hir::ModuleDef::Adt(adt) => {
+            target_name = Some(adt.name(db));
+            match adt {
+                hir::Adt::Struct(s) => offset_target_and_file_id(db, s),
+                hir::Adt::Union(u) => offset_target_and_file_id(db, u),
+                hir::Adt::Enum(e) => offset_target_and_file_id(db, e),
+            }
+        }
+        hir::ModuleDef::Const(c) => {
+            target_name = c.name(db);
+            offset_target_and_file_id(db, c)
+        }
+        hir::ModuleDef::Static(s) => {
+            target_name = s.name(db);
+            offset_target_and_file_id(db, s)
+        }
+        hir::ModuleDef::Trait(t) => {
+            target_name = Some(t.name(db));
+            offset_target_and_file_id(db, t)
+        }
+        hir::ModuleDef::TypeAlias(t) => {
+            target_name = Some(t.name(db));
+            offset_target_and_file_id(db, t)
+        }
+        hir::ModuleDef::Module(m) => {
+            target_name = m.name(db);
+            let in_file_source = m.declaration_source(db)?;
+            let file_id = in_file_source.file_id.original_file(db.upcast());
+            let syntax = in_file_source.value.syntax();
+            (vis_offset(syntax), syntax.text_range(), file_id)
+        }
+        // Enum variants can't be private, we can't modify builtin types
+        hir::ModuleDef::EnumVariant(_) | hir::ModuleDef::BuiltinType(_) => return None,
+    };
+
+    Some((offset, target, target_file, target_name))
+}
+
+fn vis_offset(node: &SyntaxNode) -> TextSize {
+    node.children_with_tokens()
+        .skip_while(|it| match it.kind() {
+            WHITESPACE | COMMENT | ATTR => true,
+            _ => false,
+        })
+        .next()
+        .map(|it| it.text_range().start())
+        .unwrap_or_else(|| node.text_range().start())
+}
+
+#[cfg(test)]
+mod tests {
+    use crate::tests::{check_assist, check_assist_not_applicable};
+
+    use super::*;
+
+    #[test]
+    fn fix_visibility_of_fn() {
+        check_assist(
+            fix_visibility,
+            r"mod foo { fn foo() {} }
+              fn main() { foo::foo<|>() } ",
+            r"mod foo { <|>pub(crate) fn foo() {} }
+              fn main() { foo::foo() } ",
+        );
+        check_assist_not_applicable(
+            fix_visibility,
+            r"mod foo { pub fn foo() {} }
+              fn main() { foo::foo<|>() } ",
+        )
+    }
+
+    #[test]
+    fn fix_visibility_of_adt_in_submodule() {
+        check_assist(
+            fix_visibility,
+            r"mod foo { struct Foo; }
+              fn main() { foo::Foo<|> } ",
+            r"mod foo { <|>pub(crate) struct Foo; }
+              fn main() { foo::Foo } ",
+        );
+        check_assist_not_applicable(
+            fix_visibility,
+            r"mod foo { pub struct Foo; }
+              fn main() { foo::Foo<|> } ",
+        );
+        check_assist(
+            fix_visibility,
+            r"mod foo { enum Foo; }
+              fn main() { foo::Foo<|> } ",
+            r"mod foo { <|>pub(crate) enum Foo; }
+              fn main() { foo::Foo } ",
+        );
+        check_assist_not_applicable(
+            fix_visibility,
+            r"mod foo { pub enum Foo; }
+              fn main() { foo::Foo<|> } ",
+        );
+        check_assist(
+            fix_visibility,
+            r"mod foo { union Foo; }
+              fn main() { foo::Foo<|> } ",
+            r"mod foo { <|>pub(crate) union Foo; }
+              fn main() { foo::Foo } ",
+        );
+        check_assist_not_applicable(
+            fix_visibility,
+            r"mod foo { pub union Foo; }
+              fn main() { foo::Foo<|> } ",
+        );
+    }
+
+    #[test]
+    fn fix_visibility_of_adt_in_other_file() {
+        check_assist(
+            fix_visibility,
+            r"
+              //- /main.rs
+              mod foo;
+              fn main() { foo::Foo<|> }
+
+              //- /foo.rs
+              struct Foo;
+              ",
+            r"<|>pub(crate) struct Foo;
+
+",
+        );
+    }
+
+    #[test]
+    fn fix_visibility_of_struct_field() {
+        check_assist(
+            fix_visibility,
+            r"mod foo { pub struct Foo { bar: (), } }
+              fn main() { foo::Foo { <|>bar: () }; } ",
+            r"mod foo { pub struct Foo { <|>pub(crate) bar: (), } }
+              fn main() { foo::Foo { bar: () }; } ",
+        );
+        check_assist(
+            fix_visibility,
+            r"//- /lib.rs
+              mod foo;
+              fn main() { foo::Foo { <|>bar: () }; }
+              //- /foo.rs
+              pub struct Foo { bar: () }
+              ",
+            r"pub struct Foo { <|>pub(crate) bar: () }
+
+",
+        );
+        check_assist_not_applicable(
+            fix_visibility,
+            r"mod foo { pub struct Foo { pub bar: (), } }
+              fn main() { foo::Foo { <|>bar: () }; } ",
+        );
+        check_assist_not_applicable(
+            fix_visibility,
+            r"//- /lib.rs
+              mod foo;
+              fn main() { foo::Foo { <|>bar: () }; }
+              //- /foo.rs
+              pub struct Foo { pub bar: () }
+              ",
+        );
+    }
+
+    #[test]
+    fn fix_visibility_of_enum_variant_field() {
+        check_assist(
+            fix_visibility,
+            r"mod foo { pub enum Foo { Bar { bar: () } } }
+              fn main() { foo::Foo::Bar { <|>bar: () }; } ",
+            r"mod foo { pub enum Foo { Bar { <|>pub(crate) bar: () } } }
+              fn main() { foo::Foo::Bar { bar: () }; } ",
+        );
+        check_assist(
+            fix_visibility,
+            r"//- /lib.rs
+              mod foo;
+              fn main() { foo::Foo::Bar { <|>bar: () }; }
+              //- /foo.rs
+              pub enum Foo { Bar { bar: () } }
+              ",
+            r"pub enum Foo { Bar { <|>pub(crate) bar: () } }
+
+",
+        );
+        check_assist_not_applicable(
+            fix_visibility,
+            r"mod foo { pub struct Foo { pub bar: (), } }
+              fn main() { foo::Foo { <|>bar: () }; } ",
+        );
+        check_assist_not_applicable(
+            fix_visibility,
+            r"//- /lib.rs
+              mod foo;
+              fn main() { foo::Foo { <|>bar: () }; }
+              //- /foo.rs
+              pub struct Foo { pub bar: () }
+              ",
+        );
+    }
+
+    #[test]
+    #[ignore]
+    // FIXME reenable this test when `Semantics::resolve_record_field` works with union fields
+    fn fix_visibility_of_union_field() {
+        check_assist(
+            fix_visibility,
+            r"mod foo { pub union Foo { bar: (), } }
+              fn main() { foo::Foo { <|>bar: () }; } ",
+            r"mod foo { pub union Foo { <|>pub(crate) bar: (), } }
+              fn main() { foo::Foo { bar: () }; } ",
+        );
+        check_assist(
+            fix_visibility,
+            r"//- /lib.rs
+              mod foo;
+              fn main() { foo::Foo { <|>bar: () }; }
+              //- /foo.rs
+              pub union Foo { bar: () }
+              ",
+            r"pub union Foo { <|>pub(crate) bar: () }
+
+",
+        );
+        check_assist_not_applicable(
+            fix_visibility,
+            r"mod foo { pub union Foo { pub bar: (), } }
+              fn main() { foo::Foo { <|>bar: () }; } ",
+        );
+        check_assist_not_applicable(
+            fix_visibility,
+            r"//- /lib.rs
+              mod foo;
+              fn main() { foo::Foo { <|>bar: () }; }
+              //- /foo.rs
+              pub union Foo { pub bar: () }
+              ",
+        );
+    }
+
+    #[test]
+    fn fix_visibility_of_const() {
+        check_assist(
+            fix_visibility,
+            r"mod foo { const FOO: () = (); }
+              fn main() { foo::FOO<|> } ",
+            r"mod foo { <|>pub(crate) const FOO: () = (); }
+              fn main() { foo::FOO } ",
+        );
+        check_assist_not_applicable(
+            fix_visibility,
+            r"mod foo { pub const FOO: () = (); }
+              fn main() { foo::FOO<|> } ",
+        );
+    }
+
+    #[test]
+    fn fix_visibility_of_static() {
+        check_assist(
+            fix_visibility,
+            r"mod foo { static FOO: () = (); }
+              fn main() { foo::FOO<|> } ",
+            r"mod foo { <|>pub(crate) static FOO: () = (); }
+              fn main() { foo::FOO } ",
+        );
+        check_assist_not_applicable(
+            fix_visibility,
+            r"mod foo { pub static FOO: () = (); }
+              fn main() { foo::FOO<|> } ",
+        );
+    }
+
+    #[test]
+    fn fix_visibility_of_trait() {
+        check_assist(
+            fix_visibility,
+            r"mod foo { trait Foo { fn foo(&self) {} } }
+              fn main() { let x: &dyn foo::<|>Foo; } ",
+            r"mod foo { <|>pub(crate) trait Foo { fn foo(&self) {} } }
+              fn main() { let x: &dyn foo::Foo; } ",
+        );
+        check_assist_not_applicable(
+            fix_visibility,
+            r"mod foo { pub trait Foo { fn foo(&self) {} } }
+              fn main() { let x: &dyn foo::Foo<|>; } ",
+        );
+    }
+
+    #[test]
+    fn fix_visibility_of_type_alias() {
+        check_assist(
+            fix_visibility,
+            r"mod foo { type Foo = (); }
+              fn main() { let x: foo::Foo<|>; } ",
+            r"mod foo { <|>pub(crate) type Foo = (); }
+              fn main() { let x: foo::Foo; } ",
+        );
+        check_assist_not_applicable(
+            fix_visibility,
+            r"mod foo { pub type Foo = (); }
+              fn main() { let x: foo::Foo<|>; } ",
+        );
+    }
+
+    #[test]
+    fn fix_visibility_of_module() {
+        check_assist(
+            fix_visibility,
+            r"mod foo { mod bar { fn bar() {} } }
+              fn main() { foo::bar<|>::bar(); } ",
+            r"mod foo { <|>pub(crate) mod bar { fn bar() {} } }
+              fn main() { foo::bar::bar(); } ",
+        );
+
+        check_assist(
+            fix_visibility,
+            r"
+            //- /main.rs
+            mod foo;
+            fn main() { foo::bar<|>::baz(); }
+
+            //- /foo.rs
+            mod bar {
+                pub fn baz() {}
+            }
+            ",
+            r"<|>pub(crate) mod bar {
+    pub fn baz() {}
+}
+
+",
+        );
+
+        check_assist_not_applicable(
+            fix_visibility,
+            r"mod foo { pub mod bar { pub fn bar() {} } }
+              fn main() { foo::bar<|>::bar(); } ",
+        );
+    }
+
+    #[test]
+    fn fix_visibility_of_inline_module_in_other_file() {
+        check_assist(
+            fix_visibility,
+            r"
+            //- /main.rs
+            mod foo;
+            fn main() { foo::bar<|>::baz(); }
+
+            //- /foo.rs
+            mod bar;
+
+            //- /foo/bar.rs
+            pub fn baz() {}
+            }
+            ",
+            r"<|>pub(crate) mod bar;
+",
+        );
+    }
+
+    #[test]
+    fn fix_visibility_of_module_declaration_in_other_file() {
+        check_assist(
+            fix_visibility,
+            r"//- /main.rs
+              mod foo;
+              fn main() { foo::bar<|>>::baz(); }
+
+              //- /foo.rs
+              mod bar {
+                  pub fn baz() {}
+              }",
+            r"<|>pub(crate) mod bar {
+    pub fn baz() {}
+}
+",
+        );
+    }
+
+    #[test]
+    fn adds_pub_when_target_is_in_another_crate() {
+        check_assist(
+            fix_visibility,
+            r"//- /main.rs crate:a deps:foo
+              foo::Bar<|>
+              //- /lib.rs crate:foo
+              struct Bar;",
+            r"<|>pub struct Bar;
+",
+        )
+    }
+
+    #[test]
+    #[ignore]
+    // FIXME handle reexports properly
+    fn fix_visibility_of_reexport() {
+        check_assist(
+            fix_visibility,
+            r"
+            mod foo {
+                use bar::Baz;
+                mod bar { pub(super) struct Baz; }
+            }
+            foo::Baz<|>
+            ",
+            r"
+            mod foo {
+                <|>pub(crate) use bar::Baz;
+                mod bar { pub(super) struct Baz; }
+            }
+            foo::Baz
+            ",
+        )
+    }
+}
diff --git a/crates/ra_assists/src/lib.rs b/crates/ra_assists/src/lib.rs
index a4a497c73..464bc03dd 100644
--- a/crates/ra_assists/src/lib.rs
+++ b/crates/ra_assists/src/lib.rs
@@ -116,6 +116,7 @@ mod handlers {
     mod change_visibility;
     mod early_return;
     mod fill_match_arms;
+    mod fix_visibility;
     mod flip_binexpr;
     mod flip_comma;
     mod flip_trait_bound;
@@ -154,6 +155,7 @@ mod handlers {
             change_visibility::change_visibility,
             early_return::convert_to_guarded_return,
             fill_match_arms::fill_match_arms,
+            fix_visibility::fix_visibility,
             flip_binexpr::flip_binexpr,
             flip_comma::flip_comma,
             flip_trait_bound::flip_trait_bound,
diff --git a/crates/ra_assists/src/tests/generated.rs b/crates/ra_assists/src/tests/generated.rs
index cd6129dc5..417ee89a8 100644
--- a/crates/ra_assists/src/tests/generated.rs
+++ b/crates/ra_assists/src/tests/generated.rs
@@ -345,6 +345,29 @@ fn handle(action: Action) {
 }
 
 #[test]
+fn doctest_fix_visibility() {
+    check_doc_test(
+        "fix_visibility",
+        r#####"
+mod m {
+    fn frobnicate() {}
+}
+fn main() {
+    m::frobnicate<|>() {}
+}
+"#####,
+        r#####"
+mod m {
+    pub(crate) fn frobnicate() {}
+}
+fn main() {
+    m::frobnicate() {}
+}
+"#####,
+    )
+}
+
+#[test]
 fn doctest_flip_binexpr() {
     check_doc_test(
         "flip_binexpr",