-
-
Notifications
You must be signed in to change notification settings - Fork 2.2k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
vvet: fix for dir running + new features (track long fns, empty fns and repeated code) #23405
Merged
spytheman
merged 16 commits into
vlang:master
from
felipensp:feature_vvet_repeated_code_long_and_empty_fns
Jan 9, 2025
Merged
Changes from all commits
Commits
Show all changes
16 commits
Select commit
Hold shift + click to select a range
9500100
implement
felipensp ca394dc
fix
felipensp 2ed146e
cleanup
felipensp c2fbc37
fix cleanup
felipensp c6360a5
add option -r -F
felipensp 61e2b54
cleanup + add nested SelectorExpr detection
felipensp de0b8be
implement fn scoped repeated code detection
felipensp 568ddba
fix cleanup
felipensp 6c8b321
cleanup
felipensp 71beb58
cleanup
felipensp 9034316
improve
felipensp 650149e
cleanup
felipensp 1341735
cleanup
felipensp 9e50472
cleanup
felipensp b42f8e7
improve
felipensp 5d5fc8f
added tests
felipensp File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
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 | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Why is it shared? |
||
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) | ||
} | ||
} |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
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. |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,3 @@ | ||
// vtest vflags: -F | ||
fn foo() { | ||
} |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
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). |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
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' | ||
} |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
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). |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
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]) | ||
} | ||
} | ||
} |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
These should be customizable.
An easy way for that is with something like:
const indexexpr_cutoff = os.getenv_opt('VVET_INDEXEXPR', '10').int()
The negative is that setting env variables on windows in cmd for one off commands is clumsier.