[PATCH] json: Add support for encoding structs

[PATCH] json: Add support for encoding structs

[Vibhav Pant]

The following patch to json.el adds support for encoding record types
declared via cl-defstruct. It also allows the user to change the json
key for a struct field by using the `:json' keyword while declaring
a slot.

An example of the new json-encode:
ELISP> (cl-defstruct foo (f1 "foo") (f2 "bar" :json "field2"))
foo
ELISP> (json-encode (make-foo))
"{\"f1\":\"foo\",\"field2\":\"bar\"}"


---
diff --git a/lisp/json.el b/lisp/json.el
index 049c9b1951..cc09b46b7c 100644
--- a/lisp/json.el
+++ b/lisp/json.el
@@ -53,6 +53,7 @@
 ;;; Code:

 (require 'map)
+(require 'cl-lib)

 ;; Parameters

@@ -549,6 +550,33 @@ json-encode-hash-table
                 ""
               json--encoding-current-indentation))))

+;; Struct encoding
+(defun json-encode-struct (struct)
+  "Return a JSON representation of STRUCT."
+  (let* ((struct-type (type-of struct))
+         (slots-info (cdr (cl-struct-slot-info struct-type))))
+    (format "{%s%s}"
+            (json-join
+             (json--with-indentation
+              (mapcar #'(lambda (slot)
+                          (let* ((slot-name (car slot))
+                                 (opts (cddr slot))
+                                 (key (or (plist-get opts :json) slot-name)))
+                            (format (if json-encoding-pretty-print
+                                        "%s%s: %s"
+                                      "%s%s:%s")
+                                    json--encoding-current-indentation
+                                    (json-encode-key key)
+                                    (json-encode
+                                     (cl-struct-slot-value
struct-type slot-name
+                                                           struct)))))
+                      slots-info))
+             json-encoding-separator)
+            (if (or (not json-encoding-pretty-print)
+                    json-encoding-lisp-style-closings)
+                ""
+              json--encoding-current-indentation))))
+
 ;; List encoding (including alists and plists)

 (defun json-encode-alist (alist)
@@ -721,6 +749,7 @@ json-encode
         ((arrayp object)       (json-encode-array object))
         ((hash-table-p object) (json-encode-hash-table object))
         ((listp object)        (json-encode-list object))
+        ((cl-struct-p object)  (json-encode-struct object))
         (t                     (signal 'json-error (list object)))))

 ;; Pretty printing


-- 
Vibhav Pant
vibhavp@gmail.com

Re: [PATCH] json: Add support for encoding structs

[Ted Zlatanov]

On Sun, 23 Apr 2017 15:46:35 +0530 Vibhav Pant <vibhavp@gmail.com> wrote: 

VP> The following patch to json.el adds support for encoding record types
VP> declared via cl-defstruct. It also allows the user to change the json
VP> key for a struct field by using the `:json' keyword while declaring
VP> a slot.

Nice! I hope that goes in. Can you please write tests for it?

Is it possible to also exclude fields with :json nil? I think that's
very useful.

Finally, it would be great to be able to declare the JSON type of the
field.

Ted

Re: [PATCH] json: Add support for encoding structs

[Clément Pit-Claudel]

