Skip to content

refactor(ast_tools): search all crates which depend on oxc_ast_macros#20577

Merged
graphite-app[bot] merged 1 commit intomainfrom
om/02-14-refactor_ast_tools_search_all_crates_which_depend_on_oxc_ast_macros_
Mar 20, 2026
Merged

refactor(ast_tools): search all crates which depend on oxc_ast_macros#20577
graphite-app[bot] merged 1 commit intomainfrom
om/02-14-refactor_ast_tools_search_all_crates_which_depend_on_oxc_ast_macros_

Conversation

@overlookmotel
Copy link
Copy Markdown
Member

@overlookmotel overlookmotel commented Mar 20, 2026

One annoyance with ast_tools, our codegen, has been that when you want it to generate code from types in some file, you had to manually add the file paths to a list in ast_tools itself.

This PR removes that list and instead finds crates in the monorepo which depend on oxc_ast_macros crate, via cargo metadata, and searches all files in those crates for types with #[ast] attributes etc - types that it need to act on.

There is no longer a list to keep updated, you just mark types #[ast] and the codegen will find them.

This produces some churn in generated files. None of the generated code changes, it just gets re-ordered in some places, due to the different order that files get found in now. This order is deterministic, and it should not change again.

Copy link
Copy Markdown
Member Author

overlookmotel commented Mar 20, 2026


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.

@codspeed-hq
Copy link
Copy Markdown

codspeed-hq bot commented Mar 20, 2026

Merging this PR will not alter performance

✅ 53 untouched benchmarks
⏩ 3 skipped benchmarks1


Comparing om/02-14-refactor_ast_tools_search_all_crates_which_depend_on_oxc_ast_macros_ (98a1757) with main (d176ecc)

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 marked this pull request as ready for review March 20, 2026 21:07
@overlookmotel overlookmotel requested a review from camc314 as a code owner March 20, 2026 21:07
Copilot AI review requested due to automatic review settings March 20, 2026 21:07
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Refactors oxc_ast_tools to automatically discover which Rust crates/files contain AST definitions by querying workspace metadata, removing the need to maintain a hard-coded source file list in the codegen tool.

Changes:

  • Add find_crates_and_files() (via cargo metadata) to locate workspace crates depending on oxc_ast_macros and enumerate their src/**/*.rs inputs (excluding src/generated).
  • Thread the discovered root_path/source_paths into Codegen::new(...) and parse_files(...), removing the previous SOURCE_PATHS list.
  • Rework the CI watch-list YAML generation to watch relevant crate globs plus generated outputs, producing deterministic but reordered generated artifacts.

Reviewed changes

Copilot reviewed 5 out of 21 changed files in this pull request and generated no comments.

Show a summary per file
File Description
tasks/ast_tools/src/parse/mod.rs Updates parsing entrypoint to accept discovered Vec<String> file paths and simplifies collection.
tasks/ast_tools/src/output/yaml.rs Reworks watch-list generation to use discovered crate paths + outputs and emit RawOutput.
tasks/ast_tools/src/main.rs Removes hard-coded source list; integrates crate/file discovery and new watch-list generation flow.
tasks/ast_tools/src/find.rs New metadata-driven crate/file discovery (workspace crates depending on oxc_ast_macros).
tasks/ast_tools/src/codegen.rs Codegen::new now takes an explicit workspace root path.
npm/oxc-types/types.d.ts Generated ordering change (no semantic change).
napi/parser/src-js/generated/lazy/constructors.js Generated ordering change (no semantic change).
napi/parser/src-js/generated/deserialize/ts_range_parent.js Generated ordering change (no semantic change).
napi/parser/src-js/generated/deserialize/ts_range.js Generated ordering change (no semantic change).
napi/parser/src-js/generated/deserialize/ts_parent.js Generated ordering change (no semantic change).
napi/parser/src-js/generated/deserialize/ts.js Generated ordering change (no semantic change).
napi/parser/src-js/generated/deserialize/js_range_parent.js Generated ordering change (no semantic change).
napi/parser/src-js/generated/deserialize/js_range.js Generated ordering change (no semantic change).
napi/parser/src-js/generated/deserialize/js_parent.js Generated ordering change (no semantic change).
napi/parser/src-js/generated/deserialize/js.js Generated ordering change (no semantic change).
crates/oxc_syntax/src/generated/assert_layouts.rs Generated ordering change (no semantic change).
crates/oxc_span/src/generated/derive_estree.rs Generated ordering change (no semantic change).
crates/oxc_span/src/generated/assert_layouts.rs Generated ordering change (no semantic change).
apps/oxlint/src-js/generated/types.d.ts Generated ordering change (no semantic change).
apps/oxlint/src-js/generated/deserialize.js Generated ordering change (no semantic change).
.github/generated/ast_changes_watch_list.yml Updates watch patterns to crate src/**/*.rs globs + relevant generated outputs; header source updated.

