From 2335d8ecec34c09429154b30b4e69dd43ef953fb Mon Sep 17 00:00:00 2001 From: rdlabo Date: Sat, 22 Aug 2026 03:03:36 +0900 Subject: [PATCH 1/6] Require grouped Ionic list items --- README.md | 2 + docs/rules.md | 3 +- docs/rules/require-ion-item-group.md | 72 ++++++++ scripts/lib/recommended-rule-names.ts | 1 + scripts/lib/update-lib-configs-recommended.ts | 1 + src/configs/recommended.ts | 1 + src/index.ts | 2 + src/rules/require-ion-item-group.ts | 77 +++++++++ tests/rules/require-ion-item-group.ts | 154 ++++++++++++++++++ 9 files changed, 312 insertions(+), 1 deletion(-) create mode 100644 docs/rules/require-ion-item-group.md create mode 100644 src/rules/require-ion-item-group.ts create mode 100644 tests/rules/require-ion-item-group.ts diff --git a/README.md b/README.md index 98b42f8..9a29053 100644 --- a/README.md +++ b/README.md @@ -32,6 +32,8 @@ The package root provides Angular and Ionic rules. Install `@angular-eslint/temp The recommended preset is designed for ESLint Flat Config. Add it at the top level so its TypeScript and HTML file selectors remain intact. +For Ionic templates, the preset also requires `ion-item` elements inside `ion-list` to use `ion-item-group`, `ion-reorder-group`, `ion-accordion-group`, or `ion-radio-group`, matching the iOS 26 and Material Design 3 list structure. + ## Next step Continue to [Configuration](./docs/configuration.md) to enable the recommended preset or individual rules. diff --git a/docs/rules.md b/docs/rules.md index edc0e64..9a2b2f6 100644 --- a/docs/rules.md +++ b/docs/rules.md @@ -1,4 +1,4 @@ -The package exposes 18 rules. Rules marked “recommended” are enabled by `rdlabo.configs.recommended`; the remaining rules are opt-in. +The package exposes 19 rules. Rules marked “recommended” are enabled by `rdlabo.configs.recommended`; the remaining rules are opt-in. | Rule | Purpose | Fix | Preset | | ----------------------------------------------------------------------------------------- | ---------------------------------------------------------------------------------- | :-: | :----: | @@ -16,6 +16,7 @@ The package exposes 18 rules. Rules marked “recommended” are enabled by `rdl | [`prefer-disable-handler`](./rules/prefer-disable-handler.md) | Wrap configured event handlers to prevent duplicate async actions. | No | Yes | | [`prefer-ionic-standalone`](./rules/prefer-ionic-standalone.md) | Prefer Ionic 9 standalone imports and disallow `IonicModule`. | Yes | Yes | | [`prefer-modal-launcher`](./rules/prefer-modal-launcher.md) | Restrict `presentModal` calls to `launch*` functions. | No | Yes | +| [`require-ion-item-group`](./rules/require-ion-item-group.md) | Require grouped Ionic list items for iOS 26 and Material Design 3. | No | Yes | | [`require-viewmodel`](./rules/require-viewmodel.md) | Enforce component ownership and the `ViewModelStore` boundary. | No | Yes | | [`restrict-try-block`](./rules/restrict-try-block.md) | Keep `try` blocks small and exclude Promise, RxJS, and Signal contexts by policy. | No | Yes | | [`signal-use-as-signal-template`](./rules/signal-use-as-signal-template.md) | Require `()` when reading Angular Signals in templates. | No | Yes | diff --git a/docs/rules/require-ion-item-group.md b/docs/rules/require-ion-item-group.md new file mode 100644 index 0000000..ce3363d --- /dev/null +++ b/docs/rules/require-ion-item-group.md @@ -0,0 +1,72 @@ +# @rdlabo/rules/require-ion-item-group + +> Require ion-item elements in ion-list to be wrapped by a supported Ionic item group. +> +> - ⭐️ This rule is included in `plugin:@rdlabo/rules/recommended` preset. + +Ionic's iOS 26 and Material Design 3 list styling expects list items to be organized through the group component that matches their behavior. This rule prevents a bare `ion-item` from being rendered directly under `ion-list`. + +## Rule Details + +An `ion-item` within `ion-list` must use exactly one of these structures: + +- `ion-list > ion-item-group > ion-item` +- `ion-list > ion-reorder-group > ion-item` +- `ion-list > ion-accordion-group > ion-item` +- `ion-list > ion-radio-group > ion-item` + +Angular control-flow blocks such as `@if`, `@for`, and `@switch` are transparent for this structural check because they do not render an element. Other HTML or Angular elements are not transparent: inserting a `div` between the list, group, or item is reported. + +The rule only checks `ion-item` elements contained by `ion-list`. An `ion-item` outside a list is not reported, and `.spec.html` files are ignored. + +## Examples + +### Incorrect + +```html + + Direct item + +``` + +```html + + @for (item of items; track item.id) { + {{ item.name }} + } + +``` + +### Correct + +```html + + + @for (item of items; track item.id) { + {{ item.name }} + } + + +``` + +```html + + + First choice + Second choice + + +``` + +## Options + +This rule has no options. + +## When to enable + +Enable this rule in Ionic Angular applications that target the iOS 26 and Material Design 3 list designs. It is included in the recommended preset and has no effect when a template does not contain an `ion-item` within `ion-list`. + +## Implementation + +- [Rule source](../../src/rules/require-ion-item-group.ts) +- [Test source](../../tests/rules/require-ion-item-group.ts) diff --git a/scripts/lib/recommended-rule-names.ts b/scripts/lib/recommended-rule-names.ts index ac82dd1..19ec554 100644 --- a/scripts/lib/recommended-rule-names.ts +++ b/scripts/lib/recommended-rule-names.ts @@ -14,4 +14,5 @@ export const RECOMMENDED_RULE_NAMES = new Set([ 'ionic-attr-type-check', 'deny-element', 'prefer-disable-handler', + 'require-ion-item-group', ]); diff --git a/scripts/lib/update-lib-configs-recommended.ts b/scripts/lib/update-lib-configs-recommended.ts index 2283349..aa4c265 100644 --- a/scripts/lib/update-lib-configs-recommended.ts +++ b/scripts/lib/update-lib-configs-recommended.ts @@ -48,6 +48,7 @@ const recommended: Linter.Config[] = [ }, ], '@rdlabo/rules/prefer-disable-handler': 'error', + '@rdlabo/rules/require-ion-item-group': 'error', }, }, ]; diff --git a/src/configs/recommended.ts b/src/configs/recommended.ts index 05407a6..456ea9f 100644 --- a/src/configs/recommended.ts +++ b/src/configs/recommended.ts @@ -37,6 +37,7 @@ const recommended: Linter.Config[] = [ }, ], '@rdlabo/rules/prefer-disable-handler': 'error', + '@rdlabo/rules/require-ion-item-group': 'error', }, }, ]; diff --git a/src/index.ts b/src/index.ts index 1b260c5..ef1399f 100644 --- a/src/index.ts +++ b/src/index.ts @@ -15,6 +15,7 @@ import notemplatedrivenforms from './rules/no-template-driven-forms'; import preferdisablehandler from './rules/prefer-disable-handler'; import preferionicstandalone from './rules/prefer-ionic-standalone'; import prefermodallauncher from './rules/prefer-modal-launcher'; +import requireionitemgroup from './rules/require-ion-item-group'; import requireviewmodel from './rules/require-viewmodel'; import restricttryblock from './rules/restrict-try-block'; import signaluseassignaltemplate from './rules/signal-use-as-signal-template'; @@ -39,6 +40,7 @@ export = { 'prefer-disable-handler': preferdisablehandler, 'prefer-ionic-standalone': preferionicstandalone, 'prefer-modal-launcher': prefermodallauncher, + 'require-ion-item-group': requireionitemgroup, 'require-viewmodel': requireviewmodel, 'restrict-try-block': restricttryblock, 'signal-use-as-signal-template': signaluseassignaltemplate, diff --git a/src/rules/require-ion-item-group.ts b/src/rules/require-ion-item-group.ts new file mode 100644 index 0000000..c2b4ba4 --- /dev/null +++ b/src/rules/require-ion-item-group.ts @@ -0,0 +1,77 @@ +import { TSESLint } from '@typescript-eslint/utils'; +import type { TSESTree } from '@typescript-eslint/utils'; + +interface TemplateNode { + name?: string; + type: string; + loc?: TSESTree.SourceLocation; + children?: TemplateNode[]; + branches?: TemplateNode[]; + cases?: TemplateNode[]; + groups?: TemplateNode[]; + then?: { children?: TemplateNode[] }; + else?: { children?: TemplateNode[] }; +} + +const ITEM_GROUPS = new Set(['ion-item-group', 'ion-reorder-group', 'ion-accordion-group', 'ion-radio-group']); + +const rule: TSESLint.RuleModule<'requireIonItemGroup', []> = { + defaultOptions: [], + meta: { + docs: { + description: 'Require ion-item elements in ion-list to be wrapped by a supported Ionic item group.', + url: '', + }, + messages: { + requireIonItemGroup: 'ion-item inside ion-list must be wrapped by ion-item-group, ion-reorder-group, ion-accordion-group, or ion-radio-group.', + }, + schema: [], + type: 'problem', + }, + create(context) { + const isHtmlFile = (filename: string) => filename.includes('.html') && !filename.includes('.spec'); + const isElement = (node: TemplateNode) => node.type.includes('Element'); + + function visit(nodes: TemplateNode[] | undefined, ancestors: string[]): void { + for (const node of nodes ?? []) { + const nextAncestors = isElement(node) && node.name ? [...ancestors, node.name] : ancestors; + + if (isElement(node) && node.name === 'ion-item') { + const nearestListIndex = ancestors.lastIndexOf('ion-list'); + if (nearestListIndex >= 0) { + const elementsAfterList = ancestors.slice(nearestListIndex + 1); + const hasRequiredStructure = elementsAfterList.length === 1 && ITEM_GROUPS.has(elementsAfterList[0]); + + if (!hasRequiredStructure) { + context.report({ + node: node as unknown as TSESTree.Node, + loc: node.loc, + messageId: 'requireIonItemGroup', + }); + } + } + } + + visit(node.children, nextAncestors); + visit(node.branches, nextAncestors); + visit(node.cases, nextAncestors); + visit(node.groups, nextAncestors); + visit(node.then?.children, nextAncestors); + visit(node.else?.children, nextAncestors); + } + } + + return { + Program(node) { + if (!isHtmlFile(context.filename)) { + return; + } + + const templateNodes = (node as unknown as { templateNodes?: TemplateNode[] }).templateNodes; + visit(templateNodes, []); + }, + }; + }, +}; + +export = rule; diff --git a/tests/rules/require-ion-item-group.ts b/tests/rules/require-ion-item-group.ts new file mode 100644 index 0000000..372c661 --- /dev/null +++ b/tests/rules/require-ion-item-group.ts @@ -0,0 +1,154 @@ +import { RuleTester } from '@angular-eslint/test-utils'; +import rule from '../../src/rules/require-ion-item-group'; + +const ruleTester = new RuleTester({ + languageOptions: { + // eslint-disable-next-line @typescript-eslint/no-require-imports + parser: require('@angular-eslint/template-parser'), + }, +}); + +ruleTester.run('require-ion-item-group', rule, { + valid: [ + { code: '', filename: 'template.html' }, + { code: '', filename: 'template.html' }, + { + code: '', + filename: 'template.html', + }, + { + code: '', + filename: 'template.html', + }, + { + code: '', + filename: 'template.html', + }, + { + code: '', + filename: 'template.html', + }, + { + code: ` + + + @for (item of items; track item.id) { + {{ item.name }} + } + + + `, + filename: 'template.html', + }, + { + code: ` + + + @switch (selected) { + @case ('first') { First } + @default { Default } + } + + + `, + filename: 'template.html', + }, + { + code: ` + + @if (grouped) { + + @if (visible) { + Choice + } + + } + + `, + filename: 'template.html', + }, + { + code: '', + filename: 'template.spec.html', + }, + ], + invalid: [ + { + code: '', + filename: 'template.html', + errors: [{ messageId: 'requireIonItemGroup' }], + }, + { + code: '
', + filename: 'template.html', + errors: [{ messageId: 'requireIonItemGroup' }], + }, + { + code: '
', + filename: 'template.html', + errors: [{ messageId: 'requireIonItemGroup' }], + }, + { + code: '', + filename: 'template.html', + errors: [{ messageId: 'requireIonItemGroup' }], + }, + { + code: ` + + @for (item of items; track item.id) { + {{ item.name }} + } + + `, + filename: 'template.html', + errors: [{ messageId: 'requireIonItemGroup', line: 4 }], + }, + { + code: ` + + @if (visible) { + First + } @else { + Second + } + + `, + filename: 'template.html', + errors: [ + { messageId: 'requireIonItemGroup', line: 4 }, + { messageId: 'requireIonItemGroup', line: 6 }, + ], + }, + { + code: ` + + @switch (selected) { + @case ('first') { First } + @default { Default } + } + + `, + filename: 'template.html', + errors: [ + { messageId: 'requireIonItemGroup', line: 4 }, + { messageId: 'requireIonItemGroup', line: 5 }, + ], + }, + { + code: ` + + Outer + + Inner + + + `, + filename: 'template.html', + errors: [ + { messageId: 'requireIonItemGroup', line: 3 }, + { messageId: 'requireIonItemGroup', line: 5 }, + ], + }, + ], +}); From a73fedfc44f0de37c0f2d7bf4f74898fb2050db3 Mon Sep 17 00:00:00 2001 From: rdlabo Date: Sat, 22 Aug 2026 12:17:07 +0900 Subject: [PATCH 2/6] fix(rule): handle Angular blocks and accordions --- docs/rules/require-ion-item-group.md | 4 +- src/rules/require-ion-item-group.ts | 24 +++++++++--- tests/rules/require-ion-item-group.ts | 53 +++++++++++++++++++++++++-- 3 files changed, 70 insertions(+), 11 deletions(-) diff --git a/docs/rules/require-ion-item-group.md b/docs/rules/require-ion-item-group.md index ce3363d..56bde83 100644 --- a/docs/rules/require-ion-item-group.md +++ b/docs/rules/require-ion-item-group.md @@ -12,10 +12,10 @@ An `ion-item` within `ion-list` must use exactly one of these structures: - `ion-list > ion-item-group > ion-item` - `ion-list > ion-reorder-group > ion-item` -- `ion-list > ion-accordion-group > ion-item` +- `ion-list > ion-accordion-group > ion-accordion > ion-item` - `ion-list > ion-radio-group > ion-item` -Angular control-flow blocks such as `@if`, `@for`, and `@switch` are transparent for this structural check because they do not render an element. Other HTML or Angular elements are not transparent: inserting a `div` between the list, group, or item is reported. +Angular control-flow blocks such as `@if`, `@for`, `@empty`, `@switch`, and `@defer` are transparent for this structural check because they do not render an element. `ng-container` is also transparent. Other HTML or Angular elements are not transparent: inserting a `div` between the list, group, or item is reported. The rule only checks `ion-item` elements contained by `ion-list`. An `ion-item` outside a list is not reported, and `.spec.html` files are ignored. diff --git a/src/rules/require-ion-item-group.ts b/src/rules/require-ion-item-group.ts index c2b4ba4..71da5ff 100644 --- a/src/rules/require-ion-item-group.ts +++ b/src/rules/require-ion-item-group.ts @@ -11,9 +11,13 @@ interface TemplateNode { groups?: TemplateNode[]; then?: { children?: TemplateNode[] }; else?: { children?: TemplateNode[] }; + empty?: { children?: TemplateNode[] }; + placeholder?: { children?: TemplateNode[] }; + loading?: { children?: TemplateNode[] }; + error?: { children?: TemplateNode[] }; } -const ITEM_GROUPS = new Set(['ion-item-group', 'ion-reorder-group', 'ion-accordion-group', 'ion-radio-group']); +const DIRECT_ITEM_GROUPS = new Set(['ion-item-group', 'ion-reorder-group', 'ion-radio-group']); const rule: TSESLint.RuleModule<'requireIonItemGroup', []> = { defaultOptions: [], @@ -23,24 +27,28 @@ const rule: TSESLint.RuleModule<'requireIonItemGroup', []> = { url: '', }, messages: { - requireIonItemGroup: 'ion-item inside ion-list must be wrapped by ion-item-group, ion-reorder-group, ion-accordion-group, or ion-radio-group.', + requireIonItemGroup: + 'ion-item inside ion-list must be wrapped by ion-item-group, ion-reorder-group, ion-radio-group, or ion-accordion within ion-accordion-group.', }, schema: [], type: 'problem', }, create(context) { const isHtmlFile = (filename: string) => filename.includes('.html') && !filename.includes('.spec'); - const isElement = (node: TemplateNode) => node.type.includes('Element'); + const isRenderedElement = (node: TemplateNode) => node.type.includes('Element') && node.name !== 'ng-container'; function visit(nodes: TemplateNode[] | undefined, ancestors: string[]): void { for (const node of nodes ?? []) { - const nextAncestors = isElement(node) && node.name ? [...ancestors, node.name] : ancestors; + const nextAncestors = isRenderedElement(node) && node.name ? [...ancestors, node.name] : ancestors; - if (isElement(node) && node.name === 'ion-item') { + if (isRenderedElement(node) && node.name === 'ion-item') { const nearestListIndex = ancestors.lastIndexOf('ion-list'); if (nearestListIndex >= 0) { const elementsAfterList = ancestors.slice(nearestListIndex + 1); - const hasRequiredStructure = elementsAfterList.length === 1 && ITEM_GROUPS.has(elementsAfterList[0]); + const hasDirectItemGroup = elementsAfterList.length === 1 && DIRECT_ITEM_GROUPS.has(elementsAfterList[0]); + const hasAccordionGroup = + elementsAfterList.length === 2 && elementsAfterList[0] === 'ion-accordion-group' && elementsAfterList[1] === 'ion-accordion'; + const hasRequiredStructure = hasDirectItemGroup || hasAccordionGroup; if (!hasRequiredStructure) { context.report({ @@ -58,6 +66,10 @@ const rule: TSESLint.RuleModule<'requireIonItemGroup', []> = { visit(node.groups, nextAncestors); visit(node.then?.children, nextAncestors); visit(node.else?.children, nextAncestors); + visit(node.empty?.children, nextAncestors); + visit(node.placeholder?.children, nextAncestors); + visit(node.loading?.children, nextAncestors); + visit(node.error?.children, nextAncestors); } } diff --git a/tests/rules/require-ion-item-group.ts b/tests/rules/require-ion-item-group.ts index 372c661..a591451 100644 --- a/tests/rules/require-ion-item-group.ts +++ b/tests/rules/require-ion-item-group.ts @@ -21,7 +21,7 @@ ruleTester.run('require-ion-item-group', rule, { filename: 'template.html', }, { - code: '', + code: '', filename: 'template.html', }, { @@ -45,8 +45,12 @@ ruleTester.run('require-ion-item-group', rule, { @switch (selected) { - @case ('first') { First } - @default { Default } + @case ('first') { + First + } + @default { + Default + } } @@ -71,6 +75,10 @@ ruleTester.run('require-ion-item-group', rule, { code: '', filename: 'template.spec.html', }, + { + code: '', + filename: 'template.html', + }, ], invalid: [ { @@ -93,6 +101,11 @@ ruleTester.run('require-ion-item-group', rule, { filename: 'template.html', errors: [{ messageId: 'requireIonItemGroup' }], }, + { + code: '', + filename: 'template.html', + errors: [{ messageId: 'requireIonItemGroup' }], + }, { code: ` @@ -120,6 +133,40 @@ ruleTester.run('require-ion-item-group', rule, { { messageId: 'requireIonItemGroup', line: 6 }, ], }, + { + code: ` + + @for (item of items; track item.id) { + {{ item.name }} + } @empty { + Empty + } + + `, + filename: 'template.html', + errors: [{ messageId: 'requireIonItemGroup', line: 6 }], + }, + { + code: ` + + @defer (when ready) { + Ready + } @placeholder { + Placeholder + } @loading { + Loading + } @error { + Error + } + + `, + filename: 'template.html', + errors: [ + { messageId: 'requireIonItemGroup', line: 6 }, + { messageId: 'requireIonItemGroup', line: 8 }, + { messageId: 'requireIonItemGroup', line: 10 }, + ], + }, { code: ` From 2ddb6c37b164f7a0ac1be47b4e7a07700b6f6281 Mon Sep 17 00:00:00 2001 From: rdlabo Date: Sat, 22 Aug 2026 12:28:50 +0900 Subject: [PATCH 3/6] docs(rule): clarify supported template structures --- README.md | 2 +- docs/rules/require-ion-item-group.md | 8 +++++--- tests/rules/require-ion-item-group.ts | 4 ++++ 3 files changed, 10 insertions(+), 4 deletions(-) diff --git a/README.md b/README.md index 9a29053..765133d 100644 --- a/README.md +++ b/README.md @@ -32,7 +32,7 @@ The package root provides Angular and Ionic rules. Install `@angular-eslint/temp The recommended preset is designed for ESLint Flat Config. Add it at the top level so its TypeScript and HTML file selectors remain intact. -For Ionic templates, the preset also requires `ion-item` elements inside `ion-list` to use `ion-item-group`, `ion-reorder-group`, `ion-accordion-group`, or `ion-radio-group`, matching the iOS 26 and Material Design 3 list structure. +For Ionic templates, the preset also requires `ion-item` elements inside `ion-list` to use `ion-item-group`, `ion-reorder-group`, `ion-radio-group`, or `ion-accordion` within `ion-accordion-group`, matching the iOS 26 and Material Design 3 list structure. ## Next step diff --git a/docs/rules/require-ion-item-group.md b/docs/rules/require-ion-item-group.md index 56bde83..53c375f 100644 --- a/docs/rules/require-ion-item-group.md +++ b/docs/rules/require-ion-item-group.md @@ -15,7 +15,7 @@ An `ion-item` within `ion-list` must use exactly one of these structures: - `ion-list > ion-accordion-group > ion-accordion > ion-item` - `ion-list > ion-radio-group > ion-item` -Angular control-flow blocks such as `@if`, `@for`, `@empty`, `@switch`, and `@defer` are transparent for this structural check because they do not render an element. `ng-container` is also transparent. Other HTML or Angular elements are not transparent: inserting a `div` between the list, group, or item is reported. +Angular control-flow blocks such as `@if`, `@for`, `@empty`, `@switch`, and `@defer` are transparent for this structural check because they do not render an element. `ng-container` and `ng-template` are also transparent. Rendered HTML or Angular elements are not transparent: inserting a `div` between the list, group, or item is reported. The rule only checks `ion-item` elements contained by `ion-list`. An `ion-item` outside a list is not reported, and `.spec.html` files are ignored. @@ -29,21 +29,23 @@ The rule only checks `ion-item` elements contained by `ion-list`. An `ion-item` ``` + ```html @for (item of items; track item.id) { - {{ item.name }} + {{ item.name }} } ``` ### Correct + ```html @for (item of items; track item.id) { - {{ item.name }} + {{ item.name }} } diff --git a/tests/rules/require-ion-item-group.ts b/tests/rules/require-ion-item-group.ts index a591451..fb426dd 100644 --- a/tests/rules/require-ion-item-group.ts +++ b/tests/rules/require-ion-item-group.ts @@ -79,6 +79,10 @@ ruleTester.run('require-ion-item-group', rule, { code: '', filename: 'template.html', }, + { + code: '', + filename: 'template.html', + }, ], invalid: [ { From 7b9477c4d82d9b5c0b6529095328fb7bba2e01ea Mon Sep 17 00:00:00 2001 From: rdlabo Date: Sat, 22 Aug 2026 12:55:23 +0900 Subject: [PATCH 4/6] feat(rule): autofix ungrouped Ionic list items --- docs/rules.md | 2 +- docs/rules/require-ion-item-group.md | 9 + src/rules/require-ion-item-group.ts | 147 +++++++++++++-- tests/rules/require-ion-item-group.ts | 250 ++++++++++++++++++++++++-- 4 files changed, 376 insertions(+), 32 deletions(-) diff --git a/docs/rules.md b/docs/rules.md index 9a2b2f6..c7e5cca 100644 --- a/docs/rules.md +++ b/docs/rules.md @@ -16,7 +16,7 @@ The package exposes 19 rules. Rules marked “recommended” are enabled by `rdl | [`prefer-disable-handler`](./rules/prefer-disable-handler.md) | Wrap configured event handlers to prevent duplicate async actions. | No | Yes | | [`prefer-ionic-standalone`](./rules/prefer-ionic-standalone.md) | Prefer Ionic 9 standalone imports and disallow `IonicModule`. | Yes | Yes | | [`prefer-modal-launcher`](./rules/prefer-modal-launcher.md) | Restrict `presentModal` calls to `launch*` functions. | No | Yes | -| [`require-ion-item-group`](./rules/require-ion-item-group.md) | Require grouped Ionic list items for iOS 26 and Material Design 3. | No | Yes | +| [`require-ion-item-group`](./rules/require-ion-item-group.md) | Require grouped Ionic list items for iOS 26 and Material Design 3. | Yes | Yes | | [`require-viewmodel`](./rules/require-viewmodel.md) | Enforce component ownership and the `ViewModelStore` boundary. | No | Yes | | [`restrict-try-block`](./rules/restrict-try-block.md) | Keep `try` blocks small and exclude Promise, RxJS, and Signal contexts by policy. | No | Yes | | [`signal-use-as-signal-template`](./rules/signal-use-as-signal-template.md) | Require `()` when reading Angular Signals in templates. | No | Yes | diff --git a/docs/rules/require-ion-item-group.md b/docs/rules/require-ion-item-group.md index 53c375f..3f53257 100644 --- a/docs/rules/require-ion-item-group.md +++ b/docs/rules/require-ion-item-group.md @@ -3,6 +3,7 @@ > Require ion-item elements in ion-list to be wrapped by a supported Ionic item group. > > - ⭐️ This rule is included in `plugin:@rdlabo/rules/recommended` preset. +> - ✒️ The `--fix` option on the [command line](https://eslint.org/docs/user-guide/command-line-interface#fixing-problems) can automatically fix some of the problems reported by this rule. Ionic's iOS 26 and Material Design 3 list styling expects list items to be organized through the group component that matches their behavior. This rule prevents a bare `ion-item` from being rendered directly under `ion-list`. @@ -64,6 +65,14 @@ The rule only checks `ion-item` elements contained by `ion-list`. An `ion-item` This rule has no options. +## Automatic fixes + +When a list contains only ungrouped `ion-item` elements, including through transparent Angular control-flow blocks, `ng-container`, or `ng-template`, the rule can wrap the entire list contents in one `ion-item-group`. + +The automatic fix is available when the same template already uses `ion-item-group`, which indicates that the standalone `IonItemGroup` component is available to the template. Otherwise, the rule offers an editor suggestion that also reminds you to add `IonItemGroup` to the component imports if needed. + +No fix or suggestion is offered when the list mixes grouped and ungrouped content, contains other rendered content, contains a nested list, has an intervening rendered element, or uses an invalid accordion structure. In these cases, the intended group boundary cannot be determined safely. + ## When to enable Enable this rule in Ionic Angular applications that target the iOS 26 and Material Design 3 list designs. It is included in the recommended preset and has no effect when a template does not contain an `ion-item` within `ion-list`. diff --git a/src/rules/require-ion-item-group.ts b/src/rules/require-ion-item-group.ts index 71da5ff..d01baf7 100644 --- a/src/rules/require-ion-item-group.ts +++ b/src/rules/require-ion-item-group.ts @@ -3,9 +3,16 @@ import type { TSESTree } from '@typescript-eslint/utils'; interface TemplateNode { name?: string; + tagName?: string; + value?: string; type: string; loc?: TSESTree.SourceLocation; + sourceSpan?: { start: { offset: number }; end: { offset: number } }; + startSourceSpan?: { end: { offset: number } }; + endSourceSpan?: { start: { offset: number } }; children?: TemplateNode[]; + inputs?: TemplateNode[]; + templateAttrs?: TemplateNode[]; branches?: TemplateNode[]; cases?: TemplateNode[]; groups?: TemplateNode[]; @@ -18,8 +25,20 @@ interface TemplateNode { } const DIRECT_ITEM_GROUPS = new Set(['ion-item-group', 'ion-reorder-group', 'ion-radio-group']); +const TRANSPARENT_CONTROL_FLOW_NODES = new Set([ + 'DeferredBlock', + 'ForLoopBlock', + 'IfBlock', + 'IfBlockBranch', + 'SwitchBlock', + 'SwitchBlockCase', + 'SwitchBlockCaseGroup', +]); +const DYNAMIC_OUTLETS = new Set(['ngComponentOutlet', 'ngTemplateOutlet']); -const rule: TSESLint.RuleModule<'requireIonItemGroup', []> = { +type MessageIds = 'requireIonItemGroup' | 'wrapIonItemGroup'; + +const rule: TSESLint.RuleModule = { defaultOptions: [], meta: { docs: { @@ -29,47 +48,137 @@ const rule: TSESLint.RuleModule<'requireIonItemGroup', []> = { messages: { requireIonItemGroup: 'ion-item inside ion-list must be wrapped by ion-item-group, ion-reorder-group, ion-radio-group, or ion-accordion within ion-accordion-group.', + wrapIonItemGroup: 'Wrap the list items in ion-item-group. Add IonItemGroup to the component imports if needed.', }, schema: [], type: 'problem', + fixable: 'code', + hasSuggestions: true, }, create(context) { const isHtmlFile = (filename: string) => filename.includes('.html') && !filename.includes('.spec'); const isRenderedElement = (node: TemplateNode) => node.type.includes('Element') && node.name !== 'ng-container'; + const isRenderedText = (node: TemplateNode) => (node.type === 'Text' && Boolean(node.value?.trim())) || node.type.includes('BoundText'); + const hasDynamicOutlet = (node: TemplateNode) => + [...(node.inputs ?? []), ...(node.templateAttrs ?? [])].some((binding) => DYNAMIC_OUTLETS.has(binding.name ?? '')); + const isTransparentStructure = (node: TemplateNode) => + (node.type === 'Text' && !node.value?.trim()) || + (node.type.includes('Element') && node.name === 'ng-container' && !hasDynamicOutlet(node)) || + (node.type === 'Template' && !hasDynamicOutlet(node)) || + TRANSPARENT_CONTROL_FLOW_NODES.has(node.type); + + const visitChildren = (node: TemplateNode, visit: (nodes: TemplateNode[] | undefined) => void): void => { + visit(node.children); + visit(node.branches); + visit(node.cases); + visit(node.groups); + visit(node.then?.children); + visit(node.else?.children); + visit(node.empty?.children); + visit(node.placeholder?.children); + visit(node.loading?.children); + visit(node.error?.children); + }; + + const containsElement = (nodes: TemplateNode[] | undefined, name: string): boolean => { + for (const node of nodes ?? []) { + if (isRenderedElement(node) && node.name === name) { + return true; + } + let found = false; + visitChildren(node, (children) => { + found ||= containsElement(children, name); + }); + if (found) { + return true; + } + } + return false; + }; + + const renderedRoots = (nodes: TemplateNode[] | undefined): TemplateNode[] => { + const roots: TemplateNode[] = []; + for (const node of nodes ?? []) { + if (isRenderedElement(node) || isRenderedText(node) || !isTransparentStructure(node)) { + roots.push(node); + } else { + visitChildren(node, (children) => { + roots.push(...renderedRoots(children)); + }); + } + } + return roots; + }; + + const groupableListRange = (list: TemplateNode): TSESTree.Range | undefined => { + const roots = renderedRoots(list.children); + if ( + roots.length === 0 || + roots.some((node) => !isRenderedElement(node) || node.name !== 'ion-item') || + roots.some((node) => containsElement(node.children, 'ion-item')) || + containsElement(list.children, 'ion-list') || + !list.startSourceSpan || + !list.endSourceSpan + ) { + return undefined; + } + return [list.startSourceSpan.end.offset, list.endSourceSpan.start.offset]; + }; + + const wrapListContents = (range: TSESTree.Range): string => `${context.sourceCode.text.slice(...range)}`; + + const nearestListIndex = (ancestors: TemplateNode[]): number => { + for (let index = ancestors.length - 1; index >= 0; index -= 1) { + if (ancestors[index].name === 'ion-list') { + return index; + } + } + return -1; + }; - function visit(nodes: TemplateNode[] | undefined, ancestors: string[]): void { + function visit(nodes: TemplateNode[] | undefined, ancestors: TemplateNode[], templateHasItemGroup: boolean, handledLists: Set): void { for (const node of nodes ?? []) { - const nextAncestors = isRenderedElement(node) && node.name ? [...ancestors, node.name] : ancestors; + const nextAncestors = isRenderedElement(node) && node.name ? [...ancestors, node] : ancestors; if (isRenderedElement(node) && node.name === 'ion-item') { - const nearestListIndex = ancestors.lastIndexOf('ion-list'); - if (nearestListIndex >= 0) { - const elementsAfterList = ancestors.slice(nearestListIndex + 1); - const hasDirectItemGroup = elementsAfterList.length === 1 && DIRECT_ITEM_GROUPS.has(elementsAfterList[0]); + const listIndex = nearestListIndex(ancestors); + if (listIndex >= 0) { + const nearestList = ancestors[listIndex]; + const elementsAfterList = ancestors.slice(listIndex + 1).map((ancestor) => ancestor.name); + const hasDirectItemGroup = elementsAfterList.length === 1 && DIRECT_ITEM_GROUPS.has(elementsAfterList[0] ?? ''); const hasAccordionGroup = elementsAfterList.length === 2 && elementsAfterList[0] === 'ion-accordion-group' && elementsAfterList[1] === 'ion-accordion'; const hasRequiredStructure = hasDirectItemGroup || hasAccordionGroup; if (!hasRequiredStructure) { + const reportNode = node as unknown as TSESTree.Node; + const groupRange = elementsAfterList.length === 0 ? groupableListRange(nearestList) : undefined; + const canOfferGroup = groupRange && !handledLists.has(nearestList); + if (canOfferGroup) { + handledLists.add(nearestList); + } context.report({ - node: node as unknown as TSESTree.Node, + node: reportNode, loc: node.loc, messageId: 'requireIonItemGroup', + fix: canOfferGroup && templateHasItemGroup ? (fixer) => fixer.replaceTextRange(groupRange, wrapListContents(groupRange)) : undefined, + suggest: + canOfferGroup && !templateHasItemGroup + ? [ + { + messageId: 'wrapIonItemGroup', + fix: (fixer) => fixer.replaceTextRange(groupRange, wrapListContents(groupRange)), + }, + ] + : undefined, }); } } } - visit(node.children, nextAncestors); - visit(node.branches, nextAncestors); - visit(node.cases, nextAncestors); - visit(node.groups, nextAncestors); - visit(node.then?.children, nextAncestors); - visit(node.else?.children, nextAncestors); - visit(node.empty?.children, nextAncestors); - visit(node.placeholder?.children, nextAncestors); - visit(node.loading?.children, nextAncestors); - visit(node.error?.children, nextAncestors); + visitChildren(node, (children) => { + visit(children, nextAncestors, templateHasItemGroup, handledLists); + }); } } @@ -80,7 +189,7 @@ const rule: TSESLint.RuleModule<'requireIonItemGroup', []> = { } const templateNodes = (node as unknown as { templateNodes?: TemplateNode[] }).templateNodes; - visit(templateNodes, []); + visit(templateNodes, [], containsElement(templateNodes, 'ion-item-group'), new Set()); }, }; }, diff --git a/tests/rules/require-ion-item-group.ts b/tests/rules/require-ion-item-group.ts index fb426dd..294416a 100644 --- a/tests/rules/require-ion-item-group.ts +++ b/tests/rules/require-ion-item-group.ts @@ -88,11 +88,22 @@ ruleTester.run('require-ion-item-group', rule, { { code: '', filename: 'template.html', - errors: [{ messageId: 'requireIonItemGroup' }], + errors: [ + { + messageId: 'requireIonItemGroup', + suggestions: [ + { + messageId: 'wrapIonItemGroup', + output: '', + }, + ], + }, + ], }, { code: '
', filename: 'template.html', + output: null, errors: [{ messageId: 'requireIonItemGroup' }], }, { @@ -108,6 +119,7 @@ ruleTester.run('require-ion-item-group', rule, { { code: '', filename: 'template.html', + output: null, errors: [{ messageId: 'requireIonItemGroup' }], }, { @@ -119,7 +131,24 @@ ruleTester.run('require-ion-item-group', rule, { `, filename: 'template.html', - errors: [{ messageId: 'requireIonItemGroup', line: 4 }], + errors: [ + { + messageId: 'requireIonItemGroup', + line: 4, + suggestions: [ + { + messageId: 'wrapIonItemGroup', + output: ` + + @for (item of items; track item.id) { + {{ item.name }} + } + + `, + }, + ], + }, + ], }, { code: ` @@ -133,8 +162,25 @@ ruleTester.run('require-ion-item-group', rule, { `, filename: 'template.html', errors: [ - { messageId: 'requireIonItemGroup', line: 4 }, - { messageId: 'requireIonItemGroup', line: 6 }, + { + messageId: 'requireIonItemGroup', + line: 4, + suggestions: [ + { + messageId: 'wrapIonItemGroup', + output: ` + + @if (visible) { + First + } @else { + Second + } + + `, + }, + ], + }, + { messageId: 'requireIonItemGroup', line: 6, suggestions: null }, ], }, { @@ -148,7 +194,7 @@ ruleTester.run('require-ion-item-group', rule, { `, filename: 'template.html', - errors: [{ messageId: 'requireIonItemGroup', line: 6 }], + errors: [{ messageId: 'requireIonItemGroup', line: 6, suggestions: null }], }, { code: ` @@ -166,9 +212,9 @@ ruleTester.run('require-ion-item-group', rule, { `, filename: 'template.html', errors: [ - { messageId: 'requireIonItemGroup', line: 6 }, - { messageId: 'requireIonItemGroup', line: 8 }, - { messageId: 'requireIonItemGroup', line: 10 }, + { messageId: 'requireIonItemGroup', line: 6, suggestions: null }, + { messageId: 'requireIonItemGroup', line: 8, suggestions: null }, + { messageId: 'requireIonItemGroup', line: 10, suggestions: null }, ], }, { @@ -182,8 +228,24 @@ ruleTester.run('require-ion-item-group', rule, { `, filename: 'template.html', errors: [ - { messageId: 'requireIonItemGroup', line: 4 }, - { messageId: 'requireIonItemGroup', line: 5 }, + { + messageId: 'requireIonItemGroup', + line: 4, + suggestions: [ + { + messageId: 'wrapIonItemGroup', + output: ` + + @switch (selected) { + @case ('first') { First } + @default { Default } + } + + `, + }, + ], + }, + { messageId: 'requireIonItemGroup', line: 5, suggestions: null }, ], }, { @@ -197,8 +259,172 @@ ruleTester.run('require-ion-item-group', rule, { `, filename: 'template.html', errors: [ - { messageId: 'requireIonItemGroup', line: 3 }, - { messageId: 'requireIonItemGroup', line: 5 }, + { messageId: 'requireIonItemGroup', line: 3, suggestions: null }, + { + messageId: 'requireIonItemGroup', + line: 5, + suggestions: [ + { + messageId: 'wrapIonItemGroup', + output: ` + + Outer + + Inner + + + `, + }, + ], + }, + ], + }, + { + code: 'FirstSecond', + filename: 'template.html', + errors: [ + { + messageId: 'requireIonItemGroup', + suggestions: [ + { + messageId: 'wrapIonItemGroup', + output: 'FirstSecond', + }, + ], + }, + { messageId: 'requireIonItemGroup', suggestions: null }, + ], + }, + { + code: '😀', + filename: 'template.html', + errors: [ + { + messageId: 'requireIonItemGroup', + suggestions: [ + { + messageId: 'wrapIonItemGroup', + output: '😀', + }, + ], + }, + ], + }, + { + code: '', + filename: 'template.html', + errors: [ + { + messageId: 'requireIonItemGroup', + suggestions: [ + { + messageId: 'wrapIonItemGroup', + output: '', + }, + ], + }, + ], + }, + { + code: 'GroupedBare', + filename: 'template.html', + output: null, + errors: [{ messageId: 'requireIonItemGroup', suggestions: null }], + }, + { + code: 'OuterInner', + filename: 'template.html', + errors: [ + { messageId: 'requireIonItemGroup', suggestions: null }, + { + messageId: 'requireIonItemGroup', + suggestions: [ + { + messageId: 'wrapIonItemGroup', + output: 'OuterInner', + }, + ], + }, + ], + }, + { + code: 'AB', + filename: 'template.html', + output: 'AB', + errors: [{ messageId: 'requireIonItemGroup' }, { messageId: 'requireIonItemGroup' }], + }, + { + code: ` + + + @for (item of items; track item.id) { + {{ item.name }} + } + + `, + filename: 'template.html', + output: ` + + + @for (item of items; track item.id) { + {{ item.name }} + } + + `, + errors: [{ messageId: 'requireIonItemGroup' }], + }, + { + code: 'HeadingItem', + filename: 'template.html', + output: null, + errors: [{ messageId: 'requireIonItemGroup', suggestions: null }], + }, + { + code: '{{ heading }}Item', + filename: 'template.html', + output: null, + errors: [{ messageId: 'requireIonItemGroup', suggestions: null }], + }, + { + code: 'Item', + filename: 'template.html', + output: null, + errors: [{ messageId: 'requireIonItemGroup', suggestions: null }], + }, + { + code: '{count, plural, =0 {none} other {{{count}} items}}Item', + filename: 'template.html', + output: null, + errors: [{ messageId: 'requireIonItemGroup', suggestions: null }], + }, + { + code: 'Item', + filename: 'template.html', + output: null, + errors: [{ messageId: 'requireIonItemGroup', suggestions: null }], + }, + { + code: 'Item', + filename: 'template.html', + output: null, + errors: [{ messageId: 'requireIonItemGroup', suggestions: null }], + }, + { + code: 'OuterInner', + filename: 'template.html', + output: null, + errors: [ + { messageId: 'requireIonItemGroup', suggestions: null }, + { messageId: 'requireIonItemGroup', suggestions: null }, + ], + }, + { + code: 'Outer
Inner
', + filename: 'template.html', + output: null, + errors: [ + { messageId: 'requireIonItemGroup', suggestions: null }, + { messageId: 'requireIonItemGroup', suggestions: null }, ], }, ], From e0c8b23c2781c48fde1e681cc15a8d92ca27c5d1 Mon Sep 17 00:00:00 2001 From: rdlabo Date: Sat, 22 Aug 2026 13:08:01 +0900 Subject: [PATCH 5/6] refactor(rules): share Angular template traversal --- src/rules/deny-element.ts | 87 +++--------------------- src/rules/no-reactive-forms.ts | 57 ++++++---------- src/rules/no-template-driven-forms.ts | 32 ++------- src/rules/prefer-disable-handler.ts | 40 ++--------- src/rules/require-ion-item-group.ts | 83 +++++------------------ src/rules/template-ast-utils.ts | 97 +++++++++++++++++++++++++++ tests/rules/no-reactive-forms.ts | 5 ++ tests/rules/template-ast-utils.ts | 90 +++++++++++++++++++++++++ 8 files changed, 252 insertions(+), 239 deletions(-) create mode 100644 src/rules/template-ast-utils.ts create mode 100644 tests/rules/template-ast-utils.ts diff --git a/src/rules/deny-element.ts b/src/rules/deny-element.ts index badaec8..b39dc3b 100644 --- a/src/rules/deny-element.ts +++ b/src/rules/deny-element.ts @@ -1,16 +1,6 @@ import { TSESLint } from '@typescript-eslint/utils'; import type { TSESTree } from '@typescript-eslint/utils'; - -interface TemplateNode { - name: string; - type: string; - loc: { - start: { line: number; column: number }; - end: { line: number; column: number }; - }; - children: TemplateNodes; -} -type TemplateNodes = TemplateNode[]; +import { type TemplateAstNode, walkTemplateNodes } from './template-ast-utils'; interface Scheme { elements: string[]; } @@ -46,10 +36,8 @@ const rule: TSESLint.RuleModule<'denyElement', [Scheme]> = { create: (context) => { const isHtmlFile = (filename: string) => !filename.includes('.spec') && filename.includes('.html'); - const isElementNode = (node: TemplateNode) => node.type.includes('Element'); - - const checkElement = (node: TemplateNode, deniedElements: string[]) => { - if (deniedElements.includes(node.name)) { + const checkElement = (node: TemplateAstNode, deniedElements: string[]) => { + if (node.name && deniedElements.includes(node.name)) { context.report({ node: node as unknown as TSESTree.Node, loc: node.loc, @@ -61,65 +49,6 @@ const rule: TSESLint.RuleModule<'denyElement', [Scheme]> = { } }; - const processNode = (node: TemplateNode, deniedElements: string[]) => { - checkElement(node, deniedElements); - - // 子ノードを再帰的に処理 - if (node.children) { - node.children.filter(isElementNode).forEach((child) => processNode(child, deniedElements)); - } - }; - - // 制御フロー構文を含む汎用的なノード処理 - const processTemplateNodes = (templateNodes: TemplateNode[]) => { - const traverseTemplateNodes = (nodes: TemplateNode[]) => { - if (!Array.isArray(nodes)) return; - - for (const node of nodes) { - // Element ノードの場合、属性をチェック - if (isElementNode(node)) { - processNode(node, context.options[0]?.elements || []); - } - - // その他のノード（制御フロー構文など）の子ノードを再帰的に処理 - else if (node && typeof node === 'object' && 'type' in node) { - const nodeWithChildren = node as unknown as { - children?: TemplateNode[]; - branches?: TemplateNode[]; - then?: { children?: TemplateNode[] }; - else?: { children?: TemplateNode[] }; - [key: string]: unknown; - }; - - // 制御フロー構文でよく使われる子ノードプロパティのみを探索 - const childProperties = ['children', 'branches']; - const nestedChildProperties = ['then', 'else']; - - // 直接の子ノードプロパティを処理 - for (const prop of childProperties) { - const childNodes = nodeWithChildren[prop]; - if (Array.isArray(childNodes)) { - traverseTemplateNodes(childNodes); - } - } - - // ネストした子ノードプロパティを処理 - for (const prop of nestedChildProperties) { - const nestedNode = nodeWithChildren[prop]; - if (nestedNode && typeof nestedNode === 'object' && 'children' in nestedNode) { - const childObj = nestedNode as { children?: TemplateNode[] }; - if (Array.isArray(childObj.children)) { - traverseTemplateNodes(childObj.children); - } - } - } - } - } - }; - - traverseTemplateNodes(templateNodes); - }; - return { Program(node) { const filename = context.filename; @@ -130,12 +59,16 @@ const rule: TSESLint.RuleModule<'denyElement', [Scheme]> = { throw new Error('elements is not defined. Please define elements using array.'); } - const templateNodes: TemplateNodes = ( + const templateNodes = ( node as unknown as { - templateNodes: TemplateNodes; + templateNodes: TemplateAstNode[]; } ).templateNodes; - processTemplateNodes(templateNodes); + walkTemplateNodes(templateNodes, (templateNode) => { + if (templateNode.type.includes('Element')) { + checkElement(templateNode, scheme.elements); + } + }); }, }; }, diff --git a/src/rules/no-reactive-forms.ts b/src/rules/no-reactive-forms.ts index 6ec0902..d94273d 100644 --- a/src/rules/no-reactive-forms.ts +++ b/src/rules/no-reactive-forms.ts @@ -1,4 +1,5 @@ import { TSESLint, TSESTree } from '@typescript-eslint/utils'; +import { type TemplateAstNode, walkTemplateNodes } from './template-ast-utils'; const REACTIVE_IMPORTS = new Set([ 'AbstractControl', @@ -28,28 +29,6 @@ function importedName(node: TSESTree.Identifier | TSESTree.StringLiteral): strin return node.type === 'Identifier' ? node.name : node.value; } -interface TemplateNode { - type: string; - name?: string; - loc: TSESTree.SourceLocation; - children?: TemplateNode[]; - branches?: TemplateNode[]; - cases?: TemplateNode[]; - inputs?: TemplateNode[]; - attributes?: TemplateNode[]; -} - -function visitTemplate(nodes: TemplateNode[] | undefined, visit: (node: TemplateNode) => void): void { - for (const node of nodes ?? []) { - visit(node); - visitTemplate(node.inputs, visit); - visitTemplate(node.attributes, visit); - visitTemplate(node.children, visit); - visitTemplate(node.branches, visit); - visitTemplate(node.cases, visit); - } -} - const rule: TSESLint.RuleModule = { defaultOptions: [], meta: { @@ -85,21 +64,25 @@ const rule: TSESLint.RuleModule = { } }, Program(node) { - const templateNodes = (node as unknown as { templateNodes?: TemplateNode[] }).templateNodes; - visitTemplate(templateNodes, (templateNode) => { - if ( - (templateNode.type === 'BoundAttribute' || templateNode.type === 'TextAttribute') && - templateNode.name && - REACTIVE_TEMPLATE_BINDINGS.has(templateNode.name) - ) { - context.report({ - node: templateNode as unknown as TSESTree.Node, - loc: templateNode.loc, - messageId: 'reactiveFormsBinding', - data: { name: templateNode.name }, - }); - } - }); + const templateNodes = (node as unknown as { templateNodes?: TemplateAstNode[] }).templateNodes; + walkTemplateNodes( + templateNodes, + (templateNode) => { + if ( + (templateNode.type === 'BoundAttribute' || templateNode.type === 'TextAttribute') && + templateNode.name && + REACTIVE_TEMPLATE_BINDINGS.has(templateNode.name) + ) { + context.report({ + node: templateNode as unknown as TSESTree.Node, + loc: templateNode.loc, + messageId: 'reactiveFormsBinding', + data: { name: templateNode.name }, + }); + } + }, + ['inputs', 'attributes'], + ); }, }; }, diff --git a/src/rules/no-template-driven-forms.ts b/src/rules/no-template-driven-forms.ts index dd94898..b39aa99 100644 --- a/src/rules/no-template-driven-forms.ts +++ b/src/rules/no-template-driven-forms.ts @@ -1,35 +1,12 @@ import { TSESLint, TSESTree } from '@typescript-eslint/utils'; +import { type TemplateAstNode, walkTemplateNodes } from './template-ast-utils'; interface RuleOptions { allowedElements?: string[]; } -interface TemplateNode { - type: string; - name?: string; - loc: TSESTree.SourceLocation; - children?: TemplateNode[]; - branches?: TemplateNode[]; - cases?: TemplateNode[]; - inputs?: TemplateNode[]; - attributes?: TemplateNode[]; - references?: TemplateNode[]; - value?: string; -} - type MessageIds = 'templateDrivenForms' | 'templateDrivenFormsDirective'; -function visitElements(nodes: TemplateNode[] | undefined, visit: (node: TemplateNode) => void): void { - for (const node of nodes ?? []) { - if (node.type === 'Element') { - visit(node); - } - visitElements(node.children, visit); - visitElements(node.branches, visit); - visitElements(node.cases, visit); - } -} - const rule: TSESLint.RuleModule = { defaultOptions: [{ allowedElements: [] }], meta: { @@ -60,8 +37,11 @@ const rule: TSESLint.RuleModule = { const allowedElements = new Set(context.options[0]?.allowedElements ?? []); return { Program(node) { - const templateNodes = (node as unknown as { templateNodes?: TemplateNode[] }).templateNodes; - visitElements(templateNodes, (element) => { + const templateNodes = (node as unknown as { templateNodes?: TemplateAstNode[] }).templateNodes; + walkTemplateNodes(templateNodes, (element) => { + if (element.type !== 'Element') { + return; + } const attributes = [...(element.inputs ?? []), ...(element.attributes ?? [])]; const hasNgModel = attributes.some((attribute) => attribute.name === 'ngModel'); if (hasNgModel && element.name && !allowedElements.has(element.name)) { diff --git a/src/rules/prefer-disable-handler.ts b/src/rules/prefer-disable-handler.ts index e85de9e..4edb09a 100644 --- a/src/rules/prefer-disable-handler.ts +++ b/src/rules/prefer-disable-handler.ts @@ -1,19 +1,14 @@ import { TSESLint } from '@typescript-eslint/utils'; import type { TSESTree } from '@typescript-eslint/utils'; +import { type TemplateAstNode as SharedTemplateAstNode, walkTemplateNodes } from './template-ast-utils'; interface TemplateLoc { start: { line: number; column: number }; end: { line: number; column: number }; } -interface TemplateAstNode { - type: string; - name?: string; +interface TemplateAstNode extends SharedTemplateAstNode { loc?: TemplateLoc; - children?: TemplateAstNode[]; - branches?: TemplateAstNode[]; - then?: { children?: TemplateAstNode[] }; - else?: { children?: TemplateAstNode[] }; outputs?: BoundEventNode[]; handler?: TemplateExpression; [key: string]: unknown; @@ -213,31 +208,6 @@ const rule: TSESLint.RuleModule<'preferDisableHandler', [Scheme]> = { } }; - const traverseTemplateNodes = (nodes: TemplateAstNode[] | undefined) => { - if (!Array.isArray(nodes)) return; - - for (const node of nodes) { - if (!node || typeof node !== 'object' || !('type' in node)) continue; - - if (String(node.type).includes('Element')) { - processElement(node); - } - - if (Array.isArray(node.children)) { - traverseTemplateNodes(node.children); - } - if (Array.isArray(node.branches)) { - traverseTemplateNodes(node.branches); - } - if (node.then?.children) { - traverseTemplateNodes(node.then.children); - } - if (node.else?.children) { - traverseTemplateNodes(node.else.children); - } - } - }; - return { Program(node) { if (!isHtmlFile(context.filename)) return; @@ -248,7 +218,11 @@ const rule: TSESLint.RuleModule<'preferDisableHandler', [Scheme]> = { } ).templateNodes; - traverseTemplateNodes(templateNodes); + walkTemplateNodes(templateNodes, (templateNode) => { + if (templateNode.type.includes('Element')) { + processElement(templateNode as TemplateAstNode); + } + }); }, }; }, diff --git a/src/rules/require-ion-item-group.ts b/src/rules/require-ion-item-group.ts index d01baf7..25fa06e 100644 --- a/src/rules/require-ion-item-group.ts +++ b/src/rules/require-ion-item-group.ts @@ -1,40 +1,8 @@ import { TSESLint } from '@typescript-eslint/utils'; import type { TSESTree } from '@typescript-eslint/utils'; - -interface TemplateNode { - name?: string; - tagName?: string; - value?: string; - type: string; - loc?: TSESTree.SourceLocation; - sourceSpan?: { start: { offset: number }; end: { offset: number } }; - startSourceSpan?: { end: { offset: number } }; - endSourceSpan?: { start: { offset: number } }; - children?: TemplateNode[]; - inputs?: TemplateNode[]; - templateAttrs?: TemplateNode[]; - branches?: TemplateNode[]; - cases?: TemplateNode[]; - groups?: TemplateNode[]; - then?: { children?: TemplateNode[] }; - else?: { children?: TemplateNode[] }; - empty?: { children?: TemplateNode[] }; - placeholder?: { children?: TemplateNode[] }; - loading?: { children?: TemplateNode[] }; - error?: { children?: TemplateNode[] }; -} +import { isRenderedElement, isRenderedText, isTransparentTemplateStructure, type TemplateAstNode, visitTemplateChildren } from './template-ast-utils'; const DIRECT_ITEM_GROUPS = new Set(['ion-item-group', 'ion-reorder-group', 'ion-radio-group']); -const TRANSPARENT_CONTROL_FLOW_NODES = new Set([ - 'DeferredBlock', - 'ForLoopBlock', - 'IfBlock', - 'IfBlockBranch', - 'SwitchBlock', - 'SwitchBlockCase', - 'SwitchBlockCaseGroup', -]); -const DYNAMIC_OUTLETS = new Set(['ngComponentOutlet', 'ngTemplateOutlet']); type MessageIds = 'requireIonItemGroup' | 'wrapIonItemGroup'; @@ -57,36 +25,14 @@ const rule: TSESLint.RuleModule = { }, create(context) { const isHtmlFile = (filename: string) => filename.includes('.html') && !filename.includes('.spec'); - const isRenderedElement = (node: TemplateNode) => node.type.includes('Element') && node.name !== 'ng-container'; - const isRenderedText = (node: TemplateNode) => (node.type === 'Text' && Boolean(node.value?.trim())) || node.type.includes('BoundText'); - const hasDynamicOutlet = (node: TemplateNode) => - [...(node.inputs ?? []), ...(node.templateAttrs ?? [])].some((binding) => DYNAMIC_OUTLETS.has(binding.name ?? '')); - const isTransparentStructure = (node: TemplateNode) => - (node.type === 'Text' && !node.value?.trim()) || - (node.type.includes('Element') && node.name === 'ng-container' && !hasDynamicOutlet(node)) || - (node.type === 'Template' && !hasDynamicOutlet(node)) || - TRANSPARENT_CONTROL_FLOW_NODES.has(node.type); - - const visitChildren = (node: TemplateNode, visit: (nodes: TemplateNode[] | undefined) => void): void => { - visit(node.children); - visit(node.branches); - visit(node.cases); - visit(node.groups); - visit(node.then?.children); - visit(node.else?.children); - visit(node.empty?.children); - visit(node.placeholder?.children); - visit(node.loading?.children); - visit(node.error?.children); - }; - const containsElement = (nodes: TemplateNode[] | undefined, name: string): boolean => { + const containsElement = (nodes: TemplateAstNode[] | undefined, name: string): boolean => { for (const node of nodes ?? []) { if (isRenderedElement(node) && node.name === name) { return true; } let found = false; - visitChildren(node, (children) => { + visitTemplateChildren(node, (children) => { found ||= containsElement(children, name); }); if (found) { @@ -96,13 +42,13 @@ const rule: TSESLint.RuleModule = { return false; }; - const renderedRoots = (nodes: TemplateNode[] | undefined): TemplateNode[] => { - const roots: TemplateNode[] = []; + const renderedRoots = (nodes: TemplateAstNode[] | undefined): TemplateAstNode[] => { + const roots: TemplateAstNode[] = []; for (const node of nodes ?? []) { - if (isRenderedElement(node) || isRenderedText(node) || !isTransparentStructure(node)) { + if (isRenderedElement(node) || isRenderedText(node) || !isTransparentTemplateStructure(node)) { roots.push(node); } else { - visitChildren(node, (children) => { + visitTemplateChildren(node, (children) => { roots.push(...renderedRoots(children)); }); } @@ -110,7 +56,7 @@ const rule: TSESLint.RuleModule = { return roots; }; - const groupableListRange = (list: TemplateNode): TSESTree.Range | undefined => { + const groupableListRange = (list: TemplateAstNode): TSESTree.Range | undefined => { const roots = renderedRoots(list.children); if ( roots.length === 0 || @@ -127,7 +73,7 @@ const rule: TSESLint.RuleModule = { const wrapListContents = (range: TSESTree.Range): string => `${context.sourceCode.text.slice(...range)}`; - const nearestListIndex = (ancestors: TemplateNode[]): number => { + const nearestListIndex = (ancestors: TemplateAstNode[]): number => { for (let index = ancestors.length - 1; index >= 0; index -= 1) { if (ancestors[index].name === 'ion-list') { return index; @@ -136,7 +82,12 @@ const rule: TSESLint.RuleModule = { return -1; }; - function visit(nodes: TemplateNode[] | undefined, ancestors: TemplateNode[], templateHasItemGroup: boolean, handledLists: Set): void { + function visit( + nodes: TemplateAstNode[] | undefined, + ancestors: TemplateAstNode[], + templateHasItemGroup: boolean, + handledLists: Set, + ): void { for (const node of nodes ?? []) { const nextAncestors = isRenderedElement(node) && node.name ? [...ancestors, node] : ancestors; @@ -176,7 +127,7 @@ const rule: TSESLint.RuleModule = { } } - visitChildren(node, (children) => { + visitTemplateChildren(node, (children) => { visit(children, nextAncestors, templateHasItemGroup, handledLists); }); } @@ -188,7 +139,7 @@ const rule: TSESLint.RuleModule = { return; } - const templateNodes = (node as unknown as { templateNodes?: TemplateNode[] }).templateNodes; + const templateNodes = (node as unknown as { templateNodes?: TemplateAstNode[] }).templateNodes; visit(templateNodes, [], containsElement(templateNodes, 'ion-item-group'), new Set()); }, }; diff --git a/src/rules/template-ast-utils.ts b/src/rules/template-ast-utils.ts new file mode 100644 index 0000000..ee03e23 --- /dev/null +++ b/src/rules/template-ast-utils.ts @@ -0,0 +1,97 @@ +import type { TSESTree } from '@typescript-eslint/utils'; + +export interface TemplateAstNode { + type: string; + name?: string; + tagName?: string; + value?: unknown; + loc?: TSESTree.SourceLocation; + sourceSpan?: { + start: { offset?: number; line?: number; col?: number }; + end: { offset?: number; line?: number; col?: number }; + }; + startSourceSpan?: { end: { offset: number } }; + endSourceSpan?: { start: { offset: number } }; + children?: TemplateAstNode[]; + inputs?: TemplateAstNode[]; + attributes?: TemplateAstNode[]; + references?: TemplateAstNode[]; + templateAttrs?: TemplateAstNode[]; + outputs?: TemplateAstNode[]; + branches?: TemplateAstNode[]; + cases?: TemplateAstNode[]; + groups?: TemplateAstNode[]; + then?: { children?: TemplateAstNode[] }; + else?: { children?: TemplateAstNode[] }; + empty?: { children?: TemplateAstNode[] }; + placeholder?: { children?: TemplateAstNode[] }; + loading?: { children?: TemplateAstNode[] }; + error?: { children?: TemplateAstNode[] }; +} + +export const TRANSPARENT_CONTROL_FLOW_NODES: ReadonlySet = new Set([ + 'DeferredBlock', + 'ForLoopBlock', + 'IfBlock', + 'IfBlockBranch', + 'SwitchBlock', + 'SwitchBlockCase', + 'SwitchBlockCaseGroup', +]); + +const DYNAMIC_OUTLETS = new Set(['ngComponentOutlet', 'ngTemplateOutlet']); +const CHILD_ARRAY_KEYS = ['children', 'branches', 'cases', 'groups'] as const; +const CHILD_BLOCK_KEYS = ['then', 'else', 'empty', 'placeholder', 'loading', 'error'] as const; + +export function visitTemplateChildren(node: TemplateAstNode, visit: (nodes: TemplateAstNode[] | undefined) => void): void { + for (const key of CHILD_ARRAY_KEYS) { + visit(node[key]); + } + for (const key of CHILD_BLOCK_KEYS) { + visit(node[key]?.children); + } +} + +export function walkTemplateNodes( + nodes: TemplateAstNode[] | undefined, + visit: (node: TemplateAstNode) => void, + additionalArrayKeys: readonly ('attributes' | 'inputs' | 'outputs' | 'references' | 'templateAttrs')[] = [], +): void { + const visited = new Set(); + + const walk = (currentNodes: TemplateAstNode[] | undefined): void => { + for (const node of currentNodes ?? []) { + if (visited.has(node)) { + continue; + } + visited.add(node); + visit(node); + visitTemplateChildren(node, walk); + for (const key of additionalArrayKeys) { + walk(node[key]); + } + } + }; + + walk(nodes); +} + +export function isRenderedElement(node: TemplateAstNode): boolean { + return node.type.includes('Element') && node.name !== 'ng-container'; +} + +export function isRenderedText(node: TemplateAstNode): boolean { + return (node.type === 'Text' && typeof node.value === 'string' && Boolean(node.value.trim())) || node.type.includes('BoundText'); +} + +export function isTransparentTemplateStructure(node: TemplateAstNode): boolean { + const bindings = [...(node.inputs ?? []), ...(node.templateAttrs ?? [])]; + const hasDynamicOutlet = bindings.some((binding) => DYNAMIC_OUTLETS.has(binding.name ?? '')); + + return ( + (node.type === 'Text' && (typeof node.value !== 'string' || !node.value.trim())) || + (node.type.includes('Element') && node.name === 'ng-container' && !hasDynamicOutlet) || + (node.type === 'Template' && !hasDynamicOutlet) || + TRANSPARENT_CONTROL_FLOW_NODES.has(node.type) + ); +} diff --git a/tests/rules/no-reactive-forms.ts b/tests/rules/no-reactive-forms.ts index e038cc3..4a795b0 100644 --- a/tests/rules/no-reactive-forms.ts +++ b/tests/rules/no-reactive-forms.ts @@ -49,5 +49,10 @@ new TemplateRuleTester({ filename: 'template.html', errors: [{ messageId: 'reactiveFormsBinding', data: { name: 'formControl' } }], }, + { + code: '
', + filename: 'template.html', + errors: [{ messageId: 'reactiveFormsBinding', data: { name: 'formGroup' } }], + }, ], }); diff --git a/tests/rules/template-ast-utils.ts b/tests/rules/template-ast-utils.ts new file mode 100644 index 0000000..a445680 --- /dev/null +++ b/tests/rules/template-ast-utils.ts @@ -0,0 +1,90 @@ +import { type TemplateAstNode, isTransparentTemplateStructure, walkTemplateNodes } from '../../src/rules/template-ast-utils'; + +// eslint-disable-next-line @typescript-eslint/no-require-imports +const parser = require('@angular-eslint/template-parser') as { + parseForESLint(code: string, options: { filePath: string }): { ast: { templateNodes?: TemplateAstNode[] } }; +}; + +function parseTemplate(code: string): TemplateAstNode[] | undefined { + return parser.parseForESLint(code, { filePath: 'template.html' }).ast.templateNodes; +} + +describe('template AST utilities', () => { + it('walks every Angular control-flow branch', () => { + const nodes = parseTemplate(` + @if (visible) { + If + } @else { + Else + } + @for (item of items; track item.id) { + For + } @empty { + Empty + } + @switch (selected) { + @case ('first') { Case } + @default { Default } + } + @defer (when ready) { + Deferred + } @placeholder { + Placeholder + } @loading { + Loading + } @error { + Error + } + `); + const itemLabels: string[] = []; + + walkTemplateNodes(nodes, (node) => { + if (node.type === 'Element' && node.name === 'ion-item') { + const text = node.children?.find((child) => child.type === 'Text')?.value; + if (typeof text === 'string') { + itemLabels.push(text); + } + } + }); + + expect(itemLabels).toEqual(['If', 'Else', 'For', 'Empty', 'Case', 'Default', 'Deferred', 'Placeholder', 'Loading', 'Error']); + }); + + it('visits a binding shared by a structural Template and Element once', () => { + const nodes = parseTemplate('
'); + const formGroupBindings: TemplateAstNode[] = []; + + walkTemplateNodes( + nodes, + (node) => { + if (node.type === 'BoundAttribute' && node.name === 'formGroup') { + formGroupBindings.push(node); + } + }, + ['inputs'], + ); + + expect(formGroupBindings).toHaveLength(1); + }); + + it('only treats statically transparent containers as transparent', () => { + const nodes = parseTemplate(` + + + + + + `); + const structures: TemplateAstNode[] = []; + walkTemplateNodes(nodes, (node) => { + if (node.type !== 'Text') { + structures.push(node); + } + }); + + expect(isTransparentTemplateStructure(structures[0])).toBe(true); + expect(isTransparentTemplateStructure(structures[1])).toBe(true); + expect(structures.some((node) => node.type === 'Template' && !isTransparentTemplateStructure(node))).toBe(true); + expect(structures.some((node) => node.type === 'Content' && !isTransparentTemplateStructure(node))).toBe(true); + }); +}); From d8c76f225ec131d8a13644b7648f7e3d48b032de Mon Sep 17 00:00:00 2001 From: rdlabo Date: Sat, 22 Aug 2026 13:34:58 +0900 Subject: [PATCH 6/6] fix(rule): avoid fixing reusable templates --- docs/rules/require-ion-item-group.md | 4 ++-- src/rules/require-ion-item-group.ts | 3 ++- src/rules/template-ast-utils.ts | 3 ++- tests/rules/require-ion-item-group.ts | 12 ++++++++++++ 4 files changed, 18 insertions(+), 4 deletions(-) diff --git a/docs/rules/require-ion-item-group.md b/docs/rules/require-ion-item-group.md index 3f53257..4f359a0 100644 --- a/docs/rules/require-ion-item-group.md +++ b/docs/rules/require-ion-item-group.md @@ -67,11 +67,11 @@ This rule has no options. ## Automatic fixes -When a list contains only ungrouped `ion-item` elements, including through transparent Angular control-flow blocks, `ng-container`, or `ng-template`, the rule can wrap the entire list contents in one `ion-item-group`. +When a list contains only ungrouped `ion-item` elements, including through transparent Angular control-flow blocks or `ng-container`, the rule can wrap the entire list contents in one `ion-item-group`. The automatic fix is available when the same template already uses `ion-item-group`, which indicates that the standalone `IonItemGroup` component is available to the template. Otherwise, the rule offers an editor suggestion that also reminds you to add `IonItemGroup` to the component imports if needed. -No fix or suggestion is offered when the list mixes grouped and ungrouped content, contains other rendered content, contains a nested list, has an intervening rendered element, or uses an invalid accordion structure. In these cases, the intended group boundary cannot be determined safely. +No fix or suggestion is offered when the list mixes grouped and ungrouped content, contains other rendered content, contains a reusable `ng-template` definition, contains a nested list, has an intervening rendered element, or uses an invalid accordion structure. In these cases, the intended group boundary cannot be determined safely. ## When to enable diff --git a/src/rules/require-ion-item-group.ts b/src/rules/require-ion-item-group.ts index 25fa06e..df5dd0a 100644 --- a/src/rules/require-ion-item-group.ts +++ b/src/rules/require-ion-item-group.ts @@ -45,7 +45,8 @@ const rule: TSESLint.RuleModule = { const renderedRoots = (nodes: TemplateAstNode[] | undefined): TemplateAstNode[] => { const roots: TemplateAstNode[] = []; for (const node of nodes ?? []) { - if (isRenderedElement(node) || isRenderedText(node) || !isTransparentTemplateStructure(node)) { + const isReusableTemplate = node.type === 'Template' && node.tagName === 'ng-template'; + if (isRenderedElement(node) || isRenderedText(node) || isReusableTemplate || !isTransparentTemplateStructure(node)) { roots.push(node); } else { visitTemplateChildren(node, (children) => { diff --git a/src/rules/template-ast-utils.ts b/src/rules/template-ast-utils.ts index ee03e23..3a06aa2 100644 --- a/src/rules/template-ast-utils.ts +++ b/src/rules/template-ast-utils.ts @@ -29,7 +29,7 @@ export interface TemplateAstNode { error?: { children?: TemplateAstNode[] }; } -export const TRANSPARENT_CONTROL_FLOW_NODES: ReadonlySet = new Set([ +const TRANSPARENT_CONTROL_FLOW_NODES: ReadonlySet = new Set([ 'DeferredBlock', 'ForLoopBlock', 'IfBlock', @@ -55,6 +55,7 @@ export function visitTemplateChildren(node: TemplateAstNode, visit: (nodes: Temp export function walkTemplateNodes( nodes: TemplateAstNode[] | undefined, visit: (node: TemplateAstNode) => void, + // Metadata arrays are opt-in because most rules only need rendered template structure. additionalArrayKeys: readonly ('attributes' | 'inputs' | 'outputs' | 'references' | 'templateAttrs')[] = [], ): void { const visited = new Set(); diff --git a/tests/rules/require-ion-item-group.ts b/tests/rules/require-ion-item-group.ts index 294416a..a118b6b 100644 --- a/tests/rules/require-ion-item-group.ts +++ b/tests/rules/require-ion-item-group.ts @@ -427,5 +427,17 @@ ruleTester.run('require-ion-item-group', rule, { { messageId: 'requireIonItemGroup', suggestions: null }, ], }, + { + code: 'Item', + filename: 'template.html', + output: null, + errors: [{ messageId: 'requireIonItemGroup', suggestions: null }], + }, + { + code: 'Item', + filename: 'template.html', + output: null, + errors: [{ messageId: 'requireIonItemGroup', suggestions: null }], + }, ], });