diff --git a/cmd/tools/vvet/analyze.v b/cmd/tools/vvet/analyze.v new file mode 100644 index 00000000000000..d651118f197291 --- /dev/null +++ b/cmd/tools/vvet/analyze.v @@ -0,0 +1,145 @@ +// Copyright (c) 2025 Felipe Pena. All rights reserved. +// Use of this source code is governed by an MIT license that can be found in the LICENSE file. +module main + +import v.ast +import v.token +import arrays + +// cutoffs +const indexexpr_cutoff = 10 +const infixexpr_cutoff = 10 +const selectorexpr_cutoff = 10 +const callexpr_cutoff = 10 +const stringinterliteral_cutoff = 10 +const stringliteral_cutoff = 10 +const ascast_cutoff = 10 +const stringconcat_cutoff = 10 + +// minimum size for string literals +const stringliteral_min_size = 20 + +// long functions cutoff +const long_fns_cutoff = 300 + +struct VetAnalyze { +mut: + repeated_expr_cutoff shared map[string]int // repeated code cutoff + repeated_expr shared map[string]map[string]map[string][]token.Pos // repeated exprs in fn scope + cur_fn ast.FnDecl // current fn declaration +} + +// stmt checks for repeated code in statements +fn (mut vt VetAnalyze) stmt(vet &Vet, stmt ast.Stmt) { + match stmt { + ast.AssignStmt { + if stmt.op == .plus_assign { + if stmt.right[0] in [ast.StringLiteral, ast.StringInterLiteral] { + vt.save_expr(stringconcat_cutoff, '${stmt.left[0].str()} += ${stmt.right[0].str()}', + vet.file, stmt.pos) + } + } + } + else {} + } +} + +// save_expr registers a repeated code occurrence +fn (mut vt VetAnalyze) save_expr(cutoff int, expr string, file string, pos token.Pos) { + lock vt.repeated_expr { + vt.repeated_expr[vt.cur_fn.name][expr][file] << pos + } + lock vt.repeated_expr_cutoff { + vt.repeated_expr_cutoff[expr] = cutoff + } +} + +// exprs checks for repeated code in expressions +fn (mut vt VetAnalyze) exprs(vet &Vet, exprs []ast.Expr) { + for expr in exprs { + vt.expr(vet, expr) + } +} + +// expr checks for repeated code +fn (mut vt VetAnalyze) expr(vet &Vet, expr ast.Expr) { + match expr { + ast.InfixExpr { + vt.save_expr(infixexpr_cutoff, '${expr.left} ${expr.op} ${expr.right}', vet.file, + expr.pos) + } + ast.IndexExpr { + vt.save_expr(indexexpr_cutoff, '${expr.left}[${expr.index}]', vet.file, expr.pos) + } + ast.SelectorExpr { + // nested selectors + if expr.expr !is ast.Ident { + vt.save_expr(selectorexpr_cutoff, '${expr.expr.str()}.${expr.field_name}', + vet.file, expr.pos) + } + } + ast.CallExpr { + if expr.is_static_method || expr.is_method { + vt.save_expr(callexpr_cutoff, '${expr.left}.${expr.name}(${expr.args.map(it.str()).join(', ')})', + vet.file, expr.pos) + } else { + vt.save_expr(callexpr_cutoff, '${expr.name}(${expr.args.map(it.str()).join(', ')})', + vet.file, expr.pos) + } + } + ast.AsCast { + vt.save_expr(ascast_cutoff, ast.Expr(expr).str(), vet.file, expr.pos) + } + ast.StringLiteral { + if expr.val.len > stringliteral_min_size { + vt.save_expr(stringliteral_cutoff, ast.Expr(expr).str(), vet.file, expr.pos) + } + } + ast.StringInterLiteral { + vt.save_expr(stringinterliteral_cutoff, ast.Expr(expr).str(), vet.file, expr.pos) + } + else {} + } +} + +// long_or_empty_fns checks for long or empty functions +fn (mut vt VetAnalyze) long_or_empty_fns(mut vet Vet, fn_decl ast.FnDecl) { + nr_lines := if fn_decl.stmts.len == 0 { + 0 + } else { + fn_decl.stmts.last().pos.line_nr - fn_decl.pos.line_nr + } + if nr_lines > long_fns_cutoff { + vet.notice('Long function - ${nr_lines} lines long.', fn_decl.pos.line_nr, .long_fns) + } else if nr_lines == 0 { + vet.notice('Empty function.', fn_decl.pos.line_nr, .empty_fn) + } +} + +// vet_fn_analysis reports repeated code by scope +fn (mut vt VetAnalyze) vet_repeated_code(mut vet Vet) { + rlock vt.repeated_expr { + for fn_name, ref_expr in vt.repeated_expr { + scope_name := if fn_name == '' { 'global scope' } else { 'function scope (${fn_name})' } + for expr, info in ref_expr { + occurrences := arrays.sum(info.values().map(it.len)) or { 0 } + if occurrences < vt.repeated_expr_cutoff[expr] { + continue + } + for file, info_pos in info { + for k, pos in info_pos { + vet.notice_with_file(file, '${expr} occurs ${k + 1}/${occurrences} times in ${scope_name}.', + pos.line_nr, .repeated_code) + } + } + } + } + } +} + +// vet_code_analyze performs code analysis +fn (mut vt Vet) vet_code_analyze() { + if vt.opt.repeated_code { + vt.analyze.vet_repeated_code(mut vt) + } +} diff --git a/cmd/tools/vvet/errors.v b/cmd/tools/vvet/errors.v index 82aa38188adea9..7c9f63eacd22bd 100644 --- a/cmd/tools/vvet/errors.v +++ b/cmd/tools/vvet/errors.v @@ -13,6 +13,9 @@ pub enum FixKind { unknown doc vfmt + repeated_code + long_fns + empty_fn } // ErrorType is used to filter out false positive errors under specific conditions @@ -20,6 +23,7 @@ pub enum ErrorType { default space_indent trailing_space + long_fns } @[minify] @@ -40,13 +44,15 @@ fn (mut vt Vet) error(msg string, line int, fix FixKind) { pos := token.Pos{ line_nr: line + 1 } - vt.errors << VetError{ - message: msg - file_path: vt.file - pos: pos - kind: .error - fix: fix - typ: .default + lock vt.errors { + vt.errors << VetError{ + message: msg + file_path: vt.file + pos: pos + kind: .error + fix: fix + typ: .default + } } } @@ -64,9 +70,13 @@ fn (mut vt Vet) warn(msg string, line int, fix FixKind) { } if vt.opt.is_werror { w.kind = .error - vt.errors << w + lock vt.errors { + vt.errors << w + } } else { - vt.warns << w + lock vt.warns { + vt.warns << w + } } } @@ -74,13 +84,31 @@ fn (mut vt Vet) notice(msg string, line int, fix FixKind) { pos := token.Pos{ line_nr: line + 1 } - vt.notices << VetError{ - message: msg - file_path: vt.file - pos: pos - kind: .notice - fix: fix - typ: .default + lock vt.notices { + vt.notices << VetError{ + message: msg + file_path: vt.file + pos: pos + kind: .notice + fix: fix + typ: .default + } + } +} + +fn (mut vt Vet) notice_with_file(file string, msg string, line int, fix FixKind) { + pos := token.Pos{ + line_nr: line + 1 + } + lock vt.notices { + vt.notices << VetError{ + message: msg + file_path: file + pos: pos + kind: .notice + fix: fix + typ: .default + } } } diff --git a/cmd/tools/vvet/tests/empty_fn_decl.out b/cmd/tools/vvet/tests/empty_fn_decl.out new file mode 100644 index 00000000000000..d03f4a679dfa1c --- /dev/null +++ b/cmd/tools/vvet/tests/empty_fn_decl.out @@ -0,0 +1 @@ +cmd/tools/vvet/tests/empty_fn_decl.vv:2: notice: Empty function. diff --git a/cmd/tools/vvet/tests/empty_fn_decl.vv b/cmd/tools/vvet/tests/empty_fn_decl.vv new file mode 100644 index 00000000000000..6b2e6b95bb7e92 --- /dev/null +++ b/cmd/tools/vvet/tests/empty_fn_decl.vv @@ -0,0 +1,3 @@ +// vtest vflags: -F +fn foo() { +} diff --git a/cmd/tools/vvet/tests/repeated_assign.out b/cmd/tools/vvet/tests/repeated_assign.out new file mode 100644 index 00000000000000..34dd5648a23a1d --- /dev/null +++ b/cmd/tools/vvet/tests/repeated_assign.out @@ -0,0 +1,11 @@ +cmd/tools/vvet/tests/repeated_assign.vv:4: notice: a += 'foo' occurs 1/11 times in function scope (main.main). +cmd/tools/vvet/tests/repeated_assign.vv:5: notice: a += 'foo' occurs 2/11 times in function scope (main.main). +cmd/tools/vvet/tests/repeated_assign.vv:6: notice: a += 'foo' occurs 3/11 times in function scope (main.main). +cmd/tools/vvet/tests/repeated_assign.vv:7: notice: a += 'foo' occurs 4/11 times in function scope (main.main). +cmd/tools/vvet/tests/repeated_assign.vv:8: notice: a += 'foo' occurs 5/11 times in function scope (main.main). +cmd/tools/vvet/tests/repeated_assign.vv:9: notice: a += 'foo' occurs 6/11 times in function scope (main.main). +cmd/tools/vvet/tests/repeated_assign.vv:10: notice: a += 'foo' occurs 7/11 times in function scope (main.main). +cmd/tools/vvet/tests/repeated_assign.vv:11: notice: a += 'foo' occurs 8/11 times in function scope (main.main). +cmd/tools/vvet/tests/repeated_assign.vv:12: notice: a += 'foo' occurs 9/11 times in function scope (main.main). +cmd/tools/vvet/tests/repeated_assign.vv:13: notice: a += 'foo' occurs 10/11 times in function scope (main.main). +cmd/tools/vvet/tests/repeated_assign.vv:14: notice: a += 'foo' occurs 11/11 times in function scope (main.main). diff --git a/cmd/tools/vvet/tests/repeated_assign.vv b/cmd/tools/vvet/tests/repeated_assign.vv new file mode 100644 index 00000000000000..f41d75a27dfdea --- /dev/null +++ b/cmd/tools/vvet/tests/repeated_assign.vv @@ -0,0 +1,15 @@ +// vtest vflags: -r +fn main() { + mut a := '' + a += 'foo' + a += 'foo' + a += 'foo' + a += 'foo' + a += 'foo' + a += 'foo' + a += 'foo' + a += 'foo' + a += 'foo' + a += 'foo' + a += 'foo' +} diff --git a/cmd/tools/vvet/tests/repeated_code.out b/cmd/tools/vvet/tests/repeated_code.out new file mode 100644 index 00000000000000..d71e7315319628 --- /dev/null +++ b/cmd/tools/vvet/tests/repeated_code.out @@ -0,0 +1,12 @@ +cmd/tools/vvet/tests/repeated_code.vv:4: notice: a[0] occurs 1/12 times in function scope (main.main). +cmd/tools/vvet/tests/repeated_code.vv:5: notice: a[0] occurs 2/12 times in function scope (main.main). +cmd/tools/vvet/tests/repeated_code.vv:6: notice: a[0] occurs 3/12 times in function scope (main.main). +cmd/tools/vvet/tests/repeated_code.vv:6: notice: a[0] occurs 4/12 times in function scope (main.main). +cmd/tools/vvet/tests/repeated_code.vv:7: notice: a[0] occurs 5/12 times in function scope (main.main). +cmd/tools/vvet/tests/repeated_code.vv:8: notice: a[0] occurs 6/12 times in function scope (main.main). +cmd/tools/vvet/tests/repeated_code.vv:9: notice: a[0] occurs 7/12 times in function scope (main.main). +cmd/tools/vvet/tests/repeated_code.vv:9: notice: a[0] occurs 8/12 times in function scope (main.main). +cmd/tools/vvet/tests/repeated_code.vv:11: notice: a[0] occurs 9/12 times in function scope (main.main). +cmd/tools/vvet/tests/repeated_code.vv:12: notice: a[0] occurs 10/12 times in function scope (main.main). +cmd/tools/vvet/tests/repeated_code.vv:13: notice: a[0] occurs 11/12 times in function scope (main.main). +cmd/tools/vvet/tests/repeated_code.vv:13: notice: a[0] occurs 12/12 times in function scope (main.main). diff --git a/cmd/tools/vvet/tests/repeated_code.vv b/cmd/tools/vvet/tests/repeated_code.vv new file mode 100644 index 00000000000000..469a00ab5f1d77 --- /dev/null +++ b/cmd/tools/vvet/tests/repeated_code.vv @@ -0,0 +1,16 @@ +// vtest vflags: -r +fn main() { + mut a := [0, 2, 4, 6] + if a[0] { + dump(a[0]) + dump(a[0] + a[0]) + if a[0] { + dump(a[0]) + dump(a[0] + a[0]) + } + if a[0] { + dump(a[0]) + dump(a[0] + a[0]) + } + } +} diff --git a/cmd/tools/vvet/vet_test.v b/cmd/tools/vvet/vet_test.v index aefa820735d24b..a5e56b4ef95df4 100644 --- a/cmd/tools/vvet/vet_test.v +++ b/cmd/tools/vvet/vet_test.v @@ -3,6 +3,22 @@ import term import v.util.vtest import v.util.diff +struct FileOptions { +mut: + vflags string +} + +fn get_file_options(file string) FileOptions { + mut res := FileOptions{} + lines := os.read_lines(file) or { [] } + for line in lines { + if line.starts_with('// vtest vflags:') { + res.vflags = line.all_after(':').trim_space() + } + } + return res +} + fn test_vet() { vexe := os.getenv('VEXE') vroot := os.dir(vexe) @@ -26,7 +42,8 @@ fn check_path(vexe string, dir string, tests []string) int { for path in paths { program := path print(path + ' ') - res := os.execute('${os.quoted_path(vexe)} vet -nocolor ${os.quoted_path(program)}') + file_options := get_file_options(path) + res := os.execute('${os.quoted_path(vexe)} vet -nocolor ${file_options.vflags} ${os.quoted_path(program)}') if res.exit_code < 0 { panic(res.output) } diff --git a/cmd/tools/vvet/vvet.v b/cmd/tools/vvet/vvet.v index 46def5d7b7e812..64f1ed25529db4 100644 --- a/cmd/tools/vvet/vvet.v +++ b/cmd/tools/vvet/vvet.v @@ -11,14 +11,16 @@ import v.help import term import arrays +@[heap] struct Vet { mut: opt Options - errors []VetError - warns []VetError - notices []VetError + errors shared []VetError + warns shared []VetError + notices shared []VetError file string filtered_lines FilteredLines + analyze VetAnalyze } struct Options { @@ -28,6 +30,8 @@ struct Options { show_warnings bool use_color bool doc_private_fns_too bool + fn_sizing bool + repeated_code bool mut: is_vfmt_off bool } @@ -46,6 +50,8 @@ fn main() { doc_private_fns_too: '-p' in vet_options use_color: '-color' in vet_options || (term_colors && '-nocolor' !in vet_options) + repeated_code: '-r' in vet_options + fn_sizing: '-F' in vet_options } } mut paths := cmdline.only_non_options(vet_options) @@ -82,6 +88,7 @@ fn main() { }) } } + vt.vet_code_analyze() vfmt_err_count := vt.errors.filter(it.fix == .vfmt).len for n in vt.notices { eprintln(vt.e2string(n)) @@ -95,8 +102,10 @@ fn main() { eprintln(vt.e2string(err)) } if vfmt_err_count > 0 { - filtered_out := arrays.distinct(vt.errors.map(it.file_path)) - eprintln('Note: You can run `v fmt -w ${filtered_out.join(' ')}` to fix these errors automatically') + rlock vt.errors { + filtered_out := arrays.distinct(vt.errors.map(it.file_path)) + eprintln('Note: You can run `v fmt -w ${filtered_out.join(' ')}` to fix these errors automatically') + } } if vt.errors.len > 0 { exit(1) @@ -263,12 +272,35 @@ fn (mut vt Vet) stmts(stmts []ast.Stmt) { fn (mut vt Vet) stmt(stmt ast.Stmt) { match stmt { - ast.ConstDecl { vt.const_decl(stmt) } - ast.ExprStmt { vt.expr(stmt.expr) } - ast.Return { vt.exprs(stmt.exprs) } - ast.AssertStmt { vt.expr(stmt.expr) } - ast.AssignStmt { vt.exprs(stmt.right) } - ast.FnDecl { vt.stmts(stmt.stmts) } + ast.ConstDecl { + vt.const_decl(stmt) + } + ast.ExprStmt { + vt.expr(stmt.expr) + } + ast.Return { + vt.exprs(stmt.exprs) + } + ast.AssertStmt { + vt.expr(stmt.expr) + } + ast.AssignStmt { + vt.exprs(stmt.left) + vt.exprs(stmt.right) + vt.analyze.stmt(&vt, stmt) + } + ast.FnDecl { + old_fn_decl := vt.analyze.cur_fn + vt.analyze.cur_fn = stmt + vt.stmts(stmt.stmts) + if vt.opt.fn_sizing { + vt.analyze.long_or_empty_fns(mut vt, stmt) + } + vt.analyze.cur_fn = old_fn_decl + } + ast.StructDecl { + vt.exprs(stmt.fields.map(it.default_expr)) + } else {} } } @@ -284,16 +316,27 @@ fn (mut vt Vet) expr(expr ast.Expr) { ast.Comment { vt.filtered_lines.comments(expr.is_multi, expr.pos) } - ast.StringLiteral, ast.StringInterLiteral { + ast.StringLiteral { vt.filtered_lines.assigns(expr.pos) + vt.analyze.expr(&vt, expr) + } + ast.StringInterLiteral { + vt.filtered_lines.assigns(expr.pos) + vt.analyze.expr(&vt, expr) } ast.ArrayInit { vt.filtered_lines.assigns(expr.pos) + vt.expr(expr.len_expr) + vt.expr(expr.cap_expr) + vt.expr(expr.init_expr) + vt.exprs(expr.exprs) } ast.InfixExpr { vt.vet_in_condition(expr) vt.vet_empty_str(expr) + vt.expr(expr.left) vt.expr(expr.right) + vt.analyze.expr(&vt, expr) } ast.ParExpr { vt.expr(expr.expr) @@ -301,9 +344,12 @@ fn (mut vt Vet) expr(expr ast.Expr) { ast.CallExpr { vt.expr(expr.left) vt.exprs(expr.args.map(it.expr)) + vt.analyze.expr(&vt, expr) } ast.MatchExpr { + vt.expr(expr.cond) for b in expr.branches { + vt.exprs(b.exprs) vt.stmts(b.stmts) } } @@ -313,6 +359,29 @@ fn (mut vt Vet) expr(expr ast.Expr) { vt.stmts(b.stmts) } } + ast.SelectorExpr { + vt.analyze.expr(&vt, expr) + } + ast.IndexExpr { + vt.analyze.expr(&vt, expr) + } + ast.AsCast { + vt.analyze.expr(&vt, expr) + vt.expr(expr.expr) + } + ast.UnsafeExpr { + vt.expr(expr.expr) + } + ast.CastExpr { + vt.expr(expr.expr) + } + ast.StructInit { + vt.expr(expr.update_expr) + vt.exprs(expr.init_fields.map(it.expr)) + } + ast.DumpExpr { + vt.expr(expr.expr) + } else {} } } @@ -328,11 +397,7 @@ fn (mut vt Vet) const_decl(stmt ast.ConstDecl) { } fn (mut vt Vet) vet_empty_str(expr ast.InfixExpr) { - if expr.left is ast.InfixExpr { - vt.expr(expr.left) - } else if expr.right is ast.InfixExpr { - vt.expr(expr.right) - } else if expr.left is ast.SelectorExpr && expr.right is ast.IntegerLiteral { + if expr.left is ast.SelectorExpr && expr.right is ast.IntegerLiteral { operand := (expr.left as ast.SelectorExpr) // TODO: remove as-casts when multiple conds can be smart-casted. if operand.expr is ast.Ident && operand.expr.info.typ == ast.string_type_idx && operand.field_name == 'len' { diff --git a/vlib/v/help/common/vet.txt b/vlib/v/help/common/vet.txt index 7cfdb865a7019b..42e17af9e0cd75 100644 --- a/vlib/v/help/common/vet.txt +++ b/vlib/v/help/common/vet.txt @@ -12,5 +12,9 @@ Options: -v, -verbose Enable verbose logging. + -F Report empty and long function declaration (>300 lines). + -p Report private functions with missing documentation too (by default, only the `pub fn` functions will be reported). + + -r Report repeated piece of code (e.g. var[0], call()). \ No newline at end of file