From e350937de23a73a5d7c0311a61fc7b9b29e182ae Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Johan=20R=C3=B8ed?= Date: Tue, 21 Apr 2026 12:37:54 +0200 Subject: [PATCH 1/3] refactor: extract isNativeElement util (fix component-vs-HTML-tag misclassification) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Adds `lib/utils/is-native-element.js` — a shared util for the recurring "is this a native HTML/SVG/MathML element or a component?" question. Uses the authoritative `html-tags` + `svg-tags` + `mathml-tag-names` packages plus scope analysis, mirroring the pattern established by @NullVoxPopuli in #2689 (template-no-block-params-for-html-elements). ## Why not heuristics An earlier draft of this PR used a lexical heuristic (PascalCase / `@` / `this.` / dot / `::`). That approach was rejected in the #2689 discussion — a lowercase tag CAN be a component in GJS/GTS when its name is bound in scope (`const div = MyComponent; `), so heuristics produce both false positives (web components misclassified as native) and false negatives (scope-shadowed tags misclassified as native). Lists + scope handles both correctly. ## What the util does `isNativeElement(node, sourceCode)` returns true iff: - The tag name is in the HTML/SVG/MathML authoritative lists, AND - The tag name is NOT a scope-bound identifier. Returns false for components (PascalCase, dotted, @-prefixed, etc. — none of those tag-name shapes appear in the lists), custom elements (`` — accepted false negative; web component namespace is open-ended), and scope-bound identifiers. ## Rules migrated Four rules now share the util: - `template-no-empty-headings` — fixes the `
…
` misclassification (PascalCase component containing text, previously treated as an empty heading via `.toLowerCase()` lookup). - `template-no-invalid-interactive` — fixes `
` / `
` false positives. - `template-no-arguments-for-html-elements` — previously carried inline copy of the lists+scope pattern (established in #2689); now uses the shared util. - `template-no-block-params-for-html-elements` — same. This is the rule that introduced the pattern, now deduplicated. Net: 2 inline copies of the canonical pattern dropped; 2 buggy rules fixed; +173 -57 lines. --- ...template-no-arguments-for-html-elements.js | 16 +--- ...plate-no-block-params-for-html-elements.js | 16 +--- lib/rules/template-no-empty-headings.js | 32 ++----- lib/rules/template-no-invalid-interactive.js | 14 +-- lib/utils/is-native-element.js | 61 +++++++++++++ tests/lib/rules/template-no-empty-headings.js | 6 ++ .../rules/template-no-invalid-interactive.js | 5 + tests/lib/utils/is-native-element-test.js | 91 +++++++++++++++++++ 8 files changed, 184 insertions(+), 57 deletions(-) create mode 100644 lib/utils/is-native-element.js create mode 100644 tests/lib/utils/is-native-element-test.js diff --git a/lib/rules/template-no-arguments-for-html-elements.js b/lib/rules/template-no-arguments-for-html-elements.js index 4ea227f994..101e2c7232 100644 --- a/lib/rules/template-no-arguments-for-html-elements.js +++ b/lib/rules/template-no-arguments-for-html-elements.js @@ -1,8 +1,4 @@ -const htmlTags = require('html-tags'); -const svgTags = require('svg-tags'); -const { mathmlTagNames } = require('mathml-tag-names'); - -const ELEMENT_TAGS = new Set([...htmlTags, ...svgTags, ...mathmlTagNames]); +const { isNativeElement } = require('../utils/is-native-element'); /** @type {import('eslint').Rule.RuleModule} */ module.exports = { @@ -33,15 +29,7 @@ module.exports = { return { GlimmerElementNode(node) { - if (!ELEMENT_TAGS.has(node.tag)) { - return; - } - - // A known HTML/SVG tag can still be a component if it's bound in scope - // (block param, import, local). - const scope = sourceCode.getScope(node.parent); - const isVariable = scope.references.some((ref) => ref.identifier === node.parts[0]); - if (isVariable) { + if (!isNativeElement(node, sourceCode)) { return; } diff --git a/lib/rules/template-no-block-params-for-html-elements.js b/lib/rules/template-no-block-params-for-html-elements.js index e6792f5d56..711441140b 100644 --- a/lib/rules/template-no-block-params-for-html-elements.js +++ b/lib/rules/template-no-block-params-for-html-elements.js @@ -1,8 +1,4 @@ -const htmlTags = require('html-tags'); -const svgTags = require('svg-tags'); -const { mathmlTagNames } = require('mathml-tag-names'); - -const ELEMENT_TAGS = new Set([...htmlTags, ...svgTags, ...mathmlTagNames]); +const { isNativeElement } = require('../utils/is-native-element'); /** @type {import('eslint').Rule.RuleModule} */ module.exports = { @@ -33,15 +29,7 @@ module.exports = { return { GlimmerElementNode(node) { - if (!ELEMENT_TAGS.has(node.tag)) { - return; - } - - // A known HTML/SVG tag can still be a component if it's bound in scope - // (block param, import, local). - const scope = sourceCode.getScope(node.parent); - const isVariable = scope.references.some((ref) => ref.identifier === node.parts[0]); - if (isVariable) { + if (!isNativeElement(node, sourceCode)) { return; } diff --git a/lib/rules/template-no-empty-headings.js b/lib/rules/template-no-empty-headings.js index edb71caae6..f0fff6f782 100644 --- a/lib/rules/template-no-empty-headings.js +++ b/lib/rules/template-no-empty-headings.js @@ -1,3 +1,5 @@ +const { isNativeElement } = require('../utils/is-native-element'); + const HEADINGS = new Set(['h1', 'h2', 'h3', 'h4', 'h5', 'h6']); function isHidden(node) { @@ -14,28 +16,12 @@ function isHidden(node) { return false; } -function isComponent(node) { - if (node.type !== 'GlimmerElementNode') { - return false; - } - const tag = node.tag; - // PascalCase (), namespaced (), this.-prefixed - // (), arg-prefixed (<@component>), or dot-path () - return ( - /^[A-Z]/.test(tag) || - tag.includes('::') || - tag.startsWith('this.') || - tag.startsWith('@') || - tag.includes('.') - ); -} - function isTextEmpty(text) { // Treat   (U+00A0) and regular whitespace as empty return text.replaceAll(/\s/g, '').replaceAll(' ', '').length === 0; } -function hasAccessibleContent(node) { +function hasAccessibleContent(node, sourceCode) { if (!node.children || node.children.length === 0) { return false; } @@ -61,13 +47,14 @@ function hasAccessibleContent(node) { continue; } - // Component invocations count as content (they may render text) - if (isComponent(child)) { + // Component invocations (including custom elements and scope-bound + // identifiers) are opaque — we can't see inside, so assume content. + if (!isNativeElement(child, sourceCode)) { return true; } - // Recurse into non-hidden, non-component elements - if (hasAccessibleContent(child)) { + // Recurse into native HTML/SVG/MathML elements. + if (hasAccessibleContent(child, sourceCode)) { return true; } } @@ -110,6 +97,7 @@ module.exports = { }, }, create(context) { + const sourceCode = context.sourceCode; return { GlimmerElementNode(node) { if (isHeadingElement(node)) { @@ -118,7 +106,7 @@ module.exports = { return; } - if (!hasAccessibleContent(node)) { + if (!hasAccessibleContent(node, sourceCode)) { context.report({ node, messageId: 'emptyHeading' }); } } diff --git a/lib/rules/template-no-invalid-interactive.js b/lib/rules/template-no-invalid-interactive.js index e7ef96d49e..57df493826 100644 --- a/lib/rules/template-no-invalid-interactive.js +++ b/lib/rules/template-no-invalid-interactive.js @@ -1,3 +1,5 @@ +const { isNativeElement } = require('../utils/is-native-element'); + function hasAttr(node, name) { return node.attributes?.some((a) => a.name === name); } @@ -65,6 +67,7 @@ module.exports = { }, create(context) { + const sourceCode = context.sourceCode; const options = context.options[0] || {}; const additionalInteractiveTags = new Set(options.additionalInteractiveTags || []); const ignoredTags = new Set(options.ignoredTags || []); @@ -179,13 +182,10 @@ module.exports = { return; } - // Skip components (PascalCase, @-prefixed, this.-prefixed, path-based like foo.bar) - if ( - /^[A-Z]/.test(node.tag) || - node.tag.startsWith('@') || - node.tag.startsWith('this.') || - node.tag.includes('.') - ) { + // Only analyze native HTML / SVG / MathML elements. Skip components + // (including tag names shadowed by in-scope bindings) and custom + // elements — their a11y contracts are author-defined. + if (!isNativeElement(node, sourceCode)) { return; } diff --git a/lib/utils/is-native-element.js b/lib/utils/is-native-element.js new file mode 100644 index 0000000000..ee5bf4991a --- /dev/null +++ b/lib/utils/is-native-element.js @@ -0,0 +1,61 @@ +'use strict'; + +const htmlTags = require('html-tags'); +const svgTags = require('svg-tags'); +const { mathmlTagNames } = require('mathml-tag-names'); + +// Authoritative set of native element tag names. Mirrors the approach +// established by #2689 (template-no-block-params-for-html-elements), which +// the maintainer requires for component-vs-element discrimination in this +// plugin. Heuristic approaches (PascalCase detection, etc.) were explicitly +// rejected there because a lowercase tag CAN be a component in GJS/GTS when +// the name is bound in scope (e.g. `const div = MyComponent;
`). +const ELEMENT_TAGS = new Set([...htmlTags, ...svgTags, ...mathmlTagNames]); + +/** + * Returns true if the Glimmer element node is a native HTML / SVG / MathML + * element — i.e. the tag name is in the authoritative list AND is not + * shadowed by an in-scope binding. + * + * Returns false for: + * - components (PascalCase, dotted, @-prefixed, this.-prefixed, ::-namespaced — + * none of these tag names appear in the HTML/SVG/MathML lists) + * - custom elements (``) — accepted false negative; the web- + * components namespace is open and can't be enumerated + * - scope-bound identifiers (`
` when `div` is a local `let` / `const` / + * import / block-param in the enclosing scope) + * + * @param {object} node - GlimmerElementNode + * @param {object} [sourceCode] - ESLint SourceCode, for scope lookup. When + * omitted, the scope check is skipped (the result is then list-based only — + * suitable for unit tests). + */ +function isNativeElement(node, sourceCode) { + if (!node || typeof node.tag !== 'string') { + return false; + } + if (!ELEMENT_TAGS.has(node.tag)) { + return false; + } + if (!sourceCode || !node.parent) { + return true; + } + const scope = sourceCode.getScope(node.parent); + const firstPart = node.parts && node.parts[0]; + if (firstPart && scope.references.some((ref) => ref.identifier === firstPart)) { + return false; + } + return true; +} + +/** + * Inverse of {@link isNativeElement}. Returns true when the node should NOT + * be treated as a native HTML element — either because it's a component + * invocation (PascalCase, dotted, @-prefixed, this.-prefixed, custom element) + * OR a tag name that's shadowed by a scope binding. + */ +function isComponentInvocation(node, sourceCode) { + return !isNativeElement(node, sourceCode); +} + +module.exports = { isNativeElement, isComponentInvocation, ELEMENT_TAGS }; diff --git a/tests/lib/rules/template-no-empty-headings.js b/tests/lib/rules/template-no-empty-headings.js index cce6b806da..5355b2e718 100644 --- a/tests/lib/rules/template-no-empty-headings.js +++ b/tests/lib/rules/template-no-empty-headings.js @@ -43,6 +43,12 @@ ruleTester.run('template-no-empty-headings', rule, { '', '', '', + + // Custom elements (hyphenated lowercase) aren't in the html-tags / svg-tags / + // mathml-tag-names allowlists — treated as opaque, assume content. Matches + // the accepted-false-negative convention established in #2689. + '', + '', ], invalid: [ { diff --git a/tests/lib/rules/template-no-invalid-interactive.js b/tests/lib/rules/template-no-invalid-interactive.js index 5f47ee7604..6d95b064ed 100644 --- a/tests/lib/rules/template-no-invalid-interactive.js +++ b/tests/lib/rules/template-no-invalid-interactive.js @@ -106,6 +106,11 @@ ruleTester.run('template-no-invalid-interactive', rule, { code: '', options: [{ ignoredTags: ['div'] }], }, + + // Custom elements (hyphenated lowercase) — accepted false negative per #2689. + // Their a11y contract is author-defined; ESLint can't introspect. + '', + '', ], invalid: [ diff --git a/tests/lib/utils/is-native-element-test.js b/tests/lib/utils/is-native-element-test.js new file mode 100644 index 0000000000..ddf12916c2 --- /dev/null +++ b/tests/lib/utils/is-native-element-test.js @@ -0,0 +1,91 @@ +'use strict'; + +const { isNativeElement, ELEMENT_TAGS } = require('../../../lib/utils/is-native-element'); + +// Tests exercise the list-lookup path only. Scope-based shadowing is covered +// by the rule-level test suites (see tests/lib/rules/template-no-block-params- +// for-html-elements.js and siblings) because it requires a real ESLint +// SourceCode / scope manager that's only built up by the rule tester. + +describe('isNativeElement — list-only behavior (no sourceCode)', () => { + it('returns true for lowercase HTML tag names', () => { + expect(isNativeElement({ tag: 'div' })).toBe(true); + expect(isNativeElement({ tag: 'article' })).toBe(true); + expect(isNativeElement({ tag: 'h1' })).toBe(true); + expect(isNativeElement({ tag: 'button' })).toBe(true); + expect(isNativeElement({ tag: 'form' })).toBe(true); + expect(isNativeElement({ tag: 'section' })).toBe(true); + }); + + it('returns true for SVG tag names', () => { + expect(isNativeElement({ tag: 'svg' })).toBe(true); + expect(isNativeElement({ tag: 'circle' })).toBe(true); + expect(isNativeElement({ tag: 'path' })).toBe(true); + }); + + it('returns true for MathML tag names', () => { + expect(isNativeElement({ tag: 'mfrac' })).toBe(true); + expect(isNativeElement({ tag: 'math' })).toBe(true); + }); + + it('returns false for PascalCase component tags', () => { + expect(isNativeElement({ tag: 'Button' })).toBe(false); + expect(isNativeElement({ tag: 'MyWidget' })).toBe(false); + // Native-tag names in PascalCase — the core bug case. + expect(isNativeElement({ tag: 'Article' })).toBe(false); + expect(isNativeElement({ tag: 'Form' })).toBe(false); + expect(isNativeElement({ tag: 'Main' })).toBe(false); + expect(isNativeElement({ tag: 'Nav' })).toBe(false); + expect(isNativeElement({ tag: 'Section' })).toBe(false); + expect(isNativeElement({ tag: 'Table' })).toBe(false); + }); + + it('returns false for named-arg invocations', () => { + expect(isNativeElement({ tag: '@heading' })).toBe(false); + expect(isNativeElement({ tag: '@tag.foo' })).toBe(false); + }); + + it('returns false for this-path invocations', () => { + expect(isNativeElement({ tag: 'this.myComponent' })).toBe(false); + expect(isNativeElement({ tag: 'this.comp.sub' })).toBe(false); + }); + + it('returns false for dot-path invocations', () => { + expect(isNativeElement({ tag: 'foo.bar' })).toBe(false); + expect(isNativeElement({ tag: 'ns.widget' })).toBe(false); + }); + + it('returns false for named-block / namespaced invocations', () => { + expect(isNativeElement({ tag: 'foo::bar' })).toBe(false); + expect(isNativeElement({ tag: 'Foo::Bar' })).toBe(false); + }); + + it('returns false for custom elements (accepted false negative)', () => { + // Custom elements aren't in the html-tags/svg-tags/mathml-tag-names + // allowlists. They're treated as "not a native element" so downstream + // rules skip them — matches the convention established in PR #2689. + expect(isNativeElement({ tag: 'my-element' })).toBe(false); + expect(isNativeElement({ tag: 'x-foo' })).toBe(false); + }); + + it('returns false for empty / missing / non-string tag', () => { + expect(isNativeElement({ tag: '' })).toBe(false); + expect(isNativeElement({ tag: undefined })).toBe(false); + expect(isNativeElement({ tag: null })).toBe(false); + expect(isNativeElement({ tag: 123 })).toBe(false); + expect(isNativeElement({})).toBe(false); + expect(isNativeElement()).toBe(false); + expect(isNativeElement(null)).toBe(false); + }); +}); + +describe('ELEMENT_TAGS', () => { + it('includes all HTML, SVG, and MathML tag names', () => { + // Sanity check — if this ever drops below a reasonable size, one of the + // underlying packages has changed contract. + expect(ELEMENT_TAGS.size).toBeGreaterThan(200); + expect(ELEMENT_TAGS.has('div')).toBe(true); + expect(ELEMENT_TAGS.has('circle')).toBe(true); + expect(ELEMENT_TAGS.has('mfrac')).toBe(true); + }); +}); From 2faeef5f3439392867e2f8d1e5fc65f9b94fdf88 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Johan=20R=C3=B8ed?= Date: Wed, 22 Apr 2026 12:49:37 +0200 Subject: [PATCH 2/3] docs(is-native-element): disambiguate 'native' in the JSDoc MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 'Native' is overloaded in the web platform context. The util name remains isNativeElement (common convention in React/Vue/Angular ecosystems for 'platform-provided, not a component'), but the JSDoc now explicitly names three alternative meanings of 'native' that this util does NOT answer: - 'native accessibility' / 'widget-ness' — interactive-roles.js (aria-query widget taxonomy) - 'native interactive content' — html-interactive-content.js (HTML §3.2.5.2.7 content-model question) - 'natively focusable' — HTML §6.6.3 sequential focus navigation This util answers only: is this tag a first-class built-in element of HTML/SVG/MathML, rather than a component invocation or a scope-shadowed local binding? Callers compose it with the more specific utils when they need a narrower question. --- lib/utils/is-native-element.js | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) diff --git a/lib/utils/is-native-element.js b/lib/utils/is-native-element.js index ee5bf4991a..398d87b4a0 100644 --- a/lib/utils/is-native-element.js +++ b/lib/utils/is-native-element.js @@ -17,6 +17,24 @@ const ELEMENT_TAGS = new Set([...htmlTags, ...svgTags, ...mathmlTagNames]); * element — i.e. the tag name is in the authoritative list AND is not * shadowed by an in-scope binding. * + * "Native" here means **spec-registered tag name** (in the HTML, SVG, or + * 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) + * - "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`). + * * Returns false for: * - components (PascalCase, dotted, @-prefixed, this.-prefixed, ::-namespaced — * none of these tag names appear in the HTML/SVG/MathML lists) From 6e6037e8f33fc2b1e6a185fcdbb522f90acf94ae Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Johan=20R=C3=B8ed?= Date: Wed, 22 Apr 2026 14:21:01 +0200 Subject: [PATCH 3/3] fix: sourceCode fallback + name-based scope-ref compare + soften size assertion (Copilot review) --- lib/rules/template-no-empty-headings.js | 4 +++- lib/rules/template-no-invalid-interactive.js | 4 +++- lib/utils/is-native-element.js | 6 +++++- tests/lib/utils/is-native-element-test.js | 9 ++++++--- 4 files changed, 17 insertions(+), 6 deletions(-) diff --git a/lib/rules/template-no-empty-headings.js b/lib/rules/template-no-empty-headings.js index f0fff6f782..e5ea94c7a9 100644 --- a/lib/rules/template-no-empty-headings.js +++ b/lib/rules/template-no-empty-headings.js @@ -97,7 +97,9 @@ module.exports = { }, }, create(context) { - const sourceCode = context.sourceCode; + // `context.sourceCode` is the ESLint >= 8.40 shape; `context.getSourceCode()` + // covers older versions. Keep both for cross-version compatibility. + const sourceCode = context.sourceCode || context.getSourceCode(); return { GlimmerElementNode(node) { if (isHeadingElement(node)) { diff --git a/lib/rules/template-no-invalid-interactive.js b/lib/rules/template-no-invalid-interactive.js index 57df493826..2415f36118 100644 --- a/lib/rules/template-no-invalid-interactive.js +++ b/lib/rules/template-no-invalid-interactive.js @@ -67,7 +67,9 @@ module.exports = { }, create(context) { - const sourceCode = context.sourceCode; + // `context.sourceCode` is the ESLint >= 8.40 shape; `context.getSourceCode()` + // covers older versions. Keep both for cross-version compatibility. + const sourceCode = context.sourceCode || context.getSourceCode(); const options = context.options[0] || {}; const additionalInteractiveTags = new Set(options.additionalInteractiveTags || []); const ignoredTags = new Set(options.ignoredTags || []); diff --git a/lib/utils/is-native-element.js b/lib/utils/is-native-element.js index 398d87b4a0..182196dd62 100644 --- a/lib/utils/is-native-element.js +++ b/lib/utils/is-native-element.js @@ -60,7 +60,11 @@ function isNativeElement(node, sourceCode) { } const scope = sourceCode.getScope(node.parent); const firstPart = node.parts && node.parts[0]; - if (firstPart && scope.references.some((ref) => ref.identifier === firstPart)) { + // Compare by identifier name rather than AST node object identity — object + // identity isn't guaranteed across parser versions (ember-eslint-parser can + // produce distinct node objects for the same token depending on how the + // scope manager walks the tree), but the resolved `.name` is stable. + if (firstPart && scope.references.some((ref) => ref.identifier?.name === firstPart?.name)) { return false; } return true; diff --git a/tests/lib/utils/is-native-element-test.js b/tests/lib/utils/is-native-element-test.js index ddf12916c2..31ca5f5c11 100644 --- a/tests/lib/utils/is-native-element-test.js +++ b/tests/lib/utils/is-native-element-test.js @@ -81,9 +81,12 @@ describe('isNativeElement — list-only behavior (no sourceCode)', () => { describe('ELEMENT_TAGS', () => { it('includes all HTML, SVG, and MathML tag names', () => { - // Sanity check — if this ever drops below a reasonable size, one of the - // underlying packages has changed contract. - expect(ELEMENT_TAGS.size).toBeGreaterThan(200); + // Contract check — the set must be non-empty and must contain at least + // one representative tag from each of the three source packages. An exact + // size assertion would be brittle (the underlying packages add/remove tags + // across minor releases without changing their contract), so we assert the + // shape instead. + expect(ELEMENT_TAGS.size).toBeGreaterThan(0); expect(ELEMENT_TAGS.has('div')).toBe(true); expect(ELEMENT_TAGS.has('circle')).toBe(true); expect(ELEMENT_TAGS.has('mfrac')).toBe(true);