Skip to content

refactor(parser): shorten code handling illegal modifiers #20781

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
graphite-app merged 1 commit into from
Mar 29, 2026
+44 −126
Merged

refactor(parser): shorten code handling illegal modifiers #20781

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
168 changes: 43 additions & 125 deletions crates/oxc_parser/src/modifiers.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -341,6 +341,19 @@ mod modifier_kinds {
Self(self.0 & !(1 << (kind as u8)))
}

/// Return intersection of this set with `other`.
///
/// # Example
/// ```ignore
/// let kinds1 = ModifierKinds::new([ModifierKind::Public, ModifierKind::Async]);
/// let kinds2 = ModifierKinds::new([ModifierKind::Async, ModifierKind::Static]);
/// assert_eq!(kinds1.intersection(kinds2), ModifierKinds::new([ModifierKind::Async]));
/// ```
#[inline]
pub const fn intersection(self, other: Self) -> Self {
Self(self.0 & other.0)
}

/// Count how many [`ModifierKind`]s are in this set.
#[inline]
pub const fn count(self) -> usize {
Expand Down Expand Up @@ -567,7 +580,6 @@ static ILLEGAL_PRECEDING_MODIFIERS: [ModifierKinds; ModifierKind::VARIANTS.len()
};

/// Get which modifiers are illegal to precede a modifier.
/// This must match the logic in `illegal_modifier_error`.
const fn get_illegal_preceding_modifiers(kind: ModifierKind) -> ModifierKinds {
match kind {
ModifierKind::Public | ModifierKind::Private | ModifierKind::Protected => {
Expand Down Expand Up @@ -633,133 +645,39 @@ impl<C: Config> ParserImpl<'_, C> {
/// Create an error for an illegal modifier.
#[cold]
#[inline(never)]
fn illegal_modifier_error(&mut self, kinds: ModifierKinds, modifier: &Modifier) {
match modifier.kind {
ModifierKind::Public | ModifierKind::Private | ModifierKind::Protected => {
if kinds.intersects(ModifierKinds::new([
ModifierKind::Public,
ModifierKind::Private,
ModifierKind::Protected,
])) {
self.error(diagnostics::accessibility_modifier_already_seen(modifier));
} else if kinds.contains(ModifierKind::Override) {
self.error(diagnostics::modifier_must_precede_other_modifier(
modifier,
ModifierKind::Override,
));
} else if kinds.contains(ModifierKind::Static) {
self.error(diagnostics::modifier_must_precede_other_modifier(
modifier,
ModifierKind::Static,
));
} else if kinds.contains(ModifierKind::Accessor) {
self.error(diagnostics::modifier_must_precede_other_modifier(
modifier,
ModifierKind::Accessor,
));
} else if kinds.contains(ModifierKind::Readonly) {
self.error(diagnostics::modifier_must_precede_other_modifier(
modifier,
ModifierKind::Readonly,
));
} else if kinds.contains(ModifierKind::Async) {
self.error(diagnostics::modifier_must_precede_other_modifier(
modifier,
ModifierKind::Async,
));
} else if kinds.contains(ModifierKind::Abstract) {
self.error(diagnostics::modifier_must_precede_other_modifier(
modifier,
ModifierKind::Abstract,
));
}
fn illegal_modifier_error(&mut self, existing_kinds: ModifierKinds, modifier: &Modifier) {
const ACCESSIBILITY_KINDS: ModifierKinds = ModifierKinds::new([
ModifierKind::Public,
ModifierKind::Private,
ModifierKind::Protected,
]);

let this_kind = modifier.kind;
let this_kinds = ModifierKinds::new([this_kind]);

if this_kinds.intersects(ACCESSIBILITY_KINDS) {
// This modifier is `public`, `private`, or `protected`.
// Using multiple accessibility modifiers is illegal.
if existing_kinds.intersects(ACCESSIBILITY_KINDS) {
self.error(diagnostics::accessibility_modifier_already_seen(modifier));
return;
}
ModifierKind::Static => {
if kinds.contains(ModifierKind::Static) {
self.error(diagnostics::modifier_already_seen(modifier));
} else if kinds.contains(ModifierKind::Readonly) {
self.error(diagnostics::modifier_must_precede_other_modifier(
modifier,
ModifierKind::Readonly,
));
} else if kinds.contains(ModifierKind::Async) {
self.error(diagnostics::modifier_must_precede_other_modifier(
modifier,
ModifierKind::Async,
));
} else if kinds.contains(ModifierKind::Accessor) {
self.error(diagnostics::modifier_must_precede_other_modifier(
modifier,
ModifierKind::Accessor,
));
} else if kinds.contains(ModifierKind::Override) {
self.error(diagnostics::modifier_must_precede_other_modifier(
modifier,
ModifierKind::Override,
));
}
}
ModifierKind::Override => {
if kinds.contains(ModifierKind::Override) {
self.error(diagnostics::modifier_already_seen(modifier));
} else if kinds.contains(ModifierKind::Readonly) {
self.error(diagnostics::modifier_must_precede_other_modifier(
modifier,
ModifierKind::Readonly,
));
} else if kinds.contains(ModifierKind::Accessor) {
self.error(diagnostics::modifier_must_precede_other_modifier(
modifier,
ModifierKind::Accessor,
));
} else if kinds.contains(ModifierKind::Async) {
self.error(diagnostics::modifier_must_precede_other_modifier(
modifier,
ModifierKind::Async,
));
}
}
ModifierKind::Abstract => {
if kinds.contains(ModifierKind::Abstract) {
self.error(diagnostics::modifier_already_seen(modifier));
} else if kinds.contains(ModifierKind::Override) {
self.error(diagnostics::modifier_must_precede_other_modifier(
modifier,
ModifierKind::Override,
));
} else if kinds.contains(ModifierKind::Accessor) {
self.error(diagnostics::modifier_must_precede_other_modifier(
modifier,
ModifierKind::Accessor,
));
}
}
ModifierKind::Export => {
if kinds.contains(ModifierKind::Export) {
self.error(diagnostics::modifier_already_seen(modifier));
} else if kinds.contains(ModifierKind::Declare) {
self.error(diagnostics::modifier_must_precede_other_modifier(
modifier,
ModifierKind::Declare,
));
} else if kinds.contains(ModifierKind::Abstract) {
self.error(diagnostics::modifier_must_precede_other_modifier(
modifier,
ModifierKind::Abstract,
));
} else if kinds.contains(ModifierKind::Async) {
self.error(diagnostics::modifier_must_precede_other_modifier(
modifier,
ModifierKind::Async,
));
}
}
_ => {
if kinds.contains(modifier.kind) {
self.error(diagnostics::modifier_already_seen(modifier));
}
} else {
// Modifiers cannot be repeated
if existing_kinds.intersects(this_kinds) {
self.error(diagnostics::modifier_already_seen(modifier));
return;
}
}

let illegal_preceding_modifier_kinds = ILLEGAL_PRECEDING_MODIFIERS[this_kind as usize];

// `illegal_preceding_modifier_kinds` are modifiers which this modifier cannot follow.
// Find which of them it *is* following, and raise an error for one of them.
// If multiple illegal kinds, it's arbitrary which one the error is raised for.
let illegal_kinds = illegal_preceding_modifier_kinds.intersection(existing_kinds);
let illegal_kind = illegal_kinds.iter().next().unwrap();
self.error(diagnostics::modifier_must_precede_other_modifier(modifier, illegal_kind));
}

#[inline]
Expand Down
2 changes: 1 addition & 1 deletion tasks/coverage/snapshots/parser_babel.snap
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12527,7 +12527,7 @@ Expect to Parse: tasks/coverage/babel/packages/babel-parser/test/fixtures/typesc
3 │ }
╰────

× TS(1029): 'public' modifier must precede 'override' modifier.
× TS(1029): 'public' modifier must precede 'readonly' modifier.
╭─[babel/packages/babel-parser/test/fixtures/typescript/class/constructor-with-invalid-order-modifiers-3/input.ts:2:33]
1 │ class D extends B {
2 │ constructor(override readonly public foo: string) {}
Expand Down
Loading