check-line-alignment forces a misalignment

[TheJaredWilcurt]

Happened when updating from 30.7.6 to 31.2.1

• Reproduction Repo: https://github.com/nwutils/create-desktop-shortcuts

---

My existing block

/**
 * Creates OS based shortcuts for files, folders, and applications.
 *
 * @param  {object}  options  Options object for each OS.
 * @return {boolean}          True = success, false = failed to create the icon
 */

After running -fix with check-line-alignment enabled it outputs this:

/**
 * Creates OS based shortcuts for files, folders, and applications.
 *
 * @param {object} options Options object for each OS.
 * @return {boolean}          True = success, false = failed to create the icon
 */

It was nice and vertically aligned, but, ironically, the linting rule that attempts to enforce such behavior, instead breaks it across all files.

---

Another example:

    /**
     * Returns the value stored in the process.env for a given
     * environment variable.
     *
     * @param  {string} withPercents    '%USERNAME%'
     * @param  {string} withoutPercents 'USERNAME'
     * @return {string}                 'bob' || '%USERNAME%'
     */

is "fixed" to

    /**
     * Returns the value stored in the process.env for a given
     * environment variable.
     *
     * @param {string} withPercents '%USERNAME%'
     * @param {string} withoutPercents 'USERNAME'
     * @return {string}                 'bob' || '%USERNAME%'
     */

---

  'rules': {
    'jsdoc/check-line-alignment': 1
  }

• https://github.com/nwutils/create-desktop-shortcuts/blob/master/.eslintrc.js

---

Environment

• Node.js 14.15.4
• ESLint: 7.18.0
• eslint-plugin-jsdoc: 31.2.1

[brettz9]

@TheJaredWilcurt : The rule has changed so that if you want alignment, you need to add the "always" option:

  'rules': {
    'jsdoc/check-line-alignment': [1, 'always']
  }

However, for your case, the code still does not seem to align properly. @renatho : Do you think you could take a look for the "always" code that is the problem here? I don't think this is related to my later changes which were for the "never" option, as the issue still occurs after adding the "always" option.

(It also looks like the "never" behavior is not fully working either, both because the rule hasn't to date checked @return tags and because of issues that causes when a type is missing).

[renatho]

Hey @brettz9 and @TheJaredWilcurt! o/

Currently, the rule just aligns the comment for the tags: ['param', 'arg', 'argument', 'property', 'prop'].

I tested the following comment:

    /**
     * Returns the value stored in the process.env for a given
     * environment variable.
     *
     * @param  {string} withPercents    '%USERNAME%'
     * @param  {string} withoutPercents 'USERNAME'
     * @return {string}                 'bob' || '%USERNAME%'
     */

And it was fixed to (aligned the @param tags, and ignored the @return, as expected):

    /**
     * Returns the value stored in the process.env for a given
     * environment variable.
     *
     * @param {string} withPercents    '%USERNAME%'
     * @param {string} withoutPercents 'USERNAME'
     * @return {string}                 'bob' || '%USERNAME%'
     */

In my particular case, I'm currently using the comments like that:

    /**
     * Returns the value stored in the process.env for a given
     * environment variable.
     *
     * @param {string} withPercents    '%USERNAME%'
     * @param {string} withoutPercents 'USERNAME'
     *
     * @return {string} 'bob' || '%USERNAME%'
     */

Probably when we update the rule to use the comment-parser, we'll be able to align all the tags (maybe we can add an option to configure the rule to align all the tags or only some). I tested recently that we have an issue in the comment-parser to align the @return tag because it was identifying the first part of the description as the variable name. But I didn't explore so much, and hopefully, we'll have a way to circumvent it. :)

[brettz9]

Hi, @renatho . Thanks for chiming in.

Currently, the rule just aligns the comment for the tags: ['param', 'arg', 'argument', 'property', 'prop'].

I've added a new release (v31.3.3) to support returns by default (and which also fixes the "never" issues I mentioned), and filed #681 which should allow you to configure again through an array of tags mentioned in a tags option. So you might not want to upgrade until that may be merged--sorry about this, as I was making the changes before looking carefully at your comment to see that you wanted the ability not to have returns aligned when doing aligning.

I tested recently that we have an issue in the comment-parser to align the @return tag because it was identifying the first part of the description as the variable name. But I didn't explore so much, and hopefully, we'll have a way to circumvent it. :)

