Skip to content

Conversation

Sysix
Copy link
Member

@Sysix Sysix commented Sep 12, 2025

Formatting requests has no source text included:
https://microsoft.github.io/language-server-protocol/specifications/lsp/3.17/specification/#documentFormattingParams

Because users can format in memory files, the server needs to keep track of the changes.
Created in Backend struct, so when we later support textDocument/diagnostic (we support currently only textDocument/publishDiagnostic), we can use the same File System for it too :)

Putting the struct into a Workspace Worker is an option too, but there can be multiple of it.
With this solution, there is only one instance of it :)

Copy link
Member Author

Sysix commented Sep 12, 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.

@github-actions github-actions bot added A-editor Area - Editor and Language Server C-cleanup Category - technical debt or refactoring. Solution not expected to change behavior labels Sep 12, 2025
Copy link

codspeed-hq bot commented Sep 12, 2025

CodSpeed Instrumentation Performance Report

Merging #13731 will not alter performance

Comparing 09-12-refactor_langauge_server_introdude_lspfilesystem_ (dc700f5) with main (42e2c1d)1

Summary

✅ 37 untouched

Footnotes

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

@Sysix Sysix force-pushed the 09-09-chore_language_server_split_options_for_linting_and_formatting branch from 23313c4 to 389b108 Compare September 14, 2025 00:27
@Sysix Sysix force-pushed the 09-12-refactor_langauge_server_introdude_lspfilesystem_ branch from 3b08bf6 to 179e34f Compare September 14, 2025 00:27
@Sysix Sysix force-pushed the 09-09-chore_language_server_split_options_for_linting_and_formatting branch from 389b108 to f70cdae Compare September 15, 2025 18:54
@Sysix Sysix force-pushed the 09-12-refactor_langauge_server_introdude_lspfilesystem_ branch from 179e34f to f391c08 Compare September 15, 2025 18:54
@Sysix Sysix changed the base branch from 09-09-chore_language_server_split_options_for_linting_and_formatting to graphite-base/13731 September 15, 2025 19:19
@Sysix Sysix force-pushed the 09-12-refactor_langauge_server_introdude_lspfilesystem_ branch from f391c08 to b988882 Compare September 15, 2025 19:25
@Sysix Sysix force-pushed the graphite-base/13731 branch from f70cdae to e2e7901 Compare September 15, 2025 19:25
@Sysix Sysix changed the base branch from graphite-base/13731 to 09-09-chore_language_server_split_options_for_linting_and_formatting September 15, 2025 19:25
@Sysix Sysix changed the title refactor(langauge_server): introdude LSPFileSystem refactor(language_server): introdude LSPFileSystem Sep 15, 2025
@graphite-app graphite-app bot changed the base branch from 09-09-chore_language_server_split_options_for_linting_and_formatting to graphite-base/13731 September 16, 2025 01:17
@graphite-app graphite-app bot force-pushed the 09-12-refactor_langauge_server_introdude_lspfilesystem_ branch from b988882 to 64fafe3 Compare September 16, 2025 01:22
@graphite-app graphite-app bot force-pushed the graphite-base/13731 branch from e2e7901 to 137896a Compare September 16, 2025 01:22
@graphite-app graphite-app bot changed the base branch from graphite-base/13731 to main September 16, 2025 01:22
@graphite-app graphite-app bot force-pushed the 09-12-refactor_langauge_server_introdude_lspfilesystem_ branch from 64fafe3 to bdbd19b Compare September 16, 2025 01:23
@Sysix Sysix force-pushed the 09-12-refactor_langauge_server_introdude_lspfilesystem_ branch 3 times, most recently from 4ddbcd8 to dabd34c Compare September 19, 2025 09:19
@Sysix Sysix marked this pull request as ready for review September 19, 2025 11:10
@Sysix Sysix requested a review from camc314 as a code owner September 19, 2025 11:10
@Copilot Copilot AI review requested due to automatic review settings September 19, 2025 11:10
Copy link
Contributor

