From 28f1a2d039e718435257ae2ce69c6258f38a9078 Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 22 Oct 2026 09:45:16 +0000 Subject: [PATCH 1/2] Document per-owner capabilities and lint conditional none branches Add React guide sections on `none` versus declinable acceptance, per-owner capability services, and emissions, plus review and agent guide notes. Add the recommended `effect-machine/no-conditional-none-branch` Oxlint rule. Co-Authored-By: Claude Fable 5.1 Claude-Session: https://claude.ai/code/session_01MHnF2EL6ReDTKwoM9TVH7L --- .changeset/no-conditional-none-branch.md | 15 + packages/effect-machine/docs/agent-guide.md | 19 +- .../effect-machine/docs/effect-atom-react.md | 126 +++++++++ .../effect-machine/docs/machine-review.md | 22 +- packages/oxlint-plugin/README.md | 52 +++- packages/oxlint-plugin/src/index.ts | 2 + .../internal/rules/noConditionalNoneBranch.ts | 264 ++++++++++++++++++ packages/oxlint-plugin/src/recommended.ts | 1 + .../test/noConditionalNoneBranch.test.ts | 135 +++++++++ scripts/oxlint-plugin-pack-check.mjs | 1 + 10 files changed, 624 insertions(+), 13 deletions(-) create mode 100644 .changeset/no-conditional-none-branch.md create mode 100644 packages/oxlint-plugin/src/internal/rules/noConditionalNoneBranch.ts create mode 100644 packages/oxlint-plugin/test/noConditionalNoneBranch.test.ts diff --git a/.changeset/no-conditional-none-branch.md b/.changeset/no-conditional-none-branch.md new file mode 100644 index 00000000..70559bcb --- /dev/null +++ b/.changeset/no-conditional-none-branch.md @@ -0,0 +1,15 @@ +--- +"@typeonce/oxlint-plugin-effect-machine": minor +--- + +Add the recommended `effect-machine/no-conditional-none-branch` rule. + +A `{ none: true }` branch accepts its event, so `Machine.can` and `AtomMachine.can` return `true` when a resolver selects it. The rule reports a resolver that selects a `none` branch conditionally without `declinable: true`. When the event must be refused, for example to disable a control, decline it instead: + +```ts +Submitted: { + branches: "submit", + declinable: true, + resolve: ({ state, select, decline }) => state.complete ? select.review() : decline() +} +``` diff --git a/packages/effect-machine/docs/agent-guide.md b/packages/effect-machine/docs/agent-guide.md index 44cc533e..646ff5ac 100644 --- a/packages/effect-machine/docs/agent-guide.md +++ b/packages/effect-machine/docs/agent-guide.md @@ -318,6 +318,12 @@ const LoadMachine = Machine.make({ implemented machine type, so the runtime must provide them when it starts the machine. +Model a per-instance capability, such as a router or a callback owned by a UI +component, as such a service. Do not put functions in state data or startup +input: snapshots must remain encodable. A React owner provides the capability +through its own runtime, as described in +[Per-owner capabilities](./effect-atom-react.md#per-owner-capabilities). + Do not start a promise inside a transition callback. A transition has no lifetime in which to own that work. A state does. @@ -405,6 +411,11 @@ Do not read the clock, generate randomness, call a service, or await work while choosing a transition. Receive such values in an event or produce them through state-owned work first. +A `{ none: true }` branch accepts its event without changing state, so +`Machine.can` reports `true` when a resolver selects it. When the event must be +refused, for example to disable a control, use `guard`, or declare +`declinable: true` and return `decline()` from the resolver. + When only an active state's value changes, use `update` to preserve its active descendants and running work: @@ -472,9 +483,11 @@ events, or runtime scheduling. Use atom selectors to render state, invocation outcomes to express workflow behavior, `ref.join` to await final output, and `ref.changes` for ongoing -observation. Reach for `Machine.waitFor(ref, predicate)` only when an external -Effect or test must wait for a specific current or subsequent snapshot, such as -a long-lived connection becoming ready. Compose `Effect.timeout` when needed. +observation. Emissions are non-replayed notifications; do not use them for work +the workflow depends on, such as navigation. Reach for +`Machine.waitFor(ref, predicate)` only when an external Effect or test must wait +for a specific current or subsequent snapshot, such as a long-lived connection +becoming ready. Compose `Effect.timeout` when needed. Do not use `send` followed by `waitFor` as an acknowledgement protocol. The current snapshot might already match, and a match does not identify which event caused diff --git a/packages/effect-machine/docs/effect-atom-react.md b/packages/effect-machine/docs/effect-atom-react.md index e89960f3..7ddacbfd 100644 --- a/packages/effect-machine/docs/effect-atom-react.md +++ b/packages/effect-machine/docs/effect-atom-react.md @@ -70,6 +70,73 @@ machines, even when their inputs are equal. `useMachineAtom(() => makeMachine(input))` remains available for custom owners. It mounts the reference after commit without reading `result` or `snapshot`. +## Per-owner capabilities + +A router, a callback owned by a component, or another per-instance capability +is a service, never a schema field or startup input. Declare the service, let a +state invoke the work that needs it, and provide the implementation from the +React owner: + +```tsx +import { Machine } from "@typeonce/effect-machine" +import { AtomMachine } from "@typeonce/effect-machine/reactivity" +import { useMachineAtom } from "@typeonce/effect-machine-react" +import { Context, Effect, Layer } from "effect" +import { Atom } from "effect/reactivity" +import { AppLayer } from "../lib/app-layer" + +export class Navigation extends Context.Service Effect.Effect +}>()("Navigation") {} + +export const OnboardingMachine = Machine.make({ + root: Machine.state({ + states: { Editing: {}, Leaving: {}, Done: { type: "final" } } + }), + events: Machine.events({ Submit: {} }), + effects: { + navigateNext: Effect.flatMap(Navigation, (navigation) => + navigation.push("/onboarding/next")) + } +}).handle({ + initial: { target: "Editing" }, + states: { + Editing: { on: { Submit: { target: "Leaving" } } }, + Leaving: { invoke: { src: "navigateNext", onDone: { target: "Done" } } }, + Done: {} + } +}) + +function OnboardingRoute() { + const router = useRouter() + const machine = useMachineAtom(() => + AtomMachine + .bind(Atom.runtime(Layer.merge( + AppLayer, + Layer.succeed(Navigation, { + push: (href) => Effect.sync(() => router.push(href)) + }) + ))) + .make(OnboardingMachine) + ) + + return +} +``` + +The requirement stays on the machine type. Binding the machine to a runtime +without `Navigation`, such as the application-wide `MachineAtoms`, is a compile +error. Snapshots contain only schema data, so `Machine.encodeSnapshot` keeps +working, and tests substitute the capability with `Layer.mock(Navigation, ...)`. + +Create the runtime inside the `useMachineAtom` factory so each owner builds it +once. `Atom.runtime` builds layers through the registry's memo map: the shared +`AppLayer` is built once per registry and reused, and each owner builds only its +own capability layer. Use `Layer.provideMerge(ownerLayer, AppLayer)` when the +owner layer itself needs application services. `createMachineContext` takes a +module-level factory, so distribute an owner-bound machine through props or a +React Context of your own. + ## Render state-owned data Render a typed path in the smallest component that needs it: @@ -206,6 +273,47 @@ const submitAllowed = AtomMachine.can(submitEvent) Changes to `draftAtom` recompute acceptance. The event atom contains the event input itself rather than an `AsyncResult`. +`can` is `true` whenever the active configuration selects a transition. A +`{ none: true }` branch is a selected transition: it accepts the event without +changing state. A resolver that falls back to a `none` branch therefore keeps +the button enabled: + +```ts +// Before: an incomplete form still accepts Submitted, so `can` is true. +// In make: branches: { submit: { review: { target: "Reviewing" }, incomplete: { none: true } } } +Submitted: { + branches: "submit", + resolve: ({ state, select }) => + state.email.length > 0 ? select.review() : select.incomplete() +} +``` + +When the event must be refused, decline it. Use `guard` for an ordinary +transition, or `declinable: true` with `decline()` in a resolver: + +```ts +// After: an incomplete form declines Submitted, so `can` is false. +Submitted: { target: "Reviewing", guard: ({ state }) => state.email.length > 0 } + +// A resolver that needs commands or construction declines explicitly. +// In make: branches: { submit: { review: { target: "Reviewing" } } } +Submitted: { + branches: "submit", + declinable: true, + resolve: ({ state, select, decline }) => + state.email.length > 0 ? select.review() : decline() +} +``` + +Keep `none` for events that are deliberately accepted and ignored. The +`effect-machine/no-conditional-none-branch` lint rule reports the conditional +fallback. + +Query acceptance during render through `AtomMachine.can(event)(machine)`. Do +not call `Effect.runSync(Machine.can(...))` in a component: the atom shares the +running machine's snapshot, suspends during startup, and recomputes when the +snapshot changes. + ## Whole-result and custom selections Reading the full result is correct when a component renders the complete @@ -328,6 +436,24 @@ export const plantAtoms = AtomMachine.familyChild(CentralMachineAtom, { }) ``` +## Keep workflow effects out of emissions + +Emissions are ephemeral notifications for observers. They are not replayed, so +a subscriber that mounts late never sees an earlier emission. Do not drive work +the workflow depends on, such as navigation, from emissions. Invoke that work +from the state that requires it through a capability service, as in +[Per-owner capabilities](#per-owner-capabilities); the machine then decides when +it runs, what cancels it, and how failure changes the workflow. + +An emission that only feeds UI feedback has two stream-atom details: + +- `Atom.make(AtomMachine.emissions(machine))` publishes each emission as a + waiting `Success`, then publishes the last emission again, no longer waiting, + when the machine completes. Handlers must tolerate that repeated value. +- `useAtomSubscribe(atom, f)` subscribes after commit without reading the atom. + Pass `{ immediate: true }` so a lazy emissions atom starts; without it the + callback never runs. The callback then also receives the initial result. + ## Registry and rendering semantics A `MachineAtom` identifies one machine per `AtomRegistry`. Passing the same diff --git a/packages/effect-machine/docs/machine-review.md b/packages/effect-machine/docs/machine-review.md index 06690479..566c5d70 100644 --- a/packages/effect-machine/docs/machine-review.md +++ b/packages/effect-machine/docs/machine-review.md @@ -11,10 +11,10 @@ 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. +asynchronous work, browser access, nondeterminism, redundant resolvers, and +conditional `none` fallbacks 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 @@ -43,6 +43,12 @@ constructors derived from those declarations. Avoid an extra branch group for an ordinary transition. Use `reenter: true` when the source must restart. An empty targetless resolver simplifies to `{ none: true }`. +A `{ none: true }` branch accepts its event, so `Machine.can` and +`AtomMachine.can` return `true` when a resolver selects it. Review check: for +every event that enables or disables a control, refuse it with `guard` or with +`declinable: true` and `decline()`. Keep `none` only for events that are +deliberately accepted and ignored. + ## Choose React ownership or keyed family lookup Use `useMachineAtom` when one React subtree owns the workflow, including a @@ -187,6 +193,14 @@ Review check: search components, hooks, and atom modules for service calls, `Effect.run*`, promises, browser APIs, analytics, timers, and chains of `send` calls. Move workflow coordination into states, transitions, and invoked work. +Per-instance capabilities, such as a router or a component callback, are +services provided by the React owner, not schema fields or startup input. A +function in state data or input cannot be encoded by `Machine.encodeSnapshot`. +Review check: search schemas for `Schema.declare` guards that accept functions +and move each one to a service, as in +[Per-owner capabilities](./effect-atom-react.md#per-owner-capabilities). Do not +drive navigation or other workflow effects from emissions. + ## Report findings For each issue, cite the file and line, name the violated boundary, and show the diff --git a/packages/oxlint-plugin/README.md b/packages/oxlint-plugin/README.md index b3ec2a58..a3dde717 100644 --- a/packages/oxlint-plugin/README.md +++ b/packages/oxlint-plugin/README.md @@ -1,8 +1,9 @@ # Effect Machine Oxlint plugin `@typeonce/oxlint-plugin-effect-machine` checks Effect Machine definitions for -invalid invocation identities, impure planning, redundant resolvers, and -one-use intermediate machine definitions. +invalid invocation identities, impure planning, redundant resolvers, +conditional `none` fallbacks that keep an event accepted, 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 @@ -41,6 +42,7 @@ JSON configurations can list the same rules directly: "rules": { "effect-machine/no-async-planning-callback": "error", "effect-machine/no-browser-api-in-planning": "error", + "effect-machine/no-conditional-none-branch": "error", "effect-machine/no-conflicting-invocation-identity": "error", "effect-machine/no-nondeterministic-planning": "error", "effect-machine/no-redundant-resolve": "error", @@ -74,6 +76,44 @@ Ignore: { none: true } The fixer preserves callbacks with comments or meaningful work. Ordinary transitions with default construction use `{ target: "Ready" }`. +### `effect-machine/no-conditional-none-branch` + +A `{ none: true }` branch accepts its event without changing state, so +`Machine.can` and `AtomMachine.can` report `true` when a resolver selects it. +The rule reports a resolver that selects a `none` branch conditionally without +`declinable: true`, because a control enabled by `can` would stay enabled: + +```ts +// In make: +branches: { + submit: { + review: { target: "Reviewing" }, + incomplete: { none: true } + } +} + +// Incorrect: Submitted is still accepted while the form is incomplete. +Submitted: { + branches: "submit", + resolve: ({ state, select }) => + state.complete ? select.review() : select.incomplete() +} + +// Correct: the incomplete form declines Submitted. +Submitted: { + branches: "submit", + declinable: true, + resolve: ({ state, select, decline }) => + state.complete ? select.review() : decline() +} +``` + +Remove the unused `none` branch from the group after the change, or use +`guard` on an ordinary transition. The rule recognizes `select.()`, +destructured selectors, ternaries, and `if` returns against a group declared in +the same module's `make` call. An unconditional `none` selection and a resolver +with `declinable: true` remain valid. + ### `effect-machine/no-async-planning-callback` Rejects asynchronous work in transition construction, guards, resolvers, @@ -143,17 +183,17 @@ Receive facts through events or machine input, or obtain them in invoked work: // In make: branches: { expiry: { - expired: { target: "Expired" }, - current: { none: true } + expired: { target: "Expired" } } } // In handle: Check: { branches: "expiry", - resolve: ({ event, select }) => event.now >= event.deadline + declinable: true, + resolve: ({ event, select, decline }) => event.now >= event.deadline ? select.expired.from() - : select.current() + : decline() } ``` diff --git a/packages/oxlint-plugin/src/index.ts b/packages/oxlint-plugin/src/index.ts index c3598792..156ada87 100644 --- a/packages/oxlint-plugin/src/index.ts +++ b/packages/oxlint-plugin/src/index.ts @@ -1,6 +1,7 @@ import type { Plugin } from "@oxlint/plugins" import { noAsyncPlanningCallback } from "./internal/rules/noAsyncPlanningCallback.js" import { noBrowserApiInPlanning } from "./internal/rules/noBrowserApiInPlanning.js" +import { noConditionalNoneBranch } from "./internal/rules/noConditionalNoneBranch.js" import { noConflictingInvocationIdentity } from "./internal/rules/noConflictingInvocationIdentity.js" import { noNondeterministicPlanning } from "./internal/rules/noNondeterministicPlanning.js" import { noRedundantResolve } from "./internal/rules/noRedundantResolve.js" @@ -13,6 +14,7 @@ const plugin = { rules: { "no-async-planning-callback": noAsyncPlanningCallback, "no-browser-api-in-planning": noBrowserApiInPlanning, + "no-conditional-none-branch": noConditionalNoneBranch, "no-conflicting-invocation-identity": noConflictingInvocationIdentity, "no-nondeterministic-planning": noNondeterministicPlanning, "no-redundant-resolve": noRedundantResolve, diff --git a/packages/oxlint-plugin/src/internal/rules/noConditionalNoneBranch.ts b/packages/oxlint-plugin/src/internal/rules/noConditionalNoneBranch.ts new file mode 100644 index 00000000..6d309295 --- /dev/null +++ b/packages/oxlint-plugin/src/internal/rules/noConditionalNoneBranch.ts @@ -0,0 +1,264 @@ +import type { ESTree, Rule } from "@oxlint/plugins" +import { resolvedVariable, staticMemberName, unwrapExpression } from "../ast.js" +import { + isMachineHandleCall, + isMachineMakeCall, + type MachineBindings, + makeMachineBindings, + recordMachineDefinition, + recordMachineImport +} from "../imports.js" +import { isPlanningCallback, type PlanningFunction } from "../planning.js" + +const propertyKey = (node: ESTree.Node): string | undefined => { + if (node.type !== "Property" || node.computed) return undefined + if (node.key.type === "Identifier") return node.key.name + return node.key.type === "Literal" && typeof node.key.value === "string" ? node.key.value : undefined +} + +const findProperty = ( + node: ESTree.ObjectExpression, + name: string +): ESTree.ObjectProperty | undefined => + node.properties.find((property): property is ESTree.ObjectProperty => propertyKey(property) === name) + +const isTrueLiteral = (node: ESTree.Node | undefined): boolean => + node?.type === "Literal" && node.value === true + +/** The `make` configuration that a `.handle(...)` call implements, when it is declared in this module. */ +const makeConfiguration = ( + handle: ESTree.CallExpression, + bindings: MachineBindings +): ESTree.ObjectExpression | undefined => { + if (handle.callee.type !== "MemberExpression") return undefined + const callee = unwrapExpression(handle.callee.object) + let receiver: ESTree.Node | null | undefined = callee + if (callee.type === "Identifier") { + const declaration = resolvedVariable(bindings.context, callee)?.identifiers[0]?.parent + receiver = declaration?.type === "VariableDeclarator" ? declaration.init : undefined + } + if (receiver?.type !== "CallExpression" || !isMachineMakeCall(receiver, bindings)) return undefined + const configuration = receiver.arguments[0] + return configuration?.type === "ObjectExpression" ? configuration : undefined +} + +const enclosingHandleCall = ( + node: ESTree.Node, + bindings: MachineBindings +): ESTree.CallExpression | undefined => { + let current: ESTree.Node | null = node.parent + while (current !== null && current.type !== "Program") { + if (current.type === "CallExpression" && isMachineHandleCall(current, bindings)) return current + current = current.parent + } + return undefined +} + +/** Keys of the branches declared as `{ none: true }` in one named group. */ +const noneBranches = ( + configuration: ESTree.ObjectExpression, + group: string +): ReadonlySet => { + const keys = new Set() + const groups = findProperty(configuration, "branches")?.value + if (groups?.type !== "ObjectExpression") return keys + const branches = findProperty(groups, group)?.value + if (branches?.type !== "ObjectExpression") return keys + for (const branch of branches.properties) { + const key = propertyKey(branch) + if ( + key !== undefined && branch.type === "Property" && branch.value.type === "ObjectExpression" && + isTrueLiteral(findProperty(branch.value, "none")?.value) + ) keys.add(key) + } + return keys +} + +interface Selectors { + /** Identifiers bound to the whole `select` object, or to the resolver context. */ + readonly select: ReadonlySet + readonly context: ReadonlySet + /** Identifiers bound to one destructured selector, by branch key. */ + readonly aliases: ReadonlyMap +} + +const resolverSelectors = (callback: PlanningFunction): Selectors | undefined => { + const parameter = callback.params[0] + if (parameter?.type === "Identifier") { + return { select: new Set(), context: new Set([parameter]), aliases: new Map() } + } + if (parameter?.type !== "ObjectPattern") return undefined + const selectProperty = parameter.properties.find((property): property is ESTree.BindingProperty => + property.type === "Property" && propertyKey(property) === "select" + ) + const value = selectProperty?.value + if (value?.type === "Identifier") { + return { select: new Set([value]), context: new Set(), aliases: new Map() } + } + if (value?.type !== "ObjectPattern") return undefined + const aliases = new Map() + for (const property of value.properties) { + const key = property.type === "Property" ? propertyKey(property) : undefined + if (key !== undefined && property.type === "Property" && property.value.type === "Identifier") { + aliases.set(property.value, key) + } + } + return { select: new Set(), context: new Set(), aliases } +} + +const bindsTo = ( + node: ESTree.Expression, + declarations: ReadonlySet | ReadonlyMap, + bindings: MachineBindings +): ESTree.Node | undefined => + node.type === "Identifier" + ? resolvedVariable(bindings.context, node)?.identifiers.find((identifier) => declarations.has(identifier)) + : undefined + +/** The branch key selected by a returned selector call such as `select.key()` or `alias.from()`. */ +const selectedBranch = ( + node: ESTree.Expression, + selectors: Selectors, + bindings: MachineBindings +): string | undefined => { + let current = unwrapExpression(node) + if (current.type !== "CallExpression") return undefined + while (current.type === "CallExpression" || current.type === "MemberExpression") { + if (current.type === "MemberExpression") { + const object = unwrapExpression(current.object) + if (bindsTo(object, selectors.select, bindings) !== undefined) return staticMemberName(current) + if ( + object.type === "MemberExpression" && staticMemberName(object) === "select" && + bindsTo(unwrapExpression(object.object), selectors.context, bindings) !== undefined + ) return staticMemberName(current) + } + current = unwrapExpression(current.type === "CallExpression" ? current.callee : current.object) + } + const alias = bindsTo(current, selectors.aliases, bindings) + return alias === undefined ? undefined : selectors.aliases.get(alias) +} + +interface Outcome { + readonly expression: ESTree.Expression + /** Whether a condition decides if this expression becomes the result. */ + readonly conditional: boolean +} + +const expressionOutcomes = ( + node: ESTree.Expression, + conditional: boolean, + outcomes: Array +): void => { + const expression = unwrapExpression(node) + if (expression.type === "ConditionalExpression") { + expressionOutcomes(expression.consequent, true, outcomes) + expressionOutcomes(expression.alternate, true, outcomes) + } else if (expression.type === "LogicalExpression") { + expressionOutcomes(expression.left, true, outcomes) + expressionOutcomes(expression.right, true, outcomes) + } else if (expression.type === "SequenceExpression") { + const last = expression.expressions.at(-1) + if (last !== undefined) expressionOutcomes(last, conditional, outcomes) + } else { + outcomes.push({ expression, conditional }) + } +} + +const statementOutcomes = ( + node: ESTree.Node, + conditional: boolean, + outcomes: Array +): void => { + if (node.type === "ReturnStatement") { + if (node.argument !== null) expressionOutcomes(node.argument, conditional, outcomes) + return + } + if (node.type === "BlockStatement") { + for (const statement of node.body) statementOutcomes(statement, conditional, outcomes) + return + } + if (node.type === "IfStatement") { + statementOutcomes(node.consequent, true, outcomes) + if (node.alternate !== null) statementOutcomes(node.alternate, true, outcomes) + return + } + if (node.type === "SwitchStatement") { + for (const branch of node.cases) { + for (const statement of branch.consequent) statementOutcomes(statement, true, outcomes) + } + return + } + if (node.type === "TryStatement") { + statementOutcomes(node.block, true, outcomes) + if (node.handler !== null) statementOutcomes(node.handler.body, true, outcomes) + if (node.finalizer !== null) statementOutcomes(node.finalizer, true, outcomes) + return + } + if ( + node.type === "LabeledStatement" || node.type === "ForStatement" || node.type === "ForInStatement" || + node.type === "ForOfStatement" || node.type === "WhileStatement" || node.type === "DoWhileStatement" + ) { + statementOutcomes(node.body, true, outcomes) + } +} + +/** Every expression the resolver can return, excluding nested functions. */ +const resolverOutcomes = (callback: PlanningFunction): ReadonlyArray => { + const outcomes: Array = [] + if (callback.body === null) return outcomes + if (callback.body.type === "BlockStatement") statementOutcomes(callback.body, false, outcomes) + else expressionOutcomes(callback.body, false, outcomes) + return outcomes +} + +export const noConditionalNoneBranch: Rule = { + meta: { + type: "problem", + docs: { + description: "Require declinable resolvers when a none branch is selected conditionally.", + recommended: true + }, + schema: [], + messages: { + conditionalNoneBranch: + "A conditionally selected `none` branch still accepts the event for `Machine.can`; declare `declinable: true` and return `decline()` when the event must be refused." + } + }, + create(context) { + const bindings = makeMachineBindings(context) + return { + ImportDeclaration: (node) => recordMachineImport(bindings, node), + Property(node) { + if (propertyKey(node) !== "resolve" || node.parent.type !== "ObjectExpression") return + const callback = node.value + if (callback.type !== "ArrowFunctionExpression" && callback.type !== "FunctionExpression") return + const transition = node.parent + const group = findProperty(transition, "branches")?.value + if (group?.type !== "Literal" || typeof group.value !== "string") return + const declinable = findProperty(transition, "declinable") + if (declinable !== undefined && (isTrueLiteral(declinable.value) || declinable.value.type !== "Literal")) { + return + } + if (!isPlanningCallback(callback, bindings)) return + + const handle = enclosingHandleCall(transition, bindings) + const configuration = handle === undefined ? undefined : makeConfiguration(handle, bindings) + if (configuration === undefined) return + const none = noneBranches(configuration, group.value) + if (none.size === 0) return + const selectors = resolverSelectors(callback) + if (selectors === undefined) return + + const outcomes = resolverOutcomes(callback) + for (const outcome of outcomes) { + if (!outcome.conditional && outcomes.length === 1) continue + const branch = selectedBranch(outcome.expression, selectors, bindings) + if (branch !== undefined && none.has(branch)) { + context.report({ node: outcome.expression, messageId: "conditionalNoneBranch" }) + } + } + }, + VariableDeclarator: (node) => recordMachineDefinition(bindings, node) + } + } +} diff --git a/packages/oxlint-plugin/src/recommended.ts b/packages/oxlint-plugin/src/recommended.ts index f4e4700f..7fa24204 100644 --- a/packages/oxlint-plugin/src/recommended.ts +++ b/packages/oxlint-plugin/src/recommended.ts @@ -2,6 +2,7 @@ export const recommended = { "effect-machine/no-async-planning-callback": "error", "effect-machine/no-browser-api-in-planning": "error", + "effect-machine/no-conditional-none-branch": "error", "effect-machine/no-conflicting-invocation-identity": "error", "effect-machine/no-nondeterministic-planning": "error", "effect-machine/no-redundant-resolve": "error", diff --git a/packages/oxlint-plugin/test/noConditionalNoneBranch.test.ts b/packages/oxlint-plugin/test/noConditionalNoneBranch.test.ts new file mode 100644 index 00000000..1e9ac453 --- /dev/null +++ b/packages/oxlint-plugin/test/noConditionalNoneBranch.test.ts @@ -0,0 +1,135 @@ +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-conditional-none-branch"] + +const make = (branches: string, transition: string, machine = "Machine") => + `import { ${machine === "Machine" ? "Machine" : `Machine as ${machine}`} } from "@typeonce/effect-machine" +${machine}.make({ branches: { ${branches} } }).handle({ states: { Editing: { on: { Submitted: ${transition} } } } })` + +const submit = `submit: { review: { target: "Reviewing" }, incomplete: { none: true } }` + +tester.run("no-conditional-none-branch", rule, { + valid: [ + // Targeted branches only. + make( + `submit: { review: { target: "Reviewing" }, editing: { target: "Editing" } }`, + `{ branches: "submit", resolve: ({ state, select }) => state.ok ? select.review() : select.editing() }` + ), + // Unconditional none selection. + make(submit, `{ branches: "submit", resolve: ({ select }) => select.incomplete() }`), + make(submit, `{ branches: "submit", resolve: ({ select }) => { return select.incomplete() } }`), + // Declinable resolvers may still select none deliberately. + make( + submit, + `{ branches: "submit", declinable: true, resolve: ({ state, select, decline }) => state.ok ? select.review() : state.draft ? select.incomplete() : decline() }` + ), + make( + submit, + `{ branches: "submit", declinable: true, resolve: ({ state, select }) => state.ok ? select.review() : select.incomplete() }` + ), + // A non-literal declinable flag is not guessed. + make( + submit, + `{ branches: "submit", declinable: flag, resolve: ({ state, select }) => state.ok ? select.review() : select.incomplete() }` + ), + // Conditional choice between targeted branches in a group that also declares none. + make( + submit, + `{ branches: "submit", resolve: ({ state, select }) => state.ok ? select.review() : select.review({ data: {} }) }` + ), + // A different group whose branch shares the none key name. + make( + `${submit}, other: { incomplete: { target: "Editing" }, review: { target: "Reviewing" } }`, + `{ branches: "other", resolve: ({ state, select }) => state.ok ? select.review() : select.incomplete() }` + ), + // A shadowed selector is not the resolver's select. + make( + submit, + `{ branches: "submit", resolve: ({ state, select }) => state.ok ? select.review() : ((select) => select.incomplete())(other) }` + ), + // Without a Machine import nothing is a machine definition. + `const Machine = { make: (_: unknown) => ({ handle: (_: unknown) => undefined }) } +Machine.make({ branches: { ${submit} } }).handle({ states: { Editing: { on: { Submitted: { branches: "submit", resolve: ({ state, select }) => state.ok ? select.review() : select.incomplete() } } } } })`, + // An unrelated object outside a machine definition. + `import { Machine } from "@typeonce/effect-machine" +const config = { branches: "submit", resolve: ({ state, select }) => state.ok ? select.review() : select.incomplete() }` + ], + invalid: [ + { + code: make( + submit, + `{ branches: "submit", resolve: ({ state, select }) => state.ok ? select.review() : select.incomplete() }` + ), + errors: [{ messageId: "conditionalNoneBranch" }] + }, + { + code: make( + submit, + `{ branches: "submit", declinable: false, resolve: ({ state, select }) => state.ok ? select.review() : select.incomplete() }`, + "StateMachine" + ), + errors: [{ messageId: "conditionalNoneBranch" }] + }, + { + code: make( + submit, + `{ branches: "submit", resolve: ({ state, select: { review, incomplete: skip } }) => state.ok ? review() : skip() }` + ), + errors: [{ messageId: "conditionalNoneBranch" }] + }, + { + code: make( + `submit: { review: { target: "Reviewing" }, editing: { target: "Editing" }, "incomplete": { none: true } }`, + `{ branches: "submit", resolve: ({ state, select }) => state.ok ? select.review() : state.draft ? select.editing() : select.incomplete() }` + ), + errors: [{ messageId: "conditionalNoneBranch" }] + }, + { + code: make( + submit, + `{ branches: "submit", resolve: ({ state, select }) => { + if (state.ok) return select.review() + return select.incomplete() +} }` + ), + errors: [{ messageId: "conditionalNoneBranch" }] + }, + { + code: make( + submit, + `{ branches: "submit", resolve(context) { + if (!context.state.ok) { + return context.select.incomplete() + } else { + return context.select.review() + } +} }` + ), + errors: [{ messageId: "conditionalNoneBranch" }] + }, + { + code: `import * as EM from "@typeonce/effect-machine" +const definition = EM.Machine.make({ branches: { ${submit} } }) +export const machine = definition.handle({ states: { Editing: { states: { Draft: { on: { + Submitted: { branches: "submit", resolve: ({ state, select }) => state.ok ? select.review() : select.incomplete.from() } +} } } } } })`, + errors: [{ messageId: "conditionalNoneBranch" }] + }, + { + code: make( + `submit: { incomplete: { none: true }, skipped: { none: true }, review: { target: "Reviewing" } }`, + `{ branches: "submit", resolve: ({ state, select }) => state.ok ? select.review() : state.draft ? select.incomplete() : select.skipped() }` + ), + errors: [{ messageId: "conditionalNoneBranch" }, { messageId: "conditionalNoneBranch" }] + } + ] +}) diff --git a/scripts/oxlint-plugin-pack-check.mjs b/scripts/oxlint-plugin-pack-check.mjs index 9c193baf..126f13e2 100644 --- a/scripts/oxlint-plugin-pack-check.mjs +++ b/scripts/oxlint-plugin-pack-check.mjs @@ -115,6 +115,7 @@ try { const expected = [ "no-async-planning-callback", "no-browser-api-in-planning", + "no-conditional-none-branch", "no-conflicting-invocation-identity", "no-nondeterministic-planning", "no-redundant-resolve", From 14c5569b28be1eece7dff548ccbbd975ffc08b87 Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 22 Oct 2026 09:51:09 +0000 Subject: [PATCH 2/2] Format the conditional none branch rule Co-Authored-By: Claude Fable 5.1 Claude-Session: https://claude.ai/code/session_01MHnF2EL6ReDTKwoM9TVH7L --- .../src/internal/rules/noConditionalNoneBranch.ts | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/packages/oxlint-plugin/src/internal/rules/noConditionalNoneBranch.ts b/packages/oxlint-plugin/src/internal/rules/noConditionalNoneBranch.ts index 6d309295..21a2d72f 100644 --- a/packages/oxlint-plugin/src/internal/rules/noConditionalNoneBranch.ts +++ b/packages/oxlint-plugin/src/internal/rules/noConditionalNoneBranch.ts @@ -22,8 +22,7 @@ const findProperty = ( ): ESTree.ObjectProperty | undefined => node.properties.find((property): property is ESTree.ObjectProperty => propertyKey(property) === name) -const isTrueLiteral = (node: ESTree.Node | undefined): boolean => - node?.type === "Literal" && node.value === true +const isTrueLiteral = (node: ESTree.Node | undefined): boolean => node?.type === "Literal" && node.value === true /** The `make` configuration that a `.handle(...)` call implements, when it is declared in this module. */ const makeConfiguration = (