Skip to content

Commit b71e729

Browse files
johanrdjohanrd
committed
Fix template-require-iframe-title: split messageIds; report each duplicate with index
Mirror upstream's diagnostic: 4 messageIds (missingTitle, emptyTitle, dynamicFalseTitle, duplicateTitle). Every iframe with a duplicate title is reported with cross-index, matching upstream's behavior for 3+ duplicates (port previously reported the first occurrence only once).
1 parent 56f913d commit b71e729

2 files changed

Lines changed: 126 additions & 17 deletions

File tree

‎lib/rules/template-require-iframe-title.js‎

Lines changed: 42 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,14 @@ module.exports = {
1010
},
1111
schema: [],
1212
messages: {
13+
// Split from a single `missingTitle` into four messageIds aligned with
14+
// upstream ember-template-lint, providing richer diagnostic detail.
1315
missingTitle: '<iframe> elements must have a unique title property.',
16+
emptyTitle: '<iframe> elements must have a unique title property.',
17+
dynamicFalseTitle: '<iframe> elements must have a unique title property.',
18+
duplicateTitleFirst: 'This title is not unique. #{{index}}',
19+
duplicateTitleOther:
20+
'<iframe> elements must have a unique title property. Value title="{{title}}" already used for different iframe. #{{index}}',
1421
},
1522
originallyFrom: {
1623
name: 'ember-template-lint',
@@ -20,7 +27,12 @@ module.exports = {
2027
},
2128
},
2229
create(context) {
30+
// Each entry: { value, node, index }
31+
// - value: trimmed title string
32+
// - node: original element node for the first occurrence
33+
// - index: duplicate-group index (1-based), assigned lazily on collision
2334
const knownTitles = [];
35+
let nextDuplicateIndex = 1;
2436

2537
return {
2638
GlimmerElementNode(node) {
@@ -47,22 +59,44 @@ module.exports = {
4759
case 'GlimmerTextNode': {
4860
const value = titleAttr.value.chars.trim();
4961
if (value.length === 0) {
50-
context.report({ node, messageId: 'missingTitle' });
62+
context.report({ node, messageId: 'emptyTitle' });
5163
} else {
52-
// Check for duplicate titles
53-
const existingIdx = knownTitles.findIndex(([val]) => val === value);
54-
if (existingIdx === -1) {
55-
knownTitles.push([value, node]);
64+
// Check for duplicate titles. Upstream reports BOTH the
65+
// first and the current occurrence of a duplicated title
66+
// on every collision, sharing a `#N` index so users can
67+
// correlate them. For three or more duplicates the first
68+
// occurrence is therefore re-reported once per collision.
69+
const existing = knownTitles.find((entry) => entry.value === value);
70+
if (existing) {
71+
if (existing.index === null) {
72+
existing.index = nextDuplicateIndex++;
73+
}
74+
const index = existing.index;
75+
76+
// Report on the first occurrence on every collision,
77+
// matching upstream ember-template-lint behavior.
78+
context.report({
79+
node: existing.node,
80+
messageId: 'duplicateTitleFirst',
81+
data: { index: String(index) },
82+
});
83+
84+
// Report on the current (duplicate) occurrence.
85+
context.report({
86+
node,
87+
messageId: 'duplicateTitleOther',
88+
data: { title: titleAttr.value.chars, index: String(index) },
89+
});
5690
} else {
57-
context.report({ node, messageId: 'missingTitle' });
91+
knownTitles.push({ value, node, index: null });
5892
}
5993
}
6094
break;
6195
}
6296
case 'GlimmerMustacheStatement': {
6397
// title={{false}} → BooleanLiteral false is invalid
6498
if (titleAttr.value.path?.type === 'GlimmerBooleanLiteral') {
65-
context.report({ node, messageId: 'missingTitle' });
99+
context.report({ node, messageId: 'dynamicFalseTitle' });
66100
}
67101
break;
68102
}
@@ -74,7 +108,7 @@ module.exports = {
74108
parts[0].type === 'GlimmerMustacheStatement' &&
75109
parts[0].path?.type === 'GlimmerBooleanLiteral'
76110
) {
77-
context.report({ node, messageId: 'missingTitle' });
111+
context.report({ node, messageId: 'dynamicFalseTitle' });
78112
}
79113
break;
80114
}

‎tests/lib/rules/template-require-iframe-title.js‎

Lines changed: 84 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -28,18 +28,61 @@ ruleTester.run('template-require-iframe-title', rule, {
2828
{
2929
code: '<template><iframe title=""></iframe></template>',
3030
output: null,
31-
errors: [{ messageId: 'missingTitle' }],
31+
errors: [{ messageId: 'emptyTitle' }],
3232
},
3333

3434
{
35+
// Upstream reports BOTH occurrences with a shared `#N` index.
3536
code: '<template><iframe title="foo" /><iframe title="foo" /></template>',
3637
output: null,
37-
errors: [{ messageId: 'missingTitle' }],
38+
errors: [
39+
{ message: 'This title is not unique. #1' },
40+
{
41+
message:
42+
'<iframe> elements must have a unique title property. Value title="foo" already used for different iframe. #1',
43+
},
44+
],
3845
},
3946
{
47+
// Three duplicates → upstream re-reports the first occurrence on every
48+
// collision, so iframe #1 is flagged twice (once per later collision)
49+
// and iframes #2 and #3 are each flagged once. ESLint sorts by source
50+
// location, so the two first-occurrence reports (same location) come
51+
// before the two later occurrences.
52+
code: '<template><iframe title="foo" /><iframe title="foo" /><iframe title="foo" /></template>',
53+
output: null,
54+
errors: [
55+
{ message: 'This title is not unique. #1' },
56+
{ message: 'This title is not unique. #1' },
57+
{
58+
message:
59+
'<iframe> elements must have a unique title property. Value title="foo" already used for different iframe. #1',
60+
},
61+
{
62+
message:
63+
'<iframe> elements must have a unique title property. Value title="foo" already used for different iframe. #1',
64+
},
65+
],
66+
},
67+
{
68+
// Two distinct duplicate groups → 4 reports, indices #1 and #2.
69+
// ESLint sorts errors by source location; the two "first-occurrence"
70+
// reports attach to the first two iframes and so precede the two
71+
// "other-occurrence" reports attached to the later iframes.
4072
code: '<template><iframe title="foo" /><iframe title="boo" /><iframe title="foo" /><iframe title="boo" /></template>',
4173
output: null,
42-
errors: [{ messageId: 'missingTitle' }, { messageId: 'missingTitle' }],
74+
errors: [
75+
{ message: 'This title is not unique. #1' },
76+
{ message: 'This title is not unique. #2' },
77+
{
78+
message:
79+
'<iframe> elements must have a unique title property. Value title="foo" already used for different iframe. #1',
80+
},
81+
{
82+
message:
83+
'<iframe> elements must have a unique title property. Value title="boo" already used for different iframe. #2',
84+
},
85+
],
4386
},
4487
{
4588
code: '<template><iframe src="12" /></template>',
@@ -49,17 +92,17 @@ ruleTester.run('template-require-iframe-title', rule, {
4992
{
5093
code: '<template><iframe src="12" title={{false}} /></template>',
5194
output: null,
52-
errors: [{ messageId: 'missingTitle' }],
95+
errors: [{ messageId: 'dynamicFalseTitle' }],
5396
},
5497
{
5598
code: '<template><iframe src="12" title="{{false}}" /></template>',
5699
output: null,
57-
errors: [{ messageId: 'missingTitle' }],
100+
errors: [{ messageId: 'dynamicFalseTitle' }],
58101
},
59102
{
60103
code: '<template><iframe src="12" title="" /></template>',
61104
output: null,
62-
errors: [{ messageId: 'missingTitle' }],
105+
errors: [{ messageId: 'emptyTitle' }],
63106
},
64107
],
65108
});
@@ -84,14 +127,46 @@ hbsRuleTester.run('template-require-iframe-title', rule, {
84127
{
85128
code: '<iframe title="foo" /><iframe title="foo" />',
86129
output: null,
87-
errors: [{ message: '<iframe> elements must have a unique title property.' }],
130+
errors: [
131+
{ message: 'This title is not unique. #1' },
132+
{
133+
message:
134+
'<iframe> elements must have a unique title property. Value title="foo" already used for different iframe. #1',
135+
},
136+
],
137+
},
138+
{
139+
// Three duplicates: upstream re-reports the first occurrence on every
140+
// collision, so iframe #1 is flagged twice and each later iframe once.
141+
code: '<iframe title="foo" /><iframe title="foo" /><iframe title="foo" />',
142+
output: null,
143+
errors: [
144+
{ message: 'This title is not unique. #1' },
145+
{ message: 'This title is not unique. #1' },
146+
{
147+
message:
148+
'<iframe> elements must have a unique title property. Value title="foo" already used for different iframe. #1',
149+
},
150+
{
151+
message:
152+
'<iframe> elements must have a unique title property. Value title="foo" already used for different iframe. #1',
153+
},
154+
],
88155
},
89156
{
90157
code: '<iframe title="foo" /><iframe title="boo" /><iframe title="foo" /><iframe title="boo" />',
91158
output: null,
92159
errors: [
93-
{ message: '<iframe> elements must have a unique title property.' },
94-
{ message: '<iframe> elements must have a unique title property.' },
160+
{ message: 'This title is not unique. #1' },
161+
{ message: 'This title is not unique. #2' },
162+
{
163+
message:
164+
'<iframe> elements must have a unique title property. Value title="foo" already used for different iframe. #1',
165+
},
166+
{
167+
message:
168+
'<iframe> elements must have a unique title property. Value title="boo" already used for different iframe. #2',
169+
},
95170
],
96171
},
97172
{

0 commit comments

Comments
 (0)