If you were working directly with comment-parser, then yes, that package is agnostic to tag, so it will do that by default. However, in src/iterateJsdoc.js, within the getTokenizers function, our name tokenizer, ensures that this behavior does not get applied for tags where a "name" is not expected, like @returns. So unless you are calling comment-parser directly, you should not see this behavior. Since we are using the updated version, all you have to do to see the output, is introspect on the source of jsdoc or an individual tag.

Note that if you make changes on tags, until comment-parser may automate this, you need to use the special methods on utils (changeTag, setTag, removeTag, addTag) to sync this back up to the main jsdoc.source. You can see that with the "never" option, I use utils.setTag.

This is necessary so as to be able to fully stringify a jsdoc block (not just an individual tag). Although stringifying is done by utils.stringify, you probably will find it more convenient to use utils.reportJSDoc with the specRewire argument set to true, which will in turn call utils.stringify. I use utils.reportJSDoc with the "never" option also.

However, you may need to fix your "always" part of the rule (as I do in my call to utils.tagMightHaveNamepath), as it seems it still does not work with the return examples. But please do so on top of my PR #681 .

[brettz9]

Basically, I think we will want to add these two test cases:

For invalid (noting that the ESLint linter only expects output which fixes the first line of a problem in the output):

{
      code: `
      /**
       * Creates OS based shortcuts for files, folders, and applications.
       *
       * @param {object} options Options object for each OS.
       * @return {boolean} True = success, false = failed to create the icon
       */
       function quux () {}
      `,
      errors: [
        {
          message: 'Expected JSDoc block lines to be aligned.',
          type: 'Block',
        },
        {
          message: 'Expected JSDoc block lines to be aligned.',
          type: 'Block',
        },
      ],
      options: ['always'],
      output: `
      /**
       * Creates OS based shortcuts for files, folders, and applications.
       *
       * @param  {object}  options  Options object for each OS.
       * @return {boolean} True = success, false = failed to create the icon
       */
       function quux () {}
      `,
    },

For valid:

    {
      code: `
      /**
       * Creates OS based shortcuts for files, folders, and applications.
       *
       * @param  {object}  options  Options object for each OS.
       * @return {boolean}          True = success, false = failed to create the icon
       */
       function quux () {}
      `,
      options: ['always'],

[renatho]

Hey @brettz9!

Thank you for your return (pun 😅) and your suggestions. I invested some time today to investigate that. Probably the best option will be already updating it to use the comment-parser through the utils.stringify, as you said. Otherwise, we'll have to create some ugly exceptions for the return to skip the variable name for example. And we'll have other issues if the users want to align other tags with different behaviors (composed of different parts to be aligned).

But I still need to explore the functions you mentioned to understand how we'll handle the @return and other tags correctly. =)

And I'm thinking to extend the utils.stringify to support the comment-parser align. Do you think it's a good approach? Or did you already plan something different to abstract the comment-parser transforms?

[renatho]

Ah, my bad! Actually the eslint-plugin-jsdoc with the new comment-parser already handles that giving the correct position even for the @return tag. But still probably the solution with the stringify will be better making this rule much cleaner :)

[brettz9]

Thank you for your return (pun 😅)

Good one! Hopefully on my "return", I haven't unnecessarily "yielded" too much (re: new @yields support). 🤦

And I'm thinking to extend the utils.stringify to support the comment-parser align. Do you think it's a good approach? Or did you already plan something different to abstract the comment-parser transforms?

I haven't explored the align feature of comment-parser, but of course if it meets our needs, all things being equal, it is probably better to use such already available functionality. I'm afraid I don't follow your train of thought on 'abstracting' transforms, but if you mean implementing our own, I don't have any plans and haven't dug too deeply on that side of things. You're also of course welcome to offer refactoring changes on my own existing code if you find a better or cleaner way. Thanks very much for your work on this!

[renatho]

I'm afraid I don't follow your train of thought on 'abstracting' transforms

I meant if you thought how we can use that. If we just import the transforms from the comment-parser, and we send them through parameter to the utils.stringify, of if we add custom parameters to the utils.stringify like align: true, for example.

[brettz9]

Sure, whatever works! :-)