Skip to content
Permalink
Browse files

[MERGE #6347 @rhuanjl] Error improvements

Merge pull request #6347 from rhuanjl:parseErrorImprovements

This PR turns early reference errors into syntax errors per  tc39/ecma262#1527

And makes a few other error message improvements including fixing #6339

fix: #6339
  • Loading branch information
Kevin Smith
Kevin Smith committed Dec 6, 2019
2 parents 9dab549 + f2aad9f commit fd545b428de1a62e3de11962f76699a117bfc4df
@@ -2215,7 +2215,6 @@ void Parser::AddToNodeList(ParseNode ** ppnodeList, ParseNode *** pppnodeLast,
}
else
{
//
Assert(*ppnodeList);
Assert(**pppnodeLast);

@@ -2398,7 +2397,7 @@ void Parser::ParseNamedImportOrExportClause(ModuleImportOrExportEntryList* impor

if (!(m_token.IsIdentifier() || m_token.IsReservedWord()))
{
Error(ERRsyntax);
Error(ERRTokenAfter, GetTokenString(m_token.tk), GetTokenString(GetScanner()->GetPrevious()));
}

IdentPtr identifierName = m_token.GetIdentifier(this->GetHashTbl());
@@ -2412,7 +2411,7 @@ void Parser::ParseNamedImportOrExportClause(ModuleImportOrExportEntryList* impor
// We have the pattern "IdentifierName as"
if (!CheckContextualKeyword(wellKnownPropertyPids.as))
{
Error(ERRsyntax);
Error(ERRInvalidIdentifier, m_token.GetIdentifier(this->GetHashTbl())->Psz(), identifierName->Psz());
}

this->GetScanner()->Scan();
@@ -2437,7 +2436,7 @@ void Parser::ParseNamedImportOrExportClause(ModuleImportOrExportEntryList* impor
{
// If we are parsing an import statement and this ImportSpecifier clause did not have
// 'as ImportedBinding' at the end of it, identifierName must be a BindingIdentifier.
Error(ERRsyntax);
Error(ERRnoIdent);
}

if (m_token.tk == tkComma)
@@ -2460,7 +2459,7 @@ void Parser::ParseNamedImportOrExportClause(ModuleImportOrExportEntryList* impor
}

// Final token in a named import or export clause must be a '}'
ChkCurTokNoScan(tkRCurly, ERRsyntax);
ChkCurTokNoScan(tkRCurly, ERRnoRcurly);
}

IdentPtrList* Parser::GetRequestedModulesList()
@@ -2668,12 +2667,12 @@ void Parser::ParseImportClause(ModuleImportOrExportEntryList* importEntryList, b
this->GetScanner()->Scan();
if (!CheckContextualKeyword(wellKnownPropertyPids.as))
{
Error(ERRsyntax);
Error(ERRValidIfFollowedBy, _u("import *"), _u("as"));
}

// Token following 'as' must be a binding identifier.
this->GetScanner()->Scan();
ChkCurTokNoScan(tkID, ERRsyntax);
ChkCurTokNoScan(tkID, ERRnoIdent);

if (buildAST)
{
@@ -2688,7 +2687,7 @@ void Parser::ParseImportClause(ModuleImportOrExportEntryList* importEntryList, b
break;

default:
Error(ERRsyntax);
Error(ERRTokenAfter, GetTokenString(m_token.tk), GetTokenString(this->GetScanner()->GetPrevious()));
}

this->GetScanner()->Scan();
@@ -2708,17 +2707,22 @@ void Parser::ParseImportClause(ModuleImportOrExportEntryList* importEntryList, b
}
}

bool Parser::IsImportOrExportStatementValidHere()
void Parser::CheckIfImportOrExportStatementValidHere()
{
ParseNodeFnc * curFunc = GetCurrentFunctionNode();

// Import must be located in the top scope of the module body.
return curFunc->nop == knopFncDecl
&& curFunc->IsModule()
&& this->m_currentBlockInfo->pnodeBlock == curFunc->pnodeBodyScope
&& (this->m_grfscr & fscrEvalCode) != fscrEvalCode
&& this->m_tryCatchOrFinallyDepth == 0
&& !this->m_disallowImportExportStmt;
if (curFunc->nop != knopFncDecl || !curFunc->IsModule())
{
Error(ERRModuleImportOrExportInScript);
}

if (this->m_currentBlockInfo->pnodeBlock != curFunc->pnodeBodyScope
|| (this->m_grfscr & fscrEvalCode) == fscrEvalCode
|| this->m_tryCatchOrFinallyDepth != 0
|| this->m_disallowImportExportStmt)
{
Error(ERRInvalidModuleImportOrExport);
}
}

bool Parser::IsTopLevelModuleFunc()
@@ -2774,10 +2778,7 @@ ParseNodePtr Parser::ParseImport()

this->GetScanner()->SeekTo(parsedImport);

if (!IsImportOrExportStatementValidHere())
{
Error(ERRInvalidModuleImportOrExport);
}
CheckIfImportOrExportStatementValidHere();

// We just parsed an import token. Next valid token is *, {, string constant, or binding identifier.
this->GetScanner()->Scan();
@@ -2833,7 +2834,7 @@ IdentPtr Parser::ParseImportOrExportFromClause(bool throwIfNotFound)
this->GetScanner()->Scan();

// Token following the 'from' token must be a string constant - the module specifier.
ChkCurTokNoScan(tkStrCon, ERRsyntax);
ChkCurTokNoScan(tkStrCon, ERRValidIfFollowedBy, _u("'from'"), _u("a module specifier."));

if (buildAST)
{
@@ -2844,7 +2845,7 @@ IdentPtr Parser::ParseImportOrExportFromClause(bool throwIfNotFound)
}
else if (throwIfNotFound)
{
Error(ERRsyntax);
Error(ERRMissingFrom);
}

return moduleSpecifier;
@@ -2997,10 +2998,7 @@ ParseNodePtr Parser::ParseExportDeclaration(bool *needTerminator)
Assert(m_scriptContext->GetConfig()->IsES6ModuleEnabled());
Assert(m_token.tk == tkEXPORT);

if (!IsImportOrExportStatementValidHere())
{
Error(ERRInvalidModuleImportOrExport);
}
CheckIfImportOrExportStatementValidHere();

ParseNodePtr pnode = nullptr;
IdentPtr moduleIdentifier = nullptr;
@@ -8938,7 +8936,7 @@ ParseNodePtr Parser::ParseExpr(int oplMin,
? !PHASE_OFF_RAW(Js::EarlyReferenceErrorsPhase, m_sourceContextInfo->sourceContextId, GetCurrentFunctionNode()->functionId)
: !PHASE_OFF1(Js::EarlyReferenceErrorsPhase)))
{
Error(JSERR_CantAssignTo);
Error(ERRInvalidAsgTarget);
}
TrackAssignment<buildAST>(pnodeT, &operandToken);
if (buildAST)
@@ -9118,7 +9116,7 @@ ParseNodePtr Parser::ParseExpr(int oplMin,
? !PHASE_OFF_RAW(Js::EarlyReferenceErrorsPhase, m_sourceContextInfo->sourceContextId, GetCurrentFunctionNode()->functionId)
: !PHASE_OFF1(Js::EarlyReferenceErrorsPhase)))
{
Error(JSERR_CantAssignTo);
Error(ERRInvalidAsgTarget);
}
TrackAssignment<buildAST>(pnode, &term);
fCanAssign = FALSE;
@@ -9205,7 +9203,7 @@ ParseNodePtr Parser::ParseExpr(int oplMin,
? !PHASE_OFF_RAW(Js::EarlyReferenceErrorsPhase, m_sourceContextInfo->sourceContextId, GetCurrentFunctionNode()->functionId)
: !PHASE_OFF1(Js::EarlyReferenceErrorsPhase)))
{
Error(JSERR_CantAssignTo);
Error(ERRInvalidAsgTarget);
// No recovery necessary since this is a semantic, not structural, error.
}
}
@@ -975,7 +975,7 @@ class Parser
charcount_t ichMin,
_Out_opt_ BOOL* pfCanAssign = nullptr);

