Fix unstable comment just before or-patterns (ocaml-ppx#1167)

bogdan2412 · Mar 28, 2020 · 3921f57 · 3921f57
1 parent e7a185c
commit 3921f57
Show file tree

Hide file tree

Showing 15 changed files with 305 additions and 3 deletions.
diff --git a/CHANGES.md b/CHANGES.md
@@ -7,7 +7,9 @@
 
 #### Bug fixes
 
-  + Fix unstable comment in variant declarations (#1108) (Jules Aguillon)
+  + Fix wrapping of or-patterns in presence of comments with `break-cases=fit` (#1167) (Jules Aguillon)
+    This also fixes an unstable comment bug in or-patterns
+  + Fix an unstable comment bug in variant declarations (#1108) (Jules Aguillon)
   + Fix: break multiline comments (#1122) (Guillaume Petiot)
   + Fix: types on named arguments were wrapped incorrectly when preceding comments (#1124) (Guillaume Petiot)
   + Fix the indentation produced by max-indent (#1118) (Guillaume Petiot)

diff --git a/lib/Fmt_ast.ml b/lib/Fmt_ast.ml
@@ -1031,8 +1031,11 @@ and fmt_pattern c ?pro ?parens ({ctx= ctx0; ast= pat} as xpat) =
                   let loc = xpat.ast.ppat_loc in
                   let force_break = Cmts.has_before c.cmts loc in
                   let leading_cmt =
-                    Cmts.fmt_before ~pro:(Fmt.break 1000 0) ~adj:noop c loc
-                      ~eol:noop
+                    let pro, adj =
+                      if first_grp && first then (noop, fmt "@ ")
+                      else (fmt "@ ", noop)
+                    in
+                    Cmts.fmt_before ~pro c loc ~adj ~eol:noop
                   in
                   let pro =
                     if first_grp && first then

diff --git a/test/passing/break_cases-align.ml.ref b/test/passing/break_cases-align.ml.ref
@@ -210,3 +210,27 @@ let handler =
           Errors.isset_in_strict p
       | _ -> ()
   end
+
+let _ =
+  match abc with
+  | Fooooooooooooooooo
+  (* comment *)
+  | Baaaaaaaaaaaaaaaar
+  (* comment *)
+  | Baaaaaaaaaaaaaaaaz (* comment *) ->
+      ()
+
+let _ =
+  match x with
+  | { y=
+        (* _____________________________________________________________________ *)
+        ( X _ | Y _ ) } ->
+      ()
+
+let _ =
+  match x with
+  | { y=
+        ( Z
+        (* _____________________________________________________________________ *)
+        | X _ | Y _ ) } ->
+      ()
diff --git a/test/passing/break_cases-all.ml.ref b/test/passing/break_cases-all.ml.ref
@@ -210,3 +210,27 @@ let handler =
           Errors.isset_in_strict p
       | _ -> ()
   end
+
+let _ =
+  match abc with
+  | Fooooooooooooooooo
+  (* comment *)
+  | Baaaaaaaaaaaaaaaar
+  (* comment *)
+  | Baaaaaaaaaaaaaaaaz (* comment *) ->
+      ()
+
+let _ =
+  match x with
+  | { y=
+        (* _____________________________________________________________________ *)
+        ( X _ | Y _ ) } ->
+      ()
+
+let _ =
+  match x with
+  | { y=
+        ( Z
+        (* _____________________________________________________________________ *)
+        | X _ | Y _ ) } ->
+      ()
diff --git a/test/passing/break_cases-closing_on_separate_line.ml.ref b/test/passing/break_cases-closing_on_separate_line.ml.ref
@@ -217,3 +217,27 @@ let handler =
           Errors.isset_in_strict p
       | _ -> ()
   end
+
+let _ =
+  match abc with
+  | Fooooooooooooooooo
+  (* comment *)
+  | Baaaaaaaaaaaaaaaar
+  (* comment *)
+  | Baaaaaaaaaaaaaaaaz (* comment *) ->
+      ()
+
+let _ =
+  match x with
+  | { y=
+        (* _____________________________________________________________________ *)
+        ( X _ | Y _ ) } ->
+      ()
+
+let _ =
+  match x with
+  | { y=
+        ( Z
+        (* _____________________________________________________________________ *)
+        | X _ | Y _ ) } ->
+      ()
diff --git a/test/passing/break_cases-closing_on_separate_line_leading_nested_match_parens.ml.ref b/test/passing/break_cases-closing_on_separate_line_leading_nested_match_parens.ml.ref
@@ -216,3 +216,27 @@ let handler =
           Errors.isset_in_strict p
       | _ -> ()
   end
+
+let _ =
+  match abc with
+  | Fooooooooooooooooo
+  (* comment *)
+  | Baaaaaaaaaaaaaaaar
+  (* comment *)
+  | Baaaaaaaaaaaaaaaaz (* comment *) ->
+      ()
+
+let _ =
+  match x with
+  | { y=
+        (* _____________________________________________________________________ *)
+        ( X _ | Y _ ) } ->
+      ()
+
+let _ =
+  match x with
+  | { y=
+        ( Z
+        (* _____________________________________________________________________ *)
+        | X _ | Y _ ) } ->
+      ()
diff --git a/test/passing/break_cases-cosl_lnmp_cmei.ml.ref b/test/passing/break_cases-cosl_lnmp_cmei.ml.ref
@@ -216,3 +216,27 @@ let handler =
           Errors.isset_in_strict p
       | _ -> ()
   end
+
+let _ =
+  match abc with
+  | Fooooooooooooooooo
+  (* comment *)
+  | Baaaaaaaaaaaaaaaar
+  (* comment *)
+  | Baaaaaaaaaaaaaaaaz (* comment *) ->
+      ()
+
+let _ =
+  match x with
+  | { y=
+        (* _____________________________________________________________________ *)
+        ( X _ | Y _ ) } ->
+      ()
+
+let _ =
+  match x with
+  | { y=
+        ( Z
+        (* _____________________________________________________________________ *)
+        | X _ | Y _ ) } ->
+      ()
diff --git a/test/passing/break_cases-fit_or_vertical.ml.ref b/test/passing/break_cases-fit_or_vertical.ml.ref
@@ -175,3 +175,26 @@ let handler =
       | Call OtherThing -> Errors.isset_in_strict p
       | _ -> ()
   end
+
+let _ =
+  match abc with
+  | Fooooooooooooooooo
+  (* comment *)
+  | Baaaaaaaaaaaaaaaar
+  (* comment *)
+  | Baaaaaaaaaaaaaaaaz
+  (* comment *) -> ()
+
+let _ =
+  match x with
+  | { y=
+        (* _____________________________________________________________________ *)
+        ( X _ | Y _ ) } -> ()
+
+let _ =
+  match x with
+  | { y=
+        ( Z
+        (* _____________________________________________________________________ *)
+        | X _
+        | Y _ ) } -> ()
diff --git a/test/passing/break_cases-nested.ml.ref b/test/passing/break_cases-nested.ml.ref
@@ -180,3 +180,28 @@ let handler =
       | _ ->
           ()
   end
+
+let _ =
+  match abc with
+  | Fooooooooooooooooo (* comment *)
+  | Baaaaaaaaaaaaaaaar
+  (* comment *)
+  | Baaaaaaaaaaaaaaaaz (* comment *) ->
+      ()
+
+let _ =
+  match x with
+  | { y=
+        (* _____________________________________________________________________ *)
+        ( X _
+        | Y _ ) } ->
+      ()
+
+let _ =
+  match x with
+  | { y=
+        ( Z
+        (* _____________________________________________________________________ *)
+        | X _
+        | Y _ ) } ->
+      ()
diff --git a/test/passing/break_cases-normal_indent.ml.ref b/test/passing/break_cases-normal_indent.ml.ref
@@ -210,3 +210,27 @@ let handler =
           Errors.isset_in_strict p
       | _ -> ()
   end
+
+let _ =
+  match abc with
+  | Fooooooooooooooooo
+  (* comment *)
+  | Baaaaaaaaaaaaaaaar
+  (* comment *)
+  | Baaaaaaaaaaaaaaaaz (* comment *) ->
+      ()
+
+let _ =
+  match x with
+  | { y=
+        (* _____________________________________________________________________ *)
+        ( X _ | Y _ ) } ->
+      ()
+
+let _ =
+  match x with
+  | { y=
+        ( Z
+        (* _____________________________________________________________________ *)
+        | X _ | Y _ ) } ->
+      ()
diff --git a/test/passing/break_cases-toplevel.ml.ref b/test/passing/break_cases-toplevel.ml.ref
@@ -178,3 +178,25 @@ let handler =
           Errors.isset_in_strict p
       | _ -> ()
   end
+
+let _ =
+  match abc with
+  | Fooooooooooooooooo (* comment *) | Baaaaaaaaaaaaaaaar
+  (* comment *)
+  | Baaaaaaaaaaaaaaaaz (* comment *) ->
+      ()
+
+let _ =
+  match x with
+  | { y=
+        (* _____________________________________________________________________ *)
+        ( X _ | Y _ ) } ->
+      ()
+
+let _ =
+  match x with
+  | { y=
+        ( Z
+        (* _____________________________________________________________________ *)
+        | X _ | Y _ ) } ->
+      ()
diff --git a/test/passing/break_cases.ml b/test/passing/break_cases.ml
@@ -188,3 +188,29 @@ let handler =
           Errors.isset_in_strict p
       | _ -> ()
   end
+
+let _ =
+  match abc with
+  | Fooooooooooooooooo (* comment *)
+  | Baaaaaaaaaaaaaaaar
+  (* comment *)
+  | Baaaaaaaaaaaaaaaaz
+  (* comment *)
+    -> ()
+
+let _ =
+  match x with
+  | { y =
+        (* _____________________________________________________________________ *)
+        ( X _ | Y _ )
+    } -> ()
+;;
+
+let _ =
+  match x with
+  | { y =
+        Z |
+        (* _____________________________________________________________________ *)
+        ( X _ | Y _ )
+    } -> ()
+;;
diff --git a/test/passing/break_cases.ml.ref b/test/passing/break_cases.ml.ref
@@ -155,3 +155,25 @@ let handler =
           Errors.isset_in_strict p
       | _ -> ()
   end
+
+let _ =
+  match abc with
+  | Fooooooooooooooooo (* comment *) | Baaaaaaaaaaaaaaaar
+  (* comment *)
+  | Baaaaaaaaaaaaaaaaz (* comment *) ->
+      ()
+
+let _ =
+  match x with
+  | { y=
+        (* _____________________________________________________________________ *)
+        ( X _ | Y _ ) } ->
+      ()
+
+let _ =
+  match x with
+  | { y=
+        ( Z
+        (* _____________________________________________________________________ *)
+        | X _ | Y _ ) } ->
+      ()
diff --git a/test/passing/js_source.ml b/test/passing/js_source.ml
@@ -7482,3 +7482,20 @@ let f =
        comment *)
     ~y
 ;;
+
+let _ =
+  match x with
+  | { y =
+        (* _____________________________________________________________________ *)
+        ( X _ | Y _ )
+    } -> ()
+;;
+
+let _ =
+  match x with
+  | { y =
+        Z |
+        (* _____________________________________________________________________ *)
+        ( X _ | Y _ )
+    } -> ()
+;;
diff --git a/test/passing/js_source.ml.ref b/test/passing/js_source.ml.ref
@@ -9856,3 +9856,21 @@ let f =
          comment *)
     ~y
 ;;
+
+let _ =
+  match x with
+  | { y =
+        (* _____________________________________________________________________ *)
+        ( X _ | Y _ )
+    } -> ()
+;;
+
+let _ =
+  match x with
+  | { y =
+        ( Z
+        (* _____________________________________________________________________ *)
+        | X _
+        | Y _ )
+    } -> ()
+;;