diff --git a/docs/plans/SCRIPTING-FILEBASED-PLAN.md b/docs/plans/SCRIPTING-FILEBASED-PLAN.md index 633e777b..065c3573 100644 --- a/docs/plans/SCRIPTING-FILEBASED-PLAN.md +++ b/docs/plans/SCRIPTING-FILEBASED-PLAN.md @@ -116,15 +116,18 @@ remaining gaps are explicit rather than implied. - [x] **`SolutionLoader` returned any existing file as a project target**, so `Program.cs` was handed to `MSBuildWorkspace.OpenProjectAsync` and file-based mode was unreachable. Explicit file targets are now gated on `.sln`/`.slnx`/`.csproj` — implements [SCRIPT-DETECT] -- [x] Ambiguous multi-solution discovery returns an **error** rather than a synthetic workspace: the - directory path fails the file-existence guard in `OpenProjectlessAsync` — [SCRIPT-DEGRADE] +- [x] Ambiguous multi-solution discovery returns an **error** rather than a synthetic workspace, and + keeps doing so now that a project-less directory defers instead of failing: + `SolutionLoader.FindAmbiguousSolutions` separates ambiguity from absence, so only the genuinely + empty root takes the deferred path — [SCRIPT-DEGRADE] - [x] Confirmed `AddCrossLanguageMetadataReferences` still runs on the MSBuild path after the `OpenCoreAsync` reordering - [x] Wrap per-file I/O in closure expansion so one unreadable file degrades that file only — [SCRIPT-DEGRADE] - [x] Honor `CancellationToken` inside the closure read loop -- [ ] Distinguish "absent" from "ambiguous" in the error *message*; both currently report the - no-solution-found text — [SCRIPT-DEGRADE] +- [x] Distinguish "absent" from "ambiguous" in the error *message*: absent now defers to lazy + per-file loading, and ambiguous reports every candidate solution plus the + `csharp.solution_path` setting that resolves it — [SCRIPT-DEGRADE] ### F# Sidecar — scripts @@ -171,8 +174,11 @@ Coarse, real-artifact tests only — real files on disk, real Roslyn, real FCS, - [x] Shebang produces no diagnostic — [FILEBASED-SHEBANG] - [x] `.csx`: script semantics load and the script `#load` path resolves — [CSX-OPTIONS] - [x] Closure cycle (`a.cs` includes `b.cs` includes `a.cs`) terminates — [SCRIPT-CLOSURE] -- [x] A directory with neither project nor root file is an error, not a synthetic workspace — +- [x] A directory with neither project nor root file defers to lazy per-file loading rather than + building a synthetic workspace, and each loose file becomes its own ad-hoc project — [SCRIPT-DEGRADE] +- [x] A directory holding several solutions is an **error** naming the candidates and + `csharp.solution_path`, never the deferred path — [SCRIPT-DEGRADE] - [x] `Classify` maps extensions to compilation models — [SCRIPT-DETECT] `sidecars/SharpLsp.Sidecar.FSharp.Tests/FSharpScriptTests.fs`: diff --git a/docs/specs/SCRIPTING-FILEBASED-SPEC.md b/docs/specs/SCRIPTING-FILEBASED-SPEC.md index 6a470c52..9b70852e 100644 --- a/docs/specs/SCRIPTING-FILEBASED-SPEC.md +++ b/docs/specs/SCRIPTING-FILEBASED-SPEC.md @@ -306,12 +306,23 @@ yields two closures, not one project containing both. ## 8. Error handling and degradation `[SCRIPT-DEGRADE]` -- A path that resolves to no supported document kind returns a `Result` failure. It must not be - silently converted into an empty synthetic workspace — that turns a real "I could not load your +- A **file** path that resolves to no supported document kind returns a `Result` failure. It must not + be silently converted into an empty synthetic workspace — that turns a real "I could not load your code" into a wall of phantom diagnostics. -- Ambiguous solution discovery (multiple `.sln` under the root) already returns "no target" from - `SolutionLoader`. That case is **ambiguity, not absence**, and must surface as an error asking the - user to choose. Treating it as file-based mode would silently mis-analyze an entire repository. +- A **directory** holding no solution or project at all is not a failure. The host opens a workspace + folder eagerly, before any document exists, so `OpenCoreAsync` records the root as project-less and + returns success, deferring workspace creation to the first document update. That document is then + loaded as a file-based app or script, and each subsequent loose file is added as its own ad-hoc + project — two independent files in one folder stay two compilations, per [SCRIPT-ANTIPATTERN]. + `IsLoaded` stays false until a document arrives, so nothing claims a workspace exists before one + does. +- Ambiguous solution discovery (multiple `.sln` under the root) also returns "no target" from + `SolutionLoader`. That case is **ambiguity, not absence**, and must surface as an error naming every + candidate and the `csharp.solution_path` setting that resolves it — never the project-less deferral + above. Treating it as file-based mode would silently mis-analyze an entire repository: no project + reference resolves, and every cross-project type becomes a phantom "not found" diagnostic. + `SolutionLoader.FindAmbiguousSolutions` is what distinguishes the two, and + [WORKSPACE-SOLUTION-PATH] specifies the setting the message points at. - Any I/O during closure expansion is wrapped; a failure to read one included file degrades that file only and is reported as a diagnostic, leaving the rest of the closure loaded. diff --git a/docs/specs/SHARPLSP-SPEC.md b/docs/specs/SHARPLSP-SPEC.md index 050b0514..16c6d2ac 100644 --- a/docs/specs/SHARPLSP-SPEC.md +++ b/docs/specs/SHARPLSP-SPEC.md @@ -121,6 +121,33 @@ The project system is the hardest engineering problem in .NET tooling. [MSBuild] - **Multi-targeting:** Projects targeting multiple TFMs (e.g., `net8.0;net48;netstandard2.0`) present multiple analysis contexts. SharpLsp exposes a custom LSP extension for users to select the active TFM, defaulting to the first. - **Project-less files:** A `.cs` [file-based app](https://learn.microsoft.com/en-us/dotnet/core/sdk/file-based-apps), a `.csx` Roslyn script, and a `.fsx` F# script are all first-class editing targets with no owning project. Their compilation closure is derived from the root file — `#:include` for file-based apps, `#load` for scripts — and never from the containing directory. See [SCRIPTING-FILEBASED-SPEC.md](SCRIPTING-FILEBASED-SPEC.md). +#### Choosing the Solution to Open `[WORKSPACE-SOLUTION-PATH]` + +The host sends one path to each sidecar's `workspace/open`. When that path is a +directory, the C# sidecar discovers a target under it: an unambiguous `.sln`, +`.slnx`, or `.csproj` is opened directly. Discovery **never guesses** between +several nested solutions — a monorepo root holding `app/App.sln` and +`other/Other.sln` is ambiguous, and guessing would silently load the wrong half +of the repository. + +`csharp.solution_path` in `sharplsp.toml` resolves that ambiguity by naming the +solution to open, absolute or relative to the workspace root: + +```toml +[csharp] +solution_path = "app/App.sln" +``` + +The host resolves the setting and sends the **solution file** rather than the +root, so the sidecar opens it without running discovery at all. The setting +falls back to workspace-root discovery when unset, and when it names a path that +is not an existing file — a stale or misspelled entry degrades to auto-discovery +instead of wedging the workspace on a path that cannot load. + +Without this, an ambiguous root loads no solution, and every semantic +request — hover, completion, diagnostics, navigation — returns empty for the +whole workspace. + ### 2.6 Binary Layout & Installation **The `sharplsp` binary is bundled inside every per-platform VSIX.** A user who installs the VS Code extension gets a fully working LSP server with zero additional steps. Extensions are NOT thin clients that require a system-installed binary — the binary ships inside the extension. diff --git a/docs/specs/SOLUTION-EXPLORER-SPEC.md b/docs/specs/SOLUTION-EXPLORER-SPEC.md index 78210a8e..ccfbc8eb 100644 --- a/docs/specs/SOLUTION-EXPLORER-SPEC.md +++ b/docs/specs/SOLUTION-EXPLORER-SPEC.md @@ -36,6 +36,26 @@ Per-file symbols are sourced by language, never by a single parser: The F# path reuses the **same** sidecar `documentSymbol` request that powers the editor outline, mapping the nested FCS symbols (module, namespace, type, DU case, member) into the shared `FileSymbol`/`SymbolNode` tree model using each symbol's full range. The F# sidecar must be threaded into `workspace_symbols::handle`; when it is unavailable the project's `.fs` files contribute no symbols rather than failing the whole request. +### Live-Buffer Path Identity [SE-LIVE-BUFFER] + +`sharplsp/workspaceSymbols` MUST parse the latest open-buffer text, including +unsaved and rapid successive edits. Disk content is used only when no open VFS +document denotes the source file. + +The editor URI and the project model can use different native paths for the same +file. In particular, Windows runners can send an 8.3 path such as +`C:\Users\RUNNER~1\...`, while the sidecar reports the expanded +`C:\Users\runneradmin\...` path. The VFS therefore resolves and caches the +editor path when the document opens, then compares both the original URI path +and that canonical path during native-path lookup. Canonicalizing only the +project-model path is insufficient because it leaves the editor's aliased path +unchanged and incorrectly falls back to stale disk text. + +Path comparison also ignores Windows verbatim prefixes and casing differences. +The coarse VS Code explorer tests prove that the tree reflects an unsaved rename +and the final value in a burst of renames; the VFS alias regression test covers +the reverse-alias lookup independently of hosted-runner path spelling. + ### Request: `sharplsp/workspaceSymbols` **Params:** diff --git a/editors/vscode/package-lock.json b/editors/vscode/package-lock.json index f8d69baa..6e37789f 100644 --- a/editors/vscode/package-lock.json +++ b/editors/vscode/package-lock.json @@ -17,11 +17,11 @@ "@eslint/js": "^10.0.1", "@types/mocha": "^10.0.6", "@types/node": "^26.1.1", - "@types/vscode": "^1.99.0", + "@types/vscode": "^1.125.0", "@typescript-eslint/eslint-plugin": "^8.64.0", "@typescript-eslint/parser": "^8.64.0", "@vscode/test-cli": "^0.0.15", - "@vscode/test-electron": "^3.0.0", + "@vscode/test-electron": "^3.1.0", "@vscode/vsce": "^3.9.2", "esbuild": "^0.28.0", "eslint": "^10.7.0", @@ -32,7 +32,7 @@ "typescript-eslint": "^8.64.0" }, "engines": { - "vscode": "^1.99.0" + "vscode": "^1.125.0" } }, "node_modules/@azu/format-text": { @@ -762,16 +762,16 @@ } }, "node_modules/@eslint/config-array/node_modules/brace-expansion": { - "version": "5.0.7", - "resolved": "https://registry.npmjs.org/brace-expansion/-/brace-expansion-5.0.7.tgz", - "integrity": "sha512-7oFy703dxfY3/NLxC1fh2SUCQ0H9rmAY+5EpDVfXjUTTs+HEwR2nYaqLv+GWcTsumwxPfiz6CzCNkwXwBUwqCA==", + "version": "5.0.8", + "resolved": "https://registry.npmjs.org/brace-expansion/-/brace-expansion-5.0.8.tgz", + "integrity": "sha512-JZyDyq3D4AUifKTPOB7DELf6XsB3WdPuNxCtob1vFXPsSXhdAiHBWJ/tJ8HAc9aH84BK+5JFZLNkJKx3G9kzQg==", "dev": true, "license": "MIT", "dependencies": { "balanced-match": "^4.0.2" }, "engines": { - "node": "18 || 20 || >=22" + "node": "20 || >=22" } }, "node_modules/@eslint/config-array/node_modules/minimatch": { @@ -1661,16 +1661,16 @@ } }, "node_modules/@typescript-eslint/eslint-plugin/node_modules/brace-expansion": { - "version": "5.0.7", - "resolved": "https://registry.npmjs.org/brace-expansion/-/brace-expansion-5.0.7.tgz", - "integrity": "sha512-7oFy703dxfY3/NLxC1fh2SUCQ0H9rmAY+5EpDVfXjUTTs+HEwR2nYaqLv+GWcTsumwxPfiz6CzCNkwXwBUwqCA==", + "version": "5.0.8", + "resolved": "https://registry.npmjs.org/brace-expansion/-/brace-expansion-5.0.8.tgz", + "integrity": "sha512-JZyDyq3D4AUifKTPOB7DELf6XsB3WdPuNxCtob1vFXPsSXhdAiHBWJ/tJ8HAc9aH84BK+5JFZLNkJKx3G9kzQg==", "dev": true, "license": "MIT", "dependencies": { "balanced-match": "^4.0.2" }, "engines": { - "node": "18 || 20 || >=22" + "node": "20 || >=22" } }, "node_modules/@typescript-eslint/eslint-plugin/node_modules/minimatch": { @@ -1791,16 +1791,16 @@ } }, "node_modules/@typescript-eslint/parser/node_modules/brace-expansion": { - "version": "5.0.7", - "resolved": "https://registry.npmjs.org/brace-expansion/-/brace-expansion-5.0.7.tgz", - "integrity": "sha512-7oFy703dxfY3/NLxC1fh2SUCQ0H9rmAY+5EpDVfXjUTTs+HEwR2nYaqLv+GWcTsumwxPfiz6CzCNkwXwBUwqCA==", + "version": "5.0.8", + "resolved": "https://registry.npmjs.org/brace-expansion/-/brace-expansion-5.0.8.tgz", + "integrity": "sha512-JZyDyq3D4AUifKTPOB7DELf6XsB3WdPuNxCtob1vFXPsSXhdAiHBWJ/tJ8HAc9aH84BK+5JFZLNkJKx3G9kzQg==", "dev": true, "license": "MIT", "dependencies": { "balanced-match": "^4.0.2" }, "engines": { - "node": "18 || 20 || >=22" + "node": "20 || >=22" } }, "node_modules/@typescript-eslint/parser/node_modules/minimatch": { @@ -1958,16 +1958,16 @@ } }, "node_modules/@vscode/test-cli/node_modules/brace-expansion": { - "version": "5.0.7", - "resolved": "https://registry.npmjs.org/brace-expansion/-/brace-expansion-5.0.7.tgz", - "integrity": "sha512-7oFy703dxfY3/NLxC1fh2SUCQ0H9rmAY+5EpDVfXjUTTs+HEwR2nYaqLv+GWcTsumwxPfiz6CzCNkwXwBUwqCA==", + "version": "5.0.8", + "resolved": "https://registry.npmjs.org/brace-expansion/-/brace-expansion-5.0.8.tgz", + "integrity": "sha512-JZyDyq3D4AUifKTPOB7DELf6XsB3WdPuNxCtob1vFXPsSXhdAiHBWJ/tJ8HAc9aH84BK+5JFZLNkJKx3G9kzQg==", "dev": true, "license": "MIT", "dependencies": { "balanced-match": "^4.0.2" }, "engines": { - "node": "18 || 20 || >=22" + "node": "20 || >=22" } }, "node_modules/@vscode/test-cli/node_modules/cliui": { @@ -2102,9 +2102,9 @@ } }, "node_modules/@vscode/test-electron": { - "version": "3.0.0", - "resolved": "https://registry.npmjs.org/@vscode/test-electron/-/test-electron-3.0.0.tgz", - "integrity": "sha512-TY5mC7aAjxSLDXsyjhrG8cJHgc/HLdiE5lvtW7hABYQrY24Qwozzr5UoO3HiuAM4Hzz4b7K/eZlwrCILj94CcA==", + "version": "3.1.0", + "resolved": "https://registry.npmjs.org/@vscode/test-electron/-/test-electron-3.1.0.tgz", + "integrity": "sha512-CRqv5u+YYoseuNVJ6Tyo4k0sF0mx4qnKMihRB0PjsUF8Dc0WKtCXo6CNL6nWWm5esfFQsQA/pejMj4ZbpJVLTw==", "dev": true, "license": "MIT", "dependencies": { @@ -2321,16 +2321,16 @@ } }, "node_modules/@vscode/vsce/node_modules/brace-expansion": { - "version": "5.0.7", - "resolved": "https://registry.npmjs.org/brace-expansion/-/brace-expansion-5.0.7.tgz", - "integrity": "sha512-7oFy703dxfY3/NLxC1fh2SUCQ0H9rmAY+5EpDVfXjUTTs+HEwR2nYaqLv+GWcTsumwxPfiz6CzCNkwXwBUwqCA==", + "version": "5.0.8", + "resolved": "https://registry.npmjs.org/brace-expansion/-/brace-expansion-5.0.8.tgz", + "integrity": "sha512-JZyDyq3D4AUifKTPOB7DELf6XsB3WdPuNxCtob1vFXPsSXhdAiHBWJ/tJ8HAc9aH84BK+5JFZLNkJKx3G9kzQg==", "dev": true, "license": "MIT", "dependencies": { "balanced-match": "^4.0.2" }, "engines": { - "node": "18 || 20 || >=22" + "node": "20 || >=22" } }, "node_modules/@vscode/vsce/node_modules/minimatch": { @@ -2559,15 +2559,6 @@ "dev": true, "license": "BSD-2-Clause" }, - "node_modules/brace-expansion": { - "version": "2.0.3", - "resolved": "https://registry.npmjs.org/brace-expansion/-/brace-expansion-2.0.3.tgz", - "integrity": "sha512-MCV/fYJEbqx68aE58kv2cA/kiky1G8vux3OR6/jbS+jIMe/6fJWa0DTzJU7dqijOWYwHi1t29FlfYI9uytqlpA==", - "license": "MIT", - "dependencies": { - "balanced-match": "^1.0.0" - } - }, "node_modules/braces": { "version": "3.0.3", "resolved": "https://registry.npmjs.org/braces/-/braces-3.0.3.tgz", @@ -3502,16 +3493,16 @@ } }, "node_modules/eslint/node_modules/brace-expansion": { - "version": "5.0.7", - "resolved": "https://registry.npmjs.org/brace-expansion/-/brace-expansion-5.0.7.tgz", - "integrity": "sha512-7oFy703dxfY3/NLxC1fh2SUCQ0H9rmAY+5EpDVfXjUTTs+HEwR2nYaqLv+GWcTsumwxPfiz6CzCNkwXwBUwqCA==", + "version": "5.0.8", + "resolved": "https://registry.npmjs.org/brace-expansion/-/brace-expansion-5.0.8.tgz", + "integrity": "sha512-JZyDyq3D4AUifKTPOB7DELf6XsB3WdPuNxCtob1vFXPsSXhdAiHBWJ/tJ8HAc9aH84BK+5JFZLNkJKx3G9kzQg==", "dev": true, "license": "MIT", "dependencies": { "balanced-match": "^4.0.2" }, "engines": { - "node": "18 || 20 || >=22" + "node": "20 || >=22" } }, "node_modules/eslint/node_modules/eslint-visitor-keys": { @@ -4044,16 +4035,16 @@ } }, "node_modules/glob/node_modules/brace-expansion": { - "version": "5.0.7", - "resolved": "https://registry.npmjs.org/brace-expansion/-/brace-expansion-5.0.7.tgz", - "integrity": "sha512-7oFy703dxfY3/NLxC1fh2SUCQ0H9rmAY+5EpDVfXjUTTs+HEwR2nYaqLv+GWcTsumwxPfiz6CzCNkwXwBUwqCA==", + "version": "5.0.8", + "resolved": "https://registry.npmjs.org/brace-expansion/-/brace-expansion-5.0.8.tgz", + "integrity": "sha512-JZyDyq3D4AUifKTPOB7DELf6XsB3WdPuNxCtob1vFXPsSXhdAiHBWJ/tJ8HAc9aH84BK+5JFZLNkJKx3G9kzQg==", "dev": true, "license": "MIT", "dependencies": { "balanced-match": "^4.0.2" }, "engines": { - "node": "18 || 20 || >=22" + "node": "20 || >=22" } }, "node_modules/glob/node_modules/minimatch": { @@ -5103,6 +5094,16 @@ "url": "https://github.com/sponsors/isaacs" } }, + "node_modules/minimatch/node_modules/brace-expansion": { + "version": "2.0.3", + "resolved": "https://registry.npmjs.org/brace-expansion/-/brace-expansion-2.0.3.tgz", + "integrity": "sha512-MCV/fYJEbqx68aE58kv2cA/kiky1G8vux3OR6/jbS+jIMe/6fJWa0DTzJU7dqijOWYwHi1t29FlfYI9uytqlpA==", + "dev": true, + "license": "MIT", + "dependencies": { + "balanced-match": "^1.0.0" + } + }, "node_modules/minimist": { "version": "1.2.8", "resolved": "https://registry.npmjs.org/minimist/-/minimist-1.2.8.tgz", @@ -6842,16 +6843,16 @@ } }, "node_modules/test-exclude/node_modules/brace-expansion": { - "version": "5.0.7", - "resolved": "https://registry.npmjs.org/brace-expansion/-/brace-expansion-5.0.7.tgz", - "integrity": "sha512-7oFy703dxfY3/NLxC1fh2SUCQ0H9rmAY+5EpDVfXjUTTs+HEwR2nYaqLv+GWcTsumwxPfiz6CzCNkwXwBUwqCA==", + "version": "5.0.8", + "resolved": "https://registry.npmjs.org/brace-expansion/-/brace-expansion-5.0.8.tgz", + "integrity": "sha512-JZyDyq3D4AUifKTPOB7DELf6XsB3WdPuNxCtob1vFXPsSXhdAiHBWJ/tJ8HAc9aH84BK+5JFZLNkJKx3G9kzQg==", "dev": true, "license": "MIT", "dependencies": { "balanced-match": "^4.0.2" }, "engines": { - "node": "18 || 20 || >=22" + "node": "20 || >=22" } }, "node_modules/test-exclude/node_modules/minimatch": { @@ -7142,16 +7143,16 @@ } }, "node_modules/typescript-eslint/node_modules/brace-expansion": { - "version": "5.0.7", - "resolved": "https://registry.npmjs.org/brace-expansion/-/brace-expansion-5.0.7.tgz", - "integrity": "sha512-7oFy703dxfY3/NLxC1fh2SUCQ0H9rmAY+5EpDVfXjUTTs+HEwR2nYaqLv+GWcTsumwxPfiz6CzCNkwXwBUwqCA==", + "version": "5.0.8", + "resolved": "https://registry.npmjs.org/brace-expansion/-/brace-expansion-5.0.8.tgz", + "integrity": "sha512-JZyDyq3D4AUifKTPOB7DELf6XsB3WdPuNxCtob1vFXPsSXhdAiHBWJ/tJ8HAc9aH84BK+5JFZLNkJKx3G9kzQg==", "dev": true, "license": "MIT", "dependencies": { "balanced-match": "^4.0.2" }, "engines": { - "node": "18 || 20 || >=22" + "node": "20 || >=22" } }, "node_modules/typescript-eslint/node_modules/minimatch": { @@ -7310,6 +7311,15 @@ "vscode": "^1.82.0" } }, + "node_modules/vscode-languageclient/node_modules/brace-expansion": { + "version": "2.0.3", + "resolved": "https://registry.npmjs.org/brace-expansion/-/brace-expansion-2.0.3.tgz", + "integrity": "sha512-MCV/fYJEbqx68aE58kv2cA/kiky1G8vux3OR6/jbS+jIMe/6fJWa0DTzJU7dqijOWYwHi1t29FlfYI9uytqlpA==", + "license": "MIT", + "dependencies": { + "balanced-match": "^1.0.0" + } + }, "node_modules/vscode-languageclient/node_modules/minimatch": { "version": "5.1.9", "resolved": "https://registry.npmjs.org/minimatch/-/minimatch-5.1.9.tgz", diff --git a/editors/vscode/package.json b/editors/vscode/package.json index 2d3d2104..36817148 100644 --- a/editors/vscode/package.json +++ b/editors/vscode/package.json @@ -7,7 +7,7 @@ "license": "MIT", "icon": "icons/sharplsp.png", "engines": { - "vscode": "^1.99.0" + "vscode": "^1.125.0" }, "extensionDependencies": [ "ms-dotnettools.vscode-dotnet-runtime" @@ -1051,11 +1051,11 @@ "@eslint/js": "^10.0.1", "@types/mocha": "^10.0.6", "@types/node": "^26.1.1", - "@types/vscode": "^1.99.0", + "@types/vscode": "^1.125.0", "@typescript-eslint/eslint-plugin": "^8.64.0", "@typescript-eslint/parser": "^8.64.0", "@vscode/test-cli": "^0.0.15", - "@vscode/test-electron": "^3.0.0", + "@vscode/test-electron": "^3.1.0", "@vscode/vsce": "^3.9.2", "esbuild": "^0.28.0", "eslint": "^10.7.0", @@ -1078,6 +1078,6 @@ "overrides": { "serialize-javascript": "7.0.5", "brace-expansion@^2.0.0": "2.0.3", - "brace-expansion@^5.0.0": "5.0.7" + "brace-expansion@^5.0.0": "5.0.8" } } diff --git a/editors/vscode/src/output-filter.ts b/editors/vscode/src/output-filter.ts index b915d114..e93a4f41 100644 --- a/editors/vscode/src/output-filter.ts +++ b/editors/vscode/src/output-filter.ts @@ -9,7 +9,7 @@ * The stripper is a small character scanner (no regular expressions, per the * project's "use real parsers, not regex" rule). */ -import type { OutputChannel, ViewColumn } from 'vscode'; +import type { LogOutputChannel, OutputChannel, ViewColumn } from 'vscode'; const ESC = '\u001b'; const BEL = '\u0007'; @@ -72,13 +72,11 @@ export function stripAnsi(text: string): string { return parts.join(''); } -/** - * Wrap an output channel so every write has ANSI escape sequences stripped. - * Non-writing operations (show/hide/clear/dispose) delegate unchanged. - */ -export function createAnsiStrippingChannel(inner: OutputChannel): OutputChannel { +/** Forward the free-text writes with ANSI stripped. */ +function writeMethods( + inner: LogOutputChannel, +): Pick { return { - name: inner.name, append: (value: string): void => { inner.append(stripAnsi(value)); }, @@ -88,6 +86,41 @@ export function createAnsiStrippingChannel(inner: OutputChannel): OutputChannel replace: (value: string): void => { inner.replace(stripAnsi(value)); }, + }; +} + +/** + * Forward the level-tagged log methods with ANSI stripped from the message. + * An `Error` passed to `error` is forwarded as-is: its message is rendered by + * the channel, not written as raw text, so it carries no escape sequences. + */ +function logMethods( + inner: LogOutputChannel, +): Pick { + return { + trace: (message: string, ...args: unknown[]): void => { + inner.trace(stripAnsi(message), ...args); + }, + debug: (message: string, ...args: unknown[]): void => { + inner.debug(stripAnsi(message), ...args); + }, + info: (message: string, ...args: unknown[]): void => { + inner.info(stripAnsi(message), ...args); + }, + warn: (message: string, ...args: unknown[]): void => { + inner.warn(stripAnsi(message), ...args); + }, + error: (error: string | Error, ...args: unknown[]): void => { + inner.error(typeof error === 'string' ? stripAnsi(error) : error, ...args); + }, + }; +} + +/** Forward the non-writing operations unchanged. */ +function lifecycleMethods( + inner: LogOutputChannel, +): Pick { + return { clear: (): void => { inner.clear(); }, @@ -103,3 +136,30 @@ export function createAnsiStrippingChannel(inner: OutputChannel): OutputChannel }, }; } + +/** + * Wrap a log output channel so every write has ANSI escape sequences stripped. + * Non-writing operations (show/hide/clear/dispose) delegate unchanged. + * + * `vscode-languageclient` v10 types `LanguageClientOptions.outputChannel` as a + * `LogOutputChannel`, so the wrapper forwards the log-level surface too. + * `name`, `logLevel`, and `onDidChangeLogLevel` are getters rather than copied + * values: the user can change the channel's log level at any time, and a + * snapshot would silently report a stale level forever. + */ +export function createAnsiStrippingChannel(inner: LogOutputChannel): LogOutputChannel { + return { + get name(): string { + return inner.name; + }, + get logLevel() { + return inner.logLevel; + }, + get onDidChangeLogLevel() { + return inner.onDidChangeLogLevel; + }, + ...writeMethods(inner), + ...logMethods(inner), + ...lifecycleMethods(inner), + }; +} diff --git a/editors/vscode/src/test/suite/fsi-build-output-e2e.test.ts b/editors/vscode/src/test/suite/fsi-build-output-e2e.test.ts index dd797166..bad15871 100644 --- a/editors/vscode/src/test/suite/fsi-build-output-e2e.test.ts +++ b/editors/vscode/src/test/suite/fsi-build-output-e2e.test.ts @@ -31,11 +31,13 @@ import { removeDirRecursive } from './test-helpers.js'; // ── Fake OutputChannel ──────────────────────────────────────────── -/** Records everything an OutputChannel receives so we can assert on it. */ -interface RecordingChannel extends vscode.OutputChannel { +/** Records everything a LogOutputChannel receives so we can assert on it. */ +interface RecordingChannel extends vscode.LogOutputChannel { readonly appended: string[]; readonly appendedLines: string[]; readonly replaced: string[]; + /** Level-tagged log calls, as `level:message`. */ + readonly logged: string[]; cleared: number; shown: number; hidden: number; @@ -47,15 +49,19 @@ function recordingChannel(name: string): RecordingChannel { const appended: string[] = []; const appendedLines: string[] = []; const replaced: string[] = []; + const logged: string[] = []; const channel: RecordingChannel = { name, appended, appendedLines, replaced, + logged, cleared: 0, shown: 0, hidden: 0, disposed: 0, + logLevel: vscode.LogLevel.Info, + onDidChangeLogLevel: new vscode.EventEmitter().event, append(value: string): void { appended.push(value); }, @@ -65,6 +71,21 @@ function recordingChannel(name: string): RecordingChannel { replace(value: string): void { replaced.push(value); }, + trace(message: string): void { + logged.push(`trace:${message}`); + }, + debug(message: string): void { + logged.push(`debug:${message}`); + }, + info(message: string): void { + logged.push(`info:${message}`); + }, + warn(message: string): void { + logged.push(`warn:${message}`); + }, + error(error: string | Error): void { + logged.push(`error:${typeof error === 'string' ? error : error.message}`); + }, clear(): void { channel.cleared += 1; }, @@ -171,6 +192,38 @@ suite('FSI / Build / Output-filter / Hot-reload E2E', () => { assert.strictEqual(inner.appended[inner.appended.length - 1], 'plain line'); }); + test('createAnsiStrippingChannel strips ANSI from the level-tagged log methods', function () { + this.timeout(20_000); + const inner = recordingChannel('SharpLsp'); + const wrapped = createAnsiStrippingChannel(inner); + + // The sequences below embed real ESC control bytes, matching the convention + // the stripAnsi assertions above already use. This matters: if the ESC bytes + // are ever lost, the literals degrade to bare "[2m" text and the test + // silently asserts nothing at all, because stripAnsi correctly leaves + // non-ANSI text alone. It is the ESC that makes a sequence a sequence. + wrapped.trace('trace line'); + wrapped.debug('debug line'); + wrapped.info('info line'); + wrapped.warn('warn line'); + wrapped.error('error line'); + + assert.deepStrictEqual(inner.logged, [ + 'trace:trace line', + 'debug:debug line', + 'info:info line', + 'warn:warn line', + 'error:error line', + ]); + + // An Error forwards intact — its message is rendered, never written raw. + wrapped.error(new Error('boom')); + assert.strictEqual(inner.logged[inner.logged.length - 1], 'error:boom'); + + // logLevel delegates live rather than snapshotting at wrap time. + assert.strictEqual(wrapped.logLevel, inner.logLevel); + }); + // ── build.ts ──────────────────────────────────────────────────── test('dotnet build/rebuild/clean commands resolve and create the build terminal', async function () { diff --git a/editors/vscode/src/test/suite/solution-explorer.test.ts b/editors/vscode/src/test/suite/solution-explorer.test.ts index 370c5d2b..1fed139c 100644 --- a/editors/vscode/src/test/suite/solution-explorer.test.ts +++ b/editors/vscode/src/test/suite/solution-explorer.test.ts @@ -669,7 +669,7 @@ public class EventSource } }); - // ── VFS vs Disk Stale Data Bug ─────────────────────────────── + // ── Live-buffer fidelity [SE-LIVE-BUFFER] ─────────────────────────────── test('documentSymbol reflects unsaved edits (VFS-based, should pass)', async function () { this.timeout(15_000); diff --git a/sharplsp.example.toml b/sharplsp.example.toml index 838a04f2..f8e824ab 100644 --- a/sharplsp.example.toml +++ b/sharplsp.example.toml @@ -11,7 +11,9 @@ debounce_ms = 150 [csharp] # Enable C# language support enabled = true -# Path to .sln or .slnx file (auto-detected if empty) +# Path to .sln or .slnx file, absolute or relative to the workspace root +# (auto-detected if empty). Required when the root holds more than one solution: +# auto-detection refuses to guess between them and loads nothing. solution_path = "" [fsharp] diff --git a/sidecars/SharpLsp.Sidecar.CSharp.Tests/GlobalJsonSdkPinEndToEndTests.cs b/sidecars/SharpLsp.Sidecar.CSharp.Tests/GlobalJsonSdkPinEndToEndTests.cs index 045535a4..22cda90a 100644 --- a/sidecars/SharpLsp.Sidecar.CSharp.Tests/GlobalJsonSdkPinEndToEndTests.cs +++ b/sidecars/SharpLsp.Sidecar.CSharp.Tests/GlobalJsonSdkPinEndToEndTests.cs @@ -49,6 +49,7 @@ public async Task Sidecar_reaches_ready_and_serves_solution_read_when_global_jso // SDK satisfies it — the exact shape of the Fantomas failure. await File.WriteAllTextAsync( Path.Combine(workspace, "global.json"), + /*lang=json,strict*/ """ { "sdk": { "version": "999.999.100", "rollForward": "latestPatch" } } """ diff --git a/sidecars/SharpLsp.Sidecar.CSharp.Tests/WorkspaceManagerDegenerateCoverageTests.cs b/sidecars/SharpLsp.Sidecar.CSharp.Tests/WorkspaceManagerDegenerateCoverageTests.cs index bc34a1fc..b6f92d85 100644 --- a/sidecars/SharpLsp.Sidecar.CSharp.Tests/WorkspaceManagerDegenerateCoverageTests.cs +++ b/sidecars/SharpLsp.Sidecar.CSharp.Tests/WorkspaceManagerDegenerateCoverageTests.cs @@ -142,7 +142,7 @@ await Assert } [Fact] - public async Task Open_on_directory_without_a_project_fails() + public async Task Open_on_directory_without_a_project_succeeds_lazily() { var emptyDir = Path.Combine(_root, "empty"); Directory.CreateDirectory(emptyDir); @@ -152,7 +152,7 @@ public async Task Open_on_directory_without_a_project_fails() var result = await manager.OpenAsync(emptyDir); #pragma warning restore CS0618 - Assert.True(result.IsError, "opening a project-less directory must fail"); + Assert.False(result.IsError, "opening a project-less directory succeeds lazily"); } [Fact] diff --git a/sidecars/SharpLsp.Sidecar.CSharp.Tests/WorkspaceManagerSingleFileTests.cs b/sidecars/SharpLsp.Sidecar.CSharp.Tests/WorkspaceManagerSingleFileTests.cs index 4521a6b4..c67b9cbf 100644 --- a/sidecars/SharpLsp.Sidecar.CSharp.Tests/WorkspaceManagerSingleFileTests.cs +++ b/sidecars/SharpLsp.Sidecar.CSharp.Tests/WorkspaceManagerSingleFileTests.cs @@ -178,21 +178,68 @@ public async Task CsxScript_loads_with_script_semantics() } /// - /// A directory with no project and no root file is a load FAILURE, not a synthetic empty - /// workspace. Silently succeeding turns "I could not load your code" into phantom - /// diagnostics across the whole repo. Implements [SCRIPT-DEGRADE]. + /// A directory with no project and no root file defers loading, returning success so that + /// the workspace can lazily inject independently requested files as ad-hoc projects. /// [Fact] - public async Task Directory_without_project_or_root_file_is_an_error() + public async Task Directory_without_project_or_root_file_succeeds_for_lazy_loading() { using var manager = new WorkspaceManager(); var result = await OpenAsync(manager, _root); - Assert.True(result.IsError); + Assert.False(result.IsError); Assert.False(manager.IsLoaded); } + /// + /// Ambiguity is NOT absence. A root holding several solutions must surface an error + /// naming the knob that resolves it, never degrade to lazy per-file ad-hoc projects: + /// loose-file analysis of a real repository resolves no project reference and reports + /// a wall of phantom "type not found" diagnostics across the whole tree, which is + /// strictly worse than refusing to load. Implements [SCRIPT-DEGRADE]. + /// + [Fact] + public async Task Ambiguous_multi_solution_root_is_an_error_not_lazy_loading() + { + _ = WriteIn("app", "App.sln", ""); + _ = WriteIn("other", "Other.sln", ""); + using var manager = new WorkspaceManager(); + + var result = await OpenAsync(manager, _root); + + Assert.True(result.IsError, "an ambiguous multi-solution root must not load lazily"); + var message = result.Match(_ => "", error => error); + Assert.Contains("solution_path", message, StringComparison.Ordinal); + Assert.Contains("App.sln", message, StringComparison.Ordinal); + Assert.Contains("Other.sln", message, StringComparison.Ordinal); + } + + /// + /// Opening multiple independent script files in a projectless directory lazily creates + /// a new ad-hoc project for each of them simultaneously. + /// + [Fact] + public async Task Independent_scripts_in_projectless_directory_are_lazily_loaded_simultaneously() + { + using var manager = new WorkspaceManager(); + var result = await OpenAsync(manager, _root); + Assert.False(result.IsError); + + var file1 = Write("file1.cs", "Console.WriteLine(1);\n"); + var file2 = Write("file2.cs", "Console.WriteLine(2);\n"); + + var update1 = await manager.UpdateDocumentTextAsync(file1, "Console.WriteLine(1);\n"); + Assert.False(update1.IsError); + + var update2 = await manager.UpdateDocumentTextAsync(file2, "Console.WriteLine(2);\n"); + Assert.False(update2.IsError); + + Assert.True(manager.IsLoaded); + Assert.Empty(await ErrorsAsync(manager, file1)); + Assert.Empty(await ErrorsAsync(manager, file2)); + } + /// /// The whole .NET 10 directive vocabulary must parse cleanly. Roslyn lexes #: as /// IGNORED trivia — the SDK owns their meaning — so a correct header contributes zero compiler diff --git a/sidecars/SharpLsp.Sidecar.CSharp/Workspace/SolutionLoader.cs b/sidecars/SharpLsp.Sidecar.CSharp/Workspace/SolutionLoader.cs index 9281e585..49392456 100644 --- a/sidecars/SharpLsp.Sidecar.CSharp/Workspace/SolutionLoader.cs +++ b/sidecars/SharpLsp.Sidecar.CSharp/Workspace/SolutionLoader.cs @@ -83,6 +83,24 @@ private static string PickBestSolution(string[] solutionFiles, string workspaceP return match ?? solutionFiles[0]; } + /// + /// The competing solutions under when recursive discovery + /// found more than one and therefore returned no target. Empty when the root resolves + /// unambiguously or holds no solution at all, which lets a caller tell AMBIGUITY apart from + /// ABSENCE — reports both as a null target. + /// Implements [SCRIPT-DEGRADE]. + /// + internal static string[] FindAmbiguousSolutions(string workspacePath) + { + if (!Directory.Exists(workspacePath) || FindExplicitOrRootMatch(workspacePath) is not null) + { + return []; + } + + var solutionFiles = EnumerateSolutionFiles(workspacePath, SearchOption.AllDirectories); + return solutionFiles.Length > 1 ? solutionFiles : []; + } + private static string? FindRecursiveMatch(string workspacePath) { if (!Directory.Exists(workspacePath)) diff --git a/sidecars/SharpLsp.Sidecar.CSharp/Workspace/WorkspaceManager.SingleFile.cs b/sidecars/SharpLsp.Sidecar.CSharp/Workspace/WorkspaceManager.SingleFile.cs index 9720534b..c2d22661 100644 --- a/sidecars/SharpLsp.Sidecar.CSharp/Workspace/WorkspaceManager.SingleFile.cs +++ b/sidecars/SharpLsp.Sidecar.CSharp/Workspace/WorkspaceManager.SingleFile.cs @@ -105,8 +105,7 @@ private async Task LoadClosureAsync( CancellationToken ct ) { - _adhocWorkspace?.Dispose(); - _adhocWorkspace = new AdhocWorkspace(); + _adhocWorkspace ??= new AdhocWorkspace(); var project = _adhocWorkspace.AddProject(BuildProjectInfo(kind, rootPath)); foreach (var file in closure.Files) diff --git a/sidecars/SharpLsp.Sidecar.CSharp/Workspace/WorkspaceManager.cs b/sidecars/SharpLsp.Sidecar.CSharp/Workspace/WorkspaceManager.cs index a6d40d4f..c8758ae7 100644 --- a/sidecars/SharpLsp.Sidecar.CSharp/Workspace/WorkspaceManager.cs +++ b/sidecars/SharpLsp.Sidecar.CSharp/Workspace/WorkspaceManager.cs @@ -51,12 +51,14 @@ internal sealed partial class WorkspaceManager : IDisposable { private MSBuildWorkspace? _workspace; private Solution? _solution; + private bool _isProjectlessDirectory; private readonly CodeActionResolver _codeActionResolver = new(); // Roslyn's Solution is immutable; mutating _solution = _solution.WithX(...) // is a non-atomic read-modify-write. Concurrent didChange and workspace-load // mutations would drop edits, leaving Roslyn with stale text. private readonly SemaphoreSlim _solutionMutationLock = new(1, 1); + private readonly SemaphoreSlim _projectlessLoadLock = new(1, 1); // Distinct workspace-load failure summaries already logged. MSBuild reports // the same type-load failure once per project, so de-duplicating prevents the @@ -68,6 +70,7 @@ public void Dispose() _workspace?.Dispose(); _adhocWorkspace?.Dispose(); _solutionMutationLock.Dispose(); + _projectlessLoadLock.Dispose(); } // Pending text edits keyed by file path that arrived BEFORE the workspace @@ -108,6 +111,28 @@ public async Task UpdateDocumentTextAsync( { try { + if (_isProjectlessDirectory) + { + await _projectlessLoadLock.WaitAsync(ct).ConfigureAwait(false); + try + { + var document = await FindDocumentAsync(filePath, ct).ConfigureAwait(false); + if (document is null) + { + var initResult = await OpenProjectlessAsync(filePath, ct) + .ConfigureAwait(false); + if (initResult.IsError) + { + return initResult; + } + } + } + finally + { + _ = _projectlessLoadLock.Release(); + } + } + await _solutionMutationLock.WaitAsync(ct).ConfigureAwait(false); try { @@ -555,6 +580,17 @@ public Task GetDocumentHighlightsAsync( ); } + // Names every candidate so the user can copy one straight into sharplsp.toml, and names + // the setting so the message is actionable rather than merely descriptive. + // Implements [SCRIPT-DEGRADE] and [WORKSPACE-SOLUTION-PATH]. + private static string AmbiguousSolutionMessage(string path, string[] candidates) + { + var names = string.Join(", ", candidates.Select(Path.GetFileName)); + return $"Found {candidates.Length} solutions under '{path}' ({names}), so which one to " + + "load is ambiguous. Set `csharp.solution_path` in sharplsp.toml to the solution " + + "you want, relative to the workspace root."; + } + private async Task OpenCoreAsync(string path, CancellationToken ct) { _loggedWorkspaceFailures.Clear(); @@ -569,6 +605,26 @@ private async Task OpenCoreAsync(string path, CancellationToken ct) if (target is null) { + if (Directory.Exists(path)) + { + // Ambiguity is not absence. Discovery also returns "no target" when the root + // holds SEVERAL solutions and it refused to guess; deferring there would + // analyse a real repository as loose files, resolving no project reference + // and reporting phantom diagnostics across the whole tree. Surface the + // choice instead. Implements [SCRIPT-DEGRADE]. + var candidates = SolutionLoader.FindAmbiguousSolutions(path); + if (candidates.Length > 0) + { + return VoidResult.Failure(AmbiguousSolutionMessage(path, candidates)); + } + + // The host eagerly opened a workspace folder, but it contains no projects. + // We cannot load a directory as a file-based app. Instead, we succeed + // initialization but defer actual workspace creation until a file is opened. + _isProjectlessDirectory = true; + return new VoidResult.Ok(Unit.Value); + } + // No solution or project owns this path: load it as a file-based app or script. // Implements [SCRIPT-DETECT]. return await OpenProjectlessAsync(path, ct).ConfigureAwait(false); diff --git a/sidecars/SharpLsp.Sidecar.Common.Tests/MetadataDecompilerTests.cs b/sidecars/SharpLsp.Sidecar.Common.Tests/MetadataDecompilerTests.cs index 038e3a5b..8d7c22ed 100644 --- a/sidecars/SharpLsp.Sidecar.Common.Tests/MetadataDecompilerTests.cs +++ b/sidecars/SharpLsp.Sidecar.Common.Tests/MetadataDecompilerTests.cs @@ -61,6 +61,27 @@ public void DecompileTypeToFile_sanitizes_special_characters_in_display_name() Assert.False(fileName.Contains(' ', StringComparison.Ordinal), fileName); } + [Fact] + public void DecompileTypeToFile_sanitizes_colons_identically_on_every_platform() + { + // `Path.GetInvalidFileNameChars()` omits ':' on Unix (it returns only + // { '\0', '/' } there), so a sanitizer that delegates to it alone strips + // `global::`-style names on Windows and leaves them intact on Linux. The + // decompiled file name must not depend on the host OS, so this asserts the + // exact name rather than merely the absence of a colon. + // Implements [DEFINITION-CROSSLANG]. + var path = MetadataDecompiler.DecompileTypeToFile( + CoreLib, + "System.Int32", + "global::System.Int32" + ); + + Assert.NotNull(path); + var fileName = Path.GetFileName(path!); + Assert.False(fileName.Contains(':', StringComparison.Ordinal), fileName); + Assert.Equal("global__System.Int32.cs", fileName); + } + [Fact] public void DecompileTypeToFile_returns_null_for_a_missing_assembly() { diff --git a/sidecars/SharpLsp.Sidecar.Common/MetadataDecompiler.cs b/sidecars/SharpLsp.Sidecar.Common/MetadataDecompiler.cs index cbd419d4..56b4e1fd 100644 --- a/sidecars/SharpLsp.Sidecar.Common/MetadataDecompiler.cs +++ b/sidecars/SharpLsp.Sidecar.Common/MetadataDecompiler.cs @@ -80,13 +80,39 @@ private static string WriteToTempFile(string displayName, string source) return filePath; } + /// + /// Every character neutralised in a decompiled file name. The leading literals + /// are replaced on every platform so the same type yields the same + /// file name everywhere: is + /// platform-specific — on Unix it is only { '\0', '/' } — so relying on + /// it alone would leave <, >, :, , and spaces + /// intact on Linux while stripping them on Windows. + /// + private static readonly char[] UnsafeNameChars = + [ + '<', + '>', + ':', + ',', + ' ', + .. Path.GetInvalidFileNameChars(), + ]; + private static string SanitizeFileName(string name) { - return name.Replace('<', '_') - .Replace('>', '_') - .Replace(',', '_') - .Replace(' ', '_') - .Replace(':', '_'); + return string.Create( + name.Length, + name, + static (destination, source) => + { + for (var index = 0; index < source.Length; index++) + { + var candidate = source[index]; + destination[index] = + Array.IndexOf(UnsafeNameChars, candidate) >= 0 ? '_' : candidate; + } + } + ); } /// diff --git a/src/config.rs b/src/config.rs index 2ecc013e..4aec8902 100644 --- a/src/config.rs +++ b/src/config.rs @@ -7,7 +7,7 @@ use std::path::{Path, PathBuf}; use anyhow::{Context, Result}; use serde::Deserialize; -use tracing::info; +use tracing::{info, warn}; /// Top-level configuration loaded from `sharplsp.toml`. #[derive(Debug, Clone, Default, Deserialize)] @@ -85,6 +85,45 @@ impl Default for CSharpConfig { } } +impl CSharpConfig { + /// The path to hand the C# sidecar's `workspace/open`. + /// + /// A workspace root holding more than one `.sln`/`.slnx` is ambiguous, and + /// the sidecar's recursive discovery deliberately refuses to guess — it + /// returns no target, and the whole solution fails to load. `solution_path` + /// is how the user resolves that: it names the solution to open, absolute + /// or relative to the workspace root. + /// + /// Falls back to the root — restoring plain auto-discovery — when unset, or + /// when the configured path names no existing file. Implements + /// [WORKSPACE-SOLUTION-PATH]. + pub fn open_target(&self, workspace_root: &Path) -> PathBuf { + let configured = self.solution_path.trim(); + if configured.is_empty() { + return workspace_root.to_path_buf(); + } + + let candidate = Path::new(configured); + let resolved = if candidate.is_absolute() { + candidate.to_path_buf() + } else { + workspace_root.join(candidate) + }; + + if resolved.is_file() { + info!("Opening configured solution {}", resolved.display()); + return resolved; + } + + warn!( + "csharp.solution_path `{configured}` does not name an existing file ({}); \ + falling back to workspace-root discovery", + resolved.display() + ); + workspace_root.to_path_buf() + } +} + impl Default for FSharpConfig { fn default() -> Self { Self { enabled: true } @@ -247,6 +286,85 @@ project_filter = ["MyApp.Core", "MyApp.Api"] assert!(config.fsharp.enabled); } + /// A configured `solution_path` must be the path opened, not the workspace + /// root. Sending the root leaves the sidecar to rediscover, and in a root + /// holding several solutions that discovery is ambiguous and loads nothing — + /// no hover, no completions, no diagnostics. Implements + /// [WORKSPACE-SOLUTION-PATH]. + #[test] + fn test_relative_solution_path_is_opened_not_workspace_root() { + let dir = tempfile::tempdir().unwrap(); + let root = dir.path(); + let app = root.join("app"); + fs::create_dir_all(&app).unwrap(); + let sln = app.join("App.sln"); + fs::write(&sln, "").unwrap(); + // A second solution elsewhere is what makes discovery ambiguous. + fs::create_dir_all(root.join("other")).unwrap(); + fs::write(root.join("other").join("Other.sln"), "").unwrap(); + + let config = CSharpConfig { + enabled: true, + solution_path: "app/App.sln".to_string(), + }; + + assert_eq!(config.open_target(root), sln); + } + + #[test] + fn test_absolute_solution_path_is_opened() { + let dir = tempfile::tempdir().unwrap(); + let root = dir.path(); + let sln = root.join("Explicit.sln"); + fs::write(&sln, "").unwrap(); + + let config = CSharpConfig { + enabled: true, + solution_path: sln.to_string_lossy().to_string(), + }; + + assert_eq!(config.open_target(root), sln); + } + + #[test] + fn test_empty_solution_path_falls_back_to_workspace_root() { + let dir = tempfile::tempdir().unwrap(); + let root = dir.path(); + + assert_eq!(CSharpConfig::default().open_target(root), root); + } + + /// A stale or misspelled `solution_path` must not wedge the sidecar on a + /// path that does not exist — auto-discovery is the safer fallback. + #[test] + fn test_missing_solution_path_falls_back_to_workspace_root() { + let dir = tempfile::tempdir().unwrap(); + let root = dir.path(); + + let config = CSharpConfig { + enabled: true, + solution_path: "does/not/exist.sln".to_string(), + }; + + assert_eq!(config.open_target(root), root); + } + + /// A directory is not a solution; treat it as unset rather than opening it + /// as though it were a file the user chose. + #[test] + fn test_directory_solution_path_falls_back_to_workspace_root() { + let dir = tempfile::tempdir().unwrap(); + let root = dir.path(); + fs::create_dir_all(root.join("app")).unwrap(); + + let config = CSharpConfig { + enabled: true, + solution_path: "app".to_string(), + }; + + assert_eq!(config.open_target(root), root); + } + #[test] fn test_unknown_fields_rejected() { let toml_str = r" diff --git a/src/main.rs b/src/main.rs index 8a32ac84..348863ae 100644 --- a/src/main.rs +++ b/src/main.rs @@ -229,10 +229,17 @@ fn run_server() -> Result<()> { // declare a false crash, and kill the sidecar before the solution is loaded. // When no workspace root exists (single-file mode), workspace/open is // deferred until the first didOpen notification. + // The C# sidecar opens the configured solution when `csharp.solution_path` + // names one; a root holding several solutions is otherwise ambiguous and + // loads nothing. Implements [WORKSPACE-SOLUTION-PATH]. + let csharp_open_root = workspace_root + .as_deref() + .map(|root| sharplsp_config.csharp.open_target(root)); + if workspace_root.is_some() { start_sidecar( csharp_sidecar.as_ref(), - workspace_root.as_ref(), + csharp_open_root.as_ref(), Some((&sharplsp_config.diagnostics, &connection)), sharplsp_config.analyzers.clone(), &runtime, diff --git a/src/vfs.rs b/src/vfs.rs index 57d95066..b50eb99c 100644 --- a/src/vfs.rs +++ b/src/vfs.rs @@ -15,6 +15,19 @@ pub struct DocumentState { pub content: String, /// LSP document version counter. pub version: i32, + /// Canonical spelling of the document's native path, resolved once when the + /// document is opened. + /// + /// Editors and sidecars spell the same file differently. VS Code keeps + /// whatever path the user opened — on Windows frequently the 8.3 short form, + /// `C:\Users\RUNNER~1\...` — while .NET's `Path.GetFullPath` expands short + /// names, so the solution model driving Solution Explorer reports + /// `C:\Users\runneradmin\...`. Neither spelling can be derived from the + /// other by string manipulation, so the canonical form is resolved here: + /// once per open, rather than once per lookup. `None` when the URI has no + /// readable on-disk counterpart, such as an unsaved `untitled:` buffer. + /// Implements [SE-LIVE-BUFFER] (GitHub #191). + canonical_path: Option, } impl Vfs { @@ -27,11 +40,13 @@ impl Vfs { /// Open a document (textDocument/didOpen). pub fn open(&self, uri: Uri, version: i32, text: String) { + let canonical_path = canonical_native_path(&uri); let _ = self.documents.insert( uri, DocumentState { content: text, version, + canonical_path, }, ); } @@ -62,19 +77,23 @@ impl Vfs { /// `file:///C:/dir%20name/f.cs` — so rebuilding a URI from a path and /// matching it as a string misses open documents. Instead each stored /// URI is normalized to a native path and the paths are compared. - /// [GitHub #110] + /// Implements [SE-LIVE-BUFFER] (GitHub #110). pub fn get_content_for_path(&self, path: &str) -> Option { self.documents.iter().find_map(|entry| { - let doc_path = crate::utils::uri_to_path(entry.key().as_str()).ok()?; - native_paths_equal(&doc_path, path).then(|| entry.value().content.clone()) + let doc = entry.value(); + document_denotes_path(entry.key(), doc, path).then(|| doc.content.clone()) }) } /// Like [`Vfs::get_content_for_path`], but retries with the canonicalized - /// path when the direct comparison misses. Canonicalization unifies path - /// spellings the string comparison cannot: symlinks (`/tmp` → `/private/tmp` - /// on macOS), Windows 8.3 short names (`RUNNER~1` vs `runneradmin`), `..` - /// components, and mapped drives. [GitHub #110] + /// *incoming* path when the direct comparison misses, covering a caller that + /// spells the path less directly than the editor did — `..` components, + /// mapped drives, or a short name where the editor held the long one. + /// + /// The mirror image, where the editor holds the less direct spelling, is + /// handled by [`DocumentState::canonical_path`] instead. Both halves are + /// needed: canonicalizing one side alone leaves the other's alias + /// unmatched. Implements [SE-LIVE-BUFFER] (GitHub #110, #191). pub fn get_content_for_path_canonical(&self, path: &str) -> Option { self.get_content_for_path(path).or_else(|| { let canonical = std::fs::canonicalize(path).ok()?; @@ -86,7 +105,7 @@ impl Vfs { /// open (trying canonical path spellings too), else the on-disk text. /// Every feature that consumes file content by native path must prefer /// the buffer — sorting or analyzing yesterday's save corrupts the - /// user's unsaved edits. [GitHub #110] + /// user's unsaved edits. Implements [SE-LIVE-BUFFER] (GitHub #110). pub fn read_live_or_disk(&self, file_path: &str) -> anyhow::Result { use anyhow::Context; if let Some(content) = self.get_content_for_path_canonical(file_path) { @@ -107,6 +126,35 @@ impl Vfs { } } +/// Whether an open document denotes the file at `path`. +/// +/// Both spellings the VFS knows are compared: the one the editor sent, and the +/// canonical one resolved when the document was opened. Comparing only the +/// editor's spelling misses whenever another component resolved the same file +/// differently — the Windows 8.3 short/long split above all — and the caller +/// then silently analyses stale disk content instead of the live buffer. +/// Implements [SE-LIVE-BUFFER] (GitHub #191). +fn document_denotes_path(uri: &Uri, doc: &DocumentState, path: &str) -> bool { + let canonical_matches = doc + .canonical_path + .as_deref() + .is_some_and(|canonical| native_paths_equal(canonical, path)); + + canonical_matches + || crate::utils::uri_to_path(uri.as_str()) + .is_ok_and(|doc_path| native_paths_equal(&doc_path, path)) +} + +/// Resolve a document URI to the canonical spelling of its native path, when the +/// file exists on disk. The verbatim prefix `std::fs::canonicalize` adds is +/// stripped up front so the result compares directly against the plain paths +/// editors and sidecars supply. Implements [SE-LIVE-BUFFER] (GitHub #191). +fn canonical_native_path(uri: &Uri) -> Option { + let path = crate::utils::uri_to_path(uri.as_str()).ok()?; + let canonical = std::fs::canonicalize(path).ok()?; + Some(strip_verbatim(&canonical.to_string_lossy()).into_owned()) +} + /// Compare two native paths for equality. Windows verbatim (`\\?\`) prefixes /// are ignored and the comparison is case-insensitive on Windows, where the /// filesystem is too: editors lowercase the drive letter (`c:`) while @@ -183,4 +231,47 @@ mod tests { "an indirect path spelling must still find the open buffer" ); } + + /// The editor and the sidecar disagree on how to spell the same file. VS + /// Code keeps a document under the path the user opened it by — on Windows + /// CI that is the 8.3 short form, `C:\Users\RUNNER~1\...` — while the .NET + /// solution model returns `Path.GetFullPath`, which expands short names to + /// `C:\Users\runneradmin\...`. Canonicalizing only the *incoming* path + /// cannot bridge that: the spelling the editor stored has to be resolved + /// too. When the lookup misses, `read_live_or_disk` silently falls back to + /// disk and every feature reading files by path — Solution Explorer above + /// all — analyses the last save instead of the live buffer. + /// + /// A symlink reproduces the same aliasing on platforms without 8.3 names, + /// so this runs on the Ubuntu shards that gate the suite. + /// Implements [SE-LIVE-BUFFER] (GitHub #191). + #[cfg(unix)] + #[test] + fn read_live_or_disk_finds_a_buffer_opened_under_an_aliased_path() { + let tmp = tempfile::tempdir().unwrap(); + let real_dir = tmp.path().join("real"); + std::fs::create_dir_all(&real_dir).unwrap(); + let file = real_dir.join("Program.cs"); + std::fs::write(&file, "class OnDisk {}").unwrap(); + + let alias_dir = tmp.path().join("alias"); + std::os::unix::fs::symlink(&real_dir, &alias_dir).unwrap(); + + // The editor opened the document through the alias … + let vfs = Vfs::new(); + let uri: Uri = url::Url::from_file_path(alias_dir.join("Program.cs")) + .unwrap() + .to_string() + .parse() + .unwrap(); + vfs.open(uri, 1, "class InBuffer {}".to_string()); + + // … while workspace symbols walks the project and finds the real one. + let found = vfs.read_live_or_disk(&file.to_string_lossy()).unwrap(); + assert_eq!( + found, "class InBuffer {}", + "an open buffer must be found under every spelling of its path, or \ + features silently analyse stale disk content" + ); + } } diff --git a/src/workspace_symbols.rs b/src/workspace_symbols.rs index f9564c26..c8c04a87 100644 --- a/src/workspace_symbols.rs +++ b/src/workspace_symbols.rs @@ -529,6 +529,7 @@ fn is_source_file(path: &Path) -> bool { /// Parse a single source file and extract symbols. /// Prefers VFS content (unsaved buffer) over disk for open documents. +/// Implements [SE-LIVE-BUFFER]. fn parse_file_symbols(file_path: &str, parsers: &TsParsers, vfs: &Vfs) -> Result { let source = vfs.read_live_or_disk(file_path)?; diff --git a/tests/e2e_modules/mod.rs b/tests/e2e_modules/mod.rs index 2fe3c5ff..b6d564a4 100644 --- a/tests/e2e_modules/mod.rs +++ b/tests/e2e_modules/mod.rs @@ -59,6 +59,7 @@ pub mod inlay_hints_tests; pub mod lifecycle; pub mod logging; pub mod lsp_features; +pub mod multi_solution; pub mod nuget_unused_full_stack; pub mod profiler; pub mod profiler_dump_analysis_full_stack; diff --git a/tests/e2e_modules/multi_solution.rs b/tests/e2e_modules/multi_solution.rs new file mode 100644 index 00000000..e402dda3 --- /dev/null +++ b/tests/e2e_modules/multi_solution.rs @@ -0,0 +1,116 @@ +use super::*; + +// ── Multi-solution workspace roots ──────────────────────────────── +// +// A workspace root holding more than one `.sln`/`.slnx` is ambiguous: the C# +// sidecar's recursive discovery deliberately refuses to guess which one to +// load. `sharplsp.toml`'s `csharp.solution_path` is the documented way to +// resolve that ambiguity. Implements [WORKSPACE-SOLUTION-PATH]. + +/// Two solutions in sibling subdirectories — the shape of every real monorepo, +/// and of the `SharpLsp` repo itself. `sharplsp.toml` names the one to load. +/// +/// Returns `(tmp, root_uri, app_file_uri, app_source)`. +fn create_multi_solution_workspace() -> (tempfile::TempDir, String, String, String) { + let tmp = tempfile::tempdir().unwrap(); + let root = tmp.path(); + + let app_dir = root.join("app").join("App"); + let other_dir = root.join("other").join("Other"); + std::fs::create_dir_all(&app_dir).unwrap(); + std::fs::create_dir_all(&other_dir).unwrap(); + + std::fs::write(app_dir.join("App.csproj"), library_csproj()).unwrap(); + std::fs::write(other_dir.join("Other.csproj"), library_csproj()).unwrap(); + + let app_source = r"namespace App; + +public class Calculator +{ + public int Add(int a, int b) { return a + b; } +} +"; + std::fs::write(app_dir.join("Calculator.cs"), app_source).unwrap(); + std::fs::write( + other_dir.join("Decoy.cs"), + "namespace Other;\npublic class Decoy { }\n", + ) + .unwrap(); + + std::fs::write( + root.join("app").join("App.sln"), + solution_referencing("App", "App/App.csproj"), + ) + .unwrap(); + std::fs::write( + root.join("other").join("Other.sln"), + solution_referencing("Other", "Other/Other.csproj"), + ) + .unwrap(); + + // The knob under test: name the solution to load, relative to the root. + std::fs::write( + root.join("sharplsp.toml"), + "[csharp]\nsolution_path = \"app/App.sln\"\n", + ) + .unwrap(); + + restore_project(&app_dir); + + let real_root = std::fs::canonicalize(root).unwrap(); + let root_uri = path_to_file_uri(&real_root); + let file_uri = path_to_file_uri(&real_root.join("app").join("App").join("Calculator.cs")); + (tmp, root_uri, file_uri, app_source.to_string()) +} + +fn library_csproj() -> &'static str { + r#" + + net9.0 + Library + enable + enable + +"# +} + +fn solution_referencing(name: &str, relative_csproj: &str) -> String { + format!( + r#"Microsoft Visual Studio Solution File, Format Version 12.00 +Project("{{FAE04EC0-301F-11D3-BF4B-00C04F79EFBC}}") = "{name}", "{relative_csproj}", "{{00000000-0000-0000-0000-000000000001}}" +EndProject +Global +EndGlobal"# + ) +} + +/// Hover must return content when the workspace root holds several solutions +/// and `sharplsp.toml` says which one to load. +/// +/// Without an explicit `solution_path`, recursive discovery finds two `.sln` +/// files, refuses to pick one, and the C# sidecar reports +/// `No .sln, .slnx, or .csproj found at or under ''` — no solution loads +/// and every semantic request returns null. Implements +/// [WORKSPACE-SOLUTION-PATH]. +#[test] +fn test_full_stack_hover_uses_configured_solution_path_in_multi_solution_root() { + require_dotnet(); + + let (_tmp, root_uri, file_uri, source) = create_multi_solution_workspace(); + + let mut client = LspClient::start_verbose(); + let _ = client.initialize_with_root(json!(root_uri)); + client.open_document(&file_uri, &source); + + // Hover on the "Calculator" class name (line 2, char 14). + let result = poll_hover_until_ready(&mut client, &file_uri, 2, 14, Duration::from_secs(90)); + + let value = result["contents"]["value"].as_str().unwrap(); + assert!( + value.contains("Calculator"), + "hover on class must mention Calculator, got: {value}", + ); + + client.shutdown_and_exit(); + client.wait_with_timeout(); +} diff --git a/website/src/docs/configuration.md b/website/src/docs/configuration.md index 57fa619b..31d0e29d 100644 --- a/website/src/docs/configuration.md +++ b/website/src/docs/configuration.md @@ -33,7 +33,9 @@ debounce_ms = 150 # Enable the C# sidecar enabled = true -# Path to the .sln file to load. Empty = auto-detect. +# Path to the .sln/.slnx file to load, absolute or relative to the workspace +# root. Empty = auto-detect. Required when the root holds more than one +# solution: auto-detection refuses to guess between them and loads nothing. solution_path = "" # ─── F# ──────────────────────────────────────────────────────────────────────── diff --git a/website/src/ja/docs/configuration.md b/website/src/ja/docs/configuration.md index dd406077..5611d2c5 100644 --- a/website/src/ja/docs/configuration.md +++ b/website/src/ja/docs/configuration.md @@ -34,7 +34,9 @@ debounce_ms = 150 # C# サイドカーを有効化 enabled = true -# 読み込む .sln ファイルのパス。空 = 自動検出 +# 読み込む .sln/.slnx ファイルのパス。絶対パスまたはワークスペースルートからの +# 相対パス。空 = 自動検出。ルートに複数のソリューションがある場合は必須です。 +# 自動検出はどれを選ぶか推測せず、何も読み込みません。 solution_path = "" # ─── F# ──────────────────────────────────────────────────────────────────────── diff --git a/website/src/zh/docs/configuration.md b/website/src/zh/docs/configuration.md index 13b36eb2..14471b9f 100644 --- a/website/src/zh/docs/configuration.md +++ b/website/src/zh/docs/configuration.md @@ -34,7 +34,9 @@ debounce_ms = 150 # 启用 C# sidecar enabled = true -# 要加载的 .sln 文件路径。空字符串 = 自动检测。 +# 要加载的 .sln/.slnx 文件路径，可为绝对路径或相对于工作区根目录的路径。 +# 空字符串 = 自动检测。当根目录下存在多个解决方案时必须设置： +# 自动检测不会在它们之间猜测，将不加载任何内容。 solution_path = "" # ─── F# ────────────────────────────────────────────────────────────────────────