diff --git a/crates/compiler/fmt/src/def.rs b/crates/compiler/fmt/src/def.rs index 98a905f469..d45e37b733 100644 --- a/crates/compiler/fmt/src/def.rs +++ b/crates/compiler/fmt/src/def.rs @@ -61,7 +61,7 @@ impl<'a> Formattable for TypeDef<'a> { &self, buf: &mut Buf<'buf>, _parens: Parens, - _newlines: Newlines, + newlines: Newlines, indent: u16, ) { use roc_parse::ast::TypeDef::*; @@ -97,22 +97,10 @@ impl<'a> Formattable for TypeDef<'a> { ann.format(buf, indent) } Opaque { - header: TypeHeader { name, vars }, + header, typ: ann, derived: has_abilities, } => { - buf.indent(indent); - buf.push_str(name.value); - - for var in *vars { - buf.spaces(1); - fmt_pattern(buf, &var.value, indent, Parens::NotNeeded); - buf.indent(indent); - } - - buf.push_str(" :="); - buf.spaces(1); - let ann_is_where_clause = matches!(ann.extract_spaces().item, TypeAnnotation::Where(..)); @@ -126,7 +114,7 @@ impl<'a> Formattable for TypeDef<'a> { let make_multiline = ann.is_multiline() || has_abilities_multiline; - ann.format(buf, indent); + fmt_general_def(header, buf, indent, ":=", &ann.value, newlines); if let Some(has_abilities) = has_abilities { buf.spaces(1); @@ -178,6 +166,29 @@ impl<'a> Formattable for TypeDef<'a> { } } +impl<'a> Formattable for TypeHeader<'a> { + fn is_multiline(&self) -> bool { + self.vars.iter().any(|v| v.is_multiline()) + } + + fn format_with_options<'buf>( + &self, + buf: &mut Buf<'buf>, + _parens: Parens, + _newlines: Newlines, + indent: u16, + ) { + buf.indent(indent); + buf.push_str(self.name.value); + + for var in self.vars.iter() { + buf.spaces(1); + fmt_pattern(buf, &var.value, indent, Parens::NotNeeded); + buf.indent(indent); + } + } +} + impl<'a> Formattable for ValueDef<'a> { fn is_multiline(&self) -> bool { use roc_parse::ast::ValueDef::*; @@ -204,7 +215,14 @@ impl<'a> Formattable for ValueDef<'a> { use roc_parse::ast::ValueDef::*; match self { Annotation(loc_pattern, loc_annotation) => { - fmt_annotation(loc_pattern, buf, indent, loc_annotation, newlines); + fmt_general_def( + loc_pattern, + buf, + indent, + ":", + &loc_annotation.value, + newlines, + ); } Body(loc_pattern, loc_expr) => { fmt_body(buf, &loc_pattern.value, &loc_expr.value, indent); @@ -221,7 +239,7 @@ impl<'a> Formattable for ValueDef<'a> { body_pattern, body_expr, } => { - fmt_annotation(ann_pattern, buf, indent, ann_type, newlines); + fmt_general_def(ann_pattern, buf, indent, ":", &ann_type.value, newlines); if let Some(comment_str) = comment { buf.push_str(" #"); @@ -236,54 +254,63 @@ impl<'a> Formattable for ValueDef<'a> { } } -fn fmt_annotation( - loc_pattern: &Loc, +fn fmt_general_def( + lhs: L, buf: &mut Buf, indent: u16, - loc_annotation: &Loc, + sep: &str, + rhs: &TypeAnnotation, newlines: Newlines, ) { - loc_pattern.format(buf, indent); + lhs.format(buf, indent); buf.indent(indent); - if loc_annotation.is_multiline() { - buf.push_str(" :"); + if rhs.is_multiline() { + buf.spaces(1); + buf.push_str(sep); buf.spaces(1); - let should_outdent = match loc_annotation.value { - TypeAnnotation::SpaceBefore(sub_def, spaces) => match sub_def { - TypeAnnotation::Record { .. } | TypeAnnotation::TagUnion { .. } => { - let is_only_newlines = spaces.iter().all(|s| s.is_newline()); - is_only_newlines && sub_def.is_multiline() - } - _ => false, - }, - TypeAnnotation::Record { .. } | TypeAnnotation::TagUnion { .. } => true, - _ => false, - }; + let should_outdent = should_outdent(rhs); if should_outdent { - match loc_annotation.value { + match rhs { TypeAnnotation::SpaceBefore(sub_def, _) => { sub_def.format_with_options(buf, Parens::NotNeeded, Newlines::No, indent); } _ => { - loc_annotation.format_with_options( - buf, - Parens::NotNeeded, - Newlines::No, - indent, - ); + rhs.format_with_options(buf, Parens::NotNeeded, Newlines::No, indent); } } } else { - loc_annotation.format_with_options(buf, Parens::NotNeeded, newlines, indent + INDENT); + rhs.format_with_options(buf, Parens::NotNeeded, newlines, indent + INDENT); } } else { buf.spaces(1); - buf.push(':'); + buf.push_str(sep); buf.spaces(1); - loc_annotation.format_with_options(buf, Parens::NotNeeded, Newlines::No, indent); + rhs.format_with_options(buf, Parens::NotNeeded, Newlines::No, indent); + } +} + +fn should_outdent(mut rhs: &TypeAnnotation) -> bool { + loop { + match rhs { + TypeAnnotation::SpaceBefore(sub_def, spaces) => { + let is_only_newlines = spaces.iter().all(|s| s.is_newline()); + if !is_only_newlines || !sub_def.is_multiline() { + return false; + } + rhs = sub_def; + } + TypeAnnotation::Where(ann, _clauses) => { + if !ann.is_multiline() { + return false; + } + rhs = &ann.value; + } + TypeAnnotation::Record { .. } | TypeAnnotation::TagUnion { .. } => return true, + _ => return false, + } } } diff --git a/crates/compiler/fmt/src/expr.rs b/crates/compiler/fmt/src/expr.rs index 69d14f1ed3..8e716e1772 100644 --- a/crates/compiler/fmt/src/expr.rs +++ b/crates/compiler/fmt/src/expr.rs @@ -1,4 +1,4 @@ -use crate::annotation::{except_last, Formattable, Newlines, Parens}; +use crate::annotation::{except_last, is_collection_multiline, Formattable, Newlines, Parens}; use crate::collection::{fmt_collection, Braces}; use crate::def::fmt_defs; use crate::pattern::fmt_pattern; @@ -49,7 +49,7 @@ impl<'a> Formattable for Expr<'a> { // These expressions always have newlines Defs(_, _) | When(_, _) => true, - List(items) => items.iter().any(|loc_expr| loc_expr.is_multiline()), + List(items) => is_collection_multiline(items), Str(literal) => is_str_multiline(literal), Apply(loc_expr, args, _) => { @@ -96,9 +96,9 @@ impl<'a> Formattable for Expr<'a> { .any(|loc_pattern| loc_pattern.is_multiline()) } - Record(fields) => fields.iter().any(|loc_field| loc_field.is_multiline()), - Tuple(fields) => fields.iter().any(|loc_field| loc_field.is_multiline()), - RecordUpdate { fields, .. } => fields.iter().any(|loc_field| loc_field.is_multiline()), + Record(fields) => is_collection_multiline(fields), + Tuple(fields) => is_collection_multiline(fields), + RecordUpdate { fields, .. } => is_collection_multiline(fields), } } @@ -1319,7 +1319,7 @@ fn fmt_record<'a, 'buf>( let loc_fields = fields.items; let final_comments = fields.final_comments(); buf.indent(indent); - if loc_fields.is_empty() && final_comments.iter().all(|c| c.is_newline()) { + if loc_fields.is_empty() && final_comments.iter().all(|c| c.is_newline()) && update.is_none() { buf.push_str("{}"); } else { buf.push('{'); diff --git a/crates/compiler/parse/src/expr.rs b/crates/compiler/parse/src/expr.rs index 79c1d7fcd7..9dce58ddbb 100644 --- a/crates/compiler/parse/src/expr.rs +++ b/crates/compiler/parse/src/expr.rs @@ -1,6 +1,6 @@ use crate::ast::{ AssignedField, Collection, CommentOrNewline, Defs, Expr, ExtractSpaces, Has, HasAbilities, - Pattern, Spaceable, TypeAnnotation, TypeDef, TypeHeader, ValueDef, + Pattern, Spaceable, Spaces, TypeAnnotation, TypeDef, TypeHeader, ValueDef, }; use crate::blankspace::{ space0_after_e, space0_around_e_no_after_indent_check, space0_around_ee, space0_before_e, @@ -1085,6 +1085,29 @@ enum AliasOrOpaque { Opaque, } +fn extract_tag_and_spaces<'a>(arena: &'a Bump, expr: Expr<'a>) -> Option> { + let mut expr = expr.extract_spaces(); + + loop { + match &expr.item { + Expr::ParensAround(inner_expr) => { + let inner_expr = inner_expr.extract_spaces(); + expr.item = inner_expr.item; + expr.before = merge_spaces(arena, expr.before, inner_expr.before); + expr.after = merge_spaces(arena, inner_expr.after, expr.after); + } + Expr::Tag(tag) => { + return Some(Spaces { + before: expr.before, + item: tag, + after: expr.after, + }); + } + _ => return None, + } + } +} + #[allow(clippy::too_many_arguments)] fn finish_parsing_alias_or_opaque<'a>( min_indent: u32, @@ -1105,120 +1128,113 @@ fn finish_parsing_alias_or_opaque<'a>( let mut defs = Defs::default(); - let state = match &expr.value.extract_spaces().item { - Expr::ParensAround(Expr::SpaceBefore(Expr::Tag(name), _)) - | Expr::ParensAround(Expr::SpaceAfter(Expr::Tag(name), _)) - | Expr::ParensAround(Expr::Tag(name)) - | Expr::Tag(name) => { - let mut type_arguments = Vec::with_capacity_in(arguments.len(), arena); + let state = if let Some(tag) = extract_tag_and_spaces(arena, expr.value) { + let name = tag.item; + let mut type_arguments = Vec::with_capacity_in(arguments.len(), arena); - for argument in arguments { - match expr_to_pattern_help(arena, &argument.value) { - Ok(good) => { - type_arguments.push(Loc::at(argument.region, good)); - } - Err(()) => { - return Err(( - MadeProgress, - EExpr::Pattern( - arena.alloc(EPattern::NotAPattern(state.pos())), - state.pos(), - ), - )); - } + for argument in arguments { + match expr_to_pattern_help(arena, &argument.value) { + Ok(good) => { + type_arguments.push(Loc::at(argument.region, good)); } - } - - match kind { - AliasOrOpaque::Alias => { - let (_, signature, state) = - alias_signature_with_space_before().parse(arena, state, min_indent)?; - - let def_region = Region::span_across(&expr.region, &signature.region); - - let header = TypeHeader { - name: Loc::at(expr.region, name), - vars: type_arguments.into_bump_slice(), - }; - - let def = TypeDef::Alias { - header, - ann: signature, - }; - - defs.push_type_def(def, def_region, &[], &[]); - - state - } - - AliasOrOpaque::Opaque => { - let (_, (signature, derived), state) = - opaque_signature_with_space_before().parse(arena, state, indented_more)?; - - let def_region = Region::span_across(&expr.region, &signature.region); - - let header = TypeHeader { - name: Loc::at(expr.region, name), - vars: type_arguments.into_bump_slice(), - }; - - let def = TypeDef::Opaque { - header, - typ: signature, - derived, - }; - - defs.push_type_def(def, def_region, &[], &[]); - - state + Err(()) => { + return Err(( + MadeProgress, + EExpr::Pattern( + arena.alloc(EPattern::NotAPattern(state.pos())), + state.pos(), + ), + )); } } } - _ => { - let call = to_call(arena, arguments, expr); + match kind { + AliasOrOpaque::Alias => { + let (_, signature, state) = + alias_signature_with_space_before().parse(arena, state, min_indent)?; - match expr_to_pattern_help(arena, &call.value) { - Ok(good) => { - let parser = specialize( - EExpr::Type, - space0_before_e( - set_min_indent(indented_more, type_annotation::located(false)), - EType::TIndentStart, - ), - ); + let def_region = Region::span_across(&expr.region, &signature.region); - match parser.parse(arena, state.clone(), min_indent) { - Err((_, fail)) => return Err((MadeProgress, fail)), - Ok((_, mut ann_type, state)) => { - // put the spaces from after the operator in front of the call - if !spaces_after_operator.is_empty() { - ann_type = arena - .alloc(ann_type.value) - .with_spaces_before(spaces_after_operator, ann_type.region); - } + let header = TypeHeader { + name: Loc::at(expr.region, name), + vars: type_arguments.into_bump_slice(), + }; - let def_region = Region::span_across(&call.region, &ann_type.region); + let def = TypeDef::Alias { + header, + ann: signature, + }; - let value_def = - ValueDef::Annotation(Loc::at(expr_region, good), ann_type); + defs.push_type_def(def, def_region, &[], &[]); - defs.push_value_def(value_def, def_region, &[], &[]); + state + } - state + AliasOrOpaque::Opaque => { + let (_, (signature, derived), state) = + opaque_signature_with_space_before().parse(arena, state, indented_more)?; + + let def_region = Region::span_across(&expr.region, &signature.region); + + let header = TypeHeader { + name: Loc::at(expr.region, name), + vars: type_arguments.into_bump_slice(), + }; + + let def = TypeDef::Opaque { + header, + typ: signature, + derived, + }; + + defs.push_type_def(def, def_region, &[], &[]); + + state + } + } + } else { + let call = to_call(arena, arguments, expr); + + match expr_to_pattern_help(arena, &call.value) { + Ok(good) => { + let parser = specialize( + EExpr::Type, + space0_before_e( + set_min_indent(indented_more, type_annotation::located(false)), + EType::TIndentStart, + ), + ); + + match parser.parse(arena, state.clone(), min_indent) { + Err((_, fail)) => return Err((MadeProgress, fail)), + Ok((_, mut ann_type, state)) => { + // put the spaces from after the operator in front of the call + if !spaces_after_operator.is_empty() { + ann_type = arena + .alloc(ann_type.value) + .with_spaces_before(spaces_after_operator, ann_type.region); } + + let def_region = Region::span_across(&call.region, &ann_type.region); + + let value_def = ValueDef::Annotation(Loc::at(expr_region, good), ann_type); + + defs.push_value_def(value_def, def_region, &[], &[]); + + state } } - Err(_) => { - // this `:`/`:=` likely occurred inline; treat it as an invalid operator - let op = match kind { - AliasOrOpaque::Alias => ":", - AliasOrOpaque::Opaque => ":=", - }; - let fail = EExpr::BadOperator(op, loc_op.region.start()); + } + Err(_) => { + // this `:`/`:=` likely occurred inline; treat it as an invalid operator + let op = match kind { + AliasOrOpaque::Alias => ":", + AliasOrOpaque::Opaque => ":=", + }; + let fail = EExpr::BadOperator(op, loc_op.region.start()); - return Err((MadeProgress, fail)); - } + return Err((MadeProgress, fail)); } } }; diff --git a/crates/compiler/test_syntax/src/test_helpers.rs b/crates/compiler/test_syntax/src/test_helpers.rs index 27e9f22a8a..e0038c5705 100644 --- a/crates/compiler/test_syntax/src/test_helpers.rs +++ b/crates/compiler/test_syntax/src/test_helpers.rs @@ -243,10 +243,11 @@ impl<'a> Input<'a> { let reformatted = reparsed_ast.format(); if output != reformatted { - eprintln!("Formatting bug; formatting is not stable.\nOriginal code:\n{}\n\nFormatted code:\n{}\n\nAST:\n{:#?}\n\n", + eprintln!("Formatting bug; formatting is not stable.\nOriginal code:\n{}\n\nFormatted code:\n{}\n\nAST:\n{:#?}\n\nReparsed AST:\n{:#?}\n\n", self.as_str(), output.as_ref().as_str(), - actual); + actual, + reparsed_ast); eprintln!("Reformatting the formatted code changed it again, as follows:\n\n"); assert_multiline_str_eq!(output.as_ref().as_str(), reformatted.as_ref().as_str()); diff --git a/crates/compiler/test_syntax/tests/snapshots/pass/empty_record_update.expr.formatted.roc b/crates/compiler/test_syntax/tests/snapshots/pass/empty_record_update.expr.formatted.roc new file mode 100644 index 0000000000..727025b89a --- /dev/null +++ b/crates/compiler/test_syntax/tests/snapshots/pass/empty_record_update.expr.formatted.roc @@ -0,0 +1 @@ +{ e & } \ No newline at end of file diff --git a/crates/compiler/test_syntax/tests/snapshots/pass/empty_record_update.expr.result-ast b/crates/compiler/test_syntax/tests/snapshots/pass/empty_record_update.expr.result-ast new file mode 100644 index 0000000000..f80fadf9a5 --- /dev/null +++ b/crates/compiler/test_syntax/tests/snapshots/pass/empty_record_update.expr.result-ast @@ -0,0 +1,7 @@ +RecordUpdate { + update: @1-2 Var { + module_name: "", + ident: "e", + }, + fields: [], +} diff --git a/crates/compiler/test_syntax/tests/snapshots/pass/empty_record_update.expr.roc b/crates/compiler/test_syntax/tests/snapshots/pass/empty_record_update.expr.roc new file mode 100644 index 0000000000..41c40656bf --- /dev/null +++ b/crates/compiler/test_syntax/tests/snapshots/pass/empty_record_update.expr.roc @@ -0,0 +1 @@ +{e&} \ No newline at end of file diff --git a/crates/compiler/test_syntax/tests/snapshots/pass/list_minus_newlines.expr.formatted.roc b/crates/compiler/test_syntax/tests/snapshots/pass/list_minus_newlines.expr.formatted.roc new file mode 100644 index 0000000000..4fa5c5f894 --- /dev/null +++ b/crates/compiler/test_syntax/tests/snapshots/pass/list_minus_newlines.expr.formatted.roc @@ -0,0 +1,4 @@ +[ + K, +] +- i \ No newline at end of file diff --git a/crates/compiler/test_syntax/tests/snapshots/pass/list_minus_newlines.expr.result-ast b/crates/compiler/test_syntax/tests/snapshots/pass/list_minus_newlines.expr.result-ast new file mode 100644 index 0000000000..89d20368d7 --- /dev/null +++ b/crates/compiler/test_syntax/tests/snapshots/pass/list_minus_newlines.expr.result-ast @@ -0,0 +1,23 @@ +BinOps( + [ + ( + @0-5 List( + Collection { + items: [ + @1-2 Tag( + "K", + ), + ], + final_comments: [ + Newline, + ], + }, + ), + @5-6 Minus, + ), + ], + @6-7 Var { + module_name: "", + ident: "i", + }, +) diff --git a/crates/compiler/test_syntax/tests/snapshots/pass/list_minus_newlines.expr.roc b/crates/compiler/test_syntax/tests/snapshots/pass/list_minus_newlines.expr.roc new file mode 100644 index 0000000000..e8153c3ccb --- /dev/null +++ b/crates/compiler/test_syntax/tests/snapshots/pass/list_minus_newlines.expr.roc @@ -0,0 +1,2 @@ +[K, +]-i \ No newline at end of file diff --git a/crates/compiler/test_syntax/tests/snapshots/pass/opaque_type_def_with_newline.expr.formatted.roc b/crates/compiler/test_syntax/tests/snapshots/pass/opaque_type_def_with_newline.expr.formatted.roc new file mode 100644 index 0000000000..d8c2642763 --- /dev/null +++ b/crates/compiler/test_syntax/tests/snapshots/pass/opaque_type_def_with_newline.expr.formatted.roc @@ -0,0 +1,4 @@ +a : e +Na := + e +e0 \ No newline at end of file diff --git a/crates/compiler/test_syntax/tests/snapshots/pass/opaque_type_def_with_newline.expr.result-ast b/crates/compiler/test_syntax/tests/snapshots/pass/opaque_type_def_with_newline.expr.result-ast new file mode 100644 index 0000000000..caf6bad6a3 --- /dev/null +++ b/crates/compiler/test_syntax/tests/snapshots/pass/opaque_type_def_with_newline.expr.result-ast @@ -0,0 +1,54 @@ +Defs( + Defs { + tags: [ + Index(2147483648), + Index(0), + ], + regions: [ + @0-3, + @4-11, + ], + space_before: [ + Slice(start = 0, length = 0), + Slice(start = 0, length = 1), + ], + space_after: [ + Slice(start = 0, length = 0), + Slice(start = 1, length = 0), + ], + spaces: [ + Newline, + ], + type_defs: [ + Opaque { + header: TypeHeader { + name: @4-6 "Na", + vars: [], + }, + typ: @10-11 SpaceBefore( + BoundVariable( + "e", + ), + [ + Newline, + ], + ), + derived: None, + }, + ], + value_defs: [ + Annotation( + @0-1 Identifier( + "a", + ), + @2-3 BoundVariable( + "e", + ), + ), + ], + }, + @12-14 Var { + module_name: "", + ident: "e0", + }, +) diff --git a/crates/compiler/test_syntax/tests/snapshots/pass/opaque_type_def_with_newline.expr.roc b/crates/compiler/test_syntax/tests/snapshots/pass/opaque_type_def_with_newline.expr.roc new file mode 100644 index 0000000000..92ed4b6c5b --- /dev/null +++ b/crates/compiler/test_syntax/tests/snapshots/pass/opaque_type_def_with_newline.expr.roc @@ -0,0 +1,3 @@ +a:e +Na:= + e e0 \ No newline at end of file diff --git a/crates/compiler/test_syntax/tests/test_snapshots.rs b/crates/compiler/test_syntax/tests/test_snapshots.rs index e96c7e2c03..afb979f2f3 100644 --- a/crates/compiler/test_syntax/tests/test_snapshots.rs +++ b/crates/compiler/test_syntax/tests/test_snapshots.rs @@ -304,6 +304,7 @@ mod test_snapshots { pass/empty_package_header.header, pass/empty_platform_header.header, pass/empty_record.expr, + pass/empty_record_update.expr, pass/empty_string.expr, pass/equals.expr, pass/equals_with_spaces.expr, @@ -327,6 +328,7 @@ mod test_snapshots { pass/list_closing_indent_not_enough.expr, pass/list_closing_same_indent_no_trailing_comma.expr, pass/list_closing_same_indent_with_trailing_comma.expr, + pass/list_minus_newlines.expr, pass/list_pattern_weird_indent.expr, pass/list_patterns.expr, pass/lowest_float.expr, @@ -389,6 +391,7 @@ mod test_snapshots { pass/opaque_reference_pattern.expr, pass/opaque_reference_pattern_with_arguments.expr, pass/opaque_simple.moduledefs, + pass/opaque_type_def_with_newline.expr, pass/opaque_with_type_arguments.moduledefs, pass/ops_with_newlines.expr, pass/outdented_app_with_record.expr,