Don't validate `Yojson.Safe.t` abstract type
MisterDA opened this issue · 4 comments
Yojson rename it's Yojson.Safe.json type to Yojson.Safe.t and deprecated the old type. It also removed the Yojson.Safe.validate_json and deliberately not introduced a new Yojson.Safe.validate_t. I have an atd document using the json type, and I'd like to validation for the values produced, but however I write it, I cannot get atd to turn off validation for that type: it always produces code that call the non-existent Yojson.Safe.validate_t function.
cat > test.atd <<EOF
type json <ocaml module="Yojson.Safe" t="t"> = abstract <ocaml valid="fun _x -> true">
type json' <ocaml module="Yojson.Safe" t="t"> = abstract <ocaml validator="fun _path _x -> None">
EOF
atdgen -v test.atd
ocamlfind ocamlc -o test.exe -linkpkg -package yojson,atdgen test_v.mlproduces
(* Auto-generated from "test.atd" *)
[@@@ocaml.warning "-27-32-35-39"]
type json' = Yojson.Safe.t
type json = Yojson.Safe.t
let validate_json' = (
(fun path x -> match ( fun _path _x -> None ) path x with | Some _ as err -> err | None -> (Yojson.Safe.validate_t) path x)
)
let validate_json = (
(fun path x -> match ( fun path x ->
let msg = "Failed check by fun _x -> true" in
if (fun _x -> true) x then
None
else
Some (Atdgen_runtime.Util.Validation.error ~msg path) ) path x with | Some _ as err -> err | None -> (Yojson.Safe.validate_t) path x)
)and
File "test_v.ml", line 9, characters 91-113:
9 | (fun path x -> match ( fun _x _y -> None ) path x with | Some _ as err -> err | None -> (Yojson.Safe.validate_t) path x)
^^^^^^^^^^^^^^^^^^^^^^
Error: Unbound value Yojson.Safe.validate_t
I think this change in yojson is going to break user code no matter what we fix in atdgen, because the user code is type json <ocaml module="Yojson.Safe" t="t"> in their ATD file. I don't think atdgen should interpret Yojson.Safe in a special manner either because it's already complicated enough as it is (but at least a warning showing how to fix the problem seems desirable).
Some ideas:
- Add dedicated support to atd/atdgen for handling raw JSON. The idea would be to make JSON support more built-in or more abstract so that we're not tied to a specific OCaml signature or library in the future.
- Impose a version constraint on yojson for all past atdgen releases to keep things working.
- Only relax the package constraint on yojson once atdgen offers a solution for fixing legacy code. This won't fix legacy code automatically, though.
We could also have the atdgen runtime provide its own standardized interface for using raw JSON. It would define the JSON type like Yojson.Safe.t except that it wouldn't change if yojson decides to change its interface. So, instead of
type json <ocaml module="Yojson.Safe" t="t"> = abstractwe would write:
type json <ocaml module="JSON"> = abstractwhere JSON is a module provided by the atdgen runtime library. The generated code would start with open Atdgen_runtime (or whatever it's called) so as to have the JSON module directly in scope.
Initially, the interface of JSON would be the same as Yojson.Safe, until the latter changes.
JSON/OCaml adapters would also use the standardized JSON.t type instead of Yojson.Safe.t.
This wouldn't break compatibility with legacy atd files since the types Yojson.Safe.t and JSON.t are equal (at least initially) thanks to the use of polymorphic variants (i.e. structural typing).
Perhaps (almost) all of this and more would be solved by having proper module/multifile support in the ATD language.