Skip to content

Commit 047b0b5

Browse files
authored
feat!: Update no-missing-placeholders and no-unused-placeholders to handle messageIds (#252)
* breaking: update no-missing-placeholders and no-unused-placeholders to handle messageIds * use utils
1 parent 1c96c77 commit 047b0b5

5 files changed

+283
-25
lines changed

lib/rules/no-missing-message-ids.js

+1-1
Original file line numberDiff line numberDiff line change
@@ -71,7 +71,7 @@ module.exports = {
7171
values.forEach((val) => {
7272
if (
7373
val.type === 'Literal' &&
74-
val.value !== null &&
74+
typeof val.value === 'string' &&
7575
val.value !== '' &&
7676
!utils.getMessageIdNodeById(
7777
val.value,

lib/rules/no-missing-placeholders.js

+34-12
Original file line numberDiff line numberDiff line change
@@ -31,19 +31,17 @@ module.exports = {
3131
},
3232

3333
create(context) {
34+
const sourceCode = context.getSourceCode();
35+
const { scopeManager } = sourceCode;
36+
3437
let contextIdentifiers;
3538

36-
// ----------------------------------------------------------------------
37-
// Public
38-
// ----------------------------------------------------------------------
39+
const ruleInfo = utils.getRuleInfo(sourceCode);
40+
const messagesNode = utils.getMessagesNode(ruleInfo, scopeManager);
3941

4042
return {
4143
Program(ast) {
42-
const sourceCode = context.getSourceCode();
43-
contextIdentifiers = utils.getContextIdentifiers(
44-
sourceCode.scopeManager,
45-
ast
46-
);
44+
contextIdentifiers = utils.getContextIdentifiers(scopeManager, ast);
4745
},
4846
CallExpression(node) {
4947
if (
@@ -57,10 +55,34 @@ module.exports = {
5755
return;
5856
}
5957

60-
const reportMessagesAndDataArray = utils
61-
.collectReportViolationAndSuggestionData(reportInfo)
62-
.filter((obj) => obj.message);
63-
for (const { message, data } of reportMessagesAndDataArray) {
58+
const reportMessagesAndDataArray =
59+
utils.collectReportViolationAndSuggestionData(reportInfo);
60+
61+
if (messagesNode) {
62+
// Check for any potential instances where we can use the messageId to fill in the message for convenience.
63+
reportMessagesAndDataArray.forEach((obj) => {
64+
if (
65+
!obj.message &&
66+
obj.messageId &&
67+
obj.messageId.type === 'Literal' &&
68+
typeof obj.messageId.value === 'string'
69+
) {
70+
const correspondingMessage = utils.getMessageIdNodeById(
71+
obj.messageId.value,
72+
ruleInfo,
73+
scopeManager,
74+
context.getScope()
75+
);
76+
if (correspondingMessage) {
77+
obj.message = correspondingMessage.value;
78+
}
79+
}
80+
});
81+
}
82+
83+
for (const { message, data } of reportMessagesAndDataArray.filter(
84+
(obj) => obj.message
85+
)) {
6486
const messageStaticValue = getStaticValue(
6587
message,
6688
context.getScope()

lib/rules/no-unused-placeholders.js

+34-12
Original file line numberDiff line numberDiff line change
@@ -30,19 +30,17 @@ module.exports = {
3030
},
3131

3232
create(context) {
33+
const sourceCode = context.getSourceCode();
34+
const { scopeManager } = sourceCode;
35+
3336
let contextIdentifiers;
3437

35-
// ----------------------------------------------------------------------
36-
// Public
37-
// ----------------------------------------------------------------------
38+
const ruleInfo = utils.getRuleInfo(sourceCode);
39+
const messagesNode = utils.getMessagesNode(ruleInfo, scopeManager);
3840

3941
return {
4042
Program(ast) {
41-
const sourceCode = context.getSourceCode();
42-
contextIdentifiers = utils.getContextIdentifiers(
43-
sourceCode.scopeManager,
44-
ast
45-
);
43+
contextIdentifiers = utils.getContextIdentifiers(scopeManager, ast);
4644
},
4745
CallExpression(node) {
4846
if (
@@ -56,10 +54,34 @@ module.exports = {
5654
return;
5755
}
5856

59-
const reportMessagesAndDataArray = utils
60-
.collectReportViolationAndSuggestionData(reportInfo)
61-
.filter((obj) => obj.message);
62-
for (const { message, data } of reportMessagesAndDataArray) {
57+
const reportMessagesAndDataArray =
58+
utils.collectReportViolationAndSuggestionData(reportInfo);
59+
60+
if (messagesNode) {
61+
// Check for any potential instances where we can use the messageId to fill in the message for convenience.
62+
reportMessagesAndDataArray.forEach((obj) => {
63+
if (
64+
!obj.message &&
65+
obj.messageId &&
66+
obj.messageId.type === 'Literal' &&
67+
typeof obj.messageId.value === 'string'
68+
) {
69+
const correspondingMessage = utils.getMessageIdNodeById(
70+
obj.messageId.value,
71+
ruleInfo,
72+
scopeManager,
73+
context.getScope()
74+
);
75+
if (correspondingMessage) {
76+
obj.message = correspondingMessage.value;
77+
}
78+
}
79+
});
80+
}
81+
82+
for (const { message, data } of reportMessagesAndDataArray.filter(
83+
(obj) => obj.message
84+
)) {
6385
const messageStaticValue = getStaticValue(
6486
message,
6587
context.getScope()

tests/lib/rules/no-missing-placeholders.js

+107
Original file line numberDiff line numberDiff line change
@@ -113,6 +113,60 @@ ruleTester.run('no-missing-placeholders', rule, {
113113
}
114114
};
115115
`,
116+
// messageId but no placeholder.
117+
`
118+
module.exports = {
119+
meta: {
120+
messages: { myMessageId: 'foo' }
121+
},
122+
create(context) {
123+
context.report({ node, messageId: 'myMessageId' });
124+
}
125+
};
126+
`,
127+
// messageId but the message doesn't exist in `meta.messages`.
128+
`
129+
module.exports = {
130+
meta: {
131+
messages: { }
132+
},
133+
create(context) {
134+
context.report({ node, messageId: 'myMessageId' });
135+
}
136+
};
137+
`,
138+
// messageId but no `meta.messages`.
139+
`
140+
module.exports = {
141+
meta: { },
142+
create(context) {
143+
context.report({ node, messageId: 'myMessageId' });
144+
}
145+
};
146+
`,
147+
// messageId but no `meta`.
148+
`
149+
module.exports = {
150+
create(context) {
151+
context.report({ node, messageId: 'myMessageId' });
152+
}
153+
};
154+
`,
155+
// messageId with correctly-used placeholder.
156+
`
157+
module.exports = {
158+
meta: {
159+
messages: { myMessageId: 'foo {{bar}}' }
160+
},
161+
create(context) {
162+
context.report({
163+
node,
164+
messageId: 'myMessageId',
165+
data: { bar: 'baz' }
166+
});
167+
}
168+
};
169+
`,
116170
// Message in variable.
117171
`
118172
const MESSAGE = 'foo {{bar}}';
@@ -145,6 +199,25 @@ ruleTester.run('no-missing-placeholders', rule, {
145199
}
146200
};
147201
`,
202+
// Suggestion with messageId
203+
`
204+
module.exports = {
205+
meta: { messages: { myMessageId: 'Remove {{functionName}}' } },
206+
create(context) {
207+
context.report({
208+
node,
209+
suggest: [
210+
{
211+
messageId: 'myMessageId',
212+
data: {
213+
functionName: 'foo'
214+
}
215+
}
216+
]
217+
});
218+
}
219+
};
220+
`,
148221
],
149222

150223
invalid: [
@@ -269,6 +342,25 @@ ruleTester.run('no-missing-placeholders', rule, {
269342
`,
270343
errors: [error('bar')],
271344
},
345+
{
346+
// Suggestion and messageId
347+
code: `
348+
module.exports = {
349+
meta: { messages: { myMessageId: 'foo {{bar}}' } },
350+
create(context) {
351+
context.report({
352+
node,
353+
suggest: [
354+
{
355+
messageId: 'myMessageId',
356+
}
357+
]
358+
});
359+
}
360+
};
361+
`,
362+
errors: [error('bar')],
363+
},
272364
{
273365
// `create` in variable.
274366
code: `
@@ -283,5 +375,20 @@ ruleTester.run('no-missing-placeholders', rule, {
283375
`,
284376
errors: [error('hasOwnProperty')],
285377
},
378+
{
379+
// messageId.
380+
code: `
381+
module.exports = {
382+
meta: { messages: { myMessageId: 'foo {{bar}}' } },
383+
create(context) {
384+
context.report({
385+
node,
386+
messageId: 'myMessageId'
387+
});
388+
}
389+
};
390+
`,
391+
errors: [error('bar')],
392+
},
286393
],
287394
});

tests/lib/rules/no-unused-placeholders.js

+107
Original file line numberDiff line numberDiff line change
@@ -122,6 +122,77 @@ ruleTester.run('no-unused-placeholders', rule, {
122122
}
123123
};
124124
`,
125+
// Suggestion with messageId
126+
`
127+
module.exports = {
128+
meta: { messages: { myMessageId: 'foo {{bar}}' } },
129+
create(context) {
130+
context.report({
131+
node,
132+
suggest: [
133+
{
134+
messageId: 'myMessageId',
135+
data: { 'bar': 'baz' }
136+
}
137+
]
138+
});
139+
}
140+
};
141+
`,
142+
// messageId but no placeholder.
143+
`
144+
module.exports = {
145+
meta: {
146+
messages: { myMessageId: 'foo' }
147+
},
148+
create(context) {
149+
context.report({ node, messageId: 'myMessageId' });
150+
}
151+
};
152+
`,
153+
// messageId but the message property doesn't exist yet.
154+
`
155+
module.exports = {
156+
meta: {
157+
messages: { }
158+
},
159+
create(context) {
160+
context.report({ node, messageId: 'myMessageId' });
161+
}
162+
};
163+
`,
164+
// messageId but no `meta.messages`.
165+
`
166+
module.exports = {
167+
meta: {},
168+
create(context) {
169+
context.report({ node, messageId: 'myMessageId' });
170+
}
171+
};
172+
`,
173+
// messageId but no `meta`.
174+
`
175+
module.exports = {
176+
create(context) {
177+
context.report({ node, messageId: 'myMessageId' });
178+
}
179+
};
180+
`,
181+
// messageId with correctly-used placeholder.
182+
`
183+
module.exports = {
184+
meta: {
185+
messages: { myMessageId: 'foo {{bar}}' }
186+
},
187+
create(context) {
188+
context.report({
189+
node,
190+
messageId: 'myMessageId',
191+
data: { bar: 'baz' }
192+
});
193+
}
194+
};
195+
`,
125196
],
126197

127198
invalid: [
@@ -197,6 +268,22 @@ ruleTester.run('no-unused-placeholders', rule, {
197268
`,
198269
errors: [error('baz')],
199270
},
271+
{
272+
// messageId.
273+
code: `
274+
module.exports = {
275+
meta: { messages: { myMessageId: 'foo' } },
276+
create(context) {
277+
context.report({
278+
node,
279+
messageId: 'myMessageId',
280+
data: { bar }
281+
});
282+
}
283+
};
284+
`,
285+
errors: [error('bar')],
286+
},
200287
{
201288
// Suggestion
202289
code: `
@@ -216,5 +303,25 @@ ruleTester.run('no-unused-placeholders', rule, {
216303
`,
217304
errors: [error('bar')],
218305
},
306+
{
307+
// Suggestion and messageId
308+
code: `
309+
module.exports = {
310+
meta: { messages: { myMessageId: 'foo' } },
311+
create(context) {
312+
context.report({
313+
node,
314+
suggest: [
315+
{
316+
messageId: 'myMessageId',
317+
data: { bar }
318+
}
319+
]
320+
});
321+
}
322+
};
323+
`,
324+
errors: [error('bar')],
325+
},
219326
],
220327
});

0 commit comments

Comments
 (0)