Cache DocumentDiagnosticAnalyzer results by version stamp to avoid redundant typecheck on unchanged documents

docs/release-notes/.VisualStudio/18.vNext.md

@@ -7,6 +7,7 @@
 
 * Fixed Rename incorrectly renaming `get` and `set` keywords for properties with explicit accessors. ([Issue #18270](https://github.com/dotnet/fsharp/issues/18270), [PR #19252](https://github.com/dotnet/fsharp/pull/19252))
 * Fixed Find All References crash when F# project contains non-F# files like `.cshtml`. ([Issue #16394](https://github.com/dotnet/fsharp/issues/16394), [PR #19252](https://github.com/dotnet/fsharp/pull/19252))
+* Prevent stale and leaked per-document diagnostics by tightening cache validity and evicting entries for removed documents. ([PR #20121](https://github.com/dotnet/fsharp/pull/20121))
 * Find All References for external DLL symbols now only searches projects that reference the specific assembly. ([Issue #10227](https://github.com/dotnet/fsharp/issues/10227), [PR #19252](https://github.com/dotnet/fsharp/pull/19252))
 * Improve static compilation of state machines. ([PR #19297](https://github.com/dotnet/fsharp/pull/19297))
 * Make Alt+F1 (momentary toggle) work for inlay hints. ([PR #19421](https://github.com/dotnet/fsharp/pull/19421))

vsintegration/src/FSharp.Editor/Diagnostics/DocumentDiagnosticAnalyzer.fs

@@ -1,8 +1,9 @@
-// Copyright (c) Microsoft Corporation.  All Rights Reserved.  See License.txt in the project root for license information.
+// Copyright (c) Microsoft Corporation.  All Rights Reserved.  See License.txt in the project root for license information.
 
 namespace Microsoft.VisualStudio.FSharp.Editor
 
 open System.Composition
+open System.Collections.Concurrent
 open System.Collections.Immutable
 open System.Collections.Generic
 open System.Threading
@@ -21,12 +22,31 @@ type internal DiagnosticsType =
     | Syntax
     | Semantic
 
+type private CachedDiagnosticsEntry =
+    {
+        TextVersion: VersionStamp
+        ProjectVersion: VersionStamp
+        FilePath: string
+        IsRemoveParensEnabled: bool
+        Diagnostics: ImmutableArray<Diagnostic>
+    }
+
 [<Export(typeof<IFSharpDocumentDiagnosticAnalyzer>)>]
 type internal FSharpDocumentDiagnosticAnalyzer [<ImportingConstructor>] () =
 
     let shouldProduceDiagnostics (document: Document) =
         document.Project.Solution.GetFSharpExtensionConfig().ShouldProduceDiagnostics()
 
+    static let cache =
+        ConcurrentDictionary<struct (DocumentId * DiagnosticsType), CachedDiagnosticsEntry>()
+
+    static let evictRemovedDocuments (solution: Solution) =
+        for entry in cache do
+            let struct (documentId, _) = entry.Key
+
+            if isNull (solution.GetDocument(documentId)) then
+                cache.TryRemove(entry.Key) |> ignore
+
     static let diagnosticEqualityComparer =
         { new IEqualityComparer<FSharpDiagnostic> with
 
@@ -72,61 +92,104 @@ type internal FSharpDocumentDiagnosticAnalyzer [<ImportingConstructor>] () =
 
             let! ct = CancellableTask.getCancellationToken ()
 
-            let! sourceText = document.GetTextAsync(ct)
-            let filePath = document.FilePath
+            let! textVersion = document.GetTextVersionAsync(ct)
 
-            let errors = HashSet<FSharpDiagnostic>(diagnosticEqualityComparer)
+            let! projectVersion =
+                match diagnosticType with
+                | DiagnosticsType.Syntax -> CancellableTask.singleton VersionStamp.Default
+                | DiagnosticsType.Semantic -> (fun ct -> document.Project.GetDependentVersionAsync(ct))
 
-            let! parseResults = document.GetFSharpParseResultsAsync("GetDiagnostics")
+            let filePath = document.FilePath
 
-            match diagnosticType with
-            | DiagnosticsType.Syntax ->
-                for diagnostic in parseResults.Diagnostics do
-                    errors.Add(diagnostic) |> ignore
+            let isRemoveParensEnabled =
+                match diagnosticType with
+                | DiagnosticsType.Syntax -> document.Project.IsFsharpRemoveParensEnabled
+                | DiagnosticsType.Semantic -> false
 
-            | DiagnosticsType.Semantic ->
-                let! _, checkResults = document.GetFSharpParseAndCheckResultsAsync("GetDiagnostics")
+            evictRemovedDocuments document.Project.Solution
 
-                for diagnostic in checkResults.Diagnostics do
-                    errors.Add(diagnostic) |> ignore
+            let cacheKey = struct (document.Id, diagnosticType)
 
-                errors.ExceptWith(parseResults.Diagnostics)
+            let cached =
+                match cache.TryGetValue(cacheKey) with
+                | true, cachedEntry when
+                    cachedEntry.TextVersion = textVersion
+                    && cachedEntry.ProjectVersion = projectVersion
+                    && cachedEntry.FilePath = filePath
+                    && cachedEntry.IsRemoveParensEnabled = isRemoveParensEnabled
+                    ->
+                    ValueSome cachedEntry.Diagnostics
+                | _ -> ValueNone
 
-            let! unnecessaryParentheses =
-                match diagnosticType with
-                | DiagnosticsType.Syntax when document.Project.IsFsharpRemoveParensEnabled ->
-                    UnnecessaryParenthesesDiagnosticAnalyzer.GetDiagnostics document
-                | _ -> CancellableTask.singleton ImmutableArray.Empty
+            match cached with
+            | ValueSome cachedDiagnostics -> return cachedDiagnostics
+            | ValueNone ->
 
-            if errors.Count = 0 && unnecessaryParentheses.IsEmpty then
-                return ImmutableArray.Empty
-            else
-                let iab = ImmutableArray.CreateBuilder(errors.Count + unnecessaryParentheses.Length)
+                let! sourceText = document.GetTextAsync(ct)
 
-                for diagnostic in errors do
-                    if diagnostic.StartLine <> 0 && diagnostic.EndLine <> 0 then
-                        let linePositionSpan =
-                            LinePositionSpan(
-                                LinePosition(diagnostic.StartLine - 1, diagnostic.StartColumn),
-                                LinePosition(diagnostic.EndLine - 1, diagnostic.EndColumn)
-                            )
+                let errors = HashSet<FSharpDiagnostic>(diagnosticEqualityComparer)
 
-                        let textSpan = sourceText.Lines.GetTextSpan(linePositionSpan)
+                let! parseResults = document.GetFSharpParseResultsAsync("GetDiagnostics")
 
-                        // F# compiler report errors at end of file if parsing fails. It should be corrected to match Roslyn boundaries
-                        let correctedTextSpan =
-                            if textSpan.End <= sourceText.Length then
-                                textSpan
-                            else
-                                let start = min textSpan.Start (sourceText.Length - 1) |> max 0
+                match diagnosticType with
+                | DiagnosticsType.Syntax ->
+                    for diagnostic in parseResults.Diagnostics do
+                        errors.Add(diagnostic) |> ignore
+
+                | DiagnosticsType.Semantic ->
+                    let! _, checkResults = document.GetFSharpParseAndCheckResultsAsync("GetDiagnostics")
 
-                                TextSpan.FromBounds(start, sourceText.Length)
+                    for diagnostic in checkResults.Diagnostics do
+                        errors.Add(diagnostic) |> ignore
 
-                        let location = Location.Create(filePath, correctedTextSpan, linePositionSpan)
-                        iab.Add(RoslynHelpers.ConvertError(diagnostic, location))
+                    errors.ExceptWith(parseResults.Diagnostics)
 
-                iab.AddRange unnecessaryParentheses
-                return iab.ToImmutable()
+                let! unnecessaryParentheses =
+                    match diagnosticType with
+                    | DiagnosticsType.Syntax when isRemoveParensEnabled -> UnnecessaryParenthesesDiagnosticAnalyzer.GetDiagnostics document
+                    | _ -> CancellableTask.singleton ImmutableArray.Empty
+
+                let result =
+                    if errors.Count = 0 && unnecessaryParentheses.IsEmpty then
+                        ImmutableArray.Empty
+                    else
+                        let iab = ImmutableArray.CreateBuilder(errors.Count + unnecessaryParentheses.Length)
+
+                        for diagnostic in errors do
+                            if diagnostic.StartLine <> 0 && diagnostic.EndLine <> 0 then
+                                let linePositionSpan =
+                                    LinePositionSpan(
+                                        LinePosition(diagnostic.StartLine - 1, diagnostic.StartColumn),
+                                        LinePosition(diagnostic.EndLine - 1, diagnostic.EndColumn)
+                                    )
+
+                                let textSpan = sourceText.Lines.GetTextSpan(linePositionSpan)
+
+                                // F# compiler report errors at end of file if parsing fails. It should be corrected to match Roslyn boundaries
+                                let correctedTextSpan =
+                                    if textSpan.End <= sourceText.Length then
+                                        textSpan
+                                    else
+                                        let start = min textSpan.Start (sourceText.Length - 1) |> max 0
+
+                                        TextSpan.FromBounds(start, sourceText.Length)
+
+                                let location = Location.Create(filePath, correctedTextSpan, linePositionSpan)
+                                iab.Add(RoslynHelpers.ConvertError(diagnostic, location))
+
+                        iab.AddRange unnecessaryParentheses
+                        iab.ToImmutable()
+
+                cache.[cacheKey] <-
+                    {
+                        TextVersion = textVersion
+                        ProjectVersion = projectVersion
+                        FilePath = filePath
+                        IsRemoveParensEnabled = isRemoveParensEnabled
+                        Diagnostics = result
+                    }
+
+                return result
         }
 
     interface IFSharpDocumentDiagnosticAnalyzer with