Introduce proper type for TypeVar's, mark anything not a lowercase ident as malformed

crates/compiler/fmt/src/annotation.rs

@@ -1,11 +1,11 @@
 use crate::{
     collection::{fmt_collection, Braces},
-    expr::merge_spaces_conservative,
+    expr::{expr_lift_spaces, expr_lift_spaces_after, expr_prec, merge_spaces_conservative},
     node::{
         parens_around_node, DelimitedItem, Item, Node, NodeInfo, NodeSequenceBuilder, Nodify, Prec,
         Sp,
     },
-    pattern::{pattern_lift_spaces_after, snakify_camel_ident},
+    pattern::snakify_camel_ident,
     spaces::{fmt_comments_only, fmt_spaces, NewlineAt, INDENT},
     Buf, MigrationFlags,
 };
@@ -13,7 +13,6 @@ use bumpalo::{
     collections::{String, Vec},
     Bump,
 };
-use roc_parse::{ast::Spaced, ident::UppercaseIdent};
 use roc_parse::{
     ast::{
         AbilityImpls, AssignedField, Collection, CommentOrNewline, Expr, ExtractSpaces,
@@ -22,6 +21,10 @@ use roc_parse::{
     },
     expr::merge_spaces,
 };
+use roc_parse::{
+    ast::{Spaced, TypeVar},
+    ident::UppercaseIdent,
+};
 use roc_region::all::Loc;
 
 /// Does an AST node need parens around it?
@@ -966,7 +969,7 @@ pub fn type_head_lift_spaces_after<'a, 'b: 'a>(
 ) -> SpacesAfter<'a, TypeHeader<'a>> {
     let new_vars = arena.alloc_slice_copy(header.vars);
     let after = if let Some(last) = new_vars.last_mut() {
-        let lifted = pattern_lift_spaces_after(arena, &last.value);
+        let lifted = type_var_lift_spaces_after(arena, last.value);
         last.value = lifted.item;
         lifted.after
     } else {
@@ -981,6 +984,135 @@ pub fn type_head_lift_spaces_after<'a, 'b: 'a>(
     }
 }
 
+fn type_var_lift_spaces_after<'a, 'b: 'a>(
+    arena: &'a Bump,
+    var: TypeVar<'b>,
+) -> SpacesAfter<'a, TypeVar<'a>> {
+    match var {
+        item @ TypeVar::Identifier(_) => SpacesAfter { item, after: &[] },
+        TypeVar::Malformed(expr) => {
+            let lifted = expr_lift_spaces_after(Parens::NotNeeded, arena, expr);
+            SpacesAfter {
+                item: TypeVar::Malformed(arena.alloc(lifted.item)),
+                after: lifted.after,
+            }
+        }
+        TypeVar::SpaceBefore(inner, spaces) => {
+            let lifted = type_var_lift_spaces_after(arena, *inner);
+            SpacesAfter {
+                item: TypeVar::SpaceBefore(arena.alloc(lifted.item), spaces),
+                after: lifted.after,
+            }
+        }
+        TypeVar::SpaceAfter(inner, spaces) => {
+            let mut lifted = type_var_lift_spaces_after(arena, *inner);
+            lifted.after = merge_spaces_conservative(arena, lifted.after, spaces);
+            lifted
+        }
+    }
+}
+
+impl<'a> Formattable for TypeHeader<'a> {
+    fn is_multiline(&self) -> bool {
+        self.vars.iter().any(|v| v.is_multiline())
+    }
+
+    fn format_with_options(
+        &self,
+        buf: &mut Buf,
+        _parens: Parens,
+        _newlines: Newlines,
+        indent: u16,
+    ) {
+        let node = self.to_node(buf.text.bump(), buf.flags());
+        node.format(buf, indent);
+    }
+}
+
+impl<'a> Formattable for TypeVar<'a> {
+    fn is_multiline(&self) -> bool {
+        match self {
+            TypeVar::Identifier(_) => false,
+            TypeVar::Malformed(expr) => expr.is_multiline(),
+            TypeVar::SpaceBefore(inner, spaces) | TypeVar::SpaceAfter(inner, spaces) => {
+                inner.is_multiline() || !spaces.is_empty()
+            }
+        }
+    }
+
+    fn format_with_options(
+        &self,
+        buf: &mut Buf,
+        _parens: Parens,
+        _newlines: Newlines,
+        indent: u16,
+    ) {
+        let node = self.to_node(buf.text.bump(), buf.flags());
+        node.format(buf, indent);
+    }
+}
+
+impl<'a> Nodify<'a> for TypeHeader<'a> {
+    fn to_node<'b>(&'a self, arena: &'b Bump, flags: MigrationFlags) -> NodeInfo<'b>
+    where
+        'a: 'b,
+    {
+        NodeInfo::apply(
+            arena,
+            NodeInfo::item(Node::Literal(self.name.value)),
+            self.vars.iter().map(|v| v.value.to_node(arena, flags)),
+        )
+    }
+}
+
+impl<'a> Nodify<'a> for TypeVar<'a> {
+    fn to_node<'b>(&'a self, arena: &'b Bump, flags: MigrationFlags) -> NodeInfo<'b>
+    where
+        'a: 'b,
+    {
+        match self {
+            TypeVar::SpaceBefore(inner, spaces) => {
+                let mut inner = inner.to_node(arena, flags);
+                inner.before = merge_spaces_conservative(arena, spaces, inner.before);
+                inner
+            }
+            TypeVar::SpaceAfter(inner, spaces) => {
+                let mut inner = inner.to_node(arena, flags);
+                inner.after = merge_spaces_conservative(arena, inner.after, spaces);
+                inner
+            }
+            TypeVar::Identifier(text) => {
+                let var_name = if flags.snakify {
+                    let mut buf = Buf::new_in(arena, flags);
+                    buf.indent(0); // Take out of beginning of line
+                    snakify_camel_ident(&mut buf, text);
+                    let s: &str = arena.alloc_str(buf.as_str());
+                    s
+                } else {
+                    text
+                };
+                let item = NodeInfo::item(Node::Literal(var_name));
+
+                if *text == "implements" {
+                    parens_around_node(arena, item, false)
+                } else {
+                    item
+                }
+            }
+            TypeVar::Malformed(expr) => {
+                let lifted = expr_lift_spaces(Parens::InApply, arena, expr);
+                NodeInfo {
+                    before: lifted.before,
+                    node: Node::Expr(lifted.item),
+                    after: lifted.after,
+                    needs_indent: true,
+                    prec: expr_prec(**expr),
+                }
+            }
+        }
+    }
+}
+
 impl<'a> Nodify<'a> for TypeAnnotation<'a> {
     fn to_node<'b>(&'a self, arena: &'b Bump, flags: MigrationFlags) -> NodeInfo<'b>
     where

