diff --git a/eslint.config.js b/eslint.config.js index e2e569879be6b..5b3767d5b2a49 100644 --- a/eslint.config.js +++ b/eslint.config.js @@ -11,6 +11,7 @@ import reactCompiler from 'eslint-plugin-react-compiler'; import astryxPlugin from './internal/eslint-plugin-astryx/index.js'; import {scope as internalBarrelScope} from './internal/eslint-plugin-astryx/no-internal-barrel-import.js'; import {scope as nullishGuardScope} from './internal/eslint-plugin-astryx/no-nullish-jsx-guard.js'; +import {scope as privateAttributeScope} from './internal/eslint-plugin-astryx/no-raw-private-attribute.js'; /* global process */ @@ -59,6 +60,19 @@ const nullishGuardBaseline = JSON.parse( ), ); +// Private `data-astryx-*` attributes written by hand before +// `@astryx/no-raw-private-attribute` arrived, by repo-relative file, once per +// site. The list only shrinks. +const privateAttributeBaseline = JSON.parse( + readFileSync( + path.join( + import.meta.dirname, + 'internal/eslint-plugin-astryx/no-raw-private-attribute.baseline.json', + ), + 'utf8', + ), +); + // The internal plugin is plain untyped JS (kept out of the lint/type surface), // so its inferred shape doesn't satisfy ESLint's strict `Plugin` type. One // localized assertion here keeps every `plugins` entry below type-checked. @@ -916,6 +930,28 @@ export default defineConfig( ], }, }, + // A private `data-astryx-*` attribute is written once, in its owner module + // under packages//src/privateAttributes, which exports ready-made + // styles, setter props and readers (`architecture:public-component-api` + // INV23, which also lists the public attributes the rule allows). The + // hand-written sites that predate this block are listed in the baseline, + // once per site, and that many stay allowed until they move into an owner + // module; one more is reported (an error in strict mode, so a pull request + // that adds one fails lint, while main stays green). Tests are out of scope: + // they assert the DOM a component renders. + { + files: privateAttributeScope.files, + ignores: privateAttributeScope.ignores, + plugins: { + '@astryx': astryxEslintPlugin, + }, + rules: { + '@astryx/no-raw-private-attribute': [ + reactSeverity, + {baseline: privateAttributeBaseline, root: import.meta.dirname}, + ], + }, + }, // richtext — the pre-existing backlog, held at `warn` while it is worked // off. Must come last so it overrides the blocks above. // diff --git a/internal/eslint-plugin-astryx/README.md b/internal/eslint-plugin-astryx/README.md index 6bee9f3af2572..6e3434098be65 100644 --- a/internal/eslint-plugin-astryx/README.md +++ b/internal/eslint-plugin-astryx/README.md @@ -210,6 +210,58 @@ removes them; remove an entry when you fix its import, and never add one. rewrites the list from the source (`--check` verifies it), for example after merging `main` into this rule's own branch or after a codemod. +### `@astryx/no-raw-private-attribute` + +Reports a private `data-astryx-*` attribute written by hand outside its owner +module (`architecture:public-component-api` INV23). Each private attribute has +one owner module in the package that writes it, +`packages//src/privateAttributes/.stylex.ts`, named in camelCase +(`focusOwner.stylex.ts` owns `data-astryx-focus-owner`), that writes its +name and selectors once and exports ready-made styles, setter props and the +runtime readers its package needs; every other file spreads those exports. + +```ts +// flagged outside the owner module +el.setAttribute('data-astryx-popover-fallback-close', ''); +const styles = stylex.create({ + base: {transform: {default: null, '[data-astryx-press="on"]': 'scale(0.98)'}}, +}); +
; +style.setAttribute(dataAttr('theme-prose'), name); + +// fine: public attributes INV23 lists +
; +const scope = `[data-astryx-media="${surface}"]`; +el.setAttribute(dataAttr('theme'), name); +``` + +It checks string literals in any position (object keys, call arguments, +selectors inside `stylex.create` and `stylex.when.*`), +template literals (including `data-${dataAttrNamespace}-…`), JSX attribute +names, and `dataAttr()` calls. A name it cannot read (`data-astryx-${part}`, +`dataAttr(name)`) is reported as `data-astryx-*`. Inside an owner module only +its own attribute may be written, and the module may not export the raw name or +a selector as a string: not as an exported or default-exported value, not inside +an exported object, array or function's return value, and not through a module +constant one of those uses, including a selector built from that constant in a +template or with `+`. The name stays allowed there as an object key +(setter props and `stylex.create` selectors) and as a call argument (a writer, +or a `closest`/`matches` reader). An import of another package's owner module is +reported. The public attributes INV23 lists are never reported: `theme`, +`media`, `aspect-ratio-override`, `token`, `token-value`, `menu-press`, +`pressable` and `edge-comp`, each after `data-astryx-`. `naming.ts`, which +builds every name, is not checked. + +The repo config turns it on for `packages/*/src`, outside tests, stories and +test helpers, at the strict-mode severity (an error in CI). The repo config does +not lint `.mjs` outside the CLI, so `.doc.mjs` text stays with public-API +review. Sites that existed when the rule arrived are listed by file in +`no-raw-private-attribute.baseline.json`, once per site, and that many stay +allowed until they move into an owner module; remove an entry when you move its +site, and never add one. +`node internal/eslint-plugin-astryx/no-raw-private-attribute.baseline.mjs` +rewrites the list from the source (`--check` verifies it). + ### `@astryx/no-core-internal-import` Reports imports of Core's internal subpaths (`@astryxdesign/core/internal` and diff --git a/internal/eslint-plugin-astryx/index.js b/internal/eslint-plugin-astryx/index.js index beea64894f094..5bfa843f18ff7 100644 --- a/internal/eslint-plugin-astryx/index.js +++ b/internal/eslint-plugin-astryx/index.js @@ -23,6 +23,7 @@ * - no-light-dark-outside-theme: Flags CSS light-dark() in component source (a light/dark decision belongs to the theme layer, where a token pair reaches both schemes in every theme) * - no-core-internal-import: Keeps Core's internal subpaths (@astryxdesign/core/internal/*) inside Core and Lab * - no-internal-barrel-import: Flags a package-internal relative import that resolves to a folder's index (a barrel) instead of the defining module + * - no-raw-private-attribute: Flags a private data-astryx-* attribute written by hand outside its owner module in packages//src/privateAttributes (INV23 lists the public ones) * - no-raw-color: Flags a raw colour value (hex, rgb(), hsl(), oklch(), …) anywhere in component source, including inside light-dark()/color-mix(), behind a const, in a template literal, or as a var() fallback — the shapes no-hardcoded-styles cannot see * * Philosophy: Strict for agents (CI), lenient for humans (local dev) @@ -64,6 +65,7 @@ import i18nKeyFormatRule from './i18n-key-format.js'; import requireTableSectionRule from './require-table-section.js'; import noLightDarkOutsideThemeRule from './no-light-dark-outside-theme.js'; import noRawColorRule from './no-raw-color.js'; +import noRawPrivateAttributeRule from './no-raw-private-attribute.js'; // ============================================================================= // Rule: no-hardcoded-styles @@ -368,6 +370,7 @@ const plugin = { 'require-table-section': requireTableSectionRule, 'no-light-dark-outside-theme': noLightDarkOutsideThemeRule, 'no-raw-color': noRawColorRule, + 'no-raw-private-attribute': noRawPrivateAttributeRule, }, configs: {}, }; diff --git a/internal/eslint-plugin-astryx/no-nullish-jsx-guard.js b/internal/eslint-plugin-astryx/no-nullish-jsx-guard.js index 2d82bad41fbe3..1da903085a750 100644 --- a/internal/eslint-plugin-astryx/no-nullish-jsx-guard.js +++ b/internal/eslint-plugin-astryx/no-nullish-jsx-guard.js @@ -68,7 +68,7 @@ * are not flagged — the guarded branch is not a JSX element. */ -import path from 'node:path'; +import {baselineAllowance} from './rule-baseline.mjs'; /** * Where the repository config enforces the rule with its baseline; the @@ -383,23 +383,9 @@ const rule = { return {}; } const sourceCode = context.sourceCode ?? context.getSourceCode(); - const options = context.options[0] ?? {}; - const root = options.root ?? context.cwd; - const relative = path.relative(root, filename).split(path.sep).join('/'); // Each listed value allows as many checks as it is listed; the next one // is reported. - const allowance = new Map(); - for (const value of options.baseline?.[relative] ?? []) { - allowance.set(value, (allowance.get(value) ?? 0) + 1); - } - function isAllowed(valueText) { - const left = allowance.get(valueText) ?? 0; - if (left === 0) { - return false; - } - allowance.set(valueText, left - 1); - return true; - } + const isAllowed = baselineAllowance(context); function reportHandRolled(chain, value) { const valueText = sourceCode.getText(value); diff --git a/internal/eslint-plugin-astryx/no-raw-private-attribute.baseline.json b/internal/eslint-plugin-astryx/no-raw-private-attribute.baseline.json new file mode 100644 index 0000000000000..e2dd8379f4d57 --- /dev/null +++ b/internal/eslint-plugin-astryx/no-raw-private-attribute.baseline.json @@ -0,0 +1,160 @@ +{ + "packages/core/src/Button/Button.tsx": [ + "data-astryx-press", + "data-astryx-press", + "data-astryx-press" + ], + "packages/core/src/Chat/ChatComposerInput.tsx": [ + "data-astryx-dictation-interim" + ], + "packages/core/src/Chat/ChatLayout.tsx": ["data-astryx-scrolling"], + "packages/core/src/Chat/useChatComposerTokens.ts": ["data-astryx-token-id"], + "packages/core/src/Chat/useChatDictation.ts": [ + "data-astryx-dictation-interim" + ], + "packages/core/src/Chat/useChatStreamScroll.ts": [ + "data-astryx-chat-following", + "data-astryx-chat-following-style" + ], + "packages/core/src/Chat/useTriggerMenu.tsx": ["data-astryx-trigger-anchor"], + "packages/core/src/CheckboxInput/CheckboxInput.tsx": [ + "data-astryx-press", + "data-astryx-press" + ], + "packages/core/src/ClickableCard/ClickableCard.tsx": [ + "data-astryx-press", + "data-astryx-press" + ], + "packages/core/src/CodeBlock/highlightRanges.ts": [ + "data-astryx-highlight-dynamic" + ], + "packages/core/src/CodeBlock/highlightStyles.ts": [ + "data-astryx-highlight-styles" + ], + "packages/core/src/ComplexSelector/ComplexSelector.tsx": [ + "data-astryx-press", + "data-astryx-press", + "data-astryx-press" + ], + "packages/core/src/DateInput/nativeDateSegments.ts": [ + "data-astryx-date-probe" + ], + "packages/core/src/DropdownMenu/DropdownMenu.tsx": [ + "data-astryx-submenu-host", + "data-astryx-submenu-host", + "data-astryx-submenu-regions" + ], + "packages/core/src/DropdownMenu/DropdownMenuSubMenu.tsx": [ + "data-astryx-submenu-regions", + "data-astryx-submenu-regions" + ], + "packages/core/src/InputGroup/groupStyles.ts": [ + "data-astryx-input-group-item", + "data-astryx-input-group-item" + ], + "packages/core/src/Layer/layerSlideRules.ts": [ + "data-astryx-clears-anchor", + "data-astryx-layer-scrolls" + ], + "packages/core/src/MultiSelector/MultiSelector.tsx": [ + "data-astryx-press", + "data-astryx-press", + "data-astryx-press", + "data-astryx-press" + ], + "packages/core/src/NavItem/navItemStyles.stylex.ts": [ + "data-astryx-press", + "data-astryx-press" + ], + "packages/core/src/Popover/Popover.tsx": [ + "data-astryx-popover-fallback-close", + "data-astryx-popover-fallback-close" + ], + "packages/core/src/Popover/usePopover.tsx": [ + "data-astryx-popover-fallback-close", + "data-astryx-popover-fallback-close" + ], + "packages/core/src/RadioList/RadioListItem.tsx": [ + "data-astryx-press", + "data-astryx-press" + ], + "packages/core/src/SelectableCard/SelectableCard.tsx": [ + "data-astryx-press", + "data-astryx-press" + ], + "packages/core/src/Selector/Selector.tsx": [ + "data-astryx-press", + "data-astryx-press", + "data-astryx-press", + "data-astryx-press" + ], + "packages/core/src/Switch/Switch.tsx": [ + "data-astryx-press", + "data-astryx-press", + "data-astryx-press", + "data-astryx-press" + ], + "packages/core/src/TabList/Tab.tsx": [ + "data-astryx-press", + "data-astryx-press" + ], + "packages/core/src/Table/glidingDivider.ts": [ + "data-astryx-cell-gliding", + "data-astryx-cell-gliding", + "data-astryx-cell-gliding", + "data-astryx-row-gliding", + "data-astryx-row-gliding", + "data-astryx-table-pinned" + ], + "packages/core/src/Table/plugins/stickyColumns/useTableStickyColumns.tsx": [ + "data-astryx-cell-gliding", + "data-astryx-table-pinned", + "data-astryx-table-pinned" + ], + "packages/core/src/Thumbnail/Thumbnail.tsx": [ + "data-astryx-press", + "data-astryx-press" + ], + "packages/core/src/Toast/useToast.tsx": ["data-astryx-toast-fallback"], + "packages/core/src/TopNav/TopNavItem.tsx": [ + "data-astryx-press", + "data-astryx-press", + "data-astryx-press", + "data-astryx-press" + ], + "packages/core/src/gestures/haptics.ts": ["data-astryx-haptic-switch"], + "packages/core/src/gestures/touchActionRule.ts": ["data-astryx-touch-action"], + "packages/core/src/hooks/useAnnounce.ts": [ + "data-astryx-live-region", + "data-astryx-modal-live-region" + ], + "packages/core/src/hooks/useMenuPress.ts": ["data-astryx-menu-press-surface"], + "packages/core/src/theme/Theme.tsx": [ + "data-astryx-id", + "data-astryx-id", + "data-astryx-id", + "data-astryx-id", + "data-astryx-theme-prose", + "data-astryx-theme-prose" + ], + "packages/core/src/theme/syntax/SyntaxTheme.tsx": [ + "data-astryx-syntax-theme" + ], + "packages/core/src/utils/interactionOverlay.stylex.ts": [ + "data-astryx-press", + "data-astryx-press", + "data-astryx-press", + "data-astryx-press", + "data-astryx-press", + "data-astryx-press", + "data-astryx-press", + "data-astryx-press" + ], + "packages/core/src/utils/pressFeedback.ts": ["data-astryx-press"], + "packages/lab/src/Schedule/useScheduleViewPopover.ts": [ + "data-astryx-popover-fallback-close" + ], + "packages/richtext/src/CodeSyntaxPlugin.tsx": [ + "data-astryx-richtext-code-syntax" + ] +} diff --git a/internal/eslint-plugin-astryx/no-raw-private-attribute.baseline.mjs b/internal/eslint-plugin-astryx/no-raw-private-attribute.baseline.mjs new file mode 100644 index 0000000000000..78266b69c7677 --- /dev/null +++ b/internal/eslint-plugin-astryx/no-raw-private-attribute.baseline.mjs @@ -0,0 +1,39 @@ +// Copyright (c) Meta Platforms, Inc. and affiliates. + +/** + * @file no-raw-private-attribute.baseline.mjs + * @description Writes (or, with --check, verifies) the baseline that + * `@astryx/no-raw-private-attribute` allows: each private `data-astryx-*` + * attribute, per file, once per site that writes it by hand today. Run it + * after moving sites into an owner module under + * `packages/core/src/privateAttributes`, or after merging main into a branch + * that changes the rule, so the list only names sites that still exist. + * + * node internal/eslint-plugin-astryx/no-raw-private-attribute.baseline.mjs [--check] + */ + +import fs from 'node:fs'; +import path from 'node:path'; +import {scope} from './no-raw-private-attribute.js'; +import {runRuleBaseline} from './rule-baseline.mjs'; + +const root = path.resolve(import.meta.dirname, '../..'); + +await runRuleBaseline({ + ruleId: '@astryx/no-raw-private-attribute', + scope, + targets: fs + .readdirSync(path.join(root, 'packages')) + .filter(name => fs.existsSync(path.join(root, 'packages', name, 'src'))) + .map(name => `packages/${name}/src`), + baselineFile: path.join( + import.meta.dirname, + 'no-raw-private-attribute.baseline.json', + ), + // Owner-module reports (an exported name) start differently and are never + // baselined. + entryFromMessage: message => + /^`(data-astryx-[^`]*)`/.exec(message)?.[1] ?? null, + unique: false, + noun: 'private-attribute sites', +}); diff --git a/internal/eslint-plugin-astryx/no-raw-private-attribute.js b/internal/eslint-plugin-astryx/no-raw-private-attribute.js new file mode 100644 index 0000000000000..a1c19cdfa019b --- /dev/null +++ b/internal/eslint-plugin-astryx/no-raw-private-attribute.js @@ -0,0 +1,521 @@ +// Copyright (c) Meta Platforms, Inc. and affiliates. + +/** + * @file no-raw-private-attribute.js + * @description Reports a private `data-astryx-*` attribute written by hand + * outside its owner module (`architecture:public-component-api` INV23). + * + * A `data-astryx-*` attribute is private unless INV23 names it public. Each + * private attribute has one owner module in the package that writes it, + * `packages//src/privateAttributes/.stylex.ts` with the name in + * camelCase (`focusOwner.stylex.ts` for `data-astryx-focus-owner`), that writes its + * name and selectors once and exports ready-made styles, setter props and the + * runtime readers its package needs. Every other file spreads those exports, + * so the attribute can be renamed or reshaped in one place and never leaks + * into the public surface by accident. + * + * Reported, outside the owner modules: + * - a string literal that contains `data-astryx-`, in any position + * (an object key, a `setAttribute` argument, a selector inside + * `stylex.create` or `stylex.when.*`); + * - a template literal that does, including one built from the namespace + * constant (`data-${dataAttrNamespace}-press`); + * - a JSX attribute named `data-astryx-`; + * - a `dataAttr('')` call that names a private attribute. + * A name the rule cannot read (`data-astryx-${part}`, `dataAttr(name)`) is + * reported as `data-astryx-*`: it cannot be shown to be public. + * + * Inside an owner module, only its own attribute may be written, and the + * module may not export the raw name or a selector as a string: not as an + * exported or default-exported value, not inside an exported object, array or + * function's return value, and not through a module constant that one of + * those uses, including a string built from that constant in a template or + * with `+`. The name stays allowed as an object key (setter props and + * `stylex.create` selectors) and as a call argument (a writer or a + * `closest`/`matches` reader). Anywhere, an + * import of another package's owner module is reported: a private attribute + * stays inside the package that writes it. + * + * Public, never reported: the names INV23 lists (`PUBLIC_ATTRIBUTES`). + * `naming.ts`, which builds every `data-astryx-*` name, is not checked. + * + * Known limitations: a name assembled from the namespace with `+` + * (`'data-' + NAMESPACE + '-press'`), a template that reaches the namespace + * through anything but `dataAttrNamespace` or `NAMESPACE` (a member access, + * an aliased import, `classPrefix`), and a name read through `element.dataset` + * are not seen; no shipped source does any of these today, and public-API + * review covers them. + * + * Options: `{baseline: {[repoRelativeFile]: string[]}, root: string}`. The + * baseline lists, per file, each private attribute once per site that + * predates the rule; that many sites stay allowed until moved into the owner + * module, and one more is reported. The list only shrinks. + * `no-raw-private-attribute.baseline.mjs` writes it. + */ + +import path from 'node:path'; +import {baselineAllowance} from './rule-baseline.mjs'; + +/** + * Where the repository config enforces the rule with its baseline: shipped + * package source. Tests, stories and test helpers never ship, and they read + * attributes back from the DOM to check behavior, so they may name them. + * Shared with the baseline script so both lint the same files. + */ +export const scope = { + files: ['packages/*/src/**/*.{ts,tsx,mts,cts,js,jsx,mjs}'], + ignores: [ + '**/*.test.*', + '**/*.spec.*', + '**/*.stories.*', + '**/*.testing.*', + '**/*.render-testing.*', + '**/__tests__/**', + '**/__fixtures__/**', + '**/dist/**', + ], +}; + +const PREFIX = 'data-astryx-'; + +/** + * The data attributes INV23 names public, after the `data-astryx-` prefix: + * the theme and media-surface scopes, AspectRatio's override hook, the Chat + * composer token's attributes, the menu-root and press markers, and the + * edge-compensation marker. They change only under INV9. + */ +export const PUBLIC_ATTRIBUTES = new Set([ + 'theme', + 'media', + 'aspect-ratio-override', + 'token', + 'token-value', + 'menu-press', + 'pressable', + 'edge-comp', +]); + +/** + * Identifiers that hold the `astryx` namespace segment, exported by + * `naming.ts`; a template that interpolates one still spells the attribute. + */ +const NAMESPACE_IDENTIFIERS = new Set(['dataAttrNamespace', 'NAMESPACE']); + +/** Stands in for an interpolated value whose text is unknown. */ +const UNKNOWN = '\u0000'; + +const OWNER_MODULE = + /(?:^|\/)packages\/[^/]+\/src\/privateAttributes\/([^/]+)\.stylex\.ts$/; +const PACKAGE_SOURCE = /(?:^|\/)packages\/([^/]+)\/src\//; +const NAMING_MODULE = /(?:^|\/)packages\/core\/src\/naming\.ts$/; + +/** + * Every `data-astryx-*` attribute a piece of text names, as the attribute + * name after the prefix, or `null` where the name is interpolated and so + * cannot be read. + * + * @param {string} text + * @returns {Array} + */ +export function attributeNames(text) { + const names = []; + const pattern = /data-astryx-([a-z0-9-]*)/gi; + let match; + while ((match = pattern.exec(text)) != null) { + const name = match[1].replace(/-+$/, '').toLowerCase(); + const next = text[match.index + match[0].length]; + names.push(name === '' || next === UNKNOWN ? null : name); + } + return names; +} + +/** `focusOwner` → `focus-owner`; a kebab-case stem is returned unchanged. */ +function kebabCase(stem) { + return stem.replace(/[A-Z]/g, letter => `-${letter.toLowerCase()}`); +} + +/** `focus-owner` → `focusOwner`, the owner module's file stem. */ +function camelCase(name) { + return name.replace(/-([a-z0-9])/g, (_, letter) => letter.toUpperCase()); +} + +/** `data-astryx-press`, or `data-astryx-*` for a name that cannot be read. */ +function attributeLabel(name) { + return name == null ? `${PREFIX}*` : `${PREFIX}${name}`; +} + +/** The text of a template literal, interpolating the namespace constant. */ +function templateText(node) { + let text = ''; + node.quasis.forEach((quasi, index) => { + text += quasi.value.cooked ?? quasi.value.raw; + const expression = node.expressions[index]; + if (expression != null) { + text += + expression.type === 'Identifier' && + NAMESPACE_IDENTIFIERS.has(expression.name) + ? 'astryx' + : UNKNOWN; + } + }); + return text; +} + +/** The static text of a call argument, or `null` when it is computed. */ +function staticText(node) { + if (node?.type === 'Literal' && typeof node.value === 'string') { + return node.value; + } + if (node?.type === 'TemplateLiteral' && node.expressions.length === 0) { + return node.quasis[0].value.cooked ?? node.quasis[0].value.raw; + } + return null; +} + +function isDataAttrCall(node) { + const callee = node.callee; + return ( + (callee.type === 'Identifier' && callee.name === 'dataAttr') || + (callee.type === 'MemberExpression' && + !callee.computed && + callee.property.type === 'Identifier' && + callee.property.name === 'dataAttr') + ); +} + +/** Wrappers that pass their operand's value through unchanged. */ +const PASS_THROUGH = new Set([ + 'TSAsExpression', + 'TSSatisfiesExpression', + 'TSNonNullExpression', + 'TSTypeAssertion', + 'ChainExpression', + 'AwaitExpression', + 'LogicalExpression', + 'ArrayExpression', + 'ObjectExpression', + 'SpreadElement', +]); + +const FUNCTIONS = new Set([ + 'FunctionDeclaration', + 'FunctionExpression', + 'ArrowFunctionExpression', +]); + +/** + * Where the value of `node` ends up, following it out through wrappers, + * strings built from it (a template slot, a `+` operand), object property + * values, array elements and function return values: + * `{exported: true}` when it reaches an export, `{binding}` when it is bound + * to a name (a variable declarator or a function declaration) whose uses + * decide, or `null` when it is consumed some other way (an object key, a call + * argument, a member access). + */ +function valueDestination(node) { + let current = node; + for (;;) { + const parent = current.parent; + if (parent == null) { + return null; + } + if ( + PASS_THROUGH.has(parent.type) || + (parent.type === 'ConditionalExpression' && parent.test !== current) || + // A string built from the value carries it on: a template slot, or an + // operand of `+`. + parent.type === 'TemplateLiteral' || + (parent.type === 'BinaryExpression' && parent.operator === '+') || + (parent.type === 'SequenceExpression' && + parent.expressions.at(-1) === current) || + (parent.type === 'Property' && parent.value === current) || + (parent.type === 'ArrowFunctionExpression' && parent.body === current) + ) { + current = parent; + continue; + } + if (parent.type === 'ReturnStatement') { + let fn = parent.parent; + while (fn != null && !FUNCTIONS.has(fn.type)) { + fn = fn.parent; + } + if (fn == null) { + return null; + } + if (fn.type === 'FunctionDeclaration') { + const holder = fn.parent?.type; + if ( + holder === 'ExportNamedDeclaration' || + holder === 'ExportDefaultDeclaration' + ) { + return {exported: true}; + } + return fn.id == null ? null : {binding: fn}; + } + current = fn; + continue; + } + if (parent.type === 'VariableDeclarator' && parent.init === current) { + if (parent.parent?.parent?.type === 'ExportNamedDeclaration') { + return {exported: true}; + } + return parent.id.type === 'Identifier' ? {binding: parent} : null; + } + if ( + parent.type === 'ExportDefaultDeclaration' || + parent.type === 'ExportSpecifier' + ) { + return {exported: true}; + } + return null; + } +} + +const rule = { + meta: { + type: 'problem', + docs: { + description: + 'Disallow private data-astryx-* attributes written outside their owner module in packages//src/privateAttributes', + }, + schema: [ + { + type: 'object', + properties: { + baseline: { + type: 'object', + additionalProperties: {type: 'array', items: {type: 'string'}}, + }, + root: {type: 'string'}, + }, + additionalProperties: false, + }, + ], + messages: { + raw: + '`{{attribute}}` is a private data attribute. Write it only in its owner module, ' + + '{{owner}}, and spread the styles, setter props and readers it exports ' + + '(architecture:public-component-api INV23 lists the public attributes).', + unreadable: + '`{{attribute}}` names a data attribute this rule cannot read, so it cannot be ' + + 'shown to be public. Write a private attribute only in its owner module under ' + + "{{ownerDirectory}}, and build a public one by name, as in dataAttr('theme').", + foreignOwner: + '`{{attribute}}` belongs in {{owner}}; each owner module writes only its own ' + + 'attribute.', + foreignPackage: + "'{{source}}' imports another package's private-attribute owner module. A private " + + 'attribute stays inside the package that writes it; reach it through that ' + + "package's public API.", + exportedName: + 'An owner module exports styles, setter props and readers, never its attribute name ' + + 'or a selector on it as a string ({{attribute}}).', + }, + }, + create(context) { + const filename = (context.filename ?? context.getFilename()) + .split(path.sep) + .join('/'); + if (NAMING_MODULE.test(filename)) { + return {}; + } + // Owner modules are named in camelCase (`focusOwner.stylex.ts` owns + // `data-astryx-focus-owner`); compare in the attribute's kebab case. + const ownerStem = OWNER_MODULE.exec(filename)?.[1] ?? null; + const ownedName = ownerStem == null ? null : kebabCase(ownerStem); + const packageName = PACKAGE_SOURCE.exec(filename)?.[1] ?? null; + const ownerDirectory = `packages/${packageName ?? ''}/src/privateAttributes`; + const ownerOf = name => + name == null + ? ownerDirectory + : `${ownerDirectory}/${camelCase(name)}.stylex.ts`; + const sourceCode = context.sourceCode ?? context.getSourceCode(); + // Each listed attribute allows as many sites as it is listed; the next + // one is reported. + const isAllowed = baselineAllowance(context); + + // Local names an owner module exports, by `export {name}`. + const exportedLocals = new Set(); + + function report(node, name) { + if (name != null && PUBLIC_ATTRIBUTES.has(name)) { + return; + } + const attribute = attributeLabel(name); + if (ownedName != null) { + if (name === ownedName) { + return; + } + if (isAllowed(attribute)) { + return; + } + context.report({ + node, + messageId: name == null ? 'unreadable' : 'foreignOwner', + data: {attribute, owner: ownerOf(name), ownerDirectory}, + }); + return; + } + if (isAllowed(attribute)) { + return; + } + context.report({ + node, + messageId: name == null ? 'unreadable' : 'raw', + data: {attribute, owner: ownerOf(name), ownerDirectory}, + }); + } + + // In an owner module: strings (and `dataAttr()` calls) that spell its + // own attribute, checked at the end for a path to an export. + const ownerValues = []; + function noteOwnerValue(node, names) { + if (ownedName != null && names.includes(ownedName)) { + ownerValues.push(node); + } + } + + /** Does the value of `node` reach an export, directly or through a name? */ + function reachesExport(node, seen) { + const destination = valueDestination(node); + if (destination == null) { + return false; + } + if (destination.exported) { + return true; + } + const {binding} = destination; + if (seen.has(binding)) { + return false; + } + seen.add(binding); + const name = binding.id.name; + const scopeHolder = + binding.type === 'VariableDeclarator' ? binding.parent : binding; + if (exportedLocals.has(name) && scopeHolder.parent?.type === 'Program') { + return true; + } + return sourceCode + .getDeclaredVariables(binding) + .filter(variable => variable.name === name) + .some(variable => + variable.references.some( + reference => + reference.identifier !== binding.id && + reference.isRead() && + reachesExport(reference.identifier, seen), + ), + ); + } + + /** + * Another package's owner module: a package specifier that reaches a + * `privateAttributes` folder, or a relative path into one under another + * package's `src`. + */ + function checkImport(source) { + const specifier = source?.value; + if ( + typeof specifier !== 'string' || + !/(?:^|\/)privateAttributes(?:\/|$)/.test(specifier) + ) { + return; + } + let foreign = !specifier.startsWith('.'); + if (!foreign) { + const target = path + .resolve( + path.dirname(context.filename ?? context.getFilename()), + specifier, + ) + .split(path.sep) + .join('/'); + const targetPackage = PACKAGE_SOURCE.exec(target)?.[1] ?? null; + foreign = targetPackage != null && targetPackage !== packageName; + } + if (foreign) { + context.report({ + node: source, + messageId: 'foreignPackage', + data: {source: specifier}, + }); + } + } + + function checkText(node, text) { + const names = attributeNames(text); + for (const name of names) { + report(node, name); + } + noteOwnerValue(node, names); + } + + return { + ImportDeclaration(node) { + checkImport(node.source); + }, + ExportNamedDeclaration(node) { + checkImport(node.source); + }, + ExportAllDeclaration(node) { + checkImport(node.source); + }, + ImportExpression(node) { + if (node.source.type === 'Literal') { + checkImport(node.source); + } + }, + Literal(node) { + if (typeof node.value !== 'string') { + return; + } + checkText(node, node.value); + }, + TemplateLiteral(node) { + checkText(node, templateText(node)); + }, + JSXAttribute(node) { + if (node.name.type !== 'JSXIdentifier') { + return; + } + if (node.name.name.toLowerCase().startsWith(PREFIX)) { + for (const name of attributeNames(node.name.name)) { + report(node.name, name); + } + } + }, + CallExpression(node) { + if (!isDataAttrCall(node)) { + return; + } + const text = staticText(node.arguments[0]); + const name = text == null || text === '' ? null : text.toLowerCase(); + // A literal argument that already spells the prefix is reported by + // the literal check. + if (text != null && text.toLowerCase().includes(PREFIX)) { + return; + } + report(node, name); + noteOwnerValue(node, [name]); + }, + ExportSpecifier(node) { + if (node.local.type === 'Identifier') { + exportedLocals.add(node.local.name); + } + }, + 'Program:exit'() { + for (const node of ownerValues) { + if (reachesExport(node, new Set())) { + context.report({ + node, + messageId: 'exportedName', + data: {attribute: attributeLabel(ownedName)}, + }); + } + } + }, + }; + }, +}; + +export default rule; diff --git a/internal/eslint-plugin-astryx/no-raw-private-attribute.test.mjs b/internal/eslint-plugin-astryx/no-raw-private-attribute.test.mjs new file mode 100644 index 0000000000000..4afbcbe85dc73 --- /dev/null +++ b/internal/eslint-plugin-astryx/no-raw-private-attribute.test.mjs @@ -0,0 +1,417 @@ +// Copyright (c) Meta Platforms, Inc. and affiliates. + +/** + * @file no-raw-private-attribute.test.mjs + * @description Tests for the Astryx no-raw-private-attribute ESLint rule. + */ + +import {RuleTester} from 'eslint'; +import tseslint from 'typescript-eslint'; +import rule from './no-raw-private-attribute.js'; + +const tester = new RuleTester({ + languageOptions: { + parser: tseslint.parser, + parserOptions: { + ecmaVersion: 2022, + sourceType: 'module', + ecmaFeatures: {jsx: true}, + }, + }, +}); + +const site = '/repo/packages/core/src/Button/Button.tsx'; +const owner = '/repo/packages/core/src/privateAttributes/press.stylex.ts'; +const baselined = attributes => [ + { + baseline: {'packages/core/src/Button/Button.tsx': attributes}, + root: '/repo', + }, +]; + +// RuleTester registers its own describe/it blocks internally, so it must run +// at the top level (Vitest 4 forbids calling suite functions inside an it()). +tester.run('no-raw-private-attribute', rule, { + valid: [ + // ✅ The public theme and media scopes, in every shape. + {code: `const s = '[data-astryx-theme="dark"]';`, filename: site}, + {code: `const C = () =>
;`, filename: site}, + { + code: `const C = () =>
;`, + filename: site, + }, + {code: `const p = {'data-astryx-media': surface};`, filename: site}, + {code: 'const s = `[data-astryx-theme="${name}"]`;', filename: site}, + { + code: 'const s = `[data-${dataAttrNamespace}-media="${surface}"]`;', + filename: site, + }, + {code: `el.setAttribute(dataAttr('theme'), name);`, filename: site}, + {code: `const p = {[dataAttr('media')]: resolved};`, filename: site}, + // ✅ The rest of the public list INV23 names. + { + code: ` + const C = () => ( + + ); + const EDGE_COMP_ATTR = 'data-astryx-edge-comp'; + const s = ':has(> [data-astryx-edge-comp]:first-child)'; + `, + filename: site, + }, + // ✅ Other data attributes and prose that only mentions the prefix's parts. + { + code: `const C = () =>
;`, + filename: site, + }, + {code: `const s = '[data-astryx]';`, filename: site}, + // ✅ The owner module writes its own attribute: selector, key and setter. + { + code: ` + const styles = stylex.create({ + pressed: {transform: {default: null, '[data-astryx-press="on"]': 'scale(0.98)'}}, + ring: {[stylex.when.ancestor('[data-astryx-press="on"]', scope)]: {opacity: 1}}, + }); + export const pressedStyles = styles.pressed; + export const pressOn = {'data-astryx-press': 'on'}; + `, + filename: owner, + }, + // ✅ An owner module in another package writes its own attribute. + { + code: `export const syntaxOn = {'data-astryx-richtext-code-syntax': ''};`, + filename: + '/repo/packages/richtext/src/privateAttributes/richtextCodeSyntax.stylex.ts', + }, + // ✅ A camelCase owner module owns its kebab-case attribute. + { + code: ` + const FOCUS_OWNER = '[data-astryx-focus-owner]'; + export const focusOwnerProps = {'data-astryx-focus-owner': ''}; + export const closestFocusOwner = element => element.closest(FOCUS_OWNER); + `, + filename: + '/repo/packages/core/src/privateAttributes/focusOwner.stylex.ts', + }, + // ✅ A file imports its own package's owner module. + { + code: `import {pressOn} from '../privateAttributes/press.stylex';`, + filename: site, + }, + // ✅ A non-exported name inside the owner module. + { + code: `const ATTRIBUTE = 'data-astryx-press'; export function setPress(el) { el.setAttribute(ATTRIBUTE, 'on'); }`, + filename: owner, + }, + // ✅ Owner-module readers and writers take the name as a call argument, + // and setter props use it as a key, including through a constant. + { + code: ` + const ATTRIBUTE = 'data-astryx-press'; + const PRESSED = '[data-astryx-press="on"]'; + export const pressOn = {[ATTRIBUTE]: 'on'}; + export function isPressed(element) { return element.matches(PRESSED); } + export const closestPressed = element => element.closest(PRESSED); + export const isOn = element => element.matches(\`[\${ATTRIBUTE}="on"]\`); + export function setPress(element, value) { element.setAttribute(ATTRIBUTE, value); } + export const styles = stylex.create({base: {opacity: {default: 1, [PRESSED]: 0.8}}}); + `, + filename: owner, + }, + // ✅ naming.ts builds every data-astryx-* name. + { + code: 'export function dataAttr(name) { return `data-${dataAttrNamespace}-${name}`; }', + filename: '/repo/packages/core/src/naming.ts', + }, + // ✅ Baselined sites, once per site, in their own file. + { + code: ` + const s = '[data-astryx-press="on"]'; + const C = () =>
; + `, + filename: site, + options: baselined(['data-astryx-press', 'data-astryx-submenu-host']), + }, + { + code: `const a = '[data-astryx-press="on"]'; const b = '[data-astryx-press="fading"]';`, + filename: site, + options: baselined(['data-astryx-press', 'data-astryx-press']), + }, + { + code: 'const s = `data-astryx-${part}`;', + filename: site, + options: baselined(['data-astryx-*']), + }, + ], + invalid: [ + // ❌ A string literal. + { + code: `el.setAttribute('data-astryx-press', 'on');`, + filename: site, + errors: [ + { + message: + '`data-astryx-press` is a private data attribute. Write it only in its owner module, packages/core/src/privateAttributes/press.stylex.ts, and spread the styles, setter props and readers it exports (architecture:public-component-api INV23 lists the public attributes).', + }, + ], + }, + // ❌ A template literal, plain or built from the namespace constant. + { + code: 'const s = `[data-astryx-cell-gliding="${state}"]`;', + filename: site, + errors: [ + { + messageId: 'raw', + data: { + attribute: 'data-astryx-cell-gliding', + owner: 'packages/core/src/privateAttributes/cellGliding.stylex.ts', + }, + }, + ], + }, + { + code: 'const s = `[data-${dataAttrNamespace}-table-pinned]`;', + filename: site, + errors: [{messageId: 'raw'}], + }, + // ❌ A JSX attribute name. + { + code: `const C = () =>
;`, + filename: site, + errors: [{messageId: 'raw'}], + }, + // ❌ An object key, including a setter-props object. + { + code: `const props = {'data-astryx-input-group-item': ''};`, + filename: site, + errors: [{messageId: 'raw'}], + }, + // ❌ A selector inside stylex.create, as a key and through stylex.when. + { + code: `const styles = stylex.create({base: {transform: {default: null, '[data-astryx-press="on"]': 'scale(0.98)'}}});`, + filename: site, + errors: [{messageId: 'raw'}], + }, + { + code: `const styles = stylex.create({base: {[stylex.when.ancestor('[data-astryx-press="fading"]', scope)]: {opacity: 0}}});`, + filename: site, + errors: [{messageId: 'raw'}], + }, + // ❌ One selector naming two private attributes reports each. + { + code: `const s = ':has(~ [data-astryx-submenu-host] > [data-astryx-submenu-regions="view"])';`, + filename: site, + errors: [{messageId: 'raw'}, {messageId: 'raw'}], + }, + // ❌ A name that starts like a public one is still private. + { + code: `style.setAttribute('data-astryx-theme-prose', name);`, + filename: site, + errors: [ + { + messageId: 'raw', + data: { + attribute: 'data-astryx-theme-prose', + owner: 'packages/core/src/privateAttributes/themeProse.stylex.ts', + }, + }, + ], + }, + // ❌ dataAttr() naming a private attribute, or a name it cannot read. + { + code: `style.setAttribute(dataAttr('id'), id);`, + filename: site, + errors: [ + { + messageId: 'raw', + data: { + attribute: 'data-astryx-id', + owner: 'packages/core/src/privateAttributes/id.stylex.ts', + }, + }, + ], + }, + { + code: `style.setAttribute(dataAttr(name), id);`, + filename: site, + errors: [{messageId: 'unreadable'}], + }, + // ❌ An interpolated name, or the bare prefix. + { + code: 'const s = `data-astryx-${part}`;', + filename: site, + errors: [ + { + messageId: 'unreadable', + data: { + attribute: 'data-astryx-*', + ownerDirectory: 'packages/core/src/privateAttributes', + }, + }, + ], + }, + { + code: `const isOurs = name.startsWith('data-astryx-');`, + filename: site, + errors: [{messageId: 'unreadable'}], + }, + // ❌ A private attribute in Lab names Lab's own owner module. + { + code: `const s = '[data-astryx-popover-fallback-close]';`, + filename: '/repo/packages/lab/src/Schedule/useScheduleViewPopover.ts', + errors: [ + { + messageId: 'raw', + data: { + attribute: 'data-astryx-popover-fallback-close', + owner: + 'packages/lab/src/privateAttributes/popoverFallbackClose.stylex.ts', + }, + }, + ], + }, + // ❌ A new site beyond the baseline's count for that attribute. + { + code: `const a = '[data-astryx-press="on"]'; const b = '[data-astryx-press="fading"]';`, + filename: site, + options: baselined(['data-astryx-press']), + errors: [{messageId: 'raw'}], + }, + // ❌ A baseline for another file or another attribute does not cover it. + { + code: `const s = '[data-astryx-press="on"]';`, + filename: site, + options: [ + { + baseline: {'packages/core/src/Tab/Tab.tsx': ['data-astryx-press']}, + root: '/repo', + }, + ], + errors: [{messageId: 'raw'}], + }, + { + code: `const s = '[data-astryx-press="on"]';`, + filename: site, + options: baselined(['data-astryx-token']), + errors: [{messageId: 'raw'}], + }, + // ❌ An owner module writing another attribute. + { + code: `export const pinnedOn = {'data-astryx-table-pinned': ''};`, + filename: owner, + errors: [ + { + messageId: 'foreignOwner', + data: { + attribute: 'data-astryx-table-pinned', + owner: 'packages/core/src/privateAttributes/tablePinned.stylex.ts', + }, + }, + ], + }, + // ❌ A camelCase owner module writing an attribute it does not own. + { + code: `const s = '[data-astryx-focus-ring]';`, + filename: + '/repo/packages/core/src/privateAttributes/focusOwner.stylex.ts', + errors: [ + { + messageId: 'foreignOwner', + data: { + attribute: 'data-astryx-focus-ring', + owner: 'packages/core/src/privateAttributes/focusRing.stylex.ts', + }, + }, + ], + }, + // ❌ An owner module exporting its raw name or a selector on it. + { + code: `export const PRESS_ATTRIBUTE = 'data-astryx-press';`, + filename: owner, + errors: [{messageId: 'exportedName'}], + }, + { + code: `const PRESSED = '[data-astryx-press="on"]' as const; export {PRESSED};`, + filename: owner, + errors: [{messageId: 'exportedName'}], + }, + // ❌ The owner's name or a selector leaving the module as a string, by + // every route: default export, object value, array, return value, + // module constant, dataAttr(). + { + code: `export default 'data-astryx-press';`, + filename: owner, + errors: [{messageId: 'exportedName'}], + }, + { + code: `export const selectors = {pressed: '[data-astryx-press="on"]'};`, + filename: owner, + errors: [{messageId: 'exportedName'}], + }, + { + code: `export const names = ['data-astryx-press'];`, + filename: owner, + errors: [{messageId: 'exportedName'}], + }, + { + code: `export function pressSelector() { return '[data-astryx-press="on"]'; }`, + filename: owner, + errors: [{messageId: 'exportedName'}], + }, + { + code: `export const pressAttribute = () => 'data-astryx-press';`, + filename: owner, + errors: [{messageId: 'exportedName'}], + }, + { + code: `const PRESSED = '[data-astryx-press="on"]'; export function pressSelector() { return PRESSED; }`, + filename: owner, + errors: [{messageId: 'exportedName'}], + }, + { + code: `const ATTRIBUTE = 'data-astryx-press'; export default ATTRIBUTE;`, + filename: owner, + errors: [{messageId: 'exportedName'}], + }, + { + code: `export const PRESS_ATTRIBUTE = dataAttr('press');`, + filename: owner, + errors: [{messageId: 'exportedName'}], + }, + // ❌ A selector built from the name constant, in a template or with `+`. + { + code: 'const ATTRIBUTE = \'data-astryx-press\'; export const PRESSED_SELECTOR = `[${ATTRIBUTE}="on"]`;', + filename: owner, + errors: [{messageId: 'exportedName'}], + }, + { + code: 'const ATTRIBUTE = \'data-astryx-press\'; export function pressedSelector(state) { return `[${ATTRIBUTE}="${state}"]`; }', + filename: owner, + errors: [{messageId: 'exportedName'}], + }, + { + code: "const ATTRIBUTE = 'data-astryx-press'; export const PRESSED_SELECTOR = '[' + ATTRIBUTE + ']';", + filename: owner, + errors: [{messageId: 'exportedName'}], + }, + // ❌ Importing another package's owner module, by package path or by a + // relative path into its source. + { + code: `import {fallbackClose} from '@astryxdesign/core/privateAttributes/popover-fallback-close.stylex';`, + filename: '/repo/packages/lab/src/Schedule/useScheduleViewPopover.ts', + errors: [{messageId: 'foreignPackage'}], + }, + { + code: `import {fallbackClose} from '../../../core/src/privateAttributes/popover-fallback-close.stylex';`, + filename: '/repo/packages/lab/src/Schedule/useScheduleViewPopover.ts', + errors: [{messageId: 'foreignPackage'}], + }, + ], +}); diff --git a/internal/eslint-plugin-astryx/rule-baseline.mjs b/internal/eslint-plugin-astryx/rule-baseline.mjs index 0d2675ac2bc12..633bc8bdaa72a 100644 --- a/internal/eslint-plugin-astryx/rule-baseline.mjs +++ b/internal/eslint-plugin-astryx/rule-baseline.mjs @@ -6,13 +6,45 @@ * violations, per repo-relative file, that existed when the repository config * began enforcing it. The rule takes the baseline as `{baseline, root}` and * reports only what it does not list. Shared by each rule's - * `.baseline.mjs` so the write and `--check` flows stay one. + * `.baseline.mjs` so the write and `--check` flows stay one, and by the + * rules that spend a counted baseline (`baselineAllowance`). */ import fs from 'node:fs'; import path from 'node:path'; import {ESLint} from 'eslint'; +/** + * The allowance a baselined rule spends as it reports, for the file being + * linted. The rule's options are `{baseline, root}`: `baseline` maps a file, + * relative to `root` (the repository root; default: the directory ESLint runs + * from), to its entries. Each entry allows as many reports as it is listed; + * the next one is reported. Returns `isAllowed(entry)`, which spends one + * allowance when one is left. Shared by every rule that takes a counted + * baseline, so matching stays one implementation. + * + * @param {import('eslint').Rule.RuleContext} context + * @returns {(entry: string) => boolean} + */ +export function baselineAllowance(context) { + const options = context.options[0] ?? {}; + const root = options.root ?? context.cwd; + const filename = context.filename ?? context.getFilename(); + const relative = path.relative(root, filename).split(path.sep).join('/'); + const left = new Map(); + for (const entry of options.baseline?.[relative] ?? []) { + left.set(entry, (left.get(entry) ?? 0) + 1); + } + return entry => { + const count = left.get(entry) ?? 0; + if (count === 0) { + return false; + } + left.set(entry, count - 1); + return true; + }; +} + /** * Lint `targets` with the repository config and an empty baseline, so every * violation in `scope` is reported, then write or check `baselineFile`.