Skip to content

Comments

refactor(linter): LintService and Runtime accept file_system and paths as parameters#15515

Merged
graphite-app[bot] merged 1 commit intomainfrom
sysix/refactor-linter-fs-and-path-as-params
Nov 10, 2025
Merged

refactor(linter): LintService and Runtime accept file_system and paths as parameters#15515
graphite-app[bot] merged 1 commit intomainfrom
sysix/refactor-linter-fs-and-path-as-params

Conversation

@Sysix
Copy link
Member

@Sysix Sysix commented Nov 9, 2025

from #15506

Refactor LintService and Runtime to remove mutable builder-style methods

Changes Made

Core Refactoring

  • Removed paths and file_system fields from Runtime struct - they are no longer stored as mutable state
  • Removed with_file_system() and with_paths() builder methods from both Runtime and LintService
  • Updated method signatures to accept these as parameters:
    • run(&self, file_system: &(dyn RuntimeFileSystem + Sync + Send), paths: Vec<Arc<OsStr>>, tx_error: &DiagnosticSender)
    • run_source(&self, file_system: &(dyn RuntimeFileSystem + Sync + Send), paths: Vec<Arc<OsStr>>)
    • run_test_source(&self, file_system: &(dyn RuntimeFileSystem + Sync + Send), paths: Vec<Arc<OsStr>>, check_syntax_errors: bool, tx_error: &DiagnosticSender)

Internal Changes

  • Updated resolve_modules, process_path, process_path_to_module, and get_source_type_and_text to accept file_system and paths as parameters
  • Made OsFileSystem public and exported it from the crate for use by calling code
  • Converted paths to IndexSet at the entry points for efficient lookup during processing
  • Changed methods from &mut self to &self where appropriate (papaya HashMap is concurrent and doesn't require mutable access)
  • Made get_source_type_and_text an associated function since it doesn't use self
  • Fixed outdated comments that referenced removed self.paths field

Call Site Updates

  • lint_runner.rs: Updated lint_files() and run_source() to take references and pass parameters directly
  • apps/oxlint: Updated call to lint_files() to pass reference
  • tester.rs: Updated test infrastructure to use new parameter-based API and removed unnecessary mut
  • oxc_language_server: Updated IsolatedLintHandler methods to use &self and pass references

Testing

  • ✅ All 820 oxc_linter tests pass
  • ✅ All 136 oxlint tests pass
  • ✅ All 92 oxc_language_server tests pass
  • ✅ All clippy warnings resolved (including just lint)
  • ✅ Code formatted with cargo fmt
  • ✅ Clean rebase onto latest main (2f0518d)
Original prompt

Refactor the oxc_linter crate so that LintService and Runtime no longer mutate internal state via with_file_system and with_paths builder-style methods. Instead, pass the file system and paths as parameters to the run, run_source and run_test_source APIs.

Scope and requirements:

  • Identify the places in crates/oxc_linter/src where LintService and Runtime expose with_file_system and with_paths methods that mutate internal state. Remove these mutation methods.
  • Add parameters to the runtime execution methods to accept the file system and paths at call time. Specifically update signatures for run, run_source, and run_test_source to accept the file system and paths. The exact types should follow existing crate types (e.g., &FileSystem or FileSystem, and &[PathBuf] or Vec<PathBuf]) — keep API ergonomic and idiomatic for the codebase.
  • Update any internal implementations to use the passed-in file system and paths instead of using mutated state.
  • Update all call sites across the repository that previously relied on with_file_system/with_paths to instead pass those values into run/run_source/run_test_source.
    • This includes code in crates/oxlint and crates/oxc_language_server and tests under crates/oxc_linter.
  • Run the test suites for crates:
    • crates/oxc_linter
    • crates/oxlint
    • crates/oxc_language_server
      Ensure all tests pass and behavior is unchanged.
  • If any tests or call sites need minor adjustments due to signature changes, update them accordingly.
  • Keep changes localized and avoid unrelated refactors. Maintain public API compatibility where feasible for external consumers by providing deprecation shims if necessary.
  • Add or update unit tests if the refactor requires changes to ensure behavior remains the same.
  • Run cargo fmt/cargo clippy where appropriate and ensure build is clean.

Deliverables:

  • A Git branch with the changes and a single PR opened against main in oxc-project/oxc.
  • A concise PR description summarizing what changed and why, and listing which tests were run and their results.

