[PATCH v4 0/1] MR4418: cmd: Delay variable substitution until after the if clause is parsed.

[Alex Henrie (＠alexhenrie)]

Wine-Bug: https://bugs.winehq.org/show_bug.cgi?id=55859

--
  v4: cmd: Delay variable substitution until after the if clause is parsed.

https://gitlab.winehq.org/wine/wine/-/merge_requests/4418

[PATCH v4 1/1] cmd: Delay variable substitution until after the if clause is parsed.

[Alex Henrie]

From: Alex Henrie

Wine-Bug: https://bugs.winehq.org/show_bug.cgi?id=55859
---
 programs/cmd/builtins.c                  | 35 +++++++++++++++++++-----
 programs/cmd/tests/test_builtins.cmd     |  5 ++++
 programs/cmd/tests/test_builtins.cmd.exp |  1 +
 programs/cmd/wcmd.h                      |  1 +
 programs/cmd/wcmdmain.c                  |  9 +++---
 5 files changed, 40 insertions(+), 11 deletions(-)

diff --git a/programs/cmd/builtins.c b/programs/cmd/builtins.c
index ca703af52ec..85870193f55 100644
--- a/programs/cmd/builtins.c
+++ b/programs/cmd/builtins.c
@@ -2818,9 +2818,24 @@ static int evaluate_if_comparison(const WCHAR *leftOperand, const WCHAR *operato
     return -1;
 }

