From 3f8ff5ffb0a07857c1ed9f7d592e62b27b726def Mon Sep 17 00:00:00 2001 From: Ahmed Abdelbaset Date: Sat, 28 Sep 2024 02:10:29 +0300 Subject: [PATCH 1/2] Option to allow physical inset with absolute/fixed center --- eslint.config.js | 8 +- src/flags.ts | 7 ++ src/index.test.ts | 19 ++-- src/index.ts | 7 +- src/rules/no-phyisical-properties/ast.ts | 29 ++++-- src/rules/no-phyisical-properties/rule.ts | 54 ++++++++--- src/rules/no-phyisical-properties/tailwind.ts | 51 ++++++++-- src/rules/no-phyisical-properties/test.ts | 61 ++++++++++++ src/utils/eslint.ts | 94 +++++++++---------- vite.config.ts | 1 + 10 files changed, 246 insertions(+), 85 deletions(-) create mode 100644 src/flags.ts diff --git a/eslint.config.js b/eslint.config.js index 91e69cd..0324ffe 100644 --- a/eslint.config.js +++ b/eslint.config.js @@ -4,7 +4,7 @@ import js from "@eslint/js"; import eslintPlugin from "eslint-plugin-eslint-plugin"; import { config, configs } from "typescript-eslint"; -import rtlFriendly from "./dist/index.js"; +import rtlFriendly, { ruleSettings } from "./dist/index.js"; export default config( { @@ -25,5 +25,11 @@ export default config( { files: ["**/*.{tsx,jsx}"], ...rtlFriendly.configs.recommended, + rules: { + "rtl-friendly/no-physical-properties": [ + "warn", + ruleSettings({ allowPhysicalInsetWithAbsolute: true }), + ], + }, } ); diff --git a/src/flags.ts b/src/flags.ts new file mode 100644 index 0000000..bf10cb5 --- /dev/null +++ b/src/flags.ts @@ -0,0 +1,7 @@ +/** + * Act as default options for rules + */ +export const FLAGS = { + allowPhysicalInsetWithAbsolute: false, + debug: false, +}; diff --git a/src/index.test.ts b/src/index.test.ts index 3c04bc4..eac4a1b 100644 --- a/src/index.test.ts +++ b/src/index.test.ts @@ -1,8 +1,7 @@ import { describe, it, expect, expectTypeOf } from "vitest"; -import rtlFriendly from "./index.js"; +import rtlFriendly, { ruleSettings } from "./index.js"; import { name, version } from "../package.json"; -import type { RuleModule } from "@typescript-eslint/utils/ts-eslint"; -import type { MessageId } from "./rules/no-phyisical-properties/rule.js"; +import type { Rule } from "./rules/no-phyisical-properties/rule.js"; // useless tests generated by copilot :) @@ -14,9 +13,9 @@ describe("rtlFriendly", () => { it("should have the no-physical-properties rule", () => { expect(rtlFriendly.rules).toHaveProperty("no-physical-properties"); - expectTypeOf(rtlFriendly.rules["no-physical-properties"]).toMatchTypeOf< - RuleModule - >(); + expectTypeOf( + rtlFriendly.rules["no-physical-properties"] + ).toMatchTypeOf(); }); describe("configs", () => { @@ -46,3 +45,11 @@ describe("rtlFriendly", () => { }); }); }); + +describe("ruleSettings", () => { + it("should return the options", () => { + expect(ruleSettings({ allowPhysicalInsetWithAbsolute: true })).toEqual({ + allowPhysicalInsetWithAbsolute: true, + }); + }); +}); diff --git a/src/index.ts b/src/index.ts index ba230e9..d20623a 100644 --- a/src/index.ts +++ b/src/index.ts @@ -1,6 +1,9 @@ import type { FlatConfig } from "@typescript-eslint/utils/ts-eslint"; import { name, version } from "../package.json"; -import { noPhysicalProperties } from "./rules/no-phyisical-properties/rule.js"; +import { + noPhysicalProperties, + ruleSettings, +} from "./rules/no-phyisical-properties/rule.js"; const rtlFriendly = { meta: { name, version }, @@ -32,3 +35,5 @@ const configs = { Object.assign(rtlFriendly.configs, configs); export default rtlFriendly; + +export { ruleSettings, rtlFriendly }; diff --git a/src/rules/no-phyisical-properties/ast.ts b/src/rules/no-phyisical-properties/ast.ts index d5d66bf..35bde31 100644 --- a/src/rules/no-phyisical-properties/ast.ts +++ b/src/rules/no-phyisical-properties/ast.ts @@ -23,7 +23,7 @@ export function extractTokensFromNode( // node: TSESTree.JSXAttribute, node: TSESTree.Node, ctx: Context, - runner: "checker" | "fixer" + { debug }: { debug: boolean } ): Token[] { if (node.type === "JSXAttribute") { // value: Literal | JSXExpressionContainer | JSXElement | JSXFragment | null @@ -40,7 +40,7 @@ export function extractTokensFromNode( if (!expression || expression?.type === "JSXEmptyExpression") return []; - return extractTokensFromExpression(expression, ctx, runner); + return extractTokensFromExpression(expression, ctx, { debug }); } // if (value.type === "JSXElement" || value.type === "JSXSpreadChild") { @@ -66,12 +66,15 @@ type Exp = TSESTree.Expression | TSESTree.TemplateElement; function extractTokensFromExpression( exp: Exp, ctx: Context, - runner: "checker" | "fixer", - { isIdentifier = false }: { isIdentifier?: boolean } = {} + { + isIdentifier = false, + debug, + }: { isIdentifier?: boolean; debug?: boolean } = {} ): Token[] { const rerun = (expression: Exp, referenceIsIdentifier?: boolean) => { - return extractTokensFromExpression(expression, ctx, runner, { + return extractTokensFromExpression(expression, ctx, { isIdentifier: referenceIsIdentifier || isIdentifier, + debug, }); }; @@ -175,9 +178,10 @@ function extractTokensFromExpression( return writes.flatMap((n) => rerun(n, true)); } - // if (is(exp, "MemberExpression") && is(exp.property, "Identifier")) { - // return []; - // } + if (is(exp, "MemberExpression")) { + // Unsupported + return []; + } /* if (is(exp, "ArrowFunctionExpression")) { @@ -225,7 +229,14 @@ function extractTokensFromExpression( // } if (!unimplemented.has(exp.type)) { - console.log("Unimplemented: ", exp.type, exp); + if (debug) { + console.log( + "rtl-friendly plugin detected that you are writing your writing your tailwind classes in a way that is not yet supported by this plugin.\n" + + "Kindly open an issue on GitHub so we can add support for this case. Thanks!\n" + + `https://github.com/AhmedBaset/eslint-plugin-rtl-friendly/issues/new?title=Unimplemented+Node%3A+%60${exp.type}%60\n`, + "You can disable this warning by setteng the `debug` option to `false` the rule options." + ); + } unimplemented.add(exp.type); } diff --git a/src/rules/no-phyisical-properties/rule.ts b/src/rules/no-phyisical-properties/rule.ts index 5012e4a..cb5ce03 100644 --- a/src/rules/no-phyisical-properties/rule.ts +++ b/src/rules/no-phyisical-properties/rule.ts @@ -5,6 +5,7 @@ import type { } from "@typescript-eslint/utils/ts-eslint"; import { type Token, extractTokensFromNode } from "./ast.js"; import { parseForPhysicalClasses } from "./tailwind.js"; +import { FLAGS } from "../../flags.js"; // const cache = new Map< // /** invalid */ string, @@ -15,15 +16,20 @@ import { parseForPhysicalClasses } from "./tailwind.js"; // return `https://github.com/AhmedBaset/eslint-plugin-rtl-friendly/blob/main/src/rules/${ruleName}/README.md`; // }); +// Since the rule is no longer for physical properties specifically, +// we consider renaming it e.g. `rtl-friendly/tailwind` export const RULE_NAME = "no-physical-properties"; export const NO_PHYSICAL_CLASSESS = "NO_PHYSICAL_CLASSESS"; export const IDENTIFIER_USED = "IDENTIFIER_USED"; export type MessageId = "NO_PHYSICAL_CLASSESS" | "IDENTIFIER_USED"; +export type Rule = RuleModule< + MessageId, + [{ allowPhysicalInsetWithAbsolute?: boolean; debug?: boolean }] +>; -export const noPhysicalProperties: RuleModule = { +export const noPhysicalProperties: Rule = { // name: RULE_NAME, - defaultOptions: [], meta: { type: "suggestion", docs: { @@ -35,8 +41,24 @@ export const noPhysicalProperties: RuleModule = { [NO_PHYSICAL_CLASSESS]: `Avoid using physical properties such as "{{ invalid }}". Instead, use logical properties like "{{ valid }}" for better RTL support.`, [IDENTIFIER_USED]: `This text is used later as a class name but contains physical properties such as "{{ invalid }}". It's better to use logical properties like "{{ valid }}" for improved RTL support.`, }, - schema: [], + schema: [ + { + type: "object", + properties: { + allowPhysicalInsetWithAbsolute: { + type: "boolean", + default: false, + }, + debug: { + type: "boolean", + default: false, + }, + }, + additionalProperties: false, + }, + ], }, + defaultOptions: [FLAGS], create: (ctx) => { return { JSXAttribute: (node) => { @@ -46,12 +68,6 @@ export const noPhysicalProperties: RuleModule = { const isClassAttribute = ["className", "class"].includes(attr); if (!isClassAttribute) return; - // let result = extractFromNode(node); - // if (!result) return; - - // result = result.filter((c) => typeof c === "string"); - // if (!result.length) return; - // const classesAsString = result.join(" "); // const cachedValid = cache.get(classesAsString); // if (cachedValid) { @@ -60,14 +76,20 @@ export const noPhysicalProperties: RuleModule = { // return; // } - const tokens = extractTokensFromNode(node, ctx, "checker"); + const allowPhysicalInsetWithAbsolute = + ctx.options[0]?.allowPhysicalInsetWithAbsolute ?? + FLAGS.allowPhysicalInsetWithAbsolute; + const debug = ctx.options[0]?.debug ?? FLAGS.debug; + + const tokens = extractTokensFromNode(node, ctx, { debug }); tokens?.forEach((token) => { const classValue = token?.getValue(); if (!classValue) return; - const classes = classValue.split(" "); - - const parsed = parseForPhysicalClasses(classes); + const parsed = parseForPhysicalClasses( + classValue, + allowPhysicalInsetWithAbsolute + ); const isInvalid = parsed.some((p) => p.isInvalid); if (!isInvalid) return; @@ -86,8 +108,12 @@ export const noPhysicalProperties: RuleModule = { }, }; +export function ruleSettings(options: Partial) { + return options; +} + export type Context = Readonly< - RuleContext<"NO_PHYSICAL_CLASSESS" | "IDENTIFIER_USED", []> + RuleContext >; function report({ diff --git a/src/rules/no-phyisical-properties/tailwind.ts b/src/rules/no-phyisical-properties/tailwind.ts index d3d9b4b..aeff3a2 100644 --- a/src/rules/no-phyisical-properties/tailwind.ts +++ b/src/rules/no-phyisical-properties/tailwind.ts @@ -3,8 +3,8 @@ export const twLogicalClasses = [ { physical: "mr-", /* */ logical: "me-" }, { physical: "pl-", /* */ logical: "ps-" }, { physical: "pr-", /* */ logical: "pe-" }, - { physical: "left-", /* */ logical: "start-" }, - { physical: "right-", /* */ logical: "end-" }, + { physical: "left-", /* */ logical: "start-", if: isNotAbsoluteCenterd }, + { physical: "right-", /* */ logical: "end-", if: isNotAbsoluteCenterd }, { physical: "text-left", /* */ logical: "text-start" }, { physical: "text-right", /* */ logical: "text-end" }, { physical: "border-l-", /* */ logical: "border-s-" }, @@ -19,7 +19,11 @@ export const twLogicalClasses = [ { physical: "scroll-mr-", /* */ logical: "scroll-me-" }, { physical: "scroll-pl-", /* */ logical: "scroll-ps-" }, { physical: "scroll-pr-", /* */ logical: "scroll-pe-" }, -] satisfies { physical: string; logical: string }[]; +] satisfies { + physical: string; + logical: string; + if?: (className: string) => boolean; +}[]; export function tailwindClassCases(cls: string) { return [ @@ -33,11 +37,35 @@ export function tailwindClassCases(cls: string) { ]; } -const allCases = twLogicalClasses.flatMap(({ physical, logical }) => - tailwindClassCases(physical).map((regex) => ({ regex, physical, logical })) -); +function getAllCases( + className: string, + allowPhysicalInsetWithAbsolute: boolean +) { + return twLogicalClasses.flatMap((cls) => { + const shouldValidate = allowPhysicalInsetWithAbsolute + ? cls.if?.(className) ?? true + : true; + if (!shouldValidate) return []; + + const { physical, logical } = cls; + return tailwindClassCases(physical).map((regex) => { + return { + regex, + physical, + logical, + }; + }); + }); +} + +export function parseForPhysicalClasses( + className: string, + allowPhysicalInsetWithAbsolute: boolean +) { + const allCases = getAllCases(className, allowPhysicalInsetWithAbsolute); + + const classes = className.split(" "); -export function parseForPhysicalClasses(classes: string[]) { return classes.map((cls) => { const isInvalid = allCases.some(({ regex }) => regex.test(cls)); const valid = allCases.reduce( @@ -64,3 +92,12 @@ export function parseForPhysicalClasses(classes: string[]) { // }); // }); } + +function isNotAbsoluteCenterd(className: string) { + return !["absolute", "fixed", "sticky"].some((c) => { + // We match absolute-CENTERED not every absolute position + // We encourage the usage of logical properties with positioning except for valid + // cases like center with fixed/absolute + return className.includes(c) && className.includes("translate-x"); + }); +} diff --git a/src/rules/no-phyisical-properties/test.ts b/src/rules/no-phyisical-properties/test.ts index fa17b15..eff09e2 100644 --- a/src/rules/no-phyisical-properties/test.ts +++ b/src/rules/no-phyisical-properties/test.ts @@ -611,4 +611,65 @@ vitest.describe(RULE_NAME, () => { }, ], }); + + tester.run("inset with absolute centerd", noPhysicalProperties, { + valid: [ + ``, + ``, + { + // Valid even with left-* because it has fixed and translate-x + code: ``, + options: [{ allowPhysicalInsetWithAbsolute: true }], + }, + { + code: ``, + options: [{ allowPhysicalInsetWithAbsolute: true }], + }, + { + code: ``, + options: [{ allowPhysicalInsetWithAbsolute: true }], + }, + ], + invalid: [ + { + code: ``, + output: ``, + errors: [{ messageId: NO_PHYSICAL_CLASSESS }], + }, + { + code: ``, + output: ``, + errors: [{ messageId: NO_PHYSICAL_CLASSESS }], + }, + { + code: ``, + output: ``, + errors: [{ messageId: NO_PHYSICAL_CLASSESS }], + }, + { + code: ``, + output: ``, + errors: [{ messageId: NO_PHYSICAL_CLASSESS }], + }, + { + options: [{ allowPhysicalInsetWithAbsolute: false }], + code: ``, + errors: [{ messageId: NO_PHYSICAL_CLASSESS }], + output: ``, + }, + { + options: [ + { allowPhysicalInsetWithAbsolute: undefined as unknown as boolean }, + ], + code: ``, + errors: [{ messageId: NO_PHYSICAL_CLASSESS }], + output: ``, + }, + { + code: ``, + errors: [{ messageId: NO_PHYSICAL_CLASSESS }], + output: ``, + }, + ], + }); }); diff --git a/src/utils/eslint.ts b/src/utils/eslint.ts index 331447a..5723ecb 100644 --- a/src/utils/eslint.ts +++ b/src/utils/eslint.ts @@ -1,47 +1,47 @@ -/// - -import type { TSESLint, TSESTree } from "@typescript-eslint/utils"; - -export function findVariable( - initialScope: TSESLint.Scope.Scope, - node: TSESTree.Identifier -) { - let scope: TSESLint.Scope.Scope | null = initialScope; - const name = node.name; - - scope = getInnermostScope(scope, node); - - while (scope != null) { - const variable = scope.set.get(name); - if (variable != null) { - return variable; - } - scope = scope.upper; - } - - return null; -} - -export function getInnermostScope( - initialScope: TSESLint.Scope.Scope, - node: TSESTree.Node -): TSESLint.Scope.Scope { - const location = node.range[0]; - - let scope = initialScope; - let found = false; - do { - found = false; - for (const childScope of scope.childScopes) { - const range = childScope.block.range; - - if (range[0] <= location && location < range[1]) { - scope = childScope; - found = true; - break; - } - } - } while (found); - - return scope; -} +// /// + +// import type { TSESLint, TSESTree } from "@typescript-eslint/utils"; + +// export function findVariable( +// initialScope: TSESLint.Scope.Scope, +// node: TSESTree.Identifier +// ) { +// let scope: TSESLint.Scope.Scope | null = initialScope; +// const name = node.name; + +// scope = getInnermostScope(scope, node); + +// while (scope != null) { +// const variable = scope.set.get(name); +// if (variable != null) { +// return variable; +// } +// scope = scope.upper; +// } + +// return null; +// } + +// export function getInnermostScope( +// initialScope: TSESLint.Scope.Scope, +// node: TSESTree.Node +// ): TSESLint.Scope.Scope { +// const location = node.range[0]; + +// let scope = initialScope; +// let found = false; +// do { +// found = false; +// for (const childScope of scope.childScopes) { +// const range = childScope.block.range; + +// if (range[0] <= location && location < range[1]) { +// scope = childScope; +// found = true; +// break; +// } +// } +// } while (found); + +// return scope; +// } diff --git a/vite.config.ts b/vite.config.ts index b67efc8..21b47cd 100644 --- a/vite.config.ts +++ b/vite.config.ts @@ -8,5 +8,6 @@ export default defineConfig({ coverage: { include: ["src/**"], }, + slowTestThreshold: 10, }, }); From f7fa50a8bab6f1ca648c270c6abb745cb1118566 Mon Sep 17 00:00:00 2001 From: Ahmed Abdelbaset Date: Sat, 28 Sep 2024 02:15:47 +0300 Subject: [PATCH 2/2] changeset --- .changeset/young-ties-relate.md | 7 +++++++ 1 file changed, 7 insertions(+) create mode 100644 .changeset/young-ties-relate.md diff --git a/.changeset/young-ties-relate.md b/.changeset/young-ties-relate.md new file mode 100644 index 0000000..12c55e1 --- /dev/null +++ b/.changeset/young-ties-relate.md @@ -0,0 +1,7 @@ +--- +"eslint-plugin-rtl-friendly": minor +--- + +Add option `allowPhysicalInsetWithAbsolute` to allow the use of `left-1/2` with `fixed -translate-x-1/2` + +Add option `debug`