Skip to content

refactor(ast_tools): remove special-case logic for NodeId from Dummy generator#21676

Merged
graphite-app[bot] merged 1 commit into
mainfrom
om/02-14-refactor_ast_tools_remove_special-case_logic_for_nodeid_from_dummy_generator
Apr 23, 2026
Merged

refactor(ast_tools): remove special-case logic for NodeId from Dummy generator#21676
graphite-app[bot] merged 1 commit into
mainfrom
om/02-14-refactor_ast_tools_remove_special-case_logic_for_nodeid_from_dummy_generator

Conversation

@overlookmotel

@overlookmotel overlookmotel commented Apr 23, 2026 •
edited
Loading

Copy link
Copy Markdown
Member

Simplify Dummy generator in ast_tools.

Previously it had "special case" code for NodeId. Instead, implement Dummy on NodeId, and then node_id: Cell<NodeId> fields can be treated the same as any other field.

This was referenced Apr 23, 2026
Merged
Merged
@overlookmotel Graphite App

overlookmotel commented Apr 23, 2026 •
edited
Loading

Copy link
Copy Markdown
Member Author

How to use the Graphite Merge Queue

Add either label to this PR to merge it via the merge queue:

  • 0-merge - adds this PR to the back of the merge queue
  • hotfix - for urgent changes, fast-track this PR to the front of the merge queue

You must have a Graphite account in order to use the merge queue. Sign up using this link.

An organization admin has enabled the Graphite Merge Queue in this repository.

Please do not merge from GitHub as this will restart CI on PRs being processed by the merge queue.

This stack of pull requests is managed by Graphite. Learn more about stacking.

This was referenced Apr 23, 2026
Merged
Merged
Merged
Merged
Merged
@codspeed-hq

codspeed-hq Bot commented Apr 23, 2026 •
edited
Loading

Copy link
Copy Markdown

Merging this PR will not alter performance

✅ 48 untouched benchmarks
⏩ 3 skipped benchmarks1

Comparing om/02-14-refactor_ast_tools_remove_special-case_logic_for_nodeid_from_dummy_generator (06fa11e) with om/02-14-refactor_ast_tools_remove_special-case_logic_for_nodeid_from_clonein_generator (94772d0)

Open in CodSpeed

Footnotes

  1. 3 benchmarks were skipped, so the baseline results were used instead. If they were deleted from the codebase, click here and archive them to remove them from the performance reports. ↩

@overlookmotel overlookmotel self-assigned this Apr 23, 2026
@overlookmotel
overlookmotel marked this pull request as ready for review April 23, 2026 12:57
Copilot AI review requested due to automatic review settings April 23, 2026 12:57
Copilot AI reviewed Apr 23, 2026
View reviewed changes

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR simplifies the Dummy derive generator by removing a hardcoded special case for node_id fields, and instead makes NodeId participate in the normal Dummy trait flow. This aligns NodeId with other AST helper ID types and keeps the generator logic uniform.

Changes:

  • Remove the node_id field-name special case from the Dummy impl generator for structs.
  • Implement Dummy for oxc_syntax::node::NodeId (returning NodeId::DUMMY).
  • Regenerate crates/oxc_ast’s derive_dummy.rs so node_id fields use Dummy::dummy(allocator) like other fields.

Reviewed changes

Copilot reviewed 2 out of 3 changed files in this pull request and generated no comments.

File Description
tasks/ast_tools/src/derives/dummy.rs Removes node_id-specific codegen path so struct fields are generated uniformly.
crates/oxc_syntax/src/node.rs Adds Dummy impl for NodeId to support uniform dummy construction.
crates/oxc_ast/src/generated/derive_dummy.rs Regenerated output: node_id initialization now goes through Dummy::dummy.

@overlookmotel overlookmotel added C-cleanup Category - technical debt or refactoring. Solution not expected to change behavior A-ast Area - AST A-ast-tools Area - AST tools labels Apr 23, 2026
@overlookmotel
overlookmotel force-pushed the om/02-14-refactor_ast_tools_remove_special-case_logic_for_nodeid_from_clonein_generator branch from cabcc3c to f368805 Compare April 23, 2026 13:56
@overlookmotel
overlookmotel force-pushed the om/02-14-refactor_ast_tools_remove_special-case_logic_for_nodeid_from_dummy_generator branch from fe4ed7c to 00d9078 Compare April 23, 2026 13:56
This was referenced Apr 23, 2026
Merged
Merged
Merged
Merged
@graphite-app

