Minor improvements from a few more translation examples (#90)

packages/esify/README.md

@@ -48,5 +48,13 @@ module.exports = {
       properties: ['spy', 'stub', 'mock', 'server', 'requests'],
     },
   },
+
+  // A list of globals and their associated import paths for global-identifier-to-import
+  globalIdentifiers: {
+    _: 'lodash',
+    $: 'jquery',
+    jQuery: 'jquery',
+    moment: 'moment',
+  },
 }
 ```

packages/esify/index.js

@@ -7,14 +7,13 @@ require('babel-register')({ignore: false});
 var TRANSFORMS = [
   {path: 'shopify-codemod/transforms/coffeescript-soak-to-condition'},
   {path: 'shopify-codemod/transforms/ternary-statement-to-if-statement'},
+  {path: 'shopify-codemod/transforms/remove-useless-return-from-test', test: true},
   {path: 'shopify-codemod/transforms/mocha-context-to-closure', test: true},
   {path: 'shopify-codemod/transforms/mocha-context-to-global-reference', test: true},
   {path: 'shopify-codemod/transforms/coffeescript-range-output-to-helper'},
-  {path: 'shopify-codemod/transforms/remove-useless-return-from-test', test: true},
   {path: 'shopify-codemod/transforms/remove-addeventlistener-returns'},
   {path: 'shopify-codemod/transforms/conditional-assign-to-if-statement'},
   {path: 'shopify-codemod/transforms/global-assignment-to-default-export', test: false},
-  {path: 'shopify-codemod/transforms/global-reference-to-import'},
   // Order is significant for these initial assert transforms; think carefully before reordering.
   {path: 'shopify-codemod/transforms/assert/assert-false-to-assert-fail', test: true},
   {path: 'shopify-codemod/transforms/assert/assert-to-assert-ok', test: true},
@@ -29,14 +28,18 @@ var TRANSFORMS = [
   {path: 'shopify-codemod/transforms/assert/falsy-called-method-to-assert-not-called', test: true},
   // These are more generic, stylistic transforms, so they should come last to catch any
   // new nodes introduced by other transforms
-  {path: 'shopify-codemod/transforms/constant-function-expression-to-statement'},
   {path: 'shopify-codemod/transforms/remove-empty-returns'},
   {path: 'shopify-codemod/transforms/function-to-arrow'},
   {path: 'js-codemod/transforms/arrow-function'},
   {path: 'js-codemod/transforms/template-literals'},
   {path: 'shopify-codemod/transforms/strip-template-literal-parenthesis'},
   {path: 'js-codemod/transforms/object-shorthand'},
+  // constant-function-expression-to-statement and global-reference-to-import need
+  // `const` references, so they must happen after `no-vars`
   {path: 'js-codemod/transforms/no-vars'},
+  {path: 'shopify-codemod/transforms/constant-function-expression-to-statement'},
+  {path: 'shopify-codemod/transforms/global-reference-to-import'},
+  {path: 'shopify-codemod/transforms/global-identifier-to-import'},
   {path: 'js-codemod/transforms/unquote-properties'},
 ];
 
@@ -64,6 +67,21 @@ function loadOptions() {
           properties: ['spy', 'stub', 'mock', 'server', 'requests'],
         },
       },
+      globalIdentifiers: {
+        _: 'lodash',
+        $: 'jquery',
+        jQuery: 'jquery',
+        moment: 'moment',
+        jstz: 'jstimezonedetect',
+        mousetrap: 'mousetrap',
+        URI: 'urijs',
+        URITemplate: 'urijs/src/URITemplate',
+        ReconnectingWebSocket: 'shopify-reconnecting-websocket',
+        d3: 'd3',
+        NProgress: 'NProgress',
+        FastClick: 'shopify-fastclick',
+        Clipboard: 'clipboard',
+      },
     };
   }
 }

packages/shopify-codemod/README.md

@@ -12,6 +12,29 @@ This repository contains a collection of Codemods written with [JSCodeshift](htt
 
 ## Included Transforms
 
+### `global-identifer-to-import`
+
+Creates import statements for global identifiers. Use the `globalIdentifiers` option to specify identifier/ import path pairs.
+
+```sh
+jscodeshift -t shopify-codemods/transforms/global-identifier-to-import <file>
+```
+
+#### Example
+
+```js
+// with {globalIdentifiers: {_: 'lodash', $: 'jquery'}}
+_.map([], _.identity);
+window.$('.foo').find('.bar');
+
+// BECOMES:
+
+import _ from 'lodash';
+import $ from 'jquery';
+_.map([], _.identity);
+$('.foo').find('.bar');
+```
+
 ### `coffeescript-soak-to-condition`
 
 Changes the output of CoffeeScript’s soak operations (`foo?.bar.baz?()`) into a cleaner, more idiomatic JavaScript expression appropriate for its location in code.

packages/shopify-codemod/test/fixtures/global-identifier-to-import/basic.input.js

@@ -0,0 +1,7 @@
+'expose Foo.Bar';
+
+[].map(_.identity);
+window.moment.fromDate(Date.now());
+foo.$.find('.bar');
+window.React.renderToString(<div />);
+jstz(Date.now());

packages/shopify-codemod/test/fixtures/global-identifier-to-import/basic.output.js

@@ -0,0 +1,11 @@
+'expose Foo.Bar';
+
+import jstz from 'jstimezone/jstz';
+import moment from 'moment';
+import _ from 'lodash';
+
+[].map(_.identity);
+moment.fromDate(Date.now());
+foo.$.find('.bar');
+window.React.renderToString(<div />);
+jstz(Date.now());

packages/shopify-codemod/test/fixtures/remove-useless-return-from-test/iife.input.js

@@ -0,0 +1,25 @@
+suite('a', () => {
+  beforeEach(function() {
+    return (function() {
+      for (var i in [1, 2, 3]) {
+        assert(typeof i === 'number');
+      }
+    })();
+  });
+
+  afterEach(() => {
+    return (() => {
+      for (var i in [1, 2, 3]) {
+        assert(typeof i === 'number');
+      }
+    })();
+  });
+
+  it(() => {
+    return ((i) => {
+      for (var i in [1, 2, 3]) {
+        assert(typeof i === 'number');
+      }
+    })(i);
+  });
+});

packages/shopify-codemod/test/fixtures/remove-useless-return-from-test/iife.output.js

@@ -0,0 +1,21 @@
+suite('a', () => {
+  beforeEach(function() {
+    for (var i in [1, 2, 3]) {
+      assert(typeof i === 'number');
+    }
+  });
+
+  afterEach(() => {
+    for (var i in [1, 2, 3]) {
+      assert(typeof i === 'number');
+    }
+  });
+
+  it(() => {
+    ((i) => {
+      for (var i in [1, 2, 3]) {
+        assert(typeof i === 'number');
+      }
+    })(i);
+  });
+});

packages/shopify-codemod/test/transforms/global-identifier-to-import.test.js

@@ -0,0 +1,14 @@
+import 'test-helper';
+import globalIdentifierToImport from 'global-identifier-to-import';
+
+describe('globalIdentifierToImport', () => {
+  it('transforms basic setup variables', () => {
+    expect(globalIdentifierToImport).to.transform('global-identifier-to-import/basic', {
+      globalIdentifiers: {
+        _: 'lodash',
+        moment: 'moment',
+        jstz: 'jstimezone/jstz',
+      },
+    });
+  });
+});

packages/shopify-codemod/test/transforms/remove-useless-return-from-test.test.js

@@ -5,4 +5,8 @@ describe('removeUselessReturnFromTest', () => {
   it('transforms basic setup variables', () => {
     expect(removeUselessReturnFromTest).to.transform('remove-useless-return-from-test/basic');
   });
+
+  it('transforms empty returned IIFEs', () => {
+    expect(removeUselessReturnFromTest).to.transform('remove-useless-return-from-test/iife');
+  });
 });

packages/shopify-codemod/transforms/coffeescript-range-output-to-helper.js

@@ -1,10 +1,8 @@
-import {matchLast as matchLastNode} from './utils';
+import {matchLast} from './utils';
 
 const INCLUSIVE_OPERATORS = new Set(['<=', '>=']);
 
 export default function coffeescriptRangeOutputToHelper({source}, {jscodeshift: j}, {printOptions = {}}) {
-  const matchLast = matchLastNode.bind(null, j);
-
   function isCoffeeScriptRangeFunctionBody(statements) {
     const lastStatement = statements[statements.length - 1];
     const isProperReturn = j.match(lastStatement, {

packages/shopify-codemod/transforms/global-identifier-to-import.js

@@ -0,0 +1,49 @@
+import {insertAfterDirectives} from './utils';
+
+export default function globalIdentifierToImport({source}, {jscodeshift: j}, {printOptions = {}, globalIdentifiers = {}}) {
+  return j(source)
+    .find(j.Program)
+    .forEach((path) => {
+      const imports = new Set();
+
+      function isGlobalIdentifierName(name) {
+        return globalIdentifiers.hasOwnProperty(name);
+      }
+
+      function isWindow(windowPath) {
+        return windowPath.get('name').value === 'window' && !isNestedInMemberExpression(windowPath);
+      }
+
+      function isNestedInMemberExpression(aPath) {
+        const parentNode = aPath.parentPath.node;
+        return j.MemberExpression.check(parentNode) && parentNode.property === aPath.node;
+      }
+
+      function isGlobalIdentifier(identifierPath) {
+        return !isNestedInMemberExpression(identifierPath) || isWindow(identifierPath.parentPath.get('object'));
+      }
+
+      j(path)
+        .find(j.Identifier, {name: isGlobalIdentifierName})
+        .filter(isGlobalIdentifier)
+        .forEach((identifierPath) => {
+          imports.add(identifierPath.node.name);
+
+          // can only happen for window.globalIdentifier
+          if (isNestedInMemberExpression(identifierPath)) {
+            identifierPath.parentPath.replace(identifierPath.node);
+          }
+        });
+
+      const {node: {body}} = path;
+      for (const anImport of imports.values()) {
+        insertAfterDirectives(
+          body,
+          j.importDeclaration([
+            j.importDefaultSpecifier(j.identifier(anImport)),
+          ], j.literal(globalIdentifiers[anImport]))
+        );
+      }
+    })
+    .toSource(printOptions);
+}

packages/shopify-codemod/transforms/global-reference-to-import.js

@@ -23,6 +23,7 @@ export default function globalReferenceToImport(
     appGlobalIdentifiers,
   }) {
   const [binary, args] = determineFileSearcher();
+  const fileForIdentifier = {};
 
   /*
   * findDeclaringFile uses ack or the_silver_searcher to find the file in which something is declared.
@@ -53,6 +54,15 @@ export default function globalReferenceToImport(
     return relative(absolutePath, files[0]).replace(/\.[a-z]+$/, '');
   }
 
+  function getDeclaringFile(identifier) {
+    if (!fileForIdentifier.hasOwnProperty(identifier)) {
+      fileForIdentifier[identifier] = findDeclaringFile(identifier);
+    }
+
+    return fileForIdentifier[identifier];
+  }
+
+
   function isGlobalReference(object) {
     return appGlobalIdentifiers.indexOf(findFirstMember(object).name) >= 0;
   }
@@ -67,10 +77,8 @@ export default function globalReferenceToImport(
         if (imports.has(member)) {
           return imports.get(member).name;
         } else {
-          const file = findDeclaringFile(member);
-          if (file === null) {
-            return null;
-          }
+          const file = getDeclaringFile(member);
+          if (file === null) { return null; }
           const name = findLastMember(node).name;
           imports.set(member, {file, name});
           return name;

packages/shopify-codemod/transforms/mocha-context-to-global-reference.js

@@ -1,13 +1,6 @@
 import {MOCHA_FUNCTIONS} from './utils';
 
-export default function mochaContextToGlobalReference(
-  {source},
-  {jscodeshift: j},
-  {
-    printOptions = {},
-    testContextToGlobals = {},
-  }
-) {
+export default function mochaContextToGlobalReference({source}, {jscodeshift: j}, {printOptions = {}, testContextToGlobals = {}}) {
   const propMapping = Object.keys(testContextToGlobals).reduce((map, testGlobal) => {
     const testGlobalDetails = testContextToGlobals[testGlobal];
     testGlobalDetails.properties.forEach((prop) => {

packages/shopify-codemod/transforms/remove-empty-returns.js

@@ -4,7 +4,7 @@ export default function removeEmptyReturns({source}, {jscodeshift: j}, {printOpt
   return j(source)
     .find(j.Function, {
       body: {
-        body: matchLast(j, {
+        body: matchLast({
           type: 'ReturnStatement',
           argument: null,
         }),

packages/shopify-codemod/transforms/remove-useless-return-from-test.js

@@ -1,7 +1,19 @@
-import {MOCHA_FUNCTIONS, matchLast as matchLastNode} from './utils';
+import {MOCHA_FUNCTIONS, matchLast} from './utils';
 
 export default function removeUselessReturnFromTest({source}, {jscodeshift: j}, {printOptions = {}}) {
-  const matchLast = matchLastNode.bind(null, j);
+  function isUselessIIFE(node) {
+    return j.match(node, {
+      type: j.CallExpression.name,
+      callee: {
+        type: isFunctionExpressionType,
+        params: (params) => params.length === 0,
+      },
+    });
+  }
+
+  function isFunctionExpressionType(type) {
+    return type === j.ArrowFunctionExpression.name || type === j.FunctionExpression.name;
+  }
 
   return j(source)
     .find(j.CallExpression, {
@@ -10,11 +22,11 @@ export default function removeUselessReturnFromTest({source}, {jscodeshift: j},
         name: (name) => MOCHA_FUNCTIONS.has(name),
       },
       arguments: matchLast({
-        type: (type) => type === 'ArrowFunctionExpression' || type === 'FunctionExpression',
+        type: isFunctionExpressionType,
         body: {
-          type: 'BlockStatement',
+          type: j.BlockStatement.name,
           body: matchLast({
-            type: 'ReturnStatement',
+            type: j.ReturnStatement.name,
           }),
         },
       }),
@@ -23,7 +35,11 @@ export default function removeUselessReturnFromTest({source}, {jscodeshift: j},
       const {body: {body}} = path.node.arguments[path.node.arguments.length - 1];
       const returnStatement = body[body.length - 1];
       if (returnStatement.argument) {
-        body[body.length - 1] = j.expressionStatement(returnStatement.argument);
+        if (isUselessIIFE(returnStatement.argument)) {
+          returnStatement.argument.callee.body.body.forEach((statement) => { body[body.length - 1] = statement; });
+        } else {
+          body[body.length - 1] = j.expressionStatement(returnStatement.argument);
+        }
       } else {
         body.pop();
       }

packages/shopify-codemod/transforms/utils.js

@@ -1,3 +1,5 @@
+import j from 'jscodeshift';
+
 export function findFirstMember(node) {
   if (node.type === 'MemberExpression') {
     return findFirstMember(node.object);
@@ -12,7 +14,7 @@ export function findLastMember(node) {
   return node;
 }
 
-export function matchLast(j, matcher) {
+export function matchLast(matcher) {
   return (nodes) => nodes.length > 0 && j.match(nodes[nodes.length - 1], matcher);
 }