From d9faaf91f76d94d76262a2c73d08b5fc87c8c6ee Mon Sep 17 00:00:00 2001 From: Marcello Duarte Date: Sat, 1 Aug 2026 11:52:14 +0100 Subject: [PATCH] Fix fatal errors: normalise the REPL error spectrum MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Route every evaluation through a single boundary (withEvaluationBoundary) that turns PHP warnings, notices and Throwables into one clean phunkie error — fixing undefined-property access, which previously leaked a raw E_WARNING and a stray null. Render uncatchable fatals (e.g. "Cannot redeclare" from loading a file twice) as a clean error via a shutdown formatter instead of a raw stack trace, and capture a loaded file's variables into the session. --- features/repl/load_files.feature | 15 ++++++ src/Functions/evaluation.php | 83 ++++++++++++++++++++++++++++++-- src/Repl/ReplLoop.php | 81 ++++++++++++++++++++++++++++--- 3 files changed, 168 insertions(+), 11 deletions(-) diff --git a/features/repl/load_files.feature b/features/repl/load_files.feature index cd8907d..88e4bbd 100644 --- a/features/repl/load_files.feature +++ b/features/repl/load_files.feature @@ -46,6 +46,21 @@ Feature: Loading .phunkie files When I enter "helper()" Then I should see output containing "helper result" + Scenario: Loading a file with variable definitions + Given I have a file "config.php" with content: + """ + $parsed */ - $parsed = \Phunkie\Console\Functions\parseInput($input); - return $parsed->flatMap(fn(array $ast) => evaluateAst($ast, $session)); + return withEvaluationBoundary(trim($input), static function () use ($input, $session): Validation { + /** @var Validation $parsed */ + $parsed = \Phunkie\Console\Functions\parseInput($input); + + return $parsed->flatMap(fn(array $ast) => evaluateAst($ast, $session)); + }); +} + +/** + * Runs an evaluation behind a single boundary that turns PHP's diagnostic + * spectrum into one cleanly formatted phunkie error. + * + * PHP reports problems in three incompatible ways, and every REPL evaluation + * path funnels through here so each is handled once, in one place: + * + * - Warnings, notices and user errors are NOT throwables — a bare + * `$obj->missing` emits an E_WARNING and returns null, so without this the + * warning prints raw and the null is shown as the result. A temporary error + * handler traps them and surfaces them as a Failure. + * - Throwables (Error, TypeError, DivisionByZeroError, …) are caught. + * - Deprecations are advisory, so they are swallowed without failing otherwise + * valid code. + * + * Uncatchable fatals (E_ERROR / E_COMPILE_ERROR, e.g. "Cannot redeclare") cannot + * be caught in-process and are prevented earlier, before they reach eval(). + * + * @param callable(): Validation $evaluate + * @return Validation + */ +function withEvaluationBoundary(string $expression, callable $evaluate): Validation +{ + $trappedMessage = null; + + set_error_handler(static function (int $severity, string $message) use (&$trappedMessage): bool { + // Honour @-suppression and the configured error_reporting level. + if ((error_reporting() & $severity) === 0) { + return false; + } + + // Deprecations are advisory: let the evaluation keep its result. + if (($severity & (E_DEPRECATED | E_USER_DEPRECATED)) !== 0) { + return true; + } + + $trappedMessage ??= $message; + + return true; + }); + + try { + /** @var Validation $result */ + $result = $evaluate(); + } catch (\Throwable $e) { + return Failure(new EvaluationError($expression, cleanErrorMessage($e->getMessage()))); + } finally { + restore_error_handler(); + } + + // A trapped warning on an otherwise-successful evaluation becomes the result, + // so the user sees a clean error rather than a leaked warning plus a stray null. + if ($trappedMessage !== null && !$result->isLeft()) { + return Failure(new EvaluationError($expression, cleanErrorMessage($trappedMessage))); + } + + return $result; } /** @@ -4299,8 +4361,19 @@ function cleanErrorMessage(string $message): string // Remove "and exactly N expected" part to shorten the message $message = preg_replace('/\s+and exactly \d+ expected$/', '', $message); - // Remove any remaining eval()'d code references - $message = preg_replace('/\s+in\s+[^\s]+:\s*eval\(\)\'d code[^\s]*/', '', $message); + // Remove any remaining eval()'d code references, including the parenthetical + // "(previously declared in /path : eval()'d code:1)" that a redeclare fatal + // carries, whichever way the location is spelled. + $message = preg_replace('/\s*\([^)]*eval\(\)\'d code[^)]*\)/', '', $message); + $message = preg_replace('/\s+in\s+\S+\s*:\s*eval\(\)\'d code(?::\d+| on line \d+)?/', '', $message); + + // A redeclare fatal's location lived inside "(previously declared …)"; once + // the location is gone the empty parenthetical is just noise. + $message = preg_replace('/\s*\(previously declared\s*\)/', '', $message); + + // Remove a trailing internal location, e.g. "in /path/evaluation.php on line 781", + // which leaks the REPL's own source into a message meant for the user. + $message = preg_replace('/\s+in\s+\S+\.php\s+on line \d+/', '', $message); // Clean up any double spaces that might result $message = preg_replace('/\s+/', ' ', $message); diff --git a/src/Repl/ReplLoop.php b/src/Repl/ReplLoop.php index 1f31785..987bf77 100644 --- a/src/Repl/ReplLoop.php +++ b/src/Repl/ReplLoop.php @@ -21,7 +21,7 @@ use Phunkie\Utils\Trampoline\Trampoline; use function Phunkie\Effect\Functions\console\printLn; -use function Phunkie\Console\Functions\{evaluateExpression, addToHistory, setVariable, nextVariable, isColorEnabled, printHelp, printVariables, printHistory, printBanner, readLineFiltered, resetSession, setNamespace, addUseStatement}; +use function Phunkie\Console\Functions\{evaluateExpression, addToHistory, setVariable, nextVariable, isColorEnabled, printHelp, printVariables, printHistory, printBanner, readLineFiltered, resetSession, setNamespace, addUseStatement, cleanErrorMessage}; use function Phunkie\Functions\trampoline\{More, Done}; /** @@ -36,7 +36,47 @@ function replLoop(ReplSession $session): IO { // Run the trampolined loop - return new IO(fn() => replLoopTrampoline($session)->run()); + return new IO(function () use ($session) { + installFatalErrorFormatter(); + + return replLoopTrampoline($session)->run(); + }); +} + +/** + * Renders an uncatchable fatal as a clean phunkie error. + * + * The evaluation boundary already turns warnings, notices and every Throwable + * into a formatted error. A handful of failures — E_ERROR and the compile-time + * fatals such as "Cannot redeclare" — reach neither try/catch nor a custom error + * handler and terminate the process. PHP's own rendering of those is silenced + * and re-emitted here through a shutdown handler, so a fatal reads like any other + * REPL error rather than a raw stack trace pointing at the REPL's internals. + * + * This formats the fatal; it cannot resume the session. Surviving a fatal (so the + * REPL keeps its state and carries on) needs each evaluation to run in its own + * process — a separate, larger change. + */ +function installFatalErrorFormatter(): void +{ + ini_set('display_errors', '0'); + ini_set('log_errors', '0'); + + register_shutdown_function(static function (): void { + $error = error_get_last(); + + if ($error === null) { + return; + } + + $fatalSeverities = E_ERROR | E_PARSE | E_CORE_ERROR | E_COMPILE_ERROR | E_USER_ERROR; + + if (($error['type'] & $fatalSeverities) === 0) { + return; + } + + fwrite(STDOUT, "\nError: " . cleanErrorMessage($error['message']) . "\n"); + }); } /** @@ -364,10 +404,22 @@ function loadFile(string $filepath, ReplSession $session): IO // Capture output to suppress it during load ob_start(); - // Evaluate the file content as a single block - // This will execute the entire file and capture all definitions + // Evaluate the file content as a single block. Functions and classes it + // defines land in the global scope and persist. Its top-level variables, + // however, are locals of the eval, so they are captured here (via an + // isolated closure whose only other local is the source) and threaded + // back into the session below, so a loaded file's variables are usable + // in the REPL just like its functions and classes. try { - eval($contents); + $definedVariables = (static function (string $phunkieLoadSource): array { + eval($phunkieLoadSource); + + /** @var array $phunkieLoadVariables the variables eval defined, opaque to static analysis */ + $phunkieLoadVariables = get_defined_vars(); + unset($phunkieLoadVariables['phunkieLoadSource']); + + return $phunkieLoadVariables; + })($contents); } catch (\Throwable $e) { ob_end_clean(); printLn("Error loading file: " . $e->getMessage())->unsafeRun(); @@ -377,10 +429,27 @@ function loadFile(string $filepath, ReplSession $session): IO // Discard captured output ob_end_clean(); + // Thread the file's variables into the session, keyed like every other + // REPL variable (with the leading `$`). + $variables = $session->variables; + foreach ($definedVariables as $name => $value) { + $variables = $variables->plus('$' . $name, $value); + } + + $newSession = new ReplSession( + $session->history, + $variables, + $session->colorEnabled, + $session->variableCounter, + $session->incompleteInput, + $session->currentNamespace, + $session->useStatements + ); + // Get the basename for the message $filename = basename($filepath); printLn("// file $filename loaded")->unsafeRun(); - return new ContinueRepl($session); + return new ContinueRepl($newSession); }); }