Minor codemod fixups

CHANGELOG.md

@@ -5,6 +5,13 @@
 - Added a `add-missing-parseint-radix` transform to add missing radix parameters to `parseInt` calls.
 - Added a `implicit-coercion-to-explicit` transform to convert `!!foo` and `+foo` to their more explicit counterparts.
 - Added a `empty-func-to-lodash-noop` transform to correct empty function linting errors by replacing empty functions with `_.noop`.
+- Added a `remove-empty-statements` transform to get rid of pesky excess semicolons.
+
+### Fixed
+- Fixed edge cases that were failing for the `default-export-object-to-named-exports` transform.
+
+### Build
+- Fixed the name for transform tests generated by `bin/create-transform`.
 
 ## [12.0.2] - 2016-05-30
 ### Added

CONTRIBUTING.md

@@ -4,13 +4,13 @@ This repo actually contains multiple projects in addition to the formal stylegui
 script/setup
 ```
 
-Make *every* required change across all repos. For a given rule change, this will often involve at least a change to `eslint-plugin-shopify` and to the README for this repo. You can lint and test your changes across all repos by running:
+Make *every* required change across all repos. For a given rule change, this will often involve at least a change to `eslint-plugin-shopify` and to the README for this repo. You should also update the README(s) for the affected packages and add your changes to the unreleased section in the top-level CHANGELOG. You can lint and test your changes across all repos by running:
 
 ```bash
 script/test
 ```
 
-Once you are satisfied with your changes, open a pull request and get your changes merged. Then, update and commit the `CHANGELOG.md` file at the root of this repo with the changes you have made. Finally, run the publishing command:
+Once you are satisfied with your changes, open a pull request and get your changes merged. Then, update the unreleased section of the CHANGELOG with links to the relevant pull requests, rename the section to the new version, and commit these changes. Finally, run the publishing command:
 
 ```bash
 script/publish

packages/esify/index.js