@overlookmotel overlookmotel self-assigned this Mar 20, 2026
@graphite-app graphite-app bot added the 0-merge Merge with Graphite Merge Queue label Mar 20, 2026
@graphite-app
Copy link
Copy Markdown
Contributor

graphite-app bot commented Mar 20, 2026

Merge activity

…s` (#20577)

One annoyance with `ast_tools`, our codegen, has been that when you want it to generate code from types in some file, you had to manually add the file paths to a list in `ast_tools` itself.

This PR removes that list and instead finds crates in the monorepo which depend on `oxc_ast_macros` crate, via `cargo metadata`, and searches all files in those crates for types with `#[ast]` attributes etc - types that it need to act on.

There is no longer a list to keep updated, you just mark types `#[ast]` and the codegen will find them.

This produces some churn in generated files. None of the generated code changes, it just gets re-ordered in some places, due to the different order that files get found in now. This order is deterministic, and it should not change again.
@graphite-app graphite-app bot force-pushed the om/02-14-refactor_ast_tools_search_all_crates_which_depend_on_oxc_ast_macros_ branch from 98a1757 to 1be1ebe Compare March 20, 2026 22:16
graphite-app bot pushed a commit that referenced this pull request Mar 20, 2026
#20577 made `ast_tools` search for and parse all files in all crates that depend on `oxc_ast_macros`.

The one downside of this change is that it makes running `ast_tools` slower.

Speed it up again, by parsing files in parallel, using `rayon`.

There is one annoyance. Due to feature unification, `syn` crate gets compiled with `proc-macro` feature enabled, which makes `syn`s ASTs not `Send`. We don't use the AST in ways which can produce data races (and we can't, because `ast_tools` isn't a proc macro), so it is safe in practice to send ASTs across threads here. So we have to hack it to make it compile (see comment in `tasks/ast_tools/src/parse/mod.rs`).
@graphite-app graphite-app bot merged commit 1be1ebe into main Mar 20, 2026
25 checks passed
@graphite-app graphite-app bot removed the 0-merge Merge with Graphite Merge Queue label Mar 20, 2026
@graphite-app graphite-app bot deleted the om/02-14-refactor_ast_tools_search_all_crates_which_depend_on_oxc_ast_macros_ branch March 20, 2026 22:22
costajohnt pushed a commit to costajohnt/oxc that referenced this pull request Mar 22, 2026
…s` (oxc-project#20577)

One annoyance with `ast_tools`, our codegen, has been that when you want it to generate code from types in some file, you had to manually add the file paths to a list in `ast_tools` itself.

This PR removes that list and instead finds crates in the monorepo which depend on `oxc_ast_macros` crate, via `cargo metadata`, and searches all files in those crates for types with `#[ast]` attributes etc - types that it need to act on.

There is no longer a list to keep updated, you just mark types `#[ast]` and the codegen will find them.

This produces some churn in generated files. None of the generated code changes, it just gets re-ordered in some places, due to the different order that files get found in now. This order is deterministic, and it should not change again.
costajohnt pushed a commit to costajohnt/oxc that referenced this pull request Mar 22, 2026
oxc-project#20577 made `ast_tools` search for and parse all files in all crates that depend on `oxc_ast_macros`.

The one downside of this change is that it makes running `ast_tools` slower.

Speed it up again, by parsing files in parallel, using `rayon`.

There is one annoyance. Due to feature unification, `syn` crate gets compiled with `proc-macro` feature enabled, which makes `syn`s ASTs not `Send`. We don't use the AST in ways which can produce data races (and we can't, because `ast_tools` isn't a proc macro), so it is safe in practice to send ASTs across threads here. So we have to hack it to make it compile (see comment in `tasks/ast_tools/src/parse/mod.rs`).
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

A-ast-tools Area - AST tools A-cli Area - CLI A-linter Area - Linter A-linter-plugins Area - Linter JS plugins A-parser Area - Parser C-cleanup Category - technical debt or refactoring. Solution not expected to change behavior

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants