patch 8.2.1824: Vim9: variables at the script level escape their scope

Problem:    Vim9: variables at the script level escape their scope.
Solution:   When leaving a scope remove variables declared in it.

src/evalvars.c

@@ -174,7 +174,6 @@ static char_u *list_arg_vars(exarg_T *eap, char_u *arg, int *first);
 static char_u *ex_let_one(char_u *arg, typval_T *tv, int copy, int flags, char_u *endchars, char_u *op);
 static int do_unlet_var(lval_T *lp, char_u *name_end, exarg_T *eap, int deep, void *cookie);
 static int do_lock_var(lval_T *lp, char_u *name_end, exarg_T *eap, int deep, void *cookie);
-static void delete_var(hashtab_T *ht, hashitem_T *hi);
 static void list_one_var(dictitem_T *v, char *prefix, int *first);
 static void list_one_var_a(char *prefix, char_u *name, int type, char_u *string, int *first);
 
@@ -2890,7 +2889,7 @@ vars_clear_ext(hashtab_T *ht, int free_val)
  * Delete a variable from hashtab "ht" at item "hi".
  * Clear the variable value and free the dictitem.
  */
-    static void
+    void
 delete_var(hashtab_T *ht, hashitem_T *hi)
 {
     dictitem_T	*di = HI2DI(hi);

src/ex_eval.c

@@ -905,6 +905,48 @@ ex_eval(exarg_T *eap)
     clear_evalarg(&evalarg, eap);
 }
 
+/*
+ * Start a new scope/block.  Caller should have checked that cs_idx is not
+ * exceeding CSTACK_LEN.
+ */
+    static void
+enter_block(cstack_T *cstack)
+{
+    ++cstack->cs_idx;
+    if (in_vim9script())
+	cstack->cs_script_var_len[cstack->cs_idx] =
+			  SCRIPT_ITEM(current_sctx.sc_sid)->sn_var_vals.ga_len;
+}
+
+    static void
+leave_block(cstack_T *cstack)
+{
+    int i;
+
+    if (in_vim9script())
+    {
+	scriptitem_T *si = SCRIPT_ITEM(current_sctx.sc_sid);
+
+	for (i = cstack->cs_script_var_len[cstack->cs_idx];
+					       i < si->sn_var_vals.ga_len; ++i)
+	{
+	    svar_T	*sv = ((svar_T *)si->sn_var_vals.ga_data) + i;
+	    hashtab_T	*ht = get_script_local_ht();
+	    hashitem_T	*hi;
+
+	    if (ht != NULL)
+	    {
+		// Remove a variable declared inside the block, if it still
+		// exists.
+		hi = hash_find(ht, sv->sv_name);
+		if (!HASHITEM_EMPTY(hi))
+		    delete_var(ht, hi);
+	    }
+	}
+    }
+    --cstack->cs_idx;
+}
+
 /*
  * ":if".
  */
