refactor(semantic): add strict mode in scope flags for class definitions

[Dunqing]

related: #4142 (comment)

Although we called enter_node(Class), that doesn't mean we're in the class scope. It causes our must to visit decorators before enter_node.

Let's look at this case. It causes a syntax error if we don't visit decorators before enter_node

// This file was procedurally generated from the following sources:
// - src/decorator/decorator-call-expr-identifier-reference-yield.case
// - src/decorator/syntax/valid/cls-expr-decorators-valid-syntax.template
/*---
description: Decorator @ DecoratorCallExpression (Valid syntax for decorator on class expression)
esid: prod-ClassExpression
features: [class, decorators]
flags: [generated, noStrict]
info: |
    ClassExpression[Yield, Await] :
      DecoratorList[?Yield, ?Await]opt class BindingIdentifier[?Yield, ?Await]opt ClassTail[?Yield, ?Await]

    DecoratorList[Yield, Await] :
      DecoratorList[?Yield, ?Await]opt Decorator[?Yield, ?Await]

    Decorator[Yield, Await] :
      @ DecoratorMemberExpression[?Yield, ?Await]
      @ DecoratorParenthesizedExpression[?Yield, ?Await]
      @ DecoratorCallExpression[?Yield, ?Await]

    ...


    DecoratorCallExpression[Yield, Await] :
      DecoratorMemberExpression[?Yield, ?Await] Arguments[?Yield, ?Await]

    DecoratorMemberExpression[Yield, Await] :
      IdentifierReference[?Yield, ?Await]
      DecoratorMemberExpression[?Yield, ?Await] . IdentifierName
      DecoratorMemberExpression[?Yield, ?Await] . PrivateIdentifier

    IdentifierReference[Yield, Await] :
      [~Yield] yield
      ...

---*/
function decorator() {
  return () => {};
}
var yield = decorator;



var C = @yield() class {};

Errors:

  × The keyword 'yield' is reserved
    ╭─[language/statements/class/decorator/syntax/valid/decorator-call-expr-identifier-reference-yield.js:45:2]
 44 │ 
 45 │ @yield() class C {}
    ·  ─────
    ╰────

The changed code makes more sense. Only if we call enter_scope for class, the flags will contain StrictMode. Also, we can get the exact flags of the scope in the class at the transformer

For example:

class A {
   B() {
       // Before: flags is `Function`
      //  After: flags is `Function | StrictMode`
   }
}

The regression tests will be fixed in follow-up PRs

[graphite-app]

Your org has enabled the Graphite merge queue for merging into main

Add the label “merge” to the PR and Graphite will automatically add it to the merge queue when it’s ready to merge. Or use the label “hotfix” to add to the merge queue as a hot fix.

You must have a Graphite account and log in to Graphite in order to use the merge queue. Sign up using this link.

[Dunqing]

• feat(semantic): add some scopes to align TypeScript's container #3969
• refactor(semantic): add strict mode in scope flags for class definitions #4156 👈
• main

This stack of pull requests is managed by Graphite. Learn more about stacking.

Join @Dunqing and the rest of your teammates on Graphite

[codspeed-hq]

CodSpeed Performance Report

Merging #4156 will not alter performance

_{Comparing 07-10-refactor_semantic_add_strict_mode_in_scope_flags_for_class_definitions (dc2b3c4) with main (20cdb1f)}

Summary

✅ 32 untouched benchmarks

[Boshen]

I can't keep up with who should review this anymore, can you request the right person to review?

[overlookmotel]

I'm afraid I don't have time this week to get into this properly, but a few thoughts:

1. Can we solve Inherit ScopeFlags::StrictMode from parent scope #3862 first? The logic around strict mode in scopes is a bit of a mess, so it's hard to understand whether further changes to it are correct or not.

2. Strict mode also applies to the extends clause as well as the class body:

// Sloppy mode
var yield = class {};
class X extends yield {}
// SyntaxError: Unexpected strict mode reserved word

3. I don't understand this: Should this be a syntax error or not?

// Sloppy mode
var C = @yield() class {};

i.e. is the decorator considered part of the class itself, or just sugar for:

var C = yield()(class {});

[Dunqing]

I'm afraid I don't have time this week to get into this properly, but a few thoughts:

1. Can we solve Inherit ScopeFlags::StrictMode from parent scope #3862 first? The logic around strict mode in scopes is a bit of a mess, so it's hard to understand whether further changes to it are correct or not.

Oh, I just saw this issue and this PR would solve the second point of #3862 (comment). The first point I can create another PR to fix

2. Strict mode also applies to the extends clause as well as the class body:

// Sloppy mode
var yield = class {};
class X extends yield {}
// SyntaxError: Unexpected strict mode reserved word

Yes

3. I don't understand this: Should this be a syntax error or not?

// Sloppy mode
var C = @yield() class {};

Should not be a syntax error. Because @yield is not inside class

i.e. is the decorator considered part of the class itself, or just sugar for:

var C = yield()(class {});

You can think of it as sugar

[overlookmotel]

Should not be a syntax error. Because @yield is not inside class

In that case, Babel's decorator transform is incorrect (Babel REPL). Should we raise an issue on Babel?

[Dunqing]

Should not be a syntax error. Because @yield is not inside class

In that case, Babel's decorator transform is incorrect (Babel REPL). Should we raise an issue on Babel?

Looks correct in the latest version Babel REPL

[overlookmotel]

Looks correct in the latest version Babel REPL

Ah ha yes you're right.

[graphite-app]

Merge activity

• Jul 13, 11:15 PM EDT: The merge label 'merge' was detected. This PR will be added to the Graphite merge queue once it meets the requirements.
• Jul 13, 11:40 PM EDT: Boshen merged this pull request with the Graphite merge queue.