From b00af701f2b4ee362c1b336c89b82f8cadc2cb29 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/3] 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 e8d70af13eaa580f6a46070095f77defd70875ee 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/3] 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 d13942ed2b482dcd65d6a4c07e76760dc4732f7d 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/3] 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