[WIP] JavaScript: destructuring binding #3435

masatake · 2022-07-09T19:34:14Z

Limitations:

If an object literal is specified as a default value in object restructuring, the parser may fail to extract the variable (or constant):
```
const {var = {a: 1}};
```
I will try to remove this limitation.

key in [key] is extracted unexpectedly.

let key = 'z';
let {[key]: foo} = {z: 'bar'};

the parser fills the signature field for userDisplayName with a wrong value.
```
function userDisplayName({displayName: dname}) {
   return dname;
}
```
f is not tagged with function kind.
```
 let [f] = [function() {}];
```
I will not try to fix this. I cannot find any easy way to remove this limitation.

ref. https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Operators/Destructuring_assignment

codecov · 2022-07-10T01:03:45Z

Codecov Report

Attention: Patch coverage is 93.28358% with 9 lines in your changes are missing coverage. Please review.

Project coverage is 85.38%. Comparing base (38fd8e3) to head (501e14b).

Files	Patch %	Lines
parsers/jscript.c	93.28%	9 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##           master    #3435      +/-   ##
==========================================
+ Coverage   85.37%   85.38%   +0.01%     
==========================================
  Files         235      235              
  Lines       56458    56565     +107     
==========================================
+ Hits        48200    48299      +99     
- Misses       8258     8266       +8

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

masatake · 2022-08-26T17:52:44Z

Rebased on the latest master branch.
@jafl, this pull request still has some TODO items. But I would like to get your review.

jafl · 2022-08-26T19:45:08Z

@masatake Thanks for rebasing! I have had this one on my todo list for a while. I will try to get to it soon.

AdrienGiboire · 2023-01-09T15:18:21Z

It seems to be working to a limit.

Deconstruction in a multilines block of code fails:

files.push({
  relative, // <----
  basename: name,
  absolute: loc,
  mtime: +stat.mtime
});

I don't know if it is related but the following form fails as well:

computed: {
  ...mapGetters([
    'getAsylumAssistant',
    'getModel',
    'getType',
  ]),
},

I thought these forms would work but apparently not:

this.stagedlocalRecord = { localRecord, localRecordIndex }
// ---
return this.saveData({ partyId, id, data, rubyType: dbTypes[type], jsType: type })
// ---
createDocketEntryModel ({ commit, state }, requestData) {
  // ...
}

And there might be more but it takes too long for me to check all the warnings 😅

masatake · 2023-01-11T23:45:39Z

@AdrienGiboire thank you.

