Add any to some autofixer for no-array-prototype-extensions rule
#1609
Conversation
|
Thank you @tgvrssanthosh ! cc: @bmish |
| url: 'https://github.com/ember-cli/eslint-plugin-ember/tree/master/docs/rules/no-array-prototype-extensions.md', | ||
| }, | ||
| fixable: null, | ||
| fixable: 'code', |
There was a problem hiding this comment.
Wondering shall we use suggestions here for all fixers?
Giving: 1. there are multiple ways of fixing the issue 2. some fixers might not work very well (eg. for those who need to use ember getter/setters). Having it as a suggestion would implicitly encourage them to put more thought into whether to accept the suggestion.
There was a problem hiding this comment.
In this case, I do think we should keep it as an autofixer. Why?
anytosomeis a straightforward conversion where there's only one obvious way of fixing the issue.- An autofixer is a lot more useful as it can be used for mass conversion, whereas a suggestion needs to be manually applied for each instance.
- We already know this rule is not 100% perfect and can experience false positives as described in its rule doc, which is why it's not a recommended rule, but for anyone who still proceeds to enable it, they ideally should be aware of these limitations, and regardless, any resulting fixes still need to go through code review.
I do agree we should consider suggestions for some autofixes, but only under the following circumstances:
- There are multiple ways of fixing an issue and we don't know which way is appropriate for a given instance. Note that multiple ways of fixing an issue does not automatically preclude an autofix as we can still choose an autofix that we think is most likely to be satisfactory. And there could even be a rule option if the user needs to choose which autofixing strategy to use.
- We aren't sure if the fix is correct or there's a chance the fix could break the code.
I hope we can eventually autofix many or most cases for this lint rule to increase the chance of successful migrations off these array prototype methods, and only use suggestions where truly needed. If any of the autofixers turn out to be problematic, we can always convert them to suggestions later.
bmish
changed the title
some -> any autofixer for no-array-prototype-extensions rule
bmish
changed the title
some -> any autofixer for no-array-prototype-extensions ruleany to some autofixer for no-array-prototype-extensions rule
|
@tgvrssanthosh can you run the command to update the README rules list? |
@bmish Ran the script to update the readme file. |
|
@bmish If you are fine with the changes, can you please merge the PR so that I can create follow-up PRs? |
|
Thanks for getting this autofixer started! |
…orn-44.0.0 * master: Add `compact` to `filter` autofixer for `no-array-prototype-extensions` rule (#1611) build(deps-dev): bump release-it from 15.4.2 to 15.5.0 (#1612) build(deps-dev): bump @babel/plugin-proposal-decorators from 7.19.1 to 7.19.3 (#1602) build(deps-dev): bump @typescript-eslint/parser from 5.38.0 to 5.38.1 (#1604) build(deps-dev): bump jsdom from 20.0.0 to 20.0.1 (#1607) build(deps-dev): bump sort-package-json from 1.57.0 to 2.0.0 (#1606) Add `filterBy` to `filter` autofixer for `no-array-prototype-extensions` rule (#1610) Add `any` to `some` autofixer for `no-array-prototype-extensions` rule (#1609)
3 participants
Summary
Added auto-fixer for
no-array-prototype-extensionswhich replacesanywithsome.Description
Recently, the proposal to deprecate array prototype extensions has been approved and got merged. The plan is to add autoFixers to replace ember array prototype extensions with the native array methods. In this PR, an auto fixer has been added which will replace the ember array method
anywith the native array methodsome.Testing
Modified test case to check if the right output is generated after the fix.