This repository has been archived by the owner. It is now read-only.

Adding standard Json AST - https://github.com/mdedetrich/scala-json-ast #28

Closed
wants to merge 7 commits into
from

Conversation

Projects
None yet
@mdedetrich

READ THIS FIRST

This discussion is about designing a library for a representation of JSON that can be shared between many different implementations of JSON parsing/pretty printing/transformation/.... As it's a SLIP (and not a SIP, so no language changes!), this shared API may eventually ship as a module that's included in the standard set of Scala libraries (e.g., it would be like a modern, minimalist scala-xml without the language support).

Everyone's welcome to join the discussion, provided your contribution is truly a contribution to the precise topic of this SLIP, as I tried to capture it above. This SLIP is not trying to solve the general problem of how to modularize the standard library, for example. I'm happy to refine my summary if needed. As always, there are plenty of other channels to discuss other (related) topics.

To ensure a productive discussion, please be kind to each other, stay as focussed as possible, and refrain from comments with very little technical content. As we are still refining this SLIP, it's too early to vote.

Thanks,
@adriaanm

@mdedetrich mdedetrich referenced this pull request Nov 4, 2015

Open

Submit for SLIP #12

@xuwei-k

This comment has been minimized.

Show comment
Hide comment

xuwei-k commented Nov 4, 2015

-1

@mdedetrich

This comment has been minimized.

Show comment
Hide comment

-1

?

@SethTisue

This comment has been minimized.

Show comment
Hide comment
@SethTisue

SethTisue Nov 4, 2015

Member

@sjrd want to weigh in on the Scala.js aspects of this, and/or call the Scala.js community's attention to it?

Member

SethTisue commented Nov 4, 2015

@sjrd want to weigh in on the Scala.js aspects of this, and/or call the Scala.js community's attention to it?

@sjrd

This comment has been minimized.

Show comment
Hide comment
@sjrd

sjrd Nov 4, 2015

Member

Sure, I already did a surface reading of the proposal. I will have a deeper look at it later.
I'll also mention the PR on the Scala.js gitter channel.

Member

sjrd commented Nov 4, 2015

Sure, I already did a surface reading of the proposal. I will have a deeper look at it later.
I'll also mention the PR on the Scala.js gitter channel.

@SethTisue

This comment has been minimized.

Show comment
Hide comment
@SethTisue

SethTisue Nov 4, 2015

Member

This question about key ordering in objects is interesting. In the proposal fast preserves key order, safe doesn't. I had expected the opposite.

If it's possible to preserve key order and still get good performance, great! But then I would expect safe to preserve it, too. (After all, the spec doesn't actually forbid order preservation.)

I imagine this will be the subject of further discussion...

Member

SethTisue commented Nov 4, 2015

This question about key ordering in objects is interesting. In the proposal fast preserves key order, safe doesn't. I had expected the opposite.

If it's possible to preserve key order and still get good performance, great! But then I would expect safe to preserve it, too. (After all, the spec doesn't actually forbid order preservation.)

I imagine this will be the subject of further discussion...

@xuwei-k

This comment has been minimized.

Show comment
Hide comment
@xuwei-k

xuwei-k Nov 4, 2015

I believe Scala stdlib does not need json library.

xuwei-k commented Nov 4, 2015

I believe Scala stdlib does not need json library.

@odersky

This comment has been minimized.

Show comment
Hide comment
@odersky

odersky Nov 4, 2015

@xuwei-k We see that sort comments a lot from the scalaz people. I must admit it is getting annoying. If you have serious concerns about a proposal let's hear them. But I am not interested in the commonly voiced opinion that no improvements should be made to the standard library. That's just obstructionism. It's like me telling you there should be no scalaz, I guess you would not like that either.

odersky commented Nov 4, 2015

@xuwei-k We see that sort comments a lot from the scalaz people. I must admit it is getting annoying. If you have serious concerns about a proposal let's hear them. But I am not interested in the commonly voiced opinion that no improvements should be made to the standard library. That's just obstructionism. It's like me telling you there should be no scalaz, I guess you would not like that either.

@hepin1989

This comment has been minimized.

Show comment
Hide comment
@hepin1989

hepin1989 Nov 4, 2015

I think a standard way to work with json would be great,there are so many json there in the scala community,and look at the https://github.com/tminglei/slick-pg,it write extends for json4s, play-json, spray-json and argonaut json,and on the play side,I once looked at the google-group,@jroper,said there maybe go with json4j in Play3.0.and the current json4s way I think it's great,share some common thing in the scala community would really help,especially for those new comers.

I think a standard way to work with json would be great,there are so many json there in the scala community,and look at the https://github.com/tminglei/slick-pg,it write extends for json4s, play-json, spray-json and argonaut json,and on the play side,I once looked at the google-group,@jroper,said there maybe go with json4j in Play3.0.and the current json4s way I think it's great,share some common thing in the scala community would really help,especially for those new comers.

@milessabin

This comment has been minimized.

Show comment
Hide comment
@milessabin

milessabin Nov 4, 2015

-1.

This is downright damaging when the FOSS community already contains several first class implementations, each with its own particular emphases and design goals, and all in use in production in systems large and small.

If you want to promote Scala for Json processing, get behind those libraries. By all means fork or create another one if none of the existing ones exactly meet your needs, but then recognize that you're doing this because one size doesn't fit all, which is precisely why promoting one as a so-called "standard" is harmful.

-1.

This is downright damaging when the FOSS community already contains several first class implementations, each with its own particular emphases and design goals, and all in use in production in systems large and small.

If you want to promote Scala for Json processing, get behind those libraries. By all means fork or create another one if none of the existing ones exactly meet your needs, but then recognize that you're doing this because one size doesn't fit all, which is precisely why promoting one as a so-called "standard" is harmful.

@lihaoyi

This comment has been minimized.

Show comment
Hide comment
@lihaoyi

lihaoyi Nov 4, 2015

We see that sort comments a lot from the scalaz people. I must admit it is getting annoying.

+1. It's totally unconstructive, and adds nothing to discussion.

