Skip to content

Commit 5d0b700

Browse files
lukewilliamboswelllukewilliamboswell
authored
Merge pull request #7493 from joshuawarner32/pnc-fuzzing-fixes
Fix some bugs in PNC formatting found via fuzzing
2 parents a69a326 + 9f395e0 commit 5d0b700

17 files changed

+285
-84
lines changed

‎crates/compiler/fmt/src/expr.rs

+24-19
Original file line numberDiff line numberDiff line change
@@ -91,14 +91,6 @@ fn format_expr_only(
9191
buf.indent(indent);
9292
buf.push_str("try");
9393
}
94-
Expr::PncApply(
95-
loc_expr @ Loc {
96-
value: Expr::Dbg, ..
97-
},
98-
loc_args,
99-
) => {
100-
fmt_apply(loc_expr, loc_args.items, indent, buf);
101-
}
10294
Expr::PncApply(loc_expr, loc_args) => {
10395
fmt_pnc_apply(loc_expr, loc_args, indent, buf);
10496
}
@@ -521,10 +513,7 @@ pub fn expr_is_multiline(me: &Expr<'_>, comments_only: bool) -> bool {
521513
.any(|loc_arg| expr_is_multiline(&loc_arg.value, comments_only))
522514
}
523515
Expr::PncApply(loc_expr, args) => {
524-
expr_is_multiline(&loc_expr.value, comments_only)
525-
|| args
526-
.iter()
527-
.any(|loc_arg| expr_is_multiline(&loc_arg.value, comments_only))
516+
expr_is_multiline(&loc_expr.value, comments_only) || is_collection_multiline(args)
528517
}
529518

530519
Expr::DbgStmt { .. } => true,
@@ -1343,9 +1332,16 @@ pub fn expr_lift_spaces<'a, 'b: 'a>(
13431332

13441333
let right_lifted = expr_lift_spaces_after(Parens::InOperator, arena, &right.value);
13451334

1335+
let mut item =
1336+
Expr::BinOps(lefts, arena.alloc(Loc::at(right.region, right_lifted.item)));
1337+
1338+
if parens == Parens::InApply || parens == Parens::InApplyLastArg {
1339+
item = Expr::ParensAround(arena.alloc(item));
1340+
}
1341+
13461342
Spaces {
13471343
before,
1348-
item: Expr::BinOps(lefts, arena.alloc(Loc::at(right.region, right_lifted.item))),
1344+
item,
13491345
after: right_lifted.after,
13501346
}
13511347
}
@@ -1759,12 +1755,21 @@ fn fmt_dbg_stmt<'a>(
17591755
args.push(condition);
17601756
args.extend_from_slice(extra_args);
17611757

1762-
Expr::Apply(
1763-
&Loc::at_zero(Expr::Dbg),
1764-
args.into_bump_slice(),
1765-
called_via::CalledVia::Space,
1766-
)
1767-
.format_with_options(buf, parens, Newlines::Yes, indent);
1758+
if args.is_empty() {
1759+
Expr::PncApply(&Loc::at_zero(Expr::Dbg), Collection::empty()).format_with_options(
1760+
buf,
1761+
parens,
1762+
Newlines::Yes,
1763+
indent,
1764+
);
1765+
} else {
1766+
Expr::Apply(
1767+
&Loc::at_zero(Expr::Dbg),
1768+
args.into_bump_slice(),
1769+
called_via::CalledVia::Space,
1770+
)
1771+
.format_with_options(buf, parens, Newlines::Yes, indent);
1772+
}
17681773

17691774
let cont_lifted = expr_lift_spaces(Parens::NotNeeded, buf.text.bump(), &continuation.value);
17701775

‎crates/compiler/fmt/src/pattern.rs

+1-1
Original file line numberDiff line numberDiff line change
@@ -722,7 +722,7 @@ pub fn pattern_lift_spaces<'a, 'b: 'a>(
722722

