From d680d8a073b45995bc4552d816c4cf8ec9d4e546 Mon Sep 17 00:00:00 2001 From: Gabriel Nordeborn Date: Fri, 14 Apr 2023 17:40:05 +0200 Subject: [PATCH 1/8] add failing test for spread optional fields not remaining optional --- jscomp/test/DotDotDot.res | 2 ++ 1 file changed, 2 insertions(+) diff --git a/jscomp/test/DotDotDot.res b/jscomp/test/DotDotDot.res index 657d264ff6..14f4dc657a 100644 --- a/jscomp/test/DotDotDot.res +++ b/jscomp/test/DotDotDot.res @@ -35,6 +35,8 @@ type svgProps = { y?: string, } +let x: svgProps = {x: "test"} + module MultipleDotDotDots = { type t1 = {x: int} type t2 = {y: string} From e95220fca75c37b3f0a821ef57adf1f925dc504b Mon Sep 17 00:00:00 2001 From: Gabriel Nordeborn Date: Fri, 14 Apr 2023 17:50:47 +0200 Subject: [PATCH 2/8] add commented out way to show parser issue --- jscomp/test/DotDotDot.res | 3 +++ 1 file changed, 3 insertions(+) diff --git a/jscomp/test/DotDotDot.res b/jscomp/test/DotDotDot.res index 14f4dc657a..58a4769a7b 100644 --- a/jscomp/test/DotDotDot.res +++ b/jscomp/test/DotDotDot.res @@ -37,6 +37,9 @@ type svgProps = { let x: svgProps = {x: "test"} +// uncomment this to reveal a parser error +// type copiedSvgProps = {...svgProps} + module MultipleDotDotDots = { type t1 = {x: int} type t2 = {y: string} From 26d73c972b9854144adf934ac38d3ccc5eb0708e Mon Sep 17 00:00:00 2001 From: Gabriel Nordeborn Date: Fri, 14 Apr 2023 18:37:07 +0200 Subject: [PATCH 3/8] process optional fields also for record spreads --- jscomp/ml/typedecl.ml | 46 +++++++++++++++++++++++---------------- jscomp/test/DotDotDot.js | 6 +++++ jscomp/test/DotDotDot.res | 2 +- 3 files changed, 34 insertions(+), 20 deletions(-) diff --git a/jscomp/ml/typedecl.ml b/jscomp/ml/typedecl.ml index 77fc421d95..de698bb0c5 100644 --- a/jscomp/ml/typedecl.ml +++ b/jscomp/ml/typedecl.ml @@ -426,26 +426,19 @@ let transl_declaration ~typeRecordAsObject env sdecl id = Ttype_variant tcstrs, Type_variant cstrs, sdecl | Ptype_record lbls_ -> let has_optional attrs = Ext_list.exists attrs (fun ({txt },_) -> txt = "res.optional") in - let optionalLabels = - Ext_list.filter_map lbls_ - (fun lbl -> if has_optional lbl.pld_attributes then Some lbl.pld_name.txt else None) in - let lbls = - if optionalLabels = [] then lbls_ - else Ext_list.map lbls_ (fun lbl -> - let typ = lbl.pld_type in - let typ = + let hasOptionalLabels = + lbls_ |> List.exists(fun lbl -> has_optional lbl.pld_attributes) + in + let lbls = + if hasOptionalLabels then + Ext_list.map lbls_ (fun lbl -> + let typ = lbl.pld_type in if has_optional lbl.pld_attributes then - {typ with ptyp_desc = Ptyp_constr ({txt = Lident "option"; loc=typ.ptyp_loc}, [typ])} - else typ in - {lbl with pld_type = typ }) in - let lbls, lbls' = transl_labels env true lbls in - let rep = - if unbox then Record_unboxed false - else - if optionalLabels <> [] - then Record_optional_labels optionalLabels - else Record_regular + {lbl with pld_type={typ with ptyp_desc = Ptyp_constr ({txt = Lident "option"; loc=typ.ptyp_loc}, [typ])}} + else lbl) + else lbls_ in + let lbls, lbls' = transl_labels env true lbls in let lbls_opt = match lbls, lbls' with | {ld_name = {txt = "..."}; ld_type} :: _, _ :: _ -> let rec extract t = match t.desc with @@ -462,7 +455,13 @@ let transl_declaration ~typeRecordAsObject env sdecl id = | {ld_name = {txt = "..."}; ld_type} :: rest, _ :: rest' -> (match Ctype.extract_concrete_typedecl env (extract ld_type.ctyp_type) with (_p0, _p, {type_kind=Type_record (fields, _repr)}) -> - process_lbls (fst acc @ (fields |> List.map mkLbl), snd acc @ fields) rest rest' + process_lbls (fst acc @ (fields |> List.map(fun (lbl: Types.label_declaration) -> + let typ = lbl.ld_type in + let typ = + if has_optional lbl.ld_attributes then + (Ctype.newconstr (Path.Pident (Ident.create "option")) [typ]) + else typ in + {lbl with ld_type = typ }) |> List.map mkLbl), snd acc @ fields) rest rest' | _ -> assert false | exception _ -> None) | lbl::rest, lbl'::rest' -> process_lbls (fst acc @ [lbl], snd acc @ [lbl']) rest rest' @@ -479,6 +478,15 @@ let transl_declaration ~typeRecordAsObject env sdecl id = (match lbls_opt with | Some (lbls, lbls') -> check_duplicates lbls StringSet.empty; + let optionalLabels = + Ext_list.filter_map lbls (fun lbl -> + if has_optional lbl.ld_attributes then Some lbl.ld_name.txt else None) + in + let rep = + if unbox then Record_unboxed false + else if optionalLabels <> [] then Record_optional_labels optionalLabels + else Record_regular + in Ttype_record lbls, Type_record(lbls', rep), sdecl | None -> (* Could not find record type decl for ...t: assume t is an object type and this is syntax ambiguity *) diff --git a/jscomp/test/DotDotDot.js b/jscomp/test/DotDotDot.js index c2634c6cd8..011f4ba327 100644 --- a/jscomp/test/DotDotDot.js +++ b/jscomp/test/DotDotDot.js @@ -22,7 +22,13 @@ var v2 = { w: 2.0 }; +var x = { + name: "test", + x: "test" +}; + exports.v = v; exports.v2 = v2; +exports.x = x; exports.MultipleDotDotDots = MultipleDotDotDots; /* No side effect */ diff --git a/jscomp/test/DotDotDot.res b/jscomp/test/DotDotDot.res index 58a4769a7b..101b8594f2 100644 --- a/jscomp/test/DotDotDot.res +++ b/jscomp/test/DotDotDot.res @@ -35,7 +35,7 @@ type svgProps = { y?: string, } -let x: svgProps = {x: "test"} +let x: svgProps = {x: "test", name: "test"} // uncomment this to reveal a parser error // type copiedSvgProps = {...svgProps} From baac23886ab8dbc19dbd1d58a0dfbca7adadf099 Mon Sep 17 00:00:00 2001 From: Gabriel Nordeborn Date: Fri, 14 Apr 2023 21:02:32 +0200 Subject: [PATCH 4/8] revert unneeded changes --- jscomp/ml/typedecl.ml | 37 +++++++++++++++++-------------------- 1 file changed, 17 insertions(+), 20 deletions(-) diff --git a/jscomp/ml/typedecl.ml b/jscomp/ml/typedecl.ml index de698bb0c5..66e4c949f4 100644 --- a/jscomp/ml/typedecl.ml +++ b/jscomp/ml/typedecl.ml @@ -425,20 +425,20 @@ let transl_declaration ~typeRecordAsObject env sdecl id = Ast_untagged_variants.check_well_formed ~isUntaggedDef cstrs; Ttype_variant tcstrs, Type_variant cstrs, sdecl | Ptype_record lbls_ -> - let has_optional attrs = Ext_list.exists attrs (fun ({txt },_) -> txt = "res.optional") in - let hasOptionalLabels = - lbls_ |> List.exists(fun lbl -> has_optional lbl.pld_attributes) - in - let lbls = - if hasOptionalLabels then - Ext_list.map lbls_ (fun lbl -> - let typ = lbl.pld_type in - if has_optional lbl.pld_attributes then - {lbl with pld_type={typ with ptyp_desc = Ptyp_constr ({txt = Lident "option"; loc=typ.ptyp_loc}, [typ])}} - else lbl) - else lbls_ - in - let lbls, lbls' = transl_labels env true lbls in + let has_optional attrs = Ext_list.exists attrs (fun ({txt },_) -> txt = "res.optional") in + let optionalLabels = + Ext_list.filter_map lbls_ + (fun lbl -> if has_optional lbl.pld_attributes then Some lbl.pld_name.txt else None) in + let lbls = + if optionalLabels = [] then lbls_ + else Ext_list.map lbls_ (fun lbl -> + let typ = lbl.pld_type in + let typ = + if has_optional lbl.pld_attributes then + {typ with ptyp_desc = Ptyp_constr ({txt = Lident "option"; loc=typ.ptyp_loc}, [typ])} + else typ in + {lbl with pld_type = typ }) in + let lbls, lbls' = transl_labels env true lbls in let lbls_opt = match lbls, lbls' with | {ld_name = {txt = "..."}; ld_type} :: _, _ :: _ -> let rec extract t = match t.desc with @@ -482,12 +482,9 @@ let transl_declaration ~typeRecordAsObject env sdecl id = Ext_list.filter_map lbls (fun lbl -> if has_optional lbl.ld_attributes then Some lbl.ld_name.txt else None) in - let rep = - if unbox then Record_unboxed false - else if optionalLabels <> [] then Record_optional_labels optionalLabels - else Record_regular - in - Ttype_record lbls, Type_record(lbls', rep), sdecl + Ttype_record lbls, Type_record(lbls', if unbox then Record_unboxed false + else if optionalLabels <> [] then Record_optional_labels optionalLabels + else Record_regular), sdecl | None -> (* Could not find record type decl for ...t: assume t is an object type and this is syntax ambiguity *) typeRecordAsObject := true; From db7f427433e4401f3c1c247b17392e2bf707d080 Mon Sep 17 00:00:00 2001 From: Gabriel Nordeborn Date: Fri, 14 Apr 2023 21:03:16 +0200 Subject: [PATCH 5/8] revert unneeded changes --- jscomp/ml/typedecl.ml | 28 ++++++++++++++-------------- 1 file changed, 14 insertions(+), 14 deletions(-) diff --git a/jscomp/ml/typedecl.ml b/jscomp/ml/typedecl.ml index 66e4c949f4..93ce50e2e8 100644 --- a/jscomp/ml/typedecl.ml +++ b/jscomp/ml/typedecl.ml @@ -425,20 +425,20 @@ let transl_declaration ~typeRecordAsObject env sdecl id = Ast_untagged_variants.check_well_formed ~isUntaggedDef cstrs; Ttype_variant tcstrs, Type_variant cstrs, sdecl | Ptype_record lbls_ -> - let has_optional attrs = Ext_list.exists attrs (fun ({txt },_) -> txt = "res.optional") in - let optionalLabels = - Ext_list.filter_map lbls_ - (fun lbl -> if has_optional lbl.pld_attributes then Some lbl.pld_name.txt else None) in - let lbls = - if optionalLabels = [] then lbls_ - else Ext_list.map lbls_ (fun lbl -> - let typ = lbl.pld_type in - let typ = - if has_optional lbl.pld_attributes then - {typ with ptyp_desc = Ptyp_constr ({txt = Lident "option"; loc=typ.ptyp_loc}, [typ])} - else typ in - {lbl with pld_type = typ }) in - let lbls, lbls' = transl_labels env true lbls in + let has_optional attrs = Ext_list.exists attrs (fun ({txt },_) -> txt = "res.optional") in + let optionalLabels = + Ext_list.filter_map lbls_ + (fun lbl -> if has_optional lbl.pld_attributes then Some lbl.pld_name.txt else None) in + let lbls = + if optionalLabels = [] then lbls_ + else Ext_list.map lbls_ (fun lbl -> + let typ = lbl.pld_type in + let typ = + if has_optional lbl.pld_attributes then + {typ with ptyp_desc = Ptyp_constr ({txt = Lident "option"; loc=typ.ptyp_loc}, [typ])} + else typ in + {lbl with pld_type = typ }) in + let lbls, lbls' = transl_labels env true lbls in let lbls_opt = match lbls, lbls' with | {ld_name = {txt = "..."}; ld_type} :: _, _ :: _ -> let rec extract t = match t.desc with From ff3448f94327ac3d34a57bc234974251407eebbd Mon Sep 17 00:00:00 2001 From: Gabriel Nordeborn Date: Sat, 15 Apr 2023 09:51:24 +0200 Subject: [PATCH 6/8] revert more uneccessary changes --- jscomp/ml/typedecl.ml | 8 +------- 1 file changed, 1 insertion(+), 7 deletions(-) diff --git a/jscomp/ml/typedecl.ml b/jscomp/ml/typedecl.ml index 93ce50e2e8..0e6246a759 100644 --- a/jscomp/ml/typedecl.ml +++ b/jscomp/ml/typedecl.ml @@ -455,13 +455,7 @@ let transl_declaration ~typeRecordAsObject env sdecl id = | {ld_name = {txt = "..."}; ld_type} :: rest, _ :: rest' -> (match Ctype.extract_concrete_typedecl env (extract ld_type.ctyp_type) with (_p0, _p, {type_kind=Type_record (fields, _repr)}) -> - process_lbls (fst acc @ (fields |> List.map(fun (lbl: Types.label_declaration) -> - let typ = lbl.ld_type in - let typ = - if has_optional lbl.ld_attributes then - (Ctype.newconstr (Path.Pident (Ident.create "option")) [typ]) - else typ in - {lbl with ld_type = typ }) |> List.map mkLbl), snd acc @ fields) rest rest' + process_lbls (fst acc @ (fields |> List.map mkLbl), snd acc @ fields) rest rest' | _ -> assert false | exception _ -> None) | lbl::rest, lbl'::rest' -> process_lbls (fst acc @ [lbl], snd acc @ [lbl']) rest rest' From f9bcb41c49f70bcfadf3f291e31df34c4fb71771 Mon Sep 17 00:00:00 2001 From: Gabriel Nordeborn Date: Sat, 15 Apr 2023 12:17:47 +0200 Subject: [PATCH 7/8] formatting --- jscomp/ml/typedecl.ml | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/jscomp/ml/typedecl.ml b/jscomp/ml/typedecl.ml index 0e6246a759..b1cd3190e1 100644 --- a/jscomp/ml/typedecl.ml +++ b/jscomp/ml/typedecl.ml @@ -476,9 +476,11 @@ let transl_declaration ~typeRecordAsObject env sdecl id = Ext_list.filter_map lbls (fun lbl -> if has_optional lbl.ld_attributes then Some lbl.ld_name.txt else None) in - Ttype_record lbls, Type_record(lbls', if unbox then Record_unboxed false - else if optionalLabels <> [] then Record_optional_labels optionalLabels - else Record_regular), sdecl + Ttype_record lbls, Type_record(lbls', if unbox then + Record_unboxed false + else if optionalLabels <> [] then + Record_optional_labels optionalLabels + else Record_regular), sdecl | None -> (* Could not find record type decl for ...t: assume t is an object type and this is syntax ambiguity *) typeRecordAsObject := true; From c362789e45ad1c4297b9029fb5d721f3f96a8e4e Mon Sep 17 00:00:00 2001 From: Gabriel Nordeborn Date: Sat, 15 Apr 2023 12:19:04 +0200 Subject: [PATCH 8/8] changelog --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index f4269b52cd..a020c478be 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -15,6 +15,7 @@ #### :bug: Bug Fix - Fix broken formatting in uncurried mode for functions with _ placeholder args. https://github.com/rescript-lang/rescript-compiler/pull/6148 +- Fix issue where spreading record types with optional labels would not have their labels preserved as optional. https://github.com/rescript-lang/rescript-compiler/pull/6154 # 11.0.0-alpha.3