mikros/rust
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

Merge #768

Browse Source 
768: Sort assists by the range of the affected element r=matklad a=robojumper

Closes #763.

3be98f2ac9/crates/ra_assists/src/lib.rs (L233-L236)

This could be made more robust by a) adding a way to identify actions by things other than their label and b) allowing arbitrary actions to appear in the list as long as the tested actions are there in the correct order. Let me know if I should do any of that.


Co-authored-by: robojumper <robojumper@gmail.com>
This commit is contained in:
bors[bot] 2019-02-09 08:52:09 +00:00
commit 3e8351fb06
12 changed files with 264 additions and 22 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

2
Cargo.lock generated
View File

@ -1,3 +1,5 @@
# This file is automatically @generated by Cargo.
# It is not intended for manual editing.
[[package]]
name = "aho-corasick"
version = "0.6.9"

20
crates/ra_assists/src/add_derive.rs
View File

@ -24,6 +24,7 @@ pub(crate) fn add_derive(ctx: AssistCtx<impl HirDatabase>) -> Option<Assist> {
}
Some(tt) => tt.syntax().range().end() - TextUnit::of_char(')'),
};
edit.target(nominal.syntax().range());
edit.set_cursor(offset)
})
}
@ -38,7 +39,7 @@ fn derive_insertion_offset(nominal: &ast::NominalDef) -> Option<TextUnit> {
#[cfg(test)]
mod tests {
use super::*;
use crate::helpers::check_assist;
use crate::helpers::{check_assist, check_assist_target};
#[test]
fn add_derive_new() {
@ -80,4 +81,21 @@ struct Foo { a: i32, }
",
);
}
#[test]
fn add_derive_target() {
check_assist_target(
add_derive,
"
struct SomeThingIrrelevant;
/// `Foo` is a pretty important struct.
/// It does stuff.
struct Foo { a: i32<|>, }
struct EvenMoreIrrelevant;
",
"/// `Foo` is a pretty important struct.
/// It does stuff.
struct Foo { a: i32, }",
);
}
}

17
crates/ra_assists/src/add_impl.rs
View File

