Skip to content

Commit

Permalink
refactor(semantic): add strict mode in scope flags for class definiti…
Browse files Browse the repository at this point in the history
…ons (#4156)

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`
```js
// 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:
```shell
  × 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
  • Loading branch information
Dunqing committed Jul 14, 2024
1 parent 20cdb1f commit dc2b3c4
Show file tree
Hide file tree
Showing 2 changed files with 7 additions and 50 deletions.
7 changes: 6 additions & 1 deletion crates/oxc_semantic/src/builder.rs
Original file line number Diff line number Diff line change
Expand Up @@ -269,7 +269,6 @@ impl<'a> SemanticBuilder<'a> {

pub fn strict_mode(&self) -> bool {
self.current_scope_flags().is_strict_mode()
|| self.current_node_flags.contains(NodeFlags::Class)
}

pub fn set_function_node_flag(&mut self, flag: NodeFlags) {
Expand Down Expand Up @@ -432,6 +431,12 @@ impl<'a> Visit<'a> for SemanticBuilder<'a> {
if flags.contains(ScopeFlags::Top) { None } else { Some(self.current_scope_id) };

let mut flags = flags;

if !flags.is_strict_mode() && self.current_node_flags.has_class() {
// NOTE A class definition is always strict mode code.
flags |= ScopeFlags::StrictMode;
};

if let Some(parent_scope_id) = parent_scope_id {
flags = self.scope.get_new_scope_flags(flags, parent_scope_id);
}
Expand Down
50 changes: 1 addition & 49 deletions tasks/coverage/parser_test262.snap
Original file line number Diff line number Diff line change
Expand Up @@ -2,60 +2,12 @@ commit: a1587416

parser_test262 Summary:
AST Parsed : 45859/45859 (100.00%)
Positive Passed: 45853/45859 (99.99%)
Positive Passed: 45859/45859 (100.00%)
Negative Passed: 3925/3929 (99.90%)
Expect Syntax Error: "language/import/import-assertions/json-invalid.js"
Expect Syntax Error: "language/import/import-assertions/json-named-bindings.js"
Expect Syntax Error: "language/import/import-attributes/json-invalid.js"
Expect Syntax Error: "language/import/import-attributes/json-named-bindings.js"
Expect to Parse: "language/expressions/class/decorator/syntax/valid/decorator-call-expr-identifier-reference-yield.js"

× The keyword 'yield' is reserved
╭─[language/expressions/class/decorator/syntax/valid/decorator-call-expr-identifier-reference-yield.js:44:10]
43 │
44 │ var C = @yield() class {};
· ─────
╰────
Expect to Parse: "language/expressions/class/decorator/syntax/valid/decorator-member-expr-identifier-reference-yield.js"

× The keyword 'yield' is reserved
╭─[language/expressions/class/decorator/syntax/valid/decorator-member-expr-identifier-reference-yield.js:33:10]
32 │
33 │ var C = @yield class {};
· ─────
╰────
Expect to Parse: "language/expressions/class/decorator/syntax/valid/decorator-parenthesized-expr-identifier-reference-yield.js"

× The keyword 'yield' is reserved
╭─[language/expressions/class/decorator/syntax/valid/decorator-parenthesized-expr-identifier-reference-yield.js:51:11]
50 │
51 │ var C = @(yield) class {};
· ─────
╰────
Expect to Parse: "language/statements/class/decorator/syntax/valid/decorator-call-expr-identifier-reference-yield.js"

× 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 {}
· ─────
╰────
Expect to Parse: "language/statements/class/decorator/syntax/valid/decorator-member-expr-identifier-reference-yield.js"

× The keyword 'yield' is reserved
╭─[language/statements/class/decorator/syntax/valid/decorator-member-expr-identifier-reference-yield.js:34:2]
33 │
34 │ @yield class C {}
· ─────
╰────
Expect to Parse: "language/statements/class/decorator/syntax/valid/decorator-parenthesized-expr-identifier-reference-yield.js"

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

× '0'-prefixed octal literals and octal escape sequences are deprecated
╭─[annexB/language/expressions/template-literal/legacy-octal-escape-sequence-strict.js:18:4]
Expand Down

0 comments on commit dc2b3c4

Please sign in to comment.