Merge pull request #7409 from joshuawarner32/fuzzing-bugs-5

Fix another batch of parsing/formatting bugs found in fuzzing
roc-lang · Dec 28, 2024 · f7dbf85 · f7dbf85
2 parents 005e9ad + ee3619f
commit f7dbf85
Show file tree

Hide file tree

Showing 31 changed files with 748 additions and 72 deletions.
diff --git a/crates/compiler/fmt/src/annotation.rs b/crates/compiler/fmt/src/annotation.rs
@@ -1329,6 +1329,7 @@ impl<'a> Nodify<'a> for TypeAnnotation<'a> {
                             item,
                             after,
                         },
+                        true,
                     )
                 } else {
                     Spaces {
@@ -1369,7 +1370,7 @@ impl<'a> Nodify<'a> for TypeAnnotation<'a> {
                     after: new_res.after,
                 };
                 if parens == Parens::InCollection || parens == Parens::InApply {
-                    parens_around_node(arena, inner)
+                    parens_around_node(arena, inner, true)
                 } else {
                     inner
                 }
@@ -1382,7 +1383,7 @@ impl<'a> Nodify<'a> for TypeAnnotation<'a> {
                     after: lifted.after,
                 };
                 if parens == Parens::InApply || parens == Parens::InAsPattern {
-                    parens_around_node(arena, item)
+                    parens_around_node(arena, item, true)
                 } else {
                     item
                 }
@@ -1402,12 +1403,24 @@ impl<'a> Nodify<'a> for TypeAnnotation<'a> {
 fn parens_around_node<'a, 'b: 'a>(
     arena: &'a Bump,
     item: Spaces<'b, Node<'b>>,
+    allow_spaces_before: bool,
 ) -> Spaces<'a, Node<'a>> {
     Spaces {
-        before: &[],
+        before: if allow_spaces_before {
+            item.before
+        } else {
+            &[]
+        },
         item: Node::DelimitedSequence(
             Braces::Round,
-            arena.alloc_slice_copy(&[(item.before.into(), item.item)]),
+            arena.alloc_slice_copy(&[(
+                if allow_spaces_before {
+                    Sp::empty()
+                } else {
+                    item.before.into()
+                },
+                item.item,
+            )]),
             Sp::empty(),
         ),
         // We move the comments/newlines to the outer scope, since they tend to migrate there when re-parsed
@@ -1432,6 +1445,10 @@ impl<'a, T: Copy> ExtractSpaces<'a> for NodeSpaces<'a, T> {
             after: self.after,
         }
     }
+
+    fn without_spaces(&self) -> T {
+        self.item
+    }
 }
 
 impl<'a, V: Formattable> Formattable for NodeSpaces<'a, V> {

diff --git a/crates/compiler/fmt/src/expr.rs b/crates/compiler/fmt/src/expr.rs
@@ -291,7 +291,7 @@ fn format_expr_only(
                 let inner_parens = if needs_parens {
                     Parens::NotNeeded
                 } else {
-                    Parens::InApply
+                    Parens::InApplyLastArg
                 };
 
                 if !before_all_newlines {
@@ -1314,13 +1314,21 @@ pub fn expr_lift_spaces<'a, 'b: 'a>(
                 after: right_lifted.after,
             }
         }
-        Expr::UnaryOp(expr, op) => {
-            let expr_lifted = expr_lift_spaces_after(Parens::InOperator, arena, &expr.value);
+        Expr::UnaryOp(inner, op) => {
+            if parens == Parens::InApply && matches!(inner.without_spaces(), Expr::Closure(..)) {
+                return Spaces {
+                    before: &[],
+                    item: Expr::ParensAround(arena.alloc(*expr)),
+                    after: &[],
+                };
+            }
+
+            let inner_lifted = expr_lift_spaces_after(Parens::InOperator, arena, &inner.value);
 
             Spaces {
                 before: &[],
-                item: Expr::UnaryOp(arena.alloc(Loc::at(expr.region, expr_lifted.item)), *op),
-                after: expr_lifted.after,
+                item: Expr::UnaryOp(arena.alloc(Loc::at(inner.region, inner_lifted.item)), *op),
+                after: inner_lifted.after,
             }
         }
 

diff --git a/crates/compiler/parse/src/ast.rs b/crates/compiler/parse/src/ast.rs
@@ -34,6 +34,10 @@ impl<'a, T: Copy> ExtractSpaces<'a> for Spaces<'a, T> {
     fn extract_spaces(&self) -> Spaces<'a, T> {
         *self
     }
+
+    fn without_spaces(&self) -> T {
+        self.item
+    }
 }
 
 #[derive(Debug, Copy, Clone, PartialEq, Eq)]