On 2017-04-23 06:16, Vibhav Pant wrote:
> The following patch to json.el adds support for encoding record types
> declared via cl-defstruct. It also allows the user to change the json
> key for a struct field by using the `:json' keyword while declaring
> a slot.
> 
> An example of the new json-encode:
> ELISP> (cl-defstruct foo (f1 "foo") (f2 "bar" :json "field2"))
> foo
> ELISP> (json-encode (make-foo))
> "{\"f1\":\"foo\",\"field2\":\"bar\"}"

This looks nice, thanks.  Two questions:

* Will this break existing code? Or did the switch to records already break things? IIUC serializing a record until now would have serialized the underlying vector, whereas in the new code the representation will change.
* Do you plan to add deserializing support, too ?

Thanks!
Clément.

Re: [PATCH] json: Add support for encoding structs

[Vibhav Pant]

On Mon, Apr 24, 2017 at 6:55 PM, Ted Zlatanov <tzz@lifelogs.com> wrote:
> Nice! I hope that goes in. Can you please write tests for it?
Done.


> Is it possible to also exclude fields with :json nil? I think that's
> very useful.
That would mean that all struct field will have to be explicitly declared
with a :json option to not be ignored. Instead, I've added a :json-ignore
option that makes json-encode-struct ignore the slot ignore it when it's
non-nil.

> Finally, it would be great to be able to declare the JSON type of the
> field.
Yep, done.


---
diff --git a/lisp/json.el b/lisp/json.el
index 049c9b1951..95946b0b0c 100644
--- a/lisp/json.el
+++ b/lisp/json.el
@@ -53,6 +53,7 @@
 ;;; Code:

 (require 'map)
+(require 'cl-lib)

 ;; Parameters

@@ -549,6 +550,60 @@ json-encode-hash-table
                 ""
               json--encoding-current-indentation))))

+(defun json-to-string (object)
+  (if (numberp object)
+      (number-to-string object)
+    object))
+
+(defun json-to-number (object)
+  (if (stringp object)
+      (string-to-number object)
+    object))
+
+(defun json-convert-to-type (object type)
+  (cond ((memq type '(number integer))
+         (json-to-number object))
+        ((eq type 'float)
+         (float (string-to-number object)))
+        ((eq type 'string)
+         (json-to-string object))
+        ((eq type nil) object)
+        (t (error "Unknown :json-type value."))))
+
+;; Struct encoding
+(defun json-encode-struct (struct)
+  "Return a JSON representation of STRUCT."
+  (let* ((struct-type (type-of struct))
+         (slots-info (cdr (cl-struct-slot-info struct-type))))
+    (format "{%s%s}"
+            (json-join
+             (json--with-indentation
+              (remq
+               nil
+               (mapcar #'(lambda (slot)
+                           (let* ((slot-name (car slot))
+                                  (opts (cddr slot))
+                                  (key (or (plist-get opts :json) slot-name))
+                                  (ignore (plist-get opts :json-ignore))
+                                  (type (plist-get opts :json-type))
+                                  (slot-value (cl-struct-slot-value struct-type
+                                                                    slot-name
+                                                                    struct)))
+                             (unless ignore
+                               (format (if json-encoding-pretty-print
+                                           "%s%s: %s"
+                                         "%s%s:%s")
+                                       json--encoding-current-indentation
+                                       (json-encode-key key)
+                                       (json-encode (json-convert-to-type
+                                                     slot-value type))))))
+                       slots-info)))
+             json-encoding-separator)
+            (if (or (not json-encoding-pretty-print)
+                    json-encoding-lisp-style-closings)
+                ""
+              json--encoding-current-indentation))))
+
 ;; List encoding (including alists and plists)

 (defun json-encode-alist (alist)
@@ -721,6 +776,7 @@ json-encode
         ((arrayp object)       (json-encode-array object))
         ((hash-table-p object) (json-encode-hash-table object))
         ((listp object)        (json-encode-list object))
+        ((cl-struct-p object)  (json-encode-struct object))
         (t                     (signal 'json-error (list object)))))

 ;; Pretty printing
diff --git a/test/lisp/json-tests.el b/test/lisp/json-tests.el
index 38672de066..58065d22e5 100644
--- a/test/lisp/json-tests.el
+++ b/test/lisp/json-tests.el
@@ -31,6 +31,30 @@ json-tests--with-temp-buffer
      (goto-char (point-min))
      ,@body))

+;;; Structs
+
+(cl-defstruct json-test-struct
+  (f1 1 :json "field-1" :json-type number)
+  (f2 'foo :json "field-2" :json-type string)
+  (f3 (current-buffer) :json-ignore t) ;; this field should be ignored
+  (f4 "1" :json "field-4" :json-type number))
+
+(ert-deftest test-json-structs ()
+  (should (equal (json-encode-struct (make-json-test-struct))
+                 "{\"field-1\":1,\"field-2\":\"foo\",\"field-4\":1}"))
+  (should (equal (json-encode-struct (make-json-test-struct
+                                      :f1 "123"
+                                      :f2 123
+                                      :f4 423))
+                 "{\"field-1\":123,\"field-2\":\"123\",\"field-4\":423}"))
+  (should (equal (json-encode-struct (make-json-test-struct
+                                      :f1 1.79e+308
+                                      :f2 -1.79e+308
+                                      :f4 423))
+
"{\"field-1\":1.79e+308,\"field-2\":\"-1.79e+308\",\"field-4\":423}"))
+  (should-error (json-encode (make-json-test-struct :f1 (current-buffer)))
+                :type 'json-error))
+
 ;;; Utilities

 (ert-deftest test-json-join ()


-- 
Vibhav Pant
vibhavp@gmail.com

Re: [PATCH] json: Add support for encoding structs

[Ted Zlatanov]

On Mon, 24 Apr 2017 23:24:13 +0530 Vibhav Pant <vibhavp@gmail.com> wrote: 

>> Is it possible to also exclude fields with :json nil? I think that's
>> very useful.
VP> That would mean that all struct field will have to be explicitly declared
VP> with a :json option to not be ignored. Instead, I've added a :json-ignore
VP> option that makes json-encode-struct ignore the slot ignore it when it's
VP> non-nil.

Oh, right! Thank you for contributing this nice improvement. I look
forward to using it.

I also want to ask if the keywords could be less JSON-specific, since
other serializers could be used. So you'd use keywords like
:serialize-field, :serialize-ignore, :serialize-to-type instead of the
ones you proposed.

Finally, maybe we could also consider using the existing facility
:print-function (the manual says that's currently unused). But I don't
know the full implications of that, so maybe it's a bad suggestion.

Ted

Re: [PATCH] json: Add support for encoding structs

[Ted Zlatanov]

On Mon, 24 Apr 2017 15:17:08 -0400 Ted Zlatanov <tzz@lifelogs.com> wrote: 

TZ> I also want to ask if the keywords could be less JSON-specific, since
TZ> other serializers could be used. So you'd use keywords like
TZ> :serialize-field, :serialize-ignore, :serialize-to-type instead of the
TZ> ones you proposed.

TZ> Finally, maybe we could also consider using the existing facility
TZ> :print-function (the manual says that's currently unused). But I don't
TZ> know the full implications of that, so maybe it's a bad suggestion.

BTW EIEIO has support for serializing already, so maybe it makes sense
to make your feature use their keywords (:printer etc.):
https://www.gnu.org/software/emacs/manual/html_node/eieio/Slot-Options.html#Slot-Options

...or maybe their stuff is too object-oriented :)

Ted

Re: [PATCH] json: Add support for encoding structs

[Stefan Monnier]

> I also want to ask if the keywords could be less JSON-specific, since
> other serializers could be used. So you'd use keywords like
> :serialize-field, :serialize-ignore, :serialize-to-type instead of the
> ones you proposed.

Indeed.  Other thing: please try and think about the impact those field
might have on the round-tripping of data through JSON (i.e. how to make
it so that decoding the JSON output back to Elisp gives equivalent data).

> Finally, maybe we could also consider using the existing facility
> :print-function (the manual says that's currently unused).

I think this :print-function is now obsolete since it was meant to
hook into `princ`, but that can now be done in cl-print.

> But I don't know the full implications of that, so maybe it's
> a bad suggestion.

I don't think :print-function and cl-print can be of much use for
encoding to JSON.


        Stefan

Re: [PATCH] json: Add support for encoding structs

[Davis Herring]

> That would mean that all struct field will have to be explicitly declared
> with a :json option to not be ignored.

You could use :json "" or :json 'ignore as the special value.

> Instead, I've added a :json-ignore option that makes
> json-encode-struct ignore the slot ignore it when it's non-nil.

I'd prefer not to have

   :json "purported" :json-ignore t

as a possible input.

Davis

-- 
This product is sold by volume, not by mass.  If it appears too dense or 
too sparse, it is because mass-energy conversion has occurred during 
shipping.

Re: [PATCH] json: Add support for encoding structs

[Vibhav Pant]

On Tue, Apr 25, 2017 at 12:47 AM, Ted Zlatanov <tzz@lifelogs.com> wrote:
> I also want to ask if the keywords could be less JSON-specific, since
> other serializers could be used. So you'd use keywords like
> :serialize-field, :serialize-ignore, :serialize-to-type instead of the
> ones you proposed.

Done!

> Finally, maybe we could also consider using the existing facility
> :print-function (the manual says that's currently unused). But I don't
> know the full implications of that, so maybe it's a bad suggestion.

Wouldn't that restrict encoding only to JSON?, I think passing all
(json) encoding through json.el would be a better design.

Thanks,
Vibhav
-- 
Vibhav Pant
vibhavp@gmail.com

Re: [PATCH] json: Add support for encoding structs

[Vibhav Pant]

On Tue, Apr 25, 2017 at 4:22 AM, Davis Herring <herring@lanl.gov> wrote:
> You could use :json "" or :json 'ignore as the special value.

IIRC, the empty string is a valid JSON key (`JSON.parse(`{"":1}`)`
works in JS).
Symbols are also considered a valid type according to
`json-encode-key'.

-- 
Vibhav Pant
vibhavp@gmail.com

Re: [PATCH] json: Add support for encoding structs

[Ted Zlatanov]

On Tue, 25 Apr 2017 18:10:59 +0530 Vibhav Pant <vibhavp@gmail.com> wrote: 

VP> On Tue, Apr 25, 2017 at 4:22 AM, Davis Herring <herring@lanl.gov> wrote:
>> You could use :json "" or :json 'ignore as the special value.

VP> IIRC, the empty string is a valid JSON key (`JSON.parse(`{"":1}`)`
VP> works in JS).
VP> Symbols are also considered a valid type according to
VP> `json-encode-key'.

Maybe one way is to distinguish '(:serialize-field nil) from '() by
checking with memq for the key?

On Tue, 25 Apr 2017 17:57:22 +0530 Vibhav Pant <vibhavp@gmail.com> wrote: 

>> Finally, maybe we could also consider using the existing facility
>> :print-function (the manual says that's currently unused). But I don't
>> know the full implications of that, so maybe it's a bad suggestion.

VP> Wouldn't that restrict encoding only to JSON?, I think passing all
VP> (json) encoding through json.el would be a better design.

Yeah, it's not a great suggestion :)

Thanks for all the work.

Ted

Re: [PATCH] json: Add support for encoding structs

[Vibhav Pant]

On Tue, Apr 25, 2017 at 7:56 PM, Ted Zlatanov <tzz@lifelogs.com> wrote:
> Maybe one way is to distinguish '(:serialize-field nil) from '() by
> checking with memq for the key?

Good idea. `plist-member' seems to do the job. I've updated the patch
accordingly, thanks.


