From 0f8edd918a83e40125921945d80dbd07e8762a61 Mon Sep 17 00:00:00 2001 From: Nick Treleaven Date: Wed, 17 Feb 2021 04:19:25 +0000 Subject: [PATCH] checker: disallow `unsafe` map copy (#8720) --- vlib/net/http/http.v | 9 ++++--- vlib/v/checker/checker.v | 4 ++-- .../checker/tests/array_or_map_assign_err.out | 4 ++-- vlib/v/gen/js/js.v | 24 +++++++++++-------- 4 files changed, 22 insertions(+), 19 deletions(-) diff --git a/vlib/net/http/http.v b/vlib/net/http/http.v index 2274bb94a4..c54731927e 100644 --- a/vlib/net/http/http.v +++ b/vlib/net/http/http.v @@ -163,13 +163,12 @@ fn fetch_with_method(method Method, url string, _config FetchConfig) ?Response { fn build_url_from_fetch(_url string, config FetchConfig) ?string { mut url := urllib.parse(_url) ? - params := unsafe {config.params} - if params.len == 0 { + if config.params.len == 0 { return url.str() } - mut pieces := []string{} - for key in params.keys() { - pieces << '$key=${params[key]}' + mut pieces := []string{cap: config.params.len} + for key, val in config.params { + pieces << '$key=$val' } mut query := pieces.join('&') if url.raw_query.len > 1 { diff --git a/vlib/v/checker/checker.v b/vlib/v/checker/checker.v index 22e778be9d..d862f7d79a 100644 --- a/vlib/v/checker/checker.v +++ b/vlib/v/checker/checker.v @@ -2726,11 +2726,11 @@ pub fn (mut c Checker) assign_stmt(mut assign_stmt ast.AssignStmt) { c.error('use `array2 $assign_stmt.op.str() array1.clone()` instead of `array2 $assign_stmt.op.str() array1` (or use `unsafe`)', assign_stmt.pos) } - if left_sym.kind == .map && !c.inside_unsafe && assign_stmt.op in [.assign, .decl_assign] + if left_sym.kind == .map && assign_stmt.op in [.assign, .decl_assign] && right_sym.kind == .map && !right_type.is_ptr() && !left.is_blank_ident() && right.is_lvalue() { // Do not allow `a = b` - c.error('cannot copy map: call `move` or `clone` method first (or use `unsafe`)', + c.error('cannot copy map: call `move` or `clone` method (or use a reference)', right.position()) } left_is_ptr := left_type.is_ptr() || left_sym.is_pointer() diff --git a/vlib/v/checker/tests/array_or_map_assign_err.out b/vlib/v/checker/tests/array_or_map_assign_err.out index b299e84c09..594732d635 100644 --- a/vlib/v/checker/tests/array_or_map_assign_err.out +++ b/vlib/v/checker/tests/array_or_map_assign_err.out @@ -12,14 +12,14 @@ vlib/v/checker/tests/array_or_map_assign_err.vv:5:5: error: use `array2 = array1 | ^ 6 | 7 | m1 := {'one': 1} -vlib/v/checker/tests/array_or_map_assign_err.vv:8:8: error: cannot copy map: call `move` or `clone` method first (or use `unsafe`) +vlib/v/checker/tests/array_or_map_assign_err.vv:8:8: error: cannot copy map: call `move` or `clone` method (or use a reference) 6 | 7 | m1 := {'one': 1} 8 | m2 := m1 | ~~ 9 | mut m3 := map[string]int{} 10 | m3 = m1 -vlib/v/checker/tests/array_or_map_assign_err.vv:10:7: error: cannot copy map: call `move` or `clone` method first (or use `unsafe`) +vlib/v/checker/tests/array_or_map_assign_err.vv:10:7: error: cannot copy map: call `move` or `clone` method (or use a reference) 8 | m2 := m1 9 | mut m3 := map[string]int{} 10 | m3 = m1 diff --git a/vlib/v/gen/js/js.v b/vlib/v/gen/js/js.v index e5fe87000c..c6f1a10049 100644 --- a/vlib/v/gen/js/js.v +++ b/vlib/v/gen/js/js.v @@ -115,16 +115,18 @@ pub fn gen(files []ast.File, table &table.Table, pref &pref.Preferences) string out += '/** @namespace $name */\n' } out += 'const $name = (function (' - imports := unsafe {g.namespaces[node.name].imports} - for i, key in imports.keys() { - if i > 0 { + mut namespace := g.namespaces[node.name] + mut first := true + for _, val in namespace.imports { + if !first { out += ', ' } - out += imports[key] + first = false + out += val } out += ') {\n\t' // private scope - out += g.namespaces[node.name].out.str().trim_space() + out += namespace.out.str().trim_space() // public scope out += '\n' if g.enable_doc { @@ -137,21 +139,23 @@ pub fn gen(files []ast.File, table &table.Table, pref &pref.Preferences) string out += '\n\t\t$typ,' } } - for i, pub_var in g.namespaces[node.name].pub_vars { + for i, pub_var in namespace.pub_vars { out += '\n\t\t$pub_var' - if i < g.namespaces[node.name].pub_vars.len - 1 { + if i < namespace.pub_vars.len - 1 { out += ',' } } - if g.namespaces[node.name].pub_vars.len > 0 { + if namespace.pub_vars.len > 0 { out += '\n\t' } out += '};' out += '\n})(' - for i, key in imports.keys() { - if i > 0 { + first = true + for key, _ in namespace.imports { + if !first { out += ', ' } + first = false out += key.replace('.', '_') } out += ');\n'