Don't type check skipped impl files of referenced project

docs/release-notes/.FSharp.Compiler.Service/11.0.200.md

@@ -19,6 +19,7 @@
 * Preserve tupled lambda argument groups when reducing state-machine applications, avoiding invalid argument substitutions and runtime failures with inline formatters. ([Issue #20675](https://github.com/dotnet/fsharp/issues/20675), [PR #20700](https://github.com/dotnet/fsharp/pull/20700))
 * Transparent compiler: `ParseFile` no longer imports the references of the project, which also type checked the projects it references. Parsing only needs the compiler options of the project. ([PR #20704](https://github.com/dotnet/fsharp/pull/20704))
 * Preserve SRTP extension scopes across source files and competing witnesses through imported inline calls. Compile generic extension forwarding, including quotations, in unoptimized builds. Preserve applicable built-in operator priority in imported inline quotations. Do not reuse SRTP specializations across different selected witnesses. ([Issue #20683](https://github.com/dotnet/fsharp/issues/20683), [Issue #20684](https://github.com/dotnet/fsharp/issues/20684), [Issue #20685](https://github.com/dotnet/fsharp/issues/20685), [PR #20717](https://github.com/dotnet/fsharp/pull/20717))
+* With `enablePartialTypeChecking`, a project that is only referenced by another project no longer type checks its implementation files that have a signature file. Producing the referenced assembly only needs the signatures, but finalizing the project also collected the diagnostics of the skipped files, which type checked all of them and kept their results in memory. `ParseAndCheckProject` still reports the diagnostics of these files. ([PR #20699](https://github.com/dotnet/fsharp/pull/20699))
 
 ### Changed
 

src/Compiler/Service/IncrementalBuild.fs

@@ -400,6 +400,13 @@ type BoundModel private (
 
     member val Diagnostics = diagnostics
 
+    /// Diagnostics for this file, without forcing the type check of an implementation file that was skipped
+    /// because it has a signature file. Such a file has no diagnostics yet, so it reports none.
+    member _.GetDiagnosticsIfTypeChecked() =
+        match tcStateOpt, skippedImplementationTypeCheck with
+        | None, Some _ when not typeCheckNode.HasValue -> async { return [||] }
+        | _ -> diagnostics.GetOrComputeValue()
+
     member val TcInfo = tcInfo
 
     member val TcInfoExtras = tcInfoExtras
@@ -881,10 +888,12 @@ module IncrementalBuilderHelpers =
 
         let finalBoundModel = Array.last computedBoundModels
 
-        // Collect diagnostics. This will type check in parallel any implementation files skipped so far.
+        // Collect the diagnostics that are available without type checking the implementation files that were skipped
+        // because they have a signature file. The assembly data only needs the signatures, so a project that is only
+        // referenced by another project does not pay for checking these files. FullyCheckFinalizedBoundModelTask checks them.
         let! partialDiagnostics =
             computedBoundModels
-            |> Seq.map (fun m -> m.Diagnostics.GetOrComputeValue())
+            |> Seq.map (fun m -> m.GetDiagnosticsIfTypeChecked())
             |> MultipleDiagnosticsLoggers.Parallel
         let diagnostics = [
             diagnosticsLogger.GetDiagnostics()
@@ -895,6 +904,30 @@ module IncrementalBuilderHelpers =
         return ilAssemRef, tcAssemblyDataOpt, tcAssemblyExprOpt, finalBoundModelWithErrors
     }
 
+    /// Type check in parallel any implementation files skipped so far, and collect the diagnostics of all files.
+    let FullyCheckFinalizedBoundModelTask (finalizedBoundModel: BoundModel) (boundModels: GraphNode<BoundModel> seq) =
+      async {
+        let diagnosticsLogger = CompilationDiagnosticLogger("FullyCheckFinalizedBoundModelTask", finalizedBoundModel.TcConfig.diagnosticsOptions)
+        use _ = new CompilationGlobalsScope(diagnosticsLogger, BuildPhase.TypeCheck)
+
+        let! computedBoundModels = boundModels |> Seq.map (fun g -> g.GetOrComputeValue()) |> MultipleDiagnosticsLoggers.Sequential
+        let! tcInfo = finalizedBoundModel.GetOrComputeTcInfo()
+
+        let! fileDiagnostics =
+            computedBoundModels
+            |> Seq.map (fun m -> m.Diagnostics.GetOrComputeValue())
+            |> MultipleDiagnosticsLoggers.Parallel
+
+        // The first entry holds the diagnostics of finalizing the type check, the rest are per file.
+        let finalizeDiagnostics = List.head tcInfo.tcDiagnosticsRev
+        let diagnostics = [
+            Array.append finalizeDiagnostics (diagnosticsLogger.GetDiagnostics())
+            yield! fileDiagnostics |> Seq.rev
+        ]
+
+        return! finalizedBoundModel.Finish(diagnostics, tcInfo.topAttribs)
+      }
+
 [<NoComparison;NoEquality>]
 type IncrementalBuilderInitialState =
     {
@@ -993,6 +1026,8 @@ type IncrementalBuilderState =
         slots: Slot list
         stampedReferencedAssemblies: ImmutableArray<DateTime>
         finalizedBoundModel: GraphNode<(ILAssemblyRef * ProjectAssemblyDataResult * CheckedImplFile list option * BoundModel) * DateTime>
+        /// Like finalizedBoundModel, but with the implementation files that were skipped type checked, to report their diagnostics.
+        fullyCheckedFinalizedBoundModel: GraphNode<(ILAssemblyRef * ProjectAssemblyDataResult * CheckedImplFile list option * BoundModel) * DateTime>
     }
     member this.stampedFileNames = this.slots |> List.map (fun s -> s.Stamp)
     member this.logicalStampedFileNames = this.slots |> List.map (fun s -> s.LogicalStamp)
@@ -1024,6 +1059,15 @@ module IncrementalBuilderStateHelpers =
             return result, DateTime.UtcNow
         })
 
+    let createFullyCheckedFinalizeBoundModelGraphNode
+        (finalizedBoundModel: GraphNode<(ILAssemblyRef * ProjectAssemblyDataResult * CheckedImplFile list option * BoundModel) * DateTime>)
+        (boundModels: GraphNode<BoundModel> seq) =
+        GraphNode(async {
+            let! (ilAssemRef, tcAssemblyDataOpt, tcAssemblyExprOpt, boundModel), timestamp = finalizedBoundModel.GetOrComputeValue()
+            let! fullyCheckedBoundModel = FullyCheckFinalizedBoundModelTask boundModel boundModels
+            return (ilAssemRef, tcAssemblyDataOpt, tcAssemblyExprOpt, fullyCheckedBoundModel), timestamp
+        })
+
     let computeStampedFileNames (initialState: IncrementalBuilderInitialState) (state: IncrementalBuilderState) (cache: TimeStampCache) =
         let slots =
             if initialState.useChangeNotifications then
@@ -1057,9 +1101,11 @@ module IncrementalBuilderStateHelpers =
         if slots |> List.exists (fun s -> s.Notified) then
             let slots, _ = slots |> List.mapFold mapping (Good, GraphNode.FromResult initialState.initialBoundModel)
             let boundModels = slots |> Seq.map (fun s -> s.BoundModel)
+            let finalizedBoundModel = createFinalizeBoundModelGraphNode initialState boundModels
             { state with
                 slots = slots
-                finalizedBoundModel = createFinalizeBoundModelGraphNode initialState boundModels }
+                finalizedBoundModel = finalizedBoundModel
+                fullyCheckedFinalizedBoundModel = createFullyCheckedFinalizeBoundModelGraphNode finalizedBoundModel boundModels }
         else
             state
 
@@ -1136,11 +1182,13 @@ type IncrementalBuilderState with
                     }
             ]
 
+        let finalizedBoundModel = createFinalizeBoundModelGraphNode initialState boundModels
         let state =
             {
                 slots = slots
                 stampedReferencedAssemblies = ImmutableArray.init referencedAssemblies.Length (fun _ -> DateTime.MinValue)
-                finalizedBoundModel = createFinalizeBoundModelGraphNode initialState boundModels
+                finalizedBoundModel = finalizedBoundModel
+                fullyCheckedFinalizedBoundModel = createFullyCheckedFinalizeBoundModelGraphNode finalizedBoundModel boundModels
             }
         let state = computeStampedReferencedAssemblies initialState state false cache
         let state = computeStampedFileNames initialState state cache
@@ -1339,21 +1387,24 @@ type IncrementalBuilder(initialState: IncrementalBuilderInitialState, state: Inc
     member builder.GetCheckResultsAfterLastFileInProject () =
         builder.GetCheckResultsBeforeSlotInProject(builder.GetSlotsCount())
 
-    member builder.GetCheckResultsAndImplementationsForProject() =
+    member private builder.GetFinalizedCheckResultsAndImplementationsForProject(getFinalizedBoundModel: IncrementalBuilderState -> GraphNode<_>) =
       async {
         let cache = TimeStampCache(defaultTimeStamp)
         do! checkFileTimeStamps cache
-        let! result = currentState.finalizedBoundModel.GetOrComputeValue()
+        let! result = (getFinalizedBoundModel currentState).GetOrComputeValue()
         match result with
         | (ilAssemRef, tcAssemblyDataOpt, tcAssemblyExprOpt, boundModel), timestamp ->
             let cache = TimeStampCache defaultTimeStamp
             let projectTimeStamp = builder.GetLogicalTimeStampForProject(cache)
             return PartialCheckResults (boundModel, timestamp, projectTimeStamp), ilAssemRef, tcAssemblyDataOpt, tcAssemblyExprOpt
       }
 
+    member builder.GetCheckResultsAndImplementationsForProject() =
+        builder.GetFinalizedCheckResultsAndImplementationsForProject(fun state -> state.finalizedBoundModel)
+
     member builder.GetFullCheckResultsAndImplementationsForProject() =
         async {
-            let! result = builder.GetCheckResultsAndImplementationsForProject()
+            let! result = builder.GetFinalizedCheckResultsAndImplementationsForProject(fun state -> state.fullyCheckedFinalizedBoundModel)
             let results, _, _, _ = result
             let! _ = results.GetOrComputeTcInfoWithExtras() // Make sure we forcefully evaluate the info
             return result

tests/FSharp.Compiler.ComponentTests/FSharpChecker/CommonWorkflows.fs

@@ -101,6 +101,34 @@ let ``Changes in a referenced project`` () =
         checkFile "Last" expectSignatureChanged
     }
 
+[<Theory>]
+[<InlineData false>]
+let ``Referenced project does not type check implementation files that have a signature file`` useTransparentCompiler =
+    let library = SyntheticProject.Create("library", sourceFile "Library" [] |> addSignatureFile)
+
+    let project =
+        { makeTestProject() with DependsOn = [library] }
+        |> updateFile "First" (addDependency "Library")
+
+    // Generate the signature file with another checker, so the checker under test only type checks for the workflow.
+    let project = project |> absorbAutoGeneratedSignatures (FSharpChecker.Create()) |> Async.RunSynchronously
+
+    // A private checker because we subscribe to FileChecked.
+    let checker = FSharpChecker.Create(enablePartialTypeChecking = true, useTransparentCompiler = useTransparentCompiler)
+    let checkedFiles = Collections.Concurrent.ConcurrentQueue<string>()
+    checker.FileChecked.Add(fun (fileName, _) -> checkedFiles.Enqueue fileName)
+
+    ProjectWorkflowBuilder(project, checker = checker, useTransparentCompiler = useTransparentCompiler) {
+        checkFile "Last" expectOk
+    }
+    |> ignore
+
+    // Checking a file that depends on the library only needs the signature of the library.
+    let library = project.DependsOn.Head
+    let libraryFile = library.Find "Library"
+    Assert.Contains(getSignatureFilePath library libraryFile, checkedFiles)
+    Assert.DoesNotContain(getFilePath library libraryFile, checkedFiles)
+
 [<Fact(Skip="TODO")>]
 // TODO: This will probably require some special care in TransparentCompiler...
 let ``Language service works if the same file is listed twice`` () =