From bc68c87f21c117797a77bc87a71cf98144d6a58b Mon Sep 17 00:00:00 2001 From: Turiiya <34311583+ttytm@users.noreply.github.com> Date: Wed, 27 Mar 2024 13:12:08 +0100 Subject: [PATCH] checker: optimize option and result typ check, add more typinfo to error details (#21105) --- vlib/v/checker/checker.v | 20 +++++++------------ ..._type_cast_option_result_unhandled_err.out | 2 +- .../as_cast_option_result_unhandled_err.out | 2 +- vlib/v/checker/tests/option_type_call_err.out | 2 +- .../tests/result_missing_propagate_err.out | 6 +++--- vlib/v/checker/tests/result_type_call_err.out | 4 ++-- .../struct_field_init_with_result_err.out | 4 ++-- .../tests/struct_init_field_result_err.out | 2 +- 8 files changed, 18 insertions(+), 24 deletions(-) diff --git a/vlib/v/checker/checker.v b/vlib/v/checker/checker.v index 0b4424b8c..e93e08666 100644 --- a/vlib/v/checker/checker.v +++ b/vlib/v/checker/checker.v @@ -1161,24 +1161,18 @@ fn (mut c Checker) check_expr_option_or_result_call(expr ast.Expr, ret_type ast. } } if expr_ret_type.has_option_or_result() { - return_modifier_kind := if expr_ret_type.has_flag(.option) { - 'an Option' - } else { - 'a Result' - } - return_modifier := if expr_ret_type.has_flag(.option) { '?' } else { '!' } - if expr_ret_type.has_flag(.result) && expr.or_block.kind == .absent { + if expr.or_block.kind == .absent && expr_ret_type.has_flag(.result) { + ret_sym := c.table.sym(expr_ret_type) + ret_typ_tok := if expr_ret_type.has_flag(.option) { '?' } else { '!' } if c.inside_defer { - c.error('${expr.name}() returns ${return_modifier_kind}, so it should have an `or {}` block at the end', + c.error('${expr.name}() returns `${ret_typ_tok}${ret_sym.name}`, so it should have an `or {}` block at the end', expr.pos) } else { - c.error('${expr.name}() returns ${return_modifier_kind}, so it should have either an `or {}` block, or `${return_modifier}` at the end', + c.error('${expr.name}() returns `${ret_typ_tok}${ret_sym.name}`, so it should have either an `or {}` block, or `${ret_typ_tok}` at the end', expr.pos) } - } else { - if expr.or_block.kind != .absent { - c.check_or_expr(expr.or_block, ret_type, expr_ret_type, expr) - } + } else if expr.or_block.kind != .absent { + c.check_or_expr(expr.or_block, ret_type, expr_ret_type, expr) } return ret_type.clear_flag(.result) } else { diff --git a/vlib/v/checker/tests/alias_type_cast_option_result_unhandled_err.out b/vlib/v/checker/tests/alias_type_cast_option_result_unhandled_err.out index fc3eb2cfe..2eb907223 100644 --- a/vlib/v/checker/tests/alias_type_cast_option_result_unhandled_err.out +++ b/vlib/v/checker/tests/alias_type_cast_option_result_unhandled_err.out @@ -1,4 +1,4 @@ -vlib/v/checker/tests/alias_type_cast_option_result_unhandled_err.vv:15:12: error: ret_abc_result() returns a Result, so it should have either an `or {}` block, or `!` at the end +vlib/v/checker/tests/alias_type_cast_option_result_unhandled_err.vv:15:12: error: ret_abc_result() returns `!Abc`, so it should have either an `or {}` block, or `!` at the end 13 | } 14 | 15 | a := Alias(ret_abc_result()) diff --git a/vlib/v/checker/tests/as_cast_option_result_unhandled_err.out b/vlib/v/checker/tests/as_cast_option_result_unhandled_err.out index 291b9cfd6..36aa998cb 100644 --- a/vlib/v/checker/tests/as_cast_option_result_unhandled_err.out +++ b/vlib/v/checker/tests/as_cast_option_result_unhandled_err.out @@ -1,4 +1,4 @@ -vlib/v/checker/tests/as_cast_option_result_unhandled_err.vv:11:6: error: ret_sum_result() returns a Result, so it should have either an `or {}` block, or `!` at the end +vlib/v/checker/tests/as_cast_option_result_unhandled_err.vv:11:6: error: ret_sum_result() returns `!Sum`, so it should have either an `or {}` block, or `!` at the end 9 | } 10 | 11 | _ := ret_sum_result() as int diff --git a/vlib/v/checker/tests/option_type_call_err.out b/vlib/v/checker/tests/option_type_call_err.out index a785626be..90de5d9ee 100644 --- a/vlib/v/checker/tests/option_type_call_err.out +++ b/vlib/v/checker/tests/option_type_call_err.out @@ -1,4 +1,4 @@ -vlib/v/checker/tests/option_type_call_err.vv:4:5: error: os.ls() returns a Result, so it should have either an `or {}` block, or `!` at the end +vlib/v/checker/tests/option_type_call_err.vv:4:5: error: os.ls() returns `![]string`, so it should have either an `or {}` block, or `!` at the end 2 | 3 | fn main() { 4 | os.ls('.').filter(it.ends_with('.v')) or { return } diff --git a/vlib/v/checker/tests/result_missing_propagate_err.out b/vlib/v/checker/tests/result_missing_propagate_err.out index 38ec89df1..f3c108ad7 100644 --- a/vlib/v/checker/tests/result_missing_propagate_err.out +++ b/vlib/v/checker/tests/result_missing_propagate_err.out @@ -1,7 +1,7 @@ -vlib/v/checker/tests/result_missing_propagate_err.vv:6:7: error: result() returns a Result, so it should have either an `or {}` block, or `!` at the end - 4 | +vlib/v/checker/tests/result_missing_propagate_err.vv:6:7: error: result() returns `!int`, so it should have either an `or {}` block, or `!` at the end + 4 | 5 | fn main() { 6 | a := result() | ~~~~~~~~ 7 | println(a) - 8 | } \ No newline at end of file + 8 | } diff --git a/vlib/v/checker/tests/result_type_call_err.out b/vlib/v/checker/tests/result_type_call_err.out index e4c945d5d..60a05993d 100644 --- a/vlib/v/checker/tests/result_type_call_err.out +++ b/vlib/v/checker/tests/result_type_call_err.out @@ -1,4 +1,4 @@ -vlib/v/checker/tests/result_type_call_err.vv:12:2: error: new_foo() returns a Result, so it should have either an `or {}` block, or `!` at the end +vlib/v/checker/tests/result_type_call_err.vv:12:2: error: new_foo() returns `!Foo`, so it should have either an `or {}` block, or `!` at the end 10 | 11 | fn main() { 12 | new_foo().foo() @@ -10,7 +10,7 @@ vlib/v/checker/tests/result_type_call_err.vv:12:2: error: Result type cannot be 12 | new_foo().foo() | ~~~~~~~~~ 13 | } -vlib/v/checker/tests/result_type_call_err.vv:12:12: error: foo() returns a Result, so it should have either an `or {}` block, or `!` at the end +vlib/v/checker/tests/result_type_call_err.vv:12:12: error: foo() returns `!Foo`, so it should have either an `or {}` block, or `!` at the end 10 | 11 | fn main() { 12 | new_foo().foo() diff --git a/vlib/v/checker/tests/struct_field_init_with_result_err.out b/vlib/v/checker/tests/struct_field_init_with_result_err.out index f21302d6e..74f8f6e87 100644 --- a/vlib/v/checker/tests/struct_field_init_with_result_err.out +++ b/vlib/v/checker/tests/struct_field_init_with_result_err.out @@ -1,5 +1,5 @@ -vlib/v/checker/tests/struct_field_init_with_result_err.vv:10:18: error: example() returns a Result, so it should have either an `or {}` block, or `!` at the end - 8 | +vlib/v/checker/tests/struct_field_init_with_result_err.vv:10:18: error: example() returns `!int`, so it should have either an `or {}` block, or `!` at the end + 8 | 9 | fn main() { 10 | println(Example{example()}) | ~~~~~~~~~ diff --git a/vlib/v/checker/tests/struct_init_field_result_err.out b/vlib/v/checker/tests/struct_init_field_result_err.out index be3a50daf..0262d6338 100644 --- a/vlib/v/checker/tests/struct_init_field_result_err.out +++ b/vlib/v/checker/tests/struct_init_field_result_err.out @@ -1,4 +1,4 @@ -vlib/v/checker/tests/struct_init_field_result_err.vv:14:15: error: time.parse() returns a Result, so it should have either an `or {}` block, or `!` at the end +vlib/v/checker/tests/struct_init_field_result_err.vv:14:15: error: time.parse() returns `!time.Time`, so it should have either an `or {}` block, or `!` at the end 12 | _ := Payload{ 13 | sub: "1234567890", 14 | exp: time.parse("2019-01-01 00:00:00") -- 2.39.5