Correct IDE diagnostics and lifetimes for editing with #r package references (dotnet#10160)

Co-authored-by: Don Syme <[email protected]>

Author: 2 people

src/fsharp/CompileOps.fs

@@ -413,8 +413,6 @@ type TcConfigBuilder =
       mutable pathMap : PathMap
 
       mutable langVersion : LanguageVersion
-
-      mutable dependencyProvider: DependencyProvider
     }
 
     static member Initial: TcConfigBuilder
@@ -443,6 +441,9 @@ type TcConfigBuilder =
 
     static member SplitCommandLineResourceInfo: string -> string * string * ILResourceAccess
 
+    // Directories to start probing in for native DLLs for FSI dynamic loading
+    member GetNativeProbingRoots: unit -> seq<string>
+
 [<Sealed>]
 // Immutable TcConfig
 type TcConfig =
@@ -559,8 +560,8 @@ type TcConfig =
     member isInteractive: bool
     member isInvalidationSupported: bool 
 
-
     member ComputeLightSyntaxInitialStatus: string -> bool
+
     member GetTargetFrameworkDirectories: unit -> string list
     
     /// Get the loaded sources that exist and issue a warning for the ones that don't
@@ -575,8 +576,11 @@ type TcConfig =
     member MakePathAbsolute: string -> string
 
     member copyFSharpCore: CopyFSharpCoreFlag
+
     member shadowCopyReferences: bool
+
     member useSdkRefs: bool
+
     member langVersion: LanguageVersion
 
     static member Create: TcConfigBuilder * validate: bool -> TcConfig
@@ -640,18 +644,28 @@ type TcImports =
     interface System.IDisposable
     //new: TcImports option -> TcImports
     member DllTable: NameMap<ImportedBinary> with get
+
     member GetImportedAssemblies: unit -> ImportedAssembly list
+
     member GetCcusInDeclOrder: unit -> CcuThunk list
+
     /// This excludes any framework imports (which may be shared between multiple builds)
     member GetCcusExcludingBase: unit -> CcuThunk list 
+
     member FindDllInfo: CompilationThreadToken * range * string -> ImportedBinary
+
     member TryFindDllInfo: CompilationThreadToken * range * string * lookupOnly: bool -> option<ImportedBinary>
+
     member FindCcuFromAssemblyRef: CompilationThreadToken * range * ILAssemblyRef -> CcuResolutionResult
+
 #if !NO_EXTENSIONTYPING
     member ProviderGeneratedTypeRoots: ProviderGeneratedType list
 #endif
+
     member GetImportMap: unit -> Import.ImportMap
 
+    member DependencyProvider: DependencyProvider
+
     /// Try to resolve a referenced assembly based on TcConfig settings.
     member TryResolveAssemblyReference: CompilationThreadToken * AssemblyReference * ResolveAssemblyReferenceMode -> OperationResult<AssemblyResolution list>
 
@@ -671,13 +685,33 @@ type TcImports =
 #endif
     /// Report unresolved references that also weren't consumed by any type providers.
     member ReportUnresolvedAssemblyReferences: UnresolvedAssemblyReference list -> unit
+
     member SystemRuntimeContainsType: string -> bool
 
     member internal Base: TcImports option
 
-    static member BuildFrameworkTcImports     : CompilationThreadToken * TcConfigProvider * AssemblyResolution list * AssemblyResolution list -> Cancellable<TcGlobals * TcImports>
-    static member BuildNonFrameworkTcImports  : CompilationThreadToken * TcConfigProvider * TcGlobals * TcImports * AssemblyResolution list * UnresolvedAssemblyReference list -> Cancellable<TcImports>
-    static member BuildTcImports              : CompilationThreadToken * TcConfigProvider -> Cancellable<TcGlobals * TcImports>
+    static member BuildFrameworkTcImports:
+        CompilationThreadToken *
+        TcConfigProvider *
+        AssemblyResolution list *
+        AssemblyResolution list
+            -> Cancellable<TcGlobals * TcImports>
+
+    static member BuildNonFrameworkTcImports:
+        CompilationThreadToken * 
+        TcConfigProvider * 
+        TcGlobals * 
+        TcImports * 
+        AssemblyResolution list * 
+        UnresolvedAssemblyReference list * 
+        DependencyProvider 
+            -> Cancellable<TcImports>
+
+    static member BuildTcImports:
+        CompilationThreadToken *
+        TcConfigProvider * 
+        DependencyProvider 
+            -> Cancellable<TcGlobals * TcImports>
 
 //----------------------------------------------------------------------------
 // Special resources in DLLs
