From 5be2fcab7bb566124c7f8ef50cbffe0690595487 Mon Sep 17 00:00:00 2001
From: Felipe Pena <felipensp@gmail.com>
Date: Sat, 25 Jan 2025 02:23:05 -0300
Subject: [PATCH] checker: make `option_var.str()` an error, when done without
 unwrapping it first (fix #23557, fix #23558) (#23563)

---
 vlib/v/ast/ast.v                              | 10 +++---
 vlib/v/checker/assign.v                       | 20 ++++++++++-
 vlib/v/checker/comptime.v                     |  3 ++
 vlib/v/checker/fn.v                           |  4 +--
 vlib/v/checker/postfix.v                      |  8 +++--
 vlib/v/checker/tests/option_str_call.out      | 12 +++++++
 vlib/v/checker/tests/option_str_call.vv       |  8 +++++
 vlib/v/gen/c/assign.v                         | 14 ++++++++
 vlib/v/gen/c/cgen.v                           | 14 +++++---
 vlib/v/parser/comptime.v                      |  5 +++
 .../c_struct_with_reserved_field_name_test.v  |  2 +-
 vlib/v/tests/comptime/comptime_unwrap_test.v  | 35 +++++++++++++++++++
 .../comptime/comptime_var_assignment_test.v   |  4 +--
 vlib/v/tests/options/option_var_cast_test.v   |  8 ++---
 .../sumtypes/sumtype_generic_checking_test.v  |  4 +--
 vlib/v/type_resolver/comptime_resolver.v      |  5 +++
 vlib/v/type_resolver/type_resolver.v          | 24 +++++++++++--
 vlib/x/json2/encoder.v                        | 11 ++++--
 vlib/x/sessions/tests/session_app_test.v      |  6 ++--
 19 files changed, 167 insertions(+), 30 deletions(-)
 create mode 100644 vlib/v/checker/tests/option_str_call.out
 create mode 100644 vlib/v/checker/tests/option_str_call.vv
 create mode 100644 vlib/v/tests/comptime/comptime_unwrap_test.v

diff --git a/vlib/v/ast/ast.v b/vlib/v/ast/ast.v
index be6372c9b..c2dab9038 100644
--- a/vlib/v/ast/ast.v
+++ b/vlib/v/ast/ast.v
@@ -906,10 +906,11 @@ pub mut:
 	// 10 <- original type (orig_type)
 	//   [11, 12, 13] <- cast order (smartcasts)
 	//        12 <- the current casted type (typ)
-	pos         token.Pos
-	is_used     bool            // whether the local variable was used in other expressions
-	is_changed  bool            // to detect mutable vars that are never changed
-	ct_type_var ComptimeVarKind // comptime variable type
+	pos               token.Pos
+	is_used           bool            // whether the local variable was used in other expressions
+	is_changed        bool            // to detect mutable vars that are never changed
+	ct_type_var       ComptimeVarKind // comptime variable type
+	ct_type_unwrapped bool            // true when the comptime variable gets unwrapped
 	// (for setting the position after the or block for autofree)
 	is_or        bool // `x := foo() or { ... }`
 	is_tmp       bool // for tmp for loop vars, so that autofree can skip them
@@ -1995,6 +1996,7 @@ pub struct ComptimeSelector {
 pub:
 	has_parens bool // if $() is used, for vfmt
 	pos        token.Pos
+	or_block   OrExpr
 pub mut:
 	left       Expr
 	left_type  Type
diff --git a/vlib/v/checker/assign.v b/vlib/v/checker/assign.v
index 8d2c592e0..26d867ac2 100644
--- a/vlib/v/checker/assign.v
+++ b/vlib/v/checker/assign.v
@@ -925,8 +925,12 @@ or use an explicit `unsafe{ a[..] }`, if you do not want a copy of the slice.',
 fn (mut c Checker) change_flags_if_comptime_expr(mut left ast.Ident, right ast.Expr) {
 	if mut left.obj is ast.Var {
 		if right is ast.ComptimeSelector {
-			left.obj.ct_type_var = .field_var
 			left.obj.typ = c.comptime.comptime_for_field_type
+			if right.or_block.kind == .propagate_option {
+				left.obj.typ = left.obj.typ.clear_flag(.option)
+				left.obj.ct_type_unwrapped = true
+			}
+			left.obj.ct_type_var = .field_var
 		} else if right is ast.InfixExpr {
 			right_ct_var := c.comptime.get_ct_type_var(right.left)
 			if right_ct_var != .no_comptime {
@@ -960,6 +964,20 @@ fn (mut c Checker) change_flags_if_comptime_expr(mut left ast.Ident, right ast.E
 				// mark variable as generic var because its type changes according to fn return generic resolution type
 				left.obj.ct_type_var = .generic_var
 			}
+		} else if right is ast.PostfixExpr && right.op == .question {
+			if right.expr is ast.Ident && right.expr.ct_expr {
+				right_obj_var := right.expr.obj as ast.Var
+				ctyp := c.type_resolver.get_type(right)
+				if ctyp != ast.void_type {
+					left.obj.ct_type_unwrapped = true
+					left.obj.ct_type_var = right_obj_var.ct_type_var
+					left.obj.typ = ctyp.clear_flag(.option)
+				}
+			} else if right.expr is ast.ComptimeSelector {
+				left.obj.ct_type_unwrapped = true
+				left.obj.ct_type_var = .field_var
+				left.obj.typ = c.comptime.comptime_for_field_type.clear_flag(.option)
+			}
 		}
 	}
 }
diff --git a/vlib/v/checker/comptime.v b/vlib/v/checker/comptime.v
index f1348a673..e25440191 100644
--- a/vlib/v/checker/comptime.v
+++ b/vlib/v/checker/comptime.v
@@ -223,6 +223,9 @@ fn (mut c Checker) comptime_selector(mut node ast.ComptimeSelector) ast.Type {
 		}
 		expr_type = c.type_resolver.get_comptime_selector_type(node, ast.void_type)
 		if expr_type != ast.void_type {
+			if node.or_block.kind == .propagate_option {
+				return expr_type.clear_flag(.option)
+			}
 			return expr_type
 		}
 		expr_name := node.field_expr.expr.str()
diff --git a/vlib/v/checker/fn.v b/vlib/v/checker/fn.v
index 090e7cd3e..1c251486c 100644
--- a/vlib/v/checker/fn.v
+++ b/vlib/v/checker/fn.v
@@ -1973,8 +1973,8 @@ fn (mut c Checker) method_call(mut node ast.CallExpr, mut continue_check &bool)
 	final_left_sym := c.table.final_sym(unwrapped_left_type)
 
 	method_name := node.name
-	if left_type.has_flag(.option) && method_name != 'str' {
-		c.error('Option type cannot be called directly', node.left.pos())
+	if left_type.has_flag(.option) {
+		c.error('Option type cannot be called directly, you should unwrap it first', node.left.pos())
 		return ast.void_type
 	} else if left_type.has_flag(.result) {
 		c.error('Result type cannot be called directly', node.left.pos())
diff --git a/vlib/v/checker/postfix.v b/vlib/v/checker/postfix.v
index 352b052f1..559d0b864 100644
--- a/vlib/v/checker/postfix.v
+++ b/vlib/v/checker/postfix.v
@@ -3,7 +3,7 @@ module checker
 import v.ast
 
 fn (mut c Checker) postfix_expr(mut node ast.PostfixExpr) ast.Type {
-	typ := c.unwrap_generic(c.expr(mut node.expr))
+	typ := c.unwrap_generic(c.type_resolver.get_type_or_default(node, c.expr(mut node.expr)))
 	typ_sym := c.table.sym(typ)
 	is_non_void_pointer := typ.is_any_kind_of_pointer() && typ_sym.kind != .voidptr
 
@@ -37,7 +37,11 @@ fn (mut c Checker) postfix_expr(mut node ast.PostfixExpr) ast.Type {
 		c.error('invalid operation: ${node.op.str()} (non-numeric type `${typ_str}`)',
 			node.pos)
 	} else {
-		node.auto_locked, _ = c.fail_if_immutable(mut node.expr)
+		if node.op == .question {
+			c.table.used_features.option_or_result = true
+		} else {
+			node.auto_locked, _ = c.fail_if_immutable(mut node.expr)
+		}
 	}
 	node.typ = typ
 	return typ
diff --git a/vlib/v/checker/tests/option_str_call.out b/vlib/v/checker/tests/option_str_call.out
new file mode 100644
index 000000000..0e03a150e
--- /dev/null
+++ b/vlib/v/checker/tests/option_str_call.out
@@ -0,0 +1,12 @@
+vlib/v/checker/tests/option_str_call.vv:7:10: error: Option type cannot be called directly, you should unwrap it first
+    5 |     footer := doc.get_tags_by_class_name('Box-footer')[0]
+    6 |     invalid := footer.get_tag_by_class_name('invalid')
+    7 |     println(invalid.str())
+      |             ~~~~~~~
+    8 | }
+vlib/v/checker/tests/option_str_call.vv:7:2: error: `println` can not print void expressions
+    5 |     footer := doc.get_tags_by_class_name('Box-footer')[0]
+    6 |     invalid := footer.get_tag_by_class_name('invalid')
+    7 |     println(invalid.str())
+      |     ~~~~~~~~~~~~~~~~~~~~~~
+    8 | }
diff --git a/vlib/v/checker/tests/option_str_call.vv b/vlib/v/checker/tests/option_str_call.vv
new file mode 100644
index 000000000..00cf01c06
--- /dev/null
+++ b/vlib/v/checker/tests/option_str_call.vv
@@ -0,0 +1,8 @@
+import net.html
+
+fn main() {
+	mut doc := html.parse('<body><div class="Box-footer"><div class="Truncate">abc</div></div></body>')
+	footer := doc.get_tags_by_class_name('Box-footer')[0]
+	invalid := footer.get_tag_by_class_name('invalid')
+	println(invalid.str())
+}
diff --git a/vlib/v/gen/c/assign.v b/vlib/v/gen/c/assign.v
index 6508f6a03..c6d38dca3 100644
--- a/vlib/v/gen/c/assign.v
+++ b/vlib/v/gen/c/assign.v
@@ -389,6 +389,20 @@ fn (mut g Gen) assign_stmt(node_ ast.AssignStmt) {
 							left.obj.typ = var_type
 							g.assign_ct_type = var_type
 						}
+					} else if val is ast.PostfixExpr && val.op == .question
+						&& (val.expr is ast.Ident && val.expr.ct_expr) {
+						ctyp := g.unwrap_generic(g.type_resolver.get_type(val))
+						if ctyp != ast.void_type {
+							var_type = ctyp
+							val_type = var_type
+							left.obj.typ = var_type
+							g.assign_ct_type = var_type
+
+							ct_type_var := g.comptime.get_ct_type_var(val.expr)
+							if ct_type_var == .field_var {
+								g.type_resolver.update_ct_type(left.name, ctyp)
+							}
+						}
 					}
 				}
 				is_auto_heap = left.obj.is_auto_heap
diff --git a/vlib/v/gen/c/cgen.v b/vlib/v/gen/c/cgen.v
index cf3fed1da..cc75bdf57 100644
--- a/vlib/v/gen/c/cgen.v
+++ b/vlib/v/gen/c/cgen.v
@@ -3656,20 +3656,26 @@ fn (mut g Gen) expr(node_ ast.Expr) {
 			} else if node.op == .question {
 				cur_line := g.go_before_last_stmt().trim_space()
 				mut expr_str := ''
+				mut is_unwrapped := true
 				if mut node.expr is ast.ComptimeSelector && node.expr.left is ast.Ident {
 					// val.$(field.name)?
 					expr_str = '${node.expr.left.str()}.${g.comptime.comptime_for_field_value.name}'
 				} else if mut node.expr is ast.Ident && node.expr.ct_expr {
 					// val?
 					expr_str = node.expr.name
+					is_unwrapped = !g.inside_assign
 				}
 				g.writeln('if (${expr_str}.state != 0) {')
 				g.writeln2('\tpanic_option_not_set(_SLIT("none"));', '}')
 				g.write(cur_line)
-				typ := g.resolve_comptime_type(node.expr, node.typ)
-				g.write('*(${g.base_type(typ)}*)&')
-				g.expr(node.expr)
-				g.write('.data')
+				if is_unwrapped {
+					typ := g.resolve_comptime_type(node.expr, node.typ)
+					g.write('*(${g.base_type(typ)}*)&')
+					g.expr(node.expr)
+					g.write('.data')
+				} else {
+					g.expr(node.expr)
+				}
 			} else {
 				g.expr(node.expr)
 			}
diff --git a/vlib/v/parser/comptime.v b/vlib/v/parser/comptime.v
index 19fe361c4..639ae728a 100644
--- a/vlib/v/parser/comptime.v
+++ b/vlib/v/parser/comptime.v
@@ -526,5 +526,10 @@ fn (mut p Parser) comptime_selector(left ast.Expr) ast.Expr {
 		left:       left
 		field_expr: expr
 		pos:        start_pos.extend(p.prev_tok.pos())
+		or_block:   ast.OrExpr{
+			stmts: []ast.Stmt{}
+			kind:  if p.tok.kind == .question { .propagate_option } else { .absent }
+			pos:   p.tok.pos()
+		}
 	}
 }
diff --git a/vlib/v/tests/c_struct_with_reserved_field_name_test.v b/vlib/v/tests/c_struct_with_reserved_field_name_test.v
index 7dcd76e66..814a78629 100644
--- a/vlib/v/tests/c_struct_with_reserved_field_name_test.v
+++ b/vlib/v/tests/c_struct_with_reserved_field_name_test.v
@@ -17,6 +17,6 @@ fn test_c_struct_with_reserved_field_name() {
 		window_title: 'Polygons'
 	)
 	game.gg = cont
-	game.gg.str()
+	game.gg?.str()
 	assert true
 }
