Skip to content

Commit

Permalink
Disallow nested destructuring or binding
Browse files Browse the repository at this point in the history 
Per tc39/proposal-object-rest-spread#50
and tc39/proposal-object-rest-spread#51.
  • Loading branch information
@adrianheine
adrianheine committed Nov 4, 2017
1 parent 93c2f7f commit 6677deb
Show file tree
Hide file tree
Showing 2 changed files with 10 additions and 174 deletions.
8 changes: 7 additions & 1 deletion inject.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,13 @@ module.exports = function(acorn) {
if (this.options.ecmaVersion >= 6) {
// ...the spread logic borrowed from babylon :)
if (this.type === tt.ellipsis) {
prop = isPattern ? this.parseRestBinding() : this.parseSpread(refDestructuringErrors)
if (isPattern) {
this.next()
prop.argument = this.parseIdent()
this.finishNode(prop, "RestElement")
} else {
prop = this.parseSpread(refDestructuringErrors)
}
node.properties.push(prop)

This comment has been minimized.

Sign in to view

Copy link
@jdalton

jdalton Nov 17, 2017

I noticed here when adopting this plugin for my use that I ran into

TypeError: Cannot read property 'type' of undefined
      at Parser.module.exports.pp.checkLVal

If I changed the bit of code to

const prop = this.parseSpread(refDestructuringErrors)

  if (isPattern) {
    prop.type = "RestElement"
    prop.value = this.toAssignable(prop.argument)
  }

The issue was resolved. I think it comes down to the prop.argument = this.parseIdent() bit as the parseSpread method will do

node.argument = this.parseMaybeAssign(false, refDestructuringErrors)

which I didn't dig into further.

This comment has been minimized.

Sign in to view

Copy link
@adrianheine

adrianheine Nov 17, 2017

Author Owner

That's why I extended checkLVal.

This comment has been minimized.

Sign in to view

Copy link
@jdalton

jdalton Nov 17, 2017

The checkLVal looks like the unmodified form. What's different?

This comment has been minimized.

Sign in to view

Copy link
@adrianheine

adrianheine Nov 17, 2017

Author Owner

Right, I already pushed that upstream in acornjs/acorn#613, I forgot :)
if (this.type === tt.comma) {
if (isPattern) {
Expand Down
176 changes: 3 additions & 173 deletions test/tests-object-spread.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -540,178 +540,6 @@ var fbTestFixture = {
}
}
},
'let {...{x, y}} = {}': {
"type": "VariableDeclaration",
"start": 0,
"end": 20,
"declarations": [
{
"type": "VariableDeclarator",
"start": 4,
"end": 20,
"id": {
"type": "ObjectPattern",
"start": 4,
"end": 15,
"properties": [
{
"type": "RestElement",
"start": 5,
"end": 14,
"argument": {
"type": "ObjectPattern",
"start": 8,
"end": 14,
"properties": [
{
"type": "Property",
"start": 9,
"end": 10,
"method": false,
"shorthand": true,
"computed": false,
"key": {
"type": "Identifier",
"start": 9,
"end": 10,
"name": "x"
},
"kind": "init",
"value": {
"type": "Identifier",
"start": 9,
"end": 10,
"name": "x"
}
},
{
"type": "Property",
"start": 12,
"end": 13,
"method": false,
"shorthand": true,
"computed": false,
"key": {
"type": "Identifier",
"start": 12,
"end": 13,
"name": "y"
},
"kind": "init",
"value": {
"type": "Identifier",
"start": 12,
"end": 13,
"name": "y"
}
}
]
}
}
]
},
"init": {
"type": "ObjectExpression",
"start": 18,
"end": 20,
"properties": []
}
}
],
"kind": "let"
},
'let {...{...{x, y}}} = {}': {
"type": "VariableDeclaration",
"start": 0,
"end": 25,
"declarations": [
{
"type": "VariableDeclarator",
"start": 4,
"end": 25,
"id": {
"type": "ObjectPattern",
"start": 4,
"end": 20,
"properties": [
{
"type": "RestElement",
"start": 5,
"end": 19,
"argument": {
"type": "ObjectPattern",
"start": 8,
"end": 19,
"properties": [
{
"type": "RestElement",
"start": 9,
"end": 18,
"argument": {
"type": "ObjectPattern",
"start": 12,
"end": 18,
"properties": [
{
"type": "Property",
"start": 13,
"end": 14,
"method": false,
"shorthand": true,
"computed": false,
"key": {
"type": "Identifier",
"start": 13,
"end": 14,
"name": "x"
},
"kind": "init",
"value": {
"type": "Identifier",
"start": 13,
"end": 14,
"name": "x"
}
},
{
"type": "Property",
"start": 16,
"end": 17,
"method": false,
"shorthand": true,
"computed": false,
"key": {
"type": "Identifier",
"start": 16,
"end": 17,
"name": "y"
},
"kind": "init",
"value": {
"type": "Identifier",
"start": 16,
"end": 17,
"name": "y"
}
}
]
}
}
]
}
}
]
},
"init": {
"type": "ObjectExpression",
"start": 23,
"end": 25,
"properties": []
}
}
],
"kind": "let"
},
'const fn = ({text = "default", ...props}) => text + props.children': {
"type": "VariableDeclaration",
"start": 0,
Expand Down Expand Up @@ -834,10 +662,12 @@ if (typeof exports !== "undefined") {
testFail("({get x() {}}) => {}", "Object pattern can't contain getter or setter (1:6)")
testFail("let {...x, ...y} = {}", "Comma is not permitted after the rest element (1:9)")
testFail("({...x,}) => z", "Comma is not permitted after the rest element (1:6)")
testFail("({...{...x,}}) => z", "Comma is not permitted after the rest element (1:10)")
testFail("export const { foo, ...bar } = baz;\nexport const bar = 1;\n", "Identifier 'bar' has already been declared (2:13)", {
sourceType: "module"
})
testFail("function ({...x,}) { z }", "Unexpected token (1:9)")
testFail("let {...{x, y}} = {}", "Unexpected token (1:8)")
testFail("let {...{...{x, y}}} = {}", "Unexpected token (1:8)")

for (var ns in fbTestFixture) {
ns = fbTestFixture[ns];
Expand Down

0 comments on commit 6677deb

Please sign in to comment.