From 69b4b41273d5e8c8caa5ca75896b9ba402609bbd Mon Sep 17 00:00:00 2001 From: Swastik Baranwal Date: Wed, 21 Jan 2026 22:57:27 +0530 Subject: [PATCH] checker: disallow `foo()? == foo` where foo returns `!string` (fix #26383) (#26403) --- cmd/tools/vdoc/document/doc_private_fn_test.v | 10 +++---- vlib/datatypes/linked_list_test.v | 16 +++++------ vlib/net/http/header_test.v | 28 +++++++++---------- vlib/strconv/atoi_test.v | 2 +- vlib/v/checker/checker.v | 7 +++++ .../tests/fn_result_option_call_infix_err.out | 6 ++++ .../tests/fn_result_option_call_infix_err.vv | 8 ++++++ ...wrapped_option_result_in_operation_err.out | 21 ++++++++++++++ .../tests/unwrapped_result_infix_err.out | 7 +++++ .../ifs/ifexpr_with_option_result_test.v | 2 +- ...match_expr_with_auto_promote_number_test.v | 4 +-- 11 files changed, 80 insertions(+), 31 deletions(-) create mode 100644 vlib/v/checker/tests/fn_result_option_call_infix_err.out create mode 100644 vlib/v/checker/tests/fn_result_option_call_infix_err.vv diff --git a/cmd/tools/vdoc/document/doc_private_fn_test.v b/cmd/tools/vdoc/document/doc_private_fn_test.v index be55a4ef5..208fc4433 100644 --- a/cmd/tools/vdoc/document/doc_private_fn_test.v +++ b/cmd/tools/vdoc/document/doc_private_fn_test.v @@ -28,7 +28,7 @@ fn test_get_parent_mod_current_folder() { fn test_get_parent_mod_on_temp_dir() { // TODO: fix this on windows $if !windows { - assert get_parent_mod(os.temp_dir())? == '' + assert get_parent_mod(os.temp_dir())! == '' } } @@ -37,9 +37,9 @@ fn test_get_parent_mod_normal_cases() { assert err.msg() == 'No V files found.' '---' } - assert get_parent_mod(os.join_path(@VMODROOT, 'vlib', 'v', 'token'))? == 'v' - assert get_parent_mod(os.join_path(@VMODROOT, 'vlib', 'os', 'os.v'))? == 'os' - assert get_parent_mod(os.join_path(@VMODROOT, 'cmd'))? == '' + assert get_parent_mod(os.join_path(@VMODROOT, 'vlib', 'v', 'token'))! == 'v' + assert get_parent_mod(os.join_path(@VMODROOT, 'vlib', 'os', 'os.v'))! == 'os' + assert get_parent_mod(os.join_path(@VMODROOT, 'cmd'))! == '' assert get_parent_mod(os.join_path(@VMODROOT, 'cmd', 'tools', 'modules', 'testing', - 'common.v'))? == 'tools.modules.testing' + 'common.v'))! == 'tools.modules.testing' } diff --git a/vlib/datatypes/linked_list_test.v b/vlib/datatypes/linked_list_test.v index ff24f42ea..81abc4516 100644 --- a/vlib/datatypes/linked_list_test.v +++ b/vlib/datatypes/linked_list_test.v @@ -159,10 +159,10 @@ fn test_linked_list_separate_iterators() { mut it1 := list.iterator() mut it2 := list.iterator() mut it3 := list.iterator() - assert it1.next()! == 1 - assert it1.next()! == 2 - assert it1.next()! == 3 - assert it2.next()! == 1 + assert it1.next()? == 1 + assert it1.next()? == 2 + assert it1.next()? == 3 + assert it2.next()? == 1 if _ := it1.next() { assert false } else { @@ -173,8 +173,8 @@ fn test_linked_list_separate_iterators() { } else { assert true } - assert it2.next()! == 2 - assert it2.next()! == 3 + assert it2.next()? == 2 + assert it2.next()? == 3 if _ := it2.next() { assert false } else { @@ -202,10 +202,10 @@ fn test_linked_list_map() { }) println(foo) mut iter := foo.field.iterator() - assert iter.next()! == { + assert iter.next()? == { 'one': 1 } - assert iter.next()! == { + assert iter.next()? == { 'two': 2 } } diff --git a/vlib/net/http/header_test.v b/vlib/net/http/header_test.v index ef126a24c..966e11eb5 100644 --- a/vlib/net/http/header_test.v +++ b/vlib/net/http/header_test.v @@ -123,9 +123,9 @@ fn test_contains_custom() { fn test_get_custom() { mut h := new_header() h.add_custom('Hello', 'world')! - assert h.get_custom('hello')? == 'world' - assert h.get_custom('HELLO')? == 'world' - assert h.get_custom('Hello', exact: true)? == 'world' + assert h.get_custom('hello')! == 'world' + assert h.get_custom('HELLO')! == 'world' + assert h.get_custom('Hello', exact: true)! == 'world' if _ := h.get_custom('hello', exact: true) { // should be none assert false @@ -140,8 +140,8 @@ fn test_starting_with() { mut h := new_header() h.add_custom('Hello-1', 'world')! h.add_custom('Hello-21', 'world')! - assert h.starting_with('Hello-')? == 'Hello-1' - assert h.starting_with('Hello-2')? == 'Hello-21' + assert h.starting_with('Hello-')! == 'Hello-1' + assert h.starting_with('Hello-2')! == 'Hello-21' } fn test_custom_values() { @@ -347,35 +347,35 @@ fn test_header_join() { assert h3.contains_custom('foo') } -fn parse_headers_test(s string, expected map[string]string) ? { - assert parse_headers(s)? == new_custom_header_from_map(expected)? +fn parse_headers_test(s string, expected map[string]string) ! { + assert parse_headers(s)! == new_custom_header_from_map(expected)! } fn test_parse_headers() ! { parse_headers_test('foo: bar', { 'foo': 'bar' - })? + })! parse_headers_test('foo: \t bar', { 'foo': 'bar' - })? + })! parse_headers_test('foo: bar\r\n\tbaz', { 'foo': 'bar baz' - })? + })! parse_headers_test('foo: bar \r\n\tbaz\r\n buzz', { 'foo': 'bar baz buzz' - })? + })! parse_headers_test('foo: bar\r\nbar:baz', { 'foo': 'bar' 'bar': 'baz' - })? + })! parse_headers_test('foo: bar\r\nbar:baz\r\n', { 'foo': 'bar' 'bar': 'baz' - })? + })! parse_headers_test('foo: bar\r\nbar:baz\r\n\r\n', { 'foo': 'bar' 'bar': 'baz' - })? + })! assert parse_headers('foo: bar\r\nfoo:baz')!.custom_values('foo') == ['bar', 'baz'] if x := parse_headers(' oops: oh no') { diff --git a/vlib/strconv/atoi_test.v b/vlib/strconv/atoi_test.v index aaf1e1bf2..e54ea520b 100644 --- a/vlib/strconv/atoi_test.v +++ b/vlib/strconv/atoi_test.v @@ -325,7 +325,7 @@ fn test_parse_int() { assert strconv.parse_int('2147483648', 10, 32)! == 2147483647 assert strconv.parse_int('9223372036854775807', 10, 64)! == 9223372036854775807 assert strconv.parse_int('9223372036854775808', 10, 64)! == 9223372036854775807 - assert strconv.parse_int('baobab', 36, 64)? == 683058467 + assert strconv.parse_int('baobab', 36, 64)! == 683058467 // Invalid bit sizes if x := strconv.parse_int('123', 10, -1) { println(x) diff --git a/vlib/v/checker/checker.v b/vlib/v/checker/checker.v index b2704ec07..2a8fb3690 100644 --- a/vlib/v/checker/checker.v +++ b/vlib/v/checker/checker.v @@ -1582,6 +1582,10 @@ fn (mut c Checker) check_expr_option_or_result_call(expr ast.Expr, ret_type ast. ast.ParExpr { c.check_expr_option_or_result_call(expr.expr, ret_type) } + ast.InfixExpr { + c.check_expr_option_or_result_call(expr.left, ret_type) + c.check_expr_option_or_result_call(expr.right, ret_type) + } else {} } return ret_type @@ -1713,6 +1717,9 @@ fn (mut c Checker) check_or_last_stmt(mut stmt ast.Stmt, ret_type ast.Type, expr if ret_type.has_flag(.generic) { return } + if ret_type.clear_option_and_result() != expr_return_type { + return + } c.error('wrong return type `${type_name}` in the `or {}` block, expected `${expected_type_name}`', stmt.expr.pos()) } diff --git a/vlib/v/checker/tests/fn_result_option_call_infix_err.out b/vlib/v/checker/tests/fn_result_option_call_infix_err.out new file mode 100644 index 000000000..98d59639b --- /dev/null +++ b/vlib/v/checker/tests/fn_result_option_call_infix_err.out @@ -0,0 +1,6 @@ +vlib/v/checker/tests/fn_result_option_call_infix_err.vv:7:14: error: propagating a Result like an Option is deprecated, use `foo()!` instead of `foo()?` + 5 | fn main() { + 6 | assert foo()! == 'foo' + 7 | assert foo()? == 'foo' + | ^ + 8 | } diff --git a/vlib/v/checker/tests/fn_result_option_call_infix_err.vv b/vlib/v/checker/tests/fn_result_option_call_infix_err.vv new file mode 100644 index 000000000..dcc3b9b84 --- /dev/null +++ b/vlib/v/checker/tests/fn_result_option_call_infix_err.vv @@ -0,0 +1,8 @@ +fn foo() !string { + return 'foo' +} + +fn main() { + assert foo()! == 'foo' + assert foo()? == 'foo' +} diff --git a/vlib/v/checker/tests/unwrapped_option_result_in_operation_err.out b/vlib/v/checker/tests/unwrapped_option_result_in_operation_err.out index 0708c848e..f135be2df 100644 --- a/vlib/v/checker/tests/unwrapped_option_result_in_operation_err.out +++ b/vlib/v/checker/tests/unwrapped_option_result_in_operation_err.out @@ -5,6 +5,13 @@ vlib/v/checker/tests/unwrapped_option_result_in_operation_err.vv:3:6: error: unw | ~~~~~~~~~~~~~~~~~~~~~~~~ 4 | _ = return_string_or_error() in list 5 | _ = return_string_or_none() in list +vlib/v/checker/tests/unwrapped_option_result_in_operation_err.vv:3:6: error: return_string_or_error() returns `!string`, so it should have either an `or {}` block, or `!` at the end + 1 | fn main() { + 2 | list := ['string'] + 3 | _ = return_string_or_error() !in list + | ~~~~~~~~~~~~~~~~~~~~~~~~ + 4 | _ = return_string_or_error() in list + 5 | _ = return_string_or_none() in list vlib/v/checker/tests/unwrapped_option_result_in_operation_err.vv:4:6: error: unwrapped Result cannot be used with `in` 2 | list := ['string'] 3 | _ = return_string_or_error() !in list @@ -12,6 +19,13 @@ vlib/v/checker/tests/unwrapped_option_result_in_operation_err.vv:4:6: error: unw | ~~~~~~~~~~~~~~~~~~~~~~~~ 5 | _ = return_string_or_none() in list 6 | _ = return_string_or_error() !in list +vlib/v/checker/tests/unwrapped_option_result_in_operation_err.vv:4:6: error: return_string_or_error() returns `!string`, so it should have either an `or {}` block, or `!` at the end + 2 | list := ['string'] + 3 | _ = return_string_or_error() !in list + 4 | _ = return_string_or_error() in list + | ~~~~~~~~~~~~~~~~~~~~~~~~ + 5 | _ = return_string_or_none() in list + 6 | _ = return_string_or_error() !in list vlib/v/checker/tests/unwrapped_option_result_in_operation_err.vv:5:6: error: unwrapped Option cannot be used with `in` 3 | _ = return_string_or_error() !in list 4 | _ = return_string_or_error() in list @@ -26,3 +40,10 @@ vlib/v/checker/tests/unwrapped_option_result_in_operation_err.vv:6:6: error: unw | ~~~~~~~~~~~~~~~~~~~~~~~~ 7 | } 8 | +vlib/v/checker/tests/unwrapped_option_result_in_operation_err.vv:6:6: error: return_string_or_error() returns `!string`, so it should have either an `or {}` block, or `!` at the end + 4 | _ = return_string_or_error() in list + 5 | _ = return_string_or_none() in list + 6 | _ = return_string_or_error() !in list + | ~~~~~~~~~~~~~~~~~~~~~~~~ + 7 | } + 8 | diff --git a/vlib/v/checker/tests/unwrapped_result_infix_err.out b/vlib/v/checker/tests/unwrapped_result_infix_err.out index 7ef97c8df..45307b1ce 100644 --- a/vlib/v/checker/tests/unwrapped_result_infix_err.out +++ b/vlib/v/checker/tests/unwrapped_result_infix_err.out @@ -5,3 +5,10 @@ vlib/v/checker/tests/unwrapped_result_infix_err.vv:7:9: error: unwrapped Result | ~~~~~~ 8 | } 9 | +vlib/v/checker/tests/unwrapped_result_infix_err.vv:7:9: error: f() returns `!bool`, so it should have either an `or {}` block, or `!` at the end + 5 | fn g() ! { + 6 | assert f('1')! == true + 7 | assert f('1') == true + | ~~~~~~ + 8 | } + 9 | diff --git a/vlib/v/tests/conditions/ifs/ifexpr_with_option_result_test.v b/vlib/v/tests/conditions/ifs/ifexpr_with_option_result_test.v index b88f67110..d7e6a928d 100644 --- a/vlib/v/tests/conditions/ifs/ifexpr_with_option_result_test.v +++ b/vlib/v/tests/conditions/ifs/ifexpr_with_option_result_test.v @@ -7,5 +7,5 @@ fn comptime_ret_result() !string { } fn test_main() { - assert comptime_ret_result()? == 'not debug' + assert comptime_ret_result()! == 'not debug' } diff --git a/vlib/v/tests/conditions/matches/match_expr_with_auto_promote_number_test.v b/vlib/v/tests/conditions/matches/match_expr_with_auto_promote_number_test.v index b15b4801d..c2d255be8 100644 --- a/vlib/v/tests/conditions/matches/match_expr_with_auto_promote_number_test.v +++ b/vlib/v/tests/conditions/matches/match_expr_with_auto_promote_number_test.v @@ -97,6 +97,6 @@ fn string_to_i64(s string) ?i64 { } fn test_match_expr_with_auto_promote_number() { - assert string_to_i64('19P')! == 19 * peta - assert string_to_i64('18T')! == 18 * terra + assert string_to_i64('19P')? == 19 * peta + assert string_to_i64('18T')? == 18 * terra } -- 2.39.5