Experimental: Unified LSP server in rewatch#8243
Draft
nojaf wants to merge 105 commits intorescript-lang:masterfrom
Draft
Experimental: Unified LSP server in rewatch#8243nojaf wants to merge 105 commits intorescript-lang:masterfrom
nojaf wants to merge 105 commits intorescript-lang:masterfrom
Conversation
Add optional OTLP tracing export to rewatch, controlled by the OTEL_EXPORTER_OTLP_ENDPOINT environment variable. When set, rewatch exports spans via HTTP OTLP; when unset, tracing is a no-op. Instrument key build system functions (initialize_build, incremental_build, compile, parse, clean, format, packages) with tracing spans and attributes such as module counts and package names. Restructure main.rs to support telemetry lifecycle (init/flush/shutdown) and fix show_progress to use >= LevelFilter::Info so -v/-vv don't suppress progress messages. Also print 'Finished compilation' in plain_output mode during watch full rebuilds. Introduce a new Vitest-based test infrastructure in tests/rewatch_tests/ that replaces the bash integration tests. Tests spawn rewatch with an OTLP endpoint pointing to an in-process HTTP receiver, collect spans, and snapshot the resulting span tree for deterministic assertions. Update CI, Makefile, and scripts/test.js to use the new test runner.
When stdin is a pipe (not a TTY), spawn a background thread that monitors for EOF. This allows a parent process (such as the test harness) to signal a graceful shutdown by closing stdin, without relying on signals or lock file removal.
Add mtime and content-hash based deduplication to filter out phantom and duplicate file system events. Normalize event kinds from atomic writes (temp file + rename) so they are treated as content modifications rather than create/remove cycles that trigger unnecessary full rebuilds. This fixes issues on macOS (Create events from atomic writes), Linux (duplicate inotify IN_MODIFY events), and Windows (Remove+Rename sequences from atomic writes).
On Windows, bsc writes CRLF to stdout in text mode. When the original source file uses LF line endings, the formatted output would introduce unwanted CRLF conversions. Detect the original file's line ending style and normalize the formatted output to match.
Propagate parent span through rayon in build.parse so build.parse_file spans are properly nested under build.parse instead of appearing as orphaned root spans. Enrich build.compile_file span with package, suffix, module_system, and namespace attributes for better observability. Handle invalid config changes gracefully during watch mode: replace .expect() with match to report the error and continue watching, allowing the user to fix the config without restarting the watcher.
Add 7 new fixture packages to cover more configuration dimensions: - commonjs: CommonJS module output with .bs.js suffix - namespaced: namespace package with TestNS - noop-ppx: lightweight cross-platform no-op PPX for testing - with-deps: package depending on rescript-bun for clean tests - with-dev-deps: multi-source dirs with dev dependencies - with-jsx: JSX v4 with @rescript/react - with-ppx: PPX integration using noop-ppx Enhance test helpers: - Normalize CRLF line endings in process output for Windows - Support .bs.js artifacts in sandbox cleanup detection - Add createCli, readFileInSandbox, writeFileInSandbox helpers - Add OTEL config for build.parse_file and enriched compile_file spans - Exclude noop-ppx from biome linting (CommonJS required)
Add tests for core build functionality: - Build from a package subdirectory - No stale artifacts on second build - Filter flag to compile only matching modules Add build error tests: - Parse error reporting with file location - Type error reporting - Errors when a dependency module is deleted - Circular dependency detection
Add module operation tests: - File rename with and without dependents - Duplicate module name detection - Interface file compilation and error cases Add namespace package tests: - Build with namespace flag - Namespace in compiler args - File rename in namespaced package Add dev-dependency tests: - Dev source compiles with dev dependencies - Non-dev source cannot use dev dependencies - Clean removes dev source artifacts
Add build config tests: - Experimental feature flags (valid, invalid key, invalid format) - After-build hook execution (success and failure) - Warning configuration in compiler args - Warn-error CLI override - Deprecated and unknown config field warnings Add module system tests: - CommonJS package with .bs.js suffix - CommonJS in compiler args - Suffix change triggers rebuild - Duplicate package-spec suffix error Add PPX integration tests using lightweight noop-ppx: - PPX build produces output - PPX flags in parser args - PPX flags not in compiler args Add JSX tests: - JSX v4 build with @rescript/react - JSX flags in parser args - JSX preserve flag
Add tests for scoped clean, node_modules dependency cleanup, and verifying no false compiler-update message after clean+rebuild.
Add format tests: - Stdin formatting for .res and .resi - Single file and all-files formatting - Subdirectory-scoped formatting - Check mode (pass and fail cases) Add compiler-args tests: - CWD invariance (same output from root and subdirectory) - Warning flags in both parser and compiler args
Verify that a concurrent build is prevented while watch mode holds the lock file.
Add watch mode tests: - New file creation triggers compilation - Warning persistence across incremental builds - Config change triggers full rebuild - Changes outside source dirs are ignored - Missing source folder does not crash watcher - Invalid config change recovery (watcher keeps running) - File rename removes old artifacts and compiles new file - File deletion removes artifacts
Tracing spans are thread-local, so compile_file spans created inside Rayon's par_iter had no parent connection to the compile_wave span on the main thread. Pass the wave span explicitly via `parent: &wave_span` to establish the correct parent-child relationship.
When a file is saved in the LSP, only compile the saved file and its transitive dependencies instead of every module in the project. After the initial LSP build (TypecheckOnly), all modules sit at CompilationStage::TypeChecked. A TypecheckAndEmit build targets Built, so every module would enter the compile universe. In a large project this means the first save compiles the entire codebase to JS. Fix this by computing the downward dependency closure of the saved file and temporarily promoting modules outside that closure to Built. After the incremental build, promoted modules are restored to TypeChecked. Modules already at Built from a previous save are left untouched. Also change mark_file_parse_dirty to return Option<String> (the module name) so did_save can identify the entry point for the closure walk.
package get build to js.
Add single-file typecheck on unsaved edits (didChange). The unsaved buffer content is written to a temp file in the build directory and passed to bsc directly with TypecheckOnly. Diagnostics are remapped back to the original source path. Refactor didSave into two phases: - compile_dependencies (TypecheckAndEmit): compile the saved file and its transitive imports to produce JS output. - typecheck_dependents (TypecheckOnly): re-typecheck modules that transitively import the saved file to surface errors from API changes, without emitting JS. This means saving Library.res immediately shows type errors in App.res without needing to save App.res first. Other changes: - Extract find_module_for_file helper on BuildCommandState - Add get_dependent_closure (reverse dependency traversal) - Use #[instrument] consistently for OTEL spans in the lsp/ folder - Register new OTEL spans in test-context.mjs
Add an internal `-bs-read-stdin` flag to bsc that reads source from stdin instead of from the file argument. The filename argument is still required for error locations, file kind classification, and output prefix derivation. Update the LSP didChange handler to pipe unsaved buffer content directly to bsc's stdin instead of writing temporary files to disk. This eliminates unnecessary filesystem I/O on every keystroke. Key changes: - compiler: add `Js_config.read_stdin` flag and `-bs-read-stdin` CLI option - compiler: add `Res_io.read_stdin` and `Res_driver.parse_*_from_stdin` - compiler: disable `binary_annotations` when reading from stdin (avoids Digest.file call on non-existent source file) - rewatch: replace temp file write/cleanup in did_change.rs with stdin piping
Add a new `completion-rewatch` subcommand to the analysis binary that receives all needed context (pathsForModule, opens, package config) via JSON on stdin, bypassing the expensive project discovery that the existing `completion` command performs. Analysis binary changes: - Add `CommandsRewatch.ml` with JSON parsing and package construction - Add `CompletionFrontEnd.completionWithParserFromSource` to parse from a source string instead of reading from disk - Add `Completions.getCompletionsFromSource` that takes source + package - Add `Cmt.loadFullCmtWithPackage` that uses a pre-built package record instead of calling `Packages.getPackage` Rust LSP changes: - Track open buffers in `Backend.open_buffers` (updated on didChange) - Enable completion_provider capability with trigger characters - Add `lsp/completion.rs` that builds the JSON blob with all module/ package context, spawns `rescript-editor-analysis.exe completion-rewatch`, and deserializes the LSP-conformant response - If no .cmt exists yet (completion before any didChange), run a typecheck first to produce it Test infrastructure: - Add `completeFor` helper to lsp-client.mjs - Add `lsp.completion` span to OTEL summary - Add completion integration test
Redesign the LSP queue from action-oriented events (Typecheck, Build, FullBuild) to fact-based events that describe what happened: - BufferOpened, BufferChanged (didOpen / didChange) - FileChangedOnDisk (didSave / didChangeWatchedFiles CHANGED) - FileCreated, FileDeleted (didChangeWatchedFiles CREATED / DELETED) The merge() function derives the correct build actions from these facts, applying promotion rules (e.g. BufferChanged + FileChangedOnDisk promotes a typecheck to an incremental build with post-build recheck). PendingState is now purely action-oriented with three clear fields: - typechecks: files needing typecheck (unsaved buffer content) - compile_files: files needing incremental build (saved to disk) - build_projects: paths requiring full project rebuild Key improvements: - FileCreated/FileDeleted evict stale per-file entries from typechecks and compile_files since the full rebuild will cover them - Merged FileSaved and FileChangedOnDisk into a single variant — they carried the same payload and had identical merge behavior - Removed deleted_uris tracking from PendingState — the old/new URI diff after reinitialize_project naturally clears stale diagnostics - File creation/deletion events now carry only file_path (no URI), and project root resolution happens at flush time Split the ~1630-line queue.rs into focused submodules: queue.rs — types, public API, consumer, merge, flush orchestration queue/file_build.rs — per-file incremental build (dependency + dependent closure) queue/file_typecheck.rs — per-file typecheck (parallel, wave-based, staleness-aware) queue/project_build.rs — per-project full rebuild + artifact cleanup Added tests for file deletion cleanup (.res deletes .resi + JS, .resi deletion is a no-op) and comprehensive merge unit tests covering all promotion rules and cross-cutting scenarios (93 unit tests, 101 integration tests).
Replace rayon `into_par_iter` with `std::thread::scope` for workspace-group-level parallelism in the initial build. Each workspace build uses rayon internally for file-level parallelism (parse, compile), where tasks block on `bsc` subprocess I/O via `Command::output()`. Nesting this inside an outer `par_iter` on the same global thread pool caused thread starvation: outer tasks occupied all rayon threads, leaving none for inner tasks to make progress. Using dedicated OS threads for the outer level avoids this — they don't consume rayon pool slots, so the full pool remains available for the inner build parallelism.
When `diagnostics_http: true` is set in initializationOptions, the LSP spawns a lightweight HTTP server on a random free port. External tools (e.g. LLM agents) can query `GET /diagnostics` to retrieve current compiler diagnostics without triggering a separate build. The endpoint blocks until the LSP is idle (no pending events or flushes in progress), with a 30-second timeout. A pending-ID model with a FlushGuard ensures correct idle detection even when events arrive during a flush or a flush panics.
errors When saving a file in the LSP, the compile loop would expand the compile universe to include dependents of shared dependencies. If any of those dependents (in unrelated packages) had errors, the loop would abort before the saved file got compiled — producing no JS output. Fix by scoping the compile universe to only the dependency closure for TypecheckAndEmit builds: - Skip the dependent expansion in compile() for TypecheckAndEmit - Don't dirty dependents outside the compile universe during compilation The LSP already handles dependents separately via typecheck_dependents, so the expansion was redundant and harmful in multi-package projects. Add LSP integration test verifying JS is produced when another package has errors. Add doc comments explaining the two-step save-build flow.
- Centralize file classification helpers (is_rescript_source, is_rescript_config, is_rescript_file) in file_args.rs - Guard did_open, did_change, did_save to skip non-ReScript files - Route rescript.json saves to full project rebuilds via new ConfigChanged queue event - Add LSP integration tests for config change handling - Update LSP.md: note ACP diagnostics discussion, update Next Up items
Wait for both publishDiagnostics and buildFinished concurrently using Promise.all, so the test completes regardless of which notification arrives first. Filter timing-dependent lsp.typecheck spans from the snapshot to handle the two possible execution orders.
ConfigChanged now clears pending typechecks for the affected project, since the full TypecheckOnly rebuild already covers them. Only files under the config's project root are cleared to avoid discarding work for unrelated projects in a monorepo. compile_files are kept because the full rebuild doesn't emit JS.
When a file is saved that starts importing a new module (e.g., one created externally by a shell command or LLM agent), the dependency closure was computed from stale module deps. This excluded the new dependency from the compile universe, causing "module not found" errors. Pre-parse dirty files and resolve their deps in compile_dependencies before computing the closure, so the saved content's imports are reflected in the compile universe.
`rescript clean` was removing lib/lsp/ and lib/lsp-ocaml/, which are owned by a running LSP server. This caused panics and broken IDE features when the LSP tried to use deleted artifacts. Also remove panicking .expect() calls when copying source files during compilation — if a file is deleted between build start and copy (common in watch/LSP), the build continues gracefully instead of crashing. Add a `cli` helper to `runLspTest` context so LSP tests can invoke rescript CLI commands without manual sandbox lifecycle management. Refactor concurrent-build tests to use it, and add a new clean test verifying the LSP survives `rescript clean`.
Replace BuildProfile with BuildConfig { OutputTarget, CompileScope },
and split incremental_build into separate parse and compile phases.
Build pipeline:
- Extract parse_and_resolve() for parsing and dep resolution
- Extract compute_compile_universe() for universe computation
- incremental_build() now takes an explicit compile universe
- All callers follow: parse → compute universe → compile
Compile loop correctness:
- Only dirty dependents on successful compilation
- Snapshot/restore module stages to prevent orphan dirty leaks
- Remove post-hoc cleanup from LSP typecheck_dependents
LSP simplification:
- Eliminate duplicate parsing in compile_dependencies
- Remove verbose debug logging from file_build.rs
- Handle parse error diagnostics in initial_build and project_build
Add dep-chain test fixture and stale-dirty regression test.
Update LSP.md build flow documentation.
Introduce a `CompileUniverse` struct that holds both the full set of modules participating in a compile cycle (`all`) and the subset that were directly dirty (`originally_dirty`). This replaces the bare `AHashSet<String>` parameter and removes the architectural mismatch where per-module `compilation_stage` mutations inside the loop re-derived the same information. Key changes: - Replace the `needs_compile` gate with `clean_modules`-based logic: a module skips compilation if it was not originally dirty and all its in-universe dependencies had unchanged `.cmi` output. - Remove in-loop dirtying of dependents (`compilation_stage = Dirty`). - Remove `pre_loop_stages` snapshot/restore (no longer needed without in-loop mutations). - Move stage/timestamp updates to a post-loop pass that runs even when the loop breaks early due to compilation errors. - Remove LSP compensating hacks: `typecheck_dependents` dirtying, and `Built → TypeChecked` downgrades in initial and project builds. - Replace the 5-element result tuple with `CompileModuleResult` struct and `CompileFileOutcome` enum for readability.
Rename `needs_deps_rescan` to `needs_dependencies_rescan` across all usage sites. Add doc comments to every field in the Module struct, organized into three sections: - Module identity (immutable after construction) - Dependency graph (mutated in deps.rs after each AST rescan) - Build status (mutated throughout the build pipeline) Each comment describes the field's purpose and where it is mutated.
CompileUniverse CompileScope now carries anchor module names in its scoped variants (CompileDependencies, TypecheckDependents) as AHashSet<String>, and incremental_build derives the CompileUniverse internally via compute_universe_for_scope. This eliminates the pattern where every caller had to construct a matching CompileUniverse separately — a mismatch would silently produce wrong results. Other changes: - Move dependency_closure.rs from lsp/ to build/ (no LSP dependency) - Change compile::compile() to borrow &CompileUniverse (zero clones) - Return modules in IncrementalBuildResult/Error for caller use - Use AHashSet<String> consistently in closure function signatures - Update LSP test snapshots for new typecheck_dependents trace spans
Consolidate show_progress, plain_output, initial_build, and
only_incremental into an OutputMode enum with two variants:
- Standard { show_progress, plain_output, initial_build } for
CLI/watcher
- Silent for LSP (no user-facing output)
Make create_sourcedirs unconditional — it now always runs and writes
to the correct output folder (lib/bs or lib/lsp) based on OutputTarget.
Derive only_incremental internally from !initial_build, removing it
as a parameter entirely.
This reduces incremental_build from 8+ parameters down to 4 and makes
LSP call sites cleaner — they just use OutputMode::Silent without
needing to think about display settings.
write_build_ninja - Take incremental_build's build_config by reference (&BuildConfig) instead of by value, consistent with parse_and_resolve - Gate logs::initialize/finalize on OutputMode (is_silent) instead of OutputTarget, decoupling artifact location from display behavior - Guard all eprintln! calls in parse_and_resolve and incremental_build with !is_silent() so LSP builds never leak to stderr - Remove redundant show_progress parameter from compile::compile; it already receives &BuildConfig and can read it from there - Skip sourcedirs::print on scoped LSP builds (CompileDependencies, TypecheckDependents) where no files were added or removed - Remove write_build_ninja entirely (no longer needed)
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This branch explores embedding a full LSP server directly into the
rescriptbinary (rescript lsp), replacing the current architecture where a Node.js extension mediates between the editor and separate build/analysis processes.The core idea
Today, the ReScript editor experience involves three processes: a Node.js VS Code extension, the
rescriptbuild watcher, and therescript-editor-analysis.exebinary. They communicate through files on disk — the editor extension launches builds, waits for artifacts, then shells out to the analysis binary for each request.This branch collapses the build system and LSP server into a single Rust process using
tower-lsp. The build state lives in memory, and analysis requests shell out to the samerescript-editor-analysis.exebut with source code passed via stdin instead of being read from disk.No temp files — stdin everywhere
Both
bscand the analysis binary receive source code via stdin rather than through temporary files. FordidChange(unsaved edits),bsc -bs-read-stdinproduces diagnostics without writing anything to disk. For analysis requests (hover, completion, code actions, etc.), the analysis binary receives a JSON blob on stdin containing the source text, cursor position, and package metadata. The OCaml analysis code was refactored withFromSourcevariants that parse from a string rather than opening files — so everything works correctly on unsaved editor buffers.Separate build profile:
lib/lspThe LSP server writes its build artifacts to
lib/lsp/instead oflib/bs/. This means it doesn't conflict withrescript buildorrescript build -wrunning in a terminal — both can operate independently on the same project without stepping on each other's artifacts.Initial build: typecheck only
On
initialized, the server runs a full build but only goes as far as producing.cmt/.cmifiles (theTypecheckOnlyprofile). It deliberately skips JS emission. This gets the editor operational as fast as possible — type information for hover, completion, go-to-definition etc. is all available, without paying the cost of generating JavaScript for every module upfront.Smart incremental builds on save
When a file is saved, the server runs a two-phase incremental build:
Emit JS for the dependency closure — the server computes the transitive imports of the saved file and only emits JavaScript for that file and its dependencies. Modules outside this closure are skipped entirely. So saving a module produces JS for it and any imports that haven't been compiled yet — not the entire project.
Typecheck reverse dependencies — modules that transitively depend on the saved file are re-typechecked to surface errors caused by API changes (e.g. a removed export). This gives you project-wide diagnostics on save — if you rename a function, you immediately see errors in every file that uses it, even files you don't have open. No JS is emitted for these — they get their JS when they are themselves saved.
What's implemented
All standard analysis endpoints are wired up: completion (with resolve), hover, signature help, go to definition, type definition, references, rename (with prepare), document symbols, code lens, inlay hints, semantic tokens, code actions, and formatting.
Observability
Every LSP request and build operation is traced with OpenTelemetry spans, viewable in Jaeger. This makes it straightforward to profile request latency and understand what the server is doing.
Test infrastructure
Each endpoint has integration tests using
vscode-languageserver-protocolthat boot a real LSP server in a sandbox, send requests, and snapshot both the results and the OTEL trace structure.What's not here yet
workspace/didChangeWatchedFiles— handling external file changes (git checkout, etc.)createInterfaceandopenCompiledcustom commandsThis is an experiment to validate the architecture. If it proves useful, individual pieces can be split into focused PRs.