It's even more annoying because oftentimes the objections come out before the exact thing being proposed has even been decided, which is totally ridiculous because by that point you clearly haven't even considered the proposal (because there isn't any yet!)


@hepin1989 OTOH this won't actually solve your use case. This is purely for interop in JSON ASTs, and doesn't provide any facilities for parsing or serializing.

IMHO that makes it considerably less useful. I don't find myself passing around parsed but un-converted JSON ASTs around enough for it to be a hassle; most of my work involves going straight from String -> case class or case class -> string, but maybe other people have different usage patterns.

Do people find themselves mangling, passing around and transforming parsed-by-untyped JSON ASTs between different libraries often?

lihaoyi commented Nov 4, 2015

We see that sort comments a lot from the scalaz people. I must admit it is getting annoying.

+1. It's totally unconstructive, and adds nothing to discussion.

It's even more annoying because oftentimes the objections come out before the exact thing being proposed has even been decided, which is totally ridiculous because by that point you clearly haven't even considered the proposal (because there isn't any yet!)


@hepin1989 OTOH this won't actually solve your use case. This is purely for interop in JSON ASTs, and doesn't provide any facilities for parsing or serializing.

IMHO that makes it considerably less useful. I don't find myself passing around parsed but un-converted JSON ASTs around enough for it to be a hassle; most of my work involves going straight from String -> case class or case class -> string, but maybe other people have different usage patterns.

Do people find themselves mangling, passing around and transforming parsed-by-untyped JSON ASTs between different libraries often?

@seanparsons

This comment has been minimized.

Show comment
Hide comment
@seanparsons

seanparsons Nov 4, 2015

As the maintainer of Argonaut I don't see a huge benefit to Scala itself in there being an inbuilt JSON library, mostly because it runs the usual risk of not changing once first created. Which means it can be a bit of a lowest common denominator implementation that gets rusty really quickly.

I agree with @lihaoyi that it's very rare to need to convert between implementations of ASTs, even to the point where I can't recall a time where I've actually had to do that.

It's also worth mentioning that the JSON standard as I think it stands now calls for arbitrary precision numbers which was a serious piece of work in Argonaut to support in an efficient way, which is why some of this gets hard really quickly.

As the maintainer of Argonaut I don't see a huge benefit to Scala itself in there being an inbuilt JSON library, mostly because it runs the usual risk of not changing once first created. Which means it can be a bit of a lowest common denominator implementation that gets rusty really quickly.

I agree with @lihaoyi that it's very rare to need to convert between implementations of ASTs, even to the point where I can't recall a time where I've actually had to do that.

It's also worth mentioning that the JSON standard as I think it stands now calls for arbitrary precision numbers which was a serious piece of work in Argonaut to support in an efficient way, which is why some of this gets hard really quickly.

@odersky

This comment has been minimized.

Show comment
Hide comment
@odersky

odersky Nov 4, 2015

@milessabin You are making a political argument, not a technical one. By
the same yardstick I could argue that TypeLevel should not contain specs2
because users should choose freely between it and ScalaTest or one of the
other available libraries. We have had this discussion before, and decided
we will go with a batteries included model. Recycling this debate on
individual SLIPs is obstructionist IMO.

[Some more thoughts]
This in no way endorses the present SLIP which might be promising or not; I have
not studied it. But I reject the argument that we should not even consider adding
a JSON parser to the standard library, in particular since the one we have
is so sorely lacking. Neither need the standard library stay monolithic and
unchangeable. We are talking about modules that can be updated at their own
pace, with not all sharing the same stability guarantees as the core.

On Wed, Nov 4, 2015 at 7:12 PM, Sean Parsons notifications@github.com
wrote:

As the maintainer of Argonaut I don't see a huge benefit to Scala itself
in there being an inbuilt JSON library, mostly because it runs the usual
risk of not changing once first created. Which means it can be a bit of a
lowest common denominator implementation that gets rusty really quickly.

I agree with @lihaoyi https://github.com/lihaoyi that it's very rare to
need to convert between implementations of ASTs, even to the point where I
can't recall a time where I've actually had to do that.

It's also worth mentioning that the JSON standard as I think it stands now
calls for arbitrary precision numbers which was a serious piece of work in
Argonaut to support in an efficient way, which is why some of this gets
hard really quickly.


Reply to this email directly or view it on GitHub
#28 (comment).

Martin Odersky
EPFL and Typesafe

odersky commented Nov 4, 2015

@milessabin You are making a political argument, not a technical one. By
the same yardstick I could argue that TypeLevel should not contain specs2
because users should choose freely between it and ScalaTest or one of the
other available libraries. We have had this discussion before, and decided
we will go with a batteries included model. Recycling this debate on
individual SLIPs is obstructionist IMO.

[Some more thoughts]
This in no way endorses the present SLIP which might be promising or not; I have
not studied it. But I reject the argument that we should not even consider adding
a JSON parser to the standard library, in particular since the one we have
is so sorely lacking. Neither need the standard library stay monolithic and
unchangeable. We are talking about modules that can be updated at their own
pace, with not all sharing the same stability guarantees as the core.

On Wed, Nov 4, 2015 at 7:12 PM, Sean Parsons notifications@github.com
wrote:

As the maintainer of Argonaut I don't see a huge benefit to Scala itself
in there being an inbuilt JSON library, mostly because it runs the usual
risk of not changing once first created. Which means it can be a bit of a
lowest common denominator implementation that gets rusty really quickly.

I agree with @lihaoyi https://github.com/lihaoyi that it's very rare to
need to convert between implementations of ASTs, even to the point where I
can't recall a time where I've actually had to do that.

It's also worth mentioning that the JSON standard as I think it stands now
calls for arbitrary precision numbers which was a serious piece of work in
Argonaut to support in an efficient way, which is why some of this gets
hard really quickly.


Reply to this email directly or view it on GitHub
#28 (comment).

Martin Odersky
EPFL and Typesafe

@lihaoyi

This comment has been minimized.

Show comment
Hide comment
@lihaoyi

lihaoyi Nov 4, 2015

Here's a few more opinions:

  • scala-offheap should have nothing to do with this. Zero. Nada. Making your core library dependent on a zero-user super-experimental library for no particular gain seems silly. JSON isn't FlatBuffers or protobufs, it's generally accepted you won't be able to just mmap something and start using it.
  • What's the exact treatment of numbers? Ints? Longs? Doubles/ BigNums? Strings? In Javascript people always parse doubles, but I've seen various JSON libraries parse different things.
  • Scala.js isn't just about js.Arrays, we have js.Dictionarys too; would we encode the JSON dicts as those? That wouldn't work with the fast/safe distinction, because Javascript objects are un-ordered and that's that.
  • In Scala.js you'd typically want to wrap "raw" Javascript objects coming from JSON.parse. This is partly for performance, partly due to the fact that other JS libraries will be giving you these things. You hardly ever will want to parse them yourself into your own AST if you can help it.
  • Nobody uses any of the listed libraries in Scala.js, and this proposal as discussed won't be of any use to anyone using Scala.js AFAICT. Cross compiling the AST would be of course trivial, but I do not see any use case as proposed (and I do think Scala.js needs a nice way of handling JSON!)
  • This could very well live outside the standard lib and get adopted by various libraries. In particular, the consumers of this AST are few enough that it's actually feasible to talk to all of them and get them to try and migrate. This is in contrast to e.g. collections where the consumers are every-scala-programmer-ever.

lihaoyi commented Nov 4, 2015

Here's a few more opinions:

  • scala-offheap should have nothing to do with this. Zero. Nada. Making your core library dependent on a zero-user super-experimental library for no particular gain seems silly. JSON isn't FlatBuffers or protobufs, it's generally accepted you won't be able to just mmap something and start using it.
  • What's the exact treatment of numbers? Ints? Longs? Doubles/ BigNums? Strings? In Javascript people always parse doubles, but I've seen various JSON libraries parse different things.
  • Scala.js isn't just about js.Arrays, we have js.Dictionarys too; would we encode the JSON dicts as those? That wouldn't work with the fast/safe distinction, because Javascript objects are un-ordered and that's that.
  • In Scala.js you'd typically want to wrap "raw" Javascript objects coming from JSON.parse. This is partly for performance, partly due to the fact that other JS libraries will be giving you these things. You hardly ever will want to parse them yourself into your own AST if you can help it.
  • Nobody uses any of the listed libraries in Scala.js, and this proposal as discussed won't be of any use to anyone using Scala.js AFAICT. Cross compiling the AST would be of course trivial, but I do not see any use case as proposed (and I do think Scala.js needs a nice way of handling JSON!)
  • This could very well live outside the standard lib and get adopted by various libraries. In particular, the consumers of this AST are few enough that it's actually feasible to talk to all of them and get them to try and migrate. This is in contrast to e.g. collections where the consumers are every-scala-programmer-ever.
@milessabin

This comment has been minimized.

Show comment
Hide comment
@milessabin

milessabin Nov 4, 2015

@odersky the very idea of a "standard" is a political one. This whole enterprise is political and it's appropriate to respond to it in kind.

@odersky the very idea of a "standard" is a political one. This whole enterprise is political and it's appropriate to respond to it in kind.

@SethTisue

This comment has been minimized.

Show comment
Hide comment
@SethTisue

SethTisue Nov 4, 2015

Member

If you want to promote Scala for Json processing, get behind those libraries

The code in this SLIP will literally be behind those libraries...!

Member

SethTisue commented Nov 4, 2015

If you want to promote Scala for Json processing, get behind those libraries

The code in this SLIP will literally be behind those libraries...!

@densh

This comment has been minimized.

Show comment
Hide comment
@densh

densh Nov 4, 2015

scala-offheap should have nothing to do with this. Zero. Nada. Making your core library dependent on a zero-user super-experimental library for no particular gain seems silly.

I wholeheartedly agree. It's way too young for getting into committed relationship with standard library.

densh commented Nov 4, 2015

scala-offheap should have nothing to do with this. Zero. Nada. Making your core library dependent on a zero-user super-experimental library for no particular gain seems silly.

I wholeheartedly agree. It's way too young for getting into committed relationship with standard library.

@milessabin

This comment has been minimized.

Show comment
Hide comment
@milessabin

milessabin Nov 4, 2015

@SethTisue no, it will be a competitor with code which already exists in those libraries. I know you've see the relevant xkcd cartoon.

@SethTisue no, it will be a competitor with code which already exists in those libraries. I know you've see the relevant xkcd cartoon.

@fommil

This comment has been minimized.

Show comment
Hide comment
@fommil

fommil Nov 4, 2015

Why not just pick one that already works well? spray-json in really good. Importantly it uses arbitrary precision for numbers and reminds us all that JSON maps do not preserve ordering. If you really must create a new AST (replacing this one tiny file of awesome) then please do not forget these points.

The only change I would ask of spray-json is to remove the Root* traits, and it the marshalling layer to perhaps move all implicits to the companion of JsonFormat.

Also it has a super fast parser. And years of production usage with lots of tiny performance tweaks along the way.

Plus it has spray-json-shapeless ;-)

Let's just get everybody to standardise on spray-json

fommil commented Nov 4, 2015

Why not just pick one that already works well? spray-json in really good. Importantly it uses arbitrary precision for numbers and reminds us all that JSON maps do not preserve ordering. If you really must create a new AST (replacing this one tiny file of awesome) then please do not forget these points.

The only change I would ask of spray-json is to remove the Root* traits, and it the marshalling layer to perhaps move all implicits to the companion of JsonFormat.

Also it has a super fast parser. And years of production usage with lots of tiny performance tweaks along the way.

Plus it has spray-json-shapeless ;-)

