diff --git a/cmd/tools/vvet.v b/cmd/tools/vvet.v index 6a6dbfdd7e..1201b4ef93 100644 --- a/cmd/tools/vvet.v +++ b/cmd/tools/vvet.v @@ -3,7 +3,6 @@ module main import v.vet -import v.ast import v.pref import v.parser import v.util @@ -13,6 +12,7 @@ import os.cmdline struct VetOptions { is_verbose bool + errors []string } fn (vet_options &VetOptions) vprintln(s string) { @@ -39,18 +39,20 @@ fn main() { } } } + if vet_options.errors.len > 0 { + for err in vet_options.errors { + eprintln(err) + } + eprintln('NB: You can run `v fmt -w file.v` to fix these automatically') + exit(1) + } } fn (vet_options &VetOptions) vet_file(path string) { mut prefs := pref.new_preferences() prefs.is_vet = true table := table.new_table() - if path.contains('/tests') { - return - } - file_ast := parser.parse_file(path, table, .parse_comments, prefs, &ast.Scope{ - parent: 0 - }) vet_options.vprintln("vetting file '$path'...") + file_ast := parser.parse_vet_file(path, table, prefs, vet_options.errors) vet.vet(file_ast, table, true) } diff --git a/vlib/v/checker/checker.v b/vlib/v/checker/checker.v index 5f8bf601eb..b6f32572c4 100644 --- a/vlib/v/checker/checker.v +++ b/vlib/v/checker/checker.v @@ -1073,7 +1073,7 @@ pub fn (mut c Checker) call_fn(mut call_expr ast.CallExpr) table.Type { found = true } // try prefix with current module as it would have never gotten prefixed - if !found && !fn_name.contains('.') && call_expr.mod !in ['builtin'] { + if !found && !fn_name.contains('.') && call_expr.mod != 'builtin' { name_prefixed := '${call_expr.mod}.$fn_name' if f1 := c.table.find_fn(name_prefixed) { call_expr.name = name_prefixed @@ -1562,7 +1562,7 @@ pub fn (mut c Checker) assign_stmt(mut assign_stmt ast.AssignStmt) { if right_first is ast.CallExpr || right_first is ast.IfExpr || right_first is ast.MatchExpr { right_type0 := c.expr(right_first) assign_stmt.right_types = [ - c.check_expr_opt_call(right_first, right_type0) + c.check_expr_opt_call(right_first, right_type0), ] right_type_sym0 := c.table.get_type_symbol(right_type0) if right_type_sym0.kind == .multi_return { @@ -2219,7 +2219,7 @@ pub fn (mut c Checker) expr(node ast.Expr) table.Type { c.error('cannot cast type `$type_name` to string, use `x.str()` instead', node.pos) } else if node.expr_type == table.string_type { - if to_type_sym.kind !in [.alias] { + if to_type_sym.kind != .alias { mut error_msg := 'cannot cast a string' if node.expr is ast.StringLiteral { str_lit := node.expr as ast.StringLiteral @@ -2387,7 +2387,7 @@ pub fn (mut c Checker) ident(mut ident ast.Ident) table.Type { // TODO: move this if c.const_deps.len > 0 { mut name := ident.name - if !name.contains('.') && ident.mod !in ['builtin'] { + if !name.contains('.') && ident.mod != 'builtin' { name = '${ident.mod}.$ident.name' } if name == c.const_decl { @@ -2465,7 +2465,7 @@ pub fn (mut c Checker) ident(mut ident ast.Ident) table.Type { } // prepend mod to look for fn call or const mut name := ident.name - if !name.contains('.') && ident.mod !in ['builtin'] { + if !name.contains('.') && ident.mod != 'builtin' { name = '${ident.mod}.$ident.name' } if obj := c.file.global_scope.find(name) { diff --git a/vlib/v/fmt/fmt.v b/vlib/v/fmt/fmt.v index d32f57901a..fe881a8016 100644 --- a/vlib/v/fmt/fmt.v +++ b/vlib/v/fmt/fmt.v @@ -11,7 +11,7 @@ import v.util const ( tabs = ['', '\t', '\t\t', '\t\t\t', '\t\t\t\t', '\t\t\t\t\t', '\t\t\t\t\t\t', '\t\t\t\t\t\t\t', - '\t\t\t\t\t\t\t\t' + '\t\t\t\t\t\t\t\t', ] // when to break a line dependant on penalty max_len = [0, 35, 85, 93, 100] @@ -829,48 +829,7 @@ pub fn (mut f Fmt) expr(node ast.Expr) { f.expr(node.expr) } ast.InfixExpr { - if f.is_inside_interp { - f.expr(node.left) - f.write('$node.op.str()') - f.expr(node.right) - } else { - buffering_save := f.buffering - if !f.buffering { - f.out_save = f.out - f.out = strings.new_builder(60) - f.buffering = true - } - f.expr(node.left) - f.write(' $node.op.str() ') - f.expr_bufs << f.out.str() - mut penalty := 3 - if node.left is ast.InfixExpr || node.left is ast.ParExpr { - penalty-- - } - if node.right is ast.InfixExpr || node.right is ast.ParExpr { - penalty-- - } - f.penalties << penalty - // combine parentheses level with operator precedence to form effective precedence - f.precedences << int(token.precedences[node.op]) | (f.par_level << 16) - f.out = strings.new_builder(60) - f.buffering = true - f.expr(node.right) - if !buffering_save && f.buffering { // now decide if and where to break - f.expr_bufs << f.out.str() - f.out = f.out_save - f.buffering = false - f.adjust_complete_line() - for i, p in f.penalties { - f.write(f.expr_bufs[i]) - f.wrap_long_line(p, true) - } - f.write(f.expr_bufs[f.expr_bufs.len - 1]) - f.expr_bufs = []string{} - f.penalties = []int{} - f.precedences = []int{} - } - } + f.infix_expr(node) } ast.IndexExpr { f.expr(node.left) @@ -1254,6 +1213,67 @@ pub fn (mut f Fmt) lock_expr(lex ast.LockExpr) { f.write('}') } +pub fn (mut f Fmt) infix_expr(node ast.InfixExpr) { + if f.is_inside_interp { + f.expr(node.left) + f.write('$node.op.str()') + f.expr(node.right) + } else { + buffering_save := f.buffering + if !f.buffering { + f.out_save = f.out + f.out = strings.new_builder(60) + f.buffering = true + } + f.expr(node.left) + is_one_val_array_init := node.op in [.key_in, .not_in] && + node.right is ast.ArrayInit && (node.right as ast.ArrayInit).exprs.len == 1 + if is_one_val_array_init { + // `var in [val]` => `var == val` + f.write(if node.op == .key_in { + ' == ' + } else { + ' != ' + }) + } else { + f.write(' $node.op.str() ') + } + f.expr_bufs << f.out.str() + mut penalty := 3 + if node.left is ast.InfixExpr || node.left is ast.ParExpr { + penalty-- + } + if node.right is ast.InfixExpr || node.right is ast.ParExpr { + penalty-- + } + f.penalties << penalty + // combine parentheses level with operator precedence to form effective precedence + f.precedences << int(token.precedences[node.op]) | (f.par_level << 16) + f.out = strings.new_builder(60) + f.buffering = true + if is_one_val_array_init { + // `var in [val]` => `var == val` + f.expr((node.right as ast.ArrayInit).exprs[0]) + } else { + f.expr(node.right) + } + if !buffering_save && f.buffering { // now decide if and where to break + f.expr_bufs << f.out.str() + f.out = f.out_save + f.buffering = false + f.adjust_complete_line() + for i, p in f.penalties { + f.write(f.expr_bufs[i]) + f.wrap_long_line(p, true) + } + f.write(f.expr_bufs[f.expr_bufs.len - 1]) + f.expr_bufs = []string{} + f.penalties = []int{} + f.precedences = []int{} + } + } +} + pub fn (mut f Fmt) if_expr(it ast.IfExpr) { single_line := it.branches.len == 2 && it.has_else && it.branches[0].stmts.len == 1 && it.branches[1].stmts.len == 1 && diff --git a/vlib/v/fmt/tests/array_init_expected.vv b/vlib/v/fmt/tests/array_init_expected.vv new file mode 100644 index 0000000000..17d53276a1 --- /dev/null +++ b/vlib/v/fmt/tests/array_init_expected.vv @@ -0,0 +1,8 @@ +fn main() { + if 1 == 1 { + println('yeah !') + } + if 1 != 1 { + println("oh no :'(") + } +} diff --git a/vlib/v/fmt/tests/array_init_input.vv b/vlib/v/fmt/tests/array_init_input.vv new file mode 100644 index 0000000000..f56eba5cc6 --- /dev/null +++ b/vlib/v/fmt/tests/array_init_input.vv @@ -0,0 +1,8 @@ +fn main() { + if 1 in [1] { + println('yeah !') + } + if 1 !in [1] { + println("oh no :'(") + } +} diff --git a/vlib/v/gen/cgen.v b/vlib/v/gen/cgen.v index 48f2b9913f..abf1350f61 100644 --- a/vlib/v/gen/cgen.v +++ b/vlib/v/gen/cgen.v @@ -1503,13 +1503,10 @@ fn (mut g Gen) get_ternary_name(name string) string { } fn (mut g Gen) gen_clone_assignment(val ast.Expr, right_sym table.TypeSymbol, add_eq bool) bool { - mut is_ident := false - match val { - ast.Ident { is_ident = true } - ast.SelectorExpr { is_ident = true } - else { return false } + if val !is ast.Ident && val !is ast.SelectorExpr { + return false } - if g.autofree && right_sym.kind == .array && is_ident { + if g.autofree && right_sym.kind == .array { // `arr1 = arr2` => `arr1 = arr2.clone()` if add_eq { g.write('=') @@ -1517,7 +1514,7 @@ fn (mut g Gen) gen_clone_assignment(val ast.Expr, right_sym table.TypeSymbol, ad g.write(' array_clone_static(') g.expr(val) g.write(')') - } else if g.autofree && right_sym.kind == .string && is_ident { + } else if g.autofree && right_sym.kind == .string { if add_eq { g.write('=') } @@ -2962,7 +2959,7 @@ fn (mut g Gen) struct_init(struct_init ast.StructInit) { g.write('.$field_name = ') field_type_sym := g.table.get_type_symbol(field.typ) mut cloned := false - if g.autofree && field_type_sym.kind in [.array, .string] { + if g.autofree && !field.typ.is_ptr() && field_type_sym.kind in [.array, .string] { g.write('/*clone1*/') if g.gen_clone_assignment(field.expr, field_type_sym, false) { cloned = true @@ -3001,7 +2998,7 @@ fn (mut g Gen) struct_init(struct_init ast.StructInit) { g.write('.$field_name = ') field_type_sym := g.table.get_type_symbol(sfield.typ) mut cloned := false - if g.autofree && field_type_sym.kind in [.array, .string] { + if g.autofree && !sfield.typ.is_ptr() && field_type_sym.kind in [.array, .string] { g.write('/*clone1*/') if g.gen_clone_assignment(sfield.expr, field_type_sym, false) { cloned = true diff --git a/vlib/v/parser/parse_type.v b/vlib/v/parser/parse_type.v index 1c517a4d5a..fccc7b2fc3 100644 --- a/vlib/v/parser/parse_type.v +++ b/vlib/v/parser/parse_type.v @@ -185,7 +185,7 @@ pub fn (mut p Parser) parse_any_type(language table.Language, is_ptr bool) table name = '${p.imports[name]}.$p.tok.lit' } else if p.expr_mod != '' { name = p.expr_mod + '.' + name - } else if p.mod !in ['builtin'] && name !in p.table.type_idxs && name.len > 1 { + } else if p.mod != 'builtin' && name !in p.table.type_idxs && name.len > 1 { // `Foo` in module `mod` means `mod.Foo` name = p.mod + '.' + name } diff --git a/vlib/v/parser/parser.v b/vlib/v/parser/parser.v index 52d894b595..0aec310d57 100644 --- a/vlib/v/parser/parser.v +++ b/vlib/v/parser/parser.v @@ -54,6 +54,7 @@ mut: expecting_type bool // `is Type`, expecting type errors []errors.Error warnings []errors.Warning + vet_errors &[]string cur_fn_name string } @@ -70,6 +71,7 @@ pub fn parse_stmt(text string, table &table.Table, scope &ast.Scope) ast.Stmt { start_pos: 0 parent: 0 } + vet_errors: 0 } p.init_parse_fns() p.read_first_token() @@ -86,6 +88,7 @@ pub fn parse_text(text string, b_table &table.Table, pref &pref.Preferences, sco errors: []errors.Error{} warnings: []errors.Warning{} global_scope: global_scope + vet_errors: 0 } return p.parse() } @@ -113,21 +116,41 @@ pub fn parse_file(path string, b_table &table.Table, comments_mode scanner.Comme errors: []errors.Error{} warnings: []errors.Warning{} global_scope: global_scope + vet_errors: 0 } - if pref.is_vet && p.scanner.text.contains('\n ') { + return p.parse() +} + +pub fn parse_vet_file(path string, table_ &table.Table, pref &pref.Preferences, vet_errors &[]string) ast.File { + global_scope := &ast.Scope{ + parent: 0 + } + mut p := Parser{ + scanner: scanner.new_vet_scanner_file(path, .parse_comments, pref, vet_errors) + comments_mode: .parse_comments + table: table_ + file_name: path + file_name_dir: os.dir(path) + pref: pref + scope: &ast.Scope{ + start_pos: 0 + parent: global_scope + } + errors: []errors.Error{} + warnings: []errors.Warning{} + global_scope: global_scope + vet_errors: vet_errors + } + if p.scanner.text.contains('\n ') { source_lines := os.read_lines(path) or { []string{} } for lnumber, line in source_lines { - if line.starts_with(' ') { - eprintln('$p.scanner.file_path:${lnumber+1}: Looks like you are using spaces for indentation.') + if line.starts_with(' ') { + p.vet_error('Looks like you are using spaces for indentation.', lnumber) } } - eprintln('NB: You can run `v fmt -w file.v` to fix these automatically') - exit(1) } - // if pref.is_vet && p.scanner.text.contains('( '\n ') { - // } return p.parse() } @@ -777,6 +800,10 @@ pub fn (mut p Parser) warn_with_pos(s string, pos token.Position) { } } +pub fn (mut p Parser) vet_error(s string, line int) { + p.vet_errors << '$p.scanner.file_path:${line+1}: $s' +} + fn (mut p Parser) parse_multi_expr(is_top_level bool) ast.Stmt { // in here might be 1) multi-expr 2) multi-assign // 1, a, c ... } // multi-expression @@ -1352,9 +1379,9 @@ fn (mut p Parser) import_stmt() ast.Import { mod_alias = p.check_name() } node := ast.Import{ - pos: pos, - mod: mod_name, - alias: mod_alias, + pos: pos + mod: mod_name + alias: mod_alias } if p.tok.kind == .lcbr { // import module { fn1, Type2 } syntax p.import_syms(node) @@ -1380,7 +1407,8 @@ fn (mut p Parser) import_syms(mut parent ast.Import) { p.error_with_pos('empty `$parent.mod` import set, remove `{}`', pos_t) } if p.tok.kind != .name { // not a valid inner name - p.error_with_pos('import syntax error, please specify a valid fn or type name', pos_t) + p.error_with_pos('import syntax error, please specify a valid fn or type name', + pos_t) } for p.tok.kind == .name { pos := p.tok.position() diff --git a/vlib/v/parser/pratt.v b/vlib/v/parser/pratt.v index ade32eb2b2..7a3831bcb0 100644 --- a/vlib/v/parser/pratt.v +++ b/vlib/v/parser/pratt.v @@ -283,6 +283,10 @@ fn (mut p Parser) infix_expr(left ast.Expr) ast.Expr { p.expecting_type = true } right = p.expr(precedence) + if p.pref.is_vet && op in [.key_in, .not_in] && + right is ast.ArrayInit && (right as ast.ArrayInit).exprs.len == 1 { + p.vet_error('Use `var == value` instead of `var in [value]`', pos.line_nr) + } return ast.InfixExpr{ left: left right: right diff --git a/vlib/v/scanner/scanner.v b/vlib/v/scanner/scanner.v index a9e19cb51a..abc83b108e 100644 --- a/vlib/v/scanner/scanner.v +++ b/vlib/v/scanner/scanner.v @@ -49,6 +49,7 @@ pub mut: tidx int eofs int pref &pref.Preferences + vet_errors &[]string } /* @@ -96,7 +97,10 @@ pub enum CommentsMode { // new scanner from file. pub fn new_scanner_file(file_path string, comments_mode CommentsMode, pref &pref.Preferences) &Scanner { - // is_fmt := pref.is_fmt + return new_vet_scanner_file(file_path, comments_mode, pref, voidptr(0)) +} + +pub fn new_vet_scanner_file(file_path string, comments_mode CommentsMode, pref &pref.Preferences, vet_errors &[]string) &Scanner { if !os.exists(file_path) { verror("$file_path doesn't exist") } @@ -104,14 +108,17 @@ pub fn new_scanner_file(file_path string, comments_mode CommentsMode, pref &pref verror(err) return voidptr(0) } - mut s := new_scanner(raw_text, comments_mode, pref) // .skip_comments) - // s.init_fmt() + mut s := new_vet_scanner(raw_text, comments_mode, pref, vet_errors) s.file_path = file_path return s } // new scanner from string. pub fn new_scanner(text string, comments_mode CommentsMode, pref &pref.Preferences) &Scanner { + return new_vet_scanner(text, comments_mode, pref, voidptr(0)) +} + +pub fn new_vet_scanner(text string, comments_mode CommentsMode, pref &pref.Preferences, vet_errors &[]string) &Scanner { is_fmt := pref.is_fmt s := &Scanner{ pref: pref @@ -121,6 +128,7 @@ pub fn new_scanner(text string, comments_mode CommentsMode, pref &pref.Preferenc is_print_rel_paths_on_error: true is_fmt: is_fmt comments_mode: comments_mode + vet_errors: vet_errors } return s } @@ -793,14 +801,14 @@ fn (mut s Scanner) text_scan() token.Token { `(` { // TODO `$if vet {` for performance if s.pref.is_vet && s.text[s.pos + 1] == ` ` { - println('$s.file_path:$s.line_nr: Looks like you are adding a space after `(`') + s.vet_error('Looks like you are adding a space after `(`') } return s.new_token(.lpar, '', 1) } `)` { // TODO `$if vet {` for performance if s.pref.is_vet && s.text[s.pos - 1] == ` ` { - println('$s.file_path:$s.line_nr: Looks like you are adding a space before `)`') + s.vet_error('Looks like you are adding a space before `)`') } return s.new_token(.rpar, '', 1) } @@ -1351,6 +1359,10 @@ pub fn (s &Scanner) error(msg string) { exit(1) } +fn (mut s Scanner) vet_error(msg string) { + s.vet_errors << '$s.file_path:$s.line_nr: $msg' +} + pub fn verror(s string) { util.verror('scanner error', s) } diff --git a/vlib/v/vet/tests/array_init_one_val.out b/vlib/v/vet/tests/array_init_one_val.out new file mode 100644 index 0000000000..32b934cffb --- /dev/null +++ b/vlib/v/vet/tests/array_init_one_val.out @@ -0,0 +1,2 @@ +vlib/v/vet/tests/array_init_one_val.v:2: Use `var == value` instead of `var in [value]` +NB: You can run `v fmt -w file.v` to fix these automatically diff --git a/vlib/v/vet/tests/array_init_one_val.vv b/vlib/v/vet/tests/array_init_one_val.vv new file mode 100644 index 0000000000..2aa35146db --- /dev/null +++ b/vlib/v/vet/tests/array_init_one_val.vv @@ -0,0 +1,5 @@ +fn main() { + if 1 in [1] { + println('hello world') + } +} diff --git a/vlib/v/vet/tests/indent_with_space.out b/vlib/v/vet/tests/indent_with_space.out new file mode 100644 index 0000000000..2eceb1d9b3 --- /dev/null +++ b/vlib/v/vet/tests/indent_with_space.out @@ -0,0 +1,2 @@ +vlib/v/vet/tests/indent_with_space.v:2: Looks like you are using spaces for indentation. +NB: You can run `v fmt -w file.v` to fix these automatically diff --git a/vlib/v/vet/tests/indent_with_space.vv b/vlib/v/vet/tests/indent_with_space.vv new file mode 100644 index 0000000000..c965d2131c --- /dev/null +++ b/vlib/v/vet/tests/indent_with_space.vv @@ -0,0 +1,3 @@ +fn main() { + _ = 1 == 2 +} diff --git a/vlib/v/vet/tests/parens_space_a.out b/vlib/v/vet/tests/parens_space_a.out new file mode 100644 index 0000000000..f476657297 --- /dev/null +++ b/vlib/v/vet/tests/parens_space_a.out @@ -0,0 +1,2 @@ +vlib/v/vet/tests/parens_space_a.v:1: Looks like you are adding a space after `(` +NB: You can run `v fmt -w file.v` to fix these automatically diff --git a/vlib/v/vet/tests/parens_space_a.vv b/vlib/v/vet/tests/parens_space_a.vv new file mode 100644 index 0000000000..2b3b508170 --- /dev/null +++ b/vlib/v/vet/tests/parens_space_a.vv @@ -0,0 +1,4 @@ +fn main() { + _ = 1 + ( 1 + 2) +} + diff --git a/vlib/v/vet/tests/parens_space_b.out b/vlib/v/vet/tests/parens_space_b.out new file mode 100644 index 0000000000..a395988766 --- /dev/null +++ b/vlib/v/vet/tests/parens_space_b.out @@ -0,0 +1,2 @@ +vlib/v/vet/tests/parens_space_b.v:1: Looks like you are adding a space before `)` +NB: You can run `v fmt -w file.v` to fix these automatically diff --git a/vlib/v/vet/tests/parens_space_b.vv b/vlib/v/vet/tests/parens_space_b.vv new file mode 100644 index 0000000000..9ea8932bd1 --- /dev/null +++ b/vlib/v/vet/tests/parens_space_b.vv @@ -0,0 +1,4 @@ +fn main() { + _ = 1 + (1 + 2 ) +} + diff --git a/vlib/v/vet/vet_test.v b/vlib/v/vet/vet_test.v new file mode 100644 index 0000000000..67f24bab4e --- /dev/null +++ b/vlib/v/vet/vet_test.v @@ -0,0 +1,81 @@ +import os +import term + +fn test_vet() { + vexe := os.getenv('VEXE') + vroot := os.dir(vexe) + os.chdir(vroot) + test_dir := 'vlib/v/vet/tests' + tests := get_tests_in_dir(test_dir) + assert check_path(vexe, test_dir, tests) == 0 +} + +fn get_tests_in_dir(dir string) []string { + files := os.ls(dir) or { + panic(err) + } + mut tests := files.filter(it.ends_with('.vv')) + tests.sort() + return tests +} + +fn check_path(vexe, dir string, tests []string) int { + vtest_only := os.getenv('VTEST_ONLY').split(',') + mut nb_fail := 0 + mut paths := []string{} + for test in tests { + path := os.join_path(dir, test).replace('\\', '/') + if vtest_only.len > 0 { + mut found := 0 + for substring in vtest_only { + if path.contains(substring) { + found++ + break + } + } + if found == 0 { + continue + } + } + paths << path + } + for path in paths { + program := path.replace('.vv', '.v') + print(path + ' ') + os.cp(path, program) or { + panic(err) + } + res := os.exec('$vexe vet $program') or { + panic(err) + } + mut expected := os.read_file(program.replace('.v', '') + '.out') or { + panic(err) + } + expected = clean_line_endings(expected) + found := clean_line_endings(res.output) + if expected != found { + println(term.red('FAIL')) + println('============') + println('expected:') + println(expected) + println('============') + println('found:') + println(found) + println('============\n') + nb_fail++ + } else { + println(term.green('OK')) + os.rm(program) + } + } + return nb_fail +} + +fn clean_line_endings(s string) string { + mut res := s.trim_space() + res = res.replace(' \n', '\n') + res = res.replace(' \r\n', '\n') + res = res.replace('\r\n', '\n') + res = res.trim('\n') + return res +}