fix: short-circuit && and || - #64
Open
bryan-ferry wants to merge 1 commit into
Open
bryan-ferry wants to merge 1 commit into
bryan-ferry wants to merge 1 commit into
Conversation
`a && b` and `a || b` evaluated both operands before applying the op, so `obj && obj.prop` threw on a null `obj` and right-hand side effects ran unconditionally. Only `??` was lazy. Extend the existing `??` guard in execSync/execAsync to return `a` early when it already decides the result: `&&` on a falsy `a`, `||` on a truthy `a`. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
This branch has not been deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
a && banda || bevaluated both operands before applying the op, soobj && obj.propthrew on a nullobjand right-hand side effects ran unconditionally. Only??was lazy.Fixes #63.
Fix
Extend the existing
??short-circuit guard inexecSync/execAsync(src/executor/executorUtils.ts) so it also returnsaearly for&&whenais falsy and for||whenais truthy.bis only evaluated when it is needed. The op handlers insrc/executor/ops/comparison.tsare unchanged and still produce the result in that case.Tests
New
test/logicalShortCircuit.spec.ts, 12 cases:null,undefined,0,"",false,NaN) return the left value and never evaluate the right1,"x",true,[],{}) for||likewiseobj && obj.propand!obj || obj.propguard patterns no longer throwhit(1) && hit(0) && hit(2)runs two calls)??regression: still short-circuits only onnull/undefinedcompileAsyncpath, including an awaited left operand9 of the 12 fail on
main; all pass with the fix.Verification
npm test: 52 suites / 1923 tests pass (was 51 / 1911)npm run lint: 0 errors, warnings unchanged (pre-existingno-explicit-any)tsc --noEmitandnpm run build: cleanNot in this PR
&&=,||=and??=evaluate their right-hand side eagerly as well (details in #63). The same guard shape would cover them; this PR is kept to the two operators.🤖 Generated with Claude Code