Improvements to addCallbackParam helper & associated tests

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;

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$)/ } }),
 };

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)