os: simplify and unify os.join_path and os.join_path_single, and add more tests (#21494)

Author: spytheman

vlib/os/join_path_test.v

@@ -0,0 +1,70 @@
+import os
+
+fn test_join_path() {
+	assert os.join_path('', '', '') == ''
+	assert os.join_path('', '') == ''
+	assert os.join_path('') == ''
+	assert os.join_path('b', '', '') == 'b'
+	assert os.join_path('b', '') == 'b'
+	assert os.join_path('b') == 'b'
+	assert os.join_path('', '', './b') == 'b'
+	assert os.join_path('', '', '/b') == 'b'
+	assert os.join_path('', '', 'b') == 'b'
+	assert os.join_path('', './b') == 'b'
+	assert os.join_path('', '/b') == 'b'
+	assert os.join_path('', 'b') == 'b'
+	assert os.join_path('b', '') == 'b'
+	$if windows {
+		assert os.join_path('./b', '') == r'.\b'
+		assert os.join_path('/b', '') == r'\b'
+		assert os.join_path('./a', './b') == r'.\a\b'
+		assert os.join_path('/', 'test') == r'\test'
+		assert os.join_path('v', 'vlib', 'os') == r'v\vlib\os'
+		assert os.join_path('', 'f1', 'f2') == r'f1\f2'
+		assert os.join_path('v', '', 'dir') == r'v\dir'
+		assert os.join_path(r'foo\bar', r'.\file.txt') == r'foo\bar\file.txt'
+		assert os.join_path('foo/bar', './file.txt') == r'foo\bar\file.txt'
+		assert os.join_path('/opt/v', './x') == r'\opt\v\x'
+		assert os.join_path('v', 'foo/bar', 'dir') == r'v\foo\bar\dir'
+		assert os.join_path('v', 'foo/bar\\baz', '/dir') == r'v\foo\bar\baz\dir'
+		assert os.join_path('C:', 'f1\\..', 'f2') == r'C:\f1\..\f2'
+	} $else {
+		assert os.join_path('./b', '') == './b'
+		assert os.join_path('/b', '') == '/b'
+		assert os.join_path('./a', './b') == './a/b'
+		assert os.join_path('/', 'test') == '/test'
+		assert os.join_path('v', 'vlib', 'os') == 'v/vlib/os'
+		assert os.join_path('', 'f1', 'f2') == 'f1/f2'
+		assert os.join_path('v', '', 'dir') == 'v/dir'
+		assert os.join_path('/foo/bar', './file.txt') == '/foo/bar/file.txt'
+		assert os.join_path('foo/bar', './file.txt') == 'foo/bar/file.txt'
+		assert os.join_path('/opt/v', './x') == '/opt/v/x'
+		assert os.join_path('/foo/bar', './.././file.txt') == '/foo/bar/../file.txt'
+		assert os.join_path('v', 'foo/bar\\baz', '/dir') == r'v/foo/bar/baz/dir'
+	}
+}
+
+fn test_join_path_single() {
+	assert os.join_path_single('', '') == ''
+	assert os.join_path_single('', './b') == 'b'
+	assert os.join_path_single('', '/b') == 'b'
+	assert os.join_path_single('', 'b') == 'b'
+	assert os.join_path_single('b', '') == 'b'
+	$if windows {
+		assert os.join_path_single('./b', '') == r'.\b'
+		assert os.join_path_single('/b', '') == r'\b'
+		assert os.join_path_single('/foo/bar', './file.txt') == r'\foo\bar\file.txt'
+		assert os.join_path_single('/', 'test') == r'\test'
+		assert os.join_path_single('foo\\bar', '.\\file.txt') == r'foo\bar\file.txt'
+		assert os.join_path_single('/opt/v', './x') == r'\opt\v\x'
+	} $else {
+		assert os.join_path_single('./b', '') == r'./b'
+		assert os.join_path_single('/b', '') == r'/b'
+		assert os.join_path_single('/foo/bar', './file.txt') == '/foo/bar/file.txt'
+		assert os.join_path_single('/', 'test') == '/test'
+		assert os.join_path_single('foo/bar', './file.txt') == 'foo/bar/file.txt'
+		assert os.join_path_single('/opt/v', './x') == '/opt/v/x'
+		assert os.join_path_single('./a', './b') == './a/b'
+		assert os.join_path_single('a', './b') == 'a/b'
+	}
+}

vlib/os/os.v

@@ -578,36 +578,22 @@ pub fn join_path(base string, dirs ...string) string {
 	defer {
 		unsafe { sb.free() }
 	}
-	mut needs_sep := false
-	if base != '' {
-		$if windows {
-			sb.write_string(base.replace('/', '\\'))
-		} $else {
-			sb.write_string(base)
-		}
-		needs_sep = !base.ends_with(path_separator)
-	}
-	for od in dirs {
-		if od != '' && od != '.' {
-			mut md := od
-			$if windows {
-				md = md.replace('/', '\\')
-			}
-			// NOTE(hholst80): split_any not available in js backend,
-			// which could have been more clean way to implement this.
-			nestdirs := md.split(path_separator)
-			for id in nestdirs {
-				if id != '' && id != '.' {
-					if needs_sep {
-						sb.write_string(path_separator)
-					}
-					sb.write_string(id)
-					needs_sep = !id.ends_with(path_separator)
-				}
-			}
+	sbase := base.trim_right('\\/')
+	defer {
+		unsafe { sbase.free() }
+	}
+	sb.write_string(sbase)
+	for d in dirs {
+		if d != '' {
+			sb.write_string(path_separator)
+			sb.write_string(d)
 		}
 	}
-	res := sb.str()
+	normalize_path_in_builder(mut sb)
+	mut res := sb.str()
+	if base == '' {
+		res = res.trim_left(path_separator)
+	}
 	return res
 }
 
@@ -625,12 +611,54 @@ pub fn join_path_single(base string, elem string) string {
 	defer {
 		unsafe { sbase.free() }
 	}
-	if base != '' {
-		sb.write_string(sbase)
+	sb.write_string(sbase)
+	if elem != '' {
 		sb.write_string(path_separator)
+		sb.write_string(elem)
+	}
+	normalize_path_in_builder(mut sb)
+	mut res := sb.str()
+	if base == '' {
+		res = res.trim_left(path_separator)
+	}
+	return res
+}
+
+@[direct_array_access]
+fn normalize_path_in_builder(mut sb strings.Builder) {
+	mut fs := `\\`
+	mut rs := `/`
+	$if windows {
+		fs = `/`
+		rs = `\\`
+	}
+	for idx in 0 .. sb.len {
+		unsafe {
+			if sb[idx] == fs {
+				sb[idx] = rs
+			}
+		}
+	}
+	for idx in 0 .. sb.len - 3 {
+		if sb[idx] == rs && sb[idx + 1] == `.` && sb[idx + 2] == rs {
+			unsafe {
+				// let `/foo/./bar.txt` become `/foo/bar.txt` in place
+				for j := idx + 1; j < sb.len - 2; j++ {
+					sb[j] = sb[j + 2]
+				}
+				sb.len -= 2
+			}
+		}
+		if sb[idx] == rs && sb[idx + 1] == rs {
+			unsafe {
+				// let `/foo//bar.txt` become `/foo/bar.txt` in place
+				for j := idx + 1; j < sb.len - 1; j++ {
+					sb[j] = sb[j + 1]
+				}
+				sb.len -= 1
+			}
+		}
 	}
-	sb.write_string(elem)
-	return sb.str()
 }
 
 // walk_ext returns a recursive list of all files in `path` ending with `ext`.

vlib/os/os_test.c.v

@@ -614,24 +614,6 @@ fn test_file_ext() {
 	assert os.file_ext('\\.git\\') == ''
 }
 
-fn test_join() {
-	$if windows {
-		assert os.join_path('v', 'vlib', 'os') == 'v\\vlib\\os'
-		assert os.join_path('', 'f1', 'f2') == 'f1\\f2'
-		assert os.join_path('v', '', 'dir') == 'v\\dir'
-		assert os.join_path('v', 'foo/bar', 'dir') == 'v\\foo\\bar\\dir'
-		assert os.join_path('v', 'foo/bar\\baz', '/dir') == 'v\\foo\\bar\\baz\\dir'
-		assert os.join_path('C:', 'f1\\..', 'f2') == 'C:\\f1\\..\\f2'
-	} $else {
-		assert os.join_path('v', 'vlib', 'os') == 'v/vlib/os'
-		assert os.join_path('/foo/bar', './file.txt') == '/foo/bar/file.txt'
-		assert os.join_path('', 'f1', 'f2') == 'f1/f2'
-		assert os.join_path('v', '', 'dir') == 'v/dir'
-		assert os.join_path('/', 'test') == '/test'
-		assert os.join_path('/foo/bar', './.././file.txt') == '/foo/bar/../file.txt'
-	}
-}
-
 fn test_rmdir_all() {
 	mut dirs := ['some/dir', 'some/.hidden/directory']
 	$if windows {

vlib/v/parser/parser.v

@@ -3632,6 +3632,10 @@ fn (mut p Parser) import_stmt() ast.Import {
 		return import_node
 	}
 	mut source_name := p.check_name()
+	if source_name == '' {
+		p.error_with_pos('import name can not be empty', pos)
+		return import_node
+	}
 	mut mod_name_arr := []string{}
 	mod_name_arr << source_name
 	if import_pos.line_nr != pos.line_nr {