Update Eslint JSDoc package, introducing JSDoc line alignment check

[renatho]

Fixes #24984

Description

This PR updates the eslint-plugin-jsdoc, which now contains a rule to validate JSDoc alignment, as recommended in WordPress standards.

How has this been tested?

Eslint checking in the Gutenberg code.

Screenshots

Types of changes

It's about Eslint rule updates. As it updates the rules, some new Eslint warnings/errors can appear considering the new rule added and updates in the plugin.

Checklist:

• My code is tested.
• My code follows the WordPress code style.
• My code follows the accessibility standards.
• My code has proper inline documentation.
• I've included developer documentation if appropriate.
• I've updated all React Native files affected by any refactorings/renamings in this PR.

[renatho]

I didn't fix the new reported Gutenbeg Eslint issues in this PR. How should we proceed in cases like that when we update Eslint dependencies?

Maybe the cleaner way could be creating another PR with the fixes and merge it before this PR. If it makes sense, I can create this PR.

[ntwb]

I didn't fix the new reported Gutenbeg Eslint issues in this PR. How should we proceed in cases like that when we update Eslint dependencies?

Maybe the cleaner way could be creating another PR with the fixes and merge it before this PR. If it makes sense, I can create this PR.

This I think makes sense? Is there many instances to fix?

[renatho]

This I think makes sense? Is there many instances to fix?

Yep! There were a lot of issues to fix. But the most part was automatically updated.
I created the separate PR #25352 for this purpose.

I also updated the new rule to warning instead of error here: 4191f71

I think it makes more sense as it's related to documentation format only.

[renatho]

I also updated the new rule to warning instead of error here: 4191f71

I think it makes more sense as it's related to documentation format only.

It'd also allow us to merge before the other PR with the fixes.

[renatho]

I'd like to highlight this issue that @noahtallen found: #25352 (review)

Just to check if someone thinks we should hold this change until we fix that in the plugin. In my opinion, I think it wouldn't be a big deal because it's easy to fix stuff.

[ntwb]

Looking good @renatho, I've left some comments o the follow up PR

[ntwb]

I mentioned this on the PR, but I think this should be explored also:

It may in fact we worthwhile trying to use Prettier prettier-plugin-jsdoc here to update the docs s mentioned here in #24984 (comment) as this may be signifantly quicker for both this PR and future code written will be formatted automatically rather than a devloper pushing up code that then fails linting

[renatho]

I mentioned this on the PR, but I think this should be explored also:

It may in fact we worthwhile trying to use Prettier prettier-plugin-jsdoc here to update the docs s mentioned here in #24984 (comment) as this may be signifantly quicker for both this PR and future code written will be formatted automatically rather than a devloper pushing up code that then fails linting

Hey @ntwb!

I totally agree! It's much easier while developing. I checked that plugin, but running against the Sensei LMS code, it threw some errors for me. I'm planning to take a look at this too and contribute with some PR with fixes if it makes sense.

[renatho]

I'd like to highlight this issue that @noahtallen found: #25352 (review)

Just to check if someone thinks we should hold this change until we fix that in the plugin. In my opinion, I think it wouldn't be a big deal because it's easy to fix stuff.

I added a new PR with fixes to the Eslint plugin here gajus/eslint-plugin-jsdoc#638

The maintainer suggested a different approach which I think makes sense. It'll just take a little more time, so I think I'll keep this PR (and the #25352) in stand by until going with this fix. And in these changes, I can explore more the issues we found while formatting in the #25352.

[gziolo]

@renatho, nice work on this PR. Looking forward to seeing future revisions :)

[gziolo]

I believe it's still blocked by the pending patch in the ESLint plugin, right?

Is this rule fixable, in effect does it work with eslint --fix?

[renatho]

I believe it's still blocked by the pending patch in the ESLint plugin, right?

Actually, now it's waiting for this change: syavorsky/comment-parser#93.
It's a dependency of ESLint plugin, which is being refactored, and it'll include a stringify feature with a format: "align" property that can auto-align the whole JSDoc comment. After this is ready, the ESLint plugin will be updated, and then we can add it to Gutenberg. After that, we can also easily create a prettier rule using the same dependency.

Is this rule fixable, in effect does it work with eslint --fix?

Yes! The current implementation is prepared to autofix, and after the changes it'll continue with this feature, but more complete than the current implementation.

