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 <[email protected]>
Co-authored-by: Yegappan Lakshmanan <[email protected]>
Co-authored-by: Ernie Rael <[email protected]>

Author: 2 people

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

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;

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);

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

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;

src/version.c

@@ -699,6 +699,8 @@ static char *(features[]) =
 
 static int included_patches[] =
 {   /* Add new patch number below this line */
+/**/
+    1829,
 /**/
     1828,
 /**/

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 @@ ex_class(exarg_T *eap)
  */
     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;
 	}
     }

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;