refactor(oxlint/lsp): use fs as param in TsGoLintState, instead of source text#16667
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 Performance ReportMerging #16667 will not alter performanceComparing Summary
Footnotes
|
17dd958 to
aee5bc6
Compare
72cc184 to
4e370b1
Compare
There was a problem hiding this comment.
Pull request overview
This PR refactors the TsGoLintState::lint_source method and its caller LintRunner::run_source to accept a file system parameter instead of source text. This change aligns the API design by delegating file reading responsibility to the function itself rather than requiring callers to provide the source text.
Key changes:
- Modified
TsGoLintState::lint_sourceto acceptfile_systemparameter instead ofsource_textparameter - Updated
LintRunner::run_sourcesignature to removesource_textparameter - Updated
IsolatedLintHandlerto callrun_sourcewithout passing source text
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| crates/oxc_linter/src/tsgolint.rs | Refactored lint_source to accept file system parameter and read file internally using arena allocator, then convert to owned String |
| crates/oxc_linter/src/lint_runner.rs | Removed source_text parameter from run_source signature and updated call to lint_source |
| crates/oxc_language_server/src/linter/isolated_lint_handler.rs | Updated caller to remove source_text argument from run_source call |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
4e370b1 to
0e13f34
Compare
aee5bc6 to
dd22b62
Compare
There was a problem hiding this comment.
The refactor is directionally good, but TsGoLintState::lint_source now does extra allocations (Allocator + to_string() + cloning) and loses useful error context by not including the underlying read error. Also, the pipeline likely reads the same file twice (regular + type-aware lint), which can become a real performance issue when scaling to multi-file linting.
Summary of changes
Summary of changes
- Refactored the language-server lint flow to pass a
RuntimeFileSysteminto linting instead of passingsource_textthrough the API. - Updated
LintRunner::run_sourceto remove thesource_text: Stringparameter and forward thefile_systemto the type-aware linter. - Updated
TsGoLintState::lint_sourceto read the source fromfile_systemviaread_to_arena_str(usingoxc_allocator::Allocator) and to use an ownedString(source_text_owned) inside the spawned thread.
Merge activity
|
…urce text (#16667) In the future, the language server wants to lint multiple files at the same time. Refactored the code to accept the RuntimeFS (like the CLI tool) for reading the source text.
dd22b62 to
73da317
Compare

In the future, the language server wants to lint multiple files at the same time.
Refactored the code to accept the RuntimeFS (like the CLI tool) for reading the source text.