[PATCH v2] vbscript: Support properties with parameters.

[Robert Wilhelm]

Wine-Bug: https://bugs.winehq.org/show_bug.cgi?id=33996
Signed-off-by: Robert Wilhelm <robert.wilhelm(a)gmx.net>
---
 dlls/vbscript/parser.y       |  2 +-
 dlls/vbscript/tests/lang.vbs | 22 ++++++++++++++++++
 dlls/vbscript/vbdisp.c       | 45 ++++++++++++++++++++++++++----------
 3 files changed, 56 insertions(+), 13 deletions(-)

diff --git a/dlls/vbscript/parser.y b/dlls/vbscript/parser.y
index a9f4c540fcb..8f1c1307794 100644
--- a/dlls/vbscript/parser.y
+++ b/dlls/vbscript/parser.y
@@ -454,7 +454,7 @@ ClassBody
 PropertyDecl
     : Storage_opt tPROPERTY tGET Identifier ArgumentsDecl_opt StSep BodyStatements tEND tPROPERTY
                                     { $$ = new_function_decl(ctx, $4, FUNC_PROPGET, $1, $5, $7); CHECK_ERROR; }
-    | Storage_opt tPROPERTY tLET Identifier '(' ArgumentDecl ')' StSep BodyStatements tEND tPROPERTY
+    | Storage_opt tPROPERTY tLET Identifier '(' ArgumentDeclList ')' StSep BodyStatements tEND tPROPERTY
                                     { $$ = new_function_decl(ctx, $4, FUNC_PROPLET, $1, $6, $9); CHECK_ERROR; }
     | Storage_opt tPROPERTY tSET Identifier '(' ArgumentDecl ')' StSep BodyStatements tEND tPROPERTY
                                     { $$ = new_function_decl(ctx, $4, FUNC_PROPSET, $1, $6, $9); CHECK_ERROR; }
diff --git a/dlls/vbscript/tests/lang.vbs b/dlls/vbscript/tests/lang.vbs
index 53f36b6e710..c36302c0c2f 100644
--- a/dlls/vbscript/tests/lang.vbs
+++ b/dlls/vbscript/tests/lang.vbs
@@ -1770,6 +1770,28 @@ class TestPropSyntax
     end property
 end class

+Class TestPropParam
+    Public oDict
+    Public m_obj
+    Public Property Let Key(oldKey, newKey)
+        oDict =oldKey & newKey
+    End Property
+    Public Property Let three(uno,due,tre)
+        oDict =uno & due & tre
+    End Property
+    Public Property Let ten(a,b,c,d,e,f,g,h,i,j)
+        oDict =a & b & c & d & e & f & g & h & i & j
+    End Property
+End Class
+
+Set x = new TestPropParam
+x.key("old") = "new"
+call ok(x.oDict = "oldnew","x.oDict = " & x.oDict & " expected oldnew")
+x.three(1,2) = 3
+call ok(x.oDict = "123","x.oDict = " & x.oDict & " expected 123")
+x.ten(1,2,3,4,5,6,7,8,9) = 0
+call ok(x.oDict = "1234567890","x.oDict = " & x.oDict & " expected 1234567890")
+
 set x = new TestPropSyntax
 set x.prop = new TestPropSyntax
 set x.prop.prop = new TestPropSyntax
diff --git a/dlls/vbscript/vbdisp.c b/dlls/vbscript/vbdisp.c
index 5f493d420bd..75b7badc5c7 100644
--- a/dlls/vbscript/vbdisp.c
+++ b/dlls/vbscript/vbdisp.c
@@ -78,7 +78,7 @@ HRESULT vbdisp_get_id(vbdisp_t *This, BSTR name, vbdisp_invoke_type_t invoke_typ
     return DISP_E_UNKNOWNNAME;
 }

