From cc190f5290eeff5e66708d8ffda0a16f6c9eac30 Mon Sep 17 00:00:00 2001 From: Owen Lamont Date: Sun, 6 Sep 2026 20:07:51 +0800 Subject: [PATCH 1/3] fix(lsp): long-poll workspace/diagnostic instead of answering idle pulls Result ids let an unchanged file answer `unchanged`; an all-unchanged report is held open and resumed on the next state change, so an idle workspace costs nothing. A `partialResultToken` streams changed files as `$/progress` batches. Bumps to 0.22.0. Claude-Session: https://claude.ai/code/session_01L2XE37GUJWcHp7hPKMdHU8 --- AGENTS.md | 33 +- Cargo.lock | 122 +++---- Cargo.toml | 2 +- docs/editor-integration.md | 40 ++- docs/llms-full.txt | 40 ++- img/benchmark-5x5-5runs.svg | 656 ++++++++++++++++++------------------ package.json | 2 +- pyproject.toml | 2 +- src/lsp/analysis.rs | 16 + src/lsp/mod.rs | 598 ++++++++++++++++++++++++++------ src/report.rs | 7 +- tests/lsp_server.rs | 464 ++++++++++++++++++++++++- tests/lsp_unit.rs | 147 ++++++-- uv.lock | 2 +- 14 files changed, 1556 insertions(+), 575 deletions(-) diff --git a/AGENTS.md b/AGENTS.md index 997ae2e..cf204ac 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -516,13 +516,32 @@ user skills; `.agents/skills/` is in-repo contributor tooling and is never publi pull, not twice — clients like VS Code merge the two channels); pull diagnostics (`textDocument/diagnostic` + `workspace/diagnostic`, the latter over a per-entry-cancellable - `discover::gather_yaml_from_dir_cancellable` walk of every root, deduped, full report - each call, no result-id caching; it runs on a background worker thread — so the message - loop stays responsive — lints files in parallel via `rayon`, and is cancellable via - `$/cancelRequest`/shutdown through an `AtomicBool` the worker checks (the walk per - entry; only an in-progress single-file read is uninterrupted). A new pull - supersedes/cancels any in-flight one (bounding workers); `serve` joins outstanding - workers before returning); `source.fixAll.ryl` + per-rule + `discover::gather_yaml_from_dir_cancellable` walk of every root, deduped; it runs on a + background worker thread — so the message loop stays responsive — lints files in + parallel via `rayon`, and is cancellable via `$/cancelRequest`/shutdown through an + `AtomicBool` the worker checks (the walk per entry, the lint per `SCAN_BATCH` batch, so + only an in-flight batch is uninterruptible). A new pull supersedes/cancels any in-flight + one (bounding workers); `serve` joins outstanding workers before returning). Each report + carries a `result_id` (`analysis::result_id`, SHA-256 of the serialized diagnostics; + `None` for a clean file, which is then omitted), so a matching `previousResultIds` entry + answers `Unchanged`, and + a previously-reported path the walk no longer covers is cleared with an empty, id-less + report. **`workspace/diagnostic` is long-polled** (#408, after ty): the VS Code client + re-pulls a fixed 2 s after every response with no knob, so an all-`Unchanged` report (an + empty one included) is *not* answered — `finish_scan` parks it in `Server::pull` and + `wake` re-scans on the next didOpen/didChange/didClose/watched-file/config notification. + A worker cannot see session state, so scans return over `scan_tx`/`scan_rx` into a + `crossbeam_channel::select!` in `run_loop`, and `Server::revision` (bumped by those + notifications) stops a scan that raced a change from suspending on a stale report. A + parked pull is answered on `$/cancelRequest`, when a new pull supersedes it, and at + shutdown. The watcher registration covers `**/*.{yaml,yml}` as well as config names so + an out-of-editor change can wake it; `is_config_uri` keeps a source change from being + taken for a config one. A `partialResultToken` switches `ReportSink` from bulk to + streaming: the scan lints in `SCAN_BATCH` batches (also the cancellation granularity) + and `Full` reports go out as `$/progress` batches — the first at once, then per + `STREAM_INTERVAL` — while `Unchanged` ones are held for the response, so nothing is + sent twice. `ScanOutcome::streamed` then forces an answer: having streamed, the + request can no longer be held open); `source.fixAll.ryl` + per-rule `source.fixAll.ryl.` (via `fix::SAFE_FIX_RULE_IDS`, YAML only) + `quickfix` disable-rule inserts (`# ryl disable-line` / first-line `# ryl disable-file`; the disable-line is suppressed for a diagnostic inside a block scalar, where a `#` would be diff --git a/Cargo.lock b/Cargo.lock index 302139a..57d1b26 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -57,9 +57,9 @@ checksum = "f2032f911046de80f0a198e0901378627c33f59ea0ac00e363d481118bd70a53" [[package]] name = "aws-lc-rs" -version = "1.18.0" +version = "1.18.1" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "ce2b2dcc879c3bae0d371e77c99f2238400ef24ec001394befa67b6e543add9e" +checksum = "b281d307588d634de920874890732659e2e7672f72b5e10e81badc1a8a83621e" dependencies = [ "aws-lc-sys", "zeroize", @@ -67,9 +67,9 @@ dependencies = [ [[package]] name = "aws-lc-sys" -version = "0.44.0" +version = "0.45.0" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "f09fae7be8bb3174e05c6afdb34199e6dc0c7c04ba9fa237b1967adfbde27483" +checksum = "9bff6c3b54fad79a2e60b8102caf565819711497c1f5f092f49508e2f5c31b27" dependencies = [ "cc", "cmake", @@ -156,9 +156,9 @@ checksum = "fc652a48c352aef3ea3aed32080501cf3ef6ed5da78602a020c991775b0aff04" [[package]] name = "cc" -version = "1.4.4" +version = "1.4.5" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "0ad534f4357a5264cce5019c989cf66a4f0dc4e0d1b1d15f8aacec0ff7360273" +checksum = "005ec2760ca554fae18df7a11195552ec576cd665632a881bc011d5bb2fd4d80" dependencies = [ "find-msvc-tools", "jobserver", @@ -211,7 +211,7 @@ dependencies = [ "heck", "proc-macro2", "quote", - "syn 3.0.4", + "syn 3.0.5", ] [[package]] @@ -272,18 +272,18 @@ dependencies = [ [[package]] name = "crossbeam-channel" -version = "0.5.16" +version = "0.5.17" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "d85363c37faeca707aef026efa9f3b34d077bce547e48f770770625c6013679e" +checksum = "98b0cc327b5bc766e7fda9c9260cc0fa81b43a8e240440422dff70788e3f9ef1" dependencies = [ "crossbeam-utils", ] [[package]] name = "crossbeam-deque" -version = "0.8.7" +version = "0.8.8" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "5181e0de7b61eb03a81e347d6dd8797bae9da5146707b51077e2d71a54ec0ceb" +checksum = "622f3fc73690be383c7214310406f28a90e6edeadc3cea882f9d71e495b9711a" dependencies = [ "crossbeam-epoch", "crossbeam-utils", @@ -291,18 +291,18 @@ dependencies = [ [[package]] name = "crossbeam-epoch" -version = "0.9.20" +version = "0.9.21" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "2d6914041f254d6e9176c01941b21115dcfb7089e55135a35411081bd106ef3f" +checksum = "dc74980687109a3b14c72fd458107bf0baa1da1a1a805e178d15501ba9b86d9d" dependencies = [ "crossbeam-utils", ] [[package]] name = "crossbeam-utils" -version = "0.8.22" +version = "0.8.23" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "61803da095bee82a81bb1a452ecc25d3b2f1416d1897eb86430c6159ef717c17" +checksum = "a31eee39dddec8330830986fcd7625edb5a24ec90ea038215273bbc3adb08ac6" [[package]] name = "crypto-common" @@ -321,9 +321,9 @@ checksum = "4583a4551df46e2792f82ceeac45e850d2e2d5debba0b91f102385cda5b11f06" [[package]] name = "diffy" -version = "0.5.1" +version = "0.5.2" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "10aec8f7f9393bd6a4f2762be0ceb012d3cbe2478987258cc9960de148561914" +checksum = "3e3dc2f773b6aaa63b1a7684b8589f670a8a0146a510b74d23a401c882364b49" dependencies = [ "hashbrown", ] @@ -368,7 +368,7 @@ checksum = "c6232dd377dcc64799954cbd3a9bb882e9cdc1308ccd87b1c098f1fb2eaf82a8" dependencies = [ "proc-macro2", "quote", - "syn 3.0.4", + "syn 3.0.5", ] [[package]] @@ -442,9 +442,9 @@ checksum = "da7c62ceae207dd37ea5b845da6a0696c799f85e97da1ab5b7910be3c1c80223" [[package]] name = "find-msvc-tools" -version = "0.1.11" +version = "0.1.12" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "d45db016d36b838f563236e9193d0ee6ce38f3f68b6c94e914b4929c96bbb890" +checksum = "3e0f1c7c3a72c66fd80abe965175f7523475c0489a87d3ff9d6e8c87d87a9d2d" [[package]] name = "fluent-uri" @@ -885,9 +885,9 @@ dependencies = [ [[package]] name = "indexmap" -version = "2.14.1" +version = "2.14.2" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "07aa2048142242915a31d35844fb311e0e53fcca590c3a0a40dcf1b841fa09eb" +checksum = "cc4e190f5d26ca7051642629da2c52fc03bde85a03197c99408dcd291734c855" dependencies = [ "equivalent", "hashbrown", @@ -895,9 +895,9 @@ dependencies = [ [[package]] name = "ipnet" -version = "2.12.1" +version = "2.12.2" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "6a756c3fac73139e83f14c2d742155dd2b78d3ee56597b419a0579b7bdd6dd78" +checksum = "791930b43c0d5973160d90a8f3894509f2b273430f5c5c73b668636d0287c5c0" [[package]] name = "itoa" @@ -966,9 +966,9 @@ dependencies = [ [[package]] name = "js-sys" -version = "0.3.104" +version = "0.3.105" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "0e0c1080212aad755ea003d18543e8768dd432c48819efd73a7bf1e39b7a5a3a" +checksum = "ce57d20d1ea864ce2ac172ab472d409214f4fd359f0b2a2775abdf522e2af99e" dependencies = [ "cfg-if", "futures-util", @@ -1038,9 +1038,9 @@ checksum = "3eaf3ede3fee6db1a4c2ee091bf8a8b4dccdc6d17f656fb07896ee72867612f2" [[package]] name = "libredox" -version = "0.1.21" +version = "0.1.23" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "d7955dfc218a8afb29dfeffd540e3a6e96baeb94fe7138228dd7cc6937fbbf96" +checksum = "8d8f1ea3f21fd3405dcaf6c9b5c1630af9afc422d9073ea39c5f6d6c772e08ed" dependencies = [ "libc", ] @@ -1112,9 +1112,9 @@ checksum = "c2a86d3146ed3995b5913c414f6664344b9617457320782e64f0bb44afd49d74" [[package]] name = "mio" -version = "1.2.2" +version = "1.2.3" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "30d65c71f1ce40ab09135ce117d742b9f8a19ff91a41a8b57ed50bc2de59c427" +checksum = "4b18443e9c262bfe8fa82f51666e2642c53393f7e5c27b3e1aeab922cff5b9d8" dependencies = [ "libc", "wasi", @@ -1455,7 +1455,7 @@ checksum = "92ecd8964f8453721699a1ed72037b0db49ce2f5a5138486ee89bed6f67cdf3a" dependencies = [ "proc-macro2", "quote", - "syn 3.0.4", + "syn 3.0.5", ] [[package]] @@ -1673,7 +1673,7 @@ dependencies = [ [[package]] name = "ryl" -version = "0.21.0" +version = "0.22.0" dependencies = [ "clap", "clap_complete", @@ -1746,7 +1746,7 @@ dependencies = [ "proc-macro2", "quote", "serde_derive_internals", - "syn 3.0.4", + "syn 3.0.5", ] [[package]] @@ -1811,7 +1811,7 @@ checksum = "e7a5d71263a5a7d47b41f6b3f06ba276f10cc18b0931f1799f710578e2309348" dependencies = [ "proc-macro2", "quote", - "syn 3.0.4", + "syn 3.0.5", ] [[package]] @@ -1822,7 +1822,7 @@ checksum = "f852137cce035d6a4df67ccce505ff6b3e9fd3a10e3e52b24dc71e650bb1a9bd" dependencies = [ "proc-macro2", "quote", - "syn 3.0.4", + "syn 3.0.5", ] [[package]] @@ -1846,7 +1846,7 @@ checksum = "8d3b1629de253c70a0508c3899572da79ca359fdab27c7920ff00406df418906" dependencies = [ "proc-macro2", "quote", - "syn 3.0.4", + "syn 3.0.5", ] [[package]] @@ -1905,9 +1905,9 @@ checksum = "0c790de23124f9ab44544d7ac05d60440adc586479ce501c1d6d7da3cd8c9cf5" [[package]] name = "smallvec" -version = "1.15.2" +version = "1.16.0" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "8ed6a63f02c8539c91a8685a86f4099661ba3da017932f6ebbea6de3f0fa7c90" +checksum = "b9be42f50aa861c555654aa3a37f52f4b1074bacf4e48fe0ef7fa584e80f1f0f" [[package]] name = "socket2" @@ -1971,9 +1971,9 @@ dependencies = [ [[package]] name = "syn" -version = "3.0.4" +version = "3.0.5" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "e6275cddf4610d1775e6d1fe9469b2e77d0f39fd98fb7450901b821e0c53649f" +checksum = "12df2e0110f65b775f769bb17ef989067a1d931b2eb822bd4346631eeada89f9" dependencies = [ "proc-macro2", "quote", @@ -2050,7 +2050,7 @@ checksum = "bc04cd3e1236dd4a98afca4569f2deb3f120e5422a4023be2cb683f8486292af" dependencies = [ "proc-macro2", "quote", - "syn 3.0.4", + "syn 3.0.5", ] [[package]] @@ -2065,9 +2065,9 @@ dependencies = [ [[package]] name = "tinyvec" -version = "1.12.0" +version = "1.13.2" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "bb4ebadaa0af04fab11ae01eb5f9fdb5f9c5b875506e210e71c07873528baa7f" +checksum = "4cf0ded5c4e56918d8f8a339e1bb67d038d3bc6d144ac407904015ba2e4cde9b" dependencies = [ "tinyvec_macros", ] @@ -2094,9 +2094,9 @@ dependencies = [ [[package]] name = "tokio-rustls" -version = "0.26.4" +version = "0.26.5" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "1729aa945f29d91ba541258c8df89027d5792d85a8841fb65e8bf0f4ede4ef61" +checksum = "b0c85f2c3ef0b1cd58b36682f4b17aaa995f0e5db534d85692b4903abce21f67" dependencies = [ "rustls", "tokio", @@ -2118,9 +2118,9 @@ dependencies = [ [[package]] name = "toml" -version = "1.1.4+spec-1.1.0" +version = "1.1.5+spec-1.1.0" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "3aace63f4bbcdfc2c965b059de67119c89c4017a70d633be6c104910f67056f5" +checksum = "12c0ba9680044b4ce98d391a62094047eada0d64860b80166c39f4a6b5640785" dependencies = [ "indexmap", "serde_core", @@ -2355,9 +2355,9 @@ dependencies = [ [[package]] name = "wasm-bindgen" -version = "0.2.127" +version = "0.2.128" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "1b70935747edd64d89de3efa29d73789b806c15798f8e7dca4d8ac356b50ce70" +checksum = "aecb87a33d3b0c5e3b7aa46336eaf486cffafbd281b195e4c8b80d50df2351bf" dependencies = [ "cfg-if", "once_cell", @@ -2368,9 +2368,9 @@ dependencies = [ [[package]] name = "wasm-bindgen-futures" -version = "0.4.77" +version = "0.4.78" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "6b7777d5cc23d0e91404e53ce2d5e8ec7acae3026b16233dba62cd3246457950" +checksum = "6ef4c5d3d2cdf5c54f4231181768f5510842e350db025faf1f7163b1030ed928" dependencies = [ "js-sys", "wasm-bindgen", @@ -2378,9 +2378,9 @@ dependencies = [ [[package]] name = "wasm-bindgen-macro" -version = "0.2.127" +version = "0.2.128" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "77775f8f3f7217702089053b94958f8f54061a3f663417df76e19cbdcca29bc1" +checksum = "a690d511e3c1a8b3a55e33511e3c2c00c78415cd23650f32b808627f5696b9ed" dependencies = [ "quote", "wasm-bindgen-macro-support", @@ -2388,31 +2388,31 @@ dependencies = [ [[package]] name = "wasm-bindgen-macro-support" -version = "0.2.127" +version = "0.2.128" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "e11d33f857dc2fb11b8bc75aee111aa9cbeb12cd9f25efd3d4c2a3dd4e235284" +checksum = "411e4887f0071ef2d2164a9d5fdf2d20efbef78fccd3a78b0c10a1dc5295e48a" dependencies = [ "bumpalo", "proc-macro2", "quote", - "syn 2.0.119", + "syn 3.0.5", "wasm-bindgen-shared", ] [[package]] name = "wasm-bindgen-shared" -version = "0.2.127" +version = "0.2.128" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "7ef64dbcc55df09c7e5a46182d181c2cfa3e925f3da937ea764728b4bbb9dcbf" +checksum = "81941cd78d0c92026c33e5e01312845a4cb1e9af3407f9134b100dd03144103e" dependencies = [ "unicode-ident", ] [[package]] name = "web-sys" -version = "0.3.104" +version = "0.3.105" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "c435338968042f4f59a557f690a253676d47ce13ceb55d70100e7facf6620a30" +checksum = "9fbddc4a036f00ec4f18c83445bd3115cb306a91da554919a099d9222fe4a7f8" dependencies = [ "js-sys", "wasm-bindgen", @@ -2664,7 +2664,7 @@ checksum = "34df6fc39dbd26ddc9c10e6a2984476e13acce22e64e4487636ef494369225da" dependencies = [ "proc-macro2", "quote", - "syn 3.0.4", + "syn 3.0.5", ] [[package]] diff --git a/Cargo.toml b/Cargo.toml index 9b9fa93..a0e52ee 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -1,6 +1,6 @@ [package] name = "ryl" -version = "0.21.0" +version = "0.22.0" edition = "2024" description = "Fast YAML linter inspired by yamllint" readme = "README.md" diff --git a/docs/editor-integration.md b/docs/editor-integration.md index d05887f..4037832 100644 --- a/docs/editor-integration.md +++ b/docs/editor-integration.md @@ -15,14 +15,14 @@ hover only explains ryl's own diagnostics. | Capability | LSP feature | Behaviour | | --- | --- | --- | | Diagnostics (push) | `textDocument/publishDiagnostics` | Every enabled rule, re-linted on open and on each change. Sent only to a client that does *not* advertise the pull model (see below) | -| Diagnostics (pull) | `textDocument/diagnostic`, `workspace/diagnostic` | On-demand diagnostics for one document or every `*.yaml`/`*.yml` under the workspace root | +| Diagnostics (pull) | `textDocument/diagnostic`, `workspace/diagnostic` | On-demand diagnostics for one document or every `*.yaml`/`*.yml` under the workspace root; result ids and long polling keep an idle workspace free | | Fix all | `source.fixAll.ryl` code action | Applies every safe fix to the document (the `--fix` set) | | Fix all of one rule | `source.fixAll.ryl.` code action | Applies just one safe-fixable rule's fixes (offered per rule with a diagnostic) | | Disable a rule | `quickfix` code action | Inserts `# ryl disable-line rule:` (per line) or a first-line `# ryl disable-file` (whole file) | | Formatting | `textDocument/formatting` | Same as "fix all": formatting *is* applying safe fixes | | Hover | `textDocument/hover` | The rule and message for a diagnostic under the cursor, with a link to the rules reference | | Rename | `textDocument/rename`, `textDocument/prepareRename` | Rename a YAML anchor/alias and every same-name use in its document | -| Config watching | `workspace/didChangeWatchedFiles` | When a ryl/yamllint config file changes on disk, re-lints open documents (push clients) or asks pull clients to re-pull via `workspace/diagnostic/refresh` | +| File watching | `workspace/didChangeWatchedFiles` | Watches ryl/yamllint config files and `*.yaml`/`*.yml` sources: a config change re-lints open documents (push clients) or asks pull clients to re-pull via `workspace/diagnostic/refresh`; any change resumes a long-polling workspace pull | The fix-all action and formatting both apply ryl's whole-file safe fixes; ryl has no per-occurrence "fix just this one" action, because its fix engine operates per file (the @@ -119,12 +119,13 @@ issue tracker: [owenlamont/ryl-vscode](https://github.com/owenlamont/ryl-vscode) - **Position encoding** is negotiated at startup (UTF-8, UTF-16, or UTF-32); ryl supports all three and defaults to UTF-16 when the client states no preference, so columns line up correctly even for multi-byte and astral-plane characters. -- **Config watching** re-lints open documents when a ryl/yamllint config file changes on - disk, for clients that support dynamic `didChangeWatchedFiles` registration. It watches - the standard config filenames anywhere in the workspace plus an explicit `configPath`; - files pulled in via a config's `extends:` are not individually watched, so re-open a - document to refresh after editing those. A client without the capability simply picks up - a config change on the next edit or re-open. +- **File watching** covers both the standard config filenames anywhere in the workspace + (plus an explicit `configPath`) and `*.yaml`/`*.yml` sources, for clients that support + dynamic `didChangeWatchedFiles` registration. A config change re-lints open documents; + a source change out of the editor (a `git checkout`, another editor) resumes a + long-polling workspace pull. Files pulled in via a config's `extends:` are not + individually watched, so re-open a document to refresh after editing those. A client + without the capability simply picks up a change on the next edit or re-open. - **Document sync is incremental** — the editor sends only the edited range — but ryl re-lints the whole reconstructed document each time (it is fast enough that this is invisible). @@ -149,10 +150,25 @@ issue tracker: [owenlamont/ryl-vscode](https://github.com/owenlamont/ryl-vscode) (git-ignored files excluded); Markdown and files matched only by custom `[files]` globs are diagnosed when opened or pulled individually. The scan runs on a background thread (so editing, hover, and other requests stay responsive while it works) and lints files - in parallel across CPU cores. A new pull supersedes any in-flight one, and a - `$/cancelRequest` (and shutdown) stops it promptly: the directory walk is cancelled - per entry and the lint pass is fast, so only an in-progress single-file read is not - interrupted. + in parallel across CPU cores, in batches. A new pull supersedes any in-flight one, and a + `$/cancelRequest` (and shutdown) stops it promptly: the directory walk is cancelled per + entry and the lint pass per batch, so only an in-flight batch is not interrupted. +- **A workspace pull streams its results** when the client offers a `partialResultToken` + (VS Code does): changed files go out as `$/progress` batches while the scan is still + running — the first batch immediately, then at most one every 50 ms — so a large repo + fills the Problems panel progressively instead of in one jump at the end. Files that are + still `unchanged` are not streamed (they would tell the client nothing sooner) and come + back in the response, which is otherwise empty: a streamed report is never repeated. +- **Workspace pulls are long-polled, so an idle workspace costs nothing.** Every report + carries a `resultId` fingerprinting that file's diagnostics; a later pull that sends the + same id back is answered `unchanged`. When *every* file would be `unchanged` (a clean + workspace included) the server does not answer at all — it holds the request open, as the + LSP suggests for a workspace request that "can be long running", and responds only once an + edit, a watched-file change, or a config change makes something differ. This matters + because `vscode-languageclient` re-issues `workspace/diagnostic` a fixed two seconds after + every response with no client-side knob, so answering an idle pull just books another full + repo walk and lint two seconds later. A held request is still answered on + `$/cancelRequest` and at shutdown, so the client never waits on it forever. - **Disable-rule actions are offered for YAML documents only** — in Markdown the diagnostic's line is a host-file line whose embedded YAML carries a prefix, so a raw comment insert would be unreliable; Markdown documents get the fix-all action (which is diff --git a/docs/llms-full.txt b/docs/llms-full.txt index 82a7cde..6239fc3 100644 --- a/docs/llms-full.txt +++ b/docs/llms-full.txt @@ -2118,14 +2118,14 @@ hover only explains ryl's own diagnostics. | Capability | LSP feature | Behaviour | | --- | --- | --- | | Diagnostics (push) | `textDocument/publishDiagnostics` | Every enabled rule, re-linted on open and on each change. Sent only to a client that does *not* advertise the pull model (see below) | -| Diagnostics (pull) | `textDocument/diagnostic`, `workspace/diagnostic` | On-demand diagnostics for one document or every `*.yaml`/`*.yml` under the workspace root | +| Diagnostics (pull) | `textDocument/diagnostic`, `workspace/diagnostic` | On-demand diagnostics for one document or every `*.yaml`/`*.yml` under the workspace root; result ids and long polling keep an idle workspace free | | Fix all | `source.fixAll.ryl` code action | Applies every safe fix to the document (the `--fix` set) | | Fix all of one rule | `source.fixAll.ryl.` code action | Applies just one safe-fixable rule's fixes (offered per rule with a diagnostic) | | Disable a rule | `quickfix` code action | Inserts `# ryl disable-line rule:` (per line) or a first-line `# ryl disable-file` (whole file) | | Formatting | `textDocument/formatting` | Same as "fix all": formatting *is* applying safe fixes | | Hover | `textDocument/hover` | The rule and message for a diagnostic under the cursor, with a link to the rules reference | | Rename | `textDocument/rename`, `textDocument/prepareRename` | Rename a YAML anchor/alias and every same-name use in its document | -| Config watching | `workspace/didChangeWatchedFiles` | When a ryl/yamllint config file changes on disk, re-lints open documents (push clients) or asks pull clients to re-pull via `workspace/diagnostic/refresh` | +| File watching | `workspace/didChangeWatchedFiles` | Watches ryl/yamllint config files and `*.yaml`/`*.yml` sources: a config change re-lints open documents (push clients) or asks pull clients to re-pull via `workspace/diagnostic/refresh`; any change resumes a long-polling workspace pull | The fix-all action and formatting both apply ryl's whole-file safe fixes; ryl has no per-occurrence "fix just this one" action, because its fix engine operates per file (the @@ -2222,12 +2222,13 @@ issue tracker: [owenlamont/ryl-vscode](https://github.com/owenlamont/ryl-vscode) - **Position encoding** is negotiated at startup (UTF-8, UTF-16, or UTF-32); ryl supports all three and defaults to UTF-16 when the client states no preference, so columns line up correctly even for multi-byte and astral-plane characters. -- **Config watching** re-lints open documents when a ryl/yamllint config file changes on - disk, for clients that support dynamic `didChangeWatchedFiles` registration. It watches - the standard config filenames anywhere in the workspace plus an explicit `configPath`; - files pulled in via a config's `extends:` are not individually watched, so re-open a - document to refresh after editing those. A client without the capability simply picks up - a config change on the next edit or re-open. +- **File watching** covers both the standard config filenames anywhere in the workspace + (plus an explicit `configPath`) and `*.yaml`/`*.yml` sources, for clients that support + dynamic `didChangeWatchedFiles` registration. A config change re-lints open documents; + a source change out of the editor (a `git checkout`, another editor) resumes a + long-polling workspace pull. Files pulled in via a config's `extends:` are not + individually watched, so re-open a document to refresh after editing those. A client + without the capability simply picks up a change on the next edit or re-open. - **Document sync is incremental** — the editor sends only the edited range — but ryl re-lints the whole reconstructed document each time (it is fast enough that this is invisible). @@ -2252,10 +2253,25 @@ issue tracker: [owenlamont/ryl-vscode](https://github.com/owenlamont/ryl-vscode) (git-ignored files excluded); Markdown and files matched only by custom `[files]` globs are diagnosed when opened or pulled individually. The scan runs on a background thread (so editing, hover, and other requests stay responsive while it works) and lints files - in parallel across CPU cores. A new pull supersedes any in-flight one, and a - `$/cancelRequest` (and shutdown) stops it promptly: the directory walk is cancelled - per entry and the lint pass is fast, so only an in-progress single-file read is not - interrupted. + in parallel across CPU cores, in batches. A new pull supersedes any in-flight one, and a + `$/cancelRequest` (and shutdown) stops it promptly: the directory walk is cancelled per + entry and the lint pass per batch, so only an in-flight batch is not interrupted. +- **A workspace pull streams its results** when the client offers a `partialResultToken` + (VS Code does): changed files go out as `$/progress` batches while the scan is still + running — the first batch immediately, then at most one every 50 ms — so a large repo + fills the Problems panel progressively instead of in one jump at the end. Files that are + still `unchanged` are not streamed (they would tell the client nothing sooner) and come + back in the response, which is otherwise empty: a streamed report is never repeated. +- **Workspace pulls are long-polled, so an idle workspace costs nothing.** Every report + carries a `resultId` fingerprinting that file's diagnostics; a later pull that sends the + same id back is answered `unchanged`. When *every* file would be `unchanged` (a clean + workspace included) the server does not answer at all — it holds the request open, as the + LSP suggests for a workspace request that "can be long running", and responds only once an + edit, a watched-file change, or a config change makes something differ. This matters + because `vscode-languageclient` re-issues `workspace/diagnostic` a fixed two seconds after + every response with no client-side knob, so answering an idle pull just books another full + repo walk and lint two seconds later. A held request is still answered on + `$/cancelRequest` and at shutdown, so the client never waits on it forever. - **Disable-rule actions are offered for YAML documents only** — in Markdown the diagnostic's line is a host-file line whose embedded YAML carries a prefix, so a raw comment insert would be unreliable; Markdown documents get the fix-all action (which is diff --git a/img/benchmark-5x5-5runs.svg b/img/benchmark-5x5-5runs.svg index ee0be4b..a4b10d7 100644 --- a/img/benchmark-5x5-5runs.svg +++ b/img/benchmark-5x5-5runs.svg @@ -6,11 +6,11 @@ - 2026-06-22T15:47:10.551098 + 2026-09-06T20:03:54.922253 image/svg+xml - Matplotlib v3.11.0, https://matplotlib.org/ + Matplotlib v3.11.1, https://matplotlib.org/ @@ -42,7 +42,7 @@ z +" clip-path="url(#pac1d6da3a6)" style="fill: none; stroke: #cccccc; stroke-width: 0.8; stroke-linecap: round"/> @@ -107,7 +107,7 @@ z +" clip-path="url(#pac1d6da3a6)" style="fill: none; stroke: #cccccc; stroke-width: 0.8; stroke-linecap: round"/> @@ -156,7 +156,7 @@ z +" clip-path="url(#pac1d6da3a6)" style="fill: none; stroke: #cccccc; stroke-width: 0.8; stroke-linecap: round"/> @@ -196,7 +196,7 @@ z +" clip-path="url(#pac1d6da3a6)" style="fill: none; stroke: #cccccc; stroke-width: 0.8; stroke-linecap: round"/> @@ -237,7 +237,7 @@ z +" clip-path="url(#pac1d6da3a6)" style="fill: none; stroke: #cccccc; stroke-width: 0.8; stroke-linecap: round"/> @@ -282,7 +282,7 @@ z +" clip-path="url(#pac1d6da3a6)" style="fill: none; stroke: #cccccc; stroke-width: 0.8; stroke-linecap: round"/> @@ -312,7 +312,7 @@ z +" clip-path="url(#pac1d6da3a6)" style="fill: none; stroke: #cccccc; stroke-width: 0.8; stroke-linecap: round"/> @@ -368,7 +368,7 @@ z +" clip-path="url(#pac1d6da3a6)" style="fill: none; stroke: #cccccc; stroke-width: 0.8; stroke-linecap: round"/> @@ -415,7 +415,7 @@ z +" clip-path="url(#pac1d6da3a6)" style="fill: none; stroke: #cccccc; stroke-width: 0.8; stroke-linecap: round"/> @@ -782,14 +782,14 @@ z - + - + - + - + @@ -823,14 +823,14 @@ L 514.270137 282.823746 - + - + @@ -839,14 +839,14 @@ L 514.270137 220.151607 - + - + @@ -855,14 +855,14 @@ L 514.270137 157.479468 - + - + @@ -1064,118 +1064,118 @@ z - - - + + - - - + + - - - + + - - - + + - - - + + - + - - - - - - - + + + + + + - + - - - - - - - + + + + + + - + - - - - - - - + + + + + + - + - - - - - - - + + + + + + - + - - - - - - - + + + + + + @@ -1328,7 +1328,7 @@ L 514.270137 48.680684 " style="fill: none; stroke: #cccccc; stroke-linejoin: miter; stroke-linecap: square"/> - + - + @@ -1380,7 +1380,7 @@ z +" clip-path="url(#pa4f4a24b05)" style="fill: none; stroke: #cccccc; stroke-width: 0.8; stroke-linecap: round"/> @@ -1395,7 +1395,7 @@ L 546.530585 48.680684 +" clip-path="url(#pa4f4a24b05)" style="fill: none; stroke: #cccccc; stroke-width: 0.8; stroke-linecap: round"/> @@ -1410,7 +1410,7 @@ L 600.181706 48.680684 +" clip-path="url(#pa4f4a24b05)" style="fill: none; stroke: #cccccc; stroke-width: 0.8; stroke-linecap: round"/> @@ -1425,7 +1425,7 @@ L 653.832827 48.680684 +" clip-path="url(#pa4f4a24b05)" style="fill: none; stroke: #cccccc; stroke-width: 0.8; stroke-linecap: round"/> @@ -1440,7 +1440,7 @@ L 707.483948 48.680684 +" clip-path="url(#pa4f4a24b05)" style="fill: none; stroke: #cccccc; stroke-width: 0.8; stroke-linecap: round"/> @@ -1455,7 +1455,7 @@ L 761.135068 48.680684 +" clip-path="url(#pa4f4a24b05)" style="fill: none; stroke: #cccccc; stroke-width: 0.8; stroke-linecap: round"/> @@ -1470,7 +1470,7 @@ L 814.786189 48.680684 +" clip-path="url(#pa4f4a24b05)" style="fill: none; stroke: #cccccc; stroke-width: 0.8; stroke-linecap: round"/> @@ -1485,7 +1485,7 @@ L 868.43731 48.680684 +" clip-path="url(#pa4f4a24b05)" style="fill: none; stroke: #cccccc; stroke-width: 0.8; stroke-linecap: round"/> @@ -1500,7 +1500,7 @@ L 922.088431 48.680684 +" clip-path="url(#pa4f4a24b05)" style="fill: none; stroke: #cccccc; stroke-width: 0.8; stroke-linecap: round"/> @@ -1541,223 +1541,223 @@ L 975.739552 48.680684 - + - + - + - + - + - - - + + - - - + + - - - + + - - - + + - - - + + - - - - - - - + + + + + + + - - - - - - - + + + + + + + - - - - - - - + + + + + + + - - - - - - - + + + + + + + - - - - - - - + + + + + + + @@ -1849,7 +1849,7 @@ L 544.070137 75.845723 L 554.070137 75.845723 " style="fill: none; stroke: #a6cee4; stroke-width: 2; stroke-linecap: round"/> - + @@ -1915,7 +1915,7 @@ L 544.070137 90.230488 L 554.070137 90.230488 " style="fill: none; stroke: #6aaed6; stroke-width: 2; stroke-linecap: round"/> - + @@ -1934,7 +1934,7 @@ L 544.070137 104.615254 L 554.070137 104.615254 " style="fill: none; stroke: #3b8bc2; stroke-width: 2; stroke-linecap: round"/> - + @@ -1953,7 +1953,7 @@ L 544.070137 119.00002 L 554.070137 119.00002 " style="fill: none; stroke: #1764ab; stroke-width: 2; stroke-linecap: round"/> - + @@ -1972,7 +1972,7 @@ L 544.070137 133.384785 L 554.070137 133.384785 " style="fill: none; stroke: #083c7d; stroke-width: 2; stroke-linecap: round"/> - + @@ -2119,10 +2119,10 @@ z - + - + diff --git a/package.json b/package.json index 20961c1..56348eb 100644 --- a/package.json +++ b/package.json @@ -47,5 +47,5 @@ }, "sideEffects": false, "type": "commonjs", - "version": "0.21.0" + "version": "0.22.0" } diff --git a/pyproject.toml b/pyproject.toml index 16dddc1..f142a16 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -4,7 +4,7 @@ build-backend = "maturin" [project] name = "ryl" -version = "0.21.0" +version = "0.22.0" description = "Fast YAML linter inspired by yamllint" requires-python = ">=3.10" readme = "README.md" diff --git a/src/lsp/analysis.rs b/src/lsp/analysis.rs index 1f3efb1..a9ca6ac 100644 --- a/src/lsp/analysis.rs +++ b/src/lsp/analysis.rs @@ -4,6 +4,7 @@ use std::path::Path; use lsp_types::{Diagnostic, DiagnosticSeverity, NumberOrString, TextEdit}; +use sha2::{Digest, Sha256}; use crate::config::{SourceKind, YamlLintConfig}; use crate::fix::{ @@ -101,3 +102,18 @@ pub fn fix_rule_edit( let fixed = apply_safe_fixes_filtered(text, cfg, path, base_dir, &skip); (fixed != text).then(|| TextEdit::new(full_range(text, enc), fixed)) } + +/// A fingerprint of `items`, returned as a `resultId` and sent back on the next pull so an +/// unchanged file is answered `Unchanged`. `None` for a clean file, which the client then +/// stops tracking. +#[must_use] +pub fn result_id(items: &[Diagnostic]) -> Option { + if items.is_empty() { + return None; + } + // `Debug` over serialisation: derived, so it renders every field, and infallible. + let rendered = format!("{items:?}"); + Some(crate::report::hex_digest(&Sha256::digest( + rendered.as_bytes(), + ))) +} diff --git a/src/lsp/mod.rs b/src/lsp/mod.rs index abc6811..f632cb8 100644 --- a/src/lsp/mod.rs +++ b/src/lsp/mod.rs @@ -1,7 +1,8 @@ //! The `ryl server` language server: a synchronous protocol adapter over ryl's lint/fix //! engine, built on `lsp-server` + `lsp-types`. Malformed client input (a bad -//! `initialize`, an unknown request) is handled gracefully rather than panicking; the only -//! `expect` is on serialising ryl's own capabilities, which cannot fail. +//! `initialize`, an unknown request) is handled gracefully rather than panicking; the two +//! `expect`s are on serialising ryl's own capabilities and on a channel whose sender the +//! receiving loop owns, neither of which can fail. pub mod actions; pub mod analysis; @@ -10,12 +11,15 @@ pub mod hover; pub mod rename; use std::collections::{HashMap, HashSet}; +use std::ffi::OsStr; use std::path::{Path, PathBuf}; use std::process::ExitCode; use std::sync::Arc; use std::sync::atomic::{AtomicBool, Ordering}; use std::thread::{self, JoinHandle}; +use std::time::{Duration, Instant}; +use crossbeam_channel::{Receiver, Sender, select, unbounded}; use rayon::prelude::*; use lsp_server::{ @@ -25,18 +29,21 @@ use lsp_types::{ CancelParams, CodeActionParams, CodeActionProviderCapability, CodeActionResponse, Diagnostic, DiagnosticOptions, DiagnosticServerCapabilities, DiagnosticSeverity, DidChangeConfigurationParams, DidChangeTextDocumentParams, - DidChangeWatchedFilesRegistrationOptions, DidCloseTextDocumentParams, - DidOpenTextDocumentParams, DocumentDiagnosticParams, DocumentDiagnosticReport, - DocumentFormattingParams, FileSystemWatcher, FullDocumentDiagnosticReport, - GlobPattern, Hover, HoverParams, HoverProviderCapability, InitializeParams, - InitializeResult, MessageType, NumberOrString, OneOf, Position, - PrepareRenameResponse, PublishDiagnosticsParams, Range, Registration, - RegistrationParams, RelatedFullDocumentDiagnosticReport, RenameOptions, - RenameParams, ServerCapabilities, ServerInfo, ShowMessageParams, + DidChangeWatchedFilesParams, DidChangeWatchedFilesRegistrationOptions, + DidCloseTextDocumentParams, DidOpenTextDocumentParams, DocumentDiagnosticParams, + DocumentDiagnosticReport, DocumentFormattingParams, FileSystemWatcher, + FullDocumentDiagnosticReport, GlobPattern, Hover, HoverParams, + HoverProviderCapability, InitializeParams, InitializeResult, MessageType, + NumberOrString, OneOf, Position, PrepareRenameResponse, PreviousResultId, + ProgressToken, PublishDiagnosticsParams, Range, Registration, RegistrationParams, + RelatedFullDocumentDiagnosticReport, RelatedUnchangedDocumentDiagnosticReport, + RenameOptions, RenameParams, ServerCapabilities, ServerInfo, ShowMessageParams, TextDocumentPositionParams, TextDocumentSyncCapability, TextDocumentSyncKind, - TextEdit, Uri, WorkDoneProgressOptions, WorkspaceDiagnosticReport, - WorkspaceDocumentDiagnosticReport, WorkspaceEdit, - WorkspaceFullDocumentDiagnosticReport, + TextEdit, UnchangedDocumentDiagnosticReport, Uri, WorkDoneProgressOptions, + WorkspaceDiagnosticParams, WorkspaceDiagnosticReport, + WorkspaceDiagnosticReportPartialResult, WorkspaceDocumentDiagnosticReport, + WorkspaceEdit, WorkspaceFullDocumentDiagnosticReport, + WorkspaceUnchangedDocumentDiagnosticReport, }; use crate::config::{ConfigContext, Overrides, SourceKind, discover_config}; @@ -136,6 +143,7 @@ pub fn serve(connection: &Connection) -> SessionOutcome { register_config_watchers(connection, settings.config_file.as_deref()); } + let (scan_tx, scan_rx) = unbounded(); let server = Server { encoding, roots: workspace_roots(¶ms), @@ -153,6 +161,10 @@ pub fn serve(connection: &Connection) -> SessionOutcome { documents: HashMap::new(), reported_errors: HashSet::new(), workers: Vec::new(), + pull: None, + revision: 0, + scan_tx, + scan_rx, }; server.message_loop(connection) } @@ -245,20 +257,37 @@ fn workspace_roots(params: &InitializeParams) -> Vec { .collect() } -/// Ask the client to watch ryl's config files so an out-of-editor edit re-lints open -/// documents. Fire-and-forget (the response is ignored). +/// The config file names ryl discovers: watched so an out-of-editor edit re-lints, and +/// reused by [`Server::is_config_uri`] to tell a config change from a source one. +const WATCHED_CONFIG_NAMES: [&str; 6] = [ + "ryl.toml", + ".ryl.toml", + "pyproject.toml", + ".yamllint", + ".yamllint.yaml", + ".yamllint.yml", +]; + +/// Ask the client to watch ryl's config files and YAML sources, so an out-of-editor edit +/// re-lints open documents and wakes a long-polling pull. Fire-and-forget. /// /// Known limitation: files pulled in via a config's `extends:`, and a `configPath` changed /// after startup, are not (re-)watched; re-open a document to refresh after editing those. fn register_config_watchers(connection: &Connection, config_file: Option<&Path>) { - let mut watchers = vec![FileSystemWatcher { - glob_pattern: GlobPattern::String( - "**/{ryl.toml,.ryl.toml,pyproject.toml,.yamllint,.yamllint.yaml,\ - .yamllint.yml}" - .to_string(), - ), - kind: None, - }]; + let mut watchers = vec![ + FileSystemWatcher { + glob_pattern: GlobPattern::String(format!( + "**/{{{}}}", + WATCHED_CONFIG_NAMES.join(",") + )), + kind: None, + }, + // Else a pull suspended for long polling never learns of a `git checkout`. + FileSystemWatcher { + glob_pattern: GlobPattern::String("**/*.{yaml,yml}".to_string()), + kind: None, + }, + ]; // An explicit config path may live outside the roots or use a non-standard name, which // the `**/` glob above would miss, so watch it directly. if let Some(path) = config_file.and_then(Path::to_str) { @@ -382,18 +411,41 @@ struct Server { /// Config errors already surfaced via `window/showMessage`, so a broken config is /// reported once rather than on every file/keystroke. reported_errors: HashSet, - /// In-flight `workspace/diagnostic` scans, each on its own thread so the repo walk never - /// blocks the message loop. Each carries a cancellation flag the loop flips on - /// `$/cancelRequest` or at shutdown. + /// In-flight `workspace/diagnostic` scans, each on its own thread (so the repo walk + /// never blocks the message loop) with a flag the loop flips to cancel it. workers: Vec, + /// The client's outstanding `workspace/diagnostic` pull; at most one, since the client + /// sends the next only once this is answered. + pull: Option, + /// Bumped by every notification that can change a lint, marking older scans stale. + revision: u64, + /// Scans report back here rather than answering, so the hold-or-respond decision is + /// made where the session state lives. + scan_tx: Sender, + scan_rx: Receiver, } struct Worker { - id: RequestId, cancel: Arc, handle: JoinHandle<()>, } +/// A `workspace/diagnostic` request, being scanned or held open (LSP long polling). +struct Pull { + id: RequestId, + /// Replayed on every retry: the client's knowledge cannot move on while unanswered. + previous: Vec, + /// Present when the client offered to receive results as they are produced. + token: Option, +} + +struct ScanResult { + id: RequestId, + revision: u64, + /// `None` when the scan was cancelled. + scan: Option, +} + /// Open-document text/version snapshot, keyed by path, handed to a worker so it can prefer /// unsaved buffer content over on-disk without sharing the live document store. pub type OpenText = HashMap; @@ -401,6 +453,8 @@ pub type OpenText = HashMap; impl Server { fn message_loop(mut self, connection: &Connection) -> SessionOutcome { let outcome = self.run_loop(connection); + // Else a suspended pull is never answered, hanging a client that waits on it. + self.cancel_pull(connection); // Cancel and join in-flight scans so no thread outlives the session (each checks its // flag between files, so this returns promptly). for worker in self.workers.drain(..) { @@ -410,30 +464,44 @@ impl Server { outcome } + /// Wait on the client and on finished scans at once, so a report is decided on the + /// thread that owns the session state. fn run_loop(&mut self, connection: &Connection) -> SessionOutcome { - for message in &connection.receiver { - match message { - Message::Request(request) => { - // `Ok(true)` is a clean shutdown; an `Err` means the client vanished - // mid-handshake. Either way the session is over: end gracefully. - if connection.handle_shutdown(&request).unwrap_or(true) { + let scans = self.scan_rx.clone(); + loop { + select! { + recv(connection.receiver) -> message => { + // Connection dropped without a shutdown/exit: a normal end. + let Ok(message) = message else { return SessionOutcome::Clean; + }; + match message { + Message::Request(request) => { + // `Ok(true)` is a clean shutdown; an `Err` means the client + // vanished mid-handshake. Either way the session is over. + if connection.handle_shutdown(&request).unwrap_or(true) { + return SessionOutcome::Clean; + } + self.handle_request(connection, request); + } + Message::Notification(notification) => { + // A bare `exit` (spec-allowed without a prior `shutdown`) is an + // abnormal exit; the normal sequence is consumed above. + if notification.method == "exit" { + return SessionOutcome::Abnormal; + } + self.handle_notification(connection, notification); + } + Message::Response(_) => {} } - self.handle_request(connection, request); } - Message::Notification(notification) => { - // A bare `exit` (spec-allowed without a prior `shutdown`) is an abnormal - // exit; the normal sequence is consumed by `handle_shutdown` above. - if notification.method == "exit" { - return SessionOutcome::Abnormal; - } - self.handle_notification(connection, notification); + recv(scans) -> result => { + // `scan_tx` is a field of `self`, so it outlives this loop. + let result = result.expect("the scan channel outlives the loop"); + self.finish_scan(connection, result); } - Message::Response(_) => {} } } - // The client dropped the connection without a shutdown/exit: a normal end. - SessionOutcome::Clean } fn handle_request(&mut self, connection: &Connection, request: Request) { @@ -465,7 +533,9 @@ impl Server { .map(|params| self.document_diagnostic(¶ms)); respond(connection, id, result); } - "workspace/diagnostic" => self.spawn_workspace_diagnostic(connection, id), + "workspace/diagnostic" => { + self.start_workspace_diagnostic(connection, id, ¶ms); + } other => { send( connection, @@ -495,11 +565,13 @@ impl Server { document.version, document.text, ); + self.wake(connection); } } "textDocument/didChange" => { if let Some(params) = parse::(¶ms) { self.apply_changes(connection, params); + self.wake(connection); } } "textDocument/didClose" => { @@ -507,23 +579,39 @@ impl Server { let uri = params.text_document.uri; self.documents.remove(uri.as_str()); self.push(connection, uri, None, Vec::new()); + self.wake(connection); + } + } + "workspace/didChangeWatchedFiles" => { + // Source files are watched too, so only a config-named change moved the + // config; unparsable params keep the conservative reading. + let config_changed = parse::(¶ms) + .is_none_or(|watched| { + watched + .changes + .iter() + .any(|event| self.is_config_uri(event.uri.as_str())) + }); + if config_changed { + self.handle_config_change(connection); } + self.wake(connection); } - "workspace/didChangeWatchedFiles" => self.handle_config_change(connection), "workspace/didChangeConfiguration" => { if let Some(params) = parse::(¶ms) { self.settings = Settings::from_options(Some(¶ms.settings)); self.handle_config_change(connection); + self.wake(connection); } } "$/cancelRequest" => { - if let Some(params) = parse::(¶ms) { - let target = request_id(params.id); - for worker in &self.workers { - if worker.id == target { - worker.cancel.store(true, Ordering::Relaxed); - } - } + if let Some(params) = parse::(¶ms) + && self + .pull + .as_ref() + .is_some_and(|pull| pull.id == request_id(params.id)) + { + self.cancel_pull(connection); } } _ => {} @@ -769,13 +857,27 @@ impl Server { self.diagnostics_for(uri, &text) .unwrap_or_else(|error| vec![config_error_diagnostic(&error)]) }); - DocumentDiagnosticReport::Full(RelatedFullDocumentDiagnosticReport { - related_documents: None, - full_document_diagnostic_report: FullDocumentDiagnosticReport { - result_id: None, - items, - }, - }) + let result_id = analysis::result_id(&items); + match &result_id { + Some(id) if params.previous_result_id.as_ref() == Some(id) => { + DocumentDiagnosticReport::Unchanged( + RelatedUnchangedDocumentDiagnosticReport { + related_documents: None, + unchanged_document_diagnostic_report: + UnchangedDocumentDiagnosticReport { + result_id: id.clone(), + }, + }, + ) + } + _ => DocumentDiagnosticReport::Full(RelatedFullDocumentDiagnosticReport { + related_documents: None, + full_document_diagnostic_report: FullDocumentDiagnosticReport { + result_id, + items, + }, + }), + } } /// Snapshot open documents by path, so a worker can prefer unsaved buffer content over @@ -790,30 +892,141 @@ impl Server { .collect() } - /// Start a `workspace/diagnostic` scan on a background thread so the repo walk + parallel - /// lint never blocks the message loop. The worker answers the request over the connection. - fn spawn_workspace_diagnostic(&mut self, connection: &Connection, id: RequestId) { - // Supersede any in-flight scan (it answers RequestCancelled) so rapid pulls (e.g. on - // every save) do not accumulate concurrent walks, keeping the worker count bounded. - // Then drop the handles of any already finished. - for worker in &self.workers { - worker.cancel.store(true, Ordering::Relaxed); + /// Begin a `workspace/diagnostic` pull, closing one still outstanding (a client that + /// did not wait) with an empty report: it was told nothing, and this supersedes it. + fn start_workspace_diagnostic( + &mut self, + connection: &Connection, + id: RequestId, + params: &serde_json::Value, + ) { + if let Some(superseded) = self.pull.take() { + self.cancel_workers(); + let empty = WorkspaceDiagnosticReport { items: Vec::new() }; + respond(connection, superseded.id, empty); } + let params = parse::(params); + let token = params.as_ref().and_then(|params| { + params.partial_result_params.partial_result_token.clone() + }); + let previous = params + .map(|params| params.previous_result_ids) + .unwrap_or_default(); + let by_path = previous_by_path(&previous); + self.pull = Some(Pull { + id: id.clone(), + previous, + token: token.clone(), + }); + self.spawn_scan(connection, id, by_path, token); + } + + /// Scan for the outstanding pull on a background thread, reporting over `scan_tx`. + fn spawn_scan( + &mut self, + connection: &Connection, + id: RequestId, + previous: PreviousIds, + token: Option, + ) { + // Supersede any earlier scan (its result is dropped as stale), then reap the dead. + self.cancel_workers(); self.workers.retain(|worker| !worker.handle.is_finished()); let cancel = Arc::new(AtomicBool::new(false)); - let token = Arc::clone(&cancel); - let sender = connection.sender.clone(); + let flag = Arc::clone(&cancel); + let sink = ReportSink::new(connection.sender.clone(), token); + let results = self.scan_tx.clone(); let roots = self.roots.clone(); let settings = self.settings.clone(); let encoding = self.encoding; let open = self.open_snapshot(); - let response_id = id.clone(); + let revision = self.revision; let handle = thread::spawn(move || { - let scan = workspace_scan(&roots, &open, &settings, encoding, &token); - let _ = - sender.send(Message::Response(workspace_response(response_id, scan))); + let scan = workspace_scan( + &roots, &open, &settings, encoding, &previous, &flag, sink, + ); + let _ = results.send(ScanResult { id, revision, scan }); }); - self.workers.push(Worker { id, cancel, handle }); + self.workers.push(Worker { cancel, handle }); + } + + /// Answer the outstanding pull, or leave it open. + fn finish_scan(&mut self, connection: &Connection, result: ScanResult) { + let Some(pull) = self.pull.take() else { + return; + }; + match self.scan_answer(&pull, result) { + Some(items) => { + respond(connection, pull.id, WorkspaceDiagnosticReport { items }); + } + None => self.pull = Some(pull), + } + } + + /// The report to answer `pull` with, or `None` to leave it outstanding. A cancelled or + /// overtaken scan says nothing, and neither does an all-`Unchanged` report (an empty one + /// included) — answering that would only invite the client's fixed 2 s re-pull, so it is + /// held open and resumed by [`Self::wake`], the LSP's own suggestion for a request that + /// "can be long running and is not bound to a specific workspace or document state". + fn scan_answer( + &self, + pull: &Pull, + result: ScanResult, + ) -> Option> { + if pull.id != result.id || result.revision != self.revision { + return None; + } + result + .scan + .filter(|outcome| outcome.streamed || !outcome.says_nothing()) + .map(|outcome| outcome.items) + } + + /// Note that something a lint depends on changed, and rescan for a pull being held + /// open so the client hears about it now rather than on its own cadence. + fn wake(&mut self, connection: &Connection) { + self.revision += 1; + if let Some(pull) = &self.pull { + let id = pull.id.clone(); + let previous = previous_by_path(&pull.previous); + let token = pull.token.clone(); + self.spawn_scan(connection, id, previous, token); + } + } + + /// Answer the outstanding pull `RequestCancelled` and stop its scan; unanswered would + /// hang a client that drains its outstanding requests before exiting. + fn cancel_pull(&mut self, connection: &Connection) { + let Some(pull) = self.pull.take() else { + return; + }; + self.cancel_workers(); + send( + connection, + Message::Response(Response::new_err( + pull.id, + ErrorCode::RequestCanceled as i32, + "workspace diagnostic cancelled".to_string(), + )), + ); + } + + fn cancel_workers(&self) { + for worker in &self.workers { + worker.cancel.store(true, Ordering::Relaxed); + } + } + + /// Whether a watched-file event names a config ryl discovers, not a linted source. + fn is_config_uri(&self, uri: &str) -> bool { + let Some(path) = uri_to_path(uri) else { + return false; + }; + self.settings.config_file.as_ref() == Some(&path) + || path + .file_name() + .and_then(OsStr::to_str) + .is_some_and(|name| WATCHED_CONFIG_NAMES.contains(&name)) } fn document_text(&self, uri: &str) -> Option { @@ -938,6 +1151,23 @@ fn resolve_for_path( })) } +/// The result ids the client already holds, keyed by path (see [`previous_by_path`]). +pub type PreviousIds = HashMap; + +/// Index the client's result ids by path: the URI it echoes back need not be +/// byte-identical to ryl's (percent-encoding and drive-letter case vary), but the path it +/// decodes to is. The URI is kept to address a file the walk has lost. +#[must_use] +pub fn previous_by_path(previous: &[PreviousResultId]) -> PreviousIds { + previous + .iter() + .filter_map(|entry| { + uri_to_path(entry.uri.as_str()) + .map(|path| (path, (entry.uri.clone(), entry.value.clone()))) + }) + .collect() +} + /// Lint one workspace file for a pull report, preferring the open buffer's text. `None` /// skips a non-linted/ignored/unreadable file; a config failure becomes an error report, /// not a silent omit (a pull client would read absence as clean). @@ -946,16 +1176,15 @@ fn file_report( settings: &Settings, encoding: PositionEncoding, open: &OpenText, + previous: &PreviousIds, ) -> Option { + let previous_id = previous.get(path).map(|(_, id)| id.as_str()); let target = match resolve_for_path(path.to_path_buf(), true, true, settings) { Ok(Some(target)) => target, Ok(None) => return None, Err(error) => { - return Some(workspace_report( - path_to_uri(path), - None, - vec![config_error_diagnostic(&error)], - )); + let items = vec![config_error_diagnostic(&error)]; + return file_pull_report(path, None, items, previous_id); } }; let (text, version) = match open.get(path) { @@ -970,25 +1199,170 @@ fn file_report( target.kind, encoding, ); - Some(workspace_report(path_to_uri(path), version, items)) + file_pull_report(path, version, items, previous_id) +} + +/// One file's entry in a workspace pull: `Unchanged` while the client's result id matches, +/// a full report when it does not, an empty one to clear a file the client still holds +/// diagnostics for, and `None` for an untracked clean file — nothing to say, which is what +/// lets an idle pull suspend. +fn file_pull_report( + path: &Path, + version: Option, + items: Vec, + previous_id: Option<&str>, +) -> Option { + let uri = path_to_uri(path); + match analysis::result_id(&items) { + Some(id) if previous_id == Some(id.as_str()) => { + Some(WorkspaceDocumentDiagnosticReport::Unchanged( + WorkspaceUnchangedDocumentDiagnosticReport { + uri, + version, + unchanged_document_diagnostic_report: + UnchangedDocumentDiagnosticReport { result_id: id }, + }, + )) + } + None if previous_id.is_none() => None, + result_id => Some(workspace_report(uri, version, result_id, items)), + } +} + +/// Files linted between two cancellation checks, and the granularity at which a streaming +/// scan hands results to the client; the per-batch [`rayon`] join costs nothing beside it. +const SCAN_BATCH: usize = 64; + +/// At most one `$/progress` batch this often, so findings repaint steadily. +const STREAM_INTERVAL: Duration = Duration::from_millis(50); + +/// What a completed scan has to say; `streamed` commits the request to being answered. +pub struct ScanOutcome { + pub items: Vec, + /// Whether part of the report already went out as `$/progress` partial results. + pub streamed: bool, +} + +impl ScanOutcome { + /// Whether the report leaves the client exactly where it was, so the pull can be held. + #[must_use] + pub fn says_nothing(&self) -> bool { + !self.streamed + && self.items.iter().all(|item| { + matches!(item, WorkspaceDocumentDiagnosticReport::Unchanged(_)) + }) + } +} + +/// Where a scan's reports go: held for the response, or — given a partial-result token — +/// streamed as `$/progress` batches as they are produced, leaving only the `Unchanged` +/// remainder to answer with, as a streamed report must not be repeated. +pub struct ReportSink { + stream: Option, + held: Vec, + streamed: bool, +} + +struct Stream { + client: Sender, + token: ProgressToken, + /// `None` until the first batch, which goes out at once so something paints early. + last_flush: Option, + pending: Vec, +} + +impl ReportSink { + /// Holds everything for the response, for a client that offered no token. + #[must_use] + pub fn bulk() -> Self { + Self { + stream: None, + held: Vec::new(), + streamed: false, + } + } + + fn new(client: Sender, token: Option) -> Self { + Self { + stream: token.map(|token| Stream { + client, + token, + last_flush: None, + pending: Vec::new(), + }), + held: Vec::new(), + streamed: false, + } + } + + /// Route one file's report; `Unchanged` is always held, streaming it says nothing. + fn push(&mut self, report: WorkspaceDocumentDiagnosticReport) { + match (&mut self.stream, &report) { + (Some(stream), WorkspaceDocumentDiagnosticReport::Full(_)) => { + stream.pending.push(report); + } + _ => self.held.push(report), + } + } + + /// Send what has accumulated, if enough time has passed since the last batch. + fn flush_batch(&mut self) { + if let Some(stream) = &mut self.stream + && !stream.pending.is_empty() + && stream + .last_flush + .is_none_or(|at| at.elapsed() >= STREAM_INTERVAL) + { + stream.send(); + self.streamed = true; + } + } + + fn finish(mut self) -> ScanOutcome { + if let Some(stream) = &mut self.stream + && !stream.pending.is_empty() + { + stream.send(); + self.streamed = true; + } + ScanOutcome { + items: self.held, + streamed: self.streamed, + } + } +} + +impl Stream { + /// A `$/progress` carrying the batch. The spec asks for a `WorkspaceDiagnosticReport` + /// first and partial results after, but the two share a wire shape, so one form serves. + fn send(&mut self) { + let partial = WorkspaceDiagnosticReportPartialResult { + items: std::mem::take(&mut self.pending), + }; + let params = serde_json::json!({ "token": self.token, "value": partial }); + let _ = self.client.send(Message::Notification(Notification::new( + "$/progress".to_string(), + params, + ))); + self.last_flush = Some(Instant::now()); + } } /// The `workspace/diagnostic` scan: enumerate `*.yaml`/`*.yml` under each root (git-ignore -/// honoured), de-duplicate across roots, then lint them in parallel ([`rayon`]). `None` when -/// `cancel` is set, so the worker answers with `RequestCancelled`. `pub` for unit testing. +/// honoured), de-duplicate across roots, then lint them in [`rayon`]-parallel batches routed +/// through `sink`. `None` when `cancel` is set, answering the pull `RequestCancelled`. pub fn workspace_scan( roots: &[PathBuf], open: &OpenText, settings: &Settings, encoding: PositionEncoding, + previous: &PreviousIds, cancel: &AtomicBool, -) -> Option> { + mut sink: ReportSink, +) -> Option { let mut files = Vec::new(); let mut seen = HashSet::new(); for root in roots { - // The walk is per-entry cancellable (`?` propagates cancellation); the lint pass - // below is not separately interrupted. The residual mid-`read_file` window is per - // ryl's threat model (realistic payloads, not a degraded-fs racer). for path in gather_yaml_from_dir_cancellable(root, cancel)? { // De-duplicate so a file reachable from two (e.g. nested) roots is linted once. if seen.insert(path.clone()) { @@ -996,29 +1370,34 @@ pub fn workspace_scan( } } } - Some( - files + let mut covered: HashSet<&PathBuf> = HashSet::new(); + for batch in files.chunks(SCAN_BATCH) { + if cancel.load(Ordering::Relaxed) { + return None; + } + let reports: Vec<(&PathBuf, WorkspaceDocumentDiagnosticReport)> = batch .par_iter() - .filter_map(|path| file_report(path, settings, encoding, open)) - .collect(), - ) -} - -/// The `workspace/diagnostic` response: the report, or a `RequestCancelled` error when the -/// scan was cancelled. `pub` for unit testing. -#[must_use] -pub fn workspace_response( - id: RequestId, - scan: Option>, -) -> Response { - match scan { - Some(items) => Response::new_ok(id, WorkspaceDiagnosticReport { items }), - None => Response::new_err( - id, - ErrorCode::RequestCanceled as i32, - "workspace diagnostic cancelled".to_string(), - ), + .filter_map(|path| { + file_report(path, settings, encoding, open, previous) + .map(|report| (path, report)) + }) + .collect(); + for (path, report) in reports { + covered.insert(path); + sink.push(report); + } + sink.flush_batch(); + } + // A path the client holds an id for that the walk no longer reports was deleted, + // renamed, or newly ignored: an empty, id-less report clears it and stops the echo. + for (uri, _) in previous + .iter() + .filter(|(path, _)| !covered.contains(path)) + .map(|(_, entry)| entry) + { + sink.push(workspace_report(uri.clone(), None, None, Vec::new())); } + Some(sink.finish()) } fn request_id(id: NumberOrString) -> RequestId { @@ -1068,13 +1447,14 @@ fn config_error_diagnostic(error: &str) -> Diagnostic { fn workspace_report( uri: Uri, version: Option, + result_id: Option, items: Vec, ) -> WorkspaceDocumentDiagnosticReport { WorkspaceDocumentDiagnosticReport::Full(WorkspaceFullDocumentDiagnosticReport { uri, version, full_document_diagnostic_report: FullDocumentDiagnosticReport { - result_id: None, + result_id, items, }, }) diff --git a/src/report.rs b/src/report.rs index 8eb6325..8b430f6 100644 --- a/src/report.rs +++ b/src/report.rs @@ -306,7 +306,12 @@ fn fingerprint(path: &str, check_name: &str, message: &str, salt: u64) -> String hasher.update(check_name.as_bytes()); hasher.update([0u8]); hasher.update(message.as_bytes()); - let digest = hasher.finalize(); + hex_digest(&hasher.finalize()) +} + +/// Lower-case hex of a digest, for the identifiers callers hand to other tools (GitLab's +/// fingerprint, an LSP `resultId`). +pub(crate) fn hex_digest(digest: &[u8]) -> String { let mut out = String::with_capacity(digest.len() * 2); for byte in digest { write!(out, "{byte:02x}").expect("writing to a String is infallible"); diff --git a/tests/lsp_server.rs b/tests/lsp_server.rs index 93e4089..c3b01a1 100644 --- a/tests/lsp_server.rs +++ b/tests/lsp_server.rs @@ -9,8 +9,11 @@ use std::path::Path; use std::process::{Child, Command, Stdio}; use std::str::FromStr; use std::thread::{self, JoinHandle}; +use std::time::{Duration, Instant}; -use lsp_server::{Connection, Message, Notification, Request, RequestId, Response}; +use lsp_server::{ + Connection, ErrorCode, Message, Notification, Request, RequestId, Response, +}; use lsp_types::{ ClientCapabilities, CodeActionContext, CodeActionKind, CodeActionOrCommand, CodeActionParams, CodeActionResponse, Diagnostic, DiagnosticClientCapabilities, @@ -24,8 +27,8 @@ use lsp_types::{ TextDocumentIdentifier, TextDocumentItem, TextEdit, Uri, VersionedTextDocumentIdentifier, WorkDoneProgressParams, WorkspaceClientCapabilities, WorkspaceDiagnosticReport, - WorkspaceDocumentDiagnosticReport, WorkspaceEdit, WorkspaceEditClientCapabilities, - WorkspaceFolder, + WorkspaceDiagnosticReportPartialResult, WorkspaceDocumentDiagnosticReport, + WorkspaceEdit, WorkspaceEditClientCapabilities, WorkspaceFolder, }; use serde_json::{Value, json}; use tempfile::{TempDir, tempdir}; @@ -36,6 +39,35 @@ const TRAILING: &str = "[rules]\ntrailing-spaces = \"enable\"\n"; /// `MethodNotFound` error, used to probe that the server is still responsive. const UNHANDLED_METHOD: &str = "ryl/internalUnhandledProbe"; +/// How long a suspended `workspace/diagnostic` must stay unanswered before a test accepts +/// that the server is long-polling it rather than merely slow. +const SUSPEND_GRACE: Duration = Duration::from_millis(500); + +/// The `didChangeWatchedFiles` payload a client sends when the project's config changed +/// (`type: 2` is the spec's `Changed`). +fn changed_config(dir: &Path) -> Value { + json!({ "changes": [{ "uri": file_uri(dir, ".ryl.toml"), "type": 2 }] }) +} + +/// What the client would send back as `previousResultIds` after `report`. +fn previous_result_ids(report: &WorkspaceDiagnosticReport) -> Vec { + report + .items + .iter() + .filter_map(|item| match item { + WorkspaceDocumentDiagnosticReport::Full(full) => full + .full_document_diagnostic_report + .result_id + .as_ref() + .map(|value| json!({ "uri": full.uri, "value": value })), + WorkspaceDocumentDiagnosticReport::Unchanged(unchanged) => Some(json!({ + "uri": unchanged.uri, + "value": unchanged.unchanged_document_diagnostic_report.result_id, + })), + }) + .collect() +} + fn uri(text: &str) -> Uri { Uri::from_str(text).expect("valid URI") } @@ -274,6 +306,23 @@ impl Client { } } + /// The response to `id` if one arrives within [`SUSPEND_GRACE`]. A `workspace/diagnostic` + /// pull with nothing to report is deliberately held open, so its absence can only be + /// asserted by waiting. + fn response_within(&self, id: &RequestId) -> Option { + let deadline = Instant::now() + SUSPEND_GRACE; + loop { + let remaining = deadline.saturating_duration_since(Instant::now()); + match self.conn().receiver.recv_timeout(remaining) { + Ok(Message::Response(response)) if &response.id == id => { + return Some(response); + } + Ok(_) => {} + Err(_) => return None, + } + } + } + /// Drain messages up to and including the response to `id`, returning every /// notification seen on the way. Because the server's loop is single-threaded and /// in-order, any push triggered by an earlier notification arrives before this @@ -578,6 +627,20 @@ impl Client { } /// Receive the next server-to-client request, skipping notifications/responses. + /// Block until the first `$/progress` carrying `token`, so a test can act on a scan + /// that is provably past its directory walk. + fn recv_progress(&self, token: &str) { + loop { + if let Message::Notification(note) = + self.conn().receiver.recv().expect("recv") + && note.method == "$/progress" + && note.params["token"] == token + { + return; + } + } + } + fn recv_request(&self) -> Request { loop { if let Message::Request(request) = @@ -784,7 +847,10 @@ fn pull_client_is_asked_to_refresh_after_config_change() { let (mut client, _init) = Client::launch_pull(Some(dir.path()), true); client.did_open(file_uri(dir.path(), "x.yaml"), "a: 1 \n"); // Simulate the watched config file changing on disk. - client.notify("workspace/didChangeWatchedFiles", json!({ "changes": [] })); + client.notify( + "workspace/didChangeWatchedFiles", + changed_config(dir.path()), + ); // Flush with a probe; any refresh request / stray push arrives before its response. let id = client.request(UNHANDLED_METHOD, Value::Null); let (messages, response) = client.messages_until_response(&id); @@ -819,8 +885,14 @@ fn repeated_config_changes_send_distinct_refresh_request_ids() { client.did_open(file_uri(dir.path(), "x.yaml"), "a: 1 \n"); // Two changes before any response: their refresh requests are concurrently outstanding, // so reusing one id would break the client's response correlation (JSON-RPC). - client.notify("workspace/didChangeWatchedFiles", json!({ "changes": [] })); - client.notify("workspace/didChangeWatchedFiles", json!({ "changes": [] })); + client.notify( + "workspace/didChangeWatchedFiles", + changed_config(dir.path()), + ); + client.notify( + "workspace/didChangeWatchedFiles", + changed_config(dir.path()), + ); let id = client.request(UNHANDLED_METHOD, Value::Null); let (messages, response) = client.messages_until_response(&id); assert!( @@ -854,7 +926,10 @@ fn pull_client_without_refresh_support_gets_no_refresh() { let dir = project(TRAILING); let (mut client, _init) = Client::launch_pull(Some(dir.path()), false); client.did_open(file_uri(dir.path(), "x.yaml"), "a: 1 \n"); - client.notify("workspace/didChangeWatchedFiles", json!({ "changes": [] })); + client.notify( + "workspace/didChangeWatchedFiles", + changed_config(dir.path()), + ); let id = client.request(UNHANDLED_METHOD, Value::Null); let (messages, response) = client.messages_until_response(&id); assert!( @@ -1450,7 +1525,10 @@ fn watched_file_change_relints_open_documents() { ); // Swap in a config that enables trailing-spaces, then signal the watcher. std::fs::write(dir.path().join(".ryl.toml"), TRAILING).expect("rewrite config"); - client.notify("workspace/didChangeWatchedFiles", json!({ "changes": [] })); + client.notify( + "workspace/didChangeWatchedFiles", + changed_config(dir.path()), + ); assert_eq!( client.diagnostics().len(), 1, @@ -1934,6 +2012,71 @@ fn workspace_diagnostic_can_be_cancelled() { ); } +/// Enough files that the walk is still running when the next client message is handled, +/// so a scan is reliably in flight for the cancel/supersede races below. +fn slow_workspace() -> TempDir { + let dir = project(TRAILING); + for index in 0..2000 { + std::fs::write(dir.path().join(format!("f{index}.yaml")), "a: 1 \n") + .expect("write"); + } + dir +} + +#[test] +fn cancelling_an_in_flight_scan_discards_its_report() { + let dir = slow_workspace(); + let (mut client, _init) = Client::launch_pull(Some(dir.path()), false); + let id = client.request("workspace/diagnostic", json!({ "previousResultIds": [] })); + client.notify("$/cancelRequest", json!({ "id": client.next_id })); + assert_eq!( + client + .response(&id) + .response_result + .expect_err("a cancelled pull is an error response") + .code, + ErrorCode::RequestCanceled as i32, + "the cancel answers the pull; the scan's own report arrives afterwards with \ + nothing left to answer and is dropped" + ); + // Stay connected while the cancelled scan's report reaches the loop: dropping the + // client first would end the session before it is seen (and dropped). + thread::sleep(Duration::from_millis(200)); + client.assert_alive(); +} + +#[test] +fn a_new_pull_supersedes_an_in_flight_scan() { + let dir = slow_workspace(); + let (mut client, _init) = Client::launch_pull(Some(dir.path()), false); + let first = + client.request("workspace/diagnostic", json!({ "previousResultIds": [] })); + // No wait: the second pull lands while the first is still walking, so the first's + // report reaches the loop belonging to a pull that has moved on. + let second = + client.request("workspace/diagnostic", json!({ "previousResultIds": [] })); + let superseded: WorkspaceDiagnosticReport = serde_json::from_value( + client + .response(&first) + .response_result + .expect("the superseded pull is answered"), + ) + .expect("WorkspaceDiagnosticReport"); + assert!( + superseded.items.is_empty(), + "a superseded pull is closed with an empty report" + ); + assert!( + !serde_json::from_value::( + client.response(&second).response_result.expect("report"), + ) + .expect("WorkspaceDiagnosticReport") + .items + .is_empty(), + "the replacement pull still reports the flagged files" + ); +} + #[test] fn cancel_request_for_unknown_or_malformed_id_is_ignored() { let dir = project(TRAILING); @@ -1951,7 +2094,9 @@ fn cancel_request_for_unknown_or_malformed_id_is_ignored() { #[test] fn workspace_diagnostic_reaps_finished_workers() { let dir = project(TRAILING); - std::fs::write(dir.path().join("a.yaml"), "a: 1\n").expect("write"); + // A flagged file, so each pull has something to report and is answered rather than + // held open for long polling. + std::fs::write(dir.path().join("a.yaml"), "a: 1 \n").expect("write"); let (mut client, _init) = Client::launch_with(None, Some(dir.path()), true, None, false, None); // Two sequential pulls: the second's spawn reaps the first's finished worker. @@ -1961,11 +2106,306 @@ fn workspace_diagnostic_reaps_finished_workers() { } #[test] -fn workspace_diagnostic_without_a_root_is_empty() { +fn workspace_diagnostic_without_a_root_is_held_open() { let (mut client, _init) = Client::launch(None, None); + let id = client.request("workspace/diagnostic", json!({ "previousResultIds": [] })); + assert!( + client.response_within(&id).is_none(), + "without a root there is nothing to say, so the pull is held open rather than \ + answered into the client's fixed re-pull loop" + ); + client.assert_alive(); +} + +#[test] +fn a_malformed_watched_files_payload_still_relints() { + let dir = project("[rules]\nkey-duplicates = \"enable\"\n"); + let (client, _init) = + Client::launch_with(None, Some(dir.path()), true, None, true, None); + assert_eq!( + client.recv_request().method, + "client/registerCapability", + "the watcher is registered first" + ); + client.did_open(file_uri(dir.path(), "x.yaml"), "a: 1 \n"); + assert!( + client.diagnostics().is_empty(), + "clean under the initial config" + ); + std::fs::write(dir.path().join(".ryl.toml"), TRAILING).expect("rewrite config"); + // Params ryl cannot read say nothing about *what* changed, so the config is assumed + // to have: a needless re-lint beats leaving stale diagnostics on screen. + client.notify( + "workspace/didChangeWatchedFiles", + json!("not the params shape"), + ); + assert_eq!( + client.diagnostics().len(), + 1, + "an unreadable payload is still treated as a config change" + ); +} + +#[test] +fn an_unchanged_workspace_pull_is_held_open_until_a_change() { + let dir = project(TRAILING); + std::fs::write(dir.path().join("bad.yaml"), "a: 1 \n").expect("write"); + let (mut client, _init) = Client::launch_pull(Some(dir.path()), false); + let previous = previous_result_ids(&client.workspace_diagnostic()); + assert_eq!(previous.len(), 1, "the flagged file carries a result id"); + + let id = client.request( + "workspace/diagnostic", + json!({ "previousResultIds": previous }), + ); assert!( - client.workspace_diagnostic().items.is_empty(), - "without a workspace root there is nothing to scan" + client.response_within(&id).is_none(), + "every report would be Unchanged, so the pull is held open" + ); + + client.did_open(file_uri(dir.path(), "bad.yaml"), "a: 1 \nb: 2 \n"); + let report: WorkspaceDiagnosticReport = serde_json::from_value( + client + .response(&id) + .response_result + .expect("the held pull is answered once the buffer changes"), + ) + .expect("WorkspaceDiagnosticReport"); + assert!( + matches!( + report.items.as_slice(), + [WorkspaceDocumentDiagnosticReport::Full(full)] + if !full.full_document_diagnostic_report.items.is_empty() + ), + "the resumed pull carries the new diagnostics, got {:?}", + report.items + ); +} + +#[test] +fn a_watched_yaml_change_wakes_a_suspended_pull() { + let dir = project(TRAILING); + let path = dir.path().join("bad.yaml"); + std::fs::write(&path, "a: 1 \n").expect("write"); + let (mut client, _init) = Client::launch_pull(Some(dir.path()), false); + let previous = previous_result_ids(&client.workspace_diagnostic()); + let id = client.request( + "workspace/diagnostic", + json!({ "previousResultIds": previous }), + ); + assert!( + client.response_within(&id).is_none(), + "held open while idle" + ); + + // An edit from outside the editor: without the source-file watcher a long-polling pull + // would never learn of it. + std::fs::write(&path, "a: 1\n").expect("fixed"); + client.notify( + "workspace/didChangeWatchedFiles", + json!({ "changes": [ + { "uri": uri("untitled:scratch"), "type": 2 }, + { "uri": file_uri(dir.path(), "bad.yaml"), "type": 2 }, + ] }), + ); + assert!( + client.response(&id).response_result.is_ok(), + "an out-of-editor YAML change resumes the pull" + ); +} + +#[test] +fn a_suspended_pull_is_answered_on_cancel_and_on_shutdown() { + let dir = project(TRAILING); + let (mut client, _init) = Client::launch_pull(Some(dir.path()), false); + let cancelled = + client.request("workspace/diagnostic", json!({ "previousResultIds": [] })); + assert!(client.response_within(&cancelled).is_none(), "held open"); + client.notify("$/cancelRequest", json!({ "id": client.next_id })); + assert_eq!( + client + .response(&cancelled) + .response_result + .expect_err("a cancelled pull is an error response") + .code, + ErrorCode::RequestCanceled as i32, + "cancelling a held pull answers it rather than leaving the client waiting" + ); + + let outstanding = + client.request("workspace/diagnostic", json!({ "previousResultIds": [] })); + assert!(client.response_within(&outstanding).is_none(), "held open"); + client.shutdown(); + assert_eq!( + client.response(&outstanding).id, + outstanding, + "shutdown answers a still-held pull instead of hanging the client" + ); +} + +#[test] +fn a_new_workspace_pull_supersedes_a_held_one() { + let dir = project(TRAILING); + let (mut client, _init) = Client::launch_pull(Some(dir.path()), false); + let first = + client.request("workspace/diagnostic", json!({ "previousResultIds": [] })); + assert!(client.response_within(&first).is_none(), "held open"); + let second = + client.request("workspace/diagnostic", json!({ "previousResultIds": [] })); + let report: WorkspaceDiagnosticReport = serde_json::from_value( + client + .response(&first) + .response_result + .expect("the superseded pull is answered"), + ) + .expect("WorkspaceDiagnosticReport"); + assert!( + report.items.is_empty(), + "a superseded pull is closed with an empty report, having been told nothing" + ); + assert!( + client.response_within(&second).is_none(), + "the replacement pull is itself held open" + ); +} + +/// The `$/progress` payloads carrying `token`, drained alongside the response to `id`. +fn partial_results( + client: &Client, + id: &RequestId, + token: &str, +) -> (Vec>, Response) { + let (messages, response) = client.messages_until_response(id); + let batches = messages + .iter() + .filter_map(|message| match message { + Message::Notification(note) if note.method == "$/progress" => { + (note.params["token"] == token).then(|| { + serde_json::from_value::( + note.params["value"].clone(), + ) + .expect("WorkspaceDiagnosticReportPartialResult") + .items + }) + } + _ => None, + }) + .collect(); + (batches, response) +} + +#[test] +fn a_partial_result_token_streams_changed_reports_as_they_are_produced() { + let dir = slow_workspace(); + let (mut client, _init) = Client::launch_pull(Some(dir.path()), false); + let id = client.request( + "workspace/diagnostic", + json!({ "previousResultIds": [], "partialResultToken": "ryl-pull" }), + ); + let (batches, response) = partial_results(&client, &id, "ryl-pull"); + let streamed: usize = batches.iter().map(Vec::len).sum(); + assert!( + batches.len() > 1, + "a workspace this size is delivered over several batches, not one: {}", + batches.len() + ); + assert_eq!( + streamed, 2000, + "every flagged file is streamed exactly once" + ); + assert!( + serde_json::from_value::( + response.response_result.expect("the pull is answered"), + ) + .expect("WorkspaceDiagnosticReport") + .items + .is_empty(), + "a streamed report is not repeated in the response" + ); +} + +#[test] +fn cancelling_a_streaming_scan_stops_it_between_batches() { + let dir = slow_workspace(); + let (mut client, _init) = Client::launch_pull(Some(dir.path()), false); + let id = client.request( + "workspace/diagnostic", + json!({ "previousResultIds": [], "partialResultToken": "ryl-pull" }), + ); + // A streamed batch proves the walk is done and the lint is under way, so the cancel + // lands on the per-batch check rather than on the directory walk. + client.recv_progress("ryl-pull"); + client.notify("$/cancelRequest", json!({ "id": client.next_id })); + assert_eq!( + client + .response(&id) + .response_result + .expect_err("a cancelled pull is an error response") + .code, + ErrorCode::RequestCanceled as i32, + "the pull is answered without linting the rest of the workspace" + ); + client.assert_alive(); +} + +#[test] +fn a_streaming_pull_with_nothing_to_say_is_still_held_open() { + let dir = project(TRAILING); + std::fs::write(dir.path().join("bad.yaml"), "a: 1 \n").expect("write"); + let (mut client, _init) = Client::launch_pull(Some(dir.path()), false); + let first = client.request( + "workspace/diagnostic", + json!({ "previousResultIds": [], "partialResultToken": "ryl-pull" }), + ); + let (batches, _) = partial_results(&client, &first, "ryl-pull"); + let previous = previous_result_ids(&WorkspaceDiagnosticReport { + items: batches.concat(), + }); + assert_eq!( + previous.len(), + 1, + "the flagged file was streamed with an id" + ); + + let id = client.request( + "workspace/diagnostic", + json!({ "previousResultIds": previous, "partialResultToken": "ryl-pull" }), + ); + assert!( + client.response_within(&id).is_none(), + "nothing changed, so nothing is streamed and the pull is held open as usual" + ); + client.assert_alive(); +} + +#[test] +fn document_diagnostic_reports_unchanged_for_a_matching_result_id() { + let dir = project(TRAILING); + let (mut client, _init) = Client::launch_pull(Some(dir.path()), false); + let uri = file_uri(dir.path(), "a.yaml"); + client.did_open(uri.clone(), "a: 1 \n"); + let DocumentDiagnosticReport::Full(full) = client.document_diagnostic(&uri) else { + panic!("a flagged document yields a full report"); + }; + let result_id = full + .full_document_diagnostic_report + .result_id + .expect("a flagged document carries a result id"); + let id = client.request( + "textDocument/diagnostic", + json!({ "textDocument": { "uri": uri }, "previousResultId": result_id }), + ); + let report: DocumentDiagnosticReport = serde_json::from_value( + client.response(&id).response_result.expect("diagnostic"), + ) + .expect("DocumentDiagnosticReport"); + assert!( + matches!( + report, + DocumentDiagnosticReport::Unchanged(unchanged) + if unchanged.unchanged_document_diagnostic_report.result_id == result_id + ), + "an unedited document is answered Unchanged rather than re-sending every item" ); } diff --git a/tests/lsp_unit.rs b/tests/lsp_unit.rs index a44f7f1..7e9944a 100644 --- a/tests/lsp_unit.rs +++ b/tests/lsp_unit.rs @@ -7,10 +7,9 @@ use std::path::Path; use std::sync::atomic::AtomicBool; -use lsp_server::{ErrorCode, RequestId}; use lsp_types::{ Diagnostic, NumberOrString, Position, PositionEncodingKind, PrepareRenameResponse, - Range, + PreviousResultId, Range, Uri, WorkspaceDocumentDiagnosticReport, }; use tempfile::tempdir; @@ -22,7 +21,9 @@ use ryl::lsp::encoding::{ }; use ryl::lsp::hover::hover; use ryl::lsp::rename::{prepare_rename, rename_edits}; -use ryl::lsp::{OpenText, Settings, workspace_response, workspace_scan}; +use ryl::lsp::{ + OpenText, PreviousIds, ReportSink, Settings, previous_by_path, workspace_scan, +}; #[test] fn negotiate_prefers_clients_first_supported_kind() { @@ -765,7 +766,7 @@ fn rename_edits_reject_a_name_that_collides_with_another_anchor() { ); } -// --- workspace_scan / workspace_response: the background pull's pure core --- +// --- workspace_scan / previous_by_path: the background pull's pure core --- fn workspace_project() -> tempfile::TempDir { // An adjacent .ryl.toml shields config discovery from the walk (no HOME needed). @@ -778,21 +779,125 @@ fn workspace_project() -> tempfile::TempDir { dir } -#[test] -fn workspace_scan_lints_each_root_file() { - let dir = workspace_project(); - std::fs::write(dir.path().join("bad.yaml"), "a: 1 \n").expect("yaml"); - let reports = workspace_scan( - &[dir.path().to_path_buf()], +fn scan(dir: &Path, previous: &PreviousIds) -> Vec { + workspace_scan( + &[dir.to_path_buf()], &OpenText::new(), &Settings::default(), PositionEncoding::Utf16, + previous, &AtomicBool::new(false), + ReportSink::bulk(), ) - .expect("an uncancelled scan returns reports"); + .expect("an uncancelled scan returns reports") + .items +} + +fn only_result_id(reports: &[WorkspaceDocumentDiagnosticReport]) -> (Uri, String) { + match reports { + [WorkspaceDocumentDiagnosticReport::Full(full)] => ( + full.uri.clone(), + full.full_document_diagnostic_report + .result_id + .clone() + .expect("a flagged file carries a result id"), + ), + other => panic!("expected one full report, got {other:?}"), + } +} + +#[test] +fn workspace_scan_lints_each_root_file() { + let dir = workspace_project(); + std::fs::write(dir.path().join("bad.yaml"), "a: 1 \n").expect("yaml"); + let reports = scan(dir.path(), &PreviousIds::new()); assert!(!reports.is_empty(), "the flagged file is reported"); } +#[test] +fn workspace_scan_omits_a_clean_untracked_file() { + let dir = workspace_project(); + std::fs::write(dir.path().join("good.yaml"), "a: 1\n").expect("yaml"); + assert!( + scan(dir.path(), &PreviousIds::new()).is_empty(), + "a clean file the client is not tracking has nothing to report, which is what \ + lets an idle pull suspend" + ); +} + +#[test] +fn workspace_scan_reports_unchanged_for_a_matching_result_id() { + let dir = workspace_project(); + std::fs::write(dir.path().join("bad.yaml"), "a: 1 \n").expect("yaml"); + let (uri, result_id) = only_result_id(&scan(dir.path(), &PreviousIds::new())); + let previous = previous_by_path(&[PreviousResultId { + uri, + value: result_id.clone(), + }]); + match scan(dir.path(), &previous).as_slice() { + [WorkspaceDocumentDiagnosticReport::Unchanged(unchanged)] => assert_eq!( + unchanged.unchanged_document_diagnostic_report.result_id, result_id, + "an unchanged file echoes the id the client already holds" + ), + other => panic!("expected one unchanged report, got {other:?}"), + } +} + +#[test] +fn workspace_scan_clears_a_tracked_file_that_became_clean() { + let dir = workspace_project(); + let path = dir.path().join("bad.yaml"); + std::fs::write(&path, "a: 1 \n").expect("yaml"); + let (uri, result_id) = only_result_id(&scan(dir.path(), &PreviousIds::new())); + std::fs::write(&path, "a: 1\n").expect("fixed"); + let previous = previous_by_path(&[PreviousResultId { + uri, + value: result_id, + }]); + match scan(dir.path(), &previous).as_slice() { + [WorkspaceDocumentDiagnosticReport::Full(full)] => { + assert!( + full.full_document_diagnostic_report.items.is_empty() + && full.full_document_diagnostic_report.result_id.is_none(), + "a fixed file is cleared with an empty, untracked report" + ); + } + other => panic!("expected one clearing report, got {other:?}"), + } +} + +#[test] +fn workspace_scan_clears_a_tracked_file_that_vanished() { + let dir = workspace_project(); + let path = dir.path().join("bad.yaml"); + std::fs::write(&path, "a: 1 \n").expect("yaml"); + let (uri, result_id) = only_result_id(&scan(dir.path(), &PreviousIds::new())); + std::fs::remove_file(&path).expect("delete"); + let previous = previous_by_path(&[PreviousResultId { + uri: uri.clone(), + value: result_id, + }]); + match scan(dir.path(), &previous).as_slice() { + [WorkspaceDocumentDiagnosticReport::Full(full)] => assert_eq!( + full.uri, uri, + "a deleted file the client still tracks is cleared by URI" + ), + other => panic!("expected one clearing report, got {other:?}"), + } +} + +#[test] +fn previous_by_path_skips_a_non_file_uri() { + assert!( + previous_by_path(&[PreviousResultId { + uri: "untitled:scratch".parse::().expect("untitled URI"), + value: "id".to_string(), + }]) + .is_empty(), + "an untitled buffer has no path to key a workspace result id on" + ); +} + #[test] fn workspace_scan_returns_none_when_cancelled() { let dir = workspace_project(); @@ -804,27 +909,11 @@ fn workspace_scan_returns_none_when_cancelled() { &OpenText::new(), &Settings::default(), PositionEncoding::Utf16, + &PreviousIds::new(), &AtomicBool::new(true), + ReportSink::bulk(), ) .is_none(), "a cancelled scan yields no report" ); } - -#[test] -fn workspace_response_is_ok_or_cancelled() { - let ok = workspace_response(RequestId::from(1), Some(Vec::new())); - assert!( - ok.response_result.is_ok(), - "a completed scan is an ok response" - ); - let cancelled = workspace_response(RequestId::from(2), None); - assert_eq!( - cancelled - .response_result - .expect_err("a cancelled scan is an error") - .code, - ErrorCode::RequestCanceled as i32, - "cancellation maps to the RequestCancelled code" - ); -} diff --git a/uv.lock b/uv.lock index a855dae..7717567 100644 --- a/uv.lock +++ b/uv.lock @@ -226,7 +226,7 @@ wheels = [ [[package]] name = "ryl" -version = "0.21.0" +version = "0.22.0" source = { editable = "." } [package.dev-dependencies] From d2e77eb49f8d4b646a9543423761e87380ec65a4 Mon Sep 17 00:00:00 2001 From: Owen Lamont Date: Sun, 6 Sep 2026 20:18:59 +0800 Subject: [PATCH 2/3] refactor(lsp): drop a redundant streamed check and pin the final flush `says_nothing` already returns false once anything has streamed, so the extra `streamed ||` in `scan_answer` read as an independent condition it was not. The sink's final flush only ran when a batch happened not to have flushed on the timer, so a deleted-file test now pins it deterministically. Claude-Session: https://claude.ai/code/session_01L2XE37GUJWcHp7hPKMdHU8 --- src/lsp/mod.rs | 2 +- tests/lsp_server.rs | 37 +++++++++++++++++++++++++++++++++++++ 2 files changed, 38 insertions(+), 1 deletion(-) diff --git a/src/lsp/mod.rs b/src/lsp/mod.rs index f632cb8..28c9847 100644 --- a/src/lsp/mod.rs +++ b/src/lsp/mod.rs @@ -978,7 +978,7 @@ impl Server { } result .scan - .filter(|outcome| outcome.streamed || !outcome.says_nothing()) + .filter(|outcome| !outcome.says_nothing()) .map(|outcome| outcome.items) } diff --git a/tests/lsp_server.rs b/tests/lsp_server.rs index c3b01a1..63db2cf 100644 --- a/tests/lsp_server.rs +++ b/tests/lsp_server.rs @@ -2348,6 +2348,43 @@ fn cancelling_a_streaming_scan_stops_it_between_batches() { client.assert_alive(); } +#[test] +fn a_streaming_pull_clears_a_deleted_file_through_the_stream() { + let dir = project(TRAILING); + let path = dir.path().join("bad.yaml"); + std::fs::write(&path, "a: 1 \n").expect("write"); + let (mut client, _init) = Client::launch_pull(Some(dir.path()), false); + let first = client.request( + "workspace/diagnostic", + json!({ "previousResultIds": [], "partialResultToken": "ryl-pull" }), + ); + let (batches, _) = partial_results(&client, &first, "ryl-pull"); + let previous = previous_result_ids(&WorkspaceDiagnosticReport { + items: batches.concat(), + }); + + // A clearing report is produced after the last batch of the walk, so it is the final + // flush rather than a batched one that carries it. + std::fs::remove_file(&path).expect("delete"); + let id = client.request( + "workspace/diagnostic", + json!({ "previousResultIds": previous, "partialResultToken": "ryl-pull" }), + ); + let (batches, response) = partial_results(&client, &id, "ryl-pull"); + assert!( + matches!( + batches.concat().as_slice(), + [WorkspaceDocumentDiagnosticReport::Full(full)] + if full.full_document_diagnostic_report.items.is_empty() + ), + "the deleted file is cleared through the stream, got {batches:?}" + ); + assert!( + response.response_result.is_ok(), + "having streamed, the pull is answered rather than held open" + ); +} + #[test] fn a_streaming_pull_with_nothing_to_say_is_still_held_open() { let dir = project(TRAILING); From 66983fbb98ab3c73547021fd2591a32781e69344 Mon Sep 17 00:00:00 2001 From: Owen Lamont Date: Sun, 6 Sep 2026 20:39:39 +0800 Subject: [PATCH 3/3] docs: cut comments the signature already carries An audit of every comment the branch adds, against this repo's "comments earn their place" bar: seven doc comments restated their own signature or duplicated the type's doc, and two test comments duplicated their assertion message. The rest are why-comments the code cannot convey on its own. Claude-Session: https://claude.ai/code/session_01L2XE37GUJWcHp7hPKMdHU8 --- src/lsp/mod.rs | 25 +++++++------------------ tests/lsp_server.rs | 5 +---- 2 files changed, 8 insertions(+), 22 deletions(-) diff --git a/src/lsp/mod.rs b/src/lsp/mod.rs index 28c9847..2dee17d 100644 --- a/src/lsp/mod.rs +++ b/src/lsp/mod.rs @@ -411,8 +411,8 @@ struct Server { /// Config errors already surfaced via `window/showMessage`, so a broken config is /// reported once rather than on every file/keystroke. reported_errors: HashSet, - /// In-flight `workspace/diagnostic` scans, each on its own thread (so the repo walk - /// never blocks the message loop) with a flag the loop flips to cancel it. + /// In-flight `workspace/diagnostic` scans, each on its own thread so the repo walk + /// never blocks the message loop. workers: Vec, /// The client's outstanding `workspace/diagnostic` pull; at most one, since the client /// sends the next only once this is answered. @@ -583,8 +583,7 @@ impl Server { } } "workspace/didChangeWatchedFiles" => { - // Source files are watched too, so only a config-named change moved the - // config; unparsable params keep the conservative reading. + // Params ryl cannot read say nothing about what changed, so assume the worst. let config_changed = parse::(¶ms) .is_none_or(|watched| { watched @@ -921,7 +920,6 @@ impl Server { self.spawn_scan(connection, id, by_path, token); } - /// Scan for the outstanding pull on a background thread, reporting over `scan_tx`. fn spawn_scan( &mut self, connection: &Connection, @@ -929,7 +927,7 @@ impl Server { previous: PreviousIds, token: Option, ) { - // Supersede any earlier scan (its result is dropped as stale), then reap the dead. + // A superseded scan's result is dropped as stale by `finish_scan`. self.cancel_workers(); self.workers.retain(|worker| !worker.handle.is_finished()); let cancel = Arc::new(AtomicBool::new(false)); @@ -950,7 +948,6 @@ impl Server { self.workers.push(Worker { cancel, handle }); } - /// Answer the outstanding pull, or leave it open. fn finish_scan(&mut self, connection: &Connection, result: ScanResult) { let Some(pull) = self.pull.take() else { return; @@ -1017,7 +1014,6 @@ impl Server { } } - /// Whether a watched-file event names a config ryl discovers, not a linted source. fn is_config_uri(&self, uri: &str) -> bool { let Some(path) = uri_to_path(uri) else { return false; @@ -1202,10 +1198,8 @@ fn file_report( file_pull_report(path, version, items, previous_id) } -/// One file's entry in a workspace pull: `Unchanged` while the client's result id matches, -/// a full report when it does not, an empty one to clear a file the client still holds -/// diagnostics for, and `None` for an untracked clean file — nothing to say, which is what -/// lets an idle pull suspend. +/// One file's entry in a workspace pull. `None` for an untracked clean file: nothing to +/// say about it, which is what lets an idle pull suspend. fn file_pull_report( path: &Path, version: Option, @@ -1239,7 +1233,6 @@ const STREAM_INTERVAL: Duration = Duration::from_millis(50); /// What a completed scan has to say; `streamed` commits the request to being answered. pub struct ScanOutcome { pub items: Vec, - /// Whether part of the report already went out as `$/progress` partial results. pub streamed: bool, } @@ -1272,7 +1265,6 @@ struct Stream { } impl ReportSink { - /// Holds everything for the response, for a client that offered no token. #[must_use] pub fn bulk() -> Self { Self { @@ -1295,7 +1287,6 @@ impl ReportSink { } } - /// Route one file's report; `Unchanged` is always held, streaming it says nothing. fn push(&mut self, report: WorkspaceDocumentDiagnosticReport) { match (&mut self.stream, &report) { (Some(stream), WorkspaceDocumentDiagnosticReport::Full(_)) => { @@ -1305,7 +1296,6 @@ impl ReportSink { } } - /// Send what has accumulated, if enough time has passed since the last batch. fn flush_batch(&mut self) { if let Some(stream) = &mut self.stream && !stream.pending.is_empty() @@ -1388,8 +1378,7 @@ pub fn workspace_scan( } sink.flush_batch(); } - // A path the client holds an id for that the walk no longer reports was deleted, - // renamed, or newly ignored: an empty, id-less report clears it and stops the echo. + // A tracked path the walk no longer reports was deleted, renamed or newly ignored. for (uri, _) in previous .iter() .filter(|(path, _)| !covered.contains(path)) diff --git a/tests/lsp_server.rs b/tests/lsp_server.rs index 63db2cf..4d0b7dc 100644 --- a/tests/lsp_server.rs +++ b/tests/lsp_server.rs @@ -2133,8 +2133,6 @@ fn a_malformed_watched_files_payload_still_relints() { "clean under the initial config" ); std::fs::write(dir.path().join(".ryl.toml"), TRAILING).expect("rewrite config"); - // Params ryl cannot read say nothing about *what* changed, so the config is assumed - // to have: a needless re-lint beats leaving stale diagnostics on screen. client.notify( "workspace/didChangeWatchedFiles", json!("not the params shape"), @@ -2198,8 +2196,7 @@ fn a_watched_yaml_change_wakes_a_suspended_pull() { "held open while idle" ); - // An edit from outside the editor: without the source-file watcher a long-polling pull - // would never learn of it. + // An edit from outside the editor, which only the source-file watcher reports. std::fs::write(&path, "a: 1\n").expect("fixed"); client.notify( "workspace/didChangeWatchedFiles",