Skip to content

Commit 66815af

Browse files
committed
fix(template-no-noninteractive-tabindex): unwrap mustache literals via shared helper (Copilot review)
Extract a new `getStaticAttrValue` util that resolves literal-valued mustaches (`{{"foo"}}`, `{{true}}`, `{{-1}}`) and single-part concat statements (`"{{true}}"`) to their static string value. `getStaticTabindexValue` now delegates to the helper and parses the resolved string with `Number()` — `tabindex="{{-1}}"` and `tabindex={{"-1"}}` resolve to `-1` the same as `tabindex="-1"` and `tabindex={{-1}}`, so the canonical "focusable but not in tab order" exemption applies to all equivalent forms. Byte-identical carrier of lib/utils/static-attr-value.js across all PRs that land it.
1 parent 6641a1f commit 66815af

4 files changed

Lines changed: 206 additions & 16 deletions

File tree

lib/rules/template-no-noninteractive-tabindex.js

Lines changed: 9 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@ const { dom, roles } = require('aria-query');
22
const { isNativeElement } = require('../utils/is-native-element');
33
const { isHtmlInteractiveContent } = require('../utils/html-interactive-content');
44
const { INTERACTIVE_ROLES } = require('../utils/interactive-roles');
5+
const { getStaticAttrValue } = require('../utils/static-attr-value');
56