Let's just get everybody to standardise on spray-json

@fommil

This comment has been minimized.

Show comment
Hide comment
@fommil

fommil Nov 4, 2015

(Actually if the marshallers returned Either instead of throwing exceptions, it would significantly speed up Option marshalling, but that's a rather minor technical point).

fommil commented Nov 4, 2015

(Actually if the marshallers returned Either instead of throwing exceptions, it would significantly speed up Option marshalling, but that's a rather minor technical point).

@milessabin

This comment has been minimized.

Show comment
Hide comment
@milessabin

milessabin Nov 4, 2015

@fommil There's no need to "pick one" and give it an official stamp ... if you want to use or advocate for spray-json, that's fine, but there's no grounds for your (or anyone else's) preferences being legislated for.

@fommil There's no need to "pick one" and give it an official stamp ... if you want to use or advocate for spray-json, that's fine, but there's no grounds for your (or anyone else's) preferences being legislated for.

@fommil

This comment has been minimized.

Show comment
Hide comment
@fommil

fommil Nov 4, 2015

BTW is this just about the AST or an entire JSON framework? Parsing/marshalling/querying/validation/etc?

I don't believe the standard library even has the power to provide what is needed for a proper marshalling framework. We need shapeless for that.

fommil commented Nov 4, 2015

BTW is this just about the AST or an entire JSON framework? Parsing/marshalling/querying/validation/etc?

I don't believe the standard library even has the power to provide what is needed for a proper marshalling framework. We need shapeless for that.

@odersky

This comment has been minimized.

Show comment
Hide comment
@odersky

odersky Nov 4, 2015

the very idea of a "standard" is a political one. This whole enterprise is political and it's appropriate to respond to it in kind.

It's good to know how you feel about it. My "politics" is to make Scala more accessible for newcomers and people who do not want to spend a lot of time trying out different alternatives. I stand by that. I am very happy co-opting a best of breed library that's out there; no need to reinvent the wheel. But I am deadset against leaving stdlib with the crippled JSON parser it has, and also leaving it without one.

odersky commented Nov 4, 2015

the very idea of a "standard" is a political one. This whole enterprise is political and it's appropriate to respond to it in kind.

It's good to know how you feel about it. My "politics" is to make Scala more accessible for newcomers and people who do not want to spend a lot of time trying out different alternatives. I stand by that. I am very happy co-opting a best of breed library that's out there; no need to reinvent the wheel. But I am deadset against leaving stdlib with the crippled JSON parser it has, and also leaving it without one.

@lvicentesanchez

This comment has been minimized.

Show comment
Hide comment
@lvicentesanchez

lvicentesanchez Nov 4, 2015

Unless I'm missing the point of this SLIP, this is just a proposal for a unified JSON AST and its goal is to allow different JSON libraries to interop between them; that is... no improvements to the existing JSON parser in the scala standard library.

If the whole purpose of this SLIP is just that, I don't thing it belongs to the standard library, not even as a module. If anything it's a library that the authors of the different JSON libraries might or might not want to use.

As an user I can't even imagine a use case where I would want to have two different JSON libraries in my classpath and rely on a unified JSON AST to interop between them.

Unless I'm missing the point of this SLIP, this is just a proposal for a unified JSON AST and its goal is to allow different JSON libraries to interop between them; that is... no improvements to the existing JSON parser in the scala standard library.

If the whole purpose of this SLIP is just that, I don't thing it belongs to the standard library, not even as a module. If anything it's a library that the authors of the different JSON libraries might or might not want to use.

As an user I can't even imagine a use case where I would want to have two different JSON libraries in my classpath and rely on a unified JSON AST to interop between them.

@SethTisue

This comment has been minimized.

Show comment
Hide comment
@SethTisue

SethTisue Nov 4, 2015

Member

is this just about the AST or an entire JSON framework?

This SLIP is about the ASTs only.

I suggest any broader discussion happen on scala-debate instead, but maybe those cows are already out of the barn.

Member

SethTisue commented Nov 4, 2015

is this just about the AST or an entire JSON framework?

This SLIP is about the ASTs only.

I suggest any broader discussion happen on scala-debate instead, but maybe those cows are already out of the barn.

@SethTisue

This comment has been minimized.

Show comment
Hide comment
@SethTisue

SethTisue Nov 4, 2015

Member

I am deadset against leaving stdlib with the crippled JSON parser it has, and also leaving it without one.

Do you want to issue a separate call for a SLIP that addresses that? This one doesn't.

Member

SethTisue commented Nov 4, 2015

I am deadset against leaving stdlib with the crippled JSON parser it has, and also leaving it without one.

Do you want to issue a separate call for a SLIP that addresses that? This one doesn't.

@tperrigo

This comment has been minimized.

Show comment
Hide comment
@tperrigo

tperrigo Nov 4, 2015

-1. I'm in agreement with @milessabin, and agree with @fommil that a proper JSON framework should provide parsing/marshalling/validation/etc (which, as he points out, would require something like shapeless...Circe, eg, is making excellent progress on this front). I simply don't see this as something that needs to be (or should be) in the standard library.

tperrigo commented Nov 4, 2015

-1. I'm in agreement with @milessabin, and agree with @fommil that a proper JSON framework should provide parsing/marshalling/validation/etc (which, as he points out, would require something like shapeless...Circe, eg, is making excellent progress on this front). I simply don't see this as something that needs to be (or should be) in the standard library.

@odersky

This comment has been minimized.

Show comment
Hide comment
@odersky

odersky Nov 4, 2015

@SethTisue Yes, I think the current JSON parser is overdue for a replacement. It would be good to call for proposals.

odersky commented Nov 4, 2015

@SethTisue Yes, I think the current JSON parser is overdue for a replacement. It would be good to call for proposals.

@lihaoyi

This comment has been minimized.

Show comment
Hide comment
@lihaoyi

lihaoyi Nov 4, 2015

I don't think the current proposal is useful enough to warrant inclusion, but I think a good AST + a fast parser + a serializer, with the useful knobs (e.g. serialize with proper indentation) would be useful enough to put in there.

lihaoyi commented Nov 4, 2015

I don't think the current proposal is useful enough to warrant inclusion, but I think a good AST + a fast parser + a serializer, with the useful knobs (e.g. serialize with proper indentation) would be useful enough to put in there.

@odersky

This comment has been minimized.

Show comment
Hide comment
@odersky

odersky Nov 4, 2015

Almost every language library has a decent JSON parser. Ours sucks, as do quite a few modules that were introduced in the very early days of Scala when every contribution was welcome, because there was so little of it and the community was tiny anyway. It's time we corrected that, don't you think?

odersky commented Nov 4, 2015

Almost every language library has a decent JSON parser. Ours sucks, as do quite a few modules that were introduced in the very early days of Scala when every contribution was welcome, because there was so little of it and the community was tiny anyway. It's time we corrected that, don't you think?

@odersky

This comment has been minimized.

Show comment
Hide comment
@odersky

odersky Nov 4, 2015

And, sure, maybe with shapeless one can do a more fancy thing with lots of typelevel stuff. That's no reason not to strive for something that's simple and works efficiently and can be put in the standard library.

odersky commented Nov 4, 2015

And, sure, maybe with shapeless one can do a more fancy thing with lots of typelevel stuff. That's no reason not to strive for something that's simple and works efficiently and can be put in the standard library.

@lihaoyi

This comment has been minimized.

Show comment
Hide comment
@lihaoyi

lihaoyi Nov 4, 2015

There are multiple stages that JSON-support can reach

  1. Just AST; bring your own parser and serializer and operations
  2. AST + operations + parser + serializer, bring your own case-class-mapper/etc.
  3. AST + operations + parser + serializer + case-class-mapper a.l.a. shapeless, uPickle, etc.
  4. Fancy operations: lenses, traversals, zippers, etc.

IMHO 1 is not useful enough to warrant inclusion, 2 is useful and straightforward to implement with current technology, 3 is probably just a bit too experimental, even as the author of a library which does exactly that, I would not want my code in the std lib just yet. 4 I have no experience with.

lihaoyi commented Nov 4, 2015

There are multiple stages that JSON-support can reach

  1. Just AST; bring your own parser and serializer and operations
  2. AST + operations + parser + serializer, bring your own case-class-mapper/etc.
  3. AST + operations + parser + serializer + case-class-mapper a.l.a. shapeless, uPickle, etc.
  4. Fancy operations: lenses, traversals, zippers, etc.

