Less strict parsing for @param tags

[dzearing]

@pgonzal and I were talking in an email thread about parsing params in api-extractor/tsdoc, and suggested I log an issue here to track the param parsing concerns.

Goals for param parsing should be;

1. Public exported classes/functions have correct jsdoc comments describing the thing.
2. If param comments are missing, api-extractor catches them and throws an error.
3. If params are out of order, also worthy of throwing an error.
4. If the params have types in them, api-extractor ignores them and parses the types from typescript typings if available.
5. If the params have dashes in them, api-extractor ignores the dashes. If they are missing dashes, api-extractor ignores that. (The purpose is to extract the docs and api surface, not be nit picky about unimportant things, especially when VSCode jsdoc formatting defaults to having types AND no dashes.)

Examples that should work:

  /**
   * @param foo This is fine.
   * @param foo {boolean} This is fine.
   * @param foo - This is fine.
   * @param foo {boolean} - This is fine.
   * @param foo - {boolean} This is fine.
   */

It is nice to stay consistent with param documentation, but the tools should also be resilient in the parsing and let formatting cleanup be the duty of tools like Prettier.

There might be a strict mode which enforces the VSCode default jsdoc comments. but please do not deviate from what most other open source projects snap to (the second example above.)

See Airbnb specs for their guidance as an example. Look at React jsdoc comments. etc

https://github.com/airbnb/javascript#comments

[octogonz]

2. If param comments are missing, api-extractor catches them and throws an error.

This might be a little overly strict. Consider this example:

/** Represents a 2D vector. */
public class Vector {
  /** The first component of the vector */
  public x: number;
  /** The second component of the vector */
  public y: number;

  /** 
   * Constructs a new instance of the `Vector` class 
   * @param x - the first component of the vector
   * @param y - the second component of the vector
   */
  public constructor(x: number, y: number) {
  }
}

Would we really want to insist that the person write this @param documentation for the constructor? I recall that the old MSDN was fairly dogmatic about this, however this documentation clearly doesn't convey any useful information. Although I'm personally a big advocate of documentation, I tend to be wary of practices that encourage people to think of commenting as an empty formalism or "going through the motions." When we author docs, we should always be thinking about the audience, what questions they will have, and how our content will help them.

3. If params are out of order, also worthy of throwing an error.

Hmm... I hadn't thought of that one. Tools like API Extractor could normalize this automatically when they generate the .d.ts files, but yeah, I can see the value of doing this in the source code.

4. If the params have types in them, api-extractor ignores them and parses the types from typescript typings if available.
5. If the params have dashes in them, api-extractor ignores the dashes. If they are missing dashes, api-extractor ignores that. (The purpose is to extract the docs and api surface, not be nit picky about unimportant things, especially when VSCode jsdoc formatting defaults to having types AND no dashes.)

Agreed. My motivation for being strict about parsing @param tags was to protect against mistakes. But you have a fair point that the documentation tool itself already needs to match the parameter name against the function arguments, so for example if @param This is not fine incorrectly interprets "This" as a parameter name, it will still get caught during matching.

I'll point out that today the @microsoft/tsdoc library only looks at comments -- it has no idea what the function parameters are, or even whether the comment is for a class or function etc. When the dust settles on API Extractor 7, I'd like to consider whether it would be useful to provide some context to @microsoft/tsdoc so that we can move some of this validation down into the library. We might get better error messages that way.

See Airbnb specs for their guidance as an example.

Hrmmm... the example you linked to doesn't use @param at all. It might not be the best example of a modern style guide any more: 😋

// good
/**
 * make() returns a new element
 * based on the passed-in tag name
 */
function make(tag) {

  // ...

  return element;
}

[dzearing]

Oops on the link; edited above and replaced with a jsdoc one that's more applicable.

This might be a little overly strict.

I should have worded that better; i was thinking more about "if you provide comments for one param, you should provide them for all". Or, if someone modifies a signature but the comments don't match (e.g. they're out of order, some are unrecognized or missing) then that could be caught. Totally agree we definitely don't want it to be frictiony by erroring on things that are not useful to correct. :)

[rbuckton]

This still seems to be an issue for @typeParam, without the hyphen it fails to parse the parameter name for the tag.

[octogonz]

I tagged it for follow up. If it's easy maybe I can tackle this in the next round of work on the parser.

[rbuckton]

Locally I wrote a patch for NodeParser.prototype._parseParamBlock in JS to partially address this case:

    var hyphenExcerpt;
    var spacingAfterHyphenExcerpt;
    if (tokenReader.peekTokenKind() === Token_1.TokenKind.Hyphen) {
        tokenReader.readToken();
        hyphenExcerpt = tokenReader.extractAccumulatedSequence();
        // TODO: Only read one space
        spacingAfterHyphenExcerpt = this._tryReadSpacingAndNewlines(tokenReader);
    }
    else if (tokenReader.peekTokenKind() !== Token_1.TokenKind.AsciiWord) {
        tokenReader.backtrackToMarker(startMarker);
        this._parserContext.log.addMessageForTokenSequence("tsdoc-param-tag-missing-hyphen" /* ParamTagMissingHyphen */, 'The @param block should be followed by a parameter name and then a hyphen', docBlockTag.getTokenSequence(), docBlockTag);
        return new nodes_1.DocParamBlock({
            configuration: this._configuration,
            blockTag: docBlockTag,
            parameterName: ''
        });
    }

Essentially, it allows @typeParam T this is a comment (or @param p this is a comment), but it doesn't yet permit @typeParam T {constraint} comment (or @param p {type} comment).

[rbuckton]

I formalized my patch above and added additional parsing support for {type} and [name]/[name=default] syntax in a PR at #206.

[octogonz]

This fix was published with TSDoc 0.12.18. Thanks @rbuckton for making the PR!