More testing

.github/workflows/build.yml

@@ -28,6 +28,7 @@ jobs:
         dotnet-version: ["7.0.x"]
         CodeFixTests: ["lsp", "direct"]
         TestSet: ["justCodeFix", "all"]
+        Checker: ["OneCheckerOneFile", "OneCheckerCachedFiles", "CachedCheckersOneFile"]
         # these entries will mesh with the above combinations
         include:
           # # just use what's in the repo
@@ -55,7 +56,7 @@ jobs:
 
     runs-on: ${{ matrix.os }}
 
-    name: Build on ${{matrix.os}} for ${{ matrix.label }} (CodeFixeTests=${{ matrix.CodeFixTests}}; TestSet=${{ matrix.TestSet}} )
+    name: Build on ${{matrix.os}} for ${{ matrix.label }} (TestSet=${{ matrix.TestSet}}; CodeFixeTests=${{ matrix.CodeFixTests}}; Checker = ${{ matrix.Checker }})
 
     steps:
     - uses: actions/checkout@v3
@@ -96,5 +97,6 @@ jobs:
       working-directory: test/FsAutoComplete.Tests.Lsp
       env:
         BuildNet7: ${{ matrix.build_net7 }}
-        TestSet: ${{ matrix.TestSet }}
-        CodeFixTests: ${{ matrix.CodeFixTests }}
+        FSAC_TestSet: ${{ matrix.TestSet }}
+        FSAC_CodeFixTests: ${{ matrix.CodeFixTests }}
+        FSAC_Checker: ${{ matrix.Checker }}

test/FsAutoComplete.Tests.Lsp/CodeFixTests/Direct/DirectCodeFix.fs

@@ -1,4 +1,4 @@
-module private FsAutoComplete.Tests.CodeFixTests.Direct.DirectCodeFix
+module FsAutoComplete.Tests.CodeFixTests.Direct.DirectCodeFix
 
 open System
 open Helpers
@@ -185,96 +185,179 @@ module SourceData =
     return diags
   }
 
+[<RequireQualifiedAccess>]
+type Checker =
+  | Single of FSharpChecker
+  | Multi of ConcurrentBag<FSharpChecker>
+[<RequireQualifiedAccess>]
+type FileName =
+  | Single of string<LocalPath>
+  | Multi of ConcurrentBag<string<LocalPath>>
+
+type InternalCheckImpl =
+  abstract GetChecker: unit -> FSharpChecker
+  abstract ReturnChecker: FSharpChecker -> unit
+
+  abstract GetFileName: unit -> string<LocalPath>
+  abstract ReturnFileName: string<LocalPath> -> unit
+module InternalCheckImpl =
+  open System.Threading
+
+  let defaultProjectOptions (checker: InternalCheckImpl) =
+    let chkr = checker.GetChecker()
+    let fileName = checker.GetFileName()
+    try
+      let projOptions =
+        //TODO: settings?
+        chkr.GetProjectOptionsFromScript(
+          UMX.untag fileName, 
+          SourceText.ofString "",
+          assumeDotNetFramework = false,
+          useSdkRefs = true,
+          // useFsiAuxLib = false,
+          useFsiAuxLib = true,
+          otherFlags = [|
+            "--preferreduilang:en-US"
+            "--targetprofile:netcore"
+            // "--targetprofile:netstandard"
+          |]
+        )
+        //TODO: make async?
+        |> Async.RunSynchronously
+        |> fun (o, diags) -> Expect.isEmpty diags "There should be no proj diags"; o
+
+      projOptions
+    finally
+      checker.ReturnFileName fileName
+      checker.ReturnChecker chkr
+
+  let createChecker () =
+    FSharpChecker.Create(
+      projectCacheSize = 0,
+      keepAssemblyContents = false,
+      keepAllBackgroundResolutions = false,
+      useSyntaxTreeCache = false,
+
+      // Adds `Maybe you want one of the following: ...`
+      // for error `The value or constructor 'XXX' is not defined.`
+      suggestNamesForErrors = true
+    )
+  let fileName = "/test.fsx" |> Utils.normalizePath 
+  let private root = 
+    let name = UMX.untag fileName 
+    name.Substring(0, name.Length - 4)
+  let createFileName (i: int) : string<LocalPath> = UMX.tag $"{root}{i}.fsx"
+
+  type OneCheckerOneFile = 
+    {
+      FileName: string<LocalPath>
+      Checker: FSharpChecker
+    }
+    interface InternalCheckImpl with
+      member t.GetFileName() = t.FileName
+      member t.ReturnFileName _ = ()
+      member t.GetChecker() = t.Checker
+      member t.ReturnChecker _ = ()
+  let oneCheckerOneFile () =
+    {
+      FileName = fileName
+      Checker = createChecker ()
+    }
+  type OneCheckerCachedFiles =
+    {
+      mutable fileCounter: int
+      FileNameTemplate: int -> string<LocalPath>
+      FileNames: ConcurrentBag<string<LocalPath>>
+      Checker: FSharpChecker
+    }
+    interface InternalCheckImpl with
+      member t.GetFileName() =
+        match t.FileNames.TryTake() with
+        | true, fileName -> fileName
+        | false, _ ->
+            let i = Interlocked.Increment(&t.fileCounter)
+            t.FileNameTemplate i
+      member t.ReturnFileName fileName = t.FileNames.Add fileName
+      member t.GetChecker() = t.Checker
+      member t.ReturnChecker _ = ()
+  let oneCheckerCachedFiles () =
+    {
+      fileCounter = 0
+      FileNameTemplate = createFileName
+      FileNames = ConcurrentBag()
+      Checker = createChecker ()
+    }
+  type CachedCheckersOneFile =
+    {
+      FileName: string<LocalPath>
+      CreateChecker: unit -> FSharpChecker
+      Checkers: ConcurrentBag<FSharpChecker>
+    }
+    interface InternalCheckImpl with
+      member t.GetFileName() = t.FileName
+      member t.ReturnFileName _ = ()
+      member t.GetChecker() = 
+        match t.Checkers.TryTake() with
+        | true, checker -> checker
+        | false, _ ->
+            t.CreateChecker ()
+      member t.ReturnChecker checker = t.Checkers.Add checker
+  let cachedCheckersOneFile () =
+    {
+      FileName = fileName
+      CreateChecker = createChecker
+      Checkers = ConcurrentBag()
+    }
+
 type CheckState =
   {
     SourceTextFactory: ISourceTextFactory
-
-    FileName: string<LocalPath>
-    Checker: FSharpChecker
-    Checkers: ConcurrentBag<FSharpChecker>
+    Checker: InternalCheckImpl
+
     ProjectOptions: FSharpProjectOptions
-    mutable FileCounter: int
-    FileNames: ConcurrentBag<string<LocalPath>>
-
+
     /// Can be used to add additional Diagnostics (analyzers)
     AdjustResult: SourceData -> Async<SourceData>
   }
 
