Skip to content

Commit 25864c0

Browse files
bmishbmish
committed
Breaking: Update no-missing-placeholders and no-unused-placeholders rules to also apply to suggestion messages
1 parent d85c446 commit 25864c0

File tree

5 files changed

+170
-49
lines changed

5 files changed

+170
-49
lines changed

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

+25-22
Original file line numberDiff line numberDiff line change
@@ -45,33 +45,36 @@ module.exports = {
4545
node.callee.property.type === 'Identifier' && node.callee.property.name === 'report'
4646
) {
4747
const reportInfo = utils.getReportInfo(node.arguments, context);
48-
if (!reportInfo || !reportInfo.message) {
48+
if (!reportInfo) {
4949
return;
5050
}
5151

52-
const messageStaticValue = getStaticValue(reportInfo.message, context.getScope());
53-
if (
54-
(
55-
(reportInfo.message.type === 'Literal' && typeof reportInfo.message.value === 'string') ||
56-
(messageStaticValue && typeof messageStaticValue.value === 'string')
57-
) &&
58-
(!reportInfo.data || reportInfo.data.type === 'ObjectExpression')
59-
) {
60-
// Same regex as the one ESLint uses
61-
// https://github.com/eslint/eslint/blob/e5446449d93668ccbdb79d78cc69f165ce4fde07/lib/eslint.js#L990
62-
const PLACEHOLDER_MATCHER = /{{\s*([^{}]+?)\s*}}/g;
63-
let match;
52+
const reportMessagesAndDataArray = utils.collectReportMessagesAndData(reportInfo);
53+
for (const { message, data } of reportMessagesAndDataArray) {
54+
const messageStaticValue = getStaticValue(message, context.getScope());
55+
if (
56+
(
57+
(message.type === 'Literal' && typeof message.value === 'string') ||
58+
(messageStaticValue && typeof messageStaticValue.value === 'string')
59+
) &&
60+
(!data || data.type === 'ObjectExpression')
61+
) {
62+
// Same regex as the one ESLint uses
63+
// https://github.com/eslint/eslint/blob/e5446449d93668ccbdb79d78cc69f165ce4fde07/lib/eslint.js#L990
64+
const PLACEHOLDER_MATCHER = /{{\s*([^{}]+?)\s*}}/g;
65+
let match;
6466

65-
while ((match = PLACEHOLDER_MATCHER.exec(reportInfo.message.value || messageStaticValue.value))) { // eslint-disable-line no-extra-parens
66-
const matchingProperty = reportInfo.data &&
67-
reportInfo.data.properties.find(prop => utils.getKeyName(prop) === match[1]);
67+
while ((match = PLACEHOLDER_MATCHER.exec(message.value || messageStaticValue.value))) { // eslint-disable-line no-extra-parens
68+
const matchingProperty = data &&
69+
data.properties.find(prop => utils.getKeyName(prop) === match[1]);
6870

69-
if (!matchingProperty) {
70-
context.report({
71-
node: reportInfo.message,
72-
messageId: 'placeholderDoesNotExist',
73-
data: { missingKey: match[1] },
74-
});
71+
if (!matchingProperty) {
72+
context.report({
73+
node: message,
74+
messageId: 'placeholderDoesNotExist',
75+
data: { missingKey: match[1] },
76+
});
77+
}
7578
}
7679
}
7780
}

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

+30-27
Original file line numberDiff line numberDiff line change
@@ -45,38 +45,41 @@ module.exports = {
4545
node.callee.property.type === 'Identifier' && node.callee.property.name === 'report'
4646
) {
4747
const reportInfo = utils.getReportInfo(node.arguments, context);
48-
if (!reportInfo || !reportInfo.message) {
48+
if (!reportInfo) {
4949
return;
5050
}
5151

52-
const messageStaticValue = getStaticValue(reportInfo.message, context.getScope());
53-
if (
54-
(
55-
(reportInfo.message.type === 'Literal' && typeof reportInfo.message.value === 'string') ||
56-
(messageStaticValue && typeof messageStaticValue.value === 'string')
57-
) &&
58-
reportInfo.data &&
59-
reportInfo.data.type === 'ObjectExpression'
60-
) {
61-
const message = reportInfo.message.value || messageStaticValue.value;
62-
// https://github.com/eslint/eslint/blob/2874d75ed8decf363006db25aac2d5f8991bd969/lib/linter.js#L986
63-
const PLACEHOLDER_MATCHER = /{{\s*([^{}]+?)\s*}}/g;
64-
const placeholdersInMessage = new Set();
52+
const reportMessagesAndDataArray = utils.collectReportMessagesAndData(reportInfo);
53+
for (const { message, data } of reportMessagesAndDataArray) {
54+
const messageStaticValue = getStaticValue(message, context.getScope());
55+
if (
56+
(
57+
(message.type === 'Literal' && typeof message.value === 'string') ||
58+
(messageStaticValue && typeof messageStaticValue.value === 'string')
59+
) &&
60+
data &&
61+
data.type === 'ObjectExpression'
62+
) {
63+
const messageValue = message.value || messageStaticValue.value;
64+
// https://github.com/eslint/eslint/blob/2874d75ed8decf363006db25aac2d5f8991bd969/lib/linter.js#L986
65+
const PLACEHOLDER_MATCHER = /{{\s*([^{}]+?)\s*}}/g;
66+
const placeholdersInMessage = new Set();
6567

66-
message.replace(PLACEHOLDER_MATCHER, (fullMatch, term) => {
67-
placeholdersInMessage.add(term);
68-
});
68+
messageValue.replace(PLACEHOLDER_MATCHER, (fullMatch, term) => {
69+
placeholdersInMessage.add(term);
70+
});
6971

70-
reportInfo.data.properties.forEach(prop => {
71-
const key = utils.getKeyName(prop);
72-
if (!placeholdersInMessage.has(key)) {
73-
context.report({
74-
node: reportInfo.message,
75-
messageId: 'placeholderUnused',
76-
data: { unusedKey: key },
77-
});
78-
}
79-
});
72+
data.properties.forEach(prop => {
73+
const key = utils.getKeyName(prop);
74+
if (!placeholdersInMessage.has(key)) {
75+
context.report({
76+
node: message,
77+
messageId: 'placeholderUnused',
78+
data: { unusedKey: key },
79+
});
80+
}
81+
});
82+
}
8083
}
8184
}
8285
},

‎lib/utils.js

+23
Original file line numberDiff line numberDiff line change
@@ -385,4 +385,27 @@ module.exports = {
385385
`,\n${propertyText}`
386386
);
387387
},
388+
389+
/**
390+
* Collect all violation/suggestion message nodes into a standardized array.
391+
* @param {Object} reportInfo - Result of getReportInfo().
392+
* @returns {message:String,data:Object}[]
393+
*/
394+
collectReportMessagesAndData (reportInfo) {
395+
return [
396+
// Violation message
397+
{ message: reportInfo.message, data: reportInfo.data },
398+
// Suggestion messages
399+
...((reportInfo.suggest && reportInfo.suggest.elements) || [])
400+
.map(suggestObjNode => {
401+
const propertyNodeDesc = suggestObjNode.properties.find(prop => prop.key.type === 'Identifier' && prop.key.name === 'desc');
402+
const propertyNodeData = suggestObjNode.properties.find(prop => prop.key.type === 'Identifier' && prop.key.name === 'data');
403+
return {
404+
message: propertyNodeDesc ? propertyNodeDesc.value : undefined,
405+
data: propertyNodeData ? propertyNodeData.value : undefined,
406+
};
407+
}
408+
),
409+
].filter(obj => obj.message);
410+
},
388411
};

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

+58
Original file line numberDiff line numberDiff line change
@@ -128,6 +128,24 @@ ruleTester.run('no-missing-placeholders', rule, {
128128
context.report(node, MESSAGE, { baz: 'qux' });
129129
};
130130
`,
131+
// Suggestion with placeholder
132+
`
133+
module.exports = {
134+
create(context) {
135+
context.report({
136+
node,
137+
suggest: [
138+
{
139+
desc: 'Remove {{functionName}}',
140+
data: {
141+
functionName: 'foo'
142+
}
143+
}
144+
]
145+
});
146+
}
147+
};
148+
`,
131149
],
132150

133151
invalid: [
@@ -212,5 +230,45 @@ ruleTester.run('no-missing-placeholders', rule, {
212230
`,
213231
errors: [error('bar')],
214232
},
233+
234+
{
235+
// Suggestion (no `data`)
236+
code: `
237+
module.exports = {
238+
create(context) {
239+
context.report({
240+
node,
241+
suggest: [
242+
{
243+
desc: 'Remove {{bar}}'
244+
}
245+
]
246+
});
247+
}
248+
};
249+
`,
250+
errors: [error('bar')],
251+
},
252+
{
253+
// Suggestion (`data` but missing placeholder)
254+
code: `
255+
module.exports = {
256+
create(context) {
257+
context.report({
258+
node,
259+
suggest: [
260+
{
261+
desc: 'Remove {{bar}}',
262+
data: {
263+
notBar: 'abc'
264+
}
265+
}
266+
]
267+
});
268+
}
269+
};
270+
`,
271+
errors: [error('bar')],
272+
},
215273
],
216274
});

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

+34
Original file line numberDiff line numberDiff line change
@@ -107,6 +107,21 @@ ruleTester.run('no-unused-placeholders', rule, {
107107
context.report(node, { line: 1, column: 3 }, 'foo {{bar}}', { bar: 'baz' });
108108
};
109109
`,
110+
`
111+
module.exports = {
112+
create(context) {
113+
context.report({
114+
node,
115+
suggest: [
116+
{
117+
desc: 'foo {{bar}}',
118+
data: { 'bar': 'baz' }
119+
}
120+
]
121+
});
122+
}
123+
};
124+
`,
110125
],
111126

112127
invalid: [
@@ -168,5 +183,24 @@ ruleTester.run('no-unused-placeholders', rule, {
168183
`,
169184
errors: [error('baz')],
170185
},
186+
{
187+
// Suggestion
188+
code: `
189+
module.exports = {
190+
create(context) {
191+
context.report({
192+
node,
193+
suggest: [
194+
{
195+
desc: 'foo',
196+
data: { bar }
197+
}
198+
]
199+
});
200+
}
201+
};
202+
`,
203+
errors: [error('bar')],
204+
},
171205
],
172206
});

0 commit comments

Comments
 (0)