From cd6acaa0e1618d0876fc2cc0f926de1b756622b9 Mon Sep 17 00:00:00 2001 From: Jordan Mackie Date: Tue, 19 Apr 2022 10:27:20 +0100 Subject: [PATCH] Format match expressions --- .../golden-tests/match_expressions.ditto | 49 +++++++++++ crates/ditto-fmt/src/expression.rs | 83 +++++++++++++++++-- crates/ditto-fmt/src/has_comments.rs | 52 ++++++++++++ crates/ditto-fmt/src/token.rs | 2 + 4 files changed, 181 insertions(+), 5 deletions(-) create mode 100644 crates/ditto-fmt/golden-tests/match_expressions.ditto diff --git a/crates/ditto-fmt/golden-tests/match_expressions.ditto b/crates/ditto-fmt/golden-tests/match_expressions.ditto new file mode 100644 index 000000000..e3a00f13f --- /dev/null +++ b/crates/ditto-fmt/golden-tests/match_expressions.ditto @@ -0,0 +1,49 @@ +module If.Then.Else exports (..); + + +octopus = + match arm with + | Arm1 -> "octopus" + | Arm2 -> -- comment + "octopus" + | Arm3 -> + -- comment + "octopus" + | Arm4 -> "octopus" -- comment + | -- pls don't do this + Arm5 -> "octopus" + | Arm6 -- or this + -> "octopus" + -- + -- + -- + | Arm.Arm7 -> if true then "octopus" else "octopus" + | Arm.Arm8 -> + if true then + -- still octopus + "octopus" + else + "octopus"; + +-- it's a classic +map_maybe = (fn, maybe) -> + -- it really is + match maybe with + | Just(a) -> fn(a) + | Nothing -> Nothing; + +all_the_args = () -> + match not_sure with + | Foo(a, b, c, d) -> unit + | Bar(Foo(a, b), b, Baz.Barrr, d) -> unit + | Foo( + -- comment + a, + B.B, + -- comment + C.C(a, b, c), + D.D( + -- comment + d, + ), + ) -> unit; diff --git a/crates/ditto-fmt/src/expression.rs b/crates/ditto-fmt/src/expression.rs index f642715fa..4ecc5541a 100644 --- a/crates/ditto-fmt/src/expression.rs +++ b/crates/ditto-fmt/src/expression.rs @@ -3,13 +3,14 @@ use super::{ helpers::{group, space}, name::{gen_name, gen_qualified_name, gen_qualified_proper_name}, r#type::gen_type, - syntax::{gen_brackets_list, gen_parens, gen_parens_list}, + syntax::{gen_brackets_list, gen_parens, gen_parens_list, gen_parens_list1}, token::{ - gen_colon, gen_else_keyword, gen_false_keyword, gen_if_keyword, gen_right_arrow, - gen_string_token, gen_then_keyword, gen_true_keyword, gen_unit_keyword, + gen_colon, gen_else_keyword, gen_false_keyword, gen_if_keyword, gen_match_keyword, + gen_pipe, gen_right_arrow, gen_string_token, gen_then_keyword, gen_true_keyword, + gen_unit_keyword, gen_with_keyword, }, }; -use ditto_cst::{Expression, StringToken, TypeAnnotation}; +use ditto_cst::{Expression, MatchArm, Pattern, StringToken, TypeAnnotation}; use dprint_core::formatting::{ condition_helpers, conditions, ir_helpers, ConditionResolver, ConditionResolverContext, Info, PrintItems, Signal, @@ -156,6 +157,66 @@ pub fn gen_expression(expr: Expression) -> PrintItems { })); items } + Expression::Match { + match_keyword, + box expression, + with_keyword, + head_arm, + tail_arms, + } => { + let mut items = PrintItems::new(); + // REVIEW: do we want to support an inline format for single-arm matches? + // + // e.g. `match x with | foo -> bar` + // + // If so, we should probably make that leading `|` optional in the parser + // like we do for type declarations. + items.extend(gen_match_keyword(match_keyword)); + items.extend(space()); + items.extend(gen_expression(expression)); + items.extend(space()); + items.extend(gen_with_keyword(with_keyword)); + items.extend(gen_match_arm(head_arm)); + for match_arm in tail_arms { + items.extend(gen_match_arm(match_arm)); + } + items + } + } +} + +fn gen_match_arm(match_arm: MatchArm) -> PrintItems { + let mut items = PrintItems::new(); + items.push_signal(Signal::ExpectNewLine); + items.extend(gen_pipe(match_arm.pipe)); + items.extend(space()); + items.extend(gen_pattern(match_arm.pattern)); + items.extend(space()); + let right_arrow_has_trailing_comment = match_arm.right_arrow.0.has_trailing_comment(); + items.extend(gen_right_arrow(match_arm.right_arrow)); + items.extend(gen_body_expression( + *match_arm.expression, + right_arrow_has_trailing_comment, + )); + items +} + +fn gen_pattern(pattern: Pattern) -> PrintItems { + match pattern { + Pattern::Variable { name } => gen_name(name), + Pattern::NullaryConstructor { constructor } => gen_qualified_proper_name(constructor), + Pattern::Constructor { + constructor, + arguments, + } => { + let mut items = gen_qualified_proper_name(constructor); + items.extend(gen_parens_list1( + arguments, + |box pattern| gen_pattern(pattern), + false, + )); + items + } } } @@ -168,7 +229,8 @@ pub fn gen_body_expression(expr: Expression, force_use_new_lines: bool) -> Print let end_info = Info::new("end"); let has_leading_comments = expr.has_leading_comments(); - let deserves_new_line_if_multi_lines = matches!(expr, Expression::If { .. }); + let deserves_new_line_if_multi_lines = + matches!(expr, Expression::If { .. } | Expression::Match { .. }); let expression_should_be_on_new_line: ConditionResolver = Rc::new(move |ctx: &mut ConditionResolverContext| -> Option { @@ -350,4 +412,15 @@ mod tests { 20 ); } + + #[test] + fn it_formats_matches() { + assert_fmt!("match foo with\n| var -> 5"); + assert_fmt!("-- comment\nmatch foo with\n| var -> 5"); + assert_fmt!("match foo with\n-- comment\n| var -> 5"); + assert_fmt!("match foo with\n| a -> 5\n| b -> 5\n| c -> 5"); + assert_fmt!("match foo with\n| Foo.Bar -> -- comment\n\t5"); + assert_fmt!("match Foo with\n| Foo(a, b, c) -> a"); + assert_fmt!("match Foo with\n| Foo(\n\t--comment\n\ta,\n\tb,\n\tc,\n) -> a"); + } } diff --git a/crates/ditto-fmt/src/has_comments.rs b/crates/ditto-fmt/src/has_comments.rs index a9096c669..6323b3b9b 100644 --- a/crates/ditto-fmt/src/has_comments.rs +++ b/crates/ditto-fmt/src/has_comments.rs @@ -57,6 +57,19 @@ impl HasComments for Expression { function, arguments, } => function.has_comments() || arguments.has_comments(), + Self::Match { + match_keyword, + expression, + with_keyword, + head_arm, + tail_arms, + } => { + match_keyword.0.has_comments() + || expression.has_comments() + || with_keyword.0.has_comments() + || head_arm.has_comments() + || tail_arms.iter().any(|arm| arm.has_comments()) + } } } @@ -75,6 +88,45 @@ impl HasComments for Expression { Self::If { if_keyword, .. } => if_keyword.0.has_leading_comments(), Self::Function { box parameters, .. } => parameters.open_paren.0.has_leading_comments(), Self::Call { function, .. } => function.has_leading_comments(), + Self::Match { match_keyword, .. } => match_keyword.0.has_leading_comments(), + } + } +} + +impl HasComments for MatchArm { + fn has_comments(&self) -> bool { + let Self { + pipe, + pattern, + right_arrow, + expression, + } = self; + pipe.0.has_comments() + || pattern.has_comments() + || right_arrow.0.has_comments() + || expression.has_comments() + } + fn has_leading_comments(&self) -> bool { + self.pipe.0.has_leading_comments() + } +} + +impl HasComments for Pattern { + fn has_comments(&self) -> bool { + match self { + Self::NullaryConstructor { constructor } => constructor.has_comments(), + Self::Constructor { + constructor, + arguments, + } => constructor.has_comments() || arguments.has_comments(), + Self::Variable { name } => name.has_comments(), + } + } + fn has_leading_comments(&self) -> bool { + match self { + Self::NullaryConstructor { constructor } => constructor.has_leading_comments(), + Self::Constructor { constructor, .. } => constructor.has_leading_comments(), + Self::Variable { name } => name.has_leading_comments(), } } } diff --git a/crates/ditto-fmt/src/token.rs b/crates/ditto-fmt/src/token.rs index 4603666de..36ce7573c 100644 --- a/crates/ditto-fmt/src/token.rs +++ b/crates/ditto-fmt/src/token.rs @@ -32,6 +32,8 @@ gen_empty_token_like!(gen_unit_keyword, cst::UnitKeyword, "unit"); gen_empty_token_like!(gen_if_keyword, cst::IfKeyword, "if"); gen_empty_token_like!(gen_then_keyword, cst::ThenKeyword, "then"); gen_empty_token_like!(gen_else_keyword, cst::ElseKeyword, "else"); +gen_empty_token_like!(gen_match_keyword, cst::MatchKeyword, "match"); +gen_empty_token_like!(gen_with_keyword, cst::WithKeyword, "with"); gen_empty_token_like!(gen_exports_keyword, cst::ExportsKeyword, "exports"); gen_empty_token_like!(gen_as_keyword, cst::AsKeyword, "as"); gen_empty_token_like!(gen_type_keyword, cst::TypeKeyword, "type");