IMHO 1 is not useful enough to warrant inclusion, 2 is useful and straightforward to implement with current technology, 3 is probably just a bit too experimental, even as the author of a library which does exactly that, I would not want my code in the std lib just yet. 4 I have no experience with.

@duncan

This comment has been minimized.

Show comment
Hide comment
@duncan

duncan Nov 4, 2015

A standard built-in awesome JSON parser would be awesome. Yes, there's room for other parsers that do things beyond a threshold, for example all the stuff you can do with shapeless. But personally, I'm finding that I'm happier with projects that stick with the basics than with those that go too far down the rabbit hole.

duncan commented Nov 4, 2015

A standard built-in awesome JSON parser would be awesome. Yes, there's room for other parsers that do things beyond a threshold, for example all the stuff you can do with shapeless. But personally, I'm finding that I'm happier with projects that stick with the basics than with those that go too far down the rabbit hole.

@wheaties

This comment has been minimized.

Show comment
Hide comment
@wheaties

wheaties Nov 4, 2015

-1

We already had an XML library. It was arguably good and bad at the same time (Daniel's Anti-XML was an improvement.) That said, look how it all worked out. It was removed. There's already a ton of approaches to make with so many libraries to "solve" this issue. I have to agree with @milessabin.

That said, I see there being nothing wrong with having a Scala official JSON library the same as the Scala XML library, separate. I could +1 that approach.

wheaties commented Nov 4, 2015

-1

We already had an XML library. It was arguably good and bad at the same time (Daniel's Anti-XML was an improvement.) That said, look how it all worked out. It was removed. There's already a ton of approaches to make with so many libraries to "solve" this issue. I have to agree with @milessabin.

That said, I see there being nothing wrong with having a Scala official JSON library the same as the Scala XML library, separate. I could +1 that approach.

@ktoso

This comment has been minimized.

Show comment
Hide comment
@ktoso

ktoso Nov 4, 2015

One datapoint (so please refrain from ranting about the JSR process. thank you.) which may be interesting to keep in mind is that the JDK is rolling in their own JSON types and parser.
This may be meaningful in the long-term plan we foresee if we'd roll our own types (or rather not).

https://json-processing-spec.java.net/nonav/releases/1.0/fcs/javadocs/index.html
https://json-processing-spec.java.net/

It might be just that this answers the inter-op story?
I'm not sure if it will make it into JDK9 though.


UPDATE: The one considered for inclusion in JDK9 was JEP198 but seems like it may slip from the release. // pointed out by @soc, thanks! #28 (comment)


UPDATE 2: The JSON JEP was dropped from JDK9, in theory it could come back for JDK10, but that's many many years in front of us. (details: #28 (comment) )

ktoso commented Nov 4, 2015

One datapoint (so please refrain from ranting about the JSR process. thank you.) which may be interesting to keep in mind is that the JDK is rolling in their own JSON types and parser.
This may be meaningful in the long-term plan we foresee if we'd roll our own types (or rather not).

https://json-processing-spec.java.net/nonav/releases/1.0/fcs/javadocs/index.html
https://json-processing-spec.java.net/

It might be just that this answers the inter-op story?
I'm not sure if it will make it into JDK9 though.


UPDATE: The one considered for inclusion in JDK9 was JEP198 but seems like it may slip from the release. // pointed out by @soc, thanks! #28 (comment)


UPDATE 2: The JSON JEP was dropped from JDK9, in theory it could come back for JDK10, but that's many many years in front of us. (details: #28 (comment) )

@mandubian

This comment has been minimized.

Show comment
Hide comment
@mandubian

mandubian Nov 4, 2015

A standard AST in Scala could be useful to stop having multiple AST in many projects...
Specific language syntaxes, certainly not, XML was a failure...
A parser I'm not so sure, there are different approaches to parsing, different requirements... "Official" lightweight extensions to (de)serialize Json AST using existing parsers from the Java world, why not...
More about it, I don't see what: pure functional approaches to manipulate Json AST that are present in several libs will stay there or it would mean pure functional structures have appeared in standard...

A standard AST in Scala could be useful to stop having multiple AST in many projects...
Specific language syntaxes, certainly not, XML was a failure...
A parser I'm not so sure, there are different approaches to parsing, different requirements... "Official" lightweight extensions to (de)serialize Json AST using existing parsers from the Java world, why not...
More about it, I don't see what: pure functional approaches to manipulate Json AST that are present in several libs will stay there or it would mean pure functional structures have appeared in standard...

@djspiewak

This comment has been minimized.

Show comment
Hide comment
@djspiewak

djspiewak Nov 4, 2015

The current JSON situation in the standard library is really awful. It's basically worse than nothing, since it's useless to anyone who knows what it is and how it works, and it entraps newcomers into using it "because it's there".

I think in general, standard library things more often skew towards entrapment than usefulness. This is not an indictment of Scala; most standard libraries fall into the same bucket (see Java). I think this is @milessabin's general point. If we take this observation to its natural conclusion, then the correct response is to apply the scala.xml treatment to JSON: delete with prejudice and don't replace.

With that said, I can see merits in the desire for a "batteries included" language. If you're a newcomer to Scala, you don't want to just fiddle around with encoding your own List and writing merge sort. You probably want to solve an actual problem with actual tasks that you would expect an actual language to have a solution for in the standard library. Like JSON parsing.

Exactly what set of tasks falls into the category of "batteries that should be included" is a very subjective question, and there is no way to answer this definitively in a way that will satisfy everyone. Personally, I don't want JSON parsing in the standard library any more than I want XML. But that's just me.

I certainly don't agree with the nuclear "freeze/delete the standard library" viewpoint.

The current JSON situation in the standard library is really awful. It's basically worse than nothing, since it's useless to anyone who knows what it is and how it works, and it entraps newcomers into using it "because it's there".

I think in general, standard library things more often skew towards entrapment than usefulness. This is not an indictment of Scala; most standard libraries fall into the same bucket (see Java). I think this is @milessabin's general point. If we take this observation to its natural conclusion, then the correct response is to apply the scala.xml treatment to JSON: delete with prejudice and don't replace.

With that said, I can see merits in the desire for a "batteries included" language. If you're a newcomer to Scala, you don't want to just fiddle around with encoding your own List and writing merge sort. You probably want to solve an actual problem with actual tasks that you would expect an actual language to have a solution for in the standard library. Like JSON parsing.

Exactly what set of tasks falls into the category of "batteries that should be included" is a very subjective question, and there is no way to answer this definitively in a way that will satisfy everyone. Personally, I don't want JSON parsing in the standard library any more than I want XML. But that's just me.

I certainly don't agree with the nuclear "freeze/delete the standard library" viewpoint.

@odersky

This comment has been minimized.

Show comment
Hide comment
@odersky

odersky Nov 4, 2015

@wheaties XML was put into a separate module, and that's what I would propose for a new JSON library as well. The days of a monolithic standard library are over. I would have loved to have Anti-XML as an alternative, in fact was trying to convince @djspiewak to contribute it, but it never got that far.

odersky commented Nov 4, 2015

@wheaties XML was put into a separate module, and that's what I would propose for a new JSON library as well. The days of a monolithic standard library are over. I would have loved to have Anti-XML as an alternative, in fact was trying to convince @djspiewak to contribute it, but it never got that far.

@fommil

This comment has been minimized.

Show comment
Hide comment
@fommil

fommil Nov 4, 2015

@odersky shapeless isn't needed for a fast parser, formatter or marshalling framework as spray-json demonstrates. But it is needed to automatically derive marshallers for user domain objects, as in spray-json-shapeless. Various other libraries have chosen to do this with their own macros, but fundamentally they are all doing the same thing. Without this, users need to provide a lot of boilerplate if they want these things to be defined and verified at compile time.

fommil commented Nov 4, 2015

@odersky shapeless isn't needed for a fast parser, formatter or marshalling framework as spray-json demonstrates. But it is needed to automatically derive marshallers for user domain objects, as in spray-json-shapeless. Various other libraries have chosen to do this with their own macros, but fundamentally they are all doing the same thing. Without this, users need to provide a lot of boilerplate if they want these things to be defined and verified at compile time.

@dickwall

This comment has been minimized.

Show comment
Hide comment
@dickwall

dickwall Nov 16, 2015

Contributor

There is clearly enough interest in this issue to keep it open for discussion. Further more, it will be assigned SLIP number 0028 and enter a public review phase.

Please assemble an expert group to answer questions like:

  • Scope of the effort (just a lightweight minimal JSON representation vs best of breed parser or anything in between)
  • Should it go into the Scala name space? Separate namespace?

It seems like accepted wisdom is that this would not be a core library implementation but more likely a platform library level.

Please read the full thread to familiarize yourself thoroughly with this issue.

Contributor

dickwall commented Nov 16, 2015

There is clearly enough interest in this issue to keep it open for discussion. Further more, it will be assigned SLIP number 0028 and enter a public review phase.

Please assemble an expert group to answer questions like:

  • Scope of the effort (just a lightweight minimal JSON representation vs best of breed parser or anything in between)
  • Should it go into the Scala name space? Separate namespace?

It seems like accepted wisdom is that this would not be a core library implementation but more likely a platform library level.

Please read the full thread to familiarize yourself thoroughly with this issue.

@kiritsuku

This comment has been minimized.

Show comment
Hide comment
@kiritsuku

kiritsuku Nov 16, 2015

On 11/16/2015 06:35 PM, Dick Wall wrote:

Please read the full thread to familiarize yourself thoroughly with
this issue.

What if I read it but already forgot what is mentioned there, do I have
to reread everything again?

On 11/16/2015 06:35 PM, Dick Wall wrote:

Please read the full thread to familiarize yourself thoroughly with
this issue.

What if I read it but already forgot what is mentioned there, do I have
to reread everything again?

@mdedetrich

This comment has been minimized.

Show comment
Hide comment
@mdedetrich

mdedetrich Nov 16, 2015

Please assemble an expert group to answer questions like

I think the current list of active contributors/developers as listed here https://github.com/json4s/json4s-ast/blob/master/build.sbt#L41-L115 should be part of the group, how exactly does this process work?

Scope of the effort (just a lightweight minimal JSON representation vs best of breed parser or anything in between)

Personally I don't have an issue with implementing a parser, I would probably use some stripped down version of https://github.com/non/jawn to target the AST

Should it go into the Scala name space? Separate namespace?

I would argue that the AST should go into the scala namespace (something like scala.json) so it makes it obvious that people should target the AST, however the parser can sit in its own namespace if desirable.

It seems like accepted wisdom is that this would not be a core library implementation but more likely a platform library level.

Agreed

Please assemble an expert group to answer questions like

I think the current list of active contributors/developers as listed here https://github.com/json4s/json4s-ast/blob/master/build.sbt#L41-L115 should be part of the group, how exactly does this process work?

Scope of the effort (just a lightweight minimal JSON representation vs best of breed parser or anything in between)

Personally I don't have an issue with implementing a parser, I would probably use some stripped down version of https://github.com/non/jawn to target the AST

Should it go into the Scala name space? Separate namespace?

I would argue that the AST should go into the scala namespace (something like scala.json) so it makes it obvious that people should target the AST, however the parser can sit in its own namespace if desirable.

It seems like accepted wisdom is that this would not be a core library implementation but more likely a platform library level.

Agreed

@densh

This comment has been minimized.

Show comment
Hide comment
@densh

densh Nov 17, 2015

Scope of the effort (just a lightweight minimal JSON representation vs best of breed parser or anything in between)

In my opinion, we need to agree on AST first and then make an open call for parser proposals with clearly defined requirements & quality evaluation criteria (i.e. shared set of benchmarks and unit tests). Similarly to dotty's strawman proposals regarding collections redesign this can be used to obtain the best parser through competition and constructive discussion.

densh commented Nov 17, 2015

Scope of the effort (just a lightweight minimal JSON representation vs best of breed parser or anything in between)

In my opinion, we need to agree on AST first and then make an open call for parser proposals with clearly defined requirements & quality evaluation criteria (i.e. shared set of benchmarks and unit tests). Similarly to dotty's strawman proposals regarding collections redesign this can be used to obtain the best parser through competition and constructive discussion.

@SethTisue

This comment has been minimized.

Show comment
Hide comment
@SethTisue

SethTisue Nov 17, 2015

Member

I agree that parsing should be a separate SLIP.

Member

SethTisue commented Nov 17, 2015

I agree that parsing should be a separate SLIP.

@SethTisue

This comment has been minimized.

Show comment
Hide comment
@SethTisue

SethTisue Nov 18, 2015

Member

how exactly does this process work?

@mdedetrich this is all brand new, we don't have a structure for how these "expert groups" should work. (it isn't even clear we need to formalize that.) actually the phrase "expert group" doesn't even occur in https://github.com/scala/slip/blob/master/README.md . but "Help this is all too informal!" does :-)

if you want to discuss that, we could talk on a new issue at https://github.com/scala/slip/issues. (and mention @dickwall since I think this concept of expert groups is his?)

Member

SethTisue commented Nov 18, 2015

how exactly does this process work?

@mdedetrich this is all brand new, we don't have a structure for how these "expert groups" should work. (it isn't even clear we need to formalize that.) actually the phrase "expert group" doesn't even occur in https://github.com/scala/slip/blob/master/README.md . but "Help this is all too informal!" does :-)

