forked from ember-cli/eslint-plugin-ember
-
Notifications
You must be signed in to change notification settings - Fork 0
Add template-require-input-type: reject invalid input types; opt-in requireExplicit
#40
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
Draft
johanrd
wants to merge
4
commits into
master
Choose a base branch
from
html-validate/template-require-input-type
base: master
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Draft
Changes from all commits
Commits
Show all changes
4 commits
Select commit
Hold shift + click to select a range
148a2a3
feat: add template-require-input-type — reject invalid input types; o…
johanrd 755726c
fix: template-require-input-type — tests for valueless type/shadowing…
johanrd 983e906
fix(template-require-input-type): normalize attr.name to lowercase be…
johanrd 307e2b7
test(template-require-input-type): clarify why autofix collapses the …
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
Some comments aren't visible on the classic Files Changed page.
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,66 @@ | ||
| # ember/template-require-input-type | ||
|
|
||
| 🔧 This rule is automatically fixable by the [`--fix` CLI option](https://eslint.org/docs/latest/user-guide/command-line-interface#--fix). | ||
|
|
||
| <!-- end auto-generated rule header --> | ||
|
|
||
| This rule rejects `<input type="...">` values that are not one of the input | ||
| types defined by the HTML spec, and (optionally) requires every `<input>` to | ||
| declare a `type` attribute. | ||
|
|
||
| An invalid value like `<input type="foo">` silently falls back to the Text | ||
| state — the browser reports no error, but the author's intent (validation, | ||
| inputmode hint, platform keyboard) is lost. That's a genuine silent-failure | ||
| class, which this rule always flags and auto-fixes to `type="text"`. | ||
|
|
||
| A missing `type` attribute (`<input />`) is _spec-compliant_ — the | ||
| missing-value default is the Text state — so flagging it is a style / | ||
| consistency choice, not a correctness one. Opt in with `requireExplicit: true` | ||
| if your team wants parity with `template-require-button-type`. | ||
|
|
||
| ## Examples | ||
|
|
||
| This rule **forbids** the following (always): | ||
|
|
||
| ```hbs | ||
| <input type='' /> | ||
| <input type='foo' /> | ||
| <input type='TEXTY' /> | ||
| ``` | ||
|
|
||
| With `requireExplicit: true` the rule **also forbids**: | ||
|
|
||
| ```hbs | ||
| <input /> | ||
| <input name='email' /> | ||
| ``` | ||
|
|
||
| This rule **allows** the following: | ||
|
|
||
| ```hbs | ||
| <input type='text' /> | ||
| <input type='email' /> | ||
| <input type='checkbox' /> | ||
| <input type={{this.inputType}} /> | ||
| ``` | ||
|
|
||
| Dynamic values such as `type={{this.inputType}}` are not flagged at lint time. | ||
|
|
||
| ## Configuration | ||
|
|
||
| - `requireExplicit` (`boolean`, default `false`): when true, also flag | ||
| `<input>` elements that have no `type` attribute. Auto-fix inserts | ||
| `type="text"`. | ||
|
|
||
| ```js | ||
| module.exports = { | ||
| rules: { | ||
| 'ember/template-require-input-type': ['error', { requireExplicit: true }], | ||
| }, | ||
| }; | ||
| ``` | ||
|
|
||
| ## References | ||
|
|
||
| - [HTML spec — the input element](https://html.spec.whatwg.org/multipage/input.html#the-input-element) | ||
| - Adapted from [`html-validate`'s `no-implicit-input-type`](https://html-validate.org/rules/no-implicit-input-type.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,145 @@ | ||
| 'use strict'; | ||
|
|
||
| // See html-validate (https://html-validate.org) for the peer rule concept. | ||
|
|
||
| const { isNativeElement } = require('../utils/is-native-element'); | ||
|
|
||
| const VALID_TYPES = new Set([ | ||
| 'button', | ||
| 'checkbox', | ||
| 'color', | ||
| 'date', | ||
| 'datetime-local', | ||
| 'email', | ||
| 'file', | ||
| 'hidden', | ||
| 'image', | ||
| 'month', | ||
| 'number', | ||
| 'password', | ||
| 'radio', | ||
| 'range', | ||
| 'reset', | ||
| 'search', | ||
| 'submit', | ||
| 'tel', | ||
| 'text', | ||
| 'time', | ||
| 'url', | ||
| 'week', | ||
| ]); | ||
|
|
||
| /** @type {import('eslint').Rule.RuleModule} */ | ||
| module.exports = { | ||
| meta: { | ||
| type: 'problem', | ||
| docs: { | ||
| description: 'require input elements to have a valid type attribute', | ||
| category: 'Best Practices', | ||
| url: 'https://github.com/ember-cli/eslint-plugin-ember/tree/master/docs/rules/template-require-input-type.md', | ||
| templateMode: 'both', | ||
| }, | ||
| fixable: 'code', | ||
| schema: [ | ||
| { | ||
| type: 'object', | ||
| properties: { | ||
| requireExplicit: { | ||
| type: 'boolean', | ||
| }, | ||
| }, | ||
| additionalProperties: false, | ||
| }, | ||
| ], | ||
| messages: { | ||
| missing: 'All `<input>` elements should have a `type` attribute', | ||
| invalid: '`<input type="{{value}}">` is not a valid input type', | ||
| }, | ||
| }, | ||
|
|
||
| create(context) { | ||
| // Flagging a missing `type` is a style/consistency check, not a correctness | ||
| // one: `<input>` without `type` is spec-compliant (defaults to the Text | ||
| // state). Opt-in so teams that want parity with template-require-button- | ||
| // type can enable it without imposing it on others. | ||
| const requireExplicit = Boolean(context.options[0]?.requireExplicit); | ||
| const sourceCode = context.sourceCode || context.getSourceCode(); | ||
|
|
||
| return { | ||
| GlimmerElementNode(node) { | ||
| if (node.tag !== 'input') { | ||
| return; | ||
| } | ||
| // In strict GJS, a lowercase local binding can shadow the native | ||
| // `<input>` element. `isNativeElement` consults html/svg/mathml tag | ||
| // lists and checks bindings in the scope chain to filter out | ||
| // scope-shadowed cases. | ||
| if (!isNativeElement(node, sourceCode)) { | ||
| return; | ||
| } | ||
|
|
||
| const typeAttr = node.attributes?.find((attr) => attr.name.toLowerCase() === 'type'); | ||
|
|
||
| if (!typeAttr) { | ||
| if (!requireExplicit) { | ||
| return; | ||
| } | ||
| context.report({ | ||
| node, | ||
| messageId: 'missing', | ||
| fix(fixer) { | ||
| // Insert right after `<input` so the new attribute is the first | ||
| // one — avoids the fragile "find end of open tag" regex that can | ||
| // mis-place the attribute past the `/` in self-closing syntax. | ||
| const insertPos = node.range[0] + '<input'.length; | ||
| return fixer.insertTextBeforeRange([insertPos, insertPos], ' type="text"'); | ||
| }, | ||
| }); | ||
| return; | ||
| } | ||
|
|
||
| const value = typeAttr.value; | ||
|
|
||
| // Valueless attribute form (`<input type />`) — per HTML spec, a | ||
| // present-but-empty type attribute resolves to the missing-value | ||
| // default ("Text state"). That's the same runtime result as | ||
| // `type=""`, which we already flag. Treat them consistently: | ||
| // flag as invalid('') and autofix to `type="text"`. | ||
| if (!value) { | ||
| context.report({ | ||
| node: typeAttr, | ||
| messageId: 'invalid', | ||
| data: { value: '' }, | ||
| fix(fixer) { | ||
| return fixer.replaceText(typeAttr, 'type="text"'); | ||
| }, | ||
| }); | ||
| return; | ||
| } | ||
|
|
||
| if (value.type === 'GlimmerTextNode') { | ||
| const typeValue = value.chars.toLowerCase(); | ||
| if (typeValue === '') { | ||
| context.report({ | ||
| node: typeAttr, | ||
| messageId: 'invalid', | ||
| data: { value: '' }, | ||
| fix(fixer) { | ||
| return fixer.replaceText(typeAttr, 'type="text"'); | ||
| }, | ||
| }); | ||
| } else if (!VALID_TYPES.has(typeValue)) { | ||
| context.report({ | ||
| node: typeAttr, | ||
| messageId: 'invalid', | ||
| data: { value: value.chars }, | ||
| fix(fixer) { | ||
| return fixer.replaceText(typeAttr, 'type="text"'); | ||
| }, | ||
| }); | ||
| } | ||
| } | ||
| }, | ||
| }; | ||
| }, | ||
| }; |
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,131 @@ | ||
| const rule = require('../../../lib/rules/template-require-input-type'); | ||
| const RuleTester = require('eslint').RuleTester; | ||
|
|
||
| const ERROR_MISSING = 'All `<input>` elements should have a `type` attribute'; | ||
| const errInvalid = (value) => `\`<input type="${value}">\` is not a valid input type`; | ||
|
|
||
| const validHbs = [ | ||
| '<input type="text" />', | ||
| '<input type="email" />', | ||
| '<input type="checkbox" />', | ||
| '<input type="submit" />', | ||
| '<input type="datetime-local" />', | ||
| '<input type="{{this.inputType}}" />', | ||
| '<input type={{this.inputType}} />', | ||
| '<div />', | ||
| '<div type="foo" />', | ||
| '<MyInput type="unknown" />', | ||
| // Default (requireExplicit=false): missing `type` is allowed. | ||
| '<input />', | ||
| '<input name="email" />', | ||
| ]; | ||
|
|
||
| const invalidHbs = [ | ||
| { | ||
| code: '<input type="" />', | ||
| output: '<input type="text" />', | ||
| errors: [{ message: errInvalid('') }], | ||
| }, | ||
| { | ||
| code: '<input type="foo" />', | ||
| output: '<input type="text" />', | ||
| errors: [{ message: errInvalid('foo') }], | ||
| }, | ||
| { | ||
| code: '<input type="TEXTY" />', | ||
| output: '<input type="text" />', | ||
| errors: [{ message: errInvalid('TEXTY') }], | ||
| }, | ||
| // Valueless type attribute — per HTML spec resolves to the missing-value | ||
| // default (Text state — https://html.spec.whatwg.org/multipage/input.html#text-(type=text)-state-and-search-state-(type=search)), | ||
| // same runtime result as `type=""`. Flag and autofix to `type="text"`. The | ||
| // Glimmer AST attribute range extends to the next attribute or tag-close | ||
| // boundary, so `replaceText` swallows the trailing space before `/>`. A | ||
| // formatter can restore preferred spacing. | ||
| { | ||
| code: '<input type />', | ||
| output: '<input type="text"/>', | ||
| errors: [{ message: errInvalid('') }], | ||
| }, | ||
| ]; | ||
|
johanrd marked this conversation as resolved.
|
||
|
|
||
| const requireExplicitInvalid = [ | ||
| { | ||
| code: '<input />', | ||
| options: [{ requireExplicit: true }], | ||
| output: '<input type="text" />', | ||
| errors: [{ message: ERROR_MISSING }], | ||
| }, | ||
| { | ||
| code: '<input name="email" />', | ||
| options: [{ requireExplicit: true }], | ||
| output: '<input type="text" name="email" />', | ||
| errors: [{ message: ERROR_MISSING }], | ||
| }, | ||
| { | ||
| code: '<input name="email" />', | ||
| options: [{ requireExplicit: true }], | ||
| output: '<input type="text" name="email" />', | ||
| errors: [{ message: ERROR_MISSING }], | ||
| }, | ||
| ]; | ||
|
|
||
| const requireExplicitValid = [ | ||
| // With requireExplicit: an explicit known type satisfies the rule. | ||
| { code: '<input type="text" />', options: [{ requireExplicit: true }] }, | ||
| // Dynamic type also satisfies — we can't know the runtime value. | ||
| { code: '<input type={{this.inputType}} />', options: [{ requireExplicit: true }] }, | ||
| ]; | ||
|
|
||
| const gjsValid = validHbs.map((code) => `<template>${code}</template>`); | ||
| const gjsInvalid = invalidHbs.map(({ code, output, errors }) => ({ | ||
| code: `<template>${code}</template>`, | ||
| output: `<template>${output}</template>`, | ||
| errors, | ||
| })); | ||
|
|
||
| const gjsRuleTester = new RuleTester({ | ||
| parser: require.resolve('ember-eslint-parser'), | ||
| parserOptions: { ecmaVersion: 2022, sourceType: 'module' }, | ||
| }); | ||
|
|
||
| gjsRuleTester.run('template-require-input-type', rule, { | ||
| valid: [ | ||
| ...gjsValid, | ||
| ...requireExplicitValid.map(({ code, options }) => ({ | ||
| code: `<template>${code}</template>`, | ||
| options, | ||
| })), | ||
| // Scope-shadowed `input` — the template's `<input>` refers to the local | ||
| // const binding (a component), not the native HTML element. The rule | ||
| // skips it via `isNativeElement`'s scope check. | ||
| `const input = 'foo'; | ||
| <template><input type="not-a-valid-type" /></template>`, | ||
| `const input = 'foo'; | ||
| <template><input /></template>`, | ||
| // Block-param shadowing — `<Foo as |input|>` binds `input` inside the | ||
| // yield block. The inner `<input>` should resolve to the block-param, | ||
| // not the native tag. | ||
| `import Foo from 'whatever'; | ||
| <template><Foo as |input|><input type="not-a-valid-type" /></Foo></template>`, | ||
| ], | ||
| invalid: [ | ||
| ...gjsInvalid, | ||
| ...requireExplicitInvalid.map(({ code, options, output, errors }) => ({ | ||
| code: `<template>${code}</template>`, | ||
| options, | ||
| output: `<template>${output}</template>`, | ||
| errors, | ||
| })), | ||
| ], | ||
| }); | ||
|
|
||
| const hbsRuleTester = new RuleTester({ | ||
| parser: require.resolve('ember-eslint-parser/hbs'), | ||
| parserOptions: { ecmaVersion: 2022, sourceType: 'module' }, | ||
| }); | ||
|
|
||
| hbsRuleTester.run('template-require-input-type', rule, { | ||
| valid: [...validHbs, ...requireExplicitValid], | ||
| invalid: [...invalidHbs, ...requireExplicitInvalid], | ||
| }); | ||
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.