refactor(ast)!: replace Modifiers with declare and const on `En…

…umDeclaration` (#3845)
oxc-project · Jun 23, 2024 · 1af5ed3 · 1af5ed3
1 parent 0673677
commit 1af5ed3
Show file tree

Hide file tree

Showing 15 changed files with 98 additions and 73 deletions.
diff --git a/crates/oxc_ast/src/ast/mod.rs b/crates/oxc_ast/src/ast/mod.rs
@@ -241,6 +241,10 @@ impl<'a> Modifiers<'a> {
         self.0.as_ref().and_then(|modifiers| modifiers.iter().find(|modifier| f(modifier)))
     }
 
+    pub fn is_contains_const(&self) -> bool {
+        self.contains(ModifierKind::Const)
+    }
+
     pub fn is_contains_declare(&self) -> bool {
         self.contains(ModifierKind::Declare)
     }

diff --git a/crates/oxc_ast/src/ast/ts.rs b/crates/oxc_ast/src/ast/ts.rs
@@ -55,8 +55,8 @@ pub struct TSEnumDeclaration<'a> {
     pub span: Span,
     pub id: BindingIdentifier<'a>,
     pub members: Vec<'a, TSEnumMember<'a>>,
-    /// Valid Modifiers: `const`, `export`, `declare`
-    pub modifiers: Modifiers<'a>,
+    pub r#const: bool,
+    pub declare: bool,
     pub scope_id: Cell<Option<ScopeId>>,
 }
 

