Skip to content

Commit ccdec51

Browse files
cristianocclaude
andcommitted
Fix null/array guards dropped for untagged variant switch in statement position
When a switch on an untagged variant (e.g. JSON.t) with an Object case was in statement position (not tail), compile_general_cases would set default=None for empty default bodies, causing the null/array guard to be silently skipped. This let null and arrays fall into the typeof "object" branch at runtime. Two fixes: - Handle the Some guard, None default case by emitting if (!(guard)) { ... } - Use the type's block_cases to determine if arrays can appear at runtime, avoiding unnecessary Array.isArray guards for types without an Array case Fixes #8251 (comment) Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
1 parent 11bbd58 commit ccdec51

File tree

4 files changed

+77
-16
lines changed

4 files changed

+77
-16
lines changed

CHANGELOG.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,7 @@
2323
#### :bug: Bug fix
2424

2525
- Reanalyze server: invalidate cache and recompute results when config changes in `rescript.json`. https://github.com/rescript-lang/rescript/pull/8262
26+
- Fix `null` and array values incorrectly matching the `Object` branch when pattern matching on `JSON.t` (or other untagged variants with an `Object` case) in statement position. https://github.com/rescript-lang/rescript/pull/8279
2627

2728
#### :memo: Documentation
2829

compiler/core/lam_compile.ml

Lines changed: 13 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -825,18 +825,27 @@ let compile output_prefix =
825825
| _ -> false)
826826
typeof_clauses
827827
in
828-
let has_array_case =
828+
let clauses_have_array_case =
829829
List.exists
830830
(function
831831
| Ast_untagged_variants.Untagged (InstanceType Array), _ -> true
832832
| _ -> false)
833833
not_typeof_clauses
834834
in
835+
let type_has_array_case =
836+
List.exists
837+
(function
838+
| Ast_untagged_variants.InstanceType Array -> true
839+
| _ -> false)
840+
block_cases
841+
in
835842
(* When there's an ObjectType typeof case, null and arrays can
836843
incorrectly match it (typeof null === typeof [] === "object").
837844
Guard against them when they should fall through to default. *)
838845
let needs_null_guard = has_object_typeof && has_null_case in
839-
let needs_array_guard = has_object_typeof && not has_array_case in
846+
let needs_array_guard =
847+
has_object_typeof && type_has_array_case && not clauses_have_array_case
848+
in
840849
let rec build_if_chain remaining_clauses =
841850
match remaining_clauses with
842851
| ( Ast_untagged_variants.Untagged (InstanceType instance_type),
@@ -860,7 +869,8 @@ let compile output_prefix =
860869
match (guard, default) with
861870
| Some guard, Some default_body ->
862871
S.if_ guard default_body ~else_:[typeof_switch ()]
863-
| _ -> typeof_switch ())
872+
| Some guard, None -> S.if_ (E.not guard) [typeof_switch ()]
873+
| None, _ -> typeof_switch ())
864874
in
865875
build_if_chain not_typeof_clauses
866876
in

tests/tests/src/js_json_test.mjs