crates/compiler/fmt/src/def.rs

@@ -7,20 +7,19 @@ use crate::expr::{
     expr_lift_and_lower, expr_lift_spaces, expr_lift_spaces_after, expr_lift_spaces_before,
     fmt_str_literal, is_str_multiline, merge_spaces_conservative, sub_expr_requests_parens,
 };
-use crate::node::{NodeInfo, Nodify};
-use crate::pattern::{pattern_apply_to_node, pattern_fmt_apply};
-use crate::pattern::{pattern_lift_spaces, pattern_lift_spaces_before};
+use crate::node::Nodify;
+use crate::pattern::pattern_lift_spaces_before;
 use crate::spaces::{
     fmt_comments_only, fmt_default_newline, fmt_default_spaces, fmt_spaces, NewlineAt, INDENT,
 };
-use crate::{Buf, MigrationFlags};
+use crate::Buf;
 use bumpalo::Bump;
 use roc_error_macros::internal_error;
 use roc_parse::ast::{
     AbilityMember, Defs, Expr, ExtractSpaces, ImportAlias, ImportAsKeyword, ImportExposingKeyword,
     ImportedModuleName, IngestedFileAnnotation, IngestedFileImport, ModuleImport,
     ModuleImportParams, Pattern, Spaces, SpacesBefore, StrLiteral, TypeAnnotation, TypeDef,
-    TypeHeader, ValueDef,
+    ValueDef,
 };
 use roc_parse::expr::merge_spaces;
 use roc_parse::header::Keyword;
