diff --git a/compiler/forget/packages/playground/lib/forgetMonacoDiagnostics.ts b/compiler/forget/packages/playground/lib/forgetMonacoDiagnostics.ts index 2c3d470b93..a6510920c0 100644 --- a/compiler/forget/packages/playground/lib/forgetMonacoDiagnostics.ts +++ b/compiler/forget/packages/playground/lib/forgetMonacoDiagnostics.ts @@ -23,18 +23,18 @@ function mapForgetDiagnosticToMonacoMarker( detail: CompilerErrorDetail, monaco: Monaco ): editor.IMarkerData | null { - if (detail.nodePath == null || detail.nodePath.node?.loc == null) { + if (detail.loc == null) { return null; } const severity = mapForgetSeverityToMonaco(detail.severity, monaco); - let message = detail.errorMessage; + let message = detail.printErrorMessage(); return { severity, message, - startLineNumber: detail.nodePath.node.loc.start.line, - startColumn: detail.nodePath.node.loc.start.column + 1, - endLineNumber: detail.nodePath.node.loc.end.line, - endColumn: detail.nodePath.node.loc.end.column + 1, + startLineNumber: detail.loc.start.line, + startColumn: detail.loc.start.column + 1, + endLineNumber: detail.loc.end.line, + endColumn: detail.loc.end.column + 1, }; } diff --git a/compiler/forget/src/CompilerError.ts b/compiler/forget/src/CompilerError.ts index 802c0d579e..d3481560e8 100644 --- a/compiler/forget/src/CompilerError.ts +++ b/compiler/forget/src/CompilerError.ts @@ -1,4 +1,6 @@ -import { Node, NodePath } from "@babel/core"; +import { Node, NodePath } from "@babel/traverse"; +import { SourceLocation } from "@babel/types"; +import { ExtractClassProperties } from "./Utils/types"; import { assertExhaustive } from "./Utils/utils"; export enum ErrorSeverity { @@ -9,10 +11,11 @@ export enum ErrorSeverity { export type CompilerErrorOptions = { reason: string; severity: ErrorSeverity; - nodePath: AnyNodePath | null; + nodePath: AnyNodePath; }; type AnyNodePath = NodePath; type CompilerErrorKind = typeof InvalidInputError | typeof TodoError; +type CompilerErrorDetailOptions = ExtractClassProperties; function mapSeverityToErrorCtor(severity: ErrorSeverity): CompilerErrorKind { switch (severity) { @@ -37,6 +40,22 @@ class TodoError extends Error { } } +export function tryPrintCodeFrame( + options: CompilerErrorOptions +): string | null { + if (options.nodePath == null) return null; + try { + return options.nodePath + .buildCodeFrameError( + options.reason, + mapSeverityToErrorCtor(options.severity) + ) + .toString(); + } catch { + return null; + } +} + /** * Each bailout or invariant in HIR lowering creates an {@link CompilerErrorDetail}, which is then * aggregated into a single {@link CompilerError} later. @@ -44,43 +63,29 @@ class TodoError extends Error { export class CompilerErrorDetail { reason: string; severity: ErrorSeverity; - /** - * If a NodePath is provided, we will prefer Babel's built in codeframe error generation which - * will print error markers in the correct location. - */ - nodePath: AnyNodePath | null; + codeframe: string | null; + loc: SourceLocation | null; - constructor(options: CompilerErrorOptions) { + constructor(options: CompilerErrorDetailOptions) { this.reason = options.reason; this.severity = options.severity; - this.nodePath = options.nodePath; + this.codeframe = options.codeframe; + this.loc = options.loc; } - get errorMessage(): string { + printErrorMessage(): string { + if (this.codeframe != null) { + return this.codeframe; + } const buffer = [`${this.severity}: ${this.reason}`]; - if (this.nodePath != null && this.nodePath.node?.loc != null) { - buffer.push( - ` (${this.nodePath.node.loc.start.line}:${this.nodePath.node.loc.end.line})` - ); + if (this.loc != null) { + buffer.push(` (${this.loc.start.line}:${this.loc.end.line})`); } return buffer.join(""); } - get codeFrame() { - if (this.nodePath == null) { - return this.errorMessage; - } - try { - return this.nodePath - .buildCodeFrameError(this.reason, mapSeverityToErrorCtor(this.severity)) - .toString(); - } catch { - return this.errorMessage; - } - } - toString(): string { - return `[ReactForget] ${this.errorMessage}`; + return `[ReactForget] ${this.printErrorMessage()}`; } } diff --git a/compiler/forget/src/HIR/BuildHIR.ts b/compiler/forget/src/HIR/BuildHIR.ts index 0b00e12ed9..66d7eb35f1 100644 --- a/compiler/forget/src/HIR/BuildHIR.ts +++ b/compiler/forget/src/HIR/BuildHIR.ts @@ -73,7 +73,7 @@ export function lower( }; params.push(place); } else { - builder.pushError({ + builder.recordError({ reason: `(BuildHIR::lower) Handle ${param.node.type} params`, severity: ErrorSeverity.Todo, nodePath: param, @@ -94,7 +94,7 @@ export function lower( } else if (body.isBlockStatement()) { lowerStatement(builder, body); } else { - builder.pushError({ + builder.recordError({ reason: `(BuildHIR::lower) Unexpected function body kind: ${body.type}}`, severity: ErrorSeverity.InvalidInput, nodePath: body, @@ -341,7 +341,7 @@ function lowerStatement( const initBlock = builder.enter("value", (blockId) => { const init = stmt.get("init"); if (!init.isVariableDeclaration()) { - builder.pushError({ + builder.recordError({ reason: "(BuildHIR::lowerStatement) Handle non-variable initialization in ForStatement", severity: ErrorSeverity.Todo, @@ -361,7 +361,7 @@ function lowerStatement( const updateBlock = builder.enter("value", (blockId) => { const update = stmt.get("update"); if (update.node == null) { - builder.pushError({ + builder.recordError({ reason: `(BuildHIR::lowerStatement) Handle empty update in ForStatement`, severity: ErrorSeverity.Todo, nodePath: stmt, @@ -404,7 +404,7 @@ function lowerStatement( const test = stmt.get("test"); if (test.node == null) { - builder.pushError({ + builder.recordError({ reason: `(BuildHIR::lowerStatement) Handle empty test in ForStatement`, severity: ErrorSeverity.Todo, nodePath: stmt, @@ -573,7 +573,7 @@ function lowerStatement( const test = case_.get("test"); if (test.node == null) { if (hasDefault) { - builder.pushError({ + builder.recordError({ reason: "(BuildHIR::lowerStatement) Expected at most one `default` branch in SwitchStatement, this code should have failed to parse", severity: ErrorSeverity.InvalidInput, @@ -659,7 +659,7 @@ function lowerStatement( const stmt = stmtPath as NodePath; const nodeKind: string = stmt.node.kind; if (nodeKind === "var") { - builder.pushError({ + builder.recordError({ reason: `(BuildHIR::lowerStatement) Handle ${nodeKind} kinds in VariableDeclaration`, severity: ErrorSeverity.Todo, nodePath: stmt, @@ -742,7 +742,7 @@ function lowerStatement( case "TSNamespaceExportDeclaration": case "TSTypeAliasDeclaration": case "WithStatement": { - builder.pushError({ + builder.recordError({ reason: `(BuildHIR::lowerStatement) Handle ${stmtPath.type} statements`, severity: ErrorSeverity.Todo, nodePath: stmtPath, @@ -814,7 +814,7 @@ function lowerExpression( let hasError = false; for (const propertyPath of propertyPaths) { if (!propertyPath.isObjectProperty()) { - builder.pushError({ + builder.recordError({ reason: `(BuildHIR::lowerExpression) Handle ${propertyPath.type} properties in ObjectExpression`, severity: ErrorSeverity.Todo, nodePath: propertyPath, @@ -824,7 +824,7 @@ function lowerExpression( } const key = propertyPath.node.key; if (key.type !== "Identifier") { - builder.pushError({ + builder.recordError({ reason: `(BuildHIR::lowerExpression) Expected Identifier, got ${key.type} key in ObjectExpression`, severity: ErrorSeverity.InvalidInput, nodePath: propertyPath, @@ -834,7 +834,7 @@ function lowerExpression( } const valuePath = propertyPath.get("value"); if (!valuePath.isExpression()) { - builder.pushError({ + builder.recordError({ reason: `(BuildHIR::lowerExpression) Handle ${valuePath.type} values in ObjectExpression`, severity: ErrorSeverity.Todo, nodePath: valuePath, @@ -859,7 +859,7 @@ function lowerExpression( let elements: Place[] = []; for (const element of expr.get("elements")) { if (element.node == null || !element.isExpression()) { - builder.pushError({ + builder.recordError({ reason: `(BuildHIR::lowerExpression) Handle ${element.type} elements in ArrayExpression`, severity: ErrorSeverity.Todo, nodePath: element, @@ -883,7 +883,7 @@ function lowerExpression( const expr = exprPath as NodePath; const calleePath = expr.get("callee"); if (!calleePath.isExpression()) { - builder.pushError({ + builder.recordError({ reason: `(BuildHIR::lowerExpression) Expected Expression, got ${calleePath.type} in NewExpression (v8 intrinsics not supported): ${calleePath.type}`, severity: ErrorSeverity.InvalidInput, nodePath: calleePath, @@ -895,7 +895,7 @@ function lowerExpression( let hasError = false; for (const argPath of expr.get("arguments")) { if (!argPath.isExpression()) { - builder.pushError({ + builder.recordError({ reason: `(BuildHIR::lowerExpression) Handle ${argPath.type} arguments in NewExpression`, severity: ErrorSeverity.Todo, nodePath: argPath, @@ -920,7 +920,7 @@ function lowerExpression( const calleePath = expr.get("callee"); let hasError = false; if (!calleePath.isExpression()) { - builder.pushError({ + builder.recordError({ reason: `(BuildHIR::lowerExpression) Expected Expression, got ${calleePath.type} in CallExpression (v8 intrinsics not supported)`, severity: ErrorSeverity.InvalidInput, nodePath: calleePath, @@ -935,7 +935,7 @@ function lowerExpression( let args: Place[] = []; for (const argPath of expr.get("arguments")) { if (!argPath.isExpression()) { - builder.pushError({ + builder.recordError({ reason: `(BuildHIR::lowerExpression) Handle ${argPath.type} arguments in CallExpression`, severity: ErrorSeverity.Todo, nodePath: argPath, @@ -967,7 +967,7 @@ function lowerExpression( let args: Place[] = []; for (const argPath of expr.get("arguments")) { if (!argPath.isExpression()) { - builder.pushError({ + builder.recordError({ reason: `(BuildHIR::lowerExpression) Handle ${argPath.type} arguments in CallExpression`, severity: ErrorSeverity.Todo, nodePath: argPath, @@ -991,7 +991,7 @@ function lowerExpression( const expr = exprPath as NodePath; const leftPath = expr.get("left"); if (!leftPath.isExpression()) { - builder.pushError({ + builder.recordError({ reason: `(BuildHIR::lowerExpression) Expected Expression, got ${leftPath.type} lval in BinaryExpression`, severity: ErrorSeverity.InvalidInput, nodePath: leftPath, @@ -1115,7 +1115,7 @@ function lowerExpression( }; const binaryOperator = operators[operator]; if (binaryOperator == null) { - builder.pushError({ + builder.recordError({ reason: `(BuildHIR::lowerExpression) Handle ${operator} operaators in AssignmentExpression`, severity: ErrorSeverity.Todo, nodePath: expr.get("operator"), @@ -1154,7 +1154,7 @@ function lowerExpression( // Extract the final property to be read from and re-assigned, eg 'c' const property = leftExpr.get("property"); if (!property.isIdentifier()) { - builder.pushError({ + builder.recordError({ reason: `(BuildHIR::lowerExpression) Handle ${property.type} properties in MemberExpression`, severity: ErrorSeverity.Todo, nodePath: property, @@ -1213,7 +1213,7 @@ function lowerExpression( }; } default: { - builder.pushError({ + builder.recordError({ reason: `(BuildHIR::lowerExpression) Expected Identifier or MemberExpression, got ${expr.type} lval in AssignmentExpression`, severity: ErrorSeverity.InvalidInput, nodePath: expr, @@ -1248,7 +1248,7 @@ function lowerExpression( let hasError = false; for (const attribute of opening.get("attributes")) { if (!attribute.isJSXAttribute()) { - builder.pushError({ + builder.recordError({ reason: `(BuildHIR::lowerExpression) Handle ${attribute.type} attributes in JSXElement`, severity: ErrorSeverity.Todo, nodePath: attribute, @@ -1258,7 +1258,7 @@ function lowerExpression( } const name = attribute.get("name"); if (!name.isJSXIdentifier()) { - builder.pushError({ + builder.recordError({ reason: `(BuildHIR::lowerExpression) Handle ${name.type} attribute names in JSXElement`, severity: ErrorSeverity.Todo, nodePath: name, @@ -1272,7 +1272,7 @@ function lowerExpression( value = lowerExpressionToPlace(builder, valueExpr); } else { if (!valueExpr.isJSXExpressionContainer()) { - builder.pushError({ + builder.recordError({ reason: `(BuildHIR::lowerExpression) Handle ${valueExpr.type} attribute values in JSXElement`, severity: ErrorSeverity.Todo, nodePath: valueExpr, @@ -1282,7 +1282,7 @@ function lowerExpression( } const expression = valueExpr.get("expression"); if (!expression.isExpression()) { - builder.pushError({ + builder.recordError({ reason: `(BuildHIR::lowerExpression) Handle ${expression.type} expressions in JSXExpressionContainer within JSXElement`, severity: ErrorSeverity.Todo, nodePath: valueExpr, @@ -1334,7 +1334,7 @@ function lowerExpression( const lowering = lower(expr); let loweredFunc: HIRFunction; if (lowering.isErr()) { - lowering.unwrapErr().forEach((e) => builder.pushError(e)); + lowering.unwrapErr().forEach((e) => builder.pushErrorDetail(e)); return { kind: "UnsupportedNode", node: exprNode, @@ -1347,7 +1347,7 @@ function lowerExpression( const params: Array = []; for (const p of expr.get("params")) { if (!p.isIdentifier()) { - builder.pushError({ + builder.recordError({ reason: `(BuildHIR::lowerExpression) Handle ${p.type} params in FunctionExpression`, severity: ErrorSeverity.Todo, nodePath: p, @@ -1373,7 +1373,7 @@ function lowerExpression( case "TaggedTemplateExpression": { const expr = exprPath as NodePath; if (expr.get("quasi").get("expressions").length !== 0) { - builder.pushError({ + builder.recordError({ reason: "Unhandled tagged template with interpolations", severity: ErrorSeverity.Todo, nodePath: exprPath, @@ -1387,7 +1387,7 @@ function lowerExpression( const value = expr.get("quasi").get("quasis").at(0)!.node.value; if (value.raw !== value.cooked) { - builder.pushError({ + builder.recordError({ reason: "Unhandled tagged template where cooked value is different from raw value", severity: ErrorSeverity.Todo, @@ -1413,7 +1413,7 @@ function lowerExpression( }; } default: { - builder.pushError({ + builder.recordError({ reason: `(BuildHIR::lowerExpression) Handle ${exprPath.type} expressions`, severity: ErrorSeverity.Todo, nodePath: exprPath, @@ -1433,7 +1433,7 @@ function lowerMemberExpression( const property = expr.get("property"); if (!expr.node.computed) { if (!property.isIdentifier()) { - builder.pushError({ + builder.recordError({ reason: `(BuildHIR::lowerExpression) Handle ${property.type} property`, severity: ErrorSeverity.Todo, nodePath: property, @@ -1454,7 +1454,7 @@ function lowerMemberExpression( return { object, property: property.node.name, value }; } else { if (!property.isExpression()) { - builder.pushError({ + builder.recordError({ reason: `(BuildHIR::lowerMemberExpression) Expected Expression, got ${property.type} property`, severity: ErrorSeverity.InvalidInput, nodePath: property, @@ -1543,7 +1543,7 @@ function lowerJsxElementName( const exprNode = exprPath.node; const exprLoc = exprNode.loc ?? GeneratedSource; if (!exprPath.isJSXIdentifier()) { - builder.pushError({ + builder.recordError({ reason: `(BuildHIR::lowerJsxElementName) Handle ${exprPath.type} tags`, severity: ErrorSeverity.Todo, nodePath: exprPath, @@ -1604,7 +1604,7 @@ function lowerJsxElement( } else if (exprPath.isJSXExpressionContainer()) { const expression = exprPath.get("expression"); if (!expression.isExpression()) { - builder.pushError({ + builder.recordError({ reason: `(BuildHIR::lowerJsxElement) Handle ${expression.type} expressions`, severity: ErrorSeverity.Todo, nodePath: expression, @@ -1638,7 +1638,7 @@ function lowerJsxElement( return place; } else { if (!(t.isJSXFragment(exprNode) || t.isJSXSpreadChild(exprNode))) { - builder.pushError({ + builder.recordError({ reason: `(BuildHIR::lowerJsxElement) Expected refinement to work, got: ${exprPath.type}`, severity: ErrorSeverity.InvalidInput, nodePath: exprPath, @@ -1765,7 +1765,7 @@ function lowerAssignment( } if (!lvalue.node.computed) { if (!property.isIdentifier()) { - builder.pushError({ + builder.recordError({ reason: `(BuildHIR::lowerAssignment) Handle ${property.type} properties in MemberExpression`, severity: ErrorSeverity.Todo, nodePath: property, @@ -1781,7 +1781,7 @@ function lowerAssignment( }; } else { if (!property.isExpression()) { - builder.pushError({ + builder.recordError({ reason: "Expected private name to appear as a non-computed property", severity: ErrorSeverity.InvalidInput, @@ -1816,7 +1816,7 @@ function lowerAssignment( continue; } if (element.node.type === "RestElement") { - builder.pushError({ + builder.recordError({ reason: `(BuildHIR::lowerAssignment) Handle ${element.type} in ArrayPattern`, severity: ErrorSeverity.Todo, nodePath: element, @@ -1864,7 +1864,7 @@ function lowerAssignment( for (let i = 0; i < properties.length; i++) { const property = properties[i]; if (!property.isObjectProperty()) { - builder.pushError({ + builder.recordError({ reason: `(BuildHIR::lowerAssignment) Handle ${property.type} properties in ObjectPattern`, severity: ErrorSeverity.Todo, nodePath: property, @@ -1874,7 +1874,7 @@ function lowerAssignment( } const key = property.get("key"); if (!key.isIdentifier()) { - builder.pushError({ + builder.recordError({ reason: `(BuildHIR::lowerAssignment) Handle ${key.type} keys in ObjectPattern`, severity: ErrorSeverity.Todo, nodePath: key, @@ -1884,7 +1884,7 @@ function lowerAssignment( } const element = property.get("value"); if (!element.isLVal()) { - builder.pushError({ + builder.recordError({ reason: `(BuildHIR::lowerAssignment) Expected object property value to be an LVal, got: ${element.type}`, severity: ErrorSeverity.InvalidInput, nodePath: element, @@ -1906,7 +1906,7 @@ function lowerAssignment( : objectPlace; } default: { - builder.pushError({ + builder.recordError({ reason: `(BuildHIR::lowerAssignment) Handle ${lvaluePath.type} assignments`, severity: ErrorSeverity.Todo, nodePath: lvaluePath, diff --git a/compiler/forget/src/HIR/HIRBuilder.ts b/compiler/forget/src/HIR/HIRBuilder.ts index 347dc61782..3119c3ac16 100644 --- a/compiler/forget/src/HIR/HIRBuilder.ts +++ b/compiler/forget/src/HIR/HIRBuilder.ts @@ -8,7 +8,11 @@ import { NodePath } from "@babel/traverse"; import * as t from "@babel/types"; import invariant from "invariant"; -import { CompilerErrorDetail, CompilerErrorOptions } from "../CompilerError"; +import { + CompilerErrorDetail, + CompilerErrorOptions, + tryPrintCodeFrame, +} from "../CompilerError"; import { logHIR } from "../Utils/logger"; import { assertExhaustive } from "../Utils/utils"; import { getOrAddGlobal } from "./Globals"; @@ -415,8 +419,17 @@ export default class HIRBuilder { invariant(false, "Expected a loop to be in scope"); } - pushError(options: CompilerErrorOptions): CompilerErrorDetail { - const detail = new CompilerErrorDetail(options); + recordError(options: CompilerErrorOptions): CompilerErrorDetail { + const detail = new CompilerErrorDetail({ + reason: options.reason, + severity: options.severity, + codeframe: tryPrintCodeFrame(options), + loc: options.nodePath?.node?.loc ?? null, + }); + return this.pushErrorDetail(detail); + } + + pushErrorDetail(detail: CompilerErrorDetail): CompilerErrorDetail { this.errors.push(detail); return detail; } diff --git a/compiler/forget/src/__tests__/hir-test.ts b/compiler/forget/src/__tests__/hir-test.ts index 6d6bb4fe7c..13e76ab4de 100644 --- a/compiler/forget/src/__tests__/hir-test.ts +++ b/compiler/forget/src/__tests__/hir-test.ts @@ -10,7 +10,6 @@ import { wasmFolder } from "@hpcc-js/wasm"; import path from "path"; import runReactForgetBabelPlugin from "../Babel/RunReactForgetBabelPlugin"; -import { CompilerError } from "../CompilerError"; import { toggleLogging } from "../Utils/logger"; import generateTestsFromFixtures from "./test-utils/generateTestsFromFixtures"; @@ -99,13 +98,7 @@ ${outputs.join("\n")} }); function formatErrorOutput(error: Error): string { - if (error instanceof CompilerError) { - error.message = error.details - .map((detail) => `[ReactForget] ${detail.codeFrame}`) - .join("\n\n"); - } else { - error.message = error.message.replace(/^\/.*?:\s/, ""); - } + error.message = error.message.replace(/^\/.*?:\s/, ""); return ` ## Error