From 1af5ed3d89c16dab6d9f63323753e9bc8d7121ea Mon Sep 17 00:00:00 2001 From: Boshen <1430279+Boshen@users.noreply.github.com> Date: Sun, 23 Jun 2024 10:34:55 +0000 Subject: [PATCH] refactor(ast)!: replace `Modifiers` with `declare` and `const` on `EnumDeclaration` (#3845) --- crates/oxc_ast/src/ast/mod.rs | 4 ++ crates/oxc_ast/src/ast/ts.rs | 4 +- crates/oxc_ast/src/ast_builder.rs | 5 +- crates/oxc_ast/src/ast_impl/js.rs | 2 +- crates/oxc_ast/src/ast_impl/ts.rs | 8 +-- crates/oxc_codegen/src/gen.rs | 7 +-- crates/oxc_isolated_declarations/src/enum.rs | 10 +++- .../oxc_linter/src/rules/oxc/no_const_enum.rs | 15 +++--- crates/oxc_parser/src/ts/statement.rs | 23 ++++++-- crates/oxc_semantic/src/binder.rs | 2 +- crates/oxc_semantic/src/checker/mod.rs | 2 +- crates/oxc_semantic/src/checker/typescript.rs | 7 +-- crates/oxc_transformer/src/typescript/enum.rs | 2 +- crates/oxc_traverse/src/ancestor.rs | 28 +++++----- tasks/coverage/parser_typescript.snap | 52 +++++++++---------- 15 files changed, 98 insertions(+), 73 deletions(-) diff --git a/crates/oxc_ast/src/ast/mod.rs b/crates/oxc_ast/src/ast/mod.rs index 51441ef82bb8d..2c9b1fb49b913 100644 --- 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 index da7d9519cbc8a..53e1ec11dc14f 100644 --- 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>, } diff --git a/crates/oxc_ast/src/ast_builder.rs b/crates/oxc_ast/src/ast_builder.rs index 0afc747317d5e..a3ef09dc50910 100644 --- 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 index 08f56d10dc4e2..f9430026eceba 100644 --- 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 index 639c21509b055..1e5af02caf98c 100644 --- a/crates/oxc_ast/src/ast_impl/ts.rs +++ b/crates/oxc_ast/src/ast_impl/ts.rs @@ -19,9 +19,10 @@ 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() } } } @@ -29,7 +30,8 @@ impl<'a> Hash for TSEnumDeclaration<'a> { fn hash(&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 index a09e6d2ad3ba4..ed415e4bb4427 100644 --- a/crates/oxc_codegen/src/gen.rs +++ b/crates/oxc_codegen/src/gen.rs @@ -3357,13 +3357,10 @@ impl<'a, const MINIFY: bool> Gen for TSInterfaceHeritage<'a> { impl<'a, const MINIFY: bool> Gen 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 index df4ce36e9a9cc..113d23af8aff9 100644 --- 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 index ac57a0983c129..c1a94195080c2 100644 --- 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 index 283b715570516..b26fbe1952eb9 100644 --- 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> { 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> { @@ -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 index d3fbc67f9636e..b06821c5f4c5b 100644 --- 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 index 76798224500b2..7d521891fcfc3 100644 --- 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 index 353a25db4200c..f3e73cc825552 100644 --- 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 index b4648db4566b4..7d053a0a43e71 100644 --- 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> { - 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 index 4d2069d2f52b8..7fece0005c756 100644 --- 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 index 66bf96afb519a..306fdd28ed83e 100644 --- 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 { 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 { 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 { 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 {