Skip to content

Commit 11787d4

Browse files
committed
fix(template-no-invalid-interactive): switch shadowing detection to binding-based + normalize role (Copilot review)
- Byte-identical is-native-element: walk scope.variables up scope.upper instead of matching scope.references, so a bare helper reference like {{div}} that creates an entry in scope.references without binding an identifier does not incorrectly shadow the native <div> tag. Byte-identical to the carrier on PR #50. - Add getRole() helper that trims, lowercases, and takes the first whitespace- separated token — mirrors template-no-nested-interactive / template-no- invalid-role normalization. isInteractive() now calls it so 'BUTTON', ' button ', and 'button link' all resolve to 'button' before INTERACTIVE_ROLES lookup.
1 parent 9c49f26 commit 11787d4

2 files changed

Lines changed: 46 additions & 16 deletions

File tree

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

Lines changed: 19 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,21 @@ function getTextAttr(node, name) {
1212
return undefined;
1313
}
1414

15+
// ARIA role values are case-insensitive and the spec honors the first
16+
// recognised token when a space-separated fallback list is given
17+
// (WAI-ARIA 1.2 role attribute). Normalize via trim + lowercase + split on
18+
// whitespace + pick the first token; callers compare that against
19+
// INTERACTIVE_ROLES. Mirrors the getRole() helper in
20+
// template-no-nested-interactive / template-no-invalid-role for
21+
// consistency.
22+
function getRole(node) {
23+
const raw = getTextAttr(node, 'role');
24+
if (!raw) {
25+
return raw;
26+
}
27+
return raw.trim().toLowerCase().split(/\s+/u)[0];
28+
}
29+
1530
const DISALLOWED_DOM_EVENTS = new Set([
1631
// Mouse events:
1732
'click',
@@ -118,8 +133,10 @@ module.exports = {
118133
return true;
119134
}
120135

121-
// Check role
122-
const role = getTextAttr(node, 'role');
136+
// Check role — normalize via getRole() so that `role="BUTTON"`,
137+
// `role=" button "`, and `role="button link"` all resolve to "button"
138+
// before looking up against INTERACTIVE_ROLES.
139+
const role = getRole(node);
123140
if (role && INTERACTIVE_ROLES.has(role)) {
124141
return true;
125142
}

lib/utils/is-native-element.js

Lines changed: 27 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -21,19 +21,17 @@ const ELEMENT_TAGS = new Set([...htmlTags, ...svgTags, ...mathmlTagNames]);
2121
* MathML spec registries, reached via the `html-tags` / `svg-tags` /
2222
* `mathml-tag-names` packages). It is NOT the same as:
2323
*
24-
* - "native accessibility" / "widget-ness" — see `interactive-roles.js`
25-
* (aria-query widget taxonomy; an ARIA-tree-semantics question)
26-
* - "native interactive content" / "focus behavior" — see
27-
* `html-interactive-content.js` (HTML §3.2.5.2.7; an HTML-content-model
28-
* question about which tags can be nested inside what)
24+
* - "native accessibility" / "widget-ness" — an ARIA-tree-semantics
25+
* question (for example, whether something maps to a widget role)
26+
* - "native interactive content" / "focus behavior" — an HTML content-model
27+
* question about which elements are considered interactive in the spec
2928
* - "natively focusable" / sequential-focus — see HTML §6.6.3
3029
*
3130
* This util answers only: "is this tag a first-class built-in element of one
3231
* of the three markup-language standards, rather than a component invocation
33-
* or a shadowed local binding?" Callers compose it with the other utils
34-
* above when they need a more specific question (see e.g. `template-no-
35-
* noninteractive-tabindex`, which consults both this and
36-
* `html-interactive-content`).
32+
* or a shadowed local binding?" Callers should combine it with whatever
33+
* accessibility, interactivity, or focusability checks they need for more
34+
* specific questions.
3735
*
3836
* Returns false for:
3937
* - components (PascalCase, dotted, @-prefixed, this.-prefixed, ::-namespaced —
@@ -60,16 +58,31 @@ function isNativeElement(node, sourceCode) {
6058
}
6159
const scope = sourceCode.getScope(node.parent);
6260
const firstPart = node.parts && node.parts[0];
63-
// Compare by identifier name rather than AST node object identity — object
64-
// identity isn't guaranteed across parser versions (ember-eslint-parser can
65-
// produce distinct node objects for the same token depending on how the
66-
// scope manager walks the tree), but the resolved `.name` is stable.
67-
if (firstPart && scope.references.some((ref) => ref.identifier?.name === firstPart?.name)) {
61+
// Scope-shadowing detection: treat the tag as a component invocation only
62+
// when its name resolves to an actual BINDING (const / let / import / block-
63+
// param) in the enclosing scope chain — not just any reference. A bare
64+
// reference like `{{div}}` (helper invocation elsewhere in the template)
65+
// creates an entry in `scope.references` without binding a local
66+
// identifier; it must not shadow the native `<div>` tag. Walking upward
67+
// through `scope.upper` catches bindings declared in outer scopes — e.g.
68+
// a module-level `const div = ...` shadowing templates deeper in the file.
69+
if (firstPart && hasBindingInScopeChain(scope, firstPart.name)) {
6870
return false;
6971
}
7072
return true;
7173
}
7274

75+
function hasBindingInScopeChain(scope, name) {
76+
let current = scope;
77+
while (current) {
78+
if (current.variables && current.variables.some((v) => v.name === name)) {
79+
return true;
80+
}
81+
current = current.upper;
82+
}
83+
return false;
84+
}
85+
7386
/**
7487
* Inverse of {@link isNativeElement}. Returns true when the node should NOT
7588
* be treated as a native HTML element — either because it's a component

0 commit comments

Comments
 (0)