+
   member private t.GetChecker() =
-    t.Checker
-    // if t.Checkers.Count >= 2 then
-    //   failwithf ">= 2 checkers: %i" t.Checkers.Count
-    // match t.Checkers.TryTake () with
-    // | true, checker -> checker
-    // | false, _ ->
-    //   FSharpChecker.Create(
-    //     projectCacheSize = 0,
-    //     keepAssemblyContents = false,
-    //     keepAllBackgroundResolutions = false,
-    //     useSyntaxTreeCache = false,
-
-    //     // projectCacheSize = 200,
-    //     // keepAssemblyContents = false,
-    //     // keepAllBackgroundResolutions = true,
-    //     // keepAllBackgroundSymbolUses = true,
-    //     // enableBackgroundItemKeyStoreAndSemanticClassification = true,
-    //     // enablePartialTypeChecking = true,
-    //     // parallelReferenceResolution = true,
-    //     // captureIdentifiersWhenParsing = true,
-    //     // useSyntaxTreeCache = true,
-
-    //     // Adds `Maybe you want one of the following: ...`
-    //     // for error `The value or constructor 'XXX' is not defined.`
-    //     suggestNamesForErrors = true
-    //   )
-  member private t.ReturnChecker checker =
-    // t.Checkers.Add checker
-    ()
-  member private t.GetFileName () =
-    // // let i = System.Threading.Interlocked.Increment (&t.FileCounter)
-    // // let fileName = $"/test%i{i}.fsx" |> Utils.normalizePath 
-    // // fileName
-    // match t.FileNames.TryTake () with
-    // | true, fileName -> fileName
-    // | false, _ ->
-    //     let i = System.Threading.Interlocked.Increment (&t.FileCounter)
-    //     let fileName = $"/test%i{i}.fsx" |> Utils.normalizePath 
-    //     fileName
-    t.FileName
-  member private t.ReturnFileName fileName =
-    // t.FileNames.Add fileName
-    ()
+    t.Checker.GetChecker()
+  member private t.ReturnChecker checker = t.Checker.ReturnChecker checker
+  member private t.GetFileName () = t.Checker.GetFileName ()
+  member private t.ReturnFileName fileName = t.Checker.ReturnFileName fileName
 
   member t.CreateSource (text: string) =
