Skip to content

Commit bdb9cb8

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 bdb9cb8

File tree

4 files changed

+119
-1
lines changed

4 files changed

+119
-1
lines changed

lib/utils.js

+7
Original file line numberDiff line numberDiff line change
@@ -875,6 +875,13 @@ module.exports = {
875875
scopeManager.acquire(node) || scopeManager.globalScope,
876876
node
877877
);
878+
879+
if (variable?.defs[0]?.type === 'Parameter') {
880+
// If this variable is a function parameter, then we can't currently know what all possible values could be passed to it, so bail out.
881+
// TODO: as a future improvement, we should instead attempt to gather the values that this function parameter is called with.
882+
return [];
883+
}
884+
878885
return ((variable && variable.references) || []).flatMap((ref) => {
879886
if (
880887
ref.writeExpr &&

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

+42
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, outside 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+
function report(node, messageId) {
231+
context.report({node, messageId});
232+
}
233+
module.exports = {
234+
meta: { messages: { foo: 'hello' } },
235+
create(context) {
236+
report(node, 'foo');
237+
report(node, 'bar');
238+
}
239+
};
240+
`,
199241
],
200242

201243
invalid: [

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
}

tests/lib/utils.js

+25
Original file line numberDiff line numberDiff line change
@@ -1645,5 +1645,30 @@ describe('utils', () => {
16451645
]
16461646
);
16471647
});
1648+
1649+
it('returns nothing for function parameter (TODO: future improvement is to detect what values the function parameter is called with)', () => {
1650+
const code =
1651+
'function myFunc(x) { if (foo) { x = "abc"; } console.log(x) }; myFunc("def");';
1652+
const ast = espree.parse(code, {
1653+
ecmaVersion: 9,
1654+
range: true,
1655+
});
1656+
1657+
// Add parent to each node.
1658+
estraverse.traverse(ast, {
1659+
enter(node, parent) {
1660+
node.parent = parent;
1661+
},
1662+
});
1663+
1664+
const scopeManager = eslintScope.analyze(ast);
1665+
assert.deepEqual(
1666+
utils.findPossibleVariableValues(
1667+
ast.body[0].body.body[1].expression.arguments[0],
1668+
scopeManager
1669+
),
1670+
[]
1671+
);
1672+
});
16481673
});
16491674
});

0 commit comments

Comments
 (0)