vet: add an -I option to notice fns, with the potential to be inlined (#23534)

felipensp · web-flow · commit 45b79dfb9711 · 2025-01-22T15:58:02.000+02:00
diff --git a/cmd/tools/vast/vast.v b/cmd/tools/vast/vast.v
@@ -582,6 +582,7 @@ fn (t Tree) fn_decl(node ast.FnDecl) &Node {
 	obj.add('is_direct_arr', t.bool_node(node.is_direct_arr))
 	obj.add('ctdefine_idx', t.number_node(node.ctdefine_idx))
 	obj.add('pos', t.pos(node.pos))
+	obj.add('end_pos', t.pos(node.end_pos))
 	obj.add('body_pos', t.pos(node.body_pos))
 	obj.add('return_type_pos', t.pos(node.return_type_pos))
 	obj.add('file', t.string_node(node.file))
diff --git a/cmd/tools/vvet/analyze.v b/cmd/tools/vvet/analyze.v
@@ -4,29 +4,36 @@ module main
 
 import v.ast
 import v.token
+import os
 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
+const indexexpr_cutoff = os.getenv_opt('VET_INDEXEXPR_CUTOFF') or { '10' }.int()
+const infixexpr_cutoff = os.getenv_opt('VET_INFIXEXPR_CUTOFF') or { '10' }.int()
+const selectorexpr_cutoff = os.getenv_opt('VET_SELECTOREXPR_CUTOFF') or { '10' }.int()
+const callexpr_cutoff = os.getenv_opt('VET_CALLEXPR_CUTOFF') or { '10' }.int()
+const stringinterliteral_cutoff = os.getenv_opt('STRINGINTERLITERAL_CUTOFF') or { '10' }.int()
+const stringliteral_cutoff = os.getenv_opt('STRINGLITERAL_CUTOFF') or { '10' }.int()
+const ascast_cutoff = os.getenv_opt('ASCAST_CUTOFF') or { '10' }.int()
+const stringconcat_cutoff = os.getenv_opt('STRINGCONCAT_CUTOFF') or { '10' }.int()
+
+// possibly inline fn cutoff
+const fns_call_cutoff = os.getenv_opt('VET_FNS_CALL_CUTOFF') or { '10' }.int() // at least N calls
+const short_fns_cutoff = os.getenv_opt('VET_SHORT_FNS_CUTOFF') or { '3' }.int() // lines
 
 // minimum size for string literals
-const stringliteral_min_size = 20
+const stringliteral_min_size = os.getenv_opt('VET_STRINGLITERAL_MIN_SIZE') or { '20' }.int()
 
 // long functions cutoff
-const long_fns_cutoff = 300
+const long_fns_cutoff = os.getenv_opt('VET_LONG_FNS_CUTOFF') or { '300' }.int()
 
 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
+	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
+	potential_non_inlined shared map[string]map[string]token.Pos              // fns might be inlined
+	call_counter          shared map[string]int // fn call counter
+	cur_fn                ast.FnDecl            // current fn declaration
 }
 
 // stmt checks for repeated code in statements
@@ -80,9 +87,18 @@ fn (mut vt VetAnalyze) expr(vet &Vet, expr ast.Expr) {
 		}
 		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(', ')})',
+				left_str := expr.left.str()
+				lock vt.call_counter {
+					if vt.cur_fn.receiver.name == left_str {
+						vt.call_counter['${int(vt.cur_fn.receiver.typ)}.${expr.name}']++
+					}
+				}
+				vt.save_expr(callexpr_cutoff, '${left_str}.${expr.name}(${expr.args.map(it.str()).join(', ')})',
 					vet.file, expr.pos)
 			} else {
+				lock vt.call_counter {
+					vt.call_counter[expr.name]++
+				}
 				vt.save_expr(callexpr_cutoff, '${expr.name}(${expr.args.map(it.str()).join(', ')})',
 					vet.file, expr.pos)
 			}
@@ -104,18 +120,27 @@ fn (mut vt VetAnalyze) expr(vet &Vet, expr ast.Expr) {
 
 // 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
-	}
+	nr_lines := fn_decl.end_pos.line_nr - fn_decl.pos.line_nr - 2
 	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)
 	}
 }
 
+// potential_non_inlined checks for potential fns to be inlined
+fn (mut vt VetAnalyze) potential_non_inlined(mut vet Vet, fn_decl ast.FnDecl) {
+	nr_lines := fn_decl.end_pos.line_nr - fn_decl.pos.line_nr - 2
+	if nr_lines < short_fns_cutoff {
+		attr := fn_decl.attrs.find_first('inline')
+		if attr == none {
+			lock vt.potential_non_inlined {
+				vt.potential_non_inlined[fn_decl.fkey()][vet.file] = fn_decl.pos
+			}
+		}
+	}
+}
+
 // vet_fn_analysis reports repeated code by scope
 fn (mut vt VetAnalyze) vet_repeated_code(mut vet Vet) {
 	rlock vt.repeated_expr {
@@ -137,9 +162,26 @@ fn (mut vt VetAnalyze) vet_repeated_code(mut vet Vet) {
 	}
 }
 
+// vet_inlining_fn reports possible fn to be inlined
+fn (mut vt VetAnalyze) vet_inlining_fn(mut vet Vet) {
+	for fn_name, info in vt.potential_non_inlined {
+		for file, pos in info {
+			calls := vt.call_counter[fn_name] or { 0 }
+			if calls < fns_call_cutoff {
+				continue
+			}
+			vet.notice_with_file(file, '${fn_name.all_after('.')} fn might be inlined (possibly called at least ${calls} times)',
+				pos.line_nr, .inline_fn)
+		}
+	}
+}
+
 // 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)
 	}
+	if vt.opt.fn_inlining {
+		vt.analyze.vet_inlining_fn(mut vt)
+	}
 }