@Copilot 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 introduces an LSPFileSystem to track in-memory file changes for the language server, specifically to support formatting requests that don't include source text per the LSP specification.

  • Adds a new LSPFileSystem struct to manage file content in memory
  • Integrates the file system into the Backend struct and LSP lifecycle methods
  • Updates capabilities to include dynamic formatting support (currently disabled)

Reviewed Changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 3 comments.

File Description
crates/oxc_language_server/src/main.rs Integrates LSPFileSystem into Backend and adds file tracking to LSP event handlers
crates/oxc_language_server/src/file_system.rs Implements new LSPFileSystem struct with methods for managing file content
crates/oxc_language_server/src/capabilities.rs Adds dynamic_formatting capability field (currently set to false)

Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.

@Sysix Sysix changed the title refactor(language_server): introdude LSPFileSystem refactor(language_server): introduce LSPFileSystem Sep 19, 2025
@Sysix Sysix force-pushed the 09-12-refactor_langauge_server_introdude_lspfilesystem_ branch from dabd34c to 4649c44 Compare September 19, 2025 11:41
@camc314 camc314 added the 0-merge Merge with Graphite Merge Queue label Sep 19, 2025
@camc314 camc314 self-assigned this Sep 19, 2025
Copy link
Contributor

camc314 commented Sep 19, 2025

Merge activity

Formatting requests has no source text included:
https://microsoft.github.io/language-server-protocol/specifications/lsp/3.17/specification/#documentFormattingParams

Because users can format in memory files, the server needs to keep track of the changes.
Created in `Backend` struct, so when we later support `textDocument/diagnostic` (we support currently only `textDocument/publishDiagnostic)`, we can use the same File System for it too :)

Putting the struct into a Workspace Worker is an option too, but there can be multiple of it.
With this solution, there is only one instance of it :)
@graphite-app graphite-app bot force-pushed the 09-12-refactor_langauge_server_introdude_lspfilesystem_ branch from 4649c44 to dc700f5 Compare September 19, 2025 11:47
@graphite-app graphite-app bot merged commit dc700f5 into main Sep 19, 2025
26 checks passed
@graphite-app graphite-app bot deleted the 09-12-refactor_langauge_server_introdude_lspfilesystem_ branch September 19, 2025 11:51
@graphite-app graphite-app bot removed the 0-merge Merge with Graphite Merge Queue label Sep 19, 2025
camc314 added a commit that referenced this pull request Sep 23, 2025
## [1.17.0] - 2025-09-23

### 🚀 Features

