Value constraint pattern parens (#2913)

* type-constrained alias patterns must be parenthesized

* add more cases on value-constraint-alias

* Fix assertion failure when expression and binding both have type constraints

* Fix rtopIntegration test for Windows (#2914)

* test(rtop): capture stderr in integration test for Windows

* ci: setup tmate

* test(rtop): avoid running rtop in windows 4.14.*

* ci: enable 5.4.0 in windows

* ci: remove action-tmate

* ci: revert all changes, keep building

* ci: add -preasonrtop in esy-ci

* ci: update cache keys

* ci: fix Windows CI failures

- Disable rtopIntegration test on Windows due to lambda-term segfault
  when stdout is not a real console handle (ConPTY issue on GH Actions).
  See: ocaml-community/lambda-term#125

- Add setup-mingw step in esy-ci for Windows to provide x86_64-w64-mingw32-gcc
  needed to compile lambda-term C stubs.

- Bump esy cache keys to v0.0.2 to avoid stale caches.

* ci: replace setup-mingw action with direct choco install

The egor-tensin/setup-mingw@v2 action fails on MinGW 15.2.0 because
its cleanup script tries to remove libpthread.dll.a which no longer
exists in UCRT-based builds. Use choco directly and add the bin
directory to PATH manually.

* ci: esy-ci doesn't run rtop tests on windows

* test: remove comment for rtopIntegration

Author: davesnx

.github/workflows/esy-ci.yml

@@ -36,6 +36,13 @@ jobs:
         with:
           node-version: 20
 
+      - name: Set up MinGW (Windows)
+        if: runner.os == 'Windows'
+        shell: pwsh
+        run: |
+          choco upgrade mingw -y --no-progress
+          echo "C:\ProgramData\mingw64\mingw64\bin" | Out-File -FilePath $env:GITHUB_PATH -Encoding utf8 -Append
+
       - name: Install esy
         run: npm install -g esy@0.9.0-beta.1
 
@@ -44,7 +51,7 @@ jobs:
         uses: actions/cache/restore@v4
         with:
           path: ~/.esy/source
-          key: esy-source-${{ matrix.os }}-${{ matrix.ocaml-compiler }}-${{ hashFiles('esy.lock.json') }}
+          key: v0.0.2-esy-source-${{ matrix.os }}-${{ matrix.ocaml-compiler }}-${{ hashFiles('esy.lock.json') }}
 
       - name: Print esy cache
         id: print_esy_cache
@@ -57,8 +64,8 @@ jobs:
           path: |
             ${{ steps.print_esy_cache.outputs.ESY_CACHE }}
             _export
-          key: esy-build-${{ matrix.os }}-${{ matrix.ocaml-compiler }}-${{ hashFiles('esy.lock.json') }}
-          restore-keys: esy-build-${{ matrix.os }}-
+          key: v0.0.2-esy-build-${{ matrix.os }}-${{ matrix.ocaml-compiler }}-${{ hashFiles('esy.lock.json') }}
+          restore-keys: v0.0.2-esy-build-${{ matrix.os }}-
 
       - name: Install dependencies
         run: esy install
@@ -94,7 +101,7 @@ jobs:
         if: steps.global-cache.outputs.cache-hit != 'true'
         with:
           path: ~/.esy/source
-          key: esy-source-${{ matrix.os }}-${{ matrix.ocaml-compiler }}-${{ hashFiles('esy.lock.json') }}
+          key: v0.0.2-esy-source-${{ matrix.os }}-${{ matrix.ocaml-compiler }}-${{ hashFiles('esy.lock.json') }}
 
       - name: Save dependencies cache
         if: steps.deps-cache.outputs.cache-hit != 'true'
@@ -103,7 +110,7 @@ jobs:
           path: |
             ${{ steps.print_esy_cache.outputs.ESY_CACHE }}
             _export
-          key: esy-build-${{ matrix.os }}-${{ matrix.ocaml-compiler }}-${{ hashFiles('esy.lock.json') }}
+          key: v0.0.2-esy-build-${{ matrix.os }}-${{ matrix.ocaml-compiler }}-${{ hashFiles('esy.lock.json') }}
 
       # Cleanup build cache in case dependencies have changed
       - name: Cleanup

.github/workflows/opam-ci.yml

@@ -32,6 +32,7 @@ jobs:
             # OCaml >= 4.13
             # https://github.com/ocaml/setup-ocaml/issues/822#issuecomment-2215525942
           - {ocaml-compiler: '4.14.x', os: windows-latest}
+          - {ocaml-compiler: 'ocaml-base-compiler.5.4.0', os: windows-latest}
 
     runs-on: ${{ matrix.setup.os }}
 
@@ -51,15 +52,15 @@ jobs:
         uses: actions/cache/restore@v4
         with:
           path: ~/.opam
-          key: opam-${{ matrix.setup.os }}-${{ matrix.setup.ocaml-compiler }}-${{ hashFiles('*.opam') }}
+          key: v0.0.1-opam-${{ matrix.setup.os }}-${{ matrix.setup.ocaml-compiler }}-${{ hashFiles('*.opam') }}
 
       - name: Load opam cache when Windows
         if: runner.os == 'Windows'
         id: opam-cache-windows
         uses: actions/cache/restore@v4
         with:
           path: _opam
-          key: opam-${{ matrix.setup.os }}-${{ matrix.setup.ocaml-compiler }}-${{ hashFiles('**.opam') }}
+          key: v0.0.1-opam-${{ matrix.setup.os }}-${{ matrix.setup.ocaml-compiler }}-${{ hashFiles('**.opam') }}
 
       - name: Install dependencies
         run: opam install . --deps-only
@@ -87,12 +88,12 @@ jobs:
         if: steps.opam-cache.outputs.cache-hit != 'true' && runner.os != 'Windows'
         with:
           path: ~/.opam
-          key: opam-${{ matrix.setup.os }}-${{ matrix.setup.ocaml-compiler }}-${{ hashFiles('**.opam') }}
+          key: v0.0.1-opam-${{ matrix.setup.os }}-${{ matrix.setup.ocaml-compiler }}-${{ hashFiles('**.opam') }}
 
       - name: Save cache when Windows
         uses: actions/cache/save@v4
         if: steps.opam-cache-windows.outputs.cache-hit != 'true' && runner.os == 'Windows'
         with:
           path: _opam
-          key: opam-${{ matrix.setup.os }}-${{ matrix.setup.ocaml-compiler }}-${{ hashFiles('**.opam') }}
+          key: v0.0.1-opam-${{ matrix.setup.os }}-${{ matrix.setup.ocaml-compiler }}-${{ hashFiles('**.opam') }}
 

src/reason-parser/reason_pprint_ast.ml

@@ -5913,8 +5913,7 @@ let createFormatter () =
                 (pattern :: patternAux)
           in
           match argsList, return.pexp_desc with
-          | [], Pexp_constraint (e, ct) ->
-            assert (vbct = None);
+          | [], Pexp_constraint (e, ct) when vbct = None ->
             let typeLayout =
               source_map
                 ~loc:ct.ptyp_loc
@@ -6244,7 +6243,18 @@ let createFormatter () =
                   ~loc:pat.ppat_loc
                   (match vbct with
                   | Some _ ->
-                    self#pattern_with_precedence ~attrs:pat.ppat_attributes pat
+                    (match pat.ppat_desc with
+                    | Ppat_alias _ ->
+                      makeList
+                        ~wrap:("(", ")")
+                        [ self#pattern_with_precedence
+                            ~attrs:pat.ppat_attributes
+                            pat
+                        ]
+                    | _ ->
+                      self#pattern_with_precedence
+                        ~attrs:pat.ppat_attributes
+                        pat)
                   | None -> self#pattern pat)
               in
               let appTerms = self#unparseExprApplicationItems expr in

test/dune

@@ -15,4 +15,5 @@
 (cram
  (applies_to rtopIntegration)
  (package rtop)
+ (enabled_if (= %{os_type} Unix))
  (deps %{bin:ocamlc} %{bin:rtop}))

test/expr-constraint-with-vbct.t/input.re

@@ -0,0 +1,15 @@
+/* Array literal with type constraint */
+let x: array(int) = ([|1, 2|]: array(int));
+
+/* List literal with type constraint */
+let x: list(int) = ([1, 2, 3]: list(int));
+
+/* Tuple with type constraint */
+let x: (int, string) = ((1, "a"): (int, string));
+
+/* Function expression with type constraint */
+let f: int => int = ((x => x + 1): int => int);
+
+/* Record with type constraint */
+type t = {a: int};
+let x: t = ({a: 1}: t);

test/expr-constraint-with-vbct.t/run.t

@@ -0,0 +1,41 @@
+Format expression type constraints when binding also has a type constraint
+  $ refmt ./input.re | tee formatted.re
+  /* Array literal with type constraint */
+  let x: array(int) = ([|1, 2|]: array(int));
+  
+  /* List literal with type constraint */
+  let x: list(int) = ([1, 2, 3]: list(int));
+  
+  /* Tuple with type constraint */
+  let x: (int, string) = (
+    (1, "a"): (int, string)
+  );
+  
+  /* Function expression with type constraint */
+  let f: int => int = (x => x + 1: int => int);
+  
+  /* Record with type constraint */
+  type t = {a: int};
+  let x: t = ({ a: 1 }: t);
+
+Idempotency check
+  $ refmt ./formatted.re | tee formatted_back.re
+  /* Array literal with type constraint */
+  let x: array(int) = ([|1, 2|]: array(int));
+  
+  /* List literal with type constraint */
+  let x: list(int) = ([1, 2, 3]: list(int));
+  
+  /* Tuple with type constraint */
+  let x: (int, string) = (
+    (1, "a"): (int, string)
+  );
+  
+  /* Function expression with type constraint */
+  let f: int => int = (x => x + 1: int => int);
+  
+  /* Record with type constraint */
+  type t = {a: int};
+  let x: t = ({ a: 1 }: t);
+
+  $ diff formatted.re formatted_back.re

test/rtopIntegration.t

@@ -11,8 +11,8 @@ always error.
 Given the above, we're gonna test that utop integration works by piping code
 into it and asserting the existence of some output.
 
-  $ echo "let f = a => a;" | rtop | grep -o "let f: 'a => 'a = <fun>;"
+  $ echo "let f = a => a;" | rtop 2>&1 | grep -o "let f: 'a => 'a = <fun>;"
   let f: 'a => 'a = <fun>;
 
-  $ echo "let f = (a) => 1 + \"hi\";" | rtop | grep -o "has type"
+  $ echo "let f = (a) => 1 + \"hi\";" | rtop 2>&1 | grep -o "has type"
   has type

test/value-constraint-alias-pattern.t/input.re

@@ -0,0 +1,14 @@
+let x: t = x;
+let (x as y): t = x;
+let ((x, y) as pair): t = x;
+let (Some(x) as opt): t = opt;
+let ({url, mode} as target): t = x;
+let ({url, mode, protocol} as target): TargetT.Safe.t =
+  multiTarget->MultiTargetT.toIgnorableTargetT;
+let ({url, mode}): t = x;
+let ({url: u, mode: m} as target): t = x;
+let Foo.{url, mode}: t = x;
+let (Foo.{url, mode} as target): t = x;
+let ([x, y] as listPair): t = value;
+let (_ as anyValue): t = value;
+let ((x as y: u): t) = value;

test/value-constraint-alias-pattern.t/run.t

@@ -0,0 +1,33 @@
+  $ refmt ./input.re | tee formatted.re
+  let x: t = x;
+  let (x as y): t = x;
+  let ((x, y) as pair): t = x;
+  let (Some(x) as opt): t = opt;
+  let ({ url, mode } as target): t = x;
+  let ({ url, mode, protocol } as target): TargetT.Safe.t =
+    multiTarget->MultiTargetT.toIgnorableTargetT;
+  let { url, mode }: t = x;
+  let ({ url: u, mode: m } as target): t = x;
+  let Foo.{ url, mode } : t = x;
+  let (Foo.{ url, mode } as target): t = x;
+  let ([x, y] as listPair): t = value;
+  let (_ as anyValue): t = value;
+  let ((x as y: u): t) = value;
+
+  $ refmt ./formatted.re | tee formatted_back.re
+  let x: t = x;
+  let (x as y): t = x;
+  let ((x, y) as pair): t = x;
+  let (Some(x) as opt): t = opt;
+  let ({ url, mode } as target): t = x;
+  let ({ url, mode, protocol } as target): TargetT.Safe.t =
+    multiTarget->MultiTargetT.toIgnorableTargetT;
+  let { url, mode }: t = x;
+  let ({ url: u, mode: m } as target): t = x;
+  let Foo.{ url, mode } : t = x;
+  let (Foo.{ url, mode } as target): t = x;
+  let ([x, y] as listPair): t = value;
+  let (_ as anyValue): t = value;
+  let ((x as y: u): t) = value;
+
+  $ diff formatted.re formatted_back.re