Remove unnecessary GetBackgroundCheckResultsForFileInProject (#977)

* Remove unnecessery GetBackgroundCheckResultsForFileInProject * Clean up CompilerServiceInterface.fs * bind to the nuget provided by the SDK * minor refactor to make the logic more debuggable * Update deps * Ignore failing test * Remove NuGet.Frameworks top level deps Co-authored-by: Chet Husk <chusk3@gmail.com>
ionide · Aug 16, 2022 · 7f6917d · 7f6917d
1 parent 40416f9
commit 7f6917d
Show file tree

Hide file tree

Showing 4 changed files with 59 additions and 100 deletions.
diff --git a/paket.lock b/paket.lock
@@ -148,7 +148,7 @@ NUGET
     Microsoft.Build.Framework (17.2) - copy_local: false
       Microsoft.Win32.Registry (>= 4.3)
       System.Security.Permissions (>= 4.7)
-    Microsoft.Build.Locator (1.4.1) - restriction: || (== net6.0) (&& (== netstandard2.0) (>= net5.0))
+    Microsoft.Build.Locator (1.5.3) - restriction: || (== net6.0) (&& (== netstandard2.0) (>= net5.0))
     Microsoft.Build.Tasks.Core (17.2) - copy_local: false
       Microsoft.Build.Framework (>= 17.2)
       Microsoft.Build.Utilities.Core (>= 17.2)
@@ -171,7 +171,7 @@ NUGET
       System.Configuration.ConfigurationManager (>= 4.7)
       System.Security.Permissions (>= 4.7) - restriction: == netstandard2.0
       System.Text.Encoding.CodePages (>= 4.0.1) - restriction: == netstandard2.0
-    Microsoft.CodeCoverage (17.2) - restriction: || (== net6.0) (&& (== netstandard2.0) (>= net45)) (&& (== netstandard2.0) (>= netcoreapp1.0))
+    Microsoft.CodeCoverage (17.3) - restriction: || (== net6.0) (&& (== netstandard2.0) (>= net45)) (&& (== netstandard2.0) (>= netcoreapp1.0))
     Microsoft.DotNet.PlatformAbstractions (3.1.6) - restriction: || (== net6.0) (&& (== netstandard2.0) (>= net5.0))
     Microsoft.Extensions.DependencyModel (6.0) - restriction: || (== net6.0) (&& (== netstandard2.0) (>= net5.0))
       System.Buffers (>= 4.5.1)
@@ -182,9 +182,9 @@ NUGET
     Microsoft.NET.StringTools (1.0) - copy_local: false
       System.Memory (>= 4.5.4)
       System.Runtime.CompilerServices.Unsafe (>= 5.0)
-    Microsoft.NET.Test.Sdk (17.2)
-      Microsoft.CodeCoverage (>= 17.2) - restriction: || (== net6.0) (&& (== netstandard2.0) (>= net45)) (&& (== netstandard2.0) (>= netcoreapp1.0))
-      Microsoft.TestPlatform.TestHost (>= 17.2) - restriction: || (== net6.0) (&& (== netstandard2.0) (>= netcoreapp1.0))
+    Microsoft.NET.Test.Sdk (17.3)
+      Microsoft.CodeCoverage (>= 17.3) - restriction: || (== net6.0) (&& (== netstandard2.0) (>= net45)) (&& (== netstandard2.0) (>= netcoreapp1.0))
+      Microsoft.TestPlatform.TestHost (>= 17.3) - restriction: || (== net6.0) (&& (== netstandard2.0) (>= netcoreapp1.0))
     Microsoft.NETCore.Platforms (6.0.5)
     Microsoft.NETCore.Targets (5.0)
     Microsoft.NETFramework.ReferenceAssemblies (1.0.2)
@@ -201,20 +201,20 @@ NUGET
     Microsoft.SourceLink.GitLab (1.1.1) - copy_local: true
       Microsoft.Build.Tasks.Git (>= 1.1.1)
       Microsoft.SourceLink.Common (>= 1.1.1)
-    Microsoft.TestPlatform.ObjectModel (17.2)
+    Microsoft.TestPlatform.ObjectModel (17.3)
       NuGet.Frameworks (>= 5.11)
       System.Reflection.Metadata (>= 1.6)
-    Microsoft.TestPlatform.TestHost (17.2) - restriction: || (== net6.0) (&& (== netstandard2.0) (>= netcoreapp1.0))
-      Microsoft.TestPlatform.ObjectModel (>= 17.2) - restriction: || (== net6.0) (&& (== netstandard2.0) (>= netcoreapp1.0)) (&& (== netstandard2.0) (>= uap10.0))
+    Microsoft.TestPlatform.TestHost (17.3) - restriction: || (== net6.0) (&& (== netstandard2.0) (>= netcoreapp1.0))
+      Microsoft.TestPlatform.ObjectModel (>= 17.3) - restriction: || (== net6.0) (&& (== netstandard2.0) (>= netcoreapp1.0)) (&& (== netstandard2.0) (>= uap10.0))
       Newtonsoft.Json (>= 9.0.1) - restriction: || (== net6.0) (&& (== netstandard2.0) (>= netcoreapp1.0)) (&& (== netstandard2.0) (>= uap10.0))
-    Microsoft.VisualStudio.Threading (17.2.32)
+    Microsoft.VisualStudio.Threading (17.3.44)
       Microsoft.Bcl.AsyncInterfaces (>= 6.0)
-      Microsoft.VisualStudio.Threading.Analyzers (>= 17.2.32)
-      Microsoft.VisualStudio.Validation (>= 17.0.53)
+      Microsoft.VisualStudio.Threading.Analyzers (>= 17.3.44)
+      Microsoft.VisualStudio.Validation (>= 17.0.58)
       Microsoft.Win32.Registry (>= 5.0)
       System.Threading.Tasks.Extensions (>= 4.5.4)
-    Microsoft.VisualStudio.Threading.Analyzers (17.2.32)
-    Microsoft.VisualStudio.Validation (17.0.53)
+    Microsoft.VisualStudio.Threading.Analyzers (17.3.44)
+    Microsoft.VisualStudio.Validation (17.0.64)
     Microsoft.Win32.Primitives (4.3)
       Microsoft.NETCore.Platforms (>= 1.1)
       Microsoft.NETCore.Targets (>= 1.1)
@@ -233,7 +233,7 @@ NUGET
       System.IO.Pipelines (>= 5.0.1)
       System.Runtime.CompilerServices.Unsafe (>= 5.0)
     Newtonsoft.Json (13.0.1)
-    NuGet.Frameworks (6.2.1)
+    NuGet.Frameworks (6.3)
     runtime.debian.8-x64.runtime.native.System.Security.Cryptography.OpenSsl (4.3.3)
     runtime.debian.9-x64.runtime.native.System.Security.Cryptography.OpenSsl (4.3.3)
     runtime.fedora.23-x64.runtime.native.System.Security.Cryptography.OpenSsl (4.3.3)
@@ -285,15 +285,15 @@ NUGET
       Serilog (>= 2.10)
     Serilog.Sinks.File (5.0)
       Serilog (>= 2.10)
-    StreamJsonRpc (2.11.35)
+    StreamJsonRpc (2.12.27)
       MessagePack (>= 2.3.85)
       Microsoft.Bcl.AsyncInterfaces (>= 6.0)
       Microsoft.VisualStudio.Threading (>= 17.1.46)
       Nerdbank.Streams (>= 2.8.57)
       Newtonsoft.Json (>= 13.0.1)
-      System.Collections.Immutable (>= 5.0)
+      System.Collections.Immutable (>= 6.0)
       System.Diagnostics.DiagnosticSource (>= 6.0)
-      System.IO.Pipelines (>= 6.0.1)
+      System.IO.Pipelines (>= 6.0.3)
       System.Threading.Tasks.Dataflow (>= 6.0)
     System.Buffers (4.5.1)
     System.CodeDom (6.0) - copy_local: false
@@ -684,10 +684,10 @@ NUGET
       System.Security.Cryptography.Primitives (>= 4.3)
       System.Text.Encoding (>= 4.3)
       System.Threading (>= 4.3)
-    System.Security.Cryptography.Xml (6.0) - copy_local: false
+    System.Security.Cryptography.Xml (6.0.1) - copy_local: false
       System.Memory (>= 4.5.4) - restriction: == netstandard2.0
       System.Security.AccessControl (>= 6.0)
-      System.Security.Cryptography.Pkcs (>= 6.0)
+      System.Security.Cryptography.Pkcs (>= 6.0.1)
     System.Security.Permissions (6.0)
       System.Security.AccessControl (>= 6.0)
       System.Windows.Extensions (>= 6.0) - restriction: || (== net6.0) (&& (== netstandard2.0) (>= netcoreapp3.1))
@@ -935,21 +935,21 @@ NUGET
       Microsoft.Build.Tasks.Core (>= 16.10) - restriction: >= netstandard2.0
       Microsoft.Build.Utilities.Core (>= 16.10) - restriction: >= netstandard2.0
     Newtonsoft.Json (13.0.1) - restriction: >= netstandard2.0
-    NuGet.Common (6.2.1) - restriction: >= netstandard2.0
-      NuGet.Frameworks (>= 6.2.1) - restriction: || (>= net45) (>= netstandard2.0)
-    NuGet.Configuration (6.2.1) - restriction: >= netstandard2.0
-      NuGet.Common (>= 6.2.1) - restriction: || (>= net45) (>= netstandard2.0)
-      System.Security.Cryptography.ProtectedData (>= 4.4) - restriction: && (< net45) (>= netstandard2.0)
-    NuGet.Frameworks (6.2.1) - restriction: >= netstandard2.0
-    NuGet.Packaging (6.2.1) - restriction: >= netstandard2.0
+    NuGet.Common (6.3) - restriction: >= netstandard2.0
+      NuGet.Frameworks (>= 6.3) - restriction: >= netstandard2.0
+    NuGet.Configuration (6.3) - restriction: >= netstandard2.0
+      NuGet.Common (>= 6.3) - restriction: >= netstandard2.0
+      System.Security.Cryptography.ProtectedData (>= 4.4) - restriction: && (< net472) (>= netstandard2.0)
+    NuGet.Frameworks (6.3) - restriction: >= netstandard2.0
+    NuGet.Packaging (6.3) - restriction: >= netstandard2.0
       Newtonsoft.Json (>= 13.0.1) - restriction: >= netstandard2.0
-      NuGet.Configuration (>= 6.2.1) - restriction: >= netstandard2.0
-      NuGet.Versioning (>= 6.2.1) - restriction: >= netstandard2.0
+      NuGet.Configuration (>= 6.3) - restriction: >= netstandard2.0
+      NuGet.Versioning (>= 6.3) - restriction: >= netstandard2.0
       System.Security.Cryptography.Cng (>= 5.0) - restriction: || (&& (< net472) (>= netstandard2.0)) (>= net5.0)
       System.Security.Cryptography.Pkcs (>= 5.0) - restriction: || (&& (< net472) (>= netstandard2.0)) (>= net5.0)
-    NuGet.Protocol (6.2.1) - restriction: >= netstandard2.0
-      NuGet.Packaging (>= 6.2.1) - restriction: >= netstandard2.0
-    NuGet.Versioning (6.2.1) - restriction: >= netstandard2.0
+    NuGet.Protocol (6.3) - restriction: >= netstandard2.0
+      NuGet.Packaging (>= 6.3) - restriction: >= netstandard2.0
+    NuGet.Versioning (6.3) - restriction: >= netstandard2.0
     Octokit (0.48)
     System.Buffers (4.5.1) - restriction: || (&& (>= monoandroid) (>= net6.0) (< netstandard1.3)) (&& (>= monotouch) (>= net6.0)) (&& (>= net461) (>= netstandard2.1)) (&& (< net461) (< net6.0) (>= netstandard2.0)) (&& (< net461) (>= netstandard2.0) (< netstandard2.1)) (>= net472) (&& (>= net6.0) (< netcoreapp2.0)) (&& (>= net6.0) (< netstandard2.1)) (&& (>= net6.0) (>= xamarinios)) (&& (>= net6.0) (>= xamarinmac)) (&& (>= net6.0) (>= xamarintvos)) (&& (>= net6.0) (>= xamarinwatchos)) (&& (< net6.0) (>= netstandard2.1))
     System.CodeDom (6.0) - restriction: || (&& (< net472) (>= netstandard2.0)) (>= net6.0)
@@ -991,11 +991,11 @@ NUGET
       System.Formats.Asn1 (>= 6.0) - restriction: || (&& (< net461) (>= netstandard2.0)) (>= netstandard2.1)
       System.Memory (>= 4.5.4) - restriction: && (< net461) (>= netstandard2.0) (< netstandard2.1)
       System.Security.Cryptography.Cng (>= 5.0) - restriction: || (&& (< net461) (>= netstandard2.0) (< netstandard2.1)) (&& (< net6.0) (>= netcoreapp3.1)) (&& (< netcoreapp3.1) (>= netstandard2.1))
-    System.Security.Cryptography.ProtectedData (6.0) - restriction: || (&& (< net45) (>= netstandard2.0)) (&& (< net461) (>= net472)) (>= net6.0)
-    System.Security.Cryptography.Xml (6.0) - restriction: || (&& (< net472) (>= netstandard2.0)) (>= net6.0)
+    System.Security.Cryptography.ProtectedData (6.0) - restriction: || (&& (< net461) (>= net472)) (&& (< net472) (>= netstandard2.0)) (>= net6.0)
+    System.Security.Cryptography.Xml (6.0.1) - restriction: || (&& (< net472) (>= netstandard2.0)) (>= net6.0)
       System.Memory (>= 4.5.4) - restriction: && (< net461) (< net6.0) (>= netstandard2.0)
       System.Security.AccessControl (>= 6.0) - restriction: || (>= net461) (>= netstandard2.0)
-      System.Security.Cryptography.Pkcs (>= 6.0) - restriction: || (&& (< net461) (>= netstandard2.0)) (>= net6.0)
+      System.Security.Cryptography.Pkcs (>= 6.0.1) - restriction: || (&& (< net461) (>= netstandard2.0)) (>= net6.0)
     System.Security.Permissions (6.0) - restriction: >= netstandard2.0
       System.Security.AccessControl (>= 6.0) - restriction: || (>= net461) (>= netstandard2.0)
       System.Windows.Extensions (>= 6.0) - restriction: >= netcoreapp3.1

diff --git a/src/FsAutoComplete.Core/Commands.fs b/src/FsAutoComplete.Core/Commands.fs
@@ -243,24 +243,7 @@ type Commands(checker: FSharpCompilerServiceChecker, state: State, hasAnalyzers:
         >> Log.addContextDestructured "optionCount" count
       ))
 
