diff options
author | bors[bot] <bors[bot]@users.noreply.github.com> | 2019-02-21 16:55:26 +0000 |
---|---|---|
committer | bors[bot] <bors[bot]@users.noreply.github.com> | 2019-02-21 16:55:26 +0000 |
commit | 5100aeac429919d1758908efb2f9cbe0d02c7510 (patch) | |
tree | 5921ac7c9d20fa729a3d97d8e6a81f87431f57e3 | |
parent | 55da8e786d2cac3cb6332f80c1ee715fbcbdee0d (diff) | |
parent | 82173c8de4b1283b6b54bd0def25b9c432614841 (diff) |
Merge #876
876: Fix join_lines not adding a comma after join_single_expr_block with match arm r=matklad a=vipentti
Fixes #868
Co-authored-by: Ville Penttinen <[email protected]>
-rw-r--r-- | crates/ra_assists/src/flip_comma.rs | 3 | ||||
-rw-r--r-- | crates/ra_assists/src/lib.rs | 6 | ||||
-rw-r--r-- | crates/ra_ide_api_light/src/join_lines.rs | 137 | ||||
-rw-r--r-- | crates/ra_syntax/src/algo.rs | 7 |
4 files changed, 143 insertions, 10 deletions
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; | |||
2 | use ra_syntax::{ | 2 | use ra_syntax::{ |
3 | Direction, | 3 | Direction, |
4 | SyntaxKind::COMMA, | 4 | SyntaxKind::COMMA, |
5 | algo::non_trivia_sibling, | ||
5 | }; | 6 | }; |
6 | 7 | ||
7 | use crate::{AssistCtx, Assist, non_trivia_sibling}; | 8 | use crate::{AssistCtx, Assist}; |
8 | 9 | ||
9 | pub(crate) fn flip_comma(mut ctx: AssistCtx<impl HirDatabase>) -> Option<Assist> { | 10 | pub(crate) fn flip_comma(mut ctx: AssistCtx<impl HirDatabase>) -> Option<Assist> { |
10 | let comma = ctx.leaf_at_offset().find(|leaf| leaf.kind() == COMMA)?; | 11 | 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; | |||
10 | use itertools::Itertools; | 10 | use itertools::Itertools; |
11 | 11 | ||
12 | use ra_text_edit::TextEdit; | 12 | use ra_text_edit::TextEdit; |
13 | use ra_syntax::{TextRange, TextUnit, SyntaxNode, Direction}; | 13 | use ra_syntax::{TextRange, TextUnit}; |
14 | use ra_db::FileRange; | 14 | use ra_db::FileRange; |
15 | use hir::db::HirDatabase; | 15 | use hir::db::HirDatabase; |
16 | 16 | ||
@@ -104,10 +104,6 @@ fn all_assists<DB: HirDatabase>() -> &'static [fn(AssistCtx<DB>) -> Option<Assis | |||
104 | ] | 104 | ] |
105 | } | 105 | } |
106 | 106 | ||
107 | fn non_trivia_sibling(node: &SyntaxNode, direction: Direction) -> Option<&SyntaxNode> { | ||
108 | node.siblings(direction).skip(1).find(|node| !node.kind().is_trivia()) | ||
109 | } | ||
110 | |||
111 | #[cfg(test)] | 107 | #[cfg(test)] |
112 | mod helpers { | 108 | mod helpers { |
113 | use hir::mock::MockDatabase; | 109 | 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 949ee1544..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; | |||
2 | use ra_syntax::{ | 2 | use ra_syntax::{ |
3 | SourceFile, TextRange, TextUnit, AstNode, SyntaxNode, | 3 | SourceFile, TextRange, TextUnit, AstNode, SyntaxNode, |
4 | SyntaxKind::{self, WHITESPACE, COMMA, R_CURLY, R_PAREN, R_BRACK}, | 4 | SyntaxKind::{self, WHITESPACE, COMMA, R_CURLY, R_PAREN, R_BRACK}, |
5 | algo::find_covering_node, | 5 | algo::{find_covering_node, non_trivia_sibling}, |
6 | ast, | 6 | ast, |
7 | Direction, | ||
7 | }; | 8 | }; |
8 | use ra_fmt::{ | 9 | use ra_fmt::{ |
9 | compute_ws, extract_trivial_expression | 10 | compute_ws, extract_trivial_expression |
@@ -120,11 +121,30 @@ fn remove_newline( | |||
120 | } | 121 | } |
121 | } | 122 | } |
122 | 123 | ||
124 | fn has_comma_after(node: &SyntaxNode) -> bool { | ||
125 | match non_trivia_sibling(node, Direction::Next) { | ||
126 | Some(n) => n.kind() == COMMA, | ||
127 | _ => false, | ||
128 | } | ||
129 | } | ||
130 | |||
123 | fn join_single_expr_block(edit: &mut TextEditBuilder, node: &SyntaxNode) -> Option<()> { | 131 | fn join_single_expr_block(edit: &mut TextEditBuilder, node: &SyntaxNode) -> Option<()> { |
124 | let block = ast::Block::cast(node.parent()?)?; | 132 | let block = ast::Block::cast(node.parent()?)?; |
125 | let block_expr = ast::BlockExpr::cast(block.syntax().parent()?)?; | 133 | let block_expr = ast::BlockExpr::cast(block.syntax().parent()?)?; |
126 | let expr = extract_trivial_expression(block)?; | 134 | let expr = extract_trivial_expression(block)?; |
127 | edit.replace(block_expr.syntax().range(), expr.syntax().text().to_string()); | 135 | |
136 | let block_range = block_expr.syntax().range(); | ||
137 | let mut buf = expr.syntax().text().to_string(); | ||
138 | |||
139 | // Match block needs to have a comma after the block | ||
140 | if let Some(match_arm) = block_expr.syntax().parent().and_then(ast::MatchArm::cast) { | ||
141 | if !has_comma_after(match_arm.syntax()) { | ||
142 | buf.push(','); | ||
143 | } | ||
144 | } | ||
145 | |||
146 | edit.replace(block_range, buf); | ||
147 | |||
128 | Some(()) | 148 | Some(()) |
129 | } | 149 | } |
130 | 150 | ||
@@ -208,7 +228,6 @@ fn foo() { | |||
208 | } | 228 | } |
209 | 229 | ||
210 | #[test] | 230 | #[test] |
211 | #[ignore] // FIXME: https://github.com/rust-analyzer/rust-analyzer/issues/868 | ||
212 | fn join_lines_adds_comma_for_block_in_match_arm() { | 231 | fn join_lines_adds_comma_for_block_in_match_arm() { |
213 | check_join_lines( | 232 | check_join_lines( |
214 | r" | 233 | r" |
@@ -231,6 +250,118 @@ fn foo(e: Result<U, V>) { | |||
231 | } | 250 | } |
232 | 251 | ||
233 | #[test] | 252 | #[test] |
253 | fn join_lines_keeps_comma_for_block_in_match_arm() { | ||
254 | // We already have a comma | ||
255 | check_join_lines( | ||
256 | r" | ||
257 | fn foo(e: Result<U, V>) { | ||
258 | match e { | ||
259 | Ok(u) => <|>{ | ||
260 | u.foo() | ||
261 | }, | ||
262 | Err(v) => v, | ||
263 | } | ||
264 | }", | ||
265 | r" | ||
266 | fn foo(e: Result<U, V>) { | ||
267 | match e { | ||
268 | Ok(u) => <|>u.foo(), | ||
269 | Err(v) => v, | ||
270 | } | ||
271 | }", | ||
272 | ); | ||
273 | |||
274 | // comma with whitespace between brace and , | ||
275 | check_join_lines( | ||
276 | r" | ||
277 | fn foo(e: Result<U, V>) { | ||
278 | match e { | ||
279 | Ok(u) => <|>{ | ||
280 | u.foo() | ||
281 | } , | ||
282 | Err(v) => v, | ||
283 | } | ||
284 | }", | ||
285 | r" | ||
286 | fn foo(e: Result<U, V>) { | ||
287 | match e { | ||
288 | Ok(u) => <|>u.foo() , | ||
289 | Err(v) => v, | ||
290 | } | ||
291 | }", | ||
292 | ); | ||
293 | |||
294 | // comma with newline between brace and , | ||
295 | check_join_lines( | ||
296 | r" | ||
297 | fn foo(e: Result<U, V>) { | ||
298 | match e { | ||
299 | Ok(u) => <|>{ | ||
300 | u.foo() | ||
301 | } | ||
302 | , | ||
303 | Err(v) => v, | ||
304 | } | ||
305 | }", | ||
306 | r" | ||
307 | fn foo(e: Result<U, V>) { | ||
308 | match e { | ||
309 | Ok(u) => <|>u.foo() | ||
310 | , | ||
311 | Err(v) => v, | ||
312 | } | ||
313 | }", | ||
314 | ); | ||
315 | } | ||
316 | |||
317 | #[test] | ||
318 | fn join_lines_keeps_comma_with_single_arg_tuple() { | ||
319 | // A single arg tuple | ||
320 | check_join_lines( | ||
321 | r" | ||
322 | fn foo() { | ||
323 | let x = (<|>{ | ||
324 | 4 | ||
325 | },); | ||
326 | }", | ||
327 | r" | ||
328 | fn foo() { | ||
329 | let x = (<|>4,); | ||
330 | }", | ||
331 | ); | ||
332 | |||
333 | // single arg tuple with whitespace between brace and comma | ||
334 | check_join_lines( | ||
335 | r" | ||
336 | fn foo() { | ||
337 | let x = (<|>{ | ||
338 | 4 | ||
339 | } ,); | ||
340 | }", | ||
341 | r" | ||
342 | fn foo() { | ||
343 | let x = (<|>4 ,); | ||
344 | }", | ||
345 | ); | ||
346 | |||
347 | // single arg tuple with newline between brace and comma | ||
348 | check_join_lines( | ||
349 | r" | ||
350 | fn foo() { | ||
351 | let x = (<|>{ | ||
352 | 4 | ||
353 | } | ||
354 | ,); | ||
355 | }", | ||
356 | r" | ||
357 | fn foo() { | ||
358 | let x = (<|>4 | ||
359 | ,); | ||
360 | }", | ||
361 | ); | ||
362 | } | ||
363 | |||
364 | #[test] | ||
234 | fn test_join_lines_use_items_left() { | 365 | fn test_join_lines_use_items_left() { |
235 | // No space after the '{' | 366 | // No space after the '{' |
236 | check_join_lines( | 367 | check_join_lines( |
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; | |||
2 | 2 | ||
3 | use rowan::TransparentNewType; | 3 | use rowan::TransparentNewType; |
4 | 4 | ||
5 | use crate::{SyntaxNode, TextRange, TextUnit, AstNode}; | 5 | use crate::{SyntaxNode, TextRange, TextUnit, AstNode, Direction}; |
6 | 6 | ||
7 | pub use rowan::LeafAtOffset; | 7 | pub use rowan::LeafAtOffset; |
8 | 8 | ||
@@ -29,6 +29,11 @@ pub fn find_node_at_offset<N: AstNode>(syntax: &SyntaxNode, offset: TextUnit) -> | |||
29 | find_leaf_at_offset(syntax, offset).find_map(|leaf| leaf.ancestors().find_map(N::cast)) | 29 | find_leaf_at_offset(syntax, offset).find_map(|leaf| leaf.ancestors().find_map(N::cast)) |
30 | } | 30 | } |
31 | 31 | ||
32 | /// Finds the first sibling in the given direction which is not `trivia` | ||
33 | pub fn non_trivia_sibling(node: &SyntaxNode, direction: Direction) -> Option<&SyntaxNode> { | ||
34 | node.siblings(direction).skip(1).find(|node| !node.kind().is_trivia()) | ||
35 | } | ||
36 | |||
32 | pub fn find_covering_node(root: &SyntaxNode, range: TextRange) -> &SyntaxNode { | 37 | pub fn find_covering_node(root: &SyntaxNode, range: TextRange) -> &SyntaxNode { |
33 | SyntaxNode::from_repr(root.0.covering_node(range)) | 38 | SyntaxNode::from_repr(root.0.covering_node(range)) |
34 | } | 39 | } |