patch 9.0.1829: Vim9 missing access-checks for private vars

Problem:  Vim9 missing access-checks for private vars
Solution: Use the proper check for private/readonly variable.  Access
          level for a member cannot be changed in a class implementing an
          interface.  Update the code indentation

closes: #12978

Signed-off-by: Christian Brabandt <cb@256bit.org>
Co-authored-by: Yegappan Lakshmanan <yegappan@yahoo.com>
Co-authored-by: Ernie Rael <errael@raelity.com>

diff --git a/src/errors.h b/src/errors.h
index 24977c4ba73739ed39dd92ac27151a20430dce20..1662043998aa0c0d0ded3911ea05c4652ebaa5df 100644 (file)
--- a/src/errors.h
+++ b/src/errors.h
@@ -3486,6 +3486,8 @@ EXTERN char e_cannot_use_a_return_type_with_new[]
        INIT(= N_("E1365: Cannot use a return type with the \"new\" function"));
 EXTERN char e_cannot_access_private_method_str[]
        INIT(= N_("E1366: Cannot access private method: %s"));
+EXTERN char e_member_str_of_interface_str_has_different_access[]
+       INIT(= N_("E1367: Access level of member \"%s\" of interface \"%s\" is different"));
 
 EXTERN char e_static_cannot_be_followed_by_this[]
        INIT(= N_("E1368: Static cannot be followed by \"this\" in a member name"));
@@ -3510,4 +3512,4 @@ EXTERN char e_member_str_type_mismatch_expected_str_but_got_str[]
 EXTERN char e_method_str_type_mismatch_expected_str_but_got_str[]
        INIT(= N_("E1407: Member \"%s\": type mismatch, expected %s but got %s"));
 
-// E1367, E1371 - E1399 unused
+// E1371 - E1399 unused

diff --git a/src/option.c b/src/option.c
index 095653da75a8d0d7609ddcb232022c09ebcdbf84..9f20bb244bef674c3f629c679d2671f595b9a32d 100644 (file)
--- a/src/option.c
+++ b/src/option.c
@@ -2982,7 +2982,8 @@ insecure_flag(int opt_idx, int opt_flags)
 /*
  * Redraw the window title and/or tab page text later.
  */
