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):
```
var{ c = {a: 1} } = { c: undefined };
var{ d = {a: 1} } = {d: 3};
var a = 1
var [x = {a: 2}, y] = [, 4];
var [x = [a, 2], y] = [, 4];
```
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.57143% with 9 lines in your changes missing coverage. Please review.

Project coverage is 85.92%. Comparing base (a484466) to head (617d290).

Files with missing lines	Patch %	Lines
parsers/jscript.c	93.57%	9 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##           master    #3435      +/-   ##
==========================================
+ Coverage   85.91%   85.92%   +0.01%     
==========================================
  Files         239      239              
  Lines       58843    58955     +112     
==========================================
+ Hits        50555    50659     +104     
- Misses       8288     8296       +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.

masatake · 2024-12-20T09:08:04Z

@jafl, can I merge this?

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]>

A computed object property, surrounded by [ and ], should not be extracted as variable. 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 force-pushed the js--destructural-binding branch from 68ee390 to 501e14b Compare March 9, 2024 01:28

jafl approved these changes Dec 20, 2024

View reviewed changes

masatake changed the title ~~[WIP] JavaScript: destructuring binding~~ JavaScript: destructuring binding Dec 21, 2024

masatake force-pushed the js--destructural-binding branch from 501e14b to cf7d4ab Compare December 23, 2024 16:07

masatake added 6 commits December 24, 2024 11:13

JavaScript: extract variables and constants in destructuring assignment

9b613f9

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

JavaScript: add more trace code

4a9650b

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

JavaScript: don't analyze inside parameter lists

1e10189

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

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

4c649c2

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

c954d39

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

JavaScript: recognize "Computed object property names and destructuring"

617d290

A computed object property, surrounded by [ and ], should not be extracted as variable. Signed-off-by: Masatake YAMATO <[email protected]>

masatake force-pushed the js--destructural-binding branch from cf7d4ab to 617d290 Compare December 24, 2024 02:46

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

JavaScript: destructuring binding #3435

JavaScript: destructuring binding #3435

masatake commented Jul 9, 2022 •

edited

Loading

codecov bot commented Jul 10, 2022 •

edited

Loading

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

Loading

masatake commented Jan 12, 2023 •

edited

Loading

masatake commented Jan 12, 2023 •

edited

Loading

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

Loading

AdrienGiboire commented Nov 16, 2023

masatake commented Nov 16, 2023 •

edited

Loading

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 Dec 20, 2024

JavaScript: destructuring binding #3435

Are you sure you want to change the base?

JavaScript: destructuring binding #3435

Conversation

masatake commented Jul 9, 2022 • edited Loading

codecov bot commented Jul 10, 2022 • edited Loading

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 Loading

masatake commented Jan 12, 2023 • edited Loading

masatake commented Jan 12, 2023 • edited Loading

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 Loading

AdrienGiboire commented Nov 16, 2023

masatake commented Nov 16, 2023 • edited Loading

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 Dec 20, 2024

masatake commented Jul 9, 2022 •

edited

Loading

codecov bot commented Jul 10, 2022 •

edited

Loading

AdrienGiboire commented Jan 12, 2023 •

edited

Loading

masatake commented Jan 12, 2023 •

edited

Loading

masatake commented Jan 12, 2023 •

edited

Loading

masatake commented Nov 16, 2023 •

edited

Loading

masatake commented Nov 16, 2023 •

edited

Loading