From 42497b60f5935c14fdbcd4b6250d507aa3149e63 Mon Sep 17 00:00:00 2001 From: Sathya Gunasekaran Date: Wed, 8 Nov 2023 16:09:13 +0000 Subject: [PATCH] [error] Prefix side-effecting function names with throw I did a double take when I thought we didn't handle returning the error when reading the code and when I edited the code, typescript told me that there's no need to return as creating the error will throw. This PR makes it clear from the name of the function that we will throw. --- .../babel-plugin-react-forget/src/CompilerError.ts | 10 ++++++---- .../src/Entrypoint/Imports.ts | 4 ++-- .../babel-plugin-react-forget/src/HIR/Environment.ts | 6 +++--- .../src/HIR/FindContextIdentifiers.ts | 2 +- .../src/Inference/InferReferenceEffects.ts | 4 ++-- .../src/ReactiveScopes/CodegenReactiveFunction.ts | 2 +- .../babel-plugin-react-forget/src/SSA/EnterSSA.ts | 2 +- .../src/Validation/ValidateUseMemo.ts | 4 ++-- 8 files changed, 18 insertions(+), 16 deletions(-) diff --git a/compiler/packages/babel-plugin-react-forget/src/CompilerError.ts b/compiler/packages/babel-plugin-react-forget/src/CompilerError.ts index 4686d8af35..c515991dd4 100644 --- a/compiler/packages/babel-plugin-react-forget/src/CompilerError.ts +++ b/compiler/packages/babel-plugin-react-forget/src/CompilerError.ts @@ -116,7 +116,9 @@ export class CompilerError extends Error { } } - static todo(options: Omit): never { + static throwTodo( + options: Omit + ): never { const errors = new CompilerError(); errors.pushErrorDetail( new CompilerErrorDetail({ ...options, severity: ErrorSeverity.Todo }) @@ -124,7 +126,7 @@ export class CompilerError extends Error { throw errors; } - static invalidJS( + static throwInvalidJS( options: Omit ): never { const errors = new CompilerError(); @@ -137,7 +139,7 @@ export class CompilerError extends Error { throw errors; } - static invalidReact( + static throwInvalidReact( options: Omit ): never { const errors = new CompilerError(); @@ -150,7 +152,7 @@ export class CompilerError extends Error { throw errors; } - static invalidConfig( + static throwInvalidConfig( options: Omit ): never { const errors = new CompilerError(); diff --git a/compiler/packages/babel-plugin-react-forget/src/Entrypoint/Imports.ts b/compiler/packages/babel-plugin-react-forget/src/Entrypoint/Imports.ts index 306f19b87f..9098f17ca6 100644 --- a/compiler/packages/babel-plugin-react-forget/src/Entrypoint/Imports.ts +++ b/compiler/packages/babel-plugin-react-forget/src/Entrypoint/Imports.ts @@ -24,7 +24,7 @@ export function addImportsToProgram( * validation here */ if (identifiers.has(importSpecifierName)) { - CompilerError.invalidConfig({ + CompilerError.throwInvalidConfig({ reason: `Encountered conflicting import specifier for ${importSpecifierName} in Forget config.`, description: null, loc: GeneratedSource, @@ -32,7 +32,7 @@ export function addImportsToProgram( }); } if (path.scope.hasBinding(importSpecifierName)) { - CompilerError.invalidConfig({ + CompilerError.throwInvalidConfig({ reason: `Encountered conflicting import specifiers for ${importSpecifierName} in generated program.`, description: null, loc: GeneratedSource, diff --git a/compiler/packages/babel-plugin-react-forget/src/HIR/Environment.ts b/compiler/packages/babel-plugin-react-forget/src/HIR/Environment.ts index 7625627cc9..29b8e8cfa9 100644 --- a/compiler/packages/babel-plugin-react-forget/src/HIR/Environment.ts +++ b/compiler/packages/babel-plugin-react-forget/src/HIR/Environment.ts @@ -288,7 +288,7 @@ export function parseConfigPragma(pragma: string): EnvironmentConfig { if (config.success) { return config.data; } - CompilerError.invalidConfig({ + CompilerError.throwInvalidConfig({ reason: `${fromZodError(config.error)}`, description: "Update Forget config to fix the error", loc: null, @@ -456,7 +456,7 @@ export function validateEnvironmentConfig( return config.data; } - CompilerError.invalidConfig({ + CompilerError.throwInvalidConfig({ reason: `${fromZodError(config.error)}`, description: "Update Forget config to fix the error", loc: null, @@ -474,7 +474,7 @@ export function tryParseExternalFunction( return externalFunction.data; } - CompilerError.invalidConfig({ + CompilerError.throwInvalidConfig({ reason: `${fromZodError(externalFunction.error)}`, description: "Update Forget config to fix the error", loc: null, diff --git a/compiler/packages/babel-plugin-react-forget/src/HIR/FindContextIdentifiers.ts b/compiler/packages/babel-plugin-react-forget/src/HIR/FindContextIdentifiers.ts index 420e043572..a7a3b6813b 100644 --- a/compiler/packages/babel-plugin-react-forget/src/HIR/FindContextIdentifiers.ts +++ b/compiler/packages/babel-plugin-react-forget/src/HIR/FindContextIdentifiers.ts @@ -189,7 +189,7 @@ function handleAssignment( break; } default: { - CompilerError.todo({ + CompilerError.throwTodo({ reason: `[FindContextIdentifiers] Cannot handle Object destructuring assignment target ${lvalNode.type}`, description: null, loc: lvalNode.loc ?? GeneratedSource, diff --git a/compiler/packages/babel-plugin-react-forget/src/Inference/InferReferenceEffects.ts b/compiler/packages/babel-plugin-react-forget/src/Inference/InferReferenceEffects.ts index b39fe3e634..5c02ee5331 100644 --- a/compiler/packages/babel-plugin-react-forget/src/Inference/InferReferenceEffects.ts +++ b/compiler/packages/babel-plugin-react-forget/src/Inference/InferReferenceEffects.ts @@ -353,7 +353,7 @@ class InferenceState { ) { effect = Effect.Mutate; } else { - CompilerError.invalidReact({ + CompilerError.throwInvalidReact({ reason: `This mutates a variable after it was passed to React, which means that React cannot observe changes to it`, description: place.identifier.name !== null @@ -370,7 +370,7 @@ class InferenceState { valueKind !== ValueKind.Mutable && valueKind !== ValueKind.Context ) { - CompilerError.invalidReact({ + CompilerError.throwInvalidReact({ reason: `This mutates a variable after it was passed to React, which means that React cannot observe changes to it`, description: place.identifier.name !== null diff --git a/compiler/packages/babel-plugin-react-forget/src/ReactiveScopes/CodegenReactiveFunction.ts b/compiler/packages/babel-plugin-react-forget/src/ReactiveScopes/CodegenReactiveFunction.ts index b36972a07d..0e77111ec5 100644 --- a/compiler/packages/babel-plugin-react-forget/src/ReactiveScopes/CodegenReactiveFunction.ts +++ b/compiler/packages/babel-plugin-react-forget/src/ReactiveScopes/CodegenReactiveFunction.ts @@ -464,7 +464,7 @@ function codegenTerminal( suggestions: null, }); if (terminal.init.instructions.length !== 2) { - CompilerError.todo({ + CompilerError.throwTodo({ reason: "Support non-trivial ForOf inits", description: null, loc: terminal.init.loc, diff --git a/compiler/packages/babel-plugin-react-forget/src/SSA/EnterSSA.ts b/compiler/packages/babel-plugin-react-forget/src/SSA/EnterSSA.ts index 7dcbac1fdd..ab7604a03c 100644 --- a/compiler/packages/babel-plugin-react-forget/src/SSA/EnterSSA.ts +++ b/compiler/packages/babel-plugin-react-forget/src/SSA/EnterSSA.ts @@ -98,7 +98,7 @@ class SSABuilder { definePlace(oldPlace: Place): Place { const oldId = oldPlace.identifier; if (this.#unknown.has(oldId)) { - CompilerError.todo({ + CompilerError.throwTodo({ reason: `EnterSSA: Expected identifier to be defined before being used`, description: `Identifier ${printIdentifier(oldId)} is undefined`, loc: oldPlace.loc, diff --git a/compiler/packages/babel-plugin-react-forget/src/Validation/ValidateUseMemo.ts b/compiler/packages/babel-plugin-react-forget/src/Validation/ValidateUseMemo.ts index 8ab8a0ba72..5e7a40f972 100644 --- a/compiler/packages/babel-plugin-react-forget/src/Validation/ValidateUseMemo.ts +++ b/compiler/packages/babel-plugin-react-forget/src/Validation/ValidateUseMemo.ts @@ -61,7 +61,7 @@ export function validateUseMemo(fn: HIRFunction): void { } if (body.loweredFunc.func.params.length > 0) { - CompilerError.invalidReact({ + CompilerError.throwInvalidReact({ reason: "useMemo callbacks may not accept any arguments", description: null, loc: body.loc, @@ -70,7 +70,7 @@ export function validateUseMemo(fn: HIRFunction): void { } if (body.loweredFunc.func.async || body.loweredFunc.func.generator) { - CompilerError.invalidReact({ + CompilerError.throwInvalidReact({ reason: "useMemo callbacks may not be async or generator functions", description: null,