Invert Await parameter, add ContainsAwait, updates

[rbuckton]

This inverts the ClassStaticBlockStatementList to pass +Await instead of ~Await, and adds early errors using a ContainsAwait SDO to make it clearer how await should be handled.

This also fixes a few issues and updates the spec text to align with Stage 4 Class Fields. This should also hopefully address the confusion in #43 as well.

Fixes #40
Fixes #41
Fixes #43

[rbuckton]

/cc @jugglinmike

[bakkot]

I haven't reviewed the rest of it, but ContainsAwait LGTM.

[rbuckton]

As soon as my reviewers have had a chance to look at this and approve, I'll start on the ecma262 PR.

[ptomato]

I will try to look through it today or tomorrow.

[ptomato]

It would make this easier to review if the ContainsAwait / invert Await parameter changes were split into a separate commit from the reorganization into the new SDO structure of Ecma-262. I think I managed OK with it as is, but it might help the other reviewers if it were split. However, if that's going to be a big pain, then no need to do it on my account.

[jugglinmike]

The references to ClassBlockStatementList appear to be a typo for ClassStaticBlockStatementList

(Good suggestion about ins, @ptomato --that definitely helped with reviewing!)

[github-actions]

A preview of this PR can be found at https://tc39.es/proposal-class-static-block/pr/46.

[rbuckton]

My apologies for the noise, trying to set up a way to preview changes to the PR... which didn't work :/

[rbuckton]

Ok, now the html preview seems to be working.

[rbuckton]

The references to ClassBlockStatementList appear to be a typo for ClassStaticBlockStatementList

Thanks for pointing that out, this has been fixed.

[Kingwl]

Might need constructor declaration too?

[Kingwl]

And also property declaration?

[rbuckton]

Constructors are just MethodDefinitions with the name constructor. I'll look into the property definition concern, as that may also have been missed by the class fields proposal.

[rbuckton]

Contains gets around this by shunting to ComputedPropertyContains for a ClassTail. It looks like this was missed in ContainsArguments, which would incorrectly look for arguments in a FieldDefinition's initializer. In general this isn't a problem because that would be a static error anyways, but it does mean that ContainsArguments incorrectly and unnecessarily recurs through nested FieldDefinition declarations:

class A {
  static B = class {
    static C = class {
      static D = arguments; //
    }
  }
}

In this case, ContainsArguments would descend all the way to D when checking the static semantics of A, B, and C, when only ContainsArguments for C really needs to do that descent. Instead, ContainsArguments should only check a ComputedPropertyName of the FieldDefinition. This seems to be a spec bug for class fields, which I can file shortly in the ecma262 repo.

@littledan, can you clarify for me why FieldInitializer is specified with ?Await? It seems like it should be ~Await:

While a class field's ComputedPropertyName could contain await in an Await context, an initializer can never be async since its wrapped in its own implicit function. Plus this doesn't make sense:

async function f(p) {
  return class C {
    x = await p; // when could this be awaited?
  };
}
let C = await f(Promise.resolve(1));
let obj = new C();
obj.x; // what is this value?

[rbuckton]

I've filed tc39/ecma262#2438 for the issue with ContainsArguments, and tc39/ecma262#2437 for the issue with FieldDefinition.

[rbuckton]

I've refactored ContainsAwait into two parts to better match Contains, adding a ComputedPropertyContainsAwait that mirrors ComputedPropertyContains. I've also updated ContainsAwait to more accurately mirror Contains except in the specific places where the algorithms must differ.

[rbuckton]

@littledan, as one of the Stage 3 reviewers can you please review this change?

[rbuckton]

@littledan: Friendly ping, thanks.

Just in case @littledan is unavailable: @bakkot, @michaelficarra, @syg do you know if this change would require approval from all stage 3 reviewers? I'm currently leaning towards "yes it does", so I'm willing to wait on @littledan if necessary, but want to be sure.

[bakkot]

do you know if this change would require approval from all stage 3 reviewers

I think for bugfixes we haven't historically really worried about getting re-reviews, as long as there's no doubt that the new semantics are what were originally intended (which is the case here). Editors will of course review again carefully before merging to upstream.

[rbuckton]

If that's the case, then I'd say this is probably fine to merge, since this clarifies the intended semantics without changing any observable behavior. I'll likely have time to start on the ECMA262 PR next week since we're trying to wrap the TypeScript 4.4 beta this week.