From 31efb48fc509d245894b201fadc602c915bf8665 Mon Sep 17 00:00:00 2001 From: Larpon Date: Fri, 27 May 2022 17:19:06 +0200 Subject: [PATCH] tools: implement `v missdoc --diff oldv newv` (#14537) --- .github/workflows/docs_ci.yml | 14 +-- cmd/tools/vcomplete.v | 2 + cmd/tools/vmissdoc.v | 162 ++++++++++++++++++++++++++++------ cmd/v/help/missdoc.txt | 24 ++++- 4 files changed, 163 insertions(+), 39 deletions(-) diff --git a/.github/workflows/docs_ci.yml b/.github/workflows/docs_ci.yml index 6fb54553ca..d25c1ae25c 100644 --- a/.github/workflows/docs_ci.yml +++ b/.github/workflows/docs_ci.yml @@ -21,7 +21,7 @@ jobs: runs-on: ubuntu-20.04 timeout-minutes: 5 env: - MOPTIONS: --no-line-numbers --relative-paths --exclude /vlib/v/ --exclude /builtin/linux_bare/ --exclude /testdata/ --exclude /tests/ vlib/ + MOPTIONS: --relative-paths --exclude /vlib/v/ --exclude /builtin/linux_bare/ --exclude /testdata/ --exclude /tests/ steps: - uses: actions/checkout@v2 - name: Build V @@ -35,14 +35,4 @@ jobs: - name: Check against parent commit run: | - ./v missdoc $MOPTIONS | sort > /tmp/n_v.txt - cd pv/ && ../v missdoc $MOPTIONS | sort > /tmp/o_v.txt - count_new=$(cat /tmp/n_v.txt | wc -l) - count_old=$(cat /tmp/o_v.txt | wc -l) - echo "new pubs: $count_new | old pubs: $count_old" - echo "new head: $(head -n1 /tmp/n_v.txt)" - echo "old head: $(head -n1 /tmp/o_v.txt)" - if [[ ${count_new} -gt ${count_old} ]]; then - echo "The following $((count_new-count_old)) function(s) are introduced with no documentation:" - diff /tmp/n_v.txt /tmp/o_v.txt ## diff does exit(1) when files are different - fi + ./v missdoc --diff $MOPTIONS pv/vlib vlib diff --git a/cmd/tools/vcomplete.v b/cmd/tools/vcomplete.v index 78f150a969..a7581c0a0d 100644 --- a/cmd/tools/vcomplete.v +++ b/cmd/tools/vcomplete.v @@ -259,6 +259,8 @@ const ( '--relative-paths', '-r', '--js', + '--verify', + '--diff', ] auto_complete_flags_self = [ '-prod', diff --git a/cmd/tools/vmissdoc.v b/cmd/tools/vmissdoc.v index 11e0afad2e..cd1d9bfa77 100644 --- a/cmd/tools/vmissdoc.v +++ b/cmd/tools/vmissdoc.v @@ -6,12 +6,13 @@ import flag const ( tool_name = 'v missdoc' - tool_version = '0.0.5' + tool_version = '0.1.0' tool_description = 'Prints all V functions in .v files under PATH/, that do not yet have documentation comments.' - work_dir_prefix = normalise_path(os.real_path(os.wd_at_startup) + '/') + work_dir_prefix = normalise_path(os.real_path(os.wd_at_startup) + os.path_separator) ) struct UndocumentedFN { + file string line int signature string tags []string @@ -26,12 +27,15 @@ struct Options { no_line_numbers bool exclude []string relative_paths bool +mut: verify bool + diff bool + additional_args []string } -fn (opt Options) report_undocumented_functions_in_path(path string) int { +fn (opt Options) collect_undocumented_functions_in_dir(directory string) []UndocumentedFN { mut files := []string{} - collect(path, mut files, fn (npath string, mut accumulated_paths []string) { + collect(directory, mut files, fn (npath string, mut accumulated_paths []string) { if !npath.ends_with('.v') { return } @@ -40,7 +44,7 @@ fn (opt Options) report_undocumented_functions_in_path(path string) int { } accumulated_paths << npath }) - mut undocumented_fn_total := 0 + mut undocumented_fns := []UndocumentedFN{} for file in files { if !opt.js && file.ends_with('.js.v') { continue @@ -48,16 +52,16 @@ fn (opt Options) report_undocumented_functions_in_path(path string) int { if opt.exclude.len > 0 && opt.exclude.any(file.contains(it)) { continue } - undocumented_fn_total += opt.report_undocumented_functions_in_file(file) + undocumented_fns << opt.collect_undocumented_functions_in_file(file) } - return undocumented_fn_total + return undocumented_fns } -fn (opt &Options) report_undocumented_functions_in_file(nfile string) int { +fn (opt &Options) collect_undocumented_functions_in_file(nfile string) []UndocumentedFN { file := os.real_path(nfile) contents := os.read_file(file) or { panic(err) } lines := contents.split('\n') - mut info := []UndocumentedFN{} + mut list := []UndocumentedFN{} for i, line in lines { if line.starts_with('pub fn') || (opt.private && (line.starts_with('fn ') && !(line.starts_with('fn C.') || line.starts_with('fn main')))) { @@ -81,14 +85,39 @@ fn (opt &Options) report_undocumented_functions_in_file(nfile string) int { } if grab { clean_line := line.all_before_last(' {') - info << UndocumentedFN{i + 1, clean_line, tags} + list << UndocumentedFN{ + line: i + 1 + signature: clean_line + tags: tags + file: file + } } } } } } - if info.len > 0 { - for undocumented_fn in info { + return list +} + +fn (opt &Options) collect_undocumented_functions_in_path(path string) []UndocumentedFN { + mut undocumented_functions := []UndocumentedFN{} + if os.is_file(path) { + undocumented_functions << opt.collect_undocumented_functions_in_file(path) + } else { + undocumented_functions << opt.collect_undocumented_functions_in_dir(path) + } + return undocumented_functions +} + +fn (opt &Options) report_undocumented_functions_in_path(path string) int { + mut list := opt.collect_undocumented_functions_in_path(path) + opt.report_undocumented_functions(list) + return list.len +} + +fn (opt &Options) report_undocumented_functions(list []UndocumentedFN) { + if list.len > 0 { + for undocumented_fn in list { mut line_numbers := '$undocumented_fn.line:0:' if opt.no_line_numbers { line_numbers = '' @@ -98,10 +127,11 @@ fn (opt &Options) report_undocumented_functions_in_file(nfile string) int { } else { '' } + file := undocumented_fn.file ofile := if opt.relative_paths { - nfile.replace(work_dir_prefix, '') + file.replace(work_dir_prefix, '') } else { - os.real_path(nfile) + os.real_path(file) } if opt.deprecated { println('$ofile:$line_numbers$undocumented_fn.signature $tags_str') @@ -119,7 +149,54 @@ fn (opt &Options) report_undocumented_functions_in_file(nfile string) int { } } } - return info.len +} + +fn (opt &Options) diff_undocumented_functions_in_paths(path_old string, path_new string) []UndocumentedFN { + old := os.real_path(path_old) + new := os.real_path(path_new) + + mut old_undocumented_functions := opt.collect_undocumented_functions_in_path(old) + mut new_undocumented_functions := opt.collect_undocumented_functions_in_path(new) + + mut differs := []UndocumentedFN{} + if new_undocumented_functions.len > old_undocumented_functions.len { + for new_undoc_fn in new_undocumented_functions { + new_relative_file := new_undoc_fn.file.replace(new, '').trim_string_left(os.path_separator) + mut found := false + for old_undoc_fn in old_undocumented_functions { + old_relative_file := old_undoc_fn.file.replace(old, '').trim_string_left(os.path_separator) + if new_relative_file == old_relative_file + && new_undoc_fn.signature == old_undoc_fn.signature { + found = true + break + } + } + if !found { + differs << new_undoc_fn + } + } + } + differs.sort_with_compare(sort_undoc_fns) + return differs +} + +fn sort_undoc_fns(a &UndocumentedFN, b &UndocumentedFN) int { + if a.file < b.file { + return -1 + } + if a.file > b.file { + return 1 + } + // same file sort by signature + else { + if a.signature < b.signature { + return -1 + } + if a.signature > b.signature { + return 1 + } + return 0 + } } fn normalise_path(path string) string { @@ -149,17 +226,15 @@ fn collect_tags(line string) []string { } fn main() { - if os.args.len == 1 { - println('Usage: $tool_name PATH \n$tool_description\n$tool_name -h for more help...') - exit(1) - } - mut fp := flag.new_flag_parser(os.args[1..]) + mut fp := flag.new_flag_parser(os.args[1..]) // skip the "v" command. fp.application(tool_name) fp.version(tool_version) fp.description(tool_description) fp.arguments_description('PATH [PATH]...') + fp.skip_executable() // skip the "missdoc" command. + // Collect tool options - opt := Options{ + mut opt := Options{ show_help: fp.bool('help', `h`, false, 'Show this help text.') deprecated: fp.bool('deprecated', `d`, false, 'Include deprecated functions in output.') private: fp.bool('private', `p`, false, 'Include private functions in output.') @@ -168,17 +243,54 @@ fn main() { collect_tags: fp.bool('tags', `t`, false, 'Also print function tags if any is found.') exclude: fp.string_multi('exclude', `e`, '') relative_paths: fp.bool('relative-paths', `r`, false, 'Use relative paths in output.') + diff: fp.bool('diff', 0, false, 'exit(1) and show difference between two PATH inputs, return 0 otherwise.') verify: fp.bool('verify', 0, false, 'exit(1) if documentation is missing, 0 otherwise.') } + + opt.additional_args = fp.finalize() or { panic(err) } + if opt.show_help { println(fp.usage()) exit(0) } + if opt.additional_args.len == 0 { + println(fp.usage()) + eprintln('Error: $tool_name is missing PATH input') + exit(1) + } + // Allow short-long versions to prevent false positive situations, should + // the user miss a `-`. E.g.: the `-verify` flag would be ignored and missdoc + // will return 0 for success plus a list of any undocumented functions. + if '-verify' in opt.additional_args { + opt.verify = true + } + if '-diff' in opt.additional_args { + opt.diff = true + } + if opt.diff { + if opt.additional_args.len < 2 { + println(fp.usage()) + eprintln('Error: $tool_name --diff needs two valid PATH inputs') + exit(1) + } + path_old := opt.additional_args[0] + path_new := opt.additional_args[1] + if !(os.is_file(path_old) || os.is_dir(path_old)) || !(os.is_file(path_new) + || os.is_dir(path_new)) { + println(fp.usage()) + eprintln('Error: $tool_name --diff needs two valid PATH inputs') + exit(1) + } + list := opt.diff_undocumented_functions_in_paths(path_old, path_new) + if list.len > 0 { + opt.report_undocumented_functions(list) + exit(1) + } + exit(0) + } mut total := 0 - for path in os.args[1..] { - if os.is_file(path) { - total += opt.report_undocumented_functions_in_file(path) - } else { + for path in opt.additional_args { + if os.is_file(path) || os.is_dir(path) { total += opt.report_undocumented_functions_in_path(path) } } diff --git a/cmd/v/help/missdoc.txt b/cmd/v/help/missdoc.txt index ef90ee899f..c64e6e07bb 100644 --- a/cmd/v/help/missdoc.txt +++ b/cmd/v/help/missdoc.txt @@ -1,4 +1,4 @@ -v missdoc 0.0.4 +v missdoc 0.1.0 ----------------------------------------------- Usage: v missdoc [options] PATH [PATH]... @@ -12,5 +12,25 @@ Options: --js Include JavaScript functions in output. -n, --no-line-numbers Exclude line numbers in output. -e, --exclude - + -r, --relative-paths Use relative paths in output. + --verify exit(1) if documentation is missing, 0 otherwise. + --diff exit(1) and show difference between two PATH inputs, return 0 otherwise. + --version output version information and exit + +----------------------------------------------- + +PATH can be both files and directories. + +The `--verify` flag is useful for use in CI setups for checking if a V project +has all it's functions and methods documented: +``` +v missdoc --verify path/to/code +``` + +The `--diff` flag is useful if your project is not yet fully documented +but you want to ensure that no new functions or methods are introduced +between commits or branches: +``` +v missdoc --diff current/code new/code +```