-  do
-    disposables.Add
-    <| checker.FileChecked.Subscribe(fun (n, _) ->
-      checkerLogger.info (Log.setMessage "{file} checked" >> Log.addContextDestructured "file" n)
-
-      async {
-        try
-          match state.GetProjectOptions n with
-          | Some opts ->
-            let! res = checker.GetBackgroundCheckResultsForFileInProject(n, opts)
-            fileChecked.Trigger(res, res.FileName, -1) // filename comes from compiler, safe to just tag here
-          | _ -> ()
-        with _ ->
-          ()
-      }
-      |> Async.Start)
-
-  //Triggered by `FSharpChecker.FileChecked` if background service is disabled
+  //Diagnostics handler - Triggered by `CheckCore`
   do
     disposables.Add
     <| fileChecked.Publish.Subscribe(fun (parseAndCheck, file, _) ->
@@ -283,6 +266,7 @@ type Commands(checker: FSharpCompilerServiceChecker, state: State, hasAnalyzers:
       }
       |> Async.Start)
 
+  //Analyzers handler - Triggered by `CheckCore`
   do
     disposables.Add
     <| fileChecked.Publish.Subscribe(fun (parseAndCheck, file, _) ->
@@ -339,6 +323,7 @@ type Commands(checker: FSharpCompilerServiceChecker, state: State, hasAnalyzers:
       }
       |> Async.Start)
 
