Merge #413

413: add visibility owner, refactor assists r=matklad a=matklad



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

crates/ra_analysis/src/imp.rs

@@ -6,7 +6,7 @@
     self, FnSignatureInfo, Problem, source_binder,
 };
 use ra_db::{FilesDatabase, SourceRoot, SourceRootId, SyntaxDatabase};
-use ra_editor::{self, find_node_at_offset, LocalEdit, Severity};
+use ra_editor::{self, find_node_at_offset, assists, LocalEdit, Severity};
 use ra_syntax::{
     algo::{find_covering_node, visit::{visitor, Visitor}},
     ast::{self, ArgListOwner, Expr, FnDef, NameOwner},
@@ -335,11 +335,11 @@ pub(crate) fn assists(&self, frange: FileRange) -> Vec<SourceChange> {
         let file = self.source_file(frange.file_id);
         let offset = frange.range.start();
         let actions = vec![
-            ra_editor::flip_comma(&file, offset).map(|f| f()),
-            ra_editor::add_derive(&file, offset).map(|f| f()),
-            ra_editor::add_impl(&file, offset).map(|f| f()),
-            ra_editor::make_pub_crate(&file, offset).map(|f| f()),
-            ra_editor::introduce_variable(&file, frange.range).map(|f| f()),
+            assists::flip_comma(&file, offset).map(|f| f()),
+            assists::add_derive(&file, offset).map(|f| f()),
+            assists::add_impl(&file, offset).map(|f| f()),
+            assists::change_visibility(&file, offset).map(|f| f()),
+            assists::introduce_variable(&file, frange.range).map(|f| f()),
         ];
         actions
             .into_iter()

crates/ra_editor/src/assists.rs

@@ -0,0 +1,34 @@
+//! This modules contains various "assits": suggestions for source code edits
+//! which are likely to occur at a given cursor positon. For example, if the
+//! cursor is on the `,`, a possible assist is swapping the elments around the
+//! comma.
+
+mod flip_comma;
+mod add_derive;
+mod add_impl;
+mod introduce_variable;
+mod change_visibility;
+
+use ra_text_edit::TextEdit;
+use ra_syntax::{Direction, SyntaxNodeRef, TextUnit};
+
+pub use self::{
+    flip_comma::flip_comma,
+    add_derive::add_derive,
+    add_impl::add_impl,
+    introduce_variable::introduce_variable,
+    change_visibility::change_visibility,
+};
+
+#[derive(Debug)]
+pub struct LocalEdit {
+    pub label: String,
+    pub edit: TextEdit,
+    pub cursor_position: Option<TextUnit>,
+}
+
+fn non_trivia_sibling(node: SyntaxNodeRef, direction: Direction) -> Option<SyntaxNodeRef> {
+    node.siblings(direction)
+        .skip(1)
+        .find(|node| !node.kind().is_trivia())
+}

crates/ra_editor/src/assists/add_derive.rs

@@ -0,0 +1,97 @@
+use ra_text_edit::TextEditBuilder;
+use ra_syntax::{
+    ast::{self, AstNode, AttrsOwner},
+    SourceFileNode,
+    SyntaxKind::{WHITESPACE, COMMENT},
+    TextUnit,
+};
+
+use crate::{
+    find_node_at_offset,
+    assists::LocalEdit,
+};
+
+pub fn add_derive<'a>(
+    file: &'a SourceFileNode,
+    offset: TextUnit,
+) -> Option<impl FnOnce() -> LocalEdit + 'a> {
+    let nominal = find_node_at_offset::<ast::NominalDef>(file.syntax(), offset)?;
+    let node_start = derive_insertion_offset(nominal)?;
+    return Some(move || {
+        let derive_attr = nominal
+            .attrs()
+            .filter_map(|x| x.as_call())
+            .filter(|(name, _arg)| name == "derive")
+            .map(|(_name, arg)| arg)
+            .next();
+        let mut edit = TextEditBuilder::new();
+        let offset = match derive_attr {
+            None => {
+                edit.insert(node_start, "#[derive()]\n".to_string());
+                node_start + TextUnit::of_str("#[derive(")
+            }
+            Some(tt) => tt.syntax().range().end() - TextUnit::of_char(')'),
+        };
+        LocalEdit {
+            label: "add `#[derive]`".to_string(),
+            edit: edit.finish(),
+            cursor_position: Some(offset),
+        }
+    });
+
+    // Insert `derive` after doc comments.
+    fn derive_insertion_offset(nominal: ast::NominalDef) -> Option<TextUnit> {
+        let non_ws_child = nominal
+            .syntax()
+            .children()
+            .find(|it| it.kind() != COMMENT && it.kind() != WHITESPACE)?;
+        Some(non_ws_child.range().start())
+    }
+}
+
+#[cfg(test)]
+mod tests {
+    use super::*;
+    use crate::test_utils::check_action;
+
+    #[test]
+    fn add_derive_new() {
+        check_action(
+            "struct Foo { a: i32, <|>}",
+            "#[derive(<|>)]\nstruct Foo { a: i32, }",
+            |file, off| add_derive(file, off).map(|f| f()),
+        );
+        check_action(
+            "struct Foo { <|> a: i32, }",
+            "#[derive(<|>)]\nstruct Foo {  a: i32, }",
+            |file, off| add_derive(file, off).map(|f| f()),
+        );
+    }
+
+    #[test]
+    fn add_derive_existing() {
+        check_action(
+            "#[derive(Clone)]\nstruct Foo { a: i32<|>, }",
+            "#[derive(Clone<|>)]\nstruct Foo { a: i32, }",
+            |file, off| add_derive(file, off).map(|f| f()),
+        );
+    }
+
+    #[test]
+    fn add_derive_new_with_doc_comment() {
+        check_action(
+            "
+/// `Foo` is a pretty important struct.
+/// It does stuff.
+struct Foo { a: i32<|>, }
+            ",
+            "
+/// `Foo` is a pretty important struct.
+/// It does stuff.
+#[derive(<|>)]
+struct Foo { a: i32, }
+            ",
+            |file, off| add_derive(file, off).map(|f| f()),
+        );
+    }
+}

