diff --git a/CHANGELOG.md b/CHANGELOG.md index 890dc2548..60bf05a6a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,11 @@ # Changelog +## [0.86.1] - 2026-10-29 + +### Changed + +- [Fewer progress notifications](https://github.com/ionide/FsAutoComplete/pull/1545) - Typechecks share one "Typechecking" progress report, instead of one report per file. Its message lists the files being checked and, after a save, how many of the dependent files are done, with a percentage. Cancelling the report stops the checks of the dependent files that did not run yet. With `backgroundServiceProgress` on, the analyzers of a file share one "Analyzing" report the same way. (thanks @TheAngryByrd!) + ## [0.86.0] - 2026-10-23 ### Changed diff --git a/src/FsAutoComplete/LspServers/AdaptiveServerState.fs b/src/FsAutoComplete/LspServers/AdaptiveServerState.fs index c0537f977..8bdb81cf6 100644 --- a/src/FsAutoComplete/LspServers/AdaptiveServerState.fs +++ b/src/FsAutoComplete/LspServers/AdaptiveServerState.fs @@ -88,15 +88,19 @@ type ServerProgressLookup() = /// 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() + let progress = + lock progressTable (fun () -> + match progressTable.TryGetValue(token) with + | true, weakRef -> progressTable.Remove(token) |> ignore - | _ -> () - | _ -> ()) + + match weakRef.TryGetTarget() with + | true, progress -> Some progress + | _ -> None + | _ -> None) + + // Cancel outside the lock: the token callbacks may take other locks. + progress |> Option.iter (fun p -> p.Cancel()) interface IDisposable with member x.Dispose() = @@ -219,6 +223,25 @@ type AdaptiveState let progressLookup = new ServerProgressLookup() do disposables.Add progressLookup + let config = cval FSharpConfig.Default + + let typecheckProgressReporter = + new SharedProgressReporter( + "Typechecking", + (fun () -> progressLookup.CreateProgressReport(lspClient, cancellable = true)) + ) + + do disposables.Add typecheckProgressReporter + + let analyzerProgressReporter = + new SharedProgressReporter( + "Analyzing", + (fun () -> progressLookup.CreateProgressReport(lspClient, cancellable = true)), + isEnabled = (fun () -> (AVal.force config).Notifications.BackgroundServiceProgress) + ) + + do disposables.Add analyzerProgressReporter + let serilogLoggerFactory = new SerilogLoggerFactory(Serilog.Log.Logger) do disposables.Add serilogLoggerFactory @@ -226,8 +249,6 @@ type AdaptiveState let rootPath = cval None - let config = cval FSharpConfig.Default - let checker = config |> AVal.map (fun c -> @@ -531,84 +552,85 @@ type AdaptiveState let inline getSourceLine lineNo = (source: ISourceText).GetLineString(lineNo - 1) - let checkUnusedOpens = + let checkUnusedOpens (batch: SharedProgressBatch) = asyncEx { try let! ct = Async.CancellationToken + use _operation = batch.Begin() use! _l = analyzersLocker.LockAsync(ct) - use progress = progressLookup.CreateProgressReport(lspClient, cancellable = true) - - if config.Notifications.BackgroundServiceProgress then - do! progress.Begin($"Checking unused opens {fileName}...", message = filePathUntag) + batch.CancellationToken.ThrowIfCancellationRequested() + use _running = analyzerProgressReporter.Begin($"Unused opens - {fileName}") let! unused = UnusedOpens.getUnusedOpens (tyRes.GetCheckResults, getSourceLine) - |> Async.withCancellation progress.CancellationToken + |> Async.withCancellation batch.CancellationToken do! triggerNotificationAndWait (NotificationEvent.UnusedOpens(filePath, (unused |> List.toArray), file.Version)) ct - with e -> - logger.error (Log.setMessage "checkUnusedOpens failed" >> Log.addExn e) + with + | :? OperationCanceledException -> logger.debug (Log.setMessage "checkUnusedOpens cancelled") + | e -> logger.error (Log.setMessage "checkUnusedOpens failed" >> Log.addExn e) } - let checkUnusedDeclarations = + let checkUnusedDeclarations (batch: SharedProgressBatch) = asyncEx { try let! ct = Async.CancellationToken + use _operation = batch.Begin() use! _l = analyzersLocker.LockAsync(ct) - use progress = progressLookup.CreateProgressReport(lspClient, cancellable = true) - - if config.Notifications.BackgroundServiceProgress then - do! progress.Begin($"Checking unused declarations {fileName}...", message = filePathUntag) + batch.CancellationToken.ThrowIfCancellationRequested() + use _running = analyzerProgressReporter.Begin($"Unused declarations - {fileName}") let isScript = Utils.isAScript (filePathUntag) let! unused = UnusedDeclarations.getUnusedDeclarations (tyRes.GetCheckResults, isScript) - |> Async.withCancellation progress.CancellationToken + |> Async.withCancellation batch.CancellationToken let unused = unused |> Seq.toArray do! triggerNotificationAndWait (NotificationEvent.UnusedDeclarations(filePath, unused, file.Version)) ct - with e -> - logger.error (Log.setMessage "checkUnusedDeclarations failed" >> Log.addExn e) + with + | :? OperationCanceledException -> logger.debug (Log.setMessage "checkUnusedDeclarations cancelled") + | e -> logger.error (Log.setMessage "checkUnusedDeclarations failed" >> Log.addExn e) } - let checkSimplifiedNames = + let checkSimplifiedNames (batch: SharedProgressBatch) = asyncEx { try let! ct = Async.CancellationToken + use _operation = batch.Begin() use! _l = analyzersLocker.LockAsync(ct) - use progress = progressLookup.CreateProgressReport(lspClient, cancellable = true) - - if config.Notifications.BackgroundServiceProgress then - do! progress.Begin($"Checking simplifying of names {fileName}...", message = filePathUntag) + batch.CancellationToken.ThrowIfCancellationRequested() + use _running = analyzerProgressReporter.Begin($"Simplify names - {fileName}") let! simplified = SimplifyNames.getSimplifiableNames (tyRes.GetCheckResults, getSourceLine) - |> Async.withCancellation progress.CancellationToken + |> Async.withCancellation batch.CancellationToken let simplified = Array.ofSeq simplified do! triggerNotificationAndWait (NotificationEvent.SimplifyNames(filePath, simplified, file.Version)) ct - with e -> - logger.error (Log.setMessage "checkSimplifiedNames failed" >> Log.addExn e) + with + | :? OperationCanceledException -> logger.debug (Log.setMessage "checkSimplifiedNames cancelled") + | e -> logger.error (Log.setMessage "checkSimplifiedNames failed" >> Log.addExn e) } - let checkUnnecessaryParentheses = + let checkUnnecessaryParentheses (batch: SharedProgressBatch) = asyncEx { try let! ct = Async.CancellationToken + use _operation = batch.Begin() use! _l = analyzersLocker.LockAsync(ct) - use progress = progressLookup.CreateProgressReport(lspClient) - - if config.Notifications.BackgroundServiceProgress then - do! progress.Begin($"Checking for unnecessary parentheses {fileName}...", message = filePathUntag) + batch.CancellationToken.ThrowIfCancellationRequested() + use _running = analyzerProgressReporter.Begin($"Unnecessary parens - {fileName}") let unnecessaryParentheses = (System.Collections.Generic.HashSet(comparer = Range.comparer), tyRes.GetAST) ||> ParsedInput.fold (fun ranges path node -> + batch.CancellationToken.ThrowIfCancellationRequested() + match node with | SyntaxNode.SynExpr(SynExpr.Paren(expr = inner; rightParenRange = Some _; range = range)) when not (SynExpr.shouldBeParenthesizedInContext getSourceLine path inner) @@ -624,12 +646,15 @@ type AdaptiveState | _ -> ranges) + batch.CancellationToken.ThrowIfCancellationRequested() + do! triggerNotificationAndWait (NotificationEvent.UnnecessaryParentheses(filePath, Array.ofSeq unnecessaryParentheses, file.Version)) ct - with e -> - logger.error (Log.setMessage "checkUnnecessaryParentheses failed" >> Log.addExn e) + with + | :? OperationCanceledException -> logger.debug (Log.setMessage "checkUnnecessaryParentheses cancelled") + | e -> logger.error (Log.setMessage "checkUnnecessaryParentheses failed" >> Log.addExn e) } let inline isNotExcluded (exclusions: Regex array) = @@ -658,7 +683,17 @@ type AdaptiveState checkUnnecessaryParentheses ] - async { do! analyzers |> Async.parallel75 |> Async.Ignore } + async { + // The analyzers run one at a time behind analyzersLocker. One batch keeps them in one progress report, + // and a cancel of that report stops the analyzers that did not run yet. + use batch = analyzerProgressReporter.BeginBatch(analyzers.Length) + + do! + analyzers + |> List.map (fun check -> check batch) + |> Async.parallel75 + |> Async.Ignore + } let tryUriCreate (s: string) = match Uri.TryCreate(s, UriKind.Absolute) with @@ -678,10 +713,8 @@ type AdaptiveState let file = volatileFile.FileName try - use progress = new ServerProgressReport(lspClient) - - if config.Notifications.BackgroundServiceProgress then - do! progress.Begin("Running analyzers...", message = UMX.untag file) + let fileName = Path.GetFileName(UMX.untag file) + use progress = analyzerProgressReporter.Begin($"External analyzers - {fileName}") Loggers.analyzers.info ( Log.setMessage "begin analysis of {file}" @@ -731,6 +764,7 @@ type AdaptiveState analyzerOptions, analyzerPredicate ) + |> Async.withCancellation progress.CancellationToken let! ct = Async.CancellationToken do! triggerNotificationAndWait (NotificationEvent.AnalyzerMessage(res, file, volatileFile.Version)) ct @@ -741,8 +775,9 @@ type AdaptiveState Loggers.analyzers.info (Log.setMessageI $"missing components of {file:file} to run analyzers, skipped them") () - with ex -> - Loggers.analyzers.error (Log.setMessageI $"Run failed for {file:file}" >> Log.addExn ex) + with + | :? OperationCanceledException -> Loggers.analyzers.debug (Log.setMessageI $"Run cancelled for {file:file}") + | ex -> Loggers.analyzers.error (Log.setMessageI $"Run failed for {file:file}" >> Log.addExn ex) } @@ -1950,11 +1985,10 @@ type AdaptiveState ) - 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}") + // The token of this scope is not used: a cancelled check of an open file would stay cached until the file changes. + // The dependent-file check links the token of its batch instead. + use _progress = + typecheckProgressReporter.Begin(Path.GetFileName(UMX.untag file.Source.FileName)) let! result = match compilerOptions with @@ -2725,47 +2759,35 @@ type AdaptiveState |> List.collect (fun (proj, snap) -> snap.SourceFilesTagged |> List.map (fun sourceFile -> proj, sourceFile)) |> List.toArray - let mutable checksCompleted = 0 + let innerChecks = + Array.concat [| dependentFiles; dependentProjectsAndSourceFiles |] + |> Array.filter (fun (_, file) -> + let file = UMX.untag file - use progressReporter = - progressLookup.CreateProgressReport(lspClient, cancellable = true) + file.Contains "AssemblyInfo.fs" |> not + && file.Contains "AssemblyAttributes.fs" |> not) - let percentage numerator denominator = - if denominator = 0 then - 0u - else - ((float numerator) / (float denominator)) * 100.0 |> uint32 + use progress = typecheckProgressReporter.BeginBatch(innerChecks.Length) + let rootToken = sourceFilePath |> getOpenFileTokenOrDefault let checksToPerform = - let innerChecks = - Array.concat [| dependentFiles; dependentProjectsAndSourceFiles |] - |> Array.filter (fun (_, file) -> - let file = UMX.untag file - - file.Contains "AssemblyInfo.fs" |> not - && file.Contains "AssemblyAttributes.fs" |> not) - - let checksToPerformLength = innerChecks.Length - let rootToken = sourceFilePath |> getOpenFileTokenOrDefault - innerChecks |> Array.map (fun (proj, file) -> - async { - - use joinedToken = + asyncEx { + let fileToken = if file = sourceFilePath then // dont reset the token for the incoming file as it would cancel the whole operation - CancellationTokenSource.CreateLinkedTokenSource(rootToken, progressReporter.CancellationToken) + CancellationToken.None else // only cancel other files // If we have multiple saves from separate root files we want only one to be running - let fileToken = resetCancellationToken file 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, - fileToken, - progressReporter.CancellationToken - ) + resetCancellationToken file None + + // join with the root token as well since we want to cancel the whole operation if the root files changes + use joinedToken = + CancellationTokenSource.CreateLinkedTokenSource(rootToken, fileToken, progress.CancellationToken) + + use _fileProgress = progress.Begin() try let! _ = @@ -2773,30 +2795,23 @@ type AdaptiveState |> Async.withCancellation joinedToken.Token () - with :? OperationCanceledException -> + with :? OperationCanceledException when + fileToken.IsCancellationRequested + && not rootToken.IsCancellationRequested + && not progress.CancellationToken.IsCancellationRequested -> // 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 - ) }) - - - do! - progressReporter.Begin( - "Typechecking Dependent F# files", - message = $"0/{checksToPerform.Length} remaining", - percentage = percentage 0 checksToPerform.Length + try + do! checksToPerform |> Async.parallel75 |> Async.Ignore + with :? OperationCanceledException -> + // The root file changed again or the user cancelled the progress report. + logger.debug ( + Log.setMessage "Typechecking the dependent files of {file} was cancelled" + >> Log.addContextDestructured "file" sourceFilePath ) - do! checksToPerform |> Async.parallel75 |> Async.Ignore - } diff --git a/src/FsAutoComplete/LspServers/FSharpLspClient.fs b/src/FsAutoComplete/LspServers/FSharpLspClient.fs index d015bc88a..ea1ea25b5 100644 --- a/src/FsAutoComplete/LspServers/FSharpLspClient.fs +++ b/src/FsAutoComplete/LspServers/FSharpLspClient.fs @@ -187,6 +187,340 @@ type ServerProgressReport(lspClient: FSharpLspClient, ?token: ProgressToken, ?ca member x.Dispose() = (x :> IAsyncDisposable).DisposeAsync() |> ignore +open FsAutoComplete.Logging + +/// One operation tracked by a . Dispose it when the operation ends. +[] +type SharedProgressScope(cancellationToken: CancellationToken, onDispose: unit -> unit) = + let mutable disposed = 0 + + member _.CancellationToken = cancellationToken + + interface IDisposable with + member _.Dispose() = + if Interlocked.Exchange(&disposed, 1) = 0 then + onDispose () + +/// A batch of operations tracked by a . Dispose it when the batch ends. +[] +type SharedProgressBatch + (cancellationToken: CancellationToken, beginOperation: unit -> SharedProgressScope, onDispose: unit -> unit) = + let mutable disposed = 0 + + member _.CancellationToken = cancellationToken + + member _.Begin() = beginOperation () + + interface IDisposable with + member _.Dispose() = + if Interlocked.Exchange(&disposed, 1) = 0 then + onDispose () + +/// One progress report of a SharedProgressReporter, from its creation until its last operation ends. +/// Only access it while holding the lock of the reporter. +type SharedProgressGeneration(report: ServerProgressReport) = + member val Report = report + + /// The running operations and their labels, in the order they started. + member val Operations = Map.empty with get, set + member val Batches = 0 with get, set + /// The operations that count towards the percentage, and how many of them completed. + member val Total = 0 with get, set + member val Completed = 0 with get, set + /// The running operations that count towards Total outside of a batch. + member val Counted = Set.empty with get, set + /// Set by the agent once it read this generation as the wanted state. + member val Seen = false with get, set + member x.IsIdle = Map.isEmpty x.Operations && x.Batches = 0 + +type SharedProgressRequest = + | SyncProgress + | FlushProgress of AsyncReplyChannel + +/// +/// Shows concurrent operations, such as typechecks, as one LSP progress report. +/// The report starts with the first operation, lists the running operations in its message, +/// and ends when the last operation ends. +/// +[] +type SharedProgressReporter(title: string, createReport: unit -> ServerProgressReport, ?isEnabled: unit -> bool) = + + let isEnabled = defaultArg isEnabled (fun () -> true) + let logger = LogProvider.getLoggerByName "SharedProgressReporter" + let syncRoot = obj () + let mutable current: SharedProgressGeneration option = None + let mutable lastOperationId = 0L + let mutable syncPending = 0 + let mutable disposed = false + + let buildMessage (generation: SharedProgressGeneration) = + let labels = + generation.Operations + |> Map.values + |> Seq.choose id + |> Seq.distinct + |> Seq.toList + + let labelsPart = + match labels with + | [] -> None + | [ label ] -> Some label + | labels -> + let shown = labels |> List.truncate 3 |> String.concat ", " + let hidden = labels.Length - 3 + + Some(if hidden > 0 then $"{shown} (+{hidden} more)" else shown) + + let batchPart = + if generation.Total > 0 then + Some $"{generation.Completed}/{generation.Total} completed" + else + None + + match labelsPart, batchPart with + | Some labels, Some batch -> Some $"{labels} ({batch})" + | Some labels, None -> Some labels + | None, Some batch -> Some batch + | None, None -> None + + let calcPercentage (generation: SharedProgressGeneration) = + if generation.Total > 0 then + min 100 (generation.Completed * 100 / generation.Total) |> uint32 |> Some + else + None + + let send (kind: string) (work: CancellationToken -> Task) = + task { + try + do! work CancellationToken.None + with e -> + logger.warn ( + Log.setMessage "Could not send {kind} of {title} progress" + >> Log.addContext "kind" kind + >> Log.addContext "title" title + >> Log.addExn e + ) + } + + /// Brings the client from the shown progress state to the wanted one. + /// A report that started and ended between two syncs is never sent. + let sync + (shown: (ServerProgressReport * string option * uint32 option) option) + (wanted: (ServerProgressReport * string option * uint32 option) option) + = + task { + match shown, wanted with + | Some(shownReport, shownMessage, shownPercentage), Some(report, message, percentage) when + obj.ReferenceEquals(shownReport, report) + -> + if shownMessage <> message || shownPercentage <> percentage then + do! send "report" (report.Report(?message = message, ?percentage = percentage)) + | _ -> + match shown with + | Some(shownReport, _, _) -> + do! send "end" (shownReport.End()) + (shownReport :> IDisposable).Dispose() + | None -> () + + match wanted with + | Some(report, message, percentage) -> + do! send "begin" (report.Begin(title, ?message = message, ?percentage = percentage)) + | None -> () + + return wanted + } + + // The callers only change the state; this agent alone talks to the client, in order, + // so no caller waits for a client round-trip. + let sender = + MailboxProcessor.Start(fun inbox -> + let rec loop shown = + async { + let! request = inbox.Receive() + + let! shown = + async { + try + let wanted = + lock syncRoot (fun () -> + syncPending <- 0 + + current + |> Option.map (fun generation -> + generation.Seen <- true + generation.Report, buildMessage generation, calcPercentage generation)) + + return! sync shown wanted |> Async.AwaitTask + with e -> + logger.warn ( + Log.setMessage "Could not sync {title} progress" + >> Log.addContext "title" title + >> Log.addExn e + ) + + return shown + } + + match request with + | FlushProgress reply -> reply.Reply() + | SyncProgress -> () + + return! loop shown + } + + loop None) + + let requestSync () = + if Interlocked.Exchange(&syncPending, 1) = 0 then + sender.Post SyncProgress + + /// Call while holding syncRoot. + let currentGeneration () = + match current with + | Some generation when not generation.Report.CancellationToken.IsCancellationRequested -> generation + | _ -> + // The user cancelled the current report: new operations get a fresh report and token. + let generation = SharedProgressGeneration(createReport ()) + current <- Some generation + generation + + /// Call while holding syncRoot. + let retireIfIdle (generation: SharedProgressGeneration) = + match current with + | Some c when obj.ReferenceEquals(c, generation) && generation.IsIdle -> + current <- None + + // The agent never saw this report, so it will not end and dispose it either. + if not generation.Seen then + (generation.Report :> IDisposable).Dispose() + | _ -> () + + /// Call while holding syncRoot. + let countOperation (generation: SharedProgressGeneration) operationId = + generation.Counted <- Set.add operationId generation.Counted + generation.Total <- generation.Total + 1 + + /// Call while holding syncRoot. onEnd is called while holding syncRoot. + let startOperation (generation: SharedProgressGeneration) (label: string option) (onEnd: unit -> unit) = + lastOperationId <- lastOperationId + 1L + let operationId = lastOperationId + generation.Operations <- Map.add operationId label generation.Operations + + // After the last batch ended, the report still shows a percentage, so new operations count towards it. + if generation.Batches = 0 && generation.Total > 0 then + countOperation generation operationId + + new SharedProgressScope( + generation.Report.CancellationToken, + fun () -> + lock syncRoot (fun () -> + generation.Operations <- Map.remove operationId generation.Operations + + if Set.contains operationId generation.Counted then + generation.Counted <- Set.remove operationId generation.Counted + generation.Completed <- generation.Completed + 1 + else + onEnd () + + retireIfIdle generation) + + requestSync () + ) + + let disabledScope () = new SharedProgressScope(CancellationToken.None, ignore) + + let disabledBatch () = new SharedProgressBatch(CancellationToken.None, disabledScope, ignore) + + /// Starts an operation. Its label shows in the message of the report until the scope is disposed. + member _.Begin(label: string) = + if not (isEnabled ()) then + disabledScope () + else + let scope = + lock syncRoot (fun () -> + if disposed then + disabledScope () + else + startOperation (currentGeneration ()) (Some label) ignore) + + requestSync () + scope + + /// Starts a batch of operations. The report shows how many of them completed. + member _.BeginBatch(count: int) = + if count <= 0 || not (isEnabled ()) then + disabledBatch () + else + let generation = + lock syncRoot (fun () -> + if disposed then + None + else + let generation = currentGeneration () + generation.Batches <- generation.Batches + 1 + generation.Total <- generation.Total + count + Some generation) + + match generation with + | None -> disabledBatch () + | Some generation -> + requestSync () + + let token = generation.Report.CancellationToken + // Only accessed while holding syncRoot. + let completed = ref 0 + let ended = ref false + + let completeOperation () = + if not ended.Value then + completed.Value <- completed.Value + 1 + generation.Completed <- generation.Completed + 1 + + let beginOperation () = + let scope = + lock syncRoot (fun () -> + if token.IsCancellationRequested then + // The user cancelled the batch, so its remaining operations do not show. + new SharedProgressScope(token, ignore) + else + startOperation generation None completeOperation) + + requestSync () + scope + + let endBatch () = + lock syncRoot (fun () -> + ended.Value <- true + generation.Batches <- generation.Batches - 1 + + // Operations of this batch that never ran no longer count. + generation.Total <- generation.Total - max 0 (count - completed.Value) + + // A report cannot drop its percentage, so the operations that still run count towards it. + // Otherwise the percentage would stay at its last value until the report ends. + if generation.Batches = 0 then + for operationId in generation.Operations.Keys do + if not (Set.contains operationId generation.Counted) then + countOperation generation operationId + + retireIfIdle generation) + + requestSync () + + new SharedProgressBatch(token, beginOperation, endBatch) + + /// Completes when the client has been sent the current progress state. + member _.Flush() : Task = sender.PostAndAsyncReply FlushProgress |> Async.StartImmediateAsTask + + interface IDisposable with + member _.Dispose() = + lock syncRoot (fun () -> + disposed <- true + current <- None) + + requestSync () + + open System.Diagnostics.Tracing open System.Collections.Concurrent open System.Diagnostics diff --git a/src/FsAutoComplete/LspServers/FSharpLspClient.fsi b/src/FsAutoComplete/LspServers/FSharpLspClient.fsi index da3f4ee6e..f16b96f9e 100644 --- a/src/FsAutoComplete/LspServers/FSharpLspClient.fsi +++ b/src/FsAutoComplete/LspServers/FSharpLspClient.fsi @@ -7,6 +7,7 @@ open Ionide.LanguageServerProtocol.JsonRpc open FsAutoComplete.LspHelpers open System open System.Threading +open System.Threading.Tasks open IcedTasks type FSharpLspClient = @@ -79,6 +80,49 @@ type ServerProgressReport = interface IAsyncDisposable interface IDisposable +/// One operation tracked by a . Dispose it when the operation ends. +[] +type SharedProgressScope = + /// Cancelled when the user cancels the progress report that shows the operation. + member CancellationToken: CancellationToken + interface IDisposable + +/// A batch of operations tracked by a . Dispose it when the batch ends. +[] +type SharedProgressBatch = + /// Cancelled when the user cancels the progress report that shows the batch. + member CancellationToken: CancellationToken + + /// + /// Starts an operation of the batch. When its scope is disposed, it counts as completed. + /// It has no label: the work it runs shows its own label once it actually starts. + /// + member Begin: unit -> SharedProgressScope + interface IDisposable + +/// +/// Shows concurrent operations, such as typechecks, as one LSP progress report. +/// The report starts with the first operation, lists the running operations in its message, +/// and ends when the last operation ends. +/// +[] +type SharedProgressReporter = + /// The title of the progress report. + /// Creates the report that a new progress cycle uses. + /// When it returns false, operations are not shown and their token is never cancelled. + new: + title: string * createReport: (unit -> ServerProgressReport) * ?isEnabled: (unit -> bool) -> SharedProgressReporter + + /// Starts an operation. Its label shows in the message of the report until the scope is disposed. + member Begin: label: string -> SharedProgressScope + + /// Starts a batch of operations. The report shows how many of them completed. + member BeginBatch: count: int -> SharedProgressBatch + + /// Completes when the client has been sent the current progress state. + member Flush: unit -> Task + interface IDisposable + /// listener for the the events generated from the fsc ActivitySource type ProgressListener = new: lspClient: FSharpLspClient * traceNamespace: string array -> ProgressListener diff --git a/test/FsAutoComplete.Tests.Lsp/Program.fs b/test/FsAutoComplete.Tests.Lsp/Program.fs index 7d49a8e83..9b3cb17b6 100644 --- a/test/FsAutoComplete.Tests.Lsp/Program.fs +++ b/test/FsAutoComplete.Tests.Lsp/Program.fs @@ -13,6 +13,7 @@ open FsAutoComplete.Tests.ExtensionsTests open FsAutoComplete.Tests.InteractiveDirectivesTests open FsAutoComplete.Tests.Lsp.CoreUtilsTests open FsAutoComplete.Tests.Lsp.DecompilerTests +open FsAutoComplete.Tests.SharedProgressReporterTests open FsAutoComplete.Tests.CallHierarchy open Ionide.ProjInfo open System.Threading @@ -374,6 +375,7 @@ let generalTests = testCase "test host uses target runtime" (fun _ -> Expect.equal Environment.Version.Major expectedRuntimeMajor "Test host runtime must match the target framework") testList (nameof (Utils)) [ Utils.Tests.Utils.tests; Utils.Tests.TextEdit.tests ] + SharedProgressReporterTests.tests InlayHintTests.explicitTypeInfoTests sourceTextFactory FindReferences.tryFixupRangeTests sourceTextFactory UtilsTests.allTests diff --git a/test/FsAutoComplete.Tests.Lsp/SharedProgressReporterTests.fs b/test/FsAutoComplete.Tests.Lsp/SharedProgressReporterTests.fs new file mode 100644 index 000000000..cf14b2cc3 --- /dev/null +++ b/test/FsAutoComplete.Tests.Lsp/SharedProgressReporterTests.fs @@ -0,0 +1,515 @@ +module FsAutoComplete.Tests.SharedProgressReporterTests + +open Expecto +open System +open System.Collections.Concurrent +open FsAutoComplete.Lsp +open Ionide.LanguageServerProtocol +open Ionide.LanguageServerProtocol.Server +open Ionide.LanguageServerProtocol.Types +open Ionide.LanguageServerProtocol.JsonRpc +open Newtonsoft.Json.Linq + +/// Captured progress call from the mock client +type ProgressCall = + | Begin of title: string * message: string option * percentage: uint32 option + | Report of message: string option * percentage: uint32 option + | End + +/// Creates a mock FSharpLspClient that captures all progress notifications +let private createMockClient () = + let progressCalls = ConcurrentQueue() + + let notificationSender: ClientNotificationSender = + fun methodName payload -> + if methodName = "$/progress" then + let value = (payload :?> ProgressParams).Value + + let tryGet (name: string) = + match value.[name] with + | null -> None + | token -> Some token + + let message = tryGet "message" |> Option.map (fun t -> t.Value()) + let percentage = tryGet "percentage" |> Option.map (fun t -> t.Value()) + + match value.Value("kind") with + | "begin" -> progressCalls.Enqueue(Begin(value.Value("title"), message, percentage)) + | "report" -> progressCalls.Enqueue(Report(message, percentage)) + | "end" -> progressCalls.Enqueue End + | kind -> failwith $"Unexpected progress kind {kind}" + + AsyncLspResult.success () + + let requestSender = + { new ClientRequestSender with + member _.Send _name _payload = AsyncLspResult.success Unchecked.defaultof<_> + } + + let client = new FSharpLspClient(notificationSender, requestSender) + + client.ClientCapabilities <- + Some + { + Workspace = None + TextDocument = None + Experimental = None + Window = + Some + { + WorkDoneProgress = Some true + ShowMessage = None + ShowDocument = None + } + General = None + NotebookDocument = None + } + + client, progressCalls + +type private TestReporter = + { + Reporter: SharedProgressReporter + Calls: ConcurrentQueue + Reports: ConcurrentQueue + } + + /// Waits until the client has been sent the current state, then returns all calls so far. + member x.Flush() = + async { + do! x.Reporter.Flush() |> Async.AwaitTask + return x.Calls.ToArray() |> Array.toList + } + +let private createReporter (isEnabled: bool) = + let client, calls = createMockClient () + let reports = ConcurrentQueue() + + let reporter = + new SharedProgressReporter( + "Typechecking", + (fun () -> + let report = new ServerProgressReport(client, cancellableDefault = true) + reports.Enqueue report + report), + isEnabled = fun () -> isEnabled + ) + + { + Reporter = reporter + Calls = calls + Reports = reports + } + +let private dispose (d: IDisposable) = d.Dispose() + +/// The message and percentage the client shows after the given calls. +let private lastState calls = + match List.last calls with + | Begin(_, message, percentage) + | Report(message, percentage) -> Some(message, percentage) + | End -> None + +let operationTests = + testList + "Operations" + [ + testCaseAsync "One operation begins and ends a report" + <| async { + let t = createReporter true + let scope = t.Reporter.Begin "Foo.fs" + let! calls = t.Flush() + Expect.equal calls [ Begin("Typechecking", Some "Foo.fs", None) ] "The report should begin with the label" + + dispose scope + let! calls = t.Flush() + Expect.equal (List.last calls) End "The report should end with the operation" + Expect.equal t.Reports.Count 1 "One report should be created" + } + + testCaseAsync "Operations that start after the previous one ended use a new report" + <| async { + let t = createReporter true + let first = t.Reporter.Begin "A.fs" + let! _ = t.Flush() + dispose first + let! _ = t.Flush() + + let second = t.Reporter.Begin "B.fs" + let! _ = t.Flush() + dispose second + let! calls = t.Flush() + + Expect.equal + calls + [ + Begin("Typechecking", Some "A.fs", None) + End + Begin("Typechecking", Some "B.fs", None) + End + ] + "Each operation should have its own report cycle" + + Expect.equal t.Reports.Count 2 "Two reports should be created" + } + + testCaseAsync "Concurrent operations share one report until the last ends" + <| async { + let t = createReporter true + let a = t.Reporter.Begin "A.fs" + let b = t.Reporter.Begin "B.fs" + let! calls = t.Flush() + Expect.equal (lastState calls) (Some(Some "A.fs, B.fs", None)) "Both labels should show" + + dispose a + let! calls = t.Flush() + Expect.equal (List.last calls) (Report(Some "B.fs", None)) "Only the running operation should show" + + dispose b + let! calls = t.Flush() + Expect.equal (List.last calls) End "The report should end with the last operation" + Expect.equal t.Reports.Count 1 "One report should be shared" + } + + testCaseAsync "Operations with the same label keep the report open until both end" + <| async { + let t = createReporter true + let first = t.Reporter.Begin "Foo.fs" + let second = t.Reporter.Begin "Foo.fs" + let! calls = t.Flush() + Expect.equal (lastState calls) (Some(Some "Foo.fs", None)) "The label should show once" + + dispose first + let! calls = t.Flush() + Expect.isFalse (List.contains End calls) "The report should stay open for the second operation" + + dispose second + let! calls = t.Flush() + Expect.equal (List.last calls) End "The report should end with the second operation" + } + + testCaseAsync "Disposing a scope twice ends the operation once" + <| async { + let t = createReporter true + let first = t.Reporter.Begin "Foo.fs" + let second = t.Reporter.Begin "Foo.fs" + dispose first + dispose first + let! calls = t.Flush() + Expect.isFalse (List.contains End calls) "The second operation still runs" + dispose second + } + + testCaseAsync "More than three operations show the first three and a count" + <| async { + let t = createReporter true + let scopes = [ "A.fs"; "B.fs"; "C.fs"; "D.fs"; "E.fs" ] |> List.map t.Reporter.Begin + let! calls = t.Flush() + + Expect.equal (lastState calls) (Some(Some "A.fs, B.fs, C.fs (+2 more)", None)) "The message should be truncated" + + scopes |> List.iter dispose + } + + testCaseAsync "Disposing the reporter ends the shown report" + <| async { + let t = createReporter true + let _scope = t.Reporter.Begin "Foo.fs" + let! _ = t.Flush() + dispose t.Reporter + let! calls = t.Flush() + Expect.equal (List.last calls) End "The report should end" + + let _afterDispose = t.Reporter.Begin "Bar.fs" + let _batchAfterDispose = t.Reporter.BeginBatch 2 + let! callsAfter = t.Flush() + Expect.equal callsAfter calls "A disposed reporter should start no new report" + Expect.equal t.Reports.Count 1 "No report should be created after dispose" + } + ] + +let batchTests = + testList + "Batches" + [ + testCaseAsync "A batch shows how many of its operations completed" + <| async { + let t = createReporter true + let batch = t.Reporter.BeginBatch 4 + let! calls = t.Flush() + Expect.equal calls [ Begin("Typechecking", Some "0/4 completed", Some 0u) ] "The batch should begin at 0%" + + let first = batch.Begin() + let! calls = t.Flush() + Expect.equal calls.Length 1 "A batch operation has no label of its own" + + let work = t.Reporter.Begin "A.fs" + let! calls = t.Flush() + + Expect.equal (List.last calls) (Report(Some "A.fs (0/4 completed)", Some 0u)) "The running work should show" + + dispose work + dispose first + let! calls = t.Flush() + Expect.equal (List.last calls) (Report(Some "1/4 completed", Some 25u)) "One of four operations completed" + + dispose batch + let! calls = t.Flush() + Expect.equal (List.last calls) End "The report should end with the batch" + } + + testCaseAsync "Operations outside the batch do not count towards it" + <| async { + let t = createReporter true + let batch = t.Reporter.BeginBatch 2 + let outside = t.Reporter.Begin "Outside.fs" + dispose outside + let! calls = t.Flush() + Expect.equal (lastState calls) (Some(Some "0/2 completed", Some 0u)) "Only batch operations should count" + + dispose batch + } + + testCaseAsync "Batch operations count separately" + <| async { + let t = createReporter true + let batch = t.Reporter.BeginBatch 2 + let first = batch.Begin() + let second = batch.Begin() + dispose first + let! calls = t.Flush() + Expect.equal (lastState calls) (Some(Some "1/2 completed", Some 50u)) "One operation completed" + + dispose second + let! calls = t.Flush() + Expect.equal (List.last calls) (Report(Some "2/2 completed", Some 100u)) "Both operations completed" + dispose batch + } + + testCaseAsync "Overlapping batches add up" + <| async { + let t = createReporter true + let first = t.Reporter.BeginBatch 2 + let second = t.Reporter.BeginBatch 2 + first.Begin() |> dispose + second.Begin() |> dispose + let! calls = t.Flush() + Expect.equal (lastState calls) (Some(Some "2/4 completed", Some 50u)) "Both batches should count" + + dispose first + dispose second + let! calls = t.Flush() + Expect.equal (List.last calls) End "The report should end with the last batch" + } + + testCaseAsync "When the last batch ends, the operations that still run count towards the percentage" + <| async { + let t = createReporter true + let batch = t.Reporter.BeginBatch 2 + let running = t.Reporter.Begin "Open.fs" + batch.Begin() |> dispose + batch.Begin() |> dispose + let! calls = t.Flush() + Expect.equal (lastState calls) (Some(Some "Open.fs (2/2 completed)", Some 100u)) "The batch completed" + + dispose batch + let! calls = t.Flush() + + Expect.equal + (lastState calls) + (Some(Some "Open.fs (2/3 completed)", Some 66u)) + "The running operation should count, so the report does not stay at 100%" + + let late = t.Reporter.Begin "Late.fs" + let! calls = t.Flush() + + Expect.equal + (lastState calls) + (Some(Some "Open.fs, Late.fs (2/4 completed)", Some 50u)) + "An operation that starts after the batch ended should count too" + + dispose running + let! calls = t.Flush() + Expect.equal (lastState calls) (Some(Some "Late.fs (3/4 completed)", Some 75u)) "One more operation completed" + + dispose late + let! calls = t.Flush() + Expect.equal (List.last calls) End "The report should end with the last operation" + Expect.equal t.Reports.Count 1 "One report should be used" + } + + testCaseAsync "A new batch adds to the count of a report that is still open" + <| async { + let t = createReporter true + // Keeps the report open between the two batches. + let keepOpen = t.Reporter.Begin "Open.fs" + let first = t.Reporter.BeginBatch 2 + first.Begin() |> dispose + dispose first + let! calls = t.Flush() + Expect.equal (lastState calls) (Some(Some "Open.fs (1/2 completed)", Some 50u)) "The first batch ended early" + + let second = t.Reporter.BeginBatch 3 + let! calls = t.Flush() + + Expect.equal + (lastState calls) + (Some(Some "Open.fs (1/5 completed)", Some 20u)) + "The second batch should add to the count" + + second.Begin() |> dispose + let! calls = t.Flush() + + Expect.equal + (lastState calls) + (Some(Some "Open.fs (2/5 completed)", Some 40u)) + "An operation of the second batch completed" + + dispose second + dispose keepOpen + let! calls = t.Flush() + Expect.equal (List.last calls) End "The report should end with the last operation" + } + + testCaseAsync "A batch that ends early stops counting its operations that did not complete" + <| async { + let t = createReporter true + let aborted = t.Reporter.BeginBatch 3 + let running = t.Reporter.BeginBatch 2 + aborted.Begin() |> dispose + let stillRunning = aborted.Begin() + dispose aborted + let! calls = t.Flush() + Expect.equal (lastState calls) (Some(Some "1/3 completed", Some 33u)) "Only the completed operation should stay" + + dispose stillRunning + let! calls = t.Flush() + Expect.equal (lastState calls) (Some(Some "1/3 completed", Some 33u)) "A late operation should not count" + + running.Begin() |> dispose + let! calls = t.Flush() + Expect.equal (lastState calls) (Some(Some "2/3 completed", Some 66u)) "The running batch should count" + dispose running + } + + testCaseAsync "A batch keeps one report across operations that run one after another" + <| async { + let t = createReporter true + let batch = t.Reporter.BeginBatch 2 + + for label in [ "A.fs"; "B.fs" ] do + use _operation = batch.Begin() + use _running = t.Reporter.Begin label + let! _ = t.Flush() + () + + let! calls = t.Flush() + Expect.isFalse (List.contains End calls) "The report should stay open between the operations" + Expect.equal t.Reports.Count 1 "One report should be used" + Expect.equal (lastState calls) (Some(Some "2/2 completed", Some 100u)) "Both operations completed" + + dispose batch + let! calls = t.Flush() + Expect.equal (List.last calls) End "The report should end with the batch" + } + + testCaseAsync "An empty batch shows nothing" + <| async { + let t = createReporter true + let batch = t.Reporter.BeginBatch 0 + let! calls = t.Flush() + Expect.isEmpty calls "No progress should be sent" + Expect.equal t.Reports.Count 0 "No report should be created" + dispose batch + } + ] + +let cancellationTests = + testList + "Cancellation" + [ + testCaseAsync "The scope token is cancelled when the user cancels the report" + <| async { + let t = createReporter true + let scope = t.Reporter.Begin "Foo.fs" + Expect.isFalse scope.CancellationToken.IsCancellationRequested "The token should not start cancelled" + t.Reports.ToArray().[0].Cancel() + Expect.isTrue scope.CancellationToken.IsCancellationRequested "The token should follow the report" + dispose scope + } + + testCaseAsync "An operation that starts after a cancel gets a new report and token" + <| async { + let t = createReporter true + let cancelled = t.Reporter.Begin "A.fs" + let! _ = t.Flush() + t.Reports.ToArray().[0].Cancel() + + let fresh = t.Reporter.Begin "B.fs" + Expect.isFalse fresh.CancellationToken.IsCancellationRequested "The new operation should not be cancelled" + Expect.equal t.Reports.Count 2 "A new report should be created" + + let! calls = t.Flush() + + Expect.equal + calls + [ + Begin("Typechecking", Some "A.fs", None) + End + Begin("Typechecking", Some "B.fs", None) + ] + "The cancelled report should end and the new one begin" + + dispose cancelled + let! calls = t.Flush() + Expect.equal (List.last calls) (Begin("Typechecking", Some "B.fs", None)) "The old operation is not shown" + + dispose fresh + let! calls = t.Flush() + Expect.equal (List.last calls) End "The new report should end" + } + + testCaseAsync "Batch operations that start after a cancel get the cancelled token" + <| async { + let t = createReporter true + let batch = t.Reporter.BeginBatch 2 + let! _ = t.Flush() + t.Reports.ToArray().[0].Cancel() + + let operation = batch.Begin() + Expect.isTrue operation.CancellationToken.IsCancellationRequested "The batch was cancelled" + Expect.equal t.Reports.Count 1 "No new report should be created for the batch" + + dispose operation + dispose batch + let! calls = t.Flush() + Expect.equal (List.last calls) End "The report should end with the batch" + } + ] + +let disabledTests = + testList + "Disabled" + [ + testCaseAsync "A disabled reporter sends nothing and never cancels" + <| async { + let t = createReporter false + let scope = t.Reporter.Begin "Foo.fs" + let batch = t.Reporter.BeginBatch 2 + let operation = batch.Begin() + + Expect.equal scope.CancellationToken Threading.CancellationToken.None "No token for an operation" + Expect.equal batch.CancellationToken Threading.CancellationToken.None "No token for a batch" + Expect.equal operation.CancellationToken Threading.CancellationToken.None "No token for a batch operation" + + dispose operation + dispose batch + dispose scope + let! calls = t.Flush() + Expect.isEmpty calls "No progress should be sent" + Expect.equal t.Reports.Count 0 "No report should be created" + } + ] + +let tests = + testList "SharedProgressReporter" [ operationTests; batchTests; cancellationTests; disabledTests ]