Skip to content

Commit 65be394

Browse files
committed
refactor: extract isNativeElement util (fix component-vs-HTML-tag misclassification)
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; <template><div /></template>`), 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 (`<my-element>` — 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 `<Article><span>…</span></Article>` misclassification (PascalCase component containing text, previously treated as an empty heading via `.toLowerCase()` lookup). - `template-no-invalid-interactive` — fixes `<Article role="button">` / `<Article tabindex={{0}}>` 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.
1 parent 24882a3 commit 65be394

6 files changed

Lines changed: 173 additions & 57 deletions

lib/rules/template-no-arguments-for-html-elements.js

Lines changed: 2 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,4 @@
1-
const htmlTags = require('html-tags');
2-
const svgTags = require('svg-tags');
3-
const { mathmlTagNames } = require('mathml-tag-names');
4-
5-
const ELEMENT_TAGS = new Set([...htmlTags, ...svgTags, ...mathmlTagNames]);
1+
const { isNativeElement } = require('../utils/is-native-element');
62

73
/** @type {import('eslint').Rule.RuleModule} */
84
module.exports = {
@@ -33,15 +29,7 @@ module.exports = {
3329

3430
return {
3531
GlimmerElementNode(node) {
36-
if (!ELEMENT_TAGS.has(node.tag)) {
37-
return;
38-
}
39-
40-
// A known HTML/SVG tag can still be a component if it's bound in scope
41-
// (block param, import, local).
42-
const scope = sourceCode.getScope(node.parent);
43-
const isVariable = scope.references.some((ref) => ref.identifier === node.parts[0]);
44-
if (isVariable) {
32+
if (!isNativeElement(node, sourceCode)) {
4533
return;
4634
}
4735

lib/rules/template-no-block-params-for-html-elements.js

Lines changed: 2 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,4 @@
1-
const htmlTags = require('html-tags');
2-
const svgTags = require('svg-tags');
3-
const { mathmlTagNames } = require('mathml-tag-names');
4-
5-
const ELEMENT_TAGS = new Set([...htmlTags, ...svgTags, ...mathmlTagNames]);
1+
const { isNativeElement } = require('../utils/is-native-element');
62

73
/** @type {import('eslint').Rule.RuleModule} */
84
module.exports = {
@@ -33,15 +29,7 @@ module.exports = {
3329

3430
return {
3531
GlimmerElementNode(node) {
36-
if (!ELEMENT_TAGS.has(node.tag)) {
37-
return;
38-
}
39-
40-
// A known HTML/SVG tag can still be a component if it's bound in scope
41-
// (block param, import, local).
42-
const scope = sourceCode.getScope(node.parent);
43-
const isVariable = scope.references.some((ref) => ref.identifier === node.parts[0]);
44-
if (isVariable) {
32+
if (!isNativeElement(node, sourceCode)) {
4533
return;
4634
}
4735

lib/rules/template-no-empty-headings.js

Lines changed: 10 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,5 @@
1+
const { isNativeElement } = require('../utils/is-native-element');
2+
13
const HEADINGS = new Set(['h1', 'h2', 'h3', 'h4', 'h5', 'h6']);
24

35
function isHidden(node) {
@@ -14,28 +16,12 @@ function isHidden(node) {
1416
return false;
1517
}
1618

17-
function isComponent(node) {
18-
if (node.type !== 'GlimmerElementNode') {
19-
return false;
20-
}
21-
const tag = node.tag;
22-
// PascalCase (<MyComponent>), namespaced (<Foo::Bar>), this.-prefixed
23-
// (<this.Component>), arg-prefixed (<@component>), or dot-path (<ns.Widget>)
24-
return (
25-
/^[A-Z]/.test(tag) ||
26-
tag.includes('::') ||
27-
tag.startsWith('this.') ||
28-
tag.startsWith('@') ||
29-
tag.includes('.')
30-
);
31-
}
32-
3319
function isTextEmpty(text) {
3420
// Treat &nbsp; (U+00A0) and regular whitespace as empty
3521
return text.replaceAll(/\s/g, '').replaceAll('&nbsp;', '').length === 0;
3622
}
3723

38-
function hasAccessibleContent(node) {
24+
function hasAccessibleContent(node, sourceCode) {
3925
if (!node.children || node.children.length === 0) {
4026
return false;
4127
}
@@ -61,13 +47,14 @@ function hasAccessibleContent(node) {
6147
continue;
6248
}
6349

64-
// Component invocations count as content (they may render text)
65-
if (isComponent(child)) {
50+
// Component invocations (including custom elements and scope-bound
51+
// identifiers) are opaque — we can't see inside, so assume content.
52+
if (!isNativeElement(child, sourceCode)) {
6653
return true;
6754
}
6855

69-
// Recurse into non-hidden, non-component elements
70-
if (hasAccessibleContent(child)) {
56+
// Recurse into native HTML/SVG/MathML elements.
57+
if (hasAccessibleContent(child, sourceCode)) {
7158
return true;
7259
}
7360
}
@@ -110,6 +97,7 @@ module.exports = {
11097
},
11198
},
11299
create(context) {
100+
const sourceCode = context.sourceCode;
113101
return {
114102
GlimmerElementNode(node) {
115103
if (isHeadingElement(node)) {
@@ -118,7 +106,7 @@ module.exports = {
118106
return;
119107
}
120108

121-
if (!hasAccessibleContent(node)) {
109+
if (!hasAccessibleContent(node, sourceCode)) {
122110
context.report({ node, messageId: 'emptyHeading' });
123111
}
124112
}

lib/rules/template-no-invalid-interactive.js

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,5 @@
1+
const { isNativeElement } = require('../utils/is-native-element');
2+
13
function hasAttr(node, name) {
24
return node.attributes?.some((a) => a.name === name);
35
}
@@ -65,6 +67,7 @@ module.exports = {
6567
},
6668

6769
create(context) {
70+
const sourceCode = context.sourceCode;
6871
const options = context.options[0] || {};
6972
const additionalInteractiveTags = new Set(options.additionalInteractiveTags || []);
7073
const ignoredTags = new Set(options.ignoredTags || []);
@@ -179,13 +182,10 @@ module.exports = {
179182
return;
180183
}
181184

182-
// Skip components (PascalCase, @-prefixed, this.-prefixed, path-based like foo.bar)
183-
if (
184-
/^[A-Z]/.test(node.tag) ||
185-
node.tag.startsWith('@') ||
186-
node.tag.startsWith('this.') ||
187-
node.tag.includes('.')
188-
) {
185+
// Only analyze native HTML / SVG / MathML elements. Skip components
186+
// (including tag names shadowed by in-scope bindings) and custom
187+
// elements — their a11y contracts are author-defined.
188+
if (!isNativeElement(node, sourceCode)) {
189189
return;
190190
}
191191

lib/utils/is-native-element.js

Lines changed: 61 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,61 @@
1+
'use strict';
2+
3+
const htmlTags = require('html-tags');
4+
const svgTags = require('svg-tags');
5+
const { mathmlTagNames } = require('mathml-tag-names');
6+
7+
// Authoritative set of native element tag names. Mirrors the approach
8+
// established by #2689 (template-no-block-params-for-html-elements), which
9+
// the maintainer requires for component-vs-element discrimination in this
10+
// plugin. Heuristic approaches (PascalCase detection, etc.) were explicitly
11+
// rejected there because a lowercase tag CAN be a component in GJS/GTS when
12+
// the name is bound in scope (e.g. `const div = MyComponent; <div />`).
13+
const ELEMENT_TAGS = new Set([...htmlTags, ...svgTags, ...mathmlTagNames]);
14+
15+
/**
16+
* Returns true if the Glimmer element node is a native HTML / SVG / MathML
17+
* element — i.e. the tag name is in the authoritative list AND is not
18+
* shadowed by an in-scope binding.
19+
*
20+
* Returns false for:
21+
* - components (PascalCase, dotted, @-prefixed, this.-prefixed, ::-namespaced —
22+
* none of these tag names appear in the HTML/SVG/MathML lists)
23+
* - custom elements (`<my-widget>`) — accepted false negative; the web-
24+
* components namespace is open and can't be enumerated
25+
* - scope-bound identifiers (`<div>` when `div` is a local `let` / `const` /
26+
* import / block-param in the enclosing scope)
27+
*
28+
* @param {object} node - GlimmerElementNode
29+
* @param {object} [sourceCode] - ESLint SourceCode, for scope lookup. When
30+
* omitted, the scope check is skipped (the result is then list-based only —
31+
* suitable for unit tests).
32+
*/
33+
function isNativeElement(node, sourceCode) {
34+
if (!node || typeof node.tag !== 'string') {
35+
return false;
36+
}
37+
if (!ELEMENT_TAGS.has(node.tag)) {
38+
return false;
39+
}
40+
if (!sourceCode || !node.parent) {
41+
return true;
42+
}
43+
const scope = sourceCode.getScope(node.parent);
44+
const firstPart = node.parts && node.parts[0];
45+
if (firstPart && scope.references.some((ref) => ref.identifier === firstPart)) {
46+
return false;
47+
}
48+
return true;
49+
}
50+
51+
/**
52+
* Inverse of {@link isNativeElement}. Returns true when the node should NOT
53+
* be treated as a native HTML element — either because it's a component
54+
* invocation (PascalCase, dotted, @-prefixed, this.-prefixed, custom element)
55+
* OR a tag name that's shadowed by a scope binding.
56+
*/
57+
function isComponentInvocation(node, sourceCode) {
58+
return !isNativeElement(node, sourceCode);
59+
}
60+
61+
module.exports = { isNativeElement, isComponentInvocation, ELEMENT_TAGS };
Lines changed: 91 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,91 @@
1+
'use strict';
2+
3+
const { isNativeElement, ELEMENT_TAGS } = require('../../../lib/utils/is-native-element');
4+
5+
// Tests exercise the list-lookup path only. Scope-based shadowing is covered
6+
// by the rule-level test suites (see tests/lib/rules/template-no-block-params-
7+
// for-html-elements.js and siblings) because it requires a real ESLint
8+
// SourceCode / scope manager that's only built up by the rule tester.
9+
10+
describe('isNativeElement — list-only behavior (no sourceCode)', () => {
11+
it('returns true for lowercase HTML tag names', () => {
12+
expect(isNativeElement({ tag: 'div' })).toBe(true);
13+
expect(isNativeElement({ tag: 'article' })).toBe(true);
14+
expect(isNativeElement({ tag: 'h1' })).toBe(true);
15+
expect(isNativeElement({ tag: 'button' })).toBe(true);
16+
expect(isNativeElement({ tag: 'form' })).toBe(true);
17+
expect(isNativeElement({ tag: 'section' })).toBe(true);
18+
});
19+
20+
it('returns true for SVG tag names', () => {
21+
expect(isNativeElement({ tag: 'svg' })).toBe(true);
22+
expect(isNativeElement({ tag: 'circle' })).toBe(true);
23+
expect(isNativeElement({ tag: 'path' })).toBe(true);
24+
});
25+
26+
it('returns true for MathML tag names', () => {
27+
expect(isNativeElement({ tag: 'mfrac' })).toBe(true);
28+
expect(isNativeElement({ tag: 'math' })).toBe(true);
29+
});
30+
31+
it('returns false for PascalCase component tags', () => {
32+
expect(isNativeElement({ tag: 'Button' })).toBe(false);
33+
expect(isNativeElement({ tag: 'MyWidget' })).toBe(false);
34+
// Native-tag names in PascalCase — the core bug case.
35+
expect(isNativeElement({ tag: 'Article' })).toBe(false);
36+
expect(isNativeElement({ tag: 'Form' })).toBe(false);
37+
expect(isNativeElement({ tag: 'Main' })).toBe(false);
38+
expect(isNativeElement({ tag: 'Nav' })).toBe(false);
39+
expect(isNativeElement({ tag: 'Section' })).toBe(false);
40+
expect(isNativeElement({ tag: 'Table' })).toBe(false);
41+
});
42+
43+
it('returns false for named-arg invocations', () => {
44+
expect(isNativeElement({ tag: '@heading' })).toBe(false);
45+
expect(isNativeElement({ tag: '@tag.foo' })).toBe(false);
46+
});
47+
48+
it('returns false for this-path invocations', () => {
49+
expect(isNativeElement({ tag: 'this.myComponent' })).toBe(false);
50+
expect(isNativeElement({ tag: 'this.comp.sub' })).toBe(false);
51+
});
52+
53+
it('returns false for dot-path invocations', () => {
54+
expect(isNativeElement({ tag: 'foo.bar' })).toBe(false);
55+
expect(isNativeElement({ tag: 'ns.widget' })).toBe(false);
56+
});
57+
58+
it('returns false for named-block / namespaced invocations', () => {
59+
expect(isNativeElement({ tag: 'foo::bar' })).toBe(false);
60+
expect(isNativeElement({ tag: 'Foo::Bar' })).toBe(false);
61+
});
62+
63+
it('returns false for custom elements (accepted false negative)', () => {
64+
// Custom elements aren't in the html-tags/svg-tags/mathml-tag-names
65+
// allowlists. They're treated as "not a native element" so downstream
66+
// rules skip them — matches the convention established in PR #2689.
67+
expect(isNativeElement({ tag: 'my-element' })).toBe(false);
68+
expect(isNativeElement({ tag: 'x-foo' })).toBe(false);
69+
});
70+
71+
it('returns false for empty / missing / non-string tag', () => {
72+
expect(isNativeElement({ tag: '' })).toBe(false);
73+
expect(isNativeElement({ tag: undefined })).toBe(false);
74+
expect(isNativeElement({ tag: null })).toBe(false);
75+
expect(isNativeElement({ tag: 123 })).toBe(false);
76+
expect(isNativeElement({})).toBe(false);
77+
expect(isNativeElement()).toBe(false);
78+
expect(isNativeElement(null)).toBe(false);
79+
});
80+
});
81+
82+
describe('ELEMENT_TAGS', () => {
83+
it('includes all HTML, SVG, and MathML tag names', () => {
84+
// Sanity check — if this ever drops below a reasonable size, one of the
85+
// underlying packages has changed contract.
86+
expect(ELEMENT_TAGS.size).toBeGreaterThan(200);
87+
expect(ELEMENT_TAGS.has('div')).toBe(true);
88+
expect(ELEMENT_TAGS.has('circle')).toBe(true);
89+
expect(ELEMENT_TAGS.has('mfrac')).toBe(true);
90+
});
91+
});

0 commit comments

Comments
 (0)