feat(ast): add GetNodeId trait - #26145
Conversation
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 8428d2690d
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
GetNodeId trait
Merging this PR will not alter performance
Comparing 
Footnotes
|
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 311da02f0b
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
overlookmotel
left a comment
There was a problem hiding this comment.
Not sure what need is for this, but I assume you have your reasons!
5593290 to
b6b8adb
Compare
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
Ah! The context here is that comments are attached to node ids. and to avoid a bunch of duplicated code, i'm planning to print comments like: /// Generate source code for an AST node.
pub trait Gen: GetSpan + GetNodeId {
fn print(&self, p: &mut Codegen, ctx: Context) {
let node_id = self.node_id();
p.print_leading_comments(node_id);
self.r#gen(p, ctx);
p.print_trailing_comments(node_id);
}
}And not doing this via a trait gets pretty messy |
Merge activity
|
## Summary - add `GetNodeId` alongside `NodeId` in `oxc_syntax` - add a dedicated `DeriveGetNodeId` following the existing `GetSpan` derive architecture - generate implementations into `derive_get_node_id.rs`, delegating uniformly through `self.node_id()`
2c3b64c to
1d9b9d3
Compare
### 💥 BREAKING CHANGES - 66744f0 parser: [**BREAKING**] Rename `panicked` to `fatal_error` in `ParserReturn` (#26382) (overlookmotel) - 2c9a947 parser: [**BREAKING**] Reduce `MAX_LEN` to 256 bytes below `u32::MAX` (#26352) (overlookmotel) ### 🚀 Features - 32bdc5b allocator: Construct `ArenaHashSet` with any `Default` hasher (#26372) (Dunqing) - a17de58 minifier: Process of return/throw in non last position and fix async * inlining (#26214) (Armano) - bfb4c57 parser: Distinguish unapplied no-side-effects comments (#26120) (碳苯 Carbon) - 1d9b9d3 ast: Add `GetNodeId` trait (#26145) (camc314) ### 🐛 Bug Fixes - b156333 codegen: Order accessibility before abstract on accessors (#26392) (camc314) - 116c8b8 transformer/class-properties: Keep private methods in class-expression scope (#26308) (Changsu Seong) - 1400a0f parser: Require a string source after `export ... from` (#26389) (camc314) - 2da73b7 codegen: Print matching quoted import names as identifiers (#26386) (camc314) - 0a81d29 codegen: Preserve private-in left operand precedence (#26383) (camc314) - 9a02337 parser: Correctly round large nondecimal literals (#26379) (camc314) - 3d95477 transformer: Gate async-only rewrites by owning transform (#26326) (Dunqing) - f45a9ee transformer: Limit arguments capture to lowered async functions (#26317) (Dunqing) - 789969f transformer: Gate this capture by owning async transform (#26227) (Dunqing) - affeb14 minifier: Mangle class private members in Node API (#26118) (碳苯 Carbon) - 8b35abd semantic: Detect duplicate private class elements (#26361) (camc314) - 457ed57 semantic: Reject await and yield in rest parameter defaults (#26359) (camc314) - 4b89c7b semantic: Reject jumps across arrow functions (#26357) (camc314) - c966aca parser: Do not omit `<` token opening type argument list (#26328) (overlookmotel) - f07dca5 ast_visit: Add the trimmed prefix length to translation table offsets (#26173) (Bharadwaj Pendyala) - 3ab9bd1 parser: Do not re-lex template substitution tail after fatal error (#26230) (overlookmotel) - 07851b9 parser: Fix debug assert failure when lexer error with tokens enabled (#26229) (overlookmotel) - 00dea7a allocator: Gate `Allocator::data_end_ptr` behind fixed_size feature (#26248) (camc314) - 853ffab ecmascript: Avoid `charAt` panic on 32-bit (#26244) (camc314) - 34feccb runtime: Remove stale regenerator exports (#26243) (camc314) ### ⚡ Performance - 34a242e minifier: Use `Ident` for property mangler (#26334) (sapphi-red) - 5303f5c minifier: Unify merging of last expression into target sequence (#26264) (Armano) - 2ca7d29 minifier: Consume nodes in minimize_statements in reverse order (#26258) (Armano) - d2354b4 mangler: Use `Ident` for variable names to keep (#26333) (sapphi-red) - 5b3e335 minifier: Use `Ident` instead of `Str` in `KeepVar` (#26332) (sapphi-red) - 51366fb minifier: Use `IdentHashSet` in `PrivateMemberUsageStack` (#26331) (sapphi-red) - d19c42a minifier: Merge nested if stmt in place instead of creating dummies (#26351) (Armano) - 356def6 parser: Shrink annotation comment ranges (#26356) (overlookmotel) - a5be474 parser: Shave instruction off `parse_jsx_element_name` (#26355) (overlookmotel) - 9780663 parser: Remove fatal error guard from `parse_jsx_element_name` (#26354) (overlookmotel) - 766e12f parser: Remove `token` field from `LexerCheckpoint` (#26350) (overlookmotel) ### 📚 Documentation - d4b4e61 transformer: Document ES2015 target floor (#26378) (Dunqing) - 67dd054 ast_visit: Correct out-of-date comments on `Utf8ToUtf16` converter (#26285) (overlookmotel) - a8ed2a7 minifier: Fix stale validation instructions (#26251) (camc314)
Summary
GetNodeIdalongsideNodeIdinoxc_syntaxDeriveGetNodeIdfollowing the existingGetSpanderive architecturederive_get_node_id.rs, delegating uniformly throughself.node_id()