Notes for implementer:

  • Use repository root as workspace. The task only touches oxc_linter public API and call sites.
  • Do not change unrelated crates or versions.
  • If there are ambiguous type choices, prefer explicit & references (borrowed) rather than owning where feasible to avoid cloning large structures.
  • Ensure the language server integration (oxc_language_server) still works with the new call signatures.

Run tests and report failures if any, fix them so the tests pass.

This pull request was created as a result of the following prompt from Copilot chat.

Refactor the oxc_linter crate so that LintService and Runtime no longer mutate internal state via with_file_system and with_paths builder-style methods. Instead, pass the file system and paths as parameters to the run, run_source and run_test_source APIs.

Scope and requirements:

  • Identify the places in crates/oxc_linter/src where LintService and Runtime expose with_file_system and with_paths methods that mutate internal state. Remove these mutation methods.
  • Add parameters to the runtime execution methods to accept the file system and paths at call time. Specifically update signatures for run, run_source, and run_test_source to accept the file system and paths. The exact types should follow existing crate types (e.g., &FileSystem or FileSystem, and &[PathBuf] or Vec<PathBuf]) — keep API ergonomic and idiomatic for the codebase.
  • Update any internal implementations to use the passed-in file system and paths instead of using mutated state.
  • Update all call sites across the repository that previously relied on with_file_system/with_paths to instead pass those values into run/run_source/run_test_source.
    • This includes code in crates/oxlint and crates/oxc_language_server and tests under crates/oxc_linter.
  • Run the test suites for crates:
    • crates/oxc_linter
    • crates/oxlint
    • crates/oxc_language_server
      Ensure all tests pass and behavior is unchanged.
  • If any tests or call sites need minor adjustments due to signature changes, update them accordingly.
  • Keep changes localized and avoid unrelated refactors. Maintain public API compatibility where feasible for external consumers by providing deprecation shims if necessary.
  • Add or update unit tests if the refactor requires changes to ensure behavior remains the same.
  • Run cargo fmt/cargo clippy where appropriate and ensure build is clean.

Deliverables:

  • A Git branch with the changes and a single PR opened against main in oxc-project/oxc.
  • A concise PR description summarizing what changed and why, and listing which tests were run and their results.

Notes for implementer:

  • Use repository root as workspace. The task only touches oxc_linter public API and call sites.
  • Do not change unrelated crates or versions.
  • If there are ambiguous type choices, prefer explicit & references (borrowed) rather than owning where feasible to avoid cloning large structures.
  • Ensure the language server integration (oxc_language_server) still works with the new call signatures.

Run tests and report failures if any, fix them so the tests pass.


💬 We'd love your input! Share your thoughts on Copilot coding agent in our 2 minute survey.

@graphite-app
Copy link
Contributor

graphite-app bot commented Nov 9, 2025

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 hot fixes, skip the queue and merge this PR next

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.

@github-actions github-actions bot added A-linter Area - Linter A-cli Area - CLI A-editor Area - Editor and Language Server C-cleanup Category - technical debt or refactoring. Solution not expected to change behavior labels Nov 9, 2025
@Sysix Sysix marked this pull request as ready for review November 9, 2025 13:58
@Sysix Sysix requested a review from camc314 as a code owner November 9, 2025 13:58
Copilot AI review requested due to automatic review settings November 9, 2025 13:58
Copy link
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

This PR refactors the linting service to pass file_system and paths as method parameters instead of storing them as struct fields. This change improves the API design by reducing mutable state and making the dependencies explicit at call sites.

Key Changes

  • Removed file_system and paths fields from the Runtime struct
  • Updated all public methods (run, run_source, run_test_source) to accept file_system and paths as parameters
  • Changed method signatures from &mut self to &self where applicable, improving concurrency safety
  • Made OsFileSystem public to allow external instantiation

Reviewed Changes

Copilot reviewed 8 out of 8 changed files in this pull request and generated 1 comment.

Show a summary per file
File Description
crates/oxc_linter/src/service/runtime.rs Removed file_system and paths fields; updated all methods to accept them as parameters; made helper methods static where appropriate
crates/oxc_linter/src/service/mod.rs Updated LintService public API to pass file_system and paths to Runtime methods; removed builder-style methods
crates/oxc_linter/src/tester.rs Updated to pass file_system and paths directly to run_test_source
crates/oxc_linter/src/lint_runner.rs Updated to instantiate OsFileSystem locally and pass it along with paths to service methods
crates/oxc_linter/src/lib.rs Exported OsFileSystem for public use
crates/oxc_language_server/src/linter/isolated_lint_handler.rs Updated to pass file system reference directly without boxing
crates/oxc_language_server/src/linter/server_linter.rs Removed mut qualifier from lock as mutation is no longer needed
apps/oxlint/src/lint.rs Updated to convert boxed file system to reference when calling lint_files

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@codspeed-hq
Copy link