if you want to discuss that, we could talk on a new issue at https://github.com/scala/slip/issues. (and mention @dickwall since I think this concept of expert groups is his?)

@mdedetrich

This comment has been minimized.

Show comment
Hide comment
@mdedetrich

mdedetrich Nov 18, 2015

@mdedetrich this is all brand new, we don't have a structure for how these "expert groups" should work. (it isn't even clear we need to formalize that.) actually the phrase "expert group" doesn't even occur in https://github.com/scala/slip/blob/master/README.md . but "Help this is all too informal!" does :-)

I guess I am kind of unclear what goes on from here. Like am I meant to wait for this "expert group" to formalise so we can discuss the different adjustments for the JSON AST? Are we going to be using json4s-ast as a reference?

I just don't want to be doing work that is wasted. @dickwall , can you help out here?

@mdedetrich this is all brand new, we don't have a structure for how these "expert groups" should work. (it isn't even clear we need to formalize that.) actually the phrase "expert group" doesn't even occur in https://github.com/scala/slip/blob/master/README.md . but "Help this is all too informal!" does :-)

I guess I am kind of unclear what goes on from here. Like am I meant to wait for this "expert group" to formalise so we can discuss the different adjustments for the JSON AST? Are we going to be using json4s-ast as a reference?

I just don't want to be doing work that is wasted. @dickwall , can you help out here?

@SethTisue

This comment has been minimized.

Show comment
Hide comment
@SethTisue

SethTisue Nov 19, 2015

Member

I guess I am kind of unclear what goes on from here

I think we got this sorted out on Gitter: https://gitter.im/scala/slip?at=564d337675346dea413351d2

Member

SethTisue commented Nov 19, 2015

I guess I am kind of unclear what goes on from here

I think we got this sorted out on Gitter: https://gitter.im/scala/slip?at=564d337675346dea413351d2

@eed3si9n

This comment has been minimized.

Show comment
Hide comment
@eed3si9n

eed3si9n Dec 5, 2015

Member

As an sbt maintainer, I'd like the new JSON AST to be designed with long-term binary compatibility in mind. One way to do this might be to declare support window like 24 months and/or putting the major version number in the package name like scala.data.json1, and. Having long-term stability allows other libraries to depend on it more reliably. (e.g. Scala 2.9+ promising binary compatibility across minor releases)

Another requirement the AST should go for, I think, is literal round trip ability from a legal JSON string -> AST -> JSON string. Having more requirements on AST than the JSON spec doesn't interfere with the interop of JSON strings. If not the round trip (aka insertion order), at least we should aim for idempotency from the input type A -> String. Having referential transparency comes with many benefits, like caching.

