Is this.state.isDeclareContext true in d.ts files?

It is true if there is the declare keyword before the function declaration.

In that case this PR doesn't fully fix #13100

We don't have a separate parsing goal for .d.ts files I think, since any code that is valid in .d.ts files was valid in normal .ts files (before we learned about this).

(Q: Do you know if I can use the .d.ts parse goal on the TS playground?)

any code that is valid in .d.ts files was valid in normal .ts

class Foo {
    foo(): any;
}

is invalid in .ts unless it's nested in declare namespace.

As for function foo(...args: any[], ) without the body, it can be used for two different things:

1. for overloads
2. for type-only declarations (inside declare namespace, declare global, declare module "name" { ... }, and probably something else)

In the first case, the rest parameter trailing comma isn't allowed. In the latter, it's allowed as it's an "ambient context".

Do you know if I can use the .d.ts parse goal on the TS playground

I believe if you wrap you code e.g. in declare module 'foo' { ... }, it'll be the same as if it were a d.ts file.

I think we can start by fixing the "easy" case (which is what this PR does), but we'll eventually need to add a { dts: boolean } option to the typescript parser plugin.

From Prettier's perspective, making the error recoverable would suffice.

I've created a new issue so we don't forget #13108

It is need to add tests to check raising this error.

This method is inherited from here adding some adjustments:

Then I'd prefer to do something like this rather than copy-pasting the code:

checkCommaAfterRest(close) {
  if (
    this.state.isDeclareContext &&
    this.match(tt.comma) &&
    this.lookaheadCharCode() === close
  ) {
    this.next(); // tt.comma
  } else {
    super.checkCommaAfterRest(close);
  }
}

by doing so, if we'll make any change to checkCommaAfterRest we don't risk forgetting to also update this second implementation.

Don't we risk to call twice lookaheadCharCode by doing so?
edit: I confused it with lookahead()

Just thinking, maybe we can add some parser contributing.md section (or somewhere) mentioning that for plugins (TS/Flow) we would prefer overwritten methods to use super? Not sure if that's worth making a lint thing either though

Not sure if that's worth making a lint thing either though

It's hard to do so, because we need to know which methods shadow methods on the superclass. Maybe it's doable with tslint, but not with eslint.

Ah yeah I was thinking of a simple heuristic of methods that don't start with ts since it seems we do that? Not a big deal