---
diff --git a/lisp/json.el b/lisp/json.el
index 049c9b1951..0d4503fe3b 100644
--- a/lisp/json.el
+++ b/lisp/json.el
@@ -53,6 +53,7 @@
 ;;; Code:

 (require 'map)
+(require 'cl-lib)

 ;; Parameters

@@ -222,6 +223,8 @@ 'json-key-format
 (define-error 'json-object-format "Bad JSON object" 'json-error)
 (define-error 'json-end-of-file "End of file while parsing JSON"
   '(end-of-file json-error))
+(define-error 'json-unkown-serialize-type "Unknown :serialize-to-type value"
+  'json-error)



@@ -549,6 +552,62 @@ json-encode-hash-table
                 ""
               json--encoding-current-indentation))))

+(defun json-to-string (object)
+  (if (numberp object)
+      (number-to-string object)
+    object))
+
+(defun json-to-number (object)
+  (if (stringp object)
+      (string-to-number object)
+    object))
+
+(defun json-convert-to-type (object type)
+  (cond ((memq type '(number integer))
+         (json-to-number object))
+        ((eq type 'float)
+         (float (string-to-number object)))
+        ((eq type 'string)
+         (json-to-string object))
+        ((eq type nil) object)
+        (t (signal 'json-unkown-serialize-type type))))
+
+;; Struct encoding
+(defun json-encode-struct (struct)
+  "Return a JSON representation of STRUCT."
+  (let* ((struct-type (type-of struct))
+         (slots-info (cdr (cl-struct-slot-info struct-type))))
+    (format "{%s%s}"
+            (json-join
+             (json--with-indentation
+              (remq
+               nil
+               (mapcar #'(lambda (slot)
+                           (let* ((slot-name (car slot))
+                                  (opts (cddr slot))
+                                  (serialize-field (plist-get opts
:serialize-field))
+                                  (key (or serialize-field slot-name))
+                                  (ignore (and (null serialize-field)
+                                               (plist-member opts
:serialize-field)))
+                                  (type (plist-get opts :serialize-to-type))
+                                  (slot-value (cl-struct-slot-value struct-type
+                                                                    slot-name
+                                                                    struct)))
+                             (unless ignore
+                               (format (if json-encoding-pretty-print
+                                           "%s%s: %s"
+                                         "%s%s:%s")
+                                       json--encoding-current-indentation
+                                       (json-encode-key key)
+                                       (json-encode (json-convert-to-type
+                                                     slot-value type))))))
+                       slots-info)))
+             json-encoding-separator)
+            (if (or (not json-encoding-pretty-print)
+                    json-encoding-lisp-style-closings)
+                ""
+              json--encoding-current-indentation))))
+
 ;; List encoding (including alists and plists)

 (defun json-encode-alist (alist)
@@ -721,6 +780,7 @@ json-encode
         ((arrayp object)       (json-encode-array object))
         ((hash-table-p object) (json-encode-hash-table object))
         ((listp object)        (json-encode-list object))
+        ((cl-struct-p object)  (json-encode-struct object))
         (t                     (signal 'json-error (list object)))))

 ;; Pretty printing
diff --git a/test/lisp/json-tests.el b/test/lisp/json-tests.el
index 38672de066..1b17c3c55d 100644
--- a/test/lisp/json-tests.el
+++ b/test/lisp/json-tests.el
@@ -294,6 +294,30 @@ json-tests--with-temp-buffer
     (should (equal (json-encode-array [1 2 "a" "b"])
                    "[1,2,\"a\",\"b\"]"))))

+;;; Structs
+
+(cl-defstruct json-test-struct
+  (f1 1 :serialize-field "field-1" :serialize-to-type number)
+  (f2 'foo :serialize-field "field-2" :serialize-to-type string)
+  (f3 (current-buffer) :serialize-field nil) ;; this field should be ignored
+  (f4 "1" :serialize-field "field-4" :serialize-to-type number))
+
+(ert-deftest test-json-structs ()
+  (should (equal (json-encode-struct (make-json-test-struct))
+                 "{\"field-1\":1,\"field-2\":\"foo\",\"field-4\":1}"))
+  (should (equal (json-encode-struct (make-json-test-struct
+                                      :f1 "123"
+                                      :f2 123
+                                      :f4 423))
+                 "{\"field-1\":123,\"field-2\":\"123\",\"field-4\":423}"))
+  (should (equal (json-encode-struct (make-json-test-struct
+                                      :f1 1.79e+308
+                                      :f2 -1.79e+308
+                                      :f4 423))
+
"{\"field-1\":1.79e+308,\"field-2\":\"-1.79e+308\",\"field-4\":423}"))
+  (should-error (json-encode (make-json-test-struct :f1 (current-buffer)))
+                :type 'json-error))
+
 ;;; Reader

 (ert-deftest test-json-read ()

-- 
Vibhav Pant
vibhavp@gmail.com

Re: [PATCH] json: Add support for encoding structs

[Ted Zlatanov]

On Tue, 25 Apr 2017 21:56:16 +0530 Vibhav Pant <vibhavp@gmail.com> wrote: 

VP> On Tue, Apr 25, 2017 at 7:56 PM, Ted Zlatanov <tzz@lifelogs.com> wrote:
>> Maybe one way is to distinguish '(:serialize-field nil) from '() by
>> checking with memq for the key?

VP> Good idea. `plist-member' seems to do the job. I've updated the patch
VP> accordingly, thanks.

It looks good to me!

(Aside: it would be nice if ERT had a way to check JSON or other
structured data in a whitespace-insensitive, canonical way, like what
`jq --sort-keys .' might produce.)

Ted

Re: [PATCH] json: Add support for encoding structs

[Stefan Monnier]

> Maybe one way is to distinguish '(:serialize-field nil) from '() by
> checking with memq for the key?

FWIW, while you can make this work, I think it's a bad API: Elisp
usually behaves like "an absent argument is equivalent to nil", and it's
easier for the coder if we stick to this behavior (also, occasionally
it's handy to be able to use ":key nil" in order not to pass :key).

> IIRC, the empty string is a valid JSON key (`JSON.parse(`{"":1}`)`
> works in JS).  Symbols are also considered a valid type according to
> `json-encode-key'.

I don't see why that forces us to also accept symbols here.


        Stefan

Re: [PATCH] json: Add support for encoding structs

[Vibhav Pant]

On Wed, Apr 26, 2017 at 5:43 PM, Stefan Monnier
<monnier@iro.umontreal.ca> wrote:
> FWIW, while you can make this work, I think it's a bad API: Elisp
> usually behaves like "an absent argument is equivalent to nil", and it's
> easier for the coder if we stick to this behavior (also, occasionally
> it's handy to be able to use ":key nil" in order not to pass :key).
Fair point, I was thinking that slot opts aren't exactly an API, so this
was an exception.

>> IIRC, the empty string is a valid JSON key (`JSON.parse(`{"":1}`)`
>> works in JS).  Symbols are also considered a valid type according to
>> `json-encode-key'.
>
> I don't see why that forces us to also accept symbols here.

Wouldn't it be consistent to allow symbols in both `json-encode' and
`json-encode-key`?




-- 
Vibhav Pant
vibhavp@gmail.com

Re: [PATCH] json: Add support for encoding structs

[Stefan Monnier]

>> FWIW, while you can make this work, I think it's a bad API: Elisp
>> usually behaves like "an absent argument is equivalent to nil", and it's
>> easier for the coder if we stick to this behavior (also, occasionally
>> it's handy to be able to use ":key nil" in order not to pass :key).
> Fair point, I was thinking that slot opts aren't exactly an API, so this
> was an exception.

That's the point: it would be an exception.
There are a few places where we distinguish "absence" from nil, and in
my experience these are better avoided.

>>> IIRC, the empty string is a valid JSON key (`JSON.parse(`{"":1}`)`
>>> works in JS).  Symbols are also considered a valid type according to
>>> `json-encode-key'.
>> I don't see why that forces us to also accept symbols here.
> Wouldn't it be consistent to allow symbols in both `json-encode' and
> `json-encode-key`?

I don't see how that's relevant: we're talking about the thing that
follows :json (or somesuch) in a cl-defstruct, not a call to some json
encoding function.


        Stefan