From fa4268fd48b1e582bb66c6a95a370142dc2b3ef6 Mon Sep 17 00:00:00 2001 From: SandroMaglione Date: Thu, 27 Aug 2026 09:54:17 +0200 Subject: [PATCH] Add reliable planning lint rules --- .changeset/calm-machines-plan.md | 7 + packages/effect-machine/docs/agent-guide.md | 19 + .../effect-machine/docs/machine-review.md | 8 + packages/oxlint-plugin/README.md | 160 +++++++- packages/oxlint-plugin/src/index.ts | 6 + .../oxlint-plugin/src/internal/ambient.ts | 134 +++++++ packages/oxlint-plugin/src/internal/ast.ts | 83 +++++ .../oxlint-plugin/src/internal/imports.ts | 35 +- .../oxlint-plugin/src/internal/planning.ts | 76 +++- .../internal/rules/noAsyncPlanningCallback.ts | 25 +- .../internal/rules/noBrowserApiInPlanning.ts | 57 +++ .../rules/noConflictingInvocationIdentity.ts | 343 ++++++++++++++++++ .../rules/noNondeterministicPlanning.ts | 39 ++ .../src/internal/rules/noRedundantResolve.ts | 64 +++- packages/oxlint-plugin/src/recommended.ts | 3 + .../test/noAsyncPlanningCallback.test.ts | 64 +++- .../test/noBrowserApiInPlanning.test.ts | 57 +++ .../noConflictingInvocationIdentity.test.ts | 93 +++++ .../test/noNondeterministicPlanning.test.ts | 55 +++ .../test/noRedundantResolve.test.ts | 27 +- scripts/oxlint-plugin-pack-check.mjs | 14 +- 21 files changed, 1321 insertions(+), 48 deletions(-) create mode 100644 .changeset/calm-machines-plan.md create mode 100644 packages/oxlint-plugin/src/internal/ambient.ts create mode 100644 packages/oxlint-plugin/src/internal/ast.ts create mode 100644 packages/oxlint-plugin/src/internal/rules/noBrowserApiInPlanning.ts create mode 100644 packages/oxlint-plugin/src/internal/rules/noConflictingInvocationIdentity.ts create mode 100644 packages/oxlint-plugin/src/internal/rules/noNondeterministicPlanning.ts create mode 100644 packages/oxlint-plugin/test/noBrowserApiInPlanning.test.ts create mode 100644 packages/oxlint-plugin/test/noConflictingInvocationIdentity.test.ts create mode 100644 packages/oxlint-plugin/test/noNondeterministicPlanning.test.ts diff --git a/.changeset/calm-machines-plan.md b/.changeset/calm-machines-plan.md new file mode 100644 index 0000000..9c3f9db --- /dev/null +++ b/.changeset/calm-machines-plan.md @@ -0,0 +1,7 @@ +--- +"@typeonce/oxlint-plugin-effect-machine": minor +--- + +Add recommended rules that reject duplicate invocation identities, browser API access during planning, and nondeterministic time or randomness during planning. + +Strengthen `no-async-planning-callback` to detect direct Promise, fetch, timer, and scheduling operations, and extend `no-redundant-resolve` fixes to resolver-only reentry and empty targetless resolvers. Diagnostics now explain how to move work into state-owned invocations, pass external facts through input or events, or model sequential work with separate states. diff --git a/packages/effect-machine/docs/agent-guide.md b/packages/effect-machine/docs/agent-guide.md index 8219f22..6eeefae 100644 --- a/packages/effect-machine/docs/agent-guide.md +++ b/packages/effect-machine/docs/agent-guide.md @@ -318,6 +318,25 @@ machine. Do not start a promise inside a transition callback. A transition has no lifetime in which to own that work. A state does. +Give every invocation declared by one state a unique lifecycle ID. Give every +logic or child process that can be active at the same time a unique runtime +address as well: + +```ts +Loading: { + invoke: (from) => [ + from.effect("load-document", loadDocument), + from.timer("load-timeout", "10 seconds") + ] +} +``` + +Invocation outcomes are routed by state path and lifecycle ID, and overlapping +children cannot own the same runtime address. If work is sequential, represent +the sequence with separate states and transition from the first outcome. Do +not depend on one child completing quickly enough for another declaration to +reuse its identity. + ### Choose state-owned or process-owned children Use `from.child(...)` when the child belongs to one state and must stop when diff --git a/packages/effect-machine/docs/machine-review.md b/packages/effect-machine/docs/machine-review.md index 1b677ea..d91d8d1 100644 --- a/packages/effect-machine/docs/machine-review.md +++ b/packages/effect-machine/docs/machine-review.md @@ -8,6 +8,14 @@ Read the [Effect Machine agent guide](./agent-guide.md) for statechart modeling and [Effect Atom and React patterns](./effect-atom-react.md) for integration patterns. +Run the recommended rules from +[`@typeonce/oxlint-plugin-effect-machine`](../../oxlint-plugin/README.md) before +the manual review. They catch provable duplicate invocation identities, +asynchronous work, browser access, nondeterminism, and redundant resolvers in +direct same-module machine definitions. Continue with this review for +cross-module ownership and architectural questions that syntax alone cannot +answer. + ## Review the responsibility boundaries Use this split when deciding where code belongs: diff --git a/packages/oxlint-plugin/README.md b/packages/oxlint-plugin/README.md index cec6edb..7222771 100644 --- a/packages/oxlint-plugin/README.md +++ b/packages/oxlint-plugin/README.md @@ -1,8 +1,8 @@ # Effect Machine Oxlint plugin `@typeonce/oxlint-plugin-effect-machine` checks Effect Machine definitions for -redundant resolvers, asynchronous planning, and one-use intermediate machine -definitions. +invalid invocation identities, impure planning, redundant resolvers, and +one-use intermediate machine definitions. The plugin uses Oxlint's JavaScript plugin interface. Custom JavaScript plugins are currently alpha in Oxlint, so keep Oxlint and this package on versions that @@ -40,16 +40,24 @@ JSON configurations can list the same rules directly: "jsPlugins": ["@typeonce/oxlint-plugin-effect-machine"], "rules": { "effect-machine/no-async-planning-callback": "error", + "effect-machine/no-browser-api-in-planning": "error", + "effect-machine/no-conflicting-invocation-identity": "error", + "effect-machine/no-nondeterministic-planning": "error", "effect-machine/no-redundant-resolve": "error", "effect-machine/prefer-inline-handle": "error" } } ``` -The rules are syntax-based. They recognize `Machine.make(...)`, direct chained -`.handle(...)` calls, and `.handle(...)` calls on definitions declared in the -same module. They do not resolve a machine definition imported from another -module. +The rules are syntax-based and deliberately conservative. They recognize +`Machine.make(...)`, direct chained `.handle(...)` calls, and `.handle(...)` +calls on definitions declared in the same module. They do not resolve an +imported machine definition or guess the result of an arbitrary function call. + +Planning checks cover initial, transition, resolution, lifecycle, choice, +initialization, output, history fallback, and invocation declaration +callbacks. A nested invocation source is state-owned work and is not treated +as planning. ## Rules @@ -72,11 +80,145 @@ const handlers = { The fixer does not run when the resolver has options, comments, construction input, or any other work. +Resolver-only reentry uses `.reenter()`: + +```ts +// Before +to.local.Ready().resolve(({ target }) => target.from(), { reenter: true }) + +// After `oxlint --fix` +to.local.Ready().reenter() +``` + +An empty `to.none.resolve(() => {})` is similarly reduced to `to.none`. + ### `effect-machine/no-async-planning-callback` -Rejects asynchronous transition, lifecycle, initial, choice, entry, exit, and -invocation-planning callbacks. A machine plans synchronously. Put asynchronous -work in state-owned `invoke` sources instead. +Rejects `async` planning callbacks and direct Promise, fetch, timer, or +scheduling operations during planning. A machine plans synchronously and has +no lifetime in which to own work started by a transition. + +```ts +// Incorrect: the transition starts unowned work. +const incorrect = { + Submit: (to) => { + fetch("/orders", { method: "POST" }) + return to.full.Complete() + } +} + +// Correct: the state owns and cancels the work. +const correct = { + Submitting: { + invoke: (from) => + from.effect("submit-order", () => submitOrder()) + .onDone((to) => to.full.Complete()) + .onFailure((to) => to.full.Failed()) + } +} +``` + +The rule only reports known unshadowed globals. Calls inside the source passed +to `from.effect`, `from.stream`, or another invocation builder remain valid. + +### `effect-machine/no-conflicting-invocation-identity` + +Requires every invocation declared by one state to have a unique lifecycle ID +and every concurrently owned logic or child process to have a unique runtime +address. + +```ts +// Incorrect: both outcomes use the "load" lifecycle ID. +const incorrect = { + invoke: (from) => [ + from.effect("load", loadAccount), + from.timer("load", "10 seconds") + ] +} + +// Correct +const correct = { + invoke: (from) => [ + from.effect("load-account", loadAccount), + from.timer("load-timeout", "10 seconds") + ] +} +``` + +Duplicate lifecycle IDs make outcome routing ambiguous. Duplicate active +addresses fail at runtime. If two operations must run sequentially, put them +in separate states and transition from the first invocation's outcome instead +of relying on completion timing. + +The rule compares only identities it can prove equal, such as literals, the +same binding, static `Machine.childAddress(...)` values, and local +`Machine.child(...)` descriptors. It checks one state's invocation declaration +at a time and does not guess whether separate states can be active together. + +### `effect-machine/no-browser-api-in-planning` + +Rejects direct access to stateful browser APIs such as `document`, +`localStorage`, `navigator`, `location`, workers, and browser event APIs during +planning. + +```ts +// Incorrect: transition planning reads ambient storage. +const incorrect = { + Restore: (to) => + localStorage.getItem("draft") === null + ? to.full.Empty() + : to.full.Editing() +} + +// Correct: state-owned work reads storage and reports an outcome. +const correct = { + Restoring: { + invoke: (from) => + from.effect("restore-draft", () => restoreDraft()) + .onDone((to) => to.full.Editing()) + .onFailure((to) => to.full.Empty()) + } +} +``` + +If browser work affects the workflow, move it into a state-owned invocation. +If it only focuses, measures, or renders UI, keep it in the UI adapter. Pure +data utilities such as `URL`, `URLSearchParams`, `TextEncoder`, and +`structuredClone` are not reported. + +### `effect-machine/no-nondeterministic-planning` + +Rejects direct reads of ambient time or randomness during planning, including +`Date.now()`, zero-argument `new Date()`, `Math.random()`, crypto randomness, +performance clocks, `Temporal.Now`, and process clocks. + +```ts +// Incorrect: the same event and snapshot can select different results. +const incorrect = { + Check: (to) => + Date.now() >= deadline + ? to.full.Expired() + : to.none +} + +// Correct: receive the external fact as part of the event protocol. +const correct = { + Check: (to) => + to.branches({ + expired: { target: to.full.Expired() }, + current: { target: to.full.Current() } + }).resolve(({ event, select }) => + event.now >= event.deadline + ? select.expired.from() + : select.current.from() + ) +} +``` + +Pass the value through machine input or an event when it is already known. If +the machine must obtain it, produce it in a state-owned invocation and +transition from the invocation outcome. Deterministic operations such as +`new Date(event.timestamp)` and `Date.parse(state.createdAt)` remain valid. ### `effect-machine/prefer-inline-handle` diff --git a/packages/oxlint-plugin/src/index.ts b/packages/oxlint-plugin/src/index.ts index 7b7007c..c359879 100644 --- a/packages/oxlint-plugin/src/index.ts +++ b/packages/oxlint-plugin/src/index.ts @@ -1,5 +1,8 @@ import type { Plugin } from "@oxlint/plugins" import { noAsyncPlanningCallback } from "./internal/rules/noAsyncPlanningCallback.js" +import { noBrowserApiInPlanning } from "./internal/rules/noBrowserApiInPlanning.js" +import { noConflictingInvocationIdentity } from "./internal/rules/noConflictingInvocationIdentity.js" +import { noNondeterministicPlanning } from "./internal/rules/noNondeterministicPlanning.js" import { noRedundantResolve } from "./internal/rules/noRedundantResolve.js" import { preferInlineHandle } from "./internal/rules/preferInlineHandle.js" @@ -9,6 +12,9 @@ const plugin = { }, rules: { "no-async-planning-callback": noAsyncPlanningCallback, + "no-browser-api-in-planning": noBrowserApiInPlanning, + "no-conflicting-invocation-identity": noConflictingInvocationIdentity, + "no-nondeterministic-planning": noNondeterministicPlanning, "no-redundant-resolve": noRedundantResolve, "prefer-inline-handle": preferInlineHandle } diff --git a/packages/oxlint-plugin/src/internal/ambient.ts b/packages/oxlint-plugin/src/internal/ambient.ts new file mode 100644 index 0000000..84079ae --- /dev/null +++ b/packages/oxlint-plugin/src/internal/ambient.ts @@ -0,0 +1,134 @@ +import type { Context, ESTree } from "@oxlint/plugins" +import { canonicalGlobalPath } from "./ast.js" + +const scheduledFunctions = new Set([ + "fetch", + "queueMicrotask", + "requestAnimationFrame", + "requestIdleCallback", + "setImmediate", + "setInterval", + "setTimeout" +]) + +const promiseMethods = new Set([ + "all", + "allSettled", + "any", + "race", + "reject", + "resolve", + "try" +]) + +export const asyncOperation = ( + context: Context, + node: ESTree.CallExpression | ESTree.NewExpression +): string | undefined => { + const path = canonicalGlobalPath(context, node.callee) + if (path === undefined) return undefined + if (node.type === "NewExpression") { + return path.length === 1 && path[0] === "Promise" ? "new Promise(...)" : undefined + } + if (path.length === 1 && scheduledFunctions.has(path[0]!)) return `${path[0]}(...)` + if (path.length === 2 && path[0] === "Promise" && promiseMethods.has(path[1]!)) { + return `Promise.${path[1]}(...)` + } + return path.length === 2 && path[0] === "process" && path[1] === "nextTick" + ? "process.nextTick(...)" + : undefined +} + +export const nondeterministicOperation = ( + context: Context, + node: ESTree.CallExpression | ESTree.NewExpression +): string | undefined => { + const path = canonicalGlobalPath(context, node.callee) + if (path === undefined) return undefined + if (node.type === "NewExpression") { + return path.length === 1 && path[0] === "Date" && node.arguments.length === 0 + ? "new Date()" + : undefined + } + if (path.length === 1 && path[0] === "Date") return "Date()" + if (path.length === 2 && path[0] === "Date" && path[1] === "now") return "Date.now()" + if (path.length === 2 && path[0] === "Math" && path[1] === "random") return "Math.random()" + if ( + path.length === 2 && + path[0] === "crypto" && + (path[1] === "getRandomValues" || path[1] === "randomUUID") + ) return `crypto.${path[1]}(...)` + if (path.length === 2 && path[0] === "performance" && path[1] === "now") { + return "performance.now()" + } + if (path.length >= 3 && path[0] === "Temporal" && path[1] === "Now") { + return `${path.join(".")}()` + } + if ( + path[0] === "process" && + (path.join(".") === "process.hrtime" || + path.join(".") === "process.hrtime.bigint" || + path.join(".") === "process.uptime") + ) return `${path.join(".")}()` + return undefined +} + +export const nondeterministicProperty = ( + context: Context, + node: ESTree.MemberExpression +): string | undefined => { + const path = canonicalGlobalPath(context, node) + return path?.length === 2 && path[0] === "performance" && path[1] === "timeOrigin" + ? "performance.timeOrigin" + : undefined +} + +const browserRoots = new Set([ + "caches", + "cookieStore", + "document", + "history", + "indexedDB", + "localStorage", + "location", + "navigator", + "screen", + "self", + "sessionStorage", + "visualViewport", + "window" +]) + +const browserFunctions = new Set([ + "addEventListener", + "alert", + "confirm", + "dispatchEvent", + "getComputedStyle", + "matchMedia", + "open", + "postMessage", + "prompt", + "removeEventListener" +]) + +const browserConstructors = new Set([ + "BroadcastChannel", + "EventSource", + "SharedWorker", + "WebSocket", + "Worker", + "XMLHttpRequest" +]) + +export const browserApi = ( + context: Context, + node: ESTree.Expression +): string | undefined => { + const path = canonicalGlobalPath(context, node) + if (path === undefined || path.length === 0) return undefined + if (browserRoots.has(path[0]!) || browserFunctions.has(path[0]!) || browserConstructors.has(path[0]!)) { + return path.join(".") + } + return undefined +} diff --git a/packages/oxlint-plugin/src/internal/ast.ts b/packages/oxlint-plugin/src/internal/ast.ts new file mode 100644 index 0000000..59a0aad --- /dev/null +++ b/packages/oxlint-plugin/src/internal/ast.ts @@ -0,0 +1,83 @@ +import type { Context, ESTree, Scope, Variable } from "@oxlint/plugins" +import { staticMemberName } from "./imports.js" + +export const unwrapExpression = (node: ESTree.Expression): ESTree.Expression => { + let current = node + while ( + current.type === "ChainExpression" || + current.type === "ParenthesizedExpression" || + current.type === "TSAsExpression" || + current.type === "TSNonNullExpression" || + current.type === "TSSatisfiesExpression" || + current.type === "TSTypeAssertion" + ) current = current.expression + return current +} + +const referenceIn = ( + scope: Scope, + node: ESTree.IdentifierReference +) => + scope.references.find((reference) => reference.identifier === node) ?? + scope.through.find((reference) => reference.identifier === node) + +export const resolvedVariable = ( + context: Context, + node: ESTree.IdentifierReference +): Variable | undefined => { + let scope: Scope | null = context.sourceCode.getScope(node) + while (scope !== null) { + const reference = referenceIn(scope, node) + if (reference !== undefined) return reference.resolved ?? undefined + scope = scope.upper + } + return undefined +} + +export const isUnshadowedGlobal = ( + context: Context, + node: ESTree.Expression, + name: string +): node is ESTree.IdentifierReference => { + const expression = unwrapExpression(node) + if (expression.type !== "Identifier" || expression.name !== name) return false + if (context.sourceCode.isGlobalReference(expression)) return true + + let scope: Scope | null = context.sourceCode.getScope(expression) + while (scope !== null) { + const reference = referenceIn(scope, expression) + if (reference !== undefined) return reference.resolved === null + scope = scope.upper + } + return false +} + +export const globalPath = ( + context: Context, + node: ESTree.Expression +): ReadonlyArray | undefined => { + const expression = unwrapExpression(node) + if (expression.type === "Identifier") { + return isUnshadowedGlobal(context, expression, expression.name) + ? [expression.name] + : undefined + } + if (expression.type !== "MemberExpression") return undefined + + const member = staticMemberName(expression) + if (member === undefined) return undefined + const owner = globalPath(context, expression.object) + return owner === undefined ? undefined : [...owner, member] +} + +const ambientQualifiers = new Set(["globalThis", "self", "window"]) + +export const canonicalGlobalPath = ( + context: Context, + node: ESTree.Expression +): ReadonlyArray | undefined => { + const path = globalPath(context, node) + return path !== undefined && path.length > 1 && ambientQualifiers.has(path[0]!) + ? path.slice(1) + : path +} diff --git a/packages/oxlint-plugin/src/internal/imports.ts b/packages/oxlint-plugin/src/internal/imports.ts index bdd94dd..5b0cdeb 100644 --- a/packages/oxlint-plugin/src/internal/imports.ts +++ b/packages/oxlint-plugin/src/internal/imports.ts @@ -37,12 +37,16 @@ export const recordMachineImport = ( export const hasMachineImport = (bindings: MachineBindings): boolean => bindings.machine.size > 0 || bindings.namespaces.size > 0 -export const staticMemberName = (node: ESTree.Node): string | undefined => - node.type === "MemberExpression" && - !node.computed && - node.property.type === "Identifier" +export const staticMemberName = (node: ESTree.Node): string | undefined => { + if (node.type !== "MemberExpression") return undefined + return node.computed + ? node.property.type === "Literal" && typeof node.property.value === "string" + ? node.property.value + : undefined + : node.property.type === "Identifier" ? node.property.name : undefined +} const isNamespaceMachine = ( node: ESTree.Node, @@ -55,6 +59,24 @@ const isNamespaceMachine = ( node.property.type === "Identifier" && node.property.name === "Machine" +const isMachineReference = ( + node: ESTree.Node, + bindings: MachineBindings +): boolean => + node.type === "Identifier" + ? bindings.machine.has(node.name) + : isNamespaceMachine(node, bindings) + +export const isMachineMemberCall = ( + node: ESTree.Node, + member: string, + bindings: MachineBindings +): node is ESTree.CallExpression => + node.type === "CallExpression" && + node.callee.type === "MemberExpression" && + staticMemberName(node.callee) === member && + isMachineReference(node.callee.object, bindings) + export const isMachineMakeCall = ( node: ESTree.CallExpression, bindings: MachineBindings @@ -64,10 +86,7 @@ export const isMachineMakeCall = ( staticMemberName(node.callee) !== "make" ) return false - const receiver = node.callee.object - return receiver.type === "Identifier" - ? bindings.machine.has(receiver.name) - : isNamespaceMachine(receiver, bindings) + return isMachineReference(node.callee.object, bindings) } export const recordMachineDefinition = ( diff --git a/packages/oxlint-plugin/src/internal/planning.ts b/packages/oxlint-plugin/src/internal/planning.ts index 09de350..191b181 100644 --- a/packages/oxlint-plugin/src/internal/planning.ts +++ b/packages/oxlint-plugin/src/internal/planning.ts @@ -1,4 +1,5 @@ import type { ESTree } from "@oxlint/plugins" +import { unwrapExpression } from "./ast.js" import { isMachineHandleCall, isMachineMakeCall, type MachineBindings, staticMemberName } from "./imports.js" export type PlanningFunction = ESTree.ArrowFunctionExpression | ESTree.Function @@ -16,7 +17,10 @@ const statePlanningProperties = new Set([ "choice", "entry", "exit", - "invoke" + "initialize", + "invoke", + "onDone", + "output" ]) const propertyName = (node: ESTree.Node): string | undefined => { @@ -74,6 +78,26 @@ const isEventHandlerProperty = ( isStateConfig(onProperty.parent, bindings) } +const isHistoryDefaultProperty = ( + node: ESTree.Node, + bindings: MachineBindings +): boolean => { + if (node.type !== "Property" || propertyName(node) !== "default") return false + const historyEntry = node.parent + if ( + historyEntry.type !== "ObjectExpression" || + historyEntry.parent.type !== "Property" || + historyEntry.parent.parent.type !== "ObjectExpression" + ) return false + const historyEntries = historyEntry.parent.parent + if ( + historyEntries.parent.type !== "Property" || + propertyName(historyEntries.parent) !== "history" || + historyEntries.parent.parent.type !== "ObjectExpression" + ) return false + return isStateConfig(historyEntries.parent.parent, bindings) +} + const isPropertyPlanningCallback = ( node: PlanningFunction, bindings: MachineBindings @@ -89,10 +113,11 @@ const isPropertyPlanningCallback = ( return name === "initial" ? isMachineMakeConfig(property.parent, bindings) : (name !== undefined && statePlanningProperties.has(name) && isStateConfig(property.parent, bindings)) || - isEventHandlerProperty(property, bindings) + isEventHandlerProperty(property, bindings) || + isHistoryDefaultProperty(property, bindings) } -const enclosingFunction = (node: ESTree.Node): PlanningFunction | undefined => { +export const enclosingFunction = (node: ESTree.Node): PlanningFunction | undefined => { let current: ESTree.Node | null = node.parent while (current !== null && current.type !== "Program") { if ( @@ -104,6 +129,40 @@ const enclosingFunction = (node: ESTree.Node): PlanningFunction | undefined => { return undefined } +const selectorRoot = (node: ESTree.Expression): string | undefined => { + let expression = unwrapExpression(node) + while (expression.type === "CallExpression" || expression.type === "MemberExpression") { + expression = unwrapExpression( + expression.type === "CallExpression" + ? expression.callee + : expression.object + ) + } + return expression.type === "Identifier" ? expression.name : undefined +} + +export const enclosingPlanningCallback = ( + node: ESTree.Node, + bindings: MachineBindings +): PlanningFunction | undefined => { + const callback = enclosingFunction(node) + return callback !== undefined && isPlanningCallback(callback, bindings) + ? callback + : undefined +} + +export const isInvokePlanningCallback = ( + node: PlanningFunction, + bindings: MachineBindings +): boolean => { + const property = node.parent + return property.type === "Property" && + property.value === node && + propertyName(property) === "invoke" && + property.parent.type === "ObjectExpression" && + isStateConfig(property.parent, bindings) +} + export const isPlanningCallback = ( node: PlanningFunction, bindings: MachineBindings @@ -112,12 +171,15 @@ export const isPlanningCallback = ( const parent = node.parent if (parent.type === "CallExpression" && parent.arguments.includes(node)) { - const method = parent.callee.type === "MemberExpression" - ? staticMemberName(parent.callee) - : undefined + if (parent.callee.type !== "MemberExpression") return false + const method = staticMemberName(parent.callee) if (method !== undefined && directPlanningMethods.has(method)) { const owner = enclosingFunction(parent) - return owner !== undefined && isPlanningCallback(owner, bindings) + const selector = owner?.params[0] + return owner !== undefined && + selector?.type === "Identifier" && + selectorRoot(parent.callee.object) === selector.name && + isPlanningCallback(owner, bindings) } } return false diff --git a/packages/oxlint-plugin/src/internal/rules/noAsyncPlanningCallback.ts b/packages/oxlint-plugin/src/internal/rules/noAsyncPlanningCallback.ts index 1b6c061..2cb5733 100644 --- a/packages/oxlint-plugin/src/internal/rules/noAsyncPlanningCallback.ts +++ b/packages/oxlint-plugin/src/internal/rules/noAsyncPlanningCallback.ts @@ -1,6 +1,7 @@ -import type { Rule } from "@oxlint/plugins" +import type { ESTree, Rule } from "@oxlint/plugins" +import { asyncOperation } from "../ambient.js" import { hasMachineImport, makeMachineBindings, recordMachineDefinition, recordMachineImport } from "../imports.js" -import { isPlanningCallback, type PlanningFunction } from "../planning.js" +import { enclosingPlanningCallback, isPlanningCallback, type PlanningFunction } from "../planning.js" export const noAsyncPlanningCallback: Rule = { meta: { @@ -11,7 +12,10 @@ export const noAsyncPlanningCallback: Rule = { }, schema: [], messages: { - asyncPlanning: "Planning callbacks must be synchronous. Move asynchronous work into state-owned invocation." + asyncOperation: + "{{operation}} starts asynchronous work during synchronous planning. Move it into the owning state's invoke declaration using from.effect(...), from.stream(...), or from.timer(...), then handle completion or failure with onDone/onFailure.", + asyncPlanning: + "Planning callbacks must be synchronous. Remove async and move the asynchronous work into the owning state's invoke declaration, then transition from onDone/onFailure." } }, create(context) { @@ -25,11 +29,24 @@ export const noAsyncPlanningCallback: Rule = { context.report({ node, messageId: "asyncPlanning" }) } } + const inspectOperation = ( + node: ESTree.CallExpression | ESTree.NewExpression + ): void => { + if (!hasMachineImport(bindings)) return + const callback = enclosingPlanningCallback(node, bindings) + if (callback === undefined || callback.async) return + const operation = asyncOperation(context, node) + if (operation !== undefined) { + context.report({ node, messageId: "asyncOperation", data: { operation } }) + } + } return { ImportDeclaration: (node) => recordMachineImport(bindings, node), VariableDeclarator: (node) => recordMachineDefinition(bindings, node), ArrowFunctionExpression: inspect, - FunctionExpression: inspect + FunctionExpression: inspect, + CallExpression: inspectOperation, + NewExpression: inspectOperation } } } diff --git a/packages/oxlint-plugin/src/internal/rules/noBrowserApiInPlanning.ts b/packages/oxlint-plugin/src/internal/rules/noBrowserApiInPlanning.ts new file mode 100644 index 0000000..032c27e --- /dev/null +++ b/packages/oxlint-plugin/src/internal/rules/noBrowserApiInPlanning.ts @@ -0,0 +1,57 @@ +import type { ESTree, Rule } from "@oxlint/plugins" +import { browserApi } from "../ambient.js" +import { hasMachineImport, makeMachineBindings, recordMachineDefinition, recordMachineImport } from "../imports.js" +import { enclosingPlanningCallback } from "../planning.js" + +const isOutermostAccess = (node: ESTree.Expression): boolean => { + const parent = node.parent + if (parent.type === "MemberExpression" && parent.object === node) return false + if ( + (parent.type === "CallExpression" || parent.type === "NewExpression") && + parent.callee === node + ) return false + return true +} + +export const noBrowserApiInPlanning: Rule = { + meta: { + type: "problem", + docs: { + description: "Keep browser state and side effects out of Effect Machine planning.", + recommended: true + }, + schema: [], + messages: { + browserApi: + "{{api}} reads browser state or performs a browser side effect during synchronous planning. If it affects the workflow, run it in a state-owned invocation and transition from its outcome. If it only affects presentation, keep it in the UI adapter." + } + }, + create(context) { + const bindings = makeMachineBindings() + const inspect = (node: ESTree.Expression): void => { + if ( + !hasMachineImport(bindings) || + !isOutermostAccess(node) || + enclosingPlanningCallback(node, bindings) === undefined + ) return + const api = browserApi(context, node) + if (api !== undefined) context.report({ node, messageId: "browserApi", data: { api } }) + } + const inspectOperation = (node: ESTree.CallExpression | ESTree.NewExpression): void => { + if ( + !hasMachineImport(bindings) || + enclosingPlanningCallback(node, bindings) === undefined + ) return + const api = browserApi(context, node.callee) + if (api !== undefined) context.report({ node, messageId: "browserApi", data: { api } }) + } + return { + ImportDeclaration: (node) => recordMachineImport(bindings, node), + Identifier: (node) => inspect(node as ESTree.IdentifierReference), + MemberExpression: inspect, + CallExpression: inspectOperation, + NewExpression: inspectOperation, + VariableDeclarator: (node) => recordMachineDefinition(bindings, node) + } + } +} diff --git a/packages/oxlint-plugin/src/internal/rules/noConflictingInvocationIdentity.ts b/packages/oxlint-plugin/src/internal/rules/noConflictingInvocationIdentity.ts new file mode 100644 index 0000000..3f662f5 --- /dev/null +++ b/packages/oxlint-plugin/src/internal/rules/noConflictingInvocationIdentity.ts @@ -0,0 +1,343 @@ +import type { Context, ESTree, Rule, Variable } from "@oxlint/plugins" +import { resolvedVariable, unwrapExpression } from "../ast.js" +import { + hasMachineImport, + isMachineMemberCall, + type MachineBindings, + makeMachineBindings, + recordMachineDefinition, + recordMachineImport, + staticMemberName +} from "../imports.js" +import { isInvokePlanningCallback, type PlanningFunction } from "../planning.js" + +interface Identity { + readonly key: string + readonly label: string + readonly node: ESTree.Expression +} + +interface InvocationIdentity { + readonly address?: Identity + readonly lifecycle: Identity +} + +const completionMethods = new Set(["onDone", "onElement", "onFailure", "onSnapshot"]) +const sourceMethods = new Set(["effect", "logic", "stream", "timer"]) + +const variableKey = (variable: Variable): string | undefined => { + const identifier = variable.identifiers[0] + return identifier === undefined ? undefined : `binding:${identifier.range[0]}:${identifier.range[1]}` +} + +const isStableVariable = (variable: Variable): boolean => + variable.defs.some((definition) => + definition.type === "ImportBinding" || + (definition.type === "Variable" && + definition.node.type === "VariableDeclarator" && + definition.node.parent.type === "VariableDeclaration" && + definition.node.parent.kind === "const") + ) + +const constInitializer = ( + variable: Variable +): ESTree.Expression | undefined => { + const definition = variable.defs.find((candidate) => candidate.type === "Variable") + const declaration = definition?.node + if ( + declaration?.type !== "VariableDeclarator" || + declaration.parent.type !== "VariableDeclaration" || + declaration.parent.kind !== "const" || + declaration.init === null + ) return undefined + return declaration.init +} + +const staticIdentity = ( + context: Context, + node: ESTree.Expression, + bindings: MachineBindings, + seen: Set = new Set() +): Identity | undefined => { + const expression = unwrapExpression(node) + if ( + expression.type === "Literal" && + (typeof expression.value === "string" || typeof expression.value === "number") + ) { + return { + key: `value:${String(expression.value)}`, + label: JSON.stringify(expression.value), + node: expression + } + } + if (expression.type === "TemplateLiteral" && expression.expressions.length === 0) { + const value = expression.quasis[0]?.value.cooked + return value === null || value === undefined + ? undefined + : { key: `value:${value}`, label: JSON.stringify(value), node: expression } + } + if (expression.type === "Identifier") { + const variable = resolvedVariable(context, expression) + if (variable === undefined || !isStableVariable(variable)) return undefined + const key = variableKey(variable) + if (key === undefined || seen.has(key)) return undefined + const initializer = constInitializer(variable) + if (initializer !== undefined) { + const nextSeen = new Set(seen) + nextSeen.add(key) + const initialized = staticIdentity(context, initializer, bindings, nextSeen) + if (initialized !== undefined) return { ...initialized, node: expression } + } + return { key, label: expression.name, node: expression } + } + if ( + isMachineMemberCall(expression, "childAddress", bindings) + ) { + const argument = expression.arguments[0] + if (expression.arguments.length === 1 && argument !== undefined && argument.type !== "SpreadElement") { + return staticIdentity(context, argument, bindings, seen) + } + } + return undefined +} + +const descriptorIdentity = ( + context: Context, + node: ESTree.Expression, + bindings: MachineBindings, + seen: Set = new Set() +): Identity | undefined => { + const expression = unwrapExpression(node) + if (expression.type === "Identifier") { + const variable = resolvedVariable(context, expression) + if (variable === undefined || !isStableVariable(variable)) return undefined + const key = variableKey(variable) + if (key === undefined || seen.has(key)) return undefined + const initializer = constInitializer(variable) + if (initializer !== undefined) { + const nextSeen = new Set(seen) + nextSeen.add(key) + const initialized = descriptorIdentity(context, initializer, bindings, nextSeen) + if (initialized !== undefined) return { ...initialized, node: expression } + } + return { key: `descriptor:${key}`, label: expression.name, node: expression } + } + if ( + isMachineMemberCall(expression, "child", bindings) + ) { + const argument = expression.arguments[0] + if (argument !== undefined && argument.type !== "SpreadElement") { + return staticIdentity(context, argument, bindings) + } + } + return undefined +} + +const objectProperty = ( + object: ESTree.ObjectExpression, + name: string +): ESTree.Expression | undefined => { + for (const property of object.properties) { + if (property.type !== "Property" || property.kind !== "init") continue + const propertyName = property.computed + ? property.key.type === "Literal" && typeof property.key.value === "string" + ? property.key.value + : undefined + : property.key.type === "Identifier" || property.key.type === "Literal" + ? String(property.key.type === "Identifier" ? property.key.name : property.key.value) + : undefined + if (propertyName === name) return property.value + } + return undefined +} + +const resolvedObject = ( + context: Context, + node: ESTree.Expression, + seen: Set = new Set() +): ESTree.ObjectExpression | undefined => { + const expression = unwrapExpression(node) + if (expression.type === "ObjectExpression") return expression + if (expression.type !== "Identifier") return undefined + const variable = resolvedVariable(context, expression) + if (variable === undefined) return undefined + const key = variableKey(variable) + if (key === undefined || seen.has(key)) return undefined + const initializer = constInitializer(variable) + if (initializer === undefined) return undefined + const nextSeen = new Set(seen) + nextSeen.add(key) + return resolvedObject(context, initializer, nextSeen) +} + +const sourceCall = ( + node: ESTree.Expression +): ESTree.CallExpression | undefined => { + let expression = unwrapExpression(node) + while ( + expression.type === "CallExpression" && + expression.callee.type === "MemberExpression" && + completionMethods.has(staticMemberName(expression.callee) ?? "") + ) expression = unwrapExpression(expression.callee.object) + return expression.type === "CallExpression" ? expression : undefined +} + +const invocationIdentity = ( + context: Context, + node: ESTree.Expression, + from: string, + bindings: MachineBindings +): InvocationIdentity | undefined => { + const call = sourceCall(node) + if ( + call?.callee.type !== "MemberExpression" || + call.callee.object.type !== "Identifier" || + call.callee.object.name !== from + ) return undefined + const method = staticMemberName(call.callee) + if (method === "child") { + const child = call.arguments[0] + if (child === undefined || child.type === "SpreadElement") return undefined + const identity = descriptorIdentity(context, child, bindings) + return identity === undefined ? undefined : { lifecycle: identity, address: identity } + } + if (method === undefined || !sourceMethods.has(method)) return undefined + const id = call.arguments[0] + if (id === undefined || id.type === "SpreadElement") return undefined + const lifecycle = staticIdentity(context, id, bindings) + if (lifecycle === undefined) return undefined + if (method !== "logic") return { lifecycle } + const options = call.arguments[1] + if (options === undefined || options.type === "SpreadElement") return { lifecycle } + const object = resolvedObject(context, options) + const addressNode = object === undefined ? undefined : objectProperty(object, "address") + const address = addressNode === undefined + ? undefined + : staticIdentity(context, addressNode, bindings) + return address === undefined ? { lifecycle } : { lifecycle, address } +} + +const returnedExpression = ( + node: PlanningFunction +): ESTree.Expression | undefined => { + if (node.body === null) return undefined + if (node.body.type !== "BlockStatement") return node.body + const returns = node.body.body.filter((statement) => statement.type === "ReturnStatement") + if (returns.length !== 1) return undefined + return returns[0]!.argument ?? undefined +} + +const returnedEntries = ( + context: Context, + node: PlanningFunction +): ReadonlyArray => { + const returned = returnedExpression(node) + if (returned === undefined) return [] + const array = resolvedObjectOrArray(context, returned) + if (array?.type !== "ArrayExpression") return [returned] + return array.elements.filter((entry): entry is ESTree.Expression => entry !== null && entry.type !== "SpreadElement") +} + +const resolvedObjectOrArray = ( + context: Context, + node: ESTree.Expression, + seen: Set = new Set() +): ESTree.ObjectExpression | ESTree.ArrayExpression | undefined => { + const expression = unwrapExpression(node) + if (expression.type === "ObjectExpression" || expression.type === "ArrayExpression") return expression + if (expression.type !== "Identifier") return undefined + const variable = resolvedVariable(context, expression) + if (variable === undefined) return undefined + const key = variableKey(variable) + if (key === undefined || seen.has(key)) return undefined + const initializer = constInitializer(variable) + if (initializer === undefined) return undefined + const nextSeen = new Set(seen) + nextSeen.add(key) + return resolvedObjectOrArray(context, initializer, nextSeen) +} + +const resolvedEntry = ( + context: Context, + node: ESTree.Expression +): ESTree.Expression => { + const expression = unwrapExpression(node) + if (expression.type !== "Identifier") return expression + const variable = resolvedVariable(context, expression) + const initializer = variable === undefined ? undefined : constInitializer(variable) + return initializer === undefined ? expression : unwrapExpression(initializer) +} + +export const noConflictingInvocationIdentity: Rule = { + meta: { + type: "problem", + docs: { + description: "Require unique invocation lifecycle IDs and runtime addresses within a state.", + recommended: true + }, + schema: [], + messages: { + conflictingAddress: + "Invocation runtime address {{identity}} is reused in this state. Concurrent children cannot own the same address. Give each from.logic(...) invocation a distinct Machine.childAddress(...), or put sequential work in separate states.", + conflictingBoth: + "Invocation identity {{identity}} is reused as both lifecycle ID and runtime address in this state. Outcomes become ambiguous and overlapping starts fail. Give each invocation a unique ID/address, or put sequential work in separate states.", + conflictingLifecycle: + "Invocation lifecycle ID {{identity}} is reused in this state. Outcomes are routed by state path and ID, so duplicate IDs are ambiguous and overlapping starts fail. Give each invocation a unique ID, or put sequential work in separate states." + } + }, + create(context) { + const bindings = makeMachineBindings() + const inspect = (node: PlanningFunction): void => { + if (!hasMachineImport(bindings) || !isInvokePlanningCallback(node, bindings)) return + const parameter = node.params[0] + if (parameter?.type !== "Identifier") return + const lifecycle = new Map() + const addresses = new Map() + const entries = returnedEntries(context, node) + entries.forEach((entry, index) => { + const identity = invocationIdentity(context, resolvedEntry(context, entry), parameter.name, bindings) + if (identity === undefined) return + const lifecycleConflict = lifecycle.get(identity.lifecycle.key) + const addressConflict = identity.address === undefined + ? undefined + : addresses.get(identity.address.key) + if ( + lifecycleConflict !== undefined && + addressConflict !== undefined && + lifecycleConflict === addressConflict + ) { + context.report({ + node: identity.lifecycle.node, + messageId: "conflictingBoth", + data: { identity: identity.lifecycle.label } + }) + } else { + if (lifecycleConflict !== undefined) { + context.report({ + node: identity.lifecycle.node, + messageId: "conflictingLifecycle", + data: { identity: identity.lifecycle.label } + }) + } + if (addressConflict !== undefined && identity.address !== undefined) { + context.report({ + node: identity.address.node, + messageId: "conflictingAddress", + data: { identity: identity.address.label } + }) + } + } + if (!lifecycle.has(identity.lifecycle.key)) lifecycle.set(identity.lifecycle.key, index) + if (identity.address !== undefined && !addresses.has(identity.address.key)) { + addresses.set(identity.address.key, index) + } + }) + } + return { + ImportDeclaration: (node) => recordMachineImport(bindings, node), + VariableDeclarator: (node) => recordMachineDefinition(bindings, node), + ArrowFunctionExpression: inspect, + FunctionExpression: inspect + } + } +} diff --git a/packages/oxlint-plugin/src/internal/rules/noNondeterministicPlanning.ts b/packages/oxlint-plugin/src/internal/rules/noNondeterministicPlanning.ts new file mode 100644 index 0000000..6fc249f --- /dev/null +++ b/packages/oxlint-plugin/src/internal/rules/noNondeterministicPlanning.ts @@ -0,0 +1,39 @@ +import type { ESTree, Rule } from "@oxlint/plugins" +import { nondeterministicOperation, nondeterministicProperty } from "../ambient.js" +import { hasMachineImport, makeMachineBindings, recordMachineDefinition, recordMachineImport } from "../imports.js" +import { enclosingPlanningCallback } from "../planning.js" + +export const noNondeterministicPlanning: Rule = { + meta: { + type: "problem", + docs: { + description: "Keep ambient time and randomness out of Effect Machine planning.", + recommended: true + }, + schema: [], + messages: { + nondeterministic: + "{{operation}} produces a different result without a machine event or state change. Pass the value through machine input or an event, or produce it in a state-owned invocation and transition from its outcome." + } + }, + create(context) { + const bindings = makeMachineBindings() + const report = ( + node: ESTree.CallExpression | ESTree.MemberExpression | ESTree.NewExpression, + operation: string | undefined + ): void => { + if ( + operation !== undefined && + hasMachineImport(bindings) && + enclosingPlanningCallback(node, bindings) !== undefined + ) context.report({ node, messageId: "nondeterministic", data: { operation } }) + } + return { + ImportDeclaration: (node) => recordMachineImport(bindings, node), + CallExpression: (node) => report(node, nondeterministicOperation(context, node)), + MemberExpression: (node) => report(node, nondeterministicProperty(context, node)), + NewExpression: (node) => report(node, nondeterministicOperation(context, node)), + VariableDeclarator: (node) => recordMachineDefinition(bindings, node) + } + } +} diff --git a/packages/oxlint-plugin/src/internal/rules/noRedundantResolve.ts b/packages/oxlint-plugin/src/internal/rules/noRedundantResolve.ts index 2cbb589..b752f01 100644 --- a/packages/oxlint-plugin/src/internal/rules/noRedundantResolve.ts +++ b/packages/oxlint-plugin/src/internal/rules/noRedundantResolve.ts @@ -49,6 +49,24 @@ const isDefaultTargetConstruction = ( node.callee.object.type === "Identifier" && node.callee.object.name === binding +const isEmptyResolver = ( + node: ESTree.ArrowFunctionExpression | ESTree.Function +): boolean => node.body?.type === "BlockStatement" && node.body.body.length === 0 + +const isTargetlessReceiver = (node: ESTree.Expression): boolean => + node.type === "MemberExpression" && staticMemberName(node) === "none" + +const isReenterOnlyOptions = (node: ESTree.Expression | undefined): boolean => { + if (node?.type !== "ObjectExpression" || node.properties.length !== 1) return false + const property = node.properties[0] + return property?.type === "Property" && + !property.computed && + property.key.type === "Identifier" && + property.key.name === "reenter" && + property.value.type === "Literal" && + property.value.value === true +} + export const noRedundantResolve: Rule = { meta: { type: "suggestion", @@ -59,7 +77,12 @@ export const noRedundantResolve: Rule = { fixable: "code", schema: [], messages: { - redundantResolver: "Remove this resolver. The selected target already applies default construction." + redundantResolver: + "Remove this resolver. The selected target already applies default construction, so use the target selector directly.", + redundantReenterResolver: + "Replace this resolver with .reenter(). It applies the same default construction while explicitly reentering the selected state.", + redundantTargetlessResolver: + "Remove this empty resolver. A targetless transition performs the same work as to.none; use to.none directly." } }, create(context) { @@ -69,7 +92,7 @@ export const noRedundantResolve: Rule = { CallExpression(node) { if ( !hasMachineImport(bindings) || - node.arguments.length !== 1 || + (node.arguments.length !== 1 && node.arguments.length !== 2) || node.callee.type !== "MemberExpression" || staticMemberName(node.callee) !== "resolve" ) return @@ -82,22 +105,31 @@ export const noRedundantResolve: Rule = { if (callback.async || callback.generator) return if (!isPlanningCallback(callback, bindings)) return + const receiver = node.callee.object const binding = targetBinding(callback) - if ( - binding === undefined || - !isDefaultTargetConstruction(returnedExpression(callback), binding) - ) return + const defaultConstruction = binding !== undefined && + isDefaultTargetConstruction(returnedExpression(callback), binding) + const targetless = isTargetlessReceiver(receiver) && isEmptyResolver(callback) + if (!defaultConstruction && !targetless) return - const receiver = node.callee.object - if (context.sourceCode.getCommentsInside(node).length === 0) { - context.report({ - node, - messageId: "redundantResolver", - fix: (fixer) => fixer.replaceText(node, context.sourceCode.getText(receiver)) - }) - } else { - context.report({ node, messageId: "redundantResolver" }) - } + const options = node.arguments[1] + if (options?.type === "SpreadElement") return + const reenter = options === undefined ? false : isReenterOnlyOptions(options) + if (options !== undefined && !reenter) return + + const messageId = reenter + ? "redundantReenterResolver" + : targetless + ? "redundantTargetlessResolver" + : "redundantResolver" + const replacement = `${context.sourceCode.getText(receiver)}${reenter ? ".reenter()" : ""}` + context.report({ + node, + messageId, + ...(context.sourceCode.getCommentsInside(node).length === 0 + ? { fix: (fixer) => fixer.replaceText(node, replacement) } + : undefined) + }) }, VariableDeclarator: (node) => recordMachineDefinition(bindings, node) } diff --git a/packages/oxlint-plugin/src/recommended.ts b/packages/oxlint-plugin/src/recommended.ts index 8605c65..f4e4700 100644 --- a/packages/oxlint-plugin/src/recommended.ts +++ b/packages/oxlint-plugin/src/recommended.ts @@ -1,6 +1,9 @@ /** Rules recommended for every Effect Machine model. */ export const recommended = { "effect-machine/no-async-planning-callback": "error", + "effect-machine/no-browser-api-in-planning": "error", + "effect-machine/no-conflicting-invocation-identity": "error", + "effect-machine/no-nondeterministic-planning": "error", "effect-machine/no-redundant-resolve": "error", "effect-machine/prefer-inline-handle": "error" } as const diff --git a/packages/oxlint-plugin/test/noAsyncPlanningCallback.test.ts b/packages/oxlint-plugin/test/noAsyncPlanningCallback.test.ts index 34789aa..ba05b6e 100644 --- a/packages/oxlint-plugin/test/noAsyncPlanningCallback.test.ts +++ b/packages/oxlint-plugin/test/noAsyncPlanningCallback.test.ts @@ -23,7 +23,26 @@ Machine.make({ initial: (to) => to.Ready(), metadata: { initial: async () => und `import { Machine } from "@typeonce/effect-machine" const other = { handle: (_config: unknown) => undefined } other.handle({ Ready: { entry: async () => undefined } })`, - `const machine = { initial: async () => undefined }` + `const machine = { initial: async () => undefined }`, + `import { Machine } from "@typeonce/effect-machine" +Machine.make({ initial: (to) => to.Ready() }).handle({ Ready: { invoke: (from) => [ + from.effect("fetch", () => fetch("/api")), + from.timer("delay", () => setTimeout(() => undefined, 1)) +] } })`, + `import { Machine } from "@typeonce/effect-machine" +Machine.make({ initial: (to) => to.Ready() }).handle({ Ready: { entry: ({ fetch, setTimeout }) => { + fetch() + setTimeout() +} } })`, + `import { Machine } from "@typeonce/effect-machine" +Machine.make({ initial: (to) => to.Ready() }).handle({ Ready: { entry: () => { + const later = () => Promise.resolve() + return later +} } })`, + `import { Machine } from "@typeonce/effect-machine" +Machine.make({ initial: (to) => to.Ready() }).handle({ Ready: { entry: () => + other.resolve(async () => fetch("/helper")) +} })` ], invalid: [ { @@ -62,6 +81,49 @@ Machine.make({ initial: (to) => to.Ready() }).handle({ Ready: { invoke: async (from) => from.timer("tick", 1) } })`, errors: Array.from({ length: 5 }, () => ({ messageId: "asyncPlanning" })) + }, + { + code: `import { Machine } from "@typeonce/effect-machine" +Machine.make({ initial: (to) => { + fetch("/initial") + new Promise(() => undefined) + Promise.all([]) + setTimeout(() => undefined, 1) + queueMicrotask(() => undefined) + process.nextTick(() => undefined) + return to.Ready() +} })`, + errors: Array.from({ length: 6 }, () => ({ messageId: "asyncOperation" })) + }, + { + code: `import { Machine } from "@typeonce/effect-machine" +Machine.make({ initial: (to) => to.Ready() }).handle({ Ready: { + initialize: ({ builder }) => { + globalThis["fetch"]("/initialize") + return builder.from() + }, + output: ({ state }) => { + window.setInterval(() => undefined, 100) + return state + }, + onDone: (to) => { + self.requestAnimationFrame(() => undefined) + return to.none + }, + history: { recent: { default: (to) => { + requestIdleCallback(() => undefined) + return to.none + } } } +} })`, + errors: Array.from({ length: 4 }, () => ({ messageId: "asyncOperation" })) + }, + { + code: `import { Machine } from "@typeonce/effect-machine" +Machine.make({ initial: async (to) => { + await fetch("/initial") + return to.Ready() +} })`, + errors: [{ messageId: "asyncPlanning" }] } ] }) diff --git a/packages/oxlint-plugin/test/noBrowserApiInPlanning.test.ts b/packages/oxlint-plugin/test/noBrowserApiInPlanning.test.ts new file mode 100644 index 0000000..124701c --- /dev/null +++ b/packages/oxlint-plugin/test/noBrowserApiInPlanning.test.ts @@ -0,0 +1,57 @@ +import { RuleTester } from "oxlint/plugins-dev" +import { describe, it } from "vitest" +import plugin from "../src/index.js" + +RuleTester.describe = describe +RuleTester.it = it + +const tester = new RuleTester({ + languageOptions: { parserOptions: { lang: "ts" } } +}) + +const rule = plugin.rules["no-browser-api-in-planning"] + +tester.run("no-browser-api-in-planning", rule, { + valid: [ + `import { Machine } from "@typeonce/effect-machine" +Machine.make({ initial: (to) => to.Ready() }).handle({ Ready: { invoke: (from) => from.effect("storage", () => localStorage.getItem("key")) } })`, + `import { Machine } from "@typeonce/effect-machine" +Machine.make({ initial: (to) => to.Ready() }).handle({ Ready: { entry: ({ document, navigator }) => document.read(navigator) } })`, + `import { Machine } from "@typeonce/effect-machine" +Machine.make({ initial: (to) => to.Ready() }).handle({ Ready: { entry: () => { + const url = new URL("https://example.com") + const params = new URLSearchParams(url.search) + return structuredClone(params) +} } })`, + `import { Machine } from "@typeonce/effect-machine" +Machine.make({ initial: (to) => { + window.fetch("/api") + window.crypto.randomUUID() + return to.Ready() +} })` + ], + invalid: [ + { + code: `import { Machine } from "@typeonce/effect-machine" +Machine.make({ initial: (to) => { + document.querySelector("main") + localStorage.getItem("key") + globalThis.navigator.onLine + matchMedia("(dark-mode)") + new WebSocket("wss://example.com") + void window + void self + return to.Ready() +} })`, + errors: Array.from({ length: 7 }, () => ({ messageId: "browserApi" })) + }, + { + code: `import { Machine } from "@typeonce/effect-machine" +Machine.make({ initial: (to) => to.Ready() }).handle({ Ready: { + output: () => sessionStorage.length, + history: { recent: { default: (to) => location.pathname ? to.none : to.none } } +} })`, + errors: Array.from({ length: 2 }, () => ({ messageId: "browserApi" })) + } + ] +}) diff --git a/packages/oxlint-plugin/test/noConflictingInvocationIdentity.test.ts b/packages/oxlint-plugin/test/noConflictingInvocationIdentity.test.ts new file mode 100644 index 0000000..6fd8b33 --- /dev/null +++ b/packages/oxlint-plugin/test/noConflictingInvocationIdentity.test.ts @@ -0,0 +1,93 @@ +import { RuleTester } from "oxlint/plugins-dev" +import { describe, it } from "vitest" +import plugin from "../src/index.js" + +RuleTester.describe = describe +RuleTester.it = it + +const tester = new RuleTester({ + languageOptions: { parserOptions: { lang: "ts" } } +}) + +const rule = plugin.rules["no-conflicting-invocation-identity"] + +tester.run("no-conflicting-invocation-identity", rule, { + valid: [ + `import { Machine } from "@typeonce/effect-machine" +Machine.make({ initial: (to) => to.Ready() }).handle({ Ready: { invoke: (from) => [ + from.effect("load", load), + from.timer("timeout", 1000) +] } })`, + `import { Machine } from "@typeonce/effect-machine" +Machine.make({ initial: (to) => to.One() }).handle({ + One: { invoke: (from) => from.effect("load", load) }, + Two: { invoke: (from) => from.effect("load", load) } +})`, + `import { Machine } from "@typeonce/effect-machine" +Machine.make({ initial: (to) => to.Ready() }).handle({ Ready: { invoke: (from) => [ + from.effect(makeId(), load), + from.effect(makeId(), load) +] } })`, + `import { Machine } from "@typeonce/effect-machine" +Machine.make({ initial: (to) => to.Ready() }).handle({ Ready: { invoke: (from) => { + let id = "first" + const first = from.effect(id, load) + id = "second" + const second = from.effect(id, load) + return [first, second] +} } })`, + `import { Machine } from "@typeonce/effect-machine" +Machine.make({ initial: (to) => to.Ready() }).handle({ Ready: { invoke: (from) => from.effect("outer", () => [ + from.effect("nested", load), + from.effect("nested", load) +]) } })` + ], + invalid: [ + { + code: `import { Machine } from "@typeonce/effect-machine" +Machine.make({ initial: (to) => to.Ready() }).handle({ Ready: { invoke: (from) => [ + from.effect("load", load), + from.timer("load", 1000) +] } })`, + errors: [{ messageId: "conflictingLifecycle" }] + }, + { + code: `import { Machine } from "@typeonce/effect-machine" +const address = Machine.childAddress("worker") +Machine.make({ initial: (to) => to.Ready() }).handle({ Ready: { invoke: (from) => [ + from.logic("first", { address, logic }), + from.logic("second", { address, logic }) +] } })`, + errors: [{ messageId: "conflictingAddress" }] + }, + { + code: `import { Machine } from "@typeonce/effect-machine" +const Child = Machine.child("worker", childMachine) +Machine.make({ initial: (to) => to.Ready() }).handle({ Ready: { invoke: (from) => [ + from.child(Child), + from.child(Child) +] } })`, + errors: [{ messageId: "conflictingBoth" }] + }, + { + code: `import { Machine } from "@typeonce/effect-machine" +const id = "same" +Machine.make({ initial: (to) => to.Ready() }).handle({ Ready: { invoke: (from) => { + const first = from.effect(id, load).onDone((to) => to.none) + const second = from.stream(\`same\`, stream) + const invocations = [first, second] + return invocations +} } })`, + errors: [{ messageId: "conflictingLifecycle" }] + }, + { + code: `import { Machine } from "@typeonce/effect-machine" +const Child = Machine.child("shared", childMachine) +Machine.make({ initial: (to) => to.Ready() }).handle({ Ready: { invoke: (from) => [ + from.effect("shared", load), + from.child(Child) +] } })`, + errors: [{ messageId: "conflictingLifecycle" }] + } + ] +}) diff --git a/packages/oxlint-plugin/test/noNondeterministicPlanning.test.ts b/packages/oxlint-plugin/test/noNondeterministicPlanning.test.ts new file mode 100644 index 0000000..d22d1c1 --- /dev/null +++ b/packages/oxlint-plugin/test/noNondeterministicPlanning.test.ts @@ -0,0 +1,55 @@ +import { RuleTester } from "oxlint/plugins-dev" +import { describe, it } from "vitest" +import plugin from "../src/index.js" + +RuleTester.describe = describe +RuleTester.it = it + +const tester = new RuleTester({ + languageOptions: { parserOptions: { lang: "ts" } } +}) + +const rule = plugin.rules["no-nondeterministic-planning"] + +tester.run("no-nondeterministic-planning", rule, { + valid: [ + `import { Machine } from "@typeonce/effect-machine" +Machine.make({ initial: (to) => to.Ready() }).handle({ Ready: { entry: ({ event }) => new Date(event.timestamp) } })`, + `import { Machine } from "@typeonce/effect-machine" +Machine.make({ initial: (to) => to.Ready() }).handle({ Ready: { entry: ({ Date, Math }) => [Date.now(), Math.random()] } })`, + `import { Machine } from "@typeonce/effect-machine" +Machine.make({ initial: (to) => to.Ready() }).handle({ Ready: { invoke: (from) => from.effect("random", () => crypto.randomUUID()) } })`, + `import { Machine } from "@typeonce/effect-machine" +Machine.make({ initial: (to) => to.Ready() }).handle({ Ready: { output: ({ state }) => Date.parse(state.createdAt) } })` + ], + invalid: [ + { + code: `import { Machine } from "@typeonce/effect-machine" +Machine.make({ initial: (to) => { + Date() + new Date() + Date.now() + Math.random() + crypto.randomUUID() + crypto.getRandomValues(new Uint8Array(1)) + performance.now() + performance.timeOrigin + Temporal.Now.instant() + process.hrtime() + process.hrtime.bigint() + process.uptime() + return to.Ready() +} })`, + errors: Array.from({ length: 12 }, () => ({ messageId: "nondeterministic" })) + }, + { + code: `import { Machine } from "@typeonce/effect-machine" +Machine.make({ initial: (to) => to.Ready() }).handle({ Ready: { + initialize: ({ builder }) => globalThis.Date.now() ? builder.from() : builder.from(), + onDone: (to) => window.Math.random() ? to.none : to.none, + history: { recent: { default: (to) => self.crypto.randomUUID() ? to.none : to.none } } +} })`, + errors: Array.from({ length: 3 }, () => ({ messageId: "nondeterministic" })) + } + ] +}) diff --git a/packages/oxlint-plugin/test/noRedundantResolve.test.ts b/packages/oxlint-plugin/test/noRedundantResolve.test.ts index 765d54d..7d08a81 100644 --- a/packages/oxlint-plugin/test/noRedundantResolve.test.ts +++ b/packages/oxlint-plugin/test/noRedundantResolve.test.ts @@ -16,7 +16,7 @@ tester.run("no-redundant-resolve", rule, { `import { Machine } from "@typeonce/effect-machine" Machine.make({ initial: (to) => to.Ready().resolve(({ target }) => target.from({ id: "ready" })) })`, `import { Machine } from "@typeonce/effect-machine" -Machine.make({ initial: (to) => to.Ready().resolve(({ target }) => target.from(), { reenter: true }) })`, +Machine.make({ initial: (to) => to.Ready().resolve(({ target }) => target.from(), { reenter: true, actions: [] }) })`, `import { Machine } from "@typeonce/effect-machine" const other = { resolve: (_callback: unknown) => undefined } other.resolve(({ target }) => target.from())`, @@ -54,6 +54,31 @@ StateMachine.make({ initial: (to) => to.Ready() })`, EM.Machine.make({ initial: (to) => to.Ready().resolve(({ target }) => /* preserve */ target.from()) })`, output: null, errors: [{ messageId: "redundantResolver" }] + }, + { + code: `import { Machine } from "@typeonce/effect-machine" +Machine.make({ initial: (to) => to.Ready() }).handle({ Ready: { on: { + Reset: (to) => to.full.Ready().resolve(({ target }) => target.from(), { reenter: true }) +} } })`, + output: `import { Machine } from "@typeonce/effect-machine" +Machine.make({ initial: (to) => to.Ready() }).handle({ Ready: { on: { + Reset: (to) => to.full.Ready().reenter() +} } })`, + errors: [{ messageId: "redundantReenterResolver" }] + }, + { + code: `import { Machine } from "@typeonce/effect-machine" +Machine.make({ initial: (to) => to.Ready() }).handle({ Ready: { always: (to) => to.none.resolve(() => {}) } })`, + output: `import { Machine } from "@typeonce/effect-machine" +Machine.make({ initial: (to) => to.Ready() }).handle({ Ready: { always: (to) => to.none } })`, + errors: [{ messageId: "redundantTargetlessResolver" }] + }, + { + code: `import { Machine } from "@typeonce/effect-machine" +Machine.make({ initial: (to) => to.Ready() }).handle({ Ready: { always: (to) => to.none.resolve(() => {}, { reenter: true }) } })`, + output: `import { Machine } from "@typeonce/effect-machine" +Machine.make({ initial: (to) => to.Ready() }).handle({ Ready: { always: (to) => to.none.reenter() } })`, + errors: [{ messageId: "redundantReenterResolver" }] } ] }) diff --git a/scripts/oxlint-plugin-pack-check.mjs b/scripts/oxlint-plugin-pack-check.mjs index f553f73..3653fd5 100644 --- a/scripts/oxlint-plugin-pack-check.mjs +++ b/scripts/oxlint-plugin-pack-check.mjs @@ -112,8 +112,18 @@ try { "--eval", `const plugin = (await import("@typeonce/oxlint-plugin-effect-machine")).default; const { recommended } = await import("@typeonce/oxlint-plugin-effect-machine/recommended"); - if (plugin.meta?.name !== "effect-machine" || Object.keys(plugin.rules).length !== 3) process.exit(1); - if (Object.keys(recommended).length !== 3) process.exit(1);` + const expected = [ + "no-async-planning-callback", + "no-browser-api-in-planning", + "no-conflicting-invocation-identity", + "no-nondeterministic-planning", + "no-redundant-resolve", + "prefer-inline-handle" + ]; + const actual = Object.keys(plugin.rules).sort(); + const configured = Object.keys(recommended).map((rule) => rule.replace("effect-machine/", "")).sort(); + if (plugin.meta?.name !== "effect-machine" || JSON.stringify(actual) !== JSON.stringify(expected)) process.exit(1); + if (JSON.stringify(configured) !== JSON.stringify(expected)) process.exit(1);` ], { cwd: consumer }) const lint = spawnSync(oxlint, ["-c", ".oxlintrc.json", "machine.ts"], {