metro-bundler: collect-dependencies: harden requirements for require()

Reviewed By: davidaurelio

Differential Revision: D5658522

fbshipit-source-id: 2970d62bcb18d57595c666d680407a23eccda77b

packages/metro-bundler/src/ModuleGraph/worker/__tests__/collect-dependencies-test.js

@@ -18,6 +18,8 @@ const {codeFromAst, comparableCode} = require('../../test-helpers');
 
 const {any} = expect;
 
+const {InvalidRequireCallError} = collectDependencies;
+
 describe('dependency collection from ASTs:', () => {
   it('collects dependency identifiers from the code', () => {
     const ast = astFromCode(`
@@ -41,16 +43,20 @@ describe('dependency collection from ASTs:', () => {
     expect(collectDependencies(ast).dependencies).toEqual(['left-pad']);
   });
 
-  it('ignores template literals with interpolations', () => {
+  it('throws on template literals with interpolations', () => {
     const ast = astFromCode('require(`left${"-"}pad`)');
 
-    expect(collectDependencies(ast).dependencies).toEqual([]);
+    expect(() => collectDependencies(ast).dependencies).toThrowError(
+      InvalidRequireCallError,
+    );
   });
 
-  it('ignores tagged template literals', () => {
+  it('throws on tagged template literals', () => {
     const ast = astFromCode('require(tag`left-pad`)');
 
-    expect(collectDependencies(ast).dependencies).toEqual([]);
+    expect(() => collectDependencies(ast).dependencies).toThrowError(
+      InvalidRequireCallError,
+    );
   });
 
   it('exposes a string as `dependencyMapName`', () => {
@@ -66,7 +72,6 @@ describe('dependency collection from ASTs:', () => {
   it('replaces all required module ID strings with array lookups, keeps the ID as second argument', () => {
     const ast = astFromCode(`
         const a = require('b/lib/a');
-        const b = require(123);
         exports.do = () => require("do");
         if (!something) {
           require("setup/something");
@@ -78,7 +83,6 @@ describe('dependency collection from ASTs:', () => {
     expect(codeFromAst(ast)).toEqual(
       comparableCode(`
         const a = require(${dependencyMapName}[0], 'b/lib/a');
-        const b = require(123);
         exports.do = () => require(${dependencyMapName}[1], "do");
         if (!something) {
           require(${dependencyMapName}[2], "setup/something");
@@ -96,7 +100,6 @@ describe('Dependency collection from optimized ASTs:', () => {
   beforeEach(() => {
     ast = astFromCode(`
       const a = require(${dependencyMapName}[0], 'b/lib/a');
-      const b = require(123);
       exports.do = () => require(${dependencyMapName}[1], "do");
       if (!something) {
         require(${dependencyMapName}[2], "setup/something");
@@ -126,7 +129,6 @@ describe('Dependency collection from optimized ASTs:', () => {
     expect(codeFromAst(ast)).toEqual(
       comparableCode(`
       const a = require(${dependencyMapName}[0]);
-      const b = require(123);
       exports.do = () => require(${dependencyMapName}[1]);
       if (!something) {
         require(${dependencyMapName}[2]);

packages/metro-bundler/src/ModuleGraph/worker/collect-dependencies.js

@@ -15,6 +15,7 @@
 const nullthrows = require('fbjs/lib/nullthrows');
 
 const {traverse, types} = require('babel-core');
+const prettyPrint = require('babel-generator').default;
 
 type AST = Object;
 
@@ -27,13 +28,16 @@ class Replacement {
     this.nextIndex = 0;
   }
 
-  isRequireCall(callee, firstArg) {
+  getRequireCallArg(node) {
-    return (
+    const args = node.arguments;
-      callee.type === 'Identifier' &&
+    if (args.length !== 1 || !isLiteralString(args[0])) {
-      callee.name === 'require' &&
+      throw new InvalidRequireCallError(
-      firstArg &&
+        'Calls to require() expect exactly 1 string literal argument, but ' +
-      isLiteralString(firstArg)
+          'this was found: ' +
-    );
+          prettyPrint(node).code,
+      );
+    }
+    return args[0];
   }
 
   getIndex(stringLiteralOrTemplateLiteral) {
@@ -59,6 +63,14 @@ class Replacement {
   }
 }
 
+function getInvalidProdRequireMessage(node) {
+  return (
+    'Post-transform calls to require() expect 2 arguments, the first ' +
+    'of which has the shape `_dependencyMapName[123]`, but this was found: ' +
+    prettyPrint(node).code
+  );
+}
+
 class ProdReplacement {
   replacement: Replacement;
   names: Array<string>;
@@ -68,15 +80,22 @@ class ProdReplacement {
     this.names = names;
   }
 
-  isRequireCall(callee, firstArg) {
+  getRequireCallArg(node) {
-    return (
+    const args = node.arguments;
-      callee.type === 'Identifier' &&
+    if (args.length !== 2) {
-      callee.name === 'require' &&
+      throw new InvalidRequireCallError(getInvalidProdRequireMessage(node));
-      firstArg &&
+    }
-      firstArg.type === 'MemberExpression' &&
+    const arg = args[0];
-      firstArg.property &&
+    if (
-      firstArg.property.type === 'NumericLiteral'
+      !(
-    );
+        arg.type === 'MemberExpression' &&
+        arg.property &&
+        arg.property.type === 'NumericLiteral'
+      )
+    ) {
+      throw new InvalidRequireCallError(getInvalidProdRequireMessage(node));
+    }
+    return args[0];
   }
 
   getIndex(memberExpression) {
@@ -111,6 +130,7 @@ function createMapLookup(dependencyMapIdentifier, propertyIdentifier) {
 }
 
 function collectDependencies(ast, replacement, dependencyMapIdentifier) {
+  const visited = new WeakSet();
   const traversalState = {dependencyMapIdentifier};
   traverse(
     ast,
@@ -124,14 +144,18 @@ function collectDependencies(ast, replacement, dependencyMapIdentifier) {
       },
       CallExpression(path, state) {
         const node = path.node;
-        const arg = node.arguments[0];
+        if (visited.has(node)) {
-        if (replacement.isRequireCall(node.callee, arg)) {
+          return;
+        }
+        if (isRequireCall(node.callee)) {
+          const arg = replacement.getRequireCallArg(node);
           const index = replacement.getIndex(arg);
           node.arguments = replacement.makeArgs(
             types.numericLiteral(index),
             arg,
             state.dependencyMapIdentifier,
           );
+          visited.add(node);
         }
       },
     },
@@ -152,6 +176,16 @@ function isLiteralString(node) {
   );
 }
 
+function isRequireCall(callee) {
+  return callee.type === 'Identifier' && callee.name === 'require';
+}
+
+class InvalidRequireCallError extends Error {
+  constructor(message) {
+    super(message);
+  }
+}
+
 const xp = (module.exports = (ast: AST) =>
   collectDependencies(ast, new Replacement()));
 
@@ -165,3 +199,5 @@ xp.forOptimization = (
     new ProdReplacement(names),
     dependencyMapName ? types.identifier(dependencyMapName) : undefined,
   );
+
+xp.InvalidRequireCallError = InvalidRequireCallError;