Skip to content

Commit 110e0c0

Browse files
committed
fix: false positives with violation reporting helper function in no-missing-message-ids and no-unused-message-ids rules
1 parent aeac273 commit 110e0c0

File tree

4 files changed

+129
-2
lines changed

4 files changed

+129
-2
lines changed

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

+2-1
Original file line numberDiff line numberDiff line change
@@ -120,7 +120,8 @@ module.exports = {
120120

121121
if (
122122
values.length === 0 ||
123-
values.some((val) => val.type !== 'Literal')
123+
values.some((val) => val.type !== 'Literal') ||
124+
utils.isVariableFromParameter(node.value, scopeManager)
124125
) {
125126
// When a dynamic messageId is used and we can't detect its value, disable the rule to avoid false positives.
126127
hasSeenUnknownMessageId = true;

lib/utils.js

+15
Original file line numberDiff line numberDiff line change
@@ -888,4 +888,19 @@ module.exports = {
888888
return [];
889889
});
890890
},
891+
892+
/**
893+
* Check whether a variable's definition is from a function parameter.
894+
* @param {Node} node - the Identifier node for the variable.
895+
* @param {ScopeManager} scopeManager
896+
* @returns {boolean} whether the variable comes from a function parameter
897+
*/
898+
isVariableFromParameter(node, scopeManager) {
899+
const variable = findVariable(
900+
scopeManager.acquire(node) || scopeManager.globalScope,
901+
node
902+
);
903+
904+
return variable?.defs[0]?.type === 'Parameter';
905+
},
891906
};

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

+67
Original file line numberDiff line numberDiff line change
@@ -196,6 +196,48 @@ ruleTester.run('no-missing-message-ids', rule, {
196196
}
197197
};
198198
`,
199+
// Helper function with messageId parameter, outside rule.
200+
`
201+
function report(node, messageId) {
202+
context.report({node, messageId});
203+
}
204+
module.exports = {
205+
meta: { messages: { foo: 'hello' } },
206+
create(context) {
207+
report(node, 'foo');
208+
}
209+
};
210+
`,
211+
// Helper function with messageId parameter, inside rule, with parameter reassignment.
212+
`
213+
module.exports = {
214+
meta: { messages: { foo: 'hello', bar: 'world' } },
215+
create(context) {
216+
function report(node, messageId) {
217+
if (foo) {
218+
messageId = 'bar';
219+
}
220+
context.report({node, messageId});
221+
}
222+
report(node, 'foo');
223+
}
224+
};
225+
`,
226+
// Helper function with messageId parameter, inside rule, with missing messageId.
227+
// TODO: this should be an invalid test case because a non-existent `messageId` is used.
228+
// Eventually, we should be able to detect what values are passed to this function for its `messageId` parameter.
229+
`
230+
module.exports = {
231+
meta: { messages: { foo: 'hello' } },
232+
create(context) {
233+
function report(node, messageId) {
234+
context.report({node, messageId});
235+
}
236+
report(node, 'foo');
237+
report(node, 'bar');
238+
}
239+
};
240+
`,
199241
],
200242

201243
invalid: [
@@ -287,5 +329,30 @@ ruleTester.run('no-missing-message-ids', rule, {
287329
},
288330
],
289331
},
332+
{
333+
// Helper function with messageId parameter, inside rule, with missing messageId due to parameter reassignment.
334+
code: `
335+
336+
module.exports = {
337+
meta: { messages: { foo: 'hello' } },
338+
create(context) {
339+
function report(node, messageId) {
340+
if (foo) {
341+
messageId = 'bar';
342+
}
343+
context.report({node, messageId});
344+
}
345+
report(node, 'foo');
346+
}
347+
};
348+
`,
349+
errors: [
350+
{
351+
messageId: 'missingMessage',
352+
data: { messageId: 'bar' },
353+
type: 'Literal',
354+
},
355+
],
356+
},
290357
],
291358
});

tests/lib/rules/no-unused-message-ids.js

+45-1
Original file line numberDiff line numberDiff line change
@@ -227,6 +227,50 @@ ruleTester.run('no-unused-message-ids', rule, {
227227
create(context) {}
228228
};
229229
`,
230+
// Helper function messageId parameter, outside rule.
231+
`
232+
function reportFoo(node, messageId) {
233+
context.report({ node, messageId });
234+
}
235+
module.exports = {
236+
meta: { messages: { foo: 'hello', bar: 'world', baz: 'planet' } },
237+
create(context) {
238+
reportFoo(node, 'foo');
239+
reportFoo(node, 'bar');
240+
reportFoo(node, 'baz');
241+
}
242+
};
243+
`,
244+
// Helper function with messageId parameter, inside rule, parameter reassignment.
245+
`
246+
module.exports = {
247+
meta: { messages: { foo: 'hello', bar: 'world', baz: 'planet' } },
248+
create(context) {
249+
function reportFoo(node, messageId) {
250+
if (foo) {
251+
messageId = 'baz';
252+
}
253+
context.report({ node, messageId });
254+
}
255+
reportFoo(node, 'foo');
256+
reportFoo(node, 'bar');
257+
}
258+
};
259+
`,
260+
// Helper function with messageId parameter, outside rule, with an unused messageId.
261+
// TODO: this should be an invalid test case because a messageId is unused.
262+
// Eventually, we should be able to detect what values are passed to this function for its messageId parameter.
263+
`
264+
function reportFoo(node, messageId) {
265+
context.report({ node, messageId });
266+
}
267+
module.exports = {
268+
meta: { messages: { foo: 'hello', bar: 'world' } },
269+
create(context) {
270+
reportFoo(node, 'foo');
271+
}
272+
};
273+
`,
230274
],
231275

232276
invalid: [
@@ -363,7 +407,7 @@ ruleTester.run('no-unused-message-ids', rule, {
363407
context.report({ node, messageId });
364408
}
365409
module.exports = {
366-
meta: { messages: { foo: 'hello world' } },
410+
meta: { messages: { foo: 'hello world', bar: 'baz' } },
367411
create(context) {
368412
reportFoo(node);
369413
}

0 commit comments

Comments
 (0)