@@ -920,12 +962,12 @@ ex_if(exarg_T *eap)
 	eap->errmsg = _("E579: :if nesting too deep");
     else
     {
-	++cstack->cs_idx;
+	enter_block(cstack);
 	cstack->cs_flags[cstack->cs_idx] = 0;
 
 	/*
-	 * Don't do something after an error, interrupt, or throw, or when there
-	 * is a surrounding conditional and it was not active.
+	 * Don't do something after an error, interrupt, or throw, or when
+	 * there is a surrounding conditional and it was not active.
 	 */
 	skip = did_emsg || got_int || did_throw || (cstack->cs_idx > 0
 		&& !(cstack->cs_flags[cstack->cs_idx - 1] & CSF_ACTIVE));
@@ -949,9 +991,11 @@ ex_if(exarg_T *eap)
     void
 ex_endif(exarg_T *eap)
 {
+    cstack_T	*cstack = eap->cstack;
+
     did_endif = TRUE;
-    if (eap->cstack->cs_idx < 0
-	    || (eap->cstack->cs_flags[eap->cstack->cs_idx]
+    if (cstack->cs_idx < 0
+	    || (cstack->cs_flags[cstack->cs_idx]
 					   & (CSF_WHILE | CSF_FOR | CSF_TRY)))
 	eap->errmsg = _(e_endif_without_if);
     else
@@ -965,11 +1009,11 @@ ex_endif(exarg_T *eap)
 	 * Doing this here prevents an exception for a parsing error being
 	 * discarded by throwing the interrupt exception later on.
 	 */
-	if (!(eap->cstack->cs_flags[eap->cstack->cs_idx] & CSF_TRUE)
+	if (!(cstack->cs_flags[cstack->cs_idx] & CSF_TRUE)
 						    && dbg_check_skipped(eap))
-	    (void)do_intthrow(eap->cstack);
+	    (void)do_intthrow(cstack);
 
-	--eap->cstack->cs_idx;
+	leave_block(cstack);
     }
 }
 
@@ -1086,7 +1130,7 @@ ex_while(exarg_T *eap)
 	 */
 	if ((cstack->cs_lflags & CSL_HAD_LOOP) == 0)
 	{
-	    ++cstack->cs_idx;
+	    enter_block(cstack);
 	    ++cstack->cs_looplevel;
 	    cstack->cs_line[cstack->cs_idx] = -1;
 	}
@@ -1450,7 +1494,7 @@ ex_try(exarg_T *eap)
 	eap->errmsg = _("E601: :try nesting too deep");
     else
     {
-	++cstack->cs_idx;
+	enter_block(cstack);
 	++cstack->cs_trylevel;
 	cstack->cs_flags[cstack->cs_idx] = CSF_TRY;
 	cstack->cs_pending[cstack->cs_idx] = CSTP_NONE;
@@ -1923,7 +1967,7 @@ ex_endtry(exarg_T *eap)
 	 */
 	(void)cleanup_conditionals(cstack, CSF_TRY | CSF_SILENT, TRUE);
 
-	--cstack->cs_idx;
+	leave_block(cstack);
 	--cstack->cs_trylevel;
 
 	if (!skip)
@@ -2303,7 +2347,7 @@ rewind_conditionals(
 	    --*cond_level;
 	if (cstack->cs_flags[cstack->cs_idx] & CSF_FOR)
 	    free_for_info(cstack->cs_forinfo[cstack->cs_idx]);
-	--cstack->cs_idx;
+	leave_block(cstack);
     }
 }
 

src/proto/evalvars.pro

@@ -67,6 +67,7 @@ void init_var_dict(dict_T *dict, dictitem_T *dict_var, int scope);
 void unref_var_dict(dict_T *dict);
 void vars_clear(hashtab_T *ht);
 void vars_clear_ext(hashtab_T *ht, int free_val);
+void delete_var(hashtab_T *ht, hashitem_T *hi);
 void set_var(char_u *name, typval_T *tv, int copy);
 void set_var_const(char_u *name, type_T *type, typval_T *tv_arg, int copy, int flags);
 int var_check_ro(int flags, char_u *name, int use_gettext);

src/structs.h

@@ -889,6 +889,8 @@ typedef struct {
     }		cs_pend;
     void	*cs_forinfo[CSTACK_LEN]; // info used by ":for"
     int		cs_line[CSTACK_LEN];	// line nr of ":while"/":for" line
+    int		cs_script_var_len[CSTACK_LEN];	// value of sn_var_vals.ga_len
+						// when entering the block
     int		cs_idx;			// current entry, or -1 if none
     int		cs_looplevel;		// nr of nested ":while"s and ":for"s
     int		cs_trylevel;		// nr of nested ":try"s

src/testdir/test_vim9_script.vim

@@ -2685,6 +2685,56 @@ def Run_Test_define_func_at_command_line()
   delete('Xdidcmd')
 enddef
 
+def Test_script_var_scope()
+  var lines =<< trim END
+      vim9script
+      if true
+        if true
+          var one = 'one'
+          echo one
+        endif
+        echo one
+      endif
+  END
+  CheckScriptFailure(lines, 'E121:', 7)
+
+  lines =<< trim END
+      vim9script
+      if true
+        if false
+          var one = 'one'
+          echo one
+        else
+          var one = 'one'
+          echo one
+        endif
+        echo one
+      endif
+  END
+  CheckScriptFailure(lines, 'E121:', 10)
+
+  lines =<< trim END
+      vim9script
+      while true
+        var one = 'one'
+        echo one
+        break
+      endwhile
+      echo one
+  END
+  CheckScriptFailure(lines, 'E121:', 7)
+
+  lines =<< trim END
+      vim9script
+      for i in range(1)
+        var one = 'one'
+        echo one
+      endfor
+      echo one
+  END
+  CheckScriptFailure(lines, 'E121:', 6)
+enddef
+
 " Keep this last, it messes up highlighting.
 def Test_substitute_cmd()
   new

src/version.c

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