@@ -474,8 +473,8 @@ impl<'a> Formattable for TypeDef<'a> {
                 loc_implements,
                 members,
             } => {
-                let header_lifted = type_head_lift_spaces(buf.text.bump(), *header);
-                header_lifted.item.format(buf, indent);
+                let header_lifted = header.to_node(buf.text.bump(), buf.flags());
+                header_lifted.node.format(buf, indent);
                 let implements = loc_implements.value.extract_spaces();
                 let before_implements = merge_spaces_conservative(
                     buf.text.bump(),
@@ -532,57 +531,6 @@ 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(
-        &self,
-        buf: &mut Buf,
-        _parens: Parens,
-        _newlines: Newlines,
-        indent: u16,
-    ) {
-        let old_flags = buf.flags;
-        buf.flags = MigrationFlags {
-            parens_and_commas: false,
-            ..old_flags
-        };
-        pattern_fmt_apply(
-            buf,
-            Pattern::Tag(self.name.value),
-            self.vars,
-            Parens::NotNeeded,
-            indent,
-            self.vars.iter().any(|v| v.is_multiline()),
-            false,
-            None,
-        );
-        buf.flags = old_flags;
-    }
-}
-
-fn type_head_lift_spaces<'a, 'b: 'a>(
-    arena: &'a Bump,
-    head: TypeHeader<'b>,
-) -> Spaces<'a, Pattern<'a>> {
-    let pat = Pattern::Apply(
-        arena.alloc(Loc::at(head.name.region, Pattern::Tag(head.name.value))),
-        head.vars,
-    );
-
-    pattern_lift_spaces(arena, &pat)
-}
-
-impl<'a> Nodify<'a> for TypeHeader<'a> {
-    fn to_node<'b>(&'a self, arena: &'b Bump, _flags: MigrationFlags) -> NodeInfo<'b>
-    where
-        'a: 'b,
-    {
-        pattern_apply_to_node(arena, Pattern::Tag(self.name.value), self.vars)
-    }
-}
 impl<'a> Formattable for ModuleImport<'a> {
     fn is_multiline(&self) -> bool {
         let Self {

crates/compiler/fmt/src/expr.rs

@@ -1,6 +1,7 @@
 use crate::annotation::{except_last, is_collection_multiline, Formattable, Newlines, Parens};
 use crate::collection::{fmt_collection, Braces};
 use crate::def::{fmt_defs, valdef_lift_spaces_before};
+use crate::node::Prec;
 use crate::pattern::{
     fmt_pattern, pattern_lift_spaces, snakify_camel_ident, starts_with_inline_comment,
 };
@@ -1390,11 +1391,7 @@ pub fn expr_lift_spaces<'a, 'b: 'a>(
             before: &[],
             item: *expr,
             after: &[],
-        }, // _ => Spaces {
-           //     before: &[],
-           //     item: *expr,
-           //     after: &[],
-           // },
+        },
     }
 }
 
@@ -1410,6 +1407,54 @@ pub fn expr_lift_spaces_before<'a, 'b: 'a>(
     }
 }
 
+pub fn expr_prec(expr: Expr<'_>) -> Prec {
+    match expr {
+        Expr::Float(_)
+        | Expr::Num(_)
+        | Expr::NonBase10Int { .. }
+        | Expr::Str(_)
+        | Expr::SingleQuote(_)
+        | Expr::AccessorFunction(_)
+        | Expr::RecordUpdater(_)
+        | Expr::Var { .. }
+        | Expr::Underscore(_)
+        | Expr::Crash
+        | Expr::Tag(_)
+        | Expr::OpaqueRef(_)
+        | Expr::Dbg
+        | Expr::Try
+        | Expr::MalformedIdent(_, _)
+        | Expr::EmptyRecordBuilder(_)
+        | Expr::SingleFieldRecordBuilder(_)
+        | Expr::RecordAccess(_, _)
+        | Expr::TupleAccess(_, _)
+        | Expr::TrySuffix { .. }
+        | Expr::List(_)
+        | Expr::RecordUpdate { .. }
+        | Expr::Record(_)
+        | Expr::Tuple(_)
+        | Expr::RecordBuilder { .. }
+        | Expr::LowLevelTry(_, _)
+        | Expr::LowLevelDbg(_, _, _)
+        | Expr::PncApply(_, _)
+        | Expr::OptionalFieldInRecordBuilder(_, _) => Prec::Term,
+
+        Expr::Closure(_, _)
+        | Expr::Defs(_, _)
+        | Expr::DbgStmt { .. }
+        | Expr::Apply(_, _, _)
+        | Expr::BinOps(_, _)
+        | Expr::UnaryOp(_, _)
+        | Expr::If { .. }
+        | Expr::When(_, _)
+        | Expr::Return(_, _)
+        | Expr::SpaceBefore(_, _)
+        | Expr::SpaceAfter(_, _)
+        | Expr::ParensAround(_)
+        | Expr::PrecedenceConflict(_) => Prec::Apply,
+    }
+}
+
 pub fn expr_lift_spaces_after<'a, 'b: 'a>(
     parens: Parens,
     arena: &'a Bump,

crates/compiler/fmt/src/node.rs

