-
-
Notifications
You must be signed in to change notification settings - Fork 368
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
Add no-unreadable-array-destructuring
rule
#199
Add no-unreadable-array-destructuring
rule
#199
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It would be nice to have additional tests for the array destructuring in arguments, like function a([, b]) {}
. From what I have gathered, it should handle this case well, but more test cases can't hurt.
Thanks @janowsiany for working on this!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, thanks @janowsiany!
no-unreadable-array-destructuring
rule
Thanks for working on this, @janowsiany 🙌 |
The rule needs to be added to the bottom of the list here: https://github.com/sindresorhus/eslint-plugin-unicorn#rules |
Should this rule allow |
@sindresorhus @MrHen thanks for review |
Thank you, @janowsiany :) |
@@ -55,7 +55,8 @@ Configure it in `package.json`. | |||
"unicorn/error-message": "error", | |||
"unicorn/no-unsafe-regex": "off", | |||
"unicorn/prefer-add-event-listener": "error", | |||
"unicorn/no-console-spaces": "error" | |||
"unicorn/no-console-spaces": "error", | |||
"no-unreadable-array-destructuring": "error" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Isn't this line missing a "unicorn/"
prefix?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yup, thanks for noticing: 6bd6156
At the moment the rule disallows more than one ignored value in series. However I can tune this, I can allow to ignore at most one value at the beginning if you find current behaviour too mild.
Fixes #197