@ -11,6 +11,7 @@ pub(crate) fn add_impl(ctx: AssistCtx<impl HirDatabase>) -> Option<Assist> {
let nominal = ctx.node_at_offset::<ast::NominalDef>()?;
let name = nominal.name()?;
ctx.build("add impl", |edit| {
edit.target(nominal.syntax().range());
let type_params = nominal.type_param_list();
let start_offset = nominal.syntax().range().end();
let mut buf = String::new();
@ -37,7 +38,7 @@ pub(crate) fn add_impl(ctx: AssistCtx<impl HirDatabase>) -> Option<Assist> {
#[cfg(test)]
mod tests {
use super::*;
use crate::helpers::check_assist;
use crate::helpers::{check_assist, check_assist_target};
#[test]
fn test_add_impl() {
@ -54,4 +55,18 @@ mod tests {
);
}
#[test]
fn add_impl_target() {
check_assist_target(
add_impl,
"
struct SomeThingIrrelevant;
/// Has a lifetime parameter
struct Foo<'a, T: Foo<'a>> {<|>}
struct EvenMoreIrrelevant;
",
"/// Has a lifetime parameter
struct Foo<'a, T: Foo<'a>> {}",
);
}
}

13
crates/ra_assists/src/assist_ctx.rs
View File

@ -16,7 +16,7 @@ pub(crate) enum Assist {
/// `AssistCtx` allows to apply an assist or check if it could be applied.
///
/// Assists use a somewhat overengineered approach, given the current needs. The
/// Assists use a somewhat over-engineered approach, given the current needs. The
/// assists workflow consists of two phases. In the first phase, a user asks for
/// the list of available assists. In the second phase, the user picks a
/// particular assist and it gets applied.
@ -106,6 +106,7 @@ impl<'a, DB: HirDatabase> AssistCtx<'a, DB> {
pub(crate) struct AssistBuilder {
edit: TextEditBuilder,
cursor_position: Option<TextUnit>,
target: Option<TextRange>,
}
impl AssistBuilder {
@ -138,7 +139,15 @@ impl AssistBuilder {
self.cursor_position = Some(offset)
}
pub(crate) fn target(&mut self, target: TextRange) {
self.target = Some(target)
}
fn build(self) -> AssistAction {
AssistAction { edit: self.edit.finish(), cursor_position: self.cursor_position }
AssistAction {
edit: self.edit.finish(),
cursor_position: self.cursor_position,
target: self.target,
}
}
}

22
crates/ra_assists/src/change_visibility.rs
View File

@ -20,7 +20,7 @@ fn add_vis(ctx: AssistCtx<impl HirDatabase>) -> Option<Assist> {
_ => false,
});
let offset = if let Some(keyword) = item_keyword {
let (offset, target) = if let Some(keyword) = item_keyword {
let parent = keyword.parent()?;
let def_kws = vec![FN_DEF, MODULE, STRUCT_DEF, ENUM_DEF, TRAIT_DEF];
// Parent is not a definition, can't add visibility
@ -31,17 +31,18 @@ fn add_vis(ctx: AssistCtx<impl HirDatabase>) -> Option<Assist> {
if parent.children().any(|child| child.kind() == VISIBILITY) {
return None;
}
vis_offset(parent)
(vis_offset(parent), keyword.range())
} else {
let ident = ctx.leaf_at_offset().find(|leaf| leaf.kind() == IDENT)?;
let field = ident.ancestors().find_map(ast::NamedFieldDef::cast)?;
if field.name()?.syntax().range() != ident.range() && field.visibility().is_some() {
return None;
}
vis_offset(field.syntax())
(vis_offset(field.syntax()), ident.range())
};
ctx.build("make pub(crate)", |edit| {
edit.target(target);
edit.insert(offset, "pub(crate) ");
edit.set_cursor(offset);
})
@ -60,13 +61,15 @@ fn vis_offset(node: &SyntaxNode) -> TextUnit {
fn change_vis(ctx: AssistCtx<impl HirDatabase>, vis: &ast::Visibility) -> Option<Assist> {
if vis.syntax().text() == "pub" {
return ctx.build("chage to pub(crate)", |edit| {
return ctx.build("change to pub(crate)", |edit| {
edit.target(vis.syntax().range());
edit.replace(vis.syntax().range(), "pub(crate)");
edit.set_cursor(vis.syntax().range().start());
});
}
if vis.syntax().text() == "pub(crate)" {
return ctx.build("chage to pub", |edit| {
return ctx.build("change to pub", |edit| {
edit.target(vis.syntax().range());
edit.replace(vis.syntax().range(), "pub");
edit.set_cursor(vis.syntax().range().start());
});
@ -77,7 +80,7 @@ fn change_vis(ctx: AssistCtx<impl HirDatabase>, vis: &ast::Visibility) -> Option
#[cfg(test)]
mod tests {
use super::*;
use crate::helpers::check_assist;
use crate::helpers::{check_assist, check_assist_target};
#[test]
fn change_visibility_adds_pub_crate_to_items() {
@ -135,4 +138,11 @@ mod tests {
",
)
}
#[test]
fn change_visibility_target() {
check_assist_target(change_visibility, "<|>fn foo() {}", "fn");
check_assist_target(change_visibility, "pub(crate)<|> fn foo() {}", "pub(crate)");
check_assist_target(change_visibility, "struct S { <|>field: u32 }", "field");
}
}

18
crates/ra_assists/src/fill_match_arms.rs
View File

@ -65,6 +65,7 @@ pub(crate) fn fill_match_arms(ctx: AssistCtx<impl HirDatabase>) -> Option<Assist
buf.push_str(" => (),\n");
}
buf.push_str("}");
edit.target(match_expr.syntax().range());
edit.set_cursor(expr.syntax().range().start());
edit.replace_node_and_indent(match_expr.syntax(), buf);
})
@ -72,7 +73,7 @@ pub(crate) fn fill_match_arms(ctx: AssistCtx<impl HirDatabase>) -> Option<Assist
#[cfg(test)]
mod tests {
use crate::helpers::check_assist;
use crate::helpers::{check_assist, check_assist_target};
use super::fill_match_arms;
@ -139,4 +140,19 @@ mod tests {
"#,
);
}
#[test]
fn fill_match_arms_target() {
check_assist_target(
fill_match_arms,
r#"
enum E { X, Y}
fn main() {
match E::X<|> {}
}
"#,
"match E::X {}",
);
}
}

8
crates/ra_assists/src/flip_comma.rs
View File

@ -11,6 +11,7 @@ pub(crate) fn flip_comma(ctx: AssistCtx<impl HirDatabase>) -> Option<Assist> {
let prev = non_trivia_sibling(comma, Direction::Prev)?;
let next = non_trivia_sibling(comma, Direction::Next)?;
ctx.build("flip comma", |edit| {
edit.target(comma.range());
edit.replace(prev.range(), next.text());
edit.replace(next.range(), prev.text());
})
@ -20,7 +21,7 @@ pub(crate) fn flip_comma(ctx: AssistCtx<impl HirDatabase>) -> Option<Assist> {
mod tests {
use super::*;
use crate::helpers::check_assist;
use crate::helpers::{check_assist, check_assist_target};
#[test]
fn flip_comma_works_for_function_parameters() {
@ -30,4 +31,9 @@ mod tests {
"fn foo(y: Result<(), ()>,<|> x: i32) {}",
)
}
#[test]
fn flip_comma_target() {
check_assist_target(flip_comma, "fn foo(x: i32,<|> y: Result<(), ()>) {}", ",")
}
}

35
crates/ra_assists/src/introduce_variable.rs
View File

@ -45,6 +45,7 @@ pub(crate) fn introduce_variable(ctx: AssistCtx<impl HirDatabase>) -> Option<Ass
} else {
buf.push_str(";");
indent.text().push_to(&mut buf);
edit.target(expr.syntax().range());
edit.replace(expr.syntax().range(), "var_name".to_string());
edit.insert(anchor_stmt.range().start(), buf);
if wrap_in_block {
@ -58,7 +59,7 @@ pub(crate) fn introduce_variable(ctx: AssistCtx<impl HirDatabase>) -> Option<Ass
fn valid_covering_node(node: &SyntaxNode) -> bool {
node.kind() != COMMENT
}
/// Check wether the node is a valid expression which can be extracted to a variable.
/// Check whether the node is a valid expression which can be extracted to a variable.
/// In general that's true for any expression, but in some cases that would produce invalid code.
fn valid_target_expr(node: &SyntaxNode) -> Option<&ast::Expr> {
match node.kind() {
@ -74,7 +75,7 @@ fn valid_target_expr(node: &SyntaxNode) -> Option<&ast::Expr> {
/// and a boolean indicating whether we have to wrap it within a { } block
/// to produce correct code.
/// It can be a statement, the last in a block expression or a wanna be block
/// expression like a lamba or match arm.
/// expression like a lambda or match arm.
fn anchor_stmt(expr: &ast::Expr) -> Option<(&SyntaxNode, bool)> {
expr.syntax().ancestors().find_map(|node| {
if ast::Stmt::cast(node).is_some() {
@ -100,7 +101,7 @@ fn anchor_stmt(expr: &ast::Expr) -> Option<(&SyntaxNode, bool)> {
#[cfg(test)]
mod tests {
use super::*;
use crate::helpers::{check_assist, check_assist_not_applicable, check_assist_range};
use crate::helpers::{check_assist, check_assist_not_applicable, check_assist_range, check_assist_target, check_assist_range_target};
#[test]
fn test_introduce_var_simple() {
@ -425,4 +426,32 @@ fn main() {
",
);
}
// FIXME: This is not quite correct, but good enough(tm) for the sorting heuristic
#[test]
fn introduce_var_target() {
check_assist_target(
introduce_variable,
"
fn foo() -> u32 {
r<|>eturn 2 + 2;
}
",
"2 + 2",
);
check_assist_range_target(
introduce_variable,
"
fn main() {
let x = true;
let tuple = match x {
true => (<|>2 + 2<|>, true)
_ => (0, false)
};
}
",
"2 + 2",
);
}
}

100
crates/ra_assists/src/lib.rs
View File

@ -8,7 +8,7 @@
mod assist_ctx;
use ra_text_edit::TextEdit;
use ra_syntax::{TextUnit, SyntaxNode, Direction};
use ra_syntax::{TextRange, TextUnit, SyntaxNode, Direction};
use ra_db::FileRange;
use hir::db::HirDatabase;
@ -23,6 +23,7 @@ pub struct AssistLabel {
pub struct AssistAction {
pub edit: TextEdit,
pub cursor_position: Option<TextUnit>,
pub target: Option<TextRange>,
}
/// Return all the assists applicable at the given position.
@ -53,15 +54,24 @@ pub fn assists<H>(db: &H, range: FileRange) -> Vec<(AssistLabel, AssistAction)>
where
H: HirDatabase + 'static,
{
use std::cmp::Ordering;
AssistCtx::with_ctx(db, range, true, |ctx| {
all_assists()
let mut a = all_assists()
.iter()
.filter_map(|f| f(ctx.clone()))
.map(|a| match a {
Assist::Resolved(label, action) => (label, action),
Assist::Unresolved(..) => unreachable!(),
})
.collect()
.collect::<Vec<(AssistLabel, AssistAction)>>();
a.sort_by(|a, b| match (a.1.target, b.1.target) {
(Some(a), Some(b)) => a.len().cmp(&b.len()),
(Some(_), None) => Ordering::Less,
(None, Some(_)) => Ordering::Greater,
(None, None) => Ordering::Equal,
});
a
})
}
@ -97,7 +107,7 @@ mod helpers {
use hir::mock::MockDatabase;
use ra_syntax::TextRange;
use ra_db::FileRange;
use test_utils::{extract_offset, assert_eq_text, add_cursor, extract_range};
use test_utils::{extract_offset, extract_range, assert_eq_text, add_cursor};
use crate::{AssistCtx, Assist};
@ -151,6 +161,45 @@ mod helpers {
assert_eq_text!(after, &actual);
}
pub(crate) fn check_assist_target(
assist: fn(AssistCtx<MockDatabase>) -> Option<Assist>,
before: &str,
target: &str,
) {
let (before_cursor_pos, before) = extract_offset(before);
let (db, _source_root, file_id) = MockDatabase::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");
let action = match assist {
Assist::Unresolved(_) => unreachable!(),
Assist::Resolved(_, it) => it,
};
let range = action.target.expect("expected target on action");
assert_eq_text!(&before[range.start().to_usize()..range.end().to_usize()], target);
}
pub(crate) fn check_assist_range_target(
assist: fn(AssistCtx<MockDatabase>) -> Option<Assist>,
before: &str,
target: &str,
) {
let (range, before) = extract_range(before);
let (db, _source_root, file_id) = MockDatabase::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");
let action = match assist {
Assist::Unresolved(_) => unreachable!(),
Assist::Resolved(_, it) => it,
};
let range = action.target.expect("expected target on action");
assert_eq_text!(&before[range.start().to_usize()..range.end().to_usize()], target);
}
pub(crate) fn check_assist_not_applicable(
assist: fn(AssistCtx<MockDatabase>) -> Option<Assist>,
before: &str,
@ -162,5 +211,48 @@ mod helpers {
let assist = AssistCtx::with_ctx(&db, frange, true, assist);
assert!(assist.is_none());
}
}
#[cfg(test)]
mod tests {
use hir::mock::MockDatabase;
use ra_syntax::TextRange;
use ra_db::FileRange;
use test_utils::{extract_offset};
#[test]
fn assist_order_field_struct() {
let before = "struct Foo { <|>bar: u32 }";
let (before_cursor_pos, before) = extract_offset(before);
let (db, _source_root, file_id) = MockDatabase::with_single_file(&before);
let frange =
FileRange { file_id, range: TextRange::offset_len(before_cursor_pos, 0.into()) };
let assists = super::assists(&db, frange);
let mut assists = assists.iter();
assert_eq!(assists.next().expect("expected assist").0.label, "make pub(crate)");
assert_eq!(assists.next().expect("expected assist").0.label, "add `#[derive]`");
}
#[test]
fn assist_order_if_expr() {
let before = "
pub fn test_some_range(a: int) -> bool {
if let 2..6 = 5<|> {
true
} else {
false
}
}";
let (before_cursor_pos, before) = extract_offset(before);
let (db, _source_root, file_id) = MockDatabase::with_single_file(&before);
let frange =
FileRange { file_id, range: TextRange::offset_len(before_cursor_pos, 0.into()) };
let assists = super::assists(&db, frange);
let mut assists = assists.iter();
assert_eq!(assists.next().expect("expected assist").0.label, "introduce variable");
assert_eq!(assists.next().expect("expected assist").0.label, "replace with match");
}
}

18
crates/ra_assists/src/remove_dbg.rs
View File

@ -47,6 +47,7 @@ pub(crate) fn remove_dbg(ctx: AssistCtx<impl HirDatabase>) -> Option<Assist> {
};
ctx.build("remove dbg!()", |edit| {
edit.target(macro_call.syntax().range());
edit.replace(macro_range, macro_content);
edit.set_cursor(cursor_pos);
})
@ -78,7 +79,7 @@ fn is_valid_macrocall(macro_call: &ast::MacroCall, macro_name: &str) -> Option<b
#[cfg(test)]
mod tests {
use super::*;
use crate::helpers::{check_assist, check_assist_not_applicable};
use crate::helpers::{check_assist, check_assist_not_applicable, check_assist_target};
#[test]
fn test_remove_dbg() {
@ -120,4 +121,19 @@ fn foo(n: usize) {
check_assist_not_applicable(remove_dbg, "<|>dbg(5, 6, 7)");
check_assist_not_applicable(remove_dbg, "<|>dbg!(5, 6, 7");
}
#[test]
fn remove_dbg_target() {
check_assist_target(
remove_dbg,
"
fn foo(n: usize) {
if let Some(_) = dbg!(n.<|>checked_sub(4)) {
// ...
}
}
",
"dbg!(n.checked_sub(4))",
);
}
}

25
crates/ra_assists/src/replace_if_let_with_match.rs
View File

@ -17,6 +17,7 @@ pub(crate) fn replace_if_let_with_match(ctx: AssistCtx<impl HirDatabase>) -> Opt
ctx.build("replace with match", |edit| {
let match_expr = build_match_expr(expr, pat, then_block, else_block);
edit.target(if_expr.syntax().range());
edit.replace_node_and_indent(if_expr.syntax(), match_expr);
edit.set_cursor(if_expr.syntax().range().start())
})
@ -46,7 +47,7 @@ fn format_arm(block: &ast::Block) -> String {
#[cfg(test)]
mod tests {
use super::*;
use crate::helpers::check_assist;
use crate::helpers::{check_assist, check_assist_target};
#[test]
fn test_replace_if_let_with_match_unwraps_simple_expressions() {
@ -73,4 +74,26 @@ impl VariantData {
} ",
)
}
#[test]
fn replace_if_let_with_match_target() {
check_assist_target(
replace_if_let_with_match,
"
impl VariantData {
pub fn is_struct(&self) -> bool {
if <|>let VariantData::Struct(..) = *self {
true
} else {
false
}
}
} ",
"if let VariantData::Struct(..) = *self {
true
} else {
false
}",
);
}
}

8
crates/ra_assists/src/split_import.rs
View File

@ -24,6 +24,7 @@ pub(crate) fn split_import(ctx: AssistCtx<impl HirDatabase>) -> Option<Assist> {
};
ctx.build("split import", |edit| {
edit.target(colon_colon.range());
edit.insert(l_curly, "{");
edit.insert(r_curly, "}");
edit.set_cursor(l_curly + TextUnit::of_str("{"));
@ -33,7 +34,7 @@ pub(crate) fn split_import(ctx: AssistCtx<impl HirDatabase>) -> Option<Assist> {
#[cfg(test)]
mod tests {
use super::*;
use crate::helpers::check_assist;
use crate::helpers::{check_assist, check_assist_target};
#[test]
fn test_split_import() {
@ -52,4 +53,9 @@ mod tests {
"use algo::{<|>visitor::{Visitor, visit}}",
)
}
#[test]
fn split_import_target() {
check_assist_target(split_import, "use algo::<|>visitor::{Visitor, visit}", "::");
}
}