[PATCH] vbscript: Treat tokens after a '.' as identifier
Currently, a vbscript using an identifier that is also a keyword fails with a parser error. For example: Set oLocator = CreateObject("Wbemscripting.SWbemLocator") Set oReg = oLocator.ConnectServer("", "root\default", "", "").Get("StdRegProv") results in: 0009:fixme:vbscript:parse_script parser failed around L"efault\", \"\", \"\").Get(\"StdRegProv\")\n" The 'Get' method here causes the parser error as it is also a vbscript keyword. This patch treats any token after a '.' as an identifier. Wine-Bug: https://bugs.winehq.org/show_bug.cgi?id=46318 Signed-off-by: Brendan McGrath <brendan(a)redmandi.com> --- I _think_ this patch is OK - but I don't have a lot of experience with vbscript. There may be a scenario where a keyword can immediately follow a '.'. I've also created the bug linked above (where this patch is proposed). dlls/vbscript/lex.c | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/dlls/vbscript/lex.c b/dlls/vbscript/lex.c index 571854db58e..0d27c2423c7 100644 --- a/dlls/vbscript/lex.c +++ b/dlls/vbscript/lex.c @@ -412,7 +412,10 @@ static int parse_next_token(void *lval, parser_ctx_t *ctx) return parse_numeric_literal(ctx, lval); if(isalphaW(c)) { - int ret = check_keywords(ctx); + int ret = 0; + + if(ctx->last_token != '.') + ret = check_keywords(ctx); if(!ret) return parse_identifier(ctx, lval); if(ret != tREM) -- 2.17.1
Hi Brendan, Thanks for looking at this. On 19/12/2018 01:52, Brendan McGrath wrote:
Currently, a vbscript using an identifier that is also a keyword fails with a parser error. For example: Set oLocator = CreateObject("Wbemscripting.SWbemLocator") Set oReg = oLocator.ConnectServer("", "root\default", "", "").Get("StdRegProv")
results in: 0009:fixme:vbscript:parse_script parser failed around L"efault\", \"\", \"\").Get(\"StdRegProv\")\n"
The 'Get' method here causes the parser error as it is also a vbscript keyword.
This patch treats any token after a '.' as an identifier.
Wine-Bug: https://bugs.winehq.org/show_bug.cgi?id=46318 Signed-off-by: Brendan McGrath <brendan(a)redmandi.com> ---
I _think_ this patch is OK - but I don't have a lot of experience with vbscript. There may be a scenario where a keyword can immediately follow a '.'.
I think the issue is more general than an identifier after '.' token. We already have code for similar problem with 'property' keyword (see Identifier rule in parser.y), we probably could extend that. Also jscript has a more complete solution for similar problem (see ReservedAsIdentifier) that you could use as an example. Also please include some tests in the patch. Thanks, Jacek
Wine-Bug: https://bugs.winehq.org/show_bug.cgi?id=46318 Signed-off-by: Brendan McGrath <brendan(a)redmandi.com> --- dlls/vbscript/tests/run.c | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/dlls/vbscript/tests/run.c b/dlls/vbscript/tests/run.c index 191f5a79a0..bc3e0742df 100644 --- a/dlls/vbscript/tests/run.c +++ b/dlls/vbscript/tests/run.c @@ -2406,6 +2406,10 @@ static void run_tests(void) CHECK_CALLED(global_success_d); CHECK_CALLED(global_success_i); + hres = parse_script_ar("Set oLocator = CreateObject(\"Wbemscripting.SWbemLocator\")\r" + "Set oReg = oLocator.ConnectServer(\"\", \"root\\default\", \"\", \"\").Get(\"StdRegProv\")"); + todo_wine ok(hres == S_OK, "parse_script failed: %08x\n", hres); + run_from_res("lang.vbs"); run_from_res("api.vbs"); run_from_res("regexp.vbs"); -- 2.17.1
Wine-Bug: https://bugs.winehq.org/show_bug.cgi?id=46318 Signed-off-by: Brendan McGrath <brendan(a)redmandi.com> --- I took a look at the way jscript did this - but I couldn't work out how it was using '$1' as a value. So I ended up following the approach used for 'property'. This meant I had to add a new constant definition for each keyword that could be used as an indentifier. As a result I went with a conservative approach and only added three more (in addition to 'property'). As a result - this passes the test in the bug report but perhaps doesn't address the other keywords. dlls/vbscript/parser.y | 13 +++++++++++-- 1 file changed, 11 insertions(+), 2 deletions(-) diff --git a/dlls/vbscript/parser.y b/dlls/vbscript/parser.y index 020109998e..9e9f5ab932 100644 --- a/dlls/vbscript/parser.y +++ b/dlls/vbscript/parser.y @@ -72,6 +72,9 @@ static class_decl_t *add_dim_prop(parser_ctx_t*,class_decl_t*,dim_decl_t*,unsign static statement_t *link_statements(statement_t*,statement_t*); static const WCHAR propertyW[] = {'p','r','o','p','e','r','t','y',0}; +static const WCHAR getW[] = {'g','e','t',0}; +static const WCHAR setW[] = {'s','e','t',0}; +static const WCHAR letW[] = {'l','e','t',0}; #define STORAGE_IS_PRIVATE 1 #define STORAGE_IS_DEFAULT 2 @@ -138,7 +141,7 @@ static const WCHAR propertyW[] = {'p','r','o','p','e','r','t','y',0}; %type <dim_decl> DimDeclList DimDecl %type <dim_list> DimList %type <const_decl> ConstDecl ConstDeclList -%type <string> Identifier +%type <string> Identifier ReservedAsIdentifier %type <case_clausule> CaseClausules %% @@ -445,7 +448,13 @@ ArgumentDecl /* 'property' may be both keyword and identifier, depending on context */ Identifier - : tIdentifier { $$ = $1; } + : tIdentifier { $$ = $1; } + | ReservedAsIdentifier { $$ = $1; } + +ReservedAsIdentifier + : tGET { $$ = getW; } + | tSET { $$ = setW; } + | tLET { $$ = letW; } | tPROPERTY { $$ = propertyW; } /* Most statements accept both new line and ':' as separators */ diff --git a/dlls/vbscript/tests/run.c b/dlls/vbscript/tests/run.c index bc3e0742df..2cb52f4145 100644 --- a/dlls/vbscript/tests/run.c +++ b/dlls/vbscript/tests/run.c @@ -2408,7 +2408,6 @@ static void run_tests(void) - hres = parse_script_ar("Set oLocator = CreateObject(\"Wbemscripting.SWbemLocator\")\r" - "Set oReg = oLocator.ConnectServer(\"\", \"root\\default\", \"\", \"\").Get(\"StdRegProv\")"); - todo_wine ok(hres == S_OK, "parse_script failed: %08x\n", hres); + parse_script_a("Set oLocator = CreateObject(\"Wbemscripting.SWbemLocator\")\r" + "Set oReg = oLocator.ConnectServer(\"\", \"root\\default\", \"\", \"\").Get(\"StdRegProv\")"); run_from_res("lang.vbs"); run_from_res("api.vbs"); -- 2.17.1
Hi Brendan, On 2/10/19 1:45 AM, Brendan McGrath wrote:
Wine-Bug: https://bugs.winehq.org/show_bug.cgi?id=46318 Signed-off-by: Brendan McGrath <brendan(a)redmandi.com> --- I took a look at the way jscript did this - but I couldn't work out how it was using '$1' as a value. So I ended up following the approach used for 'property'.
jscript can do that, because it declares tokens to have <identifier> type (see line 170 of its parser) and lexer returns appropriate value (see check_keywords lval handling). Anyway, I'd be fine with your version once it has proper tests. BTW, it's probably a good idea to merge test and fix into one patch in this case.
This meant I had to add a new constant definition for each keyword that could be used as an indentifier.As a result I went with a conservative approach and only added three more (in addition to 'property').
Conservative approach is fine and even preferred. Some of other keywords may cause parser conflicts, so we need to be careful about that. Thanks, Jacek
Hi Brendan, On 2/10/19 1:45 AM, Brendan McGrath wrote:
Wine-Bug: https://bugs.winehq.org/show_bug.cgi?id=46318 Signed-off-by: Brendan McGrath <brendan(a)redmandi.com> --- dlls/vbscript/tests/run.c | 4 ++++ 1 file changed, 4 insertions(+)
diff --git a/dlls/vbscript/tests/run.c b/dlls/vbscript/tests/run.c index 191f5a79a0..bc3e0742df 100644 --- a/dlls/vbscript/tests/run.c +++ b/dlls/vbscript/tests/run.c @@ -2406,6 +2406,10 @@ static void run_tests(void) CHECK_CALLED(global_success_d); CHECK_CALLED(global_success_i);
+ hres = parse_script_ar("Set oLocator = CreateObject(\"Wbemscripting.SWbemLocator\")\r" + "Set oReg = oLocator.ConnectServer(\"\", \"root\\default\", \"\", \"\").Get(\"StdRegProv\")"); + todo_wine ok(hres == S_OK, "parse_script failed: %08x\n", hres);
Please integrate the test into existing lang.vbs. Note that you don't really need to create external objects to test that. You could, for example, try declare a variable called 'get'. Thanks, Jacek
I tried 'Dim Get' and 'Get = "xx"' in Windows, but they both create a "compilation error". So I think 'Get' can be used as a Function on an external object - but maybe can not be used as a declaration in vbscript. I'm not really familiar with the grammatical notation of vbscript - but I did find this: https://rosettacode.org/wiki/BNF_Grammar#VBScript It looks like after a 'dot', all the Keywords can be used (referred to as a QualifiedIDTail in the link above), but after a 'Dim', only an ExtendedID can be used (which only includes a subset of Keywords referred to as 'SafeKeywordID' - which includes 'Property'). So I think I should only be including the SafeKeywordIDs as an Identifier (which does not include 'Get', 'Set' or 'Let'). For items after a dot, I guess I should introduce a new token (maybe called DotIdentifier) which will include all the Keywords? I believe I would therefore need to use an external object to test those. On 12/2/19 10:45 pm, Jacek Caban wrote:
Hi Brendan,
On 2/10/19 1:45 AM, Brendan McGrath wrote:
Wine-Bug: https://bugs.winehq.org/show_bug.cgi?id=46318 Signed-off-by: Brendan McGrath <brendan(a)redmandi.com> --- dlls/vbscript/tests/run.c | 4 ++++ 1 file changed, 4 insertions(+)
diff --git a/dlls/vbscript/tests/run.c b/dlls/vbscript/tests/run.c index 191f5a79a0..bc3e0742df 100644 --- a/dlls/vbscript/tests/run.c +++ b/dlls/vbscript/tests/run.c @@ -2406,6 +2406,10 @@ static void run_tests(void) CHECK_CALLED(global_success_d); CHECK_CALLED(global_success_i); + hres = parse_script_ar("Set oLocator = CreateObject(\"Wbemscripting.SWbemLocator\")\r" + "Set oReg = oLocator.ConnectServer(\"\", \"root\\default\", \"\", \"\").Get(\"StdRegProv\")"); + todo_wine ok(hres == S_OK, "parse_script failed: %08x\n", hres);
Please integrate the test into existing lang.vbs. Note that you don't really need to create external objects to test that. You could, for example, try declare a variable called 'get'.
Thanks,
Jacek
Hi Brendan, On 2/13/19 1:33 AM, Brendan McGrath wrote:
I tried 'Dim Get' and 'Get = "xx"' in Windows, but they both create a "compilation error". So I think 'Get' can be used as a Function on an external object - but maybe can not be used as a declaration in vbscript.
I'm not really familiar with the grammatical notation of vbscript - but I did find this: https://rosettacode.org/wiki/BNF_Grammar#VBScript
It looks like after a 'dot', all the Keywords can be used (referred to as a QualifiedIDTail in the link above), but after a 'Dim', only an ExtendedID can be used (which only includes a subset of Keywords referred to as 'SafeKeywordID' - which includes 'Property'). So I think I should only be including the SafeKeywordIDs as an Identifier (which does not include 'Get', 'Set' or 'Let').
For items after a dot, I guess I should introduce a new token (maybe called DotIdentifier) which will include all the Keywords?
Yes, it looks like we will need something like that.
I believe I would therefore need to use an external object to test those.
Note that we already have such object in tests. It's accessible as testObj in scripts and implemented in run.c (mostly testObj_GetDispID and testObj_InvokeEx). We could probably have a single dispid for multiple keyword identifiers there to simplify things. Thanks, Jacek
participants (2)
-
Brendan McGrath -
Jacek Caban