-
-
Notifications
You must be signed in to change notification settings - Fork 1.3k
refactor(ast): Make ast enum non_exhaustive
#11115
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.
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
base: main
Are you sure you want to change the base?
Conversation
|
Do you think this should go into the main branch? |
Sure, I can change it. |
7e9b1f3
to
5c7ab17
Compare
CodSpeed Performance ReportMerging #11115 will not alter performanceComparing Summary
|
5c7ab17
to
58133f1
Compare
58133f1
to
0c129c5
Compare
Review the following changes in direct dependencies. Learn more about Socket for GitHub.
|
Warning Review the following alerts detected in dependencies. According to your organization's Security Policy, it is recommended to resolve "Warn" alerts. Learn more about Socket for GitHub.
|
1841181
to
2fb6aad
Compare
69ffe4b
to
9ee1a15
Compare
There was a problem hiding this comment.
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 implements the first step of adding unknown variants to AST enums by making all AST enums non_exhaustive
. The changes enable the introduction of unknown variants in a future PR while ensuring that match statements are properly handled.
- Adds
#[cfg(swc_ast_unknown)]
conditional branches to handle future unknown variants - Updates the visitor code generator to conditionally generate fallback arms for
swc_ecma_ast
- Makes AST enums non-exhaustive except for specific ones (HTML, XML, regexp) that opt out
Reviewed Changes
Copilot reviewed 185 out of 186 changed files in this pull request and generated no comments.
Show a summary per file
File | Description |
---|---|
tools/generate-code/src/main.rs | Updates visitor generator to accept string parameter and detect swc_ecma_ast |
tools/generate-code/src/generators/visitor.rs | Conditionally generates fallback arms for unknown variants in match statements |
crates//src/.rs | Adds #[cfg(swc_ast_unknown)] fallback branches to existing match statements |
crates/*/Cargo.toml | Adds lint configuration to recognize the swc_ast_unknown cfg |
crates/swc_*_ast/src/base.rs | Adds no_unknown attribute to specific AST enums that shouldn't be non-exhaustive |
Comments suppressed due to low confidence (2)
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
I've switched to using a cfg flag instead of a feature to handle unknown variants, which makes it easier to ensure consistent handle. Also, since all changes are behind the |
run: | | ||
./scripts/github/run-cargo-hack.sh ${{ matrix.settings.crate }} | ||
- name: Check unknown variant with transforms |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I only added transforms
ci since that covers most of sublibrary and is the primary use case for plugin.
// error. | ||
} | ||
#[cfg(swc_ast_unknown)] | ||
_ => (), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've ignored unknown in some places. there's no particular reason for this, maybe panick would be better.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is a breaking chnage regardless
#[cfg_attr(feature = "rkyv-impl", derive(bytecheck::CheckBytes))] | ||
#[cfg_attr(feature = "rkyv-impl", repr(u32))] | ||
#[cfg_attr(feature = "serde-impl", derive(serde::Serialize, serde::Deserialize))] | ||
#[cfg_attr(swc_ast_unknown, non_exhaustive)] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Addition of this means ast_node
crate has a breaking change
Description:
This is the split of #11100 . Since we need to add unknown variants to the ast enum, this requires a lot of code changes to handle non-exhaustive match. I'm doing this in a separate PR for review.
BREAKING CHANGE:
Related issue (if exists):