Better migration errors

ast/ast.ml

@@ -55,6 +55,8 @@ open Import
 (* Note that when bumping ppxlib's internal AST you will also need to update a
    few other files:
    - [astlib/pprintast.ml], details on how to update at the head of the file.
+   - [astlib/error.ml], change the error message to taken into account the new internal AST
+     version number.
 *)
 
 (* Source code locations (ranges of positions), used in parsetree. *)

astlib/encoding_502.ml

@@ -2,8 +2,7 @@ module Ext_name = struct
   let ptyp_open = "ppxlib.migration.ptyp_open_502"
 end
 
-let invalid_encoding ~loc name =
-  Location.raise_errorf ~loc "Invalid %s encoding" name
+let invalid_encoding ~loc name = Error.invalid_encoding ~loc ~version:"5.2" name
 
 module To_501 = struct
   open Ast_501.Asttypes

astlib/encoding_503.ml

@@ -2,8 +2,7 @@ module Ext_name = struct
   let ppat_effect = "ppxlib.migration.ppat_effect_503"
 end
 
-let invalid_encoding ~loc name =
-  Location.raise_errorf ~loc "Invalid %s encoding" name
+let invalid_encoding ~loc name = Error.invalid_encoding ~loc ~version:"5.3" name
 
 module To_502 = struct
   let encode_ppat_effect ~loc ~effect_ ~k =

astlib/encoding_504.ml

@@ -10,8 +10,7 @@ module Ext_name = struct
   let bivariant_pmty_with = "ppxlib.migration.bivariant_pmty_with_5_4"
 end
 
-let invalid_encoding ~loc name =
-  Location.raise_errorf ~loc "Invalid %s encoding" name
+let invalid_encoding ~loc name = Error.invalid_encoding ~loc ~version:"5.4" name
 
 module type AST = sig
   type payload

astlib/encoding_505.ml

@@ -10,8 +10,7 @@ module Ext_name = struct
   let external_pmty_with = "ppxlib.migration.external_pmty_with_5_5"
 end
 
-let invalid_encoding ~loc name =
-  Location.raise_errorf ~loc "Invalid %s encoding" name
+let invalid_encoding ~loc name = Error.invalid_encoding ~loc ~version:"5.5" name
 
 module To_504 = struct
   open Ast_504.Asttypes

astlib/error.ml

@@ -0,0 +1,24 @@
+let raise_error_txt ~loc = Location.raise_errorf ~loc "%a" Format.pp_print_text
+
+let migration_error ~loc ~from ~to_ msg =
+  let s =
+    Format.sprintf
+      "ppxlib migration error (migrating ocaml.%s to ocaml.%s): %s\n\n\
+       Ppxlib was likely trying to migrate from %s to 5.2.0 and back again, \
+       but encountered an AST node that could not be encoded. Either remove \
+       the feature or open an issue at \
+       https://github.com/ocaml-ppx/ppxlib/issues."
+      from to_ msg Sys.ocaml_version
+  in
+  raise_error_txt ~loc s
+
+let invalid_encoding ~loc ~version msg =
+  let s =
+    Format.sprintf
+      "ppxlib invalid encoding: %s\n\n\
+       Ppxlib failed to decode a feature from the OCaml %s AST. If this does \
+       not seem right, please do open an issue at \
+       https://github.com/ocaml-ppx/ppxlib/issues."
+      msg version
+  in
+  raise_error_txt ~loc s

astlib/migrate_410_409.ml

@@ -2,8 +2,7 @@ module From = Ast_410
 module To = Ast_409
 
 let migration_error loc missing_feature =
-  Location.raise_errorf ~loc
-    "migration error: %s is not supported before OCaml 4.10" missing_feature
+  Error.migration_error ~loc ~from:"4.10" ~to_:"4.09" missing_feature
 
 let map_option f x = match x with None -> None | Some x -> Some (f x)
 

astlib/migrate_413_412.ml

@@ -3,8 +3,7 @@ module From = Ast_413
 module To = Ast_412
 
 let migration_error loc missing_feature =
-  Location.raise_errorf ~loc
-    "migration error: %s is not supported before OCaml 4.13" missing_feature
+  Error.migration_error ~loc ~from:"4.13" ~to_:"4.12" missing_feature
 
 let rec copy_toplevel_phrase :
     Ast_413.Parsetree.toplevel_phrase -> Ast_412.Parsetree.toplevel_phrase =

astlib/migrate_414_413.ml

@@ -3,8 +3,7 @@ module From = Ast_414
 module To = Ast_413
 
 let migration_error loc missing_feature =
-  Location.raise_errorf ~loc
-    "migration error: %s is not supported before OCaml 4.13" missing_feature
+  Error.migration_error ~loc ~from:"4.14" ~to_:"4.13" missing_feature
 
 let rec copy_toplevel_phrase :
     Ast_414.Parsetree.toplevel_phrase -> Ast_413.Parsetree.toplevel_phrase =

astlib/migrate_504_503.ml

