patch 9.0.1999: Vim9: some error messages can be improved

Problem:  Vim9: some error messages can be improved
Solution: Mention the defining class for variable access error message

closes: #13272

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

diff --git a/src/eval.c b/src/eval.c
index a9f7112f2deaa7375287b24f29638e898690e830..d9fbec234cba753a4cc697175466094e269e4467 100644 (file)
--- a/src/eval.c
+++ b/src/eval.c
@@ -1105,26 +1105,28 @@ get_lval_check_access(
 #endif
     if (cl_exec == NULL || cl_exec != cl)
     {
+       char *msg = NULL;
        switch (om->ocm_access)
        {
            case VIM_ACCESS_PRIVATE:
-               semsg(_(e_cannot_access_private_variable_str),
-                                               om->ocm_name, cl->class_name);
-               return FAIL;
+               msg = e_cannot_access_private_variable_str;
+               break;
            case VIM_ACCESS_READ:
                // If [idx] or .key following, read only OK.
                if (*p == '[' || *p == '.')
                    break;
                if ((flags & GLV_READ_ONLY) == 0)
-               {
-                   semsg(_(e_variable_is_not_writable_str),
-                                               om->ocm_name, cl->class_name);
-                   return FAIL;
-               }
+                   msg = e_variable_is_not_writable_str;
                break;
            case VIM_ACCESS_ALL:
                break;
        }
+       if (msg != NULL)
+       {
+           emsg_var_cl_define(msg, om->ocm_name, 0, cl);
+           return FAIL;
+       }
+
     }
     return OK;
 }

