From f297ab528ebe416e5814ab17bf4b30dcafda7c69 Mon Sep 17 00:00:00 2001 From: Martin Hochel Date: Thu, 21 May 2026 14:33:35 +0200 Subject: [PATCH 01/18] feat(eslint-rules): add consistent-base-hook rule Enforces the v9 base hook API contract for any function matching `useBase_unstable`: - 1 or 2 positional parameters; first must be named `props`, optional second must be named `ref` and typed as `React.Ref<...>`. - No references to bindings imported from configured forbidden packages (defaults to `@fluentui/react-tabster`, `tabster`, `keyborg`). Forbidden-package detection is scope-based, so local shadowing is handled correctly. Options: { forbiddenPackages?: Array } --- tools/eslint-rules/index.ts | 2 + .../rules/consistent-base-hook.spec.ts | 248 ++++++++++++++ .../rules/consistent-base-hook.ts | 311 ++++++++++++++++++ 3 files changed, 561 insertions(+) create mode 100644 tools/eslint-rules/rules/consistent-base-hook.spec.ts create mode 100644 tools/eslint-rules/rules/consistent-base-hook.ts diff --git a/tools/eslint-rules/index.ts b/tools/eslint-rules/index.ts index 6aeaae6c06da8e..8e6f5eca37fd1f 100644 --- a/tools/eslint-rules/index.ts +++ b/tools/eslint-rules/index.ts @@ -4,6 +4,7 @@ import { RULE_NAME as consistentCallbackTypeName, rule as consistentCallbackType, } from './rules/consistent-callback-type'; +import { RULE_NAME as consistentBaseHookName, rule as consistentBaseHook } from './rules/consistent-base-hook'; /** * Import your custom workspace rules at the top of this file. @@ -32,6 +33,7 @@ module.exports = { */ rules: { [consistentCallbackTypeName]: consistentCallbackType, + [consistentBaseHookName]: consistentBaseHook, [noRestrictedGlobalsName]: noRestrictedGlobals, [noMissingJsxPragmaName]: noMissingJsxPragma, }, diff --git a/tools/eslint-rules/rules/consistent-base-hook.spec.ts b/tools/eslint-rules/rules/consistent-base-hook.spec.ts new file mode 100644 index 00000000000000..a07998f2005b50 --- /dev/null +++ b/tools/eslint-rules/rules/consistent-base-hook.spec.ts @@ -0,0 +1,248 @@ +import { RuleTester } from '@typescript-eslint/rule-tester'; +import { rule, RULE_NAME } from './consistent-base-hook'; + +const ruleTester = new RuleTester(); + +ruleTester.run(RULE_NAME, rule, { + valid: [ + // Valid base hook: 2 Identifier params, no forbidden imports. + { + code: ` + import * as React from 'react'; + export const useThingBase_unstable = (props, ref: React.Ref) => { + return { props, ref }; + }; + `, + }, + // Valid base hook declared as FunctionDeclaration. + { + code: ` + import { Ref } from 'react'; + export function useThingBase_unstable(props, ref: Ref) { + return { props, ref }; + } + `, + }, + // Valid base hook with only `props` (ref is optional). + { + code: ` + export const useThingBase_unstable = (props) => { + return { props }; + }; + `, + }, + // Forbidden import exists but is only used by a non-base hook in the same file. + { + code: ` + import { useArrowNavigationGroup } from '@fluentui/react-tabster'; + export const useThingBase_unstable = (props, ref: React.Ref) => { + return { props, ref }; + }; + export const useThing_unstable = (props, ref) => { + const nav = useArrowNavigationGroup({}); + return { ...useThingBase_unstable(props, ref), nav }; + }; + `, + }, + // Non-base hook is not subject to the param-shape constraint. + { + code: ` + export const useThing_unstable = (props, ref, extra) => { + return { props, ref, extra }; + }; + `, + }, + // Allowlist opt-out: a specific imported name is permitted inside base hooks. + { + code: ` + import { useFocusFinders } from '@fluentui/react-tabster'; + export const useThingBase_unstable = (props, ref: React.Ref) => { + const finders = useFocusFinders(); + return { props, ref, finders }; + }; + `, + options: [ + { + forbiddenPackages: [{ name: '@fluentui/react-tabster', allow: ['useFocusFinders'] }], + }, + ], + }, + // Identifier with the same local name as a forbidden import alias does not collide via scope analysis. + { + code: ` + import { useArrowNavigationGroup } from '@fluentui/react-tabster'; + export const useThing_unstable = (props, ref) => { + return useArrowNavigationGroup({}); + }; + export const useThingBase_unstable = (props, ref: React.Ref) => { + const useArrowNavigationGroup = () => 1; + return { value: useArrowNavigationGroup() }; + }; + `, + }, + ], + invalid: [ + // Too few params (0). + { + code: ` + export const useThingBase_unstable = () => ({}); + `, + errors: [{ messageId: 'invalidParamCount', data: { hookName: 'useThingBase_unstable', actual: 0 } }], + }, + // Too many params. + { + code: ` + export const useThingBase_unstable = (props, ref, extra) => ({ props, ref, extra }); + `, + errors: [{ messageId: 'invalidParamCount', data: { hookName: 'useThingBase_unstable', actual: 3 } }], + }, + // Wrong param names. + { + code: ` + export const useThingBase_unstable = (p, r) => ({ p, r }); + `, + errors: [ + { + messageId: 'invalidParamName', + data: { hookName: 'useThingBase_unstable', index: 1, expected: 'props', actual: 'p' }, + }, + { + messageId: 'invalidParamName', + data: { hookName: 'useThingBase_unstable', index: 2, expected: 'ref', actual: 'r' }, + }, + ], + }, + // ObjectPattern for `props` is not allowed. + { + code: ` + export const useThingBase_unstable = ({ a }, ref: React.Ref) => ({ a, ref }); + `, + errors: [ + { + messageId: 'invalidParamName', + data: { hookName: 'useThingBase_unstable', index: 1, expected: 'props', actual: '{ ... }' }, + }, + ], + }, + // `ref` parameter without a type annotation. + { + code: ` + export const useThingBase_unstable = (props, ref) => ({ props, ref }); + `, + errors: [ + { + messageId: 'invalidRefType', + data: { hookName: 'useThingBase_unstable', actual: '' }, + }, + ], + }, + // `ref` parameter typed as something other than React.Ref. + { + code: ` + export const useThingBase_unstable = (props, ref: HTMLElement) => ({ props, ref }); + `, + errors: [ + { + messageId: 'invalidRefType', + data: { hookName: 'useThingBase_unstable', actual: 'HTMLElement' }, + }, + ], + }, + // `ref` parameter typed as React.ForwardedRef (must be React.Ref). + { + code: ` + export const useThingBase_unstable = (props, ref: React.ForwardedRef) => ({ props, ref }); + `, + errors: [ + { + messageId: 'invalidRefType', + data: { hookName: 'useThingBase_unstable', actual: 'React.ForwardedRef' }, + }, + ], + }, + // Body uses a binding imported from @fluentui/react-tabster. + { + code: ` + import { useArrowNavigationGroup } from '@fluentui/react-tabster'; + export const useThingBase_unstable = (props, ref: React.Ref) => { + const nav = useArrowNavigationGroup({}); + return { props, ref, nav }; + }; + `, + errors: [ + { + messageId: 'forbiddenPackageUsage', + data: { + hookName: 'useThingBase_unstable', + importedName: 'useArrowNavigationGroup', + package: '@fluentui/react-tabster', + }, + }, + ], + }, + // Body uses a binding imported from tabster, even when aliased. + { + code: ` + import { getTabsterAttribute as gta } from 'tabster'; + export function useThingBase_unstable(props, ref: React.Ref) { + return { attr: gta({}) }; + } + `, + errors: [ + { + messageId: 'forbiddenPackageUsage', + data: { + hookName: 'useThingBase_unstable', + importedName: 'getTabsterAttribute', + package: 'tabster', + }, + }, + ], + }, + // Body uses a binding imported from keyborg. + { + code: ` + import { createKeyborg } from 'keyborg'; + export const useThingBase_unstable = (props, ref: React.Ref) => { + return { kb: createKeyborg(window) }; + }; + `, + errors: [ + { + messageId: 'forbiddenPackageUsage', + data: { + hookName: 'useThingBase_unstable', + importedName: 'createKeyborg', + package: 'keyborg', + }, + }, + ], + }, + // Allowlist excludes one name; siblings still error. + { + code: ` + import { useFocusFinders, useArrowNavigationGroup } from '@fluentui/react-tabster'; + export const useThingBase_unstable = (props, ref: React.Ref) => { + const finders = useFocusFinders(); + const nav = useArrowNavigationGroup({}); + return { props, ref, finders, nav }; + }; + `, + options: [ + { + forbiddenPackages: [{ name: '@fluentui/react-tabster', allow: ['useFocusFinders'] }], + }, + ], + errors: [ + { + messageId: 'forbiddenPackageUsage', + data: { + hookName: 'useThingBase_unstable', + importedName: 'useArrowNavigationGroup', + package: '@fluentui/react-tabster', + }, + }, + ], + }, + ], +}); diff --git a/tools/eslint-rules/rules/consistent-base-hook.ts b/tools/eslint-rules/rules/consistent-base-hook.ts new file mode 100644 index 00000000000000..76f0bb37733b6e --- /dev/null +++ b/tools/eslint-rules/rules/consistent-base-hook.ts @@ -0,0 +1,311 @@ +import { ESLintUtils, AST_NODE_TYPES, TSESTree, TSESLint } from '@typescript-eslint/utils'; + +// NOTE: The rule will be available in ESLint configs as "@nx/workspace-consistent-base-hook" +export const RULE_NAME = 'consistent-base-hook'; + +const BASE_HOOK_NAME_PATTERN = /^use[A-Z]\w*Base_unstable$/; +const EXPECTED_PARAM_NAMES = ['props', 'ref'] as const; +const MIN_PARAM_COUNT = 1; +const MAX_PARAM_COUNT = 2; +const DEFAULT_FORBIDDEN_PACKAGES: ReadonlyArray = ['@fluentui/react-tabster', 'tabster', 'keyborg']; + +type ForbiddenPackageOption = string | { name: string; allow?: string[] }; +type Options = [ + { + forbiddenPackages?: ForbiddenPackageOption[]; + }?, +]; +type MessageIds = 'invalidParamCount' | 'invalidParamName' | 'invalidRefType' | 'forbiddenPackageUsage'; + +interface NormalizedForbiddenPackage { + name: string; + allow: Set; +} + +interface ForbiddenImport { + package: string; + importedName: string; +} + +type BaseHookFunction = TSESTree.FunctionDeclaration | TSESTree.FunctionExpression | TSESTree.ArrowFunctionExpression; + +export const rule = ESLintUtils.RuleCreator(() => __filename)({ + name: RULE_NAME, + meta: { + type: 'problem', + docs: { + description: + 'Enforce the API contract for v9 "base" hooks (`useBase_unstable`): a required `props` parameter and an optional `ref` parameter typed as `React.Ref<...>`, and no usage of bindings from configured forbidden packages (defaults to `@fluentui/react-tabster`, `tabster`, `keyborg`).', + }, + schema: [ + { + type: 'object', + properties: { + forbiddenPackages: { + type: 'array', + items: { + oneOf: [ + { type: 'string' }, + { + type: 'object', + properties: { + name: { type: 'string' }, + allow: { + type: 'array', + items: { type: 'string' }, + uniqueItems: true, + }, + }, + required: ['name'], + additionalProperties: false, + }, + ], + }, + uniqueItems: false, + }, + }, + additionalProperties: false, + }, + ], + messages: { + invalidParamCount: + 'Base hook `{{hookName}}` must take 1 or 2 positional parameters (`props`, optional `ref`), got {{actual}}.', + invalidParamName: + 'Base hook `{{hookName}}` parameter #{{index}} must be named `{{expected}}` (Identifier), got `{{actual}}`.', + invalidRefType: 'Base hook `{{hookName}}` parameter `ref` must be typed as `React.Ref<...>`, got `{{actual}}`.', + forbiddenPackageUsage: + 'Base hook `{{hookName}}` cannot reference `{{importedName}}` from forbidden package `{{package}}`. Move logic that depends on `{{package}}` to the wrapping `*_unstable` hook instead.', + }, + }, + defaultOptions: [{}], + create(context) { + const sourceCode = context.sourceCode; + const options = context.options[0] ?? {}; + const forbiddenPackages = normalizeForbiddenPackages(options.forbiddenPackages); + const forbiddenPackageByName = new Map(forbiddenPackages.map(pkg => [pkg.name, pkg])); + + // Map from import Variable -> origin (package + original imported name). + // Keyed by Variable identity (not name) so shadowing inside a base hook is handled correctly. + const forbiddenImports = new Map(); + + function checkBaseHook(hookName: string, fn: BaseHookFunction, reportNode: TSESTree.Node): void { + checkParameters(hookName, fn, reportNode); + checkBodyReferences(hookName, fn); + } + + function checkParameters(hookName: string, fn: BaseHookFunction, reportNode: TSESTree.Node): void { + if (fn.params.length < MIN_PARAM_COUNT || fn.params.length > MAX_PARAM_COUNT) { + context.report({ + node: reportNode, + messageId: 'invalidParamCount', + data: { + hookName, + actual: fn.params.length, + }, + }); + return; + } + + fn.params.forEach((param, index) => { + const expected = EXPECTED_PARAM_NAMES[index]; + if (param.type !== AST_NODE_TYPES.Identifier || param.name !== expected) { + context.report({ + node: reportNode, + messageId: 'invalidParamName', + data: { + hookName, + index: index + 1, + expected, + actual: describeParam(param), + }, + }); + return; + } + if (index === 1 && !isReactRefTypeAnnotation(param.typeAnnotation)) { + context.report({ + node: reportNode, + messageId: 'invalidRefType', + data: { + hookName, + actual: describeRefType(param.typeAnnotation), + }, + }); + } + }); + } + + function checkBodyReferences(hookName: string, fn: BaseHookFunction): void { + if (forbiddenImports.size === 0) { + return; + } + + const fnScope = sourceCode.getScope(fn); + visitScope(fnScope, fn, hookName); + } + + function visitScope(scope: TSESLint.Scope.Scope, fn: BaseHookFunction, hookName: string): void { + // Only descend into scopes that belong to this base hook's function body. + if (!isScopeWithinFunction(scope, fn)) { + return; + } + + scope.references.forEach(reference => { + const resolved = reference.resolved; + if (!resolved) { + return; + } + const origin = forbiddenImports.get(resolved); + if (!origin) { + return; + } + const pkg = forbiddenPackageByName.get(origin.package); + if (pkg?.allow.has(origin.importedName)) { + return; + } + context.report({ + node: reference.identifier, + messageId: 'forbiddenPackageUsage', + data: { + hookName, + importedName: origin.importedName, + package: origin.package, + }, + }); + }); + + scope.childScopes.forEach(child => visitScope(child, fn, hookName)); + } + + return { + ImportDeclaration(node) { + const source = node.source.value; + if (typeof source !== 'string' || !forbiddenPackageByName.has(source)) { + return; + } + + node.specifiers.forEach(specifier => { + let importedName: string; + switch (specifier.type) { + case AST_NODE_TYPES.ImportSpecifier: + importedName = + specifier.imported.type === AST_NODE_TYPES.Identifier + ? specifier.imported.name + : String(specifier.imported.value); + break; + case AST_NODE_TYPES.ImportDefaultSpecifier: + importedName = 'default'; + break; + case AST_NODE_TYPES.ImportNamespaceSpecifier: + importedName = '*'; + break; + default: + return; + } + for (const variable of sourceCode.getDeclaredVariables(specifier)) { + forbiddenImports.set(variable, { package: source, importedName }); + } + }); + }, + + [`FunctionDeclaration[id.name=/${BASE_HOOK_NAME_PATTERN.source}/]`]: (node: TSESTree.FunctionDeclaration) => { + if (!node.id) { + return; + } + checkBaseHook(node.id.name, node, node.id); + }, + + [`VariableDeclarator[id.name=/${BASE_HOOK_NAME_PATTERN.source}/]`]: (node: TSESTree.VariableDeclarator) => { + if (node.id.type !== AST_NODE_TYPES.Identifier) { + return; + } + const init = node.init; + if ( + !init || + (init.type !== AST_NODE_TYPES.ArrowFunctionExpression && init.type !== AST_NODE_TYPES.FunctionExpression) + ) { + return; + } + checkBaseHook(node.id.name, init, node.id); + }, + }; + }, +}); + +function normalizeForbiddenPackages(raw: ForbiddenPackageOption[] | undefined): NormalizedForbiddenPackage[] { + const source = raw ?? DEFAULT_FORBIDDEN_PACKAGES; + return source.map(entry => { + if (typeof entry === 'string') { + return { name: entry, allow: new Set() }; + } + return { name: entry.name, allow: new Set(entry.allow ?? []) }; + }); +} + +function describeParam(param: TSESTree.Parameter): string { + switch (param.type) { + case AST_NODE_TYPES.Identifier: + return param.name; + case AST_NODE_TYPES.ObjectPattern: + return '{ ... }'; + case AST_NODE_TYPES.ArrayPattern: + return '[ ... ]'; + case AST_NODE_TYPES.RestElement: + return '...rest'; + case AST_NODE_TYPES.AssignmentPattern: + return param.left.type === AST_NODE_TYPES.Identifier ? `${param.left.name} = …` : '… = …'; + default: + return param.type; + } +} + +function isReactRefTypeAnnotation(annotation: TSESTree.TSTypeAnnotation | undefined): boolean { + if (!annotation) { + return false; + } + const type = annotation.typeAnnotation; + if (type.type !== AST_NODE_TYPES.TSTypeReference) { + return false; + } + const { typeName } = type; + if (typeName.type === AST_NODE_TYPES.Identifier) { + return typeName.name === 'Ref'; + } + if (typeName.type === AST_NODE_TYPES.TSQualifiedName) { + return ( + typeName.left.type === AST_NODE_TYPES.Identifier && + typeName.left.name === 'React' && + typeName.right.name === 'Ref' + ); + } + return false; +} + +function describeRefType(annotation: TSESTree.TSTypeAnnotation | undefined): string { + if (!annotation) { + return ''; + } + const type = annotation.typeAnnotation; + if (type.type !== AST_NODE_TYPES.TSTypeReference) { + return type.type; + } + const { typeName } = type; + if (typeName.type === AST_NODE_TYPES.Identifier) { + return typeName.name; + } + if (typeName.type === AST_NODE_TYPES.TSQualifiedName) { + const left = typeName.left.type === AST_NODE_TYPES.Identifier ? typeName.left.name : '…'; + return `${left}.${typeName.right.name}`; + } + return type.type; +} + +function isScopeWithinFunction(scope: TSESLint.Scope.Scope, fn: BaseHookFunction): boolean { + let current: TSESLint.Scope.Scope | null = scope; + while (current) { + if (current.block === fn) { + return true; + } + current = current.upper; + } + return false; +} From 2ca0c4200b372334bd5a4608f7ffdd2a4b583a50 Mon Sep 17 00:00:00 2001 From: Martin Hochel Date: Thu, 21 May 2026 14:33:42 +0200 Subject: [PATCH 02/18] chore(eslint-plugin): enable consistent-base-hook for v9 sources --- packages/eslint-plugin/src/internal.js | 1 + 1 file changed, 1 insertion(+) diff --git a/packages/eslint-plugin/src/internal.js b/packages/eslint-plugin/src/internal.js index 02c31f5aa97d30..19afbe17a62174 100644 --- a/packages/eslint-plugin/src/internal.js +++ b/packages/eslint-plugin/src/internal.js @@ -35,6 +35,7 @@ const __internal = { /** @type {import('eslint').Linter.RulesRecord} */ rules: { '@nx/workspace-consistent-callback-type': 'error', + '@nx/workspace-consistent-base-hook': 'error', '@nx/workspace-no-restricted-globals': restrictedGlobals.react, '@nx/workspace-no-missing-jsx-pragma': ['error', { runtime: 'automatic' }], }, From 2c2c0e98651421edaa25624dc3c202a0173cabd5 Mon Sep 17 00:00:00 2001 From: Martin Hochel Date: Thu, 21 May 2026 14:33:53 +0200 Subject: [PATCH 03/18] chore(react-components): suppress consistent-base-hook violations in legacy base hooks Pre-existing base hooks that don't yet conform to the new contract (missing/non-React.Ref-typed `ref` param, or references to tabster/keyborg bindings) are suppressed with line-scoped `eslint-disable-next-line` comments so the rule can ship as `error` for new code. Refactors to remove the suppressions will follow. Affected packages: - react-checkbox, react-menu, react-radio, react-rating, react-slider, react-switch, react-tags, react-tooltip --- .../library/src/components/Checkbox/useCheckbox.tsx | 1 + .../react-radio/library/src/components/Radio/useRadio.tsx | 1 + .../library/src/components/RatingItem/useRatingItem.tsx | 1 + .../react-slider/library/src/components/Slider/useSlider.ts | 1 + .../library/src/components/Switch/useSwitch.tsx | 1 + .../library/src/components/Tooltip/useTooltipBase.tsx | 6 +++++- 6 files changed, 10 insertions(+), 1 deletion(-) diff --git a/packages/react-components/react-checkbox/library/src/components/Checkbox/useCheckbox.tsx b/packages/react-components/react-checkbox/library/src/components/Checkbox/useCheckbox.tsx index 7a89ae277ab07c..14dc123d449de8 100644 --- a/packages/react-components/react-checkbox/library/src/components/Checkbox/useCheckbox.tsx +++ b/packages/react-components/react-checkbox/library/src/components/Checkbox/useCheckbox.tsx @@ -119,6 +119,7 @@ export const useCheckboxBase_unstable = ( }, root: slot.always(props.root, { defaultProps: { + // eslint-disable-next-line @nx/workspace-consistent-base-hook -- legacy: tabster usage should be moved out of base hook ref: useFocusWithin(), ...nativeProps.root, }, diff --git a/packages/react-components/react-radio/library/src/components/Radio/useRadio.tsx b/packages/react-components/react-radio/library/src/components/Radio/useRadio.tsx index e8d8cda21c01c4..6c2af214133497 100644 --- a/packages/react-components/react-radio/library/src/components/Radio/useRadio.tsx +++ b/packages/react-components/react-radio/library/src/components/Radio/useRadio.tsx @@ -61,6 +61,7 @@ export const useRadioBase_unstable = (props: RadioBaseProps, ref: React.Ref(), ...nativeProps.root, }, diff --git a/packages/react-components/react-rating/library/src/components/RatingItem/useRatingItem.tsx b/packages/react-components/react-rating/library/src/components/RatingItem/useRatingItem.tsx index d67d28275be561..447bfa1e6179c9 100644 --- a/packages/react-components/react-rating/library/src/components/RatingItem/useRatingItem.tsx +++ b/packages/react-components/react-rating/library/src/components/RatingItem/useRatingItem.tsx @@ -61,6 +61,7 @@ export const useRatingItemBase_unstable = ( const root = slot.always( getIntrinsicElementProps('span', { + // eslint-disable-next-line @nx/workspace-consistent-base-hook -- legacy: tabster usage should be moved out of base hook ref: useMergedRefs(useFocusWithin(), ref), ...props, }), diff --git a/packages/react-components/react-slider/library/src/components/Slider/useSlider.ts b/packages/react-components/react-slider/library/src/components/Slider/useSlider.ts index bdd105f21762c4..652b1d6c839947 100644 --- a/packages/react-components/react-slider/library/src/components/Slider/useSlider.ts +++ b/packages/react-components/react-slider/library/src/components/Slider/useSlider.ts @@ -72,6 +72,7 @@ export const useSliderBase_unstable = (props: SliderBaseProps, ref: React.Ref()); useSliderState_unstable(state, props); diff --git a/packages/react-components/react-switch/library/src/components/Switch/useSwitch.tsx b/packages/react-components/react-switch/library/src/components/Switch/useSwitch.tsx index 79fc3262fdcb73..667d7107b1227c 100644 --- a/packages/react-components/react-switch/library/src/components/Switch/useSwitch.tsx +++ b/packages/react-components/react-switch/library/src/components/Switch/useSwitch.tsx @@ -68,6 +68,7 @@ export const useSwitchBase_unstable = (props: SwitchBaseProps, ref?: React.Ref(), ...nativeProps.root }, elementType: 'div', }); diff --git a/packages/react-components/react-tooltip/library/src/components/Tooltip/useTooltipBase.tsx b/packages/react-components/react-tooltip/library/src/components/Tooltip/useTooltipBase.tsx index 4919ea58156ac3..9c7f99dd235165 100644 --- a/packages/react-components/react-tooltip/library/src/components/Tooltip/useTooltipBase.tsx +++ b/packages/react-components/react-tooltip/library/src/components/Tooltip/useTooltipBase.tsx @@ -35,7 +35,7 @@ import { Escape } from '@fluentui/keyboard-keys'; * @param props - props from this instance of Tooltip */ export const useTooltipBase_unstable = (props: TooltipBaseProps): TooltipBaseState => { - 'use no memo'; + ('use no memo'); const context = useTooltipVisibility(); const isServerSideRender = useIsSSR(); @@ -197,10 +197,12 @@ export const useTooltipBase_unstable = (props: TooltipBaseProps): TooltipBaseSta [setDelayTimeout, setVisible, state.showDelay, context], ); + // eslint-disable-next-line @nx/workspace-consistent-base-hook -- legacy: tabster usage should be moved out of base hook const isNavigatingWithKeyboard = useIsNavigatingWithKeyboard(); // Callback ref that attaches a keyborg:focusin event listener. const [keyborgListenerCallbackRef] = React.useState(() => { + // eslint-disable-next-line @nx/workspace-consistent-base-hook -- legacy: keyborg/tabster types used in base hook const onKeyborgFocusIn = ((ev: KeyborgFocusInEvent) => { // Skip showing the tooltip if focus moved programmatically. // For example, we don't want to show the tooltip when a dialog is closed @@ -216,7 +218,9 @@ export const useTooltipBase_unstable = (props: TooltipBaseProps): TooltipBaseSta // Callback ref that attaches the listener to the element return (element: Element | null) => { + // eslint-disable-next-line @nx/workspace-consistent-base-hook -- legacy: keyborg constant used in base hook current?.removeEventListener(KEYBORG_FOCUSIN, onKeyborgFocusIn); + // eslint-disable-next-line @nx/workspace-consistent-base-hook -- legacy: keyborg constant used in base hook element?.addEventListener(KEYBORG_FOCUSIN, onKeyborgFocusIn); current = element; }; From cbbd8c51453731cb0408337905d26c29f8e0dd5f Mon Sep 17 00:00:00 2001 From: Martin Hochel Date: Thu, 21 May 2026 15:42:54 +0200 Subject: [PATCH 04/18] feat(eslint-rules): allow useFocusWithin by default in consistent-base-hook `useFocusWithin` from `@fluentui/react-tabster` is a pure ref-attaching utility with no global side effects; it's safe to use inside base hooks. Added to the default allowlist so callers don't need to opt-out per-site. --- tools/eslint-rules/rules/consistent-base-hook.spec.ts | 9 +++++++++ tools/eslint-rules/rules/consistent-base-hook.ts | 6 +++++- 2 files changed, 14 insertions(+), 1 deletion(-) diff --git a/tools/eslint-rules/rules/consistent-base-hook.spec.ts b/tools/eslint-rules/rules/consistent-base-hook.spec.ts index a07998f2005b50..a9a6fcc1661751 100644 --- a/tools/eslint-rules/rules/consistent-base-hook.spec.ts +++ b/tools/eslint-rules/rules/consistent-base-hook.spec.ts @@ -67,6 +67,15 @@ ruleTester.run(RULE_NAME, rule, { }, ], }, + // Default allowlist: `useFocusWithin` from `@fluentui/react-tabster` is permitted inside base hooks. + { + code: ` + import { useFocusWithin } from '@fluentui/react-tabster'; + export const useThingBase_unstable = (props, ref: React.Ref) => { + return { props, ref: useFocusWithin() }; + }; + `, + }, // Identifier with the same local name as a forbidden import alias does not collide via scope analysis. { code: ` diff --git a/tools/eslint-rules/rules/consistent-base-hook.ts b/tools/eslint-rules/rules/consistent-base-hook.ts index 76f0bb37733b6e..0fa5fd2ec5a84e 100644 --- a/tools/eslint-rules/rules/consistent-base-hook.ts +++ b/tools/eslint-rules/rules/consistent-base-hook.ts @@ -7,7 +7,11 @@ const BASE_HOOK_NAME_PATTERN = /^use[A-Z]\w*Base_unstable$/; const EXPECTED_PARAM_NAMES = ['props', 'ref'] as const; const MIN_PARAM_COUNT = 1; const MAX_PARAM_COUNT = 2; -const DEFAULT_FORBIDDEN_PACKAGES: ReadonlyArray = ['@fluentui/react-tabster', 'tabster', 'keyborg']; +const DEFAULT_FORBIDDEN_PACKAGES: ReadonlyArray = [ + { name: '@fluentui/react-tabster', allow: ['useFocusWithin'] }, + 'tabster', + 'keyborg', +]; type ForbiddenPackageOption = string | { name: string; allow?: string[] }; type Options = [ From a8ea39b3df06077ceab9a166eec3a1d9863fdb71 Mon Sep 17 00:00:00 2001 From: Martin Hochel Date: Thu, 21 May 2026 15:43:02 +0200 Subject: [PATCH 05/18] chore(react-components): drop now-unneeded consistent-base-hook suppressions `useFocusWithin` is now part of the rule's default allowlist, so the line-scoped suppressions in react-checkbox, react-radio, react-rating, react-slider, and react-switch are no longer needed. --- .../library/src/components/Checkbox/useCheckbox.tsx | 1 - .../react-radio/library/src/components/Radio/useRadio.tsx | 1 - .../library/src/components/RatingItem/useRatingItem.tsx | 1 - .../react-slider/library/src/components/Slider/useSlider.ts | 1 - .../react-switch/library/src/components/Switch/useSwitch.tsx | 1 - 5 files changed, 5 deletions(-) diff --git a/packages/react-components/react-checkbox/library/src/components/Checkbox/useCheckbox.tsx b/packages/react-components/react-checkbox/library/src/components/Checkbox/useCheckbox.tsx index 14dc123d449de8..7a89ae277ab07c 100644 --- a/packages/react-components/react-checkbox/library/src/components/Checkbox/useCheckbox.tsx +++ b/packages/react-components/react-checkbox/library/src/components/Checkbox/useCheckbox.tsx @@ -119,7 +119,6 @@ export const useCheckboxBase_unstable = ( }, root: slot.always(props.root, { defaultProps: { - // eslint-disable-next-line @nx/workspace-consistent-base-hook -- legacy: tabster usage should be moved out of base hook ref: useFocusWithin(), ...nativeProps.root, }, diff --git a/packages/react-components/react-radio/library/src/components/Radio/useRadio.tsx b/packages/react-components/react-radio/library/src/components/Radio/useRadio.tsx index 6c2af214133497..e8d8cda21c01c4 100644 --- a/packages/react-components/react-radio/library/src/components/Radio/useRadio.tsx +++ b/packages/react-components/react-radio/library/src/components/Radio/useRadio.tsx @@ -61,7 +61,6 @@ export const useRadioBase_unstable = (props: RadioBaseProps, ref: React.Ref(), ...nativeProps.root, }, diff --git a/packages/react-components/react-rating/library/src/components/RatingItem/useRatingItem.tsx b/packages/react-components/react-rating/library/src/components/RatingItem/useRatingItem.tsx index 447bfa1e6179c9..d67d28275be561 100644 --- a/packages/react-components/react-rating/library/src/components/RatingItem/useRatingItem.tsx +++ b/packages/react-components/react-rating/library/src/components/RatingItem/useRatingItem.tsx @@ -61,7 +61,6 @@ export const useRatingItemBase_unstable = ( const root = slot.always( getIntrinsicElementProps('span', { - // eslint-disable-next-line @nx/workspace-consistent-base-hook -- legacy: tabster usage should be moved out of base hook ref: useMergedRefs(useFocusWithin(), ref), ...props, }), diff --git a/packages/react-components/react-slider/library/src/components/Slider/useSlider.ts b/packages/react-components/react-slider/library/src/components/Slider/useSlider.ts index 652b1d6c839947..bdd105f21762c4 100644 --- a/packages/react-components/react-slider/library/src/components/Slider/useSlider.ts +++ b/packages/react-components/react-slider/library/src/components/Slider/useSlider.ts @@ -72,7 +72,6 @@ export const useSliderBase_unstable = (props: SliderBaseProps, ref: React.Ref()); useSliderState_unstable(state, props); diff --git a/packages/react-components/react-switch/library/src/components/Switch/useSwitch.tsx b/packages/react-components/react-switch/library/src/components/Switch/useSwitch.tsx index 667d7107b1227c..79fc3262fdcb73 100644 --- a/packages/react-components/react-switch/library/src/components/Switch/useSwitch.tsx +++ b/packages/react-components/react-switch/library/src/components/Switch/useSwitch.tsx @@ -68,7 +68,6 @@ export const useSwitchBase_unstable = (props: SwitchBaseProps, ref?: React.Ref(), ...nativeProps.root }, elementType: 'div', }); From 153a6360caba8c1bafe1477012c26fe4491554bc Mon Sep 17 00:00:00 2001 From: Martin Hochel Date: Thu, 21 May 2026 15:50:32 +0200 Subject: [PATCH 06/18] feat(eslint-rules): allow useFocusVisible by default in consistent-base-hook Same rationale as `useFocusWithin`: `useFocusVisible` from `@fluentui/react-tabster` is safe to use inside base hooks. --- tools/eslint-rules/rules/consistent-base-hook.spec.ts | 5 +++-- tools/eslint-rules/rules/consistent-base-hook.ts | 2 +- 2 files changed, 4 insertions(+), 3 deletions(-) diff --git a/tools/eslint-rules/rules/consistent-base-hook.spec.ts b/tools/eslint-rules/rules/consistent-base-hook.spec.ts index a9a6fcc1661751..fb6b4318d49f8e 100644 --- a/tools/eslint-rules/rules/consistent-base-hook.spec.ts +++ b/tools/eslint-rules/rules/consistent-base-hook.spec.ts @@ -67,11 +67,12 @@ ruleTester.run(RULE_NAME, rule, { }, ], }, - // Default allowlist: `useFocusWithin` from `@fluentui/react-tabster` is permitted inside base hooks. + // Default allowlist: `useFocusWithin` and `useFocusVisible` from `@fluentui/react-tabster` are permitted inside base hooks. { code: ` - import { useFocusWithin } from '@fluentui/react-tabster'; + import { useFocusWithin, useFocusVisible } from '@fluentui/react-tabster'; export const useThingBase_unstable = (props, ref: React.Ref) => { + useFocusVisible(); return { props, ref: useFocusWithin() }; }; `, diff --git a/tools/eslint-rules/rules/consistent-base-hook.ts b/tools/eslint-rules/rules/consistent-base-hook.ts index 0fa5fd2ec5a84e..ae44534fc663be 100644 --- a/tools/eslint-rules/rules/consistent-base-hook.ts +++ b/tools/eslint-rules/rules/consistent-base-hook.ts @@ -8,7 +8,7 @@ const EXPECTED_PARAM_NAMES = ['props', 'ref'] as const; const MIN_PARAM_COUNT = 1; const MAX_PARAM_COUNT = 2; const DEFAULT_FORBIDDEN_PACKAGES: ReadonlyArray = [ - { name: '@fluentui/react-tabster', allow: ['useFocusWithin'] }, + { name: '@fluentui/react-tabster', allow: ['useFocusWithin', 'useFocusVisible'] }, 'tabster', 'keyborg', ]; From 3504c3a293c1f8b3b808cf9171b325daa369d7a3 Mon Sep 17 00:00:00 2001 From: Martin Hochel Date: Thu, 21 May 2026 19:21:53 +0200 Subject: [PATCH 07/18] feat(eslint-rules): allow keyborg-only react-tabster APIs and drop keyborg from forbidden list - Remove `keyborg` from the default forbidden packages list. Base hooks may freely depend on `keyborg` since it carries no tabster runtime. - Expand the default allowlist for `@fluentui/react-tabster` with APIs that internally depend only on `keyborg` (no `tabster` runtime): - useKeyboardNavAttribute - useIsNavigatingWithKeyboard - useSetKeyboardNavigation - useOnKeyboardNavigationChange - applyFocusVisiblePolyfill - KEYBORG_FOCUSIN / KeyborgFocusInEvent (re-exports from `keyborg`) --- .../rules/consistent-base-hook.spec.ts | 28 ++++++------------- .../rules/consistent-base-hook.ts | 18 ++++++++++-- 2 files changed, 25 insertions(+), 21 deletions(-) diff --git a/tools/eslint-rules/rules/consistent-base-hook.spec.ts b/tools/eslint-rules/rules/consistent-base-hook.spec.ts index fb6b4318d49f8e..c15112c95fb9fb 100644 --- a/tools/eslint-rules/rules/consistent-base-hook.spec.ts +++ b/tools/eslint-rules/rules/consistent-base-hook.spec.ts @@ -77,6 +77,15 @@ ruleTester.run(RULE_NAME, rule, { }; `, }, + // `keyborg` is not forbidden by default — bindings imported from it are allowed inside base hooks. + { + code: ` + import { createKeyborg, KEYBORG_FOCUSIN } from 'keyborg'; + export const useThingBase_unstable = (props, ref: React.Ref) => { + return { kb: createKeyborg(window), evt: KEYBORG_FOCUSIN }; + }; + `, + }, // Identifier with the same local name as a forbidden import alias does not collide via scope analysis. { code: ` @@ -209,25 +218,6 @@ ruleTester.run(RULE_NAME, rule, { }, ], }, - // Body uses a binding imported from keyborg. - { - code: ` - import { createKeyborg } from 'keyborg'; - export const useThingBase_unstable = (props, ref: React.Ref) => { - return { kb: createKeyborg(window) }; - }; - `, - errors: [ - { - messageId: 'forbiddenPackageUsage', - data: { - hookName: 'useThingBase_unstable', - importedName: 'createKeyborg', - package: 'keyborg', - }, - }, - ], - }, // Allowlist excludes one name; siblings still error. { code: ` diff --git a/tools/eslint-rules/rules/consistent-base-hook.ts b/tools/eslint-rules/rules/consistent-base-hook.ts index ae44534fc663be..55dde83ad1acb2 100644 --- a/tools/eslint-rules/rules/consistent-base-hook.ts +++ b/tools/eslint-rules/rules/consistent-base-hook.ts @@ -8,9 +8,23 @@ const EXPECTED_PARAM_NAMES = ['props', 'ref'] as const; const MIN_PARAM_COUNT = 1; const MAX_PARAM_COUNT = 2; const DEFAULT_FORBIDDEN_PACKAGES: ReadonlyArray = [ - { name: '@fluentui/react-tabster', allow: ['useFocusWithin', 'useFocusVisible'] }, + { + name: '@fluentui/react-tabster', + // APIs that only depend on `keyborg` (no `tabster` runtime) are safe to use inside base hooks. + allow: [ + 'useFocusWithin', + 'useFocusVisible', + 'useKeyboardNavAttribute', + 'useIsNavigatingWithKeyboard', + 'useSetKeyboardNavigation', + 'useOnKeyboardNavigationChange', + 'applyFocusVisiblePolyfill', + // re-exports from `keyborg` + 'KEYBORG_FOCUSIN', + 'KeyborgFocusInEvent', + ], + }, 'tabster', - 'keyborg', ]; type ForbiddenPackageOption = string | { name: string; allow?: string[] }; From 578c7daea4b38690e589c29c2072c4aaf691dbcc Mon Sep 17 00:00:00 2001 From: Martin Hochel Date: Thu, 21 May 2026 19:21:55 +0200 Subject: [PATCH 08/18] chore(react-tooltip): drop now-unneeded consistent-base-hook suppressions `useIsNavigatingWithKeyboard`, `KEYBORG_FOCUSIN`, and `KeyborgFocusInEvent` are now part of the rule's default allowlist. --- .../library/src/components/Tooltip/useTooltipBase.tsx | 4 ---- 1 file changed, 4 deletions(-) diff --git a/packages/react-components/react-tooltip/library/src/components/Tooltip/useTooltipBase.tsx b/packages/react-components/react-tooltip/library/src/components/Tooltip/useTooltipBase.tsx index 9c7f99dd235165..30d586b5e79b43 100644 --- a/packages/react-components/react-tooltip/library/src/components/Tooltip/useTooltipBase.tsx +++ b/packages/react-components/react-tooltip/library/src/components/Tooltip/useTooltipBase.tsx @@ -197,12 +197,10 @@ export const useTooltipBase_unstable = (props: TooltipBaseProps): TooltipBaseSta [setDelayTimeout, setVisible, state.showDelay, context], ); - // eslint-disable-next-line @nx/workspace-consistent-base-hook -- legacy: tabster usage should be moved out of base hook const isNavigatingWithKeyboard = useIsNavigatingWithKeyboard(); // Callback ref that attaches a keyborg:focusin event listener. const [keyborgListenerCallbackRef] = React.useState(() => { - // eslint-disable-next-line @nx/workspace-consistent-base-hook -- legacy: keyborg/tabster types used in base hook const onKeyborgFocusIn = ((ev: KeyborgFocusInEvent) => { // Skip showing the tooltip if focus moved programmatically. // For example, we don't want to show the tooltip when a dialog is closed @@ -218,9 +216,7 @@ export const useTooltipBase_unstable = (props: TooltipBaseProps): TooltipBaseSta // Callback ref that attaches the listener to the element return (element: Element | null) => { - // eslint-disable-next-line @nx/workspace-consistent-base-hook -- legacy: keyborg constant used in base hook current?.removeEventListener(KEYBORG_FOCUSIN, onKeyborgFocusIn); - // eslint-disable-next-line @nx/workspace-consistent-base-hook -- legacy: keyborg constant used in base hook element?.addEventListener(KEYBORG_FOCUSIN, onKeyborgFocusIn); current = element; }; From 338183aabb2608d6996ec4b57818fbdb51248063 Mon Sep 17 00:00:00 2001 From: Martin Hochel Date: Mon, 25 May 2026 10:42:04 +0200 Subject: [PATCH 09/18] feat(eslint-rules): auto-detect forbidden runtime deps via TS Program in consistent-base-hook Replace the hand-curated allow/forbidden package lists with per-symbol transitive value-import analysis powered by typescript-eslint ParserServices and the TypeScript Program. The rule now traces each base-hook import back to its defining source file and walks non-type-only re-exports/imports to detect whether the symbol's module graph reaches any configured forbidden runtime package. Options shape: { watchedPackages?: string[]; forbiddenRuntimes?: string[] }. Defaults: watchedPackages=['@fluentui/react-tabster'], forbiddenRuntimes=['tabster']. New message ids: forbiddenRuntimeDirect, forbiddenRuntimeReach. Type-only imports are skipped. When typed services are unavailable, only direct forbidden-runtime imports are reported. --- .../consistent-base-hook/src/dummy.ts | 4 + .../consistent-base-hook/src/test.ts | 4 + .../stubs/cyclic-pkg/a.ts | 5 + .../stubs/cyclic-pkg/b.ts | 6 + .../stubs/cyclic-pkg/index.ts | 3 + .../stubs/heavy-runtime/index.ts | 3 + .../stubs/light-helper/index.ts | 3 + .../stubs/watched-pkg/heavy.ts | 7 + .../stubs/watched-pkg/index.ts | 5 + .../stubs/watched-pkg/light.ts | 7 + .../consistent-base-hook/tsconfig.json | 21 + .../rules/consistent-base-hook.spec.ts | 232 +++++---- .../rules/consistent-base-hook.ts | 461 ++++++++++++++---- 13 files changed, 585 insertions(+), 176 deletions(-) create mode 100644 tools/eslint-rules/rules/__fixtures__/consistent-base-hook/src/dummy.ts create mode 100644 tools/eslint-rules/rules/__fixtures__/consistent-base-hook/src/test.ts create mode 100644 tools/eslint-rules/rules/__fixtures__/consistent-base-hook/stubs/cyclic-pkg/a.ts create mode 100644 tools/eslint-rules/rules/__fixtures__/consistent-base-hook/stubs/cyclic-pkg/b.ts create mode 100644 tools/eslint-rules/rules/__fixtures__/consistent-base-hook/stubs/cyclic-pkg/index.ts create mode 100644 tools/eslint-rules/rules/__fixtures__/consistent-base-hook/stubs/heavy-runtime/index.ts create mode 100644 tools/eslint-rules/rules/__fixtures__/consistent-base-hook/stubs/light-helper/index.ts create mode 100644 tools/eslint-rules/rules/__fixtures__/consistent-base-hook/stubs/watched-pkg/heavy.ts create mode 100644 tools/eslint-rules/rules/__fixtures__/consistent-base-hook/stubs/watched-pkg/index.ts create mode 100644 tools/eslint-rules/rules/__fixtures__/consistent-base-hook/stubs/watched-pkg/light.ts create mode 100644 tools/eslint-rules/rules/__fixtures__/consistent-base-hook/tsconfig.json diff --git a/tools/eslint-rules/rules/__fixtures__/consistent-base-hook/src/dummy.ts b/tools/eslint-rules/rules/__fixtures__/consistent-base-hook/src/dummy.ts new file mode 100644 index 00000000000000..73f11d7b5c58a4 --- /dev/null +++ b/tools/eslint-rules/rules/__fixtures__/consistent-base-hook/src/dummy.ts @@ -0,0 +1,4 @@ +// Placeholder so the tsconfig has an actual file to anchor the project. +// RuleTester test cases reference filenames inside this directory but their +// content comes from the inline `code` field. +export const dummy = 1; diff --git a/tools/eslint-rules/rules/__fixtures__/consistent-base-hook/src/test.ts b/tools/eslint-rules/rules/__fixtures__/consistent-base-hook/src/test.ts new file mode 100644 index 00000000000000..1e866fcbc83226 --- /dev/null +++ b/tools/eslint-rules/rules/__fixtures__/consistent-base-hook/src/test.ts @@ -0,0 +1,4 @@ +// Placeholder anchor for typed RuleTester cases. The actual code being linted +// comes from each test's inline `code` field; this file just needs to exist so +// that the fixture tsconfig's Program contains the filename used by tests. +export {}; diff --git a/tools/eslint-rules/rules/__fixtures__/consistent-base-hook/stubs/cyclic-pkg/a.ts b/tools/eslint-rules/rules/__fixtures__/consistent-base-hook/stubs/cyclic-pkg/a.ts new file mode 100644 index 00000000000000..12e940cc7f1fde --- /dev/null +++ b/tools/eslint-rules/rules/__fixtures__/consistent-base-hook/stubs/cyclic-pkg/a.ts @@ -0,0 +1,5 @@ +import { useB } from './b'; + +export function useA(): number { + return useB() + 1; +} diff --git a/tools/eslint-rules/rules/__fixtures__/consistent-base-hook/stubs/cyclic-pkg/b.ts b/tools/eslint-rules/rules/__fixtures__/consistent-base-hook/stubs/cyclic-pkg/b.ts new file mode 100644 index 00000000000000..777b389e45df54 --- /dev/null +++ b/tools/eslint-rules/rules/__fixtures__/consistent-base-hook/stubs/cyclic-pkg/b.ts @@ -0,0 +1,6 @@ +import { useA } from './a'; + +export function useB(): number { + // Pretend lazy ref to break true cycle at runtime; the static graph is cyclic. + return (useA as unknown as () => number).length; +} diff --git a/tools/eslint-rules/rules/__fixtures__/consistent-base-hook/stubs/cyclic-pkg/index.ts b/tools/eslint-rules/rules/__fixtures__/consistent-base-hook/stubs/cyclic-pkg/index.ts new file mode 100644 index 00000000000000..81c4ddc44291b1 --- /dev/null +++ b/tools/eslint-rules/rules/__fixtures__/consistent-base-hook/stubs/cyclic-pkg/index.ts @@ -0,0 +1,3 @@ +// Cyclic re-export — exercises the cycle-safety of the transitive walk. +export { useA } from './a'; +export { useB } from './b'; diff --git a/tools/eslint-rules/rules/__fixtures__/consistent-base-hook/stubs/heavy-runtime/index.ts b/tools/eslint-rules/rules/__fixtures__/consistent-base-hook/stubs/heavy-runtime/index.ts new file mode 100644 index 00000000000000..5fc7a81d62453b --- /dev/null +++ b/tools/eslint-rules/rules/__fixtures__/consistent-base-hook/stubs/heavy-runtime/index.ts @@ -0,0 +1,3 @@ +export function runHeavy(): { tag: 'heavy' } { + return { tag: 'heavy' }; +} diff --git a/tools/eslint-rules/rules/__fixtures__/consistent-base-hook/stubs/light-helper/index.ts b/tools/eslint-rules/rules/__fixtures__/consistent-base-hook/stubs/light-helper/index.ts new file mode 100644 index 00000000000000..9ab573b97e25e7 --- /dev/null +++ b/tools/eslint-rules/rules/__fixtures__/consistent-base-hook/stubs/light-helper/index.ts @@ -0,0 +1,3 @@ +export function runLight(_opts?: { mode: 'light' }): void { + /* noop */ +} diff --git a/tools/eslint-rules/rules/__fixtures__/consistent-base-hook/stubs/watched-pkg/heavy.ts b/tools/eslint-rules/rules/__fixtures__/consistent-base-hook/stubs/watched-pkg/heavy.ts new file mode 100644 index 00000000000000..9798a4b8d6b35e --- /dev/null +++ b/tools/eslint-rules/rules/__fixtures__/consistent-base-hook/stubs/watched-pkg/heavy.ts @@ -0,0 +1,7 @@ +import { runHeavy } from 'heavy-runtime'; + +export type HeavyType = { tag: 'heavy' }; + +export function useHeavy(): HeavyType { + return runHeavy(); +} diff --git a/tools/eslint-rules/rules/__fixtures__/consistent-base-hook/stubs/watched-pkg/index.ts b/tools/eslint-rules/rules/__fixtures__/consistent-base-hook/stubs/watched-pkg/index.ts new file mode 100644 index 00000000000000..f20537a7b393de --- /dev/null +++ b/tools/eslint-rules/rules/__fixtures__/consistent-base-hook/stubs/watched-pkg/index.ts @@ -0,0 +1,5 @@ +export { useHeavy } from './heavy'; +export { useLight } from './light'; +export type { LightOptions } from './light'; +// Re-export of a type-only thing from the heavy module — must not count as a runtime reach. +export type { HeavyType } from './heavy'; diff --git a/tools/eslint-rules/rules/__fixtures__/consistent-base-hook/stubs/watched-pkg/light.ts b/tools/eslint-rules/rules/__fixtures__/consistent-base-hook/stubs/watched-pkg/light.ts new file mode 100644 index 00000000000000..970deab9edbbd4 --- /dev/null +++ b/tools/eslint-rules/rules/__fixtures__/consistent-base-hook/stubs/watched-pkg/light.ts @@ -0,0 +1,7 @@ +import { runLight } from 'light-helper'; + +export type LightOptions = { mode: 'light' }; + +export function useLight(opts?: LightOptions): void { + runLight(opts); +} diff --git a/tools/eslint-rules/rules/__fixtures__/consistent-base-hook/tsconfig.json b/tools/eslint-rules/rules/__fixtures__/consistent-base-hook/tsconfig.json new file mode 100644 index 00000000000000..432b9967e725a7 --- /dev/null +++ b/tools/eslint-rules/rules/__fixtures__/consistent-base-hook/tsconfig.json @@ -0,0 +1,21 @@ +{ + "compilerOptions": { + "target": "ES2022", + "module": "ESNext", + "moduleResolution": "node", + "lib": ["ES2022", "DOM"], + "strict": true, + "esModuleInterop": true, + "skipLibCheck": true, + "noEmit": true, + "baseUrl": ".", + "paths": { + "watched-pkg": ["./stubs/watched-pkg/index.ts"], + "watched-pkg/*": ["./stubs/watched-pkg/*"], + "heavy-runtime": ["./stubs/heavy-runtime/index.ts"], + "light-helper": ["./stubs/light-helper/index.ts"], + "cyclic-pkg": ["./stubs/cyclic-pkg/index.ts"] + } + }, + "include": ["src/**/*.ts", "src/**/*.tsx", "stubs/**/*.ts"] +} diff --git a/tools/eslint-rules/rules/consistent-base-hook.spec.ts b/tools/eslint-rules/rules/consistent-base-hook.spec.ts index c15112c95fb9fb..0069ed215cadc2 100644 --- a/tools/eslint-rules/rules/consistent-base-hook.spec.ts +++ b/tools/eslint-rules/rules/consistent-base-hook.spec.ts @@ -1,6 +1,20 @@ +import * as path from 'node:path'; import { RuleTester } from '@typescript-eslint/rule-tester'; import { rule, RULE_NAME } from './consistent-base-hook'; +const FIXTURE_ROOT = path.join(__dirname, '__fixtures__/consistent-base-hook'); +const TYPED_FILENAME = 'src/test.ts'; + +const typedLanguageOptions = { + parserOptions: { + project: path.join(FIXTURE_ROOT, 'tsconfig.json'), + tsconfigRootDir: FIXTURE_ROOT, + }, +}; + +// --------------------------------------------------------------------------- +// Parameter-shape checks — no typed linting required. +// --------------------------------------------------------------------------- const ruleTester = new RuleTester(); ruleTester.run(RULE_NAME, rule, { @@ -23,7 +37,7 @@ ruleTester.run(RULE_NAME, rule, { } `, }, - // Valid base hook with only `props` (ref is optional). + // Valid base hook with only \`props\` (ref is optional). { code: ` export const useThingBase_unstable = (props) => { @@ -31,19 +45,6 @@ ruleTester.run(RULE_NAME, rule, { }; `, }, - // Forbidden import exists but is only used by a non-base hook in the same file. - { - code: ` - import { useArrowNavigationGroup } from '@fluentui/react-tabster'; - export const useThingBase_unstable = (props, ref: React.Ref) => { - return { props, ref }; - }; - export const useThing_unstable = (props, ref) => { - const nav = useArrowNavigationGroup({}); - return { ...useThingBase_unstable(props, ref), nav }; - }; - `, - }, // Non-base hook is not subject to the param-shape constraint. { code: ` @@ -52,32 +53,20 @@ ruleTester.run(RULE_NAME, rule, { }; `, }, - // Allowlist opt-out: a specific imported name is permitted inside base hooks. + // Identifier with the same local name as a forbidden import alias does not collide via scope analysis. { code: ` - import { useFocusFinders } from '@fluentui/react-tabster'; - export const useThingBase_unstable = (props, ref: React.Ref) => { - const finders = useFocusFinders(); - return { props, ref, finders }; + import { useArrowNavigationGroup } from '@fluentui/react-tabster'; + export const useThing_unstable = (props, ref) => { + return useArrowNavigationGroup({}); }; - `, - options: [ - { - forbiddenPackages: [{ name: '@fluentui/react-tabster', allow: ['useFocusFinders'] }], - }, - ], - }, - // Default allowlist: `useFocusWithin` and `useFocusVisible` from `@fluentui/react-tabster` are permitted inside base hooks. - { - code: ` - import { useFocusWithin, useFocusVisible } from '@fluentui/react-tabster'; - export const useThingBase_unstable = (props, ref: React.Ref) => { - useFocusVisible(); - return { props, ref: useFocusWithin() }; + export const useThingBase_unstable = (props, ref: React.Ref) => { + const useArrowNavigationGroup = () => 1; + return { value: useArrowNavigationGroup() }; }; `, }, - // `keyborg` is not forbidden by default — bindings imported from it are allowed inside base hooks. + // \`keyborg\` is not in the default forbidden runtime list — bindings imported from it are allowed inside base hooks. { code: ` import { createKeyborg, KEYBORG_FOCUSIN } from 'keyborg'; @@ -86,19 +75,6 @@ ruleTester.run(RULE_NAME, rule, { }; `, }, - // Identifier with the same local name as a forbidden import alias does not collide via scope analysis. - { - code: ` - import { useArrowNavigationGroup } from '@fluentui/react-tabster'; - export const useThing_unstable = (props, ref) => { - return useArrowNavigationGroup({}); - }; - export const useThingBase_unstable = (props, ref: React.Ref) => { - const useArrowNavigationGroup = () => 1; - return { value: useArrowNavigationGroup() }; - }; - `, - }, ], invalid: [ // Too few params (0). @@ -131,7 +107,7 @@ ruleTester.run(RULE_NAME, rule, { }, ], }, - // ObjectPattern for `props` is not allowed. + // ObjectPattern for \`props\` is not allowed. { code: ` export const useThingBase_unstable = ({ a }, ref: React.Ref) => ({ a, ref }); @@ -143,7 +119,7 @@ ruleTester.run(RULE_NAME, rule, { }, ], }, - // `ref` parameter without a type annotation. + // \`ref\` parameter without a type annotation. { code: ` export const useThingBase_unstable = (props, ref) => ({ props, ref }); @@ -155,7 +131,7 @@ ruleTester.run(RULE_NAME, rule, { }, ], }, - // `ref` parameter typed as something other than React.Ref. + // \`ref\` parameter typed as something other than React.Ref. { code: ` export const useThingBase_unstable = (props, ref: HTMLElement) => ({ props, ref }); @@ -167,7 +143,7 @@ ruleTester.run(RULE_NAME, rule, { }, ], }, - // `ref` parameter typed as React.ForwardedRef (must be React.Ref). + // \`ref\` parameter typed as React.ForwardedRef (must be React.Ref). { code: ` export const useThingBase_unstable = (props, ref: React.ForwardedRef) => ({ props, ref }); @@ -179,67 +155,159 @@ ruleTester.run(RULE_NAME, rule, { }, ], }, - // Body uses a binding imported from @fluentui/react-tabster. + ], +}); + +// --------------------------------------------------------------------------- +// Forbidden-runtime + transitive-reach checks — require typed linting. +// --------------------------------------------------------------------------- +const typedRuleTester = new RuleTester(); + +const transitiveOptions: readonly [{ watchedPackages: string[]; forbiddenRuntimes: string[] }] = [ + { + watchedPackages: ['watched-pkg'], + forbiddenRuntimes: ['heavy-runtime'], + }, +]; + +typedRuleTester.run(`${RULE_NAME} (typed)`, rule, { + valid: [ + // The defining file of \`useLight\` only reaches \`light-helper\`, not \`heavy-runtime\`. { + languageOptions: typedLanguageOptions, + filename: TYPED_FILENAME, + options: transitiveOptions, code: ` - import { useArrowNavigationGroup } from '@fluentui/react-tabster'; - export const useThingBase_unstable = (props, ref: React.Ref) => { - const nav = useArrowNavigationGroup({}); - return { props, ref, nav }; + import { useLight } from 'watched-pkg'; + export const useThingBase_unstable = (props: { a: number }, ref: React.Ref) => { + useLight(); + return { props, ref }; }; `, - errors: [ + }, + // Type-only import of a symbol whose defining file reaches the forbidden runtime is allowed. + { + languageOptions: typedLanguageOptions, + filename: TYPED_FILENAME, + options: transitiveOptions, + code: ` + import type { HeavyType } from 'watched-pkg'; + export const useThingBase_unstable = (props: HeavyType, ref: React.Ref) => { + return { props, ref }; + }; + `, + }, + // Per-specifier type-only import is also OK. + { + languageOptions: typedLanguageOptions, + filename: TYPED_FILENAME, + options: transitiveOptions, + code: ` + import { type HeavyType, useLight } from 'watched-pkg'; + export const useThingBase_unstable = (props: HeavyType, ref: React.Ref) => { + useLight(); + return { props, ref }; + }; + `, + }, + // Watched-package import exists but only used by a non-base hook in the same file. + { + languageOptions: typedLanguageOptions, + filename: TYPED_FILENAME, + options: transitiveOptions, + code: ` + import { useHeavy } from 'watched-pkg'; + export const useThingBase_unstable = (props: { a: number }, ref: React.Ref) => { + return { props, ref }; + }; + export const useThing_unstable = (props, ref) => { + return useHeavy(); + }; + `, + }, + // Cyclic re-export graph must not infinite-loop; \`useA\` does not reach heavy-runtime. + { + languageOptions: typedLanguageOptions, + filename: TYPED_FILENAME, + options: [ { - messageId: 'forbiddenPackageUsage', - data: { - hookName: 'useThingBase_unstable', - importedName: 'useArrowNavigationGroup', - package: '@fluentui/react-tabster', - }, + watchedPackages: ['cyclic-pkg'], + forbiddenRuntimes: ['heavy-runtime'], }, ], + code: ` + import { useA } from 'cyclic-pkg'; + export const useThingBase_unstable = (props: { a: number }, ref: React.Ref) => { + return { props, ref, value: useA() }; + }; + `, }, - // Body uses a binding imported from tabster, even when aliased. + ], + invalid: [ + // Direct import from a forbidden-runtime package. { + languageOptions: typedLanguageOptions, + filename: TYPED_FILENAME, + options: transitiveOptions, code: ` - import { getTabsterAttribute as gta } from 'tabster'; - export function useThingBase_unstable(props, ref: React.Ref) { - return { attr: gta({}) }; - } + import { runHeavy } from 'heavy-runtime'; + export const useThingBase_unstable = (props: { a: number }, ref: React.Ref) => { + return { props, ref, x: runHeavy() }; + }; `, errors: [ { - messageId: 'forbiddenPackageUsage', + messageId: 'forbiddenRuntimeDirect', data: { hookName: 'useThingBase_unstable', - importedName: 'getTabsterAttribute', - package: 'tabster', + importedName: 'runHeavy', + package: 'heavy-runtime', }, }, ], }, - // Allowlist excludes one name; siblings still error. + // Symbol from watched package whose defining file transitively imports the forbidden runtime. { + languageOptions: typedLanguageOptions, + filename: TYPED_FILENAME, + options: transitiveOptions, code: ` - import { useFocusFinders, useArrowNavigationGroup } from '@fluentui/react-tabster'; - export const useThingBase_unstable = (props, ref: React.Ref) => { - const finders = useFocusFinders(); - const nav = useArrowNavigationGroup({}); - return { props, ref, finders, nav }; + import { useHeavy } from 'watched-pkg'; + export const useThingBase_unstable = (props: { a: number }, ref: React.Ref) => { + return { props, ref, x: useHeavy() }; }; `, - options: [ + errors: [ { - forbiddenPackages: [{ name: '@fluentui/react-tabster', allow: ['useFocusFinders'] }], + messageId: 'forbiddenRuntimeReach', + data: { + hookName: 'useThingBase_unstable', + importedName: 'useHeavy', + package: 'watched-pkg', + runtime: 'heavy-runtime', + viaFile: 'rules/__fixtures__/consistent-base-hook/stubs/watched-pkg/heavy.ts', + }, }, ], + }, + // Aliased import from a forbidden-runtime package is still flagged on the alias use site. + { + languageOptions: typedLanguageOptions, + filename: TYPED_FILENAME, + options: transitiveOptions, + code: ` + import { runHeavy as go } from 'heavy-runtime'; + export function useThingBase_unstable(props: { a: number }, ref: React.Ref) { + return go(); + } + `, errors: [ { - messageId: 'forbiddenPackageUsage', + messageId: 'forbiddenRuntimeDirect', data: { hookName: 'useThingBase_unstable', - importedName: 'useArrowNavigationGroup', - package: '@fluentui/react-tabster', + importedName: 'runHeavy', + package: 'heavy-runtime', }, }, ], diff --git a/tools/eslint-rules/rules/consistent-base-hook.ts b/tools/eslint-rules/rules/consistent-base-hook.ts index 55dde83ad1acb2..c08226d808958e 100644 --- a/tools/eslint-rules/rules/consistent-base-hook.ts +++ b/tools/eslint-rules/rules/consistent-base-hook.ts @@ -1,4 +1,11 @@ -import { ESLintUtils, AST_NODE_TYPES, TSESTree, TSESLint } from '@typescript-eslint/utils'; +import { + ESLintUtils, + AST_NODE_TYPES, + TSESTree, + TSESLint, + ParserServicesWithTypeInformation, +} from '@typescript-eslint/utils'; +import * as ts from 'typescript'; // NOTE: The rule will be available in ESLint configs as "@nx/workspace-consistent-base-hook" export const RULE_NAME = 'consistent-base-hook'; @@ -7,79 +14,83 @@ const BASE_HOOK_NAME_PATTERN = /^use[A-Z]\w*Base_unstable$/; const EXPECTED_PARAM_NAMES = ['props', 'ref'] as const; const MIN_PARAM_COUNT = 1; const MAX_PARAM_COUNT = 2; -const DEFAULT_FORBIDDEN_PACKAGES: ReadonlyArray = [ - { - name: '@fluentui/react-tabster', - // APIs that only depend on `keyborg` (no `tabster` runtime) are safe to use inside base hooks. - allow: [ - 'useFocusWithin', - 'useFocusVisible', - 'useKeyboardNavAttribute', - 'useIsNavigatingWithKeyboard', - 'useSetKeyboardNavigation', - 'useOnKeyboardNavigationChange', - 'applyFocusVisiblePolyfill', - // re-exports from `keyborg` - 'KEYBORG_FOCUSIN', - 'KeyborgFocusInEvent', - ], - }, - 'tabster', -]; -type ForbiddenPackageOption = string | { name: string; allow?: string[] }; +const DEFAULT_WATCHED_PACKAGES: ReadonlyArray = ['@fluentui/react-tabster']; +const DEFAULT_FORBIDDEN_RUNTIMES: ReadonlyArray = ['tabster']; + type Options = [ { - forbiddenPackages?: ForbiddenPackageOption[]; + /** + * Packages whose imported symbols must be analyzed transitively. + * A symbol imported from one of these packages is allowed inside a base + * hook only if its defining source file does not reach any + * `forbiddenRuntimes` package via value imports. + */ + watchedPackages?: string[]; + /** + * Runtime packages whose presence in the transitive value-import graph of + * a referenced symbol is forbidden inside base hooks. Direct imports from + * these packages are also forbidden. + */ + forbiddenRuntimes?: string[]; }?, ]; -type MessageIds = 'invalidParamCount' | 'invalidParamName' | 'invalidRefType' | 'forbiddenPackageUsage'; -interface NormalizedForbiddenPackage { - name: string; - allow: Set; -} +type MessageIds = + | 'invalidParamCount' + | 'invalidParamName' + | 'invalidRefType' + | 'forbiddenRuntimeDirect' + | 'forbiddenRuntimeReach'; + +type ImportSpecifierNode = + | TSESTree.ImportSpecifier + | TSESTree.ImportDefaultSpecifier + | TSESTree.ImportNamespaceSpecifier; -interface ForbiddenImport { +interface TrackedImport { + /** The package the binding came from (a watched OR forbidden-runtime package). */ package: string; + /** Original imported name (not the local alias). `default` or `*` for default / namespace. */ importedName: string; + /** Kind of package — controls how the reference is checked. */ + kind: 'watched' | 'forbidden'; + /** The specifier node (used for symbol lookup via ParserServices). */ + specifier: ImportSpecifierNode; } type BaseHookFunction = TSESTree.FunctionDeclaration | TSESTree.FunctionExpression | TSESTree.ArrowFunctionExpression; +/** + * Per-Program cache: source file path → set of bare package specifiers transitively reached + * via value (non type-only) imports starting from that file. + * + * Keyed by `ts.Program` identity so the cache is invalidated whenever + * typescript-eslint rebuilds the Program. + */ +const programCache = new WeakMap>>(); + export const rule = ESLintUtils.RuleCreator(() => __filename)({ name: RULE_NAME, meta: { type: 'problem', docs: { description: - 'Enforce the API contract for v9 "base" hooks (`useBase_unstable`): a required `props` parameter and an optional `ref` parameter typed as `React.Ref<...>`, and no usage of bindings from configured forbidden packages (defaults to `@fluentui/react-tabster`, `tabster`, `keyborg`).', + 'Enforce the API contract for v9 "base" hooks (`useBase_unstable`): a required `props` parameter and an optional `ref` parameter typed as `React.Ref<...>`, and disallow referencing any binding whose defining module transitively pulls a forbidden runtime package (default `tabster`).', }, schema: [ { type: 'object', properties: { - forbiddenPackages: { + watchedPackages: { type: 'array', - items: { - oneOf: [ - { type: 'string' }, - { - type: 'object', - properties: { - name: { type: 'string' }, - allow: { - type: 'array', - items: { type: 'string' }, - uniqueItems: true, - }, - }, - required: ['name'], - additionalProperties: false, - }, - ], - }, - uniqueItems: false, + items: { type: 'string' }, + uniqueItems: true, + }, + forbiddenRuntimes: { + type: 'array', + items: { type: 'string' }, + uniqueItems: true, }, }, additionalProperties: false, @@ -91,20 +102,37 @@ export const rule = ESLintUtils.RuleCreator(() => __filename)`, got `{{actual}}`.', - forbiddenPackageUsage: - 'Base hook `{{hookName}}` cannot reference `{{importedName}}` from forbidden package `{{package}}`. Move logic that depends on `{{package}}` to the wrapping `*_unstable` hook instead.', + forbiddenRuntimeDirect: + 'Base hook `{{hookName}}` cannot reference `{{importedName}}` from forbidden runtime package `{{package}}`. Move logic that depends on `{{package}}` to the wrapping `*_unstable` hook instead.', + forbiddenRuntimeReach: + 'Base hook `{{hookName}}` cannot reference `{{importedName}}` from `{{package}}` because its defining module transitively imports forbidden runtime `{{runtime}}` (via `{{viaFile}}`). Move logic that depends on `{{runtime}}` to the wrapping `*_unstable` hook instead.', }, }, defaultOptions: [{}], create(context) { const sourceCode = context.sourceCode; const options = context.options[0] ?? {}; - const forbiddenPackages = normalizeForbiddenPackages(options.forbiddenPackages); - const forbiddenPackageByName = new Map(forbiddenPackages.map(pkg => [pkg.name, pkg])); + const watchedPackages = new Set(options.watchedPackages ?? DEFAULT_WATCHED_PACKAGES); + const forbiddenRuntimes = new Set(options.forbiddenRuntimes ?? DEFAULT_FORBIDDEN_RUNTIMES); + // `forbidden` takes precedence: if the same name appears in both lists, treat the binding as forbidden. + const trackedPackages = new Set([...watchedPackages, ...forbiddenRuntimes]); + + // Variable → import origin. Keyed by Variable identity so shadowing inside the base hook is handled. + const trackedImports = new Map(); - // Map from import Variable -> origin (package + original imported name). - // Keyed by Variable identity (not name) so shadowing inside a base hook is handled correctly. - const forbiddenImports = new Map(); + // Lazily-acquired typed services. Untyped lint silently skips transitive analysis. + let typedServices: ParserServicesWithTypeInformation | null | undefined; + function getTypedServices(): ParserServicesWithTypeInformation | null { + if (typedServices !== undefined) { + return typedServices; + } + try { + typedServices = ESLintUtils.getParserServices(context); + } catch { + typedServices = null; + } + return typedServices; + } function checkBaseHook(hookName: string, fn: BaseHookFunction, reportNode: TSESTree.Node): void { checkParameters(hookName, fn, reportNode); @@ -116,10 +144,7 @@ export const rule = ESLintUtils.RuleCreator(() => __filename) __filename) __filename) __filename) visitScope(child, fn, hookName)); } + function computeSymbolReach(origin: TrackedImport): { reached: ReadonlySet; viaFile: string } | null { + const services = getTypedServices(); + if (!services) { + return null; + } + const checker = services.program.getTypeChecker(); + const tsNode = services.esTreeNodeToTSNodeMap.get(origin.specifier); + if (!tsNode) { + return null; + } + + // For an ImportSpecifier we want the imported (right-hand) identifier so the symbol resolves to + // the exported name on the source module, not the local alias. + let nameNode: ts.Node | undefined; + if (ts.isImportSpecifier(tsNode)) { + nameNode = tsNode.propertyName ?? tsNode.name; + } else if (ts.isImportClause(tsNode) || ts.isNamespaceImport(tsNode)) { + nameNode = tsNode.name; + } else { + nameNode = tsNode; + } + if (!nameNode) { + return null; + } + + let symbol = checker.getSymbolAtLocation(nameNode); + if (!symbol) { + return null; + } + if (symbol.flags & ts.SymbolFlags.Alias) { + try { + symbol = checker.getAliasedSymbol(symbol); + } catch { + return null; + } + } + const decl = symbol.declarations?.[0]; + const sf = decl?.getSourceFile(); + if (!sf) { + return null; + } + + const reached = transitiveValuePackages(services.program, sf); + return { reached, viaFile: shortenPath(sf.fileName) }; + } + return { ImportDeclaration(node) { const source = node.source.value; - if (typeof source !== 'string' || !forbiddenPackageByName.has(source)) { + if (typeof source !== 'string' || !trackedPackages.has(source)) { return; } + // Purely type-only imports cannot pull runtime. + if (node.importKind === 'type') { + return; + } + const kind: TrackedImport['kind'] = forbiddenRuntimes.has(source) ? 'forbidden' : 'watched'; node.specifiers.forEach(specifier => { + // Per-specifier type-only imports are also runtime-free. + if (specifier.type === AST_NODE_TYPES.ImportSpecifier && specifier.importKind === 'type') { + return; + } let importedName: string; switch (specifier.type) { case AST_NODE_TYPES.ImportSpecifier: @@ -220,7 +313,7 @@ export const rule = ESLintUtils.RuleCreator(() => __filename) __filename) { - if (typeof entry === 'string') { - return { name: entry, allow: new Set() }; +// --------------------------------------------------------------------------- +// Transitive value-import analysis +// --------------------------------------------------------------------------- + +/** + * Returns the set of bare package specifiers transitively reachable from `sf` + * via non-type-only imports. Memoized per Program × file. Cycle-safe. + */ +function transitiveValuePackages(program: ts.Program, sf: ts.SourceFile): ReadonlySet { + let cache = programCache.get(program); + if (!cache) { + cache = new Map(); + programCache.set(program, cache); + } + return computeReach(program, sf, cache, new Set()); +} + +function computeReach( + program: ts.Program, + sf: ts.SourceFile, + cache: Map>, + inProgress: Set, +): ReadonlySet { + const cached = cache.get(sf.fileName); + if (cached) { + return cached; + } + if (inProgress.has(sf.fileName)) { + // Cycle: return an empty set without caching so the eventual full result is committed by the originator. + return EMPTY_SET; + } + inProgress.add(sf.fileName); + + const result = new Set(); + for (const { specifier, literal } of collectValueImports(sf)) { + if (isBareSpecifier(specifier)) { + result.add(packageNameOf(specifier)); + } + const resolved = resolveModule(program, sf, specifier, literal); + if (!resolved) { + continue; + } + const childSf = program.getSourceFile(resolved); + if (!childSf) { + continue; + } + const childReach = computeReach(program, childSf, cache, inProgress); + for (const pkg of childReach) { + result.add(pkg); + } + } + + inProgress.delete(sf.fileName); + cache.set(sf.fileName, result); + return result; +} + +const EMPTY_SET: ReadonlySet = new Set(); + +interface ValueImport { + specifier: string; + literal: ts.StringLiteralLike; +} + +function collectValueImports(sf: ts.SourceFile): ValueImport[] { + const result: ValueImport[] = []; + for (const stmt of sf.statements) { + if (ts.isImportDeclaration(stmt)) { + if (stmt.importClause?.isTypeOnly) { + continue; + } + // `import './side-effect'` (no clause) IS a value import. + // For named imports, drop if every specifier is type-only AND there's no default/namespace. + if (stmt.importClause && stmt.importClause.namedBindings && ts.isNamedImports(stmt.importClause.namedBindings)) { + const named = stmt.importClause.namedBindings; + const hasValue = !!stmt.importClause.name || named.elements.some(el => !el.isTypeOnly); + if (!hasValue) { + continue; + } + } + if (ts.isStringLiteralLike(stmt.moduleSpecifier)) { + result.push({ specifier: stmt.moduleSpecifier.text, literal: stmt.moduleSpecifier }); + } + continue; + } + if (ts.isExportDeclaration(stmt) && stmt.moduleSpecifier && ts.isStringLiteralLike(stmt.moduleSpecifier)) { + if (stmt.isTypeOnly) { + continue; + } + if (stmt.exportClause && ts.isNamedExports(stmt.exportClause)) { + const allTypeOnly = stmt.exportClause.elements.every(el => el.isTypeOnly); + if (allTypeOnly) { + continue; + } + } + result.push({ specifier: stmt.moduleSpecifier.text, literal: stmt.moduleSpecifier }); + continue; + } + if ( + ts.isImportEqualsDeclaration(stmt) && + ts.isExternalModuleReference(stmt.moduleReference) && + ts.isStringLiteralLike(stmt.moduleReference.expression) + ) { + if (stmt.isTypeOnly) { + continue; + } + result.push({ + specifier: stmt.moduleReference.expression.text, + literal: stmt.moduleReference.expression, + }); + } + } + return result; +} + +function resolveModule( + program: ts.Program, + sf: ts.SourceFile, + specifier: string, + literal: ts.StringLiteralLike, +): string | undefined { + // TS ≥ 5.3 exposes program.getResolvedModule + const getResolvedModule = ( + program as unknown as { + getResolvedModule?: ( + file: ts.SourceFile, + moduleName: string, + mode?: ts.ResolutionMode, + ) => { resolvedModule?: ts.ResolvedModuleFull } | undefined; + } + ).getResolvedModule; + const mode = ( + ts as unknown as { + getModeForUsageLocation?: (file: ts.SourceFile, usage: ts.StringLiteralLike) => ts.ResolutionMode; + } + ).getModeForUsageLocation?.(sf, literal); + + if (typeof getResolvedModule === 'function') { + const r = getResolvedModule.call(program, sf, specifier, mode); + if (r?.resolvedModule) { + return r.resolvedModule.resolvedFileName; } - return { name: entry.name, allow: new Set(entry.allow ?? []) }; - }); + } + + // Fallback for older TS: use ts.resolveModuleName against the compiler host. + const compilerOptions = program.getCompilerOptions(); + const host = + (program as unknown as { getCompilerHost?: () => ts.ModuleResolutionHost }).getCompilerHost?.() ?? ts.sys; + const result = ts.resolveModuleName( + specifier, + sf.fileName, + compilerOptions, + host as ts.ModuleResolutionHost, + undefined, + undefined, + mode, + ); + return result.resolvedModule?.resolvedFileName; +} + +function isBareSpecifier(specifier: string): boolean { + return !specifier.startsWith('.') && !specifier.startsWith('/'); +} + +function packageNameOf(specifier: string): string { + if (specifier.startsWith('@')) { + const [scope, name] = specifier.split('/', 2); + return name ? `${scope}/${name}` : scope; + } + const slash = specifier.indexOf('/'); + return slash === -1 ? specifier : specifier.slice(0, slash); +} + +function shortenPath(absolute: string): string { + const marker = '/node_modules/'; + const idx = absolute.lastIndexOf(marker); + if (idx !== -1) { + return absolute.slice(idx + marker.length); + } + const cwd = process.cwd(); + if (absolute.startsWith(cwd)) { + return absolute.slice(cwd.length + 1); + } + return absolute; } +// --------------------------------------------------------------------------- +// AST helpers (unchanged from previous version) +// --------------------------------------------------------------------------- + function describeParam(param: TSESTree.Parameter): string { switch (param.type) { case AST_NODE_TYPES.Identifier: From b4ed75940677d331041a2905653f8e093fbc3b65 Mon Sep 17 00:00:00 2001 From: Martin Hochel Date: Mon, 25 May 2026 11:26:47 +0200 Subject: [PATCH 10/18] feat(eslint-rules): add allowTypeImports option to consistent-base-hook By default (allowTypeImports: false) type-only imports from forbiddenRuntimes packages are disallowed inside base hooks, to keep the base hook's public API fully decoupled from those packages. Setting allowTypeImports: true restores the previous behavior of skipping type-only imports. Type-only imports from watched packages are unaffected (types cannot transitively pull runtime). --- .../stubs/heavy-runtime/index.ts | 2 + .../rules/consistent-base-hook.spec.ts | 77 +++++++++++++++++++ .../rules/consistent-base-hook.ts | 26 +++++-- 3 files changed, 100 insertions(+), 5 deletions(-) diff --git a/tools/eslint-rules/rules/__fixtures__/consistent-base-hook/stubs/heavy-runtime/index.ts b/tools/eslint-rules/rules/__fixtures__/consistent-base-hook/stubs/heavy-runtime/index.ts index 5fc7a81d62453b..8f5f7d2b4e91fb 100644 --- a/tools/eslint-rules/rules/__fixtures__/consistent-base-hook/stubs/heavy-runtime/index.ts +++ b/tools/eslint-rules/rules/__fixtures__/consistent-base-hook/stubs/heavy-runtime/index.ts @@ -1,3 +1,5 @@ export function runHeavy(): { tag: 'heavy' } { return { tag: 'heavy' }; } + +export type HeavyOptions = { kind: 'heavy' }; diff --git a/tools/eslint-rules/rules/consistent-base-hook.spec.ts b/tools/eslint-rules/rules/consistent-base-hook.spec.ts index 0069ed215cadc2..3713c9a4f43a1d 100644 --- a/tools/eslint-rules/rules/consistent-base-hook.spec.ts +++ b/tools/eslint-rules/rules/consistent-base-hook.spec.ts @@ -170,6 +170,16 @@ const transitiveOptions: readonly [{ watchedPackages: string[]; forbiddenRuntime }, ]; +const transitiveOptionsAllowTypeImports: readonly [ + { watchedPackages: string[]; forbiddenRuntimes: string[]; allowTypeImports: boolean }, +] = [ + { + watchedPackages: ['watched-pkg'], + forbiddenRuntimes: ['heavy-runtime'], + allowTypeImports: true, + }, +]; + typedRuleTester.run(`${RULE_NAME} (typed)`, rule, { valid: [ // The defining file of \`useLight\` only reaches \`light-helper\`, not \`heavy-runtime\`. @@ -241,6 +251,29 @@ typedRuleTester.run(`${RULE_NAME} (typed)`, rule, { return { props, ref, value: useA() }; }; `, + }, // With `allowTypeImports: true`, type-only imports from a forbidden runtime are permitted. + { + languageOptions: typedLanguageOptions, + filename: TYPED_FILENAME, + options: transitiveOptionsAllowTypeImports, + code: ` + import type { HeavyOptions } from 'heavy-runtime'; + export const useThingBase_unstable = (props: HeavyOptions, ref: React.Ref) => { + return { props, ref }; + }; + `, + }, + // With `allowTypeImports: true`, per-specifier type-only import from a forbidden runtime is permitted. + { + languageOptions: typedLanguageOptions, + filename: TYPED_FILENAME, + options: transitiveOptionsAllowTypeImports, + code: ` + import { type HeavyOptions } from 'heavy-runtime'; + export const useThingBase_unstable = (props: HeavyOptions, ref: React.Ref) => { + return { props, ref }; + }; + `, }, ], invalid: [ @@ -312,5 +345,49 @@ typedRuleTester.run(`${RULE_NAME} (typed)`, rule, { }, ], }, + // By default, a top-level type-only import from a forbidden runtime is disallowed. + { + languageOptions: typedLanguageOptions, + filename: TYPED_FILENAME, + options: transitiveOptions, + code: ` + import type { HeavyOptions } from 'heavy-runtime'; + export const useThingBase_unstable = (props: HeavyOptions, ref: React.Ref) => { + return { props, ref }; + }; + `, + errors: [ + { + messageId: 'forbiddenRuntimeDirect', + data: { + hookName: 'useThingBase_unstable', + importedName: 'HeavyOptions', + package: 'heavy-runtime', + }, + }, + ], + }, + // By default, a per-specifier type-only import from a forbidden runtime is also disallowed. + { + languageOptions: typedLanguageOptions, + filename: TYPED_FILENAME, + options: transitiveOptions, + code: ` + import { type HeavyOptions } from 'heavy-runtime'; + export const useThingBase_unstable = (props: HeavyOptions, ref: React.Ref) => { + return { props, ref }; + }; + `, + errors: [ + { + messageId: 'forbiddenRuntimeDirect', + data: { + hookName: 'useThingBase_unstable', + importedName: 'HeavyOptions', + package: 'heavy-runtime', + }, + }, + ], + }, ], }); diff --git a/tools/eslint-rules/rules/consistent-base-hook.ts b/tools/eslint-rules/rules/consistent-base-hook.ts index c08226d808958e..481d221315f71d 100644 --- a/tools/eslint-rules/rules/consistent-base-hook.ts +++ b/tools/eslint-rules/rules/consistent-base-hook.ts @@ -33,6 +33,15 @@ type Options = [ * these packages are also forbidden. */ forbiddenRuntimes?: string[]; + /** + * When `true`, type-only imports from `forbiddenRuntimes` packages are + * permitted inside base hooks (they emit no runtime code). + * + * Defaults to `false` — type-only imports from forbidden runtimes are + * disallowed as well, to keep the base hook's public API fully decoupled + * from those packages. + */ + allowTypeImports?: boolean; }?, ]; @@ -92,6 +101,9 @@ export const rule = ESLintUtils.RuleCreator(() => __filename) __filename)([...watchedPackages, ...forbiddenRuntimes]); @@ -284,15 +297,18 @@ export const rule = ESLintUtils.RuleCreator(() => __filename) { - // Per-specifier type-only imports are also runtime-free. - if (specifier.type === AST_NODE_TYPES.ImportSpecifier && specifier.importKind === 'type') { + const specTypeOnly = specifier.type === AST_NODE_TYPES.ImportSpecifier && specifier.importKind === 'type'; + if (specTypeOnly && (!isForbiddenPkg || allowTypeImports)) { return; } let importedName: string; From 77938a0afe3f16771ef6b39611102e9d5910e848 Mon Sep 17 00:00:00 2001 From: Martin Hochel Date: Mon, 25 May 2026 13:08:27 +0200 Subject: [PATCH 11/18] style: encapsulate import tracking and apply explicit type imports/ignore fixtures from type check --- .../rules/consistent-base-hook.ts | 94 ++++++++++--------- .../rules/consistent-callback-type.ts | 3 +- tools/eslint-rules/tsconfig.lint.json | 2 +- 3 files changed, 52 insertions(+), 47 deletions(-) diff --git a/tools/eslint-rules/rules/consistent-base-hook.ts b/tools/eslint-rules/rules/consistent-base-hook.ts index 481d221315f71d..d4118052bd2033 100644 --- a/tools/eslint-rules/rules/consistent-base-hook.ts +++ b/tools/eslint-rules/rules/consistent-base-hook.ts @@ -1,10 +1,5 @@ -import { - ESLintUtils, - AST_NODE_TYPES, - TSESTree, - TSESLint, - ParserServicesWithTypeInformation, -} from '@typescript-eslint/utils'; +import type { TSESTree, TSESLint, ParserServicesWithTypeInformation } from '@typescript-eslint/utils'; +import { ESLintUtils, AST_NODE_TYPES } from '@typescript-eslint/utils'; import * as ts from 'typescript'; // NOTE: The rule will be available in ESLint configs as "@nx/workspace-consistent-base-hook" @@ -274,7 +269,8 @@ export const rule = ESLintUtils.RuleCreator(() => __filename) __filename) { + const specTypeOnly = specifier.type === AST_NODE_TYPES.ImportSpecifier && specifier.importKind === 'type'; + if (specTypeOnly && (!isForbiddenPkg || allowTypeImports)) { return; } - const isForbiddenPkg = forbiddenRuntimes.has(source); - const declTypeOnly = node.importKind === 'type'; - // Type-only imports from a watched package can never pull runtime; always skip. - // Type-only imports from a forbidden-runtime package are skipped only when explicitly allowed. - if (declTypeOnly && (!isForbiddenPkg || allowTypeImports)) { + const importedName = getImportedName(specifier); + if (importedName === undefined) { return; } - const kind: TrackedImport['kind'] = isForbiddenPkg ? 'forbidden' : 'watched'; + for (const variable of sourceCode.getDeclaredVariables(specifier)) { + trackedImports.set(variable, { package: source, importedName, kind, specifier }); + } + }); + } - node.specifiers.forEach(specifier => { - const specTypeOnly = specifier.type === AST_NODE_TYPES.ImportSpecifier && specifier.importKind === 'type'; - if (specTypeOnly && (!isForbiddenPkg || allowTypeImports)) { - return; - } - let importedName: string; - switch (specifier.type) { - case AST_NODE_TYPES.ImportSpecifier: - importedName = - specifier.imported.type === AST_NODE_TYPES.Identifier - ? specifier.imported.name - : String(specifier.imported.value); - break; - case AST_NODE_TYPES.ImportDefaultSpecifier: - importedName = 'default'; - break; - case AST_NODE_TYPES.ImportNamespaceSpecifier: - importedName = '*'; - break; - default: - return; - } - for (const variable of sourceCode.getDeclaredVariables(specifier)) { - trackedImports.set(variable, { package: source, importedName, kind, specifier }); - } - }); - }, + return { + ImportDeclaration: trackImportDeclaration, [`FunctionDeclaration[id.name=/${BASE_HOOK_NAME_PATTERN.source}/]`]: (node: TSESTree.FunctionDeclaration) => { if (!node.id) { @@ -548,6 +532,26 @@ function shortenPath(absolute: string): string { // AST helpers (unchanged from previous version) // --------------------------------------------------------------------------- +/** + * Resolves the original (imported) name from an import specifier. + * Returns `'default'` for default imports, `'*'` for namespace imports, + * and the imported identifier/string-literal name for named imports. + */ +function getImportedName(specifier: ImportSpecifierNode): string | undefined { + switch (specifier.type) { + case AST_NODE_TYPES.ImportSpecifier: + return specifier.imported.type === AST_NODE_TYPES.Identifier + ? specifier.imported.name + : String(specifier.imported.value); + case AST_NODE_TYPES.ImportDefaultSpecifier: + return 'default'; + case AST_NODE_TYPES.ImportNamespaceSpecifier: + return '*'; + default: + return undefined; + } +} + function describeParam(param: TSESTree.Parameter): string { switch (param.type) { case AST_NODE_TYPES.Identifier: diff --git a/tools/eslint-rules/rules/consistent-callback-type.ts b/tools/eslint-rules/rules/consistent-callback-type.ts index 20f012ef6d3ea3..ed5ea947187f1a 100644 --- a/tools/eslint-rules/rules/consistent-callback-type.ts +++ b/tools/eslint-rules/rules/consistent-callback-type.ts @@ -1,4 +1,5 @@ -import { ESLintUtils, AST_NODE_TYPES, TSESTree } from '@typescript-eslint/utils'; +import type { TSESTree } from '@typescript-eslint/utils'; +import { ESLintUtils, AST_NODE_TYPES } from '@typescript-eslint/utils'; // NOTE: The rule will be available in ESLint configs as "@nx/workspace-consistent-callback-type" export const RULE_NAME = 'consistent-callback-type'; diff --git a/tools/eslint-rules/tsconfig.lint.json b/tools/eslint-rules/tsconfig.lint.json index bb717c5e289e34..5215cb85923e4e 100644 --- a/tools/eslint-rules/tsconfig.lint.json +++ b/tools/eslint-rules/tsconfig.lint.json @@ -4,6 +4,6 @@ "outDir": "../../dist/out-tsc", "types": ["node"] }, - "exclude": ["**/*.spec.ts"], + "exclude": ["**/*.spec.ts", "**/__fixtures__/**"], "include": ["**/*.ts"] } From 6ada519c81110819d3fd9ecff96727b2796e573f Mon Sep 17 00:00:00 2001 From: Martin Hochel Date: Mon, 25 May 2026 20:13:04 +0200 Subject: [PATCH 12/18] fix(eslint-rules): validate Ref import origin in consistent-base-hook; fix tooltip directive - isReactRefTypeAnnotation now resolves the Ref/React identifier via scope analysis and requires it to be imported from 'react'. Adds tests for local aliases, non-react imports, and shadowed React.- useTooltipBase_unstable: use bare 'use no memo' directive prologue instead of parenthesized expression so React Compiler detects it. --- .../src/components/Tooltip/useTooltipBase.tsx | 2 +- .../rules/consistent-base-hook.spec.ts | 67 ++++++++++++++++- .../rules/consistent-base-hook.ts | 73 ++++++++++++++++++- 3 files changed, 135 insertions(+), 7 deletions(-) diff --git a/packages/react-components/react-tooltip/library/src/components/Tooltip/useTooltipBase.tsx b/packages/react-components/react-tooltip/library/src/components/Tooltip/useTooltipBase.tsx index 30d586b5e79b43..4919ea58156ac3 100644 --- a/packages/react-components/react-tooltip/library/src/components/Tooltip/useTooltipBase.tsx +++ b/packages/react-components/react-tooltip/library/src/components/Tooltip/useTooltipBase.tsx @@ -35,7 +35,7 @@ import { Escape } from '@fluentui/keyboard-keys'; * @param props - props from this instance of Tooltip */ export const useTooltipBase_unstable = (props: TooltipBaseProps): TooltipBaseState => { - ('use no memo'); + 'use no memo'; const context = useTooltipVisibility(); const isServerSideRender = useIsSSR(); diff --git a/tools/eslint-rules/rules/consistent-base-hook.spec.ts b/tools/eslint-rules/rules/consistent-base-hook.spec.ts index 3713c9a4f43a1d..4319b36824f459 100644 --- a/tools/eslint-rules/rules/consistent-base-hook.spec.ts +++ b/tools/eslint-rules/rules/consistent-base-hook.spec.ts @@ -19,7 +19,7 @@ const ruleTester = new RuleTester(); ruleTester.run(RULE_NAME, rule, { valid: [ - // Valid base hook: 2 Identifier params, no forbidden imports. + // Valid base hook: namespace import — `import * as React from 'react'` + `React.Ref<...>`. { code: ` import * as React from 'react'; @@ -28,7 +28,7 @@ ruleTester.run(RULE_NAME, rule, { }; `, }, - // Valid base hook declared as FunctionDeclaration. + // Valid base hook: named import — `import { Ref } from 'react'` + `Ref<...>` (FunctionDeclaration form). { code: ` import { Ref } from 'react'; @@ -37,6 +37,15 @@ ruleTester.run(RULE_NAME, rule, { } `, }, + // Valid base hook: default import — `import React from 'react'` + `React.Ref<...>`. + { + code: ` + import React from 'react'; + export const useThingBase_unstable = (props, ref: React.Ref) => { + return { props, ref }; + }; + `, + }, // Valid base hook with only \`props\` (ref is optional). { code: ` @@ -56,6 +65,7 @@ ruleTester.run(RULE_NAME, rule, { // Identifier with the same local name as a forbidden import alias does not collide via scope analysis. { code: ` + import * as React from 'react'; import { useArrowNavigationGroup } from '@fluentui/react-tabster'; export const useThing_unstable = (props, ref) => { return useArrowNavigationGroup({}); @@ -69,6 +79,7 @@ ruleTester.run(RULE_NAME, rule, { // \`keyborg\` is not in the default forbidden runtime list — bindings imported from it are allowed inside base hooks. { code: ` + import * as React from 'react'; import { createKeyborg, KEYBORG_FOCUSIN } from 'keyborg'; export const useThingBase_unstable = (props, ref: React.Ref) => { return { kb: createKeyborg(window), evt: KEYBORG_FOCUSIN }; @@ -110,6 +121,7 @@ ruleTester.run(RULE_NAME, rule, { // ObjectPattern for \`props\` is not allowed. { code: ` + import * as React from 'react'; export const useThingBase_unstable = ({ a }, ref: React.Ref) => ({ a, ref }); `, errors: [ @@ -155,6 +167,45 @@ ruleTester.run(RULE_NAME, rule, { }, ], }, + // \`Ref\` is a locally declared type alias, not imported from react. + { + code: ` + type Ref = { current: T | null }; + export const useThingBase_unstable = (props, ref: Ref) => ({ props, ref }); + `, + errors: [ + { + messageId: 'invalidRefType', + data: { hookName: 'useThingBase_unstable', actual: 'Ref' }, + }, + ], + }, + // \`Ref\` imported from a non-react package is not accepted. + { + code: ` + import { Ref } from 'not-react'; + export const useThingBase_unstable = (props, ref: Ref) => ({ props, ref }); + `, + errors: [ + { + messageId: 'invalidRefType', + data: { hookName: 'useThingBase_unstable', actual: 'Ref' }, + }, + ], + }, + // \`React\` is a locally declared identifier, not the react module namespace. + { + code: ` + const React = { Ref: null }; + export const useThingBase_unstable = (props, ref: React.Ref) => ({ props, ref }); + `, + errors: [ + { + messageId: 'invalidRefType', + data: { hookName: 'useThingBase_unstable', actual: 'React.Ref' }, + }, + ], + }, ], }); @@ -188,6 +239,7 @@ typedRuleTester.run(`${RULE_NAME} (typed)`, rule, { filename: TYPED_FILENAME, options: transitiveOptions, code: ` + import * as React from 'react'; import { useLight } from 'watched-pkg'; export const useThingBase_unstable = (props: { a: number }, ref: React.Ref) => { useLight(); @@ -201,6 +253,7 @@ typedRuleTester.run(`${RULE_NAME} (typed)`, rule, { filename: TYPED_FILENAME, options: transitiveOptions, code: ` + import * as React from 'react'; import type { HeavyType } from 'watched-pkg'; export const useThingBase_unstable = (props: HeavyType, ref: React.Ref) => { return { props, ref }; @@ -213,6 +266,7 @@ typedRuleTester.run(`${RULE_NAME} (typed)`, rule, { filename: TYPED_FILENAME, options: transitiveOptions, code: ` + import * as React from 'react'; import { type HeavyType, useLight } from 'watched-pkg'; export const useThingBase_unstable = (props: HeavyType, ref: React.Ref) => { useLight(); @@ -226,6 +280,7 @@ typedRuleTester.run(`${RULE_NAME} (typed)`, rule, { filename: TYPED_FILENAME, options: transitiveOptions, code: ` + import * as React from 'react'; import { useHeavy } from 'watched-pkg'; export const useThingBase_unstable = (props: { a: number }, ref: React.Ref) => { return { props, ref }; @@ -246,6 +301,7 @@ typedRuleTester.run(`${RULE_NAME} (typed)`, rule, { }, ], code: ` + import * as React from 'react'; import { useA } from 'cyclic-pkg'; export const useThingBase_unstable = (props: { a: number }, ref: React.Ref) => { return { props, ref, value: useA() }; @@ -257,6 +313,7 @@ typedRuleTester.run(`${RULE_NAME} (typed)`, rule, { filename: TYPED_FILENAME, options: transitiveOptionsAllowTypeImports, code: ` + import * as React from 'react'; import type { HeavyOptions } from 'heavy-runtime'; export const useThingBase_unstable = (props: HeavyOptions, ref: React.Ref) => { return { props, ref }; @@ -269,6 +326,7 @@ typedRuleTester.run(`${RULE_NAME} (typed)`, rule, { filename: TYPED_FILENAME, options: transitiveOptionsAllowTypeImports, code: ` + import * as React from 'react'; import { type HeavyOptions } from 'heavy-runtime'; export const useThingBase_unstable = (props: HeavyOptions, ref: React.Ref) => { return { props, ref }; @@ -283,6 +341,7 @@ typedRuleTester.run(`${RULE_NAME} (typed)`, rule, { filename: TYPED_FILENAME, options: transitiveOptions, code: ` + import * as React from 'react'; import { runHeavy } from 'heavy-runtime'; export const useThingBase_unstable = (props: { a: number }, ref: React.Ref) => { return { props, ref, x: runHeavy() }; @@ -305,6 +364,7 @@ typedRuleTester.run(`${RULE_NAME} (typed)`, rule, { filename: TYPED_FILENAME, options: transitiveOptions, code: ` + import * as React from 'react'; import { useHeavy } from 'watched-pkg'; export const useThingBase_unstable = (props: { a: number }, ref: React.Ref) => { return { props, ref, x: useHeavy() }; @@ -329,6 +389,7 @@ typedRuleTester.run(`${RULE_NAME} (typed)`, rule, { filename: TYPED_FILENAME, options: transitiveOptions, code: ` + import * as React from 'react'; import { runHeavy as go } from 'heavy-runtime'; export function useThingBase_unstable(props: { a: number }, ref: React.Ref) { return go(); @@ -351,6 +412,7 @@ typedRuleTester.run(`${RULE_NAME} (typed)`, rule, { filename: TYPED_FILENAME, options: transitiveOptions, code: ` + import * as React from 'react'; import type { HeavyOptions } from 'heavy-runtime'; export const useThingBase_unstable = (props: HeavyOptions, ref: React.Ref) => { return { props, ref }; @@ -373,6 +435,7 @@ typedRuleTester.run(`${RULE_NAME} (typed)`, rule, { filename: TYPED_FILENAME, options: transitiveOptions, code: ` + import * as React from 'react'; import { type HeavyOptions } from 'heavy-runtime'; export const useThingBase_unstable = (props: HeavyOptions, ref: React.Ref) => { return { props, ref }; diff --git a/tools/eslint-rules/rules/consistent-base-hook.ts b/tools/eslint-rules/rules/consistent-base-hook.ts index d4118052bd2033..712e96b424c37b 100644 --- a/tools/eslint-rules/rules/consistent-base-hook.ts +++ b/tools/eslint-rules/rules/consistent-base-hook.ts @@ -167,7 +167,7 @@ export const rule = ESLintUtils.RuleCreator(() => __filename) { + if (def.type !== 'ImportBinding') { + return false; + } + const importDecl = def.parent; + if (!importDecl || importDecl.type !== AST_NODE_TYPES.ImportDeclaration) { + return false; + } + if (importDecl.source.value !== 'react') { + return false; + } + const specifier = def.node; + switch (specifier.type) { + case AST_NODE_TYPES.ImportSpecifier: { + const importedName = + specifier.imported.type === AST_NODE_TYPES.Identifier + ? specifier.imported.name + : String(specifier.imported.value); + return importedName === expectedImportedName; + } + case AST_NODE_TYPES.ImportNamespaceSpecifier: + return expectedImportedName === '*'; + case AST_NODE_TYPES.ImportDefaultSpecifier: + // `import React from 'react'` is also a valid way to access `React.Ref`. + return expectedImportedName === '*' || expectedImportedName === 'default'; + default: + return false; + } + }); +} + +function findVariableInScope(scope: TSESLint.Scope.Scope, name: string): TSESLint.Scope.Variable | undefined { + let current: TSESLint.Scope.Scope | null = scope; + while (current) { + const variable = current.set.get(name); + if (variable) { + return variable; + } + current = current.upper; + } + return undefined; +} + function describeRefType(annotation: TSESTree.TSTypeAnnotation | undefined): string { if (!annotation) { return ''; From dffca5aad2846cdf0db9518a578b32d57288cb76 Mon Sep 17 00:00:00 2001 From: Martin Hochel Date: Tue, 26 May 2026 11:13:43 +0200 Subject: [PATCH 13/18] revert(eslint-rules): undo unrelated import change in consistent-callback-type --- tools/eslint-rules/rules/consistent-callback-type.ts | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/tools/eslint-rules/rules/consistent-callback-type.ts b/tools/eslint-rules/rules/consistent-callback-type.ts index ed5ea947187f1a..20f012ef6d3ea3 100644 --- a/tools/eslint-rules/rules/consistent-callback-type.ts +++ b/tools/eslint-rules/rules/consistent-callback-type.ts @@ -1,5 +1,4 @@ -import type { TSESTree } from '@typescript-eslint/utils'; -import { ESLintUtils, AST_NODE_TYPES } from '@typescript-eslint/utils'; +import { ESLintUtils, AST_NODE_TYPES, TSESTree } from '@typescript-eslint/utils'; // NOTE: The rule will be available in ESLint configs as "@nx/workspace-consistent-callback-type" export const RULE_NAME = 'consistent-callback-type'; From 1084d65b51be128fd7326cee1d564e9055e61a6f Mon Sep 17 00:00:00 2001 From: Martin Hochel Date: Tue, 26 May 2026 12:13:35 +0200 Subject: [PATCH 14/18] feat(eslint-rules): detect type-leakage through watched packages in consistent-base-hook Extend the rule's transitive analysis so type references in a base hook signature are also checked against forbidden runtimes when they originate from a watched package whose defining module reaches a forbidden runtime (via either value or type imports). - Track type-only imports from watched packages (previously skipped). - Per-Program cache now stores { value, all } reach sets, filled in a single DFS pass so the new analysis adds no extra resolution work. - visitScope picks the reach set based on reference.isTypeReference: value refs use value reach (runtime coupling), type refs use all reach (API-surface coupling). - allowTypeImports is now symmetric: when true, type-only imports are exempt from BOTH direct forbidden checks AND transitive watched reach checks. - New tests cover symmetric exemption, type-only watched -> clean module (allowed), and indirect type leakage via HeavyWrapper whose defining file value-re-exports the heavy module. --- .../stubs/watched-pkg/index.ts | 4 + .../rules/consistent-base-hook.spec.ts | 136 +++++- .../rules/consistent-base-hook.ts | 388 +++++++++++++----- 3 files changed, 411 insertions(+), 117 deletions(-) diff --git a/tools/eslint-rules/rules/__fixtures__/consistent-base-hook/stubs/watched-pkg/index.ts b/tools/eslint-rules/rules/__fixtures__/consistent-base-hook/stubs/watched-pkg/index.ts index f20537a7b393de..ecc54ec64c43da 100644 --- a/tools/eslint-rules/rules/__fixtures__/consistent-base-hook/stubs/watched-pkg/index.ts +++ b/tools/eslint-rules/rules/__fixtures__/consistent-base-hook/stubs/watched-pkg/index.ts @@ -1,5 +1,9 @@ +import type { HeavyType } from './heavy'; + export { useHeavy } from './heavy'; export { useLight } from './light'; export type { LightOptions } from './light'; // Re-export of a type-only thing from the heavy module — must not count as a runtime reach. export type { HeavyType } from './heavy'; + +export type HeavyWrapper = { tag: 'heavy-wrapper'; inner: HeavyType }; diff --git a/tools/eslint-rules/rules/consistent-base-hook.spec.ts b/tools/eslint-rules/rules/consistent-base-hook.spec.ts index 4319b36824f459..58ed8c66266e12 100644 --- a/tools/eslint-rules/rules/consistent-base-hook.spec.ts +++ b/tools/eslint-rules/rules/consistent-base-hook.spec.ts @@ -206,6 +206,26 @@ ruleTester.run(RULE_NAME, rule, { }, ], }, + // Referencing a watched-package binding inside a base hook without typed services available + // surfaces a one-shot `typedServicesUnavailable` diagnostic so the misconfiguration is visible. + { + code: ` + import * as React from 'react'; + import { useArrowNavigationGroup } from '@fluentui/react-tabster'; + export const useThingBase_unstable = (props, ref: React.Ref) => { + return useArrowNavigationGroup({}); + }; + `, + errors: [ + { + messageId: 'typedServicesUnavailable', + data: { + watchedPackages: '@fluentui/react-tabster', + forbiddenRuntimes: 'tabster', + }, + }, + ], + }, ], }); @@ -247,29 +267,17 @@ typedRuleTester.run(`${RULE_NAME} (typed)`, rule, { }; `, }, - // Type-only import of a symbol whose defining file reaches the forbidden runtime is allowed. + // Type-only import of a watched-package symbol whose defining file does NOT reach the + // forbidden runtime is allowed (`LightOptions` is defined in `light.ts` which only pulls + // `light-helper`). { languageOptions: typedLanguageOptions, filename: TYPED_FILENAME, options: transitiveOptions, code: ` import * as React from 'react'; - import type { HeavyType } from 'watched-pkg'; - export const useThingBase_unstable = (props: HeavyType, ref: React.Ref) => { - return { props, ref }; - }; - `, - }, - // Per-specifier type-only import is also OK. - { - languageOptions: typedLanguageOptions, - filename: TYPED_FILENAME, - options: transitiveOptions, - code: ` - import * as React from 'react'; - import { type HeavyType, useLight } from 'watched-pkg'; - export const useThingBase_unstable = (props: HeavyType, ref: React.Ref) => { - useLight(); + import type { LightOptions } from 'watched-pkg'; + export const useThingBase_unstable = (props: LightOptions, ref: React.Ref) => { return { props, ref }; }; `, @@ -333,6 +341,20 @@ typedRuleTester.run(`${RULE_NAME} (typed)`, rule, { }; `, }, + // Symmetric `allowTypeImports`: also exempts watched-package type-only imports whose defining + // module reaches a forbidden runtime (no runtime coupling is possible from a type). + { + languageOptions: typedLanguageOptions, + filename: TYPED_FILENAME, + options: transitiveOptionsAllowTypeImports, + code: ` + import * as React from 'react'; + import type { HeavyType } from 'watched-pkg'; + export const useThingBase_unstable = (props: HeavyType, ref: React.Ref) => { + return { props, ref }; + }; + `, + }, ], invalid: [ // Direct import from a forbidden-runtime package. @@ -452,5 +474,85 @@ typedRuleTester.run(`${RULE_NAME} (typed)`, rule, { }, ], }, + // Type-leakage through a watched package: a top-level `import type` of a watched-package + // symbol whose defining module transitively reaches the forbidden runtime is disallowed + // because the type still ties the base hook's public API to the forbidden runtime. + { + languageOptions: typedLanguageOptions, + filename: TYPED_FILENAME, + options: transitiveOptions, + code: ` + import * as React from 'react'; + import type { HeavyType } from 'watched-pkg'; + export const useThingBase_unstable = (props: HeavyType, ref: React.Ref) => { + return { props, ref }; + }; + `, + errors: [ + { + messageId: 'forbiddenRuntimeReach', + data: { + hookName: 'useThingBase_unstable', + importedName: 'HeavyType', + package: 'watched-pkg', + runtime: 'heavy-runtime', + viaFile: 'rules/__fixtures__/consistent-base-hook/stubs/watched-pkg/heavy.ts', + }, + }, + ], + }, + // Per-specifier `type` modifier variant of the same scenario. + { + languageOptions: typedLanguageOptions, + filename: TYPED_FILENAME, + options: transitiveOptions, + code: ` + import * as React from 'react'; + import { type HeavyType, useLight } from 'watched-pkg'; + export const useThingBase_unstable = (props: HeavyType, ref: React.Ref) => { + useLight(); + return { props, ref }; + }; + `, + errors: [ + { + messageId: 'forbiddenRuntimeReach', + data: { + hookName: 'useThingBase_unstable', + importedName: 'HeavyType', + package: 'watched-pkg', + runtime: 'heavy-runtime', + viaFile: 'rules/__fixtures__/consistent-base-hook/stubs/watched-pkg/heavy.ts', + }, + }, + ], + }, + // Indirect type leakage: `HeavyWrapper` is declared in `watched-pkg/index.ts` (not in `heavy.ts`), + // but its defining file value-re-exports `./heavy`, so the type-graph reach from `index.ts` still + // includes `heavy-runtime`. The base hook surface is therefore tied to the forbidden runtime. + { + languageOptions: typedLanguageOptions, + filename: TYPED_FILENAME, + options: transitiveOptions, + code: ` + import * as React from 'react'; + import type { HeavyWrapper } from 'watched-pkg'; + export const useThingBase_unstable = (props: HeavyWrapper, ref: React.Ref) => { + return { props, ref }; + }; + `, + errors: [ + { + messageId: 'forbiddenRuntimeReach', + data: { + hookName: 'useThingBase_unstable', + importedName: 'HeavyWrapper', + package: 'watched-pkg', + runtime: 'heavy-runtime', + viaFile: 'rules/__fixtures__/consistent-base-hook/stubs/watched-pkg/index.ts', + }, + }, + ], + }, ], }); diff --git a/tools/eslint-rules/rules/consistent-base-hook.ts b/tools/eslint-rules/rules/consistent-base-hook.ts index 712e96b424c37b..165582b0b8be1a 100644 --- a/tools/eslint-rules/rules/consistent-base-hook.ts +++ b/tools/eslint-rules/rules/consistent-base-hook.ts @@ -29,12 +29,13 @@ type Options = [ */ forbiddenRuntimes?: string[]; /** - * When `true`, type-only imports from `forbiddenRuntimes` packages are - * permitted inside base hooks (they emit no runtime code). + * When `true`, type-only imports (both from `forbiddenRuntimes` packages directly and + * from `watchedPackages` whose defining module reaches a forbidden runtime) are permitted + * inside base hooks. Type-only imports emit no runtime code, so this option trades API + * decoupling for ergonomics. * - * Defaults to `false` — type-only imports from forbidden runtimes are - * disallowed as well, to keep the base hook's public API fully decoupled - * from those packages. + * Defaults to `false` — type-only imports are checked the same way as value imports, to + * keep the base hook's public API fully decoupled from forbidden runtimes. */ allowTypeImports?: boolean; }?, @@ -45,7 +46,8 @@ type MessageIds = | 'invalidParamName' | 'invalidRefType' | 'forbiddenRuntimeDirect' - | 'forbiddenRuntimeReach'; + | 'forbiddenRuntimeReach' + | 'typedServicesUnavailable'; type ImportSpecifierNode = | TSESTree.ImportSpecifier @@ -59,6 +61,12 @@ interface TrackedImport { importedName: string; /** Kind of package — controls how the reference is checked. */ kind: 'watched' | 'forbidden'; + /** + * `true` when the binding is type-only (either the declaration is `import type ...` + * or the specifier is `import { type Foo }`). Used to gate whether direct usage in a + * value position is even possible (type-only bindings only surface in type positions). + */ + isTypeOnly: boolean; /** The specifier node (used for symbol lookup via ParserServices). */ specifier: ImportSpecifierNode; } @@ -66,13 +74,26 @@ interface TrackedImport { type BaseHookFunction = TSESTree.FunctionDeclaration | TSESTree.FunctionExpression | TSESTree.ArrowFunctionExpression; /** - * Per-Program cache: source file path → set of bare package specifiers transitively reached - * via value (non type-only) imports starting from that file. + * Result of a single transitive-reach DFS over a source file's import graph. + * - `value` — packages reachable via value (non type-only) imports only. Used to decide + * whether a runtime reference can pull a forbidden runtime at execution time. + * - `all` — packages reachable via value OR type imports. Used to decide whether a + * type reference can leak a forbidden runtime through the public API surface. + * `value` is always a subset of `all`. + */ +interface Reach { + value: ReadonlySet; + all: ReadonlySet; +} + +/** + * Per-Program cache: source file path → reach sets transitively computed from that file. + * Both `value` and `all` sets are filled in a single DFS pass to share resolution work. * * Keyed by `ts.Program` identity so the cache is invalidated whenever * typescript-eslint rebuilds the Program. */ -const programCache = new WeakMap>>(); +const programCache = new WeakMap>(); export const rule = ESLintUtils.RuleCreator(() => __filename)({ name: RULE_NAME, @@ -80,7 +101,7 @@ export const rule = ESLintUtils.RuleCreator(() => __filename)Base_unstable`): a required `props` parameter and an optional `ref` parameter typed as `React.Ref<...>`, and disallow referencing any binding whose defining module transitively pulls a forbidden runtime package (default `tabster`).', + 'Enforce the API contract for v9 "base" hooks (`useBase_unstable`): a required `props` parameter and an optional `ref` parameter typed as `React.Ref<...>`, and disallow referencing any binding whose defining module transitively pulls a forbidden runtime package (default `tabster`) \u2014 both at value positions (runtime coupling) and at type positions (API surface coupling).', }, schema: [ { @@ -113,6 +134,8 @@ export const rule = ESLintUtils.RuleCreator(() => __filename) __filename)([...watchedPackages, ...forbiddenRuntimes]); - // Variable → import origin. Keyed by Variable identity so shadowing inside the base hook is handled. + // Map of locally-declared variable identity → original import origin metadata. Keyed by Variable + // identity (not name) so re-declarations / shadowing inside the base hook resolve correctly. const trackedImports = new Map(); - // Lazily-acquired typed services. Untyped lint silently skips transitive analysis. + // Tracks whether `computeSymbolReach` was invoked while typed services were unavailable. When set, + // we emit a single diagnostic on `Program:exit` so the user knows transitive analysis was skipped. + let typedServicesNeededButMissing = false; + + // Lazily-acquired typed services. Resolved once per file, cached in `typedServices` (undefined = + // not yet attempted, null = attempted and unavailable, value = available). let typedServices: ParserServicesWithTypeInformation | null | undefined; + + /** + * Returns typed services (TS Program + checker) for the current file, or `null` if untyped + * lint is in effect. Result is memoized for the lifetime of the per-file rule instance. + */ function getTypedServices(): ParserServicesWithTypeInformation | null { if (typedServices !== undefined) { return typedServices; @@ -142,22 +176,31 @@ export const rule = ESLintUtils.RuleCreator(() => __filename) MAX_PARAM_COUNT) { + /** + * Validates the base-hook signature: 1 or 2 positional params, first must be Identifier `props`, + * optional second must be Identifier `ref` typed as `React.Ref<...>` (verified to originate from + * the `react` package so collisions with same-named locals don't pass). + */ + function checkParameters(hookName: string, hookFn: BaseHookFunction, reportNode: TSESTree.Node): void { + if (hookFn.params.length < MIN_PARAM_COUNT || hookFn.params.length > MAX_PARAM_COUNT) { context.report({ node: reportNode, messageId: 'invalidParamCount', - data: { hookName, actual: fn.params.length }, + data: { hookName, actual: hookFn.params.length }, }); return; } - fn.params.forEach((param, index) => { + hookFn.params.forEach((param, index) => { const expected = EXPECTED_PARAM_NAMES[index]; if (param.type !== AST_NODE_TYPES.Identifier || param.name !== expected) { context.report({ @@ -177,16 +220,31 @@ export const rule = ESLintUtils.RuleCreator(() => __filename) __filename) __filename) __filename) visitScope(child, fn, hookName)); + scope.childScopes.forEach(child => visitScope(child, hookFn, hookName)); } - function computeSymbolReach(origin: TrackedImport): { reached: ReadonlySet; viaFile: string } | null { + /** + * Resolves the watched-package import to its defining module via TS `Program`, then queries the + * transitive import graph for forbidden runtimes (both value-only and value+type sets). + * Returns `null` (and flips the `typedServicesNeededButMissing` flag) when typed services + * aren't available, so the caller can silently skip and we can warn once on `Program:exit`. + */ + function computeSymbolReach( + origin: TrackedImport, + ): { value: ReadonlySet; all: ReadonlySet; viaFile: string } | null { const services = getTypedServices(); if (!services) { + typedServicesNeededButMissing = true; return null; } const checker = services.program.getTypeChecker(); @@ -269,7 +344,7 @@ export const rule = ESLintUtils.RuleCreator(() => __filename) __filename) { - const specTypeOnly = specifier.type === AST_NODE_TYPES.ImportSpecifier && specifier.importKind === 'type'; - if (specTypeOnly && (!isForbiddenPkg || allowTypeImports)) { + const specTypeOnly = + stmtTypeOnly || (specifier.type === AST_NODE_TYPES.ImportSpecifier && specifier.importKind === 'type'); + if (specTypeOnly && allowTypeImports) { return; } const importedName = getImportedName(specifier); @@ -310,7 +395,13 @@ export const rule = ESLintUtils.RuleCreator(() => __filename) __filename) { + // `export default function () {}` produces an anonymous FunctionDeclaration (id === null). + // The esquery selector above requires `id.name`, so this branch should be unreachable in + // practice — kept as a type-narrowing guard so TS treats `node.id` as non-null below. if (!node.id) { return; } @@ -326,9 +420,17 @@ export const rule = ESLintUtils.RuleCreator(() => __filename) { + // The declarator's `id` can be a destructuring pattern (e.g. `const { useFooBase_unstable } = mod`). + // Such patterns never define a function literal we can inspect, and the selector only matched + // because esquery probed the nested Identifier name. Skip anything that isn't a plain Identifier. if (node.id.type !== AST_NODE_TYPES.Identifier) { return; } + // We only check inline function literals (`= (props, ref) => {}` or `= function () {}`) because + // the param-shape / body-reference analysis needs a function node we can walk. Any other Right Hand Side + // — missing initializer (`let useFooBase_unstable;`, ambient `declare const ... : Hook`), a call + // expression (`= wrap(impl)`), an identifier alias (`= someOtherHook`), `require(...)`, etc. — + // is out of scope for this rule. const init = node.init; if ( !init || @@ -338,106 +440,151 @@ export const rule = ESLintUtils.RuleCreator(() => __filename) { +function transitiveReach(program: ts.Program, sourceFile: ts.SourceFile): Reach { let cache = programCache.get(program); if (!cache) { cache = new Map(); programCache.set(program, cache); } - return computeReach(program, sf, cache, new Set()); + return computeReach(program, sourceFile, cache, new Set()); } +/** + * Recursive worker for `transitiveReach`. Walks the import graph DFS, recording every bare + * specifier encountered (separately for value-only vs value+type follow modes) and recursing into + * each resolved source file. Uses `inProgress` to break cycles (cycle hits return empty sets + * without caching, so the originating call still commits the complete result). + */ function computeReach( program: ts.Program, - sf: ts.SourceFile, - cache: Map>, + sourceFile: ts.SourceFile, + cache: Map, inProgress: Set, -): ReadonlySet { - const cached = cache.get(sf.fileName); +): Reach { + const cached = cache.get(sourceFile.fileName); if (cached) { return cached; } - if (inProgress.has(sf.fileName)) { - // Cycle: return an empty set without caching so the eventual full result is committed by the originator. - return EMPTY_SET; + if (inProgress.has(sourceFile.fileName)) { + // Cycle: return empty sets without caching so the eventual full result is committed by the originator. + return EMPTY_REACH; } - inProgress.add(sf.fileName); - - const result = new Set(); - for (const { specifier, literal } of collectValueImports(sf)) { - if (isBareSpecifier(specifier)) { - result.add(packageNameOf(specifier)); + inProgress.add(sourceFile.fileName); + + const value = new Set(); + const all = new Set(); + for (const imp of collectImports(sourceFile)) { + if (isBareSpecifier(imp.specifier)) { + const pkg = packageNameOf(imp.specifier); + all.add(pkg); + if (!imp.typeOnly) { + value.add(pkg); + } } - const resolved = resolveModule(program, sf, specifier, literal); + const resolved = resolveModule(program, sourceFile, imp.specifier, imp.literal); if (!resolved) { continue; } - const childSf = program.getSourceFile(resolved); - if (!childSf) { + const childSourceFile = program.getSourceFile(resolved); + if (!childSourceFile) { continue; } - const childReach = computeReach(program, childSf, cache, inProgress); - for (const pkg of childReach) { - result.add(pkg); + const childReach = computeReach(program, childSourceFile, cache, inProgress); + for (const pkg of childReach.all) { + all.add(pkg); + } + if (!imp.typeOnly) { + // A type-only edge does not propagate runtime reach: it can only widen the `all` set. + for (const pkg of childReach.value) { + value.add(pkg); + } } } - inProgress.delete(sf.fileName); - cache.set(sf.fileName, result); + inProgress.delete(sourceFile.fileName); + const result: Reach = { value, all }; + cache.set(sourceFile.fileName, result); return result; } -const EMPTY_SET: ReadonlySet = new Set(); +const EMPTY_REACH: Reach = { value: new Set(), all: new Set() }; -interface ValueImport { +interface ImportEdge { specifier: string; literal: ts.StringLiteralLike; + /** `true` when this edge only carries type information (no runtime side-effect). */ + typeOnly: boolean; } -function collectValueImports(sf: ts.SourceFile): ValueImport[] { - const result: ValueImport[] = []; - for (const stmt of sf.statements) { +/** + * Enumerates every module specifier in `sourceFile`, tagging each edge as value (`typeOnly: false`) + * or type-only (`typeOnly: true`). `import type` / `export type`, fully type-only named import or + * export clauses, and `import type =` are emitted with `typeOnly: true`. Side-effect imports + * (no clause) are emitted as value edges. + */ +function collectImports(sourceFile: ts.SourceFile): ImportEdge[] { + const result: ImportEdge[] = []; + for (const stmt of sourceFile.statements) { if (ts.isImportDeclaration(stmt)) { + let typeOnly = false; if (stmt.importClause?.isTypeOnly) { - continue; - } - // `import './side-effect'` (no clause) IS a value import. - // For named imports, drop if every specifier is type-only AND there's no default/namespace. - if (stmt.importClause && stmt.importClause.namedBindings && ts.isNamedImports(stmt.importClause.namedBindings)) { + typeOnly = true; + } else if ( + stmt.importClause && + stmt.importClause.namedBindings && + ts.isNamedImports(stmt.importClause.namedBindings) + ) { const named = stmt.importClause.namedBindings; - const hasValue = !!stmt.importClause.name || named.elements.some(el => !el.isTypeOnly); - if (!hasValue) { - continue; - } + const hasValue = !!stmt.importClause.name || named.elements.some(element => !element.isTypeOnly); + typeOnly = !hasValue; } if (ts.isStringLiteralLike(stmt.moduleSpecifier)) { - result.push({ specifier: stmt.moduleSpecifier.text, literal: stmt.moduleSpecifier }); + result.push({ specifier: stmt.moduleSpecifier.text, literal: stmt.moduleSpecifier, typeOnly }); } continue; } if (ts.isExportDeclaration(stmt) && stmt.moduleSpecifier && ts.isStringLiteralLike(stmt.moduleSpecifier)) { - if (stmt.isTypeOnly) { - continue; - } - if (stmt.exportClause && ts.isNamedExports(stmt.exportClause)) { - const allTypeOnly = stmt.exportClause.elements.every(el => el.isTypeOnly); - if (allTypeOnly) { - continue; - } + let typeOnly = stmt.isTypeOnly; + if (!typeOnly && stmt.exportClause && ts.isNamedExports(stmt.exportClause)) { + typeOnly = stmt.exportClause.elements.every(element => element.isTypeOnly); } - result.push({ specifier: stmt.moduleSpecifier.text, literal: stmt.moduleSpecifier }); + result.push({ specifier: stmt.moduleSpecifier.text, literal: stmt.moduleSpecifier, typeOnly }); continue; } if ( @@ -445,21 +592,25 @@ function collectValueImports(sf: ts.SourceFile): ValueImport[] { ts.isExternalModuleReference(stmt.moduleReference) && ts.isStringLiteralLike(stmt.moduleReference.expression) ) { - if (stmt.isTypeOnly) { - continue; - } result.push({ specifier: stmt.moduleReference.expression.text, literal: stmt.moduleReference.expression, + typeOnly: stmt.isTypeOnly, }); } } return result; } +/** + * Resolves `specifier` (as used in `sourceFile`) to an absolute file path using the same algorithm + * the host TS Program uses. Prefers the (faster) `program.getResolvedModule` API exposed in TS ≥ 5.3 + * and falls back to `ts.resolveModuleName` for older toolchains. Returns `undefined` if the module + * cannot be resolved (e.g. ambient declarations, broken paths). + */ function resolveModule( program: ts.Program, - sf: ts.SourceFile, + sourceFile: ts.SourceFile, specifier: string, literal: ts.StringLiteralLike, ): string | undefined { @@ -477,12 +628,12 @@ function resolveModule( ts as unknown as { getModeForUsageLocation?: (file: ts.SourceFile, usage: ts.StringLiteralLike) => ts.ResolutionMode; } - ).getModeForUsageLocation?.(sf, literal); + ).getModeForUsageLocation?.(sourceFile, literal); if (typeof getResolvedModule === 'function') { - const r = getResolvedModule.call(program, sf, specifier, mode); - if (r?.resolvedModule) { - return r.resolvedModule.resolvedFileName; + const resolutionResult = getResolvedModule.call(program, sourceFile, specifier, mode); + if (resolutionResult?.resolvedModule) { + return resolutionResult.resolvedModule.resolvedFileName; } } @@ -492,7 +643,7 @@ function resolveModule( (program as unknown as { getCompilerHost?: () => ts.ModuleResolutionHost }).getCompilerHost?.() ?? ts.sys; const result = ts.resolveModuleName( specifier, - sf.fileName, + sourceFile.fileName, compilerOptions, host as ts.ModuleResolutionHost, undefined, @@ -502,10 +653,18 @@ function resolveModule( return result.resolvedModule?.resolvedFileName; } +/** + * `true` when the import specifier refers to a package (e.g. `react`, `@scope/pkg`, `pkg/sub`) + * rather than a relative or absolute path. + */ function isBareSpecifier(specifier: string): boolean { return !specifier.startsWith('.') && !specifier.startsWith('/'); } +/** + * Extracts the npm package name from a bare specifier. Handles both unscoped (`pkg/sub` → `pkg`) + * and scoped (`@scope/pkg/sub` → `@scope/pkg`) forms. + */ function packageNameOf(specifier: string): string { if (specifier.startsWith('@')) { const [scope, name] = specifier.split('/', 2); @@ -515,6 +674,11 @@ function packageNameOf(specifier: string): string { return slash === -1 ? specifier : specifier.slice(0, slash); } +/** + * Shortens an absolute file path for display in diagnostics: returns the part after the last + * `node_modules/` segment when present (so users see e.g. `tabster/dist/index.js`), or makes the + * path workspace-relative when inside the current working directory. + */ function shortenPath(absolute: string): string { const marker = '/node_modules/'; const idx = absolute.lastIndexOf(marker); @@ -552,6 +716,11 @@ function getImportedName(specifier: ImportSpecifierNode): string | undefined { } } +/** + * Returns a human-readable label for a function parameter, used in `invalidParamName` diagnostics + * so the user sees what they actually wrote (destructuring, rest, default value, …) instead of + * just `Identifier`. + */ function describeParam(param: TSESTree.Parameter): string { switch (param.type) { case AST_NODE_TYPES.Identifier: @@ -569,6 +738,11 @@ function describeParam(param: TSESTree.Parameter): string { } } +/** + * Returns `true` when `annotation` is `React.Ref<...>` (qualified) or `Ref<...>` (named) AND the + * referenced identifier was imported from the `react` package in the surrounding scope. The scope + * check guards against false positives when a local `Ref` shadows the React import. + */ function isReactRefTypeAnnotation( annotation: TSESTree.TSTypeAnnotation | undefined, scope: TSESLint.Scope.Scope, @@ -644,6 +818,11 @@ function isReactImportedIdentifier( }); } +/** + * Walks the scope chain looking for a variable with the given name. Plain `scope.set.get` only + * inspects the local scope, so this helper enables identifier resolution that matches JavaScript's + * lookup semantics. + */ function findVariableInScope(scope: TSESLint.Scope.Scope, name: string): TSESLint.Scope.Variable | undefined { let current: TSESLint.Scope.Scope | null = scope; while (current) { @@ -656,6 +835,10 @@ function findVariableInScope(scope: TSESLint.Scope.Scope, name: string): TSESLin return undefined; } +/** + * Renders the actual ref type annotation as a string for `invalidRefType` diagnostics, so users + * see what they wrote (`HTMLAttributes`, `Ref`, `MyType`…) instead of bare AST node types. + */ function describeRefType(annotation: TSESTree.TSTypeAnnotation | undefined): string { if (!annotation) { return ''; @@ -675,10 +858,15 @@ function describeRefType(annotation: TSESTree.TSTypeAnnotation | undefined): str return type.type; } -function isScopeWithinFunction(scope: TSESLint.Scope.Scope, fn: BaseHookFunction): boolean { +/** + * `true` when `scope` (or any of its ancestor scopes) is the function scope of `hookFn`. Used to + * confine the body-reference walk to the base hook itself — references in sibling functions are + * out of scope for this rule. + */ +function isScopeWithinFunction(scope: TSESLint.Scope.Scope, hookFn: BaseHookFunction): boolean { let current: TSESLint.Scope.Scope | null = scope; while (current) { - if (current.block === fn) { + if (current.block === hookFn) { return true; } current = current.upper; From f441aaf8f194a336236838552d16f9382c38b282 Mon Sep 17 00:00:00 2001 From: Martin Hochel Date: Tue, 26 May 2026 15:02:00 +0200 Subject: [PATCH 15/18] feat(eslint-rules): extend consistent-base-hook with paired state hook signature detection Wrapping state hooks (`use_unstable`) that pair with a base hook (`useBase_unstable`) must also follow the `(props, ref)` signature contract. Pair detection checks the same file first, then sibling files (`.ts`/`.tsx`) in the same directory. Suppress the violation on `useTagGroupBase_unstable`, which takes a third `options` argument used internally by `useTagGroup_unstable`. --- .../src/components/TagGroup/useTagGroup.ts | 1 + .../src/components/Sibling/useSiblingBase.ts | 7 + .../rules/consistent-base-hook.spec.ts | 70 +++++++- .../rules/consistent-base-hook.ts | 164 +++++++++++++++--- 4 files changed, 217 insertions(+), 25 deletions(-) create mode 100644 tools/eslint-rules/rules/__fixtures__/consistent-base-hook/src/components/Sibling/useSiblingBase.ts diff --git a/packages/react-components/react-tags/library/src/components/TagGroup/useTagGroup.ts b/packages/react-components/react-tags/library/src/components/TagGroup/useTagGroup.ts index e6b0b19aca97f1..79ace1ae06ba86 100644 --- a/packages/react-components/react-tags/library/src/components/TagGroup/useTagGroup.ts +++ b/packages/react-components/react-tags/library/src/components/TagGroup/useTagGroup.ts @@ -27,6 +27,7 @@ type UseTagGroupBaseOptions = { * @param props - props from this instance of TagGroup (without appearance, size) * @param ref - reference to root HTMLDivElement of TagGroup */ +// eslint-disable-next-line @nx/workspace-consistent-base-hook -- accepts an extra `options` arg used internally by `useTagGroup_unstable` to coordinate focus after a tag is dismissed export const useTagGroupBase_unstable = ( props: TagGroupBaseProps, ref: React.Ref, diff --git a/tools/eslint-rules/rules/__fixtures__/consistent-base-hook/src/components/Sibling/useSiblingBase.ts b/tools/eslint-rules/rules/__fixtures__/consistent-base-hook/src/components/Sibling/useSiblingBase.ts new file mode 100644 index 00000000000000..52eaf50766185d --- /dev/null +++ b/tools/eslint-rules/rules/__fixtures__/consistent-base-hook/src/components/Sibling/useSiblingBase.ts @@ -0,0 +1,7 @@ +import * as React from 'react'; + +// Sibling base hook used to verify cross-file pair detection in the rule's spec. +// The wrapping state hook lives in `useSibling.ts` next to this file. +export const useSiblingBase_unstable = (props: { a: number }, ref: React.Ref) => { + return { props, ref }; +}; diff --git a/tools/eslint-rules/rules/consistent-base-hook.spec.ts b/tools/eslint-rules/rules/consistent-base-hook.spec.ts index 58ed8c66266e12..ab03e0222861e9 100644 --- a/tools/eslint-rules/rules/consistent-base-hook.spec.ts +++ b/tools/eslint-rules/rules/consistent-base-hook.spec.ts @@ -4,6 +4,8 @@ import { rule, RULE_NAME } from './consistent-base-hook'; const FIXTURE_ROOT = path.join(__dirname, '__fixtures__/consistent-base-hook'); const TYPED_FILENAME = 'src/test.ts'; +const SIBLING_FILENAME = path.join(FIXTURE_ROOT, 'src/components/Sibling/useSibling.ts'); +const ORPHAN_FILENAME = path.join(FIXTURE_ROOT, 'src/components/Orphan/useOrphan.ts'); const typedLanguageOptions = { parserOptions: { @@ -62,12 +64,47 @@ ruleTester.run(RULE_NAME, rule, { }; `, }, + // Pair detection (same file): a state hook `useThing_unstable` next to its base hook + // `useThingBase_unstable` IS subject to the contract. Correct signature passes. + { + code: ` + import * as React from 'react'; + export const useThing_unstable = (props, ref: React.Ref) => { + return { props, ref }; + }; + export const useThingBase_unstable = (props, ref: React.Ref) => { + return { props, ref }; + }; + `, + }, + // Pair detection (no sibling base hook on disk): `useOrphanContextValues_unstable(state)` + // is NOT a paired wrapping hook and must NOT be flagged for its non-(props, ref) signature. + // The Orphan folder has no `useOrphanContextValuesBase.ts(x)` next to it. + { + filename: ORPHAN_FILENAME, + code: ` + export function useOrphanContextValues_unstable(state) { + return { state }; + } + `, + }, + // Pair detection (sibling file): wrapping state hook lives in `useSibling.ts`, paired with + // `useSiblingBase.ts` in the same folder. Correct (props, ref) signature passes. + { + filename: SIBLING_FILENAME, + code: ` + import * as React from 'react'; + export const useSibling_unstable = (props, ref: React.Ref) => { + return { props, ref }; + }; + `, + }, // Identifier with the same local name as a forbidden import alias does not collide via scope analysis. { code: ` import * as React from 'react'; import { useArrowNavigationGroup } from '@fluentui/react-tabster'; - export const useThing_unstable = (props, ref) => { + export const useThing_unstable = (props, ref: React.Ref) => { return useArrowNavigationGroup({}); }; export const useThingBase_unstable = (props, ref: React.Ref) => { @@ -226,6 +263,35 @@ ruleTester.run(RULE_NAME, rule, { }, ], }, + // Pair detection (same file): wrapping state hook with too many params is flagged because + // its sibling base hook in the same file marks it as a paired wrapper. + { + code: ` + import * as React from 'react'; + export const useThing_unstable = (props, ref, extra) => ({ props, ref, extra }); + export const useThingBase_unstable = (props, ref: React.Ref) => ({ props, ref }); + `, + errors: [{ messageId: 'invalidParamCount', data: { hookName: 'useThing_unstable', actual: 3 } }], + }, + // Pair detection (sibling file): wrapping state hook in `useSibling.ts` is paired with + // `useSiblingBase.ts` in the same folder. Wrong param names are flagged. + { + filename: SIBLING_FILENAME, + code: ` + import * as React from 'react'; + export const useSibling_unstable = (p, r: React.Ref) => ({ p, r }); + `, + errors: [ + { + messageId: 'invalidParamName', + data: { hookName: 'useSibling_unstable', index: 1, expected: 'props', actual: 'p' }, + }, + { + messageId: 'invalidParamName', + data: { hookName: 'useSibling_unstable', index: 2, expected: 'ref', actual: 'r' }, + }, + ], + }, ], }); @@ -293,7 +359,7 @@ typedRuleTester.run(`${RULE_NAME} (typed)`, rule, { export const useThingBase_unstable = (props: { a: number }, ref: React.Ref) => { return { props, ref }; }; - export const useThing_unstable = (props, ref) => { + export const useThing_unstable = (props, ref: React.Ref) => { return useHeavy(); }; `, diff --git a/tools/eslint-rules/rules/consistent-base-hook.ts b/tools/eslint-rules/rules/consistent-base-hook.ts index 165582b0b8be1a..81da34002f77f3 100644 --- a/tools/eslint-rules/rules/consistent-base-hook.ts +++ b/tools/eslint-rules/rules/consistent-base-hook.ts @@ -1,11 +1,20 @@ import type { TSESTree, TSESLint, ParserServicesWithTypeInformation } from '@typescript-eslint/utils'; import { ESLintUtils, AST_NODE_TYPES } from '@typescript-eslint/utils'; import * as ts from 'typescript'; +import * as fs from 'node:fs'; +import * as path from 'node:path'; // NOTE: The rule will be available in ESLint configs as "@nx/workspace-consistent-base-hook" export const RULE_NAME = 'consistent-base-hook'; const BASE_HOOK_NAME_PATTERN = /^use[A-Z]\w*Base_unstable$/; +// State hook candidates are any `_unstable` hook NOT ending in `Base_unstable`. They are only +// subjected to the signature contract when they are paired with a sibling base hook (Option C — +// pair detection). See `hasPairedBaseHook`. +const STATE_HOOK_NAME_PATTERN = /^use[A-Z]\w*_unstable$/; +const BASE_SUFFIX = 'Base_unstable'; +const UNSTABLE_SUFFIX = '_unstable'; +const SIBLING_EXTENSIONS: ReadonlyArray = ['.ts', '.tsx']; const EXPECTED_PARAM_NAMES = ['props', 'ref'] as const; const MIN_PARAM_COUNT = 1; const MAX_PARAM_COUNT = 2; @@ -101,7 +110,7 @@ export const rule = ESLintUtils.RuleCreator(() => __filename)Base_unstable`): a required `props` parameter and an optional `ref` parameter typed as `React.Ref<...>`, and disallow referencing any binding whose defining module transitively pulls a forbidden runtime package (default `tabster`) \u2014 both at value positions (runtime coupling) and at type positions (API surface coupling).', + 'Enforce the API contract for v9 "base" hooks (`useBase_unstable`) and their paired wrapping state hooks (`use_unstable` declared in the same file or sibling component-folder file): a required `props` parameter and an optional `ref` parameter typed as `React.Ref<...>`. Additionally, disallow inside base hooks any binding whose defining module transitively pulls a forbidden runtime package (default `tabster`) \u2014 both at value positions (runtime coupling) and at type positions (API surface coupling).', }, schema: [ { @@ -126,10 +135,10 @@ export const rule = ESLintUtils.RuleCreator(() => __filename)`, got `{{actual}}`.', + 'Hook `{{hookName}}` parameter #{{index}} must be named `{{expected}}` (Identifier), got `{{actual}}`.', + invalidRefType: 'Hook `{{hookName}}` parameter `ref` must be typed as `React.Ref<...>`, got `{{actual}}`.', forbiddenRuntimeDirect: 'Base hook `{{hookName}}` cannot reference `{{importedName}}` from forbidden runtime package `{{package}}`. Move logic that depends on `{{package}}` to the wrapping `*_unstable` hook instead.', forbiddenRuntimeReach: @@ -152,6 +161,17 @@ export const rule = ESLintUtils.RuleCreator(() => __filename)(); + // Names of base hooks (`useBase_unstable`) declared at the top level of the current file. + // Populated on `Program` enter so the state-hook visitor can synchronously decide pairing for + // the same-file (82 / 85) case without re-scanning. + const baseHooksInCurrentFile = new Set(); + + // Caches sibling-file existence checks (`./useXBase.ts` / `./useXBase.tsx`) for the lifetime of + // this rule instance. The keys are absolute candidate paths; values are `true` if the file exists + // on disk. Used to cover the 3 outliers where the base hook and wrapping hook live in sibling + // files within the same component folder (Tooltip, Field, MenuItem). + const siblingFileExistsCache = new Map(); + // Tracks whether `computeSymbolReach` was invoked while typed services were unavailable. When set, // we emit a single diagnostic on `Program:exit` so the user knows transitive analysis was skipped. let typedServicesNeededButMissing = false; @@ -185,6 +205,50 @@ export const rule = ESLintUtils.RuleCreator(() => __filename)/useXBase.ts(x)`). + * + * The base hook is the structural marker for the `(props, ref)` contract. When found, the + * wrapping state hook is required to honor the same contract; otherwise it is left alone, + * which avoids false positives on unrelated `_unstable` hooks such as + * `useFooContextValues_unstable`, `useFooStyles_unstable`, etc. + */ + function hasPairedBaseHook(stateHookName: string): boolean { + const baseHookName = stateHookName.slice(0, -UNSTABLE_SUFFIX.length) + BASE_SUFFIX; + if (baseHooksInCurrentFile.has(baseHookName)) { + return true; + } + const filename = context.filename; + // ESLint passes synthetic filenames like `` for inline code; nothing to check. + if (!filename || !path.isAbsolute(filename)) { + return false; + } + const dir = path.dirname(filename); + // Sibling base-hook file (the wrapping hook lives in `useFoo.tsx`, the base in `useFooBase.tsx`). + const siblingBasename = baseHookName.slice(0, -UNSTABLE_SUFFIX.length); // e.g. `useFooBase` + for (const ext of SIBLING_EXTENSIONS) { + const candidate = path.join(dir, siblingBasename + ext); + if (candidate === filename) { + continue; + } + let exists = siblingFileExistsCache.get(candidate); + if (exists === undefined) { + try { + exists = fs.statSync(candidate).isFile(); + } catch { + exists = false; + } + siblingFileExistsCache.set(candidate, exists); + } + if (exists) { + return true; + } + } + return false; + } + /** * Validates the base-hook signature: 1 or 2 positional params, first must be Identifier `props`, * optional second must be Identifier `ref` typed as `React.Ref<...>` (verified to originate from @@ -406,39 +470,65 @@ export const rule = ESLintUtils.RuleCreator(() => __filename) { + // Broader selector matches both base hooks and state-hook candidates; the handler dispatches + // by name. State hooks only get the signature check when paired with a sibling base hook. + [`FunctionDeclaration[id.name=/${STATE_HOOK_NAME_PATTERN.source}/]`]: (node: TSESTree.FunctionDeclaration) => { // `export default function () {}` produces an anonymous FunctionDeclaration (id === null). // The esquery selector above requires `id.name`, so this branch should be unreachable in // practice — kept as a type-narrowing guard so TS treats `node.id` as non-null below. if (!node.id) { return; } - checkBaseHook(node.id.name, node, node.id); + const name = node.id.name; + if (BASE_HOOK_NAME_PATTERN.test(name)) { + checkBaseHook(name, node, node.id); + } else if (hasPairedBaseHook(name)) { + checkParameters(name, node, node.id); + } }, - [`VariableDeclarator[id.name=/${BASE_HOOK_NAME_PATTERN.source}/]`]: (node: TSESTree.VariableDeclarator) => { - // The declarator's `id` can be a destructuring pattern (e.g. `const { useFooBase_unstable } = mod`). - // Such patterns never define a function literal we can inspect, and the selector only matched - // because esquery probed the nested Identifier name. Skip anything that isn't a plain Identifier. - if (node.id.type !== AST_NODE_TYPES.Identifier) { + [`VariableDeclarator[id.name=/${STATE_HOOK_NAME_PATTERN.source}/]`]: (node: TSESTree.VariableDeclarator) => { + const init = getFunctionInit(node); + if (!init || node.id.type !== AST_NODE_TYPES.Identifier) { return; } - // We only check inline function literals (`= (props, ref) => {}` or `= function () {}`) because - // the param-shape / body-reference analysis needs a function node we can walk. Any other Right Hand Side - // — missing initializer (`let useFooBase_unstable;`, ambient `declare const ... : Hook`), a call - // expression (`= wrap(impl)`), an identifier alias (`= someOtherHook`), `require(...)`, etc. — - // is out of scope for this rule. - const init = node.init; - if ( - !init || - (init.type !== AST_NODE_TYPES.ArrowFunctionExpression && init.type !== AST_NODE_TYPES.FunctionExpression) - ) { - return; + const name = node.id.name; + if (BASE_HOOK_NAME_PATTERN.test(name)) { + checkBaseHook(name, init, node.id); + } else if (hasPairedBaseHook(name)) { + checkParameters(name, init, node.id); } - checkBaseHook(node.id.name, init, node.id); }, /** @@ -696,6 +786,34 @@ function shortenPath(absolute: string): string { // AST helpers (unchanged from previous version) // --------------------------------------------------------------------------- +/** + * Collects names of top-level declarations matching `BASE_HOOK_NAME_PATTERN` into `out`. + * Handles both `export const useFooBase_unstable = ...` (incl. `export const` chains) and + * `export function useFooBase_unstable() {}`, plus the unexported / `export { ... }` forms. + */ +function collectBaseHookNames(stmt: TSESTree.Node, out: Set): void { + // `export const useFooBase_unstable = ...` / `export function useFooBase_unstable() {}` + if (stmt.type === AST_NODE_TYPES.ExportNamedDeclaration && stmt.declaration) { + collectBaseHookNames(stmt.declaration, out); + return; + } + // `function useFooBase_unstable() {}` + if (stmt.type === AST_NODE_TYPES.FunctionDeclaration) { + if (stmt.id && BASE_HOOK_NAME_PATTERN.test(stmt.id.name)) { + out.add(stmt.id.name); + } + return; + } + // `const useFooBase_unstable = ...` (incl. multi-declarator forms) + if (stmt.type === AST_NODE_TYPES.VariableDeclaration) { + for (const decl of stmt.declarations) { + if (decl.id.type === AST_NODE_TYPES.Identifier && BASE_HOOK_NAME_PATTERN.test(decl.id.name)) { + out.add(decl.id.name); + } + } + } +} + /** * Resolves the original (imported) name from an import specifier. * Returns `'default'` for default imports, `'*'` for namespace imports, From 4dd78a0bc5c962ab873e6689a5e247177abb0c76 Mon Sep 17 00:00:00 2001 From: Martin Hochel Date: Tue, 26 May 2026 15:22:38 +0200 Subject: [PATCH 16/18] refactor(eslint-rules): split consistent-base-hook into base-hook-signature + base-hook-no-forbidden-runtime MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Replaces the single `@nx/workspace-consistent-base-hook` rule with two focused rules: - `@nx/workspace-base-hook-signature` (untyped) — enforces the API contract for base hooks: 1–2 positional parameters named `props` and optional `ref`, with `ref` typed as `React.Ref<...>`. Detects targets by name pattern (`use*Base_unstable`) OR by pair detection — when a `use_unstable` exists alongside a co-located or sibling-file `useBase_unstable`, the state hook is also validated. - `@nx/workspace-base-hook-no-forbidden-runtime` (type-aware) — enforces that base hooks do not transitively pull forbidden runtime packages (default `tabster`) directly or via configured watched packages (default `@fluentui/react-tabster`). Extracted shared utilities: - `utils/tracked-imports.ts` — import-specifier metadata and naming. - `utils/module-resolver.ts` — TS module resolution + bare-spec helpers. - `utils/transitive-reach.ts` — per-program cached value/all reach analysis with cycle handling. - `utils/base-hook-detector.ts` — name patterns, base/state pairing, same-file + sibling-file detection. Other changes: - `packages/eslint-plugin/src/internal.js` — register both new rules in place of the old one. - `packages/react-components/react-tags/.../useTagGroup.ts` — suppression comment updated to point at the new `base-hook-signature` rule name. - Fixture dir renamed `__fixtures__/consistent-base-hook/` → `__fixtures__/base-hook-no-forbidden-runtime/`. Signature fixture lives at `__fixtures__/base-hook-signature/`. Tests: 76/76 pass (5 suites). vNext lint sweep clean. --- packages/eslint-plugin/src/internal.js | 3 +- .../src/components/TagGroup/useTagGroup.ts | 2 +- tools/eslint-rules/index.ts | 9 +- .../src/dummy.ts | 0 .../src/test.ts | 0 .../stubs/cyclic-pkg/a.ts | 0 .../stubs/cyclic-pkg/b.ts | 0 .../stubs/cyclic-pkg/index.ts | 0 .../stubs/heavy-runtime/index.ts | 0 .../stubs/light-helper/index.ts | 0 .../stubs/watched-pkg/heavy.ts | 0 .../stubs/watched-pkg/index.ts | 0 .../stubs/watched-pkg/light.ts | 0 .../tsconfig.json | 0 .../src/components/Sibling/useSiblingBase.ts | 3 + .../src/components/Sibling/useSiblingBase.ts | 7 - ...=> base-hook-no-forbidden-runtime.spec.ts} | 304 +----- .../rules/base-hook-no-forbidden-runtime.ts | 362 +++++++ .../rules/base-hook-signature.spec.ts | 241 +++++ .../eslint-rules/rules/base-hook-signature.ts | 263 +++++ .../rules/consistent-base-hook.ts | 993 ------------------ .../eslint-rules/utils/base-hook-detector.ts | 140 +++ tools/eslint-rules/utils/module-resolver.ts | 91 ++ tools/eslint-rules/utils/tracked-imports.ts | 53 + tools/eslint-rules/utils/transitive-reach.ts | 160 +++ 25 files changed, 1357 insertions(+), 1274 deletions(-) rename tools/eslint-rules/rules/__fixtures__/{consistent-base-hook => base-hook-no-forbidden-runtime}/src/dummy.ts (100%) rename tools/eslint-rules/rules/__fixtures__/{consistent-base-hook => base-hook-no-forbidden-runtime}/src/test.ts (100%) rename tools/eslint-rules/rules/__fixtures__/{consistent-base-hook => base-hook-no-forbidden-runtime}/stubs/cyclic-pkg/a.ts (100%) rename tools/eslint-rules/rules/__fixtures__/{consistent-base-hook => base-hook-no-forbidden-runtime}/stubs/cyclic-pkg/b.ts (100%) rename tools/eslint-rules/rules/__fixtures__/{consistent-base-hook => base-hook-no-forbidden-runtime}/stubs/cyclic-pkg/index.ts (100%) rename tools/eslint-rules/rules/__fixtures__/{consistent-base-hook => base-hook-no-forbidden-runtime}/stubs/heavy-runtime/index.ts (100%) rename tools/eslint-rules/rules/__fixtures__/{consistent-base-hook => base-hook-no-forbidden-runtime}/stubs/light-helper/index.ts (100%) rename tools/eslint-rules/rules/__fixtures__/{consistent-base-hook => base-hook-no-forbidden-runtime}/stubs/watched-pkg/heavy.ts (100%) rename tools/eslint-rules/rules/__fixtures__/{consistent-base-hook => base-hook-no-forbidden-runtime}/stubs/watched-pkg/index.ts (100%) rename tools/eslint-rules/rules/__fixtures__/{consistent-base-hook => base-hook-no-forbidden-runtime}/stubs/watched-pkg/light.ts (100%) rename tools/eslint-rules/rules/__fixtures__/{consistent-base-hook => base-hook-no-forbidden-runtime}/tsconfig.json (100%) create mode 100644 tools/eslint-rules/rules/__fixtures__/base-hook-signature/src/components/Sibling/useSiblingBase.ts delete mode 100644 tools/eslint-rules/rules/__fixtures__/consistent-base-hook/src/components/Sibling/useSiblingBase.ts rename tools/eslint-rules/rules/{consistent-base-hook.spec.ts => base-hook-no-forbidden-runtime.spec.ts} (54%) create mode 100644 tools/eslint-rules/rules/base-hook-no-forbidden-runtime.ts create mode 100644 tools/eslint-rules/rules/base-hook-signature.spec.ts create mode 100644 tools/eslint-rules/rules/base-hook-signature.ts delete mode 100644 tools/eslint-rules/rules/consistent-base-hook.ts create mode 100644 tools/eslint-rules/utils/base-hook-detector.ts create mode 100644 tools/eslint-rules/utils/module-resolver.ts create mode 100644 tools/eslint-rules/utils/tracked-imports.ts create mode 100644 tools/eslint-rules/utils/transitive-reach.ts diff --git a/packages/eslint-plugin/src/internal.js b/packages/eslint-plugin/src/internal.js index 19afbe17a62174..323c79840c04d8 100644 --- a/packages/eslint-plugin/src/internal.js +++ b/packages/eslint-plugin/src/internal.js @@ -35,7 +35,8 @@ const __internal = { /** @type {import('eslint').Linter.RulesRecord} */ rules: { '@nx/workspace-consistent-callback-type': 'error', - '@nx/workspace-consistent-base-hook': 'error', + '@nx/workspace-base-hook-signature': 'error', + '@nx/workspace-base-hook-no-forbidden-runtime': 'error', '@nx/workspace-no-restricted-globals': restrictedGlobals.react, '@nx/workspace-no-missing-jsx-pragma': ['error', { runtime: 'automatic' }], }, diff --git a/packages/react-components/react-tags/library/src/components/TagGroup/useTagGroup.ts b/packages/react-components/react-tags/library/src/components/TagGroup/useTagGroup.ts index 79ace1ae06ba86..95d485df9910e2 100644 --- a/packages/react-components/react-tags/library/src/components/TagGroup/useTagGroup.ts +++ b/packages/react-components/react-tags/library/src/components/TagGroup/useTagGroup.ts @@ -27,7 +27,7 @@ type UseTagGroupBaseOptions = { * @param props - props from this instance of TagGroup (without appearance, size) * @param ref - reference to root HTMLDivElement of TagGroup */ -// eslint-disable-next-line @nx/workspace-consistent-base-hook -- accepts an extra `options` arg used internally by `useTagGroup_unstable` to coordinate focus after a tag is dismissed +// eslint-disable-next-line @nx/workspace-base-hook-signature -- accepts an extra `options` arg used internally by `useTagGroup_unstable` to coordinate focus after a tag is dismissed export const useTagGroupBase_unstable = ( props: TagGroupBaseProps, ref: React.Ref, diff --git a/tools/eslint-rules/index.ts b/tools/eslint-rules/index.ts index 8e6f5eca37fd1f..b0ed1faac661ed 100644 --- a/tools/eslint-rules/index.ts +++ b/tools/eslint-rules/index.ts @@ -4,7 +4,11 @@ import { RULE_NAME as consistentCallbackTypeName, rule as consistentCallbackType, } from './rules/consistent-callback-type'; -import { RULE_NAME as consistentBaseHookName, rule as consistentBaseHook } from './rules/consistent-base-hook'; +import { RULE_NAME as baseHookSignatureName, rule as baseHookSignature } from './rules/base-hook-signature'; +import { + RULE_NAME as baseHookNoForbiddenRuntimeName, + rule as baseHookNoForbiddenRuntime, +} from './rules/base-hook-no-forbidden-runtime'; /** * Import your custom workspace rules at the top of this file. @@ -33,7 +37,8 @@ module.exports = { */ rules: { [consistentCallbackTypeName]: consistentCallbackType, - [consistentBaseHookName]: consistentBaseHook, + [baseHookSignatureName]: baseHookSignature, + [baseHookNoForbiddenRuntimeName]: baseHookNoForbiddenRuntime, [noRestrictedGlobalsName]: noRestrictedGlobals, [noMissingJsxPragmaName]: noMissingJsxPragma, }, diff --git a/tools/eslint-rules/rules/__fixtures__/consistent-base-hook/src/dummy.ts b/tools/eslint-rules/rules/__fixtures__/base-hook-no-forbidden-runtime/src/dummy.ts similarity index 100% rename from tools/eslint-rules/rules/__fixtures__/consistent-base-hook/src/dummy.ts rename to tools/eslint-rules/rules/__fixtures__/base-hook-no-forbidden-runtime/src/dummy.ts diff --git a/tools/eslint-rules/rules/__fixtures__/consistent-base-hook/src/test.ts b/tools/eslint-rules/rules/__fixtures__/base-hook-no-forbidden-runtime/src/test.ts similarity index 100% rename from tools/eslint-rules/rules/__fixtures__/consistent-base-hook/src/test.ts rename to tools/eslint-rules/rules/__fixtures__/base-hook-no-forbidden-runtime/src/test.ts diff --git a/tools/eslint-rules/rules/__fixtures__/consistent-base-hook/stubs/cyclic-pkg/a.ts b/tools/eslint-rules/rules/__fixtures__/base-hook-no-forbidden-runtime/stubs/cyclic-pkg/a.ts similarity index 100% rename from tools/eslint-rules/rules/__fixtures__/consistent-base-hook/stubs/cyclic-pkg/a.ts rename to tools/eslint-rules/rules/__fixtures__/base-hook-no-forbidden-runtime/stubs/cyclic-pkg/a.ts diff --git a/tools/eslint-rules/rules/__fixtures__/consistent-base-hook/stubs/cyclic-pkg/b.ts b/tools/eslint-rules/rules/__fixtures__/base-hook-no-forbidden-runtime/stubs/cyclic-pkg/b.ts similarity index 100% rename from tools/eslint-rules/rules/__fixtures__/consistent-base-hook/stubs/cyclic-pkg/b.ts rename to tools/eslint-rules/rules/__fixtures__/base-hook-no-forbidden-runtime/stubs/cyclic-pkg/b.ts diff --git a/tools/eslint-rules/rules/__fixtures__/consistent-base-hook/stubs/cyclic-pkg/index.ts b/tools/eslint-rules/rules/__fixtures__/base-hook-no-forbidden-runtime/stubs/cyclic-pkg/index.ts similarity index 100% rename from tools/eslint-rules/rules/__fixtures__/consistent-base-hook/stubs/cyclic-pkg/index.ts rename to tools/eslint-rules/rules/__fixtures__/base-hook-no-forbidden-runtime/stubs/cyclic-pkg/index.ts diff --git a/tools/eslint-rules/rules/__fixtures__/consistent-base-hook/stubs/heavy-runtime/index.ts b/tools/eslint-rules/rules/__fixtures__/base-hook-no-forbidden-runtime/stubs/heavy-runtime/index.ts similarity index 100% rename from tools/eslint-rules/rules/__fixtures__/consistent-base-hook/stubs/heavy-runtime/index.ts rename to tools/eslint-rules/rules/__fixtures__/base-hook-no-forbidden-runtime/stubs/heavy-runtime/index.ts diff --git a/tools/eslint-rules/rules/__fixtures__/consistent-base-hook/stubs/light-helper/index.ts b/tools/eslint-rules/rules/__fixtures__/base-hook-no-forbidden-runtime/stubs/light-helper/index.ts similarity index 100% rename from tools/eslint-rules/rules/__fixtures__/consistent-base-hook/stubs/light-helper/index.ts rename to tools/eslint-rules/rules/__fixtures__/base-hook-no-forbidden-runtime/stubs/light-helper/index.ts diff --git a/tools/eslint-rules/rules/__fixtures__/consistent-base-hook/stubs/watched-pkg/heavy.ts b/tools/eslint-rules/rules/__fixtures__/base-hook-no-forbidden-runtime/stubs/watched-pkg/heavy.ts similarity index 100% rename from tools/eslint-rules/rules/__fixtures__/consistent-base-hook/stubs/watched-pkg/heavy.ts rename to tools/eslint-rules/rules/__fixtures__/base-hook-no-forbidden-runtime/stubs/watched-pkg/heavy.ts diff --git a/tools/eslint-rules/rules/__fixtures__/consistent-base-hook/stubs/watched-pkg/index.ts b/tools/eslint-rules/rules/__fixtures__/base-hook-no-forbidden-runtime/stubs/watched-pkg/index.ts similarity index 100% rename from tools/eslint-rules/rules/__fixtures__/consistent-base-hook/stubs/watched-pkg/index.ts rename to tools/eslint-rules/rules/__fixtures__/base-hook-no-forbidden-runtime/stubs/watched-pkg/index.ts diff --git a/tools/eslint-rules/rules/__fixtures__/consistent-base-hook/stubs/watched-pkg/light.ts b/tools/eslint-rules/rules/__fixtures__/base-hook-no-forbidden-runtime/stubs/watched-pkg/light.ts similarity index 100% rename from tools/eslint-rules/rules/__fixtures__/consistent-base-hook/stubs/watched-pkg/light.ts rename to tools/eslint-rules/rules/__fixtures__/base-hook-no-forbidden-runtime/stubs/watched-pkg/light.ts diff --git a/tools/eslint-rules/rules/__fixtures__/consistent-base-hook/tsconfig.json b/tools/eslint-rules/rules/__fixtures__/base-hook-no-forbidden-runtime/tsconfig.json similarity index 100% rename from tools/eslint-rules/rules/__fixtures__/consistent-base-hook/tsconfig.json rename to tools/eslint-rules/rules/__fixtures__/base-hook-no-forbidden-runtime/tsconfig.json diff --git a/tools/eslint-rules/rules/__fixtures__/base-hook-signature/src/components/Sibling/useSiblingBase.ts b/tools/eslint-rules/rules/__fixtures__/base-hook-signature/src/components/Sibling/useSiblingBase.ts new file mode 100644 index 00000000000000..4133e7b12d5f5b --- /dev/null +++ b/tools/eslint-rules/rules/__fixtures__/base-hook-signature/src/components/Sibling/useSiblingBase.ts @@ -0,0 +1,3 @@ +export const useSiblingBase_unstable = (props: { a: number }, ref: React.Ref) => { + return { props, ref }; +}; diff --git a/tools/eslint-rules/rules/__fixtures__/consistent-base-hook/src/components/Sibling/useSiblingBase.ts b/tools/eslint-rules/rules/__fixtures__/consistent-base-hook/src/components/Sibling/useSiblingBase.ts deleted file mode 100644 index 52eaf50766185d..00000000000000 --- a/tools/eslint-rules/rules/__fixtures__/consistent-base-hook/src/components/Sibling/useSiblingBase.ts +++ /dev/null @@ -1,7 +0,0 @@ -import * as React from 'react'; - -// Sibling base hook used to verify cross-file pair detection in the rule's spec. -// The wrapping state hook lives in `useSibling.ts` next to this file. -export const useSiblingBase_unstable = (props: { a: number }, ref: React.Ref) => { - return { props, ref }; -}; diff --git a/tools/eslint-rules/rules/consistent-base-hook.spec.ts b/tools/eslint-rules/rules/base-hook-no-forbidden-runtime.spec.ts similarity index 54% rename from tools/eslint-rules/rules/consistent-base-hook.spec.ts rename to tools/eslint-rules/rules/base-hook-no-forbidden-runtime.spec.ts index ab03e0222861e9..40b5b7217c80d5 100644 --- a/tools/eslint-rules/rules/consistent-base-hook.spec.ts +++ b/tools/eslint-rules/rules/base-hook-no-forbidden-runtime.spec.ts @@ -1,11 +1,9 @@ import * as path from 'node:path'; import { RuleTester } from '@typescript-eslint/rule-tester'; -import { rule, RULE_NAME } from './consistent-base-hook'; +import { rule, RULE_NAME } from './base-hook-no-forbidden-runtime'; -const FIXTURE_ROOT = path.join(__dirname, '__fixtures__/consistent-base-hook'); +const FIXTURE_ROOT = path.join(__dirname, '__fixtures__/base-hook-no-forbidden-runtime'); const TYPED_FILENAME = 'src/test.ts'; -const SIBLING_FILENAME = path.join(FIXTURE_ROOT, 'src/components/Sibling/useSibling.ts'); -const ORPHAN_FILENAME = path.join(FIXTURE_ROOT, 'src/components/Orphan/useOrphan.ts'); const typedLanguageOptions = { parserOptions: { @@ -15,99 +13,21 @@ const typedLanguageOptions = { }; // --------------------------------------------------------------------------- -// Parameter-shape checks — no typed linting required. +// Untyped checks: direct forbidden imports, scope shadowing, default allow-list, +// and the `typedServicesUnavailable` one-shot warning. // --------------------------------------------------------------------------- const ruleTester = new RuleTester(); ruleTester.run(RULE_NAME, rule, { valid: [ - // Valid base hook: namespace import — `import * as React from 'react'` + `React.Ref<...>`. - { - code: ` - import * as React from 'react'; - export const useThingBase_unstable = (props, ref: React.Ref) => { - return { props, ref }; - }; - `, - }, - // Valid base hook: named import — `import { Ref } from 'react'` + `Ref<...>` (FunctionDeclaration form). - { - code: ` - import { Ref } from 'react'; - export function useThingBase_unstable(props, ref: Ref) { - return { props, ref }; - } - `, - }, - // Valid base hook: default import — `import React from 'react'` + `React.Ref<...>`. - { - code: ` - import React from 'react'; - export const useThingBase_unstable = (props, ref: React.Ref) => { - return { props, ref }; - }; - `, - }, - // Valid base hook with only \`props\` (ref is optional). - { - code: ` - export const useThingBase_unstable = (props) => { - return { props }; - }; - `, - }, - // Non-base hook is not subject to the param-shape constraint. - { - code: ` - export const useThing_unstable = (props, ref, extra) => { - return { props, ref, extra }; - }; - `, - }, - // Pair detection (same file): a state hook `useThing_unstable` next to its base hook - // `useThingBase_unstable` IS subject to the contract. Correct signature passes. - { - code: ` - import * as React from 'react'; - export const useThing_unstable = (props, ref: React.Ref) => { - return { props, ref }; - }; - export const useThingBase_unstable = (props, ref: React.Ref) => { - return { props, ref }; - }; - `, - }, - // Pair detection (no sibling base hook on disk): `useOrphanContextValues_unstable(state)` - // is NOT a paired wrapping hook and must NOT be flagged for its non-(props, ref) signature. - // The Orphan folder has no `useOrphanContextValuesBase.ts(x)` next to it. - { - filename: ORPHAN_FILENAME, - code: ` - export function useOrphanContextValues_unstable(state) { - return { state }; - } - `, - }, - // Pair detection (sibling file): wrapping state hook lives in `useSibling.ts`, paired with - // `useSiblingBase.ts` in the same folder. Correct (props, ref) signature passes. - { - filename: SIBLING_FILENAME, - code: ` - import * as React from 'react'; - export const useSibling_unstable = (props, ref: React.Ref) => { - return { props, ref }; - }; - `, - }, // Identifier with the same local name as a forbidden import alias does not collide via scope analysis. { code: ` - import * as React from 'react'; import { useArrowNavigationGroup } from '@fluentui/react-tabster'; - export const useThing_unstable = (props, ref: React.Ref) => { + export const useThing_unstable = (props, ref) => { return useArrowNavigationGroup({}); }; - export const useThingBase_unstable = (props, ref: React.Ref) => { + export const useThingBase_unstable = (props, ref) => { const useArrowNavigationGroup = () => 1; return { value: useArrowNavigationGroup() }; }; @@ -116,140 +36,28 @@ ruleTester.run(RULE_NAME, rule, { // \`keyborg\` is not in the default forbidden runtime list — bindings imported from it are allowed inside base hooks. { code: ` - import * as React from 'react'; import { createKeyborg, KEYBORG_FOCUSIN } from 'keyborg'; - export const useThingBase_unstable = (props, ref: React.Ref) => { + export const useThingBase_unstable = (props, ref) => { return { kb: createKeyborg(window), evt: KEYBORG_FOCUSIN }; }; `, }, - ], - invalid: [ - // Too few params (0). - { - code: ` - export const useThingBase_unstable = () => ({}); - `, - errors: [{ messageId: 'invalidParamCount', data: { hookName: 'useThingBase_unstable', actual: 0 } }], - }, - // Too many params. - { - code: ` - export const useThingBase_unstable = (props, ref, extra) => ({ props, ref, extra }); - `, - errors: [{ messageId: 'invalidParamCount', data: { hookName: 'useThingBase_unstable', actual: 3 } }], - }, - // Wrong param names. + // No watched/forbidden imports — base hook body is not inspected at all. { code: ` - export const useThingBase_unstable = (p, r) => ({ p, r }); - `, - errors: [ - { - messageId: 'invalidParamName', - data: { hookName: 'useThingBase_unstable', index: 1, expected: 'props', actual: 'p' }, - }, - { - messageId: 'invalidParamName', - data: { hookName: 'useThingBase_unstable', index: 2, expected: 'ref', actual: 'r' }, - }, - ], - }, - // ObjectPattern for \`props\` is not allowed. - { - code: ` - import * as React from 'react'; - export const useThingBase_unstable = ({ a }, ref: React.Ref) => ({ a, ref }); - `, - errors: [ - { - messageId: 'invalidParamName', - data: { hookName: 'useThingBase_unstable', index: 1, expected: 'props', actual: '{ ... }' }, - }, - ], - }, - // \`ref\` parameter without a type annotation. - { - code: ` - export const useThingBase_unstable = (props, ref) => ({ props, ref }); - `, - errors: [ - { - messageId: 'invalidRefType', - data: { hookName: 'useThingBase_unstable', actual: '' }, - }, - ], - }, - // \`ref\` parameter typed as something other than React.Ref. - { - code: ` - export const useThingBase_unstable = (props, ref: HTMLElement) => ({ props, ref }); - `, - errors: [ - { - messageId: 'invalidRefType', - data: { hookName: 'useThingBase_unstable', actual: 'HTMLElement' }, - }, - ], - }, - // \`ref\` parameter typed as React.ForwardedRef (must be React.Ref). - { - code: ` - export const useThingBase_unstable = (props, ref: React.ForwardedRef) => ({ props, ref }); - `, - errors: [ - { - messageId: 'invalidRefType', - data: { hookName: 'useThingBase_unstable', actual: 'React.ForwardedRef' }, - }, - ], - }, - // \`Ref\` is a locally declared type alias, not imported from react. - { - code: ` - type Ref = { current: T | null }; - export const useThingBase_unstable = (props, ref: Ref) => ({ props, ref }); - `, - errors: [ - { - messageId: 'invalidRefType', - data: { hookName: 'useThingBase_unstable', actual: 'Ref' }, - }, - ], - }, - // \`Ref\` imported from a non-react package is not accepted. - { - code: ` - import { Ref } from 'not-react'; - export const useThingBase_unstable = (props, ref: Ref) => ({ props, ref }); - `, - errors: [ - { - messageId: 'invalidRefType', - data: { hookName: 'useThingBase_unstable', actual: 'Ref' }, - }, - ], - }, - // \`React\` is a locally declared identifier, not the react module namespace. - { - code: ` - const React = { Ref: null }; - export const useThingBase_unstable = (props, ref: React.Ref) => ({ props, ref }); + export const useThingBase_unstable = (props, ref) => { + return { props, ref }; + }; `, - errors: [ - { - messageId: 'invalidRefType', - data: { hookName: 'useThingBase_unstable', actual: 'React.Ref' }, - }, - ], }, + ], + invalid: [ // Referencing a watched-package binding inside a base hook without typed services available // surfaces a one-shot `typedServicesUnavailable` diagnostic so the misconfiguration is visible. { code: ` - import * as React from 'react'; import { useArrowNavigationGroup } from '@fluentui/react-tabster'; - export const useThingBase_unstable = (props, ref: React.Ref) => { + export const useThingBase_unstable = (props, ref) => { return useArrowNavigationGroup({}); }; `, @@ -263,35 +71,6 @@ ruleTester.run(RULE_NAME, rule, { }, ], }, - // Pair detection (same file): wrapping state hook with too many params is flagged because - // its sibling base hook in the same file marks it as a paired wrapper. - { - code: ` - import * as React from 'react'; - export const useThing_unstable = (props, ref, extra) => ({ props, ref, extra }); - export const useThingBase_unstable = (props, ref: React.Ref) => ({ props, ref }); - `, - errors: [{ messageId: 'invalidParamCount', data: { hookName: 'useThing_unstable', actual: 3 } }], - }, - // Pair detection (sibling file): wrapping state hook in `useSibling.ts` is paired with - // `useSiblingBase.ts` in the same folder. Wrong param names are flagged. - { - filename: SIBLING_FILENAME, - code: ` - import * as React from 'react'; - export const useSibling_unstable = (p, r: React.Ref) => ({ p, r }); - `, - errors: [ - { - messageId: 'invalidParamName', - data: { hookName: 'useSibling_unstable', index: 1, expected: 'props', actual: 'p' }, - }, - { - messageId: 'invalidParamName', - data: { hookName: 'useSibling_unstable', index: 2, expected: 'ref', actual: 'r' }, - }, - ], - }, ], }); @@ -325,9 +104,8 @@ typedRuleTester.run(`${RULE_NAME} (typed)`, rule, { filename: TYPED_FILENAME, options: transitiveOptions, code: ` - import * as React from 'react'; import { useLight } from 'watched-pkg'; - export const useThingBase_unstable = (props: { a: number }, ref: React.Ref) => { + export const useThingBase_unstable = (props: { a: number }, ref) => { useLight(); return { props, ref }; }; @@ -341,9 +119,8 @@ typedRuleTester.run(`${RULE_NAME} (typed)`, rule, { filename: TYPED_FILENAME, options: transitiveOptions, code: ` - import * as React from 'react'; import type { LightOptions } from 'watched-pkg'; - export const useThingBase_unstable = (props: LightOptions, ref: React.Ref) => { + export const useThingBase_unstable = (props: LightOptions, ref) => { return { props, ref }; }; `, @@ -354,12 +131,11 @@ typedRuleTester.run(`${RULE_NAME} (typed)`, rule, { filename: TYPED_FILENAME, options: transitiveOptions, code: ` - import * as React from 'react'; import { useHeavy } from 'watched-pkg'; - export const useThingBase_unstable = (props: { a: number }, ref: React.Ref) => { + export const useThingBase_unstable = (props: { a: number }, ref) => { return { props, ref }; }; - export const useThing_unstable = (props, ref: React.Ref) => { + export const useThing_unstable = (props, ref) => { return useHeavy(); }; `, @@ -375,9 +151,8 @@ typedRuleTester.run(`${RULE_NAME} (typed)`, rule, { }, ], code: ` - import * as React from 'react'; import { useA } from 'cyclic-pkg'; - export const useThingBase_unstable = (props: { a: number }, ref: React.Ref) => { + export const useThingBase_unstable = (props: { a: number }, ref) => { return { props, ref, value: useA() }; }; `, @@ -387,9 +162,8 @@ typedRuleTester.run(`${RULE_NAME} (typed)`, rule, { filename: TYPED_FILENAME, options: transitiveOptionsAllowTypeImports, code: ` - import * as React from 'react'; import type { HeavyOptions } from 'heavy-runtime'; - export const useThingBase_unstable = (props: HeavyOptions, ref: React.Ref) => { + export const useThingBase_unstable = (props: HeavyOptions, ref) => { return { props, ref }; }; `, @@ -400,9 +174,8 @@ typedRuleTester.run(`${RULE_NAME} (typed)`, rule, { filename: TYPED_FILENAME, options: transitiveOptionsAllowTypeImports, code: ` - import * as React from 'react'; import { type HeavyOptions } from 'heavy-runtime'; - export const useThingBase_unstable = (props: HeavyOptions, ref: React.Ref) => { + export const useThingBase_unstable = (props: HeavyOptions, ref) => { return { props, ref }; }; `, @@ -414,9 +187,8 @@ typedRuleTester.run(`${RULE_NAME} (typed)`, rule, { filename: TYPED_FILENAME, options: transitiveOptionsAllowTypeImports, code: ` - import * as React from 'react'; import type { HeavyType } from 'watched-pkg'; - export const useThingBase_unstable = (props: HeavyType, ref: React.Ref) => { + export const useThingBase_unstable = (props: HeavyType, ref) => { return { props, ref }; }; `, @@ -429,9 +201,8 @@ typedRuleTester.run(`${RULE_NAME} (typed)`, rule, { filename: TYPED_FILENAME, options: transitiveOptions, code: ` - import * as React from 'react'; import { runHeavy } from 'heavy-runtime'; - export const useThingBase_unstable = (props: { a: number }, ref: React.Ref) => { + export const useThingBase_unstable = (props: { a: number }, ref) => { return { props, ref, x: runHeavy() }; }; `, @@ -452,9 +223,8 @@ typedRuleTester.run(`${RULE_NAME} (typed)`, rule, { filename: TYPED_FILENAME, options: transitiveOptions, code: ` - import * as React from 'react'; import { useHeavy } from 'watched-pkg'; - export const useThingBase_unstable = (props: { a: number }, ref: React.Ref) => { + export const useThingBase_unstable = (props: { a: number }, ref) => { return { props, ref, x: useHeavy() }; }; `, @@ -466,7 +236,7 @@ typedRuleTester.run(`${RULE_NAME} (typed)`, rule, { importedName: 'useHeavy', package: 'watched-pkg', runtime: 'heavy-runtime', - viaFile: 'rules/__fixtures__/consistent-base-hook/stubs/watched-pkg/heavy.ts', + viaFile: 'rules/__fixtures__/base-hook-no-forbidden-runtime/stubs/watched-pkg/heavy.ts', }, }, ], @@ -477,9 +247,8 @@ typedRuleTester.run(`${RULE_NAME} (typed)`, rule, { filename: TYPED_FILENAME, options: transitiveOptions, code: ` - import * as React from 'react'; import { runHeavy as go } from 'heavy-runtime'; - export function useThingBase_unstable(props: { a: number }, ref: React.Ref) { + export function useThingBase_unstable(props: { a: number }, ref) { return go(); } `, @@ -500,9 +269,8 @@ typedRuleTester.run(`${RULE_NAME} (typed)`, rule, { filename: TYPED_FILENAME, options: transitiveOptions, code: ` - import * as React from 'react'; import type { HeavyOptions } from 'heavy-runtime'; - export const useThingBase_unstable = (props: HeavyOptions, ref: React.Ref) => { + export const useThingBase_unstable = (props: HeavyOptions, ref) => { return { props, ref }; }; `, @@ -523,9 +291,8 @@ typedRuleTester.run(`${RULE_NAME} (typed)`, rule, { filename: TYPED_FILENAME, options: transitiveOptions, code: ` - import * as React from 'react'; import { type HeavyOptions } from 'heavy-runtime'; - export const useThingBase_unstable = (props: HeavyOptions, ref: React.Ref) => { + export const useThingBase_unstable = (props: HeavyOptions, ref) => { return { props, ref }; }; `, @@ -548,9 +315,8 @@ typedRuleTester.run(`${RULE_NAME} (typed)`, rule, { filename: TYPED_FILENAME, options: transitiveOptions, code: ` - import * as React from 'react'; import type { HeavyType } from 'watched-pkg'; - export const useThingBase_unstable = (props: HeavyType, ref: React.Ref) => { + export const useThingBase_unstable = (props: HeavyType, ref) => { return { props, ref }; }; `, @@ -562,7 +328,7 @@ typedRuleTester.run(`${RULE_NAME} (typed)`, rule, { importedName: 'HeavyType', package: 'watched-pkg', runtime: 'heavy-runtime', - viaFile: 'rules/__fixtures__/consistent-base-hook/stubs/watched-pkg/heavy.ts', + viaFile: 'rules/__fixtures__/base-hook-no-forbidden-runtime/stubs/watched-pkg/heavy.ts', }, }, ], @@ -573,9 +339,8 @@ typedRuleTester.run(`${RULE_NAME} (typed)`, rule, { filename: TYPED_FILENAME, options: transitiveOptions, code: ` - import * as React from 'react'; import { type HeavyType, useLight } from 'watched-pkg'; - export const useThingBase_unstable = (props: HeavyType, ref: React.Ref) => { + export const useThingBase_unstable = (props: HeavyType, ref) => { useLight(); return { props, ref }; }; @@ -588,7 +353,7 @@ typedRuleTester.run(`${RULE_NAME} (typed)`, rule, { importedName: 'HeavyType', package: 'watched-pkg', runtime: 'heavy-runtime', - viaFile: 'rules/__fixtures__/consistent-base-hook/stubs/watched-pkg/heavy.ts', + viaFile: 'rules/__fixtures__/base-hook-no-forbidden-runtime/stubs/watched-pkg/heavy.ts', }, }, ], @@ -601,9 +366,8 @@ typedRuleTester.run(`${RULE_NAME} (typed)`, rule, { filename: TYPED_FILENAME, options: transitiveOptions, code: ` - import * as React from 'react'; import type { HeavyWrapper } from 'watched-pkg'; - export const useThingBase_unstable = (props: HeavyWrapper, ref: React.Ref) => { + export const useThingBase_unstable = (props: HeavyWrapper, ref) => { return { props, ref }; }; `, @@ -615,7 +379,7 @@ typedRuleTester.run(`${RULE_NAME} (typed)`, rule, { importedName: 'HeavyWrapper', package: 'watched-pkg', runtime: 'heavy-runtime', - viaFile: 'rules/__fixtures__/consistent-base-hook/stubs/watched-pkg/index.ts', + viaFile: 'rules/__fixtures__/base-hook-no-forbidden-runtime/stubs/watched-pkg/index.ts', }, }, ], diff --git a/tools/eslint-rules/rules/base-hook-no-forbidden-runtime.ts b/tools/eslint-rules/rules/base-hook-no-forbidden-runtime.ts new file mode 100644 index 00000000000000..d5c03db7e9f343 --- /dev/null +++ b/tools/eslint-rules/rules/base-hook-no-forbidden-runtime.ts @@ -0,0 +1,362 @@ +import type { TSESTree, TSESLint, ParserServicesWithTypeInformation } from '@typescript-eslint/utils'; +import { ESLintUtils, AST_NODE_TYPES } from '@typescript-eslint/utils'; +import * as ts from 'typescript'; +import { BASE_HOOK_NAME_PATTERN, type BaseHookFunction, getFunctionInit } from '../utils/base-hook-detector'; +import { transitiveReach } from '../utils/transitive-reach'; +import { shortenPath } from '../utils/module-resolver'; +import { type ImportSpecifierNode, type TrackedImport, getImportedName } from '../utils/tracked-imports'; + +// NOTE: The rule will be available in ESLint configs as "@nx/workspace-base-hook-no-forbidden-runtime" +export const RULE_NAME = 'base-hook-no-forbidden-runtime'; + +const DEFAULT_WATCHED_PACKAGES: ReadonlyArray = ['@fluentui/react-tabster']; +const DEFAULT_FORBIDDEN_RUNTIMES: ReadonlyArray = ['tabster']; + +type Options = [ + { + /** + * Packages whose imported symbols must be analyzed transitively. + * A symbol imported from one of these packages is allowed inside a base + * hook only if its defining source file does not reach any + * `forbiddenRuntimes` package via value imports. + */ + watchedPackages?: string[]; + /** + * Runtime packages whose presence in the transitive value-import graph of + * a referenced symbol is forbidden inside base hooks. Direct imports from + * these packages are also forbidden. + */ + forbiddenRuntimes?: string[]; + /** + * When `true`, type-only imports (both from `forbiddenRuntimes` packages directly and + * from `watchedPackages` whose defining module reaches a forbidden runtime) are permitted + * inside base hooks. Type-only imports emit no runtime code, so this option trades API + * decoupling for ergonomics. + * + * Defaults to `false` — type-only imports are checked the same way as value imports, to + * keep the base hook's public API fully decoupled from forbidden runtimes. + */ + allowTypeImports?: boolean; + }?, +]; + +type MessageIds = 'forbiddenRuntimeDirect' | 'forbiddenRuntimeReach' | 'typedServicesUnavailable'; + +export const rule = ESLintUtils.RuleCreator(() => __filename)({ + name: RULE_NAME, + meta: { + type: 'problem', + docs: { + description: + 'Disallow inside v9 base hooks (`useBase_unstable`) any binding whose defining module transitively pulls a forbidden runtime package (default `tabster`) — both at value positions (runtime coupling) and at type positions (API surface coupling).', + }, + schema: [ + { + type: 'object', + properties: { + watchedPackages: { + type: 'array', + items: { type: 'string' }, + uniqueItems: true, + }, + forbiddenRuntimes: { + type: 'array', + items: { type: 'string' }, + uniqueItems: true, + }, + allowTypeImports: { + type: 'boolean', + }, + }, + additionalProperties: false, + }, + ], + messages: { + forbiddenRuntimeDirect: + 'Base hook `{{hookName}}` cannot reference `{{importedName}}` from forbidden runtime package `{{package}}`. Move logic that depends on `{{package}}` to the wrapping `*_unstable` hook instead.', + forbiddenRuntimeReach: + 'Base hook `{{hookName}}` cannot reference `{{importedName}}` from `{{package}}` because its defining module transitively imports forbidden runtime `{{runtime}}` (via `{{viaFile}}`). Move logic that depends on `{{runtime}}` to the wrapping `*_unstable` hook instead.', + typedServicesUnavailable: + 'base-hook-no-forbidden-runtime: transitive runtime analysis was skipped because TypeScript type information is unavailable. Enable typescript-eslint type-aware linting (set `parserOptions.projectService: true` or `parserOptions.project`) so references through watched packages (e.g. `{{watchedPackages}}`) can be verified against forbidden runtimes (e.g. `{{forbiddenRuntimes}}`).', + }, + }, + defaultOptions: [{}], + create(context) { + const sourceCode = context.sourceCode; + const options = context.options[0] ?? {}; + const watchedPackages = new Set(options.watchedPackages ?? DEFAULT_WATCHED_PACKAGES); + const forbiddenRuntimes = new Set(options.forbiddenRuntimes ?? DEFAULT_FORBIDDEN_RUNTIMES); + const allowTypeImports = options.allowTypeImports ?? false; + // `forbidden` takes precedence: if the same name appears in both lists, treat the binding as forbidden. + const trackedPackages = new Set([...watchedPackages, ...forbiddenRuntimes]); + + // Map of locally-declared variable identity → original import origin metadata. Keyed by Variable + // identity (not name) so re-declarations / shadowing inside the base hook resolve correctly. + const trackedImports = new Map(); + + // Tracks whether `computeSymbolReach` was invoked while typed services were unavailable. When set, + // we emit a single diagnostic on `Program:exit` so the user knows transitive analysis was skipped. + let typedServicesNeededButMissing = false; + + // Lazily-acquired typed services. Resolved once per file, cached in `typedServices` (undefined = + // not yet attempted, null = attempted and unavailable, value = available). + let typedServices: ParserServicesWithTypeInformation | null | undefined; + + /** + * Returns typed services (TS Program + checker) for the current file, or `null` if untyped + * lint is in effect. Result is memoized for the lifetime of the per-file rule instance. + */ + function getTypedServices(): ParserServicesWithTypeInformation | null { + if (typedServices !== undefined) { + return typedServices; + } + try { + typedServices = ESLintUtils.getParserServices(context); + } catch { + typedServices = null; + } + return typedServices; + } + + /** + * Walks the base hook's scope graph looking for references to any tracked import. Bails out + * early if nothing is tracked, so the typical case (no watched/forbidden imports in the file) + * stays free. + */ + function checkBodyReferences(hookName: string, hookFn: BaseHookFunction): void { + if (trackedImports.size === 0) { + return; + } + const hookScope = sourceCode.getScope(hookFn); + visitScope(hookScope, hookFn, hookName); + } + + /** + * Recursively visits `scope` and all its descendants that are still inside the base hook body. + * For every resolved reference whose declaration is a tracked import, either flag the direct + * usage or delegate to `computeSymbolReach` for the transitive check. + * + * The chosen reach set depends on the reference position: + * - value reference → `value` reach (runtime coupling) + * - type reference → `all` reach (API coupling — a type alias can still tie the public + * API to a forbidden runtime via its defining module) + */ + function visitScope(scope: TSESLint.Scope.Scope, hookFn: BaseHookFunction, hookName: string): void { + if (!isScopeWithinFunction(scope, hookFn)) { + return; + } + + scope.references.forEach(reference => { + const resolved = reference.resolved; + if (!resolved) { + return; + } + const origin = trackedImports.get(resolved); + if (!origin) { + return; + } + + const isTypeRef = reference.isTypeReference === true; + // A type-only binding can only legally appear in type positions; ignore the (invalid) + // value reference — TS will flag it independently. + if (origin.isTypeOnly && !isTypeRef) { + return; + } + + if (origin.kind === 'forbidden') { + context.report({ + node: reference.identifier, + messageId: 'forbiddenRuntimeDirect', + data: { + hookName, + importedName: origin.importedName, + package: origin.package, + }, + }); + return; + } + + // Watched package: only flag if the defining module transitively reaches a forbidden runtime. + const reach = computeSymbolReach(origin); + if (!reach) { + return; // untyped lint or unresolvable — silently skip + } + const reached = isTypeRef ? reach.all : reach.value; + + for (const runtime of forbiddenRuntimes) { + if (reached.has(runtime)) { + context.report({ + node: reference.identifier, + messageId: 'forbiddenRuntimeReach', + data: { + hookName, + importedName: origin.importedName, + package: origin.package, + runtime, + viaFile: reach.viaFile, + }, + }); + return; + } + } + }); + + scope.childScopes.forEach(child => visitScope(child, hookFn, hookName)); + } + + /** + * Resolves the watched-package import to its defining module via TS `Program`, then queries the + * transitive import graph for forbidden runtimes (both value-only and value+type sets). + * Returns `null` (and flips the `typedServicesNeededButMissing` flag) when typed services + * aren't available, so the caller can silently skip and we can warn once on `Program:exit`. + */ + function computeSymbolReach( + origin: TrackedImport, + ): { value: ReadonlySet; all: ReadonlySet; viaFile: string } | null { + const services = getTypedServices(); + if (!services) { + typedServicesNeededButMissing = true; + return null; + } + const checker = services.program.getTypeChecker(); + const tsNode = services.esTreeNodeToTSNodeMap.get(origin.specifier); + if (!tsNode) { + return null; + } + + // For an ImportSpecifier we want the imported (right-hand) identifier so the symbol resolves to + // the exported name on the source module, not the local alias. + let nameNode: ts.Node | undefined; + if (ts.isImportSpecifier(tsNode)) { + nameNode = tsNode.propertyName ?? tsNode.name; + } else if (ts.isImportClause(tsNode) || ts.isNamespaceImport(tsNode)) { + nameNode = tsNode.name; + } else { + nameNode = tsNode; + } + if (!nameNode) { + return null; + } + + let symbol = checker.getSymbolAtLocation(nameNode); + if (!symbol) { + return null; + } + // eslint-disable-next-line no-bitwise -- ts.SymbolFlags is a bitfield enum + if ((symbol.flags & ts.SymbolFlags.Alias) !== 0) { + try { + symbol = checker.getAliasedSymbol(symbol); + } catch { + return null; + } + } + const declaration = symbol.declarations?.[0]; + const definingFile = declaration?.getSourceFile(); + if (!definingFile) { + return null; + } + + const reach = transitiveReach(services.program, definingFile); + return { value: reach.value, all: reach.all, viaFile: shortenPath(definingFile.fileName) }; + } + + /** + * `ImportDeclaration` visitor: records every named/default/namespace specifier coming from a + * watched or forbidden-runtime package so body references can later be resolved via + * `sourceCode.getDeclaredVariables`. Tracks both value AND type-only specifiers — type refs + * are still inspected at the hook signature because a type from a watched package can + * transitively expose a forbidden runtime through its defining module. + */ + function trackImportDeclaration(node: TSESTree.ImportDeclaration): void { + const source = node.source.value; + if (typeof source !== 'string' || !trackedPackages.has(source)) { + return; + } + const isForbiddenPkg = forbiddenRuntimes.has(source); + const stmtTypeOnly = node.importKind === 'type'; + // Symmetric semantics: when `allowTypeImports` is true, type-only imports are exempt from + // both direct forbidden-runtime checks AND transitive watched-package reach checks (a type + // can never pull runtime code at execution time). + if (stmtTypeOnly && allowTypeImports) { + return; + } + const kind: TrackedImport['kind'] = isForbiddenPkg ? 'forbidden' : 'watched'; + + node.specifiers.forEach(specifier => { + const specTypeOnly = + stmtTypeOnly || (specifier.type === AST_NODE_TYPES.ImportSpecifier && specifier.importKind === 'type'); + if (specTypeOnly && allowTypeImports) { + return; + } + const importedName = getImportedName(specifier as ImportSpecifierNode); + if (importedName === undefined) { + return; + } + for (const variable of sourceCode.getDeclaredVariables(specifier)) { + trackedImports.set(variable, { + package: source, + importedName, + kind, + isTypeOnly: specTypeOnly, + specifier: specifier as ImportSpecifierNode, + }); + } + }); + } + + return { + ImportDeclaration: trackImportDeclaration, + + // Match only base hooks — wrapping state hook signature is enforced by the sibling + // `base-hook-signature` rule (which also handles pair detection). + [`FunctionDeclaration[id.name=/${BASE_HOOK_NAME_PATTERN.source}/]`]: (node: TSESTree.FunctionDeclaration) => { + if (!node.id) { + return; + } + checkBodyReferences(node.id.name, node); + }, + + [`VariableDeclarator[id.name=/${BASE_HOOK_NAME_PATTERN.source}/]`]: (node: TSESTree.VariableDeclarator) => { + const init = getFunctionInit(node); + if (!init || node.id.type !== AST_NODE_TYPES.Identifier) { + return; + } + checkBodyReferences(node.id.name, init); + }, + + /** + * One-shot diagnostic so the user is informed (rather than silently degraded) when the + * transitive runtime check was needed but skipped due to missing typed services. + */ + 'Program:exit'(programNode) { + if (!typedServicesNeededButMissing) { + return; + } + context.report({ + node: programNode, + messageId: 'typedServicesUnavailable', + data: { + watchedPackages: [...watchedPackages].join(', '), + forbiddenRuntimes: [...forbiddenRuntimes].join(', '), + }, + }); + }, + }; + }, +}); + +/** + * `true` when `scope` (or any of its ancestor scopes) is the function scope of `hookFn`. Used to + * confine the body-reference walk to the base hook itself — references in sibling functions are + * out of scope for this rule. + */ +function isScopeWithinFunction(scope: TSESLint.Scope.Scope, hookFn: BaseHookFunction): boolean { + let current: TSESLint.Scope.Scope | null = scope; + while (current) { + if (current.block === hookFn) { + return true; + } + current = current.upper; + } + return false; +} diff --git a/tools/eslint-rules/rules/base-hook-signature.spec.ts b/tools/eslint-rules/rules/base-hook-signature.spec.ts new file mode 100644 index 00000000000000..8dd4ed49e25a5a --- /dev/null +++ b/tools/eslint-rules/rules/base-hook-signature.spec.ts @@ -0,0 +1,241 @@ +import * as path from 'node:path'; +import { RuleTester } from '@typescript-eslint/rule-tester'; +import { rule, RULE_NAME } from './base-hook-signature'; + +const FIXTURE_ROOT = path.join(__dirname, '__fixtures__/base-hook-signature'); +const SIBLING_FILENAME = path.join(FIXTURE_ROOT, 'src/components/Sibling/useSibling.ts'); +const ORPHAN_FILENAME = path.join(FIXTURE_ROOT, 'src/components/Orphan/useOrphan.ts'); + +const ruleTester = new RuleTester(); + +ruleTester.run(RULE_NAME, rule, { + valid: [ + // Valid base hook: namespace import — `import * as React from 'react'` + `React.Ref<...>`. + { + code: ` + import * as React from 'react'; + export const useThingBase_unstable = (props, ref: React.Ref) => { + return { props, ref }; + }; + `, + }, + // Valid base hook: named import — `import { Ref } from 'react'` + `Ref<...>` (FunctionDeclaration form). + { + code: ` + import { Ref } from 'react'; + export function useThingBase_unstable(props, ref: Ref) { + return { props, ref }; + } + `, + }, + // Valid base hook: default import — `import React from 'react'` + `React.Ref<...>`. + { + code: ` + import React from 'react'; + export const useThingBase_unstable = (props, ref: React.Ref) => { + return { props, ref }; + }; + `, + }, + // Valid base hook with only \`props\` (ref is optional). + { + code: ` + export const useThingBase_unstable = (props) => { + return { props }; + }; + `, + }, + // Non-base hook without a paired base hook is not subject to the contract. + { + code: ` + export const useThing_unstable = (props, ref, extra) => { + return { props, ref, extra }; + }; + `, + }, + // Pair detection (same file): a state hook `useThing_unstable` next to its base hook + // `useThingBase_unstable` IS subject to the contract. Correct signature passes. + { + code: ` + import * as React from 'react'; + export const useThing_unstable = (props, ref: React.Ref) => { + return { props, ref }; + }; + export const useThingBase_unstable = (props, ref: React.Ref) => { + return { props, ref }; + }; + `, + }, + // Pair detection (no sibling base hook on disk): `useOrphanContextValues_unstable(state)` + // is NOT a paired wrapping hook and must NOT be flagged for its non-(props, ref) signature. + // The Orphan folder has no `useOrphanContextValuesBase.ts(x)` next to it. + { + filename: ORPHAN_FILENAME, + code: ` + export function useOrphanContextValues_unstable(state) { + return { state }; + } + `, + }, + // Pair detection (sibling file): wrapping state hook lives in `useSibling.ts`, paired with + // `useSiblingBase.ts` in the same folder. Correct (props, ref) signature passes. + { + filename: SIBLING_FILENAME, + code: ` + import * as React from 'react'; + export const useSibling_unstable = (props, ref: React.Ref) => { + return { props, ref }; + }; + `, + }, + ], + invalid: [ + // Too few params (0). + { + code: ` + export const useThingBase_unstable = () => ({}); + `, + errors: [{ messageId: 'invalidParamCount', data: { hookName: 'useThingBase_unstable', actual: 0 } }], + }, + // Too many params. + { + code: ` + export const useThingBase_unstable = (props, ref, extra) => ({ props, ref, extra }); + `, + errors: [{ messageId: 'invalidParamCount', data: { hookName: 'useThingBase_unstable', actual: 3 } }], + }, + // Wrong param names. + { + code: ` + export const useThingBase_unstable = (p, r) => ({ p, r }); + `, + errors: [ + { + messageId: 'invalidParamName', + data: { hookName: 'useThingBase_unstable', index: 1, expected: 'props', actual: 'p' }, + }, + { + messageId: 'invalidParamName', + data: { hookName: 'useThingBase_unstable', index: 2, expected: 'ref', actual: 'r' }, + }, + ], + }, + // ObjectPattern for \`props\` is not allowed. + { + code: ` + import * as React from 'react'; + export const useThingBase_unstable = ({ a }, ref: React.Ref) => ({ a, ref }); + `, + errors: [ + { + messageId: 'invalidParamName', + data: { hookName: 'useThingBase_unstable', index: 1, expected: 'props', actual: '{ ... }' }, + }, + ], + }, + // \`ref\` parameter without a type annotation. + { + code: ` + export const useThingBase_unstable = (props, ref) => ({ props, ref }); + `, + errors: [ + { + messageId: 'invalidRefType', + data: { hookName: 'useThingBase_unstable', actual: '' }, + }, + ], + }, + // \`ref\` parameter typed as something other than React.Ref. + { + code: ` + export const useThingBase_unstable = (props, ref: HTMLElement) => ({ props, ref }); + `, + errors: [ + { + messageId: 'invalidRefType', + data: { hookName: 'useThingBase_unstable', actual: 'HTMLElement' }, + }, + ], + }, + // \`ref\` parameter typed as React.ForwardedRef (must be React.Ref). + { + code: ` + export const useThingBase_unstable = (props, ref: React.ForwardedRef) => ({ props, ref }); + `, + errors: [ + { + messageId: 'invalidRefType', + data: { hookName: 'useThingBase_unstable', actual: 'React.ForwardedRef' }, + }, + ], + }, + // \`Ref\` is a locally declared type alias, not imported from react. + { + code: ` + type Ref = { current: T | null }; + export const useThingBase_unstable = (props, ref: Ref) => ({ props, ref }); + `, + errors: [ + { + messageId: 'invalidRefType', + data: { hookName: 'useThingBase_unstable', actual: 'Ref' }, + }, + ], + }, + // \`Ref\` imported from a non-react package is not accepted. + { + code: ` + import { Ref } from 'not-react'; + export const useThingBase_unstable = (props, ref: Ref) => ({ props, ref }); + `, + errors: [ + { + messageId: 'invalidRefType', + data: { hookName: 'useThingBase_unstable', actual: 'Ref' }, + }, + ], + }, + // \`React\` is a locally declared identifier, not the react module namespace. + { + code: ` + const React = { Ref: null }; + export const useThingBase_unstable = (props, ref: React.Ref) => ({ props, ref }); + `, + errors: [ + { + messageId: 'invalidRefType', + data: { hookName: 'useThingBase_unstable', actual: 'React.Ref' }, + }, + ], + }, + // Pair detection (same file): wrapping state hook with too many params is flagged because + // its sibling base hook in the same file marks it as a paired wrapper. + { + code: ` + import * as React from 'react'; + export const useThing_unstable = (props, ref, extra) => ({ props, ref, extra }); + export const useThingBase_unstable = (props, ref: React.Ref) => ({ props, ref }); + `, + errors: [{ messageId: 'invalidParamCount', data: { hookName: 'useThing_unstable', actual: 3 } }], + }, + // Pair detection (sibling file): wrapping state hook in `useSibling.ts` is paired with + // `useSiblingBase.ts` in the same folder. Wrong param names are flagged. + { + filename: SIBLING_FILENAME, + code: ` + import * as React from 'react'; + export const useSibling_unstable = (p, r: React.Ref) => ({ p, r }); + `, + errors: [ + { + messageId: 'invalidParamName', + data: { hookName: 'useSibling_unstable', index: 1, expected: 'props', actual: 'p' }, + }, + { + messageId: 'invalidParamName', + data: { hookName: 'useSibling_unstable', index: 2, expected: 'ref', actual: 'r' }, + }, + ], + }, + ], +}); diff --git a/tools/eslint-rules/rules/base-hook-signature.ts b/tools/eslint-rules/rules/base-hook-signature.ts new file mode 100644 index 00000000000000..ea322cd0cfa9b6 --- /dev/null +++ b/tools/eslint-rules/rules/base-hook-signature.ts @@ -0,0 +1,263 @@ +import type { TSESTree, TSESLint } from '@typescript-eslint/utils'; +import { ESLintUtils, AST_NODE_TYPES } from '@typescript-eslint/utils'; +import { + BASE_HOOK_NAME_PATTERN, + STATE_HOOK_NAME_PATTERN, + type BaseHookFunction, + collectBaseHookNames, + createPairDetector, + getFunctionInit, +} from '../utils/base-hook-detector'; + +// NOTE: The rule will be available in ESLint configs as "@nx/workspace-base-hook-signature" +export const RULE_NAME = 'base-hook-signature'; + +const EXPECTED_PARAM_NAMES = ['props', 'ref'] as const; +const MIN_PARAM_COUNT = 1; +const MAX_PARAM_COUNT = 2; + +type Options = []; + +type MessageIds = 'invalidParamCount' | 'invalidParamName' | 'invalidRefType'; + +export const rule = ESLintUtils.RuleCreator(() => __filename)({ + name: RULE_NAME, + meta: { + type: 'problem', + docs: { + description: + 'Enforce the API contract for v9 "base" hooks (`useBase_unstable`) and their paired wrapping state hooks (`use_unstable` declared in the same file or a sibling component-folder file): a required `props` parameter and an optional `ref` parameter typed as `React.Ref<...>`.', + }, + schema: [], + messages: { + invalidParamCount: + 'Hook `{{hookName}}` must take 1 or 2 positional parameters (`props`, optional `ref`), got {{actual}}.', + invalidParamName: + 'Hook `{{hookName}}` parameter #{{index}} must be named `{{expected}}` (Identifier), got `{{actual}}`.', + invalidRefType: 'Hook `{{hookName}}` parameter `ref` must be typed as `React.Ref<...>`, got `{{actual}}`.', + }, + }, + defaultOptions: [], + create(context) { + const sourceCode = context.sourceCode; + const pairDetector = createPairDetector(context.filename); + + /** + * Validates the hook signature: 1 or 2 positional params, first must be Identifier `props`, + * optional second must be Identifier `ref` typed as `React.Ref<...>` (verified to originate + * from the `react` package so collisions with same-named locals don't pass). + */ + function checkParameters(hookName: string, hookFn: BaseHookFunction, reportNode: TSESTree.Node): void { + if (hookFn.params.length < MIN_PARAM_COUNT || hookFn.params.length > MAX_PARAM_COUNT) { + context.report({ + node: reportNode, + messageId: 'invalidParamCount', + data: { hookName, actual: hookFn.params.length }, + }); + return; + } + + hookFn.params.forEach((param, index) => { + const expected = EXPECTED_PARAM_NAMES[index]; + if (param.type !== AST_NODE_TYPES.Identifier || param.name !== expected) { + context.report({ + node: reportNode, + messageId: 'invalidParamName', + data: { hookName, index: index + 1, expected, actual: describeParam(param) }, + }); + return; + } + if (index === 1 && !isReactRefTypeAnnotation(param.typeAnnotation, sourceCode.getScope(param))) { + context.report({ + node: reportNode, + messageId: 'invalidRefType', + data: { hookName, actual: describeRefType(param.typeAnnotation) }, + }); + } + }); + } + + return { + // Populate the pair detector's same-file index from top-level declarations so the state-hook + // visitor can synchronously decide pairing for the same-file case (82 / 85 occurrences across + // react-components). + Program(node: TSESTree.Program): void { + for (const stmt of node.body) { + collectBaseHookNames(stmt, pairDetector.baseHooksInCurrentFile); + } + }, + + // Broader selector matches both base hooks and state-hook candidates; the handler dispatches + // by name. State hooks only get the signature check when paired with a sibling base hook. + [`FunctionDeclaration[id.name=/${STATE_HOOK_NAME_PATTERN.source}/]`]: (node: TSESTree.FunctionDeclaration) => { + // `export default function () {}` produces an anonymous FunctionDeclaration (id === null). + // The esquery selector above requires `id.name`, so this branch should be unreachable in + // practice — kept as a type-narrowing guard so TS treats `node.id` as non-null below. + if (!node.id) { + return; + } + const name = node.id.name; + if (BASE_HOOK_NAME_PATTERN.test(name) || pairDetector.hasPairedBaseHook(name)) { + checkParameters(name, node, node.id); + } + }, + + [`VariableDeclarator[id.name=/${STATE_HOOK_NAME_PATTERN.source}/]`]: (node: TSESTree.VariableDeclarator) => { + const init = getFunctionInit(node); + if (!init || node.id.type !== AST_NODE_TYPES.Identifier) { + return; + } + const name = node.id.name; + if (BASE_HOOK_NAME_PATTERN.test(name) || pairDetector.hasPairedBaseHook(name)) { + checkParameters(name, init, node.id); + } + }, + }; + }, +}); + +// --------------------------------------------------------------------------- +// AST helpers +// --------------------------------------------------------------------------- + +/** + * Returns a human-readable label for a function parameter, used in `invalidParamName` diagnostics + * so the user sees what they actually wrote (destructuring, rest, default value, …) instead of + * just `Identifier`. + */ +function describeParam(param: TSESTree.Parameter): string { + switch (param.type) { + case AST_NODE_TYPES.Identifier: + return param.name; + case AST_NODE_TYPES.ObjectPattern: + return '{ ... }'; + case AST_NODE_TYPES.ArrayPattern: + return '[ ... ]'; + case AST_NODE_TYPES.RestElement: + return '...rest'; + case AST_NODE_TYPES.AssignmentPattern: + return param.left.type === AST_NODE_TYPES.Identifier ? `${param.left.name} = …` : '… = …'; + default: + return param.type; + } +} + +/** + * Returns `true` when `annotation` is `React.Ref<...>` (qualified) or `Ref<...>` (named) AND the + * referenced identifier was imported from the `react` package in the surrounding scope. The scope + * check guards against false positives when a local `Ref` shadows the React import. + */ +function isReactRefTypeAnnotation( + annotation: TSESTree.TSTypeAnnotation | undefined, + scope: TSESLint.Scope.Scope, +): boolean { + if (!annotation) { + return false; + } + const type = annotation.typeAnnotation; + if (type.type !== AST_NODE_TYPES.TSTypeReference) { + return false; + } + const { typeName } = type; + if (typeName.type === AST_NODE_TYPES.Identifier) { + return typeName.name === 'Ref' && isReactImportedIdentifier(typeName, scope, 'Ref'); + } + if (typeName.type === AST_NODE_TYPES.TSQualifiedName) { + return ( + typeName.left.type === AST_NODE_TYPES.Identifier && + typeName.left.name === 'React' && + typeName.right.name === 'Ref' && + isReactImportedIdentifier(typeName.left, scope, '*') + ); + } + return false; +} + +/** + * Resolves the given identifier in `scope` and verifies it was imported from the `react` + * package. `expectedImportedName` is matched against the original import name: + * - a named-import specifier (e.g. `import { Ref } from 'react'`) must match the name, + * - a namespace/default import (e.g. `import * as React from 'react'`) matches `'*'`/`'default'`. + * + * Scope-based (no ParserServices required), so the rule still works without TypeScript type + * information. + */ +function isReactImportedIdentifier( + identifier: TSESTree.Identifier, + scope: TSESLint.Scope.Scope, + expectedImportedName: string, +): boolean { + const variable = findVariableInScope(scope, identifier.name); + if (!variable) { + return false; + } + return variable.defs.some(def => { + if (def.type !== 'ImportBinding') { + return false; + } + const importDecl = def.parent; + if (!importDecl || importDecl.type !== AST_NODE_TYPES.ImportDeclaration) { + return false; + } + if (importDecl.source.value !== 'react') { + return false; + } + const specifier = def.node; + switch (specifier.type) { + case AST_NODE_TYPES.ImportSpecifier: { + const importedName = + specifier.imported.type === AST_NODE_TYPES.Identifier + ? specifier.imported.name + : String(specifier.imported.value); + return importedName === expectedImportedName; + } + case AST_NODE_TYPES.ImportNamespaceSpecifier: + return expectedImportedName === '*'; + case AST_NODE_TYPES.ImportDefaultSpecifier: + // `import React from 'react'` is also a valid way to access `React.Ref`. + return expectedImportedName === '*' || expectedImportedName === 'default'; + default: + return false; + } + }); +} + +/** + * Walks the scope chain looking for a variable with the given name. Plain `scope.set.get` only + * inspects the local scope, so this helper enables identifier resolution that matches JavaScript's + * lookup semantics. + */ +function findVariableInScope(scope: TSESLint.Scope.Scope, name: string): TSESLint.Scope.Variable | undefined { + let current: TSESLint.Scope.Scope | null = scope; + while (current) { + const variable = current.set.get(name); + if (variable) { + return variable; + } + current = current.upper; + } + return undefined; +} + +/** + * Renders the actual ref type annotation as a string for `invalidRefType` diagnostics, so users + * see what they wrote (`HTMLAttributes`, `Ref`, `MyType`…) instead of bare AST node types. + */ +function describeRefType(annotation: TSESTree.TSTypeAnnotation | undefined): string { + if (!annotation) { + return ''; + } + const type = annotation.typeAnnotation; + if (type.type !== AST_NODE_TYPES.TSTypeReference) { + return type.type; + } + const { typeName } = type; + if (typeName.type === AST_NODE_TYPES.Identifier) { + return typeName.name; + } + if (typeName.type === AST_NODE_TYPES.TSQualifiedName) { + const left = typeName.left.type === AST_NODE_TYPES.Identifier ? typeName.left.name : '…'; + return `${left}.${typeName.right.name}`; + } + return type.type; +} diff --git a/tools/eslint-rules/rules/consistent-base-hook.ts b/tools/eslint-rules/rules/consistent-base-hook.ts deleted file mode 100644 index 81da34002f77f3..00000000000000 --- a/tools/eslint-rules/rules/consistent-base-hook.ts +++ /dev/null @@ -1,993 +0,0 @@ -import type { TSESTree, TSESLint, ParserServicesWithTypeInformation } from '@typescript-eslint/utils'; -import { ESLintUtils, AST_NODE_TYPES } from '@typescript-eslint/utils'; -import * as ts from 'typescript'; -import * as fs from 'node:fs'; -import * as path from 'node:path'; - -// NOTE: The rule will be available in ESLint configs as "@nx/workspace-consistent-base-hook" -export const RULE_NAME = 'consistent-base-hook'; - -const BASE_HOOK_NAME_PATTERN = /^use[A-Z]\w*Base_unstable$/; -// State hook candidates are any `_unstable` hook NOT ending in `Base_unstable`. They are only -// subjected to the signature contract when they are paired with a sibling base hook (Option C — -// pair detection). See `hasPairedBaseHook`. -const STATE_HOOK_NAME_PATTERN = /^use[A-Z]\w*_unstable$/; -const BASE_SUFFIX = 'Base_unstable'; -const UNSTABLE_SUFFIX = '_unstable'; -const SIBLING_EXTENSIONS: ReadonlyArray = ['.ts', '.tsx']; -const EXPECTED_PARAM_NAMES = ['props', 'ref'] as const; -const MIN_PARAM_COUNT = 1; -const MAX_PARAM_COUNT = 2; - -const DEFAULT_WATCHED_PACKAGES: ReadonlyArray = ['@fluentui/react-tabster']; -const DEFAULT_FORBIDDEN_RUNTIMES: ReadonlyArray = ['tabster']; - -type Options = [ - { - /** - * Packages whose imported symbols must be analyzed transitively. - * A symbol imported from one of these packages is allowed inside a base - * hook only if its defining source file does not reach any - * `forbiddenRuntimes` package via value imports. - */ - watchedPackages?: string[]; - /** - * Runtime packages whose presence in the transitive value-import graph of - * a referenced symbol is forbidden inside base hooks. Direct imports from - * these packages are also forbidden. - */ - forbiddenRuntimes?: string[]; - /** - * When `true`, type-only imports (both from `forbiddenRuntimes` packages directly and - * from `watchedPackages` whose defining module reaches a forbidden runtime) are permitted - * inside base hooks. Type-only imports emit no runtime code, so this option trades API - * decoupling for ergonomics. - * - * Defaults to `false` — type-only imports are checked the same way as value imports, to - * keep the base hook's public API fully decoupled from forbidden runtimes. - */ - allowTypeImports?: boolean; - }?, -]; - -type MessageIds = - | 'invalidParamCount' - | 'invalidParamName' - | 'invalidRefType' - | 'forbiddenRuntimeDirect' - | 'forbiddenRuntimeReach' - | 'typedServicesUnavailable'; - -type ImportSpecifierNode = - | TSESTree.ImportSpecifier - | TSESTree.ImportDefaultSpecifier - | TSESTree.ImportNamespaceSpecifier; - -interface TrackedImport { - /** The package the binding came from (a watched OR forbidden-runtime package). */ - package: string; - /** Original imported name (not the local alias). `default` or `*` for default / namespace. */ - importedName: string; - /** Kind of package — controls how the reference is checked. */ - kind: 'watched' | 'forbidden'; - /** - * `true` when the binding is type-only (either the declaration is `import type ...` - * or the specifier is `import { type Foo }`). Used to gate whether direct usage in a - * value position is even possible (type-only bindings only surface in type positions). - */ - isTypeOnly: boolean; - /** The specifier node (used for symbol lookup via ParserServices). */ - specifier: ImportSpecifierNode; -} - -type BaseHookFunction = TSESTree.FunctionDeclaration | TSESTree.FunctionExpression | TSESTree.ArrowFunctionExpression; - -/** - * Result of a single transitive-reach DFS over a source file's import graph. - * - `value` — packages reachable via value (non type-only) imports only. Used to decide - * whether a runtime reference can pull a forbidden runtime at execution time. - * - `all` — packages reachable via value OR type imports. Used to decide whether a - * type reference can leak a forbidden runtime through the public API surface. - * `value` is always a subset of `all`. - */ -interface Reach { - value: ReadonlySet; - all: ReadonlySet; -} - -/** - * Per-Program cache: source file path → reach sets transitively computed from that file. - * Both `value` and `all` sets are filled in a single DFS pass to share resolution work. - * - * Keyed by `ts.Program` identity so the cache is invalidated whenever - * typescript-eslint rebuilds the Program. - */ -const programCache = new WeakMap>(); - -export const rule = ESLintUtils.RuleCreator(() => __filename)({ - name: RULE_NAME, - meta: { - type: 'problem', - docs: { - description: - 'Enforce the API contract for v9 "base" hooks (`useBase_unstable`) and their paired wrapping state hooks (`use_unstable` declared in the same file or sibling component-folder file): a required `props` parameter and an optional `ref` parameter typed as `React.Ref<...>`. Additionally, disallow inside base hooks any binding whose defining module transitively pulls a forbidden runtime package (default `tabster`) \u2014 both at value positions (runtime coupling) and at type positions (API surface coupling).', - }, - schema: [ - { - type: 'object', - properties: { - watchedPackages: { - type: 'array', - items: { type: 'string' }, - uniqueItems: true, - }, - forbiddenRuntimes: { - type: 'array', - items: { type: 'string' }, - uniqueItems: true, - }, - allowTypeImports: { - type: 'boolean', - }, - }, - additionalProperties: false, - }, - ], - messages: { - invalidParamCount: - 'Hook `{{hookName}}` must take 1 or 2 positional parameters (`props`, optional `ref`), got {{actual}}.', - invalidParamName: - 'Hook `{{hookName}}` parameter #{{index}} must be named `{{expected}}` (Identifier), got `{{actual}}`.', - invalidRefType: 'Hook `{{hookName}}` parameter `ref` must be typed as `React.Ref<...>`, got `{{actual}}`.', - forbiddenRuntimeDirect: - 'Base hook `{{hookName}}` cannot reference `{{importedName}}` from forbidden runtime package `{{package}}`. Move logic that depends on `{{package}}` to the wrapping `*_unstable` hook instead.', - forbiddenRuntimeReach: - 'Base hook `{{hookName}}` cannot reference `{{importedName}}` from `{{package}}` because its defining module transitively imports forbidden runtime `{{runtime}}` (via `{{viaFile}}`). Move logic that depends on `{{runtime}}` to the wrapping `*_unstable` hook instead.', - typedServicesUnavailable: - 'consistent-base-hook: transitive runtime analysis was skipped because TypeScript type information is unavailable. Enable typescript-eslint type-aware linting (set `parserOptions.projectService: true` or `parserOptions.project`) so references through watched packages (e.g. `{{watchedPackages}}`) can be verified against forbidden runtimes (e.g. `{{forbiddenRuntimes}}`).', - }, - }, - defaultOptions: [{}], - create(context) { - const sourceCode = context.sourceCode; - const options = context.options[0] ?? {}; - const watchedPackages = new Set(options.watchedPackages ?? DEFAULT_WATCHED_PACKAGES); - const forbiddenRuntimes = new Set(options.forbiddenRuntimes ?? DEFAULT_FORBIDDEN_RUNTIMES); - const allowTypeImports = options.allowTypeImports ?? false; - // `forbidden` takes precedence: if the same name appears in both lists, treat the binding as forbidden. - const trackedPackages = new Set([...watchedPackages, ...forbiddenRuntimes]); - - // Map of locally-declared variable identity → original import origin metadata. Keyed by Variable - // identity (not name) so re-declarations / shadowing inside the base hook resolve correctly. - const trackedImports = new Map(); - - // Names of base hooks (`useBase_unstable`) declared at the top level of the current file. - // Populated on `Program` enter so the state-hook visitor can synchronously decide pairing for - // the same-file (82 / 85) case without re-scanning. - const baseHooksInCurrentFile = new Set(); - - // Caches sibling-file existence checks (`./useXBase.ts` / `./useXBase.tsx`) for the lifetime of - // this rule instance. The keys are absolute candidate paths; values are `true` if the file exists - // on disk. Used to cover the 3 outliers where the base hook and wrapping hook live in sibling - // files within the same component folder (Tooltip, Field, MenuItem). - const siblingFileExistsCache = new Map(); - - // Tracks whether `computeSymbolReach` was invoked while typed services were unavailable. When set, - // we emit a single diagnostic on `Program:exit` so the user knows transitive analysis was skipped. - let typedServicesNeededButMissing = false; - - // Lazily-acquired typed services. Resolved once per file, cached in `typedServices` (undefined = - // not yet attempted, null = attempted and unavailable, value = available). - let typedServices: ParserServicesWithTypeInformation | null | undefined; - - /** - * Returns typed services (TS Program + checker) for the current file, or `null` if untyped - * lint is in effect. Result is memoized for the lifetime of the per-file rule instance. - */ - function getTypedServices(): ParserServicesWithTypeInformation | null { - if (typedServices !== undefined) { - return typedServices; - } - try { - typedServices = ESLintUtils.getParserServices(context); - } catch { - typedServices = null; - } - return typedServices; - } - - /** - * Entry point invoked for every function matching the base-hook naming pattern. Runs the - * signature check first (cheap, AST-only) and then walks the body for forbidden references. - */ - function checkBaseHook(hookName: string, hookFn: BaseHookFunction, reportNode: TSESTree.Node): void { - checkParameters(hookName, hookFn, reportNode); - checkBodyReferences(hookName, hookFn); - } - - /** - * Returns `true` when `stateHookName` (e.g. `useFoo_unstable`) is paired with a base hook - * `useFooBase_unstable` declared either in the current file or in a sibling file within the - * same directory (component folder convention `components//useXBase.ts(x)`). - * - * The base hook is the structural marker for the `(props, ref)` contract. When found, the - * wrapping state hook is required to honor the same contract; otherwise it is left alone, - * which avoids false positives on unrelated `_unstable` hooks such as - * `useFooContextValues_unstable`, `useFooStyles_unstable`, etc. - */ - function hasPairedBaseHook(stateHookName: string): boolean { - const baseHookName = stateHookName.slice(0, -UNSTABLE_SUFFIX.length) + BASE_SUFFIX; - if (baseHooksInCurrentFile.has(baseHookName)) { - return true; - } - const filename = context.filename; - // ESLint passes synthetic filenames like `` for inline code; nothing to check. - if (!filename || !path.isAbsolute(filename)) { - return false; - } - const dir = path.dirname(filename); - // Sibling base-hook file (the wrapping hook lives in `useFoo.tsx`, the base in `useFooBase.tsx`). - const siblingBasename = baseHookName.slice(0, -UNSTABLE_SUFFIX.length); // e.g. `useFooBase` - for (const ext of SIBLING_EXTENSIONS) { - const candidate = path.join(dir, siblingBasename + ext); - if (candidate === filename) { - continue; - } - let exists = siblingFileExistsCache.get(candidate); - if (exists === undefined) { - try { - exists = fs.statSync(candidate).isFile(); - } catch { - exists = false; - } - siblingFileExistsCache.set(candidate, exists); - } - if (exists) { - return true; - } - } - return false; - } - - /** - * Validates the base-hook signature: 1 or 2 positional params, first must be Identifier `props`, - * optional second must be Identifier `ref` typed as `React.Ref<...>` (verified to originate from - * the `react` package so collisions with same-named locals don't pass). - */ - function checkParameters(hookName: string, hookFn: BaseHookFunction, reportNode: TSESTree.Node): void { - if (hookFn.params.length < MIN_PARAM_COUNT || hookFn.params.length > MAX_PARAM_COUNT) { - context.report({ - node: reportNode, - messageId: 'invalidParamCount', - data: { hookName, actual: hookFn.params.length }, - }); - return; - } - - hookFn.params.forEach((param, index) => { - const expected = EXPECTED_PARAM_NAMES[index]; - if (param.type !== AST_NODE_TYPES.Identifier || param.name !== expected) { - context.report({ - node: reportNode, - messageId: 'invalidParamName', - data: { hookName, index: index + 1, expected, actual: describeParam(param) }, - }); - return; - } - if (index === 1 && !isReactRefTypeAnnotation(param.typeAnnotation, sourceCode.getScope(param))) { - context.report({ - node: reportNode, - messageId: 'invalidRefType', - data: { hookName, actual: describeRefType(param.typeAnnotation) }, - }); - } - }); - } - - /** - * Walks the base hook's scope graph looking for references to any tracked import. Bails out - * early if nothing is tracked, so the typical case (no watched/forbidden imports in the file) - * stays free. - */ - function checkBodyReferences(hookName: string, hookFn: BaseHookFunction): void { - if (trackedImports.size === 0) { - return; - } - const hookScope = sourceCode.getScope(hookFn); - visitScope(hookScope, hookFn, hookName); - } - - /** - * Recursively visits `scope` and all its descendants that are still inside the base hook body. - * For every resolved reference whose declaration is a tracked import, either flag the direct - * usage or delegate to `computeSymbolReach` for the transitive check. - * - * The chosen reach set depends on the reference position: - * - value reference → `value` reach (runtime coupling) - * - type reference → `all` reach (API coupling — a type alias can still tie the public - * API to a forbidden runtime via its defining module) - */ - function visitScope(scope: TSESLint.Scope.Scope, hookFn: BaseHookFunction, hookName: string): void { - if (!isScopeWithinFunction(scope, hookFn)) { - return; - } - - scope.references.forEach(reference => { - const resolved = reference.resolved; - if (!resolved) { - return; - } - const origin = trackedImports.get(resolved); - if (!origin) { - return; - } - - const isTypeRef = reference.isTypeReference === true; - // A type-only binding can only legally appear in type positions; ignore the (invalid) - // value reference — TS will flag it independently. - if (origin.isTypeOnly && !isTypeRef) { - return; - } - - if (origin.kind === 'forbidden') { - context.report({ - node: reference.identifier, - messageId: 'forbiddenRuntimeDirect', - data: { - hookName, - importedName: origin.importedName, - package: origin.package, - }, - }); - return; - } - - // Watched package: only flag if the defining module transitively reaches a forbidden runtime. - const reach = computeSymbolReach(origin); - if (!reach) { - return; // untyped lint or unresolvable — silently skip - } - const reached = isTypeRef ? reach.all : reach.value; - - for (const runtime of forbiddenRuntimes) { - if (reached.has(runtime)) { - context.report({ - node: reference.identifier, - messageId: 'forbiddenRuntimeReach', - data: { - hookName, - importedName: origin.importedName, - package: origin.package, - runtime, - viaFile: reach.viaFile, - }, - }); - return; - } - } - }); - - scope.childScopes.forEach(child => visitScope(child, hookFn, hookName)); - } - - /** - * Resolves the watched-package import to its defining module via TS `Program`, then queries the - * transitive import graph for forbidden runtimes (both value-only and value+type sets). - * Returns `null` (and flips the `typedServicesNeededButMissing` flag) when typed services - * aren't available, so the caller can silently skip and we can warn once on `Program:exit`. - */ - function computeSymbolReach( - origin: TrackedImport, - ): { value: ReadonlySet; all: ReadonlySet; viaFile: string } | null { - const services = getTypedServices(); - if (!services) { - typedServicesNeededButMissing = true; - return null; - } - const checker = services.program.getTypeChecker(); - const tsNode = services.esTreeNodeToTSNodeMap.get(origin.specifier); - if (!tsNode) { - return null; - } - - // For an ImportSpecifier we want the imported (right-hand) identifier so the symbol resolves to - // the exported name on the source module, not the local alias. - let nameNode: ts.Node | undefined; - if (ts.isImportSpecifier(tsNode)) { - nameNode = tsNode.propertyName ?? tsNode.name; - } else if (ts.isImportClause(tsNode) || ts.isNamespaceImport(tsNode)) { - nameNode = tsNode.name; - } else { - nameNode = tsNode; - } - if (!nameNode) { - return null; - } - - let symbol = checker.getSymbolAtLocation(nameNode); - if (!symbol) { - return null; - } - // eslint-disable-next-line no-bitwise -- ts.SymbolFlags is a bitfield enum - if ((symbol.flags & ts.SymbolFlags.Alias) !== 0) { - try { - symbol = checker.getAliasedSymbol(symbol); - } catch { - return null; - } - } - const declaration = symbol.declarations?.[0]; - const definingFile = declaration?.getSourceFile(); - if (!definingFile) { - return null; - } - - const reach = transitiveReach(services.program, definingFile); - return { value: reach.value, all: reach.all, viaFile: shortenPath(definingFile.fileName) }; - } - - /** - * `ImportDeclaration` visitor: records every named/default/namespace specifier coming from a - * watched or forbidden-runtime package so body references can later be resolved via - * `sourceCode.getDeclaredVariables`. Tracks both value AND type-only specifiers — type refs - * are still inspected at the hook signature because a type from a watched package can - * transitively expose a forbidden runtime through its defining module. - */ - function trackImportDeclaration(node: TSESTree.ImportDeclaration): void { - const source = node.source.value; - if (typeof source !== 'string' || !trackedPackages.has(source)) { - return; - } - const isForbiddenPkg = forbiddenRuntimes.has(source); - const stmtTypeOnly = node.importKind === 'type'; - // Symmetric semantics: when `allowTypeImports` is true, type-only imports are exempt from - // both direct forbidden-runtime checks AND transitive watched-package reach checks (a type - // can never pull runtime code at execution time). - if (stmtTypeOnly && allowTypeImports) { - return; - } - const kind: TrackedImport['kind'] = isForbiddenPkg ? 'forbidden' : 'watched'; - - node.specifiers.forEach(specifier => { - const specTypeOnly = - stmtTypeOnly || (specifier.type === AST_NODE_TYPES.ImportSpecifier && specifier.importKind === 'type'); - if (specTypeOnly && allowTypeImports) { - return; - } - const importedName = getImportedName(specifier); - if (importedName === undefined) { - return; - } - for (const variable of sourceCode.getDeclaredVariables(specifier)) { - trackedImports.set(variable, { - package: source, - importedName, - kind, - isTypeOnly: specTypeOnly, - specifier, - }); - } - }); - } - - /** - * Returns the function literal initializer of a `VariableDeclarator` when the declarator is a - * plain Identifier bound to an inline arrow/function expression; otherwise `undefined`. Skips - * destructuring patterns (no inspectable function literal) and non-function initializers - * (call expressions, identifier aliases, missing initializer for ambients). - */ - function getFunctionInit(node: TSESTree.VariableDeclarator): BaseHookFunction | undefined { - if (node.id.type !== AST_NODE_TYPES.Identifier) { - return undefined; - } - const init = node.init; - if ( - !init || - (init.type !== AST_NODE_TYPES.ArrowFunctionExpression && init.type !== AST_NODE_TYPES.FunctionExpression) - ) { - return undefined; - } - return init; - } - - return { - // Populate `baseHooksInCurrentFile` from top-level declarations so the state-hook visitor can - // synchronously decide pairing for the same-file case (82 / 85 occurrences across react-components). - Program(node: TSESTree.Program): void { - for (const stmt of node.body) { - collectBaseHookNames(stmt, baseHooksInCurrentFile); - } - }, - - ImportDeclaration: trackImportDeclaration, - - // Broader selector matches both base hooks and state-hook candidates; the handler dispatches - // by name. State hooks only get the signature check when paired with a sibling base hook. - [`FunctionDeclaration[id.name=/${STATE_HOOK_NAME_PATTERN.source}/]`]: (node: TSESTree.FunctionDeclaration) => { - // `export default function () {}` produces an anonymous FunctionDeclaration (id === null). - // The esquery selector above requires `id.name`, so this branch should be unreachable in - // practice — kept as a type-narrowing guard so TS treats `node.id` as non-null below. - if (!node.id) { - return; - } - const name = node.id.name; - if (BASE_HOOK_NAME_PATTERN.test(name)) { - checkBaseHook(name, node, node.id); - } else if (hasPairedBaseHook(name)) { - checkParameters(name, node, node.id); - } - }, - - [`VariableDeclarator[id.name=/${STATE_HOOK_NAME_PATTERN.source}/]`]: (node: TSESTree.VariableDeclarator) => { - const init = getFunctionInit(node); - if (!init || node.id.type !== AST_NODE_TYPES.Identifier) { - return; - } - const name = node.id.name; - if (BASE_HOOK_NAME_PATTERN.test(name)) { - checkBaseHook(name, init, node.id); - } else if (hasPairedBaseHook(name)) { - checkParameters(name, init, node.id); - } - }, - - /** - * One-shot diagnostic so the user is informed (rather than silently degraded) when the - * transitive runtime check was needed but skipped due to missing typed services. - */ - 'Program:exit'(programNode) { - if (!typedServicesNeededButMissing) { - return; - } - context.report({ - node: programNode, - messageId: 'typedServicesUnavailable', - data: { - watchedPackages: [...watchedPackages].join(', '), - forbiddenRuntimes: [...forbiddenRuntimes].join(', '), - }, - }); - }, - }; - }, -}); - -// --------------------------------------------------------------------------- -// Transitive import-graph analysis -// --------------------------------------------------------------------------- - -/** - * Returns the bare package specifiers transitively reachable from `sourceFile`, computed in two - * granularities in a single DFS pass: - * - `value` — only value (non type-only) imports are followed. Used to decide whether a runtime - * reference can pull a forbidden runtime at execution time. - * - `all` — both value and type imports are followed. Used to decide whether a type reference - * can leak a forbidden runtime through the public API surface of a base hook. - * - * Memoized per Program × file. Cycle-safe. - */ -function transitiveReach(program: ts.Program, sourceFile: ts.SourceFile): Reach { - let cache = programCache.get(program); - if (!cache) { - cache = new Map(); - programCache.set(program, cache); - } - return computeReach(program, sourceFile, cache, new Set()); -} - -/** - * Recursive worker for `transitiveReach`. Walks the import graph DFS, recording every bare - * specifier encountered (separately for value-only vs value+type follow modes) and recursing into - * each resolved source file. Uses `inProgress` to break cycles (cycle hits return empty sets - * without caching, so the originating call still commits the complete result). - */ -function computeReach( - program: ts.Program, - sourceFile: ts.SourceFile, - cache: Map, - inProgress: Set, -): Reach { - const cached = cache.get(sourceFile.fileName); - if (cached) { - return cached; - } - if (inProgress.has(sourceFile.fileName)) { - // Cycle: return empty sets without caching so the eventual full result is committed by the originator. - return EMPTY_REACH; - } - inProgress.add(sourceFile.fileName); - - const value = new Set(); - const all = new Set(); - for (const imp of collectImports(sourceFile)) { - if (isBareSpecifier(imp.specifier)) { - const pkg = packageNameOf(imp.specifier); - all.add(pkg); - if (!imp.typeOnly) { - value.add(pkg); - } - } - const resolved = resolveModule(program, sourceFile, imp.specifier, imp.literal); - if (!resolved) { - continue; - } - const childSourceFile = program.getSourceFile(resolved); - if (!childSourceFile) { - continue; - } - const childReach = computeReach(program, childSourceFile, cache, inProgress); - for (const pkg of childReach.all) { - all.add(pkg); - } - if (!imp.typeOnly) { - // A type-only edge does not propagate runtime reach: it can only widen the `all` set. - for (const pkg of childReach.value) { - value.add(pkg); - } - } - } - - inProgress.delete(sourceFile.fileName); - const result: Reach = { value, all }; - cache.set(sourceFile.fileName, result); - return result; -} - -const EMPTY_REACH: Reach = { value: new Set(), all: new Set() }; - -interface ImportEdge { - specifier: string; - literal: ts.StringLiteralLike; - /** `true` when this edge only carries type information (no runtime side-effect). */ - typeOnly: boolean; -} - -/** - * Enumerates every module specifier in `sourceFile`, tagging each edge as value (`typeOnly: false`) - * or type-only (`typeOnly: true`). `import type` / `export type`, fully type-only named import or - * export clauses, and `import type =` are emitted with `typeOnly: true`. Side-effect imports - * (no clause) are emitted as value edges. - */ -function collectImports(sourceFile: ts.SourceFile): ImportEdge[] { - const result: ImportEdge[] = []; - for (const stmt of sourceFile.statements) { - if (ts.isImportDeclaration(stmt)) { - let typeOnly = false; - if (stmt.importClause?.isTypeOnly) { - typeOnly = true; - } else if ( - stmt.importClause && - stmt.importClause.namedBindings && - ts.isNamedImports(stmt.importClause.namedBindings) - ) { - const named = stmt.importClause.namedBindings; - const hasValue = !!stmt.importClause.name || named.elements.some(element => !element.isTypeOnly); - typeOnly = !hasValue; - } - if (ts.isStringLiteralLike(stmt.moduleSpecifier)) { - result.push({ specifier: stmt.moduleSpecifier.text, literal: stmt.moduleSpecifier, typeOnly }); - } - continue; - } - if (ts.isExportDeclaration(stmt) && stmt.moduleSpecifier && ts.isStringLiteralLike(stmt.moduleSpecifier)) { - let typeOnly = stmt.isTypeOnly; - if (!typeOnly && stmt.exportClause && ts.isNamedExports(stmt.exportClause)) { - typeOnly = stmt.exportClause.elements.every(element => element.isTypeOnly); - } - result.push({ specifier: stmt.moduleSpecifier.text, literal: stmt.moduleSpecifier, typeOnly }); - continue; - } - if ( - ts.isImportEqualsDeclaration(stmt) && - ts.isExternalModuleReference(stmt.moduleReference) && - ts.isStringLiteralLike(stmt.moduleReference.expression) - ) { - result.push({ - specifier: stmt.moduleReference.expression.text, - literal: stmt.moduleReference.expression, - typeOnly: stmt.isTypeOnly, - }); - } - } - return result; -} - -/** - * Resolves `specifier` (as used in `sourceFile`) to an absolute file path using the same algorithm - * the host TS Program uses. Prefers the (faster) `program.getResolvedModule` API exposed in TS ≥ 5.3 - * and falls back to `ts.resolveModuleName` for older toolchains. Returns `undefined` if the module - * cannot be resolved (e.g. ambient declarations, broken paths). - */ -function resolveModule( - program: ts.Program, - sourceFile: ts.SourceFile, - specifier: string, - literal: ts.StringLiteralLike, -): string | undefined { - // TS ≥ 5.3 exposes program.getResolvedModule - const getResolvedModule = ( - program as unknown as { - getResolvedModule?: ( - file: ts.SourceFile, - moduleName: string, - mode?: ts.ResolutionMode, - ) => { resolvedModule?: ts.ResolvedModuleFull } | undefined; - } - ).getResolvedModule; - const mode = ( - ts as unknown as { - getModeForUsageLocation?: (file: ts.SourceFile, usage: ts.StringLiteralLike) => ts.ResolutionMode; - } - ).getModeForUsageLocation?.(sourceFile, literal); - - if (typeof getResolvedModule === 'function') { - const resolutionResult = getResolvedModule.call(program, sourceFile, specifier, mode); - if (resolutionResult?.resolvedModule) { - return resolutionResult.resolvedModule.resolvedFileName; - } - } - - // Fallback for older TS: use ts.resolveModuleName against the compiler host. - const compilerOptions = program.getCompilerOptions(); - const host = - (program as unknown as { getCompilerHost?: () => ts.ModuleResolutionHost }).getCompilerHost?.() ?? ts.sys; - const result = ts.resolveModuleName( - specifier, - sourceFile.fileName, - compilerOptions, - host as ts.ModuleResolutionHost, - undefined, - undefined, - mode, - ); - return result.resolvedModule?.resolvedFileName; -} - -/** - * `true` when the import specifier refers to a package (e.g. `react`, `@scope/pkg`, `pkg/sub`) - * rather than a relative or absolute path. - */ -function isBareSpecifier(specifier: string): boolean { - return !specifier.startsWith('.') && !specifier.startsWith('/'); -} - -/** - * Extracts the npm package name from a bare specifier. Handles both unscoped (`pkg/sub` → `pkg`) - * and scoped (`@scope/pkg/sub` → `@scope/pkg`) forms. - */ -function packageNameOf(specifier: string): string { - if (specifier.startsWith('@')) { - const [scope, name] = specifier.split('/', 2); - return name ? `${scope}/${name}` : scope; - } - const slash = specifier.indexOf('/'); - return slash === -1 ? specifier : specifier.slice(0, slash); -} - -/** - * Shortens an absolute file path for display in diagnostics: returns the part after the last - * `node_modules/` segment when present (so users see e.g. `tabster/dist/index.js`), or makes the - * path workspace-relative when inside the current working directory. - */ -function shortenPath(absolute: string): string { - const marker = '/node_modules/'; - const idx = absolute.lastIndexOf(marker); - if (idx !== -1) { - return absolute.slice(idx + marker.length); - } - const cwd = process.cwd(); - if (absolute.startsWith(cwd)) { - return absolute.slice(cwd.length + 1); - } - return absolute; -} - -// --------------------------------------------------------------------------- -// AST helpers (unchanged from previous version) -// --------------------------------------------------------------------------- - -/** - * Collects names of top-level declarations matching `BASE_HOOK_NAME_PATTERN` into `out`. - * Handles both `export const useFooBase_unstable = ...` (incl. `export const` chains) and - * `export function useFooBase_unstable() {}`, plus the unexported / `export { ... }` forms. - */ -function collectBaseHookNames(stmt: TSESTree.Node, out: Set): void { - // `export const useFooBase_unstable = ...` / `export function useFooBase_unstable() {}` - if (stmt.type === AST_NODE_TYPES.ExportNamedDeclaration && stmt.declaration) { - collectBaseHookNames(stmt.declaration, out); - return; - } - // `function useFooBase_unstable() {}` - if (stmt.type === AST_NODE_TYPES.FunctionDeclaration) { - if (stmt.id && BASE_HOOK_NAME_PATTERN.test(stmt.id.name)) { - out.add(stmt.id.name); - } - return; - } - // `const useFooBase_unstable = ...` (incl. multi-declarator forms) - if (stmt.type === AST_NODE_TYPES.VariableDeclaration) { - for (const decl of stmt.declarations) { - if (decl.id.type === AST_NODE_TYPES.Identifier && BASE_HOOK_NAME_PATTERN.test(decl.id.name)) { - out.add(decl.id.name); - } - } - } -} - -/** - * Resolves the original (imported) name from an import specifier. - * Returns `'default'` for default imports, `'*'` for namespace imports, - * and the imported identifier/string-literal name for named imports. - */ -function getImportedName(specifier: ImportSpecifierNode): string | undefined { - switch (specifier.type) { - case AST_NODE_TYPES.ImportSpecifier: - return specifier.imported.type === AST_NODE_TYPES.Identifier - ? specifier.imported.name - : String(specifier.imported.value); - case AST_NODE_TYPES.ImportDefaultSpecifier: - return 'default'; - case AST_NODE_TYPES.ImportNamespaceSpecifier: - return '*'; - default: - return undefined; - } -} - -/** - * Returns a human-readable label for a function parameter, used in `invalidParamName` diagnostics - * so the user sees what they actually wrote (destructuring, rest, default value, …) instead of - * just `Identifier`. - */ -function describeParam(param: TSESTree.Parameter): string { - switch (param.type) { - case AST_NODE_TYPES.Identifier: - return param.name; - case AST_NODE_TYPES.ObjectPattern: - return '{ ... }'; - case AST_NODE_TYPES.ArrayPattern: - return '[ ... ]'; - case AST_NODE_TYPES.RestElement: - return '...rest'; - case AST_NODE_TYPES.AssignmentPattern: - return param.left.type === AST_NODE_TYPES.Identifier ? `${param.left.name} = …` : '… = …'; - default: - return param.type; - } -} - -/** - * Returns `true` when `annotation` is `React.Ref<...>` (qualified) or `Ref<...>` (named) AND the - * referenced identifier was imported from the `react` package in the surrounding scope. The scope - * check guards against false positives when a local `Ref` shadows the React import. - */ -function isReactRefTypeAnnotation( - annotation: TSESTree.TSTypeAnnotation | undefined, - scope: TSESLint.Scope.Scope, -): boolean { - if (!annotation) { - return false; - } - const type = annotation.typeAnnotation; - if (type.type !== AST_NODE_TYPES.TSTypeReference) { - return false; - } - const { typeName } = type; - if (typeName.type === AST_NODE_TYPES.Identifier) { - return typeName.name === 'Ref' && isReactImportedIdentifier(typeName, scope, 'Ref'); - } - if (typeName.type === AST_NODE_TYPES.TSQualifiedName) { - return ( - typeName.left.type === AST_NODE_TYPES.Identifier && - typeName.left.name === 'React' && - typeName.right.name === 'Ref' && - isReactImportedIdentifier(typeName.left, scope, '*') - ); - } - return false; -} - -/** - * Resolves the given identifier in `scope` and verifies it was imported from the `react` - * package. `expectedImportedName` is matched against the original import name: - * - a named-import specifier (e.g. `import { Ref } from 'react'`) must match the name, - * - a namespace/default import (e.g. `import * as React from 'react'`) matches `'*'`/`'default'`. - * - * Untyped fallback (scope-only): does not require ParserServices, so the rule still works - * without TypeScript type information. - */ -function isReactImportedIdentifier( - identifier: TSESTree.Identifier, - scope: TSESLint.Scope.Scope, - expectedImportedName: string, -): boolean { - const variable = findVariableInScope(scope, identifier.name); - if (!variable) { - return false; - } - return variable.defs.some(def => { - if (def.type !== 'ImportBinding') { - return false; - } - const importDecl = def.parent; - if (!importDecl || importDecl.type !== AST_NODE_TYPES.ImportDeclaration) { - return false; - } - if (importDecl.source.value !== 'react') { - return false; - } - const specifier = def.node; - switch (specifier.type) { - case AST_NODE_TYPES.ImportSpecifier: { - const importedName = - specifier.imported.type === AST_NODE_TYPES.Identifier - ? specifier.imported.name - : String(specifier.imported.value); - return importedName === expectedImportedName; - } - case AST_NODE_TYPES.ImportNamespaceSpecifier: - return expectedImportedName === '*'; - case AST_NODE_TYPES.ImportDefaultSpecifier: - // `import React from 'react'` is also a valid way to access `React.Ref`. - return expectedImportedName === '*' || expectedImportedName === 'default'; - default: - return false; - } - }); -} - -/** - * Walks the scope chain looking for a variable with the given name. Plain `scope.set.get` only - * inspects the local scope, so this helper enables identifier resolution that matches JavaScript's - * lookup semantics. - */ -function findVariableInScope(scope: TSESLint.Scope.Scope, name: string): TSESLint.Scope.Variable | undefined { - let current: TSESLint.Scope.Scope | null = scope; - while (current) { - const variable = current.set.get(name); - if (variable) { - return variable; - } - current = current.upper; - } - return undefined; -} - -/** - * Renders the actual ref type annotation as a string for `invalidRefType` diagnostics, so users - * see what they wrote (`HTMLAttributes`, `Ref`, `MyType`…) instead of bare AST node types. - */ -function describeRefType(annotation: TSESTree.TSTypeAnnotation | undefined): string { - if (!annotation) { - return ''; - } - const type = annotation.typeAnnotation; - if (type.type !== AST_NODE_TYPES.TSTypeReference) { - return type.type; - } - const { typeName } = type; - if (typeName.type === AST_NODE_TYPES.Identifier) { - return typeName.name; - } - if (typeName.type === AST_NODE_TYPES.TSQualifiedName) { - const left = typeName.left.type === AST_NODE_TYPES.Identifier ? typeName.left.name : '…'; - return `${left}.${typeName.right.name}`; - } - return type.type; -} - -/** - * `true` when `scope` (or any of its ancestor scopes) is the function scope of `hookFn`. Used to - * confine the body-reference walk to the base hook itself — references in sibling functions are - * out of scope for this rule. - */ -function isScopeWithinFunction(scope: TSESLint.Scope.Scope, hookFn: BaseHookFunction): boolean { - let current: TSESLint.Scope.Scope | null = scope; - while (current) { - if (current.block === hookFn) { - return true; - } - current = current.upper; - } - return false; -} diff --git a/tools/eslint-rules/utils/base-hook-detector.ts b/tools/eslint-rules/utils/base-hook-detector.ts new file mode 100644 index 00000000000000..21d37b2dd13cfc --- /dev/null +++ b/tools/eslint-rules/utils/base-hook-detector.ts @@ -0,0 +1,140 @@ +import type { TSESTree } from '@typescript-eslint/utils'; +import { AST_NODE_TYPES } from '@typescript-eslint/utils'; +import * as fs from 'node:fs'; +import * as path from 'node:path'; + +/** + * Names of v9 "base hooks": the implementation-only half of a `useFoo` / `useFooBase_unstable` + * pair, kept free of focus/keyboard runtime so it can be composed by callers that may opt out + * of those concerns. This is the structural marker used by both the signature rule and the + * forbidden-runtime rule. + */ +export const BASE_HOOK_NAME_PATTERN = /^use[A-Z]\w*Base_unstable$/; + +/** + * Names of any `_unstable` hook, including base hooks themselves. The signature rule uses this + * broader pattern in its selector and then dispatches by whether the name matches + * `BASE_HOOK_NAME_PATTERN` (always checked) or is a wrapping state hook paired with a base hook + * via {@link hasPairedBaseHook} (only checked when a pair exists). + */ +export const STATE_HOOK_NAME_PATTERN = /^use[A-Z]\w*_unstable$/; + +export const BASE_SUFFIX = 'Base_unstable'; +export const UNSTABLE_SUFFIX = '_unstable'; +const SIBLING_EXTENSIONS: ReadonlyArray = ['.ts', '.tsx']; + +/** + * Any function-literal form a base or paired state hook can take: top-level function declaration, + * inline arrow function, or function expression bound to a variable / export. The signature rule + * runs the same parameter validation against all three. + */ +export type BaseHookFunction = + | TSESTree.FunctionDeclaration + | TSESTree.FunctionExpression + | TSESTree.ArrowFunctionExpression; + +/** + * Collects names of top-level declarations matching `BASE_HOOK_NAME_PATTERN` into `out`. + * Handles both `export const useFooBase_unstable = ...` (incl. `export const` chains) and + * `export function useFooBase_unstable() {}`, plus the unexported / `export { ... }` forms. + */ +export function collectBaseHookNames(stmt: TSESTree.Node, out: Set): void { + // `export const useFooBase_unstable = ...` / `export function useFooBase_unstable() {}` + if (stmt.type === AST_NODE_TYPES.ExportNamedDeclaration && stmt.declaration) { + collectBaseHookNames(stmt.declaration, out); + return; + } + // `function useFooBase_unstable() {}` + if (stmt.type === AST_NODE_TYPES.FunctionDeclaration) { + if (stmt.id && BASE_HOOK_NAME_PATTERN.test(stmt.id.name)) { + out.add(stmt.id.name); + } + return; + } + // `const useFooBase_unstable = ...` (incl. multi-declarator forms) + if (stmt.type === AST_NODE_TYPES.VariableDeclaration) { + for (const decl of stmt.declarations) { + if (decl.id.type === AST_NODE_TYPES.Identifier && BASE_HOOK_NAME_PATTERN.test(decl.id.name)) { + out.add(decl.id.name); + } + } + } +} + +/** + * Returns the function literal initializer of a `VariableDeclarator` when the declarator is a + * plain Identifier bound to an inline arrow/function expression; otherwise `undefined`. Skips + * destructuring patterns (no inspectable function literal) and non-function initializers + * (call expressions, identifier aliases, missing initializer for ambients). + */ +export function getFunctionInit(node: TSESTree.VariableDeclarator): BaseHookFunction | undefined { + if (node.id.type !== AST_NODE_TYPES.Identifier) { + return undefined; + } + const init = node.init; + if ( + !init || + (init.type !== AST_NODE_TYPES.ArrowFunctionExpression && init.type !== AST_NODE_TYPES.FunctionExpression) + ) { + return undefined; + } + return init; +} + +/** + * Stateful helper that decides whether a wrapping state hook (`useFoo_unstable`) is paired with + * a sibling base hook (`useFooBase_unstable`) — either declared in the same file or as a sibling + * `.ts` / `.tsx` file in the same directory. The base hook is the structural marker; when found, + * the wrapping hook is required to honor the `(props, ref)` signature contract. + * + * Per-instance state: + * - `baseHooksInCurrentFile` is populated by the rule's `Program` visitor. + * - `siblingFileExistsCache` memoizes `fs.statSync` results so each component directory pays at + * most one syscall per linted run. + * + * Anchoring detection on the base hook eliminates false positives on other `_unstable` hooks + * (e.g. `useFooContextValues_unstable`, `useFooStyles_unstable`) that intentionally take + * different signatures. + */ +export function createPairDetector(filename: string | undefined) { + const baseHooksInCurrentFile = new Set(); + const siblingFileExistsCache = new Map(); + + function hasPairedBaseHook(stateHookName: string): boolean { + const baseHookName = stateHookName.slice(0, -UNSTABLE_SUFFIX.length) + BASE_SUFFIX; + if (baseHooksInCurrentFile.has(baseHookName)) { + return true; + } + // ESLint passes synthetic filenames like `` for inline code; nothing to check. + if (!filename || !path.isAbsolute(filename)) { + return false; + } + const dir = path.dirname(filename); + // Sibling base-hook file (the wrapping hook lives in `useFoo.tsx`, the base in `useFooBase.tsx`). + const siblingBasename = baseHookName.slice(0, -UNSTABLE_SUFFIX.length); // e.g. `useFooBase` + for (const ext of SIBLING_EXTENSIONS) { + const candidate = path.join(dir, siblingBasename + ext); + if (candidate === filename) { + continue; + } + let exists = siblingFileExistsCache.get(candidate); + if (exists === undefined) { + try { + exists = fs.statSync(candidate).isFile(); + } catch { + exists = false; + } + siblingFileExistsCache.set(candidate, exists); + } + if (exists) { + return true; + } + } + return false; + } + + return { + baseHooksInCurrentFile, + hasPairedBaseHook, + }; +} diff --git a/tools/eslint-rules/utils/module-resolver.ts b/tools/eslint-rules/utils/module-resolver.ts new file mode 100644 index 00000000000000..3a1fa156c6c001 --- /dev/null +++ b/tools/eslint-rules/utils/module-resolver.ts @@ -0,0 +1,91 @@ +import * as ts from 'typescript'; + +/** + * Resolves `specifier` (as used in `sourceFile`) to an absolute file path using the same algorithm + * the host TS Program uses. Prefers the (faster) `program.getResolvedModule` API exposed in TS ≥ 5.3 + * and falls back to `ts.resolveModuleName` for older toolchains. Returns `undefined` if the module + * cannot be resolved (e.g. ambient declarations, broken paths). + */ +export function resolveModule( + program: ts.Program, + sourceFile: ts.SourceFile, + specifier: string, + literal: ts.StringLiteralLike, +): string | undefined { + // TS ≥ 5.3 exposes program.getResolvedModule + const getResolvedModule = ( + program as unknown as { + getResolvedModule?: ( + file: ts.SourceFile, + moduleName: string, + mode?: ts.ResolutionMode, + ) => { resolvedModule?: ts.ResolvedModuleFull } | undefined; + } + ).getResolvedModule; + const mode = ( + ts as unknown as { + getModeForUsageLocation?: (file: ts.SourceFile, usage: ts.StringLiteralLike) => ts.ResolutionMode; + } + ).getModeForUsageLocation?.(sourceFile, literal); + + if (typeof getResolvedModule === 'function') { + const resolutionResult = getResolvedModule.call(program, sourceFile, specifier, mode); + if (resolutionResult?.resolvedModule) { + return resolutionResult.resolvedModule.resolvedFileName; + } + } + + // Fallback for older TS: use ts.resolveModuleName against the compiler host. + const compilerOptions = program.getCompilerOptions(); + const host = + (program as unknown as { getCompilerHost?: () => ts.ModuleResolutionHost }).getCompilerHost?.() ?? ts.sys; + const result = ts.resolveModuleName( + specifier, + sourceFile.fileName, + compilerOptions, + host as ts.ModuleResolutionHost, + undefined, + undefined, + mode, + ); + return result.resolvedModule?.resolvedFileName; +} + +/** + * `true` when the import specifier refers to a package (e.g. `react`, `@scope/pkg`, `pkg/sub`) + * rather than a relative or absolute path. + */ +export function isBareSpecifier(specifier: string): boolean { + return !specifier.startsWith('.') && !specifier.startsWith('/'); +} + +/** + * Extracts the npm package name from a bare specifier. Handles both unscoped (`pkg/sub` → `pkg`) + * and scoped (`@scope/pkg/sub` → `@scope/pkg`) forms. + */ +export function packageNameOf(specifier: string): string { + if (specifier.startsWith('@')) { + const [scope, name] = specifier.split('/', 2); + return name ? `${scope}/${name}` : scope; + } + const slash = specifier.indexOf('/'); + return slash === -1 ? specifier : specifier.slice(0, slash); +} + +/** + * Shortens an absolute file path for display in diagnostics: returns the part after the last + * `node_modules/` segment when present (so users see e.g. `tabster/dist/index.js`), or makes the + * path workspace-relative when inside the current working directory. + */ +export function shortenPath(absolute: string): string { + const marker = '/node_modules/'; + const idx = absolute.lastIndexOf(marker); + if (idx !== -1) { + return absolute.slice(idx + marker.length); + } + const cwd = process.cwd(); + if (absolute.startsWith(cwd)) { + return absolute.slice(cwd.length + 1); + } + return absolute; +} diff --git a/tools/eslint-rules/utils/tracked-imports.ts b/tools/eslint-rules/utils/tracked-imports.ts new file mode 100644 index 00000000000000..f65fa6e1bd75e9 --- /dev/null +++ b/tools/eslint-rules/utils/tracked-imports.ts @@ -0,0 +1,53 @@ +import type { TSESTree } from '@typescript-eslint/utils'; +import { AST_NODE_TYPES } from '@typescript-eslint/utils'; + +/** + * The original (imported) name of an import specifier, used for diagnostics and for matching + * against a forbidden/watched package's exports. + * + * - named import (`import { Foo }`) → `'Foo'` + * - aliased named import (`import { Foo as Bar }`) → `'Foo'` (the original, not the alias) + * - default import (`import X from 'pkg'`) → `'default'` + * - namespace import (`import * as X`) → `'*'` + */ +export type ImportSpecifierNode = + | TSESTree.ImportSpecifier + | TSESTree.ImportDefaultSpecifier + | TSESTree.ImportNamespaceSpecifier; + +export function getImportedName(specifier: ImportSpecifierNode): string | undefined { + switch (specifier.type) { + case AST_NODE_TYPES.ImportSpecifier: + return specifier.imported.type === AST_NODE_TYPES.Identifier + ? specifier.imported.name + : String(specifier.imported.value); + case AST_NODE_TYPES.ImportDefaultSpecifier: + return 'default'; + case AST_NODE_TYPES.ImportNamespaceSpecifier: + return '*'; + default: + return undefined; + } +} + +/** + * A locally-declared binding originating from a tracked import (a watched or forbidden-runtime + * package). Built by the runtime rule when walking `ImportDeclaration` nodes so body references + * can be matched in O(1) via a `Map`. + */ +export interface TrackedImport { + /** The package the binding came from (a watched OR forbidden-runtime package). */ + package: string; + /** Original imported name (not the local alias). `default` or `*` for default / namespace. */ + importedName: string; + /** Kind of package — controls how the reference is checked. */ + kind: 'watched' | 'forbidden'; + /** + * `true` when the binding is type-only (either the declaration is `import type ...` + * or the specifier is `import { type Foo }`). Used to gate whether direct usage in a + * value position is even possible (type-only bindings only surface in type positions). + */ + isTypeOnly: boolean; + /** The specifier node (used for symbol lookup via ParserServices). */ + specifier: ImportSpecifierNode; +} diff --git a/tools/eslint-rules/utils/transitive-reach.ts b/tools/eslint-rules/utils/transitive-reach.ts new file mode 100644 index 00000000000000..13d16c0e4bfe7b --- /dev/null +++ b/tools/eslint-rules/utils/transitive-reach.ts @@ -0,0 +1,160 @@ +import * as ts from 'typescript'; +import { isBareSpecifier, packageNameOf, resolveModule } from './module-resolver'; + +/** + * Result of a single transitive-reach DFS over a source file's import graph. + * - `value` — packages reachable via value (non type-only) imports only. Used to decide + * whether a runtime reference can pull a forbidden runtime at execution time. + * - `all` — packages reachable via value OR type imports. Used to decide whether a + * type reference can leak a forbidden runtime through the public API surface. + * `value` is always a subset of `all`. + */ +export interface Reach { + value: ReadonlySet; + all: ReadonlySet; +} + +export const EMPTY_REACH: Reach = { value: new Set(), all: new Set() }; + +/** + * Per-Program cache: source file path → reach sets transitively computed from that file. + * Both `value` and `all` sets are filled in a single DFS pass to share resolution work. + * + * Keyed by `ts.Program` identity so the cache is invalidated whenever + * typescript-eslint rebuilds the Program. + */ +const programCache = new WeakMap>(); + +/** + * Returns the bare package specifiers transitively reachable from `sourceFile`, computed in two + * granularities in a single DFS pass: + * - `value` — only value (non type-only) imports are followed. Used to decide whether a runtime + * reference can pull a forbidden runtime at execution time. + * - `all` — both value and type imports are followed. Used to decide whether a type reference + * can leak a forbidden runtime through the public API surface of a base hook. + * + * Memoized per Program × file. Cycle-safe. + */ +export function transitiveReach(program: ts.Program, sourceFile: ts.SourceFile): Reach { + let cache = programCache.get(program); + if (!cache) { + cache = new Map(); + programCache.set(program, cache); + } + return computeReach(program, sourceFile, cache, new Set()); +} + +/** + * Recursive worker for `transitiveReach`. Walks the import graph DFS, recording every bare + * specifier encountered (separately for value-only vs value+type follow modes) and recursing into + * each resolved source file. Uses `inProgress` to break cycles (cycle hits return empty sets + * without caching, so the originating call still commits the complete result). + */ +export function computeReach( + program: ts.Program, + sourceFile: ts.SourceFile, + cache: Map, + inProgress: Set, +): Reach { + const cached = cache.get(sourceFile.fileName); + if (cached) { + return cached; + } + if (inProgress.has(sourceFile.fileName)) { + // Cycle: return empty sets without caching so the eventual full result is committed by the originator. + return EMPTY_REACH; + } + inProgress.add(sourceFile.fileName); + + const value = new Set(); + const all = new Set(); + for (const imp of collectImports(sourceFile)) { + if (isBareSpecifier(imp.specifier)) { + const pkg = packageNameOf(imp.specifier); + all.add(pkg); + if (!imp.typeOnly) { + value.add(pkg); + } + } + const resolved = resolveModule(program, sourceFile, imp.specifier, imp.literal); + if (!resolved) { + continue; + } + const childSourceFile = program.getSourceFile(resolved); + if (!childSourceFile) { + continue; + } + const childReach = computeReach(program, childSourceFile, cache, inProgress); + for (const pkg of childReach.all) { + all.add(pkg); + } + if (!imp.typeOnly) { + // A type-only edge does not propagate runtime reach: it can only widen the `all` set. + for (const pkg of childReach.value) { + value.add(pkg); + } + } + } + + inProgress.delete(sourceFile.fileName); + const result: Reach = { value, all }; + cache.set(sourceFile.fileName, result); + return result; +} + +export interface ImportEdge { + specifier: string; + literal: ts.StringLiteralLike; + /** `true` when this edge only carries type information (no runtime side-effect). */ + typeOnly: boolean; +} + +/** + * Enumerates every module specifier in `sourceFile`, tagging each edge as value (`typeOnly: false`) + * or type-only (`typeOnly: true`). `import type` / `export type`, fully type-only named import or + * export clauses, and `import type =` are emitted with `typeOnly: true`. Side-effect imports + * (no clause) are emitted as value edges. + */ +export function collectImports(sourceFile: ts.SourceFile): ImportEdge[] { + const result: ImportEdge[] = []; + for (const stmt of sourceFile.statements) { + if (ts.isImportDeclaration(stmt)) { + let typeOnly = false; + if (stmt.importClause?.isTypeOnly) { + typeOnly = true; + } else if ( + stmt.importClause && + stmt.importClause.namedBindings && + ts.isNamedImports(stmt.importClause.namedBindings) + ) { + const named = stmt.importClause.namedBindings; + const hasValue = !!stmt.importClause.name || named.elements.some(element => !element.isTypeOnly); + typeOnly = !hasValue; + } + if (ts.isStringLiteralLike(stmt.moduleSpecifier)) { + result.push({ specifier: stmt.moduleSpecifier.text, literal: stmt.moduleSpecifier, typeOnly }); + } + continue; + } + if (ts.isExportDeclaration(stmt) && stmt.moduleSpecifier && ts.isStringLiteralLike(stmt.moduleSpecifier)) { + let typeOnly = stmt.isTypeOnly; + if (!typeOnly && stmt.exportClause && ts.isNamedExports(stmt.exportClause)) { + typeOnly = stmt.exportClause.elements.every(element => element.isTypeOnly); + } + result.push({ specifier: stmt.moduleSpecifier.text, literal: stmt.moduleSpecifier, typeOnly }); + continue; + } + if ( + ts.isImportEqualsDeclaration(stmt) && + ts.isExternalModuleReference(stmt.moduleReference) && + ts.isStringLiteralLike(stmt.moduleReference.expression) + ) { + result.push({ + specifier: stmt.moduleReference.expression.text, + literal: stmt.moduleReference.expression, + typeOnly: stmt.isTypeOnly, + }); + } + } + return result; +} From 07a9b9962e51bd75cbe72a1040746d0e7c463392 Mon Sep 17 00:00:00 2001 From: Martin Hochel Date: Tue, 26 May 2026 15:36:48 +0200 Subject: [PATCH 17/18] refactor(eslint-rules): inline runtime helpers into base-hook-no-forbidden-runtime MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The shared utilities split off in the previous commit were only consumed by `base-hook-no-forbidden-runtime`: - `utils/tracked-imports.ts` - `utils/transitive-reach.ts` - `utils/module-resolver.ts` Inline them back into the rule file so `utils/` only contains genuinely cross-rule helpers (currently `base-hook-detector.ts`, used by both the signature and runtime rules). Also drop the `export` keyword from `BASE_SUFFIX` / `UNSTABLE_SUFFIX` in `base-hook-detector.ts` — both are only consumed by the in-file `createPairDetector` factory. --- .../rules/base-hook-no-forbidden-runtime.ts | 318 +++++++++++++++++- .../eslint-rules/utils/base-hook-detector.ts | 4 +- tools/eslint-rules/utils/module-resolver.ts | 91 ----- tools/eslint-rules/utils/tracked-imports.ts | 53 --- tools/eslint-rules/utils/transitive-reach.ts | 160 --------- 5 files changed, 317 insertions(+), 309 deletions(-) delete mode 100644 tools/eslint-rules/utils/module-resolver.ts delete mode 100644 tools/eslint-rules/utils/tracked-imports.ts delete mode 100644 tools/eslint-rules/utils/transitive-reach.ts diff --git a/tools/eslint-rules/rules/base-hook-no-forbidden-runtime.ts b/tools/eslint-rules/rules/base-hook-no-forbidden-runtime.ts index d5c03db7e9f343..2d4a7a6b39c5da 100644 --- a/tools/eslint-rules/rules/base-hook-no-forbidden-runtime.ts +++ b/tools/eslint-rules/rules/base-hook-no-forbidden-runtime.ts @@ -2,9 +2,6 @@ import type { TSESTree, TSESLint, ParserServicesWithTypeInformation } from '@typ import { ESLintUtils, AST_NODE_TYPES } from '@typescript-eslint/utils'; import * as ts from 'typescript'; import { BASE_HOOK_NAME_PATTERN, type BaseHookFunction, getFunctionInit } from '../utils/base-hook-detector'; -import { transitiveReach } from '../utils/transitive-reach'; -import { shortenPath } from '../utils/module-resolver'; -import { type ImportSpecifierNode, type TrackedImport, getImportedName } from '../utils/tracked-imports'; // NOTE: The rule will be available in ESLint configs as "@nx/workspace-base-hook-no-forbidden-runtime" export const RULE_NAME = 'base-hook-no-forbidden-runtime'; @@ -42,6 +39,66 @@ type Options = [ type MessageIds = 'forbiddenRuntimeDirect' | 'forbiddenRuntimeReach' | 'typedServicesUnavailable'; +/** + * The original (imported) name of an import specifier, used for diagnostics and for matching + * against a forbidden/watched package's exports. + * + * - named import (`import { Foo }`) → `'Foo'` + * - aliased named import (`import { Foo as Bar }`) → `'Foo'` (the original, not the alias) + * - default import (`import X from 'pkg'`) → `'default'` + * - namespace import (`import * as X`) → `'*'` + */ +type ImportSpecifierNode = + | TSESTree.ImportSpecifier + | TSESTree.ImportDefaultSpecifier + | TSESTree.ImportNamespaceSpecifier; + +/** + * A locally-declared binding originating from a tracked import (a watched or forbidden-runtime + * package). Built when walking `ImportDeclaration` nodes so body references can be matched in + * O(1) via a `Map`. + */ +interface TrackedImport { + /** The package the binding came from (a watched OR forbidden-runtime package). */ + package: string; + /** Original imported name (not the local alias). `default` or `*` for default / namespace. */ + importedName: string; + /** Kind of package — controls how the reference is checked. */ + kind: 'watched' | 'forbidden'; + /** + * `true` when the binding is type-only (either the declaration is `import type ...` + * or the specifier is `import { type Foo }`). Used to gate whether direct usage in a + * value position is even possible (type-only bindings only surface in type positions). + */ + isTypeOnly: boolean; + /** The specifier node (used for symbol lookup via ParserServices). */ + specifier: ImportSpecifierNode; +} + +/** + * Result of a single transitive-reach DFS over a source file's import graph. + * - `value` — packages reachable via value (non type-only) imports only. Used to decide + * whether a runtime reference can pull a forbidden runtime at execution time. + * - `all` — packages reachable via value OR type imports. Used to decide whether a + * type reference can leak a forbidden runtime through the public API surface. + * `value` is always a subset of `all`. + */ +interface Reach { + value: ReadonlySet; + all: ReadonlySet; +} + +const EMPTY_REACH: Reach = { value: new Set(), all: new Set() }; + +/** + * Per-Program cache: source file path → reach sets transitively computed from that file. + * Both `value` and `all` sets are filled in a single DFS pass to share resolution work. + * + * Keyed by `ts.Program` identity so the cache is invalidated whenever + * typescript-eslint rebuilds the Program. + */ +const programCache = new WeakMap>(); + export const rule = ESLintUtils.RuleCreator(() => __filename)({ name: RULE_NAME, meta: { @@ -345,6 +402,29 @@ export const rule = ESLintUtils.RuleCreator(() => __filename), + inProgress: Set, +): Reach { + const cached = cache.get(sourceFile.fileName); + if (cached) { + return cached; + } + if (inProgress.has(sourceFile.fileName)) { + // Cycle: return empty sets without caching so the eventual full result is committed by the originator. + return EMPTY_REACH; + } + inProgress.add(sourceFile.fileName); + + const value = new Set(); + const all = new Set(); + for (const imp of collectImports(sourceFile)) { + if (isBareSpecifier(imp.specifier)) { + const pkg = packageNameOf(imp.specifier); + all.add(pkg); + if (!imp.typeOnly) { + value.add(pkg); + } + } + const resolved = resolveModule(program, sourceFile, imp.specifier, imp.literal); + if (!resolved) { + continue; + } + const childSourceFile = program.getSourceFile(resolved); + if (!childSourceFile) { + continue; + } + const childReach = computeReach(program, childSourceFile, cache, inProgress); + for (const pkg of childReach.all) { + all.add(pkg); + } + if (!imp.typeOnly) { + // A type-only edge does not propagate runtime reach: it can only widen the `all` set. + for (const pkg of childReach.value) { + value.add(pkg); + } + } + } + + inProgress.delete(sourceFile.fileName); + const result: Reach = { value, all }; + cache.set(sourceFile.fileName, result); + return result; +} + +interface ImportEdge { + specifier: string; + literal: ts.StringLiteralLike; + /** `true` when this edge only carries type information (no runtime side-effect). */ + typeOnly: boolean; +} + +/** + * Enumerates every module specifier in `sourceFile`, tagging each edge as value (`typeOnly: false`) + * or type-only (`typeOnly: true`). `import type` / `export type`, fully type-only named import or + * export clauses, and `import type =` are emitted with `typeOnly: true`. Side-effect imports + * (no clause) are emitted as value edges. + */ +function collectImports(sourceFile: ts.SourceFile): ImportEdge[] { + const result: ImportEdge[] = []; + for (const stmt of sourceFile.statements) { + if (ts.isImportDeclaration(stmt)) { + let typeOnly = false; + if (stmt.importClause?.isTypeOnly) { + typeOnly = true; + } else if ( + stmt.importClause && + stmt.importClause.namedBindings && + ts.isNamedImports(stmt.importClause.namedBindings) + ) { + const named = stmt.importClause.namedBindings; + const hasValue = !!stmt.importClause.name || named.elements.some(element => !element.isTypeOnly); + typeOnly = !hasValue; + } + if (ts.isStringLiteralLike(stmt.moduleSpecifier)) { + result.push({ specifier: stmt.moduleSpecifier.text, literal: stmt.moduleSpecifier, typeOnly }); + } + continue; + } + if (ts.isExportDeclaration(stmt) && stmt.moduleSpecifier && ts.isStringLiteralLike(stmt.moduleSpecifier)) { + let typeOnly = stmt.isTypeOnly; + if (!typeOnly && stmt.exportClause && ts.isNamedExports(stmt.exportClause)) { + typeOnly = stmt.exportClause.elements.every(element => element.isTypeOnly); + } + result.push({ specifier: stmt.moduleSpecifier.text, literal: stmt.moduleSpecifier, typeOnly }); + continue; + } + if ( + ts.isImportEqualsDeclaration(stmt) && + ts.isExternalModuleReference(stmt.moduleReference) && + ts.isStringLiteralLike(stmt.moduleReference.expression) + ) { + result.push({ + specifier: stmt.moduleReference.expression.text, + literal: stmt.moduleReference.expression, + typeOnly: stmt.isTypeOnly, + }); + } + } + return result; +} + +// --------------------------------------------------------------------------- +// Module resolution helpers +// --------------------------------------------------------------------------- + +/** + * Resolves `specifier` (as used in `sourceFile`) to an absolute file path using the same algorithm + * the host TS Program uses. Prefers the (faster) `program.getResolvedModule` API exposed in TS ≥ 5.3 + * and falls back to `ts.resolveModuleName` for older toolchains. Returns `undefined` if the module + * cannot be resolved (e.g. ambient declarations, broken paths). + */ +function resolveModule( + program: ts.Program, + sourceFile: ts.SourceFile, + specifier: string, + literal: ts.StringLiteralLike, +): string | undefined { + // TS ≥ 5.3 exposes program.getResolvedModule + const getResolvedModule = ( + program as unknown as { + getResolvedModule?: ( + file: ts.SourceFile, + moduleName: string, + mode?: ts.ResolutionMode, + ) => { resolvedModule?: ts.ResolvedModuleFull } | undefined; + } + ).getResolvedModule; + const mode = ( + ts as unknown as { + getModeForUsageLocation?: (file: ts.SourceFile, usage: ts.StringLiteralLike) => ts.ResolutionMode; + } + ).getModeForUsageLocation?.(sourceFile, literal); + + if (typeof getResolvedModule === 'function') { + const resolutionResult = getResolvedModule.call(program, sourceFile, specifier, mode); + if (resolutionResult?.resolvedModule) { + return resolutionResult.resolvedModule.resolvedFileName; + } + } + + // Fallback for older TS: use ts.resolveModuleName against the compiler host. + const compilerOptions = program.getCompilerOptions(); + const host = + (program as unknown as { getCompilerHost?: () => ts.ModuleResolutionHost }).getCompilerHost?.() ?? ts.sys; + const result = ts.resolveModuleName( + specifier, + sourceFile.fileName, + compilerOptions, + host as ts.ModuleResolutionHost, + undefined, + undefined, + mode, + ); + return result.resolvedModule?.resolvedFileName; +} + +/** + * `true` when the import specifier refers to a package (e.g. `react`, `@scope/pkg`, `pkg/sub`) + * rather than a relative or absolute path. + */ +function isBareSpecifier(specifier: string): boolean { + return !specifier.startsWith('.') && !specifier.startsWith('/'); +} + +/** + * Extracts the npm package name from a bare specifier. Handles both unscoped (`pkg/sub` → `pkg`) + * and scoped (`@scope/pkg/sub` → `@scope/pkg`) forms. + */ +function packageNameOf(specifier: string): string { + if (specifier.startsWith('@')) { + const [scope, name] = specifier.split('/', 2); + return name ? `${scope}/${name}` : scope; + } + const slash = specifier.indexOf('/'); + return slash === -1 ? specifier : specifier.slice(0, slash); +} + +/** + * Shortens an absolute file path for display in diagnostics: returns the part after the last + * `node_modules/` segment when present (so users see e.g. `tabster/dist/index.js`), or makes the + * path workspace-relative when inside the current working directory. + */ +function shortenPath(absolute: string): string { + const marker = '/node_modules/'; + const idx = absolute.lastIndexOf(marker); + if (idx !== -1) { + return absolute.slice(idx + marker.length); + } + const cwd = process.cwd(); + if (absolute.startsWith(cwd)) { + return absolute.slice(cwd.length + 1); + } + return absolute; +} diff --git a/tools/eslint-rules/utils/base-hook-detector.ts b/tools/eslint-rules/utils/base-hook-detector.ts index 21d37b2dd13cfc..3d301678a58067 100644 --- a/tools/eslint-rules/utils/base-hook-detector.ts +++ b/tools/eslint-rules/utils/base-hook-detector.ts @@ -19,8 +19,8 @@ export const BASE_HOOK_NAME_PATTERN = /^use[A-Z]\w*Base_unstable$/; */ export const STATE_HOOK_NAME_PATTERN = /^use[A-Z]\w*_unstable$/; -export const BASE_SUFFIX = 'Base_unstable'; -export const UNSTABLE_SUFFIX = '_unstable'; +const BASE_SUFFIX = 'Base_unstable'; +const UNSTABLE_SUFFIX = '_unstable'; const SIBLING_EXTENSIONS: ReadonlyArray = ['.ts', '.tsx']; /** diff --git a/tools/eslint-rules/utils/module-resolver.ts b/tools/eslint-rules/utils/module-resolver.ts deleted file mode 100644 index 3a1fa156c6c001..00000000000000 --- a/tools/eslint-rules/utils/module-resolver.ts +++ /dev/null @@ -1,91 +0,0 @@ -import * as ts from 'typescript'; - -/** - * Resolves `specifier` (as used in `sourceFile`) to an absolute file path using the same algorithm - * the host TS Program uses. Prefers the (faster) `program.getResolvedModule` API exposed in TS ≥ 5.3 - * and falls back to `ts.resolveModuleName` for older toolchains. Returns `undefined` if the module - * cannot be resolved (e.g. ambient declarations, broken paths). - */ -export function resolveModule( - program: ts.Program, - sourceFile: ts.SourceFile, - specifier: string, - literal: ts.StringLiteralLike, -): string | undefined { - // TS ≥ 5.3 exposes program.getResolvedModule - const getResolvedModule = ( - program as unknown as { - getResolvedModule?: ( - file: ts.SourceFile, - moduleName: string, - mode?: ts.ResolutionMode, - ) => { resolvedModule?: ts.ResolvedModuleFull } | undefined; - } - ).getResolvedModule; - const mode = ( - ts as unknown as { - getModeForUsageLocation?: (file: ts.SourceFile, usage: ts.StringLiteralLike) => ts.ResolutionMode; - } - ).getModeForUsageLocation?.(sourceFile, literal); - - if (typeof getResolvedModule === 'function') { - const resolutionResult = getResolvedModule.call(program, sourceFile, specifier, mode); - if (resolutionResult?.resolvedModule) { - return resolutionResult.resolvedModule.resolvedFileName; - } - } - - // Fallback for older TS: use ts.resolveModuleName against the compiler host. - const compilerOptions = program.getCompilerOptions(); - const host = - (program as unknown as { getCompilerHost?: () => ts.ModuleResolutionHost }).getCompilerHost?.() ?? ts.sys; - const result = ts.resolveModuleName( - specifier, - sourceFile.fileName, - compilerOptions, - host as ts.ModuleResolutionHost, - undefined, - undefined, - mode, - ); - return result.resolvedModule?.resolvedFileName; -} - -/** - * `true` when the import specifier refers to a package (e.g. `react`, `@scope/pkg`, `pkg/sub`) - * rather than a relative or absolute path. - */ -export function isBareSpecifier(specifier: string): boolean { - return !specifier.startsWith('.') && !specifier.startsWith('/'); -} - -/** - * Extracts the npm package name from a bare specifier. Handles both unscoped (`pkg/sub` → `pkg`) - * and scoped (`@scope/pkg/sub` → `@scope/pkg`) forms. - */ -export function packageNameOf(specifier: string): string { - if (specifier.startsWith('@')) { - const [scope, name] = specifier.split('/', 2); - return name ? `${scope}/${name}` : scope; - } - const slash = specifier.indexOf('/'); - return slash === -1 ? specifier : specifier.slice(0, slash); -} - -/** - * Shortens an absolute file path for display in diagnostics: returns the part after the last - * `node_modules/` segment when present (so users see e.g. `tabster/dist/index.js`), or makes the - * path workspace-relative when inside the current working directory. - */ -export function shortenPath(absolute: string): string { - const marker = '/node_modules/'; - const idx = absolute.lastIndexOf(marker); - if (idx !== -1) { - return absolute.slice(idx + marker.length); - } - const cwd = process.cwd(); - if (absolute.startsWith(cwd)) { - return absolute.slice(cwd.length + 1); - } - return absolute; -} diff --git a/tools/eslint-rules/utils/tracked-imports.ts b/tools/eslint-rules/utils/tracked-imports.ts deleted file mode 100644 index f65fa6e1bd75e9..00000000000000 --- a/tools/eslint-rules/utils/tracked-imports.ts +++ /dev/null @@ -1,53 +0,0 @@ -import type { TSESTree } from '@typescript-eslint/utils'; -import { AST_NODE_TYPES } from '@typescript-eslint/utils'; - -/** - * The original (imported) name of an import specifier, used for diagnostics and for matching - * against a forbidden/watched package's exports. - * - * - named import (`import { Foo }`) → `'Foo'` - * - aliased named import (`import { Foo as Bar }`) → `'Foo'` (the original, not the alias) - * - default import (`import X from 'pkg'`) → `'default'` - * - namespace import (`import * as X`) → `'*'` - */ -export type ImportSpecifierNode = - | TSESTree.ImportSpecifier - | TSESTree.ImportDefaultSpecifier - | TSESTree.ImportNamespaceSpecifier; - -export function getImportedName(specifier: ImportSpecifierNode): string | undefined { - switch (specifier.type) { - case AST_NODE_TYPES.ImportSpecifier: - return specifier.imported.type === AST_NODE_TYPES.Identifier - ? specifier.imported.name - : String(specifier.imported.value); - case AST_NODE_TYPES.ImportDefaultSpecifier: - return 'default'; - case AST_NODE_TYPES.ImportNamespaceSpecifier: - return '*'; - default: - return undefined; - } -} - -/** - * A locally-declared binding originating from a tracked import (a watched or forbidden-runtime - * package). Built by the runtime rule when walking `ImportDeclaration` nodes so body references - * can be matched in O(1) via a `Map`. - */ -export interface TrackedImport { - /** The package the binding came from (a watched OR forbidden-runtime package). */ - package: string; - /** Original imported name (not the local alias). `default` or `*` for default / namespace. */ - importedName: string; - /** Kind of package — controls how the reference is checked. */ - kind: 'watched' | 'forbidden'; - /** - * `true` when the binding is type-only (either the declaration is `import type ...` - * or the specifier is `import { type Foo }`). Used to gate whether direct usage in a - * value position is even possible (type-only bindings only surface in type positions). - */ - isTypeOnly: boolean; - /** The specifier node (used for symbol lookup via ParserServices). */ - specifier: ImportSpecifierNode; -} diff --git a/tools/eslint-rules/utils/transitive-reach.ts b/tools/eslint-rules/utils/transitive-reach.ts deleted file mode 100644 index 13d16c0e4bfe7b..00000000000000 --- a/tools/eslint-rules/utils/transitive-reach.ts +++ /dev/null @@ -1,160 +0,0 @@ -import * as ts from 'typescript'; -import { isBareSpecifier, packageNameOf, resolveModule } from './module-resolver'; - -/** - * Result of a single transitive-reach DFS over a source file's import graph. - * - `value` — packages reachable via value (non type-only) imports only. Used to decide - * whether a runtime reference can pull a forbidden runtime at execution time. - * - `all` — packages reachable via value OR type imports. Used to decide whether a - * type reference can leak a forbidden runtime through the public API surface. - * `value` is always a subset of `all`. - */ -export interface Reach { - value: ReadonlySet; - all: ReadonlySet; -} - -export const EMPTY_REACH: Reach = { value: new Set(), all: new Set() }; - -/** - * Per-Program cache: source file path → reach sets transitively computed from that file. - * Both `value` and `all` sets are filled in a single DFS pass to share resolution work. - * - * Keyed by `ts.Program` identity so the cache is invalidated whenever - * typescript-eslint rebuilds the Program. - */ -const programCache = new WeakMap>(); - -/** - * Returns the bare package specifiers transitively reachable from `sourceFile`, computed in two - * granularities in a single DFS pass: - * - `value` — only value (non type-only) imports are followed. Used to decide whether a runtime - * reference can pull a forbidden runtime at execution time. - * - `all` — both value and type imports are followed. Used to decide whether a type reference - * can leak a forbidden runtime through the public API surface of a base hook. - * - * Memoized per Program × file. Cycle-safe. - */ -export function transitiveReach(program: ts.Program, sourceFile: ts.SourceFile): Reach { - let cache = programCache.get(program); - if (!cache) { - cache = new Map(); - programCache.set(program, cache); - } - return computeReach(program, sourceFile, cache, new Set()); -} - -/** - * Recursive worker for `transitiveReach`. Walks the import graph DFS, recording every bare - * specifier encountered (separately for value-only vs value+type follow modes) and recursing into - * each resolved source file. Uses `inProgress` to break cycles (cycle hits return empty sets - * without caching, so the originating call still commits the complete result). - */ -export function computeReach( - program: ts.Program, - sourceFile: ts.SourceFile, - cache: Map, - inProgress: Set, -): Reach { - const cached = cache.get(sourceFile.fileName); - if (cached) { - return cached; - } - if (inProgress.has(sourceFile.fileName)) { - // Cycle: return empty sets without caching so the eventual full result is committed by the originator. - return EMPTY_REACH; - } - inProgress.add(sourceFile.fileName); - - const value = new Set(); - const all = new Set(); - for (const imp of collectImports(sourceFile)) { - if (isBareSpecifier(imp.specifier)) { - const pkg = packageNameOf(imp.specifier); - all.add(pkg); - if (!imp.typeOnly) { - value.add(pkg); - } - } - const resolved = resolveModule(program, sourceFile, imp.specifier, imp.literal); - if (!resolved) { - continue; - } - const childSourceFile = program.getSourceFile(resolved); - if (!childSourceFile) { - continue; - } - const childReach = computeReach(program, childSourceFile, cache, inProgress); - for (const pkg of childReach.all) { - all.add(pkg); - } - if (!imp.typeOnly) { - // A type-only edge does not propagate runtime reach: it can only widen the `all` set. - for (const pkg of childReach.value) { - value.add(pkg); - } - } - } - - inProgress.delete(sourceFile.fileName); - const result: Reach = { value, all }; - cache.set(sourceFile.fileName, result); - return result; -} - -export interface ImportEdge { - specifier: string; - literal: ts.StringLiteralLike; - /** `true` when this edge only carries type information (no runtime side-effect). */ - typeOnly: boolean; -} - -/** - * Enumerates every module specifier in `sourceFile`, tagging each edge as value (`typeOnly: false`) - * or type-only (`typeOnly: true`). `import type` / `export type`, fully type-only named import or - * export clauses, and `import type =` are emitted with `typeOnly: true`. Side-effect imports - * (no clause) are emitted as value edges. - */ -export function collectImports(sourceFile: ts.SourceFile): ImportEdge[] { - const result: ImportEdge[] = []; - for (const stmt of sourceFile.statements) { - if (ts.isImportDeclaration(stmt)) { - let typeOnly = false; - if (stmt.importClause?.isTypeOnly) { - typeOnly = true; - } else if ( - stmt.importClause && - stmt.importClause.namedBindings && - ts.isNamedImports(stmt.importClause.namedBindings) - ) { - const named = stmt.importClause.namedBindings; - const hasValue = !!stmt.importClause.name || named.elements.some(element => !element.isTypeOnly); - typeOnly = !hasValue; - } - if (ts.isStringLiteralLike(stmt.moduleSpecifier)) { - result.push({ specifier: stmt.moduleSpecifier.text, literal: stmt.moduleSpecifier, typeOnly }); - } - continue; - } - if (ts.isExportDeclaration(stmt) && stmt.moduleSpecifier && ts.isStringLiteralLike(stmt.moduleSpecifier)) { - let typeOnly = stmt.isTypeOnly; - if (!typeOnly && stmt.exportClause && ts.isNamedExports(stmt.exportClause)) { - typeOnly = stmt.exportClause.elements.every(element => element.isTypeOnly); - } - result.push({ specifier: stmt.moduleSpecifier.text, literal: stmt.moduleSpecifier, typeOnly }); - continue; - } - if ( - ts.isImportEqualsDeclaration(stmt) && - ts.isExternalModuleReference(stmt.moduleReference) && - ts.isStringLiteralLike(stmt.moduleReference.expression) - ) { - result.push({ - specifier: stmt.moduleReference.expression.text, - literal: stmt.moduleReference.expression, - typeOnly: stmt.isTypeOnly, - }); - } - } - return result; -} From c7614a7a75d13b80f70c5e4b4a7d2712c3f2d892 Mon Sep 17 00:00:00 2001 From: Martin Hochel Date: Tue, 26 May 2026 15:43:32 +0200 Subject: [PATCH 18/18] refactor(eslint-rules): inline base-hook-detector into each rule, drop utils/ Premature shared-utils abstraction: only two rules existed and the duplicated surface area is ~18 lines (BASE_HOOK_NAME_PATTERN, the BaseHookFunction type, getFunctionInit). Inlining makes each rule self-contained at the cost of trivial duplication. - Move all base-hook-detector exports into base-hook-signature.ts (signature rule owns the full pair-detection mechanism). - Duplicate BASE_HOOK_NAME_PATTERN / BaseHookFunction / getFunctionInit into base-hook-no-forbidden-runtime.ts. - Delete tools/eslint-rules/utils/ entirely. --- .../rules/base-hook-no-forbidden-runtime.ts | 35 ++++- .../eslint-rules/rules/base-hook-signature.ts | 143 +++++++++++++++++- .../eslint-rules/utils/base-hook-detector.ts | 140 ----------------- 3 files changed, 169 insertions(+), 149 deletions(-) delete mode 100644 tools/eslint-rules/utils/base-hook-detector.ts diff --git a/tools/eslint-rules/rules/base-hook-no-forbidden-runtime.ts b/tools/eslint-rules/rules/base-hook-no-forbidden-runtime.ts index 2d4a7a6b39c5da..75284f067f667c 100644 --- a/tools/eslint-rules/rules/base-hook-no-forbidden-runtime.ts +++ b/tools/eslint-rules/rules/base-hook-no-forbidden-runtime.ts @@ -1,11 +1,26 @@ import type { TSESTree, TSESLint, ParserServicesWithTypeInformation } from '@typescript-eslint/utils'; import { ESLintUtils, AST_NODE_TYPES } from '@typescript-eslint/utils'; import * as ts from 'typescript'; -import { BASE_HOOK_NAME_PATTERN, type BaseHookFunction, getFunctionInit } from '../utils/base-hook-detector'; // NOTE: The rule will be available in ESLint configs as "@nx/workspace-base-hook-no-forbidden-runtime" export const RULE_NAME = 'base-hook-no-forbidden-runtime'; +/** + * Names of v9 "base hooks": the implementation-only half of a `useFoo` / `useFooBase_unstable` + * pair, kept free of focus/keyboard runtime so it can be composed by callers that may opt out + * of those concerns. + */ +const BASE_HOOK_NAME_PATTERN = /^use[A-Z]\w*Base_unstable$/; + +/** + * Any function-literal form a base hook can take: top-level function declaration, inline arrow + * function, or function expression bound to a variable / export. + */ +type BaseHookFunction = + | TSESTree.FunctionDeclaration + | TSESTree.FunctionExpression + | TSESTree.ArrowFunctionExpression; + const DEFAULT_WATCHED_PACKAGES: ReadonlyArray = ['@fluentui/react-tabster']; const DEFAULT_FORBIDDEN_RUNTIMES: ReadonlyArray = ['tabster']; @@ -425,6 +440,24 @@ function getImportedName(specifier: ImportSpecifierNode): string | undefined { // Scope helpers // --------------------------------------------------------------------------- +/** + * Returns the function literal initializer of a `VariableDeclarator` when the declarator is a + * plain Identifier bound to an inline arrow/function expression; otherwise `undefined`. + */ +function getFunctionInit(node: TSESTree.VariableDeclarator): BaseHookFunction | undefined { + if (node.id.type !== AST_NODE_TYPES.Identifier) { + return undefined; + } + const init = node.init; + if ( + !init || + (init.type !== AST_NODE_TYPES.ArrowFunctionExpression && init.type !== AST_NODE_TYPES.FunctionExpression) + ) { + return undefined; + } + return init; +} + /** * `true` when `scope` (or any of its ancestor scopes) is the function scope of `hookFn`. Used to * confine the body-reference walk to the base hook itself — references in sibling functions are diff --git a/tools/eslint-rules/rules/base-hook-signature.ts b/tools/eslint-rules/rules/base-hook-signature.ts index ea322cd0cfa9b6..4b178e4fc380de 100644 --- a/tools/eslint-rules/rules/base-hook-signature.ts +++ b/tools/eslint-rules/rules/base-hook-signature.ts @@ -1,21 +1,43 @@ import type { TSESTree, TSESLint } from '@typescript-eslint/utils'; import { ESLintUtils, AST_NODE_TYPES } from '@typescript-eslint/utils'; -import { - BASE_HOOK_NAME_PATTERN, - STATE_HOOK_NAME_PATTERN, - type BaseHookFunction, - collectBaseHookNames, - createPairDetector, - getFunctionInit, -} from '../utils/base-hook-detector'; +import * as fs from 'node:fs'; +import * as path from 'node:path'; // NOTE: The rule will be available in ESLint configs as "@nx/workspace-base-hook-signature" export const RULE_NAME = 'base-hook-signature'; +/** + * Names of v9 "base hooks": the implementation-only half of a `useFoo` / `useFooBase_unstable` + * pair, kept free of focus/keyboard runtime so it can be composed by callers that may opt out + * of those concerns. + */ +const BASE_HOOK_NAME_PATTERN = /^use[A-Z]\w*Base_unstable$/; + +/** + * Names of any `_unstable` hook, including base hooks themselves. Used in the rule's selector + * which then dispatches by whether the name matches `BASE_HOOK_NAME_PATTERN` (always checked) + * or is a wrapping state hook paired with a base hook (only checked when a pair exists). + */ +const STATE_HOOK_NAME_PATTERN = /^use[A-Z]\w*_unstable$/; + +const BASE_SUFFIX = 'Base_unstable'; +const UNSTABLE_SUFFIX = '_unstable'; +const SIBLING_EXTENSIONS: ReadonlyArray = ['.ts', '.tsx']; + const EXPECTED_PARAM_NAMES = ['props', 'ref'] as const; const MIN_PARAM_COUNT = 1; const MAX_PARAM_COUNT = 2; +/** + * Any function-literal form a base or paired state hook can take: top-level function declaration, + * inline arrow function, or function expression bound to a variable / export. The signature rule + * runs the same parameter validation against all three. + */ +type BaseHookFunction = + | TSESTree.FunctionDeclaration + | TSESTree.FunctionExpression + | TSESTree.ArrowFunctionExpression; + type Options = []; type MessageIds = 'invalidParamCount' | 'invalidParamName' | 'invalidRefType'; @@ -142,6 +164,111 @@ function describeParam(param: TSESTree.Parameter): string { } } +/** + * Collects names of top-level declarations matching `BASE_HOOK_NAME_PATTERN` into `out`. + * Handles both `export const useFooBase_unstable = ...` (incl. `export const` chains) and + * `export function useFooBase_unstable() {}`, plus the unexported / `export { ... }` forms. + */ +function collectBaseHookNames(stmt: TSESTree.Node, out: Set): void { + // `export const useFooBase_unstable = ...` / `export function useFooBase_unstable() {}` + if (stmt.type === AST_NODE_TYPES.ExportNamedDeclaration && stmt.declaration) { + collectBaseHookNames(stmt.declaration, out); + return; + } + // `function useFooBase_unstable() {}` + if (stmt.type === AST_NODE_TYPES.FunctionDeclaration) { + if (stmt.id && BASE_HOOK_NAME_PATTERN.test(stmt.id.name)) { + out.add(stmt.id.name); + } + return; + } + // `const useFooBase_unstable = ...` (incl. multi-declarator forms) + if (stmt.type === AST_NODE_TYPES.VariableDeclaration) { + for (const decl of stmt.declarations) { + if (decl.id.type === AST_NODE_TYPES.Identifier && BASE_HOOK_NAME_PATTERN.test(decl.id.name)) { + out.add(decl.id.name); + } + } + } +} + +/** + * Returns the function literal initializer of a `VariableDeclarator` when the declarator is a + * plain Identifier bound to an inline arrow/function expression; otherwise `undefined`. Skips + * destructuring patterns (no inspectable function literal) and non-function initializers. + */ +function getFunctionInit(node: TSESTree.VariableDeclarator): BaseHookFunction | undefined { + if (node.id.type !== AST_NODE_TYPES.Identifier) { + return undefined; + } + const init = node.init; + if ( + !init || + (init.type !== AST_NODE_TYPES.ArrowFunctionExpression && init.type !== AST_NODE_TYPES.FunctionExpression) + ) { + return undefined; + } + return init; +} + +/** + * Stateful helper that decides whether a wrapping state hook (`useFoo_unstable`) is paired with + * a sibling base hook (`useFooBase_unstable`) — either declared in the same file or as a sibling + * `.ts` / `.tsx` file in the same directory. The base hook is the structural marker; when found, + * the wrapping hook is required to honor the `(props, ref)` signature contract. + * + * Per-instance state: + * - `baseHooksInCurrentFile` is populated by the rule's `Program` visitor. + * - `siblingFileExistsCache` memoizes `fs.statSync` results so each component directory pays at + * most one syscall per linted run. + * + * Anchoring detection on the base hook eliminates false positives on other `_unstable` hooks + * (e.g. `useFooContextValues_unstable`, `useFooStyles_unstable`) that intentionally take + * different signatures. + */ +function createPairDetector(filename: string | undefined) { + const baseHooksInCurrentFile = new Set(); + const siblingFileExistsCache = new Map(); + + function hasPairedBaseHook(stateHookName: string): boolean { + const baseHookName = stateHookName.slice(0, -UNSTABLE_SUFFIX.length) + BASE_SUFFIX; + if (baseHooksInCurrentFile.has(baseHookName)) { + return true; + } + // ESLint passes synthetic filenames like `` for inline code; nothing to check. + if (!filename || !path.isAbsolute(filename)) { + return false; + } + const dir = path.dirname(filename); + // Sibling base-hook file (the wrapping hook lives in `useFoo.tsx`, the base in `useFooBase.tsx`). + const siblingBasename = baseHookName.slice(0, -UNSTABLE_SUFFIX.length); // e.g. `useFooBase` + for (const ext of SIBLING_EXTENSIONS) { + const candidate = path.join(dir, siblingBasename + ext); + if (candidate === filename) { + continue; + } + let exists = siblingFileExistsCache.get(candidate); + if (exists === undefined) { + try { + exists = fs.statSync(candidate).isFile(); + } catch { + exists = false; + } + siblingFileExistsCache.set(candidate, exists); + } + if (exists) { + return true; + } + } + return false; + } + + return { + baseHooksInCurrentFile, + hasPairedBaseHook, + }; +} + /** * Returns `true` when `annotation` is `React.Ref<...>` (qualified) or `Ref<...>` (named) AND the * referenced identifier was imported from the `react` package in the surrounding scope. The scope diff --git a/tools/eslint-rules/utils/base-hook-detector.ts b/tools/eslint-rules/utils/base-hook-detector.ts deleted file mode 100644 index 3d301678a58067..00000000000000 --- a/tools/eslint-rules/utils/base-hook-detector.ts +++ /dev/null @@ -1,140 +0,0 @@ -import type { TSESTree } from '@typescript-eslint/utils'; -import { AST_NODE_TYPES } from '@typescript-eslint/utils'; -import * as fs from 'node:fs'; -import * as path from 'node:path'; - -/** - * Names of v9 "base hooks": the implementation-only half of a `useFoo` / `useFooBase_unstable` - * pair, kept free of focus/keyboard runtime so it can be composed by callers that may opt out - * of those concerns. This is the structural marker used by both the signature rule and the - * forbidden-runtime rule. - */ -export const BASE_HOOK_NAME_PATTERN = /^use[A-Z]\w*Base_unstable$/; - -/** - * Names of any `_unstable` hook, including base hooks themselves. The signature rule uses this - * broader pattern in its selector and then dispatches by whether the name matches - * `BASE_HOOK_NAME_PATTERN` (always checked) or is a wrapping state hook paired with a base hook - * via {@link hasPairedBaseHook} (only checked when a pair exists). - */ -export const STATE_HOOK_NAME_PATTERN = /^use[A-Z]\w*_unstable$/; - -const BASE_SUFFIX = 'Base_unstable'; -const UNSTABLE_SUFFIX = '_unstable'; -const SIBLING_EXTENSIONS: ReadonlyArray = ['.ts', '.tsx']; - -/** - * Any function-literal form a base or paired state hook can take: top-level function declaration, - * inline arrow function, or function expression bound to a variable / export. The signature rule - * runs the same parameter validation against all three. - */ -export type BaseHookFunction = - | TSESTree.FunctionDeclaration - | TSESTree.FunctionExpression - | TSESTree.ArrowFunctionExpression; - -/** - * Collects names of top-level declarations matching `BASE_HOOK_NAME_PATTERN` into `out`. - * Handles both `export const useFooBase_unstable = ...` (incl. `export const` chains) and - * `export function useFooBase_unstable() {}`, plus the unexported / `export { ... }` forms. - */ -export function collectBaseHookNames(stmt: TSESTree.Node, out: Set): void { - // `export const useFooBase_unstable = ...` / `export function useFooBase_unstable() {}` - if (stmt.type === AST_NODE_TYPES.ExportNamedDeclaration && stmt.declaration) { - collectBaseHookNames(stmt.declaration, out); - return; - } - // `function useFooBase_unstable() {}` - if (stmt.type === AST_NODE_TYPES.FunctionDeclaration) { - if (stmt.id && BASE_HOOK_NAME_PATTERN.test(stmt.id.name)) { - out.add(stmt.id.name); - } - return; - } - // `const useFooBase_unstable = ...` (incl. multi-declarator forms) - if (stmt.type === AST_NODE_TYPES.VariableDeclaration) { - for (const decl of stmt.declarations) { - if (decl.id.type === AST_NODE_TYPES.Identifier && BASE_HOOK_NAME_PATTERN.test(decl.id.name)) { - out.add(decl.id.name); - } - } - } -} - -/** - * Returns the function literal initializer of a `VariableDeclarator` when the declarator is a - * plain Identifier bound to an inline arrow/function expression; otherwise `undefined`. Skips - * destructuring patterns (no inspectable function literal) and non-function initializers - * (call expressions, identifier aliases, missing initializer for ambients). - */ -export function getFunctionInit(node: TSESTree.VariableDeclarator): BaseHookFunction | undefined { - if (node.id.type !== AST_NODE_TYPES.Identifier) { - return undefined; - } - const init = node.init; - if ( - !init || - (init.type !== AST_NODE_TYPES.ArrowFunctionExpression && init.type !== AST_NODE_TYPES.FunctionExpression) - ) { - return undefined; - } - return init; -} - -/** - * Stateful helper that decides whether a wrapping state hook (`useFoo_unstable`) is paired with - * a sibling base hook (`useFooBase_unstable`) — either declared in the same file or as a sibling - * `.ts` / `.tsx` file in the same directory. The base hook is the structural marker; when found, - * the wrapping hook is required to honor the `(props, ref)` signature contract. - * - * Per-instance state: - * - `baseHooksInCurrentFile` is populated by the rule's `Program` visitor. - * - `siblingFileExistsCache` memoizes `fs.statSync` results so each component directory pays at - * most one syscall per linted run. - * - * Anchoring detection on the base hook eliminates false positives on other `_unstable` hooks - * (e.g. `useFooContextValues_unstable`, `useFooStyles_unstable`) that intentionally take - * different signatures. - */ -export function createPairDetector(filename: string | undefined) { - const baseHooksInCurrentFile = new Set(); - const siblingFileExistsCache = new Map(); - - function hasPairedBaseHook(stateHookName: string): boolean { - const baseHookName = stateHookName.slice(0, -UNSTABLE_SUFFIX.length) + BASE_SUFFIX; - if (baseHooksInCurrentFile.has(baseHookName)) { - return true; - } - // ESLint passes synthetic filenames like `` for inline code; nothing to check. - if (!filename || !path.isAbsolute(filename)) { - return false; - } - const dir = path.dirname(filename); - // Sibling base-hook file (the wrapping hook lives in `useFoo.tsx`, the base in `useFooBase.tsx`). - const siblingBasename = baseHookName.slice(0, -UNSTABLE_SUFFIX.length); // e.g. `useFooBase` - for (const ext of SIBLING_EXTENSIONS) { - const candidate = path.join(dir, siblingBasename + ext); - if (candidate === filename) { - continue; - } - let exists = siblingFileExistsCache.get(candidate); - if (exists === undefined) { - try { - exists = fs.statSync(candidate).isFile(); - } catch { - exists = false; - } - siblingFileExistsCache.set(candidate, exists); - } - if (exists) { - return true; - } - } - return false; - } - - return { - baseHooksInCurrentFile, - hasPairedBaseHook, - }; -}