From 7b1216bab7cbaca27fda44dd103becc970e141f4 Mon Sep 17 00:00:00 2001 From: Austin Sullivan Date: Mon, 13 Mar 2023 13:05:57 -0400 Subject: [PATCH 1/9] chore(helpers): add swap param support to param adding helper --- .../eslint-plugin-pf-codemods/lib/helpers.js | 50 +++++- .../v5/dataListCheck-updated-callback.js | 4 +- .../test/testHelpers.js | 144 ++++++++++++++++-- 3 files changed, 179 insertions(+), 19 deletions(-) diff --git a/packages/eslint-plugin-pf-codemods/lib/helpers.js b/packages/eslint-plugin-pf-codemods/lib/helpers.js index 44fa3f56d..0b0a81e11 100644 --- a/packages/eslint-plugin-pf-codemods/lib/helpers.js +++ b/packages/eslint-plugin-pf-codemods/lib/helpers.js @@ -291,16 +291,58 @@ function addCallbackParam(componentsArray, propMap) { ); }; + const isNewParam = (param) => { + if (newParam[0] === "_") { + return [newParam, newParam.slice(1)].includes(param.name); + } + + return [newParam, "_" + newParam].includes(param.name) + }; + + const createSwapFix = (param) => { + const isFirst = + context.getTokenBefore(param).value === "("; + + if (isNewParam(param) && isFirst) { + return; + } + + if (isNewParam(param)) { + return fixer.replaceText( + { + ...param, + range: [param.range[0] - 2, param.range[1]], + }, + "" + ); + } + + if (isFirst) { + return fixer.replaceText( + param, + `${newParam}, ${param.name}` + ); + } + }; + if ( - ["ArrowFunctionExpression", "Identifier"].includes( + !["ArrowFunctionExpression", "Identifier"].includes( type - ) && - params.length === 1 + ) ) { + return fixes; + } + + if (params.length === 1) { fixes.push(createReplacerFix(params[0])); + } else if (params?.some((param) => isNewParam(param))) { + params.forEach((param) => + fixes.push(createSwapFix(param)) + ); } - return fixes; + // remove any fixes that aren't actually fixes (i.e. undefined) as those will break eslint + return fixes.filter((fix) => fix?.range); }, }); } 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..ca2741367 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') diff --git a/packages/eslint-plugin-pf-codemods/test/testHelpers.js b/packages/eslint-plugin-pf-codemods/test/testHelpers.js index e0baa68ac..d95683168 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,11 @@ function getValidAddCallbackParamTests(componentNameArray, propNameArray) { return tests; } -function getInvalidAddCallbackParamTests(componentNameArray, propNameArray, newParamName, getCustomMessage) { +function getInvalidAddCallbackParamTests( + componentNameArray, + propNameArray, + newParamName +) { let tests = []; componentNameArray.forEach((componentName) => { @@ -37,7 +41,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", }, ], @@ -47,7 +55,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 +69,11 @@ 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", }, ], @@ -67,7 +83,11 @@ 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", }, ], @@ -77,7 +97,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,7 +111,11 @@ 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", }, ], @@ -97,19 +125,109 @@ function getInvalidAddCallbackParamTests(componentNameArray, propNameArray, newP return tests; } -function addCallbackParamTester(ruleName, componentNames, propNames, newParamName = '_event') { +function getInvalidSwapCallbackParamTests( + componentNameArray, + propNameArray, + newParamName +) { + let tests = getInvalidAddCallbackParamTests( + componentNameArray, + propNameArray, + newParamName + ); + + componentNameArray.forEach((componentName) => { + propNameArray.forEach((propName) => { + tests.push({ + code: `import { ${componentName} } from '@patternfly/react-core'; <${componentName} ${propName}={(id, event) => handler(id)} />;`, + output: `import { ${componentName} } from '@patternfly/react-core'; <${componentName} ${propName}={(${newParamName}, id) => handler(id)} />;`, + errors: [ + { + message: getAddCallbackParamMessage( + componentName, + propName, + newParamName + ), + type: "JSXOpeningElement", + }, + ], + }); + tests.push({ + code: `import { ${componentName} } from '@patternfly/react-core'; const handler = (id, event) => {}; <${componentName} ${propName}={handler} />;`, + output: `import { ${componentName} } from '@patternfly/react-core'; const handler = (${newParamName}, id) => {}; <${componentName} ${propName}={handler} />;`, + errors: [ + { + message: getAddCallbackParamMessage( + componentName, + propName, + newParamName + ), + type: "JSXOpeningElement", + }, + ], + }); + tests.push({ + code: `import { ${componentName} } from '@patternfly/react-core'; function handler(id, event) {}; <${componentName} ${propName}={handler} />;`, + output: `import { ${componentName} } from '@patternfly/react-core'; function handler(${newParamName}, id) {}; <${componentName} ${propName}={handler} />;`, + errors: [ + { + message: getAddCallbackParamMessage( + componentName, + propName, + newParamName + ), + 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, + 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, + newParamName + ), }); } module.exports = { - addCallbackParamTester + addCallbackParamTester, + swapCallbackParamTester, }; From 76ad1c285f1de39a86fbc9d64814d07751d88d27 Mon Sep 17 00:00:00 2001 From: Austin Sullivan Date: Mon, 13 Mar 2023 13:46:24 -0400 Subject: [PATCH 2/9] chore(helpers): add support for addition to multi params --- .../eslint-plugin-pf-codemods/lib/helpers.js | 58 ++++++---- .../test/testHelpers.js | 104 +++++++++++++++++- 2 files changed, 138 insertions(+), 24 deletions(-) diff --git a/packages/eslint-plugin-pf-codemods/lib/helpers.js b/packages/eslint-plugin-pf-codemods/lib/helpers.js index 0b0a81e11..c46f58455 100644 --- a/packages/eslint-plugin-pf-codemods/lib/helpers.js +++ b/packages/eslint-plugin-pf-codemods/lib/helpers.js @@ -278,36 +278,52 @@ 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 getIsNewParam = (param) => { + if (param.name === newParam) { + return true; + } + + if (newParam[0] === "_") { + return param.name === newParam.slice(1); + } + + return param.name === "_" + newParam; + }; + + const getIsFirst = (param) => + context.getTokenBefore(param).value === "("; + + const createParamAdditionFix = (params) => { + const firstParam = params[0]; + if (params.length > 1) { + return fixer.replaceText( + firstParam, + `${newParam}, ${firstParam.name}` + ); + } + const hasParenthesis = - context.getTokenAfter(functionParam).value === ")"; - const replacementParams = `${newParam}, ${functionParam.name}`; + context.getTokenAfter(firstParam).value === ")"; + const replacementParams = `${newParam}, ${firstParam.name}`; return fixer.replaceText( - functionParam, + firstParam, hasParenthesis ? replacementParams : `(${replacementParams})` ); }; - const isNewParam = (param) => { - if (newParam[0] === "_") { - return [newParam, newParam.slice(1)].includes(param.name); - } - - return [newParam, "_" + newParam].includes(param.name) - }; - - const createSwapFix = (param) => { - const isFirst = - context.getTokenBefore(param).value === "("; + const createParamSwapFix = (param) => { + const isNew = getIsNewParam(param); + const isFirst = getIsFirst(param); - if (isNewParam(param) && isFirst) { + if (isNew && isFirst) { return; } - if (isNewParam(param)) { + if (isNew) { return fixer.replaceText( { ...param, @@ -333,12 +349,12 @@ function addCallbackParam(componentsArray, propMap) { return fixes; } - if (params.length === 1) { - fixes.push(createReplacerFix(params[0])); - } else if (params?.some((param) => isNewParam(param))) { + if (params?.some((param) => getIsNewParam(param))) { params.forEach((param) => - fixes.push(createSwapFix(param)) + fixes.push(createParamSwapFix(param)) ); + } else { + fixes.push(createParamAdditionFix(params)); } // remove any fixes that aren't actually fixes (i.e. undefined) as those will break eslint diff --git a/packages/eslint-plugin-pf-codemods/test/testHelpers.js b/packages/eslint-plugin-pf-codemods/test/testHelpers.js index d95683168..f63e9867a 100644 --- a/packages/eslint-plugin-pf-codemods/test/testHelpers.js +++ b/packages/eslint-plugin-pf-codemods/test/testHelpers.js @@ -50,6 +50,20 @@ function getInvalidAddCallbackParamTests( }, ], }); + 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'; <${componentName} ${propName}={id => handler(id)} />;`, output: `import { ${componentName} } from '@patternfly/react-core'; <${componentName} ${propName}={(${newParamName}, id) => handler(id)} />;`, @@ -78,6 +92,20 @@ function getInvalidAddCallbackParamTests( }, ], }); + 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) {}; <${componentName} ${propName}={handler} />;`, output: `import { ${componentName} } from '@patternfly/react-core'; function handler(${newParamName}, id) {}; <${componentName} ${propName}={handler} />;`, @@ -92,6 +120,20 @@ function getInvalidAddCallbackParamTests( }, ], }); + 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} } from '@patternfly/react-core'; <${componentName} ${propName}={this.handler} />;`, output: `import { ${componentName} } from '@patternfly/react-core'; <${componentName} ${propName}={this.handler} />;`, @@ -120,6 +162,20 @@ function getInvalidAddCallbackParamTests( }, ], }); + 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; @@ -139,7 +195,7 @@ function getInvalidSwapCallbackParamTests( componentNameArray.forEach((componentName) => { propNameArray.forEach((propName) => { tests.push({ - code: `import { ${componentName} } from '@patternfly/react-core'; <${componentName} ${propName}={(id, event) => handler(id)} />;`, + code: `import { ${componentName} } from '@patternfly/react-core'; <${componentName} ${propName}={(id, ${newParamName}) => handler(id)} />;`, output: `import { ${componentName} } from '@patternfly/react-core'; <${componentName} ${propName}={(${newParamName}, id) => handler(id)} />;`, errors: [ { @@ -153,7 +209,21 @@ function getInvalidSwapCallbackParamTests( ], }); tests.push({ - code: `import { ${componentName} } from '@patternfly/react-core'; const handler = (id, event) => {}; <${componentName} ${propName}={handler} />;`, + code: `import { ${componentName} } from '@patternfly/react-core'; <${componentName} ${propName}={(id, ${newParamName}, 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, ${newParamName}) => {}; <${componentName} ${propName}={handler} />;`, output: `import { ${componentName} } from '@patternfly/react-core'; const handler = (${newParamName}, id) => {}; <${componentName} ${propName}={handler} />;`, errors: [ { @@ -167,7 +237,21 @@ function getInvalidSwapCallbackParamTests( ], }); tests.push({ - code: `import { ${componentName} } from '@patternfly/react-core'; function handler(id, event) {}; <${componentName} ${propName}={handler} />;`, + code: `import { ${componentName} } from '@patternfly/react-core'; const handler = (id, ${newParamName}, 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, ${newParamName}) {}; <${componentName} ${propName}={handler} />;`, output: `import { ${componentName} } from '@patternfly/react-core'; function handler(${newParamName}, id) {}; <${componentName} ${propName}={handler} />;`, errors: [ { @@ -180,6 +264,20 @@ function getInvalidSwapCallbackParamTests( }, ], }); + tests.push({ + code: `import { ${componentName} } from '@patternfly/react-core'; function handler(id, ${newParamName}, 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", + }, + ], + }); }); }); return tests; From b61d29c3111de785dc62a0c233bd384d62185702 Mon Sep 17 00:00:00 2001 From: Austin Sullivan Date: Mon, 13 Mar 2023 15:55:23 -0400 Subject: [PATCH 3/9] chore(helpers): add support for preserving leading _ usage --- .../eslint-plugin-pf-codemods/lib/helpers.js | 63 +++++++--- .../test/testHelpers.js | 110 ++++++++++++++++-- 2 files changed, 148 insertions(+), 25 deletions(-) diff --git a/packages/eslint-plugin-pf-codemods/lib/helpers.js b/packages/eslint-plugin-pf-codemods/lib/helpers.js index c46f58455..ba69ea66e 100644 --- a/packages/eslint-plugin-pf-codemods/lib/helpers.js +++ b/packages/eslint-plugin-pf-codemods/lib/helpers.js @@ -279,12 +279,14 @@ function addCallbackParam(componentsArray, propMap) { fix(fixer) { const fixes = []; + const newParamHasUnderscore = newParam[0] === "_"; + const getIsNewParam = (param) => { if (param.name === newParam) { return true; } - if (newParam[0] === "_") { + if (newParamHasUnderscore) { return param.name === newParam.slice(1); } @@ -294,6 +296,25 @@ function addCallbackParam(componentsArray, propMap) { const getIsFirst = (param) => context.getTokenBefore(param).value === "("; + // meant to preserve _ being used to signal that the param isn't being used (or not) + const formatNewParam = (paramNameInCurrentUse) => { + const currentUseOfNewParamHasUnderscore = + paramNameInCurrentUse[0] === "_"; + + if ( + newParamHasUnderscore === + currentUseOfNewParamHasUnderscore + ) { + return newParam; + } + + if (newParamHasUnderscore) { + return newParam.slice(1); + } + + return "_" + newParam; + }; + const createParamAdditionFix = (params) => { const firstParam = params[0]; if (params.length > 1) { @@ -315,15 +336,25 @@ function addCallbackParam(componentsArray, propMap) { ); }; - const createParamSwapFix = (param) => { - const isNew = getIsNewParam(param); + const createParamSwapFix = ( + param, + currentUseOfNewParam + ) => { + const isCurrentUseOfNewParam = param === currentUseOfNewParam; const isFirst = getIsFirst(param); - if (isNew && isFirst) { + // guard clause for if the new param is already the first, prevents issues if the swap doesn't need to be done + if (isCurrentUseOfNewParam && isFirst) { return; } - if (isNew) { + // guard clause for if the param being evaluated is neither the param to be swapped or the first, since in that case we don't need to do anything with it + if (!isCurrentUseOfNewParam && !isFirst) { + return; + } + + if (isCurrentUseOfNewParam) { + // the range manipulation here is to remove the leading comma and space, since we know the param in question isn't the first at this point return fixer.replaceText( { ...param, @@ -333,12 +364,12 @@ function addCallbackParam(componentsArray, propMap) { ); } - if (isFirst) { - return fixer.replaceText( - param, - `${newParam}, ${param.name}` - ); - } + return fixer.replaceText( + param, + `${formatNewParam(currentUseOfNewParam.name)}, ${ + param.name + }` + ); }; if ( @@ -349,9 +380,15 @@ function addCallbackParam(componentsArray, propMap) { return fixes; } - if (params?.some((param) => getIsNewParam(param))) { + const currentUseOfNewParam = params?.find((param) => + getIsNewParam(param) + ); + + if (currentUseOfNewParam) { params.forEach((param) => - fixes.push(createParamSwapFix(param)) + fixes.push( + createParamSwapFix(param, currentUseOfNewParam) + ) ); } else { fixes.push(createParamAdditionFix(params)); diff --git a/packages/eslint-plugin-pf-codemods/test/testHelpers.js b/packages/eslint-plugin-pf-codemods/test/testHelpers.js index f63e9867a..3c38123d3 100644 --- a/packages/eslint-plugin-pf-codemods/test/testHelpers.js +++ b/packages/eslint-plugin-pf-codemods/test/testHelpers.js @@ -192,11 +192,13 @@ function getInvalidSwapCallbackParamTests( newParamName ); + const formattedNewParamName = newParamName[0] === '_' ? newParamName.slice(1) : newParamName; + componentNameArray.forEach((componentName) => { propNameArray.forEach((propName) => { tests.push({ - code: `import { ${componentName} } from '@patternfly/react-core'; <${componentName} ${propName}={(id, ${newParamName}) => handler(id)} />;`, - output: `import { ${componentName} } from '@patternfly/react-core'; <${componentName} ${propName}={(${newParamName}, id) => handler(id)} />;`, + code: `import { ${componentName} } from '@patternfly/react-core'; <${componentName} ${propName}={(id, _${formattedNewParamName}) => handler(id)} />;`, + output: `import { ${componentName} } from '@patternfly/react-core'; <${componentName} ${propName}={(_${formattedNewParamName}, id) => handler(id)} />;`, errors: [ { message: getAddCallbackParamMessage( @@ -209,8 +211,8 @@ function getInvalidSwapCallbackParamTests( ], }); tests.push({ - code: `import { ${componentName} } from '@patternfly/react-core'; <${componentName} ${propName}={(id, ${newParamName}, text) => handler(id)} />;`, - output: `import { ${componentName} } from '@patternfly/react-core'; <${componentName} ${propName}={(${newParamName}, id, text) => handler(id)} />;`, + code: `import { ${componentName} } from '@patternfly/react-core'; <${componentName} ${propName}={(id, ${formattedNewParamName}) => handler(id, ${formattedNewParamName})} />;`, + output: `import { ${componentName} } from '@patternfly/react-core'; <${componentName} ${propName}={(${formattedNewParamName}, id) => handler(id, ${formattedNewParamName})} />;`, errors: [ { message: getAddCallbackParamMessage( @@ -223,8 +225,8 @@ function getInvalidSwapCallbackParamTests( ], }); tests.push({ - code: `import { ${componentName} } from '@patternfly/react-core'; const handler = (id, ${newParamName}) => {}; <${componentName} ${propName}={handler} />;`, - output: `import { ${componentName} } from '@patternfly/react-core'; const handler = (${newParamName}, id) => {}; <${componentName} ${propName}={handler} />;`, + code: `import { ${componentName} } from '@patternfly/react-core'; <${componentName} ${propName}={(id, _${formattedNewParamName}, text) => handler(id, text)} />;`, + output: `import { ${componentName} } from '@patternfly/react-core'; <${componentName} ${propName}={(_${formattedNewParamName}, id, text) => handler(id, text)} />;`, errors: [ { message: getAddCallbackParamMessage( @@ -237,8 +239,8 @@ function getInvalidSwapCallbackParamTests( ], }); tests.push({ - code: `import { ${componentName} } from '@patternfly/react-core'; const handler = (id, ${newParamName}, text) => {}; <${componentName} ${propName}={handler} />;`, - output: `import { ${componentName} } from '@patternfly/react-core'; const handler = (${newParamName}, id, text) => {}; <${componentName} ${propName}={handler} />;`, + code: `import { ${componentName} } from '@patternfly/react-core'; <${componentName} ${propName}={(id, ${formattedNewParamName}, text) => handler(id, ${formattedNewParamName}, text)} />;`, + output: `import { ${componentName} } from '@patternfly/react-core'; <${componentName} ${propName}={(${formattedNewParamName}, id, text) => handler(id, ${formattedNewParamName}, text)} />;`, errors: [ { message: getAddCallbackParamMessage( @@ -251,8 +253,8 @@ function getInvalidSwapCallbackParamTests( ], }); tests.push({ - code: `import { ${componentName} } from '@patternfly/react-core'; function handler(id, ${newParamName}) {}; <${componentName} ${propName}={handler} />;`, - output: `import { ${componentName} } from '@patternfly/react-core'; function handler(${newParamName}, id) {}; <${componentName} ${propName}={handler} />;`, + code: `import { ${componentName} } from '@patternfly/react-core'; const handler = (id, _${formattedNewParamName}) => {}; <${componentName} ${propName}={handler} />;`, + output: `import { ${componentName} } from '@patternfly/react-core'; const handler = (_${formattedNewParamName}, id) => {}; <${componentName} ${propName}={handler} />;`, errors: [ { message: getAddCallbackParamMessage( @@ -265,8 +267,92 @@ function getInvalidSwapCallbackParamTests( ], }); tests.push({ - code: `import { ${componentName} } from '@patternfly/react-core'; function handler(id, ${newParamName}, text) {}; <${componentName} ${propName}={handler} />;`, - output: `import { ${componentName} } from '@patternfly/react-core'; function handler(${newParamName}, id, text) {}; <${componentName} ${propName}={handler} />;`, + code: `import { ${componentName} } from '@patternfly/react-core'; const handler = (id, ${formattedNewParamName}) => {}; <${componentName} ${propName}={handler} />;`, + output: `import { ${componentName} } from '@patternfly/react-core'; const handler = (${formattedNewParamName}, id) => {}; <${componentName} ${propName}={handler} />;`, + errors: [ + { + message: getAddCallbackParamMessage( + componentName, + propName, + newParamName + ), + type: "JSXOpeningElement", + }, + ], + }); + tests.push({ + code: `import { ${componentName} } from '@patternfly/react-core'; const handler = (id, _${formattedNewParamName}, text) => {}; <${componentName} ${propName}={handler} />;`, + output: `import { ${componentName} } from '@patternfly/react-core'; const handler = (_${formattedNewParamName}, id, text) => {}; <${componentName} ${propName}={handler} />;`, + errors: [ + { + message: getAddCallbackParamMessage( + componentName, + propName, + newParamName + ), + type: "JSXOpeningElement", + }, + ], + }); + tests.push({ + code: `import { ${componentName} } from '@patternfly/react-core'; const handler = (id, ${formattedNewParamName}, text) => {}; <${componentName} ${propName}={handler} />;`, + output: `import { ${componentName} } from '@patternfly/react-core'; const handler = (${formattedNewParamName}, 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, _${formattedNewParamName}) {}; <${componentName} ${propName}={handler} />;`, + output: `import { ${componentName} } from '@patternfly/react-core'; function handler(_${formattedNewParamName}, id) {}; <${componentName} ${propName}={handler} />;`, + errors: [ + { + message: getAddCallbackParamMessage( + componentName, + propName, + newParamName + ), + type: "JSXOpeningElement", + }, + ], + }); + tests.push({ + code: `import { ${componentName} } from '@patternfly/react-core'; function handler(id, ${formattedNewParamName}) {}; <${componentName} ${propName}={handler} />;`, + output: `import { ${componentName} } from '@patternfly/react-core'; function handler(${formattedNewParamName}, id) {}; <${componentName} ${propName}={handler} />;`, + errors: [ + { + message: getAddCallbackParamMessage( + componentName, + propName, + newParamName + ), + type: "JSXOpeningElement", + }, + ], + }); + tests.push({ + code: `import { ${componentName} } from '@patternfly/react-core'; function handler(id, _${formattedNewParamName}, text) {}; <${componentName} ${propName}={handler} />;`, + output: `import { ${componentName} } from '@patternfly/react-core'; function handler(_${formattedNewParamName}, 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, ${formattedNewParamName}, text) {}; <${componentName} ${propName}={handler} />;`, + output: `import { ${componentName} } from '@patternfly/react-core'; function handler(${formattedNewParamName}, id, text) {}; <${componentName} ${propName}={handler} />;`, errors: [ { message: getAddCallbackParamMessage( From 69c9fb1f0aee753ff8d1212e19ec1a88d3b40d66 Mon Sep 17 00:00:00 2001 From: Austin Sullivan Date: Tue, 14 Mar 2023 11:55:38 -0400 Subject: [PATCH 4/9] chore(helpers): simplify swap fix logic --- .../eslint-plugin-pf-codemods/lib/helpers.js | 89 ++++++++----------- 1 file changed, 36 insertions(+), 53 deletions(-) diff --git a/packages/eslint-plugin-pf-codemods/lib/helpers.js b/packages/eslint-plugin-pf-codemods/lib/helpers.js index ba69ea66e..99d4ffc58 100644 --- a/packages/eslint-plugin-pf-codemods/lib/helpers.js +++ b/packages/eslint-plugin-pf-codemods/lib/helpers.js @@ -293,11 +293,8 @@ function addCallbackParam(componentsArray, propMap) { return param.name === "_" + newParam; }; - const getIsFirst = (param) => - context.getTokenBefore(param).value === "("; - // meant to preserve _ being used to signal that the param isn't being used (or not) - const formatNewParam = (paramNameInCurrentUse) => { + const formatNewParam = (paramNameInCurrentUse = "_") => { const currentUseOfNewParamHasUnderscore = paramNameInCurrentUse[0] === "_"; @@ -315,61 +312,41 @@ function addCallbackParam(componentsArray, propMap) { return "_" + newParam; }; - const createParamAdditionFix = (params) => { + const createParamAdditionFix = ( + params, + paramNameInCurrentUse + ) => { const firstParam = params[0]; - if (params.length > 1) { + + const replacementParams = `${formatNewParam( + paramNameInCurrentUse + )}, ${firstParam.name}`; + + const hasParenthesis = + context.getTokenBefore(firstParam).value === "("; + + if (hasParenthesis) { return fixer.replaceText( firstParam, - `${newParam}, ${firstParam.name}` + replacementParams ); } - const hasParenthesis = - context.getTokenAfter(firstParam).value === ")"; - const replacementParams = `${newParam}, ${firstParam.name}`; - return fixer.replaceText( firstParam, - hasParenthesis - ? replacementParams - : `(${replacementParams})` + `(${replacementParams})` ); }; - const createParamSwapFix = ( - param, + const createRemoveCurrentParamUseFix = ( currentUseOfNewParam ) => { - const isCurrentUseOfNewParam = param === currentUseOfNewParam; - const isFirst = getIsFirst(param); - - // guard clause for if the new param is already the first, prevents issues if the swap doesn't need to be done - if (isCurrentUseOfNewParam && isFirst) { - return; - } + const targetRange = [ + currentUseOfNewParam.range[0] - 2, + currentUseOfNewParam.range[1], + ]; - // guard clause for if the param being evaluated is neither the param to be swapped or the first, since in that case we don't need to do anything with it - if (!isCurrentUseOfNewParam && !isFirst) { - return; - } - - if (isCurrentUseOfNewParam) { - // the range manipulation here is to remove the leading comma and space, since we know the param in question isn't the first at this point - return fixer.replaceText( - { - ...param, - range: [param.range[0] - 2, param.range[1]], - }, - "" - ); - } - - return fixer.replaceText( - param, - `${formatNewParam(currentUseOfNewParam.name)}, ${ - param.name - }` - ); + return fixer.replaceTextRange(targetRange, ""); }; if ( @@ -380,22 +357,28 @@ function addCallbackParam(componentsArray, propMap) { return fixes; } - const currentUseOfNewParam = params?.find((param) => - getIsNewParam(param) + const currentIndexOfNewParam = params?.findIndex( + (param) => getIsNewParam(param) ); - if (currentUseOfNewParam) { - params.forEach((param) => - fixes.push( - createParamSwapFix(param, currentUseOfNewParam) + if (currentIndexOfNewParam > 0) { + const currentUseOfNewParam = + params[currentIndexOfNewParam]; + + fixes.push( + createRemoveCurrentParamUseFix(currentUseOfNewParam) + ); + fixes.push( + createParamAdditionFix( + params, + currentUseOfNewParam.name ) ); } else { fixes.push(createParamAdditionFix(params)); } - // remove any fixes that aren't actually fixes (i.e. undefined) as those will break eslint - return fixes.filter((fix) => fix?.range); + return fixes; }, }); } From 43160041a547546acec000b16dcfe075a3747d90 Mon Sep 17 00:00:00 2001 From: Austin Sullivan Date: Tue, 14 Mar 2023 13:42:12 -0400 Subject: [PATCH 5/9] chore(helpers): refactor createParamAdditionFix to use ternary again --- packages/eslint-plugin-pf-codemods/lib/helpers.js | 11 +++-------- 1 file changed, 3 insertions(+), 8 deletions(-) diff --git a/packages/eslint-plugin-pf-codemods/lib/helpers.js b/packages/eslint-plugin-pf-codemods/lib/helpers.js index 99d4ffc58..ce92e5568 100644 --- a/packages/eslint-plugin-pf-codemods/lib/helpers.js +++ b/packages/eslint-plugin-pf-codemods/lib/helpers.js @@ -325,16 +325,11 @@ function addCallbackParam(componentsArray, propMap) { const hasParenthesis = context.getTokenBefore(firstParam).value === "("; - if (hasParenthesis) { - return fixer.replaceText( - firstParam, - replacementParams - ); - } - return fixer.replaceText( firstParam, - `(${replacementParams})` + hasParenthesis + ? replacementParams + : `(${replacementParams})` ); }; From cf4806616d18c9bedb5702b78b78cab74a35513d Mon Sep 17 00:00:00 2001 From: Austin Sullivan Date: Wed, 15 Mar 2023 09:23:02 -0400 Subject: [PATCH 6/9] chore(helpers): add support for preserving explicit types --- .../eslint-plugin-pf-codemods/lib/helpers.js | 2 +- .../test/testHelpers.js | 40 +++++++++++++++++++ 2 files changed, 41 insertions(+), 1 deletion(-) diff --git a/packages/eslint-plugin-pf-codemods/lib/helpers.js b/packages/eslint-plugin-pf-codemods/lib/helpers.js index ce92e5568..86e81386f 100644 --- a/packages/eslint-plugin-pf-codemods/lib/helpers.js +++ b/packages/eslint-plugin-pf-codemods/lib/helpers.js @@ -320,7 +320,7 @@ function addCallbackParam(componentsArray, propMap) { const replacementParams = `${formatNewParam( paramNameInCurrentUse - )}, ${firstParam.name}`; + )}, ${context.getSourceCode().getText(firstParam)}`; const hasParenthesis = context.getTokenBefore(firstParam).value === "("; diff --git a/packages/eslint-plugin-pf-codemods/test/testHelpers.js b/packages/eslint-plugin-pf-codemods/test/testHelpers.js index 3c38123d3..51036ce63 100644 --- a/packages/eslint-plugin-pf-codemods/test/testHelpers.js +++ b/packages/eslint-plugin-pf-codemods/test/testHelpers.js @@ -64,6 +64,16 @@ function getInvalidAddCallbackParamTests( }, ], }); + 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", + }, + ], + }); tests.push({ code: `import { ${componentName} } from '@patternfly/react-core'; <${componentName} ${propName}={id => handler(id)} />;`, output: `import { ${componentName} } from '@patternfly/react-core'; <${componentName} ${propName}={(${newParamName}, id) => handler(id)} />;`, @@ -106,6 +116,16 @@ function getInvalidAddCallbackParamTests( }, ], }); + 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", + }, + ], + }); tests.push({ code: `import { ${componentName} } from '@patternfly/react-core'; function handler(id) {}; <${componentName} ${propName}={handler} />;`, output: `import { ${componentName} } from '@patternfly/react-core'; function handler(${newParamName}, id) {}; <${componentName} ${propName}={handler} />;`, @@ -134,6 +154,16 @@ function getInvalidAddCallbackParamTests( }, ], }); + 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", + }, + ], + }); tests.push({ code: `import { ${componentName} } from '@patternfly/react-core'; <${componentName} ${propName}={this.handler} />;`, output: `import { ${componentName} } from '@patternfly/react-core'; <${componentName} ${propName}={this.handler} />;`, @@ -176,6 +206,16 @@ function getInvalidAddCallbackParamTests( }, ], }); + 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; From 0c4c573cbec9a582c3791219927ebaeeae8077aa Mon Sep 17 00:00:00 2001 From: Austin Sullivan Date: Wed, 15 Mar 2023 16:35:05 -0400 Subject: [PATCH 7/9] chore(helpers): refactor to support param name and location variations --- .../eslint-plugin-pf-codemods/lib/helpers.js | 85 +++-- .../v5/dataListCheck-updated-callback.js | 3 +- .../v5/dataListCheck-updated-callback.js | 2 +- .../test/testHelpers.js | 321 ++++++------------ test/test.tsx | 4 +- 5 files changed, 149 insertions(+), 266 deletions(-) diff --git a/packages/eslint-plugin-pf-codemods/lib/helpers.js b/packages/eslint-plugin-pf-codemods/lib/helpers.js index 86e81386f..f1296a138 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)) || @@ -279,48 +314,12 @@ function addCallbackParam(componentsArray, propMap) { fix(fixer) { const fixes = []; - const newParamHasUnderscore = newParam[0] === "_"; - - const getIsNewParam = (param) => { - if (param.name === newParam) { - return true; - } - - if (newParamHasUnderscore) { - return param.name === newParam.slice(1); - } - - return param.name === "_" + newParam; - }; - - // meant to preserve _ being used to signal that the param isn't being used (or not) - const formatNewParam = (paramNameInCurrentUse = "_") => { - const currentUseOfNewParamHasUnderscore = - paramNameInCurrentUse[0] === "_"; - - if ( - newParamHasUnderscore === - currentUseOfNewParamHasUnderscore - ) { - return newParam; - } - - if (newParamHasUnderscore) { - return newParam.slice(1); - } - - return "_" + newParam; - }; - - const createParamAdditionFix = ( - params, - paramNameInCurrentUse - ) => { + const createParamAdditionFix = (params) => { const firstParam = params[0]; - const replacementParams = `${formatNewParam( - paramNameInCurrentUse - )}, ${context.getSourceCode().getText(firstParam)}`; + const replacementParams = `${newParam}, ${context + .getSourceCode() + .getText(firstParam)}`; const hasParenthesis = context.getTokenBefore(firstParam).value === "("; @@ -353,7 +352,7 @@ function addCallbackParam(componentsArray, propMap) { } const currentIndexOfNewParam = params?.findIndex( - (param) => getIsNewParam(param) + (param) => param.name === newParam ); if (currentIndexOfNewParam > 0) { @@ -369,7 +368,7 @@ function addCallbackParam(componentsArray, propMap) { currentUseOfNewParam.name ) ); - } else { + } else if (params[0].name !== newParam) { fixes.push(createParamAdditionFix(params)); } 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..794f59b53 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,6 @@ 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: "_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 ca2741367..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 { swapCallbackParamTester } = require("../../testHelpers"); -swapCallbackParamTester('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 51036ce63..94526ef99 100644 --- a/packages/eslint-plugin-pf-codemods/test/testHelpers.js +++ b/packages/eslint-plugin-pf-codemods/test/testHelpers.js @@ -30,7 +30,8 @@ function getValidAddCallbackParamTests(componentNameArray, propNameArray) { function getInvalidAddCallbackParamTests( componentNameArray, propNameArray, - newParamName + newParamName, + previousParamIndex ) { let tests = []; @@ -51,8 +52,8 @@ function getInvalidAddCallbackParamTests( ], }); 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)} />;`, + 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( @@ -64,16 +65,6 @@ function getInvalidAddCallbackParamTests( }, ], }); - 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", - }, - ], - }); tests.push({ code: `import { ${componentName} } from '@patternfly/react-core'; <${componentName} ${propName}={id => handler(id)} />;`, output: `import { ${componentName} } from '@patternfly/react-core'; <${componentName} ${propName}={(${newParamName}, id) => handler(id)} />;`, @@ -103,8 +94,8 @@ function getInvalidAddCallbackParamTests( ], }); 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} />;`, + 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( @@ -116,16 +107,6 @@ function getInvalidAddCallbackParamTests( }, ], }); - 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", - }, - ], - }); tests.push({ code: `import { ${componentName} } from '@patternfly/react-core'; function handler(id) {}; <${componentName} ${propName}={handler} />;`, output: `import { ${componentName} } from '@patternfly/react-core'; function handler(${newParamName}, id) {}; <${componentName} ${propName}={handler} />;`, @@ -141,8 +122,8 @@ function getInvalidAddCallbackParamTests( ], }); 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} />;`, + 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( @@ -154,16 +135,6 @@ function getInvalidAddCallbackParamTests( }, ], }); - 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", - }, - ], - }); tests.push({ code: `import { ${componentName} } from '@patternfly/react-core'; <${componentName} ${propName}={this.handler} />;`, output: `import { ${componentName} } from '@patternfly/react-core'; <${componentName} ${propName}={this.handler} />;`, @@ -193,8 +164,8 @@ function getInvalidAddCallbackParamTests( ], }); 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)} />;`, + 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( @@ -206,16 +177,66 @@ function getInvalidAddCallbackParamTests( }, ], }); - 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; @@ -224,186 +245,46 @@ function getInvalidAddCallbackParamTests( function getInvalidSwapCallbackParamTests( componentNameArray, propNameArray, + previousParamIndex, newParamName ) { let tests = getInvalidAddCallbackParamTests( componentNameArray, propNameArray, - newParamName + newParamName, + previousParamIndex ); - const formattedNewParamName = newParamName[0] === '_' ? newParamName.slice(1) : newParamName; + const formattedNewParamName = + newParamName[0] === "_" ? newParamName.slice(1) : newParamName; componentNameArray.forEach((componentName) => { propNameArray.forEach((propName) => { - tests.push({ - code: `import { ${componentName} } from '@patternfly/react-core'; <${componentName} ${propName}={(id, _${formattedNewParamName}) => handler(id)} />;`, - output: `import { ${componentName} } from '@patternfly/react-core'; <${componentName} ${propName}={(_${formattedNewParamName}, id) => handler(id)} />;`, - errors: [ - { - message: getAddCallbackParamMessage( - componentName, - propName, - newParamName - ), - type: "JSXOpeningElement", - }, - ], - }); - tests.push({ - code: `import { ${componentName} } from '@patternfly/react-core'; <${componentName} ${propName}={(id, ${formattedNewParamName}) => handler(id, ${formattedNewParamName})} />;`, - output: `import { ${componentName} } from '@patternfly/react-core'; <${componentName} ${propName}={(${formattedNewParamName}, id) => handler(id, ${formattedNewParamName})} />;`, - errors: [ - { - message: getAddCallbackParamMessage( - componentName, - propName, - newParamName - ), - type: "JSXOpeningElement", - }, - ], - }); - tests.push({ - code: `import { ${componentName} } from '@patternfly/react-core'; <${componentName} ${propName}={(id, _${formattedNewParamName}, text) => handler(id, text)} />;`, - output: `import { ${componentName} } from '@patternfly/react-core'; <${componentName} ${propName}={(_${formattedNewParamName}, id, text) => handler(id, text)} />;`, - errors: [ - { - message: getAddCallbackParamMessage( - componentName, - propName, - newParamName - ), - type: "JSXOpeningElement", - }, - ], - }); - tests.push({ - code: `import { ${componentName} } from '@patternfly/react-core'; <${componentName} ${propName}={(id, ${formattedNewParamName}, text) => handler(id, ${formattedNewParamName}, text)} />;`, - output: `import { ${componentName} } from '@patternfly/react-core'; <${componentName} ${propName}={(${formattedNewParamName}, id, text) => handler(id, ${formattedNewParamName}, text)} />;`, - errors: [ - { - message: getAddCallbackParamMessage( - componentName, - propName, - newParamName - ), - type: "JSXOpeningElement", - }, - ], - }); - tests.push({ - code: `import { ${componentName} } from '@patternfly/react-core'; const handler = (id, _${formattedNewParamName}) => {}; <${componentName} ${propName}={handler} />;`, - output: `import { ${componentName} } from '@patternfly/react-core'; const handler = (_${formattedNewParamName}, id) => {}; <${componentName} ${propName}={handler} />;`, - errors: [ - { - message: getAddCallbackParamMessage( - componentName, - propName, - newParamName - ), - type: "JSXOpeningElement", - }, - ], - }); - tests.push({ - code: `import { ${componentName} } from '@patternfly/react-core'; const handler = (id, ${formattedNewParamName}) => {}; <${componentName} ${propName}={handler} />;`, - output: `import { ${componentName} } from '@patternfly/react-core'; const handler = (${formattedNewParamName}, id) => {}; <${componentName} ${propName}={handler} />;`, - errors: [ - { - message: getAddCallbackParamMessage( - componentName, - propName, - newParamName - ), - type: "JSXOpeningElement", - }, - ], - }); - tests.push({ - code: `import { ${componentName} } from '@patternfly/react-core'; const handler = (id, _${formattedNewParamName}, text) => {}; <${componentName} ${propName}={handler} />;`, - output: `import { ${componentName} } from '@patternfly/react-core'; const handler = (_${formattedNewParamName}, id, text) => {}; <${componentName} ${propName}={handler} />;`, - errors: [ - { - message: getAddCallbackParamMessage( - componentName, - propName, - newParamName - ), - type: "JSXOpeningElement", - }, - ], - }); - tests.push({ - code: `import { ${componentName} } from '@patternfly/react-core'; const handler = (id, ${formattedNewParamName}, text) => {}; <${componentName} ${propName}={handler} />;`, - output: `import { ${componentName} } from '@patternfly/react-core'; const handler = (${formattedNewParamName}, 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, _${formattedNewParamName}) {}; <${componentName} ${propName}={handler} />;`, - output: `import { ${componentName} } from '@patternfly/react-core'; function handler(_${formattedNewParamName}, id) {}; <${componentName} ${propName}={handler} />;`, - errors: [ - { - message: getAddCallbackParamMessage( - componentName, - propName, - newParamName - ), - type: "JSXOpeningElement", - }, - ], - }); - tests.push({ - code: `import { ${componentName} } from '@patternfly/react-core'; function handler(id, ${formattedNewParamName}) {}; <${componentName} ${propName}={handler} />;`, - output: `import { ${componentName} } from '@patternfly/react-core'; function handler(${formattedNewParamName}, id) {}; <${componentName} ${propName}={handler} />;`, - errors: [ - { - message: getAddCallbackParamMessage( - componentName, - propName, - newParamName - ), - type: "JSXOpeningElement", - }, - ], - }); - tests.push({ - code: `import { ${componentName} } from '@patternfly/react-core'; function handler(id, _${formattedNewParamName}, text) {}; <${componentName} ${propName}={handler} />;`, - output: `import { ${componentName} } from '@patternfly/react-core'; function handler(_${formattedNewParamName}, 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, ${formattedNewParamName}, text) {}; <${componentName} ${propName}={handler} />;`, - output: `import { ${componentName} } from '@patternfly/react-core'; function handler(${formattedNewParamName}, id, text) {}; <${componentName} ${propName}={handler} />;`, - errors: [ - { - message: getAddCallbackParamMessage( - componentName, - propName, - newParamName - ), - type: "JSXOpeningElement", - }, - ], - }); + // the following tests are specific to 'event' at the moment as I can't think of a good totally generic way to do this. Make new tests relevant to your param name if it isn't event. + if (formattedNewParamName === "event") { + const expectedEventVariations = ['e', '_e', 'ev', '_ev', 'evt', '_evt', 'event', '_event'] + const genericArgs = ['id', 'text', 'foo', 'bar', 'bash', 'bang'] + + expectedEventVariations.forEach(variation => { + const stringifyArgs = (args) => { + return args.reduce((acc, arg) => `${acc}, ${arg}`, '').slice(2) + } + + const initialArgs = stringifyArgs([...genericArgs.slice(0, previousParamIndex), variation]) + const fixedArgs = stringifyArgs([variation, ...genericArgs.slice(0, previousParamIndex)]) + + 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; @@ -434,6 +315,7 @@ function swapCallbackParamTester( ruleName, componentNames, propNames, + previousParamIndex, newParamName = "_event" ) { const rule = require(`../lib/rules/v5/${ruleName}`); @@ -446,6 +328,7 @@ function swapCallbackParamTester( invalid: getInvalidSwapCallbackParamTests( componentNameArray, propNameArray, + previousParamIndex, newParamName ), }); 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");