diff --git a/crates/oxc_parser/src/diagnostics.rs b/crates/oxc_parser/src/diagnostics.rs index 8f5749e6a0828..e2d4c67d6778d 100644 --- a/crates/oxc_parser/src/diagnostics.rs +++ b/crates/oxc_parser/src/diagnostics.rs @@ -1079,8 +1079,11 @@ pub fn invalid_rest_assignment_target(span: Span) -> OxcDiagnostic { } #[cold] -pub fn modifiers_cannot_appear_here(span: Span) -> OxcDiagnostic { - ts_error("1184", "Modifiers cannot appear here.").with_label(span) +pub fn modifiers_cannot_appear_here( + modifier: &Modifier, + _: Option, +) -> OxcDiagnostic { + ts_error("1184", "Modifiers cannot appear here.").with_label(modifier.span) } #[cold] diff --git a/crates/oxc_parser/src/js/class.rs b/crates/oxc_parser/src/js/class.rs index f99329bebc04a..587492925d04d 100644 --- a/crates/oxc_parser/src/js/class.rs +++ b/crates/oxc_parser/src/js/class.rs @@ -224,24 +224,28 @@ impl<'a> ParserImpl<'a> { /* permit_const_as_modifier */ true, /* stop_on_start_of_class_static_block */ true, ); - self.verify_modifiers( - &modifiers, - ModifierFlags::all() - ModifierFlags::EXPORT, - false, - diagnostics::cannot_appear_on_class_elements, - ); // static { block } if self.at(Kind::Static) && self.lexer.peek_token().kind() == Kind::LCurly { for decorator in decorators { self.error(diagnostics::decorators_are_not_valid_here(decorator.span)); } - for modifier in modifiers.iter() { - self.error(diagnostics::modifiers_cannot_appear_here(modifier.span)); - } + self.verify_modifiers( + &modifiers, + ModifierFlags::empty(), + false, + diagnostics::modifiers_cannot_appear_here, + ); return self.parse_class_static_block(span); } + self.verify_modifiers( + &modifiers, + !ModifierFlags::EXPORT, + false, + diagnostics::cannot_appear_on_class_elements, + ); + let r#abstract = modifiers.contains(ModifierKind::Abstract); let r#type = if r#abstract { @@ -282,22 +286,29 @@ impl<'a> ParserImpl<'a> { self.error(diagnostics::decorators_are_not_valid_here(decorator.span)); } - let mut has_seen_readonly_modifier = false; - for modifier in modifiers.iter() { - match modifier.kind { - ModifierKind::Readonly => has_seen_readonly_modifier = true, - ModifierKind::Static => { - if has_seen_readonly_modifier { - self.error(diagnostics::modifier_must_precede_other_modifier( - modifier, - ModifierKind::Readonly, - )); + // No modifiers except `static` and `readonly` are valid here, and they must appear in that order + self.verify_modifiers( + &modifiers, + ModifierFlags::READONLY | ModifierFlags::STATIC, + true, + diagnostics::cannot_appear_on_an_index_signature, + ); + if modifiers.contains_all_flags(ModifierFlags::READONLY | ModifierFlags::STATIC) { + // Has both `readonly` and `static` modifiers. Make sure `static` comes before `readonly`. + let mut has_seen_readonly_modifier = false; + for modifier in modifiers.iter() { + match modifier.kind { + ModifierKind::Readonly => has_seen_readonly_modifier = true, + ModifierKind::Static => { + if has_seen_readonly_modifier { + self.error(diagnostics::modifier_must_precede_other_modifier( + modifier, + ModifierKind::Readonly, + )); + } } + _ => {} } - _ => self.error(diagnostics::cannot_appear_on_an_index_signature( - modifier, - Some(ModifierFlags::READONLY | ModifierFlags::STATIC), - )), } } @@ -322,17 +333,21 @@ impl<'a> ParserImpl<'a> { } fn parse_class_element_name(&mut self, modifiers: &Modifiers<'a>) -> (PropertyKey<'a>, bool) { - for modifier in modifiers.iter() { - match modifier.kind { - ModifierKind::Const => { - self.error(diagnostics::const_class_member(modifier.span)); - } - ModifierKind::In | ModifierKind::Out => { - self.error(diagnostics::can_only_appear_on_a_type_parameter_of_a_class_interface_or_type_alias(modifier.kind, modifier.span)); + self.verify_modifiers( + modifiers, + !(ModifierFlags::CONST | ModifierFlags::IN | ModifierFlags::OUT), + false, + |modifier, _| { + match modifier.kind { + ModifierKind::Const => diagnostics::const_class_member(modifier.span), + ModifierKind::In | ModifierKind::Out => { + diagnostics::can_only_appear_on_a_type_parameter_of_a_class_interface_or_type_alias(modifier.kind, modifier.span) + } + _ => unreachable!(), } - _ => {} - } - } + }, + ); + match self.cur_kind() { Kind::PrivateIdentifier => { let private_ident = self.parse_private_identifier(); @@ -340,7 +355,7 @@ impl<'a> ParserImpl<'a> { if self.is_ts { self.verify_modifiers( modifiers, - ModifierFlags::all() - ModifierFlags::ACCESSIBILITY, + !ModifierFlags::ACCESSIBILITY, false, diagnostics::accessibility_modifier_on_private_property, ); @@ -436,7 +451,7 @@ impl<'a> ParserImpl<'a> { self.check_method_definition_accessor(&method_definition); self.verify_modifiers( modifiers, - ModifierFlags::all() - ModifierFlags::ASYNC - ModifierFlags::DECLARE, + !(ModifierFlags::ASYNC | ModifierFlags::DECLARE), false, diagnostics::modifier_cannot_be_used_here, ); @@ -510,10 +525,23 @@ impl<'a> ParserImpl<'a> { let optional = optional_span.is_some(); if generator || matches!(self.cur_kind(), Kind::LParen | Kind::LAngle) { - for modifier in modifiers.iter() { - match modifier.kind { - ModifierKind::Declare => { - self.error(diagnostics::cannot_appear_on_class_elements( + self.verify_modifiers( + modifiers, + !(ModifierFlags::DECLARE | ModifierFlags::READONLY), + false, + |modifier, _| match modifier.kind { + ModifierKind::Declare => diagnostics::cannot_appear_on_class_elements( + modifier, + Some( + ModifierFlags::ACCESSIBILITY + | ModifierFlags::STATIC + | ModifierFlags::ABSTRACT + | ModifierFlags::OVERRIDE + | ModifierFlags::ASYNC, + ), + ), + ModifierKind::Readonly => { + diagnostics::modifier_only_on_property_declaration_or_index_signature( modifier, Some( ModifierFlags::ACCESSIBILITY @@ -522,25 +550,11 @@ impl<'a> ParserImpl<'a> { | ModifierFlags::OVERRIDE | ModifierFlags::ASYNC, ), - )); + ) } - ModifierKind::Readonly => { - self.error( - diagnostics::modifier_only_on_property_declaration_or_index_signature( - modifier, - Some( - ModifierFlags::ACCESSIBILITY - | ModifierFlags::STATIC - | ModifierFlags::ABSTRACT - | ModifierFlags::OVERRIDE - | ModifierFlags::ASYNC, - ), - ), - ); - } - _ => {} - } - } + _ => unreachable!(), + }, + ); return self.parse_method_declaration( span, r#type, generator, name, computed, optional, modifiers, decorators, ); diff --git a/crates/oxc_parser/src/modifiers.rs b/crates/oxc_parser/src/modifiers.rs index f556c9a3447b3..2196948b14fa5 100644 --- a/crates/oxc_parser/src/modifiers.rs +++ b/crates/oxc_parser/src/modifiers.rs @@ -187,6 +187,10 @@ impl<'a> Modifiers<'a> { self.flags.contains(target.into()) } + pub fn contains_all_flags(&self, flags: ModifierFlags) -> bool { + self.flags.contains(flags) + } + pub fn iter(&self) -> impl Iterator + '_ { self.modifiers.as_ref().into_iter().flat_map(|modifiers| modifiers.iter()) } @@ -495,21 +499,42 @@ impl<'a> ParserImpl<'a> { } } + #[inline] pub(crate) fn verify_modifiers( &mut self, modifiers: &Modifiers<'a>, allowed: ModifierFlags, - // if true, allowed is exact match; if false, allowed is a superset - // used whether to use for the diagnostic message + // If `true`, `allowed` is exact match; if `false`, `allowed` is a superset. + // Used for whether to pass `allowed` to `create_diagnostic` function. strict: bool, - diagnose: F, + create_diagnostic: F, ) where F: Fn(&Modifier, Option) -> OxcDiagnostic, { - for modifier in modifiers.iter() { - if !allowed.contains(modifier.kind.into()) { - self.error(diagnose(modifier, strict.then_some(allowed))); + if modifiers.flags.intersects(!allowed) { + // Invalid modifiers are rare, so handle this case in `#[cold]` function. + // Also `#[inline(never)]` to help `verify_modifiers` to get inlined. + #[cold] + #[inline(never)] + fn report<'a, F>( + parser: &mut ParserImpl<'a>, + modifiers: &Modifiers<'a>, + allowed: ModifierFlags, + strict: bool, + create_diagnostic: F, + ) where + F: Fn(&Modifier, Option) -> OxcDiagnostic, + { + let mut found_invalid_modifier = false; + for modifier in modifiers.iter() { + if !allowed.contains(ModifierFlags::from(modifier.kind)) { + parser.error(create_diagnostic(modifier, strict.then_some(allowed))); + found_invalid_modifier = true; + } + } + debug_assert!(found_invalid_modifier); } + report(self, modifiers, allowed, strict, create_diagnostic); } } } diff --git a/crates/oxc_parser/src/ts/types.rs b/crates/oxc_parser/src/ts/types.rs index fe592e286a61c..21a3cdeea6d9f 100644 --- a/crates/oxc_parser/src/ts/types.rs +++ b/crates/oxc_parser/src/ts/types.rs @@ -1195,15 +1195,13 @@ impl<'a> ParserImpl<'a> { let kind = self.cur_kind(); if kind == Kind::LParen || kind == Kind::LAngle { - for modifier in modifiers.iter() { - if modifier.kind == ModifierKind::Readonly { - self.error( - diagnostics::modifier_only_on_property_declaration_or_index_signature( - modifier, None, - ), - ); - } - } + self.verify_modifiers( + modifiers, + !ModifierFlags::READONLY, + false, + diagnostics::modifier_only_on_property_declaration_or_index_signature, + ); + let type_parameters = self.parse_ts_type_parameters(); let (this_param, params) = self .parse_formal_parameters(FunctionKind::Declaration, FormalParameterKind::Signature);