The current implementation only checks and autofix the tags 'param', 'arg', 'argument', 'property', 'prop'. And it wasn't checking if the multi-line descriptions were aligned. The new implementation will check and fix the whole thing. =)

[gziolo]

@renatho, super cool stuff. Thank you for your very detailed response 💯

[skorasaurus]

@gziolo @renatho
FYI: the ticket syavorsky/comment-parser#93 was closed yesterday and 1.0 of comment-parser was released. Details inside that ticket.

[gziolo]

@skorasaurus, great news! I guess, we still need to wait until ESLint plugin gets updated as it's using 0.x version:

https://github.com/gajus/eslint-plugin-jsdoc/blob/2577f66a13eca9f8bac5895ccd0a634048dc5857/package.json#L8

[renatho]

@skorasaurus @gziolo Yep! Now the eslint-plugin-jsdoc needs to be updated to use the new Comment Parser, and we need to refactor the jsdoc/check-line-alignment rule to use it. =)

Very excited about that! As soon as I can I want to make refactor the rule.

[renatho]

Good news! comment-parser is released with their updates and eslint-plugin-jsdoc already updated the dependency! So now we just need to create a PR to eslint-plugin-jsdoc updating the rule to use it! I wanna implement it as soon as I can!

[renatho]

I rebased the branch and fixed the issues again. Is there something I could do in order to go ahead with this PR to avoid more conflicts? 🙂

[kevin940726]

This is looking great to me! 👍

As long as it doesn't break anything, I think we should merge this, and create follow-up PRs for other improvements if there's any.

[gziolo]

I don't see new warnings introduced with this PR so we are good:

As a follow-up, we should promote the reporting level from a warning to error for the new rule, but this is something for the future when people get a chance to update their codebases. One thing that we will have to look at is whether @wordpress/create-block generates JS files that follow the rule. At least the one that is tested with CI job (https://github.com/WordPress/gutenberg/pull/25300/checks?check_run_id=2778385774) looks good in that regard.

[github-actions]

Congratulations on your first merged pull request, @renatho! We'd like to credit you for your contribution in the post announcing the next WordPress release, but we can't find a WordPress.org profile associated with your GitHub account. When you have a moment, visit the following URL and click "link your GitHub account" under "GitHub Username" to link your accounts:

https://profiles.wordpress.org/me/profile/edit/

And if you don't have a WordPress.org account, you can create one on this page:

https://login.wordpress.org/register

Kudos!

[gziolo]

@renatho, congrats on your first merged pull request. It took some time but it required a ton of work so major props for all the time spent to make it happen 👏🏻 🎉

[renatho]

Hey folks! Thank you all for your reviews! And thank you for your patience in this very long PR! 😉

[hypest]

👋 friends! Over at the gutenberg-mobile repo, we seem to run into check failures related to the JSDoc updates from this PR, and I wonder if anyone can give a quick hand or share insight. I'm sure we need to update how we invoke the lint tests at this point but any insight on what's the new proper way to do it will be appreciated! Thanks!

[gziolo]

@hypest, are you sure it's coming from this PR? There are no new errors added here, I would rather look at #31792 first.

[renatho]

👋 friends! Over at the gutenberg-mobile repo, we seem to run into check failures related to the JSDoc updates from this PR, and I wonder if anyone can give a quick hand or share insight. I'm sure we need to update how we invoke the lint tests at this point but any insight on what's the new proper way to do it will be appreciated! Thanks!

Hey there! Checking the Gutenberg Mobile repo, I noticed 2 things.

1. The git submodule is pointing to an older Gutenberg commit, and this old commit had a different version of the eslint-plugin-jsdoc.
2. The package.json from the root also installs the eslint-plugin-jsdoc in a older version.

Could it be related to these questions?

And since the @wordpress/eslint-plugin is coming from an older Gutenberg commit (without the jsdoc rule yet), I didn't identify quickly where it was adding this rule.

[mkevins]

Thanks for the insight @renatho and @gziolo !

Could it be related to these questions?

Indeed it is! I updated the version of eslint-plugin-jsdoc in gutenberg-mobile to align with the version used in gutenberg, which seemed to resolve the issues on CI.

[hypest]

Thanks for the quick look @renatho and @gziolo ❤️ , and 👏 @mkevins for fixing the gutenberg-mobile side now 🙇