diff --git a/cmd/tools/vvet/errors.v b/cmd/tools/vvet/errors.v
@@ -16,14 +16,14 @@ pub enum FixKind {
 	repeated_code
 	long_fns
 	empty_fn
+	inline_fn
 }
 
 // ErrorType is used to filter out false positive errors under specific conditions
 pub enum ErrorType {
 	default
 	space_indent
 	trailing_space
-	long_fns
 }
 
 @[minify]
diff --git a/cmd/tools/vvet/vvet.v b/cmd/tools/vvet/vvet.v
@@ -32,6 +32,7 @@ struct Options {
 	doc_private_fns_too bool
 	fn_sizing           bool
 	repeated_code       bool
+	fn_inlining         bool
 mut:
 	is_vfmt_off bool
 }
@@ -52,6 +53,7 @@ fn main() {
 				|| (term_colors && '-nocolor' !in vet_options)
 			repeated_code:       '-r' in vet_options
 			fn_sizing:           '-F' in vet_options
+			fn_inlining:         '-I' in vet_options
 		}
 	}
 	mut paths := cmdline.only_non_options(vet_options)
@@ -296,6 +298,9 @@ fn (mut vt Vet) stmt(stmt ast.Stmt) {
 			if vt.opt.fn_sizing {
 				vt.analyze.long_or_empty_fns(mut vt, stmt)
 			}
+			if vt.opt.fn_inlining {
+				vt.analyze.potential_non_inlined(mut vt, stmt)
+			}
 			vt.analyze.cur_fn = old_fn_decl
 		}
 		ast.StructDecl {
diff --git a/vlib/v/ast/ast.v b/vlib/v/ast/ast.v
@@ -630,6 +630,7 @@ pub mut:
 	scope       &Scope = unsafe { nil }
 	label_names []string
 	pos         token.Pos // function declaration position
+	end_pos     token.Pos // end position
 	//
 	is_expand_simple_interpolation bool // true, when @[expand_simple_interpolation] is used on a fn. It should have a single string argument.
 }
diff --git a/vlib/v/help/common/vet.txt b/vlib/v/help/common/vet.txt
@@ -12,7 +12,10 @@ Options:
 
   -v, -verbose        Enable verbose logging.
 
-  -F                  Report empty and long function declaration (>300 lines).
+  -F                  Report empty and long function declaration 
+                      (default: >300 lines).
+
+  -I                  Report potential function to be inlined.
 
   -p                  Report private functions with missing documentation too
                       (by default, only the `pub fn` functions will be reported).
diff --git a/vlib/v/parser/fn.v b/vlib/v/parser/fn.v
@@ -707,6 +707,7 @@ run them via `v file.v` instead',
 		language:              language
 		no_body:               no_body
 		pos:                   start_pos.extend_with_last_line(end_pos, p.prev_tok.line_nr)
+		end_pos:               p.tok.pos()
 		name_pos:              name_pos
 		body_pos:              body_start_pos
 		file:                  p.file_path

Original file line number	Diff line number	Diff line change
`@@ -16,14 +16,14 @@ pub enum FixKind {`
`16`	`16`	`repeated_code`
`17`	`17`	`long_fns`
`18`	`18`	`empty_fn`
	`19`	`+ inline_fn`
`19`	`20`	`}`
`20`	`21`
`21`	`22`	`// ErrorType is used to filter out false positive errors under specific conditions`
`22`	`23`	`pub enum ErrorType {`
`23`	`24`	`default`
`24`	`25`	`space_indent`
`25`	`26`	`trailing_space`
`26`		`- long_fns`
`27`	`27`	`}`
`28`	`28`
`29`	`29`	`@[minify]`
Original file line number	Diff line number	Diff line change
`@@ -32,6 +32,7 @@ struct Options {`
`32`	`32`	`doc_private_fns_too bool`
`33`	`33`	`fn_sizing bool`
`34`	`34`	`repeated_code bool`
	`35`	`+ fn_inlining bool`
`35`	`36`	`mut:`
`36`	`37`	`is_vfmt_off bool`
`37`	`38`	`}`
`@@ -52,6 +53,7 @@ fn main() {`
`52`	`53`	`\|\| (term_colors && '-nocolor' !in vet_options)`
`53`	`54`	`repeated_code: '-r' in vet_options`
`54`	`55`	`fn_sizing: '-F' in vet_options`
	`56`	`+ fn_inlining: '-I' in vet_options`
`55`	`57`	`}`
`56`	`58`	`}`
`57`	`59`	`mut paths := cmdline.only_non_options(vet_options)`
`@@ -296,6 +298,9 @@ fn (mut vt Vet) stmt(stmt ast.Stmt) {`
`296`	`298`	`if vt.opt.fn_sizing {`
`297`	`299`	`vt.analyze.long_or_empty_fns(mut vt, stmt)`
`298`	`300`	`}`
	`301`	`+ if vt.opt.fn_inlining {`
	`302`	`+ vt.analyze.potential_non_inlined(mut vt, stmt)`
	`303`	`+ }`
`299`	`304`	`vt.analyze.cur_fn = old_fn_decl`
`300`	`305`	`}`
`301`	`306`	`ast.StructDecl {`
Original file line number	Diff line number	Diff line change
`@@ -630,6 +630,7 @@ pub mut:`
`630`	`630`	`scope &Scope = unsafe { nil }`
`631`	`631`	`label_names []string`
`632`	`632`	`pos token.Pos // function declaration position`
	`633`	`+ end_pos token.Pos // end position`
`633`	`634`	`//`
`634`	`635`	`is_expand_simple_interpolation bool // true, when @[expand_simple_interpolation] is used on a fn. It should have a single string argument.`
`635`	`636`	`}`