From 24f04cb2376b80c6fab66538fdfdfcddf1b0532f Mon Sep 17 00:00:00 2001
From: Jules Aguillon <jules@j3s.fr>
Date: Mon, 24 Apr 2023 14:55:55 +0200
Subject: [PATCH 1/2] Consistently wrap (::)

The (::) was handled differently in several places: pattern/expressions,
with/without argument.
Take care of parenthesis in the `fmt_longident` function instead.
---
 lib/Fmt_ast.ml                | 33 ++++++---------------------------
 test/passing/tests/invalid.ml |  2 +-
 test/passing/tests/symbol.ml  |  4 ++++
 3 files changed, 11 insertions(+), 28 deletions(-)

diff --git a/lib/Fmt_ast.ml b/lib/Fmt_ast.ml
index 3ca07c52a9..705d3e5eec 100644
--- a/lib/Fmt_ast.ml
+++ b/lib/Fmt_ast.ml
@@ -234,13 +234,12 @@ let fmt_recmodule c ctx items fmt_item ast sub =
    enclosing box to break across multiple lines. *)
 
 let rec fmt_longident (li : Longident.t) =
+  let fmt_id id =
+    wrap_if (Std_longident.String_id.is_symbol id) "( " " )" (str id)
+  in
   match li with
-  | Lident id -> str id
-  | Ldot (li, id) ->
-      hvbox 0
-        ( fmt_longident li $ fmt "@,."
-        $ wrap_if (Std_longident.String_id.is_symbol id) "( " " )" (str id)
-        )
+  | Lident id -> fmt_id id
+  | Ldot (li, id) -> hvbox 0 (fmt_longident li $ fmt "@,." $ fmt_id id)
   | Lapply (li1, li2) ->
       hvbox 2 (fmt_longident li1 $ wrap "@,(" ")" (fmt_longident li2))
 
@@ -2068,9 +2067,6 @@ and fmt_expression c ?(box = true) ?pro ?epi ?eol ?parens ?(indent_wrap = 0)
            (Params.parens_if parens c.conf
               ( wrap_k opn cls (Cmts.fmt_within c ~pro ~epi pexp_loc)
               $ fmt_atrs ) )
-  | Pexp_construct (({txt= Lident "::"; loc= _} as lid), None) ->
-      Params.parens_if parens c.conf
-        (Params.parens c.conf (fmt_longident_loc c lid $ fmt_atrs))
   | Pexp_construct (lid, None) ->
       Params.parens_if parens c.conf (fmt_longident_loc c lid $ fmt_atrs)
   | Pexp_cons l ->
@@ -2081,20 +2077,6 @@ and fmt_expression c ?(box = true) ?pro ?epi ?eol ?parens ?(indent_wrap = 0)
                     (false, noop, noop, (fmt_if (i > 0) "::", sub_exp ~ctx e)) )
                ) )
         $ fmt_atrs )
-  | Pexp_construct (({txt= Lident "::"; loc= _} as lid), Some arg) ->
-      let opn, cls =
-        match c.conf.fmt_opts.indicate_multiline_delimiters.v with
-        | `No -> (str "(", str ")")
-        | `Space -> (str "( ", str " )")
-        | `Closing_on_separate_line ->
-            (str "( ", fits_breaks ")" ~hint:(1000, -2) ")")
-      in
-      Params.parens_if parens c.conf
-        ( hvbox 2
-            ( wrap_k opn cls (fmt_longident_loc c lid)
-            $ fmt "@ "
-            $ fmt_expression c (sub_exp ~ctx arg) )
-        $ fmt_atrs )
   | Pexp_construct (lid, Some arg) ->
       Params.parens_if parens c.conf
         ( hvbox 2
@@ -2154,12 +2136,9 @@ and fmt_expression c ?(box = true) ?pro ?epi ?eol ?parens ?(indent_wrap = 0)
          $ hvbox 0 (fmt_cases c ctx cs) )
   | Pexp_ident {txt; loc} ->
       let outer_parens = has_attr && parens in
-      let inner_parens = Exp.is_symbol exp || Exp.is_monadic_binding exp in
       Cmts.fmt c loc
       @@ wrap_if outer_parens "(" ")"
-      @@ ( wrap_if inner_parens "( " " )"
-             (fmt_longident txt $ Cmts.fmt_within c loc)
-         $ fmt_atrs )
+      @@ (fmt_longident txt $ Cmts.fmt_within c loc $ fmt_atrs)
   | Pexp_ifthenelse (if_branches, else_) ->
       let last_loc =
         match else_ with
diff --git a/test/passing/tests/invalid.ml b/test/passing/tests/invalid.ml
index 579a9278c6..d2c547c553 100644
--- a/test/passing/tests/invalid.ml
+++ b/test/passing/tests/invalid.ml
@@ -10,4 +10,4 @@ let f = (`A) a b
 
 let f = (`A x) a b
 
-let f = ((::)) a b c
+let f = (( :: )) a b c
diff --git a/test/passing/tests/symbol.ml b/test/passing/tests/symbol.ml
index d2708d532c..d8f8ba33b4 100644
--- a/test/passing/tests/symbol.ml
+++ b/test/passing/tests/symbol.ml
@@ -21,3 +21,7 @@ let _ = ( let++ ) [@attr] ;;
 ( let++ ) [@attr]
 
 let ( let++ ), (( and++ ) [@attr]) = X.((( let++ ) [@attr]), ( and++ ))
+
+let is_empty = function [] -> true | ( :: ) _ -> false
+
+let is_empty = (( :: ), ( :: ) 1, (Foo) 2)

From 7fca6e22e42b217df47e9e130cfbe80da71196c5 Mon Sep 17 00:00:00 2001
From: Jules Aguillon <jules@j3s.fr>
Date: Mon, 24 Apr 2023 15:08:13 +0200
Subject: [PATCH 2/2] Update Changes

---
 CHANGES.md | 1 +
 1 file changed, 1 insertion(+)

diff --git a/CHANGES.md b/CHANGES.md
index 8a66ce84de..2ab96fcd76 100644
--- a/CHANGES.md
+++ b/CHANGES.md
@@ -8,6 +8,7 @@
 
 ### Bug fixes
 
+- Fix invalid formatting of `(::)` (#2347, @Julow)
 - Fix formatting of string literals in code blocks (#2338, @Julow)
 - Improve formatting of module arguments (#2322, @Julow)
 - Consistent indentation of `@@ let+ x = ...` (#2315, @Julow)