Ast_pattern: add patterns drop and as__ (#310)

[Kakadu]

No description provided.

[pitag-ha]

Thanks for the PR @Kakadu! Adding as__ and exposing drop seems like a good addition to Ast_pattern to me.

About the tests: They're a bit tedious to read since the test code and test result are in two different files. What would you think about making [%ppx_expect]-tests out of them? You can see an example of that e.g. here. What do you think?

[Kakadu]

What would you think about making [%ppx_expect]-tests out of them? You can see an example of that e.g. here. What do you think?

I copied your example, and your expect-like executable writes empty file when something is wrong with the test. For example, I changed the output, got empty '.corrected' file, changed back, and got again empty '.corrected' file. So, it is almost impossible to understand what is wrong.

Using a clever guess I realized that it should be somehow connected to autoformatting. Indeed, when I added [@@@ocamlformat "disable"] to test/extensions_and_deriving/test.ml I managed to brake test and revert it back. But I can't do the same trick with my test, because it is already formatted wrong (in a style that your tool test/extensions_and_deriving/.bin/expect-test doesn't understand).

What do you think?

Could we somehow agree on that PR without me touching an abomination called expect-test?

[pitag-ha]

For example, I changed the output, got empty '.corrected' file, changed back, and got again empty '.corrected' file. So, it is almost impossible to understand what is wrong.

Yes, I agree, that's very annoying. That happens when you don't have any %%expect extension node in your test file (for example, if you have a typo like %%espect or %expect or so). That's because we're using our own expect test version as opposed to the ppx_expect opam package to avoid circular dependencies.

it should be somehow connected to autoformatting.

The reason why we disable ocamlformat for expect test files is that our expect test doesn't respect ocamlformat formatting, so promoting dune runtest and dune build @fmt would yield conflicting results when ocamlformat isn't disabled.

In case you want to give it another try: I'd expect the following to work (you'd simply have to dune promote it):

(* The `as__` combinator captures the whole pattern it is given
  in addition to the internally captured values in that pattern.
  The value it captures is passed to the continuation before the internally captured values. *)
let parse_result =
  let loc = Location.none in
  let ast = [%expr List.length xs = 0] in
  let pat =
    let open Ast_pattern in
    let length () =
      as__
        (pexp_apply
           (pexp_ident (ldot (lident (string "List")) (string "length")))
           ((nolabel ** drop) ^:: nil))
    in
    let zero () = as__ (pexp_constant __) in

    pexp_apply
      (pexp_ident (lident (string "=")))
      ((nolabel ** length ()) ^:: (nolabel ** zero ()) ^:: nil)
  in

  Ast_pattern.parse pat loc ast
    (fun first_as_captured_val second_as_captured_val intrenally_captured_val ->
      (first_as_captured_val, second_as_captured_val, intrenally_captured_val))
[%%expect{|
|}]

It's pretty much your second test with one modification: it tests the behavior of as__ itself. Testing the pitfall as well is also great though!

If you still really don't want to use the expect test framework, we can find another solution.

[Kakadu]

If you still really don't want to use the expect test framework, we can find another solution.

I managed to rewrite with expect-tests

[pitag-ha]

Thanks for the improvements! I only have a couple of minor comments about the documentation that I'd like to be addressed before merging (notice that my suggestions aren't formatted).

[Kakadu]

@pitag-ha ^^

[pitag-ha]

Thanks for the change. LGTM!