Lines changed: 39 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -404,24 +404,50 @@ Mocha.describe("Js_json_test", () => {
404404
Test_utils.eq("File \"js_json_test.res\", line 342, characters 7-14", classifyObjectOnly({}), "Object");
405405
Test_utils.eq("File \"js_json_test.res\", line 343, characters 7-14", classifyObjectOnly("hi"), "String");
406406
});
407+
Mocha.test("JSON Object switch as statement guards null and array", () => {
408+
let result = {
409+
contents: "none"
410+
};
411+
let classifyStatement = json => {
412+
if (json === null || Array.isArray(json)) {
413+
return;
414+
}
415+
switch (typeof json) {
416+
case "object" :
417+
result.contents = "object";
418+
return;
419+
default:
420+
return;
421+
}
422+
};
423+
result.contents = "none";
424+
classifyStatement(null);
425+
Test_utils.eq("File \"js_json_test.res\", line 359, characters 7-14", result.contents, "none");
426+
result.contents = "none";
427+
classifyStatement([]);
428+
Test_utils.eq("File \"js_json_test.res\", line 363, characters 7-14", result.contents, "none");
429+
result.contents = "none";
430+
classifyStatement({});
431+
Test_utils.eq("File \"js_json_test.res\", line 367, characters 7-14", result.contents, "object");
432+
});
407433
Mocha.test("JSON decodeBoolean", () => {
408-
Test_utils.eq("File \"js_json_test.res\", line 347, characters 7-14", Js_json.decodeBoolean("test"), undefined);
409-
Test_utils.eq("File \"js_json_test.res\", line 348, characters 7-14", Js_json.decodeBoolean(true), true);
410-
Test_utils.eq("File \"js_json_test.res\", line 349, characters 7-14", Js_json.decodeBoolean([]), undefined);
411-
Test_utils.eq("File \"js_json_test.res\", line 350, characters 7-14", Js_json.decodeBoolean(null), undefined);
412-
Test_utils.eq("File \"js_json_test.res\", line 351, characters 7-14", Js_json.decodeBoolean({}), undefined);
413-
Test_utils.eq("File \"js_json_test.res\", line 352, characters 7-14", Js_json.decodeBoolean(1.23), undefined);
434+
Test_utils.eq("File \"js_json_test.res\", line 371, characters 7-14", Js_json.decodeBoolean("test"), undefined);
435+
Test_utils.eq("File \"js_json_test.res\", line 372, characters 7-14", Js_json.decodeBoolean(true), true);
436+
Test_utils.eq("File \"js_json_test.res\", line 373, characters 7-14", Js_json.decodeBoolean([]), undefined);
437+
Test_utils.eq("File \"js_json_test.res\", line 374, characters 7-14", Js_json.decodeBoolean(null), undefined);
438+
Test_utils.eq("File \"js_json_test.res\", line 375, characters 7-14", Js_json.decodeBoolean({}), undefined);
439+
Test_utils.eq("File \"js_json_test.res\", line 376, characters 7-14", Js_json.decodeBoolean(1.23), undefined);
414440
});
415441
Mocha.test("JSON decodeNull", () => {
416-
Test_utils.eq("File \"js_json_test.res\", line 356, characters 7-14", Js_json.decodeNull("test"), undefined);
417-
Test_utils.eq("File \"js_json_test.res\", line 357, characters 7-14", Js_json.decodeNull(true), undefined);
418-
Test_utils.eq("File \"js_json_test.res\", line 358, characters 7-14", Js_json.decodeNull([]), undefined);
419-
Test_utils.eq("File \"js_json_test.res\", line 359, characters 7-14", Js_json.decodeNull(null), null);
420-
Test_utils.eq("File \"js_json_test.res\", line 360, characters 7-14", Js_json.decodeNull({}), undefined);
421-
Test_utils.eq("File \"js_json_test.res\", line 361, characters 7-14", Js_json.decodeNull(1.23), undefined);
442+
Test_utils.eq("File \"js_json_test.res\", line 380, characters 7-14", Js_json.decodeNull("test"), undefined);
443+
Test_utils.eq("File \"js_json_test.res\", line 381, characters 7-14", Js_json.decodeNull(true), undefined);
444+
Test_utils.eq("File \"js_json_test.res\", line 382, characters 7-14", Js_json.decodeNull([]), undefined);
445+
Test_utils.eq("File \"js_json_test.res\", line 383, characters 7-14", Js_json.decodeNull(null), null);
446+
Test_utils.eq("File \"js_json_test.res\", line 384, characters 7-14", Js_json.decodeNull({}), undefined);
447+
Test_utils.eq("File \"js_json_test.res\", line 385, characters 7-14", Js_json.decodeNull(1.23), undefined);
422448
});
423449
Mocha.test("JSON serialize/deserialize identity", () => {
424-
let idtest = obj => Test_utils.eq("File \"js_json_test.res\", line 367, characters 27-34", obj, Js_json.deserializeUnsafe(Js_json.serializeExn(obj)));
450+
let idtest = obj => Test_utils.eq("File \"js_json_test.res\", line 391, characters 27-34", obj, Js_json.deserializeUnsafe(Js_json.serializeExn(obj)));
425451
idtest(undefined);
426452
idtest({
427453
hd: [

tests/tests/src/js_json_test.res

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -343,6 +343,30 @@ describe(__MODULE__, () => {
343343
eq(__LOC__, classifyObjectOnly(J.string("hi")), "String")
344344
})
345345

346+
test("JSON Object switch as statement guards null and array", () => {
347+
let result = ref("none")
348+
let classifyStatement = (json: J.t) => {
349+
switch json {
350+
| J.Object(_) => result := "object"
351+
| J.Array(_) => ()
352+
| J.String(_) => ()
353+
| _ => ()
354+
}
355+
}
356+
357+
result := "none"
358+
classifyStatement(J.null)
359+
eq(__LOC__, result.contents, "none")
360+
361+
result := "none"
362+
classifyStatement(J.array([]))
363+
eq(__LOC__, result.contents, "none")
364+
365+
result := "none"
366+
classifyStatement(J.object_(Js.Dict.empty()))
367+
eq(__LOC__, result.contents, "object")
368+
})
369+
346370
test("JSON decodeBoolean", () => {
347371
eq(__LOC__, J.decodeBoolean(J.string("test")), None)
348372
eq(__LOC__, J.decodeBoolean(J.boolean(true)), Some(true))

0 commit comments

Comments
 (0)