67
function findAttr(node, name) {
78
// HTML attribute names are case-insensitive. Normalize both sides so
@@ -12,24 +13,16 @@ function findAttr(node, name) {
1213
}
1314

1415
function getStaticTabindexValue(attr) {
15-
const value = attr?.value;
16-
if (!value) {
16+
// Resolve the attribute's static string via the shared helper — covers
17+
// plain text (`tabindex="-1"`), mustache literals (`tabindex={{-1}}` /
18+
// `tabindex={{"-1"}}`), and single-part quoted-mustache concat
19+
// (`tabindex="{{-1}}"`). Dynamic paths yield `undefined`.
20+
const resolved = getStaticAttrValue(attr?.value);
21+
if (resolved === undefined || resolved === '') {
1722
return undefined;
1823
}
19-
if (value.type === 'GlimmerTextNode') {
20-
const parsed = Number.parseInt(value.chars, 10);
21-
return Number.isFinite(parsed) ? parsed : undefined;
22-
}
23-
if (value.type === 'GlimmerMustacheStatement' && value.path) {
24-
if (value.path.type === 'GlimmerNumberLiteral') {
25-
return value.path.value;
26-
}
27-
if (value.path.type === 'GlimmerStringLiteral') {
28-
const parsed = Number.parseInt(value.path.value, 10);
29-
return Number.isFinite(parsed) ? parsed : undefined;
30-
}
31-
}
32-
return undefined;
24+
const parsed = Number(resolved);
25+
return Number.isFinite(parsed) ? parsed : undefined;
3326
}
3427

3528
function getTextAttrValue(node, name) {

lib/utils/static-attr-value.js

Lines changed: 64 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,64 @@
1+
'use strict';
2+
3+
/**
4+
* Return the statically-known string value of a Glimmer attribute value node,
5+
* or `undefined` when the value is dynamic (cannot be resolved at lint time).
6+
*
7+
* Unwraps:
8+
* - GlimmerTextNode → chars
9+
* - GlimmerMustacheStatement with a literal path (boolean/string/number) → stringified value
10+
* - GlimmerConcatStatement whose parts are all statically resolvable → joined string
11+
*
12+
* A missing/undefined value (valueless attribute, e.g. `<input disabled>`)
13+
* returns the empty string. Pass `attr.value` — not the attribute itself.
14+
*/
15+
function getStaticAttrValue(value) {
16+
if (value === null || value === undefined) {
17+
return '';
18+
}
19+
if (value.type === 'GlimmerTextNode') {
20+
return value.chars;
21+
}
22+
if (value.type === 'GlimmerMustacheStatement') {
23+
return extractLiteral(value.path);
24+
}
25+
if (value.type === 'GlimmerConcatStatement') {
26+
const parts = value.parts || [];
27+
let out = '';
28+
for (const part of parts) {
29+
if (part.type === 'GlimmerTextNode') {
30+
out += part.chars;
31+
continue;
32+
}
33+
if (part.type === 'GlimmerMustacheStatement') {
34+
const literal = extractLiteral(part.path);
35+
if (literal === undefined) {
36+
return undefined;
37+
}
38+
out += literal;
39+
continue;
40+
}
41+
return undefined;
42+
}
43+
return out;
44+
}
45+
return undefined;
46+
}
47+
48+
function extractLiteral(path) {
49+
if (!path) {
50+
return undefined;
51+
}
52+
if (path.type === 'GlimmerBooleanLiteral') {
53+
return path.value ? 'true' : 'false';
54+
}
55+
if (path.type === 'GlimmerStringLiteral') {
56+
return path.value;
57+
}
58+
if (path.type === 'GlimmerNumberLiteral') {
59+
return String(path.value);
60+
}
61+
return undefined;
62+
}
63+
64+
module.exports = { getStaticAttrValue };

tests/lib/rules/template-no-noninteractive-tabindex.js

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -81,6 +81,10 @@ ruleTester.run('template-no-noninteractive-tabindex', rule, {
8181
'<template><span tabindex="-1">text</span></template>',
8282
'<template><section tabindex="-1">scroll target</section></template>',
8383
'<template><div tabindex={{-1}}></div></template>',
84+
// Quoted-mustache (GlimmerConcatStatement) form resolves to the same
85+
// static `-1` via the shared `getStaticAttrValue` helper.
86+
'<template><div tabindex="{{-1}}"></div></template>',
87+
'<template><div tabindex={{"-1"}}></div></template>',
8488

8589
// WAI-ARIA 1.2 §4.1 role fallback: unrecognized first token is skipped,
8690
// the next recognized token applies. `button` is interactive → allowed.
Lines changed: 129 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,129 @@
1+
'use strict';
2+
3+
const { getStaticAttrValue } = require('../../../lib/utils/static-attr-value');
4+
5+
describe('getStaticAttrValue', () => {
6+
it('returns empty string for null/undefined (valueless attribute)', () => {
7+
expect(getStaticAttrValue(null)).toBe('');
8+
expect(getStaticAttrValue(undefined)).toBe('');
9+
});
10+
11+
it('returns chars for GlimmerTextNode', () => {
12+
expect(getStaticAttrValue({ type: 'GlimmerTextNode', chars: 'hello' })).toBe('hello');
13+
expect(getStaticAttrValue({ type: 'GlimmerTextNode', chars: '' })).toBe('');
14+
});
15+
16+
it('unwraps GlimmerMustacheStatement with BooleanLiteral', () => {
17+
expect(
18+
getStaticAttrValue({
19+
type: 'GlimmerMustacheStatement',
20+
path: { type: 'GlimmerBooleanLiteral', value: true },
21+
})
22+
).toBe('true');
23+
expect(
24+
getStaticAttrValue({
25+
type: 'GlimmerMustacheStatement',
26+
path: { type: 'GlimmerBooleanLiteral', value: false },
27+
})
28+
).toBe('false');
29+
});
30+
31+
it('unwraps GlimmerMustacheStatement with StringLiteral', () => {
32+
expect(
33+
getStaticAttrValue({
34+
type: 'GlimmerMustacheStatement',
35+
path: { type: 'GlimmerStringLiteral', value: 'foo' },
36+
})
37+
).toBe('foo');
38+
expect(
39+
getStaticAttrValue({
40+
type: 'GlimmerMustacheStatement',
41+
path: { type: 'GlimmerStringLiteral', value: '' },
42+
})
43+
).toBe('');
44+
});
45+
46+
it('unwraps GlimmerMustacheStatement with NumberLiteral', () => {
47+
expect(
48+
getStaticAttrValue({
49+
type: 'GlimmerMustacheStatement',
50+
path: { type: 'GlimmerNumberLiteral', value: -1 },
51+
})
52+
).toBe('-1');
53+
expect(
54+
getStaticAttrValue({
55+
type: 'GlimmerMustacheStatement',
56+
path: { type: 'GlimmerNumberLiteral', value: 0 },
57+
})
58+
).toBe('0');
59+
});
60+
61+
it('returns undefined for GlimmerMustacheStatement with a dynamic PathExpression', () => {
62+
expect(
63+
getStaticAttrValue({
64+
type: 'GlimmerMustacheStatement',
65+
path: { type: 'GlimmerPathExpression', original: 'this.foo' },
66+
})
67+
).toBeUndefined();
68+
});
69+
70+
it('joins GlimmerConcatStatement with only static parts', () => {
71+
expect(
72+
getStaticAttrValue({
73+
type: 'GlimmerConcatStatement',
74+
parts: [
75+
{ type: 'GlimmerTextNode', chars: 'prefix-' },
76+
{
77+
type: 'GlimmerMustacheStatement',
78+
path: { type: 'GlimmerStringLiteral', value: 'mid' },
79+
},
80+
{ type: 'GlimmerTextNode', chars: '-suffix' },
81+
],
82+
})
83+
).toBe('prefix-mid-suffix');
84+
});
85+
86+
it('joins concat with boolean and number literal parts', () => {
87+
expect(
88+
getStaticAttrValue({
89+
type: 'GlimmerConcatStatement',
90+
parts: [
91+
{
92+
type: 'GlimmerMustacheStatement',
93+
path: { type: 'GlimmerBooleanLiteral', value: true },
94+
},
95+
],
96+
})
97+
).toBe('true');
98+
expect(
99+
getStaticAttrValue({
100+
type: 'GlimmerConcatStatement',
101+
parts: [
102+
{
103+
type: 'GlimmerMustacheStatement',
104+
path: { type: 'GlimmerNumberLiteral', value: -1 },
105+
},
106+
],
107+
})
108+
).toBe('-1');
109+
});
110+
111+
it('returns undefined for GlimmerConcatStatement with a dynamic part', () => {
112+
expect(
113+
getStaticAttrValue({
114+
type: 'GlimmerConcatStatement',
115+
parts: [
116+
{ type: 'GlimmerTextNode', chars: 'x-' },
117+
{
118+
type: 'GlimmerMustacheStatement',
119+
path: { type: 'GlimmerPathExpression', original: 'this.foo' },
120+
},
121+
],
122+
})
123+
).toBeUndefined();
124+
});
125+
126+
it('returns undefined for an unknown node type', () => {
127+
expect(getStaticAttrValue({ type: 'GlimmerSubExpression' })).toBeUndefined();
128+
});
129+
});

0 commit comments

Comments
 (0)