From 113266443798bbc0ed490ff5a76b2c82dc6317a5 Mon Sep 17 00:00:00 2001 From: Shulhi Sapli <913103+shulhi@users.noreply.github.com> Date: Mon, 24 Nov 2025 20:46:37 +0800 Subject: [PATCH 1/6] Remove underscore if placeholder is in first pos --- compiler/syntax/src/res_printer.ml | 44 ++++++++++++++++++- .../expr/expected/underscoreApply.res.txt | 6 +-- 2 files changed, 46 insertions(+), 4 deletions(-) diff --git a/compiler/syntax/src/res_printer.ml b/compiler/syntax/src/res_printer.ml index 7132e16dd9..67c32f8811 100644 --- a/compiler/syntax/src/res_printer.ml +++ b/compiler/syntax/src/res_printer.ml @@ -3985,7 +3985,49 @@ and print_binary_expression ~state (expr : Parsetree.expression) cmt_tbl = -> let lhs_has_comment_below = has_comment_below cmt_tbl lhs.pexp_loc in let lhs_doc = print_operand ~is_lhs:true ~is_multiline:false lhs op in - let rhs_doc = print_operand ~is_lhs:false ~is_multiline:false rhs op in + let rhs_doc = + (* For pipe operator, if RHS is (__x) => f(__x, ...), remove the first __x arg + so it prints as a->f(...) instead of a->f(_, ...) *) + let rhs_to_print = + match rhs.pexp_desc with + | Pexp_fun + { + arg_label = Nolabel; + default = None; + lhs = {ppat_desc = Ppat_var {txt = "__x"}} as pat; + rhs = + {pexp_desc = Pexp_apply {funct; args; partial; transformed_jsx}} + as body; + arity; + async; + } -> ( + match args with + | (Nolabel, {pexp_desc = Pexp_ident {txt = Longident.Lident "__x"}}) + :: rest_args -> + { + rhs with + pexp_desc = + Pexp_fun + { + arg_label = Nolabel; + default = None; + lhs = pat; + rhs = + { + body with + pexp_desc = + Pexp_apply + {funct; args = rest_args; partial; transformed_jsx}; + }; + arity; + async; + }; + } + | _ -> rhs) + | _ -> rhs + in + print_operand ~is_lhs:false ~is_multiline:false rhs_to_print op + in Doc.group (Doc.concat [ diff --git a/tests/syntax_tests/data/printer/expr/expected/underscoreApply.res.txt b/tests/syntax_tests/data/printer/expr/expected/underscoreApply.res.txt index 022a0f759a..488b19aa9e 100644 --- a/tests/syntax_tests/data/printer/expr/expected/underscoreApply.res.txt +++ b/tests/syntax_tests/data/printer/expr/expected/underscoreApply.res.txt @@ -12,7 +12,7 @@ let nested2 = (x, y, z) => List.length(_) let l = [1, 2, 3]->List.map(i => i + 1, _)->List.filter(i => i > 0, _) -let l = (i => i + 1)->List.map(_, [1, 2, 3]) +let l = (i => i + 1)->List.map([1, 2, 3]) let x = List.length(_) @@ -52,8 +52,8 @@ f(a, b, _)[ix] = 2 getDirector(a, b, _).name = "Steve" -filterNone->Array.get(_, 0) -filterNone->Array.get(_, 0) +filterNone->Array.get(0) +filterNone->Array.get(0) Array.get(_, 0) 1 + Array.get(_, 0) Array.get(_, 1) + Array.get(_, 0) From e2c34b3ba310443a9fe68ade0acfb3b7529bdc2e Mon Sep 17 00:00:00 2001 From: Shulhi Sapli <913103+shulhi@users.noreply.github.com> Date: Fri, 28 Nov 2025 19:24:13 +0800 Subject: [PATCH 2/6] Only apply in pipe context --- compiler/syntax/src/res_parsetree_viewer.ml | 44 ++++++++++++++++++ compiler/syntax/src/res_parsetree_viewer.mli | 2 + compiler/syntax/src/res_printer.ml | 47 +++----------------- 3 files changed, 51 insertions(+), 42 deletions(-) diff --git a/compiler/syntax/src/res_parsetree_viewer.ml b/compiler/syntax/src/res_parsetree_viewer.ml index 0d972819d8..54f2378edf 100644 --- a/compiler/syntax/src/res_parsetree_viewer.ml +++ b/compiler/syntax/src/res_parsetree_viewer.ml @@ -141,6 +141,50 @@ let rewrite_underscore_apply expr = } | _ -> expr +(* For pipe RHS: (__x) => f(__x, a, b) -----> f(a, b) + Omits the first __x argument since the piped value fills that position *) +let rewrite_underscore_apply_in_pipe expr = + match expr.pexp_desc with + | Pexp_fun + { + arg_label = Nolabel; + default = None; + lhs = {ppat_desc = Ppat_var {txt = "__x"}}; + rhs = {pexp_desc = Pexp_apply {funct = call_expr; args}} as e; + } -> ( + match args with + | (Nolabel, {pexp_desc = Pexp_ident {txt = Longident.Lident "__x"}}) + :: rest_args -> + (* First arg is __x, skip it and convert remaining __x to _ *) + let new_args = + List.map + (fun arg -> + match arg with + | ( lbl, + ({pexp_desc = Pexp_ident ({txt = Longident.Lident "__x"} as lid)} + as arg_expr) ) -> + ( lbl, + { + arg_expr with + pexp_desc = Pexp_ident {lid with txt = Longident.Lident "_"}; + } ) + | arg -> arg) + rest_args + in + { + e with + pexp_desc = + Pexp_apply + { + funct = call_expr; + args = new_args; + partial = false; + transformed_jsx = false; + }; + } + | _ -> rewrite_underscore_apply expr) + | _ -> expr + type fun_param_kind = | Parameter of { attrs: Parsetree.attributes; diff --git a/compiler/syntax/src/res_parsetree_viewer.mli b/compiler/syntax/src/res_parsetree_viewer.mli index 92108d5018..919d08a92a 100644 --- a/compiler/syntax/src/res_parsetree_viewer.mli +++ b/compiler/syntax/src/res_parsetree_viewer.mli @@ -146,6 +146,8 @@ val is_single_pipe_expr : Parsetree.expression -> bool (* (__x) => f(a, __x, c) -----> f(a, _, c) *) val rewrite_underscore_apply : Parsetree.expression -> Parsetree.expression +val rewrite_underscore_apply_in_pipe : Parsetree.expression -> Parsetree.expression + (* (__x) => f(a, __x, c) -----> f(a, _, c) *) val is_underscore_apply_sugar : Parsetree.expression -> bool diff --git a/compiler/syntax/src/res_printer.ml b/compiler/syntax/src/res_printer.ml index 67c32f8811..a2780aaf7e 100644 --- a/compiler/syntax/src/res_printer.ml +++ b/compiler/syntax/src/res_printer.ml @@ -3985,49 +3985,12 @@ and print_binary_expression ~state (expr : Parsetree.expression) cmt_tbl = -> let lhs_has_comment_below = has_comment_below cmt_tbl lhs.pexp_loc in let lhs_doc = print_operand ~is_lhs:true ~is_multiline:false lhs op in - let rhs_doc = - (* For pipe operator, if RHS is (__x) => f(__x, ...), remove the first __x arg - so it prints as a->f(...) instead of a->f(_, ...) *) - let rhs_to_print = - match rhs.pexp_desc with - | Pexp_fun - { - arg_label = Nolabel; - default = None; - lhs = {ppat_desc = Ppat_var {txt = "__x"}} as pat; - rhs = - {pexp_desc = Pexp_apply {funct; args; partial; transformed_jsx}} - as body; - arity; - async; - } -> ( - match args with - | (Nolabel, {pexp_desc = Pexp_ident {txt = Longident.Lident "__x"}}) - :: rest_args -> - { - rhs with - pexp_desc = - Pexp_fun - { - arg_label = Nolabel; - default = None; - lhs = pat; - rhs = - { - body with - pexp_desc = - Pexp_apply - {funct; args = rest_args; partial; transformed_jsx}; - }; - arity; - async; - }; - } - | _ -> rhs) - | _ -> rhs - in - print_operand ~is_lhs:false ~is_multiline:false rhs_to_print op + (* For pipe RHS, use pipe-specific rewrite to omit redundant first underscore *) + let rhs = + if op = "->" then ParsetreeViewer.rewrite_underscore_apply_in_pipe rhs + else rhs in + let rhs_doc = print_operand ~is_lhs:false ~is_multiline:false rhs op in Doc.group (Doc.concat [ From ab3c5728962d019dffa4b7c4b13897110e51f2e3 Mon Sep 17 00:00:00 2001 From: Shulhi Sapli <913103+shulhi@users.noreply.github.com> Date: Fri, 28 Nov 2025 21:10:23 +0800 Subject: [PATCH 3/6] Format to canonical form before checking for idempotency test --- compiler/syntax/src/res_parsetree_viewer.ml | 5 +++-- compiler/syntax/src/res_parsetree_viewer.mli | 3 ++- scripts/test_syntax.sh | 10 +++++++++- 3 files changed, 14 insertions(+), 4 deletions(-) diff --git a/compiler/syntax/src/res_parsetree_viewer.ml b/compiler/syntax/src/res_parsetree_viewer.ml index 54f2378edf..233ac8067f 100644 --- a/compiler/syntax/src/res_parsetree_viewer.ml +++ b/compiler/syntax/src/res_parsetree_viewer.ml @@ -161,8 +161,9 @@ let rewrite_underscore_apply_in_pipe expr = (fun arg -> match arg with | ( lbl, - ({pexp_desc = Pexp_ident ({txt = Longident.Lident "__x"} as lid)} - as arg_expr) ) -> + ({ + pexp_desc = Pexp_ident ({txt = Longident.Lident "__x"} as lid); + } as arg_expr) ) -> ( lbl, { arg_expr with diff --git a/compiler/syntax/src/res_parsetree_viewer.mli b/compiler/syntax/src/res_parsetree_viewer.mli index 919d08a92a..5fcfb6883c 100644 --- a/compiler/syntax/src/res_parsetree_viewer.mli +++ b/compiler/syntax/src/res_parsetree_viewer.mli @@ -146,7 +146,8 @@ val is_single_pipe_expr : Parsetree.expression -> bool (* (__x) => f(a, __x, c) -----> f(a, _, c) *) val rewrite_underscore_apply : Parsetree.expression -> Parsetree.expression -val rewrite_underscore_apply_in_pipe : Parsetree.expression -> Parsetree.expression +val rewrite_underscore_apply_in_pipe : + Parsetree.expression -> Parsetree.expression (* (__x) => f(a, __x, c) -----> f(a, _, c) *) val is_underscore_apply_sugar : Parsetree.expression -> bool diff --git a/scripts/test_syntax.sh b/scripts/test_syntax.sh index 85f09f7d57..d6e11166b5 100755 --- a/scripts/test_syntax.sh +++ b/scripts/test_syntax.sh @@ -81,6 +81,7 @@ if [[ $ROUNDTRIP_TEST = 1 ]]; then mkdir -p temp/$(dirname $file) sexpAst1=temp/$file.sexp sexpAst2=temp/$file.2.sexp + sexpAst3=temp/$file.3.sexp rescript1=temp/$file.res rescript2=temp/$file.2.res @@ -89,14 +90,21 @@ if [[ $ROUNDTRIP_TEST = 1 ]]; then *.resi ) resIntf=-interface ;; esac + # First pass: original file -> AST1 and text1 $DUNE_BIN_DIR/res_parser $resIntf -print sexp $file > $sexpAst1 $DUNE_BIN_DIR/res_parser $resIntf -print res $file > $rescript1 + # Second pass: text1 -> AST2 and text2 $DUNE_BIN_DIR/res_parser $resIntf -print sexp $rescript1 > $sexpAst2 $DUNE_BIN_DIR/res_parser $resIntf -print res $rescript1 > $rescript2 - diff --unified $sexpAst1 $sexpAst2 + # Third pass: text2 -> AST3 (to check idempotency after normalization) + $DUNE_BIN_DIR/res_parser $resIntf -print sexp $rescript2 > $sexpAst3 + + # Check AST idempotency: AST2 should equal AST3 (allows AST1 != AST2 for canonicalization) + diff --unified $sexpAst2 $sexpAst3 [[ "$?" = 1 ]] && echo 1 > $roundtripTestsResult + # Check text idempotency: text1 should equal text2 diff --unified $rescript1 $rescript2 [[ "$?" = 1 ]] && echo 1 > $roundtripTestsResult } & maybeWait From 22007f7a80e9663218587c16682772517518d063 Mon Sep 17 00:00:00 2001 From: Shulhi Sapli <913103+shulhi@users.noreply.github.com> Date: Fri, 28 Nov 2025 22:03:03 +0800 Subject: [PATCH 4/6] Only format for single placeholder occurence to preserve semantics --- compiler/syntax/src/res_parsetree_viewer.ml | 65 +++++++++++++-------- 1 file changed, 40 insertions(+), 25 deletions(-) diff --git a/compiler/syntax/src/res_parsetree_viewer.ml b/compiler/syntax/src/res_parsetree_viewer.ml index 233ac8067f..787cffdb61 100644 --- a/compiler/syntax/src/res_parsetree_viewer.ml +++ b/compiler/syntax/src/res_parsetree_viewer.ml @@ -142,7 +142,8 @@ let rewrite_underscore_apply expr = | _ -> expr (* For pipe RHS: (__x) => f(__x, a, b) -----> f(a, b) - Omits the first __x argument since the piped value fills that position *) + Omits the first __x argument only if it's the sole occurrence. + If multiple __x exist (e.g., f(__x, __x, b)), keeps all to preserve semantics. *) let rewrite_underscore_apply_in_pipe expr = match expr.pexp_desc with | Pexp_fun @@ -155,34 +156,48 @@ let rewrite_underscore_apply_in_pipe expr = match args with | (Nolabel, {pexp_desc = Pexp_ident {txt = Longident.Lident "__x"}}) :: rest_args -> - (* First arg is __x, skip it and convert remaining __x to _ *) - let new_args = - List.map + (* Count occurrences of __x in remaining arguments *) + let has_other_underscore = + List.exists (fun arg -> match arg with - | ( lbl, - ({ - pexp_desc = Pexp_ident ({txt = Longident.Lident "__x"} as lid); - } as arg_expr) ) -> - ( lbl, - { - arg_expr with - pexp_desc = Pexp_ident {lid with txt = Longident.Lident "_"}; - } ) - | arg -> arg) + | _, {pexp_desc = Pexp_ident {txt = Longident.Lident "__x"}} -> true + | _ -> false) rest_args in - { - e with - pexp_desc = - Pexp_apply - { - funct = call_expr; - args = new_args; - partial = false; - transformed_jsx = false; - }; - } + if has_other_underscore then + (* Multiple __x found - don't skip first one, use regular rewrite *) + rewrite_underscore_apply expr + else + (* Only one __x (in first position) - safe to skip *) + let new_args = + List.map + (fun arg -> + match arg with + | ( lbl, + ({ + pexp_desc = + Pexp_ident ({txt = Longident.Lident "__x"} as lid); + } as arg_expr) ) -> + ( lbl, + { + arg_expr with + pexp_desc = Pexp_ident {lid with txt = Longident.Lident "_"}; + } ) + | arg -> arg) + rest_args + in + { + e with + pexp_desc = + Pexp_apply + { + funct = call_expr; + args = new_args; + partial = false; + transformed_jsx = false; + }; + } | _ -> rewrite_underscore_apply expr) | _ -> expr From 65ac40c802be9279a5791691302a494a513a45b2 Mon Sep 17 00:00:00 2001 From: Shulhi Sapli <913103+shulhi@users.noreply.github.com> Date: Wed, 24 Dec 2025 20:46:12 +0800 Subject: [PATCH 5/6] WIP --- compiler/syntax/src/res_parsetree_viewer.ml | 57 +++++++++------------ 1 file changed, 23 insertions(+), 34 deletions(-) diff --git a/compiler/syntax/src/res_parsetree_viewer.ml b/compiler/syntax/src/res_parsetree_viewer.ml index 787cffdb61..cb89217d13 100644 --- a/compiler/syntax/src/res_parsetree_viewer.ml +++ b/compiler/syntax/src/res_parsetree_viewer.ml @@ -145,55 +145,44 @@ let rewrite_underscore_apply expr = Omits the first __x argument only if it's the sole occurrence. If multiple __x exist (e.g., f(__x, __x, b)), keeps all to preserve semantics. *) let rewrite_underscore_apply_in_pipe expr = + let is_underscore_arg = function + | _, {pexp_desc = Pexp_ident {txt = Longident.Lident "__x"}} -> true + | _ -> false + in + let convert_underscore_to_placeholder arg = + match arg with + | ( lbl, + ({pexp_desc = Pexp_ident ({txt = Longident.Lident "__x"} as lid)} as + arg_expr) ) -> + ( lbl, + { + arg_expr with + pexp_desc = Pexp_ident {lid with txt = Longident.Lident "_"}; + } ) + | arg -> arg + in match expr.pexp_desc with | Pexp_fun { arg_label = Nolabel; default = None; lhs = {ppat_desc = Ppat_var {txt = "__x"}}; - rhs = {pexp_desc = Pexp_apply {funct = call_expr; args}} as e; + rhs = {pexp_desc = Pexp_apply {funct; args}} as e; } -> ( match args with - | (Nolabel, {pexp_desc = Pexp_ident {txt = Longident.Lident "__x"}}) - :: rest_args -> - (* Count occurrences of __x in remaining arguments *) - let has_other_underscore = - List.exists - (fun arg -> - match arg with - | _, {pexp_desc = Pexp_ident {txt = Longident.Lident "__x"}} -> true - | _ -> false) - rest_args - in - if has_other_underscore then - (* Multiple __x found - don't skip first one, use regular rewrite *) + | first_arg :: rest_args when is_underscore_arg first_arg -> + if List.exists is_underscore_arg rest_args then + (* Multiple __x - keep all to preserve semantics *) rewrite_underscore_apply expr else - (* Only one __x (in first position) - safe to skip *) - let new_args = - List.map - (fun arg -> - match arg with - | ( lbl, - ({ - pexp_desc = - Pexp_ident ({txt = Longident.Lident "__x"} as lid); - } as arg_expr) ) -> - ( lbl, - { - arg_expr with - pexp_desc = Pexp_ident {lid with txt = Longident.Lident "_"}; - } ) - | arg -> arg) - rest_args - in + (* Single __x in first position - safe to omit *) { e with pexp_desc = Pexp_apply { - funct = call_expr; - args = new_args; + funct; + args = List.map convert_underscore_to_placeholder rest_args; partial = false; transformed_jsx = false; }; From 46eba0e1155bcc0a8f60841c9352e51a2b3e179b Mon Sep 17 00:00:00 2001 From: Shulhi Sapli <913103+shulhi@users.noreply.github.com> Date: Wed, 24 Dec 2025 21:10:41 +0800 Subject: [PATCH 6/6] Add tests --- .../data/printer/expr/expected/underscoreApply.res.txt | 6 ++++++ tests/syntax_tests/data/printer/expr/underscoreApply.res | 6 ++++++ 2 files changed, 12 insertions(+) diff --git a/tests/syntax_tests/data/printer/expr/expected/underscoreApply.res.txt b/tests/syntax_tests/data/printer/expr/expected/underscoreApply.res.txt index 488b19aa9e..43357ac6ed 100644 --- a/tests/syntax_tests/data/printer/expr/expected/underscoreApply.res.txt +++ b/tests/syntax_tests/data/printer/expr/expected/underscoreApply.res.txt @@ -71,3 +71,9 @@ let status = json ->optional(field("status", string, _), _) ->Option.mapOr(Status.Active, Status.fromString) + +a->map2(fn) + +a->map2(fn) + +a->f(_, _, b) diff --git a/tests/syntax_tests/data/printer/expr/underscoreApply.res b/tests/syntax_tests/data/printer/expr/underscoreApply.res index ac268388b3..728d05b5a3 100644 --- a/tests/syntax_tests/data/printer/expr/underscoreApply.res +++ b/tests/syntax_tests/data/printer/expr/underscoreApply.res @@ -76,3 +76,9 @@ let status = json ->optional(field("status", string, _), _) ->Option.mapOr(Status.Active, Status.fromString) + +a->map2(_, fn) + +a->map2(fn) + +a->f(_, _, b)