bool IsImportOrExportStatementValidHere();
void CheckIfImportOrExportStatementValidHere();
bool IsTopLevelModuleFunc();

template<bool buildAST> ParseNodePtr ParseImport();
@@ -96,7 +96,7 @@ LSC_ERROR_MSG(1084, ERRGetterMustHaveNoParameters, "Getter functions must have n

LSC_ERROR_MSG(1085, ERRInvalidUseofExponentiationOperator, "Invalid unary operator on the left hand side of exponentiation (**) operator")

LSC_ERROR_MSG(1086, ERRInvalidModuleImportOrExport, "Module import or export statement unexpected here")
LSC_ERROR_MSG(1086, ERRInvalidModuleImportOrExport, "'import' or 'export' can only occur at top level.")
LSC_ERROR_MSG(1087, ERRInvalidExportName, "Unable to resolve module export name")

LSC_ERROR_MSG(1088, ERRLetIDInLexicalDecl, "'let' is not an allowed identifier in lexical declarations")
@@ -113,7 +113,10 @@ LSC_ERROR_MSG(1097, ERRExperimental, "Use of disabled experimental feature")
LSC_ERROR_MSG(1098, ERRDuplicateExport, "Duplicate export of name '%s'")
LSC_ERROR_MSG(1099, ERRStmtOfWithIsLabelledFunc, "The statement of a 'with' statement cannot be a labelled function.")
LSC_ERROR_MSG(1100, ERRUndeclaredExportName, "Export of name '%s' which has no local definition.")
//1100-1199 available for future use
LSC_ERROR_MSG(1101, ERRModuleImportOrExportInScript, "'import' or 'export' can only be used in module code.")
LSC_ERROR_MSG(1102, ERRInvalidAsgTarget, "Invalid left-hand side in assignment.")
LSC_ERROR_MSG(1103, ERRMissingFrom, "Expected 'from' after import or export clause.")
//1104-1199 available for future use

// Generic errors intended to be re-usable
LSC_ERROR_MSG(1200, ERRKeywordAfter, "Unexpected keyword '%s' after '%s'")
@@ -222,12 +222,12 @@ var tests = [
function f() {
if (0) new.target = 1;
}
`), ReferenceError, "", "Invalid left-hand side in assignment");
`), SyntaxError, "", "Invalid left-hand side in assignment.");
assert.throws(() => eval(`
function f() {
if (0) this = 1;
}
`), ReferenceError, "", "Invalid left-hand side in assignment");
`), SyntaxError, "", "Invalid left-hand side in assignment.");
assert.throws(() => eval(`
function f() {
if (0) super = 1;
@@ -661,7 +661,7 @@ var tests = [
name: "Assignment to special symbols is invalid",
body: function() {
function test(code, message) {
assert.throws(() => WScript.LoadScript(code), ReferenceError, message, "Invalid left-hand side in assignment");
assert.throws(() => WScript.LoadScript(code), SyntaxError, message, "Invalid left-hand side in assignment.");
}

function testglobal(symname) {
@@ -16,4 +16,4 @@ Inner foobaz 2
Finally bar 2
Except foobaz 2 thrown
english (passed)
ReferenceError: Invalid left-hand side in assignment
SyntaxError: Invalid left-hand side in assignment.
@@ -1,7 +1,7 @@
42 = 42 :: ReferenceError
'x' = 42 :: ReferenceError
true = 42 :: ReferenceError
null = 42 :: ReferenceError
42 = 42 :: SyntaxError
'x' = 42 :: SyntaxError
true = 42 :: SyntaxError
null = 42 :: SyntaxError
delete this .. true
delete true .. true
delete 10 .. true
@@ -1 +1 @@
ReferenceError: Invalid left-hand side in assignment
SyntaxError: Invalid left-hand side in assignment.
@@ -1,2 +1,2 @@
Expected Invalid left-hand side in assignment
Expected Invalid left-hand side in assignment.
PASS
@@ -11,7 +11,7 @@ try {
}
} catch(e) {
var desc = e.description;
if(desc == "Invalid left-hand side in assignment")
if(desc == "Invalid left-hand side in assignment.")
{
WScript.Echo("Expected " + desc);
}
@@ -52,8 +52,8 @@ var tests = [
{
name: "new.target is not valid for assignment",
body: function() {
assert.throws(function() { eval("new.target = 'something';"); }, ReferenceError, "new.target cannot be a lhs in an assignment expression - this is an early reference error", "Invalid left-hand side in assignment");
assert.throws(function() { eval("((new.target)) = 'something';"); }, ReferenceError, "new.target cannot be a lhs in an assignment expression - this is an early reference error", "Invalid left-hand side in assignment");
assert.throws(function() { eval("new.target = 'something';"); }, SyntaxError, "new.target cannot be a lhs in an assignment expression - this is an early reference error", "Invalid left-hand side in assignment.");
assert.throws(function() { eval("((new.target)) = 'something';"); }, SyntaxError, "new.target cannot be a lhs in an assignment expression - this is an early reference error", "Invalid left-hand side in assignment.");
}
},

@@ -12,6 +12,6 @@ function foo() {
let a;
bar();
}
assert.throws(function () { foo(); }, ReferenceError, "Invalid assignment to array throws runtime reference error when destructuring is disabled", "Invalid left-hand side in assignment");
assert.throws(function () { foo(); }, SyntaxError, "Invalid assignment to array throws runtime reference error when destructuring is disabled", "Invalid left-hand side in assignment.");

WScript.Echo("PASS");
@@ -18,7 +18,7 @@ var tests = [
assert.throws(function () { eval("(function () { 'use strict'; [eval] = []; })();"); }, SyntaxError, "variable name 'eval' defined in array pattern is not valid in strict mode", "Invalid usage of 'eval' in strict mode");
assert.throws(function () { eval("let a = [a] = [10]"); }, ReferenceError, "A let variable is used in the array pattern in the same statement where it is declared", "Use before declaration");
assert.throws(function () { eval("let a = {a:a} = {}"); }, ReferenceError, "A let variable is used in object pattern in the same statement where it is declared", "Use before declaration");
assert.throws(function () { eval("var a = 1; (delete [a] = [2]);"); }, ReferenceError, "Array literal in unary expression should not be converted to array pattern", "Invalid left-hand side in assignment");
assert.throws(function () { eval("var a = 1; (delete [a] = [2]);"); }, SyntaxError, "Array literal in unary expression should not be converted to array pattern", "Invalid left-hand side in assignment.");
assert.throws(function () { eval("var x, b; for ([x] = [((b) = 1)] of ' ') { }"); }, ReferenceError, "Initializer in for..in is not valid but no assert should be thrown", "Invalid left-hand side in assignment");
assert.throws(function () { eval("for (let []; ;) { }"); }, SyntaxError, "Native for loop's head has one destructuring pattern without initializer", "Destructuring declarations must have an initializer");
assert.throws(function () { eval("for (let a = 1, []; ;) { }"); }, SyntaxError, "Native for loop's head has second param as destructuring pattern without initializer", "Destructuring declarations must have an initializer");
@@ -510,7 +510,7 @@ var tests = [
body: function () {
assert.throws(function () {
eval("if (false) { [11] += [1, 2, 3] }");
}, ReferenceError, "", "Invalid left-hand side in assignment");
}, SyntaxError, "", "Invalid left-hand side in assignment.");
assert.throws(function () {
eval("if (false) { [11] = [1, 2, 3] }");
}, SyntaxError, "", "Destructuring expressions can only have identifier references");
@@ -142,7 +142,7 @@ var tests = [
assert.doesNotThrow(function () { eval("let a; ({a:((((a1))))} = {a:20})"); }, "Object expression pattern with parens is valid syntax");
assert.doesNotThrow(function () { eval("let a, r1; ({a:a1 = r1} = {})"); }, "Object expression pattern with defaults as reference is valid syntax");
assert.doesNotThrow(function () { eval("let a, r1; ({a:a1 = r1 = 44} = {})"); }, "Object expression pattern with chained assignments as defaults is valid syntax");
assert.throws(function () { eval("let a, r1; ({a:(a1 = r1) = 44} = {})"); }, ReferenceError, "Object expression pattern with chained assignments but paren in between is not valid syntax", "Invalid left-hand side in assignment");
assert.throws(function () { eval("let a, r1; ({a:(a1 = r1) = 44} = {})"); }, SyntaxError, "Object expression pattern with chained assignments but paren in between is not valid syntax", "Invalid left-hand side in assignment.");
assert.doesNotThrow(function () { eval("var a; `${({a} = {})}`"); }, "Object expression pattern inside a string template is valid syntax");
assert.throws(function () { eval("for (let {x} = {} of []) {}"); }, SyntaxError, "for.of has declaration pattern with initializer is not valid syntax", "for-of loop head declarations cannot have an initializer");
assert.throws(function () { eval("for (let {x} = {} in []) {}"); }, SyntaxError, "for.in has declaration pattern with initializer is not valid syntax", "for-in loop head declarations cannot have an initializer");
@@ -77,9 +77,9 @@ var tests = [
{
name: "Invalid use of yield in lvalue positions that are runtime errors",
body: function () {
assert.throws(function () { eval('function* gf() { (yield) = 10; }'); var g = gf(); g.next(); g.next(); }, ReferenceError, "yield cannot be the LHS target of an assignment", "Invalid left-hand side in assignment");
assert.throws(function () { eval('function* gf() { ++(yield); }'); var g = gf(); g.next(); g.next(); }, ReferenceError, "yield cannot be the target of an increment operator", "Invalid left-hand side in assignment");
assert.throws(function () { eval('function* gf() { (yield)++; }'); var g = gf(); g.next(); g.next(); }, ReferenceError, "yield cannot be the target of an increment operator", "Invalid left-hand side in assignment");
assert.throws(function () { eval('function* gf() { (yield) = 10; }'); var g = gf(); g.next(); g.next(); }, SyntaxError, "yield cannot be the LHS target of an assignment", "Invalid left-hand side in assignment.");
assert.throws(function () { eval('function* gf() { ++(yield); }'); var g = gf(); g.next(); g.next(); }, SyntaxError, "yield cannot be the target of an increment operator", "Invalid left-hand side in assignment.");
assert.throws(function () { eval('function* gf() { (yield)++; }'); var g = gf(); g.next(); g.next(); }, SyntaxError, "yield cannot be the target of an increment operator", "Invalid left-hand side in assignment.");
}
},
{

0 comments on commit fd545b4

Please sign in to comment.
You can’t perform that action at this time.