guile-json icon indicating copy to clipboard operation
guile-json copied to clipboard

`define-json-mapping` allows nonsensical `key` subforms

Open LiberalArtist opened this issue 2 years ago • 0 comments

I recently wrote something like the following buggy code:

(define-json-mapping <project-info> make-project-info project-info?
  json->project-info
  (license project-info-license
           (lambda (x)
             (if (string? x)
                 (spdx-string->license x)
                 #f))))

and was confused to find that project-info-license always produced *unspecified*.

Eventually, I worked out that I needed instead to write:

(define-json-mapping <project-info> make-project-info project-info?
  json->project-info
  (license project-info-license
           "license" (lambda (x)
                       (if (string? x)
                           (spdx-string->license x)
                           #f))))

i.e. that writing "license" was required even though it matched license.

Originally, I had read the grammar at:

https://github.com/aconchillo/guile-json/blob/81bc5dad9f588fd49816d2fd3983551e56e078ea/README.md#L331-L348

to mean the field and getter subforms were required, but the key, scm->value, and value->scm subforms were optional, so I thought I would only need to specify key if it were different than field. However, it seems like what I intended as a scm->value expression was being treated as a key, and of course there would never be a key equal? to that procedure, so I ended up with *unspecified* and no scm->value procedure to convert it.

I can see a few possible improvements, depending on what behavior is preferred:

  1. If you want key to be optional in the way I originally imagined:

    1. The expansion of define-json-mapping could check at run-time if the first of the optional sub-forms evaluates to a string and treat it as key if so; scm->value otherwise.

    2. If key is restricted to a string literal, rather than an expression that evaluates to a string, define-json-mapping could perform such a check at compile-time.

  2. If you want key to be required in order to provide scm->value, I think define-json-mapping should still check, either at run-time or compile-time, that key actually is/evaluates to a string, since supplying a procedure for key would never be a reasonable thing to do.

LiberalArtist avatar May 18 '22 17:05 LiberalArtist