feat: Add checkJSDoc option to multiline-comment-style

[laurent22]

Prerequisites checklist

• I have read the contributing guidelines.

What is the purpose of this pull request? (put an "X" next to an item)

[ ] Documentation update
[ ] Bug fix (template)
[ ] New rule (template)
[x] Changes an existing rule (template)

What rule do you want to change?

multiline-comment-style

What change do you want to make (place an "X" next to just one item)?

[ ] Generate more warnings
[ ] Generate fewer warnings
[x] Implement autofix
[x] Implement suggestions

How will the change be implemented (place an "X" next to just one item)?

[x] A new option
[ ] A new default behavior
[ ] Other

Please provide some example code that this change will affect:

With this rule: 'multiline-comment-style': ['error', 'separate-lines', { allowJsDoc: false }], this is not allowed:

/**
 * Some
 * comment
 */

And is converted to this:

// Some
// comment

By default allowJsDoc is true so all other behaviours remain the same.

The rational for adding this is that we disallow block comments in our codebase but some contributors (voluntarily or not) bypass this by formatting the block comment as a jsdoc comment. We don't want this, so this option would help with this.

I have just realised I forgot to update the documentation but happy to do so if you would consider this change.

[linux-foundation-easycla]

The committers listed above are authorized under a signed CLA.

• ✅ login: laurent22 / name: Laurent Cozic (3dde96a)

[eslint-github-bot]

Hi @laurent22!, thanks for the Pull Request

The first commit message isn't properly formatted. We ask that you update the message to match this format, as we use it to generate changelogs and automate releases.

• The commit message tag wasn't recognized. Did you mean "docs", "fix", or "feat"?
• There should be a space following the initial tag and colon, for example 'feat: Message'.
• The first letter of the tag should be in lowercase
• The length of the commit message must be less than or equal to 72

To Fix: You can fix this problem by running git commit --amend, editing your commit message, and then running git push -f to update this pull request.

Read more about contributing to ESLint here

[netlify]

✅ Deploy Preview for docs-eslint ready!

Name	Link
🔨 Latest commit	ccd1f12
🔍 Latest deploy log	https://app.netlify.com/sites/docs-eslint/deploys/64076d24d590e90008189227
😎 Deploy Preview	https://deploy-preview-16807--docs-eslint.netlify.app/rules/multiline-comment-style
📱 Preview on mobile	Toggle QR Code... Use your smartphone camera to open QR code link.

---

To edit notification comments on pull requests, go to your Netlify site settings.

[nzakas]

Thanks for this. Can you explain a bit more about the change you made here? What was the rule not doing?

[laurent22]

Sure, the issue is with this separate-lines option - normally it disallows block comments in favor of consecutive line comments. So this is ok:

// My
// comment

This is not:

/* My
   comment */

The problem, is that there's currently a hard-coded exception for jsdoc. So with the existing rule, this type of block comment is ok:

/**
 * My
 * comment
 */

Basically, formatting the comment as jsdoc allows bypassing the separate-lines option. This might be fine for some codebases but not when jsdoc is not in use (like in our codebase).

So this is why I'm proposing this allowJsDoc option. By default it is true, which means existing configurations won't be affected, but those who don't want this option can set it to false and disallow jsdoc-style comments too.

[aladdin-add]

Thanks for the input. As stated in the README, we actually aren't making any further updates to rules that enforce stylistic preferences. If you have a strong preference for such a change, you can always copy the existing rule into a custom rule and make it behave the way you want.

[laurent22]

Thanks, but isn't it a bug when the definition for separate-lines is "Disallows block comments", but block comments are actually still allowed?

[aladdin-add]

In that way, we just need to update the docs to be more correct?

[github-actions]

Hi everyone, it looks like we lost track of this pull request. Please review and see what the next steps are. This pull request will auto-close in 7 days without an update.

[laurent22]

Changing the doc wouldn't help since the separate-lines parameter would still not be doing what it's supposed to be dong.

[nzakas]

@laurent22 thanks for the explanation. So it sounds like what you want to do is just opt-out of the hardcoded JSDoc exception?

[nzakas]

TSC Summary: This PR seeks to add a way to opt-out of the hardcoded JSDoc comment exception in multiline-comment-style (full description). The multiline-comment-style rule is stylistic and frozen, meaning we won't make any further changes to the rule, but this pull request is fully implemented.

TSC Questions:

1. Should we accept this pull request?
2. Should the option be named allowJsDoc or allowJSDoc?

[btmills]

In the 2023-02-23 TSC meeting, we decided to accept this pull request because this rule was designed with this hard-coded exception but no way to opt out.

@mdjermanovic while preparing the notes, I noticed we never answered part 2: should it be allowJsDoc or allowJSDoc? rg --stats JSDoc lib reports 59 matches across seven JS files for comments and code containing JSDoc, whereas rg --stats JsDoc lib only reports five occurrences of JsDoc, all referencing one checkJsDoc function in lib/rules/require-jsdoc.js. Given that precedent, I think the option should be named allowJSDoc.

[mdjermanovic]

while preparing the notes, I noticed we never answered part 2: should it be allowJsDoc or allowJSDoc? rg --stats JSDoc lib reports 59 matches across seven JS files for comments and code containing JSDoc, whereas rg --stats JsDoc lib only reports five occurrences of JsDoc, all referencing one checkJsDoc function in lib/rules/require-jsdoc.js. Given that precedent, I think the option should be named allowJSDoc.

Indeed, I missed the second part. Agreed, the option should be named allowJSDoc.

[mdjermanovic]

@laurent22 this PR is now accepted, but the option name should be changed to allowJSDoc. You can also update the documentation for this rule now.

[laurent22]

Thank you for considering the PR, I will implement the required changes as soon as possible

[laurent22]

I've now renamed the option to allowJSDoc and added the documentation.

[laurent22]

I believe the pull request is ready now, and just needs to wait for CI

[laurent22]

Thank you for your feedback, I have now renamed the option to checkJSDoc and also updated the documentation as suggested by @nzakas

[laurent22]

Thank you for the change suggestions @mdjermanovic. I've integrated them and ran the unit tests again, so I think it is ready now

[nzakas]

LGTM. Thanks for your patience on this. Just want @mdjermanovic to approve before merging.

[mdjermanovic]

LGTM, thanks!

[nzakas]

@laurent22 we'd like to pay you for this contribution. Please contact us at contact (at) eslint (dot) org and we'll send you the instructions. Thanks!