(I was away from internet when this discussion was active, but I figured I'd put my 2 cents)

Member

eed3si9n commented Dec 5, 2015

As an sbt maintainer, I'd like the new JSON AST to be designed with long-term binary compatibility in mind. One way to do this might be to declare support window like 24 months and/or putting the major version number in the package name like scala.data.json1, and. Having long-term stability allows other libraries to depend on it more reliably. (e.g. Scala 2.9+ promising binary compatibility across minor releases)

Another requirement the AST should go for, I think, is literal round trip ability from a legal JSON string -> AST -> JSON string. Having more requirements on AST than the JSON spec doesn't interfere with the interop of JSON strings. If not the round trip (aka insertion order), at least we should aim for idempotency from the input type A -> String. Having referential transparency comes with many benefits, like caching.

(I was away from internet when this discussion was active, but I figured I'd put my 2 cents)

@mdedetrich

This comment has been minimized.

Show comment
Hide comment
@mdedetrich

mdedetrich Dec 6, 2015

As an sbt maintainer, I'd like the new JSON AST to be designed with long-term binary compatibility in mind. One way to do this might be to declare support window like 24 months and/or putting the major version number in the package name like scala.data.json1, and. Having long-term stability allows other libraries to depend on it more reliably. (e.g. Scala 2.9+ promising binary compatibility across minor releases)

Definitely agreed. It kind of kills the point if the library is not stable. Regarding versioning, I am not sure if versioning by packages is needed, especially if the release cycle will be so long.

Another requirement the AST should go for, I think, is literal round trip ability from a legal JSON string -> AST -> JSON string. Having more requirements on AST than the JSON spec doesn't interfere with the interop of JSON strings. If not the round trip (aka insertion order), at least we should aim for idempotency from the input type A -> String. Having referential transparency comes with many benefits, like caching.

The current design does deal with this problem

Just letting you guys know, that I have been incredibly busy lately, however I will have a lot more free time in the holiday period to work on this

As an sbt maintainer, I'd like the new JSON AST to be designed with long-term binary compatibility in mind. One way to do this might be to declare support window like 24 months and/or putting the major version number in the package name like scala.data.json1, and. Having long-term stability allows other libraries to depend on it more reliably. (e.g. Scala 2.9+ promising binary compatibility across minor releases)

Definitely agreed. It kind of kills the point if the library is not stable. Regarding versioning, I am not sure if versioning by packages is needed, especially if the release cycle will be so long.

Another requirement the AST should go for, I think, is literal round trip ability from a legal JSON string -> AST -> JSON string. Having more requirements on AST than the JSON spec doesn't interfere with the interop of JSON strings. If not the round trip (aka insertion order), at least we should aim for idempotency from the input type A -> String. Having referential transparency comes with many benefits, like caching.

The current design does deal with this problem

Just letting you guys know, that I have been incredibly busy lately, however I will have a lot more free time in the holiday period to work on this

@furaoing

This comment has been minimized.

Show comment
Hide comment
@furaoing

furaoing Feb 28, 2016

@jeffmay Huge +1, build-in language support of JSON is the common expectation of programmers with a dynamic language background (python, javascript, Ruby, etc).

@jeffmay Huge +1, build-in language support of JSON is the common expectation of programmers with a dynamic language background (python, javascript, Ruby, etc).

@mdedetrich

This comment has been minimized.

Show comment
Hide comment
@mdedetrich

mdedetrich Feb 28, 2016

Hey guys, thought I would give a little bit of an update from my side regarding whats going on. In regards to time I am about to move overseas, so I don't have as much time as I used to, I should be more free around march/april. I am also helping write a Scala book.

@SethTisue , @Ichoran would it be possible to give an update on the progress of this issue. As it currently stands

  • There is an implementation that already exists, which has been developed with feedback from (most) of the major framework providers.
  • I haven't received any extra feedback, technical or otherwise on the proposition that is outlined in the SLIP.

Should I re-namespace the current proposal, "submit" it to the community and see what they say?

Hey guys, thought I would give a little bit of an update from my side regarding whats going on. In regards to time I am about to move overseas, so I don't have as much time as I used to, I should be more free around march/april. I am also helping write a Scala book.

@SethTisue , @Ichoran would it be possible to give an update on the progress of this issue. As it currently stands

  • There is an implementation that already exists, which has been developed with feedback from (most) of the major framework providers.
  • I haven't received any extra feedback, technical or otherwise on the proposition that is outlined in the SLIP.

Should I re-namespace the current proposal, "submit" it to the community and see what they say?

@SethTisue

This comment has been minimized.

Show comment
Hide comment
@SethTisue

SethTisue Feb 29, 2016

Member

@SethTisue , @Ichoran would it be possible to give an update on the progress of this issue. As it currently stands

I think we covered this in the scala/slip room on Gitter? Perhaps you'd like to summarize for this audience?

Member

SethTisue commented Feb 29, 2016

@SethTisue , @Ichoran would it be possible to give an update on the progress of this issue. As it currently stands

I think we covered this in the scala/slip room on Gitter? Perhaps you'd like to summarize for this audience?

@mdedetrich

This comment has been minimized.

Show comment
Hide comment
@mdedetrich

mdedetrich Feb 29, 2016

@SethTisue Sure thing

The current proposed AST at https://github.com/json4s/json4s-ast is going to be repackaged so its under the scala.json package.

Again to be clear, this is not going to be released as part of Scala stdlib. Its going to be a seperate module which uses the scala namespace, which is the exact same situation as scala-xml. It will also only contain a fast and safe variation of the JSON AST. The easiest way to explain the distinction is the difference between a String (safe) and Array[Char] (fast). One is immutable and mainly used for general use, the other is lower level and fast, meant to be used for performance reasons.

There are going to be some minor modifications to the AST, which include the following. Hopefully this will be submitted by next week

  1. Constructors and converters to Byte for JNumber have been removed. Since they aren't idempotent, they are confusing.
  2. For the apply of JNumber which applies to Double, if you have a NaN or Infinity it will return a JNull in the safe version of the AST. In the fast version, this is ignored, which actually means you will get a "NaN" or "Infinity" as a string stored in JNumber. This is done for performance reasons, however there is nothing preventing parsers from manually checking the validity of the double before constructing the fast JNumber
  3. The implicit .to[T] converters have been moved out of a package object (package objects aren't meant to contain classes/object).
  4. JNumberConverter is an abstract class (from a trait), this should improve binary compatibility

@travisbrown has raised an issue with JNumber being a BigDecimal, which is that stuff like JNumber("1e2147483647").to[BigInt] takes a huge amount of time to compute and it can cause security issues if abused for this reason. Although obviously an issue, this has more to do with the limitation of BigDecimal rather than the proposed AST, and I believe the best way to solve this problem would be to create a proper lazy number library for Scala (Scala doesn't have an unbounded lazy real data type in the stdlib). Creating custom number types is obviously out of scope for the AST.

One solution would be to document this, another would be to remove the .to[BigInt]

@SethTisue Sure thing

The current proposed AST at https://github.com/json4s/json4s-ast is going to be repackaged so its under the scala.json package.

Again to be clear, this is not going to be released as part of Scala stdlib. Its going to be a seperate module which uses the scala namespace, which is the exact same situation as scala-xml. It will also only contain a fast and safe variation of the JSON AST. The easiest way to explain the distinction is the difference between a String (safe) and Array[Char] (fast). One is immutable and mainly used for general use, the other is lower level and fast, meant to be used for performance reasons.

There are going to be some minor modifications to the AST, which include the following. Hopefully this will be submitted by next week

  1. Constructors and converters to Byte for JNumber have been removed. Since they aren't idempotent, they are confusing.
  2. For the apply of JNumber which applies to Double, if you have a NaN or Infinity it will return a JNull in the safe version of the AST. In the fast version, this is ignored, which actually means you will get a "NaN" or "Infinity" as a string stored in JNumber. This is done for performance reasons, however there is nothing preventing parsers from manually checking the validity of the double before constructing the fast JNumber
  3. The implicit .to[T] converters have been moved out of a package object (package objects aren't meant to contain classes/object).
  4. JNumberConverter is an abstract class (from a trait), this should improve binary compatibility

@travisbrown has raised an issue with JNumber being a BigDecimal, which is that stuff like JNumber("1e2147483647").to[BigInt] takes a huge amount of time to compute and it can cause security issues if abused for this reason. Although obviously an issue, this has more to do with the limitation of BigDecimal rather than the proposed AST, and I believe the best way to solve this problem would be to create a proper lazy number library for Scala (Scala doesn't have an unbounded lazy real data type in the stdlib). Creating custom number types is obviously out of scope for the AST.

One solution would be to document this, another would be to remove the .to[BigInt]

@travisbrown

This comment has been minimized.

Show comment
Hide comment
@travisbrown

travisbrown Feb 29, 2016

Really expensive conversions: Just to clarify, my concern specifically is that library users working with JValue values don't necessarily have any idea that the underlying representation could be a BigDecimal, so there's no reason for them to suspect that to[BigInt] is dangerous and that they need either to avoid it or to sanitize user input.

If my library includes a method that allows valid user input to result in computations that run forever and consume lots of resources (both processing and memory), I'd want that to be painfully clear in the documentation, in the method name, etc.

circe addresses this problem by having BigInt decoding fail immediately for values above a certain size (by default 218 digits). It does this by introducing a pretty minimal new numeric type that isn't intended to be a new number library—just to solve this particular set of problems in a reasonably clean way.

Representing structure vs. decoding: More generally (and probably more importantly), I think the presence of the to method here muddles up different concerns. If this is purely an AST library, I don't see why it needs this decoding-related stuff, but if it's going to include some decoding-related stuff, it should do it responsibly and in a way that isn't going to make life harder for maintainers of JSON libraries with a more complete approach to decoding.

Other clunky number stuff: I have a number of other complaints about the representation of JSON values—e.g. 1e2147483648 is a perfectly legitimate JSON number, but I have to jump through hoops if I want a safe.JValue that represents it—this doesn't work:

scala> org.json4s.ast.fast.JNumber("1e2147483648").toSafe
java.lang.NumberFormatException
  at java.math.BigDecimal.<init>(BigDecimal.java:491)
  ...

In my view this kind of thing should be hidden from the library user (which is not that difficult to do).

Fast vs. safe: I'm also just not convinced that the fast / safe distinction is a good idea. Is there any concrete evidence that the fast implementation is actually faster in any meaningful way? Some of the specific choices about the representation are also kind of surprising or confusing—e.g. I wouldn't expect anyone to guess that lookup by key would be linear in the fast package but constant-time in safe.

Lastly, there's no reason a "strictly adhering" JSON object representation can't preserve field order. What I want is a fast, safe AST that holds on to a few useful facts that aren't strictly mandated by the spec (like field order, the sign of zeroes, etc.). circe has that now, and adopting this AST representation seems like it would be a step backward in every respect.

Really expensive conversions: Just to clarify, my concern specifically is that library users working with JValue values don't necessarily have any idea that the underlying representation could be a BigDecimal, so there's no reason for them to suspect that to[BigInt] is dangerous and that they need either to avoid it or to sanitize user input.

If my library includes a method that allows valid user input to result in computations that run forever and consume lots of resources (both processing and memory), I'd want that to be painfully clear in the documentation, in the method name, etc.

circe addresses this problem by having BigInt decoding fail immediately for values above a certain size (by default 218 digits). It does this by introducing a pretty minimal new numeric type that isn't intended to be a new number library—just to solve this particular set of problems in a reasonably clean way.

Representing structure vs. decoding: More generally (and probably more importantly), I think the presence of the to method here muddles up different concerns. If this is purely an AST library, I don't see why it needs this decoding-related stuff, but if it's going to include some decoding-related stuff, it should do it responsibly and in a way that isn't going to make life harder for maintainers of JSON libraries with a more complete approach to decoding.

Other clunky number stuff: I have a number of other complaints about the representation of JSON values—e.g. 1e2147483648 is a perfectly legitimate JSON number, but I have to jump through hoops if I want a safe.JValue that represents it—this doesn't work:

scala> org.json4s.ast.fast.JNumber("1e2147483648").toSafe
java.lang.NumberFormatException
  at java.math.BigDecimal.<init>(BigDecimal.java:491)
  ...

In my view this kind of thing should be hidden from the library user (which is not that difficult to do).

Fast vs. safe: I'm also just not convinced that the fast / safe distinction is a good idea. Is there any concrete evidence that the fast implementation is actually faster in any meaningful way? Some of the specific choices about the representation are also kind of surprising or confusing—e.g. I wouldn't expect anyone to guess that lookup by key would be linear in the fast package but constant-time in safe.

Lastly, there's no reason a "strictly adhering" JSON object representation can't preserve field order. What I want is a fast, safe AST that holds on to a few useful facts that aren't strictly mandated by the spec (like field order, the sign of zeroes, etc.). circe has that now, and adopting this AST representation seems like it would be a step backward in every respect.

@mdedetrich

This comment has been minimized.

Show comment
Hide comment
@mdedetrich

mdedetrich Feb 29, 2016

Representing structure vs. decoding: More generally (and probably more importantly), I think the presence of the to method here muddles up different concerns. If this is purely an AST library, I don't see why it needs this decoding-related stuff, but if it's going to include some decoding-related stuff, it should do it responsibly and in a way that isn't going to make life harder for maintainers of JSON libraries with a more complete approach to decoding.

I don't disagree with this at all, and I wouldn't have an issue in removing it (although if we end up using something like BigInt under the covers for large numbers, .to would need to remain so it hides the underlying representation.)

I made an issue about this and didn't get an overwhelming response to remove it json4s/json4s-ast#3 (comment). I think if we leave JNumber as a BigDecimal, we can remove it (and leave it to other libraries to implement their own converters), however if we use BigInt as a fallback for larger numbers, we need to provide the functionality

I have a number of other complaints about the representation of JSON values—e.g. 1e2147483648 is a perfectly legitimate JSON number, but I have to jump through hoops if I want a safe.JValue that represents it—this doesn't work:

Can you point to the source in your code where you do this. I may end up implementing it if there isn't too much ceremony in how it works

Fast vs. safe: I'm also just not convinced that the fast / safe distinction is a good idea. Is there any concrete evidence that the fast implementation is actually faster in any meaningful way? Some of the specific choices about the representation are also kind of surprising or confusing—e.g. I wouldn't expect anyone to guess that lookup by key would be linear in the fast package but constant-time in safe.

Fast takes up much less memory (uses mutable arrays under the hood). More importantly though, it doesn't do any runtime checking and it uses the minimal available datastructure (i.e. String for a JNumber). fast is something that is typically used for parsers, but it does address other concerns. @eed3si9n needs an AST that preserves the semantics of equality under conversion (i.e. if you go from String -> JValue -> String, you get the same string back). This means to represent JObject you need to use something like an Array or List (to preserve ordering, and possible key duplicates)

Representing structure vs. decoding: More generally (and probably more importantly), I think the presence of the to method here muddles up different concerns. If this is purely an AST library, I don't see why it needs this decoding-related stuff, but if it's going to include some decoding-related stuff, it should do it responsibly and in a way that isn't going to make life harder for maintainers of JSON libraries with a more complete approach to decoding.

I don't disagree with this at all, and I wouldn't have an issue in removing it (although if we end up using something like BigInt under the covers for large numbers, .to would need to remain so it hides the underlying representation.)

I made an issue about this and didn't get an overwhelming response to remove it json4s/json4s-ast#3 (comment). I think if we leave JNumber as a BigDecimal, we can remove it (and leave it to other libraries to implement their own converters), however if we use BigInt as a fallback for larger numbers, we need to provide the functionality

I have a number of other complaints about the representation of JSON values—e.g. 1e2147483648 is a perfectly legitimate JSON number, but I have to jump through hoops if I want a safe.JValue that represents it—this doesn't work:

Can you point to the source in your code where you do this. I may end up implementing it if there isn't too much ceremony in how it works

Fast vs. safe: I'm also just not convinced that the fast / safe distinction is a good idea. Is there any concrete evidence that the fast implementation is actually faster in any meaningful way? Some of the specific choices about the representation are also kind of surprising or confusing—e.g. I wouldn't expect anyone to guess that lookup by key would be linear in the fast package but constant-time in safe.

Fast takes up much less memory (uses mutable arrays under the hood). More importantly though, it doesn't do any runtime checking and it uses the minimal available datastructure (i.e. String for a JNumber). fast is something that is typically used for parsers, but it does address other concerns. @eed3si9n needs an AST that preserves the semantics of equality under conversion (i.e. if you go from String -> JValue -> String, you get the same string back). This means to represent JObject you need to use something like an Array or List (to preserve ordering, and possible key duplicates)

@mdedetrich

This comment has been minimized.

Show comment
Hide comment
@mdedetrich

mdedetrich Mar 1, 2016

Hey guys, just letting you know that a git repo has been set up at https://github.com/mdedetrich/scala-json-ast, it also includes a gitter room, so feel free to ask questions/file issues!

Hey guys, just letting you know that a git repo has been set up at https://github.com/mdedetrich/scala-json-ast, it also includes a gitter room, so feel free to ask questions/file issues!

@mdedetrich

This comment has been minimized.

Show comment
Hide comment
@mdedetrich

mdedetrich Apr 6, 2016

Just giving everyone an update, the final design of the scala json AST is now out, the main change is that JNumber is now represented as a String (validated by a regex at runtime to make sure its a proper JSON number) with implementations of equals and hashcode to make to check for number equality.

Unless there are major objections to this last change, this will probably be the latest revision. The only thing that needs to be updated is to add some tests/benchmarks. Hashcode for the new JNumber may need to be update in later revisions to make the hash more consistent, however this won't break binary compatibliity

@SethTisue As far as I am concerned, from a design perspective the AST is pretty much done, I just want to add some tests/benchmarks to provide some greater confidence. When that is done, I think its a good time for a release?

Just giving everyone an update, the final design of the scala json AST is now out, the main change is that JNumber is now represented as a String (validated by a regex at runtime to make sure its a proper JSON number) with implementations of equals and hashcode to make to check for number equality.

Unless there are major objections to this last change, this will probably be the latest revision. The only thing that needs to be updated is to add some tests/benchmarks. Hashcode for the new JNumber may need to be update in later revisions to make the hash more consistent, however this won't break binary compatibliity

@SethTisue As far as I am concerned, from a design perspective the AST is pretty much done, I just want to add some tests/benchmarks to provide some greater confidence. When that is done, I think its a good time for a release?

@furaoing

This comment has been minimized.

Show comment
Hide comment
@furaoing

furaoing Apr 6, 2016

Great work !

Rao Fu

On Apr 6, 2016, at 08:13, Matthew de Detrich notifications@github.com wrote:

Just giving everyone an update, the final design of the scala json AST is now out, the main change is that JNumber is now represent as a String (validated by a regex at runtime to make sure its a proper JSON number) with implementations of equals and hashcode to make to check for number equality.

Unless there are major objections to this last change, this will probably be the latest revision. The only thing that needs to be updated is to add some tests/benchmarks. Hashcode for the new JNumber may need to be update in later revisions to make the hash more consistent, however this won't break binary compatibliity

@SethTisue As far as I am concerned, from a design perspective the AST is pretty much done, I just want to add some tests/benchmarks to provide some greater confidence. When that is done, I think its a good time for a release?


You are receiving this because you commented.
Reply to this email directly or view it on GitHub

furaoing commented Apr 6, 2016

Great work !

Rao Fu

On Apr 6, 2016, at 08:13, Matthew de Detrich notifications@github.com wrote:

Just giving everyone an update, the final design of the scala json AST is now out, the main change is that JNumber is now represent as a String (validated by a regex at runtime to make sure its a proper JSON number) with implementations of equals and hashcode to make to check for number equality.

Unless there are major objections to this last change, this will probably be the latest revision. The only thing that needs to be updated is to add some tests/benchmarks. Hashcode for the new JNumber may need to be update in later revisions to make the hash more consistent, however this won't break binary compatibliity

@SethTisue As far as I am concerned, from a design perspective the AST is pretty much done, I just want to add some tests/benchmarks to provide some greater confidence. When that is done, I think its a good time for a release?


You are receiving this because you commented.
Reply to this email directly or view it on GitHub

@SethTisue

This comment has been minimized.

Show comment
Hide comment
@SethTisue

SethTisue Apr 6, 2016

Member

I think its a good time for a release?

sure — ideally in tandem with an updated version of the SLIP itself, reflecting the latest discussions and design thinking?

Member

SethTisue commented Apr 6, 2016

I think its a good time for a release?

sure — ideally in tandem with an updated version of the SLIP itself, reflecting the latest discussions and design thinking?

@mdedetrich

This comment has been minimized.

Show comment
Hide comment
@mdedetrich

mdedetrich Apr 17, 2016

@SethTisue The final design is now pretty much complete, I am going to update the SLIP itself to reflect the most recent changes (amongst other things). There may be more to be done (documentation, some more tests for confidence?) but I think its at a point where it can get a good looking at

@SethTisue The final design is now pretty much complete, I am going to update the SLIP itself to reflect the most recent changes (amongst other things). There may be more to be done (documentation, some more tests for confidence?) but I think its at a point where it can get a good looking at

@chaotic3quilibrium

This comment has been minimized.

Show comment
Hide comment
@chaotic3quilibrium

chaotic3quilibrium Apr 17, 2016

How can I find a URL to the SLIP?
On Apr 17, 2016 8:08 AM, "Matthew de Detrich" notifications@github.com
wrote:

@SethTisue https://github.com/SethTisue The final design is now pretty
much complete, I am going to update the SLIP itself to reflect the most
recent changes (amongst other things). There may be more to be done
(documentation, some more tests for confidence?) but I think its at a point
where it can get a good looking at


You are receiving this because you commented.
Reply to this email directly or view it on GitHub
#28 (comment)

How can I find a URL to the SLIP?
On Apr 17, 2016 8:08 AM, "Matthew de Detrich" notifications@github.com
wrote:

@SethTisue https://github.com/SethTisue The final design is now pretty
much complete, I am going to update the SLIP itself to reflect the most
recent changes (amongst other things). There may be more to be done
(documentation, some more tests for confidence?) but I think its at a point
where it can get a good looking at


You are receiving this because you commented.
Reply to this email directly or view it on GitHub
#28 (comment)

@mdedetrich

This comment has been minimized.

Show comment
Hide comment
@mdedetrich

mdedetrich Apr 17, 2016

@chaotic3quilibrium Its in the SLIP, but for reference its located at https://github.com/mdedetrich/scala-json-ast

I will edit the title so its easier for people to spot

@chaotic3quilibrium Its in the SLIP, but for reference its located at https://github.com/mdedetrich/scala-json-ast

I will edit the title so its easier for people to spot

@mdedetrich mdedetrich changed the title from Adding standard Json AST to Adding standard Json AST - https://github.com/mdedetrich/scala-json-ast Apr 17, 2016

@akkie

This comment has been minimized.

Show comment
Hide comment
@akkie

akkie Nov 26, 2016

What's the status of this proposal?

akkie commented Nov 26, 2016

What's the status of this proposal?

@mdedetrich

This comment has been minimized.

Show comment
Hide comment
@mdedetrich

mdedetrich Nov 27, 2016

@akkie Afaik, the proposal has been accepted however we are waiting for the Scala center to release their plans for the SLIP process in order for this to proceed

@SethTisue @jvican Correct me if I am wrong or if something else needs to be added

@akkie Afaik, the proposal has been accepted however we are waiting for the Scala center to release their plans for the SLIP process in order for this to proceed

@SethTisue @jvican Correct me if I am wrong or if something else needs to be added

@akkie

This comment has been minimized.

Show comment
Hide comment
@akkie

akkie Nov 27, 2016

@mdedetrich Thanks, great news!

Next SIP meeting is happening on the 29th of November. Agenda: Static SIP, SIP-24, SIP-27 and the Meta proposals SIP-28 and SIP-29.

https://twitter.com/jvican/status/798506152887730176

akkie commented Nov 27, 2016

@mdedetrich Thanks, great news!

Next SIP meeting is happening on the 29th of November. Agenda: Static SIP, SIP-24, SIP-27 and the Meta proposals SIP-28 and SIP-29.

https://twitter.com/jvican/status/798506152887730176

@mdedetrich

This comment has been minimized.

Show comment
Hide comment
@mdedetrich

mdedetrich Nov 27, 2016

@akkie I am not sure if this specific SLIP will be discussed in those meetings, those are for sips afaik

@akkie I am not sure if this specific SLIP will be discussed in those meetings, those are for sips afaik

@jvican

This comment has been minimized.

Show comment
Hide comment
@jvican

jvican Nov 28, 2016

Member

@mdedetrich Yes, correct, once the new Scala Platform process we will discuss this.
@akkie SIP meetings are meant to discuss proposals related to the Scala language, not libraries. Library changes will be discussed in our future Scala Platform meetings 😄.

Member

jvican commented Nov 28, 2016

@mdedetrich Yes, correct, once the new Scala Platform process we will discuss this.
@akkie SIP meetings are meant to discuss proposals related to the Scala language, not libraries. Library changes will be discussed in our future Scala Platform meetings 😄.

@akkie

This comment has been minimized.

Show comment
Hide comment
@akkie

akkie Nov 29, 2016

@mdedetrich @jvican Thanks for the info!

akkie commented Nov 29, 2016

@mdedetrich @jvican Thanks for the info!

@ktoso

This comment has been minimized.

Show comment
Hide comment
@ktoso

ktoso Nov 29, 2016

Can't wait to see the SPP process kick-off, we have a strong interest and would back this proposal as the combination of Lightbend's libraries (as we discussed across the Play/Lagom/Akka/Spray teams internally).

Awaiting the new process then :-)

ktoso commented Nov 29, 2016

Can't wait to see the SPP process kick-off, we have a strong interest and would back this proposal as the combination of Lightbend's libraries (as we discussed across the Play/Lagom/Akka/Spray teams internally).

Awaiting the new process then :-)

@mdedetrich

This comment has been minimized.

Show comment
Hide comment
@mdedetrich

mdedetrich Nov 29, 2016

@ktoso Likewise!

Note that there is already a reference implementation if you want something to code against if you find that helpful. Its highly unlikely that it will change in the future, but I don't have a complete guarantee in this area

@ktoso Likewise!

Note that there is already a reference implementation if you want something to code against if you find that helpful. Its highly unlikely that it will change in the future, but I don't have a complete guarantee in this area

@SethTisue

This comment has been minimized.

Show comment
Hide comment
@SethTisue

SethTisue Nov 30, 2016

Member

The SLIP process has been replaced by the new Scala Platform Process. Matthew has moved this proposal there: https://contributors.scala-lang.org/t/scala-json-ast-sp-proposal/175

Member

SethTisue commented Nov 30, 2016

The SLIP process has been replaced by the new Scala Platform Process. Matthew has moved this proposal there: https://contributors.scala-lang.org/t/scala-json-ast-sp-proposal/175

@SethTisue SethTisue closed this Nov 30, 2016

@scala scala locked and limited conversation to collaborators Nov 30, 2016

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.