Yojson 3 compat

[Leonidas-from-XIV]

Yojson 3 removed Tuple and Variant which makes the Json type mismatch with the one defined in this repo. This PR removes these variants and updates the constraint to require Yojson 3 so the type matches again.

Depends on ocaml/opam-repository#27956 merged or pinning Yojson.

[coveralls]

Pull Request Test Coverage Report for Build 4870

Details

• 0 of 0 changed or added relevant lines in 0 files are covered.
• No unchanged relevant lines lost coverage.
• Overall coverage increased (+0.003%) to 23.48%

---

Totals
Change from base Build 4864:	0.003%
Covered Lines:	6042
Relevant Lines:	25733

---

💛 - Coveralls

[giltho]

Re-ran the CI now that yojson3 has been merged, it seems that only the changelog change is required :)

[Leonidas-from-XIV]

Added a changelog entry, hope it is fine as-is :)

[voodoos]

Thank you for taking the time to do the upgrade!

We haven't been doing a release in quite some time, and it is now imminent.
Are these changes going to prevent users that rely on a package that is not compatible with YJS 3 to install ocaml-lsp ?
Are they many incompatible packages ?

[Leonidas-from-XIV]

Are these changes going to prevent users that rely on a package that is not compatible with YJS 3 to install ocaml-lsp ?

Yes, unfortunately. Maybe the code can be changed so the Tuple and Variant constructors are only conditionally added after parsing, something akin to [ #Yojson.Safe.t | Tuple | Variant ] but that would require some work to make sure that these two constructors get translated to values that can be represented in Yojson.Safe.t before passing it on to any Yojson.Safe.* functions.

If it matters, I can look into it on the next hackday if I can find an implementation that accepts the 2.x style type and always outputs the 3.x style type.

(A bit of self-promotion, but this is not a problem in dune pkg, as LSP is installed in a separate location and its dependencies do not conflict with the users project)

Are they many incompatible packages ?

Unfortunately all packages that currently use atdgen as it hasn't been updated for Yojson 3 yet. Apart from that not that many: ocaml/opam-repository#27956, ocf, ppx_deriving (only when building with tests), kind2, goblint, pa_ppx.

[voodoos]

(A bit of self-promotion, but this is not a problem in dune pkg, as LSP is installed in a separate location and its dependencies do not conflict with the users project)

That's great to know !

What I propose is that we hold onto this until the upcoming release, and after the release we will release a version for Yojson 3, hopefully backward compatible if someone finds a way 🙂

[giltho]

If it matters, I can look into it on the next hackday if I can find an implementation that accepts the 2.x style type and always outputs the 3.x style type.

I tried that for a second, but you need to also modify ppx_yojson_conv (I think) because it hardcodes Yojson.Safe.t as an input to functions.

@voodoos Note that the inverse problem also exists, people using Yojson3 cannot install ocaml-lsp-server.

[voodoos]

It seems quite simple to have ocaml-lsp compatible with both yojson 2 and 3 but this comes with a drawback: we need to effectively introduce a dependency on yojson to the jsonrpc package. @rgrinberg is that something we can do ? I looked at the packages published on opam that use jsonrpc and they all already depend on yojson anyway.

cc @pitag-ha and @xvw who will probably have to follow up on this :-)

[rgrinberg]

No objection to using yojson in jsonrpc, but I don't see the point of avoiding the dependency on yojson > 3. Ocamllsp is an application anyway, nobody is going to care much that json library we're using.

[voodoos]

Opam remains the main way to install ocaml-lsp today, and I guess most users do that in their project's switch. Ecosystem splits are even more sensible than usual for projects like ocaml-lsp, because they are installed in almost every switch.

[voodoos]

Thanks everyone, I am going to merge this ahead of the final release for OCaml 5.4.