patch 8.2.2331: Vim9: wrong error when modifying dict declared with :final

Problem:    Vim9: wrong error when modifying dict declared with :final.
Solution:   Do not check for writable variable when an index follows.
            (closes #7657)

src/evalvars.c

@@ -3153,7 +3153,7 @@ set_var_const(
 	    // A Vim9 script-local variable is also present in sn_all_vars and
 	    // sn_var_vals.  It may set "type" from "tv".
 	    if (is_script_local && vim9script)
-		update_vim9_script_var(FALSE, di, tv, &type);
+		update_vim9_script_var(FALSE, di, flags, tv, &type);
 	}
 
 	// existing variable, need to clear the value
@@ -3243,7 +3243,7 @@ set_var_const(
 	// A Vim9 script-local variable is also added to sn_all_vars and
 	// sn_var_vals. It may set "type" from "tv".
 	if (is_script_local && vim9script)
-	    update_vim9_script_var(TRUE, di, tv, &type);
+	    update_vim9_script_var(TRUE, di, flags, tv, &type);
     }
 
     if (copy || tv->v_type == VAR_NUMBER || tv->v_type == VAR_FLOAT)

src/proto/vim9script.pro

@@ -10,7 +10,7 @@ void ex_import(exarg_T *eap);
 int find_exported(int sid, char_u *name, ufunc_T **ufunc, type_T **type, cctx_T *cctx);
 char_u *handle_import(char_u *arg_start, garray_T *gap, int import_sid, evalarg_T *evalarg, void *cctx);
 char_u *vim9_declare_scriptvar(exarg_T *eap, char_u *arg);
-void update_vim9_script_var(int create, dictitem_T *di, typval_T *tv, type_T **type);
+void update_vim9_script_var(int create, dictitem_T *di, int flags, typval_T *tv, type_T **type);
 void hide_script_var(scriptitem_T *si, int idx, int func_defined);
 void free_all_script_vars(scriptitem_T *si);
 svar_T *find_typval_in_script(typval_T *dest);

src/structs.h

@@ -1777,7 +1777,7 @@ struct svar_S {
     char_u	*sv_name;	// points into "sn_all_vars" di_key
     typval_T	*sv_tv;		// points into "sn_vars" or "sn_all_vars" di_tv
     type_T	*sv_type;
-    int		sv_const;
+    int		sv_const;	// 0, ASSIGN_CONST or ASSIGN_FINAL
     int		sv_export;	// "export let var = val"
 };
 

src/testdir/test_vim9_assign.vim

@@ -1225,6 +1225,12 @@ def Test_var_declaration()
       g:dict_val = s:dict[key]
     enddef
     GetDictVal('a')
+
+    final adict: dict<string> = {}
+    def ChangeAdict()
+      adict.foo = 'foo'
+    enddef
+    ChangeAdict()
   END
   CheckScriptSuccess(lines)
   assert_equal('', g:var_uninit)
@@ -1260,6 +1266,16 @@ def Test_var_declaration_fails()
   CheckScriptFailure(lines, 'E741:')
   unlet g:constvar
 
+  lines =<< trim END
+    vim9script
+    const cdict: dict<string> = {}
+    def Change()
+      cdict.foo = 'foo'
+    enddef
+    defcompile
+  END
+  CheckScriptFailure(lines, 'E46:')
+
   lines =<< trim END
     vim9script
     final w:finalvar = [9]

src/version.c

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

src/vim9compile.c

@@ -2126,11 +2126,30 @@ free_locals(cctx_T *cctx)
     ga_clear(&cctx->ctx_locals);
 }
 
+/*
+ * If "check_writable" is ASSIGN_CONST give an error if the variable was
+ * defined with :final or :const, if "check_writable" is ASSIGN_FINAL give an
+ * error if the variable was defined with :const.
+ */
+    static int
+check_item_writable(svar_T *sv, int check_writable, char_u *name)
+{
+    if ((check_writable == ASSIGN_CONST && sv->sv_const != 0)
+	    || (check_writable == ASSIGN_FINAL
+					      && sv->sv_const == ASSIGN_CONST))
+    {
+	semsg(_(e_readonlyvar), name);
+	return FAIL;
+    }
+    return OK;
+}
+
 /*
  * Find "name" in script-local items of script "sid".
+ * Pass "check_writable" to check_item_writable().
  * Returns the index in "sn_var_vals" if found.
  * If found but not in "sn_var_vals" returns -1.
- * If not found returns -2.
+ * If not found or the variable is not writable returns -2.
  */
     int
 get_script_item_idx(int sid, char_u *name, int check_writable, cctx_T *cctx)
@@ -2151,8 +2170,8 @@ get_script_item_idx(int sid, char_u *name, int check_writable, cctx_T *cctx)
 	    return -2;
 	idx = sav->sav_var_vals_idx;
 	sv = ((svar_T *)si->sn_var_vals.ga_data) + idx;
-	if (check_writable && sv->sv_const)
+	if (check_item_writable(sv, check_writable, name) == FAIL)
-	    semsg(_(e_readonlyvar), name);
+	    return -2;
 	return idx;
     }
 
