language-servers/roc
1
0
Fork 0
mirror of https://github.com/roc-lang/roc.git synced 2025-08-04 20:28:02 +00:00
Code Issues Projects Releases Packages Wiki Activity Actions 7

Merge pull request #7060 from snobee/early-return-if-else

Browse source 
Support "early return" version of if-else statements
This commit is contained in:
Sam Mohr 2024-09-06 20:19:46 -07:00 • committed by GitHub
commit 9a4d556725
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
19 changed files with 262 additions and 88 deletions
Download patch file Download diff file Expand all files Collapse all files

15
crates/compiler/can/src/desugar.rs
View file

@ -993,10 +993,13 @@ pub fn desugar_expr<'a>(
region: loc_expr.region,
})
}
If(if_thens, final_else_branch) => {
If {
if_thens,
final_else,
indented_else,
} => {
// If does not get desugared into `when` so we can give more targeted error messages during type checking.
let desugared_final_else =
&*env.arena.alloc(desugar_expr(env, scope, final_else_branch));
let desugared_final_else = &*env.arena.alloc(desugar_expr(env, scope, final_else));
let mut desugared_if_thens = Vec::with_capacity_in(if_thens.len(), env.arena);
@ -1008,7 +1011,11 @@ pub fn desugar_expr<'a>(
}
env.arena.alloc(Loc {
value: If(desugared_if_thens.into_bump_slice(), desugared_final_else),
value: If {
if_thens: desugared_if_thens.into_bump_slice(),
final_else: desugared_final_else,
indented_else: *indented_else,
},
region: loc_expr.region,
})
}

12
crates/compiler/can/src/expr.rs
View file

