Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Helper documentation/restructure POC #811

Draft
wants to merge 1 commit into
base: main
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,12 @@ import {
MemberExpression,
} from "estree-jsx";

/**
* Use inline to pair with other logic, such as to get the value of an attribute or call logic based on the attribute being present or not.
* @param node The node in which to find a specified attribute.
* @param attributeName An attribute name to find on the node.
* @returns The found JSXAttribute object, or undefined if the attribute is not found.
*/
export function getAttribute(
node: JSXElement | JSXOpeningElement,
attributeName: string
Expand All @@ -19,6 +25,12 @@ export function getAttribute(
) as JSXAttribute | undefined;
}

/**
* Use inline to pair with other logic based on whether any of the specified attributes are present, but not necessarily important which ones are.
* @param node The node in which to find a specified attribute.
* @param attributeNames An array of attribute names to find on the node.
* @returns The first found JSXAttribute object, or undefined if none are found.
*/
export function getAnyAttribute(
node: JSXElement | JSXOpeningElement,
attributeNames: string[]
Expand All @@ -35,6 +47,25 @@ export function getAnyAttribute(
return foundAttribute;
}

/**
* Use inline to pair with other logic (such as getting attribute values or calling logic based on an attribute being present or not) and when you need to get several attributes at once.
* @param node - The node in which to find the specified attributes.
* @param attributeNames - An array of attribute names to find on the node.
* @returns An object containing key:value pairs of the attribute names and either their found JSXAttribute object or undefined.
*/
export function getSpecifiedAttributes(
node: JSXElement | JSXOpeningElement,
attributeNames: string[]
) {
const foundAttributes: { [attributeName: string]: JSXAttribute | undefined } =
{};
for (const attribute of attributeNames) {
foundAttributes[attribute] = getAttribute(node, attribute);
}

return foundAttributes;
}

export function getAttributeValue(
context: Rule.RuleContext,
node?: JSXAttribute["value"]
Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,10 @@
import { JSXElement } from "estree-jsx";

/** Checks whether children is empty (no children or only whitespaces) */
/**
* Use inline to pair with logic that depends on whether a JSXElement has children or not.
* @param children The children property of the JSX element node object.
* @returns A boolean depending on whether the node has valid children or not.
*/
export function childrenIsEmpty(children: JSXElement["children"]) {
return (
!children.length ||
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
import { Rule } from "eslint";
import { JSXElement } from "estree-jsx";
import { removeElement, removeEmptyLineAfter } from "../index";

/**
* Use inline within the context.report's fix method to remove entire JSX elements. This can be returned on its own or spread as part of a fixes array.
* @param context The context object from the top level create method of the rule being written.
* @param fixer The fixer parameter from the fix method of the context.report object.
* @param elementsToRemove An array of JSXElement objects to remove.
* @returns An array of fixers to apply when the rule is ran with the fix flag.
*/
export const getRemoveElementFixes = (
context: Rule.RuleContext,
fixer: Rule.RuleFixer,
elementsToRemove: JSXElement[]
) => {
return elementsToRemove
.map((element) => [
...removeElement(fixer, element),
...removeEmptyLineAfter(context, fixer, element),
])
.flat();
};
Original file line number Diff line number Diff line change
@@ -1,19 +1,25 @@
import { Rule } from "eslint";
import {
JSXElement,
ImportDeclaration,
ExportNamedDeclaration,
ImportSpecifier,
ExportSpecifier,
ImportDefaultSpecifier,
ImportNamespaceSpecifier,
} from "estree-jsx";
import { getEndRange } from "./getEndRange";
import { removeElement, removeEmptyLineAfter } from "./index";
import { getEndRange } from "../getEndRange";

export function removeSpecifierFromDeclaration(
fixer: Rule.RuleFixer,
/**
* Use inline within the context.report's fix method to remove a specifier from an import/export declaration. This can be returned on its own or spread as part of a fixes array.
* @param context The context object from the top level create method of the rule being written.
* @param fixer The fixer parameter from the fix method of the context.report object.
* @param node The import or export node to remove the specifier from.
* @param specifierToRemove The specifier to remove from the import or export declaration.
* @returns An array of fixers to apply when the rule is ran with the fix flag.
*/
export function getRemoveSpecifierFixes(
context: Rule.RuleContext,
fixer: Rule.RuleFixer,
node: ImportDeclaration | ExportNamedDeclaration,
specifierToRemove:
| ImportSpecifier
Expand All @@ -35,16 +41,3 @@ export function removeSpecifierFromDeclaration(
}
return [fixer.removeRange([startRange, endRange])];
}

export const getRemoveElementFixes = (
context: Rule.RuleContext,
fixer: Rule.RuleFixer,
elementsToRemove: JSXElement[]
) => {
return elementsToRemove
.map((element) => [
...removeElement(fixer, element),
...removeEmptyLineAfter(context, fixer, element),
])
.flat();
};
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
export * from "./getRemoveSpecifierFixes";
export * from "./getRemoveElementFixes";
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
import { ImportSpecifier } from "estree-jsx";
import { Rule, SourceCode } from "eslint";
import { ImportDefaultSpecifierWithParent } from "./interfaces";

/**
* Use inline to check whether an import specifier has a data-codemod comment, and conditionally run logic based on its existence or absence.
* @param context The context object from the ancestor create method.
* @param importSpecifier The specifier in which to find whether the data-codemod comment exists.
* @param commentToFind The string to check whether is included in a comment. By default this is "data-codemods", but can be customized for more fine-tuned logic.
* @returns The first Comment object if the data-codemod string was found, or undefined otherwise.
*/
export function getCodeModDataComment(
sourceCode: SourceCode,
importSpecifier: ImportSpecifier | ImportDefaultSpecifierWithParent,
commentToFind: string = "data-codemods"
) {
const { leading, trailing } = sourceCode.getComments(importSpecifier);

return [...leading, ...trailing].find((comment) =>
comment.value.includes(commentToFind)
);
}
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,11 @@ import {
ImportSpecifierWithParent,
} from "./interfaces";

/** Gets the import path string for an import specifier (named or default) */
/**
* Use inline to pair with other logic that depends on the import path of a specifier.
* @param importSpecifier The import specifier object to get the path of.
* @returns The import path string if found, otherwise undefined.
*/
export function getImportPath(
importSpecifier: ImportDefaultSpecifierWithParent | ImportSpecifierWithParent
) {
Expand Down
5 changes: 3 additions & 2 deletions packages/eslint-plugin-pf-codemods/src/rules/helpers/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ export * from "./findAncestor";
export * from "./fixers";
export * from "./getAttributeName";
export * from "./getChildJSXElementByName";
export * from "./getCodeModDataComment";
export * from "./getCodeModDataTag";
export * from "./getComponentImportName";
export * from "./getEndRange";
Expand All @@ -25,10 +26,10 @@ export * from "./interfaces";
export * from "./isReactIcon";
export * from "./JSXAttributes";
export * from "./makeJSXElementSelfClosing";
export * from "./nodeMatches/checkMatchingImportDeclaration";
export * from "./nodeMatches/checkMatchingJSXOpeningElement";
export * from "./nodeMatches";
export * from "./pfPackageMatches";
export * from "./removeElement";
export * from "./removeEmptyLineAfter";
export * from "./removePropertiesFromObjectExpression";
export * from "./renameProps";
export * from "./testHelpers";
Original file line number Diff line number Diff line change
@@ -1,21 +1,34 @@
import { ImportDeclaration, ImportSpecifier } from "estree-jsx";
import { pfPackageMatches } from "../pfPackageMatches";
import { ImportDefaultSpecifierWithParent } from "../interfaces";

function checkSpecifierExists(
node: ImportDeclaration,
importSpecifier: ImportSpecifier
importSpecifier: ImportSpecifier | ImportDefaultSpecifierWithParent
) {
return node.specifiers.some(
(specifier) =>
specifier.type === "ImportSpecifier" &&
specifier.imported.name === importSpecifier.imported.name
(specifier.type === "ImportSpecifier" &&
specifier.imported.name ===
(importSpecifier as ImportSpecifier).imported.name) ||
(specifier.type === "ImportDefaultSpecifier" &&
specifier.local.name === importSpecifier.local.name)
);
}

/** Used to check whether the current ImportDeclaration node matches at least 1 of the import specifiers. */
/**
* Use inline at the start of an ImportDeclaration block to early return if no specifiers are found, or only run logic if at least 1 specifier is found.
* @param node - The node to check for any import specifiers.
* @param imports - A single import specifier or an array of specifiers to find within the import declaration.
* @param packageName - The package name to check against the import declaration source.
* @returns A boolean depending on whether the import declaration source matches the packageName, or whether at least 1 import specifier is found.
*/
export function checkMatchingImportDeclaration(
node: ImportDeclaration,
imports: ImportSpecifier | ImportSpecifier[],
imports:
| ImportSpecifier
| ImportDefaultSpecifierWithParent
| (ImportSpecifier | ImportDefaultSpecifierWithParent)[],
packageName: string = "@patternfly/react-core"
) {
if (!pfPackageMatches(packageName, node.source.value)) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,12 @@ import {
ImportDefaultSpecifier,
} from "estree-jsx";

/** Used to check whether the current JSXOpeningElement node matches at least 1 of the import specifiers. */
/**
* Use inline at the start of a JSXOpeningElement block to early return if no specifiers match the node name, or only run logic if at least 1 specifier matches the node name.
* @param node - The node to check for any import specifiers.
* @param imports - A single import specifier or an array of specifiers to check against the opening element node name.
* @returns A boolean depending on whether at least 1 import specifier matches the opening element node name.
*/
export function checkMatchingJSXOpeningElement(
node: JSXOpeningElement,
imports:
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
export * from "./checkMatchingImportDeclaration";
export * from "./checkMatchingJSXOpeningElement";

This file was deleted.

Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
import { TestErrors } from "./testingTypes";

/**
* Use inline in a test file to push a test object to an invalid test array.
* @param code The invalid code snippet that should trigger an error.
* @param output The code snippet that should be output upon the fixer being ran. If no fixer exists, this shoujld be the same as the code parameter.
* @param errors An array of error objects, each consisting of the error message and the node type that should trigger the error.
* @returns An invalid test object.
*/
export function createInvalidTest(
code: string,
output: string,
errors: TestErrors
) {
return {
code,
output,
errors,
};
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
/**
* Use inline in a test file to push a test object to a valid test array.
* @param code The valid code snippet.
* @returns A valid test object.
*/
export function createValidTest(code: string) {
return {
code,
};
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
export * from "./testingTypes";
export * from "./createValidTest";
export * from "./createInvalidTest";
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
import { RuleTester } from "eslint";

export type ValidTests = Array<string | RuleTester.ValidTestCase>;
export type InvalidTests = RuleTester.InvalidTestCase[];
export type TestErrors = {
message: string;
type: string;
}[];
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ import {
IdentifierWithParent,
getAttribute,
getAttributeValue,
removeSpecifierFromDeclaration,
getRemoveSpecifierFixes,
getAllImportDeclarations,
} from "../../helpers";

Expand Down Expand Up @@ -72,9 +72,9 @@ module.exports = {
}

fixes.push(
...removeSpecifierFromDeclaration(
fixer,
...getRemoveSpecifierFixes(
context,
fixer,
node,
kebabImportSpecifier
)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ import {
getFromPackage,
getObjectProperty,
ImportSpecifierWithParent,
removeSpecifierFromDeclaration,
getRemoveSpecifierFixes,
} from "../../helpers";

// https://github.com/patternfly/patternfly-react/pull/11096
Expand Down Expand Up @@ -123,9 +123,9 @@ module.exports = {
node,
message: interfaceRemovedMessage,
fix(fixer) {
return removeSpecifierFromDeclaration(
fixer,
return getRemoveSpecifierFixes(
context,
fixer,
(node as ImportSpecifierWithParent).parent!,
node
);
Expand All @@ -142,9 +142,9 @@ module.exports = {
node,
message: interfaceRemovedMessage,
fix(fixer) {
return removeSpecifierFromDeclaration(
fixer,
return getRemoveSpecifierFixes(
context,
fixer,
node,
specifierToRemove
);
Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import { Rule } from "eslint";
import { ImportDeclaration, ImportSpecifier } from "estree-jsx";
import { getFromPackage, removeSpecifierFromDeclaration } from "../../helpers";
import { getFromPackage, getRemoveSpecifierFixes } from "../../helpers";

// Cleanup from other rules
module.exports = {
Expand Down Expand Up @@ -34,9 +34,9 @@ module.exports = {
node,
message: `Duplicate import specifier ${node.local.name} imported from '${importDeclaration.source.value}'.`,
fix(fixer) {
return removeSpecifierFromDeclaration(
fixer,
return getRemoveSpecifierFixes(
context,
fixer,
importDeclaration,
node
);
Expand Down
Loading