+    //Test detection handler
     do
       disposables.Add
       <| fileParsed.Publish.Subscribe(fun parseResults ->
@@ -818,10 +803,9 @@ type Commands(checker: FSharpCompilerServiceChecker, state: State, hasAnalyzers:
     async {
       match! checker.ParseAndCheckFileInProject(fileName, version, text, options) with
       | Ok parseAndCheck ->
-        let parseResult = parseAndCheck.GetParseResults
-        do fileParsed.Trigger parseResult
         do lastCheckResult <- Some parseAndCheck
         do state.SetLastCheckedVersion fileName version
+        do fileParsed.Trigger parseAndCheck.GetParseResults
         do fileChecked.Trigger(parseAndCheck, fileName, version)
       | Error e -> ()
     }

diff --git a/src/FsAutoComplete.Core/CompilerServiceInterface.fs b/src/FsAutoComplete.Core/CompilerServiceInterface.fs
@@ -20,12 +20,10 @@ type FSharpCompilerServiceChecker(hasAnalyzers) =
       keepAssemblyContents = hasAnalyzers,
       suggestNamesForErrors = true,
       enablePartialTypeChecking = not hasAnalyzers,
-      enableBackgroundItemKeyStoreAndSemanticClassification = true
+      enableBackgroundItemKeyStoreAndSemanticClassification = true,
+      keepAllBackgroundSymbolUses = true
     )
 
-  // we only want to let people hook onto the underlying checker event if there's not a background service actually compiling things for us
-  let safeFileCheckedEvent = checker.FileChecked
-
   // /// FCS only accepts absolute file paths, so this ensures that by
   // /// rooting relative paths onto HOME on *nix and %HOMRDRIVE%%HOMEPATH% on windows
   // let ensureAbsolutePath path =
@@ -39,7 +37,6 @@ type FSharpCompilerServiceChecker(hasAnalyzers) =
 
   let entityCache = EntityCache()
 
-  let sdkRefsLogger = LogProvider.getLoggerByName "SdkRefs"
   let checkerLogger = LogProvider.getLoggerByName "Checker"
   let optsLogger = LogProvider.getLoggerByName "Opts"
 
@@ -238,21 +235,6 @@ type FSharpCompilerServiceChecker(hasAnalyzers) =
       return projOptions
     }
 
