From ba577e4e5fda52388a5755712659631a81e66093 Mon Sep 17 00:00:00 2001 From: Turiiya <34311583+ttytm@users.noreply.github.com> Date: Thu, 4 Apr 2024 11:43:22 +0200 Subject: [PATCH] parser, checker: move more match validation from the parser into the checker, add error for match without branches (#21181) --- vlib/v/checker/match.v | 25 ++++++++++---- vlib/v/checker/tests/match_missing.out | 33 +++++++++++++++++++ vlib/v/checker/tests/match_missing.vv | 11 +++++++ .../tests/return_missing_match_simple.out | 7 ---- .../tests/return_missing_match_simple.vv | 12 ------- vlib/v/parser/if_match.v | 14 ++++---- .../tests/match_multi_else_branch_err.out | 7 ---- .../tests/match_multi_else_branch_err.vv | 7 ---- 8 files changed, 68 insertions(+), 48 deletions(-) create mode 100644 vlib/v/checker/tests/match_missing.out create mode 100644 vlib/v/checker/tests/match_missing.vv delete mode 100644 vlib/v/checker/tests/return_missing_match_simple.out delete mode 100644 vlib/v/checker/tests/return_missing_match_simple.vv delete mode 100644 vlib/v/parser/tests/match_multi_else_branch_err.out delete mode 100644 vlib/v/parser/tests/match_multi_else_branch_err.vv diff --git a/vlib/v/checker/match.v b/vlib/v/checker/match.v index 374c62f25c44fa..31030097f0706d 100644 --- a/vlib/v/checker/match.v +++ b/vlib/v/checker/match.v @@ -535,17 +535,28 @@ fn (mut c Checker) match_exprs(mut node ast.MatchExpr, cond_type_sym ast.TypeSym } } } + if node.branches.len == 0 { + c.error('`match` must have at least two branches including `else`, or an exhaustive set of branches', + node.pos) + return + } mut else_branch := node.branches.last() - mut has_else := else_branch.is_else - if !has_else { - for i, branch in node.branches { - if branch.is_else && i != node.branches.len - 1 { - c.error('`else` must be the last branch of `match`', branch.pos) - else_branch = branch - has_else = true + mut has_else, mut has_non_else := else_branch.is_else, !else_branch.is_else + for branch in node.branches[..node.branches.len - 1] { + if branch.is_else { + if has_else { + c.error('`match` can have only one `else` branch', branch.pos) } + c.error('`else` must be the last branch of `match`', branch.pos) + else_branch = branch + has_else = true + } else { + has_non_else = true } } + if !has_non_else { + c.error('`match` must have at least one non `else` branch', else_branch.pos) + } if is_exhaustive { if has_else && !c.pref.translated && !c.file.is_translated { c.error('match expression is exhaustive, `else` is unnecessary', else_branch.pos) diff --git a/vlib/v/checker/tests/match_missing.out b/vlib/v/checker/tests/match_missing.out new file mode 100644 index 00000000000000..85f831c7b93f38 --- /dev/null +++ b/vlib/v/checker/tests/match_missing.out @@ -0,0 +1,33 @@ +vlib/v/checker/tests/match_missing.vv:1:1: error: `match` must have at least two branches including `else`, or an exhaustive set of branches + 1 | match true { + | ~~~~~ + 2 | } + 3 | +vlib/v/checker/tests/match_missing.vv:5:2: error: `match` can have only one `else` branch + 3 | + 4 | match true { + 5 | else {} + | ~~~~ + 6 | else {} + 7 | } +vlib/v/checker/tests/match_missing.vv:5:2: error: `else` must be the last branch of `match` + 3 | + 4 | match true { + 5 | else {} + | ~~~~ + 6 | else {} + 7 | } +vlib/v/checker/tests/match_missing.vv:5:2: error: `match` must have at least one non `else` branch + 3 | + 4 | match true { + 5 | else {} + | ~~~~ + 6 | else {} + 7 | } +vlib/v/checker/tests/match_missing.vv:9:1: error: match must be exhaustive (add match branches for: `true`, `false` or `else {}` at the end) + 7 | } + 8 | + 9 | match true { + | ~~~~~~~~~~~~ + 10 | 'foo' == 'bar' {} + 11 | } diff --git a/vlib/v/checker/tests/match_missing.vv b/vlib/v/checker/tests/match_missing.vv new file mode 100644 index 00000000000000..2ff9113866f61c --- /dev/null +++ b/vlib/v/checker/tests/match_missing.vv @@ -0,0 +1,11 @@ +match true { +} + +match true { + else {} + else {} +} + +match true { + 'foo' == 'bar' {} +} diff --git a/vlib/v/checker/tests/return_missing_match_simple.out b/vlib/v/checker/tests/return_missing_match_simple.out deleted file mode 100644 index 73221738577a23..00000000000000 --- a/vlib/v/checker/tests/return_missing_match_simple.out +++ /dev/null @@ -1,7 +0,0 @@ -vlib/v/checker/tests/return_missing_match_simple.vv:4:3: error: `match` must have at least one non `else` branch - 2 | a := 1 - 3 | match a { - 4 | else { println('abc') } - | ~~~~ - 5 | } - 6 | println('hello') diff --git a/vlib/v/checker/tests/return_missing_match_simple.vv b/vlib/v/checker/tests/return_missing_match_simple.vv deleted file mode 100644 index 463c2b20f07e74..00000000000000 --- a/vlib/v/checker/tests/return_missing_match_simple.vv +++ /dev/null @@ -1,12 +0,0 @@ -fn h() int { - a := 1 - match a { - else { println('abc') } - } - println('hello') -} - -fn main() { - d := h() - println('h returned: $d') -} diff --git a/vlib/v/parser/if_match.v b/vlib/v/parser/if_match.v index 97d3ff3d433602..961aaf0d6f717f 100644 --- a/vlib/v/parser/if_match.v +++ b/vlib/v/parser/if_match.v @@ -237,7 +237,6 @@ fn (mut p Parser) is_match_sumtype_type() bool { fn (mut p Parser) match_expr() ast.MatchExpr { match_first_pos := p.tok.pos() - mut else_count := 0 p.inside_match = true p.check(.key_match) mut is_sum_type := false @@ -258,7 +257,6 @@ fn (mut p Parser) match_expr() ast.MatchExpr { mut is_else := false if p.tok.kind == .key_else { is_else = true - else_count += 1 p.next() } else if p.is_match_sumtype_type() || p.is_only_array_type() || p.tok.kind == .key_fn @@ -289,6 +287,12 @@ fn (mut p Parser) match_expr() ast.MatchExpr { } is_sum_type = true } else { + if p.tok.kind == .rcbr { + p.next() + return ast.MatchExpr{ + pos: match_first_pos + } + } // Expression match for { p.inside_match_case = true @@ -351,12 +355,6 @@ fn (mut p Parser) match_expr() ast.MatchExpr { post_comments: post_comments scope: branch_scope } - if is_else && branches.len == 1 { - p.error_with_pos('`match` must have at least one non `else` branch', pos) - } - if else_count > 1 { - p.error_with_pos('`match` can have only one `else` branch', pos) - } if p.tok.kind == .rcbr || (is_else && no_lcbr) { break } diff --git a/vlib/v/parser/tests/match_multi_else_branch_err.out b/vlib/v/parser/tests/match_multi_else_branch_err.out deleted file mode 100644 index 1f0534fe442408..00000000000000 --- a/vlib/v/parser/tests/match_multi_else_branch_err.out +++ /dev/null @@ -1,7 +0,0 @@ -vlib/v/parser/tests/match_multi_else_branch_err.vv:5:3: error: `match` can have only one `else` branch - 3 | true { 1 } - 4 | else { 3 } - 5 | else { 5 } - | ~~~~ - 6 | } - 7 | } diff --git a/vlib/v/parser/tests/match_multi_else_branch_err.vv b/vlib/v/parser/tests/match_multi_else_branch_err.vv deleted file mode 100644 index 2aefa812d34a62..00000000000000 --- a/vlib/v/parser/tests/match_multi_else_branch_err.vv +++ /dev/null @@ -1,7 +0,0 @@ -fn main() { - _ := match true { - true { 1 } - else { 3 } - else { 5 } - } -}