From dc2b3c44fb2dc99f1838c8fb4c97eafe58980d0d Mon Sep 17 00:00:00 2001 From: Dunqing <29533304+Dunqing@users.noreply.github.com> Date: Sun, 14 Jul 2024 03:35:12 +0000 Subject: [PATCH] refactor(semantic): add strict mode in scope flags for class definitions (#4156) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit related: https://github.com/oxc-project/oxc/issues/4142#issuecomment-2219125356 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 --- crates/oxc_semantic/src/builder.rs | 7 ++++- tasks/coverage/parser_test262.snap | 50 +----------------------------- 2 files changed, 7 insertions(+), 50 deletions(-) diff --git a/crates/oxc_semantic/src/builder.rs b/crates/oxc_semantic/src/builder.rs index fb97b57573f05..1be4ce0963df1 100644 --- a/crates/oxc_semantic/src/builder.rs +++ b/crates/oxc_semantic/src/builder.rs @@ -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) { @@ -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); } diff --git a/tasks/coverage/parser_test262.snap b/tasks/coverage/parser_test262.snap index ad84a4c5b0c87..7f24f39228e43 100644 --- a/tasks/coverage/parser_test262.snap +++ b/tasks/coverage/parser_test262.snap @@ -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]