atdgen: update abstract type validation to accept all input by default

[metanivek]

This PR follows up on #295 and #263 by making the default validation for abstract types pass for any input, while still allowing for custom validation.

I used the following definition for testing, which appears to my eyes to be correct and compiles successfully (with a small util.ml with always-pass validation definitions).

type raw_json = abstract
type raw_json_valid = abstract <ocaml valid="fun _ -> true">

type demo = {
  one: raw_json;
  two: raw_json <ocaml validator="Util.validate_abstract">;
  three: raw_json <ocaml valid="fun _ -> true">;
  four: abstract;
  five: abstract <ocaml validator="Util.validate_abstract">;
  six: abstract <ocaml valid="fun _ -> true">;
  seven: raw_json_valid;
}

Feedback very much welcome since this is my first time in this code base!

PR checklist

• New code has tests to catch future regressions
• Documentation is up-to-date
• CHANGES.md is up-to-date

[mjambon]

Hi! Personally, I haven't done much with validators in a while, so it's not something that was on my mind when I recently extended what we can do with abstract. It would be great if you could post an example illustrating the problem you're solving (input, output, expected output). After that, you should add some tests that show that your fix works.

[mjambon]

After looking at the tests we have, it looks indeed that we have nothing for testing the validation of abstract values. There's the file test_abstract.atd which could be reused for testing validation, or you can create a new one just for this. I think it's nice to keep test files focused on one feature rather than putting everything in a giant test.atd like we still have (note that you may not be able to add type foo = abstract to test.atd anyway due to the biniou backend not supporting it). You'll probably have to edit atdgen/test/dune to add instructions for running atdgen -v.

[metanivek]

Thanks for the pointers on the tests. I will work on extending the existing abstract test for validations (and/or add more if there are use cases that aren't covered -- I agree with focusing test files).

---

Here are some focused examples that don't work as expected (in fact, don't compile) with the current atdgen.

Untyped json without validator

type demo {
  json: abstract
}

atdgen.2.8.0 output (just validation part of .ml). Notice the undefined validate_abstract (note: this used to be an undefined call to Yojson.Safe.validate_t which prompted a PR I previously posted for that project that led to the decision to remove validation from yojson altogether).

let validate_demo : _ -> demo -> _ = (
  fun path x ->
    (
      validate_abstract
    ) (`Field "json" :: path) x.json
)

This PR output. Validation is now an always-pass validation (which is what Yojson.Safe.validate_json did before its removal). Note: I'm not sure why it does not output a collapsed always-pass, like for example type demo { json: string }, but this is functionally the same.

let validate_demo : _ -> demo -> _ = (
  fun path x ->
    (
      (fun _ _ -> None)
    ) (`Field "json" :: path) x.json
)

Untyped json with validator

type demo = {
  json: abstract <ocaml valid="fun _ -> true">
}

atdgen.2.8.0. Again, undefined validate_abstract.

let validate_demo : _ -> demo -> _ = (
  fun path x ->
    (
      (fun path x -> match ( fun path x ->
    let msg = "Failed check by fun _ -> true" in
    if (fun _ -> true) x then
      None
    else
      Some (Atdgen_runtime.Util.Validation.error ~msg path) ) path x with | Some _ as err -> err | None -> (validate_abstract) path x)
    ) (`Field "json" :: path) x.json
)

This PR output. Uses defined validator without a fallback.

let validate_demo : _ -> demo -> _ = (
  fun path x ->
    (
      fun path x ->
    let msg = "Failed check by fun _ -> true" in
    if (fun _ -> true) x then
      None
    else
      Some (Atdgen_runtime.Util.Validation.error ~msg path)
    ) (`Field "json" :: path) x.json
)

---

I think this is pointing in a good direction, but it's totally possible I'm missing something! I'll post more if I run into anything while adding tests, too.

[metanivek]

@mjambon just pushed added/updated tests. If you think it would be good to have tests for the validators (like in my examples), let me know. I'm happy to add that as well.

[mjambon]

The tests you added are sufficient, I think. Thank you! I'll let you update the changelog.

[metanivek]

@mjambon changelog updated (and title of PR :)

[mjambon]

Thank you! Merging and starting a release.