From f8afc417e5d92f2317a4e5d6842cd0154c4757dd Mon Sep 17 00:00:00 2001 From: Jimmy Byrd Date: Tue, 23 Apr 2024 20:37:36 -0400 Subject: [PATCH 1/2] Add support for Cancel WorkDoneProgress --- Directory.Build.props | 4 +- paket.dependencies | 2 +- paket.lock | 2 +- .../LspServers/AdaptiveFSharpLspServer.fs | 14 ++- .../LspServers/AdaptiveServerState.fs | 107 ++++++++++++++++-- .../LspServers/AdaptiveServerState.fsi | 8 ++ src/FsAutoComplete/LspServers/Common.fs | 2 +- .../LspServers/FSharpLspClient.fs | 39 +++++-- .../LspServers/FSharpLspClient.fsi | 34 +++++- .../CompletionTests.fs | 68 +++++------ .../EmptyFileTests.fs | 4 +- 11 files changed, 212 insertions(+), 72 deletions(-) diff --git a/Directory.Build.props b/Directory.Build.props index ed5de172f..8edd45739 100644 --- a/Directory.Build.props +++ b/Directory.Build.props @@ -7,8 +7,8 @@ $(NoWarn);3186,0042 $(NoWarn);NU1902 - $(WarnOn);1182 + $(WarnOn);1182 + $(NoWarn);FS0044 $(WarnOn);3390 true $(MSBuildThisFileDirectory)CHANGELOG.md diff --git a/paket.dependencies b/paket.dependencies index 65369732b..31cbcdaf1 100644 --- a/paket.dependencies +++ b/paket.dependencies @@ -52,7 +52,7 @@ nuget Expecto.Diff nuget YoloDev.Expecto.TestSdk nuget AltCover nuget GitHubActionsTestLogger -nuget Ionide.LanguageServerProtocol >= 0.4.20 +nuget Ionide.LanguageServerProtocol >= 0.4.23 nuget Microsoft.Extensions.Caching.Memory nuget OpenTelemetry.Api >= 1.3.2 nuget OpenTelemetry.Exporter.OpenTelemetryProtocol >= 1.3.2 # 1.4 bumps to 7.0 versions of System.Diagnostics libs, so can't use it diff --git a/paket.lock b/paket.lock index fe151928a..19c5cc39f 100644 --- a/paket.lock +++ b/paket.lock @@ -133,7 +133,7 @@ NUGET System.Reflection.Metadata (>= 5.0) Ionide.Analyzers (0.10) Ionide.KeepAChangelog.Tasks (0.1.8) - copy_local: true - Ionide.LanguageServerProtocol (0.4.20) + Ionide.LanguageServerProtocol (0.4.23) FSharp.Core (>= 6.0) Newtonsoft.Json (>= 13.0.1) StreamJsonRpc (>= 2.16.36) diff --git a/src/FsAutoComplete/LspServers/AdaptiveFSharpLspServer.fs b/src/FsAutoComplete/LspServers/AdaptiveFSharpLspServer.fs index fcbdb86fc..0dd29c212 100644 --- a/src/FsAutoComplete/LspServers/AdaptiveFSharpLspServer.fs +++ b/src/FsAutoComplete/LspServers/AdaptiveFSharpLspServer.fs @@ -578,8 +578,8 @@ type AdaptiveFSharpLspServer // Otherwise we'll fail here and our retry logic will come into place do! match p.Context with - | Some({ triggerKind = CompletionTriggerKind.TriggerCharacter } as context) -> - volatileFile.Source.TryGetChar pos = context.triggerCharacter + | Some({ TriggerKind = CompletionTriggerKind.TriggerCharacter } as context) -> + volatileFile.Source.TryGetChar pos = context.TriggerCharacter | _ -> true |> Result.requireTrue $"TextDocumentCompletion was sent before TextDocumentDidChange" @@ -2960,25 +2960,27 @@ type AdaptiveFSharpLspServer override x.Dispose() = disposables.Dispose() - member this.WorkDoneProgressCancel(token: ProgressToken) : Async = + member this.WorkDoneProgressCancel(param: WorkDoneProgressCancelParams) : Async = async { - let tags = [ "ProgressToken", box token ] + let tags = [ "WorkDoneProgressCancelParams", box param ] use trace = fsacActivitySource.StartActivityForType(thisType, tags = tags) try logger.info ( Log.setMessage "WorkDoneProgressCancel Request: {params}" - >> Log.addContextDestructured "params" token + >> Log.addContextDestructured "params" param.token ) + state.CancelServerProgress param.token + with e -> trace |> Tracing.recordException e logException e (Log.setMessage "WorkDoneProgressCancel Request Errored {p}" - >> Log.addContextDestructured "token" token) + >> Log.addContextDestructured "token" param.token) return () } diff --git a/src/FsAutoComplete/LspServers/AdaptiveServerState.fs b/src/FsAutoComplete/LspServers/AdaptiveServerState.fs index 82020dad9..ceb56f3a4 100644 --- a/src/FsAutoComplete/LspServers/AdaptiveServerState.fs +++ b/src/FsAutoComplete/LspServers/AdaptiveServerState.fs @@ -3,6 +3,7 @@ namespace FsAutoComplete.Lsp open System open System.IO open System.Threading +open System.Collections.Generic open FsAutoComplete open FsAutoComplete.CodeFix open FsAutoComplete.Logging @@ -37,6 +38,68 @@ open FsAutoComplete.Lsp open FsAutoComplete.Lsp.Helpers +/// Handle tracking in-flight ServerProgressReport and allow cancellation of actions if a client decides to. +type ServerProgressLookup() = + // This is a dictionary of all the progress reports that are currently active + // Use a WeakReference to avoid memory leaks + let progressTable = Dictionary>() + + // Although it's small data, we don't want to keep it around forever + let timer = + new Timers.Timer((TimeSpan.FromMinutes 1.).TotalMilliseconds, AutoReset = true) + + let timerSub = + timer.Elapsed.Subscribe(fun _ -> + lock progressTable (fun () -> + let derefs = + progressTable + |> Seq.filter (fun (KeyValue(_, reporters)) -> + match reporters.TryGetTarget() with + | (true, _) -> false + | _ -> true) + |> Seq.toList + + for (KeyValue(tokens, _)) in derefs do + progressTable.Remove(tokens) |> ignore)) + + + /// Creates a ServerProgressReport and keeps track of it. + /// The FSharpLspClient to communicate to the client with. + /// Optional token. It will be generated otherwise. + /// Informs that the ServerProgressReport is cancellable to the client. + /// + member x.CreateProgressReport(lspClient: FSharpLspClient, ?token: ProgressToken, ?cancellable: bool) = + let progress = + new ServerProgressReport(lspClient, ?token = token, ?cancellableDefault = cancellable) + + lock progressTable (fun () -> + progressTable.Add(progress.ProgressToken, new WeakReference(progress))) + + progress + + + /// Signal a ServerProgressReport to Cancel it's CancellationTokenSource. + /// The ProgressToken used to identify the ServerProgressReport + /// + /// + /// See LSP Spec on WorkDoneProgress Cancel for more information. + /// + member x.Cancel(token: ProgressToken) = + lock progressTable (fun () -> + match progressTable.TryGetValue(token) with + | true, weakRef -> + match weakRef.TryGetTarget() with + | true, progress -> + progress.Cancel() + progressTable.Remove(token) |> ignore + | _ -> () + | _ -> ()) + + interface IDisposable with + member x.Dispose() = + timerSub.Dispose() + timer.Dispose() + [] type WorkspaceChosen = | Projs of HashSet> @@ -88,6 +151,8 @@ type AdaptiveState(lspClient: FSharpLspClient, sourceTextFactory: ISourceTextFac let logger = LogProvider.getLoggerFor () let thisType = typeof let disposables = new Disposables.CompositeDisposable() + let progressLookup = new ServerProgressLookup() + do disposables.Add progressLookup let projectSelector = cval (FindFirstProject()) @@ -307,10 +372,12 @@ type AdaptiveState(lspClient: FSharpLspClient, sourceTextFactory: ISourceTextFac let checkUnusedOpens = async { try - use progress = new ServerProgressReport(lspClient) + use progress = progressLookup.CreateProgressReport(lspClient, cancellable = true) do! progress.Begin($"Checking unused opens {fileName}...", message = filePathUntag) - let! unused = UnusedOpens.getUnusedOpens (tyRes.GetCheckResults, getSourceLine) + let! unused = + UnusedOpens.getUnusedOpens (tyRes.GetCheckResults, getSourceLine) + |> Async.withCancellation progress.CancellationToken let! ct = Async.CancellationToken notifications.Trigger(NotificationEvent.UnusedOpens(filePath, (unused |> List.toArray), file.Version), ct) @@ -321,11 +388,15 @@ type AdaptiveState(lspClient: FSharpLspClient, sourceTextFactory: ISourceTextFac let checkUnusedDeclarations = async { try - use progress = new ServerProgressReport(lspClient) + use progress = progressLookup.CreateProgressReport(lspClient, cancellable = true) do! progress.Begin($"Checking unused declarations {fileName}...", message = filePathUntag) let isScript = Utils.isAScript (filePathUntag) - let! unused = UnusedDeclarations.getUnusedDeclarations (tyRes.GetCheckResults, isScript) + + let! unused = + UnusedDeclarations.getUnusedDeclarations (tyRes.GetCheckResults, isScript) + |> Async.withCancellation progress.CancellationToken + let unused = unused |> Seq.toArray let! ct = Async.CancellationToken @@ -337,10 +408,13 @@ type AdaptiveState(lspClient: FSharpLspClient, sourceTextFactory: ISourceTextFac let checkSimplifiedNames = async { try - use progress = new ServerProgressReport(lspClient) + use progress = progressLookup.CreateProgressReport(lspClient, cancellable = true) do! progress.Begin($"Checking simplifying of names {fileName}...", message = filePathUntag) - let! simplified = SimplifyNames.getSimplifiableNames (tyRes.GetCheckResults, getSourceLine) + let! simplified = + SimplifyNames.getSimplifiableNames (tyRes.GetCheckResults, getSourceLine) + |> Async.withCancellation progress.CancellationToken + let simplified = Array.ofSeq simplified let! ct = Async.CancellationToken notifications.Trigger(NotificationEvent.SimplifyNames(filePath, simplified, file.Version), ct) @@ -351,10 +425,13 @@ type AdaptiveState(lspClient: FSharpLspClient, sourceTextFactory: ISourceTextFac let checkUnnecessaryParentheses = async { try - use progress = new ServerProgressReport(lspClient) + use progress = progressLookup.CreateProgressReport(lspClient, cancellable = true) do! progress.Begin($"Checking for unnecessary parentheses {fileName}...", message = filePathUntag) - let! unnecessaryParentheses = UnnecessaryParentheses.getUnnecessaryParentheses getSourceLine tyRes.GetAST + let! unnecessaryParentheses = + UnnecessaryParentheses.getUnnecessaryParentheses getSourceLine tyRes.GetAST + |> Async.withCancellation progress.CancellationToken + let! ct = Async.CancellationToken @@ -1369,9 +1446,9 @@ type AdaptiveState(lspClient: FSharpLspClient, sourceTextFactory: ISourceTextFac >> Log.addContextDestructured "date" (file.LastTouched) ) - let! ct = Async.CancellationToken - use progressReport = new ServerProgressReport(lspClient) + use progressReport = + progressLookup.CreateProgressReport(lspClient, cancellable = true) let simpleName = Path.GetFileName(UMX.untag file.Source.FileName) do! progressReport.Begin($"Typechecking {simpleName}", message = $"{file.Source.FileName}") @@ -1384,10 +1461,12 @@ type AdaptiveState(lspClient: FSharpLspClient, sourceTextFactory: ISourceTextFac options, shouldCache = shouldCache ) - |> Debug.measureAsync $"checker.ParseAndCheckFileInProject - {file.Source.FileName}" + |> Async.withCancellation progressReport.CancellationToken do! progressReport.End($"Typechecked {file.Source.FileName}") + let! ct = Async.CancellationToken + notifications.Trigger(NotificationEvent.FileParsed(file.Source.FileName), ct) match result with @@ -2035,7 +2114,8 @@ type AdaptiveState(lspClient: FSharpLspClient, sourceTextFactory: ISourceTextFac let mutable checksCompleted = 0 - use progressReporter = new ServerProgressReport(lspClient) + use progressReporter = + progressLookup.CreateProgressReport(lspClient, cancellable = true) let percentage numerator denominator = if denominator = 0 then @@ -2219,6 +2299,9 @@ type AdaptiveState(lspClient: FSharpLspClient, sourceTextFactory: ISourceTextFac member x.GlyphToSymbolKind = glyphToSymbolKind |> AVal.force + member x.CancelServerProgress(progressToken: ProgressToken) = progressLookup.Cancel progressToken + + interface IDisposable with member this.Dispose() = diff --git a/src/FsAutoComplete/LspServers/AdaptiveServerState.fsi b/src/FsAutoComplete/LspServers/AdaptiveServerState.fsi index 3ed4e9ebf..b9a72d4af 100644 --- a/src/FsAutoComplete/LspServers/AdaptiveServerState.fsi +++ b/src/FsAutoComplete/LspServers/AdaptiveServerState.fsi @@ -118,4 +118,12 @@ type AdaptiveState = member GetDeclarations: filename: string -> Async> member GetAllDeclarations: unit -> Async<(string * NavigationTopLevelDeclaration array) array> member GlyphToSymbolKind: (FSharpGlyph -> SymbolKind option) + /// + /// Signals the server to cancel an operation that is associated with the given progress token. + /// + /// + /// + /// See LSP Spec on WorkDoneProgress Cancel for more information. + /// + member CancelServerProgress: progressToken: ProgressToken -> unit interface IDisposable diff --git a/src/FsAutoComplete/LspServers/Common.fs b/src/FsAutoComplete/LspServers/Common.fs index 60d6e1534..5882b9a80 100644 --- a/src/FsAutoComplete/LspServers/Common.fs +++ b/src/FsAutoComplete/LspServers/Common.fs @@ -158,7 +158,7 @@ module Async = let! ct2 = Async.CancellationToken use cts = CancellationTokenSource.CreateLinkedTokenSource(ct, ct2) let tcs = new TaskCompletionSource<'a>() - use _reg = cts.Token.Register(fun () -> tcs.TrySetCanceled() |> ignore) + use _reg = cts.Token.Register(fun () -> tcs.TrySetCanceled(cts.Token) |> ignore) let a = async { diff --git a/src/FsAutoComplete/LspServers/FSharpLspClient.fs b/src/FsAutoComplete/LspServers/FSharpLspClient.fs index cbd1e2fcb..c8c183f9b 100644 --- a/src/FsAutoComplete/LspServers/FSharpLspClient.fs +++ b/src/FsAutoComplete/LspServers/FSharpLspClient.fs @@ -79,8 +79,6 @@ type FSharpLspClient(sendServerNotification: ClientNotificationSender, sendServe let progress: ProgressParams<_> = { token = token; value = value } sendServerNotification "$/progress" (box progress) |> Async.Ignore - - /// /// An awaitable wrapper around a task whose result is disposable. The wrapper is not disposable, so this prevents usage errors like "use _lock = myAsync()" when the appropriate usage should be "use! _lock = myAsync())". /// @@ -114,21 +112,31 @@ module private SemaphoreSlimExtensions = } ) -type ServerProgressReport(lspClient: FSharpLspClient, ?token: ProgressToken) = +type ServerProgressReport(lspClient: FSharpLspClient, ?token: ProgressToken, ?cancellableDefault: bool) = let mutable canReportProgress = false let mutable endSent = false let locker = new SemaphoreSlim(1, 1) + let cts = new CancellationTokenSource() + + member val ProgressToken = defaultArg token (ProgressToken.Second((Guid.NewGuid().ToString()))) + + member val CancellationToken = cts.Token + + member x.Cancel() = + try + cts.Cancel() + with _ -> + () - member val Token = defaultArg token (ProgressToken.Second((Guid.NewGuid().ToString()))) member x.Begin(title, ?cancellable, ?message, ?percentage) = cancellableTask { use! __ = fun ct -> locker.LockAsync(ct) if not endSent then - let! result = lspClient.WorkDoneProgressCreate x.Token + let! result = lspClient.WorkDoneProgressCreate x.ProgressToken match result with | Ok() -> canReportProgress <- true @@ -137,10 +145,10 @@ type ServerProgressReport(lspClient: FSharpLspClient, ?token: ProgressToken) = if canReportProgress then do! lspClient.Progress( - x.Token, + x.ProgressToken, WorkDoneProgressBegin.Create( title, - ?cancellable = cancellable, + ?cancellable = (cancellable |> Option.orElse cancellableDefault), ?message = message, ?percentage = percentage ) @@ -154,8 +162,12 @@ type ServerProgressReport(lspClient: FSharpLspClient, ?token: ProgressToken) = if canReportProgress && not endSent then do! lspClient.Progress( - x.Token, - WorkDoneProgressReport.Create(?cancellable = cancellable, ?message = message, ?percentage = percentage) + x.ProgressToken, + WorkDoneProgressReport.Create( + ?cancellable = (cancellable |> Option.orElse cancellableDefault), + ?message = message, + ?percentage = percentage + ) ) } @@ -165,12 +177,17 @@ type ServerProgressReport(lspClient: FSharpLspClient, ?token: ProgressToken) = let stillNeedsToSend = canReportProgress && not endSent if stillNeedsToSend then - do! lspClient.Progress(x.Token, WorkDoneProgressEnd.Create(?message = message)) + do! lspClient.Progress(x.ProgressToken, WorkDoneProgressEnd.Create(?message = message)) endSent <- true } interface IAsyncDisposable with - member x.DisposeAsync() = task { do! x.End () CancellationToken.None } |> ValueTask + member x.DisposeAsync() = + task { + cts.Dispose() + do! x.End () CancellationToken.None + } + |> ValueTask interface IDisposable with member x.Dispose() = (x :> IAsyncDisposable).DisposeAsync() |> ignore diff --git a/src/FsAutoComplete/LspServers/FSharpLspClient.fsi b/src/FsAutoComplete/LspServers/FSharpLspClient.fsi index 984844d7c..b9a182f97 100644 --- a/src/FsAutoComplete/LspServers/FSharpLspClient.fsi +++ b/src/FsAutoComplete/LspServers/FSharpLspClient.fsi @@ -5,6 +5,7 @@ open Ionide.LanguageServerProtocol.Server open Ionide.LanguageServerProtocol.Types open FsAutoComplete.LspHelpers open System +open System.Threading open IcedTasks type FSharpLspClient = @@ -34,11 +35,40 @@ type FSharpLspClient = override WorkDoneProgressCreate: ProgressToken -> AsyncLspResult override Progress: ProgressToken * 'Progress -> Async +/// +/// Represents a progress report that can be used to report progress to the client. +/// +/// +/// +/// This implements and to allow for the ending of the progress report without explicitly calling End. +/// +/// See LSP Spec on WorkDoneProgress for more information. +/// type ServerProgressReport = - new: lspClient: FSharpLspClient * ?token: ProgressToken -> ServerProgressReport - member Token: ProgressToken + new: lspClient: FSharpLspClient * ?token: ProgressToken * ?cancellableDefault: bool -> ServerProgressReport + /// The progress token to identify the progress report. + member ProgressToken: ProgressToken + /// A cancellation token that can be used to used to cancel actions that are associated with this progress report. + member CancellationToken: CancellationToken + /// Triggers the CancellationToken to cancel. + member Cancel: unit -> unit + /// Used to start reporting progress to the client. + /// Mandatory title of the progress operation + /// Controls if a cancel button should show to allow the user to cancel the long running operation + /// more detailed associated progress message. Contains complementary information to the `title`. + /// percentage to display (value 100 is considered 100%). If not provided infinite progress is assumed member Begin: title: string * ?cancellable: bool * ?message: string * ?percentage: uint -> CancellableTask + /// Report additional progress + /// Controls if a cancel button should show to allow the user to cancel the long running operation + /// more detailed associated progress message. Contains complementary information to the `title`. + /// percentage to display (value 100 is considered 100%). If not provided infinite progress is assumed member Report: ?cancellable: bool * ?message: string * ?percentage: uint -> CancellableTask + /// Signaling the end of a progress reporting is done. + /// more detailed associated progress message. Contains complementary information to the `title`. + /// + /// This will be called if this object is disposed either via Dispose or DisposeAsync. + /// + /// member End: ?message: string -> CancellableTask interface IAsyncDisposable interface IDisposable diff --git a/test/FsAutoComplete.Tests.Lsp/CompletionTests.fs b/test/FsAutoComplete.Tests.Lsp/CompletionTests.fs index 5fd468981..ada300e9c 100644 --- a/test/FsAutoComplete.Tests.Lsp/CompletionTests.fs +++ b/test/FsAutoComplete.Tests.Lsp/CompletionTests.fs @@ -41,8 +41,8 @@ let tests state = Position = { Line = 3; Character = 9 } // the '.' in 'Async.' Context = Some - { triggerKind = CompletionTriggerKind.TriggerCharacter - triggerCharacter = Some '.' } } + { TriggerKind = CompletionTriggerKind.TriggerCharacter + TriggerCharacter = Some '.' } } let! response = server.TextDocumentCompletion completionParams @@ -94,8 +94,8 @@ let tests state = Character = character + 1 } Context = Some - { triggerKind = CompletionTriggerKind.TriggerCharacter - triggerCharacter = Some '.' } } + { TriggerKind = CompletionTriggerKind.TriggerCharacter + TriggerCharacter = Some '.' } } let! response = server.TextDocumentCompletion completionParams do! c @@ -131,8 +131,8 @@ let tests state = Position = { Line = line; Character = character } // the '.' in 'GetDirectoryName().' Context = Some - { triggerKind = CompletionTriggerKind.TriggerCharacter - triggerCharacter = Some '.' } } + { TriggerKind = CompletionTriggerKind.TriggerCharacter + TriggerCharacter = Some '.' } } let! response = server.TextDocumentCompletion completionParams @@ -183,8 +183,8 @@ let tests state = Character = character + 1 } Context = Some - { triggerKind = CompletionTriggerKind.TriggerCharacter - triggerCharacter = Some '.' } } + { TriggerKind = CompletionTriggerKind.TriggerCharacter + TriggerCharacter = Some '.' } } let! response = server.TextDocumentCompletion completionParams do! c @@ -220,8 +220,8 @@ let tests state = Position = { Line = line; Character = character } Context = Some - { triggerKind = CompletionTriggerKind.TriggerCharacter - triggerCharacter = Some '.' } } + { TriggerKind = CompletionTriggerKind.TriggerCharacter + TriggerCharacter = Some '.' } } let! response = server.TextDocumentCompletion completionParams @@ -272,8 +272,8 @@ let tests state = Character = character + 1 } Context = Some - { triggerKind = CompletionTriggerKind.TriggerCharacter - triggerCharacter = Some '.' } } + { TriggerKind = CompletionTriggerKind.TriggerCharacter + TriggerCharacter = Some '.' } } let! response = server.TextDocumentCompletion completionParams do! c @@ -309,8 +309,8 @@ let tests state = Position = { Line = line; Character = character } Context = Some - { triggerKind = CompletionTriggerKind.TriggerCharacter - triggerCharacter = Some '.' } } + { TriggerKind = CompletionTriggerKind.TriggerCharacter + TriggerCharacter = Some '.' } } let! response = server.TextDocumentCompletion completionParams @@ -341,8 +341,8 @@ let tests state = Position = { Line = 6; Character = 5 } // the '.' in 'List.' Context = Some - { triggerKind = CompletionTriggerKind.TriggerCharacter - triggerCharacter = Some '.' } } + { TriggerKind = CompletionTriggerKind.TriggerCharacter + TriggerCharacter = Some '.' } } let! response = server.TextDocumentCompletion completionParams @@ -369,8 +369,8 @@ let tests state = Position = { Line = 8; Character = 16 } // the '.' in 'List.' Context = Some - { triggerKind = CompletionTriggerKind.TriggerCharacter - triggerCharacter = Some '.' } } + { TriggerKind = CompletionTriggerKind.TriggerCharacter + TriggerCharacter = Some '.' } } let! response = server.TextDocumentCompletion completionParams @@ -396,8 +396,8 @@ let tests state = Position = { Line = 11; Character = 10 } // after Lis partial type name in Id record field declaration Context = Some - { triggerKind = CompletionTriggerKind.Invoked - triggerCharacter = None } } + { TriggerKind = CompletionTriggerKind.Invoked + TriggerCharacter = None } } let! response = server.TextDocumentCompletion completionParams @@ -428,8 +428,8 @@ let tests state = Position = { Line = 8; Character = 12 } // after the 'L' in 'List.' Context = Some - { triggerKind = CompletionTriggerKind.Invoked - triggerCharacter = None } } + { TriggerKind = CompletionTriggerKind.Invoked + TriggerCharacter = None } } let! response = server.TextDocumentCompletion completionParams @@ -456,8 +456,8 @@ let tests state = Position = { Line = 8; Character = 11 } // before the 'L' in 'List.' Context = Some - { triggerKind = CompletionTriggerKind.Invoked - triggerCharacter = None } } + { TriggerKind = CompletionTriggerKind.Invoked + TriggerCharacter = None } } let! response = server.TextDocumentCompletion completionParams @@ -484,8 +484,8 @@ let tests state = Position = { Line = 3; Character = 9 } // the '.' in 'Async.' Context = Some - { triggerKind = CompletionTriggerKind.TriggerCharacter - triggerCharacter = Some '.' } } + { TriggerKind = CompletionTriggerKind.TriggerCharacter + TriggerCharacter = Some '.' } } let! response = server.TextDocumentCompletion completionParams |> AsyncResult.map Option.get let ctokMember = response.Items[0] @@ -514,8 +514,8 @@ let tests state = Position = { Line = 23; Character = 8 } // the '.' in 'List.' Context = Some - { triggerKind = CompletionTriggerKind.TriggerCharacter - triggerCharacter = Some '.' } } + { TriggerKind = CompletionTriggerKind.TriggerCharacter + TriggerCharacter = Some '.' } } let! response = server.TextDocumentCompletion completionParams @@ -542,8 +542,8 @@ let tests state = Position = { Line = 24; Character = 9 } // the '.' in 'List.' Context = Some - { triggerKind = CompletionTriggerKind.TriggerCharacter - triggerCharacter = Some '.' } } + { TriggerKind = CompletionTriggerKind.TriggerCharacter + TriggerCharacter = Some '.' } } let! response = server.TextDocumentCompletion completionParams @@ -1017,8 +1017,8 @@ let fullNameExternalAutocompleteTest state = Position = { Line = line; Character = character } Context = Some - { triggerKind = CompletionTriggerKind.Invoked - triggerCharacter = None } } + { TriggerKind = CompletionTriggerKind.Invoked + TriggerCharacter = None } } let! res = server.TextDocumentCompletion p @@ -1049,8 +1049,8 @@ let fullNameExternalAutocompleteTest state = Position = { Line = 3; Character = 4 } Context = Some - { triggerKind = CompletionTriggerKind.Invoked - triggerCharacter = None } } + { TriggerKind = CompletionTriggerKind.Invoked + TriggerCharacter = None } } let! response = server.TextDocumentCompletion p |> AsyncResult.map Option.get diff --git a/test/FsAutoComplete.Tests.Lsp/EmptyFileTests.fs b/test/FsAutoComplete.Tests.Lsp/EmptyFileTests.fs index e5095e16e..85bccf9f8 100644 --- a/test/FsAutoComplete.Tests.Lsp/EmptyFileTests.fs +++ b/test/FsAutoComplete.Tests.Lsp/EmptyFileTests.fs @@ -53,8 +53,8 @@ let tests state = Position = { Line = 0; Character = 0 } Context = Some - { triggerKind = CompletionTriggerKind.Invoked - triggerCharacter = None } } + { TriggerKind = CompletionTriggerKind.Invoked + TriggerCharacter = None } } match! server.TextDocumentCompletion completionParams with | Ok (Some _) -> failtest "An empty file has empty completions" From 570691a279c66f1a65c8ffdb4931621a9d223a2a Mon Sep 17 00:00:00 2001 From: Jimmy Byrd Date: Tue, 23 Apr 2024 20:53:43 -0400 Subject: [PATCH 2/2] Fix up saving cancellation --- .../LspServers/AdaptiveServerState.fs | 82 ++++++++++++------- .../EmptyFileTests.fs | 4 +- 2 files changed, 54 insertions(+), 32 deletions(-) diff --git a/src/FsAutoComplete/LspServers/AdaptiveServerState.fs b/src/FsAutoComplete/LspServers/AdaptiveServerState.fs index ceb56f3a4..cf414a2bb 100644 --- a/src/FsAutoComplete/LspServers/AdaptiveServerState.fs +++ b/src/FsAutoComplete/LspServers/AdaptiveServerState.fs @@ -1121,13 +1121,13 @@ type AdaptiveState(lspClient: FSharpLspClient, sourceTextFactory: ISourceTextFac }) - let cancelToken filePath (cts: CancellationTokenSource) = + let cancelToken filePath version (cts: CancellationTokenSource) = try logger.info ( Log.setMessage "Cancelling {filePath} - {version}" >> Log.addContextDestructured "filePath" filePath - // >> Log.addContextDestructured "version" oldFile.Version + >> Log.addContextDestructured "version" version ) cts.Cancel() @@ -1138,16 +1138,14 @@ type AdaptiveState(lspClient: FSharpLspClient, sourceTextFactory: ISourceTextFac // ignore if already cancelled () - let resetCancellationToken (filePath: string) = - + let resetCancellationToken (filePath: string) version = let adder _ = new CancellationTokenSource() let updater _key value = - cancelToken filePath value + cancelToken filePath version value new CancellationTokenSource() - openFilesTokens.AddOrUpdate(filePath, adder, updater) - |> ignore + openFilesTokens.AddOrUpdate(filePath, adder, updater).Token let updateOpenFiles (file: VolatileFile) = @@ -1155,14 +1153,16 @@ type AdaptiveState(lspClient: FSharpLspClient, sourceTextFactory: ISourceTextFac let updater _ (v: cval<_>) = v.Value <- file - resetCancellationToken file.FileName + resetCancellationToken file.FileName file.Version |> ignore transact (fun () -> openFiles.AddOrElse(file.Source.FileName, adder, updater)) - let updateTextChanges filePath p = + let updateTextChanges filePath ((changes: DidChangeTextDocumentParams, _) as p) = let adder _ = cset<_> [ p ] let updater _ (v: cset<_>) = v.Add p |> ignore - resetCancellationToken filePath + resetCancellationToken filePath changes.TextDocument.Version + |> ignore + transact (fun () -> textChanges.AddOrElse(filePath, adder, updater)) let isFileOpen file = openFiles |> AMap.tryFindA file |> AVal.map (Option.isSome) @@ -2046,7 +2046,7 @@ type AdaptiveState(lspClient: FSharpLspClient, sourceTextFactory: ISourceTextFac openFiles.Remove filePath |> ignore match openFilesTokens.TryRemove(filePath) with - | (true, cts) -> cancelToken filePath cts + | (true, cts) -> cancelToken filePath None cts | _ -> () textChanges.Remove filePath |> ignore) @@ -2096,13 +2096,15 @@ type AdaptiveState(lspClient: FSharpLspClient, sourceTextFactory: ISourceTextFac } - let bypassAdaptiveAndCheckDependenciesForFile (filePath: string) = + let bypassAdaptiveAndCheckDependenciesForFile (sourceFilePath: string) = async { - let tags = [ SemanticConventions.fsac_sourceCodePath, box (UMX.untag filePath) ] + let tags = + [ SemanticConventions.fsac_sourceCodePath, box (UMX.untag sourceFilePath) ] + use _ = fsacActivitySource.StartActivityForType(thisType, tags = tags) - let! dependentFiles = getDependentFilesForFile filePath + let! dependentFiles = getDependentFilesForFile sourceFilePath - let! projs = getProjectOptionsForFile filePath |> AsyncAVal.forceAsync + let! projs = getProjectOptionsForFile sourceFilePath |> AsyncAVal.forceAsync let projs = projs |> Result.toOption |> Option.defaultValue [] let dependentProjects = @@ -2131,26 +2133,46 @@ type AdaptiveState(lspClient: FSharpLspClient, sourceTextFactory: ISourceTextFac && file.Contains "AssemblyAttributes.fs" |> not) let checksToPerformLength = innerChecks.Length + let rootToken = sourceFilePath |> getOpenFileTokenOrDefault innerChecks - |> Array.map (fun (proj, file) -> - let file = UMX.tag file + |> Array.map (fun (opts, file) -> + let fileTagged = Utils.normalizePath file + + async { + + use joinedToken = + if fileTagged = sourceFilePath then + // dont reset the token for the incoming file as it would cancel the whole operation + CancellationTokenSource.CreateLinkedTokenSource(rootToken, progressReporter.CancellationToken) + else + // only cancel other files + // If we have multiple saves from separate root files we want only one to be running + let token = resetCancellationToken fileTagged None + // and join with the root token as well since we want to cancel the whole operation if the root files changes + CancellationTokenSource.CreateLinkedTokenSource(rootToken, token, progressReporter.CancellationToken) + + try + let! _ = + bypassAdaptiveTypeCheck fileTagged opts + |> Async.withCancellation joinedToken.Token + + () + with :? OperationCanceledException -> + // if a file shows up multiple times in the list such as Microsoft.NET.Test.Sdk.Program.fs we may cancel it but we don't want to stop the whole operation for it + () + + let checksCompleted = Interlocked.Increment(&checksCompleted) + + do! + progressReporter.Report( + message = $"{checksCompleted}/{checksToPerformLength} remaining", + percentage = percentage checksCompleted checksToPerformLength + ) + }) - let token = getOpenFileTokenOrDefault filePath - bypassAdaptiveTypeCheck (file) (proj) - |> Async.withCancellation token - |> Async.Ignore - |> Async.bind (fun _ -> - async { - let checksCompleted = Interlocked.Increment(&checksCompleted) - do! - progressReporter.Report( - message = $"{checksCompleted}/{checksToPerformLength} remaining", - percentage = percentage checksCompleted checksToPerformLength - ) - })) do! diff --git a/test/FsAutoComplete.Tests.Lsp/EmptyFileTests.fs b/test/FsAutoComplete.Tests.Lsp/EmptyFileTests.fs index 85bccf9f8..7e9b959bc 100644 --- a/test/FsAutoComplete.Tests.Lsp/EmptyFileTests.fs +++ b/test/FsAutoComplete.Tests.Lsp/EmptyFileTests.fs @@ -82,8 +82,8 @@ let tests state = Position = { Line = 0; Character = 1 } Context = Some - { triggerKind = CompletionTriggerKind.Invoked - triggerCharacter = None } + { TriggerKind = CompletionTriggerKind.Invoked + TriggerCharacter = None } } |> Async.StartChild let! compilerResults = waitForCompilerDiagnosticsForFile "EmptyFile.fsx" events |> Async.StartChild