Implement new module type syntax

[tatchi]

Note: This contains extra commits from #80, #81 and #82 that should be merged first

This PR implements the new module type syntax, so instead of

module type S_rec = [%import: (module Stuff.S_rec)]

we now write:

module type%import S_rec = Stuff.S_rec or [%%import: module type S_rec = Stuff.S_rec]

I didn't keep the old syntax, but it should be possible to have both coexist if desired.

As for the implementation, I first tried to declare an extender applied to structure_item with the following pattern psig (psig_modtype __ ^:: nil) ||| pstr (pstr_modtype __ ^:: nil) )

let module_declaration_extension =
  Ppxlib.Extension.V3.declare "import" Ppxlib.Extension.Context.structure_item
    Ppxlib.Ast_pattern.(
      psig (psig_modtype __ ^:: nil) ||| pstr (pstr_modtype __ ^:: nil) )
    module_declaration_expand

Unfortunately, this results in the following build error:

❯ dune build
File "src_test/ppx_deriving/.test_ppx_import.eobjs/_unknown_", line 1, characters 0-0:
Fatal error: exception Failure("Some ppx-es tried to register conflicting transformations: Extension 'import' on structure items declared at src/ppx_import.ml:616 matches extension 'import' declared at src/ppx_import.ml:599")

This is a bit more annoying, because we have to manually pattern-match to extract the cases we want to support, and return an error in all other cases.

let type_declaration_extension =
  Ppxlib.Extension.V3.declare "import" Ppxlib.Extension.Context.structure_item
    Ppxlib.Ast_pattern.(
      psig (psig_type __ __ ^:: nil) ||| pstr (pstr_type __ __ ^:: nil) )
    type_declaration_expand

I tried to combine both but psig (psig_modtype __ ^:: nil) ||| pstr (pstr_modtype __ ^:: nil) ) and psig (psig_type __ __ ^:: nil) ||| pstr (pstr_type __ __ ^:: nil) ) don't extract the same value. The only solution I found is to relax the pattern and extract the whole structure_item

let type_declaration_extension =
  Ppxlib.Extension.V3.declare "import" Ppxlib.Extension.Context.structure_item
    Ppxlib.Ast_pattern.(__)
    type_declaration_expander

That's a bit more annoying because we manually have to pattern match to extract the cases we want to support, and return an error in all the other cases.

I wonder if we could build a more "refined" pattern here. I've tried a few combinators to build patterns, but I haven't managed to build anything that works. I'm not sure I understood all of them though, so it could be that I'm missing something. Maybe @pitag-ha or @panglesd could help? Or confirm that there is no other way than using the whole structure_item like I did.

Fixes #74

[panglesd]

I wonder if we could build a more "refined" pattern here.

Yes, I think it is possible. The problem here is that you want, in one case, to extract something of some type t1, in some other case, to extract something of type t2 But this is not possible: what would be the type of the continuation?

The idea is to use a sum type to merge the two cases. Let me give an example (where the complex types are replaced with int and char to simplify):

type input = Case1 of int | Case2 of char

let extractor1 = Ast_pattern.(eint __ |> map1 ~f:(fun i -> Case1 i))
let extractor2 = Ast_pattern.(echar __ |> map1 ~f:(fun c -> Case2 c))
(* Ast_pattern.map1 maps the first argument of the continuation before applying it.
   So while [eint __] has type [(expression, int -> _, _) Ast_pattern.t],
         [extractor1] has type [(expression, input -> _, _) Ast_pattern.t] *)

let extractor = Ast_pattern.(extractor1 ||| extractor2)

let handle_case_1 (i:int) = ...
let handle_case_2 (c:char) = ...

let handle input =
  match input with
  | Case1 i -> handle_case_1 i
  | Case2 c -> handle_case_2 c

[tatchi]

@panglesd amazing, that's exactly what I was looking for. Thanks so much for you help! 🙏🤗

[tatchi]

Okay, so the only "downside" is that the errors are less verbose. See for example

Error: [%%import] Type pattern (PTyp) is not supported, only type and module
         type declarations are allowed

vs

Error: PStr expected

But I think that's okay. A bit more annoying is that some locations in the errors are different for OCaml versions 4.08-4.12, which is why the CI fails for these versions. We can duplicate the tests in a new folder errors_408_412 like we did for versions below 4.08, but we'll end up with 3 copies of the tests, which will be more annoying to maintain. The other solution might be to just drop the 3 failing tests. What do you think?

[panglesd]

the only "downside" is that the errors are less verbose

I totally agree, I think that is clearly something that we should improve! I'll open an issue on the ppxlib repo.

A bit more annoying is that some locations in the errors are different for OCaml versions

It's not only that: some errors are actually syntax errors in 4.08 -> 4.12... To keep those tests without maintaining 3 versions, you could test them only in the latest version of the compiler.