codspeed-hq bot commented Nov 9, 2025

CodSpeed Performance Report

Merging #15515 will not alter performance

Comparing sysix/refactor-linter-fs-and-path-as-params (c903409) with main (2f0518d)1

Summary

✅ 4 untouched
⏩ 33 skipped2

Footnotes

  1. No successful run was found on main (bc2e35d) during the generation of this report, so 2f0518d was used instead as the comparison base. There might be some changes unrelated to this pull request in this report.

  2. 33 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.

@Sysix Sysix force-pushed the sysix/refactor-linter-fs-and-path-as-params branch from 0822aeb to c903409 Compare November 9, 2025 14:05
Copy link
Member Author

Sysix commented Nov 9, 2025


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 hot fixes, skip the queue and merge this PR next

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.

@camc314 camc314 added the 0-merge Merge with Graphite Merge Queue label Nov 10, 2025
Copy link
Contributor

camc314 commented Nov 10, 2025

Merge activity

…aths` as parameters (#15515)

from #15506

Refactor LintService and Runtime to remove mutable builder-style methods

## Changes Made

### Core Refactoring
- Removed `paths` and `file_system` fields from `Runtime` struct - they are no longer stored as mutable state
- Removed `with_file_system()` and `with_paths()` builder methods from both `Runtime` and `LintService`
- Updated method signatures to accept these as parameters:
  - `run(&self, file_system: &(dyn RuntimeFileSystem + Sync + Send), paths: Vec<Arc<OsStr>>, tx_error: &DiagnosticSender)`
  - `run_source(&self, file_system: &(dyn RuntimeFileSystem + Sync + Send), paths: Vec<Arc<OsStr>>)`
  - `run_test_source(&self, file_system: &(dyn RuntimeFileSystem + Sync + Send), paths: Vec<Arc<OsStr>>, check_syntax_errors: bool, tx_error: &DiagnosticSender)`

### Internal Changes
- Updated `resolve_modules`, `process_path`, `process_path_to_module`, and `get_source_type_and_text` to accept file_system and paths as parameters
- Made `OsFileSystem` public and exported it from the crate for use by calling code
- Converted paths to `IndexSet` at the entry points for efficient lookup during processing
- Changed methods from `&mut self` to `&self` where appropriate (papaya HashMap is concurrent and doesn't require mutable access)
- Made `get_source_type_and_text` an associated function since it doesn't use `self`
- Fixed outdated comments that referenced removed `self.paths` field

### Call Site Updates
- **lint_runner.rs**: Updated `lint_files()` and `run_source()` to take references and pass parameters directly
- **apps/oxlint**: Updated call to `lint_files()` to pass reference
- **tester.rs**: Updated test infrastructure to use new parameter-based API and removed unnecessary `mut`
- **oxc_language_server**: Updated `IsolatedLintHandler` methods to use `&self` and pass references

## Testing
- ✅ All 820 oxc_linter tests pass
- ✅ All 136 oxlint tests pass
- ✅ All 92 oxc_language_server tests pass
- ✅ All clippy warnings resolved (including `just lint`)
- ✅ Code formatted with cargo fmt
- ✅ Clean rebase onto latest main (2f0518d)

<!-- START COPILOT CODING AGENT SUFFIX -->

<details>

<summary>Original prompt</summary>

> Refactor the oxc_linter crate so that LintService and Runtime no longer mutate internal state via with_file_system and with_paths builder-style methods. Instead, pass the file system and paths as parameters to the run, run_source and run_test_source APIs.
>
> Scope and requirements:
> - Identify the places in crates/oxc_linter/src where LintService and Runtime expose `with_file_system` and `with_paths` methods that mutate internal state. Remove these mutation methods.
> - Add parameters to the runtime execution methods to accept the file system and paths at call time. Specifically update signatures for `run`, `run_source`, and `run_test_source` to accept the file system and paths. The exact types should follow existing crate types (e.g., &FileSystem or FileSystem, and &[PathBuf] or Vec<PathBuf]) — keep API ergonomic and idiomatic for the codebase.
> - Update any internal implementations to use the passed-in file system and paths instead of using mutated state.
> - Update all call sites across the repository that previously relied on `with_file_system`/`with_paths` to instead pass those values into `run`/`run_source`/`run_test_source`.
>   - This includes code in crates/oxlint and crates/oxc_language_server and tests under crates/oxc_linter.
> - Run the test suites for crates:
>   - crates/oxc_linter
>   - crates/oxlint
>   - crates/oxc_language_server
>   Ensure all tests pass and behavior is unchanged.
> - If any tests or call sites need minor adjustments due to signature changes, update them accordingly.
> - Keep changes localized and avoid unrelated refactors. Maintain public API compatibility where feasible for external consumers by providing deprecation shims if necessary.
> - Add or update unit tests if the refactor requires changes to ensure behavior remains the same.
> - Run `cargo fmt`/`cargo clippy` where appropriate and ensure build is clean.
>
> Deliverables:
> - A Git branch with the changes and a single PR opened against main in oxc-project/oxc.
> - A concise PR description summarizing what changed and why, and listing which tests were run and their results.
>
> Notes for implementer:
> - Use repository root as workspace. The task only touches oxc_linter public API and call sites.
> - Do not change unrelated crates or versions.
> - If there are ambiguous type choices, prefer explicit & references (borrowed) rather than owning where feasible to avoid cloning large structures.
> - Ensure the language server integration (oxc_language_server) still works with the new call signatures.
>
> Run tests and report failures if any, fix them so the tests pass.
>

</details>

*This pull request was created as a result of the following prompt from Copilot chat.*
> Refactor the oxc_linter crate so that LintService and Runtime no longer mutate internal state via with_file_system and with_paths builder-style methods. Instead, pass the file system and paths as parameters to the run, run_source and run_test_source APIs.
>
> Scope and requirements:
> - Identify the places in crates/oxc_linter/src where LintService and Runtime expose `with_file_system` and `with_paths` methods that mutate internal state. Remove these mutation methods.
> - Add parameters to the runtime execution methods to accept the file system and paths at call time. Specifically update signatures for `run`, `run_source`, and `run_test_source` to accept the file system and paths. The exact types should follow existing crate types (e.g., &FileSystem or FileSystem, and &[PathBuf] or Vec<PathBuf]) — keep API ergonomic and idiomatic for the codebase.
> - Update any internal implementations to use the passed-in file system and paths instead of using mutated state.
> - Update all call sites across the repository that previously relied on `with_file_system`/`with_paths` to instead pass those values into `run`/`run_source`/`run_test_source`.
>   - This includes code in crates/oxlint and crates/oxc_language_server and tests under crates/oxc_linter.
> - Run the test suites for crates:
>   - crates/oxc_linter
>   - crates/oxlint
>   - crates/oxc_language_server
>   Ensure all tests pass and behavior is unchanged.
> - If any tests or call sites need minor adjustments due to signature changes, update them accordingly.
> - Keep changes localized and avoid unrelated refactors. Maintain public API compatibility where feasible for external consumers by providing deprecation shims if necessary.
> - Add or update unit tests if the refactor requires changes to ensure behavior remains the same.
> - Run `cargo fmt`/`cargo clippy` where appropriate and ensure build is clean.
>
> Deliverables:
> - A Git branch with the changes and a single PR opened against main in oxc-project/oxc.
> - A concise PR description summarizing what changed and why, and listing which tests were run and their results.
>
> Notes for implementer:
> - Use repository root as workspace. The task only touches oxc_linter public API and call sites.
> - Do not change unrelated crates or versions.
> - If there are ambiguous type choices, prefer explicit & references (borrowed) rather than owning where feasible to avoid cloning large structures.
> - Ensure the language server integration (oxc_language_server) still works with the new call signatures.
>
> Run tests and report failures if any, fix them so the tests pass.
>

<!-- START COPILOT CODING AGENT TIPS -->
---

💬 We'd love your input! Share your thoughts on Copilot coding agent in our [2 minute survey](https://gh.io/copilot-coding-agent-survey).
@graphite-app graphite-app bot force-pushed the sysix/refactor-linter-fs-and-path-as-params branch from c903409 to 9675b51 Compare November 10, 2025 15:19
@graphite-app graphite-app bot merged commit 9675b51 into main Nov 10, 2025
21 checks passed
@graphite-app graphite-app bot deleted the sysix/refactor-linter-fs-and-path-as-params branch November 10, 2025 15:25
@graphite-app graphite-app bot removed the 0-merge Merge with Graphite Merge Queue label Nov 10, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

A-cli Area - CLI A-editor Area - Editor and Language Server A-linter Area - Linter 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