diff --git a/AGENTS.md b/AGENTS.md index 6151e552..1437bac1 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -51,11 +51,28 @@ The `--no-build` flag skips recompiling the test project, which means type error code are silently ignored and the stale binary from a previous build is used instead. There is no meaningful performance win from skipping the build step. +## Anonymize private code in plans, prose, and examples + +When writing anything that will be committed or otherwise persisted (`plans/*.md`, +design docs, PR descriptions, code comments) and it draws on a real session against a +user's private solution — namespaces, type names, file names, log excerpts — replace +those with fictional equivalents (e.g. `Fabrikam.Processing`, `ExchangeRateSource`, +`MiscDataCleanupHandlers.cs`) before committing. Keep the *shape* of the example intact +(same structure, same character offsets/prefixes where they matter to the point being +made) so the technical content is unaffected — only the identifying names change. This +does not apply to this project's own public identifiers (namespaces under +`CSharpLanguageServer.*`, file paths under `src/`, GitHub issue/PR numbers on this repo) +since those aren't private. + ## Code Style & Commit Conventions See [CONTRIBUTING.md](CONTRIBUTING.md#code-style). -Commit messages should be a short single-line summary — no bullet lists of individual changes. +Commit messages use [Conventional Commits](https://www.conventionalcommits.org/) style — +`(): ` (e.g. `fix(completion): avoid NullReferenceException on +empty import list`) — as a short single-line summary, no bullet lists of individual changes. +See [CONTRIBUTING.md#commit-messages](CONTRIBUTING.md#commit-messages) for the full list of +`type`s and more examples. ## Key Docs diff --git a/CHANGELOG.md b/CHANGELOG.md index 53a14881..dc0720de 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -3,6 +3,20 @@ All notable changes to this project will be documented in this file. The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/). +## [Unreleased] +* Re-enable completion suggestions for types from unimported namespaces, now inserting the + missing `using` (via `additionalTextEdits`) when one of those items is accepted, gated behind + the new `csharp.completion.completeUnimportedTypes` setting, defaulting to `true`; also add + `csharp.completion.showNameSuggestions` to control whether `textDocument/completion` includes + suggested identifier names (e.g. `stringBuilder` for a `StringBuilder`), defaulting to `false` + and matching the previous hardcoded behavior + - Reported by @kervanaslan in https://github.com/razzmatazz/csharp-language-server/issues/210 and + fixed by @razzmatazz in https://github.com/razzmatazz/csharp-language-server/pull/425 +* Stop reporting `textDocument/completion` results as incomplete, which was forcing editors to + re-request the full completion list (including the unimported-namespace scan above) on every + keystroke instead of filtering the already-returned list locally; this could produce a large, + redundant `textDocument/completion` response per character typed on large solutions + ## [0.28.0] - 2026-09-13 / Klebiškis * Speed up analyzer-enabled diagnostics by sharing project-wide analyzer results across document requests for the same Roslyn solution snapshot - Reported and fixed by @alsi-lawr in https://github.com/razzmatazz/csharp-language-server/issues/403 and https://github.com/razzmatazz/csharp-language-server/pull/404 diff --git a/README.md b/README.md index 05a43fc7..e28f6816 100644 --- a/README.md +++ b/README.md @@ -70,6 +70,17 @@ Settings are read from the `csharp` workspace configuration section - `csharp.razorSupport` - enable Razor (`.cshtml`) document support; defaults to `false` +- `csharp.completion.completeUnimportedTypes` - include types from namespaces + that aren't yet imported in `textDocument/completion` results, inserting the + missing `using` (via `additionalTextEdits`) when one of those items is + accepted; may increase completion latency on large solutions; defaults to + `true` + +- `csharp.completion.showNameSuggestions` - include suggested identifier names + (e.g. `stringBuilder` when completing after `StringBuilder `) in + `textDocument/completion` results; may increase completion latency; + defaults to `false` + - `csharp.solutionPathOverride` - override the solution path to load; useful for specifying an alternative solution when multiple exist in the workspace; can also be set via the `--solution` CLI flag; defaults to `null` diff --git a/src/CSharpLanguageServer/Handlers/Completion.fs b/src/CSharpLanguageServer/Handlers/Completion.fs index c38ee88a..1fa2c47d 100644 --- a/src/CSharpLanguageServer/Handlers/Completion.fs +++ b/src/CSharpLanguageServer/Handlers/Completion.fs @@ -128,6 +128,29 @@ module Completion = member __.GetDescriptionAsync(doc, item, ct) = service.GetDescriptionAsync(doc, item, ct) + /// Builds Roslyn completion options from the effective `csharp.completion.*` configuration. + /// `ShowItemsFromUnimportedNamespaces` defaults to `true` (re-enabling suggestions for + /// types whose namespace isn't imported yet — https://github.com/razzmatazz/csharp-language-server/issues/210). + /// Selecting one of those items commits as a complex text edit (see `IsComplexTextEdit` below) + /// that both inserts the type name and adds the missing `using` — `completeUnimportedTypes` + /// controls both behaviors together, since offering an item that doesn't compile without an + /// import the user has to add by hand isn't very useful on its own. + /// `ShowNameSuggestions` defaults to `false` since it's rarely useful over LSP and adds latency. + let private getRoslynCompletionOptions (context: RequestContext) = + let completeUnimportedTypes = + context.Config.completion + |> Option.bind _.completeUnimportedTypes + |> Option.defaultValue true + + let showNameSuggestions = + context.Config.completion + |> Option.bind _.showNameSuggestions + |> Option.defaultValue false + + RoslynCompletionOptions.Default() + |> _.WithBool("ShowItemsFromUnimportedNamespaces", completeUnimportedTypes) + |> _.WithBool("ShowNameSuggestions", showNameSuggestions) + let private dynamicRegistration (cc: ClientCapabilities) = cc.TextDocument |> Option.bind _.Completion @@ -313,10 +336,7 @@ module Completion = Microsoft.CodeAnalysis.Completion.CompletionService.GetService(doc) |> RoslynCompletionServiceWrapper - let completionOptions = - RoslynCompletionOptions.Default() - |> _.WithBool("ShowItemsFromUnimportedNamespaces", false) - |> _.WithBool("ShowNameSuggestions", false) + let completionOptions = context |> getRoslynCompletionOptions let completionTrigger = p.Context |> codeActionContextToCompletionTrigger @@ -355,10 +375,7 @@ module Completion = Microsoft.CodeAnalysis.Completion.CompletionService.GetService(doc) |> RoslynCompletionServiceWrapper - let completionOptions = - RoslynCompletionOptions.Default() - |> _.WithBool("ShowItemsFromUnimportedNamespaces", false) - |> _.WithBool("ShowNameSuggestions", false) + let completionOptions = context |> getRoslynCompletionOptions let completionTrigger = p.Context |> codeActionContextToCompletionTrigger @@ -417,7 +434,14 @@ module Completion = lspCompletionItemsWithCacheInfo |> Array.map (fun (item, _, _, _) -> item) return - { IsIncomplete = true + // Roslyn's `CompletionList` has no concept of "there's more, ask again" — it + // always returns its full candidate set for the current cursor context in one + // call, unfiltered by the typed prefix (LSP expects the client to filter + // `items` locally using `filterText` as the user keeps typing). Reporting + // `IsIncomplete = true` here would instead make the client re-request on every + // keystroke via `TriggerForIncompleteCompletions`, redoing this same + // (potentially expensive, e.g. unimported-namespace) computation each time. + { IsIncomplete = false Items = items ItemDefaults = None } |> U2.C2 @@ -426,6 +450,42 @@ module Completion = LspWorkspaceUpdate.Empty } + /// For completion items that need more than inserting `DisplayText` at their original + /// `Span` on commit (`IsComplexTextEdit`) — e.g. an item from an unimported namespace, + /// which also needs a `using` inserted at the top of the file — resolves the full set of + /// changes Roslyn would make and returns everything other than the primary in-place edit + /// as LSP `AdditionalTextEdits`, so accepting the item also adds the missing import. + let private getAdditionalTextEditsForComplexItem + (completionService: Microsoft.CodeAnalysis.Completion.CompletionService) + (doc: Document) + (item: Microsoft.CodeAnalysis.Completion.CompletionItem) + (ct: System.Threading.CancellationToken) + : Async = + async { + if not item.IsComplexTextEdit then + return None + else + let! change = + completionService.GetChangeAsync(doc, item, System.Nullable(), ct) + |> Async.AwaitTask + + if change.TextChanges.IsDefaultOrEmpty then + return None + else + let! sourceText = doc.GetTextAsync(ct) |> Async.AwaitTask + + let additionalEdits = + change.TextChanges + |> Seq.filter (fun tc -> tc.Span <> item.Span) + |> Seq.map (TextEdit.fromTextChange sourceText.Lines) + |> Array.ofSeq + + return + match additionalEdits with + | [||] -> None + | edits -> Some edits + } + let resolve (_context: RequestContext) (item: CompletionItem) @@ -462,10 +522,14 @@ module Completion = Value = d } |> U2.C2) + let! additionalTextEdits = + getAdditionalTextEditsForComplexItem completionService doc roslynCompletionItem ct + return { item with Detail = synopsis - Documentation = updatedItemDocumentation } + Documentation = updatedItemDocumentation + AdditionalTextEdits = additionalTextEdits |> Option.orElse item.AdditionalTextEdits } |> LspResult.success, LspWorkspaceUpdate.Empty diff --git a/src/CSharpLanguageServer/Types.fs b/src/CSharpLanguageServer/Types.fs index b5275808..86dc48ef 100644 --- a/src/CSharpLanguageServer/Types.fs +++ b/src/CSharpLanguageServer/Types.fs @@ -12,12 +12,21 @@ type CSharpDebugConfiguration = { solutionLoadDelay = None debugMode = None } +type CSharpCompletionConfiguration = + { completeUnimportedTypes: bool option + showNameSuggestions: bool option } + + static member Default = + { completeUnimportedTypes = None + showNameSuggestions = None } + type CSharpConfiguration = { logLevel: string option applyFormattingOptions: bool option analyzersEnabled: bool option useMetadataUris: bool option razorSupport: bool option + completion: CSharpCompletionConfiguration option locale: string option debug: CSharpDebugConfiguration option solutionPathOverride: string option } @@ -34,6 +43,7 @@ type CSharpConfiguration = analyzersEnabled = None useMetadataUris = None razorSupport = None + completion = None locale = None debug = None solutionPathOverride = None } @@ -46,6 +56,17 @@ let mergeCSharpConfiguration (oldConfig: CSharpConfiguration) (newConfig: CSharp analyzersEnabled = newConfig.analyzersEnabled |> Option.orElse oldConfig.analyzersEnabled useMetadataUris = newConfig.useMetadataUris |> Option.orElse oldConfig.useMetadataUris razorSupport = newConfig.razorSupport |> Option.orElse oldConfig.razorSupport + completion = + match newConfig.completion with + | Some newCompletion -> + Some + { completeUnimportedTypes = + newCompletion.completeUnimportedTypes + |> Option.orElse (oldConfig.completion |> Option.bind _.completeUnimportedTypes) + showNameSuggestions = + newCompletion.showNameSuggestions + |> Option.orElse (oldConfig.completion |> Option.bind _.showNameSuggestions) } + | None -> oldConfig.completion locale = newConfig.locale |> Option.orElse oldConfig.locale debug = match newConfig.debug with diff --git a/tests/CSharpLanguageServer.Tests/CompletionTests.fs b/tests/CSharpLanguageServer.Tests/CompletionTests.fs index db85ca3a..995391d1 100644 --- a/tests/CSharpLanguageServer.Tests/CompletionTests.fs +++ b/tests/CSharpLanguageServer.Tests/CompletionTests.fs @@ -7,6 +7,7 @@ open NUnit.Framework open Ionide.LanguageServerProtocol.Types open Ionide.LanguageServerProtocol.Server +open CSharpLanguageServer.Types open CSharpLanguageServer.Tests.Tooling open CSharpLanguageServer.Tests.Fixtures @@ -38,7 +39,7 @@ let ``completion works in a .cs file`` () = match completion0 with | Some(U2.C2 cl) -> - Assert.That(cl.IsIncomplete, Is.True) + Assert.That(cl.IsIncomplete, Is.False) Assert.That(cl.ItemDefaults.IsSome, Is.False) Assert.That(cl.Items.Length, Is.EqualTo(6)) @@ -280,3 +281,214 @@ let ``completionItem/resolve handles sentinel -1 positions in textEdit`` () = let resolved: CompletionItem = client.Request("completionItem/resolve", itemJson) Assert.That(resolved.Detail, Is.EqualTo(Some "void ClassForCompletion.MethodA(string arg)")) + +[] +let ``completion suggests types from unimported namespaces by default`` () = + // https://github.com/razzmatazz/csharp-language-server/issues/210 + use client = rentFixture "genericProject" + + use classFile = client.Open("Project/UnimportedNamespaceCompletionTests.cs") + + let completionParams: CompletionParams = + { TextDocument = { Uri = classFile.Uri } + Position = { Line = 4u; Character = 16u } + WorkDoneToken = None + PartialResultToken = None + Context = None } + + // Roslyn builds the unimported-types symbol index asynchronously in the background, + // so the first request or two may not have it ready yet; retry for a bit. + let hasUnimportedItem (completion: U2 option) = + match completion with + | Some(U2.C2 cl) -> cl.Items |> Seq.exists (fun i -> i.Label = "TypeOnlyInUnimportedNamespace") + | _ -> false + + let rec pollForCompletion attemptsLeft = + let completion: U2 option = + client.Request("textDocument/completion", completionParams) + + if hasUnimportedItem completion || attemptsLeft <= 0 then + completion + else + Thread.Sleep(500) + pollForCompletion (attemptsLeft - 1) + + let completion = pollForCompletion 20 + + match completion with + | Some(U2.C2 cl) -> + let unimportedItem = + cl.Items |> Seq.tryFind (fun i -> i.Label = "TypeOnlyInUnimportedNamespace") + + Assert.That( + unimportedItem.IsSome, + Is.True, + sprintf + "expected a completion item for 'TypeOnlyInUnimportedNamespace' from an unimported namespace. Got: %s" + (cl.Items |> Array.map (fun i -> i.Label) |> String.concat ", ") + ) + | _ -> failwith "Some U2.C2 was expected" + +[] +let ``completion inserts using directive when accepting an unimported-namespace item`` () = + // https://github.com/razzmatazz/csharp-language-server/issues/210 + use client = rentFixture "genericProject" + + use classFile = client.Open("Project/UnimportedNamespaceCompletionTests.cs") + + let completionParams: CompletionParams = + { TextDocument = { Uri = classFile.Uri } + Position = { Line = 4u; Character = 16u } + WorkDoneToken = None + PartialResultToken = None + Context = None } + + let hasUnimportedItem (completion: U2 option) = + match completion with + | Some(U2.C2 cl) -> cl.Items |> Seq.exists (fun i -> i.Label = "TypeOnlyInUnimportedNamespace") + | _ -> false + + let rec pollForCompletion attemptsLeft = + let completion: U2 option = + client.Request("textDocument/completion", completionParams) + + if hasUnimportedItem completion || attemptsLeft <= 0 then + completion + else + Thread.Sleep(500) + pollForCompletion (attemptsLeft - 1) + + let completion = pollForCompletion 20 + + let item = + match completion with + | Some(U2.C2 cl) -> cl.Items |> Seq.find (fun i -> i.Label = "TypeOnlyInUnimportedNamespace") + | _ -> failwith "Some U2.C2 was expected" + + let resolved: CompletionItem = client.Request("completionItem/resolve", item) + + // The primary insertion is still done via `InsertText` (no `TextEdit`), unaffected by this change. + Assert.That(resolved.InsertText, Is.EqualTo(Some "TypeOnlyInUnimportedNamespace")) + Assert.That(resolved.TextEdit.IsSome, Is.False) + + match resolved.AdditionalTextEdits with + | Some edits -> + let hasUsingEdit = + edits |> Array.exists (fun e -> e.NewText.Contains("using Project.Unimported;")) + + Assert.That( + hasUsingEdit, + Is.True, + sprintf + "expected an additional text edit inserting 'using Project.Unimported;'. Got: %s" + (edits |> Array.map (fun e -> e.NewText) |> String.concat " | ") + ) + | None -> failwith "expected AdditionalTextEdits to be Some when accepting an unimported-namespace item" + +[] +let ``completion does not suggest types from unimported namespaces when disabled via config`` () = + let profile = + { defaultClientProfile with + ServerConfig = + { defaultClientProfile.ServerConfig with + completion = + Some + { CSharpCompletionConfiguration.Default with + completeUnimportedTypes = Some false } } } + + use client = activateFixtureExt "genericProject" profile emptyFixturePatch id + + use classFile = client.Open("Project/UnimportedNamespaceCompletionTests.cs") + + let completionParams: CompletionParams = + { TextDocument = { Uri = classFile.Uri } + Position = { Line = 4u; Character = 16u } + WorkDoneToken = None + PartialResultToken = None + Context = None } + + let hasUnimportedItem (completion: U2 option) = + match completion with + | Some(U2.C2 cl) -> cl.Items |> Seq.exists (fun i -> i.Label = "TypeOnlyInUnimportedNamespace") + | _ -> false + + // Poll repeatedly (as long as the passing test above needs to see the item appear once + // Roslyn's unimported-types index has warmed up) and assert it never shows up. + for _ in 1..20 do + let completion: U2 option = + client.Request("textDocument/completion", completionParams) + + Assert.That( + hasUnimportedItem completion, + Is.False, + "did not expect a completion item for 'TypeOnlyInUnimportedNamespace' when the setting is disabled" + ) + + Thread.Sleep(500) + +[] +let ``completion does not suggest names by default`` () = + use client = rentFixture "genericProject" + + use classFile = client.Open("Project/NameSuggestionCompletionTests.cs") + + let completionParams: CompletionParams = + { TextDocument = { Uri = classFile.Uri } + Position = { Line = 4u; Character = 34u } + WorkDoneToken = None + PartialResultToken = None + Context = None } + + let completion: U2 option = + client.Request("textDocument/completion", completionParams) + + match completion with + | Some(U2.C2 cl) -> + let nameSuggestionItem = + cl.Items |> Seq.tryFind (fun i -> i.Label = "stringBuilder") + + Assert.That( + nameSuggestionItem.IsNone, + Is.True, + "did not expect a name-suggestion completion item 'stringBuilder' by default" + ) + | _ -> failwith "Some U2.C2 was expected" + +[] +let ``completion suggests names when enabled via config`` () = + let profile = + { defaultClientProfile with + ServerConfig = + { defaultClientProfile.ServerConfig with + completion = + Some + { CSharpCompletionConfiguration.Default with + showNameSuggestions = Some true } } } + + use client = activateFixtureExt "genericProject" profile emptyFixturePatch id + + use classFile = client.Open("Project/NameSuggestionCompletionTests.cs") + + let completionParams: CompletionParams = + { TextDocument = { Uri = classFile.Uri } + Position = { Line = 4u; Character = 34u } + WorkDoneToken = None + PartialResultToken = None + Context = None } + + let completion: U2 option = + client.Request("textDocument/completion", completionParams) + + match completion with + | Some(U2.C2 cl) -> + let nameSuggestionItem = + cl.Items |> Seq.tryFind (fun i -> i.Label = "stringBuilder") + + Assert.That( + nameSuggestionItem.IsSome, + Is.True, + sprintf + "expected a name-suggestion completion item 'stringBuilder'. Got: %s" + (cl.Items |> Array.map (fun i -> i.Label) |> String.concat ", ") + ) + | _ -> failwith "Some U2.C2 was expected" diff --git a/tests/CSharpLanguageServer.Tests/Fixtures/genericProject/Project/NameSuggestionCompletionTests.cs b/tests/CSharpLanguageServer.Tests/Fixtures/genericProject/Project/NameSuggestionCompletionTests.cs new file mode 100644 index 00000000..274381eb --- /dev/null +++ b/tests/CSharpLanguageServer.Tests/Fixtures/genericProject/Project/NameSuggestionCompletionTests.cs @@ -0,0 +1,7 @@ +class NameSuggestionCompletionTests +{ + public void MethodA() + { + System.Text.StringBuilder + } +} diff --git a/tests/CSharpLanguageServer.Tests/Fixtures/genericProject/Project/TypeOnlyInUnimportedNamespace.cs b/tests/CSharpLanguageServer.Tests/Fixtures/genericProject/Project/TypeOnlyInUnimportedNamespace.cs new file mode 100644 index 00000000..b34703db --- /dev/null +++ b/tests/CSharpLanguageServer.Tests/Fixtures/genericProject/Project/TypeOnlyInUnimportedNamespace.cs @@ -0,0 +1,6 @@ +namespace Project.Unimported +{ + public class TypeOnlyInUnimportedNamespace + { + } +} diff --git a/tests/CSharpLanguageServer.Tests/Fixtures/genericProject/Project/UnimportedNamespaceCompletionTests.cs b/tests/CSharpLanguageServer.Tests/Fixtures/genericProject/Project/UnimportedNamespaceCompletionTests.cs new file mode 100644 index 00000000..57b44ee5 --- /dev/null +++ b/tests/CSharpLanguageServer.Tests/Fixtures/genericProject/Project/UnimportedNamespaceCompletionTests.cs @@ -0,0 +1,7 @@ +class UnimportedNamespaceCompletionTests +{ + public void MethodA() + { + TypeOnly + } +}