From b22305ec7fe40790310def9e36f3b50aa3f1b7e4 Mon Sep 17 00:00:00 2001 From: Alexander Medvednikov Date: Wed, 15 Apr 2026 15:59:22 +0300 Subject: [PATCH] fmt: fix match with or expressions (fixes #17201) --- vlib/v/fmt/fmt.v | 67 ++++++++- vlib/v/fmt/tests/match_expected.vv | 7 + vlib/v/fmt/tests/match_input.vv | 6 + vlib/v/parser/if_match.v | 127 ++++++++++++++++-- .../matches/match_expr_with_opt_result_test.v | 31 +++++ 5 files changed, 229 insertions(+), 9 deletions(-) diff --git a/vlib/v/fmt/fmt.v b/vlib/v/fmt/fmt.v index 4e9b9bc41..2893dc8b2 100644 --- a/vlib/v/fmt/fmt.v +++ b/vlib/v/fmt/fmt.v @@ -430,6 +430,7 @@ fn (f &Fmt) should_insert_newline_before_node(node ast.Node, prev_node ast.Node) } else {} } + match node { // Attributes are not respected in the stmts position, so this requires manual checking ast.StructDecl, ast.EnumDecl, ast.FnDecl { @@ -459,6 +460,7 @@ pub fn (mut f Fmt) node_str(node ast.Node) string { ast.Expr { f.expr(node) } else { panic('´f.node_str()´ is not implemented for ${node}.') } } + str := f.out.after(pos) f.out.go_back_to(pos) f.empty_line = was_empty_line @@ -841,6 +843,7 @@ fn expr_is_single_line(expr ast.Expr) bool { } else {} } + return true } @@ -1733,6 +1736,7 @@ pub fn (mut f Fmt) type_decl(node ast.TypeDecl) { ast.FnTypeDecl { f.fn_type_decl(node) } ast.SumTypeDecl { f.sum_type_decl(node) } } + f.writeln('') } @@ -2937,8 +2941,9 @@ fn (mut f Fmt) match_branch(branch ast.MatchBranch, single_line bool, is_comptim pub fn (mut f Fmt) match_expr(node ast.MatchExpr) { dollar := if node.is_comptime { '$' } else { '' } + cond, cond_or_expr := match_cond_with_trailing_or_expr(node.cond) f.write('${dollar}match ') - f.expr(node.cond) + f.expr(cond) f.writeln(' {') f.indent++ f.comments(node.comments) @@ -2969,6 +2974,63 @@ pub fn (mut f Fmt) match_expr(node ast.MatchExpr) { } f.indent-- f.write('}') + f.or_expr(cond_or_expr) +} + +fn match_cond_with_trailing_or_expr(expr ast.Expr) (ast.Expr, ast.OrExpr) { + match expr { + ast.CallExpr { + if expr.or_block.kind == .block { + mut cond := expr + or_expr := cond.or_block + cond.or_block = ast.OrExpr{} + return ast.Expr(cond), or_expr + } + } + ast.Ident { + if expr.or_expr.kind == .block { + mut cond := expr + or_expr := cond.or_expr + cond.or_expr = ast.OrExpr{} + return ast.Expr(cond), or_expr + } + } + ast.IndexExpr { + if expr.or_expr.kind == .block { + mut cond := expr + or_expr := cond.or_expr + cond.or_expr = ast.OrExpr{} + return ast.Expr(cond), or_expr + } + } + ast.ParExpr { + cond, or_expr := match_cond_with_trailing_or_expr(expr.expr) + if or_expr.kind == .block { + mut par_expr := expr + par_expr.expr = cond + return ast.Expr(par_expr), or_expr + } + } + ast.PrefixExpr { + if expr.op == .arrow && expr.or_block.kind == .block { + mut cond := expr + or_expr := cond.or_block + cond.or_block = ast.OrExpr{} + return ast.Expr(cond), or_expr + } + } + ast.SelectorExpr { + if expr.or_block.kind == .block { + mut cond := expr + or_expr := cond.or_block + cond.or_block = ast.OrExpr{} + return ast.Expr(cond), or_expr + } + } + else {} + } + + return expr, ast.OrExpr{} } pub fn (mut f Fmt) offset_of(node ast.OffsetOf) { @@ -3063,6 +3125,7 @@ pub fn (mut f Fmt) prefix_expr(node ast.PrefixExpr) { .not_is { f.write(' is ') } else {} } + f.expr(node.right.expr.right) return } @@ -3099,6 +3162,7 @@ pub fn (mut f Fmt) select_expr(node ast.SelectExpr) { ast.ExprStmt { f.expr(branch.stmt.expr) } else { f.stmt(branch.stmt) } } + f.single_line_if = false f.write(' {') } @@ -3222,6 +3286,7 @@ pub fn (mut f Fmt) sql_expr(node ast.SqlExpr) { .right { f.write('right join ') } .full_outer { f.write('full outer join ') } } + join_sym := f.table.sym(join.table_expr.typ) mut join_table_name := join_sym.name if !join_table_name.starts_with('C.') && !join_table_name.starts_with('JS.') { diff --git a/vlib/v/fmt/tests/match_expected.vv b/vlib/v/fmt/tests/match_expected.vv index 57d45e331..1c37c050e 100644 --- a/vlib/v/fmt/tests/match_expected.vv +++ b/vlib/v/fmt/tests/match_expected.vv @@ -42,6 +42,7 @@ fn match_branch_extra_comma() { Foo, Bar {} int, string {} } + match n { 0...5 {} 2, 3 {} @@ -56,3 +57,9 @@ fn match_index_range_expr(var string) string { else { 'i#' + var } } } + +fn match_condition_or_block() { + match read_bool() { + true {} + } or {} +} diff --git a/vlib/v/fmt/tests/match_input.vv b/vlib/v/fmt/tests/match_input.vv index a641f3bf6..00449e066 100644 --- a/vlib/v/fmt/tests/match_input.vv +++ b/vlib/v/fmt/tests/match_input.vv @@ -54,3 +54,9 @@ fn match_index_range_expr(var string) string{ else { 'i#' + var } } } + +fn match_condition_or_block() { + match read_bool() or {} { + true {} + } +} diff --git a/vlib/v/parser/if_match.v b/vlib/v/parser/if_match.v index 82ba84a66..9023d3386 100644 --- a/vlib/v/parser/if_match.v +++ b/vlib/v/parser/if_match.v @@ -302,9 +302,91 @@ fn (mut p Parser) resolve_at_expr(expr ast.AtExpr) !string { return error('top level comptime only support `@MOD` `@OS` `@CCOMPILER` `@BACKEND` or `@PLATFORM`') } } + return '' } +enum MatchCondOrBlockMode { + unsupported + no_err_var + with_err_var + already_set +} + +fn (p &Parser) match_cond_or_block_mode(cond ast.Expr) MatchCondOrBlockMode { + return match cond { + ast.CallExpr { + if cond.or_block.kind == ast.OrKind.absent { + MatchCondOrBlockMode.with_err_var + } else { + MatchCondOrBlockMode.already_set + } + } + ast.Ident { + if cond.or_expr.kind == ast.OrKind.absent { + MatchCondOrBlockMode.no_err_var + } else { + MatchCondOrBlockMode.already_set + } + } + ast.IndexExpr { + if cond.or_expr.kind == ast.OrKind.absent { + MatchCondOrBlockMode.no_err_var + } else { + MatchCondOrBlockMode.already_set + } + } + ast.ParExpr { + p.match_cond_or_block_mode(cond.expr) + } + ast.PrefixExpr { + if cond.op == .arrow { + if cond.or_block.kind == ast.OrKind.absent { + MatchCondOrBlockMode.with_err_var + } else { + MatchCondOrBlockMode.already_set + } + } else { + MatchCondOrBlockMode.unsupported + } + } + ast.SelectorExpr { + if cond.or_block.kind == ast.OrKind.absent { + MatchCondOrBlockMode.with_err_var + } else { + MatchCondOrBlockMode.already_set + } + } + else { + MatchCondOrBlockMode.unsupported + } + } +} + +fn (mut p Parser) set_match_cond_or_block(mut cond ast.Expr, or_expr ast.OrExpr) { + match mut cond { + ast.CallExpr { + cond.or_block = or_expr + } + ast.Ident { + cond.or_expr = or_expr + } + ast.IndexExpr { + cond.or_expr = or_expr + } + ast.ParExpr { + p.set_match_cond_or_block(mut cond.expr, or_expr) + } + ast.PrefixExpr { + cond.or_block = or_expr + } + ast.SelectorExpr { + cond.or_block = or_expr + } + else {} + } +} + fn (mut p Parser) match_expr(is_comptime bool, is_expr bool) ast.MatchExpr { mut match_first_pos := p.tok.pos() old_inside_ct_match := p.inside_ct_match @@ -318,10 +400,10 @@ fn (mut p Parser) match_expr(is_comptime bool, is_expr bool) ast.MatchExpr { p.inside_match = true p.check(.key_match) mut is_sum_type := false - cond := p.expr(0) + mut cond := p.expr(0) mut cond_str := '' if is_comptime && cond is ast.AtExpr && p.is_in_top_level_comptime(p.inside_assign_rhs) { - cond_str = p.resolve_at_expr(cond) or { + cond_str = p.resolve_at_expr(cond as ast.AtExpr) or { p.error(err.msg()) return ast.MatchExpr{} } @@ -412,6 +494,7 @@ fn (mut p Parser) match_expr(is_comptime bool, is_expr bool) ast.MatchExpr { } else {} } + comptime_skip_curr_stmts = cond_str != case_str if !comptime_skip_curr_stmts { comptime_has_true_branch = true @@ -493,18 +576,44 @@ fn (mut p Parser) match_expr(is_comptime bool, is_expr bool) ast.MatchExpr { break } } - match_last_pos := p.tok.pos() + mut match_last_pos := p.tok.pos() + if p.tok.kind == .rcbr { + p.check(.rcbr) + match_last_pos = p.prev_tok.pos() + } + if p.tok.kind == .key_orelse { + cond_or_mode := p.match_cond_or_block_mode(cond) + if cond_or_mode == .already_set { + p.error_with_pos('match condition already has an `or {}` block', p.tok.pos()) + return ast.MatchExpr{} + } + if cond_or_mode == .unsupported { + p.error_with_pos('trailing `or {}` is only supported for match conditions that can use `or {}` directly', + p.tok.pos()) + return ast.MatchExpr{} + } + err_var_mode := if cond_or_mode == .with_err_var { + OrBlockErrVarMode.with_err_var + } else { + OrBlockErrVarMode.no_err_var + } + or_stmts, or_pos, or_scope := p.or_block(err_var_mode) + p.set_match_cond_or_block(mut cond, ast.OrExpr{ + kind: .block + stmts: or_stmts + pos: or_pos + scope: or_scope + }) + match_last_pos = p.prev_tok.pos() + } + // return ast.StructInit{} mut pos := token.Pos{ line_nr: match_first_pos.line_nr pos: match_first_pos.pos len: match_last_pos.pos - match_first_pos.pos + match_last_pos.len col: match_first_pos.col } - if p.tok.kind == .rcbr { - p.check(.rcbr) - } - // return ast.StructInit{} - pos.update_last_line(p.prev_tok.line_nr) + pos.update_last_line(match_last_pos.line_nr) return ast.MatchExpr{ is_comptime: is_comptime is_expr: is_expr_ @@ -804,6 +913,7 @@ fn (mut p Parser) comptime_if_cond(mut cond ast.Expr) bool { return false } } + return is_true } else { @@ -811,6 +921,7 @@ fn (mut p Parser) comptime_if_cond(mut cond ast.Expr) bool { return false } } + p.error('invalid \$if condition') return false } diff --git a/vlib/v/tests/conditions/matches/match_expr_with_opt_result_test.v b/vlib/v/tests/conditions/matches/match_expr_with_opt_result_test.v index 167f33c7c..5fc6023e3 100644 --- a/vlib/v/tests/conditions/matches/match_expr_with_opt_result_test.v +++ b/vlib/v/tests/conditions/matches/match_expr_with_opt_result_test.v @@ -11,6 +11,13 @@ fn bar() ?Foo { return Foo{2} } +fn read_bool(ok bool) !bool { + if ok { + return true + } + return error('read_bool failed') +} + fn test_main() { match foo()!.a { 1 { @@ -30,3 +37,27 @@ fn test_main() { } } } + +fn test_match_expr_with_trailing_or_block() { + mut seen_err := '' + got_false := match read_bool(false) { + true { 'true' } + false { 'false' } + } or { + seen_err = err.msg() + false + } + + assert seen_err == 'read_bool failed' + assert got_false == 'false' + + got_true := match read_bool(true) { + true { 'true' } + false { 'false' } + } or { + assert false + false + } + + assert got_true == 'true' +} -- 2.39.5