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

no-underscore-dangle to check function params #12579

Closed
hnipps opened this issue Nov 19, 2019 · 7 comments · Fixed by #13545
Closed

no-underscore-dangle to check function params #12579

hnipps opened this issue Nov 19, 2019 · 7 comments · Fixed by #13545
Assignees
Labels
accepted There is consensus among the team that this change meets the criteria for inclusion archived due to age This issue has been archived; please open a new issue for any further discussion enhancement This change enhances an existing feature of ESLint rule Relates to ESLint's core rules

Comments

@hnipps
Copy link

hnipps commented Nov 19, 2019

What rule do you want to change?
no-underscore-dangle

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

How will the change be implemented? (New option, new default behavior, etc.)?
The updated rule will check if param Identifiers of FunctionDeclarations and ArrowFunctionExpressions have dangling underscores.

I suggest that underscore dangle in these situations is disallowed by default and can be configured using the same pattern as "allowAfterThis" and "allowAfterSuper".

Please provide some example code that this change will affect:

const test = (_arg) => { };
function myfunc(_arg) { };

What does the rule currently do for this code?
No error thrown.

What will the rule do after it's changed?
Throw an error because the function params have dangling underscores.

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

@hnipps hnipps 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 Nov 19, 2019
@hnipps
Copy link
Author

hnipps commented Nov 19, 2019

This update will help users migrating from TSLint to ESLint to reach feature parity because the TSLint variable-name rule already has the proposed feature.

@webOS101
Copy link
Contributor

I would suggest this be configurable, as well, or, perhaps allow a single underscore in parameter names by default, given the common pattern of using _ for unused parameter.

@kaicataldo kaicataldo 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 Nov 20, 2019
@kaicataldo
Copy link
Member

I agree that this seems like an oversight of the rule and support this change. I do think it needs to be put behind an option to avoid a breaking change (maybe allowFunctionParams: true by default) and that in addition to FunctionDeclarations and ArrowFunctionExpressions, I think it should also check FunctionExpressions.

@hnipps
Copy link
Author

hnipps commented Dec 18, 2019

Is this still being evaluated? Or do we have a decision?

@kaicataldo
Copy link
Member

I'll champion this. We still need support from two other team members before we can mark this as accepted.

@kaicataldo kaicataldo self-assigned this Dec 19, 2019
@ron23
Copy link

ron23 commented Feb 20, 2020

I would love to see that as well. We're migrating from TSLint and would really like to keep the consistency. Thanks!

@anikethsaha
Copy link
Member

Marking this as accepted as there are 3 +1 from team members.

I think allowFunctionParams might be a good approach and name which will be true by default as mentioned above

@anikethsaha anikethsaha added accepted There is consensus among the team that this change meets the criteria for inclusion and removed evaluating The team will evaluate this issue to decide whether it meets the criteria for inclusion labels Jul 23, 2020
sunghyunjo added a commit to sunghyunjo/eslint that referenced this issue Aug 1, 2020
sunghyunjo added a commit to sunghyunjo/eslint that referenced this issue Aug 2, 2020
sunghyunjo added a commit to sunghyunjo/eslint that referenced this issue Aug 2, 2020
sunghyunjo added a commit to sunghyunjo/eslint that referenced this issue Aug 2, 2020
sunghyunjo added a commit to sunghyunjo/eslint that referenced this issue Aug 2, 2020
sunghyunjo pushed a commit to sunghyunjo/eslint that referenced this issue Aug 4, 2020
sunghyunjo pushed a commit to sunghyunjo/eslint that referenced this issue Aug 4, 2020
sunghyunjo added a commit to sunghyunjo/eslint that referenced this issue Aug 4, 2020
sunghyunjo added a commit to sunghyunjo/eslint that referenced this issue Aug 4, 2020
sunghyunjo pushed a commit to sunghyunjo/eslint that referenced this issue Aug 7, 2020
sunghyunjo added a commit to sunghyunjo/eslint that referenced this issue Aug 9, 2020
sunghyunjo added a commit to sunghyunjo/eslint that referenced this issue Aug 9, 2020
kaicataldo pushed a commit that referenced this issue Aug 14, 2020
…79) (#13545)

* #12579 add allowFunctionParams option

* #12579 edit doc & function name

* #12579 add test case

* #12579 add allowFunctionParam rule in docs

* #12579 Update : test case when option is false

* Return to origin code

* Remove comments

* Edit what was reviewed

* Update: Destructuring param test

* Update: invalid test case

* Change initial state to true

* #12579 Update: Edit what was reviewed

* #12579 Fix a typo

* #12579 Update : allow option

* #12579 Update: Edit what was reviewed

* #12579 Update : check type of param

* #12579 Simplify the code

* Update : test case

* Fix : ...bar -> ..._bar
@eslint-deprecated eslint-deprecated bot locked and limited conversation to collaborators Feb 11, 2021
@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 Feb 11, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
accepted There is consensus among the team that this change meets the criteria for inclusion archived due to age This issue has been archived; please open a new issue for any further discussion enhancement This change enhances an existing feature of ESLint rule Relates to ESLint's core rules
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants