Skip to content

Commit

Permalink
Improve parsing error messages
Browse files Browse the repository at this point in the history 
- add support for use of %s
- replace some frequntely enocuntered generic messages
  • Loading branch information
@rhuanjl
rhuanjl committed Oct 9, 2018
1 parent aa9823c commit 1f18f53
Show file tree
Hide file tree
Showing 18 changed files with 307 additions and 69 deletions.
201 changes: 190 additions & 11 deletions lib/Parser/Parse.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -159,9 +159,168 @@ void Parser::OutOfMemory()
throw ParseExceptionObject(ERRnoMemory);
}

void Parser::Error(HRESULT hr)
LPCWSTR Parser::GetTokenString(tokens token)
{
switch (token)
{
case tkNone : return _u("");
case tkEOF : return _u("end of script");
case tkIntCon : return _u("integer literal");
case tkFltCon : return _u("float literal");
case tkStrCon : return _u("string literal");
case tkRegExp : return _u("regular expression literal");

// keywords
case tkABSTRACT : return _u("abstract");
case tkASSERT : return _u("assert");
case tkAWAIT : return _u("await");
case tkBOOLEAN : return _u("boolean");
case tkBREAK : return _u("break");
case tkBYTE : return _u("byte");
case tkCASE : return _u("case");
case tkCATCH : return _u("catch");
case tkCHAR : return _u("char");
case tkCONTINUE : return _u("continue");
case tkDEBUGGER : return _u("debugger");
case tkDECIMAL : return _u("decimal");
case tkDEFAULT : return _u("default");
case tkDELETE : return _u("delete");
case tkDO : return _u("do");
case tkDOUBLE : return _u("double");
case tkELSE : return _u("else");
case tkENSURE : return _u("ensure");
case tkEVENT : return _u("event");
case tkFALSE : return _u("false");
case tkFINAL : return _u("final");
case tkFINALLY : return _u("finally");
case tkFLOAT : return _u("float");
case tkFOR : return _u("for");
case tkFUNCTION : return _u("function");
case tkGET : return _u("get");
case tkGOTO : return _u("goto");
case tkIF : return _u("if");
case tkIN : return _u("in");
case tkINSTANCEOF : return _u("instanceof");
case tkINT : return _u("int");
case tkINTERNAL : return _u("internal");
case tkINVARIANT : return _u("invariant");
case tkLONG : return _u("long");
case tkNAMESPACE : return _u("namespace");
case tkNATIVE : return _u("native");
case tkNEW : return _u("new");
case tkNULL : return _u("null");
case tkREQUIRE : return _u("require");
case tkRETURN : return _u("return");
case tkSBYTE : return _u("sbyte");
case tkSET : return _u("set");
case tkSHORT : return _u("short");
case tkSWITCH : return _u("switch");
case tkSYNCHRONIZED : return _u("synchronized");
case tkTHIS : return _u("this");
case tkTHROW : return _u("throw");
case tkTHROWS : return _u("throws");
case tkTRANSIENT : return _u("transient");
case tkTRUE : return _u("true");
case tkTRY : return _u("try");
case tkTYPEOF : return _u("typeof");
case tkUINT : return _u("uint");
case tkULONG : return _u("ulong");
case tkUSE : return _u("use");
case tkUSHORT : return _u("ushort");
case tkVAR : return _u("var");
case tkVOID : return _u("void");
case tkVOLATILE : return _u("volatile");
case tkWHILE : return _u("while");
case tkWITH : return _u("with");

// Future reserved words that become keywords in ES6
case tkCLASS : return _u("class");
case tkCONST : return _u("const");
case tkEXPORT : return _u("export");
case tkEXTENDS : return _u("extends");
case tkIMPORT : return _u("import");
case tkLET : return _u("let");
case tkSUPER : return _u("super");
case tkYIELD : return _u("yield");

// Future reserved words in strict and non-strict modes
case tkENUM : return _u("enum");

// Additional future reserved words in strict mode
case tkIMPLEMENTS : return _u("implements");
case tkINTERFACE : return _u("interface");
case tkPACKAGE : return _u("package");
case tkPRIVATE : return _u("private");
case tkPROTECTED : return _u("protected");
case tkPUBLIC : return _u("public");
case tkSTATIC : return _u("static");

case tkID: return _u("identifier");

// Non-operator non-identifier tokens
case tkSColon: return _u(";");
case tkRParen: return _u(")");
case tkRBrack: return _u("]");
case tkLCurly: return _u("{");
case tkRCurly: return _u("}");

// Operator non-identifier tokens
case tkComma: return _u(",");
case tkDArrow: return _u("=>");
case tkAsg: return _u("=");
case tkAsgAdd: return _u("+=");
case tkAsgSub: return _u("-=");
case tkAsgMul: return _u("*=");
case tkAsgDiv: return _u("/=");
case tkAsgExpo: return _u("**=");
case tkAsgMod: return _u("%=");
case tkAsgAnd: return _u("&=");
case tkAsgXor: return _u("^=");
case tkAsgOr: return _u("|=");
case tkAsgLsh: return _u("<<=");
case tkAsgRsh: return _u(">>=");
case tkAsgRs2: return _u(">>>=");
case tkQMark: return _u("?");
case tkColon: return _u(":");
case tkLogOr: return _u("||");
case tkLogAnd: return _u("&&");
case tkOr: return _u("|");
case tkXor: return _u("^");
case tkAnd: return _u("&");
case tkEQ: return _u("==");
case tkNE: return _u("!=");
case tkEqv: return _u("===");
case tkNEqv: return _u("!==");
case tkLT: return _u("<");
case tkLE: return _u("<=");
case tkGT: return _u(">");
case tkGE: return _u(">=");
case tkLsh: return _u("<<");
case tkRsh: return _u(">>");
case tkRs2: return _u(">>>");
case tkAdd: return _u("+");
case tkSub: return _u("-");
case tkExpo: return _u("**");
case tkStar: return _u("*");
case tkDiv: return _u("/");
case tkPct: return _u("%");
case tkTilde: return _u("~");
case tkBang: return _u("!");
case tkInc: return _u("++");
case tkDec: return _u("--");
case tkEllipsis: return _u("...");
case tkLParen: return _u("(");
case tkLBrack: return _u("[");
case tkDot: return _u(".");

default:
return _u("unknown token");
}
}

void Parser::Error(HRESULT hr, LPCWSTR stringOne, LPCWSTR stringTwo)
{
throw ParseExceptionObject(hr);
throw ParseExceptionObject(hr, stringOne, stringTwo);
}

void Parser::Error(HRESULT hr, ParseNodePtr pnode)
Expand Down Expand Up @@ -226,6 +385,7 @@ HRESULT Parser::ValidateSyntax(LPCUTF8 pszSrc, size_t encodedCharCount, bool isG

HRESULT hr;
SmartFPUControl smartFpuControl;
bool handled = false;

BOOL fDeferSave = m_deferringAST;
try
Expand Down Expand Up @@ -287,9 +447,11 @@ HRESULT Parser::ValidateSyntax(LPCUTF8 pszSrc, size_t encodedCharCount, bool isG
{
m_deferringAST = fDeferSave;
hr = e.GetError();
hr = pse->ProcessError(this->GetScanner(), hr, /* pnodeBase */ NULL, e.GetStringOne(), e.GetStringTwo());
handled = true;
}

if (nullptr != pse && FAILED(hr))
if (handled == false && nullptr != pse && FAILED(hr))
{
hr = pse->ProcessError(this->GetScanner(), hr, /* pnodeBase */ NULL);
}
Expand Down Expand Up @@ -326,6 +488,7 @@ HRESULT Parser::ParseSourceInternal(
ParseNodeProg * pnodeBase = NULL;
HRESULT hr;
SmartFPUControl smartFpuControl;
bool handled = false;

try
{
Expand Down Expand Up @@ -364,13 +527,15 @@ HRESULT Parser::ParseSourceInternal(
catch (ParseExceptionObject& e)
{
hr = e.GetError();
hr = pse->ProcessError(this->GetScanner(), hr, pnodeBase, e.GetStringOne(), e.GetStringTwo());
handled = true;
}
catch (Js::AsmJsParseException&)
{
hr = JSERR_AsmJsCompileError;
}

if (FAILED(hr))
if (handled == false && FAILED(hr))
{
hr = pse->ProcessError(this->GetScanner(), hr, pnodeBase);
}
Expand Down Expand Up @@ -2153,7 +2318,7 @@ IdentPtr Parser::ParseMetaProperty(tokens metaParentKeyword, charcount_t ichMin,
}
else
{
Error(ERRsyntax);
Error(ERRValidIfFollowedBy, _u("'new.'"), _u("'target'"));
}
}

Expand Down Expand Up @@ -2442,7 +2607,7 @@ void Parser::ParseImportClause(ModuleImportOrExportEntryList* importEntryList, b
// There cannot be a namespace import or named imports list on the left of the comma in a module import clause.
if (parsingAfterComma || parsedNamespaceOrNamedImport)
{
Error(ERRsyntax);
Error(ERRTokenAfter, _u(","), GetTokenString(this->GetScanner()->GetPrevious()));
}

this->GetScanner()->Scan();
Expand Down Expand Up @@ -2498,7 +2663,7 @@ ParseNodePtr Parser::ParseImport()
{
if (!m_scriptContext->GetConfig()->IsESDynamicImportEnabled())
{
Error(ERRsyntax);
Error(ERRExperimental);
}

ParseNodePtr pnode = ParseImportCall<buildAST>();
Expand Down Expand Up @@ -3109,7 +3274,7 @@ ParseNodePtr Parser::ParseTerm(BOOL fAllowCall,
// If the token after the right paren is not => or if there was a newline between () and => this is a syntax error
if (!IsDoingFastScan() && (m_token.tk != tkDArrow || this->GetScanner()->FHadNewLine()))
{
Error(ERRsyntax);
Error(ERRValidIfFollowedBy, _u("Lambda parameter list"), _u("'=>' on the same line"));
}

if (buildAST)
Expand Down Expand Up @@ -3423,7 +3588,18 @@ ParseNodePtr Parser::ParseTerm(BOOL fAllowCall,

default:
LUnknown:
Error(ERRsyntax);
if (m_token.tk == tkNone)
{
Error(ERRInvalidIdentifier, m_token.GetIdentifier(this->GetHashTbl())->Psz(), GetTokenString(GetScanner()->GetPrevious()));
}
else if (m_token.IsKeyword())
{
Error(ERRKeywordAfter, GetTokenString(m_token.tk), GetTokenString(GetScanner()->GetPrevious()));
}
else
{
Error(ERRTokenAfter, GetTokenString(m_token.tk), GetTokenString(GetScanner()->GetPrevious()));
}
break;
}

Expand Down Expand Up @@ -5607,7 +5783,7 @@ void Parser::ParseFncDeclHelper(ParseNodeFnc * pnodeFnc, LPCOLESTR pNameHint, us
// this after verifying there was a => token. Otherwise we would throw the wrong error.
if (hadNewLine)
{
Error(ERRsyntax);
Error(ERRValidIfFollowedBy, _u("Lambda parameter list"), _u("'=>' on the same line"));
}
}

Expand Down Expand Up @@ -11893,6 +12069,7 @@ HRESULT Parser::ParseFunctionInBackground(ParseNodeFnc * pnodeFnc, ParseContext
ParseNodeBlock * pnodeBlock = StartParseBlock<true>(PnodeBlockType::Function, ScopeType_FunctionBody);
pnodeFnc->pnodeScopes = pnodeBlock;
m_ppnodeScope = &pnodeBlock->pnodeScopes;
bool handled = false;

uint uDeferSave = m_grfscr & (fscrCanDeferFncParse | fscrWillDeferFncParse);

Expand Down Expand Up @@ -11943,9 +12120,11 @@ HRESULT Parser::ParseFunctionInBackground(ParseNodeFnc * pnodeFnc, ParseContext
catch (ParseExceptionObject& e)
{
hr = e.GetError();
hr = pse->ProcessError(this->GetScanner(), hr, nullptr, e.GetStringOne(), e.GetStringTwo());
handled = true;
}

if (FAILED(hr))
if (handled == false && FAILED(hr))
{
hr = pse->ProcessError(this->GetScanner(), hr, nullptr);
}
Expand Down
3 changes: 2 additions & 1 deletion lib/Parser/Parse.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -313,7 +313,8 @@ class Parser
Js::ScriptContext* m_scriptContext;
HashTbl * GetHashTbl() { return this->GetScanner()->GetHashTbl(); }

__declspec(noreturn) void Error(HRESULT hr);
LPCWSTR GetTokenString(tokens token);
__declspec(noreturn) void Error(HRESULT hr, LPCWSTR stringOne = _u(""), LPCWSTR stringTwo = _u(""));
private:
__declspec(noreturn) void Error(HRESULT hr, ParseNodePtr pnode);
__declspec(noreturn) void Error(HRESULT hr, charcount_t ichMin, charcount_t ichLim);
Expand Down
1 change: 1 addition & 0 deletions lib/Parser/Scan.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -688,6 +688,7 @@ class Scanner : public IScanner, public EncodingPolicy
}
};

tokens GetPrevious() { return m_tkPrevious; }
void Capture(_Out_ RestorePoint* restorePoint);
void SeekTo(const RestorePoint& restorePoint);
void SeekToForcingPid(const RestorePoint& restorePoint);
Expand Down
10 changes: 9 additions & 1 deletion lib/Parser/cmperr.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4,8 +4,16 @@
//-------------------------------------------------------------------------------------------------------
#include "ParserPch.h"

ParseExceptionObject::ParseExceptionObject(HRESULT hr) : m_hr(hr)
ParseExceptionObject::ParseExceptionObject(HRESULT hr, LPCWSTR stringOneIn, LPCWSTR stringTwoIn)
{
m_hr = hr;
stringOne = SysAllocString(stringOneIn);
stringTwo = SysAllocString(stringTwoIn);
Assert(FAILED(hr));
}

ParseExceptionObject::~ParseExceptionObject()
{
SysFreeString(stringOne);
SysFreeString(stringTwo);
}
7 changes: 6 additions & 1 deletion lib/Parser/cmperr.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -17,8 +17,13 @@ enum
class ParseExceptionObject
{
public:
ParseExceptionObject(HRESULT hr);
ParseExceptionObject(HRESULT hr, LPCWSTR stringOneIn = _u(""), LPCWSTR stringTwoIn = _u(""));
~ParseExceptionObject();
HRESULT GetError() { return m_hr; }
LPCWSTR GetStringOne() { return stringOne; }
LPCWSTR GetStringTwo() { return stringTwo; }
private:
HRESULT m_hr;
BSTR stringOne;
BSTR stringTwo;
};
9 changes: 9 additions & 0 deletions lib/Parser/perrors.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -109,3 +109,12 @@ LSC_ERROR_MSG(1093, ERRLabelBeforeClassDeclaration, "Labels not allowed before c
LSC_ERROR_MSG(1094, ERRLabelFollowedByEOF, "Unexpected end of script after a label.")
LSC_ERROR_MSG(1095, ERRFunctionAfterLabelInStrict, "Function declarations not allowed after a label in strict mode.")
LSC_ERROR_MSG(1096, ERRAwaitAsLabelInAsync, "Use of 'await' as label in async function is not allowed.")
LSC_ERROR_MSG(1097, ERRExperimental, "Use of disabled experimental feature")
//1098-1199 available for future use

// Generic errors intended to be re-usable
LSC_ERROR_MSG(1200, ERRKeywordAfter, "Unexpected keyword '%s' after '%s'")
LSC_ERROR_MSG(1201, ERRTokenAfter, "Unexpected token '%s' after '%s'")
LSC_ERROR_MSG(1202, ERRIdentifierAfter, "Unexpected identifier '%s' after '%s'")
LSC_ERROR_MSG(1203, ERRInvalidIdentifier, "Unexpected invalid identifier '%s' after '%s'")
LSC_ERROR_MSG(1205, ERRValidIfFollowedBy, "%s is only valid if followed by %s")
9 changes: 8 additions & 1 deletion lib/Parser/screrror.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -251,7 +251,7 @@ void CompileScriptException::CopyInto(CompileScriptException* pse)
}
}

HRESULT CompileScriptException::ProcessError(IScanner * pScan, HRESULT hr, ParseNode * pnodeBase)
HRESULT CompileScriptException::ProcessError(IScanner * pScan, HRESULT hr, ParseNode * pnodeBase, LPCWSTR stringOne, LPCWSTR stringTwo)
{
// fill in the ScriptException structure
Free();
Expand All @@ -267,6 +267,13 @@ HRESULT CompileScriptException::ProcessError(IScanner * pScan, HRESULT hr, Pars
if (nullptr == (ei.bstrDescription = SysAllocString(szT)))
ei.scode = E_OUTOFMEMORY;
}
else if (wcslen(stringOne) > 0)
{
OLECHAR szT[128];
_snwprintf_s(szT, ARRAYSIZE(szT), ARRAYSIZE(szT)-1, ei.bstrDescription, stringOne, stringTwo);
SysFreeString(ei.bstrDescription);
ei.bstrDescription = SysAllocString(szT);
}

ei.bstrSource = BstrGetResourceString(IDS_COMPILATION_ERROR_SOURCE);
if (nullptr == pnodeBase && nullptr != pScan)
Expand Down
2 changes: 1 addition & 1 deletion lib/Parser/screrror.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -72,7 +72,7 @@ class CompileScriptException : public ScriptException

void CopyInto(CompileScriptException* cse);

HRESULT ProcessError(IScanner * pScan, HRESULT hr, ParseNode * pnodeBase);
HRESULT ProcessError(IScanner * pScan, HRESULT hr, ParseNode * pnodeBase, LPCWSTR stringOne = _u(""), LPCWSTR stringTwo = _u(""));

friend class ActiveScriptError;
};
Loading

0 comments on commit 1f18f53

Please sign in to comment.