diff --git a/packages/eslint-plugin-pf-codemods/lib/helpers.js b/packages/eslint-plugin-pf-codemods/lib/helpers.js index eca3b7494..44fa3f56d 100644 --- a/packages/eslint-plugin-pf-codemods/lib/helpers.js +++ b/packages/eslint-plugin-pf-codemods/lib/helpers.js @@ -1,158 +1,205 @@ function getPackageImports(context, packageName) { - return context.getSourceCode().ast.body - .filter(node => node.type === 'ImportDeclaration') - .filter(node => node.source.value === packageName) - .map(node => node.specifiers) + return context + .getSourceCode() + .ast.body.filter((node) => node.type === "ImportDeclaration") + .filter((node) => node.source.value === packageName) + .map((node) => node.specifiers) .reduce((acc, val) => acc.concat(val), []); } function renameProps0(context, imports, node, renames) { - if (imports.map(imp => imp.local.name).includes(node.name.name)) { - const renamedProps = renames[node.name.name] || renames[ - imports.find(imp => imp.local?.name === node.name.name)?.imported?.name - ]; + if (imports.map((imp) => imp.local.name).includes(node.name.name)) { + const renamedProps = + renames[node.name.name] || + renames[ + imports.find((imp) => imp.local?.name === node.name.name)?.imported + ?.name + ]; node.attributes - .filter(attribute => attribute.name && renamedProps.hasOwnProperty(attribute.name.name)) - .forEach(attribute => { - if (renamedProps[attribute.name.name] === '') { + .filter( + (attribute) => + attribute.name && renamedProps.hasOwnProperty(attribute.name.name) + ) + .forEach((attribute) => { + if (renamedProps[attribute.name.name] === "") { context.report({ node, message: `${attribute.name.name} prop for ${node.name.name} has been removed`, fix(fixer) { - return fixer.replaceText(attribute, ''); - } + return fixer.replaceText(attribute, ""); + }, }); - } - else { + } else { context.report({ node, - message: `${attribute.name.name} prop for ${node.name.name} has been renamed to ${renamedProps[attribute.name.name]}`, + message: `${attribute.name.name} prop for ${ + node.name.name + } has been renamed to ${renamedProps[attribute.name.name]}`, fix(fixer) { - return fixer.replaceText(attribute.name, renamedProps[attribute.name.name]); - } + return fixer.replaceText( + attribute.name, + renamedProps[attribute.name.name] + ); + }, }); } }); } } -function renameProps(renames, packageName='@patternfly/react-core') { - return function(context) { - const imports = getPackageImports(context, packageName) - .filter(specifier => Object.keys(renames).includes(specifier.imported.name)); - - return imports.length === 0 ? {} : { - JSXOpeningElement(node) { - renameProps0(context, imports, node, renames); - } - }; - } +function renameProps(renames, packageName = "@patternfly/react-core") { + return function (context) { + const imports = getPackageImports(context, packageName).filter( + (specifier) => Object.keys(renames).includes(specifier.imported.name) + ); + + return imports.length === 0 + ? {} + : { + JSXOpeningElement(node) { + renameProps0(context, imports, node, renames); + }, + }; + }; } -function renameProp(components, propMap, message, replaceAttribute, leaveComment = true) { - if (typeof components === 'string') { - components = [ components ]; +function renameProp( + components, + propMap, + message, + replaceAttribute, + leaveComment = true +) { + if (typeof components === "string") { + components = [components]; } - return function(context) { - const imports = getPackageImports(context, '@patternfly/react-core') - .filter(specifier => components.includes(specifier.imported.name)); - return imports.length === 0 ? {} : { - JSXOpeningElement(node) { - if (imports.find(imp => imp.local.name === node.name.name)) { - const namedAttributes = node.attributes.filter(attr => attr.name); + return function (context) { + const imports = getPackageImports(context, "@patternfly/react-core").filter( + (specifier) => components.includes(specifier.imported.name) + ); + return imports.length === 0 + ? {} + : { + JSXOpeningElement(node) { + if (imports.find((imp) => imp.local.name === node.name.name)) { + const namedAttributes = node.attributes.filter( + (attr) => attr.name + ); - namedAttributes.filter(attr => Object.keys(propMap).includes(attr.name.name)) - .forEach(attribute => { - const newName = propMap[attribute.name.name]; - context.report({ - node, - message: message(node, attribute, newName), - fix(fixer) { - // Delete entire prop if newName is empty - if (!newName || replaceAttribute) { - /** - * (dallas) - * TODO: remove extra space? the following works but issues arise when attempting to remove multiple props. - * i assume the ranges become invalid in the forEach loop? even though it seems to track attr correctly - * (see datalist-remove-ondrags for example) - * - * const tokenBefore = context.getSourceCode().getTokenBefore(attribute); - * return fixer.replaceTextRange([tokenBefore.range[1], attribute.range[1]], ''); - * - * or - * - * return fixer.replaceTextRange([attribute.range[0] - 1, attribute.range[1]], ''); - */ - return fixer.replaceText(attribute, newName); - } - const newNameAttrName = newName.split('=')[0]; - // Leave a comment if there's an existing prop with this name - if (namedAttributes.find(attr => attr.name.name === newNameAttrName)) { - return fixer.replaceText(attribute, leaveComment ? `/* ${newName} */` : ''); - } - return fixer.replaceText( - attribute.name, - newName - ); - } - }) - }); - } - } - }; - } + namedAttributes + .filter((attr) => Object.keys(propMap).includes(attr.name.name)) + .forEach((attribute) => { + const newName = propMap[attribute.name.name]; + context.report({ + node, + message: message(node, attribute, newName), + fix(fixer) { + // Delete entire prop if newName is empty + if (!newName || replaceAttribute) { + /** + * (dallas) + * TODO: remove extra space? the following works but issues arise when attempting to remove multiple props. + * i assume the ranges become invalid in the forEach loop? even though it seems to track attr correctly + * (see datalist-remove-ondrags for example) + * + * const tokenBefore = context.getSourceCode().getTokenBefore(attribute); + * return fixer.replaceTextRange([tokenBefore.range[1], attribute.range[1]], ''); + * + * or + * + * return fixer.replaceTextRange([attribute.range[0] - 1, attribute.range[1]], ''); + */ + return fixer.replaceText(attribute, newName); + } + const newNameAttrName = newName.split("=")[0]; + // Leave a comment if there's an existing prop with this name + if ( + namedAttributes.find( + (attr) => attr.name.name === newNameAttrName + ) + ) { + return fixer.replaceText( + attribute, + leaveComment ? `/* ${newName} */` : "" + ); + } + return fixer.replaceText(attribute.name, newName); + }, + }); + }); + } + }, + }; + }; } function renameComponents( componentMap, condition = (_context, _package) => true, - message = (prevName, newName) => `${prevName} has been replaced with ${newName}`, - package = '@patternfly/react-core' + message = (prevName, newName) => + `${prevName} has been replaced with ${newName}`, + package = "@patternfly/react-core" ) { - return function(context) { - const imports = getPackageImports(context, package) - .filter(specifier => Object.keys(componentMap).includes(specifier.imported.name)); - - return imports.length === 0 || !condition(context, package) ? {} : { - ImportDeclaration(node) { - ensureImports(context, node, package, Object.values(componentMap)); - }, - JSXIdentifier(node) { - const nodeName = node.name; - const importedNode = imports.find(imp => imp.local.name === nodeName); - if ( - imports.find(imp => imp.imported.name === nodeName) && - importedNode.imported.name === importedNode.local.name // don't rename an aliased component - ) { - // if data-codemods attribute, do nothing - const parentNode = node.parent; - const isOpeningTag = parentNode.type === 'JSXOpeningElement'; - const openingTagAttributes = isOpeningTag ? parentNode.attributes : parentNode.parent.openingElement.attributes; - const hasDataAttr = openingTagAttributes && openingTagAttributes.filter(attr => attr.name?.name === 'data-codemods').length; - if (hasDataAttr) { - return; - } - // if no data-codemods && opening tag, add attribute & rename - // if no data-codemods && closing tag, rename - const newName = componentMap[nodeName]; - const updateTagName = node => context.getSourceCode().getText(node).replace(nodeName, newName); - const addDataAttr = jsxStr => `${jsxStr.slice(0, -1)} data-codemods="true">`; - const newOpeningParentTag = newName.includes('Toolbar') - ? addDataAttr(updateTagName(parentNode)) - : updateTagName(parentNode); - context.report({ - node, - message: message(nodeName, newName), - fix(fixer) { - return isOpeningTag - ? fixer.replaceText(parentNode, newOpeningParentTag) - : fixer.replaceText(node, updateTagName(node)); + return function (context) { + const imports = getPackageImports(context, package).filter((specifier) => + Object.keys(componentMap).includes(specifier.imported.name) + ); + + return imports.length === 0 || !condition(context, package) + ? {} + : { + ImportDeclaration(node) { + ensureImports(context, node, package, Object.values(componentMap)); + }, + JSXIdentifier(node) { + const nodeName = node.name; + const importedNode = imports.find( + (imp) => imp.local.name === nodeName + ); + if ( + imports.find((imp) => imp.imported.name === nodeName) && + importedNode.imported.name === importedNode.local.name // don't rename an aliased component + ) { + // if data-codemods attribute, do nothing + const parentNode = node.parent; + const isOpeningTag = parentNode.type === "JSXOpeningElement"; + const openingTagAttributes = isOpeningTag + ? parentNode.attributes + : parentNode.parent.openingElement.attributes; + const hasDataAttr = + openingTagAttributes && + openingTagAttributes.filter( + (attr) => attr.name?.name === "data-codemods" + ).length; + if (hasDataAttr) { + return; + } + // if no data-codemods && opening tag, add attribute & rename + // if no data-codemods && closing tag, rename + const newName = componentMap[nodeName]; + const updateTagName = (node) => + context + .getSourceCode() + .getText(node) + .replace(nodeName, newName); + const addDataAttr = (jsxStr) => + `${jsxStr.slice(0, -1)} data-codemods="true">`; + const newOpeningParentTag = newName.includes("Toolbar") + ? addDataAttr(updateTagName(parentNode)) + : updateTagName(parentNode); + context.report({ + node, + message: message(nodeName, newName), + fix(fixer) { + return isOpeningTag + ? fixer.replaceText(parentNode, newOpeningParentTag) + : fixer.replaceText(node, updateTagName(node)); + }, + }); } - }); - } - } - }; - } + }, + }; + }; } function ensureImports(context, node, package, imports) { @@ -160,12 +207,14 @@ function ensureImports(context, node, package, imports) { return; } const patternflyImports = getPackageImports(context, package); - const patternflyImportNames = patternflyImports.map(imp => imp.imported.name); - const myImports = node.specifiers.map(imp => imp.imported.name); + const patternflyImportNames = patternflyImports.map( + (imp) => imp.imported.name + ); + const myImports = node.specifiers.map((imp) => imp.imported.name); const missingImports = imports - .filter(imp => !patternflyImportNames.includes(imp)) // not added by consumer - .filter(imp => !myImports.includes(imp)) // not added by this rule - .join(', '); + .filter((imp) => !patternflyImportNames.includes(imp)) // not added by consumer + .filter((imp) => !myImports.includes(imp)) // not added by this rule + .join(", "); if (missingImports) { const lastSpecifier = node.specifiers[node.specifiers.length - 1]; context.report({ @@ -173,7 +222,7 @@ function ensureImports(context, node, package, imports) { message: `add missing imports ${missingImports} from ${node.source.value}`, fix(fixer) { return fixer.insertTextAfter(lastSpecifier, `, ${missingImports}`); - } + }, }); } } @@ -220,13 +269,13 @@ function addCallbackParam(componentsArray, propMap) { const { type, params } = propProperties; if ( - (params?.length === 1 && + (params?.length >= 1 && ["ArrowFunctionExpression", "Identifier"].includes(type)) || type === "MemberExpression" ) { context.report({ node, - message: `The "${attribute.name.name}" prop for ${node.name.name} has been updated to include the "${newParam}" parameter as its first parameter. "${attribute.name.name}" handlers may require an update.`, + 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) => { @@ -269,5 +318,5 @@ module.exports = { renameProps0, renameProps, renameComponents, - addCallbackParam -} + addCallbackParam, +}; 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 new file mode 100644 index 000000000..da70cfd98 --- /dev/null +++ b/packages/eslint-plugin-pf-codemods/lib/rules/v5/dataListCheck-updated-callback.js @@ -0,0 +1,7 @@ +const { addCallbackParam } = require("../../helpers"); + +// https://github.com/patternfly/patternfly-react/pull/8735 +module.exports = { + meta: { fixable: "code" }, + create: addCallbackParam(["DataListCheck"], { onChange: "_event" }), +}; diff --git a/packages/eslint-plugin-pf-codemods/test/rules/v5/dataList-updated-callback.js b/packages/eslint-plugin-pf-codemods/test/rules/v5/dataList-updated-callback.js index 3498e05f7..cd2e57ce9 100644 --- a/packages/eslint-plugin-pf-codemods/test/rules/v5/dataList-updated-callback.js +++ b/packages/eslint-plugin-pf-codemods/test/rules/v5/dataList-updated-callback.js @@ -26,7 +26,7 @@ ruleTester.run("dataList-updated-callback", rule, { output: `import { DataList } from '@patternfly/react-core'; onSelect(id)} />;`, errors: [ { - message: `The "onSelectDataListItem" prop for DataList has been updated to include the "_event" parameter as its first parameter. "onSelectDataListItem" handlers may require an update.`, + message: `The "onSelectDataListItem" prop for DataList has been updated so that the "_event" parameter is the first parameter. "onSelectDataListItem" handlers may require an update.`, type: "JSXOpeningElement", }, ], @@ -36,7 +36,7 @@ ruleTester.run("dataList-updated-callback", rule, { output: `import { DataList } from '@patternfly/react-core'; onSelect(id)} />;`, errors: [ { - message: `The "onSelectDataListItem" prop for DataList has been updated to include the "_event" parameter as its first parameter. "onSelectDataListItem" handlers may require an update.`, + message: `The "onSelectDataListItem" prop for DataList has been updated so that the "_event" parameter is the first parameter. "onSelectDataListItem" handlers may require an update.`, type: "JSXOpeningElement", }, ], @@ -46,7 +46,7 @@ ruleTester.run("dataList-updated-callback", rule, { output: `import { DataList } from '@patternfly/react-core'; const onSelect = (_event, id) => {}; ;`, errors: [ { - message: `The "onSelectDataListItem" prop for DataList has been updated to include the "_event" parameter as its first parameter. "onSelectDataListItem" handlers may require an update.`, + message: `The "onSelectDataListItem" prop for DataList has been updated so that the "_event" parameter is the first parameter. "onSelectDataListItem" handlers may require an update.`, type: "JSXOpeningElement", }, ], @@ -56,7 +56,7 @@ ruleTester.run("dataList-updated-callback", rule, { output: `import { DataList } from '@patternfly/react-core'; function onSelect(_event, id) {}; ;`, errors: [ { - message: `The "onSelectDataListItem" prop for DataList has been updated to include the "_event" parameter as its first parameter. "onSelectDataListItem" handlers may require an update.`, + message: `The "onSelectDataListItem" prop for DataList has been updated so that the "_event" parameter is the first parameter. "onSelectDataListItem" handlers may require an update.`, type: "JSXOpeningElement", }, ], @@ -66,7 +66,7 @@ ruleTester.run("dataList-updated-callback", rule, { output: `import { DataList } from '@patternfly/react-core'; ;`, errors: [ { - message: `The "onSelectDataListItem" prop for DataList has been updated to include the "_event" parameter as its first parameter. "onSelectDataListItem" handlers may require an update.`, + message: `The "onSelectDataListItem" prop for DataList has been updated so that the "_event" parameter is the first parameter. "onSelectDataListItem" handlers may require an update.`, type: "JSXOpeningElement", }, ], @@ -76,7 +76,7 @@ ruleTester.run("dataList-updated-callback", rule, { output: `import { DataList as PFDataList } from '@patternfly/react-core'; onSelect(id)} />;`, errors: [ { - message: `The "onSelectDataListItem" prop for PFDataList has been updated to include the "_event" parameter as its first parameter. "onSelectDataListItem" handlers may require an update.`, + message: `The "onSelectDataListItem" prop for PFDataList has been updated so that the "_event" parameter is the first parameter. "onSelectDataListItem" handlers may require an update.`, type: "JSXOpeningElement", }, ], 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 new file mode 100644 index 000000000..5b54a83f5 --- /dev/null +++ b/packages/eslint-plugin-pf-codemods/test/rules/v5/dataListCheck-updated-callback.js @@ -0,0 +1,75 @@ +const ruleTester = require("../../ruletester"); +const rule = require("../../../lib/rules/v5/dataListCheck-updated-callback"); + +ruleTester.run("dataListCheck-updated-callback", rule, { + valid: [ + { + code: `import { DataListCheck } from '@patternfly/react-core'; ;`, + }, + { + code: `import { DataListCheck } from '@patternfly/react-core'; onChange()} />;`, + }, + { + code: `import { DataListCheck } from '@patternfly/react-core'; const onChange = () => {}; ;`, + }, + { + code: `import { DataListCheck } from '@patternfly/react-core'; function onChange() {}; ;`, + }, + { + // No @patternfly/react-core import + code: `;`, + }, + ], + invalid: [ + { + code: `import { DataListCheck } from '@patternfly/react-core'; onChange()} />;`, + output: `import { DataListCheck } from '@patternfly/react-core'; onChange()} />;`, + errors: [ + { + message: `The "onChange" prop for DataListCheck has been updated so that the "_event" parameter is the first parameter. "onChange" handlers may require an update.`, + type: "JSXOpeningElement", + }, + ], + }, + { + code: `import { DataListCheck } from '@patternfly/react-core'; const onChange = (checked) => {}; ;`, + output: `import { DataListCheck } from '@patternfly/react-core'; const onChange = (_event, checked) => {}; ;`, + errors: [ + { + message: `The "onChange" prop for DataListCheck has been updated so that the "_event" parameter is the first parameter. "onChange" handlers may require an update.`, + type: "JSXOpeningElement", + }, + ], + }, + { + code: `import { DataListCheck } from '@patternfly/react-core'; function onChange (checked) {}; ;`, + output: `import { DataListCheck } from '@patternfly/react-core'; function onChange (_event, checked) {}; ;`, + errors: [ + { + message: `The "onChange" prop for DataListCheck has been updated so that the "_event" parameter is the first parameter. "onChange" handlers may require an update.`, + type: "JSXOpeningElement", + }, + ], + }, + { + code: `import { DataListCheck } from '@patternfly/react-core'; onChange()} />;`, + output: `import { DataListCheck } from '@patternfly/react-core'; onChange()} />;`, + errors: [ + { + message: `The "onChange" prop for DataListCheck has been updated so that the "_event" parameter is the first parameter. "onChange" handlers may require an update.`, + type: "JSXOpeningElement", + }, + ], + }, + { + code: `import { DataListCheck as PFdlc } from '@patternfly/react-core'; onChange()} />;`, + output: `import { DataListCheck as PFdlc } from '@patternfly/react-core'; onChange()} />;`, + errors: [ + { + message: `The "onChange" prop for PFdlc has been updated so that the "_event" parameter is the first parameter. "onChange" handlers may require an update.`, + type: "JSXOpeningElement", + }, + ], + }, + ], +}); diff --git a/packages/eslint-plugin-pf-codemods/test/rules/v5/onToggle-updated-paramaters.js b/packages/eslint-plugin-pf-codemods/test/rules/v5/onToggle-updated-paramaters.js index 9b829056b..05fad267e 100644 --- a/packages/eslint-plugin-pf-codemods/test/rules/v5/onToggle-updated-paramaters.js +++ b/packages/eslint-plugin-pf-codemods/test/rules/v5/onToggle-updated-paramaters.js @@ -35,7 +35,7 @@ ruleTester.run("onToggle-updated-paramaters", rule, { output: `import { ${component} } from '@patternfly/react-core'; <${component} onToggle={(_event, isOpen) => onToggle(isOpen)} />;`, errors: [ { - message: `The "onToggle" prop for ${component} has been updated to include the "_event" parameter as its first parameter. "onToggle" handlers may require an update.`, + message: `The "onToggle" prop for ${component} has been updated so that the "_event" parameter is the first parameter. "onToggle" handlers may require an update.`, type: "JSXOpeningElement", }, ], @@ -45,7 +45,7 @@ ruleTester.run("onToggle-updated-paramaters", rule, { output: `import { ${component} } from '@patternfly/react-core'; <${component} onToggle={(_event, isOpen) => onToggle(isOpen)} />;`, errors: [ { - message: `The "onToggle" prop for ${component} has been updated to include the "_event" parameter as its first parameter. "onToggle" handlers may require an update.`, + message: `The "onToggle" prop for ${component} has been updated so that the "_event" parameter is the first parameter. "onToggle" handlers may require an update.`, type: "JSXOpeningElement", }, ], @@ -55,7 +55,7 @@ ruleTester.run("onToggle-updated-paramaters", rule, { output: `import { ${component} } from '@patternfly/react-core'; const onToggle = (_event, isOpen) => {}; <${component} onToggle={onToggle} />;`, errors: [ { - message: `The "onToggle" prop for ${component} has been updated to include the "_event" parameter as its first parameter. "onToggle" handlers may require an update.`, + message: `The "onToggle" prop for ${component} has been updated so that the "_event" parameter is the first parameter. "onToggle" handlers may require an update.`, type: "JSXOpeningElement", }, ], @@ -65,7 +65,7 @@ ruleTester.run("onToggle-updated-paramaters", rule, { output: `import { ${component} } from '@patternfly/react-core'; function onToggle(_event, isOpen) {}; <${component} onToggle={onToggle} />;`, errors: [ { - message: `The "onToggle" prop for ${component} has been updated to include the "_event" parameter as its first parameter. "onToggle" handlers may require an update.`, + message: `The "onToggle" prop for ${component} has been updated so that the "_event" parameter is the first parameter. "onToggle" handlers may require an update.`, type: "JSXOpeningElement", }, ], @@ -75,7 +75,7 @@ ruleTester.run("onToggle-updated-paramaters", rule, { output: `import { ${component} } from '@patternfly/react-core'; <${component} onToggle={this.onToggle} />;`, errors: [ { - message: `The "onToggle" prop for ${component} has been updated to include the "_event" parameter as its first parameter. "onToggle" handlers may require an update.`, + message: `The "onToggle" prop for ${component} has been updated so that the "_event" parameter is the first parameter. "onToggle" handlers may require an update.`, type: "JSXOpeningElement", }, ], diff --git a/packages/pf-codemods/README.md b/packages/pf-codemods/README.md index 49181c800..e1a6086c8 100644 --- a/packages/pf-codemods/README.md +++ b/packages/pf-codemods/README.md @@ -359,6 +359,37 @@ function toggle2(_event, id) {}; ``` +### dataListCheck-warn-updated-callback [(#8735)](https://github.com/patternfly/patternfly-react/pull/8735) + +We've updated the `onChange` prop for DataListCheck so that the `event` parameter is the first parameter. Handlers may require an update. + +#### Examples + +In: + +```jsx + onChange(checked)} /> + +const onChange1 = (checked) => {}; + + +function onChange2(checked) {}; + +``` + +Out: + +```jsx + onChange(checked)} /> + +const onChange1 = (_event, checked) => {}; + + +function onChange2(_event, checked) {}; + +``` + + ### datePicker-warn-appendTo-default-value-changed [(#8636)](https://github.com/patternfly/patternfly-react/pull/8636) The default value of the `appendTo` prop on DatePicker has been updated, which may cause markup changes that require updating selectors in tests. This rule will raise a warning, but will not make any changes. diff --git a/test/test.tsx b/test/test.tsx index e20d07f1e..6b31cc5eb 100644 --- a/test/test.tsx +++ b/test/test.tsx @@ -15,6 +15,7 @@ import { Button, Card, DataList, + DataListCheck, DatePicker, DropdownItem, DropdownToggle, @@ -56,6 +57,7 @@ const newTheme = getCustomTheme("1", "2", "3");