From ad162cd6fc1d2c5c2299252206c9cae6c31773af Mon Sep 17 00:00:00 2001 From: Swastik Baranwal Date: Fri, 19 Feb 2021 14:53:13 +0530 Subject: [PATCH] checker: stricter `unknown type` checks, show better suggestions (#8816) --- vlib/fontstash/fontstash_structs.v | 2 +- vlib/sqlite/sqlite.v | 2 +- vlib/time/time_windows.c.v | 2 +- vlib/v/checker/checker.v | 163 ++++++------------ .../v/checker/tests/any_int_float_ban_err.out | 6 +- .../v/checker/tests/filter_on_non_arr_err.out | 5 +- .../checker/tests/import_symbol_type_err.out | 3 +- .../tests/interface_return_parameter_err.out | 14 ++ .../tests/interface_return_parameter_err.vv | 4 + vlib/v/checker/tests/map_unknown_value.out | 2 +- .../tests/unknown_array_element_type_b.out | 5 +- .../tests/unknown_method_suggest_name.out | 2 +- vlib/v/table/table.v | 33 +++- vlib/v/util/suggestions.v | 19 +- 14 files changed, 120 insertions(+), 142 deletions(-) create mode 100644 vlib/v/checker/tests/interface_return_parameter_err.out create mode 100644 vlib/v/checker/tests/interface_return_parameter_err.vv diff --git a/vlib/fontstash/fontstash_structs.v b/vlib/fontstash/fontstash_structs.v index b851e5eec0..83daa52bde 100644 --- a/vlib/fontstash/fontstash_structs.v +++ b/vlib/fontstash/fontstash_structs.v @@ -67,7 +67,7 @@ pub struct C.FONStextIter { codepoint u32 isize i16 iblur i16 - font &FONSfont + font &C.FONSfont prevGlyphIndex int str byteptr next byteptr diff --git a/vlib/sqlite/sqlite.v b/vlib/sqlite/sqlite.v index 2ed929011a..4902e852e7 100644 --- a/vlib/sqlite/sqlite.v +++ b/vlib/sqlite/sqlite.v @@ -54,7 +54,7 @@ fn C.sqlite3_column_text(&C.sqlite3_stmt, int) byteptr fn C.sqlite3_column_int(&C.sqlite3_stmt, int) int -fn C.sqlite3_column_int64(&C.sqlite3_stmt, int) int64 +fn C.sqlite3_column_int64(&C.sqlite3_stmt, int) i64 fn C.sqlite3_column_double(&C.sqlite3_stmt, int) f64 diff --git a/vlib/time/time_windows.c.v b/vlib/time/time_windows.c.v index 2e5f7b1660..021207976f 100644 --- a/vlib/time/time_windows.c.v +++ b/vlib/time/time_windows.c.v @@ -50,7 +50,7 @@ struct C.timespec { tv_nsec i64 } -fn C._mkgmtime(&C.tm) time_t +fn C._mkgmtime(&C.tm) C.time_t fn C.QueryPerformanceCounter(&u64) C.BOOL diff --git a/vlib/v/checker/checker.v b/vlib/v/checker/checker.v index a11b433ae8..a6bd20ab9a 100644 --- a/vlib/v/checker/checker.v +++ b/vlib/v/checker/checker.v @@ -338,49 +338,21 @@ pub fn (mut c Checker) interface_decl(decl ast.InterfaceDecl) { c.check_valid_pascal_case(decl.name, 'interface name', decl.pos) for method in decl.methods { c.check_valid_snake_case(method.name, 'method name', method.pos) + if method.return_type != table.Type(0) { + c.ensure_type_exists(method.return_type, method.pos) or { return } + } + for param in method.params { + c.ensure_type_exists(param.typ, param.pos) or { return } + } } - // TODO: copy pasta from StructDecl for i, field in decl.fields { c.check_valid_snake_case(field.name, 'field name', field.pos) - sym := c.table.get_type_symbol(field.typ) + c.ensure_type_exists(field.typ, field.pos) or { return } for j in 0 .. i { if field.name == decl.fields[j].name { c.error('field name `$field.name` duplicate', field.pos) } } - if sym.kind == .placeholder && !sym.name.starts_with('C.') { - c.error(util.new_suggestion(sym.name, c.table.known_type_names()).say('unknown type `$sym.name`'), - field.type_pos) - } - // Separate error condition for `int_literal` and `float_literal` because `util.suggestion` may give different - // suggestions due to f32 comparision issue. - if sym.kind in [.int_literal, .float_literal] { - msg := if sym.kind == .int_literal { - 'unknown type `$sym.name`.\nDid you mean `int`?' - } else { - 'unknown type `$sym.name`.\nDid you mean `f64`?' - } - c.error(msg, field.type_pos) - } - if sym.kind == .array { - array_info := sym.array_info() - elem_sym := c.table.get_type_symbol(array_info.elem_type) - if elem_sym.kind == .placeholder { - c.error(util.new_suggestion(elem_sym.name, c.table.known_type_names()).say('unknown type `$elem_sym.name`'), - field.type_pos) - } - } - if sym.kind == .map { - info := sym.map_info() - key_sym := c.table.get_type_symbol(info.key_type) - value_sym := c.table.get_type_symbol(info.value_type) - if key_sym.kind == .placeholder { - c.error('unknown type `$key_sym.name`', field.type_pos) - } - if value_sym.kind == .placeholder { - c.error('unknown type `$value_sym.name`', field.type_pos) - } - } } } @@ -407,6 +379,7 @@ pub fn (mut c Checker) struct_decl(mut decl ast.StructDecl) { } } for i, field in decl.fields { + c.ensure_type_exists(field.typ, field.type_pos) or { return } if decl.language == .v { c.check_valid_snake_case(field.name, 'field name', field.pos) } @@ -416,45 +389,12 @@ pub fn (mut c Checker) struct_decl(mut decl ast.StructDecl) { c.error('field name `$field.name` duplicate', field.pos) } } - if sym.kind == .placeholder && decl.language != .c && !sym.name.starts_with('C.') { - c.error(util.new_suggestion(sym.name, c.table.known_type_names()).say('unknown type `$sym.name`'), - field.type_pos) - } - // Separate error condition for `int_literal` and `float_literal` because `util.suggestion` may give different - // suggestions due to f32 comparision issue. - if sym.kind in [.int_literal, .float_literal] { - msg := if sym.kind == .int_literal { - 'unknown type `$sym.name`.\nDid you mean `int`?' - } else { - 'unknown type `$sym.name`.\nDid you mean `f64`?' - } - c.error(msg, field.type_pos) - } - if sym.kind == .array { - array_info := sym.array_info() - elem_sym := c.table.get_type_symbol(array_info.elem_type) - if elem_sym.kind == .placeholder { - c.error(util.new_suggestion(elem_sym.name, c.table.known_type_names()).say('unknown type `$elem_sym.name`'), - field.type_pos) - } - } if sym.kind == .struct_ { info := sym.info as table.Struct if info.is_heap && !field.typ.is_ptr() { struct_sym.info.is_heap = true } } - if sym.kind == .map { - info := sym.map_info() - key_sym := c.table.get_type_symbol(info.key_type) - value_sym := c.table.get_type_symbol(info.value_type) - if key_sym.kind == .placeholder { - c.error('unknown type `$key_sym.name`', field.type_pos) - } - if value_sym.kind == .placeholder { - c.error('unknown type `$value_sym.name`', field.type_pos) - } - } if field.has_default_expr { c.expected_type = field.typ field_expr_type := c.expr(field.default_expr) @@ -509,10 +449,8 @@ pub fn (mut c Checker) struct_init(mut struct_init ast.StructInit) table.Type { struct_init.typ = c.expected_type } } - if struct_init.typ == 0 { - c.error('unknown type', struct_init.pos) - } utyp := c.unwrap_generic(struct_init.typ) + c.ensure_type_exists(utyp, struct_init.pos) or { } type_sym := c.table.get_type_symbol(utyp) if type_sym.kind == .sum_type && struct_init.fields.len == 1 { sexpr := struct_init.fields[0].expr.str() @@ -1107,14 +1045,8 @@ fn (mut c Checker) fail_if_immutable(expr ast.Expr) (string, token.Position) { } ast.SelectorExpr { // retrieve table.Field - if expr.expr_type == 0 { - c.error('0 type in SelectorExpr', expr.pos) - return '', pos - } - mut typ_sym := c.table.get_type_symbol(c.unwrap_generic(expr.expr_type)) - if mut typ_sym.info is table.Alias { - typ_sym = c.table.get_type_symbol(typ_sym.info.parent_type) - } + c.ensure_type_exists(expr.expr_type, expr.pos) or { return '', pos } + mut typ_sym := c.table.get_final_type_symbol(c.unwrap_generic(expr.expr_type)) match typ_sym.kind { .struct_ { struct_info := typ_sym.info as table.Struct @@ -1874,9 +1806,7 @@ pub fn (mut c Checker) call_fn(mut call_expr ast.CallExpr) table.Type { gts := c.table.get_type_symbol(call_expr.generic_types[0]) nrt := '$rts.name<$gts.name>' idx := c.table.type_idxs[nrt] - if idx == 0 { - c.error('unknown type: $nrt', call_expr.pos) - } + c.ensure_type_exists(idx, call_expr.pos) or { } call_expr.return_type = table.new_type(idx).derive(f.return_type) } } @@ -4278,10 +4208,8 @@ pub fn (mut c Checker) match_expr(mut node ast.MatchExpr) table.Type { cond_type := c.expr(node.cond) // we setting this here rather than at the end of the method // since it is used in c.match_exprs() it saves checking twice - node.cond_type = cond_type - if cond_type == 0 { - c.error('compiler bug: match 0 cond type', node.pos) - } + node.cond_type = c.table.mktyp(cond_type) + c.ensure_type_exists(node.cond_type, node.pos) or { return table.void_type } cond_type_sym := c.table.get_type_symbol(cond_type) if cond_type_sym.kind !in [.interface_, .sum_type] { node.is_sum_type = false @@ -5661,17 +5589,10 @@ fn (mut c Checker) sql_stmt(mut node ast.SqlStmt) table.Type { defer { c.inside_sql = false } - sym := c.table.get_type_symbol(node.table_expr.typ) - if node.table_expr.typ == 0 { - c.error('orm: unknown type `$sym.name`', node.pos) - } - if sym.kind == .placeholder { - c.error('orm: unknown type `$sym.name`', node.pos) - return table.void_type - } - c.cur_orm_ts = sym - info := sym.info as table.Struct + c.ensure_type_exists(node.table_expr.typ, node.pos) or { return table.void_type } table_sym := c.table.get_type_symbol(node.table_expr.typ) + c.cur_orm_ts = table_sym + info := table_sym.info as table.Struct fields := c.fetch_and_verify_orm_fields(info, node.table_expr.pos, table_sym.name) mut sub_structs := map[int]ast.SqlStmt{} for f in fields.filter(c.table.types[int(it.typ)].kind == .struct_) { @@ -5788,19 +5709,11 @@ fn (mut c Checker) fn_decl(mut node ast.FnDecl) { if node.language == .v { // Make sure all types are valid for arg in node.params { - sym := c.table.get_type_symbol(arg.typ) - if sym.kind == .placeholder - || (sym.kind in [table.Kind.int_literal, .float_literal] && !c.is_builtin_mod) { - c.error('unknown type `$sym.name`', node.pos) - } + c.ensure_type_exists(arg.typ, node.pos) or { return } } } if node.return_type != table.Type(0) { - return_sym := c.table.get_type_symbol(node.return_type) - if node.language == .v && return_sym.kind in [.placeholder, .int_literal, .float_literal] - && return_sym.language == .v { - c.error('unknown type `$return_sym.name`', node.pos) - } + c.ensure_type_exists(node.return_type, node.pos) or { return } if node.language == .v && node.is_method && node.name == 'str' { if node.return_type != table.string_type { c.error('.str() methods should return `string`', node.pos) @@ -5949,3 +5862,41 @@ fn (mut c Checker) trace(fbase string, message string) { println('> c.trace | ${fbase:-10s} | $message') } } + +fn (mut c Checker) ensure_type_exists(typ table.Type, pos token.Position) ? { + if typ == 0 { + c.error('unknown type', pos) + } + sym := c.table.get_type_symbol(typ) + match sym.kind { + .placeholder { + if sym.language == .v && !sym.name.starts_with('C.') { + c.error(util.new_suggestion(sym.name, c.table.known_type_names()).say('unknown type `$sym.name`'), + pos) + return none + } + } + .int_literal, .float_literal { + // Separate error condition for `int_literal` and `float_literal` because `util.suggestion` may give different + // suggestions due to f32 comparision issue. + if !c.is_builtin_mod { + msg := if sym.kind == .int_literal { + 'unknown type `$sym.name`.\nDid you mean `int`?' + } else { + 'unknown type `$sym.name`.\nDid you mean `f64`?' + } + c.error(msg, pos) + return none + } + } + .array { + c.ensure_type_exists((sym.info as table.Array).elem_type, pos) ? + } + .map { + info := sym.info as table.Map + c.ensure_type_exists(info.key_type, pos) ? + c.ensure_type_exists(info.value_type, pos) ? + } + else {} + } +} diff --git a/vlib/v/checker/tests/any_int_float_ban_err.out b/vlib/v/checker/tests/any_int_float_ban_err.out index eb29df358a..6c9f9c2574 100644 --- a/vlib/v/checker/tests/any_int_float_ban_err.out +++ b/vlib/v/checker/tests/any_int_float_ban_err.out @@ -9,16 +9,14 @@ vlib/v/checker/tests/any_int_float_ban_err.vv:2:1: error: type `int_literal` doe | ~~~~~~~~ 3 | 4 | struct Int { -vlib/v/checker/tests/any_int_float_ban_err.vv:5:7: error: unknown type `int_literal`. -Did you mean `float_literal`? +vlib/v/checker/tests/any_int_float_ban_err.vv:5:7: error: unknown type `int_literal` 3 | 4 | struct Int { 5 | i int_literal | ~~~~~~~~~~~ 6 | f float_literal 7 | } -vlib/v/checker/tests/any_int_float_ban_err.vv:6:7: error: unknown type `float_literal`. -Did you mean `int_literal`? +vlib/v/checker/tests/any_int_float_ban_err.vv:6:7: error: unknown type `float_literal` 4 | struct Int { 5 | i int_literal 6 | f float_literal diff --git a/vlib/v/checker/tests/filter_on_non_arr_err.out b/vlib/v/checker/tests/filter_on_non_arr_err.out index 2f0deabdd4..5716e571ef 100644 --- a/vlib/v/checker/tests/filter_on_non_arr_err.out +++ b/vlib/v/checker/tests/filter_on_non_arr_err.out @@ -1,6 +1,5 @@ -vlib/v/checker/tests/filter_on_non_arr_err.vv:2:14: error: unknown method: `string.filter`. -Did you mean `after`? +vlib/v/checker/tests/filter_on_non_arr_err.vv:2:14: error: unknown method: `string.filter` 1 | fn main() { 2 | _ := 'test'.filter(it == `t`) | ~~~~~~~~~~~~~~~~~ - 3 | } \ No newline at end of file + 3 | } diff --git a/vlib/v/checker/tests/import_symbol_type_err.out b/vlib/v/checker/tests/import_symbol_type_err.out index c9f238abf0..2c1cd9be8c 100644 --- a/vlib/v/checker/tests/import_symbol_type_err.out +++ b/vlib/v/checker/tests/import_symbol_type_err.out @@ -3,7 +3,8 @@ vlib/v/checker/tests/import_symbol_type_err.vv:1:17: error: module `crypto` has | ~~~~ 2 | fn main() { 3 | println(Coin{}) -vlib/v/checker/tests/import_symbol_type_err.vv:3:11: error: unknown struct: crypto.Coin +vlib/v/checker/tests/import_symbol_type_err.vv:3:11: error: unknown type `crypto.Coin`. +Did you mean `crypto.Hash`? 1 | import crypto { Coin } 2 | fn main() { 3 | println(Coin{}) diff --git a/vlib/v/checker/tests/interface_return_parameter_err.out b/vlib/v/checker/tests/interface_return_parameter_err.out new file mode 100644 index 0000000000..86236b1c6d --- /dev/null +++ b/vlib/v/checker/tests/interface_return_parameter_err.out @@ -0,0 +1,14 @@ +vlib/v/checker/tests/interface_return_parameter_err.vv:2:5: error: unknown type `Baz`. +Did you mean `Foo`? + 1 | interface Foo { + 2 | bar(string) []Baz + | ~~~~~~~~~~~ + 3 | bar2(Bax) string + 4 | } +vlib/v/checker/tests/interface_return_parameter_err.vv:3:10: error: unknown type `Bax`. +Did you mean `Foo`? + 1 | interface Foo { + 2 | bar(string) []Baz + 3 | bar2(Bax) string + | ~~~ + 4 | } diff --git a/vlib/v/checker/tests/interface_return_parameter_err.vv b/vlib/v/checker/tests/interface_return_parameter_err.vv new file mode 100644 index 0000000000..e6168cf87b --- /dev/null +++ b/vlib/v/checker/tests/interface_return_parameter_err.vv @@ -0,0 +1,4 @@ +interface Foo { + bar(string) []Baz + bar2(Bax) string +} diff --git a/vlib/v/checker/tests/map_unknown_value.out b/vlib/v/checker/tests/map_unknown_value.out index 6c408b5a18..d5f268b5cb 100644 --- a/vlib/v/checker/tests/map_unknown_value.out +++ b/vlib/v/checker/tests/map_unknown_value.out @@ -2,4 +2,4 @@ vlib/v/checker/tests/map_unknown_value.vv:2:23: error: unknown type `DoesNotExis 1 | struct App { 2 | my_map map[string]DoesNotExist | ~~~~~~~~~~~~ - 3 | } \ No newline at end of file + 3 | } diff --git a/vlib/v/checker/tests/unknown_array_element_type_b.out b/vlib/v/checker/tests/unknown_array_element_type_b.out index 3052e4db36..2efc456c97 100644 --- a/vlib/v/checker/tests/unknown_array_element_type_b.out +++ b/vlib/v/checker/tests/unknown_array_element_type_b.out @@ -1,6 +1,7 @@ -vlib/v/checker/tests/unknown_array_element_type_b.vv:2:6: error: unknown type `abc` +vlib/v/checker/tests/unknown_array_element_type_b.vv:2:6: error: unknown type `abc`. +Did you mean `Aaa`? 1 | struct Aaa { 2 | a []abc | ~~~ 3 | } - 4 | \ No newline at end of file + 4 | diff --git a/vlib/v/checker/tests/unknown_method_suggest_name.out b/vlib/v/checker/tests/unknown_method_suggest_name.out index 2d6bcff5bc..a6fa8b7f3e 100644 --- a/vlib/v/checker/tests/unknown_method_suggest_name.out +++ b/vlib/v/checker/tests/unknown_method_suggest_name.out @@ -13,4 +13,4 @@ Did you mean `translate`? 27 | z := p.tranzlate(v) | ~~~~~~~~~~~~ 28 | println('p: $p') - 29 | println('v: $v') \ No newline at end of file + 29 | println('v: $v') diff --git a/vlib/v/table/table.v b/vlib/v/table/table.v index 41cc8d202f..71986e4b7d 100644 --- a/vlib/v/table/table.v +++ b/vlib/v/table/table.v @@ -397,7 +397,27 @@ pub fn (mut t Table) register_type_symbol(typ TypeSymbol) int { } pub fn (t &Table) known_type(name string) bool { - t.find_type(name) or { return false } + return t.find_type_idx(name) != 0 +} + +pub fn (t &Table) known_type_idx(typ Type) bool { + if typ == 0 { + return false + } + sym := t.get_type_symbol(typ) + match sym.kind { + .placeholder { + return sym.language != .v || sym.name.starts_with('C.') + } + .array { + return t.known_type_idx((sym.info as Array).elem_type) + } + .map { + info := sym.info as Map + return t.known_type_idx(info.key_type) && t.known_type_idx(info.value_type) + } + else {} + } return true } @@ -756,14 +776,13 @@ pub fn (mytable &Table) sumtype_has_variant(parent Type, variant Type) bool { return false } -pub fn (mytable &Table) known_type_names() []string { - mut res := []string{} - for _, idx in mytable.type_idxs { +pub fn (t &Table) known_type_names() []string { + mut res := []string{cap: t.type_idxs.len} + for _, idx in t.type_idxs { // Skip `int_literal_type_idx` and `float_literal_type_idx` because they shouldn't be visible to the User. - if idx in [0, int_literal_type_idx, float_literal_type_idx] { - continue + if idx !in [0, int_literal_type_idx, float_literal_type_idx] && t.known_type_idx(idx) { + res << t.type_to_str(idx) } - res << mytable.type_to_str(idx) } return res } diff --git a/vlib/v/util/suggestions.v b/vlib/v/util/suggestions.v index 4ebd322a22..e1c35ea9c2 100644 --- a/vlib/v/util/suggestions.v +++ b/vlib/v/util/suggestions.v @@ -9,17 +9,6 @@ mut: similarity f32 } -fn compare_by_similarity(a &Possibility, b &Possibility) int { - if a.similarity < b.similarity { - return -1 - } - if a.similarity > b.similarity { - return 1 - } - return 0 -} - -// struct Suggestion { mut: known []Possibility @@ -45,10 +34,12 @@ pub fn (mut s Suggestion) add(val string) { if sval in [s.wanted, s.swanted] { return } + // round to 3 decimal places to avoid float comparison issues + similarity := f32(int(strings.dice_coefficient(s.swanted, sval) * 1000)) / 1000 s.known << Possibility{ value: val svalue: sval - similarity: strings.dice_coefficient(s.swanted, sval) + similarity: similarity } } @@ -59,7 +50,7 @@ pub fn (mut s Suggestion) add_many(many []string) { } pub fn (mut s Suggestion) sort() { - s.known.sort_with_compare(compare_by_similarity) + s.known.sort(a.similarity < b.similarity) } pub fn (s Suggestion) say(msg string) string { @@ -67,7 +58,7 @@ pub fn (s Suggestion) say(msg string) string { mut found := false if s.known.len > 0 { top_posibility := s.known.last() - if top_posibility.similarity > 0.10 { + if top_posibility.similarity > 0.5 { val := top_posibility.value if !val.starts_with('[]') { res += '.\nDid you mean `$val`?'