Pr feedback

patternfly · Aug 22, 2024 · 2d2a870 · 2d2a870
1 parent ccbd46b
commit 2d2a870
Show file tree

Hide file tree

Showing 6 changed files with 55 additions and 54 deletions.
diff --git a/packages/eslint-plugin-pf-codemods/src/rules/helpers/getLocalComponentName.ts b/packages/eslint-plugin-pf-codemods/src/rules/helpers/getLocalComponentName.ts
@@ -1,18 +1,16 @@
-import { ImportSpecifier, ImportDefaultSpecifier } from "estree-jsx";
+import { ImportSpecifier } from "estree-jsx";
 import { getSpecifierFromImports } from "./getSpecifierFromImports";
 
 /** Resolves the local name of an import */
 export function getLocalComponentName(
-  namedImports: (ImportSpecifier | ImportDefaultSpecifier)[],
+  namedImports: ImportSpecifier[],
   importedName: string
 ) {
   const componentImport = getSpecifierFromImports(namedImports, importedName);
-  const isDefaultImport = componentImport?.type === "ImportDefaultSpecifier";
   const isAlias =
-    !isDefaultImport &&
     componentImport?.imported.name !== componentImport?.local.name;
 
-  if ((componentImport && isAlias) || isDefaultImport) {
+  if (componentImport && isAlias) {
     return componentImport.local.name;
   }
 

diff --git a/packages/eslint-plugin-pf-codemods/src/rules/helpers/getNodeForAttributeFixer.ts b/packages/eslint-plugin-pf-codemods/src/rules/helpers/getNodeForAttributeFixer.ts
@@ -10,32 +10,37 @@ export function getNodeForAttributeFixer(
   context: Rule.RuleContext,
   attribute: JSXAttribute
 ) {
-  if (!attribute.value) {
+  if (!attribute?.value) {
     return;
   }
 
-  if (
-    attribute.value.type === "JSXExpressionContainer" &&
-    attribute.value.expression.type === "Identifier"
-  ) {
-    const scope = context.getSourceCode().getScope(attribute);
-    const variableDeclaration = getVariableDeclaration(
-      attribute.value.expression.name,
-      scope
-    );
-
-    return variableDeclaration && variableDeclaration.defs[0].node.init;
+  switch (attribute.value.type) {
+    case "Literal":
+      return attribute.value;
+    case "JSXExpressionContainer":
+      return getJSXExpressionContainerValue(context, attribute);
   }
+}
 
-  if (attribute.value.type === "Literal") {
-    return attribute.value;
+function getJSXExpressionContainerValue(
+  context: Rule.RuleContext,
+  attribute: JSXAttribute
+) {
+  if (!attribute.value || attribute.value.type !== "JSXExpressionContainer") {
+    return;
   }
-  if (
-    attribute.value.type === "JSXExpressionContainer" &&
-    ["ObjectExpression", "MemberExpression"].includes(
-      attribute.value.expression.type
-    )
-  ) {
-    return attribute.value.expression;
+
+  switch (attribute.value.expression.type) {
+    case "ObjectExpression":
+    case "MemberExpression":
+      return attribute.value.expression;
+    case "Identifier":
+      const scope = context.getSourceCode().getScope(attribute);
+      const variableDeclaration = getVariableDeclaration(
+        attribute.value.expression.name,
+        scope
+      );
+
+      return variableDeclaration && variableDeclaration.defs[0].node.init;
   }
 }
diff --git a/packages/eslint-plugin-pf-codemods/src/rules/helpers/getSpecifierFromImports.ts b/packages/eslint-plugin-pf-codemods/src/rules/helpers/getSpecifierFromImports.ts
@@ -1,15 +1,13 @@
-import { ImportSpecifier, ImportDefaultSpecifier } from "estree-jsx";
+import { ImportSpecifier } from "estree-jsx";
 
 /** Can be used to extract a specific specifier from an array of imports, such as to only
  * run logic if X and Y imports are present or to use the specifier properties in other logic. */
 export function getSpecifierFromImports(
-  imports: (ImportSpecifier | ImportDefaultSpecifier)[],
+  imports: ImportSpecifier[],
   importedName: string
 ) {
-  const importSpecifier = imports.find((imp) =>
-    imp.type === "ImportDefaultSpecifier"
-      ? imp.local.name === importedName
-      : imp.imported.name === importedName
+  const importSpecifier = imports.find(
+    (imp) => imp.imported.name === importedName
   );
 
   return importSpecifier;

diff --git a/packages/eslint-plugin-pf-codemods/src/rules/helpers/removePropertiesFromObjectExpression.ts b/packages/eslint-plugin-pf-codemods/src/rules/helpers/removePropertiesFromObjectExpression.ts
@@ -32,7 +32,7 @@ export function removePropertiesFromObjectExpression(
     }
 
     if (property.key.type === "Literal") {
-      return propertyNamesToRemove.includes(property.key.value);
+      return !propertyNamesToRemove.includes(property.key.value);
     }
 
     return false;

diff --git a/...pf-codemods/src/rules/v6/cardUpdatedClickableMarkup/card-updated-clickable-markup.test.ts b/...pf-codemods/src/rules/v6/cardUpdatedClickableMarkup/card-updated-clickable-markup.test.ts
@@ -1,6 +1,10 @@
 const ruleTester = require("../../ruletester");
 import * as rule from "./card-updated-clickable-markup";
 
+const baseMessage = "The markup for clickable-only cards has been updated.";
+const propertyMessage =
+  " Additionally, the `selectableActions.selectableActionId` and `selectableActions.name` props are no longer necessary to pass to CardHeader for clickable-only cards.";
+
 ruleTester.run("card-updated-clickable-markup", rule, {
   valid: [
     {
@@ -26,7 +30,7 @@ ruleTester.run("card-updated-clickable-markup", rule, {
       output: `import { Card, CardHeader } from '@patternfly/react-core'; <Card isClickable><CardHeader selectableActions={{to: "#"}} /></Card>`,
       errors: [
         {
-          message: "The markup for clickable-only cards has been updated.",
+          message: baseMessage,
           type: "JSXElement",
         },
       ],
@@ -37,8 +41,7 @@ ruleTester.run("card-updated-clickable-markup", rule, {
       output: `import { Card, CardHeader } from '@patternfly/react-core'; <Card isClickable><CardHeader selectableActions={{to: '#'}} /></Card>`,
       errors: [
         {
-          message:
-            "The markup for clickable-only cards has been updated.Additionally, the `selectableActions.selectableActionId` and `selectableActions.name` props are no longer necessary to pass to CardHeader for clickable-only cards.",
+          message: baseMessage + propertyMessage,
           type: "JSXElement",
         },
       ],
@@ -49,8 +52,7 @@ ruleTester.run("card-updated-clickable-markup", rule, {
       output: `import { Card, CardHeader } from '@patternfly/react-core'; <Card isClickable><CardHeader selectableActions={{}} /></Card>`,
       errors: [
         {
-          message:
-            "The markup for clickable-only cards has been updated.Additionally, the `selectableActions.selectableActionId` and `selectableActions.name` props are no longer necessary to pass to CardHeader for clickable-only cards.",
+          message: baseMessage + propertyMessage,
           type: "JSXElement",
         },
       ],
@@ -61,9 +63,7 @@ ruleTester.run("card-updated-clickable-markup", rule, {
       output: `import { Card, CardHeader } from '@patternfly/react-core'; <Card isClickable><CardHeader selectableActions={{to: "#"}} /></Card>`,
       errors: [
         {
-          message:
-            "The markup for clickable-only cards has been updated.Additionally, the `selectableActions.selectableActionId` and `selectableActions.name` props are no longer necessary to pass to CardHeader for clickable-only cards.",
-          // message: `The markup for clickable-only cards has been updated, now using button and anchor elements for the respective clickable action. The \`selectableActions.selectableActionId\` and \`selectableActions.name\` props are also no longer necessary for clickable-only cards. Passing them in will not cause any errors, but running the fix for this rule will remove them.`,
+          message: baseMessage + propertyMessage,
           type: "JSXElement",
         },
       ],
@@ -74,8 +74,7 @@ ruleTester.run("card-updated-clickable-markup", rule, {
       output: `import { Card, CardHeader } from '@patternfly/react-core'; const obj = {}; <Card isClickable><CardHeader selectableActions={obj} /></Card>`,
       errors: [
         {
-          message:
-            "The markup for clickable-only cards has been updated.Additionally, the `selectableActions.selectableActionId` and `selectableActions.name` props are no longer necessary to pass to CardHeader for clickable-only cards.",
+          message: baseMessage + propertyMessage,
           type: "JSXElement",
         },
       ],
@@ -86,8 +85,7 @@ ruleTester.run("card-updated-clickable-markup", rule, {
       output: `import { Card, CardHeader } from '@patternfly/react-core'; const obj = {to: "#", extra: "thing"}; <Card isClickable><CardHeader selectableActions={obj} /></Card>`,
       errors: [
         {
-          message:
-            "The markup for clickable-only cards has been updated.Additionally, the `selectableActions.selectableActionId` and `selectableActions.name` props are no longer necessary to pass to CardHeader for clickable-only cards.",
+          message: baseMessage + propertyMessage,
           type: "JSXElement",
         },
       ],
@@ -98,7 +96,7 @@ ruleTester.run("card-updated-clickable-markup", rule, {
       output: `import { Card as CustomCard, CardHeader as CustomCardHeader } from '@patternfly/react-core'; <CustomCard isClickable><CustomCardHeader selectableActions={{to: "#"}} /></CustomCard>`,
       errors: [
         {
-          message: "The markup for clickable-only cards has been updated.",
+          message: baseMessage,
           type: "JSXElement",
         },
       ],
@@ -109,7 +107,7 @@ ruleTester.run("card-updated-clickable-markup", rule, {
       output: `import { Card, CardHeader } from '@patternfly/react-core/dist/esm/components/Card/index.js'; <Card isClickable><CardHeader selectableActions={{to: "#"}} /></Card>`,
       errors: [
         {
-          message: "The markup for clickable-only cards has been updated.",
+          message: baseMessage,
           type: "JSXElement",
         },
       ],
@@ -119,7 +117,7 @@ ruleTester.run("card-updated-clickable-markup", rule, {
       output: `import { Card, CardHeader } from '@patternfly/react-core/dist/js/components/Card/index.js'; <Card isClickable><CardHeader selectableActions={{to: "#"}} /></Card>`,
       errors: [
         {
-          message: "The markup for clickable-only cards has been updated.",
+          message: baseMessage,
           type: "JSXElement",
         },
       ],
@@ -129,7 +127,7 @@ ruleTester.run("card-updated-clickable-markup", rule, {
       output: `import { Card, CardHeader } from '@patternfly/react-core/dist/dynamic/components/Card/index.js'; <Card isClickable><CardHeader selectableActions={{to: "#"}} /></Card>`,
       errors: [
         {
-          message: "The markup for clickable-only cards has been updated.",
+          message: baseMessage,
           type: "JSXElement",
         },
       ],

diff --git a/...ugin-pf-codemods/src/rules/v6/cardUpdatedClickableMarkup/card-updated-clickable-markup.ts b/...ugin-pf-codemods/src/rules/v6/cardUpdatedClickableMarkup/card-updated-clickable-markup.ts
@@ -2,6 +2,7 @@ import { Rule } from "eslint";
 import { JSXElement, Property, Literal } from "estree-jsx";
 import {
   getAllImportsFromPackage,
+  getFromPackage,
   checkMatchingJSXOpeningElement,
   getAttribute,
   getAttributeValue,
@@ -17,10 +18,7 @@ module.exports = {
   meta: { fixable: "code" },
   create: function (context: Rule.RuleContext) {
     const basePackage = "@patternfly/react-core";
-    const componentImports = getAllImportsFromPackage(context, basePackage, [
-      "Card",
-      "CardHeader",
-    ]);
+    const { imports: componentImports } = getFromPackage(context, basePackage);
     const cardImport = getSpecifierFromImports(componentImports, "Card");
     const cardHeaderImport = getSpecifierFromImports(
       componentImports,
@@ -57,6 +55,10 @@ module.exports = {
                 context,
                 selectableActionsProp.value
               );
+              if (!selectableActionsValue) {
+                return;
+              }
+
               const nameProperty = getObjectProperty(
                 context,
                 selectableActionsValue,
@@ -72,7 +74,7 @@ module.exports = {
                 "The markup for clickable-only cards has been updated.";
               const message = `${baseMessage}${
                 nameProperty || idProperty
-                  ? "Additionally, the `selectableActions.selectableActionId` and `selectableActions.name` props are no longer necessary to pass to CardHeader for clickable-only cards."
+                  ? " Additionally, the `selectableActions.selectableActionId` and `selectableActions.name` props are no longer necessary to pass to CardHeader for clickable-only cards."
                   : ""
               }`;
               context.report({