-  member __.GetBackgroundCheckResultsForFileInProject(fn: string<LocalPath>, opt) =
-    checkerLogger.info (
-      Log.setMessage "GetBackgroundCheckResultsForFileInProject - {file}"
-      >> Log.addContextDestructured "file" fn
-    )
-
-    let opt = clearProjectReferences opt
-
-    checker.GetBackgroundCheckResultsForFileInProject(UMX.untag fn, opt)
-    |> Async.map (fun (pr, cr) -> ParseAndCheckResults(pr, cr, entityCache))
-
-  member __.FileChecked: IEvent<string<LocalPath> * FSharpProjectOptions> =
-    safeFileCheckedEvent
-    |> Event.map (fun (fileName, blob) -> UMX.tag fileName, blob) //path comes from the compiler, so it's safe to assume the tag in this case
-
   member __.ScriptTypecheckRequirementsChanged =
     scriptTypecheckRequirementsChanged.Publish
 
@@ -296,8 +278,6 @@ type FSharpCompilerServiceChecker(hasAnalyzers) =
         return ResultOrString.Error(ex.ToString())
     }
 
-  member _.CheckProject(opts) = checker.ParseAndCheckProject(opts)
-
   member __.TryGetRecentCheckResultsForFile(file: string<LocalPath>, options, source: NamedText) =
     let opName = sprintf "TryGetRecentCheckResultsForFile - %A" file
 