crates/ra_editor/src/assists/add_impl.rs

@@ -0,0 +1,78 @@
+use join_to_string::join;
+use ra_text_edit::TextEditBuilder;
+use ra_syntax::{
+    ast::{self, AstNode, NameOwner, TypeParamsOwner},
+    SourceFileNode,
+    TextUnit,
+};
+
+use crate::{find_node_at_offset, assists::LocalEdit};
+
+pub fn add_impl<'a>(
+    file: &'a SourceFileNode,
+    offset: TextUnit,
+) -> Option<impl FnOnce() -> LocalEdit + 'a> {
+    let nominal = find_node_at_offset::<ast::NominalDef>(file.syntax(), offset)?;
+    let name = nominal.name()?;
+
+    Some(move || {
+        let type_params = nominal.type_param_list();
+        let mut edit = TextEditBuilder::new();
+        let start_offset = nominal.syntax().range().end();
+        let mut buf = String::new();
+        buf.push_str("\n\nimpl");
+        if let Some(type_params) = type_params {
+            type_params.syntax().text().push_to(&mut buf);
+        }
+        buf.push_str(" ");
+        buf.push_str(name.text().as_str());
+        if let Some(type_params) = type_params {
+            let lifetime_params = type_params
+                .lifetime_params()
+                .filter_map(|it| it.lifetime())
+                .map(|it| it.text());
+            let type_params = type_params
+                .type_params()
+                .filter_map(|it| it.name())
+                .map(|it| it.text());
+            join(lifetime_params.chain(type_params))
+                .surround_with("<", ">")
+                .to_buf(&mut buf);
+        }
+        buf.push_str(" {\n");
+        let offset = start_offset + TextUnit::of_str(&buf);
+        buf.push_str("\n}");
+        edit.insert(start_offset, buf);
+        LocalEdit {
+            label: "add impl".to_string(),
+            edit: edit.finish(),
+            cursor_position: Some(offset),
+        }
+    })
+}
+
+#[cfg(test)]
+mod tests {
+    use super::*;
+    use crate::test_utils::check_action;
+
+    #[test]
+    fn test_add_impl() {
+        check_action(
+            "struct Foo {<|>}\n",
+            "struct Foo {}\n\nimpl Foo {\n<|>\n}\n",
+            |file, off| add_impl(file, off).map(|f| f()),
+        );
+        check_action(
+            "struct Foo<T: Clone> {<|>}",
+            "struct Foo<T: Clone> {}\n\nimpl<T: Clone> Foo<T> {\n<|>\n}",
+            |file, off| add_impl(file, off).map(|f| f()),
+        );
+        check_action(
+            "struct Foo<'a, T: Foo<'a>> {<|>}",
+            "struct Foo<'a, T: Foo<'a>> {}\n\nimpl<'a, T: Foo<'a>> Foo<'a, T> {\n<|>\n}",
+            |file, off| add_impl(file, off).map(|f| f()),
+        );
+    }
+
+}

crates/ra_editor/src/assists/change_visibility.rs

@@ -0,0 +1,90 @@
+use ra_text_edit::TextEditBuilder;
+use ra_syntax::{
+    SourceFileNode,
+    algo::find_leaf_at_offset,
+    SyntaxKind::{VISIBILITY, FN_KW, MOD_KW, STRUCT_KW, ENUM_KW, TRAIT_KW, FN_DEF, MODULE, STRUCT_DEF, ENUM_DEF, TRAIT_DEF},
+    TextUnit,
+};
+
+use crate::assists::LocalEdit;
+
+pub fn change_visibility<'a>(
+    file: &'a SourceFileNode,
+    offset: TextUnit,
+) -> Option<impl FnOnce() -> LocalEdit + 'a> {
+    let syntax = file.syntax();
+
+    let keyword = find_leaf_at_offset(syntax, offset).find(|leaf| match leaf.kind() {
+        FN_KW | MOD_KW | STRUCT_KW | ENUM_KW | TRAIT_KW => true,
+        _ => false,
+    })?;
+    let parent = keyword.parent()?;
+    let def_kws = vec![FN_DEF, MODULE, STRUCT_DEF, ENUM_DEF, TRAIT_DEF];
+    let node_start = parent.range().start();
+    Some(move || {
+        let mut edit = TextEditBuilder::new();
+
+        if !def_kws.iter().any(|&def_kw| def_kw == parent.kind())
+            || parent.children().any(|child| child.kind() == VISIBILITY)
+        {
+            return LocalEdit {
+                label: "make pub crate".to_string(),
+                edit: edit.finish(),
+                cursor_position: Some(offset),
+            };
+        }
+
+        edit.insert(node_start, "pub(crate) ".to_string());
+        LocalEdit {
+            label: "make pub crate".to_string(),
+            edit: edit.finish(),
+            cursor_position: Some(node_start),
+        }
+    })
+}
+
+#[cfg(test)]
+mod tests {
+    use super::*;
+    use crate::test_utils::check_action;
+
+    #[test]
+    fn test_change_visibility() {
+        check_action(
+            "<|>fn foo() {}",
+            "<|>pub(crate) fn foo() {}",
+            |file, off| change_visibility(file, off).map(|f| f()),
+        );
+        check_action(
+            "f<|>n foo() {}",
+            "<|>pub(crate) fn foo() {}",
+            |file, off| change_visibility(file, off).map(|f| f()),
+        );
+        check_action(
+            "<|>struct Foo {}",
+            "<|>pub(crate) struct Foo {}",
+            |file, off| change_visibility(file, off).map(|f| f()),
+        );
+        check_action("<|>mod foo {}", "<|>pub(crate) mod foo {}", |file, off| {
+            change_visibility(file, off).map(|f| f())
+        });
+        check_action(
+            "<|>trait Foo {}",
+            "<|>pub(crate) trait Foo {}",
+            |file, off| change_visibility(file, off).map(|f| f()),
+        );
+        check_action("m<|>od {}", "<|>pub(crate) mod {}", |file, off| {
+            change_visibility(file, off).map(|f| f())
+        });
+        check_action(
+            "pub(crate) f<|>n foo() {}",
+            "pub(crate) f<|>n foo() {}",
+            |file, off| change_visibility(file, off).map(|f| f()),
+        );
+        check_action(
+            "unsafe f<|>n foo() {}",
+            "<|>pub(crate) unsafe fn foo() {}",
+            |file, off| change_visibility(file, off).map(|f| f()),
+        );
+    }
+}