@@ -55,6 +55,9 @@ var TRANSFORMS = [
   {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'},
+  // Must appear after constant-function-expression-to-statement in order to remove
+  // unneeded semicolons from exported function declarations
+  {path: 'shopify-codemod/transforms/remove-empty-statements'},
 ];
 
 var OPTIONS = loadOptions();
@@ -127,7 +130,7 @@ function warn(message) {
 
 var WARNING_CHECKS = [
   function checkForComments(source) {
-    if (/#[^=]/.test(source)) {
+    if (/#[^={]/.test(source)) {
       warn('Your file contains comments. Unfortunately, the CoffeeScript compiler does not expose these comments. Make sure to copy over any important comments to the appropriate place in your new JavaScript file');
     }
   },

packages/shopify-codemod/README.md

@@ -12,6 +12,24 @@ This repository contains a collection of Codemods written with [JSCodeshift](htt
 
 ## Included Transforms
 
+### `remove-empty-statements`
+
+Removes empty statements, which usually manifest as unnecessary semicolons.
+
+```sh
+jscodeshift -t shopify-codemods/transforms/remove-empty-statements <file>
+```
+
+### Example
+
+```js
+export function foo() {};
+
+// BECOMES:
+
+export function foo() {}
+```
+
 ### `implicit-coercion-to-explicit`
 
 Transforms implicit coercions to booleans (`!!foo`) and numbers (`+foo`) to their explicit counterparts (`Boolean(foo)` and `Number(foo)`, respectively).

packages/shopify-codemod/test/fixtures/convert-default-export-objects-to-named-exports/computed-keys.input.js

@@ -0,0 +1,4 @@
+export default {
+  foo: 'bar',
+  [baz]: 'qux',
+};

packages/shopify-codemod/test/fixtures/convert-default-export-objects-to-named-exports/computed-keys.output.js

@@ -0,0 +1,4 @@
+export default {
+  foo: 'bar',
+  [baz]: 'qux',
+};

packages/shopify-codemod/test/fixtures/convert-default-export-objects-to-named-exports/invalid-identifier.input.js

@@ -0,0 +1,3 @@
+export default {
+  'foo-bar': 'baz',
+};

packages/shopify-codemod/test/fixtures/convert-default-export-objects-to-named-exports/invalid-identifier.output.js

@@ -0,0 +1,3 @@
+export default {
+  'foo-bar': 'baz',
+};

packages/shopify-codemod/test/fixtures/convert-default-export-objects-to-named-exports/method.input.js

@@ -0,0 +1,7 @@
+export default {
+  foo: 'bar',
+  baz() {
+    return 'qux';
+  },
+  fuzz: () => buzz,
+};

packages/shopify-codemod/test/fixtures/convert-default-export-objects-to-named-exports/method.output.js

@@ -0,0 +1,9 @@
+export const foo = 'bar';
+
+export function baz() {
+  return 'qux';
+};
+
+export function fuzz() {
+  return buzz;
+};

packages/shopify-codemod/test/fixtures/convert-default-export-objects-to-named-exports/non-object.input.js

@@ -0,0 +1,5 @@
+export default [1, 2, 3];
+
+export const foo = {
+  bar: 'baz',
+};

packages/shopify-codemod/test/fixtures/convert-default-export-objects-to-named-exports/non-object.output.js

@@ -0,0 +1,5 @@
+export default [1, 2, 3];
+
+export const foo = {
+  bar: 'baz',
+};

packages/shopify-codemod/test/fixtures/convert-default-export-objects-to-named-exports/this.input.js

@@ -0,0 +1,6 @@
+export default {
+  foo: 'bar',
+  baz() {
+    return this.foo;
+  },
+};

packages/shopify-codemod/test/fixtures/convert-default-export-objects-to-named-exports/this.output.js

@@ -0,0 +1,6 @@
+export default {
+  foo: 'bar',
+  baz() {
+    return this.foo;
+  },
+};

packages/shopify-codemod/test/fixtures/convert-default-export-objects-to-named-exports/variables.input.js

@@ -1,4 +1,4 @@
 export default {
   dee: ta,
-  sho: foo
+  'sho': foo
 };

packages/shopify-codemod/test/fixtures/remove-empty-statements/basic.input.js

@@ -0,0 +1 @@
+export function foo() {};

packages/shopify-codemod/test/fixtures/remove-empty-statements/basic.output.js

@@ -0,0 +1 @@
+export function foo() {}

packages/shopify-codemod/test/transforms/convert-default-export-objects-to-named-exports.test.js

@@ -9,4 +9,28 @@ describe('convertDefaultExportObjectsToNamedExports', () => {
   it('converts export object variables', () => {
     expect(convertDefaultExportObjectsToNamedExports).to.transform('convert-default-export-objects-to-named-exports/variables');
   });
+
+  it('transforms methods into function declarations', () => {
+    expect(convertDefaultExportObjectsToNamedExports).to.transform('convert-default-export-objects-to-named-exports/method');
+  });
+
+  it('ignores empty default export objects', () => {
+    expect(convertDefaultExportObjectsToNamedExports).to.transform('convert-default-export-objects-to-named-exports/empty');
+  });
+
+  it('ignores non-object and non-default exports', () => {
+    expect(convertDefaultExportObjectsToNamedExports).to.transform('convert-default-export-objects-to-named-exports/non-object');
+  });
+
+  it('ignores objects with computed keys', () => {
+    expect(convertDefaultExportObjectsToNamedExports).to.transform('convert-default-export-objects-to-named-exports/computed-keys');
+  });
+
+  it('ignores objects with methods that use `this`', () => {
+    expect(convertDefaultExportObjectsToNamedExports).to.transform('convert-default-export-objects-to-named-exports/this');
+  });
+
+  it('ignores objects where any key is an invalid identifier', () => {
+    expect(convertDefaultExportObjectsToNamedExports).to.transform('convert-default-export-objects-to-named-exports/invalid-identifier');
+  });
 });

packages/shopify-codemod/test/transforms/remove-empty-statements.test.js

@@ -0,0 +1,8 @@
+import 'test-helper';
+import removeEmptyStatements from 'remove-empty-statements';
+
+describe('removeEmptyStatements', () => {
+  it('removes empty statements', () => {
+    expect(removeEmptyStatements).to.transform('remove-empty-statements/basic');
+  });
+});

packages/shopify-codemod/transform-cli/templates/test-suite.js.template

@@ -1,8 +1,8 @@
 import 'test-helper';
-import transform from '__DASHERIZED_NAME__';
+import __CAMELIZED_NAME__ from '__DASHERIZED_NAME__';
 
 describe('__CAMELIZED_NAME__', () => {
   it('CHANGE THIS', () => {
-    expect(transform).to.transform('__DASHERIZED_NAME__/basic');
+    expect(__CAMELIZED_NAME__).to.transform('__DASHERIZED_NAME__/basic');
   });
 });

packages/shopify-codemod/transforms/constant-function-expression-to-statement.js

@@ -1,27 +1,25 @@
+import {containsThisExpression, isFunctionExpression, getBlockStatementFromFunction} from './utils';
+
 export default function constantFunctionValueToStatement({source}, {jscodeshift: j}, {printOptions = {}}) {
   return j(source)
     .find(j.VariableDeclaration, (path) => j.match(path, {
       kind: 'const',
       declarations: [{
         type: 'VariableDeclarator',
-        init: {type: (type) => type === 'FunctionExpression' || type === 'ArrowFunctionExpression'},
+        init: isFunctionExpression,
       }],
-    }) && (path.declarations[0].init.type !== 'ArrowFunctionExpression' || j(path).find(j.ThisExpression).size() === 0))
+    }) && (path.declarations[0].init.type !== 'ArrowFunctionExpression' || !containsThisExpression(path)))
     .replaceWith((path) => {
       const declarator = path.node.declarations[0];
-      const {init: {params, generator, expression}} = declarator;
-      let {init: {body}} = declarator;
-      if (!j.BlockStatement.check(body)) {
-        body = j.blockStatement([j.returnStatement(body)]);
-      }
+      const {init: {params, generator}} = declarator;
 
       return j.functionDeclaration(
-          declarator.id,
-          params,
-          body,
-          generator,
-          expression
-        );
+        declarator.id,
+        params,
+        getBlockStatementFromFunction(declarator.init),
+        generator,
+        false,
+      );
     })
     .toSource(printOptions);
 }

packages/shopify-codemod/transforms/convert-default-export-objects-to-named-exports.js

@@ -1,17 +1,54 @@
+import {
+  isValidIdentifier,
+  getPropertyName,
+  containsThisExpression,
+  isFunctionExpression,
+  getBlockStatementFromFunction,
+} from './utils';
+
 export default function convertDefaultExportObjectsToNamedExports({source}, {jscodeshift: j}, {printOptions = {quote: 'single'}}) {
+  function isConvertibleProperty(property) {
+    return !property.computed && isValidIdentifier(getPropertyName(property)) && !containsThisExpression(property.value);
+  }
+
+  function isConvertibleObject(node) {
+    return j.ObjectExpression.check(node) && node.properties.every(isConvertibleProperty);
+  }
+
+  function exportDeclarationForProperty(property) {
+    const {value} = property;
+
+    if (isFunctionExpression(value)) {
+      const {params, generator} = value;
+      return j.exportNamedDeclaration(
+        j.functionDeclaration(
+          j.identifier(getPropertyName(property)),
+          params,
+          getBlockStatementFromFunction(value),
+          generator,
+          false,
+        )
+      );
+    }
+
+    return j.exportNamedDeclaration(
+      j.variableDeclaration('const', [
+        j.variableDeclarator(j.identifier(getPropertyName(property)), value),
+      ])
+    );
+  }
 
   return j(source)
-    .find(j.ExportDefaultDeclaration)
-    .forEach((nodePath) => {
-      const exportIndex = nodePath.parentPath.node.body.indexOf(nodePath.node);
-      const body = nodePath.parentPath.value;
-      const declaration = nodePath.get('declaration', 'properties').node;
+    .find(j.ExportDefaultDeclaration, {declaration: isConvertibleObject})
+    .forEach((path) => {
+      const {parentPath: {node: {body}}} = path;
+      const exportIndex = body.indexOf(path.node);
+      const declaration = path.get('declaration', 'properties').node;
+
       if (declaration.properties.length > 0) { delete body[exportIndex]; }
-      for (const dec of declaration.properties.reverse()) {
-        const exportDec = j.exportNamedDeclaration(
-          j.variableDeclaration('const', [j.variableDeclarator(dec.key, dec.value)])
-        );
-        body.splice(exportIndex, 0, exportDec);
+
+      for (const property of declaration.properties.reverse()) {
+        body.splice(exportIndex, 0, exportDeclarationForProperty(property));
       }
     })
     .toSource(printOptions);

packages/shopify-codemod/transforms/function-to-arrow.js

@@ -1,12 +1,10 @@
+import {containsThisExpression} from './utils';
+
 export default function functionToArrow({source}, {jscodeshift: j}, {printOptions = {}}) {
   function isMember({parent}) {
     return j.MethodDefinition.check(parent.node) || j.Property.check(parent.node);
   }
 
-  function containsThisExpression(path) {
-    return j(path).find(j.ThisExpression).size() > 0;
-  }
-
   function isConvertibleFunction(path) {
     return !isMember(path) && !containsThisExpression(path);
   }

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

@@ -0,0 +1,6 @@
+export default function removeEmptyStatements({source}, {jscodeshift: j}, {printOptions = {quote: 'single'}}) {
+  return j(source)
+    .find(j.EmptyStatement)
+    .replaceWith()
+    .toSource(printOptions);
+}

packages/shopify-codemod/transforms/utils.js

@@ -50,6 +50,27 @@ export function isValidIdentifier(identifier) {
   return typeof identifier === 'string' && IDENTIFIER_REGEX.test(identifier);
 }
 
+export function getPropertyName({key, computed}) {
+  if (computed) { return null; }
+  return j.Identifier.check(key) ? key.name : key.value;
+}
+
+export function containsThisExpression(node) {
+  return j(node).find(j.ThisExpression).size() > 0;
+}
+
+export function isFunctionExpression(node) {
+  return j.FunctionExpression.check(node) || j.ArrowFunctionExpression.check(node);
+}
+
+export function getBlockStatementFromFunction({body}) {
+  if (!j.BlockStatement.check(body)) {
+    return j.blockStatement([j.returnStatement(body)]);
+  }
+
+  return body;
+}
+
 // from https://github.com/sindresorhus/globals/blob/1e9ebc39828b92bd5c8ec7dc7bb07d62f2fb0153/globals.json#L852
 export const MOCHA_FUNCTIONS = new Set([
   'after',