From 1595e659eb57403d95e70f4f463640bef18899a8 Mon Sep 17 00:00:00 2001 From: Chris Sauve Date: Fri, 20 May 2016 10:17:41 -0400 Subject: [PATCH 1/3] Add global-identifier-to-import transform --- packages/esify/README.md | 8 +++ packages/esify/index.js | 16 ++++++ packages/shopify-codemod/README.md | 23 +++++++++ .../basic.input.js | 7 +++ .../basic.output.js | 11 +++++ .../global-identifier-to-import.test.js | 14 ++++++ .../transforms/global-identifier-to-import.js | 49 +++++++++++++++++++ 7 files changed, 128 insertions(+) create mode 100644 packages/shopify-codemod/test/fixtures/global-identifier-to-import/basic.input.js create mode 100644 packages/shopify-codemod/test/fixtures/global-identifier-to-import/basic.output.js create mode 100644 packages/shopify-codemod/test/transforms/global-identifier-to-import.test.js create mode 100644 packages/shopify-codemod/transforms/global-identifier-to-import.js diff --git a/packages/esify/README.md b/packages/esify/README.md index b612204..0f6537b 100644 --- a/packages/esify/README.md +++ b/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', + }, } ``` diff --git a/packages/esify/index.js b/packages/esify/index.js index 097b4e3..5289868 100644 --- a/packages/esify/index.js +++ b/packages/esify/index.js @@ -37,6 +37,7 @@ var TRANSFORMS = [ {path: 'shopify-codemod/transforms/strip-template-literal-parenthesis'}, {path: 'js-codemod/transforms/object-shorthand'}, {path: 'js-codemod/transforms/no-vars'}, + {path: 'shopify-codemod/transforms/global-identifier-to-import'}, {path: 'js-codemod/transforms/unquote-properties'}, ]; @@ -64,6 +65,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', + }, }; } } diff --git a/packages/shopify-codemod/README.md b/packages/shopify-codemod/README.md index 45174a9..9611a93 100644 --- a/packages/shopify-codemod/README.md +++ b/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` + +Transforms global identifiers that you specify into the appropriate import statements. In order for this to work, you must set the `globalIdentifiers` option to an object where the keys are the names of globals used in your script, and the keys are the import paths for those globals. + +```sh +jscodeshift -t shopify-codemods/transforms/global-identifier-to-import +``` + +#### 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. diff --git a/packages/shopify-codemod/test/fixtures/global-identifier-to-import/basic.input.js b/packages/shopify-codemod/test/fixtures/global-identifier-to-import/basic.input.js new file mode 100644 index 0000000..52381bc --- /dev/null +++ b/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(
); +jstz(Date.now()); diff --git a/packages/shopify-codemod/test/fixtures/global-identifier-to-import/basic.output.js b/packages/shopify-codemod/test/fixtures/global-identifier-to-import/basic.output.js new file mode 100644 index 0000000..32ae108 --- /dev/null +++ b/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(
); +jstz(Date.now()); diff --git a/packages/shopify-codemod/test/transforms/global-identifier-to-import.test.js b/packages/shopify-codemod/test/transforms/global-identifier-to-import.test.js new file mode 100644 index 0000000..1d20e15 --- /dev/null +++ b/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', + }, + }); + }); +}); diff --git a/packages/shopify-codemod/transforms/global-identifier-to-import.js b/packages/shopify-codemod/transforms/global-identifier-to-import.js new file mode 100644 index 0000000..e25c5a8 --- /dev/null +++ b/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); +} From e1860851b826bc5fd6f740a718a4f3dded692325 Mon Sep 17 00:00:00 2001 From: Chris Sauve Date: Fri, 20 May 2016 10:18:07 -0400 Subject: [PATCH 2/3] Other minor improvements --- packages/esify/index.js | 8 ++++-- .../iife.input.js | 25 +++++++++++++++++ .../iife.output.js | 21 ++++++++++++++ .../remove-useless-return-from-test.test.js | 4 +++ .../coffeescript-range-output-to-helper.js | 4 +-- .../transforms/global-reference-to-import.js | 16 ++++++++--- .../mocha-context-to-global-reference.js | 9 +----- .../transforms/remove-empty-returns.js | 2 +- .../remove-useless-return-from-test.js | 28 +++++++++++++++---- packages/shopify-codemod/transforms/utils.js | 4 ++- 10 files changed, 95 insertions(+), 26 deletions(-) create mode 100644 packages/shopify-codemod/test/fixtures/remove-useless-return-from-test/iife.input.js create mode 100644 packages/shopify-codemod/test/fixtures/remove-useless-return-from-test/iife.output.js diff --git a/packages/esify/index.js b/packages/esify/index.js index 5289868..b6fca83 100644 --- a/packages/esify/index.js +++ b/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,17 @@ 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'}, ]; diff --git a/packages/shopify-codemod/test/fixtures/remove-useless-return-from-test/iife.input.js b/packages/shopify-codemod/test/fixtures/remove-useless-return-from-test/iife.input.js new file mode 100644 index 0000000..a71820d --- /dev/null +++ b/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); + }); +}); diff --git a/packages/shopify-codemod/test/fixtures/remove-useless-return-from-test/iife.output.js b/packages/shopify-codemod/test/fixtures/remove-useless-return-from-test/iife.output.js new file mode 100644 index 0000000..a95637f --- /dev/null +++ b/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); + }); +}); diff --git a/packages/shopify-codemod/test/transforms/remove-useless-return-from-test.test.js b/packages/shopify-codemod/test/transforms/remove-useless-return-from-test.test.js index 8e45f45..95b813a 100644 --- a/packages/shopify-codemod/test/transforms/remove-useless-return-from-test.test.js +++ b/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'); + }); }); diff --git a/packages/shopify-codemod/transforms/coffeescript-range-output-to-helper.js b/packages/shopify-codemod/transforms/coffeescript-range-output-to-helper.js index ffb21f4..cf3a862 100644 --- a/packages/shopify-codemod/transforms/coffeescript-range-output-to-helper.js +++ b/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, { diff --git a/packages/shopify-codemod/transforms/global-reference-to-import.js b/packages/shopify-codemod/transforms/global-reference-to-import.js index 07b9b7b..df0fd9c 100644 --- a/packages/shopify-codemod/transforms/global-reference-to-import.js +++ b/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; diff --git a/packages/shopify-codemod/transforms/mocha-context-to-global-reference.js b/packages/shopify-codemod/transforms/mocha-context-to-global-reference.js index 60c00fd..ebb361c 100644 --- a/packages/shopify-codemod/transforms/mocha-context-to-global-reference.js +++ b/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) => { diff --git a/packages/shopify-codemod/transforms/remove-empty-returns.js b/packages/shopify-codemod/transforms/remove-empty-returns.js index e4d2a23..48f073f 100644 --- a/packages/shopify-codemod/transforms/remove-empty-returns.js +++ b/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, }), diff --git a/packages/shopify-codemod/transforms/remove-useless-return-from-test.js b/packages/shopify-codemod/transforms/remove-useless-return-from-test.js index f164fbf..db77fc3 100644 --- a/packages/shopify-codemod/transforms/remove-useless-return-from-test.js +++ b/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(); } diff --git a/packages/shopify-codemod/transforms/utils.js b/packages/shopify-codemod/transforms/utils.js index 3246c9a..bef2785 100644 --- a/packages/shopify-codemod/transforms/utils.js +++ b/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); } From 584080fd6dd6230d9ac4f53ec43209504c4604d7 Mon Sep 17 00:00:00 2001 From: Chris Sauve Date: Fri, 20 May 2016 13:33:40 -0400 Subject: [PATCH 3/3] Clean up the language --- packages/shopify-codemod/README.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/shopify-codemod/README.md b/packages/shopify-codemod/README.md index 9611a93..53f85dc 100644 --- a/packages/shopify-codemod/README.md +++ b/packages/shopify-codemod/README.md @@ -14,7 +14,7 @@ This repository contains a collection of Codemods written with [JSCodeshift](htt ### `global-identifer-to-import` -Transforms global identifiers that you specify into the appropriate import statements. In order for this to work, you must set the `globalIdentifiers` option to an object where the keys are the names of globals used in your script, and the keys are the import paths for those globals. +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