diff --git a/crates/ra_assists/src/assist_ctx.rs b/crates/ra_assists/src/assist_ctx.rs index f32072dbdb..81f999090a 100644 --- a/crates/ra_assists/src/assist_ctx.rs +++ b/crates/ra_assists/src/assist_ctx.rs @@ -19,6 +19,8 @@ pub(crate) enum Assist { Resolved { assist: ResolvedAssist }, } +pub(crate) type AssistHandler = fn(AssistCtx) -> Option; + /// `AssistCtx` allows to apply an assist or check if it could be applied. /// /// Assists use a somewhat over-engineered approach, given the current needs. The @@ -57,7 +59,7 @@ pub(crate) struct AssistCtx<'a> { should_compute_edit: bool, } -impl<'a> Clone for AssistCtx<'a> { +impl Clone for AssistCtx<'_> { fn clone(&self) -> Self { AssistCtx { db: self.db, @@ -69,31 +71,18 @@ impl<'a> Clone for AssistCtx<'a> { } impl<'a> AssistCtx<'a> { - pub(crate) fn with_ctx( - db: &RootDatabase, - frange: FileRange, - should_compute_edit: bool, - f: F, - ) -> T - where - F: FnOnce(AssistCtx) -> T, - { + pub fn new(db: &RootDatabase, frange: FileRange, should_compute_edit: bool) -> AssistCtx { let parse = db.parse(frange.file_id); - - let ctx = AssistCtx { db, frange, source_file: parse.tree(), should_compute_edit }; - f(ctx) + AssistCtx { db, frange, source_file: parse.tree(), should_compute_edit } } -} -impl<'a> AssistCtx<'a> { pub(crate) fn add_assist( self, id: AssistId, label: impl Into, f: impl FnOnce(&mut ActionBuilder), ) -> Option { - let label = AssistLabel { label: label.into(), id }; - assert!(label.label.chars().nth(0).unwrap().is_uppercase()); + let label = AssistLabel::new(label.into(), id); let assist = if self.should_compute_edit { let action = { @@ -115,7 +104,7 @@ impl<'a> AssistCtx<'a> { label: impl Into, f: impl FnOnce() -> Vec, ) -> Option { - let label = AssistLabel { label: label.into(), id }; + let label = AssistLabel::new(label.into(), id); let assist = if self.should_compute_edit { let actions = f(); assert!(!actions.is_empty(), "Assist cannot have no"); diff --git a/crates/ra_assists/src/assists/add_custom_impl.rs b/crates/ra_assists/src/handlers/add_custom_impl.rs similarity index 100% rename from crates/ra_assists/src/assists/add_custom_impl.rs rename to crates/ra_assists/src/handlers/add_custom_impl.rs diff --git a/crates/ra_assists/src/assists/add_derive.rs b/crates/ra_assists/src/handlers/add_derive.rs similarity index 100% rename from crates/ra_assists/src/assists/add_derive.rs rename to crates/ra_assists/src/handlers/add_derive.rs diff --git a/crates/ra_assists/src/assists/add_explicit_type.rs b/crates/ra_assists/src/handlers/add_explicit_type.rs similarity index 100% rename from crates/ra_assists/src/assists/add_explicit_type.rs rename to crates/ra_assists/src/handlers/add_explicit_type.rs diff --git a/crates/ra_assists/src/assists/add_impl.rs b/crates/ra_assists/src/handlers/add_impl.rs similarity index 100% rename from crates/ra_assists/src/assists/add_impl.rs rename to crates/ra_assists/src/handlers/add_impl.rs diff --git a/crates/ra_assists/src/assists/add_import.rs b/crates/ra_assists/src/handlers/add_import.rs similarity index 100% rename from crates/ra_assists/src/assists/add_import.rs rename to crates/ra_assists/src/handlers/add_import.rs diff --git a/crates/ra_assists/src/assists/add_missing_impl_members.rs b/crates/ra_assists/src/handlers/add_missing_impl_members.rs similarity index 100% rename from crates/ra_assists/src/assists/add_missing_impl_members.rs rename to crates/ra_assists/src/handlers/add_missing_impl_members.rs diff --git a/crates/ra_assists/src/assists/add_new.rs b/crates/ra_assists/src/handlers/add_new.rs similarity index 100% rename from crates/ra_assists/src/assists/add_new.rs rename to crates/ra_assists/src/handlers/add_new.rs diff --git a/crates/ra_assists/src/assists/apply_demorgan.rs b/crates/ra_assists/src/handlers/apply_demorgan.rs similarity index 96% rename from crates/ra_assists/src/assists/apply_demorgan.rs rename to crates/ra_assists/src/handlers/apply_demorgan.rs index ba08a8223b..239807e243 100644 --- a/crates/ra_assists/src/assists/apply_demorgan.rs +++ b/crates/ra_assists/src/handlers/apply_demorgan.rs @@ -1,7 +1,6 @@ -use super::invert_if::invert_boolean_expression; use ra_syntax::ast::{self, AstNode}; -use crate::{Assist, AssistCtx, AssistId}; +use crate::{utils::invert_boolean_expression, Assist, AssistCtx, AssistId}; // Assist: apply_demorgan // diff --git a/crates/ra_assists/src/assists/auto_import.rs b/crates/ra_assists/src/handlers/auto_import.rs similarity index 98% rename from crates/ra_assists/src/assists/auto_import.rs rename to crates/ra_assists/src/handlers/auto_import.rs index 10c4b7d7c3..84b5474f9d 100644 --- a/crates/ra_assists/src/assists/auto_import.rs +++ b/crates/ra_assists/src/handlers/auto_import.rs @@ -1,4 +1,5 @@ use hir::ModPath; +use ra_ide_db::imports_locator::ImportsLocator; use ra_syntax::{ ast::{self, AstNode}, SyntaxNode, @@ -8,7 +9,7 @@ use crate::{ assist_ctx::{ActionBuilder, Assist, AssistCtx}, auto_import_text_edit, AssistId, }; -use ra_ide_db::imports_locator::ImportsLocator; +use std::collections::BTreeSet; // Assist: auto_import // @@ -60,7 +61,8 @@ pub(crate) fn auto_import(ctx: AssistCtx) -> Option { .filter_map(|module_def| module_with_name_to_import.find_use_path(ctx.db, module_def)) .filter(|use_path| !use_path.segments.is_empty()) .take(20) - .collect::>(); + .collect::>(); + if proposed_imports.is_empty() { return None; } @@ -82,9 +84,10 @@ fn import_to_action(import: ModPath, position: &SyntaxNode, anchor: &SyntaxNode) #[cfg(test)] mod tests { - use super::*; use crate::helpers::{check_assist, check_assist_not_applicable}; + use super::*; + #[test] fn applicable_when_found_an_import() { check_assist( diff --git a/crates/ra_assists/src/assists/change_visibility.rs b/crates/ra_assists/src/handlers/change_visibility.rs similarity index 100% rename from crates/ra_assists/src/assists/change_visibility.rs rename to crates/ra_assists/src/handlers/change_visibility.rs diff --git a/crates/ra_assists/src/assists/early_return.rs b/crates/ra_assists/src/handlers/early_return.rs similarity index 99% rename from crates/ra_assists/src/assists/early_return.rs rename to crates/ra_assists/src/handlers/early_return.rs index 8f30dc5860..22f88884f4 100644 --- a/crates/ra_assists/src/assists/early_return.rs +++ b/crates/ra_assists/src/handlers/early_return.rs @@ -10,7 +10,7 @@ use ra_syntax::{ use crate::{ assist_ctx::{Assist, AssistCtx}, - assists::invert_if::invert_boolean_expression, + utils::invert_boolean_expression, AssistId, }; diff --git a/crates/ra_assists/src/assists/fill_match_arms.rs b/crates/ra_assists/src/handlers/fill_match_arms.rs similarity index 100% rename from crates/ra_assists/src/assists/fill_match_arms.rs rename to crates/ra_assists/src/handlers/fill_match_arms.rs diff --git a/crates/ra_assists/src/assists/flip_binexpr.rs b/crates/ra_assists/src/handlers/flip_binexpr.rs similarity index 100% rename from crates/ra_assists/src/assists/flip_binexpr.rs rename to crates/ra_assists/src/handlers/flip_binexpr.rs diff --git a/crates/ra_assists/src/assists/flip_comma.rs b/crates/ra_assists/src/handlers/flip_comma.rs similarity index 100% rename from crates/ra_assists/src/assists/flip_comma.rs rename to crates/ra_assists/src/handlers/flip_comma.rs diff --git a/crates/ra_assists/src/assists/flip_trait_bound.rs b/crates/ra_assists/src/handlers/flip_trait_bound.rs similarity index 100% rename from crates/ra_assists/src/assists/flip_trait_bound.rs rename to crates/ra_assists/src/handlers/flip_trait_bound.rs diff --git a/crates/ra_assists/src/assists/inline_local_variable.rs b/crates/ra_assists/src/handlers/inline_local_variable.rs similarity index 100% rename from crates/ra_assists/src/assists/inline_local_variable.rs rename to crates/ra_assists/src/handlers/inline_local_variable.rs diff --git a/crates/ra_assists/src/assists/introduce_variable.rs b/crates/ra_assists/src/handlers/introduce_variable.rs similarity index 100% rename from crates/ra_assists/src/assists/introduce_variable.rs rename to crates/ra_assists/src/handlers/introduce_variable.rs diff --git a/crates/ra_assists/src/assists/invert_if.rs b/crates/ra_assists/src/handlers/invert_if.rs similarity index 74% rename from crates/ra_assists/src/assists/invert_if.rs rename to crates/ra_assists/src/handlers/invert_if.rs index 983392f21e..a594e7e0c3 100644 --- a/crates/ra_assists/src/assists/invert_if.rs +++ b/crates/ra_assists/src/handlers/invert_if.rs @@ -1,7 +1,7 @@ -use ra_syntax::ast::{self, make, AstNode}; +use ra_syntax::ast::{self, AstNode}; use ra_syntax::T; -use crate::{Assist, AssistCtx, AssistId}; +use crate::{utils::invert_boolean_expression, Assist, AssistCtx, AssistId}; // Assist: invert_if // @@ -51,27 +51,6 @@ pub(crate) fn invert_if(ctx: AssistCtx) -> Option { None } -pub(crate) fn invert_boolean_expression(expr: ast::Expr) -> ast::Expr { - if let Some(expr) = invert_special_case(&expr) { - return expr; - } - make::expr_prefix(T![!], expr) -} - -pub(crate) fn invert_special_case(expr: &ast::Expr) -> Option { - match expr { - ast::Expr::BinExpr(bin) => match bin.op_kind()? { - ast::BinOp::NegatedEqualityTest => bin.replace_op(T![==]).map(|it| it.into()), - ast::BinOp::EqualityTest => bin.replace_op(T![!=]).map(|it| it.into()), - _ => None, - }, - ast::Expr::PrefixExpr(pe) if pe.op_kind()? == ast::PrefixOp::Not => pe.expr(), - // FIXME: - // ast::Expr::Literal(true | false ) - _ => None, - } -} - #[cfg(test)] mod tests { use super::*; diff --git a/crates/ra_assists/src/assists/merge_match_arms.rs b/crates/ra_assists/src/handlers/merge_match_arms.rs similarity index 100% rename from crates/ra_assists/src/assists/merge_match_arms.rs rename to crates/ra_assists/src/handlers/merge_match_arms.rs diff --git a/crates/ra_assists/src/assists/move_bounds.rs b/crates/ra_assists/src/handlers/move_bounds.rs similarity index 100% rename from crates/ra_assists/src/assists/move_bounds.rs rename to crates/ra_assists/src/handlers/move_bounds.rs diff --git a/crates/ra_assists/src/assists/move_guard.rs b/crates/ra_assists/src/handlers/move_guard.rs similarity index 100% rename from crates/ra_assists/src/assists/move_guard.rs rename to crates/ra_assists/src/handlers/move_guard.rs diff --git a/crates/ra_assists/src/assists/raw_string.rs b/crates/ra_assists/src/handlers/raw_string.rs similarity index 100% rename from crates/ra_assists/src/assists/raw_string.rs rename to crates/ra_assists/src/handlers/raw_string.rs diff --git a/crates/ra_assists/src/assists/remove_dbg.rs b/crates/ra_assists/src/handlers/remove_dbg.rs similarity index 100% rename from crates/ra_assists/src/assists/remove_dbg.rs rename to crates/ra_assists/src/handlers/remove_dbg.rs diff --git a/crates/ra_assists/src/assists/replace_if_let_with_match.rs b/crates/ra_assists/src/handlers/replace_if_let_with_match.rs similarity index 100% rename from crates/ra_assists/src/assists/replace_if_let_with_match.rs rename to crates/ra_assists/src/handlers/replace_if_let_with_match.rs diff --git a/crates/ra_assists/src/assists/split_import.rs b/crates/ra_assists/src/handlers/split_import.rs similarity index 100% rename from crates/ra_assists/src/assists/split_import.rs rename to crates/ra_assists/src/handlers/split_import.rs diff --git a/crates/ra_assists/src/lib.rs b/crates/ra_assists/src/lib.rs index 3f3df3f969..eca6dec4b0 100644 --- a/crates/ra_assists/src/lib.rs +++ b/crates/ra_assists/src/lib.rs @@ -9,16 +9,19 @@ mod assist_ctx; mod marks; #[cfg(test)] mod doc_tests; +mod utils; pub mod ast_transform; +use std::cmp::Ordering; + use either::Either; use ra_db::FileRange; use ra_ide_db::RootDatabase; use ra_syntax::{TextRange, TextUnit}; use ra_text_edit::TextEdit; -pub(crate) use crate::assist_ctx::{Assist, AssistCtx}; -pub use crate::assists::add_import::auto_import_text_edit; +pub(crate) use crate::assist_ctx::{Assist, AssistCtx, AssistHandler}; +pub use crate::handlers::add_import::auto_import_text_edit; /// Unique identifier of the assist, should not be shown to the user /// directly. @@ -32,11 +35,20 @@ pub struct AssistLabel { pub id: AssistId, } +impl AssistLabel { + pub(crate) fn new(label: String, id: AssistId) -> AssistLabel { + // FIXME: make fields private, so that this invariant can't be broken + assert!(label.chars().nth(0).unwrap().is_uppercase()); + AssistLabel { label: label.into(), id } + } +} + #[derive(Debug, Clone)] pub struct AssistAction { pub label: Option, pub edit: TextEdit, pub cursor_position: Option, + // FIXME: This belongs to `AssistLabel` pub target: Option, } @@ -60,16 +72,15 @@ impl ResolvedAssist { /// Assists are returned in the "unresolved" state, that is only labels are /// returned, without actual edits. pub fn unresolved_assists(db: &RootDatabase, range: FileRange) -> Vec { - AssistCtx::with_ctx(db, range, false, |ctx| { - assists::all() - .iter() - .filter_map(|f| f(ctx.clone())) - .map(|a| match a { - Assist::Unresolved { label } => label, - Assist::Resolved { .. } => unreachable!(), - }) - .collect() - }) + let ctx = AssistCtx::new(db, range, false); + handlers::all() + .iter() + .filter_map(|f| f(ctx.clone())) + .map(|a| match a { + Assist::Unresolved { label } => label, + Assist::Resolved { .. } => unreachable!(), + }) + .collect() } /// Return all the assists applicable at the given position. @@ -77,22 +88,20 @@ pub fn unresolved_assists(db: &RootDatabase, range: FileRange) -> Vec Vec { - AssistCtx::with_ctx(db, range, true, |ctx| { - let mut a = assists::all() - .iter() - .filter_map(|f| f(ctx.clone())) - .map(|a| match a { - Assist::Resolved { assist } => assist, - Assist::Unresolved { .. } => unreachable!(), - }) - .collect(); - sort_assists(&mut a); - a - }) + let ctx = AssistCtx::new(db, range, true); + let mut a = handlers::all() + .iter() + .filter_map(|f| f(ctx.clone())) + .map(|a| match a { + Assist::Resolved { assist } => assist, + Assist::Unresolved { .. } => unreachable!(), + }) + .collect::>(); + sort_assists(&mut a); + a } -fn sort_assists(assists: &mut Vec) { - use std::cmp::Ordering; +fn sort_assists(assists: &mut [ResolvedAssist]) { assists.sort_by(|a, b| match (a.get_first_action().target, b.get_first_action().target) { (Some(a), Some(b)) => a.len().cmp(&b.len()), (Some(_), None) => Ordering::Less, @@ -101,8 +110,8 @@ fn sort_assists(assists: &mut Vec) { }); } -mod assists { - use crate::{Assist, AssistCtx}; +mod handlers { + use crate::AssistHandler; mod add_derive; mod add_explicit_type; @@ -130,7 +139,7 @@ mod assists { mod move_bounds; mod early_return; - pub(crate) fn all() -> &'static [fn(AssistCtx) -> Option] { + pub(crate) fn all() -> &'static [AssistHandler] { &[ add_derive::add_derive, add_explicit_type::add_explicit_type, @@ -175,7 +184,7 @@ mod helpers { use ra_syntax::TextRange; use test_utils::{add_cursor, assert_eq_text, extract_offset, extract_range}; - use crate::{Assist, AssistCtx}; + use crate::{Assist, AssistCtx, AssistHandler}; pub(crate) fn with_single_file(text: &str) -> (RootDatabase, FileId) { let (mut db, file_id) = RootDatabase::with_single_file(text); @@ -186,13 +195,13 @@ mod helpers { (db, file_id) } - pub(crate) fn check_assist(assist: fn(AssistCtx) -> Option, before: &str, after: &str) { + pub(crate) fn check_assist(assist: AssistHandler, before: &str, after: &str) { let (before_cursor_pos, before) = extract_offset(before); let (db, file_id) = with_single_file(&before); let frange = FileRange { file_id, range: TextRange::offset_len(before_cursor_pos, 0.into()) }; let assist = - AssistCtx::with_ctx(&db, frange, true, assist).expect("code action is not applicable"); + assist(AssistCtx::new(&db, frange, true)).expect("code action is not applicable"); let action = match assist { Assist::Unresolved { .. } => unreachable!(), Assist::Resolved { assist } => assist.get_first_action(), @@ -210,16 +219,12 @@ mod helpers { assert_eq_text!(after, &actual); } - pub(crate) fn check_assist_range( - assist: fn(AssistCtx) -> Option, - before: &str, - after: &str, - ) { + pub(crate) fn check_assist_range(assist: AssistHandler, before: &str, after: &str) { let (range, before) = extract_range(before); let (db, file_id) = with_single_file(&before); let frange = FileRange { file_id, range }; let assist = - AssistCtx::with_ctx(&db, frange, true, assist).expect("code action is not applicable"); + assist(AssistCtx::new(&db, frange, true)).expect("code action is not applicable"); let action = match assist { Assist::Unresolved { .. } => unreachable!(), Assist::Resolved { assist } => assist.get_first_action(), @@ -232,17 +237,13 @@ mod helpers { assert_eq_text!(after, &actual); } - pub(crate) fn check_assist_target( - assist: fn(AssistCtx) -> Option, - before: &str, - target: &str, - ) { + pub(crate) fn check_assist_target(assist: AssistHandler, before: &str, target: &str) { let (before_cursor_pos, before) = extract_offset(before); let (db, file_id) = with_single_file(&before); let frange = FileRange { file_id, range: TextRange::offset_len(before_cursor_pos, 0.into()) }; let assist = - AssistCtx::with_ctx(&db, frange, true, assist).expect("code action is not applicable"); + assist(AssistCtx::new(&db, frange, true)).expect("code action is not applicable"); let action = match assist { Assist::Unresolved { .. } => unreachable!(), Assist::Resolved { assist } => assist.get_first_action(), @@ -252,16 +253,12 @@ mod helpers { assert_eq_text!(&before[range.start().to_usize()..range.end().to_usize()], target); } - pub(crate) fn check_assist_range_target( - assist: fn(AssistCtx) -> Option, - before: &str, - target: &str, - ) { + pub(crate) fn check_assist_range_target(assist: AssistHandler, before: &str, target: &str) { let (range, before) = extract_range(before); let (db, file_id) = with_single_file(&before); let frange = FileRange { file_id, range }; let assist = - AssistCtx::with_ctx(&db, frange, true, assist).expect("code action is not applicable"); + assist(AssistCtx::new(&db, frange, true)).expect("code action is not applicable"); let action = match assist { Assist::Unresolved { .. } => unreachable!(), Assist::Resolved { assist } => assist.get_first_action(), @@ -271,26 +268,20 @@ mod helpers { assert_eq_text!(&before[range.start().to_usize()..range.end().to_usize()], target); } - pub(crate) fn check_assist_not_applicable( - assist: fn(AssistCtx) -> Option, - before: &str, - ) { + pub(crate) fn check_assist_not_applicable(assist: AssistHandler, before: &str) { let (before_cursor_pos, before) = extract_offset(before); let (db, file_id) = with_single_file(&before); let frange = FileRange { file_id, range: TextRange::offset_len(before_cursor_pos, 0.into()) }; - let assist = AssistCtx::with_ctx(&db, frange, true, assist); + let assist = assist(AssistCtx::new(&db, frange, true)); assert!(assist.is_none()); } - pub(crate) fn check_assist_range_not_applicable( - assist: fn(AssistCtx) -> Option, - before: &str, - ) { + pub(crate) fn check_assist_range_not_applicable(assist: AssistHandler, before: &str) { let (range, before) = extract_range(before); let (db, file_id) = with_single_file(&before); let frange = FileRange { file_id, range }; - let assist = AssistCtx::with_ctx(&db, frange, true, assist); + let assist = assist(AssistCtx::new(&db, frange, true)); assert!(assist.is_none()); } } diff --git a/crates/ra_assists/src/utils.rs b/crates/ra_assists/src/utils.rs new file mode 100644 index 0000000000..0d57222956 --- /dev/null +++ b/crates/ra_assists/src/utils.rs @@ -0,0 +1,27 @@ +//! Assorted functions shared by several assists. + +use ra_syntax::{ + ast::{self, make}, + T, +}; + +pub(crate) fn invert_boolean_expression(expr: ast::Expr) -> ast::Expr { + if let Some(expr) = invert_special_case(&expr) { + return expr; + } + make::expr_prefix(T![!], expr) +} + +fn invert_special_case(expr: &ast::Expr) -> Option { + match expr { + ast::Expr::BinExpr(bin) => match bin.op_kind()? { + ast::BinOp::NegatedEqualityTest => bin.replace_op(T![==]).map(|it| it.into()), + ast::BinOp::EqualityTest => bin.replace_op(T![!=]).map(|it| it.into()), + _ => None, + }, + ast::Expr::PrefixExpr(pe) if pe.op_kind()? == ast::PrefixOp::Not => pe.expr(), + // FIXME: + // ast::Expr::Literal(true | false ) + _ => None, + } +} diff --git a/xtask/src/codegen.rs b/xtask/src/codegen.rs index efa638e060..a53d573359 100644 --- a/xtask/src/codegen.rs +++ b/xtask/src/codegen.rs @@ -25,7 +25,7 @@ const ERR_INLINE_TESTS_DIR: &str = "crates/ra_syntax/test_data/parser/inline/err pub const SYNTAX_KINDS: &str = "crates/ra_parser/src/syntax_kind/generated.rs"; pub const AST: &str = "crates/ra_syntax/src/ast/generated.rs"; -const ASSISTS_DIR: &str = "crates/ra_assists/src/assists"; +const ASSISTS_DIR: &str = "crates/ra_assists/src/handlers"; const ASSISTS_TESTS: &str = "crates/ra_assists/src/doc_tests/generated.rs"; const ASSISTS_DOCS: &str = "docs/user/assists.md"; diff --git a/xtask/tests/tidy-tests/docs.rs b/xtask/tests/tidy-tests/docs.rs index 8a005d6c4d..6a69e7d6a0 100644 --- a/xtask/tests/tidy-tests/docs.rs +++ b/xtask/tests/tidy-tests/docs.rs @@ -6,7 +6,7 @@ use xtask::project_root; fn is_exclude_dir(p: &Path) -> bool { // Test hopefully don't really need comments, and for assists we already // have special comments which are source of doc tests and user docs. - let exclude_dirs = ["tests", "test_data", "assists"]; + let exclude_dirs = ["tests", "test_data", "handlers"]; let mut cur_path = p; while let Some(path) = cur_path.parent() { if exclude_dirs.iter().any(|dir| path.ends_with(dir)) {