-
-
Notifications
You must be signed in to change notification settings - Fork 723
refactor(linter): introduce ContextSubHost for cross script block analyse
#12724
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
refactor(linter): introduce ContextSubHost for cross script block analyse
#12724
Conversation
How to use the Graphite Merge QueueAdd either label to this PR to merge it via 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 Instrumentation Performance ReportMerging #12724 will not alter performanceComparing Summary
Footnotes |
bf94758 to
23e3b37
Compare
23e3b37 to
646cd76
Compare
5f79f25 to
81fac36
Compare
81fac36 to
d04f456
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 refactors the linter architecture to support multi-script block analysis by introducing ContextSubHost. This enables proper linting of Vue files and other frameworks that contain multiple script blocks within a single file, such as <script> and <script setup> tags in Vue components.
- Introduces
ContextSubHostto handle individual script blocks within a file - Modifies
ContextHostto contain multipleContextSubHostinstances and iterate through them - Adds framework-specific options to support Vue setup script detection
Reviewed Changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| crates/oxc_linter/src/utils/jest.rs | Updates test helper to use new ContextSubHost structure |
| crates/oxc_linter/src/loader/source.rs | Adds framework options support to JavaScriptSource |
| crates/oxc_linter/src/loader/partial_loader/vue.rs | Detects Vue setup scripts and applies framework options |
| crates/oxc_linter/src/lib.rs | Wraps linting logic in a loop to process multiple script blocks |
| crates/oxc_linter/src/frameworks.rs | Defines FrameworkOptions enum for framework-specific contexts |
| crates/oxc_linter/src/context/mod.rs | Updates exports and context access methods |
| crates/oxc_linter/src/context/host.rs | Major refactor introducing ContextSubHost and updating ContextHost |
Comments suppressed due to low confidence (1)
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
d04f456 to
3fdf343
Compare
3fdf343 to
6a32eeb
Compare
Merge activity
|
…nalyse (#12724) Another approach to #12541 stack. Because `vue/valid-define-emit` and other rules required the semantics of the other script block. ## The Goal: `Linter.run` should run with the complete file context. In `vue` files and others, there can be multiple Semantics / Module Records. Because `vue` requires some magic compiler rules, the linter should have access to the complete file. This will also fix the offset bug in #12758. ~~**Don't** merge both stacks without rebasing.~~ I do not like the loop part. Maybe you guys have some better ideas. --- <img width="1160" height="422" alt="benchmark results" src="https://github.com/user-attachments/assets/88702a01-eeb2-4a4f-8d80-c5236d3fa8cc" />
6a32eeb to
077abdf
Compare
Follow-on after #12724. Small perf optimization. `Cell` is cheaper than `RefCell`, because it doesn't do runtime aliasing checks. It's preferable when the contained type is small and `Copy`, as is the case here (`usize`).
…eturns `Message` (#14428) Because `oxc_linter::fixer::Message` has now `section_offset` and thanks to #12647 in combination with #12724, the server has everything to calculate what it needs, all relevant code can be moved to the language_server crate. Next steps are to simplify the transformation to `DiagnosticReport` noticed by https://github.com/oxc-project/backlog/issues/175

Another approach to #12541 stack.
Because
vue/valid-define-emitand other rules required the semantics of the other script block.The Goal:
Linter.runshould run with the complete file context. Invuefiles and others, there can be multiple Semantics / Module Records.Because
vuerequires some magic compiler rules, the linter should have access to the complete file.This will also fix the offset bug in #12758.
Don't merge both stacks without rebasing.I do not like the loop part. Maybe you guys have some better ideas.