@@ -320,25 +300,24 @@ type FSharpCompilerServiceChecker(hasAnalyzers) =
         >> Log.addContextDestructured "file" file
       )
 
-      let projects = x.GetDependingProjects file options
-
-      return!
-        match projects with
-        | None -> async { return [||] }
-        | Some (p, projects) ->
-          async {
-            let! res =
-              p :: projects
-              |> Seq.map (fun (opts) ->
-                async {
-                  let opts = clearProjectReferences opts
-                  let! res = checker.ParseAndCheckProject opts
-                  return res.GetUsesOfSymbol symbol
-                })
-              |> Async.Parallel
-
-            return res |> Array.concat
-          }
+      match x.GetDependingProjects file options with
+      | None -> return [||]
+      | Some (opts, []) ->
+        let opts = clearProjectReferences opts
+        let! res = checker.ParseAndCheckProject opts
+        return res.GetUsesOfSymbol symbol
+      | Some (opts, dependentProjects) ->
+        let! res =
+          opts :: dependentProjects
+          |> List.map (fun (opts) ->
+            async {
+              let opts = clearProjectReferences opts
+              let! res = checker.ParseAndCheckProject opts
+              return res.GetUsesOfSymbol symbol
+            })
+          |> Async.Parallel
+
+        return res |> Array.concat
     }
 
   member _.FindReferencesForSymbolInFile(file, project, symbol) =
@@ -361,10 +340,6 @@ type FSharpCompilerServiceChecker(hasAnalyzers) =
       return parseResult.GetNavigationItems().Declarations
     }
 
-  member __.Compile = checker.Compile
-
-  member internal __.GetFSharpChecker() = checker
-
   member __.SetDotnetRoot(dotnetBinary: FileInfo, cwd: DirectoryInfo) =
     match Ionide.ProjInfo.SdkDiscovery.versionAt cwd dotnetBinary with
     | Ok sdkVersion ->

diff --git a/test/FsAutoComplete.Tests.Lsp/GoToTests.fs b/test/FsAutoComplete.Tests.Lsp/GoToTests.fs
@@ -152,7 +152,7 @@ let private gotoTest state =
                    "Result should have correct range"
            })
 
-         testCaseAsync
+         ptestCaseAsync
            "Go-to-implementation-on-interface-definition"
            (async {
              let! server, path, externalPath, definitionPath = server