Skip to content
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

Conversation

felipensp
Copy link
Member

@felipensp felipensp commented Jan 8, 2025

This PR fixes:

  • vvet was not working when running on dir.

This PR implements:

  • track for empty fns (notice for empty fns) -F option
  • track for long fns (not customized yet - >300 lines fns) -F option
  • track for repeated expr in specific fn scope (currently: CallExpr, SelectorExpr, AsCast, InfixExpr, StringLiteral, StringInterLiteral and IndexExpr and AssignStmt) -r option

image

image

Huly®: V_0.6-21836

@felipensp felipensp marked this pull request as ready for review January 8, 2025 16:30
@spytheman
Copy link
Member

spytheman commented Jan 8, 2025

Please add tests too in cmd/tools/vvet/tests/, as pairs of .vv and .out files .

You can modify cmd/tools/vvet/vet_test.v, to read each .vv file,
and match lines that start with // vtest vflags:,
then use the options on that line (if present), in order to pass:
// vtest vflags: -r or // vtest vflags: -F for each new .vv file.

(vlib/v/gen/c/coutput_test.v has a get_file_options function that does it)

@felipensp
Copy link
Member Author

felipensp commented Jan 8, 2025

Please add tests too in cmd/tools/vvet/tests/, as pairs of .vv and .out files .

You can modify cmd/tools/vvet/vet_test.v, to read each .vv file, and match lines that start with // vtest vflags:, then use the options on that line (if present), in order to pass: // vtest vflags: -r or // vtest vflags: -F for each new .vv file.

(vlib/v/gen/c/coutput_test.v has a get_file_options function that does it)

Thanks. I just add it.

Comment on lines +10 to +23
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
Copy link
Member

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.

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
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why is it shared?
Afaik v vet is not parallel?

Copy link
Member

@spytheman spytheman left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Excellent work.

@spytheman spytheman merged commit f75aa34 into vlang:master Jan 9, 2025
61 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants