Allow loading of projects that are missing imports. (#206)

## The Problem

To load project files with missing Imports, we need to supply one or two ProjectLoadSettings flags to everywhere a project file could be parsed. These flags are `ProjectLoadSettings.IgnoreMissingImports` and `ProjectLoadSettings.IgnoreInvalidImports`. Ideally we need to find everywhere that a project would be loaded (so anywhere `Project(...)` or `ProjectInstance(...)` occur) and make sure these flags are applied.

## Phase 1 - protecting the main constructors

For our purposes there are two main places that `ProjectLoadSettings` can be specified
* when the `Project` constructor is called in the `WorkspaceLoader`
* as part of the `buildParameters` for the `WorkspaceLoaderViaProjectGraph`

Applying these changes to the existing call-sites was fairly easy - there were something like 6 places that Projects/Project Instances were directly created. Here we come to our first hurdle - the `ProjectInstance` constructor doesn't surface `ProjectLoadSettings` in any way. So step one was to turn any sort of `ProjectInstance` creation into a two-phase operation:
* create the `Project` with appropriate load settings
* create the `ProjectInstance` from that `Project`

This got us most of the way through the tests.

## Phase 2 - TFM detection

With one hurdle - the way we detected the TFM for a project involved loading a `ProjectInstance` directly and reading properties from it, and this turned out to be error prone because of the reasons mentioned above - `ProjectInstance` doesn't have `ProjectLoadSettings`. So we needed to create a `Project` to get the `ProjectInstance` from, as described above.

## Phase 3 - ProjectCollection management

The above fix worked for more tests, but others still failed because the 'same project' was being created (and implicitly assigned to the global `ProjectCollection`) multiple times - a big no-no for `ProjectCollections`. So this required two changes:
* create and manage a `ProjectCollection` as a 'container' for a given call to `LoadProjects` - this allows us to cache the evaluations and design-time builds over the course of a single call to `LoadProjects` while also not cluttering/clobbering the global default `ProjectCollection`
* implement a function that safely retrieves an existing `Project` from the `ProjectCollection` if one exists for the same project path + global properties - if not, a new `Project` is created. 

With these two changes, all the tests (including the new tests) became green.

.github/workflows/build.yml

@@ -74,12 +74,6 @@ jobs:
         env:
           BuildNet7: ${{ matrix.build_net7 }}
 
-      - name: Upload NuGet packages
-        uses: actions/upload-artifact@v2
-        with:
-          name: packages
-          path: src/**/*.nupkg
-
       - name: Archive test results
         uses: actions/upload-artifact@v3
 

.vscode/launch.json

@@ -39,7 +39,6 @@
             "name": "Ionide.ProjInfo.Tests",
             "type": "coreclr",
             "request": "launch",
-            "preLaunchTask": "build",
             "program": "${workspaceFolder}/test/Ionide.ProjInfo.Tests/bin/Debug/${input:tfm}/Ionide.ProjInfo.Tests.dll",
             "args": [
                 "--filter",

test/Ionide.ProjInfo.Tests/TestAssets.fs

@@ -300,7 +300,13 @@ let ``NetSDK library referencing ProduceReferenceAssembly library`` = {
         "l2"
         / "l2.fsproj"
     TargetFrameworks = Map.ofList [ "netstandard2.0", sourceFiles [ "Library.fs" ] ]
-    ProjectReferences = [
-        ``NetSDK library with ProduceReferenceAssembly``
-    ]
+    ProjectReferences = [ ``NetSDK library with ProduceReferenceAssembly`` ]
+}
+
+let ``Console app with missing direct Import`` = {
+    ProjDir = "missing-import"
+    AssemblyName = "missing-import"
+    ProjectFile = "missing-import.fsproj"
+    TargetFrameworks = Map.ofList [ "net6.0", sourceFiles [ "Program.fs" ] ]
+    ProjectReferences = []
 }

test/Ionide.ProjInfo.Tests/Tests.fs

@@ -38,8 +38,7 @@ let pathForProject (test: TestAssetProjInfo) =
     pathForTestAssets test
     / test.ProjectFile
 
-let implAssemblyForProject (test: TestAssetProjInfo) =
-    $"{test.AssemblyName}.dll"
+let implAssemblyForProject (test: TestAssetProjInfo) = $"{test.AssemblyName}.dll"
 
 let refAssemblyForProject (test: TestAssetProjInfo) =
     Path.Combine("ref", implAssemblyForProject test)
@@ -366,7 +365,7 @@ let testLegacyFrameworkMultiProject toolsPath workspaceLoader isRelease (workspa
 let testSample2 toolsPath workspaceLoader isRelease (workspaceFactory: ToolsPath * (string * string) list -> IWorkspaceLoader) =
     testCase
     |> withLog
-        (sprintf "can load sample2 - %s - isRelease is %b" workspaceLoader isRelease)
+        (sprintf "can load sample2 - isRelease is %b - %s" isRelease workspaceLoader)
         (fun logger fs ->
             let testDir = inDir fs "load_sample2"
             copyDirFromAssets fs ``sample2 NetSdk library``.ProjDir testDir
@@ -1765,12 +1764,14 @@ let testLoadProject toolsPath =
             ]
             |> checkExitCodeZero
 
-            let projResult = ProjectLoader.getProjectInfo projPath [] BinaryLogGeneration.Off []
+            let collection = new Microsoft.Build.Evaluation.ProjectCollection()
 
-            match projResult with
-            | Result.Ok proj -> Expect.equal proj.ProjectFileName projPath "project file names"
+            match ProjectLoader.loadProject projPath BinaryLogGeneration.Off collection with
             | Result.Error err -> failwith $"{err}"
-
+            | Result.Ok proj ->
+                match ProjectLoader.getLoadedProjectInfo projPath [] proj with
+                | Result.Ok proj -> Expect.equal proj.ProjectFileName projPath "project file names"
+                | Result.Error err -> failwith $"{err}"
         )
 
 let testProjectSystem toolsPath workspaceLoader workspaceFactory =
@@ -2067,7 +2068,7 @@ let addFileToProject (projPath: string) fileName =
 let loadProjfileFromDiskTests toolsPath workspaceLoader (workspaceFactory: ToolsPath -> IWorkspaceLoader) =
     testCase
     |> withLog
-        $"can load project from disk everytime {workspaceLoader}"
+        $"can load project from disk everytime - {workspaceLoader}"
         (fun logger fs ->
 
             let loader = workspaceFactory toolsPath
@@ -2157,14 +2158,17 @@ let referenceAssemblySupportTest toolsPath prefix (workspaceFactory: ToolsPath -
                 |> Seq.toList
 
             Expect.hasLength parsed 2 "Should have loaded the F# lib and the referenced F# lib"
-            let fsharpProject = parsed |> Seq.find (fun p -> Path.GetFileName(p.ProjectFileName) = Path.GetFileName(childProj.ProjectFile))
+
+            let fsharpProject =
+                parsed
+                |> Seq.find (fun p -> Path.GetFileName(p.ProjectFileName) = Path.GetFileName(childProj.ProjectFile))
+
             let mapped = FCS.mapToFSharpProjectOptions fsharpProject parsed
             let referencedProjects = mapped.ReferencedProjects
             Expect.hasLength referencedProjects 1 "Should have a reference to the F# ProjectReference lib"
 
             match referencedProjects[0] with
-            | FSharpReferencedProject.FSharpReference(targetPath, _) ->
-                Expect.stringContains targetPath  (refAssemblyForProject parentProj) "Should have found the ref assembly for the F# lib"
+            | FSharpReferencedProject.FSharpReference(targetPath, _) -> Expect.stringContains targetPath (refAssemblyForProject parentProj) "Should have found the ref assembly for the F# lib"
             | _ -> failwith "Should have found a F# reference"
         )
 
@@ -2180,6 +2184,47 @@ let testProjectLoadBadData =
             Expect.isNone proj.Response "should have loaded, detected bad data, and defaulted to empty"
         )
 
+let canLoadMissingImports toolsPath loaderType (workspaceFactory: ToolsPath -> IWorkspaceLoader) =
+    testCase
+        $"Can load projects with missing Imports - {loaderType}"
+        (fun () ->
+            let proj = ``Console app with missing direct Import``
+            let projPath = pathForProject proj
+
+            let loader = workspaceFactory toolsPath
+            let logger = Log.create (sprintf "Test '%s'" $"Can load projects with missing Imports - {loaderType}")
+
+            loader.Notifications.Add(
+                function
+                | WorkspaceProjectState.Failed(projPath, errors) ->
+                    logger.error (
+                        Message.eventX "Failed to load project {project} with {errors}"
+                        >> Message.setField "project" projPath
+                        >> Message.setField "errors" errors
+                    )
+                | WorkspaceProjectState.Loading p ->
+                    logger.info (
+                        Message.eventX "Loading project {project}"
+                        >> Message.setField "project" p
+                    )
+                | WorkspaceProjectState.Loaded(p, knownProjects, fromCache) ->
+                    logger.info (
+                        Message.eventX "Loaded project {project}(fromCache: {fromCache})"
+                        >> Message.setField "project" p
+                        >> Message.setField "fromCache" fromCache
+                    )
+            )
+
+            let parsed =
+                loader.LoadProjects [ projPath ]
+                |> Seq.toList
+
+            Expect.equal parsed.Length 1 "Should have loaded the project"
+            let parsed = parsed[0]
+            Expect.equal 3 parsed.SourceFiles.Length "Should have Program.fs, AssemblyInfo, and AssemblyAttributes"
+            Expect.stringEnds parsed.SourceFiles[2] "Program.fs" "Filename should be Program.fs"
+        )
+
 let tests toolsPath =
     let testSample3WorkspaceLoaderExpected = [
         ExpectNotification.loading "c1.fsproj"
@@ -2297,7 +2342,10 @@ let tests toolsPath =
         expensiveTests toolsPath WorkspaceLoader.Create
         csharpLibTest toolsPath WorkspaceLoader.Create
 
-        referenceAssemblySupportTest toolsPath (nameof(WorkspaceLoader)) WorkspaceLoader.Create
-        referenceAssemblySupportTest toolsPath (nameof(WorkspaceLoaderViaProjectGraph)) WorkspaceLoaderViaProjectGraph.Create
+        referenceAssemblySupportTest toolsPath (nameof (WorkspaceLoader)) WorkspaceLoader.Create
+        referenceAssemblySupportTest toolsPath (nameof (WorkspaceLoaderViaProjectGraph)) WorkspaceLoaderViaProjectGraph.Create
 
+        // tests that cover our ability to handle missing imports
+        canLoadMissingImports toolsPath (nameof (WorkspaceLoader)) WorkspaceLoader.Create
+        canLoadMissingImports toolsPath (nameof (WorkspaceLoaderViaProjectGraph)) WorkspaceLoaderViaProjectGraph.Create
     ]

test/examples/missing-import/Program.fs

@@ -0,0 +1,3 @@
+[<EntryPoint>]
+let main args =
+    1

test/examples/missing-import/missing-import.fsproj

@@ -0,0 +1,14 @@
+<Project Sdk="Microsoft.NET.Sdk">
+
+  <PropertyGroup>
+    <TargetFramework>net6.0</TargetFramework>
+    <RootNamespace>missing_import</RootNamespace>
+  </PropertyGroup>
+
+  <ItemGroup>
+    <Compile Include="Program.fs" />
+  </ItemGroup>
+
+  <Import Project="$(MSBuildExtensionsPath)Foo\Foo.targets" />
+
+</Project>