-    t.SourceTextFactory.Create(t.FileName, text)
-  member private t.ParseAndCheck' source = async {
+    t.SourceTextFactory.Create(t.GetFileName (), text)
+  member private t.ParseAndCheck' (source: IFSACSourceText) = async {
     // let checker = t.Checker
     let checker = t.GetChecker ()
-    let fileName = t.GetFileName ()
     try
       // let fileName = t.FileName
-      let projOptions = t.ProjectOptions
-
+      let projOptions =
+        let projOptions = t.ProjectOptions
+        match projOptions.SourceFiles with
+        | [| fn |] when fn = UMX.untag source.FileName -> projOptions
+        | _ -> { t.ProjectOptions with FSharpProjectOptions.SourceFiles = [| UMX.untag source.FileName |] }
       // let projOptions = { t.ProjectOptions with FSharpProjectOptions.SourceFiles = [| UMX.untag fileName |] }
 
-      //TODO: is checker threadsafe?
-      //TODO: pool checkers?
-
       // //TODO: necessary?
       // // quite slow -> ~ 2x
       // checker.InvalidateAll()
       // //TODO: necessary?
       // // extremely slow -> ~ 15x
       // checker.ClearLanguageServiceRootCachesAndCollectAndFinalizeAllTransients()
 
-      let! (parseResults, checkResults) = checker.ParseAndCheckFileInProject(UMX.untag fileName, 0, source, projOptions)
+      let! (parseResults, checkResults) = checker.ParseAndCheckFileInProject(UMX.untag source.FileName, 0, source, projOptions)
       let checkResults =
         match checkResults with
         | FSharpCheckFileAnswer.Aborted -> failwith "aborted"
         | FSharpCheckFileAnswer.Succeeded checkResults -> checkResults
       return ParseAndCheckResults(parseResults, checkResults, EntityCache())
     finally
-      t.ReturnFileName fileName
+      // Note: This is technically unsound: `source` still holds `FileName`.
+      //       But from Checking-perspective: `FileName` usage is done here.
+      t.ReturnFileName source.FileName
       t.ReturnChecker checker
   }
 
@@ -299,52 +382,16 @@ type CheckState =
     t.ParseAndCheck source
     |> Async.Cache
 module CheckState =
-  open System.Collections.Concurrent
   //TODO: use `CompilerServiceInterface` instead?
   //TODO: to async?
   let create
     sourceTextFactory
+    (checker: InternalCheckImpl)
     =
-    //TODO: can one name be used for all? or must be unique?
-    let fileName = "/test.fsx" |> Utils.normalizePath 
-    let checker = 
-      //TODO: settings?
-      FSharpChecker.Create(
-        projectCacheSize = 0,
-        keepAssemblyContents = false,
-        keepAllBackgroundResolutions = false,
-        useSyntaxTreeCache = false,
-
-        // Adds `Maybe you want one of the following: ...`
-        // for error `The value or constructor 'XXX' is not defined.`
-        suggestNamesForErrors = true
-      )
-    let projOptions =
-      //TODO: settings?
-      checker.GetProjectOptionsFromScript(
-        UMX.untag fileName, 
-        SourceText.ofString "",
-        assumeDotNetFramework = false,
-        useSdkRefs = true,
-        // useFsiAuxLib = false,
-        useFsiAuxLib = true,
-        otherFlags = [|
-          "--preferreduilang:en-US"
-          "--targetprofile:netcore"
-          // "--targetprofile:netstandard"
-        |]
-      )
-      //TODO: make async?
-      |> Async.RunSynchronously
-      |> fun (o, diags) -> Expect.isEmpty diags "There should be no proj diags"; o
     {
       SourceTextFactory = sourceTextFactory
-      FileName = fileName
       Checker = checker
-      Checkers = ConcurrentBag()
-      FileCounter = 0
-      FileNames = ConcurrentBag()
-      ProjectOptions = projOptions
+      ProjectOptions = checker |> InternalCheckImpl.defaultProjectOptions
       AdjustResult = Async.retn
     }
 

test/FsAutoComplete.Tests.Lsp/CodeFixTests/Direct/Tests.fs

@@ -3622,9 +3622,9 @@ let private removePatternArgumentTests state =
         let (None) = None
         """ ]
 
-let tests sourceTextFactory =
-  let state =
-    DirectCodeFix.CheckState.create sourceTextFactory
+let tests state =
+  // let state =
+  //   DirectCodeFix.CheckState.create sourceTextFactory
   testList "CodeFix-tests (direct)" [
       AddExplicitTypeAnnotationTests.tests state
       AdjustConstantTests.tests state