Bug fix: Pexp_constraint with attributes should get parenthesized (#…

…2513)

Fix a bug in formatting f ((1 : int) [@A]) where the outer pair of parentheses gets stripped.

* Tweak Ast rules to avoid parentheses in records

Avoid the extra parentheses in this case:

    {((a : b)) with c}

* Add missing Ast rule for override fields

Avoid parentheses in this case:

    {<x = (x [@A])>}

CHANGES.md

@@ -34,6 +34,7 @@ profile. This started with version 0.26.0.
 - \* Fix unwanted alignment after comment (#2507, @Julow)
 - \* Fix unwanted alignment in if-then-else (#2511, @Julow)
 - Fix position of comments around and within `(type ...)` function arguments (#2503, @gpetiot)
+- Fix missing parentheses around constraint expressions with attributes (#2513, @alanechang)
 
 ## 0.26.1 (2023-09-15)
 

lib/Ast.ml

@@ -2337,11 +2337,14 @@ end = struct
           , Some
               ( { pexp_desc=
                     ( Pexp_ident _ | Pexp_constant _ | Pexp_record _
-                    | Pexp_field _ )
+                    | Pexp_constraint _ | Pexp_field _ )
                 ; _ } as e0 ) )
         when e0 == exp ->
           false
       | Pexp_record (_, Some e0) when e0 == exp -> true
+      | Pexp_override fields
+        when List.exists fields ~f:(fun (_, e0) -> e0 == exp) ->
+          exposed_right_exp Sequence exp
       | Pexp_sequence (lhs, rhs) -> exp_in_sequence lhs rhs exp
       | Pexp_apply (_, args)
         when List.exists args ~f:(fun (_, e0) ->

lib/Fmt_ast.ml

@@ -2159,11 +2159,12 @@ and fmt_expression c ?(box = true) ?(pro = noop) ?eol ?parens
       pro
       $ hvbox
           (Params.Indent.exp_constraint c.conf)
-          ( wrap_fits_breaks ~space:false c.conf "(" ")"
-              ( fmt_expression c (sub_exp ~ctx e)
-              $ fmt "@ : "
-              $ fmt_core_type c (sub_typ ~ctx t) )
-          $ fmt_atrs )
+          (Params.parens_if parens c.conf
+             ( wrap_fits_breaks ~space:false c.conf "(" ")"
+                 ( fmt_expression c (sub_exp ~ctx e)
+                 $ fmt "@ : "
+                 $ fmt_core_type c (sub_typ ~ctx t) )
+             $ fmt_atrs ) )
   | Pexp_construct ({txt= Lident (("()" | "[]") as txt); loc}, None) ->
       let opn = char txt.[0] and cls = char txt.[1] in
       pro

test/passing/tests/attributes.ml

@@ -429,3 +429,9 @@ let raise_length_mismatch name n1 n2 =
 external unsafe_memset : t -> pos:int -> len:int -> char -> unit
   = "bigstring_memset_stub"
 [@@noalloc]
+
+let _ = f ((1 : int) [@a])
+
+let _ = f ((1 : int) [@a]) ((1 : int) [@a])
+
+let _ = f ((((1 : int) [@a]) : (int[@b])) [@a]) ((1 : int) [@a])

test/passing/tests/override.ml.ref

@@ -4,6 +4,6 @@ let _ = {<x = ((x [@a]) : (t[@b])) [@c]>}
 
 let _ = {<x>}
 
-let _ = {<x = (x [@a])>}
+let _ = {<x = x [@a]>}
 
 let _ = {<x>}