From c798489eb1a66de79864cf178114bf693bef2347 Mon Sep 17 00:00:00 2001 From: Kanchalai Tanglertsampan Date: Wed, 22 Mar 2017 15:04:17 -0700 Subject: [PATCH] Move error report of incorrect grammar in dynamic import to checker --- src/compiler/checker.ts | 35 +++++++++++++++++---- src/compiler/diagnosticMessages.json | 17 +++++----- src/compiler/parser.ts | 14 --------- src/compiler/program.ts | 4 +-- src/compiler/transformers/module/module.ts | 36 +++++++++++++--------- src/compiler/transformers/module/system.ts | 2 +- 6 files changed, 62 insertions(+), 46 deletions(-) diff --git a/src/compiler/checker.ts b/src/compiler/checker.ts index 6df2efeea83..6f459770912 100644 --- a/src/compiler/checker.ts +++ b/src/compiler/checker.ts @@ -14824,16 +14824,17 @@ namespace ts { // Grammar checking; stop grammar-checking if checkGrammarTypeArguments return true checkGrammarTypeArguments(node, node.typeArguments) || checkGrammarArguments(node, node.arguments); + // Dynamic import is not need to go through regular resolve signature. + if (node.expression.kind === SyntaxKind.ImportKeyword) { + return checkImportCallExpression(node); + } + const signature = getResolvedSignature(node); if (node.expression.kind === SyntaxKind.SuperKeyword) { return voidType; } - if (node.expression.kind === SyntaxKind.ImportKeyword) { - return checkImportCallExpression(node); - } - if (node.kind === SyntaxKind.NewExpression) { const declaration = signature.declaration; @@ -14872,7 +14873,12 @@ namespace ts { return getReturnTypeOfSignature(signature); } - function checkImportCallExpression(node: CallExpression): Type { + function checkImportCallExpression(node: ImportCall): Type { + // Check grammar of dynamic import + if (checkGrammarImportCallExpression(node)) { + return createPromiseReturnType(node, anyType); + } + if (modulekind === ModuleKind.ES2015) { grammarErrorOnNode(node, Diagnostics.Dynamic_import_cannot_be_used_when_targeting_ECMAScript_2015_modules); } @@ -23364,5 +23370,22 @@ namespace ts { }); return result; } + + /** + * + * @param node + */ + function checkGrammarImportCallExpression(node: ImportCall): boolean { + const arguments = node.arguments; + if (arguments.length !== 1) { + return grammarErrorOnNode(node, Diagnostics.Dynamic_import_must_have_one_specifier_as_an_argument); + } + + // see: parseArgumentOrArrayLiteralElement...we use this function which parse arguments of callExpression to parse specifier for dynamic import. + // parseArgumentOrArrayLiteralElement allows spread element to be in an argument list which is not allowed as specifier in dynamic import. + if (isSpreadExpression(arguments[0])) { + return grammarErrorOnNode(arguments[0], Diagnostics.Specifier_of_dynamic_import_cannot_be_spread_element); + } + } } -} +} \ No newline at end of file diff --git a/src/compiler/diagnosticMessages.json b/src/compiler/diagnosticMessages.json index 4618776af26..880dc7750f1 100644 --- a/src/compiler/diagnosticMessages.json +++ b/src/compiler/diagnosticMessages.json @@ -871,6 +871,15 @@ "category": "Error", "code": 1320 }, + "Dynamic import must have one specifier as an argument.": { + "category": "Error", + "code": 1321 + }, + "Specifier of dynamic import cannot be spread element.": { + "category": "Error", + "code": 1322 + }, + "Duplicate identifier '{0}'.": { "category": "Error", "code": 2300 @@ -3297,14 +3306,6 @@ "category": "Error", "code": 17013 }, - "Dynamic import can only have one specifier as an argument.": { - "category": "Error", - "code": 17014 - }, - "Specifier of dynamic import cannot be spread element.": { - "category": "Error", - "code": 17015 - }, "Circularity detected while resolving configuration: {0}": { "category": "Error", diff --git a/src/compiler/parser.ts b/src/compiler/parser.ts index f9bb5afd945..1575ec0c0f0 100644 --- a/src/compiler/parser.ts +++ b/src/compiler/parser.ts @@ -3652,20 +3652,6 @@ namespace ts { return finishNode(node); } - if (isImportCall(expression)) { - // Check that the argument array is strictly of length 1 and the argument is assignment-expression - const arguments = expression.arguments; - if (arguments.length !== 1) { - parseErrorAtPosition(arguments.pos, arguments.end - arguments.pos, Diagnostics.Dynamic_import_can_only_have_one_specifier_as_an_argument); - } - - // see: parseArgumentOrArrayLiteralElement...we use this function which parse arguments of callExpression to parse specifier for dynamic import. - // parseArgumentOrArrayLiteralElement allows spread element to be in an argument list which is not allowed in dynamic import. - if (expression.arguments.length >= 1 && isSpreadExpression(arguments[0])) { - parseErrorAtPosition(arguments.pos, arguments.end - arguments.pos, Diagnostics.Specifier_of_dynamic_import_cannot_be_spread_element); - } - } - return expression; } diff --git a/src/compiler/program.ts b/src/compiler/program.ts index e6581c382ea..b50c96a5f0c 100644 --- a/src/compiler/program.ts +++ b/src/compiler/program.ts @@ -1268,8 +1268,8 @@ namespace ts { if (isJavaScriptFile && isRequireCall(node, /*checkArgumentIsStringLiteral*/true)) { (imports || (imports = [])).push((node).arguments[0]); } - // we can safely get the first argument in the list here because we already issue parsing error if the length is not 1 - else if (isImportCall(node) && node.arguments[0].kind === SyntaxKind.StringLiteral) { + // we have to check the argument list has length of 1. We will still have to process these even though we have parsing error. + else if (isImportCall(node) && node.arguments.length === 1 && node.arguments[0].kind === SyntaxKind.StringLiteral) { (imports || (imports = [])).push((node).arguments[0]); } else { diff --git a/src/compiler/transformers/module/module.ts b/src/compiler/transformers/module/module.ts index 4524f660ddf..d7f3a2ad743 100644 --- a/src/compiler/transformers/module/module.ts +++ b/src/compiler/transformers/module/module.ts @@ -543,17 +543,14 @@ namespace ts { return createNew( createIdentifier("Promise"), /*typeArguments*/ undefined, - [ - createArrowFunction( - /*modifiers*/undefined, - /*typeParameters*/ undefined, - [createParameter(/*decorator*/ undefined, /*modifiers*/ undefined, /*dotDotDotToken*/ undefined, /*name*/ resolve)], - /*type*/ undefined, - createToken(SyntaxKind.EqualsGreaterThanToken), - createCall(createIdentifier("require"), /*typeArguments*/ undefined, [createArrayLiteral([node.arguments[0]]), resolve]) - ) - ] - ); + [createArrowFunction( + /*modifiers*/undefined, + /*typeParameters*/ undefined, + [createParameter(/*decorator*/ undefined, /*modifiers*/ undefined, /*dotDotDotToken*/ undefined, /*name*/ resolve)], + /*type*/ undefined, + createToken(SyntaxKind.EqualsGreaterThanToken), + createCall(createIdentifier("require"), /*typeArguments*/ undefined, node.arguments.concat([resolve])) + )]); } function transformImportCallExpressionCommonJS(node: ImportCall): Expression { @@ -564,11 +561,20 @@ namespace ts { // if we simply do require in resolve callback in Promise constructor. We will execute the loading immediately return createCall( createPropertyAccess( - createCall(/*expression*/ createPropertyAccess(createIdentifier("Promise"), "resolve"), /*typeArguments*/ undefined, /*argumentsArray*/[]), - "then"), + createCall( + createPropertyAccess(createIdentifier("Promise"), "resolve"), + /*typeArguments*/ undefined, + /*argumentsArray*/[] + ), "then"), /*typeArguments*/ undefined, - [createArrowFunction(/*modifiers*/ undefined, /*typeParameters*/ undefined, /*parameters*/ undefined, /*type*/ undefined, createToken(SyntaxKind.EqualsGreaterThanToken), createCall(createIdentifier("require"), /*typeArguments*/ undefined, [node.arguments[0]]))] - ); + [createArrowFunction( + /*modifiers*/ undefined, + /*typeParameters*/ undefined, + /*parameters*/ undefined, + /*type*/ undefined, + createToken(SyntaxKind.EqualsGreaterThanToken), + createCall(createIdentifier("require"), /*typeArguments*/ undefined, node.arguments) + )]); } /** diff --git a/src/compiler/transformers/module/system.ts b/src/compiler/transformers/module/system.ts index 03a1c8cb00b..f7395ab0791 100644 --- a/src/compiler/transformers/module/system.ts +++ b/src/compiler/transformers/module/system.ts @@ -1487,7 +1487,7 @@ namespace ts { createIdentifier("import") ), /*typeArguments*/ undefined, - [node.arguments[0]] + node.arguments ); }