diff --git a/compiler/core/lam_compile.ml b/compiler/core/lam_compile.ml index 680bce50a7..5de8127194 100644 --- a/compiler/core/lam_compile.ml +++ b/compiler/core/lam_compile.ml @@ -814,9 +814,23 @@ let compile output_prefix = | _ -> false in let clause_is_not_typeof (tag, _) = tag_is_not_typeof tag in - let clause_is_object_typeof = function - | Ast_untagged_variants.Untagged ObjectType, _ -> true - | _ -> false + let has_null_clause clauses = + Ext_list.exists clauses (fun (tag, _) -> + match (tag : Ast_untagged_variants.tag_type) with + | Null -> true + | _ -> false) + in + let has_object_clause clauses = + Ext_list.exists clauses (fun (tag, _) -> + match (tag : Ast_untagged_variants.tag_type) with + | Untagged ObjectType -> true + | _ -> false) + in + let has_array_clause clauses = + Ext_list.exists clauses (fun (tag, _) -> + match (tag : Ast_untagged_variants.tag_type) with + | Untagged (InstanceType Array) -> true + | _ -> false) in let switch ?default ?declaration e clauses = let not_typeof_clauses, typeof_clauses = @@ -831,17 +845,42 @@ let compile output_prefix = (E.emit_check (IsInstanceOf (instance_type, Expr e))) switch_body ~else_:[build_if_chain rest] - | _ -> - let typeof_switch () = + | _ -> ( + let switch_stmt = S.string_switch ?default ?declaration (E.typeof e) typeof_clauses in - if has_null_case && List.exists clause_is_object_typeof typeof_clauses - then - match default with - | Some default_body -> - S.if_ (E.is_null e) default_body ~else_:[typeof_switch ()] - | None -> typeof_switch () - else typeof_switch () + (* If there's an Object clause but no Null clause and null is a possible value, + we need to exclude null from matching the Object case. + In JavaScript, typeof null === "object", so null would incorrectly match Object. *) + let needs_null_check = + has_null_case + && (not (has_null_clause typeof_clauses)) + && has_object_clause typeof_clauses + in + (* If Array is a possible block type but there's no Array clause and there is an Object clause, + we need to exclude arrays from matching the Object case. + In JavaScript, typeof [] === "object", so arrays would incorrectly match Object. *) + let needs_array_check = + List.mem Ast_untagged_variants.(InstanceType Array) block_cases + && (not (has_array_clause typeof_clauses)) + && has_object_clause typeof_clauses + in + let cond = + match (needs_null_check, needs_array_check) with + | true, true -> + Some (E.and_ (E.not (E.is_null e)) (E.not (E.is_array e))) + | true, false -> Some (E.not (E.is_null e)) + | false, true -> Some (E.not (E.is_array e)) + | false, false -> None + in + match cond with + | Some c -> + S.if_ c [switch_stmt] + ~else_: + (match default with + | Some block -> block + | None -> []) + | None -> switch_stmt) in build_if_chain not_typeof_clauses in diff --git a/tests/tests/src/JSONSwitchNullCheck_test.mjs b/tests/tests/src/JSONSwitchNullCheck_test.mjs new file mode 100644 index 0000000000..d1152f8bce --- /dev/null +++ b/tests/tests/src/JSONSwitchNullCheck_test.mjs @@ -0,0 +1,199 @@ +// Generated by ReScript, PLEASE EDIT WITH CARE + +import * as Mocha from "mocha"; +import * as Test_utils from "./test_utils.mjs"; + +let jsonNull = null; + +let jsonArray = ["a", "b"]; + +let jsonObject = {"key": "value"}; + +let jsonString = "hello"; + +let jsonNumber = 42; + +let jsonBool = true; + +Mocha.describe("JSONSwitchNullCheck_test", () => { + Mocha.describe("JSON direct switch with Null - should NOT match Object", () => { + Mocha.test("switch null { | Object(_) => Object | Array(_) => Array | _ => default }", () => { + let result; + if (Array.isArray(jsonNull)) { + result = "Array"; + } else if (jsonNull !== null && !Array.isArray(jsonNull)) { + switch (typeof jsonNull) { + case "object" : + result = "Object"; + break; + default: + result = "default"; + } + } else { + result = "default"; + } + Test_utils.eq("File \"JSONSwitchNullCheck_test.res\", line 26, characters 11-18", result, "default"); + }); + Mocha.test("switch null { | Object(_) => Object | String(_) => String | _ => default }", () => { + let result; + if (jsonNull !== null && !Array.isArray(jsonNull)) { + switch (typeof jsonNull) { + case "string" : + result = "String"; + break; + case "object" : + result = "Object"; + break; + default: + result = "default"; + } + } else { + result = "default"; + } + Test_utils.eq("File \"JSONSwitchNullCheck_test.res\", line 39, characters 11-18", result, "default"); + }); + Mocha.test("switch null { | Object(_) => Object | Number(_) => Number | _ => default }", () => { + let result; + if (jsonNull !== null && !Array.isArray(jsonNull)) { + switch (typeof jsonNull) { + case "number" : + result = "Number"; + break; + case "object" : + result = "Object"; + break; + default: + result = "default"; + } + } else { + result = "default"; + } + Test_utils.eq("File \"JSONSwitchNullCheck_test.res\", line 52, characters 11-18", result, "default"); + }); + Mocha.test("switch null { | Object(_) => Object | Array(_) => Array | String(_) => String | _ => default }", () => { + let result; + if (Array.isArray(jsonNull)) { + result = "Array"; + } else if (jsonNull !== null && !Array.isArray(jsonNull)) { + switch (typeof jsonNull) { + case "string" : + result = "String"; + break; + case "object" : + result = "Object"; + break; + default: + result = "default"; + } + } else { + result = "default"; + } + Test_utils.eq("File \"JSONSwitchNullCheck_test.res\", line 66, characters 11-18", result, "default"); + }); + }); + Mocha.describe("JSON direct switch with Array - should NOT match Object", () => { + Mocha.test("switch array { | Object(_) => Object | String(_) => String | _ => default }", () => { + let result; + if (jsonArray !== null && !Array.isArray(jsonArray)) { + switch (typeof jsonArray) { + case "string" : + result = "String"; + break; + case "object" : + result = "Object"; + break; + default: + result = "default"; + } + } else { + result = "default"; + } + Test_utils.eq("File \"JSONSwitchNullCheck_test.res\", line 81, characters 11-18", result, "default"); + }); + Mocha.test("switch array { | Object(_) => Object | Number(_) => Number | _ => default }", () => { + let result; + if (jsonArray !== null && !Array.isArray(jsonArray)) { + switch (typeof jsonArray) { + case "number" : + result = "Number"; + break; + case "object" : + result = "Object"; + break; + default: + result = "default"; + } + } else { + result = "default"; + } + Test_utils.eq("File \"JSONSwitchNullCheck_test.res\", line 94, characters 11-18", result, "default"); + }); + Mocha.test("switch array { | Object(_) => Object | Boolean(_) => Boolean | _ => default }", () => { + let result; + if (jsonArray !== null && !Array.isArray(jsonArray)) { + switch (typeof jsonArray) { + case "boolean" : + result = "Boolean"; + break; + case "object" : + result = "Object"; + break; + default: + result = "default"; + } + } else { + result = "default"; + } + Test_utils.eq("File \"JSONSwitchNullCheck_test.res\", line 107, characters 11-18", result, "default"); + }); + }); + Mocha.describe("JSON switch - correct positive cases", () => { + Mocha.test("switch object { | Object(_) => Object | _ => default }", () => { + let result; + result = typeof jsonObject === "object" && jsonObject !== null && !Array.isArray(jsonObject) ? "Object" : "default"; + Test_utils.eq("File \"JSONSwitchNullCheck_test.res\", line 120, characters 11-18", result, "Object"); + }); + Mocha.test("switch array { | Array(_) => Array | _ => default }", () => { + let result; + result = Array.isArray(jsonArray) ? "Array" : "default"; + Test_utils.eq("File \"JSONSwitchNullCheck_test.res\", line 131, characters 11-18", result, "Array"); + }); + Mocha.test("switch string { | String(_) => String | _ => default }", () => { + let result; + result = typeof jsonString === "string" ? "String" : "default"; + Test_utils.eq("File \"JSONSwitchNullCheck_test.res\", line 142, characters 11-18", result, "String"); + }); + Mocha.test("switch number { | Number(_) => Number | _ => default }", () => { + let result; + result = typeof jsonNumber === "number" ? "Number" : "default"; + Test_utils.eq("File \"JSONSwitchNullCheck_test.res\", line 153, characters 11-18", result, "Number"); + }); + Mocha.test("switch bool { | Boolean(_) => Boolean | _ => default }", () => { + let result; + result = typeof jsonBool === "boolean" ? "Boolean" : "default"; + Test_utils.eq("File \"JSONSwitchNullCheck_test.res\", line 164, characters 11-18", result, "Boolean"); + }); + Mocha.test("switch null { | Null => Null | _ => default }", () => { + let result; + result = jsonNull === null ? "Null" : "default"; + Test_utils.eq("File \"JSONSwitchNullCheck_test.res\", line 175, characters 11-18", result, "Null"); + }); + Mocha.test("switch null { | Null => Null | Object(_) => Object | _ => default }", () => { + let result; + result = jsonNull === null ? "Null" : ( + typeof jsonNull === "object" && !Array.isArray(jsonNull) ? "Object" : "default" + ); + Test_utils.eq("File \"JSONSwitchNullCheck_test.res\", line 187, characters 11-18", result, "Null"); + }); + }); +}); + +export { + jsonNull, + jsonArray, + jsonObject, + jsonString, + jsonNumber, + jsonBool, +} +/* Not a pure module */ diff --git a/tests/tests/src/JSONSwitchNullCheck_test.res b/tests/tests/src/JSONSwitchNullCheck_test.res new file mode 100644 index 0000000000..612492fc67 --- /dev/null +++ b/tests/tests/src/JSONSwitchNullCheck_test.res @@ -0,0 +1,191 @@ +open Mocha +open Test_utils + +// Test cases for JSON switch with null and various block types +// These test the fix for: null incorrectly matching Object case in switch + +// Helper to create test JSON values +let jsonNull: JSON.t = %raw(`null`) +let jsonArray: JSON.t = %raw(`["a", "b"]`) +let jsonObject: JSON.t = %raw(`{"key": "value"}`) +let jsonString: JSON.t = %raw(`"hello"`) +let jsonNumber: JSON.t = %raw(`42`) +let jsonBool: JSON.t = %raw(`true`) + +describe(__MODULE__, () => { + describe("JSON direct switch with Null - should NOT match Object", () => { + // Test: Object | Array | _ with null - null should go to default + test( + "switch null { | Object(_) => Object | Array(_) => Array | _ => default }", + () => { + let result = switch jsonNull { + | Object(_) => "Object" + | Array(_) => "Array" + | _ => "default" + } + eq(__LOC__, result, "default") + }, + ) + + // Test: Object | String | _ with null - null should go to default + test( + "switch null { | Object(_) => Object | String(_) => String | _ => default }", + () => { + let result = switch jsonNull { + | Object(_) => "Object" + | String(_) => "String" + | _ => "default" + } + eq(__LOC__, result, "default") + }, + ) + + // Test: Object | Number | _ with null - null should go to default + test( + "switch null { | Object(_) => Object | Number(_) => Number | _ => default }", + () => { + let result = switch jsonNull { + | Object(_) => "Object" + | Number(_) => "Number" + | _ => "default" + } + eq(__LOC__, result, "default") + }, + ) + + // Test: Multiple block cases with null - null should go to default + test( + "switch null { | Object(_) => Object | Array(_) => Array | String(_) => String | _ => default }", + () => { + let result = switch jsonNull { + | Object(_) => "Object" + | Array(_) => "Array" + | String(_) => "String" + | _ => "default" + } + eq(__LOC__, result, "default") + }, + ) + }) + + describe("JSON direct switch with Array - should NOT match Object", () => { + // Test: Array being matched as Object - Object | String | _ with array + test( + "switch array { | Object(_) => Object | String(_) => String | _ => default }", + () => { + let result = switch jsonArray { + | Object(_) => "Object" + | String(_) => "String" + | _ => "default" + } + eq(__LOC__, result, "default") + }, + ) + + // Test: Array being matched as Object - Object | Number | _ with array + test( + "switch array { | Object(_) => Object | Number(_) => Number | _ => default }", + () => { + let result = switch jsonArray { + | Object(_) => "Object" + | Number(_) => "Number" + | _ => "default" + } + eq(__LOC__, result, "default") + }, + ) + + // Test: Array being matched as Object - Object | Boolean | _ with array + test( + "switch array { | Object(_) => Object | Boolean(_) => Boolean | _ => default }", + () => { + let result = switch jsonArray { + | Object(_) => "Object" + | Boolean(_) => "Boolean" + | _ => "default" + } + eq(__LOC__, result, "default") + }, + ) + }) + + describe("JSON switch - correct positive cases", () => { + test( + "switch object { | Object(_) => Object | _ => default }", + () => { + let result = switch jsonObject { + | Object(_) => "Object" + | _ => "default" + } + eq(__LOC__, result, "Object") + }, + ) + + test( + "switch array { | Array(_) => Array | _ => default }", + () => { + let result = switch jsonArray { + | Array(_) => "Array" + | _ => "default" + } + eq(__LOC__, result, "Array") + }, + ) + + test( + "switch string { | String(_) => String | _ => default }", + () => { + let result = switch jsonString { + | String(_) => "String" + | _ => "default" + } + eq(__LOC__, result, "String") + }, + ) + + test( + "switch number { | Number(_) => Number | _ => default }", + () => { + let result = switch jsonNumber { + | Number(_) => "Number" + | _ => "default" + } + eq(__LOC__, result, "Number") + }, + ) + + test( + "switch bool { | Boolean(_) => Boolean | _ => default }", + () => { + let result = switch jsonBool { + | Boolean(_) => "Boolean" + | _ => "default" + } + eq(__LOC__, result, "Boolean") + }, + ) + + test( + "switch null { | Null => Null | _ => default }", + () => { + let result = switch jsonNull { + | Null => "Null" + | _ => "default" + } + eq(__LOC__, result, "Null") + }, + ) + + test( + "switch null { | Null => Null | Object(_) => Object | _ => default }", + () => { + let result = switch jsonNull { + | Null => "Null" + | Object(_) => "Object" + | _ => "default" + } + eq(__LOC__, result, "Null") + }, + ) + }) +}) diff --git a/tests/tests/src/UntaggedVariants.mjs b/tests/tests/src/UntaggedVariants.mjs index b57405ea26..5c1dc4383f 100644 --- a/tests/tests/src/UntaggedVariants.mjs +++ b/tests/tests/src/UntaggedVariants.mjs @@ -157,13 +157,15 @@ function classify$5(x) { if (Array.isArray(x)) { return "array"; } - switch (typeof x) { - case "string" : - return "string"; - case "number" : - return "int"; - case "object" : - return "Object" + x.name; + if (!Array.isArray(x)) { + switch (typeof x) { + case "string" : + return "string"; + case "number" : + return "int"; + case "object" : + return "Object" + x.name; + } } } @@ -188,22 +190,24 @@ function classify$6(x) { _0: x }; } - switch (typeof x) { - case "string" : - return { - TAG: "JSONString", - _0: x - }; - case "number" : - return { - TAG: "JSONNumber", - _0: x - }; - case "object" : - return { - TAG: "JSONObject", - _0: x - }; + if (!Array.isArray(x)) { + switch (typeof x) { + case "string" : + return { + TAG: "JSONString", + _0: x + }; + case "number" : + return { + TAG: "JSONNumber", + _0: x + }; + case "object" : + return { + TAG: "JSONObject", + _0: x + }; + } } } } @@ -357,11 +361,13 @@ function classify$9(v) { if (Array.isArray(v)) { return v[0]; } - switch (typeof v) { - case "object" : - return v.x; - case "function" : - return v(3); + if (!Array.isArray(v)) { + switch (typeof v) { + case "object" : + return v.x; + case "function" : + return v(3); + } } } @@ -481,13 +487,15 @@ async function classify$10(a) { console.log(await a); return; } - switch (typeof a) { - case "string" : - console.log(a); - return; - case "object" : - console.log(a.userName); - return; + if (!Array.isArray(a)) { + switch (typeof a) { + case "string" : + console.log(a); + return; + case "object" : + console.log(a.userName); + return; + } } } } @@ -589,13 +597,15 @@ async function classifyAll(t) { console.log("WeakMap"); return; } - switch (typeof t) { - case "string" : - console.log(t); - return; - case "object" : - console.log(t.userName); - return; + if (!Array.isArray(t)) { + switch (typeof t) { + case "string" : + console.log(t); + return; + case "object" : + console.log(t.userName); + return; + } } } @@ -629,11 +639,13 @@ function should_not_merge(x) { if (x instanceof Date) { return "do not merge"; } - switch (typeof x) { - case "boolean" : - return "boolean"; - case "object" : - return "do not merge"; + if (!Array.isArray(x)) { + switch (typeof x) { + case "boolean" : + return "boolean"; + case "object" : + return "do not merge"; + } } } @@ -644,10 +656,12 @@ function can_merge(x) { if (x instanceof Date) { return "do not merge"; } - switch (typeof x) { - case "boolean" : - case "object" : - return "merge"; + if (!Array.isArray(x)) { + switch (typeof x) { + case "boolean" : + case "object" : + return "merge"; + } } }