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

Parameter "varsIgnorePattern" should ensure that they will never be read #13183

Closed
macabeus opened this issue Apr 15, 2020 · 5 comments
Closed
Labels
archived due to age This issue has been archived; please open a new issue for any further discussion auto closed The bot closed this issue enhancement This change enhances an existing feature of ESLint evaluating The team will evaluate this issue to decide whether it meets the criteria for inclusion rule Relates to ESLint's core rules

Comments

@macabeus
Copy link

macabeus commented Apr 15, 2020

What rule do you want to change?
no-unused-vars

Does this change cause the rule to produce more or fewer warnings?
Nop

How will the change be implemented? (New option, new default behavior, etc.)?
Currently, we could set the parameter varsIgnorePattern on no-unused-vars to ignore some patterns on variables that we expected that will be assigned but never reader.

A common convention on others languages is to add a landing _ on a variable that is never read. On JS it's useful, for example, on array destructuring:

const [, , job, , address] = person // "feels wrong" to write, and confuse to read
const [_name, _age, job, _nationality, address] = person // better to read

So if a variable follow the pattern, the expected is to be never read, so it'll should show an error:

const [_name, _age, job, _nationality, address] = person // better to read

console.log(_name) // hey! I'm reading an ignored variable! It's wrong!

Would be useful to have a configuration to set that. Something like showErrorWhenReadIgnoredVariable: true

Please provide some example code that this change will affect:

It's an additional feature and won't break any current code.

What does the rule currently do for this code?

Set variables that we can write but never read.

What will the rule do after it's changed?

It'll be possible to use the parameter showErrorWhenReadIgnoredVariable.

Are you willing to submit a pull request to implement this change?

I could do that.

@macabeus macabeus added enhancement This change enhances an existing feature of ESLint rule Relates to ESLint's core rules triage An ESLint team member will look at this issue soon labels Apr 15, 2020
@macabeus macabeus changed the title Parameter "varsIgnorePattern" on "no-unused-vars" should work with regex anchor and expected never will be read Parameter "varsIgnorePattern" on "no-unused-vars" should work with regex anchors and ensure that they will never be read Apr 15, 2020
@macabeus macabeus changed the title Parameter "varsIgnorePattern" on "no-unused-vars" should work with regex anchors and ensure that they will never be read Parameter "varsIgnorePattern" should ensure that they will never be read Apr 15, 2020
@anikethsaha
Copy link
Member

just to make sure I am getting this correctly, you need to report a variable that are in ignore list whenever they are read or written? or just read?

according to your example, if you don't want to use those variables _<name>, I would do something like this

-  const [_name, _age, job, _nationality, address] = person
+ const [  , , , , job, , , address] = person 

cause as per the required, no need of having those variables in the first place.

maybe I am missing something. let me know 👍

@macabeus
Copy link
Author

macabeus commented Apr 15, 2020

@anikethsaha Thank you for the reply =]
And yeah, the idea is really to be able to write a variable and ensure that nobody is reading it.
Adding the rule showErrorWhenReadIgnoredVariable ensure that everyone on a project is using the pattern on varsIgnorePattern for variables that really are never read, not for some other idea.

IMO is strange to write const [ , , , , a]. I don't know what are the others variables that I'm ignoring.
For situations like that we already have the parameter varsIgnorePattern. I'm just proposing a more strict rule for a parameter that we already have, to enforce a convention on the codebase.

@anikethsaha
Copy link
Member

And yeah, the idea is really to be able to write a variable and ensure that nobody is reading it.

this means that the variable is being used. it will be lint-free.

Adding the rule showErrorWhenReadIgnoredVariable ensure that everyone on a project is using the pattern on varsIgnorePattern for variables that really are never read, not for some other idea.

so you meant something strict in which ignore variables should not be used at all?
if so, why we need that variable?.

maybe I am not getting the proposal correctly, can you give some more example for this.

@mdjermanovic mdjermanovic added evaluating The team will evaluate this issue to decide whether it meets the criteria for inclusion and removed triage An ESLint team member will look at this issue soon labels Apr 18, 2020
@mdjermanovic
Copy link
Member

@macabeus thanks for the issue!

This is certainly an interesting proposal. I don't have a strong opinion at the moment on whether this feature belongs to this rule.

Enforcing naming conventions seem to be beyond the scope of responsibility for no-unused-vars.

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.

@eslint-deprecated eslint-deprecated bot added the auto closed The bot closed this issue label May 20, 2020
@eslint-deprecated
Copy link

Unfortunately, it looks like there wasn't enough interest from the team
or community to implement this change. While we wish we'd be able to
accommodate everyone's requests, we do need to prioritize. We've found
that issues failing to reach accepted status after 21 days tend to
never be accepted, and as such, we close those issues.
This doesn't mean the idea isn't interesting or useful, just that it's
not something the team can commit to.

Thanks for contributing to ESLint and we appreciate your understanding.

@eslint-deprecated eslint-deprecated bot locked and limited conversation to collaborators Nov 17, 2020
@eslint-deprecated eslint-deprecated bot added the archived due to age This issue has been archived; please open a new issue for any further discussion label Nov 17, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
archived due to age This issue has been archived; please open a new issue for any further discussion auto closed The bot closed this issue enhancement This change enhances an existing feature of ESLint evaluating The team will evaluate this issue to decide whether it meets the criteria for inclusion rule Relates to ESLint's core rules
Projects
None yet
Development

No branches or pull requests

3 participants