From 63751d1c6b8b2dbc2a7eb8df1f8c508693227cd6 Mon Sep 17 00:00:00 2001 From: petr-tik Date: Fri, 24 Jul 2020 18:57:40 +0100 Subject: Added failing tests --- crates/ra_assists/src/handlers/remove_dbg.rs | 21 +++++++++++++++++++++ 1 file changed, 21 insertions(+) (limited to 'crates/ra_assists/src/handlers/remove_dbg.rs') diff --git a/crates/ra_assists/src/handlers/remove_dbg.rs b/crates/ra_assists/src/handlers/remove_dbg.rs index a616cca57..e19620d5a 100644 --- a/crates/ra_assists/src/handlers/remove_dbg.rs +++ b/crates/ra_assists/src/handlers/remove_dbg.rs @@ -126,4 +126,25 @@ fn foo(n: usize) { "dbg!(n.checked_sub(4))", ); } + + #[test] + fn remove_dbg_leave_semicolon() { + // https://github.com/rust-analyzer/rust-analyzer/issues/5129#issuecomment-651399779 + // not quite though + let code = " +let res = <|>dbg!(1 * 20); // needless comment +"; + let expected = " +let res = 1 * 20; // needless comment +"; + check_assist(remove_dbg, code, expected); + } + + #[test] + fn remove_dbg_keep_expression() { + let code = " +let res = <|>dbg!(a + b).foo();"; + let expected = "let res = (a + b).foo();"; + check_assist(remove_dbg, code, expected); + } } -- cgit v1.2.3 From 01bdeaad71ea87e2e989fc9ded06f69805929e42 Mon Sep 17 00:00:00 2001 From: petr-tik Date: Mon, 27 Jul 2020 21:28:41 +0100 Subject: Make all test fn names consistent in remove_dbg --- crates/ra_assists/src/handlers/remove_dbg.rs | 32 +++++++++++++++++++++++++--- 1 file changed, 29 insertions(+), 3 deletions(-) (limited to 'crates/ra_assists/src/handlers/remove_dbg.rs') diff --git a/crates/ra_assists/src/handlers/remove_dbg.rs b/crates/ra_assists/src/handlers/remove_dbg.rs index e19620d5a..1116d2a2c 100644 --- a/crates/ra_assists/src/handlers/remove_dbg.rs +++ b/crates/ra_assists/src/handlers/remove_dbg.rs @@ -99,6 +99,7 @@ fn foo(n: usize) { ", ); } + #[test] fn test_remove_dbg_with_brackets_and_braces() { check_assist(remove_dbg, "dbg![<|>1 + 1]", "1 + 1"); @@ -113,7 +114,7 @@ fn foo(n: usize) { } #[test] - fn remove_dbg_target() { + fn test_remove_dbg_target() { check_assist_target( remove_dbg, " @@ -128,7 +129,7 @@ fn foo(n: usize) { } #[test] - fn remove_dbg_leave_semicolon() { + fn test_remove_dbg_keep_semicolon() { // https://github.com/rust-analyzer/rust-analyzer/issues/5129#issuecomment-651399779 // not quite though let code = " @@ -141,10 +142,35 @@ let res = 1 * 20; // needless comment } #[test] - fn remove_dbg_keep_expression() { + fn test_remove_dbg_keep_expression() { let code = " let res = <|>dbg!(a + b).foo();"; let expected = "let res = (a + b).foo();"; check_assist(remove_dbg, code, expected); } + + #[test] + fn test_remove_dbg_from_inside_fn() { + let code = " +fn square(x: u32) -> u32 { + x * x +} + +fn main() { + let x = square(dbg<|>!(5 + 10)); + println!(\"{}\", x); +}"; + + let expected = " +fn square(x: u32) -> u32 { + x * x +} + +fn main() { + let x = square(5 + 10); + println!(\"{}\", x); +}"; + check_assist_target(remove_dbg, code, "dbg!(5 + 10)"); + check_assist(remove_dbg, code, expected); + } } -- cgit v1.2.3 From 6ea28c377947b7e26efcb107d0dbbd804ed5e27f Mon Sep 17 00:00:00 2001 From: petr-tik Date: Mon, 27 Jul 2020 22:17:15 +0100 Subject: Fixed #5129 Addresses two issues: - keep the parens from dbg!() in case the call is chained or there is semantic difference if parens are excluded - Exclude the semicolon after the dbg!(); by checking if it was accidentally included in the macro_call investigated, but decided against: fix ast::MacroCall extraction to never include semicolons at the end - this logic lives in rowan. Defensively shorten the macro_range if there is a semicolon token. Deleted unneccessary temp variable macro_args Renamed macro_content to "paste_instead_of_dbg", because it isn't a simple extraction of text inside dbg!() anymore --- crates/ra_assists/src/handlers/remove_dbg.rs | 31 +++++++++++++++++++++------- 1 file changed, 23 insertions(+), 8 deletions(-) (limited to 'crates/ra_assists/src/handlers/remove_dbg.rs') diff --git a/crates/ra_assists/src/handlers/remove_dbg.rs b/crates/ra_assists/src/handlers/remove_dbg.rs index 1116d2a2c..c43b89661 100644 --- a/crates/ra_assists/src/handlers/remove_dbg.rs +++ b/crates/ra_assists/src/handlers/remove_dbg.rs @@ -1,6 +1,6 @@ use ra_syntax::{ ast::{self, AstNode}, - TextSize, T, + TextRange, TextSize, T, }; use crate::{AssistContext, AssistId, AssistKind, Assists}; @@ -27,19 +27,32 @@ pub(crate) fn remove_dbg(acc: &mut Assists, ctx: &AssistContext) -> Option<()> { return None; } - let macro_range = macro_call.syntax().text_range(); + let semicolon_on_end = macro_call.semicolon_token().is_some(); + let is_leaf = macro_call.syntax().next_sibling().is_none(); - let macro_content = { - let macro_args = macro_call.token_tree()?.syntax().clone(); + let macro_end = match semicolon_on_end { + true => macro_call.syntax().text_range().end() - TextSize::of(';'), + false => macro_call.syntax().text_range().end(), + }; - let text = macro_args.text(); - let without_parens = TextSize::of('(')..text.len() - TextSize::of(')'); - text.slice(without_parens).to_string() + // macro_range determines what will be deleted and replaced with macro_content + let macro_range = TextRange::new(macro_call.syntax().text_range().start(), macro_end); + let paste_instead_of_dbg = { + let text = macro_call.token_tree()?.syntax().text(); + + // leafines determines if we should include the parenthesis or not + let slice_index: TextRange = match is_leaf { + // leaf means - we can extract the contents of the dbg! in text + true => TextRange::new(TextSize::of('('), text.len() - TextSize::of(')')), + // not leaf - means we should keep the parens + false => TextRange::new(TextSize::from(0 as u32), text.len()), + }; + text.slice(slice_index).to_string() }; let target = macro_call.syntax().text_range(); acc.add(AssistId("remove_dbg", AssistKind::Refactor), "Remove dbg!()", target, |builder| { - builder.replace(macro_range, macro_content); + builder.replace(macro_range, paste_instead_of_dbg); }) } @@ -132,6 +145,8 @@ fn foo(n: usize) { fn test_remove_dbg_keep_semicolon() { // https://github.com/rust-analyzer/rust-analyzer/issues/5129#issuecomment-651399779 // not quite though + // adding a comment at the end of the line makes + // the ast::MacroCall to include the semicolon at the end let code = " let res = <|>dbg!(1 * 20); // needless comment "; -- cgit v1.2.3 From 6ea393a8d181dfeebbab9cc1f8a08af7a675be14 Mon Sep 17 00:00:00 2001 From: petr-tik Date: Wed, 29 Jul 2020 22:12:53 +0100 Subject: Addressed code review replaced match with let-if variable assignment removed the unnecessary semicolon_on_end variable converted all code and expected test variables to raw strings and inlined them in asserts --- crates/ra_assists/src/handlers/remove_dbg.rs | 68 +++++++++++++++++----------- 1 file changed, 41 insertions(+), 27 deletions(-) (limited to 'crates/ra_assists/src/handlers/remove_dbg.rs') diff --git a/crates/ra_assists/src/handlers/remove_dbg.rs b/crates/ra_assists/src/handlers/remove_dbg.rs index c43b89661..9430ce1b5 100644 --- a/crates/ra_assists/src/handlers/remove_dbg.rs +++ b/crates/ra_assists/src/handlers/remove_dbg.rs @@ -27,12 +27,12 @@ pub(crate) fn remove_dbg(acc: &mut Assists, ctx: &AssistContext) -> Option<()> { return None; } - let semicolon_on_end = macro_call.semicolon_token().is_some(); let is_leaf = macro_call.syntax().next_sibling().is_none(); - let macro_end = match semicolon_on_end { - true => macro_call.syntax().text_range().end() - TextSize::of(';'), - false => macro_call.syntax().text_range().end(), + let macro_end = if macro_call.semicolon_token().is_some() { + macro_call.syntax().text_range().end() - TextSize::of(';') + } else { + macro_call.syntax().text_range().end() }; // macro_range determines what will be deleted and replaced with macro_content @@ -40,12 +40,13 @@ pub(crate) fn remove_dbg(acc: &mut Assists, ctx: &AssistContext) -> Option<()> { let paste_instead_of_dbg = { let text = macro_call.token_tree()?.syntax().text(); - // leafines determines if we should include the parenthesis or not - let slice_index: TextRange = match is_leaf { + // leafiness determines if we should include the parenthesis or not + let slice_index: TextRange = if is_leaf { // leaf means - we can extract the contents of the dbg! in text - true => TextRange::new(TextSize::of('('), text.len() - TextSize::of(')')), + TextRange::new(TextSize::of('('), text.len() - TextSize::of(')')) + } else { // not leaf - means we should keep the parens - false => TextRange::new(TextSize::from(0 as u32), text.len()), + TextRange::up_to(text.len()) }; text.slice(slice_index).to_string() }; @@ -147,45 +148,58 @@ fn foo(n: usize) { // not quite though // adding a comment at the end of the line makes // the ast::MacroCall to include the semicolon at the end - let code = " -let res = <|>dbg!(1 * 20); // needless comment -"; - let expected = " -let res = 1 * 20; // needless comment -"; - check_assist(remove_dbg, code, expected); + check_assist( + remove_dbg, + r#"let res = <|>dbg!(1 * 20); // needless comment"#, + r#"let res = 1 * 20; // needless comment"#, + ); } #[test] fn test_remove_dbg_keep_expression() { - let code = " -let res = <|>dbg!(a + b).foo();"; - let expected = "let res = (a + b).foo();"; - check_assist(remove_dbg, code, expected); + check_assist( + remove_dbg, + r#"let res = <|>dbg!(a + b).foo();"#, + r#"let res = (a + b).foo();"#, + ); } #[test] fn test_remove_dbg_from_inside_fn() { - let code = " + check_assist_target( + remove_dbg, + r#" fn square(x: u32) -> u32 { x * x } fn main() { let x = square(dbg<|>!(5 + 10)); - println!(\"{}\", x); -}"; + println!("{}", x); +}"#, + "dbg!(5 + 10)", + ); - let expected = " + check_assist( + remove_dbg, + r#" +fn square(x: u32) -> u32 { + x * x +} + +fn main() { + let x = square(dbg<|>!(5 + 10)); + println!("{}", x); +}"#, + r#" fn square(x: u32) -> u32 { x * x } fn main() { let x = square(5 + 10); - println!(\"{}\", x); -}"; - check_assist_target(remove_dbg, code, "dbg!(5 + 10)"); - check_assist(remove_dbg, code, expected); + println!("{}", x); +}"#, + ); } } -- cgit v1.2.3