[PATCH v6 0/1] MR10405: Draft: vbscript: Return proper error for undefined variables with Option Explicit.

[Francis De Brabandere (＠francisdb)]

When Option Explicit is set and an undefined variable is used, return
VBS error 500 (Variable is undefined) with the variable name in the
description, matching Windows behavior. Previously Wine returned E_FAIL
with no description.

--
  v6: vbscript: Return proper error for undefined variables with Option Explicit.
 
https://gitlab.winehq.org/wine/wine/-/merge_requests/10405

[PATCH v6 1/1] vbscript: Return proper error for undefined variables with Option Explicit.

[Francis De Brabandere]

From: Francis De Brabandere <[e-mail]>

When Option Explicit is set and an undefined variable is used, return
VBS error 500 (Variable is undefined) with the variable name in the
description, matching Windows behavior. Previously Wine returned E_FAIL
with no description.
---
 dlls/vbscript/interp.c        | 58 +++++++++++++++++++++++++++--------
 dlls/vbscript/tests/error.vbs |  4 +--
 dlls/vbscript/tests/run.c     | 33 ++++++++++++++++++++
 dlls/vbscript/vbscript.rc     |  1 +
 dlls/vbscript/vbscript_defs.h |  1 +
 5 files changed, 82 insertions(+), 15 deletions(-)

diff --git a/dlls/vbscript/interp.c b/dlls/vbscript/interp.c
index 00ef7d93eb6..621d0a844d2 100644
--- a/dlls/vbscript/interp.c
+++ b/dlls/vbscript/interp.c
@@ -301,6 +301,37 @@ static void clear_error_loc(script_ctx_t *ctx)
     }
 }
 
+static HRESULT throw_error(script_ctx_t *ctx, HRESULT error, const WCHAR *identifier)
+{
+    BSTR desc, source;
+
+    desc = get_vbscript_string(HRESULT_CODE(error));
+    if(desc && identifier) {
+        unsigned desc_len = SysStringLen(desc);
+        unsigned ident_len = lstrlenW(identifier);
+        /* format: "Error text: 'identifier'" */
+        BSTR new_desc = SysAllocStringLen(NULL, desc_len + 3 + ident_len + 1);
+        if(new_desc) {
+            memcpy(new_desc, desc, desc_len * sizeof(WCHAR));
+            new_desc[desc_len] = ':';
+            new_desc[desc_len + 1] = ' ';
+            new_desc[desc_len + 2] = '\'';
+            memcpy(new_desc + desc_len + 3, identifier, ident_len * sizeof(WCHAR));
+            new_desc[desc_len + 3 + ident_len] = '\'';
+            SysFreeString(desc);
+            desc = new_desc;
+        }
+    }
+
+    source = get_vbscript_string(VBS_RUNTIME_ERROR);
+
+    clear_ei(&ctx->ei);
+    ctx->ei.scode = error;
+    ctx->ei.bstrDescription = desc;
+    ctx->ei.bstrSource = source;
+    return SCRIPT_E_RECORDED;
+}
+
 static inline VARIANT *stack_pop(exec_ctx_t *ctx)
 {
     assert(ctx->top);
@@ -689,6 +720,8 @@ static HRESULT do_icall(exec_ctx_t *ctx, VARIANT *res, BSTR identifier, unsigned
             V_BYREF(res) = new;
             break;
         }
+        if(ctx->func->code_ctx->option_explicit)
+            return throw_error(ctx->script, MAKE_VBSERROR(VBSE_VARIABLE_NOT_DEFINED), identifier);
         FIXME("%s not found\n", debugstr_w(identifier));
         return DISP_E_UNKNOWNNAME;
     }
@@ -921,21 +954,20 @@ static HRESULT assign_ident(exec_ctx_t *ctx, BSTR name, WORD flags, DISPPARAMS *
     case REF_CONST:
         FIXME("REF_CONST\n");
         return E_NOTIMPL;
-    case REF_NONE:
-        if(ctx->func->code_ctx->option_explicit) {
-            FIXME("throw exception\n");
-            hres = E_FAIL;
-        }else {
-            VARIANT *new_var;
+    case REF_NONE: {
+        VARIANT *new_var;
 
-            if(arg_cnt(dp))
-                return DISP_E_TYPEMISMATCH;
+        if(ctx->func->code_ctx->option_explicit)
+            return throw_error(ctx->script, MAKE_VBSERROR(VBSE_VARIABLE_NOT_DEFINED), name);
 
-            TRACE("creating variable %s\n", debugstr_w(name));
-            hres = add_dynamic_var(ctx, name, FALSE, &new_var);
-            if(SUCCEEDED(hres))
-                hres = assign_value(ctx, new_var, dp->rgvarg, flags);
-        }
+        if(arg_cnt(dp))
+            return DISP_E_TYPEMISMATCH;
+
+        TRACE("creating variable %s\n", debugstr_w(name));
+        hres = add_dynamic_var(ctx, name, FALSE, &new_var);
+        if(SUCCEEDED(hres))
+            hres = assign_value(ctx, new_var, dp->rgvarg, flags);
+    }
     }
 
     return hres;
