From 1400f649c9c03c941aab923713bf53ba8b509289 Mon Sep 17 00:00:00 2001
From: Swastik Baranwal <swstkbaranwal@gmail.com>
Date: Fri, 29 Mar 2024 13:49:03 +0530
Subject: [PATCH]  checker: disallow using aliases of ?Type as !Type (#21128)

---
 vlib/v/checker/fn.v                           |  9 +++++++
 vlib/v/checker/return.v                       | 21 ++++++++++++++-
 .../tests/option_alias_result_type_err.out    | 21 +++++++++++++++
 .../tests/option_alias_result_type_err.vv     | 27 +++++++++++++++++++
 4 files changed, 77 insertions(+), 1 deletion(-)
 create mode 100644 vlib/v/checker/tests/option_alias_result_type_err.out
 create mode 100644 vlib/v/checker/tests/option_alias_result_type_err.vv

diff --git a/vlib/v/checker/fn.v b/vlib/v/checker/fn.v
index ba1446095..0e893cc4b 100644
--- a/vlib/v/checker/fn.v
+++ b/vlib/v/checker/fn.v
@@ -107,6 +107,15 @@ fn (mut c Checker) fn_decl(mut node ast.FnDecl) {
 		}
 	}
 	c.fn_return_type = node.return_type
+	return_type_unaliased := c.table.unaliased_type(node.return_type)
+	if node.return_type.has_flag(.option) && return_type_unaliased.has_flag(.result) {
+		c.error('the fn returns ${c.error_type_name(node.return_type)}, but ${c.error_type_name(node.return_type.clear_flag(.option))} is a Result alias, you can not mix them',
+			node.return_type_pos)
+	}
+	if node.return_type.has_flag(.result) && return_type_unaliased.has_flag(.option) {
+		c.error('the fn returns ${c.error_type_name(node.return_type)}, but ${c.error_type_name(node.return_type.clear_flag(.result))} is an Option alias, you can not mix them',
+			node.return_type_pos)
+	}
 	if node.return_type != ast.void_type {
 		if ct_attr_idx := node.attrs.find_comptime_define() {
 			sexpr := node.attrs[ct_attr_idx].ct_expr.str()
diff --git a/vlib/v/checker/return.v b/vlib/v/checker/return.v
index f5d03b090..131ca273e 100644
--- a/vlib/v/checker/return.v
+++ b/vlib/v/checker/return.v
@@ -9,7 +9,6 @@ import v.pref
 // ? => Option type
 // ! => Result type
 // others => type `name`
-@[inline]
 fn (mut c Checker) error_type_name(exp_type ast.Type) string {
 	return if exp_type == ast.void_type.set_flag(.result) {
 		'Result type'
@@ -20,6 +19,11 @@ fn (mut c Checker) error_type_name(exp_type ast.Type) string {
 	}
 }
 
+@[inline]
+fn (mut c Checker) error_unaliased_type_name(exp_type ast.Type) string {
+	return c.error_type_name(c.table.unaliased_type(exp_type))
+}
+
 // TODO: non deferred
 fn (mut c Checker) return_stmt(mut node ast.Return) {
 	if c.table.cur_fn == unsafe { nil } {
@@ -135,6 +139,21 @@ fn (mut c Checker) return_stmt(mut node ast.Return) {
 	} else if exp_is_result && got_types_0_idx == ast.none_type_idx {
 		c.error('Option and Result types have been split, use `?` to return none', node.pos)
 	}
+	expected_fn_return_type_has_option := c.table.cur_fn.return_type.has_flag(.option)
+	expected_fn_return_type_has_result := c.table.cur_fn.return_type.has_flag(.result)
+	if exp_is_option && expected_fn_return_type_has_result {
+		if got_types_0_idx == ast.none_type_idx {
+			c.error('cannot use `none` as ${c.error_type_name(c.table.unaliased_type(c.table.cur_fn.return_type))} in return argument',
+				node.pos)
+			return
+		}
+		return
+	}
+	if exp_is_result && expected_fn_return_type_has_option {
+		c.error('expecting to return a ?Type, but you are returning ${c.error_type_name(expected_type)} instead',
+			node.pos)
+		return
+	}
 	if (exp_is_option
 		&& got_types_0_idx in [ast.none_type_idx, ast.error_type_idx, option_type_idx])
 		|| (exp_is_result && got_types_0_idx in [ast.error_type_idx, result_type_idx]) {
diff --git a/vlib/v/checker/tests/option_alias_result_type_err.out b/vlib/v/checker/tests/option_alias_result_type_err.out
new file mode 100644
index 000000000..874a3954b
--- /dev/null
+++ b/vlib/v/checker/tests/option_alias_result_type_err.out
@@ -0,0 +1,21 @@
+vlib/v/checker/tests/option_alias_result_type_err.vv:13:13: error: the fn returns type `!Bar`, but type `Bar` is an Option alias, you can not mix them
+   11 | }
+   12 | 
+   13 | fn f(b Baz) !Bar {
+      |             ~~~~
+   14 |     match b {
+   15 |         .nothing {
+vlib/v/checker/tests/option_alias_result_type_err.vv:16:4: error: cannot use `none` as type `?Foo` in return argument
+   14 |     match b {
+   15 |         .nothing {
+   16 |             return none
+      |             ~~~~~~~~~~~
+   17 |         }
+   18 |         .value {
+vlib/v/checker/tests/option_alias_result_type_err.vv:24:4: error: cannot use `none` as type `?Foo` in return argument
+   22 |         }
+   23 |         else {
+   24 |             return none
+      |             ~~~~~~~~~~~
+   25 |         }
+   26 |     }
diff --git a/vlib/v/checker/tests/option_alias_result_type_err.vv b/vlib/v/checker/tests/option_alias_result_type_err.vv
new file mode 100644
index 000000000..d05e2c844
--- /dev/null
+++ b/vlib/v/checker/tests/option_alias_result_type_err.vv
@@ -0,0 +1,27 @@
+struct Foo {
+	x int
+}
+
+type Bar = ?Foo
+
+enum Baz {
+	nothing
+	bad
+	value
+}
+
+fn f(b Baz) !Bar {
+	match b {
+		.nothing {
+			return none
+		}
+		.value {
+			return Foo{
+				x: 42
+			}
+		}
+		else {
+			return none
+		}
+	}
+}
-- 
2.39.5