diff --git a/src/proto/vim9class.pro b/src/proto/vim9class.pro
index f2e642ec70b3dfd6ba35ce4ae2ff28880e54e598..e36c7e288ec1f46f61e40ba479d8235ed98c8a67 100644 (file)
--- a/src/proto/vim9class.pro
+++ b/src/proto/vim9class.pro
@@ -16,6 +16,7 @@ ocmember_T *object_member_lookup(class_T *cl, char_u *name, size_t namelen, int
 int object_method_idx(class_T *cl, char_u *name, size_t namelen);
 ufunc_T *object_method_lookup(class_T *cl, char_u *name, size_t namelen, int *idx);
 ocmember_T *member_lookup(class_T *cl, vartype_T v_type, char_u *name, size_t namelen, int *idx);
+void emsg_var_cl_define(char *msg, char_u *name, size_t len, class_T *cl);
 ufunc_T *method_lookup(class_T *cl, vartype_T v_type, char_u *name, size_t namelen, int *idx);
 int inside_class(cctx_T *cctx_arg, class_T *cl);
 void copy_object(typval_T *from, typval_T *to);

diff --git a/src/testdir/test_vim9_class.vim b/src/testdir/test_vim9_class.vim
index dc539aca4990c521fc7a7a5e486c8ceec8bdfeab..0c8fd7057a2712ed90a7f686b57b340611236b61 100644 (file)
--- a/src/testdir/test_vim9_class.vim
+++ b/src/testdir/test_vim9_class.vim
@@ -1725,6 +1725,119 @@ def Test_class_member()
   v9.CheckSourceFailure(lines, 'E1326: Variable not found on object "A": bar', 5)
 enddef
 
+" These messages should show the defining class of the variable (base class),
+" not the class that did the reference (super class)
+def Test_defining_class_message()
+  var lines =<< trim END
+    vim9script
+
+    class Base
+      this._v1: list<list<number>>
+    endclass
+
+    class Child extends Base
+    endclass
+
+    var o = Child.new()
+    var x = o._v1
+  END
+  v9.CheckSourceFailure(lines, 'E1333: Cannot access private variable "_v1" in class "Base"', 11)
+  lines =<< trim END
+    vim9script
+
+    class Base
+      this._v1: list<list<number>>
+    endclass
+
+    class Child extends Base
+    endclass
+
+    def F()
+      var o = Child.new()
+      var x = o._v1
+    enddef
+    F()
+  END
+  v9.CheckSourceFailure(lines, 'E1333: Cannot access private variable "_v1" in class "Base"', 2)
+  lines =<< trim END
+    vim9script
+
+    class Base
+      this.v1: list<list<number>>
+    endclass
+
+    class Child extends Base
+    endclass
+
+    var o = Child.new()
+    o.v1 = []
+  END
+  v9.CheckSourceFailure(lines, 'E1335: Variable "v1" in class "Base" is not writable', 11)
+  lines =<< trim END
+    vim9script
+
+    class Base
+      this.v1: list<list<number>>
+    endclass
+
+    class Child extends Base
+    endclass
+
+    def F()
+      var o = Child.new()
+      o.v1 = []
+    enddef
+    F()
+  END
+
+  # Attempt to read a private variable that is in the middle
+  # of the class hierarchy.
+  v9.CheckSourceFailure(lines, 'E1335: Variable "v1" in class "Base" is not writable', 2)
+  lines =<< trim END
+    vim9script
+
+    class Base0
+    endclass
+
+    class Base extends Base0
+      this._v1: list<list<number>>
+    endclass
+
+    class Child extends Base
+    endclass
+
+    def F()
+      var o = Child.new()
+      var x = o._v1
+    enddef
+    F()
+  END
+  v9.CheckSourceFailure(lines, 'E1333: Cannot access private variable "_v1" in class "Base"', 2)
+
+  # Attempt to read a private variable that is at the start
+  # of the class hierarchy.
+  lines =<< trim END
+    vim9script
+
+    class Base0
+    endclass
+
+    class Base extends Base0
+    endclass
+
+    class Child extends Base
+      this._v1: list<list<number>>
+    endclass
+
+    def F()
+      var o = Child.new()
+      var x = o._v1
+    enddef
+    F()
+  END
+  v9.CheckSourceFailure(lines, 'E1333: Cannot access private variable "_v1" in class "Child"', 2)
+enddef
+
 func Test_class_garbagecollect()
   let lines =<< trim END
     vim9script

diff --git a/src/version.c b/src/version.c
index 5335352b0330f0b609f1ee3967c438d27296d273..8dea204b2b340f245cfc29d386ceed52d5142b15 100644 (file)
--- a/src/version.c
+++ b/src/version.c
@@ -704,6 +704,8 @@ static char *(features[]) =
 
 static int included_patches[] =
 {   /* Add new patch number below this line */
+/**/
+    1999,
 /**/
     1998,
 /**/

diff --git a/src/vim9class.c b/src/vim9class.c
index 295716e18e81416cd5bb9a6333e0ddf2e68e8b30..7d1a7fdcaadddb119efdb61700aaefe9787dace1 100644 (file)
--- a/src/vim9class.c
+++ b/src/vim9class.c
@@ -2167,8 +2167,8 @@ get_member_tv(
 
     if (*name == '_')
     {
-       semsg(_(e_cannot_access_private_variable_str), m->ocm_name,
-                                                           cl->class_name);
+       emsg_var_cl_define(e_cannot_access_private_variable_str,
+                                                       m->ocm_name, 0, cl);
        return FAIL;
     }
 
@@ -2329,8 +2329,8 @@ class_object_index(
 
        if (*name == '_')
        {
-           semsg(_(e_cannot_access_private_variable_str), m->ocm_name,
-                                                           cl->class_name);
+           emsg_var_cl_define(e_cannot_access_private_variable_str,
+                                                       m->ocm_name, 0, cl);
            return FAIL;
        }
 
@@ -2580,6 +2580,57 @@ member_lookup(
        return object_member_lookup(cl, name, namelen, idx);
 }
 
+/*
+ * Find the class that defines the named member. Look up the hierarchy
+ * starting at "cl".
+ *
+ * Return the class that defines the member "name", else NULL.
+ * Fill in "p_m", if specified, for ocmember_T in found class.
+ */
+// NOTE: if useful for something could also indirectly return vartype and idx.
+    static class_T *
+class_defining_member(class_T *cl, char_u *name, size_t len, ocmember_T **p_m)
+{
+    class_T    *cl_found = NULL;
+    vartype_T  vartype = VAR_UNKNOWN;
+    ocmember_T *m_found = NULL;
+
+    len = len != 0 ? len : STRLEN(name);
+
+    // Loop assumes if member is not defined in "cl", then it is not
+    // defined in any super class; the last class where it's found is the
+    // class where it is defined. Once the vartype is found, the other
+    // type is no longer checked.
+    for (class_T *super = cl; super != NULL; super = super->class_extends)
+    {
+       class_T         *cl_tmp = NULL;
+       ocmember_T      *m = NULL;
+       if (vartype == VAR_UNKNOWN || vartype == VAR_OBJECT)
+       {
+           if ((m = object_member_lookup(super, name, len, NULL)) != NULL)
+           {
+               cl_tmp = super;
+               vartype = VAR_OBJECT;
+           }
+       }
+       if (vartype == VAR_UNKNOWN || vartype == VAR_CLASS)
+       {
+           if (( m = class_member_lookup(super, name, len, NULL)) != NULL)
+           {
+               cl_tmp = super;
+               vartype = VAR_OBJECT;
+           }
+       }
+       if (cl_tmp == NULL)
+           break;  // member is not in this or any super class.
+       cl_found = cl_tmp;
+       m_found = m;
+    }
+    if (p_m != NULL)
+       *p_m = m_found;
+    return cl_found;
+}
+
 /*
  * Lookup a class or object method by name.  If v_type is VAR_CLASS, then
  * lookup a class method and if it is VAR_OBJECT, then lookup a object method.
@@ -2853,6 +2904,22 @@ object_free_nonref(int copyID)
     return did_free;
 }
 
+/*
+ * Output message which takes a variable name and the class that defines it.
+ * "cl" is that class where the name was found. Search "cl"'s hierarchy to
+ * find the defining class.
+ */
+    void
+emsg_var_cl_define(char *msg, char_u *name, size_t len, class_T *cl)
+{
+    ocmember_T *m;
+    class_T    *cl_def = class_defining_member(cl, name, len, &m);
+    if (cl_def != NULL)
+       semsg(_(msg), m->ocm_name, cl_def->class_name);
+    else
+       emsg(_(e_internal_error_please_report_a_bug));
+}
+
 /*
  * Echo a class or object method not found message.
  */

diff --git a/src/vim9compile.c b/src/vim9compile.c
index 136bea49d3cc878242876e248a458ca78783178d..828fe02e81247339bd9b3ae384cf7a44ec4d88d8 100644 (file)
--- a/src/vim9compile.c
+++ b/src/vim9compile.c
@@ -1616,7 +1616,7 @@ lhs_class_member_modifiable(lhs_T *lhs, char_u    *var_start, cctx_T *cctx)
        char *msg = (m->ocm_access == VIM_ACCESS_PRIVATE)
                                ? e_cannot_access_private_variable_str
                                : e_variable_is_not_writable_str;
-       semsg(_(msg), m->ocm_name, cl->class_name);
+       emsg_var_cl_define(msg, m->ocm_name, 0, cl);
        return FALSE;
     }
 

diff --git a/src/vim9execute.c b/src/vim9execute.c
index a899745207a7ed1ee822b1e8266015f0472ddb53..f237132a896d3543796a1fab4ad209670a9a3700 100644 (file)
--- a/src/vim9execute.c
+++ b/src/vim9execute.c
@@ -2180,8 +2180,8 @@ execute_storeindex(isn_T *iptr, ectx_T *ectx)
            {
                if (*member == '_')
                {
-                   semsg(_(e_cannot_access_private_variable_str),
-                                               m->ocm_name, cl->class_name);
+                   emsg_var_cl_define(e_cannot_access_private_variable_str,
+                                                       m->ocm_name, 0, cl);
                    status = FAIL;
                }
 

diff --git a/src/vim9expr.c b/src/vim9expr.c
index 05eb5241801a3eedccb5440909dbec7c17d9fa3f..c15021e82ac78039601ce31bef5c4cf376ff847e 100644 (file)
--- a/src/vim9expr.c
+++ b/src/vim9expr.c
@@ -407,8 +407,8 @@ compile_class_object_index(cctx_T *cctx, char_u **arg, type_T *type)
        {
            if (*name == '_' && !inside_class(cctx, cl))
            {
-               semsg(_(e_cannot_access_private_variable_str), m->ocm_name,
-                                                           cl->class_name);
+               emsg_var_cl_define(e_cannot_access_private_variable_str,
+                                                       m->ocm_name, 0, cl);
                return FAIL;
            }
 
@@ -443,8 +443,8 @@ compile_class_object_index(cctx_T *cctx, char_u **arg, type_T *type)
            // it is defined.
            if (*name == '_' && cctx->ctx_ufunc->uf_class != cl)
            {
-               semsg(_(e_cannot_access_private_variable_str), m->ocm_name,
-                                                           cl->class_name);
+               emsg_var_cl_define(e_cannot_access_private_variable_str,
+                                                       m->ocm_name, 0, cl);
                return FAIL;
            }