diff --git a/packages/eslint-plugin-pf-codemods/lib/helpers.js b/packages/eslint-plugin-pf-codemods/lib/helpers.js index 44fa3f56d..3cfd4b60d 100644 --- a/packages/eslint-plugin-pf-codemods/lib/helpers.js +++ b/packages/eslint-plugin-pf-codemods/lib/helpers.js @@ -227,6 +227,8 @@ function ensureImports(context, node, package, imports) { } } +// propMap structure: { propName: { defaultParamName: string, previousParamIndex?: number, otherMatchers?: /regex/ } | string } +// example: { onClick: { defaultParamName: '_event', previousParamIndex: 1, otherMatchers?: /_?(event|ev|e)/ } } function addCallbackParam(componentsArray, propMap) { return function (context) { const imports = getPackageImports(context, "@patternfly/react-core").filter( @@ -243,7 +245,7 @@ function addCallbackParam(componentsArray, propMap) { ); namedAttributes.forEach((attribute) => { - const newParam = propMap[attribute.name.name]; + let newParam; const propProperties = { type: attribute.value?.expression?.type, @@ -268,6 +270,39 @@ function addCallbackParam(componentsArray, propMap) { } const { type, params } = propProperties; + // if a simple string is passed for the parameter just assign it to newParam like we used to and skip everything else + if (typeof propMap[attribute.name.name] === "string") { + newParam = propMap[attribute.name.name]; + } else { + const { + defaultParamName, + previousParamIndex, + otherMatchers, + } = propMap[attribute.name.name]; + + const paramNameAtGivenIndex = + params && params[previousParamIndex]?.name; + + // if the expected index of the newParam exceeds the number of current params just set treat it like a param addition with the default param value + if (previousParamIndex >= params?.length) { + newParam = defaultParamName; + } + + // if there is a param in the location where we expect the newParam to be, and it matches the supplied matcher, treat that matched value as the new param value + else if (paramNameAtGivenIndex?.match(otherMatchers)) { + newParam = paramNameAtGivenIndex; + } + + // if it doesn't match the supplied matcher, early return with no fixer + else { + context.report({ + node, + message: `The "${attribute.name.name}" prop for ${node.name.name} has been updated so that the "${defaultParamName}" parameter is the first parameter. "${attribute.name.name}" handlers may require an update.`, + }); + return; + } + } + if ( (params?.length >= 1 && ["ArrowFunctionExpression", "Identifier"].includes(type)) || @@ -278,26 +313,58 @@ function addCallbackParam(componentsArray, propMap) { message: `The "${attribute.name.name}" prop for ${node.name.name} has been updated so that the "${newParam}" parameter is the first parameter. "${attribute.name.name}" handlers may require an update.`, fix(fixer) { const fixes = []; - const createReplacerFix = (functionParam) => { + + const createParamAdditionFix = (params) => { + const firstParam = params[0]; + + const replacementParams = `${newParam}, ${context + .getSourceCode() + .getText(firstParam)}`; + const hasParenthesis = - context.getTokenAfter(functionParam).value === ")"; - const replacementParams = `${newParam}, ${functionParam.name}`; + context.getTokenBefore(firstParam).value === "("; return fixer.replaceText( - functionParam, + firstParam, hasParenthesis ? replacementParams : `(${replacementParams})` ); }; + const createRemoveCurrentParamUseFix = ( + currentUseOfNewParam + ) => { + const targetRange = [ + currentUseOfNewParam.range[0] - 2, + currentUseOfNewParam.range[1], + ]; + + return fixer.replaceTextRange(targetRange, ""); + }; + if ( - ["ArrowFunctionExpression", "Identifier"].includes( + !["ArrowFunctionExpression", "Identifier"].includes( type - ) && - params.length === 1 + ) ) { - fixes.push(createReplacerFix(params[0])); + return fixes; + } + + const currentIndexOfNewParam = params?.findIndex( + (param) => param.name === newParam + ); + + if (currentIndexOfNewParam > 0) { + const currentUseOfNewParam = + params[currentIndexOfNewParam]; + + fixes.push( + createRemoveCurrentParamUseFix(currentUseOfNewParam) + ); + fixes.push(createParamAdditionFix(params)); + } else if (params[0].name !== newParam) { + fixes.push(createParamAdditionFix(params)); } return fixes; diff --git a/packages/eslint-plugin-pf-codemods/lib/rules/v5/dataListCheck-updated-callback.js b/packages/eslint-plugin-pf-codemods/lib/rules/v5/dataListCheck-updated-callback.js index da70cfd98..39efdb2ee 100644 --- a/packages/eslint-plugin-pf-codemods/lib/rules/v5/dataListCheck-updated-callback.js +++ b/packages/eslint-plugin-pf-codemods/lib/rules/v5/dataListCheck-updated-callback.js @@ -3,5 +3,5 @@ const { addCallbackParam } = require("../../helpers"); // https://github.com/patternfly/patternfly-react/pull/8735 module.exports = { meta: { fixable: "code" }, - create: addCallbackParam(["DataListCheck"], { onChange: "_event" }), + create: addCallbackParam(["DataListCheck"], { onChange: { defaultParamName: "_event", previousParamIndex: 1, otherMatchers: /^_?(ev\w*|e$)/ } }), }; diff --git a/packages/eslint-plugin-pf-codemods/test/rules/v5/dataListCheck-updated-callback.js b/packages/eslint-plugin-pf-codemods/test/rules/v5/dataListCheck-updated-callback.js index bf5579bf5..fc12803a7 100644 --- a/packages/eslint-plugin-pf-codemods/test/rules/v5/dataListCheck-updated-callback.js +++ b/packages/eslint-plugin-pf-codemods/test/rules/v5/dataListCheck-updated-callback.js @@ -1,3 +1,3 @@ -const { addCallbackParamTester } = require("../../testHelpers"); +const { swapCallbackParamTester } = require("../../testHelpers"); -addCallbackParamTester('dataListCheck-updated-callback', 'DataListCheck', 'onChange') +swapCallbackParamTester('dataListCheck-updated-callback', 'DataListCheck', 'onChange', 1) diff --git a/packages/eslint-plugin-pf-codemods/test/testHelpers.js b/packages/eslint-plugin-pf-codemods/test/testHelpers.js index e0baa68ac..150daf330 100644 --- a/packages/eslint-plugin-pf-codemods/test/testHelpers.js +++ b/packages/eslint-plugin-pf-codemods/test/testHelpers.js @@ -1,7 +1,7 @@ const ruleTester = require("./ruletester"); function getAddCallbackParamMessage(componentName, propName, newParamName) { - return `The "${propName}" prop for ${componentName} has been updated so that the "${newParamName}" parameter is the first parameter. "${propName}" handlers may require an update.` + return `The "${propName}" prop for ${componentName} has been updated so that the "${newParamName}" parameter is the first parameter. "${propName}" handlers may require an update.`; } function getValidAddCallbackParamTests(componentNameArray, propNameArray) { @@ -27,7 +27,12 @@ function getValidAddCallbackParamTests(componentNameArray, propNameArray) { return tests; } -function getInvalidAddCallbackParamTests(componentNameArray, propNameArray, newParamName, getCustomMessage) { +function getInvalidAddCallbackParamTests( + componentNameArray, + propNameArray, + newParamName, + previousParamIndex +) { let tests = []; componentNameArray.forEach((componentName) => { @@ -37,7 +42,25 @@ function getInvalidAddCallbackParamTests(componentNameArray, propNameArray, newP output: `import { ${componentName} } from '@patternfly/react-core'; <${componentName} ${propName}={(${newParamName}, id) => handler(id)} />;`, errors: [ { - message: getAddCallbackParamMessage(componentName, propName, newParamName), + message: getAddCallbackParamMessage( + componentName, + propName, + newParamName + ), + type: "JSXOpeningElement", + }, + ], + }); + tests.push({ + code: `import { ${componentName} } from '@patternfly/react-core'; <${componentName} ${propName}={(id: bar) => handler(id)} />;`, + output: `import { ${componentName} } from '@patternfly/react-core'; <${componentName} ${propName}={(${newParamName}, id: bar) => handler(id)} />;`, + errors: [ + { + message: getAddCallbackParamMessage( + componentName, + propName, + newParamName + ), type: "JSXOpeningElement", }, ], @@ -47,7 +70,11 @@ function getInvalidAddCallbackParamTests(componentNameArray, propNameArray, newP output: `import { ${componentName} } from '@patternfly/react-core'; <${componentName} ${propName}={(${newParamName}, id) => handler(id)} />;`, errors: [ { - message: getAddCallbackParamMessage(componentName, propName, newParamName), + message: getAddCallbackParamMessage( + componentName, + propName, + newParamName + ), type: "JSXOpeningElement", }, ], @@ -57,7 +84,25 @@ function getInvalidAddCallbackParamTests(componentNameArray, propNameArray, newP output: `import { ${componentName} } from '@patternfly/react-core'; const handler = (${newParamName}, id) => {}; <${componentName} ${propName}={handler} />;`, errors: [ { - message: getAddCallbackParamMessage(componentName, propName, newParamName), + message: getAddCallbackParamMessage( + componentName, + propName, + newParamName + ), + type: "JSXOpeningElement", + }, + ], + }); + tests.push({ + code: `import { ${componentName} } from '@patternfly/react-core'; const handler = (id: bar) => {}; <${componentName} ${propName}={handler} />;`, + output: `import { ${componentName} } from '@patternfly/react-core'; const handler = (${newParamName}, id: bar) => {}; <${componentName} ${propName}={handler} />;`, + errors: [ + { + message: getAddCallbackParamMessage( + componentName, + propName, + newParamName + ), type: "JSXOpeningElement", }, ], @@ -67,7 +112,25 @@ function getInvalidAddCallbackParamTests(componentNameArray, propNameArray, newP output: `import { ${componentName} } from '@patternfly/react-core'; function handler(${newParamName}, id) {}; <${componentName} ${propName}={handler} />;`, errors: [ { - message: getAddCallbackParamMessage(componentName, propName, newParamName), + message: getAddCallbackParamMessage( + componentName, + propName, + newParamName + ), + type: "JSXOpeningElement", + }, + ], + }); + tests.push({ + code: `import { ${componentName} } from '@patternfly/react-core'; function handler(id: bar) {}; <${componentName} ${propName}={handler} />;`, + output: `import { ${componentName} } from '@patternfly/react-core'; function handler(${newParamName}, id: bar) {}; <${componentName} ${propName}={handler} />;`, + errors: [ + { + message: getAddCallbackParamMessage( + componentName, + propName, + newParamName + ), type: "JSXOpeningElement", }, ], @@ -77,7 +140,11 @@ function getInvalidAddCallbackParamTests(componentNameArray, propNameArray, newP output: `import { ${componentName} } from '@patternfly/react-core'; <${componentName} ${propName}={this.handler} />;`, errors: [ { - message: getAddCallbackParamMessage(componentName, propName, newParamName), + message: getAddCallbackParamMessage( + componentName, + propName, + newParamName + ), type: "JSXOpeningElement", }, ], @@ -87,29 +154,204 @@ function getInvalidAddCallbackParamTests(componentNameArray, propNameArray, newP output: `import { ${componentName} as PF${componentName} } from '@patternfly/react-core'; handler(id)} />;`, errors: [ { - message: getAddCallbackParamMessage(`PF${componentName}`, propName, newParamName), + message: getAddCallbackParamMessage( + `PF${componentName}`, + propName, + newParamName + ), type: "JSXOpeningElement", }, ], }); + tests.push({ + code: `import { ${componentName} as PF${componentName} } from '@patternfly/react-core'; handler(id)} />;`, + output: `import { ${componentName} as PF${componentName} } from '@patternfly/react-core'; handler(id)} />;`, + errors: [ + { + message: getAddCallbackParamMessage( + `PF${componentName}`, + propName, + newParamName + ), + type: "JSXOpeningElement", + }, + ], + }); + + // only add these tests if this is not being used as part of the test suite for a param swap, or if the swapped param would come after the 'text' param in the below tests + if (!previousParamIndex || previousParamIndex > 1) { + tests.push({ + code: `import { ${componentName} } from '@patternfly/react-core'; <${componentName} ${propName}={(id, text) => handler(id)} />;`, + output: `import { ${componentName} } from '@patternfly/react-core'; <${componentName} ${propName}={(${newParamName}, id, text) => handler(id)} />;`, + errors: [ + { + message: getAddCallbackParamMessage( + componentName, + propName, + newParamName + ), + type: "JSXOpeningElement", + }, + ], + }); + tests.push({ + code: `import { ${componentName} } from '@patternfly/react-core'; const handler = (id, text) => {}; <${componentName} ${propName}={handler} />;`, + output: `import { ${componentName} } from '@patternfly/react-core'; const handler = (${newParamName}, id, text) => {}; <${componentName} ${propName}={handler} />;`, + errors: [ + { + message: getAddCallbackParamMessage( + componentName, + propName, + newParamName + ), + type: "JSXOpeningElement", + }, + ], + }); + tests.push({ + code: `import { ${componentName} } from '@patternfly/react-core'; function handler(id, text) {}; <${componentName} ${propName}={handler} />;`, + output: `import { ${componentName} } from '@patternfly/react-core'; function handler(${newParamName}, id, text) {}; <${componentName} ${propName}={handler} />;`, + errors: [ + { + message: getAddCallbackParamMessage( + componentName, + propName, + newParamName + ), + type: "JSXOpeningElement", + }, + ], + }); + tests.push({ + code: `import { ${componentName} as PF${componentName} } from '@patternfly/react-core'; handler(id)} />;`, + output: `import { ${componentName} as PF${componentName} } from '@patternfly/react-core'; handler(id)} />;`, + errors: [ + { + message: getAddCallbackParamMessage( + `PF${componentName}`, + propName, + newParamName + ), + type: "JSXOpeningElement", + }, + ], + }); + } }); }); return tests; } -function addCallbackParamTester(ruleName, componentNames, propNames, newParamName = '_event') { +function getInvalidSwapCallbackParamTests( + componentNameArray, + propNameArray, + previousParamIndex, + newParamName +) { + let tests = getInvalidAddCallbackParamTests( + componentNameArray, + propNameArray, + newParamName, + previousParamIndex + ); + + const formattedNewParamName = + newParamName[0] === "_" ? newParamName.slice(1) : newParamName; + + const expectedVariations = []; + const genericArgs = ["id", "text", "foo", "bar", "bash", "bang"]; + + switch (formattedNewParamName) { + case "event": + expectedVariations.push( + ...["e", "_e", "ev", "_ev", "evt", "_evt", "event", "_event"] + ); + break; + } + + if (expectedVariations.length) { + componentNameArray.forEach((componentName) => { + propNameArray.forEach((propName) => { + expectedVariations.forEach((variation) => { + const stringifyArgs = (args) => { + return args.reduce((acc, arg) => `${acc}, ${arg}`, "").slice(2); + }; + + const selectedGenericArgs = genericArgs.slice(0, previousParamIndex); + + const initialArgs = stringifyArgs([ + ...selectedGenericArgs, + variation, + ]); + const fixedArgs = stringifyArgs([variation, ...selectedGenericArgs]); + + tests.push({ + code: `import { ${componentName} } from '@patternfly/react-core'; function handler(${initialArgs}) {}; <${componentName} ${propName}={handler} />;`, + output: `import { ${componentName} } from '@patternfly/react-core'; function handler(${fixedArgs}) {}; <${componentName} ${propName}={handler} />;`, + errors: [ + { + message: getAddCallbackParamMessage( + componentName, + propName, + variation + ), + type: "JSXOpeningElement", + }, + ], + }); + }); + }); + }); + } + + return tests; +} + +function addCallbackParamTester( + ruleName, + componentNames, + propNames, + newParamName = "_event" +) { + const rule = require(`../lib/rules/v5/${ruleName}`); + const componentNameArray = + typeof componentNames === "string" ? [componentNames] : componentNames; + const propNameArray = typeof propNames === "string" ? [propNames] : propNames; + + ruleTester.run(ruleName, rule, { + valid: getValidAddCallbackParamTests(componentNameArray, propNameArray), + invalid: getInvalidAddCallbackParamTests( + componentNameArray, + propNameArray, + newParamName + ), + }); +} + +function swapCallbackParamTester( + ruleName, + componentNames, + propNames, + previousParamIndex, + newParamName = "_event" +) { const rule = require(`../lib/rules/v5/${ruleName}`); const componentNameArray = typeof componentNames === "string" ? [componentNames] : componentNames; - const propNameArray = - typeof propNames === "string" ? [propNames] : propNames; + const propNameArray = typeof propNames === "string" ? [propNames] : propNames; ruleTester.run(ruleName, rule, { valid: getValidAddCallbackParamTests(componentNameArray, propNameArray), - invalid: getInvalidAddCallbackParamTests(componentNameArray, propNameArray, newParamName), + invalid: getInvalidSwapCallbackParamTests( + componentNameArray, + propNameArray, + previousParamIndex, + newParamName + ), }); } module.exports = { - addCallbackParamTester + addCallbackParamTester, + swapCallbackParamTester, }; diff --git a/test/test.tsx b/test/test.tsx index d983f4761..1dc50722c 100644 --- a/test/test.tsx +++ b/test/test.tsx @@ -60,8 +60,8 @@ const newTheme = getCustomTheme("1", "2", "3");