crates/ra_editor/src/assists/flip_comma.rs

@@ -0,0 +1,45 @@
+use ra_text_edit::TextEditBuilder;
+use ra_syntax::{
+    algo::find_leaf_at_offset,
+    Direction, SourceFileNode,
+    SyntaxKind::COMMA,
+    TextUnit,
+};
+
+use crate::assists::{LocalEdit, non_trivia_sibling};
+
+pub fn flip_comma<'a>(
+    file: &'a SourceFileNode,
+    offset: TextUnit,
+) -> Option<impl FnOnce() -> LocalEdit + 'a> {
+    let syntax = file.syntax();
+
+    let comma = find_leaf_at_offset(syntax, offset).find(|leaf| leaf.kind() == COMMA)?;
+    let prev = non_trivia_sibling(comma, Direction::Prev)?;
+    let next = non_trivia_sibling(comma, Direction::Next)?;
+    Some(move || {
+        let mut edit = TextEditBuilder::new();
+        edit.replace(prev.range(), next.text().to_string());
+        edit.replace(next.range(), prev.text().to_string());
+        LocalEdit {
+            label: "flip comma".to_string(),
+            edit: edit.finish(),
+            cursor_position: None,
+        }
+    })
+}
+
+#[cfg(test)]
+mod tests {
+    use super::*;
+    use crate::test_utils::check_action;
+
+    #[test]
+    fn test_swap_comma() {
+        check_action(
+            "fn foo(x: i32,<|> y: Result<(), ()>) {}",
+            "fn foo(y: Result<(), ()>,<|> x: i32) {}",
+            |file, off| flip_comma(file, off).map(|f| f()),
+        )
+    }
+}

crates/ra_editor/src/assists/introduce_variable.rs

@@ -0,0 +1,156 @@
+use ra_text_edit::TextEditBuilder;
+use ra_syntax::{
+    algo::{find_covering_node},
+    ast::{self, AstNode},
+    SourceFileNode,
+    SyntaxKind::{WHITESPACE},
+    SyntaxNodeRef, TextRange, TextUnit,
+};
+
+use crate::assists::LocalEdit;
+
+pub fn introduce_variable<'a>(
+    file: &'a SourceFileNode,
+    range: TextRange,
+) -> Option<impl FnOnce() -> LocalEdit + 'a> {
+    let node = find_covering_node(file.syntax(), range);
+    let expr = node.ancestors().filter_map(ast::Expr::cast).next()?;
+
+    let anchor_stmt = anchor_stmt(expr)?;
+    let indent = anchor_stmt.prev_sibling()?;
+    if indent.kind() != WHITESPACE {
+        return None;
+    }
+    return Some(move || {
+        let mut buf = String::new();
+        let mut edit = TextEditBuilder::new();
+
+        buf.push_str("let var_name = ");
+        expr.syntax().text().push_to(&mut buf);
+        let is_full_stmt = if let Some(expr_stmt) = ast::ExprStmt::cast(anchor_stmt) {
+            Some(expr.syntax()) == expr_stmt.expr().map(|e| e.syntax())
+        } else {
+            false
+        };
+        if is_full_stmt {
+            edit.replace(expr.syntax().range(), buf);
+        } else {
+            buf.push_str(";");
+            indent.text().push_to(&mut buf);
+            edit.replace(expr.syntax().range(), "var_name".to_string());
+            edit.insert(anchor_stmt.range().start(), buf);
+        }
+        let cursor_position = anchor_stmt.range().start() + TextUnit::of_str("let ");
+        LocalEdit {
+            label: "introduce variable".to_string(),
+            edit: edit.finish(),
+            cursor_position: Some(cursor_position),
+        }
+    });
+
+    /// Statement or last in the block expression, which will follow
+    /// the freshly introduced var.
+    fn anchor_stmt(expr: ast::Expr) -> Option<SyntaxNodeRef> {
+        expr.syntax().ancestors().find(|&node| {
+            if ast::Stmt::cast(node).is_some() {
+                return true;
+            }
+            if let Some(expr) = node
+                .parent()
+                .and_then(ast::Block::cast)
+                .and_then(|it| it.expr())
+            {
+                if expr.syntax() == node {
+                    return true;
+                }
+            }
+            false
+        })
+    }
+}
+
+#[cfg(test)]
+mod tests {
+    use super::*;
+    use crate::test_utils::check_action_range;
+
+    #[test]
+    fn test_introduce_var_simple() {
+        check_action_range(
+            "
+fn foo() {
+    foo(<|>1 + 1<|>);
+}",
+            "
+fn foo() {
+    let <|>var_name = 1 + 1;
+    foo(var_name);
+}",
+            |file, range| introduce_variable(file, range).map(|f| f()),
+        );
+    }
+
+    #[test]
+    fn test_introduce_var_expr_stmt() {
+        check_action_range(
+            "
+fn foo() {
+    <|>1 + 1<|>;
+}",
+            "
+fn foo() {
+    let <|>var_name = 1 + 1;
+}",
+            |file, range| introduce_variable(file, range).map(|f| f()),
+        );
+    }
+
+    #[test]
+    fn test_introduce_var_part_of_expr_stmt() {
+        check_action_range(
+            "
+fn foo() {
+    <|>1<|> + 1;
+}",
+            "
+fn foo() {
+    let <|>var_name = 1;
+    var_name + 1;
+}",
+            |file, range| introduce_variable(file, range).map(|f| f()),
+        );
+    }
+
+    #[test]
+    fn test_introduce_var_last_expr() {
+        check_action_range(
+            "
+fn foo() {
+    bar(<|>1 + 1<|>)
+}",
+            "
+fn foo() {
+    let <|>var_name = 1 + 1;
+    bar(var_name)
+}",
+            |file, range| introduce_variable(file, range).map(|f| f()),
+        );
+    }
+
+    #[test]
+    fn test_introduce_var_last_full_expr() {
+        check_action_range(
+            "
+fn foo() {
+    <|>bar(1 + 1)<|>
+}",
+            "
+fn foo() {
+    let <|>var_name = bar(1 + 1);
+    var_name
+}",
+            |file, range| introduce_variable(file, range).map(|f| f()),
+        );
+    }
+
+}

