diff --git a/CHANGES.md b/CHANGES.md index f9df4528f..3ed53b9ae 100644 --- a/CHANGES.md +++ b/CHANGES.md @@ -272,8 +272,21 @@ To be released. when an actor dispatcher's return value does not include a `preferredUsername` property. [[#895], [#1022] by Jae-Hyuk-Jang\] + - Changed `outbox-listener-delivery-required` (`@fedify/lint`) to decide + whether a `ctx.sendActivity()`/`ctx.forwardActivity()` call actually + runs, instead of scanning the listener's source as a flat block of text. + It now reports a listener whose only delivery calls sit behind a dead + branch, after an unconditional `return`/`throw`, or inside a function + that is never used. When it cannot tell whether a delivery call runs, it + stays quiet: a function held under a name counts as used as soon as that + name is mentioned, however it is passed around, and an inline callback + counts wherever it is passed. + [[#900], [#1050] by Jae-Hyuk-Jang\] + [#895]: https://github.com/fedify-dev/fedify/issues/895 +[#900]: https://github.com/fedify-dev/fedify/issues/900 [#1022]: https://github.com/fedify-dev/fedify/pull/1022 +[#1050]: https://github.com/fedify-dev/fedify/pull/1050 ### @fedify/mysql diff --git a/changes.d/lint/outbox-listener-path-aware.md b/changes.d/lint/outbox-listener-path-aware.md new file mode 100644 index 000000000..b54c302ba --- /dev/null +++ b/changes.d/lint/outbox-listener-path-aware.md @@ -0,0 +1,15 @@ +--- +links: + '#1050': https://github.com/fedify-dev/fedify/pull/1050 + '#900': https://github.com/fedify-dev/fedify/issues/900 +--- + - Changed `outbox-listener-delivery-required` (`@fedify/lint`) to decide + whether a `ctx.sendActivity()`/`ctx.forwardActivity()` call actually + runs, instead of scanning the listener's source as a flat block of text. + It now reports a listener whose only delivery calls sit behind a dead + branch, after an unconditional `return`/`throw`, or inside a function + that is never used. When it cannot tell whether a delivery call runs, it + stays quiet: a function held under a name counts as used as soon as that + name is mentioned, however it is passed around, and an inline callback + counts wherever it is passed. + [[#900], [#1050] by Jae-Hyuk-Jang] diff --git a/docs/manual/lint.md b/docs/manual/lint.md index 4f9184c11..506219229 100644 --- a/docs/manual/lint.md +++ b/docs/manual/lint.md @@ -752,13 +752,38 @@ Warns when an outbox listener body does not deliver the posted activity with `ctx.sendActivity()` or `ctx.forwardActivity()`. **When this rule applies:** -You've registered an outbox listener with `setOutboxListeners()`, but the -listener body never calls either delivery method. +You've registered an outbox listener with `setOutboxListeners()`, and the rule +can show that no path through the listener body calls either delivery method. +It follows the listener's own control flow (`if`/`else`, `try`/`catch`, +`switch`, loops), so a delivery call that sits in a dead branch, after an +unconditional `return`, or in a function that is never used does not count. + +The rule reports only when it can account for every delivery call it can see +and show that each one does not run. When it cannot tell, it stays quiet: a +missed warning is the safe direction, while a warning on code that delivers is +not. In practice: + + - A function held under a name counts as used as soon as that name is + mentioned anywhere in code that runs, however it is mentioned: called, + passed to another function, aliased, destructured from an object, or + reached through an array or a wrapper call. The rule does not follow the + value any further, so a function that is only logged or stored, and never + called, is not reported. + - An inline callback counts wherever it is passed, since the rule cannot show + that the receiving call never runs it. + - The rule reads only the listener body. A delivery call in a helper that + is declared outside the listener, or in another module, is not seen. **Why it matters:** Fedify does not federate client-to-server outbox posts automatically. If your application intends to deliver a posted activity, the listener must choose an -explicit delivery path. +explicit delivery path, and that path must actually run. + +The rule checks that a delivery call exists and can run, not that the delivery +completes, so a listener it accepts is not guaranteed to federate. A delivery +call that is never awaited is not reported; +[#1057] tracks a rule for +that. ~~~~ typescript twoslash // @noErrors: 2345 @@ -773,6 +798,19 @@ federation console.log(ctx.identifier, activity.id?.href); }); +// ❌ Bad: The delivery call is unreachable dead code +federation + .setOutboxListeners("/users/{identifier}/outbox") + .on(Activity, async (ctx, activity) => { + if (activity.id == null) return; + return; + await ctx.sendActivity( + { identifier: ctx.identifier }, + "followers", + activity, + ); + }); + // ✅ Good: Listener federates explicitly federation .setOutboxListeners("/users/{identifier}/outbox") @@ -793,8 +831,36 @@ federation "followers", ); }); + +// ✅ Good: Delivery happens inside a helper that is actually called +federation + .setOutboxListeners("/users/{identifier}/outbox") + .on(Activity, async (ctx, activity) => { + async function deliver() { + await ctx.sendActivity( + { identifier: ctx.identifier }, + "followers", + activity, + ); + } + await deliver(); + }); + +// ✅ Good: A helper reached through a destructured property still counts +federation + .setOutboxListeners("/users/{identifier}/outbox") + .on(Activity, async (ctx, activity) => { + const handlers = { + deliver: () => + ctx.sendActivity({ identifier: ctx.identifier }, "followers", activity), + }; + const { deliver } = handlers; + await deliver(); + }); ~~~~ +[#1057]: https://github.com/fedify-dev/fedify/issues/1057 + ### `media-uploader-object-uri-required` Warns when a `setMediaUploader()` callback returns a value that is not derived diff --git a/packages/lint/src/rules/outbox-listener-delivery-required.ts b/packages/lint/src/rules/outbox-listener-delivery-required.ts index e6305fb57..91d8f4459 100644 --- a/packages/lint/src/rules/outbox-listener-delivery-required.ts +++ b/packages/lint/src/rules/outbox-listener-delivery-required.ts @@ -52,6 +52,15 @@ type FunctionLikeNode = body: unknown; }); +const FUNCTION_NODE_TYPES = new Set([ + "FunctionDeclaration", + "FunctionExpression", + "ArrowFunctionExpression", +]); + +const isFunctionLikeNode = (node: Node): node is FunctionLikeNode => + FUNCTION_NODE_TYPES.has(node.type); + const getMemberPropertyName = (expr: Expression): string | null => { if (expr.type !== "MemberExpression") return null; const property = expr.property as Node; @@ -219,7 +228,14 @@ function buildContextExpressionPattern(contextName: string): string { .raw`(?:${boundedName}|\(\s*${boundedName}(?:\s+as\s+[^)]+)?\s*\))`; } -const resolveListenerReference = ( +/** + * Resolves an expression to the function it refers to: a direct function + * literal, a local variable bound to one, or a property of a local object + * literal bound to one (e.g. `handlers.deliver` where + * `const handlers = { deliver() {} }`). Used to resolve a listener argument + * (`.on(Activity, handler)`). + */ +const resolveFunctionBinding = ( expr: Expression, bindings: Map, seen = new Set(), @@ -237,7 +253,7 @@ const resolveListenerReference = ( return binding as FunctionLikeNode; } if (binding.type === "Identifier") { - return resolveListenerReference(binding, bindings, seen); + return resolveFunctionBinding(binding, bindings, seen); } return null; } @@ -273,11 +289,577 @@ const resolveListenerReference = ( return null; }; +// --------------------------------------------------------------------------- +// Reachability: which statements can actually run, following control flow +// (if/else, try/catch/finally, switch, loops) but never descending into a +// nested function's own body, and pruning dead code (a statically-falsy `if` +// branch, or anything after a statement that always returns/throws). +// +// A control-flow statement's head expressions (an `if` test, a `switch` +// discriminant and case tests, a loop's `init`/`test`/`update`/`right`) run +// whenever the statement itself does, whichever branch is taken, so they are +// collected alongside the bodies. Collecting one never revives the branch +// behind it: `if (false)` still hides its consequent. +// --------------------------------------------------------------------------- + +const isStaticallyFalsy = (test: Expression): boolean => + test.type === "Literal" && !test.value; + +const isStaticallyTruthy = (test: Expression): boolean => + test.type === "Literal" && Boolean(test.value); + +/** + * Whether every path through this statement unconditionally returns or + * throws, meaning anything textually after it in the same statement list + * never runs. Deliberately conservative: when it can't prove that, it + * answers `false`, which keeps the following code counted as reachable + * (a missed dead-code case is safer than wrongly hiding live code). + */ +function alwaysExits(node: Node): boolean { + switch (node.type) { + case "ReturnStatement": + case "ThrowStatement": + case "BreakStatement": + case "ContinueStatement": + return true; + + case "BlockStatement": + return node.body.some((statement) => alwaysExits(statement as Node)); + + case "IfStatement": { + const test = node.test as Expression; + if (isStaticallyFalsy(test)) { + return node.alternate != null && alwaysExits(node.alternate as Node); + } + if (isStaticallyTruthy(test)) { + return alwaysExits(node.consequent as Node); + } + if (node.alternate == null) return false; + return alwaysExits(node.consequent as Node) && + alwaysExits(node.alternate as Node); + } + + case "TryStatement": + // A `finally` that always exits dominates the whole statement. Beyond + // that, a `try` block can throw partway through and jump to `catch`, + // so proving more than this would need tracking which statements can + // throw -- stay conservative and say "not sure" instead. + return node.finalizer != null && alwaysExits(node.finalizer as Node); + + default: + return false; + } +} + +function collectReachableStatements(node: Node, out: Node[]): void { + switch (node.type) { + case "BlockStatement": + for (const [index, statement] of node.body.entries()) { + collectReachableStatements(statement as Node, out); + if (alwaysExits(statement as Node)) { + // Function declarations hoist: one written below an exit is still + // callable from the code above it. + for (const rest of node.body.slice(index + 1)) { + if ((rest as Node).type === "FunctionDeclaration") { + out.push(rest as Node); + } + } + return; + } + } + return; + + case "IfStatement": { + const test = node.test as Expression; + out.push(test); + if (!isStaticallyFalsy(test)) { + collectReachableStatements(node.consequent as Node, out); + } + if (node.alternate != null && !isStaticallyTruthy(test)) { + collectReachableStatements(node.alternate as Node, out); + } + return; + } + + case "TryStatement": + collectReachableStatements(node.block as Node, out); + if (node.handler != null) { + collectReachableStatements(node.handler.body as Node, out); + } + if (node.finalizer != null) { + collectReachableStatements(node.finalizer as Node, out); + } + return; + + case "SwitchStatement": + out.push(node.discriminant as Node); + for (const switchCase of node.cases) { + if (switchCase.test != null) out.push(switchCase.test as Node); + for (const statement of switchCase.consequent) { + collectReachableStatements(statement as Node, out); + if (alwaysExits(statement as Node)) break; + } + } + return; + + case "WhileStatement": + case "DoWhileStatement": + out.push(node.test as Node); + collectReachableStatements(node.body as Node, out); + return; + + case "ForStatement": + for (const head of [node.init, node.test, node.update]) { + if (head != null) out.push(head as Node); + } + collectReachableStatements(node.body as Node, out); + return; + + case "ForInStatement": + case "ForOfStatement": + // Only `right` is evaluated as a value; `left` declares or assigns the + // loop variable. + out.push(node.right as Node); + collectReachableStatements(node.body as Node, out); + return; + + case "LabeledStatement": + collectReachableStatements(node.body as Node, out); + return; + + case "WithStatement": + out.push(node.object as Node); + collectReachableStatements(node.body as Node, out); + return; + + default: + out.push(node); + return; + } +} + +// --------------------------------------------------------------------------- +// What a listener (or a helper's own body) resolves to when scanned for a +// delivery call: two rules decide which nested function bodies are folded +// into the scan instead of being masked out. +// +// The rule reports only when neither can account for a delivery call, so +// both err toward treating a function as used. Working out how a function +// value travels through arbitrary JavaScript (an alias, a destructured +// property, an array, a wrapper call) is open-ended, and so is working out +// what a call does with a callback it receives. Showing that a name never +// appears anywhere that runs is not. So a function held under a name is +// used as soon as that name is mentioned, without tracing how it is then +// passed around, and any other function literal, such as a callback handed +// to a call, is used wherever it appears, since the rule cannot show that +// the receiving call never runs it. A missed warning is the safe direction; +// a warning on code that delivers is not. +// --------------------------------------------------------------------------- + +/** + * Collects plain-value references to identifiers: `deliver()`, + * `forEach(deliver)`, a shorthand `{ deliver }`, and so on. Skips positions + * that name something rather than reference a value: a declaration's own + * `id`/params, the non-computed `.property` of a member expression (so + * `someService.deliver()` never counts as a reference to an unrelated local + * `deliver`), and the target of an assignment, which writes to a name + * instead of reading it. + */ +function collectReferencedNames(node: unknown, out: Set): void { + if (node == null || typeof node !== "object") return; + if (Array.isArray(node)) { + for (const item of node) collectReferencedNames(item, out); + return; + } + if (!isNode(node)) return; + const n = node; + + if (n.type === "Identifier") { + out.add(n.name); + return; + } + if (n.type === "MemberExpression" && !n.computed) { + collectReferencedNames(n.object, out); + return; + } + if (n.type === "Property" && !n.computed) { + // `{ deliver: fn }` -- the key is a name, not a reference; only the + // value is (for shorthand `{ deliver }`, the value is the same name, + // so this still counts it). + collectReferencedNames(n.value, out); + return; + } + if (n.type === "VariableDeclarator") { + if ((n as VariableDeclarator).init != null) { + collectReferencedNames((n as VariableDeclarator).init, out); + } + return; + } + if (n.type === "ClassDeclaration" || n.type === "ClassExpression") { + // The class's own name is a declaration, not a mention of it. + collectReferencedNames(n.superClass, out); + collectReferencedNames(n.body, out); + return; + } + if ( + (n.type === "MethodDefinition" || n.type === "PropertyDefinition") && + !n.computed + ) { + // Same as an object literal's `Property`: the key names a member, and + // only what it holds can reference something. + collectReferencedNames(n.value, out); + return; + } + if (n.type === "AssignmentExpression" && n.operator === "=") { + // `x = fn` and `obj.x = fn` write to a name rather than mention it. + if (getAssignmentTargetName(n.left as Node) != null) { + collectReferencedNames(n.right, out); + return; + } + } + if (isFunctionLikeNode(n)) { + // Stop at a nested function's own boundary: whether a name it + // references counts is decided separately, only once that function + // itself is found to be reachable. + return; + } + + const record = n as unknown as Record; + for (const key in record) { + if (key === "parent") continue; + collectReferencedNames(record[key], out); + } +} + +/** + * The name an assignment writes to: `x` for `x = ...`, and the root object + * for `obj.a.b = ...`. `null` for anything more exotic. + */ +function getAssignmentTargetName(target: Node): string | null { + let current: Node = target; + while (current.type === "MemberExpression") current = current.object as Node; + return current.type === "Identifier" ? current.name : null; +} + +/** Every identifier a declaration pattern binds (`a`, `{ a, b: c }`, `[a]`). */ +function collectBoundNames(pattern: unknown, out: string[]): void { + if (pattern == null || typeof pattern !== "object" || !isNode(pattern)) { + return; + } + const p = pattern as Node; + switch (p.type) { + case "Identifier": + out.push(p.name); + return; + case "AssignmentPattern": + collectBoundNames(p.left, out); + return; + case "RestElement": + collectBoundNames(p.argument, out); + return; + case "ArrayPattern": + for (const element of p.elements) collectBoundNames(element, out); + return; + case "ObjectPattern": + for (const prop of p.properties) { + collectBoundNames( + (prop as { value?: unknown; argument?: unknown }).value ?? + (prop as { argument?: unknown }).argument, + out, + ); + } + return; + } +} + +/** + * Finds the function literals a value holds itself: the value is the + * function, or it sits in an object or array literal, a conditional, or a + * class body. Stops at a call, so a function handed to one as an argument, + * or invoked by it, is not held by whatever the call's result is bound to. + * Those count on their own wherever they appear. + */ +function collectHeldFunctions(node: unknown, out: FunctionLikeNode[]): void { + if (node == null || typeof node !== "object") return; + if (Array.isArray(node)) { + for (const item of node) collectHeldFunctions(item, out); + return; + } + if (!isNode(node)) return; + const n = node; + + if (isFunctionLikeNode(n)) { + out.push(n); + return; + } + if (n.type === "CallExpression" || n.type === "NewExpression") return; + + const record = n as unknown as Record; + for (const key in record) { + if (key === "parent") continue; + collectHeldFunctions(record[key], out); + } +} + +/** + * Collects the functions each name in `node`'s own scope holds: `function + * name() {}`, `class Name {}`, and the value of `const name = ...` or a + * later `name = ...` or `name.prop = ...`, including a function inside an + * object or array literal (see `collectHeldFunctions`). A function held + * under a name counts as used as soon as the name is mentioned, however it + * is mentioned, so this never has to work out how the name reaches the + * function. Does not descend into a found function's own body: a name bound + * inside it is only found once that function is itself resolved as + * reachable, so it can be layered on top of (and correctly shadow) the outer + * scope's names. + */ +function collectFunctionsByName( + node: unknown, + out: Map, +): void { + if (node == null || typeof node !== "object") return; + if (Array.isArray(node)) { + for (const item of node) collectFunctionsByName(item, out); + return; + } + if (!isNode(node)) return; + const n = node; + + const bindTo = (names: string[], from: unknown): void => { + const functions: FunctionLikeNode[] = []; + collectHeldFunctions(from, functions); + if (functions.length < 1) return; + for (const name of names) { + out.set(name, [...(out.get(name) ?? []), ...functions]); + } + }; + + if (n.type === "FunctionDeclaration") { + if (n.id?.name != null) bindTo([n.id.name], n); + return; + } + if (n.type === "ClassDeclaration") { + if (n.id?.name != null) bindTo([n.id.name], n); + return; + } + if (isFunctionLikeNode(n)) return; + if (n.type === "VariableDeclarator") { + const decl = n as VariableDeclarator; + if (decl.init != null) { + const names: string[] = []; + collectBoundNames(decl.id, names); + bindTo(names, decl.init); + collectFunctionsByName(decl.init, out); + } + return; + } + if (n.type === "AssignmentExpression") { + const name = getAssignmentTargetName(n.left as Node); + if (name != null) bindTo([name], n.right); + collectFunctionsByName(n.right, out); + return; + } + + const record = n as unknown as Record; + for (const key in record) { + if (key === "parent") continue; + collectFunctionsByName(record[key], out); + } +} + +/** + * Finds function literals directly nested in a reachable statement, without + * descending past them -- their own reachability is decided separately. + */ +function collectNestedFunctions( + node: unknown, + out: FunctionLikeNode[], +): void { + if (node == null || typeof node !== "object") return; + if (Array.isArray(node)) { + for (const item of node) collectNestedFunctions(item, out); + return; + } + if (!isNode(node)) return; + const n = node; + + if (isFunctionLikeNode(n)) { + out.push(n); + return; + } + + const record = n as unknown as Record; + for (const key in record) { + if (key === "parent") continue; + collectNestedFunctions(record[key], out); + } +} + +/** + * Computes the full set of function nodes that are actually reachable from + * `root`: `root` itself feeds a worklist, and each function it (or a + * function already on the worklist) uses from *its own* reachable + * statements -- never from a dead branch or some other not-yet-reached + * function's body -- gets queued in turn. `outerFunctionsByName` is layered + * fresh for each scope, so a name bound at an inner scope shadows a + * same-named one further out instead of overwriting it globally, and a + * dead branch that merely mentions a name never queues what it holds. + */ +function computeUsedFunctions(root: Node): Set { + const used = new Set(); + const visited = new Set(); + + const processScope = ( + scopeRoot: Node, + outerFunctionsByName: ReadonlyMap, + ): void => { + if (visited.has(scopeRoot)) return; + visited.add(scopeRoot); + + const statements: Node[] = []; + collectReachableStatements(scopeRoot, statements); + + const functionsHere = new Map(); + for (const statement of statements) { + collectFunctionsByName(statement, functionsHere); + } + const functionsByName = new Map(outerFunctionsByName); + for (const [name, functions] of functionsHere) { + functionsByName.set(name, functions); + } + + const referencedNames = new Set(); + for (const statement of statements) { + collectReferencedNames(statement, referencedNames); + } + // A function held under a name counts as reached as soon as that name + // is mentioned at all -- called, passed along, aliased, destructured, + // passed to `console.log`, stored in a variable, anything -- not only + // when it's actually invoked. That's what lets `recipients.map(deliver)` + // and `const { deliver } = handlers` resolve as used without this code + // having to know that `map` invokes its argument or how a destructured + // property gets from the object to the call. Telling a real invocation + // apart from merely holding a reference would need following every + // shape a function value can travel in and knowing which APIs call what + // they're given, which is more than this rule should carry, and any + // shape it missed would report code that delivers. The cost is a + // narrow false negative: a function that's only logged or reassigned, + // never called, is not reported. That is accepted deliberately, since + // missing a case here is the safe direction. Leave this as is. + const reached = new Set(); + for (const [name, functions] of functionsByName) { + if (!referencedNames.has(name)) continue; + for (const fn of functions) reached.add(fn); + } + + // Every other function literal counts wherever it appears: a callback + // handed to `map`, `forEach`, `queue.push` or a call the rule has never + // heard of, an immediately invoked function, a returned closure. The + // rule cannot show that the receiving code never runs it, and it does + // not check whether the result is awaited: a delivery call that is + // never awaited is left alone too. Leave this as is. + const held = new Set(); + for (const functions of functionsHere.values()) { + for (const fn of functions) held.add(fn); + } + for (const statement of statements) { + const nested: FunctionLikeNode[] = []; + collectNestedFunctions(statement, nested); + for (const fn of nested) { + if (!held.has(fn)) reached.add(fn); + } + } + + for (const fn of reached) { + used.add(fn); + processScope(fn.body as Node, functionsByName); + } + }; + + processScope(root, new Map()); + return used; +} + +/** + * A node's `[start, end)` character offsets into the whole source file. + * Both engines always populate this -- ESLint forces it on regardless of + * parser options, and Deno.lint exposes it the same way as every other + * child property (see the `for...in` note on why plain property access + * still works even though it's not an own enumerable property). + */ +function getRange(node: Node): readonly [number, number] { + return (node as unknown as { range: [number, number] }).range; +} + +/** + * Builds the source text to scan for a delivery call: the reachable + * statements of `root`, with every nested function literal either folded + * in (its own reachable text spliced in place, wherever that function's + * own declaration happens to live) or blanked out, depending on whether + * `used` (from `computeUsedFunctions`) says it is actually invoked. + * + * Splices each function by its own range rather than by matching its + * source text, and applies the splices from the end of the statement + * backward. That keeps two functions with byte-identical bodies (e.g. two + * object-literal methods that both merely call `ctx.sendActivity(...)`) + * from colliding: a text-based replacement would find and blank out both + * occurrences the first time either one is processed, since it matches by + * content everywhere in the statement rather than by which node is + * actually being replaced. Replacing from the end backward also means a + * later replacement's length change never shifts the still-unprocessed + * offsets of an earlier one. + */ +function collectDeliveryScanCode( + sourceCode: { getText(node: unknown): string }, + root: Node, + used: ReadonlySet, + visited: Set, +): string { + if (visited.has(root)) return ""; + visited.add(root); + + const statements: Node[] = []; + collectReachableStatements(root, statements); + + return statements + .map((statement) => { + const text = sourceCode.getText(statement); + const [statementStart] = getRange(statement); + + const nested: FunctionLikeNode[] = []; + collectNestedFunctions(statement, nested); + const byDescendingStart = [...nested].sort((a, b) => + getRange(b)[0] - getRange(a)[0] + ); + + let result = text; + for (const fn of byDescendingStart) { + const [fnStart, fnEnd] = getRange(fn); + const replacement = used.has(fn) + ? collectDeliveryScanCode(sourceCode, fn.body as Node, used, visited) + : ""; + result = result.slice(0, fnStart - statementStart) + + (replacement.length > 0 ? replacement : "()=>{}") + + result.slice(fnEnd - statementStart); + } + return result; + }) + .join("\n"); +} + const listenerCallsDeliveryMethod = ( sourceCode: { getText(node: unknown): string }, listener: FunctionLikeNode, ): boolean => { - const code = stripCommentsAndStrings(sourceCode.getText(listener)); + const used = computeUsedFunctions(listener.body as Node); + const code = stripCommentsAndStrings( + collectDeliveryScanCode( + sourceCode, + listener.body as Node, + used, + new Set(), + ), + ); const aliases = new Set(); const contextParam = unwrapContextParam( listener.params[0] as Node | undefined, @@ -377,11 +959,13 @@ function createRule( isNode(listener) && isFunction(listener as Expression) ? listener as FunctionLikeNode : isNode(listener) - ? resolveListenerReference(listener as Expression, bindings) + ? resolveFunctionBinding(listener as Expression, bindings) : null; if (resolvedListener == null) return; - if (listenerCallsDeliveryMethod(sourceCode, resolvedListener)) return; + if (listenerCallsDeliveryMethod(sourceCode, resolvedListener)) { + return; + } (context as { report: (arg: unknown) => void }).report({ node: resolvedListener, diff --git a/packages/lint/src/tests/outbox-listener-delivery-required.test.ts b/packages/lint/src/tests/outbox-listener-delivery-required.test.ts index 00533540a..2346eb150 100644 --- a/packages/lint/src/tests/outbox-listener-delivery-required.test.ts +++ b/packages/lint/src/tests/outbox-listener-delivery-required.test.ts @@ -223,6 +223,289 @@ fakeFederation }), ); +test( + `${ruleName}: ✅ Good - delivery via a called nested helper`, + lintTest({ + code: ` +import { Activity } from "@fedify/vocab"; + +federation + .setOutboxListeners("/users/{identifier}/outbox") + .on(Activity, async (ctx, activity) => { + async function deliver() { + await ctx.sendActivity( + { identifier: ctx.identifier }, + new URL("https://example.com/inbox"), + activity, + ); + } + await deliver(); + }); +`, + rule, + ruleName, + }), +); + +test( + `${ruleName}: ✅ Good - delivery inside a non-literal if branch`, + lintTest({ + code: ` +import { Activity } from "@fedify/vocab"; + +federation + .setOutboxListeners("/users/{identifier}/outbox") + .on(Activity, async (ctx, activity) => { + if (activity.id != null) { + await ctx.sendActivity( + { identifier: ctx.identifier }, + new URL("https://example.com/inbox"), + activity, + ); + } + }); +`, + rule, + ruleName, + }), +); + +test( + `${ruleName}: ✅ Good - delivery inside try/catch/finally`, + lintTest({ + code: ` +import { Activity } from "@fedify/vocab"; + +federation + .setOutboxListeners("/users/{identifier}/outbox") + .on(Activity, async (ctx, activity) => { + try { + await ctx.sendActivity( + { identifier: ctx.identifier }, + new URL("https://example.com/inbox"), + activity, + ); + } catch (error) { + console.error(error); + } finally { + console.log("done"); + } + }); +`, + rule, + ruleName, + }), +); + +test( + `${ruleName}: ✅ Good - delivery inside a switch case`, + lintTest({ + code: ` +import { Activity } from "@fedify/vocab"; + +federation + .setOutboxListeners("/users/{identifier}/outbox") + .on(Activity, async (ctx, activity) => { + switch (activity.constructor.name) { + case "Create": + await ctx.sendActivity( + { identifier: ctx.identifier }, + new URL("https://example.com/inbox"), + activity, + ); + break; + default: + console.log(ctx.identifier); + } + }); +`, + rule, + ruleName, + }), +); + +test( + `${ruleName}: ✅ Good - delivery inside a for-of loop`, + lintTest({ + code: ` +import { Activity } from "@fedify/vocab"; + +federation + .setOutboxListeners("/users/{identifier}/outbox") + .on(Activity, async (ctx, activity) => { + for (const inbox of [new URL("https://example.com/inbox")]) { + await ctx.sendActivity( + { identifier: ctx.identifier }, + inbox, + activity, + ); + } + }); +`, + rule, + ruleName, + }), +); + +test( + `${ruleName}: ✅ Good - awaited Promise.all(array.map(callback))`, + lintTest({ + code: ` +import { Activity } from "@fedify/vocab"; + +federation + .setOutboxListeners("/users/{identifier}/outbox") + .on(Activity, async (ctx, activity) => { + const recipients = [new URL("https://example.com/inbox")]; + await Promise.all(recipients.map((inbox) => + ctx.sendActivity({ identifier: ctx.identifier }, inbox, activity) + )); + }); +`, + rule, + ruleName, + }), +); + +test( + `${ruleName}: ✅ Good - returned Promise.all(array.map(callback))`, + lintTest({ + code: ` +import { Activity } from "@fedify/vocab"; + +federation + .setOutboxListeners("/users/{identifier}/outbox") + .on(Activity, (ctx, activity) => { + const recipients = [new URL("https://example.com/inbox")]; + return Promise.all(recipients.map((inbox) => + ctx.sendActivity({ identifier: ctx.identifier }, inbox, activity) + )); + }); +`, + rule, + ruleName, + }), +); + +test( + `${ruleName}: ✅ Good - awaited immediately invoked function expression`, + lintTest({ + code: ` +import { Activity } from "@fedify/vocab"; + +federation + .setOutboxListeners("/users/{identifier}/outbox") + .on(Activity, async (ctx, activity) => { + await (async () => { + await ctx.sendActivity( + { identifier: ctx.identifier }, + new URL("https://example.com/inbox"), + activity, + ); + })(); + }); +`, + rule, + ruleName, + }), +); + +test( + `${ruleName}: ✅ Good - named helper passed by reference to forEach`, + lintTest({ + code: ` +import { Activity } from "@fedify/vocab"; + +federation + .setOutboxListeners("/users/{identifier}/outbox") + .on(Activity, async (ctx, activity) => { + const deliver = (inbox) => + ctx.sendActivity({ identifier: ctx.identifier }, inbox, activity); + const inboxes = [new URL("https://example.com/inbox")]; + inboxes.forEach(deliver); + }); +`, + rule, + ruleName, + }), +); + +test( + `${ruleName}: ✅ Good - dollar-prefixed helper name`, + lintTest({ + code: ` +import { Activity } from "@fedify/vocab"; + +federation + .setOutboxListeners("/users/{identifier}/outbox") + .on(Activity, async (ctx, activity) => { + const $deliver = async () => { + await ctx.sendActivity( + { identifier: ctx.identifier }, + new URL("https://example.com/inbox"), + activity, + ); + }; + await $deliver(); + }); +`, + rule, + ruleName, + }), +); + +test( + `${ruleName}: ✅ Good - sibling helper calling a sibling helper`, + lintTest({ + code: ` +import { Activity } from "@fedify/vocab"; + +federation + .setOutboxListeners("/users/{identifier}/outbox") + .on(Activity, async (ctx, activity) => { + async function outer() { + await inner(); + } + async function inner() { + await ctx.sendActivity( + { identifier: ctx.identifier }, + new URL("https://example.com/inbox"), + activity, + ); + } + await outer(); + }); +`, + rule, + ruleName, + }), +); + +test( + `${ruleName}: ✅ Good - helper held in an object literal`, + lintTest({ + code: ` +import { Activity } from "@fedify/vocab"; + +federation + .setOutboxListeners("/users/{identifier}/outbox") + .on(Activity, async (ctx, activity) => { + const handlers = { + deliver: () => + ctx.sendActivity( + { identifier: ctx.identifier }, + new URL("https://example.com/inbox"), + activity, + ), + }; + await handlers.deliver(); + }); +`, + rule, + ruleName, + }), +); + test( `${ruleName}: ❌ Bad - missing delivery call`, lintTest({ @@ -427,3 +710,1379 @@ federation "Outbox listeners should deliver posted activities explicitly with ctx.sendActivity() or ctx.forwardActivity().", }), ); + +test( + `${ruleName}: ❌ Bad - unused nested delivery helper`, + lintTest({ + code: ` +import { Activity } from "@fedify/vocab"; + +federation + .setOutboxListeners("/users/{identifier}/outbox") + .on(Activity, async (ctx, activity) => { + async function deliver() { + await ctx.sendActivity( + { identifier: ctx.identifier }, + new URL("https://example.com/inbox"), + activity, + ); + } + console.log(ctx.identifier, activity.id?.href); + }); +`, + rule, + ruleName, + expectedError: + "Outbox listeners should deliver posted activities explicitly with ctx.sendActivity() or ctx.forwardActivity().", + }), +); + +test( + `${ruleName}: ❌ Bad - delivery call behind if (false)`, + lintTest({ + code: ` +import { Activity } from "@fedify/vocab"; + +federation + .setOutboxListeners("/users/{identifier}/outbox") + .on(Activity, async (ctx, activity) => { + if (false) { + await ctx.sendActivity( + { identifier: ctx.identifier }, + new URL("https://example.com/inbox"), + activity, + ); + } + console.log(ctx.identifier, activity.id?.href); + }); +`, + rule, + ruleName, + expectedError: + "Outbox listeners should deliver posted activities explicitly with ctx.sendActivity() or ctx.forwardActivity().", + }), +); + +test( + `${ruleName}: ❌ Bad - delivery call after unconditional return`, + lintTest({ + code: ` +import { Activity } from "@fedify/vocab"; + +federation + .setOutboxListeners("/users/{identifier}/outbox") + .on(Activity, async (ctx, activity) => { + console.log(ctx.identifier, activity.id?.href); + return; + await ctx.sendActivity( + { identifier: ctx.identifier }, + new URL("https://example.com/inbox"), + activity, + ); + }); +`, + rule, + ruleName, + expectedError: + "Outbox listeners should deliver posted activities explicitly with ctx.sendActivity() or ctx.forwardActivity().", + }), +); + +test( + `${ruleName}: ✅ Good - delivery call inside a callback whose result is dropped`, + lintTest({ + code: ` +import { Activity } from "@fedify/vocab"; + +federation + .setOutboxListeners("/users/{identifier}/outbox") + .on(Activity, async (ctx, activity) => { + const recipients = [new URL("https://example.com/inbox")]; + recipients.map((inbox) => + ctx.sendActivity({ identifier: ctx.identifier }, inbox, activity) + ); + console.log(ctx.identifier, activity.id?.href); + }); +`, + rule, + ruleName, + }), +); + +test( + `${ruleName}: ❌ Bad - delivery call in the dead branch of if (true)`, + lintTest({ + code: ` +import { Activity } from "@fedify/vocab"; + +federation + .setOutboxListeners("/users/{identifier}/outbox") + .on(Activity, async (ctx, activity) => { + if (true) { + console.log(ctx.identifier, activity.id?.href); + } else { + await ctx.sendActivity( + { identifier: ctx.identifier }, + new URL("https://example.com/inbox"), + activity, + ); + } + }); +`, + rule, + ruleName, + expectedError: + "Outbox listeners should deliver posted activities explicitly with ctx.sendActivity() or ctx.forwardActivity().", + }), +); + +test( + `${ruleName}: ❌ Bad - delivery call after if (true) return`, + lintTest({ + code: ` +import { Activity } from "@fedify/vocab"; + +federation + .setOutboxListeners("/users/{identifier}/outbox") + .on(Activity, async (ctx, activity) => { + if (true) { + return; + } + await ctx.sendActivity( + { identifier: ctx.identifier }, + new URL("https://example.com/inbox"), + activity, + ); + }); +`, + rule, + ruleName, + expectedError: + "Outbox listeners should deliver posted activities explicitly with ctx.sendActivity() or ctx.forwardActivity().", + }), +); + +test( + `${ruleName}: ❌ Bad - delivery call after both if/else branches return`, + lintTest({ + code: ` +import { Activity } from "@fedify/vocab"; + +federation + .setOutboxListeners("/users/{identifier}/outbox") + .on(Activity, async (ctx, activity) => { + if (activity.id == null) { + return; + } else { + return; + } + await ctx.sendActivity( + { identifier: ctx.identifier }, + new URL("https://example.com/inbox"), + activity, + ); + }); +`, + rule, + ruleName, + expectedError: + "Outbox listeners should deliver posted activities explicitly with ctx.sendActivity() or ctx.forwardActivity().", + }), +); + +test( + `${ruleName}: ❌ Bad - delivery call after a return in the same switch case`, + lintTest({ + code: ` +import { Activity } from "@fedify/vocab"; + +federation + .setOutboxListeners("/users/{identifier}/outbox") + .on(Activity, async (ctx, activity) => { + switch (activity.id) { + case null: + return; + await ctx.sendActivity( + { identifier: ctx.identifier }, + new URL("https://example.com/inbox"), + activity, + ); + } + }); +`, + rule, + ruleName, + expectedError: + "Outbox listeners should deliver posted activities explicitly with ctx.sendActivity() or ctx.forwardActivity().", + }), +); + +test( + `${ruleName}: ❌ Bad - helper mentioned only in a comment`, + lintTest({ + code: ` +import { Activity } from "@fedify/vocab"; + +federation + .setOutboxListeners("/users/{identifier}/outbox") + .on(Activity, async (ctx, activity) => { + function deliver() { + return ctx.sendActivity( + { identifier: ctx.identifier }, + new URL("https://example.com/inbox"), + activity, + ); + } + // call deliver() later + console.log(ctx.identifier); + }); +`, + rule, + ruleName, + expectedError: + "Outbox listeners should deliver posted activities explicitly with ctx.sendActivity() or ctx.forwardActivity().", + }), +); + +test( + `${ruleName}: ❌ Bad - unrelated method sharing a local helper's name`, + lintTest({ + code: ` +import { Activity } from "@fedify/vocab"; + +federation + .setOutboxListeners("/users/{identifier}/outbox") + .on(Activity, async (ctx, activity) => { + function deliver() { + return ctx.sendActivity( + { identifier: ctx.identifier }, + new URL("https://example.com/inbox"), + activity, + ); + } + const someService = { deliver: async () => {} }; + await someService.deliver(); + }); +`, + rule, + ruleName, + expectedError: + "Outbox listeners should deliver posted activities explicitly with ctx.sendActivity() or ctx.forwardActivity().", + }), +); + +test( + `${ruleName}: ❌ Bad - delivery call after break in a switch case`, + lintTest({ + code: ` +import { Activity } from "@fedify/vocab"; + +federation + .setOutboxListeners("/users/{identifier}/outbox") + .on(Activity, async (ctx, activity) => { + switch (activity.constructor.name) { + case "Create": + break; + await ctx.sendActivity( + { identifier: ctx.identifier }, + new URL("https://example.com/inbox"), + activity, + ); + } + }); +`, + rule, + ruleName, + expectedError: + "Outbox listeners should deliver posted activities explicitly with ctx.sendActivity() or ctx.forwardActivity().", + }), +); + +test( + `${ruleName}: ❌ Bad - delivery call after continue in a loop`, + lintTest({ + code: ` +import { Activity } from "@fedify/vocab"; + +federation + .setOutboxListeners("/users/{identifier}/outbox") + .on(Activity, async (ctx, activity) => { + for (const inbox of [new URL("https://example.com/inbox")]) { + continue; + await ctx.sendActivity( + { identifier: ctx.identifier }, + inbox, + activity, + ); + } + }); +`, + rule, + ruleName, + expectedError: + "Outbox listeners should deliver posted activities explicitly with ctx.sendActivity() or ctx.forwardActivity().", + }), +); + +test( + `${ruleName}: ✅ Good - awaited Promise.all assigned to a variable`, + lintTest({ + code: ` +import { Activity } from "@fedify/vocab"; + +federation + .setOutboxListeners("/users/{identifier}/outbox") + .on(Activity, async (ctx, activity) => { + const recipients = [new URL("https://example.com/inbox")]; + let result; + result = await Promise.all(recipients.map((inbox) => + ctx.sendActivity({ identifier: ctx.identifier }, inbox, activity) + )); + return result; + }); +`, + rule, + ruleName, + }), +); + +test( + `${ruleName}: ❌ Bad - helper only called from a dead branch`, + lintTest({ + code: ` +import { Activity } from "@fedify/vocab"; + +federation + .setOutboxListeners("/users/{identifier}/outbox") + .on(Activity, async (ctx, activity) => { + function deliver() { + return ctx.sendActivity( + { identifier: ctx.identifier }, + new URL("https://example.com/inbox"), + activity, + ); + } + if (false) { + deliver(); + } + }); +`, + rule, + ruleName, + expectedError: + "Outbox listeners should deliver posted activities explicitly with ctx.sendActivity() or ctx.forwardActivity().", + }), +); + +test( + `${ruleName}: ✅ Good - inner helper shadows a same-named outer helper`, + lintTest({ + code: ` +import { Activity } from "@fedify/vocab"; + +federation + .setOutboxListeners("/users/{identifier}/outbox") + .on(Activity, async (ctx, activity) => { + async function outer() { + function deliver() { + return ctx.sendActivity( + { identifier: ctx.identifier }, + new URL("https://example.com/inbox"), + activity, + ); + } + await deliver(); + } + function deliver() { + console.log("outer deliver never actually delivers"); + } + await outer(); + }); +`, + rule, + ruleName, + }), +); + +test( + `${ruleName}: ✅ Good - used helper has byte-identical text to an unused one`, + lintTest({ + code: ` +import { Activity } from "@fedify/vocab"; + +federation + .setOutboxListeners("/users/{identifier}/outbox") + .on(Activity, async (ctx, activity) => { + const inbox = new URL("https://example.com/inbox"); + const handlers = { + unused: () => + ctx.sendActivity({ identifier: ctx.identifier }, inbox, activity), + used: () => + ctx.sendActivity({ identifier: ctx.identifier }, inbox, activity), + }; + await handlers.used(); + }); +`, + rule, + ruleName, + }), +); + +test( + `${ruleName}: ❌ Bad - unused helper has byte-identical text to a used one`, + lintTest({ + code: ` +import { Activity } from "@fedify/vocab"; + +federation + .setOutboxListeners("/users/{identifier}/outbox") + .on(Activity, async (ctx, activity) => { + const inbox = new URL("https://example.com/inbox"); + const handlers = { + used: () => + ctx.sendActivity({ identifier: ctx.identifier }, inbox, activity), + unused: () => + ctx.sendActivity({ identifier: ctx.identifier }, inbox, activity), + }; + console.log("never actually calls a handler"); + }); +`, + rule, + ruleName, + expectedError: + "Outbox listeners should deliver posted activities explicitly with ctx.sendActivity() or ctx.forwardActivity().", + }), +); + +test( + `${ruleName}: ✅ Good - awaited callback nested inside an array literal and spreads`, + lintTest({ + code: ` +import { Activity } from "@fedify/vocab"; + +federation + .setOutboxListeners("/users/{identifier}/outbox") + .on(Activity, async (ctx, activity) => { + const sender = { identifier: ctx.identifier }; + await Promise.all([ + ...inboxes.map((inbox) => ctx.sendActivity(sender, inbox, activity)), + ...others.map((inbox) => ctx.sendActivity(sender, inbox, activity)), + ]); + }); +`, + rule, + ruleName, + }), +); + +test( + `${ruleName}: ✅ Good - awaited callback nested inside an array literal and a chained call`, + lintTest({ + code: ` +import { Activity } from "@fedify/vocab"; + +federation + .setOutboxListeners("/users/{identifier}/outbox") + .on(Activity, async (ctx, activity) => { + const sender = { identifier: ctx.identifier }; + await Promise.all( + [inboxes.map((inbox) => ctx.sendActivity(sender, inbox, activity))] + .flat(), + ); + }); +`, + rule, + ruleName, + }), +); + +test( + `${ruleName}: ✅ Good - awaited callback nested inside an object literal property`, + lintTest({ + code: ` +import { Activity } from "@fedify/vocab"; + +federation + .setOutboxListeners("/users/{identifier}/outbox") + .on(Activity, async (ctx, activity) => { + const sender = { identifier: ctx.identifier }; + await Promise.all( + Object.values({ + a: Promise.all( + inboxes.map((inbox) => ctx.sendActivity(sender, inbox, activity)), + ), + }), + ); + }); +`, + rule, + ruleName, + }), +); + +test( + `${ruleName}: ✅ Good - callback passed to a bare forEach that is never awaited`, + lintTest({ + code: ` +import { Activity } from "@fedify/vocab"; + +federation + .setOutboxListeners("/users/{identifier}/outbox") + .on(Activity, async (ctx, activity) => { + const sender = { identifier: ctx.identifier }; + inboxes.forEach((inbox) => ctx.sendActivity(sender, inbox, activity)); + }); +`, + rule, + ruleName, + }), +); + +test( + `${ruleName}: ❌ Bad - callback passed to a bare forEach that never delivers`, + lintTest({ + code: ` +import { Activity } from "@fedify/vocab"; + +federation + .setOutboxListeners("/users/{identifier}/outbox") + .on(Activity, async (ctx, activity) => { + inboxes.forEach((inbox) => { + console.log(inbox); + }); + }); +`, + rule, + ruleName, + expectedError: + "Outbox listeners should deliver posted activities explicitly with ctx.sendActivity() or ctx.forwardActivity().", + }), +); + +test( + `${ruleName}: ✅ Good - helper reached through an alias of an object property`, + lintTest({ + code: ` +import { Activity } from "@fedify/vocab"; + +federation + .setOutboxListeners("/users/{identifier}/outbox") + .on(Activity, async (ctx, activity) => { + const sender = { identifier: ctx.identifier }; + const inbox = new URL("https://example.com/inbox"); + const handlers = { + deliver: () => ctx.sendActivity(sender, inbox, activity), + }; + const alias = handlers.deliver; + await alias(); + }); +`, + rule, + ruleName, + }), +); + +test( + `${ruleName}: ✅ Good - helper called through a computed member access`, + lintTest({ + code: ` +import { Activity } from "@fedify/vocab"; + +federation + .setOutboxListeners("/users/{identifier}/outbox") + .on(Activity, async (ctx, activity) => { + const sender = { identifier: ctx.identifier }; + const inbox = new URL("https://example.com/inbox"); + const handlers = { + deliver: () => ctx.sendActivity(sender, inbox, activity), + }; + await handlers["deliver"](); + }); +`, + rule, + ruleName, + }), +); + +test( + `${ruleName}: ✅ Good - helper destructured from an object of helpers`, + lintTest({ + code: ` +import { Activity } from "@fedify/vocab"; + +federation + .setOutboxListeners("/users/{identifier}/outbox") + .on(Activity, async (ctx, activity) => { + const sender = { identifier: ctx.identifier }; + const inbox = new URL("https://example.com/inbox"); + const handlers = { + deliver: () => ctx.sendActivity(sender, inbox, activity), + }; + const { deliver } = handlers; + await deliver(); + }); +`, + rule, + ruleName, + }), +); + +test( + `${ruleName}: ✅ Good - helper assigned after its declaration`, + lintTest({ + code: ` +import { Activity } from "@fedify/vocab"; + +federation + .setOutboxListeners("/users/{identifier}/outbox") + .on(Activity, async (ctx, activity) => { + const sender = { identifier: ctx.identifier }; + const inbox = new URL("https://example.com/inbox"); + let deliver; + deliver = () => ctx.sendActivity(sender, inbox, activity); + await deliver(); + }); +`, + rule, + ruleName, + }), +); + +test( + `${ruleName}: ✅ Good - helper declared below an unconditional return`, + lintTest({ + code: ` +import { Activity } from "@fedify/vocab"; + +federation + .setOutboxListeners("/users/{identifier}/outbox") + .on(Activity, async (ctx, activity) => { + const sender = { identifier: ctx.identifier }; + const inbox = new URL("https://example.com/inbox"); + await deliver(); + return; + function deliver() { + return ctx.sendActivity(sender, inbox, activity); + } + }); +`, + rule, + ruleName, + }), +); + +test( + `${ruleName}: ✅ Good - helper stored in an array`, + lintTest({ + code: ` +import { Activity } from "@fedify/vocab"; + +federation + .setOutboxListeners("/users/{identifier}/outbox") + .on(Activity, async (ctx, activity) => { + const sender = { identifier: ctx.identifier }; + const inbox = new URL("https://example.com/inbox"); + const handlers = [() => ctx.sendActivity(sender, inbox, activity)]; + await handlers[0](); + }); +`, + rule, + ruleName, + }), +); + +test( + `${ruleName}: ✅ Good - helper wrapped in a call before being bound`, + lintTest({ + code: ` +import { Activity } from "@fedify/vocab"; + +federation + .setOutboxListeners("/users/{identifier}/outbox") + .on(Activity, async (ctx, activity) => { + const sender = { identifier: ctx.identifier }; + const inbox = new URL("https://example.com/inbox"); + const once = (fn) => fn; + const deliver = once(() => ctx.sendActivity(sender, inbox, activity)); + await deliver(); + }); +`, + rule, + ruleName, + }), +); + +test( + `${ruleName}: ✅ Good - map result kept in a variable before Promise.all`, + lintTest({ + code: ` +import { Activity } from "@fedify/vocab"; + +federation + .setOutboxListeners("/users/{identifier}/outbox") + .on(Activity, async (ctx, activity) => { + const sender = { identifier: ctx.identifier }; + const inbox = new URL("https://example.com/inbox"); + const promises = inboxes.map((target) => + ctx.sendActivity(sender, target, activity) + ); + await Promise.all(promises); + }); +`, + rule, + ruleName, + }), +); + +test( + `${ruleName}: ✅ Good - helper assigned as a property after the object is created`, + lintTest({ + code: ` +import { Activity } from "@fedify/vocab"; + +federation + .setOutboxListeners("/users/{identifier}/outbox") + .on(Activity, async (ctx, activity) => { + const sender = { identifier: ctx.identifier }; + const inbox = new URL("https://example.com/inbox"); + const handlers = {}; + handlers.deliver = () => ctx.sendActivity(sender, inbox, activity); + await handlers.deliver(); + }); +`, + rule, + ruleName, + }), +); + +test( + `${ruleName}: ✅ Good - helper destructured straight from an object literal`, + lintTest({ + code: ` +import { Activity } from "@fedify/vocab"; + +federation + .setOutboxListeners("/users/{identifier}/outbox") + .on(Activity, async (ctx, activity) => { + const sender = { identifier: ctx.identifier }; + const inbox = new URL("https://example.com/inbox"); + const { deliver } = { + deliver: () => ctx.sendActivity(sender, inbox, activity), + }; + await deliver(); + }); +`, + rule, + ruleName, + }), +); + +test( + `${ruleName}: ✅ Good - helper defined as a class method`, + lintTest({ + code: ` +import { Activity } from "@fedify/vocab"; + +federation + .setOutboxListeners("/users/{identifier}/outbox") + .on(Activity, async (ctx, activity) => { + const sender = { identifier: ctx.identifier }; + const inbox = new URL("https://example.com/inbox"); + class Sender { + deliver() { + return ctx.sendActivity(sender, inbox, activity); + } + } + await new Sender().deliver(); + }); +`, + rule, + ruleName, + }), +); + +test( + `${ruleName}: ❌ Bad - helper assigned to a variable but never called`, + lintTest({ + code: ` +import { Activity } from "@fedify/vocab"; + +federation + .setOutboxListeners("/users/{identifier}/outbox") + .on(Activity, async (ctx, activity) => { + const sender = { identifier: ctx.identifier }; + const inbox = new URL("https://example.com/inbox"); + let deliver; + deliver = () => ctx.sendActivity(sender, inbox, activity); + console.log("never called"); + }); +`, + rule, + ruleName, + expectedError: + "Outbox listeners should deliver posted activities explicitly with ctx.sendActivity() or ctx.forwardActivity().", + }), +); + +test( + `${ruleName}: ❌ Bad - helper assigned as a property but never called`, + lintTest({ + code: ` +import { Activity } from "@fedify/vocab"; + +federation + .setOutboxListeners("/users/{identifier}/outbox") + .on(Activity, async (ctx, activity) => { + const sender = { identifier: ctx.identifier }; + const inbox = new URL("https://example.com/inbox"); + const handlers = {}; + handlers.deliver = () => ctx.sendActivity(sender, inbox, activity); + console.log("never called"); + }); +`, + rule, + ruleName, + expectedError: + "Outbox listeners should deliver posted activities explicitly with ctx.sendActivity() or ctx.forwardActivity().", + }), +); + +test( + `${ruleName}: ✅ Good - callback passed to map whose result is never used`, + lintTest({ + code: ` +import { Activity } from "@fedify/vocab"; + +federation + .setOutboxListeners("/users/{identifier}/outbox") + .on(Activity, async (ctx, activity) => { + const sender = { identifier: ctx.identifier }; + const inbox = new URL("https://example.com/inbox"); + const promises = inboxes.map((target) => + ctx.sendActivity(sender, target, activity) + ); + console.log("never awaited"); + }); +`, + rule, + ruleName, + }), +); + +test( + `${ruleName}: ❌ Bad - helper declared below a return but never called`, + lintTest({ + code: ` +import { Activity } from "@fedify/vocab"; + +federation + .setOutboxListeners("/users/{identifier}/outbox") + .on(Activity, async (ctx, activity) => { + const sender = { identifier: ctx.identifier }; + const inbox = new URL("https://example.com/inbox"); + console.log("nothing below runs"); + return; + function deliver() { + return ctx.sendActivity(sender, inbox, activity); + } + }); +`, + rule, + ruleName, + expectedError: + "Outbox listeners should deliver posted activities explicitly with ctx.sendActivity() or ctx.forwardActivity().", + }), +); + +test( + `${ruleName}: ❌ Bad - nested helper never called by the helper that declares it`, + lintTest({ + code: ` +import { Activity } from "@fedify/vocab"; + +federation + .setOutboxListeners("/users/{identifier}/outbox") + .on(Activity, async (ctx, activity) => { + const sender = { identifier: ctx.identifier }; + const inbox = new URL("https://example.com/inbox"); + async function outer() { + function inner() { + return ctx.sendActivity(sender, inbox, activity); + } + console.log("never calls inner"); + } + await outer(); + }); +`, + rule, + ruleName, + expectedError: + "Outbox listeners should deliver posted activities explicitly with ctx.sendActivity() or ctx.forwardActivity().", + }), +); + +test( + `${ruleName}: ❌ Bad - class whose methods are never used`, + lintTest({ + code: ` +import { Activity } from "@fedify/vocab"; + +federation + .setOutboxListeners("/users/{identifier}/outbox") + .on(Activity, async (ctx, activity) => { + const sender = { identifier: ctx.identifier }; + const inbox = new URL("https://example.com/inbox"); + class Sender { + deliver() { + return ctx.sendActivity(sender, inbox, activity); + } + } + console.log("never instantiated"); + }); +`, + rule, + ruleName, + expectedError: + "Outbox listeners should deliver posted activities explicitly with ctx.sendActivity() or ctx.forwardActivity().", + }), +); + +test( + `${ruleName}: ✅ Good - delivery call in an if test`, + lintTest({ + code: ` +import { Activity } from "@fedify/vocab"; + +federation + .setOutboxListeners("/users/{identifier}/outbox") + .on(Activity, async (ctx, activity) => { + const sender = { identifier: ctx.identifier }; + const inbox = new URL("https://example.com/inbox"); + if (await ctx.sendActivity(sender, inbox, activity)) { + console.log("sent"); + } + }); +`, + rule, + ruleName, + }), +); + +test( + `${ruleName}: ✅ Good - helper call in a switch discriminant`, + lintTest({ + code: ` +import { Activity } from "@fedify/vocab"; + +federation + .setOutboxListeners("/users/{identifier}/outbox") + .on(Activity, async (ctx, activity) => { + const sender = { identifier: ctx.identifier }; + const inbox = new URL("https://example.com/inbox"); + async function deliver() { + await ctx.sendActivity(sender, inbox, activity); + return 1; + } + switch (await deliver()) { + case 1: + break; + } + }); +`, + rule, + ruleName, + }), +); + +test( + `${ruleName}: ✅ Good - helper call in a switch case test`, + lintTest({ + code: ` +import { Activity } from "@fedify/vocab"; + +federation + .setOutboxListeners("/users/{identifier}/outbox") + .on(Activity, async (ctx, activity) => { + const sender = { identifier: ctx.identifier }; + const inbox = new URL("https://example.com/inbox"); + async function deliver() { + await ctx.sendActivity(sender, inbox, activity); + return 1; + } + switch (1) { + case await deliver(): + break; + } + }); +`, + rule, + ruleName, + }), +); + +test( + `${ruleName}: ✅ Good - helper call in a while test`, + lintTest({ + code: ` +import { Activity } from "@fedify/vocab"; + +federation + .setOutboxListeners("/users/{identifier}/outbox") + .on(Activity, async (ctx, activity) => { + const sender = { identifier: ctx.identifier }; + const inbox = new URL("https://example.com/inbox"); + async function deliver() { + await ctx.sendActivity(sender, inbox, activity); + return 1; + } + while (await deliver() > 1) { + console.log("looping"); + } + }); +`, + rule, + ruleName, + }), +); + +test( + `${ruleName}: ✅ Good - helper call in a do-while test`, + lintTest({ + code: ` +import { Activity } from "@fedify/vocab"; + +federation + .setOutboxListeners("/users/{identifier}/outbox") + .on(Activity, async (ctx, activity) => { + const sender = { identifier: ctx.identifier }; + const inbox = new URL("https://example.com/inbox"); + async function deliver() { + await ctx.sendActivity(sender, inbox, activity); + return 1; + } + do { + console.log("once"); + } while (await deliver() > 1); + }); +`, + rule, + ruleName, + }), +); + +test( + `${ruleName}: ✅ Good - delivery call in a for loop init`, + lintTest({ + code: ` +import { Activity } from "@fedify/vocab"; + +federation + .setOutboxListeners("/users/{identifier}/outbox") + .on(Activity, async (ctx, activity) => { + const sender = { identifier: ctx.identifier }; + const inbox = new URL("https://example.com/inbox"); + for (let sent = await ctx.sendActivity(sender, inbox, activity); false;) { + console.log(sent); + } + }); +`, + rule, + ruleName, + }), +); + +test( + `${ruleName}: ✅ Good - helper call in a for loop test`, + lintTest({ + code: ` +import { Activity } from "@fedify/vocab"; + +federation + .setOutboxListeners("/users/{identifier}/outbox") + .on(Activity, async (ctx, activity) => { + const sender = { identifier: ctx.identifier }; + const inbox = new URL("https://example.com/inbox"); + async function deliver() { + await ctx.sendActivity(sender, inbox, activity); + return 1; + } + for (let i = 0; i < await deliver(); i++) { + console.log(i); + } + }); +`, + rule, + ruleName, + }), +); + +test( + `${ruleName}: ✅ Good - helper call in a for loop update`, + lintTest({ + code: ` +import { Activity } from "@fedify/vocab"; + +federation + .setOutboxListeners("/users/{identifier}/outbox") + .on(Activity, async (ctx, activity) => { + const sender = { identifier: ctx.identifier }; + const inbox = new URL("https://example.com/inbox"); + async function deliver() { + await ctx.sendActivity(sender, inbox, activity); + return 1; + } + for (let i = 0; i < 1; i += await deliver()) { + console.log(i); + } + }); +`, + rule, + ruleName, + }), +); + +test( + `${ruleName}: ✅ Good - helpers held in an array and run by a for-of loop`, + lintTest({ + code: ` +import { Activity } from "@fedify/vocab"; + +federation + .setOutboxListeners("/users/{identifier}/outbox") + .on(Activity, async (ctx, activity) => { + const sender = { identifier: ctx.identifier }; + const inbox = new URL("https://example.com/inbox"); + const jobs = [ + () => ctx.sendActivity(sender, inbox, activity), + () => ctx.sendActivity(sender, inbox, activity), + ]; + for (const job of jobs) await job(); + }); +`, + rule, + ruleName, + }), +); + +test( + `${ruleName}: ✅ Good - awaited callback inside an if test`, + lintTest({ + code: ` +import { Activity } from "@fedify/vocab"; + +federation + .setOutboxListeners("/users/{identifier}/outbox") + .on(Activity, async (ctx, activity) => { + const sender = { identifier: ctx.identifier }; + const inbox = new URL("https://example.com/inbox"); + if (await Promise.all(inboxes.map((target) => + ctx.sendActivity(sender, target, activity) + ))) { + console.log("sent"); + } + }); +`, + rule, + ruleName, + }), +); + +test( + `${ruleName}: ❌ Bad - unrelated helper called in an if test`, + lintTest({ + code: ` +import { Activity } from "@fedify/vocab"; + +federation + .setOutboxListeners("/users/{identifier}/outbox") + .on(Activity, async (ctx, activity) => { + const sender = { identifier: ctx.identifier }; + const inbox = new URL("https://example.com/inbox"); + async function check() { + return true; + } + async function deliver() { + await ctx.sendActivity(sender, inbox, activity); + } + if (await check()) { + console.log("checked"); + } + }); +`, + rule, + ruleName, + expectedError: + "Outbox listeners should deliver posted activities explicitly with ctx.sendActivity() or ctx.forwardActivity().", + }), +); + +test( + `${ruleName}: ❌ Bad - unrelated for-of head next to an unused helper`, + lintTest({ + code: ` +import { Activity } from "@fedify/vocab"; + +federation + .setOutboxListeners("/users/{identifier}/outbox") + .on(Activity, async (ctx, activity) => { + const sender = { identifier: ctx.identifier }; + const inbox = new URL("https://example.com/inbox"); + const deliver = () => ctx.sendActivity(sender, inbox, activity); + for (const target of [inbox]) { + console.log(target); + } + }); +`, + rule, + ruleName, + expectedError: + "Outbox listeners should deliver posted activities explicitly with ctx.sendActivity() or ctx.forwardActivity().", + }), +); + +test( + `${ruleName}: ❌ Bad - delivery call in the branch behind an if test that is scanned`, + lintTest({ + code: ` +import { Activity } from "@fedify/vocab"; + +federation + .setOutboxListeners("/users/{identifier}/outbox") + .on(Activity, async (ctx, activity) => { + const sender = { identifier: ctx.identifier }; + const inbox = new URL("https://example.com/inbox"); + if (await Promise.resolve(false)) { + return; + } + if (false) { + await ctx.sendActivity(sender, inbox, activity); + } + }); +`, + rule, + ruleName, + expectedError: + "Outbox listeners should deliver posted activities explicitly with ctx.sendActivity() or ctx.forwardActivity().", + }), +); + +test( + `${ruleName}: ✅ Good - callback passed to queue.push`, + lintTest({ + code: ` +import { Activity } from "@fedify/vocab"; + +federation + .setOutboxListeners("/users/{identifier}/outbox") + .on(Activity, async (ctx, activity) => { + const sender = { identifier: ctx.identifier }; + const inbox = new URL("https://example.com/inbox"); + const queue = []; + queue.push(() => ctx.sendActivity(sender, inbox, activity)); + }); +`, + rule, + ruleName, + }), +); + +test( + `${ruleName}: ✅ Good - callback passed to setTimeout`, + lintTest({ + code: ` +import { Activity } from "@fedify/vocab"; + +federation + .setOutboxListeners("/users/{identifier}/outbox") + .on(Activity, async (ctx, activity) => { + const sender = { identifier: ctx.identifier }; + const inbox = new URL("https://example.com/inbox"); + setTimeout(() => ctx.sendActivity(sender, inbox, activity), 0); + }); +`, + rule, + ruleName, + }), +); + +test( + `${ruleName}: ✅ Good - callback passed to a then that is not awaited`, + lintTest({ + code: ` +import { Activity } from "@fedify/vocab"; + +federation + .setOutboxListeners("/users/{identifier}/outbox") + .on(Activity, async (ctx, activity) => { + const sender = { identifier: ctx.identifier }; + const inbox = new URL("https://example.com/inbox"); + Promise.resolve().then(() => ctx.sendActivity(sender, inbox, activity)); + }); +`, + rule, + ruleName, + }), +); + +test( + `${ruleName}: ✅ Good - callback inside an object passed to a call`, + lintTest({ + code: ` +import { Activity } from "@fedify/vocab"; + +federation + .setOutboxListeners("/users/{identifier}/outbox") + .on(Activity, async (ctx, activity) => { + const sender = { identifier: ctx.identifier }; + const inbox = new URL("https://example.com/inbox"); + const handlers = {}; + Object.assign(handlers, { + deliver: () => ctx.sendActivity(sender, inbox, activity), + }); + }); +`, + rule, + ruleName, + }), +); + +test( + `${ruleName}: ✅ Good - delivery call that is never awaited`, + lintTest({ + code: ` +import { Activity } from "@fedify/vocab"; + +federation + .setOutboxListeners("/users/{identifier}/outbox") + .on(Activity, async (ctx, activity) => { + const sender = { identifier: ctx.identifier }; + const inbox = new URL("https://example.com/inbox"); + ctx.sendActivity(sender, inbox, activity); + }); +`, + rule, + ruleName, + }), +); + +test( + `${ruleName}: ❌ Bad - callback passed to a call that never delivers`, + lintTest({ + code: ` +import { Activity } from "@fedify/vocab"; + +federation + .setOutboxListeners("/users/{identifier}/outbox") + .on(Activity, async (ctx, activity) => { + const sender = { identifier: ctx.identifier }; + const inbox = new URL("https://example.com/inbox"); + const queue = []; + queue.push(() => console.log("no delivery in here")); + }); +`, + rule, + ruleName, + expectedError: + "Outbox listeners should deliver posted activities explicitly with ctx.sendActivity() or ctx.forwardActivity().", + }), +);