Emit grammar error on quoted constructors and class fields named “constructor”

[andrewbranch]

Phase 1 of #31020

This is an inherently awkward error message to give. Basically the message tells you that “This syntax compiles in every version of TypeScript except this one,” which isn’t particularly actionable. So I decided to explain how you can get either a constructor or a method in a backward-compatible and future-proof way:

In the meantime, consider using 'constructor()' to write a constructor, or '"constructor"' to write a method.

On the one hand, this makes the message actionable, no matter what your intent. On the other hand, what I’d rather say is more like “oh no please don’t define a method named constructor 😭”

[RyanCavanaugh]

This is the kind of thing it's nice to seed a StackOverflow Q/A for

[RyanCavanaugh]

We can use a normal code here; it's fine to introduce a gap in the sequence later

[andrewbranch]

I was confused about why 18004 was way down here. Now I see that's the latest of an earlier sequence.

[andrewbranch]

Updated to 18005

[weswigham]

We also need to consider emittin an error for properties named "constructor". In ES2018+, class C { "constructor"; } will throw with Uncaught SyntaxError: Classes may not have a field named 'constructor' 😦

[andrewbranch]

class C {
  "constructor" = 3; // Error in ES2018+
  ["constructor"] = 3; // No error and has exactly the same effect
}

I understand why this is, but it makes me very sad

[andrewbranch]

@weswigham should this trigger an error

1. if your script target is >= ES2018, because that's when node will give you an error
2. if your script target is >= ES2015, because that's when we emit code that will give you an error if you run it in latest node versions
3. all the time, because if JavaScript made something illegal going forward because it has confusing semantics, TypeScript should probably do the same

(I don't think option 1 is a real option, I just wanted to lay out all the arguments)

[weswigham]

if your script target is >= ES2015, because that's when we emit code that will give you an error if you run it in latest node versions

In the case of the constructor property, doesn't it only cause an error when we don't transform it? Isn't our downlevel to an assignment within the constructor perfectly serviceable (at least until we adopt define semantics)?

[weswigham]

Probably 3 though, I think. At least that's the stance we've usually taken. If we can keep backwards compatible emit/checking behavior for it so it can be @ts-ignore'd in older codebases, all the better.

[andrewbranch]

In the case of the constructor property, doesn't it only cause an error when we don't transform it? Isn't our downlevel to an assignment within the constructor perfectly serviceable

Oh right, I was only thinking about when we start emitting classes. Assigning in the constructor still works. [Verifies that assertion] ... [Is not a liar]

I agree though, seems reasonable to disallow this invariantly regardless. Chances are, if someone is currently doing this, they’ll get hit by it eventually. May as well be sooner rather than later, and they’re better off catching it at compile time than at runtime.