diff --git a/crates/oxc_ast/src/ast_builder.rs b/crates/oxc_ast/src/ast_builder.rs
@@ -1883,10 +1883,11 @@ impl<'a> AstBuilder<'a> {
         span: Span,
         id: BindingIdentifier<'a>,
         members: Vec<'a, TSEnumMember<'a>>,
-        modifiers: Modifiers<'a>,
+        r#const: bool,
+        declare: bool,
     ) -> Declaration<'a> {
         Declaration::TSEnumDeclaration(
-            self.alloc(TSEnumDeclaration::new(span, id, members, modifiers)),
+            self.alloc(TSEnumDeclaration::new(span, id, members, r#const, declare)),
         )
     }
 

diff --git a/crates/oxc_ast/src/ast_impl/js.rs b/crates/oxc_ast/src/ast_impl/js.rs
@@ -710,7 +710,7 @@ impl<'a> Declaration<'a> {
             Declaration::VariableDeclaration(decl) => decl.declare,
             Declaration::FunctionDeclaration(decl) => decl.declare,
             Declaration::ClassDeclaration(decl) => decl.declare,
-            Declaration::TSEnumDeclaration(decl) => decl.modifiers.is_contains_declare(),
+            Declaration::TSEnumDeclaration(decl) => decl.declare,
             Declaration::TSTypeAliasDeclaration(decl) => decl.modifiers.is_contains_declare(),
             Declaration::TSModuleDeclaration(decl) => decl.modifiers.is_contains_declare(),
             Declaration::TSInterfaceDeclaration(decl) => decl.modifiers.is_contains_declare(),

diff --git a/crates/oxc_ast/src/ast_impl/ts.rs b/crates/oxc_ast/src/ast_impl/ts.rs
@@ -19,17 +19,19 @@ impl<'a> TSEnumDeclaration<'a> {
         span: Span,
         id: BindingIdentifier<'a>,
         members: Vec<'a, TSEnumMember<'a>>,
-        modifiers: Modifiers<'a>,
+        r#const: bool,
+        declare: bool,
     ) -> Self {
-        Self { span, id, members, modifiers, scope_id: Cell::default() }
+        Self { span, id, members, r#const, declare, scope_id: Cell::default() }
     }
 }
 
 impl<'a> Hash for TSEnumDeclaration<'a> {
     fn hash<H: std::hash::Hasher>(&self, state: &mut H) {
         self.id.hash(state);
         self.members.hash(state);
-        self.modifiers.hash(state);
+        self.r#const.hash(state);
+        self.declare.hash(state);
     }
 }
 

diff --git a/crates/oxc_codegen/src/gen.rs b/crates/oxc_codegen/src/gen.rs
@@ -3357,13 +3357,10 @@ impl<'a, const MINIFY: bool> Gen<MINIFY> for TSInterfaceHeritage<'a> {
 impl<'a, const MINIFY: bool> Gen<MINIFY> for TSEnumDeclaration<'a> {
     fn gen(&self, p: &mut Codegen<{ MINIFY }>, ctx: Context) {
         p.print_indent();
-        if self.modifiers.contains(ModifierKind::Export) {
-            p.print_str(b"export ");
-        }
-        if self.modifiers.contains(ModifierKind::Declare) {
+        if self.declare {
             p.print_str(b"declare ");
         }
-        if self.modifiers.contains(ModifierKind::Const) {
+        if self.r#const {
             p.print_str(b"const ");
         }
         p.print_space_before_identifier();

diff --git a/crates/oxc_isolated_declarations/src/enum.rs b/crates/oxc_isolated_declarations/src/enum.rs
@@ -91,11 +91,17 @@ impl<'a> IsolatedDeclarations<'a> {
             members.push(member);
         }
         let mut modifiers = self.modifiers_declare();
-        if decl.modifiers.contains(ModifierKind::Const) {
+        if decl.r#const {
             modifiers.add_modifier(Modifier { span: SPAN, kind: ModifierKind::Const });
         }
 
-        Some(self.ast.ts_enum_declaration(decl.span, self.ast.copy(&decl.id), members, modifiers))
+        Some(self.ast.ts_enum_declaration(
+            decl.span,
+            self.ast.copy(&decl.id),
+            members,
+            modifiers.is_contains_const(),
+            modifiers.is_contains_declare(),
+        ))
     }
 
     /// Evaluate the expression to a constant value.

diff --git a/crates/oxc_linter/src/rules/oxc/no_const_enum.rs b/crates/oxc_linter/src/rules/oxc/no_const_enum.rs
@@ -1,4 +1,4 @@
-use oxc_ast::{ast::ModifierKind, AstKind};
+use oxc_ast::AstKind;
 use oxc_diagnostics::OxcDiagnostic;
 use oxc_macros::declare_oxc_lint;
 use oxc_span::Span;
@@ -41,17 +41,16 @@ declare_oxc_lint!(
 impl Rule for NoConstEnum {
     fn run<'a>(&self, node: &AstNode<'a>, ctx: &LintContext<'a>) {
         if let AstKind::TSEnumDeclaration(enum_decl) = node.kind() {
-            let Some(const_enum) = enum_decl
-                .modifiers
-                .find_where(|modifier| matches!(modifier.kind, ModifierKind::Const))
-            else {
+            if !enum_decl.r#const {
                 return;
-            };
+            }
 
-            ctx.diagnostic_with_fix(no_const_enum_diagnostic(const_enum.span), |fixer| {
+            let span = Span::new(enum_decl.span.start, enum_decl.span.start + 5);
+
+            ctx.diagnostic_with_fix(no_const_enum_diagnostic(span), |fixer| {
                 // const enum Color { Red, Green, Blue }
                 // ^
-                let start = const_enum.span.start;
+                let start = span.start;
 
                 // const enum Color { Red, Green, Blue }
                 // ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

diff --git a/crates/oxc_parser/src/ts/statement.rs b/crates/oxc_parser/src/ts/statement.rs
@@ -8,6 +8,7 @@ use super::{
     types::ModifierFlags,
 };
 use crate::{
+    diagnostics,
     js::{FunctionKind, VariableDeclarationContext, VariableDeclarationParent},
     lexer::Kind,
     list::{NormalList, SeparatedList},
@@ -25,14 +26,30 @@ impl<'a> ParserImpl<'a> {
     pub(crate) fn parse_ts_enum_declaration(
         &mut self,
         span: Span,
-        modifiers: Modifiers<'a>,
+        modifiers: &Modifiers<'a>,
     ) -> Result<Declaration<'a>> {
         self.bump_any(); // bump `enum`
 
         let id = self.parse_binding_identifier()?;
         let members = TSEnumMemberList::parse(self)?.members;
         let span = self.end_span(span);
-        Ok(self.ast.ts_enum_declaration(span, id, members, modifiers))
+
+        for modifier in modifiers.iter() {
+            if !matches!(modifier.kind, ModifierKind::Declare | ModifierKind::Const) {
+                self.error(diagnostics::modifiers_cannot_appear(
+                    modifier.span,
+                    modifier.kind.as_str(),
+                ));
+            }
+        }
+
+        Ok(self.ast.ts_enum_declaration(
+            span,
+            id,
+            members,
+            modifiers.is_contains_const(),
+            modifiers.is_contains_declare(),
+        ))
     }
 
     pub(crate) fn parse_ts_enum_member(&mut self) -> Result<TSEnumMember<'a>> {
@@ -289,7 +306,7 @@ impl<'a> ParserImpl<'a> {
                     .map(Declaration::TSModuleDeclaration)
             }
             Kind::Type => self.parse_ts_type_alias_declaration(start_span, modifiers),
-            Kind::Enum => self.parse_ts_enum_declaration(start_span, modifiers),
+            Kind::Enum => self.parse_ts_enum_declaration(start_span, &modifiers),
             Kind::Interface if self.is_at_interface_declaration() => {
                 self.parse_ts_interface_declaration(start_span, modifiers)
             }

diff --git a/crates/oxc_semantic/src/binder.rs b/crates/oxc_semantic/src/binder.rs
@@ -320,7 +320,7 @@ impl<'a> Binder for TSInterfaceDeclaration<'a> {
 
 impl<'a> Binder for TSEnumDeclaration<'a> {
     fn bind(&self, builder: &mut SemanticBuilder) {
-        let is_const = self.modifiers.contains(ModifierKind::Const);
+        let is_const = self.r#const;
         let includes = if is_const { SymbolFlags::ConstEnum } else { SymbolFlags::RegularEnum };
         let excludes = if is_const {
             SymbolFlags::ConstEnumExcludes

diff --git a/crates/oxc_semantic/src/checker/mod.rs b/crates/oxc_semantic/src/checker/mod.rs
@@ -98,7 +98,7 @@ pub fn check<'a>(node: &AstNode<'a>, ctx: &SemanticBuilder<'a>) {
             ts::check_ts_type_parameter_declaration(declaration, ctx);
         }
         AstKind::TSModuleDeclaration(decl) => ts::check_ts_module_declaration(decl, node, ctx),
-        AstKind::TSEnumDeclaration(decl) => ts::check_ts_enum_declaration(decl, node, ctx),
+        AstKind::TSEnumDeclaration(decl) => ts::check_ts_enum_declaration(decl, ctx),
         AstKind::TSTypeAliasDeclaration(decl) => {
             ts::check_ts_type_alias_declaration(decl, node, ctx);
         }

diff --git a/crates/oxc_semantic/src/checker/typescript.rs b/crates/oxc_semantic/src/checker/typescript.rs
@@ -223,13 +223,8 @@ fn enum_member_must_have_initializer(span0: Span) -> OxcDiagnostic {
     OxcDiagnostic::error("Enum member must have initializer.").with_labels([span0.into()])
 }
 
-pub fn check_ts_enum_declaration<'a>(
-    decl: &TSEnumDeclaration<'a>,
-    node: &AstNode<'a>,
-    ctx: &SemanticBuilder<'a>,
-) {
+pub fn check_ts_enum_declaration<'a>(decl: &TSEnumDeclaration<'a>, ctx: &SemanticBuilder<'a>) {
     let mut need_initializer = false;
-    check_declaration_modifiers(&decl.modifiers, node, ctx);
 
     decl.members.iter().for_each(|member| {
         #[allow(clippy::unnested_or_patterns)]

diff --git a/crates/oxc_transformer/src/typescript/enum.rs b/crates/oxc_transformer/src/typescript/enum.rs
@@ -59,7 +59,7 @@ impl<'a> TypeScriptEnum<'a> {
         is_export: bool,
         ctx: &TraverseCtx<'a>,
     ) -> Option<Statement<'a>> {
-        if decl.modifiers.contains(ModifierKind::Declare) {
+        if decl.declare {
             return None;
         }
 

diff --git a/crates/oxc_traverse/src/ancestor.rs b/crates/oxc_traverse/src/ancestor.rs
@@ -8664,8 +8664,8 @@ impl<'a> TSThisParameterWithoutTypeAnnotation<'a> {
 pub(crate) const OFFSET_TS_ENUM_DECLARATION_SPAN: usize = offset_of!(TSEnumDeclaration, span);
 pub(crate) const OFFSET_TS_ENUM_DECLARATION_ID: usize = offset_of!(TSEnumDeclaration, id);
 pub(crate) const OFFSET_TS_ENUM_DECLARATION_MEMBERS: usize = offset_of!(TSEnumDeclaration, members);
-pub(crate) const OFFSET_TS_ENUM_DECLARATION_MODIFIERS: usize =
-    offset_of!(TSEnumDeclaration, modifiers);
+pub(crate) const OFFSET_TS_ENUM_DECLARATION_CONST: usize = offset_of!(TSEnumDeclaration, r#const);
+pub(crate) const OFFSET_TS_ENUM_DECLARATION_DECLARE: usize = offset_of!(TSEnumDeclaration, declare);
 pub(crate) const OFFSET_TS_ENUM_DECLARATION_SCOPE_ID: usize =
     offset_of!(TSEnumDeclaration, scope_id);
 
@@ -8688,11 +8688,13 @@ impl<'a> TSEnumDeclarationWithoutId<'a> {
     }
 
     #[inline]
-    pub fn modifiers(&self) -> &Modifiers<'a> {
-        unsafe {
-            &*((self.0 as *const u8).add(OFFSET_TS_ENUM_DECLARATION_MODIFIERS)
-                as *const Modifiers<'a>)
-        }
+    pub fn r#const(&self) -> &bool {
+        unsafe { &*((self.0 as *const u8).add(OFFSET_TS_ENUM_DECLARATION_CONST) as *const bool) }
+    }
+
+    #[inline]
+    pub fn declare(&self) -> &bool {
+        unsafe { &*((self.0 as *const u8).add(OFFSET_TS_ENUM_DECLARATION_DECLARE) as *const bool) }
     }
 
     #[inline]
@@ -8723,11 +8725,13 @@ impl<'a> TSEnumDeclarationWithoutMembers<'a> {
     }
 
     #[inline]
-    pub fn modifiers(&self) -> &Modifiers<'a> {
-        unsafe {
-            &*((self.0 as *const u8).add(OFFSET_TS_ENUM_DECLARATION_MODIFIERS)
-                as *const Modifiers<'a>)
-        }
+    pub fn r#const(&self) -> &bool {
+        unsafe { &*((self.0 as *const u8).add(OFFSET_TS_ENUM_DECLARATION_CONST) as *const bool) }
+    }
+
+    #[inline]
+    pub fn declare(&self) -> &bool {
+        unsafe { &*((self.0 as *const u8).add(OFFSET_TS_ENUM_DECLARATION_DECLARE) as *const bool) }
     }
 
     #[inline]

diff --git a/tasks/coverage/parser_typescript.snap b/tasks/coverage/parser_typescript.snap
@@ -10683,7 +10683,7 @@ Expect to Parse: "conformance/salsa/plainJSRedeclare3.ts"
  3 │   }
    ╰────
 
-  × 'async' modifier cannot be used here.
+  × TS1044: 'async' modifier cannot appear on a module or namespace element.
    ╭─[conformance/async/es5/asyncEnum_es5.ts:1:1]
  1 │ async enum E {
    · ─────
@@ -10818,7 +10818,7 @@ Expect to Parse: "conformance/salsa/plainJSRedeclare3.ts"
  3 │   }
    ╰────
 
-  × 'async' modifier cannot be used here.
+  × TS1044: 'async' modifier cannot appear on a module or namespace element.
    ╭─[conformance/async/es6/asyncEnum_es6.ts:1:1]
  1 │ async enum E {  
    · ─────
@@ -16237,6 +16237,14 @@ Expect to Parse: "conformance/salsa/plainJSRedeclare3.ts"
  16 │ }
     ╰────
 
+  × TS1044: 'public' modifier cannot appear on a module or namespace element.
+    ╭─[conformance/internalModules/moduleBody/invalidModuleWithStatementsOfEveryKind.ts:25:5]
+ 24 │ module Y4 {
+ 25 │     public enum Color { Blue, Red }
+    ·     ──────
+ 26 │ }
+    ╰────
+
   × TS1044: 'private' modifier cannot appear on a module or namespace element.
     ╭─[conformance/internalModules/moduleBody/invalidModuleWithStatementsOfEveryKind.ts:29:5]
  28 │ module YY {
@@ -16269,6 +16277,14 @@ Expect to Parse: "conformance/salsa/plainJSRedeclare3.ts"
  41 │ }
     ╰────
 
+  × TS1044: 'private' modifier cannot appear on a module or namespace element.
+    ╭─[conformance/internalModules/moduleBody/invalidModuleWithStatementsOfEveryKind.ts:50:5]
+ 49 │ module YY4 {
+ 50 │     private enum Color { Blue, Red }
+    ·     ───────
+ 51 │ }
+    ╰────
+
   × TS1044: 'static' modifier cannot appear on a module or namespace element.
     ╭─[conformance/internalModules/moduleBody/invalidModuleWithStatementsOfEveryKind.ts:55:5]
  54 │ module YYY {
@@ -16301,6 +16317,14 @@ Expect to Parse: "conformance/salsa/plainJSRedeclare3.ts"
  67 │ }
     ╰────
 
+  × TS1044: 'static' modifier cannot appear on a module or namespace element.
+    ╭─[conformance/internalModules/moduleBody/invalidModuleWithStatementsOfEveryKind.ts:76:5]
+ 75 │ module YYY4 {
+ 76 │     static enum Color { Blue, Red }
+    ·     ──────
+ 77 │ }
+    ╰────
+
   × Modifiers cannot be used here.
     ╭─[conformance/internalModules/moduleBody/invalidModuleWithStatementsOfEveryKind.ts:13:5]
  12 │     public class AA<T> { s: T }
@@ -16317,14 +16341,6 @@ Expect to Parse: "conformance/salsa/plainJSRedeclare3.ts"
  20 │         class A { s: string }
     ╰────
 
-  × Modifiers cannot be used here.
-    ╭─[conformance/internalModules/moduleBody/invalidModuleWithStatementsOfEveryKind.ts:25:5]
- 24 │ module Y4 {
- 25 │     public enum Color { Blue, Red }
-    ·     ──────
- 26 │ }
-    ╰────
-
   × Modifiers cannot be used here.
     ╭─[conformance/internalModules/moduleBody/invalidModuleWithStatementsOfEveryKind.ts:38:5]
  37 │     private class AA<T> { s: T }
@@ -16341,14 +16357,6 @@ Expect to Parse: "conformance/salsa/plainJSRedeclare3.ts"
  45 │         class A { s: string }
     ╰────
 
-  × Modifiers cannot be used here.
-    ╭─[conformance/internalModules/moduleBody/invalidModuleWithStatementsOfEveryKind.ts:50:5]
- 49 │ module YY4 {
- 50 │     private enum Color { Blue, Red }
-    ·     ───────
- 51 │ }
-    ╰────
-
   × Modifiers cannot be used here.
     ╭─[conformance/internalModules/moduleBody/invalidModuleWithStatementsOfEveryKind.ts:64:5]
  63 │     static class AA<T> { s: T }
@@ -16365,14 +16373,6 @@ Expect to Parse: "conformance/salsa/plainJSRedeclare3.ts"
  71 │         class A { s: string }
     ╰────
 
-  × Modifiers cannot be used here.
-    ╭─[conformance/internalModules/moduleBody/invalidModuleWithStatementsOfEveryKind.ts:76:5]
- 75 │ module YYY4 {
- 76 │     static enum Color { Blue, Red }
-    ·     ──────
- 77 │ }
-    ╰────
-
   × TS1044: 'public' modifier cannot appear on a module or namespace element.
    ╭─[conformance/internalModules/moduleBody/invalidModuleWithVarStatements.ts:4:5]
  3 │ module Y {