@@ -2168,8 +2187,8 @@ get_script_item_idx(int sid, char_u *name, int check_writable, cctx_T *cctx)
 	sv = ((svar_T *)si->sn_var_vals.ga_data) + idx;
 	if (sv->sv_tv == &di->di_tv)
 	{
-	    if (check_writable && sv->sv_const)
+	    if (check_item_writable(sv, check_writable, name) == FAIL)
-		semsg(_(e_readonlyvar), name);
+		return -2;
 	    return idx;
 	}
     }
@@ -2466,7 +2485,7 @@ compile_load_scriptvar(
     if (!SCRIPT_ID_VALID(current_sctx.sc_sid))
 	return FAIL;
     si = SCRIPT_ITEM(current_sctx.sc_sid);
-    idx = get_script_item_idx(current_sctx.sc_sid, name, FALSE, cctx);
+    idx = get_script_item_idx(current_sctx.sc_sid, name, 0, cctx);
     if (idx == -1 || si->sn_version != SCRIPT_VERSION_VIM9)
     {
 	// variable is not in sn_var_vals: old style script.
@@ -5475,6 +5494,11 @@ compile_lhs(
     lhs->lhs_name = vim_strnsave(var_start, lhs->lhs_varlen);
     if (lhs->lhs_name == NULL)
 	return FAIL;
+
+    if (lhs->lhs_dest_end > var_start + lhs->lhs_varlen)
+	// Something follows after the variable: "var[idx]" or "var.key".
+	lhs->lhs_has_index = TRUE;
+
     if (heredoc)
 	lhs->lhs_type = &t_list_string;
     else
@@ -5576,9 +5600,11 @@ compile_lhs(
 			lhs->lhs_scriptvar_sid = import->imp_sid;
 		    if (SCRIPT_ID_VALID(lhs->lhs_scriptvar_sid))
 		    {
+			// Check writable only when no index follows.
 			lhs->lhs_scriptvar_idx = get_script_item_idx(
-							lhs->lhs_scriptvar_sid,
+					       lhs->lhs_scriptvar_sid, rawname,
-							  rawname, TRUE, cctx);
+			      lhs->lhs_has_index ? ASSIGN_FINAL : ASSIGN_CONST,
+									 cctx);
 			if (lhs->lhs_scriptvar_idx >= 0)
 			{
 			    scriptitem_T *si = SCRIPT_ITEM(
@@ -5665,7 +5691,7 @@ compile_lhs(
     }
 
     lhs->lhs_member_type = lhs->lhs_type;
-    if (lhs->lhs_dest_end > var_start + lhs->lhs_varlen)
+    if (lhs->lhs_has_index)
     {
 	// Something follows after the variable: "var[idx]" or "var.key".
 	// TODO: should we also handle "->func()" here?
@@ -5700,7 +5726,6 @@ compile_lhs(
 		lhs->lhs_type = &t_any;
 	    }
 
-	    lhs->lhs_has_index = TRUE;
 	    if (lhs->lhs_type->tt_member == NULL)
 		lhs->lhs_member_type = &t_any;
 	    else

src/vim9script.c

@@ -257,7 +257,7 @@ find_exported(
 
     // find name in "script"
     // TODO: also find script-local user function
-    idx = get_script_item_idx(sid, name, FALSE, cctx);
+    idx = get_script_item_idx(sid, name, 0, cctx);
     if (idx >= 0)
     {
 	sv = ((svar_T *)script->sn_var_vals.ga_data) + idx;
@@ -661,10 +661,16 @@ vim9_declare_scriptvar(exarg_T *eap, char_u *arg)
  * with a hashtable) and sn_var_vals (lookup by index).
  * When "create" is TRUE this is a new variable, otherwise find and update an
  * existing variable.
+ * "flags" can have ASSIGN_FINAL or ASSIGN_CONST.
  * When "*type" is NULL use "tv" for the type and update "*type".
  */
     void
-update_vim9_script_var(int create, dictitem_T *di, typval_T *tv, type_T **type)
+update_vim9_script_var(
+	int	    create,
+	dictitem_T  *di,
+	int	    flags,
+	typval_T    *tv,
+	type_T	    **type)
 {
     scriptitem_T    *si = SCRIPT_ITEM(current_sctx.sc_sid);
     hashitem_T	    *hi;
@@ -686,7 +692,8 @@ update_vim9_script_var(int create, dictitem_T *di, typval_T *tv, type_T **type)
 	    return;
 
 	sv->sv_tv = &di->di_tv;
-	sv->sv_const = (di->di_flags & DI_FLAGS_LOCK) ? ASSIGN_CONST : 0;
+	sv->sv_const = (flags & ASSIGN_FINAL) ? ASSIGN_FINAL
+				   : (flags & ASSIGN_CONST) ? ASSIGN_CONST : 0;
 	sv->sv_export = is_export;
 	newsav->sav_var_vals_idx = si->sn_var_vals.ga_len;
 	++si->sn_var_vals.ga_len;
@@ -864,7 +871,7 @@ check_script_var_type(typval_T *dest, typval_T *value, char_u *name)
 
     if (sv != NULL)
     {
-	if (sv->sv_const)
+	if (sv->sv_const != 0)
 	{
 	    semsg(_(e_readonlyvar), name);
 	    return FAIL;