From c23c543b67f05b468d16580072d04db9e49aa849 Mon Sep 17 00:00:00 2001 From: Delyan Angelov Date: Mon, 13 May 2024 17:33:10 +0300 Subject: [PATCH] os: simplify and unify os.join_path and os.join_path_single, and add more tests (#21494) --- vlib/os/join_path_test.v | 70 ++++++++++++++++++++++++++++++ vlib/os/os.v | 92 ++++++++++++++++++++++++++-------------- vlib/os/os_test.c.v | 18 -------- vlib/v/parser/parser.v | 4 ++ 4 files changed, 134 insertions(+), 50 deletions(-) create mode 100644 vlib/os/join_path_test.v diff --git a/vlib/os/join_path_test.v b/vlib/os/join_path_test.v new file mode 100644 index 000000000..c612ba056 --- /dev/null +++ b/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' + } +} diff --git a/vlib/os/os.v b/vlib/os/os.v index 3bb16bdc8..631735683 100644 --- a/vlib/os/os.v +++ 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`. diff --git a/vlib/os/os_test.c.v b/vlib/os/os_test.c.v index 30091969d..9cf3f469a 100644 --- a/vlib/os/os_test.c.v +++ b/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 { diff --git a/vlib/v/parser/parser.v b/vlib/v/parser/parser.v index ce8912ef4..0fa5abe60 100644 --- a/vlib/v/parser/parser.v +++ b/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 { -- 2.39.5