Do not issue parse errors in jsdoc under `checkJs`

[weswigham]

These parse errors prevent you from consuming any project without perfect JSDoc, as they prevent you from getting semantic diagnostics in your output. Maybe there should be a flag to enable visibility of all the parse issues we've had with JSDoc, but I don't think the current behavior is very good.

[weswigham]

Ignoring all our real issues parsing valid jsdoc, you find things like this:

  /**
   * Sets the timestamp of this embed.
   * @param {Date} [timestamp=current date] The timestamp
   * @returns {RichEmbed} This embed
   */
  setTimestamp(timestamp = new Date()) {
    this.timestamp = timestamp;
    return this;
  }

which errors

node_modules/discord.js/src/structures/RichEmbed.js(136,39): error TS1005: ']' expected.

which corresponds to the date in [timestamp=current date]. It's certainly not a valid type, but it is valid documentation. Flagging it as a unmovable parse error just feels wrong.

[weswigham]

It's worth noting that you can't even // @ts-ignore these errors right now because 1. you can't ignore parse errors, and 2. you can't put a comment inside a comment. There's actually no way to suppress them other than just not checking your code.

[mhegazy]

@sandersn we need to address this issue either by not reporting the parse errors completely. or by reporting them only under a flag. our position in JS experience was to only make noise when we are sure there is a problem. an invalid comment is not guaranteed to cause an issue, so we should shut-up by default.

[sandersn]

What about reporting the syntactic errors after semantic checking has happened so that you get both?

Note that checkJS is basically a strict JS flag, it just doesn't distinguish whether the jsdoc was intended for type checking or for human reading. When people add new jsdoc annotations to existing (or new) JS files, they expect full errors. Existing jsdoc is not usually machine checked and doesn't do this.

I think delaying jsdoc-syntax errors after semantic checking is a good solution in this case, but we'll face more problems like this that may eventually push us to create a flag.

[sandersn]

I need to examine the errors interactively, but reporting both the syntactic errors and semantics errors seems like it should work fine; the number of errors for chome-devtools goes from 250 to 37172, meaning that the syntactic errors were blocking lots and lots of semantic errors.

Also ts-ignore now works, which is pretty nice.

[sandersn]

Here's a few notes I took. I'll move these somewhere else shortly:

8,000 of the 37,000 errors are "Cannot find namespace X", and I think all (most?) of these are in type names. At least in the one example I looked at, the pattern is as follows:

var Runtime = class {
  /** @param {Runtime.ModuleDescriptor} x */
  m(x) { ... }
}
Runtime.ModuleDescriptor = class {
  // ...
}

I don't know how close we are to being able to support this pattern. In the meantime, chrome dev tools is likely to be too error-ridden to be of real use as a baseline.

[weswigham]

@sandersn We already support statics like that for functions; we just never considered that someone incrementally migrating to es6 may still declare class statics like that. I think we aught to be able to support that pattern.