@@ -10,9 +10,7 @@ module Bivariant_param = struct
 end
 
 let bivariant_error ~loc =
-  Location.raise_errorf ~loc
-    "Ppxlib migration error: bivariant type parameters cannot be migrated from \
-     OCaml 5.4 to 5.3"
+  Error.migration_error ~loc ~from:"5.4" ~to_:"5.3" "bivariant type parameters"
 
 let rec copy_toplevel_phrase :
     Ast_504.Parsetree.toplevel_phrase -> Ast_503.Parsetree.toplevel_phrase =

astlib/migrate_505_504.ml

@@ -33,9 +33,7 @@ module External_type = struct
     { v with value = List.rev v.value }
 
   let err ~loc =
-    Location.raise_errorf ~loc
-      "Ppxlib migration error: external type cannot be migrated from OCaml 5.5 \
-       to 5.4"
+    Error.migration_error ~loc ~from:"5.5" ~to_:"5.4" "external types"
 end
 
 let copy_location x = x

test/driver/compiler-pp/run.t

@@ -31,7 +31,13 @@ Now if we run it with `--use-compiler-pp`, we should get the migration error:
   File "test.ml", line 1, characters 0-22:
   1 | [%%named_existentials]
       ^^^^^^^^^^^^^^^^^^^^^^
-  Error: migration error: existentials in pattern-matching is not supported before OCaml 4.13
+  Error: ppxlib migration error (migrating ocaml.4.13 to ocaml.4.12):
+         existentials in pattern-matching
+
+         Ppxlib was likely trying to migrate from 4.11.2 to 5.2.0 and back
+         again, but encountered an AST node that could not be encoded. Either
+         remove the feature or open an issue at
+         https://github.com/ocaml-ppx/ppxlib/issues.
   [1]
 
 This should also work for correction based code gen:
@@ -61,5 +67,11 @@ and with the flag:
   File "test_inline.ml", lines 1-2, characters 0-38:
   1 | type t = int
   2 | [@@deriving_inline named_existentials]
-  Error: migration error: existentials in pattern-matching is not supported before OCaml 4.13
+  Error: ppxlib migration error (migrating ocaml.4.13 to ocaml.4.12):
+         existentials in pattern-matching
+
+         Ppxlib was likely trying to migrate from 4.11.2 to 5.2.0 and back
+         again, but encountered an AST node that could not be encoded. Either
+         remove the feature or open an issue at
+         https://github.com/ocaml-ppx/ppxlib/issues.
   [1]

test/encoding/503/migrations/run.t

@@ -15,3 +15,22 @@ And that it is correctly decoded when migrated back up to 5.3+ ASTs:
 
   $ ./id_driver.exe effect_pattern.ml --use-compiler-pp
   let () = try comp () with | effect Xchg n,  k -> continue k (n + 1)
+
+A test for the invalid encoding logic:
+
+  $ cat > effect_pattern_encoded.ml << EOF
+  > let () =
+  >  try comp ()
+  >  with
+  >  | [%ppxlib.migration.ppat_effect_503 ? "Badly encoded ppat_effect!"] -> continue k (n + 1)
+  > EOF
+  $ ./id_driver.exe effect_pattern_encoded.ml --use-compiler-pp 
+  File "effect_pattern_encoded.ml", line 4, characters 5-37:
+  4 |  | [%ppxlib.migration.ppat_effect_503 ? "Badly encoded ppat_effect!"] -> continue k (n + 1)
+           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+  Error: ppxlib invalid encoding: ppxlib.migration.ppat_effect_503
+
+         Ppxlib failed to decode a feature from the OCaml 5.3 AST. If this does
+         not seem right, please do open an issue at
+         https://github.com/ocaml-ppx/ppxlib/issues.
+  [1]

test/encoding/504/migrations/run.t

@@ -43,6 +43,24 @@ And same for patterns:
   $ ./id_driver.exe pattern.ml --use-compiler-pp
   let (~a, ~b:_, c, ..) = x
 
+We also check our invalid decoding logic:
+  $ cat > invalid_lbl_tuple_encoding.ml << EOF
+  > let x =
+  >  [%ppxlib.migration.pexp_labeled_tuple_5_4
+  >    (((Some \`a), 0), ((Some \`b), 1), (None, "abc"))]
+  > EOF
+
+  $ ./id_driver.exe invalid_lbl_tuple_encoding.ml --use-compiler-pp
+  File "invalid_lbl_tuple_encoding.ml", line 2, characters 3-42:
+  2 |  [%ppxlib.migration.pexp_labeled_tuple_5_4
+         ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+  Error: ppxlib invalid encoding: ppxlib.migration.pexp_labeled_tuple_5_4
+
+         Ppxlib failed to decode a feature from the OCaml 5.4 AST. If this does
+         not seem right, please do open an issue at
+         https://github.com/ocaml-ppx/ppxlib/issues.
+  [1]
+
 We also check that bivariant type parameters are correctly encoded and migrated:
 
   $ cat > bivariant.ml << EOF