starlark icon indicating copy to clipboard operation
starlark copied to clipboard

proposal: JSON Module

Open jmillikin-stripe opened this issue 6 years ago • 10 comments

This document proposes an API for encoding and decoding JSON from Starlark. If accepted, implementations of Starlark that implement JSON support would be expected to implement this API. The goal is to allow users to write JSON code that behaves consistently across Starlark implementations.

jmillikin-stripe avatar Aug 26 '19 03:08 jmillikin-stripe

@laurentlb do you (or anyone else) have concerns with this?

I'd love to see this merged so that we could move towards an implementation. A tool I'm working on needs JSON, and I've ended up copy-pasting files from this PR as there isn't a standard here yet: https://github.com/google/starlark-go/pull/179

bpowers avatar Feb 24 '20 22:02 bpowers

What is the diff to the spec? Does this proposal go as a brand new file in the repo? Assuming its accepted, what diff would we apply to the spec?

No idea! Happy to format this in whatever way seems reasonable to you. It doesn't seem like there's much prior art here, so I formatted it as an RFC-type file.

What is a module? How do you in a standards conforming way import the "json" module? I couldn't see any existing modules in the spec (maybe I missed them?). Or maybe you mean json is an always available name with some fields on it?

Bazel defines a list of "global modules" (https://docs.bazel.build/versions/3.4.0/skylark/lib/skylark-overview.html#--global-modules) but doesn't go into detail about what exactly a "module" is at the language level. For this proposal I had imagined a new global symbol named json that would be present if the current implementation supported JSON. The presence of json is optional, but if present it should conform to this API.

If so, it feels a bit weird to introduce a whole new mechanism not present elsewhere in the spec for just two functions. Or maybe you want to use this mechanism more widely?

If this proposal is successful + accepted then I would like to propose more modules, specifically proto (for protobufs as used in Skycfg) and yaml (for use in Bazel, parsing lock files from the JavaScript ecosystem).

jmillikin-stripe avatar Jul 16 '20 06:07 jmillikin-stripe

Bazel defines a list of "global modules"

But Starlark doesn't. Maybe we need a corresponding piece of the Starlark spec that says what a "module" is, whether it needs loading, how it behaves, whether it's a first-class value etc.

ndmitchell avatar Jul 16 '20 10:07 ndmitchell

Hi, back from vacation. Sorry this proposal has been neglected and that this has process not been a model of good open-source stewardship.

Regarding the the project: Starlark/Java was until very recently highly entangled with Bazel, though after a year of work nearly all its production dependencies on "Bazel proper" are now severed. However, the tests are still entangled, and there are numerous subtle constraints imposed on Starlark by Bazel that I plan to break soon to unblock significant optimizations (e.g. flat environments, byte code compilation). In addition we plan to move the Java classes to a different package (java.starlark.net) to emphasize the boundary between components, though we have no plans yet to host them in a separate repository; that would of course require the API to be refined, stabilized, and frozen. I believe the separation is in the long term interests of the Bazel project and its users, especially if Starlark becomes a widely used and more familiar language for configuration, but for the Bazel team it does pose immediate costs (in ceded control), and the team has yet to make any public commitments about Starlark.

Regarding JSON: In June I committed https://github.com/google/starlark-go/blob/master/starlarkjson/json.go#L26, a JSON module I wrote some years ago for the Go implementation of Starlark (go.starlark.net). I believe its interface addresses all the needs met by the one proposed here, and indeed was influenced by it. It is not too late to change it, so if you see any shortcomings of deficiencies, please let me know. Otherwise I propose that we port it directly to Java.

Regarding modules: Starlark doesn't yet have a standard library of packages, though there is clearly a recurring need for things like json, time, http, and html. (Brendan @b5 at https://github.com/qri-io/starlib has built many good candidates for standardization.) We don't even have a clear decision yet on how such packages should be imported. Sometimes we use load(), other times the names are predeclared (as I did with 'json' in the Go Starlark REPL, though I don't think it's very satisfactory). Personally I would like to extend load so that modules are first-class, e.g. something like load("foo.bzl"); foo.F(), similar to Python's import statement, as many users resort to doing this by hand using structs like os = struct(mkdir = _mkdir, ...).

Regarding proto: I have ported the Go implementation of the Starlark proto module so that it depends only on open-source proto reflection (which is new), so I should be able to open-source it. However, proto support for Starlark/Java will be tricky, because it has no means of representing byte strings, floating-point numbers, or integers outside the signed 32-bit range, and each of those poses surprisingly thorny problems.

BTW: Nick: I enjoyed both of your papers on build systems!

alandonovan avatar Jul 22 '20 16:07 alandonovan

I just committed https://github.com/bazelbuild/bazel/commit/6e47a409914d9f2ccb169482252f142c9fb503b8, which adds a json module to Starlark/Java. Please take a close look at the Starlark and Java interfaces, and try it out, and comment here if notice any problems.

Thanks to @brandjon for many very careful rounds of review.

alandonovan avatar Oct 19 '20 22:10 alandonovan

What's the use-case for the prefix argument in json.indent?

laurentlb avatar Oct 20 '20 17:10 laurentlb

What's the use-case for the prefix argument in json.indent?

Perhaps the caller wants to insert the result inside some other piece of text, or JSON header/footer, that imposes its own indentation.

alandonovan avatar Oct 20 '20 17:10 alandonovan

Both the Java and Go implementations now have compatible implementations of the json module. Closing.

alandonovan avatar Nov 11 '20 20:11 alandonovan

Since it's been implemented, shouldn't this PR be merged rather than closed? Optionally with updates to reflect whatever common API was decided upon by the Java/Go impl devs.

jmillikin-stripe avatar Nov 11 '20 22:11 jmillikin-stripe

Er, yes, sorry about that. I just noticed that I also have a bunch of old pending comments on your doc from months ago that some how never got flushed out; I'll do that now.

The Java implementation is here: https://github.com/bazelbuild/bazel/blob/267bdaab2f0596b65b03c06c82e3f91d957fec3b/src/main/java/net/starlark/java/lib/json/Json.java and it should match the behavior of the Go implementation here: https://github.com/google/starlark-go/blob/master/starlarkjson/json.go at least as far as the Starlark caller is concerned.

alandonovan avatar Nov 11 '20 23:11 alandonovan