From 410b57b2fa6412508c9f8977b8cc808a3a3bce11 Mon Sep 17 00:00:00 2001 From: Delyan Angelov Date: Thu, 26 May 2022 00:44:18 +0300 Subject: [PATCH] all: add support for struct field deprecation (#14527) --- doc/docs.md | 13 ++++++ vlib/v/ast/ast.v | 4 ++ vlib/v/checker/check_types.v | 4 +- vlib/v/checker/checker.v | 40 ++++++++++++++---- vlib/v/checker/containers.v | 3 +- vlib/v/checker/fn.v | 27 +++++++----- vlib/v/checker/return.v | 3 ++ vlib/v/checker/str.v | 2 +- vlib/v/checker/struct.v | 6 +-- vlib/v/checker/tests/field_deprecations.out | 42 +++++++++++++++++++ vlib/v/checker/tests/field_deprecations.vv | 36 ++++++++++++++++ .../fields.v | 25 +++++++++++ vlib/v/parser/struct.v | 22 ++++++++++ 13 files changed, 201 insertions(+), 26 deletions(-) create mode 100644 vlib/v/checker/tests/field_deprecations.out create mode 100644 vlib/v/checker/tests/field_deprecations.vv create mode 100644 vlib/v/checker/tests/module_with_structs_with_deprecated_fields/fields.v diff --git a/doc/docs.md b/doc/docs.md index e75c605098..6d9c347192 100644 --- a/doc/docs.md +++ b/doc/docs.md @@ -5905,6 +5905,19 @@ fn main() { } ``` +Struct field deprecations: +```v oksyntax +module abc + +// Note that only *direct* accesses to Xyz.d in *other modules*, will produce deprecation notices/warnings: +pub struct Xyz { +pub mut: + a int + d int [deprecated: 'use Xyz.a instead'; deprecated_after: '2999-03-01'] // produce a notice, the deprecation date is in the far future +} +``` + +Function/method deprecations: ```v // Calling this function will result in a deprecation warning [deprecated] diff --git a/vlib/v/ast/ast.v b/vlib/v/ast/ast.v index 1d95b45fc3..a45191efb0 100644 --- a/vlib/v/ast/ast.v +++ b/vlib/v/ast/ast.v @@ -304,6 +304,10 @@ pub: is_mut bool is_global bool is_volatile bool + // + is_deprecated bool + deprecation_msg string + deprecated_after string pub mut: default_expr Expr default_expr_typ Type diff --git a/vlib/v/checker/check_types.v b/vlib/v/checker/check_types.v index bae21577a4..248ac1f11e 100644 --- a/vlib/v/checker/check_types.v +++ b/vlib/v/checker/check_types.v @@ -614,7 +614,7 @@ pub fn (mut c Checker) infer_fn_generic_types(func ast.Fn, mut node ast.CallExpr sym := c.table.sym(node.receiver_type) match sym.info { ast.Struct, ast.Interface, ast.SumType { - if c.table.cur_fn.generic_names.len > 0 { // in generic fn + if !isnil(c.table.cur_fn) && c.table.cur_fn.generic_names.len > 0 { // in generic fn if gt_name in c.table.cur_fn.generic_names && c.table.cur_fn.generic_names.len == c.table.cur_concrete_types.len { idx := c.table.cur_fn.generic_names.index(gt_name) @@ -671,6 +671,7 @@ pub fn (mut c Checker) infer_fn_generic_types(func ast.Fn, mut node ast.CallExpr mut param_elem_sym := c.table.sym(param_elem_info.elem_type) for { if arg_elem_sym.kind == .array && param_elem_sym.kind == .array + && !isnil(c.table.cur_fn) && param_elem_sym.name !in c.table.cur_fn.generic_names { arg_elem_info = arg_elem_sym.info as ast.Array arg_elem_sym = c.table.sym(arg_elem_info.elem_type) @@ -690,6 +691,7 @@ pub fn (mut c Checker) infer_fn_generic_types(func ast.Fn, mut node ast.CallExpr mut param_elem_sym := c.table.sym(param_elem_info.elem_type) for { if arg_elem_sym.kind == .array_fixed && param_elem_sym.kind == .array_fixed + && !isnil(c.table.cur_fn) && param_elem_sym.name !in c.table.cur_fn.generic_names { arg_elem_info = arg_elem_sym.info as ast.ArrayFixed arg_elem_sym = c.table.sym(arg_elem_info.elem_type) diff --git a/vlib/v/checker/checker.v b/vlib/v/checker/checker.v index c965b14cf8..640ad200e5 100644 --- a/vlib/v/checker/checker.v +++ b/vlib/v/checker/checker.v @@ -934,8 +934,8 @@ pub fn (mut c Checker) check_expr_opt_call(expr ast.Expr, ret_type ast.Type) ast pub fn (mut c Checker) check_or_expr(node ast.OrExpr, ret_type ast.Type, expr_return_type ast.Type) { if node.kind == .propagate_option { - if !c.table.cur_fn.return_type.has_flag(.optional) && c.table.cur_fn.name != 'main.main' - && !c.inside_const { + if !isnil(c.table.cur_fn) && !c.table.cur_fn.return_type.has_flag(.optional) + && c.table.cur_fn.name != 'main.main' && !c.inside_const { c.error('to propagate the call, `$c.table.cur_fn.name` must return an optional type', node.pos) } @@ -951,8 +951,8 @@ pub fn (mut c Checker) check_or_expr(node ast.OrExpr, ret_type ast.Type, expr_re return } if node.kind == .propagate_result { - if !c.table.cur_fn.return_type.has_flag(.result) && c.table.cur_fn.name != 'main.main' - && !c.inside_const { + if !isnil(c.table.cur_fn) && !c.table.cur_fn.return_type.has_flag(.result) + && c.table.cur_fn.name != 'main.main' && !c.inside_const { c.error('to propagate the call, `$c.table.cur_fn.name` must return an result type', node.pos) } @@ -1071,7 +1071,8 @@ pub fn (mut c Checker) selector_expr(mut node ast.SelectorExpr) ast.Type { match mut node.expr { ast.Ident { name := node.expr.name - valid_generic := util.is_generic_type_name(name) && name in c.table.cur_fn.generic_names + valid_generic := util.is_generic_type_name(name) && !isnil(c.table.cur_fn) + && name in c.table.cur_fn.generic_names if valid_generic { name_type = ast.Type(c.table.find_type_idx(name)).set_flag(.generic) } @@ -1220,11 +1221,23 @@ pub fn (mut c Checker) selector_expr(mut node ast.SelectorExpr) ast.Type { // <<< if has_field { - if sym.mod != c.mod && !field.is_pub && sym.language != .c { + is_used_outside := sym.mod != c.mod + if is_used_outside && !field.is_pub && sym.language != .c { unwrapped_sym := c.table.sym(c.unwrap_generic(typ)) c.error('field `${unwrapped_sym.name}.$field_name` is not public', node.pos) } field_sym := c.table.sym(field.typ) + if field.is_deprecated && is_used_outside { + now := time.now() + mut after_time := now + if field.deprecated_after != '' { + after_time = time.parse_iso8601(field.deprecated_after) or { + c.error('invalid time format', field.pos) + now + } + } + c.deprecate('field', field_name, field.deprecation_msg, now, after_time, node.pos) + } if field_sym.kind in [.sum_type, .interface_] { if !prevent_sum_type_unwrapping_once { if scope_field := node.scope.find_struct_field(node.expr.str(), typ, field_name) { @@ -1452,7 +1465,7 @@ fn (mut c Checker) stmt(node_ ast.Stmt) { c.inside_const = false } ast.DeferStmt { - if node.idx_in_fn < 0 { + if node.idx_in_fn < 0 && !isnil(c.table.cur_fn) { node.idx_in_fn = c.table.cur_fn.defer_stmts.len c.table.cur_fn.defer_stmts << unsafe { &node } } @@ -1541,7 +1554,7 @@ fn (mut c Checker) stmt(node_ ast.Stmt) { c.warn('`goto` requires `unsafe` (consider using labelled break/continue)', node.pos) } - if node.name !in c.table.cur_fn.label_names { + if !isnil(c.table.cur_fn) && node.name !in c.table.cur_fn.label_names { c.error('unknown label `$node.name`', node.pos) } // TODO: check label doesn't bypass variable declarations @@ -1981,7 +1994,7 @@ fn (mut c Checker) stmts_ending_with_expression(stmts []ast.Stmt) { } pub fn (mut c Checker) unwrap_generic(typ ast.Type) ast.Type { - if typ.has_flag(.generic) { + if typ.has_flag(.generic) && !isnil(c.table.cur_fn) { if t_typ := c.table.resolve_generic_to_concrete(typ, c.table.cur_fn.generic_names, c.table.cur_concrete_types) { @@ -2531,9 +2544,15 @@ pub fn (mut c Checker) cast_expr(mut node ast.CastExpr) ast.Type { fn (mut c Checker) at_expr(mut node ast.AtExpr) ast.Type { match node.kind { .fn_name { + if isnil(c.table.cur_fn) { + return ast.void_type + } node.val = c.table.cur_fn.name.all_after_last('.') } .method_name { + if isnil(c.table.cur_fn) { + return ast.void_type + } fname := c.table.cur_fn.name.all_after_last('.') if c.table.cur_fn.is_method { node.val = c.table.type_to_str(c.table.cur_fn.receiver.typ).all_after_last('.') + @@ -2543,6 +2562,9 @@ fn (mut c Checker) at_expr(mut node ast.AtExpr) ast.Type { } } .mod_name { + if isnil(c.table.cur_fn) { + return ast.void_type + } node.val = c.table.cur_fn.mod } .struct_name { diff --git a/vlib/v/checker/containers.v b/vlib/v/checker/containers.v index 8a66b9c43c..647914c58f 100644 --- a/vlib/v/checker/containers.v +++ b/vlib/v/checker/containers.v @@ -52,7 +52,8 @@ pub fn (mut c Checker) array_init(mut node ast.ArrayInit) ast.Type { c.ensure_sumtype_array_has_default_value(node) } c.ensure_type_exists(node.elem_type, node.elem_type_pos) or {} - if node.typ.has_flag(.generic) && c.table.cur_fn.generic_names.len == 0 { + if node.typ.has_flag(.generic) && !isnil(c.table.cur_fn) + && c.table.cur_fn.generic_names.len == 0 { c.error('generic struct cannot use in non-generic function', node.pos) } return node.typ diff --git a/vlib/v/checker/fn.v b/vlib/v/checker/fn.v index df114c2d33..4e7d44115d 100644 --- a/vlib/v/checker/fn.v +++ b/vlib/v/checker/fn.v @@ -421,8 +421,8 @@ pub fn (mut c Checker) call_expr(mut node ast.CallExpr) ast.Type { c.expected_or_type = node.return_type.clear_flag(.optional) c.stmts_ending_with_expression(node.or_block.stmts) c.expected_or_type = ast.void_type - if node.or_block.kind == .propagate_option && !c.table.cur_fn.return_type.has_flag(.optional) - && !c.inside_const { + if node.or_block.kind == .propagate_option && !isnil(c.table.cur_fn) + && !c.table.cur_fn.return_type.has_flag(.optional) && !c.inside_const { if !c.table.cur_fn.is_main { c.error('to propagate the optional call, `$c.table.cur_fn.name` must return an optional', node.or_block.pos) @@ -482,7 +482,9 @@ pub fn (mut c Checker) fn_call(mut node ast.CallExpr, mut continue_check &bool) c.error('JS.await: first argument must be a promise, got `$tsym.name`', node.pos) return ast.void_type } - c.table.cur_fn.has_await = true + if !isnil(c.table.cur_fn) { + c.table.cur_fn.has_await = true + } match tsym.info { ast.Struct { mut ret_type := tsym.info.concrete_types[0] @@ -1026,14 +1028,15 @@ pub fn (mut c Checker) fn_call(mut node ast.CallExpr, mut continue_check &bool) } } // resolve return generics struct to concrete type - if func.generic_names.len > 0 && func.return_type.has_flag(.generic) + if func.generic_names.len > 0 && func.return_type.has_flag(.generic) && !isnil(c.table.cur_fn) && c.table.cur_fn.generic_names.len == 0 { node.return_type = c.table.unwrap_generic_type(func.return_type, func.generic_names, concrete_types) } else { node.return_type = func.return_type } - if node.concrete_types.len > 0 && func.return_type != 0 && c.table.cur_fn.generic_names.len == 0 { + if node.concrete_types.len > 0 && func.return_type != 0 && !isnil(c.table.cur_fn) + && c.table.cur_fn.generic_names.len == 0 { if typ := c.table.resolve_generic_to_concrete(func.return_type, func.generic_names, concrete_types) { @@ -1075,7 +1078,7 @@ pub fn (mut c Checker) method_call(mut node ast.CallExpr) ast.Type { node.return_type = left_type node.receiver_type = left_type - if c.table.cur_fn.generic_names.len > 0 { + if !isnil(c.table.cur_fn) && c.table.cur_fn.generic_names.len > 0 { c.table.unwrap_generic_type(left_type, c.table.cur_fn.generic_names, c.table.cur_concrete_types) } unwrapped_left_type := c.unwrap_generic(left_type) @@ -1155,7 +1158,9 @@ pub fn (mut c Checker) method_call(mut node ast.CallExpr) ast.Type { if node.args.len > 0 { c.error('wait() does not have any arguments', node.args[0].pos) } - c.table.cur_fn.has_await = true + if !isnil(c.table.cur_fn) { + c.table.cur_fn.has_await = true + } node.return_type = info.concrete_types[0] node.return_type.set_flag(.optional) return node.return_type @@ -1454,7 +1459,7 @@ pub fn (mut c Checker) method_call(mut node ast.CallExpr) ast.Type { c.warn('method `${left_sym.name}.$method_name` must be called from an `unsafe` block', node.pos) } - if !c.table.cur_fn.is_deprecated && method.is_deprecated { + if !isnil(c.table.cur_fn) && !c.table.cur_fn.is_deprecated && method.is_deprecated { c.deprecate_fnmethod('method', '${left_sym.name}.$method.name', method, node) } c.set_node_expected_arg_types(mut node, method) @@ -1478,13 +1483,13 @@ pub fn (mut c Checker) method_call(mut node ast.CallExpr) ast.Type { } // resolve return generics struct to concrete type if method.generic_names.len > 0 && method.return_type.has_flag(.generic) - && c.table.cur_fn.generic_names.len == 0 { + && !isnil(c.table.cur_fn) && c.table.cur_fn.generic_names.len == 0 { node.return_type = c.table.unwrap_generic_type(method.return_type, method.generic_names, concrete_types) } else { node.return_type = method.return_type } - if node.concrete_types.len > 0 && method.return_type != 0 + if node.concrete_types.len > 0 && method.return_type != 0 && !isnil(c.table.cur_fn) && c.table.cur_fn.generic_names.len == 0 { if typ := c.table.resolve_generic_to_concrete(method.return_type, method.generic_names, concrete_types) @@ -1615,7 +1620,7 @@ fn (mut c Checker) deprecate_fnmethod(kind string, name string, the_fn ast.Fn, n if attr.name == 'deprecated_after' && attr.arg != '' { after_time = time.parse_iso8601(attr.arg) or { c.error('invalid time format', attr.pos) - time.now() + now } } } diff --git a/vlib/v/checker/return.v b/vlib/v/checker/return.v index 35de9d77ca..ee67158eb1 100644 --- a/vlib/v/checker/return.v +++ b/vlib/v/checker/return.v @@ -7,6 +7,9 @@ import v.pref // TODO: non deferred pub fn (mut c Checker) return_stmt(mut node ast.Return) { + if isnil(c.table.cur_fn) { + return + } c.expected_type = c.table.cur_fn.return_type mut expected_type := c.unwrap_generic(c.expected_type) expected_type_sym := c.table.sym(expected_type) diff --git a/vlib/v/checker/str.v b/vlib/v/checker/str.v index fa1575e00b..8eea09bca1 100644 --- a/vlib/v/checker/str.v +++ b/vlib/v/checker/str.v @@ -97,7 +97,7 @@ pub fn (mut c Checker) string_inter_lit(mut node ast.StringInterLiteral) ast.Typ node.need_fmts[i] = fmt != c.get_default_fmt(ftyp, typ) } // check recursive str - if c.table.cur_fn.is_method && c.table.cur_fn.name == 'str' + if !isnil(c.table.cur_fn) && c.table.cur_fn.is_method && c.table.cur_fn.name == 'str' && c.table.cur_fn.receiver.name == expr.str() { c.error('cannot call `str()` method recursively', expr.pos()) } diff --git a/vlib/v/checker/struct.v b/vlib/v/checker/struct.v index 1419585215..c410bb04cb 100644 --- a/vlib/v/checker/struct.v +++ b/vlib/v/checker/struct.v @@ -219,7 +219,7 @@ pub fn (mut c Checker) struct_init(mut node ast.StructInit) ast.Type { && node.generic_types.len != struct_sym.info.generic_types.len { c.error('generic struct init expects $struct_sym.info.generic_types.len generic parameter, but got $node.generic_types.len', node.pos) - } else if node.generic_types.len > 0 { + } else if node.generic_types.len > 0 && !isnil(c.table.cur_fn) { for gtyp in node.generic_types { gtyp_name := c.table.sym(gtyp).name if gtyp_name !in c.table.cur_fn.generic_names { @@ -247,7 +247,7 @@ pub fn (mut c Checker) struct_init(mut node ast.StructInit) ast.Type { } } // register generic struct type when current fn is generic fn - if c.table.cur_fn.generic_names.len > 0 { + if !isnil(c.table.cur_fn) && c.table.cur_fn.generic_names.len > 0 { c.table.unwrap_generic_type(node.typ, c.table.cur_fn.generic_names, c.table.cur_concrete_types) } c.ensure_type_exists(node.typ, node.pos) or {} @@ -291,7 +291,7 @@ pub fn (mut c Checker) struct_init(mut node ast.StructInit) ast.Type { 'it cannot be initialized with `$type_sym.name{}`', node.pos) } } - if type_sym.name.len == 1 && c.table.cur_fn.generic_names.len == 0 { + if type_sym.name.len == 1 && !isnil(c.table.cur_fn) && c.table.cur_fn.generic_names.len == 0 { c.error('unknown struct `$type_sym.name`', node.pos) return 0 } diff --git a/vlib/v/checker/tests/field_deprecations.out b/vlib/v/checker/tests/field_deprecations.out new file mode 100644 index 0000000000..4ffa5003a0 --- /dev/null +++ b/vlib/v/checker/tests/field_deprecations.out @@ -0,0 +1,42 @@ +vlib/v/checker/tests/field_deprecations.vv:23:9: notice: field `d` will be deprecated after 2999-03-01, and will become an error after 2999-08-28; d use Xyz.a instead + 21 | dump(x.c) + 22 | x.c = 11 + 23 | dump(x.d) + | ^ + 24 | x.d = 45 + 25 | } +vlib/v/checker/tests/field_deprecations.vv:24:4: notice: field `d` will be deprecated after 2999-03-01, and will become an error after 2999-08-28; d use Xyz.a instead + 22 | x.c = 11 + 23 | dump(x.d) + 24 | x.d = 45 + | ^ + 25 | } + 26 | +vlib/v/checker/tests/field_deprecations.vv:19:9: warning: field `b` has been deprecated + 17 | dump(x.a) + 18 | x.a = 123 + 19 | dump(x.b) + | ^ + 20 | x.b = 456 + 21 | dump(x.c) +vlib/v/checker/tests/field_deprecations.vv:20:4: warning: field `b` has been deprecated + 18 | x.a = 123 + 19 | dump(x.b) + 20 | x.b = 456 + | ^ + 21 | dump(x.c) + 22 | x.c = 11 +vlib/v/checker/tests/field_deprecations.vv:21:9: error: field `c` has been deprecated since 2021-03-01; c use Xyz.a instead + 19 | dump(x.b) + 20 | x.b = 456 + 21 | dump(x.c) + | ^ + 22 | x.c = 11 + 23 | dump(x.d) +vlib/v/checker/tests/field_deprecations.vv:22:4: error: field `c` has been deprecated since 2021-03-01; c use Xyz.a instead + 20 | x.b = 456 + 21 | dump(x.c) + 22 | x.c = 11 + | ^ + 23 | dump(x.d) + 24 | x.d = 45 diff --git a/vlib/v/checker/tests/field_deprecations.vv b/vlib/v/checker/tests/field_deprecations.vv new file mode 100644 index 0000000000..d2775004d0 --- /dev/null +++ b/vlib/v/checker/tests/field_deprecations.vv @@ -0,0 +1,36 @@ +import v.checker.tests.module_with_structs_with_deprecated_fields as m + +struct Abc { +mut: + x int + d int [deprecated] + z int +} + +fn use_m_externally() { + x := m.Xyz{} + dump(x) +} + +fn use_m_externally_and_use_deprecated_fields() { + mut x := m.Xyz{} + dump(x.a) + x.a = 123 + dump(x.b) + x.b = 456 + dump(x.c) + x.c = 11 + dump(x.d) + x.d = 45 +} + +fn main() { + mut a := Abc{} + a.x = 1 + a.d = 1 + a.z = 1 + dump(a) + println(a.d) + x := a.d + 1 + dump(x) +} diff --git a/vlib/v/checker/tests/module_with_structs_with_deprecated_fields/fields.v b/vlib/v/checker/tests/module_with_structs_with_deprecated_fields/fields.v new file mode 100644 index 0000000000..8592c8b112 --- /dev/null +++ b/vlib/v/checker/tests/module_with_structs_with_deprecated_fields/fields.v @@ -0,0 +1,25 @@ +module module_with_structs_with_deprecated_fields + +pub struct Xyz { +pub mut: + a int + b int [deprecated] + c int [deprecated: 'c use Xyz.a instead'; deprecated_after: '2021-03-01'] + d int [deprecated: 'd use Xyz.a instead'; deprecated_after: '2999-03-01'] +} + +fn some_internal_function() { + mut x := Xyz{} // initialisation; no error + + // reads: + dump(x.a) // no error + dump(x.b) // no error internally + dump(x.c) // no error internally + dump(x.d) // no error internally + + // writes: + x.a = 1 // no error + x.b = 1 // no error internally + x.c = 1 // no error internally + x.d = 1 // no error internally +} diff --git a/vlib/v/parser/struct.v b/vlib/v/parser/struct.v index c36f59a585..c12a4437f1 100644 --- a/vlib/v/parser/struct.v +++ b/vlib/v/parser/struct.v @@ -179,6 +179,9 @@ fn (mut p Parser) struct_decl() ast.StructDecl { } field_start_pos := p.tok.pos() mut is_field_volatile := false + mut is_field_deprecated := false + mut field_deprecation_msg := '' + mut field_deprecated_after := '' if p.tok.kind == .key_volatile { p.next() is_field_volatile = true @@ -244,6 +247,19 @@ fn (mut p Parser) struct_decl() ast.StructDecl { if p.tok.kind == .lsbr { // attrs are stored in `p.attrs` p.attributes() + for fa in p.attrs { + match fa.name { + 'deprecated' { + // [deprecated: 'use a replacement'] + is_field_deprecated = true + field_deprecation_msg = fa.arg + } + 'deprecated_after' { + field_deprecated_after = fa.arg + } + else {} + } + } } mut default_expr := ast.empty_expr() mut has_default_expr := false @@ -274,6 +290,9 @@ fn (mut p Parser) struct_decl() ast.StructDecl { is_mut: is_embed || is_field_mut is_global: is_field_global is_volatile: is_field_volatile + is_deprecated: is_field_deprecated + deprecation_msg: field_deprecation_msg + deprecated_after: field_deprecated_after } } // save embeds as table fields too, it will be used in generation phase @@ -291,6 +310,9 @@ fn (mut p Parser) struct_decl() ast.StructDecl { is_mut: is_embed || is_field_mut is_global: is_field_global is_volatile: is_field_volatile + is_deprecated: is_field_deprecated + deprecation_msg: field_deprecation_msg + deprecated_after: field_deprecated_after } p.attrs = [] i++