@@ -703,24 +737,22 @@ val WriteOptimizationData: TcGlobals * filename: string * inMem: bool * CcuThunk
 // #r and other directives
 //--------------------------------------------------------------------------
 
-//----------------------------------------------------------------------------
-// #r and other directives
-//--------------------------------------------------------------------------
-
 /// Process #r in F# Interactive.
 /// Adds the reference to the tcImports and add the ccu to the type checking environment.
 val RequireDLL: CompilationThreadToken * TcImports * TcEnv * thisAssemblyName: string * referenceRange: range * file: string -> TcEnv * (ImportedBinary list * ImportedAssembly list)
 
-/// Processing # commands
+/// A general routine to process hash directives
 val ProcessMetaCommandsFromInput : 
-    (('T -> range * string -> 'T) * ('T -> range * string -> 'T) * ('T -> IDependencyManagerProvider * Directive * range * string -> 'T) * ('T -> range * string -> unit))
-    -> TcConfigBuilder * ParsedInput * string * 'T 
-    -> 'T
+    (('T -> range * string -> 'T) * 
+     ('T -> range * string * Directive -> 'T) *
+     ('T -> range * string -> unit))
+      -> TcConfigBuilder * ParsedInput * string * 'T 
+      -> 'T
 
-/// Process all the #r, #I etc. in an input
-val ApplyMetaCommandsFromInputToTcConfig: TcConfig * ParsedInput * string -> TcConfig
+/// Process all the #r, #I etc. in an input.  For non-scripts report warnings about ignored directives.
+val ApplyMetaCommandsFromInputToTcConfig: TcConfig * ParsedInput * string * DependencyProvider -> TcConfig
 
-/// Process the #nowarn in an input
+/// Process the #nowarn in an input and integrate them into the TcConfig
 val ApplyNoWarnsToTcConfig: TcConfig * ParsedInput * string -> TcConfig
 
 //----------------------------------------------------------------------------
@@ -827,6 +859,9 @@ type LoadClosure =
       /// The resolved references along with the ranges of the #r positions in each file.
       References: (string * AssemblyResolution list) list
 
+      /// The resolved pacakge references along with the ranges of the #r positions in each file.
+      PackageReferences: (range * string list)[]
+
       /// The list of references that were not resolved during load closure.
       UnresolvedReferences: UnresolvedAssemblyReference list
 
@@ -853,8 +888,31 @@ type LoadClosure =
     //
     /// A temporary TcConfig is created along the way, is why this routine takes so many arguments. We want to be sure to use exactly the
     /// same arguments as the rest of the application.
-    static member ComputeClosureOfScriptText: CompilationThreadToken * legacyReferenceResolver: ReferenceResolver.Resolver * defaultFSharpBinariesDir: string * filename: string * sourceText: ISourceText * implicitDefines:CodeContext * useSimpleResolution: bool * useFsiAuxLib: bool * useSdkRefs: bool * lexResourceManager: Lexhelp.LexResourceManager * applyCompilerOptions: (TcConfigBuilder -> unit) * assumeDotNetFramework: bool * tryGetMetadataSnapshot: ILReaderTryGetMetadataSnapshot * reduceMemoryUsage: ReduceMemoryFlag -> LoadClosure
+    static member ComputeClosureOfScriptText:
+        CompilationThreadToken * 
+        legacyReferenceResolver: ReferenceResolver.Resolver * 
+        defaultFSharpBinariesDir: string * 
+        filename: string * 
+        sourceText: ISourceText * 
+        implicitDefines:CodeContext * 
+        useSimpleResolution: bool * 
+        useFsiAuxLib: bool * 
+        useSdkRefs: bool * 
+        lexResourceManager: Lexhelp.LexResourceManager * 
+        applyCompilerOptions: (TcConfigBuilder -> unit) * 
+        assumeDotNetFramework: bool * 
+        tryGetMetadataSnapshot: ILReaderTryGetMetadataSnapshot *
+        reduceMemoryUsage: ReduceMemoryFlag *
+        dependencyProvider: DependencyProvider
+          -> LoadClosure
 
     /// Analyze a set of script files and find the closure of their references. The resulting references are then added to the given TcConfig.
     /// Used from fsi.fs and fsc.fs, for #load and command line. 
-    static member ComputeClosureOfScriptFiles: CompilationThreadToken * tcConfig:TcConfig * (string * range) list * implicitDefines:CodeContext * lexResourceManager: Lexhelp.LexResourceManager -> LoadClosure
+    static member ComputeClosureOfScriptFiles: 
+        CompilationThreadToken * 
+        tcConfig:TcConfig * 
+        (string * range) list * 
+        implicitDefines:CodeContext * 
+        lexResourceManager: Lexhelp.LexResourceManager *
+        dependencyProvider: DependencyProvider
+            -> LoadClosure

src/fsharp/CompileOps.fsi

@@ -183,11 +183,6 @@ module internal Utilities =
 
             p.WaitForExit()
 
-            if p.ExitCode <> 0 then
-                //Write StandardError.txt to err stream
-                for line in stdOut do Console.Out.WriteLine(line)
-                for line in stdErr do Console.Error.WriteLine(line)
-
             p.ExitCode = 0, stdOut, stdErr
 
         | None -> false, Array.empty, Array.empty
@@ -203,7 +198,7 @@ module internal Utilities =
             | None -> ""
 
         let arguments prefix =
-            sprintf "%s -restore %s %c%s%c /t:InteractivePackageManagement" prefix binLoggingArguments '\"' projectPath '\"'
+            sprintf "%s -restore %s %c%s%c /nologo /t:InteractivePackageManagement" prefix binLoggingArguments '\"' projectPath '\"'
 
         let workingDir = Path.GetDirectoryName projectPath
 

src/fsharp/FSharp.DependencyManager.Nuget/FSharp.DependencyManager.Utilities.fs

@@ -68,19 +68,14 @@ module ReflectionHelper =
             e.InnerException
         | _ -> e
 
-open ReflectionHelper
-open RidHelpers
-
 /// Indicate the type of error to report
 [<RequireQualifiedAccess>]
 type ErrorReportType =
     | Warning
     | Error
 
-
 type ResolvingErrorReport = delegate of ErrorReportType * int * string -> unit
 
-
 (* Shape of Dependency Manager contract, resolved using reflection *)
 /// The results of ResolveDependencies
 type IResolveDependenciesResult =
@@ -268,9 +263,17 @@ type ReflectionDependencyManagerProvider(theType: Type,
 /// Class is IDisposable
 type DependencyProvider (assemblyProbingPaths: AssemblyResolutionProbe, nativeProbingRoots: NativeResolutionProbe) =
 
-    let dllResolveHandler = new NativeDllResolveHandler(nativeProbingRoots) :> IDisposable
+    // Note: creating a NativeDllResolveHandler currently installs process-wide handlers
+    let dllResolveHandler =
+        match nativeProbingRoots with 
+        | null -> { new IDisposable with member _.Dispose() = () }
+        | _ -> new NativeDllResolveHandler(nativeProbingRoots) :> IDisposable
 
-    let assemblyResolveHandler = new AssemblyResolveHandler(assemblyProbingPaths) :> IDisposable
+    // Note: creating a AssemblyResolveHandler currently installs process-wide handlers
+    let assemblyResolveHandler = 
+        match assemblyProbingPaths with 
+        | null -> { new IDisposable with member _.Dispose() = () }
+        | _ -> new AssemblyResolveHandler(assemblyProbingPaths) :> IDisposable
 
     // Resolution Path = Location of FSharp.Compiler.Private.dll
     let assemblySearchPaths = lazy (
@@ -327,11 +330,11 @@ type DependencyProvider (assemblyProbingPaths: AssemblyResolutionProbe, nativePr
                     None
             managers
 
-    let cache = ConcurrentDictionary<_,IResolveDependenciesResult>(HashIdentity.Structural)
+    let cache = ConcurrentDictionary<_,Result<IResolveDependenciesResult, _>>(HashIdentity.Structural)
 
-    /// Returns a formatted error message for the host to presentconstruct with just nativeProbing handler
-    new (nativeProbingRoots: NativeResolutionProbe) =
-        new DependencyProvider(Unchecked.defaultof<AssemblyResolutionProbe>, nativeProbingRoots)
+    new (nativeProbingRoots: NativeResolutionProbe) = new DependencyProvider(null, nativeProbingRoots)
+
+    new () = new DependencyProvider(null, null)
 
     /// Returns a formatted help messages for registered dependencymanagers for the host to present
     member _.GetRegisteredDependencyManagerHelpText (compilerTools, outputDir, errorReport) = [|
@@ -396,21 +399,25 @@ type DependencyProvider (assemblyProbingPaths: AssemblyResolutionProbe, nativePr
 
         let key = (packageManager.Key, scriptExt, Seq.toArray packageManagerTextLines, executionTfm, executionRid, implicitIncludeDir, mainScriptName, fileName)
 
-        cache.GetOrAdd(key, System.Func<_,_>(fun _ -> 
-            try
-                let executionRid =
-                    if isNull executionRid then
-                        RidHelpers.platformRid
-                    else
-                        executionRid
-                packageManager.ResolveDependencies(implicitIncludeDir, mainScriptName, fileName, scriptExt, packageManagerTextLines, executionTfm, executionRid)
-
-            with e ->
-                let e = stripTieWrapper e
-                let err, msg = (DependencyManager.SR.packageManagerError(e.Message))
-                reportError.Invoke(ErrorReportType.Error, err, msg)
-                ReflectionDependencyManagerProvider.MakeResultFromFields(false, arrEmpty, arrEmpty, seqEmpty, seqEmpty, seqEmpty)
-        ))
+        let result = 
+            cache.GetOrAdd(key, System.Func<_,_>(fun _ -> 
+                try
+                    let executionRid =
+                        if isNull executionRid then
+                            RidHelpers.platformRid
+                        else
+                            executionRid
+                    Ok (packageManager.ResolveDependencies(implicitIncludeDir, mainScriptName, fileName, scriptExt, packageManagerTextLines, executionTfm, executionRid))
+
+                with e ->
+                    let e = stripTieWrapper e
+                    Error (DependencyManager.SR.packageManagerError(e.Message))
+            ))
+        match result with 
+        | Ok res -> res
+        | Error (errorNumber, errorData) ->
+            reportError.Invoke(ErrorReportType.Error, errorNumber, errorData)
+            ReflectionDependencyManagerProvider.MakeResultFromFields(false, arrEmpty, arrEmpty, seqEmpty, seqEmpty, seqEmpty)
 
     interface IDisposable with
 

src/fsharp/Microsoft.DotNet.DependencyManager/DependencyProvider.fs

@@ -56,16 +56,23 @@ type ErrorReportType =
 type ResolvingErrorReport = delegate of ErrorReportType * int * string -> unit
 
 /// Provides DependencyManagement functions.
-/// Class is IDisposable
+///
+/// The class incrementally collects IDependencyManagerProvider, indexed by key, and 
+/// queries them.  These are found and instantiated with respect to the compilerTools and outputDir
+/// provided each time the TryFindDependencyManagerByKey and TryFindDependencyManagerInPath are
+/// executed, which are assumed to be invariant over the lifetime of the DependencyProvider.
 type DependencyProvider =
     interface System.IDisposable
 
-    /// Construct a new DependencyProvider
-    new: assemblyProbingPaths: AssemblyResolutionProbe * nativeProbingRoots: NativeResolutionProbe -> DependencyProvider
+    /// Construct a new DependencyProvider with no dynamic load handlers (only for compilation/analysis)
+    new: unit -> DependencyProvider
 
-    /// Construct a new DependencyProvider
+    /// Construct a new DependencyProvider with only native resolution
     new: nativeProbingRoots: NativeResolutionProbe -> DependencyProvider
 
+    /// Construct a new DependencyProvider with managed and native resolution
+    new: assemblyProbingPaths: AssemblyResolutionProbe * nativeProbingRoots: NativeResolutionProbe -> DependencyProvider
+
     /// Returns a formatted help messages for registered dependencymanagers for the host to present
     member GetRegisteredDependencyManagerHelpText: string seq * string * ResolvingErrorReport -> string[]
 

src/fsharp/Microsoft.DotNet.DependencyManager/DependencyProvider.fsi

@@ -101,8 +101,9 @@ type NativeDllResolveHandlerCoreClr (_nativeProbingRoots: NativeResolutionProbe)
         let probe =
             match _nativeProbingRoots with
             | null -> None
-            | _ ->  _nativeProbingRoots.Invoke()
-                    |> Seq.tryPick(fun root ->
+            | _ ->  
+                _nativeProbingRoots.Invoke()
+                |> Seq.tryPick(fun root ->
                     probingFileNames name |> Seq.tryPick(fun name ->
                         let path = Path.Combine(root, name)
                         if File.Exists(path) then

src/fsharp/Microsoft.DotNet.DependencyManager/NativeDllResolveHandler.fs

@@ -18,7 +18,7 @@ type NativeResolutionProbe = delegate of Unit -> seq<string>
 // Cut down AssemblyLoadContext, for loading native libraries
 type NativeDllResolveHandler =
 
-    /// Construct a new DependencyProvider
+    /// Construct a new NativeDllResolveHandler
     new: nativeProbingRoots: NativeResolutionProbe -> NativeDllResolveHandler
 
     interface IDisposable