From ffbe0acb4d9872fac531307614dac079dcf01fc8 Mon Sep 17 00:00:00 2001 From: Shulhi Sapli Date: Tue, 20 Aug 2024 09:00:30 +0800 Subject: [PATCH 1/6] Handle parens when mod type is a module signature --- jscomp/syntax/src/res_printer.ml | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/jscomp/syntax/src/res_printer.ml b/jscomp/syntax/src/res_printer.ml index ae8055d7b5..dc1750071c 100644 --- a/jscomp/syntax/src/res_printer.ml +++ b/jscomp/syntax/src/res_printer.ml @@ -685,6 +685,18 @@ and print_module_binding ~state ~is_rec module_binding cmt_tbl i = Doc.concat [Doc.text ": "; print_mod_type ~state mod_type cmt_tbl] ) | mod_expr -> (print_mod_expr ~state mod_expr cmt_tbl, Doc.nil) in + let mod_expr_doc_parens = + match module_binding.pmb_expr with + | { + pmod_desc = + Pmod_constraint + ( _, + {Parsetree.pmty_desc = Pmty_signature [{psig_desc = Psig_module _}]} + ); + } -> + true + | _ -> false + in let mod_name = let doc = Doc.text module_binding.pmb_name.Location.txt in print_comments doc cmt_tbl module_binding.pmb_name.loc @@ -698,7 +710,9 @@ and print_module_binding ~state ~is_rec module_binding cmt_tbl i = mod_name; mod_constraint_doc; Doc.text " = "; + (if mod_expr_doc_parens then Doc.lparen else Doc.nil); mod_expr_doc; + (if mod_expr_doc_parens then Doc.rparen else Doc.nil); ] in print_comments doc cmt_tbl module_binding.pmb_loc From 7d02a2643ed0618e5f1e8ccdcf15a71753f6fc90 Mon Sep 17 00:00:00 2001 From: Shulhi Sapli Date: Tue, 20 Aug 2024 09:11:20 +0800 Subject: [PATCH 2/6] Refactor --- jscomp/syntax/src/res_printer.ml | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/jscomp/syntax/src/res_printer.ml b/jscomp/syntax/src/res_printer.ml index dc1750071c..1c8ae210b2 100644 --- a/jscomp/syntax/src/res_printer.ml +++ b/jscomp/syntax/src/res_printer.ml @@ -694,8 +694,8 @@ and print_module_binding ~state ~is_rec module_binding cmt_tbl i = {Parsetree.pmty_desc = Pmty_signature [{psig_desc = Psig_module _}]} ); } -> - true - | _ -> false + Doc.concat [Doc.lparen; mod_expr_doc; Doc.rparen] + | _ -> mod_expr_doc in let mod_name = let doc = Doc.text module_binding.pmb_name.Location.txt in @@ -710,9 +710,7 @@ and print_module_binding ~state ~is_rec module_binding cmt_tbl i = mod_name; mod_constraint_doc; Doc.text " = "; - (if mod_expr_doc_parens then Doc.lparen else Doc.nil); - mod_expr_doc; - (if mod_expr_doc_parens then Doc.rparen else Doc.nil); + mod_expr_doc_parens; ] in print_comments doc cmt_tbl module_binding.pmb_loc From 0b6cce76e45023300f4d044c6170d0644e802e53 Mon Sep 17 00:00:00 2001 From: Shulhi Sapli Date: Tue, 20 Aug 2024 09:13:38 +0800 Subject: [PATCH 3/6] Update CHANGELOG --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 8ac5645a55..62e5ce9631 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -26,6 +26,7 @@ #### :bug: Bug Fix - Fix issue where long layout break added a trailing comma in partial application `...`. https://github.com/rescript-lang/rescript-compiler/pull/6949 - Fix incorrect format of function under unary operator. https://github.com/rescript-lang/rescript-compiler/pull/6953 +- Fix incorrect incorrect printing of module binding with signature. https://github.com/rescript-lang/rescript-compiler/pull/6963 # 12.0.0-alpha.1 From 82ee3da883c0035ae3f00328a35e32e6621bff8f Mon Sep 17 00:00:00 2001 From: Shulhi Sapli Date: Tue, 20 Aug 2024 09:19:54 +0800 Subject: [PATCH 4/6] Add tests --- .../modExpr/expected/structure.res.txt | 19 ++++++++++++++++ .../tests/printer/modExpr/structure.res | 22 ++++++++++++++++++- 2 files changed, 40 insertions(+), 1 deletion(-) diff --git a/jscomp/syntax/tests/printer/modExpr/expected/structure.res.txt b/jscomp/syntax/tests/printer/modExpr/expected/structure.res.txt index b5016cf426..75041ed6c6 100644 --- a/jscomp/syntax/tests/printer/modExpr/expected/structure.res.txt +++ b/jscomp/syntax/tests/printer/modExpr/expected/structure.res.txt @@ -22,3 +22,22 @@ let g = { module M: T = {} 0 } + +module M7: { + module N': { + let x: int + } +} = (M6: { + module N: { + let x: int + } + module N' = N +}) + +module M8 = M7 + +module M7: { + let x: int +} = { + let x = 8 +} diff --git a/jscomp/syntax/tests/printer/modExpr/structure.res b/jscomp/syntax/tests/printer/modExpr/structure.res index a0bd2ff7c4..f6e87e4b22 100644 --- a/jscomp/syntax/tests/printer/modExpr/structure.res +++ b/jscomp/syntax/tests/printer/modExpr/structure.res @@ -21,4 +21,24 @@ module type T = {} let g = { module M: T = {} 0 -} \ No newline at end of file +} + +module M7: { + module N': { + let x: int + } +} = (M6: { + module N: { + let x: int + } + module N' = N +}) + + +module M8 = M7 + +module M7: { + let x: int +} = { + let x = 8 +} From e35efc51e1a6a2a8c837a5d9f729ce8439549134 Mon Sep 17 00:00:00 2001 From: Shulhi Sapli Date: Tue, 20 Aug 2024 12:10:00 +0800 Subject: [PATCH 5/6] Handle more edge cases --- jscomp/syntax/src/res_parens.ml | 17 +++++++++++++++++ jscomp/syntax/src/res_parens.mli | 2 ++ jscomp/syntax/src/res_printer.ml | 11 ++--------- 3 files changed, 21 insertions(+), 9 deletions(-) diff --git a/jscomp/syntax/src/res_parens.ml b/jscomp/syntax/src/res_parens.ml index b100933e86..336af9f77f 100644 --- a/jscomp/syntax/src/res_parens.ml +++ b/jscomp/syntax/src/res_parens.ml @@ -452,6 +452,23 @@ let include_mod_expr mod_expr = | Parsetree.Pmod_constraint _ -> true | _ -> false +let mod_expr_parens mod_expr = + match mod_expr with + | { + Parsetree.pmod_desc = + Pmod_constraint + ( {Parsetree.pmod_desc = Pmod_structure _}, + {Parsetree.pmty_desc = Pmty_signature [{psig_desc = Psig_module _}]} ); + } -> + false + | { + Parsetree.pmod_desc = + Pmod_constraint + (_, {Parsetree.pmty_desc = Pmty_signature [{psig_desc = Psig_module _}]}); + } -> + true + | _ -> false + let arrow_return_typ_expr typ_expr = match typ_expr.Parsetree.ptyp_desc with | Parsetree.Ptyp_arrow _ -> true diff --git a/jscomp/syntax/src/res_parens.mli b/jscomp/syntax/src/res_parens.mli index 28e35a6345..7c56c0ab56 100644 --- a/jscomp/syntax/src/res_parens.mli +++ b/jscomp/syntax/src/res_parens.mli @@ -33,6 +33,8 @@ val call_expr : Parsetree.expression -> kind val include_mod_expr : Parsetree.module_expr -> bool +val mod_expr_parens : Parsetree.module_expr -> bool + val arrow_return_typ_expr : Parsetree.core_type -> bool val pattern_record_row_rhs : Parsetree.pattern -> bool diff --git a/jscomp/syntax/src/res_printer.ml b/jscomp/syntax/src/res_printer.ml index 1c8ae210b2..7b673eddac 100644 --- a/jscomp/syntax/src/res_printer.ml +++ b/jscomp/syntax/src/res_printer.ml @@ -686,16 +686,9 @@ and print_module_binding ~state ~is_rec module_binding cmt_tbl i = | mod_expr -> (print_mod_expr ~state mod_expr cmt_tbl, Doc.nil) in let mod_expr_doc_parens = - match module_binding.pmb_expr with - | { - pmod_desc = - Pmod_constraint - ( _, - {Parsetree.pmty_desc = Pmty_signature [{psig_desc = Psig_module _}]} - ); - } -> + if Parens.mod_expr_parens module_binding.pmb_expr then Doc.concat [Doc.lparen; mod_expr_doc; Doc.rparen] - | _ -> mod_expr_doc + else mod_expr_doc in let mod_name = let doc = Doc.text module_binding.pmb_name.Location.txt in From 2a65d0eaf83a9f5a9aac9bcd1f80bac8c47b27e8 Mon Sep 17 00:00:00 2001 From: Shulhi Sapli Date: Tue, 20 Aug 2024 12:25:56 +0800 Subject: [PATCH 6/6] Add more test cases from jscomp/test/coercion_module_alias_test.res --- .../modExpr/expected/structure.res.txt | 27 +++++++++++++++++++ .../tests/printer/modExpr/structure.res | 27 +++++++++++++++++++ 2 files changed, 54 insertions(+) diff --git a/jscomp/syntax/tests/printer/modExpr/expected/structure.res.txt b/jscomp/syntax/tests/printer/modExpr/expected/structure.res.txt index 75041ed6c6..a958790e76 100644 --- a/jscomp/syntax/tests/printer/modExpr/expected/structure.res.txt +++ b/jscomp/syntax/tests/printer/modExpr/expected/structure.res.txt @@ -36,8 +36,35 @@ module M7: { module M8 = M7 +module M5 = G0() + module M7: { let x: int } = { let x = 8 } + +module M3: { + module N': { + let x: int + } +} = { + include M' +} + +module G0: (X: {}) => +{ + module N': { + let x: int + } +} = F0 + +module M6 = { + module D = { + let y = 3 + } + module N = { + let x = 1 + } + module N' = N +} diff --git a/jscomp/syntax/tests/printer/modExpr/structure.res b/jscomp/syntax/tests/printer/modExpr/structure.res index f6e87e4b22..86a882d62d 100644 --- a/jscomp/syntax/tests/printer/modExpr/structure.res +++ b/jscomp/syntax/tests/printer/modExpr/structure.res @@ -37,8 +37,35 @@ module M7: { module M8 = M7 +module M5 = G0() + module M7: { let x: int } = { let x = 8 } + +module M3: { + module N': { + let x: int + } +} = { + include M' +} + +module G0: (X: {}) => +{ + module N': { + let x: int + } +} = F0 + +module M6 = { + module D = { + let y = 3 + } + module N = { + let x = 1 + } + module N' = N +}