@ -1261,7 +1261,11 @@ pub fn canonicalize_expr<'a>(
output,
)
}
ast::Expr::If(if_thens, final_else_branch) => {
ast::Expr::If {
if_thens,
final_else: final_else_branch,
..
} => {
let mut branches = Vec::with_capacity(if_thens.len());
let mut output = Output::default();
@ -2572,7 +2576,11 @@ pub fn is_valid_interpolation(expr: &ast::Expr<'_>) -> bool {
.iter()
.all(|(loc_expr, _binop)| is_valid_interpolation(&loc_expr.value))
}
ast::Expr::If(branches, final_branch) => {
ast::Expr::If {
if_thens: branches,
final_else: final_branch,
..
} => {
is_valid_interpolation(&final_branch.value)
&& branches.iter().all(|(loc_before, loc_after)| {
is_valid_interpolation(&loc_before.value)

71
crates/compiler/can/src/suffixed.rs
View file

@ -131,7 +131,7 @@ pub fn unwrap_suffixed_expression<'a>(
Expr::When(..) => unwrap_suffixed_expression_when_help(arena, loc_expr, maybe_def_pat),
Expr::If(..) => {
Expr::If { .. } => {
unwrap_suffixed_expression_if_then_else_help(arena, loc_expr, maybe_def_pat)
}
@ -357,7 +357,11 @@ pub fn unwrap_suffixed_expression_if_then_else_help<'a>(
maybe_def_pat: Option<&'a Loc<Pattern<'a>>>,
) -> Result<&'a Loc<Expr<'a>>, EUnwrapped<'a>> {
match loc_expr.value {
Expr::If(if_thens, final_else_branch) => {
Expr::If {
if_thens,
final_else: final_else_branch,
indented_else,
} => {
for (index, if_then) in if_thens.iter().enumerate() {
let (current_if_then_statement, current_if_then_expression) = if_then;
@ -376,10 +380,11 @@ pub fn unwrap_suffixed_expression_if_then_else_help<'a>(
let new_if = arena.alloc(Loc::at(
loc_expr.region,
Expr::If(
arena.alloc_slice_copy(new_if_thens.as_slice()),
final_else_branch,
),
Expr::If {
if_thens: arena.alloc_slice_copy(new_if_thens.as_slice()),
final_else: final_else_branch,
indented_else,
},
));
return unwrap_suffixed_expression(arena, new_if, maybe_def_pat);
@ -411,10 +416,11 @@ pub fn unwrap_suffixed_expression_if_then_else_help<'a>(
let new_if = arena.alloc(Loc::at(
loc_expr.region,
Expr::If(
arena.alloc_slice_copy(new_if_thens.as_slice()),
final_else_branch,
),
Expr::If {
if_thens: arena.alloc_slice_copy(new_if_thens.as_slice()),
final_else: final_else_branch,
indented_else,
},
));
return unwrap_suffixed_expression(arena, new_if, maybe_def_pat);
@ -439,10 +445,11 @@ pub fn unwrap_suffixed_expression_if_then_else_help<'a>(
let new_if = arena.alloc(Loc::at(
loc_expr.region,
Expr::If(
arena.alloc_slice_copy(new_if_thens.as_slice()),
final_else_branch,
),
Expr::If {
if_thens: arena.alloc_slice_copy(new_if_thens.as_slice()),
final_else: final_else_branch,
indented_else,
},
));
return unwrap_suffixed_expression(arena, new_if, maybe_def_pat);
@ -465,10 +472,11 @@ pub fn unwrap_suffixed_expression_if_then_else_help<'a>(
let new_if = arena.alloc(Loc::at(
loc_expr.region,
Expr::If(
arena.alloc_slice_copy(new_if_thens.as_slice()),
final_else_branch,
),
Expr::If {
if_thens: arena.alloc_slice_copy(new_if_thens.as_slice()),
final_else: final_else_branch,
indented_else,
},
));
let unwrapped_if_then = apply_try_function(
@ -494,10 +502,11 @@ pub fn unwrap_suffixed_expression_if_then_else_help<'a>(
let after_if = arena.alloc(Loc::at(
loc_expr.region,
Expr::If(
arena.alloc_slice_copy(after_if_thens.as_slice()),
final_else_branch,
),
Expr::If {
if_thens: arena.alloc_slice_copy(after_if_thens.as_slice()),
final_else: final_else_branch,
indented_else,
},
));
let after_if_then = apply_try_function(
@ -512,7 +521,11 @@ pub fn unwrap_suffixed_expression_if_then_else_help<'a>(
let before_if_then = arena.alloc(Loc::at(
loc_expr.region,
Expr::If(before, after_if_then),
Expr::If {
if_thens: before,
final_else: after_if_then,
indented_else: false,
},
));
return unwrap_suffixed_expression(
@ -532,7 +545,11 @@ pub fn unwrap_suffixed_expression_if_then_else_help<'a>(
Ok(unwrapped_final_else) => {
return Ok(arena.alloc(Loc::at(
loc_expr.region,
Expr::If(if_thens, unwrapped_final_else),
Expr::If {
if_thens,
final_else: unwrapped_final_else,
indented_else,
},
)));
}
Err(EUnwrapped::UnwrappedDefExpr { .. }) => {
@ -556,7 +573,11 @@ pub fn unwrap_suffixed_expression_if_then_else_help<'a>(
let new_if = arena.alloc(Loc::at(
loc_expr.region,
Expr::If(if_thens, unwrapped_final_else),
Expr::If {
if_thens,
final_else: unwrapped_final_else,
indented_else,
},
));
return unwrap_suffixed_expression(arena, new_if, maybe_def_pat);

9
crates/compiler/can/tests/snapshots/test_suffixed__suffixed_tests__deps_final_expr.snap
View file

@ -63,8 +63,8 @@ Defs {
),
],
},
@66-130 If(
[
@66-130 If {
if_thens: [
(
@69-70 Var {
module_name: "",
@ -76,11 +76,12 @@ Defs {
},
),
],
@128-130 Var {
final_else: @128-130 Var {
module_name: "",
ident: "c",
},
),
indented_else: false,
},
),
guard: None,
},

18
crates/compiler/can/tests/snapshots/test_suffixed__suffixed_tests__if_complex.snap
View file

@ -136,8 +136,8 @@ Defs {
ident: "#!1_arg",
},
],
@109-298 If(
[
@109-298 If {
if_thens: [
(
@112-122 Apply(
@112-113 Var {
@ -244,7 +244,7 @@ Defs {
),
),
],
Apply(
final_else: Apply(
Var {
module_name: "Task",
ident: "await",
@ -269,8 +269,8 @@ Defs {
ident: "#!3_arg",
},
],
@109-298 If(
[
@109-298 If {
if_thens: [
(
@187-209 ParensAround(
Var {
@ -366,7 +366,7 @@ Defs {
),
),
],
@283-298 Apply(
final_else: @283-298 Apply(
@283-298 Var {
module_name: "",
ident: "line",
@ -380,12 +380,14 @@ Defs {
],
Space,
),
),
indented_else: false,
},
),
],
BangSuffix,
),
),
indented_else: false,
},
),
],
BangSuffix,

18
crates/compiler/can/tests/snapshots/test_suffixed__suffixed_tests__if_simple.snap
View file

@ -101,8 +101,8 @@ Defs {
ident: "#!0_arg",
},
],
@76-189 If(
[
@76-189 If {
if_thens: [
(
@79-87 Var {
module_name: "",
@ -124,7 +124,7 @@ Defs {
),
),
],
@125-132 Apply(
final_else: @125-132 Apply(
@125-132 Var {
module_name: "Task",
ident: "await",
@ -140,8 +140,8 @@ Defs {
ident: "#!1_arg",
},
],
@76-189 If(
[
@76-189 If {
if_thens: [
(
@125-132 Var {
module_name: "",
@ -163,7 +163,7 @@ Defs {
),
),
],
@178-189 Apply(
final_else: @178-189 Apply(
@178-182 Var {
module_name: "",
ident: "line",
@ -177,12 +177,14 @@ Defs {
],
Space,
),
),
indented_else: false,
},
),
],
BangSuffix,
),
),
indented_else: false,
},
),
],
BangSuffix,

38
crates/compiler/fmt/src/expr.rs
View file

@ -71,7 +71,11 @@ impl<'a> Formattable for Expr<'a> {
"LowLevelDbg should only exist after desugaring, not during formatting"
),
If(branches, final_else) => {
If {
if_thens: branches,
final_else,
..
} => {
final_else.is_multiline()
|| branches
.iter()
@ -464,8 +468,19 @@ impl<'a> Formattable for Expr<'a> {
LowLevelDbg(_, _, _) => unreachable!(
"LowLevelDbg should only exist after desugaring, not during formatting"
),
If(branches, final_else) => {
fmt_if(buf, branches, final_else, self.is_multiline(), indent);
If {
if_thens: branches,
final_else,
indented_else,
} => {
fmt_if(
buf,
branches,
final_else,
self.is_multiline(),
*indented_else,
indent,
);
}
When(loc_condition, branches) => fmt_when(buf, loc_condition, branches, indent),
Tuple(items) => fmt_collection(buf, indent, Braces::Round, *items, Newlines::No),
@ -1076,6 +1091,7 @@ fn fmt_if<'a>(
branches: &'a [(Loc<Expr<'a>>, Loc<Expr<'a>>)],
final_else: &'a Loc<Expr<'a>>,
is_multiline: bool,
indented_else: bool,
indent: u16,
) {
// let is_multiline_then = loc_then.is_multiline();
@ -1208,16 +1224,22 @@ fn fmt_if<'a>(
}
}
buf.indent(indent);
if is_multiline {
if indented_else {
buf.indent(indent + INDENT);
buf.push_str("else");
buf.newline();
buf.newline();
} else if is_multiline {
buf.indent(indent);
buf.push_str("else");
buf.newline();
} else {
buf.indent(indent);
buf.push_str(" else");
buf.spaces(1);
}
final_else.format(buf, return_indent);
let indent = if indented_else { indent } else { return_indent };
final_else.format(buf, indent);
}
fn fmt_closure<'a>(
@ -1753,7 +1775,7 @@ fn sub_expr_requests_parens(expr: &Expr<'_>) -> bool {
| BinOp::Pizza => true,
})
}
Expr::If(_, _) => true,
Expr::If { .. } => true,
Expr::SpaceBefore(e, _) => sub_expr_requests_parens(e),
Expr::SpaceAfter(e, _) => sub_expr_requests_parens(e),
_ => false,

26
crates/compiler/parse/src/ast.rs
View file

@ -512,7 +512,11 @@ pub enum Expr<'a> {
UnaryOp(&'a Loc<Expr<'a>>, Loc<UnaryOp>),
// Conditionals
If(&'a [(Loc<Expr<'a>>, Loc<Expr<'a>>)], &'a Loc<Expr<'a>>),
If {
if_thens: &'a [(Loc<Expr<'a>>, Loc<Expr<'a>>)],
final_else: &'a Loc<Expr<'a>>,
indented_else: bool,
},
When(
/// The condition
&'a Loc<Expr<'a>>,
@ -608,7 +612,11 @@ pub fn is_expr_suffixed(expr: &Expr) -> bool {
}
// expression in a if-then-else, `if isOk! then "ok" else doSomething!`
Expr::If(if_thens, final_else) => {
Expr::If {
if_thens,
final_else,
..
} => {
let any_if_thens_suffixed = if_thens.iter().any(|(if_then, else_expr)| {
is_expr_suffixed(&if_then.value) || is_expr_suffixed(&else_expr.value)
});
@ -993,14 +1001,18 @@ impl<'a, 'b> RecursiveValueDefIter<'a, 'b> {
expr_stack.push(&expr.value);
}
UnaryOp(expr, _) => expr_stack.push(&expr.value),
If(ifs, alternate) => {
expr_stack.reserve(ifs.len() * 2 + 1);
If {
if_thens,
final_else,
..
} => {
expr_stack.reserve(if_thens.len() * 2 + 1);
for (condition, consequent) in ifs.iter() {
for (condition, consequent) in if_thens.iter() {
expr_stack.push(&condition.value);
expr_stack.push(&consequent.value);
}
expr_stack.push(&alternate.value);
expr_stack.push(&final_else.value);
}
When(condition, branches) => {
expr_stack.reserve(branches.len() + 1);
@ -2537,7 +2549,7 @@ impl<'a> Malformed for Expr<'a> {
Apply(func, args, _) => func.is_malformed() || args.iter().any(|arg| arg.is_malformed()),
BinOps(firsts, last) => firsts.iter().any(|(expr, _)| expr.is_malformed()) || last.is_malformed(),
UnaryOp(expr, _) => expr.is_malformed(),
If(chain, els) => chain.iter().any(|(cond, body)| cond.is_malformed() || body.is_malformed()) || els.is_malformed(),
If { if_thens, final_else, ..} => if_thens.iter().any(|(cond, body)| cond.is_malformed() || body.is_malformed()) || final_else.is_malformed(),
When(cond, branches) => cond.is_malformed() || branches.iter().any(|branch| branch.is_malformed()),
SpaceBefore(expr, _) |

31
crates/compiler/parse/src/expr.rs
View file

@ -2170,7 +2170,7 @@ fn expr_to_pattern_help<'a>(arena: &'a Bump, expr: &Expr<'a>) -> Result<Pattern<
| Expr::Backpassing(_, _, _)
| Expr::BinOps { .. }
| Expr::Defs(_, _)
| Expr::If(_, _)
| Expr::If { .. }
| Expr::When(_, _)
| Expr::Expect(_, _)
| Expr::Dbg
@ -2704,6 +2704,8 @@ fn if_expr_help<'a>(options: ExprParseOptions) -> impl Parser<'a, Expr<'a>, EIf<
let (_, _, state) =
parser::keyword(keyword::IF, EIf::If).parse(arena, state, min_indent)?;
let if_indent = state.line_indent();
let mut branches = Vec::with_capacity_in(1, arena);
let mut loop_state = state;
@ -2730,16 +2732,37 @@ fn if_expr_help<'a>(options: ExprParseOptions) -> impl Parser<'a, Expr<'a>, EIf<
}
};
let (_, else_branch, state) = parse_block(
let else_indent = state_final_else.line_indent();
let indented_else = else_indent > if_indent;
let min_indent = if !indented_else {
else_indent + 1
} else {
if_indent
};
let (_, loc_first_space, state_final_else) =
loc_space0_e(EIf::IndentElseBranch).parse(arena, state_final_else, min_indent)?;
let allow_defs = !loc_first_space.value.is_empty();
// use parse_block_inner so we can set min_indent
let (_, else_branch, state) = parse_block_inner(
options,
arena,
state_final_else,
true,
min_indent,
EIf::IndentElseBranch,
EIf::ElseBranch,
loc_first_space,
allow_defs,
)?;
let expr = Expr::If(branches.into_bump_slice(), arena.alloc(else_branch));
let expr = Expr::If {
if_thens: branches.into_bump_slice(),
final_else: arena.alloc(else_branch),
indented_else,
};
Ok((MadeProgress, expr, state))
}

10
crates/compiler/parse/src/normalize.rs
View file

@ -796,7 +796,15 @@ impl<'a> Normalize<'a> for Expr<'a> {
Expr::UnaryOp(a, b) => {
Expr::UnaryOp(arena.alloc(a.normalize(arena)), b.normalize(arena))
}
Expr::If(a, b) => Expr::If(a.normalize(arena), arena.alloc(b.normalize(arena))),
Expr::If {
if_thens,
final_else,
indented_else,
} => Expr::If {
if_thens: if_thens.normalize(arena),
final_else: arena.alloc(final_else.normalize(arena)),
indented_else,
},
Expr::When(a, b) => Expr::When(arena.alloc(a.normalize(arena)), b.normalize(arena)),
Expr::ParensAround(a) => {
// The formatter can remove redundant parentheses, so also remove these when normalizing for comparison.

1
crates/compiler/test_syntax/tests/snapshots/fail/if_outdented_else_branch.expr.result-ast Normal file
View file

@ -0,0 +1 @@
Expr(If(IndentElseBranch(@31), @0), @0)

4
crates/compiler/test_syntax/tests/snapshots/fail/if_outdented_else_branch.expr.roc Normal file
View file

@ -0,0 +1,4 @@
if thing then
whatever
else
something better

9
crates/compiler/test_syntax/tests/snapshots/pass/extra_newline.expr.result-ast
View file

@ -1,6 +1,6 @@
SpaceAfter(
If(
[
If {
if_thens: [
(
@3-6 Var {
module_name: "",
@ -60,7 +60,7 @@ SpaceAfter(
),
),
],
@49-50 SpaceBefore(
final_else: @49-50 SpaceBefore(
Var {
module_name: "",
ident: "c",
@ -71,7 +71,8 @@ SpaceAfter(
),
],
),
),
indented_else: false,
},
[
LineComment(
" 4",

9
crates/compiler/test_syntax/tests/snapshots/pass/nested_if.expr.result-ast
View file

@ -1,6 +1,6 @@
SpaceAfter(
If(
[
If {
if_thens: [
(
@3-5 Var {
module_name: "",
@ -40,7 +40,7 @@ SpaceAfter(
),
),
],
@42-43 SpaceBefore(
final_else: @42-43 SpaceBefore(
Num(
"3",
),
@ -48,7 +48,8 @@ SpaceAfter(
Newline,
],
),
),
indented_else: false,
},
[
Newline,
],

9
crates/compiler/test_syntax/tests/snapshots/pass/plus_if.expr.result-ast
View file

@ -8,8 +8,8 @@ SpaceAfter(
@2-3 Star,
),
],
@4-30 If(
[
@4-30 If {
if_thens: [
(
@7-16 Var {
module_name: "Bool",
@ -20,10 +20,11 @@ SpaceAfter(
),
),
],
@29-30 Num(
final_else: @29-30 Num(
"1",
),
),
indented_else: false,
},
),
[
Newline,

9
crates/compiler/test_syntax/tests/snapshots/pass/record_with_if.expr.result-ast
View file

@ -3,8 +3,8 @@ Record(
@1-31 RequiredValue(
@1-2 "x",
[],
@5-31 If(
[
@5-31 If {
if_thens: [
(
@8-17 Var {
module_name: "Bool",
@ -15,10 +15,11 @@ Record(
),
),
],
@30-31 Num(
final_else: @30-31 Num(
"2",
),
),
indented_else: false,
},
),
@33-37 RequiredValue(
@33-34 "y",

54
crates/compiler/test_syntax/tests/test_fmt.rs
View file

@ -3628,6 +3628,60 @@ mod test_fmt {
));
}
#[test]
fn early_return_else() {
expr_formats_same(indoc!(
r"
if foo then
bar
else
baz
"
));
expr_formats_to(
indoc!(
r"
if thing then
whatever
else
too close
"
),
indoc!(
r"
if thing then
whatever
else
too close
"
),
);
expr_formats_to(
indoc!(
r"
if isGrowing plant then
LetBe
else
Water
"
),
indoc!(
r"
if isGrowing plant then
LetBe
else
Water
"
),
);
}
#[test]
fn multi_line_application() {
expr_formats_same(indoc!(

1
crates/compiler/test_syntax/tests/test_snapshots.rs
View file

@ -202,6 +202,7 @@ mod test_snapshots {
fail/expression_indentation_end.expr,
fail/if_guard_without_condition.expr,
fail/if_missing_else.expr,
fail/if_outdented_else_branch.expr,
fail/if_outdented_then.expr,
fail/import_with_lowercase_alias.moduledefs,
fail/imports_missing_comma.header,

6
crates/language_server/src/analysis/tokens.rs
View file

@ -710,7 +710,11 @@ impl IterTokens for Loc<Expr<'_>> {
Expr::UnaryOp(e1, op) => (op.iter_tokens(arena).into_iter())
.chain(e1.iter_tokens(arena))
.collect_in(arena),
Expr::If(e1, e2) => (e1.iter_tokens(arena).into_iter())
Expr::If {
if_thens: e1,
final_else: e2,
..
} => (e1.iter_tokens(arena).into_iter())
.chain(e2.iter_tokens(arena))
.collect_in(arena),
Expr::When(e, branches) => (e.iter_tokens(arena).into_iter())