Skip to content

Commit 67de7c7

Browse files
committed
fix(template-require-valid-alt-text): 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. `hasNonEmptyTextAttr` now delegates to the helper — `aria-label={{""}}` / `aria-label="{{""}}"` normalise to the empty string and flag the same as the text-node equivalent; genuinely dynamic values (PathExpressions, multi-part concat) still short-circuit to "assume truthy". Closes the bypass where authors wrapped an empty accessible name in mustaches. Byte-identical carrier of lib/utils/static-attr-value.js across all PRs that land it.
1 parent 2abfbea commit 67de7c7

4 files changed

Lines changed: 219 additions & 4 deletions

File tree

lib/rules/template-require-valid-alt-text.js

Lines changed: 11 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,5 @@
1+
const { getStaticAttrValue } = require('../utils/static-attr-value');
2+
13
const REDUNDANT_WORDS = ['image', 'photo', 'picture', 'logo', 'spacer'];
24

35
function findAttr(node, name) {
@@ -25,11 +27,16 @@ function hasNonEmptyTextAttr(node, name) {
2527
if (!attr?.value) {
2628
return false;
2729
}
28-
if (attr.value.type === 'GlimmerTextNode') {
29-
return attr.value.chars.trim() !== '';
30+
// Resolve mustache-literal / single-part concat forms to their static
31+
// string via the shared helper. `aria-label={{""}}` / `aria-label="{{""}}"`
32+
// now normalise to the empty string and are treated the same as the
33+
// text-node empty value.
34+
const resolved = getStaticAttrValue(attr.value);
35+
if (resolved === undefined) {
36+
// Genuinely dynamic — assume truthy (can't verify at lint time).
37+
return true;
3038
}
31-
// Mustache / concat — dynamic; assume truthy.
32-
return true;
39+
return resolved.trim() !== '';
3340
}
3441

3542
function hasAnyNonEmptyTextAttr(node, names) {

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-require-valid-alt-text.js

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -57,6 +57,9 @@ ruleTester.run('template-require-valid-alt-text', rule, {
5757
'<template><area aria-labelledby="some-alt"></template>',
5858
'<template><area aria-label="some-alt"></template>',
5959
'<template><img role={{unless this.altText "presentation"}} alt={{this.altText}}></template>',
60+
// Mustache-string-literal forms resolve to their static value — a
61+
// non-empty literal supplies an accessible name the same as a text node.
62+
'<template><input type="image" aria-label={{"valid"}} /></template>',
6063
],
6164
invalid: [
6265
// Empty-string aria-label / aria-labelledby / alt provides no accessible
@@ -125,6 +128,18 @@ ruleTester.run('template-require-valid-alt-text', rule, {
125128
output: null,
126129
errors: [{ messageId: 'areaMissing' }],
127130
},
131+
// Mustache-string-literal forms that resolve to an empty string are
132+
// treated the same as the text-node empty value — no accessible name.
133+
{
134+
code: '<template><input type="image" aria-label={{""}} /></template>',
135+
output: null,
136+
errors: [{ messageId: 'inputImage' }],
137+
},
138+
{
139+
code: '<template><input type="image" aria-label="{{""}}" /></template>',
140+
output: null,
141+
errors: [{ messageId: 'inputImage' }],
142+
},
128143
{
129144
code: '<template><img src="/test.jpg" /></template>',
130145
output: null,
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)