Skip to content

Commit 123d8fb

Browse files
bmishbmish
committed
Breaking: Update no-missing-placeholders, no-unused-placeholders, prefer-message-ids, prefer-placeholders rules to also apply to suggestion messages
1 parent d85c446 commit 123d8fb

10 files changed

+350
-87
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.collectReportViolationAndSuggestionData(reportInfo).filter(obj => obj.message);
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.collectReportViolationAndSuggestionData(reportInfo).filter(obj => obj.message);
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/rules/prefer-message-ids.js

+8-5
Original file line numberDiff line numberDiff line change
@@ -68,14 +68,17 @@ module.exports = {
6868
node.callee.property.type === 'Identifier' && node.callee.property.name === 'report'
6969
) {
7070
const reportInfo = utils.getReportInfo(node.arguments, context);
71-
if (!reportInfo || !reportInfo.message) {
71+
if (!reportInfo) {
7272
return;
7373
}
7474

75-
context.report({
76-
node: reportInfo.message.parent,
77-
messageId: 'foundMessage',
78-
});
75+
const reportMessagesAndDataArray = utils.collectReportViolationAndSuggestionData(reportInfo).filter(obj => obj.message);
76+
for (const { message } of reportMessagesAndDataArray) {
77+
context.report({
78+
node: message.parent,
79+
messageId: 'foundMessage',
80+
});
81+
}
7982
}
8083
},
8184
};

‎lib/rules/prefer-placeholders.js

+28-27
Original file line numberDiff line numberDiff line change
@@ -49,42 +49,43 @@ module.exports = {
4949
) {
5050
const reportInfo = utils.getReportInfo(node.arguments, context);
5151

52-
if (!reportInfo || !reportInfo.message) {
52+
if (!reportInfo) {
5353
return;
5454
}
5555

56-
let messageNode = reportInfo.message;
56+
const reportMessagesAndDataArray = utils.collectReportViolationAndSuggestionData(reportInfo).filter(obj => obj.message);
57+
for (let { message: messageNode } of reportMessagesAndDataArray) {
58+
if (messageNode.type === 'Identifier') {
59+
// See if we can find the variable declaration.
5760

58-
if (messageNode.type === 'Identifier') {
59-
// See if we can find the variable declaration.
61+
const variable = findVariable(
62+
scopeManager.acquire(messageNode) || scopeManager.globalScope,
63+
messageNode
64+
);
6065

61-
const variable = findVariable(
62-
scopeManager.acquire(messageNode) || scopeManager.globalScope,
63-
messageNode
64-
);
66+
if (
67+
!variable ||
68+
!variable.defs ||
69+
!variable.defs[0] ||
70+
!variable.defs[0].node ||
71+
variable.defs[0].node.type !== 'VariableDeclarator' ||
72+
!variable.defs[0].node.init
73+
) {
74+
return;
75+
}
76+
77+
messageNode = variable.defs[0].node.init;
78+
}
6579

6680
if (
67-
!variable ||
68-
!variable.defs ||
69-
!variable.defs[0] ||
70-
!variable.defs[0].node ||
71-
variable.defs[0].node.type !== 'VariableDeclarator' ||
72-
!variable.defs[0].node.init
81+
(messageNode.type === 'TemplateLiteral' && messageNode.expressions.length > 0) ||
82+
(messageNode.type === 'BinaryExpression' && messageNode.operator === '+')
7383
) {
74-
return;
84+
context.report({
85+
node: messageNode,
86+
messageId: 'usePlaceholders',
87+
});
7588
}
76-
77-
messageNode = variable.defs[0].node.init;
78-
}
79-
80-
if (
81-
(messageNode.type === 'TemplateLiteral' && messageNode.expressions.length > 0) ||
82-
(messageNode.type === 'BinaryExpression' && messageNode.operator === '+')
83-
) {
84-
context.report({
85-
node: messageNode,
86-
messageId: 'usePlaceholders',
87-
});
8889
}
8990
}
9091
},

‎lib/utils.js

+39
Original file line numberDiff line numberDiff line change
@@ -169,6 +169,17 @@ function getRuleExportsCJS (ast) {
169169
}, {});
170170
}
171171

172+
/**
173+
* Find the value of a property in an object by its property key name.
174+
* @param {Object} obj
175+
* @param {String} keyName
176+
* @returns property value
177+
*/
178+
function findObjectPropertyValueByKeyName (obj, keyName) {
179+
const property = obj.properties.find(prop => prop.key.type === 'Identifier' && prop.key.name === keyName);
180+
return property ? property.value : undefined;
181+
}
182+
172183
module.exports = {
173184
/**
174185
* Performs static analysis on an AST to try to determine the final value of `module.exports`.
@@ -385,4 +396,32 @@ module.exports = {
385396
`,\n${propertyText}`
386397
);
387398
},
399+
400+
/**
401+
* Collect all context.report({...}) violation/suggestion-related nodes into a standardized array for convenience.
402+
* @param {Object} reportInfo - Result of getReportInfo().
403+
* @returns {messageId?: String, message?: String, data?: Object, fix?: Function}[]
404+
*/
405+
collectReportViolationAndSuggestionData (reportInfo) {
406+
return [
407+
// Violation message
408+
{
409+
messageId: reportInfo.messageId,
410+
message: reportInfo.message,
411+
data: reportInfo.data,
412+
fix: reportInfo.fix,
413+
},
414+
// Suggestion messages
415+
...((reportInfo.suggest && reportInfo.suggest.elements) || [])
416+
.map(suggestObjNode => {
417+
return {
418+
messageId: findObjectPropertyValueByKeyName(suggestObjNode, 'messageId'),
419+
message: findObjectPropertyValueByKeyName(suggestObjNode, 'desc'), // Note: suggestion message named `desc`
420+
data: findObjectPropertyValueByKeyName(suggestObjNode, 'data'),
421+
fix: findObjectPropertyValueByKeyName(suggestObjNode, 'fix'),
422+
};
423+
}
424+
),
425+
];
426+
},
388427
};

‎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

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

112128
invalid: [
@@ -168,5 +184,24 @@ ruleTester.run('no-unused-placeholders', rule, {
168184
`,
169185
errors: [error('baz')],
170186
},
187+
{
188+
// Suggestion
189+
code: `
190+
module.exports = {
191+
create(context) {
192+
context.report({
193+
node,
194+
suggest: [
195+
{
196+
desc: 'foo',
197+
data: { bar }
198+
}
199+
]
200+
});
201+
}
202+
};
203+
`,
204+
errors: [error('bar')],
205+
},
171206
],
172207
});

0 commit comments

Comments
 (0)