From 52fd19621caa62ddd7d3f29b69a5133650bc1294 Mon Sep 17 00:00:00 2001 From: Aleksey Kladov Date: Tue, 25 Feb 2020 18:57:47 +0100 Subject: Remove code duplication in tests --- .../ra_assists/src/handlers/introduce_variable.rs | 60 +++++------ crates/ra_assists/src/lib.rs | 114 +++++++++------------ crates/test_utils/src/lib.rs | 1 + 3 files changed, 74 insertions(+), 101 deletions(-) diff --git a/crates/ra_assists/src/handlers/introduce_variable.rs b/crates/ra_assists/src/handlers/introduce_variable.rs index 7312ce687..b453c51fb 100644 --- a/crates/ra_assists/src/handlers/introduce_variable.rs +++ b/crates/ra_assists/src/handlers/introduce_variable.rs @@ -136,15 +136,13 @@ fn anchor_stmt(expr: ast::Expr) -> Option<(SyntaxNode, bool)> { mod tests { use test_utils::covers; - use crate::helpers::{ - check_assist_range, check_assist_range_not_applicable, check_assist_range_target, - }; + use crate::helpers::{check_assist, check_assist_not_applicable, check_assist_target}; use super::*; #[test] fn test_introduce_var_simple() { - check_assist_range( + check_assist( introduce_variable, " fn foo() { @@ -161,16 +159,13 @@ fn foo() { #[test] fn introduce_var_in_comment_is_not_applicable() { covers!(introduce_var_in_comment_is_not_applicable); - check_assist_range_not_applicable( - introduce_variable, - "fn main() { 1 + /* <|>comment<|> */ 1; }", - ); + check_assist_not_applicable(introduce_variable, "fn main() { 1 + /* <|>comment<|> */ 1; }"); } #[test] fn test_introduce_var_expr_stmt() { covers!(test_introduce_var_expr_stmt); - check_assist_range( + check_assist( introduce_variable, " fn foo() { @@ -181,7 +176,7 @@ fn foo() { let <|>var_name = 1 + 1; }", ); - check_assist_range( + check_assist( introduce_variable, " fn foo() { @@ -198,7 +193,7 @@ fn foo() { #[test] fn test_introduce_var_part_of_expr_stmt() { - check_assist_range( + check_assist( introduce_variable, " fn foo() { @@ -215,7 +210,7 @@ fn foo() { #[test] fn test_introduce_var_last_expr() { covers!(test_introduce_var_last_expr); - check_assist_range( + check_assist( introduce_variable, " fn foo() { @@ -227,7 +222,7 @@ fn foo() { bar(var_name) }", ); - check_assist_range( + check_assist( introduce_variable, " fn foo() { @@ -243,7 +238,7 @@ fn foo() { #[test] fn test_introduce_var_in_match_arm_no_block() { - check_assist_range( + check_assist( introduce_variable, " fn main() { @@ -268,7 +263,7 @@ fn main() { #[test] fn test_introduce_var_in_match_arm_with_block() { - check_assist_range( + check_assist( introduce_variable, " fn main() { @@ -300,7 +295,7 @@ fn main() { #[test] fn test_introduce_var_in_closure_no_block() { - check_assist_range( + check_assist( introduce_variable, " fn main() { @@ -317,7 +312,7 @@ fn main() { #[test] fn test_introduce_var_in_closure_with_block() { - check_assist_range( + check_assist( introduce_variable, " fn main() { @@ -334,7 +329,7 @@ fn main() { #[test] fn test_introduce_var_path_simple() { - check_assist_range( + check_assist( introduce_variable, " fn main() { @@ -352,7 +347,7 @@ fn main() { #[test] fn test_introduce_var_path_method() { - check_assist_range( + check_assist( introduce_variable, " fn main() { @@ -370,7 +365,7 @@ fn main() { #[test] fn test_introduce_var_return() { - check_assist_range( + check_assist( introduce_variable, " fn foo() -> u32 { @@ -388,7 +383,7 @@ fn foo() -> u32 { #[test] fn test_introduce_var_does_not_add_extra_whitespace() { - check_assist_range( + check_assist( introduce_variable, " fn foo() -> u32 { @@ -407,7 +402,7 @@ fn foo() -> u32 { ", ); - check_assist_range( + check_assist( introduce_variable, " fn foo() -> u32 { @@ -424,7 +419,7 @@ fn foo() -> u32 { ", ); - check_assist_range( + check_assist( introduce_variable, " fn foo() -> u32 { @@ -452,7 +447,7 @@ fn foo() -> u32 { #[test] fn test_introduce_var_break() { - check_assist_range( + check_assist( introduce_variable, " fn main() { @@ -474,7 +469,7 @@ fn main() { #[test] fn test_introduce_var_for_cast() { - check_assist_range( + check_assist( introduce_variable, " fn main() { @@ -492,27 +487,20 @@ fn main() { #[test] fn test_introduce_var_for_return_not_applicable() { - check_assist_range_not_applicable(introduce_variable, "fn foo() { <|>return<|>; } "); + check_assist_not_applicable(introduce_variable, "fn foo() { <|>return<|>; } "); } #[test] fn test_introduce_var_for_break_not_applicable() { - check_assist_range_not_applicable( - introduce_variable, - "fn main() { loop { <|>break<|>; }; }", - ); + check_assist_not_applicable(introduce_variable, "fn main() { loop { <|>break<|>; }; }"); } // FIXME: This is not quite correct, but good enough(tm) for the sorting heuristic #[test] fn introduce_var_target() { - check_assist_range_target( - introduce_variable, - "fn foo() -> u32 { <|>return 2 + 2<|>; }", - "2 + 2", - ); + check_assist_target(introduce_variable, "fn foo() -> u32 { <|>return 2 + 2<|>; }", "2 + 2"); - check_assist_range_target( + check_assist_target( introduce_variable, " fn main() { diff --git a/crates/ra_assists/src/lib.rs b/crates/ra_assists/src/lib.rs index d7998b0d1..79fe43aa4 100644 --- a/crates/ra_assists/src/lib.rs +++ b/crates/ra_assists/src/lib.rs @@ -162,7 +162,7 @@ mod helpers { use ra_db::{fixture::WithFixture, FileId, FileRange, SourceDatabaseExt}; use ra_ide_db::{symbol_index::SymbolsDatabase, RootDatabase}; use ra_syntax::TextRange; - use test_utils::{add_cursor, assert_eq_text, extract_offset, extract_range}; + use test_utils::{add_cursor, assert_eq_text, extract_range_or_offset, RangeOrOffset}; use crate::{AssistCtx, AssistHandler}; @@ -176,81 +176,65 @@ mod helpers { } 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 = - assist(AssistCtx::new(&db, frange, true)).expect("code action is not applicable"); - let action = assist.0[0].action.clone().unwrap(); - - let actual = action.edit.apply(&before); - let actual_cursor_pos = match action.cursor_position { - None => action - .edit - .apply_to_offset(before_cursor_pos) - .expect("cursor position is affected by the edit"), - Some(off) => off, - }; - let actual = add_cursor(&actual, actual_cursor_pos); - assert_eq_text!(after, &actual); - } - - 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 = - assist(AssistCtx::new(&db, frange, true)).expect("code action is not applicable"); - let action = assist.0[0].action.clone().unwrap(); - - let mut actual = action.edit.apply(&before); - if let Some(pos) = action.cursor_position { - actual = add_cursor(&actual, pos); - } - assert_eq_text!(after, &actual); + check(assist, before, ExpectedResult::After(after)); } + // FIXME: instead of having a separate function here, maybe use + // `extract_ranges` and mark the target as ` ` in the + // fixuture? 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 = - assist(AssistCtx::new(&db, frange, true)).expect("code action is not applicable"); - let action = assist.0[0].action.clone().unwrap(); - - let range = action.target.expect("expected target on action"); - assert_eq_text!(&before[range.start().to_usize()..range.end().to_usize()], target); + check(assist, before, ExpectedResult::Target(target)); } - 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 = - assist(AssistCtx::new(&db, frange, true)).expect("code action is not applicable"); - let action = assist.0[0].action.clone().unwrap(); - - 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: AssistHandler, before: &str) { + check(assist, before, ExpectedResult::NotApplicable); } - 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 = assist(AssistCtx::new(&db, frange, true)); - assert!(assist.is_none()); + enum ExpectedResult<'a> { + NotApplicable, + After(&'a str), + Target(&'a str), } - pub(crate) fn check_assist_range_not_applicable(assist: AssistHandler, before: &str) { - let (range, before) = extract_range(before); + fn check(assist: AssistHandler, before: &str, expected: ExpectedResult) { + let (range_or_offset, before) = extract_range_or_offset(before); + let range: TextRange = range_or_offset.into(); + let (db, file_id) = with_single_file(&before); let frange = FileRange { file_id, range }; - let assist = assist(AssistCtx::new(&db, frange, true)); - assert!(assist.is_none()); + let assist_ctx = AssistCtx::new(&db, frange, true); + + match (assist(assist_ctx), expected) { + (Some(assist), ExpectedResult::After(after)) => { + let action = assist.0[0].action.clone().unwrap(); + + let mut actual = action.edit.apply(&before); + match action.cursor_position { + None => { + if let RangeOrOffset::Offset(before_cursor_pos) = range_or_offset { + let off = action + .edit + .apply_to_offset(before_cursor_pos) + .expect("cursor position is affected by the edit"); + actual = add_cursor(&actual, off) + } + } + Some(off) => actual = add_cursor(&actual, off), + }; + + assert_eq_text!(after, &actual); + } + (Some(assist), ExpectedResult::Target(target)) => { + let action = assist.0[0].action.clone().unwrap(); + let range = action.target.expect("expected target on action"); + assert_eq_text!(&before[range.start().to_usize()..range.end().to_usize()], target); + } + (Some(_), ExpectedResult::NotApplicable) => panic!("assist should not be applicable!"), + (None, ExpectedResult::After(_)) | (None, ExpectedResult::Target(_)) => { + panic!("code action is not applicable") + } + (None, ExpectedResult::NotApplicable) => (), + }; } } diff --git a/crates/test_utils/src/lib.rs b/crates/test_utils/src/lib.rs index e6e8d7110..69deddcb5 100644 --- a/crates/test_utils/src/lib.rs +++ b/crates/test_utils/src/lib.rs @@ -83,6 +83,7 @@ fn try_extract_range(text: &str) -> Option<(TextRange, String)> { Some((TextRange::from_to(start, end), text)) } +#[derive(Clone, Copy)] pub enum RangeOrOffset { Range(TextRange), Offset(TextUnit), -- cgit v1.2.3