Typescript: `@implements` should parse a type reference, not an expression

[sandersn]

Reusing the code from @augments is incorrect, unless that should also change from expression to type reference. I'm not sure.

/** @implements {!Type} */
class C { }

Expected behavior:
No parse error.

Actual behavior:
Parse error: "Identifier expected"

[sandersn]

@dragomirtitian you might want to know about this. It showed up on our user tests: https://github.com/microsoft/TypeScript/pull/37063.
I'll take a look tomorrow if you haven't got started on it.

[dragomirtitian]

Is ! allowed in implements? In TS it would not be (ex). Or is the preference here that this should not be a parse error and it should be a checker error instead ?

BTW: Not sure about the behavior for extends and augments either.
This is valid TS:

class Base { }

class Derived extends Base! {
}

Playground Link

But this is not valid in JSDoc:

class Base { }

/**
 * @augments {!Base}
 */
class Derived extends Base {
}

(Playground does not show JS errors)

[sandersn]

Nope! You are right! I had forgotten that extends and implements are both parsed uniformly as ExpressionWithTypeArguments.

So I guess the decision is between an ad-hoc jsdoc parse, a good error, and the current error. I'll look at chrome-devtools to see what the errors on our snapshot look like. (we pinned our test to an old commit to minimise error churn) If the chrome-devtools doesn't have [many] of these errors, it will make the decision easier.

@Timvdlippe You might have an opinion on this given chrome-devtools's current code.

[sandersn]

chrome-devtools has no new "Identifier expected" errors. So that's something. Still looking at the specifics...

[sandersn]

I've looked at about 10% of the new errors in chrome-devtools, enough to understand probably 90% of the unique problems. Basically, this bug is not a problem for chrome-devtools. However, I did find an actual problem:

1. Errors from @implements on a class property assignment:

/** @implements {T} */
X.Y = class {
}
// also
/** @implements {T} */
var z = class {
}

Note that the interface check works fine, the error is just issued too much. It's just a matter of fixing the error.

Everything else so far has been expected errors, correct or incorrect:

1. Exposes existing errors instantiating generics
2. Exposes existing errors with binding namespaces.
3. Exposes an actual incorrect implementation, or lack of type annotations so that TS.

[sandersn]

Filed #37108 to track the error. I'll look at the rest of chrome-devtools errors later today to make sure no surprises are left, then close this bug afterward.

[TimvdLippe]

The clause should indeed not have any operator such as ! or ?.

I am not sure which version of devtools you use. It might be worthwhile to sync, as we have migrated to es modules. That should reduce the amount of errors Typescript produces.

We have also kicked of the migration to TS this week, with a couple of (minor) issues. On Monday I will post an overview issue on this repository. That allows us to track all blockers and notify both the Edge DevTools team as well as the TS team.

[sandersn]

It's pretty ancient: "1.0.530099". I'm a little reluctant to upgrade just because we'll lose coverage of how a pre-module code base works, although I suppose fewer and fewer people have those. Maybe we should keep both; the current version will become more and more valuable the closer it gets to 0 errors.

(From https://github.com/microsoft/TypeScript/blob/master/tests/cases/user/chrome-devtools-frontend/package.json)