From 40add5de9a717cb784dde3310ace8d61edd2dde4 Mon Sep 17 00:00:00 2001 From: Ville Penttinen Date: Thu, 21 Feb 2019 17:51:41 +0200 Subject: Fix join_lines not adding a comma after join_single_expr_block in match arm We will also remove optional whitespace between the expression and the comma. e.g. ```rust fn foo() { let x = (<|>{ 4 } ,); // NOTE: whitespace } ``` becomes ```rust fn foo() { let x = (<|>4,); } ``` --- crates/ra_ide_api_light/src/join_lines.rs | 149 +++++++++++++++++++++++++++++- 1 file changed, 147 insertions(+), 2 deletions(-) (limited to 'crates') diff --git a/crates/ra_ide_api_light/src/join_lines.rs b/crates/ra_ide_api_light/src/join_lines.rs index 949ee1544..9f59fbd43 100644 --- a/crates/ra_ide_api_light/src/join_lines.rs +++ b/crates/ra_ide_api_light/src/join_lines.rs @@ -120,11 +120,47 @@ fn remove_newline( } } +/// fixes a comma after the given expression and optionally inserts a new trailing comma +/// if no comma was found and `comma_offset` is provided +fn fix_comma_after(edit: &mut TextEditBuilder, node: &SyntaxNode, comma_offset: Option) { + let next = node.next_sibling(); + let nnext = node.next_sibling().and_then(|n| n.next_sibling()); + + match (next, nnext) { + // Whitespace followed by a comma + // remove the whitespace + (Some(ws), Some(comma)) if ws.kind() == WHITESPACE && comma.kind() == COMMA => { + edit.delete(ws.range()); + } + + // if we are not a comma and if comma_offset was provided, + // insert trailing comma after the block + (Some(n), _) if n.kind() != COMMA => { + if let Some(comma_offset) = comma_offset { + edit.insert(comma_offset, ",".to_owned()); + } + } + + _ => {} + } +} + fn join_single_expr_block(edit: &mut TextEditBuilder, node: &SyntaxNode) -> Option<()> { let block = ast::Block::cast(node.parent()?)?; let block_expr = ast::BlockExpr::cast(block.syntax().parent()?)?; let expr = extract_trivial_expression(block)?; - edit.replace(block_expr.syntax().range(), expr.syntax().text().to_string()); + + let block_range = block_expr.syntax().range(); + edit.replace(block_range, expr.syntax().text().to_string()); + + // Match block needs to have a comma after the block + // otherwise we'll maintain a comma after the block if such existed + // but we remove excess whitespace between the expression and the comma. + if let Some(match_arm) = block_expr.syntax().parent().and_then(ast::MatchArm::cast) { + fix_comma_after(edit, match_arm.syntax(), Some(block_range.end())); + } else { + fix_comma_after(edit, block_expr.syntax(), None); + } Some(()) } @@ -208,7 +244,6 @@ fn foo() { } #[test] - #[ignore] // FIXME: https://github.com/rust-analyzer/rust-analyzer/issues/868 fn join_lines_adds_comma_for_block_in_match_arm() { check_join_lines( r" @@ -230,6 +265,116 @@ fn foo(e: Result) { ); } + #[test] + fn join_lines_keeps_comma_for_block_in_match_arm() { + // We already have a comma + check_join_lines( + r" +fn foo(e: Result) { + match e { + Ok(u) => <|>{ + u.foo() + }, + Err(v) => v, + } +}", + r" +fn foo(e: Result) { + match e { + Ok(u) => <|>u.foo(), + Err(v) => v, + } +}", + ); + + // comma with whitespace between brace and , + check_join_lines( + r" +fn foo(e: Result) { + match e { + Ok(u) => <|>{ + u.foo() + } , + Err(v) => v, + } +}", + r" +fn foo(e: Result) { + match e { + Ok(u) => <|>u.foo(), + Err(v) => v, + } +}", + ); + + // comma with newline between brace and , + check_join_lines( + r" +fn foo(e: Result) { + match e { + Ok(u) => <|>{ + u.foo() + } + , + Err(v) => v, + } +}", + r" +fn foo(e: Result) { + match e { + Ok(u) => <|>u.foo(), + Err(v) => v, + } +}", + ); + } + + #[test] + fn join_lines_keeps_comma_with_single_arg_tuple() { + // A single arg tuple + check_join_lines( + r" +fn foo() { + let x = (<|>{ + 4 + },); +}", + r" +fn foo() { + let x = (<|>4,); +}", + ); + + // single arg tuple with whitespace between brace and comma + check_join_lines( + r" +fn foo() { + let x = (<|>{ + 4 + } ,); +}", + r" +fn foo() { + let x = (<|>4,); +}", + ); + + // single arg tuple with newline between brace and comma + check_join_lines( + r" +fn foo() { + let x = (<|>{ + 4 + } + ,); +}", + r" +fn foo() { + let x = (<|>4,); +}", + ); + } + #[test] fn test_join_lines_use_items_left() { // No space after the '{' -- cgit v1.2.3 From 3c22c64725a4a2ea13a30422a4f822fb1259e7b5 Mon Sep 17 00:00:00 2001 From: Ville Penttinen Date: Thu, 21 Feb 2019 18:26:27 +0200 Subject: Simplify adding a comma after match arm --- crates/ra_ide_api_light/src/join_lines.rs | 46 ++++++++++++------------------- 1 file changed, 18 insertions(+), 28 deletions(-) (limited to 'crates') diff --git a/crates/ra_ide_api_light/src/join_lines.rs b/crates/ra_ide_api_light/src/join_lines.rs index 9f59fbd43..9a400199f 100644 --- a/crates/ra_ide_api_light/src/join_lines.rs +++ b/crates/ra_ide_api_light/src/join_lines.rs @@ -120,28 +120,15 @@ fn remove_newline( } } -/// fixes a comma after the given expression and optionally inserts a new trailing comma -/// if no comma was found and `comma_offset` is provided -fn fix_comma_after(edit: &mut TextEditBuilder, node: &SyntaxNode, comma_offset: Option) { +fn has_comma_after(node: &SyntaxNode) -> bool { let next = node.next_sibling(); let nnext = node.next_sibling().and_then(|n| n.next_sibling()); match (next, nnext) { - // Whitespace followed by a comma - // remove the whitespace - (Some(ws), Some(comma)) if ws.kind() == WHITESPACE && comma.kind() == COMMA => { - edit.delete(ws.range()); - } - - // if we are not a comma and if comma_offset was provided, - // insert trailing comma after the block - (Some(n), _) if n.kind() != COMMA => { - if let Some(comma_offset) = comma_offset { - edit.insert(comma_offset, ",".to_owned()); - } - } - - _ => {} + // Whitespace followed by a comma is fine + (Some(ws), Some(comma)) if ws.kind() == WHITESPACE && comma.kind() == COMMA => true, + (Some(n), _) => n.kind() == COMMA, + _ => false, } } @@ -151,16 +138,17 @@ fn join_single_expr_block(edit: &mut TextEditBuilder, node: &SyntaxNode) -> Opti let expr = extract_trivial_expression(block)?; let block_range = block_expr.syntax().range(); - edit.replace(block_range, expr.syntax().text().to_string()); + let mut buf = expr.syntax().text().to_string(); // Match block needs to have a comma after the block - // otherwise we'll maintain a comma after the block if such existed - // but we remove excess whitespace between the expression and the comma. if let Some(match_arm) = block_expr.syntax().parent().and_then(ast::MatchArm::cast) { - fix_comma_after(edit, match_arm.syntax(), Some(block_range.end())); - } else { - fix_comma_after(edit, block_expr.syntax(), None); + if !has_comma_after(match_arm.syntax()) { + buf.push(','); + } } + + edit.replace(block_range, buf); + Some(()) } @@ -301,7 +289,7 @@ fn foo(e: Result) { r" fn foo(e: Result) { match e { - Ok(u) => <|>u.foo(), + Ok(u) => <|>u.foo() , Err(v) => v, } }", @@ -322,7 +310,8 @@ fn foo(e: Result) { r" fn foo(e: Result) { match e { - Ok(u) => <|>u.foo(), + Ok(u) => <|>u.foo() + , Err(v) => v, } }", @@ -355,7 +344,7 @@ fn foo() { }", r" fn foo() { - let x = (<|>4,); + let x = (<|>4 ,); }", ); @@ -370,7 +359,8 @@ fn foo() { }", r" fn foo() { - let x = (<|>4,); + let x = (<|>4 + ,); }", ); } -- cgit v1.2.3 From 82173c8de4b1283b6b54bd0def25b9c432614841 Mon Sep 17 00:00:00 2001 From: Ville Penttinen Date: Thu, 21 Feb 2019 18:49:03 +0200 Subject: Move `non_trivia_sibling` to `ra_syntax::algo` --- crates/ra_assists/src/flip_comma.rs | 3 ++- crates/ra_assists/src/lib.rs | 6 +----- crates/ra_ide_api_light/src/join_lines.rs | 12 ++++-------- crates/ra_syntax/src/algo.rs | 7 ++++++- 4 files changed, 13 insertions(+), 15 deletions(-) (limited to 'crates') diff --git a/crates/ra_assists/src/flip_comma.rs b/crates/ra_assists/src/flip_comma.rs index 08644d720..0d4a789fc 100644 --- a/crates/ra_assists/src/flip_comma.rs +++ b/crates/ra_assists/src/flip_comma.rs @@ -2,9 +2,10 @@ use hir::db::HirDatabase; use ra_syntax::{ Direction, SyntaxKind::COMMA, + algo::non_trivia_sibling, }; -use crate::{AssistCtx, Assist, non_trivia_sibling}; +use crate::{AssistCtx, Assist}; pub(crate) fn flip_comma(mut ctx: AssistCtx) -> Option { let comma = ctx.leaf_at_offset().find(|leaf| leaf.kind() == COMMA)?; diff --git a/crates/ra_assists/src/lib.rs b/crates/ra_assists/src/lib.rs index 7bd9b5ae6..e1e899edc 100644 --- a/crates/ra_assists/src/lib.rs +++ b/crates/ra_assists/src/lib.rs @@ -10,7 +10,7 @@ mod assist_ctx; use itertools::Itertools; use ra_text_edit::TextEdit; -use ra_syntax::{TextRange, TextUnit, SyntaxNode, Direction}; +use ra_syntax::{TextRange, TextUnit}; use ra_db::FileRange; use hir::db::HirDatabase; @@ -104,10 +104,6 @@ fn all_assists() -> &'static [fn(AssistCtx) -> Option Option<&SyntaxNode> { - node.siblings(direction).skip(1).find(|node| !node.kind().is_trivia()) -} - #[cfg(test)] mod helpers { use hir::mock::MockDatabase; diff --git a/crates/ra_ide_api_light/src/join_lines.rs b/crates/ra_ide_api_light/src/join_lines.rs index 9a400199f..b5bcd62fb 100644 --- a/crates/ra_ide_api_light/src/join_lines.rs +++ b/crates/ra_ide_api_light/src/join_lines.rs @@ -2,8 +2,9 @@ use itertools::Itertools; use ra_syntax::{ SourceFile, TextRange, TextUnit, AstNode, SyntaxNode, SyntaxKind::{self, WHITESPACE, COMMA, R_CURLY, R_PAREN, R_BRACK}, - algo::find_covering_node, + algo::{find_covering_node, non_trivia_sibling}, ast, + Direction, }; use ra_fmt::{ compute_ws, extract_trivial_expression @@ -121,13 +122,8 @@ fn remove_newline( } fn has_comma_after(node: &SyntaxNode) -> bool { - let next = node.next_sibling(); - let nnext = node.next_sibling().and_then(|n| n.next_sibling()); - - match (next, nnext) { - // Whitespace followed by a comma is fine - (Some(ws), Some(comma)) if ws.kind() == WHITESPACE && comma.kind() == COMMA => true, - (Some(n), _) => n.kind() == COMMA, + match non_trivia_sibling(node, Direction::Next) { + Some(n) => n.kind() == COMMA, _ => false, } } diff --git a/crates/ra_syntax/src/algo.rs b/crates/ra_syntax/src/algo.rs index e8cf0d4b5..e2b4f0388 100644 --- a/crates/ra_syntax/src/algo.rs +++ b/crates/ra_syntax/src/algo.rs @@ -2,7 +2,7 @@ pub mod visit; use rowan::TransparentNewType; -use crate::{SyntaxNode, TextRange, TextUnit, AstNode}; +use crate::{SyntaxNode, TextRange, TextUnit, AstNode, Direction}; pub use rowan::LeafAtOffset; @@ -29,6 +29,11 @@ pub fn find_node_at_offset(syntax: &SyntaxNode, offset: TextUnit) -> find_leaf_at_offset(syntax, offset).find_map(|leaf| leaf.ancestors().find_map(N::cast)) } +/// Finds the first sibling in the given direction which is not `trivia` +pub fn non_trivia_sibling(node: &SyntaxNode, direction: Direction) -> Option<&SyntaxNode> { + node.siblings(direction).skip(1).find(|node| !node.kind().is_trivia()) +} + pub fn find_covering_node(root: &SyntaxNode, range: TextRange) -> &SyntaxNode { SyntaxNode::from_repr(root.0.covering_node(range)) } -- cgit v1.2.3