- f47f978 linter/plugins: Add `definePlugin` API (#14008)
(overlookmotel)
- 3e117c6 linter/plugins: Add `defineRule` API (#13945) (overlookmotel)
- 2dc8adb linter/plugins: Add `createOnce` API (#13944) (overlookmotel)
- bef8753 linter/plugins: ESTree-compatible AST for JS plugins (#13942)
(overlookmotel)
- 55c2e11 editor: Add `oxc.fmt.experimental` flag (#13923) (Sysix)
- a21ff54 language_server: Introduce `ServerFormatter` (#13700) (Sysix)
- a14aa79 npm/oxlint: Convert to ES modules (#13876) (Boshen)
- 15eb326 linter: Add `vue/no-required-prop-with-default` rule (#13613)
(yefan)
- 5d3ba00 linter: Add `vue/require-typed-ref` rule (#13857) (yefan)
- b52389a node: Bump `engines` field to require Node.js 20.19.0+ for ESM
support (#13879) (Copilot)
- 8221a01 linter: Add `eslint/no-misleading-character-class` (#13365)
(Sysix)
- c75ae8c linter/plugins: Add options to `Context` (#13810)
(overlookmotel)
- 53d04dd linter: Convert `oxlint` to NAPI app (#13723) (overlookmotel)

### 🐛 Bug Fixes

- 1f4be38 napi/parser: Generate `.d.mts` extension for types (#14038)
(Daniel Roe)
- a018756 linter/plugins: Pin `tsdown` dependency to 0.15.1 (#14005)
(overlookmotel)
- b8790c2 language_server: Output correct position for parser & semantic
errors (#14010) (Sysix)
- 4d3c6e1 linter: Support let-chains in codegen node type detection
(#14006) (camc314)
- a34918a linter/plugins: Avoid lint warnings compiling WASM or
big-endian (#13968) (overlookmotel)
- 27022ab linter/switch-case-braces: Add support for string including
colon on case expression (#13687) (Lukinoh)
- dd3843f linter: Set package version in `package.json` (#13890)
(overlookmotel)
- fa866b3 linter: Sort optimized/unoptimized diagnostics before
comparison (#13850) (camc314)
- fac7624 linter/plugins: Improve error for no JS plugins (#13858)
(overlookmotel)

### 🚜 Refactor

- bb040bc parser, linter: Replace `.mjs` files with `.js` (#14045)
(overlookmotel)
- b353750 linter/plugins: Remove unnecessary `Serialize` derives
(#14031) (overlookmotel)
- 0d48511 linter/plugins: Improve handling `Context` method calls in
`createOnce` (#14032) (overlookmotel)
- 36ac0fb language_server: Don't create `mpsc` channel (#14011) (Sysix)
- 2e9ae3f linter: Make disable directives own the rule name (#13987)
(camc314)
- 6bc7664 oxlint: Run oxlint before tsgolint (#13519) (camc314)
- 0c93f33 language_server: Use minimal text edit for `ServerFormatter`
(#13960) (Sysix)
- 823cb57 langauge_server: Move `Backend` into own file (#13955) (Sysix)
- dc700f5 language_server: Introduce `LSPFileSystem` (#13731) (Sysix)
- 42e2c1d linter: Remove `Rc` from `DisableDirectives` (#13924)
(camc314)
- ac3e9e9 napi/parser: Move JS code into `src-js` directory (#13899)
(overlookmotel)
- e90bfe3 linter: Remove double reference in unicorn/error-message
(#13916) (camc314)
- 83791a0 linter: Remove double reference in vue/jsx-no-undef (#13915)
(camc314)
- ebd22de linter: Remove double reference in vue/require-typed-ref
(#13914) (camc314)
- 873e231 language_server: Move `needs_linter_restart` to `ServerLinter`
(#13834) (Sysix)
- 7e0d736 linter/plugins: Rename `--experimental-js-plugins` to
`--js-plugins` (#13860) (overlookmotel)
- 6245c8c linter/plugins: Make `Context` properties getters (#13809)
(overlookmotel)
- a2342a6 linter/plugins: Import named in tests (#13807) (overlookmotel)

### 📚 Documentation

- 601c876 linter: Add comment explaining why Mimalloc is feature-gated
(#14037) (overlookmotel)

### ⚡ Performance

- 4d04c6e linter/plugins: Flatten `LintFileResult` fields (#14033)
(overlookmotel)
- 06363e0 linter: Add node type codegen support for `match node.kind()`
(#13736) (camchenry)
- a79af0a linter: Replace `for (... of ...)` loops (#13913)
(overlookmotel)
- b53292d linter: Use unstable sort where possible (#13818) (sapphi-red)

### 🎨 Styling

- 8083740 linter: Import `Serialize` at top level (#14030)
(overlookmotel)

### 🧪 Testing

- b807b6f language_server: Skip formatter test on windows (#13986)
(Sysix)
- 2600858 language_server: Add Tester for ServerFormatter (#13969)
(Sysix)
- 37aadf0 language_server: Add `test_and_snapshot_multiple_file`
(#13966) (Sysix)
- ad14a41 editor: Fix test setup (#13929) (Sysix)
- f51240e linter/plugins: Tests for different import styles (#13859)
(overlookmotel)
- 407c95e linter/plugins: Check `this` is undefined in visit functions
(#13811) (overlookmotel)
- f023a22 linter/plugins: Include stack trace in plugin loading errors
(#13808) (overlookmotel)

### 💼 Other

- 0630d68 linter: Build `oxlint` locally with Mimalloc in release mode
(#14034) (overlookmotel)

Co-authored-by: camc314 <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-editor Area - Editor and Language Server 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