From 4c48e7809f7a7af1e37e1b5cf1407ab884b4114d Mon Sep 17 00:00:00 2001 From: Lene Gadewoll Date: Tue, 28 Jul 2026 11:33:31 +0200 Subject: [PATCH 01/10] feat: add button-group-no-invalid-children eslint rule --- packages/eslint-plugin/README.md | 80 ++ packages/eslint-plugin/src/index.ts | 3 + .../button_group_no_invalid_children.test.ts | 1230 +++++++++++++++++ .../rules/button_group_no_invalid_children.ts | 216 +++ .../src/utils/collect_jsx_children.ts | 198 +++ packages/eslint-plugin/src/utils/flat_map.ts | 13 + .../eslint-plugin/src/utils/is_fragment.ts | 25 + 7 files changed, 1765 insertions(+) create mode 100644 packages/eslint-plugin/src/rules/button_group_no_invalid_children.test.ts create mode 100644 packages/eslint-plugin/src/rules/button_group_no_invalid_children.ts create mode 100644 packages/eslint-plugin/src/utils/collect_jsx_children.ts create mode 100644 packages/eslint-plugin/src/utils/flat_map.ts create mode 100644 packages/eslint-plugin/src/utils/is_fragment.ts diff --git a/packages/eslint-plugin/README.md b/packages/eslint-plugin/README.md index 23612721f11..2d3fb6a896d 100644 --- a/packages/eslint-plugin/README.md +++ b/packages/eslint-plugin/README.md @@ -328,6 +328,86 @@ it('shows tooltip on focus', async () => { ``` +### `@elastic/eui/button-group-no-invalid-children` + +Enforce that `EuiButtonGroup` children (when using the Children API) are valid button components. + +Valid direct children are: +- `variant="default"`: `EuiButton`, `EuiButtonEmpty`, and `EuiButtonIcon` + +Besides those button components, these three wrapper components are also allowed: `EuiPopover`, `EuiToolTip` and `EuiCopy`. + +#### Examples + +```tsx +// ✗ Bad - non-button elements + +
Not a button
+ ... + + +// ✓ Good - direct buttons + + Save + Cancel + + +// ✓ Good - icon button with tooltip + + Save + + + + + +// ✓ Good - EuiCopy with render prop (expression or block body) + + + {(copy) => Copy} + + + +// ✓ Good - .map() with expression or block body + + {buttons.map((b) => {b.label})} + + +// ✓ Good - EuiPopover with a button trigger + + More} + isOpen={isOpen} + closePopover={closePopover} + > + Panel content + + + +// ✓ Good - EuiPopover with an EuiToolTip-wrapped icon trigger + + + + + } + isOpen={isOpen} + closePopover={closePopover} + > + Panel content + + +``` + +#### Custom button wrapper components + +If a project-specific button component (e.g. ``) is used as a child and the rule cannot resolve it statically, it reports `invalidUnresolvableChild` which suggests suppressing the rule inline with a comment: + +```tsx +// eslint-disable-next-line @elastic/eui/button-group-no-invalid-children -- SaveButton returns EuiButton + +``` + ## Testing ### Running unit tests diff --git a/packages/eslint-plugin/src/index.ts b/packages/eslint-plugin/src/index.ts index 7d446d9b344..bc18365226c 100644 --- a/packages/eslint-plugin/src/index.ts +++ b/packages/eslint-plugin/src/index.ts @@ -27,6 +27,7 @@ import { EuiBadgeAccessibilityRules } from './rules/a11y/badge_accessibility_rul import { EuiIconAccessibilityRules } from './rules/a11y/icon_accessibility_rules'; import { TooltipNoInteractiveContent } from './rules/a11y/tooltip_no_interactive_content'; import { TooltipButtonIconWrap } from './rules/a11y/tooltip_button_icon_wrap'; +import { ButtonGroupNoInvalidChildren } from './rules/button_group_no_invalid_children'; const config = { rules: { @@ -52,6 +53,7 @@ const config = { 'require-href-for-link': RequireHrefForLink, 'tooltip-no-interactive-content': TooltipNoInteractiveContent, 'tooltip-button-icon-wrap': TooltipButtonIconWrap, + 'button-group-no-invalid-children': ButtonGroupNoInvalidChildren, }, configs: { recommended: { @@ -78,6 +80,7 @@ const config = { '@elastic/eui/require-href-for-link': 'warn', '@elastic/eui/tooltip-no-interactive-content': 'warn', '@elastic/eui/tooltip-button-icon-wrap': 'warn', + '@elastic/eui/button-group-no-invalid-children': 'warn', }, }, }, diff --git a/packages/eslint-plugin/src/rules/button_group_no_invalid_children.test.ts b/packages/eslint-plugin/src/rules/button_group_no_invalid_children.test.ts new file mode 100644 index 00000000000..3d3be39ba6f --- /dev/null +++ b/packages/eslint-plugin/src/rules/button_group_no_invalid_children.test.ts @@ -0,0 +1,1230 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the Elastic License + * 2.0 and the Server Side Public License, v 1; you may not use this file except + * in compliance with, at your election, the Elastic License 2.0 or the Server + * Side Public License, v 1. + */ + +import dedent from 'dedent'; +import { RuleTester } from '@typescript-eslint/rule-tester'; +import { ButtonGroupNoInvalidChildren } from './button_group_no_invalid_children'; + +const languageOptions = { + parserOptions: { + ecmaFeatures: { + jsx: true, + }, + }, +}; + +const ruleTester = new RuleTester(); + +ruleTester.run( + 'button-group-no-invalid-children', + ButtonGroupNoInvalidChildren, + { + valid: [ + // Direct children + { + name: 'EuiButton child', + code: dedent` + + Save + + `, + languageOptions, + }, + { + name: 'EuiButtonEmpty child', + code: dedent` + + Cancel + + `, + languageOptions, + }, + { + name: 'EuiButtonIcon child', + code: dedent` + + + + `, + languageOptions, + }, + { + name: 'mixed valid direct children', + code: dedent` + + Save + Cancel + + + `, + languageOptions, + }, + + // Wrappers + { + name: 'EuiButtonIcon wrapped in EuiToolTip', + code: dedent` + + Save + + + + + `, + languageOptions, + }, + { + name: 'EuiButton wrapped in EuiToolTip', + code: dedent` + + + Save + + + `, + languageOptions, + }, + { + name: 'EuiPopover as direct child — JSX children (panel content) are not validated', + code: dedent` + + Normal + Open} isOpen={false} closePopover={() => {}}> + Panel content with non-button elements + + + `, + languageOptions, + }, + { + name: 'EuiPopover with EuiToolTip-wrapped trigger in button prop is accepted', + code: dedent` + + } + isOpen={false} + closePopover={() => {}} + > + Panel content + + + `, + languageOptions, + }, + { + name: 'EuiPopover button prop as a resolved variable with valid button is accepted', + code: dedent` + const trigger = Open; + + {}}> + Panel content + + + `, + languageOptions, + }, + { + name: 'EuiPopover button prop as a conditional with valid buttons is accepted', + code: dedent` + + : More} + isOpen={false} + closePopover={() => {}} + > + Panel content + + + `, + languageOptions, + }, + { + name: 'EuiCopy with valid render-prop button is accepted', + code: dedent` + + Normal + + {(copy) => Copy} + + + `, + languageOptions, + }, + { + name: 'EuiCopy block-body render prop with valid button is accepted', + code: dedent` + + + {(copy) => { return Copy; }} + + + `, + languageOptions, + }, + + // Fragments + { + name: 'shorthand fragment wrapping valid buttons', + code: dedent` + + <> + Save + Cancel + + + `, + languageOptions, + }, + { + name: ' wrapping valid buttons', + code: dedent` + + + Save + + + `, + languageOptions, + }, + { + name: ' wrapping valid buttons', + code: dedent` + + + Save + + + + `, + languageOptions, + }, + { + name: 'nested fragments wrapping valid buttons', + code: dedent` + + <> + + Save + + + + `, + languageOptions, + }, + { + name: 'fragment wrapping valid button inside EuiToolTip', + code: dedent` + + + <> + + + + + `, + languageOptions, + }, + { + name: 'conditional inside EuiToolTip with valid branches', + code: dedent` + + + {isIcon + ? + : Delete} + + + `, + languageOptions, + }, + { + name: 'variable inside EuiToolTip resolves to a valid button', + code: dedent` + const icon = ; + + + {icon} + + + `, + languageOptions, + }, + + // Text nodes + { + name: 'whitespace text nodes between buttons are silently ignored', + code: dedent` + + {' '} + Save + {' '} + + `, + languageOptions, + }, + + // Conditional rendering + { + name: '{condition && } is valid', + code: dedent` + + Save + {canDelete && } + + `, + languageOptions, + }, + { + name: 'ternary with both valid branches', + code: dedent` + + {isAdmin ? Delete : Cancel} + + `, + languageOptions, + }, + { + name: 'ternary with null alternate branch', + code: dedent` + + Save + {canDelete ? : null} + + `, + languageOptions, + }, + { + name: '{unresolvable || } — unresolvable left side is skipped, valid right side passes', + code: dedent` + + {fallback || Save} + + `, + languageOptions, + }, + { + name: '{unresolvable ?? } — unresolvable left side is skipped, valid right side passes', + code: dedent` + + {fallback ?? Save} + + `, + languageOptions, + }, + { + name: '{resolvedButton || } — valid resolved left side passes', + code: dedent` + const resolvedButton = Cancel; + + {resolvedButton || Save} + + `, + languageOptions, + }, + + // Array notation + { + name: 'array of valid buttons', + code: dedent` + + {[ + Save, + Cancel, + ]} + + `, + languageOptions, + }, + { + name: 'array containing a valid wrapper (EuiToolTip)', + code: dedent` + + {[ + Save, + + + , + ]} + + `, + languageOptions, + }, + + // Variable resolution + { + name: 'const variable holding a valid button is resolved and accepted', + code: dedent` + const button = Save; + + {button} + + `, + languageOptions, + }, + { + name: 'const variable holding a valid button array is resolved and accepted', + code: dedent` + const buttons = [ + Save, + Cancel, + ]; + + {buttons} + + `, + languageOptions, + }, + { + name: 'reassigned variable is skipped — cannot be safely resolved', + code: dedent` + let button = Not a button; + button = Save; + + {button} + + `, + languageOptions, + }, + + // Local arrow-function + { + name: 'local arrow-fn component with valid return is resolved transparently', + code: dedent` + const ActionButtons = () => Save; + + + + `, + languageOptions, + }, + { + name: 'local arrow-fn component returning a fragment of valid buttons', + code: dedent` + const ActionButtons = () => ( + <> + Save + Cancel + + ); + + + + `, + languageOptions, + }, + { + name: 'local block-body arrow-fn component with valid return is resolved and accepted', + code: dedent` + const ActionButtons = () => { return Save; }; + + + + `, + languageOptions, + }, + { + name: 'local block-body arrow-fn component with early return is resolved and accepted', + code: dedent` + const ActionButtons = ({ isIcon }) => { + if (isIcon) return ; + return Add; + }; + + + + `, + languageOptions, + }, + + // Static bail-outs (not resolvable at analysis time) + { + name: 'function call child — cannot statically resolve', + code: dedent` + + {renderButtons()} + + `, + languageOptions, + }, + { + name: 'options API (self-closing) is ignored', + code: dedent` + {}} + /> + `, + languageOptions, + }, + + // Variants + { + name: 'variant="default" with valid children is accepted', + code: dedent` + + Save + Cancel + + `, + languageOptions, + }, + { + name: 'variant="segmented" is not yet validated — invalid children pass through', + code: dedent` + +
Not a button
+ + `, + languageOptions, + }, + { + name: 'variant="selection" is not yet validated — invalid children pass through', + code: dedent` + +
Not a button
+ + `, + languageOptions, + }, + { + name: 'dynamic variant is conservatively skipped — invalid children pass through', + code: dedent` + +
Not a button
+ + `, + languageOptions, + }, + { + name: 'spread props on child — cannot statically determine', + code: dedent` + + + + `, + languageOptions, + }, + { + name: 'spread props on wrapper child — cannot statically determine', + code: dedent` + + + + + + `, + languageOptions, + }, + { + name: 'map() with expression-body valid button is accepted', + code: dedent` + + {buttons.map((b) => {b.label})} + + `, + languageOptions, + }, + { + name: 'map() with expression-body spread props is skipped — cannot statically determine element identity', + code: dedent` + + {buttons.map((b) => )} + + `, + languageOptions, + }, + { + name: 'map() with block-body single return of valid button is accepted', + code: dedent` + + {buttons.map((b) => { + return {b.label}; + })} + + `, + languageOptions, + }, + { + name: 'map() with block-body early null return and valid button is accepted', + code: dedent` + + {buttons.map((b) => { + if (b.hidden) return null; + return {b.label}; + })} + + `, + languageOptions, + }, + { + name: 'map() with block-body if/else returning different valid buttons is accepted', + code: dedent` + + {buttons.map((b) => { + if (b.type === 'icon') return ; + return {b.label}; + })} + + `, + languageOptions, + }, + { + name: 'variable holding a map() result with valid buttons is accepted', + code: dedent` + const buttons = items.map((item) => {item.label}); + + {buttons} + + `, + languageOptions, + }, + { + name: 'member-expression child (e.g. namespace component) is skipped', + code: dedent` + + + + `, + languageOptions, + }, + + // Switch statement + { + name: 'local arrow-fn with switch returning valid buttons is accepted', + code: dedent` + const ActionButton = ({ variant }) => { + switch (variant) { + case 'icon': return ; + default: return Add; + } + }; + + + + `, + languageOptions, + }, + ], + + invalid: [ + // Direct children + { + name: 'plain div child', + code: dedent` + +
Not a button
+ + `, + languageOptions, + errors: [{ messageId: 'invalidChild', data: { name: 'div' } }], + }, + { + name: 'EuiFlexGroup child', + code: dedent` + + + Save + + + `, + languageOptions, + errors: [{ messageId: 'invalidChild', data: { name: 'EuiFlexGroup' } }], + }, + { + name: 'multiple invalid children reported individually', + code: dedent` + + text + Valid + Also invalid + + `, + languageOptions, + errors: [ + { messageId: 'invalidChild', data: { name: 'span' } }, + { messageId: 'invalidChild', data: { name: 'EuiText' } }, + ], + }, + + // EuiCopy + { + name: 'invalid element returned by EuiCopy render prop', + code: dedent` + + + {(copy) => Not a button} + + + `, + languageOptions, + errors: [ + { + messageId: 'invalidWrapperChild', + data: { name: 'EuiText', wrapper: 'EuiCopy' }, + }, + ], + }, + { + name: 'EuiCopy block-body render prop with invalid return is reported', + code: dedent` + + + {(copy) => { return Not a button; }} + + + `, + languageOptions, + errors: [ + { + messageId: 'invalidWrapperChild', + data: { name: 'EuiText', wrapper: 'EuiCopy' }, + }, + ], + }, + { + name: 'EuiCopy block-body render prop with invalid branch in if/else is reported', + code: dedent` + + + {(copy) => { + if (isBad) return Not a button; + return Copy; + }} + + + `, + languageOptions, + errors: [ + { + messageId: 'invalidWrapperChild', + data: { name: 'EuiText', wrapper: 'EuiCopy' }, + }, + ], + }, + + // EuiPopover + { + name: 'invalid element in EuiPopover button prop', + code: dedent` + + Not a button} isOpen={false} closePopover={() => {}}> + Panel content + + + `, + languageOptions, + errors: [ + { messageId: 'invalidPopoverButton', data: { name: 'EuiText' } }, + ], + }, + { + name: 'HTML element in EuiPopover button prop', + code: dedent` + + Not a button} isOpen={false} closePopover={() => {}}> + Panel content + + + `, + languageOptions, + errors: [{ messageId: 'invalidPopoverButton', data: { name: 'div' } }], + }, + { + name: 'EuiToolTip in EuiPopover button prop with invalid child', + code: dedent` + + Not a button} + isOpen={false} + closePopover={() => {}} + > + Panel content + + + `, + languageOptions, + errors: [ + { messageId: 'invalidPopoverButton', data: { name: 'EuiText' } }, + ], + }, + { + name: 'EuiPopover button prop as resolved variable with invalid element is reported', + code: dedent` + const trigger = Not a button; + + {}}> + Panel content + + + `, + languageOptions, + errors: [ + { messageId: 'invalidPopoverButton', data: { name: 'EuiText' } }, + ], + }, + { + name: 'EuiPopover button prop as conditional with one invalid branch is reported', + code: dedent` + + : Bad} + isOpen={false} + closePopover={() => {}} + > + Panel content + + + `, + languageOptions, + errors: [ + { messageId: 'invalidPopoverButton', data: { name: 'EuiText' } }, + ], + }, + + // Invalid wrapper children + { + name: 'non-button inside EuiToolTip', + code: dedent` + + + + + + + + `, + languageOptions, + errors: [ + { + messageId: 'invalidWrapperChild', + data: { name: 'EuiFlexGroup', wrapper: 'EuiToolTip' }, + }, + ], + }, + + // EuiToolTip + { + name: 'conditional inside EuiToolTip with one invalid branch', + code: dedent` + + + {isIcon + ? + : Not a button} + + + `, + languageOptions, + errors: [ + { + messageId: 'invalidWrapperChild', + data: { name: 'EuiText', wrapper: 'EuiToolTip' }, + }, + ], + }, + { + name: 'variable inside EuiToolTip resolves to an invalid element', + code: dedent` + const icon = Not a button; + + + {icon} + + + `, + languageOptions, + errors: [ + { + messageId: 'invalidWrapperChild', + data: { name: 'EuiText', wrapper: 'EuiToolTip' }, + }, + ], + }, + + // Fragments + { + name: 'shorthand fragment wrapping invalid element', + code: dedent` + + <> + Save + Not allowed + + + `, + languageOptions, + errors: [{ messageId: 'invalidChild', data: { name: 'EuiText' } }], + }, + { + name: ' wrapping invalid element', + code: dedent` + + +
Not a button
+ + + `, + languageOptions, + errors: [{ messageId: 'invalidChild', data: { name: 'div' } }], + }, + { + name: ' wrapping invalid element', + code: dedent` + + + Not allowed + + + `, + languageOptions, + errors: [{ messageId: 'invalidChild', data: { name: 'EuiText' } }], + }, + { + name: 'fragment inside EuiToolTip wrapping invalid element', + code: dedent` + + + <> + Not a button + + + + `, + languageOptions, + errors: [ + { + messageId: 'invalidWrapperChild', + data: { name: 'EuiText', wrapper: 'EuiToolTip' }, + }, + ], + }, + + // Conditional children + { + name: '{condition && } right side is flagged', + code: dedent` + + Save + {show && Not allowed} + + `, + languageOptions, + errors: [{ messageId: 'invalidChild', data: { name: 'EuiText' } }], + }, + { + name: 'ternary with one invalid branch', + code: dedent` + + {isAdmin ? Delete : No access} + + `, + languageOptions, + errors: [{ messageId: 'invalidChild', data: { name: 'EuiText' } }], + }, + { + name: 'ternary with both invalid branches reported separately', + code: dedent` + + {show ? A : B} + + `, + languageOptions, + errors: [ + { messageId: 'invalidChild', data: { name: 'EuiText' } }, + { messageId: 'invalidChild', data: { name: 'EuiBadge' } }, + ], + }, + + { + name: '{unresolvable || } invalid right side is flagged', + code: dedent` + + {fallback || Not a button} + + `, + languageOptions, + errors: [{ messageId: 'invalidChild', data: { name: 'EuiText' } }], + }, + { + name: '{unresolvable ?? } invalid right side is flagged', + code: dedent` + + {fallback ?? Not a button} + + `, + languageOptions, + errors: [{ messageId: 'invalidChild', data: { name: 'EuiText' } }], + }, + { + name: '{resolvedInvalidButton || } resolved invalid left side is flagged', + code: dedent` + const resolvedButton = Not a button; + + {resolvedButton || Save} + + `, + languageOptions, + errors: [{ messageId: 'invalidChild', data: { name: 'EuiText' } }], + }, + + // Unresolvable custom component + { + name: 'unresolvable custom component inside EuiToolTip uses invalidUnresolvableWrapperChild', + code: dedent` + + + + + + `, + languageOptions, + errors: [ + { + messageId: 'invalidUnresolvableWrapperChild', + data: { name: 'SaveButton', wrapper: 'EuiToolTip' }, + }, + ], + }, + { + name: 'unresolvable custom component inside EuiCopy render prop uses invalidUnresolvableWrapperChild', + code: dedent` + + + {(copy) => } + + + `, + languageOptions, + errors: [ + { + messageId: 'invalidUnresolvableWrapperChild', + data: { name: 'SaveButton', wrapper: 'EuiCopy' }, + }, + ], + }, + + // Variable resolution + { + name: 'const variable holding an invalid element is resolved and reported', + code: dedent` + const button = Not a button; + + {button} + + `, + languageOptions, + errors: [{ messageId: 'invalidChild', data: { name: 'EuiText' } }], + }, + { + name: 'const variable holding an array with an invalid element is resolved and reported', + code: dedent` + const buttons = [ + Save, + Not a button, + ]; + + {buttons} + + `, + languageOptions, + errors: [{ messageId: 'invalidChild', data: { name: 'EuiText' } }], + }, + + // Local arrow-function + { + name: 'local arrow-fn component with invalid return is resolved and reported at the invalid element', + code: dedent` + const ActionButtons = () => Not a button; + + + + `, + languageOptions, + errors: [{ messageId: 'invalidChild', data: { name: 'EuiText' } }], + }, + { + name: 'local block-body arrow-fn component with invalid return is resolved and reported at the invalid element', + code: dedent` + const ActionButtons = () => { return Not a button; }; + + + + `, + languageOptions, + errors: [{ messageId: 'invalidChild', data: { name: 'EuiText' } }], + }, + { + name: 'local block-body arrow-fn component with invalid branch is resolved and reported', + code: dedent` + const ActionButtons = ({ isInvalid }) => { + if (isInvalid) return Bad; + return Good; + }; + + + + `, + languageOptions, + errors: [{ messageId: 'invalidChild', data: { name: 'EuiText' } }], + }, + { + name: 'imported custom component cannot be resolved — uses invalidUnresolvableChild message', + code: dedent` + + + + `, + languageOptions, + errors: [ + { + messageId: 'invalidUnresolvableChild', + data: { name: 'SaveButton' }, + }, + ], + }, + + // Array notation + { + name: 'array containing an invalid element', + code: dedent` + + {[ + Save, + Not allowed, + ]} + + `, + languageOptions, + errors: [{ messageId: 'invalidChild', data: { name: 'EuiText' } }], + }, + { + name: 'array containing a wrapper (EuiToolTip) with an invalid inner element', + code: dedent` + + {[ + + Not a button + , + ]} + + `, + languageOptions, + errors: [ + { + messageId: 'invalidWrapperChild', + data: { name: 'EuiText', wrapper: 'EuiToolTip' }, + }, + ], + }, + + // .map() + { + name: 'map() with expression-body returning invalid element is reported', + code: dedent` + + {buttons.map((b) =>
{b.label}
)} + + `, + languageOptions, + errors: [{ messageId: 'invalidChild', data: { name: 'div' } }], + }, + { + name: 'map() with block-body returning invalid element is reported', + code: dedent` + + {buttons.map((b) => { + return
{b.label}
; + })} + + `, + languageOptions, + errors: [{ messageId: 'invalidChild', data: { name: 'div' } }], + }, + { + name: 'map() with block-body invalid branch is reported', + code: dedent` + + {buttons.map((b) => { + if (b.type === 'bad') return {b.label}; + return {b.label}; + })} + + `, + languageOptions, + errors: [{ messageId: 'invalidChild', data: { name: 'EuiText' } }], + }, + { + name: 'variable holding map() result with invalid element is reported', + code: dedent` + const buttons = items.map((item) =>
{item.label}
); + + {buttons} + + `, + languageOptions, + errors: [{ messageId: 'invalidChild', data: { name: 'div' } }], + }, + + // Spread on wrapper + { + name: 'EuiToolTip with spread props still validates its children', + code: dedent` + + + Not a button + + + `, + languageOptions, + errors: [ + { + messageId: 'invalidWrapperChild', + data: { name: 'EuiText', wrapper: 'EuiToolTip' }, + }, + ], + }, + { + name: 'EuiCopy with spread props still validates its render-prop child', + code: dedent` + + + {(copy) => Not a button} + + + `, + languageOptions, + errors: [ + { + messageId: 'invalidWrapperChild', + data: { name: 'EuiText', wrapper: 'EuiCopy' }, + }, + ], + }, + + // Switch statement + { + name: 'local arrow-fn with switch containing invalid branch is reported', + code: dedent` + const ActionButton = ({ variant }) => { + switch (variant) { + case 'bad': return Not a button; + default: return Good; + } + }; + + + + `, + languageOptions, + errors: [{ messageId: 'invalidChild', data: { name: 'EuiText' } }], + }, + ], + } +); diff --git a/packages/eslint-plugin/src/rules/button_group_no_invalid_children.ts b/packages/eslint-plugin/src/rules/button_group_no_invalid_children.ts new file mode 100644 index 00000000000..9d454250807 --- /dev/null +++ b/packages/eslint-plugin/src/rules/button_group_no_invalid_children.ts @@ -0,0 +1,216 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the Elastic License + * 2.0 and the Server Side Public License, v 1; you may not use this file except + * in compliance with, at your election, the Elastic License 2.0 or the Server + * Side Public License, v 1. + */ + +import { type TSESTree, type TSESLint, ESLintUtils } from '@typescript-eslint/utils'; +import { hasSpread } from '../utils/has_spread'; +import { flatMap } from '../utils/flat_map'; +import { getElementName } from '../utils/get_element_name'; +import { collectJsxChildren } from '../utils/collect_jsx_children'; +import { findAttrValue } from '../utils/get_attr_value'; + +const BUTTON_GROUP = 'EuiButtonGroup'; +const VALID_BUTTONS = new Set(['EuiButton', 'EuiButtonEmpty', 'EuiButtonIcon']); +const VALID_WRAPPERS = new Set(['EuiToolTip', 'EuiPopover', 'EuiCopy']); + +function joinSet(set: Set): string { + const items: string[] = []; + set.forEach((v) => items.push(v)); + return items.join(', '); +} + +const VALID_BUTTONS_LIST = joinSet(VALID_BUTTONS); +const VALID_WRAPPERS_LIST = joinSet(VALID_WRAPPERS); + +function isCustomComponent(name: string): boolean { + return name[0] === name[0].toUpperCase() && !name.startsWith('Eui'); +} + +function reportInvalidWrapperChildren< + TContext extends TSESLint.RuleContext +>( + wrapper: TSESTree.JSXElement, + wrapperName: string, + context: TContext +): void { + const children = flatMap(wrapper.children, (c) => + collectJsxChildren(c, context.sourceCode) + ); + for (const wc of children) { + if (hasSpread(wc.openingElement.attributes)) continue; + const wcName = getElementName(wc.openingElement); + if (wcName === null || VALID_BUTTONS.has(wcName)) continue; + context.report({ + node: wc.openingElement, + messageId: isCustomComponent(wcName) + ? 'invalidUnresolvableWrapperChild' + : 'invalidWrapperChild', + data: { name: wcName, wrapper: wrapperName }, + }); + } +} + +/* Rule */ + +export const ButtonGroupNoInvalidChildren = ESLintUtils.RuleCreator.withoutDocs( + { + create(context) { + return { + JSXElement(node) { + const { openingElement } = node; + + if ( + openingElement.name.type !== 'JSXIdentifier' || + openingElement.name.name !== BUTTON_GROUP + ) { + return; + } + + // Only validate when the variant is "default" (or absent — the default). + // Rules for "segmented" and "selection" are added in later chunks. + // Dynamic variant values (variables) are conservatively skipped. + const variant = findAttrValue( + context, + openingElement.attributes, + 'variant' + ); + if (variant !== undefined && variant !== 'default') return; + + const children = flatMap(node.children, (c) => + collectJsxChildren(c, context.sourceCode) + ); + if (children.length === 0) return; + + for (const child of children) { + const name = getElementName(child.openingElement); + if (name === null) continue; + + if (VALID_BUTTONS.has(name)) continue; + + if (VALID_WRAPPERS.has(name)) { + if (name === 'EuiToolTip') { + // Validate JSX children (expanding fragments/conditionals). + reportInvalidWrapperChildren(child, name, context); + } else if (name === 'EuiPopover') { + // The trigger is the `button` prop, not JSX children (panel + // content). The prop value may be a JSXExpressionContainer or + // a bare JSX element; collectJsxChildren handles both. + // EuiToolTip wrapping the trigger button is also supported. + const buttonProp = child.openingElement.attributes.find( + (attr): attr is TSESTree.JSXAttribute => + attr.type === 'JSXAttribute' && + attr.name.type === 'JSXIdentifier' && + attr.name.name === 'button' + ); + + if (buttonProp?.value != null) { + const triggerElements = collectJsxChildren( + buttonProp.value as TSESTree.Node, + context.sourceCode + ); + + for (const te of triggerElements) { + if (hasSpread(te.openingElement.attributes)) continue; + + const teName = getElementName(te.openingElement); + + if (teName === null || VALID_BUTTONS.has(teName)) continue; + + if (teName === 'EuiToolTip') { + // EuiToolTip wrapping the trigger — validate its children. + const tooltipChildren = flatMap(te.children, (c) => + collectJsxChildren(c, context.sourceCode) + ); + + for (const ttc of tooltipChildren) { + if (hasSpread(ttc.openingElement.attributes)) continue; + + const ttcName = getElementName(ttc.openingElement); + + if (ttcName === null || VALID_BUTTONS.has(ttcName)) + continue; + + context.report({ + node: ttc.openingElement, + messageId: 'invalidPopoverButton', + data: { name: ttcName }, + }); + } + continue; + } + context.report({ + node: te.openingElement, + messageId: 'invalidPopoverButton', + data: { name: teName }, + }); + } + } + } else if (name === 'EuiCopy') { + // Children is a render prop: {(copy) => } + // ArrowFunctionExpression with expression body is expanded by + // collectJsxChildren, so validation works the same as EuiToolTip. + reportInvalidWrapperChildren(child, name, context); + } + continue; + } + + // For elements that aren't known valid buttons or wrappers, a spread + // prevents static classification; skip conservatively. + if (hasSpread(child.openingElement.attributes)) continue; + + context.report({ + node: child.openingElement, + messageId: isCustomComponent(name) + ? 'invalidUnresolvableChild' + : 'invalidChild', + data: { name }, + }); + } + }, + }; + }, + meta: { + type: 'problem', + docs: { + description: `Enforce that EuiButtonGroup children are ${VALID_BUTTONS_LIST}, or a supported wrapper (${VALID_WRAPPERS_LIST})`, + }, + schema: [], + messages: { + invalidChild: [ + `{{ name }} is not a valid child of EuiButtonGroup.`, + `Allowed children: ${VALID_BUTTONS_LIST}.`, + `Allowed wrappers: ${VALID_WRAPPERS_LIST}.`, + ].join(' '), + invalidUnresolvableChild: [ + `{{ name }} cannot be verified as a valid child of EuiButtonGroup.`, + `Allowed children: ${VALID_BUTTONS_LIST}.`, + `Allowed wrappers: ${VALID_WRAPPERS_LIST}.`, + `If {{ name }} is a shared button wrapper component only containing`, + `valid button children, suppress this rule inline with a comment`, + `explaining why it's valid.`, + `// eslint-disable-next-line @elastic/eui/button-group-no-invalid-children -- SaveButton only wraps EuiButton`, + ].join(' '), + invalidPopoverButton: [ + `{{ name }} is not a valid trigger button for EuiPopover in EuiButtonGroup.`, + `The \`button\` prop must be ${VALID_BUTTONS_LIST}.`, + ].join(' '), + invalidWrapperChild: [ + `{{ name }} inside {{ wrapper }} is not a valid button for EuiButtonGroup.`, + `The wrapper must contain ${VALID_BUTTONS_LIST}.`, + ].join(' '), + invalidUnresolvableWrapperChild: [ + `{{ name }} inside {{ wrapper }} cannot be verified as a valid button for EuiButtonGroup.`, + `The wrapper must contain ${VALID_BUTTONS_LIST}.`, + `If {{ name }} is a shared button wrapper component, suppress this rule inline`, + `with a comment explaining why it renders valid button children:`, + `// eslint-disable-next-line @elastic/eui/button-group-no-invalid-children -- SaveButton wraps EuiButton`, + ].join(' '), + }, + }, + defaultOptions: [], + } +); diff --git a/packages/eslint-plugin/src/utils/collect_jsx_children.ts b/packages/eslint-plugin/src/utils/collect_jsx_children.ts new file mode 100644 index 00000000000..2f9e9302684 --- /dev/null +++ b/packages/eslint-plugin/src/utils/collect_jsx_children.ts @@ -0,0 +1,198 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the Elastic License + * 2.0 and the Server Side Public License, v 1; you may not use this file except + * in compliance with, at your election, the Elastic License 2.0 or the Server + * Side Public License, v 1. + */ + +import { type TSESTree, type TSESLint } from '@typescript-eslint/utils'; +import { flatMap } from './flat_map'; +import { isFragment } from './is_fragment'; + +// A variable is only safely resolvable if it is never reassigned. A second +// write means the value at the point of use can't be determined statically. +function isReassigned(variable: TSESLint.Scope.Variable): boolean { + return variable.references.filter((ref) => ref.isWrite()).length > 1; +} + +/** + * Resolves an expression-context Identifier to the node its variable was + * initialized with (e.g. the `` in `const btn = `). + * Returns null for anything that can't be resolved safely: imports, function + * parameters, reassigned variables, or missing initializers. + */ +export function resolveIdentifierValue( + sourceCode: TSESLint.SourceCode, + node: TSESTree.Identifier +): TSESTree.Node | null { + let scope: TSESLint.Scope.Scope | null = sourceCode.getScope(node); + while (scope) { + const ref = scope.references.find((r) => r.identifier === node); + if (ref) { + const variable = ref.resolved; + if (!variable || isReassigned(variable)) return null; + const def = variable.defs[0]; + if (!def || def.type !== 'Variable' || !def.node.init) return null; + return def.node.init; + } + scope = scope.upper; + } + return null; +} + +/** + * Collects all return-statement arguments within a block body, recursing into + * `if`/`else` branches and `switch` cases. Does not recurse into nested + * functions — those have their own return scope. Used to validate block-body + * arrow functions. + */ +export function collectReturnValues( + node: TSESTree.Node +): TSESTree.Expression[] { + switch (node.type) { + case 'BlockStatement': + return flatMap(node.body, collectReturnValues); + case 'ReturnStatement': + return node.argument ? [node.argument] : []; + case 'IfStatement': + return [ + ...collectReturnValues(node.consequent), + ...(node.alternate ? collectReturnValues(node.alternate) : []), + ]; + case 'SwitchStatement': + return flatMap(node.cases, (c) => + flatMap(c.consequent, collectReturnValues) + ); + default: + return []; + } +} + +/** + * Resolves a JSX component name (e.g. `Buttons` in ``) to the + * nodes returned by its local arrow-function definition, if one exists. + * + * Handles both expression-body (`() => `) and block-body + * (`() => { return ; }`) arrow functions. Imports, class components, + * and reassigned variables return null — those stay opaque. + */ +export function resolveLocalComponent( + sourceCode: TSESLint.SourceCode, + name: string, + refNode: TSESTree.Node +): TSESTree.Node[] | null { + let scope: TSESLint.Scope.Scope | null = sourceCode.getScope(refNode); + while (scope) { + const variable = scope.variables.find((v) => v.name === name); + if (variable) { + if (isReassigned(variable)) return null; + const def = variable.defs[0]; + if (!def || def.type !== 'Variable' || !def.node.init) return null; + const init = def.node.init; + if (init.type !== 'ArrowFunctionExpression') return null; + if (init.body.type !== 'BlockStatement') return [init.body]; + const returns = collectReturnValues(init.body); + return returns.length > 0 ? returns : null; + } + scope = scope.upper; + } + return null; +} + +/** + * Recursively collects concrete JSXElement nodes to validate, expanding: + * - `<>`, ``, `` (transparent grouping) + * - `{expr}` (JSXExpressionContainer) + * - `{a && }` (LogicalExpression `&&` → right side only) + * - `{a || }`, `{a ?? }` (LogicalExpression `||`/`??` → both sides) + * - `{c ? : }` (ConditionalExpression → both branches) + * - `{[, ]}` (ArrayExpression → each element) + * - `{variable}` (Identifier → resolved via scope) + * - `` (local arrow-fn component, expression or block body) + * - `{(arg) => }` (ArrowFunctionExpression, expression or block body) + * - `{arr.map(fn)}` (CallExpression `.map()` with arrow-fn callback) + * + * Patterns that can't be resolved statically (arbitrary function calls, imported + * variables, non-arrow `.map()` callbacks) produce an empty list and are silently + * skipped. + */ +export function collectJsxChildren( + node: TSESTree.Node, + sourceCode: TSESLint.SourceCode +): TSESTree.JSXElement[] { + return collectChildren(node, sourceCode, new Set()); +} + +function collectChildren( + node: TSESTree.Node, + sourceCode: TSESLint.SourceCode, + visited: Set +): TSESTree.JSXElement[] { + const collect = (n: TSESTree.Node) => collectChildren(n, sourceCode, visited); + + switch (node.type) { + case 'JSXElement': { + if (isFragment(node.openingElement)) { + return flatMap(node.children, collect); + } + const { name } = node.openingElement; + if (name.type === 'JSXIdentifier') { + if (visited.has(name.name)) return []; + + const resolved = resolveLocalComponent( + sourceCode, + name.name, + node.openingElement + ); + if (resolved !== null) { + visited.add(name.name); + + const result = flatMap(resolved, collect); + + visited.delete(name.name); + return result; + } + } + return [node]; + } + case 'JSXFragment': + return flatMap(node.children, collect); + case 'JSXExpressionContainer': + if (node.expression.type === 'JSXEmptyExpression') return []; + return collect(node.expression); + case 'LogicalExpression': + if (node.operator === '&&') return collect(node.right); + return [...collect(node.left), ...collect(node.right)]; + case 'ConditionalExpression': + return [...collect(node.consequent), ...collect(node.alternate)]; + case 'ArrayExpression': + return flatMap(node.elements, (el) => + el && el.type !== 'SpreadElement' ? collect(el) : [] + ); + case 'Identifier': { + const resolved = resolveIdentifierValue(sourceCode, node); + return resolved ? collect(resolved) : []; + } + case 'ArrowFunctionExpression': + if (node.body.type !== 'BlockStatement') return collect(node.body); + return flatMap(collectReturnValues(node.body), collect); + case 'CallExpression': { + const { callee, arguments: args } = node; + if ( + callee.type === 'MemberExpression' && + callee.property.type === 'Identifier' && + callee.property.name === 'map' && + args.length >= 1 && + args[0].type === 'ArrowFunctionExpression' + ) { + const fn = args[0]; + if (fn.body.type !== 'BlockStatement') return collect(fn.body); + return flatMap(collectReturnValues(fn.body), collect); + } + return []; + } + default: + return []; + } +} diff --git a/packages/eslint-plugin/src/utils/flat_map.ts b/packages/eslint-plugin/src/utils/flat_map.ts new file mode 100644 index 00000000000..fee41269a24 --- /dev/null +++ b/packages/eslint-plugin/src/utils/flat_map.ts @@ -0,0 +1,13 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the Elastic License + * 2.0 and the Server Side Public License, v 1; you may not use this file except + * in compliance with, at your election, the Elastic License 2.0 or the Server + * Side Public License, v 1. + */ + +// `Array.prototype.flatMap` requires ES2019 lib; this shim keeps rules that +// need it within the package's es5 lib setting without widening it for others. +export function flatMap(arr: readonly T[], fn: (item: T) => U[]): U[] { + return arr.reduce((acc, item) => acc.concat(fn(item)), []); +} diff --git a/packages/eslint-plugin/src/utils/is_fragment.ts b/packages/eslint-plugin/src/utils/is_fragment.ts new file mode 100644 index 00000000000..c4cdf994de8 --- /dev/null +++ b/packages/eslint-plugin/src/utils/is_fragment.ts @@ -0,0 +1,25 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the Elastic License + * 2.0 and the Server Side Public License, v 1; you may not use this file except + * in compliance with, at your election, the Elastic License 2.0 or the Server + * Side Public License, v 1. + */ + +import { type TSESTree } from '@typescript-eslint/utils'; + +/** + * Returns true if the opening element is a React fragment: bare ``, + * ``, or the `<>` shorthand. + */ +export function isFragment(opening: TSESTree.JSXOpeningElement): boolean { + const { name } = opening; + if (name.type === 'JSXIdentifier' && name.name === 'Fragment') return true; + return ( + name.type === 'JSXMemberExpression' && + name.object.type === 'JSXIdentifier' && + name.object.name === 'React' && + name.property.type === 'JSXIdentifier' && + name.property.name === 'Fragment' + ); +} From 86210d5dd7a5d2f78001238e6cb9be08c02c1ada Mon Sep 17 00:00:00 2001 From: Lene Gadewoll Date: Tue, 28 Jul 2026 12:00:14 +0200 Subject: [PATCH 02/10] refactor: improve flatMap --- packages/eslint-plugin/src/utils/flat_map.ts | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/packages/eslint-plugin/src/utils/flat_map.ts b/packages/eslint-plugin/src/utils/flat_map.ts index fee41269a24..6a1907dbf66 100644 --- a/packages/eslint-plugin/src/utils/flat_map.ts +++ b/packages/eslint-plugin/src/utils/flat_map.ts @@ -9,5 +9,10 @@ // `Array.prototype.flatMap` requires ES2019 lib; this shim keeps rules that // need it within the package's es5 lib setting without widening it for others. export function flatMap(arr: readonly T[], fn: (item: T) => U[]): U[] { - return arr.reduce((acc, item) => acc.concat(fn(item)), []); + const out: U[] = []; + for (const item of arr) { + const items = fn(item); + for (const i of items) out.push(i); + } + return out; } From c6f5d1275b0cc8bda9f1de9c902bd8a8cbff9dfb Mon Sep 17 00:00:00 2001 From: Lene Gadewoll Date: Tue, 28 Jul 2026 12:00:47 +0200 Subject: [PATCH 03/10] style: fix JSX docstring --- packages/eslint-plugin/src/utils/collect_jsx_children.ts | 2 +- packages/eslint-plugin/src/utils/is_fragment.ts | 5 +++-- 2 files changed, 4 insertions(+), 3 deletions(-) diff --git a/packages/eslint-plugin/src/utils/collect_jsx_children.ts b/packages/eslint-plugin/src/utils/collect_jsx_children.ts index 2f9e9302684..471659b8fae 100644 --- a/packages/eslint-plugin/src/utils/collect_jsx_children.ts +++ b/packages/eslint-plugin/src/utils/collect_jsx_children.ts @@ -102,7 +102,7 @@ export function resolveLocalComponent( /** * Recursively collects concrete JSXElement nodes to validate, expanding: - * - `<>`, ``, `` (transparent grouping) + * - `<>...`, ``, `` (transparent grouping) * - `{expr}` (JSXExpressionContainer) * - `{a && }` (LogicalExpression `&&` → right side only) * - `{a || }`, `{a ?? }` (LogicalExpression `||`/`??` → both sides) diff --git a/packages/eslint-plugin/src/utils/is_fragment.ts b/packages/eslint-plugin/src/utils/is_fragment.ts index c4cdf994de8..db0e10ad7c2 100644 --- a/packages/eslint-plugin/src/utils/is_fragment.ts +++ b/packages/eslint-plugin/src/utils/is_fragment.ts @@ -9,8 +9,9 @@ import { type TSESTree } from '@typescript-eslint/utils'; /** - * Returns true if the opening element is a React fragment: bare ``, - * ``, or the `<>` shorthand. + * Returns true if the opening element is a React fragment: `` or + * ``. The `<>` shorthand is represented as a `JSXFragment` + * node (not a `JSXOpeningElement`) and is handled separately by callers. */ export function isFragment(opening: TSESTree.JSXOpeningElement): boolean { const { name } = opening; From 62f9fdedc3d946ac709e6173649df7e1a11d742f Mon Sep 17 00:00:00 2001 From: Lene Gadewoll Date: Tue, 28 Jul 2026 12:00:56 +0200 Subject: [PATCH 04/10] chore: add changelog --- packages/eslint-plugin/changelogs/upcoming/9849.md | 1 + 1 file changed, 1 insertion(+) create mode 100644 packages/eslint-plugin/changelogs/upcoming/9849.md diff --git a/packages/eslint-plugin/changelogs/upcoming/9849.md b/packages/eslint-plugin/changelogs/upcoming/9849.md new file mode 100644 index 00000000000..e4b3a79d03c --- /dev/null +++ b/packages/eslint-plugin/changelogs/upcoming/9849.md @@ -0,0 +1 @@ +- Added `button-group-no-invalid-children` rule \ No newline at end of file From 9b74820b6ddaa4b883d67c85d141a5344ed380b3 Mon Sep 17 00:00:00 2001 From: Lene Gadewoll Date: Thu, 30 Jul 2026 14:47:13 +0200 Subject: [PATCH 05/10] refactor: change default rule to error - since the component API is new and there aren't any usages yet, we can enforce the right usage strictly from the beginning --- packages/eslint-plugin/src/index.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/eslint-plugin/src/index.ts b/packages/eslint-plugin/src/index.ts index bc18365226c..12ce466e50b 100644 --- a/packages/eslint-plugin/src/index.ts +++ b/packages/eslint-plugin/src/index.ts @@ -60,6 +60,7 @@ const config = { plugins: ['@elastic/eslint-plugin-eui'], rules: { '@elastic/eui/accessible-interactive-element': 'warn', + '@elastic/eui/button-group-no-invalid-children': 'error', '@elastic/eui/callout-announce-on-mount': 'warn', '@elastic/eui/callout-prefer-props-for-content': 'warn', '@elastic/eui/consistent-is-invalid-props': 'warn', @@ -80,7 +81,6 @@ const config = { '@elastic/eui/require-href-for-link': 'warn', '@elastic/eui/tooltip-no-interactive-content': 'warn', '@elastic/eui/tooltip-button-icon-wrap': 'warn', - '@elastic/eui/button-group-no-invalid-children': 'warn', }, }, }, From 94f0e9df01c34bf5966fa2b6f87b378ff3fc4efc Mon Sep 17 00:00:00 2001 From: Lene Gadewoll Date: Thu, 30 Jul 2026 16:03:23 +0200 Subject: [PATCH 06/10] refactor: rename variable names --- .../rules/button_group_no_invalid_children.ts | 42 +++++++++---------- 1 file changed, 21 insertions(+), 21 deletions(-) diff --git a/packages/eslint-plugin/src/rules/button_group_no_invalid_children.ts b/packages/eslint-plugin/src/rules/button_group_no_invalid_children.ts index 9d454250807..f1dd71063a7 100644 --- a/packages/eslint-plugin/src/rules/button_group_no_invalid_children.ts +++ b/packages/eslint-plugin/src/rules/button_group_no_invalid_children.ts @@ -40,16 +40,16 @@ function reportInvalidWrapperChildren< const children = flatMap(wrapper.children, (c) => collectJsxChildren(c, context.sourceCode) ); - for (const wc of children) { - if (hasSpread(wc.openingElement.attributes)) continue; - const wcName = getElementName(wc.openingElement); - if (wcName === null || VALID_BUTTONS.has(wcName)) continue; + for (const wrapperChild of children) { + if (hasSpread(wrapperChild.openingElement.attributes)) continue; + const wrapperChildName = getElementName(wrapperChild.openingElement); + if (wrapperChildName === null || VALID_BUTTONS.has(wrapperChildName)) continue; context.report({ - node: wc.openingElement, - messageId: isCustomComponent(wcName) + node: wrapperChild.openingElement, + messageId: isCustomComponent(wrapperChildName) ? 'invalidUnresolvableWrapperChild' : 'invalidWrapperChild', - data: { name: wcName, wrapper: wrapperName }, + data: { name: wrapperChildName, wrapper: wrapperName }, }); } } @@ -113,39 +113,39 @@ export const ButtonGroupNoInvalidChildren = ESLintUtils.RuleCreator.withoutDocs( context.sourceCode ); - for (const te of triggerElements) { - if (hasSpread(te.openingElement.attributes)) continue; + for (const triggerElement of triggerElements) { + if (hasSpread(triggerElement.openingElement.attributes)) continue; - const teName = getElementName(te.openingElement); + const triggerElementName = getElementName(triggerElement.openingElement); - if (teName === null || VALID_BUTTONS.has(teName)) continue; + if (triggerElementName === null || VALID_BUTTONS.has(triggerElementName)) continue; - if (teName === 'EuiToolTip') { + if (triggerElementName === 'EuiToolTip') { // EuiToolTip wrapping the trigger — validate its children. - const tooltipChildren = flatMap(te.children, (c) => + const tooltipChildren = flatMap(triggerElement.children, (c) => collectJsxChildren(c, context.sourceCode) ); - for (const ttc of tooltipChildren) { - if (hasSpread(ttc.openingElement.attributes)) continue; + for (const tooltipChild of tooltipChildren) { + if (hasSpread(tooltipChild.openingElement.attributes)) continue; - const ttcName = getElementName(ttc.openingElement); + const tooltipChildName = getElementName(tooltipChild.openingElement); - if (ttcName === null || VALID_BUTTONS.has(ttcName)) + if (tooltipChildName === null || VALID_BUTTONS.has(tooltipChildName)) continue; context.report({ - node: ttc.openingElement, + node: tooltipChild.openingElement, messageId: 'invalidPopoverButton', - data: { name: ttcName }, + data: { name: tooltipChildName }, }); } continue; } context.report({ - node: te.openingElement, + node: triggerElement.openingElement, messageId: 'invalidPopoverButton', - data: { name: teName }, + data: { name: triggerElementName }, }); } } From ab2fc28a8ea3c8079c5087418e14ccfd968f18aa Mon Sep 17 00:00:00 2001 From: Lene Gadewoll Date: Fri, 31 Jul 2026 13:58:12 +0200 Subject: [PATCH 07/10] chore: cleanup comment and formatting --- .../rules/button_group_no_invalid_children.ts | 49 ++++++++++++------- 1 file changed, 31 insertions(+), 18 deletions(-) diff --git a/packages/eslint-plugin/src/rules/button_group_no_invalid_children.ts b/packages/eslint-plugin/src/rules/button_group_no_invalid_children.ts index f1dd71063a7..3e55e24904d 100644 --- a/packages/eslint-plugin/src/rules/button_group_no_invalid_children.ts +++ b/packages/eslint-plugin/src/rules/button_group_no_invalid_children.ts @@ -6,7 +6,11 @@ * Side Public License, v 1. */ -import { type TSESTree, type TSESLint, ESLintUtils } from '@typescript-eslint/utils'; +import { + type TSESTree, + type TSESLint, + ESLintUtils, +} from '@typescript-eslint/utils'; import { hasSpread } from '../utils/has_spread'; import { flatMap } from '../utils/flat_map'; import { getElementName } from '../utils/get_element_name'; @@ -31,19 +35,16 @@ function isCustomComponent(name: string): boolean { } function reportInvalidWrapperChildren< - TContext extends TSESLint.RuleContext ->( - wrapper: TSESTree.JSXElement, - wrapperName: string, - context: TContext -): void { + TContext extends TSESLint.RuleContext, +>(wrapper: TSESTree.JSXElement, wrapperName: string, context: TContext): void { const children = flatMap(wrapper.children, (c) => collectJsxChildren(c, context.sourceCode) ); for (const wrapperChild of children) { if (hasSpread(wrapperChild.openingElement.attributes)) continue; const wrapperChildName = getElementName(wrapperChild.openingElement); - if (wrapperChildName === null || VALID_BUTTONS.has(wrapperChildName)) continue; + if (wrapperChildName === null || VALID_BUTTONS.has(wrapperChildName)) + continue; context.report({ node: wrapperChild.openingElement, messageId: isCustomComponent(wrapperChildName) @@ -54,8 +55,6 @@ function reportInvalidWrapperChildren< } } -/* Rule */ - export const ButtonGroupNoInvalidChildren = ESLintUtils.RuleCreator.withoutDocs( { create(context) { @@ -114,24 +113,38 @@ export const ButtonGroupNoInvalidChildren = ESLintUtils.RuleCreator.withoutDocs( ); for (const triggerElement of triggerElements) { - if (hasSpread(triggerElement.openingElement.attributes)) continue; + if (hasSpread(triggerElement.openingElement.attributes)) + continue; - const triggerElementName = getElementName(triggerElement.openingElement); + const triggerElementName = getElementName( + triggerElement.openingElement + ); - if (triggerElementName === null || VALID_BUTTONS.has(triggerElementName)) continue; + if ( + triggerElementName === null || + VALID_BUTTONS.has(triggerElementName) + ) + continue; if (triggerElementName === 'EuiToolTip') { // EuiToolTip wrapping the trigger — validate its children. - const tooltipChildren = flatMap(triggerElement.children, (c) => - collectJsxChildren(c, context.sourceCode) + const tooltipChildren = flatMap( + triggerElement.children, + (c) => collectJsxChildren(c, context.sourceCode) ); for (const tooltipChild of tooltipChildren) { - if (hasSpread(tooltipChild.openingElement.attributes)) continue; + if (hasSpread(tooltipChild.openingElement.attributes)) + continue; - const tooltipChildName = getElementName(tooltipChild.openingElement); + const tooltipChildName = getElementName( + tooltipChild.openingElement + ); - if (tooltipChildName === null || VALID_BUTTONS.has(tooltipChildName)) + if ( + tooltipChildName === null || + VALID_BUTTONS.has(tooltipChildName) + ) continue; context.report({ From 2b54a6899bbb1a3d2317782239683599511dcea8 Mon Sep 17 00:00:00 2001 From: Lene Gadewoll Date: Fri, 31 Jul 2026 13:59:06 +0200 Subject: [PATCH 08/10] refactor: remove joinSet util --- .../src/rules/button_group_no_invalid_children.ts | 10 ++-------- 1 file changed, 2 insertions(+), 8 deletions(-) diff --git a/packages/eslint-plugin/src/rules/button_group_no_invalid_children.ts b/packages/eslint-plugin/src/rules/button_group_no_invalid_children.ts index 3e55e24904d..eaffb612d67 100644 --- a/packages/eslint-plugin/src/rules/button_group_no_invalid_children.ts +++ b/packages/eslint-plugin/src/rules/button_group_no_invalid_children.ts @@ -21,14 +21,8 @@ const BUTTON_GROUP = 'EuiButtonGroup'; const VALID_BUTTONS = new Set(['EuiButton', 'EuiButtonEmpty', 'EuiButtonIcon']); const VALID_WRAPPERS = new Set(['EuiToolTip', 'EuiPopover', 'EuiCopy']); -function joinSet(set: Set): string { - const items: string[] = []; - set.forEach((v) => items.push(v)); - return items.join(', '); -} - -const VALID_BUTTONS_LIST = joinSet(VALID_BUTTONS); -const VALID_WRAPPERS_LIST = joinSet(VALID_WRAPPERS); +const VALID_BUTTONS_LIST = Array.from(VALID_BUTTONS).join(', '); +const VALID_WRAPPERS_LIST = Array.from(VALID_WRAPPERS).join(', '); function isCustomComponent(name: string): boolean { return name[0] === name[0].toUpperCase() && !name.startsWith('Eui'); From 559029239a68ffb4b0cc1b7b38ee136a59af1ff4 Mon Sep 17 00:00:00 2001 From: Lene Gadewoll Date: Fri, 31 Jul 2026 14:07:39 +0200 Subject: [PATCH 09/10] refactor: remove EUI- guard in isCustomComponent and ensure custom component usages get proper hint messages --- .../button_group_no_invalid_children.test.ts | 66 +++++++++---------- .../rules/button_group_no_invalid_children.ts | 18 ++++- 2 files changed, 48 insertions(+), 36 deletions(-) diff --git a/packages/eslint-plugin/src/rules/button_group_no_invalid_children.test.ts b/packages/eslint-plugin/src/rules/button_group_no_invalid_children.test.ts index 3d3be39ba6f..6d5805c2432 100644 --- a/packages/eslint-plugin/src/rules/button_group_no_invalid_children.test.ts +++ b/packages/eslint-plugin/src/rules/button_group_no_invalid_children.test.ts @@ -635,7 +635,7 @@ ruleTester.run( `, languageOptions, - errors: [{ messageId: 'invalidChild', data: { name: 'EuiFlexGroup' } }], + errors: [{ messageId: 'invalidUnresolvableChild', data: { name: 'EuiFlexGroup' } }], }, { name: 'multiple invalid children reported individually', @@ -649,7 +649,7 @@ ruleTester.run( languageOptions, errors: [ { messageId: 'invalidChild', data: { name: 'span' } }, - { messageId: 'invalidChild', data: { name: 'EuiText' } }, + { messageId: 'invalidUnresolvableChild', data: { name: 'EuiText' } }, ], }, @@ -666,7 +666,7 @@ ruleTester.run( languageOptions, errors: [ { - messageId: 'invalidWrapperChild', + messageId: 'invalidUnresolvableWrapperChild', data: { name: 'EuiText', wrapper: 'EuiCopy' }, }, ], @@ -683,7 +683,7 @@ ruleTester.run( languageOptions, errors: [ { - messageId: 'invalidWrapperChild', + messageId: 'invalidUnresolvableWrapperChild', data: { name: 'EuiText', wrapper: 'EuiCopy' }, }, ], @@ -703,7 +703,7 @@ ruleTester.run( languageOptions, errors: [ { - messageId: 'invalidWrapperChild', + messageId: 'invalidUnresolvableWrapperChild', data: { name: 'EuiText', wrapper: 'EuiCopy' }, }, ], @@ -721,7 +721,7 @@ ruleTester.run( `, languageOptions, errors: [ - { messageId: 'invalidPopoverButton', data: { name: 'EuiText' } }, + { messageId: 'invalidUnresolvablePopoverButton', data: { name: 'EuiText' } }, ], }, { @@ -751,7 +751,7 @@ ruleTester.run( `, languageOptions, errors: [ - { messageId: 'invalidPopoverButton', data: { name: 'EuiText' } }, + { messageId: 'invalidUnresolvablePopoverButton', data: { name: 'EuiText' } }, ], }, { @@ -766,7 +766,7 @@ ruleTester.run( `, languageOptions, errors: [ - { messageId: 'invalidPopoverButton', data: { name: 'EuiText' } }, + { messageId: 'invalidUnresolvablePopoverButton', data: { name: 'EuiText' } }, ], }, { @@ -784,7 +784,7 @@ ruleTester.run( `, languageOptions, errors: [ - { messageId: 'invalidPopoverButton', data: { name: 'EuiText' } }, + { messageId: 'invalidUnresolvablePopoverButton', data: { name: 'EuiText' } }, ], }, @@ -803,7 +803,7 @@ ruleTester.run( languageOptions, errors: [ { - messageId: 'invalidWrapperChild', + messageId: 'invalidUnresolvableWrapperChild', data: { name: 'EuiFlexGroup', wrapper: 'EuiToolTip' }, }, ], @@ -824,7 +824,7 @@ ruleTester.run( languageOptions, errors: [ { - messageId: 'invalidWrapperChild', + messageId: 'invalidUnresolvableWrapperChild', data: { name: 'EuiText', wrapper: 'EuiToolTip' }, }, ], @@ -842,7 +842,7 @@ ruleTester.run( languageOptions, errors: [ { - messageId: 'invalidWrapperChild', + messageId: 'invalidUnresolvableWrapperChild', data: { name: 'EuiText', wrapper: 'EuiToolTip' }, }, ], @@ -860,7 +860,7 @@ ruleTester.run( `, languageOptions, - errors: [{ messageId: 'invalidChild', data: { name: 'EuiText' } }], + errors: [{ messageId: 'invalidUnresolvableChild', data: { name: 'EuiText' } }], }, { name: ' wrapping invalid element', @@ -884,7 +884,7 @@ ruleTester.run( `, languageOptions, - errors: [{ messageId: 'invalidChild', data: { name: 'EuiText' } }], + errors: [{ messageId: 'invalidUnresolvableChild', data: { name: 'EuiText' } }], }, { name: 'fragment inside EuiToolTip wrapping invalid element', @@ -900,7 +900,7 @@ ruleTester.run( languageOptions, errors: [ { - messageId: 'invalidWrapperChild', + messageId: 'invalidUnresolvableWrapperChild', data: { name: 'EuiText', wrapper: 'EuiToolTip' }, }, ], @@ -916,7 +916,7 @@ ruleTester.run( `, languageOptions, - errors: [{ messageId: 'invalidChild', data: { name: 'EuiText' } }], + errors: [{ messageId: 'invalidUnresolvableChild', data: { name: 'EuiText' } }], }, { name: 'ternary with one invalid branch', @@ -926,7 +926,7 @@ ruleTester.run( `, languageOptions, - errors: [{ messageId: 'invalidChild', data: { name: 'EuiText' } }], + errors: [{ messageId: 'invalidUnresolvableChild', data: { name: 'EuiText' } }], }, { name: 'ternary with both invalid branches reported separately', @@ -937,8 +937,8 @@ ruleTester.run( `, languageOptions, errors: [ - { messageId: 'invalidChild', data: { name: 'EuiText' } }, - { messageId: 'invalidChild', data: { name: 'EuiBadge' } }, + { messageId: 'invalidUnresolvableChild', data: { name: 'EuiText' } }, + { messageId: 'invalidUnresolvableChild', data: { name: 'EuiBadge' } }, ], }, @@ -950,7 +950,7 @@ ruleTester.run( `, languageOptions, - errors: [{ messageId: 'invalidChild', data: { name: 'EuiText' } }], + errors: [{ messageId: 'invalidUnresolvableChild', data: { name: 'EuiText' } }], }, { name: '{unresolvable ?? } invalid right side is flagged', @@ -960,7 +960,7 @@ ruleTester.run( `, languageOptions, - errors: [{ messageId: 'invalidChild', data: { name: 'EuiText' } }], + errors: [{ messageId: 'invalidUnresolvableChild', data: { name: 'EuiText' } }], }, { name: '{resolvedInvalidButton || } resolved invalid left side is flagged', @@ -971,7 +971,7 @@ ruleTester.run( `, languageOptions, - errors: [{ messageId: 'invalidChild', data: { name: 'EuiText' } }], + errors: [{ messageId: 'invalidUnresolvableChild', data: { name: 'EuiText' } }], }, // Unresolvable custom component @@ -1020,7 +1020,7 @@ ruleTester.run( `, languageOptions, - errors: [{ messageId: 'invalidChild', data: { name: 'EuiText' } }], + errors: [{ messageId: 'invalidUnresolvableChild', data: { name: 'EuiText' } }], }, { name: 'const variable holding an array with an invalid element is resolved and reported', @@ -1034,7 +1034,7 @@ ruleTester.run( `, languageOptions, - errors: [{ messageId: 'invalidChild', data: { name: 'EuiText' } }], + errors: [{ messageId: 'invalidUnresolvableChild', data: { name: 'EuiText' } }], }, // Local arrow-function @@ -1047,7 +1047,7 @@ ruleTester.run( `, languageOptions, - errors: [{ messageId: 'invalidChild', data: { name: 'EuiText' } }], + errors: [{ messageId: 'invalidUnresolvableChild', data: { name: 'EuiText' } }], }, { name: 'local block-body arrow-fn component with invalid return is resolved and reported at the invalid element', @@ -1058,7 +1058,7 @@ ruleTester.run( `, languageOptions, - errors: [{ messageId: 'invalidChild', data: { name: 'EuiText' } }], + errors: [{ messageId: 'invalidUnresolvableChild', data: { name: 'EuiText' } }], }, { name: 'local block-body arrow-fn component with invalid branch is resolved and reported', @@ -1072,7 +1072,7 @@ ruleTester.run( `, languageOptions, - errors: [{ messageId: 'invalidChild', data: { name: 'EuiText' } }], + errors: [{ messageId: 'invalidUnresolvableChild', data: { name: 'EuiText' } }], }, { name: 'imported custom component cannot be resolved — uses invalidUnresolvableChild message', @@ -1102,7 +1102,7 @@ ruleTester.run( `, languageOptions, - errors: [{ messageId: 'invalidChild', data: { name: 'EuiText' } }], + errors: [{ messageId: 'invalidUnresolvableChild', data: { name: 'EuiText' } }], }, { name: 'array containing a wrapper (EuiToolTip) with an invalid inner element', @@ -1118,7 +1118,7 @@ ruleTester.run( languageOptions, errors: [ { - messageId: 'invalidWrapperChild', + messageId: 'invalidUnresolvableWrapperChild', data: { name: 'EuiText', wrapper: 'EuiToolTip' }, }, ], @@ -1158,7 +1158,7 @@ ruleTester.run( `, languageOptions, - errors: [{ messageId: 'invalidChild', data: { name: 'EuiText' } }], + errors: [{ messageId: 'invalidUnresolvableChild', data: { name: 'EuiText' } }], }, { name: 'variable holding map() result with invalid element is reported', @@ -1185,7 +1185,7 @@ ruleTester.run( languageOptions, errors: [ { - messageId: 'invalidWrapperChild', + messageId: 'invalidUnresolvableWrapperChild', data: { name: 'EuiText', wrapper: 'EuiToolTip' }, }, ], @@ -1202,7 +1202,7 @@ ruleTester.run( languageOptions, errors: [ { - messageId: 'invalidWrapperChild', + messageId: 'invalidUnresolvableWrapperChild', data: { name: 'EuiText', wrapper: 'EuiCopy' }, }, ], @@ -1223,7 +1223,7 @@ ruleTester.run( `, languageOptions, - errors: [{ messageId: 'invalidChild', data: { name: 'EuiText' } }], + errors: [{ messageId: 'invalidUnresolvableChild', data: { name: 'EuiText' } }], }, ], } diff --git a/packages/eslint-plugin/src/rules/button_group_no_invalid_children.ts b/packages/eslint-plugin/src/rules/button_group_no_invalid_children.ts index eaffb612d67..e4378ac8f15 100644 --- a/packages/eslint-plugin/src/rules/button_group_no_invalid_children.ts +++ b/packages/eslint-plugin/src/rules/button_group_no_invalid_children.ts @@ -25,7 +25,7 @@ const VALID_BUTTONS_LIST = Array.from(VALID_BUTTONS).join(', '); const VALID_WRAPPERS_LIST = Array.from(VALID_WRAPPERS).join(', '); function isCustomComponent(name: string): boolean { - return name[0] === name[0].toUpperCase() && !name.startsWith('Eui'); + return name[0] === name[0].toUpperCase(); } function reportInvalidWrapperChildren< @@ -143,7 +143,9 @@ export const ButtonGroupNoInvalidChildren = ESLintUtils.RuleCreator.withoutDocs( context.report({ node: tooltipChild.openingElement, - messageId: 'invalidPopoverButton', + messageId: isCustomComponent(tooltipChildName) + ? 'invalidUnresolvablePopoverButton' + : 'invalidPopoverButton', data: { name: tooltipChildName }, }); } @@ -151,7 +153,9 @@ export const ButtonGroupNoInvalidChildren = ESLintUtils.RuleCreator.withoutDocs( } context.report({ node: triggerElement.openingElement, - messageId: 'invalidPopoverButton', + messageId: isCustomComponent(triggerElementName) + ? 'invalidUnresolvablePopoverButton' + : 'invalidPopoverButton', data: { name: triggerElementName }, }); } @@ -205,6 +209,14 @@ export const ButtonGroupNoInvalidChildren = ESLintUtils.RuleCreator.withoutDocs( `{{ name }} is not a valid trigger button for EuiPopover in EuiButtonGroup.`, `The \`button\` prop must be ${VALID_BUTTONS_LIST}.`, ].join(' '), + invalidUnresolvablePopoverButton: [ + `{{ name }} cannot be verified as a valid trigger button for EuiPopover in EuiButtonGroup.`, + `The \`button\` prop must be ${VALID_BUTTONS_LIST}.`, + `If {{ name }} is a shared button wrapper component only containing`, + `valid button children, suppress this rule inline with a comment`, + `explaining why it's valid.`, + `// eslint-disable-next-line @elastic/eui/button-group-no-invalid-children -- SaveButton only wraps EuiButton`, + ].join(' '), invalidWrapperChild: [ `{{ name }} inside {{ wrapper }} is not a valid button for EuiButtonGroup.`, `The wrapper must contain ${VALID_BUTTONS_LIST}.`, From a256b3ffd8797cf02947d4fb9a99f49c4c8207dd Mon Sep 17 00:00:00 2001 From: Lene Gadewoll Date: Fri, 31 Jul 2026 16:03:37 +0200 Subject: [PATCH 10/10] refactor: refactor collectJsxChildren to walkJsxChildren - makes the util more generic and universally useable by rules to traverse children; provides means to customize by --- .../a11y/tooltip_no_interactive_content.ts | 81 ++-- .../rules/button_group_no_invalid_children.ts | 19 +- .../rules/callout_prefer_props_for_content.ts | 349 ++++++++---------- ...t_jsx_children.ts => walk_jsx_children.ts} | 189 ++++++---- 4 files changed, 316 insertions(+), 322 deletions(-) rename packages/eslint-plugin/src/utils/{collect_jsx_children.ts => walk_jsx_children.ts} (53%) diff --git a/packages/eslint-plugin/src/rules/a11y/tooltip_no_interactive_content.ts b/packages/eslint-plugin/src/rules/a11y/tooltip_no_interactive_content.ts index a8453575831..877e7220858 100644 --- a/packages/eslint-plugin/src/rules/a11y/tooltip_no_interactive_content.ts +++ b/packages/eslint-plugin/src/rules/a11y/tooltip_no_interactive_content.ts @@ -11,6 +11,8 @@ import { INTERACTIVE_EUI_COMPONENTS, CONDITIONALLY_INTERACTIVE_EUI_COMPONENTS, } from '../../utils/constants'; +import { walkJsxChildren } from '../../utils/walk_jsx_children'; +import { getElementName } from '../../utils/get_element_name'; const TOOLTIP_COMPONENTS = ['EuiToolTip', 'EuiIconTip']; const TOOLTIP_CONTENT_PROPS = ['content', 'title']; @@ -32,43 +34,6 @@ const INTERACTIVE_ELEMENTS = new Set([ ), ]); -function findInteractiveElement( - node: TSESTree.Node -): TSESTree.JSXOpeningElement | null { - if (node.type === 'JSXElement') { - const el = node as TSESTree.JSXElement; - const { name } = el.openingElement; - - if (name.type === 'JSXIdentifier' && INTERACTIVE_ELEMENTS.has(name.name)) { - return el.openingElement; - } - - for (const child of el.children) { - const found = findInteractiveElement(child); - if (found) return found; - } - } else if (node.type === 'JSXFragment') { - for (const child of (node as TSESTree.JSXFragment).children) { - const found = findInteractiveElement(child); - if (found) return found; - } - } else if (node.type === 'JSXExpressionContainer') { - const { expression } = node as TSESTree.JSXExpressionContainer; - if (expression.type !== 'JSXEmptyExpression') { - return findInteractiveElement(expression); - } - } else if (node.type === 'LogicalExpression') { - const { left, right } = node as TSESTree.LogicalExpression; - return findInteractiveElement(left) ?? findInteractiveElement(right); - } else if (node.type === 'ConditionalExpression') { - const { consequent, alternate } = node as TSESTree.ConditionalExpression; - return ( - findInteractiveElement(consequent) ?? findInteractiveElement(alternate) - ); - } - return null; -} - export const TooltipNoInteractiveContent = ESLintUtils.RuleCreator.withoutDocs({ create(context) { return { @@ -101,19 +66,37 @@ export const TooltipNoInteractiveContent = ESLintUtils.RuleCreator.withoutDocs({ continue; } - const interactiveEl = findInteractiveElement(expression); - if (interactiveEl) { - context.report({ - node: interactiveEl, - messageId: 'noInteractiveContent', - data: { - propName: (attr.name as TSESTree.JSXIdentifier).name, - componentName, - elementName: (interactiveEl.name as TSESTree.JSXIdentifier) - .name, + let found = false; + + walkJsxChildren( + expression, + (leaf) => { + if (found || leaf.type !== 'JSXElement') return; + + const el = leaf as TSESTree.JSXElement; + const name = getElementName(el.openingElement); + + if (!name || !INTERACTIVE_ELEMENTS.has(name)) return; + + context.report({ + node: el.openingElement, + messageId: 'noInteractiveContent', + data: { + propName: (attr.name as TSESTree.JSXIdentifier).name, + componentName, + elementName: name, + }, + }); + found = true; + }, + { + shouldSkip: (el) => { + const name = getElementName(el.openingElement); + + return !name || !INTERACTIVE_ELEMENTS.has(name); }, - }); - } + } + ); } }, }; diff --git a/packages/eslint-plugin/src/rules/button_group_no_invalid_children.ts b/packages/eslint-plugin/src/rules/button_group_no_invalid_children.ts index e4378ac8f15..4ae15f271a7 100644 --- a/packages/eslint-plugin/src/rules/button_group_no_invalid_children.ts +++ b/packages/eslint-plugin/src/rules/button_group_no_invalid_children.ts @@ -14,7 +14,7 @@ import { import { hasSpread } from '../utils/has_spread'; import { flatMap } from '../utils/flat_map'; import { getElementName } from '../utils/get_element_name'; -import { collectJsxChildren } from '../utils/collect_jsx_children'; +import { walkJsxChildren } from '../utils/walk_jsx_children'; import { findAttrValue } from '../utils/get_attr_value'; const BUTTON_GROUP = 'EuiButtonGroup'; @@ -28,6 +28,23 @@ function isCustomComponent(name: string): boolean { return name[0] === name[0].toUpperCase(); } +function collectJsxChildren( + node: TSESTree.Node, + sourceCode: TSESLint.SourceCode +): TSESTree.JSXElement[] { + const results: TSESTree.JSXElement[] = []; + walkJsxChildren( + node, + (leaf) => { + if (leaf.type === 'JSXElement') { + results.push(leaf as TSESTree.JSXElement); + } + }, + { sourceCode } + ); + return results; +} + function reportInvalidWrapperChildren< TContext extends TSESLint.RuleContext, >(wrapper: TSESTree.JSXElement, wrapperName: string, context: TContext): void { diff --git a/packages/eslint-plugin/src/rules/callout_prefer_props_for_content.ts b/packages/eslint-plugin/src/rules/callout_prefer_props_for_content.ts index b1ca5f0c6cb..59ff6c025f3 100644 --- a/packages/eslint-plugin/src/rules/callout_prefer_props_for_content.ts +++ b/packages/eslint-plugin/src/rules/callout_prefer_props_for_content.ts @@ -7,8 +7,6 @@ */ import { type TSESTree, ESLintUtils } from '@typescript-eslint/utils'; -import { type RuleContext } from '@typescript-eslint/utils/ts-eslint'; - import { INTERACTIVE_EUI_COMPONENTS, HTML_TEXT_ELEMENTS, @@ -18,6 +16,7 @@ import { I18N_TEXT_COMPONENTS, } from '../utils/constants'; import { getElementName } from '../utils/get_element_name'; +import { walkJsxChildren } from '../utils/walk_jsx_children'; const DEFAULT_COMPONENTS = ['EuiCallOut']; @@ -33,209 +32,159 @@ type MessageIds = | 'childrenHaveActions'; type Options = [{ components?: string[] }]; -/** - * Recursively checks a node for text or action elements that should not be in callout children. - * It unwraps fragments, layout containers, conditional expressions, and logical expressions. - * It does NOT traverse into custom/complex component children. - */ -function checkNode( - node: TSESTree.Node, - context: RuleContext, - componentName: string -): void { - switch (node.type) { - case 'JSXText': { - // Plain string content - if ((node as TSESTree.JSXText).value.trim().length > 0) { - context.report({ node, messageId: 'childrenHavePlainText', data: { componentName } }); - } - break; - } - case 'Literal': { - const { value } = node as TSESTree.Literal; - - if (typeof value === 'string' && value.trim().length > 0) { - context.report({ node, messageId: 'childrenHavePlainText', data: { componentName } }); - } - break; - } - case 'JSXElement': { - const el = node as TSESTree.JSXElement; - const { name } = el.openingElement; - - if ( - name.type === 'JSXMemberExpression' && - name.object.type === 'JSXIdentifier' && - name.object.name === 'React' && - name.property.type === 'JSXIdentifier' && - name.property.name === 'Fragment' - ) { - for (const child of el.children) { - checkNode(child, context, componentName); +export const CallOutPreferPropsForContent = ESLintUtils.RuleCreator.withoutDocs< + Options, + MessageIds +>({ + create(context) { + const components = new Set( + context.options[0]?.components ?? DEFAULT_COMPONENTS + ); + + return { + JSXElement(node) { + const { openingElement, children } = node; + + if ( + openingElement.name.type !== 'JSXIdentifier' || + !components.has(openingElement.name.name) + ) { + return; } - break; - } - const elementName = getElementName(el.openingElement); - - if (!elementName) return; - - if ( - HTML_TEXT_ELEMENTS.has(elementName) || - EUI_TEXT_COMPONENTS.has(elementName) || - I18N_TEXT_COMPONENTS.has(elementName) - ) { - context.report({ - node, - messageId: 'childrenHaveText', - data: { elementName, componentName }, - }); - } else if ( - HTML_ACTION_ELEMENTS.has(elementName) || - EUI_ACTION_COMPONENTS.has(elementName) - ) { - context.report({ - node, - messageId: 'childrenHaveActions', - data: { elementName, componentName }, - }); - } else if (CALLOUT_LAYOUT_CONTAINERS.has(elementName)) { - // Transparent layout wrapper, traverse its children - for (const child of el.children) { - checkNode(child, context, componentName); + const componentName = openingElement.name.name; + + for (const child of children) { + walkJsxChildren( + child, + (leaf) => { + switch (leaf.type) { + case 'JSXText': { + if ((leaf as TSESTree.JSXText).value.trim().length > 0) { + context.report({ + node: leaf, + messageId: 'childrenHavePlainText', + data: { componentName }, + }); + } + break; + } + case 'Literal': { + const { value } = leaf as TSESTree.Literal; + if (typeof value === 'string' && value.trim().length > 0) { + context.report({ + node: leaf, + messageId: 'childrenHavePlainText', + data: { componentName }, + }); + } + break; + } + case 'TemplateLiteral': { + context.report({ + node: leaf, + messageId: 'childrenHavePlainText', + data: { componentName }, + }); + break; + } + case 'MemberExpression': { + const { object } = leaf as TSESTree.MemberExpression; + if (object.type === 'Identifier' && object.name === 'i18n') { + context.report({ + node: leaf, + messageId: 'childrenHavePlainText', + data: { componentName }, + }); + } + break; + } + case 'JSXElement': { + const el = leaf as TSESTree.JSXElement; + const elementName = getElementName(el.openingElement); + if (!elementName) break; + if ( + HTML_TEXT_ELEMENTS.has(elementName) || + EUI_TEXT_COMPONENTS.has(elementName) || + I18N_TEXT_COMPONENTS.has(elementName) + ) { + context.report({ + node: leaf, + messageId: 'childrenHaveText', + data: { elementName, componentName }, + }); + } else if ( + HTML_ACTION_ELEMENTS.has(elementName) || + EUI_ACTION_COMPONENTS.has(elementName) + ) { + context.report({ + node: leaf, + messageId: 'childrenHaveActions', + data: { elementName, componentName }, + }); + } + break; + } + } + }, + { + shouldSkip: (el) => { + const name = getElementName(el.openingElement); + + return !!name && CALLOUT_LAYOUT_CONTAINERS.has(name); + }, + } + ); } - } - - // Custom/complex component, don't traverse its children - break; - } - case 'JSXFragment': { - for (const child of (node as TSESTree.JSXFragment).children) { - checkNode(child, context, componentName); - } - - break; - } - case 'MemberExpression': { - const { object } = node as TSESTree.MemberExpression; - - if (object.type === 'Identifier' && object.name === 'i18n') { - context.report({ node, messageId: 'childrenHavePlainText', data: { componentName } }); - } - break; - } - case 'TemplateLiteral': { - context.report({ node, messageId: 'childrenHavePlainText', data: { componentName } }); - break; - } - case 'ArrayExpression': { - for (const element of (node as TSESTree.ArrayExpression).elements) { - if (element) checkNode(element, context, componentName); - } - break; - } - case 'JSXExpressionContainer': { - const { expression } = node as TSESTree.JSXExpressionContainer; - - if (expression.type !== 'JSXEmptyExpression') { - checkNode(expression, context, componentName); - } - - break; - } - case 'LogicalExpression': { - const { left, right } = node as TSESTree.LogicalExpression; - - checkNode(left, context, componentName); - checkNode(right, context, componentName); - - break; - } - case 'ConditionalExpression': { - // e.g. {condition ?

