Report builder creation warnings according to compiler args (#4125)

* Filter incremental builder creation errors according to compiler args

* Fix CompilationErrorLogger ignores WarsAsError options

* Add test for WarnAsError

* Cleanup

* Add more tests; cover WarnAsError-, no warnings at all

* Refactor tests

* Add test

src/fsharp/service/IncrementalBuild.fs

@@ -1703,8 +1703,11 @@ type IncrementalBuilder(tcGlobals, frameworkTcImports, nonFrameworkAssemblyInput
       cancellable {
 
         // Trap and report warnings and errors from creation.
-        use errorScope = new ErrorScope()
-        let! builderOpt = 
+        let delayedLogger = CapturingErrorLogger("IncrementalBuilderCreation")
+        use _unwindEL = PushErrorLoggerPhaseUntilUnwind (fun _ -> delayedLogger)
+        use _unwindBP = PushThreadBuildPhaseUntilUnwind BuildPhase.Parameter
+
+        let! builderOpt =
          cancellable {
           try
 
@@ -1814,7 +1817,18 @@ type IncrementalBuilder(tcGlobals, frameworkTcImports, nonFrameworkAssemblyInput
             return None
          }
 
-        return builderOpt, errorScope.Diagnostics
+        let diagnostics =
+            match builderOpt with
+            | Some builder ->
+                let errorSeverityOptions = builder.TcConfig.errorSeverityOptions
+                let errorLogger = CompilationErrorLogger("IncrementalBuilderCreation", errorSeverityOptions)
+                delayedLogger.CommitDelayedDiagnostics(errorLogger)
+                errorLogger.GetErrors() |> List.map (fun (d, severity) -> d, severity = FSharpErrorSeverity.Error)
+            | _ ->
+                delayedLogger.Diagnostics
+            |> List.map (fun (d, isError) -> FSharpErrorInfo.CreateFromException(d, isError, range.Zero))
+
+        return builderOpt, diagnostics
       }
     static member KeepBuilderAlive (builderOpt: IncrementalBuilder option) = 
         match builderOpt with 

src/fsharp/symbols/SymbolHelpers.fs

@@ -150,15 +150,14 @@ type internal CompilationErrorLogger (debugName: string, options: FSharpErrorSev
 
     override x.DiagnosticSink(exn, isError) = 
         if isError || ReportWarningAsError options exn then
-            diagnostics.Add(exn, isError)
+            diagnostics.Add(exn, FSharpErrorSeverity.Error)
             errorCount <- errorCount + 1
         else if ReportWarning options exn then
-            diagnostics.Add(exn, isError)
+            diagnostics.Add(exn, FSharpErrorSeverity.Warning)
 
     override x.ErrorCount = errorCount
 
-    member x.GetErrors() = 
-        [ for (e, isError) in diagnostics -> e, (if isError then FSharpErrorSeverity.Error else FSharpErrorSeverity.Warning) ]
+    member x.GetErrors() = List.ofSeq diagnostics
 
 
 /// This represents the global state established as each task function runs as part of the build.

tests/service/Common.fs

@@ -160,6 +160,23 @@ let mkProjectCommandLineArgsForScript (dllName, fileNames) =
      |]
 #endif
 
+let mkTestFileAndOptions source additionalArgs =
+    let fileName = Path.ChangeExtension(Path.GetTempFileName(), ".fs")
+    let project = Path.GetTempFileName()
+    let dllName = Path.ChangeExtension(project, ".dll")
+    let projFileName = Path.ChangeExtension(project, ".fsproj")
+    let fileSource1 = "module M"
+    File.WriteAllText(fileName, fileSource1)
+
+    let args = Array.append (mkProjectCommandLineArgs (dllName, [fileName])) additionalArgs
+    let options = checker.GetProjectOptionsFromCommandLineArgs (projFileName, args)
+    fileName, options
+
+let parseAndCheckFile fileName source options =
+    match checker.ParseAndCheckFileInProject(fileName, 0, source, options) |> Async.RunSynchronously with
+    | parseResults, FSharpCheckFileAnswer.Succeeded(checkResults) -> parseResults, checkResults
+    | _ -> failwithf "Parsing aborted unexpectedly..."
+
 let parseAndCheckScript (file, input) = 
 
 #if DOTNETCORE

tests/service/ProjectAnalysisTests.fs

@@ -5195,3 +5195,16 @@ type A(i:int) =
 
     | Some decl -> failwithf "unexpected declaration %A" decl
     | None -> failwith "declaration list is empty"
+
+
+[<TestCase(([||]: string[]), ([||]: bool[]))>]
+[<TestCase([| "--times" |], [| false |])>]
+[<TestCase([| "--times"; "--nowarn:75" |], ([||]: bool[]))>]
+[<TestCase([| "--times"; "--warnaserror:75" |], [| true |])>]
+[<TestCase([| "--times"; "--warnaserror-:75"; "--warnaserror" |], [| false |])>]
+let ``#4030, Incremental builder creation warnings`` (args, errorSeverities) =
+    let source = "module M"
+    let fileName, options = mkTestFileAndOptions source args
+
+    let _, checkResults = parseAndCheckFile fileName source options
+    checkResults.Errors |> Array.map (fun e -> e.Severity = FSharpErrorSeverity.Error) |> shouldEqual errorSeverities