Javascript: multiple prototype assignments

[jafl]

No description provided.

[codecov]

Codecov Report

Patch coverage: 100.00% and project coverage change: +0.01 🎉

Comparison is base (2f57104) 83.03% compared to head (461905b) 83.04%.

Additional details and impacted files

@@            Coverage Diff             @@
##           master    #3770      +/-   ##
==========================================
+ Coverage   83.03%   83.04%   +0.01%     
==========================================
  Files         227      227              
  Lines       55135    55175      +40     
==========================================
+ Hits        45779    45819      +40     
  Misses       9356     9356              

Impacted Files	Coverage Δ
parsers/jscript.c	97.40% <100.00%> (+0.07%)	⬆️

... and 1 file with indirect coverage changes

☔ View full report in Codecov by Sentry.
📢 Do you have feedback about the report comment? Let us know in this issue.

[masatake]

Instead of adjusting a parser to vString API, you can add a function to vString API.
extra-cmds/utiltest.c is for writing a test case.

[jafl]

@masatake Thanks for all the feedback! I figured I was missing some useful stuff, e.g., ptrArray.

[masatake]

I don't know JavaScript well but...

$ /tmp/foo.js 
B.prototype.m5 = () => {};
$ node /tmp/foo.js 
/tmp/foo.js:1
B.prototype.m5 = () => {};
^

ReferenceError: B is not defined
    at Object.<anonymous> (/tmp/foo.js:1:1)
    at Module._compile (node:internal/modules/cjs/loader:1254:14)
    at Module._extensions..js (node:internal/modules/cjs/loader:1308:10)
    at Module.load (node:internal/modules/cjs/loader:1117:32)
    at Module._load (node:internal/modules/cjs/loader:958:12)
    at Function.executeUserEntryPoint [as runMain] (node:internal/modules/run_main:81:12)
    at node:internal/main/run_main_module:23:47

Node.js v18.16.0

The error tells us B in B.prototype.m5 = () => {}; is not a definition but it is a reference to its definition. In such case, ctags should not make a definition tag for B.

This is very unlike variables in a shell script:

source something.sh
X=0

It is difficult to know whether X is a definition or reference (with left side role).

However, you want to make a tag for B. You need it to make a scope for m5.
Understandable.

The nameChain I introduced in 19f653c provides a solution.
With it, you can make reference tags for a, and b in a.b.c easily.
The nameChain helps you to make a cork Index for b. So you can use the index for filling the scope field of c.

However, we must add some new code for handling prototype in nameChain as a special case.

Here is my proposal:

• Delete B from the expected.tags.
• Pass deleteToken to the ptrArrayNew. The passed function is called to free elements in the array. So you can reduce the code for deleting tokens in the array manually.
• Don't make a definition tag for a class in a name chain. As the side effect, I deleted some calls of vString APIs.
• Skip "prototype" in makeJsRefTagsForNameChain().

Comparing a given token with "prototype" twice is a quirk. However, making parsePrototype short is more important.

Your code does strstr while iterating the tokens in the array.
However, the strstr can be done in the stage adding tokens to the array.

