Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
20 changes: 17 additions & 3 deletions src/fsharp/service/IncrementalBuild.fs
Original file line number Diff line number Diff line change
Expand Up @@ -1704,8 +1704,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

Expand Down Expand Up @@ -1825,7 +1828,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
Expand Down
7 changes: 3 additions & 4 deletions src/fsharp/symbols/SymbolHelpers.fs
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
17 changes: 17 additions & 0 deletions tests/service/Common.fs
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
13 changes: 13 additions & 0 deletions tests/service/ProjectAnalysisTests.fs
Original file line number Diff line number Diff line change
Expand Up @@ -5195,3 +5195,16 @@ type A(i:int) =

| Some decl -> failwithf "unexpected declaration %A" decl
| None -> failwith "declaration list is empty"


[<TestCase(([||]: string[]), ([||]: bool[]))>]
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This isn't urgent, but just to say that I personally never use TestCase attributes in the F# tests, and prefer a loop over an F# list of test case data inputs. Indeed I usually remove any uses of TestCase when I come across them later

While using TestCase attribute has some advantages (e.g. they get reported as separate tests in a test explorer), I believe that just using F# code to program the space of test cases is more uniform and in the long run easier to understand - and encourages people to be really expansive in how they meta-program up a very broad range of systematic test cases.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(Programming up the test case matrix with F# data is also more typesafe of course)

[<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