add extra argument to 'isRequireCall' to check if argument is string literal

vladima · vladima · commit 3661b66be41a · 2016-01-28T14:18:23.000-08:00
diff --git 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
@@ -10239,7 +10239,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(<StringLiteral>node.arguments[0]);
             }
 
diff --git a/src/compiler/program.ts b/src/compiler/program.ts
@@ -1328,7 +1328,7 @@ namespace ts {
             }
 
             function collectRequireCalls(node: Node): void {
-                if (isRequireCall(node)) {
+                if (isRequireCall(node, /*checkArgumentIsStringLiteral*/true)) {
                     (imports || (imports = [])).push(<StringLiteral>(<CallExpression>node).arguments[0]);
                 }
                 else {
diff --git a/src/compiler/utilities.ts b/src/compiler/utilities.ts
@@ -1086,12 +1086,13 @@ 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 &&
+        const isRequire = expression.kind === SyntaxKind.CallExpression &&
                 (<CallExpression>expression).expression.kind === SyntaxKind.Identifier &&
                 (<Identifier>(<CallExpression>expression).expression).text === "require" &&
                 (<CallExpression>expression).arguments.length === 1;
+        return isRequire && (!checkArgumentIsStringLiteral || (<CallExpression>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
@@ -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
@@ -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
@@ -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
@@ -0,0 +1,8 @@
+// @allowJs: true
+// @module: amd
+// @out: a_out.js
+
+// @filename: a.js
+function foo(name) {
+    var s = require("t/" + name)
+}

Original file line number	Diff line number	Diff line change
`@@ -1462,7 +1462,7 @@ namespace ts {`
`1462`	`1462`	`function bindCallExpression(node: CallExpression) {`
`1463`	`1463`	`// We're only inspecting call expressions to detect CommonJS modules, so we can skip`
`1464`	`1464`	`// this check if we've already seen the module indicator`
`1465`		`- if (!file.commonJsModuleIndicator && isRequireCall(node)) {`
	`1465`	`+ if (!file.commonJsModuleIndicator && isRequireCall(node, /checkArgumentIsStringLiteral/false)) {`
`1466`	`1466`	`setCommonJsModuleIndicator(node);`
`1467`	`1467`	`}`
`1468`	`1468`	`}`
Original file line number	Diff line number	Diff line change
`@@ -10239,7 +10239,7 @@ namespace ts {`
`10239`	`10239`	`}`
`10240`	`10240`
`10241`	`10241`	`// In JavaScript files, calls to any identifier 'require' are treated as external module imports`
`10242`		`- if (isInJavaScriptFile(node) && isRequireCall(node)) {`
	`10242`	`+ if (isInJavaScriptFile(node) && isRequireCall(node, /checkArgumentIsStringLiteral/true)) {`
`10243`	`10243`	`return resolveExternalModuleTypeByLiteral(<StringLiteral>node.arguments[0]);`
`10244`	`10244`	`}`
`10245`	`10245`
Original file line number	Diff line number	Diff line change
`@@ -1328,7 +1328,7 @@ namespace ts {`
`1328`	`1328`	`}`
`1329`	`1329`
`1330`	`1330`	`function collectRequireCalls(node: Node): void {`
`1331`		`- if (isRequireCall(node)) {`
	`1331`	`+ if (isRequireCall(node, /checkArgumentIsStringLiteral/true)) {`
`1332`	`1332`	`(imports \|\| (imports = [])).push(<StringLiteral>(<CallExpression>node).arguments[0]);`
`1333`	`1333`	`}`
`1334`	`1334`	`else {`