forked from ember-cli/eslint-plugin-ember
-
Notifications
You must be signed in to change notification settings - Fork 0
Add template-no-duplicate-form-names: flag duplicate form-control name within a form
#43
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Closed
Closed
Changes from all commits
Commits
Show all changes
4 commits
Select commit
Hold shift + click to select a range
0c19b04
feat: add template-no-duplicate-form-names — flag duplicate form-cont…
johanrd a859f42
fix: button valueless type defaults to submit; clarify shareable-type…
johanrd 0198d68
fix: flag duplicate names on disabled controls too
johanrd a922d73
fix: prettier formatting
johanrd File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,82 @@ | ||
| # ember/template-no-duplicate-form-names | ||
|
|
||
| <!-- end auto-generated rule header --> | ||
|
|
||
| This rule disallows two form controls sharing the same `name` attribute | ||
| within the same `<form>` (or within the template root, if no `<form>` | ||
| wraps the controls). | ||
|
|
||
| Duplicate names break form serialization: both values are emitted into | ||
| the entry list, and server-side code that expects a single value typically | ||
| reads only one — often not the one the author intended. | ||
|
|
||
| Three categories are exempt from the duplicate check: | ||
|
|
||
| - **Non-submitting controls** (`<input type="button">`, `<input type="reset">`, | ||
| `<button type="button">`, `<button type="reset">`) are skipped entirely. | ||
| Per [HTML §4.10.21.4](https://html.spec.whatwg.org/multipage/form-control-infrastructure.html#constructing-the-form-data-set) | ||
| they do not contribute to the form data, so their `name` can't collide | ||
| with anything. | ||
| - **Radio groups** (`<input type="radio">`) share a `name` with same-type | ||
| siblings to express mutual exclusion — exactly one contributes per | ||
| submission. | ||
| - **Submit-like controls** may share a `name` across any mix of | ||
| `<input type="submit">`, `<input type="image">`, and `<button>` (bare | ||
| `<button>` defaults to `type="submit"` per HTML §4.10.9). Only one | ||
| submit-like control contributes to the form-data entry list per | ||
| submission — the one the user activated — so same-name is unambiguous. | ||
|
|
||
| ## Examples | ||
|
|
||
| This rule **forbids** the following: | ||
|
|
||
| ```hbs | ||
| <form> | ||
| <input name='email' /> | ||
| <input name='email' /> | ||
| </form> | ||
|
|
||
| <form> | ||
| <input type='text' name='field' /> | ||
| <textarea name='field'></textarea> | ||
| </form> | ||
| ``` | ||
|
|
||
| This rule **allows** the following: | ||
|
|
||
| ```hbs | ||
| <form> | ||
| <input type='radio' name='color' value='red' /> | ||
| <input type='radio' name='color' value='blue' /> | ||
| <input type='submit' name='action' value='save' /> | ||
| <input type='submit' name='action' value='publish' /> | ||
| </form> | ||
| ``` | ||
|
|
||
| Controls with the HTML `disabled` attribute are ignored — per | ||
| [HTML §4.10.21.4](https://html.spec.whatwg.org/multipage/form-control-infrastructure.html#constructing-the-form-data-set) | ||
| they do not contribute to the form-data entry list and cannot collide. | ||
|
|
||
| Controls with the HTML `hidden` attribute are **not** exempt: `hidden` | ||
| affects rendering, not form submission. A `hidden` control still submits | ||
| its `name`/`value`, so a duplicate name involving a hidden control is a | ||
| real collision. | ||
|
|
||
| ## Limitations | ||
|
|
||
| - Controls associated with a form via the `form="id"` attribute (rather than | ||
| by nesting inside a `<form>` element) are not tracked by this rule. Only | ||
| controls that are descendants of a `<form>` element are scoped to that form. | ||
| - `name` values via mustache (`name={{this.fieldName}}`) are skipped — we | ||
| cannot know the value at lint time. | ||
| - The `name[]` PHP-style array pattern is treated as an ordinary name; if | ||
| you declare two `name="x[]"` controls in the same form this rule will | ||
| still flag them. Support for array brackets may be added later. | ||
| - The `<input type="hidden">` + `<input type="checkbox">` default-value | ||
| pattern (which is permitted in HTML) is not recognized — the pair will | ||
| be flagged if you use it. Rename the hidden input if you hit this. | ||
|
|
||
| ## References | ||
|
|
||
| - [HTML spec: Form submission](https://html.spec.whatwg.org/multipage/form-control-infrastructure.html#form-submission-algorithm) | ||
| - Adapted from [`html-validate`'s `form-dup-name`](https://html-validate.org/rules/form-dup-name.html) (MIT). | ||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,275 @@ | ||
| 'use strict'; | ||
|
|
||
| // See html-validate (https://html-validate.org/rules/form-dup-name.html) for the peer rule concept. | ||
| // | ||
| // Simplifications vs. upstream for v1: we don't support `name[]` array syntax, | ||
| // the <input type="hidden"> + <input type="checkbox"> default-value pattern, | ||
| // or the full form-associated element registry. Scope is <input>/<select>/ | ||
| // <textarea>/<button>/<output>. | ||
| // | ||
| // Types that do not contribute to the form-data entry list (per HTML spec | ||
| // §4.10.21.4) are skipped entirely — no name collision is possible because | ||
| // they never submit. This covers <input type="button"|"reset"> and | ||
| // <button type="button"|"reset">. | ||
| // | ||
| // Types whose duplicate-name pattern is legitimate are tracked but allowed to | ||
| // share a name within their "share category": | ||
| // - radio group: multiple <input type=radio> share a name by design (one | ||
| // selected at a time); | ||
| // - submit-like controls: at most one submit-like control contributes per | ||
| // submission, so any mix of <input type=submit>, <input type=image>, and | ||
| // <button> (bare or type=submit) can share a name. | ||
| // Derived from aria-query's button-role mapping; see `getShareCategory` below. | ||
|
|
||
| const { elementRoles } = require('aria-query'); | ||
|
|
||
| const FORM_CONTROL_TAGS = new Set(['input', 'select', 'textarea', 'button', 'output']); | ||
| const NON_SUBMITTING_TYPES = new Set(['button', 'reset']); | ||
|
johanrd marked this conversation as resolved.
|
||
|
|
||
| // Submit-like <input> types — derived from aria-query's `button`-role mapping | ||
| // (input[type=submit|image|button|reset] all map to role=button), minus the | ||
| // non-submitting types. At most ONE submit-like control contributes to the | ||
| // form-data entry list per submission (the one the user clicks), so any mix | ||
| // of these can legitimately share a name. | ||
| // | ||
| // "Radio group" semantics are orthogonal: multiple <input type=radio> share a | ||
| // name because selection is mutually exclusive, and exactly one contributes | ||
| // its value. So radios get their own category. | ||
| const SUBMIT_LIKE_INPUT_TYPES = buildSubmitLikeInputTypes(); | ||
|
|
||
| function buildSubmitLikeInputTypes() { | ||
| const result = new Set(); | ||
| for (const [schema, rolesSet] of elementRoles) { | ||
| if (schema.name !== 'input') { | ||
| continue; | ||
| } | ||
| if (!rolesSet.includes('button')) { | ||
| continue; | ||
| } | ||
| const typeAttr = (schema.attributes || []).find((a) => a.name === 'type'); | ||
| if (!typeAttr || typeof typeAttr.value !== 'string') { | ||
| continue; | ||
| } | ||
| if (NON_SUBMITTING_TYPES.has(typeAttr.value)) { | ||
| continue; | ||
| } | ||
| result.add(typeAttr.value); | ||
| } | ||
| return result; | ||
|
johanrd marked this conversation as resolved.
|
||
| } | ||
|
|
||
| // Returns the "share category" for a control type: entries with the same | ||
| // non-null category can legitimately share a name. | ||
| // - 'radio': radio-group semantics (one selected at a time) | ||
| // - 'submit-like': submit-control semantics (one triggers submission) | ||
| // - null: not shareable; any same-name collision is a real duplicate | ||
| function getShareCategory(tag, type) { | ||
| if (tag === 'input' && type === 'radio') { | ||
| return 'radio'; | ||
| } | ||
| if (tag === 'button') { | ||
| // Bare <button> defaults to type=submit; <button type=submit> too. | ||
| if (type === 'submit') { | ||
| return 'submit-like'; | ||
| } | ||
| return null; | ||
| } | ||
| if (tag === 'input' && SUBMIT_LIKE_INPUT_TYPES.has(type)) { | ||
| return 'submit-like'; | ||
| } | ||
| return null; | ||
| } | ||
|
|
||
|
johanrd marked this conversation as resolved.
|
||
| function findAttr(node, name) { | ||
| return node.attributes?.find((attr) => attr.name === name); | ||
| } | ||
|
|
||
| function getStaticAttrValue(node, name) { | ||
| const attr = findAttr(node, name); | ||
| if (!attr || !attr.value) { | ||
| return { kind: attr ? 'empty' : 'absent', value: '' }; | ||
| } | ||
| if (attr.value.type === 'GlimmerTextNode') { | ||
| return { kind: 'static', value: attr.value.chars }; | ||
| } | ||
|
johanrd marked this conversation as resolved.
|
||
| if (attr.value.type === 'GlimmerMustacheStatement' && attr.value.path) { | ||
| if (attr.value.path.type === 'GlimmerStringLiteral') { | ||
| return { kind: 'static', value: attr.value.path.value }; | ||
| } | ||
| if (attr.value.path.type === 'GlimmerBooleanLiteral') { | ||
| return { kind: 'static', value: String(attr.value.path.value) }; | ||
| } | ||
| } | ||
| return { kind: 'dynamic', value: '' }; | ||
| } | ||
|
|
||
| // HTML §4.10.18 — `<button>` and `<input>` with missing/invalid/unknown type | ||
| // fall back to the default state ('submit' for <button>, 'text' for <input>). | ||
| const BUTTON_TYPES = new Set(['submit', 'reset', 'button']); | ||
| const INPUT_TYPES = new Set([ | ||
| 'hidden', | ||
| 'text', | ||
| 'search', | ||
| 'tel', | ||
| 'url', | ||
| 'email', | ||
| 'password', | ||
| 'date', | ||
| 'month', | ||
| 'week', | ||
| 'time', | ||
| 'datetime-local', | ||
| 'number', | ||
| 'range', | ||
| 'color', | ||
| 'checkbox', | ||
| 'radio', | ||
| 'file', | ||
| 'submit', | ||
| 'image', | ||
| 'reset', | ||
| 'button', | ||
| ]); | ||
|
|
||
| function getControlType(node) { | ||
| if (node.tag === 'button') { | ||
| const t = getStaticAttrValue(node, 'type'); | ||
| if (t.kind === 'static') { | ||
| return BUTTON_TYPES.has(t.value.toLowerCase()) ? t.value.toLowerCase() : 'submit'; | ||
| } | ||
| if (t.kind === 'absent' || t.kind === 'empty') { | ||
| // No type attribute, or valueless `<button type />` — both default to | ||
| // 'submit' per HTML §4.10.9 (invalid/missing type → submit state). | ||
| return 'submit'; | ||
| } | ||
| // Dynamic type (mustache / concat) — runtime value is unknown. Return a | ||
| // sentinel so the caller can skip duplicate-name checks for this node | ||
| // rather than guessing the wrong default. | ||
| return 'unknown'; | ||
| } | ||
| if (node.tag === 'input') { | ||
| const t = getStaticAttrValue(node, 'type'); | ||
| if (t.kind === 'static') { | ||
| return INPUT_TYPES.has(t.value.toLowerCase()) ? t.value.toLowerCase() : 'text'; | ||
| } | ||
| if (t.kind === 'absent' || t.kind === 'empty') { | ||
| return 'text'; | ||
| } | ||
| return 'unknown'; | ||
| } | ||
|
johanrd marked this conversation as resolved.
|
||
| return node.tag; | ||
| } | ||
|
|
||
| function findEnclosingFormOrRoot(node) { | ||
| let current = node.parent; | ||
| while (current) { | ||
| if (current.type === 'GlimmerElementNode' && current.tag === 'form') { | ||
| return current; | ||
| } | ||
| current = current.parent; | ||
| } | ||
| return null; | ||
| } | ||
|
johanrd marked this conversation as resolved.
|
||
|
|
||
| const { getBranchPath, areMutuallyExclusive } = require('../utils/control-flow'); | ||
|
|
||
| /** @type {import('eslint').Rule.RuleModule} */ | ||
| module.exports = { | ||
| meta: { | ||
| type: 'problem', | ||
| docs: { | ||
| description: 'disallow duplicate form control names within the same form', | ||
| category: 'Possible Errors', | ||
| url: 'https://github.com/ember-cli/eslint-plugin-ember/tree/master/docs/rules/template-no-duplicate-form-names.md', | ||
| templateMode: 'both', | ||
| }, | ||
| schema: [], | ||
| messages: { | ||
| duplicate: 'Duplicate form control `name="{{name}}"` within the same form', | ||
| }, | ||
| }, | ||
|
|
||
| create(context) { | ||
| // Per-form: Map<name, entries[]>. Each entry records { type, path } so we | ||
| // can pairwise compare against subsequent occurrences — mutually exclusive | ||
| // branches (different `program`/`inverse` subtrees of the same | ||
| // `{{#if}}`/`{{#unless}}`) never both render, so their same-name | ||
| // "collision" is a false positive. | ||
| const nameMapByForm = new WeakMap(); | ||
| const rootMap = new Map(); | ||
|
|
||
| function getMapForForm(formNode) { | ||
| if (!formNode) { | ||
| return rootMap; | ||
| } | ||
| let map = nameMapByForm.get(formNode); | ||
| if (!map) { | ||
| map = new Map(); | ||
| nameMapByForm.set(formNode, map); | ||
| } | ||
| return map; | ||
| } | ||
|
johanrd marked this conversation as resolved.
|
||
|
|
||
| return { | ||
| GlimmerElementNode(node) { | ||
| if (!FORM_CONTROL_TAGS.has(node.tag)) { | ||
| return; | ||
| } | ||
| const nameInfo = getStaticAttrValue(node, 'name'); | ||
| if (nameInfo.kind !== 'static' || nameInfo.value === '') { | ||
| return; | ||
| } | ||
| const name = nameInfo.value; | ||
| const type = getControlType(node); | ||
| // Dynamic type (`type={{this.kind}}` / concat) — we can't classify | ||
| // the control's submission behavior. Skip duplicate-name collision | ||
| // checks for this node rather than guessing; false negatives here | ||
| // are safer than false positives on legitimate branches. | ||
| if ((node.tag === 'input' || node.tag === 'button') && type === 'unknown') { | ||
| return; | ||
| } | ||
| // Non-submitting controls contribute nothing to the form-data entry | ||
| // list, so their `name` can't collide with anything. | ||
| if ((node.tag === 'input' || node.tag === 'button') && NON_SUBMITTING_TYPES.has(type)) { | ||
| return; | ||
| } | ||
| const form = findEnclosingFormOrRoot(node); | ||
| const map = getMapForForm(form); | ||
| const path = getBranchPath(node); | ||
|
|
||
| const entries = map.get(name); | ||
| const currCategory = getShareCategory(node.tag, type); | ||
|
|
||
| if (!entries) { | ||
| map.set(name, [{ tag: node.tag, type, path, category: currCategory }]); | ||
| return; | ||
| } | ||
|
|
||
| const collides = entries.some((prev) => { | ||
| // Same share-category (radio group, or any mix of submit-like | ||
| // controls) coexist legitimately — at most one contributes to the | ||
| // form-data entry list per submission. | ||
| if (currCategory !== null && currCategory === prev.category) { | ||
| return false; | ||
| } | ||
| // Mutually exclusive control-flow branches never render together. | ||
| if (areMutuallyExclusive(prev.path, path)) { | ||
| return false; | ||
| } | ||
| return true; | ||
| }); | ||
|
|
||
| entries.push({ tag: node.tag, type, path, category: currCategory }); | ||
|
|
||
| if (collides) { | ||
| const nameAttr = findAttr(node, 'name'); | ||
| context.report({ | ||
| node: nameAttr || node, | ||
| messageId: 'duplicate', | ||
| data: { name }, | ||
| }); | ||
| } | ||
| }, | ||
| }; | ||
| }, | ||
| }; | ||
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Uh oh!
There was an error while loading. Please reload this page.