checker: fix and improve return stmt error messages (#17477)

Author: felipensp

vlib/v/checker/return.v

@@ -5,6 +5,21 @@ module checker
 import v.ast
 import v.pref
 
+// error_type_name returns a proper type name reference for error messages
+// ? => 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'
+	} else if exp_type == ast.void_type.set_flag(.option) {
+		'Option type'
+	} else {
+		'type `${c.table.type_to_str(exp_type)}`'
+	}
+}
+
 // TODO: non deferred
 fn (mut c Checker) return_stmt(mut node ast.Return) {
 	if c.table.cur_fn == unsafe { nil } {
@@ -22,6 +37,12 @@ fn (mut c Checker) return_stmt(mut node ast.Return) {
 	if node.exprs.len > 0 && c.table.cur_fn.return_type == ast.void_type {
 		c.error('unexpected argument, current function does not return anything', node.exprs[0].pos())
 		return
+	} else if node.exprs.len > 1 && c.table.cur_fn.return_type == ast.void_type.set_flag(.option) {
+		c.error('can only return `none` from an Option-only return function', node.exprs[0].pos())
+		return
+	} else if node.exprs.len > 1 && c.table.cur_fn.return_type == ast.void_type.set_flag(.result) {
+		c.error('functions with Result-only return types can only return an error', node.exprs[0].pos())
+		return
 	} else if node.exprs.len == 0 && !(c.expected_type == ast.void_type
 		|| expected_type_sym.kind == .void) {
 		stype := c.table.type_to_str(expected_type)
@@ -100,7 +121,7 @@ fn (mut c Checker) return_stmt(mut node ast.Return) {
 		c.warn('Option and Result types have been split, use `!Foo` to return errors',
 			node.pos)
 	} else if exp_is_result && got_types_0_idx == ast.none_type_idx {
-		c.warn('Option and Result types have been split, use `?Foo` to return none', node.pos)
+		c.warn('Option and Result types have been split, use `?` to return none', node.pos)
 	}
 	if (exp_is_option
 		&& got_types_0_idx in [ast.none_type_idx, ast.error_type_idx, option_type_idx])
@@ -143,13 +164,13 @@ fn (mut c Checker) return_stmt(mut node ast.Return) {
 		got_typ := c.unwrap_generic(got_types[i])
 		if got_typ.has_flag(.option) && got_typ.clear_flag(.option) != exp_type.clear_flag(.option) {
 			pos := node.exprs[expr_idxs[i]].pos()
-			c.error('cannot use `${c.table.type_to_str(got_typ)}` as type `${c.table.type_to_str(exp_type)}` in return argument',
+			c.error('cannot use `${c.table.type_to_str(got_typ)}` as ${c.error_type_name(exp_type)} in return argument',
 				pos)
 		}
 		if got_typ.has_flag(.result) && (!exp_type.has_flag(.result)
 			|| c.table.type_to_str(got_typ) != c.table.type_to_str(exp_type)) {
 			pos := node.exprs[expr_idxs[i]].pos()
-			c.error('cannot use `${c.table.type_to_str(got_typ)}` as type `${c.table.type_to_str(exp_type)}` in return argument',
+			c.error('cannot use `${c.table.type_to_str(got_typ)}` as ${c.error_type_name(exp_type)} in return argument',
 				pos)
 		}
 		if node.exprs[expr_idxs[i]] !is ast.ComptimeCall {
@@ -161,7 +182,7 @@ fn (mut c Checker) return_stmt(mut node ast.Return) {
 					if node.exprs[expr_idxs[i]] is ast.IntegerLiteral {
 						var := (node.exprs[expr_idxs[i]] as ast.IntegerLiteral).val
 						if var[0] == `-` {
-							c.note('cannot use a negative value as value of type `${c.table.type_to_str(exp_type)}` in return argument',
+							c.note('cannot use a negative value as value of ${c.error_type_name(exp_type)} in return argument',
 								pos)
 						}
 					}
@@ -194,7 +215,7 @@ fn (mut c Checker) return_stmt(mut node ast.Return) {
 				} else {
 					got_typ_sym.name
 				}
-				c.error('cannot use `${got_typ_name}` as type `${c.table.type_to_str(exp_type)}` in return argument',
+				c.error('cannot use `${got_typ_name}` as ${c.error_type_name(exp_type)} in return argument',
 					pos)
 			}
 		}

vlib/v/checker/tests/void_fn_multiple_ret_err.out

@@ -0,0 +1,47 @@
+vlib/v/checker/tests/void_fn_multiple_ret_err.vv:6:2: warning: Option and Result types have been split, use `?` to return none
+    4 | 
+    5 | fn foo_result_2() ! {
+    6 |     return none
+      |     ~~~~~~~~~~~
+    7 | }
+    8 |
+vlib/v/checker/tests/void_fn_multiple_ret_err.vv:2:9: error: functions with Result-only return types can only return an error
+    1 | fn foo_result_1() ! {
+    2 |     return none, 100
+      |            ~~~~
+    3 | }
+    4 |
+vlib/v/checker/tests/void_fn_multiple_ret_err.vv:6:9: error: cannot use `none` as Result type in return argument
+    4 | 
+    5 | fn foo_result_2() ! {
+    6 |     return none
+      |            ~~~~
+    7 | }
+    8 |
+vlib/v/checker/tests/void_fn_multiple_ret_err.vv:14:9: error: cannot use `int literal` as Result type in return argument
+   12 | 
+   13 | fn foo_result_4() ! {
+   14 |     return 1
+      |            ^
+   15 | }
+   16 |
+vlib/v/checker/tests/void_fn_multiple_ret_err.vv:21:9: error: cannot use `string` as type `!int` in return argument
+   19 | 
+   20 | fn foo_result_6() !int {
+   21 |     return ''
+      |            ~~
+   22 | }
+   23 |
+vlib/v/checker/tests/void_fn_multiple_ret_err.vv:25:9: error: can only return `none` from an Option-only return function
+   23 | 
+   24 | fn foo_option_1() ? {
+   25 |     return none, 100
+      |            ~~~~
+   26 | }
+   27 |
+vlib/v/checker/tests/void_fn_multiple_ret_err.vv:36:9: error: cannot use `int literal` as Option type in return argument
+   34 | 
+   35 | fn foo_option_3() ? {
+   36 |     return 1
+      |            ^
+   37 | }

vlib/v/checker/tests/void_fn_multiple_ret_err.vv

@@ -0,0 +1,37 @@
+fn foo_result_1() ! {
+	return none, 100
+}
+
+fn foo_result_2() ! {
+	return none
+}
+
+fn foo_result_3() ! {
+	return error('')
+}
+
+fn foo_result_4() ! {
+	return 1
+}
+
+fn foo_result_5() ! {
+}
+
+fn foo_result_6() !int {
+	return ''
+}
+
+fn foo_option_1() ? {
+	return none, 100
+}
+
+fn foo_option_2() ? {
+	return none
+}
+
+fn foo_option_3() ? {
+}
+
+fn foo_option_3() ? {
+	return 1
+}