graphite-app Bot commented Apr 23, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

Merge activity

@graphite-app
graphite-app Bot force-pushed the om/02-14-refactor_ast_tools_remove_special-case_logic_for_nodeid_from_clonein_generator branch from f368805 to d8fc207 Compare April 23, 2026 15:36
graphite-app Bot pushed a commit that referenced this pull request Apr 23, 2026
@overlookmotel
refactor(ast_tools): remove special-case logic for NodeId from `Dum…
150d25b 
…my` generator (#21676)

Simplify `Dummy` generator in `ast_tools`.

Previously it had "special case" code for `NodeId`. Instead, implement `Dummy` on `NodeId`, and then `node_id: Cell<NodeId>` fields can be treated the same as any other field.
@graphite-app
graphite-app Bot force-pushed the om/02-14-refactor_ast_tools_remove_special-case_logic_for_nodeid_from_dummy_generator branch from 00d9078 to 150d25b Compare April 23, 2026 15:36
@graphite-app
graphite-app Bot force-pushed the om/02-14-refactor_ast_tools_remove_special-case_logic_for_nodeid_from_clonein_generator branch from d8fc207 to 773bd50 Compare April 23, 2026 15:46
graphite-app Bot pushed a commit that referenced this pull request Apr 23, 2026
@overlookmotel
refactor(ast_tools): remove special-case logic for NodeId from `Dum…
28e97e7 
…my` generator (#21676)

Simplify `Dummy` generator in `ast_tools`.

Previously it had "special case" code for `NodeId`. Instead, implement `Dummy` on `NodeId`, and then `node_id: Cell<NodeId>` fields can be treated the same as any other field.
@graphite-app
graphite-app Bot force-pushed the om/02-14-refactor_ast_tools_remove_special-case_logic_for_nodeid_from_dummy_generator branch from 150d25b to 28e97e7 Compare April 23, 2026 15:46
@graphite-app
graphite-app Bot force-pushed the om/02-14-refactor_ast_tools_remove_special-case_logic_for_nodeid_from_clonein_generator branch from 773bd50 to 4ab852a Compare April 23, 2026 17:20
graphite-app Bot pushed a commit that referenced this pull request Apr 23, 2026
@overlookmotel
refactor(ast_tools): remove special-case logic for NodeId from `Dum…
30d2660 
…my` generator (#21676)

Simplify `Dummy` generator in `ast_tools`.

Previously it had "special case" code for `NodeId`. Instead, implement `Dummy` on `NodeId`, and then `node_id: Cell<NodeId>` fields can be treated the same as any other field.
@graphite-app
graphite-app Bot force-pushed the om/02-14-refactor_ast_tools_remove_special-case_logic_for_nodeid_from_dummy_generator branch from 28e97e7 to 30d2660 Compare April 23, 2026 17:20
@overlookmotel
refactor(ast_tools): remove special-case logic for NodeId from `Dum…
06fa11e 
…my` generator (#21676)

Simplify `Dummy` generator in `ast_tools`.

Previously it had "special case" code for `NodeId`. Instead, implement `Dummy` on `NodeId`, and then `node_id: Cell<NodeId>` fields can be treated the same as any other field.
@graphite-app
graphite-app Bot force-pushed the om/02-14-refactor_ast_tools_remove_special-case_logic_for_nodeid_from_clonein_generator branch from 4ab852a to 94772d0 Compare April 23, 2026 17:28
@graphite-app
graphite-app Bot force-pushed the om/02-14-refactor_ast_tools_remove_special-case_logic_for_nodeid_from_dummy_generator branch from 30d2660 to 06fa11e Compare April 23, 2026 17:29
Base automatically changed from om/02-14-refactor_ast_tools_remove_special-case_logic_for_nodeid_from_clonein_generator to main April 23, 2026 18:40
@graphite-app
graphite-app Bot merged commit 06fa11e into main Apr 23, 2026
37 checks passed
@graphite-app
graphite-app Bot deleted the om/02-14-refactor_ast_tools_remove_special-case_logic_for_nodeid_from_dummy_generator branch April 23, 2026 18:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

Copilot code review Copilot Copilot left review comments

Assignees

@overlookmotel overlookmotel

Labels

A-ast Area - AST A-ast-tools Area - AST tools C-cleanup Category - technical debt or refactoring. Solution not expected to change behavior

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@overlookmotel