diff --git a/cmd/tools/check-md.v b/cmd/tools/check-md.v index 7049000e09..59feed7c74 100644 --- a/cmd/tools/check-md.v +++ b/cmd/tools/check-md.v @@ -30,12 +30,12 @@ c) `v run cmd/tools/check-md.v -hide-warnings file.md` - same, but will not prin NB: There are several special keywords, which you can put after the code fences for v. These are: - compile - default, you do not need to specify it. cmd/tools/check-md.v compile the example. - ignore - ignore the example, useful for examples that just use the syntax highlighting - failcompile - known failing compilation. Useful for examples demonstrating compiler errors. - oksyntax - it should parse, it may not compile. Useful for partial examples. - badsyntax - known bad syntax, it should not even parse - wip - like ignore; a planned feature; easy to search. + compile - default, you do not need to specify it. cmd/tools/check-md.v compile the example. + ignore - ignore the example, useful for examples that just use the syntax highlighting + failcompile - known failing compilation. Useful for examples demonstrating compiler errors. + oksyntax - it should parse, it may not compile. Useful for partial examples. + badsyntax - known bad syntax, it should not even parse + wip - like ignore; a planned feature; easy to search. ') exit(0) } diff --git a/cmd/tools/vvet/tests/array_init_one_val.out b/cmd/tools/vvet/tests/array_init_one_val.out index 4b9c552af6..4486a126cd 100644 --- a/cmd/tools/vvet/tests/array_init_one_val.out +++ b/cmd/tools/vvet/tests/array_init_one_val.out @@ -1,2 +1,2 @@ -cmd/tools/vvet/tests/array_init_one_val.vv:2: Use `var == value` instead of `var in [value]` +cmd/tools/vvet/tests/array_init_one_val.vv:2: error: Use `var == value` instead of `var in [value]` NB: You can run `v fmt -w file.v` to fix these automatically diff --git a/cmd/tools/vvet/tests/indent_with_space.out b/cmd/tools/vvet/tests/indent_with_space.out index 782d40773c..bf0ed89463 100644 --- a/cmd/tools/vvet/tests/indent_with_space.out +++ b/cmd/tools/vvet/tests/indent_with_space.out @@ -1,2 +1,2 @@ -cmd/tools/vvet/tests/indent_with_space.vv:2: Looks like you are using spaces for indentation. +cmd/tools/vvet/tests/indent_with_space.vv:2: error: Looks like you are using spaces for indentation. NB: You can run `v fmt -w file.v` to fix these automatically diff --git a/cmd/tools/vvet/tests/module_file_test.out b/cmd/tools/vvet/tests/module_file_test.out new file mode 100644 index 0000000000..2c863adce1 --- /dev/null +++ b/cmd/tools/vvet/tests/module_file_test.out @@ -0,0 +1,4 @@ +cmd/tools/vvet/tests/module_file_test.vv:7: warning: Function documentation seems to be missing for "pub fn foo() string". +cmd/tools/vvet/tests/module_file_test.vv:13: warning: A function name is missing from the documentation of "pub fn bar() string". +cmd/tools/vvet/tests/module_file_test.vv:35: warning: Function documentation seems to be missing for "pub fn (f Foo) foo() string". +cmd/tools/vvet/tests/module_file_test.vv:46: warning: A function name is missing from the documentation of "pub fn (f Foo) fooo() string". diff --git a/cmd/tools/vvet/tests/module_file_test.vv b/cmd/tools/vvet/tests/module_file_test.vv new file mode 100644 index 0000000000..fe613263b7 --- /dev/null +++ b/cmd/tools/vvet/tests/module_file_test.vv @@ -0,0 +1,49 @@ +module foo + +struct Foo { + foo int +} + +pub fn foo() string { + // Missing doc + return 'foo' +} + +// foo does bar +pub fn bar() string { + // not using convention style: '// ' + return 'bar' +} + +// fooo does x +pub fn fooo() string { + // Documented + return 'fooo' +} + +// booo does x +fn booo() string { + // Documented, but not pub + return 'booo' +} + +fn boo() string { + // Missing doc + return 'boo' +} + +pub fn (f Foo) foo() string { + // Missing doc + return f.fo() +} + +fn (f Foo) fo() string { + // Missing doc, but not pub + return 'foo' +} + +// wrong doc +pub fn (f Foo) fooo() string { + // not using convention + return f.fo() +} diff --git a/cmd/tools/vvet/tests/parens_space_a.out b/cmd/tools/vvet/tests/parens_space_a.out index 6ae9fed8e1..bcb1c2db9e 100644 --- a/cmd/tools/vvet/tests/parens_space_a.out +++ b/cmd/tools/vvet/tests/parens_space_a.out @@ -1,2 +1,2 @@ -cmd/tools/vvet/tests/parens_space_a.vv:1: Looks like you are adding a space after `(` +cmd/tools/vvet/tests/parens_space_a.vv:1: error: Looks like you are adding a space after `(` NB: You can run `v fmt -w file.v` to fix these automatically diff --git a/cmd/tools/vvet/tests/parens_space_b.out b/cmd/tools/vvet/tests/parens_space_b.out index c9a5194a5c..96149c678d 100644 --- a/cmd/tools/vvet/tests/parens_space_b.out +++ b/cmd/tools/vvet/tests/parens_space_b.out @@ -1,2 +1,2 @@ -cmd/tools/vvet/tests/parens_space_b.vv:1: Looks like you are adding a space before `)` +cmd/tools/vvet/tests/parens_space_b.vv:1: error: Looks like you are adding a space before `)` NB: You can run `v fmt -w file.v` to fix these automatically diff --git a/cmd/tools/vvet/vet_test.v b/cmd/tools/vvet/vet_test.v index c4da78a07a..6209f9168c 100644 --- a/cmd/tools/vvet/vet_test.v +++ b/cmd/tools/vvet/vet_test.v @@ -1,6 +1,14 @@ import os import term import v.util.vtest +import v.util + +const diff_cmd = find_diff_cmd() + +fn find_diff_cmd() string { + res := util.find_working_diff_command() or { '' } + return res +} fn test_vet() { vexe := os.getenv('VEXE') @@ -25,7 +33,8 @@ fn check_path(vexe string, dir string, tests []string) int { for path in paths { program := path print(path + ' ') - res := os.exec('$vexe vet $program') or { panic(err) } + // -force is needed so that `v vet` would not skip the regression files + res := os.exec('$vexe vet -force $program') or { panic(err) } mut expected := os.read_file(program.replace('.vv', '') + '.out') or { panic(err) } expected = clean_line_endings(expected) found := clean_line_endings(res.output) @@ -38,6 +47,9 @@ fn check_path(vexe string, dir string, tests []string) int { println('found:') println(found) println('============\n') + println('diff:') + println(util.color_compare_strings(diff_cmd, found, expected)) + println('============\n') nb_fail++ } else { println(term.green('OK')) diff --git a/cmd/tools/vvet/vvet.v b/cmd/tools/vvet/vvet.v index f16f7fd243..2f1cd6d704 100644 --- a/cmd/tools/vvet/vvet.v +++ b/cmd/tools/vvet/vvet.v @@ -2,80 +2,248 @@ // Use of this source code is governed by an MIT license that can be found in the LICENSE file. module main +import os +import os.cmdline import v.vet import v.pref import v.parser -import v.util import v.table -import os -import os.cmdline +import v.token -struct VetOptions { - is_verbose bool +struct Vet { + opt Options mut: - errors []vet.Error + errors []vet.Error + file string } -fn (vet_options &VetOptions) vprintln(s string) { - if !vet_options.is_verbose { +struct Options { + is_verbose bool +} + +fn (vet &Vet) vprintln(s string) { + if !vet.opt.is_verbose { return } println(s) } +const vet_options = cmdline.options_after(os.args, ['vet']) + +const is_force = '-force' in vet_options + +const is_verbose = '-verbose' in vet_options || '-v' in vet_options + +const show_warnings = '-hide-warnings' !in vet_options + fn main() { - args := util.join_env_vflags_and_os_args() - paths := cmdline.only_non_options(cmdline.options_after(args, ['vet'])) - mut vet_options := VetOptions{ - is_verbose: '-verbose' in args || '-v' in args + opt := Options{ + is_verbose: is_verbose } + mut vet := Vet{ + opt: opt + } + mut paths := cmdline.only_non_options(vet_options) + vtmp := os.getenv('VTMP') + if vtmp != '' { + // `v test-cleancode` passes also `-o tmpfolder` as well as all options in VFLAGS + paths = paths.filter(!it.starts_with(vtmp)) + } + // for path in paths { if !os.exists(path) { eprintln('File/folder $path does not exist') continue } - if path.ends_with('_test.v') || - (path.contains('/tests/') && !path.contains('cmd/tools/vvet/tests/')) { - eprintln('skipping $path') - continue - } if path.ends_with('.v') || path.ends_with('.vv') { - vet_options.vet_file(path) - } else if os.is_dir(path) { - vet_options.vprintln("vetting folder '$path'...") + if path.contains('cmd/tools/vvet/tests/') { + if is_force || paths.len == 1 { + vet.vet_file(path, true) + continue + } else { + // The .vv files in that folder, are regression tests + // for `v vet` itself and thus are known to fail in + // a predictable way. They are run 1 by 1 by vet_test.v. + // They *should be skipped*, when run by more general + // invocations like for example `v vet cmd/tools` + eprintln("skipping vvet regression file: '$path' ...") + continue + } + } + } + if os.is_dir(path) { + vet.vprintln("vetting folder: '$path' ...") vfiles := os.walk_ext(path, '.v') vvfiles := os.walk_ext(path, '.vv') mut files := []string{} files << vfiles files << vvfiles for file in files { - if file.ends_with('_test.v') || file.contains('/tests/') { // TODO copy pasta + if !is_force && file.ends_with('.vv') && file.contains('cmd/tools/vvet/tests/') { continue } - vet_options.vet_file(file) + vet.vet_file(file, false) } } } - if vet_options.errors.len > 0 { - for err in vet_options.errors.filter(it.kind == .error) { - eprintln('$err.file_path:$err.pos.line_nr: $err.message') + // + warnings := vet.errors.filter(it.kind == .warning) + errors := vet.errors.filter(it.kind == .error) + errors_vfmt := vet.errors.filter(it.kind == .error && it.fix == .vfmt) + if show_warnings { + for err in warnings { + eprintln('$err.file_path:$err.pos.line_nr: warning: $err.message') } + } + for err in errors { + eprintln('$err.file_path:$err.pos.line_nr: error: $err.message') + } + if errors_vfmt.len > 0 { eprintln('NB: You can run `v fmt -w file.v` to fix these automatically') - /* - for err in vet_options.errors.filter(it.kind == .warning) { - eprintln('$err.file_path:$err.pos.line_nr: err.message') - } - */ + } + if errors.len > 0 { exit(1) } } -fn (mut vet_options VetOptions) vet_file(path string) { +fn (mut v Vet) error(msg string, line int, fix vet.FixKind) { + pos := token.Position{ + line_nr: line + 1 + } + v.errors << vet.Error{ + message: msg + file_path: v.file + pos: pos + kind: .error + fix: fix + } +} + +fn (mut v Vet) warn(msg string, line int, fix vet.FixKind) { + pos := token.Position{ + line_nr: line + 1 + } + v.errors << vet.Error{ + message: msg + file_path: v.file + pos: pos + kind: .warning + fix: fix + } +} + +// vet_file vets the file read from `path`. +fn (mut vt Vet) vet_file(path string, is_regression_test bool) { + if path.contains('/tests/') && !is_regression_test { + // skip all /tests/ files, since usually their content is not + // important enough to be documented/vetted, and they may even + // contain intentionally invalid code. + eprintln("skipping test file: '$path' ...") + return + } + vt.file = path mut prefs := pref.new_preferences() prefs.is_vet = true table := table.new_table() - vet_options.vprintln("vetting file '$path'...") + vt.vprintln("vetting file '$path'...") _, errors := parser.parse_vet_file(path, table, prefs) // Transfer errors from scanner and parser - vet_options.errors << errors + vt.errors << errors + // Scan each line in file for things to improve + source_lines := os.read_lines(vt.file) or { []string{} } + for lnumber, line in source_lines { + vt.vet_line(source_lines, line, lnumber) + } +} + +// vet_line vets the contents of `line` from `vet.file`. +fn (mut vet Vet) vet_line(lines []string, line string, lnumber int) { + // Vet public functions + if line.starts_with('pub fn') || + (line.starts_with('fn ') && !(line.starts_with('fn C.') || line.starts_with('fn main'))) { + // Scan function declarations for missing documentation + is_pub_fn := line.starts_with('pub fn') + if lnumber > 0 { + collect_tags := fn (line string) []string { + mut cleaned := line.all_before('/') + cleaned = cleaned.replace_each(['[', '', ']', '', ' ', '']) + return cleaned.split(',') + } + ident_fn_name := fn (line string) string { + mut fn_idx := line.index(' fn ') or { return '' } + mut skip := false + mut p_count := 0 + mut fn_name := '' + for i := fn_idx + 4; i < line.len; i++ { + char := line[i] + if !skip && char == `(` { + p_count++ + skip = true + continue + } else if skip && char == `)` { + skip = false + continue + } else if char == ` ` { + continue + } else if char.is_letter() { + // fn_name += char.str() + fn_name = line[i..].all_before('(') + break + } + if p_count > 1 { + break + } + } + return fn_name + } + mut line_above := lines[lnumber - 1] + mut tags := []string{} + if !line_above.starts_with('//') { + mut grab := true + for j := lnumber - 1; j >= 0; j-- { + prev_line := lines[j] + if prev_line.contains('}') { // We've looked back to the above scope, stop here + break + } else if prev_line.starts_with('[') { + tags << collect_tags(prev_line) + continue + } else if prev_line.starts_with('//') { // Single-line comment + grab = false + break + } + } + if grab { + clean_line := line.all_before_last('{').trim(' ') + if is_pub_fn { + vet.warn('Function documentation seems to be missing for "$clean_line".', + lnumber, .doc) + } + } + } else { + fn_name := ident_fn_name(line) + mut grab := true + for j := lnumber - 1; j >= 0; j-- { + prev_line := lines[j] + if prev_line.contains('}') { // We've looked back to the above scope, stop here + break + } else if prev_line.starts_with('// $fn_name ') { + grab = false + break + } else if prev_line.starts_with('[') { + tags << collect_tags(prev_line) + continue + } else if prev_line.starts_with('//') { // Single-line comment + continue + } + } + if grab { + clean_line := line.all_before_last('{').trim(' ') + if is_pub_fn { + vet.warn('A function name is missing from the documentation of "$clean_line".', + lnumber, .doc) + } + } + } + } + } } diff --git a/vlib/v/vet/vet.v b/vlib/v/vet/vet.v index 7176379d19..c0e4bcd36d 100644 --- a/vlib/v/vet/vet.v +++ b/vlib/v/vet/vet.v @@ -11,6 +11,7 @@ pub enum ErrorKind { pub enum FixKind { unknown + doc vfmt }