diff --git a/dlls/vbscript/tests/error.vbs b/dlls/vbscript/tests/error.vbs
index a9d4dc9c0be..b7a95afb9f5 100644
--- a/dlls/vbscript/tests/error.vbs
+++ b/dlls/vbscript/tests/error.vbs
@@ -506,13 +506,13 @@ err.clear
 
 ' Option Explicit: assigning to undeclared variable should give error 500
 undeclaredVar = 1
-todo_wine_ok err.number = 500, "err.number = " & err.number
+ok err.number = 500, "err.number = " & err.number
 err.clear
 
 ' Option Explicit: reading undeclared variable should give error 500
 dim unused
 unused = undeclaredVar2
-todo_wine_ok err.number = 500, "err.number = " & err.number
+ok err.number = 500, "err.number = " & err.number
 on error goto 0
 
 call reportSuccess()
diff --git a/dlls/vbscript/tests/run.c b/dlls/vbscript/tests/run.c
index 1b46cd49393..4c8417c0b5d 100644
--- a/dlls/vbscript/tests/run.c
+++ b/dlls/vbscript/tests/run.c
@@ -2195,6 +2195,38 @@ static HRESULT parse_script_wr(const WCHAR *src)
     return hres;
 }
 
+static void test_option_explicit_errors(void)
+{
+    IActiveScriptError *error;
+    EXCEPINFO ei;
+    HRESULT hres;
+
+    /* Without Option Explicit: assigning to undefined variable should succeed (implicit creation) */
+    parse_script_wf(SCRIPTITEM_GLOBALMEMBERS, L"x = 1\nCall ok(x = 1, \"x = \" & x)");
+
+    /* Without Option Explicit: reading undefined variable should succeed and return Empty */
+    parse_script_wf(SCRIPTITEM_GLOBALMEMBERS, L"Call ok(getVT(y) = \"VT_EMPTY*\", \"getVT(y) = \" & getVT(y))");
+
+    /* Option Explicit: assigning to undefined variable should give error 500 */
+    store_script_error = &error;
+    SET_EXPECT(OnScriptError);
+    hres = parse_script_wr(L"Option Explicit\nx = 1");
+    ok(hres == MAKE_VBSERROR(500), "expected MAKE_VBSERROR(500), got: %08lx\n", hres);
+    CHECK_CALLED(OnScriptError);
+
+    memset(&ei, 0, sizeof(ei));
+    hres = IActiveScriptError_GetExceptionInfo(error, &ei);
+    ok(hres == S_OK, "GetExceptionInfo returned %08lx\n", hres);
+    ok(ei.scode == MAKE_VBSERROR(500), "scode = %lx\n", ei.scode);
+    if(is_english)
+        ok(ei.bstrDescription && !wcscmp(ei.bstrDescription, L"Variable is undefined: 'x'"),
+           "bstrDescription = %s\n", wine_dbgstr_w(ei.bstrDescription));
+    SysFreeString(ei.bstrSource);
+    SysFreeString(ei.bstrDescription);
+    SysFreeString(ei.bstrHelpFile);
+    IActiveScriptError_Release(error);
+}
+
 static void test_parse_context(void)
 {
     IActiveScriptParse *parser;
@@ -3550,6 +3582,7 @@ static void run_tests(void)
     test_gc();
     test_msgbox();
     test_isexpression();
+    test_option_explicit_errors();
     test_parse_errors();
     test_parse_context();
     test_callbacks();
diff --git a/dlls/vbscript/vbscript.rc b/dlls/vbscript/vbscript.rc
index b9a1c3ad5c3..882e168e20b 100644
--- a/dlls/vbscript/vbscript.rc
+++ b/dlls/vbscript/vbscript.rc
@@ -58,6 +58,7 @@ STRINGTABLE
     VBSE_INVALID_DLL_FUNCTION_NAME     "Specified DLL function not found"
     VBSE_INVALID_TYPELIB_VARIABLE      "Variable uses an Automation type not supported in VBScript"
     VBSE_SERVER_NOT_FOUND              "The remote server machine does not exist or is unavailable"
+    VBSE_VARIABLE_NOT_DEFINED          "Variable is undefined"
     VBSE_UNQUALIFIED_REFERENCE         "Invalid or unqualified reference"
 
     VBS_COMPILE_ERROR                  "Microsoft VBScript compilation error"
diff --git a/dlls/vbscript/vbscript_defs.h b/dlls/vbscript/vbscript_defs.h
index e0e9d1cac94..0fb69a8c967 100644
--- a/dlls/vbscript/vbscript_defs.h
+++ b/dlls/vbscript/vbscript_defs.h
@@ -269,6 +269,7 @@
 #define VBSE_INVALID_DLL_FUNCTION_NAME    453
 #define VBSE_INVALID_TYPELIB_VARIABLE     458
 #define VBSE_SERVER_NOT_FOUND             462
+#define VBSE_VARIABLE_NOT_DEFINED         500
 #define VBSE_UNQUALIFIED_REFERENCE        505
 
 #define VBS_COMPILE_ERROR                4096
-- 
GitLab

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