Skip to content

Commit ebed19a

Browse files
cristianocclaude
andcommitted
Fix null falling into object branch in untagged variant switch (#8251)
When pattern matching on untagged variants with both an Object case and a null/wildcard case, null (typeof "object") would incorrectly match the object branch. Emit a null check before the typeof switch when both conditions are present. Fixes #8251 Signed-Off-By: Cristiano Calcagno <cristiano.calcagno@gmail.com> Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
1 parent cfda8c1 commit ebed19a

4 files changed

Lines changed: 64 additions & 14 deletions

File tree

CHANGELOG.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,7 @@
2121
#### :bug: Bug fix
2222

2323
- Fix compiler crash (`Fatal error: Parmatch.all_record_args`) when matching empty dict/record patterns. https://github.com/rescript-lang/rescript/pull/8246
24+
- Fix `null` falling into the object branch instead of the wildcard when pattern matching on untagged variants with both `Object` and `null` cases. https://github.com/rescript-lang/rescript/pull/8253
2425

2526
#### :memo: Documentation
2627

compiler/core/lam_compile.ml

Lines changed: 15 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -814,6 +814,10 @@ let compile output_prefix =
814814
| _ -> false
815815
in
816816
let clause_is_not_typeof (tag, _) = tag_is_not_typeof tag in
817+
let clause_is_object_typeof = function
818+
| Ast_untagged_variants.Untagged ObjectType, _ -> true
819+
| _ -> false
820+
in
817821
let switch ?default ?declaration e clauses =
818822
let not_typeof_clauses, typeof_clauses =
819823
List.partition clause_is_not_typeof clauses
@@ -827,7 +831,17 @@ let compile output_prefix =
827831
(E.emit_check (IsInstanceOf (instance_type, Expr e)))
828832
switch_body
829833
~else_:[build_if_chain rest]
830-
| _ -> S.string_switch ?default ?declaration (E.typeof e) typeof_clauses
834+
| _ ->
835+
let typeof_switch () =
836+
S.string_switch ?default ?declaration (E.typeof e) typeof_clauses
837+
in
838+
if has_null_case && List.exists clause_is_object_typeof typeof_clauses
839+
then
840+
match default with
841+
| Some default_body ->
842+
S.if_ (E.is_null e) default_body ~else_:[typeof_switch ()]
843+
| None -> typeof_switch ()
844+
else typeof_switch ()
831845
in
832846
build_if_chain not_typeof_clauses
833847
in

tests/tests/src/js_json_test.mjs

Lines changed: 33 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -367,24 +367,44 @@ Mocha.describe("Js_json_test", () => {
367367
Test_utils.eq("File \"js_json_test.res\", line 314, characters 7-14", Js_json.decodeArray({}), undefined);
368368
Test_utils.eq("File \"js_json_test.res\", line 315, characters 7-14", Js_json.decodeArray(1.23), undefined);
369369
});
370+
Mocha.test("JSON Array/Object switch falls through to wildcard on null", () => {
371+
let classifyArrayOrObject = json => {
372+
if (Array.isArray(json)) {
373+
return json.length;
374+
}
375+
if (json === null) {
376+
return;
377+
}
378+
switch (typeof json) {
379+
case "object" :
380+
Js_dict.get(json, "x");
381+
return 0;
382+
default:
383+
return;
384+
}
385+
};
386+
Test_utils.eq("File \"js_json_test.res\", line 328, characters 7-14", classifyArrayOrObject(null), undefined);
387+
Test_utils.eq("File \"js_json_test.res\", line 329, characters 7-14", classifyArrayOrObject([1]), 1);
388+
Test_utils.eq("File \"js_json_test.res\", line 330, characters 7-14", classifyArrayOrObject({}), 0);
389+
});
370390
Mocha.test("JSON decodeBoolean", () => {
371-
Test_utils.eq("File \"js_json_test.res\", line 319, characters 7-14", Js_json.decodeBoolean("test"), undefined);
372-
Test_utils.eq("File \"js_json_test.res\", line 320, characters 7-14", Js_json.decodeBoolean(true), true);
373-
Test_utils.eq("File \"js_json_test.res\", line 321, characters 7-14", Js_json.decodeBoolean([]), undefined);
374-
Test_utils.eq("File \"js_json_test.res\", line 322, characters 7-14", Js_json.decodeBoolean(null), undefined);
375-
Test_utils.eq("File \"js_json_test.res\", line 323, characters 7-14", Js_json.decodeBoolean({}), undefined);
376-
Test_utils.eq("File \"js_json_test.res\", line 324, characters 7-14", Js_json.decodeBoolean(1.23), undefined);
391+
Test_utils.eq("File \"js_json_test.res\", line 334, characters 7-14", Js_json.decodeBoolean("test"), undefined);
392+
Test_utils.eq("File \"js_json_test.res\", line 335, characters 7-14", Js_json.decodeBoolean(true), true);
393+
Test_utils.eq("File \"js_json_test.res\", line 336, characters 7-14", Js_json.decodeBoolean([]), undefined);
394+
Test_utils.eq("File \"js_json_test.res\", line 337, characters 7-14", Js_json.decodeBoolean(null), undefined);
395+
Test_utils.eq("File \"js_json_test.res\", line 338, characters 7-14", Js_json.decodeBoolean({}), undefined);
396+
Test_utils.eq("File \"js_json_test.res\", line 339, characters 7-14", Js_json.decodeBoolean(1.23), undefined);
377397
});
378398
Mocha.test("JSON decodeNull", () => {
379-
Test_utils.eq("File \"js_json_test.res\", line 328, characters 7-14", Js_json.decodeNull("test"), undefined);
380-
Test_utils.eq("File \"js_json_test.res\", line 329, characters 7-14", Js_json.decodeNull(true), undefined);
381-
Test_utils.eq("File \"js_json_test.res\", line 330, characters 7-14", Js_json.decodeNull([]), undefined);
382-
Test_utils.eq("File \"js_json_test.res\", line 331, characters 7-14", Js_json.decodeNull(null), null);
383-
Test_utils.eq("File \"js_json_test.res\", line 332, characters 7-14", Js_json.decodeNull({}), undefined);
384-
Test_utils.eq("File \"js_json_test.res\", line 333, characters 7-14", Js_json.decodeNull(1.23), undefined);
399+
Test_utils.eq("File \"js_json_test.res\", line 343, characters 7-14", Js_json.decodeNull("test"), undefined);
400+
Test_utils.eq("File \"js_json_test.res\", line 344, characters 7-14", Js_json.decodeNull(true), undefined);
401+
Test_utils.eq("File \"js_json_test.res\", line 345, characters 7-14", Js_json.decodeNull([]), undefined);
402+
Test_utils.eq("File \"js_json_test.res\", line 346, characters 7-14", Js_json.decodeNull(null), null);
403+
Test_utils.eq("File \"js_json_test.res\", line 347, characters 7-14", Js_json.decodeNull({}), undefined);
404+
Test_utils.eq("File \"js_json_test.res\", line 348, characters 7-14", Js_json.decodeNull(1.23), undefined);
385405
});
386406
Mocha.test("JSON serialize/deserialize identity", () => {
387-
let idtest = obj => Test_utils.eq("File \"js_json_test.res\", line 339, characters 27-34", obj, Js_json.deserializeUnsafe(Js_json.serializeExn(obj)));
407+
let idtest = obj => Test_utils.eq("File \"js_json_test.res\", line 354, characters 27-34", obj, Js_json.deserializeUnsafe(Js_json.serializeExn(obj)));
388408
idtest(undefined);
389409
idtest({
390410
hd: [

tests/tests/src/js_json_test.res

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -315,6 +315,21 @@ describe(__MODULE__, () => {
315315
eq(__LOC__, J.decodeArray(J.number(1.23)), None)
316316
})
317317

318+
test("JSON Array/Object switch falls through to wildcard on null", () => {
319+
let classifyArrayOrObject = (json: J.t) =>
320+
switch json {
321+
| J.Array(items) => Some(items->Js.Array2.length)
322+
| J.Object(dict) =>
323+
ignore(Js.Dict.get(dict, "x"))
324+
Some(0)
325+
| _ => None
326+
}
327+
328+
eq(__LOC__, classifyArrayOrObject(J.null), None)
329+
eq(__LOC__, classifyArrayOrObject(J.array([J.number(1.)])), Some(1))
330+
eq(__LOC__, classifyArrayOrObject(J.object_(Js.Dict.empty())), Some(0))
331+
})
332+
318333
test("JSON decodeBoolean", () => {
319334
eq(__LOC__, J.decodeBoolean(J.string("test")), None)
320335
eq(__LOC__, J.decodeBoolean(J.boolean(true)), Some(true))

0 commit comments

Comments
 (0)