From 37dafcd7df05668c3e9c60f499486ed0a62c4898 Mon Sep 17 00:00:00 2001 From: Vladimir Matveev Date: Thu, 28 Jan 2016 15:16:53 -0800 Subject: [PATCH] Merge pull request #6704 from Microsoft/isRequireCall_fix MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit add extra argument to 'isRequireCall' to check if argument is string … --- src/compiler/binder.ts | 2 +- src/compiler/checker.ts | 2 +- src/compiler/program.ts | 2 +- src/compiler/utilities.ts | 12 +++++++----- tests/baselines/reference/dynamicRequire.js | 10 ++++++++++ tests/baselines/reference/dynamicRequire.symbols | 10 ++++++++++ tests/baselines/reference/dynamicRequire.types | 14 ++++++++++++++ tests/cases/compiler/dynamicRequire.ts | 8 ++++++++ 8 files changed, 52 insertions(+), 8 deletions(-) create mode 100644 tests/baselines/reference/dynamicRequire.js create mode 100644 tests/baselines/reference/dynamicRequire.symbols create mode 100644 tests/baselines/reference/dynamicRequire.types create mode 100644 tests/cases/compiler/dynamicRequire.ts diff --git a/src/compiler/binder.ts b/src/compiler/binder.ts index 88a1a62ce4c..cc81f92fbfc 100644 --- a/src/compiler/binder.ts +++ b/src/compiler/binder.ts @@ -1462,7 +1462,7 @@ namespace ts { function bindCallExpression(node: CallExpression) { // We're only inspecting call expressions to detect CommonJS modules, so we can skip // this check if we've already seen the module indicator - if (!file.commonJsModuleIndicator && isRequireCall(node)) { + if (!file.commonJsModuleIndicator && isRequireCall(node, /*checkArgumentIsStringLiteral*/false)) { setCommonJsModuleIndicator(node); } } diff --git a/src/compiler/checker.ts b/src/compiler/checker.ts index 55b7a1f0864..91613c0a531 100644 --- a/src/compiler/checker.ts +++ b/src/compiler/checker.ts @@ -10210,7 +10210,7 @@ namespace ts { } // In JavaScript files, calls to any identifier 'require' are treated as external module imports - if (isInJavaScriptFile(node) && isRequireCall(node)) { + if (isInJavaScriptFile(node) && isRequireCall(node, /*checkArgumentIsStringLiteral*/true)) { return resolveExternalModuleTypeByLiteral(node.arguments[0]); } diff --git a/src/compiler/program.ts b/src/compiler/program.ts index fb5f3866f7a..a3d6de8e2f7 100644 --- a/src/compiler/program.ts +++ b/src/compiler/program.ts @@ -990,7 +990,7 @@ namespace ts { } function collectRequireCalls(node: Node): void { - if (isRequireCall(node)) { + if (isRequireCall(node, /*checkArgumentIsStringLiteral*/true)) { (imports || (imports = [])).push((node).arguments[0]); } else { diff --git a/src/compiler/utilities.ts b/src/compiler/utilities.ts index 4a20a54b001..8bcf89ce1f2 100644 --- a/src/compiler/utilities.ts +++ b/src/compiler/utilities.ts @@ -1086,12 +1086,14 @@ namespace ts { * exactly one argument. * This function does not test if the node is in a JavaScript file or not. */ - export function isRequireCall(expression: Node): expression is CallExpression { + export function isRequireCall(expression: Node, checkArgumentIsStringLiteral: boolean): expression is CallExpression { // of the form 'require("name")' - return expression.kind === SyntaxKind.CallExpression && - (expression).expression.kind === SyntaxKind.Identifier && - ((expression).expression).text === "require" && - (expression).arguments.length === 1; + const isRequire = expression.kind === SyntaxKind.CallExpression && + (expression).expression.kind === SyntaxKind.Identifier && + ((expression).expression).text === "require" && + (expression).arguments.length === 1; + + return isRequire && (!checkArgumentIsStringLiteral || (expression).arguments[0].kind === SyntaxKind.StringLiteral); } /// Given a BinaryExpression, returns SpecialPropertyAssignmentKind for the various kinds of property diff --git a/tests/baselines/reference/dynamicRequire.js b/tests/baselines/reference/dynamicRequire.js new file mode 100644 index 00000000000..28b35970601 --- /dev/null +++ b/tests/baselines/reference/dynamicRequire.js @@ -0,0 +1,10 @@ +//// [a.js] + +function foo(name) { + var s = require("t/" + name) +} + +//// [a_out.js] +function foo(name) { + var s = require("t/" + name); +} diff --git a/tests/baselines/reference/dynamicRequire.symbols b/tests/baselines/reference/dynamicRequire.symbols new file mode 100644 index 00000000000..c307d578de7 --- /dev/null +++ b/tests/baselines/reference/dynamicRequire.symbols @@ -0,0 +1,10 @@ +=== tests/cases/compiler/a.js === + +function foo(name) { +>foo : Symbol(foo, Decl(a.js, 0, 0)) +>name : Symbol(name, Decl(a.js, 1, 13)) + + var s = require("t/" + name) +>s : Symbol(s, Decl(a.js, 2, 7)) +>name : Symbol(name, Decl(a.js, 1, 13)) +} diff --git a/tests/baselines/reference/dynamicRequire.types b/tests/baselines/reference/dynamicRequire.types new file mode 100644 index 00000000000..43eea88a05b --- /dev/null +++ b/tests/baselines/reference/dynamicRequire.types @@ -0,0 +1,14 @@ +=== tests/cases/compiler/a.js === + +function foo(name) { +>foo : (name: any) => void +>name : any + + var s = require("t/" + name) +>s : any +>require("t/" + name) : any +>require : any +>"t/" + name : string +>"t/" : string +>name : any +} diff --git a/tests/cases/compiler/dynamicRequire.ts b/tests/cases/compiler/dynamicRequire.ts new file mode 100644 index 00000000000..8d72a37dceb --- /dev/null +++ b/tests/cases/compiler/dynamicRequire.ts @@ -0,0 +1,8 @@ +// @allowJs: true +// @module: amd +// @out: a_out.js + +// @filename: a.js +function foo(name) { + var s = require("t/" + name) +} \ No newline at end of file