-void redraw_titles(void)
+    void
+redraw_titles(void)
 {
     need_maketitle = TRUE;
     redraw_tabline = TRUE;

diff --git a/src/proto/vim9class.pro b/src/proto/vim9class.pro
index ffe1c6444ce3a45447c2345925d5e86674cc10be..df0ed35f32b84927e10762e87eceb76cc7f5215d 100644 (file)
--- a/src/proto/vim9class.pro
+++ b/src/proto/vim9class.pro
@@ -1,7 +1,7 @@
 /* vim9class.c */
 int object_index_from_itf_index(class_T *itf, int is_method, int idx, class_T *cl);
 void ex_class(exarg_T *eap);
-type_T *class_member_type(class_T *cl, char_u *name, char_u *name_end, int *member_idx);
+type_T *class_member_type(class_T *cl, char_u *name, char_u *name_end, int *member_idx, omacc_T *access);
 void ex_enum(exarg_T *eap);
 void ex_type(exarg_T *eap);
 int class_object_index(char_u **arg, typval_T *rettv, evalarg_T *evalarg, int verbose);

diff --git a/src/testdir/test_vim9_class.vim b/src/testdir/test_vim9_class.vim
index ad0be5b5114490ad578ff5f19c4dcf72253c5fbd..98db71d03ff0e0d78203df18ee4bfe525c76cf97 100644 (file)
--- a/src/testdir/test_vim9_class.vim
+++ b/src/testdir/test_vim9_class.vim
@@ -490,7 +490,7 @@ def Test_assignment_with_operator()
       vim9script
 
       class Foo
-        this.x: number
+        public this.x: number
 
         def Add(n: number)
           this.x += n
@@ -2593,15 +2593,15 @@ def Test_multi_level_member_access()
     vim9script
 
     class A
-      this.val1: number = 0
+      public this.val1: number = 0
     endclass
 
     class B extends A
-      this.val2: number = 0
+      public this.val2: number = 0
     endclass
 
     class C extends B
-      this.val3: number = 0
+      public this.val3: number = 0
     endclass
 
     def A_members(a: A)
@@ -3672,18 +3672,60 @@ def Test_private_member_access_outside_class()
   END
   v9.CheckScriptFailure(lines, 'E1333: Cannot access private member: _val')
 
-  # private class member variable
+  # access a non-existing private object member variable
   lines =<< trim END
     vim9script
     class A
-      static _val: number = 10
+      this._val = 10
     endclass
     def T()
-      A._val = 20
+      var a = A.new()
+      a._a = 1
     enddef
     T()
   END
-  v9.CheckScriptFailure(lines, 'E1333: Cannot access private member: _val')
+  v9.CheckScriptFailure(lines, 'E1089: Unknown variable: _a = 1')
+enddef
+
+" Test for changing the member access of an interface in a implementation class
+def Test_change_interface_member_access()
+  var lines =<< trim END
+    vim9script
+    interface A
+      public this.val: number
+    endinterface
+    class B implements A
+      this.val = 10
+    endclass
+  END
+  v9.CheckScriptFailure(lines, 'E1367: Access level of member "val" of interface "A" is different')
+
+  lines =<< trim END
+    vim9script
+    interface A
+      this.val: number
+    endinterface
+    class B implements A
+      public this.val = 10
+    endclass
+  END
+  v9.CheckScriptFailure(lines, 'E1367: Access level of member "val" of interface "A" is different')
+enddef
+
+" Test for trying to change a readonly member from a def function
+def Test_readonly_member_change_in_def_func()
+  var lines =<< trim END
+    vim9script
+    class A
+      this.val: number
+    endclass
+    def T()
+      var a = A.new()
+      a.val = 20
+    enddef
+    T()
+  END
+  v9.CheckScriptFailure(lines, 'E46: Cannot change read-only variable "val"')
 enddef
 
 " vim: ts=8 sw=2 sts=2 expandtab tw=80 fdm=marker

diff --git a/src/typval.c b/src/typval.c
index c159cc99ab113e5da8492bfe52766bbeb9185c2d..f9f6bd691622320173e2a2627698b76000817cfa 100644 (file)
--- a/src/typval.c
+++ b/src/typval.c
@@ -539,7 +539,7 @@ check_for_list_arg(typval_T *args, int idx)
 {
     if (args[idx].v_type != VAR_LIST)
     {
-           semsg(_(e_list_required_for_argument_nr), idx + 1);
+       semsg(_(e_list_required_for_argument_nr), idx + 1);
        return FAIL;
     }
     return OK;
@@ -981,7 +981,7 @@ check_for_object_arg(typval_T *args, int idx)
 {
     if (args[idx].v_type != VAR_OBJECT)
     {
-           semsg(_(e_object_required_for_argument_nr), idx + 1);
+       semsg(_(e_object_required_for_argument_nr), idx + 1);
        return FAIL;
     }
     return OK;
@@ -995,7 +995,7 @@ check_for_class_or_list_arg(typval_T *args, int idx)
 {
     if (args[idx].v_type != VAR_CLASS && args[idx].v_type != VAR_LIST)
     {
-           semsg(_(e_list_or_class_required_for_argument_nr), idx + 1);
+       semsg(_(e_list_or_class_required_for_argument_nr), idx + 1);
        return FAIL;
     }
     return OK;

diff --git a/src/version.c b/src/version.c
index 1ab29e80012817700a85e30ebc785c4fd2bf6703..634d6552956d8f86874de32580d63961586e2070 100644 (file)
--- a/src/version.c
+++ b/src/version.c
@@ -699,6 +699,8 @@ static char *(features[]) =
 
 static int included_patches[] =
 {   /* Add new patch number below this line */
+/**/
+    1829,
 /**/
     1828,
 /**/

diff --git a/src/vim9class.c b/src/vim9class.c
index 4c0c13fd383e6962aa6d3d7c53002a9cae96aef8..79780c0c8d5c9a7b00338fb25b0f8cd2b2ecde36 100644 (file)
--- a/src/vim9class.c
+++ b/src/vim9class.c
@@ -30,14 +30,14 @@
  */
     static int
 parse_member(
-       exarg_T *eap,
-       char_u  *line,
-       char_u  *varname,
-       int     has_public,         // TRUE if "public" seen before "varname"
-       char_u  **varname_end,
-       garray_T *type_list,
-       type_T  **type_ret,
-       char_u  **init_expr)
+    exarg_T    *eap,
+    char_u     *line,
+    char_u     *varname,
+    int        has_public,         // TRUE if "public" seen before "varname"
+    char_u     **varname_end,
+    garray_T *type_list,
+    type_T     **type_ret,
+    char_u     **init_expr)
 {
     *varname_end = to_name_end(varname, FALSE);
     if (*varname == '_' && has_public)
@@ -119,12 +119,12 @@ parse_member(
  */
     static int
 add_member(
-       garray_T    *gap,
-       char_u      *varname,
-       char_u      *varname_end,
-       int         has_public,
-       type_T      *type,
-       char_u      *init_expr)
+    garray_T    *gap,
+    char_u         *varname,
+    char_u         *varname_end,
+    int            has_public,
+    type_T         *type,
+    char_u         *init_expr)
 {
     if (ga_grow(gap, 1) == FAIL)
        return FAIL;
@@ -357,6 +357,13 @@ validate_interface_members(
                                                                where) == FAIL)
                    return FALSE;
 
+               if (if_ms[if_i].ocm_access != m->ocm_access)
+               {
+                   semsg(_(e_member_str_of_interface_str_has_different_access),
+                           if_ms[if_i].ocm_name, intf_class_name);
+                   return FALSE;
+               }
+
                break;
            }
            if (cl_i == cl_count)
@@ -622,11 +629,11 @@ is_valid_constructor(ufunc_T *uf, int is_abstract, int has_static)
  */
     static int
 update_member_method_lookup_table(
-       class_T         *ifcl,
-       class_T         *cl,
-       garray_T        *objmethods,
-       int             pobj_method_offset,
-       int             is_interface)
+    class_T            *ifcl,
+    class_T            *cl,
+    garray_T   *objmethods,
+    int                pobj_method_offset,
+    int                is_interface)
 {
     if (ifcl == NULL)
        return OK;
@@ -1550,10 +1557,11 @@ cleanup:
  */
     type_T *
 class_member_type(
-       class_T *cl,
-       char_u  *name,
-       char_u  *name_end,
-       int     *member_idx)
+    class_T    *cl,
+    char_u     *name,
+    char_u     *name_end,
+    int                *member_idx,
+    omacc_T    *access)
 {
     *member_idx = -1;  // not found (yet)
     size_t len = name_end - name;
@@ -1564,6 +1572,7 @@ class_member_type(
        if (STRNCMP(m->ocm_name, name, len) == 0 && m->ocm_name[len] == NUL)
        {
            *member_idx = i;
+           *access = m->ocm_access;
            return m->ocm_type;
        }
     }

diff --git a/src/vim9compile.c b/src/vim9compile.c
index 5c173c190dcf5ed5e205a4c8d0044cd5ef6c9292..0d2de9953bfe7f05314b3ed015bd482666cbd4a6 100644 (file)
--- a/src/vim9compile.c
+++ b/src/vim9compile.c
@@ -1865,20 +1865,30 @@ compile_lhs(
        else if (use_class)
        {
            // for an object or class member get the type of the member
-           class_T *cl = lhs->lhs_type->tt_class;
+           class_T     *cl = lhs->lhs_type->tt_class;
+           omacc_T     access;
+
+           lhs->lhs_member_type = class_member_type(cl, after + 1,
+                                       lhs->lhs_end, &lhs->lhs_member_idx,
+                                       &access);
+           if (lhs->lhs_member_idx < 0)
+               return FAIL;
+
            // If it is private member variable, then accessing it outside the
            // class is not allowed.
-           if (*(after + 1) == '_' && !inside_class(cctx, cl))
+           if ((access != VIM_ACCESS_ALL) && !inside_class(cctx, cl))
            {
-               char_u *m_name = vim_strnsave(after + 1, lhs->lhs_end - after);
-               semsg(_(e_cannot_access_private_member_str), m_name);
+               char_u  *m_name;
+               char    *msg;
+
+               m_name = vim_strnsave(after + 1, lhs->lhs_end - after - 1);
+               msg = (access == VIM_ACCESS_PRIVATE)
+                               ? e_cannot_access_private_member_str
+                               : e_cannot_change_readonly_variable_str;
+               semsg(_(msg), m_name);
                vim_free(m_name);
                return FAIL;
            }
-           lhs->lhs_member_type = class_member_type(cl, after + 1,
-                                          lhs->lhs_end, &lhs->lhs_member_idx);
-           if (lhs->lhs_member_idx < 0)
-               return FAIL;
        }
        else
        {
@@ -2086,9 +2096,11 @@ compile_load_lhs_with_index(lhs_T *lhs, char_u *var_start, cctx_T *cctx)
        if (dot == NULL)
           return FAIL;
 
-       class_T *cl = lhs->lhs_type->tt_class;
-       type_T *type = class_member_type(cl, dot + 1,
-                                          lhs->lhs_end, &lhs->lhs_member_idx);
+       class_T *cl = lhs->lhs_type->tt_class;
+       omacc_T access;
+       type_T  *type = class_member_type(cl, dot + 1,
+                                          lhs->lhs_end, &lhs->lhs_member_idx,
+                                          &access);
        if (lhs->lhs_member_idx < 0)
            return FAIL;