Eslint-plugin-jsdoc: 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:

```js
'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! :-)

[renatho]

Hey @brettz9!

I started a PR to align the @return, and the other tags too. The code is much cleaner now using comment-parser stringify 😁. I thought we had a way to stringify like the never rule too, but it seems not, so I kept that part without touch. ;)

I found 2 issues in the comment parser align transform, and I created some temporary workarounds (commented in the code) to make the rule work. The issues are:

• https://github.com/syavorsky/comment-parser/issues/119
• https://github.com/syavorsky/comment-parser/issues/120

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 () {}
      `,
    },

I'm sorry, but I didn't understand exactly that. Do you want the @return not aligned by default? And you expect 2 errors in the array or was it a mistake?

In the PR, only the tag option is still pending because I still don't know how to do that.

And just to let you know, I'm starting a 2 weeks vacation and I'll travel soon 🏖️, so I'm not sure if I'll be able to finish that before my travel.

[brettz9]

Hi @renatho ,

I started a PR to align the @return, and the other tags too. The code is much cleaner now using comment-parser stringify 😁.

Excellent! Looking good!

I thought we had a way to stringify like the never rule too, but it seems not, so I kept that part without touch. ;)

I guess you mean how it uses the reportJSDoc utility which then calls stringify? Yeah, currently seems your approach works (though feel free to refactor utilities as needed).

I found 2 issues in the comment parser align transform, and I created some temporary workarounds (commented in the code) to make the rule work. The issues are:

* [syavorsky/comment-parser#119](https://github.com/syavorsky/comment-parser/issues/119)

* [syavorsky/comment-parser#120](https://github.com/syavorsky/comment-parser/issues/120)

Cool. Appreciate the references here and in the code as you've done.

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 () {}
      `,
    },

I'm sorry, but I didn't understand exactly that. Do you want the @return not aligned by default? And you expect 2 errors in the array or was it a mistake?

I'm not sure whether people would consider having each line reported (as with the "never" option) to be noisy, but the "never" approach might be superior in identifying the precise tags (and thus line number) having problems, so I think we should try to do that if possible. When calling report (or reportJSDoc), one can supply a tag object (or a pseudo-tag object) whose line number info will be used when reporting.

No, the @return tag should be aligned in the same way as the other tags, but the reason the output in the test code I supplied doesn't fix that tag is because the ESLint testing framework doesn't work recursively, so the output unfortunately is only supposed to show the first fix being applied, i.e., in this case, the @param. But I've tested it works fine recursively when run with regular ESLint.

In the PR, only the tag option is still pending because I still don't know how to do that.

I wonder whether we'd want the alignment transformer to support a callback or whitelist/blacklist or something to determine which tags should be aligned.

And just to let you know, I'm starting a 2 weeks vacation and I'll travel soon 🏖️, so I'm not sure if I'll be able to finish that before my travel.

Sure, no worries... Enjoy! I think I will mention in the README now that tags currently only works with "never".

[renatho]

I guess you mean how it uses the reportJSDoc utility which then calls stringify?

I meant that it'd be nice if we could make something similar with the stringify to the never option, but there is no clean way like the align() to remove the alignment. :)

Sure, no worries... Enjoy! I think I will mention in the README now that tags currently only works with "never".

Thank you, man! I worked a little more trying to finish it checking by tag, instead of the whole block, as you suggested (and the tags option became easier with that). But it's not complete. So I'm putting the current PR as ready, in case you wanna merge it, and I'll create a new one with the unfinished work. ;)

[TheJaredWilcurt]

So it looks like this was fixed and merged in and released. But I'm not sure what my linting rule should be so that everything passes.

• https://github.com/nwutils/create-desktop-shortcuts/pull/29/files

currently gives me 60 warnings and doing --fix just breaks things

[brettz9]

tags used with always is not presently working. @renatho has a PR at https://github.com/renatho/eslint-plugin-jsdoc/pull/1 but was going on a couple week vacation. In the meantime, anyone can feel free to see about completing it.

I did add a note to the docs that there is this current lack of support for tags when "always" is set, though I was not aware of other issues (besides some comment-parser issues being worked around).

It seems from this though that one issue is that we are looking at all tags, e.g., @example in the block at https://github.com/nwutils/create-desktop-shortcuts/blob/d89454b3ec19ab4e98463b4e2f5ce9bcd03eb8c9/src/validation.js#L16-L25 is challenging enough with the @example (which you'd probably want to exclude in tags and/or be excluded with our default for tags once we can distinguish between tags with the "always" option).

And even after simplifying to this:

const validation = {
      /**
       * Creates, validates, and/or defaults the options object
       * and its values, including global settings, and each OS.
       *
       * @param  {object} options  User's options
       * @return {object}          Validated or mutated user options
       */
      validateOptions: function (options) {
      }
    }

..another question is raised because it seems that the aligner we are using is stripping the extra space between the name and description, so that it produces:

* @param  {object} options User's options
* @return {object}          Validated or mutated user options

instead of leaving it as:

* @param  {object} options  User's options
* @return {object}          Validated or mutated user options

(The reason there are still two spaces after the first param seems to be because it is necessary to keep aligned with@return)

I don't think this particular issue is too problematic as the Wordpress doc page referenced in our docs gives an example where only one space is present. However, if feasible, perhaps we can add config to allow extra whitespace in the first line (or first line with a name or description) to be used as the basis for further alignment (or maybe optionally the line with the max spacing), e.g., so that having extra padding is ok, but just not misaligned padding, e.g., like:

 * @param
 * @param {object}       Ok here.
 * @param {object}       Ok too.

I had initially marked the issue as fixed because it should no longer misalign as in your original examples (though in the first example, again, we would currently remove one space while keeping it aligned), assuming you have them aligned properly and to a minimum, but I will reopen as we do of course at least want to be able to allow choices of tags, and if someone can implement, an option to allow extra padding. I think we will probably need our own aligner function as the comment-parser one doesn't distinguish tags.

I imagine that should be enough to worry about for now, but if other issues remain, we can look at that time.

[TheJaredWilcurt]

okay :)

I just add two spaces so it doesn't look like connected words in a sentence. Showing a clear delineation between one section and the next. I like the idea of using the line with the max spacing as the basis for aligning the rest

[gajus]

:tada: This issue has been resolved in version 31.6.1 :tada:

The release is available on:

• npm package (@latest dist-tag)
• GitHub release

Your semantic-release bot :package::rocket:

[brettz9]

I just add two spaces so it doesn't look like connected words in a sentence. Showing a clear delineation between one section and the next. I like the idea of using the line with the max spacing as the basis for aligning the rest

Just bear in mind that if you accidentally have a lot of extra whitespace somewhere, it will force that on all tags.

Btw, please ignore the notice about this being fixed. That was an automated notice since the partial fix has now been released.

@renatho : I've pushed an update for comment-parser. I also dropped the temporary fixes you had added for the sake of those bugs, but I unfortunately released before remembering to add them in the release. I think they can wait for the next release.

[TheJaredWilcurt]

This goes at the top of my file and it is affected. I assume this is a bug

- Expected
+ Outcome of using --fix

 /**
  * @file    Entry point for the library. Exposes the external facing function that accepts the input defined in the API documentation.
- * @author  TheJaredWilcurt
+ * @author        TheJaredWilcurt
  */

You had mentioned that the @example would be tricky, this is what it's doing with 31.6.1
```diff
/**

• Creates OS based shortcuts for files, folders, and applications.
  *
• @example
  
  
  
  • 
    
    
    • createDesktopShortcut({
    
    
    
  
  
  • 
    
    
    • windows: { filePath: 'C:\path\to\executable.exe' },
    
    
    
  
  
  • 
    
    
    • linux: { filePath: '/home/path/to/executable' },
    
    
    
  
  
  • 
    
    
    • osx: { filePath: '/home/path/to/executable' }
    
    
    
  
  
  • 
    
    
    • });
    
    
    
  
  
  • 
    
    
    • createDesktopShortcut({
    
    
    
  
  
  • 
    
    
    • windows: { filePath: 'C:\path\to\executable.exe' },
    
    
    
  
  
  • 
    
    
    • linux: { filePath: '/home/path/to/executable' },
    
    
    
  
  
  • 
    
    
    • osx: { filePath: '/home/path/to/executable' }
    
    
    
  
  
  • 
    
    
    • });
      
      
      *
    
    
    
  
  
  • 
    
    
    • @param {object} options Options object for each OS.
    
    
    
  
  
  • 
    
    
    • @return {boolean} True = success, false = failed to create the icon or set its permissions (Linux).
    
    
    
  
  
  • 
    
    
    • @param {object} options Options object for each OS.
    
    
    
  
  
  • 
    
    
    • @return {boolean} True = success, false = failed to create the icon or set its permissions (Linux).
      
      
      */
      
      
      ```
    
    
    
  
  

---

EDIT: Nevermind, false alarm, just saw the "ignore this, it isn't fixed" note

[brettz9]

Yes, as mentioned, you have to ignore the automatic notice here about the issue being fixed. The issue is not yet fully fixed. It could be a couple weeks or so. In the meantime, you might wish to disable the rule.

[renatho]

Hey @TheJaredWilcurt !

I took a quick break from my vacation to finish that. =)

This PR adds the support for the tags also when using the always option: https://github.com/gajus/eslint-plugin-jsdoc/pull/689. I didn't test it in a project, I tested it only through the unit tests. So if someone has the opportunity to confirm if it's working properly, I'd appreciate it. =)

[renatho]

Ah, I didn't do anything for the extra spacings in this PR 👆. If someone wants to implement that option, feel free to. =)

[brettz9]

Thanks, I think we indeed ought to treat that as a separate issue. Thanks for taking a look on the issue. But please do get the rest you need.

When you do have time though, I did test against a larger code base (svgedit), and found problems with the likes of this (all with always):

/**
 * @returns {string} The string
 * @todo Do this
 */
function test () {
}

and the following only when it is multi-line

    /**
     * @property {string} entry Multi
     * line
     * @todo Do this
    */

also:

/**
 * @param {boolean} [opts.noAlert]
 * @throws {Error} Upon failure to load SVG
 */

And this one only gets an error during fix:

/**
 * @param {boolean} isIt
 * @throws {Error} Something
 */
function load (a) {
}

Let me know and I can try once more if you get a fix for these cases.