diff --git a/README.md b/README.md index 454b020..2aae3ed 100644 --- a/README.md +++ b/README.md @@ -48,6 +48,14 @@ jscodeshift -t js-codemod/transforms/invalid-requires.js jscodeshift -t js-codemod/transforms/jest-update.js ``` +#### `no-reassign-params` + +Converts functions to not reassign to parameters. This is useful to turn on in conjunction with [Flow's const_params](https://flow.org/en/docs/config/options/#toc-experimental-const-params-boolean) option. + +```sh +jscodeshift -t js-codemod/transforms/no-reassign-params.js +``` + #### `no-vars` Conservatively converts `var` to `const` or `let`. diff --git a/transforms/__tests__/no-reassign-params-test.js b/transforms/__tests__/no-reassign-params-test.js new file mode 100644 index 0000000..dfe31e8 --- /dev/null +++ b/transforms/__tests__/no-reassign-params-test.js @@ -0,0 +1,385 @@ +'use strict'; + +const defineInlineTest = require('jscodeshift/dist/testUtils').defineInlineTest; +const transform = require('../no-reassign-params'); + +describe('no-reassign-params', () => { + // Ignore generated files + defineInlineTest( + transform, + undefined, + ` +// @` + + `generated +function foo(boo) { + boo++ +}`, + '', + ); + + // Function declaration + defineInlineTest( + transform, + undefined, + ` +function foo(boo, val) { + val = 4; + + function bar(val) { + return val + 1; + } + + return boo++ + val; +}`, + ` +function foo(boo, val) { + let localBoo = boo; + let localVal = val; + localVal = 4; + + function bar(val) { + return val + 1; + } + + return localBoo++ + localVal; +}`, + ); + + // arrow function + defineInlineTest( + transform, + undefined, + ` +(boo, val) => { + val = 4; + + function bar(val) { + return val + 1; + } + + return boo++ + val; +}`, + ` +(boo, val) => { + let localBoo = boo; + let localVal = val; + localVal = 4; + + function bar(val) { + return val + 1; + } + + return localBoo++ + localVal; +}`, + ); + + // function assignment + defineInlineTest( + transform, + undefined, + ` +const foo3 = function(boo, val) { + val = 4; + + function bar(val) { + return val + 1; + } + + return boo++ + val; +} +`, + ` +const foo3 = function(boo, val) { + let localBoo = boo; + let localVal = val; + localVal = 4; + + function bar(val) { + return val + 1; + } + + return localBoo++ + localVal; +}`, + ); + + // inner function + defineInlineTest( + transform, + undefined, + ` +function foo(bar) { + function boo(bar) { + bar = 2; + return bar; + } +}`, + ` +function foo(bar) { + function boo(bar) { + let localBar = bar; + localBar = 2; + return localBar; + } +}`, + ); + + // defined in outer scope. Should leave unchanged. + // Can support this later if necessary + defineInlineTest( + transform, + undefined, + ` +let localFoo = 2; +function foo(foo) { + foo = 4; + return foo; +}`, + '', + ); + + defineInlineTest( + transform, + undefined, + ` +function foo({x}) { + x = 4; + return x; +}`, + ` +function foo({x}) { + let localX = x; + localX = 4; + return localX; +}`, + ); + + defineInlineTest( + transform, + undefined, + ` +function foo(keys) { + keys++; + [].push({ + keys: keys + }); +}`, + ` +function foo(keys) { + let localKeys = keys; + localKeys++; + [].push({ + keys: localKeys + }); +}`, + ); + + defineInlineTest( + transform, + undefined, + ` +function foo(x, y, z) { + ({y, z} = x); + + return y + z; +}`, + ` +function foo(x, y, z) { + let localY = y; + let localZ = z; + ({y: localY, z: localZ} = x); + + return localY + localZ; +}`, + ); + + defineInlineTest( + transform, + undefined, + ` +function foo(x, y, z) { + ({y: y, z: z} = x); + + return y + z; +}`, + ` +function foo(x, y, z) { + let localY = y; + let localZ = z; + ({y: localY, z: localZ} = x); + + return localY + localZ; +}`, + ); + + defineInlineTest( + transform, + undefined, + ` +function foo(...args) { + args = args.slice(0, 1); +}`, + ` +function foo(...args) { + let localArgs = args; + localArgs = localArgs.slice(0, 1); +}`, + ); + + defineInlineTest( + transform, + undefined, + ` +function foo(arg) { + arg = { + arg: arg + }; +}`, + ` +function foo(arg) { + let localArg = arg; + localArg = { + arg: localArg + }; +}`, + ); + + defineInlineTest( + transform, + undefined, + ` +const toImage = ({focus, image, ...rest}) => ({ + ...rest, + ...image, + ...(focus ? {focusX: focus.x, focusY: focus.y} : null), +});`, + '', + ); + + defineInlineTest( + transform, + undefined, + ` +const GKSwitch = ({label, value, onChange, ...props}) => ( + + {label} + + +);`, + '', + ); + + defineInlineTest( + transform, + undefined, + ` +function foo([{value: Component}, props]) { + Component = 4; +}`, + ` +function foo([{value: Component}, props]) { + let LocalComponent = Component; + LocalComponent = 4; +} + `, + ); + + defineInlineTest( + transform, + undefined, + ` +function foo([, props]) { + props = 4; +}`, + ` +function foo([, props]) { + let localProps = props; + localProps = 4; +}`, + ); + + defineInlineTest( + transform, + undefined, + ` +function foo(num) { + num++; + + return
; +} +`, + ` +function foo(num) { + let localNum = num; + localNum++; + + return
; +} +`, + ); + + defineInlineTest( + transform, + undefined, + ` +function func(i) { + var range = foo(i), i = -1; + + i++; +} +`, + '', + ); + + defineInlineTest( + transform, + undefined, + ` +function foo(Component) { + Component =
; +} +`, + ` +function foo(Component) { + let LocalComponent = Component; + LocalComponent =
; +} + `, + ); + + // Function declaration + defineInlineTest( + transform, + undefined, + ` +// @format +function foo(boo) {boo++;}`, + ` +// @format +function foo(boo) { + let localBoo = boo; + localBoo++; +}`, + ); + + // Shadowing function argument + // This doesn't work yet and is the major blocker to us being able to + // use this at Facebook. +// defineInlineTest( +// transform, +// undefined, +// ` +// function foo(x) { +// x = 2; +// var x = 4; +// return x; +// }`, +// ` +// function foo({x}) { +// let localX = x; +// localX = 2; + +// var localX2 = 4; +// return localX2; +// }`, +// ); +}); diff --git a/transforms/no-reassign-params.js b/transforms/no-reassign-params.js new file mode 100644 index 0000000..5ea6aa0 --- /dev/null +++ b/transforms/no-reassign-params.js @@ -0,0 +1,258 @@ +'use strict'; + +module.exports = function transformer(file, api) { + const j = api.jscodeshift; + const statement = j.template.statement; + + const FUNCTION_TYPES = [ + j.FunctionDeclaration, + j.ArrowFunctionExpression, + j.FunctionExpression, + ]; + + let updated = false; + + function getNewName(paramName) { + const firstChar = paramName.charAt(0); + const isUpperCase = paramName.charAt(0).toUpperCase() === firstChar; + + if (isUpperCase) { + return `Local${paramName}`; + } else { + const upperCase = paramName.charAt(0).toUpperCase() + paramName.slice(1); + return `local${upperCase}`; + } + } + + function getLocalVarStatement(paramName, newName) { + const localVar = statement`let ${j.identifier(newName)} = ${paramName};\n`; + return localVar; + } + + function definedInParentScope(identifierName, scope) { + let localScope = scope; + while (localScope) { + if (localScope.declares(identifierName)) { + return true; + } + localScope = localScope.parent; + } + + return false; + } + + function getParamNames(params) { + return [].concat( + ...params.map(param => { + if (param === null) { + return null; + } else if (param.type === 'Identifier') { + return param.name; + } else if (param.type === 'ObjectPattern') { + return param.properties.map(property => { + if (j.Property.check(property)) { + return property.value.name; + } else if ( + j.SpreadProperty.check(property) || + j.RestProperty.check(property) + ) { + return property.argument.name; + } else { + throw new Error( + `Unexpected Property Type ${property.type} ${j( + property, + ).toSource()}`, + ); + } + }); + } else if (param.type === 'RestElement') { + return param.argument.name; + } else if (j.AssignmentPattern.check(param)) { + return param.left.name; + } else if (j.ArrayPattern.check(param)) { + return [].concat(...getParamNames(param.elements)); + } else { + throw new Error( + `Unexpected Param Type ${param.type} ${j(param).toSource()}`, + ); + } + }), + ); + } + + function updateFunction(func) { + const params = func.get('params'); + + const functionScope = func.scope; + + const newBindings = new Set(); + + const paramNames = getParamNames(params.value); + + const reassignedParamNames = paramNames.filter(paramName => { + const numAssignments = j(func) + .find(j.AssignmentExpression) + .filter(assignment => { + const left = assignment.node.left; + + // old = 4; + if (j.Identifier.check(left)) { + return left.name === paramName; + } else if (j.ObjectPattern.check(left)) { + return left.properties.some(property => { + if (j.Property.check(property)) { + return property.key.name === paramName; + } else if (j.RestProperty.check(property)) { + return property.argument.name === paramName; + } else { + throw new Error( + `Unexpected Property Type ${property.type} ${j( + property, + ).toSource()}`, + ); + } + }); + } + + return false; + }).length; + + const numUpdated = j(func).find(j.UpdateExpression, { + argument: { + name: paramName, + }, + }).length; + + return numAssignments > 0 || numUpdated > 0; + }); + + if (reassignedParamNames.length === 0) { + return; + } + + reassignedParamNames.forEach(paramName => { + const oldName = paramName; + const newName = getNewName(paramName); + const localVar = getLocalVarStatement(paramName, newName); + + if (definedInParentScope(newName, func.scope)) { + return; + } + + j(func.get('body')) + .find(j.Identifier, {name: paramName}) + .forEach(identifier => { + const parent = identifier.parent.node; + + if ( + j.MemberExpression.check(parent) && + parent.property === identifier.node && + !parent.computed + ) { + // obj.oldName + return; + } + + if ( + j.Property.check(parent) && + parent.key === identifier.node && + !parent.computed + ) { + // { oldName: 3 } + + const closestAssignment = j(identifier).closest( + j.AssignmentExpression, + ); + const assignmentHasProperty = + closestAssignment.filter(assignment => { + return ( + j.ObjectPattern.check(assignment.node.left) && + assignment.node.left.properties.includes(parent) + ); + }).length > 0; + + if (!assignmentHasProperty) { + // ({oldName} = x); + return; + } + } + + if ( + j.MethodDefinition.check(parent) && + parent.key === identifier.node && + !parent.computed + ) { + // class A { oldName() {} } + return; + } + + if (j.JSXAttribute.check(parent)) { + // + return; + } + + let scope = identifier.scope; + + if (scope === functionScope) { + const bindings = scope.getBindings()[oldName]; + if (bindings) { + const recentBinding = bindings[bindings.length - 1]; + if (recentBinding.name === 'id') { + return; + } + } + } else { + while (scope !== functionScope) { + if (scope.declares(oldName)) { + return; + } + + scope = scope.parent; + } + } + + if (scope) { + newBindings.add(localVar); + + // ObjectPattern + if (identifier.parent && j.Property.check(identifier.parent.node)) { + const property = identifier.parent; + property.shorthand = false; + property.get('shorthand').replace(false); + property + .get('value') + .get('name') + .replace(newName); + } else { + identifier.get('name').replace(newName); + } + } + }); + }); + + const newBindingStatements = Array.from(newBindings).reverse(); + newBindingStatements.forEach(binding => { + updated = true; + functionScope.node.body.body.unshift(binding); + }); + } + + // Facebook has generated files with an annotation. We don't want to modify these. + // Instead, we should modify the code that generates the files. + // eslint-disable-next-line no-useless-concat + if (file.source.includes('@' + 'generated')) { + return null; + } + + const root = j(file.source); + + FUNCTION_TYPES.forEach(type => { + root.find(type).forEach(updateFunction); + }); + + if (updated) { + return root.toSource({quote: 'single'}); + } + + return null; +};