Njs: Shorthand property names syntax support for objects

[xeioex]

// Shorthand property names (ES2015)
var a = 'foo', b = 42, c = {};
var o = {a, b, c};

to support expressions like export {a,b c} in #86.

[xeioex]

@hongzhidao, @drsm

Please, take a look:
https://gist.github.com/xeioex/2ee68d88aad9fdccb0e58a6c4f862260

[drsm]

Hi @xeioex !
The patch works fine for me, except some minor issues:

// 1
>> ({ Math })
ReferenceError: "Math" is not defined in shell:1
// node
> ({ Math })
{ Math: Object [Math] {} }
// also
>> var Math
SyntaxError: Unexpected token "Math" in shell:1

// 2
>> ({ default })
ReferenceError: "default" is not defined in shell:1
//node
> ({ default })
({ default })
           ^
SyntaxError: Unexpected token }

// 3
>> var y = function() { return { this }; }
undefined
//node
var y = function() { return { this }; }
                                   ^
SyntaxError: Unexpected token }

[xeioex]

@drsm
Thank you for the feedback.

except some minor issues:
Math

Yep, known issue. Currently, global built-in objects are special keywords.

[xeioex]

@drsm

Fixed the issue with reserved words: https://gist.github.com/xeioex/1664cc5c76a6e8da12cc4e5d8abcfbeb

Currently, global built-in objects are special keywords.

is a separate issue, will be fixed later.

[hongzhidao]

@xeioex

1. > Currently, global built-in objects are special keywords.
   is a separate issue, will be fixed later.

Agree, and it needs to be fixed first since the bellow is ok in njs now.

var a = { Math: Math }

But the following doesn't work with the patch, this is unreasonable.

var a = { Math }

It seems builtin objects and functions need to be preprocessed.

1. Improved njs_parser_property_token.
   https://gist.github.com/hongzhidao/a9f48bb96e5c050fae0d0776572c6c55
   The function njs_parser_property_token() can be improved before implementing the feature.

[hongzhidao]

@xeioex @drsm, take a look.

var a = { Math }

I think we need to regard 'variable', 'external', and some of keywords as the same symbol.
So I introduced njs_parser_reference().

Here's the patch, I split it into three parts.
https://gist.github.com/hongzhidao/245f2a04b457e1314a0c4a6816ae6819

[xeioex]

@hongzhidao

I think we need to regard 'variable', 'external', and some of keywords as the same symbol.
So I introduced njs_parser_reference().

Good idea! Thanks.

[xeioex]

@hongzhidao

I reworked 2nd and 3rd patches a bit. Take a look:
https://gist.github.com/xeioex/6240f1300ca4e62b7c55651280beb8c3

[hongzhidao]

@xeioex
No problem except some places.

1. Add (void).
   https://gist.github.com/xeioex/6240f1300ca4e62b7c55651280beb8c3#file-github87-3-patch-L430

(void) njs_parser_unexpected_token(vm, parser, token);

1. Style.
   https://gist.github.com/xeioex/6240f1300ca4e62b7c55651280beb8c3#file-github87-3-patch-L906
2. What about using prev_token instead of property_token?
   But it looks good too to introduce property_token.
3. Unit tests need include '{Number}' etc.

[hongzhidao]

@xeioex @drsm Take a look, please.

It's an issue in njs.

var a = { Math   /* broken with NJS_TOKEN_LINE_END */
}
/*
Thrown:
SyntaxError: Unexpected token "}" in test.js:2
*/

The reason is that.

diff -r 2b75ee955589 src/njs_parser_terminal.c
--- a/src/njs_parser_terminal.c Mon Jan 20 13:24:09 2020 +0300
+++ b/src/njs_parser_terminal.c Wed Jan 29 05:08:16 2020 +0800
@@ -314,7 +314,7 @@
     njs_int_t              ret, __proto__;
     njs_str_t              name;
     njs_bool_t             computed, proto_init;
-    njs_token_t            token, accessor;
+    njs_token_t            token, prev_token, accessor;
     njs_lexer_t            *lexer;
     njs_parser_node_t      *object, *property, *expression;
     njs_function_lambda_t  *lambda;
@@ -347,7 +347,9 @@
         proto_init = 0;
         njs_memzero(&name, sizeof(njs_str_t));

-        if (token == NJS_TOKEN_NAME || lexer->keyword) {
+        if (token == NJS_TOKEN_NAME) {
+            prev_token = token;
+
             name = *njs_parser_text(parser);
             hash = njs_parser_key_hash(parser);
             token_line = njs_parser_token_line(parser);
@@ -487,13 +489,13 @@
             case NJS_TOKEN_CLOSE_BRACE:

                 if (name.length == 0
-                    || lexer->prev_token == NJS_TOKEN_THIS
-                    || lexer->prev_token == NJS_TOKEN_GLOBAL_OBJECT)
+                    || lexer->prev_token == NJS_TOKEN_THIS  /* wrong */ (ii)
+                    || lexer->prev_token == NJS_TOKEN_GLOBAL_OBJECT /* wrong */) (iii)
                 {
                     return NJS_TOKEN_ILLEGAL;
                 }

                 /* now, lexer->prev_token is NJS_TOKEN_LINE_END, but we hope it's NJS_TOKEN_NAME. */
-                expression = njs_parser_reference(vm, parser, lexer->prev_token,
+                expression = njs_parser_reference(vm, parser, prev_token,
                                                   &name, hash, token_line);
                 if (njs_slow_path(expression == NULL)) {
                     return NJS_TOKEN_ERROR;

After global object refactoring, I'm not sure the patch is good enough.

1. See (i), is lexer->keyword unnecessary?
2. See (ii, iii), I think we need to check all the places including lexer->prev_token.
   lexer->prev_token may be NJS_TOKEN_LINE_END.

[hongzhidao]

BTW, I see many modules in nodejs with this format return.

module["exports"] = ...

It's also mentioned in njs document.
http://nginx.org/en/docs/njs/node_modules.html

But it seems it's not a standard usage in JS language.
What's your thought/plan with working with nodejs? @xeioex

[lexborisov]

Hi @hongzhidao

Thanks for the patches, but at the moment this is not relevant since I completely rewrite the tokenizer and parser in the project.

[hongzhidao]

@lexborisov

I completely rewrite the tokenizer and parser in the project.

Why? And is it ready to preview?

[lexborisov]

@hongzhidao

We just have nothing to do 😄 (just a joke)

The current implementation does not allow us to expand the capabilities of the engine. More precisely, it requires a lot of work.

Plus, having gained experience in developing the JS engine (njs), we know better how the parser should look.

[hongzhidao]

Sounds great. I totally agree to do such refactoring.
Tell me if there is something I can help with.