diff --git a/README.md b/README.md index 406ed89301..79c8cec965 100644 --- a/README.md +++ b/README.md @@ -358,6 +358,7 @@ To disable a rule for an entire `.gjs`/`.gts` file, use a regular ESLint file-le | [template-require-form-method](docs/rules/template-require-form-method.md) | require form method attribute | | ๐Ÿ”ง | | | [template-require-has-block-helper](docs/rules/template-require-has-block-helper.md) | require (has-block) helper usage instead of hasBlock property | ๐Ÿ“‹ | ๐Ÿ”ง | | | [template-require-iframe-src-attribute](docs/rules/template-require-iframe-src-attribute.md) | require iframe elements to have src attribute | | ๐Ÿ”ง | | +| [template-require-input-type](docs/rules/template-require-input-type.md) | require input elements to have a valid type attribute | | ๐Ÿ”ง | | | [template-require-splattributes](docs/rules/template-require-splattributes.md) | require splattributes usage in component templates | | | | | [template-require-strict-mode](docs/rules/template-require-strict-mode.md) | require templates to be in strict mode | | | | | [template-require-valid-named-block-naming-format](docs/rules/template-require-valid-named-block-naming-format.md) | require valid named block naming format | ๐Ÿ“‹ | ๐Ÿ”ง | | diff --git a/docs/rules/template-require-input-type.md b/docs/rules/template-require-input-type.md new file mode 100644 index 0000000000..7fc59e28b0 --- /dev/null +++ b/docs/rules/template-require-input-type.md @@ -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). + + + +This rule rejects `` values that are not one of the input +types defined by the HTML spec, and (optionally) requires every `` to +declare a `type` attribute. + +An invalid value like `` 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 (``) 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 + + + +``` + +With `requireExplicit: true` the rule **also forbids**: + +```hbs + + +``` + +This rule **allows** the following: + +```hbs + + + + +``` + +Dynamic values such as `type={{this.inputType}}` are not flagged at lint time. + +## Configuration + +- `requireExplicit` (`boolean`, default `false`): when true, also flag + `` 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). diff --git a/lib/rules/template-require-input-type.js b/lib/rules/template-require-input-type.js new file mode 100644 index 0000000000..efe2377d96 --- /dev/null +++ b/lib/rules/template-require-input-type.js @@ -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 `` elements should have a `type` attribute', + invalid: '`` is not a valid input type', + }, + }, + + create(context) { + // Flagging a missing `type` is a style/consistency check, not a correctness + // one: `` 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 + // `` 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 ``) โ€” 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"'); + }, + }); + } + } + }, + }; + }, +}; diff --git a/lib/utils/is-native-element.js b/lib/utils/is-native-element.js index 190374d9bb..ebdad3b9c4 100644 --- a/lib/utils/is-native-element.js +++ b/lib/utils/is-native-element.js @@ -21,19 +21,17 @@ const ELEMENT_TAGS = new Set([...htmlTags, ...svgTags, ...mathmlTagNames]); * MathML spec registries, reached via the `html-tags` / `svg-tags` / * `mathml-tag-names` packages). It is NOT the same as: * - * - "native accessibility" / "widget-ness" โ€” see `interactive-roles.js` - * (aria-query widget taxonomy; an ARIA-tree-semantics question) - * - "native interactive content" / "focus behavior" โ€” see - * `html-interactive-content.js` (HTML ยง3.2.5.2.7; an HTML-content-model - * question about which tags can be nested inside what) + * - "native accessibility" / "widget-ness" โ€” an ARIA-tree-semantics + * question (for example, whether something maps to a widget role) + * - "native interactive content" / "focus behavior" โ€” an HTML content-model + * question about which elements are considered interactive in the spec * - "natively focusable" / sequential-focus โ€” see HTML ยง6.6.3 * * This util answers only: "is this tag a first-class built-in element of one * of the three markup-language standards, rather than a component invocation - * or a shadowed local binding?" Callers compose it with the other utils - * above when they need a more specific question (see e.g. `template-no- - * noninteractive-tabindex`, which consults both this and - * `html-interactive-content`). + * or a shadowed local binding?" Callers should combine it with whatever + * accessibility, interactivity, or focusability checks they need for more + * specific questions. * * Returns false for: * - components (PascalCase, dotted, @-prefixed, this.-prefixed, ::-namespaced โ€” diff --git a/tests/lib/rules/template-require-input-type.js b/tests/lib/rules/template-require-input-type.js new file mode 100644 index 0000000000..f5c35fcf80 --- /dev/null +++ b/tests/lib/rules/template-require-input-type.js @@ -0,0 +1,131 @@ +const rule = require('../../../lib/rules/template-require-input-type'); +const RuleTester = require('eslint').RuleTester; + +const ERROR_MISSING = 'All `` elements should have a `type` attribute'; +const errInvalid = (value) => `\`\` is not a valid input type`; + +const validHbs = [ + '', + '', + '', + '', + '', + '', + '', + '
', + '
', + '', + // Default (requireExplicit=false): missing `type` is allowed. + '', + '', +]; + +const invalidHbs = [ + { + code: '', + output: '', + errors: [{ message: errInvalid('') }], + }, + { + code: '', + output: '', + errors: [{ message: errInvalid('foo') }], + }, + { + code: '', + output: '', + 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: '', + output: '', + errors: [{ message: errInvalid('') }], + }, +]; + +const requireExplicitInvalid = [ + { + code: '', + options: [{ requireExplicit: true }], + output: '', + errors: [{ message: ERROR_MISSING }], + }, + { + code: '', + options: [{ requireExplicit: true }], + output: '', + errors: [{ message: ERROR_MISSING }], + }, + { + code: '', + options: [{ requireExplicit: true }], + output: '', + errors: [{ message: ERROR_MISSING }], + }, +]; + +const requireExplicitValid = [ + // With requireExplicit: an explicit known type satisfies the rule. + { code: '', options: [{ requireExplicit: true }] }, + // Dynamic type also satisfies โ€” we can't know the runtime value. + { code: '', options: [{ requireExplicit: true }] }, +]; + +const gjsValid = validHbs.map((code) => ``); +const gjsInvalid = invalidHbs.map(({ code, output, errors }) => ({ + code: ``, + output: ``, + 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: ``, + options, + })), + // Scope-shadowed `input` โ€” the template's `` 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'; +`, + `const input = 'foo'; +`, + // Block-param shadowing โ€” `` binds `input` inside the + // yield block. The inner `` should resolve to the block-param, + // not the native tag. + `import Foo from 'whatever'; +`, + ], + invalid: [ + ...gjsInvalid, + ...requireExplicitInvalid.map(({ code, options, output, errors }) => ({ + code: ``, + options, + output: ``, + 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], +});