Skip to content

Commit

Permalink
vvet: fix for v vet folder/ + new features (track long fns, empty f…
Browse files Browse the repository at this point in the history
…ns and repeated code), enabled by the new -F and -r flags (#23405)
  • Loading branch information
felipensp authored Jan 9, 2025
1 parent 0ee49c6 commit f75aa34
Show file tree
Hide file tree
Showing 11 changed files with 351 additions and 34 deletions.
145 changes: 145 additions & 0 deletions cmd/tools/vvet/analyze.v
Original file line number Diff line number Diff line change
@@ -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)
}
}
60 changes: 44 additions & 16 deletions cmd/tools/vvet/errors.v
Original file line number Diff line number Diff line change
Expand Up @@ -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]
Expand All @@ -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
}
}
}

Expand All @@ -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
}
}
}

Expand Down
1 change: 1 addition & 0 deletions cmd/tools/vvet/tests/empty_fn_decl.out
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
cmd/tools/vvet/tests/empty_fn_decl.vv:2: notice: Empty function.
3 changes: 3 additions & 0 deletions cmd/tools/vvet/tests/empty_fn_decl.vv
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
// vtest vflags: -F
fn foo() {
}
11 changes: 11 additions & 0 deletions cmd/tools/vvet/tests/repeated_assign.out
Original file line number Diff line number Diff line change
@@ -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).
15 changes: 15 additions & 0 deletions cmd/tools/vvet/tests/repeated_assign.vv
Original file line number Diff line number Diff line change
@@ -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'
}
12 changes: 12 additions & 0 deletions cmd/tools/vvet/tests/repeated_code.out
Original file line number Diff line number Diff line change
@@ -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).
16 changes: 16 additions & 0 deletions cmd/tools/vvet/tests/repeated_code.vv
Original file line number Diff line number Diff line change
@@ -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])
}
}
}
19 changes: 18 additions & 1 deletion cmd/tools/vvet/vet_test.v
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand All @@ -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)
}
Expand Down
Loading

0 comments on commit f75aa34

Please sign in to comment.