From f4d3c7eb7090fe709f8625674356f9855ae66dca Mon Sep 17 00:00:00 2001
From: Swastik Baranwal <swstkbaranwal@gmail.com>
Date: Wed, 13 Nov 2024 06:16:12 +0530
Subject: [PATCH] parser,checker: improve the position underlining, for last
 statements in branches of `if` expressions  (#22845)

---
 vlib/v/checker/if.v                                   | 5 ++++-
 vlib/v/checker/tests/if_expr_last_branch_stmt_err.out | 7 +++++++
 vlib/v/checker/tests/if_expr_last_branch_stmt_err.vv  | 7 +++++++
 vlib/v/parser/expr.v                                  | 8 ++++++--
 vlib/v/parser/if_match.v                              | 6 +++---
 vlib/v/parser/parser.v                                | 4 ++--
 6 files changed, 29 insertions(+), 8 deletions(-)
 create mode 100644 vlib/v/checker/tests/if_expr_last_branch_stmt_err.out
 create mode 100644 vlib/v/checker/tests/if_expr_last_branch_stmt_err.vv

diff --git a/vlib/v/checker/if.v b/vlib/v/checker/if.v
index 602ce9dbb..95ef6a757 100644
--- a/vlib/v/checker/if.v
+++ b/vlib/v/checker/if.v
@@ -32,6 +32,8 @@ fn (mut c Checker) if_expr(mut node ast.IfExpr) ast.Type {
 		stmts := node.branches[0].stmts
 		if stmts.len > 0 && stmts.last() is ast.ExprStmt && stmts.last().typ != ast.void_type {
 			node_is_expr = true
+		} else if node.is_expr {
+			node_is_expr = true
 		}
 	}
 	if c.expected_type == ast.void_type && node_is_expr {
@@ -520,8 +522,9 @@ fn (mut c Checker) if_expr(mut node ast.IfExpr) ast.Type {
 						}
 					}
 				} else if !node.is_comptime && stmt !in [ast.Return, ast.BranchStmt] {
+					pos := if node_is_expr { stmt.pos } else { branch.pos }
 					c.error('`${if_kind}` expression requires an expression as the last statement of every branch',
-						branch.pos)
+						pos)
 				}
 			} else if !node.is_comptime {
 				c.error('`${if_kind}` expression requires an expression as the last statement of every branch',
diff --git a/vlib/v/checker/tests/if_expr_last_branch_stmt_err.out b/vlib/v/checker/tests/if_expr_last_branch_stmt_err.out
new file mode 100644
index 000000000..209f4dd13
--- /dev/null
+++ b/vlib/v/checker/tests/if_expr_last_branch_stmt_err.out
@@ -0,0 +1,7 @@
+vlib/v/checker/tests/if_expr_last_branch_stmt_err.vv:5:30: error: `if` expression requires an expression as the last statement of every branch
+    3 |     mut start := 0
+    4 | 
+    5 |     start = if val < 20 { start += 1 } else { start }
+      |                                 ~~
+    6 |     dump(start)
+    7 | }
diff --git a/vlib/v/checker/tests/if_expr_last_branch_stmt_err.vv b/vlib/v/checker/tests/if_expr_last_branch_stmt_err.vv
new file mode 100644
index 000000000..889bb5209
--- /dev/null
+++ b/vlib/v/checker/tests/if_expr_last_branch_stmt_err.vv
@@ -0,0 +1,7 @@
+fn main() {
+	val := 15
+	mut start := 0
+
+	start = if val < 20 { start += 1 } else { start }
+	dump(start)
+}
diff --git a/vlib/v/parser/expr.v b/vlib/v/parser/expr.v
index 07b1dc143..c9c745074 100644
--- a/vlib/v/parser/expr.v
+++ b/vlib/v/parser/expr.v
@@ -128,7 +128,7 @@ fn (mut p Parser) check_expr(precedence int) !ast.Expr {
 					p.is_stmt_ident = is_stmt_ident
 				}
 				.key_if {
-					return p.if_expr(true)
+					return p.if_expr(true, false)
 				}
 				else {
 					return p.unexpected_with_pos(p.peek_tok.pos(),
@@ -222,7 +222,11 @@ fn (mut p Parser) check_expr(precedence int) !ast.Expr {
 			if p.peek_tok.kind in [.lpar, .lsbr] && p.peek_tok.is_next_to(p.tok) {
 				node = p.call_expr(p.language, p.mod)
 			} else {
-				node = p.if_expr(false)
+				mut is_expr := false
+				if p.prev_tok.kind.is_assign() {
+					is_expr = true
+				}
+				node = p.if_expr(false, is_expr)
 			}
 		}
 		.key_unsafe {
diff --git a/vlib/v/parser/if_match.v b/vlib/v/parser/if_match.v
index d395370d5..7ebc25b11 100644
--- a/vlib/v/parser/if_match.v
+++ b/vlib/v/parser/if_match.v
@@ -6,7 +6,7 @@ module parser
 import v.ast
 import v.token
 
-fn (mut p Parser) if_expr(is_comptime bool) ast.IfExpr {
+fn (mut p Parser) if_expr(is_comptime bool, is_expr bool) ast.IfExpr {
 	was_inside_if_expr := p.inside_if_expr
 	was_inside_ct_if_expr := p.inside_ct_if_expr
 	defer {
@@ -14,7 +14,7 @@ fn (mut p Parser) if_expr(is_comptime bool) ast.IfExpr {
 		p.inside_ct_if_expr = was_inside_ct_if_expr
 	}
 	p.inside_if_expr = true
-	is_expr := p.prev_tok.kind == .key_return
+	is_expr_ := p.prev_tok.kind == .key_return || is_expr
 	mut pos := p.tok.pos()
 	if is_comptime {
 		p.inside_ct_if_expr = true
@@ -213,7 +213,7 @@ fn (mut p Parser) if_expr(is_comptime bool) ast.IfExpr {
 		post_comments: comments
 		pos:           pos
 		has_else:      has_else
-		is_expr:       is_expr
+		is_expr:       is_expr_
 	}
 }
 
diff --git a/vlib/v/parser/parser.v b/vlib/v/parser/parser.v
index fac47ae3e..1db20ec8d 100644
--- a/vlib/v/parser/parser.v
+++ b/vlib/v/parser/parser.v
@@ -795,7 +795,7 @@ fn (mut p Parser) top_stmt() ast.Stmt {
 					comptime_for_stmt := p.comptime_for()
 					return p.other_stmts(comptime_for_stmt)
 				} else if p.peek_tok.kind == .key_if {
-					if_expr := p.if_expr(true)
+					if_expr := p.if_expr(true, false)
 					cur_stmt := ast.ExprStmt{
 						expr: if_expr
 						pos:  if_expr.pos
@@ -1077,7 +1077,7 @@ fn (mut p Parser) stmt(is_top_level bool) ast.Stmt {
 			match p.peek_tok.kind {
 				.key_if {
 					mut pos := p.tok.pos()
-					expr := p.if_expr(true)
+					expr := p.if_expr(true, false)
 					pos.update_last_line(p.prev_tok.line_nr)
 					return ast.ExprStmt{
 						expr: expr
-- 
2.39.5