Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

False positive with no-unused-message-ids when violation reporting is done with external helper function #283

Closed
bmish opened this issue Aug 10, 2022 · 4 comments · Fixed by #286
Labels

Comments

@bmish
Copy link
Member

bmish commented Aug 10, 2022

In this example, the context.report() calls are all in a separate helper file, so we can't detect that the messageIds are used.

We should ignore this situation where we can't find any context.report() calls.

https://github.com/typescript-eslint/typescript-eslint/blob/b6e5413fb12c2b1943a288b514f864c339380388/packages/eslint-plugin/src/rules/naming-convention.ts#L11

@aladdin-add
Copy link
Contributor

I've encountered the issue in eslint-plugin-node as well: https://github.com/weiran-zsd/eslint-plugin-node/blob/d7b975a07e1b876ca2c75597d054a81564b25685/lib/rules/no-unsupported-features/es-builtins.js#L173-L179

it's really helpful to ignore these cases, at least it can be an option. 👍

@JoshuaKGoldberg
Copy link
Contributor

JoshuaKGoldberg commented Aug 11, 2022

We should ignore this situation where we can't find any context.report() calls.

Edge case: a rule that calls context.report() for some message IDs, but calls to an external function for other message IDs.

Suggestion: how about disabling when the rule can see that a value set to context or its validate property is passed to a function?

@aladdin-add
Copy link
Contributor

aladdin-add commented Aug 11, 2022

a value set to context or its validate property is passed to a function?

it sounds it's guessing the user's wants, while it's not always reliable. I'm 👍 to @bmish 's suggestion.

@bmish
Copy link
Member Author

bmish commented Aug 11, 2022

We should ignore this situation where we can't find any context.report() calls.

Edge case: a rule that calls context.report() for some message IDs, but calls to an external function for other message IDs.

Suggestion: how about disabling when the rule can see that a value set to context or its validate property is passed to a function?

That's definitely a valid edge case. Regarding your suggestion, I'm open to it as a further improvement, especially if we start seeing real-world examples of that edge case, but note that there could be false negatives from it. Also we would want to limit that suggestion to external/imported functions that we can't analyze.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
3 participants