gajus / eslint-plugin-jsdoc

JSDoc specific linting rules for ESLint.
Other
1.09k stars 157 forks source link

Missing fixers #336

Open brettz9 opened 5 years ago

brettz9 commented 5 years ago

The following can have fixers added, with some fixers being by option only:

(To facilitate such fixer work, we can use the new stringify method of https://github.com/yavorskiy/comment-parser/ which allows stringifying (modified) comment-parser JSON back to a string.)

As per #408 these fixers should apply recursively (as with existing ones).

--- Want to back this issue? **[Post a bounty on it!](https://app.bountysource.com/issues/76881896-missing-fixers?utm_campaign=plugin&utm_content=tracker%2F23037809&utm_medium=issues&utm_source=github)** We accept bounties via [Bountysource](https://app.bountysource.com/?utm_campaign=plugin&utm_content=tracker%2F23037809&utm_medium=issues&utm_source=github).
yeonjuan commented 4 years ago

@brettz9 #396 - Adding fixer for require-param is done! plz check it. :)

brettz9 commented 4 years ago

@yeonjuan : Would you mind submitting a PR with:

  1. An additional test for fixing bar in the following:
         /**
           * @param foo
           * @param baz
           */
          function quux (foo, bar, baz) {
          }
  1. Can we not get all params to be fixed in https://github.com/yeonjuan/eslint-plugin-jsdoc/blob/893916ce6e42c24309d27ac6b5eb71deb6014d5a/test/rules/assertions/requireParam.js#L32-L56 , https://github.com/yeonjuan/eslint-plugin-jsdoc/blob/893916ce6e42c24309d27ac6b5eb71deb6014d5a/test/rules/assertions/requireParam.js#L83-L106 and https://github.com/yeonjuan/eslint-plugin-jsdoc/blob/893916ce6e42c24309d27ac6b5eb71deb6014d5a/test/rules/assertions/requireParam.js#L136-L160

  2. I'd also like to know what happens if there is a destructured param like:

          function quux (foo, {bar, baz}) {
          }

Lastly, while I wouldn't expect this now, particularly if we add destructuring support (#11), I just thought I'd mention here that it would be nice to have an option to auto-insert some configurable value like cfg or options:

          /**
           * @param foo
           * @param cfg
           * @param cfg.bar
           * @param cfg.baz
          */
          function quux (foo, {bar, baz}) {
          }
brettz9 commented 4 years ago

I brought up in #404 the idea of our going forward to have the fixers apply all fixes in one go (as with that PR) despite eslint running multiple fix passes for us, with the reason being that the eslint testing framework only shows output with one fix applied.

While I can understand an argument for not changing code to suit testing quirks, I think it is advantageous in our case to do things this way, since our documentation is built on the tests, and I think we all agree we don't want to mislead users into which items are fixed or not.

And I am not aware of any advantages to expressing rules such that they can be used to make only one fix of a given problem.

Any thoughts on this, @gajus ?

brettz9 commented 4 years ago

Another advantage of doing so for us would be that we should be more likely to catch recursive errors such as #403 .

gajus commented 4 years ago

@brettz9 Everything you said makes sense.

yeonjuan commented 4 years ago

@brettz9 👍 Nice! that's the point that I struggled - #404.

brettz9 commented 3 years ago

For cases where multiple types of fixing may be desirable, we can use "suggestions": https://eslint.org/docs/developer-guide/working-with-rules#providing-suggestions

vlafranca commented 1 year ago

I think the require-returns fixer is not also missing, is it possible to add it to the list ? Or am I mistaken ?

brettz9 commented 1 year ago

@vlafranca : Added, thanks!