patch 8.2.0602: :unlet $VAR does not work properly

Problem:    :unlet $VAR does not work properly.
Solution:   Make ":lockvar $VAR" fail.  Check the "skip" flag.

src/evalvars.c

@@ -1417,14 +1417,17 @@ ex_unletlock(
     {
 	if (*arg == '$')
 	{
-	    char_u    *name = ++arg;
+	    lv.ll_name = arg;
-
+	    lv.ll_tv = NULL;
+	    ++arg;
 	    if (get_env_len(&arg) == 0)
 	    {
-		semsg(_(e_invarg2), name - 1);
+		semsg(_(e_invarg2), arg - 1);
 		return;
 	    }
-	    vim_unsetenv(name);
+	    if (!error && !eap->skip
+			      && callback(&lv, arg, eap, deep, cookie) == FAIL)
+		error = TRUE;
 	    arg = skipwhite(arg);
 	    continue;
 	}
@@ -1477,8 +1480,10 @@ do_unlet_var(
 	cc = *name_end;
 	*name_end = NUL;
 
-	// Normal name or expanded name.
+	// Environment variable, normal name or expanded name.
-	if (do_unlet(lp->ll_name, forceit) == FAIL)
+	if (*lp->ll_name == '$')
+	    vim_unsetenv(lp->ll_name + 1);
+	else if (do_unlet(lp->ll_name, forceit) == FAIL)
 	    ret = FAIL;
 	*name_end = cc;
     }
@@ -1608,24 +1613,34 @@ do_lock_var(
     {
 	cc = *name_end;
 	*name_end = NUL;
-
+	if (*lp->ll_name == '$')
-	// Normal name or expanded name.
+	{
-	di = find_var(lp->ll_name, NULL, TRUE);
+	    semsg(_(e_lock_unlock), lp->ll_name);
-	if (di == NULL)
 	    ret = FAIL;
-	else if ((di->di_flags & DI_FLAGS_FIX)
+	}
-			&& di->di_tv.v_type != VAR_DICT
-			&& di->di_tv.v_type != VAR_LIST)
-	    // For historic reasons this error is not given for a list or dict.
-	    // E.g., the b: dict could be locked/unlocked.
-	    semsg(_("E940: Cannot lock or unlock variable %s"), lp->ll_name);
 	else
 	{
-	    if (lock)
+	    // Normal name or expanded name.
-		di->di_flags |= DI_FLAGS_LOCK;
+	    di = find_var(lp->ll_name, NULL, TRUE);
+	    if (di == NULL)
+		ret = FAIL;
+	    else if ((di->di_flags & DI_FLAGS_FIX)
+			    && di->di_tv.v_type != VAR_DICT
+			    && di->di_tv.v_type != VAR_LIST)
+	    {
+		// For historic reasons this error is not given for a list or
+		// dict.  E.g., the b: dict could be locked/unlocked.
+		semsg(_(e_lock_unlock), lp->ll_name);
+		ret = FAIL;
+	    }
 	    else
-		di->di_flags &= ~DI_FLAGS_LOCK;
+	    {
-	    item_lock(&di->di_tv, deep, lock);
+		if (lock)
+		    di->di_flags |= DI_FLAGS_LOCK;
+		else
+		    di->di_flags &= ~DI_FLAGS_LOCK;
+		item_lock(&di->di_tv, deep, lock);
+	    }
 	}
 	*name_end = cc;
     }

src/globals.h

@@ -1749,6 +1749,7 @@ EXTERN char e_endif_without_if[] INIT(= N_("E580: :endif without :if"));
 EXTERN char e_continue[]	INIT(= N_("E586: :continue without :while or :for"));
 EXTERN char e_break[]		INIT(= N_("E587: :break without :while or :for"));
 EXTERN char e_nowhitespace[]	INIT(= N_("E274: No white space allowed before parenthesis"));
+EXTERN char e_lock_unlock[]	INIT(= N_("E940: Cannot lock or unlock variable %s"));
 #endif
 
 #ifdef FEAT_GUI_MAC

src/testdir/test_vimscript.vim

@@ -1711,6 +1711,20 @@ func Test_compound_assignment_operators()
     let @/ = ''
 endfunc
 
+func Test_unlet_env()
+  let $TESTVAR = 'yes'
+  call assert_equal('yes', $TESTVAR)
+  call assert_fails('lockvar $TESTVAR', 'E940')
+  call assert_fails('unlockvar $TESTVAR', 'E940')
+  call assert_equal('yes', $TESTVAR)
+  if 0
+    unlet $TESTVAR
+  endif
+  call assert_equal('yes', $TESTVAR)
+  unlet $TESTVAR
+  call assert_equal('', $TESTVAR)
+endfunc
+
 func Test_refcount()
     " Immediate values
     call assert_equal(-1, test_refcount(1))

src/version.c

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