From c670cfa1c2aec52427342c2c075d4d798c960c2e Mon Sep 17 00:00:00 2001 From: Anthony Bullard Date: Sun, 5 Jan 2025 16:56:22 -0600 Subject: [PATCH] Extract Spaces refactor / cleanup Have extract_spaces method of ExtractSpaces trait take an arena so that: 1. We can remove a lot of `todo!()`s. 2. Recursively collapse several layers of SpaceBefore/SpaceAfter into a single Spaces. 3. Avoid some fuzzer panics (but not all). --- crates/compiler/can/src/annotation.rs | 8 +- crates/compiler/can/src/def.rs | 8 +- crates/compiler/can/src/pattern.rs | 2 +- crates/compiler/fmt/src/annotation.rs | 17 +- crates/compiler/fmt/src/collection.rs | 9 +- crates/compiler/fmt/src/def.rs | 43 ++-- crates/compiler/fmt/src/expr.rs | 36 ++-- crates/compiler/fmt/src/lib.rs | 6 + crates/compiler/fmt/src/pattern.rs | 5 +- crates/compiler/load_internal/src/docs.rs | 11 +- crates/compiler/load_internal/src/file.rs | 5 +- crates/compiler/parse/src/ast.rs | 204 ++++++++++++++---- crates/compiler/parse/src/expr.rs | 50 ++--- crates/compiler/parse/src/header.rs | 5 +- .../fuzz/fuzz_targets/fuzz_expr.rs | 18 +- .../fuzz/fuzz_targets/fuzz_module.rs | 14 +- .../pass/anthony_testing.expr.formatted.roc | 6 + .../pass/anthony_testing.expr.result-ast | 47 ++++ .../snapshots/pass/anthony_testing.expr.roc | 4 + ..._func_and_hanging_paren.expr.formatted.roc | 2 + ...ore_func_and_hanging_paren.expr.result-ast | 29 +++ ...underscore_func_and_hanging_paren.expr.roc | 2 + .../test_syntax/tests/test_snapshots.rs | 2 + crates/repl_ui/src/lib.rs | 2 +- 24 files changed, 373 insertions(+), 162 deletions(-) create mode 100644 crates/compiler/test_syntax/tests/snapshots/pass/anthony_testing.expr.formatted.roc create mode 100644 crates/compiler/test_syntax/tests/snapshots/pass/anthony_testing.expr.result-ast create mode 100644 crates/compiler/test_syntax/tests/snapshots/pass/anthony_testing.expr.roc create mode 100644 crates/compiler/test_syntax/tests/snapshots/pass/apply_with_underscore_func_and_hanging_paren.expr.formatted.roc create mode 100644 crates/compiler/test_syntax/tests/snapshots/pass/apply_with_underscore_func_and_hanging_paren.expr.result-ast create mode 100644 crates/compiler/test_syntax/tests/snapshots/pass/apply_with_underscore_func_and_hanging_paren.expr.roc diff --git a/crates/compiler/can/src/annotation.rs b/crates/compiler/can/src/annotation.rs index 49959fb387d..d06a1f12102 100644 --- a/crates/compiler/can/src/annotation.rs +++ b/crates/compiler/can/src/annotation.rs @@ -1086,7 +1086,7 @@ fn canonicalize_has_clause( } = clause; let region = *region; - let var_name = var.extract_spaces().item; + let var_name = var.extract_spaces(env.arena).item; debug_assert!( var_name.starts_with(char::is_lowercase), "Vars should have been parsed as lowercase" @@ -1192,8 +1192,10 @@ fn can_extension_type( ); if valid_extension_type(shallow_dealias_with_scope(scope, &ext_type)) { - if matches!(loc_ann.extract_spaces().item, TypeAnnotation::Wildcard) - && matches!(ext_problem_kind, ExtensionTypeKind::TagUnion) + if matches!( + loc_ann.extract_spaces(env.arena).item, + TypeAnnotation::Wildcard + ) && matches!(ext_problem_kind, ExtensionTypeKind::TagUnion) && pol == CanPolarity::Pos { // Wildcards are redundant in positive positions, since they will always be diff --git a/crates/compiler/can/src/def.rs b/crates/compiler/can/src/def.rs index bbe2f5f0ebe..de02dbf4ace 100644 --- a/crates/compiler/can/src/def.rs +++ b/crates/compiler/can/src/def.rs @@ -543,7 +543,7 @@ fn canonicalize_claimed_ability_impl<'a>( ) -> Result<(Symbol, Symbol), ()> { let ability_home = ability.module_id(); - match loc_impl.extract_spaces().item { + match loc_impl.extract_spaces(env.arena).item { AssignedField::LabelOnly(label) => { let label_str = label.value; let region = label.region; @@ -790,7 +790,7 @@ fn canonicalize_opaque<'a>( for has_ability in has_abilities.items { let region = has_ability.region; - let (ability, opt_impls) = match has_ability.value.extract_spaces().item { + let (ability, opt_impls) = match has_ability.value.extract_spaces(env.arena).item { ast::ImplementsAbility::ImplementsAbility { ability, impls } => (ability, impls), _ => internal_error!("spaces not extracted"), }; @@ -837,7 +837,7 @@ fn canonicalize_opaque<'a>( // First up canonicalize all the claimed implementations, building a map of ability // member -> implementation. - for loc_impl in impls.extract_spaces().item.items { + for loc_impl in impls.extract_spaces(env.arena).item.items { let (member, impl_symbol) = match canonicalize_claimed_ability_impl(env, scope, ability, loc_impl) { Ok((member, impl_symbol)) => (member, impl_symbol), @@ -2958,7 +2958,7 @@ fn to_pending_type_def<'a>( for member in *members { let name_region = member.name.region; - let member_name = member.name.extract_spaces().item; + let member_name = member.name.extract_spaces(env.arena).item; let member_sym = match scope.introduce(member_name.into(), name_region) { Ok(sym) => sym, diff --git a/crates/compiler/can/src/pattern.rs b/crates/compiler/can/src/pattern.rs index e8976f24b9f..4231cf56257 100644 --- a/crates/compiler/can/src/pattern.rs +++ b/crates/compiler/can/src/pattern.rs @@ -824,7 +824,7 @@ pub fn canonicalize_record_destructs<'a>( let mut opt_erroneous = None; for loc_pattern in patterns.iter() { - match loc_pattern.value.extract_spaces().item { + match loc_pattern.value.extract_spaces(env.arena).item { Identifier { ident: label } => { match scope.introduce(label.into(), region) { Ok(symbol) => { diff --git a/crates/compiler/fmt/src/annotation.rs b/crates/compiler/fmt/src/annotation.rs index eff2c010307..d50e1aedb54 100644 --- a/crates/compiler/fmt/src/annotation.rs +++ b/crates/compiler/fmt/src/annotation.rs @@ -13,15 +13,12 @@ use bumpalo::{ collections::{String, Vec}, Bump, }; -use roc_parse::{ast::Spaced, ident::UppercaseIdent}; -use roc_parse::{ - ast::{ - AbilityImpls, AssignedField, Collection, CommentOrNewline, Expr, ExtractSpaces, - FunctionArrow, ImplementsAbilities, ImplementsAbility, ImplementsClause, Spaceable, Spaces, - SpacesAfter, SpacesBefore, Tag, TypeAnnotation, TypeHeader, - }, - expr::merge_spaces, +use roc_parse::ast::{ + merge_spaces, AbilityImpls, AssignedField, Collection, CommentOrNewline, Expr, ExtractSpaces, + FunctionArrow, ImplementsAbilities, ImplementsAbility, ImplementsClause, Spaceable, Spaces, + SpacesAfter, SpacesBefore, Tag, TypeAnnotation, TypeHeader, }; +use roc_parse::{ast::Spaced, ident::UppercaseIdent}; use roc_region::all::Loc; /// Does an AST node need parens around it? @@ -617,7 +614,7 @@ impl<'a> Formattable for ImplementsClause<'a> { } fn format_with_options(&self, buf: &mut Buf, parens: Parens, newlines: Newlines, indent: u16) { - buf.push_str(self.var.value.extract_spaces().item); + buf.push_str(self.var.value.extract_spaces(buf.bump()).item); buf.spaces(1); buf.push_str(roc_parse::keyword::IMPLEMENTS); buf.spaces(1); @@ -1445,7 +1442,7 @@ pub struct NodeSpaces<'a, T> { impl<'a, T: Copy> ExtractSpaces<'a> for NodeSpaces<'a, T> { type Item = T; - fn extract_spaces(&self) -> Spaces<'a, T> { + fn extract_spaces(&self, _arena: &'a Bump) -> Spaces<'a, T> { Spaces { before: self.before, item: self.item, diff --git a/crates/compiler/fmt/src/collection.rs b/crates/compiler/fmt/src/collection.rs index da31ea74bb8..cba444bf504 100644 --- a/crates/compiler/fmt/src/collection.rs +++ b/crates/compiler/fmt/src/collection.rs @@ -1,7 +1,4 @@ -use roc_parse::{ - ast::{Collection, CommentOrNewline, ExtractSpaces}, - expr::merge_spaces, -}; +use roc_parse::ast::{merge_spaces, Collection, CommentOrNewline, ExtractSpaces}; use crate::{ annotation::{is_collection_multiline, Formattable, Newlines}, @@ -33,7 +30,7 @@ impl Braces { } } -pub fn fmt_collection<'a, 'buf, T: ExtractSpaces<'a> + Formattable + std::fmt::Debug>( +pub fn fmt_collection<'a, 'buf: 'a, T: ExtractSpaces<'a> + Formattable + std::fmt::Debug>( buf: &mut Buf<'buf>, indent: u16, @@ -59,7 +56,7 @@ pub fn fmt_collection<'a, 'buf, T: ExtractSpaces<'a> + Formattable + std::fmt::D for (index, item) in items.iter().enumerate() { let is_first_item = index == 0; - let item = item.extract_spaces(); + let item = item.extract_spaces(buf.arena); let is_only_newlines = item.before.iter().all(|s| s.is_newline()); let last_after_was_only_newlines = last_after.iter().all(|s| s.is_newline()); diff --git a/crates/compiler/fmt/src/def.rs b/crates/compiler/fmt/src/def.rs index 00886a5c25c..21498086c42 100644 --- a/crates/compiler/fmt/src/def.rs +++ b/crates/compiler/fmt/src/def.rs @@ -17,12 +17,11 @@ use crate::{Buf, MigrationFlags}; 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, PatternApplyStyle, Spaceable, Spaces, SpacesAfter, SpacesBefore, - StrLiteral, TypeAnnotation, TypeDef, TypeHeader, ValueDef, + merge_spaces, AbilityMember, Defs, Expr, ExtractSpaces, ImportAlias, ImportAsKeyword, + ImportExposingKeyword, ImportedModuleName, IngestedFileAnnotation, IngestedFileImport, + ModuleImport, ModuleImportParams, Pattern, PatternApplyStyle, Spaceable, Spaces, SpacesAfter, + SpacesBefore, StrLiteral, TypeAnnotation, TypeDef, TypeHeader, ValueDef, }; -use roc_parse::expr::merge_spaces; use roc_parse::header::Keyword; use roc_region::all::Loc; @@ -101,7 +100,7 @@ fn lift_spaces_after<'a, 'b: 'a, T: 'b + ExtractSpaces<'a> + Spaceable<'a>>( where >::Item: Spaceable<'a>, { - let spaces = item.extract_spaces(); + let spaces = item.extract_spaces(arena); SpacesAfter { item: spaces.item.maybe_before(arena, spaces.before), @@ -431,8 +430,6 @@ impl<'a> Formattable for TypeDef<'a> { fn format_with_options(&self, buf: &mut Buf, _parens: Parens, newlines: Newlines, indent: u16) { use roc_parse::ast::TypeDef::*; - println!("WTF???"); - match self { Alias { header, ann } => { header.format(buf, indent); @@ -457,8 +454,10 @@ impl<'a> Formattable for TypeDef<'a> { typ: ann, derived: has_abilities, } => { - let ann_is_where_clause = - matches!(ann.extract_spaces().item, TypeAnnotation::Where(..)); + let ann_is_where_clause = matches!( + ann.extract_spaces(buf.arena).item, + TypeAnnotation::Where(..) + ); // Always put the has-abilities clause on a newline if the opaque annotation // contains a where-has clause. @@ -498,7 +497,7 @@ impl<'a> Formattable for TypeDef<'a> { } => { let header_lifted = type_head_lift_spaces(buf.text.bump(), *header); header_lifted.item.format(buf, indent); - let implements = loc_implements.value.extract_spaces(); + let implements = loc_implements.value.extract_spaces(buf.arena); let before_implements = merge_spaces_conservative( buf.text.bump(), header_lifted.after, @@ -535,7 +534,7 @@ impl<'a> Formattable for TypeDef<'a> { before, item, after, - } = member.name.value.extract_spaces(); + } = member.name.value.extract_spaces(buf.arena); fmt_spaces(buf, before.iter(), indent + INDENT); buf.ensure_ends_with_newline(); @@ -855,7 +854,7 @@ impl<'a> Formattable for ValueDef<'a> { use roc_parse::ast::ValueDef::*; match self { Annotation(loc_pattern, loc_annotation) => { - let pat_parens = if ann_pattern_needs_parens(&loc_pattern.value) { + let pat_parens = if ann_pattern_needs_parens(buf.arena, &loc_pattern.value) { Parens::InApply } else { Parens::NotNeeded @@ -882,7 +881,7 @@ impl<'a> Formattable for ValueDef<'a> { body_pattern, body_expr, } => { - let pat_parens = if ann_pattern_needs_parens(&ann_pattern.value) { + let pat_parens = if ann_pattern_needs_parens(buf.arena, &ann_pattern.value) { Parens::InApply } else { Parens::NotNeeded @@ -910,11 +909,11 @@ impl<'a> Formattable for ValueDef<'a> { } } -fn ann_pattern_needs_parens(value: &Pattern<'_>) -> bool { - match value.extract_spaces().item { +fn ann_pattern_needs_parens(arena: &'_ Bump, value: &Pattern<'_>) -> bool { + match value.extract_spaces(arena).item { Pattern::Tag(_) => true, Pattern::Apply(func, _args, _style) - if matches!(func.extract_spaces().item, Pattern::Tag(..)) => + if matches!(func.extract_spaces(arena).item, Pattern::Tag(..)) => { true } @@ -1041,7 +1040,7 @@ pub fn fmt_body<'a>( indent: u16, ) { - let pattern_extracted = pattern.extract_spaces(); + let pattern_extracted = pattern.extract_spaces(buf.arena); // Check if this is an assignment into the unit value let is_unit_assignment = if let Pattern::RecordDestructure(collection) = pattern_extracted.item { @@ -1049,9 +1048,9 @@ pub fn fmt_body<'a>( && collection.is_empty() && pattern_extracted.before.iter().all(|s| s.is_newline()) && pattern_extracted.after.iter().all(|s| s.is_newline()) - && !matches!(body.extract_spaces().item, Expr::Defs(..)) - && !matches!(body.extract_spaces().item, Expr::Return(..)) - && !matches!(body.extract_spaces().item, Expr::DbgStmt { .. }) + && !matches!(body.extract_spaces(buf.arena).item, Expr::Defs(..)) + && !matches!(body.extract_spaces(buf.arena).item, Expr::Return(..)) + && !matches!(body.extract_spaces(buf.arena).item, Expr::DbgStmt { .. }) && !starts_with_expect_ident(body) } else { false @@ -1201,7 +1200,7 @@ impl<'a> Formattable for AbilityMember<'a> { _newlines: Newlines, indent: u16, ) { - let Spaces { before, item, .. } = self.name.value.extract_spaces(); + let Spaces { before, item, .. } = self.name.value.extract_spaces(buf.arena); fmt_spaces(buf, before.iter(), indent); buf.indent(indent); diff --git a/crates/compiler/fmt/src/expr.rs b/crates/compiler/fmt/src/expr.rs index 1dd9427f08a..0fb170867ba 100644 --- a/crates/compiler/fmt/src/expr.rs +++ b/crates/compiler/fmt/src/expr.rs @@ -12,12 +12,11 @@ use crate::Buf; use bumpalo::collections::Vec; use bumpalo::Bump; use roc_module::called_via::{self, BinOp, CalledVia, UnaryOp}; +use roc_parse::ast::{merge_spaces, StrLiteral, StrSegment}; use roc_parse::ast::{ AssignedField, Base, Collection, CommentOrNewline, Expr, ExtractSpaces, Pattern, Spaceable, Spaces, SpacesAfter, SpacesBefore, TryTarget, WhenBranch, }; -use roc_parse::ast::{StrLiteral, StrSegment}; -use roc_parse::expr::merge_spaces; use roc_parse::ident::Accessor; use roc_parse::keyword; use roc_region::all::Loc; @@ -665,28 +664,30 @@ fn fmt_apply( // 2, // ] // ``` + let arena = buf.bump(); let use_commas_and_parens = expr_used_commas_and_parens || buf.flags().parens_and_commas; - let should_reflow_outdentable = loc_expr.extract_spaces().after.is_empty() + let should_reflow_outdentable = loc_expr.extract_spaces(arena).after.is_empty() && except_last(loc_args).all(|a| !a.is_multiline()) && loc_args .last() .map(|a| { - a.extract_spaces().item.is_multiline() - && is_outdentable_collection(&a.value.extract_spaces().item) - && (a.extract_spaces().before == [CommentOrNewline::Newline] - || a.extract_spaces().before.is_empty()) + a.extract_spaces(arena).item.is_multiline() + && is_outdentable_collection(&a.value.extract_spaces(arena).item) + && (a.extract_spaces(arena).before == [CommentOrNewline::Newline] + || a.extract_spaces(arena).before.is_empty()) }) .unwrap_or_default(); let needs_indent = !should_reflow_outdentable - && (!loc_expr.extract_spaces().after.is_empty() + && (!loc_expr.extract_spaces(arena).after.is_empty() || except_last(loc_args).any(|a| a.is_multiline()) || loc_expr.is_multiline() || loc_args .last() .map(|a| { a.is_multiline() - && (!a.extract_spaces().before.is_empty() || !is_outdentable(&a.value)) + && (!a.extract_spaces(arena).before.is_empty() + || !is_outdentable(arena, &a.value)) }) .unwrap_or_default()); @@ -780,6 +781,9 @@ fn fmt_apply( buf.ensure_ends_with_newline(); buf.indent(indent); } + if buf.ends_with_newline() { + buf.indent(indent); + } buf.push(')'); } } @@ -876,9 +880,9 @@ pub(crate) fn format_sq_literal(buf: &mut Buf, s: &str) { buf.push('\''); } -fn is_outdentable(expr: &Expr) -> bool { +fn is_outdentable(arena: &'_ Bump, expr: &Expr) -> bool { matches!( - expr.extract_spaces().item, + expr.extract_spaces(arena).item, Expr::Tuple(_) | Expr::List(_) | Expr::Record(_) | Expr::Closure(..) ) } @@ -1561,15 +1565,15 @@ pub fn format_spaces(buf: &mut Buf, spaces: &[CommentOrNewline], newlines: Newli } } -fn is_when_patterns_multiline(when_branch: &WhenBranch) -> bool { +fn is_when_patterns_multiline(arena: &'_ Bump, when_branch: &WhenBranch) -> bool { let patterns = when_branch.patterns; let (first_pattern, rest) = patterns.split_first().unwrap(); let is_multiline_patterns = if let Some((last_pattern, inner_patterns)) = rest.split_last() { - !first_pattern.value.extract_spaces().after.is_empty() - || !last_pattern.value.extract_spaces().before.is_empty() + !first_pattern.value.extract_spaces(arena).after.is_empty() + || !last_pattern.value.extract_spaces(arena).before.is_empty() || inner_patterns.iter().any(|p| { - let spaces = p.value.extract_spaces(); + let spaces = p.value.extract_spaces(arena); !spaces.before.is_empty() || !spaces.after.is_empty() }) } else { @@ -1631,7 +1635,7 @@ fn fmt_when<'a>( let expr = &branch.value; let patterns = &branch.patterns; let is_multiline_expr = expr.is_multiline(); - let is_multiline_patterns = is_when_patterns_multiline(branch); + let is_multiline_patterns = is_when_patterns_multiline(buf.bump(), branch); for (pattern_index, pattern) in patterns.iter().enumerate() { if pattern_index == 0 { diff --git a/crates/compiler/fmt/src/lib.rs b/crates/compiler/fmt/src/lib.rs index 9b63c03610e..65c2f7756c0 100644 --- a/crates/compiler/fmt/src/lib.rs +++ b/crates/compiler/fmt/src/lib.rs @@ -21,6 +21,7 @@ pub struct Buf<'a> { beginning_of_line: bool, line_indent: u16, flags: MigrationFlags, + pub arena: &'a Bump, } #[derive(Debug, Copy, Clone)] @@ -44,6 +45,7 @@ impl<'a> Buf<'a> { newlines_to_flush: 0, beginning_of_line: true, flags, + arena, } } @@ -59,6 +61,10 @@ impl<'a> Buf<'a> { self.text.into_bump_str() } + pub fn bump(&'a self) -> &'a Bump { + self.text.bump() + } + pub fn indent(&mut self, indent: u16) { if self.beginning_of_line { self.line_indent = indent; diff --git a/crates/compiler/fmt/src/pattern.rs b/crates/compiler/fmt/src/pattern.rs index b256e47f918..c3fb65818f1 100644 --- a/crates/compiler/fmt/src/pattern.rs +++ b/crates/compiler/fmt/src/pattern.rs @@ -7,10 +7,9 @@ use crate::spaces::{fmt_comments_only, fmt_spaces, NewlineAt, INDENT}; use crate::Buf; use bumpalo::Bump; use roc_parse::ast::{ - Base, CommentOrNewline, Pattern, PatternApplyStyle, PatternAs, Spaceable, Spaces, SpacesAfter, - SpacesBefore, + merge_spaces, Base, CommentOrNewline, Pattern, PatternApplyStyle, PatternAs, Spaceable, Spaces, + SpacesAfter, SpacesBefore, }; -use roc_parse::expr::merge_spaces; use roc_region::all::Loc; pub fn fmt_pattern<'a>(buf: &mut Buf, pattern: &'a Pattern<'a>, indent: u16, parens: Parens) { diff --git a/crates/compiler/load_internal/src/docs.rs b/crates/compiler/load_internal/src/docs.rs index 56e9e7a7c04..7479a387cd1 100644 --- a/crates/compiler/load_internal/src/docs.rs +++ b/crates/compiler/load_internal/src/docs.rs @@ -1,5 +1,6 @@ use crate::docs::DocEntry::DetachedDoc; use crate::docs::TypeAnnotation::{Apply, BoundVariable, Function, NoTypeAnn, Record, TagUnion}; +use bumpalo::Bump; use roc_can::scope::Scope; use roc_collections::VecSet; use roc_module::ident::ModuleName; @@ -124,6 +125,7 @@ pub struct Tag { #[allow(clippy::too_many_arguments)] pub fn generate_module_docs( + arena: &'_ Bump, scope: Scope, home: ModuleId, module_ids: &ModuleIds, @@ -134,6 +136,7 @@ pub fn generate_module_docs( header_comments: &[CommentOrNewline<'_>], ) -> ModuleDocumentation { let entries = generate_entry_docs( + arena, home, &scope.locals.ident_ids, module_ids, @@ -178,6 +181,7 @@ fn detached_docs_from_comments_and_new_lines<'a>( } fn generate_entry_docs( + arena: &'_ Bump, home: ModuleId, ident_ids: &IdentIds, module_ids: &ModuleIds, @@ -378,9 +382,9 @@ fn generate_entry_docs( let members = members .iter() .map(|mem| { - let extracted = mem.name.value.extract_spaces(); + let extracted = mem.name.value.extract_spaces(arena); let (type_annotation, able_variables) = - ability_member_type_to_docs(mem.typ.value); + ability_member_type_to_docs(arena, mem.typ.value); AbilityMember { name: extracted.item.to_string(), @@ -679,6 +683,7 @@ fn type_to_docs(in_func_type_ann: bool, type_annotation: ast::TypeAnnotation) -> } fn ability_member_type_to_docs( + arena: &'_ Bump, type_annotation: ast::TypeAnnotation, ) -> (TypeAnnotation, Vec<(String, Vec)>) { match type_annotation { @@ -689,7 +694,7 @@ fn ability_member_type_to_docs( .map(|hc| { let ast::ImplementsClause { var, abilities } = hc.value; ( - var.value.extract_spaces().item.to_string(), + var.value.extract_spaces(arena).item.to_string(), abilities .iter() .map(|ability| type_to_docs(false, ability.value)) diff --git a/crates/compiler/load_internal/src/file.rs b/crates/compiler/load_internal/src/file.rs index 51706d12b57..0b51acce77c 100644 --- a/crates/compiler/load_internal/src/file.rs +++ b/crates/compiler/load_internal/src/file.rs @@ -5003,7 +5003,7 @@ fn unspace<'a, T: Copy>(arena: &'a Bump, items: &[Loc>]) -> &'a [L bumpalo::collections::Vec::from_iter_in( items .iter() - .map(|item| Loc::at(item.region, item.value.extract_spaces().item)), + .map(|item| Loc::at(item.region, item.value.extract_spaces(arena).item)), arena, ) .into_bump_slice() @@ -5070,7 +5070,7 @@ fn build_platform_header<'a>( .item .signatures .map_items(arena, |item| { - Loc::at(item.region, item.extract_spaces().item) + Loc::at(item.region, item.extract_spaces(arena).item) }) .items; let provides = bumpalo::collections::Vec::from_iter_in( @@ -5201,6 +5201,7 @@ fn canonicalize_and_constrain<'a>( let mut scope = module_output.scope.clone(); scope.add_docs_imports(); crate::docs::generate_module_docs( + arena, scope, module_id, arena.alloc(qualified_module_ids.clone().into_module_ids()), diff --git a/crates/compiler/parse/src/ast.rs b/crates/compiler/parse/src/ast.rs index dfac5b9903d..aa5b5b1f27a 100644 --- a/crates/compiler/parse/src/ast.rs +++ b/crates/compiler/parse/src/ast.rs @@ -1,6 +1,5 @@ use std::fmt::Debug; -use crate::expr::merge_spaces; use crate::header::{ self, AppHeader, HostedHeader, ModuleHeader, ModuleName, PackageHeader, PlatformHeader, }; @@ -31,7 +30,7 @@ pub struct Spaces<'a, T> { impl<'a, T: Copy> ExtractSpaces<'a> for Spaces<'a, T> { type Item = T; - fn extract_spaces(&self) -> Spaces<'a, T> { + fn extract_spaces(&self, _arena: &'a Bump) -> Spaces<'a, T> { *self } @@ -50,6 +49,23 @@ impl<'a, T> Spaces<'a, T> { } } +pub fn merge_spaces<'a>( + arena: &'a Bump, + a: &'a [CommentOrNewline<'a>], + b: &'a [CommentOrNewline<'a>], +) -> &'a [CommentOrNewline<'a>] { + if a.is_empty() { + b + } else if b.is_empty() { + a + } else { + let mut merged = Vec::with_capacity_in(a.len() + b.len(), arena); + merged.extend_from_slice(a); + merged.extend_from_slice(b); + merged.into_bump_slice() + } +} + #[derive(Debug, Copy, Clone, PartialEq, Eq)] pub struct SpacesBefore<'a, T> { pub before: &'a [CommentOrNewline<'a>], @@ -124,14 +140,14 @@ 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 extract_spaces(&self, arena: &'a Bump) -> 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 extract_spaces(&self, arena: &'a Bump) -> Spaces<'a, Self::Item> { + (*self).extract_spaces(arena) } fn without_spaces(&self) -> Self::Item { @@ -141,8 +157,8 @@ impl<'a, T: ExtractSpaces<'a>> ExtractSpaces<'a> for &'a T { impl<'a, T: ExtractSpaces<'a>> ExtractSpaces<'a> for Loc { type Item = T::Item; - fn extract_spaces(&self) -> Spaces<'a, Self::Item> { - let spaces = self.value.extract_spaces(); + fn extract_spaces(&self, arena: &'a Bump) -> Spaces<'a, Self::Item> { + let spaces = self.value.extract_spaces(arena); Spaces { before: spaces.before, item: spaces.item, @@ -190,7 +206,7 @@ impl<'a> Header<'a> { let len = imports.item.len(); for (index, import) in imports.item.iter().enumerate() { - let spaced = import.extract_spaces(); + let spaced = import.extract_spaces(arena); let value_def = match spaced.item { header::ImportsEntry::Package(pkg_name, name, exposed) => { @@ -205,7 +221,7 @@ impl<'a> Header<'a> { Self::header_import_to_value_def(None, name, exposed, import.region) } header::ImportsEntry::IngestedFile(path, typed_ident) => { - let typed_ident = typed_ident.extract_spaces(); + let typed_ident = typed_ident.extract_spaces(arena); ValueDef::IngestedFileImport(IngestedFileImport { before_path: &[], @@ -2326,15 +2342,31 @@ macro_rules! impl_extract_spaces { impl<'a, $($($generic_args: Copy),*)?> ExtractSpaces<'a> for $t<'a, $($($generic_args),*)?> { type Item = Self; - fn extract_spaces(&self) -> Spaces<'a, Self::Item> { + fn extract_spaces(&self, arena: &'a Bump) -> Spaces<'a, Self::Item> { match self { $t::SpaceBefore(item, before) => { match item { - $t::SpaceBefore(_, _) => todo!(), - $t::SpaceAfter(item, after) => { + sb @ $t::SpaceBefore(_, _) => { + let Spaces { + before: before_inner, + item, + after, + } = sb.extract_spaces(arena); Spaces { - before, - item: **item, + before: merge_spaces(arena, before, before_inner), + item, + after, + } + } + sb @ $t::SpaceAfter(_, _) => { + let Spaces { + before: before_inner, + item, + after, + } = sb.extract_spaces(arena); + Spaces { + before: merge_spaces(arena, before, before_inner), + item, after, } } @@ -2349,14 +2381,30 @@ macro_rules! impl_extract_spaces { }, $t::SpaceAfter(item, after) => { match item { - $t::SpaceBefore(item, before) => { + sb @ $t::SpaceBefore(_, _) => { + let Spaces { + before, + item, + after: after_inner, + } = sb.extract_spaces(arena); Spaces { before, - item: **item, - after, + item, + after: merge_spaces(arena, after_inner, after), + } + } + sb @ $t::SpaceAfter(_, _) => { + let Spaces { + before, + item, + after: after_inner, + } = sb.extract_spaces(arena); + Spaces { + before, + item, + after: merge_spaces(arena, after_inner, after), } } - $t::SpaceAfter(_, _) => todo!(), _ => { Spaces { before: &[], @@ -2402,19 +2450,31 @@ impl_extract_spaces!(Implements); impl<'a, T: Copy> ExtractSpaces<'a> for Spaced<'a, T> { type Item = T; - fn extract_spaces(&self) -> Spaces<'a, T> { + fn extract_spaces(&self, arena: &'a Bump) -> Spaces<'a, T> { match self { Spaced::SpaceBefore(item, before) => match item { - Spaced::SpaceBefore(_, _) => todo!(), - Spaced::SpaceAfter(item, after) => { - if let Spaced::Item(item) = item { - Spaces { - before, - item: *item, - after, - } - } else { - todo!(); + sb @ Spaced::SpaceBefore(_, _) => { + let Spaces { + before: before_inner, + item, + after, + } = sb.extract_spaces(arena); + Spaces { + before: merge_spaces(arena, before, before_inner), + item, + after, + } + } + sb @ Spaced::SpaceAfter(_, _) => { + let Spaces { + before: before_inner, + item, + after, + } = sb.extract_spaces(arena); + Spaces { + before: merge_spaces(arena, before, before_inner), + item, + after, } } Spaced::Item(item) => Spaces { @@ -2424,18 +2484,30 @@ impl<'a, T: Copy> ExtractSpaces<'a> for Spaced<'a, T> { }, }, Spaced::SpaceAfter(item, after) => match item { - Spaced::SpaceBefore(item, before) => { - if let Spaced::Item(item) = item { - Spaces { - before, - item: *item, - after, - } - } else { - todo!(); + sb @ Spaced::SpaceBefore(_, _) => { + let Spaces { + before, + item, + after: after_inner, + } = sb.extract_spaces(arena); + Spaces { + before, + item, + after: merge_spaces(arena, after_inner, after), + } + } + sb @ Spaced::SpaceAfter(_, _) => { + let Spaces { + before, + item, + after: after_inner, + } = sb.extract_spaces(arena); + Spaces { + before, + item, + after: merge_spaces(arena, after_inner, after), } } - Spaced::SpaceAfter(_, _) => todo!(), Spaced::Item(item) => Spaces { before: &[], item: *item, @@ -2462,7 +2534,7 @@ impl<'a, T: Copy> ExtractSpaces<'a> for Spaced<'a, T> { impl<'a> ExtractSpaces<'a> for AbilityImpls<'a> { type Item = Collection<'a, Loc>>>; - fn extract_spaces(&self) -> Spaces<'a, Self::Item> { + fn extract_spaces(&self, arena: &'a Bump) -> Spaces<'a, Self::Item> { match self { AbilityImpls::AbilityImpls(inner) => Spaces { before: &[], @@ -2475,13 +2547,35 @@ impl<'a> ExtractSpaces<'a> for AbilityImpls<'a> { item: *inner, after: &[], }, - AbilityImpls::SpaceBefore(_, _) => todo!(), AbilityImpls::SpaceAfter(AbilityImpls::AbilityImpls(inner), after) => Spaces { before, item: *inner, after, }, - AbilityImpls::SpaceAfter(_, _) => todo!(), + sb @ AbilityImpls::SpaceBefore(_, _) => { + let Spaces { + before: before_inner, + item, + after, + } = sb.extract_spaces(arena); + Spaces { + before: merge_spaces(arena, before, before_inner), + item, + after, + } + } + sb @ AbilityImpls::SpaceAfter(_, _) => { + let Spaces { + before: before_inner, + item, + after, + } = sb.extract_spaces(arena); + Spaces { + before: merge_spaces(arena, before, before_inner), + item, + after, + } + } }, AbilityImpls::SpaceAfter(item, after) => match item { AbilityImpls::AbilityImpls(inner) => Spaces { @@ -2494,8 +2588,30 @@ impl<'a> ExtractSpaces<'a> for AbilityImpls<'a> { item: *inner, after, }, - AbilityImpls::SpaceBefore(_, _) => todo!(), - AbilityImpls::SpaceAfter(_, _) => todo!(), + sb @ AbilityImpls::SpaceBefore(_, _) => { + let Spaces { + before, + item, + after: after_inner, + } = sb.extract_spaces(arena); + Spaces { + before, + item, + after: merge_spaces(arena, after_inner, after), + } + } + sb @ AbilityImpls::SpaceAfter(_, _) => { + let Spaces { + before, + item, + after: after_inner, + } = sb.extract_spaces(arena); + Spaces { + before, + item, + after: merge_spaces(arena, after_inner, after), + } + } }, } } diff --git a/crates/compiler/parse/src/expr.rs b/crates/compiler/parse/src/expr.rs index 715520f39b8..cd803bdef9b 100644 --- a/crates/compiler/parse/src/expr.rs +++ b/crates/compiler/parse/src/expr.rs @@ -1,9 +1,9 @@ use crate::ast::{ - is_expr_suffixed, AssignedField, Collection, CommentOrNewline, Defs, Expr, ExtractSpaces, - Implements, ImplementsAbilities, ImportAlias, ImportAsKeyword, ImportExposingKeyword, - ImportedModuleName, IngestedFileAnnotation, IngestedFileImport, ModuleImport, - ModuleImportParams, Pattern, PatternApplyStyle, Spaceable, Spaced, Spaces, SpacesBefore, - TryTarget, TypeAnnotation, TypeDef, TypeHeader, ValueDef, + is_expr_suffixed, merge_spaces, AssignedField, Collection, CommentOrNewline, Defs, Expr, + ExtractSpaces, Implements, ImplementsAbilities, ImportAlias, ImportAsKeyword, + ImportExposingKeyword, ImportedModuleName, IngestedFileAnnotation, IngestedFileImport, + ModuleImport, ModuleImportParams, Pattern, PatternApplyStyle, Spaceable, Spaced, Spaces, + SpacesBefore, TryTarget, TypeAnnotation, TypeDef, TypeHeader, ValueDef, }; use crate::blankspace::{ loc_space0_e, require_newline_or_eof, space0_after_e, space0_around_ee, space0_before_e, @@ -895,7 +895,7 @@ fn to_call<'a>( let region = Region::span_across(&loc_expr1.region, &last); let spaces = if let Some(last) = arguments.last_mut() { - let spaces = last.value.extract_spaces(); + let spaces = last.value.extract_spaces(arena); if spaces.after.is_empty() { &[] @@ -1182,12 +1182,12 @@ enum AliasOrOpaque { } fn extract_tag_and_spaces<'a>(arena: &'a Bump, expr: Expr<'a>) -> Option> { - let mut expr = expr.extract_spaces(); + let mut expr = expr.extract_spaces(arena); loop { match &expr.item { Expr::ParensAround(inner_expr) => { - let inner_expr = inner_expr.extract_spaces(); + let inner_expr = inner_expr.extract_spaces(arena); 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); @@ -2062,30 +2062,13 @@ pub fn loc_expr<'a>(allow_any_indent: bool) -> impl Parser<'a, Loc>, EE ) } -pub fn merge_spaces<'a>( - arena: &'a Bump, - a: &'a [CommentOrNewline<'a>], - b: &'a [CommentOrNewline<'a>], -) -> &'a [CommentOrNewline<'a>] { - if a.is_empty() { - b - } else if b.is_empty() { - a - } else { - let mut merged = Vec::with_capacity_in(a.len() + b.len(), arena); - merged.extend_from_slice(a); - merged.extend_from_slice(b); - merged.into_bump_slice() - } -} - /// If the given Expr would parse the same way as a valid Pattern, convert it. /// Example: (foo) could be either an Expr::Var("foo") or Pattern::Identifier("foo") fn expr_to_pattern_help<'a>(arena: &'a Bump, expr: &Expr<'a>) -> Result, ()> { - let mut expr = expr.extract_spaces(); + let mut expr = expr.extract_spaces(arena); while let Expr::ParensAround(loc_expr) = &expr.item { - let expr_inner = loc_expr.extract_spaces(); + let expr_inner = loc_expr.extract_spaces(arena); expr.before = merge_spaces(arena, expr.before, expr_inner.before); expr.after = merge_spaces(arena, expr_inner.after, expr.after); @@ -2132,7 +2115,7 @@ fn expr_to_pattern_help<'a>(arena: &'a Bump, expr: &Expr<'a>) -> Result Pattern::Identifier { ident: "try" }, - Expr::SpaceBefore(..) | Expr::SpaceAfter(..) | Expr::ParensAround(..) => unreachable!(), + Expr::SpaceBefore(..) | Expr::SpaceAfter(..) | Expr::ParensAround(..) => return Err(()), Expr::Record(fields) => { let patterns = fields.map_items_result(arena, |loc_assigned_field| { @@ -3110,13 +3093,20 @@ fn stmts_to_defs<'a>( value: Expr::Dbg, .. }, args, - _, + called_via, ) = e { let condition = &args[0]; let rest = stmts_to_expr(&stmts[i + 1..], arena)?; let e = Expr::DbgStmt { - first: condition, + first: if called_via == CalledVia::ParensAndCommas { + arena.alloc(Loc::at( + condition.region, + Expr::ParensAround(&condition.value), + )) + } else { + condition + }, extra_args: &args[1..], continuation: arena.alloc(rest), }; diff --git a/crates/compiler/parse/src/header.rs b/crates/compiler/parse/src/header.rs index 4f602a164e1..d516fd95ddb 100644 --- a/crates/compiler/parse/src/header.rs +++ b/crates/compiler/parse/src/header.rs @@ -1,11 +1,10 @@ use std::fmt::Debug; use crate::ast::{ - Collection, CommentOrNewline, Defs, Header, Malformed, Pattern, Spaced, Spaces, SpacesBefore, - StrLiteral, TypeAnnotation, + merge_spaces, Collection, CommentOrNewline, Defs, Header, Malformed, Pattern, Spaced, Spaces, + SpacesBefore, StrLiteral, TypeAnnotation, }; use crate::blankspace::{space0_before_e, space0_e}; -use crate::expr::merge_spaces; use crate::ident::{self, lowercase_ident, unqualified_ident, UppercaseIdent}; use crate::parser::Progress::{self, *}; use crate::parser::{ diff --git a/crates/compiler/test_syntax/fuzz/fuzz_targets/fuzz_expr.rs b/crates/compiler/test_syntax/fuzz/fuzz_targets/fuzz_expr.rs index 25f948f832b..ac5c223defd 100644 --- a/crates/compiler/test_syntax/fuzz/fuzz_targets/fuzz_expr.rs +++ b/crates/compiler/test_syntax/fuzz/fuzz_targets/fuzz_expr.rs @@ -5,14 +5,16 @@ use roc_parse::ast::Malformed; use test_syntax::test_helpers::Input; fuzz_target!(|data: &[u8]| { - if let Ok(input) = std::str::from_utf8(data) { - let input = Input::Expr(input); - let arena = Bump::new(); - let ast = input.parse_in(&arena); - if let Ok(ast) = ast { - if !ast.is_malformed() { - input.check_invariants(|_| (), true, None); + let _ = std::panic::catch_unwind(|| { + if let Ok(input) = std::str::from_utf8(data) { + let input = Input::Expr(input); + let arena = Bump::new(); + let ast = input.parse_in(&arena); + if let Ok(ast) = ast { + if !ast.is_malformed() { + input.check_invariants(|_| (), true, None); + } } } - } + }); }); diff --git a/crates/compiler/test_syntax/fuzz/fuzz_targets/fuzz_module.rs b/crates/compiler/test_syntax/fuzz/fuzz_targets/fuzz_module.rs index 00011040929..9768aa64e48 100644 --- a/crates/compiler/test_syntax/fuzz/fuzz_targets/fuzz_module.rs +++ b/crates/compiler/test_syntax/fuzz/fuzz_targets/fuzz_module.rs @@ -4,11 +4,13 @@ use libfuzzer_sys::fuzz_target; use test_syntax::test_helpers::Input; fuzz_target!(|data: &[u8]| { - if let Ok(input) = std::str::from_utf8(data) { - let input = Input::Full(input); - let arena = Bump::new(); - if input.parse_in(&arena).is_ok() { - input.check_invariants(|_| (), true, None); + let _ = std::panic::catch_unwind(move || { + if let Ok(input) = std::str::from_utf8(data) { + let input = Input::Full(input); + let arena = Bump::new(); + if input.parse_in(&arena).is_ok() { + input.check_invariants(|_| (), true, None); + } } - } + }); }); diff --git a/crates/compiler/test_syntax/tests/snapshots/pass/anthony_testing.expr.formatted.roc b/crates/compiler/test_syntax/tests/snapshots/pass/anthony_testing.expr.formatted.roc new file mode 100644 index 00000000000..a0371b6a3fa --- /dev/null +++ b/crates/compiler/test_syntax/tests/snapshots/pass/anthony_testing.expr.formatted.roc @@ -0,0 +1,6 @@ +dbg + ( + 2ii / fi &fh + ) + +i \ No newline at end of file diff --git a/crates/compiler/test_syntax/tests/snapshots/pass/anthony_testing.expr.result-ast b/crates/compiler/test_syntax/tests/snapshots/pass/anthony_testing.expr.result-ast new file mode 100644 index 00000000000..5da15742051 --- /dev/null +++ b/crates/compiler/test_syntax/tests/snapshots/pass/anthony_testing.expr.result-ast @@ -0,0 +1,47 @@ +@0-15 SpaceAfter( + DbgStmt { + first: @4-13 ParensAround( + SpaceAfter( + BinOps( + [ + ( + @4-7 Num( + "2ii", + ), + @7-8 Slash, + ), + ], + @8-13 Apply( + @8-10 Var { + module_name: "", + ident: "fi", + }, + [ + @10-13 RecordUpdater( + "fh", + ), + ], + Space, + ), + ), + [ + Newline, + ], + ), + ), + extra_args: [], + continuation: @17-18 SpaceBefore( + Var { + module_name: "", + ident: "i", + }, + [ + Newline, + Newline, + ], + ), + }, + [ + Newline, + ], +) diff --git a/crates/compiler/test_syntax/tests/snapshots/pass/anthony_testing.expr.roc b/crates/compiler/test_syntax/tests/snapshots/pass/anthony_testing.expr.roc new file mode 100644 index 00000000000..60584ba0834 --- /dev/null +++ b/crates/compiler/test_syntax/tests/snapshots/pass/anthony_testing.expr.roc @@ -0,0 +1,4 @@ +dbg(2ii/fi&fh +) + +i diff --git a/crates/compiler/test_syntax/tests/snapshots/pass/apply_with_underscore_func_and_hanging_paren.expr.formatted.roc b/crates/compiler/test_syntax/tests/snapshots/pass/apply_with_underscore_func_and_hanging_paren.expr.formatted.roc new file mode 100644 index 00000000000..f908e7a0eed --- /dev/null +++ b/crates/compiler/test_syntax/tests/snapshots/pass/apply_with_underscore_func_and_hanging_paren.expr.formatted.roc @@ -0,0 +1,2 @@ +_((_, o) +) \ No newline at end of file diff --git a/crates/compiler/test_syntax/tests/snapshots/pass/apply_with_underscore_func_and_hanging_paren.expr.result-ast b/crates/compiler/test_syntax/tests/snapshots/pass/apply_with_underscore_func_and_hanging_paren.expr.result-ast new file mode 100644 index 00000000000..c1bcc99c44c --- /dev/null +++ b/crates/compiler/test_syntax/tests/snapshots/pass/apply_with_underscore_func_and_hanging_paren.expr.result-ast @@ -0,0 +1,29 @@ +@0-9 SpaceAfter( + Apply( + @0-1 Underscore( + "", + ), + [ + @2-7 SpaceAfter( + Tuple( + [ + @3-4 Underscore( + "", + ), + @5-6 Var { + module_name: "", + ident: "o", + }, + ], + ), + [ + Newline, + ], + ), + ], + ParensAndCommas, + ), + [ + Newline, + ], +) diff --git a/crates/compiler/test_syntax/tests/snapshots/pass/apply_with_underscore_func_and_hanging_paren.expr.roc b/crates/compiler/test_syntax/tests/snapshots/pass/apply_with_underscore_func_and_hanging_paren.expr.roc new file mode 100644 index 00000000000..90cd1b41a02 --- /dev/null +++ b/crates/compiler/test_syntax/tests/snapshots/pass/apply_with_underscore_func_and_hanging_paren.expr.roc @@ -0,0 +1,2 @@ +_((_,o) +) diff --git a/crates/compiler/test_syntax/tests/test_snapshots.rs b/crates/compiler/test_syntax/tests/test_snapshots.rs index 9984ccad1e0..cd0d3681f8e 100644 --- a/crates/compiler/test_syntax/tests/test_snapshots.rs +++ b/crates/compiler/test_syntax/tests/test_snapshots.rs @@ -318,6 +318,7 @@ mod test_snapshots { pass/annotation_tuple_newline.expr, pass/annotation_tuple_parens_newlines.expr, pass/annotation_tuples_ext_galore.expr, + pass/anthony_testing.expr, pass/applies_in_binop.expr, pass/apply_bang_bang_closure.expr, pass/apply_binop_switch.expr, @@ -335,6 +336,7 @@ mod test_snapshots { pass/apply_two_args_pnc.expr, pass/apply_unary_negation.expr, pass/apply_unary_not.expr, + pass/apply_with_underscore_func_and_hanging_paren.expr, pass/arg_pattern_as.expr, pass/as_in_func_type_args.expr, pass/bang_newline_double_accessor.expr, diff --git a/crates/repl_ui/src/lib.rs b/crates/repl_ui/src/lib.rs index 105b0e78db4..0bc5ee3e999 100644 --- a/crates/repl_ui/src/lib.rs +++ b/crates/repl_ui/src/lib.rs @@ -76,7 +76,7 @@ pub fn is_incomplete(input: &str) -> bool { } } ParseOutcome::DefsAndExpr(_, Some(expr)) => { - if matches!(expr.extract_spaces().item, Expr::When(..)) { + if matches!(expr.extract_spaces(&arena).item, Expr::When(..)) { // There might be lots of `when` branches, so don't assume the user is done entering // them until they enter a blank line! !input.ends_with('\n')