next.js
1095b9eb - Fix short circuit evaluation of AND / OR in the Turbopack analyzer for string & nullish-related methods (#94159)

Commit
57 days ago
Fix short circuit evaluation of AND / OR in the Turbopack analyzer for string & nullish-related methods (#94159) Here is the original description of the bug: ``` 1. shortcircuit_if_known predicates are swapped for Logical(And) and Logical(Or) (lines 2541–2548, 2573–2582, 2634–2643) is_nullish, is_string, and is_empty_string all do this for AND/OR: LogicalOperator::And => shortcircuit_if_known(list, JsValue::is_truthy, …), LogicalOperator::Or => shortcircuit_if_known(list, JsValue::is_falsy, …), shortcircuit_if_known returns item_value(item) for the first item where use_item == Some(true). But: - a && b returns a when a is falsy, else b → use_item should be is_falsy. - a || b returns a when a is truthy, else b → use_item should be is_truthy. Concretely: - is_nullish(1 && null): code does is_truthy(1) = Some(true) → return is_nullish(1) = Some(false). JS answer is true. Wrong. - is_nullish(null || 2): code does is_falsy(null) = Some(true) → return is_nullish(null) = Some(true). JS answer is false. Wrong. ``` As mentioned, in the issue `is_string`, `is_empty_string`, and `is_nullish`'s short circuit evaluation of `&&` and `||` was reversed. This PR reverses them so that for AND expressions, the first false-y value is evaluated (otherwise the last value is). For OR expressions, the first truth-y value is evaluated (otherwise the last value is). I’ve added unit tests to this PR. To verify these unit tests with JavaScript’s behaviour I used Claude Code to generate this node script: ```javascript const PREDICATES = { is_string: (v) => typeof v === 'string', is_empty_string: (v) => v === '', is_nullish: (v) => v == null, is_not_nullish: (v) => v != null, } const cases = [ { test: 'is_string_constant', rust: `assert_eq!(value.is_string(), Some(true));`, expr: `'hello'`, predicate: 'is_string', expected: true, }, { test: 'is_string_and_short_circuit', rust: `assert_eq!(eval("'hello' && 2").is_string(), Some(false));`, expr: `'hello' && 2`, predicate: 'is_string', expected: false, }, { test: 'is_string_and_short_circuit', rust: `assert_eq!(eval("2 && 1 && 'hello'").is_string(), Some(true));`, expr: `2 && 1 && 'hello'`, predicate: 'is_string', expected: true, }, { test: 'is_string_or_short_circuit', rust: `assert_eq!(eval("'hello' || 'bye' || 2").is_string(), Some(true));`, expr: `'hello' || 'bye' || 2`, predicate: 'is_string', expected: true, }, { test: 'is_string_or_short_circuit', rust: `assert_eq!(eval("'hello' || 2 || 1 || 'bye'").is_string(), Some(true));`, expr: `'hello' || 2 || 1 || 'bye'`, predicate: 'is_string', expected: true, }, { test: 'is_string_or_short_circuit', rust: `assert_eq!(eval("2 || 1 || 'hello' || 'bye'").is_string(), Some(false));`, expr: `2 || 1 || 'hello' || 'bye'`, predicate: 'is_string', expected: false, }, { test: 'is_empty_string_and_short_circuit', rust: `assert_eq!(eval("'' && 'string'").is_empty_string(), Some(true));`, expr: `'' && 'string'`, predicate: 'is_empty_string', expected: true, }, { test: 'is_empty_string_and_short_circuit', rust: `assert_eq!(eval("false && ''").is_empty_string(), Some(false));`, expr: `false && ''`, predicate: 'is_empty_string', expected: false, }, { test: 'is_empty_string_and_short_circuit', rust: `assert_eq!(eval("0 && false && ''").is_empty_string(), Some(false));`, expr: `0 && false && ''`, predicate: 'is_empty_string', expected: false, }, { test: 'is_empty_string_or_short_circuit', rust: `assert_eq!(eval("'' || 'string'").is_empty_string(), Some(false));`, expr: `'' || 'string'`, predicate: 'is_empty_string', expected: false, }, { test: 'is_empty_string_or_short_circuit', rust: `assert_eq!(eval("false || ''").is_empty_string(), Some(true));`, expr: `false || ''`, predicate: 'is_empty_string', expected: true, }, { test: 'is_empty_string_or_short_circuit', rust: `assert_eq!(eval("0 || false || ''").is_empty_string(), Some(true));`, expr: `0 || false || ''`, predicate: 'is_empty_string', expected: true, }, { test: 'is_nullish_and_short_circuit', rust: `assert_eq!(eval("'' && null").is_nullish(), Some(false));`, expr: `'' && null`, predicate: 'is_nullish', expected: false, }, { test: 'is_nullish_and_short_circuit', rust: `assert_eq!(eval("null && ''").is_nullish(), Some(true));`, expr: `null && ''`, predicate: 'is_nullish', expected: true, }, { test: 'is_nullish_and_short_circuit', rust: `assert_eq!(eval("0 && null && ''").is_nullish(), Some(false));`, expr: `0 && null && ''`, predicate: 'is_nullish', expected: false, }, { test: 'is_nullish_and_short_circuit', rust: `assert_eq!(eval("null && 1 && 2").is_nullish(), Some(true));`, expr: `null && 1 && 2`, predicate: 'is_nullish', expected: true, }, { test: 'is_nullish_or_short_circuit', rust: `assert_eq!(eval("'' || null").is_nullish(), Some(true));`, expr: `'' || null`, predicate: 'is_nullish', expected: true, }, { test: 'is_nullish_or_short_circuit', rust: `assert_eq!(eval("null || ''").is_nullish(), Some(false));`, expr: `null || ''`, predicate: 'is_nullish', expected: false, }, { test: 'is_nullish_or_short_circuit', rust: `assert_eq!(eval("0 || '' || null").is_nullish(), Some(true));`, expr: `0 || '' || null`, predicate: 'is_nullish', expected: true, }, { test: 'is_nullish_or_short_circuit', rust: `assert_eq!(eval("null || 1 || 2").is_nullish(), Some(false));`, expr: `null || 1 || 2`, predicate: 'is_nullish', expected: false, }, { test: 'is_not_nullish_and_short_circuit', rust: `assert_eq!(eval("'' && null").is_not_nullish(), Some(true));`, expr: `'' && null`, predicate: 'is_not_nullish', expected: true, }, { test: 'is_not_nullish_and_short_circuit', rust: `assert_eq!(eval("null && ''").is_not_nullish(), Some(false));`, expr: `null && ''`, predicate: 'is_not_nullish', expected: false, }, { test: 'is_not_nullish_and_short_circuit', rust: `assert_eq!(eval("0 && null && ''").is_not_nullish(), Some(true));`, expr: `0 && null && ''`, predicate: 'is_not_nullish', expected: true, }, { test: 'is_not_nullish_and_short_circuit', rust: `assert_eq!(eval("null && 1 && 2").is_not_nullish(), Some(false));`, expr: `null && 1 && 2`, predicate: 'is_not_nullish', expected: false, }, { test: 'is_not_nullish_or_short_circuit', rust: `assert_eq!(eval("'' || null").is_not_nullish(), Some(false));`, expr: `'' || null`, predicate: 'is_not_nullish', expected: false, }, { test: 'is_not_nullish_or_short_circuit', rust: `assert_eq!(eval("null || ''").is_not_nullish(), Some(true));`, expr: `null || ''`, predicate: 'is_not_nullish', expected: true, }, { test: 'is_not_nullish_or_short_circuit', rust: `assert_eq!(eval("0 || '' || null").is_not_nullish(), Some(false));`, expr: `0 || '' || null`, predicate: 'is_not_nullish', expected: false, }, { test: 'is_not_nullish_or_short_circuit', rust: `assert_eq!(eval("null || 1 || 2").is_not_nullish(), Some(true));`, expr: `null || 1 || 2`, predicate: 'is_not_nullish', expected: true, }, ] let passed = 0 let failed = 0 let currentTest = null for (const c of cases) { if (c.test !== currentTest) { console.log(`\n#[test] fn ${c.test}()`) currentTest = c.test } console.log(` ${c.rust}`) const result = (0, eval)(c.expr) const actual = PREDICATES[c.predicate](result) const ok = actual === c.expected const status = ok ? 'PASS' : 'FAIL' console.log( ` -> JS result = ${JSON.stringify(result)}, ${c.predicate} = ${actual}, expected = ${c.expected} [${status}]` ) if (ok) passed++ else failed++ } console.log(`\n${passed} passed, ${failed} failed`) ``` After running it on my machine: ``` 28 passed, 0 failed ``` --------- Co-authored-by: vercel[bot] <35613825+vercel[bot]@users.noreply.github.com> Co-authored-by: Luke Sandberg <lukesandberg@users.noreply.github.com>
Author
Parents
Loading