files.push({
  relative, // <----

I don't understand this. Is relative a variable?
Is there any document that describes this syntax?

I cannot find the notation in https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Operators/Destructuring_assignment.

jafl · 2023-01-12T01:15:31Z

files.push({
  relative, // <----

This assumes that relative is a variable, and it is shorthand for

files.push({
  relative: relative,

masatake · 2023-01-12T03:58:16Z

@jafl Thank you. So the question is, in which context can we use the shorthand syntax?

AdrienGiboire · 2023-01-12T05:24:33Z

Here it describes the case you're wondering about, especially this part:

Function parameter definitions

As developers, we can often expose more ergonomic APIs by accepting a single object with multiple properties as a parameter instead of forcing our API consumers to remember the order of many individual parameters. We can use destructuring to avoid repeating this single parameter object whenever we want to reference one of its properties:
function removeBreakpoint({ url, line, column }) {
  // ...
}

Knowing that it should support default values.

jQuery.ajax = function (url, {
  async = true,
  beforeSend = noop,
  cache = true,
  complete = noop,
  crossDomain = false,
  global = true,
  // ... more config
}) {
  // ... do stuff
};

Basically, if a variable is between {} and there is no explicit key, it's meant to be desconstructed.
If an assignment is between {}, it's meant to be desconstructed and have a default value.

masatake · 2023-01-12T21:47:05Z

@AdrienGiboire Thank you.

function removeBreakpoint({ url, line, column }) {
  // ...
}

I found even this input causes the warning message.
I found we can skip such parameter lists because the JavaScript parser doesn't extract parameters as tags. I added a commit for this change.

files.push({
  relative, // <----
  basename: name,
  absolute: loc,
  mtime: +stat.mtime
});

It seems that we must improve skipArgumentList(). I will inspect this one more.

masatake · 2023-01-12T23:35:49Z

$ cat /tmp/foo.js                                                                                                                                     
cat /tmp/foo.js                                                                                                                                       
computed: {                                                                                                                                           
  ...mapGetters([                                                                                                                                     
    'getAsylumAssistant',                                                                                                                             
    'getModel',                                                                                                                                       
    'getType',                                                                                                                                        
  ]),                                                                                                                                                 
},                                                                                                                                                    
$ node foo.js                                                                                                                                         
node foo.js                                                                                                                                           
node:internal/modules/cjs/loader:998                                                                                                                  
  throw err;                                                                                                                                          
  ^                                                                                                                                                   
                                                                                                                                                      
Error: Cannot find module '/home/jet/var/ctags-github/foo.js'                                                                                         
    at Module._resolveFilename (node:internal/modules/cjs/loader:995:15)                                                                              
    at Module._load (node:internal/modules/cjs/loader:841:27)                                                                                         
    at Function.executeUserEntryPoint [as runMain] (node:internal/modules/run_main:81:12)                                                             
    at node:internal/main/run_main_module:23:47 {                                                                                                     
  code: 'MODULE_NOT_FOUND',                                                                                                                           
  requireStack: []                                                                                                                                    
}                                                                                                                                                     
                                                                                                                                                      
Node.js v18.12.1

Could you provide an example input that node accepts?
Thank you.

AdrienGiboire · 2023-11-16T10:04:13Z

$ cat /tmp/foo.js                                                                                                                                     
cat /tmp/foo.js                                                                                                                                       
computed: {                                                                                                                                           
  ...mapGetters([                                                                                                                                     
    'getAsylumAssistant',                                                                                                                             
    'getModel',                                                                                                                                       
    'getType',                                                                                                                                        
  ]),                                                                                                                                                 
},                                                                                                                                                    
$ node foo.js                                                                                                                                         
node foo.js                                                                                                                                           
node:internal/modules/cjs/loader:998                                                                                                                  
  throw err;                                                                                                                                          
  ^                                                                                                                                                   
                                                                                                                                                      
Error: Cannot find module '/home/jet/var/ctags-github/foo.js'                                                                                         
    at Module._resolveFilename (node:internal/modules/cjs/loader:995:15)                                                                              
    at Module._load (node:internal/modules/cjs/loader:841:27)                                                                                         
    at Function.executeUserEntryPoint [as runMain] (node:internal/modules/run_main:81:12)                                                             
    at node:internal/main/run_main_module:23:47 {                                                                                                     
  code: 'MODULE_NOT_FOUND',                                                                                                                           
  requireStack: []                                                                                                                                    
}                                                                                                                                                     
                                                                                                                                                      
Node.js v18.12.1

Could you provide an example input that node accepts? Thank you.

Did you mean to run node /tmp/foo.js instead?

masatake · 2023-11-16T19:12:22Z

Did you mean to run node /tmp/foo.js instead?

You are correct. However, even though I specified /tmp/foo.js, I got an error.

$ cat /tmp/foo.js 
computed: {                                                                                                                                           
  ...mapGetters([                                                                                                                                     
    'getAsylumAssistant',                                                                                                                             
    'getModel',                                                                                                                                       
    'getType',                                                                                                                                        
  ]),                                                                                                                                                 
},
$ node /tmp/foo.js 
/tmp/foo.js:2
  ...mapGetters([                                                                                                                                     
  ^^^

SyntaxError: Unexpected token '...'
    at internalCompileFunction (node:internal/vm:73:18)
    at wrapSafe (node:internal/modules/cjs/loader:1178:20)
    at Module._compile (node:internal/modules/cjs/loader:1220:27)
    at Module._extensions..js (node:internal/modules/cjs/loader:1310:10)
    at Module.load (node:internal/modules/cjs/loader:1119:32)
    at Module._load (node:internal/modules/cjs/loader:960:12)
    at Function.executeUserEntryPoint [as runMain] (node:internal/modules/run_main:86:12)
    at node:internal/main/run_main_module:23:47

Node.js v18.18.2

AdrienGiboire · 2023-11-16T19:38:45Z

You can try something like this:

const embedded = { foo: 'bar' }
const something = {
  ...embedded,
  baz: 'fox',
}

masatake · 2023-11-16T19:52:03Z

Thank you. Based on your hit, I modified /tmp/foo.js:

$ cat /tmp/foo.js
const mapGetters = function() {}
const computed = {                                                                                                                                           
  ...mapGetters([                                                                                                                                     
    'getAsylumAssistant',                                                                                                                             
    'getModel',                                                                                                                                       
    'getType',                                                                                                                                        
  ]),                                                                                                                                                 
}
$ node /tmp/foo.js
$

Works fine. Thank you.

[yamato@dev64]~/var/ctags-github% git rebase master
Successfully rebased and updated refs/heads/js--destructural-binding.

[yamato@dev64]~/var/ctags-github% make
...
[yamato@dev64]~/var/ctags-github% ./ctags -o - /tmp/foo.js 
ctags: Notice: ignoring null tag in /tmp/foo.js(line: 3, language: JavaScript)
computed	/tmp/foo.js	/^const computed = {                                                                              /;"	C
mapGetters	/tmp/foo.js	/^const mapGetters = function() {}$/;"	f

I cannot remember what we were talking about quickly. But I guess the issue is about ignoring null tag warning.

masatake · 2023-11-16T19:58:49Z

[yamato@dev64]~/var/ctags-github% cat /tmp/bar.js         
const embedded = { foo: 'bar' }
const something = {
  ...embedded,
  baz: 'fox',
}
[yamato@dev64]~/var/ctags-github% ./ctags -o - /tmp/bar.js
ctags: Notice: ignoring null tag in /tmp/bar.js(line: 3, language: JavaScript)
embedded	/tmp/bar.js	/^const embedded = { foo: 'bar' }$/;"	v
foo	/tmp/bar.js	/^const embedded = { foo: 'bar' }$/;"	p	variable:embedded
something	/tmp/bar.js	/^const something = {$/;"	C

I found two issues here:

the null tag warning
Though foo in embedded is extracted, baz in something is not.

embedded in something is not a definition, so we don't have to extract it as a definition tag. Instead, THe parser should skip the ...embedded.

https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Operators/Spread_syntax

AdrienGiboire · 2023-11-16T20:45:51Z

It makes sense but at the same time would it not be legit to have the definition?

For example, you can have something like:

const something = {
  fn: () => { /* doing something */ },
}

It would make sense to have the definition of the function referenced. What do you think?

masatake · 2023-11-16T20:54:10Z

This is a difficult question. However, regarding the example, fn should be tagged as a definition tag. fn is introduced as a new name.

const embedded = { foo: 'bar' }
const something = {
  ...embedded,
  baz: 'fox',
}

In my understanding, this is the same as:

const something = {
  foo: 'bar',
  baz: 'fox',
}

Am I correct?

The embedded in something is not a key.
So embedded should not be tagged as a definition tag.
It is a reference tag having shredded role of variable (or unknown) kind.

Added after commenting

[yamato@dev64]~/var/ctags-github% cat /tmp/bar.js 
const embedded = { foo: 'bar' }
const something = {
  ...embedded,
  baz: 'fox',
}

something.foo;

[yamato@dev64]~/var/ctags-github% node /tmp/bar.js

My understanding may be correct.

masatake · 2023-11-16T20:57:41Z

[yamato@dev64]~/var/ctags-github% git diff                
diff --git a/parsers/jscript.c b/parsers/jscript.c
index 78f563dde..f316d7f7d 100644
--- a/parsers/jscript.c
+++ b/parsers/jscript.c
@@ -2019,6 +2019,12 @@ static bool parseMethods (tokenInfo *const token, int class_index,
 
            deleteToken (saved_token);
        }
+       else if (isType (token, TOKEN_DOTS))
+       {
+           /* maybe spread operator. Just skip the next expression. */
+           findCmdTerm(token, true, true);
+           continue;
+       }
 
        if (! isType (token, TOKEN_KEYWORD) &&
            ! isType (token, TOKEN_SEMICOLON))
[yamato@dev64]~/var/ctags-github% cat /tmp/bar.js 
const embedded = { foo: 'bar' }
const something = {
  ...embedded,
  baz: 'fox',
}
[yamato@dev64]~/var/ctags-github% ./ctags --sort=no -o - /tmp/bar.js
foo	/tmp/bar.js	/^const embedded = { foo: 'bar' }$/;"	p	variable:embedded
embedded	/tmp/bar.js	/^const embedded = { foo: 'bar' }$/;"	v
baz	/tmp/bar.js	/^  baz: 'fox',$/;"	p	variable:something
something	/tmp/bar.js	/^const something = {$/;"	v

masatake · 2023-11-16T21:04:50Z

Surprisingly, the change for skipping spread operators works well with the current master branch. So I can submit it as a standalone pull request.

Related to universal-ctags#3435. Signed-off-by: Masatake YAMATO <[email protected]>

masatake · 2023-11-16T21:50:01Z

https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Operators/Spread_syntax

This tells spread syntax.
I'm looking for the spread operator, ",@" in lisp..

AdrienGiboire · 2023-11-17T09:04:31Z

const embedded = { foo: 'bar' }
const something = {
  ...embedded,
  baz: 'fox',
}

In my understanding, this is the same as:

const something = {
  foo: 'bar',
  baz: 'fox',
}

Am I correct?

Pretty much, yes.

Surprisingly, the change for skipping spread operators works well with the current master branch. So I can submit it as a standalone pull request.

It does look good 👍

https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Operators/Spread_syntax

This tells spread syntax. I'm looking for the spread operator, ",@" in lisp..

I'm not sure I understand what you mean. And I know nothing about Lisp so I can't relate ^^' I tried googling but I have not seen ,@ 🤷‍♂️

masatake · 2023-11-17T19:04:49Z

@AdrienGiboire Sorry, after rereading the page at developer.mozilla.org, "spread syntax" is the correct word.

With this change, the parser skip "..." and the subsequent expression. ref. https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Operators/Spread_syntax Related to universal-ctags#3435. Signed-off-by: Masatake YAMATO <[email protected]>

masatake · 2023-12-09T18:37:34Z

@jafl, can I merge this?

masatake · 2023-12-22T20:26:11Z

.b test cases must be added.

Close universal-ctags#1112. Signed-off-by: Masatake YAMATO <[email protected]>

Signed-off-by: Masatake YAMATO <[email protected]>

… name: name, ...} Signed-off-by: Masatake YAMATO <[email protected]>

masatake force-pushed the js--destructural-binding branch from 2edf82b to 732de02 Compare July 10, 2022 00:57

masatake mentioned this pull request Jul 12, 2022

javascript: improved parsing of anonymous functions #3437

Merged

masatake force-pushed the js--destructural-binding branch from 732de02 to ec3b205 Compare August 26, 2022 17:50

masatake requested a review from jafl August 26, 2022 17:52

masatake added the Parser bug/enhancement label Aug 26, 2022

masatake removed the Parser bug/enhancement label Nov 29, 2022

masatake mentioned this pull request Jan 5, 2023

javascript: Got "ctags: Warning: ignoring null tag" #900

Closed

masatake force-pushed the js--destructural-binding branch from ec3b205 to 3b52348 Compare January 12, 2023 21:35

masatake added a commit to masatake/ctags that referenced this pull request Nov 16, 2023

JavaScript: skip spread operators

18fb843

Related to universal-ctags#3435. Signed-off-by: Masatake YAMATO <[email protected]>

masatake added a commit to masatake/ctags that referenced this pull request Nov 16, 2023

JavaScript: skip spread operators

f07356d

Related to universal-ctags#3435. Signed-off-by: Masatake YAMATO <[email protected]>

masatake mentioned this pull request Nov 16, 2023

JavaScript: handle spread syntax #3846

Merged

masatake force-pushed the js--destructural-binding branch from 343d606 to 68ee390 Compare November 17, 2023 23:21

masatake added this to the 6.2 milestone Dec 28, 2023

masatake added the next-release label Dec 28, 2023

masatake modified the milestones: 6.2, 6.3 Feb 24, 2024

masatake removed the next-release label Feb 24, 2024

masatake added 5 commits March 9, 2024 10:28

JavaScript: extract variables and constants in destructuring assignment

b1a1fc0

Close universal-ctags#1112. Signed-off-by: Masatake YAMATO <[email protected]>

JavaScript: add more trace code

85ba307

Signed-off-by: Masatake YAMATO <[email protected]>

JavaScript: don't analyze inside parameter lists

11d956c

Signed-off-by: Masatake YAMATO <[email protected]>

JavaScript: make trace output easier to read by adding quote characters

d688a83

JavaScript: extract name in { ..., name, ...}, a shorthand for { ...,…

501e14b

… name: name, ...} Signed-off-by: Masatake YAMATO <[email protected]>

masatake force-pushed the js--destructural-binding branch from 68ee390 to 501e14b Compare March 9, 2024 01:28

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[WIP] JavaScript: destructuring binding #3435

[WIP] JavaScript: destructuring binding #3435

masatake commented Jul 9, 2022 •

edited by dtikhonov

codecov bot commented Jul 10, 2022 •

edited

masatake commented Aug 26, 2022

jafl commented Aug 26, 2022

AdrienGiboire commented Jan 9, 2023

masatake commented Jan 11, 2023

jafl commented Jan 12, 2023

masatake commented Jan 12, 2023

AdrienGiboire commented Jan 12, 2023 •

edited

masatake commented Jan 12, 2023 •

edited

masatake commented Jan 12, 2023 •

edited

AdrienGiboire commented Nov 16, 2023

masatake commented Nov 16, 2023

AdrienGiboire commented Nov 16, 2023

masatake commented Nov 16, 2023

masatake commented Nov 16, 2023 •

edited

AdrienGiboire commented Nov 16, 2023

masatake commented Nov 16, 2023 •

edited

masatake commented Nov 16, 2023

masatake commented Nov 16, 2023

masatake commented Nov 16, 2023

AdrienGiboire commented Nov 17, 2023

masatake commented Nov 17, 2023

masatake commented Dec 9, 2023

masatake commented Dec 22, 2023

[WIP] JavaScript: destructuring binding #3435

Are you sure you want to change the base?

[WIP] JavaScript: destructuring binding #3435

Conversation

masatake commented Jul 9, 2022 • edited by dtikhonov

codecov bot commented Jul 10, 2022 • edited

Codecov Report

masatake commented Aug 26, 2022

jafl commented Aug 26, 2022

AdrienGiboire commented Jan 9, 2023

masatake commented Jan 11, 2023

jafl commented Jan 12, 2023

masatake commented Jan 12, 2023

AdrienGiboire commented Jan 12, 2023 • edited

masatake commented Jan 12, 2023 • edited

masatake commented Jan 12, 2023 • edited

AdrienGiboire commented Nov 16, 2023

masatake commented Nov 16, 2023

AdrienGiboire commented Nov 16, 2023

masatake commented Nov 16, 2023

masatake commented Nov 16, 2023 • edited

AdrienGiboire commented Nov 16, 2023

masatake commented Nov 16, 2023 • edited

masatake commented Nov 16, 2023

masatake commented Nov 16, 2023

masatake commented Nov 16, 2023

AdrienGiboire commented Nov 17, 2023

masatake commented Nov 17, 2023

masatake commented Dec 9, 2023

masatake commented Dec 22, 2023

masatake commented Jul 9, 2022 •

edited by dtikhonov

codecov bot commented Jul 10, 2022 •

edited

AdrienGiboire commented Jan 12, 2023 •

edited

masatake commented Jan 12, 2023 •

edited

masatake commented Jan 12, 2023 •

edited

masatake commented Nov 16, 2023 •

edited

masatake commented Nov 16, 2023 •

edited