+static void do_delayed_expansion(WCHAR *p)
+{
+  /* Perform delayed substitution after the if clause is parsed */
+  if (!delayedsubst) return;
+  for (p = wcschr(p, '!'); p; p = wcschr(p, '!'))
+    WCMD_expand_envvar(p, '!');
+}
+
+static void expand_first_param(WCHAR *dst, WCHAR *src, int negate)
+{
+  wcscpy(dst, WCMD_parameter(src, 1 + negate, NULL, FALSE, FALSE));
+  do_delayed_expansion(dst);
+}
+
 int evaluate_if_condition(WCHAR *p, WCHAR **command, int *test, int *negate)
 {
   WCHAR condition[MAX_PATH];
+  WCHAR param[MAXSTRING];
   int caseInsensitive = (wcsstr(quals, L"/I") != NULL);

*negate = !lstrcmpiW(param1,L"not");
@@ -2828,9 +2843,10 @@ int evaluate_if_condition(WCHAR *p, WCHAR **command, int *test, int *negate)
   WINE_TRACE("Condition: %s\n", wine_dbgstr_w(condition));

if (!lstrcmpiW(condition, L"errorlevel")) {
-    WCHAR *param = WCMD_parameter(p, 1+(*negate), NULL, FALSE, FALSE);
     WCHAR *endptr;
-    long int param_int = wcstol(param, &endptr, 10);
+    long int param_int;
+    expand_first_param(param, p, *negate);
+    param_int = wcstol(param, &endptr, 10);
     if (*endptr) goto syntax_err;
     *test = ((long int)errorlevel >= param_int);
     WCMD_parameter(p, 2+(*negate), command, FALSE, FALSE);
@@ -2838,8 +2854,10 @@ int evaluate_if_condition(WCHAR *p, WCHAR **command, int *test, int *negate)
   else if (!lstrcmpiW(condition, L"exist")) {
     WIN32_FIND_DATAW fd;
     HANDLE hff;
-    WCHAR *param = WCMD_parameter(p, 1+(*negate), NULL, FALSE, FALSE);
-    int    len = lstrlenW(param);
+    int len;
+
+    expand_first_param(param, p, *negate);
+    len = wcslen(param);

if (!len) {
         *test = FALSE;
@@ -2855,8 +2873,8 @@ int evaluate_if_condition(WCHAR *p, WCHAR **command, int *test, int *negate)
     WCMD_parameter(p, 2+(*negate), command, FALSE, FALSE);
   }
   else if (!lstrcmpiW(condition, L"defined")) {
-    *test = (GetEnvironmentVariableW(WCMD_parameter(p, 1+(*negate), NULL, FALSE, FALSE),
-                                    NULL, 0) > 0);
+    expand_first_param(param, p, *negate);
+    *test = (GetEnvironmentVariableW(param, NULL, 0) > 0);
     WCMD_parameter(p, 2+(*negate), command, FALSE, FALSE);
   }
   else { /* comparison operation */
@@ -2883,12 +2901,15 @@ int evaluate_if_condition(WCHAR *p, WCHAR **command, int *test, int *negate)
     lstrcpyW(rightOperand, WCMD_parameter(p, 0, &paramStart, TRUE, FALSE));
     if (!*rightOperand)
       goto syntax_err;
+    p = paramStart + lstrlenW(rightOperand);
+
+    do_delayed_expansion(leftOperand);
+    do_delayed_expansion(rightOperand);

*test = evaluate_if_comparison(leftOperand, operator, rightOperand, caseInsensitive);
     if (*test == -1)
       goto syntax_err;

-    p = paramStart + lstrlenW(rightOperand);
     WCMD_parameter(p, 0, command, FALSE, FALSE);
   }

diff --git a/programs/cmd/tests/test_builtins.cmd b/programs/cmd/tests/test_builtins.cmd
index 7517cca343f..5a4dcb76243 100644
--- a/programs/cmd/tests/test_builtins.cmd
+++ b/programs/cmd/tests/test_builtins.cmd
@@ -785,6 +785,11 @@ if %WINE_FOO% == foo (
     set WINE_FOO=bar
     if !WINE_FOO! == bar (echo bar) else echo foo
 )
+
+rem if expansion is delayed, spaces in the variable do not cause a syntax error
+set WINE_FOO=test kest
+if !WINE_FOO!=="" (echo empty) else echo not empty
+
 echo %ErrorLevel%
 setlocal DisableDelayedExpansion
 echo %ErrorLevel%
diff --git a/programs/cmd/tests/test_builtins.cmd.exp b/programs/cmd/tests/test_builtins.cmd.exp
index 1365a5c3474..2b0e91283e2 100644
--- a/programs/cmd/tests/test_builtins.cmd.exp
+++ b/programs/cmd/tests/test_builtins.cmd.exp
@@ -614,6 +614,7 @@ foo
 foo@or_broken@!WINE_FOO!
 foo
 bar@or_broken@foo
+not empty
 0
 0@or_broken@1
 foo
diff --git a/programs/cmd/wcmd.h b/programs/cmd/wcmd.h
index 1935bbcdcd2..ae06e864a5f 100644
--- a/programs/cmd/wcmd.h
+++ b/programs/cmd/wcmd.h
@@ -69,6 +69,7 @@ void WCMD_echo (const WCHAR *);
 void WCMD_endlocal (void);
 void WCMD_enter_paged_mode(const WCHAR *);
 void WCMD_exit (CMD_LIST **cmdList);
+WCHAR* WCMD_expand_envvar(WCHAR *start, WCHAR startchar);
 void WCMD_for (WCHAR *, CMD_LIST **cmdList);
 BOOL WCMD_get_fullpath(const WCHAR *, SIZE_T, WCHAR *, WCHAR **);
 void WCMD_give_help (const WCHAR *args);
diff --git a/programs/cmd/wcmdmain.c b/programs/cmd/wcmdmain.c
index eb66b29836e..80e28ae1518 100644
--- a/programs/cmd/wcmdmain.c
+++ b/programs/cmd/wcmdmain.c
@@ -551,7 +551,7 @@ static inline BOOL WCMD_is_magic_envvar(const WCHAR *s, const WCHAR *magicvar)
  *
  *	Expands environment variables, allowing for WCHARacter substitution
  */
-static WCHAR *WCMD_expand_envvar(WCHAR *start, WCHAR startchar)
+WCHAR *WCMD_expand_envvar(WCHAR *start, WCHAR startchar)
 {
     WCHAR *endOfVar = NULL, *s;
     WCHAR *colonpos = NULL;
@@ -1360,9 +1360,10 @@ void WCMD_execute (const WCHAR *command, const WCHAR *redirects,
     }

/* Expand variables in command line mode only (batch mode will
-       be expanded as the line is read in, except for 'for' loops) */
-    handleExpansion(new_cmd, (context != NULL), delayedsubst);
-    handleExpansion(new_redir, (context != NULL), delayedsubst);
+       be expanded as the line is read in, except for 'for' loops).
+       Do delayed substitution now if it is enabled and the command is not IF. */
+    handleExpansion(new_cmd, context != NULL, delayedsubst && cmd_index != WCMD_IF);
+    handleExpansion(new_redir, context != NULL, delayedsubst && cmd_index != WCMD_IF);

/*
  * Changing default drive has to be handled as a special case, anything

[eric pouech (＠epo)]

I still have the same comments as two months ago, but since, I rewrote cmd.exe internal command engine and it now properly supports this (as well as its siblings in the FOR command - see MR!4504 -, and bunch of other features like command chaining, precedence of command chaining).

I started pushing MR!5592 for it (there will be several more), which conflicts with this MR.