From e782148fef66e041d3054b5ad9c2330d2674f41b Mon Sep 17 00:00:00 2001 From: Adam Boniecki <20281641+abonie@users.noreply.github.com> Date: Fri, 7 Oct 2022 18:33:05 +0200 Subject: [PATCH] Eagerly format exceptions in FSI's interactive scripting API (#14026) * Add eager formatting of exceptions to fsi Eval* methods of FSIEvaluationSession that are using CompilationDiagnosticsLogger are at risk of mutating data that's referred to in the exception before error message is printed. To prevent that, this commit makes CompilationDiagnosticsLogger take additional argument that can control early pre-processing of logged exceptions. * Add a test for ValueRestriction error * Minor fixes from code review Co-authored-by: Adam Boniecki --- src/Compiler/Interactive/fsi.fs | 8 +++++--- src/Compiler/Symbols/FSharpDiagnostic.fs | 9 +++++++-- src/Compiler/Symbols/FSharpDiagnostic.fsi | 4 +++- .../FSharpScriptTests.fs | 9 +++++++++ 4 files changed, 24 insertions(+), 6 deletions(-) diff --git a/src/Compiler/Interactive/fsi.fs b/src/Compiler/Interactive/fsi.fs index 6552c6a62da..f4ee6685a11 100644 --- a/src/Compiler/Interactive/fsi.fs +++ b/src/Compiler/Interactive/fsi.fs @@ -3521,6 +3521,8 @@ type FsiEvaluationSession (fsi: FsiEvaluationSessionHostConfig, argv:string[], i let dummyScriptFileName = "input.fsx" + let eagerFormat (diag : PhasedDiagnostic) = diag.EagerlyFormatCore true + interface IDisposable with member _.Dispose() = (tcImports :> IDisposable).Dispose() @@ -3639,7 +3641,7 @@ type FsiEvaluationSession (fsi: FsiEvaluationSessionHostConfig, argv:string[], i let ctok = AssumeCompilationThreadWithoutEvidence() let errorOptions = TcConfig.Create(tcConfigB,validate = false).diagnosticsOptions - let diagnosticsLogger = CompilationDiagnosticLogger("EvalInteraction", errorOptions) + let diagnosticsLogger = CompilationDiagnosticLogger("EvalInteraction", errorOptions, eagerFormat) fsiInteractionProcessor.EvalExpression(ctok, code, dummyScriptFileName, diagnosticsLogger) |> commitResultNonThrowing errorOptions dummyScriptFileName diagnosticsLogger @@ -3661,7 +3663,7 @@ type FsiEvaluationSession (fsi: FsiEvaluationSessionHostConfig, argv:string[], i let cancellationToken = defaultArg cancellationToken CancellationToken.None let errorOptions = TcConfig.Create(tcConfigB,validate = false).diagnosticsOptions - let diagnosticsLogger = CompilationDiagnosticLogger("EvalInteraction", errorOptions) + let diagnosticsLogger = CompilationDiagnosticLogger("EvalInteraction", errorOptions, eagerFormat) fsiInteractionProcessor.EvalInteraction(ctok, code, dummyScriptFileName, diagnosticsLogger, cancellationToken) |> commitResultNonThrowing errorOptions "input.fsx" diagnosticsLogger @@ -3682,7 +3684,7 @@ type FsiEvaluationSession (fsi: FsiEvaluationSessionHostConfig, argv:string[], i let ctok = AssumeCompilationThreadWithoutEvidence() let errorOptions = TcConfig.Create(tcConfigB, validate = false).diagnosticsOptions - let diagnosticsLogger = CompilationDiagnosticLogger("EvalInteraction", errorOptions) + let diagnosticsLogger = CompilationDiagnosticLogger("EvalInteraction", errorOptions, eagerFormat) fsiInteractionProcessor.EvalScript(ctok, filePath, diagnosticsLogger) |> commitResultNonThrowing errorOptions filePath diagnosticsLogger |> function Choice1Of2 _, errs -> Choice1Of2 (), errs | Choice2Of2 exn, errs -> Choice2Of2 exn, errs diff --git a/src/Compiler/Symbols/FSharpDiagnostic.fs b/src/Compiler/Symbols/FSharpDiagnostic.fs index a0c9a47fa88..c4b57e84c9f 100644 --- a/src/Compiler/Symbols/FSharpDiagnostic.fs +++ b/src/Compiler/Symbols/FSharpDiagnostic.fs @@ -158,13 +158,18 @@ type DiagnosticsScope() = | None -> err "" /// A diagnostics logger that capture diagnostics, filtering them according to warning levels etc. -type internal CompilationDiagnosticLogger (debugName: string, options: FSharpDiagnosticOptions) = +type internal CompilationDiagnosticLogger (debugName: string, options: FSharpDiagnosticOptions, ?preprocess: (PhasedDiagnostic -> PhasedDiagnostic)) = inherit DiagnosticsLogger("CompilationDiagnosticLogger("+debugName+")") let mutable errorCount = 0 let diagnostics = ResizeArray<_>() override _.DiagnosticSink(diagnostic, severity) = + let diagnostic = + match preprocess with + | Some f -> f diagnostic + | None -> diagnostic + if diagnostic.ReportAsError (options, severity) then diagnostics.Add(diagnostic, FSharpDiagnosticSeverity.Error) errorCount <- errorCount + 1 @@ -172,7 +177,7 @@ type internal CompilationDiagnosticLogger (debugName: string, options: FSharpDia diagnostics.Add(diagnostic, FSharpDiagnosticSeverity.Warning) elif diagnostic.ReportAsInfo (options, severity) then diagnostics.Add(diagnostic, severity) - + override _.ErrorCount = errorCount member _.GetDiagnostics() = diagnostics.ToArray() diff --git a/src/Compiler/Symbols/FSharpDiagnostic.fsi b/src/Compiler/Symbols/FSharpDiagnostic.fsi index 2ebfcb71980..313c14240e4 100644 --- a/src/Compiler/Symbols/FSharpDiagnostic.fsi +++ b/src/Compiler/Symbols/FSharpDiagnostic.fsi @@ -106,7 +106,9 @@ type internal CompilationDiagnosticLogger = inherit DiagnosticsLogger /// Create the diagnostics logger - new: debugName: string * options: FSharpDiagnosticOptions -> CompilationDiagnosticLogger + new: + debugName: string * options: FSharpDiagnosticOptions * ?preprocess: (PhasedDiagnostic -> PhasedDiagnostic) -> + CompilationDiagnosticLogger /// Get the captured diagnostics member GetDiagnostics: unit -> (PhasedDiagnostic * FSharpDiagnosticSeverity)[] diff --git a/tests/FSharp.Compiler.Private.Scripting.UnitTests/FSharpScriptTests.fs b/tests/FSharp.Compiler.Private.Scripting.UnitTests/FSharpScriptTests.fs index c218cc2f408..2c4a8dd198f 100644 --- a/tests/FSharp.Compiler.Private.Scripting.UnitTests/FSharpScriptTests.fs +++ b/tests/FSharp.Compiler.Private.Scripting.UnitTests/FSharpScriptTests.fs @@ -16,6 +16,15 @@ open Xunit type InteractiveTests() = + [] + member _.``ValueRestriction error message should not have type variables fully solved``() = + use script = new FSharpScript() + let code = "id id" + let _, errors = script.Eval(code) + Assert.Equal(1, errors.Length) + let msg = errors[0].Message + Assert.Matches("'_\\w+ -> '_\\w+", msg) + [] member _.``Eval object value``() = use script = new FSharpScript()