723723
Spaces {
724724
before: func_lifted.before,
725-
item: Pattern::PncApply(arena.alloc(func), *args),
725+
item: Pattern::PncApply(arena.alloc(Loc::at_zero(func_lifted.item)), *args),
726726
after: &[],
727727
}
728728
}

‎crates/compiler/parse/src/expr.rs

+47-39
Original file line numberDiff line numberDiff line change
@@ -242,15 +242,7 @@ fn loc_term<'a>() -> impl Parser<'a, Loc<Expr<'a>>, EExpr<'a>> {
242242
let mut e = expr;
243243
let orig_region = e.region;
244244
for (args_loc, maybe_suffixes) in arg_locs_with_suffixes_vec.iter() {
245-
let value = if matches!(
246-
e,
247-
Loc {
248-
value: Expr::Dbg,
249-
..
250-
}
251-
) {
252-
Expr::Apply(arena.alloc(e), args_loc.value.items, CalledVia::Space)
253-
} else if let Some(suffixes) = maybe_suffixes {
245+
let value = if let Some(suffixes) = maybe_suffixes {
254246
apply_expr_access_chain(
255247
arena,
256248
Expr::PncApply(arena.alloc(e), args_loc.value),
@@ -3150,49 +3142,65 @@ fn stmts_to_defs<'a>(
31503142
_,
31513143
) = e
31523144
{
3153-
let condition = &args[0];
3154-
let rest = stmts_to_expr(&stmts[i + 1..], arena)?;
3155-
let e = Expr::DbgStmt {
3156-
first: condition,
3157-
extra_args: &args[1..],
3158-
continuation: arena.alloc(rest),
3159-
};
3145+
if let Some((first, extra_args)) = args.split_first() {
3146+
let rest = stmts_to_expr(&stmts[i + 1..], arena)?;
3147+
let e = Expr::DbgStmt {
3148+
first,
3149+
extra_args,
3150+
continuation: arena.alloc(rest),
3151+
};
31603152

3161-
let e = if sp_stmt.before.is_empty() {
3162-
e
3163-
} else {
3164-
arena.alloc(e).before(sp_stmt.before)
3165-
};
3153+
let e = if sp_stmt.before.is_empty() {
3154+
e
3155+
} else {
3156+
arena.alloc(e).before(sp_stmt.before)
3157+
};
31663158

3167-
last_expr = Some(Loc::at(sp_stmt.item.region, e));
3159+
last_expr = Some(Loc::at(sp_stmt.item.region, e));
31683160

3169-
// don't re-process the rest of the statements; they got consumed by the dbg expr
3170-
break;
3161+
// don't re-process the rest of the statements; they got consumed by the dbg expr
3162+
break;
3163+
} else {
3164+
defs.push_value_def(
3165+
ValueDef::Stmt(arena.alloc(Loc::at(sp_stmt.item.region, e))),
3166+
sp_stmt.item.region,
3167+
sp_stmt.before,
3168+
&[],
3169+
);
3170+
}
31713171
} else if let Expr::PncApply(
31723172
Loc {
31733173
value: Expr::Dbg, ..
31743174
},
31753175
args,
31763176
) = e
31773177
{
3178-
let condition = &args.items[0];
3179-
let rest = stmts_to_expr(&stmts[i + 1..], arena)?;
3180-
let e = Expr::DbgStmt {
3181-
first: condition,
3182-
extra_args: &args.items[1..],
3183-
continuation: arena.alloc(rest),
3184-
};
3178+
if let Some((first, extra_args)) = args.items.split_first() {
3179+
let rest = stmts_to_expr(&stmts[i + 1..], arena)?;
3180+
let e = Expr::DbgStmt {
3181+
first,
3182+
extra_args,
3183+
continuation: arena.alloc(rest),
3184+
};
31853185

3186-
let e = if sp_stmt.before.is_empty() {
3187-
e
3188-
} else {
3189-
arena.alloc(e).before(sp_stmt.before)
3190-
};
3186+
let e = if sp_stmt.before.is_empty() {
3187+
e
3188+
} else {
3189+
arena.alloc(e).before(sp_stmt.before)
3190+
};
31913191

3192-
last_expr = Some(Loc::at(sp_stmt.item.region, e));
3192+
last_expr = Some(Loc::at(sp_stmt.item.region, e));
31933193

3194-
// don't re-process the rest of the statements; they got consumed by the dbg expr
3195-
break;
3194+
// don't re-process the rest of the statements; they got consumed by the dbg expr
3195+
break;
3196+
} else {
3197+
defs.push_value_def(
3198+
ValueDef::Stmt(arena.alloc(Loc::at(sp_stmt.item.region, e))),
3199+
sp_stmt.item.region,
3200+
sp_stmt.before,
3201+
&[],
3202+
);
3203+
}
31963204
} else {
31973205
defs.push_value_def(
31983206
ValueDef::Stmt(arena.alloc(Loc::at(sp_stmt.item.region, e))),

‎crates/compiler/parse/src/normalize.rs

+32-24
Original file line numberDiff line numberDiff line change
@@ -822,19 +822,23 @@ fn fold_defs<'a>(
822822
),
823823
..
824824
}) => {
825-
let rest = fold_defs(arena, defs, final_expr);
826-
let new_final = Expr::DbgStmt {
827-
first: args[0],
828-
extra_args: &args[1..],
829-
continuation: arena.alloc(Loc::at_zero(rest)),
830-
};
831-
if new_defs.is_empty() {
832-
return new_final;
825+
if let Some((first, extra_args)) = args.split_first() {
826+
let rest = fold_defs(arena, defs, final_expr);
827+
let new_final = Expr::DbgStmt {
828+
first,
829+
extra_args,
830+
continuation: arena.alloc(Loc::at_zero(rest)),
831+
};
832+
if new_defs.is_empty() {
833+
return new_final;
834+
}
835+
return Expr::Defs(
836+
arena.alloc(new_defs),
837+
arena.alloc(Loc::at_zero(new_final)),
838+
);
839+
} else {
840+
new_defs.push_value_def(vd, Region::zero(), &[], &[]);
833841
}
834-
return Expr::Defs(
835-
arena.alloc(new_defs),
836-
arena.alloc(Loc::at_zero(new_final)),
837-
);
838842
}
839843
ValueDef::Stmt(&Loc {
840844
value:
@@ -846,19 +850,23 @@ fn fold_defs<'a>(
846850
),
847851
..
848852
}) => {
849-
let rest = fold_defs(arena, defs, final_expr);
850-
let new_final = Expr::DbgStmt {
851-
first: args.items[0],
852-
extra_args: &args.items[1..],
853-
continuation: arena.alloc(Loc::at_zero(rest)),
854-
};
855-
if new_defs.is_empty() {
856-
return new_final;
853+
if let Some((first, extra_args)) = args.items.split_first() {
854+
let rest = fold_defs(arena, defs, final_expr);
855+
let new_final = Expr::DbgStmt {
856+
first,
857+
extra_args,
858+
continuation: arena.alloc(Loc::at_zero(rest)),
859+
};
860+
if new_defs.is_empty() {
861+
return new_final;
862+
}
863+
return Expr::Defs(
864+
arena.alloc(new_defs),
865+
arena.alloc(Loc::at_zero(new_final)),
866+
);
867+
} else {
868+
new_defs.push_value_def(vd, Region::zero(), &[], &[]);
857869
}
858-
return Expr::Defs(
859-
arena.alloc(new_defs),
860-
arena.alloc(Loc::at_zero(new_final)),
861-
);
862870
}
863871
_ => {
864872
new_defs.push_value_def(vd, Region::zero(), &[], &[]);

‎crates/compiler/test_syntax/tests/snapshots/pass/dbg_pnc_a_over_a.expr.formatted.roc

+2
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,2 @@
1+
dbg (a / a)
2+
d

‎crates/compiler/test_syntax/tests/snapshots/pass/dbg_pnc_a_over_a.expr.result-ast

+32
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,32 @@
1+
@0-8 SpaceAfter(
2+
DbgStmt {
3+
first: @4-7 BinOps(
4+
[
5+
(
6+
@4-5 Var {
7+
module_name: "",
8+
ident: "a",
9+
},
10+
@5-6 Slash,
11+
),
12+
],
13+
@6-7 Var {
14+
module_name: "",
15+
ident: "a",
16+
},
17+
),
18+
extra_args: [],
19+
continuation: @9-10 SpaceBefore(
20+
Var {
21+
module_name: "",
22+
ident: "d",
23+
},
24+
[
25+
Newline,
26+
],
27+
),
28+
},
29+
[
30+
Newline,
31+
],
32+
)

‎crates/compiler/test_syntax/tests/snapshots/pass/dbg_pnc_a_over_a.expr.roc

+2
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,2 @@
1+
dbg(a/a)
2+
d

‎crates/compiler/test_syntax/tests/snapshots/pass/dbg_pnc_zero_args.expr.formatted.roc

+2
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,2 @@
1+
dbg()
2+
d

‎crates/compiler/test_syntax/tests/snapshots/pass/dbg_pnc_zero_args.expr.result-ast

+40
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,40 @@
1+
@0-7 SpaceAfter(
2+
Defs(
3+
Defs {
4+
tags: [
5+
EitherIndex(2147483648),
6+
],
7+
regions: [
8+
@0-5,
9+
],
10+
space_before: [
11+
Slice<roc_parse::ast::CommentOrNewline> { start: 0, length: 0 },
12+
],
13+
space_after: [
14+
Slice<roc_parse::ast::CommentOrNewline> { start: 0, length: 0 },
15+
],
16+
spaces: [],
17+
type_defs: [],
18+
value_defs: [
19+
Stmt(
20+
@0-5 PncApply(
21+
@0-3 Dbg,
22+
[],
23+
),
24+
),
25+
],
26+
},
27+
@6-7 SpaceBefore(
28+
Var {
29+
module_name: "",
30+
ident: "d",
31+
},
32+
[
33+
Newline,
34+
],
35+
),
36+
),
37+
[
38+
Newline,
39+
],
40+
)

‎crates/compiler/test_syntax/tests/snapshots/pass/dbg_pnc_zero_args.expr.roc

+2
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,2 @@
1+
dbg()
2+
d

‎crates/compiler/test_syntax/tests/snapshots/pass/pnc_apply_comment_after_newline.expr.formatted.roc

+4
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,4 @@
1+
i(
2+
i,
3+
)
4+
t

‎crates/compiler/test_syntax/tests/snapshots/pass/pnc_apply_comment_after_newline.expr.result-ast

+31
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,31 @@
1+
@0-7 SpaceAfter(
2+
Apply(
3+
@0-6 PncApply(
4+
@0-1 Var {
5+
module_name: "",
6+
ident: "i",
7+
},
8+
Collection {
9+
items: [
10+
@2-3 Var {
11+
module_name: "",
12+
ident: "i",
13+
},
14+
],
15+
final_comments: [
16+
Newline,
17+
],
18+
},
19+
),
20+
[
21+
@6-7 Var {
22+
module_name: "",
23+
ident: "t",
24+
},
25+
],
26+
Space,
27+
),
28+
[
29+
Newline,
30+
],
31+
)

‎crates/compiler/test_syntax/tests/snapshots/pass/pnc_apply_comment_after_newline.expr.roc

+2
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,2 @@
1+
i(i,
2+
)t

‎crates/compiler/test_syntax/tests/snapshots/pass/pnc_parens_apply_etc.expr.formatted.roc

+2
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,2 @@
1+
3() : B
2+
z

0 commit comments

Comments
 (0)