vvet: fix for v vet folder/ + new features (track long fns, empty fns and repeated code), enabled by the new -F and -r flags (#23405)

felipensp · web-flow · commit f75aa3458092 · 2025-01-09T09:46:35.000+02:00
diff --git a/cmd/tools/vvet/analyze.v 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
@@ -13,13 +13,17 @@ pub enum FixKind {
 	unknown
 	doc
 	vfmt
+	repeated_code
+	long_fns
+	empty_fn
 }
 
 // ErrorType is used to filter out false positive errors under specific conditions
 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,23 +70,45 @@ 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
+		}
 	}
 }
 
 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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
diff --git a/vlib/v/help/common/vet.txt b/vlib/v/help/common/vet.txt

Original file line number	Diff line number	Diff line change
`@@ -0,0 +1 @@`
	`1`	`+cmd/tools/vvet/tests/empty_fn_decl.vv:2: notice: Empty function.`
Original file line number	Diff line number	Diff line change
`@@ -0,0 +1,3 @@`
	`1`	`+// vtest vflags: -F`
	`2`	`+fn foo() {`
	`3`	`+}`