Repository: skovhus/jest-codemods
Pull request #127 (2018-08-28 22:56:20)

fix: Proxyquire matches wrong CallExpression
Hey! Love this project thanks for all the efforts.

This change fixes the mechanism that finds Proxyquire instances. Before this change it was not finding the correct CallExpression. After this change, it precisely finds any CallExpression where the callee's name is the provided identifier.

Here's some sample code that led me to this pull request:
```js
beforeEach( function(){
	const something = pq( '../../../../some/file', {
		'some-key' : {
			SomeClass : spies.someSpy
		}
	});
});
```

Before this change, it was trying to identify `beforeEach` as the "outer call expression" when it should be targeting `pq()`. With this change the codemod was able to accurately replace these instances.

Cheers!

coleturner: It took a lot of finesse for the matching sequence:

https://github.com/skovhus/jest-codemods/pull/127/commits/86ef574b2eb97adbb984e39333ab80410ee4c886#diff-6df57da017a02b0def2332808961cdb9L35

But I managed to have it compliant with the existing test suite and added a new test. 

Patch:
diff --git a/src/utils/proxyquire.js b/src/utils/proxyquire.js
index 24d7cf0..493f59e 100644
--- a/src/utils/proxyquire.js
+++ b/src/utils/proxyquire.js
@@ -8,16 +8,6 @@ function findChildOfProgram(path, childPath) {
     return findChildOfProgram(path.parent, path);
 }
 
-function findFirstParentCallExpression(path) {
-    if (!path) {
-        return null;
-    }
-    if (path.node.type === 'CallExpression') {
-        return findFirstParentCallExpression(path.parentPath) || path;
-    }
-    return findFirstParentCallExpression(path.parentPath);
-}
-
 const getJestMockStatement = ({ j, mockName, mockBody }) =>
     j.expressionStatement(
         j.callExpression(j.identifier('jest.mock'), [
@@ -26,21 +16,47 @@ const getJestMockStatement = ({ j, mockName, mockBody }) =>
         ])
     );
 
+function getIdentifierValue(ast, name) {
+    const varDec = ast.findVariableDeclarators(name).get(0);
+
+    if (varDec && 'value' in varDec.node.init) {
+        return varDec.node.init.value;
+    }
+
+    return null;
+}
+
 export default function proxyquireTransformer(fileInfo, j, ast) {
     const importVariableName = removeRequireAndImport(j, ast, 'proxyquire');
     if (importVariableName) {
         const mocks = new Set();
 
         ast
-            .find(j.Identifier, {
-                name: importVariableName,
-            })
-            .forEach(p => {
-                const outerCallExpression = findFirstParentCallExpression(p);
-                if (!outerCallExpression) {
-                    return;
+            .find(j.CallExpression, match => {
+                if (
+                    match.callee.type === 'CallExpression' &&
+                    match.callee.callee.type === 'MemberExpression'
+                ) {
+                    return match.callee.callee.object.name === importVariableName;
+                } else if (
+                    match.callee.type === 'MemberExpression' &&
+                    match.callee.object.type === 'CallExpression' &&
+                    match.callee.object.callee.type === 'MemberExpression'
+                ) {
+                    return match.callee.object.callee.object.name === importVariableName;
+                } else if (
+                    match.callee.type === 'MemberExpression' &&
+                    match.callee.object.name === importVariableName
+                ) {
+                    const parentFind = ast.find(j.CallExpression, { callee: match });
+                    const parent = parentFind.size() && parentFind.get();
+
+                    return !parent || parent.node.type !== 'CallExpression';
                 }
 
+                return match.callee.name === importVariableName;
+            })
+            .forEach(outerCallExpression => {
                 const args = outerCallExpression.node.arguments;
                 if (args.length === 0) {
                     // proxyquire is called with no arguments
@@ -48,11 +64,20 @@ export default function proxyquireTransformer(fileInfo, j, ast) {
                     return;
                 }
 
-                const requireFile = args[0].value;
+                const pathArg = args[0];
+                const requireFile =
+                    pathArg.type === 'Identifier'
+                        ? getIdentifierValue(ast, pathArg.name)
+                        : pathArg.value;
+
                 const mocksNode = args[1];
 
                 if (mocks.has(requireFile)) {
-                    logger(fileInfo, 'Multiple mocks of same file is not supported', p);
+                    logger(
+                        fileInfo,
+                        'Multiple mocks of same file is not supported',
+                        outerCallExpression
+                    );
                     return;
                 }
                 mocks.add(requireFile);
@@ -84,7 +109,7 @@ export default function proxyquireTransformer(fileInfo, j, ast) {
                         logger(
                             fileInfo,
                             'proxyrequire mocks not transformed due to missing declaration',
-                            p
+                            outerCallExpression
                         );
                         return;
                     }


Test patch:
diff --git a/src/utils/proxyquire.test.js b/src/utils/proxyquire.test.js
index 1f06b61..de73eea 100644
--- a/src/utils/proxyquire.test.js
+++ b/src/utils/proxyquire.test.js
@@ -165,6 +165,27 @@ it('supports a chained `noCallThru().load()` call', () => {
     expect(mockedLogger).not.toBeCalled();
 });
 
+it('supports named imports scoped to the variable name', () => {
+    const ast = j(
+        `
+        import pq from 'proxyquire';
+        beforeEach( function(){
+            const something = pq( 'a', {'b': 'c'});
+        });
+    `
+    );
+    proxyquireTransformer(fileInfo, j, ast);
+    expect(ast.toSource(getOptions())).toEqual(
+        `
+        jest.mock('b', () => 'c');
+        beforeEach( function(){
+            const something = require('a');
+        });
+    `
+    );
+    expect(mockedLogger).not.toBeCalled();
+});
+
 it('logs error when proxyquire mocks are not defined in the file', () => {
     // TODO: this is kind of a bad state, but also a funny usage of proxyquire
     const ast = j(