Fix short circuit evaluation of AND / OR in the Turbopack analyzer for string & nullish-related methods

turbopack/crates/turbopack-ecmascript/Cargo.toml

@@ -88,6 +88,7 @@ swc_core = { workspace = true, features = [
 
 [dev-dependencies]
 criterion = { workspace = true, features = ["async_tokio"] }
+rstest = { workspace = true }
 turbo-tasks-backend = { workspace = true }
 turbo-tasks-malloc = { workspace = true, features  = ["custom_allocator"] }
 turbo-tasks-testing = { workspace = true }

turbopack/crates/turbopack-ecmascript/src/analyzer/mod.rs

@@ -2526,10 +2526,10 @@ impl JsValue {
             },
             JsValue::Logical(_, op, list) => match op {
                 LogicalOperator::And => {
-                    shortcircuit_if_known(list, JsValue::is_truthy, JsValue::is_nullish)
+                    shortcircuit_if_known(list, JsValue::is_falsy, JsValue::is_nullish)
                 }
                 LogicalOperator::Or => {
-                    shortcircuit_if_known(list, JsValue::is_falsy, JsValue::is_nullish)
+                    shortcircuit_if_known(list, JsValue::is_truthy, JsValue::is_nullish)
                 }
                 LogicalOperator::NullishCoalescing => all_if_known(list, JsValue::is_nullish),
             },
@@ -2558,10 +2558,10 @@ impl JsValue {
             } => merge_if_known(values, JsValue::is_empty_string),
             JsValue::Logical(_, op, list) => match op {
                 LogicalOperator::And => {
-                    shortcircuit_if_known(list, JsValue::is_truthy, JsValue::is_empty_string)
+                    shortcircuit_if_known(list, JsValue::is_falsy, JsValue::is_empty_string)
                 }
                 LogicalOperator::Or => {
-                    shortcircuit_if_known(list, JsValue::is_falsy, JsValue::is_empty_string)
+                    shortcircuit_if_known(list, JsValue::is_truthy, JsValue::is_empty_string)
                 }
                 LogicalOperator::NullishCoalescing => {
                     shortcircuit_if_known(list, JsValue::is_not_nullish, JsValue::is_empty_string)
@@ -2619,10 +2619,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_truthy, JsValue::is_string)
+                    shortcircuit_if_known(list, JsValue::is_falsy, JsValue::is_string)
                 }
                 LogicalOperator::Or => {
-                    shortcircuit_if_known(list, JsValue::is_falsy, JsValue::is_string)
+                    shortcircuit_if_known(list, JsValue::is_truthy, JsValue::is_string)
                 }
                 LogicalOperator::NullishCoalescing => {
                     shortcircuit_if_known(list, JsValue::is_not_nullish, JsValue::is_string)
@@ -3996,6 +3996,7 @@ mod tests {
     use std::{mem::take, path::PathBuf, time::Instant};
 
     use parking_lot::Mutex;
+    use rstest::rstest;
     use rustc_hash::FxHashMap;
     use swc_core::{
         common::{Mark, comments::SingleThreadedComments},
@@ -4491,4 +4492,198 @@ mod tests {
     fn jsvalue_size() {
         assert_eq!(32, size_of::<JsValue>());
     }
+
+    #[test]
+    fn is_string_constant() {
+        let value = EvalContext::eval_single_expr_lit(&rcstr!("'hello'")).unwrap();
+        assert_eq!(value.is_string(), Some(true));
+    }
+
+    #[rstest]
+    #[case("1 && 'hello'")]
+    #[case("'hello' || 'bye' || 2")]
+    fn is_string_short_circuiting_positive(#[case] input: &str) {
+        assert_eq!(
+            EvalContext::eval_single_expr_lit(&input.into())
+                .unwrap()
+                .is_string(),
+            Some(true),
+            "expected '{}' to be a string",
+            input
+        );
+    }
+
+    #[rstest]
+    #[case("'hello' && 2")]
+    #[case("2 || 1 || 'hello' || 'bye'")]
+    fn is_string_short_circuiting_negative(#[case] input: &str) {
+        assert_eq!(
+            EvalContext::eval_single_expr_lit(&input.into())
+                .unwrap()
+                .is_string(),
+            Some(false),
+            "expected '{}' not to be a string",
+            input
+        );
+    }
+
+    #[rstest]
+    #[case("x && 2")]
+    #[case("1 && x")]
+    #[case("1 && 'a' && x")]
+    #[case("x || 'bye'")]
+    #[case("false || x")]
+    fn is_string_short_circuiting_unknown(#[case] input: &str) {
+        assert_eq!(
+            EvalContext::eval_single_expr_lit(&input.into())
+                .unwrap()
+                .is_string(),
+            None,
+            "expected to be unable to determine whether '{}' is a string",
+            input
+        );
+    }
+
+    #[rstest]
+    #[case("'' && 'string'")]
+    #[case("false || ''")]
+    #[case("1 && 'a' && ''")]
+    fn is_empty_string_short_circuiting_positive(#[case] input: &str) {
+        assert_eq!(
+            EvalContext::eval_single_expr_lit(&input.into())
+                .unwrap()
+                .is_empty_string(),
+            Some(true),
+            "expected '{}' to be an empty string",
+            input
+        );
+    }
+
+    #[rstest]
+    #[case("false && ''")]
+    #[case("'' || 'string'")]
+    #[case("'' || 0 || 'string'")]
+    fn is_empty_string_short_circuiting_negative(#[case] input: &str) {
+        assert_eq!(
+            EvalContext::eval_single_expr_lit(&input.into())
+                .unwrap()
+                .is_empty_string(),
+            Some(false),
+            "expected '{}' not to be an empty string",
+            input
+        );
+    }
+
+    #[rstest]
+    #[case("x && ''")]
+    #[case("1 && x")]
+    #[case("x || ''")]
+    #[case("'' || x")]
+    #[case("false || 0 || x")]
+    fn is_empty_string_short_circuiting_unknown(#[case] input: &str) {
+        assert_eq!(
+            EvalContext::eval_single_expr_lit(&input.into())
+                .unwrap()
+                .is_empty_string(),
+            None,
+            "expected to be unable to determine whether '{}' is an empty string",
+            input
+        );
+    }
+
+    #[rstest]
+    #[case("null && ''")]
+    #[case("'' || null")]
+    #[case("1 && 2 && null")]
+    fn is_nullish_short_circuiting_positive(#[case] input: &str) {
+        assert_eq!(
+            EvalContext::eval_single_expr_lit(&input.into())
+                .unwrap()
+                .is_nullish(),
+            Some(true),
+            "expected '{}' to be nullish",
+            input
+        );
+    }
+
+    #[rstest]
+    #[case("'' && null")]
+    #[case("null || ''")]
+    #[case("null || '' || 'a'")]
+    fn is_nullish_short_circuiting_negative(#[case] input: &str) {
+        assert_eq!(
+            EvalContext::eval_single_expr_lit(&input.into())
+                .unwrap()
+                .is_nullish(),
+            Some(false),
+            "expected '{}' not to be nullish",
+            input
+        );
+    }
+
+    #[rstest]
+    #[case("x && null")]
+    #[case("1 && x")]
+    #[case("x || null")]
+    #[case("null || x")]
+    #[case("false || x")]
+    #[case("1 && x && null")]
+    fn is_nullish_short_circuiting_unknown(#[case] input: &str) {
+        assert_eq!(
+            EvalContext::eval_single_expr_lit(&input.into())
+                .unwrap()
+                .is_nullish(),
+            None,
+            "expected to be unable to determine whether '{}' is nullish",
+            input
+        );
+    }
+
+    #[rstest]
+    #[case("'' && null")]
+    #[case("null || ''")]
+    #[case("null || 0 || 'a'")]
+    fn is_not_nullish_short_circuiting_positive(#[case] input: &str) {
+        assert_eq!(
+            EvalContext::eval_single_expr_lit(&input.into())
+                .unwrap()
+                .is_not_nullish(),
+            Some(true),
+            "expected '{}' to be not-nullish",
+            input
+        );
+    }
+
+    #[rstest]
+    #[case("null && ''")]
+    #[case("'' || null")]
+    #[case("'' || 0 || null")]
+    fn is_not_nullish_short_circuiting_negative(#[case] input: &str) {
+        assert_eq!(
+            EvalContext::eval_single_expr_lit(&input.into())
+                .unwrap()
+                .is_not_nullish(),
+            Some(false),
+            "expected '{}' not to be not-nullish",
+            input
+        );
+    }
+
+    #[rstest]
+    #[case("x && null")]
+    #[case("1 && x")]
+    #[case("x || null")]
+    #[case("null || x")]
+    #[case("false || x")]
+    #[case("false || x || ''")]
+    fn is_not_nullish_short_circuiting_unknown(#[case] input: &str) {
+        assert_eq!(
+            EvalContext::eval_single_expr_lit(&input.into())
+                .unwrap()
+                .is_not_nullish(),
+            None,
+            "expected to be unable to determine whether '{}' is not-nullish",
+            input
+        );
+    }
 }