crates/ra_editor/src/code_actions.rs

@@ -1,415 +0,0 @@
-use join_to_string::join;
-
-use ra_syntax::{
-    algo::{find_covering_node, find_leaf_at_offset},
-    ast::{self, AstNode, AttrsOwner, NameOwner, TypeParamsOwner},
-    Direction, SourceFileNode,
-    SyntaxKind::{COMMA, WHITESPACE, COMMENT, VISIBILITY, FN_KW, MOD_KW, STRUCT_KW, ENUM_KW, TRAIT_KW, FN_DEF, MODULE, STRUCT_DEF, ENUM_DEF, TRAIT_DEF},
-    SyntaxNodeRef, TextRange, TextUnit,
-};
-
-use crate::{find_node_at_offset, TextEdit, TextEditBuilder};
-
-#[derive(Debug)]
-pub struct LocalEdit {
-    pub label: String,
-    pub edit: TextEdit,
-    pub cursor_position: Option<TextUnit>,
-}
-
-pub fn flip_comma<'a>(
-    file: &'a SourceFileNode,
-    offset: TextUnit,
-) -> Option<impl FnOnce() -> LocalEdit + 'a> {
-    let syntax = file.syntax();
-
-    let comma = find_leaf_at_offset(syntax, offset).find(|leaf| leaf.kind() == COMMA)?;
-    let prev = non_trivia_sibling(comma, Direction::Prev)?;
-    let next = non_trivia_sibling(comma, Direction::Next)?;
-    Some(move || {
-        let mut edit = TextEditBuilder::new();
-        edit.replace(prev.range(), next.text().to_string());
-        edit.replace(next.range(), prev.text().to_string());
-        LocalEdit {
-            label: "flip comma".to_string(),
-            edit: edit.finish(),
-            cursor_position: None,
-        }
-    })
-}
-
-pub fn add_derive<'a>(
-    file: &'a SourceFileNode,
-    offset: TextUnit,
-) -> Option<impl FnOnce() -> LocalEdit + 'a> {
-    let nominal = find_node_at_offset::<ast::NominalDef>(file.syntax(), offset)?;
-    let node_start = derive_insertion_offset(nominal)?;
-    return Some(move || {
-        let derive_attr = nominal
-            .attrs()
-            .filter_map(|x| x.as_call())
-            .filter(|(name, _arg)| name == "derive")
-            .map(|(_name, arg)| arg)
-            .next();
-        let mut edit = TextEditBuilder::new();
-        let offset = match derive_attr {
-            None => {
-                edit.insert(node_start, "#[derive()]\n".to_string());
-                node_start + TextUnit::of_str("#[derive(")
-            }
-            Some(tt) => tt.syntax().range().end() - TextUnit::of_char(')'),
-        };
-        LocalEdit {
-            label: "add `#[derive]`".to_string(),
-            edit: edit.finish(),
-            cursor_position: Some(offset),
-        }
-    });
-
-    // Insert `derive` after doc comments.
-    fn derive_insertion_offset(nominal: ast::NominalDef) -> Option<TextUnit> {
-        let non_ws_child = nominal
-            .syntax()
-            .children()
-            .find(|it| it.kind() != COMMENT && it.kind() != WHITESPACE)?;
-        Some(non_ws_child.range().start())
-    }
-}
-
-pub fn add_impl<'a>(
-    file: &'a SourceFileNode,
-    offset: TextUnit,
-) -> Option<impl FnOnce() -> LocalEdit + 'a> {
-    let nominal = find_node_at_offset::<ast::NominalDef>(file.syntax(), offset)?;
-    let name = nominal.name()?;
-
-    Some(move || {
-        let type_params = nominal.type_param_list();
-        let mut edit = TextEditBuilder::new();
-        let start_offset = nominal.syntax().range().end();
-        let mut buf = String::new();
-        buf.push_str("\n\nimpl");
-        if let Some(type_params) = type_params {
-            type_params.syntax().text().push_to(&mut buf);
-        }
-        buf.push_str(" ");
-        buf.push_str(name.text().as_str());
-        if let Some(type_params) = type_params {
-            let lifetime_params = type_params
-                .lifetime_params()
-                .filter_map(|it| it.lifetime())
-                .map(|it| it.text());
-            let type_params = type_params
-                .type_params()
-                .filter_map(|it| it.name())
-                .map(|it| it.text());
-            join(lifetime_params.chain(type_params))
-                .surround_with("<", ">")
-                .to_buf(&mut buf);
-        }
-        buf.push_str(" {\n");
-        let offset = start_offset + TextUnit::of_str(&buf);
-        buf.push_str("\n}");
-        edit.insert(start_offset, buf);
-        LocalEdit {
-            label: "add impl".to_string(),
-            edit: edit.finish(),
-            cursor_position: Some(offset),
-        }
-    })
-}
-
-pub fn introduce_variable<'a>(
-    file: &'a SourceFileNode,
-    range: TextRange,
-) -> Option<impl FnOnce() -> LocalEdit + 'a> {
-    let node = find_covering_node(file.syntax(), range);
-    let expr = node.ancestors().filter_map(ast::Expr::cast).next()?;
-
-    let anchor_stmt = anchor_stmt(expr)?;
-    let indent = anchor_stmt.prev_sibling()?;
-    if indent.kind() != WHITESPACE {
-        return None;
-    }
-    return Some(move || {
-        let mut buf = String::new();
-        let mut edit = TextEditBuilder::new();
-
-        buf.push_str("let var_name = ");
-        expr.syntax().text().push_to(&mut buf);
-        let is_full_stmt = if let Some(expr_stmt) = ast::ExprStmt::cast(anchor_stmt) {
-            Some(expr.syntax()) == expr_stmt.expr().map(|e| e.syntax())
-        } else {
-            false
-        };
-        if is_full_stmt {
-            edit.replace(expr.syntax().range(), buf);
-        } else {
-            buf.push_str(";");
-            indent.text().push_to(&mut buf);
-            edit.replace(expr.syntax().range(), "var_name".to_string());
-            edit.insert(anchor_stmt.range().start(), buf);
-        }
-        let cursor_position = anchor_stmt.range().start() + TextUnit::of_str("let ");
-        LocalEdit {
-            label: "introduce variable".to_string(),
-            edit: edit.finish(),
-            cursor_position: Some(cursor_position),
-        }
-    });
-
-    /// Statement or last in the block expression, which will follow
-    /// the freshly introduced var.
-    fn anchor_stmt(expr: ast::Expr) -> Option<SyntaxNodeRef> {
-        expr.syntax().ancestors().find(|&node| {
-            if ast::Stmt::cast(node).is_some() {
-                return true;
-            }
-            if let Some(expr) = node
-                .parent()
-                .and_then(ast::Block::cast)
-                .and_then(|it| it.expr())
-            {
-                if expr.syntax() == node {
-                    return true;
-                }
-            }
-            false
-        })
-    }
-}
-
-pub fn make_pub_crate<'a>(
-    file: &'a SourceFileNode,
-    offset: TextUnit,
-) -> Option<impl FnOnce() -> LocalEdit + 'a> {
-    let syntax = file.syntax();
-
-    let keyword = find_leaf_at_offset(syntax, offset).find(|leaf| match leaf.kind() {
-        FN_KW | MOD_KW | STRUCT_KW | ENUM_KW | TRAIT_KW => true,
-        _ => false,
-    })?;
-    let parent = keyword.parent()?;
-    let def_kws = vec![FN_DEF, MODULE, STRUCT_DEF, ENUM_DEF, TRAIT_DEF];
-    let node_start = parent.range().start();
-    Some(move || {
-        let mut edit = TextEditBuilder::new();
-
-        if !def_kws.iter().any(|&def_kw| def_kw == parent.kind())
-            || parent.children().any(|child| child.kind() == VISIBILITY)
-        {
-            return LocalEdit {
-                label: "make pub crate".to_string(),
-                edit: edit.finish(),
-                cursor_position: Some(offset),
-            };
-        }
-
-        edit.insert(node_start, "pub(crate) ".to_string());
-        LocalEdit {
-            label: "make pub crate".to_string(),
-            edit: edit.finish(),
-            cursor_position: Some(node_start),
-        }
-    })
-}
-
-fn non_trivia_sibling(node: SyntaxNodeRef, direction: Direction) -> Option<SyntaxNodeRef> {
-    node.siblings(direction)
-        .skip(1)
-        .find(|node| !node.kind().is_trivia())
-}
-
-#[cfg(test)]
-mod tests {
-    use super::*;
-    use crate::test_utils::{check_action, check_action_range};
-
-    #[test]
-    fn test_swap_comma() {
-        check_action(
-            "fn foo(x: i32,<|> y: Result<(), ()>) {}",
-            "fn foo(y: Result<(), ()>,<|> x: i32) {}",
-            |file, off| flip_comma(file, off).map(|f| f()),
-        )
-    }
-
-    #[test]
-    fn add_derive_new() {
-        check_action(
-            "struct Foo { a: i32, <|>}",
-            "#[derive(<|>)]\nstruct Foo { a: i32, }",
-            |file, off| add_derive(file, off).map(|f| f()),
-        );
-        check_action(
-            "struct Foo { <|> a: i32, }",
-            "#[derive(<|>)]\nstruct Foo {  a: i32, }",
-            |file, off| add_derive(file, off).map(|f| f()),
-        );
-    }
-
-    #[test]
-    fn add_derive_existing() {
-        check_action(
-            "#[derive(Clone)]\nstruct Foo { a: i32<|>, }",
-            "#[derive(Clone<|>)]\nstruct Foo { a: i32, }",
-            |file, off| add_derive(file, off).map(|f| f()),
-        );
-    }
-
-    #[test]
-    fn add_derive_new_with_doc_comment() {
-        check_action(
-            "
-/// `Foo` is a pretty important struct.
-/// It does stuff.
-struct Foo { a: i32<|>, }
-            ",
-            "
-/// `Foo` is a pretty important struct.
-/// It does stuff.
-#[derive(<|>)]
-struct Foo { a: i32, }
-            ",
-            |file, off| add_derive(file, off).map(|f| f()),
-        );
-    }
-
-    #[test]
-    fn test_add_impl() {
-        check_action(
-            "struct Foo {<|>}\n",
-            "struct Foo {}\n\nimpl Foo {\n<|>\n}\n",
-            |file, off| add_impl(file, off).map(|f| f()),
-        );
-        check_action(
-            "struct Foo<T: Clone> {<|>}",
-            "struct Foo<T: Clone> {}\n\nimpl<T: Clone> Foo<T> {\n<|>\n}",
-            |file, off| add_impl(file, off).map(|f| f()),
-        );
-        check_action(
-            "struct Foo<'a, T: Foo<'a>> {<|>}",
-            "struct Foo<'a, T: Foo<'a>> {}\n\nimpl<'a, T: Foo<'a>> Foo<'a, T> {\n<|>\n}",
-            |file, off| add_impl(file, off).map(|f| f()),
-        );
-    }
-
-    #[test]
-    fn test_introduce_var_simple() {
-        check_action_range(
-            "
-fn foo() {
-    foo(<|>1 + 1<|>);
-}",
-            "
-fn foo() {
-    let <|>var_name = 1 + 1;
-    foo(var_name);
-}",
-            |file, range| introduce_variable(file, range).map(|f| f()),
-        );
-    }
-
-    #[test]
-    fn test_introduce_var_expr_stmt() {
-        check_action_range(
-            "
-fn foo() {
-    <|>1 + 1<|>;
-}",
-            "
-fn foo() {
-    let <|>var_name = 1 + 1;
-}",
-            |file, range| introduce_variable(file, range).map(|f| f()),
-        );
-    }
-
-    #[test]
-    fn test_introduce_var_part_of_expr_stmt() {
-        check_action_range(
-            "
-fn foo() {
-    <|>1<|> + 1;
-}",
-            "
-fn foo() {
-    let <|>var_name = 1;
-    var_name + 1;
-}",
-            |file, range| introduce_variable(file, range).map(|f| f()),
-        );
-    }
-
-    #[test]
-    fn test_introduce_var_last_expr() {
-        check_action_range(
-            "
-fn foo() {
-    bar(<|>1 + 1<|>)
-}",
-            "
-fn foo() {
-    let <|>var_name = 1 + 1;
-    bar(var_name)
-}",
-            |file, range| introduce_variable(file, range).map(|f| f()),
-        );
-    }
-
-    #[test]
-    fn test_introduce_var_last_full_expr() {
-        check_action_range(
-            "
-fn foo() {
-    <|>bar(1 + 1)<|>
-}",
-            "
-fn foo() {
-    let <|>var_name = bar(1 + 1);
-    var_name
-}",
-            |file, range| introduce_variable(file, range).map(|f| f()),
-        );
-    }
-
-    #[test]
-    fn test_make_pub_crate() {
-        check_action(
-            "<|>fn foo() {}",
-            "<|>pub(crate) fn foo() {}",
-            |file, off| make_pub_crate(file, off).map(|f| f()),
-        );
-        check_action(
-            "f<|>n foo() {}",
-            "<|>pub(crate) fn foo() {}",
-            |file, off| make_pub_crate(file, off).map(|f| f()),
-        );
-        check_action(
-            "<|>struct Foo {}",
-            "<|>pub(crate) struct Foo {}",
-            |file, off| make_pub_crate(file, off).map(|f| f()),
-        );
-        check_action("<|>mod foo {}", "<|>pub(crate) mod foo {}", |file, off| {
-            make_pub_crate(file, off).map(|f| f())
-        });
-        check_action(
-            "<|>trait Foo {}",
-            "<|>pub(crate) trait Foo {}",
-            |file, off| make_pub_crate(file, off).map(|f| f()),
-        );
-        check_action("m<|>od {}", "<|>pub(crate) mod {}", |file, off| {
-            make_pub_crate(file, off).map(|f| f())
-        });
-        check_action(
-            "pub(crate) f<|>n foo() {}",
-            "pub(crate) f<|>n foo() {}",
-            |file, off| make_pub_crate(file, off).map(|f| f()),
-        );
-        check_action(
-            "unsafe f<|>n foo() {}",
-            "<|>pub(crate) unsafe fn foo() {}",
-            |file, off| make_pub_crate(file, off).map(|f| f()),
-        );
-    }
-}