index 82724eb97..a5d13ced4 100644
--- a/Units/parser-javascript.r/js-scope.d/expected.tags
+++ b/Units/parser-javascript.r/js-scope.d/expected.tags
@@ -1,5 +1,4 @@
 A      input.js        /^function A() {$/;"    c
-B      input.js        /^B.prototype.m4 =$/;"  c
 m1     input.js        /^    m1 : function() {$/;"     m       class:A
 m2     input.js        /^    m2: function() {$/;"      m       class:A
 m3     input.js        /^A.prototype.m3 =$/;"  m       class:A
diff --git a/parsers/jscript.c b/parsers/jscript.c
index 4a59759d8..a880feac0 100644
--- a/parsers/jscript.c
+++ b/parsers/jscript.c
@@ -145,6 +145,7 @@ static tokenInfo *NextToken;
 static langType Lang_js;
 
 static objPool *TokenPool = NULL;
+static void deleteTokenFn(void *token) { deleteToken(token); }
 
 #ifdef HAVE_ICONV
 static iconv_t JSUnicodeConverter = (iconv_t) -2;
@@ -448,7 +449,16 @@ static int makeJsRefTagsForNameChain (char *name_chain, const tokenInfo *token,
        char *next = strchr(name_chain, '.');
        if (next)
                *next = '\0';
-       int index = bestJSEntryInScope (scope, name);
+
+       int index;
+       if (strcmp(name, "prototype") == 0)
+       {
+               /* Let's skip this link in the chain. */
+               index = scope;
+               goto out;
+       }
+       else
+               index = bestJSEntryInScope (scope, name);
 
        if (index == CORK_NIL)
        {
@@ -478,6 +488,7 @@ static int makeJsRefTagsForNameChain (char *name_chain, const tokenInfo *token,
                        registerEntry (index);
        }
 
+ out:
        return next
                ? makeJsRefTagsForNameChain (next + 1, token, leaf_kind, index)
                : index;
@@ -919,7 +930,7 @@ static void parseRegExp (void)
  * "name".
  */
 
-static bool include_period_in_identifier = 0;
+static int include_period_in_identifier = 0;
 
 static void accept_period_in_identifier(bool incl)
 {
@@ -2426,7 +2437,7 @@ static bool parsePrototype (tokenInfo *const name, tokenInfo *const token, state
                                                ! isType (identifier_token, TOKEN_UNDEFINED))
                                        {
                                                if (prototype_tokens == NULL)
-                                                       prototype_tokens = ptrArrayNew (NULL);
+                                                       prototype_tokens = ptrArrayNew (deleteTokenFn);
 
                                                tokenInfo *const save_token = newToken ();
                                                copyToken (save_token, identifier_token, true);
@@ -2450,20 +2461,8 @@ static bool parsePrototype (tokenInfo *const name, tokenInfo *const token, state
                                {
                                        identifier_token = ptrArrayItem (prototype_tokens, i);
                                        char * s1 = vStringValue (identifier_token->string);
-                                       char * s2 = strstr(s1, ".prototype.");
-                                       if (s2 != NULL)
-                                       {
-                                               s1[ s2-s1 ] = 0;        // terminate before "prototype"
-                                               vStringSetLength (identifier_token->string);
-                                               identifier_token->scope = makeClassTag (identifier_token, NULL, NULL);
-
-                                               s1[ s2-s1 ] = '.';
-                                               vStringSetLength (identifier_token->string);
-                                               vStringTruncateLeading (identifier_token->string, s2+11-s1);    // after ".prototype."
-
+                                       if (strstr(s1, ".prototype."))
                                                makeJsTag (identifier_token, JSTAG_METHOD, signature, NULL);
-                                       }
-                                       deleteToken (identifier_token);
                                }
                                ptrArrayUnref (prototype_tokens);
                        }

[jafl]

Yes! Much better. However, I can't pass deleteTokenFn to ptrArrayNew because it's a macro, not a function.

[masatake]

deleteToken is a macro but deleteTokenFn is a function.

+static void deleteTokenFn(void *token) { deleteToken(token); }

So you can pass it to ptrArrayNew.

[masatake]

Thank you for updating. After re-reading your change, I found we can reuse your original idea of the string manipulation earlier.

					if ( isType (method_body_token, TOKEN_EQUAL_SIGN) &&
						! isType (identifier_token, TOKEN_UNDEFINED))
					{
						char *str = vStringValue (identifier_token->string);
						char *needle = strstr(str, ".prototype.")

						if (prototype_tokens == NULL)
							prototype_tokens = ptrArrayNew (deleteTokenFn);

						tokenInfo *const save_token = newToken ();
						copyToken (save_token, identifier_token, true);
						/* Keep \1 of #/(.*)\.prototype\.(.*)/. */
						vStringTruncate (save_token->strng, needle - str);
						vStringPut (save_token->strng, '.');
						/* Append \2 of #/(.*)\.prototype\.(.*)/. */						
						vStringNCatS (save_token->strng, needle + 11);
						ptrArrayAdd (prototype_tokens, save_token);
						identifier_token->type = TOKEN_UNDEFINED;
					}

With this code, we no longer need the change for makeJsRefTagsForNameChain() I proposed.
We can reduce redundant string comparisons. However, the new optimized code makes parsePrototype() larger. Please, choose the combination (modifying makeJsRefTagsForNameChain and modifying parsePrototype) you like.

[jafl]

Definitely better to do the string processing in parsePrototype. I found an even simpler way to do it.

[masatake]

Thank you.