feat(Button): Extend icon moving to support Icon and react-icon child…

…ren (#735)

* feat(Button): Extend icon moving to support Icon and react-icon children

* fix(Button): fix bugs found during review

* chore(Button): Update single test files

* chore(helpers): Update helper import after merge

packages/eslint-plugin-pf-codemods/src/rules/helpers/index.ts

@@ -20,6 +20,7 @@ export * from "./helpers";
 export * from "./importAndExport";
 export * from "./includesImport";
 export * from "./interfaces";
+export * from "./isReactIcon";
 export * from "./JSXAttributes";
 export * from "./nodeMatches";
 export * from "./pfPackageMatches";

packages/eslint-plugin-pf-codemods/src/rules/helpers/isReactIcon.ts

@@ -0,0 +1,31 @@
+import { Rule } from "eslint";
+import { JSXElement } from "estree-jsx";
+import { getFromPackage, getDefaultImportsFromPackage } from "./index";
+
+/** Returns true if an element is a patternfly/react-icons import, false if it isn't, and undefined
+ *  if no element is passed (for type safety) */
+export function isReactIcon(context: Rule.RuleContext, element?: JSXElement) {
+  if (!element) {
+    return;
+  }
+
+  const openingElementIdentifier = element.openingElement.name;
+
+  // TODO: update this to use the appropriate getNodeName helper once it lands
+  if (openingElementIdentifier.type !== "JSXIdentifier") {
+    return;
+  }
+  const elementName = openingElementIdentifier.name;
+
+  const pfIconsPackage = "@patternfly/react-icons";
+  const { imports: iconImports } = getFromPackage(context, pfIconsPackage);
+  const iconDefaultImports = getDefaultImportsFromPackage(
+    context,
+    pfIconsPackage
+  );
+  const allIconImports = [...iconImports, ...iconDefaultImports];
+
+  return allIconImports.some(
+    (iconImport) => iconImport.local.name === elementName
+  );
+}

packages/eslint-plugin-pf-codemods/src/rules/v6/buttonMoveIconsIconProp/button-moveIcons-icon-prop.test.ts

@@ -9,6 +9,10 @@ ruleTester.run("button-moveIcons-icon-prop", rule, {
     {
       code: `import { Button } from '@patternfly/react-core'; <Button icon={<span>Some icon</span>} />`,
     },
+    // with non PF Icon child
+    {
+      code: `import { Button } from '@patternfly/react-core'; <Button><Icon>Some icon</Icon></Button>`,
+    },
   ],
   invalid: [
     {
@@ -81,5 +85,49 @@ ruleTester.run("button-moveIcons-icon-prop", rule, {
         },
       ],
     },
+    // with Icon component child
+    {
+      code: `import { Button, Icon } from '@patternfly/react-core'; <Button><Icon>Some icon</Icon></Button>`,
+      output: `import { Button, Icon } from '@patternfly/react-core'; <Button icon={<Icon>Some icon</Icon>}></Button>`,
+      errors: [
+        {
+          message: `Icons must now be passed to the \`icon\` prop of Button instead of as children. If you are passing anything other than an icon as children, ignore this rule when running fixes.`,
+          type: "JSXElement",
+        },
+      ],
+    },
+    // with react-icons icon child
+    {
+      code: `import { Button } from '@patternfly/react-core'; import { SomeIcon } from "@patternfly/react-icons"; <Button><SomeIcon /></Button>`,
+      output: `import { Button } from '@patternfly/react-core'; import { SomeIcon } from "@patternfly/react-icons"; <Button icon={<SomeIcon />}></Button>`,
+      errors: [
+        {
+          message: `Icons must now be passed to the \`icon\` prop of Button instead of as children. If you are passing anything other than an icon as children, ignore this rule when running fixes.`,
+          type: "JSXElement",
+        },
+      ],
+    },
+    // with react-icons icon child and another child
+    {
+      code: `import { Button } from '@patternfly/react-core'; import { SomeIcon } from "@patternfly/react-icons"; <Button>Text<SomeIcon /></Button>`,
+      output: `import { Button } from '@patternfly/react-core'; import { SomeIcon } from "@patternfly/react-icons"; <Button icon={<SomeIcon />}>Text</Button>`,
+      errors: [
+        {
+          message: `Icons must now be passed to the \`icon\` prop of Button instead of as children. If you are passing anything other than an icon as children, ignore this rule when running fixes.`,
+          type: "JSXElement",
+        },
+      ],
+    },
+    // with children prop
+    {
+      code: `import { Button } from '@patternfly/react-core'; <Button variant="plain" children={<span>Some icon</span>} />`,
+      output: `import { Button } from '@patternfly/react-core'; <Button icon={<span>Some icon</span>} variant="plain"  />`,
+      errors: [
+        {
+          message: `Icons must now be passed to the \`icon\` prop of Button instead of as children. If you are passing anything other than an icon as children, ignore this rule when running fixes.`,
+          type: "JSXElement",
+        },
+      ],
+    },
   ],
 });

packages/eslint-plugin-pf-codemods/src/rules/v6/buttonMoveIconsIconProp/button-moveIcons-icon-prop.ts