crates/ra_editor/src/lib.rs

@@ -1,4 +1,4 @@
-mod code_actions;
+pub mod assists;
 mod extend_selection;
 mod folding_ranges;
 mod line_index;
@@ -10,7 +10,7 @@
 mod diagnostics;
 
 pub use self::{
-    code_actions::{add_derive, add_impl, flip_comma, introduce_variable, make_pub_crate, LocalEdit},
+    assists::LocalEdit,
     extend_selection::extend_selection,
     folding_ranges::{folding_ranges, Fold, FoldKind},
     line_index::{LineCol, LineIndex},
@@ -19,7 +19,7 @@
     typing::{join_lines, on_enter, on_eq_typed},
     diagnostics::diagnostics
 };
-use ra_text_edit::{TextEdit, TextEditBuilder};
+use ra_text_edit::TextEditBuilder;
 use ra_syntax::{
     algo::find_leaf_at_offset,
     ast::{self, AstNode},

crates/ra_syntax/src/ast.rs

@@ -30,6 +30,12 @@ fn name(self) -> Option<Name<'a>> {
     }
 }
 
+pub trait VisibilityOwner<'a>: AstNode<'a> {
+    fn visibility(self) -> Option<Visibility<'a>> {
+        child_opt(self)
+    }
+}
+
 pub trait LoopBodyOwner<'a>: AstNode<'a> {
     fn loop_body(self) -> Option<Block<'a>> {
         child_opt(self)

crates/ra_syntax/src/ast/generated.rs

@@ -695,6 +695,7 @@ pub fn owned(&self) -> ConstDefNode {
 }
 
 
+impl<'a> ast::VisibilityOwner<'a> for ConstDef<'a> {}
 impl<'a> ast::NameOwner<'a> for ConstDef<'a> {}
 impl<'a> ast::TypeParamsOwner<'a> for ConstDef<'a> {}
 impl<'a> ast::AttrsOwner<'a> for ConstDef<'a> {}
@@ -810,6 +811,7 @@ pub fn owned(&self) -> EnumDefNode {
 }
 
 
+impl<'a> ast::VisibilityOwner<'a> for EnumDef<'a> {}
 impl<'a> ast::NameOwner<'a> for EnumDef<'a> {}
 impl<'a> ast::TypeParamsOwner<'a> for EnumDef<'a> {}
 impl<'a> ast::AttrsOwner<'a> for EnumDef<'a> {}
@@ -1213,6 +1215,7 @@ pub fn owned(&self) -> FnDefNode {
 }
 
 
+impl<'a> ast::VisibilityOwner<'a> for FnDef<'a> {}
 impl<'a> ast::NameOwner<'a> for FnDef<'a> {}
 impl<'a> ast::TypeParamsOwner<'a> for FnDef<'a> {}
 impl<'a> ast::AttrsOwner<'a> for FnDef<'a> {}
@@ -2136,6 +2139,7 @@ pub fn owned(&self) -> ModuleNode {
 }
 
 
+impl<'a> ast::VisibilityOwner<'a> for Module<'a> {}
 impl<'a> ast::NameOwner<'a> for Module<'a> {}
 impl<'a> ast::AttrsOwner<'a> for Module<'a> {}
 impl<'a> ast::DocCommentsOwner<'a> for Module<'a> {}
@@ -2351,6 +2355,7 @@ pub fn owned(&self) -> NamedFieldDefNode {
 }
 
 
+impl<'a> ast::VisibilityOwner<'a> for NamedFieldDef<'a> {}
 impl<'a> ast::NameOwner<'a> for NamedFieldDef<'a> {}
 impl<'a> ast::AttrsOwner<'a> for NamedFieldDef<'a> {}
 impl<'a> NamedFieldDef<'a> {
@@ -3082,6 +3087,7 @@ pub fn owned(&self) -> PosFieldNode {
 }
 
 
+impl<'a> ast::VisibilityOwner<'a> for PosField<'a> {}
 impl<'a> ast::AttrsOwner<'a> for PosField<'a> {}
 impl<'a> PosField<'a> {
     pub fn type_ref(self) -> Option<TypeRef<'a>> {
@@ -3639,6 +3645,7 @@ pub fn owned(&self) -> StaticDefNode {
 }
 
 
+impl<'a> ast::VisibilityOwner<'a> for StaticDef<'a> {}
 impl<'a> ast::NameOwner<'a> for StaticDef<'a> {}
 impl<'a> ast::TypeParamsOwner<'a> for StaticDef<'a> {}
 impl<'a> ast::AttrsOwner<'a> for StaticDef<'a> {}
@@ -3742,6 +3749,7 @@ pub fn owned(&self) -> StructDefNode {
 }
 
 
+impl<'a> ast::VisibilityOwner<'a> for StructDef<'a> {}
 impl<'a> ast::NameOwner<'a> for StructDef<'a> {}
 impl<'a> ast::TypeParamsOwner<'a> for StructDef<'a> {}
 impl<'a> ast::AttrsOwner<'a> for StructDef<'a> {}
@@ -3902,6 +3910,7 @@ pub fn owned(&self) -> TraitDefNode {
 }
 
 
+impl<'a> ast::VisibilityOwner<'a> for TraitDef<'a> {}
 impl<'a> ast::NameOwner<'a> for TraitDef<'a> {}
 impl<'a> ast::AttrsOwner<'a> for TraitDef<'a> {}
 impl<'a> ast::DocCommentsOwner<'a> for TraitDef<'a> {}
@@ -4135,6 +4144,7 @@ pub fn owned(&self) -> TypeDefNode {
 }
 
 
+impl<'a> ast::VisibilityOwner<'a> for TypeDef<'a> {}
 impl<'a> ast::NameOwner<'a> for TypeDef<'a> {}
 impl<'a> ast::TypeParamsOwner<'a> for TypeDef<'a> {}
 impl<'a> ast::AttrsOwner<'a> for TypeDef<'a> {}
@@ -4409,6 +4419,43 @@ pub fn use_trees(self) -> impl Iterator<Item = UseTree<'a>> + 'a {
     }
 }
 
+// Visibility
+#[derive(Debug, Clone, Copy,)]
+pub struct VisibilityNode<R: TreeRoot<RaTypes> = OwnedRoot> {
+    pub(crate) syntax: SyntaxNode<R>,
+}
+pub type Visibility<'a> = VisibilityNode<RefRoot<'a>>;
+
+impl<R1: TreeRoot<RaTypes>, R2: TreeRoot<RaTypes>> PartialEq<VisibilityNode<R1>> for VisibilityNode<R2> {
+    fn eq(&self, other: &VisibilityNode<R1>) -> bool { self.syntax == other.syntax }
+}
+impl<R: TreeRoot<RaTypes>> Eq for VisibilityNode<R> {}
+impl<R: TreeRoot<RaTypes>> Hash for VisibilityNode<R> {
+    fn hash<H: Hasher>(&self, state: &mut H) { self.syntax.hash(state) }
+}
+
+impl<'a> AstNode<'a> for Visibility<'a> {
+    fn cast(syntax: SyntaxNodeRef<'a>) -> Option<Self> {
+        match syntax.kind() {
+            VISIBILITY => Some(Visibility { syntax }),
+            _ => None,
+        }
+    }
+    fn syntax(self) -> SyntaxNodeRef<'a> { self.syntax }
+}
+
+impl<R: TreeRoot<RaTypes>> VisibilityNode<R> {
+    pub fn borrowed(&self) -> Visibility {
+        VisibilityNode { syntax: self.syntax.borrowed() }
+    }
+    pub fn owned(&self) -> VisibilityNode {
+        VisibilityNode { syntax: self.syntax.owned() }
+    }
+}
+
+
+impl<'a> Visibility<'a> {}
+
 // WhereClause
 #[derive(Debug, Clone, Copy,)]
 pub struct WhereClauseNode<R: TreeRoot<RaTypes> = OwnedRoot> {

crates/ra_syntax/src/grammar.ron

@@ -247,6 +247,7 @@ Grammar(
         ),
         "FnDef": (
             traits: [
+                "VisibilityOwner",
                 "NameOwner",
                 "TypeParamsOwner",
                 "AttrsOwner",
@@ -257,6 +258,7 @@ Grammar(
         "RetType": (options: ["TypeRef"]),
         "StructDef": (
             traits: [
+                "VisibilityOwner",
                 "NameOwner",
                 "TypeParamsOwner",
                 "AttrsOwner",
@@ -264,10 +266,11 @@ Grammar(
             ]
         ),
         "NamedFieldDefList": (collections: [["fields", "NamedFieldDef"]]),
-        "NamedFieldDef": ( traits: ["NameOwner", "AttrsOwner"], options: ["TypeRef"] ),
+        "NamedFieldDef": ( traits: ["VisibilityOwner", "NameOwner", "AttrsOwner"], options: ["TypeRef"] ),
         "PosFieldList": (collections: [["fields", "PosField"]]),
-        "PosField": ( traits: ["AttrsOwner"], options: ["TypeRef"]),
+        "PosField": ( traits: ["VisibilityOwner", "AttrsOwner"], options: ["TypeRef"]),
         "EnumDef": ( traits: [
+            "VisibilityOwner",
             "NameOwner",
             "TypeParamsOwner",
             "AttrsOwner",
@@ -275,27 +278,30 @@ Grammar(
         ], options: [["variant_list", "EnumVariantList"]] ),
         "EnumVariantList": ( collections: [["variants", "EnumVariant"]] ),
         "EnumVariant": ( traits: ["NameOwner"], options: ["Expr"] ),
-        "TraitDef": ( traits: ["NameOwner", "AttrsOwner", "DocCommentsOwner"] ),
+        "TraitDef": ( traits: ["VisibilityOwner", "NameOwner", "AttrsOwner", "DocCommentsOwner"] ),
         "Module": (
-            traits: ["NameOwner", "AttrsOwner", "DocCommentsOwner" ],
+            traits: ["VisibilityOwner", "NameOwner", "AttrsOwner", "DocCommentsOwner" ],
             options: [ "ItemList" ]
         ),
         "ItemList": (
             traits: [ "FnDefOwner", "ModuleItemOwner" ],
         ),
         "ConstDef": ( traits: [
+            "VisibilityOwner",
             "NameOwner",
             "TypeParamsOwner",
             "AttrsOwner",
             "DocCommentsOwner"
         ] ),
         "StaticDef": ( traits: [
+            "VisibilityOwner",
             "NameOwner",
             "TypeParamsOwner",
             "AttrsOwner",
             "DocCommentsOwner"
         ] ),
         "TypeDef": ( traits: [
+            "VisibilityOwner",
             "NameOwner",
             "TypeParamsOwner",
             "AttrsOwner",
@@ -482,6 +488,7 @@ Grammar(
             ],
         ),
 
+        "Visibility": (),
         "Name": (),
         "NameRef": (),
         "MacroCall": ( options: [ "TokenTree", "Path" ] ),