Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: implement To/FromJSON Empty #5421

Open
wants to merge 4 commits into
base: master
Choose a base branch
from

Conversation

TomasPuverle
Copy link
Contributor

Resolve cases when the To/FromJSON type classes are used with Empty, e.g. in the following motivating example.

import Lean

structure Foo (α : Type) where
  y : Option α
deriving Lean.ToJson

#eval Lean.toJson (⟨none⟩ : Foo Empty) -- fails

This is a follow-up to this PR #5415, as suggested by @eric-wieser. It expands on the original suggestion by also handling FromJSON.

@TomasPuverle
Copy link
Contributor Author

CC: @kmill in case interested/relevant.

@github-actions github-actions bot added the toolchain-available A toolchain is available for this PR, at leanprover/lean4-pr-releases:pr-release-NNNN label Sep 23, 2024
@leanprover-community-mathlib4-bot
Copy link
Collaborator

Mathlib CI status (docs):

  • ❗ Batteries/Mathlib CI will not be attempted unless your PR branches off the nightly-with-mathlib branch. Try git rebase 738435b90aaa251a731cc463d6e49560f2037df9 --onto a6830f90ab365e14ccb7ca31201de37f8c1e978c. (2024-09-23 03:47:18)

/-! Ensure we can parse the type back -/
/-- info: Except.ok { y := some 1 } -/
#guard_msgs in
#eval fromStrHelper (Foo Nat) "{\"y\": 1}"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Perhaps it's not in core, but somewhere there's a json% elaborator which should avoid the quoting awkwardness here.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There's also raw strings:

Suggested change
#eval fromStrHelper (Foo Nat) "{\"y\": 1}"
#eval fromStrHelper (Foo Nat) r#"{"y": 1}"#

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
#eval fromStrHelper (Foo Nat) "{\"y\": 1}"
#eval Lean.fromJson? (α := Foo Nat) <| json% {"y": 1}

@@ -28,6 +28,9 @@ instance : ToJson Json := ⟨id⟩
instance : FromJson JsonNumber := ⟨Json.getNum?⟩
instance : ToJson JsonNumber := ⟨Json.num⟩

instance : FromJson Empty where
fromJson? j := throw s!"no constructor matched JSON value '{j}'"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As long as the string is a literal here, rather than something generated by deriving, it might save someone some debugging pain someday to write:

Suggested change
fromJson? j := throw s!"no constructor matched JSON value '{j}'"
fromJson? j := throw s!"type Empty has no constructor to match JSON value '{j}'"

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
toolchain-available A toolchain is available for this PR, at leanprover/lean4-pr-releases:pr-release-NNNN
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants