From e7bf44e820584b52be6c1c198780acc7388d7dee Mon Sep 17 00:00:00 2001 From: Wesley Wigham Date: Fri, 21 Jul 2017 17:20:56 -0700 Subject: [PATCH 1/4] Fix for loop which retains jsdoc behaviors --- src/compiler/parser.ts | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/src/compiler/parser.ts b/src/compiler/parser.ts index 0be7734968fa9..7b294467d7c49 100644 --- a/src/compiler/parser.ts +++ b/src/compiler/parser.ts @@ -1881,6 +1881,11 @@ namespace ts { if (considerSemicolonAsDelimiter && token() === SyntaxKind.SemicolonToken && !scanner.hasPrecedingLineBreak()) { nextToken(); } + else if (isJSDocParameterStart() && parsingContext & (1 << ParsingContext.Parameters)) { + // If the token was a jsdoc parameter start and we're parsing parameter lists, + // we need to consume the (mostly erroneous) parameter token + nextToken(); + } continue; } @@ -2202,7 +2207,11 @@ namespace ts { return token() === SyntaxKind.DotDotDotToken || isIdentifierOrPattern() || isModifierKind(token()) || - token() === SyntaxKind.AtToken || token() === SyntaxKind.ThisKeyword || token() === SyntaxKind.NewKeyword || + token() === SyntaxKind.AtToken || token() === SyntaxKind.ThisKeyword || isJSDocParameterStart(); + } + + function isJSDocParameterStart(): boolean { + return token() === SyntaxKind.NewKeyword || token() === SyntaxKind.StringLiteral || token() === SyntaxKind.NumericLiteral; } From 7040df2094a5d36c76408689274444fe9fd8251e Mon Sep 17 00:00:00 2001 From: Wesley Wigham Date: Fri, 21 Jul 2017 17:30:01 -0700 Subject: [PATCH 2/4] Tests covering the bug --- .../parseCommaSeperatedNewlineNew.errors.txt | 14 ++++++++++++++ .../reference/parseCommaSeperatedNewlineNew.js | 7 +++++++ .../parseCommaSeperatedNewlineNumber.errors.txt | 11 +++++++++++ .../reference/parseCommaSeperatedNewlineNumber.js | 7 +++++++ .../parseCommaSeperatedNewlineString.errors.txt | 11 +++++++++++ .../reference/parseCommaSeperatedNewlineString.js | 7 +++++++ .../compiler/parseCommaSeperatedNewlineNew.ts | 2 ++ .../compiler/parseCommaSeperatedNewlineNumber.ts | 2 ++ .../compiler/parseCommaSeperatedNewlineString.ts | 2 ++ 9 files changed, 63 insertions(+) create mode 100644 tests/baselines/reference/parseCommaSeperatedNewlineNew.errors.txt create mode 100644 tests/baselines/reference/parseCommaSeperatedNewlineNew.js create mode 100644 tests/baselines/reference/parseCommaSeperatedNewlineNumber.errors.txt create mode 100644 tests/baselines/reference/parseCommaSeperatedNewlineNumber.js create mode 100644 tests/baselines/reference/parseCommaSeperatedNewlineString.errors.txt create mode 100644 tests/baselines/reference/parseCommaSeperatedNewlineString.js create mode 100644 tests/cases/compiler/parseCommaSeperatedNewlineNew.ts create mode 100644 tests/cases/compiler/parseCommaSeperatedNewlineNumber.ts create mode 100644 tests/cases/compiler/parseCommaSeperatedNewlineString.ts diff --git a/tests/baselines/reference/parseCommaSeperatedNewlineNew.errors.txt b/tests/baselines/reference/parseCommaSeperatedNewlineNew.errors.txt new file mode 100644 index 0000000000000..ff3ff1a033d6c --- /dev/null +++ b/tests/baselines/reference/parseCommaSeperatedNewlineNew.errors.txt @@ -0,0 +1,14 @@ +tests/cases/compiler/parseCommaSeperatedNewlineNew.ts(1,2): error TS2304: Cannot find name 'a'. +tests/cases/compiler/parseCommaSeperatedNewlineNew.ts(1,2): error TS2695: Left side of comma operator is unused and has no side effects. +tests/cases/compiler/parseCommaSeperatedNewlineNew.ts(2,4): error TS1109: Expression expected. + + +==== tests/cases/compiler/parseCommaSeperatedNewlineNew.ts (3 errors) ==== + (a, + ~ +!!! error TS2304: Cannot find name 'a'. + ~ +!!! error TS2695: Left side of comma operator is unused and has no side effects. + new) + ~ +!!! error TS1109: Expression expected. \ No newline at end of file diff --git a/tests/baselines/reference/parseCommaSeperatedNewlineNew.js b/tests/baselines/reference/parseCommaSeperatedNewlineNew.js new file mode 100644 index 0000000000000..c1326c47802a8 --- /dev/null +++ b/tests/baselines/reference/parseCommaSeperatedNewlineNew.js @@ -0,0 +1,7 @@ +//// [parseCommaSeperatedNewlineNew.ts] +(a, +new) + +//// [parseCommaSeperatedNewlineNew.js] +(a, + new ); diff --git a/tests/baselines/reference/parseCommaSeperatedNewlineNumber.errors.txt b/tests/baselines/reference/parseCommaSeperatedNewlineNumber.errors.txt new file mode 100644 index 0000000000000..73f087cc1e680 --- /dev/null +++ b/tests/baselines/reference/parseCommaSeperatedNewlineNumber.errors.txt @@ -0,0 +1,11 @@ +tests/cases/compiler/parseCommaSeperatedNewlineNumber.ts(1,2): error TS2304: Cannot find name 'a'. +tests/cases/compiler/parseCommaSeperatedNewlineNumber.ts(1,2): error TS2695: Left side of comma operator is unused and has no side effects. + + +==== tests/cases/compiler/parseCommaSeperatedNewlineNumber.ts (2 errors) ==== + (a, + ~ +!!! error TS2304: Cannot find name 'a'. + ~ +!!! error TS2695: Left side of comma operator is unused and has no side effects. + 1) \ No newline at end of file diff --git a/tests/baselines/reference/parseCommaSeperatedNewlineNumber.js b/tests/baselines/reference/parseCommaSeperatedNewlineNumber.js new file mode 100644 index 0000000000000..167e604665a87 --- /dev/null +++ b/tests/baselines/reference/parseCommaSeperatedNewlineNumber.js @@ -0,0 +1,7 @@ +//// [parseCommaSeperatedNewlineNumber.ts] +(a, +1) + +//// [parseCommaSeperatedNewlineNumber.js] +(a, + 1); diff --git a/tests/baselines/reference/parseCommaSeperatedNewlineString.errors.txt b/tests/baselines/reference/parseCommaSeperatedNewlineString.errors.txt new file mode 100644 index 0000000000000..2c70bfbfc3017 --- /dev/null +++ b/tests/baselines/reference/parseCommaSeperatedNewlineString.errors.txt @@ -0,0 +1,11 @@ +tests/cases/compiler/parseCommaSeperatedNewlineString.ts(1,2): error TS2304: Cannot find name 'a'. +tests/cases/compiler/parseCommaSeperatedNewlineString.ts(1,2): error TS2695: Left side of comma operator is unused and has no side effects. + + +==== tests/cases/compiler/parseCommaSeperatedNewlineString.ts (2 errors) ==== + (a, + ~ +!!! error TS2304: Cannot find name 'a'. + ~ +!!! error TS2695: Left side of comma operator is unused and has no side effects. + '') \ No newline at end of file diff --git a/tests/baselines/reference/parseCommaSeperatedNewlineString.js b/tests/baselines/reference/parseCommaSeperatedNewlineString.js new file mode 100644 index 0000000000000..f7f96541c77c8 --- /dev/null +++ b/tests/baselines/reference/parseCommaSeperatedNewlineString.js @@ -0,0 +1,7 @@ +//// [parseCommaSeperatedNewlineString.ts] +(a, +'') + +//// [parseCommaSeperatedNewlineString.js] +(a, + ''); diff --git a/tests/cases/compiler/parseCommaSeperatedNewlineNew.ts b/tests/cases/compiler/parseCommaSeperatedNewlineNew.ts new file mode 100644 index 0000000000000..a0062db6cf119 --- /dev/null +++ b/tests/cases/compiler/parseCommaSeperatedNewlineNew.ts @@ -0,0 +1,2 @@ +(a, +new) \ No newline at end of file diff --git a/tests/cases/compiler/parseCommaSeperatedNewlineNumber.ts b/tests/cases/compiler/parseCommaSeperatedNewlineNumber.ts new file mode 100644 index 0000000000000..cc07845674db9 --- /dev/null +++ b/tests/cases/compiler/parseCommaSeperatedNewlineNumber.ts @@ -0,0 +1,2 @@ +(a, +1) \ No newline at end of file diff --git a/tests/cases/compiler/parseCommaSeperatedNewlineString.ts b/tests/cases/compiler/parseCommaSeperatedNewlineString.ts new file mode 100644 index 0000000000000..e6cff2438a5ae --- /dev/null +++ b/tests/cases/compiler/parseCommaSeperatedNewlineString.ts @@ -0,0 +1,2 @@ +(a, +'') \ No newline at end of file From 06beee1cc8e0712706b06542287fa25b6a809ca2 Mon Sep 17 00:00:00 2001 From: Wesley Wigham Date: Mon, 24 Jul 2017 15:16:21 -0700 Subject: [PATCH 3/4] Much simpler fix, rolls in really old fix, removed unused comment --- src/compiler/parser.ts | 38 +++++++++----------------------------- 1 file changed, 9 insertions(+), 29 deletions(-) diff --git a/src/compiler/parser.ts b/src/compiler/parser.ts index 7b294467d7c49..926c52273a28a 100644 --- a/src/compiler/parser.ts +++ b/src/compiler/parser.ts @@ -1881,11 +1881,6 @@ namespace ts { if (considerSemicolonAsDelimiter && token() === SyntaxKind.SemicolonToken && !scanner.hasPrecedingLineBreak()) { nextToken(); } - else if (isJSDocParameterStart() && parsingContext & (1 << ParsingContext.Parameters)) { - // If the token was a jsdoc parameter start and we're parsing parameter lists, - // we need to consume the (mostly erroneous) parameter token - nextToken(); - } continue; } @@ -2207,11 +2202,7 @@ namespace ts { return token() === SyntaxKind.DotDotDotToken || isIdentifierOrPattern() || isModifierKind(token()) || - token() === SyntaxKind.AtToken || token() === SyntaxKind.ThisKeyword || isJSDocParameterStart(); - } - - function isJSDocParameterStart(): boolean { - return token() === SyntaxKind.NewKeyword || + token() === SyntaxKind.AtToken || token() === SyntaxKind.ThisKeyword || token() === SyntaxKind.NewKeyword || token() === SyntaxKind.StringLiteral || token() === SyntaxKind.NumericLiteral; } @@ -2230,30 +2221,19 @@ namespace ts { // FormalParameter [Yield,Await]: // BindingElement[?Yield,?Await] node.name = parseIdentifierOrPattern(); - if (getFullWidth(node.name) === 0 && !hasModifiers(node) && isModifierKind(token())) { - // in cases like - // 'use strict' - // function foo(static) - // isParameter('static') === true, because of isModifier('static') - // however 'static' is not a legal identifier in a strict mode. - // so result of this function will be ParameterDeclaration (flags = 0, name = missing, type = undefined, initializer = undefined) - // and current token will not change => parsing of the enclosing parameter list will last till the end of time (or OOM) - // to avoid this we'll advance cursor to the next token. - nextToken(); - } node.questionToken = parseOptionalToken(SyntaxKind.QuestionToken); node.type = parseParameterType(); node.initializer = parseBindingElementInitializer(/*inParameter*/ true); - // Do not check for initializers in an ambient context for parameters. This is not - // a grammar error because the grammar allows arbitrary call signatures in - // an ambient context. - // It is actually not necessary for this to be an error at all. The reason is that - // function/constructor implementations are syntactically disallowed in ambient - // contexts. In addition, parameter initializers are semantically disallowed in - // overload signatures. So parameter initializers are transitively disallowed in - // ambient contexts. + const zeroLengthName = getFullWidth(node.name) === 0; + if (zeroLengthName && !node.type && !node.initializer && !node.decorators && !node.modifiers && !node.dotDotDotToken && !node.questionToken) { + // What we're parsing isn't actually remotely recognizable as a parameter and we've consumed no tokens whatsoever + // Consume a token to advance the parser in some way and avoid an infinite loop in `parseDelimitedList` + // This can happen when we're speculatively parsing parenthesized expressions which we think may be arrow functions, + // or when a modifier keyword which is disallowed as a parameter name (ie, `static` in strict mode) is supplied + nextToken(); + } return addJSDocComment(finishNode(node)); } From 98d58308313ccfd7141b895a1e4bf88b63a0cc54 Mon Sep 17 00:00:00 2001 From: Wesley Wigham Date: Mon, 24 Jul 2017 17:49:44 -0700 Subject: [PATCH 4/4] Use scanner position instead of node members --- src/compiler/parser.ts | 15 +++++++++++++-- 1 file changed, 13 insertions(+), 2 deletions(-) diff --git a/src/compiler/parser.ts b/src/compiler/parser.ts index 926c52273a28a..156a6698e2313 100644 --- a/src/compiler/parser.ts +++ b/src/compiler/parser.ts @@ -2214,6 +2214,7 @@ namespace ts { return finishNode(node); } + const startPos = scanner.getStartPos(); node.decorators = parseDecorators(); node.modifiers = parseModifiers(); node.dotDotDotToken = parseOptionalToken(SyntaxKind.DotDotDotToken); @@ -2221,13 +2222,23 @@ namespace ts { // FormalParameter [Yield,Await]: // BindingElement[?Yield,?Await] node.name = parseIdentifierOrPattern(); + if (getFullWidth(node.name) === 0 && !hasModifiers(node) && isModifierKind(token())) { + // in cases like + // 'use strict' + // function foo(static) + // isParameter('static') === true, because of isModifier('static') + // however 'static' is not a legal identifier in a strict mode. + // so result of this function will be ParameterDeclaration (flags = 0, name = missing, type = undefined, initializer = undefined) + // and current token will not change => parsing of the enclosing parameter list will last till the end of time (or OOM) + // to avoid this we'll advance cursor to the next token. + nextToken(); + } node.questionToken = parseOptionalToken(SyntaxKind.QuestionToken); node.type = parseParameterType(); node.initializer = parseBindingElementInitializer(/*inParameter*/ true); - const zeroLengthName = getFullWidth(node.name) === 0; - if (zeroLengthName && !node.type && !node.initializer && !node.decorators && !node.modifiers && !node.dotDotDotToken && !node.questionToken) { + if (startPos === scanner.getStartPos()) { // What we're parsing isn't actually remotely recognizable as a parameter and we've consumed no tokens whatsoever // Consume a token to advance the parser in some way and avoid an infinite loop in `parseDelimitedList` // This can happen when we're speculatively parsing parenthesized expressions which we think may be arrow functions,