search

cartant / eslint-plugin-rxjs

ESLint rules for RxJS
MIT License
311 stars 36 forks source link

feat(prefer-observer): add fix suggestions #92

Closed benlesh closed 2 years ago

benlesh commented 2 years ago

I've (attempted at least) to add a rudimentary fix for instances where users are using the deprecated signatures of tap and subscribe.

cartant commented 2 years ago

I usually specify a fix and a suggestion using the same fix implementation. For example, this fix:

https://github.com/cartant/eslint-plugin-etc/blob/a671952ec685c57ae40c20424c5cc7124cd8a5ca/source/rules/prefer-less-than.ts#L37-L48

gets used as a fix:

https://github.com/cartant/eslint-plugin-etc/blob/a671952ec685c57ae40c20424c5cc7124cd8a5ca/source/rules/prefer-less-than.ts#L49-L50

and as a suggestion:

https://github.com/cartant/eslint-plugin-etc/blob/a671952ec685c57ae40c20424c5cc7124cd8a5ca/source/rules/prefer-less-than.ts#L53-L58

And in the test, the specified output tests the fix:

https://github.com/cartant/eslint-plugin-etc/blob/a671952ec685c57ae40c20424c5cc7124cd8a5ca/tests/rules/prefer-less-than.ts#L162-L164

and the suggestion in the suggestions array (in conjunction with the suggest annotation in the fromFixture call) tests the suggestion:

https://github.com/cartant/eslint-plugin-etc/blob/a671952ec685c57ae40c20424c5cc7124cd8a5ca/tests/rules/prefer-less-than.ts#L165-L172

And - note to self - I'll need to remember to bump the major version of this when we merge this, as some folks might be surprised by the fixer.

benlesh commented 2 years ago

Okay, I've added both fixes and suggestions and appropriate tests.

However, please notice that the output isn't "perfectly" formatted. I wasn't entirely sure what to do there, and I was unable to find any best practices. Presumably people will have their own autoformatters (Pretter, clangformat, etc) that they'll want to use regardless if eslint is outputting code.