From 80d1045fb699e41956e491b998298acf066a925b Mon Sep 17 00:00:00 2001 From: Sam Poder Date: Thu, 28 May 2026 19:50:50 -0700 Subject: [PATCH 1/2] Update `shortcircuit_if_known` to `eval_shortcircuit()` (#94163) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Originally this PR was a comment fix that was flagged by @bgw's Claude: ``` ▎ "Otherwise returns None." The function actually returns item_value(last_item) when no element matches. Comment doesn't describe behavior. --- The two most consequential bugs are #1 (swapped AND/OR predicates) and #2 (regex). #1 silently produces wrong nullishness/string-ness results for any code path that asks about &&/|| chains and feeds into linker decisions; #2 silently corrupts every default-filter require.context resolution. ``` Related to #94159, updates the comment to say that it returns the last item. Which appears to be what is happening: ```rust if it.peek().is_none() { return item_value(item); } ``` Afterwards, it now includes a reworking of `shortcircuit_if_known` into `eval_shortcircuit()`: https://github.com/vercel/next.js/pull/94163#discussion_r3313537931 --- .../turbopack-ecmascript/src/analyzer/mod.rs | 45 ++++++++----------- 1 file changed, 18 insertions(+), 27 deletions(-) diff --git a/turbopack/crates/turbopack-ecmascript/src/analyzer/mod.rs b/turbopack/crates/turbopack-ecmascript/src/analyzer/mod.rs index 8b8ca0c8954..4a4f0991d4c 100644 --- a/turbopack/crates/turbopack-ecmascript/src/analyzer/mod.rs +++ b/turbopack/crates/turbopack-ecmascript/src/analyzer/mod.rs @@ -2449,7 +2449,7 @@ impl JsValue { LogicalOperator::And => all_if_known(list, JsValue::is_truthy), LogicalOperator::Or => any_if_known(list, JsValue::is_truthy), LogicalOperator::NullishCoalescing => { - shortcircuit_if_known(list, JsValue::is_not_nullish, JsValue::is_truthy) + eval_shortcircuit(list, JsValue::is_not_nullish)?.is_truthy() } }, JsValue::Binary(_, box a, op, box b) => { @@ -2527,12 +2527,8 @@ impl JsValue { _ => merge_if_known(values, JsValue::is_nullish), }, JsValue::Logical(_, op, list) => match op { - LogicalOperator::And => { - shortcircuit_if_known(list, JsValue::is_falsy, JsValue::is_nullish) - } - LogicalOperator::Or => { - shortcircuit_if_known(list, JsValue::is_truthy, JsValue::is_nullish) - } + LogicalOperator::And => eval_shortcircuit(list, JsValue::is_falsy)?.is_nullish(), + LogicalOperator::Or => eval_shortcircuit(list, JsValue::is_truthy)?.is_nullish(), LogicalOperator::NullishCoalescing => all_if_known(list, JsValue::is_nullish), }, _ => None, @@ -2560,13 +2556,13 @@ impl JsValue { } => merge_if_known(values, JsValue::is_empty_string), JsValue::Logical(_, op, list) => match op { LogicalOperator::And => { - shortcircuit_if_known(list, JsValue::is_falsy, JsValue::is_empty_string) + eval_shortcircuit(list, JsValue::is_falsy)?.is_empty_string() } LogicalOperator::Or => { - shortcircuit_if_known(list, JsValue::is_truthy, JsValue::is_empty_string) + eval_shortcircuit(list, JsValue::is_truthy)?.is_empty_string() } LogicalOperator::NullishCoalescing => { - shortcircuit_if_known(list, JsValue::is_not_nullish, JsValue::is_empty_string) + eval_shortcircuit(list, JsValue::is_not_nullish)?.is_empty_string() } }, // Booleans are not empty strings @@ -2620,14 +2616,10 @@ impl JsValue { JsValue::Add(_, list) => any_if_known(list, JsValue::is_string), JsValue::Logical(_, op, list) => match op { - LogicalOperator::And => { - shortcircuit_if_known(list, JsValue::is_falsy, JsValue::is_string) - } - LogicalOperator::Or => { - shortcircuit_if_known(list, JsValue::is_truthy, JsValue::is_string) - } + LogicalOperator::And => eval_shortcircuit(list, JsValue::is_falsy)?.is_string(), + LogicalOperator::Or => eval_shortcircuit(list, JsValue::is_truthy)?.is_string(), LogicalOperator::NullishCoalescing => { - shortcircuit_if_known(list, JsValue::is_not_nullish, JsValue::is_string) + eval_shortcircuit(list, JsValue::is_not_nullish)?.is_string() } }, @@ -2790,26 +2782,25 @@ fn any_if_known( all_if_known(list, |x| func(x).map(|x| !x)).map(|x| !x) } -/// Selects the first element of the list where `use_item` is compile-time true. -/// For this element returns the result of `item_value`. Otherwise returns None. -fn shortcircuit_if_known( +/// Selects the first element of the list where `matches` is compile-time true. +/// Returns this element; if no elements match, it returns the last item. +fn eval_shortcircuit( list: impl IntoIterator, - use_item: impl Fn(T) -> Option, - item_value: impl FnOnce(T) -> Option, -) -> Option { + matches: impl Fn(T) -> Option, +) -> Option { let mut it = list.into_iter().peekable(); while let Some(item) = it.next() { if it.peek().is_none() { - return item_value(item); + return Some(item); } else { - match use_item(item) { - Some(true) => return item_value(item), + match matches(item) { + Some(true) => return Some(item), None => return None, _ => {} } } } - None + unreachable!("Binary operators should always have operands.") } // Visiting From 418293859cc5a712a26036723f0661ad842cad52 Mon Sep 17 00:00:00 2001 From: Sam Poder Date: Thu, 28 May 2026 19:51:27 -0700 Subject: [PATCH 2/2] Remove dead code from `JsValue::Binary` in `is_truthy` (#94165) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This is another thing flagged by @bgw's Claude: ``` 3. Dead duplicate match arm in is_truthy for StrictEqual (lines 2469–2494) ( … StrictEqual, Constant(a), Constant(b)) if a.is_value_type() => Some(a == b), ( … StrictEqual, Constant(a), Constant(b)) if a.is_value_type() => { /* same-type check */ } Identical pattern + guard. The second arm — the only one that handles cross-type equality conservatively — is unreachable. Looks like a half-applied fix. ``` I've removed the second arm with a double type check. Tests appear to still pass and I can't think of a case where we'd hit the second arm. --- .../turbopack-ecmascript/src/analyzer/mod.rs | 19 ------------------- 1 file changed, 19 deletions(-) diff --git a/turbopack/crates/turbopack-ecmascript/src/analyzer/mod.rs b/turbopack/crates/turbopack-ecmascript/src/analyzer/mod.rs index 4a4f0991d4c..dc523d0988c 100644 --- a/turbopack/crates/turbopack-ecmascript/src/analyzer/mod.rs +++ b/turbopack/crates/turbopack-ecmascript/src/analyzer/mod.rs @@ -2460,25 +2460,6 @@ impl JsValue { JsValue::Constant(a), JsValue::Constant(b), ) if a.is_value_type() => Some(a == b), - ( - PositiveBinaryOperator::StrictEqual, - JsValue::Constant(a), - JsValue::Constant(b), - ) if a.is_value_type() => { - let same_type = { - use ConstantValue::*; - matches!( - (a, b), - (Num(_), Num(_)) - | (Str(_), Str(_)) - | (BigInt(_), BigInt(_)) - | (True | False, True | False) - | (Undefined, Undefined) - | (Null, Null) - ) - }; - if same_type { Some(a == b) } else { None } - } ( PositiveBinaryOperator::Equal, JsValue::Constant(ConstantValue::Str(a)),