Rule Change: Add option to no-unused-vars that will report a variable that has been marked as ignored but is being used.

[Pearce-Ropion]

What rule do you want to change?

no-unused-vars

What change to do you want to make?

Generate more warnings

How do you think the change should be implemented?

A new option

Example code

/* eslint no-unused-vars: ["error", { "varsIgnorePattern": "^_" }] */

const _myUnusedVariable = 10;

export const myUsedVariable = _myUnusedVariable + 10;

What does the rule currently do for this code?

Currently, the rule will not report this case because _myUnusedVariable is being used.

What will the rule do after it's changed?

The rule would report that a variable that has been marked as an "ignored unused variable" is actually being used.

I propose a new option, potentially called reportUsedIgnorePattern. Optionally, there could be a version of this option for each of the different ignore pattern options.

Participation

• I am willing to submit a pull request to implement this change.

Additional comments

The main reason for this is that I've found that devs will name a variable with our ignore pattern since its a simple way to have another variable with a very similar name. I would like to instead encourage devs to rename their variables with a clear meaning by not allowing them to use the ignore pattern for a used variable.

I acknowledge that part of this is that we have a very simple ignore pattern, but I feel like this is still something that should be enforceable by the lint rule.

[Rec0iL99]

Hi @Pearce-Ropion, thanks for the issue. Can you please provide a few more usage examples to support this proposal?

Currently, the rule will not report this case because _myUnusedVariable is being used.

The rule will not check _myUnusedVariable for usage since it matches the ignore pattern (docs).

[Rec0iL99]

No response so closing.

[eslint-github-bot]

It looks like there wasn't enough information for us to know how to help you, so we're closing the issue.

Thanks for your understanding.

[Pearce-Ropion]

@Rec0iL99 Apologies, I had written out a response but I guess I never sent it. TIL GitHub caches draft comments a lot longer than expected.

The idea, is that if an ignore pattern is set, users should only be able to use that variable naming pattern for unused variables.

This increases code readability as a quick scan of the code would indicate to the user that a variable that is marked with an ignore pattern is unused.

In the current implementation, users are able to use variables that are marked with by the unused variable ignore pattern which decreases readability as you wonder why the variable was marked as "ignore-unused" given that it is actually being used.

I think my original example snippet makes the most sense but I can see how further explanation would be helpful.

Given

/* eslint no-unused-vars: ["error", { "varsIgnorePattern": "^_" }] */

const _myUnusedVariable = 10;

export const myUsedVariable = _myUnusedVariable + 10;

My immediate thought when reading this code is: "why is _myUnusedVariable marked as an ignored unused variable (given the ignore pattern) when it is being used in the next line". Did the developer use the variable incorrectly? Should this variable actually be ignored?

Another example could be:

import { createElement } from './utils';

const _createElement = (tagName, id) => {
  return createElement(tagName, {
    id,
  });
};

const table = _createElement('table', 'goo');

Again, my immediate thought my here is why is _createTable marked as unused. I my mind, a marked unused variable should actually be unused so that I can just discount it as not important when reading over the code. But here, I have to go back and re-read the code to understand why the unused variable syntax was used and understand why the variable is important.

[Rec0iL99]

Thanks for the extra info. I think I now understand your proposal. This looks quite similar to #13183. Can you please verify?

[Pearce-Ropion]

@Rec0iL99 Yes, this looks like the same idea. By marking a variable as ignored we are saying that it is now expected that it should never be read. By then using it, it violates the expected contract of what an unused variable should be.

My idea with making this rule suggesting is akin to what @mdjermanovic states here:

On the other hand, this looks useful, and duplicating the configured patterns and the overall logic for unused vars in another rule might not make sense.

It would be relatively easy to integrate this into the existing rule but duplicating the all of the logic that this rule employs into another rule just to provide this functionality might not make sense.

[Rec0iL99]

Thanks. Pushing it forward for the core team to look into.

[nzakas]

To summarize my understanding of the request:

The varsIgnorePattern option is meant to prevent the rule from flagging variables that match a certain naming pattern as unused. This allows developers to have unused variables that the rule won't complain about. However, if a variable matching varsIgnorePattern is actually used, then something isn't right: either the variable is named incorrectly or the rule is configured incorrectly. Right now, there is no way for developers to know that such a condition exists.

The proposal is to add an option (reportUsedIgnorePattern) that allows the rule to warn in this situation.

I wonder if this would actually make sense to enable this option by default?

I think this is a good option to add to the rule, and as it seemed that @mdjermanovic believed a similar proposal was useful in #13183, I'm marking as accepted.

@Pearce-Ropion if you'd like to open a PR, we can get the ball rolling.

[Pearce-Ropion]

Great! I'll start looking into the code.