text

: null} - const { consequent, alternate } = node as TSESTree.ConditionalExpression; - - checkNode(consequent, context, componentName); - checkNode(alternate, context, componentName); - - break; - } - default: - break; - } -} - -export const CallOutPreferPropsForContent = ESLintUtils.RuleCreator.withoutDocs( - { - create(context) { - const components = new Set( - context.options[0]?.components ?? DEFAULT_COMPONENTS - ); - - return { - JSXElement(node) { - const { openingElement, children } = node; - - if ( - openingElement.name.type !== 'JSXIdentifier' || - !components.has(openingElement.name.name) - ) { - return; - } - - for (const child of children) { - checkNode(child, context, openingElement.name.name); - } - }, - }; - }, - meta: { - type: 'suggestion', - docs: { - description: [ - 'Enforce correct usage of `text` and `actionProps` props and discourage using `children` for content.', - 'Text elements should be passed via the `text` prop.', - 'Action elements (buttons, links) should be passed via `actionProps` instead.', - ].join(' '), }, - schema: [ - { - type: 'object', - properties: { - components: { - type: 'array', - items: { type: 'string', minLength: 1 }, - minItems: 1, - }, + }; + }, + meta: { + type: 'suggestion', + docs: { + description: [ + 'Enforce correct usage of `text` and `actionProps` props and discourage using `children` for content.', + 'Text elements should be passed via the `text` prop.', + 'Action elements (buttons, links) should be passed via `actionProps` instead.', + ].join(' '), + }, + schema: [ + { + type: 'object', + properties: { + components: { + type: 'array', + items: { type: 'string', minLength: 1 }, + minItems: 1, }, - additionalProperties: false, }, - ], - messages: { - childrenHavePlainText: [ - 'Plain text passed as `children` of `{{componentName}}` should be moved to the `text` prop instead.', - 'Example:', - ' <{{componentName}} title="Callout title" text="Callout text content" />', - ].join('\n'), - childrenHaveText: [ - '`<{{elementName}}>` passed as `children` of `{{componentName}}` should be moved to the `text` prop instead.', - 'The `text` prop accepts string text, block text elements (e.g. `

`) or inline elements (e.g. ``, ``, ``).', - 'Use `children` only for complex non-text content that cannot be expressed via `text`.', - 'Example:', - ' <{{componentName}} title="Callout title" text={

Callout text content

} />', - ].join('\n'), - childrenHaveActions: [ - '`<{{elementName}}>` passed as `children` of `{{componentName}}` should be moved to the `actionProps` prop instead.', - 'Use `actionProps` to render standardized primary or secondary action buttons, including buttons-as-links via `href`.', - 'If a link is part of inline copy, move the surrounding content to the `text` prop instead.', - 'Example:', - ' <{{componentName}}', - ' title="Callout title"', - ' actionProps={{ primary: { children: "Primary action", onClick: onClick }, secondary: { children: "Secondary action", href: "/"} }}', - ' />', - ].join('\n'), + additionalProperties: false, }, + ], + messages: { + childrenHavePlainText: [ + 'Plain text passed as `children` of `{{componentName}}` should be moved to the `text` prop instead.', + 'Example:', + ' <{{componentName}} title="Callout title" text="Callout text content" />', + ].join('\n'), + childrenHaveText: [ + '`<{{elementName}}>` passed as `children` of `{{componentName}}` should be moved to the `text` prop instead.', + 'The `text` prop accepts string text, block text elements (e.g. `

`) or inline elements (e.g. ``, ``, ``).', + 'Use `children` only for complex non-text content that cannot be expressed via `text`.', + 'Example:', + ' <{{componentName}} title="Callout title" text={

Callout text content

} />', + ].join('\n'), + childrenHaveActions: [ + '`<{{elementName}}>` passed as `children` of `{{componentName}}` should be moved to the `actionProps` prop instead.', + 'Use `actionProps` to render standardized primary or secondary action buttons, including buttons-as-links via `href`.', + 'If a link is part of inline copy, move the surrounding content to the `text` prop instead.', + 'Example:', + ' <{{componentName}}', + ' title="Callout title"', + ' actionProps={{ primary: { children: "Primary action", onClick: onClick }, secondary: { children: "Secondary action", href: "/"} }}', + ' />', + ].join('\n'), }, - defaultOptions: [{}], - } -); + }, + defaultOptions: [{}], +}); diff --git a/packages/eslint-plugin/src/utils/collect_jsx_children.ts b/packages/eslint-plugin/src/utils/walk_jsx_children.ts similarity index 53% rename from packages/eslint-plugin/src/utils/collect_jsx_children.ts rename to packages/eslint-plugin/src/utils/walk_jsx_children.ts index 471659b8fae..0a40a32a0e4 100644 --- a/packages/eslint-plugin/src/utils/collect_jsx_children.ts +++ b/packages/eslint-plugin/src/utils/walk_jsx_children.ts @@ -22,7 +22,7 @@ function isReassigned(variable: TSESLint.Scope.Variable): boolean { * Returns null for anything that can't be resolved safely: imports, function * parameters, reassigned variables, or missing initializers. */ -export function resolveIdentifierValue( +function resolveIdentifierValue( sourceCode: TSESLint.SourceCode, node: TSESTree.Identifier ): TSESTree.Node | null { @@ -44,12 +44,10 @@ export function resolveIdentifierValue( /** * Collects all return-statement arguments within a block body, recursing into * `if`/`else` branches and `switch` cases. Does not recurse into nested - * functions — those have their own return scope. Used to validate block-body + * functions - those have their own return scope. Used to validate block-body * arrow functions. */ -export function collectReturnValues( - node: TSESTree.Node -): TSESTree.Expression[] { +function collectReturnValues(node: TSESTree.Node): TSESTree.Expression[] { switch (node.type) { case 'BlockStatement': return flatMap(node.body, collectReturnValues); @@ -77,7 +75,7 @@ export function collectReturnValues( * (`() => { return ; }`) arrow functions. Imports, class components, * and reassigned variables return null — those stay opaque. */ -export function resolveLocalComponent( +function resolveLocalComponent( sourceCode: TSESLint.SourceCode, name: string, refNode: TSESTree.Node @@ -100,83 +98,80 @@ export function resolveLocalComponent( return null; } -/** - * Recursively collects concrete JSXElement nodes to validate, expanding: - * - `<>...`, ``, `` (transparent grouping) - * - `{expr}` (JSXExpressionContainer) - * - `{a && }` (LogicalExpression `&&` → right side only) - * - `{a || }`, `{a ?? }` (LogicalExpression `||`/`??` → both sides) - * - `{c ? : }` (ConditionalExpression → both branches) - * - `{[, ]}` (ArrayExpression → each element) - * - `{variable}` (Identifier → resolved via scope) - * - `` (local arrow-fn component, expression or block body) - * - `{(arg) => }` (ArrowFunctionExpression, expression or block body) - * - `{arr.map(fn)}` (CallExpression `.map()` with arrow-fn callback) - * - * Patterns that can't be resolved statically (arbitrary function calls, imported - * variables, non-arrow `.map()` callbacks) produce an empty list and are silently - * skipped. - */ -export function collectJsxChildren( +function walkJsxChild( node: TSESTree.Node, - sourceCode: TSESLint.SourceCode -): TSESTree.JSXElement[] { - return collectChildren(node, sourceCode, new Set()); -} - -function collectChildren( - node: TSESTree.Node, - sourceCode: TSESLint.SourceCode, - visited: Set -): TSESTree.JSXElement[] { - const collect = (n: TSESTree.Node) => collectChildren(n, sourceCode, visited); + visit: (node: TSESTree.Node) => void, + sourceCode: TSESLint.SourceCode | undefined, + visited: Set, + shouldSkip: ((el: TSESTree.JSXElement) => boolean) | undefined +): void { + const walk = (n: TSESTree.Node) => + walkJsxChild(n, visit, sourceCode, visited, shouldSkip); switch (node.type) { case 'JSXElement': { if (isFragment(node.openingElement)) { - return flatMap(node.children, collect); + node.children.forEach(walk); + return; } - const { name } = node.openingElement; - if (name.type === 'JSXIdentifier') { - if (visited.has(name.name)) return []; - - const resolved = resolveLocalComponent( - sourceCode, - name.name, - node.openingElement - ); - if (resolved !== null) { - visited.add(name.name); - - const result = flatMap(resolved, collect); - - visited.delete(name.name); - return result; + if (sourceCode) { + const { name } = node.openingElement; + if (name.type === 'JSXIdentifier' && !visited.has(name.name)) { + const resolved = resolveLocalComponent( + sourceCode, + name.name, + node.openingElement + ); + if (resolved !== null) { + visited.add(name.name); + resolved.forEach(walk); + visited.delete(name.name); + return; + } } } - return [node]; + if (shouldSkip?.(node)) { + node.children.forEach(walk); + return; + } + visit(node); + return; } case 'JSXFragment': - return flatMap(node.children, collect); + node.children.forEach(walk); + return; case 'JSXExpressionContainer': - if (node.expression.type === 'JSXEmptyExpression') return []; - return collect(node.expression); + if (node.expression.type !== 'JSXEmptyExpression') walk(node.expression); + return; case 'LogicalExpression': - if (node.operator === '&&') return collect(node.right); - return [...collect(node.left), ...collect(node.right)]; + walk(node.left); + walk(node.right); + return; case 'ConditionalExpression': - return [...collect(node.consequent), ...collect(node.alternate)]; + walk(node.consequent); + walk(node.alternate); + return; case 'ArrayExpression': - return flatMap(node.elements, (el) => - el && el.type !== 'SpreadElement' ? collect(el) : [] - ); - case 'Identifier': { - const resolved = resolveIdentifierValue(sourceCode, node); - return resolved ? collect(resolved) : []; - } + node.elements.forEach((el) => { + if (el && el.type !== 'SpreadElement') walk(el); + }); + return; + case 'Identifier': + if (sourceCode) { + const resolved = resolveIdentifierValue(sourceCode, node); + if (resolved) { + walk(resolved); + return; + } + } + return; case 'ArrowFunctionExpression': - if (node.body.type !== 'BlockStatement') return collect(node.body); - return flatMap(collectReturnValues(node.body), collect); + if (node.body.type !== 'BlockStatement') { + walk(node.body); + } else { + collectReturnValues(node.body).forEach(walk); + } + return; case 'CallExpression': { const { callee, arguments: args } = node; if ( @@ -187,12 +182,62 @@ function collectChildren( args[0].type === 'ArrowFunctionExpression' ) { const fn = args[0]; - if (fn.body.type !== 'BlockStatement') return collect(fn.body); - return flatMap(collectReturnValues(fn.body), collect); + if (fn.body.type !== 'BlockStatement') { + walk(fn.body); + } else { + collectReturnValues(fn.body).forEach(walk); + } } - return []; + return; } + case 'JSXText': + case 'Literal': + case 'TemplateLiteral': + case 'MemberExpression': + visit(node); + return; default: - return []; + return; } } + +/** + * Recursively walks JSX children, expanding structural nodes transparently + * and calling `visit` for each content node. + * + * Always expanded (structural nodes): + * - `<>...`, ``, `` (transparent grouping) + * - `{expr}` (JSXExpressionContainer) + * - `{a && b}`, `{a || b}`, `{a ?? b}` (LogicalExpression → both sides) + * - `{c ? a : b}` (ConditionalExpression → both branches) + * - `{[a, b]}` (ArrayExpression → each element) + * - `{(arg) => }` (ArrowFunctionExpression) + * - `{arr.map(fn)}` (CallExpression `.map()` with arrow-fn callback) + * + * With `options.sourceCode`: Identifiers and local arrow-fn components are + * also expanded via scope lookup when resolvable. + * + * `options.shouldSkip` - when provided, called for each JSXElement reached + * (after fragment and local-component checks). Returning true skips the + * element itself and recurses into its children instead. Use this to make + * layout wrappers or non-interactive shells transparent to the visitor. + * + * `visit` is called for leaf nodes: JSXElement, JSXText, Literal, + * TemplateLiteral, MemberExpression. + */ +export function walkJsxChildren( + node: TSESTree.Node, + visit: (node: TSESTree.Node) => void, + options: { + sourceCode?: TSESLint.SourceCode; + shouldSkip?: (el: TSESTree.JSXElement) => boolean; + } = {} +): void { + walkJsxChild( + node, + visit, + options.sourceCode, + new Set(), + options.shouldSkip + ); +}