@@ -1,5 +1,5 @@
 use bumpalo::{collections::Vec, Bump};
-use roc_parse::ast::{CommentOrNewline, Pattern, TypeAnnotation};
+use roc_parse::ast::{CommentOrNewline, Expr, Pattern, TypeAnnotation};
 
 use crate::{
     annotation::{Formattable, Newlines, Parens},
@@ -96,6 +96,7 @@ pub enum Node<'a> {
     // Temporary! TODO: translate these into proper Node elements
     TypeAnnotation(TypeAnnotation<'a>),
     Pattern(Pattern<'a>),
+    Expr(Expr<'a>),
 }
 
 #[derive(Copy, Clone, Debug)]
@@ -321,6 +322,24 @@ fn fmt_sp(buf: &mut Buf, sp: Sp<'_>, indent: u16) {
     }
 }
 
+impl<'a> Formattable for NodeInfo<'a> {
+    fn is_multiline(&self) -> bool {
+        !self.before.is_empty() || self.node.is_multiline() || !self.after.is_empty()
+    }
+
+    fn format_with_options(
+        &self,
+        buf: &mut Buf,
+        _parens: Parens,
+        _newlines: Newlines,
+        indent: u16,
+    ) {
+        fmt_spaces(buf, self.before.iter(), indent);
+        self.node.format(buf, indent);
+        fmt_spaces(buf, self.after.iter(), indent);
+    }
+}
+
 impl<'a> Formattable for Node<'a> {
     fn is_multiline(&self) -> bool {
         match self {
@@ -350,6 +369,7 @@ impl<'a> Formattable for Node<'a> {
             Node::Literal(_) => false,
             Node::TypeAnnotation(type_annotation) => type_annotation.is_multiline(),
             Node::Pattern(pat) => pat.is_multiline(),
+            Node::Expr(expr) => expr.is_multiline(),
         }
     }
 
@@ -452,6 +472,9 @@ impl<'a> Formattable for Node<'a> {
             Node::Pattern(pat) => {
                 pat.format_with_options(buf, parens, newlines, indent);
             }
+            Node::Expr(expr) => {
+                expr.format_with_options(buf, parens, newlines, indent);
+            }
         }
     }
 }

crates/compiler/fmt/src/pattern.rs

@@ -91,6 +91,7 @@ impl<'a> Formattable for Pattern<'a> {
             | Pattern::Underscore(_)
             | Pattern::Malformed(_)
             | Pattern::MalformedIdent(_, _)
+            | Pattern::MalformedExpr(_)
             | Pattern::QualifiedIdentifier { .. } => false,
 
             Pattern::Tuple(patterns) | Pattern::List(patterns) => {
@@ -447,6 +448,10 @@ fn fmt_pattern_only(
             buf.indent(indent);
             buf.push_str(string);
         }
+        Pattern::MalformedExpr(expr) => {
+            buf.indent(indent);
+            expr.format(buf, indent);
+        }
         Pattern::QualifiedIdentifier { module_name, ident } => {
             buf.indent(indent);
             if !module_name.is_empty() {
@@ -538,20 +543,6 @@ pub fn pattern_fmt_apply(
         let mut before = merge_spaces(buf.text.bump(), last_after, arg.before);
 
         if !before.is_empty() {
-            if starts_with_block_str(&arg.item) {
-                // Ick!
-                // The block string will keep "generating" newlines when formatted (it wants to start on its own line),
-                // so we strip one out here.
-                //
-                // Note that this doesn't affect Expr's because those have explicit parens, and we can control
-                // whether spaces cross that boundary.
-                let chop_off = before
-                    .iter()
-                    .rev()
-                    .take_while(|&&s| matches!(s, CommentOrNewline::Newline))
-                    .count();
-                before = &before[..before.len() - chop_off];
-            }
             handle_multiline_str_spaces(&arg.item, &mut before);
 
             if !is_multiline {
@@ -674,7 +665,9 @@ fn pattern_prec(pat: Pattern<'_>) -> Prec {
         | Pattern::PncApply(_, _) => Prec::Term,
         Pattern::Apply(_, _) | Pattern::As(_, _) => Prec::Apply,
         Pattern::SpaceBefore(inner, _) | Pattern::SpaceAfter(inner, _) => pattern_prec(*inner),
-        Pattern::Malformed(_) | Pattern::MalformedIdent(..) => Prec::Term,
+        Pattern::Malformed(_) | Pattern::MalformedIdent(..) | Pattern::MalformedExpr(_) => {
+            Prec::Term
+        }
     }
 }