diff --git a/vlib/v/tests/comptime/comptime_unwrap_test.v b/vlib/v/tests/comptime/comptime_unwrap_test.v
new file mode 100644
index 000000000..88361e9f4
--- /dev/null
+++ b/vlib/v/tests/comptime/comptime_unwrap_test.v
@@ -0,0 +1,35 @@
+struct Foo {
+	a ?int
+	b ?string
+}
+
+fn receives_int(a int) {}
+
+fn receives_string(a string) {}
+
+fn test_main() {
+	t := Foo{
+		a: 1
+		b: 'foo'
+	}
+	mut c := 0
+	$for f in t.fields {
+		$if f.typ is ?int {
+			assert t.$(f.name) ?.str() == '1'
+			w := t.$(f.name) ?
+			assert w == 1
+			receives_int(w)
+			receives_int(t.$(f.name) ?)
+			c++
+		}
+		$if f.typ is ?string {
+			assert t.$(f.name) ?.str() == 'foo'
+			a := t.$(f.name) ?
+			assert a == 'foo'
+			receives_string(a)
+			receives_string(t.$(f.name) ?)
+			c++
+		}
+	}
+	assert c == 2
+}
diff --git a/vlib/v/tests/comptime/comptime_var_assignment_test.v b/vlib/v/tests/comptime/comptime_var_assignment_test.v
index 70f582db9..1dc36626b 100644
--- a/vlib/v/tests/comptime/comptime_var_assignment_test.v
+++ b/vlib/v/tests/comptime/comptime_var_assignment_test.v
@@ -10,9 +10,9 @@ fn encode_struct[T](val T) []string {
 	$for field in T.fields {
 		value := val.$(field.name)
 		$if field.is_option {
-			gg := value
+			gg := value ?
 			println(gg)
-			out << gg.str()
+			out << '${value}'
 		} $else {
 			gg := value
 			println(gg)
diff --git a/vlib/v/tests/options/option_var_cast_test.v b/vlib/v/tests/options/option_var_cast_test.v
index cad0ee9dd..113153dc4 100644
--- a/vlib/v/tests/options/option_var_cast_test.v
+++ b/vlib/v/tests/options/option_var_cast_test.v
@@ -54,8 +54,8 @@ fn test_cast() {
 }
 
 fn test_cast_alias() {
-	assert ?MyByte(0).str() == 'Option(MyByte(0))'
-	assert ?MyByte(255).str() == 'Option(MyByte(255))'
-	assert ?MyByte(?u8(0)).str() == 'Option(MyByte(0))'
-	assert ?MyByte(?u8(255)).str() == 'Option(MyByte(255))'
+	assert '${?MyByte(0)}' == 'Option(MyByte(0))'
+	assert '${?MyByte(255)}' == 'Option(MyByte(255))'
+	assert '${?MyByte(?u8(0))}' == 'Option(MyByte(0))'
+	assert '${?MyByte(?u8(255))}' == 'Option(MyByte(255))'
 }
diff --git a/vlib/v/tests/sumtypes/sumtype_generic_checking_test.v b/vlib/v/tests/sumtypes/sumtype_generic_checking_test.v
index 66e1f6734..7b8903c4e 100644
--- a/vlib/v/tests/sumtypes/sumtype_generic_checking_test.v
+++ b/vlib/v/tests/sumtypes/sumtype_generic_checking_test.v
@@ -15,6 +15,6 @@ fn generic_fn[T]() ?T {
 }
 
 fn test_main() {
-	assert generic_fn[string]().str() == "Option('123')"
-	assert generic_fn[int]().str() == 'Option(123)'
+	assert '${generic_fn[string]()}' == "Option('123')"
+	assert '${generic_fn[int]()}' == 'Option(123)'
 }
diff --git a/vlib/v/type_resolver/comptime_resolver.v b/vlib/v/type_resolver/comptime_resolver.v
index 7e06e0f2e..633f907ea 100644
--- a/vlib/v/type_resolver/comptime_resolver.v
+++ b/vlib/v/type_resolver/comptime_resolver.v
@@ -19,6 +19,7 @@ pub fn (t &ResolverInfo) is_comptime_expr(node ast.Expr) bool {
 	return (node is ast.Ident && node.ct_expr)
 		|| (node is ast.IndexExpr && t.is_comptime_expr(node.left))
 		|| node is ast.ComptimeSelector
+		|| (node is ast.PostfixExpr && t.is_comptime_expr(node.expr))
 }
 
 // has_comptime_expr checks if the expr contains some comptime expr
@@ -27,6 +28,7 @@ pub fn (t &ResolverInfo) has_comptime_expr(node ast.Expr) bool {
 	return (node is ast.Ident && node.ct_expr)
 		|| (node is ast.IndexExpr && t.has_comptime_expr(node.left))
 		|| node is ast.ComptimeSelector
+		|| (node is ast.PostfixExpr && t.has_comptime_expr(node.expr))
 		|| (node is ast.SelectorExpr && t.has_comptime_expr(node.expr))
 		|| (node is ast.InfixExpr && (t.has_comptime_expr(node.left)
 		|| t.has_comptime_expr(node.right)))
@@ -58,6 +60,9 @@ pub fn (t &ResolverInfo) is_comptime(node ast.Expr) bool {
 		ast.ComptimeSelector {
 			return true
 		}
+		ast.PostfixExpr {
+			return t.is_comptime(node.expr)
+		}
 		else {
 			false
 		}
diff --git a/vlib/v/type_resolver/type_resolver.v b/vlib/v/type_resolver/type_resolver.v
index bc8c3f84b..d6d36c98a 100644
--- a/vlib/v/type_resolver/type_resolver.v
+++ b/vlib/v/type_resolver/type_resolver.v
@@ -142,6 +142,12 @@ pub fn (mut t TypeResolver) get_type_or_default(node ast.Expr, default_typ ast.T
 				return t.resolver.unwrap_generic(node.typ)
 			}
 		}
+		ast.PostfixExpr {
+			if node.op == .question && node.expr is ast.Ident && node.expr.ct_expr {
+				ctyp := t.get_type(node)
+				return if ctyp != ast.void_type { ctyp } else { default_typ }
+			}
+		}
 		else {
 			return default_typ
 		}
@@ -182,7 +188,11 @@ pub fn (mut t TypeResolver) get_type(node ast.Expr) ast.Type {
 				}
 				.field_var {
 					// field var from $for loop
-					t.info.comptime_for_field_type
+					if node.obj.ct_type_unwrapped {
+						t.info.comptime_for_field_type.clear_flag(.option)
+					} else {
+						t.info.comptime_for_field_type
+					}
 				}
 				else {
 					ast.void_type
@@ -191,7 +201,11 @@ pub fn (mut t TypeResolver) get_type(node ast.Expr) ast.Type {
 		}
 	} else if node is ast.ComptimeSelector {
 		// val.$(field.name)
-		return t.get_comptime_selector_type(node, ast.void_type)
+		ctyp := t.get_comptime_selector_type(node, ast.void_type)
+		if node.or_block.kind == .propagate_option {
+			return ctyp.clear_flag(.option)
+		}
+		return ctyp
 	} else if node is ast.SelectorExpr {
 		if t.info.is_comptime_selector_type(node) {
 			return t.get_type_from_comptime_var(node.expr as ast.Ident)
@@ -231,6 +245,12 @@ pub fn (mut t TypeResolver) get_type(node ast.Expr) ast.Type {
 	} else if node is ast.CastExpr && node.typ.has_flag(.generic) {
 		// T(expr)
 		return t.resolver.unwrap_generic(node.typ)
+	} else if node is ast.PostfixExpr && node.op == .question
+		&& node.expr in [ast.Ident, ast.ComptimeSelector] {
+		// var?
+		// f.$(field.name)?
+		ctyp := t.get_type(node.expr)
+		return ctyp.clear_flag(.option)
 	}
 	return ast.void_type
 }
diff --git a/vlib/x/json2/encoder.v b/vlib/x/json2/encoder.v
index 22d65d527..b270f87c0 100644
--- a/vlib/x/json2/encoder.v
+++ b/vlib/x/json2/encoder.v
@@ -229,9 +229,14 @@ fn (e &Encoder) encode_struct[U](val U, level int, mut buf []u8) ! {
 		mut ignore_field := false
 
 		value := val.$(field.name)
-
-		is_nil := val.$(field.name).str() == '&nil'
-
+		mut is_nil := false
+		$if value is $option {
+			if field.indirections > 0 {
+				is_nil = value == none
+			}
+		} $else $if field.indirections > 0 {
+			is_nil = value == unsafe { nil }
+		}
 		mut json_name := ''
 
 		for attr in field.attrs {
diff --git a/vlib/x/sessions/tests/session_app_test.v b/vlib/x/sessions/tests/session_app_test.v
index 6523f17a7..0f64fcbb3 100644
--- a/vlib/x/sessions/tests/session_app_test.v
+++ b/vlib/x/sessions/tests/session_app_test.v
@@ -40,7 +40,7 @@ pub fn (mut app App) before_accept_loop() {
 }
 
 pub fn (app &App) session_data(mut ctx Context) veb.Result {
-	return ctx.text(ctx.session_data.str())
+	return ctx.text('${ctx.session_data}')
 }
 
 pub fn (app &App) protected(mut ctx Context) veb.Result {
@@ -122,7 +122,7 @@ fn test_save_session() {
 	x := make_request_with_session_id(.get, '/session_data', sid)!
 
 	// cast to `?User` since, session_data is of type `?T`
-	assert x.body == ?User(default_user).str()
+	assert x.body == '${?User(default_user)}'
 }
 
 fn test_update_session() {
@@ -133,7 +133,7 @@ fn test_update_session() {
 
 	mut updated_user := default_user
 	updated_user.age++
-	assert x.body == ?User(updated_user).str()
+	assert x.body == '${?User(updated_user)}'
 }
 
 fn test_destroy_session() {
-- 
2.39.5