# HG changeset patch
# User Christian Brabandt <cb@256bit.org>
# Date 1696615206 -7200
# Node ID bff8ac203a22cbd62eb0cf73106253e6c47ead99
# Parent  5bb56da92a88206848cb22a3cf3cbf858f3975d1
patch 9.0.1999: Vim9: some error messages can be improved

Commit: https://github.com/vim/vim/commit/e6c9aa5e6a88d539a412a9b5526f41ea101aa185
Author: Ernie Rael <errael@raelity.com>
Date:   Fri Oct 6 19:55:52 2023 +0200

    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
--- 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
--- a/src/proto/vim9class.pro
+++ b/src/proto/vim9class.pro
@@ -16,6 +16,7 @@ ocmember_T *object_member_lookup(class_T
 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
--- 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
--- a/src/version.c
+++ b/src/version.c
@@ -705,6 +705,8 @@ static char *(features[]) =
 static int included_patches[] =
 {   /* Add new patch number below this line */
 /**/
+    1999,
+/**/
     1998,
 /**/
     1997,
diff --git a/src/vim9class.c b/src/vim9class.c
--- 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;
 	}
 
@@ -2581,6 +2581,57 @@ member_lookup(
 }
 
 /*
+ * 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.
  *
@@ -2854,6 +2905,22 @@ object_free_nonref(int copyID)
 }
 
 /*
+ * 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.
  */
     void
diff --git a/src/vim9compile.c b/src/vim9compile.c
--- a/src/vim9compile.c
+++ b/src/vim9compile.c
@@ -1616,7 +1616,7 @@ lhs_class_member_modifiable(lhs_T *lhs, 
 	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
--- a/src/vim9execute.c
+++ b/src/vim9execute.c
@@ -2180,8 +2180,8 @@ execute_storeindex(isn_T *iptr, ectx_T *
 	    {
 		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
--- a/src/vim9expr.c
+++ b/src/vim9expr.c
@@ -407,8 +407,8 @@ compile_class_object_index(cctx_T *cctx,
 	{
 	    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,
 	    // 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;
 	    }