refactor(es/parser): Compare token kind rather than strings

[CPunisher]

Description:

This PR refactors the parser to use Token comparisons instead of string comparisons for checking keywords, modifiers, and identifiers. Previously, the parser often checked ident.sym == "keyword".

[changeset-bot]

🦋 Changeset detected

Latest commit: dd3f1c9

The changes in this PR will be included in the next version bump.

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

[Copilot]

Pull request overview

This PR refactors the TypeScript parser to use Token comparisons instead of string comparisons when checking for keywords, modifiers, and identifiers. Previously, the parser often checked ident.sym == "keyword", which is less efficient and type-safe than directly comparing the token type before parsing.

Changes:

• Updated parse_ts_modifier to accept and return Token types instead of string slices
• Refactored all call sites to capture the token before parsing and compare tokens instead of identifier symbols
• Updated error message generation to convert tokens back to strings only when needed for error reporting

Reviewed changes

Copilot reviewed 9 out of 9 changed files in this pull request and generated 1 comment.

Show a summary per file

File	Description
typescript_stubs.rs	Updated function signatures for parse_ts_modifier to use Token types
typescript.rs	Main refactoring of modifier parsing and type parameter handling to use token comparisons
stmt.rs	Updated statement parsing to capture and compare tokens for interface, public, static, abstract
pat.rs	Updated parameter parsing to pass Token arrays to parse_ts_modifier
object.rs	Updated object property parsing to capture key tokens and compare for get, set, async
module_item.rs	Updated import/export specifier parsing to capture and compare tokens for type and as keywords
ident.rs	Updated binding identifier parsing to compare tokens for await and yield
expr.rs	Updated expression parsing to capture and compare tokens for as keyword
class_and_fn.rs	Updated class member parsing to use token comparisons; contains one critical bug

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[codspeed-hq]

Merging this PR will not alter performance

✅ 184 untouched benchmarks

---

_{Comparing 02-04-refactor/token (dd3f1c9) with main (3e63627)¹}

Footnotes

1. No successful run was found on main (f523e4b) during the generation of this report, so 3e63627 was used instead as the comparison base. There might be some changes unrelated to this pull request in this report. ↩

[Copilot]

Pull request overview

Copilot reviewed 9 out of 9 changed files in this pull request and generated no new comments.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[github-actions]

Binary Sizes

File	Size
swc.linux-x64-gnu.node	28M (28689992 bytes)

Commit: 6dc4e75

[claude]

PR Review: refactor(es/parser): Compare token kind rather than strings

Summary

This PR refactors the parser to use Token enum comparisons instead of string comparisons for keywords, modifiers, and identifiers. This is a well-motivated change that improves both performance and type safety.

✅ Strengths

1. Performance improvement: Token enum comparisons are O(1) integer comparisons (comparing u8 discriminants) versus string comparisons which require character-by-character matching. This is particularly beneficial in hot paths like parse_ts_modifier.

2. Type safety: Using Token variants like Token::Public, Token::Get, etc. instead of string literals eliminates the possibility of typos and enables compile-time checking.

3. Clean refactoring: The changes correctly capture the token before parsing (since parsing advances the token stream), then compare the captured token. This pattern is applied consistently across all modified files.

4. API improvement: The parse_ts_modifier signature change from &[&'static str] to &[Token] is cleaner and more idiomatic Rust.

📝 Minor Observations

1. class_and_fn.rs:977: Uses self.input.cur() directly instead of self.input().cur(). While functionally equivalent, using the method accessor would be slightly more consistent with the rest of the codebase.

2. Getter/setter handling (class_and_fn.rs:1121-1130): The original code also verified Key::Public(PropName::Ident(...)), while the new code only checks the token type. This appears safe since Token::Get/Token::Set tokens will always produce ident keys in this context, and the extensive test suite passing confirms correctness.

🧪 Test Coverage

The existing parser tests should adequately cover this behavior-preserving refactoring. The CI benchmarks show no performance regression (as expected for a comparison-only change).

✅ Verdict

Looks good! The maintainer has already approved this PR and added the changeset. This is a clean, well-executed refactoring that improves the codebase quality.

[claude]

Code Review

Overall Assessment: This is a well-motivated refactoring PR that replaces string comparisons with token kind comparisons for checking keywords, modifiers, and identifiers. This is generally a good practice as token comparisons are more type-safe and potentially faster than string comparisons.

Strengths

1. Performance Improvement: Token comparisons (Token::Get, Token::Set, etc.) are more efficient than string comparisons (i.sym == "get"), as they compare discriminants rather than strings.

2. Type Safety: Using strongly-typed Token enum variants catches more errors at compile time compared to string literals.

3. Consistency: The parse_ts_modifier function now returns Option<Token> instead of Option<&'static str>, making the API more consistent with the rest of the parser.

4. Good Code Pattern in class_and_fn.rs: The change to capture key_token before parsing the key and then using it for getter/setter detection is a clean pattern:

   let key_token = self.input.cur();
   let key = ...parse...;
   // Later use key_token for checking

Potential Issues / Questions

1. Getter/Setter Detection Change (class_and_fn.rs:1124-1130): The original code checked Key::Public(PropName::Ident(ref i)) to ensure the key was an identifier before checking for "get"/"set". The new code only checks key_token:

   // Original
   match key {
       Key::Public(PropName::Ident(ref i)) if (i.sym == "get" || i.sym == "set") ...
   
   // New
   match key_token {
       Token::Get | Token::Set => ...

   This could theoretically behave differently if the key token was Get/Set but parsed into something other than an ident (e.g., in edge cases). However, since Token::Get and Token::Set would always parse to identifiers named "get"/"set", this should be functionally equivalent.

2. Error Message Construction (typescript.rs:702): For error messages with unexpected modifiers, the code now uses other.to_string().into():

   SyntaxError::TS1273(other.to_string().into())

   This is fine, but note that Token::to_string() returns strings like "get", "set", "readonly" which match the expected format. Verified this in token.rs:693-839.

3. _ Catch-all Pattern (class_and_fn.rs:152): The match arm:

   _ => None,

   Should never be reached since allowed_modifiers is [Public, Protected, Private, In, Out], but it's good defensive programming.

Tests

The PR compiles and passes parser tests. The one test failure I encountered (type_only_import_specifier/invalid_type_only) is a pre-existing bug in main that occurs when running without the verify feature - it's not introduced by this PR. CI runs tests with --features verify, so it should pass in CI.

Suggestions (Non-blocking)

1. Consider adding a comment explaining why key_token is captured before parsing in class_and_fn.rs - this pattern is now used in multiple places and the rationale (needing to know what the token was before it gets consumed) might not be immediately obvious.

2. The changeset correctly marks this as a patch release - this is appropriate since it's purely internal refactoring with no API changes.

Verdict

Approved ✅ - This is a clean refactoring that improves type safety and performance without changing behavior. The code follows established patterns in the parser and is well-structured.