Skip to content

Commit 68edfad

Browse files
author
Kevin Smith
committed
[MERGE #6281 @zenparsing] Throw invalid LHS errors at parse time
Merge pull request #6281 from zenparsing:early-lhs-errors Fixes #6278 This PR sets the `fCanAssign` flag to false for array literals, object literals, `new` meta properties, `this`, and `super`. It relies on the caller to reset their `fCanAssign` flags to true if they end up parsing a destructuring assignment pattern.
2 parents 8595cce + 8d73ce8 commit 68edfad

File tree

3 files changed

+40
-14
lines changed

3 files changed

+40
-14
lines changed

lib/Parser/Parse.cpp

Lines changed: 9 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -3352,6 +3352,7 @@ ParseNodePtr Parser::ParseTerm(BOOL fAllowCall,
33523352

33533353
pid = ParseSuper<buildAST>(!!fAllowCall);
33543354
isSpecialName = true;
3355+
fCanAssign = FALSE;
33553356

33563357
// Super reference and super call need to push a pid ref to 'this' even when not building an AST
33573358
ReferenceSpecialName(wellKnownPropertyPids._this, ichMin, ichLim);
@@ -3376,6 +3377,7 @@ ParseNodePtr Parser::ParseTerm(BOOL fAllowCall,
33763377

33773378
this->GetScanner()->Scan();
33783379
isSpecialName = true;
3380+
fCanAssign = FALSE;
33793381

33803382
goto LIdentifier;
33813383

@@ -3581,6 +3583,7 @@ ParseNodePtr Parser::ParseTerm(BOOL fAllowCall,
35813583

35823584
this->GetScanner()->Scan();
35833585
isSpecialName = true;
3586+
fCanAssign = FALSE;
35843587

35853588
goto LIdentifier;
35863589
}
@@ -3617,14 +3620,11 @@ ParseNodePtr Parser::ParseTerm(BOOL fAllowCall,
36173620
this->m_funcInArrayDepth = 0;
36183621
}
36193622
ChkCurTok(tkRBrack, ERRnoRbrack);
3620-
if (!IsES6DestructuringEnabled())
3621-
{
3622-
fCanAssign = FALSE;
3623-
}
3624-
else if (pfLikelyPattern != nullptr && !IsPostFixOperators())
3623+
if (IsES6DestructuringEnabled() && pfLikelyPattern != nullptr && !IsPostFixOperators())
36253624
{
36263625
*pfLikelyPattern = TRUE;
36273626
}
3627+
fCanAssign = FALSE;
36283628
break;
36293629
}
36303630

@@ -3640,14 +3640,11 @@ ParseNodePtr Parser::ParseTerm(BOOL fAllowCall,
36403640
pnode->ichLim = this->GetScanner()->IchLimTok();
36413641
}
36423642
ChkCurTok(tkRCurly, ERRnoRcurly);
3643-
if (!IsES6DestructuringEnabled())
3644-
{
3645-
fCanAssign = FALSE;
3646-
}
3647-
else if (pfLikelyPattern != nullptr && !IsPostFixOperators())
3643+
if (IsES6DestructuringEnabled() && pfLikelyPattern != nullptr && !IsPostFixOperators())
36483644
{
36493645
*pfLikelyPattern = TRUE;
36503646
}
3647+
fCanAssign = FALSE;
36513648
break;
36523649
}
36533650

@@ -9055,6 +9052,7 @@ ParseNodePtr Parser::ParseExpr(int oplMin,
90559052
m_nextBlockId = parserState.m_nextBlockId;
90569053

90579054
ParseDestructuredLiteralWithScopeSave(tkLCurly, false/*isDecl*/, false /*topLevel*/, DIC_ShouldNotParseInitializer);
9055+
fCanAssign = TRUE;
90589056

90599057
// Restore the Block ID at the end of the reparsing so it matches the one at the end of the first pass. We need to do this
90609058
// because we don't parse initializers during reparse and there may be additional blocks (e.g. a class declaration)
@@ -10409,6 +10407,7 @@ ParseNodePtr Parser::ParseStatement()
1040910407
{
1041010408
this->GetScanner()->SeekTo(exprStart);
1041110409
ParseDestructuredLiteralWithScopeSave(tkNone, false/*isDecl*/, false /*topLevel*/, DIC_None, false /*allowIn*/);
10410+
fCanAssign = TRUE;
1041210411

1041310412
if (buildAST)
1041410413
{

test/Basics/SpecialSymbolCapture.js

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -215,6 +215,26 @@ var tests = [
215215
`);
216216
}
217217
},
218+
{
219+
name: "Special names are invalid assignment targets",
220+
body: function () {
221+
assert.throws(() => eval(`
222+
function f() {
223+
if (0) new.target = 1;
224+
}
225+
`), ReferenceError, "", "Invalid left-hand side in assignment");
226+
assert.throws(() => eval(`
227+
function f() {
228+
if (0) this = 1;
229+
}
230+
`), ReferenceError, "", "Invalid left-hand side in assignment");
231+
assert.throws(() => eval(`
232+
function f() {
233+
if (0) super = 1;
234+
}
235+
`), SyntaxError, "", "Invalid use of the 'super' keyword");
236+
}
237+
},
218238
{
219239
name: "Global-scope `new.target` keyword is early syntax error",
220240
body: function () {

test/es6/destructuring_bugs.js

Lines changed: 11 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -367,10 +367,6 @@ var tests = [
367367
{
368368
name: "Destructuring pattern at param has nested blocks.",
369369
body: function () {
370-
assert.doesNotThrow(function () { eval("var e = 1; ( {abcdef = ((((({})) = (1))))} = (e)) => { try{ } catch(e) {}}") }, "Should not throw when the default value and it's initializer both have extra parentheses.");
371-
372-
assert.doesNotThrow(function () { eval("var e = 1; ( {ghijkl = ((((({})) = 1 )))} = (e)) => { try{ } catch(e) {}}") }, "Should not throw when the default value has extra parentheses.");
373-
374370
assert.doesNotThrow(function () { eval("var e = 1; ( {mnopqrs = ((( {} = (1))))} = (e)) => { try{ } catch(e) {}}") }, "Should not throw when the default value initializer has extra parentheses.");
375371

376372
assert.doesNotThrow(function () { eval("var e = 1; ( {tuvwxy = ((( {} = 1 )))} = (e)) => { try{ } catch(e) {}}") }, "Should not throw when the default value and initializer are wrapped in extra parentheses.");
@@ -509,6 +505,17 @@ var tests = [
509505
test1();
510506
}
511507
},
508+
{
509+
name: "Invalid destructuring patterns should be early errors",
510+
body: function () {
511+
assert.throws(function () {
512+
eval("if (false) { [11] += [1, 2, 3] }");
513+
}, ReferenceError, "", "Invalid left-hand side in assignment");
514+
assert.throws(function () {
515+
eval("if (false) { [11] = [1, 2, 3] }");
516+
}, SyntaxError, "", "Destructuring expressions can only have identifier references");
517+
}
518+
},
512519
{
513520
name: "Destructuring expression : Array expression instead of name ",
514521
body: function () {

0 commit comments

Comments
 (0)