@@ -111,13 +115,18 @@ impl<'a, T: Debug> Debug for Spaced<'a, T> {
 pub trait ExtractSpaces<'a>: Sized + Copy {
     type Item;
     fn extract_spaces(&self) -> Spaces<'a, Self::Item>;
+    fn without_spaces(&self) -> Self::Item;
 }
 
 impl<'a, T: ExtractSpaces<'a>> ExtractSpaces<'a> for &'a T {
     type Item = T::Item;
     fn extract_spaces(&self) -> Spaces<'a, Self::Item> {
         (*self).extract_spaces()
     }
+
+    fn without_spaces(&self) -> Self::Item {
+        (*self).without_spaces()
+    }
 }
 
 impl<'a, T: ExtractSpaces<'a>> ExtractSpaces<'a> for Loc<T> {
@@ -130,6 +139,10 @@ impl<'a, T: ExtractSpaces<'a>> ExtractSpaces<'a> for Loc<T> {
             after: spaces.after,
         }
     }
+
+    fn without_spaces(&self) -> Self::Item {
+        self.value.without_spaces()
+    }
 }
 
 impl<'a> Header<'a> {
@@ -2350,6 +2363,17 @@ macro_rules! impl_extract_spaces {
                     }
                 }
             }
+            fn without_spaces(&self) -> Self::Item {
+                match self {
+                    $t::SpaceBefore(item, _) => {
+                        item.without_spaces()
+                    },
+                    $t::SpaceAfter(item, _) => {
+                        item.without_spaces()
+                    },
+                    _ => *self,
+                }
+            }
         }
     };
 }
@@ -2412,6 +2436,14 @@ impl<'a, T: Copy> ExtractSpaces<'a> for Spaced<'a, T> {
             },
         }
     }
+
+    fn without_spaces(&self) -> T {
+        match self {
+            Spaced::SpaceBefore(item, _) => item.without_spaces(),
+            Spaced::SpaceAfter(item, _) => item.without_spaces(),
+            Spaced::Item(item) => *item,
+        }
+    }
 }
 
 impl<'a> ExtractSpaces<'a> for AbilityImpls<'a> {
@@ -2454,6 +2486,14 @@ impl<'a> ExtractSpaces<'a> for AbilityImpls<'a> {
             },
         }
     }
+
+    fn without_spaces(&self) -> Self::Item {
+        match self {
+            AbilityImpls::AbilityImpls(inner) => *inner,
+            AbilityImpls::SpaceBefore(item, _) => item.without_spaces(),
+            AbilityImpls::SpaceAfter(item, _) => item.without_spaces(),
+        }
+    }
 }
 
 pub trait Malformed {

diff --git a/crates/compiler/parse/src/pattern.rs b/crates/compiler/parse/src/pattern.rs
@@ -1,4 +1,4 @@
-use crate::ast::{Collection, Implements, Pattern, PatternAs, Spaceable};
+use crate::ast::{Collection, ExtractSpaces, Implements, Pattern, PatternAs, Spaceable};
 use crate::blankspace::{space0_e, spaces, spaces_before};
 use crate::ident::{lowercase_ident, parse_ident, Accessor, Ident};
 use crate::keyword;
@@ -150,7 +150,7 @@ fn loc_tag_pattern_arg<'a>(
 
         if stop_on_has_kw
             && matches!(
-                value,
+                value.without_spaces(),
                 Pattern::Identifier {
                     ident: crate::keyword::IMPLEMENTS,
                     ..