From 0fb616ba69a6a86b212daf6fb2693600c4dddf5d Mon Sep 17 00:00:00 2001 From: Eric Olkowski Date: Thu, 15 Aug 2024 13:29:31 -0400 Subject: [PATCH 1/4] fix(Card): updated markup for actionable cards --- .../src/rules/helpers/JSXElements.ts | 27 ------------ .../rules/helpers/getChildJSXElementByName.ts | 41 +++++++++++++++++++ .../rules/helpers/getLocalComponentName.ts | 14 +++---- .../rules/helpers/getSpecifierFromImports.ts | 16 ++++++++ .../src/rules/helpers/index.ts | 4 +- .../src/rules/helpers/nodeIsComponentNamed.ts | 9 ++++ .../emptyStateHeader-move-into-emptyState.ts | 8 ++-- .../masthead-structure-changes.ts | 4 +- .../wizardFooter-warn-update-markup.ts | 11 +++-- 9 files changed, 90 insertions(+), 44 deletions(-) delete mode 100644 packages/eslint-plugin-pf-codemods/src/rules/helpers/JSXElements.ts create mode 100644 packages/eslint-plugin-pf-codemods/src/rules/helpers/getChildJSXElementByName.ts create mode 100644 packages/eslint-plugin-pf-codemods/src/rules/helpers/getSpecifierFromImports.ts create mode 100644 packages/eslint-plugin-pf-codemods/src/rules/helpers/nodeIsComponentNamed.ts diff --git a/packages/eslint-plugin-pf-codemods/src/rules/helpers/JSXElements.ts b/packages/eslint-plugin-pf-codemods/src/rules/helpers/JSXElements.ts deleted file mode 100644 index 9428d7ab6..000000000 --- a/packages/eslint-plugin-pf-codemods/src/rules/helpers/JSXElements.ts +++ /dev/null @@ -1,27 +0,0 @@ -import { JSXElement } from "estree-jsx"; - -export function getChildElementByName(node: JSXElement, name: string) { - return node.children?.find( - (child) => - child.type === "JSXElement" && - child.openingElement.name.type === "JSXIdentifier" && - child.openingElement.name.name === name - ) as JSXElement | undefined; -} - -export function getAllChildElementsByName(node: JSXElement, name: string) { - return node.children?.filter( - (child) => - child.type === "JSXElement" && - child.openingElement.name.type === "JSXIdentifier" && - child.openingElement.name.name === name - ) as JSXElement[]; -} - -export function nodeIsComponentNamed(node: JSXElement, componentName: string) { - if (node.openingElement.name.type === "JSXIdentifier") { - return node.openingElement.name.name === componentName; - } - - return false; -} diff --git a/packages/eslint-plugin-pf-codemods/src/rules/helpers/getChildJSXElementByName.ts b/packages/eslint-plugin-pf-codemods/src/rules/helpers/getChildJSXElementByName.ts new file mode 100644 index 000000000..4f00e951b --- /dev/null +++ b/packages/eslint-plugin-pf-codemods/src/rules/helpers/getChildJSXElementByName.ts @@ -0,0 +1,41 @@ +import { + JSXElement, + JSXText, + JSXExpressionContainer, + JSXSpreadChild, + JSXFragment, +} from "estree-jsx"; + +function getChildJSXElementCallback( + childNode: + | JSXElement + | JSXText + | JSXExpressionContainer + | JSXSpreadChild + | JSXFragment, + name: string +) { + return ( + childNode.type === "JSXElement" && + childNode.openingElement.name.type === "JSXIdentifier" && + childNode.openingElement.name.name === name + ); +} + +/** Can be used to run logic if the specific child element exists, or to run logic on the + * specified element. + */ +export function getChildJSXElementByName(node: JSXElement, name: string) { + return node.children?.find((child) => + getChildJSXElementCallback(child, name) + ) as JSXElement | undefined; +} + +/** Can be used to run logic if the specific child elements exist, or to run logic on the + * specified elements. + */ +export function getAllChildJSXElementsByName(node: JSXElement, name: string) { + return node.children?.filter((child) => + getChildJSXElementCallback(child, name) + ) as JSXElement[]; +} diff --git a/packages/eslint-plugin-pf-codemods/src/rules/helpers/getLocalComponentName.ts b/packages/eslint-plugin-pf-codemods/src/rules/helpers/getLocalComponentName.ts index 7fd25eac9..b0e65492c 100644 --- 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,18 @@ -import { ImportSpecifier } from "estree-jsx"; +import { ImportSpecifier, ImportDefaultSpecifier } from "estree-jsx"; +import { getSpecifierFromImports } from "./getSpecifierFromImports"; /** Resolves the local name of an import */ export function getLocalComponentName( - namedImports: ImportSpecifier[], + namedImports: (ImportSpecifier | ImportDefaultSpecifier)[], importedName: string ) { - const componentImport = namedImports.find( - (name) => name.imported.name === importedName - ); - + const componentImport = getSpecifierFromImports(namedImports, importedName); + const isDefaultImport = componentImport?.type === "ImportDefaultSpecifier"; const isAlias = + !isDefaultImport && componentImport?.imported.name !== componentImport?.local.name; - if (componentImport && isAlias) { + if ((componentImport && isAlias) || isDefaultImport) { return componentImport.local.name; } diff --git a/packages/eslint-plugin-pf-codemods/src/rules/helpers/getSpecifierFromImports.ts b/packages/eslint-plugin-pf-codemods/src/rules/helpers/getSpecifierFromImports.ts new file mode 100644 index 000000000..83ab9d75a --- /dev/null +++ b/packages/eslint-plugin-pf-codemods/src/rules/helpers/getSpecifierFromImports.ts @@ -0,0 +1,16 @@ +import { ImportSpecifier, ImportDefaultSpecifier } 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)[], + importedName: string +) { + const importSpecifier = imports.find((imp) => + imp.type === "ImportDefaultSpecifier" + ? imp.local.name === importedName + : imp.imported.name === importedName + ); + + return importSpecifier; +} diff --git a/packages/eslint-plugin-pf-codemods/src/rules/helpers/index.ts b/packages/eslint-plugin-pf-codemods/src/rules/helpers/index.ts index 5e5d6407b..dc0bbe6bd 100644 --- a/packages/eslint-plugin-pf-codemods/src/rules/helpers/index.ts +++ b/packages/eslint-plugin-pf-codemods/src/rules/helpers/index.ts @@ -2,6 +2,7 @@ export * from "./contextReports"; export * from "./findAncestor"; export * from "./fixers"; export * from "./getAttributeName"; +export * from "./getChildJSXElementByName"; export * from "./getCodeModDataTag"; export * from "./getComponentImportName"; export * from "./getDefaultDeclarationString"; @@ -10,6 +11,7 @@ export * from "./getFromPackage"; export * from "./getImportedName"; export * from "./getLocalComponentName"; export * from "./getNodeName"; +export * from "./getSpecifierFromImports"; export * from "./getText"; export * from "./hasCodemodDataTag"; export * from "./helpers"; @@ -17,7 +19,7 @@ export * from "./importAndExport"; export * from "./includesImport"; export * from "./interfaces"; export * from "./JSXAttributes"; -export * from "./JSXElements"; +export * from "./nodeIsComponentNamed"; export * from "./nodeMatches"; export * from "./pfPackageMatches"; export * from "./removeElement"; diff --git a/packages/eslint-plugin-pf-codemods/src/rules/helpers/nodeIsComponentNamed.ts b/packages/eslint-plugin-pf-codemods/src/rules/helpers/nodeIsComponentNamed.ts new file mode 100644 index 000000000..991a6073b --- /dev/null +++ b/packages/eslint-plugin-pf-codemods/src/rules/helpers/nodeIsComponentNamed.ts @@ -0,0 +1,9 @@ +import { JSXElement } from "estree-jsx"; + +export function nodeIsComponentNamed(node: JSXElement, componentName: string) { + if (node.openingElement.name.type === "JSXIdentifier") { + return node.openingElement.name.name === componentName; + } + + return false; +} diff --git a/packages/eslint-plugin-pf-codemods/src/rules/v6/emptyStateHeaderMoveIntoEmptyState/emptyStateHeader-move-into-emptyState.ts b/packages/eslint-plugin-pf-codemods/src/rules/v6/emptyStateHeaderMoveIntoEmptyState/emptyStateHeader-move-into-emptyState.ts index 80027d92b..655ba2302 100644 --- a/packages/eslint-plugin-pf-codemods/src/rules/v6/emptyStateHeaderMoveIntoEmptyState/emptyStateHeader-move-into-emptyState.ts +++ b/packages/eslint-plugin-pf-codemods/src/rules/v6/emptyStateHeaderMoveIntoEmptyState/emptyStateHeader-move-into-emptyState.ts @@ -10,7 +10,7 @@ import { getAttribute, getAttributeText, getAttributeValueText, - getChildElementByName, + getChildJSXElementByName, getDefaultImportsFromPackage, getExpression, getFromPackage, @@ -159,7 +159,7 @@ module.exports = { return; } - const header = getChildElementByName(node, "EmptyStateHeader"); + const header = getChildJSXElementByName(node, "EmptyStateHeader"); const emptyStateTitleTextAttribute = getAttribute(node, "titleText"); if (!header && emptyStateTitleTextAttribute) { @@ -168,7 +168,7 @@ module.exports = { return; } - const titleChild = getChildElementByName(node, "Title"); + const titleChild = getChildJSXElementByName(node, "Title"); if ( (!header || header.type !== "JSXElement") && @@ -194,7 +194,7 @@ module.exports = { removeElements.push(titleChild); } - const emptyStateIconChild = getChildElementByName( + const emptyStateIconChild = getChildJSXElementByName( node, "EmptyStateIcon" ); diff --git a/packages/eslint-plugin-pf-codemods/src/rules/v6/mastheadStructureChanges/masthead-structure-changes.ts b/packages/eslint-plugin-pf-codemods/src/rules/v6/mastheadStructureChanges/masthead-structure-changes.ts index 95f47fe5f..d980355d3 100644 --- a/packages/eslint-plugin-pf-codemods/src/rules/v6/mastheadStructureChanges/masthead-structure-changes.ts +++ b/packages/eslint-plugin-pf-codemods/src/rules/v6/mastheadStructureChanges/masthead-structure-changes.ts @@ -3,7 +3,7 @@ import { ImportSpecifier } from "estree-jsx"; import { JSXOpeningElementWithParent } from "../../helpers"; import { getAllImportsFromPackage, - getChildElementByName, + getChildJSXElementByName, getImportedName, getLocalComponentName, hasCodeModDataTag, @@ -21,7 +21,7 @@ function moveNodeIntoMastheadMain( } const localMastheadMain = getLocalComponentName(namedImports, "MastheadMain"); - const mastheadMain = getChildElementByName( + const mastheadMain = getChildJSXElementByName( node.parent.parent, localMastheadMain ); diff --git a/packages/eslint-plugin-pf-codemods/src/rules/v6/wizardFooterWarnUpdateMarkup/wizardFooter-warn-update-markup.ts b/packages/eslint-plugin-pf-codemods/src/rules/v6/wizardFooterWarnUpdateMarkup/wizardFooter-warn-update-markup.ts index 5741f38f6..dab342646 100644 --- a/packages/eslint-plugin-pf-codemods/src/rules/v6/wizardFooterWarnUpdateMarkup/wizardFooter-warn-update-markup.ts +++ b/packages/eslint-plugin-pf-codemods/src/rules/v6/wizardFooterWarnUpdateMarkup/wizardFooter-warn-update-markup.ts @@ -3,7 +3,7 @@ import { JSXElement } from "estree-jsx"; import { getFromPackage, getAttribute, - getAllChildElementsByName, + getAllChildJSXElementsByName, } from "../../helpers"; // https://github.com/patternfly/patternfly-react/pull/10378 @@ -29,10 +29,15 @@ module.exports = { ) { const wizardFooterProp = getAttribute(node, "footer"); const wizardSteps = wizardStepImport - ? getAllChildElementsByName(node, wizardStepImport.local.name) + ? getAllChildJSXElementsByName( + node, + wizardStepImport.local.name + ) : undefined; const allWizardStepsHaveFooter = wizardSteps - ? wizardSteps.every((step) => getAttribute(step, "footer")) + ? (wizardSteps as JSXElement[]).every((step) => + getAttribute(step, "footer") + ) : false; if (wizardFooterProp || allWizardStepsHaveFooter) { From 7ae6100e8fabde3e9ae29fb8eff40c2ca43454c3 Mon Sep 17 00:00:00 2001 From: Eric Olkowski Date: Fri, 16 Aug 2024 10:46:12 -0400 Subject: [PATCH 2/4] Added helpers and rule logic --- .../rules/helpers/getNodeForAttributeFixer.ts | 41 ++++++ .../src/rules/helpers/getObjectProperty.ts | 39 ++++++ .../src/rules/helpers/index.ts | 3 + .../removePropertiesFromObjectExpression.ts | 42 ++++++ .../card-updated-clickable-markup.md | 17 +++ .../card-updated-clickable-markup.test.ts | 121 ++++++++++++++++++ .../card-updated-clickable-markup.ts | 118 +++++++++++++++++ .../cardUpdatedClickableMarkupInput.tsx | 31 +++++ .../cardUpdatedClickableMarkupOutput.tsx | 22 ++++ 9 files changed, 434 insertions(+) create mode 100644 packages/eslint-plugin-pf-codemods/src/rules/helpers/getNodeForAttributeFixer.ts create mode 100644 packages/eslint-plugin-pf-codemods/src/rules/helpers/getObjectProperty.ts create mode 100644 packages/eslint-plugin-pf-codemods/src/rules/helpers/removePropertiesFromObjectExpression.ts create mode 100644 packages/eslint-plugin-pf-codemods/src/rules/v6/cardUpdatedClickableMarkup/card-updated-clickable-markup.md create mode 100644 packages/eslint-plugin-pf-codemods/src/rules/v6/cardUpdatedClickableMarkup/card-updated-clickable-markup.test.ts create mode 100644 packages/eslint-plugin-pf-codemods/src/rules/v6/cardUpdatedClickableMarkup/card-updated-clickable-markup.ts create mode 100644 packages/eslint-plugin-pf-codemods/src/rules/v6/cardUpdatedClickableMarkup/cardUpdatedClickableMarkupInput.tsx create mode 100644 packages/eslint-plugin-pf-codemods/src/rules/v6/cardUpdatedClickableMarkup/cardUpdatedClickableMarkupOutput.tsx diff --git a/packages/eslint-plugin-pf-codemods/src/rules/helpers/getNodeForAttributeFixer.ts b/packages/eslint-plugin-pf-codemods/src/rules/helpers/getNodeForAttributeFixer.ts new file mode 100644 index 000000000..a55ecd0df --- /dev/null +++ b/packages/eslint-plugin-pf-codemods/src/rules/helpers/getNodeForAttributeFixer.ts @@ -0,0 +1,41 @@ +import { Rule } from "eslint"; +import { JSXAttribute } from "estree-jsx"; +import { getVariableDeclaration } from "./JSXAttributes"; + +/** Used to find the node where a prop value is initially assigned, to then be passed + * as a fixer function's nodeOrToken argument. Useful for when a prop may have an inline value, e.g. ``, or + * is passed an identifier, e.g. `const val = "value"; ` + */ +export function getNodeForAttributeFixer( + context: Rule.RuleContext, + attribute: JSXAttribute +) { + 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; + } + + if (attribute.value.type === "Literal") { + return attribute.value; + } + if ( + attribute.value.type === "JSXExpressionContainer" && + ["ObjectExpression", "MemberExpression"].includes( + attribute.value.expression.type + ) + ) { + return attribute.value.expression; + } +} diff --git a/packages/eslint-plugin-pf-codemods/src/rules/helpers/getObjectProperty.ts b/packages/eslint-plugin-pf-codemods/src/rules/helpers/getObjectProperty.ts new file mode 100644 index 000000000..8446eaf23 --- /dev/null +++ b/packages/eslint-plugin-pf-codemods/src/rules/helpers/getObjectProperty.ts @@ -0,0 +1,39 @@ +import { Rule } from "eslint"; +import { Property, Identifier } from "estree-jsx"; +import { getVariableDeclaration } from "./JSXAttributes"; + +/** Can be used to run logic on the specified property of an ObjectExpression or + * only if the specified property exists. + */ +export function getObjectProperty( + context: Rule.RuleContext, + properties: Property[], + name: string +) { + if (!properties.length) { + return; + } + + const matchedProperty = properties.find((property) => { + const isIdentifier = property.key.type === "Identifier"; + const { computed } = property; + + // E.g. const key = "key"; {[key]: value} + if (isIdentifier && computed) { + const scope = context.getSourceCode().getScope(property); + const propertyName = (property.key as Identifier).name; + const propertyVariable = getVariableDeclaration(propertyName, scope); + return propertyVariable?.defs[0].node.init.value === name; + } + // E.g. {key: value} + if (isIdentifier && !computed) { + return (property.key as Identifier).name === name; + } + // E.g. {"key": value} or {["key"]: value} + if (property.key.type === "Literal") { + return property.key.value === name; + } + }); + + return matchedProperty; +} diff --git a/packages/eslint-plugin-pf-codemods/src/rules/helpers/index.ts b/packages/eslint-plugin-pf-codemods/src/rules/helpers/index.ts index dc0bbe6bd..196081667 100644 --- a/packages/eslint-plugin-pf-codemods/src/rules/helpers/index.ts +++ b/packages/eslint-plugin-pf-codemods/src/rules/helpers/index.ts @@ -10,7 +10,9 @@ export * from "./getEndRange"; export * from "./getFromPackage"; export * from "./getImportedName"; export * from "./getLocalComponentName"; +export * from "./getNodeForAttributeFixer"; export * from "./getNodeName"; +export * from "./getObjectProperty"; export * from "./getSpecifierFromImports"; export * from "./getText"; export * from "./hasCodemodDataTag"; @@ -24,4 +26,5 @@ export * from "./nodeMatches"; export * from "./pfPackageMatches"; export * from "./removeElement"; export * from "./removeEmptyLineAfter"; +export * from "./removePropertiesFromObjectExpression"; export * from "./renameProps"; diff --git a/packages/eslint-plugin-pf-codemods/src/rules/helpers/removePropertiesFromObjectExpression.ts b/packages/eslint-plugin-pf-codemods/src/rules/helpers/removePropertiesFromObjectExpression.ts new file mode 100644 index 000000000..288229241 --- /dev/null +++ b/packages/eslint-plugin-pf-codemods/src/rules/helpers/removePropertiesFromObjectExpression.ts @@ -0,0 +1,42 @@ +import { Property } from "estree-jsx"; + +/** Can be used to take the returned array and join it into a replacement string of object + * key:value pairs. + */ +export function removePropertiesFromObjectExpression( + currentProperties: Property[], + propertiesToRemove: (Property | undefined)[] +) { + if (!currentProperties) { + return []; + } + if (!propertiesToRemove) { + return currentProperties; + } + + const propertyNamesToRemove = propertiesToRemove.map((property) => { + if (property?.key.type === "Identifier") { + return property.key.name; + } + + if (property?.key.type === "Literal") { + return property.key.value; + } + + return ""; + }); + + const propertiesToKeep = currentProperties.filter((property) => { + if (property.key.type === "Identifier") { + return !propertyNamesToRemove.includes(property.key.name); + } + + if (property.key.type === "Literal") { + return propertyNamesToRemove.includes(property.key.value); + } + + return false; + }); + + return propertiesToKeep; +} diff --git a/packages/eslint-plugin-pf-codemods/src/rules/v6/cardUpdatedClickableMarkup/card-updated-clickable-markup.md b/packages/eslint-plugin-pf-codemods/src/rules/v6/cardUpdatedClickableMarkup/card-updated-clickable-markup.md new file mode 100644 index 000000000..3bf9cfbcf --- /dev/null +++ b/packages/eslint-plugin-pf-codemods/src/rules/v6/cardUpdatedClickableMarkup/card-updated-clickable-markup.md @@ -0,0 +1,17 @@ +### card-updated-clickable-markup [(#10859)](https://github.com/patternfly/patternfly-react/pull/10859) + +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. Passing them in will not cause any errors, but running the fix for this rule will remove them. + +#### Examples + +In: + +```jsx +%inputExample% +``` + +Out: + +```jsx +%outputExample% +``` diff --git a/packages/eslint-plugin-pf-codemods/src/rules/v6/cardUpdatedClickableMarkup/card-updated-clickable-markup.test.ts b/packages/eslint-plugin-pf-codemods/src/rules/v6/cardUpdatedClickableMarkup/card-updated-clickable-markup.test.ts new file mode 100644 index 000000000..d61619aec --- /dev/null +++ b/packages/eslint-plugin-pf-codemods/src/rules/v6/cardUpdatedClickableMarkup/card-updated-clickable-markup.test.ts @@ -0,0 +1,121 @@ +const ruleTester = require("../../ruletester"); +import * as rule from "./card-updated-clickable-markup"; + +ruleTester.run("card-updated-clickable-markup", rule, { + valid: [ + { + code: ``, + }, + { + code: ``, + }, + { + code: `import { Card } from '@patternfly/react-core'; `, + }, + { + code: `import { Card } from '@patternfly/react-core'; `, + }, + { + code: `import { CardHeader } from '@patternfly/react-core'; `, + }, + ], + invalid: [ + { + code: `import { Card, CardHeader } from '@patternfly/react-core'; `, + output: `import { Card, CardHeader } from '@patternfly/react-core'; `, + errors: [ + { + message: "The markup for clickable-only cards has been updated.", + type: "JSXElement", + }, + ], + }, + { + code: `import { Card, CardHeader } from '@patternfly/react-core'; `, + output: `import { Card, CardHeader } from '@patternfly/react-core'; `, + 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.", + type: "JSXElement", + }, + ], + }, + { + code: `import { Card, CardHeader } from '@patternfly/react-core'; `, + output: `import { Card, CardHeader } from '@patternfly/react-core'; `, + 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.`, + type: "JSXElement", + }, + ], + }, + { + code: `import { Card, CardHeader } from '@patternfly/react-core'; const obj = {name: 'Test', selectableActionId: 'Id'}; `, + output: `import { Card, CardHeader } from '@patternfly/react-core'; const obj = {}; `, + 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.", + type: "JSXElement", + }, + ], + }, + { + code: `import { Card, CardHeader } from '@patternfly/react-core'; const obj = {to: "#", name: 'Test', extra: "thing", selectableActionId: 'Id'}; `, + output: `import { Card, CardHeader } from '@patternfly/react-core'; const obj = {to: "#", extra: "thing"}; `, + 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.", + type: "JSXElement", + }, + ], + }, + // Aliased + { + code: `import { Card as CustomCard, CardHeader as CustomCardHeader } from '@patternfly/react-core'; `, + output: `import { Card as CustomCard, CardHeader as CustomCardHeader } from '@patternfly/react-core'; `, + errors: [ + { + message: "The markup for clickable-only cards has been updated.", + type: "JSXElement", + }, + ], + }, + // Dist Imports + { + code: `import { Card, CardHeader } from '@patternfly/react-core/dist/esm/components/Card/index.js'; `, + output: `import { Card, CardHeader } from '@patternfly/react-core/dist/esm/components/Card/index.js'; `, + errors: [ + { + message: "The markup for clickable-only cards has been updated.", + type: "JSXElement", + }, + ], + }, + { + code: `import { Card, CardHeader } from '@patternfly/react-core/dist/js/components/Card/index.js'; `, + output: `import { Card, CardHeader } from '@patternfly/react-core/dist/js/components/Card/index.js'; `, + errors: [ + { + message: "The markup for clickable-only cards has been updated.", + type: "JSXElement", + }, + ], + }, + { + code: `import { Card, CardHeader } from '@patternfly/react-core/dist/dynamic/components/Card/index.js'; `, + output: `import { Card, CardHeader } from '@patternfly/react-core/dist/dynamic/components/Card/index.js'; `, + errors: [ + { + message: "The markup for clickable-only cards has been updated.", + type: "JSXElement", + }, + ], + }, + ], +}); diff --git a/packages/eslint-plugin-pf-codemods/src/rules/v6/cardUpdatedClickableMarkup/card-updated-clickable-markup.ts b/packages/eslint-plugin-pf-codemods/src/rules/v6/cardUpdatedClickableMarkup/card-updated-clickable-markup.ts new file mode 100644 index 000000000..c440f4e86 --- /dev/null +++ b/packages/eslint-plugin-pf-codemods/src/rules/v6/cardUpdatedClickableMarkup/card-updated-clickable-markup.ts @@ -0,0 +1,118 @@ +import { Rule } from "eslint"; +import { JSXElement, Property, Literal } from "estree-jsx"; +import { + getAllImportsFromPackage, + checkMatchingJSXOpeningElement, + getAttribute, + getAttributeValue, + getSpecifierFromImports, + getChildJSXElementByName, + getObjectProperty, + removePropertiesFromObjectExpression, + getNodeForAttributeFixer, +} from "../../helpers"; + +// https://github.com/patternfly/patternfly-react/pull/10859 +module.exports = { + meta: { fixable: "code" }, + create: function (context: Rule.RuleContext) { + const basePackage = "@patternfly/react-core"; + const componentImports = getAllImportsFromPackage(context, basePackage, [ + "Card", + "CardHeader", + ]); + const cardImport = getSpecifierFromImports(componentImports, "Card"); + const cardHeaderImport = getSpecifierFromImports( + componentImports, + "CardHeader" + ); + + // Actionable cards won't work as intended if both components aren't imported, hence + // we shouldn't need to relay any message if that is the case + return !cardImport || !cardHeaderImport + ? {} + : { + JSXElement(node: JSXElement) { + if ( + checkMatchingJSXOpeningElement(node.openingElement, cardImport) + ) { + const isClickableProp = getAttribute(node, "isClickable"); + const isSelectableProp = getAttribute(node, "isSelectable"); + + if ((isClickableProp && isSelectableProp) || !isClickableProp) { + return; + } + + const cardHeaderChild = getChildJSXElementByName( + node, + cardHeaderImport.local.name + ); + const selectableActionsProp = cardHeaderChild + ? getAttribute(cardHeaderChild, "selectableActions") + : undefined; + if (!cardHeaderChild || !selectableActionsProp) { + return; + } + const selectableActionsValue = getAttributeValue( + context, + selectableActionsProp.value + ); + const nameProperty = getObjectProperty( + context, + selectableActionsValue, + "name" + ); + const idProperty = getObjectProperty( + context, + selectableActionsValue, + "selectableActionId" + ); + + const baseMessage = + "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." + : "" + }`; + context.report({ + node, + message, + fix(fixer) { + const validPropertiesToRemove = [ + nameProperty, + idProperty, + ].filter((property) => !!property); + if ( + !validPropertiesToRemove.length || + !selectableActionsProp.value + ) { + return []; + } + const propertiesToKeep = removePropertiesFromObjectExpression( + selectableActionsValue, + validPropertiesToRemove + ); + const replacementProperties = propertiesToKeep + .map((property: Property) => + context.getSourceCode().getText(property) + ) + .join(", "); + + const nodeToUpdate = getNodeForAttributeFixer( + context, + selectableActionsProp + ); + return fixer.replaceText( + nodeToUpdate, + propertiesToKeep.length + ? `{${replacementProperties}}` + : "{}" + ); + }, + }); + } + }, + }; + }, +}; diff --git a/packages/eslint-plugin-pf-codemods/src/rules/v6/cardUpdatedClickableMarkup/cardUpdatedClickableMarkupInput.tsx b/packages/eslint-plugin-pf-codemods/src/rules/v6/cardUpdatedClickableMarkup/cardUpdatedClickableMarkupInput.tsx new file mode 100644 index 000000000..5df735c1d --- /dev/null +++ b/packages/eslint-plugin-pf-codemods/src/rules/v6/cardUpdatedClickableMarkup/cardUpdatedClickableMarkupInput.tsx @@ -0,0 +1,31 @@ +import { Card, CardHeader } from "@patternfly/react-core"; + +export const CardUpdatedClickableMarkupInput = () => { + const selectableActionsObj = { name: "Test2", selectableActionId: "Id2" }; + const selectableActionsObjMany = { + to: "#", + name: "Test2", + selectableActionProps: {}, + selectableActionId: "Id2", + }; + + return ( + <> + + {}, + }} + /> + + + + + + + + + ); +}; diff --git a/packages/eslint-plugin-pf-codemods/src/rules/v6/cardUpdatedClickableMarkup/cardUpdatedClickableMarkupOutput.tsx b/packages/eslint-plugin-pf-codemods/src/rules/v6/cardUpdatedClickableMarkup/cardUpdatedClickableMarkupOutput.tsx new file mode 100644 index 000000000..469780ad1 --- /dev/null +++ b/packages/eslint-plugin-pf-codemods/src/rules/v6/cardUpdatedClickableMarkup/cardUpdatedClickableMarkupOutput.tsx @@ -0,0 +1,22 @@ +import { Card, CardHeader } from "@patternfly/react-core"; + +export const CardUpdatedClickableMarkupInput = () => { + const selectableActionsObj = {}; + const selectableActionsObjMany = {to: "#", selectableActionProps: {}}; + + return ( + <> + + + + + + + + + + + ); +}; From ccbd46ba9d438a04971ece6d301a489c75297bf3 Mon Sep 17 00:00:00 2001 From: Eric Olkowski Date: Fri, 16 Aug 2024 10:51:52 -0400 Subject: [PATCH 3/4] Added test for 1 valid and 1 invalid property --- .../card-updated-clickable-markup.test.ts | 17 +++++++++++++++++ 1 file changed, 17 insertions(+) diff --git a/packages/eslint-plugin-pf-codemods/src/rules/v6/cardUpdatedClickableMarkup/card-updated-clickable-markup.test.ts b/packages/eslint-plugin-pf-codemods/src/rules/v6/cardUpdatedClickableMarkup/card-updated-clickable-markup.test.ts index d61619aec..01765df28 100644 --- a/packages/eslint-plugin-pf-codemods/src/rules/v6/cardUpdatedClickableMarkup/card-updated-clickable-markup.test.ts +++ b/packages/eslint-plugin-pf-codemods/src/rules/v6/cardUpdatedClickableMarkup/card-updated-clickable-markup.test.ts @@ -20,6 +20,7 @@ ruleTester.run("card-updated-clickable-markup", rule, { }, ], invalid: [ + // No invalid properties { code: `import { Card, CardHeader } from '@patternfly/react-core'; `, output: `import { Card, CardHeader } from '@patternfly/react-core'; `, @@ -30,6 +31,19 @@ ruleTester.run("card-updated-clickable-markup", rule, { }, ], }, + // 1 valid property + 1 invalid property + { + code: `import { Card, CardHeader } from '@patternfly/react-core'; `, + output: `import { Card, CardHeader } from '@patternfly/react-core'; `, + 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.", + type: "JSXElement", + }, + ], + }, + // 2 invalid properties { code: `import { Card, CardHeader } from '@patternfly/react-core'; `, output: `import { Card, CardHeader } from '@patternfly/react-core'; `, @@ -41,6 +55,7 @@ ruleTester.run("card-updated-clickable-markup", rule, { }, ], }, + // 1 valid property + 2 invalid properties { code: `import { Card, CardHeader } from '@patternfly/react-core'; `, output: `import { Card, CardHeader } from '@patternfly/react-core'; `, @@ -53,6 +68,7 @@ ruleTester.run("card-updated-clickable-markup", rule, { }, ], }, + // Passed as a variable reference { code: `import { Card, CardHeader } from '@patternfly/react-core'; const obj = {name: 'Test', selectableActionId: 'Id'}; `, output: `import { Card, CardHeader } from '@patternfly/react-core'; const obj = {}; `, @@ -64,6 +80,7 @@ ruleTester.run("card-updated-clickable-markup", rule, { }, ], }, + // Passed as a variable reference with 2 valid and 2 invalid properties { code: `import { Card, CardHeader } from '@patternfly/react-core'; const obj = {to: "#", name: 'Test', extra: "thing", selectableActionId: 'Id'}; `, output: `import { Card, CardHeader } from '@patternfly/react-core'; const obj = {to: "#", extra: "thing"}; `, From 2d2a870fbd104cee453b8d5c86c2447821f692a5 Mon Sep 17 00:00:00 2001 From: Eric Olkowski Date: Thu, 22 Aug 2024 13:27:13 -0400 Subject: [PATCH 4/4] Pr feedback --- .../rules/helpers/getLocalComponentName.ts | 8 ++-- .../rules/helpers/getNodeForAttributeFixer.ts | 47 ++++++++++--------- .../rules/helpers/getSpecifierFromImports.ts | 10 ++-- .../removePropertiesFromObjectExpression.ts | 2 +- .../card-updated-clickable-markup.test.ts | 30 ++++++------ .../card-updated-clickable-markup.ts | 12 +++-- 6 files changed, 55 insertions(+), 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 index b0e65492c..643855a21 100644 --- 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 index a55ecd0df..2c72749d4 100644 --- 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 index 83ab9d75a..7d4693e7c 100644 --- 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 index 288229241..91010a35f 100644 --- 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/packages/eslint-plugin-pf-codemods/src/rules/v6/cardUpdatedClickableMarkup/card-updated-clickable-markup.test.ts b/packages/eslint-plugin-pf-codemods/src/rules/v6/cardUpdatedClickableMarkup/card-updated-clickable-markup.test.ts index 01765df28..10f32858a 100644 --- a/packages/eslint-plugin-pf-codemods/src/rules/v6/cardUpdatedClickableMarkup/card-updated-clickable-markup.test.ts +++ b/packages/eslint-plugin-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'; `, 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'; `, 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'; `, 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'; `, 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 = {}; `, 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"}; `, 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'; `, 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'; `, 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'; `, 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'; `, errors: [ { - message: "The markup for clickable-only cards has been updated.", + message: baseMessage, type: "JSXElement", }, ], diff --git a/packages/eslint-plugin-pf-codemods/src/rules/v6/cardUpdatedClickableMarkup/card-updated-clickable-markup.ts b/packages/eslint-plugin-pf-codemods/src/rules/v6/cardUpdatedClickableMarkup/card-updated-clickable-markup.ts index c440f4e86..5940c269f 100644 --- a/packages/eslint-plugin-pf-codemods/src/rules/v6/cardUpdatedClickableMarkup/card-updated-clickable-markup.ts +++ b/packages/eslint-plugin-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({