Why
outbox-listener-delivery-required and outbox-listener-delivery-not-awaited both decide which code in an outbox listener can run through collectReachableStatements() in packages/lint/src/lib/reachability.ts. The function already skips an if branch whose test is statically false, but two loop forms are handled inaccurately. CodeRabbit pointed both out while reviewing #1067, where the function was moved into the shared module unchanged; the behavior dates back to #1050.
First, a loop whose test is statically false still has its body and update treated as reachable:
while (false) { ctx.sendActivity(sender, inbox, activity); }
for (;false;) { ctx.sendActivity(sender, inbox, activity); }
for (let i = 0; false; ctx.sendActivity(sender, inbox, activity)) {}None of these ever delivers, yet outbox-listener-delivery-required accepts all three listeners, and outbox-listener-delivery-not-awaited reports the unawaited call in the first two although it never runs. A do…while (false) body runs once, and both rules already handle it correctly.
Second, for…of and for…in visit only right and body, so an expression evaluated inside the loop variable's binding pattern is never seen:
for (const { id = ctx.sendActivity(sender, inbox, activity) } of [{}]) {}This listener does deliver, but outbox-listener-delivery-required reports it as not delivering, and outbox-listener-delivery-not-awaited misses the dropped promise.
I checked each case against the current rules. They are rare in real code, which is why they were left out of #1067, but the fix is contained in one function.
Scope
In collectReachableStatements():
- For
while and for, skip the body when the test is statically false, and for for also skip the update, while keeping the initializer and the test. Keep the do…while body, which always runs once.
- For
for…of and for…in, include the expressions evaluated in left, such as default values and computed keys in a binding pattern, without treating the bound names as reads.
Use the existing isStaticallyFalsy() helper that the if handling already uses.
Suggested checks
Add tests for both rules covering each example above, plus a do…while (false) case that stays as it is. The shared behavior is easiest to cover through the tests of the existing rule, the same way #1067 covered the rest of lib/reachability.ts. This changes what outbox-listener-delivery-required reports, so it needs a changelog fragment for @fedify/lint.
Why
outbox-listener-delivery-requiredandoutbox-listener-delivery-not-awaitedboth decide which code in an outbox listener can run throughcollectReachableStatements()in packages/lint/src/lib/reachability.ts. The function already skips anifbranch whose test is statically false, but two loop forms are handled inaccurately. CodeRabbit pointed both out while reviewing #1067, where the function was moved into the shared module unchanged; the behavior dates back to #1050.First, a loop whose test is statically false still has its body and update treated as reachable:
None of these ever delivers, yet
outbox-listener-delivery-requiredaccepts all three listeners, andoutbox-listener-delivery-not-awaitedreports the unawaited call in the first two although it never runs. Ado…while (false)body runs once, and both rules already handle it correctly.Second,
for…ofandfor…invisit onlyrightandbody, so an expression evaluated inside the loop variable's binding pattern is never seen:This listener does deliver, but
outbox-listener-delivery-requiredreports it as not delivering, andoutbox-listener-delivery-not-awaitedmisses the dropped promise.I checked each case against the current rules. They are rare in real code, which is why they were left out of #1067, but the fix is contained in one function.
Scope
In
collectReachableStatements():whileandfor, skip the body when the test is statically false, and forforalso skip the update, while keeping the initializer and the test. Keep thedo…whilebody, which always runs once.for…ofandfor…in, include the expressions evaluated inleft, such as default values and computed keys in a binding pattern, without treating the bound names as reads.Use the existing
isStaticallyFalsy()helper that theifhandling already uses.Suggested checks
Add tests for both rules covering each example above, plus a
do…while (false)case that stays as it is. The shared behavior is easiest to cover through the tests of the existing rule, the same way #1067 covered the rest of lib/reachability.ts. This changes whatoutbox-listener-delivery-requiredreports, so it needs a changelog fragment for@fedify/lint.