From 45dd90b0e8d54fe0467f9e0f886c7c05d2400eb0 Mon Sep 17 00:00:00 2001 From: Aleksey Kladov Date: Wed, 5 Feb 2020 11:46:05 +0100 Subject: Cleanup --- crates/ra_assists/src/assists/merge_match_arms.rs | 23 ++++++++++++----------- 1 file changed, 12 insertions(+), 11 deletions(-) (limited to 'crates/ra_assists/src/assists') diff --git a/crates/ra_assists/src/assists/merge_match_arms.rs b/crates/ra_assists/src/assists/merge_match_arms.rs index aca391155..885d5036d 100644 --- a/crates/ra_assists/src/assists/merge_match_arms.rs +++ b/crates/ra_assists/src/assists/merge_match_arms.rs @@ -1,6 +1,7 @@ -use crate::{Assist, AssistCtx, AssistId, TextRange, TextUnit}; use hir::db::HirDatabase; -use ra_syntax::ast::{AstNode, MatchArm}; +use ra_syntax::ast::{self, AstNode}; + +use crate::{Assist, AssistCtx, AssistId, TextRange, TextUnit}; // Assist: merge_match_arms // @@ -27,12 +28,12 @@ use ra_syntax::ast::{AstNode, MatchArm}; // } // ``` pub(crate) fn merge_match_arms(ctx: AssistCtx) -> Option { - let current_arm = ctx.find_node_at_offset::()?; + let current_arm = ctx.find_node_at_offset::()?; // We check if the following match arm matches this one. We could, but don't, // compare to the previous match arm as well. let next = current_arm.syntax().next_sibling(); - let next_arm = MatchArm::cast(next?)?; + let next_arm = ast::MatchArm::cast(next?)?; // Don't try to handle arms with guards for now - can add support for this later if current_arm.guard().is_some() || next_arm.guard().is_some() { @@ -53,13 +54,6 @@ pub(crate) fn merge_match_arms(ctx: AssistCtx) -> Option bool { - a.pats().any(|x| match x { - ra_syntax::ast::Pat::PlaceholderPat(..) => true, - _ => false, - }) - } - let pats = if contains_placeholder(¤t_arm) || contains_placeholder(&next_arm) { "_".into() } else { @@ -83,6 +77,13 @@ pub(crate) fn merge_match_arms(ctx: AssistCtx) -> Option bool { + a.pats().any(|x| match x { + ra_syntax::ast::Pat::PlaceholderPat(..) => true, + _ => false, + }) +} + #[cfg(test)] mod tests { use super::merge_match_arms; -- cgit v1.2.3 From 28acd01c638590a64a06148021ae60c0ccef8bb6 Mon Sep 17 00:00:00 2001 From: Aleksey Kladov Date: Wed, 5 Feb 2020 11:53:33 +0100 Subject: Merge match arms works with many arms --- crates/ra_assists/src/assists/merge_match_arms.rs | 99 ++++++++++++++++------- 1 file changed, 70 insertions(+), 29 deletions(-) (limited to 'crates/ra_assists/src/assists') diff --git a/crates/ra_assists/src/assists/merge_match_arms.rs b/crates/ra_assists/src/assists/merge_match_arms.rs index 885d5036d..8af30866c 100644 --- a/crates/ra_assists/src/assists/merge_match_arms.rs +++ b/crates/ra_assists/src/assists/merge_match_arms.rs @@ -1,7 +1,12 @@ +use std::iter::successors; + use hir::db::HirDatabase; -use ra_syntax::ast::{self, AstNode}; +use ra_syntax::{ + ast::{self, AstNode}, + Direction, TextUnit, +}; -use crate::{Assist, AssistCtx, AssistId, TextRange, TextUnit}; +use crate::{Assist, AssistCtx, AssistId, TextRange}; // Assist: merge_match_arms // @@ -29,51 +34,52 @@ use crate::{Assist, AssistCtx, AssistId, TextRange, TextUnit}; // ``` pub(crate) fn merge_match_arms(ctx: AssistCtx) -> Option { let current_arm = ctx.find_node_at_offset::()?; - - // We check if the following match arm matches this one. We could, but don't, - // compare to the previous match arm as well. - let next = current_arm.syntax().next_sibling(); - let next_arm = ast::MatchArm::cast(next?)?; - // Don't try to handle arms with guards for now - can add support for this later - if current_arm.guard().is_some() || next_arm.guard().is_some() { + if current_arm.guard().is_some() { return None; } - let current_expr = current_arm.expr()?; - let next_expr = next_arm.expr()?; + let current_text_range = current_arm.syntax().text_range(); + let cursor_offset_back = current_text_range.end() - ctx.frange.range.start(); - // Check for match arm equality by comparing lengths and then string contents - if current_expr.syntax().text_range().len() != next_expr.syntax().text_range().len() { - return None; - } - if current_expr.syntax().text() != next_expr.syntax().text() { + // We check if the following match arms match this one. We could, but don't, + // compare to the previous match arm as well. + let arms_to_merge = successors(Some(current_arm), next_arm) + .take_while(|arm| { + if arm.guard().is_some() { + return false; + } + match arm.expr() { + Some(expr) => expr.syntax().text() == current_expr.syntax().text(), + None => false, + } + }) + .collect::>(); + + if arms_to_merge.len() <= 1 { return None; } - let cursor_to_end = current_arm.syntax().text_range().end() - ctx.frange.range.start(); - ctx.add_assist(AssistId("merge_match_arms"), "Merge match arms", |edit| { - let pats = if contains_placeholder(¤t_arm) || contains_placeholder(&next_arm) { + let pats = if arms_to_merge.iter().any(contains_placeholder) { "_".into() } else { - let ps: Vec = current_arm - .pats() + arms_to_merge + .iter() + .flat_map(ast::MatchArm::pats) .map(|x| x.syntax().to_string()) - .chain(next_arm.pats().map(|x| x.syntax().to_string())) - .collect(); - ps.join(" | ") + .collect::>() + .join(" | ") }; let arm = format!("{} => {}", pats, current_expr.syntax().text()); - let offset = TextUnit::from_usize(arm.len()) - cursor_to_end; - let start = current_arm.syntax().text_range().start(); - let end = next_arm.syntax().text_range().end(); + let start = arms_to_merge.first().unwrap().syntax().text_range().start(); + let end = arms_to_merge.last().unwrap().syntax().text_range().end(); - edit.target(current_arm.syntax().text_range()); + edit.target(current_text_range); + edit.set_cursor(start + TextUnit::from_usize(arm.len()) - cursor_offset_back); edit.replace(TextRange::from_to(start, end), arm); - edit.set_cursor(start + offset); }) } @@ -84,6 +90,10 @@ fn contains_placeholder(a: &ast::MatchArm) -> bool { }) } +fn next_arm(arm: &ast::MatchArm) -> Option { + arm.syntax().siblings(Direction::Next).skip(1).find_map(ast::MatchArm::cast) +} + #[cfg(test)] mod tests { use super::merge_match_arms; @@ -185,6 +195,37 @@ mod tests { ); } + #[test] + fn merges_all_subsequent_arms() { + check_assist( + merge_match_arms, + r#" + enum X { A, B, C, D, E } + + fn main() { + match X::A { + X::A =><|> 92, + X::B => 92, + X::C => 92, + X::D => 62, + _ => panic!(), + } + } + "#, + r#" + enum X { A, B, C, D, E } + + fn main() { + match X::A { + X::A | X::B | X::C =><|> 92, + X::D => 62, + _ => panic!(), + } + } + "#, + ) + } + #[test] fn merge_match_arms_rejects_guards() { check_assist_not_applicable( -- cgit v1.2.3 From f756d5da063461c99c20266dc4896d8388fe28b9 Mon Sep 17 00:00:00 2001 From: Aleksey Kladov Date: Wed, 5 Feb 2020 13:41:43 +0100 Subject: Better cursor placement when merging arms --- crates/ra_assists/src/assists/merge_match_arms.rs | 21 +++++++++++++++++---- 1 file changed, 17 insertions(+), 4 deletions(-) (limited to 'crates/ra_assists/src/assists') diff --git a/crates/ra_assists/src/assists/merge_match_arms.rs b/crates/ra_assists/src/assists/merge_match_arms.rs index 8af30866c..64c9379da 100644 --- a/crates/ra_assists/src/assists/merge_match_arms.rs +++ b/crates/ra_assists/src/assists/merge_match_arms.rs @@ -40,7 +40,17 @@ pub(crate) fn merge_match_arms(ctx: AssistCtx) -> Option) -> Option start + TextUnit::from_usize(arm.len()) - back_offset, + CursorPos::InPat(offset) => offset, + }); edit.replace(TextRange::from_to(start, end), arm); }) } @@ -204,7 +217,7 @@ mod tests { fn main() { match X::A { - X::A =><|> 92, + X::A<|> => 92, X::B => 92, X::C => 92, X::D => 62, @@ -217,7 +230,7 @@ mod tests { fn main() { match X::A { - X::A | X::B | X::C =><|> 92, + X::A<|> | X::B | X::C => 92, X::D => 62, _ => panic!(), } -- cgit v1.2.3