From affc583c48778feea27ff068eaa7ae068e8920c3 Mon Sep 17 00:00:00 2001 From: Ahmed Abdelbaset Date: Mon, 5 Aug 2024 03:44:25 +0300 Subject: [PATCH] feat: Auto Fixing for ternary operator --- .changeset/great-ducks-smile.md | 5 + .github/workflows/release-canary.yml | 1 + e2e/no-physical-properties.tsx | 8 +- package.json | 5 +- pnpm-lock.yaml | 56 +++--- src/index.ts | 15 +- src/rules/no-phyisical-properties/rule.ts | 81 +++++---- src/rules/no-phyisical-properties/test.ts | 24 ++- src/utils/ast.ts | 210 +++++++++------------- 9 files changed, 192 insertions(+), 213 deletions(-) create mode 100644 .changeset/great-ducks-smile.md diff --git a/.changeset/great-ducks-smile.md b/.changeset/great-ducks-smile.md new file mode 100644 index 0000000..c8c3cd9 --- /dev/null +++ b/.changeset/great-ducks-smile.md @@ -0,0 +1,5 @@ +--- +"eslint-plugin-rtl-friendly": patch +--- + +Add auto-fixing for ternary operator diff --git a/.github/workflows/release-canary.yml b/.github/workflows/release-canary.yml index 19fbfdd..9d4f6f2 100644 --- a/.github/workflows/release-canary.yml +++ b/.github/workflows/release-canary.yml @@ -3,6 +3,7 @@ name: Release Canary on: pull_request: types: [labeled] + branches: ["*"] concurrency: ${{ github.workflow }}-${{ github.ref }} diff --git a/e2e/no-physical-properties.tsx b/e2e/no-physical-properties.tsx index df66af7..87b890d 100644 --- a/e2e/no-physical-properties.tsx +++ b/e2e/no-physical-properties.tsx @@ -478,7 +478,11 @@ declare const React; <> - {/* eslint-disable-next-line rtl-friendly/no-physical-properties */} -
0.5 ? "pl-1 text-right mr-2" : "pl-1 text-right mr-2"} /> +
0.5 ? "pl-1 text-right mr-2" : "pr-1 text-left ml-2" + } + /> ; diff --git a/package.json b/package.json index c978ccc..802378b 100644 --- a/package.json +++ b/package.json @@ -57,11 +57,10 @@ "@changesets/changelog-github": "^0.5.0", "@changesets/cli": "^2.27.7", "@eslint/js": "9.8.0", - "@types/eslint": "^9.6.0", - "@types/estree": "^1.0.5", - "@types/estree-jsx": "^1.0.5", "@types/jest": "^29.5.12", "@types/node": "20", + "@typescript-eslint/rule-tester": "^8.0.0", + "@typescript-eslint/utils": "^8.0.0", "eslint": "^9.8.0", "eslint-plugin-eslint-plugin": "^6.2.0", "globals": "^15.9.0", diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml index f581cd1..4550169 100644 --- a/pnpm-lock.yaml +++ b/pnpm-lock.yaml @@ -20,21 +20,18 @@ importers: '@eslint/js': specifier: 9.8.0 version: 9.8.0 - '@types/eslint': - specifier: ^9.6.0 - version: 9.6.0 - '@types/estree': - specifier: ^1.0.5 - version: 1.0.5 - '@types/estree-jsx': - specifier: ^1.0.5 - version: 1.0.5 '@types/jest': specifier: ^29.5.12 version: 29.5.12 '@types/node': specifier: '20' version: 20.14.14 + '@typescript-eslint/rule-tester': + specifier: ^8.0.0 + version: 8.0.0(@eslint/eslintrc@3.1.0)(eslint@9.8.0)(typescript@5.5.4) + '@typescript-eslint/utils': + specifier: ^8.0.0 + version: 8.0.0(eslint@9.8.0)(typescript@5.5.4) eslint: specifier: ^9.8.0 version: 9.8.0 @@ -778,12 +775,6 @@ packages: '@types/babel__traverse@7.20.6': resolution: {integrity: sha512-r1bzfrm0tomOI8g1SzvCaQHo6Lcv6zu0EA+W2kHrt8dyrHQxGzBBL4kdkzIS+jBMV+EYcMAEAqXqYaLJq5rOZg==} - '@types/eslint@9.6.0': - resolution: {integrity: sha512-gi6WQJ7cHRgZxtkQEoyHMppPjq9Kxo5Tjn2prSKDSmZrCz8TZ3jSRCeTJm+WoM+oB0WG37bRqLzaaU3q7JypGg==} - - '@types/estree-jsx@1.0.5': - resolution: {integrity: sha512-52CcUVNFyfb1A2ALocQw/Dd1BQFNmSdkuC3BkZ6iqhdMfQz7JWOFRuJFloOzjk+6WijU56m9oKXFAXc7o3Towg==} - '@types/estree@1.0.5': resolution: {integrity: sha512-/kYRxGDLWzHOB7q+wtSUQlFrtcdUccpfy+X+9iMBpHK8QLLhx2wIPYuS5DYtR9Wa/YlZAbIovy7qVdB1Aq6Lyw==} @@ -802,9 +793,6 @@ packages: '@types/jest@29.5.12': resolution: {integrity: sha512-eDC8bTvT/QhYdxJAulQikueigY5AsdBRH2yDKW3yveW7svY3+DzN84/2NUgkw10RTiJbWqZrTtoGVdYlvFJdLw==} - '@types/json-schema@7.0.15': - resolution: {integrity: sha512-5+fP8P8MFNC+AyZCDxrB2pkZFPGzqQWUzpSeuuVLvm8VMcorNYavBqoFcxK8bQz4Qsbn4oUEEem4wDLfcysGHA==} - '@types/node@12.20.55': resolution: {integrity: sha512-J8xLz7q2OFulZ2cyGTLE1TbbZcjpno7FaN6zdJNrgAdrJ+DZzh/uFR6YrTb4C+nXakvud8Q4+rbhoIWlYQbUFQ==} @@ -844,6 +832,13 @@ packages: typescript: optional: true + '@typescript-eslint/rule-tester@8.0.0': + resolution: {integrity: sha512-mYINoxt2DnRDl+X0Er134e6lxTrpb6enfKkea4RIjucd+YjsLzTSSkN40hiU4CB5kOjM17xJVm25TiZJLZMRMw==} + engines: {node: ^18.18.0 || ^20.9.0 || >=21.1.0} + peerDependencies: + '@eslint/eslintrc': '>=2' + eslint: ^8.57.0 || ^9.0.0 + '@typescript-eslint/scope-manager@8.0.0': resolution: {integrity: sha512-V0aa9Csx/ZWWv2IPgTfY7T4agYwJyILESu/PVqFtTFz9RIS823mAze+NbnBI8xiwdX3iqeQbcTYlvB04G9wyQw==} engines: {node: ^18.18.0 || ^20.9.0 || >=21.1.0} @@ -3366,15 +3361,6 @@ snapshots: dependencies: '@babel/types': 7.24.7 - '@types/eslint@9.6.0': - dependencies: - '@types/estree': 1.0.5 - '@types/json-schema': 7.0.15 - - '@types/estree-jsx@1.0.5': - dependencies: - '@types/estree': 1.0.5 - '@types/estree@1.0.5': {} '@types/graceful-fs@4.1.9': @@ -3396,8 +3382,6 @@ snapshots: expect: 29.7.0 pretty-format: 29.7.0 - '@types/json-schema@7.0.15': {} - '@types/node@12.20.55': {} '@types/node@20.14.14': @@ -3445,6 +3429,20 @@ snapshots: transitivePeerDependencies: - supports-color + '@typescript-eslint/rule-tester@8.0.0(@eslint/eslintrc@3.1.0)(eslint@9.8.0)(typescript@5.5.4)': + dependencies: + '@eslint/eslintrc': 3.1.0 + '@typescript-eslint/typescript-estree': 8.0.0(typescript@5.5.4) + '@typescript-eslint/utils': 8.0.0(eslint@9.8.0)(typescript@5.5.4) + ajv: 6.12.6 + eslint: 9.8.0 + json-stable-stringify-without-jsonify: 1.0.1 + lodash.merge: 4.6.2 + semver: 7.6.3 + transitivePeerDependencies: + - supports-color + - typescript + '@typescript-eslint/scope-manager@8.0.0': dependencies: '@typescript-eslint/types': 8.0.0 diff --git a/src/index.ts b/src/index.ts index 5cb3894..fa3f772 100644 --- a/src/index.ts +++ b/src/index.ts @@ -1,17 +1,16 @@ -import type { ESLint, Linter } from "eslint"; -import pkg from "../package.json"; +import type { FlatConfig } from "@typescript-eslint/utils/ts-eslint"; import { noPhysicalProperties } from "./rules/no-phyisical-properties/rule"; const rtlFriendly = { meta: { - name: pkg.name, - version: pkg.version, + name: "eslint-plugin-rtl-friendly", + // version: pkg.version, }, - configs: {} as { recommended: Linter.FlatConfig }, + configs: {} as { recommended: FlatConfig.Config }, rules: { "no-physical-properties": noPhysicalProperties, }, -} satisfies ESLint.Plugin; +}; const configs = { recommended: { @@ -29,8 +28,8 @@ const configs = { }, }, }, - }, -} satisfies ESLint.Plugin["configs"]; + } satisfies FlatConfig.Config, +}; Object.assign(rtlFriendly.configs, configs); diff --git a/src/rules/no-phyisical-properties/rule.ts b/src/rules/no-phyisical-properties/rule.ts index 9f00f1a..0b6684f 100644 --- a/src/rules/no-phyisical-properties/rule.ts +++ b/src/rules/no-phyisical-properties/rule.ts @@ -1,8 +1,9 @@ -import { Rule, type AST } from "eslint"; - -import * as ESTree from "estree"; -import type { JSXAttribute } from "estree-jsx"; -import { extractTokenFromNode } from "../../utils/ast.js"; +import type { TSESTree } from "@typescript-eslint/utils"; +import type { + RuleContext, + RuleModule, +} from "@typescript-eslint/utils/ts-eslint"; +import { type Token, extractTokenFromNode } from "../../utils/ast.js"; import { parseForPhysicalClasses } from "../../utils/tailwind.js"; // const cache = new Map< @@ -10,26 +11,33 @@ import { parseForPhysicalClasses } from "../../utils/tailwind.js"; // /** valid */ string // >(); +// const createRule = ESLintUtils.RuleCreator((ruleName) => { +// return `https://github.com/AhmedBaset/eslint-plugin-rtl-friendly/blob/main/src/rules/${ruleName}/README.md`; +// }); + +// const RULE_NAME = "no-physical-properties"; export const NO_PHYSICAL_CLASSESS = "NO_PHYSICAL_CLASSESS"; +type NO_PHYSICAL_CLASSESS = typeof NO_PHYSICAL_CLASSESS; -export const noPhysicalProperties: Rule.RuleModule = { +export const noPhysicalProperties: RuleModule = { + // name: RULE_NAME, + defaultOptions: [], meta: { type: "suggestion", docs: { description: "Encourage the use of RTL-friendly styles", - recommended: true, + url: "https://github.com/AhmedBaset/eslint-plugin-rtl-friendly/blob/main/src/rules/no-physical-properties/README.md", }, fixable: "code", messages: { + // eslint-disable-next-line eslint-plugin/no-unused-message-ids [NO_PHYSICAL_CLASSESS]: `Avoid using physical properties such as "{{ invalid }}". Instead, use logical properties like "{{ valid }}" for better RTL support.`, }, schema: [], }, - create(ctx) { + create: (ctx) => { return { - JSXAttribute: (estreeNode: ESTree.Node) => { - const node = estreeNode as JSXAttribute; - + JSXAttribute: (node) => { if (node.name.type !== "JSXIdentifier") return; const attr = node.name.name; @@ -50,58 +58,59 @@ export const noPhysicalProperties: Rule.RuleModule = { // return; // } - const classesAsString = extractTokenFromNode(node, "checker")?.value; - if (!classesAsString) return; - - const classes = classesAsString.split(" "); + const tokens = extractTokenFromNode(node, "checker"); + tokens?.forEach((token) => { + const classValue = token?.getValue(); + if (!classValue) return; - const parsed = parseForPhysicalClasses(classes); + const classes = classValue.split(" "); - const isInvalid = parsed.some((p) => p.isInvalid); - if (!isInvalid) return; + const parsed = parseForPhysicalClasses(classes); - const invalid = parsed.map((p) => p.original).join(" "); - const valid = parsed.map((p) => p.valid).join(" "); + const isInvalid = parsed.some((p) => p.isInvalid); + if (!isInvalid) return; - // cache.set(classesAsString, valid); - report({ ctx, node, invalid, valid }); + const invalid = parsed.map((p) => p.original).join(" "); + const valid = parsed.map((p) => p.valid).join(" "); + + // cache.set(classesAsString, valid); + report({ ctx, node, invalid, valid, token: token ?? null }); + }); }, }; }, }; +type Context = Readonly>; + function report({ ctx, invalid, valid, node, + token, }: { - ctx: Rule.RuleContext; - node: JSXAttribute; + ctx: Context; + node: TSESTree.JSXAttribute; invalid: string; valid: string; + token: Token | null; }) { return ctx.report({ node, - messageId: "NO_PHYSICAL_CLASSESS", + messageId: NO_PHYSICAL_CLASSESS, data: { invalid, valid, }, loc: { - start: node.loc!.start, - end: node.loc!.end, + start: token?.loc?.start ?? node.loc!.start, + end: token?.loc?.end ?? node.loc!.end, }, fix: (fixer) => { - const token = extractTokenFromNode(node, "fixer"); - if (token?.raw) { - return fixer.replaceText( - token as AST.Token, - token.raw?.replace(invalid, valid) - ); - } - - return null; + if (!token) return null; + + return fixer.replaceText(token, token?.getRaw()?.replace(invalid, valid)); }, }); } diff --git a/src/rules/no-phyisical-properties/test.ts b/src/rules/no-phyisical-properties/test.ts index b5c6446..b40a3b4 100644 --- a/src/rules/no-phyisical-properties/test.ts +++ b/src/rules/no-phyisical-properties/test.ts @@ -1,5 +1,5 @@ -import { RuleTester } from "eslint"; -import { noPhysicalProperties, NO_PHYSICAL_CLASSESS } from "./rule"; +import { RuleTester } from "@typescript-eslint/rule-tester"; +import { NO_PHYSICAL_CLASSESS, noPhysicalProperties } from "./rule"; const tester = new RuleTester({ languageOptions: { @@ -106,9 +106,23 @@ tester.run("no-physical-properties", noPhysicalProperties, { }, { name: '{isCondition ? "..." : "..."}', - code: `
`, - // output: `
`, // TODO: Fix this - errors: [{ messageId: NO_PHYSICAL_CLASSESS }], + code: `
`, + output: + '
', + errors: [ + { messageId: NO_PHYSICAL_CLASSESS }, + { messageId: NO_PHYSICAL_CLASSESS }, + ], + }, + { + name: "{isCondition ? `...` : `...`}", + code: "
", + output: + "
", + errors: [ + { messageId: NO_PHYSICAL_CLASSESS }, + { messageId: NO_PHYSICAL_CLASSESS }, + ], }, { name: "should report if physical margin properties are used and fix them", diff --git a/src/utils/ast.ts b/src/utils/ast.ts index 937f099..e3c0859 100644 --- a/src/utils/ast.ts +++ b/src/utils/ast.ts @@ -1,110 +1,46 @@ -import type { Expression, JSXAttribute } from "estree-jsx"; +import type { TSESTree } from "@typescript-eslint/utils"; -type Val = string | number | bigint | boolean | RegExp | null | undefined; - -export function extractFromNode(node: JSXAttribute) { - // const token = extractTokenFromNode(node) - // if (token) { - // return token.raw; - // } - - // value: Literal | JSXExpressionContainer | JSXElement | JSXFragment | null - const valueType = node.value?.type; - if (!valueType) return; - - const result: Val[] = []; - - // 1. Literal className="..." - if (valueType === "Literal") result.push(node.value?.value); - // 2. JSXExpressionContainer className={...} - else if (valueType === "JSXExpressionContainer") { - const expression = node.value?.expression; - - if (expression?.type === "JSXEmptyExpression" || !expression) return; - - if (expression.type === "Literal") return [expression.value]; - if (expression.type === "TemplateLiteral") - result.push(expression.quasis[0].value.raw); - - result.push(...extractFromExpression(expression)); - } - - // Exit if JSXElement | JSXFragment | null - if ( - valueType === "JSXElement" || - valueType === "JSXFragment" || - !node.value - ) { - return; - } - - return result; -} - -function extractFromExpression(expression: Expression) { - // We care about: - // -> Literal; - // -> TemplateLiteral; - // -> BinaryExpression - // -> CallExpression; - // -> ConditionalExpression; - // -> LogicalExpression; - - const result: Val[] = []; - - if (expression.type === "Literal") result.push(expression.value); - if (expression.type === "TemplateLiteral") - result.push(expression.quasis[0].value.raw); - if (expression.type === "ConditionalExpression") { - console.log(expression); - // result.push(extractFromExpression(expression.left)); - // result.push(extractFromExpression(expression.right)); - } - if (expression.type === "CallExpression") { - expression.arguments.forEach((arg) => { - if (arg.type === "SpreadElement") { - result.push(...extractFromExpression(arg.argument)); - } else { - result.push(...extractFromExpression(arg)); - } - }); - } - if (expression.type === "ConditionalExpression") { - result.push(...extractFromExpression(expression.consequent)); - result.push(...extractFromExpression(expression.alternate)); - } - if (expression.type === "LogicalExpression") { - result.push(...extractFromExpression(expression.right)); - } - - return result; -} +export type Token = ( + | TSESTree.JSXAttribute + | TSESTree.Expression + | TSESTree.TemplateElement +) & { + getValue: () => string; + getRaw: () => string; +}; export function extractTokenFromNode( - node: JSXAttribute, + node: TSESTree.JSXAttribute, runner: "checker" | "fixer" -): { type: string; value?: string; raw?: string } | undefined { +): (Token | undefined | null)[] { // value: Literal | JSXExpressionContainer | JSXElement | JSXFragment | null const type = node.value?.type; - if (!type) return; + if (!type) return []; - if (type === "Literal") return validate(node.value); + const nodeValue = node.value; + + if (isStringLiteral(nodeValue)) + return format( + nodeValue, + (n) => n.value, + (n) => n.raw + ); if (type === "JSXExpressionContainer") { const expression = node.value?.expression; - if (expression?.type === "JSXEmptyExpression" || !expression) return; + if (!expression || expression?.type === "JSXEmptyExpression") return []; return extractTokenFromExpression(expression, runner); } - return; + return []; } function extractTokenFromExpression( - expression: Expression, + exp: TSESTree.Expression, runner: "checker" | "fixer" -): { type: string; value: string; raw: string } | undefined { +): (Token | undefined)[] { // We care about: // -> Literal; // -> TemplateLiteral; @@ -113,43 +49,39 @@ function extractTokenFromExpression( // -> ConditionalExpression; // -> LogicalExpression; - const rerun = (expression: Expression) => { + const rerun = (expression: TSESTree.Expression) => { return extractTokenFromExpression(expression, runner); }; - const isFixer = runner === "fixer"; - const type = expression.type; - - if (type === "Literal") - return validate({ - ...expression, - value: expression.value || expression.raw, - }); - if (type === "TemplateLiteral") { - return validate({ - ...expression.quasis[0], - value: expression.quasis[0].value.cooked, - raw: "`" + expression.quasis[0].value.raw + "`", - }); + // const isFixer = runner === "fixer"; + const type = exp.type; + + if (isStringLiteral(exp)) + return format( + exp, + () => exp.value, + () => exp.raw + ); + + if (exp?.type === "TemplateLiteral") { + return format( + exp.quasis, + (q) => q.value.cooked, + (q) => `\`${q.value.raw}\`` + ); } - if (type === "LogicalExpression") { - return rerun(expression.right); + if (exp.type === "LogicalExpression") { + // isCondition && "..." + return rerun(exp.right); } - if (type === "ConditionalExpression") { - // TODO: Currently, Auto Fixer works on the consequent only - if (isFixer) { - return rerun(expression.consequent); - } else { - const consequent = rerun(expression.consequent); - const alternate = rerun(expression.alternate); - consequent!.value = `${consequent!.value}" : "${alternate!.value}`; - return consequent; - } + if (exp.type === "ConditionalExpression") { + return [...rerun(exp.consequent), ...rerun(exp.alternate)]; + } - console.log("UNIMPLEMENTED: ", type); + // console.log("UNIMPLEMENTED: ", type); // if (expression.type === "BinaryExpression") { // result.push(...extractFromExpression(expression.left)); @@ -172,20 +104,38 @@ function extractTokenFromExpression( // result.push(...extractFromExpression(expression.right)); // } - return; + return []; +} + +function format< + T extends + | TSESTree.JSXAttribute + | TSESTree.Expression + | TSESTree.TemplateElement, +>( + nodeOrToken: T | T[], + getValue: (t: T) => string, + getRaw: (t: T) => string +): (T & { getValue: () => string; getRaw: () => string })[] { + if (Array.isArray(nodeOrToken)) { + return nodeOrToken.map((t) => ({ + ...t, + getValue: () => getValue(t), + getRaw: getRaw ? () => getRaw(t) : () => getValue(t), + })); + } + + return [ + { + ...nodeOrToken, + getValue: () => getValue(nodeOrToken), + getRaw: () => (getRaw ?? getValue)(nodeOrToken), + }, + ] as const; } -// eslint-disable-next-line @typescript-eslint/no-explicit-any -function validate(result: null | { type: string; value?: any; raw?: string }): - | undefined - | { - type: string; - value: string; - raw: string; - } { - if (!result) return; - if (typeof result.value !== "string") return; - if (typeof result.raw !== "string") return; - - return result as { type: string; value: string; raw: string }; +function isStringLiteral( + value: TSESTree.JSXAttribute["value"] | TSESTree.Expression +): value is TSESTree.StringLiteral { + return value?.type === "Literal" && typeof value?.value === "string"; }