tools: implement v missdoc --diff oldv newv (#14537)

.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
+          ./v missdoc --diff $MOPTIONS pv/vlib vlib
-          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

cmd/tools/vcomplete.v

@@ -259,6 +259,8 @@ const (
 		'--relative-paths',
 		'-r',
 		'--js',
+		'--verify',
+		'--diff',
 	]
 	auto_complete_flags_self    = [
 		'-prod',

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 {
+	mut fp := flag.new_flag_parser(os.args[1..]) // skip the "v" command.
-		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..])
 	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..] {
+	for path in opt.additional_args {
-		if os.is_file(path) {
+		if os.is_file(path) || os.is_dir(path) {
-			total += opt.report_undocumented_functions_in_file(path)
-		} else {
 			total += opt.report_undocumented_functions_in_path(path)
 		}
 	}

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]...
 
@@ -14,3 +14,23 @@ Options:
   -e, --exclude <multiple strings>
 
   -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
+```