-static HRESULT get_propput_arg(script_ctx_t *ctx, const DISPPARAMS *dp, WORD flags, VARIANT *v, BOOL *is_owned)
+static HRESULT get_propput_arg(script_ctx_t *ctx, const DISPPARAMS *dp, WORD flags, VARIANT *v, BOOL *is_owned, INT *index)
 {
     unsigned i;

@@ -88,9 +88,11 @@ static HRESULT get_propput_arg(script_ctx_t *ctx, const DISPPARAMS *dp, WORD fla
     }
     if(i == dp->cNamedArgs) {
         WARN("no value to set\n");
+        *index=-1;
         return DISP_E_PARAMNOTOPTIONAL;
     }

+    *index = i;
     *v = dp->rgvarg[i];
     if(V_VT(v) == (VT_VARIANT|VT_BYREF))
         *v = *V_VARIANTREF(v);
@@ -134,8 +136,9 @@ static HRESULT invoke_variant_prop(script_ctx_t *ctx, VARIANT *v, WORD flags, DI
     case DISPATCH_PROPERTYPUT|DISPATCH_PROPERTYPUTREF: {
         VARIANT put_val;
         BOOL own_val;
+        INT index;

-        hres = get_propput_arg(ctx, dp, flags, &put_val, &own_val);
+        hres = get_propput_arg(ctx, dp, flags, &put_val, &own_val, &index);
         if(FAILED(hres))
             return hres;

@@ -197,28 +200,46 @@ static HRESULT invoke_vbdisp(vbdisp_t *This, DISPID id, DWORD flags, BOOL extern
         case DISPATCH_PROPERTYPUT|DISPATCH_PROPERTYPUTREF: {
             DISPPARAMS dp = {NULL, NULL, 1, 0};
             BOOL needs_release;
-            VARIANT put_val;
+            VARIANT buf[6];
             HRESULT hres;
-
-            if(arg_cnt(params)) {
-                FIXME("arguments not implemented\n");
-                return E_NOTIMPL;
+            INT index,i,j;
+
+            dp.cArgs = arg_cnt(params) + 1;
+            if(dp.cArgs > ARRAY_SIZE(buf)) {
+                dp.rgvarg = heap_alloc(dp.cArgs*sizeof(VARIANT));
+                if(!dp.rgvarg)
+                    return E_OUTOFMEMORY;
+            }else {
+                dp.rgvarg = buf;
             }

-            hres = get_propput_arg(This->desc->ctx, params, flags, &put_val, &needs_release);
-            if(FAILED(hres))
+            hres = get_propput_arg(This->desc->ctx, params, flags, dp.rgvarg, &needs_release, &index);
+            if(FAILED(hres)) {
+                if(dp.rgvarg != buf)
+                    heap_free(dp.rgvarg);
                 return hres;
+            }

-            dp.rgvarg = &put_val;
-            func = This->desc->funcs[id].entries[V_VT(&put_val) == VT_DISPATCH ? VBDISP_SET : VBDISP_LET];
+            func = This->desc->funcs[id].entries[V_VT(dp.rgvarg) == VT_DISPATCH ? VBDISP_SET : VBDISP_LET];
             if(!func) {
                 FIXME("no letter/setter\n");
+                if(dp.rgvarg != buf)
+                    heap_free(dp.rgvarg);
                 return DISP_E_MEMBERNOTFOUND;
             }

+            for( i=1,j=0; i<dp.cArgs;i++,j++) {
+                if (j==index) {
+                    j++;
+                }
+                dp.rgvarg[i]=params->rgvarg[j];
+            }
+
             hres = exec_script(This->desc->ctx, extern_caller, func, This, &dp, NULL);
             if(needs_release)
-                VariantClear(&put_val);
+                VariantClear(dp.rgvarg);
+            if(dp.rgvarg != buf)
+                heap_free(dp.rgvarg);
             return hres;
         }
         default:
--
2.26.2

[Jacek Caban]

Hi Robert,

On 11/12/20 7:52 AM, Robert Wilhelm wrote:
> +            for( i=1,j=0; i<dp.cArgs;i++,j++) {
> +                if (j==index) {
> +                    j++;
> +                }
> +                dp.rgvarg[i]=params->rgvarg[j];
> +            }

This is inconsistent with how you allocate dp, where you use arg_cnt(). 
I think that allocation is right and we should skip first cNamedArgs 
entries of rgvargs and copy the rest here. You could then avoid 
returning index from get_propput_arg.

Thanks,
Jacek

[Robert Wilhelm]

Hi Jacek,

thank you for the review.
I assumed there is only one Named Arg.
I will send updated patch shortly.

Regards,
Robert

On Fri, 2020-11-13 at 22:22 +0100, Jacek Caban wrote:
> Hi Robert,
>
> On 11/12/20 7:52 AM, Robert Wilhelm wrote:
> > +            for( i=1,j=0; i<dp.cArgs;i++,j++) {
> > +                if (j==index) {
> > +                    j++;
> > +                }
> > +                dp.rgvarg[i]=params->rgvarg[j];
> > +            }
>
> This is inconsistent with how you allocate dp, where you use
> arg_cnt().
> I think that allocation is right and we should skip first cNamedArgs
> entries of rgvargs and copy the rest here. You could then avoid
> returning index from get_propput_arg.
>
> Thanks,
> Jacek