From 5b2185876a0f92b7f08131bc177e2b1a2d747b6b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Michael=20R=C3=A4tzel?= Date: Sun, 5 May 2024 16:41:14 +0000 Subject: [PATCH] Improve runtime errors on invalid type-mismatched Elm record access Improve errors in scenarios where Elm syntax was compiled with incomplete type-checking, resulting in an invalid record access using a nonexistent field name. In the previous version, such a program caused a stack overflow. The changes in this commit improve the errors as follows: + Expand the framework for testing using Elm Interactive scenarios to support expectations for errors: Add a syntax to encode the expected contents of an error in a test scenario step and adapt the processing of step results accordingly. + Change the PineVM implementation to include more details when crashing due to failed parsing of a value as expression: Include details on both function value and environment. + Change the Elm compiler to crash by invalid parsing instead of the infinite recursion used earlier. Use the function and environment properties of the parse-and-eval expression to specify the error, including the name of the searched record field. --- implement/Pine.Core/Pine.Core.csproj | 6 +- .../pine/ElmInteractive/TestElmInteractive.cs | 105 +++++++++++++----- .../compile-elm-program/src/ElmCompiler.elm | 12 +- implement/pine/Pine/PineVM/PineVM.cs | 18 ++- implement/pine/Program.cs | 2 +- implement/pine/pine.csproj | 4 +- .../steps/131/expected-error-contains.txt | 1 + .../errors/steps/131/submission.txt | 4 + 8 files changed, 107 insertions(+), 45 deletions(-) create mode 100644 implement/test-and-train/elm-interactive-scenarios-core/errors/steps/131/expected-error-contains.txt create mode 100644 implement/test-and-train/elm-interactive-scenarios-core/errors/steps/131/submission.txt diff --git a/implement/Pine.Core/Pine.Core.csproj b/implement/Pine.Core/Pine.Core.csproj index 4eb28ed3..dc9867b7 100644 --- a/implement/Pine.Core/Pine.Core.csproj +++ b/implement/Pine.Core/Pine.Core.csproj @@ -3,13 +3,13 @@ net8.0 enable - 0.3.5 - 0.3.5 + 0.3.6 + 0.3.6 Pine.Core - 0.3.5 + 0.3.6 The cross-platform Elm runtime environment Functional;Elm;Runtime;Compiler;VM;DBMS https://github.com/pine-vm/pine.git diff --git a/implement/pine/ElmInteractive/TestElmInteractive.cs b/implement/pine/ElmInteractive/TestElmInteractive.cs index cc42ce3f..056ef7b6 100644 --- a/implement/pine/ElmInteractive/TestElmInteractive.cs +++ b/implement/pine/ElmInteractive/TestElmInteractive.cs @@ -52,11 +52,12 @@ public record Scenario( public record ScenarioStep( string Submission, - string? ExpectedResponse); + string? ExpectedResponse, + string? ExpectedErrorContains); public record InteractiveScenarioTestReport( Scenario Scenario, - ImmutableList<(string name, Result result)> StepsReports, + ImmutableList<(string name, Result result)> StepsReports, TimeSpan ElapsedTime) { public bool Passed => StepsReports.All(s => s.result.IsOk()); @@ -66,6 +67,15 @@ public record InteractiveScenarioTestStepFailure( string submission, string errorAsText); + public abstract record InteractiveScenarioTestStepSuccess + { + public record ErrorAsExpected(string ErrorAsText) + : InteractiveScenarioTestStepSuccess; + + public record SubmissionResponseOk(IInteractiveSession.SubmissionResponse SubmissionResponse) + : InteractiveScenarioTestStepSuccess; + } + public static ParsedScenarios ParseElmInteractiveScenarios( TreeNodeWithStringPath scenariosTree, IConsole console) @@ -224,7 +234,38 @@ public static InteractiveScenarioTestReport TestElmInteractiveScenario( parsedScenario.Steps .Select(sessionStep => { - Result getResult() + Result continueWithErrorMessage( + string errorMessage) + { + if (sessionStep.step.ExpectedErrorContains is { } expectedErrorContains) + { + if (!errorMessage.Contains(expectedErrorContains, StringComparison.InvariantCultureIgnoreCase)) + { + var errorText = + "Error from interactive does not contain expected value. Expected:\n" + + expectedErrorContains + + "\nBut got this error:\n" + + errorMessage; + + return + (Result) + new InteractiveScenarioTestStepFailure( + submission: sessionStep.step.Submission, + errorAsText: errorText); + } + + return + (Result) + new InteractiveScenarioTestStepSuccess.ErrorAsExpected(errorMessage); + } + + return + new InteractiveScenarioTestStepFailure( + submission: sessionStep.step.Submission, + errorAsText: errorMessage); + } + + Result getResult() { try { @@ -241,42 +282,36 @@ public static InteractiveScenarioTestReport TestElmInteractiveScenario( new TestInteractiveScenarioLogEntry.SubmissionResponseStruct( Result: submissionResult))); - var submissionResultMappedErr = - submissionResult - .MapError(err => new InteractiveScenarioTestStepFailure( - submission: sessionStep.step.Submission, - errorAsText: "Submission result has error: " + err)); - return - submissionResultMappedErr - .AndThen(submissionResultOk => - { - if (sessionStep.step.ExpectedResponse is { } expectedResponse) + submissionResult + .Unpack( + fromErr: err => continueWithErrorMessage("Submission result has error: " + err), + fromOk: submissionResultOk => { - if (expectedResponse != submissionResultOk.InteractiveResponse?.DisplayText) + if (sessionStep.step.ExpectedResponse is { } expectedResponse) { - var errorText = - "Response from interactive does not match expected value. Expected:\n" + - expectedResponse + - "\nBut got this response:\n" + - submissionResultOk.InteractiveResponse?.DisplayText; - - return Result.err( + if (expectedResponse != submissionResultOk.InteractiveResponse?.DisplayText) + { + var errorText = + "Response from interactive does not match expected value. Expected:\n" + + expectedResponse + + "\nBut got this response:\n" + + submissionResultOk.InteractiveResponse?.DisplayText; + + return + (Result) new InteractiveScenarioTestStepFailure( submission: sessionStep.step.Submission, - errorAsText: errorText)); + errorAsText: errorText); + } } - } - return submissionResultMappedErr; - }); + return new InteractiveScenarioTestStepSuccess.SubmissionResponseOk(submissionResultOk); + }); } catch (Exception e) { - return Result.err( - new InteractiveScenarioTestStepFailure( - submission: sessionStep.step.Submission, - errorAsText: "Runtime exception:\n" + e)); + return continueWithErrorMessage("Runtime exception:\n" + e); } } @@ -344,6 +379,13 @@ public static Result ParseScenarioStep(TreeNodeWithStringP : null; + var expectedErrorContains = + sessionStep.GetNodeAtPath(["expected-error-contains.txt"]) is TreeNodeWithStringPath.BlobNode expectedErrorContainsBlob + ? + Encoding.UTF8.GetString(expectedErrorContainsBlob.Bytes.Span) + : + null; + return (sessionStep.GetNodeAtPath(["submission.txt"]) switch { @@ -353,6 +395,9 @@ public static Result ParseScenarioStep(TreeNodeWithStringP _ => Result.err("Missing submission"), }) - .Map(submission => new ScenarioStep(submission, expectedResponse)); + .Map(submission => new ScenarioStep( + submission, + ExpectedResponse: expectedResponse, + ExpectedErrorContains: expectedErrorContains)); } } diff --git a/implement/pine/ElmTime/compile-elm-program/src/ElmCompiler.elm b/implement/pine/ElmTime/compile-elm-program/src/ElmCompiler.elm index c5e0d590..9a7e4434 100644 --- a/implement/pine/ElmTime/compile-elm-program/src/ElmCompiler.elm +++ b/implement/pine/ElmTime/compile-elm-program/src/ElmCompiler.elm @@ -431,9 +431,9 @@ expandEnvWithModulesRecursive environmentBefore parsedElmModules = nextEnvironment = Dict.insert moduleName moduleValue environmentBefore in - expandEnvWithModulesRecursive - nextEnvironment - followingModules + expandEnvWithModulesRecursive + nextEnvironment + followingModules compileElmModuleIntoNamedExports : @@ -2784,7 +2784,11 @@ recursiveFunctionToLookupFieldInRecord = [ Pine.ListExpression [] , remainingFieldsLocalExpression ] - , ifTrue = continueWithRemainingExpression + , ifTrue = + Pine.ParseAndEvalExpression + { expression = Pine.LiteralExpression (Pine.valueFromString "Invalid record access") + , environment = fieldNameLocalExpression + } , ifFalse = Pine.ConditionalExpression { condition = diff --git a/implement/pine/Pine/PineVM/PineVM.cs b/implement/pine/Pine/PineVM/PineVM.cs index c22b21a1..fdd48dc6 100644 --- a/implement/pine/Pine/PineVM/PineVM.cs +++ b/implement/pine/Pine/PineVM/PineVM.cs @@ -125,21 +125,29 @@ public Result EvaluateExpressionDefault( public Result EvaluateParseAndEvalExpression( Expression.ParseAndEvalExpression parseAndEval, PineValue environment) => + EvaluateExpression(parseAndEval.environment, environment) + .AndThen(environmentValue => EvaluateExpression(parseAndEval.expression, environment) .MapError(error => "Failed to evaluate function: " + error) .AndThen(functionValue => ParseExpressionFromValue(functionValue) - .MapError(error => "Failed to parse expression from function value: " + error) - .AndThen(functionExpression => EvaluateExpression(parseAndEval.environment, environment) + .MapError(error => + "Failed to parse expression from function value: " + error + + " - functionValue is " + + PineValueAsString.StringFromValue(functionValue) + .Unpack(fromErr: _ => "not a string", fromOk: asString => "string \'" + asString + "\'") + + " - environmentValue is " + + PineValueAsString.StringFromValue(environmentValue) + .Unpack(fromErr: _ => "not a string", fromOk: asString => "string \'" + asString + "\'")) .MapError(error => "Failed to evaluate argument: " + error) - .AndThen(argumentValue => + .AndThen(functionExpression => { - if (argumentValue is PineValue.ListValue list) + if (environmentValue is PineValue.ListValue list) { FunctionApplicationMaxEnvSize = FunctionApplicationMaxEnvSize < list.Elements.Count ? list.Elements.Count : FunctionApplicationMaxEnvSize; } - var evalResult = EvaluateExpression(environment: argumentValue, expression: functionExpression); + var evalResult = EvaluateExpression(environment: environmentValue, expression: functionExpression); return evalResult; }))); diff --git a/implement/pine/Program.cs b/implement/pine/Program.cs index 14a4c2ef..4154c39d 100644 --- a/implement/pine/Program.cs +++ b/implement/pine/Program.cs @@ -18,7 +18,7 @@ namespace ElmTime; public class Program { - public static string AppVersionId => "0.3.5"; + public static string AppVersionId => "0.3.6"; private static int AdminInterfaceDefaultPort => 4000; diff --git a/implement/pine/pine.csproj b/implement/pine/pine.csproj index b73356e3..f52932b6 100644 --- a/implement/pine/pine.csproj +++ b/implement/pine/pine.csproj @@ -4,8 +4,8 @@ Exe net8.0 pine - 0.3.5 - 0.3.5 + 0.3.6 + 0.3.6 enable true diff --git a/implement/test-and-train/elm-interactive-scenarios-core/errors/steps/131/expected-error-contains.txt b/implement/test-and-train/elm-interactive-scenarios-core/errors/steps/131/expected-error-contains.txt new file mode 100644 index 00000000..75dec562 --- /dev/null +++ b/implement/test-and-train/elm-interactive-scenarios-core/errors/steps/131/expected-error-contains.txt @@ -0,0 +1 @@ +invalid record access \ No newline at end of file diff --git a/implement/test-and-train/elm-interactive-scenarios-core/errors/steps/131/submission.txt b/implement/test-and-train/elm-interactive-scenarios-core/errors/steps/131/submission.txt new file mode 100644 index 00000000..b1025a0a --- /dev/null +++ b/implement/test-and-train/elm-interactive-scenarios-core/errors/steps/131/submission.txt @@ -0,0 +1,4 @@ +let + theRecord = { alpha = 1, beta = 2 } +in +theRecord.gamma \ No newline at end of file