From dd9f9c271892559155fdddf1f35bb881dfb30d96 Mon Sep 17 00:00:00 2001 From: yuyi Date: Wed, 17 Mar 2021 08:42:51 +0800 Subject: [PATCH] checker: check array.sort(a < b) (#9321) --- vlib/builtin/sorting_test.v | 22 +++---- vlib/v/checker/checker.v | 83 +++++++++++++------------ vlib/v/checker/tests/array_sort_err.out | 23 ++++++- vlib/v/checker/tests/array_sort_err.vv | 3 + 4 files changed, 79 insertions(+), 52 deletions(-) diff --git a/vlib/builtin/sorting_test.v b/vlib/builtin/sorting_test.v index c5c726c5a5..9d7840ae83 100644 --- a/vlib/builtin/sorting_test.v +++ b/vlib/builtin/sorting_test.v @@ -24,18 +24,18 @@ fn test_sorting_primitives_with_condition_expression() { assert x == ['9', '87', '654', '3210'] } -fn get_score(word string) int { - mut total := 0 - for letter in word { - total += int(letter) - 97 - } - return total -} +// fn get_score(word string) int { +// mut total := 0 +// for letter in word { +// total += int(letter) - 97 +// } +// return total +// } -fn test_sorting_with_fn_call_in_condition_expression() { - mut words := ['aaaa', 'a', 'b', 'foo', 'bar'] - words.sort(get_score(a) < get_score(b)) -} +// fn test_sorting_with_fn_call_in_condition_expression() { +// mut words := ['aaaa', 'a', 'b', 'foo', 'bar'] +// words.sort(get_score(a) < get_score(b)) +// } fn mysort(mut a []int) { a.sort() diff --git a/vlib/v/checker/checker.v b/vlib/v/checker/checker.v index abaf900d95..5df8ed0bd1 100644 --- a/vlib/v/checker/checker.v +++ b/vlib/v/checker/checker.v @@ -1660,52 +1660,55 @@ pub fn (mut c Checker) call_method(mut call_expr ast.CallExpr) table.Type { fn (mut c Checker) call_array_builtin_method(mut call_expr ast.CallExpr, left_type table.Type, left_type_sym table.TypeSymbol) table.Type { method_name := call_expr.name mut elem_typ := table.void_type - is_filter_map := method_name in ['filter', 'map'] - is_sort := method_name == 'sort' - is_slice := method_name == 'slice' - is_wait := method_name == 'wait' - if is_slice && !c.is_builtin_mod { + if method_name == 'slice' && !c.is_builtin_mod { c.error('.slice() is a private method, use `x[start..end]` instead', call_expr.pos) } - if is_filter_map || is_sort || is_wait { - array_info := left_type_sym.info as table.Array - if is_filter_map { - // position of `it` doesn't matter - scope_register_it(mut call_expr.scope, call_expr.pos, array_info.elem_type) - } else if is_sort { - c.fail_if_immutable(call_expr.left) - // position of `a` and `b` doesn't matter, they're the same - scope_register_ab(mut call_expr.scope, call_expr.pos, array_info.elem_type) + array_info := left_type_sym.info as table.Array + elem_typ = array_info.elem_type + if method_name in ['filter', 'map'] { + // position of `it` doesn't matter + scope_register_it(mut call_expr.scope, call_expr.pos, elem_typ) + } else if method_name == 'sort' { + c.fail_if_immutable(call_expr.left) + // position of `a` and `b` doesn't matter, they're the same + scope_register_a_b(mut call_expr.scope, call_expr.pos, elem_typ) - if call_expr.args.len > 1 { - c.error('expected 0 or 1 argument, but got $call_expr.args.len', call_expr.pos) - } - // Verify `.sort(a < b)` - if call_expr.args.len > 0 { - if call_expr.args[0].expr !is ast.InfixExpr { - c.error( - '`.sort()` requires a `<` or `>` comparison as the first and only argument' + - '\ne.g. `users.sort(a.id < b.id)`', call_expr.pos) + if call_expr.args.len > 1 { + c.error('expected 0 or 1 argument, but got $call_expr.args.len', call_expr.pos) + } else if call_expr.args.len == 1 { + if call_expr.args[0].expr is ast.InfixExpr { + if call_expr.args[0].expr.op !in [.gt, .lt] { + c.error('`.sort()` can only use `<` or `>` comparison', call_expr.pos) } + left_name := '${call_expr.args[0].expr.left}'[0] + right_name := '${call_expr.args[0].expr.right}'[0] + if left_name !in [`a`, `b`] || right_name !in [`a`, `b`] { + c.error('`.sort()` can only use `a` or `b` as argument, e.g. `arr.sort(a < b)`', + call_expr.pos) + } else if left_name == right_name { + c.error('`.sort()` cannot use same argument', call_expr.pos) + } + } else { + c.error( + '`.sort()` requires a `<` or `>` comparison as the first and only argument' + + '\ne.g. `users.sort(a.id < b.id)`', call_expr.pos) } } - elem_typ = array_info.elem_type - if is_wait { - elem_sym := c.table.get_type_symbol(elem_typ) - if elem_sym.kind == .thread { - if call_expr.args.len != 0 { - c.error('`.wait()` does not have any arguments', call_expr.args[0].pos) - } - thread_ret_type := elem_sym.thread_info().return_type - if thread_ret_type.has_flag(.optional) { - c.error('`.wait()` cannot be called for an array when thread functions return optionals. Iterate over the arrays elements instead and handle each returned optional with `or`.', - call_expr.pos) - } - call_expr.return_type = c.table.find_or_register_array(thread_ret_type) - } else { - c.error('`$left_type_sym.name` has no method `wait()` (only thread handles and arrays of them have)', - call_expr.left.position()) + } else if method_name == 'wait' { + elem_sym := c.table.get_type_symbol(elem_typ) + if elem_sym.kind == .thread { + if call_expr.args.len != 0 { + c.error('`.wait()` does not have any arguments', call_expr.args[0].pos) } + thread_ret_type := elem_sym.thread_info().return_type + if thread_ret_type.has_flag(.optional) { + c.error('`.wait()` cannot be called for an array when thread functions return optionals. Iterate over the arrays elements instead and handle each returned optional with `or`.', + call_expr.pos) + } + call_expr.return_type = c.table.find_or_register_array(thread_ret_type) + } else { + c.error('`$left_type_sym.name` has no method `wait()` (only thread handles and arrays of them have)', + call_expr.left.position()) } } // map/filter are supposed to have 1 arg only @@ -3002,7 +3005,7 @@ fn scope_register_it(mut s ast.Scope, pos token.Position, typ table.Type) { }) } -fn scope_register_ab(mut s ast.Scope, pos token.Position, typ table.Type) { +fn scope_register_a_b(mut s ast.Scope, pos token.Position, typ table.Type) { s.register(ast.Var{ name: 'a' pos: pos diff --git a/vlib/v/checker/tests/array_sort_err.out b/vlib/v/checker/tests/array_sort_err.out index bc399e9303..04c3dbf8f3 100644 --- a/vlib/v/checker/tests/array_sort_err.out +++ b/vlib/v/checker/tests/array_sort_err.out @@ -3,4 +3,25 @@ vlib/v/checker/tests/array_sort_err.vv:3:6: error: expected 0 or 1 argument, but 2 | mut arr := [3, 2, 1] 3 | arr.sort(a < b, a) | ~~~~~~~~~~~~~~ - 4 | } + 4 | arr.sort(a == b) + 5 | arr.sort(a > a) +vlib/v/checker/tests/array_sort_err.vv:4:9: error: `.sort()` can only use `<` or `>` comparison + 2 | mut arr := [3, 2, 1] + 3 | arr.sort(a < b, a) + 4 | arr.sort(a == b) + | ~~~~~~~~~~~~ + 5 | arr.sort(a > a) + 6 | arr.sort(c > d) +vlib/v/checker/tests/array_sort_err.vv:5:9: error: `.sort()` cannot use same argument + 3 | arr.sort(a < b, a) + 4 | arr.sort(a == b) + 5 | arr.sort(a > a) + | ~~~~~~~~~~~ + 6 | arr.sort(c > d) + 7 | } +vlib/v/checker/tests/array_sort_err.vv:6:9: error: `.sort()` can only use `a` or `b` as argument, e.g. `arr.sort(a < b)` + 4 | arr.sort(a == b) + 5 | arr.sort(a > a) + 6 | arr.sort(c > d) + | ~~~~~~~~~~~ + 7 | } diff --git a/vlib/v/checker/tests/array_sort_err.vv b/vlib/v/checker/tests/array_sort_err.vv index 01e5ebcecf..821cfd018f 100644 --- a/vlib/v/checker/tests/array_sort_err.vv +++ b/vlib/v/checker/tests/array_sort_err.vv @@ -1,4 +1,7 @@ fn main() { mut arr := [3, 2, 1] arr.sort(a < b, a) + arr.sort(a == b) + arr.sort(a > a) + arr.sort(c > d) }