diff --git a/compiler/core/lam_compile.ml b/compiler/core/lam_compile.ml index 680bce50a7..87a25bfe2a 100644 --- a/compiler/core/lam_compile.ml +++ b/compiler/core/lam_compile.ml @@ -814,14 +814,29 @@ 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 - in let switch ?default ?declaration e clauses = let not_typeof_clauses, typeof_clauses = List.partition clause_is_not_typeof clauses in + let has_object_typeof = + List.exists + (function + | Ast_untagged_variants.Untagged ObjectType, _ -> true + | _ -> false) + typeof_clauses + in + let has_array_case = + List.exists + (function + | Ast_untagged_variants.Untagged (InstanceType Array), _ -> true + | _ -> false) + not_typeof_clauses + in + (* When there's an ObjectType typeof case, null and arrays can + incorrectly match it (typeof null === typeof [] === "object"). + Guard against them when they should fall through to default. *) + let needs_null_guard = has_object_typeof && has_null_case in + let needs_array_guard = has_object_typeof && not has_array_case in let rec build_if_chain remaining_clauses = match remaining_clauses with | ( Ast_untagged_variants.Untagged (InstanceType instance_type), @@ -831,17 +846,21 @@ let compile output_prefix = (E.emit_check (IsInstanceOf (instance_type, Expr e))) switch_body ~else_:[build_if_chain rest] - | _ -> + | _ -> ( let typeof_switch () = 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 () + let guard = + match (needs_null_guard, needs_array_guard) with + | true, true -> Some (E.or_ (E.is_null e) (E.is_array e)) + | true, false -> Some (E.is_null e) + | false, true -> Some (E.is_array e) + | false, false -> None + in + match (guard, default) with + | Some guard, Some default_body -> + S.if_ guard default_body ~else_:[typeof_switch ()] + | _ -> typeof_switch ()) in build_if_chain not_typeof_clauses in diff --git a/tests/tests/src/js_json_test.mjs b/tests/tests/src/js_json_test.mjs index c69a9fd6f3..8ff5ed4fa0 100644 --- a/tests/tests/src/js_json_test.mjs +++ b/tests/tests/src/js_json_test.mjs @@ -367,7 +367,7 @@ Mocha.describe("Js_json_test", () => { Test_utils.eq("File \"js_json_test.res\", line 314, characters 7-14", Js_json.decodeArray({}), undefined); Test_utils.eq("File \"js_json_test.res\", line 315, characters 7-14", Js_json.decodeArray(1.23), undefined); }); - Mocha.test("JSON Array/Object switch falls through to wildcard on null", () => { + Mocha.test("JSON Array/Object switch falls through to wildcard on null and array", () => { let classifyArrayOrObject = json => { if (Array.isArray(json)) { return json.length; @@ -386,25 +386,42 @@ Mocha.describe("Js_json_test", () => { Test_utils.eq("File \"js_json_test.res\", line 328, characters 7-14", classifyArrayOrObject(null), undefined); Test_utils.eq("File \"js_json_test.res\", line 329, characters 7-14", classifyArrayOrObject([1]), 1); Test_utils.eq("File \"js_json_test.res\", line 330, characters 7-14", classifyArrayOrObject({}), 0); + let classifyObjectOnly = json => { + if (json === null || Array.isArray(json)) { + return "default"; + } + switch (typeof json) { + case "string" : + return "String"; + case "object" : + return "Object"; + default: + return "default"; + } + }; + Test_utils.eq("File \"js_json_test.res\", line 340, characters 7-14", classifyObjectOnly(null), "default"); + Test_utils.eq("File \"js_json_test.res\", line 341, characters 7-14", classifyObjectOnly([]), "default"); + Test_utils.eq("File \"js_json_test.res\", line 342, characters 7-14", classifyObjectOnly({}), "Object"); + Test_utils.eq("File \"js_json_test.res\", line 343, characters 7-14", classifyObjectOnly("hi"), "String"); }); Mocha.test("JSON decodeBoolean", () => { - Test_utils.eq("File \"js_json_test.res\", line 334, characters 7-14", Js_json.decodeBoolean("test"), undefined); - Test_utils.eq("File \"js_json_test.res\", line 335, characters 7-14", Js_json.decodeBoolean(true), true); - Test_utils.eq("File \"js_json_test.res\", line 336, characters 7-14", Js_json.decodeBoolean([]), undefined); - Test_utils.eq("File \"js_json_test.res\", line 337, characters 7-14", Js_json.decodeBoolean(null), undefined); - Test_utils.eq("File \"js_json_test.res\", line 338, characters 7-14", Js_json.decodeBoolean({}), undefined); - Test_utils.eq("File \"js_json_test.res\", line 339, characters 7-14", Js_json.decodeBoolean(1.23), undefined); + Test_utils.eq("File \"js_json_test.res\", line 347, characters 7-14", Js_json.decodeBoolean("test"), undefined); + Test_utils.eq("File \"js_json_test.res\", line 348, characters 7-14", Js_json.decodeBoolean(true), true); + Test_utils.eq("File \"js_json_test.res\", line 349, characters 7-14", Js_json.decodeBoolean([]), undefined); + Test_utils.eq("File \"js_json_test.res\", line 350, characters 7-14", Js_json.decodeBoolean(null), undefined); + Test_utils.eq("File \"js_json_test.res\", line 351, characters 7-14", Js_json.decodeBoolean({}), undefined); + Test_utils.eq("File \"js_json_test.res\", line 352, characters 7-14", Js_json.decodeBoolean(1.23), undefined); }); Mocha.test("JSON decodeNull", () => { - Test_utils.eq("File \"js_json_test.res\", line 343, characters 7-14", Js_json.decodeNull("test"), undefined); - Test_utils.eq("File \"js_json_test.res\", line 344, characters 7-14", Js_json.decodeNull(true), undefined); - Test_utils.eq("File \"js_json_test.res\", line 345, characters 7-14", Js_json.decodeNull([]), undefined); - Test_utils.eq("File \"js_json_test.res\", line 346, characters 7-14", Js_json.decodeNull(null), null); - Test_utils.eq("File \"js_json_test.res\", line 347, characters 7-14", Js_json.decodeNull({}), undefined); - Test_utils.eq("File \"js_json_test.res\", line 348, characters 7-14", Js_json.decodeNull(1.23), undefined); + Test_utils.eq("File \"js_json_test.res\", line 356, characters 7-14", Js_json.decodeNull("test"), undefined); + Test_utils.eq("File \"js_json_test.res\", line 357, characters 7-14", Js_json.decodeNull(true), undefined); + Test_utils.eq("File \"js_json_test.res\", line 358, characters 7-14", Js_json.decodeNull([]), undefined); + Test_utils.eq("File \"js_json_test.res\", line 359, characters 7-14", Js_json.decodeNull(null), null); + Test_utils.eq("File \"js_json_test.res\", line 360, characters 7-14", Js_json.decodeNull({}), undefined); + Test_utils.eq("File \"js_json_test.res\", line 361, characters 7-14", Js_json.decodeNull(1.23), undefined); }); Mocha.test("JSON serialize/deserialize identity", () => { - let idtest = obj => Test_utils.eq("File \"js_json_test.res\", line 354, characters 27-34", obj, Js_json.deserializeUnsafe(Js_json.serializeExn(obj))); + let idtest = obj => Test_utils.eq("File \"js_json_test.res\", line 367, characters 27-34", obj, Js_json.deserializeUnsafe(Js_json.serializeExn(obj))); idtest(undefined); idtest({ hd: [ diff --git a/tests/tests/src/js_json_test.res b/tests/tests/src/js_json_test.res index 26fa6320d0..0e1bad5add 100644 --- a/tests/tests/src/js_json_test.res +++ b/tests/tests/src/js_json_test.res @@ -315,7 +315,7 @@ describe(__MODULE__, () => { eq(__LOC__, J.decodeArray(J.number(1.23)), None) }) - test("JSON Array/Object switch falls through to wildcard on null", () => { + test("JSON Array/Object switch falls through to wildcard on null and array", () => { let classifyArrayOrObject = (json: J.t) => switch json { | J.Array(items) => Some(items->Js.Array2.length) @@ -328,6 +328,19 @@ describe(__MODULE__, () => { eq(__LOC__, classifyArrayOrObject(J.null), None) eq(__LOC__, classifyArrayOrObject(J.array([J.number(1.)])), Some(1)) eq(__LOC__, classifyArrayOrObject(J.object_(Js.Dict.empty())), Some(0)) + + // When there's no Array case, arrays should fall to wildcard + let classifyObjectOnly = (json: J.t) => + switch json { + | J.Object(_) => "Object" + | J.String(_) => "String" + | _ => "default" + } + + eq(__LOC__, classifyObjectOnly(J.null), "default") + eq(__LOC__, classifyObjectOnly(J.array([])), "default") + eq(__LOC__, classifyObjectOnly(J.object_(Js.Dict.empty())), "Object") + eq(__LOC__, classifyObjectOnly(J.string("hi")), "String") }) test("JSON decodeBoolean", () => {