@@ -6,12 +6,15 @@ import {
   getAttributeValue,
   getExpression,
   getChildrenAsAttributeValueText,
+  getChildJSXElementByName,
+  isReactIcon,
 } from "../../helpers";
 
 // https://github.com/patternfly/patternfly-react/pull/10663
 module.exports = {
   meta: { fixable: "code" },
   create: function (context: Rule.RuleContext) {
+    const source = context.getSourceCode();
     const { imports } = getFromPackage(context, "@patternfly/react-core");
 
     const buttonImport = imports.find(
@@ -20,6 +23,9 @@ module.exports = {
     const buttonVariantEnumImport = imports.find(
       (specifier) => specifier.imported.name === "ButtonVariant"
     );
+    const hasIconImport = imports.some(
+      (specifier) => specifier.imported.name === "Icon"
+    );
 
     return !buttonImport
       ? {}
@@ -31,41 +37,61 @@ module.exports = {
             ) {
               const variantProp = getAttribute(node.openingElement, "variant");
               const iconProp = getAttribute(node.openingElement, "icon");
-              if (!variantProp || iconProp) {
+              if (iconProp) {
                 return;
               }
               const variantValue = getAttributeValue(
                 context,
-                variantProp.value
+                variantProp?.value
               );
 
               const isEnumValuePlain =
                 buttonVariantEnumImport &&
                 variantValue?.object?.name ===
                   buttonVariantEnumImport.local.name &&
                 variantValue?.property.name === "plain";
-              if (variantValue !== "plain" && !isEnumValuePlain) {
-                return;
-              }
+
+              const isPlain = variantValue === "plain" || isEnumValuePlain;
+
               const childrenProp = getAttribute(node, "children");
-              let childrenValue;
+
+              let childrenValue: string | undefined;
               if (childrenProp) {
                 const childrenPropExpression = getExpression(
                   childrenProp?.value
                 );
                 childrenValue = childrenPropExpression
-                  ? context.getSourceCode().getText(childrenPropExpression)
+                  ? `{${source.getText(childrenPropExpression)}}`
                   : "";
-              } else {
+              } else if (isPlain) {
                 childrenValue = getChildrenAsAttributeValueText(
                   context,
                   node.children
                 );
               }
-              if (!childrenValue) {
+
+              if (!childrenValue && isPlain) {
                 return;
               }
 
+              const iconComponentChild =
+                hasIconImport && getChildJSXElementByName(node, "Icon");
+
+              const jsxElementChildren = node.children.filter(
+                (child) => child.type === "JSXElement"
+              ) as JSXElement[];
+              const reactIconChild = jsxElementChildren.find((child) =>
+                isReactIcon(context, child)
+              );
+
+              const iconChild = iconComponentChild || reactIconChild;
+
+              if (!isPlain && !iconChild) {
+                return;
+              }
+
+              const iconChildString = `{${source.getText(iconChild)}}`;
+
               context.report({
                 node,
                 message: `Icons must now be passed to the \`icon\` prop of Button instead of as children. If you are passing anything other than an icon as children, ignore this rule when running fixes.`,
@@ -74,18 +100,20 @@ module.exports = {
                   fixes.push(
                     fixer.insertTextAfter(
                       node.openingElement.name,
-                      ` icon=${childrenValue}`
+                      ` icon=${childrenValue || iconChildString}`
                     )
                   );
 
                   if (childrenProp) {
                     fixes.push(fixer.remove(childrenProp));
-                  } else {
+                  } else if (isPlain) {
                     node.children.forEach(
                       (child) =>
                         child.type !== "JSXSpreadChild" &&
                         fixes.push(fixer.replaceText(child, ""))
                     );
+                  } else if (iconChild) {
+                    fixes.push(fixer.replaceText(iconChild, ""));
                   }
                   return fixes;
                 },

packages/eslint-plugin-pf-codemods/src/rules/v6/buttonMoveIconsIconProp/buttonMoveIconsIconPropInput.tsx

@@ -1,7 +1,18 @@
-import { Button } from "@patternfly/react-core";
+import { Button, Icon } from "@patternfly/react-core";
+import { SomeIcon } from "@patternfly/react-icons";
 
 export const ButtonMoveIconsIconPropInput = () => (
-  <Button variant='plain'>
-    <span>Icon</span>
-  </Button>
+  <>
+    <Button variant="plain">
+      <span>Icon</span>
+    </Button>
+    <Button>
+      <Icon>
+        <SomeIcon />
+      </Icon>
+    </Button>
+    <Button>
+      <SomeIcon />
+    </Button>
+  </>
 );

packages/eslint-plugin-pf-codemods/src/rules/v6/buttonMoveIconsIconProp/buttonMoveIconsIconPropOutput.tsx

@@ -1,5 +1,16 @@
-import { Button } from "@patternfly/react-core";
+import { Button, Icon } from "@patternfly/react-core";
+import { SomeIcon } from "@patternfly/react-icons";
 
 export const ButtonMoveIconsIconPropInput = () => (
-  <Button icon={<span>Icon</span>} variant='plain'></Button>
+  <>
+    <Button icon={<span>Icon</span>} variant="plain"></Button>
+    <Button icon={<Icon>
+        <SomeIcon />
+      </Icon>}>
+
+    </Button>
+    <Button icon={<SomeIcon />}>
+
+    </Button>
+  </>
 );